From ef4192a408170c506725adf2fc1ad8aee2e49bf7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 20:10:04 +0000 Subject: [PATCH] fix(schedules): defer settlement when the Stop barrier times out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex finding: waitForStopPersistence returned after its 5s poll budget even when the Stop was still fresh and unacknowledged, and recordScheduleOutcome then went straight on to terminalize the run — releasing its capacity, deletion, and erasure barriers while beforePublish may still have been writing. Slow checkpoint cleanup is indistinguishable from a dead route on that signal, so the timeout was being treated as if the barrier had been satisfied. The poll budget now means "undecided", not "clear". On timeout with a fresh, unacknowledged Stop the barrier DEFERS: recordScheduleOutcome returns false without recording, leaving the run active/preserved. Settlement then happens either when the route acknowledges, or once the existing stale-owner cutoff (ABORT_OWNER_PRESUMED_ALIVE_MS) authorizes a later attempt — which the loop already treats as clear-to-settle. Callers with durable retry (the approval-expiry host action, reconciliation) re-drive it, so a deferral converges rather than stranding the run. Test: a fresh Stop that never acknowledges within the budget reports not-settled and records no outcome. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq --- packages/api/src/schedules/service.spec.ts | 25 +++++++++++++++++++++ packages/api/src/schedules/service.ts | 26 +++++++++++++++++----- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/packages/api/src/schedules/service.spec.ts b/packages/api/src/schedules/service.spec.ts index 59b230be46..efc00f97b7 100644 --- a/packages/api/src/schedules/service.spec.ts +++ b/packages/api/src/schedules/service.spec.ts @@ -718,6 +718,31 @@ describe('interactive Stop persistence barrier', () => { expect(methods.recordRunOutcome).toHaveBeenCalled(); }); + /** + * The poll budget expiring proves nothing about the Stop route's writes (slow checkpoint + * cleanup looks identical), so the barrier must DEFER rather than terminalize the run and + * release its capacity mid-persistence. + */ + it('defers settlement when a fresh Stop never acknowledges within the poll budget', async () => { + const { service, methods } = outcomeService(); + methods.getScheduleRunAbortState = jest.fn(async () => ({ + status: 'started', + abortSource: 'stop', + abortRequestedAt: new Date(), + })); + + const settled = await service.recordScheduleOutcome({ + scheduleId: 's1', + scheduledFor: '2026-01-01T00:00:00.000Z', + status: 'interrupted', + }); + + // Reported NOT settled so a durable-retry caller re-drives it, and the run is left + // active rather than terminalized while persistence may still be in flight. + expect(settled).toBe(false); + expect(methods.recordRunOutcome).not.toHaveBeenCalled(); + }); + it('does not gate a pause (requires_action) on the Stop barrier', async () => { const { service, methods } = outcomeService(); methods.getScheduleRunAbortState = jest.fn(async () => null); diff --git a/packages/api/src/schedules/service.ts b/packages/api/src/schedules/service.ts index 1977f3801b..6f4887d416 100644 --- a/packages/api/src/schedules/service.ts +++ b/packages/api/src/schedules/service.ts @@ -772,7 +772,7 @@ export function createSchedulesService( * marker, a non-stop source, or a stamp gone stale (its route presumed dead) proceeds * immediately, so the stale-owner timeout remains the bounded recovery path. */ - async function waitForStopPersistence(scheduleId: string, scheduledFor: Date): Promise { + async function waitForStopPersistence(scheduleId: string, scheduledFor: Date): Promise { const deadline = Date.now() + STOP_BARRIER_TIMEOUT_MS; for (;;) { const state = await methods.getScheduleRunAbortState(scheduleId, scheduledFor); @@ -782,10 +782,18 @@ export function createSchedulesService( state.abortPersistedAt != null || !hasAbortInFlight(state, Date.now()) ) { - return; + // Cleared to settle: no Stop owns the run, it acknowledged, or its owner is past + // the stale cutoff and is presumed dead (the bounded recovery path). + return true; } if (Date.now() >= deadline) { - return; + // Still an UNACKNOWLEDGED, FRESH Stop. The poll budget expiring proves nothing about + // the route's writes — slow checkpoint cleanup looks exactly like this — so treating + // the barrier as satisfied would terminalize the run and release its capacity (and + // its deletion/erasure barriers) while beforePublish may still be writing. DEFER + // instead: the acknowledgement settles it, or the stale-owner cutoff authorizes a + // later attempt. + return false; } await new Promise((resolve) => setTimeout(resolve, STOP_BARRIER_POLL_MS)); } @@ -806,8 +814,16 @@ export function createSchedulesService( } const terminal = status !== 'requires_action'; if (terminal) { - // Honor an in-flight interactive Stop's persistence before terminalizing. - await waitForStopPersistence(scheduleId, new Date(scheduledFor)); + // Honor an in-flight interactive Stop's persistence before terminalizing. A deferral + // is NOT a failure to record — the run is deliberately left active/preserved — but it + // must report "not settled" so callers with durable retry (the approval-expiry host + // action, reconciliation) re-drive it rather than assuming the outcome landed. + if (!(await waitForStopPersistence(scheduleId, new Date(scheduledFor)))) { + logger.info( + `[schedules] deferring terminal settlement for ${scheduleId}: interactive Stop persistence is still unacknowledged`, + ); + return false; + } } if (terminal && streamId && jobCreatedAt != null) { try {