mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 01:53:33 +00:00
fix(schedules): defer settlement when the Stop barrier times out
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZwZPDBgxy4mCWWg6zQSjq
This commit is contained in:
parent
0a85688ac3
commit
ef4192a408
2 changed files with 46 additions and 5 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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<void> {
|
||||
async function waitForStopPersistence(scheduleId: string, scheduledFor: Date): Promise<boolean> {
|
||||
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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue