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 <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
@@ -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<string, unknown>) => ({ 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);
|
||||
});
|
||||
});
|
||||
@@ -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: <branch> 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
|
||||
|
||||
Reference in New Issue
Block a user