diff --git a/packages/coding-agent/test/agent-session-compaction-thinking-threading.test.ts b/packages/coding-agent/test/agent-session-compaction-thinking-threading.test.ts deleted file mode 100644 index 8d1b9e908..000000000 --- a/packages/coding-agent/test/agent-session-compaction-thinking-threading.test.ts +++ /dev/null @@ -1,165 +0,0 @@ -import { describe, expect, test } from "bun:test"; - -// Audit gate for the compaction-effort fix. The plan calls out three -// production call sites in `agent-session.ts` that MUST thread -// `thinkingLevel: this.thinkingLevel` into the compaction LLM options: -// -// - `compact(...)` at the manual `/compact` site (`#compactWithFallbackModel`) -// - `compact(...)` at the auto-compaction site (the most-fired path) -// - `generateHandoff(...)` at the handoff site -// -// `SummaryOptions.thinkingLevel` is optional, so a future contributor adding -// a new `compact(...)` or `generateHandoff(...)` call without threading it -// would silently fall back to the historical `Effort.High` default — exactly -// the regression Codex caught during plan review (auto-compaction was -// initially missed). The typecheck won't catch this; this test does. - -const AGENT_SESSION_PATH = `${import.meta.dir}/../src/session/agent-session.ts`; - -// Lines that are NOT direct LLM call sites and don't need threading: -// - The `async compact(...)` method declaration itself. -// - `this.compact(...)` invocations that route through the method (and from -// there into the threaded `#compactWithFallbackModel`). -const NON_LLM_CALL_PATTERNS = [ - /async compact\(customInstructions/, // method declaration - /await this\.compact\(/, // self-invocation routes to threaded site -]; - -interface CallSite { - line: number; - headerLine: string; - callExpression: string; -} - -/** - * Scan source for `compact(` / `generateHandoff(` and extract the - * brace-balanced call expression for each match. Skips comments, - * string contents, and non-LLM lines (method declarations, self-calls). - */ -function findCompactionCallSites(src: string): CallSite[] { - const lines = src.split("\n"); - const sites: CallSite[] = []; - - for (let lineIdx = 0; lineIdx < lines.length; lineIdx++) { - const line = lines[lineIdx]; - if (!line) continue; - const headerMatch = /\b(compact|generateHandoff)\(/.exec(line); - if (!headerMatch) continue; - if (NON_LLM_CALL_PATTERNS.some(rx => rx.test(line))) continue; - // Skip lines that are themselves comments - const trimmed = line.trim(); - if (trimmed.startsWith("//") || trimmed.startsWith("*") || trimmed.startsWith("/*")) continue; - - // Compute absolute offset of the opening `(` after the matched name - let offset = 0; - for (let l = 0; l < lineIdx; l++) { - offset += (lines[l]?.length ?? 0) + 1; // +1 for newline - } - const openParenIdx = offset + headerMatch.index + headerMatch[0].length - 1; - - // Walk forward, balancing parens. Track string / comment context to - // avoid counting `(`/`)` inside literals. - let depth = 0; - let i = openParenIdx; - let inString: '"' | "'" | "`" | null = null; - let inLineComment = false; - let inBlockComment = false; - let end = -1; - for (; i < src.length; i++) { - const ch = src[i]; - const next = src[i + 1]; - - if (inLineComment) { - if (ch === "\n") inLineComment = false; - continue; - } - if (inBlockComment) { - if (ch === "*" && next === "/") { - inBlockComment = false; - i++; - } - continue; - } - if (inString) { - if (ch === "\\") { - i++; - continue; - } - if (ch === inString) inString = null; - continue; - } - if (ch === "/" && next === "/") { - inLineComment = true; - continue; - } - if (ch === "/" && next === "*") { - inBlockComment = true; - i++; - continue; - } - if (ch === '"' || ch === "'" || ch === "`") { - inString = ch; - continue; - } - if (ch === "(") depth++; - else if (ch === ")") { - depth--; - if (depth === 0) { - end = i; - break; - } - } - } - - if (end === -1) { - throw new Error(`Unterminated call expression at agent-session.ts:${lineIdx + 1}`); - } - - sites.push({ - line: lineIdx + 1, - headerLine: line.trim(), - callExpression: src.slice(openParenIdx + 1, end), - }); - } - - return sites; -} - -function expressionThreadsThinkingLevel(callExpression: string): boolean { - // `thinkingLevel:` must appear as a property key — i.e. preceded by `{` - // or `,` or whitespace + `,` or start of expression, and followed by a - // value. Loose check via the substring is sufficient because the call - // expression is brace-balanced (we extracted only one call's contents). - // We also require the value to be the session field, to defend against - // `thinkingLevel: undefined` accidentally satisfying the gate. - return /\bthinkingLevel\s*:\s*this\.thinkingLevel\b/.test(callExpression); -} - -describe("agent-session.ts compaction threading (audit gate)", () => { - test("every direct compact()/generateHandoff() call threads thinkingLevel: this.thinkingLevel", async () => { - const src = await Bun.file(AGENT_SESSION_PATH).text(); - const sites = findCompactionCallSites(src); - const offenders = sites.filter(s => !expressionThreadsThinkingLevel(s.callExpression)); - - expect( - offenders, - `Found ${offenders.length} compaction call site(s) missing 'thinkingLevel: this.thinkingLevel':\n${offenders - .map(o => ` agent-session.ts:${o.line} — ${o.headerLine}`) - .join( - "\n", - )}\n\nFix: add 'thinkingLevel: this.thinkingLevel' to the options object on every direct compact()/generateHandoff() call. The historical Effort.High default lives in resolveCompactionEffort (packages/agent/src/compaction/compaction.ts) and applies when thinkingLevel is undefined.`, - ).toEqual([]); - }); - - test("at least 3 threaded sites exist (manual /compact + auto-compaction + handoff)", async () => { - const src = await Bun.file(AGENT_SESSION_PATH).text(); - const sites = findCompactionCallSites(src); - const threaded = sites.filter(s => expressionThreadsThinkingLevel(s.callExpression)); - // Floor at 3 — the plan explicitly enumerates three sites. If the - // production surface grows new entry points, the first test fails - // until they thread too; this guards against accidental removal of - // any of the original three. Count is derived from the same - // brace-balanced scanner as the offender check above. - expect(threaded.length).toBeGreaterThanOrEqual(3); - }); -}); diff --git a/packages/coding-agent/test/agent-session-steer-idle-drain.test.ts b/packages/coding-agent/test/agent-session-steer-idle-drain.test.ts index 577fe5d6b..a9bc766a2 100644 --- a/packages/coding-agent/test/agent-session-steer-idle-drain.test.ts +++ b/packages/coding-agent/test/agent-session-steer-idle-drain.test.ts @@ -108,4 +108,16 @@ describe("AgentSession steer idle drain", () => { expect(session.agent.hasQueuedMessages()).toBe(true); expect(session.getQueuedMessages().steering).toContain("wait for the run"); }); + + it("round-trips queued images through clearQueue for editor restoration", async () => { + // Non-resumable state so the idle drain stays out of the way. + await createSession([{ role: "user", content: "hello", timestamp: Date.now() }]); + const image = { type: "image" as const, data: "abc", mimeType: "image/png" }; + + await session.steer("with image", [image]); + + const { steering } = session.clearQueue(); + expect(steering).toEqual([{ text: "with image", images: [image] }]); + expect(session.agent.hasQueuedMessages()).toBe(false); + }); }); diff --git a/packages/coding-agent/test/input-controller-orphan-submit.test.ts b/packages/coding-agent/test/input-controller-orphan-submit.test.ts index 4e9399c13..8d69874a8 100644 --- a/packages/coding-agent/test/input-controller-orphan-submit.test.ts +++ b/packages/coding-agent/test/input-controller-orphan-submit.test.ts @@ -138,8 +138,10 @@ describe("InputController orphaned submit", () => { expect(ctx.pendingImages.length).toBe(0); }); - it("surfaces a steer rejection instead of failing silently", async () => { + it("restores text and images to the editor when the steer rejects", async () => { const { ctx, editor, spies } = createContext(); + const image = { type: "image" as const, data: "abc", mimeType: "image/png" }; + (ctx.pendingImages as unknown[]).push(image); spies.steer.mockImplementationOnce(async () => { throw new Error("queue exploded"); }); @@ -149,7 +151,28 @@ describe("InputController orphaned submit", () => { await editor.onSubmit?.("doomed message"); expect(spies.showError).toHaveBeenCalledWith("queue exploded"); + // The message survives the failure: text and images return to the editor. + expect(editor.getText()).toBe("doomed message"); + expect(ctx.pendingImages).toEqual([image]); // The signature must not leak for a message that never queued. - expect(ctx.locallySubmittedUserSignatures.has("doomed message\u00000")).toBe(false); + expect(ctx.locallySubmittedUserSignatures.has("doomed message\u00001")).toBe(false); + }); + + it("returns queued images to the pending-image buffer on queue restore", async () => { + const { ctx, editor } = createContext(); + const image = { type: "image" as const, data: "abc", mimeType: "image/png" }; + const session = ctx.session as unknown as { clearQueue: () => unknown }; + session.clearQueue = () => ({ + steering: [{ text: "queued with image", images: [image] }], + followUp: [], + }); + const controller = new InputController(ctx); + + const restored = controller.restoreQueuedMessagesToEditor(); + + expect(restored).toBe(1); + expect(editor.getText()).toBe("queued with image"); + expect(ctx.pendingImages).toEqual([image]); + expect(ctx.pendingImageLinks).toEqual([undefined]); }); }); diff --git a/packages/coding-agent/test/input-controller-skill-queue.test.ts b/packages/coding-agent/test/input-controller-skill-queue.test.ts index 14c070c12..cce5e04c1 100644 --- a/packages/coding-agent/test/input-controller-skill-queue.test.ts +++ b/packages/coding-agent/test/input-controller-skill-queue.test.ts @@ -337,7 +337,7 @@ describe("AgentSession custom-role tag dequeue (E4-E7)", () => { const { session } = fixture; const firstTag = session.enqueueCustomMessageDisplay("/skill:foo bar", "steer"); const popped = session.popLastQueuedMessage(); - expect(popped).toBe("/skill:foo bar"); + expect(popped?.text).toBe("/skill:foo bar"); expect(session.getQueuedMessages().steering).toEqual([]); // Push a NEW tagged entry with the same text. Emitting `message_start` for the diff --git a/packages/coding-agent/test/issue-825-repro.test.ts b/packages/coding-agent/test/issue-825-repro.test.ts index 83775acf2..7ae016928 100644 --- a/packages/coding-agent/test/issue-825-repro.test.ts +++ b/packages/coding-agent/test/issue-825-repro.test.ts @@ -31,8 +31,8 @@ beforeAll(() => { type PromptOpts = { streamingBehavior?: "steer" | "followUp" } | undefined; function makeFakeSession() { - const steering: string[] = []; - const followUp: string[] = []; + const steering: { text: string }[] = []; + const followUp: { text: string }[] = []; const promptCalls: Array<{ text: string; opts: PromptOpts }> = []; const prompt = mock(async (text: string, opts?: PromptOpts): Promise => { @@ -43,18 +43,18 @@ function makeFakeSession() { throw new AgentBusyError(); } if (opts.streamingBehavior === "followUp") { - followUp.push(text); + followUp.push({ text }); } else { - steering.push(text); + steering.push({ text }); } }); const steer = mock(async (text: string): Promise => { - steering.push(text); + steering.push({ text }); }); const followUpFn = mock(async (text: string): Promise => { - followUp.push(text); + followUp.push({ text }); }); const session = { @@ -62,7 +62,7 @@ function makeFakeSession() { isCompacting: false, extensionRunner: undefined, customCommands: [] as Array<{ command: { name: string } }>, - getQueuedMessages: () => ({ steering, followUp }), + getQueuedMessages: () => ({ steering: steering.map(e => e.text), followUp: followUp.map(e => e.text) }), clearQueue: () => { const s = [...steering]; const f = [...followUp]; @@ -139,7 +139,7 @@ describe("issue #825: steer preview stuck after compaction", () => { // that is what `restoreQueuedMessagesToEditor` (Alt+Up) and the // post-stream drain consult. Otherwise it is stranded in // compactionQueuedMessages with no consumer. - expect(fake.steering).toContain("address review feedback"); + expect(fake.steering).toContainEqual({ text: "address review feedback" }); // And it must not also remain duplicated in compactionQueuedMessages. const remaining = (ctx as unknown as { compactionQueuedMessages: CompactionQueuedMessage[] })