F6a: push the rewritten head behind a durable marker

[mchwang]

Summary

F6a of #22: prepare-merge now pushes the rewritten head to the task's pull request.

• When: after a rebase whose refreshed review has no blocker, before the cmd: checks run. A rewrite that made an approval stale stays local until a person reviews it.
• What it may overwrite: exactly the PR head the rebase rewrote. GitBranchPusher.push gains expected: the lease is pinned to that commit, and any other tip refuses with nothing pushed, even a commit codeboost made. Production wires expected: from; MergeCoordinator.remotePair now reports the task PR's branch.
• Durable marker (schema v18): tasks.push_in_progress is written before the first external write and cleared only after the outcome is known (push read back, refusal, or one branch read). While it exists, merge admission, runner admission, status changes, reassignment, rebases and another push refuse, and merge status names it. A cancel waits for it and closes the task when it clears.
• Settling an unknown outcome: one branch read, bounded by 30 s and the preparation deadline; only shutdown aborts it. If it cannot run, the marker is settled by the next preparation, by a tracked job started under the verified runner lock at startup, or by that job when a cancel arrives while no preparation runs (and again on a repeated cancel). A task it closes has its PRs closed.
• GitHub's PR API lag: after a push or refusal, preparation waits (at most 30 s plus one inspection, within the deadline) until the PR reports a new head. The pushed head continues only if the task, review and snapshot are exactly what the push left; any other head is reviewed through the rewrite lineage.
• Trust: refreshed before the claim and revalidated immediately before the write, together with the task state version, review version, snapshot and cancel state.

F6b (next, stacked on this) adds the required-check wait, the already-fixed rerun and the one-click merge handoff. F6c adds common CI.

Validation

Exact pair: base 22e76a8bc90b59c8407bb78bee1c229b249b4936 (main), head 749a977c04b626b7c253310732f6c361ae16348c (current head; main merged in 143b5f6).

• current head, independent full-diff review: clean on 22e76a8..749a977 (post-merge round 8; 11 focused files, 787 of 787 tests passed; the new regression fails with its fix removed)
• intermediate checkpoint 57b7ccb: independent review clean (post-merge round 7); CI and real-Docker passed; Copilot round 5 returned 0 open findings
• intermediate checkpoint f3bc3e1: independent review clean (post-merge round 3); CI and real-Docker passed
• intermediate checkpoint 93af176, independent full-diff review: clean on 22e76a8..93af176 (post-merge round 2), including the merge integration with Size the plan import request limit from the parser bound (#146) #158, Close out Lane G status records (#145) #154 and Read an oversized body before refusing it, and close on early refusals (#157) #160; 11 focused files, 774 of 775 tests passed, the one failure a known load-sensitive F5 deadline test that passed alone; 22 of 23 mutation probes killed (the survivor guards a case where continuing is safe)
• intermediate checkpoint 143b5f6 (the merge; 93af176 changes one doc line): full non-Docker suite (CI's command), local: 74 files, 2,047 of 2,048 passed; the failure (refreshes a pull request head that moves during final authorization, a 15 s timeout at load average ~120) passed alone. Browser suite: 134 of 137 passed under load; the 3 failures passed on rerun. npm run typecheck passed
• current head, CI and real-Docker: pending
• intermediate checkpoint 93af176: CI and real-Docker passed

Earlier validated head 9e1f695 (base ae9fc3d):

• earlier head, full non-Docker suite (CI's command), local and run alone: 74 files, 2,028 passed; npm run typecheck and git diff --check passed
• current head, independent full-diff review: clean on ae9fc3d..9e1f695; 6 focused files, 341 tests passed; 7 mutation probes all caught
• current head, CI: push and pull-request runs passed; each ran 74 non-Docker files with 2,028 tests, 137 browser tests, and type checking
• current head, remote full/real-Docker gate: 81 files, 2,282 passed, 1 skipped
• local load note for this round: two overlapping full-suite runs on 58e63e0 timed out in 10 and 20 tests under a host load average of 150–200; the deadline tests that failed by assertion passed alone, and the suite passed when run alone

Earlier validated head 87dd3bf:

• full non-Docker suite (CI's command), local: 74 files, 2,025 passed; npm run typecheck and git diff --check passed
• independent full-diff review: clean on ae9fc3d..87dd3bf; 8 focused files, 527 tests passed in the review
• every new regression was shown to fail before its fix, or to fail under a mutation that removes the guarded behaviour
• local load note: several full-suite runs on intermediate heads hit 15–30 s timeouts in review.test.ts and pre-merge.test.ts under a host load average near 200; each affected file passed alone and the full suite passed on rerun. These are recorded as infrastructure, not product, failures.
• exact-head CI: push and pull-request runs passed; each ran 74 non-Docker files with 2,025 tests, 137 browser tests, and type checking
• exact-head remote full/real-Docker gate: 81 files, 2,279 passed, 1 skipped

Review ledger

Round	Head	Findings	Remediation
independent 1	12e96ba	cancel aborted settlement reads; startup settlement did not close PRs; PR API lag; wrong comment; no wiring tests	a63e5cf
independent 2	a63e5cf	base move during lag ended the wait; fake read ignored its signal	b1eaaae
independent 3	b1eaaae	wait bound stated as 30 s; really 30 s plus one inspection	9e98ad7
independent 4	9e98ad7	stranded marker on cancel; refused path re-inspected without waiting; stale doc bullet	50ba765
independent 5	50ba765	no actionable defects; noted an unbound failure and a non-production test setup	e5ce306
independent 6	e5ce306	repeated cancel could not retry settlement; push-boundary checks untested	496d4b3
independent 7	496d4b3	refusal traced from the wrong head after an earlier landed push; failure after a push reported stale	d0639c9
independent 8	d0639c9	state-version check is load-bearing (plan revisions) and untested; settlement and wait bounds untested; doc overclaimed "not pushed"	6ab346d
independent 9	6ab346d	cancel during next preparation's settlement, attempt admission, shutdown reads untested; non-production cancel setup	0be7462
independent 10	0be7462	failed-push binding untested	7cc5477
independent 11	7cc5477	failed push bound after an awaited read adopted a concurrent plan revision	cedd594
independent 12	cedd594	same on success and refusal paths	a0ecaf0
independent 13	a0ecaf0	mid-push plan revision adopted before checks; unreachable publish guard	fc26627
independent 14	fc26627	stale pusher doc comments	365cda0
independent 15	365cda0	admission guards and busy settlement skip untested	87dd3bf
independent 16	87dd3bf	no actionable defects	—
Copilot 1	87dd3bf	claim version read after the trust refresh; guard ran before an awaited gap instead of as the pusher's finalizer; settlement read did not reserve Git's settlement time	58e63e0; three failing-before regressions; threads answered and resolved
independent 17	58e63e0	Copilot fixes correct; docs claimed a read after a deadline-ended push, but none starts	9e1f695
independent 18	9e1f695	no actionable defects	—
Copilot 2	9e1f695	0 open findings; the three round-1 findings resolved; summary notes the base advance must be integrated, re-reviewed and revalidated before merge	already recorded under Gate status as a merge prerequisite
merge main	143b5f6	merged 22e76a8 (#158, #154, #160) with no conflicts	—
independent post-merge 1	143b5f6	architecture.md still named schema version 17	93af176
independent post-merge 2	93af176	no actionable defects	—
Copilot 3	93af176	the push-settlement branch read could wait without bound on a process that escaped Git's group holding its output	f3bc3e1 (reads return failed after the bounded drain; the marker stays)
independent post-merge 3	f3bc3e1	no actionable defects (noted, not changed: git push itself still waits fail-closed for an escaped holder, per the process-group contract; no realistic trigger with HTTPS and the gh credential helper)	—
Copilot 4	f3bc3e1	allowUnsettledReturn also abandoned processes still in Git's group, which nothing owns	97b6305 (new boundEscapedOutput: bounds only an escaped output holder; group members waited for)
independent post-merge 4	97b6305	three comments still claimed every branch read is bounded	230e5b6
independent post-merge 5	230e5b6	reserve comment wrong about which calls are bounded; one inspection's settle time understated	33450c8
independent post-merge 6	33450c8	cat-file reads also waited without bound on an escaped holder	57b7ccb (bounded like ls-remote; stale rebaser doc line corrected)
independent post-merge 7	57b7ccb	no actionable defects	—
Copilot 4 (previously missed)	f3bc3e1	when a collaborator moved the branch between the push and its read-back, preparation failed instead of reviewing that head	749a977 (a moved settlement is reviewed, as after a refusal)
Copilot 5	57b7ccb	0 open findings	—
independent post-merge 8	749a977	no actionable defects	—

Declined: none. One noted out-of-scope item: F5's rebase-cleanup failure binding (merged in #149) reads the task state after an awaited cleanup, the pattern fixed here for pushes; recorded as a follow-up.

Review-lesson audit

• lifecycle, cancellation, shutdown, deadline and race findings (rounds 1, 2, 4, 6, 9, 11–13, 15, Copilot 1, Copilot 3, Copilot 4 including its previously missed finding, post-merge 6): covered by Async jobs and polling and Required race regressions in AGENTS.md
• untested-claim and fail-before findings (rounds 2, 8–10, 15): covered by Review readiness ("give each claim a test that fails if the claim is false") and the Design system and plan format #1 rule of tests
• non-production test setups (rounds 5, 9): covered by Review readiness ("A test's setup must leave the state the production path would")
• documentation claims (rounds 3, 4, 8, 14, 17, post-merge 1, 4, 5): covered by Review readiness (treat every behavioural claim as something to verify)
• no new rule is needed: each finding is an instance of existing guidance

Gate status

• independent exact-head review: clean on base 22e76a8bc90b59c8407bb78bee1c229b249b4936 / head 749a977c04b626b7c253310732f6c361ae16348c (earlier: clean on ae9fc3d / 9e1f695)
• CI and real-Docker: pending on 749a977; passed on 57b7ccb, f3bc3e1, 93af176 and 9e1f695
• Copilot review: rounds 1–5 addressed (round 5 on 57b7ccb: 0 open findings; round 4's previously missed finding fixed in 749a977); round 6 requested only after exact-head CI passes
• base: main (22e76a8) merged in at 143b5f6; reviewed and revalidated on the new pair
• merge: not authorized

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Push-boundary version races and an under-reserved settlement deadline can violate the durable lifecycle guarantees.

3 open findings

• Capture task version before authorization refresh · New
• Return finalizer to close validation-to-push race · New
• Reserve process settlement time within preparation timeout · New

What changed in this PR

Adds F6a pre-merge publishing: rewritten heads are pushed to the task PR under an exact lease and durable recovery marker.

Changes:

• Adds schema-v18 push lifecycle persistence and admission guards.
• Pushes reviewed rewrites before command checks, with startup/cancellation recovery.
• Adds extensive lifecycle, race, migration, and production-wiring coverage.

File	Description
web/​server.ts	Settles interrupted pushes during startup and cancellation.
runner/​store.ts	Adds schema-v18 push markers and lifecycle guards.
runner/​pre-merge.ts	Coordinates guarded pushes, settlement, and visibility polling.
runner/​production.ts	Connects preparation to the production branch pusher.
runner/​merge.ts	Exposes the task branch and blocks unsettled pushes.
runner/​branch-push.ts	Adds exact expected-head leases and branch reads.
test/​pre-merge.test.ts	Covers push lifecycle and race scenarios.
test/​runner-lifecycle-store.test.ts	Tests marker persistence and admission guards.
test/​runner-publishing.test.ts	Tests startup and cancellation settlement.
test/​runner-production.test.ts	Tests production push wiring.
test/​runner-branch-push.test.ts	Tests exact leases and branch reads.
test/​merge-task-pr.test.ts	Tests branch resolution and merge blockers.
test/​store.test.ts	Updates migration expectations.
test/​publish.test.ts	Updates schema-version expectations.
test/​planning-drafts.test.ts	Updates migration expectations.
docs/​implementation/​pre-merge-rebase.md	Documents the durable push lifecycle.
docs/​architecture.md	Records F6a architecture and remaining work.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The durable external-push lifecycle is high risk, and the documented base advancement requires integration, re-review, and revalidation.

0 open findings

3 resolved since last review

• Return finalizer to close validation-to-push race
• Capture task version before authorization refresh
• Reserve process settlement time within preparation timeout

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Git process settlement can exceed the documented hard deadline and indefinitely block recovery or shutdown.

1 open finding

• Settlement timeout does not bound Git process recovery · New

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Branch reads can orphan subprocesses, and collaborator moves detected during push read-back are not refreshed for review.

1 open finding

• Avoid abandoning live descendants during bounded Git read timeouts · New

1 resolved since last review

• Settlement timeout does not bound Git process recovery

Previously missed (1)

In code that hasn't changed since last review

Handle moved outcome after push read-back before rethrowing

runner/​pre-merge.ts:336

The settlement result is discarded and the original push error is always rethrown. In production, if git push succeeds and a collaborator advances the branch before GitBranchPusher's mandatory read-back, remote.push throws for the mismatched head; this read then returns moved and clears the marker, but preparation reports failed without refreshing that collaborator head. The afterPush fixture does not reproduce the pusher's read-back, so its review-required regression misses this path. Route a settled moved outcome through the rewrite-lineage refresh path (with an accurate reason) instead of unconditionally rethrowing.

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

The pushed-head visibility wait can miss cancellation or shutdown signals during listener registration.

0 open findings

1 resolved since last review

• Avoid abandoning live descendants during bounded Git read timeouts

Previously missed (1)

In code that hasn't changed since last review

Handle abort race after checking but before listener registration

runner/​pre-merge.ts:290

An abort can occur after the check on line 281 but before this listener is attached. AbortSignal does not replay that event, so cancellation or shutdown can be ignored until the backoff timer expires (up to 4 seconds). Check signal.aborted immediately after registering the listener, as the command-check path already does, and add the corresponding race regression.

🧠 Review effort: Balanced