diff --git a/packages/api/src/schedules/fire.spec.ts b/packages/api/src/schedules/fire.spec.ts index 76bf1a5787..db328db297 100644 --- a/packages/api/src/schedules/fire.spec.ts +++ b/packages/api/src/schedules/fire.spec.ts @@ -281,6 +281,28 @@ describe('fireSchedule', () => { expect(calls.advance).toBe(1); }); + it('treats a message-limiter 429 as a skip, not a schedule failure', async () => { + const { methods, runs } = makeMethods(); + mockFetch( + async () => + ({ + ok: false, + status: 429, + text: async () => '{"message":"Too many requests"}', + }) as Response, + ); + const result = await fireSchedule(makeDeps(methods), makeSchedule(), LIMITS, dueAt(), { + manual: true, + }); + expect(result.skipped).toBe('rate_limited'); + // Counting this as a failure would let an owner merely over their message quota + // auto-disable a healthy schedule by clicking Run Now enough times. + expect(methods.recordRunOutcome).not.toHaveBeenCalled(); + // Nothing reached the controller, so no outcome was recorded for the occurrence and + // the reservation must not be left holding its capacity slot. + expect([...runs.values()].filter((r) => r.status === 'started')).toHaveLength(0); + }); + it('records a definite HTTP rejection as error', async () => { const { methods, calls } = makeMethods(); mockFetch(async () => ({ ok: false, status: 500, text: async () => 'boom' }) as Response); diff --git a/packages/api/src/schedules/fire.ts b/packages/api/src/schedules/fire.ts index 65bc3ec547..bfdfbb5e1f 100644 --- a/packages/api/src/schedules/fire.ts +++ b/packages/api/src/schedules/fire.ts @@ -27,6 +27,11 @@ class ScheduleFireError extends Error { * controller already recorded the occurrence's outcome — so this is a SKIP, not a * fault, and must not count toward auto-disable. */ readonly preStartAbort = false, + /** The server's own message limiter refused the fire before it reached the + * controller. Reachable for a manual Run Now, which is deliberately NOT exempt from + * the interactive limiters. Nothing started and the schedule is not at fault, so + * this must not count toward auto-disable either. */ + readonly throttled = false, ) { super(message); } @@ -162,10 +167,15 @@ async function postChatMessageInner( } if (!response.ok) { const body = await response.text().catch(() => ''); - // A received error response is a definite rejection (nothing started). + // A received error response is a definite rejection (nothing started). 429 is the + // exception: the server's OWN message limiter refused the request, which says + // nothing about the schedule's health. Only a manual Run Now can reach it, since + // automatic occurrences are exempt. throw new ScheduleFireError( `Fire POST failed (${response.status}): ${body.slice(0, 300)}`, false, + false, + response.status === 429, ); } // The accept path always answers with JSON ({ streamId, conversationId, status }). @@ -530,6 +540,19 @@ export async function fireSchedule( await advance(); return { fired: false, skipped: 'superseded' as const }; } + if (error instanceof ScheduleFireError && error.throttled) { + // The server's own message limiter refused this before it reached the + // controller, so nothing was billed and the SCHEDULE is not at fault. Counting + // it as a failure would let an owner who is merely over their message quota + // auto-disable a perfectly healthy schedule by clicking Run Now enough times. + // Unlike the controller fence above, nothing recorded an outcome for this + // occurrence, so the reservation has to be rolled back or it holds its capacity + // slot and blocks overlap until the orphan sweep. + logger.info(`[schedules] fire refused by the message limiter for ${schedule.id}`); + await rollbackReservation(); + await advance(); + return { fired: false, skipped: 'rate_limited' as const }; + } // Definite rejection (an error response was received): nothing started. logger.error(`[schedules] fire rejected for ${schedule.id}:`, error); await methods.recordRunOutcome({ diff --git a/packages/api/src/schedules/handlers.ts b/packages/api/src/schedules/handlers.ts index cb040635c5..9dd88ebed7 100644 --- a/packages/api/src/schedules/handlers.ts +++ b/packages/api/src/schedules/handlers.ts @@ -412,8 +412,13 @@ export function createSchedulesHandlers(deps: SchedulesHandlersDeps): SchedulesH return; } if (!result.fired) { - res.status(409).json({ - error: result.error ?? `Run skipped (${result.skipped ?? 'unknown'})`, + // A limiter refusal is the caller's own quota, not a conflicting schedule state, + // so answer 429 rather than burying it in the generic 409. + res.status(result.skipped === 'rate_limited' ? 429 : 409).json({ + error: + result.skipped === 'rate_limited' + ? 'Too many messages. Try running this schedule again shortly.' + : (result.error ?? `Run skipped (${result.skipped ?? 'unknown'})`), skipped: result.skipped, }); return; diff --git a/packages/api/src/schedules/types.ts b/packages/api/src/schedules/types.ts index f3d03c83df..0708b26d60 100644 --- a/packages/api/src/schedules/types.ts +++ b/packages/api/src/schedules/types.ts @@ -138,6 +138,7 @@ export interface FireResult { | 'user_missing' | 'user_deleting' | 'permission_revoked' + | 'rate_limited' | 'disabled'; error?: string; }