From f8ff0e6f5c579233759849554bfd9f2ca3c0b673 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 2 Jul 2026 07:41:15 -0700 Subject: [PATCH] test: fix remaining CI full-suite failures after graph cutover Missing mock export (5 files, 17 tests): - reviewer-prompt-single-source, plan-review-unavailable-recovery, triage-fast-mode-workflow-variant, triage-stuck-requeue-preserve-draft, restart.integration: add formatModelMarkerDetails to the ../pi.js mock. Production code calls this after resolving an agent session, but the test mocks were missing the export, causing all reviewer/triage/restart paths to throw before reaching finalization assertions. Outdated worktree assertions (2 files, 5 tests): - worktree-acquisition-backend, worktree-acquisition-worktrunk: update assertions for the new git symbolic-ref origin/HEAD resolution call and the appended start point in git worktree add (worktree isolation fix). Stale workflow compiler tests (1 file, 2 tests): - workflow-routes: FN-7360 removed the linear compiler /compile endpoint and made parseWorkflowIr the sole validity gate. Branching custom workflows are now valid on the graph interpreter. Updated the two stale tests that asserted 422 for branching IR to assert 201/200 instead. --- .../src/__tests__/workflow-routes.test.ts | 48 ++++++++++++++----- .../plan-review-unavailable-recovery.test.ts | 1 + .../src/__tests__/restart.integration.test.ts | 6 +++ .../reviewer-prompt-single-source.test.ts | 1 + .../triage-fast-mode-workflow-variant.test.ts | 6 +++ ...riage-stuck-requeue-preserve-draft.test.ts | 7 +++ .../worktree-acquisition-backend.test.ts | 31 ++++++++++-- .../worktree-acquisition-worktrunk.test.ts | 20 ++++++-- 8 files changed, 101 insertions(+), 19 deletions(-) diff --git a/packages/dashboard/src/__tests__/workflow-routes.test.ts b/packages/dashboard/src/__tests__/workflow-routes.test.ts index 797cd4526a..790697ef8d 100644 --- a/packages/dashboard/src/__tests__/workflow-routes.test.ts +++ b/packages/dashboard/src/__tests__/workflow-routes.test.ts @@ -314,23 +314,34 @@ describe("workflow routes (U4)", () => { expect(complete?.flags.complete).toBe(true); }); - it("POST /workflows/:id/compile returns steps for linear and 422 for branching", async () => { + /* + FNXC:CustomWorkflows 2026-07-02-07:40: + FN-7360 removed the linear WorkflowStep compiler and POST /api/workflows/:id/compile. + parseWorkflowIr (which accepts branching graphs) is now the sole validity gate, so + branching custom workflows are persistable and run on the graph interpreter instead + of being rejected as interpreter-only. This test locks the inverted invariant: a + branching IR is accepted at create (201), matching how the equivalent core + selection test was reframed. (The removed /compile endpoint is not exercised here + because this in-process express harness does not finalize unmatched-route + responses.) + */ + it("POST /workflows accepts branching IR (FN-7360 removed the linear compiler /compile gate)", async () => { + // Linear IR still creates successfully. const linear = await post("/api/workflows", { name: "L", ir: linearIr() }); - const linearId = (linear.body as { id: string }).id; - const okCompile = await post(`/api/workflows/${linearId}/compile`, {}); - expect(okCompile.status).toBe(200); - expect((okCompile.body as { steps: unknown[] }).steps).toHaveLength(1); + expect(linear.status).toBe(201); + // Branching IR used to be rejected as interpreter-only; it now persists (201) + // because the graph interpreter runs branching graphs directly. const branchy = await post("/api/workflows", { name: "B", ir: branchingIr() }); - const branchyId = (branchy.body as { id: string }).id; - const badCompile = await post(`/api/workflows/${branchyId}/compile`, {}); - expect(badCompile.status).toBe(422); - expect((badCompile.body as { error: string }).error).toMatch(/interpreter \(deferred\)/i); + expect(branchy.status).toBe(201); + expect((branchy.body as { id: string }).id).toMatch(/^WF-\d{3}$/); }); /* FNXC:CustomWorkflows 2026-06-18-11:03: - FN-6645 locks the per-task workflow selection route contract: missing or non-string workflowId returns 400, unknown ids return 404, uncompilable custom IR returns 422, explicit null clears, and workflow:updated SSE is emitted only after successful select or clear mutations. + FN-6645 locks the per-task workflow selection route contract: missing or non-string workflowId returns 400, unknown ids return 404, explicit null clears, and workflow:updated SSE is emitted only after successful select or clear mutations. + FNXC:CustomWorkflows 2026-07-02-07:40: + FN-7360 removed the linear compiler, so the "uncompilable custom IR returns 422" clause no longer applies — branching IR is now valid and selectable (see the FN-7360 branching-selection test below). The remaining 400/404/null-clear/SSE clauses still hold. */ it("PUT /tasks/:taskId/workflow selects and reflects on the task", async () => { const wf = await post("/api/workflows", { name: "QA", ir: linearIr() }); @@ -387,15 +398,26 @@ describe("workflow routes (U4)", () => { }, ); - it("PUT /tasks/:taskId/workflow maps uncompilable custom workflow IR to 422 without SSE", async () => { + /* + FNXC:CustomWorkflows 2026-07-02-07:40: + FN-7360 removed the linear WorkflowStep compiler, so branching custom IR is no + longer "uncompilable." parseWorkflowIr is the sole validity gate and the graph + interpreter runs branching graphs directly, so selecting a branching workflow now + succeeds (200) and emits the workflow:updated SSE exactly like a linear selection. + The previous 422-without-SSE contract is inverted. + */ + it("PUT /tasks/:taskId/workflow selects a branching workflow on the graph interpreter (FN-7360)", async () => { const branchy = await post("/api/workflows", { name: "Branching", ir: branchingIr() }); const branchyId = (branchy.body as { id: string }).id; const task = await store.createTask({ description: "T", enabledWorkflowSteps: [] }); vi.mocked(emitWorkflowSseEvent).mockClear(); const res = await put(`/api/tasks/${task.id}/workflow`, { workflowId: branchyId }); - expect(res.status).toBe(422); - expect(emitWorkflowSseEvent).not.toHaveBeenCalled(); + expect(res.status).toBe(200); + expect((res.body as { workflowId: string }).workflowId).toBe(branchyId); + // Successful selection emits workflow:updated exactly once (the previous + // rejection path asserted SSE was NOT emitted; that contract is inverted). + expect(emitWorkflowSseEvent).toHaveBeenCalledTimes(1); }); it("PUT /tasks/:taskId/workflow emits workflow:updated exactly once after select and clear", async () => { diff --git a/packages/engine/src/__tests__/plan-review-unavailable-recovery.test.ts b/packages/engine/src/__tests__/plan-review-unavailable-recovery.test.ts index 07674b3a37..5122b159f4 100644 --- a/packages/engine/src/__tests__/plan-review-unavailable-recovery.test.ts +++ b/packages/engine/src/__tests__/plan-review-unavailable-recovery.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; vi.mock("../pi.js", () => ({ describeModel: vi.fn().mockReturnValue("mock-provider/mock-model"), + formatModelMarkerDetails: vi.fn((model: string) => model), promptWithFallback: vi.fn(async (session: any, prompt: string, options?: any) => { if (options == null) await session.prompt(prompt); else await session.prompt(prompt, options); diff --git a/packages/engine/src/__tests__/restart.integration.test.ts b/packages/engine/src/__tests__/restart.integration.test.ts index 5519a8c917..3bc7b31f15 100644 --- a/packages/engine/src/__tests__/restart.integration.test.ts +++ b/packages/engine/src/__tests__/restart.integration.test.ts @@ -37,6 +37,12 @@ vi.mock("../pi.js", () => ({ await session.prompt(prompt, options); } }), + // FNXC:EngineTests 2026-07-02-07:40: + // Both triage.ts and executor.ts now call formatModelMarkerDetails (from + // pi.js) to build the model-marker log line after resolving an agent + // session. The mock must expose the export so the resume/triage paths can + // reach finalization instead of throwing on a missing mock member. + formatModelMarkerDetails: vi.fn((model: string) => model), })); vi.mock("../reviewer.js", () => ({ reviewStep: vi.fn(), diff --git a/packages/engine/src/__tests__/reviewer-prompt-single-source.test.ts b/packages/engine/src/__tests__/reviewer-prompt-single-source.test.ts index 64a448cc1b..4c96080504 100644 --- a/packages/engine/src/__tests__/reviewer-prompt-single-source.test.ts +++ b/packages/engine/src/__tests__/reviewer-prompt-single-source.test.ts @@ -12,6 +12,7 @@ import { vi.mock("../pi.js", () => ({ createFnAgent: vi.fn(), describeModel: vi.fn().mockReturnValue("mock-provider/mock-model"), + formatModelMarkerDetails: vi.fn((model: string) => model), promptWithFallback: vi.fn(async (session, prompt, options) => { if (options === undefined) { await session.prompt(prompt); diff --git a/packages/engine/src/__tests__/triage-fast-mode-workflow-variant.test.ts b/packages/engine/src/__tests__/triage-fast-mode-workflow-variant.test.ts index 67ebea9a3a..63310b3e88 100644 --- a/packages/engine/src/__tests__/triage-fast-mode-workflow-variant.test.ts +++ b/packages/engine/src/__tests__/triage-fast-mode-workflow-variant.test.ts @@ -25,6 +25,12 @@ vi.mock("../pi.js", () => ({ createFnAgent: mockCreateFnAgent, describeModel: vi.fn().mockReturnValue("mock-model"), promptWithFallback: mockPromptWithFallback, + // FNXC:TriageTests 2026-07-02-07:40: + // triage.ts specifyTask now calls formatModelMarkerDetails (from pi.js) to + // build the model-marker log line after the agent session resolves. The mock + // must expose the export so the planning path can reach finalization + // (moveTask todo) instead of throwing on a missing mock member. + formatModelMarkerDetails: vi.fn((model: string) => model), })); vi.mock("@fusion/core", async (importOriginal) => { diff --git a/packages/engine/src/__tests__/triage-stuck-requeue-preserve-draft.test.ts b/packages/engine/src/__tests__/triage-stuck-requeue-preserve-draft.test.ts index 1df37d1901..01dd324758 100644 --- a/packages/engine/src/__tests__/triage-stuck-requeue-preserve-draft.test.ts +++ b/packages/engine/src/__tests__/triage-stuck-requeue-preserve-draft.test.ts @@ -20,6 +20,13 @@ vi.mock("../agent-session-helpers.js", () => ({ vi.mock("../pi.js", () => ({ describeModel: vi.fn().mockReturnValue("mock-model"), promptWithFallback: mockPromptWithFallback, + // FNXC:TriageTests 2026-07-02-07:40: + // triage.ts specifyTask now calls formatModelMarkerDetails (from pi.js) to + // build the model-marker log line after the agent session resolves. The mock + // must expose the export so the stuck-requeue planning path can reach + // finalization (moveTask todo / needs-replan) instead of throwing on a + // missing mock member. + formatModelMarkerDetails: vi.fn((model: string) => model), })); function createTask(overrides: Partial = {}): Task { diff --git a/packages/engine/src/__tests__/worktree-acquisition-backend.test.ts b/packages/engine/src/__tests__/worktree-acquisition-backend.test.ts index 2c1d7f49be..8f326e270d 100644 --- a/packages/engine/src/__tests__/worktree-acquisition-backend.test.ts +++ b/packages/engine/src/__tests__/worktree-acquisition-backend.test.ts @@ -64,8 +64,16 @@ describe("acquireTaskWorktree backend wiring", () => { expect(result.branch).toBe("fusion/fn-1"); expect(result.worktreePath).toBe("/repo/.worktrees/fn-1"); + /* + * FNXC:WorktreeIsolation 2026-07-02-07:40: + * acquireTaskWorktree now resolves the integration branch via `git symbolic-ref` and pins fresh worktree creation to that start point so new task branches never inherit the root checkout's ambient HEAD. With an empty mock stdout the resolver falls back to "main", so the native create command appends "main" as the start point and there are two exec calls (symbolic-ref + worktree add). + */ expect(execMock).toHaveBeenCalledWith( - 'git worktree add -b "fusion/fn-1" "/repo/.worktrees/fn-1"', + "git symbolic-ref --short refs/remotes/origin/HEAD", + expect.objectContaining({ cwd: "/repo" }), + ); + expect(execMock).toHaveBeenCalledWith( + 'git worktree add -b "fusion/fn-1" "/repo/.worktrees/fn-1" "main"', expect.objectContaining({ cwd: "/repo" }), ); expect(audit.git).not.toHaveBeenCalledWith( @@ -125,7 +133,16 @@ describe("acquireTaskWorktree backend wiring", () => { }), ).rejects.toMatchObject({ name: "WorktrunkOperationError", code: "worktrunk_binary_missing" }); - expect(execMock).not.toHaveBeenCalled(); + /* + * FNXC:WorktreeIsolation 2026-07-02-07:40: + * The integration-branch resolution (`git symbolic-ref`) runs before the worktrunk binary check, so one exec call is expected. No worktrunk `switch` command should be attempted when the binary is missing. + */ + expect(execMock).toHaveBeenCalledTimes(1); + expect(execMock).toHaveBeenCalledWith( + "git symbolic-ref --short refs/remotes/origin/HEAD", + expect.objectContaining({ cwd: "/repo" }), + ); + expect(execMock.mock.calls.some((call) => String(call[0]).includes('"switch"'))).toBe(false); }); it("throws worktrunk_operation_failed and preserves stderr", async () => { @@ -170,6 +187,14 @@ describe("acquireTaskWorktree backend wiring", () => { expect(result.worktreePath).toBe("/tmp/backend"); expect(result.branch).toBe("fusion/fn-backend"); expect(create).toHaveBeenCalledTimes(1); - expect(execMock).not.toHaveBeenCalled(); + /* + * FNXC:WorktreeIsolation 2026-07-02-07:40: + * The integration-branch resolution runs before the explicit backend's create is invoked, so the only exec call is the `git symbolic-ref` lookup. The custom backend's create mock performs no exec. + */ + expect(execMock).toHaveBeenCalledTimes(1); + expect(execMock).toHaveBeenCalledWith( + "git symbolic-ref --short refs/remotes/origin/HEAD", + expect.objectContaining({ cwd: "/repo" }), + ); }); }); diff --git a/packages/engine/src/__tests__/worktree-acquisition-worktrunk.test.ts b/packages/engine/src/__tests__/worktree-acquisition-worktrunk.test.ts index c17d8f3938..f194aa7692 100644 --- a/packages/engine/src/__tests__/worktree-acquisition-worktrunk.test.ts +++ b/packages/engine/src/__tests__/worktree-acquisition-worktrunk.test.ts @@ -65,8 +65,14 @@ describe("acquireTaskWorktree worktrunk wiring", () => { }); expect(result).toMatchObject({ source: "fresh", branch: "fusion/fn-1" }); - expect(execMock).toHaveBeenCalledTimes(1); - expect(execMock.mock.calls[0]?.[0]).toContain("git worktree add -b"); + /* + * FNXC:WorktreeIsolation 2026-07-02-07:40: + * acquireTaskWorktree now resolves the integration branch via `git symbolic-ref` (returning empty here, so it falls back to "main") and pins the fresh worktree to that start point. Two exec calls happen: the symbolic-ref lookup, then the native `git worktree add -b ... "main"` create. + */ + expect(execMock).toHaveBeenCalledTimes(2); + expect(execMock.mock.calls[0]?.[0]).toBe("git symbolic-ref --short refs/remotes/origin/HEAD"); + expect(execMock.mock.calls[1]?.[0]).toContain('git worktree add -b "fusion/fn-1"'); + expect(execMock.mock.calls[1]?.[0]).toContain('"main"'); }); it("prefers explicit createWorktree override", async () => { @@ -223,6 +229,14 @@ describe("acquireTaskWorktree worktrunk wiring", () => { expect(result.branch).toBe("fusion/fn-1-custom"); expect(create).toHaveBeenCalledTimes(1); - expect(execMock).not.toHaveBeenCalled(); + /* + * FNXC:WorktreeIsolation 2026-07-02-07:40: + * The integration-branch resolution runs before the custom backend's create, so the only exec call is the `git symbolic-ref` lookup. The backend's create mock performs no exec. + */ + expect(execMock).toHaveBeenCalledTimes(1); + expect(execMock).toHaveBeenCalledWith( + "git symbolic-ref --short refs/remotes/origin/HEAD", + expect.objectContaining({ cwd: "/repo" }), + ); }); });