Keep the rebase conflict resolver's trust-read timeout over a later stop (#174)

docs/implementation/merge-queue.md

@@ -33,7 +33,13 @@ A GitHub read that stops reports the first reason it stopped for, and the merge
 - **Gateway reads.** A GitHub read that runs out of its own time throws `RemoteTimeout` (`github/timeout.ts`): the merge-state inspection, the queue watermark, the queue inspection and the issue read. When the caller aborts later, while gh is still stopping, the read still reports its timeout. A failure gh reported before either abort (its own exit, `CommandFailed`) is reported as itself, not replaced by the abort, except in the rules read, which treats any failure as unknown rules (traced, not tested). When one inspection stage's gh exit stops the other stages, the inspection waits for every stage to settle and then reports that exit, even when its own deadline fired meanwhile (tested). The already-fixed check's result is handled from the moment it starts, so a check that a failed stage stopped and that settles first is not an unhandled rejection, which Node would raise process-wide (found by the #48 tests; it predated #48; tested); a successful answer that arrives after the abort is still discarded, and so is an error the gateway raised while checking a successful answer (for example a different issue returned; traced, not tested). An inspection stage's own validation error that comes before the abort, while the other stages are still settling, is also answered with the abort: resendable, never a wrong saved answer (traced, not tested). The issue gateway's rule also covers execution's issue text and the issue board's refresh; the board's prioritizer still answers a refresh stopped by shutdown with the shutdown reason (traced, not tested).
 - **Issue-trust reads.** The server wraps a failed trust read as an upstream failure and keeps the timeout as its cause. On the merge path, a trust read that timed out, or that gh answered, before the caller's abort still reports that; the runner actions keep a trust read's own timeout too, even when shutdown started while gh stopped (tested), and answer any other failed trust read as shutting down once shutdown has started (the server is stopping, the runner is closing, or the request was aborted, which only shutdown does). Either way nothing is admitted.
   - The trust read that gates the runner actions (start, resume, approve-continuation, publish, prepare-merge) is not saved as the action's outcome when it times out (tested for start; the other actions share the same branch): the action answers 503, and the same action ID reads trust again; an outcome already saved under that ID is still the answer, also when shutdown started while gh stopped (tested).
-  - The trust read inside merge preparation (`runner/pre-merge.ts`) is different: a timeout or gh answer there settles the preparation as failed, which is saved and replayed, and a shutdown that arrives after it is reported as that timeout or answer (tested: the preparation layer in `test/pre-merge.test.ts`, with the error shape the server's trust read builds, and that read itself through publishing in `test/runner-publishing.test.ts`); a cancel takes the same path (traced, not tested). The rebase conflict resolver's trust read (`runner/rebase-conflict.ts`) is not covered: `Rebaser` (`runner/rebase.ts`) reports a cancel, shutdown or expired rebase deadline that is pending when the resolver's error is handled in its place. This is unchanged by #48 and left as a follow-up (#174).
+  - The trust read inside merge preparation (`runner/pre-merge.ts`) is different: a timeout or gh answer there settles the preparation as failed, which is saved and replayed, and a shutdown that arrives after it is reported as that timeout or answer (tested: the preparation layer in `test/pre-merge.test.ts`, with the error shape the server's trust read builds, and that read itself through publishing in `test/runner-publishing.test.ts`); a cancel takes the same path (traced, not tested). The rebase conflict resolver also reads issue trust (`runner/rebase-conflict.ts`), just before its agent would start. When that read fails, `GitRebaser` (`runner/rebase.ts`) reports the failure as follows (#174):
+    - **Cancel or shutdown after the read failed.** If the read timed out, or gh answered, before the cancel or shutdown, the rebase reports that timeout or answer, and the preparation is saved as failed with it. Tested: the resolver in `test/runner-rebase-conflict.test.ts` (a timeout, then a cancel), the rebaser in `test/runner-rebase.test.ts`, and the saved outcome through a real rebase in `test/pre-merge.test.ts`.
+    - **Cancel or shutdown before the read.** The read reports the stop, and so does the rebase (tested).
+    - **Rebase work deadline.** The rebaser itself keeps a timeout or answer that came before its deadline, when the resolver settles (tested in `test/runner-rebase.test.ts`). The production resolver usually does not report the read's failure itself then: its own deadline is the same work deadline, so its cleanup has no time left. It keeps its resources for startup recovery and reports that, and the preparation is saved with that message, which includes the timeout or answer (the resolver's message is tested in `test/runner-rebase-conflict.test.ts`; that the preparation saves any `RebaseResourcesUnsettled` message is tested in `test/pre-merge.test.ts`). A deadline that passes before the read fails is traced, not tested. This includes a deadline that passes while the read is still running, and one that passes after the read's own timeout fired but before gh stopped. The resolver then stops waiting for the read at its deadline and reports retained resources, and that message has no timeout text. Usually the message is "Conflict resolution retained resources for startup recovery: Conflict child settlement exceeded the rebase work deadline." It ends with "The rebase deadline expired during conflict resolution." instead when the rebaser's own deadline timer fires first and the read reports that stop before the resolver's own deadline timer fires.
+    - **Other stops.** Any other stop of the rebase, such as the preparation's own deadline, takes the same path as a cancel here (traced, not tested).
+    - **Other failures.** A read failure that is neither a timeout nor gh's answer still reports a stop that is pending when it is handled (tested).
+    - **Retained resources.** A resolver that keeps its resources for startup recovery (`RebaseResourcesUnsettled`) is reported as that before any of the cases above (tested).
   - A publish whose trust read timed out, or that gh answered, before shutdown or cancellation is recorded as failed with that reason, not as stopped (tested for shutdown, `test/runner-publishing.test.ts`); cancellation takes the same path, both leave the publish owed, and a cancelled task's PRs are still closed (traced, not tested).
   - Execution's issue read reports its own timeout when a cancel arrives after it; the runner decides the attempt's outcome from its recorded cancel, not from that error (traced, not tested).
   - Out of this change: the issue board's trust toggle still saves a timed-out author read as that action's failure, and planning's issue reads (`web/server.ts`, `web/planning.ts`) still answer a timeout as an upstream failure without that cause.

docs/implementation/pre-merge-rebase.md

@@ -24,7 +24,7 @@ Before any clone or Docker operation, the production adapter records a unique ch
 
 The Store records each resolved foreign source SHA before the result ref write. Preparation refuses duplicates, sources outside the captured history, owned sources, and sources that did not actually rewrite. On publication, a foreign mapped commit retains `owner: null`, `origin: foreign`, and `sourceSha`, plus `conflictResolved: true`. Linking therefore keeps its lines in Unplanned while the review UI adds the text label “conflict resolved by agent.” An owned mapped commit instead retains its Px owner and source SHA without foreign-resolution provenance; its preserved message keeps the original plan trailers, and the rewritten mapping lets review recompute whether Px's approval became stale. Every replay preserves the original commit author.
 
-All other conflicts are a review outcome (`RebaseConflict`). Cancellation and failures settle the Git process group before cleanup. Cleanup removes only the validated UUID workspace. A partially created but unregistered worktree is removed only after Git's own worktree list proves it is not registered. Result refs are created atomically from the absent state; a reused attempt ID cannot overwrite or delete a previously retained result, while an ambiguous create is reconciled against the exact attempted value.
+All other conflicts are a review outcome (`RebaseConflict`). Cancellation and failures settle the Git process group before cleanup. When the resolver fails while a cancel, shutdown, the rebase work deadline or any other stop of the rebase (such as the preparation's own deadline) is pending, the rebaser reports the stop, with two exceptions. A resolver that kept its resources for startup recovery reports `RebaseResourcesUnsettled`. A remote read that timed out on its own, or that gh answered, before the stop (the issue-trust read before launch) reports that timeout or answer (#174). When the stop is the work deadline, the production resolver has no time left for cleanup, so it usually keeps its resources and reports `RebaseResourcesUnsettled`, whose message includes the timeout or answer. Cleanup removes only the validated UUID workspace. A partially created but unregistered worktree is removed only after Git's own worktree list proves it is not registered. Result refs are created atomically from the absent state; a reused attempt ID cannot overwrite or delete a previously retained result, while an ambiguous create is reconciled against the exact attempted value.
 
 ## Restart recovery
 
@@ -54,4 +54,4 @@ Every review reload in a preparation awaits a history worker (#170), so each one
 
 ## Regression evidence
 
-`test/runner-rebase.test.ts` covers clean one-to-one replay, the already-current no-op, process-group ownership, the whole-operation deadline, owned and foreign conflict resolution, clean sibling files, gitlink-pointer auditing and manual gitlink-conflict routing, embedded-repository refusal, literal replacement-character paths, invalid ledger classification, outside-file, resolver-commit and rebase-control refusal, cancellation before and during Git, exact cleanup, retained refs, and a read-only source checkout. `test/runner-rebase-conflict.test.ts` covers owned-item prompt binding, plan-revision races before launch and copy-back, claim-before-prepare ordering, distinct allocation identities, bounded no-follow host snapshots, nested modify/delete copy-in, copy-back audit, child settlement deadlines, cancellation settlement, cleanup ordering, and retained ownership for unreleased resources. `test/runner-lifecycle-store.test.ts` covers admission, durable process and child ownership, compare-and-swap races, resolved-source validation, and owned/foreign provenance. `test/runner-recovery.test.ts` covers marker validation, live-group fail-closed recovery, exact child-storage recovery, partial-match refusal, and restart ordering. `test/agent-process-group.test.ts` covers delayed-signal suppression after exit and retained ownership for an orphaned group. The real-container suite covers dirty-conflict admission and bounded exact-path export. `test/history.test.ts` and `test/browser/review.spec.ts` cover Unplanned attribution and the visible conflict-resolution label. `test/runner-production.test.ts` covers exact-plan recovery routing and adapter construction.
+`test/runner-rebase.test.ts` covers clean one-to-one replay, the already-current no-op, process-group ownership, the whole-operation deadline, owned and foreign conflict resolution, clean sibling files, gitlink-pointer auditing and manual gitlink-conflict routing, embedded-repository refusal, literal replacement-character paths, invalid ledger classification, outside-file, resolver-commit and rebase-control refusal, cancellation before and during Git, the reason a failed resolver reports when a stop is pending (#174), exact cleanup, retained refs, and a read-only source checkout. `test/runner-rebase-conflict.test.ts` covers owned-item prompt binding, plan-revision races before launch and copy-back, claim-before-prepare ordering, distinct allocation identities, bounded no-follow host snapshots, nested modify/delete copy-in, copy-back audit, child settlement deadlines, an authorization read's own timeout kept through a later cancel and carried in the retained-resources message when the work deadline passed (#174), cancellation settlement, cleanup ordering, and retained ownership for unreleased resources. `test/pre-merge.test.ts` covers, through a real conflicting rebase, the saved preparation outcome when the resolver's trust read timed out or gh answered before a cancel or shutdown, and when the stop came first (#174). `test/runner-lifecycle-store.test.ts` covers admission, durable process and child ownership, compare-and-swap races, resolved-source validation, and owned/foreign provenance. `test/runner-recovery.test.ts` covers marker validation, live-group fail-closed recovery, exact child-storage recovery, partial-match refusal, and restart ordering. `test/agent-process-group.test.ts` covers delayed-signal suppression after exit and retained ownership for an orphaned group. The real-container suite covers dirty-conflict admission and bounded exact-path export. `test/history.test.ts` and `test/browser/review.spec.ts` cover Unplanned attribution and the visible conflict-resolution label. `test/runner-production.test.ts` covers exact-plan recovery routing and adapter construction.

runner/rebase.ts

@@ -4,6 +4,8 @@ import { fileURLToPath } from 'node:url';
 import { runInProcessGroup, type ProcessGroup } from '../agents/process-group.ts';
 import type { DockerOutcome } from '../agents/docker.ts';
 import { GIT_OPTIONS, gitEnvironment } from '../git/clone.ts';
+import { ghAnswered } from '../github/run-with-input.ts';
+import { timedOut } from '../github/timeout.ts';
 import { isUuidV4 } from './lifecycle.ts';
 import { ownerOnlyDirectory, type RunnerRepository } from './runner-repository.ts';
 import { MAX_INDEX_BYTES } from './verify-checkout.ts';
@@ -440,6 +442,12 @@ export class GitRebaser {
       catch (error) {
         // Retained ownership outranks cancellation/deadline reporting: live cleanup must preserve its parent workspace.
         if (error instanceof RebaseResourcesUnsettled) throw error;
+        // A remote read that timed out on its own, or that gh answered, keeps that reason though a cancel, shutdown or
+        // this deadline became pending while it stopped (#174). Had the stop come first, the read would have reported
+        // the stop instead: the gateways and the server's trust read keep the first reason. This relies on the
+        // issue-trust read being the production resolver's only GitHub read. A later gh call in the resolver must also
+        // report a stop that came first as the stop, or this check would report its answer in the stop's place.
+        if (timedOut(error) || ghAnswered(error)) throw error;
         abortExpired();
         controller.signal.throwIfAborted();
         throw error;

test/fixtures/gh-answers.ts

@@ -0,0 +1,22 @@
+import { CommandFailed } from '../../github/run-with-input.ts';
+
+/**
+ * gh's failed exit as `runWithInput` (github/run-with-input.ts) reports it: a CommandFailed whose message is
+ * `gh failed (exit N): ` plus the first 2,000 characters of stderr, then, when stdout is not empty, a newline and the first
+ * 2,000 characters of stdout. `stderr` and `stdout` are kept whole, already trimmed as runWithInput trims them.
+ */
+export function ghFailedExit(exitCode: number, stderr: string, stdout: string): CommandFailed {
+  return new CommandFailed(`gh failed (exit ${exitCode}): ${stderr.slice(0, 2000)}${stdout ? `\n${stdout.slice(0, 2000)}` : ''}`,
+    stderr, stdout);
+}
+
+/**
+ * gh's own exit for an issue read of an issue that does not exist. Checked with gh 2.102.0
+ * (`gh api repos/cli/cli/issues/999999999`): exit status 1, `gh: Not Found (HTTP 404)` on stderr, and GitHub's JSON body
+ * on stdout. The tests that use it read only the error's class (through `ghAnswered`) and its message, not the separate
+ * stderr and stdout fields.
+ */
+export function ghIssueNotFound(): CommandFailed {
+  return ghFailedExit(1, 'gh: Not Found (HTTP 404)',
+    '{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/issues#get-an-issue","status":"404"}');
+}

test/fixtures/gh/recorded.ts

@@ -1,5 +1,5 @@
 import { readFileSync } from 'node:fs';
-import { CommandFailed } from '../../../github/run-with-input.ts';
+import { ghFailedExit } from '../gh-answers.ts';
 import type { RunGh } from '../../../github/merge.ts';
 
 /**
@@ -35,8 +35,7 @@ export function replayGh(...recordings: RecordedGh[]): RunGh & { calls: string[]
     // The gateway may turn this failure into unknown rules, so callers also assert `unrecorded` is empty.
     if (!answer) { unrecorded.push([...args]); throw new Error(`No recorded answer for gh ${args.join(' ')}`); }
     if (answer.exitCode === 0) return answer.stdout;
-    throw new CommandFailed(`gh failed (exit ${answer.exitCode}): ${answer.stderr.slice(0, 2000)}${answer.stdout ? `\n${answer.stdout.slice(0, 2000)}` : ''}`,
-      answer.stderr, answer.stdout);
+    throw ghFailedExit(answer.exitCode, answer.stderr, answer.stdout);
   };
   return Object.assign(run, { calls, unrecorded });
 }

test/merge-queue-poll.test.ts

@@ -10,6 +10,7 @@ import { Store } from '../runner/store.ts';
 import { createDemo } from '../scripts/demo.ts';
 import { startServer } from '../web/server.ts';
 import { fixtureGit } from './fixtures/git.ts';
+import { ghIssueNotFound } from './fixtures/gh-answers.ts';
 import { RemoteTimeout } from '../github/timeout.ts';
 import { CommandFailed } from '../github/run-with-input.ts';
 
@@ -349,7 +350,7 @@ it('answers a merge click whose issue-trust read gh answered before the merge de
       // The first read admits the click; the refresh before admission is held until the deadline aborts it.
       if (++reads === 1) { slowNext = true; return { number, authorLogin: 'member', collaborator: true }; }
       readStarted(); await new Promise(resolve => options.signal?.addEventListener('abort', resolve, { once: true }));
-      throw new CommandFailed('gh failed (exit 1): HTTP 404: Not Found', '', '');
+      throw ghIssueNotFound();
     },
     async issueText(number: number) { return { number, title: '', body: '', comments: [] }; },
   };
@@ -362,7 +363,7 @@ it('answers a merge click whose issue-trust read gh answered before the merge de
   }), { github: true, issues });
   try {
     const answer = async (response: Response) => ({ status: response.status, error: (await response.json() as { error: string }).error });
-    const expected = { status: 502, error: 'Issue trust could not be verified: gh failed (exit 1): HTTP 404: Not Found' };
+    const expected = { status: 502, error: `Issue trust could not be verified: ${ghIssueNotFound().message}` };
     expect(await answer(await send())).toEqual(expected);
     await reading;
     // Saved: resending the same click replays the answer without reading trust again.