Skip to content

Commit aded16e

Browse files
mchwangclaude
andauthored
Back T6 and T12 with tests; read ruleset-only branch rules (#183)
* Test T6 check reruns and rule refreshes; read ruleset-only branches T6: a job reruns `cmd:` checks on a collaborator head once the person re-attributes and re-approves it (base unchanged), and a fresh merge inspection reads both rule sources again when base and head match. Recording real gh output for T12 showed two bugs in #rules, which made every ruleset-only branch (this repository's main among them) read as "Required branch checks could not be read.": - GitHub omits `integration_id` for a required check any app may report; that now reads as no app pin. - `branches/<b>.protected` is true when only a ruleset applies, and `/protection` then answers 404 "Branch not protected"; that answer now means no classic protection. Other 404s still fail closed. scripts/record-merge-gh.ts records the gateway's real gh calls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Replay recorded gh answers for T12 and tick T6 and T12 test/merge-recorded.test.ts drives the real parser and the merge job with GitHub's recorded answers: zero required checks (PR 104), and PR 183's two required checks pending, then passing at the same head. The job waits and merges, blocks at the time limit, and stops at once against the repository's real non-strict ruleset. Fail-closed tests cover every other 404 on a protected branch. The design plan ticks T6 and T12 with evidence; ci.md says how to record the gh fixtures again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 1: stronger non-strict test, real 404 shapes - The non-strict ruleset job test answers with pending checks, so a job that waited before reading the guard would fail it. - The hand-written gh 404 answers are built as runWithInput reports them; the "Not Found" case is the answer a token without admin rights gets on a protected branch (checked on cli/cli trunk). - T6 evidence cites the test that shows the review recomputed. - Drop a test that repeated the classic missing app_id case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 2: name the recordings test and the bug's scope exactly Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 3: zero checks pass a job only with a merge queue Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 4: show the check reruns after an earlier pass Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 5: keep only branch protection in recordings Branch reads in the gh recordings keep only name and protected (the gateway reads only protected), dropping commit authors' emails; the recorder does the same. Test titles state each 404 row's outcome, and the rule-refresh fake sends ruleset checks as GitHub does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 6: test that required reviews are ignored Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 7: require a 404 status; refuse partial recordings Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 8: assert every gh call had a recorded answer Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 9: state that the ruleset-only fix needs an admin token Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 10: say what the review-rules fake leaves out Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 11: require classic protection off; real breaking base - A "Branch not protected" 404 counts as no classic protection only when the same branch read shows protection.enabled false. The recordings keep `protection` again, from the original answers. - A new job test fails the command check only on a head holding the moved base's new file, so the base itself breaks it. - The pr merge fakes say what they leave out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 12: scope the non-admin claim, name #176's place Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Word the pr merge fake's version note as what it stands for Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 13: one accurate note for the hand-written gh answers Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 14: say the hand-written answers may carry ignored fields Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 15: give the rule-refresh fake's branch its name Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 16: pin #176's fail-closed reading with a test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review round 17: cite the job test for a missing check's block Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 42abf4a commit aded16e

13 files changed

Lines changed: 709 additions & 28 deletions

‎docs/architecture.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ These are the decisions that shape the structure. The full list, with the reason
410410
| C, K | Guarded merge, merge queue | Done |
411411
| D | Agent isolation boundary | Done. Since the isolation gate: bounded diff export (#76), task-change inspection and the runner's commit as a bounded `git bundle` (#66, PRs #85 and #90), owner-scoped recovery for Ask (#95), the plan schema for Claude (#97), gitlink mounts and the pre-launch tree check (#81, #99, PR #100), and Codex refused in every phase (#93, PR #106). The runner-contract follow-ups (#51) are closed: the last one tests export and removal through a recovery handle after a real restart (PR #109). |
412412
| E | Planning library and suggestions | Done. E4 (PR #45) added the acceptance fixtures and recorded Claude output that replays through validation and the Store. |
413-
| F | Runner | Merged: F1 (lifecycle store, coordinator, shutdown wiring, startup recovery and the lock, planning API, feedback events, and stops that wait for their commit: #53, #74, #57, #59, #60, #96); F2a–F2b (execute prompts, audit, per-item execution, real workspace, durable safety findings: #67, #68, #92, #94, #128); F2d (already-fixed check, PR opening, branch pusher, production publishing, closing PRs on cancel: #84, #101, #110, #115, #116, #119, #123); production wiring and `start`/`resume` (#91, PRs #102, #105); planning through lane D (#117, #124, PRs #118, #120, #125); merging the task's own PR (#121, PR #126); start/resume approval, review-version, recovery and completed-prefix guards (#107, PR #130); continuation after a scope pause (#88, PR #134); the F3 durable trusted local-rebase foundation (#22, PR #136); the F4 foreign-conflict engine plus production sandbox adapter (#22, PRs #138 and #140); and sandboxed owned-commit conflict resolution (#22, PR #144). F5 (PR #149) added current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6a (PR #153) added the durable push of the rewritten head. F6b (PR #161) added the one-click merge job (preparation, the required-check wait, the exact-pair guarded merge) and an already-fixed scan covering every base commit after the task's original base. F6c (PR #177) runs every T9 suite in CI and adds the nightly live Claude probes; T9 is complete (PR #178) and #22 is closed; its follow-ups are #174 and #176. T6 and T12 still have Verify gaps (design plan, Implementation Tasks). Startup now refuses a `pathIdentity` that disagrees with a probe of task storage (#162, PR #180); follow-ups #179 and #181. |
413+
| F | Runner | Merged: F1 (lifecycle store, coordinator, shutdown wiring, startup recovery and the lock, planning API, feedback events, and stops that wait for their commit: #53, #74, #57, #59, #60, #96); F2a–F2b (execute prompts, audit, per-item execution, real workspace, durable safety findings: #67, #68, #92, #94, #128); F2d (already-fixed check, PR opening, branch pusher, production publishing, closing PRs on cancel: #84, #101, #110, #115, #116, #119, #123); production wiring and `start`/`resume` (#91, PRs #102, #105); planning through lane D (#117, #124, PRs #118, #120, #125); merging the task's own PR (#121, PR #126); start/resume approval, review-version, recovery and completed-prefix guards (#107, PR #130); continuation after a scope pause (#88, PR #134); the F3 durable trusted local-rebase foundation (#22, PR #136); the F4 foreign-conflict engine plus production sandbox adapter (#22, PRs #138 and #140); and sandboxed owned-commit conflict resolution (#22, PR #144). F5 (PR #149) added current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6a (PR #153) added the durable push of the rewritten head. F6b (PR #161) added the one-click merge job (preparation, the required-check wait, the exact-pair guarded merge) and an already-fixed scan covering every base commit after the task's original base. F6c (PR #177) runs every T9 suite in CI and adds the nightly live Claude probes; T9 is complete (PR #178) and #22 is closed; its follow-ups are #174 and #176. T6 and T12 are verified, and PR #183 also fixed reading required checks that come only from a ruleset, for a token with admin rights. Startup now refuses a `pathIdentity` that disagrees with a probe of task storage (#162, PR #180); follow-ups #179 and #181. |
414414
| Hardening | Allowlisted subprocess environments | Done: every Git command (#82, #83) and every `gh` runner (#84, #86). |
415415
| Tests | Test suite reliability | Done. The full suite runs ordinary tests first and Docker-backed files serially; CI and the real-Docker gate passed on PR #132 (#129). |
416416
| G | Planning screen | Done (#145). The screen displays/imports the current plan, requests and renders a next-revision draft or suggestion cards, and applies results with revision-bound exact ambiguous replay. G4 composes the browser with the production planning setup, D-backed provider boundary, F endpoints and SQLite persistence; focused adapter, recorded-output and exact-head real-Docker planning suites cover the lower boundary. |

‎docs/designs/codeboost-plan-indexed-review.md‎

Lines changed: 8 additions & 8 deletions
Large diffs are not rendered by default.

‎docs/implementation/ci.md‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,10 @@ them. They do not call a vendor or GitHub.
8989

9090
- Recorded Claude planning output: `test/fixtures/planning/recorded/`, replayed
9191
by `test/planning-recorded.test.ts`.
92-
- GitHub (`gh`) responses: fixtures inside `test/merge.test.ts` and the other
93-
merge and publishing tests.
92+
- Recorded GitHub (`gh`) answers to the merge inspection's reads:
93+
`test/fixtures/gh/`, replayed by `test/merge-recorded.test.ts` (T12).
94+
Hand-written `gh` responses: fixtures inside `test/merge.test.ts` and the
95+
other merge and publishing tests.
9496
- Hostile issue texts: `test/fixtures/hostile-issues.json`, used by
9597
`test/execution-prompt.test.ts`.
9698

@@ -195,6 +197,26 @@ Read each file in `test/fixtures/planning/recorded/` before you commit it. No
195197
workflow records fixtures. [Planning acceptance](planning-acceptance.md#recorded-vendor-output)
196198
describes the recordings.
197199

200+
## Record the gh merge fixtures again
201+
202+
`scripts/record-merge-gh.ts` runs one fresh merge inspection of a pull request
203+
through the production gateway and the real `gh`, signed in as you, and saves
204+
every `gh` call it makes (argv, exit status, stdout, stderr) to
205+
`test/fixtures/gh/<name>.json`. `test/merge-recorded.test.ts` fails when the
206+
gateway asks GitHub something no recording holds: each test asserts that every
207+
`gh` call had a recorded answer, apart from `gh pr merge` in the job tests,
208+
which a fake answers because recording it would merge the pull request.
209+
210+
```sh
211+
node scripts/record-merge-gh.ts checks-pending 183
212+
```
213+
214+
Record only from a public repository: the answers are committed. The three
215+
recordings are `zero-checks` (pull request 104, into a branch no rule
216+
covers), and `checks-pending` and `checks-passing` (pull request 183 at one
217+
head, while its required checks ran and after they passed). Read each file
218+
before you commit it. No workflow records fixtures.
219+
198220
## Run the same checks locally
199221

200222
```sh

‎docs/implementation/guarded-merge.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ The target is resolved again for each validation pass. Both passes, and the thir
2727

2828
Merging blocks when any plan item is unreviewed or stale, an attributed file is outside that item's declared plan scope, an Ambiguous or Unplanned segment remains, a current change request is open, or a `cmd:` acceptance check lacks a passing result for the current head. Approval does not override an out-of-scope file; the plan must be amended. Historical change requests from an earlier revision or snapshot remain visible but do not block the current revision.
2929

30-
The GitHub adapter reads the current PR base/head, effective branch rulesets, classic branch protection, required check contexts and app identities, and issue cross-references. It ignores review records that are not required checks. The already-fixed check is the pre-PR check, `GhAlreadyFixedGateway` in `github/already-fixed.ts`, with the same rules (see “The check” in `pull-request-opening.md`): a closed issue, another open or merged PR that would close the issue or is linked manually, or a new commit on the base branch that mentions the issue. A PR that only mentions the issue does not count, in this repository or another; when the PR's base is not the default branch, a PR in this repository into that same base that references the issue counts too (decided 2026-09-30). The merge passes this PR, and every other PR the task records in this repository (#121), as the task's own PRs, matched by repository and number: each is excluded while open and counts once merged, as in the pre-PR check. Without a runner block only this PR is passed. Without a runner block, the commit scan starts at the PR's base commit on its base branch, and no own commits are passed. For a runner-published PR, it starts at the base-branch tip that the pre-PR already-fixed check of the task's first opening into the current base branch scanned through (each opening records its check from schema v19) when that check scanned from the task's original base, and otherwise, including a first opening recorded before v19, at the task's original base, so base commits that a pre-merge rebase brought under the PR are covered too, whichever of the task's openings the merge follows (F6b of #22, `merge-handoff.md`); the ledger's owned commits are passed as the task's own, as in the pre-PR check. Inspections that follow a submitting, queued or merged attempt's pinned pull request scan from the PR's base, as without a runner block; they only show status, since no merge is asked for while an attempt is active. Any result the check cannot complete is unknown and blocks merging, including a base branch name outside the pattern the check accepts. The check starts as soon as the PR is read, alongside the rule reads, and stops 0.65 seconds before the inspection's deadline (`MERGE_CHECK_SETTLE_MS` in `github/merge.ts`: both grace periods of the merge's `gh` runner, which the check also uses, plus a margin), so its processes settle before the inspection ends; a check stopped that way is unknown. If the rule reads fail, the check is stopped and awaited before the inspection rejects. When the PR read ends too late to leave that much time, the check is not started and is unknown. A check answer that arrives after the check was stopped is unknown, and an inspection whose caller aborted or whose deadline passed rejects even when its reads answer late. An injected check must name the merge repository. The blocker names what the check found (at most five closes, PRs and commits, by repository, number, state and short SHA, with a closing commit's SHA shortened too; never a commit message) or why it could not finish. Once this PR has merged, the check counts it as the fix, so the coordinator shows only the `pr-state` blocker, not an already-fixed one. Malformed pagination, rules without a type, malformed protection objects, missing or malformed check-app identities, and inconsistent check status/conclusion pairs fail closed, while an explicit `null` remains an unbound check. Display status is cached for five seconds and the combined inspection has a 12-second deadline. Each of the two sequential merge-validation inspections has a six-second deadline, keeping their combined validation budget below the 15-second serving request budget. A deadline aborts the underlying GitHub CLI process before releasing the shared in-flight request. A caller's own abort during the already-fixed check waits for its processes like any other `gh` call of the gateway, up to 0.4 seconds. An unreadable or incomplete rule source, a pending/missing/failing check, an already-fixed match or unknown result, a conflict, or a closed PR blocks merging.
30+
The GitHub adapter reads the current PR base/head, effective branch rulesets, classic branch protection, required check contexts and app identities, and issue cross-references. It ignores review records that are not required checks. The already-fixed check is the pre-PR check, `GhAlreadyFixedGateway` in `github/already-fixed.ts`, with the same rules (see “The check” in `pull-request-opening.md`): a closed issue, another open or merged PR that would close the issue or is linked manually, or a new commit on the base branch that mentions the issue. A PR that only mentions the issue does not count, in this repository or another; when the PR's base is not the default branch, a PR in this repository into that same base that references the issue counts too (decided 2026-09-30). The merge passes this PR, and every other PR the task records in this repository (#121), as the task's own PRs, matched by repository and number: each is excluded while open and counts once merged, as in the pre-PR check. Without a runner block only this PR is passed. Without a runner block, the commit scan starts at the PR's base commit on its base branch, and no own commits are passed. For a runner-published PR, it starts at the base-branch tip that the pre-PR already-fixed check of the task's first opening into the current base branch scanned through (each opening records its check from schema v19) when that check scanned from the task's original base, and otherwise, including a first opening recorded before v19, at the task's original base, so base commits that a pre-merge rebase brought under the PR are covered too, whichever of the task's openings the merge follows (F6b of #22, `merge-handoff.md`); the ledger's owned commits are passed as the task's own, as in the pre-PR check. Inspections that follow a submitting, queued or merged attempt's pinned pull request scan from the PR's base, as without a runner block; they only show status, since no merge is asked for while an attempt is active. Any result the check cannot complete is unknown and blocks merging, including a base branch name outside the pattern the check accepts. The check starts as soon as the PR is read, alongside the rule reads, and stops 0.65 seconds before the inspection's deadline (`MERGE_CHECK_SETTLE_MS` in `github/merge.ts`: both grace periods of the merge's `gh` runner, which the check also uses, plus a margin), so its processes settle before the inspection ends; a check stopped that way is unknown. If the rule reads fail, the check is stopped and awaited before the inspection rejects. When the PR read ends too late to leave that much time, the check is not started and is unknown. A check answer that arrives after the check was stopped is unknown, and an inspection whose caller aborted or whose deadline passed rejects even when its reads answer late. An injected check must name the merge repository. The blocker names what the check found (at most five closes, PRs and commits, by repository, number, state and short SHA, with a closing commit's SHA shortened too; never a commit message) or why it could not finish. Once this PR has merged, the check counts it as the fix, so the coordinator shows only the `pr-state` blocker, not an already-fixed one. Malformed pagination, rules without a type, malformed protection objects, malformed check-app identities, a classic check without `app_id`, and inconsistent check status/conclusion pairs fail closed. An explicit `null` remains an unbound check, and so does a ruleset check with no `integration_id`: GitHub leaves that field out for a check any app may report. GitHub reports a branch as `protected` when only a ruleset applies; its classic protection then answers a token with admin rights 404 with the message "Branch not protected", and the branch read shows `protection.enabled: false`. Only both together count as a confirmed absence of classic protection. Any other failed protection read on a protected branch blocks merging, including the 404 "Not Found" a token without admin rights gets on a branch with classic protection (checked on another repository). What such a token gets on a ruleset-only branch was not checked; if it is "Not Found", that branch still blocks merging, as unknown rules. `test/merge-recorded.test.ts` replays GitHub's recorded answers, read with an admin token, for a ruleset check with no `integration_id` and for the "Branch not protected" 404 (T12); the "Not Found" case is hand-written in `test/merge.test.ts`. Display status is cached for five seconds and the combined inspection has a 12-second deadline. Each of the two sequential merge-validation inspections has a six-second deadline, keeping their combined validation budget below the 15-second serving request budget. A deadline aborts the underlying GitHub CLI process before releasing the shared in-flight request. A caller's own abort during the already-fixed check waits for its processes like any other `gh` call of the gateway, up to 0.4 seconds. An unreadable or incomplete rule source, a pending/missing/failing check, an already-fixed match or unknown result, a conflict, or a closed PR blocks merging.
3131

3232
Automatic merging also requires strict required status checks as a server-enforced current-base policy. A client-side fetch cannot supply that guarantee. Merge-queue branches were blocked in this increment because `gh pr merge` can report a successful enqueue before the pull request is merged. Queue support has since landed (#24; see `merge-queue.md`): a queue-capable adapter tracks each attempt until GitHub confirms the merge, removal or failure, and queue rules count as the server-side current-base guard. An adapter without queue inspection still fails closed with a `merge-queue` blocker. The coordinator performs the complete gate twice without using the display cache, verifies the same base/head pair, then re-reads the local review generation immediately before invoking `gh pr merge --match-head-commit` with literal argv. Each `gh` process gets only the allowlisted variables in `github/gh-env.ts`, plus fixed settings that turn off prompts, the pager, colour and update checks. No other server variables are passed on. A timeout or abort sends the `gh` process SIGTERM, then SIGKILL after a quarter of a second, and the call returns only after that process has exited (or 0.15 s after it exits, if a process it started keeps its output open). So a merge call aborted at its 14-second deadline settles by 14.4 s. *Later (#48, #170):* a click can also be waiting on its issue-trust read, whose stop wait is 0.75 s, or on a review load, whose history worker gets 1 s after the abort before it is terminated; so it settles about 0.75 s after its deadline, or about 1 s or a little more after it when a review load was running, past the 14.5-second shutdown drain (traced, not tested); see [merge-queue.md](merge-queue.md). Stopping `gh` cannot recall a merge request it had already sent: GitHub can still apply it, so a cancelled merge is reported as unknown and its attempt stays in flight, with the action disabled. A direct attempt is settled only when GitHub shows the pull request merged; a queued attempt also settles when GitHub reports it removed or failed (`merge-queue.md`). The adapter invalidates its pre-action cache before the command and after either success or refusal. A generation guard prevents inspections started before or during the command from restoring stale cache entries afterward. After command success, the server returns “merge submitted” even if its best-effort status reload fails, and the browser keeps the action disabled until Refresh confirms GitHub state. GitHub's command refusal text is returned to the local UI, and a failed attempt also remains disabled until Refresh loads fresh state. There is no “merge anyway” path around missing atomic protection. Demo mode never constructs the coordinator, including when a gateway is injected.
3333

0 commit comments

Comments
 (0)