Skip to content

Commit c719f84

Browse files
authored
fix(core,sdk): stop chat.agent losing messages during recovery (#4907)
## Summary When a chat.agent run boots to continue a session (a version handover, or a retry after a crash), it replays the unacknowledged user messages off `session.in` and dispatches them itself. A previous change stopped the live tail from re-answering those same messages by folding them into the resume cursor in one step. That cursor is what the next boot reads to know where to resume, and folding in every recovered message at once let it advance past a message the run had not answered yet. So if the run answered the first recovered message, wrote its turn boundary, then crashed before dispatching the rest, the next boot resumed past those messages and they were never answered. ## Fix A recovered message is now claimed on the session-stream router instead of folded into the cursor. A claim does two independent things: - it drops the message however late the live tail re-delivers it, so a recovered message is never answered twice; - it holds the resume cursor behind that message until the boot has dispatched it, so a turn boundary never publishes a cursor past a message still waiting for a turn. The boot settles each claim as it dispatches the message, or right away for a message it folds into the seed chain or deliberately skips, so the cursor only advances over messages that have actually been handled. A claimed record whose route re-read never arrives over the tail degrades to being answered twice on the next boot, never to being dropped. Covered by router-level unit tests for the claim/settle floor and a chat.agent boot test asserting the cursor published after the first recovered turn stays behind the still-unanswered ones.
1 parent 6f5c49c commit c719f84

5 files changed

Lines changed: 413 additions & 33 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@trigger.dev/sdk": patch
3+
"@trigger.dev/core": patch
4+
---
5+
6+
`chat.agent`: a run that recovers a session with more than one in-flight user message no longer drops the unanswered ones if it restarts mid-recovery. Recovered messages now hold the resume cursor until each has been answered, so a restart re-answers the rest instead of resuming past them. Previously the cursor could advance past messages that were only held in memory, so a crash before they were dispatched lost them.

packages/core/src/v3/sessionStreams/router.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,3 +573,76 @@ describe("SessionChannelRouter: untake", () => {
573573
expect(r.pendingCount("messages")).toBe(1);
574574
});
575575
});
576+
577+
describe("SessionChannelRouter: recovered claim/settle floor", () => {
578+
it("drops a claimed record on ingest instead of queueing it", () => {
579+
const r = router();
580+
r.restore({ resumeFrom: 0 });
581+
r.markRecovered([1, 2]);
582+
expect(r.ingest(rec(1, "message"))).toEqual({ action: "drop", reason: "recovered" });
583+
expect(r.hasPending("messages")).toBe(false);
584+
});
585+
586+
it("holds the resume floor below the earliest owed record until it is settled", () => {
587+
const r = router();
588+
r.restore({ resumeFrom: 0 });
589+
r.markRecovered([1, 2]);
590+
expect(r.resumeFloor()).toBe(0);
591+
r.settleRecovered(1);
592+
expect(r.resumeFloor()).toBe(1);
593+
r.settleRecovered(2);
594+
expect(r.resumeFloor()).toBe(2);
595+
});
596+
597+
it("advances the floor after settling even when the tail never re-delivers", () => {
598+
const r = router();
599+
r.restore({ resumeFrom: 0 });
600+
r.markRecovered([1, 2]);
601+
r.settleRecovered(1);
602+
r.settleRecovered(2);
603+
expect(r.resumeFloor()).toBe(2);
604+
expect(r.appliedThrough()).toBe(2);
605+
});
606+
607+
it("keeps dropping a claimed record after it is settled, so a late tail re-read is never answered", () => {
608+
const r = router();
609+
r.restore({ resumeFrom: 0 });
610+
r.markRecovered([1]);
611+
r.settleRecovered(1);
612+
expect(r.ingest(rec(1, "message"))).toEqual({ action: "drop", reason: "recovered" });
613+
expect(r.hasPending("messages")).toBe(false);
614+
});
615+
616+
it("queues a live record whose sequence was never claimed", () => {
617+
const r = router();
618+
r.restore({ resumeFrom: 0 });
619+
r.markRecovered([1, 2]);
620+
expect(r.ingest(rec(3, "message"))).toEqual({ action: "queue", route: "messages" });
621+
});
622+
623+
it("advances only over the contiguous claimed run, holding the floor below a gap", () => {
624+
const r = router();
625+
r.restore({ resumeFrom: 0 });
626+
r.markRecovered([1, 3]);
627+
r.settleRecovered(1);
628+
r.settleRecovered(3);
629+
expect(r.resumeFloor()).toBe(1);
630+
});
631+
632+
it("holds the floor below an unclaimed gap while later claims are owed and the tail is silent", () => {
633+
const r = router();
634+
r.restore({ resumeFrom: 0 });
635+
r.markRecovered([1, 2, 5, 6]);
636+
r.settleRecovered(1);
637+
r.settleRecovered(2);
638+
expect(r.resumeFloor()).toBe(2);
639+
});
640+
641+
it("clears claims and owed records on reset", () => {
642+
const r = router();
643+
r.restore({ resumeFrom: 0 });
644+
r.markRecovered([1, 2]);
645+
r.reset();
646+
expect(r.ingest(rec(1, "message"))).toEqual({ action: "queue", route: "messages" });
647+
});
648+
});

packages/core/src/v3/sessionStreams/router.ts

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,14 @@ export type RouterDropReason =
6161
*/
6262
| "replayed"
6363
/** An `at-arrival` record with no handler attached right now. */
64-
| "no-handler";
64+
| "no-handler"
65+
/**
66+
* A record the boot took responsibility for over HTTP (see
67+
* {@link SessionChannelRouter.markRecovered}). It is dropped however late the
68+
* live tail re-delivers it, so a recovered message is never also answered as
69+
* a router turn.
70+
*/
71+
| "recovered";
6572

6673
export type RouterDecision =
6774
/** Handed to a consumer that was already waiting, or to a live handler. */
@@ -143,6 +150,8 @@ export class SessionChannelRouter {
143150
#highestSeq: number | undefined;
144151
#resumeFrom: number | undefined;
145152
#appliedThrough: number | undefined;
153+
#claimed = new Set<number>();
154+
#owed = new Set<number>();
146155
#onDrop?: (record: SessionStreamRecord, reason: RouterDropReason, route?: string) => void;
147156

148157
constructor(
@@ -197,6 +206,57 @@ export class SessionChannelRouter {
197206
return this.#resumeFrom;
198207
}
199208

209+
/**
210+
* Declare the sequences a continuation boot already read over HTTP and took
211+
* responsibility for dispatching itself, so the live tail's re-read of the
212+
* same records does not answer them a second time.
213+
*
214+
* Two effects, deliberately separate:
215+
*
216+
* - Every claimed sequence is dropped in {@link ingest} however late it
217+
* arrives (`"recovered"`), so a record the boot owns never also becomes a
218+
* router turn. This survives the boot settling it — the boot owns its
219+
* disposition for the whole run, and the tail may re-deliver at any time.
220+
* - Each claimed sequence is also *owed*: it holds the resume floor exactly
221+
* like a queued record would, until the boot {@link settleRecovered}s it.
222+
* Suppressing the second delivery without this would let a turn boundary
223+
* publish a floor past a message the boot has not answered yet, turning a
224+
* duplicate into a dropped message.
225+
*
226+
* `#highestSeq` is advanced over the contiguous run of claimed sequences from
227+
* the current high water, so once every claim is settled the floor can move
228+
* past them even if the tail never re-delivers (a silent tail then degrades
229+
* to a duplicate, never a loss). The advance stops at the first gap so a
230+
* record the boot left for the router is never skipped.
231+
*/
232+
markRecovered(seqNums: Iterable<number>): void {
233+
const nums = [...seqNums].filter((n) => Number.isFinite(n)).sort((a, b) => a - b);
234+
if (nums.length === 0) return;
235+
for (const n of nums) {
236+
this.#claimed.add(n);
237+
this.#owed.add(n);
238+
}
239+
let base = this.#highestSeq ?? nums[0]! - 1;
240+
while (this.#claimed.has(base + 1)) base++;
241+
if (this.#highestSeq === undefined || base > this.#highestSeq) {
242+
this.#highestSeq = base;
243+
}
244+
const maxClaimed = nums[nums.length - 1]!;
245+
if (this.#appliedThrough === undefined || maxClaimed > this.#appliedThrough) {
246+
this.#appliedThrough = maxClaimed;
247+
}
248+
}
249+
250+
/**
251+
* Release a claimed sequence's hold on the resume floor once the boot has
252+
* decided its disposition (dispatched it as a turn, folded it into the seed
253+
* chain, or deliberately dropped it). It stays claimed, so a late tail
254+
* re-delivery is still dropped rather than answered again.
255+
*/
256+
settleRecovered(seqNum: number): void {
257+
this.#owed.delete(seqNum);
258+
}
259+
200260
/**
201261
* Classify one record and act on it. The record's destination is decided
202262
* here, once, and never by whichever consumer happens to be waiting.
@@ -213,6 +273,10 @@ export class SessionChannelRouter {
213273
}
214274
}
215275

276+
if (this.#claimed.has(record.seqNum)) {
277+
return this.#drop(record, "recovered");
278+
}
279+
216280
const kind = this.#kindOf(record.data);
217281
if (kind === undefined) {
218282
return this.#drop(record, "malformed");
@@ -308,6 +372,9 @@ export class SessionChannelRouter {
308372
const pending = state.earliestUnrecovered();
309373
if (pending !== undefined) earliestPending = Math.min(earliestPending, pending);
310374
}
375+
for (const owed of this.#owed) {
376+
if (owed < earliestPending) earliestPending = owed;
377+
}
311378

312379
if (earliestPending === Infinity) return this.#highestSeq;
313380

@@ -518,5 +585,7 @@ export class SessionChannelRouter {
518585
this.#highestSeq = undefined;
519586
this.#resumeFrom = undefined;
520587
this.#appliedThrough = undefined;
588+
this.#claimed.clear();
589+
this.#owed.clear();
521590
}
522591
}

packages/trigger-sdk/src/v3/ai.ts

Lines changed: 61 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2322,7 +2322,11 @@ async function findSessionInReplayWindowEnd(
23222322
*/
23232323
async function installChatInputRouter(
23242324
chatId: string,
2325-
options?: { fallbackResumeFrom?: number; recoveredThrough?: number; resuming?: boolean }
2325+
options?: {
2326+
fallbackResumeFrom?: number;
2327+
recoveredSeqNums?: readonly number[];
2328+
resuming?: boolean;
2329+
}
23262330
): Promise<SessionChannelRouter> {
23272331
const entry = chatInputRouterEntry(chatId);
23282332
if (entry.attached) return entry.router;
@@ -2353,20 +2357,13 @@ async function installChatInputRouter(
23532357
}
23542358
}
23552359

2356-
// A boot that replayed `.in` itself has already answered everything up to
2357-
// `recoveredThrough`, so the floor has to cover it before the tail opens.
2358-
if (options?.recoveredThrough !== undefined) {
2359-
const recovered = options.recoveredThrough;
2360-
checkpoint.resumeFrom = Math.max(checkpoint.resumeFrom ?? recovered, recovered);
2361-
checkpoint.appliedThrough = Math.max(
2362-
checkpoint.appliedThrough ?? checkpoint.resumeFrom,
2363-
checkpoint.resumeFrom
2364-
);
2365-
}
2366-
23672360
const router = entry.router;
23682361
router.restore(checkpoint);
23692362

2363+
if (options?.recoveredSeqNums && options.recoveredSeqNums.length > 0) {
2364+
router.markRecovered(options.recoveredSeqNums);
2365+
}
2366+
23702367
const floor = router.resumeFrom();
23712368
if (floor !== undefined) {
23722369
sessionStreams.setLastSeqNum(chatId, "in", floor);
@@ -7272,6 +7269,20 @@ function chatAgent<
72727269
// `messagesInput.waitWithIdleTimeout` so recovered turns fire first.
72737270
const bootInjectedQueue: ChatTaskWirePayload<TUIMessage, inferSchemaIn<TClientDataSchema>>[] =
72747271
[];
7272+
const recoveredSeqByPayload = new WeakMap<
7273+
ChatTaskWirePayload<TUIMessage, inferSchemaIn<TClientDataSchema>>,
7274+
number
7275+
>();
7276+
const dispatchBootInjected = (): ChatTaskWirePayload<
7277+
TUIMessage,
7278+
inferSchemaIn<TClientDataSchema>
7279+
> => bootInjectedQueue.shift()!;
7280+
const settleRecoveredTurn = (
7281+
wirePayload: ChatTaskWirePayload<TUIMessage, inferSchemaIn<TClientDataSchema>>
7282+
) => {
7283+
const settledSeq = recoveredSeqByPayload.get(wirePayload);
7284+
if (settledSeq !== undefined) chatInputRouter().settleRecovered(settledSeq);
7285+
};
72757286
const couldHavePriorState = payload.continuation === true || ctx.attempt.number > 1;
72767287

72777288
// `.in` resume cursor, computed at most once per boot. The boot
@@ -7437,18 +7448,11 @@ function chatAgent<
74377448

74387449
// ── session.in router ──────────────────────────────────────────
74397450
//
7440-
// Reads the turn boundary and subscribes in one call. `bootInCursor` is
7441-
// only a fallback: the boot block above may already have resolved a
7442-
// cursor from the snapshot, which is used when the boundary itself
7443-
// carries none. Everything the boot replayed off `.in` is dispatched from
7444-
// `bootInjectedQueue` below, so it goes into the floor here — folded in
7445-
// after the subscription opens, the live tail re-delivers it as a turn.
7446-
const lastRecoveredInSeq =
7447-
replayedInTail.length > 0 ? replayedInTail[replayedInTail.length - 1]!.seqNum : undefined;
7451+
const recoveredSeqNums = replayedInTail.map((r) => r.seqNum);
74487452

74497453
await installChatInputRouter(payload.chatId, {
74507454
fallbackResumeFrom: bootInCursorResolved ? bootInCursor : undefined,
7451-
recoveredThrough: lastRecoveredInSeq,
7455+
recoveredSeqNums,
74527456
resuming: Boolean(payload.continuation) || ctx.attempt.number > 1,
74537457
});
74547458

@@ -7538,7 +7542,7 @@ function chatAgent<
75387542
// branches: at n=1 the orphan partial is dropped and the interrupted
75397543
// user is re-dispatched as a fresh turn instead.
75407544
let seedChain: TUIMessage[];
7541-
let recoveredTurns: TUIMessage[];
7545+
let recoveredEntries: { message: TUIMessage; seqNum: number | undefined }[];
75427546
if (hookChain !== undefined) {
75437547
seedChain = hookChain;
75447548
} else if (partialAssistant !== undefined && inFlightUsers.length > 1) {
@@ -7547,11 +7551,22 @@ function chatAgent<
75477551
seedChain = settledMessages;
75487552
}
75497553
if (hookRecoveredTurns !== undefined) {
7550-
recoveredTurns = hookRecoveredTurns;
7554+
const seqNumsByRecoveredId = new Map<string, number[]>();
7555+
for (const entry of replayedInTail) {
7556+
const existing = seqNumsByRecoveredId.get(entry.message.id);
7557+
if (existing) existing.push(entry.seqNum);
7558+
else seqNumsByRecoveredId.set(entry.message.id, [entry.seqNum]);
7559+
}
7560+
recoveredEntries = hookRecoveredTurns.map((message) => ({
7561+
message,
7562+
seqNum: seqNumsByRecoveredId.get(message.id)?.shift(),
7563+
}));
75517564
} else if (partialAssistant !== undefined && inFlightUsers.length > 1) {
7552-
recoveredTurns = inFlightUsers.slice(1);
7565+
recoveredEntries = replayedInTail
7566+
.slice(1)
7567+
.map((r) => ({ message: r.message, seqNum: r.seqNum }));
75537568
} else {
7554-
recoveredTurns = inFlightUsers;
7569+
recoveredEntries = replayedInTail.map((r) => ({ message: r.message, seqNum: r.seqNum }));
75557570
}
75567571
// `beforeBoot` errors bubble — the customer opted into blocking
75577572
// persistence and a failure there should fail the run rather than
@@ -7582,12 +7597,13 @@ function chatAgent<
75827597
for (const entry of replayedInTail) {
75837598
metadataById.set(entry.message.id, entry.metadata);
75847599
}
7585-
for (const msg of recoveredTurns) {
7600+
const dispatchedRecoveredSeqs = new Set<number>();
7601+
for (const { message: msg, seqNum } of recoveredEntries) {
75867602
if (wireMessageId && msg.id === wireMessageId) continue;
75877603
const recoveredMetadata = metadataById.has(msg.id)
75887604
? metadataById.get(msg.id)
75897605
: payload.metadata;
7590-
bootInjectedQueue.push({
7606+
const injectedPayload = {
75917607
chatId: payload.chatId,
75927608
sessionId: payload.sessionId,
75937609
metadata: recoveredMetadata,
@@ -7596,7 +7612,17 @@ function chatAgent<
75967612
messageId: msg.id,
75977613
continuation: payload.continuation,
75987614
previousRunId: payload.previousRunId,
7599-
} as ChatTaskWirePayload<TUIMessage, inferSchemaIn<TClientDataSchema>>);
7615+
} as ChatTaskWirePayload<TUIMessage, inferSchemaIn<TClientDataSchema>>;
7616+
bootInjectedQueue.push(injectedPayload);
7617+
if (seqNum !== undefined) {
7618+
recoveredSeqByPayload.set(injectedPayload, seqNum);
7619+
dispatchedRecoveredSeqs.add(seqNum);
7620+
}
7621+
}
7622+
for (const entry of replayedInTail) {
7623+
if (!dispatchedRecoveredSeqs.has(entry.seqNum)) {
7624+
chatInputRouter().settleRecovered(entry.seqNum);
7625+
}
76007626
}
76017627

76027628
accumulatedUIMessages = seedChain;
@@ -7780,7 +7806,7 @@ function chatAgent<
77807806
*/
77817807
let dispatchedRecoveredFirstTurn = false;
77827808
if (preloaded && bootInjectedQueue.length > 0) {
7783-
currentWirePayload = bootInjectedQueue.shift()!;
7809+
currentWirePayload = dispatchBootInjected();
77847810
dispatchedRecoveredFirstTurn = true;
77857811
}
77867812

@@ -8031,7 +8057,7 @@ function chatAgent<
80318057
// waiting on the live session.in. Subsequent recovered turns
80328058
// get drained by the end-of-turn picker below.
80338059
if (bootInjectedQueue.length > 0) {
8034-
currentWirePayload = bootInjectedQueue.shift()!;
8060+
currentWirePayload = dispatchBootInjected();
80358061
} else {
80368062
const effectiveIdleTimeout = idleTimeoutInSeconds ?? payload.idleTimeoutInSeconds;
80378063
const effectiveTurnTimeout =
@@ -8686,6 +8712,7 @@ function chatAgent<
86868712
chatId: currentWirePayload.chatId,
86878713
messageId: currentWirePayload.messageId,
86888714
});
8715+
settleRecoveredTurn(currentWirePayload);
86898716
await writeTurnCompleteChunk(currentWirePayload.chatId);
86908717
// Not a turn — don't consume an iteration.
86918718
turn--;
@@ -9504,6 +9531,8 @@ function chatAgent<
95049531
locals.set(chatResponsePartsKey, []);
95059532
}
95069533

9534+
settleRecoveredTurn(currentWirePayload);
9535+
95079536
// Write turn-complete control chunk — closes the frontend stream.
95089537
const turnCompleteResult = await writeTurnCompleteChunk(
95099538
currentWirePayload.chatId,
@@ -9634,7 +9663,7 @@ function chatAgent<
96349663
// produced these from in-flight user messages on session.in
96359664
// that the dead predecessor never acknowledged.
96369665
if (bootInjectedQueue.length > 0) {
9637-
currentWirePayload = bootInjectedQueue.shift()!;
9666+
currentWirePayload = dispatchBootInjected();
96389667
return "continue";
96399668
}
96409669

@@ -10012,7 +10041,7 @@ function chatAgent<
1001210041
// recovered turn shouldn't strand the rest of the boot queue
1001310042
// until an unrelated live message arrives.
1001410043
if (bootInjectedQueue.length > 0) {
10015-
currentWirePayload = bootInjectedQueue.shift()!;
10044+
currentWirePayload = dispatchBootInjected();
1001610045
continue;
1001710046
}
1001810047

0 commit comments

Comments
 (0)