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.
This commit is contained in:
can1357
2026-06-04 16:35:45 +02:00
parent 457fe9eb26
commit 51865fc1ea
13 changed files with 147 additions and 50 deletions
+2 -2
View File
@@ -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");
});
});
@@ -918,7 +918,7 @@ describe("Codex-style Abort Handling", () => {
const guidanceMsg = transformed[3] as DeveloperMessage;
expect(guidanceMsg.role).toBe("developer");
expect(guidanceMsg.content).toContain("<turn-aborted>");
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", () => {
@@ -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<null>((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?.();
});
}
@@ -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();
}
});
});
@@ -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("<system-reminder>") &&
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<void>): Promise<void> {
@@ -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);
});
});
@@ -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 = {
@@ -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
@@ -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.
@@ -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;
@@ -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 = {
@@ -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('<plan path="local://wp-migration.md">');
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("<workstation>");
expect(systemPrompt[1]).toContain("<workspace-tree>");
expect(systemPrompt[1]).toContain("Today is ");
expect(systemPrompt[1]).toContain(`current working directory is '${dir}'.`);
expect(systemPrompt[1].indexOf("</workspace-tree>")).toBeLessThan(systemPrompt[1].indexOf("Today is "));
expect(systemPrompt[1]).toContain("Today ");
expect(systemPrompt[1]).toContain(`cwd \`${dir}\`.`);
expect(systemPrompt[1].indexOf("</workspace-tree>")).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("<workspace-tree>");
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("</dir-context>")).toBeLessThan(projectPrompt.indexOf("<workspace-tree>"));
});
});
@@ -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);
});