Skip to content

Commit 0eb8a21

Browse files
mchwangclaude
andcommitted
Log a merge job that could not be ended, and document that it stays running until restart (F6b)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 82661d6 commit 0eb8a21

3 files changed

Lines changed: 26 additions & 2 deletions

File tree

‎docs/implementation/merge-handoff.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ A rebase moves the pull request's base. The guarded merge would then scan only t
6363
| Cancel while waiting | the wait stops at once; the task closes; the job ends `cancelled` |
6464
| Cancel during the guarded merge | before GitHub is asked, the merge stops and the job ends `cancelled`; once a merge attempt is submitting, a cancel is refused, as before |
6565
| Shutdown | in shutdown's first, synchronous step, before requests drain, the job is told to stop: a wait ends at once, and nothing after it asks GitHub to merge. A preparation in progress stops only when preparation does, after the drain, as for a `prepare-merge` click; the job then stops without merging. The job is awaited before the store closes. While preparing or waiting, it stays `running` on disk. A guarded merge in progress stops as a click's does: if GitHub was not asked yet, the job stays `running`; if it was, the job ends as the merge attempt records it: `blocked` if GitHub refused (`failed` if the head changed, or the review changed while GitHub was asked), otherwise `submitted`, and the attempt reconciles as before |
66+
| Ending the job fails | if the write that ends a job fails (a store error, not shutdown), the server logs "Could not end merge job …; it shows as running until the server restarts". The job stays `running` on disk: the page keeps following it, and a new click is refused while it runs. The next start settles it as below |
6667
| Restart | a `running` job that had not asked GitHub to merge becomes `failed` ("The server restarted before the merge finished"). One that had (its merge action ID is saved before the guarded merge starts) ends as its merge attempt records it: `merged`, `queued`, `submitted`, or `blocked` after a refusal, which also moves the task to approved, merge blocked (`failed`, task unchanged, when the head changed or the review changed while GitHub was asked). A `prepare-merge` action and a push marker are settled as before |
6768

6869
## Storage

‎runner/merge-job.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,12 @@ export class MergeJobCoordinator {
139139

140140
#finish(id: string, state: Exclude<MergeJobState, 'running'>, reason: string | null = null, url: string | null = null): void {
141141
try { this.settle(() => this.service.store.finishMergeJob(this.service.config.identity, id, { state, reason, url })); }
142-
catch { /* Startup settles a job that could not be ended now. */ }
142+
catch (error) { this.#unended(id, error); }
143+
}
144+
145+
/** Startup settles a job that could not be ended now; until then it shows as running and a new click is refused. */
146+
#unended(id: string, error: unknown): void {
147+
if (!this.#closing) console.error(`Could not end merge job ${id}; it shows as running until the server restarts: ${(error as Error)?.message ?? String(error)}`);
143148
}
144149

145150
async #run(job: MergeJob, signal: AbortSignal): Promise<void> {
@@ -219,7 +224,7 @@ export class MergeJobCoordinator {
219224
#block(job: MergeJob, reason: string, whileReady = false): void {
220225
const readyFor = whileReady ? { commandPolicyDigest: this.service.commandPolicyDigest() } : undefined;
221226
try { this.settle(() => this.service.store.blockMergeJob(this.service.config.identity, job.id, reason, readyFor)); }
222-
catch { /* Startup settles a job that could not be ended now. */ }
227+
catch (error) { this.#unended(job.id, error); }
223228
}
224229

225230
async #awaitChecks(job: MergeJob, signal: AbortSignal): Promise<CheckVerdict> {

‎test/merge-job.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,24 @@ it.each([false, true])('ends submitted, not merged or queued, when GitHub accept
341341
expect(f.store.getTask(identity).status).toBe('in review');
342342
});
343343

344+
it.each(['finish', 'block'] as const)('reports a job it could not end (%s), which stays running and refuses a new click until startup settles it', async kind => {
345+
const f = setup(kind === 'block' ? { checks: () => [{ context: 'build', appId: null, state: 'failure' }] } : {});
346+
const fail = vi.fn(() => { throw new Error('disk full'); });
347+
// The finish case ends `submitted` (its attempt's outcome was not saved either), so the task stays in review and a new
348+
// click reaches the running-job check.
349+
if (kind === 'block') f.store.blockMergeJob = fail;
350+
else { f.store.finishMergeJob = fail; f.store.finishMergeAttempt = () => { throw new Error('local refresh failed'); }; }
351+
const logged = vi.spyOn(console, 'error').mockImplementation(() => {});
352+
try {
353+
const started = await f.coordinator.start('review-token', randomUUID());
354+
await f.coordinator.current;
355+
expect(fail).toHaveBeenCalledTimes(1);
356+
expect(logged).toHaveBeenCalledWith(expect.stringMatching(new RegExp(`Could not end merge job ${started.id}; it shows as running until the server restarts: disk full`)));
357+
expect(f.store.getMergeJob(identity, started.id)).toMatchObject({ state: 'running' });
358+
await expect(f.coordinator.start('review-token', randomUUID())).rejects.toThrow(/already running/);
359+
} finally { logged.mockRestore(); }
360+
});
361+
344362
it('leaves the task approved, merge blocked when a retried job is queued', async () => {
345363
const f = setup({ queue: true, checks: n => (n === 1 ? [{ context: 'build', appId: null, state: 'failure' }] : []) });
346364
await f.coordinator.start('review-token', randomUUID());

0 commit comments

Comments
 (0)