[mchwang]

Closes #174. Part of closing build step 4.

Problem

When the rebase conflict resolver's issue-trust read failed with its own timeout or with gh's answer, and a cancel, shutdown or the rebase work deadline was pending when the error was handled, Rebaser.#resolveConflict reported the pending stop instead (AGENTS.md: keep the original timeout, cancellation and shutdown reason through every layer).

Change

runner/rebase.ts #resolveConflict catch, in the #171 idiom:

• RebaseResourcesUnsettled still wins;
• timedOut(error) || ghAnswered(error) is rethrown as is (new);
• otherwise a pending stop is reported, as before.
  A stop that came first already makes the gateway and readIssueAccess throw the stop. rebase-conflict.ts, GitRebaser.run and pre-merge.ts pass the message through unchanged (checked, no change needed).

Tests

• test/runner-rebase.test.ts: timeout and gh answer before a cancel and before the rebase deadline, each asserting the stop was pending when the error was handled; a control row (another failure still reports the stop); stop first; retained ownership ahead of a timeout plus a later cancel.
• test/pre-merge.test.ts (new conflictRebase fixture: real GitRebaser, production store hooks, a resolver re-reading authorization as production does): timeout and gh answer before a cancel or shutdown, asserting the result, the saved prepare-merge response, the cleared rebase marker, and that the failing read was the resolver's own; stop-first cases.
• test/runner-rebase-conflict.test.ts: the resolver passes an authorization timeout through a later cancel and releases its resources.
• Without the fix, 8 of the 9 answer-first cases fail with the stop's text. The ninth, the resolver-layer cancel case in test/runner-rebase-conflict.test.ts, pins the unchanged resolver and passes either way, as do the stop-first, control and ownership cases, by design.

Docs: merge-queue.md (follow-up note replaced), pre-merge-rebase.md.

Validation

Current head: 5105e98 (a test title and a doc sentence only since d99298a). Each item names the commit it ran on.

• At d99298a: full unit suite without the Docker files (vitest run --project parallel): 2467 passed, 8 failed, 1 skipped. All 8 failures were timeouts (5 to 30 s) in review.test.ts, history.test.ts and pre-merge.test.ts at load average about 110; those three files re-run alone at d99298a: 177/177 passed. Recorded as load failures, not product failures.
• At d99298a: typecheck pass; affected suites (pre-merge, runner-rebase, runner-rebase-conflict, runner-publishing, merge-queue-poll, merge-recorded) 342/342.
• At 5105e98: typecheck pass; runner-rebase-conflict Keep the rebase conflict resolver's trust-read timeout over a later stop #174 tests 2/2 (the renamed test).
• Fail-without-fix (re-checked by rounds 5–9 at their heads): without the new check, 8 of the 9 answer-first cases fail with the stop's text (the resolver-layer cancel case pins the unchanged resolver); without the precedence line, the "failure of another kind" ownership row fails.
• Historical (superseded): at e224bda, the first commit, the full non-Docker suite gave 2476 passed, 1 skipped, 0 failed; at 6b782f8 the affected suites gave 341/341.
• CI and real-Docker on the final head: pending.
• Not tested: a rebase deadline that passes before the trust read fails (traced only, as the docs say).

Review record

• Self-review (author subagent): done.
• Independent review round 1 (fresh general-purpose agent, base main 46567ea / head e224bda): no defect in the code fix. Findings, all fixed in e42d502:
  1. Low: docs overclaimed the rebase-deadline case for the production resolver (its cleanup has no time left, so it usually ends as retained ownership, whose message carries the timeout). Docs scoped; a resolver-layer test asserts RebaseResourcesUnsettled carries the timeout text and that no cleanup starts. That test passes with or without the fix, by design: it pins the resolver layer.
  2. Low: the gh-answer fake did not match real gh api output. Shared fake test/fixtures/gh-answers.ts now matches gh 2.102.0 (exit 1, gh: Not Found (HTTP 404) on stderr, JSON body on stdout), version named; also replaced at the Investigate summary-only merge lifecycle concerns #48 test that used the same shape.
  3. Nit: the comment now names its assumption (the trust read is the resolver's only GitHub read).
  4. Nit: long doc sentence split into a list.
• Independent review round 2 (fresh general-purpose agent, base main / head e42d502): no correctness defects; round 1 fixes verified (8 answer-first tests fail without the fix). Findings, all fixed in 6b782f8:
  1. Low: issue-trust-read fakes in the old unreal shape remained in runner-publishing.test.ts (2) and, by grep, merge-queue-poll.test.ts (1). Now ghIssueNotFound(). Other HTTP 404/502 fakes fake other gh calls and are out of scope.
  2. Nit: deadline wording in merge-queue.md. Reworded; the deadline-during-the-read case is marked traced, not tested.
  3. Nit: stop-first tests varied an unused answer. Reduced (pre-merge 4→2, runner-rebase 2→1), with a reason.
  4. Nit: two copies of runWithInput's message format. One shared ghFailedExit helper, used by ghIssueNotFound and replayGh.
  5. Nit: evidence paragraph now names the pre-merge saved-outcome cases.
• Independent review round 3 (fresh general-purpose agent, merge base 46567ea / head 6b782f8): no correctness defects; rounds 1–2 verified; the gh fake checked live against gh 2.102.0; 8 answer-first tests confirmed to fail without the fix. Findings (fixed in a75ab81 by the PR author session):
  1. Low: two it.each titles also named the control row, which asserts the opposite. Titles now name what each row reports.
  2. Nit: docs named only cancel, shutdown and the deadline; any other stop of the rebase takes the same path. Added (traced, not tested).
  3. Nit: the saved-message claim cited a test that covers only the resolver's message. Now points at both tests.
  4. Declined: switching test/merge.test.ts's ghExit to ghFailedExit. It is outside this diff and harmless; it belongs with Read app-pinned required checks from the head's check runs (#176) #195, which edits that file.
• Independent review round 4 (fresh general-purpose agent, merge base 46567ea / head a75ab81): no code defects; rounds 1–3 verified. One finding, fixed in 864f184:
  1. Low: the retained-ownership test could not fail, because its cause was a timeout that the new check also keeps. Added a row whose cause is another failure; it fails with the precedence line removed (checked), passes with it.
• Independent review round 5 (fresh general-purpose agent, merge base 46567ea / head 864f184): clean, no defects and nothing at Low or above; 17/17 Keep the rebase conflict resolver's trust-read timeout over a later stop #174 tests; both mutations (new check, precedence line) fail exactly the tests claimed; the gh fake re-checked live. One optional doc nit, fixed in a60b468 (docs only): the deadline case now names gh still stopping after the read's own timeout, and quotes the message that results.
• Independent review round 6 (fresh general-purpose agent, merge base 46567ea / head a60b468): no correctness defects; rounds 1–5 verified; three mutations fail exactly the tests claimed. One finding, fixed in 1584cd2:
  1. Low: the docs cited test/pre-merge.test.ts for saving a RebaseResourcesUnsettled message, but that test saved nothing. It now passes an action ID and asserts the saved prepare-merge answer (passes).
• Independent review round 7 (fresh general-purpose agent, merge base 46567ea / head 1584cd2): code, tests and docs without defects; mutations fail exactly the tests claimed (including each half of the new check). Findings: (1) Low: this body's validation record named two heads for the full suite in a contradictory way; fixed here, each run now names its commit. (2) Nit: the answer-first count; fixed above (8 of 9). (3) Nit: the deadline case quoted only the inner message; fixed in c709c65 (docs only).
• Independent review round 8 (fresh general-purpose agent, merge base 46567ea / head c709c65): clean on code and tests (no findings at Low or above); mutations and the live gh fake re-checked. Nits, fixed in d99298a (docs only) or in this body: (1) the second deadline message's conditions; (2) the resolver test's cited case (timeout, then cancel); (3) this body's validation items now name their commit, and the affected suites were re-run on the final head.
• Independent review round 9 (fresh general-purpose agent, merge base 46567ea / head d99298a): clean, no findings at Low or above; mutations and the live gh fake re-checked. Nits: (1) a refusal from the trust check itself, followed by a cancel, is reported as the cancel, where the preparation's own authorization reports the refusal. Keep the rebase conflict resolver's trust-read timeout over a later stop #174 asks only for timeouts and gh answers, and merge-queue.md states this behaviour, so it is left as it is and recorded here as a follow-up candidate. (2) a test title said "during" the read; fixed in 5105e98. (3) a 90-word doc sentence; split in 5105e98. (4) this body's validation record; rewritten above.
• Independent review round 10 (fresh general-purpose agent, merge base 46567ea / head 5105e98): clean on code, tests and docs; mutations re-checked (8 of 9 answer-first cases; the precedence row); the branch merges cleanly with main 302eac3 (typecheck and the Keep the rebase conflict resolver's trust-read timeout over a later stop #174 and merge-recorded tests pass on the merged tree). Two nits in this body: two stale review-record lines (removed) and the follow-up candidate's status (recorded in the audit below).

Review-lesson audit (merge gate)

No new rule is needed.

• Covered by CLAUDE.md "Treat every behavioural claim … as something to verify … give it a test that fails if the claim is false": round 1 L1 (deadline overclaim), rounds 3–8 doc citations and wording (the saved-message citation, other stops, the quoted messages), round 4 L1 (the ownership test could not fail), round 6 L1 (the unsettled-conflict test saved nothing).
• Covered by CLAUDE.md "A fake of an external tool must give … each part of the answer … as the real tool gives it": round 1 L2 and round 2 L1 (issue-read fakes in the old shape), round 2 nit 3 (one shared message format).
• Covered by CLAUDE.md "In evidence records, label cited commits" and "Report … as separate gates": rounds 7–10 body findings (validation items naming their commit; stale lines).
• One-off (test naming and cost): round 2 nit 2 (stop-first tests varying an unused answer), round 3 L1 and round 9 nit 2 (titles that did not match their assertion).
• Declined, out of scope: round 9 nit 1, a trust refusal (not a timeout or gh answer) followed by a cancel is reported as the cancel. Keep the rebase conflict resolver's trust-read timeout over a later stop #174 covers only timeouts and gh answers, and pre-merge-rebase.md and merge-queue.md describe this behaviour. Not filed as an issue; it can be filed if wanted.

Before Copilot (head 5105e98, validated head for this request)

• Clean independent review: round 10, merge base 46567ea / head 5105e98.
• CI at request time: test and real-docker in progress on 5105e98. Mergeable: yes. Unresolved threads: 0. Closes Keep the rebase conflict resolver's trust-read timeout over a later stop #174. Deferred follow-ups: the declined trust-refusal case above (not filed).
• Copilot: requested on 5105e98 (2026-10-21 05:02 UTC).

Copilot round 1 (head 5105e98)

Copilot (copilot-pull-request-reviewer): no open issues. Its one caveat was that final-head CI and real-Docker had not finished; both are the merge gate, checked on 5105e98 before merging. No change made.

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

No actionable defects were identified, but final-head CI and real-Docker validation remain unconfirmed in the supplied record.

No open issues 🎉

What changed in this PR

Addresses #174 by preserving the rebase conflict resolver’s earlier trust-read timeout or GitHub error instead of replacing it with a later stop.

Changes:

• Preserves timeout/error reporting while retaining resource-ownership precedence.
• Adds race regressions covering stop ordering and saved preparation outcomes.
• Standardizes GitHub error fixtures and documents behavior.

File	Description
test/​runner-rebase.test.ts	Tests error precedence and cleanup.
test/​runner-rebase-conflict.test.ts	Tests resolver timeout reporting and retained resources.
test/​runner-publishing.test.ts	Uses realistic GitHub error fixtures.
test/​pre-merge.test.ts	Tests saved outcomes through a real conflicting rebase.
test/​merge-queue-poll.test.ts	Updates the issue-read error fixture.
test/​fixtures/​gh/​recorded.ts	Reuses shared failed-exit formatting.
test/​fixtures/​gh-answers.ts	Adds shared GitHub error fixtures.
runner/​rebase.ts	Preserves earlier timeout and GitHub errors.
docs/​implementation/​pre-merge-rebase.md	Documents precedence and regression coverage.
docs/​implementation/​merge-queue.md	Replaces the follow-up note with implemented behavior.

_{Review effort: Balanced}