From 02df5d1c629a5d033b5d40785926fa0f72e9072f Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 9 Jun 2026 02:30:30 -0700 Subject: [PATCH] FN-6069: restore chat manager test to dashboard quality gate Restore the chat manager API suite to the dashboard curated quality gate. - move `chat-manager.test.ts` back into the curated dashboard API quality project - remove the temporary chat-manager quarantine and broad vitest excludes that kept it out of coverage - stabilize chat-manager timing/subscriber tests with fake timers and explicit unsubscribe cleanup - align the agent import route mock so the rescued suite uses the expected CLI settings stub Files changed: $(git diff --cached --stat) Fusion-Task-Id: FN-6069 Fusion-Task-Lineage: e70e7817-9fe6-4e80-8b53-8f99c6214941 --- .../src/__tests__/chat-manager.test.ts | 89 +++++++++++-------- .../src/__tests__/routes-agent-import.test.ts | 2 + packages/dashboard/vitest.config.ts | 4 +- scripts/lib/test-quarantine.json | 8 +- 4 files changed, 55 insertions(+), 48 deletions(-) diff --git a/packages/dashboard/src/__tests__/chat-manager.test.ts b/packages/dashboard/src/__tests__/chat-manager.test.ts index d27d7c6bc4..4f375ed72b 100644 --- a/packages/dashboard/src/__tests__/chat-manager.test.ts +++ b/packages/dashboard/src/__tests__/chat-manager.test.ts @@ -1418,21 +1418,24 @@ describe("ChatManager.sendMessage", () => { const chatManager = createChatManager(); - await chatManager.sendMessage("chat-001", "This is a long message that needs to be summarized"); + vi.useFakeTimers(); + try { + await chatManager.sendMessage("chat-001", "This is a long message that needs to be summarized"); + await vi.advanceTimersByTimeAsync(100); - // Wait for the async title generation - await new Promise((resolve) => setTimeout(resolve, 100)); + // Assert - summarizeTitle was called with the message content and model params + expect(mockSummarizeTitle).toHaveBeenCalledWith( + "This is a long message that needs to be summarized", + "/tmp/test", + undefined, + undefined, + ); - // Assert - summarizeTitle was called with the message content and model params - expect(mockSummarizeTitle).toHaveBeenCalledWith( - "This is a long message that needs to be summarized", - "/tmp/test", - undefined, - undefined, - ); - - // Assert - session was updated with the generated title - expect(mockChatStore.updateSession).toHaveBeenCalledWith("chat-001", { title: "Short Title" }); + // Assert - session was updated with the generated title + expect(mockChatStore.updateSession).toHaveBeenCalledWith("chat-001", { title: "Short Title" }); + } finally { + vi.useRealTimers(); + } }); it("uses truncated content when summarizeTitle returns null", async () => { @@ -1453,16 +1456,19 @@ describe("ChatManager.sendMessage", () => { const chatManager = createChatManager(); const longMessage = "A".repeat(300); - await chatManager.sendMessage("chat-001", longMessage); + vi.useFakeTimers(); + try { + await chatManager.sendMessage("chat-001", longMessage); + await vi.advanceTimersByTimeAsync(100); - // Wait for the async title generation - await new Promise((resolve) => setTimeout(resolve, 100)); + // Assert - summarizeTitle was called + expect(mockSummarizeTitle).toHaveBeenCalled(); - // Assert - summarizeTitle was called - expect(mockSummarizeTitle).toHaveBeenCalled(); - - // Assert - session was updated with truncated content (first 60 chars) - expect(mockChatStore.updateSession).toHaveBeenCalledWith("chat-001", { title: "A".repeat(60) }); + // Assert - session was updated with truncated content (first 60 chars) + expect(mockChatStore.updateSession).toHaveBeenCalledWith("chat-001", { title: "A".repeat(60) }); + } finally { + vi.useRealTimers(); + } }); it("does not generate title when session already has a title", async () => { @@ -1487,15 +1493,18 @@ describe("ChatManager.sendMessage", () => { const chatManager = createChatManager(); - await chatManager.sendMessage("chat-001", "This is a long message"); + vi.useFakeTimers(); + try { + await chatManager.sendMessage("chat-001", "This is a long message"); + await vi.advanceTimersByTimeAsync(100); - // Wait for potential async operations - await new Promise((resolve) => setTimeout(resolve, 100)); - - // Assert - summarizeTitle was NOT called - expect(mockSummarizeTitle).not.toHaveBeenCalled(); - // Assert - updateSession was NOT called - expect(mockChatStore.updateSession).not.toHaveBeenCalled(); + // Assert - summarizeTitle was NOT called + expect(mockSummarizeTitle).not.toHaveBeenCalled(); + // Assert - updateSession was NOT called + expect(mockChatStore.updateSession).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } }); it("cancelGeneration returns false when no active generation exists", () => { @@ -1662,17 +1671,21 @@ describe("ChatManager diagnostics", () => { const throwingCallback = vi.fn(() => { throw new Error("Broadcast callback failed"); }); - chatStreamManager.subscribe("chat-001", throwingCallback); + const unsubscribe = chatStreamManager.subscribe("chat-001", throwingCallback); - expect(() => - chatStreamManager.broadcast("chat-001", { type: "thinking", data: "test" }) - ).not.toThrow(); + try { + expect(() => + chatStreamManager.broadcast("chat-001", { type: "thinking", data: "test" }) + ).not.toThrow(); - expect(throwingCallback).toHaveBeenCalledTimes(1); - expect(loggedErrors).toContainEqual({ - message: "Error broadcasting to client for session chat-001:", - args: [expect.any(Error)], - }); + expect(throwingCallback).toHaveBeenCalledTimes(1); + expect(loggedErrors).toContainEqual({ + message: "Error broadcasting to client for session chat-001:", + args: [expect.any(Error)], + }); + } finally { + unsubscribe(); + } }); it("logs error diagnostic when sendMessage encounters AI processing failure", async () => { diff --git a/packages/dashboard/src/__tests__/routes-agent-import.test.ts b/packages/dashboard/src/__tests__/routes-agent-import.test.ts index a9787ee2c9..9066abd6ec 100644 --- a/packages/dashboard/src/__tests__/routes-agent-import.test.ts +++ b/packages/dashboard/src/__tests__/routes-agent-import.test.ts @@ -74,6 +74,8 @@ vi.mock("@fusion/core", () => { CLI_AGENT_ADAPTER_IDS: ["claude-code", "codex", "droid", "pi", "generic"], sanitizeCliAgentSettings: (value: unknown) => value, AgentCompaniesParseError: MockAgentCompaniesParseError, + CLI_AGENT_ADAPTER_IDS: ["claude-code", "codex", "droid", "pi", "generic"], + sanitizeCliAgentSettings: () => undefined, isEphemeralAgent: (agent: { metadata?: Record }) => agent?.metadata?.agentKind === "task-worker", deterministicGuardLocks: new Map(), diff --git a/packages/dashboard/vitest.config.ts b/packages/dashboard/vitest.config.ts index 3fa2135cd7..7e07e6eae4 100644 --- a/packages/dashboard/vitest.config.ts +++ b/packages/dashboard/vitest.config.ts @@ -234,7 +234,7 @@ const qualityAppSettingsOnlyTests = ["app/components/__tests__/SettingsModal.tes const qualityApiTests = [ // Critical HTTP/server behavior: auth, task/project/settings mutation, // git/GitHub, agents, nodes, chat/files, realtime, and isolation guards. - "src/__tests__/{api-error,auth-middleware,auth-middleware-integration,chat-attachment-routes,chat-routes,file-service,github,github-webhooks,initialize,planning-flow-diagnostics-guardrail,pr-routes-auto-merge,pr-routes.contract,project-routes,project-store-resolver,register-git-github.pr-options-preflight-metadata,register-git-github.pr-resolve-conflicts,remote-access-routes,remote-auth,routes-agent-budget,routes-agent-keys,routes-agent-permissions,routes-agent-ratings,routes-agent-runs,routes-agent-soul-memory,routes-agents,routes-automation,routes-branch-groups,routes-git,routes-github,routes-merge-advance-push-origin,routes-nodes,routes-nodes-sync-contract,routes-planning,routes-secrets-sync,routes-settings,routes-task-commit-associations,routes-tasks,routes-tasks-deterministic-dedup,routes-tasks-duplicate-check,routes-tasks-explicit-duplicate-marker,server,server-static-assets,server-webhook,server.events,setup-routes,sse,sse-buffer,test-isolation-guard,update-check-route,websocket,recover-branch-binding-route}.test.ts", + "src/__tests__/{api-error,auth-middleware,auth-middleware-integration,chat-attachment-routes,chat-manager,chat-routes,file-service,github,github-webhooks,initialize,planning-flow-diagnostics-guardrail,pr-routes-auto-merge,pr-routes.contract,project-routes,project-store-resolver,register-git-github.pr-options-preflight-metadata,register-git-github.pr-resolve-conflicts,remote-access-routes,remote-auth,routes-agent-budget,routes-agent-keys,routes-agent-permissions,routes-agent-ratings,routes-agent-runs,routes-agent-soul-memory,routes-agents,routes-automation,routes-branch-groups,routes-git,routes-github,routes-merge-advance-push-origin,routes-nodes,routes-nodes-sync-contract,routes-planning,routes-secrets-sync,routes-settings,routes-task-commit-associations,routes-tasks,routes-tasks-deterministic-dedup,routes-tasks-duplicate-check,routes-tasks-explicit-duplicate-marker,server,server-static-assets,server-webhook,server.events,setup-routes,sse,sse-buffer,test-isolation-guard,update-check-route,websocket,recover-branch-binding-route}.test.ts", "src/__tests__/dashboard-test-config-guard.test.ts", "src/routes/__tests__/{custom-provider-routes,custom-providers,register-docker-node-routes,register-diagnostics-routes,stash-recovery-routes}.test.ts", "scripts/__tests__/run-vitest-with-heap.test.ts", @@ -257,7 +257,6 @@ const qualityAppBackfillTests = ["app/**/*.test.{ts,tsx}"]; const backfillApiExclude = [ ...qualityApiTests, ...skipListDashboardGlobs.filter((file) => file.startsWith("src/")), - "src/__tests__/chat-manager.test.ts", ]; const qualityApiBackfillTests = ["src/**/*.test.{ts,tsx}"]; @@ -449,7 +448,6 @@ export default defineConfig({ name: "dashboard-api", environment: "node", include: ["src/**/*.test.{ts,tsx}"], - exclude: ["src/__tests__/chat-manager.test.ts"], css: { include: [] }, }, }, diff --git a/scripts/lib/test-quarantine.json b/scripts/lib/test-quarantine.json index 29c0fb675e..39eac9c428 100644 --- a/scripts/lib/test-quarantine.json +++ b/scripts/lib/test-quarantine.json @@ -1,10 +1,4 @@ { "$comment": "Flaky-test quarantine ledger (deletion ratchet — see AGENTS.md 'Flaky tests: quarantine on sight' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). A test observed failing without a corresponding real bug is quarantined ON SIGHT: add an entry here AND a matching one-line `exclude` entry in that package's vitest config, in the same commit. Every entry needs a non-empty `reason` (link the failing run) and a `quarantinedAt` date — the entry expires 14 days later, at which point the test file is DELETED unless someone rescues it with evidence it catches real regressions plus a root-cause fix (never appeasement). There is deliberately no loader module and no automation around this file: it is a dated record, the vitest config exclude is the mechanism, and the sweep is policy executed by whoever touches the suite.", - "entries": [ - { - "file": "packages/dashboard/src/__tests__/chat-manager.test.ts", - "reason": "Flaky: 'old generation finally does not delete a newer generation's slot' times out under full-suite load. Prior fix attempts (FN-4012 on 2026-05-11, FN-5982 on 2026-06-06 which exhausted its stuck-kill budget 10/6 and was manually paused twice) failed to address the underlying generation-slot race. Per standing rule: quarantine on sight, not appease. Rescue requires (a) evidence the test catches real regressions and (b) a root-cause fix for the generation-slot race — not another synchronization tweak.", - "quarantinedAt": "2026-06-08" - } - ] + "entries": [] }