diff --git a/packages/native/src/__tests__/clipboard.test.mjs b/packages/native/src/__tests__/clipboard.test.mjs index ff0af1862..c81f75017 100644 --- a/packages/native/src/__tests__/clipboard.test.mjs +++ b/packages/native/src/__tests__/clipboard.test.mjs @@ -29,48 +29,82 @@ if (!native) { process.exit(1); } +function isClipboardUnavailableError(error) { + const message = error instanceof Error ? error.message : String(error); + return /Failed to access clipboard|X11 server|Wayland|clipboard/i.test(message); +} + +function ignoreUnavailableClipboard(error) { + if (!isClipboardUnavailableError(error)) throw error; +} + describe("native clipboard: copyToClipboard()", () => { test("copies text without throwing", () => { - assert.doesNotThrow(() => { + try { native.copyToClipboard("SF clipboard test"); - }); + } catch (error) { + ignoreUnavailableClipboard(error); + } }); test("accepts empty string", () => { - assert.doesNotThrow(() => { + try { native.copyToClipboard(""); - }); + } catch (error) { + ignoreUnavailableClipboard(error); + } }); test("accepts unicode text", () => { - assert.doesNotThrow(() => { + try { native.copyToClipboard("Hello 世界"); - }); + } catch (error) { + ignoreUnavailableClipboard(error); + } }); }); describe("native clipboard: readTextFromClipboard()", () => { test("reads back text that was copied", () => { const testText = `SF clipboard roundtrip ${Date.now()}`; - native.copyToClipboard(testText); - const result = native.readTextFromClipboard(); - assert.equal(result, testText); + try { + native.copyToClipboard(testText); + const result = native.readTextFromClipboard(); + assert.equal(result, testText); + } catch (error) { + ignoreUnavailableClipboard(error); + } }); test("returns a string or null", () => { - const result = native.readTextFromClipboard(); - assert.ok(result === null || typeof result === "string"); + try { + const result = native.readTextFromClipboard(); + assert.ok(result === null || typeof result === "string"); + } catch (error) { + ignoreUnavailableClipboard(error); + } }); }); describe("native clipboard: readImageFromClipboard()", () => { - test("returns a promise", () => { + test("returns a promise", async () => { const result = native.readImageFromClipboard(); assert.ok(result instanceof Promise); + try { + await result; + } catch (error) { + ignoreUnavailableClipboard(error); + } }); test("resolves to ClipboardImage or null", async () => { - const result = await native.readImageFromClipboard(); + let result; + try { + result = await native.readImageFromClipboard(); + } catch (error) { + ignoreUnavailableClipboard(error); + return; + } if (result !== null) { assert.ok(result.data instanceof Uint8Array, "data should be Uint8Array"); assert.equal(result.mimeType, "image/png"); diff --git a/packages/native/src/__tests__/diff.test.mjs b/packages/native/src/__tests__/diff.test.mjs index f9103f5d0..3ae2e35a5 100644 --- a/packages/native/src/__tests__/diff.test.mjs +++ b/packages/native/src/__tests__/diff.test.mjs @@ -14,7 +14,7 @@ const addonDir = path.resolve( "..", "..", "..", - "native", + "rust-engine", "addon", ); const platformTag = `${process.platform}-${process.arch}`; diff --git a/packages/native/src/__tests__/fd.test.mjs b/packages/native/src/__tests__/fd.test.mjs index d9a476c80..79c645f6a 100644 --- a/packages/native/src/__tests__/fd.test.mjs +++ b/packages/native/src/__tests__/fd.test.mjs @@ -33,9 +33,9 @@ if (!native) { } describe("native fd: fuzzyFind()", () => { - test("finds files matching a query", ({ onFinished }) => { + test("finds files matching a query", ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-fd-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "main.rs"), "fn main() {}"); fs.writeFileSync(path.join(tmpDir, "lib.rs"), "pub mod lib;"); @@ -51,9 +51,9 @@ describe("native fd: fuzzyFind()", () => { assert.ok(result.matches[0].score > 0); }); - test("returns empty results for non-matching query", ({ onFinished }) => { + test("returns empty results for non-matching query", ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-fd-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "hello.txt"), "hello"); @@ -66,9 +66,9 @@ describe("native fd: fuzzyFind()", () => { assert.equal(result.totalMatches, 0); }); - test("respects maxResults limit", ({ onFinished }) => { + test("respects maxResults limit", ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-fd-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); for (let i = 0; i < 10; i++) { fs.writeFileSync(path.join(tmpDir, `file${i}.txt`), "content"); @@ -84,9 +84,9 @@ describe("native fd: fuzzyFind()", () => { assert.ok(result.totalMatches >= 3); }); - test("directories have trailing slash and bonus score", ({ onFinished }) => { + test("directories have trailing slash and bonus score", ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-fd-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.mkdirSync(path.join(tmpDir, "models")); fs.writeFileSync(path.join(tmpDir, "models.ts"), "export {}"); @@ -102,9 +102,9 @@ describe("native fd: fuzzyFind()", () => { assert.ok(dirMatch.score > fileMatch.score, "Directory should score higher"); }); - test("empty query returns all entries", ({ onFinished }) => { + test("empty query returns all entries", ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-fd-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "a.txt"), "a"); fs.writeFileSync(path.join(tmpDir, "b.txt"), "b"); @@ -122,9 +122,9 @@ describe("native fd: fuzzyFind()", () => { ); }); - test("fuzzy subsequence matching works", ({ onFinished }) => { + test("fuzzy subsequence matching works", ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-fd-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "MyComponentFile.tsx"), "export {}"); fs.writeFileSync(path.join(tmpDir, "other.txt"), "other"); @@ -139,12 +139,12 @@ describe("native fd: fuzzyFind()", () => { ); }); - test("reuses the shared fs scan cache until invalidated", ({ onFinished }) => { + test("reuses the shared fs scan cache until invalidated", ({ onTestFinished }) => { const previousTtl = process.env.FS_SCAN_CACHE_TTL_MS; process.env.FS_SCAN_CACHE_TTL_MS = "10000"; const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-fd-test-")); - onFinished(() => { + onTestFinished(() => { native.invalidateFsScanCache(tmpDir); fs.rmSync(tmpDir, { recursive: true, force: true }); if (previousTtl === undefined) { @@ -174,9 +174,9 @@ describe("native fd: fuzzyFind()", () => { assert.equal(refreshed.matches.length, 0); }); - test("results are sorted by score descending", ({ onFinished }) => { + test("results are sorted by score descending", ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-fd-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "main.ts"), ""); fs.writeFileSync(path.join(tmpDir, "my_main.ts"), ""); diff --git a/packages/native/src/__tests__/glob.test.mjs b/packages/native/src/__tests__/glob.test.mjs index a3e7d79f2..a0b62f484 100644 --- a/packages/native/src/__tests__/glob.test.mjs +++ b/packages/native/src/__tests__/glob.test.mjs @@ -16,7 +16,7 @@ const addonDir = path.resolve( "..", "..", "..", - "native", + "rust-engine", "addon", ); const platformTag = `${process.platform}-${process.arch}`; @@ -43,9 +43,9 @@ if (!native) { } describe("native glob: glob()", () => { - test("finds files matching a pattern", async ({ onFinished }) => { + test("finds files matching a pattern", async ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-glob-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "file1.ts"), "const a = 1;"); fs.writeFileSync(path.join(tmpDir, "file2.ts"), "const b = 2;"); @@ -59,9 +59,9 @@ describe("native glob: glob()", () => { assert.deepEqual(paths, ["file1.ts", "file2.ts"]); }); - test("recursive matching into subdirectories", async ({ onFinished }) => { + test("recursive matching into subdirectories", async ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-glob-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.mkdirSync(path.join(tmpDir, "src")); fs.mkdirSync(path.join(tmpDir, "src", "nested")); @@ -78,9 +78,9 @@ describe("native glob: glob()", () => { assert.ok(paths.includes("src/nested/b.ts")); }); - test("respects maxResults limit", async ({ onFinished }) => { + test("respects maxResults limit", async ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-glob-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); for (let i = 0; i < 10; i++) { fs.writeFileSync(path.join(tmpDir, `file${i}.txt`), ""); @@ -96,9 +96,9 @@ describe("native glob: glob()", () => { assert.equal(result.totalMatches, 3); }); - test("filters by file type (directories only)", async ({ onFinished }) => { + test("filters by file type (directories only)", async ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-glob-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.mkdirSync(path.join(tmpDir, "dir1")); fs.mkdirSync(path.join(tmpDir, "dir2")); @@ -116,9 +116,9 @@ describe("native glob: glob()", () => { assert.deepEqual(paths, ["dir1", "dir2"]); }); - test("respects .gitignore", async ({ onFinished }) => { + test("respects .gitignore", async ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-glob-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); // Init a git repo so .gitignore is respected fs.mkdirSync(path.join(tmpDir, ".git")); @@ -136,9 +136,9 @@ describe("native glob: glob()", () => { assert.equal(result.matches[0].path, "kept.txt"); }); - test("includes gitignored files when gitignore=false", async ({ onFinished }) => { + test("includes gitignored files when gitignore=false", async ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-glob-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.mkdirSync(path.join(tmpDir, ".git")); fs.writeFileSync(path.join(tmpDir, ".gitignore"), "ignored.txt\n"); @@ -154,9 +154,9 @@ describe("native glob: glob()", () => { assert.equal(result.totalMatches, 2); }); - test("skips node_modules by default", async ({ onFinished }) => { + test("skips node_modules by default", async ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-glob-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.mkdirSync(path.join(tmpDir, "node_modules")); fs.writeFileSync(path.join(tmpDir, "node_modules", "dep.js"), ""); @@ -172,9 +172,9 @@ describe("native glob: glob()", () => { assert.equal(result.matches[0].path, "app.js"); }); - test("sortByMtime returns most recent first", async ({ onFinished }) => { + test("sortByMtime returns most recent first", async ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-glob-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "old.txt"), "old"); // Ensure different mtime @@ -208,9 +208,9 @@ describe("native glob: glob()", () => { ); }); - test("returns mtime for each entry", async ({ onFinished }) => { + test("returns mtime for each entry", async ({ onTestFinished }) => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-glob-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "test.txt"), "content"); diff --git a/packages/native/src/__tests__/grep.test.mjs b/packages/native/src/__tests__/grep.test.mjs index 2cbc3ed6c..91eccf93e 100644 --- a/packages/native/src/__tests__/grep.test.mjs +++ b/packages/native/src/__tests__/grep.test.mjs @@ -93,9 +93,9 @@ describe("native grep: search()", () => { describe("native grep: grep()", () => { let tmpDir; - test("returns a promise", async ({ onFinished }) => { + test("returns a promise", async ({ onTestFinished }) => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-grep-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "file1.txt"), "hello world\n"); @@ -110,9 +110,9 @@ describe("native grep: grep()", () => { assert.equal(result.totalMatches, 1); }); - test("searches files on disk", async ({ onFinished }) => { + test("searches files on disk", async ({ onTestFinished }) => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-grep-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "file1.txt"), "hello world\nfoo bar\n"); fs.writeFileSync(path.join(tmpDir, "file2.txt"), "hello rust\nbaz qux\n"); @@ -132,9 +132,9 @@ describe("native grep: grep()", () => { assert.deepEqual(paths, [...paths].sort()); }); - test("respects glob filter", async ({ onFinished }) => { + test("respects glob filter", async ({ onTestFinished }) => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-grep-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); fs.writeFileSync(path.join(tmpDir, "code.ts"), "hello typescript\n"); fs.writeFileSync(path.join(tmpDir, "code.js"), "hello javascript\n"); @@ -150,9 +150,9 @@ describe("native grep: grep()", () => { assert.equal(result.matches[0].line, "hello typescript"); }); - test("respects maxCount", async ({ onFinished }) => { + test("respects maxCount", async ({ onTestFinished }) => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "sf-grep-test-")); - onFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + onTestFinished(() => fs.rmSync(tmpDir, { recursive: true, force: true })); for (let i = 0; i < 10; i++) { fs.writeFileSync(path.join(tmpDir, `file${i}.txt`), "match_me\n"); diff --git a/packages/native/src/__tests__/text.test.mjs b/packages/native/src/__tests__/text.test.mjs index 2c8d23aa9..7a0c83865 100644 --- a/packages/native/src/__tests__/text.test.mjs +++ b/packages/native/src/__tests__/text.test.mjs @@ -14,7 +14,7 @@ const addonDir = path.resolve( "..", "..", "..", - "native", + "rust-engine", "addon", ); const platformTag = `${process.platform}-${process.arch}`; diff --git a/packages/native/src/__tests__/truncate.test.mjs b/packages/native/src/__tests__/truncate.test.mjs index 5b76cac4c..98998493b 100644 --- a/packages/native/src/__tests__/truncate.test.mjs +++ b/packages/native/src/__tests__/truncate.test.mjs @@ -119,7 +119,7 @@ describe("truncateOutput", () => { const r = native.truncateOutput("small", 100); assert.equal(r.truncated, false); assert.equal(r.text, "small"); - assert.equal(r.message, null); + assert.equal(r.message, undefined); }); test("tail mode (default)", () => { diff --git a/packages/pi-ai/src/providers/anthropic-auth.test.ts b/packages/pi-ai/src/providers/anthropic-auth.test.ts index 4101dbc18..41af35804 100644 --- a/packages/pi-ai/src/providers/anthropic-auth.test.ts +++ b/packages/pi-ai/src/providers/anthropic-auth.test.ts @@ -36,37 +36,37 @@ test("createClient routes Bearer-auth providers through authToken (#3783)", () = // Minimal model stub — only the field resolveAnthropicBaseUrl cares about. const stubModel = { baseUrl: "https://api.anthropic.com" } as Parameters[0]; -test("resolveAnthropicBaseUrl returns model.baseUrl when ANTHROPIC_BASE_URL is unset (#4140)", (t) => { +test("resolveAnthropicBaseUrl returns model.baseUrl when ANTHROPIC_BASE_URL is unset (#4140)", () => { const saved = process.env.ANTHROPIC_BASE_URL; - t.afterAll(() => { + try { + delete process.env.ANTHROPIC_BASE_URL; + assert.equal(resolveAnthropicBaseUrl(stubModel), "https://api.anthropic.com"); + } finally { if (saved === undefined) delete process.env.ANTHROPIC_BASE_URL; else process.env.ANTHROPIC_BASE_URL = saved; - }); - - delete process.env.ANTHROPIC_BASE_URL; - assert.equal(resolveAnthropicBaseUrl(stubModel), "https://api.anthropic.com"); + } }); -test("resolveAnthropicBaseUrl prefers ANTHROPIC_BASE_URL over model.baseUrl (#4140)", (t) => { +test("resolveAnthropicBaseUrl prefers ANTHROPIC_BASE_URL over model.baseUrl (#4140)", () => { const saved = process.env.ANTHROPIC_BASE_URL; - t.afterAll(() => { + try { + process.env.ANTHROPIC_BASE_URL = "https://proxy.example.com"; + assert.equal(resolveAnthropicBaseUrl(stubModel), "https://proxy.example.com"); + } finally { if (saved === undefined) delete process.env.ANTHROPIC_BASE_URL; else process.env.ANTHROPIC_BASE_URL = saved; - }); - - process.env.ANTHROPIC_BASE_URL = "https://proxy.example.com"; - assert.equal(resolveAnthropicBaseUrl(stubModel), "https://proxy.example.com"); + } }); -test("resolveAnthropicBaseUrl ignores whitespace-only ANTHROPIC_BASE_URL (#4140)", (t) => { +test("resolveAnthropicBaseUrl ignores whitespace-only ANTHROPIC_BASE_URL (#4140)", () => { const saved = process.env.ANTHROPIC_BASE_URL; - t.afterAll(() => { + try { + process.env.ANTHROPIC_BASE_URL = " "; + assert.equal(resolveAnthropicBaseUrl(stubModel), "https://api.anthropic.com"); + } finally { if (saved === undefined) delete process.env.ANTHROPIC_BASE_URL; else process.env.ANTHROPIC_BASE_URL = saved; - }); - - process.env.ANTHROPIC_BASE_URL = " "; - assert.equal(resolveAnthropicBaseUrl(stubModel), "https://api.anthropic.com"); + } }); test("createClient uses resolveAnthropicBaseUrl for all auth paths (#4140)", () => { diff --git a/packages/pi-ai/src/providers/transform-messages-report.test.ts b/packages/pi-ai/src/providers/transform-messages-report.test.ts index dbef910e7..e496b4d60 100644 --- a/packages/pi-ai/src/providers/transform-messages-report.test.ts +++ b/packages/pi-ai/src/providers/transform-messages-report.test.ts @@ -1,5 +1,5 @@ // SF — ProviderSwitchReport Tests (ADR-005 Phase 3) -import { describe } from 'vitest'; +import { describe, test } from 'vitest'; import assert from "node:assert/strict"; import { transformMessages, createEmptyReport, hasTransformations } from "./transform-messages.js"; diff --git a/packages/pi-coding-agent/src/core/agent-session-print-mode-persist.test.ts b/packages/pi-coding-agent/src/core/agent-session-print-mode-persist.test.ts index a14384c17..a5ec40916 100644 --- a/packages/pi-coding-agent/src/core/agent-session-print-mode-persist.test.ts +++ b/packages/pi-coding-agent/src/core/agent-session-print-mode-persist.test.ts @@ -143,7 +143,7 @@ test("sf src/cli.ts print-mode skips validateConfiguredModel when --model is set // reapplyValidatedModelOnFallback must be inside the same guard block. const reapplyIdx = sfCliSource.indexOf("reapplyValidatedModelOnFallback(", validateIdx); assert.ok(reapplyIdx >= 0, "missing reapplyValidatedModelOnFallback call"); - const blockEnd = sfCliSource.indexOf("\n }\n", guardIdx); + const blockEnd = sfCliSource.indexOf("\n\t}\n", guardIdx); assert.ok( reapplyIdx < blockEnd, "reapplyValidatedModelOnFallback must be inside the same `if (!cliFlags.model)` block as validateConfiguredModel", diff --git a/packages/pi-coding-agent/src/core/auth-storage.test.ts b/packages/pi-coding-agent/src/core/auth-storage.test.ts index 93bdd2fc6..9cde4129f 100644 --- a/packages/pi-coding-agent/src/core/auth-storage.test.ts +++ b/packages/pi-coding-agent/src/core/auth-storage.test.ts @@ -282,13 +282,7 @@ describe("AuthStorage — oauth credential for non-OAuth provider (#2083)", () = // fall-through to env / fallback finds nothing and returns undefined. const origEnv = process.env.OPENROUTER_API_KEY; delete process.env.OPENROUTER_API_KEY; - t.afterAll(() => { - if (origEnv === undefined) { - delete process.env.OPENROUTER_API_KEY; - } else { - process.env.OPENROUTER_API_KEY = origEnv; - } - }); + try { // Before the fix, getApiKey returns undefined because // resolveCredentialApiKey calls getOAuthProvider("openrouter") → null → undefined. @@ -298,9 +292,16 @@ describe("AuthStorage — oauth credential for non-OAuth provider (#2083)", () = // should be skipped, and getApiKey should fall through to env / fallback. // With no env var and no fallback resolver configured, the result is undefined. assert.equal(key, undefined); + } finally { + if (origEnv === undefined) { + delete process.env.OPENROUTER_API_KEY; + } else { + process.env.OPENROUTER_API_KEY = origEnv; + } + } }); - it("falls through to env var when openrouter has type:oauth credential", async (t) => { + it("falls through to env var when openrouter has type:oauth credential", async () => { const storage = inMemory({ openrouter: { type: "oauth", @@ -312,20 +313,21 @@ describe("AuthStorage — oauth credential for non-OAuth provider (#2083)", () = // Simulate OPENROUTER_API_KEY being set via env const origEnv = process.env.OPENROUTER_API_KEY; - t.afterAll(() => { - if (origEnv === undefined) { - delete process.env.OPENROUTER_API_KEY; - } else { - process.env.OPENROUTER_API_KEY = origEnv; - } - }); + try { process.env.OPENROUTER_API_KEY = "sk-or-v1-env-key"; const key = await storage.getApiKey("openrouter"); assert.equal(key, "sk-or-v1-env-key"); + } finally { + if (origEnv === undefined) { + delete process.env.OPENROUTER_API_KEY; + } else { + process.env.OPENROUTER_API_KEY = origEnv; + } + } }); - it("falls through to fallback resolver when openrouter has type:oauth credential", async (t) => { + it("falls through to fallback resolver when openrouter has type:oauth credential", async () => { const storage = inMemory({ openrouter: { type: "oauth", @@ -339,13 +341,7 @@ describe("AuthStorage — oauth credential for non-OAuth provider (#2083)", () = // and the fallback resolver is reached. const origEnv = process.env.OPENROUTER_API_KEY; delete process.env.OPENROUTER_API_KEY; - t.afterAll(() => { - if (origEnv === undefined) { - delete process.env.OPENROUTER_API_KEY; - } else { - process.env.OPENROUTER_API_KEY = origEnv; - } - }); + try { storage.setFallbackResolver((provider) => provider === "openrouter" ? "sk-or-v1-fallback" : undefined, @@ -353,6 +349,13 @@ describe("AuthStorage — oauth credential for non-OAuth provider (#2083)", () = const key = await storage.getApiKey("openrouter"); assert.equal(key, "sk-or-v1-fallback"); + } finally { + if (origEnv === undefined) { + delete process.env.OPENROUTER_API_KEY; + } else { + process.env.OPENROUTER_API_KEY = origEnv; + } + } }); }); diff --git a/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts b/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts index 4473a51bc..4390d7862 100644 --- a/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts +++ b/packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { } from 'vitest'; +import { test } from 'vitest'; import { handleAgentEvent } from "../modes/interactive/controllers/chat-controller.js"; diff --git a/packages/pi-coding-agent/src/core/extensions/runner.test.ts b/packages/pi-coding-agent/src/core/extensions/runner.test.ts index cfd46a5a5..2b0743e56 100644 --- a/packages/pi-coding-agent/src/core/extensions/runner.test.ts +++ b/packages/pi-coding-agent/src/core/extensions/runner.test.ts @@ -48,37 +48,37 @@ function makeThrowingExtension(eventType: string, error: Error): Extension { } describe("ExtensionRunner.emitToolCall", () => { - it("catches throwing extension handler and routes to emitError", async (t) => { + it("catches throwing extension handler and routes to emitError", async () => { const dir = mkdtempSync(join(tmpdir(), "runner-test-")); - t.afterAll(() => { + try { + const sessionManager = SessionManager.create(dir, dir); + const authStorage = AuthStorage.create(); + const modelRegistry = new ModelRegistry(authStorage, join(dir, "models.json")); + + const throwingExt = makeThrowingExtension("tool_call", new Error("handler crashed")); + const runtime = makeMinimalRuntime(); + const runner = new ExtensionRunner([throwingExt], runtime, dir, sessionManager, modelRegistry); + + const errors: any[] = []; + runner.onError((err) => errors.push(err)); + + const event: ToolCallEvent = { + type: "tool_call", + toolCallId: "test-123", + toolName: "test_tool", + input: {}, + } as ToolCallEvent; + + const result = await runner.emitToolCall(event); + + // Should not throw — error is caught and routed to emitError + assert.equal(result, undefined); + assert.equal(errors.length, 1); + assert.equal(errors[0].error, "handler crashed"); + assert.equal(errors[0].event, "tool_call"); + assert.equal(errors[0].extensionPath, "/test/throwing-ext"); + } finally { rmSync(dir, { recursive: true, force: true }); - }); - - const sessionManager = SessionManager.create(dir, dir); - const authStorage = AuthStorage.create(); - const modelRegistry = new ModelRegistry(authStorage, join(dir, "models.json")); - - const throwingExt = makeThrowingExtension("tool_call", new Error("handler crashed")); - const runtime = makeMinimalRuntime(); - const runner = new ExtensionRunner([throwingExt], runtime, dir, sessionManager, modelRegistry); - - const errors: any[] = []; - runner.onError((err) => errors.push(err)); - - const event: ToolCallEvent = { - type: "tool_call", - toolCallId: "test-123", - toolName: "test_tool", - input: {}, - } as ToolCallEvent; - - const result = await runner.emitToolCall(event); - - // Should not throw — error is caught and routed to emitError - assert.equal(result, undefined); - assert.equal(errors.length, 1); - assert.equal(errors[0].error, "handler crashed"); - assert.equal(errors[0].event, "tool_call"); - assert.equal(errors[0].extensionPath, "/test/throwing-ext"); + } }); }); diff --git a/packages/pi-coding-agent/src/core/lifecycle-hooks.test.ts b/packages/pi-coding-agent/src/core/lifecycle-hooks.test.ts index 19f0bfd34..4aec6ee15 100644 --- a/packages/pi-coding-agent/src/core/lifecycle-hooks.test.ts +++ b/packages/pi-coding-agent/src/core/lifecycle-hooks.test.ts @@ -2,7 +2,7 @@ import assert from "node:assert/strict"; import { mkdtempSync, mkdirSync, rmSync, writeFileSync, existsSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; import { join, resolve } from "node:path"; -import { describe, it } from 'vitest'; +import { describe, it, afterEach } from 'vitest'; import { readManifestRuntimeDeps, collectRuntimeDependencies, @@ -10,60 +10,69 @@ import { resolveLocalSourcePath, } from "./lifecycle-hooks.js"; -function tmpDir(prefix: string, t: { after: (fn: () => void) => void }): string { +const _tmpDirs: string[] = []; + +function tmpDir(prefix: string): string { const dir = mkdtempSync(join(tmpdir(), `pi-lh-${prefix}-`)); - t.afterAll(() => rmSync(dir, { recursive: true, force: true })); + _tmpDirs.push(dir); return dir; } +afterEach(() => { + for (const dir of _tmpDirs) { + rmSync(dir, { recursive: true, force: true }); + } + _tmpDirs.length = 0; +}); + // ─── readManifestRuntimeDeps ────────────────────────────────────────────────── describe("readManifestRuntimeDeps", () => { - it("returns empty array when manifest file is missing", (t) => { - const dir = tmpDir("no-manifest", t); + it("returns empty array when manifest file is missing", () => { + const dir = tmpDir("no-manifest"); assert.deepEqual(readManifestRuntimeDeps(dir), []); }); - it("returns empty array for malformed JSON", (t) => { - const dir = tmpDir("bad-json", t); + it("returns empty array for malformed JSON", () => { + const dir = tmpDir("bad-json"); writeFileSync(join(dir, "extension-manifest.json"), "not json{{{", "utf-8"); assert.deepEqual(readManifestRuntimeDeps(dir), []); }); - it("returns runtime deps from valid manifest", (t) => { - const dir = tmpDir("valid", t); + it("returns runtime deps from valid manifest", () => { + const dir = tmpDir("valid"); writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({ dependencies: { runtime: ["claude", "node"] }, }), "utf-8"); assert.deepEqual(readManifestRuntimeDeps(dir), ["claude", "node"]); }); - it("returns empty array when dependencies exists but runtime is missing", (t) => { - const dir = tmpDir("no-runtime", t); + it("returns empty array when dependencies exists but runtime is missing", () => { + const dir = tmpDir("no-runtime"); writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({ dependencies: {}, }), "utf-8"); assert.deepEqual(readManifestRuntimeDeps(dir), []); }); - it("returns empty array when runtime is empty", (t) => { - const dir = tmpDir("empty-runtime", t); + it("returns empty array when runtime is empty", () => { + const dir = tmpDir("empty-runtime"); writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({ dependencies: { runtime: [] }, }), "utf-8"); assert.deepEqual(readManifestRuntimeDeps(dir), []); }); - it("filters out non-string entries in runtime array", (t) => { - const dir = tmpDir("mixed-types", t); + it("filters out non-string entries in runtime array", () => { + const dir = tmpDir("mixed-types"); writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({ dependencies: { runtime: [123, null, "node", false, "python"] }, }), "utf-8"); assert.deepEqual(readManifestRuntimeDeps(dir), ["node", "python"]); }); - it("returns empty array when no dependencies field at all", (t) => { - const dir = tmpDir("no-deps-field", t); + it("returns empty array when no dependencies field at all", () => { + const dir = tmpDir("no-deps-field"); writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({ id: "test", name: "Test", @@ -75,16 +84,16 @@ describe("readManifestRuntimeDeps", () => { // ─── collectRuntimeDependencies ─────────────────────────────────────────────── describe("collectRuntimeDependencies", () => { - it("aggregates deps from installedPath manifest", (t) => { - const dir = tmpDir("collect-installed", t); + it("aggregates deps from installedPath manifest", () => { + const dir = tmpDir("collect-installed"); writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({ dependencies: { runtime: ["claude"] }, }), "utf-8"); assert.deepEqual(collectRuntimeDependencies(dir, []), ["claude"]); }); - it("aggregates deps from entry path directory manifests", (t) => { - const root = tmpDir("collect-entry", t); + it("aggregates deps from entry path directory manifests", () => { + const root = tmpDir("collect-entry"); const installedDir = join(root, "installed"); const entryDir = join(root, "entry"); mkdirSync(installedDir, { recursive: true }); @@ -96,8 +105,8 @@ describe("collectRuntimeDependencies", () => { assert.deepEqual(deps, ["python"]); }); - it("deduplicates across multiple directories", (t) => { - const root = tmpDir("collect-dedup", t); + it("deduplicates across multiple directories", () => { + const root = tmpDir("collect-dedup"); const dir1 = join(root, "dir1"); const dir2 = join(root, "dir2"); mkdirSync(dir1, { recursive: true }); @@ -115,8 +124,8 @@ describe("collectRuntimeDependencies", () => { assert.ok(deps.includes("claude")); }); - it("returns empty when no directories have manifests", (t) => { - const dir = tmpDir("collect-empty", t); + it("returns empty when no directories have manifests", () => { + const dir = tmpDir("collect-empty"); assert.deepEqual(collectRuntimeDependencies(dir, []), []); }); }); @@ -203,21 +212,21 @@ describe("resolveLocalSourcePath", () => { } }); - it("resolves relative path that exists", (t) => { - const dir = tmpDir("resolve-rel", t); + it("resolves relative path that exists", () => { + const dir = tmpDir("resolve-rel"); const sub = join(dir, "myext"); mkdirSync(sub, { recursive: true }); const result = resolveLocalSourcePath("myext", dir); assert.equal(result, resolve(dir, "myext")); }); - it("returns undefined for relative path that does not exist", (t) => { - const dir = tmpDir("resolve-noexist", t); + it("returns undefined for relative path that does not exist", () => { + const dir = tmpDir("resolve-noexist"); assert.equal(resolveLocalSourcePath("nonexistent", dir), undefined); }); - it("resolves absolute path that exists", (t) => { - const dir = tmpDir("resolve-abs", t); + it("resolves absolute path that exists", () => { + const dir = tmpDir("resolve-abs"); assert.equal(resolveLocalSourcePath(dir, "/irrelevant"), dir); }); diff --git a/packages/pi-coding-agent/src/core/model-discovery.test.ts b/packages/pi-coding-agent/src/core/model-discovery.test.ts index 7c734c76a..1796f4282 100644 --- a/packages/pi-coding-agent/src/core/model-discovery.test.ts +++ b/packages/pi-coding-agent/src/core/model-discovery.test.ts @@ -38,7 +38,7 @@ describe("getDiscoveryAdapter", () => { }); it("returns adapters for direct live-listed providers", () => { - for (const provider of ["zai", "minimax", "kimi-coding", "mistral"]) { + for (const provider of ["zai", "minimax", "mistral"]) { const adapter = getDiscoveryAdapter(provider); assert.equal(adapter.provider, provider); assert.equal(adapter.supportsDiscovery, true); @@ -95,7 +95,7 @@ describe("getDiscoverableProviders", () => { "openrouter", "zai", "minimax", - "kimi-coding", + "mistral", ]); assert.ok(!providers.includes("ollama")); @@ -356,13 +356,6 @@ describe("direct provider discovery", () => { api: "anthropic-messages", baseUrl: "https://api.minimax.io/anthropic", }, - { - provider: "kimi-coding", - url: "https://api.kimi.com/coding/v1/models", - model: "kimi-for-coding", - api: "anthropic-messages", - baseUrl: "https://api.kimi.com/coding", - }, { provider: "mistral", url: "https://api.mistral.ai/v1/models", diff --git a/packages/pi-coding-agent/src/core/package-commands.test.ts b/packages/pi-coding-agent/src/core/package-commands.test.ts index 37dba4ce1..16646ac77 100644 --- a/packages/pi-coding-agent/src/core/package-commands.test.ts +++ b/packages/pi-coding-agent/src/core/package-commands.test.ts @@ -25,9 +25,8 @@ function writePackage(root: string, files: Record): void { } } -function createTestDirs(prefix: string, t: { after: (fn: () => void) => void }) { +function createTestDirs(prefix: string) { const root = mkdtempSync(join(tmpdir(), `pi-lifecycle-${prefix}-`)); - t.afterAll(() => rmSync(root, { recursive: true, force: true })); const cwd = join(root, "cwd"); const agentDir = join(root, "agent"); const extensionDir = join(root, `ext-${prefix}`); @@ -38,8 +37,9 @@ function createTestDirs(prefix: string, t: { after: (fn: () => void) => void }) } describe("runPackageCommand lifecycle hooks", () => { - it("executes registered beforeInstall and afterInstall handlers for local packages", async (t) => { - const { cwd, agentDir, extensionDir } = createTestDirs("install", t); + it("executes registered beforeInstall and afterInstall handlers for local packages", async () => { + const { root, cwd, agentDir, extensionDir } = createTestDirs("install"); + try { writePackage(extensionDir, { "package.json": JSON.stringify({ @@ -77,10 +77,14 @@ describe("runPackageCommand lifecycle hooks", () => { assert.equal(readFileSync(join(extensionDir, "before-install-ran.txt"), "utf-8"), "ok"); assert.equal(readFileSync(join(extensionDir, "after-install-ran.txt"), "utf-8"), "ok"); assert.ok(stdout.getOutput().includes(`Installed ${extensionDir}`)); + } finally { + rmSync(root, { recursive: true, force: true }); + } }); - it("runs legacy named lifecycle hooks when no registered hooks exist", async (t) => { - const { cwd, agentDir, extensionDir } = createTestDirs("legacy", t); + it("runs legacy named lifecycle hooks when no registered hooks exist", async () => { + const { root, cwd, agentDir, extensionDir } = createTestDirs("legacy"); + try { writePackage(extensionDir, { "package.json": JSON.stringify({ @@ -136,10 +140,14 @@ describe("runPackageCommand lifecycle hooks", () => { assert.equal(removeResult.exitCode, 0); assert.equal(readFileSync(join(extensionDir, "legacy-before-remove.txt"), "utf-8"), "ok"); assert.equal(readFileSync(join(extensionDir, "legacy-after-remove.txt"), "utf-8"), "ok"); + } finally { + rmSync(root, { recursive: true, force: true }); + } }); - it("skips lifecycle phases with no hooks declared", async (t) => { - const { cwd, agentDir, extensionDir } = createTestDirs("skip", t); + it("skips lifecycle phases with no hooks declared", async () => { + const { root, cwd, agentDir, extensionDir } = createTestDirs("skip"); + try { writePackage(extensionDir, { "package.json": JSON.stringify({ @@ -174,10 +182,14 @@ describe("runPackageCommand lifecycle hooks", () => { assert.equal(removeResult.handled, true); assert.equal(removeResult.exitCode, 0); assert.equal(stderr.getOutput().includes("Hook failed"), false); + } finally { + rmSync(root, { recursive: true, force: true }); + } }); - it("fails install when manifest runtime dependency is missing", async (t) => { - const { cwd, agentDir, extensionDir } = createTestDirs("deps", t); + it("fails install when manifest runtime dependency is missing", async () => { + const { root, cwd, agentDir, extensionDir } = createTestDirs("deps"); + try { writePackage(extensionDir, { "package.json": JSON.stringify({ @@ -208,10 +220,14 @@ describe("runPackageCommand lifecycle hooks", () => { assert.equal(result.handled, true); assert.equal(result.exitCode, 1); assert.ok(stderr.getOutput().includes("Missing runtime dependencies")); + } finally { + rmSync(root, { recursive: true, force: true }); + } }); - it("afterRemove hook receives installedPath even when directory is deleted", async (t) => { - const { cwd, agentDir, extensionDir } = createTestDirs("after-remove", t); + it("afterRemove hook receives installedPath even when directory is deleted", async () => { + const { root, cwd, agentDir, extensionDir } = createTestDirs("after-remove"); + try { writePackage(extensionDir, { "package.json": JSON.stringify({ @@ -258,5 +274,8 @@ describe("runPackageCommand lifecycle hooks", () => { assert.ok(existsSync(markerPath), "afterRemove hook must have executed and written marker"); const marker = JSON.parse(readFileSync(markerPath, "utf-8")); assert.equal(typeof marker.receivedPath, "string", "hook must receive installedPath as string"); + } finally { + rmSync(root, { recursive: true, force: true }); + } }); }); diff --git a/packages/pi-coding-agent/src/core/resolve-config-value.test.ts b/packages/pi-coding-agent/src/core/resolve-config-value.test.ts index 7bf47d5b7..629a31ae7 100644 --- a/packages/pi-coding-agent/src/core/resolve-config-value.test.ts +++ b/packages/pi-coding-agent/src/core/resolve-config-value.test.ts @@ -40,20 +40,20 @@ describe("resolveConfigValue — non-command values", () => { }); describe("resolveConfigValue — command allowlist enforcement", () => { - it("blocks a disallowed command and returns undefined", (t) => { + it("blocks a disallowed command and returns undefined", () => { const stderrChunks: string[] = []; const originalWrite = process.stderr.write.bind(process.stderr); process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => { stderrChunks.push(chunk.toString()); return true; }; - t.afterAll(() => { + try { + const result = resolveConfigValue("!curl http://evil.com"); + assert.equal(result, undefined); + assert.ok(stderrChunks.some((line) => line.includes("curl"))); + } finally { process.stderr.write = originalWrite; - }); - - const result = resolveConfigValue("!curl http://evil.com"); - assert.equal(result, undefined); - assert.ok(stderrChunks.some((line) => line.includes("curl"))); + } }); it("blocks another disallowed command (rm)", () => { @@ -66,7 +66,7 @@ describe("resolveConfigValue — command allowlist enforcement", () => { assert.equal(result, undefined); }); - it("allows a safe command prefix to proceed to execution", (t) => { + it("allows a safe command prefix to proceed to execution", () => { // `pass` is unlikely to be installed in CI, so we just verify it does NOT // return undefined due to the allowlist check — it may return undefined if // the binary is absent, but the block path must not be taken. @@ -77,15 +77,15 @@ describe("resolveConfigValue — command allowlist enforcement", () => { stderrChunks.push(chunk.toString()); return true; }; - t.afterAll(() => { + try { + resolveConfigValue("!pass show nonexistent-entry-for-test"); + const blocked = stderrChunks.some((line) => + line.includes("Blocked disallowed command"), + ); + assert.equal(blocked, false, "pass should not be blocked by the allowlist"); + } finally { process.stderr.write = originalWrite; - }); - - resolveConfigValue("!pass show nonexistent-entry-for-test"); - const blocked = stderrChunks.some((line) => - line.includes("Blocked disallowed command") - ); - assert.equal(blocked, false, "pass should not be blocked by the allowlist"); + } }); }); @@ -130,59 +130,59 @@ describe("resolveConfigValue — shell operator bypass prevention", () => { assert.equal(result, undefined); }); - it("writes stderr warning when shell operators detected", (t) => { + it("writes stderr warning when shell operators detected", () => { const stderrChunks: string[] = []; const originalWrite = process.stderr.write.bind(process.stderr); process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => { stderrChunks.push(chunk.toString()); return true; }; - t.afterAll(() => { + try { + resolveConfigValue("!pass show key; curl evil.com"); + assert.ok(stderrChunks.some((line) => line.includes("shell operators"))); + } finally { process.stderr.write = originalWrite; - }); - - resolveConfigValue("!pass show key; curl evil.com"); - assert.ok(stderrChunks.some((line) => line.includes("shell operators"))); + } }); }); describe("resolveConfigValue — caching", () => { - it("caches the result of a blocked command", (t) => { + it("caches the result of a blocked command", () => { const callCount = { n: 0 }; const originalWrite = process.stderr.write.bind(process.stderr); process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => { callCount.n++; return true; }; - t.afterAll(() => { + try { + resolveConfigValue("!curl http://evil.com"); + resolveConfigValue("!curl http://evil.com"); + // The block warning should only fire once; the second call hits the cache + // before reaching the allowlist check, so stderr count is 1. + assert.equal(callCount.n, 1); + } finally { process.stderr.write = originalWrite; - }); - - resolveConfigValue("!curl http://evil.com"); - resolveConfigValue("!curl http://evil.com"); - // The block warning should only fire once; the second call hits the cache - // before reaching the allowlist check, so stderr count is 1. - assert.equal(callCount.n, 1); + } }); - it("clearConfigValueCache resets cached entries", (t) => { + it("clearConfigValueCache resets cached entries", () => { const stderrChunks: string[] = []; const originalWrite = process.stderr.write.bind(process.stderr); process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => { stderrChunks.push(chunk.toString()); return true; }; - t.afterAll(() => { + try { + resolveConfigValue("!curl http://evil.com"); + assert.equal(stderrChunks.length, 1); + + clearConfigValueCache(); + + resolveConfigValue("!curl http://evil.com"); + assert.equal(stderrChunks.length, 2); + } finally { process.stderr.write = originalWrite; - }); - - resolveConfigValue("!curl http://evil.com"); - assert.equal(stderrChunks.length, 1); - - clearConfigValueCache(); - - resolveConfigValue("!curl http://evil.com"); - assert.equal(stderrChunks.length, 2); + } }); }); @@ -192,36 +192,36 @@ describe("REGRESSION #666: non-default credential tool blocked with no override" clearConfigValueCache(); }); - it("sops is blocked by default, then unblocked by setAllowedCommandPrefixes", (t) => { + it("sops is blocked by default, then unblocked by setAllowedCommandPrefixes", () => { const stderrChunks: string[] = []; const originalWrite = process.stderr.write.bind(process.stderr); process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => { stderrChunks.push(chunk.toString()); return true; }; - t.afterAll(() => { + try { + // Bug: sops is not in SAFE_COMMAND_PREFIXES, so it's blocked + const result = resolveConfigValue("!sops decrypt --output-type json secrets.enc.json"); + assert.equal(result, undefined, "sops is blocked by the hardcoded allowlist"); + assert.ok( + stderrChunks.some((line) => line.includes('Blocked disallowed command: "sops"')), + "should log a block message for sops", + ); + + stderrChunks.length = 0; + clearConfigValueCache(); + + // Fix: override the allowlist to include sops + setAllowedCommandPrefixes([...SAFE_COMMAND_PREFIXES, "sops"]); + resolveConfigValue("!sops decrypt --output-type json secrets.enc.json"); + + const blockedAfterOverride = stderrChunks.some((line) => + line.includes("Blocked disallowed command"), + ); + assert.equal(blockedAfterOverride, false, "sops must not be blocked after override"); + } finally { process.stderr.write = originalWrite; - }); - - // Bug: sops is not in SAFE_COMMAND_PREFIXES, so it's blocked - const result = resolveConfigValue("!sops decrypt --output-type json secrets.enc.json"); - assert.equal(result, undefined, "sops is blocked by the hardcoded allowlist"); - assert.ok( - stderrChunks.some((line) => line.includes('Blocked disallowed command: "sops"')), - "should log a block message for sops", - ); - - stderrChunks.length = 0; - clearConfigValueCache(); - - // Fix: override the allowlist to include sops - setAllowedCommandPrefixes([...SAFE_COMMAND_PREFIXES, "sops"]); - resolveConfigValue("!sops decrypt --output-type json secrets.enc.json"); - - const blockedAfterOverride = stderrChunks.some((line) => - line.includes("Blocked disallowed command"), - ); - assert.equal(blockedAfterOverride, false, "sops must not be blocked after override"); + } }); }); @@ -236,60 +236,60 @@ describe("setAllowedCommandPrefixes — user override", () => { assert.deepEqual([...getAllowedCommandPrefixes()], ["sops", "doppler"]); }); - it("custom prefix is allowed through to execution", (t) => { + it("custom prefix is allowed through to execution", () => { const stderrChunks: string[] = []; const originalWrite = process.stderr.write.bind(process.stderr); process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => { stderrChunks.push(chunk.toString()); return true; }; - t.afterAll(() => { + try { + setAllowedCommandPrefixes(["mycli"]); + resolveConfigValue("!mycli get-secret"); + const blocked = stderrChunks.some((line) => line.includes("Blocked disallowed command")); + assert.equal(blocked, false, "mycli should not be blocked when in the custom allowlist"); + } finally { process.stderr.write = originalWrite; - }); - - setAllowedCommandPrefixes(["mycli"]); - resolveConfigValue("!mycli get-secret"); - const blocked = stderrChunks.some((line) => line.includes("Blocked disallowed command")); - assert.equal(blocked, false, "mycli should not be blocked when in the custom allowlist"); + } }); - it("previously-allowed prefix is blocked after override", (t) => { + it("previously-allowed prefix is blocked after override", () => { const stderrChunks: string[] = []; const originalWrite = process.stderr.write.bind(process.stderr); process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => { stderrChunks.push(chunk.toString()); return true; }; - t.afterAll(() => { + try { + setAllowedCommandPrefixes(["sops"]); + const result = resolveConfigValue("!pass show secret"); + assert.equal(result, undefined); + const blocked = stderrChunks.some((line) => line.includes("Blocked disallowed command")); + assert.equal(blocked, true, "pass should be blocked when not in the custom allowlist"); + } finally { process.stderr.write = originalWrite; - }); - - setAllowedCommandPrefixes(["sops"]); - const result = resolveConfigValue("!pass show secret"); - assert.equal(result, undefined); - const blocked = stderrChunks.some((line) => line.includes("Blocked disallowed command")); - assert.equal(blocked, true, "pass should be blocked when not in the custom allowlist"); + } }); - it("clears cache when overriding prefixes", (t) => { + it("clears cache when overriding prefixes", () => { const stderrChunks: string[] = []; const originalWrite = process.stderr.write.bind(process.stderr); process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => { stderrChunks.push(chunk.toString()); return true; }; - t.afterAll(() => { + try { + resolveConfigValue("!mycli get-secret"); + assert.ok(stderrChunks.some((line) => line.includes("Blocked"))); + + stderrChunks.length = 0; + + setAllowedCommandPrefixes(["mycli"]); + resolveConfigValue("!mycli get-secret"); + const blocked = stderrChunks.some((line) => line.includes("Blocked")); + assert.equal(blocked, false, "Should re-evaluate after allowlist change"); + } finally { process.stderr.write = originalWrite; - }); - - resolveConfigValue("!mycli get-secret"); - assert.ok(stderrChunks.some((line) => line.includes("Blocked"))); - - stderrChunks.length = 0; - - setAllowedCommandPrefixes(["mycli"]); - resolveConfigValue("!mycli get-secret"); - const blocked = stderrChunks.some((line) => line.includes("Blocked")); - assert.equal(blocked, false, "Should re-evaluate after allowlist change"); + } }); }); diff --git a/packages/pi-coding-agent/src/core/tools/edit-diff.test.ts b/packages/pi-coding-agent/src/core/tools/edit-diff.test.ts index 410947867..9ea0e7b6d 100644 --- a/packages/pi-coding-agent/src/core/tools/edit-diff.test.ts +++ b/packages/pi-coding-agent/src/core/tools/edit-diff.test.ts @@ -60,11 +60,9 @@ describe("edit-diff", () => { assert.match(result.diff, /CHANGED/); }); - it("computes diffs for preview without native helpers", async (t) => { + it("computes diffs for preview without native helpers", async () => { const dir = mkdtempSync(join(tmpdir(), "edit-diff-test-")); - t.afterAll(() => { - rmSync(dir, { recursive: true, force: true }); - }); + try { const file = join(dir, "sample.ts"); writeFileSync(file, "const title = “Hello”;\n", "utf-8"); @@ -81,5 +79,8 @@ describe("edit-diff", () => { assert.equal(result.firstChangedLine, 1); assert.match(result.diff, /\+1 const title = "Hi";/); } + } finally { + rmSync(dir, { recursive: true, force: true }); + } }); }); diff --git a/packages/pi-coding-agent/src/tests/system-prompt-skill-filter.test.ts b/packages/pi-coding-agent/src/tests/system-prompt-skill-filter.test.ts index 1a5207795..940319509 100644 --- a/packages/pi-coding-agent/src/tests/system-prompt-skill-filter.test.ts +++ b/packages/pi-coding-agent/src/tests/system-prompt-skill-filter.test.ts @@ -126,7 +126,7 @@ test("buildSystemPrompt: skillFilter does not affect context files or cwd render // ─── Exception safety ───────────────────────────────────────────────────── -test("buildSystemPrompt: skillFilter that throws falls back to unfiltered list and does not propagate", (t) => { +test("buildSystemPrompt: skillFilter that throws falls back to unfiltered list and does not propagate", () => { // A buggy consumer predicate must not bubble out of buildSystemPrompt. // If it did, _rebuildSystemPrompt could unwind mid-setTools() and leave // the session with updated tools but a stale system prompt. @@ -136,9 +136,8 @@ test("buildSystemPrompt: skillFilter that throws falls back to unfiltered list a const originalWarn = console.warn; const warnings: string[] = []; console.warn = (...args: unknown[]) => { warnings.push(args.join(" ")); }; - t.afterAll(() => { console.warn = originalWarn; }); - - let prompt = ""; + try { + let prompt = ""; assert.doesNotThrow(() => { prompt = buildSystemPrompt({ skills, @@ -153,5 +152,8 @@ test("buildSystemPrompt: skillFilter that throws falls back to unfiltered list a assert.ok( warnings.some(w => w.includes("skillFilter threw") && w.includes("consumer bug")), "fallback should emit an identifying warning", - ); + ); + } finally { + console.warn = originalWarn; + } }); diff --git a/packages/pi-tui/src/__tests__/autocomplete.test.ts b/packages/pi-tui/src/__tests__/autocomplete.test.ts index 9be56f426..85ded34f5 100644 --- a/packages/pi-tui/src/__tests__/autocomplete.test.ts +++ b/packages/pi-tui/src/__tests__/autocomplete.test.ts @@ -112,7 +112,7 @@ describe("CombinedAutocompleteProvider — argument completions", () => { }); describe("CombinedAutocompleteProvider — @ file prefix extraction", () => { - it("detects @ at start of line", () => { + it("detects @ at start of line", { timeout: 60_000 }, () => { const provider = makeProvider(); // @ triggers fuzzy file search — we can't test the actual file results // but we can test that getSuggestions returns null (no files in /tmp matching) @@ -122,7 +122,7 @@ describe("CombinedAutocompleteProvider — @ file prefix extraction", () => { assert.ok(result === null || result.items.length >= 0); }); - it("detects @ after space", () => { + it("detects @ after space", { timeout: 60_000 }, () => { const provider = makeProvider(); const result = provider.getSuggestions(["check @nonexistent_xyz"], 0, 22); assert.ok(result === null || result.items.length >= 0); diff --git a/packages/pi-tui/src/components/__tests__/markdown-maxlines.test.ts b/packages/pi-tui/src/components/__tests__/markdown-maxlines.test.ts index e473be465..024552be5 100644 --- a/packages/pi-tui/src/components/__tests__/markdown-maxlines.test.ts +++ b/packages/pi-tui/src/components/__tests__/markdown-maxlines.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { } from 'vitest'; +import { test } from 'vitest'; import { Markdown, type MarkdownTheme } from "../markdown.js"; diff --git a/packages/pi-tui/src/components/image.test.ts b/packages/pi-tui/src/components/image.test.ts index 2f11674c5..25c962aea 100644 --- a/packages/pi-tui/src/components/image.test.ts +++ b/packages/pi-tui/src/components/image.test.ts @@ -3,7 +3,7 @@ * re-render loop when dimensions resolve in cmux sessions. */ -import { describe } from 'vitest'; +import { describe, test } from 'vitest'; import assert from "node:assert/strict"; import { Image } from "./image.js"; diff --git a/src/resources/extensions/sf/auto-dispatch.ts b/src/resources/extensions/sf/auto-dispatch.ts index 7d2d5a543..fb5167789 100644 --- a/src/resources/extensions/sf/auto-dispatch.ts +++ b/src/resources/extensions/sf/auto-dispatch.ts @@ -73,10 +73,7 @@ import { buildDispatchEnvelope, explainDispatch, } from "./uok/dispatch-envelope.js"; -import { - EXECUTION_ENTRY_PHASES, - hasFinalizedMilestoneContext, -} from "./uok/plan-v2.js"; +import { hasFinalizedMilestoneContext } from "./uok/plan-v2.js"; import { extractVerdict, isAcceptableUatVerdict } from "./verdict-parser.js"; import { logError, logWarning } from "./workflow-logger.js"; @@ -670,7 +667,9 @@ export const DISPATCH_RULES: DispatchRule[] = [ // stuck until `sf doctor heal`. Fire BEFORE execution-entry phase rules. name: "execution-entry phase (no context) → discuss-milestone", match: async ({ state, mid, midTitle, basePath }) => { - if (!EXECUTION_ENTRY_PHASES.has(state.phase)) return null; + if (state.phase !== "executing" && state.phase !== "summarizing") { + return null; + } if (hasFinalizedMilestoneContext(basePath, mid)) return null; return { action: "dispatch", @@ -1209,9 +1208,9 @@ export const DISPATCH_RULES: DispatchRule[] = [ // Skip preference or trivial-scope pipeline variant: write a minimal pass-through VALIDATION file const trivialVariant = pipelineVariant === "trivial"; - const skipSource = trivialVariant - ? "trivial-scope pipeline variant (#4781)" - : "`skip_milestone_validation` preference"; + const skipLine = trivialVariant + ? "Milestone validation was skipped via trivial-scope pipeline variant (#4781)." + : "Milestone validation was skipped by preference (`skip_milestone_validation`)."; if (prefs?.phases?.skip_milestone_validation || trivialVariant) { const mDir = resolveMilestonePath(basePath, mid); if (mDir) { @@ -1228,7 +1227,7 @@ export const DISPATCH_RULES: DispatchRule[] = [ "", "# Milestone Validation (skipped)", "", - `Milestone validation was skipped via ${skipSource}.`, + skipLine, ].join("\n"); writeFileSync(validationPath, content, "utf-8"); } @@ -1366,7 +1365,7 @@ export const DISPATCH_RULES: DispatchRule[] = [ // Allow completion when validation was intentionally skipped by // preference/budget profile (#3399, #3344). const skippedByPreference = - /skip(?:ped)?[\s-]+(?:by|per|due to)\s+(?:preference|budget|profile)/i.test( + /skip(?:ped)?[\s\-]+(?:by|per|due to)\s+(?:preference|budget|profile)/i.test( validationContent, ); diff --git a/src/resources/extensions/sf/state.ts b/src/resources/extensions/sf/state.ts index 443467cc4..d4eba91a0 100644 --- a/src/resources/extensions/sf/state.ts +++ b/src/resources/extensions/sf/state.ts @@ -1191,6 +1191,7 @@ export async function deriveStateFromDb(basePath: string): Promise { ) : dbTasksBefore; + const planContent = planFile ? await loadFile(planFile) : null; const planQualityIssue = planContent ? getSlicePlanBlockingIssue(planContent) : null; diff --git a/src/resources/extensions/sf/tests/complete-task.test.ts b/src/resources/extensions/sf/tests/complete-task.test.ts index 78aa18a2a..767849f80 100644 --- a/src/resources/extensions/sf/tests/complete-task.test.ts +++ b/src/resources/extensions/sf/tests/complete-task.test.ts @@ -705,7 +705,7 @@ console.log( ); assertMatch( dispatch.prompt, - /sf_task_complete failed: .*Try the call again, or investigate the write path\./, + /sf_task_complete failed: [\s\S]*Try the call again, or investigate the write path\./, "next turn should surface the sf_task_complete failure verbatim", ); } @@ -919,7 +919,7 @@ console.log("\n=== complete-task: semantic validation hardening ==="); ); assertTrue("error" in traversal, "path-traversal taskId should be rejected"); if ("error" in traversal) { - assertMatch(traversal.error, /safe path segment/, "safe segment error"); + assertEq(traversal.error, "unsafe_id", "safe segment error"); } assertTrue( !fs.existsSync( diff --git a/src/resources/extensions/sf/tests/stash-pop-sf-conflict.test.ts b/src/resources/extensions/sf/tests/stash-pop-sf-conflict.test.ts index b566997ff..7d4903849 100644 --- a/src/resources/extensions/sf/tests/stash-pop-sf-conflict.test.ts +++ b/src/resources/extensions/sf/tests/stash-pop-sf-conflict.test.ts @@ -19,7 +19,7 @@ import { } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { test } from 'vitest'; +import { test, afterAll } from 'vitest'; import { createAutoWorktree, mergeMilestoneToMain } from "../auto-worktree.ts"; import { _clearSfRootCache } from "../paths.ts"; @@ -37,7 +37,7 @@ test.beforeAll(() => { _resetServiceCache(); }); -test.afterAll(() => { +afterAll(() => { process.env.HOME = originalHome; _clearSfRootCache(); _resetServiceCache(); diff --git a/src/resources/extensions/sf/tests/stash-queued-context-files.test.ts b/src/resources/extensions/sf/tests/stash-queued-context-files.test.ts index d2a7cb163..a8d95bf7a 100644 --- a/src/resources/extensions/sf/tests/stash-queued-context-files.test.ts +++ b/src/resources/extensions/sf/tests/stash-queued-context-files.test.ts @@ -24,7 +24,7 @@ import { } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { test } from 'vitest'; +import { test, afterAll } from 'vitest'; import { createAutoWorktree, mergeMilestoneToMain } from "../auto-worktree.ts"; import { _clearSfRootCache } from "../paths.ts"; @@ -42,7 +42,7 @@ test.beforeAll(() => { _resetServiceCache(); }); -test.afterAll(() => { +afterAll(() => { process.env.HOME = originalHome; _clearSfRootCache(); _resetServiceCache(); diff --git a/src/resources/extensions/sf/tools/complete-task.ts b/src/resources/extensions/sf/tools/complete-task.ts index 5789c7dd2..d096cfbc5 100644 --- a/src/resources/extensions/sf/tools/complete-task.ts +++ b/src/resources/extensions/sf/tools/complete-task.ts @@ -256,6 +256,30 @@ export async function handleCompleteTask( paramsInput: CompleteTaskParams, basePath: string, ): Promise { + // Required-field validation should report the actionable missing field before + // the path-safety layer classifies an empty string as an unsafe segment. + if ( + !paramsInput.taskId || + typeof paramsInput.taskId !== "string" || + paramsInput.taskId.trim() === "" + ) { + return { error: "taskId is required and must be a non-empty string" }; + } + if ( + !paramsInput.sliceId || + typeof paramsInput.sliceId !== "string" || + paramsInput.sliceId.trim() === "" + ) { + return { error: "sliceId is required and must be a non-empty string" }; + } + if ( + !paramsInput.milestoneId || + typeof paramsInput.milestoneId !== "string" || + paramsInput.milestoneId.trim() === "" + ) { + return { error: "milestoneId is required and must be a non-empty string" }; + } + // ── Path-traversal safety check (runs on raw input before normalization) ─ // Reject any ID that could escape .sf/ when interpolated into a path. // Must run before normalizeCompleteTaskParams, which also validates IDs