fix(sandcastle): Close the gaps Claude Review found after #236 merged

[mpaulosky]

Why

#236 replaced Sandcastle's local merger with one draft PR per issue. PR Auto-Merge merged it after its review cap, and Claude Review's pass over its final head then opened six threads. All six hold up:

• Agent-edited hooks could still run on the host. fix(sandcastle): Open one pull request per issue instead of merging locally #236 turned hooks off only for the host's own git. Sandcastle runs git worktree add on the host too. That runs post-checkout from the new worktree, which is the branch's copy of .github/hooks, and a branch can be one the agents wrote in an earlier run.
• Detected config tampering didn't stop the run. A GitConfigChangedError was logged as one failed issue. sandbox.close() still ran Sandcastle's git under the tampered config, and the run could end with "All done."
• Pushed commits were never checked for the sandbox's secrets. The Claude token or API key is in the sandbox, and an agent could commit it in a log, fixture or snapshot.
• The PR warning missed most ways to skip a test project. Only a RequiresDocker change was flagged. A Docker-backed package, IsTestProject=false, a ProjectReference, or a deleted test project slipped through. The comparison also used origin/main, a ref the agents can move.
• Issue titles could close other issues. A title like "Fix Bump Radzen.Blazor from 5.3.5 to 5.5.5 #12 regression" went unchanged into the PR title (the squash subject) and the body.
• Stopped issues vanished from the planner. An issue that stopped earlier in a run was in neither of the planner's lists, so the issues it blocks looked unblocked.

What changed

• lib/shell.mts, main.mts: protectHostGit() runs first. It adds core.hooksPath=/dev/null and core.fsmonitor=false through GIT_CONFIG_COUNT/GIT_CONFIG_KEY_n/GIT_CONFIG_VALUE_n, after any entries already there. Every git process on the host inherits these, Sandcastle's included. They don't reach the sandbox: Sandcastle passes it only what .sandcastle/.env names.
• lib/build.mts, main.mts: if a pipeline hits GitConfigChangedError, it leaves its sandbox open and rethrows. main.mts rethrows the first such error after the round, so the run exits non-zero with the warning.
• lib/scan.mts, lib/build.mts, lib/sandbox-env.mts: before the push, the host scans what it would publish, from main's commit to the checked commit. That is every commit message and every added line, from git log -p --cc -U0, so merge commits are read too. Context and removed lines are left out, so main's fake test tokens don't stop unrelated branches. The scan looks for the values .sandcastle/.env gives the sandbox and for Claude and GitHub token shapes. A match stops the issue as secret-in-commits, with a comment that doesn't quote it.
• lib/check.mts: the warning now covers MSBuild lines matching RequiresDocker|IsTestProject|Testcontainers|Aspire.Hosting.Testing|Playwright|ProjectReference, and files deleted or renamed away under tests/.
• lib/branches.mts, lib/build.mts: fetchMain() returns main's commit as git ls-remote origin reports it. The merge, commitsAhead, the warning and the secret scan all compare with that commit id, not with refs/remotes/origin/main.
• lib/publish.mts: withoutClosingKeywords now also rewrites the issue title, in the PR title and body, and full github.com/.../issues/n and /pull/n URLs.
• lib/prompts.mts, plan-prompt.md: the planner's second list is now the held-back issues ({{HELD_BACK_JSON}}), both in review and stopped earlier in the run, and they count as blockers.

Verification

• pnpm run test:sandcastle: the type check is clean and 99 of 99 tests pass. The new tests run against real temp repositories: a git worktree add with protectHostGit doesn't run a post-checkout hook, and the same command without it does. checkFileChanges flags a Testcontainers reference, IsTestProject=false, a deleted test project, a renamed test file and check.sh, but not an ordinary src/ package change. publishedText sees a secret added in a merge resolution, but not a fixture token in context. Other new tests cover the secret stop, the untouched sandbox on config tampering, and closing references in titles and URLs.
• The pre-push gate passed.

🤖 Generated with Claude Code

[github-actions]

Test Results Summary

485 tests  ±0   485 ✅ ±0   26s ⏱️ +2s
  9 suites ±0     0 💤 ±0 
  9 files   ±0     0 ❌ ±0 

Results for commit aafd44d. ± Comparison against base commit 2e96db0.

♻️ This comment has been updated with latest results.

[github-actions]

Claude Review

This PR tightens Sandcastle's publish path. Main is now pinned to the commit git ls-remote reports, not the agent-writable origin/main ref. Hooks and fsmonitor are turned off through GIT_CONFIG_* for every git the host runs, Sandcastle's own included. The branch's commits are scanned for sandbox secrets before the push. If an agent changes .git/config, the run stops and the sandbox is left open. checkFileChanges also flags more MSBuild lines and deleted test files. Closing references are now rewritten in PR titles and in URL form. Issues that stopped earlier in the run are held back as blockers for the planner.

I traced the git-config path. It holds even though the inner publish catch swallows a GitConfigChangedError: stop() → commentOnIssue → gh re-asserts the config and throws again, so the error reaches the outer catch and nothing is posted. Pinning base by commit id, threading base through commitsAhead/checkFileChanges, and the planner and prompt changes all look right. Squash-only merging means agents' commit messages can't close issues, so rewriting only the title and body is enough.

Verdict: mergeable once the secret scan covers merge commits. That is a real hole in a gate meant to stop a public leak, and the fix is one flag. The other two are smaller gaps worth fixing or knowingly accepting. I couldn't run git here, so the git behaviour behind these findings is from git's documented defaults:

• git log -p shows no diff for merge commits.
• git diff detects renames by default.
• Pathspecs are applied before rename detection.

I also couldn't check the Sandcastle source (node_modules isn't installed). The new comment in lib/shell.mts says Sandcastle passes the sandbox only what .sandcastle/.env names, so GIT_CONFIG_* never reaches it; I've taken that as given.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.63%. Comparing base (2e96db0) to head (aafd44d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #242   +/-   ##
=======================================
  Coverage   85.63%   85.63%           
=======================================
  Files          75       75           
  Lines        1566     1566           
  Branches      186      186           
=======================================
  Hits         1341     1341           
  Misses        190      190           
  Partials       35       35           

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Claude Review

This PR tightens the Sandcastle host in six ways. The host now merges and compares against main's commit ID from ls-remote rather than origin/main, which agents can move. Every pushed commit is scanned for sandbox secrets first. The check-file warning now also covers deleted tests/ files and more MSBuild lines. Hooks and fsmonitor are switched off for every git on the host through GIT_CONFIG_*. Closing keywords in issue titles and full GitHub URLs are rewritten. Issues that stopped earlier in the run are held back as blockers, and the run aborts when .git/config changes.

Verdict: the direction is right, but fix the first finding before merging. The handling for a changed git config misses its main case. The publish step's catch-all handler turns GitConfigChangedError into publish-failed. And a pipeline only skips close() if it saw the error itself. So Sandcastle's own git worktree remove can still run on the host under a config an agent wrote, which is exactly what this change is meant to prevent.

The secret scan has smaller gaps:

• It drops added lines whose content starts with ++ .
• It finds nothing if the host's git config forces colour.
• Replace refs (refs/replace/*) in the agent-writable .git can hide content from the host's git log/git diff.

fetchMain's ls-remote pattern matches by suffix, so another branch can be taken for main. The check-file warning can still be avoided with multi-line edits to MSBuild files.

What I relied on without being able to check here: Sandcastle isn't installed in this checkout, so I couldn't read its close() or confirm that it doesn't pass the host environment into the container. Three points come from git's documented behaviour: the -G regex is extended, ls-remote patterns match on a ref's suffix, and pack-objects ignores replace refs.