fix(sandcastle): Open one pull request per issue instead of merging locally

[mpaulosky]

Why

This addresses the Claude Review findings on the merged #228, which committed the Sandcastle setup. Three of them are design problems with the old plan, implement, merge loop:

• Done was decided by this run's commits. A branch went to the merger only when commits.length > 0 for this run. A re-planned issue whose branch already held finished work made no new commits, so it was never merged. An implementer that ran out of iterations without <promise>COMPLETE</promise> still had its commits merged and its issue closed.
• The merger closed issues before anything reached GitHub. It merged into the host checkout (which stays on main, where the hooks refuse commits and pushes) and then ran gh issue close, with no PR, checks or review.
• Untrusted text reached agents holding GH_TOKEN. Every comment and issue body on a Sandcastle issue went into the planner's prompt, and the implementer fetched the issue with gh. On a public repo anyone can comment, so injected instructions could steer agents that had an Issues-write token.

What changed

This ports the host-side design from mpaulosky/Articles#308, without the problems that design's own review found.

• One draft PR per issue, no merger. merge-prompt.md is gone. lib/build.mts runs each issue in its own sandbox: implement, merge origin/main, run .sandcastle/check.sh, review, then push and open a draft PR whose body ends Fixes #n. The issue closes when that PR merges through the normal checks. Anything that stops (unfinished, merge conflict, failed check, rejection, failed push) gets a comment on the issue and isn't pushed.
• Done means finished and ahead of main. The implementer must signal completion. After that the branch is checked, reviewed and published whenever git rev-list --count origin/main..HEAD is above 0, so earlier finished work still gets its PR.
• The reviewer returns a verdict. It ends with <verdict>{"approved", "summary"}</verdict> (lib/verdict.mts). A missing or malformed verdict counts as a rejection.
• Only the host talks to GitHub. lib/github.mts reads issues with the host's gh auth and keeps only issues opened by, and comments from, an OWNER, MEMBER or COLLABORATOR. The agents get the issue through prompt arguments. .env.example no longer has GH_TOKEN, and main.mts refuses to start if .sandcastle/.env sets GH_TOKEN or GITHUB_TOKEN, since Sandcastle passes every key in that file into the sandbox.
• Fixes for what Articles' review found:
  • The host runs git with -c core.hooksPath=/dev/null. core.hooksPath is .github/hooks, a path inside the worktree the agents edit, so running its pre-push hook on the host would run their code outside the sandbox. CI runs the full suite, the Docker-backed tests included.
  • Each branch merges a freshly fetched origin/main in the sandbox before the check. On a conflict the merge is aborted and the issue gets a comment.
  • Open PRs from forks (isCrossRepository) neither hold an issue back nor pass for its PR.
  • Repeated planner picks are dropped (lib/plan.mts).
  • A PR title like Add search . becomes feat: Add search, with no trailing space.
  • After the reviewer, the host compares HEAD with the commit that passed the check, not the commits the run reported, so an amend or reset is caught. If HEAD moved, the check runs again. The host pushes that exact commit by its id, without force.
• Branches: existing local branches are reused alongside origin's. When origin has the branch, the local branch is created from it, or fast-forwarded to it if it's behind.
• Host hardening (from this PR's review):
  • Sandcastle mounts the clone's .git into each sandbox, writable, so an agent could write commands into .git/config for the host's git to run. main.mts records git config --local --list before any sandbox starts, and the host refuses to run git or gh once it has changed (lib/shell.mts). gh takes --repo from the name read at the start, not from the remotes.
  • Issue comments and PR bodies have the sandbox's secrets removed: the values .sandcastle/.env gives it, and anything shaped like a Claude or GitHub token.
  • "fixes Bump Radzen.Blazor from 5.3.5 to 5.5.5 #12"-style references in the reviewer's summary are rewritten as "fixes issue 12", so only the host's Fixes #n line closes an issue.
  • A branch that changes the files deciding what check.sh runs or skips (check.sh, needs-docker.sh, discover_tests.py, package.json, or a RequiresDocker flag) is named in a WARNING at the top of its PR.
  • The planner gets the in-review issues as open blockers, since nothing reaches main between rounds, and an issue that stopped isn't rebuilt later in the same run.
• .sandcastle/check.sh (from chore(sandcastle): Run a Docker-free check.sh in the sandbox #231) is now enforced. The host runs it in the sandbox through lib/check.mts, and a passing check over uncommitted changes still fails. check.sh, CODING_STANDARDS.md and CLAUDE.md no longer call it advisory, and they say CI runs the Docker-backed suites. The implement and review prompts keep chore(sandcastle): Run a Docker-free check.sh in the sandbox #231's Docker guidance.
• test:sandcastle runs tsc --noEmit -p .sandcastle before the tests (new .sandcastle/tsconfig.json, with typescript and @types/node as dev dependencies). engines requires Node 22.18 or later, which strips the .mts types. The tests now import @ai-hero/sandcastle and zod, so .github/ci/sandcastle-tests.sh runs pnpm install --frozen-lockfile first. It uses pnpm from the PATH, or corepack's when there isn't one.

Findings checked but not changed here

• branches.mts:45, bug issues on hotfix/: already fixed by fix(sandcastle): Name bug branches fix/ instead of hotfix/ #232.
• test:sandcastle not run in CI: chore(sandcastle): Run a Docker-free check.sh in the sandbox #231 runs it from the gate and from CI's Build Solution job, through .github/ci/sandcastle-tests.sh.
• Shell blocks from issue titles: in @ai-hero/sandcastle 0.12.0, substitutePromptArgs marks the template's own !`…` blocks with \x01 before it fills in {{KEY}} arguments and strips that marker from argument values. preprocessPrompt runs only marked blocks, so a block written in an issue title or body isn't run. A new test in prompts.test.mts checks that only host-made values (BRANCH, BASE_BRANCH, TASK_ID) appear inside the prompts' shell blocks.

Verification

• bash .github/ci/sandcastle-tests.sh "": the type check is clean and 89 of 89 tests pass, including chore(sandcastle): Run a Docker-free check.sh in the sandbox #231's. New tests cover publishing earlier finished work, refusing unfinished work, merge conflicts, a reviewer that amends or resets, fork PRs, repeated picks, the trailing space in titles, the git config guard (against a real temp repo), redaction, closing keywords and the check-file warning.
• The pre-push gate passed (lints, Release build, every test project).
• No live Sandcastle run: one needs labelled issues and opens real PRs.

🤖 Generated with Claude Code

[github-actions]

Test Results Summary

485 tests   485 ✅  27s ⏱️
  9 suites    0 💤
  9 files      0 ❌

Results for commit 6803f7d.

♻️ This comment has been updated with latest results.

[codecov]

Codecov Report

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

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #236   +/-   ##
=======================================
  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 replaces Sandcastle's old plan → implement → agent-merge loop with a new pipeline built around one PR per issue. The host now reads only trusted issues and comments, using its own gh auth, and passes them to the agents in their prompts. The sandbox gets no GitHub token, and main.mts refuses to start if .env sets one. Each issue is implemented, merged with origin/main, checked with check.sh, reviewed for a JSON verdict, and checked again if the reviewer moved HEAD. The host then pushes the exact checked commit, with hooks off, and opens a draft PR that says "Fixes #n". The merger agent and its gh issue close are removed, and CI now installs with pnpm and type-checks .sandcastle with tsc.

The design is a clear improvement and is well tested at the unit level. Verdict: not ready to merge as is. Two gate gaps undercut the PR's own claims:

• The check is the agents' own script. The host runs the branch's .sandcastle/check.sh and needs-docker.sh, which the agents can edit.
• Hooks are only one way host git runs commands. The host now runs git in a repository the sandbox can probably write to, and git config can run commands too, not only hooks.

There are also two loop-level problems:

• Dependent issues get built too early. Later rounds build issues whose blockers are only in review, not on main.
• Failed issues are retried every round. An issue that fails is rebuilt, with a new failure comment each time, whenever any other issue publishes.

Caveat: the @ai-hero/sandcastle source isn't installed here, so I couldn't check two things against it. One is whether the main .git is mounted writable into the sandbox. The other is how createSandbox bases a branch that exists only on origin. Those two findings depend on that behaviour.

[github-actions]

Claude Review

This PR changes the Sandcastle loop from "an agent merges every branch into main and closes the issues" to one draft PR per issue. Only the host talks to GitHub now. It reads issues with its own gh auth and drops anything written by people other than the owner, members and collaborators. The sandbox gets no GitHub token. Each issue is implemented, merged with origin/main and checked with check.sh in the sandbox, then reviewed and checked again if the reviewer moved HEAD. The host pushes exactly the commit that passed, with hooks off and a guard that refuses git once .git/config changes, and opens a draft PR that says "Fixes #n". The split into lib/ modules is well tested and the merge gating itself looks right: unfinished, rejected, conflicting or failing work is never pushed, and drafts are skipped by pr-automerge.

Verdict: needs changes before merging. The host-protection work is the point of this PR, but it misses three cases:

• Sandcastle's own host git calls (worktree add and remove) still run hooks and config that agents can edit.
• A detected config tamper doesn't stop the run.
• Pushed commits aren't checked for the sandbox's secrets.

The rest:

• The PR warning about changes to the check's own files misses most of the ways a test project can be skipped.
• A closing keyword in the issue title can close an unrelated issue.
• Issues that stopped earlier in the run are hidden from the planner as blockers.

What I couldn't check here: the @ai-hero/sandcastle source isn't installed in this checkout, so the first finding assumes createSandbox/close use plain git worktree add/remove on the host, inheriting the process environment. I didn't run tsc or the tests.