Skip to content

Commit 209707e

Browse files
mchwangclaude
andauthored
F6a: push the rewritten head behind a durable marker (#153)
* F6a: push the rewritten head behind a durable marker prepare-merge now pushes a rebased head whose review is still clear over exactly the PR head it rewrote, before running command checks. A schema v18 marker (tasks.push_in_progress) is written before the first external write and cleared only after a read of the branch settles the outcome; while it exists, merge, runner admission, status changes, rebases and other pushes refuse. Preparation and a tracked startup job settle an interrupted push. GitBranchPusher gains an exact expected-head lease and a branch read; remotePair reports the task PR's branch. Part of #22 (F6a). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Settle pushes cancel-safely and wait for the pushed head Push settlement reads now abort only on shutdown, so a cancel can no longer strand the marker; the in-run read is bounded by the remaining preparation deadline. Startup settlement runs under the verified runner lock and closes the PRs of a task it closed. After a push, preparation waits (bounded) until GitHub's PR API reports the pushed head, and a move seen then is reviewed from the pushed head. Adds regressions for each, plus production push wiring and server startup wiring tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Wait for the pushed head even when the base moves GitHub can report a moved base while its PR API still shows the pre-push head. The visibility wait now ends only when the head changes, so the review is never refreshed back onto a head the branch no longer holds. The test remote's branch read honours its abort signal, so a cancel that stopped push settlement would fail a test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * State the pushed-head wait bound exactly The visibility wait starts no inspection after 30 seconds, so it lasts at most 30 seconds plus one inspection. Say so in the code and the docs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Settle a stranded push on cancel; wait for the PR head after a refusal A cancel requested while no preparation runs and a push marker remains now starts the tracked settlement job, so the task and its PRs close without a restart. After a refused push, preparation waits for GitHub's PR API to report the branch's new head before reviewing it, as it does after a push. Renames settleAtStartup to settlePendingPush and corrects the architecture doc's not-built list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Bind a post-push visibility failure; test startup settlement faithfully A preparation that fails waiting for GitHub to show the pushed head now binds its failure to the post-push task state, so it is not reported as stale at once. The startup-settlement server test now builds the state a crash leaves (a push marker plus a pending cancel) instead of having the fake job cancel the task itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Retry push settlement on a repeated cancel; test the push boundary Repeating a cancel that waits on an unsettled push now reads the branch again instead of refusing, so the task can close without a restart. Adds push-boundary regressions for a pending cancel and a changed snapshot, and documents the retry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Trace a moved PR head through the whole rewrite lineage After a push or a refusal, a moved PR head is now reviewed by tracing history from the reviewed head and its rewrite lineage, nearest first, so an earlier rewrite codeboost already pushed is reviewed against its own base instead of the original one. Any failure after the push or refusal is now bound to the post-push task state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test the push boundary's state check and every settlement bound Adds regressions for a plan revision applied at the push boundary (only the task state version changes, so that check is load-bearing), for the visibility wait ending at the preparation deadline, for the settlement read's deadline bound, and for shutdown aborting a startup settlement. States in the docs that a killed push may still land later, harmlessly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Cover cancel-safe settlement, admission and shutdown reads Adds regressions for a cancel during the next preparation's push settlement, attempt admission and reassignment refusing while a push is in flight, and shutdown starting no branch read. The push-boundary cancel test now cancels through the coordinator while the push reads the remote, as production does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test that a failed push is reported as current Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Bind a failed push to the state its own settlement left A failed push's settlement read can take up to 30 seconds. The failure now adopts the task state version that clearing the marker produced only when nothing else wrote during the read; otherwise it stays bound to the claim's version, so a plan revision applied meanwhile marks it stale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Bind every push exit to the state that push left Success, refusal and settlement now share one rule: the version clearing the push marker produced counts only when nothing else wrote since the claim; otherwise a later failure stays bound to the claim's version, so a plan revision applied while the push was in flight marks it stale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Refuse checks after a push if the task changed while it was in flight After the pushed head appears, preparation now requires the task state, review version and snapshot to be exactly what the push left before any cmd: check runs, so a plan revision or review edit applied during the push fails it instead of being adopted. The publish guard returns to its rebase-only form: publishing cannot overlap a push by status, which the docs now state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Describe the pusher's expected-head rule in its doc comments Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test the push claim's admission guards and the busy settlement skip Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot: capture the claim version, finalize at Git start, reserve settlement - The push claim uses the task state version captured before the asynchronous trust refresh, so a reassignment during it refuses. - Production returns the last guard as the pusher's synchronous finalizer, closing the gap between validation and the Git push. - A settlement read keeps the process-settlement reserve inside its limit and does not start with less than that reserve left. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * State that a deadline-ended push leaves its marker unread When the preparation's own deadline ends the push, less than Git's settlement reserve is left, so no branch read starts and the marker stays, as for an unreadable branch. The docs, the code comment and the test now say so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Name schema version 18 in the architecture doc Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot: a branch read never waits on an escaped pipe holder Production readBranch ran Git without allowUnsettledReturn, so a process that left Git's group while holding its output could hold the push settlement read, startup recovery, a repeated cancel or shutdown without bound. A read changes nothing, so it now returns failed after the bounded drain, and the push marker stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot: bound only escaped pipe holders on branch reads allowUnsettledReturn also stopped waiting for processes still in Git's group, which a read has no durable ownership of. A new process-group option, boundEscapedOutput, bounds only the wait for a process outside the group that holds the output; group members are still waited for, fail-closed. Branch reads use it. Tests cover both cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Say which branch-read case the settlement bound excludes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Name both unbounded subprocess cases, and an inspection's full settle time Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Bound cat-file reads like ls-remote; correct stale rebaser doc line cat-file only reads, so a process that left Git's group but holds its output no longer holds fetchCommits or the push's head check: the call fails after the pipe drain. The reserve comment now lists the Git calls that still wait (fetch, update-ref, push). The pre-merge-rebase doc no longer says no production request starts the rebaser. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot: review a collaborator head found after an unknown push outcome When the push's read-back fails because a collaborator moved the branch, the settlement read now returns moved and preparation reviews that head (review-required, attribution and approvals refreshed), as after a refusal, instead of failing with the read-back error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address Copilot: recheck the push binding before reviewing a refused or moved push A plan revision or review edit that landed during the wait for the new head (or the fetch) was adopted when the observed head was recorded. The binding the push left is now checked before and after the fetch, as on the pushed path; such a change fails the preparation as stale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test the binding check after the fetch; document the stale outcome Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test that the binding check before the fetch runs first Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Refuse a push whose post-rebase review reload adopted a change (F6a, #170) Since #170 the review reload after a rebase awaits a history worker. A reassignment or review edit landing during it was adopted into the push's binding, so the push could run under a state the caller never checked. The task and review the rebase left are now checked again after the reload. Regressions for both cases fail without the check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Guard every preparation reload and report its refusals as stale (F6a, #170) Since #170 each review reload in a preparation awaits a history worker. The reload after a command check adopted a reassignment into readiness, and every reload recorded the post-await state before its guard ran, so a refusal was reported as current rather than stale. A guarded reload now checks the state it must still match before recording the view; every reload uses it. Regression for the command-check reload, and stale assertions on every reload refusal; both fail without the change. Both gaps also exist on main since #170. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Bind preparation failures as current or stale by their cause (F6a, #170) - A failure of the review reload after a rebase (a worker timeout) is bound to the state the rebase left, so it is reported as current. - A command check that a reassignment settled as stale no longer adopts the reassigned state, so its failure is reported as stale (pre-existing since F5). - Regressions for both, and for a change request added while a check runs, which is refused as stale rather than adopted as review-required. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Refuse a command check whose context changed after it settled (F6a, #170) runner.settled() resolves only after the runner releases the check's workspace. A reassignment or plan change in that window left the attempt completed and was adopted into readiness. The check's state is now adopted only while its context equals the current one; otherwise the preparation is refused as stale. The test fixture's failing load now rejects after the real load settles, as a worker failure would, so no abandoned load rejects after the test. Docs note that a refresh's closing reload is not guarded and why. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Give the preparation reload rules their own doc section Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Restore the blank line before the push table Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Refuse a rewritten-head push once shutdown has begun, and kill every escaped test process Server shutdown sets the runner's admission flag in its first step but closes the pre-merge coordinator only after the HTTP drain. The push boundary and its failure-path branch read now check the runner's flag, so no push or new GitHub read starts in that interval. The cat-file escape test appends each holder's PID, so both detached sleeps are killed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Scope the shutdown comment on the push failure path to its own read Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 49f03db commit 209707e

20 files changed

Lines changed: 1618 additions & 108 deletions

‎agents/process-group.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ export interface ProcessGroupOptions {
4949
* Callers without durable ownership wait fail-closed instead of returning while unknown work is still live.
5050
*/
5151
readonly allowUnsettledReturn?: boolean;
52+
/**
53+
* Bound only the wait for the output pipes once the whole group is gone: a process that left the group but still holds
54+
* them makes the call return failed (`ESTDIOHELD`) after the pipe drain, instead of waiting for it. Members of the
55+
* group are still waited for, fail-closed. For callers whose command changes nothing (a read), so a process that
56+
* escaped the group cannot affect anything they decide.
57+
*/
58+
readonly boundEscapedOutput?: boolean;
5259
}
5360

5461
const DEFAULT_GRACE_MS = 5_000;
@@ -94,7 +101,8 @@ export const drainProcessGroup = (group: ProcessGroup): Promise<boolean> => drai
94101
* returns control to the event loop. On abort or at the deadline the whole group gets SIGTERM, then SIGKILL after the
95102
* grace period, so a child that ignores SIGTERM is still bounded. The promise settles only after the leader has
96103
* exited and every other member of its group is gone, unless a caller with durable recovery ownership explicitly
97-
* allows an unsettled return. Never rejects: failures are in the outcome, shaped like `runDocker`'s, so `status` is a
104+
* allows an unsettled return. `boundEscapedOutput` bounds only the wait for a process outside the group that holds the
105+
* output pipes. Never rejects: failures are in the outcome, shaped like `runDocker`'s, so `status` is a
98106
* number only when the command ran to completion.
99107
*/
100108
export function runInProcessGroup(file: string, args: readonly string[],
@@ -175,7 +183,7 @@ export function runInProcessGroup(file: string, args: readonly string[],
175183
options.signal?.removeEventListener('abort', onAbort);
176184
void drainGroup(group, options.allowUnsettledReturn === true).then(async drained => {
177185
let stdioTimer: ReturnType<typeof setTimeout> | undefined;
178-
if (options.allowUnsettledReturn) {
186+
if (options.allowUnsettledReturn || options.boundEscapedOutput) {
179187
await Promise.race([closed, new Promise<void>(done => { stdioTimer = setTimeout(done, STDIO_CLOSE_MS); })]);
180188
} else await closed;
181189
// Cleared so a finished call never keeps the process alive.

‎docs/architecture.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ Writing standard: plain language, ISO 24495-1:2023
4242
- **Build state:**
4343
- **In use:** plan and linking library, SQLite store, review screen, plan display/import/authoring screen, Ask (questions to a Claude agent), guarded merge with merge-queue support, ranked Issues screen, agent isolation boundary, and the single-runner lock.
4444
- **In use for a non-demo review with a `github` block:** planning. `/api/plan/suggestions` and `/api/plan/drafts` ask Claude, through lane D, for suggestion cards or a whole next plan revision (#117, #124). Nothing becomes a revision until you apply it.
45-
- **In use with an opt-in `runner` block in `review.json`:** the whole loop after planning. `start` and `resume` on `/api/runner` run a task's plan item by item with a Claude agent (#91). They require current plan-item approvals, bind both the task state and review version, recheck approvals before each later item, and refuse when a completed prefix no longer ends at the task's current head (#107). After an out-of-scope pause, an approved amended plan can reconcile the completed prefix and resume only the unfinished suffix (#88, PR #134). When a run ends, codeboost publishes the task's pull request, a draft if the task needs a person (#103). `prepare-merge` refreshes the published PR's exact base/head, rebases through the durable F3/F4 path, recomputes attribution and approvals, and runs allowlisted `cmd:` checks in a credential-free read-only container bound to the resulting head (#22). Approve & merge merges that pull request (#121). Cancelling a task closes its pull requests (#111). These are API actions; the screen has no buttons for them yet.
46-
- **Partly built for pre-merge automation:** the trusted local rebase engine records one-to-one commit mappings, verifies the resulting checkout byte for byte, owns its subprocesses durably, and recovers interrupted work (#22, PR #136). Its production conflict path gives a sandboxed agent only the bounded conflict files and imports only audited results for both foreign and plan-owned commits (PRs #138, #140 and #144). `prepare-merge` now invokes that path after current pull-request base/head reads, then refreshes attribution and approvals and runs exact-head `cmd:` checks; required-check refresh, the already-fixed rerun, rewritten-head push, and guarded merge handoff remain for F6.
47-
- **Not built:** screens for the runner actions, remote push/check refresh, review rounds, and the Queue and Learning screens.
45+
- **In use with an opt-in `runner` block in `review.json`:** the whole loop after planning. `start` and `resume` on `/api/runner` run a task's plan item by item with a Claude agent (#91). They require current plan-item approvals, bind both the task state and review version, recheck approvals before each later item, and refuse when a completed prefix no longer ends at the task's current head (#107). After an out-of-scope pause, an approved amended plan can reconcile the completed prefix and resume only the unfinished suffix (#88, PR #134). When a run ends, codeboost publishes the task's pull request, a draft if the task needs a person (#103). `prepare-merge` refreshes the published PR's exact base/head, rebases through the durable F3/F4 path, recomputes attribution and approvals, pushes a rewritten head whose review is still clear over exactly the PR head it rewrote, and runs allowlisted `cmd:` checks in a credential-free read-only container bound to the resulting head (#22). Approve & merge merges that pull request (#121). Cancelling a task closes its pull requests (#111). These are API actions; the screen has no buttons for them yet.
46+
- **Partly built for pre-merge automation:** the trusted local rebase engine records one-to-one commit mappings, verifies the resulting checkout byte for byte, owns its subprocesses durably, and recovers interrupted work (#22, PR #136). Its production conflict path gives a sandboxed agent only the bounded conflict files and imports only audited results for both foreign and plan-owned commits (PRs #138, #140 and #144). `prepare-merge` now invokes that path after current pull-request base/head reads, refreshes attribution and approvals, pushes the rewritten head behind a durable push marker (F6a), and runs exact-head `cmd:` checks; required-check refresh, the already-fixed rerun, and the one-click guarded merge handoff remain for F6b.
47+
- **Not built:** screens for the runner actions, the required-check refresh and one-click merge handoff (F6b), review rounds, and the Queue and Learning screens.
4848

4949
## Terms used
5050

@@ -357,7 +357,7 @@ The diagram shows the designed transitions. With the `runner` block, running, ne
357357

358358
### Stored data
359359

360-
All state is in one SQLite file, opened with WAL and full synchronization. Each write takes an immediate write lock. The schema version is `PRAGMA user_version`, and migrations run in explicit steps (version 17 today); an unknown version fails.
360+
All state is in one SQLite file, opened with WAL and full synchronization. Each write takes an immediate write lock. The schema version is `PRAGMA user_version`, and migrations run in explicit steps (version 18 today); an unknown version fails.
361361

362362
| Group | Tables | Notes |
363363
|---|---|---|
@@ -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). This branch adds F5: current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6 owns rewritten-head push, fresh required-check refresh, the already-fixed rerun, and exact-pair guarded-merge handoff. |
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. This branch adds F6a: the durable push of the rewritten head. F6b owns fresh required-check refresh, the already-fixed rerun, and the one-click exact-pair guarded-merge handoff; F6c owns common CI. |
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. |
@@ -419,7 +419,7 @@ These are the decisions that shape the structure. The full list, with the reason
419419

420420
## Known limits
421421

422-
- `prepare-merge` invokes the trusted rebase and sandboxed conflict machinery, refreshes the review, and runs head-bound `cmd:` checks. Until F6 of #22 lands, it does not push a rewritten head, wait for fresh required checks, rerun the already-fixed check, or hand the exact pair to guarded merge.
422+
- `prepare-merge` invokes the trusted rebase and sandboxed conflict machinery, refreshes the review, pushes the rewritten head, and runs head-bound `cmd:` checks. Until F6b of #22 lands, it does not wait for fresh required checks, rerun the already-fixed check, or hand the exact pair to guarded merge; a person clicks Merge once GitHub's checks on the pushed head pass.
423423
- Linking is line-based. It cannot see an unrelated edit inside a declared file; only the review agent and you can catch that.
424424
- The container's guarantees are those of Docker. The agent can always reach its own vendor account.
425425
- `cmd:` acceptance checks run during `prepare-merge`, and their passing evidence is bound to the exact reviewed head. A plan item without current passing evidence still blocks merging.

0 commit comments

Comments
 (0)