Skip to content

Commit 6abc529

Browse files
committed
fix(chat): keep mailbox cursor behind pending input
1 parent 39b5c57 commit 6abc529

10 files changed

Lines changed: 462 additions & 146 deletions

File tree

docs/ai-chat/custom-agents.mdx

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -228,14 +228,15 @@ For full control, skip `createSession` and compose the primitives directly:
228228
| Method | Behavior |
229229
| --- | --- |
230230
| `peek()` | Return the buffer head when it is a message, without consuming it; otherwise return `undefined` |
231-
| `hasPending()` | Resolve `true` when any message is buffered; does not consume it |
231+
| `hasPending()` | Resolve `true` when the buffer head is a message; does not consume it |
232232
| `next({ timeoutInSeconds? })` | Consume exactly one message record in channel order, or resolve `undefined` when the optional timeout elapses |
233233
| `on(handler)` | Consume messages as they arrive and invoke the handler |
234234
| `waitWithIdleTimeout(options)` | Wait warm, then suspend the run until the next message arrives |
235235

236-
`hasPending()` checks the local, already-delivered buffer. It does not query the
237-
remote Session channel or start a subscription. Use `waitWithIdleTimeout()` when
238-
the loop needs to idle until future input arrives.
236+
`hasPending()` checks whether the local, already-delivered buffer head is a
237+
message that `next()` can consume immediately. It does not query the remote
238+
Session channel or start a subscription. Use `waitWithIdleTimeout()` when the
239+
loop needs to idle until future input arrives.
239240

240241
`next()` returns a readonly record envelope:
241242

@@ -258,8 +259,9 @@ contrast, `on()` commits a record as soon as it dispatches the handler; avoid
258259
mixing `on()` and `next()` when a single loop owns mailbox consumption.
259260

260261
The Session `.in` channel also carries control records such as handovers. If one
261-
comes before a message, `next()` leaves it for its own consumer and waits until
262-
that record has been handled.
262+
comes before a message, `hasPending()` stays `false` and `next()` leaves the
263+
control record for its own consumer. After that record is handled, the message
264+
becomes pending.
263265

264266
A complete loop:
265267

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -86,18 +86,6 @@ export class SessionStreamsAPI implements SessionStreamManager {
8686
return manager.peekRecord(sessionId, io);
8787
}
8888

89-
public peekRecordWhere(
90-
sessionId: string,
91-
io: SessionChannelIO,
92-
predicate: SessionStreamRecordPredicate
93-
): SessionStreamRecord | undefined {
94-
const manager = this.#getManager();
95-
if (!manager.peekRecordWhere) {
96-
throw new Error("The configured Session stream manager does not support selective records");
97-
}
98-
return manager.peekRecordWhere(sessionId, io, predicate);
99-
}
100-
10189
public lastSeqNum(sessionId: string, io: SessionChannelIO): number | undefined {
10290
return this.#getManager().lastSeqNum(sessionId, io);
10391
}

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

Lines changed: 201 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
22
import { StandardSessionStreamManager } from "./manager.js";
33
import type { ApiClient } from "../apiClient/index.js";
44
import type { SSEStreamPart } from "../apiClient/runStream.js";
5+
import { InputStreamTimeoutError } from "../inputStreams/types.js";
56

67
// Single-shot mock that mimics S2's long-poll: delivers `records` once via
78
// `onPart` on the first subscribe call, then keeps the returned async
@@ -44,6 +45,31 @@ function singleShotApiClient(
4445
} as unknown as ApiClient;
4546
}
4647

48+
function repeatingApiClient(record: {
49+
id: string;
50+
recordId?: string;
51+
chunk: unknown;
52+
timestamp: number;
53+
}): ApiClient {
54+
return {
55+
async subscribeToSessionStream<T>(
56+
_sessionIdOrExternalId: string,
57+
_io: "out" | "in",
58+
options?: { onPart?: (part: SSEStreamPart<T>) => void; signal?: AbortSignal }
59+
) {
60+
options?.onPart?.(record as SSEStreamPart<T>);
61+
const signal = options?.signal;
62+
// eslint-disable-next-line require-yield
63+
return (async function* () {
64+
if (signal?.aborted) return;
65+
await new Promise<void>((resolve) => {
66+
signal?.addEventListener("abort", () => resolve(), { once: true });
67+
});
68+
})() as unknown as Awaited<ReturnType<ApiClient["subscribeToSessionStream"]>>;
69+
},
70+
} as unknown as ApiClient;
71+
}
72+
4773
describe("StandardSessionStreamManager — minTimestamp filter", () => {
4874
const sessionId = "session-1";
4975
const io = "in" as const;
@@ -210,24 +236,39 @@ describe("StandardSessionStreamManager — record metadata", () => {
210236
});
211237

212238
it("returns the same envelope when a record is redelivered", async () => {
213-
const firstDelivery = new StandardSessionStreamManager(
214-
singleShotApiClient([records[0]!]),
239+
const manager = new StandardSessionStreamManager(
240+
repeatingApiClient(records[0]!),
215241
"http://localhost"
216242
);
217-
const redelivery = new StandardSessionStreamManager(
218-
singleShotApiClient([records[0]!]),
243+
244+
const first = await manager.onceRecord(sessionId, io);
245+
manager.disconnectStream(sessionId, io);
246+
const replayed = await manager.onceRecord(sessionId, io);
247+
248+
expect(first).toEqual(replayed);
249+
250+
manager.disconnectStream(sessionId, io);
251+
manager.disconnect();
252+
});
253+
254+
it("returns immediately when the timeout is zero", async () => {
255+
const manager = new StandardSessionStreamManager(
256+
{
257+
subscribeToSessionStream: () => {
258+
throw new Error("zero-timeout reads must not subscribe");
259+
},
260+
} as unknown as ApiClient,
219261
"http://localhost"
220262
);
221263

222-
const first = await firstDelivery.onceRecord(sessionId, io);
223-
const replayed = await redelivery.onceRecord(sessionId, io);
264+
const result = await manager.onceRecord(sessionId, io, { timeoutMs: 0 });
224265

225-
expect(first).toEqual(replayed);
266+
expect(result.ok).toBe(false);
267+
if (!result.ok) {
268+
expect(result.error).toBeInstanceOf(InputStreamTimeoutError);
269+
}
226270

227-
firstDelivery.disconnectStream(sessionId, io);
228-
firstDelivery.disconnect();
229-
redelivery.disconnectStream(sessionId, io);
230-
redelivery.disconnect();
271+
manager.disconnect();
231272
});
232273

233274
it("does not consume a matching record past an earlier unmatched record", async () => {
@@ -256,10 +297,10 @@ describe("StandardSessionStreamManager — record metadata", () => {
256297
{ timeoutMs: 200 }
257298
);
258299

259-
expect(manager.peekRecordWhere(sessionId, io, (record) => record.id === "message-1")).toEqual({
260-
id: "message-1",
261-
seqNum: 51,
262-
data: { kind: "message", payload: { id: "u1" } },
300+
expect(manager.peekRecord(sessionId, io)).toEqual({
301+
id: "handover-1",
302+
seqNum: 50,
303+
data: { kind: "handover" },
263304
});
264305
expect(manager.lastDispatchedSeqNum(sessionId, io)).toBeUndefined();
265306

@@ -281,4 +322,149 @@ describe("StandardSessionStreamManager — record metadata", () => {
281322
manager.disconnectStream(sessionId, io);
282323
manager.disconnect();
283324
});
325+
326+
it("keeps the persisted cursor behind each earlier buffered record", async () => {
327+
const manager = new StandardSessionStreamManager(
328+
singleShotApiClient([
329+
{
330+
id: "50",
331+
recordId: "message-1",
332+
chunk: { kind: "message", payload: { id: "u1" } },
333+
timestamp: 1000,
334+
},
335+
{
336+
id: "51",
337+
recordId: "stop-1",
338+
chunk: { kind: "stop" },
339+
timestamp: 2000,
340+
},
341+
{
342+
id: "52",
343+
recordId: "message-2",
344+
chunk: { kind: "message", payload: { id: "u2" } },
345+
timestamp: 3000,
346+
},
347+
{
348+
id: "53",
349+
recordId: "stop-2",
350+
chunk: { kind: "stop" },
351+
timestamp: 4000,
352+
},
353+
]),
354+
"http://localhost"
355+
);
356+
let resolveStop!: () => void;
357+
let remainingStops = 2;
358+
const stopConsumed = new Promise<void>((resolve) => {
359+
resolveStop = resolve;
360+
});
361+
362+
manager.on(sessionId, io, (data) => {
363+
if ((data as { kind?: string }).kind !== "stop") return;
364+
remainingStops--;
365+
if (remainingStops === 0) resolveStop();
366+
return true;
367+
});
368+
await stopConsumed;
369+
370+
expect(manager.peekRecord(sessionId, io)).toEqual({
371+
id: "message-1",
372+
seqNum: 50,
373+
data: { kind: "message", payload: { id: "u1" } },
374+
});
375+
expect(manager.lastDispatchedSeqNum(sessionId, io)).toBe(49);
376+
377+
const firstMessage = await manager.onceRecord(sessionId, io);
378+
expect(firstMessage.ok && firstMessage.output.id).toBe("message-1");
379+
expect(manager.lastDispatchedSeqNum(sessionId, io)).toBe(51);
380+
381+
const secondMessage = await manager.onceRecord(sessionId, io);
382+
expect(secondMessage.ok && secondMessage.output.id).toBe("message-2");
383+
expect(manager.lastDispatchedSeqNum(sessionId, io)).toBe(53);
384+
385+
manager.disconnectStream(sessionId, io);
386+
manager.disconnect();
387+
});
388+
389+
it("retains cursor barriers when disconnect clears the buffer", async () => {
390+
const manager = new StandardSessionStreamManager(
391+
singleShotApiClient([
392+
{
393+
id: "50",
394+
recordId: "message-1",
395+
chunk: { kind: "message", payload: { id: "u1" } },
396+
timestamp: 1000,
397+
},
398+
{
399+
id: "51",
400+
recordId: "stop-1",
401+
chunk: { kind: "stop" },
402+
timestamp: 2000,
403+
},
404+
]),
405+
"http://localhost"
406+
);
407+
let resolveStop!: () => void;
408+
const stopConsumed = new Promise<void>((resolve) => {
409+
resolveStop = resolve;
410+
});
411+
412+
manager.on(sessionId, io, (data) => {
413+
if ((data as { kind?: string }).kind !== "stop") return;
414+
resolveStop();
415+
return true;
416+
});
417+
await stopConsumed;
418+
419+
expect(manager.lastDispatchedSeqNum(sessionId, io)).toBe(49);
420+
manager.disconnectStream(sessionId, io);
421+
expect(manager.peekRecord(sessionId, io)).toBeUndefined();
422+
expect(manager.lastDispatchedSeqNum(sessionId, io)).toBe(49);
423+
424+
manager.setLastDispatchedSeqNum(sessionId, io, 51);
425+
expect(manager.lastDispatchedSeqNum(sessionId, io)).toBe(49);
426+
427+
manager.reset();
428+
expect(manager.lastDispatchedSeqNum(sessionId, io)).toBeUndefined();
429+
});
430+
431+
it("does not expose a negative cursor when sequence zero is buffered", async () => {
432+
const manager = new StandardSessionStreamManager(
433+
singleShotApiClient([
434+
{
435+
id: "0",
436+
recordId: "message-0",
437+
chunk: { kind: "message", payload: { id: "u0" } },
438+
timestamp: 1000,
439+
},
440+
{
441+
id: "1",
442+
recordId: "stop-1",
443+
chunk: { kind: "stop" },
444+
timestamp: 2000,
445+
},
446+
]),
447+
"http://localhost"
448+
);
449+
let resolveStop!: () => void;
450+
const stopConsumed = new Promise<void>((resolve) => {
451+
resolveStop = resolve;
452+
});
453+
454+
manager.on(sessionId, io, (data) => {
455+
if ((data as { kind?: string }).kind !== "stop") return;
456+
resolveStop();
457+
return true;
458+
});
459+
await stopConsumed;
460+
461+
expect(manager.lastDispatchedSeqNum(sessionId, io)).toBeUndefined();
462+
463+
const message = await manager.onceRecord(sessionId, io);
464+
expect(message.ok && message.output.id).toBe("message-0");
465+
expect(manager.lastDispatchedSeqNum(sessionId, io)).toBe(1);
466+
467+
manager.disconnectStream(sessionId, io);
468+
manager.disconnect();
469+
});
284470
});

0 commit comments

Comments
 (0)