Skip to content

Commit 76c12bb

Browse files
committed
fix(session): force cold replay instead of degrading multi-turn resume
Address PR #57 review findings: - language-model: on a count/tail mismatch (classifier invariant broken), never degrade to sending only the latest message — that silently drops messages 1..N-1 while the record keeps the full fingerprint. Clear the resume id pre-acquire so a fresh agent gets the full transcript instead. - agent-events: sendAgentTurnSilently now treats any terminal status other than "finished" as non-delivery and throws, except cancellation caused by our own abort signal (caller drops the record on abort). - document silent-turn trade-offs: tool invisibility, serial latency, and why concatenating queued messages was rejected (message fidelity).
1 parent 3f6afc2 commit 76c12bb

4 files changed

Lines changed: 227 additions & 19 deletions

File tree

src/provider/agent-events.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,22 @@ async function sendWithBusyRetry(
202202
* Honors `options.abortSignal`: an abort cancels the in-flight run so the
203203
* caller can stop before sending the next queued message.
204204
*
205-
* Known trade-off: token usage from silent turns is not reported (no onDelta
206-
* means the `turn-ended` usage update is never observed), so opencode slightly
207-
* undercounts usage on multi-message turns.
205+
* Known trade-offs of silent turns being FULL agent runs:
206+
* - Tool invisibility: the agent may execute tools (shell, edits, MCP) during
207+
* a silent turn with zero streamed output or tool display — the user sees
208+
* nothing until the final message streams. Accepted because interjections
209+
* are typically short course-corrections, and opencode itself folds
210+
* interjected messages into one visible turn.
211+
* - Serial latency: each silent turn is awaited to completion before the next
212+
* send, so an N-message interjection costs N sequential agent runs.
213+
* - Usage undercount: no onDelta means the `turn-ended` usage update is never
214+
* observed, so opencode slightly undercounts tokens on multi-message turns.
215+
*
216+
* Concatenating the queued messages into one Cursor message was rejected for
217+
* message fidelity: each interjection must land as a distinct user turn in the
218+
* agent's conversation memory (mirroring opencode's transcript), so the model
219+
* sees the same message boundaries the user created and later fingerprint
220+
* classification stays aligned turn-for-turn.
208221
*/
209222
export async function sendAgentTurnSilently(
210223
agent: AgentLike,
@@ -226,9 +239,16 @@ export async function sendAgentTurnSilently(
226239
// was populated, so onAbort had nothing to cancel); cancel now.
227240
if (options.abortSignal?.aborted) void Promise.resolve(run.cancel()).catch(() => {});
228241
const result = await run.wait();
229-
if (result.status === "error") {
242+
if (result.status !== "finished") {
243+
// Our own abort cancelled the run mid-flight: expected, not a failure.
244+
// The caller's abort check stops the multi-send sequence and drops the
245+
// session record, so this partial turn is never counted as delivered.
246+
if (options.abortSignal?.aborted) return;
247+
// Anything else ("error", an external "cancelled", unknown states) means
248+
// the message was NOT delivered; treating it as success would leave the
249+
// session record claiming the agent saw a message it never received.
230250
throw new Error(
231-
`Cursor run ended with status "error"${result.result ? `: ${result.result}` : ""}`,
251+
`Cursor run ended with status "${result.status}"${result.result ? `: ${result.result}` : ""}`,
232252
);
233253
}
234254
} finally {

src/provider/language-model.ts

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
McpServerConfig,
1313
SettingSource,
1414
AgentModeOption,
15+
SDKUserMessage,
1516
} from "@cursor/sdk";
1617
import { resolveCursorApiKey } from "../api-key.js";
1718
import {
@@ -207,6 +208,28 @@ export class CursorLanguageModel implements LanguageModelV3 {
207208
}
208209
}
209210

211+
// A multi-message interjection: two-or-more user messages were queued while
212+
// the agent was busy, forming a contiguous user-turn tail (the classifier
213+
// guarantees this shape for "continuation-multi"). On a resumed agent we
214+
// replay just those new messages as sequential turns.
215+
//
216+
// Defensive invariant check: if the recovered tail doesn't match the
217+
// classifier's count (unreachable today, but one classifier refactor away
218+
// from real), we must NOT degrade to sending only the latest message —
219+
// the session record keeps the full N-message fingerprint, so messages
220+
// 1..N-1 would be silently lost. Instead force the cold path: clear the
221+
// resume id so a FRESH agent gets the FULL transcript, which matches the
222+
// record being written and loses nothing.
223+
let multiTurns: SDKUserMessage[] | undefined;
224+
if (multiNewUserCount >= 2) {
225+
const turns = trailingUserMessages(options.prompt, multiNewUserCount);
226+
if (turns.length === multiNewUserCount) {
227+
multiTurns = turns;
228+
} else {
229+
resumeAgentId = undefined;
230+
}
231+
}
232+
210233
const acquired = await acquireAgent({
211234
apiKey: this.requireApiKey(),
212235
modelSelection,
@@ -226,21 +249,13 @@ export class CursorLanguageModel implements LanguageModelV3 {
226249
...(record ? { record } : {}),
227250
});
228251

229-
// A multi-message interjection: two-or-more user messages were queued while
230-
// the agent was busy, forming a contiguous user-turn tail (the classifier
231-
// guarantees this shape for "continuation-multi"). On a resumed agent,
232-
// replay just those new messages as sequential turns — send the leading
233-
// ones silently and stream only the final one. If the tail can't be
234-
// recovered (defensive; shouldn't happen post-classifier), skip the multi
235-
// path: on a resumed agent that means sending only the latest message, so
236-
// the empty/short check below treats it as a normal single continuation.
237-
const multiTurns =
238-
acquired.resumed && multiNewUserCount >= 2
239-
? trailingUserMessages(options.prompt, multiNewUserCount)
240-
: undefined;
241-
242252
try {
243-
if (multiTurns && multiTurns.length === multiNewUserCount) {
253+
// Replay the queued messages as sequential turns: leading ones silent,
254+
// only the final one streamed. Note silent turns are FULL agent runs —
255+
// tools may execute with nothing surfaced until the last turn streams,
256+
// and each run is awaited serially (see sendAgentTurnSilently for the
257+
// trade-offs and why concatenation was rejected).
258+
if (acquired.resumed && multiTurns) {
244259
// The pool record was written optimistically with the FULL new
245260
// fingerprint before any send. If delivery stops partway (error or
246261
// abort), drop the record so the next turn classifies fresh instead

test/agent-events.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,39 @@ describe("sendAgentTurnSilently", () => {
140140
).rejects.toThrow(/error/i);
141141
});
142142

143+
it("throws when the run ends 'cancelled' without our abort (message never delivered)", async () => {
144+
// Cancellation we did NOT request (external cancel, CLI kill, …) means the
145+
// silent turn was not delivered; treating it as success would let the
146+
// caller keep a session record for a message the agent never received.
147+
const agent = fakeAgent({ result: { status: "cancelled" } });
148+
await expect(
149+
sendAgentTurnSilently(agent, MESSAGE, { mode: "agent" }),
150+
).rejects.toThrow(/cancelled/);
151+
});
152+
153+
it("throws when the run ends with an unknown terminal status", async () => {
154+
const agent = fakeAgent({ result: { status: "expired" } });
155+
await expect(
156+
sendAgentTurnSilently(agent, MESSAGE, { mode: "agent" }),
157+
).rejects.toThrow(/expired/);
158+
});
159+
160+
it("does not throw on 'cancelled' when our own abort signal caused it", async () => {
161+
const controller = new AbortController();
162+
controller.abort();
163+
// Already-aborted signal: returns early without sending at all.
164+
const sendCalls: Array<Record<string, unknown> | undefined> = [];
165+
const agent = fakeAgent({
166+
result: { status: "cancelled" },
167+
sendCalls,
168+
});
169+
await sendAgentTurnSilently(agent, MESSAGE, {
170+
mode: "agent",
171+
abortSignal: controller.signal,
172+
});
173+
expect(sendCalls).toHaveLength(0);
174+
});
175+
143176
it("retries with local.force on AgentBusyError", async () => {
144177
const busy = new Error("agent busy");
145178
busy.name = "AgentBusyError";
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import { mkdtempSync } from "node:fs";
2+
import { tmpdir } from "node:os";
3+
import { join } from "node:path";
4+
import { afterEach, describe, expect, it, vi } from "vitest";
5+
import type {
6+
LanguageModelV3CallOptions,
7+
LanguageModelV3Prompt,
8+
} from "@ai-sdk/provider";
9+
10+
// Sandbox the on-disk session store away from the user's real cache dir.
11+
process.env.XDG_CACHE_HOME = mkdtempSync(join(tmpdir(), "cursor-lm-fb-test-"));
12+
13+
interface SentTurn {
14+
text: string;
15+
streamed: boolean;
16+
}
17+
18+
const sentTurns: SentTurn[] = [];
19+
const create = vi.fn();
20+
const resume = vi.fn();
21+
22+
vi.mock("../src/cursor-runtime.js", () => ({
23+
loadCursorSdk: async () => ({ Agent: { create, resume } }),
24+
}));
25+
26+
// The classifier guarantees `continuation-multi` prompts end in a contiguous
27+
// user tail of exactly `newUserCount` messages, so the count/tail mismatch the
28+
// model guards against is unreachable through the public API. Construct it
29+
// artificially: make trailingUserMessages return one message fewer than asked,
30+
// as a stand-in for a future refactor breaking the classifier invariant.
31+
vi.mock("../src/provider/message-map.js", async (importOriginal) => {
32+
const actual =
33+
await importOriginal<typeof import("../src/provider/message-map.js")>();
34+
return {
35+
...actual,
36+
trailingUserMessages: (prompt: LanguageModelV3Prompt, count: number) =>
37+
actual.trailingUserMessages(prompt, count).slice(1),
38+
};
39+
});
40+
41+
const { CursorLanguageModel } = await import(
42+
"../src/provider/language-model.js"
43+
);
44+
const { clearAgentPool, getSessionRecord } = await import(
45+
"../src/provider/session-pool.js"
46+
);
47+
48+
const SESSION_ID = "sess-fallback";
49+
50+
function makeFakeAgent(agentId: string) {
51+
return {
52+
agentId,
53+
close: vi.fn(),
54+
send: async (
55+
message: { text: string },
56+
sendOptions?: Record<string, unknown>,
57+
) => {
58+
sentTurns.push({
59+
text: message.text,
60+
streamed: Boolean(sendOptions?.["onDelta"]),
61+
});
62+
return {
63+
id: "run",
64+
wait: async () => ({ status: "finished", result: message.text }),
65+
cancel: async () => {},
66+
};
67+
},
68+
};
69+
}
70+
71+
function model() {
72+
return new CursorLanguageModel("cursor/auto", {
73+
providerName: "cursor",
74+
apiKey: "k",
75+
cwd: "/tmp",
76+
mode: "agent",
77+
});
78+
}
79+
80+
async function drain(prompt: LanguageModelV3Prompt): Promise<void> {
81+
const options = {
82+
prompt,
83+
providerOptions: { cursor: { sessionID: SESSION_ID } },
84+
} as unknown as LanguageModelV3CallOptions;
85+
const { stream } = await model().doStream(options);
86+
const reader = stream.getReader();
87+
// eslint-disable-next-line no-constant-condition
88+
while (true) {
89+
const { done } = await reader.read();
90+
if (done) break;
91+
}
92+
}
93+
94+
const sys = { role: "system" as const, content: "S" };
95+
const user = (text: string): LanguageModelV3Prompt[number] => ({
96+
role: "user",
97+
content: [{ type: "text", text }],
98+
});
99+
const assistant = (text: string): LanguageModelV3Prompt[number] => ({
100+
role: "assistant",
101+
content: [{ type: "text", text }],
102+
});
103+
104+
afterEach(() => {
105+
create.mockReset();
106+
resume.mockReset();
107+
sentTurns.length = 0;
108+
clearAgentPool();
109+
});
110+
111+
describe("multi-turn tail mismatch (defensive fallback)", () => {
112+
it("forces a cold full-transcript replay instead of degrading to the latest message", async () => {
113+
// Turn 1: pool the agent with a single-user fingerprint.
114+
create.mockResolvedValue(makeFakeAgent("a1"));
115+
await drain([sys, user("a")]);
116+
expect(getSessionRecord(SESSION_ID)).toBeDefined();
117+
118+
// Turn 2: continuation-multi (2 new user msgs), but the mocked
119+
// trailingUserMessages recovers only 1 of them — the mismatch case.
120+
// The model must NOT resume-and-send-only-"c" (which silently drops "b");
121+
// it must fall back to a fresh agent + full transcript so nothing is lost.
122+
create.mockClear();
123+
create.mockResolvedValue(makeFakeAgent("a2"));
124+
resume.mockResolvedValue(makeFakeAgent("a1"));
125+
sentTurns.length = 0;
126+
await drain([sys, user("a"), assistant("x"), user("b"), user("c")]);
127+
128+
expect(resume).not.toHaveBeenCalled();
129+
expect(create).toHaveBeenCalledOnce();
130+
expect(sentTurns).toHaveLength(1);
131+
expect(sentTurns[0]?.streamed).toBe(true);
132+
// Full transcript: every queued message is present, none dropped.
133+
expect(sentTurns[0]?.text).toContain("b");
134+
expect(sentTurns[0]?.text).toContain("c");
135+
136+
// The record now reflects the fully delivered transcript: the NEXT turn
137+
// is a clean continuation on the fresh agent.
138+
expect(getSessionRecord(SESSION_ID)).toMatchObject({ agentId: "a2" });
139+
});
140+
});

0 commit comments

Comments
 (0)