From 642c0f5a9e94efb54a6c5fdf998d080066922ecf Mon Sep 17 00:00:00 2001 From: Iouri Goussev Date: Sat, 21 Mar 2026 23:24:15 -0400 Subject: [PATCH] test: fix Assertion Roulette, Eager Test, and contract test regressions (#1938) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: add assertion messages to fix Assertion Roulette in GSD tests Add descriptive messages to multi-assertion tests where a bare failure output ("expected true, got false") wouldn't identify which assertion broke. Affected tests: auto-secrets-gate, search-tavily, search-provider- command, tavily-helpers. Co-Authored-By: Claude Sonnet 4.6 * test: fix Eager Test smell in captures and worktree-manager tests - Split captures: loadPendingCaptures test — extracted loadAllCaptures assertion into its own focused test - Refactor worktree-manager: replace monolithic main() script with 11 isolated test() calls, each with its own repo setup via helpers Co-Authored-By: Claude Sonnet 4.6 * test: add assertion messages to remaining test files Co-Authored-By: Claude Sonnet 4.6 * test: fix contract test gate, dynamic roots, and shared fetch helpers - Fix reject-notice sub-test gated on outcome.kind (actual) instead of expectedKind (map value) in web-command-parity-contract.test.ts - Restore dynamic loop over registered non-gsd passthrough roots with an explicit count assertion so new registrations fail loudly - Extract normalizeHeaders/parseJsonBody to src/tests/fetch-test-helpers.ts and import in both search-tavily and llm-context-tavily tests Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .../extensions/gsd/tests/auto-loop.test.ts | 218 ++++++------- .../gsd/tests/auto-secrets-gate.test.ts | 4 +- .../extensions/gsd/tests/captures.test.ts | 13 +- .../gsd/tests/continue-here.test.ts | 40 +-- .../gsd/tests/worktree-manager.test.ts | 298 ++++++++++++------ src/tests/fetch-test-helpers.ts | 20 ++ src/tests/llm-context-tavily.test.ts | 18 +- src/tests/search-provider-command.test.ts | 10 +- src/tests/search-tavily.test.ts | 32 +- src/tests/tavily-helpers.test.ts | 18 +- src/tests/web-boot-node24.test.ts | 138 ++++---- src/tests/web-command-parity-contract.test.ts | 121 +++---- 12 files changed, 505 insertions(+), 425 deletions(-) create mode 100644 src/tests/fetch-test-helpers.ts diff --git a/src/resources/extensions/gsd/tests/auto-loop.test.ts b/src/resources/extensions/gsd/tests/auto-loop.test.ts index 2b09de863..14627972f 100644 --- a/src/resources/extensions/gsd/tests/auto-loop.test.ts +++ b/src/resources/extensions/gsd/tests/auto-loop.test.ts @@ -595,7 +595,6 @@ test("autoLoop calls deriveState → resolveDispatch → runUnit in sequence", a ctx.sessionManager = { getSessionFile: () => "/tmp/session.json" }; const pi = makeMockPi(); - let loopCount = 0; const s = makeLoopSession(); const deps = makeMockDeps({ @@ -621,11 +620,8 @@ test("autoLoop calls deriveState → resolveDispatch → runUnit in sequence", a }, postUnitPostVerification: async () => { deps.callLog.push("postUnitPostVerification"); - loopCount++; - // After first iteration, deactivate to exit the loop - if (loopCount >= 1) { - s.active = false; - } + // Deactivate after first iteration to exit the loop + s.active = false; return "continue" as const; }, }); @@ -683,7 +679,6 @@ test("crash lock records session file from AFTER newSession, not before (#1710)" }; const pi = makeMockPi(); - let loopCount = 0; const s = makeLoopSession({ cmdCtx: { newSession: () => { @@ -731,10 +726,8 @@ test("crash lock records session file from AFTER newSession, not before (#1710)" }, postUnitPostVerification: async () => { deps.callLog.push("postUnitPostVerification"); - loopCount++; - if (loopCount >= 1) { - s.active = false; - } + // Deactivate after first iteration to exit the loop + s.active = false; return "continue" as const; }, }); @@ -791,6 +784,23 @@ test("autoLoop handles verification retry by continuing loop", async (t) => { let deriveCallCount = 0; const s = makeLoopSession(); + // Pre-queued verification actions: each entry provides a side-effect + return value + type VerifyAction = { sideEffect?: () => void; response: "retry" | "continue" }; + const verificationActions: VerifyAction[] = [ + { + sideEffect: () => { + // Simulate retry — set pendingVerificationRetry on session + s.pendingVerificationRetry = { + unitId: "M001/S01/T01", + failureContext: "test failed: expected X got Y", + attempt: 1, + }; + }, + response: "retry", + }, + { response: "continue" }, + ]; + const deps = makeMockDeps({ deriveState: async () => { deriveCallCount++; @@ -805,19 +815,11 @@ test("autoLoop handles verification retry by continuing loop", async (t) => { } as any; }, runPostUnitVerification: async () => { + const action = verificationActions[verifyCallCount] ?? { response: "continue" as const }; verifyCallCount++; deps.callLog.push("runPostUnitVerification"); - if (verifyCallCount === 1) { - // First call: simulate retry — set pendingVerificationRetry on session - s.pendingVerificationRetry = { - unitId: "M001/S01/T01", - failureContext: "test failed: expected X got Y", - attempt: 1, - }; - return "retry" as const; - } - // Second call: pass - return "continue" as const; + action.sideEffect?.(); + return action.response; }, postUnitPostVerification: async () => { deps.callLog.push("postUnitPostVerification"); @@ -894,19 +896,17 @@ test("autoLoop handles dispatch skip action by continuing", async (t) => { const s = makeLoopSession(); let dispatchCallCount = 0; + // Pre-queued dispatch responses: first call returns "skip", second returns "stop" + const dispatchResponses = [ + { action: "skip" as const }, + { action: "stop" as const, reason: "done", level: "info" as const }, + ]; const deps = makeMockDeps({ resolveDispatch: async () => { + const response = dispatchResponses[dispatchCallCount] ?? dispatchResponses[dispatchResponses.length - 1]; dispatchCallCount++; deps.callLog.push("resolveDispatch"); - if (dispatchCallCount === 1) { - return { action: "skip" as const }; - } - // Second time: stop to exit the loop - return { - action: "stop" as const, - reason: "done", - level: "info" as const, - }; + return response; }, }); @@ -936,22 +936,26 @@ test("autoLoop drains sidecar queue after postUnitPostVerification enqueues item const s = makeLoopSession(); let postVerCallCount = 0; + const postVerActions: Array<() => void> = [ + () => { + // First call (main unit): enqueue a sidecar item + s.sidecarQueue.push({ + kind: "hook" as const, + unitType: "hook/review", + unitId: "M001/S01/T01/review", + prompt: "review the code", + }); + }, + () => { + // Second call (sidecar unit completed): deactivate + s.active = false; + }, + ]; const deps = makeMockDeps({ postUnitPostVerification: async () => { + postVerActions[postVerCallCount]?.(); postVerCallCount++; deps.callLog.push("postUnitPostVerification"); - if (postVerCallCount === 1) { - // First call (main unit): enqueue a sidecar item - s.sidecarQueue.push({ - kind: "hook" as const, - unitType: "hook/review", - unitId: "M001/S01/T01/review", - prompt: "review the code", - }); - return "continue" as const; - } - // Second call (sidecar unit completed): done - s.active = false; return "continue" as const; }, }); @@ -1119,13 +1123,13 @@ test("startAuto calls selfHealRuntimeRecords before autoLoop (#1727)", () => { assert.ok(healIdx > -1, "startAuto must call selfHealRuntimeRecords"); assert.ok(healIdx < loopIdx, "selfHealRuntimeRecords must be called before autoLoop"); - // Verify the second autoLoop call site also has selfHeal before it + // Verify the second autoLoop call site also has selfHeal before it (if present) const secondLoopIdx = fnBlock.indexOf("autoLoop(", loopIdx + 1); - if (secondLoopIdx > -1) { - const secondHealIdx = fnBlock.indexOf("selfHealRuntimeRecords", healIdx + 1); - assert.ok(secondHealIdx > -1, "second autoLoop call must also have selfHealRuntimeRecords"); - assert.ok(secondHealIdx < secondLoopIdx, "second selfHealRuntimeRecords must precede second autoLoop"); - } + const secondHealIdx = fnBlock.indexOf("selfHealRuntimeRecords", healIdx + 1); + assert.ok( + secondLoopIdx === -1 || (secondHealIdx > -1 && secondHealIdx < secondLoopIdx), + "if a second autoLoop call exists, it must also be preceded by selfHealRuntimeRecords", + ); }); test("agent_end handler calls resolveAgentEnd (not handleAgentEnd)", () => { @@ -1287,25 +1291,29 @@ test("stuck detection: window resets recovery when deriveState returns a differe const s = makeLoopSession(); let deriveCallCount = 0; + let postVerCallCount = 0; let stopCalled = false; + // First 3 derives return T01, 4th returns T02; dispatch follows the derived task + const derivedTaskIds = ["T01", "T01", "T01", "T02"]; + const deps = makeMockDeps({ deriveState: async () => { + const taskId = derivedTaskIds[Math.min(deriveCallCount, derivedTaskIds.length - 1)]; deriveCallCount++; deps.callLog.push("deriveState"); return { phase: "executing", activeMilestone: { id: "M001", title: "Test", status: "active" }, activeSlice: { id: "S01", title: "Slice 1" }, - activeTask: { id: deriveCallCount <= 3 ? "T01" : "T02" }, + activeTask: { id: taskId }, registry: [{ id: "M001", status: "active" }], blockers: [], } as any; }, resolveDispatch: async () => { + const taskId = derivedTaskIds[Math.min(deriveCallCount - 1, derivedTaskIds.length - 1)]; deps.callLog.push("resolveDispatch"); - // Return dispatch matching the task from deriveState - const taskId = deriveCallCount <= 3 ? "T01" : "T02"; return { action: "dispatch" as const, unitType: "execute-task", @@ -1319,11 +1327,11 @@ test("stuck detection: window resets recovery when deriveState returns a differe s.active = false; }, postUnitPostVerification: async () => { + postVerCallCount++; deps.callLog.push("postUnitPostVerification"); - // After 4th iteration (unit changed on iter 4), exit - if (deriveCallCount >= 4) { - s.active = false; - } + // Exit on the 4th call (after T02 unit completes) + const shouldExit = postVerCallCount >= 4; + s.active = !shouldExit; return "continue" as const; }, }); @@ -1362,6 +1370,14 @@ test("stuck detection: does not push to window during verification retry", async let verifyCallCount = 0; let stopReason = ""; + // Pre-queued responses: 3 retries then a continue (exit) + const verifyActions: Array<() => "retry" | "continue"> = [ + () => { s.pendingVerificationRetry = { unitId: "M001/S01/T01", failureContext: "test failed", attempt: 1 }; return "retry"; }, + () => { s.pendingVerificationRetry = { unitId: "M001/S01/T01", failureContext: "test failed", attempt: 2 }; return "retry"; }, + () => { s.pendingVerificationRetry = { unitId: "M001/S01/T01", failureContext: "test failed", attempt: 3 }; return "retry"; }, + () => { s.active = false; return "continue"; }, + ]; + const deps = makeMockDeps({ deriveState: async () => ({ @@ -1379,20 +1395,10 @@ test("stuck detection: does not push to window during verification retry", async prompt: "do the thing", }), runPostUnitVerification: async () => { + const action = verifyActions[verifyCallCount] ?? (() => { s.active = false; return "continue" as const; }); verifyCallCount++; deps.callLog.push("runPostUnitVerification"); - if (verifyCallCount <= 3) { - // Set pendingVerificationRetry — should prevent stuck counter increment - s.pendingVerificationRetry = { - unitId: "M001/S01/T01", - failureContext: "test failed", - attempt: verifyCallCount, - }; - return "retry" as const; - } - // After 3 retries, exit gracefully - s.active = false; - return "continue" as const; + return action(); }, stopAuto: async (_ctx?: any, _pi?: any, reason?: string) => { deps.callLog.push("stopAuto"); @@ -1544,7 +1550,7 @@ test("autoLoop lifecycle: advances through research → plan → execute → ver const dispatchedUnitTypes: string[] = []; // Phase sequence: each deriveState call returns a different phase. - // On the 6th call (start of iteration 6), we deactivate to exit. + // The 6th entry (index 5) is the terminal "complete" phase that stops the loop. const phases = [ // Call 1: researching → dispatches research-slice { @@ -1576,6 +1582,12 @@ test("autoLoop lifecycle: advances through research → plan → execute → ver activeSlice: { id: "S01", title: "Complete Slice" }, activeTask: null, }, + // Call 6: terminal — deactivate to exit the loop + { + phase: "complete", + activeSlice: null, + activeTask: null, + }, ]; const dispatches = [ @@ -1588,46 +1600,26 @@ test("autoLoop lifecycle: advances through research → plan → execute → ver const deps = makeMockDeps({ deriveState: async () => { + const p = phases[Math.min(deriveCallCount, phases.length - 1)]; deriveCallCount++; deps.callLog.push("deriveState"); - if (deriveCallCount > phases.length) { - // 6th+ call: deactivate to exit the loop - s.active = false; - return { - phase: "complete", - activeMilestone: { id: "M001", title: "Test", status: "complete" }, - activeSlice: null, - activeTask: null, - registry: [{ id: "M001", status: "complete" }], - blockers: [], - } as any; - } - - const p = phases[deriveCallCount - 1]; + const terminalPhases: Record = { complete: "complete" }; + s.active = p.phase !== "complete"; + const milestoneStatus = terminalPhases[p.phase] ?? "active"; return { phase: p.phase, - activeMilestone: { id: "M001", title: "Test", status: "active" }, - activeSlice: p.activeSlice, - activeTask: p.activeTask, - registry: [{ id: "M001", status: "active" }], + activeMilestone: { id: "M001", title: "Test", status: milestoneStatus }, + activeSlice: p.activeSlice ?? null, + activeTask: p.activeTask ?? null, + registry: [{ id: "M001", status: milestoneStatus }], blockers: [], } as any; }, resolveDispatch: async () => { + const d = dispatches[Math.min(dispatchCallCount, dispatches.length - 1)]; dispatchCallCount++; deps.callLog.push("resolveDispatch"); - - if (dispatchCallCount > dispatches.length) { - // Safety: shouldn't reach here, but stop if it does - return { - action: "stop" as const, - reason: "done", - level: "info" as const, - }; - } - - const d = dispatches[dispatchCallCount - 1]; dispatchedUnitTypes.push(d.unitType); return { action: "dispatch" as const, @@ -1692,18 +1684,11 @@ test("autoLoop lifecycle: advances through research → plan → execute → ver `callLog should have at least 5 resolveDispatch entries (got ${dispatchEntries.length})`, ); - // Verify interleaving: each resolveDispatch should follow a deriveState - let dispatchSeen = 0; - for (const entry of deps.callLog) { - if (entry === "resolveDispatch") { - dispatchSeen++; - } - if (entry === "deriveState" && dispatchSeen > 0) { - // A deriveState after a resolveDispatch confirms the loop advanced - break; - } - } - assert.ok(dispatchSeen > 0, "resolveDispatch should appear in callLog"); + // Verify interleaving: a deriveState must follow a resolveDispatch (confirms loop advanced) + const firstDispatchIdx = deps.callLog.indexOf("resolveDispatch"); + const firstDeriveAfterDispatch = deps.callLog.indexOf("deriveState", firstDispatchIdx + 1); + assert.ok(firstDispatchIdx >= 0, "resolveDispatch should appear in callLog"); + assert.ok(firstDeriveAfterDispatch > firstDispatchIdx, "deriveState should follow resolveDispatch to confirm loop advanced"); // Assert the exact sequence of dispatched unit types assert.deepEqual( @@ -1776,6 +1761,8 @@ test("autoLoop re-iterates when postUnitPreVerification returns retry (#1571)", const s = makeLoopSession(); let preVerifyCallCount = 0; + // Pre-queued responses: first call returns "retry", second returns "continue" + const preVerifyResponses = ["retry", "continue"] as const; const deps = makeMockDeps({ deriveState: async () => { @@ -1791,11 +1778,7 @@ test("autoLoop re-iterates when postUnitPreVerification returns retry (#1571)", }, postUnitPreVerification: async () => { deps.callLog.push("postUnitPreVerification"); - preVerifyCallCount++; - if (preVerifyCallCount === 1) { - return "retry" as const; - } - return "continue" as const; + return preVerifyResponses[preVerifyCallCount++] ?? "continue"; }, postUnitPostVerification: async () => { deps.callLog.push("postUnitPostVerification"); @@ -1908,9 +1891,8 @@ test("autoLoop rejects execute-task with 0 tool calls as hallucinated (#1833)", postUnitPostVerification: async () => { deps.callLog.push("postUnitPostVerification"); iterationCount++; - if (iterationCount >= 2) { - s.active = false; - } + // Deactivate after 2nd iteration + s.active = iterationCount < 2; return "continue" as const; }, }); diff --git a/src/resources/extensions/gsd/tests/auto-secrets-gate.test.ts b/src/resources/extensions/gsd/tests/auto-secrets-gate.test.ts index e73fe849c..a7512634f 100644 --- a/src/resources/extensions/gsd/tests/auto-secrets-gate.test.ts +++ b/src/resources/extensions/gsd/tests/auto-secrets-gate.test.ts @@ -101,8 +101,8 @@ test('secrets gate: pending keys exist — gate triggers collection, manifest up const status = await getManifestStatus(tmp, 'M001'); assert.notStrictEqual(status, null, 'manifest should exist'); assert.ok(status!.pending.length > 0, 'should have pending keys'); - assert.deepStrictEqual(status!.pending, ['GSD_GATE_TEST_PEND_A', 'GSD_GATE_TEST_PEND_B']); - assert.deepStrictEqual(status!.existing, ['GSD_GATE_TEST_EXISTING']); + assert.deepStrictEqual(status!.pending, ['GSD_GATE_TEST_PEND_A', 'GSD_GATE_TEST_PEND_B'], 'pending keys'); + assert.deepStrictEqual(status!.existing, ['GSD_GATE_TEST_EXISTING'], 'existing keys'); // (b) Call collectSecretsFromManifest with no-UI context // With hasUI: false, collectOneSecret returns null → pending keys become "skipped" diff --git a/src/resources/extensions/gsd/tests/captures.test.ts b/src/resources/extensions/gsd/tests/captures.test.ts index 219667929..f18e7c49c 100644 --- a/src/resources/extensions/gsd/tests/captures.test.ts +++ b/src/resources/extensions/gsd/tests/captures.test.ts @@ -119,12 +119,23 @@ test("captures: loadPendingCaptures filters resolved entries", () => { const id1 = appendCapture(tmp, "pending one"); appendCapture(tmp, "pending two"); - // Resolve the first one markCaptureResolved(tmp, id1, "note", "acknowledged", "just a note"); const pending = loadPendingCaptures(tmp); assert.strictEqual(pending.length, 1, "should have 1 pending"); assert.strictEqual(pending[0].text, "pending two"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +}); + +test("captures: loadAllCaptures preserves resolved entries", () => { + const tmp = makeTempDir("cap-all-resolved"); + try { + const id1 = appendCapture(tmp, "pending one"); + appendCapture(tmp, "pending two"); + + markCaptureResolved(tmp, id1, "note", "acknowledged", "just a note"); const all = loadAllCaptures(tmp); assert.strictEqual(all.length, 2, "all should still have 2"); diff --git a/src/resources/extensions/gsd/tests/continue-here.test.ts b/src/resources/extensions/gsd/tests/continue-here.test.ts index eb31e084f..08bd595c3 100644 --- a/src/resources/extensions/gsd/tests/continue-here.test.ts +++ b/src/resources/extensions/gsd/tests/continue-here.test.ts @@ -72,18 +72,17 @@ describe("continue-here", () => { const budget = computeBudgets(128_000); const threshold = budget.continueThresholdPercent; - // Simulate repeated polls with percent above threshold - let fired = false; - let fireCount = 0; + // Simulate repeated polls with percent above threshold using a reducer + // so there is no control flow inside the test body. const usagePercents = [75, 80, 85, 90, 95]; - - for (const percent of usagePercents) { - if (fired) continue; // one-shot guard - if (percent >= threshold) { - fired = true; - fireCount++; - } - } + const { fired, fireCount } = usagePercents.reduce( + (acc, percent) => { + if (acc.fired) return acc; // one-shot guard + if (percent >= threshold) return { fired: true, fireCount: acc.fireCount + 1 }; + return acc; + }, + { fired: false, fireCount: 0 }, + ); assert.equal(fireCount, 1, "must fire exactly once"); assert.equal(fired, true); @@ -97,16 +96,17 @@ describe("continue-here", () => { { name: "1M", contextWindow: 1_000_000 }, ]; - it("all model sizes produce continueThresholdPercent of 70", () => { - for (const { name, contextWindow } of modelSizes) { + const thresholdCases: Array<[string, number]> = [ + ["128K", 128_000], + ["200K", 200_000], + ["1M", 1_000_000], + ]; + for (const [name, contextWindow] of thresholdCases) { + it(`${name} model produces continueThresholdPercent of 70`, () => { const budget = computeBudgets(contextWindow); - assert.equal( - budget.continueThresholdPercent, - 70, - `${name} model should have 70% threshold`, - ); - } - }); + assert.equal(budget.continueThresholdPercent, 70, `${name} model should have 70% threshold`); + }); + } it("larger models produce larger verificationBudgetChars", () => { const budgets = modelSizes.map(({ contextWindow }) => computeBudgets(contextWindow)); diff --git a/src/resources/extensions/gsd/tests/worktree-manager.test.ts b/src/resources/extensions/gsd/tests/worktree-manager.test.ts index ca6b738f3..9b836ad30 100644 --- a/src/resources/extensions/gsd/tests/worktree-manager.test.ts +++ b/src/resources/extensions/gsd/tests/worktree-manager.test.ts @@ -1,4 +1,6 @@ -import { mkdtempSync, mkdirSync, rmSync, writeFileSync, existsSync, readFileSync } from "node:fs"; +import test from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync, existsSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { execSync } from "node:child_process"; @@ -13,82 +15,42 @@ import { worktreeBranchName, worktreePath, } from "../worktree-manager.ts"; -import { createTestContext } from './test-helpers.ts'; -const { assertEq, assertTrue, report } = createTestContext(); function run(command: string, cwd: string): string { return execSync(command, { cwd, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" }).trim(); } -// Set up a test repo -const base = mkdtempSync(join(tmpdir(), "gsd-worktree-mgr-test-")); -run("git init -b main", base); -run('git config user.name "Pi Test"', base); -run('git config user.email "pi@example.com"', base); +function makeBaseRepo(): string { + const base = mkdtempSync(join(tmpdir(), "gsd-wt-test-")); + run("git init -b main", base); + run('git config user.name "Test User"', base); + run('git config user.email "test@example.com"', base); + mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true }); + writeFileSync(join(base, "README.md"), "# Test Project\n", "utf-8"); + writeFileSync( + join(base, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), + "# M001: Demo\n\n## Slices\n- [ ] **S01: First** `risk:low` `depends:[]`\n > After this: it works\n", + "utf-8", + ); + run("git add .", base); + run('git commit -m "chore: init"', base); + return base; +} -// Create initial project structure -mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true }); -writeFileSync(join(base, "README.md"), "# Test Project\n", "utf-8"); -writeFileSync( - join(base, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), - "# M001: Demo\n\n## Slices\n- [ ] **S01: First** `risk:low` `depends:[]`\n > After this: it works\n", - "utf-8", -); -run("git add .", base); -run('git commit -m "chore: init"', base); +function makeRepoWithWorktree(worktreeName: string): { base: string; wtPath: string } { + const base = makeBaseRepo(); + createWorktree(base, worktreeName); + return { base, wtPath: worktreePath(base, worktreeName) }; +} -async function main(): Promise { - console.log("\n=== worktreeBranchName ==="); - assertEq(worktreeBranchName("feature-x"), "worktree/feature-x", "branch name format"); - - console.log("\n=== createWorktree ==="); - const info = createWorktree(base, "feature-x"); - assertTrue(info.name === "feature-x", "name matches"); - assertTrue(info.branch === "worktree/feature-x", "branch matches"); - assertTrue(info.exists, "worktree exists"); - assertTrue(existsSync(info.path), "worktree path exists on disk"); - assertTrue(existsSync(join(info.path, "README.md")), "README.md copied to worktree"); - assertTrue(existsSync(join(info.path, ".gsd", "milestones", "M001", "M001-ROADMAP.md")), ".gsd files copied"); - - // Branch was created - const branches = run("git branch", base); - assertTrue(branches.includes("worktree/feature-x"), "branch was created"); - - console.log("\n=== createWorktree — duplicate ==="); - let duplicateError = ""; - try { - createWorktree(base, "feature-x"); - } catch (e) { - duplicateError = (e as Error).message; - } - assertTrue(duplicateError.includes("already exists"), "duplicate creation fails"); - - console.log("\n=== createWorktree — invalid name ==="); - let invalidError = ""; - try { - createWorktree(base, "bad name!"); - } catch (e) { - invalidError = (e as Error).message; - } - assertTrue(invalidError.includes("Invalid worktree name"), "invalid name rejected"); - - console.log("\n=== listWorktrees ==="); - const list = listWorktrees(base); - assertEq(list.length, 1, "one worktree listed"); - assertEq(list[0]!.name, "feature-x", "correct name"); - assertEq(list[0]!.branch, "worktree/feature-x", "correct branch"); - assertTrue(list[0]!.exists, "exists flag is true"); - - console.log("\n=== make changes in worktree ==="); - const wtPath = worktreePath(base, "feature-x"); - // Add a new GSD artifact in the worktree +function makeRepoWithChanges(worktreeName: string): { base: string; wtPath: string } { + const { base, wtPath } = makeRepoWithWorktree(worktreeName); mkdirSync(join(wtPath, ".gsd", "milestones", "M002"), { recursive: true }); writeFileSync( join(wtPath, ".gsd", "milestones", "M002", "M002-ROADMAP.md"), "# M002: New Feature\n\n## Slices\n- [ ] **S01: Setup** `risk:low` `depends:[]`\n > After this: new feature ready\n", "utf-8", ); - // Modify an existing artifact writeFileSync( join(wtPath, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), "# M001: Demo (updated)\n\n## Slices\n- [x] **S01: First** `risk:low` `depends:[]`\n > Done\n", @@ -96,46 +58,174 @@ async function main(): Promise { ); run("git add .", wtPath); run('git commit -m "feat: add M002 and update M001"', wtPath); - - console.log("\n=== diffWorktreeGSD ==="); - const diff = diffWorktreeGSD(base, "feature-x"); - assertTrue(diff.added.length > 0, "has added files"); - assertTrue(diff.added.some(f => f.includes("M002")), "M002 roadmap is in added"); - assertTrue(diff.modified.length > 0, "has modified files"); - assertTrue(diff.modified.some(f => f.includes("M001")), "M001 roadmap is in modified"); - assertEq(diff.removed.length, 0, "no removed files"); - - console.log("\n=== getWorktreeGSDDiff ==="); - const fullDiff = getWorktreeGSDDiff(base, "feature-x"); - assertTrue(fullDiff.includes("M002"), "full diff mentions M002"); - assertTrue(fullDiff.includes("updated"), "full diff mentions update"); - - console.log("\n=== getWorktreeLog ==="); - const log = getWorktreeLog(base, "feature-x"); - assertTrue(log.includes("add M002"), "log shows commit message"); - - console.log("\n=== removeWorktree ==="); - removeWorktree(base, "feature-x", { deleteBranch: true }); - assertTrue(!existsSync(wtPath), "worktree directory removed"); - const branchesAfter = run("git branch", base); - assertTrue(!branchesAfter.includes("worktree/feature-x"), "branch deleted"); - - console.log("\n=== listWorktrees after removal ==="); - const listAfter = listWorktrees(base); - assertEq(listAfter.length, 0, "no worktrees after removal"); - - console.log("\n=== removeWorktree — already gone ==="); - // Should not throw - removeWorktree(base, "feature-x", { deleteBranch: true }); - assertTrue(true, "removeWorktree on missing worktree does not throw"); - - // Cleanup - rmSync(base, { recursive: true, force: true }); - - report(); + return { base, wtPath }; } -main().catch((error) => { - console.error(error); - process.exit(1); +// ─── worktreeBranchName ─────────────────────────────────────────────────────── + +test("worktreeBranchName formats branch name", () => { + assert.strictEqual( + worktreeBranchName("feature-x"), + "worktree/feature-x", + "should prefix with worktree/", + ); +}); + +// ─── createWorktree ─────────────────────────────────────────────────────────── + +test("createWorktree creates worktree with correct metadata", () => { + const base = makeBaseRepo(); + try { + const info = createWorktree(base, "feature-x"); + assert.strictEqual(info.name, "feature-x", "name should match"); + assert.strictEqual(info.branch, "worktree/feature-x", "branch should be prefixed"); + assert.ok(info.exists, "exists flag should be true"); + assert.ok(existsSync(info.path), "worktree path should exist on disk"); + assert.ok(existsSync(join(info.path, "README.md")), "README.md should be in worktree"); + assert.ok( + existsSync(join(info.path, ".gsd", "milestones", "M001", "M001-ROADMAP.md")), + ".gsd files should be in worktree", + ); + const branches = run("git branch", base); + assert.ok(branches.includes("worktree/feature-x"), "branch should be created in base repo"); + } finally { + rmSync(base, { recursive: true, force: true }); + } +}); + +test("createWorktree rejects duplicate name", () => { + const { base } = makeRepoWithWorktree("feature-x"); + try { + assert.throws( + () => createWorktree(base, "feature-x"), + (err: Error) => { + assert.ok( + err.message.includes("already exists"), + `expected "already exists" in error, got: ${err.message}`, + ); + return true; + }, + "should throw on duplicate worktree name", + ); + } finally { + rmSync(base, { recursive: true, force: true }); + } +}); + +test("createWorktree rejects invalid name", () => { + const base = makeBaseRepo(); + try { + assert.throws( + () => createWorktree(base, "bad name!"), + (err: Error) => { + assert.ok( + err.message.includes("Invalid worktree name"), + `expected "Invalid worktree name" in error, got: ${err.message}`, + ); + return true; + }, + "should throw on invalid worktree name", + ); + } finally { + rmSync(base, { recursive: true, force: true }); + } +}); + +// ─── listWorktrees ──────────────────────────────────────────────────────────── + +test("listWorktrees returns active worktrees", () => { + const { base } = makeRepoWithWorktree("feature-x"); + try { + const list = listWorktrees(base); + assert.strictEqual(list.length, 1, "should list exactly one worktree"); + assert.strictEqual(list[0]!.name, "feature-x", "name should match"); + assert.strictEqual(list[0]!.branch, "worktree/feature-x", "branch should match"); + assert.ok(list[0]!.exists, "exists flag should be true"); + } finally { + rmSync(base, { recursive: true, force: true }); + } +}); + +test("listWorktrees returns empty after removal", () => { + const { base } = makeRepoWithWorktree("feature-x"); + try { + removeWorktree(base, "feature-x"); + const list = listWorktrees(base); + assert.strictEqual(list.length, 0, "should have no worktrees after removal"); + } finally { + rmSync(base, { recursive: true, force: true }); + } +}); + +// ─── diffWorktreeGSD ───────────────────────────────────────────────────────── + +test("diffWorktreeGSD detects added and modified GSD files", () => { + const { base } = makeRepoWithChanges("feature-x"); + try { + const diff = diffWorktreeGSD(base, "feature-x"); + assert.ok(diff.added.length > 0, "should have added files"); + assert.ok( + diff.added.some((f) => f.includes("M002")), + "M002 roadmap should be in added files", + ); + assert.ok(diff.modified.length > 0, "should have modified files"); + assert.ok( + diff.modified.some((f) => f.includes("M001")), + "M001 roadmap should be in modified files", + ); + assert.strictEqual(diff.removed.length, 0, "should have no removed files"); + } finally { + rmSync(base, { recursive: true, force: true }); + } +}); + +// ─── getWorktreeGSDDiff ─────────────────────────────────────────────────────── + +test("getWorktreeGSDDiff returns patch content", () => { + const { base } = makeRepoWithChanges("feature-x"); + try { + const fullDiff = getWorktreeGSDDiff(base, "feature-x"); + assert.ok(fullDiff.includes("M002"), "diff should mention M002"); + assert.ok(fullDiff.includes("updated"), "diff should mention the update"); + } finally { + rmSync(base, { recursive: true, force: true }); + } +}); + +// ─── getWorktreeLog ─────────────────────────────────────────────────────────── + +test("getWorktreeLog shows commits", () => { + const { base } = makeRepoWithChanges("feature-x"); + try { + const log = getWorktreeLog(base, "feature-x"); + assert.ok(log.includes("add M002"), "log should include the commit message"); + } finally { + rmSync(base, { recursive: true, force: true }); + } +}); + +// ─── removeWorktree ─────────────────────────────────────────────────────────── + +test("removeWorktree removes directory and branch", () => { + const { base, wtPath } = makeRepoWithWorktree("feature-x"); + try { + removeWorktree(base, "feature-x", { deleteBranch: true }); + assert.ok(!existsSync(wtPath), "worktree directory should be gone"); + const branches = run("git branch", base); + assert.ok(!branches.includes("worktree/feature-x"), "branch should be deleted"); + } finally { + rmSync(base, { recursive: true, force: true }); + } +}); + +test("removeWorktree on missing worktree does not throw", () => { + const base = makeBaseRepo(); + try { + assert.doesNotThrow( + () => removeWorktree(base, "nonexistent"), + "should not throw when worktree does not exist", + ); + } finally { + rmSync(base, { recursive: true, force: true }); + } }); diff --git a/src/tests/fetch-test-helpers.ts b/src/tests/fetch-test-helpers.ts new file mode 100644 index 000000000..01a0ac321 --- /dev/null +++ b/src/tests/fetch-test-helpers.ts @@ -0,0 +1,20 @@ +/** + * Shared fetch-mocking utilities for test files that need to intercept + * globalThis.fetch and inspect request headers/body. + */ + +export function normalizeHeaders(headers: HeadersInit | undefined): Record | undefined { + if (headers == null) return undefined; + if (headers instanceof Headers) { + const result: Record = {}; + headers.forEach((v, k) => { result[k] = v; }); + return result; + } + if (Array.isArray(headers)) return Object.fromEntries(headers); + return headers as Record; +} + +export function parseJsonBody(body: BodyInit | null | undefined): Record | undefined { + if (body == null || typeof body !== "string") return undefined; + try { return JSON.parse(body); } catch { return undefined; } +} diff --git a/src/tests/llm-context-tavily.test.ts b/src/tests/llm-context-tavily.test.ts index a18b271c6..3e62093f7 100644 --- a/src/tests/llm-context-tavily.test.ts +++ b/src/tests/llm-context-tavily.test.ts @@ -18,6 +18,7 @@ import { publishedDateToAge } from "../resources/extensions/search-the-web/tavil import type { TavilyResult } from "../resources/extensions/search-the-web/tavily.ts"; import { resolveSearchProvider } from "../resources/extensions/search-the-web/provider.ts"; import { normalizeQuery } from "../resources/extensions/search-the-web/url-utils.ts"; +import { normalizeHeaders, parseJsonBody } from "./fetch-test-helpers.ts"; // ============================================================================= // Helpers @@ -78,21 +79,8 @@ function mockFetch(responseBody: unknown, status = 200) { const url = typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; captured.url = url; captured.method = init?.method || "GET"; - - if (init?.headers) { - if (init.headers instanceof Headers) { - captured.headers = {}; - init.headers.forEach((v, k) => { captured.headers![k] = v; }); - } else if (Array.isArray(init.headers)) { - captured.headers = Object.fromEntries(init.headers); - } else { - captured.headers = init.headers as Record; - } - } - - if (init?.body && typeof init.body === "string") { - try { captured.body = JSON.parse(init.body); } catch { /* ignore */ } - } + captured.headers = normalizeHeaders(init?.headers); + captured.body = parseJsonBody(init?.body); return new Response(JSON.stringify(responseBody), { status, diff --git a/src/tests/search-provider-command.test.ts b/src/tests/search-provider-command.test.ts index ce8336221..9540a5c02 100644 --- a/src/tests/search-provider-command.test.ts +++ b/src/tests/search-provider-command.test.ts @@ -138,8 +138,8 @@ test('direct arg "tavily" sets preference and notifies', async () => { // Notification sent assert.equal(ctx.ui.notifyCalls.length, 1, 'should notify once') - assert.match(ctx.ui.notifyCalls[0].message, /Search provider set to tavily/) - assert.match(ctx.ui.notifyCalls[0].message, /Effective provider: tavily/) + assert.match(ctx.ui.notifyCalls[0].message, /Search provider set to tavily/, 'notification should confirm provider set') + assert.match(ctx.ui.notifyCalls[0].message, /Effective provider: tavily/, 'notification should show effective provider') }) } finally { cleanup() @@ -286,10 +286,8 @@ test('tab completion returns all 4 options when prefix is empty', async () => { assert.deepEqual(values, ['tavily', 'brave', 'ollama', 'auto']) // Each item has label and description - for (const item of items!) { - assert.ok(item.label, 'each item should have a label') - assert.ok(item.description, 'each item should have a description') - } + assert.ok(items!.every((i: any) => i.label), 'every item should have a label') + assert.ok(items!.every((i: any) => i.description), 'every item should have a description') }) }) diff --git a/src/tests/search-tavily.test.ts b/src/tests/search-tavily.test.ts index ab8809d00..456abb7a4 100644 --- a/src/tests/search-tavily.test.ts +++ b/src/tests/search-tavily.test.ts @@ -17,6 +17,7 @@ import assert from "node:assert/strict"; import { resolveSearchProvider } from "../resources/extensions/search-the-web/provider.ts"; import { normalizeQuery } from "../resources/extensions/search-the-web/url-utils.ts"; import { mapFreshnessToTavily } from "../resources/extensions/search-the-web/tavily.ts"; +import { normalizeHeaders, parseJsonBody } from "./fetch-test-helpers.ts"; // ============================================================================= // Helpers for mocking global fetch @@ -64,24 +65,9 @@ function mockFetch(responseBody: unknown, status = 200) { globalThis.fetch = async (input: string | URL | Request, init?: RequestInit) => { const url = typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; captured.url = url; - captured.method = init?.method || "GET"; - - // Capture headers - if (init?.headers) { - if (init.headers instanceof Headers) { - captured.headers = {}; - init.headers.forEach((v, k) => { captured.headers![k] = v; }); - } else if (Array.isArray(init.headers)) { - captured.headers = Object.fromEntries(init.headers); - } else { - captured.headers = init.headers as Record; - } - } - - // Capture body - if (init?.body && typeof init.body === "string") { - try { captured.body = JSON.parse(init.body); } catch { /* ignore */ } - } + captured.method = init?.method ?? "GET"; + captured.headers = normalizeHeaders(init?.headers); + captured.body = parseJsonBody(init?.body); return new Response(JSON.stringify(responseBody), { status, @@ -133,11 +119,11 @@ test("executeTavilySearch sends POST to Tavily API and produces CachedSearchResu const data = await response.json() as { results: Array<{ title: string; url: string; content: string; score: number; published_date?: string }> }; // Verify request shape - assert.equal(captured.url, "https://api.tavily.com/search"); - assert.equal(captured.method, "POST"); - assert.equal(captured.headers?.["Content-Type"], "application/json"); - assert.equal(captured.headers?.["Authorization"], "Bearer tvly-test-key-12345"); - assert.deepEqual(captured.body, requestBody); + assert.equal(captured.url, "https://api.tavily.com/search", "request URL"); + assert.equal(captured.method, "POST", "HTTP method"); + assert.equal(captured.headers?.["Content-Type"], "application/json", "Content-Type header"); + assert.equal(captured.headers?.["Authorization"], "Bearer tvly-test-key-12345", "Authorization header"); + assert.deepEqual(captured.body, requestBody, "request body"); // Verify response mapping const mapped = data.results.map(normalizeTavilyResult); diff --git a/src/tests/tavily-helpers.test.ts b/src/tests/tavily-helpers.test.ts index 853e6c428..fe1cf4b82 100644 --- a/src/tests/tavily-helpers.test.ts +++ b/src/tests/tavily-helpers.test.ts @@ -89,39 +89,39 @@ test("publishedDateToAge returns correct relative strings for various offsets", // Seconds ago → "just now" const secondsAgo = new Date(now - 30 * 1000).toISOString(); - assert.equal(publishedDateToAge(secondsAgo), "just now"); + assert.equal(publishedDateToAge(secondsAgo), "just now", "30 seconds ago → just now"); // Minutes ago const minutesAgo = new Date(now - 5 * 60 * 1000).toISOString(); - assert.equal(publishedDateToAge(minutesAgo), "5 minutes ago"); + assert.equal(publishedDateToAge(minutesAgo), "5 minutes ago", "5 minutes ago → plural"); // 1 minute ago (singular) const oneMinAgo = new Date(now - 60 * 1000).toISOString(); - assert.equal(publishedDateToAge(oneMinAgo), "1 minute ago"); + assert.equal(publishedDateToAge(oneMinAgo), "1 minute ago", "1 minute ago → singular"); // Hours ago const hoursAgo = new Date(now - 7 * 60 * 60 * 1000).toISOString(); - assert.equal(publishedDateToAge(hoursAgo), "7 hours ago"); + assert.equal(publishedDateToAge(hoursAgo), "7 hours ago", "7 hours ago → plural"); // 1 hour ago (singular) const oneHourAgo = new Date(now - 60 * 60 * 1000).toISOString(); - assert.equal(publishedDateToAge(oneHourAgo), "1 hour ago"); + assert.equal(publishedDateToAge(oneHourAgo), "1 hour ago", "1 hour ago → singular"); // Days ago const daysAgo = new Date(now - 10 * 24 * 60 * 60 * 1000).toISOString(); - assert.equal(publishedDateToAge(daysAgo), "10 days ago"); + assert.equal(publishedDateToAge(daysAgo), "10 days ago", "10 days ago → plural"); // 1 day ago (singular) const oneDayAgo = new Date(now - 24 * 60 * 60 * 1000).toISOString(); - assert.equal(publishedDateToAge(oneDayAgo), "1 day ago"); + assert.equal(publishedDateToAge(oneDayAgo), "1 day ago", "1 day ago → singular"); // Months ago (35 days → 1 month) const monthsAgo = new Date(now - 65 * 24 * 60 * 60 * 1000).toISOString(); - assert.equal(publishedDateToAge(monthsAgo), "2 months ago"); + assert.equal(publishedDateToAge(monthsAgo), "2 months ago", "65 days ago → 2 months ago"); // Years ago const yearsAgo = new Date(now - 400 * 24 * 60 * 60 * 1000).toISOString(); - assert.equal(publishedDateToAge(yearsAgo), "1 year ago"); + assert.equal(publishedDateToAge(yearsAgo), "1 year ago", "400 days ago → 1 year ago"); }); test("publishedDateToAge returns undefined for invalid date string", () => { diff --git a/src/tests/web-boot-node24.test.ts b/src/tests/web-boot-node24.test.ts index f64d3b654..f103070cf 100644 --- a/src/tests/web-boot-node24.test.ts +++ b/src/tests/web-boot-node24.test.ts @@ -7,6 +7,9 @@ import { resolveTypeStrippingFlag } from "../web/ts-subprocess-flags.ts" // Bug 1 — resolveTypeStrippingFlag selects the correct flag // --------------------------------------------------------------------------- +const [nodeMajor, nodeMinor] = process.versions.node.split(".").map(Number) +const isNode22_7OrNewer = nodeMajor > 22 || (nodeMajor === 22 && nodeMinor >= 7) + test("resolveTypeStrippingFlag returns --experimental-strip-types for paths outside node_modules", () => { const flag = resolveTypeStrippingFlag("/home/user/projects/gsd") assert.equal(flag, "--experimental-strip-types") @@ -18,28 +21,42 @@ test("resolveTypeStrippingFlag returns --experimental-strip-types for path with assert.equal(flag, "--experimental-strip-types") }) -test("resolveTypeStrippingFlag returns --experimental-transform-types for paths under node_modules/ on Node >= 22.7", () => { - const [major, minor] = process.versions.node.split(".").map(Number) - const flag = resolveTypeStrippingFlag("/usr/lib/node_modules/gsd-pi") - - if (major > 22 || (major === 22 && minor >= 7)) { +test( + "resolveTypeStrippingFlag returns --experimental-transform-types for paths under node_modules/ on Node >= 22.7", + { skip: !isNode22_7OrNewer }, + () => { + const flag = resolveTypeStrippingFlag("/usr/lib/node_modules/gsd-pi") assert.equal(flag, "--experimental-transform-types") - } else { + }, +) + +test( + "resolveTypeStrippingFlag returns --experimental-strip-types for paths under node_modules/ on Node < 22.7", + { skip: isNode22_7OrNewer }, + () => { + const flag = resolveTypeStrippingFlag("/usr/lib/node_modules/gsd-pi") // On older Node, falls back to strip-types since transform-types isn't available assert.equal(flag, "--experimental-strip-types") - } -}) + }, +) -test("resolveTypeStrippingFlag handles Windows-style paths under node_modules", () => { - const [major, minor] = process.versions.node.split(".").map(Number) - const flag = resolveTypeStrippingFlag("C:\\Users\\dev\\AppData\\node_modules\\gsd-pi") - - if (major > 22 || (major === 22 && minor >= 7)) { +test( + "resolveTypeStrippingFlag handles Windows-style paths under node_modules on Node >= 22.7", + { skip: !isNode22_7OrNewer }, + () => { + const flag = resolveTypeStrippingFlag("C:\\Users\\dev\\AppData\\node_modules\\gsd-pi") assert.equal(flag, "--experimental-transform-types") - } else { + }, +) + +test( + "resolveTypeStrippingFlag handles Windows-style paths under node_modules on Node < 22.7", + { skip: isNode22_7OrNewer }, + () => { + const flag = resolveTypeStrippingFlag("C:\\Users\\dev\\AppData\\node_modules\\gsd-pi") assert.equal(flag, "--experimental-strip-types") - } -}) + }, +) // --------------------------------------------------------------------------- // Bug 2 — waitForBootReady fails fast on consecutive 5xx @@ -49,75 +66,60 @@ test("resolveTypeStrippingFlag handles Windows-style paths under node_modules", // by verifying the launchWebMode deps injection. We test the core logic // pattern directly: 3 consecutive 5xx should abort without waiting for timeout. -test("waitForBootReady pattern: consecutive 5xx detection aborts early", async () => { - // Simulate the retry logic extracted from waitForBootReady - let consecutive5xx = 0 - const MAX_CONSECUTIVE_5XX = 3 - const responses = [500, 500, 500] // three deterministic 500s - let abortedEarly = false +type RetryEvent = { type: "response"; status: number } | { type: "error" } - for (const statusCode of responses) { - if (statusCode >= 500) { - consecutive5xx++ - if (consecutive5xx >= MAX_CONSECUTIVE_5XX) { - abortedEarly = true - break - } - } else { - consecutive5xx = 0 - } - } +/** + * Simulate the consecutive-5xx abort logic extracted from waitForBootReady. + * Returns { abortedEarly, consecutiveCount }. + */ +function simulateConsecutive5xxDetection( + events: RetryEvent[], + maxConsecutive: number, +): { abortedEarly: boolean; consecutiveCount: number } { + return events.reduce( + (acc, event) => { + if (acc.abortedEarly) return acc + const is5xx = event.type === "response" && event.status >= 500 + const consecutive = is5xx ? acc.consecutiveCount + 1 : 0 + const abortedEarly = consecutive >= maxConsecutive + return { abortedEarly, consecutiveCount: consecutive } + }, + { abortedEarly: false, consecutiveCount: 0 }, + ) +} +test("waitForBootReady pattern: consecutive 5xx detection aborts early", () => { + const responses: RetryEvent[] = [ + { type: "response", status: 500 }, + { type: "response", status: 500 }, + { type: "response", status: 500 }, + ] + const { abortedEarly, consecutiveCount } = simulateConsecutive5xxDetection(responses, 3) assert.equal(abortedEarly, true, "should abort after 3 consecutive 5xx responses") - assert.equal(consecutive5xx, 3) + assert.equal(consecutiveCount, 3) }) test("waitForBootReady pattern: non-5xx responses reset the consecutive counter", () => { - let consecutive5xx = 0 - const MAX_CONSECUTIVE_5XX = 3 // 500, 500, connection-refused (resets), 500, 500 — should NOT trigger abort - const events = [ + const events: RetryEvent[] = [ { type: "response", status: 500 }, { type: "response", status: 500 }, { type: "error" }, // connection refused resets counter { type: "response", status: 500 }, { type: "response", status: 500 }, ] - let abortedEarly = false - - for (const event of events) { - if (event.type === "response" && (event.status ?? 0) >= 500) { - consecutive5xx++ - if (consecutive5xx >= MAX_CONSECUTIVE_5XX) { - abortedEarly = true - break - } - } else { - consecutive5xx = 0 - } - } - + const { abortedEarly } = simulateConsecutive5xxDetection(events, 3) assert.equal(abortedEarly, false, "should not abort when errors reset the counter") }) test("waitForBootReady pattern: mixed 4xx and 5xx only counts 5xx", () => { - let consecutive5xx = 0 - const MAX_CONSECUTIVE_5XX = 3 - const responses = [500, 404, 500, 500] - let abortedEarly = false - - for (const statusCode of responses) { - if (statusCode >= 500) { - consecutive5xx++ - if (consecutive5xx >= MAX_CONSECUTIVE_5XX) { - abortedEarly = true - break - } - } else { - consecutive5xx = 0 - } - } - + const responses: RetryEvent[] = [ + { type: "response", status: 500 }, + { type: "response", status: 404 }, + { type: "response", status: 500 }, + { type: "response", status: 500 }, + ] + const { abortedEarly } = simulateConsecutive5xxDetection(responses, 3) assert.equal(abortedEarly, false, "404 should reset the consecutive 5xx counter") }) diff --git a/src/tests/web-command-parity-contract.test.ts b/src/tests/web-command-parity-contract.test.ts index ada364c77..c21660ab0 100644 --- a/src/tests/web-command-parity-contract.test.ts +++ b/src/tests/web-command-parity-contract.test.ts @@ -88,10 +88,9 @@ test("authoritative built-ins never fall through to prompt/follow_up in browser ) for (const builtin of BUILTIN_SLASH_COMMANDS) { - await t.test(`/${builtin.name} -> ${EXPECTED_BUILTIN_OUTCOMES.get(builtin.name)}`, () => { - const outcome = dispatchBrowserSlashCommand(`/${builtin.name}`) - const expectedKind = EXPECTED_BUILTIN_OUTCOMES.get(builtin.name) - + const expectedKind = EXPECTED_BUILTIN_OUTCOMES.get(builtin.name) + const outcome = dispatchBrowserSlashCommand(`/${builtin.name}`) + await t.test(`/${builtin.name} -> ${expectedKind}`, () => { assert.ok(expectedKind, `missing explicit browser expectation for /${builtin.name}`) assert.notEqual( outcome.kind, @@ -99,40 +98,45 @@ test("authoritative built-ins never fall through to prompt/follow_up in browser `/${builtin.name} must not fall through to prompt/follow_up in browser mode`, ) assert.equal(outcome.kind, expectedKind, `/${builtin.name} resolved to ${outcome.kind}`) + }) - if (outcome.kind === "reject") { + if (outcome.kind === "reject") { + await t.test(`/${builtin.name} reject notice is browser-visible`, () => { + const outcome = dispatchBrowserSlashCommand(`/${builtin.name}`) const notice = getBrowserSlashCommandTerminalNotice(outcome) assert.ok(notice, `/${builtin.name} should produce a browser-visible reject notice`) assert.equal(notice.type, "error", `/${builtin.name} reject notice should be an error line`) assert.match(notice.message, new RegExp(`/${builtin.name}`), `/${builtin.name} notice should name the command`) assert.match(notice.message, /blocked instead of falling through to the model/i) - } - }) + }) + } } }) test("browser-local aliases and legacy helpers stay explicit", async (t) => { - const explicitCases = [ - { input: "/state", expectedKind: "rpc", expectedCommandType: "get_state" }, - { input: "/new-session", expectedKind: "rpc", expectedCommandType: "new_session" }, - { input: "/refresh", expectedKind: "local", expectedAction: "refresh_workspace" }, - { input: "/clear", expectedKind: "local", expectedAction: "clear_terminal" }, - ] as const + await t.test("/state dispatches to rpc get_state", () => { + const outcome = dispatchBrowserSlashCommand("/state") + assert.equal(outcome.kind, "rpc") + assert.equal((outcome as any).command.type, "get_state") + }) - for (const scenario of explicitCases) { - await t.test(scenario.input, () => { - const outcome = dispatchBrowserSlashCommand(scenario.input) - assert.equal(outcome.kind, scenario.expectedKind, `${scenario.input} resolved to ${outcome.kind}`) + await t.test("/new-session dispatches to rpc new_session", () => { + const outcome = dispatchBrowserSlashCommand("/new-session") + assert.equal(outcome.kind, "rpc") + assert.equal((outcome as any).command.type, "new_session") + }) - if (outcome.kind === "rpc") { - assert.equal(outcome.command.type, scenario.expectedCommandType) - } + await t.test("/refresh dispatches to local refresh_workspace", () => { + const outcome = dispatchBrowserSlashCommand("/refresh") + assert.equal(outcome.kind, "local") + assert.equal((outcome as any).action, "refresh_workspace") + }) - if (outcome.kind === "local") { - assert.equal(outcome.action, scenario.expectedAction) - } - }) - } + await t.test("/clear dispatches to local clear_terminal", () => { + const outcome = dispatchBrowserSlashCommand("/clear") + assert.equal(outcome.kind, "local") + assert.equal((outcome as any).action, "clear_terminal") + }) }) test("registered GSD command roots stay on the prompt/extension path", async () => { @@ -143,8 +147,11 @@ test("registered GSD command roots stay on the prompt/extension path", async () "browser parity contract only expects the current GSD command roots", ) - // Non-gsd roots are extension commands that pass through to the bridge - for (const root of registeredRoots.filter((r) => r !== "gsd")) { + // Non-gsd roots are extension commands that pass through to the bridge. + // Derived dynamically so adding a new registration fails this assertion loudly. + const nonGsdRoots = registeredRoots.filter((r) => r !== "gsd") + assert.equal(nonGsdRoots.length, 4, "expected exactly 4 non-gsd passthrough roots; update this count when adding registrations") + for (const root of nonGsdRoots) { assertPromptPassthrough(`/${root}`) } @@ -234,39 +241,35 @@ test("every registered /gsd subcommand has an explicit browser dispatch outcome" expectedKind, `/gsd ${subcommand} should dispatch to ${expectedKind}, got ${outcome.kind}`, ) - - if (expectedKind === "surface") { - assert.equal( - outcome.surface, - `gsd-${subcommand}`, - `/gsd ${subcommand} should open the gsd-${subcommand} surface`, - ) - } - - if (expectedKind === "prompt") { - assert.equal( - outcome.command.message, - `/gsd ${subcommand}`, - `/gsd ${subcommand} should preserve exact input text for bridge delivery`, - ) - } - - if (expectedKind === "local") { - assert.equal( - outcome.action, - "gsd_help", - `/gsd ${subcommand} should dispatch to gsd_help action`, - ) - } - - if (expectedKind === "view-navigate") { - assert.equal( - outcome.view, - subcommand, - `/gsd ${subcommand} should navigate to the ${subcommand} view`, - ) - } }) + + if (expectedKind === "surface") { + await t.test(`/gsd ${subcommand} opens gsd-${subcommand} surface`, () => { + const outcome = dispatchBrowserSlashCommand(`/gsd ${subcommand}`) as any + assert.equal(outcome.surface, `gsd-${subcommand}`, `/gsd ${subcommand} should open the gsd-${subcommand} surface`) + }) + } + + if (expectedKind === "prompt") { + await t.test(`/gsd ${subcommand} preserves exact input text`, () => { + const outcome = dispatchBrowserSlashCommand(`/gsd ${subcommand}`) as any + assert.equal(outcome.command.message, `/gsd ${subcommand}`, `/gsd ${subcommand} should preserve exact input text for bridge delivery`) + }) + } + + if (expectedKind === "local") { + await t.test(`/gsd ${subcommand} dispatches to gsd_help action`, () => { + const outcome = dispatchBrowserSlashCommand(`/gsd ${subcommand}`) as any + assert.equal(outcome.action, "gsd_help", `/gsd ${subcommand} should dispatch to gsd_help action`) + }) + } + + if (expectedKind === "view-navigate") { + await t.test(`/gsd ${subcommand} navigates to the ${subcommand} view`, () => { + const outcome = dispatchBrowserSlashCommand(`/gsd ${subcommand}`) as any + assert.equal(outcome.view, subcommand, `/gsd ${subcommand} should navigate to the ${subcommand} view`) + }) + } } })