mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-21 07:36:25 +00:00
🛑 fix: Confirm Scheduled Stops and Separate Terminal Scheduler Failure from Startup (#15051)
* 🛑 fix: Confirm Scheduled Stops and Separate Terminal Scheduler Failure from Startup Fixes #15042, fixes #15043. `resume.js` inferred a confirmed stop from the ABSENCE of `failureReason`, but `abortJob` had four `success: false` paths that returned no reason at all. Those settled the occurrence as `interrupted` and pruned the checkpoint on aborts that never landed — including one where a REPLACEMENT generation owned the conversation, which pruned the successor's checkpoint. Every `success: false` return now names itself (`job_not_found`, `already_settled` added alongside the existing `generation_replaced` / `job_still_active`), and a single canonical `isStopConfirmed` predicate decides whether durable state may be settled. `already_settled` confirms a stop — `awaitProviderDrain` has proven the provider segment can no longer persist — so a permanently terminal generation is not answered with a retry loop. Separately, a schedule engine that failed to arm advertised its permanent outage as a transient 503 with `Retry-After`, so a client obeying it would poll forever. Readiness is now tri-state (`starting` / `armed` / `unavailable`): the retry contract applies only while arming is genuinely pending, and a failed arm returns a terminal `SCHEDULES_UNAVAILABLE` with no `Retry-After` and an error-level log. * 🏷️ fix: Declare Schedule Write Gate Return Types `--isolatedDeclarations` requires an explicit return type on the exported factory and on the middleware it returns (TS9007). Adds a named `ScheduleWriteGate` type matching the existing `ShareMiddleware` shape.
This commit is contained in:
parent
9c9696de8b
commit
c7e355b219
13 changed files with 495 additions and 36 deletions
|
|
@ -392,6 +392,82 @@ describe('ResumeAgentController (POST /agents/chat/resume)', () => {
|
|||
expect(mockGenerationJobManager.approvals.resolve).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// `abortJob` reports `success: false` with a REASON on every failure path. Gating on
|
||||
// the absence of a reason treated an unreached job and a replacement generation as
|
||||
// confirmed stops, settling the occurrence and pruning a checkpoint on neither.
|
||||
it.each([
|
||||
['the job vanished before the abort landed', 'job_not_found'],
|
||||
['a replacement generation owns the conversation', 'generation_replaced'],
|
||||
['the generation is still live', 'job_still_active'],
|
||||
])('refuses to settle or prune when %s', async (_label, failureReason) => {
|
||||
mockGenerationJobManager.getJob.mockResolvedValue(makeScheduledJob());
|
||||
mockIsScheduleLive.mockResolvedValue(false);
|
||||
mockGenerationJobManager.abortJob.mockResolvedValue({ success: false, failureReason });
|
||||
|
||||
const res = await post(approveBody());
|
||||
|
||||
expect(res.status).toBe(503);
|
||||
expect(res.headers['retry-after']).toBe('1');
|
||||
expect(res.body).toMatchObject({ code: 'SCHEDULE_STOP_UNCONFIRMED' });
|
||||
expect(mockRecordScheduleOutcome).not.toHaveBeenCalled();
|
||||
expect(mockDeleteAgentCheckpoint).not.toHaveBeenCalled();
|
||||
expect(mockGenerationJobManager.approvals.resolve).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// The exact regression: an abort that reported `success: false` and nothing else was
|
||||
// read as a confirmed stop, so the occurrence was settled and its checkpoint pruned.
|
||||
it('refuses to settle or prune on a bare unsuccessful abort with no reason', async () => {
|
||||
mockGenerationJobManager.getJob.mockResolvedValue(makeScheduledJob());
|
||||
mockIsScheduleLive.mockResolvedValue(false);
|
||||
mockGenerationJobManager.abortJob.mockResolvedValue({ success: false });
|
||||
|
||||
const res = await post(approveBody());
|
||||
|
||||
expect(res.status).toBe(503);
|
||||
expect(res.body).toMatchObject({ code: 'SCHEDULE_STOP_UNCONFIRMED' });
|
||||
expect(mockRecordScheduleOutcome).not.toHaveBeenCalled();
|
||||
expect(mockDeleteAgentCheckpoint).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('settles an occurrence whose generation was already terminal and drained', async () => {
|
||||
mockGenerationJobManager.getJob.mockResolvedValue(makeScheduledJob());
|
||||
mockIsScheduleLive.mockResolvedValue(false);
|
||||
// No transition was needed, but `awaitProviderDrain` still proved the provider
|
||||
// segment can no longer persist — a stop, just not one this call made. Refusing
|
||||
// here would 503 a permanently terminal generation on every retry.
|
||||
mockGenerationJobManager.abortJob.mockResolvedValue({
|
||||
success: false,
|
||||
failureReason: 'already_settled',
|
||||
});
|
||||
|
||||
const res = await post(approveBody());
|
||||
|
||||
expect(res.status).toBe(409);
|
||||
expect(res.body).toMatchObject({ code: 'SCHEDULE_NO_LONGER_ACTIVE' });
|
||||
expect(mockRecordScheduleOutcome).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ scheduleId: 'schedule-1', status: 'interrupted' }),
|
||||
);
|
||||
expect(mockDeleteAgentCheckpoint).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses to settle the stale resume handoff on an unconfirmed stop', async () => {
|
||||
mockGenerationJobManager.getJob.mockResolvedValue(makeScheduledJob());
|
||||
mockFinalizeScheduleResumeClaim.mockResolvedValue(false);
|
||||
mockGenerationJobManager.abortJob.mockResolvedValue({
|
||||
success: false,
|
||||
failureReason: 'generation_replaced',
|
||||
});
|
||||
|
||||
const res = await post(approveBody());
|
||||
|
||||
expect(res.status).toBe(503);
|
||||
expect(res.headers['retry-after']).toBe('1');
|
||||
expect(res.body).toMatchObject({ code: 'SCHEDULE_STOP_UNCONFIRMED' });
|
||||
expect(mockRecordScheduleOutcome).not.toHaveBeenCalled();
|
||||
expect(mockDeleteAgentCheckpoint).not.toHaveBeenCalled();
|
||||
expect(mockInitializeClient).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('records success after resumed persistence and before terminal publication', async () => {
|
||||
mockGenerationJobManager.getJob.mockResolvedValue(makeScheduledJob());
|
||||
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ const {
|
|||
decrementPendingRequest,
|
||||
checkAndIncrementPendingRequest,
|
||||
isSteerPreemptSupported,
|
||||
isStopConfirmed,
|
||||
toPendingSteer,
|
||||
} = require('@librechat/api');
|
||||
const { disposeClient } = require('~/server/cleanup');
|
||||
|
|
@ -608,7 +609,12 @@ const ResumeAgentController = async (req, res, next, initializeClient, addTitle)
|
|||
expectedCreatedAt: job.createdAt,
|
||||
awaitProviderDrain: true,
|
||||
});
|
||||
stopped = abortResult != null && abortResult.failureReason == null;
|
||||
// `success` is the authoritative signal, exactly as the abort route gates. A
|
||||
// `success: false` result WITHOUT a failure reason no longer exists — an
|
||||
// unreached job, a replacement, or a lost CAS all report one — so the old
|
||||
// `failureReason == null` test settled the occurrence and pruned the
|
||||
// checkpoint on aborts that were never confirmed.
|
||||
stopped = isStopConfirmed(abortResult);
|
||||
} catch (error) {
|
||||
logger.warn('[ResumeAgentController] Failed to stop inactive scheduled run', error);
|
||||
}
|
||||
|
|
@ -979,7 +985,9 @@ const ResumeAgentController = async (req, res, next, initializeClient, addTitle)
|
|||
expectedCreatedAt: job.createdAt,
|
||||
awaitProviderDrain: true,
|
||||
});
|
||||
stopped = abortResult != null && abortResult.failureReason == null;
|
||||
// Same authoritative gate as the inactive-schedule path above: only a landed
|
||||
// abort (or an already-terminal, drained generation) may settle this occurrence.
|
||||
stopped = isStopConfirmed(abortResult);
|
||||
} catch (error) {
|
||||
logger.warn('[ResumeAgentController] Failed to stop stale scheduled resume', error);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue