diff --git a/AGENTS.md b/AGENTS.md index 5eaa98c45..56c39df08 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -454,6 +454,7 @@ When adding or changing tests, test the contract the system exposes — not the - Do not add placeholder tests, tautologies, or assertions that only prove the code executed (`expect(true).toBe(true)`, `not.toThrow()`, non-empty string checks, array length growth checks, or "prompt exists" checks without a stronger semantic assertion). - Prefer contract-level tests over implementation-detail tests. Avoid asserting internal helper wiring, field assignment, singleton identity, incidental ordering, prompt boilerplate, or passthrough option forwarding unless another component depends on that exact detail as a documented contract. - Do not duplicate coverage across abstraction levels. If an integration or public-surface test already proves the behavior, delete or avoid the narrower unit test that only restates it through mocks or internal plumbing. +- Tests MUST be full-suite safe, not just file-local safe. Do not use top-level `mock.module()` for shared workspace packages (`@oh-my-pi/pi-utils`, `@oh-my-pi/pi-ai`, `@oh-my-pi/pi-natives`) or long-lived file-wide mutations of globals like `Bun.*`, `process.platform`, `process.env`, or `Bun.env` when a narrower seam exists. Prefer per-test `vi.spyOn(...)`, local fakes, and immediate restoration. A test that passes in isolation but poisons later files is broken. - For lifecycle or stateful code, prefer one test per invariant or transition over several tiny tests that each assert one field from the same transition. - For error handling, prefer tests that trigger the real failure path and assert the surfaced error contract over tests that directly instantiate error classes or inspect purely internal metadata. - Smoke tests are only acceptable when they detect a failure mode narrower tests would miss. A test that only proves a package boots or a command starts is not enough. 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 2c4ac6f0d..86f67d1e6 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 { afterAll, afterEach, describe, expect, it, mock, vi } from "bun:test"; +import { afterEach, 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"; @@ -14,44 +14,6 @@ import { } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import { Snowflake } from "@oh-my-pi/pi-utils"; -const nativeModuleStub = new Proxy( - { - FileType: { File: "file", Directory: "directory" }, - ImageFormat: { Png: "png", PNG: "png", Jpeg: "jpeg", JPEG: "jpeg", WebP: "webp", WEBP: "webp" }, - SamplingFilter: { Lanczos3: "lanczos3" }, - Shell: class {}, - PtySession: class {}, - PhotonImage: class {}, - executeShell: vi.fn(() => ({ exitCode: 0, stdout: "", stderr: "" })), - getWorkProfile: vi.fn(() => ({ isInsideWorktree: false })), - sanitizeText: vi.fn((text: string) => text), - wrapTextWithAnsi: vi.fn((text: string) => text), - copyToClipboard: vi.fn(async () => undefined), - readImageFromClipboard: vi.fn(async () => undefined), - glob: vi.fn(() => []), - grep: vi.fn(() => ({ matches: [], totalMatches: 0, filesWithMatches: 0, filesSearched: 0 })), - highlightCode: vi.fn((text: string) => text), - supportsLanguage: vi.fn(() => false), - projfsOverlayProbe: vi.fn(async () => false), - projfsOverlayStart: vi.fn(async () => undefined), - projfsOverlayStop: vi.fn(async () => undefined), - astEdit: vi.fn(() => ({ changes: [], totalReplacements: 0 })), - astGrep: vi.fn(() => ({ matches: [], totalMatches: 0 })), - htmlToMarkdown: vi.fn((value: string) => value), - invalidateFsScanCache: vi.fn(), - }, - { - get(target, property) { - if (property in target) { - return target[property as keyof typeof target]; - } - return vi.fn(); - }, - }, -); - -mock.module("@oh-my-pi/pi-natives", () => nativeModuleStub); - function createUsage(): Usage { return { input: 1, @@ -237,10 +199,6 @@ async function createSessionHarness( return { session, authStorage }; } -afterAll(() => { - mock.restore(); -}); - describe("AgentSession OpenAI Responses replay boundaries", () => { const sessions: AgentSession[] = []; const authStorages: AuthStorage[] = []; diff --git a/packages/coding-agent/test/compaction.test.ts b/packages/coding-agent/test/compaction.test.ts index dd0b6d72d..0cedc4132 100644 --- a/packages/coding-agent/test/compaction.test.ts +++ b/packages/coding-agent/test/compaction.test.ts @@ -1,17 +1,12 @@ -import { afterAll, afterEach, beforeEach, describe, expect, it, mock, vi } from "bun:test"; +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; import * as path from "node:path"; import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; +import * as ai from "@oh-my-pi/pi-ai"; import { getBundledModel } from "@oh-my-pi/pi-ai/models"; import { encodeTextSignatureV1 } from "@oh-my-pi/pi-ai/providers/openai-responses-shared"; import type { AssistantMessage, Model, ProviderPayload, Usage } from "@oh-my-pi/pi-ai/types"; import { hookFetch } from "@oh-my-pi/pi-utils"; -const completeSimpleMock = vi.fn(); - -mock.module("@oh-my-pi/pi-ai", () => ({ - completeSimple: completeSimpleMock, -})); - import { type CompactionSettings, calculateContextTokens, @@ -118,12 +113,7 @@ beforeEach(() => { resetEntryCounter(); }); -afterAll(() => { - mock.restore(); -}); - afterEach(() => { - completeSimpleMock.mockReset(); vi.restoreAllMocks(); }); @@ -318,7 +308,8 @@ describe("remote compaction setting", () => { }); if (!preparation) throw new Error("Expected compaction preparation"); - completeSimpleMock + const completeSimpleSpy = vi.spyOn(ai, "completeSimple"); + completeSimpleSpy .mockResolvedValueOnce(createAssistantMessage("History summary")) .mockResolvedValueOnce(createAssistantMessage("Turn prefix summary")) .mockResolvedValueOnce(createAssistantMessage("Short summary")); @@ -327,8 +318,8 @@ describe("remote compaction setting", () => { initiatorOverride: "agent", }); - expect(completeSimpleMock).toHaveBeenCalledTimes(3); - for (const call of completeSimpleMock.mock.calls) { + expect(completeSimpleSpy).toHaveBeenCalledTimes(3); + for (const call of completeSimpleSpy.mock.calls) { const options = call[2] as { initiatorOverride?: string } | undefined; expect(options?.initiatorOverride).toBe("agent"); } @@ -353,15 +344,16 @@ describe("remote compaction setting", () => { }); if (!preparation) throw new Error("Expected compaction preparation"); - completeSimpleMock + const completeSimpleSpy = vi.spyOn(ai, "completeSimple"); + completeSimpleSpy .mockResolvedValueOnce(createAssistantMessage("History summary")) .mockResolvedValueOnce(createAssistantMessage("Turn prefix summary")) .mockResolvedValueOnce(createAssistantMessage("Short summary")); await compact(preparation, model, "test-api-key"); - expect(completeSimpleMock).toHaveBeenCalledTimes(3); - for (const call of completeSimpleMock.mock.calls) { + expect(completeSimpleSpy).toHaveBeenCalledTimes(3); + for (const call of completeSimpleSpy.mock.calls) { const options = call[2] as { initiatorOverride?: string } | undefined; expect(options?.initiatorOverride).toBeUndefined(); } @@ -400,7 +392,8 @@ describe("remote compaction setting", () => { }), ); using _hook = hookFetch(fetchSpy); - const completeSpy = completeSimpleMock + const completeSpy = vi + .spyOn(ai, "completeSimple") .mockResolvedValueOnce(createAssistantMessage("Local history summary")) .mockResolvedValueOnce(createAssistantMessage("Local turn summary")) .mockResolvedValueOnce(createAssistantMessage("Local short summary")); @@ -479,7 +472,8 @@ describe("remote compaction setting", () => { }), ); using _hook = hookFetch(fetchSpy); - completeSimpleMock + const completeSimpleSpy = vi.spyOn(ai, "completeSimple"); + completeSimpleSpy .mockResolvedValueOnce(createAssistantMessage("History summary")) .mockResolvedValueOnce(createAssistantMessage("Turn prefix summary")) .mockResolvedValueOnce(createAssistantMessage("Short summary")); @@ -546,7 +540,7 @@ describe("remote compaction setting", () => { }), ); using _hook = hookFetch(fetchSpy); - completeSimpleMock.mockResolvedValue(createAssistantMessage("Short summary")); + vi.spyOn(ai, "completeSimple").mockResolvedValue(createAssistantMessage("Short summary")); await compact(preparation, model, "test-api-key"); const requestBody = JSON.parse(String(fetchSpy.mock.calls[0]?.[1]?.body)) as { @@ -589,7 +583,7 @@ describe("remote compaction setting", () => { }), ); using _hook = hookFetch(fetchSpy); - completeSimpleMock.mockResolvedValue(createAssistantMessage("Short summary")); + vi.spyOn(ai, "completeSimple").mockResolvedValue(createAssistantMessage("Short summary")); await compact(preparation, model, "test-api-key"); @@ -640,7 +634,7 @@ describe("remote compaction setting", () => { }), ); using _hook = hookFetch(fetchSpy); - completeSimpleMock.mockResolvedValue(createAssistantMessage("Short summary")); + vi.spyOn(ai, "completeSimple").mockResolvedValue(createAssistantMessage("Short summary")); await compact(preparation, model, "test-api-key"); const requestBody = JSON.parse(String(fetchSpy.mock.calls[0]?.[1]?.body)) as { @@ -692,7 +686,7 @@ describe("remote compaction setting", () => { }), ); using _hook = hookFetch(fetchSpy); - completeSimpleMock.mockResolvedValue(createAssistantMessage("Short summary")); + vi.spyOn(ai, "completeSimple").mockResolvedValue(createAssistantMessage("Short summary")); const result = await compact(preparation, model, "test-api-key", undefined, undefined, { remoteInstructions: "BASE INSTRUCTIONS", @@ -747,7 +741,8 @@ describe("remote compaction setting", () => { }); if (!preparation) throw new Error("Expected compaction preparation"); - completeSimpleMock + const completeSimpleSpy = vi.spyOn(ai, "completeSimple"); + completeSimpleSpy .mockResolvedValueOnce(createAssistantMessage("History summary")) .mockResolvedValueOnce(createAssistantMessage("Turn prefix summary")) .mockResolvedValueOnce(createAssistantMessage("Short summary")); diff --git a/packages/coding-agent/test/core/python-kernel.lifecycle.test.ts b/packages/coding-agent/test/core/python-kernel.lifecycle.test.ts index 6e8bdc497..95f1dc5b3 100644 --- a/packages/coding-agent/test/core/python-kernel.lifecycle.test.ts +++ b/packages/coding-agent/test/core/python-kernel.lifecycle.test.ts @@ -4,7 +4,11 @@ import { PythonKernel } from "@oh-my-pi/pi-coding-agent/ipy/kernel"; import { hookFetch, TempDir } from "@oh-my-pi/pi-utils"; import type { Subprocess } from "bun"; -type SpawnOptions = Parameters[1]; +type SpawnOptions = Bun.SpawnOptions.SpawnOptions< + Bun.SpawnOptions.Writable, + Bun.SpawnOptions.Readable, + Bun.SpawnOptions.Readable +>; type FetchCall = { url: string; init?: RequestInit }; @@ -75,10 +79,6 @@ const createFakeProcess = (): Subprocess => { describe("PythonKernel gateway lifecycle", () => { const originalWebSocket = globalThis.WebSocket; - const originalSpawn = Bun.spawn; - const originalSleep = Bun.sleep; - const originalWhich = Bun.which; - const originalExecute = PythonKernel.prototype.execute; const originalGatewayUrl = Bun.env.PI_PYTHON_GATEWAY_URL; const originalGatewayToken = Bun.env.PI_PYTHON_GATEWAY_TOKEN; const originalBunEnv = Bun.env.BUN_ENV; @@ -86,6 +86,39 @@ describe("PythonKernel gateway lifecycle", () => { let tempDir: TempDir; let env: MockEnvironment; + const stubKernelRuntime = () => { + function mockSpawn(options: SpawnOptions & { cmd: string[] }): Subprocess; + function mockSpawn(cmd: string[], options?: SpawnOptions): Subprocess; + function mockSpawn(first: string[] | (SpawnOptions & { cmd: string[] }), second?: SpawnOptions): Subprocess { + if (Array.isArray(first)) { + env.spawnCalls.push({ cmd: first, options: second ?? {} }); + } else { + const { cmd, ...options } = first; + env.spawnCalls.push({ cmd, options }); + } + return createFakeProcess(); + } + + const spawnSpy = vi.spyOn(Bun, "spawn").mockImplementation(mockSpawn); + const sleepSpy = vi.spyOn(Bun, "sleep").mockImplementation(async () => undefined); + const whichSpy = vi.spyOn(Bun, "which").mockImplementation(() => "/usr/bin/python"); + const executeSpy = vi.spyOn(PythonKernel.prototype, "execute").mockResolvedValue({ + status: "ok", + cancelled: false, + timedOut: false, + stdinRequested: false, + }); + + return { + [Symbol.dispose]() { + spawnSpy.mockRestore(); + sleepSpy.mockRestore(); + whichSpy.mockRestore(); + executeSpy.mockRestore(); + }, + }; + }; + beforeEach(() => { tempDir = TempDir.createSync("@omp-python-kernel-"); env = { fetchCalls: [], spawnCalls: [] }; @@ -96,26 +129,6 @@ describe("PythonKernel gateway lifecycle", () => { FakeWebSocket.instances = []; globalThis.WebSocket = FakeWebSocket as unknown as typeof WebSocket; - - Bun.spawn = ((cmd: string[] | string, options?: SpawnOptions) => { - const normalized = Array.isArray(cmd) ? cmd : [cmd]; - env.spawnCalls.push({ cmd: normalized, options: options ?? {} }); - return createFakeProcess(); - }) as typeof Bun.spawn; - - Bun.sleep = (async () => undefined) as typeof Bun.sleep; - - Bun.which = (() => "/usr/bin/python") as typeof Bun.which; - - Object.defineProperty(PythonKernel.prototype, "execute", { - value: (async () => ({ - status: "ok", - cancelled: false, - timedOut: false, - stdinRequested: false, - })) as typeof PythonKernel.prototype.execute, - configurable: true, - }); }); afterEach(() => { @@ -140,14 +153,11 @@ describe("PythonKernel gateway lifecycle", () => { } globalThis.WebSocket = originalWebSocket; - - Bun.spawn = originalSpawn; - Bun.sleep = originalSleep; - Bun.which = originalWhich; - Object.defineProperty(PythonKernel.prototype, "execute", { value: originalExecute, configurable: true }); + vi.restoreAllMocks(); }); it("starts shared gateway, interrupts, and shuts down", async () => { + using _runtime = stubKernelRuntime(); vi.spyOn(gatewayCoordinator, "acquireSharedGateway").mockResolvedValue({ url: "http://127.0.0.1:9999", isShared: true, @@ -177,6 +187,7 @@ describe("PythonKernel gateway lifecycle", () => { }); it("throws when shared gateway kernel creation never succeeds", async () => { + using _runtime = stubKernelRuntime(); vi.spyOn(gatewayCoordinator, "acquireSharedGateway").mockResolvedValue({ url: "http://127.0.0.1:9999", isShared: true, @@ -197,6 +208,7 @@ describe("PythonKernel gateway lifecycle", () => { }); it("does not throw when shutdown API fails", async () => { + using _runtime = stubKernelRuntime(); vi.spyOn(gatewayCoordinator, "acquireSharedGateway").mockResolvedValue({ url: "http://127.0.0.1:9999", isShared: true, diff --git a/packages/coding-agent/test/custom-editor-keybindings.test.ts b/packages/coding-agent/test/custom-editor-keybindings.test.ts index 3c59d23bf..ac8d6b359 100644 --- a/packages/coding-agent/test/custom-editor-keybindings.test.ts +++ b/packages/coding-agent/test/custom-editor-keybindings.test.ts @@ -1,72 +1,6 @@ -import { describe, expect, it, mock, vi } from "bun:test"; +import { describe, expect, it, vi } from "bun:test"; import { defaultEditorTheme } from "../../tui/test/test-themes"; -function createPiNativesMock() { - function parseCtrl(data: string): string | undefined { - if (data.length !== 1) return undefined; - const code = data.charCodeAt(0); - if (code < 1 || code > 26) return undefined; - return `ctrl+${String.fromCharCode(code + 96)}`; - } - - function parseKey(data: string): string | undefined { - if (data === "\x1bp") return "alt+p"; - return parseCtrl(data); - } - - return { - Ellipsis: { Left: "left", Center: "center", Right: "right", Omit: "omit" }, - FileType: { File: "file", Dir: "dir" }, - ImageFormat: { Png: "png", Jpeg: "jpeg", WebP: "webp" }, - KeyEventType: { Press: 1, Repeat: 2, Release: 3 }, - SamplingFilter: { Nearest: "nearest" }, - PhotonImage: class PhotonImage {}, - PtySession: class PtySession {}, - Shell: class Shell {}, - astEdit: vi.fn(), - astGrep: vi.fn(), - copyToClipboard: vi.fn(), - detectMacOSAppearance: vi.fn(), - encodeSixel: vi.fn(), - executeShell: vi.fn(), - extractSegments: vi.fn((text: string) => ({ - before: text, - target: "", - after: "", - beforeWidth: text.length, - targetWidth: 0, - afterWidth: 0, - })), - fuzzyFind: vi.fn(async () => ({ matches: [] })), - getWorkProfile: vi.fn(async () => ({ cpu: [], memory: [] })), - glob: vi.fn(async () => ({ matches: [] })), - grep: vi.fn(async () => ({ matches: [], count: 0, files: [] })), - hasMatch: vi.fn(() => false), - highlightCode: vi.fn((code: string) => code), - htmlToMarkdown: vi.fn((html: string) => html), - invalidateFsScanCache: vi.fn(), - matchesKey: vi.fn((data: string, keyId: string) => parseKey(data) === keyId), - matchesKittySequence: vi.fn(() => false), - matchesLegacySequence: vi.fn(() => false), - parseKey: vi.fn((data: string) => parseKey(data)), - parseKittySequence: vi.fn(() => undefined), - projfsOverlayProbe: vi.fn(), - projfsOverlayStart: vi.fn(), - projfsOverlayStop: vi.fn(), - readImageFromClipboard: vi.fn(async () => null), - sanitizeText: (text: string) => text, - searchContent: vi.fn(async () => ({ matches: [] })), - sliceWithWidth: vi.fn((text: string) => ({ text, width: text.length })), - startMacAppearanceObserver: vi.fn(), - supportsLanguage: vi.fn(() => false), - truncateToWidth: vi.fn((text: string) => text), - visibleWidth: vi.fn((text: string) => text.length), - wrapTextWithAnsi: vi.fn((text: string) => [text]), - }; -} - -mock.module("@oh-my-pi/pi-natives", () => createPiNativesMock()); - function ctrl(key: string): string { return String.fromCharCode(key.toLowerCase().charCodeAt(0) & 31); } diff --git a/packages/coding-agent/test/image-input-normalization.test.ts b/packages/coding-agent/test/image-input-normalization.test.ts index 4d95e9e0e..1b9ef6aeb 100644 --- a/packages/coding-agent/test/image-input-normalization.test.ts +++ b/packages/coding-agent/test/image-input-normalization.test.ts @@ -1,48 +1,30 @@ -import { afterEach, describe, expect, mock, test, vi } from "bun:test"; - -const convertToPngMock = vi.fn(); -const imageInputModulePath = `${import.meta.dir}/../src/utils/image-input.ts`; -const imageConvertModulePath = `${import.meta.dir}/../src/utils/image-convert.ts`; -const imageResizeModulePath = `${import.meta.dir}/../src/utils/image-resize.ts`; -const mimeModulePath = `${import.meta.dir}/../src/utils/mime.ts`; - -async function importImageInputModule() { - mock.module(imageConvertModulePath, () => ({ - convertToPng: convertToPngMock, - })); - mock.module(imageResizeModulePath, () => ({ - formatDimensionNote: () => undefined, - resizeImage: vi.fn(), - })); - mock.module(mimeModulePath, () => ({ - detectSupportedImageMimeTypeFromFile: vi.fn(), - })); - return import(imageInputModulePath); -} +import { afterEach, describe, expect, test, vi } from "bun:test"; +import * as imageConvert from "../src/utils/image-convert"; +import { ensureSupportedImageInput } from "../src/utils/image-input"; describe("ensureSupportedImageInput", () => { afterEach(() => { - convertToPngMock.mockReset(); vi.restoreAllMocks(); }); test("returns supported image input unchanged", async () => { - const { ensureSupportedImageInput } = await importImageInputModule(); + const convertToPngSpy = vi.spyOn(imageConvert, "convertToPng"); const input = { type: "image" as const, data: "abc", mimeType: "image/png" }; const result = await ensureSupportedImageInput(input); expect(result).toEqual(input); - expect(convertToPngMock).not.toHaveBeenCalled(); + expect(convertToPngSpy).not.toHaveBeenCalled(); }); test("converts unsupported image input to png", async () => { - convertToPngMock.mockResolvedValue({ type: "image", data: "pngdata", mimeType: "image/png" }); - const { ensureSupportedImageInput } = await importImageInputModule(); + const convertToPngSpy = vi + .spyOn(imageConvert, "convertToPng") + .mockResolvedValue({ data: "pngdata", mimeType: "image/png" }); const result = await ensureSupportedImageInput({ type: "image", data: "bmpdata", mimeType: "image/bmp" }); - expect(convertToPngMock).toHaveBeenCalledWith("bmpdata", "image/bmp"); + expect(convertToPngSpy).toHaveBeenCalledWith("bmpdata", "image/bmp"); expect(result).toEqual({ type: "image", data: "pngdata", mimeType: "image/png" }); }); }); diff --git a/packages/coding-agent/test/input-controller-keybindings.test.ts b/packages/coding-agent/test/input-controller-keybindings.test.ts index 1e0b17e24..010b4240f 100644 --- a/packages/coding-agent/test/input-controller-keybindings.test.ts +++ b/packages/coding-agent/test/input-controller-keybindings.test.ts @@ -1,72 +1,6 @@ -import { describe, expect, it, mock, vi } from "bun:test"; +import { describe, expect, it, vi } from "bun:test"; import type { InteractiveModeContext } from "../src/modes/types"; -function createPiNativesMock() { - function parseCtrl(data: string): string | undefined { - if (data.length !== 1) return undefined; - const code = data.charCodeAt(0); - if (code < 1 || code > 26) return undefined; - return `ctrl+${String.fromCharCode(code + 96)}`; - } - - function parseKey(data: string): string | undefined { - if (data === "\x1bp") return "alt+p"; - return parseCtrl(data); - } - - return { - Ellipsis: { Left: "left", Center: "center", Right: "right", Omit: "omit" }, - FileType: { File: "file", Dir: "dir" }, - ImageFormat: { Png: "png", Jpeg: "jpeg", WebP: "webp" }, - KeyEventType: { Press: 1, Repeat: 2, Release: 3 }, - SamplingFilter: { Nearest: "nearest" }, - PhotonImage: class PhotonImage {}, - PtySession: class PtySession {}, - Shell: class Shell {}, - astEdit: vi.fn(), - astGrep: vi.fn(), - copyToClipboard: vi.fn(), - detectMacOSAppearance: vi.fn(), - encodeSixel: vi.fn(), - executeShell: vi.fn(), - extractSegments: vi.fn((text: string) => ({ - before: text, - target: "", - after: "", - beforeWidth: text.length, - targetWidth: 0, - afterWidth: 0, - })), - fuzzyFind: vi.fn(async () => ({ matches: [] })), - getWorkProfile: vi.fn(async () => ({ cpu: [], memory: [] })), - glob: vi.fn(async () => ({ matches: [] })), - grep: vi.fn(async () => ({ matches: [], count: 0, files: [] })), - hasMatch: vi.fn(() => false), - highlightCode: vi.fn((code: string) => code), - htmlToMarkdown: vi.fn((html: string) => html), - invalidateFsScanCache: vi.fn(), - matchesKey: vi.fn((data: string, keyId: string) => parseKey(data) === keyId), - matchesKittySequence: vi.fn(() => false), - matchesLegacySequence: vi.fn(() => false), - parseKey: vi.fn((data: string) => parseKey(data)), - parseKittySequence: vi.fn(() => undefined), - projfsOverlayProbe: vi.fn(), - projfsOverlayStart: vi.fn(), - projfsOverlayStop: vi.fn(), - readImageFromClipboard: vi.fn(async () => null), - sanitizeText: (text: string) => text, - searchContent: vi.fn(async () => ({ matches: [] })), - sliceWithWidth: vi.fn((text: string) => ({ text, width: text.length })), - startMacAppearanceObserver: vi.fn(), - supportsLanguage: vi.fn(() => false), - truncateToWidth: vi.fn((text: string) => text), - visibleWidth: vi.fn((text: string) => text.length), - wrapTextWithAnsi: vi.fn((text: string) => [text]), - }; -} - -mock.module("@oh-my-pi/pi-natives", () => createPiNativesMock()); - type FakeEditor = { onEscape?: () => void; shouldBypassAutocompleteOnEscape?: () => boolean; diff --git a/packages/coding-agent/test/session-storage.test.ts b/packages/coding-agent/test/session-storage.test.ts index 96911b40d..e19ba7ab4 100644 --- a/packages/coding-agent/test/session-storage.test.ts +++ b/packages/coding-agent/test/session-storage.test.ts @@ -1,14 +1,9 @@ -import { afterEach, beforeEach, describe, expect, it, mock, vi } from "bun:test"; +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs"; import * as fsp from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; -mock.module("@oh-my-pi/pi-utils", () => ({ - isEnoent: (err: unknown) => typeof err === "object" && err !== null && "code" in err && err.code === "ENOENT", - toError: (err: unknown) => (err instanceof Error ? err : new Error(String(err))), -})); - describe("FileSessionStorage.deleteSessionWithArtifacts", () => { let tempDir: string; let storage: { deleteSessionWithArtifacts(sessionPath: string): Promise }; diff --git a/packages/coding-agent/test/theme-auto-detection.test.ts b/packages/coding-agent/test/theme-auto-detection.test.ts index 0f2af777b..7661d831d 100644 --- a/packages/coding-agent/test/theme-auto-detection.test.ts +++ b/packages/coding-agent/test/theme-auto-detection.test.ts @@ -6,11 +6,44 @@ const originalPlatform = process.platform; const originalColorfgbg = Bun.env.COLORFGBG; const originalZellij = Bun.env.ZELLIJ; +type ThemeTestGlobals = { + platform?: NodeJS.Platform; + colorfgbg?: string; + zellij?: string; +}; + +const withThemeTestGlobals = (globals: ThemeTestGlobals = {}) => { + Object.defineProperty(process, "platform", { + value: globals.platform ?? "darwin", + configurable: true, + writable: true, + }); + + if (globals.colorfgbg === undefined) delete Bun.env.COLORFGBG; + else Bun.env.COLORFGBG = globals.colorfgbg; + + if (globals.zellij === undefined) delete Bun.env.ZELLIJ; + else Bun.env.ZELLIJ = globals.zellij; + + return { + [Symbol.dispose]() { + themeModule.stopThemeWatcher(); + Object.defineProperty(process, "platform", { + value: originalPlatform, + configurable: true, + writable: true, + }); + if (originalColorfgbg === undefined) delete Bun.env.COLORFGBG; + else Bun.env.COLORFGBG = originalColorfgbg; + if (originalZellij === undefined) delete Bun.env.ZELLIJ; + else Bun.env.ZELLIJ = originalZellij; + vi.restoreAllMocks(); + }, + }; +}; + describe("theme auto-detection", () => { beforeEach(async () => { - Object.defineProperty(process, "platform", { value: "darwin", configurable: true, writable: true }); - delete Bun.env.COLORFGBG; - delete Bun.env.ZELLIJ; themeModule.stopThemeWatcher(); const darkTheme = await themeModule.getThemeByName("dark"); if (!darkTheme) { @@ -22,21 +55,11 @@ describe("theme auto-detection", () => { afterEach(() => { themeModule.stopThemeWatcher(); - Object.defineProperty(process, "platform", { - value: originalPlatform, - configurable: true, - writable: true, - }); - if (originalColorfgbg === undefined) delete Bun.env.COLORFGBG; - else Bun.env.COLORFGBG = originalColorfgbg; - if (originalZellij === undefined) delete Bun.env.ZELLIJ; - else Bun.env.ZELLIJ = originalZellij; vi.restoreAllMocks(); }); it("prefers COLORFGBG before macOS fallback inside Zellij", async () => { - Bun.env.ZELLIJ = "1"; - Bun.env.COLORFGBG = "15;0"; + using _globals = withThemeTestGlobals({ zellij: "1", colorfgbg: "15;0" }); const detectSpy = vi.spyOn(nativesModule, "detectMacOSAppearance").mockReturnValue("light"); await themeModule.initTheme(false, undefined, undefined, "dark", "light"); @@ -46,6 +69,7 @@ describe("theme auto-detection", () => { }); it("keeps honoring terminal-reported appearance outside fallback mode", async () => { + using _globals = withThemeTestGlobals(); const detectSpy = vi.spyOn(nativesModule, "detectMacOSAppearance").mockReturnValue("light"); const observerSpy = vi.spyOn(nativesModule, "startMacAppearanceObserver"); @@ -58,7 +82,7 @@ describe("theme auto-detection", () => { }); it("updates auto theme from the native fallback observer in Zellij", async () => { - Bun.env.ZELLIJ = "1"; + using _globals = withThemeTestGlobals({ zellij: "1" }); const stop = vi.fn(); let onAppearanceChange: ((appearance: "dark" | "light") => void) | undefined; vi.spyOn(nativesModule, "detectMacOSAppearance").mockReturnValue("light"); @@ -81,8 +105,7 @@ describe("theme auto-detection", () => { expect(stop).toHaveBeenCalledTimes(1); }); it("Zellij fallback stays macOS-only (Linux + Zellij = honor terminal)", async () => { - Object.defineProperty(process, "platform", { value: "linux", configurable: true, writable: true }); - Bun.env.ZELLIJ = "1"; + using _globals = withThemeTestGlobals({ platform: "linux", zellij: "1" }); const detectSpy = vi.spyOn(nativesModule, "detectMacOSAppearance").mockReturnValue("light"); themeModule.onTerminalAppearanceChange("dark"); @@ -93,7 +116,7 @@ describe("theme auto-detection", () => { }); it("terminal-reported appearance wins over conflicting COLORFGBG", async () => { - Bun.env.COLORFGBG = "15;0"; + using _globals = withThemeTestGlobals({ colorfgbg: "15;0" }); const detectSpy = vi.spyOn(nativesModule, "detectMacOSAppearance").mockReturnValue("light"); themeModule.onTerminalAppearanceChange("light"); diff --git a/packages/coding-agent/test/tools/fetch-kagi-toggle.test.ts b/packages/coding-agent/test/tools/fetch-kagi-toggle.test.ts index 3e47544c9..fd315cafc 100644 --- a/packages/coding-agent/test/tools/fetch-kagi-toggle.test.ts +++ b/packages/coding-agent/test/tools/fetch-kagi-toggle.test.ts @@ -12,6 +12,15 @@ import * as scraperUtils from "@oh-my-pi/pi-coding-agent/web/scrapers/utils"; import * as natives from "@oh-my-pi/pi-natives"; import { hookFetch, ptree, Snowflake } from "@oh-my-pi/pi-utils"; +const withMissingSystemPython = () => { + const whichSpy = vi.spyOn(Bun, "which").mockImplementation(() => null); + return { + [Symbol.dispose]() { + whichSpy.mockRestore(); + }, + }; +}; + describe("fetch tool", () => { let testDir: string; @@ -325,41 +334,19 @@ describe("fetch tool", () => { const pageUrl = "https://bun.com/reference/bun/UnixSocketOptions"; const pageHtml = "

UnixSocketOptions

Page-specific docs.

"; const renderedMarkdown = `# UnixSocketOptions\n\n${"Page-specific API docs. ".repeat(8)}`; - const originalWhich = Bun.which; - - Bun.which = (() => null) as typeof Bun.which; - try { - const loadPageSpy = vi.spyOn(scrapers, "loadPage").mockImplementation(async (requestedUrl: string) => { - if (requestedUrl === pageUrl) { - return { - ok: true, - status: 200, - contentType: "text/html", - finalUrl: pageUrl, - content: pageHtml, - }; - } - - if (requestedUrl === `${pageUrl}.md`) { - return { - ok: false, - status: 404, - contentType: "text/plain", - finalUrl: requestedUrl, - content: "", - }; - } - - if (requestedUrl === "https://bun.com/llms.txt") { - return { - ok: true, - status: 200, - contentType: "text/plain", - finalUrl: requestedUrl, - content: `# Bun\n\n${"Site-wide overview. ".repeat(12)}`, - }; - } + using _missingSystemPython = withMissingSystemPython(); + const loadPageSpy = vi.spyOn(scrapers, "loadPage").mockImplementation(async (requestedUrl: string) => { + if (requestedUrl === pageUrl) { + return { + ok: true, + status: 200, + contentType: "text/html", + finalUrl: pageUrl, + content: pageHtml, + }; + } + if (requestedUrl === `${pageUrl}.md`) { return { ok: false, status: 404, @@ -367,24 +354,40 @@ describe("fetch tool", () => { finalUrl: requestedUrl, content: "", }; - }); - using _hook = hookFetch(() => new Response("blocked", { status: 500, statusText: "Blocked" })); - vi.spyOn(toolsManager, "ensureTool").mockResolvedValue(undefined); - vi.spyOn(natives, "htmlToMarkdown").mockResolvedValue(renderedMarkdown); + } - const result = await tool.execute("fetch-deep-page", { url: pageUrl }); - const requestedUrls = loadPageSpy.mock.calls.map(([requestedUrl]) => requestedUrl); - const textBlock = result.content.find(content => content.type === "text"); + if (requestedUrl === "https://bun.com/llms.txt") { + return { + ok: true, + status: 200, + contentType: "text/plain", + finalUrl: requestedUrl, + content: `# Bun\n\n${"Site-wide overview. ".repeat(12)}`, + }; + } - expect(result.details?.method).toBe("native"); - expect(textBlock?.type).toBe("text"); - expect(textBlock?.text).toContain("UnixSocketOptions"); - expect(requestedUrls).not.toContain("https://bun.com/.well-known/llms.txt"); - expect(requestedUrls).not.toContain("https://bun.com/llms.txt"); - expect(requestedUrls).not.toContain("https://bun.com/llms.md"); - } finally { - Bun.which = originalWhich; - } + return { + ok: false, + status: 404, + contentType: "text/plain", + finalUrl: requestedUrl, + content: "", + }; + }); + using _hook = hookFetch(() => new Response("blocked", { status: 500, statusText: "Blocked" })); + vi.spyOn(toolsManager, "ensureTool").mockResolvedValue(undefined); + vi.spyOn(natives, "htmlToMarkdown").mockResolvedValue(renderedMarkdown); + + const result = await tool.execute("fetch-deep-page", { url: pageUrl }); + const requestedUrls = loadPageSpy.mock.calls.map(([requestedUrl]) => requestedUrl); + const textBlock = result.content.find(content => content.type === "text"); + + expect(result.details?.method).toBe("native"); + expect(textBlock?.type).toBe("text"); + expect(textBlock?.text).toContain("UnixSocketOptions"); + expect(requestedUrls).not.toContain("https://bun.com/.well-known/llms.txt"); + expect(requestedUrls).not.toContain("https://bun.com/llms.txt"); + expect(requestedUrls).not.toContain("https://bun.com/llms.md"); }); it("uses section-scoped llms.txt fallback without requesting the site-wide file", async () => { @@ -393,59 +396,27 @@ describe("fetch tool", () => { const pageUrl = "https://example.com/docs/reference/widget"; const pageHtml = "

Widget

"; const lowQualityRender = `${"Please enable JavaScript to view this page.\n".repeat(6)}${"navigation\n".repeat(4)}`; - const originalWhich = Bun.which; - const execSpy = vi.spyOn(ptree, "exec").mockResolvedValue({ ok: true, stdout: lowQualityRender } as never); - - Bun.which = (() => null) as typeof Bun.which; - try { - const loadPageSpy = vi.spyOn(scrapers, "loadPage").mockImplementation(async (requestedUrl: string) => { - if (requestedUrl === pageUrl) { - return { - ok: true, - status: 200, - contentType: "text/html", - finalUrl: pageUrl, - content: pageHtml, - }; - } - - if ( - [ - `${pageUrl}.md`, - "https://example.com/docs/reference/llms.txt", - "https://example.com/docs/reference/llms.md", - "https://example.com/docs/llms.md", - ].includes(requestedUrl) - ) { - return { - ok: false, - status: 404, - contentType: "text/plain", - finalUrl: requestedUrl, - content: "", - }; - } - - if (requestedUrl === "https://example.com/docs/llms.txt") { - return { - ok: true, - status: 200, - contentType: "text/plain", - finalUrl: requestedUrl, - content: `# Example Docs\n\n${"Section-scoped fallback. ".repeat(10)}`, - }; - } - - if (requestedUrl === "https://example.com/llms.txt") { - return { - ok: true, - status: 200, - contentType: "text/plain", - finalUrl: requestedUrl, - content: `# Example\n\n${"Site-wide fallback. ".repeat(10)}`, - }; - } + using _missingSystemPython = withMissingSystemPython(); + const _execSpy = vi.spyOn(ptree, "exec").mockResolvedValue({ ok: true, stdout: lowQualityRender } as never); + const loadPageSpy = vi.spyOn(scrapers, "loadPage").mockImplementation(async (requestedUrl: string) => { + if (requestedUrl === pageUrl) { + return { + ok: true, + status: 200, + contentType: "text/html", + finalUrl: pageUrl, + content: pageHtml, + }; + } + if ( + [ + `${pageUrl}.md`, + "https://example.com/docs/reference/llms.txt", + "https://example.com/docs/reference/llms.md", + "https://example.com/docs/llms.md", + ].includes(requestedUrl) + ) { return { ok: false, status: 404, @@ -453,26 +424,51 @@ describe("fetch tool", () => { finalUrl: requestedUrl, content: "", }; - }); - using _hook = hookFetch(() => new Response("blocked", { status: 500, statusText: "Blocked" })); - vi.spyOn(toolsManager, "ensureTool").mockResolvedValue("/usr/bin/trafilatura"); + } - const result = await tool.execute("fetch-section-llms", { url: pageUrl }); - const requestedUrls = loadPageSpy.mock.calls.map(([requestedUrl]) => requestedUrl); - const textBlock = result.content.find(content => content.type === "text"); + if (requestedUrl === "https://example.com/docs/llms.txt") { + return { + ok: true, + status: 200, + contentType: "text/plain", + finalUrl: requestedUrl, + content: `# Example Docs\n\n${"Section-scoped fallback. ".repeat(10)}`, + }; + } - expect(result.details?.method).toBe("llms.txt"); - expect(result.details?.notes).toContain("Used llms.txt fallback: https://example.com/docs/llms.txt"); - expect(textBlock?.type).toBe("text"); - expect(textBlock?.text).toContain("Section-scoped fallback"); - expect(requestedUrls).toContain("https://example.com/docs/llms.txt"); - expect(requestedUrls).not.toContain("https://example.com/.well-known/llms.txt"); - expect(requestedUrls).not.toContain("https://example.com/llms.txt"); - expect(requestedUrls).not.toContain("https://example.com/llms.md"); - } finally { - Bun.which = originalWhich; - execSpy.mockRestore(); - } + if (requestedUrl === "https://example.com/llms.txt") { + return { + ok: true, + status: 200, + contentType: "text/plain", + finalUrl: requestedUrl, + content: `# Example\n\n${"Site-wide fallback. ".repeat(10)}`, + }; + } + + return { + ok: false, + status: 404, + contentType: "text/plain", + finalUrl: requestedUrl, + content: "", + }; + }); + using _hook = hookFetch(() => new Response("blocked", { status: 500, statusText: "Blocked" })); + vi.spyOn(toolsManager, "ensureTool").mockResolvedValue("/usr/bin/trafilatura"); + + const result = await tool.execute("fetch-section-llms", { url: pageUrl }); + const requestedUrls = loadPageSpy.mock.calls.map(([requestedUrl]) => requestedUrl); + const textBlock = result.content.find(content => content.type === "text"); + + expect(result.details?.method).toBe("llms.txt"); + expect(result.details?.notes).toContain("Used llms.txt fallback: https://example.com/docs/llms.txt"); + expect(textBlock?.type).toBe("text"); + expect(textBlock?.text).toContain("Section-scoped fallback"); + expect(requestedUrls).toContain("https://example.com/docs/llms.txt"); + expect(requestedUrls).not.toContain("https://example.com/.well-known/llms.txt"); + expect(requestedUrls).not.toContain("https://example.com/llms.txt"); + expect(requestedUrls).not.toContain("https://example.com/llms.md"); }); it("prefers Parallel extract before other HTML renderers when configured", async () => { process.env.PARALLEL_API_KEY = "test-parallel-key";