Skip to content

Commit 5839cc3

Browse files
committed
fix(chat): hold iterator guard through pending reads
1 parent b729865 commit 5839cc3

6 files changed

Lines changed: 207 additions & 79 deletions

File tree

apps/webapp/test/helpers/testChatAgent.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,57 @@ export const testEndAndContinueCustomAgent = chat.customAgent({
269269
},
270270
});
271271

272+
export const endAndContinueGuardEvents: Array<{
273+
chatId: string;
274+
kind: "guard-held" | "return-settled";
275+
}> = [];
276+
277+
const activeSessionIteratorError =
278+
"chat.endAndContinue() cannot be called while a chat.createSession() iterator is active. Close the iterator, then call chat.endAndContinue().";
279+
280+
async function expectActiveSessionIteratorError() {
281+
try {
282+
await chat.endAndContinue();
283+
} catch (error) {
284+
if (error instanceof Error && error.message === activeSessionIteratorError) return;
285+
throw error;
286+
}
287+
throw new Error("Expected chat.endAndContinue() to reject while the iterator is active");
288+
}
289+
290+
/** Exercises a return racing an already-started next() against real Session input. */
291+
export const testEndAndContinueIteratorGuardCustomAgent = chat.customAgent({
292+
id: "e2e-test-chat-custom-end-and-continue-iterator-guard",
293+
run: async (payload, { signal }) => {
294+
const iterator = chat.createSession(payload, { signal })[Symbol.asyncIterator]();
295+
const firstTurn = await iterator.next();
296+
if (firstTurn.done) {
297+
throw new Error("Expected an initial chat turn");
298+
}
299+
await firstTurn.value.done();
300+
await expectActiveSessionIteratorError();
301+
302+
const pendingNext = iterator.next();
303+
if (!iterator.return) {
304+
throw new Error("Expected the chat Session iterator to support return()");
305+
}
306+
const pendingReturn = iterator.return();
307+
308+
// Let an immediately-resolving return() clear a broken guard before checking it.
309+
await new Promise((resolve) => setTimeout(resolve, 0));
310+
await expectActiveSessionIteratorError();
311+
endAndContinueGuardEvents.push({ chatId: payload.chatId, kind: "guard-held" });
312+
313+
const [nextResult] = await Promise.all([pendingNext, pendingReturn]);
314+
if (nextResult.done) {
315+
throw new Error("Expected the pending next() call to receive the release input");
316+
}
317+
endAndContinueGuardEvents.push({ chatId: payload.chatId, kind: "return-settled" });
318+
319+
await chat.endAndContinue();
320+
},
321+
});
322+
272323
/**
273324
* A tool with a server-side `execute`: the agent runs it automatically and
274325
* feeds the result back to the model, so a single turn covers the whole

apps/webapp/test/session-agent.e2e.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@ import {
2929
} from "./helpers/sessionStream";
3030
import { runChatAgentSession, runRealChatAgent } from "./helpers/agentHarness";
3131
import {
32+
endAndContinueGuardEvents,
3233
suspendResumeEvents,
3334
testApprovalChatAgent,
3435
testChatAgent,
3536
testChatModelLocal,
3637
testEndAndContinueCustomAgent,
38+
testEndAndContinueIteratorGuardCustomAgent,
3739
testEndRunChatAgent,
3840
testHitlChatAgent,
3941
testHitlIdleChatAgent,
@@ -1655,4 +1657,90 @@ describe("session agent e2e (real chat.agent loop)", () => {
16551657
await agent.close();
16561658
}
16571659
});
1660+
1661+
it("EA25: custom endAndContinue keeps the guard while iterator next is active", async () => {
1662+
const { addressingKey, publicAccessToken, runId, apiKey, environment, baseUrl } =
1663+
await setupStartedSession(testEndAndContinueIteratorGuardCustomAgent.id);
1664+
const initialRun = await server.prisma.taskRun.findFirstOrThrow({
1665+
where: { friendlyId: runId },
1666+
select: { id: true },
1667+
});
1668+
1669+
const append = await appendInput({
1670+
baseUrl,
1671+
addressingKey,
1672+
token: publicAccessToken,
1673+
partId: "iterator-guard-initial-input",
1674+
body: submitBody(
1675+
addressingKey,
1676+
userMessage("start iterator guard test", "iterator-guard-initial-input")
1677+
),
1678+
});
1679+
expect(append.status).toBe(200);
1680+
1681+
const agent = runRealChatAgent({
1682+
agentId: testEndAndContinueIteratorGuardCustomAgent.id,
1683+
baseUrl,
1684+
addressingKey,
1685+
secretKey: apiKey,
1686+
model: textModel("unused"),
1687+
modelLocal: testChatModelLocal,
1688+
runId,
1689+
});
1690+
let agentSettled = false;
1691+
let agentFailure: unknown;
1692+
void agent.done.then(
1693+
() => {
1694+
agentSettled = true;
1695+
},
1696+
(error) => {
1697+
agentSettled = true;
1698+
agentFailure = error;
1699+
}
1700+
);
1701+
1702+
try {
1703+
await waitFor(
1704+
() =>
1705+
agentSettled ||
1706+
endAndContinueGuardEvents.some(
1707+
(event) => event.chatId === addressingKey && event.kind === "guard-held"
1708+
),
1709+
20_000
1710+
);
1711+
if (agentFailure) throw agentFailure;
1712+
expect(agentSettled).toBe(false);
1713+
expect(
1714+
endAndContinueGuardEvents.some(
1715+
(event) => event.chatId === addressingKey && event.kind === "guard-held"
1716+
)
1717+
).toBe(true);
1718+
1719+
const release = await appendInput({
1720+
baseUrl,
1721+
addressingKey,
1722+
token: publicAccessToken,
1723+
partId: "iterator-guard-release-input",
1724+
body: submitBody(
1725+
addressingKey,
1726+
userMessage("release pending next", "iterator-guard-release-input")
1727+
),
1728+
});
1729+
expect(release.status).toBe(200);
1730+
await expect(agent.done).resolves.toBeUndefined();
1731+
1732+
expect(
1733+
endAndContinueGuardEvents.some(
1734+
(event) => event.chatId === addressingKey && event.kind === "return-settled"
1735+
)
1736+
).toBe(true);
1737+
const session = await server.prisma.session.findFirstOrThrow({
1738+
where: { runtimeEnvironmentId: environment.id, externalId: addressingKey },
1739+
select: { currentRunId: true },
1740+
});
1741+
expect(session.currentRunId).not.toBe(initialRun.id);
1742+
} finally {
1743+
await agent.close();
1744+
}
1745+
});
16581746
});

docs/ai-chat/custom-agents.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ Without this, a resumed chat silently loses its history: the model sees only the
146146

147147
### Rotating to a new deployment
148148

149-
With `chat.createSession()`, use `chat.requestUpgrade()` to leave the current run after the turn. `chat.endAndContinue()` rejects while a `chat.createSession()` iterator is active. In a fully hand-rolled custom agent, use it to immediately hand the Session to a fresh run.
149+
With `chat.createSession()`, use `chat.requestUpgrade()` and let the iterator exit normally. For an immediate handoff, close the iterator before calling `chat.endAndContinue()`; the method rejects until the iterator and any active `next()` call have settled. In a fully hand-rolled custom agent, call it directly to hand the Session to a fresh run.
150150

151151
Call it between turns, after detaching the old run's input listeners. If the old run completed its current turn, persist its state and write the turn-complete boundary before the handoff:
152152

docs/ai-chat/patterns/version-upgrades.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ This upgrades on **every** deploy, not just breaking changes. Good for fast-movi
153153

154154
## Custom agents
155155

156-
Use `chat.requestUpgrade()` with `chat.agent()` and the `chat.createSession()` iterator. In a fully hand-rolled `chat.customAgent()` task, detach input listeners, persist the completed turn, write its boundary, then call `chat.endAndContinue()` and return immediately:
156+
Use `chat.requestUpgrade()` with `chat.agent()`. With `chat.createSession()`, call `chat.requestUpgrade()`, then advance the iterator once more so it can exit normally. For an immediate handoff, close the iterator before calling `chat.endAndContinue()`. In a fully hand-rolled `chat.customAgent()` task, detach input listeners, persist the completed turn, write its boundary, then call `chat.endAndContinue()` and return immediately:
157157

158158
```ts
159159
// Detach any chat.messages.on() subscriptions you created.

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

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8715,11 +8715,11 @@ function requestUpgrade(): void {
87158715
* Hand off the current custom agent Session to a fresh run.
87168716
*
87178717
* This is the low-level handoff for a fully hand-rolled
8718-
* `chat.customAgent()` loop. (Use {@link requestUpgrade} with
8719-
* `chat.createSession()` instead; this method rejects while its iterator is
8720-
* active.) Call only between turns and after detaching input listeners for the
8721-
* old run. If the old run completed its current turn, persist its state and
8722-
* call {@link chatWriteTurnComplete} before handing off.
8718+
* `chat.customAgent()` loop. This method rejects while a
8719+
* `chat.createSession()` iterator is active. Close the iterator before calling
8720+
* it. Call only between turns and after detaching input listeners for the old
8721+
* run. If the old run completed its current turn, persist its state and call
8722+
* {@link chatWriteTurnComplete} before handing off.
87238723
* Do not write a new turn boundary after input that the continuation run should
87248724
* process has been dispatched: the boundary acknowledges that input.
87258725
*
@@ -8747,7 +8747,7 @@ async function endAndContinue(): Promise<void> {
87478747
}
87488748
if ((locals.get(chatActiveSessionIteratorsKey) ?? 0) > 0) {
87498749
throw new Error(
8750-
"chat.endAndContinue() cannot be called while a chat.createSession() iterator is active. Use chat.requestUpgrade() instead."
8750+
"chat.endAndContinue() cannot be called while a chat.createSession() iterator is active. Close the iterator, then call chat.endAndContinue()."
87518751
);
87528752
}
87538753

@@ -9583,6 +9583,10 @@ function trackActiveChatSessionIterator(
95839583
): AsyncIterator<ChatTurn> {
95849584
locals.set(chatActiveSessionIteratorsKey, (locals.get(chatActiveSessionIteratorsKey) ?? 0) + 1);
95859585
let active = true;
9586+
let closing = false;
9587+
let activeNextCalls = 0;
9588+
let closePromise: Promise<IteratorResult<ChatTurn>> | undefined;
9589+
const nextSettledWaiters = new Set<() => void>();
95869590

95879591
function finish() {
95889592
if (!active) return;
@@ -9591,30 +9595,72 @@ function trackActiveChatSessionIterator(
95919595
locals.set(chatActiveSessionIteratorsKey, remaining);
95929596
}
95939597

9598+
function settleNextCall() {
9599+
activeNextCalls = Math.max(activeNextCalls - 1, 0);
9600+
if (activeNextCalls > 0) return;
9601+
9602+
for (const resolve of nextSettledWaiters) {
9603+
resolve();
9604+
}
9605+
nextSettledWaiters.clear();
9606+
}
9607+
9608+
function waitForNextCalls(): Promise<void> {
9609+
if (activeNextCalls === 0) return Promise.resolve();
9610+
return new Promise((resolve) => nextSettledWaiters.add(resolve));
9611+
}
9612+
9613+
function closeIterator(): Promise<IteratorResult<ChatTurn>> {
9614+
closing = true;
9615+
if (!closePromise) {
9616+
closePromise = (async () => {
9617+
// A blocked next() can install a new message listener after it resumes.
9618+
// Let every started call settle, then make the inner cleanup final.
9619+
await waitForNextCalls();
9620+
try {
9621+
return iterator.return
9622+
? await iterator.return()
9623+
: { done: true as const, value: undefined };
9624+
} finally {
9625+
finish();
9626+
}
9627+
})();
9628+
}
9629+
return closePromise;
9630+
}
9631+
95949632
return {
95959633
async next() {
9634+
if (closing) {
9635+
return { done: true as const, value: undefined };
9636+
}
9637+
9638+
activeNextCalls++;
9639+
let result: IteratorResult<ChatTurn>;
95969640
try {
9597-
const result = await iterator.next();
9598-
if (result.done) finish();
9599-
return result;
9641+
result = await iterator.next();
96009642
} catch (error) {
9643+
settleNextCall();
96019644
try {
9602-
await iterator.return?.();
9645+
await closeIterator();
96039646
} catch {
96049647
// Preserve the original iterator error after best-effort cleanup.
96059648
}
9606-
finish();
96079649
throw error;
96089650
}
9609-
},
9610-
async return() {
9611-
try {
9612-
return iterator.return
9613-
? await iterator.return()
9614-
: { done: true as const, value: undefined };
9615-
} finally {
9616-
finish();
9651+
9652+
settleNextCall();
9653+
if (result.done) {
9654+
try {
9655+
await closeIterator();
9656+
} catch {
9657+
// The inner next() already ended cleanly; cleanup remains best-effort.
9658+
}
96179659
}
9660+
return result;
9661+
},
9662+
return() {
9663+
return closeIterator();
96189664
},
96199665
};
96209666
}
Lines changed: 1 addition & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
import "../src/v3/test/index.js";
2-
3-
import { apiClientManager, resourceCatalog } from "@trigger.dev/core/v3";
4-
import { runInMockTaskContext } from "@trigger.dev/core/v3/test";
5-
import { describe, expect, it, vi } from "vitest";
1+
import { describe, expect, it } from "vitest";
62
import { chat } from "../src/v3/ai.js";
73

84
describe("chat.endAndContinue", () => {
@@ -11,57 +7,4 @@ describe("chat.endAndContinue", () => {
117
"chat.endAndContinue() can only be called from inside a chat.customAgent() run"
128
);
139
});
14-
15-
it("rejects while a createSession iterator is active and allows handoff after return", async () => {
16-
const chatId = "end-and-continue-active-session";
17-
const endAndContinueSession = vi.fn().mockResolvedValue({});
18-
const clientSpy = vi.spyOn(apiClientManager, "clientOrThrow").mockReturnValue({
19-
readSessionStreamRecords: async () => ({ records: [] }),
20-
endAndContinueSession,
21-
} as never);
22-
23-
const agent = chat.customAgent({
24-
id: "end-and-continue-active-session-agent",
25-
run: async (payload, { signal }) => {
26-
const iterator = chat.createSession(payload, { signal })[Symbol.asyncIterator]();
27-
const firstTurn = await iterator.next();
28-
expect(firstTurn.done).toBe(false);
29-
30-
await expect(chat.endAndContinue()).rejects.toThrow(
31-
"chat.endAndContinue() cannot be called while a chat.createSession() iterator is active. Use chat.requestUpgrade() instead."
32-
);
33-
expect(endAndContinueSession).not.toHaveBeenCalled();
34-
35-
await iterator.return?.();
36-
await chat.endAndContinue();
37-
},
38-
});
39-
const run = resourceCatalog.getTask(agent.id)?.fns.run;
40-
if (!run) throw new Error("custom agent was not registered");
41-
42-
try {
43-
await runInMockTaskContext((drivers) =>
44-
run(
45-
{
46-
chatId,
47-
trigger: "submit-message",
48-
message: {
49-
id: "user-1",
50-
role: "user",
51-
parts: [{ type: "text", text: "hello" }],
52-
},
53-
},
54-
{ ctx: drivers.ctx, signal: new AbortController().signal }
55-
)
56-
);
57-
58-
expect(endAndContinueSession).toHaveBeenCalledOnce();
59-
expect(endAndContinueSession).toHaveBeenCalledWith(chatId, {
60-
callingRunId: "run_test",
61-
reason: "upgrade",
62-
});
63-
} finally {
64-
clientSpy.mockRestore();
65-
}
66-
});
6710
});

0 commit comments

Comments
 (0)