From 51865fc1ea055e90e768a8a9d324819f1775e47e Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 4 Jun 2026 16:35:45 +0200 Subject: [PATCH] test(coding-agent): updated assertions for condensed prompt wording - Aligned handoff, reminder, and system-prompt expectations with shortened copy. - Added HTTP transport test for required initialize failures. - Guarded SSE startup timeout against stale connection races. --- packages/agent/test/handoff.test.ts | 4 +- .../ai/test/duplicate-tool-results.test.ts | 2 +- .../coding-agent/src/mcp/transports/http.ts | 49 +++++++++++++------ .../agent-dashboard-create-editor.test.ts | 48 +++++++++++++++++- .../agent-session-empty-stop-guard.test.ts | 21 ++++---- .../core/python-executor-per-call.test.ts | 21 +++++--- .../event-controller-abort-render.test.ts | 3 +- .../custom-commands/review.test.ts | 2 +- .../test/interactive-mode-plan-review.test.ts | 2 +- .../test/mcp-http-transport.test.ts | 20 ++++++++ .../test/streaming-preview-height.test.ts | 6 ++- .../test/system-prompt-templates.test.ts | 16 +++--- .../task/executor-subagent-reminders.test.ts | 3 +- 13 files changed, 147 insertions(+), 50 deletions(-) diff --git a/packages/agent/test/handoff.test.ts b/packages/agent/test/handoff.test.ts index 911536093..58510a2cb 100644 --- a/packages/agent/test/handoff.test.ts +++ b/packages/agent/test/handoff.test.ts @@ -41,7 +41,7 @@ afterEach(() => { describe("handoff helpers", () => { test("renders custom focus into the handoff prompt", () => { const rendered = renderHandoffPrompt("preserve failing test name"); - expect(rendered).toContain("Write a handoff document"); + expect(rendered).toContain("Write handoff doc for another instance."); expect(rendered).toContain("Additional focus: preserve failing test name"); }); @@ -108,7 +108,7 @@ describe("handoff helpers", () => { if (promptBlock?.type !== "text") { throw new Error("Expected text handoff prompt block"); } - expect(promptBlock.text).toContain("Write a handoff document"); + expect(promptBlock.text).toContain("Write handoff doc for another instance."); expect(promptBlock.text).toContain("Additional focus: preserve failing test name"); }); }); diff --git a/packages/ai/test/duplicate-tool-results.test.ts b/packages/ai/test/duplicate-tool-results.test.ts index 45d68ecbc..988817e66 100644 --- a/packages/ai/test/duplicate-tool-results.test.ts +++ b/packages/ai/test/duplicate-tool-results.test.ts @@ -918,7 +918,7 @@ describe("Codex-style Abort Handling", () => { const guidanceMsg = transformed[3] as DeveloperMessage; expect(guidanceMsg.role).toBe("developer"); expect(guidanceMsg.content).toContain(""); - expect(guidanceMsg.content).toContain("verify current state before retrying"); + expect(guidanceMsg.content).toContain("verify state before retry"); }); it("should inject synthetic 'aborted' tool results with isError true", () => { diff --git a/packages/coding-agent/src/mcp/transports/http.ts b/packages/coding-agent/src/mcp/transports/http.ts index 203c51365..6473ba9ec 100644 --- a/packages/coding-agent/src/mcp/transports/http.ts +++ b/packages/coding-agent/src/mcp/transports/http.ts @@ -87,45 +87,62 @@ export class HttpTransport implements MCPTransport { headers["Mcp-Session-Id"] = this.#sessionId; } - let response: Response; + let response: Response | null; let timedOut = false; + let startupFinished = false; + const connection = this.#sseConnection; const startupTimeoutMs = resolveSSEConnectTimeoutMs(this.config.timeout); - const timeoutId = + const fetchPromise = fetch(this.config.url, { + method: "GET", + headers, + signal: connection.signal, + }); + const timeoutPromise = startupTimeoutMs > 0 - ? setTimeout(() => { - timedOut = true; - this.#sseConnection?.abort(); - }, startupTimeoutMs) + ? new Promise((resolve) => { + setTimeout(() => { + if (!startupFinished) { + timedOut = true; + connection.abort(); + } + resolve(null); + }, startupTimeoutMs); + }) : null; try { - response = await fetch(this.config.url, { - method: "GET", - headers, - signal: this.#sseConnection.signal, - }); + response = timeoutPromise === null ? await fetchPromise : await Promise.race([fetchPromise, timeoutPromise]); } catch (error) { - this.#sseConnection = null; + if (this.#sseConnection === connection) this.#sseConnection = null; if (error instanceof Error && error.name !== "AbortError" && !timedOut) { this.onError?.(error); } return; } finally { - if (timeoutId !== null) clearTimeout(timeoutId); + startupFinished = true; + } + if (response === null) { + if (this.#sseConnection === connection) this.#sseConnection = null; + void fetchPromise.then((lateResponse) => lateResponse.body?.cancel()).catch(() => {}); + return; } + if (this.#sseConnection !== connection) { + await response.body?.cancel(); + return; + } if (response.status === 405 || !response.ok || !response.body) { await response.body?.cancel(); - this.#sseConnection = null; + if (this.#sseConnection === connection) this.#sseConnection = null; return; } // Connection established — read messages in background. // If the stream ends unexpectedly (server restart, network drop), // fire onClose so the manager can trigger reconnection. - const signal = this.#sseConnection.signal; + const signal = connection.signal; void this.#readSSEStream(response.body!, signal).finally(() => { const wasConnected = this.#connected; - this.#sseConnection = null; + if (this.#sseConnection === connection) this.#sseConnection = null; if (wasConnected) this.onClose?.(); }); } diff --git a/packages/coding-agent/test/agent-dashboard-create-editor.test.ts b/packages/coding-agent/test/agent-dashboard-create-editor.test.ts index 8b3b61570..62105509f 100644 --- a/packages/coding-agent/test/agent-dashboard-create-editor.test.ts +++ b/packages/coding-agent/test/agent-dashboard-create-editor.test.ts @@ -1,8 +1,9 @@ -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, test, vi } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; import type { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import * as discovery from "@oh-my-pi/pi-coding-agent/task/discovery"; import { AgentDashboard } from "@oh-my-pi/pi-coding-agent/modes/components/agent-dashboard"; import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; @@ -53,6 +54,7 @@ function stubStdoutGeometry(cols: number): { setRows(n: number): void; restore() } afterEach(async () => { + vi.restoreAllMocks(); await Promise.all(tempDirs.splice(0).map(dir => fs.rm(dir, { recursive: true, force: true }))); }); @@ -129,3 +131,47 @@ describe("AgentDashboard layout", () => { } }); }); + +describe("AgentDashboard tab navigation", () => { + test("left/right arrows switch source tabs", async () => { + await initTheme(false); + vi.spyOn(discovery, "discoverAgents").mockResolvedValue({ + projectAgentsDir: null, + agents: [ + { name: "proj-agent", description: "p", systemPrompt: "", source: "project" }, + { name: "bundled-agent", description: "b", systemPrompt: "", source: "bundled" }, + ], + }); + const geo = stubStdoutGeometry(120); + try { + geo.setRows(30); + const dashboard = await AgentDashboard.create(await makeTempCwd(), settingsStub, 30, {}); + const strip = () => dashboard.render(120).join("\n").replace(ANSI_PATTERN, ""); + + // "All" tab shows every source. + const all = strip(); + expect(all).toContain("proj-agent"); + expect(all).toContain("bundled-agent"); + + // Right arrow advances to the "Project" tab, filtering out bundled agents. + dashboard.handleInput("\x1b[C"); + const project = strip(); + expect(project).toContain("proj-agent"); + expect(project).not.toContain("bundled-agent"); + + // Right again lands on "Bundled". + dashboard.handleInput("\x1b[C"); + const bundled = strip(); + expect(bundled).toContain("bundled-agent"); + expect(bundled).not.toContain("proj-agent"); + + // Left arrow walks back to "Project". + dashboard.handleInput("\x1b[D"); + const back = strip(); + expect(back).toContain("proj-agent"); + expect(back).not.toContain("bundled-agent"); + } finally { + geo.restore(); + } + }); +}); diff --git a/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts b/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts index db4c45cba..8399e6e90 100644 --- a/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts +++ b/packages/coding-agent/test/agent-session-empty-stop-guard.test.ts @@ -118,16 +118,17 @@ function emptyAssistantStops(messages: AgentMessage[]): AgentMessage[] { } function reminderMessages(messages: AgentMessage[]): AgentMessage[] { - return messages.filter( - message => - message.role === "developer" && - (typeof message.content === "string" - ? message.content.includes("previous assistant turn ended with no text") - : message.content.some( - content => - content.type === "text" && content.text.includes("previous assistant turn ended with no text"), - )), - ); + const isEmptyStopRetryReminder = (text: string): boolean => + text.includes("") && + text.includes("Previous assistant turn ended with no text, reasoning, or tool call.") && + text.includes("(Empty response retry "); + + return messages.filter(message => { + if (message.role !== "developer") return false; + return typeof message.content === "string" + ? isEmptyStopRetryReminder(message.content) + : message.content.some(content => content.type === "text" && isEmptyStopRetryReminder(content.text)); + }); } async function expectPromptCompletes(prompt: Promise): Promise { diff --git a/packages/coding-agent/test/core/python-executor-per-call.test.ts b/packages/coding-agent/test/core/python-executor-per-call.test.ts index 14fb234d9..0358d28ed 100644 --- a/packages/coding-agent/test/core/python-executor-per-call.test.ts +++ b/packages/coding-agent/test/core/python-executor-per-call.test.ts @@ -122,13 +122,17 @@ describe("executePython (per-call)", () => { using tempDir = TempDir.createSync("@omp-python-executor-per-call-"); let shutdownCalls = 0; + let executeTimeoutMs: number | undefined; const kernel: KernelStub = { - execute: async () => ({ - status: "ok", - cancelled: true, - timedOut: true, - stdinRequested: false, - }), + execute: async (_code: string, options?: KernelExecuteOptions) => { + executeTimeoutMs = options?.timeoutMs; + return { + status: "ok", + cancelled: true, + timedOut: true, + stdinRequested: false, + }; + }, shutdown: async () => { shutdownCalls += 1; }, @@ -144,7 +148,10 @@ describe("executePython (per-call)", () => { expect(result.cancelled).toBe(true); expect(result.exitCode).toBeUndefined(); - expect(result.output).toContain("eval cell timed out after 2s"); + expect(executeTimeoutMs).toBeGreaterThan(0); + expect(executeTimeoutMs).toBeLessThanOrEqual(2000); + const expectedSeconds = Math.max(1, Math.round((executeTimeoutMs ?? 0) / 1000)); + expect(result.output).toContain(`eval cell timed out after ${expectedSeconds}s`); expect(shutdownCalls).toBe(1); }); }); diff --git a/packages/coding-agent/test/event-controller-abort-render.test.ts b/packages/coding-agent/test/event-controller-abort-render.test.ts index d73ecc34f..97737da62 100644 --- a/packages/coding-agent/test/event-controller-abort-render.test.ts +++ b/packages/coding-agent/test/event-controller-abort-render.test.ts @@ -49,7 +49,8 @@ function createFixture(opts: { }) { const updateContent = vi.fn(); const setUsageInfo = vi.fn(); - const streamingComponent = { updateContent, setUsageInfo }; + const setComplete = vi.fn(); + const streamingComponent = { updateContent, setUsageInfo, setComplete }; const requestRender = vi.fn(); const ctx = { diff --git a/packages/coding-agent/test/extensibility/custom-commands/review.test.ts b/packages/coding-agent/test/extensibility/custom-commands/review.test.ts index 076412a38..d3f79b56e 100644 --- a/packages/coding-agent/test/extensibility/custom-commands/review.test.ts +++ b/packages/coding-agent/test/extensibility/custom-commands/review.test.ts @@ -9,7 +9,7 @@ import * as git from "../../../src/utils/git"; import * as jj from "../../../src/utils/jj"; const LEGACY_TASK_INSTRUCTION = 'Use the Task tool with `agent: "reviewer"` to execute this review.'; -const REVIEWER_TASK_INSTRUCTION = 'Use the `task` tool with `agent: "reviewer"` and a `tasks` array.'; +const REVIEWER_TASK_INSTRUCTION = 'Use `task` tool with `agent: "reviewer"` and `tasks` array.'; const SAMPLE_JJ_DIFF = `diff --git a/src/workspace.ts b/src/workspace.ts --- a/src/workspace.ts diff --git a/packages/coding-agent/test/interactive-mode-plan-review.test.ts b/packages/coding-agent/test/interactive-mode-plan-review.test.ts index 1a96d0608..7b92b33c3 100644 --- a/packages/coding-agent/test/interactive-mode-plan-review.test.ts +++ b/packages/coding-agent/test/interactive-mode-plan-review.test.ts @@ -531,7 +531,7 @@ describe("InteractiveMode plan review rendering", () => { expect(compactSpy).toHaveBeenCalledTimes(1); const [compactInstruction] = compactSpy.mock.calls[0]!; expect(typeof compactInstruction).toBe("string"); - expect(compactInstruction as string).toContain("Preparing to execute the approved plan"); + expect(compactInstruction as string).toContain("We'll execute approved plan."); expect(compactInstruction as string).toContain(finalPlanFilePath); // Plan-approved synthetic prompt was dispatched. diff --git a/packages/coding-agent/test/mcp-http-transport.test.ts b/packages/coding-agent/test/mcp-http-transport.test.ts index d828e8de9..be1169702 100644 --- a/packages/coding-agent/test/mcp-http-transport.test.ts +++ b/packages/coding-agent/test/mcp-http-transport.test.ts @@ -68,6 +68,26 @@ describe("HTTP MCP transport", () => { } }); + it("reports required initialize request failures", async () => { + activeServer = Bun.serve({ + port: 0, + async fetch(request) { + if (request.method === "DELETE") { + return new Response(null, { status: 204 }); + } + return new Response("initialize exploded", { status: 500 }); + }, + }); + + await expect( + connectToServer("broken", { + type: "http", + url: String(activeServer.url), + timeout: 1_000, + }), + ).rejects.toThrow("HTTP 500: initialize exploded"); + }); + describe("resolveSSEConnectTimeoutMs", () => { const originalEnv = process.env.OMP_MCP_TIMEOUT_MS; diff --git a/packages/coding-agent/test/streaming-preview-height.test.ts b/packages/coding-agent/test/streaming-preview-height.test.ts index ee5e20dca..103f26eaf 100644 --- a/packages/coding-agent/test/streaming-preview-height.test.ts +++ b/packages/coding-agent/test/streaming-preview-height.test.ts @@ -269,7 +269,11 @@ describe("streaming edit preview height (stable, full tail window)", () => { tui.stop(); await term.flush(); } - }, 10_000); + // Real TUI + Ghostty WASM integration can exceed Bun's default budget on CI: + // startup, repeated native scrollback refreshes, and throttled render frames are + // intentionally exercised here. Keep the contract assertions above; only widen + // the integration-test budget. + }, 30_000); test("the underlying diff genuinely oscillates (guard against a vacuous test)", async () => { const ctx = { diff --git a/packages/coding-agent/test/system-prompt-templates.test.ts b/packages/coding-agent/test/system-prompt-templates.test.ts index c5d461cea..2d812039d 100644 --- a/packages/coding-agent/test/system-prompt-templates.test.ts +++ b/packages/coding-agent/test/system-prompt-templates.test.ts @@ -193,7 +193,7 @@ describe("system Handlebars prompt templates", () => { expect(subagentSystem).toMatch(/CONTEXT\n=+\n\nShared task background/); expect(subagentSystem).toMatch(/ROLE\n=+/); - expect(subagentUser).toContain("Complete the assignment below, thoroughly:"); + expect(subagentUser).toContain("Complete assignment below, thoroughly:"); expect(subagentUser).toContain("Do the task."); expect(subagentUser).not.toMatch(/CONTEXT\n=+/); expect(subagentUser).not.toContain("Shared task background"); @@ -210,7 +210,7 @@ describe("system Handlebars prompt templates", () => { expect(withPlan).toMatch(/PLAN\n=+/); expect(withPlan).toContain(''); expect(withPlan).toContain("1. Migrate the store\n2. Update callers"); - expect(withPlan).toContain("executing an approved plan"); + expect(withPlan).toContain("Session executing approved plan."); const withoutPlan = prompt.render(systemTemplate, { ...baseRenderContext, @@ -233,7 +233,7 @@ describe("system Handlebars prompt templates", () => { }); expect(rendered).toContain("## Discovery"); - expect(rendered).toContain("Discoverable MCP servers in this session: github (2 tools), slack (1 tool)."); + expect(rendered).toContain("Discoverable MCP servers in session: github (2 tools), slack (1 tool)."); expect(rendered).not.toContain("Example discoverable MCP tools:"); expect(rendered).toContain("call `search_tool_bm25` before concluding no such tool exists"); }); @@ -285,9 +285,9 @@ describe("system Handlebars prompt templates", () => { expect(systemPrompt[0]).not.toContain("current working directory"); expect(systemPrompt[1]).toContain(""); expect(systemPrompt[1]).toContain(""); - expect(systemPrompt[1]).toContain("Today is "); - expect(systemPrompt[1]).toContain(`current working directory is '${dir}'.`); - expect(systemPrompt[1].indexOf("")).toBeLessThan(systemPrompt[1].indexOf("Today is ")); + expect(systemPrompt[1]).toContain("Today "); + expect(systemPrompt[1]).toContain(`cwd \`${dir}\`.`); + expect(systemPrompt[1].indexOf("")).toBeLessThan(systemPrompt[1].indexOf("Today ")); }); }); test("buildSystemPrompt renders workspace tree after directory context in project prompt", async () => { @@ -310,8 +310,8 @@ describe("system Handlebars prompt templates", () => { const projectPrompt = systemPrompt[1] ?? ""; expect(projectPrompt).toContain(""); - expect(projectPrompt).toContain("Working directory layout (sorted by mtime, recent first; depth ≤ 3):"); - expect(projectPrompt).toContain("(some entries elided to keep the tree short"); + expect(projectPrompt).toContain("Working directory layout (sorted mtime, recent first; depth ≤ 3):"); + expect(projectPrompt).toContain("(some entries elided keep tree short"); expect(projectPrompt.indexOf("")).toBeLessThan(projectPrompt.indexOf("")); }); }); diff --git a/packages/coding-agent/test/task/executor-subagent-reminders.test.ts b/packages/coding-agent/test/task/executor-subagent-reminders.test.ts index 0e47b9d8f..cf0184ef6 100644 --- a/packages/coding-agent/test/task/executor-subagent-reminders.test.ts +++ b/packages/coding-agent/test/task/executor-subagent-reminders.test.ts @@ -265,7 +265,8 @@ describe("runSubprocess yield reminders", () => { expect(promptOptions).toHaveLength(2); expect(promptOptions[0]?.attribution).toBe("agent"); expect(promptOptions[1]?.attribution).toBe("agent"); - expect(prompts[1]).toContain("Your last turn ended without a tool call"); + expect(prompts[1]).toContain("Last turn ended without tool call; session idle."); + expect(prompts[1]).toContain("Every turn MUST end with tool call."); expect(result.output).toContain('"done": true'); expect(result.output.includes("SYSTEM WARNING")).toBe(false); });