From 2b228383d5f00f8affbb16901177eb70d9cc84ea Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 11 Aug 2026 02:47:24 -0700 Subject: [PATCH] test: cover model-target chat creation and non-conflict cleanup failures Both suites are mutation-verified: reverting the corresponding fix fails 4 of the chat tests and exactly the 3 new self-healing park tests. The self-healing tests carry a positive control asserting the sweep actually reached the tip-already-merged arm -- without it the park assertions passed vacuously against a task shape the candidate filter rejected. Co-Authored-By: Claude Opus 5 --- ...and-worktree-conflict-misclassification.md | 7 + .../routes-chat-sessions-model-target.test.ts | 160 ++++++++++++++++++ ...self-healing-ghost-branch-recovery.test.ts | 87 ++++++++++ 3 files changed, 254 insertions(+) create mode 100644 .changeset/fix-model-chat-404-and-worktree-conflict-misclassification.md create mode 100644 packages/dashboard/src/__tests__/routes-chat-sessions-model-target.test.ts diff --git a/.changeset/fix-model-chat-404-and-worktree-conflict-misclassification.md b/.changeset/fix-model-chat-404-and-worktree-conflict-misclassification.md new file mode 100644 index 0000000000..ab2ad9068a --- /dev/null +++ b/.changeset/fix-model-chat-404-and-worktree-conflict-misclassification.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix "Failed to create chat session" on model chats, and tasks wrongly failed as branch conflicts. +category: fix +dev: Chat — FN-8869 hoisted the agent-existence check out of its `else` branch in `register-chat-routes.ts`, so model-target chats sending the agent-less `__fn_agent__` sentinel 404'd; the agent is now required only when it is the source of model resolution. Self-healing — a failed `tip-already-merged` cleanup was rethrown and classified `branch-conflict-unrecoverable`, failing and pausing tasks whose branch was already an ancestor of the integration ref (every observed case was a `git worktree remove --force` / `ENOTEMPTY rmdir node_modules` pnpm race). Cleanup failure now retries on the next sweep, and `git worktree prune` runs before removal so stale registrations stop causing the failure they would have prevented. diff --git a/packages/dashboard/src/__tests__/routes-chat-sessions-model-target.test.ts b/packages/dashboard/src/__tests__/routes-chat-sessions-model-target.test.ts new file mode 100644 index 0000000000..c15b74bb6a --- /dev/null +++ b/packages/dashboard/src/__tests__/routes-chat-sessions-model-target.test.ts @@ -0,0 +1,160 @@ +// @vitest-environment node + +import express from "express"; +import multer from "multer"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { AgentStore } from "@fusion/core"; +import { request } from "../test-request.js"; +import { registerChatRoutes } from "../routes/register-chat-routes.js"; + +function postJson(app: express.Express, body: unknown) { + return request(app, "POST", "/api/chat/sessions", JSON.stringify(body), { + "content-type": "application/json", + }); +} + +/** The client-only sentinel from `app/hooks/useChat.ts` marking a model-target chat. + * It is intentionally never persisted as an agent row. */ +const FN_AGENT_ID = "__fn_agent__"; + +function buildApp(getAgent: (id: string) => unknown) { + const createSession = vi.fn(async (input: Record) => ({ id: "session-1", ...input })); + vi.spyOn(AgentStore.prototype, "init").mockResolvedValue(undefined); + vi.spyOn(AgentStore.prototype, "getAgent").mockImplementation(async (id) => getAgent(id) as never); + + const scopedStore = { + getFusionDir: () => "/route-project/.fusion", + getAsyncLayer: () => undefined, + getSettings: async () => ({ + defaultProvider: "global-provider", + defaultModelId: "global-model", + defaultThinkingLevelOverride: "high", + }), + }; + const app = express(); + app.use(express.json()); + const router = express.Router(); + registerChatRoutes({ + router, + store: scopedStore, + options: { chatStore: { createSession } }, + getProjectContext: async () => ({ store: scopedStore, projectId: "project-1", engine: undefined }), + rethrowAsApiError: (error: unknown) => { throw error; }, + } as never, { + parseLastEventId: () => undefined, + replayBufferedSSE: () => false, + validateOptionalModelField: () => undefined, + upload: multer(), + }); + app.use("/api", router); + // The route rethrows ApiError; without a boundary the error paths would hang + // rather than assert a status. + app.use((err: { statusCode?: number; message?: string }, _req: express.Request, res: express.Response, _next: express.NextFunction) => { + res.status(err?.statusCode ?? 500).json({ error: err?.message ?? "unknown" }); + }); + return { app, createSession }; +} + +/* +FNXC:ChatSessionCreate 2026-08-11-09:38: +Symptom Verification for the model-target chat regression. + +Original symptom: every "new chat" against a MODEL (rather than an agent) failed with the toast +"Failed to create chat session". FN-8869 hoisted the agent-existence check out of its `else` branch so it +ran unconditionally, and model-target chats send the agent-less sentinel `__fn_agent__` -> HTTP 404 +"Agent __fn_agent__ not found". + +Exact reproduction: POST /api/chat/sessions {agentId:"__fn_agent__", modelProvider, modelId}. +Assertion it is gone: that POST returns 201 and persists the client model pair. + +Surface enumeration -- the invariant is "a missing agent row only fails creation when the agent is the +model SOURCE", so all of these are covered below rather than the single reported id: + - the literal `__fn_agent__` sentinel (the reported repro) + - any other unknown agent id carrying a complete model pair (the route must not hardcode the sentinel) + - the negative case: unknown agent id with NO model pair still 404s (the check is narrowed, not deleted) + - a real agent's inheritance path is untouched (covered by routes-chat-sessions-project-model.test.ts) + - agent-less sessions inherit no thinking level, since there is no role to inherit from +*/ +describe("POST /api/chat/sessions model-target (agent-less) creation", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("creates a session for the __fn_agent__ sentinel when a model pair is supplied", async () => { + const { app, createSession } = buildApp(() => null); + + const res = await postJson(app, { + agentId: FN_AGENT_ID, + modelProvider: "anthropic", + modelId: "claude-opus-4-8", + }); + + expect(res.status).toBe(201); + expect(res.body.session).toMatchObject({ + agentId: FN_AGENT_ID, + modelProvider: "anthropic", + modelId: "claude-opus-4-8", + }); + expect(createSession).toHaveBeenCalledTimes(1); + }); + + it("does not hardcode the sentinel: any unresolvable agent id with a model pair still creates", async () => { + const { app } = buildApp(() => null); + + const res = await postJson(app, { + agentId: "some-other-agentless-marker", + modelProvider: "openai-codex", + modelId: "gpt-5.6", + }); + + expect(res.status).toBe(201); + expect(res.body.session).toMatchObject({ + modelProvider: "openai-codex", + modelId: "gpt-5.6", + }); + }); + + it("agent-less sessions inherit no thinking level", async () => { + const { app } = buildApp(() => null); + + const res = await postJson(app, { + agentId: FN_AGENT_ID, + modelProvider: "anthropic", + modelId: "claude-opus-4-8", + }); + + expect(res.status).toBe(201); + expect(res.body.session.thinkingLevel).toBeUndefined(); + }); + + it("honours an explicit client thinking level on an agent-less session", async () => { + const { app } = buildApp(() => null); + + const res = await postJson(app, { + agentId: FN_AGENT_ID, + modelProvider: "anthropic", + modelId: "claude-opus-4-8", + thinkingLevel: "high", + }); + + expect(res.status).toBe(201); + expect(res.body.session).toMatchObject({ thinkingLevel: "high" }); + }); + + it("still 404s an unknown agent when the agent is the model source", async () => { + const { app, createSession } = buildApp(() => null); + + const res = await postJson(app, { agentId: "genuinely-missing-agent" }); + + expect(res.status).toBe(404); + expect(createSession).not.toHaveBeenCalled(); + }); + + it("still rejects a half-supplied model pair", async () => { + const { app } = buildApp(() => null); + + const res = await postJson(app, { agentId: FN_AGENT_ID, modelProvider: "anthropic" }); + + expect(res.status).toBe(400); + }); +}); diff --git a/packages/engine/src/__tests__/self-healing-ghost-branch-recovery.test.ts b/packages/engine/src/__tests__/self-healing-ghost-branch-recovery.test.ts index 5dcae0f01b..91b78ff016 100644 --- a/packages/engine/src/__tests__/self-healing-ghost-branch-recovery.test.ts +++ b/packages/engine/src/__tests__/self-healing-ghost-branch-recovery.test.ts @@ -131,6 +131,93 @@ describe("self-healing ghost branch reclaim", () => { expect(store.logEntry).toHaveBeenCalledWith("FN-9001", expect.stringContaining("tip-already-merged cleanup failed")); }); + /* + FNXC:SelfHealingReclaim 2026-08-11-09:38: + Symptom Verification for the misclassified-cleanup-failure park. + + Original symptom: tasks were failed and paused with `pausedReason: "branch-conflict-unrecoverable"` and the error + "Task branch conflict: is not safely reclaimable (...)", where the parenthesised cause was always a + filesystem message -- `Command failed: git worktree remove --force ...` or `ENOTEMPTY: directory not empty, rmdir + '.../node_modules/.pnpm/...'` -- never a git conflict. 78 such parks in 16 days on this repo. + + Exact reproduction: a `tip-already-merged` verdict (branch tip is already an ancestor of the integration ref, so the + branch has nothing unique to lose) whose housekeeping throws. + Assertion it is gone: the sweep records no `branch-conflict-unrecoverable` park for that task. + + Surface enumeration -- the invariant is "no cleanup failure on an already-merged tip may be reported as a branch + conflict", so every observed failure shape is asserted rather than only the one that was easiest to reproduce: + - `git worktree remove --force` failing (FN-8979 / FN-8955 / FN-8932 shape) + - `ENOTEMPTY ... rmdir node_modules` from a pnpm write race (FN-8908 shape) + - `git branch -D` failing (covered by the half-corrupt-state test above, extended here to the park assertion) + - prune runs BEFORE removal, so a stale registration stops causing the failure it would have prevented + - negative control: a genuine `live-foreign` verdict still parks (see "keeps genuine live-foreign conflicts parked") + */ + describe("tip-already-merged cleanup failures are not branch conflicts", () => { + /* The cleanup body is one try block, so WHICH housekeeping step throws does not change the + classification -- only that something threw. These are the real messages observed on parked + tasks; they are induced through the `git branch -D` seam because it is the step reachable + from this suite's exec mock. */ + const CLEANUP_FAILURES: { label: string; message: string }[] = [ + { + label: "git worktree remove --force failure", + message: 'Command failed: git worktree remove --force "/repo/.worktrees/grand-crane"', + }, + { + label: "pnpm node_modules rmdir race", + message: "ENOTEMPTY: directory not empty, rmdir '/repo/.worktrees/happy-olive/node_modules/.pnpm/@asamuzakjp+generational-cache@1.0.1'", + }, + { + label: "git branch -D failure", + message: "Command failed: git branch -D", + }, + ]; + + for (const { label, message } of CLEANUP_FAILURES) { + it(`does not park the task as branch-conflict-unrecoverable on a ${label}`, async () => { + execMock.mockImplementation(async (command: string) => { + if (command.includes("git branch -D")) throw new Error(message); + return ""; + }); + mockSweepTask({ id: "FN-9001", column: "in-review", checkedOutBy: null, branch: "fusion/fn-9001", worktree: "/tmp/live", baseCommitSha: "m0", paused: true, pausedReason: "branch-conflict-unrecoverable", status: "failed" }); + vi.spyOn(branchConflicts, "inspectBranchConflict").mockResolvedValueOnce({ kind: "tip-already-merged", livePath: "/tmp/live", tipSha: "1234567890abcdef", integrationRef: "main" } as any); + + await manager.reclaimSelfOwnedBranchConflicts(); + + // Positive control: proves the sweep actually reached the tip-already-merged + // arm and its catch fired. Without this the park assertions below would pass + // vacuously whenever the task failed the candidate filter. + expect(store.logEntry).toHaveBeenCalledWith("FN-9001", expect.stringContaining("tip-already-merged cleanup failed")); + + const parks = (store.updateTask as any).mock.calls.filter( + (c: any[]) => c[1]?.pausedReason === "branch-conflict-unrecoverable", + ); + expect(parks).toHaveLength(0); + const failures = (store.updateTask as any).mock.calls.filter((c: any[]) => c[1]?.status === "failed"); + expect(failures).toHaveLength(0); + }); + } + + it("prunes stale worktree registrations before attempting removal", async () => { + const gitCommands: string[] = []; + execMock.mockImplementation(async (command: string) => { + gitCommands.push(command); + return ""; + }); + const removeSpy = vi.spyOn(worktreePool, "removeWorktree").mockResolvedValue(undefined as never); + mockSweepTask({ id: "FN-9001", column: "in-review", checkedOutBy: null, branch: "fusion/fn-9001", worktree: "/tmp/live", baseCommitSha: "m0", paused: true, pausedReason: "branch-conflict-unrecoverable", status: "failed" }); + vi.spyOn(branchConflicts, "inspectBranchConflict").mockResolvedValueOnce({ kind: "tip-already-merged", livePath: null, tipSha: "1234567890abcdef", integrationRef: "main" } as any); + + await manager.reclaimSelfOwnedBranchConflicts(); + + const pruneIndex = gitCommands.findIndex((c) => c.includes("git worktree prune")); + const deleteIndex = gitCommands.findIndex((c) => c.includes("git branch -D")); + expect(pruneIndex).toBeGreaterThanOrEqual(0); + expect(deleteIndex).toBeGreaterThanOrEqual(0); + expect(pruneIndex).toBeLessThan(deleteIndex); + expect(removeSpy).not.toHaveBeenCalled(); + }); + }); + /* FNXC:SelfHealingReclaim 2026-07-25-09:40: Regression contract for the inherited-tip invariant (FN-1406): the reclaim sweep's `tip-already-merged` arm must