From 20d19e80020d79ad3bc2a26f88859602560f5a8d Mon Sep 17 00:00:00 2001 From: can1357 Date: Sat, 6 Jun 2026 22:09:04 +0200 Subject: [PATCH] test: replaced blind sleeps with shared fixtures and condition polling - Shared immutable model registries and auth storage via beforeAll/afterAll. - Swapped fixed-delay settle sleeps for predicate polling and signals. - Stubbed network/timers to drop wall-clock waits in registry and history tests. - Added resetDisplay invalidation tests and startup-timing breakdown lines. --- .../src/eval/__tests__/agent-bridge.test.ts | 56 ++-- .../test/agent-session-concurrent.test.ts | 23 +- .../agent-session-context-promotion.test.ts | 30 +- .../test/agent-session-handoff.test.ts | 93 ++++-- .../agent-session-model-persistence.test.ts | 45 +-- ...nt-session-openai-responses-replay.test.ts | 91 +++--- .../test/agent-session-python-cleanup.test.ts | 3 + .../test/agent-session-retry-fallback.test.ts | 26 +- .../test/autoresearch-tools.test.ts | 34 +-- .../coding-agent/test/bash-executor.test.ts | 44 ++- .../test/extensions-runner.test.ts | 23 +- .../test/goals/goal-mode-integration.test.ts | 45 ++- .../test/interactive-mode-plan-review.test.ts | 2 - .../keybindings-selector-navigation.test.ts | 13 +- .../test/mcp-reconnect-storm.test.ts | 17 +- .../model-registry-runtime-provider.test.ts | 12 +- .../coding-agent/test/model-registry.test.ts | 5 +- .../components/transcript-container.test.ts | 19 ++ .../input-controller-tool-expansion.test.ts | 11 +- .../test/plan-mode-thinking-level.test.ts | 7 +- .../sdk-async-job-manager-singleton.test.ts | 25 +- .../sdk-credential-disabled-bridge.test.ts | 13 +- .../test/sdk-mcp-discovery.test.ts | 28 +- .../test/sdk-model-selection.test.ts | 23 +- .../test/sdk-session-isolation.test.ts | 25 +- packages/coding-agent/test/sdk-skills.test.ts | 28 +- .../test/sdk-tool-activation.test.ts | 152 +++------- packages/coding-agent/test/tools.test.ts | 31 +- .../test/tools/approval-mode.test.ts | 279 +++++++----------- .../test/tools/conflict-integration.test.ts | 6 +- .../test/tools/fetch-jina-stall.test.ts | 17 +- packages/coding-agent/test/tools/gh.test.ts | 51 +++- .../test/tools/lsp-regressions.test.ts | 4 + packages/tui/test/render-regressions.test.ts | 54 ++++ packages/utils/CHANGELOG.md | 4 + packages/utils/src/logger.ts | 15 +- 36 files changed, 854 insertions(+), 500 deletions(-) diff --git a/packages/coding-agent/src/eval/__tests__/agent-bridge.test.ts b/packages/coding-agent/src/eval/__tests__/agent-bridge.test.ts index dd66f44cc..838ed6c1a 100644 --- a/packages/coding-agent/src/eval/__tests__/agent-bridge.test.ts +++ b/packages/coding-agent/src/eval/__tests__/agent-bridge.test.ts @@ -377,18 +377,6 @@ describe("agent() through eval runtimes", () => { singleResult(options, { output: "hello from python" }), ); - const probe = await executePython('print("probe")', { - cwd: tempDir.path(), - sessionId: `${sessionId}:probe`, - sessionFile, - kernelMode: "per-call", - }); - if (probe.exitCode === undefined && probe.cancelled) { - expect(probe.output).toBe(""); - return; - } - expect(probe.exitCode).toBe(0); - const result = await executePython('print(agent("hi"))', { cwd: tempDir.path(), sessionId, @@ -396,6 +384,10 @@ describe("agent() through eval runtimes", () => { kernelMode: "per-call", toolSession: session, }); + if (result.exitCode === undefined && result.cancelled) { + expect(result.output).toBe(""); + return; // kernel unavailable in this environment + } expect(result.exitCode).toBe(0); expect(result.output.trim()).toBe("hello from python"); @@ -424,22 +416,14 @@ describe("agent() through eval runtimes", () => { } }); - const probe = await executePython('print("probe")', { - cwd: tempDir.path(), - sessionId: `${sessionId}:probe`, - sessionFile, - kernelMode: "per-call", - }); - if (probe.exitCode === undefined && probe.cancelled) { - expect(probe.output).toBe(""); - return; - } - expect(probe.exitCode).toBe(0); - const result = await executePython( 'import json\nprint(json.dumps(parallel([lambda n=n: agent(n) for n in ["a", "b", "c", "d"]])))', { cwd: tempDir.path(), sessionId, sessionFile, kernelMode: "per-call", toolSession: session }, ); + if (result.exitCode === undefined && result.cancelled) { + expect(result.output).toBe(""); + return; // kernel unavailable in this environment + } expect(result.exitCode).toBe(0); expect(JSON.parse(result.output.trim())).toEqual(["a", "b", "c", "d"]); @@ -463,7 +447,14 @@ describe("agent() through eval runtimes", () => { // The host must respond the instant the cell aborts so the kernel can // unwind via KeyboardInterrupt instead of being hard-killed (which used to // surface "[kernel] Python kernel shutdown" and lose all session state). + let inFlight = 0; + let markSaturated: (() => void) | undefined; + const saturated = new Promise(resolve => { + markSaturated = resolve; + }); vi.spyOn(taskExecutor, "runSubprocess").mockImplementation(async options => { + // task.maxConcurrency=6 → six bridge calls block at once; signal then. + if (++inFlight >= 6) markSaturated?.(); await Bun.sleep(9000); // deliberately ignores options.signal return singleResult(options, { output: options.assignment ?? "" }); }); @@ -483,8 +474,9 @@ describe("agent() through eval runtimes", () => { expect(seed.exitCode).toBe(0); const ac = new AbortController(); - // Abort ~1s in, after the worker threads are blocked in their bridge calls. - setTimeout(() => ac.abort(new Error("external interrupt")), 1000); + // Abort the instant all six worker threads are confirmed blocked in their + // bridge calls (condition-driven) instead of waiting a fixed wall second. + void saturated.then(() => ac.abort(new Error("external interrupt"))); const start = Date.now(); const result = await executePython( @@ -619,12 +611,12 @@ describe("agent() through eval runtimes", () => { // of its own. The bridge pause must make that delegated time invisible to // the watchdog. vi.spyOn(taskExecutor, "runSubprocess").mockImplementation(async options => { - await Bun.sleep(200); + await Bun.sleep(40); return singleResult(options, { output: "done" }); }); const ops: string[] = []; - using idle = new IdleTimeout(60); + using idle = new IdleTimeout(20); const result = await runEvalAgent( { prompt: "investigate" }, { @@ -642,7 +634,7 @@ describe("agent() through eval runtimes", () => { expect(ops).toEqual([EVAL_TIMEOUT_PAUSE_OP, EVAL_TIMEOUT_RESUME_OP]); expect(idle.signal.aborted).toBe(false); - await Bun.sleep(90); + await Bun.sleep(60); expect(idle.signal.aborted).toBe(true); }); @@ -655,7 +647,7 @@ describe("agent() through eval runtimes", () => { // They render as status, but timeout accounting is controlled only by the // bridge pause/resume events. vi.spyOn(taskExecutor, "runSubprocess").mockImplementation(async options => { - for (let i = 0; i < 40; i++) { + for (let i = 0; i < 20; i++) { options.onProgress?.({ index: options.index, id: options.id, @@ -672,13 +664,13 @@ describe("agent() through eval runtimes", () => { cost: 0, durationMs: i * 10, }); - await Bun.sleep(10); + await Bun.sleep(5); } return singleResult(options, { output: "done" }); }); const ops: string[] = []; - using idle = new IdleTimeout(80); + using idle = new IdleTimeout(40); const result = await runEvalAgent( { prompt: "investigate" }, { diff --git a/packages/coding-agent/test/agent-session-concurrent.test.ts b/packages/coding-agent/test/agent-session-concurrent.test.ts index f03cbf039..a5298263a 100644 --- a/packages/coding-agent/test/agent-session-concurrent.test.ts +++ b/packages/coding-agent/test/agent-session-concurrent.test.ts @@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; +import { scheduler } from "node:timers/promises"; import { Agent, AgentBusyError, type AgentTool } from "@oh-my-pi/pi-agent-core"; import { type AssistantMessage, getBundledModel, type Message, type ToolCall } from "@oh-my-pi/pi-ai"; import { createMockModel } from "@oh-my-pi/pi-ai/providers/mock"; @@ -25,6 +26,19 @@ import { createAssistantMessage } from "./helpers/agent-session-setup"; // Mock stream that mimics AssistantMessageEventStream +// AgentSession schedules its TTSR retry and context-promotion continuations +// through `scheduler.wait(delayMs, { signal })` (node:timers/promises), with +// blind 50ms/100ms "settle" delays. Tests that drive a continuation to +// completion would otherwise pay that wall-clock time on every run. This spy +// collapses the blind delay to a single macrotask hop (`scheduler.wait(0)`) +// while preserving the real abort-signal semantics, so the continuation still +// fires only after the aborted/overflowed turn has been recorded. Each test +// that opts in must run inside a block whose afterEach restores mocks. +const originalSchedulerWait = scheduler.wait.bind(scheduler); +function collapseSchedulerSettleDelays(): void { + vi.spyOn(scheduler, "wait").mockImplementation((_delayMs, options) => originalSchedulerWait(0, options)); +} + describe("AgentSession concurrent prompt guard", () => { let session: AgentSession; let tempDir: string; @@ -101,7 +115,7 @@ describe("AgentSession concurrent prompt guard", () => { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { if (predicate()) return; - await Bun.sleep(10); + await Bun.sleep(1); } throw new Error("Timed out waiting for condition"); @@ -595,13 +609,14 @@ describe("AgentSession TTSR resume gate", () => { if (tempDir && fs.existsSync(tempDir)) { fs.rmSync(tempDir, { recursive: true }); } + vi.restoreAllMocks(); }); async function waitFor(predicate: () => boolean, timeoutMs = 500): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { if (predicate()) return; - await Bun.sleep(10); + await Bun.sleep(1); } throw new Error("Timed out waiting for condition"); @@ -674,6 +689,7 @@ describe("AgentSession TTSR resume gate", () => { } it("prompt() blocks until TTSR interrupt continuation completes", async () => { + collapseSchedulerSettleDelays(); const model = getBundledModel("anthropic", "claude-sonnet-4-5")!; let streamCallCount = 0; let continuationCompleted = false; @@ -734,6 +750,7 @@ describe("AgentSession TTSR resume gate", () => { }); it("relativizes the rule file path in the TTSR interrupt injection (no absolute leak)", async () => { + collapseSchedulerSettleDelays(); const model = getBundledModel("anthropic", "claude-sonnet-4-5")!; let streamCallCount = 0; @@ -946,6 +963,7 @@ describe("AgentSession TTSR resume gate", () => { }); it("prompt() waits for TTSR continuation with tool calls to finish", async () => { + collapseSchedulerSettleDelays(); const model = getBundledModel("anthropic", "claude-sonnet-4-5")!; let streamCallCount = 0; let toolExecutionFinished = false; @@ -1306,6 +1324,7 @@ describe("AgentSession TTSR resume gate", () => { }); it("prompt() waits for context-promotion continuation to finish", async () => { + collapseSchedulerSettleDelays(); const authStorage = await AuthStorage.create(path.join(tempDir, "testauth-promo.db")); authStorages.push(authStorage); authStorage.setRuntimeApiKey("openai-codex", "test-key"); diff --git a/packages/coding-agent/test/agent-session-context-promotion.test.ts b/packages/coding-agent/test/agent-session-context-promotion.test.ts index dcbe8af92..06347689b 100644 --- a/packages/coding-agent/test/agent-session-context-promotion.test.ts +++ b/packages/coding-agent/test/agent-session-context-promotion.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; import * as path from "node:path"; import { Agent } from "@oh-my-pi/pi-agent-core"; import type { AssistantMessage, Model, ProviderSessionState } from "@oh-my-pi/pi-ai"; @@ -15,19 +15,26 @@ describe("AgentSession context promotion", () => { let modelRegistry: ModelRegistry; let authStorage: AuthStorage; - beforeEach(async () => { + beforeAll(async () => { + // ModelRegistry eagerly loads the immutable bundled model catalog in its + // constructor (~100ms). The catalog and auth fixture never change between + // tests here (tests only read models and add benign extra runtime keys), + // so build them once instead of paying ~950ms across the 9 cases. tempDir = TempDir.createSync("@pi-context-promotion-"); authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); authStorage.setRuntimeApiKey("openai-codex", "test-key"); modelRegistry = new ModelRegistry(authStorage); }); + afterAll(() => { + authStorage.close(); + tempDir.removeSync(); + }); + afterEach(async () => { if (session) { await session.dispose(); } - authStorage.close(); - tempDir.removeSync(); }); function createOverflowMessage( @@ -113,6 +120,17 @@ describe("AgentSession context promotion", () => { throw new Error("Timed out waiting for condition"); } + // Deterministically drain the fire-and-forget `agent_end` handler that + // `emitExternalEvent` dispatches. The handler's terminal maintenance work + // (`#checkCompaction`) is microtask-based on the no-promotion paths, so a + // single macrotask turn fully flushes it; `waitForIdle` then settles any + // tracked continuation. Used by the negative tests, which assert that *no* + // promotion happened and therefore need the handler to have actually run. + async function settle(): Promise { + await new Promise(resolve => setTimeout(resolve, 0)); + await session.waitForIdle(); + } + it("promotes to a larger-context model on overflow and clears codex websocket session state", async () => { const sparkModel = modelRegistry.find("openai-codex", "gpt-5.3-codex-spark"); const codexModel = modelRegistry.find("openai-codex", "gpt-5.5"); @@ -390,7 +408,7 @@ describe("AgentSession context promotion", () => { session.agent.emitExternalEvent({ type: "message_end", message: overflowMessage }); session.agent.emitExternalEvent({ type: "agent_end", messages: [overflowMessage] }); - await Bun.sleep(30); + await settle(); expect(session.model?.provider).toBe(sparkModel.provider); expect(session.model?.id).toBe(sparkModel.id); @@ -472,7 +490,7 @@ describe("AgentSession context promotion", () => { session.agent.emitExternalEvent({ type: "message_end", message: staleIncomplete }); session.agent.emitExternalEvent({ type: "agent_end", messages: [staleIncomplete] }); - await Bun.sleep(30); + await settle(); expect(session.model?.provider).toBe(codexModel.provider); expect(session.model?.id).toBe(codexModel.id); diff --git a/packages/coding-agent/test/agent-session-handoff.test.ts b/packages/coding-agent/test/agent-session-handoff.test.ts index e90506b33..3ecb612aa 100644 --- a/packages/coding-agent/test/agent-session-handoff.test.ts +++ b/packages/coding-agent/test/agent-session-handoff.test.ts @@ -1,8 +1,8 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; import * as path from "node:path"; import { Agent } from "@oh-my-pi/pi-agent-core"; import * as compactionModule from "@oh-my-pi/pi-agent-core/compaction"; -import type { AssistantMessage, ToolCall } from "@oh-my-pi/pi-ai"; +import type { AssistantMessage, Model, ToolCall } from "@oh-my-pi/pi-ai"; import { getBundledModel } from "@oh-my-pi/pi-ai/models"; import { createMockModel } from "@oh-my-pi/pi-ai/providers/mock"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; @@ -14,26 +14,67 @@ import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manage import { TempDir } from "@oh-my-pi/pi-utils"; describe("AgentSession handoff", () => { + // Immutable across the whole file: the model registry's synchronous bundled-model + // load dominates per-test setup (~100ms each), and the auth store + bundled model + // never change. Build them once. Per-test mutable state (session, session file, + // emitted events) is rebuilt in beforeEach. + let sharedDir: TempDir; + let authStorage: AuthStorage; + let modelRegistry: ModelRegistry; + let model: Model; + let tempDir: TempDir; let session: AgentSession; let sessionManager: SessionManager; - let authStorage: AuthStorage; - let modelRegistry: ModelRegistry; let events: AgentSessionEvent[]; + /** Poll `predicate` until it holds (returns as soon as the state is reached) or the + * deadline elapses. Replaces blind settle sleeps for tests with a positive signal. */ + async function waitFor(predicate: () => boolean, timeoutMs = 1_000): Promise { + const deadline = Date.now() + timeoutMs; + while (!predicate()) { + if (Date.now() >= deadline) { + throw new Error("Timed out waiting for condition"); + } + await Bun.sleep(1); + } + } + + /** Drain post-turn maintenance deterministically for negative tests (those proving + * maintenance did NOT run, where there is no positive signal to poll on). Post-turn + * work is scheduled fire-and-forget: a single event-loop turn lets the handler run to + * its decision and register any compaction pass as a tracked post-prompt task, then + * `waitForIdle()` drains that task to completion. */ + async function drainMaintenance(): Promise { + await Bun.sleep(0); + await session.waitForIdle(); + } + + beforeAll(async () => { + sharedDir = TempDir.createSync("@pi-handoff-shared-"); + authStorage = await AuthStorage.create(path.join(sharedDir.path(), "testauth.db")); + authStorage.setRuntimeApiKey("anthropic", "test-key"); + modelRegistry = new ModelRegistry(authStorage); + + const bundled = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!bundled) { + throw new Error("Expected built-in anthropic model to exist"); + } + model = bundled; + }); + + afterAll(async () => { + authStorage.close(); + try { + await sharedDir.remove(); + } catch {} + }); + beforeEach(async () => { tempDir = TempDir.createSync("@pi-handoff-"); - authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); - authStorage.setRuntimeApiKey("anthropic", "test-key"); - modelRegistry = new ModelRegistry(authStorage); sessionManager = SessionManager.create(tempDir.path(), tempDir.path()); events = []; - const model = getBundledModel("anthropic", "claude-sonnet-4-5"); - if (!model) { - throw new Error("Expected built-in anthropic model to exist"); - } - const agent = new Agent({ initialState: { model, @@ -85,7 +126,6 @@ describe("AgentSession handoff", () => { if (session) { await session.dispose(); } - authStorage.close(); try { await tempDir.remove(); } catch {} @@ -97,7 +137,7 @@ describe("AgentSession handoff", () => { const generateHandoffSpy = vi.spyOn(compactionModule, "generateHandoff").mockResolvedValue(handoffText); const result = await session.handoff(); - await Bun.sleep(20); + await drainMaintenance(); expect(generateHandoffSpy).toHaveBeenCalledTimes(1); expect(result?.document).toBe(handoffText); @@ -123,7 +163,11 @@ describe("AgentSession handoff", () => { }); await session.prompt("pending prompt ".repeat(120)); - await Bun.sleep(20); + await waitFor( + () => + compactSpy.mock.calls.length === 1 && + events.some(event => event.type === "auto_compaction_end" && event.aborted === false), + ); expect(compactSpy).toHaveBeenCalledTimes(1); expect(promptSpy).toHaveBeenCalledTimes(1); @@ -177,7 +221,7 @@ describe("AgentSession handoff", () => { isError: false, }); session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMessage] }); - await Bun.sleep(20); + await drainMaintenance(); expect(handoffSpy).not.toHaveBeenCalled(); expect(events.filter(event => event.type === "auto_compaction_start")).toHaveLength(0); @@ -259,7 +303,7 @@ describe("AgentSession handoff", () => { const handoffSpy = vi.spyOn(session, "handoff"); session.agent.emitExternalEvent({ type: "message_end", message: assistantMessage }); session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMessage] }); - await Bun.sleep(20); + await drainMaintenance(); expect(handoffSpy).not.toHaveBeenCalled(); expect(events.filter(event => event.type === "auto_compaction_start")).toHaveLength(0); @@ -307,7 +351,7 @@ describe("AgentSession handoff", () => { session.agent.emitExternalEvent({ type: "message_end", message: overflowAssistant }); session.agent.emitExternalEvent({ type: "agent_end", messages: [overflowAssistant] }); - await Bun.sleep(20); + await waitFor(() => events.filter(event => event.type === "auto_compaction_end").length === 1); expect(handoffSpy).not.toHaveBeenCalled(); const startEvents = events.filter(event => event.type === "auto_compaction_start"); @@ -352,7 +396,11 @@ describe("AgentSession handoff", () => { session.agent.emitExternalEvent({ type: "message_end", message: assistantMessage }); session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMessage] }); - await Bun.sleep(20); + await waitFor( + () => + handoffSpy.mock.calls.length === 1 && + events.filter(event => event.type === "auto_compaction_end").length === 1, + ); expect(handoffSpy).toHaveBeenCalledTimes(1); expect(handoffSpy).toHaveBeenCalledWith(expect.stringContaining("Threshold-triggered maintenance"), { @@ -500,7 +548,8 @@ describe("AgentSession handoff", () => { session.agent.emitExternalEvent({ type: "message_end", message: assistantMessage }); session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMessage] }); - await Bun.sleep(20); + await waitFor(() => handoffSpy.mock.calls.length === 1); + await session.waitForIdle(); expect(handoffSpy).toHaveBeenCalledTimes(1); // The bug surfaced as agent.continue() racing the deferred handoff. With the fix, @@ -554,7 +603,7 @@ describe("AgentSession handoff", () => { session.agent.emitExternalEvent({ type: "message_end", message: assistantMessage }); session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMessage] }); // Let the deferred handoff post-prompt task enter the generateHandoff await. - await Bun.sleep(20); + await waitFor(() => session.isGeneratingHandoff); expect(generateHandoffSpy).toHaveBeenCalledTimes(1); expect(session.isGeneratingHandoff).toBe(true); @@ -601,7 +650,7 @@ describe("AgentSession handoff", () => { session.agent.emitExternalEvent({ type: "message_end", message: assistantMessage }); session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMessage] }); - await Bun.sleep(20); + await waitFor(() => events.filter(event => event.type === "auto_compaction_end").length === 1); expect(handoffSpy).toHaveBeenCalledTimes(1); const endEvents = events.filter(event => event.type === "auto_compaction_end"); diff --git a/packages/coding-agent/test/agent-session-model-persistence.test.ts b/packages/coding-agent/test/agent-session-model-persistence.test.ts index 7b741027a..76f089355 100644 --- a/packages/coding-agent/test/agent-session-model-persistence.test.ts +++ b/packages/coding-agent/test/agent-session-model-persistence.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test"; import * as path from "node:path"; import { Agent } from "@oh-my-pi/pi-agent-core"; import { type Api, Effort, getBundledModel, type Model } from "@oh-my-pi/pi-ai"; @@ -18,7 +18,24 @@ describe("AgentSession model persistence", () => { let tempDir: TempDir; let session: AgentSession | undefined; let sessionSettings: Settings; - const authStorages: AuthStorage[] = []; + // Auth storage (SQLite DB) and the model registry are immutable across these tests: + // every test sets the same anthropic runtime key and only ever reads the bundled model + // list. Building them once avoids ~12 SQLite opens + registry constructions. + let sharedDir: TempDir; + let sharedAuthStorage: AuthStorage; + let sharedModelRegistry: ModelRegistry; + + beforeAll(async () => { + sharedDir = TempDir.createSync("@pi-model-persistence-shared-"); + sharedAuthStorage = await AuthStorage.create(path.join(sharedDir.path(), "auth.db")); + sharedAuthStorage.setRuntimeApiKey("anthropic", "test-key"); + sharedModelRegistry = new ModelRegistry(sharedAuthStorage, path.join(sharedDir.path(), "models.yml")); + }); + + afterAll(() => { + sharedAuthStorage.close(); + sharedDir.removeSync(); + }); beforeEach(() => { tempDir = TempDir.createSync("@pi-model-persistence-"); @@ -29,9 +46,6 @@ describe("AgentSession model persistence", () => { await session.dispose(); session = undefined; } - for (const authStorage of authStorages.splice(0)) { - authStorage.close(); - } tempDir.removeSync(); }); @@ -84,13 +98,7 @@ describe("AgentSession model persistence", () => { modelRoles?: Record; persist?: boolean; }): Promise<{ modelRegistry: ModelRegistry; settings: Settings; session: AgentSession }> { - const authStorage = await AuthStorage.create(path.join(tempDir.path(), `testauth-${authStorages.length}.db`)); - authStorages.push(authStorage); - authStorage.setRuntimeApiKey("anthropic", "test-key"); - const modelRegistry = new ModelRegistry( - authStorage, - path.join(tempDir.path(), `models-${authStorages.length}.yml`), - ); + const modelRegistry = sharedModelRegistry; const model = options?.initialModel ?? options?.selectInitialModel?.(modelRegistry.getAvailable()) ?? @@ -118,7 +126,7 @@ describe("AgentSession model persistence", () => { session = new AgentSession({ agent, sessionManager: options?.persist - ? SessionManager.create(tempDir.path(), path.join(tempDir.path(), `active-${authStorages.length}`)) + ? SessionManager.create(tempDir.path(), path.join(tempDir.path(), "active")) : SessionManager.inMemory(), settings: sessionSettings, modelRegistry, @@ -131,19 +139,12 @@ describe("AgentSession model persistence", () => { targetSessionFile: string, settings: Settings = Settings.isolated(), ): Promise { - const authStorage = await AuthStorage.create(path.join(tempDir.path(), `testauth-${authStorages.length}.db`)); - authStorages.push(authStorage); - authStorage.setRuntimeApiKey("anthropic", "test-key"); - const modelRegistry = new ModelRegistry( - authStorage, - path.join(tempDir.path(), `models-${authStorages.length}.yml`), - ); const sessionManager = await SessionManager.open(targetSessionFile, path.join(tempDir.path(), "startup")); const result = await createAgentSession({ cwd: tempDir.path(), agentDir: tempDir.path(), - authStorage, - modelRegistry, + authStorage: sharedAuthStorage, + modelRegistry: sharedModelRegistry, sessionManager, settings, disableExtensionDiscovery: true, diff --git a/packages/coding-agent/test/agent-session-openai-responses-replay.test.ts b/packages/coding-agent/test/agent-session-openai-responses-replay.test.ts index 7fa2c98ed..fd5452369 100644 --- a/packages/coding-agent/test/agent-session-openai-responses-replay.test.ts +++ b/packages/coding-agent/test/agent-session-openai-responses-replay.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it, vi } from "bun:test"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; @@ -12,8 +12,11 @@ import type { Usage, } from "@oh-my-pi/pi-ai/types"; import { createOpenAIResponsesHistoryPayload } from "@oh-my-pi/pi-ai/utils"; +import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk"; import type { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; -import type { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; +import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { type SessionEntry, SessionManager, @@ -209,20 +212,21 @@ async function createPersistedSession( return { sessionFile, treeTargetId: result?.treeTargetId }; } +// ModelRegistry construction loads the bundled model catalog plus the on-disk +// cache (~100ms) and dominated this file's runtime when rebuilt once per test. +// The registry and its pinned AuthStorage are immutable across these tests (none +// mutate the catalog or stored credentials), so a single instance is shared via +// createAgentSession's `modelRegistry`/`authStorage` seam. The registry pins +// itself to its AuthStorage, so both MUST be the same shared instances. +let sharedModelRegistry: ModelRegistry; +let sharedRegistryDir: string; + async function createSessionHarness( tempDir: string, sessionManager: SessionManager, options: { provider?: Parameters[0]; modelId?: string } = {}, -): Promise<{ session: AgentSession; authStorage: AuthStorage }> { +): Promise<{ session: AgentSession }> { const { provider = "openai", modelId = "gpt-5-mini" } = options; - const [{ createAgentSession }, { Settings }, { AuthStorage }] = await Promise.all([ - import("@oh-my-pi/pi-coding-agent/sdk"), - import("@oh-my-pi/pi-coding-agent/config/settings"), - import("@oh-my-pi/pi-coding-agent/session/auth-storage"), - ]); - const authStorage = await AuthStorage.create(path.join(tempDir, `testauth-${Snowflake.next()}.db`)); - authStorage.setRuntimeApiKey("openai", "test-key"); - authStorage.setRuntimeApiKey("openai-codex", "test-key"); const model = getBundledModel(provider, modelId); if (!model) { throw new Error(`Expected bundled test model ${provider}/${modelId}`); @@ -231,7 +235,8 @@ async function createSessionHarness( const { session } = await createAgentSession({ cwd: tempDir, agentDir: tempDir, - authStorage, + authStorage: sharedModelRegistry.authStorage, + modelRegistry: sharedModelRegistry, sessionManager, model, settings: Settings.isolated(), @@ -242,23 +247,42 @@ async function createSessionHarness( slashCommands: [], enableMCP: false, enableLsp: false, + // These tests exercise session reload/sanitization/provider-state, never tool + // execution, rule resolution, or the workspace-tree render. A minimal tool set + // plus empty rules and a prebuilt (empty) workspace tree skip the per-call + // startup scans (native listWorkspace + rule capability discovery) without + // touching any asserted behavior. + rules: [], + workspaceTree: { rootPath: tempDir, rendered: "", truncated: false, totalLines: 0, agentsMdFiles: [] }, + toolNames: ["read"], }); - return { session, authStorage }; + return { session }; } describe("AgentSession OpenAI Responses replay boundaries", () => { const sessions: AgentSession[] = []; - const authStorages: AuthStorage[] = []; const tempDirs: string[] = []; + beforeAll(async () => { + sharedRegistryDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-issue-505-registry-${Snowflake.next()}-`)); + const authStorage = await AuthStorage.create(path.join(sharedRegistryDir, "auth.db")); + authStorage.setRuntimeApiKey("openai", "test-key"); + authStorage.setRuntimeApiKey("openai-codex", "test-key"); + sharedModelRegistry = new ModelRegistry(authStorage); + }); + + afterAll(() => { + sharedModelRegistry?.authStorage.close(); + if (sharedRegistryDir && fs.existsSync(sharedRegistryDir)) { + fs.rmSync(sharedRegistryDir, { recursive: true, force: true }); + } + }); + afterEach(async () => { while (sessions.length > 0) { await sessions.pop()?.dispose(); } - while (authStorages.length > 0) { - authStorages.pop()?.close(); - } while (tempDirs.length > 0) { const tempDir = tempDirs.pop(); if (tempDir && fs.existsSync(tempDir)) { @@ -285,9 +309,8 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { }); const reloadedSessionManager = await SessionManager.open(sessionFile, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, reloadedSessionManager); + const { session } = await createSessionHarness(tempDir, reloadedSessionManager); sessions.push(session); - authStorages.push(authStorage); const persistedUser = findPersistedMessageEntry(session.sessionManager, "user", "Preserved summary").message; if (persistedUser.role !== "user") { @@ -383,12 +406,11 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { }); const reloadedSessionManager = await SessionManager.open(sessionFile, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, reloadedSessionManager, { + const { session } = await createSessionHarness(tempDir, reloadedSessionManager, { provider: "openai-codex", modelId: "gpt-5.2-codex", }); sessions.push(session); - authStorages.push(authStorage); const closeSpy = vi.fn(); session.providerSessionState.set("openai-codex-responses", { close: closeSpy } satisfies ProviderSessionState); @@ -421,12 +443,11 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { }); const reloadedSessionManager = await SessionManager.open(sessionFile, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, reloadedSessionManager, { + const { session } = await createSessionHarness(tempDir, reloadedSessionManager, { provider: "openai-codex", modelId: "gpt-5.2-codex", }); sessions.push(session); - authStorages.push(authStorage); const closeSpy = vi.fn(); session.providerSessionState.set("openai-codex-responses", { close: closeSpy } satisfies ProviderSessionState); @@ -476,9 +497,8 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-issue-505-reload-proxy-${Snowflake.next()}-`)); tempDirs.push(tempDir); const sessionManager = SessionManager.create(tempDir, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, sessionManager); + const { session } = await createSessionHarness(tempDir, sessionManager); sessions.push(session); - authStorages.push(authStorage); const proxyDetails = new Proxy({ ok: true, nested: { value: "preserved" } }, {}); await session.sendCustomMessage( @@ -514,12 +534,11 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { }); const reloadedSessionManager = await SessionManager.open(sessionFile, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, reloadedSessionManager, { + const { session } = await createSessionHarness(tempDir, reloadedSessionManager, { provider: "openai-codex", modelId: "gpt-5.2-codex", }); sessions.push(session); - authStorages.push(authStorage); const closeSpy = vi.fn(); session.providerSessionState.set("openai-codex-responses", { close: closeSpy } satisfies ProviderSessionState); @@ -561,12 +580,11 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { }); const reloadedSessionManager = await SessionManager.open(sessionFile, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, reloadedSessionManager, { + const { session } = await createSessionHarness(tempDir, reloadedSessionManager, { provider: "openai-codex", modelId: "gpt-5.2-codex", }); sessions.push(session); - authStorages.push(authStorage); const closeSpy = vi.fn(); session.providerSessionState.set("openai-codex-responses", { close: closeSpy } satisfies ProviderSessionState); @@ -597,9 +615,8 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { }); const reloadedSessionManager = await SessionManager.open(sessionFile, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, reloadedSessionManager); + const { session } = await createSessionHarness(tempDir, reloadedSessionManager); sessions.push(session); - authStorages.push(authStorage); const closeSpy = vi.fn(); session.providerSessionState.set("openai-responses:openai", { close: closeSpy } satisfies ProviderSessionState); @@ -623,9 +640,8 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-issue-505-switch-fail-${Snowflake.next()}-`)); tempDirs.push(tempDir); const currentSessionManager = SessionManager.create(tempDir, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, currentSessionManager); + const { session } = await createSessionHarness(tempDir, currentSessionManager); sessions.push(session); - authStorages.push(authStorage); const { sessionFile } = await createPersistedSession(tempDir, sessionManager => { appendStaleAssistantTurn(sessionManager, "Unreadable assistant snapshot"); @@ -657,9 +673,8 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { const assistantText = "Switched assistant response"; const currentSessionManager = SessionManager.create(tempDir, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, currentSessionManager); + const { session } = await createSessionHarness(tempDir, currentSessionManager); sessions.push(session); - authStorages.push(authStorage); const { sessionFile } = await createPersistedSession(tempDir, sessionManager => { sessionManager.appendMessage({ @@ -723,9 +738,8 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { } const reloadedSessionManager = await SessionManager.open(sessionFile, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, reloadedSessionManager); + const { session } = await createSessionHarness(tempDir, reloadedSessionManager); sessions.push(session); - authStorages.push(authStorage); const navigation = await session.navigateTree(treeTargetId, { summarize: false }); expect(navigation.cancelled).toBe(false); @@ -746,9 +760,8 @@ describe("AgentSession OpenAI Responses replay boundaries", () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-issue-505-new-${Snowflake.next()}-`)); tempDirs.push(tempDir); const sessionManager = SessionManager.create(tempDir, tempDir); - const { session, authStorage } = await createSessionHarness(tempDir, sessionManager); + const { session } = await createSessionHarness(tempDir, sessionManager); sessions.push(session); - authStorages.push(authStorage); const closeSpy = vi.fn(); session.providerSessionState.set("live-provider-session", { close: closeSpy } satisfies ProviderSessionState); diff --git a/packages/coding-agent/test/agent-session-python-cleanup.test.ts b/packages/coding-agent/test/agent-session-python-cleanup.test.ts index 478fc5fb7..c97f93ec7 100644 --- a/packages/coding-agent/test/agent-session-python-cleanup.test.ts +++ b/packages/coding-agent/test/agent-session-python-cleanup.test.ts @@ -116,6 +116,7 @@ const createSession = async ( disableExtensionDiscovery: true, extensions: options.extensions, skills: [], + rules: [], contextFiles: [], promptTemplates: [], workspaceTree: emptyWorkspaceTree(cwd), @@ -195,6 +196,7 @@ describe("AgentSession python cleanup", () => { disableExtensionDiscovery: true, extensions: [throwingExtension], skills: [], + rules: [], contextFiles: [], promptTemplates: [], slashCommands: [], @@ -261,6 +263,7 @@ describe("AgentSession python cleanup", () => { model: getModel(), disableExtensionDiscovery: true, skills: [], + rules: [], contextFiles: [], promptTemplates: [], slashCommands: [], diff --git a/packages/coding-agent/test/agent-session-retry-fallback.test.ts b/packages/coding-agent/test/agent-session-retry-fallback.test.ts index c9dc6d0ff..54c852f5f 100644 --- a/packages/coding-agent/test/agent-session-retry-fallback.test.ts +++ b/packages/coding-agent/test/agent-session-retry-fallback.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; import * as path from "node:path"; import { scheduler } from "node:timers/promises"; import { Agent } from "@oh-my-pi/pi-agent-core"; @@ -66,16 +66,34 @@ function createFallbackAgent(primaryModel: Model, requestedModels: string[]): Ag describe("AgentSession retry fallback", () => { let tempDir: TempDir; let authStorage: AuthStorage; + let sharedRegistry: ModelRegistry; let modelRegistry: ModelRegistry; let session: AgentSession | undefined; - beforeEach(async () => { + // The model registry is an immutable fixture whose construction builds a + // canonical index over ~2.7k bundled models (~100ms). Build it (and the + // auth DB) once for the whole file instead of per-test; reset only the + // mutable retry-fallback cooldown state between tests. + beforeAll(async () => { tempDir = TempDir.createSync("@pi-retry-fallback-"); authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); authStorage.setRuntimeApiKey("anthropic", "anthropic-test-key"); authStorage.setRuntimeApiKey("openai", "openai-test-key"); authStorage.setRuntimeApiKey("google", "google-test-key"); - modelRegistry = new ModelRegistry(authStorage); + sharedRegistry = new ModelRegistry(authStorage); + }); + + afterAll(() => { + authStorage.close(); + tempDir.removeSync(); + }); + + beforeEach(() => { + // Reset to the shared registry (a few tests reassign it to a scoped + // instance) and clear cooldown suppressions left by fallback-path tests + // (default 5-minute suppression) so state never leaks between tests. + modelRegistry = sharedRegistry; + modelRegistry.clearSuppressedSelectors(); }); afterEach(async () => { @@ -83,8 +101,6 @@ describe("AgentSession retry fallback", () => { await session.dispose(); session = undefined; } - authStorage.close(); - tempDir.removeSync(); vi.restoreAllMocks(); }); diff --git a/packages/coding-agent/test/autoresearch-tools.test.ts b/packages/coding-agent/test/autoresearch-tools.test.ts index 3b2c173f0..8dbe03a28 100644 --- a/packages/coding-agent/test/autoresearch-tools.test.ts +++ b/packages/coding-agent/test/autoresearch-tools.test.ts @@ -68,16 +68,16 @@ function createPiHarness(initialTools: string[] = []): PiHarness { return { api, activeTools, appendEntries, setActiveToolsCalls }; } -async function initGitRepo(dir: string): Promise<{ baselineCommit: string; mainBranch: string }> { - await $`git init --initial-branch=main`.cwd(dir).quiet(); - await $`git config user.email tester@example.com`.cwd(dir).quiet(); - await $`git config user.name Tester`.cwd(dir).quiet(); +async function initGitRepo(dir: string): Promise<{ baselineCommit: string }> { await Bun.write(path.join(dir, "README.md"), "# baseline\n"); - await $`git add -A`.cwd(dir).quiet(); - await $`git commit -m baseline`.cwd(dir).quiet(); + // One shell invocation instead of five: the git processes are unavoidable + // (config identity is read by the production tool's own commits), but + // chaining collapses the per-call Node↔shell spawn overhead. + await $`git init --initial-branch=main && git config user.email tester@example.com && git config user.name Tester && git add -A && git commit -m baseline` + .cwd(dir) + .quiet(); const sha = (await $`git rev-parse HEAD`.cwd(dir).text()).trim(); - const branch = (await $`git rev-parse --abbrev-ref HEAD`.cwd(dir).text()).trim(); - return { baselineCommit: sha, mainBranch: branch }; + return { baselineCommit: sha }; } async function checkoutBranch(dir: string, name: string): Promise { @@ -585,8 +585,7 @@ describe("log_experiment", () => { // Commit `src/edit-me.ts` to baseline so it is tracked, not in pre-run dirty paths. fs.mkdirSync(path.join(dir, "src"), { recursive: true }); await Bun.write(path.join(dir, "src", "edit-me.ts"), "export const v = 1;\n"); - await $`git add -A`.cwd(dir).quiet(); - await $`git commit -m seed`.cwd(dir).quiet(); + await $`git add -A && git commit -m seed`.cwd(dir).quiet(); const runtime = createSessionRuntime(); const harness = createPiHarness(); const init = createInitExperimentTool({ @@ -638,8 +637,7 @@ describe("log_experiment", () => { await initGitRepo(dir); // Commit the harness on main so it is part of the autoresearch branch's baseline. await writeHarnessStub(dir); - await $`git add -A`.cwd(dir).quiet(); - await $`git commit -m harness`.cwd(dir).quiet(); + await $`git add -A && git commit -m harness`.cwd(dir).quiet(); await checkoutBranch(dir, "autoresearch/test-20260501"); const runtime = createSessionRuntime(); const harness = createPiHarness(); @@ -651,8 +649,7 @@ describe("log_experiment", () => { await init.execute("i", { name: "x", primary_metric: "m" }, undefined, undefined, createCtx(dir)); // Simulate a previously kept iteration by committing it directly on the branch. await Bun.write(path.join(dir, "src", "kept.ts"), "export const v = 1;\n"); - await $`git add -A`.cwd(dir).quiet(); - await $`git commit -m "kept iteration"`.cwd(dir).quiet(); + await $`git add -A && git commit -m "kept iteration"`.cwd(dir).quiet(); const headBeforeDiscard = (await $`git rev-parse HEAD`.cwd(dir).text()).trim(); const run = createRunExperimentTool({ @@ -691,13 +688,11 @@ describe("log_experiment", () => { const dir = makeTempDir(); await initGitRepo(dir); await writeHarnessStub(dir); - await $`git add -A`.cwd(dir).quiet(); - await $`git commit -m harness`.cwd(dir).quiet(); + await $`git add -A && git commit -m harness`.cwd(dir).quiet(); // Seed a tracked file that the agent will edit during the iteration. fs.mkdirSync(path.join(dir, "src"), { recursive: true }); await Bun.write(path.join(dir, "src", "store.ts"), "export const v = 1;\n"); - await $`git add -A`.cwd(dir).quiet(); - await $`git commit -m seed`.cwd(dir).quiet(); + await $`git add -A && git commit -m seed`.cwd(dir).quiet(); await checkoutBranch(dir, "autoresearch/keep-test"); const runtime = createSessionRuntime(); const harness = createPiHarness(); @@ -747,8 +742,7 @@ describe("log_experiment", () => { const dir = makeTempDir(); await initGitRepo(dir); await writeHarnessStub(dir); - await $`git add -A`.cwd(dir).quiet(); - await $`git commit -m harness`.cwd(dir).quiet(); + await $`git add -A && git commit -m harness`.cwd(dir).quiet(); await checkoutBranch(dir, "autoresearch/scope-test"); const runtime = createSessionRuntime(); const harness = createPiHarness(); diff --git a/packages/coding-agent/test/bash-executor.test.ts b/packages/coding-agent/test/bash-executor.test.ts index 96d92f5b8..3ff685827 100644 --- a/packages/coding-agent/test/bash-executor.test.ts +++ b/packages/coding-agent/test/bash-executor.test.ts @@ -14,12 +14,27 @@ import * as piNatives from "@oh-my-pi/pi-natives"; const ARTIFACT_HEAD_BYTES_DEFAULT = 20 * 1024; const BACKGROUND_COMPLETION_RACE_MS = 750; const KILL_MARKER_DELAY_SECONDS = "0.4"; -const KILL_MARKER_ASSERTION_WAIT_MS = 900; +const KILL_MARKER_DELAY_MS = 400; +// We prove a killed process never wrote its marker by observing until the +// wall-clock instant the marker WOULD have appeared (spawn + delay) plus a +// margin. Anchoring the deadline to a pre-spawn timestamp — instead of blindly +// sleeping a fixed amount after executeBash returns — keeps the wait bounded +// without shrinking the kill-propagation margin: the timeout/abort fires at +// ~100ms, well before the 400ms marker write, so the margin between kill and +// write is unchanged; only the redundant observation tail goes away. +const KILL_MARKER_OBSERVE_MARGIN_MS = 300; function makeTempDir(): string { return fs.mkdtempSync(path.join(os.tmpdir(), "omp-bash-exec-")); } +/** Spin-wait until the wall-clock deadline, polling rather than blind-sleeping. */ +async function waitUntil(deadlineMs: number): Promise { + while (Date.now() < deadlineMs) { + await Bun.sleep(20); + } +} + describe("executeBash", () => { let tempDir: string; @@ -532,6 +547,7 @@ describe("executeBash", () => { const markerEscaped = marker.replace(/'/g, "'\\''"); // Command creates marker after a short delay, but we timeout before then. + const start = Date.now(); const result = await executeBash(`sleep ${KILL_MARKER_DELAY_SECONDS} && echo done > '${markerEscaped}'`, { cwd: tempDir, timeout: 100, @@ -539,10 +555,9 @@ describe("executeBash", () => { expect(result.cancelled).toBe(true); - // Wait longer than the command would have needed to create the marker. - await Bun.sleep(KILL_MARKER_ASSERTION_WAIT_MS); - - // If process was killed (not orphaned), marker should NOT exist + // Observe past the instant the marker would have been written had the + // process survived. If it was killed (not orphaned), it never appears. + await waitUntil(start + KILL_MARKER_DELAY_MS + KILL_MARKER_OBSERVE_MARGIN_MS); expect(fs.existsSync(marker)).toBe(false); }); @@ -552,6 +567,7 @@ describe("executeBash", () => { const marker = path.join(tempDir, "marker-bg.txt"); const markerEscaped = marker.replace(/'/g, "'\\''"); + const start = Date.now(); const result = await executeBash( `{ sleep ${KILL_MARKER_DELAY_SECONDS}; echo done > '${markerEscaped}'; } & sleep 10`, { @@ -562,7 +578,7 @@ describe("executeBash", () => { expect(result.cancelled).toBe(true); - await Bun.sleep(KILL_MARKER_ASSERTION_WAIT_MS); + await waitUntil(start + KILL_MARKER_DELAY_MS + KILL_MARKER_OBSERVE_MARGIN_MS); expect(fs.existsSync(marker)).toBe(false); }); @@ -597,9 +613,13 @@ describe("executeBash", () => { expect(result.cancelled).toBe(true); expect(result.output).toContain("Command cancelled"); - await Bun.sleep(KILL_MARKER_ASSERTION_WAIT_MS); + // The backgrounded subshell only writes its marker once `release` exists. + // If abort failed to kill the process group, the orphan is still polling + // for `release` every 50ms — touching it makes a survivor react within one + // poll. A short settle first lets the kill signal propagate before we probe. + await Bun.sleep(100); fs.writeFileSync(release, ""); - await Bun.sleep(150); + await Bun.sleep(200); expect(fs.existsSync(marker)).toBe(false); }); @@ -611,6 +631,7 @@ describe("executeBash", () => { const controller = new AbortController(); // Command creates marker after a short delay. + const start = Date.now(); const promise = executeBash(`sleep ${KILL_MARKER_DELAY_SECONDS} && echo done > '${markerEscaped}'`, { cwd: tempDir, timeout: 10000, @@ -625,10 +646,9 @@ describe("executeBash", () => { expect(result.cancelled).toBe(true); expect(result.output).toContain("Command cancelled"); - // Wait longer than the command would have needed to create the marker. - await Bun.sleep(KILL_MARKER_ASSERTION_WAIT_MS); - - // If process was killed (not orphaned), marker should NOT exist + // Observe past the instant the marker would have been written had the + // process survived. If it was killed (not orphaned), it never appears. + await waitUntil(start + KILL_MARKER_DELAY_MS + KILL_MARKER_OBSERVE_MARGIN_MS); expect(fs.existsSync(marker)).toBe(false); }); }); diff --git a/packages/coding-agent/test/extensions-runner.test.ts b/packages/coding-agent/test/extensions-runner.test.ts index 33e188b7f..7e0005a43 100644 --- a/packages/coding-agent/test/extensions-runner.test.ts +++ b/packages/coding-agent/test/extensions-runner.test.ts @@ -2,7 +2,7 @@ * Tests for ExtensionRunner - conflict detection, error handling, tool wrapping. */ -import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs"; import * as path from "node:path"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; @@ -20,21 +20,34 @@ describe("ExtensionRunner", () => { let tempDir: TempDir; let extensionsDir: string; let sessionManager: SessionManager; + // Shared immutable fixtures. ModelRegistry's constructor synchronously loads + // every bundled model and rebuilds the canonical index (~100ms); these tests + // never mutate the registry or auth storage, so build them once per file + // instead of paying that cost in every beforeEach. + let sharedTempDir: TempDir; let modelRegistry: ModelRegistry; let authStorage: AuthStorage; - beforeEach(async () => { + beforeAll(async () => { + sharedTempDir = TempDir.createSync("@pi-runner-shared-"); + authStorage = await AuthStorage.create(path.join(sharedTempDir.path(), "testauth.db")); + modelRegistry = new ModelRegistry(authStorage); + }); + + afterAll(() => { + authStorage.close(); + sharedTempDir.removeSync(); + }); + + beforeEach(() => { tempDir = TempDir.createSync("@pi-runner-test-"); extensionsDir = path.join(getProjectAgentDir(tempDir.path()), "extensions"); fs.mkdirSync(extensionsDir, { recursive: true }); sessionManager = SessionManager.inMemory(); - authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); - modelRegistry = new ModelRegistry(authStorage); }); afterEach(() => { testSetExtensionHandlerTimeoutMs(EXTENSION_HANDLER_TIMEOUT_MS); - authStorage.close(); tempDir.removeSync(); }); diff --git a/packages/coding-agent/test/goals/goal-mode-integration.test.ts b/packages/coding-agent/test/goals/goal-mode-integration.test.ts index 4eafffe64..83d34f4cc 100644 --- a/packages/coding-agent/test/goals/goal-mode-integration.test.ts +++ b/packages/coding-agent/test/goals/goal-mode-integration.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; import * as path from "node:path"; import { Agent } from "@oh-my-pi/pi-agent-core"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; @@ -25,7 +25,6 @@ function createToolSession(cwd: string, settings: Settings, overrides: Partial Promise; }; -async function createGoalHarness(): Promise { - resetSettingsForTest(); - const tempDir = TempDir.createSync("@pi-goal-mode-"); - await Settings.init({ inMemory: true, cwd: tempDir.path() }); - const authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); +// Immutable, expensive fixtures shared across every test. `new ModelRegistry` +// alone is ~110ms (loads + parses the bundled model catalog), which dominated +// this file's wall time when rebuilt per test. The registry, its auth storage, +// and the resolved model are never mutated by goal-mode flows, and +// AgentSession.dispose() never closes authStorage — so a single shared instance +// is safe and drops ~8×110ms of pure setup overhead. +type SharedFixture = { + authStorage: AuthStorage; + modelRegistry: ModelRegistry; + model: NonNullable>; + baseDir: TempDir; +}; + +async function createSharedFixture(): Promise { + const baseDir = TempDir.createSync("@pi-goal-mode-shared-"); + const authStorage = await AuthStorage.create(path.join(baseDir.path(), "testauth.db")); const modelRegistry = new ModelRegistry(authStorage); const model = modelRegistry.find("anthropic", "claude-sonnet-4-5"); if (!model) { throw new Error("Expected claude-sonnet-4-5 to exist in registry"); } + return { authStorage, modelRegistry, model, baseDir }; +} + +async function createGoalHarness(shared: SharedFixture): Promise { + resetSettingsForTest(); + const tempDir = TempDir.createSync("@pi-goal-mode-"); + await Settings.init({ inMemory: true, cwd: tempDir.path() }); + const { modelRegistry, model } = shared; const settings = Settings.isolated({ "compaction.enabled": false, @@ -77,7 +95,6 @@ async function createGoalHarness(): Promise { return { tempDir, - authStorage, settings, session, mode, @@ -85,7 +102,6 @@ async function createGoalHarness(): Promise { cleanup: async () => { mode.stop(); await session.dispose(); - authStorage.close(); tempDir.removeSync(); resetSettingsForTest(); }, @@ -98,13 +114,20 @@ async function toolNamesFor(harness: GoalHarness): Promise { describe("InteractiveMode goal mode integration", () => { let harness: GoalHarness; + let shared: SharedFixture; - beforeAll(() => { + beforeAll(async () => { initTheme(); + shared = await createSharedFixture(); + }); + + afterAll(() => { + shared.authStorage.close(); + shared.baseDir.removeSync(); }); beforeEach(async () => { - harness = await createGoalHarness(); + harness = await createGoalHarness(shared); }); afterEach(async () => { 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 6fb7b28eb..85ae3f784 100644 --- a/packages/coding-agent/test/interactive-mode-plan-review.test.ts +++ b/packages/coding-agent/test/interactive-mode-plan-review.test.ts @@ -68,7 +68,6 @@ describe("InteractiveMode plan review rendering", () => { }); beforeEach(async () => { - Bun.gc(true); resetSettingsForTest(); tempDir = TempDir.createSync("@pi-plan-review-"); await Settings.init({ inMemory: true, cwd: tempDir.path() }); @@ -111,7 +110,6 @@ describe("InteractiveMode plan review rendering", () => { currentAuthStorage?.close(); currentTempDir?.removeSync(); resetSettingsForTest(); - Bun.gc(true); }); it("appends each submitted plan review preview to preserve scrollback", async () => { diff --git a/packages/coding-agent/test/keybindings-selector-navigation.test.ts b/packages/coding-agent/test/keybindings-selector-navigation.test.ts index e79557af5..78dd76e76 100644 --- a/packages/coding-agent/test/keybindings-selector-navigation.test.ts +++ b/packages/coding-agent/test/keybindings-selector-navigation.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeAll, describe, expect, it } from "bun:test"; +import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -86,8 +86,15 @@ async function createHistoryStorage(prompts: string[]): Promise tempDirs.push(dir); HistoryStorage.resetInstance(); const storage = HistoryStorage.open(path.join(dir, "history.db")); - for (const prompt of prompts) { - await storage.add(prompt); + // add() batches writes behind a 100ms AsyncDrain timer. Drive that timer with + // fake timers so the flush is instant instead of waiting real wall-clock time. + vi.useFakeTimers(); + try { + const writes = prompts.map(prompt => storage.add(prompt)); + vi.advanceTimersByTime(100); + await Promise.all(writes); + } finally { + vi.useRealTimers(); } return storage; } diff --git a/packages/coding-agent/test/mcp-reconnect-storm.test.ts b/packages/coding-agent/test/mcp-reconnect-storm.test.ts index 6f75e8955..87a5ef49a 100644 --- a/packages/coding-agent/test/mcp-reconnect-storm.test.ts +++ b/packages/coding-agent/test/mcp-reconnect-storm.test.ts @@ -52,10 +52,19 @@ describe("MCP reconnect storm (issue #1592)", () => { try { await manager.connectServers({ crashy: config }, {}); - // Give the reconnect loop generous time to fire. With the bug this - // produced thousands of processes within a second; with the fix the - // circuit breaker caps the per-server spawn budget. - await Bun.sleep(3000); + // Wait for the circuit breaker to trip rather than blind-sleeping a + // fixed budget. During the storm `getConnectionStatus` is always + // "connected" or "connecting" (`#pendingReconnections` is set + // synchronously before any await in `#doReconnect`); it only reports + // "disconnected" once `#tripReconnectBreaker` opens, tears down the + // stale connection, and detaches `onClose` so no further spawns fire. + // That makes the terminal state a race-free signal: poll for it and + // return the instant the storm is capped instead of waiting out a + // fixed 3s. Generous deadline stays well under the 15s test timeout. + const deadline = Date.now() + 10_000; + while (manager.getConnectionStatus("crashy") !== "disconnected" && Date.now() < deadline) { + await Bun.sleep(5); + } const spawns = countSpawns(); // `RECONNECT_BURST_LIMIT` (5) is the per-server reconnect cap inside diff --git a/packages/coding-agent/test/model-registry-runtime-provider.test.ts b/packages/coding-agent/test/model-registry-runtime-provider.test.ts index 9538a0e34..30e40efbb 100644 --- a/packages/coding-agent/test/model-registry-runtime-provider.test.ts +++ b/packages/coding-agent/test/model-registry-runtime-provider.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, type Mock, spyOn, test } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; @@ -13,6 +13,9 @@ describe("ModelRegistry runtime provider registration", () => { let tempDir: string; let modelsJsonPath: string; let authStorage: AuthStorage; + // Neutralizes real network egress during "online" refresh tests so the merge + // path runs without wall-clock-bound DNS/socket latency. Restored in afterEach. + let fetchSpy: Mock | undefined; const sourceIds = ["ext://atomic", "ext://runtime", "ext://oauth"]; @@ -24,6 +27,8 @@ describe("ModelRegistry runtime provider registration", () => { }); afterEach(() => { + fetchSpy?.mockRestore(); + fetchSpy = undefined; clearCustomApis(); for (const sourceId of sourceIds) { unregisterOAuthProviders(sourceId); @@ -192,6 +197,11 @@ describe("ModelRegistry runtime provider registration", () => { }); test("extension-registered models survive refresh('online') cycle", async () => { + // The contract is overlay survival through the full online refresh path + // (static reload + discovery + merge), not discovery success. Stub fetch so + // the online branch runs identically to production-with-no-reachable-providers + // without paying real network latency (~400ms of DNS/socket time otherwise). + fetchSpy = spyOn(globalThis, "fetch").mockRejectedValue(new Error("network disabled in test")); const registry = new ModelRegistry(authStorage, modelsJsonPath); const config: ProviderConfigInput = { baseUrl: "https://runtime.example.com/v1", diff --git a/packages/coding-agent/test/model-registry.test.ts b/packages/coding-agent/test/model-registry.test.ts index 0cdb092ca..218d98a22 100644 --- a/packages/coding-agent/test/model-registry.test.ts +++ b/packages/coding-agent/test/model-registry.test.ts @@ -29,7 +29,10 @@ describe("ModelRegistry", () => { fs.mkdirSync(tempDir, { recursive: true }); modelsJsonPath = path.join(tempDir, "models.json"); cacheDbPath = path.join(tempDir, "models.db"); - authStorage = await AuthStorage.create(path.join(tempDir, "testauth.db")); + // In-memory auth DB: tests need a fresh, isolated credential store per case but + // never reopen it from disk, so :memory: avoids the WAL/chmod disk-open cost + // (~3ms/test) while preserving per-test isolation. + authStorage = await AuthStorage.create(":memory:"); }); afterEach(() => { diff --git a/packages/coding-agent/test/modes/components/transcript-container.test.ts b/packages/coding-agent/test/modes/components/transcript-container.test.ts index 6baf1bfac..a1b385eae 100644 --- a/packages/coding-agent/test/modes/components/transcript-container.test.ts +++ b/packages/coding-agent/test/modes/components/transcript-container.test.ts @@ -173,6 +173,25 @@ describe("TranscriptContainer", () => { expect(container.render(40)).toEqual(["a-final", "b1"]); // reconciled }); + it("invalidate() retires frozen snapshots so resetDisplay reflects current state", () => { + // resetDisplay() (Ctrl+L, and the Ctrl+O expand path) reflows by calling + // TUI.invalidate(), which propagates to this container. That must retire the + // frozen snapshots the same way thaw() does, or a forced full replay would + // still emit the pre-mutation (e.g. collapsed) render. + riskFlag.eagerEraseScrollbackRisk = true; + const container = new TranscriptContainer(); + const a = new MutableBlock(["a-collapsed"]); + const b = new MutableBlock(["b1"]); + container.addChild(a); + container.addChild(b); + container.render(40); + a.set(["a-expanded-1", "a-expanded-2"]); + expect(container.render(40)).toEqual(["a-collapsed", "b1"]); // frozen + + container.invalidate(); + expect(container.render(40)).toEqual(["a-expanded-1", "a-expanded-2", "b1"]); + }); + it("recomputes a frozen block on a width change", () => { riskFlag.eagerEraseScrollbackRisk = true; const container = new TranscriptContainer(); diff --git a/packages/coding-agent/test/modes/controllers/input-controller-tool-expansion.test.ts b/packages/coding-agent/test/modes/controllers/input-controller-tool-expansion.test.ts index bcf3900ea..38370dca3 100644 --- a/packages/coding-agent/test/modes/controllers/input-controller-tool-expansion.test.ts +++ b/packages/coding-agent/test/modes/controllers/input-controller-tool-expansion.test.ts @@ -3,20 +3,25 @@ import { InputController } from "../../../src/modes/controllers/input-controller import type { InteractiveModeContext } from "../../../src/modes/types"; describe("InputController tool output expansion", () => { - it("allows unknown viewport mutation when toggling tool output expansion", () => { + it("expands children and forces a full display reset to bypass frozen snapshots", () => { const expandable = { setExpanded: vi.fn() }; const inert = { render: vi.fn(() => []) }; const requestRender = vi.fn(); + const resetDisplay = vi.fn(); const ctx = { toolOutputExpanded: false, chatContainer: { children: [expandable, inert] }, - ui: { requestRender }, + ui: { requestRender, resetDisplay }, } as unknown as InteractiveModeContext; new InputController(ctx).toggleToolOutputExpansion(); expect(ctx.toolOutputExpanded).toBe(true); expect(expandable.setExpanded).toHaveBeenCalledWith(true); - expect(requestRender).toHaveBeenCalledWith(false, { allowUnknownViewportMutation: true }); + // resetDisplay() is the only path that retires the transcript's frozen + // block snapshots and re-emits the whole transcript at its new heights. + // A plain requestRender would replay the stale (collapsed) snapshots. + expect(resetDisplay).toHaveBeenCalledTimes(1); + expect(requestRender).not.toHaveBeenCalled(); }); }); diff --git a/packages/coding-agent/test/plan-mode-thinking-level.test.ts b/packages/coding-agent/test/plan-mode-thinking-level.test.ts index b5eee3bf3..548593336 100644 --- a/packages/coding-agent/test/plan-mode-thinking-level.test.ts +++ b/packages/coding-agent/test/plan-mode-thinking-level.test.ts @@ -6,7 +6,7 @@ * calls resolveModelRoleValue() but only returns .model, dropping the thinking level. * #applyPlanModeModel() therefore has no thinking level to apply. */ -import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "bun:test"; import * as path from "node:path"; import { Agent, ThinkingLevel } from "@oh-my-pi/pi-agent-core"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; @@ -22,7 +22,7 @@ describe("plan mode thinking level", () => { let modelRegistry: ModelRegistry; let authStorage: AuthStorage; - beforeEach(async () => { + beforeAll(async () => { tempDir = TempDir.createSync("@pi-plan-thinking-"); authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); authStorage.setRuntimeApiKey("anthropic", "test-key"); @@ -33,6 +33,9 @@ describe("plan mode thinking level", () => { if (session) { await session.dispose(); } + }); + + afterAll(() => { authStorage.close(); tempDir.removeSync(); }); diff --git a/packages/coding-agent/test/sdk-async-job-manager-singleton.test.ts b/packages/coding-agent/test/sdk-async-job-manager-singleton.test.ts index eee3e1da3..903c00722 100644 --- a/packages/coding-agent/test/sdk-async-job-manager-singleton.test.ts +++ b/packages/coding-agent/test/sdk-async-job-manager-singleton.test.ts @@ -1,14 +1,35 @@ -import { afterEach, describe, expect, it } from "bun:test"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import { AsyncJobManager } from "@oh-my-pi/pi-coding-agent/async/job-manager"; +import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk"; +import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { Snowflake } from "@oh-my-pi/pi-utils"; describe("AsyncJobManager singleton across concurrent top-level sessions", () => { const tempDirs: string[] = []; + // Building a ModelRegistry per session is the dominant cost here: createAgentSession + // otherwise runs discoverAuthStorage (a fresh AuthStorage DB create+reload) and a + // background online model refresh for every spawn (~450ms each). The singleton + // ownership behavior under test is independent of model resolution, so we hand every + // session one shared, network-free registry built once (~10ms/session instead). + let sharedTempDir: string; + let sharedAuthStorage: AuthStorage; + let sharedModelRegistry: ModelRegistry; + + beforeAll(async () => { + sharedTempDir = fs.mkdtempSync(path.join(os.tmpdir(), "pi-sdk-async-singleton-shared-")); + sharedAuthStorage = await AuthStorage.create(path.join(sharedTempDir, "auth.db")); + sharedModelRegistry = new ModelRegistry(sharedAuthStorage, path.join(sharedTempDir, "models.yml")); + }); + + afterAll(() => { + sharedAuthStorage.close(); + fs.rmSync(sharedTempDir, { recursive: true, force: true }); + }); afterEach(async () => { for (const tempDir of tempDirs.splice(0)) { @@ -34,6 +55,7 @@ describe("AsyncJobManager singleton across concurrent top-level sessions", () => slashCommands: [], enableMCP: false, enableLsp: false, + modelRegistry: sharedModelRegistry, }); return session; } @@ -153,6 +175,7 @@ describe("AsyncJobManager singleton across concurrent top-level sessions", () => slashCommands: [], enableMCP: false, enableLsp: false, + modelRegistry: sharedModelRegistry, systemPrompt: () => { throw new Error("forced startup failure"); }, diff --git a/packages/coding-agent/test/sdk-credential-disabled-bridge.test.ts b/packages/coding-agent/test/sdk-credential-disabled-bridge.test.ts index 0001a780a..a01522c99 100644 --- a/packages/coding-agent/test/sdk-credential-disabled-bridge.test.ts +++ b/packages/coding-agent/test/sdk-credential-disabled-bridge.test.ts @@ -93,10 +93,17 @@ describe("createAgentSession credential_disabled subscription", () => { cwd: dirs.cwd, agentDir: dirs.agentDir, authStorage, + // Pin the model registry at a temp models.json. Without an explicit path, ModelRegistry + // loads the developer's real ~/.omp models config on every construction (~100ms each, + // and non-isolated). Pointing it at the (absent) temp file keeps construction at ~2ms and + // avoids leaking host config into the test. Providing the registry also skips the + // fire-and-forget background model discovery, which is irrelevant to credential_disabled. + modelRegistry: new ModelRegistry(authStorage, path.join(dirs.agentDir, "models.json")), settings: Settings.isolated(), disableExtensionDiscovery: true, extensions, skills: [], + rules: [], contextFiles: [], promptTemplates: [], workspaceTree: emptyWorkspaceTree(dirs.cwd), @@ -406,7 +413,7 @@ describe("createAgentSession credential_disabled subscription", () => { embedderEvents.push(event); }, }); - const modelRegistry = new ModelRegistry(authStorage); + const modelRegistry = new ModelRegistry(authStorage, path.join(dirs.agentDir, "models.json")); const ext = makeRecordingExtension(); const { session } = await createAgentSession({ @@ -448,7 +455,7 @@ describe("createAgentSession credential_disabled subscription", () => { const dirs = makeDirs("mismatch"); const registryStorage = await AuthStorage.create(path.join(dirs.agentDir, "agent-registry.db")); const otherStorage = await AuthStorage.create(path.join(dirs.agentDir, "agent-other.db")); - const modelRegistry = new ModelRegistry(registryStorage); + const modelRegistry = new ModelRegistry(registryStorage, path.join(dirs.agentDir, "models-registry.json")); await expect( createAgentSession({ @@ -477,7 +484,7 @@ describe("createAgentSession credential_disabled subscription", () => { // by one microtask so a sync onError() registration lands in time. const dirs = makeDirs("error-routing"); const authStorage = await AuthStorage.create(path.join(dirs.agentDir, "agent.db")); - const modelRegistry = new ModelRegistry(authStorage); + const modelRegistry = new ModelRegistry(authStorage, path.join(dirs.agentDir, "models.json")); try { const throwingExtension: Extension = { path: "test://throwing-credential-disabled", diff --git a/packages/coding-agent/test/sdk-mcp-discovery.test.ts b/packages/coding-agent/test/sdk-mcp-discovery.test.ts index f9d1082b5..ed866c6b2 100644 --- a/packages/coding-agent/test/sdk-mcp-discovery.test.ts +++ b/packages/coding-agent/test/sdk-mcp-discovery.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; @@ -47,18 +47,34 @@ const oldSessionMtime = new Date("2000-01-01T00:00:00.000Z"); describe("createAgentSession MCP discovery prompt gating", () => { let tempDir: string; + let registryDir: string; let authStorage: AuthStorage; let modelRegistry: ModelRegistry; - beforeEach(async () => { - tempDir = path.join(os.tmpdir(), `pi-sdk-mcp-discovery-${Snowflake.next()}`); - fs.mkdirSync(tempDir, { recursive: true }); - authStorage = await AuthStorage.create(path.join(tempDir, "auth.db")); + // Immutable across tests: ModelRegistry's constructor eagerly loads the bundled + // model catalog (~120ms). The tests pass models explicitly and never mutate the + // registry (refreshInBackground is skipped when modelRegistry is supplied, and + // extension source sync is empty under disableExtensionDiscovery), so build it once. + beforeAll(async () => { + registryDir = path.join(os.tmpdir(), `pi-sdk-mcp-discovery-registry-${Snowflake.next()}`); + fs.mkdirSync(registryDir, { recursive: true }); + authStorage = await AuthStorage.create(path.join(registryDir, "auth.db")); modelRegistry = new ModelRegistry(authStorage); }); - afterEach(() => { + afterAll(() => { authStorage.close(); + if (registryDir && fs.existsSync(registryDir)) { + fs.rmSync(registryDir, { recursive: true, force: true }); + } + }); + + beforeEach(() => { + tempDir = path.join(os.tmpdir(), `pi-sdk-mcp-discovery-${Snowflake.next()}`); + fs.mkdirSync(tempDir, { recursive: true }); + }); + + afterEach(() => { if (tempDir && fs.existsSync(tempDir)) { fs.rmSync(tempDir, { recursive: true, force: true }); } diff --git a/packages/coding-agent/test/sdk-model-selection.test.ts b/packages/coding-agent/test/sdk-model-selection.test.ts index c702edf29..89f072394 100644 --- a/packages/coding-agent/test/sdk-model-selection.test.ts +++ b/packages/coding-agent/test/sdk-model-selection.test.ts @@ -12,6 +12,7 @@ import { Snowflake } from "@oh-my-pi/pi-utils"; describe("createAgentSession deferred model pattern resolution", () => { let tempDir: string; + const authStoragesToClose: AuthStorage[] = []; beforeEach(() => { tempDir = path.join(os.tmpdir(), `pi-sdk-model-selection-${Snowflake.next()}`); @@ -19,6 +20,10 @@ describe("createAgentSession deferred model pattern resolution", () => { }); afterEach(() => { + for (const authStorage of authStoragesToClose) { + authStorage.close(); + } + authStoragesToClose.length = 0; if (tempDir && fs.existsSync(tempDir)) { fs.rmSync(tempDir, { recursive: true, force: true }); } @@ -52,10 +57,20 @@ describe("createAgentSession deferred model pattern resolution", () => { }); }; - function buildSessionOptions(modelPattern: string) { + async function buildSessionOptions(modelPattern: string) { + // Pass an explicit ModelRegistry so createAgentSession skips its implicit + // ModelRegistry.refreshInBackground() — a network model-discovery pass + // (~250ms/session) that contributes nothing here: the model resolves from + // the inline extension provider, never from network catalogs. Mirrors the + // explicit-registry pattern the resume tests below already rely on. + const authStorage = await AuthStorage.create(path.join(tempDir, "auth.db")); + authStoragesToClose.push(authStorage); + const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir, "models.yml")); return { cwd: tempDir, agentDir: tempDir, + authStorage, + modelRegistry, sessionManager: SessionManager.inMemory(), disableExtensionDiscovery: true, extensions: [providerExtension], @@ -71,7 +86,7 @@ describe("createAgentSession deferred model pattern resolution", () => { test("resolves explicit modelPattern after extension providers register", async () => { const { session, modelFallbackMessage } = await createAgentSession( - buildSessionOptions("runtime-provider/runtime-model"), + await buildSessionOptions("runtime-provider/runtime-model"), ); expect(session.model).toBeDefined(); @@ -82,7 +97,7 @@ describe("createAgentSession deferred model pattern resolution", () => { test("does not silently fallback when explicit modelPattern is unresolved", async () => { const { session, modelFallbackMessage } = await createAgentSession( - buildSessionOptions("missing-provider/missing-model"), + await buildSessionOptions("missing-provider/missing-model"), ); expect(session.model).toBeUndefined(); @@ -95,7 +110,7 @@ describe("createAgentSession deferred model pattern resolution", () => { settings.setModelRole("default", "pi/smol:high"); const { session } = await createAgentSession({ - ...buildSessionOptions("runtime-provider/runtime-reasoning-model"), + ...(await buildSessionOptions("runtime-provider/runtime-reasoning-model")), settings, }); diff --git a/packages/coding-agent/test/sdk-session-isolation.test.ts b/packages/coding-agent/test/sdk-session-isolation.test.ts index 91355299c..7bac57a38 100644 --- a/packages/coding-agent/test/sdk-session-isolation.test.ts +++ b/packages/coding-agent/test/sdk-session-isolation.test.ts @@ -1,12 +1,14 @@ -import { afterEach, describe, expect, it } from "bun:test"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import { type AssistantMessage, getBundledModel } from "@oh-my-pi/pi-ai"; import type { Rule } from "@oh-my-pi/pi-coding-agent/capability/rule"; +import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk"; import { SecretObfuscator } from "@oh-my-pi/pi-coding-agent/secrets"; +import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import { getSessionsDir, Snowflake } from "@oh-my-pi/pi-utils"; @@ -55,6 +57,23 @@ function getAssistantText(message: AssistantMessage | undefined): string { describe("createAgentSession session storage isolation", () => { const tempDirs: string[] = []; + // One shared, fully-populated (bundled models load synchronously in the + // constructor) registry for every case. Passing it via options skips the + // per-call discoverAuthStorage() SQLite open and the refreshInBackground() + // network model probe inside createAgentSession — the two real wall-clock + // sinks here. None of these cases assert on model discovery, so an + // ambient-credential-free in-memory auth store keeps them deterministic. + let sharedAuthStorage: AuthStorage; + let sharedModelRegistry: ModelRegistry; + + beforeAll(async () => { + sharedAuthStorage = await AuthStorage.create(":memory:"); + sharedModelRegistry = new ModelRegistry(sharedAuthStorage); + }); + + afterAll(() => { + sharedAuthStorage.close(); + }); afterEach(async () => { for (const tempDir of tempDirs.splice(0)) { @@ -72,6 +91,7 @@ describe("createAgentSession session storage isolation", () => { const { session } = await createAgentSession({ cwd, agentDir, + modelRegistry: sharedModelRegistry, settings: Settings.isolated(), disableExtensionDiscovery: true, skills: [], @@ -105,6 +125,7 @@ describe("createAgentSession session storage isolation", () => { const { session } = await createAgentSession({ cwd, agentDir, + modelRegistry: sharedModelRegistry, settings: Settings.isolated(), rules: [rule], disableExtensionDiscovery: true, @@ -137,6 +158,7 @@ describe("createAgentSession session storage isolation", () => { const commonOptions = { cwd, agentDir, + modelRegistry: sharedModelRegistry, settings: Settings.isolated({ "secrets.enabled": true }), disableExtensionDiscovery: true, skills: [], @@ -206,6 +228,7 @@ describe("createAgentSession session storage isolation", () => { const { session } = await createAgentSession({ cwd, agentDir, + modelRegistry: sharedModelRegistry, sessionManager: resumedManager, model, settings: Settings.isolated({ "secrets.enabled": true }), diff --git a/packages/coding-agent/test/sdk-skills.test.ts b/packages/coding-agent/test/sdk-skills.test.ts index 81ad42029..15bccd54a 100644 --- a/packages/coding-agent/test/sdk-skills.test.ts +++ b/packages/coding-agent/test/sdk-skills.test.ts @@ -1,10 +1,12 @@ -import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; +import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import type { Skill } from "@oh-my-pi/pi-coding-agent/sdk"; import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk"; +import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import { cleanupTempHome } from "./helpers/temp-home-cleanup"; @@ -24,6 +26,25 @@ describe("createAgentSession skills option", () => { let skillsDir: string; let tempHomeDir = ""; let originalHome: string | undefined; + // Auth storage (SQLite DB) and the model registry are immutable across these tests: skill + // discovery never touches models, and building them per test would make createAgentSession call + // modelRegistry.refreshInBackground(), whose online model discovery saturates the event loop and + // serializes the otherwise-parallel capability scans (~340ms/call). Supplying a prebuilt registry + // skips that refresh entirely (~24ms/call). + let sharedDir: string; + let sharedAuthStorage: AuthStorage; + let sharedModelRegistry: ModelRegistry; + + beforeAll(async () => { + sharedDir = fs.mkdtempSync(path.join(os.tmpdir(), "pi-sdk-skills-shared-")); + sharedAuthStorage = await AuthStorage.create(path.join(sharedDir, "auth.db")); + sharedModelRegistry = new ModelRegistry(sharedAuthStorage, path.join(sharedDir, "models.yml")); + }); + + afterAll(() => { + sharedAuthStorage.close(); + fs.rmSync(sharedDir, { recursive: true, force: true }); + }); beforeEach(() => { tempDir = path.join(os.tmpdir(), `pi-sdk-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); @@ -74,6 +95,7 @@ Loaded via symbolic link. cwd: tempDir, agentDir: tempDir, sessionManager: SessionManager.inMemory(), + modelRegistry: sharedModelRegistry, settings: createIsolatedSkillsSettings(), }); @@ -87,6 +109,7 @@ Loaded via symbolic link. cwd: tempDir, agentDir: tempDir, sessionManager: SessionManager.inMemory(), + modelRegistry: sharedModelRegistry, settings: createIsolatedSkillsSettings(), }); @@ -102,6 +125,7 @@ Loaded via symbolic link. cwd: tempDir, agentDir: tempDir, sessionManager: SessionManager.inMemory(), + modelRegistry: sharedModelRegistry, settings: createIsolatedSkillsSettings(), }); @@ -112,6 +136,7 @@ Loaded via symbolic link. cwd: tempDir, agentDir: tempDir, sessionManager: SessionManager.inMemory(), + modelRegistry: sharedModelRegistry, skills: [], // Explicitly empty - like --no-skills settings: createIsolatedSkillsSettings(), }); @@ -135,6 +160,7 @@ Loaded via symbolic link. cwd: tempDir, agentDir: tempDir, sessionManager: SessionManager.inMemory(), + modelRegistry: sharedModelRegistry, skills: [customSkill], settings: createIsolatedSkillsSettings(), }); diff --git a/packages/coding-agent/test/sdk-tool-activation.test.ts b/packages/coding-agent/test/sdk-tool-activation.test.ts index 6758acde1..a4b8da987 100644 --- a/packages/coding-agent/test/sdk-tool-activation.test.ts +++ b/packages/coding-agent/test/sdk-tool-activation.test.ts @@ -4,7 +4,11 @@ import * as os from "node:os"; import * as path from "node:path"; import { getBundledModel } from "@oh-my-pi/pi-ai"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; -import { createAgentSession, type ExtensionFactory } from "@oh-my-pi/pi-coding-agent/sdk"; +import { + type CreateAgentSessionOptions, + createAgentSession, + type ExtensionFactory, +} from "@oh-my-pi/pi-coding-agent/sdk"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import { Snowflake } from "@oh-my-pi/pi-utils"; import * as z from "zod/v4"; @@ -34,6 +38,35 @@ const toolActivationExtension: ExtensionFactory = pi => { describe("createAgentSession defaultInactive tool activation", () => { const tempDirs: string[] = []; + const makeTempDir = (): string => { + const tempDir = path.join(os.tmpdir(), `pi-sdk-tool-activation-${Snowflake.next()}`); + tempDirs.push(tempDir); + fs.mkdirSync(tempDir, { recursive: true }); + return tempDir; + }; + + // Shared options for every session. `rules: []` and `workspaceTree` short-circuit + // the two slow startup scans (rule discovery + native workspace walk, ~100ms each) + // that are irrelevant to tool activation: these tests assert only which tools are + // registered/active and that tool names appear in the system prompt. Each call + // returns fresh `settings`/`sessionManager` instances to keep tests isolated. + const baseOptions = (tempDir: string): CreateAgentSessionOptions => ({ + cwd: tempDir, + agentDir: tempDir, + sessionManager: SessionManager.inMemory(), + settings: Settings.isolated(), + model: getBundledModel("openai", "gpt-4o-mini"), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + promptTemplates: [], + slashCommands: [], + enableMCP: false, + enableLsp: false, + rules: [], + workspaceTree: { rootPath: tempDir, rendered: "", truncated: false, totalLines: 0, agentsMdFiles: [] }, + }); + afterEach(() => { for (const tempDir of tempDirs.splice(0)) { fs.rmSync(tempDir, { recursive: true, force: true }); @@ -43,24 +76,11 @@ describe("createAgentSession defaultInactive tool activation", () => { }); it("excludes defaultInactive extension tools from the initial active set unless explicitly requested", async () => { - const tempDir = path.join(os.tmpdir(), `pi-sdk-tool-activation-${Snowflake.next()}`); - tempDirs.push(tempDir); - fs.mkdirSync(tempDir, { recursive: true }); + const tempDir = makeTempDir(); const { session } = await createAgentSession({ - cwd: tempDir, - agentDir: tempDir, - sessionManager: SessionManager.inMemory(), - settings: Settings.isolated(), - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, + ...baseOptions(tempDir), extensions: [toolActivationExtension], - skills: [], - contextFiles: [], - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, }); try { @@ -77,24 +97,11 @@ describe("createAgentSession defaultInactive tool activation", () => { }); it("allows explicitly requested defaultInactive extension tools into the initial active set", async () => { - const tempDir = path.join(os.tmpdir(), `pi-sdk-tool-activation-${Snowflake.next()}`); - tempDirs.push(tempDir); - fs.mkdirSync(tempDir, { recursive: true }); + const tempDir = makeTempDir(); const { session } = await createAgentSession({ - cwd: tempDir, - agentDir: tempDir, - sessionManager: SessionManager.inMemory(), - settings: Settings.isolated(), - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, + ...baseOptions(tempDir), extensions: [toolActivationExtension], - skills: [], - contextFiles: [], - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, toolNames: ["read", "default_inactive_tool"], }); @@ -113,23 +120,10 @@ describe("createAgentSession defaultInactive tool activation", () => { // (e.g. `["read", "search", "find", "lsp", "web_search"]`). Without this // invariant, `yield` ended up registered but not active, and the model // could not satisfy the idle-reminder contract that demands a `yield` call. - const tempDir = path.join(os.tmpdir(), `pi-sdk-tool-activation-${Snowflake.next()}`); - tempDirs.push(tempDir); - fs.mkdirSync(tempDir, { recursive: true }); + const tempDir = makeTempDir(); const { session } = await createAgentSession({ - cwd: tempDir, - agentDir: tempDir, - sessionManager: SessionManager.inMemory(), - settings: Settings.isolated(), - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, - skills: [], - contextFiles: [], - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, + ...baseOptions(tempDir), requireYieldTool: true, toolNames: ["read", "search", "find", "web_search"], }); @@ -149,23 +143,10 @@ describe("createAgentSession defaultInactive tool activation", () => { // the registry has no `deferrable` tool, so the previous gate dropped // `resolve` from the registry and plan mode silently activated without // it — leaving the agent stuck after drafting the plan. - const tempDir = path.join(os.tmpdir(), `pi-sdk-tool-activation-${Snowflake.next()}`); - tempDirs.push(tempDir); - fs.mkdirSync(tempDir, { recursive: true }); + const tempDir = makeTempDir(); const { session } = await createAgentSession({ - cwd: tempDir, - agentDir: tempDir, - sessionManager: SessionManager.inMemory(), - settings: Settings.isolated(), - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, - skills: [], - contextFiles: [], - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, + ...baseOptions(tempDir), toolNames: ["read", "search", "find", "web_search"], }); @@ -177,26 +158,14 @@ describe("createAgentSession defaultInactive tool activation", () => { }); it("drops the hidden resolve tool when neither a deferrable tool nor plan mode can use it", async () => { - const tempDir = path.join(os.tmpdir(), `pi-sdk-tool-activation-${Snowflake.next()}`); - tempDirs.push(tempDir); - fs.mkdirSync(tempDir, { recursive: true }); + const tempDir = makeTempDir(); const settings = Settings.isolated(); settings.set("plan.enabled", false); const { session } = await createAgentSession({ - cwd: tempDir, - agentDir: tempDir, - sessionManager: SessionManager.inMemory(), + ...baseOptions(tempDir), settings, - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, - skills: [], - contextFiles: [], - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, toolNames: ["read", "search", "find", "web_search"], }); @@ -208,23 +177,10 @@ describe("createAgentSession defaultInactive tool activation", () => { }); it("does not register the xAI TTS tool unless enabled", async () => { - const tempDir = path.join(os.tmpdir(), `pi-sdk-tool-activation-${Snowflake.next()}`); - tempDirs.push(tempDir); - fs.mkdirSync(tempDir, { recursive: true }); + const tempDir = makeTempDir(); const { session } = await createAgentSession({ - cwd: tempDir, - agentDir: tempDir, - sessionManager: SessionManager.inMemory(), - settings: Settings.isolated(), - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, - skills: [], - contextFiles: [], - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, + ...baseOptions(tempDir), }); try { @@ -237,23 +193,11 @@ describe("createAgentSession defaultInactive tool activation", () => { }); it("registers the xAI TTS tool when enabled", async () => { - const tempDir = path.join(os.tmpdir(), `pi-sdk-tool-activation-${Snowflake.next()}`); - tempDirs.push(tempDir); - fs.mkdirSync(tempDir, { recursive: true }); + const tempDir = makeTempDir(); const { session } = await createAgentSession({ - cwd: tempDir, - agentDir: tempDir, - sessionManager: SessionManager.inMemory(), + ...baseOptions(tempDir), settings: Settings.isolated({ "tts.enabled": true }), - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, - skills: [], - contextFiles: [], - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, }); try { diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index ff0825548..7493cbb1e 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -16,6 +16,7 @@ import { JobTool } from "@oh-my-pi/pi-coding-agent/tools/job"; import { wrapToolWithMetaNotice } from "@oh-my-pi/pi-coding-agent/tools/output-meta"; import { ReadTool } from "@oh-my-pi/pi-coding-agent/tools/read"; import { DEFAULT_FILE_LIMIT, MULTI_FILE_PER_FILE_MATCHES, SearchTool } from "@oh-my-pi/pi-coding-agent/tools/search"; +import * as toolTimeouts from "@oh-my-pi/pi-coding-agent/tools/tool-timeouts"; import { WriteTool } from "@oh-my-pi/pi-coding-agent/tools/write"; import { $which, Snowflake } from "@oh-my-pi/pi-utils"; import { unzipSync } from "fflate"; @@ -1164,7 +1165,7 @@ function b() { const updates: string[] = []; const result = await bashTool.execute( "test-call-8-stream", - { command: "for i in 1 2 3; do echo $i; sleep 0.2; done" }, + { command: "for i in 1 2 3; do echo $i; sleep 0.1; done" }, undefined, update => { const text = update.content?.find(c => c.type === "text")?.text ?? ""; @@ -1310,13 +1311,19 @@ function b() { ), ), ); + // Drive the effective timeout via the production clamp seam so the + // backgrounded job times out in ~0.5s instead of a real wall-clock + // second. 0.5s still renders as "1 seconds" in the executor message + // (Math.round), so that delivery assertion is unchanged; the + // auto-background-on-timeout decision path is identical. + vi.spyOn(toolTimeouts, "clampTimeout").mockReturnValue(0.5); const result = await autoBackgroundBashTool.execute("test-call-9-auto-timeout-background", { command: "printf 'start\\n'; sleep 1.2; printf 'done\\n'", timeout: 1, }); - expect(result.details?.timeoutSeconds).toBe(1); + expect(result.details?.timeoutSeconds).toBe(0.5); expect(result.details?.async?.state).toBe("running"); expect(getTextOutput(result)).toContain("Background job"); const jobId = result.details?.async?.jobId; @@ -1344,6 +1351,9 @@ function b() { }); it("should respect timeout", async () => { + // Reduce the effective timeout through the production clamp seam; the + // real subprocess kill-on-timeout path is still exercised, just faster. + vi.spyOn(toolTimeouts, "clampTimeout").mockReturnValue(0.1); await expect(bashTool.execute("test-call-10", { command: "sleep 5", timeout: 1 })).rejects.toThrow( /timed out/i, ); @@ -1351,10 +1361,19 @@ function b() { it("should abort and recover for subsequent commands", async () => { const controller = new AbortController(); - const promise = bashTool.execute("test-call-10-abort", { command: "sleep 60" }, controller.signal); - // Give the native shell a beat to enter `sleep`; do not depend on chunk - // delivery timing, which is flaky on loaded CI runners. - await Bun.sleep(100); + const started = Promise.withResolvers(); + const promise = bashTool.execute( + "test-call-10-abort", + { command: "echo READY; sleep 60" }, + controller.signal, + update => { + const text = update.content?.find(c => c.type === "text")?.text ?? ""; + if (text.includes("READY")) started.resolve(); + }, + ); + // Abort as soon as the command has emitted output (proving the shell is + // live), instead of blindly waiting a fixed beat for it to enter `sleep`. + await started.promise; controller.abort("test abort"); await expect(promise).rejects.toThrow(/abort|cancel|timed out/i); diff --git a/packages/coding-agent/test/tools/approval-mode.test.ts b/packages/coding-agent/test/tools/approval-mode.test.ts index 9b0709881..f2fa648c2 100644 --- a/packages/coding-agent/test/tools/approval-mode.test.ts +++ b/packages/coding-agent/test/tools/approval-mode.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it } from "bun:test"; +import { afterAll, beforeAll, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; @@ -19,31 +19,6 @@ function emptyWorkspaceTree(cwd: string) { return { rootPath: cwd, rendered: ".\n", truncated: false, totalLines: 1, agentsMdFiles: [] }; } -async function makeSession(extraSettings: Record = {}) { - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-approval-mode-${Snowflake.next()}-`)); - const cwd = path.join(tempDir, "cwd"); - fs.mkdirSync(cwd, { recursive: true }); - const sessionManager = SessionManager.create(cwd, path.join(tempDir, "sessions")); - const settings = Settings.isolated({ ...BASE_SETTINGS, ...extraSettings }); - const { session } = await createAgentSession({ - cwd, - agentDir: tempDir, - sessionManager, - settings, - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, - skills: [], - contextFiles: [], - workspaceTree: emptyWorkspaceTree(cwd), - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, - toolNames: ["bash"], - }); - return { tempDir, session, settings }; -} - function textOf(result: { content?: ReadonlyArray<{ type: string; text?: string }> }): string { const blocks = result.content ?? []; for (const block of blocks) { @@ -53,179 +28,155 @@ function textOf(result: { content?: ReadonlyArray<{ type: string; text?: string } describe("tools.approvalMode setting", () => { - const tempDirs: string[] = []; + // The per-tool approval gate (ExtensionToolWrapper) reads approvalMode / tools.approval / + // autoApprove exclusively from the execute-time AgentToolContext, never from the session's + // own settings. So a single shared session exercises every mode — we only vary the context + // settings per assertion. This avoids paying createAgentSession's cost (model registry, + // auth-storage discovery, settings init) nine times over. + let tempDir: string; + let session: Awaited>["session"]; - afterEach(async () => { - for (const tempDir of tempDirs.splice(0)) { - // Windows can briefly hold tempdir handles after session.dispose(); retry a few times. - for (let attempt = 0; attempt < 5; attempt++) { - try { - fs.rmSync(tempDir, { recursive: true, force: true }); - break; - } catch (err) { - const code = (err as NodeJS.ErrnoException).code; - if (code !== "EBUSY" && code !== "ENOTEMPTY" && code !== "EPERM") throw err; - if (attempt === 4) break; // best-effort: OS will reclaim - await Bun.sleep(50 * (attempt + 1)); - } + beforeAll(async () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-approval-mode-${Snowflake.next()}-`)); + const cwd = path.join(tempDir, "cwd"); + fs.mkdirSync(cwd, { recursive: true }); + const sessionManager = SessionManager.create(cwd, path.join(tempDir, "sessions")); + const created = await createAgentSession({ + cwd, + agentDir: tempDir, + sessionManager, + settings: Settings.isolated(BASE_SETTINGS), + model: getBundledModel("openai", "gpt-4o-mini"), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + workspaceTree: emptyWorkspaceTree(cwd), + promptTemplates: [], + slashCommands: [], + enableMCP: false, + enableLsp: false, + toolNames: ["bash"], + }); + session = created.session; + }); + + afterAll(async () => { + await session.dispose(); + // Windows can briefly hold tempdir handles after session.dispose(); retry a few times. + for (let attempt = 0; attempt < 5; attempt++) { + try { + fs.rmSync(tempDir, { recursive: true, force: true }); + break; + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code !== "EBUSY" && code !== "ENOTEMPTY" && code !== "EPERM") throw err; + if (attempt === 4) break; // best-effort: OS will reclaim + await Bun.sleep(50 * (attempt + 1)); } } }); + function approvalSettings(extraSettings: Record = {}): Settings { + return Settings.isolated({ ...BASE_SETTINGS, ...extraSettings }); + } + + function bashTool() { + const bash = session.getToolByName("bash"); + if (!bash) throw new Error("Expected bash tool"); + return bash; + } + it("yolo mode (default) bypasses approval for non-overriding tool calls", async () => { - const { tempDir, session, settings } = await makeSession(); - tempDirs.push(tempDir); - try { - const bash = session.getToolByName("bash"); - if (!bash) throw new Error("Expected bash tool"); - const result = await bash.execute("yolo", { command: "echo ok" }, undefined, undefined, { - settings, - } as AgentToolContext); - expect(textOf(result)).toContain("ok"); - } finally { - await session.dispose(); - } + const settings = approvalSettings(); + const result = await bashTool().execute("yolo", { command: "echo ok" }, undefined, undefined, { + settings, + } as AgentToolContext); + expect(textOf(result)).toContain("ok"); }); it("always-ask mode rejects exec tools when no UI is available", async () => { - const { tempDir, session, settings } = await makeSession({ - "tools.approvalMode": "always-ask", - }); - tempDirs.push(tempDir); - try { - const bash = session.getToolByName("bash"); - if (!bash) throw new Error("Expected bash tool"); - await expect( - bash.execute("always-ask", { command: "echo blocked" }, undefined, undefined, { - settings, - } as AgentToolContext), - ).rejects.toThrow(/requires approval but no interactive UI available/); - } finally { - await session.dispose(); - } + const settings = approvalSettings({ "tools.approvalMode": "always-ask" }); + await expect( + bashTool().execute("always-ask", { command: "echo blocked" }, undefined, undefined, { + settings, + } as AgentToolContext), + ).rejects.toThrow(/requires approval but no interactive UI available/); }); it("per-tool allow overrides are honored in every mode", async () => { - const { tempDir, session, settings } = await makeSession({ + const settings = approvalSettings({ "tools.approvalMode": "always-ask", "tools.approval": { bash: "allow" }, }); - tempDirs.push(tempDir); - try { - const bash = session.getToolByName("bash"); - if (!bash) throw new Error("Expected bash tool"); - const result = await bash.execute("always-ask-allow", { command: "echo allowed" }, undefined, undefined, { - settings, - } as AgentToolContext); - expect(textOf(result)).toContain("allowed"); - } finally { - await session.dispose(); - } + const result = await bashTool().execute("always-ask-allow", { command: "echo allowed" }, undefined, undefined, { + settings, + } as AgentToolContext); + expect(textOf(result)).toContain("allowed"); }); it("per-tool prompt overrides can tighten yolo mode", async () => { - const { tempDir, session, settings } = await makeSession({ + const settings = approvalSettings({ "tools.approvalMode": "yolo", "tools.approval": { bash: "prompt" }, }); - tempDirs.push(tempDir); - try { - const bash = session.getToolByName("bash"); - if (!bash) throw new Error("Expected bash tool"); - await expect( - bash.execute("yolo-prompt", { command: "echo blocked" }, undefined, undefined, { - settings, - } as AgentToolContext), - ).rejects.toThrow(/requires approval but no interactive UI available/); - } finally { - await session.dispose(); - } + await expect( + bashTool().execute("yolo-prompt", { command: "echo blocked" }, undefined, undefined, { + settings, + } as AgentToolContext), + ).rejects.toThrow(/requires approval but no interactive UI available/); }); it("write mode still prompts exec-tier tools", async () => { - const { tempDir, session, settings } = await makeSession({ + const settings = approvalSettings({ "tools.approvalMode": "write", "tools.approval": {}, }); - tempDirs.push(tempDir); - try { - const bash = session.getToolByName("bash"); - if (!bash) throw new Error("Expected bash tool"); - await expect( - bash.execute("write-mode", { command: "echo unconfigured" }, undefined, undefined, { - settings, - } as AgentToolContext), - ).rejects.toThrow(/requires approval but no interactive UI available/); - } finally { - await session.dispose(); - } + await expect( + bashTool().execute("write-mode", { command: "echo unconfigured" }, undefined, undefined, { + settings, + } as AgentToolContext), + ).rejects.toThrow(/requires approval but no interactive UI available/); }); it("critical bash patterns do not prompt in yolo mode with bash allowed", async () => { - const { tempDir, session, settings } = await makeSession({ + const settings = approvalSettings({ "tools.approvalMode": "yolo", "tools.approval": { bash: "allow" }, }); - tempDirs.push(tempDir); - try { - const bash = session.getToolByName("bash"); - if (!bash) throw new Error("Expected bash tool"); - - const result = await bash.execute( - "critical", - { command: "rm -f /tmp/bun-fake-timer-probe.test.ts" }, - undefined, - undefined, - { - settings, - } as AgentToolContext, - ); - expect(textOf(result)).toContain("(no output)"); - } finally { - await session.dispose(); - } + const result = await bashTool().execute( + "critical", + { command: "rm -f /tmp/bun-fake-timer-probe.test.ts" }, + undefined, + undefined, + { + settings, + } as AgentToolContext, + ); + expect(textOf(result)).toContain("(no output)"); }); it("CLI --auto-approve forces yolo mode for non-overriding tool calls", async () => { - const { tempDir, session, settings } = await makeSession({ - "tools.approvalMode": "always-ask", - }); - tempDirs.push(tempDir); - try { - const bash = session.getToolByName("bash"); - if (!bash) throw new Error("Expected bash tool"); - const result = await bash.execute("cli-override", { command: "echo override" }, undefined, undefined, { - settings, - autoApprove: true, - } as AgentToolContext); - expect(textOf(result)).toContain("override"); - } finally { - await session.dispose(); - } + const settings = approvalSettings({ "tools.approvalMode": "always-ask" }); + const result = await bashTool().execute("cli-override", { command: "echo override" }, undefined, undefined, { + settings, + autoApprove: true, + } as AgentToolContext); + expect(textOf(result)).toContain("override"); }); it("CLI --auto-approve also bypasses safety-override patterns", async () => { - const { tempDir, session, settings } = await makeSession({ - "tools.approvalMode": "always-ask", - }); - tempDirs.push(tempDir); - try { - const bash = session.getToolByName("bash"); - if (!bash) throw new Error("Expected bash tool"); - const result = await bash.execute( - "cli-critical", - { command: "rm -f /tmp/bun-fake-timer-probe.test.ts" }, - undefined, - undefined, - { - settings, - autoApprove: true, - } as AgentToolContext, - ); - expect(textOf(result)).toContain("(no output)"); - } finally { - await session.dispose(); - } + const settings = approvalSettings({ "tools.approvalMode": "always-ask" }); + const result = await bashTool().execute( + "cli-critical", + { command: "rm -f /tmp/bun-fake-timer-probe.test.ts" }, + undefined, + undefined, + { + settings, + autoApprove: true, + } as AgentToolContext, + ); + expect(textOf(result)).toContain("(no output)"); }); it("constructs an extensionRunner unconditionally so the approval gate is always installed", async () => { @@ -236,12 +187,6 @@ describe("tools.approvalMode setting", () => { // any non-yolo approval mode setting would be a no-op without feedback. The // fix is to construct the runner unconditionally; this test makes that contract explicit so // a future change to make the runner optional again cannot silently re-open the hole. - const { tempDir, session } = await makeSession(); - tempDirs.push(tempDir); - try { - expect(session.extensionRunner).toBeDefined(); - } finally { - await session.dispose(); - } + expect(session.extensionRunner).toBeDefined(); }); }); diff --git a/packages/coding-agent/test/tools/conflict-integration.test.ts b/packages/coding-agent/test/tools/conflict-integration.test.ts index 73c784d50..5d1a92a8b 100644 --- a/packages/coding-agent/test/tools/conflict-integration.test.ts +++ b/packages/coding-agent/test/tools/conflict-integration.test.ts @@ -26,7 +26,11 @@ function getText(result: { content: Array<{ type: string; text?: string }> }): s } async function getTool(session: ToolSession, name: "read" | "write") { - const tools = await createTools(session); + // Request only the tool under test: createTools(session) with no toolNames + // builds every builtin factory (LSP, MCP discovery, browser, eval preflight, + // …) on each call, which is pure overhead here. The conflict contract lives + // entirely in the read/write tools + session.conflictHistory. + const tools = await createTools(session, [name]); const tool = tools.find(entry => entry.name === name); if (!tool) throw new Error(`Missing ${name} tool`); return tool; diff --git a/packages/coding-agent/test/tools/fetch-jina-stall.test.ts b/packages/coding-agent/test/tools/fetch-jina-stall.test.ts index 03f33b1dc..7582f1cc2 100644 --- a/packages/coding-agent/test/tools/fetch-jina-stall.test.ts +++ b/packages/coding-agent/test/tools/fetch-jina-stall.test.ts @@ -45,9 +45,12 @@ describe("renderHtmlToText: jina stall does not starve local fallbacks (#1449)", }); const started = Date.now(); - // `timeout: 2` keeps the overall budget tight — the test must complete - // within ~2s even though Jina would otherwise hang for the full budget. - const result = await renderHtmlToText("https://example.com/article", html, 2, settings, undefined, null); + // Tight 300ms reader-mode budget. Jina would otherwise hang forever, but + // the remote sub-budget (min(timeout*1000, REMOTE_READER_MAX_MS)) aborts + // the stalled request so the local native renderer still runs. Kept small + // so the test exercises the same abort path without burning real + // wall-clock time waiting out the stall. + const result = await renderHtmlToText("https://example.com/article", html, 0.3, settings, undefined, null); const elapsedMs = Date.now() - started; expect(result.ok).toBe(true); @@ -56,10 +59,10 @@ describe("renderHtmlToText: jina stall does not starve local fallbacks (#1449)", // If trafilatura or lynx happened to succeed first, that's also a valid // non-aborted outcome. expect(["native", "trafilatura", "lynx"]).toContain(result.method); - // Must finish well before the overall budget elapses: the remote - // sub-budget caps Jina at min(timeout, REMOTE_READER_MAX_MS), so the - // remaining ~1s of the 2s budget is enough for the native renderer. - expect(elapsedMs).toBeLessThan(2_500); + // Must finish shortly after the 300ms budget aborts the stalled Jina + // request — never anywhere near an unbounded hang. The generous bound + // absorbs scheduler jitter under full-suite parallelism. + expect(elapsedMs).toBeLessThan(1_500); }); it("re-throws when the user signal is aborted, not when Jina sub-budget expires", async () => { diff --git a/packages/coding-agent/test/tools/gh.test.ts b/packages/coding-agent/test/tools/gh.test.ts index 2deb69989..b1776f807 100644 --- a/packages/coding-agent/test/tools/gh.test.ts +++ b/packages/coding-agent/test/tools/gh.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it, vi } from "bun:test"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -85,15 +85,24 @@ function runGit(cwd: string, args: string[]): string { return new TextDecoder().decode(result.stdout).trim(); } -async function createPrFixture(): Promise<{ +interface PrFixture { baseDir: string; repoRoot: string; originBare: string; forkBare: string; headRefName: string; headRefOid: string; -}> { - const baseDir = await fs.mkdtemp(path.join(os.tmpdir(), "gh-pr-tool-")); +} + +// Building the fixture costs ~16 real `git` subprocess spawns (~200ms). Six +// tests need it, so we build it ONCE as an immutable template in `beforeAll` +// and materialize per-test copies via `fs.cp` (~12ms). Each copy is a fully +// independent repo tree, so the mutating tests (worktree checkout, config +// writes, extra branches) can't contaminate each other. +let prFixtureTemplate: PrFixture | null = null; + +async function buildPrFixtureTemplate(): Promise { + const baseDir = await fs.mkdtemp(path.join(os.tmpdir(), "gh-pr-tool-template-")); const repoRoot = path.join(baseDir, "repo"); const originBare = path.join(baseDir, "origin.git"); const forkBare = path.join(baseDir, "fork.git"); @@ -119,13 +128,32 @@ async function createPrFixture(): Promise<{ runGit(repoRoot, ["push", "-u", "forksrc", headRefName]); runGit(repoRoot, ["checkout", "main"]); + return { baseDir, repoRoot, originBare, forkBare, headRefName, headRefOid }; +} + +async function createPrFixture(): Promise { + const template = prFixtureTemplate; + if (!template) throw new Error("PR fixture template was not built (missing beforeAll)"); + + const baseDir = await fs.mkdtemp(path.join(os.tmpdir(), "gh-pr-tool-")); + const repoRoot = path.join(baseDir, "repo"); + const originBare = path.join(baseDir, "origin.git"); + const forkBare = path.join(baseDir, "fork.git"); + + await fs.cp(template.baseDir, baseDir, { recursive: true }); + // Remote URLs in the copied repo still point at the template's absolute + // `origin.git`/`fork.git`. Repoint them at this copy so pushes/fetches stay + // isolated and `remote get-url` assertions match the returned paths. + runGit(repoRoot, ["remote", "set-url", "origin", originBare]); + runGit(repoRoot, ["remote", "set-url", "forksrc", forkBare]); + return { baseDir, repoRoot, originBare, forkBare, - headRefName, - headRefOid, + headRefName: template.headRefName, + headRefOid: template.headRefOid, }; } @@ -210,6 +238,17 @@ describe("parsePrUnifiedDiff", () => { }); describe("github tool", () => { + beforeAll(async () => { + prFixtureTemplate = await buildPrFixtureTemplate(); + }); + + afterAll(async () => { + if (prFixtureTemplate) { + await fs.rm(prFixtureTemplate.baseDir, { recursive: true, force: true }); + prFixtureTemplate = null; + } + }); + afterEach(() => { vi.useRealTimers(); vi.restoreAllMocks(); diff --git a/packages/coding-agent/test/tools/lsp-regressions.test.ts b/packages/coding-agent/test/tools/lsp-regressions.test.ts index f4e4aa2e6..f246f5545 100644 --- a/packages/coding-agent/test/tools/lsp-regressions.test.ts +++ b/packages/coding-agent/test/tools/lsp-regressions.test.ts @@ -373,6 +373,10 @@ for await (const chunk of Bun.stdin.stream()) { args: [serverPath, eventLogPath, statusCountPath, fileToUri(sourcePath)], fileTypes: ["rs"], rootMarkers: [], + // Shrink the workspace-ready polling window so the test exercises the + // timeout→retry→ready sequence without waiting out the 2s production settle. + // The status-request timeout stays generous to avoid racing the subprocess. + workspaceReadyTimings: { timeoutMs: 5_000, pollMs: 10, settleMs: 20, statusRequestTimeoutMs: 150 }, }; vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ diff --git a/packages/tui/test/render-regressions.test.ts b/packages/tui/test/render-regressions.test.ts index 1693cc817..522f230b1 100644 --- a/packages/tui/test/render-regressions.test.ts +++ b/packages/tui/test/render-regressions.test.ts @@ -27,6 +27,34 @@ class MutableLinesComponent implements Component { } } +// Models a component that caches its rendered output and only refreshes it when +// `invalidate()` fires — like a transcript block that freezes a snapshot. A +// state change behind the cache is invisible until something invalidates it, +// which is exactly what `resetDisplay()` must do to surface a Ctrl+O expansion. +class CachedComponent implements Component { + #current: string[]; + #cache: string[] | undefined; + + constructor(lines: string[]) { + this.#current = [...lines]; + } + + setLines(lines: string[]): void { + this.#current = [...lines]; + } + + invalidate(): void { + this.#cache = undefined; + } + + render(width: number): string[] { + if (this.#cache === undefined) { + this.#cache = this.#current.map(line => line.slice(0, width)); + } + return this.#cache; + } +} + class WrappingLinesComponent implements Component { #lines: string[]; @@ -366,6 +394,32 @@ describe("TUI terminal-state regressions", () => { } }); + it("resetDisplay surfaces a state change hidden behind a component's render cache", async () => { + const term = new VirtualTerminal(20, 3); + const tui = new TUI(term); + const component = new CachedComponent(rows("L", 8)); + tui.addChild(component); + + try { + tui.start(); + await settle(term); + expect(visible(term)).toEqual(["L5", "L6", "L7"]); + + // The component's content changes, but its render stays cached (a + // frozen transcript snapshot). resetDisplay() must invalidate it so the + // forced replay reflects the new content rather than the stale cache — + // the Ctrl+O expansion path depends on this. + component.setLines(rows("M", 8)); + tui.resetDisplay(); + await settle(term); + + expect(term.getScrollBuffer().map(line => line.trimEnd())).toEqual(rows("M", 8)); + expect(visible(term)).toEqual(["M5", "M6", "M7"]); + } finally { + tui.stop(); + } + }); + it("keeps appended rows in scrollback when a forced render coalesces with content growth", async () => { const term = new VirtualTerminal(20, 3); const tui = new TUI(term); diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index 23eee711b..95442dc2a 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Changed + +- `logger.printTimings()` (the `PI_TIMING` startup tree) now surfaces two previously-invisible regions: a `(before instrumentation)` line for the runtime init + static module-graph load that elapses before the first marker (the dominant real-world startup cost, ~350ms — `startTiming()` only begins inside `runRootCommand`), and an `(unattributed self)` line for the root span's own untimed work so the gap between the visible top-level spans and `Total` is no longer silently swallowed. `Total` is now labelled `(since first marker)` to make the window explicit. + ## [15.9.2] - 2026-06-05 ### Added diff --git a/packages/utils/src/logger.ts b/packages/utils/src/logger.ts index ae0f7fd36..c682bbf8f 100644 --- a/packages/utils/src/logger.ts +++ b/packages/utils/src/logger.ts @@ -187,6 +187,13 @@ export function printTimings(): void { const lines: string[] = []; lines.push(""); lines.push("--- Startup timings (hierarchical) ---"); + // performance.now() shares the process-start origin, so the root span's start + // is the wall time spent before the first marker — runtime init plus the + // static module-graph evaluation (~the dominant cost). It is otherwise + // invisible because Total only spans startTiming()→printTimings(). + if (gRootSpan.start > LOGGED_TIMING_THRESHOLD_MS) { + lines.push(`(before instrumentation): ${fmtMs(gRootSpan.start)} [runtime init + module load]`); + } const work: Span[] = []; const loads: Span[] = []; for (const child of gRootSpan.children) { @@ -199,8 +206,14 @@ export function printTimings(): void { if (loads.length > 0) { printModuleLoadSummary(loads, 0, lines); } + // Surface the root's own unattributed time so the gap between the visible + // top-level spans and Total isn't silently swallowed. + const rootSelf = selfTimeOf(gRootSpan); + if (gRootSpan.children.length > 0 && rootSelf > LOGGED_TIMING_THRESHOLD_MS) { + lines.push(`(unattributed self): ${fmtMs(rootSelf)}`); + } const totalMs = (gRootSpan.end - gRootSpan.start).toFixed(1); - lines.push(`Total: ${totalMs}ms`); + lines.push(`Total: ${totalMs}ms (since first marker)`); lines.push("--------------------------------------"); lines.push(""); console.error(lines.join("\n"));