follow-ups from the 2026-09-22 drain: six minors left open at merge

[justinhelmer]

Findings left open when the 2026-09-22 drain merged their pull requests. Each was raised by a review arm at the merged head, judged a minor, and recorded here rather than spending another round before the release.

From #2195 (record 0075 + the typed provider-failure seam, 1ee9f2b0)

1. The intake timeout path bypasses the seam. src/core/intake.ts around line 255: AbortSignal.timeout(...) yields an AbortError or TimeoutError, and the branch returns an ordinary silent timeout with no providerFailure, so Slack posts nothing. Model-proxy item 12b requires every intake provider failure to cross the typed seam and render the no-lease sentence. Classify the timeout as transient, preserve structured attempts, and let the existing delivery path post once.
2. A buffered provider-up row can replay after a local retry wins. src/core/harness/pi/harness.ts around line 2380: a provider-up row arriving while a post-loop local retry is in flight is discarded by clearTurnProviderHold() without advancing mirror.inboxConsumedSeq, so after a restart the durable row can be delivered again and release a later hold with no new provider-up transition. Mark the buffered rows consumed when the local retry wins; regression covers provider-up during an in-flight retry followed by a restart.

From #2205 (decision-record reservations, 60f45b75)

3. Durable-store transport failures bypass the typed admission refusal. src/index.ts around line 451: WorkerCoordinatorInstanceStore.reserveDecisionRecord() throws on timeouts and non-2xx responses while the callback translates only an { ok: false } result, so a state-Worker outage takes the generic setup-failure path instead of returning decision_record_store_unavailable. Catch failures from the durable reservation call and rethrow DecisionRecordReservationUnavailableError.

From #2206 (terminal pull-request transitions, a8fe55df)

All three are one shape: a transition path that still acts without a fresh, branch-existence-aware read.

4. The ship-only pre-post guard ignores branch existence. src/core/reviewRound.ts around line 843: fetchPrHead verifies only that the PR is open and reports the pinned sha, so a head branch deleted during the review still lets the verdict post, against agent-ship item 9. Use the branch-existence-aware facts reader.
5. Rebase outcomes dispatch without a fresh read. src/core/ship/coordinator.ts around line 3252: a conflict result spawns a coding child and a changed result spawns review, while the rebase itself can take long enough for the PR to merge, close or lose its branch. Route both through a fresh pr-check.
6. A recovered pushed branch skips the transition guard. src/channels/adminCoordinator.ts around line 1812: the route returns an open PR without re-reading state, head or branch existence, so review can dispatch onto a deleted branch. Refresh through fetchPrFacts/pullRequestState and retry on unknown state.

Why one issue

Justin's direction on 2026-09-22 is to land everything open, cut a release with nothing new, then sign off what shipped with receipts, and repeat before starting new work. These six are the debt that decision knowingly took on; they are not lost, and they are the first candidates after the receipts round.

[justinhelmer]

Being closed out before the receipts round rather than left filed, on Justin's word: these were taken on under a merge bar looser than his standing rule, and that bar is withdrawn. The standing rule is unchanged — the default level is minor, so a round carrying any finding at or above minor fixes every actionable finding in it down to nit, and only a round of nits and fyi alone is left with a reply and resolve.

Split into two asks because of file overlap, not scope.

Now, the three that do not touch the ship coordinator: the intake timeout bypassing the typed seam (src/core/intake.ts), the buffered provider-up row replaying after a local retry wins (src/core/harness/pi/harness.ts), and the durable-store transport failure bypassing the typed admission refusal (src/index.ts).

After #2221 merges, the three that are one shape — a transition path acting without a fresh branch-existence-aware read of the pull request: the ship-only pre-post guard in src/core/reviewRound.ts, the rebase outcomes in src/core/ship/coordinator.ts, and the recovered pushed branch in src/channels/adminCoordinator.ts. #2221 is live in src/core/ship/coordinator.ts right now and is a P0 that is actively stranding finished work, so these wait rather than race it.

[justinhelmer]

All six findings are merged. Closing.

Half	Findings	Pull request	Merge commit
non-ship	typed intake timeouts, consumed provider-up rows after a winning local retry, typed durable-reservation outages	2232	e2e99f11
ship-side	the three stale-read transition paths in reviewRound.ts, ship/coordinator.ts and adminCoordinator.ts	2234	32873f4e

Both are ancestors of main. The ship-side half shipped in 1.260.5 (tag v1.260.5, 3b665eb6); the non-ship half merged after the cut and rides 1.260.6.

What it cost, recorded because the number is the argument for the fixes it produced. The non-ship half took four attempts. The first opened this pull request and drew one major. The second stalled at a review round that dispatched no child and idled for 29 minutes (#2237), because its review child was refused by the workspace guard (#2235). The third completed the fix, rebased and validated it, then had its push refused by the branch guard (#2200) while its own run record claimed salvage had published the commit — GitHub never received it and the work was lost (#2262). The fourth landed it.

Three of those four attempts were defeated by pipeline defects rather than by the problem, and two of the four defects were already filed and open at the time.

🤖 Generated with Claude Code