-
Notifications
You must be signed in to change notification settings - Fork 353
test(session): prove canonical Cursor compaction #4521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3a9ccbb
fb4abb5
83ceb70
65128cc
e28422b
e1dd34d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -305,7 +305,7 @@ describe("AgentSession mid-run compaction (issue #2035)", () => { | |
| } | ||
| await options.afterStreamStart?.(streamOptions); | ||
| stream.push({ type: "done", reason: message.stopReason as never, message }); | ||
| })(); | ||
| })().catch(error => stream.fail(error)); | ||
| }); | ||
| return stream; | ||
| }, | ||
|
|
@@ -580,6 +580,7 @@ describe("AgentSession mid-run compaction (issue #2035)", () => { | |
|
|
||
| const loop = await buildLoopSession({ | ||
| extensionSource: shortCircuitExtensionSource(), | ||
| settings: { "compaction.keepRecentTokens": 100 }, | ||
| publishTextStartBeforeAfterStreamStart: true, | ||
| responder: call => { | ||
| if (call === 1) { | ||
|
|
@@ -629,7 +630,7 @@ describe("AgentSession mid-run compaction (issue #2035)", () => { | |
| await seedLoop(loop.session, [ | ||
| { role: "user", content: "earlier request", timestamp: Date.now() }, | ||
| assistantFor(model, { | ||
| content: [{ type: "text", text: "earlier response" }], | ||
| content: [{ type: "text", text: `earlier response ${"x".repeat(4_000)}` }], | ||
| totalTokens: 1_000, | ||
| stopReason: "stop", | ||
| }), | ||
|
|
@@ -638,25 +639,61 @@ describe("AgentSession mid-run compaction (issue #2035)", () => { | |
| await loop.session.waitForIdle(); | ||
|
|
||
| expect(originalAnchor).toBeDefined(); | ||
| expect(loop.session.messages.includes(originalAnchor!)).toBe(false); | ||
| const cursorMessages = loop.agentEvents.flatMap(event => { | ||
| const maintenanceEvents = loop.agentEvents.filter( | ||
| (event): event is Extract<AgentEvent, { type: "agent_end" }> => | ||
| event.type === "agent_end" && event.stopReason === "maintenance", | ||
| ); | ||
| expect(maintenanceEvents).toHaveLength(1); | ||
| expect(maintenanceEvents[0]?.maintenanceOutcome).toBe("compacted"); | ||
| expect(getLatestCompactionEntry(loop.session.sessionManager.getBranch())).not.toBeNull(); | ||
|
|
||
| const canonicalMessages = loop.session.buildDisplaySessionContext().messages; | ||
| const canonicalPreamble = canonicalMessages.find( | ||
| message => | ||
| message.role === "assistant" && | ||
| JSON.stringify((message as { content?: unknown }).content ?? "").includes("Cursor preamble"), | ||
| ); | ||
| const canonicalContinuation = canonicalMessages.find( | ||
| message => | ||
| message.role === "assistant" && | ||
| JSON.stringify((message as { content?: unknown }).content ?? "").includes("and continuation"), | ||
| ); | ||
| const canonicalServerResult = canonicalMessages.find( | ||
| message => message.role === "toolResult" && message.toolCallId === "cursor-server-result", | ||
| ); | ||
| expect(canonicalPreamble).toBeDefined(); | ||
| expect(canonicalContinuation).toBeDefined(); | ||
| expect(canonicalServerResult).toBeDefined(); | ||
|
Comment on lines
+664
to
+666
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When compaction persists the split messages in the wrong order, these presence-only checks still pass; the chronology assertion below examines transient AGENTS.md reference: AGENTS.md:L156-L160 Useful? React with 👍 / 👎. |
||
| const canonicalCursorOrder = canonicalMessages.flatMap(message => { | ||
| if (message === canonicalPreamble) return ["preamble"]; | ||
| if (message === canonicalServerResult) return ["server-result"]; | ||
| if (message === canonicalContinuation) return ["continuation"]; | ||
| return []; | ||
| }); | ||
| expect(canonicalCursorOrder).toEqual(["preamble", "server-result", "continuation"]); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This assertion deterministically fails against the production code in this commit: the server-result Useful? React with 👍 / 👎. |
||
| const cursorEvents = loop.agentEvents.flatMap(event => { | ||
| if (event.type !== "message_end") return []; | ||
| const content = JSON.stringify((event.message as { content?: unknown }).content ?? ""); | ||
| return content.includes("Cursor") || content.includes("and continuation") ? [event.message] : []; | ||
| }); | ||
| expect(cursorMessages.map(message => message.role)).toEqual(["assistant", "toolResult", "assistant"]); | ||
| expect(JSON.stringify((cursorMessages[0] as { content?: unknown } | undefined)?.content)).toContain( | ||
| expect(cursorEvents.map(message => message.role)).toEqual(["assistant", "toolResult", "assistant"]); | ||
| expect(JSON.stringify((cursorEvents[0] as { content?: unknown } | undefined)?.content)).toContain( | ||
| "Cursor preamble", | ||
| ); | ||
| expect(JSON.stringify((cursorMessages[1] as { content?: unknown } | undefined)?.content)).toContain( | ||
| expect(JSON.stringify((cursorEvents[1] as { content?: unknown } | undefined)?.content)).toContain( | ||
| "Cursor server-side result", | ||
| ); | ||
| expect(JSON.stringify((cursorMessages[2] as { content?: unknown } | undefined)?.content)).toContain( | ||
| expect(JSON.stringify((cursorEvents[2] as { content?: unknown } | undefined)?.content)).toContain( | ||
| "and continuation", | ||
| ); | ||
| expect( | ||
| loop.agentEvents.filter(event => event.type === "agent_end" && event.stopReason === "maintenance"), | ||
| ).toHaveLength(1); | ||
| canonicalMessages.some(message => { | ||
| if (message.role !== "assistant") return false; | ||
| const content = JSON.stringify((message as { content?: unknown }).content ?? ""); | ||
| return content.includes("Cursor preamble and continuation") && content.includes("cursor-call"); | ||
| }), | ||
| ).toBe(false); | ||
|
Comment on lines
+692
to
+695
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In this revision, the positive Cursor-message assertions have been removed altogether: this condition only proves that the unsplit anchor is absent. If compaction drops the server-side tool result, the continuation, or every Cursor message, the maintenance event and compaction entry still exist and this test passes, so it no longer protects the externally observable preservation contract it is intended to cover. Assert that the canonical context contains the split preamble, tool result, and continuation in provider order. AGENTS.md reference: AGENTS.md:L156-L160 Useful? React with 👍 / 👎. |
||
| expect(canonicalMessages).not.toContainEqual(originalAnchor!); | ||
| expect(loop.events.filter(event => event.type === "agent_end")).toHaveLength(1); | ||
| expect(loop.streamCallCount()).toBe(2); | ||
| } finally { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
#flushPendingBackgroundExchangessynchronously emitsmessage_start/message_endpairs for multiple custom messages (agent-session.ts:19274-19281), this unconditionalawaityields even when the predecessor is already resolved. The next message's start is therefore delivered before the prior message's end, producingstart1, start2, end1, end2for session, SDK, and extension subscribers instead of properly paired lifecycles. Avoid yielding for an uncontended admission, or include the associated lifecycle events in the ordered lane.Useful? React with 👍 / 👎.