fix(coding-agent): reduce redundant slow tests

This commit is contained in:
Armin Ronacher 2026-08-17 22:57:18 +02:00
parent 54d22b74b3
commit b82a374c7d
8 changed files with 146 additions and 235 deletions

View file

@ -63,6 +63,11 @@ export function isValidThinkingLevel(level: string): level is ThinkingLevel {
return VALID_THINKING_LEVELS.includes(level as ThinkingLevel);
}
export function normalizeSessionName(value: string): string | undefined {
const name = value.trim();
return name.length > 0 ? name : undefined;
}
export function parseArgs(args: string[]): Args {
const result: Args = {
messages: [],

View file

@ -8,7 +8,7 @@
import { createInterface } from "node:readline";
import { type ImageContent, modelsAreEqual } from "@earendil-works/pi-ai";
import chalk from "chalk";
import { type Args, type Mode, parseArgs, printHelp } from "./cli/args.ts";
import { type Args, type Mode, normalizeSessionName, parseArgs, printHelp } from "./cli/args.ts";
import {
type AuthCheckResult,
checkProviderAuth,
@ -357,7 +357,7 @@ function forkSessionOrExit(sourcePath: string, cwd: string, sessionDir?: string,
}
}
async function createSessionManager(
export async function createSessionManager(
parsed: Args,
cwd: string,
sessionDir: string | undefined,
@ -694,8 +694,8 @@ export async function main(args: string[], options?: MainOptions) {
}
}
if (parsed.name !== undefined) {
const name = parsed.name.trim();
if (!name) {
const name = normalizeSessionName(parsed.name);
if (name === undefined) {
console.error(chalk.red("Error: --name requires a non-empty value"));
process.exit(1);
}

View file

@ -1,5 +1,5 @@
import { describe, expect, test } from "vitest";
import { parseArgs } from "../src/cli/args.ts";
import { normalizeSessionName, parseArgs } from "../src/cli/args.ts";
describe("parseArgs", () => {
describe("--version flag", () => {
@ -173,6 +173,11 @@ describe("parseArgs", () => {
expect(result.name).toBe("");
});
test("normalizes display names and rejects whitespace-only values", () => {
expect(normalizeSessionName(" named session ")).toBe("named session");
expect(normalizeSessionName(" ")).toBeUndefined();
});
test("reports missing value", () => {
const result = parseArgs(["--name"]);
expect(result.diagnostics).toEqual([{ type: "error", message: "--name requires a value" }]);
@ -192,6 +197,21 @@ describe("parseArgs", () => {
const result = parseArgs(["--no-session"]);
expect(result.noSession).toBe(true);
});
test("preserves custom session IDs for non-persisting commands", () => {
expect(parseArgs(["--session-id", "ephemeral-id", "--help"])).toMatchObject({
sessionId: "ephemeral-id",
help: true,
});
expect(parseArgs(["--session-id", "ephemeral-id", "--list-models"])).toMatchObject({
sessionId: "ephemeral-id",
listModels: true,
});
expect(parseArgs(["--session-id", "ephemeral-id", "--no-session"])).toMatchObject({
sessionId: "ephemeral-id",
noSession: true,
});
});
});
describe("--extension flag", () => {

View file

@ -1,23 +1,19 @@
import { spawn } from "node:child_process";
import {
existsSync,
mkdirSync,
mkdtempSync,
readdirSync,
readFileSync,
realpathSync,
rmSync,
writeFileSync,
} from "node:fs";
import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, realpathSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { Args } from "../src/cli/args.ts";
import { ENV_AGENT_DIR } from "../src/config.ts";
import { SessionManager } from "../src/core/session-manager.ts";
import { SettingsManager } from "../src/core/settings-manager.ts";
import { createSessionManager } from "../src/main.ts";
const cliPath = resolve(__dirname, "../src/cli.ts");
const tempDirs: string[] = [];
afterEach(() => {
vi.restoreAllMocks();
for (const dir of tempDirs.splice(0)) {
rmSync(dir, { recursive: true, force: true });
}
@ -50,141 +46,128 @@ function hasSessionWithId(root: string, sessionId: string): boolean {
return false;
}
interface CliDirs {
agentDir: string;
projectDir: string;
sessionDir: string;
}
async function runCli(
args: string[] | ((dirs: CliDirs) => string[]),
setup?: (dirs: CliDirs) => void,
): Promise<{ code: number | null; agentDir: string; stderr: string }> {
async function runCli(args: string[]): Promise<{ code: number | null; agentDir: string }> {
const tempRoot = createTempDir();
const dirs: CliDirs = {
agentDir: join(tempRoot, "agent"),
projectDir: join(tempRoot, "project"),
sessionDir: join(tempRoot, "sessions"),
};
mkdirSync(dirs.agentDir, { recursive: true });
mkdirSync(dirs.projectDir, { recursive: true });
setup?.(dirs);
const resolvedArgs = typeof args === "function" ? args(dirs) : args;
const agentDir = join(tempRoot, "agent");
const projectDir = join(tempRoot, "project");
mkdirSync(agentDir, { recursive: true });
mkdirSync(projectDir, { recursive: true });
let stderr = "";
const code = await new Promise<number | null>((resolvePromise, reject) => {
const child = spawn(process.execPath, [cliPath, ...resolvedArgs], {
cwd: dirs.projectDir,
const child = spawn(process.execPath, [cliPath, ...args], {
cwd: projectDir,
env: {
...process.env,
[ENV_AGENT_DIR]: dirs.agentDir,
[ENV_AGENT_DIR]: agentDir,
PI_OFFLINE: "1",
TSX_TSCONFIG_PATH: resolve(__dirname, "../../../tsconfig.json"),
},
stdio: ["ignore", "ignore", "pipe"],
});
child.stderr.on("data", (chunk) => {
stderr += chunk.toString();
stdio: ["ignore", "ignore", "ignore"],
});
child.on("error", reject);
child.on("close", resolvePromise);
});
return { code, agentDir: dirs.agentDir, stderr };
return { code, agentDir };
}
function writeSession(sessionDir: string, cwd: string, id: string): void {
writeFileSync(
join(sessionDir, `${id}.jsonl`),
`${JSON.stringify({ type: "session", version: 3, id, timestamp: new Date().toISOString(), cwd })}\n`,
);
function args(overrides: Partial<Args>): Args {
return {
messages: [],
fileArgs: [],
unknownFlags: new Map(),
diagnostics: [],
...overrides,
};
}
describe("--session-id read-only commands", () => {
it("does not reserve a session for --help", async () => {
function persistSession(session: SessionManager, content: string): void {
session.appendMessage({ role: "user", content, timestamp: Date.now() });
session.appendMessage({
role: "assistant",
content: [{ type: "text", text: "persisted" }],
api: "anthropic-messages",
provider: "anthropic",
model: "test",
usage: {
input: 1,
output: 1,
cacheRead: 0,
cacheWrite: 0,
totalTokens: 2,
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
},
stopReason: "stop",
timestamp: Date.now(),
});
}
describe("--session-id", () => {
it("does not persist a custom ID for metadata commands", async () => {
const result = await runCli(["--session-id", "read-only-help", "--help"]);
expect(result.code).toBe(0);
expect(hasSessionWithId(join(result.agentDir, "sessions"), "read-only-help")).toBe(false);
});
it("allows --no-session with --session-id", async () => {
const result = await runCli(["--no-session", "--session-id", "ephemeral-id", "--help"]);
it("creates missing IDs and reopens existing IDs in process", async () => {
const tempRoot = createTempDir();
const projectDir = join(tempRoot, "project");
const sessionDir = join(tempRoot, "sessions");
mkdirSync(projectDir, { recursive: true });
const settingsManager = SettingsManager.inMemory();
const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
expect(result.code).toBe(0);
expect(hasSessionWithId(join(result.agentDir, "sessions"), "ephemeral-id")).toBe(false);
});
it("does not reserve a session for --list-models", async () => {
const result = await runCli(["--session-id", "read-only-models", "--list-models"]);
expect(result.code).toBe(0);
expect(hasSessionWithId(join(result.agentDir, "sessions"), "read-only-models")).toBe(false);
});
it("warns when a missing --session-id creates a new session", async () => {
const result = await runCli((dirs) => [
"--session-dir",
dirs.sessionDir,
"--session-id",
"missing-session-id",
"--model",
"missing-model",
"-p",
"hi",
]);
expect(result.code).toBe(1);
expect(result.stderr).toContain(
"Warning: No project session found with id 'missing-session-id'; creating a new session with that id.",
const readOnly = await createSessionManager(
args({ sessionId: "read-only", help: true }),
projectDir,
sessionDir,
settingsManager,
);
expect(readOnly.getSessionId()).toBe("read-only");
expect(readOnly.getSessionFile()).toBeUndefined();
const created = await createSessionManager(
args({ sessionId: "persisted-id" }),
projectDir,
sessionDir,
settingsManager,
);
persistSession(created, "persist me");
expect(consoleError).toHaveBeenCalledWith(expect.stringContaining("creating a new session"));
consoleError.mockClear();
const reopened = await createSessionManager(
args({ sessionId: "persisted-id" }),
projectDir,
sessionDir,
settingsManager,
);
expect(reopened.getSessionFile()).toBe(created.getSessionFile());
expect(consoleError).not.toHaveBeenCalled();
});
it("does not warn when --session-id opens an existing session", async () => {
const result = await runCli(
(dirs) => [
"--session-dir",
dirs.sessionDir,
"--session-id",
"existing-session-id",
"--model",
"missing-model",
"-p",
"hi",
],
(dirs) => {
mkdirSync(dirs.sessionDir, { recursive: true });
writeSession(dirs.sessionDir, dirs.projectDir, "existing-session-id");
},
);
it("rejects an existing fork target in process", async () => {
const tempRoot = createTempDir();
const projectDir = join(tempRoot, "project");
const sessionDir = join(tempRoot, "sessions");
mkdirSync(projectDir, { recursive: true });
const source = SessionManager.create(projectDir, sessionDir, { id: "source-id" });
persistSession(source, "source");
const target = SessionManager.create(projectDir, sessionDir, { id: "existing-id" });
persistSession(target, "target");
vi.spyOn(console, "error").mockImplementation(() => {});
vi.spyOn(process, "exit").mockImplementation((code) => {
throw new Error(`exit:${code}`);
});
expect(result.code).toBe(1);
expect(result.stderr).not.toContain("No project session found with id 'existing-session-id'");
});
it("rejects an existing fork target session id", async () => {
const result = await runCli(
(dirs) => ["--session-dir", dirs.sessionDir, "--fork", "source-id", "--session-id", "existing-id", "-p", "hi"],
(dirs) => {
mkdirSync(dirs.sessionDir, { recursive: true });
writeSession(dirs.sessionDir, dirs.projectDir, "source-id");
writeSession(dirs.sessionDir, dirs.projectDir, "existing-id");
},
);
expect(result.code).toBe(1);
expect(result.stderr).toContain("Session already exists with id 'existing-id'");
});
});
describe("--session-id validation", () => {
it("rejects ids invalid under SessionManager rules without stack traces", async () => {
for (const id of ["-bad", "bad id"]) {
const result = await runCli(["--session-id", id, "-p", "hi"]);
expect(result.code).toBe(1);
expect(result.stderr).toContain("Session id must be non-empty");
expect(result.stderr).not.toContain("SessionManager.create");
}
await expect(
createSessionManager(
args({ fork: "source-id", sessionId: "existing-id" }),
projectDir,
sessionDir,
SettingsManager.inMemory(),
),
).rejects.toThrow("exit:1");
});
});

View file

@ -119,17 +119,4 @@ describe("startup session name", () => {
expect(result.signal).toBeNull();
expect(readSessionInfoNames(dirs.sessionFile)).toEqual(["CLI Named Session"]);
});
it("rejects empty --name values without appending session metadata", async () => {
const dirs = setup();
const result = await runCli(
["--session", dirs.sessionFile, "--name", " ", "--model", "missing-model", "-p", "hi"],
dirs,
);
expect(result.code).toBe(1);
expect(result.signal).toBeNull();
expect(result.stderr).toContain("--name requires a non-empty value");
expect(readSessionInfoNames(dirs.sessionFile)).toEqual([]);
});
});

View file

@ -85,20 +85,14 @@ async function runCli(args: string[]): Promise<{ stdout: string; stderr: string;
}
describe("stdout cleanliness in non-interactive modes", () => {
it("prints --version to stdout when stdout is redirected", async () => {
const result = await runCli(["--version"]);
expect(result.code).toBe(0);
expect(result.stdout.trim()).toMatch(/^\d+\.\d+\.\d+/);
expect(result.stderr).toBe("");
});
it("prints plain --help to stdout when stdout is redirected", async () => {
const result = await runCli(["--help"]);
expect(result.code).toBe(0);
expect(result.stdout).toContain("Usage:");
expect(result.stderr).not.toContain("Usage:");
expect(result.stderr).not.toContain("changed 1 package in 471ms");
expect(result.stderr).not.toContain("found 0 vulnerabilities");
});
it("keeps stdout empty for --mode json --help while routing trusted startup chatter to stderr", async () => {
@ -110,24 +104,4 @@ describe("stdout cleanliness in non-interactive modes", () => {
expect(result.stderr).toContain("found 0 vulnerabilities");
expect(result.stderr).toContain("Usage:");
});
it("keeps stdout empty for -p --help while routing trusted startup chatter to stderr", async () => {
const result = await runCli(["-p", "--help", "--approve"]);
expect(result.code).toBe(0);
expect(result.stdout).toBe("");
expect(result.stderr).toContain("changed 1 package in 471ms");
expect(result.stderr).toContain("found 0 vulnerabilities");
expect(result.stderr).toContain("Usage:");
});
it("ignores untrusted project package installs for help", async () => {
const result = await runCli(["-p", "--help"]);
expect(result.code).toBe(0);
expect(result.stdout).toBe("");
expect(result.stderr).not.toContain("changed 1 package in 471ms");
expect(result.stderr).not.toContain("found 0 vulnerabilities");
expect(result.stderr).toContain("Usage:");
});
});

View file

@ -1,95 +1,40 @@
import { crc32, deflateSync } from "node:zlib";
import type { AgentTool } from "@earendil-works/pi-agent-core";
import type { ImageContent } from "@earendil-works/pi-ai";
import { fauxAssistantMessage, fauxToolCall } from "@earendil-works/pi-ai";
import { Type } from "typebox";
import { afterEach, describe, expect, it } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import { createHarness, type Harness } from "./harness.ts";
function pngChunk(type: string, body: Buffer): Buffer {
const header = Buffer.alloc(8);
header.writeUInt32BE(body.length, 0);
header.write(type, 4, "ascii");
const checksum = Buffer.alloc(4);
checksum.writeUInt32BE(crc32(Buffer.concat([header.subarray(4), body])), 0);
return Buffer.concat([header, body, checksum]);
}
const normalizeToolResultImages = vi.hoisted(() => vi.fn(async (content: unknown[]) => content));
vi.mock("../../src/utils/tool-result-images.ts", () => ({ normalizeToolResultImages }));
/** Build an 8-bit grayscale PNG of arbitrary dimensions without pulling in an encoder. */
function createPng(width: number, height: number): Buffer {
const ihdr = Buffer.alloc(13);
ihdr.writeUInt32BE(width, 0);
ihdr.writeUInt32BE(height, 4);
ihdr[8] = 8; // bit depth
ihdr[9] = 0; // color type: grayscale
const raw = Buffer.alloc((width + 1) * height);
for (let row = 0; row < height; row++) {
raw.fill(row % 256, row * (width + 1) + 1, (row + 1) * (width + 1));
}
return Buffer.concat([
Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]),
pngChunk("IHDR", ihdr),
pngChunk("IDAT", deflateSync(raw)),
pngChunk("IEND", Buffer.alloc(0)),
]);
}
const TINY_PNG_BASE64 =
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8DwHwAFBQIAX8jx0gAAAABJRU5ErkJggg==";
function readPngDimensions(base64Data: string): { width: number; height: number } {
const buffer = Buffer.from(base64Data, "base64");
return { width: buffer.readUInt32BE(16), height: buffer.readUInt32BE(20) };
}
const OVERSIZED_PNG_BASE64 = createPng(2400, 4800).toString("base64");
/** Stands in for extension, MCP bridge, or screenshot tools that return images they produced. */
const screenshotTool: AgentTool = {
name: "screenshot",
label: "Screenshot",
description: "Return an oversized screenshot",
description: "Return a screenshot",
parameters: Type.Object({}),
execute: async () => ({
content: [
{ type: "text", text: "captured" },
{ type: "image", data: OVERSIZED_PNG_BASE64, mimeType: "image/png" },
{ type: "image", data: TINY_PNG_BASE64, mimeType: "image/png" },
],
details: {},
}),
};
function getToolResultImages(harness: Harness): ImageContent[] {
return harness.session.messages
.filter((message) => message.role === "toolResult")
.flatMap((message) => message.content)
.filter((block): block is ImageContent => block.type === "image");
}
describe("AgentSession tool result images", () => {
const harnesses: Harness[] = [];
afterEach(() => {
normalizeToolResultImages.mockClear();
while (harnesses.length > 0) {
harnesses.pop()?.cleanup();
}
});
it("resizes oversized tool result images before they enter history", async () => {
const harness = await createHarness({ tools: [screenshotTool] });
harnesses.push(harness);
harness.setResponses([
fauxAssistantMessage([fauxToolCall("screenshot", {})], { stopReason: "toolUse" }),
fauxAssistantMessage("done"),
]);
await harness.session.prompt("take a screenshot");
const images = getToolResultImages(harness);
expect(images).toHaveLength(1);
const { width, height } = readPngDimensions(images[0].data);
expect(width).toBeLessThanOrEqual(2000);
expect(height).toBeLessThanOrEqual(2000);
});
it("honors images.autoResize being disabled", async () => {
it("passes images.autoResize to tool result normalization", async () => {
const harness = await createHarness({
tools: [screenshotTool],
settings: { images: { autoResize: false } },
@ -102,8 +47,6 @@ describe("AgentSession tool result images", () => {
await harness.session.prompt("take a screenshot");
const images = getToolResultImages(harness);
expect(images).toHaveLength(1);
expect(images[0].data).toBe(OVERSIZED_PNG_BASE64);
expect(normalizeToolResultImages).toHaveBeenCalledWith(expect.any(Array), { autoResizeImages: false });
});
});

View file

@ -486,9 +486,8 @@ describe("Coding Agent Tools", () => {
});
it("should respect timeout", async () => {
await expect(bashTool.execute("test-call-10", { command: "sleep 5", timeout: 1 })).rejects.toThrow(
/timed out/i,
);
const command = `${JSON.stringify(process.execPath)} -e ${JSON.stringify("setInterval(() => {}, 1000)")}`;
await expect(bashTool.execute("test-call-10", { command, timeout: 0.05 })).rejects.toThrow(/timed out/i);
});
it("should include full output path for truncated timeout and abort errors", async () => {