diff --git a/.changeset/workflow-steps-table-drop.md b/.changeset/workflow-steps-table-drop.md new file mode 100644 index 0000000000..66e6103921 --- /dev/null +++ b/.changeset/workflow-steps-table-drop.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Retire the legacy workflow-steps store; workflow steps now run entirely graph-native. +category: internal +dev: U7c removes the last readers/writers of the legacy `workflow_steps` table and drops it via migration 131 (SCHEMA_VERSION 130→131, idempotent DROP). Removed: store CRUD (`create`/`update`/`delete`/`getWorkflowStep`), the workflow-compilation materializer (`materializeWorkflowSteps`), `migrateLegacyWorkflowSteps` + its `POST /api/workflows/migrate-legacy-steps` route and the editor's on-open migration notice, and the merger legacy post-merge execution path (worktree + prompt/script step run). Pre/post-merge steps record into `task.workflowStepResults`; `selectTaskWorkflow` now seeds `enabledWorkflowSteps` with default-on optional-group node ids only (the graph runs the workflow IR directly). `listWorkflowSteps()` returns only the in-memory plugin palette. Executor revive sources gate-ness from the recorded result status, not the table. diff --git a/packages/core/src/__tests__/architecture-schema-compat.test.ts b/packages/core/src/__tests__/architecture-schema-compat.test.ts index 36f6e10306..8a3fa0bbd3 100644 --- a/packages/core/src/__tests__/architecture-schema-compat.test.ts +++ b/packages/core/src/__tests__/architecture-schema-compat.test.ts @@ -78,6 +78,17 @@ describe("architecture schema compatibility", () => { discoveredTables.add(match[1]); } + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: TRANSIENT migration tables — created by a + // historical migration and DROPPED by a later one (e.g. `workflow_steps`, created in + // migration 16, dropped in migration 131) — never reach the final schema, so they must + // NOT be in SCHEMA_SQL or MIGRATION_ONLY_TABLE_SCHEMAS (which would resurrect them via + // ensureSchemaCompatibility). Exclude any table that has a `DROP TABLE` in db.ts. + const droppedTables = new Set(); + for (const match of source.matchAll(/DROP TABLE\s+(?:IF EXISTS\s+)?([A-Za-z_][A-Za-z0-9_]*)/g)) { + droppedTables.add(match[1]); + } + for (const dropped of droppedTables) discoveredTables.delete(dropped); + const coveredTables = new Set([ ...[...getSchemaSqlTableSchemas().keys()], ...Object.keys(MIGRATION_ONLY_TABLE_SCHEMAS), diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index 87f3962dcc..8466b50472 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -588,15 +588,26 @@ describe("built-in workflows", () => { await expect(store.deleteWorkflowDefinition("builtin:coding")).rejects.toThrow(/cannot be deleted/i); }); - it("branching built-ins can be selected without throwing", async () => { + it("branching built-ins can be selected without throwing, seeding default-on optional-group ids", async () => { + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c — `selectTaskWorkflow` no longer + // materializes legacy `workflow_steps` rows; it seeds `enabledWorkflowSteps` with the + // workflow's DEFAULT-ON optional-group node ids, exactly matching the create-time path + // (a task that SELECTS builtin:coding now enables `code-review` just like one CREATED + // with builtin:coding — previously select returned [] and silently skipped the gate). + const expectedGroups: Record = { + "builtin:coding": ["code-review"], + "builtin:marketing": [], + "builtin:stepwise-coding": ["code-review"], + }; for (const workflowId of ["builtin:coding", "builtin:marketing", "builtin:stepwise-coding"]) { const task = await store.createTask({ description: `select ${workflowId}`, enabledWorkflowSteps: [] }); + const expected = expectedGroups[workflowId]; - await expect(store.selectTaskWorkflow(task.id, workflowId)).resolves.toEqual([]); + await expect(store.selectTaskWorkflow(task.id, workflowId)).resolves.toEqual(expected); const detail = await store.getTask(task.id); - expect(detail.enabledWorkflowSteps ?? []).toEqual([]); - expect(store.getTaskWorkflowSelection(task.id)).toEqual({ workflowId, stepIds: [] }); + expect(detail.enabledWorkflowSteps ?? []).toEqual(expected); + expect(store.getTaskWorkflowSelection(task.id)).toEqual({ workflowId, stepIds: expected }); } }); diff --git a/packages/core/src/__tests__/db-migrate.test.ts b/packages/core/src/__tests__/db-migrate.test.ts index 52bff50a73..7421bb9d51 100644 --- a/packages/core/src/__tests__/db-migrate.test.ts +++ b/packages/core/src/__tests__/db-migrate.test.ts @@ -162,19 +162,11 @@ describe("migrateFromLegacy", () => { expect(row.nextId).toBe(42); expect(row.nextWorkflowStepId).toBe(3); expect(JSON.parse(row.settings).maxConcurrent).toBe(4); + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c dropped the `workflow_steps` table. + // The legacy config.json steps are still preserved verbatim in the config column for + // archival reference, but are no longer imported as table rows (workflow steps run + // graph-native; the table no longer exists in the schema). expect(JSON.parse(row.workflowSteps)).toHaveLength(1); - - const workflowRows = db.prepare("SELECT * FROM workflow_steps ORDER BY id ASC").all() as any[]; - expect(workflowRows).toHaveLength(1); - expect(workflowRows[0]).toMatchObject({ - id: "WS-001", - name: "Test", - description: "Test step", - mode: "prompt", - phase: "pre-merge", - prompt: "test", - enabled: 1, - }); }); }); @@ -850,12 +842,19 @@ describe("schema migration", () => { db.close(); }); - it("adds workflow_steps.gateMode and backfills legacy rows by mode", () => { + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c — the legacy `workflow_steps` table is + // DROPPED by migration 131. A v75 DB with seeded legacy step rows must migrate cleanly + // through the whole chain (incl. the gateMode/migrated_fragment_id column migrations and + // the migration-130 enable-id normalization) and END with the table gone. The former + // per-row gateMode-backfill assertion is obsolete: the column is on a table nothing reads + // and that the cutover removes. + it("migrates a v75 DB with legacy workflow_steps rows and drops the table at the cutover", () => { const db = new Database(fusionDir); db.exec("CREATE TABLE IF NOT EXISTS __meta (key TEXT PRIMARY KEY, value TEXT)"); db.exec(` CREATE TABLE IF NOT EXISTS workflow_steps ( id TEXT PRIMARY KEY, + templateId TEXT, name TEXT NOT NULL, description TEXT NOT NULL, mode TEXT NOT NULL DEFAULT 'prompt', @@ -873,11 +872,10 @@ describe("schema migration", () => { db.init(); - const rows = db.prepare("SELECT id, mode, gateMode FROM workflow_steps ORDER BY id ASC").all() as Array<{ id: string; mode: string; gateMode: string }>; - expect(rows).toEqual([ - { id: "WS-001", mode: "prompt", gateMode: "advisory" }, - { id: "WS-002", mode: "script", gateMode: "advisory" }, - ]); + const table = db + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'workflow_steps'") + .get(); + expect(table).toBeUndefined(); expect(db.getSchemaVersion()).toBe(SCHEMA_VERSION); db.close(); @@ -1008,6 +1006,7 @@ describe("schema migration", () => { db.exec(` CREATE TABLE IF NOT EXISTS workflow_steps ( id TEXT PRIMARY KEY, + templateId TEXT, name TEXT NOT NULL, description TEXT NOT NULL, mode TEXT NOT NULL DEFAULT 'prompt', @@ -1026,12 +1025,13 @@ describe("schema migration", () => { db.init(); - const rows = db.prepare("SELECT id, mode, enabled, gateMode FROM workflow_steps ORDER BY id ASC").all() as Array<{ id: string; mode: string; enabled: number; gateMode: string }>; - expect(rows).toEqual([ - { id: "WS-001", mode: "prompt", enabled: 1, gateMode: "advisory" }, - { id: "WS-002", mode: "script", enabled: 1, gateMode: "advisory" }, - { id: "WS-003", mode: "prompt", enabled: 0, gateMode: "advisory" }, - ]); + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c — the cutover (migration 131) drops the + // legacy table after the historical gateMode/enabled backfills run, so the per-row + // gateMode assertion is obsolete; assert the table is gone and the chain completed. + const table = db + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'workflow_steps'") + .get(); + expect(table).toBeUndefined(); expect(db.getSchemaVersion()).toBe(SCHEMA_VERSION); db.close(); @@ -1315,6 +1315,7 @@ describe("schema migration", () => { db.exec(` CREATE TABLE IF NOT EXISTS workflow_steps ( id TEXT PRIMARY KEY, + templateId TEXT, name TEXT NOT NULL, description TEXT NOT NULL, mode TEXT NOT NULL DEFAULT 'prompt', @@ -1342,31 +1343,37 @@ describe("schema migration", () => { const wfRow = db.prepare("SELECT kind FROM workflows WHERE id = 'WF-legacy'").get() as { kind: string }; expect(wfRow.kind).toBe("workflow"); - const stepColumns = db.prepare("PRAGMA table_info(workflow_steps)").all() as Array<{ name: string }>; - expect(stepColumns.map((c) => c.name)).toContain("migrated_fragment_id"); - const stepRow = db - .prepare("SELECT migrated_fragment_id FROM workflow_steps WHERE id = 'WS-legacy'") - .get() as { migrated_fragment_id: string | null }; - expect(stepRow.migrated_fragment_id).toBeNull(); + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c — migration 109 adds + // workflow_steps.migrated_fragment_id, but the cutover (migration 131) drops the whole + // table by the time init() completes, so the column is unobservable. Assert the table + // is gone (the migration chain ran clean through the cutover). + const stepTable = db + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'workflow_steps'") + .get(); + expect(stepTable).toBeUndefined(); expect(db.getSchemaVersion()).toBe(SCHEMA_VERSION); db.close(); }); - it("migration 109 is idempotent on re-init", () => { + it("migration 109 (workflows.kind) is idempotent on re-init", () => { const db = new Database(fusionDir); db.init(); expect(db.getSchemaVersion()).toBe(SCHEMA_VERSION); db.close(); - // Re-open the same on-disk DB: already at 109, the 109 block must be a no-op. + // Re-open the same on-disk DB: already at the current version, the migration blocks + // must be a no-op. (U7c: workflow_steps no longer exists on a fresh DB — the cutover + // never creates it — so only the surviving workflows.kind column is asserted.) const reopened = new Database(fusionDir); reopened.init(); expect(reopened.getSchemaVersion()).toBe(SCHEMA_VERSION); const workflowColumns = reopened.prepare("PRAGMA table_info(workflows)").all() as Array<{ name: string }>; expect(workflowColumns.filter((c) => c.name === "kind")).toHaveLength(1); - const stepColumns = reopened.prepare("PRAGMA table_info(workflow_steps)").all() as Array<{ name: string }>; - expect(stepColumns.filter((c) => c.name === "migrated_fragment_id")).toHaveLength(1); + const stepTable = reopened + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'workflow_steps'") + .get(); + expect(stepTable).toBeUndefined(); reopened.close(); }); }); diff --git a/packages/core/src/__tests__/store-create.test.ts b/packages/core/src/__tests__/store-create.test.ts index fccb27bfa0..95a4bf4d6f 100644 --- a/packages/core/src/__tests__/store-create.test.ts +++ b/packages/core/src/__tests__/store-create.test.ts @@ -927,13 +927,8 @@ describe("TaskStore", () => { }); it("applyReplicatedTaskCreate does not auto-apply default workflow steps", async () => { - const workflowStep = await store.createWorkflowStep({ - name: "Default step", - description: "auto", - enabled: true, - defaultOn: true, - }); - + // U7c: the legacy default-on step table/CRUD is gone; the invariant under test is + // purely that a replicated create never auto-seeds enabledWorkflowSteps. const payload = { replicationVersion: 1 as const, reservationId: "res-default-step", @@ -951,7 +946,6 @@ describe("TaskStore", () => { const result = await store.applyReplicatedTaskCreate(payload); expect(result.applied).toBe(true); expect(result.task.enabledWorkflowSteps).toBeUndefined(); - expect(workflowStep.defaultOn).toBe(true); }); it("applyReplicatedTaskCreate is idempotent and detects collisions", async () => { diff --git a/packages/core/src/__tests__/store-test-helpers.shared.test.ts b/packages/core/src/__tests__/store-test-helpers.shared.test.ts index 0a58d0d897..48f7aa2ffa 100644 --- a/packages/core/src/__tests__/store-test-helpers.shared.test.ts +++ b/packages/core/src/__tests__/store-test-helpers.shared.test.ts @@ -12,14 +12,12 @@ describe("createSharedTaskStoreTestHarness", () => { afterEach(harness.afterEach); afterAll(harness.afterAll); - it("resets ids so tasks and workflow steps restart from FN-001 / WS-001", async () => { + it("resets ids so tasks restart from FN-001", async () => { const task = await harness.store().createTask({ description: "first" }); - const step = await harness.store().createWorkflowStep({ name: "Step", description: "Desc" }); expect(task.id).toBe("FN-001"); - expect(step.id).toBe("WS-001"); }); - it("clears workflow steps cache between tests", async () => { + it("workflow steps listing is empty between tests (U7c: plugin-only, table dropped)", async () => { const steps = await harness.store().listWorkflowSteps(); expect(steps).toEqual([]); }); @@ -27,7 +25,6 @@ describe("createSharedTaskStoreTestHarness", () => { it("seeds state across multiple tables for truncation coverage", async () => { const store = harness.store(); const task = await store.createTask({ description: "seed" }); - await store.createWorkflowStep({ name: "Seed Step", description: "seed" }); const db = (store as any).db; db.prepare( `INSERT INTO agents (id, name, role, state, createdAt, updatedAt, metadata, data) diff --git a/packages/core/src/__tests__/store-workflow-steps.test.ts b/packages/core/src/__tests__/store-workflow-steps.test.ts deleted file mode 100644 index 42b9d205c2..0000000000 --- a/packages/core/src/__tests__/store-workflow-steps.test.ts +++ /dev/null @@ -1,950 +0,0 @@ -import { describe, it, expect, beforeAll, beforeEach, afterEach, afterAll, vi } from "vitest"; - -import { TaskStore } from "../store.js"; -import { createSharedTaskStoreTestHarness } from "./store-test-helpers.js"; - -describe("TaskStore Workflow Steps", () => { - const harness = createSharedTaskStoreTestHarness(); - let store: TaskStore; - - beforeAll(harness.beforeAll); - - beforeEach(async () => { - await harness.beforeEach(); - store = harness.store(); - }); - - afterEach(async () => { - await harness.afterEach(); - }); - - afterAll(harness.afterAll); - - describe("Workflow Steps", () => { - it("should create a workflow step with all fields", async () => { - const ws = await store.createWorkflowStep({ - name: "Documentation Review", - description: "Verify all public APIs have documentation", - prompt: "Review the task changes and verify that all new public functions have docs.", - enabled: true, - }); - - expect(ws.id).toBe("WS-001"); - expect(ws.name).toBe("Documentation Review"); - expect(ws.description).toBe("Verify all public APIs have documentation"); - expect(ws.mode).toBe("prompt"); - expect(ws.gateMode).toBe("advisory"); - expect(ws.prompt).toBe("Review the task changes and verify that all new public functions have docs."); - expect(ws.scriptName).toBeUndefined(); - expect(ws.enabled).toBe(true); - expect(ws.createdAt).toBeDefined(); - expect(ws.updatedAt).toBeDefined(); - }); - - it("should create a workflow step with minimal fields", async () => { - const ws = await store.createWorkflowStep({ - name: "QA Check", - description: "Run tests and verify they pass", - }); - - expect(ws.id).toBe("WS-001"); - expect(ws.name).toBe("QA Check"); - expect(ws.description).toBe("Run tests and verify they pass"); - expect(ws.mode).toBe("prompt"); // Default mode - expect(ws.gateMode).toBe("advisory"); // prompt default gate mode - expect(ws.prompt).toBe(""); // Empty when not provided - expect(ws.enabled).toBe(true); // Default enabled - }); - - it("should create a script-mode workflow step", async () => { - const ws = await store.createWorkflowStep({ - name: "Run Tests", - description: "Execute the test suite", - mode: "script", - scriptName: "test", - }); - - expect(ws.id).toBe("WS-001"); - expect(ws.name).toBe("Run Tests"); - expect(ws.mode).toBe("script"); - expect(ws.gateMode).toBe("advisory"); - expect(ws.prompt).toBe(""); - expect(ws.scriptName).toBe("test"); - expect(ws.modelProvider).toBeUndefined(); - expect(ws.modelId).toBeUndefined(); - expect(ws.enabled).toBe(true); - }); - - it("should round-trip gateMode create/list/update", async () => { - const promptStep = await store.createWorkflowStep({ - name: "Prompt advisory", - description: "advisory default", - mode: "prompt", - prompt: "review", - }); - const scriptStep = await store.createWorkflowStep({ - name: "Script advisory", - description: "advisory default", - mode: "script", - scriptName: "test", - }); - - expect(promptStep.gateMode).toBe("advisory"); - expect(scriptStep.gateMode).toBe("advisory"); - - await store.updateWorkflowStep(promptStep.id, { gateMode: "gate" }); - await store.updateWorkflowStep(scriptStep.id, { gateMode: "advisory" }); - const listed = await store.listWorkflowSteps(); - const updatedPrompt = listed.find((step) => step.id === promptStep.id); - const updatedScript = listed.find((step) => step.id === scriptStep.id); - - expect(updatedPrompt?.gateMode).toBe("gate"); - expect(updatedScript?.gateMode).toBe("advisory"); - }); - - it("should preserve explicit gate opt-in for script mode", async () => { - const ws = await store.createWorkflowStep({ - name: "Script gated", - description: "explicit gate opt-in", - mode: "script", - scriptName: "test", - gateMode: "gate", - }); - - expect(ws.gateMode).toBe("gate"); - }); - - it("should reject script mode without scriptName", async () => { - await expect( - store.createWorkflowStep({ - name: "Broken", - description: "No script name", - mode: "script", - }), - ).rejects.toThrow("Script mode requires a scriptName"); - }); - - it("should reject script mode with empty scriptName", async () => { - await expect( - store.createWorkflowStep({ - name: "Broken", - description: "Empty script name", - mode: "script", - scriptName: " ", - }), - ).rejects.toThrow("Script mode requires a scriptName"); - }); - - it("should auto-increment workflow step IDs", async () => { - const ws1 = await store.createWorkflowStep({ name: "Step 1", description: "First" }); - const ws2 = await store.createWorkflowStep({ name: "Step 2", description: "Second" }); - const ws3 = await store.createWorkflowStep({ name: "Step 3", description: "Third" }); - - expect(ws1.id).toBe("WS-001"); - expect(ws2.id).toBe("WS-002"); - expect(ws3.id).toBe("WS-003"); - }); - - it("should list workflow steps", async () => { - await store.createWorkflowStep({ name: "Step 1", description: "First" }); - await store.createWorkflowStep({ name: "Step 2", description: "Second" }); - - const steps = await store.listWorkflowSteps(); - expect(steps).toHaveLength(2); - expect(steps[0].name).toBe("Step 1"); - expect(steps[1].name).toBe("Step 2"); - }); - - it("should return empty array when no workflow steps exist", async () => { - const steps = await store.listWorkflowSteps(); - expect(steps).toHaveLength(0); - }); - - it("should get a single workflow step by ID", async () => { - const ws = await store.createWorkflowStep({ name: "Docs", description: "Check docs" }); - const found = await store.getWorkflowStep(ws.id); - - expect(found).toBeDefined(); - expect(found!.id).toBe(ws.id); - expect(found!.name).toBe("Docs"); - }); - - it("should return undefined for non-existent workflow step", async () => { - const found = await store.getWorkflowStep("WS-999"); - expect(found).toBeUndefined(); - }); - - it("should resolve plugin script-mode workflow steps from injected templates", async () => { - store.setPluginWorkflowStepTemplates([ - { - pluginId: "my-plugin", - template: { - id: "plugin:my-plugin:my-step", - name: "My Plugin Step", - description: "Plugin-provided step", - mode: "script", - phase: "pre-merge", - scriptName: "my-plugin:run-step", - prompt: "", - toolMode: "readonly", - defaultOn: false, - modelProvider: "anthropic", - modelId: "claude-sonnet-4-5", - category: "Plugin", - icon: "puzzle", - }, - }, - ]); - - const listed = await store.listWorkflowSteps(); - const listedStep = listed.find((candidate) => candidate.id === "plugin:my-plugin:my-step"); - expect(listedStep).toMatchObject({ - id: "plugin:my-plugin:my-step", - templateId: "my-step", - name: "My Plugin Step", - mode: "script", - gateMode: "advisory", - phase: "pre-merge", - scriptName: "my-plugin:run-step", - defaultOn: false, - modelProvider: "anthropic", - modelId: "claude-sonnet-4-5", - }); - - const step = await store.getWorkflowStep("plugin:my-plugin:my-step"); - expect(step).toMatchObject({ - id: "plugin:my-plugin:my-step", - templateId: "my-step", - mode: "script", - gateMode: "advisory", - phase: "pre-merge", - scriptName: "my-plugin:run-step", - defaultOn: false, - modelProvider: "anthropic", - modelId: "claude-sonnet-4-5", - enabled: true, - }); - }); - - it("should resolve plugin prompt-mode workflow steps from injected templates", async () => { - store.setPluginWorkflowStepTemplates([ - { - pluginId: "my-plugin", - template: { - id: "plugin:my-plugin:prompt-step", - name: "My Prompt Step", - description: "Prompt plugin step", - mode: "prompt", - phase: "pre-merge", - prompt: "Run plugin checks", - toolMode: "readonly", - category: "Plugin", - icon: "puzzle", - }, - }, - ]); - - const step = await store.getWorkflowStep("plugin:my-plugin:prompt-step"); - expect(step).toMatchObject({ - id: "plugin:my-plugin:prompt-step", - templateId: "prompt-step", - mode: "prompt", - gateMode: "advisory", - prompt: "Run plugin checks", - }); - }); - - it("should list db workflow steps and plugin workflow steps together", async () => { - const dbStep = await store.createWorkflowStep({ name: "DB Step", description: "stored" }); - store.setPluginWorkflowStepTemplates([ - { - pluginId: "my-plugin", - template: { - id: "plugin:my-plugin:my-step", - name: "My Plugin Step", - description: "Plugin-provided step", - prompt: "Run plugin checks", - toolMode: "coding", - category: "Plugin", - icon: "puzzle", - }, - }, - ]); - - const steps = await store.listWorkflowSteps(); - expect(steps.map((step) => step.id)).toEqual([dbStep.id, "plugin:my-plugin:my-step"]); - }); - - it("should list disabled plugin steps without auto-materializing them", async () => { - store.setPluginWorkflowStepTemplates([ - { - pluginId: "my-plugin", - template: { - id: "plugin:my-plugin:disabled-step", - name: "Disabled Plugin Step", - description: "Plugin-provided step", - prompt: "Run plugin checks", - toolMode: "readonly", - category: "Plugin", - icon: "puzzle", - enabled: false, - }, - }, - ]); - - const listed = await store.listWorkflowSteps(); - expect(listed.find((step) => step.id === "plugin:my-plugin:disabled-step")?.enabled).toBe(false); - - const task = await store.createTask({ - description: "Task with plugin-only workflow steps", - enabledWorkflowSteps: ["plugin:my-plugin:disabled-step"], - }); - expect(task.enabledWorkflowSteps).toEqual(["plugin:my-plugin:disabled-step"]); - }); - - it("keeps plugin and former-built-in workflow ids unchanged (all pass through)", async () => { - store.setPluginWorkflowStepTemplates([ - { - pluginId: "my-plugin", - template: { - id: "plugin:my-plugin:my-step", - name: "My Plugin Step", - description: "Plugin-provided step", - prompt: "Run plugin checks", - toolMode: "readonly", - category: "Plugin", - icon: "puzzle", - }, - }, - ]); - - // FNXC:WorkflowStepTemplate 2026-06-25-00:00: U6 deleted the built-in catalog + - // template materializer, so resolveEnabledWorkflowSteps is a pure pass-through. Both - // the plugin id AND a former built-in template id (frontend-ux-design) are kept - // verbatim — nothing materializes into a WS row. - const task = await store.createTask({ - description: "Task with mixed workflow steps", - enabledWorkflowSteps: ["plugin:my-plugin:my-step", "frontend-ux-design"], - }); - - expect(task.enabledWorkflowSteps).toEqual(["plugin:my-plugin:my-step", "frontend-ux-design"]); - - const steps = await store.listWorkflowSteps(); - expect(steps.filter((step) => step.templateId === "frontend-ux-design")).toHaveLength(0); - }); - - it("should update a workflow step", async () => { - const ws = await store.createWorkflowStep({ - name: "Original", - description: "Original desc", - prompt: "Original prompt", - }); - - const updated = await store.updateWorkflowStep(ws.id, { - name: "Updated", - description: "Updated desc", - prompt: "Updated prompt", - enabled: false, - }); - - expect(updated.name).toBe("Updated"); - expect(updated.description).toBe("Updated desc"); - expect(updated.mode).toBe("prompt"); - expect(updated.prompt).toBe("Updated prompt"); - expect(updated.enabled).toBe(false); - expect(new Date(updated.updatedAt).getTime()).toBeGreaterThanOrEqual( - new Date(ws.updatedAt).getTime() - ); - }); - - it("should switch a workflow step from prompt to script mode", async () => { - const ws = await store.createWorkflowStep({ - name: "Docs", - description: "Check docs", - prompt: "Review documentation.", - mode: "prompt", - modelProvider: "anthropic", - modelId: "claude-sonnet-4-5", - }); - - const updated = await store.updateWorkflowStep(ws.id, { - mode: "script", - scriptName: "lint", - }); - - expect(updated.mode).toBe("script"); - expect(updated.scriptName).toBe("lint"); - expect(updated.prompt).toBe(""); // Cleared on mode switch - expect(updated.modelProvider).toBeUndefined(); // Cleared on mode switch - expect(updated.modelId).toBeUndefined(); // Cleared on mode switch - }); - - it("should switch a workflow step from script to prompt mode", async () => { - const ws = await store.createWorkflowStep({ - name: "Lint", - description: "Run linting", - mode: "script", - scriptName: "lint", - }); - - const updated = await store.updateWorkflowStep(ws.id, { - mode: "prompt", - prompt: "Review code quality.", - }); - - expect(updated.mode).toBe("prompt"); - expect(updated.scriptName).toBeUndefined(); // Cleared on mode switch - expect(updated.prompt).toBe("Review code quality."); - }); - - it("should reject switching to script mode without scriptName", async () => { - const ws = await store.createWorkflowStep({ - name: "Docs", - description: "Check docs", - prompt: "Review documentation.", - }); - - await expect( - store.updateWorkflowStep(ws.id, { mode: "script" }), - ).rejects.toThrow("Script mode requires a scriptName"); - }); - - it("should ignore prompt updates for script-mode steps", async () => { - const ws = await store.createWorkflowStep({ - name: "Lint", - description: "Run linting", - mode: "script", - scriptName: "lint", - }); - - const updated = await store.updateWorkflowStep(ws.id, { - prompt: "This should be ignored", - }); - - expect(updated.prompt).toBe(""); // Prompt not updated for script mode - }); - - it("should ignore model override updates for script-mode steps", async () => { - const ws = await store.createWorkflowStep({ - name: "Lint", - description: "Run linting", - mode: "script", - scriptName: "lint", - }); - - const updated = await store.updateWorkflowStep(ws.id, { - modelProvider: "anthropic", - modelId: "claude-sonnet-4-5", - }); - - // Model overrides should not be set for script mode - expect(updated.modelProvider).toBeUndefined(); - expect(updated.modelId).toBeUndefined(); - }); - - it("should throw when updating non-existent workflow step", async () => { - await expect( - store.updateWorkflowStep("WS-999", { name: "Nope" }) - ).rejects.toThrow("Workflow step 'WS-999' not found"); - }); - - it("should delete a workflow step", async () => { - await harness.useIsolatedStore(); - store = harness.store(); - const ws = await store.createWorkflowStep({ name: "ToDelete", description: "Gone" }); - await store.deleteWorkflowStep(ws.id); - - const steps = await store.listWorkflowSteps(); - expect(steps).toHaveLength(0); - }); - - it("should throw when deleting non-existent workflow step", async () => { - await expect(store.deleteWorkflowStep("WS-999")).rejects.toThrow( - "Workflow step 'WS-999' not found" - ); - }); - - it("should remove references from tasks when deleting a workflow step", async () => { - const ws = await store.createWorkflowStep({ name: "Docs", description: "Check docs" }); - const task = await store.createTask({ - description: "Test task with workflow steps", - enabledWorkflowSteps: [ws.id], - }); - - expect(task.enabledWorkflowSteps).toEqual([ws.id]); - - await store.deleteWorkflowStep(ws.id); - - // Wait for async cleanup - await new Promise((r) => setTimeout(r, 50)); - - const updatedTask = await store.getTask(task.id); - expect(updatedTask.enabledWorkflowSteps).toBeUndefined(); - }); - - it("should create a task with enabledWorkflowSteps", async () => { - const ws1 = await store.createWorkflowStep({ name: "Docs", description: "Check docs" }); - const ws2 = await store.createWorkflowStep({ name: "QA", description: "Run tests" }); - - const task = await store.createTask({ - description: "Task with workflow steps", - enabledWorkflowSteps: [ws1.id, ws2.id], - }); - - expect(task.enabledWorkflowSteps).toEqual([ws1.id, ws2.id]); - }); - - // FNXC:WorkflowStepTemplate 2026-06-25-00:00: U6 deleted the built-in - // WORKFLOW_STEP_TEMPLATES catalog + the template materializer, so - // resolveEnabledWorkflowSteps is now a pure identity-stable pass-through. A former - // built-in template id (frontend-ux-design) no longer materializes into a WS row — it - // passes through verbatim, exactly like any other enable id. - it("passes a former built-in template id (frontend-ux-design) through untouched without materializing", async () => { - const task = await store.createTask({ - description: "Task with frontend ux design", - enabledWorkflowSteps: ["frontend-ux-design"], - }); - - expect(task.enabledWorkflowSteps).toEqual(["frontend-ux-design"]); - - const steps = await store.listWorkflowSteps(); - expect(steps.filter((step) => step.templateId === "frontend-ux-design")).toHaveLength(0); - }); - - /* - FNXC:WorkflowOptionalGroup 2026-06-26-04:30: - FN-7039 regression. A built-in optional-group id that collides with a - WORKFLOW_STEP_TEMPLATE id (browser-verification) must pass through - `enabledWorkflowSteps` UNTOUCHED even when the task has no explicit workflow and - the project has no default workflow — the executor resolves such tasks to - builtin:coding, whose optional-group node id is "browser-verification", and gates - on `enabledWorkflowSteps.includes(node.id)`. Materializing it into a WS-NNN row - (the prior bug) left the executor unable to match the node, so the toggled - optional step silently never ran and never appeared in the unified step progress - bar. It must therefore NOT create a materialized step row. - */ - it("passes a builtin:coding optional-group id (browser-verification) through untouched without materializing (FN-7039)", async () => { - const task = await store.createTask({ - description: "Task with browser verification optional step", - enabledWorkflowSteps: ["browser-verification"], - }); - - expect(task.enabledWorkflowSteps).toEqual(["browser-verification"]); - - const steps = await store.listWorkflowSteps(); - expect(steps.filter((step) => step.templateId === "browser-verification")).toHaveLength(0); - }); - - it("passes the code-review optional-group id through untouched (FN-7039)", async () => { - const task = await store.createTask({ - description: "Task with code review optional step", - enabledWorkflowSteps: ["code-review"], - }); - - expect(task.enabledWorkflowSteps).toEqual(["code-review"]); - }); - - // FNXC:WorkflowStepTemplate 2026-06-25-00:00: with pass-through resolution, the same - // former-built-in id used across two tasks stays identical and creates no rows (the - // old "reuse the materialized row" semantics no longer apply — nothing is materialized). - it("keeps a former built-in template id identical across tasks without materializing any row", async () => { - const first = await store.createTask({ - description: "First frontend ux design task", - enabledWorkflowSteps: ["frontend-ux-design"], - }); - const second = await store.createTask({ - description: "Second frontend ux design task", - enabledWorkflowSteps: ["frontend-ux-design"], - }); - - expect(first.enabledWorkflowSteps).toEqual(["frontend-ux-design"]); - expect(second.enabledWorkflowSteps).toEqual(["frontend-ux-design"]); - - const steps = await store.listWorkflowSteps(); - expect(steps.filter((step) => step.templateId === "frontend-ux-design")).toHaveLength(0); - }); - - it("should not set enabledWorkflowSteps when empty array provided", async () => { - const task = await store.createTask({ - description: "Task without workflow steps", - enabledWorkflowSteps: [], - }); - - expect(task.enabledWorkflowSteps).toBeUndefined(); - }); - - it("should create a workflow step with model override", async () => { - const ws = await store.createWorkflowStep({ - name: "Security Audit", - description: "Check for security issues", - prompt: "Scan for vulnerabilities.", - enabled: true, - modelProvider: "anthropic", - modelId: "claude-sonnet-4-5", - }); - - expect(ws.modelProvider).toBe("anthropic"); - expect(ws.modelId).toBe("claude-sonnet-4-5"); - }); - - it("should create a workflow step without model override", async () => { - const ws = await store.createWorkflowStep({ - name: "QA Check", - description: "Run tests", - }); - - expect(ws.modelProvider).toBeUndefined(); - expect(ws.modelId).toBeUndefined(); - }); - - it("should update a workflow step model override", async () => { - const ws = await store.createWorkflowStep({ - name: "Docs", - description: "Check docs", - }); - - const updated = await store.updateWorkflowStep(ws.id, { - modelProvider: "openai", - modelId: "gpt-4o", - }); - - expect(updated.modelProvider).toBe("openai"); - expect(updated.modelId).toBe("gpt-4o"); - }); - - it("should clear a workflow step model override by setting to undefined", async () => { - const ws = await store.createWorkflowStep({ - name: "Docs", - description: "Check docs", - modelProvider: "anthropic", - modelId: "claude-sonnet-4-5", - }); - - expect(ws.modelProvider).toBe("anthropic"); - - const updated = await store.updateWorkflowStep(ws.id, { - modelProvider: undefined, - modelId: undefined, - }); - - expect(updated.modelProvider).toBeUndefined(); - expect(updated.modelId).toBeUndefined(); - }); - - it("should persist model override across list/get", async () => { - const ws = await store.createWorkflowStep({ - name: "Perf Review", - description: "Check performance", - modelProvider: "anthropic", - modelId: "claude-sonnet-4-5", - }); - - const listed = await store.listWorkflowSteps(); - expect(listed[0].modelProvider).toBe("anthropic"); - expect(listed[0].modelId).toBe("claude-sonnet-4-5"); - - const found = await store.getWorkflowStep(ws.id); - expect(found!.modelProvider).toBe("anthropic"); - expect(found!.modelId).toBe("claude-sonnet-4-5"); - }); - - it("should normalize legacy workflow steps without mode to prompt mode", async () => { - // Create a step normally (it will have mode: "prompt") - const ws = await store.createWorkflowStep({ - name: "Legacy Step", - description: "Pre-existing step", - prompt: "Review the code.", - }); - - // Simulate legacy data by writing a step without mode directly to DB - const config = await (store as any).readConfig(); - // Remove mode from the stored step to simulate legacy data - delete config.workflowSteps[0].mode; - await (store as any).writeConfig(config); - - // Re-read should normalize mode to "prompt" - const found = await store.getWorkflowStep(ws.id); - expect(found!.mode).toBe("prompt"); - expect(found!.prompt).toBe("Review the code."); - }); - - it("should persist script-mode workflow step across list/get", async () => { - const ws = await store.createWorkflowStep({ - name: "Type Check", - description: "Run TypeScript type checking", - mode: "script", - scriptName: "typecheck", - }); - - const listed = await store.listWorkflowSteps(); - expect(listed[0].mode).toBe("script"); - expect(listed[0].scriptName).toBe("typecheck"); - - const found = await store.getWorkflowStep(ws.id); - expect(found!.mode).toBe("script"); - expect(found!.scriptName).toBe("typecheck"); - }); - - // ── Workflow Step defaultOn ────────────────────────────────────────────── - - it("should persist defaultOn flag on workflow step creation", async () => { - const ws = await store.createWorkflowStep({ - name: "Default-on Step", - description: "Auto-selected for new tasks", - defaultOn: true, - }); - - expect(ws.defaultOn).toBe(true); - - const found = await store.getWorkflowStep(ws.id); - expect(found!.defaultOn).toBe(true); - - // Verify persistence - const steps = await store.listWorkflowSteps(); - expect(steps[0].defaultOn).toBe(true); - }); - - it("should not set defaultOn by default", async () => { - const ws = await store.createWorkflowStep({ - name: "Non-default Step", - description: "Not auto-selected", - }); - - expect(ws.defaultOn).toBeUndefined(); - - const found = await store.getWorkflowStep(ws.id); - expect(found!.defaultOn).toBeUndefined(); - }); - - it("should update defaultOn flag on workflow step", async () => { - const ws = await store.createWorkflowStep({ - name: "Step", - description: "Desc", - }); - - const updated = await store.updateWorkflowStep(ws.id, { defaultOn: true }); - expect(updated.defaultOn).toBe(true); - - const found = await store.getWorkflowStep(ws.id); - expect(found!.defaultOn).toBe(true); - }); - - it("should clear defaultOn flag by setting to false", async () => { - const ws = await store.createWorkflowStep({ - name: "Step", - description: "Desc", - defaultOn: true, - }); - - const updated = await store.updateWorkflowStep(ws.id, { defaultOn: false }); - expect(updated.defaultOn).toBe(false); - - const found = await store.getWorkflowStep(ws.id); - expect(found!.defaultOn).toBe(false); - }); - - it("should auto-apply default-on workflow steps when creating task without enabledWorkflowSteps", async () => { - await store.createWorkflowStep({ name: "Always Run", description: "Auto-select", enabled: true, defaultOn: true }); - await store.createWorkflowStep({ name: "Optional Check", description: "Only when manually selected", enabled: true, defaultOn: false }); - await store.createWorkflowStep({ name: "Disabled Step", description: "Disabled step", enabled: false, defaultOn: true }); - - const task = await store.createTask({ description: "Test task" }); - - // Only the enabled + defaultOn step should be auto-applied - expect(task.enabledWorkflowSteps).toEqual(["WS-001"]); - }); - - it("should use explicit enabledWorkflowSteps over default-on steps", async () => { - await store.createWorkflowStep({ name: "Always Run", description: "Auto-select", enabled: true, defaultOn: true }); - - const task = await store.createTask({ - description: "Test task", - enabledWorkflowSteps: ["WS-001", "WS-002"], - }); - - // Explicit input takes precedence - expect(task.enabledWorkflowSteps).toEqual(["WS-001", "WS-002"]); - }); - - it("should use empty enabledWorkflowSteps to override default-on steps", async () => { - await store.createWorkflowStep({ name: "Always Run", description: "Auto-select", enabled: true, defaultOn: true }); - - const task = await store.createTask({ - description: "Test task", - enabledWorkflowSteps: [], - }); - - // Explicit empty array means user intentionally wants no steps - expect(task.enabledWorkflowSteps).toBeUndefined(); - }); - - it("should not auto-apply disabled steps even with defaultOn flag", async () => { - await store.createWorkflowStep({ name: "Disabled Step", description: "Disabled step", enabled: false, defaultOn: true }); - - const task = await store.createTask({ description: "Test task" }); - - expect(task.enabledWorkflowSteps).toBeUndefined(); - }); - - it("should auto-apply multiple default-on steps in order", async () => { - await store.createWorkflowStep({ name: "First", description: "First", enabled: true, defaultOn: true }); - await store.createWorkflowStep({ name: "Second", description: "Second", enabled: true, defaultOn: true }); - await store.createWorkflowStep({ name: "Third", description: "Third", enabled: true, defaultOn: false }); - - const task = await store.createTask({ description: "Test task" }); - - expect(task.enabledWorkflowSteps).toEqual(["WS-001", "WS-002"]); - }); - - it("logs default-on resolution failures and still creates the task", async () => { - const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); - const listStepsSpy = vi.spyOn(store, "listWorkflowSteps").mockRejectedValue(new Error("workflow catalog unavailable")); - - try { - const task = await store.createTask({ description: "Best effort defaults" }); - expect(task.id).toMatch(/^FN-\d+$/); - expect(task.enabledWorkflowSteps).toBeUndefined(); - - const warningCall = warnSpy.mock.calls.find( - (call) => typeof call[0] === "string" && call[0].includes("[task-store] Failed to auto-apply default workflow steps during task creation"), - ); - expect(warningCall).toBeDefined(); - - const [, context] = warningCall as [string, Record]; - expect(context).toMatchObject({ - descriptionLength: "Best effort defaults".length, - error: "workflow catalog unavailable", - }); - } finally { - listStepsSpy.mockRestore(); - warnSpy.mockRestore(); - } - }); - - // FNXC:WorkflowStepTemplate 2026-06-25-00:00: U6 made resolveEnabledWorkflowSteps a - // pure pass-through on the update path too — a former built-in template id is kept - // verbatim and never materialized into a WS row. - it("passes a former built-in template id through updateTask untouched (no materialization)", async () => { - const task = await store.createTask({ description: "Editable task" }); - - const updated = await store.updateTask(task.id, { - enabledWorkflowSteps: ["frontend-ux-design"], - }); - - expect(updated.enabledWorkflowSteps).toEqual(["frontend-ux-design"]); - - const persisted = await store.getTask(task.id); - expect(persisted.enabledWorkflowSteps).toEqual(["frontend-ux-design"]); - - const steps = await store.listWorkflowSteps(); - expect(steps.filter((step) => step.templateId === "frontend-ux-design")).toHaveLength(0); - }); - - // FNXC:WorkflowOptionalGroup 2026-06-26-04:30: FN-7039 update-path surface — a - // builtin:coding optional-group id must also pass through updateTask untouched - // (not materialized) so toggling it on after creation still runs in the executor. - it("passes a builtin:coding optional-group id through updateTask untouched (FN-7039)", async () => { - const task = await store.createTask({ description: "Editable task" }); - - const updated = await store.updateTask(task.id, { - enabledWorkflowSteps: ["browser-verification"], - }); - - expect(updated.enabledWorkflowSteps).toEqual(["browser-verification"]); - - const persisted = await store.getTask(task.id); - expect(persisted.enabledWorkflowSteps).toEqual(["browser-verification"]); - - const steps = await store.listWorkflowSteps(); - expect(steps.filter((step) => step.templateId === "browser-verification")).toHaveLength(0); - }); - - // FNXC:WorkflowStepTemplate 2026-06-25-00:00: U6 deleted the built-in - // WORKFLOW_STEP_TEMPLATES catalog and the getWorkflowStep built-in-synthesis - // fallback. Built-in quality gates (browser-verification, code-review) are now graph - // optional-group nodes, not synthesized WorkflowStep rows — so getWorkflowStep returns - // undefined for a built-in id that has no stored row. - it("returns undefined for built-in optional-group ids (no longer synthesized)", async () => { - expect(await store.getWorkflowStep("browser-verification")).toBeUndefined(); - expect(await store.getWorkflowStep("frontend-ux-design")).toBeUndefined(); - }); - - // ── Workflow Step Phase ────────────────────────────────────────────── - - it("should default phase to 'pre-merge' when creating a workflow step", async () => { - const ws = await store.createWorkflowStep({ - name: "Pre-merge Check", - description: "Runs before merge", - }); - - expect(ws.phase).toBe("pre-merge"); - }); - - it("should create a workflow step with explicit 'post-merge' phase", async () => { - const ws = await store.createWorkflowStep({ - name: "Post-merge Notify", - description: "Runs after merge", - phase: "post-merge", - }); - - expect(ws.phase).toBe("post-merge"); - }); - - it("should create a workflow step with explicit 'pre-merge' phase", async () => { - const ws = await store.createWorkflowStep({ - name: "Pre-merge Gate", - description: "Runs before merge", - phase: "pre-merge", - }); - - expect(ws.phase).toBe("pre-merge"); - }); - - it("should update a workflow step phase from pre-merge to post-merge", async () => { - const ws = await store.createWorkflowStep({ - name: "Phase Switch", - description: "Will switch phase", - }); - - expect(ws.phase).toBe("pre-merge"); - - const updated = await store.updateWorkflowStep(ws.id, { phase: "post-merge" }); - expect(updated.phase).toBe("post-merge"); - }); - - it("should persist phase across list/get", async () => { - const ws = await store.createWorkflowStep({ - name: "Phase Persist", - description: "Check phase persistence", - phase: "post-merge", - }); - - const listed = await store.listWorkflowSteps(); - expect(listed[0].phase).toBe("post-merge"); - - const found = await store.getWorkflowStep(ws.id); - expect(found!.phase).toBe("post-merge"); - }); - - it("should normalize legacy workflow steps without phase to pre-merge", async () => { - const ws = await store.createWorkflowStep({ - name: "Legacy Step", - description: "Pre-existing step", - prompt: "Review the code.", - }); - - // Simulate legacy data by removing phase from the stored step - const config = await (store as any).readConfig(); - delete config.workflowSteps[0].phase; - await (store as any).writeConfig(config); - - // Re-read: phase should be undefined (legacy), but when used by engine - // it should be treated as "pre-merge" - const found = await store.getWorkflowStep(ws.id); - expect(found!.phase).toBeUndefined(); - }); - }); - - // ── Title Summarization Tests ──────────────────────────────────────────── - -}); diff --git a/packages/core/src/__tests__/workflow-post-merge-cutover-migration.test.ts b/packages/core/src/__tests__/workflow-post-merge-cutover-migration.test.ts index cb42bdc6cd..7a6dd3331b 100644 --- a/packages/core/src/__tests__/workflow-post-merge-cutover-migration.test.ts +++ b/packages/core/src/__tests__/workflow-post-merge-cutover-migration.test.ts @@ -19,10 +19,41 @@ leaves already-node-id and compiled-workflow entries untouched, and is idempoten const WS_BV = "WS-TEST-BV"; // legacy compiled row for the browser-verification optional group const WS_DOC = "WS-TEST-DOC"; // compiled-workflow materialization row (templateId workflow:*) +// FNXC:WorkflowPostMerge 2026-06-26-14:00: U7c dropped `workflow_steps` from SCHEMA_SQL, +// so a fresh test DB no longer has the table. To exercise migration 130's normalization of +// LEGACY data we recreate the legacy table shape on disk before seeding rows (the same shape +// historical migration 16 created). Migration 130 then reads it; migration 131 drops it. +function createLegacyWorkflowStepsTable(db: { + prepare: (sql: string) => { run: (...args: unknown[]) => unknown }; +}): void { + db.prepare( + `CREATE TABLE IF NOT EXISTS workflow_steps ( + id TEXT PRIMARY KEY, + templateId TEXT, + name TEXT NOT NULL, + description TEXT NOT NULL, + mode TEXT NOT NULL DEFAULT 'prompt', + phase TEXT NOT NULL DEFAULT 'pre-merge', + prompt TEXT NOT NULL DEFAULT '', + gateMode TEXT NOT NULL DEFAULT 'advisory', + toolMode TEXT, + scriptName TEXT, + enabled INTEGER NOT NULL DEFAULT 1, + defaultOn INTEGER DEFAULT 0, + modelProvider TEXT, + modelId TEXT, + migrated_fragment_id TEXT, + createdAt TEXT NOT NULL, + updatedAt TEXT NOT NULL + )`, + ).run(); +} + function insertWorkflowStep( db: { prepare: (sql: string) => { run: (...args: unknown[]) => unknown } }, args: { id: string; templateId: string; name: string; phase: string }, ): void { + createLegacyWorkflowStepsTable(db); const now = new Date().toISOString(); db.prepare( `INSERT OR REPLACE INTO workflow_steps @@ -99,3 +130,6 @@ describe("Migration 130: post-merge cutover enable-id normalization", () => { expect(afterSecond).toEqual([BROWSER_VERIFICATION_GROUP_ID]); }); }); + +// The seed-at-130 table-drop (migration 131) coverage lives in its own file: +// workflow-steps-table-drop-migration.test.ts. diff --git a/packages/core/src/__tests__/workflow-prompt-overrides-store.test.ts b/packages/core/src/__tests__/workflow-prompt-overrides-store.test.ts index 98e1dca066..79dbf79a37 100644 --- a/packages/core/src/__tests__/workflow-prompt-overrides-store.test.ts +++ b/packages/core/src/__tests__/workflow-prompt-overrides-store.test.ts @@ -144,15 +144,23 @@ describe("TaskStore workflow prompt overrides", () => { ); }); - it("materializes built-in non-seam prompt and gate overrides into WorkflowStep rows", async () => { + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c dropped the `workflow_steps` table and the + // compilation materializer. Built-in non-seam prompt/gate overrides are no longer baked + // into materialized WorkflowStep rows — they overlay the workflow IR the graph runs. + // Verify the override through the task's RESOLVED IR (the node config the executor reads). + it("overlays built-in non-seam prompt and gate overrides onto the resolved workflow IR", async () => { const store = harness.store(); const projectId = store.getWorkflowSettingsProjectId(); store.updateWorkflowPromptOverrides("builtin:review-heavy", projectId, { security: "Security materialized override" }); store.updateWorkflowPromptOverrides("builtin:compound-engineering", projectId, { plan: "Plan materialized override" }); + const nodePrompt = (ir: WorkflowIr, nodeId: string): string | undefined => + ir.nodes.find((node) => node.id === nodeId)?.config?.prompt as string | undefined; + const reviewTask = await store.createTask({ description: "review heavy", workflowId: "builtin:review-heavy" }); - const reviewSteps = await Promise.all((reviewTask.enabledWorkflowSteps ?? []).map((id) => store.getWorkflowStep(id))); - expect(reviewSteps.find((step) => step?.name === "Security review")?.prompt).toBe("Security materialized override"); + expect(nodePrompt(await resolveWorkflowIrForTask(store, reviewTask.id), "security")).toBe( + "Security materialized override", + ); const ceIr = getBuiltinWorkflow("builtin:compound-engineering")!.ir; const originalPlan = ceIr.nodes.find((node) => node.id === "plan")?.config?.prompt; @@ -161,9 +169,9 @@ describe("TaskStore workflow prompt overrides", () => { // project; the pure overlay test covers CE compilation directly. if (ceDef) { const ceTask = await store.createTask({ description: "compound", workflowId: "builtin:compound-engineering" }); - const ceSteps = await Promise.all((ceTask.enabledWorkflowSteps ?? []).map((id) => store.getWorkflowStep(id))); - expect(ceSteps.find((step) => step?.name === "Plan")?.prompt).toBe("Plan materialized override"); + expect(nodePrompt(await resolveWorkflowIrForTask(store, ceTask.id), "plan")).toBe("Plan materialized override"); } + // The shared builtin IR constant is never mutated by the per-project overlay. expect(ceIr.nodes.find((node) => node.id === "plan")?.config?.prompt).toBe(originalPlan); }); @@ -173,10 +181,24 @@ describe("TaskStore workflow prompt overrides", () => { const db = store.getDatabase(); db.prepare("DROP INDEX IF EXISTS idx_workflow_prompt_overrides_project").run(); db.prepare("DROP TABLE IF EXISTS workflow_prompt_overrides").run(); + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c removed `workflow_steps` from SCHEMA_SQL, + // so a freshly-created DB lacks it. A REAL v127 DB had the table (migration 16). Recreate + // it so this downgrade faithfully simulates a real v127 DB — migration 130 reads it and + // migration 131 drops it on the way up to the current schema. + db.prepare( + "CREATE TABLE IF NOT EXISTS workflow_steps (id TEXT PRIMARY KEY, templateId TEXT, name TEXT, description TEXT, mode TEXT, phase TEXT, prompt TEXT, gateMode TEXT, toolMode TEXT, scriptName TEXT, enabled INTEGER, defaultOn INTEGER, modelProvider TEXT, modelId TEXT, migrated_fragment_id TEXT, createdAt TEXT, updatedAt TEXT)", + ).run(); db.prepare("UPDATE __meta SET value = '127' WHERE key = 'schemaVersion'").run(); await harness.reopenDiskBackedStore(); + // The cutover (migration 131) dropped the legacy table on the way back to current. + expect( + harness.store().getDatabase() + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'workflow_steps'") + .get(), + ).toBeUndefined(); + const migratedDb = harness.store().getDatabase(); const table = migratedDb .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'workflow_prompt_overrides'") diff --git a/packages/core/src/__tests__/workflow-restart-durability.test.ts b/packages/core/src/__tests__/workflow-restart-durability.test.ts index 17c6bc454f..fb77a7dfa5 100644 --- a/packages/core/src/__tests__/workflow-restart-durability.test.ts +++ b/packages/core/src/__tests__/workflow-restart-durability.test.ts @@ -14,20 +14,37 @@ FNXC:CustomWorkflows 2026-06-17-10:55: FN-6580 found no restart evidence for explicit custom-workflow selections, interpreter-deferred built-ins, or their graph/foreach run progress. These tests use the disk-backed store reopen seam instead of booting the engine so restart durability stays fast while proving the store cannot silently switch an in-flight task to a different workflow after process restart. */ +/* +FNXC:WorkflowStepCRUD 2026-06-26-14:00: +U7c dropped the `workflow_steps` table + the compilation materializer. A selection's +`stepIds` are now the default-on `optional-group` node ids (executor toggle keys), not +materialized step rows. This IR carries one default-on optional-group ("review-group") so +a NON-EMPTY selection's durability across restart is still exercised. +*/ function linearIr(): WorkflowIr { return { - version: "v1", + version: "v2", name: "restart-linear", + columns: [{ id: "todo", name: "Todo", traits: [] }], nodes: [ - { id: "start", kind: "start" }, - { id: "lint", kind: "gate", config: { name: "Lint", scriptName: "lint" } }, - { id: "spec", kind: "prompt", config: { name: "Spec", prompt: "verify restart" } }, - { id: "end", kind: "end" }, + { id: "start", kind: "start", column: "todo" }, + { id: "lint", kind: "gate", column: "todo", config: { name: "Lint", scriptName: "lint" } }, + { + id: "review-group", + kind: "optional-group", + column: "todo", + config: { + name: "Review", + defaultOn: true, + template: { nodes: [{ id: "review-inner", kind: "prompt", config: { prompt: "verify restart" } }], edges: [] }, + }, + }, + { id: "end", kind: "end", column: "todo" }, ], edges: [ { from: "start", to: "lint", condition: "success" }, - { from: "lint", to: "spec", condition: "success" }, - { from: "spec", to: "end", condition: "success" }, + { from: "lint", to: "review-group", condition: "success" }, + { from: "review-group", to: "end", condition: "success" }, ], }; } @@ -123,7 +140,7 @@ describe("workflow restart durability for explicit selections", () => { const task = await store().createTask({ description: "custom selection", enabledWorkflowSteps: [] }); const selectedStepIds = await store().selectTaskWorkflow(task.id, workflow.id); - expect(selectedStepIds).toHaveLength(2); + expect(selectedStepIds).toEqual(["review-group"]); store().saveWorkflowRunBranch({ taskId: task.id, runId: "run-restart", @@ -157,9 +174,6 @@ describe("workflow restart durability for explicit selections", () => { expect(selection).toEqual({ workflowId: workflow.id, stepIds: selectedStepIds }); expect((await store().getTask(task.id)).enabledWorkflowSteps).toEqual(selectedStepIds); expect(await taskJsonEnabledWorkflowSteps(task.id)).toEqual(selectedStepIds); - for (const stepId of selectedStepIds) { - expect(await store().getWorkflowStep(stepId)).toBeDefined(); - } expect(store().loadWorkflowRunBranches(task.id, "run-restart")).toEqual( expect.arrayContaining([ @@ -217,16 +231,21 @@ describe("workflow restart durability for explicit selections", () => { ]); }); - it("persists interpreter-deferred builtin selection with zero materialized steps across restart", async () => { + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c — explicit selection of an + // interpreter-deferred builtin now seeds its DEFAULT-ON optional-group ids (here + // `code-review`), consistent with the create-time selection path. (The pre-U7c + // selectTaskWorkflow returned [] for this case — an inconsistency with create-time + // seeding — because it only returned materialized step ids, which no longer exist.) + it("persists interpreter-deferred builtin selection seeding its default-on group across restart", async () => { const task = await store().createTask({ description: "builtin selection", enabledWorkflowSteps: [] }); - await expect(store().selectTaskWorkflow(task.id, "builtin:coding")).resolves.toEqual([]); + await expect(store().selectTaskWorkflow(task.id, "builtin:coding")).resolves.toEqual(["code-review"]); await reopenAsDiskBackedStore(); - expect(store().getTaskWorkflowSelection(task.id)).toEqual({ workflowId: "builtin:coding", stepIds: [] }); - expect((await store().getTask(task.id)).enabledWorkflowSteps ?? []).toEqual([]); - expect((await taskJsonEnabledWorkflowSteps(task.id)) ?? []).toEqual([]); + expect(store().getTaskWorkflowSelection(task.id)).toEqual({ workflowId: "builtin:coding", stepIds: ["code-review"] }); + expect((await store().getTask(task.id)).enabledWorkflowSteps ?? []).toEqual(["code-review"]); + expect((await taskJsonEnabledWorkflowSteps(task.id)) ?? []).toEqual(["code-review"]); expect(privateStore().resolveTaskWorkflowIrSync(task.id)).toEqual(BUILTIN_CODING_WORKFLOW_IR); }); @@ -237,7 +256,7 @@ describe("workflow restart durability for explicit selections", () => { const customSelectionBefore = store().getTaskWorkflowSelection(customTask.id); expect(customSelectionBefore?.workflowId).toBe(workflow.id); - expect(customSelectionBefore?.stepIds).toHaveLength(2); + expect(customSelectionBefore?.stepIds).toEqual(["review-group"]); // FNXC:CodeReviewStep — builtin:coding carries the DEFAULT-ON `code-review` // optional-group, so the create-time workflowId path seeds it into the selection. expect(store().getTaskWorkflowSelection(builtinTask.id)).toEqual({ workflowId: "builtin:coding", stepIds: ["code-review"] }); @@ -248,9 +267,6 @@ describe("workflow restart durability for explicit selections", () => { expect(customSelection).toEqual(customSelectionBefore); expect((await store().getTask(customTask.id)).enabledWorkflowSteps).toEqual(customSelectionBefore?.stepIds); expect(await taskJsonEnabledWorkflowSteps(customTask.id)).toEqual(customSelectionBefore?.stepIds); - for (const stepId of customSelection?.stepIds ?? []) { - expect(await store().getWorkflowStep(stepId)).toBeDefined(); - } expect(store().getTaskWorkflowSelection(builtinTask.id)).toEqual({ workflowId: "builtin:coding", stepIds: ["code-review"] }); expect((await store().getTask(builtinTask.id)).enabledWorkflowSteps ?? []).toEqual(["code-review"]); expect((await taskJsonEnabledWorkflowSteps(builtinTask.id)) ?? []).toEqual(["code-review"]); diff --git a/packages/core/src/__tests__/workflow-selection-store.test.ts b/packages/core/src/__tests__/workflow-selection-store.test.ts index 898875d8da..e3b2124fc4 100644 --- a/packages/core/src/__tests__/workflow-selection-store.test.ts +++ b/packages/core/src/__tests__/workflow-selection-store.test.ts @@ -64,6 +64,50 @@ function branchingIr(): WorkflowIr { }; } +/* +FNXC:WorkflowStepCRUD 2026-06-26-14:00: +U7c dropped the `workflow_steps` table and the workflow-compilation materializer. +Selecting/inheriting a workflow no longer materializes per-step rows: the graph runs the +selected workflow's IR directly, and `task.enabledWorkflowSteps` / `selection.stepIds` now +hold ONLY the ids of default-on `optional-group` nodes (the executor toggle keys). A pure +v1 linear workflow has no optional-group nodes, so its selection seeds an EMPTY set. The +invariant `selection.stepIds === task.enabledWorkflowSteps` still holds. +*/ +/** v2 workflow whose success path threads through two optional-group nodes + * (og-on defaultOn:true, og-off defaultOn:false). */ +function optionalGroupIr(): WorkflowIr { + const groupTemplate = (id: string) => ({ + nodes: [{ id: `${id}-inner`, kind: "prompt" as const, config: { prompt: "x" } }], + edges: [], + }); + return { + version: "v2", + name: "og-wf", + columns: [{ id: "todo", name: "Todo", traits: [] }], + nodes: [ + { id: "start", kind: "start", column: "todo" }, + { + id: "og-on", + kind: "optional-group", + column: "todo", + config: { name: "On Group", defaultOn: true, template: groupTemplate("og-on") }, + }, + { + id: "og-off", + kind: "optional-group", + column: "todo", + config: { name: "Off Group", defaultOn: false, template: groupTemplate("og-off") }, + }, + { id: "end", kind: "end", column: "todo" }, + ], + edges: [ + { from: "start", to: "og-on", condition: "success" }, + { from: "og-on", to: "og-off", condition: "success" }, + { from: "og-off", to: "end", condition: "success" }, + ], + }; +} + describe("TaskStore workflow selection (U3)", () => { const harness = createTaskStoreTestHarness(); let store: ReturnType; @@ -77,52 +121,58 @@ describe("TaskStore workflow selection (U3)", () => { await harness.afterEach(); }); - it("selecting a workflow populates enabledWorkflowSteps and records selection", async () => { - const wf = await store.createWorkflowDefinition({ name: "QA", ir: linearIr() }); + it("selecting a workflow seeds enabledWorkflowSteps with default-on group ids and records selection", async () => { + const wf = await store.createWorkflowDefinition({ name: "QA", ir: optionalGroupIr() }); const task = await store.createTask({ description: "T", enabledWorkflowSteps: [] }); await store.selectTaskWorkflow(task.id, wf.id); const detail = await store.getTask(task.id); - expect(detail.enabledWorkflowSteps).toHaveLength(2); + // Only the defaultOn:true optional-group id is seeded (og-off is excluded). + expect(detail.enabledWorkflowSteps).toEqual(["og-on"]); const selection = store.getTaskWorkflowSelection(task.id); expect(selection?.workflowId).toBe(wf.id); expect(selection?.stepIds).toEqual(detail.enabledWorkflowSteps); }); - it("compiled steps are hidden from the user-facing step manager listing", async () => { + it("selecting a pure-linear workflow records the selection with an empty step set", async () => { const wf = await store.createWorkflowDefinition({ name: "QA", ir: linearIr() }); const task = await store.createTask({ description: "T", enabledWorkflowSteps: [] }); + await store.selectTaskWorkflow(task.id, wf.id); + const detail = await store.getTask(task.id); + // No optional-group nodes → no toggle ids; the graph runs the IR's nodes directly. + expect(detail.enabledWorkflowSteps ?? []).toEqual([]); + const selection = store.getTaskWorkflowSelection(task.id); + expect(selection?.workflowId).toBe(wf.id); + expect(selection?.stepIds).toEqual(detail.enabledWorkflowSteps ?? []); + // U7c: the legacy step manager listing is gone; the table-backed list is empty. expect(await store.listWorkflowSteps()).toHaveLength(0); - // …but the executor can still resolve them directly. - const selection = store.getTaskWorkflowSelection(task.id)!; - expect(await store.getWorkflowStep(selection.stepIds[0])).toBeDefined(); }); - it("re-selecting replaces prior compiled steps without accumulating orphans", async () => { - const wfA = await store.createWorkflowDefinition({ name: "A", ir: linearIr() }); + it("re-selecting replaces the prior selection's seeded group ids", async () => { + const wfA = await store.createWorkflowDefinition({ name: "A", ir: optionalGroupIr() }); const wfB = await store.createWorkflowDefinition({ name: "B", ir: linearIr() }); const task = await store.createTask({ description: "T", enabledWorkflowSteps: [] }); await store.selectTaskWorkflow(task.id, wfA.id); - const firstIds = store.getTaskWorkflowSelection(task.id)!.stepIds; + expect(store.getTaskWorkflowSelection(task.id)!.stepIds).toEqual(["og-on"]); + await store.selectTaskWorkflow(task.id, wfB.id); const secondIds = store.getTaskWorkflowSelection(task.id)!.stepIds; - - // Old steps are gone, only the new selection's steps remain. - for (const id of firstIds) { - expect(await store.getWorkflowStep(id)).toBeUndefined(); - } + // The prior selection's group ids are replaced wholesale by the new workflow's. + expect(secondIds).toEqual([]); const detail = await store.getTask(task.id); - expect(detail.enabledWorkflowSteps).toEqual(secondIds); + expect(detail.enabledWorkflowSteps ?? []).toEqual(secondIds); + expect(store.getTaskWorkflowSelection(task.id)!.workflowId).toBe(wfB.id); }); it("clearing selection empties enabledWorkflowSteps", async () => { - const wf = await store.createWorkflowDefinition({ name: "QA", ir: linearIr() }); + const wf = await store.createWorkflowDefinition({ name: "QA", ir: optionalGroupIr() }); const task = await store.createTask({ description: "T", enabledWorkflowSteps: [] }); await store.selectTaskWorkflow(task.id, wf.id); + expect((await store.getTask(task.id)).enabledWorkflowSteps).toEqual(["og-on"]); await store.clearTaskWorkflowSelection(task.id); const detail = await store.getTask(task.id); @@ -141,14 +191,14 @@ describe("TaskStore workflow selection (U3)", () => { }); it("force-resurrecting over a tombstoned task purges its prior workflow selection", async () => { - const wf = await store.createWorkflowDefinition({ name: "QA", ir: linearIr() }); + const wf = await store.createWorkflowDefinition({ name: "QA", ir: optionalGroupIr() }); const task = await store.createTask({ description: "T", enabledWorkflowSteps: [] }); await store.selectTaskWorkflow(task.id, wf.id); - const priorIds = store.getTaskWorkflowSelection(task.id)!.stepIds; - expect(priorIds).toHaveLength(2); + expect(store.getTaskWorkflowSelection(task.id)!.stepIds).toEqual(["og-on"]); // Soft-delete then physically resurrect the same id; the physical purge of - // the old tasks row must drop the orphaned selection + its compiled steps. + // the old tasks row must drop the orphaned selection row (U7c: no compiled + // step rows to reclaim — selection ids are optional-group node ids). await store.deleteTask(task.id); await store.createTaskWithReservedId( { description: "resurrected", enabledWorkflowSteps: [], forceResurrect: true }, @@ -156,9 +206,6 @@ describe("TaskStore workflow selection (U3)", () => { ); expect(store.getTaskWorkflowSelection(task.id)).toBeUndefined(); - for (const id of priorIds) { - expect(await store.getWorkflowStep(id)).toBeUndefined(); - } }); it("throws when selecting an unknown workflow", async () => { @@ -167,53 +214,21 @@ describe("TaskStore workflow selection (U3)", () => { }); it("new tasks inherit the project default workflow", async () => { - const wf = await store.createWorkflowDefinition({ name: "Default", ir: linearIr() }); + const wf = await store.createWorkflowDefinition({ name: "Default", ir: optionalGroupIr() }); await store.setDefaultWorkflowId(wf.id); const task = await store.createTask({ description: "inherits" }); const detail = await store.getTask(task.id); - expect(detail.enabledWorkflowSteps).toHaveLength(2); + // Inherits the default workflow's default-on optional-group seed. + expect(detail.enabledWorkflowSteps).toEqual(["og-on"]); expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe(wf.id); }); // FNXC:WorkflowOptionalGroup 2026-06-21-14:30: a new task seeds // `enabledWorkflowSteps` with exactly the defaultOn:true optional-group ids of - // its selected workflow (U3, R3), alongside the compiled workflow step ids. + // its selected workflow (U3, R3). (U7c: these are the ONLY seeded ids — there + // are no compiled workflow step ids anymore.) describe("optional-group defaultOn seeding (U3/R3)", () => { - /** v2 workflow whose success path threads through two optional-group nodes. */ - function optionalGroupIr(): WorkflowIr { - const groupTemplate = (id: string) => ({ - nodes: [{ id: `${id}-inner`, kind: "prompt" as const, config: { prompt: "x" } }], - edges: [], - }); - return { - version: "v2", - name: "og-wf", - columns: [{ id: "todo", name: "Todo", traits: [] }], - nodes: [ - { id: "start", kind: "start", column: "todo" }, - { - id: "og-on", - kind: "optional-group", - column: "todo", - config: { name: "On Group", defaultOn: true, template: groupTemplate("og-on") }, - }, - { - id: "og-off", - kind: "optional-group", - column: "todo", - config: { name: "Off Group", defaultOn: false, template: groupTemplate("og-off") }, - }, - { id: "end", kind: "end", column: "todo" }, - ], - edges: [ - { from: "start", to: "og-on", condition: "success" }, - { from: "og-on", to: "og-off", condition: "success" }, - { from: "og-off", to: "end", condition: "success" }, - ], - }; - } - it("seeds the defaultOn:true group id at creation from the default workflow", async () => { const wf = await store.createWorkflowDefinition({ name: "OG Default", ir: optionalGroupIr() }); await store.setDefaultWorkflowId(wf.id); @@ -321,14 +336,14 @@ describe("TaskStore workflow selection (U3)", () => { // U6/R3/KTD-4: create-time `workflowId` materializes the selection atomically. describe("create-time workflowId (U6/R3)", () => { - it("materializes enabledWorkflowSteps atomically when workflowId is given", async () => { - const wf = await store.createWorkflowDefinition({ name: "Pick", ir: linearIr() }); + it("seeds enabledWorkflowSteps atomically when workflowId is given", async () => { + const wf = await store.createWorkflowDefinition({ name: "Pick", ir: optionalGroupIr() }); const task = await store.createTask({ description: "with workflow", workflowId: wf.id }); - // Reading the task right after create observes the populated steps — no + // Reading the task right after create observes the populated group seed — no // intermediate empty state visible to the executor. const detail = await store.getTask(task.id); - expect(detail.enabledWorkflowSteps).toHaveLength(2); + expect(detail.enabledWorkflowSteps).toEqual(["og-on"]); expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe(wf.id); expect(store.getTaskWorkflowSelection(task.id)?.stepIds).toEqual(detail.enabledWorkflowSteps); }); @@ -353,12 +368,12 @@ describe("TaskStore workflow selection (U3)", () => { }); it("undefined workflowId still inherits the project default (unchanged)", async () => { - const def = await store.createWorkflowDefinition({ name: "Default", ir: linearIr() }); + const def = await store.createWorkflowDefinition({ name: "Default", ir: optionalGroupIr() }); await store.setDefaultWorkflowId(def.id); const task = await store.createTask({ description: "inherit" }); const detail = await store.getTask(task.id); - expect(detail.enabledWorkflowSteps).toHaveLength(2); + expect(detail.enabledWorkflowSteps).toEqual(["og-on"]); expect(store.getTaskWorkflowSelection(task.id)?.workflowId).toBe(def.id); }); diff --git a/packages/core/src/__tests__/workflow-step-migration.test.ts b/packages/core/src/__tests__/workflow-step-migration.test.ts deleted file mode 100644 index b45b6b6a18..0000000000 --- a/packages/core/src/__tests__/workflow-step-migration.test.ts +++ /dev/null @@ -1,197 +0,0 @@ -import { describe, it, expect, beforeEach, afterEach, beforeAll, afterAll } from "vitest"; - -import { isBuiltinWorkflowId } from "../builtin-workflows.js"; -import { createSharedTaskStoreTestHarness } from "./store-test-helpers.js"; - -/** - * U2 / R5 / KTD-3 — lazy idempotent migration of legacy user-authored workflow - * steps into the dual fragment + combined-workflow representation. - */ -describe("TaskStore.migrateLegacyWorkflowSteps (U2/R5)", () => { - const harness = createSharedTaskStoreTestHarness(); - - beforeAll(harness.beforeAll); - afterAll(harness.afterAll); - let store: ReturnType; - - beforeEach(async () => { - await harness.beforeEach(); - store = harness.store(); - }); - - afterEach(async () => { - await harness.afterEach(); - }); - - /** User-owned (non-builtin) workflow definitions only. */ - async function userDefs() { - return (await store.listWorkflowDefinitions()).filter((w) => !isBuiltinWorkflowId(w.id)); - } - - it("converts defaultOn + optional + disabled user steps to fragments, builds the combined workflow from defaultOn only, sets the project default, and leaves the compiled row untouched", async () => { - // defaultOn (ran automatically on new tasks) → fragment + joins combined workflow. - const on = await store.createWorkflowStep({ - name: "Default On", - description: "ran by default", - prompt: "do the default thing", - defaultOn: true, - enabled: true, - }); - // enabled-but-optional → fragment only (NOT in combined workflow). - const optional = await store.createWorkflowStep({ - name: "Optional", - description: "opt-in", - prompt: "optional work", - defaultOn: false, - enabled: true, - }); - // disabled → still gets a fragment (every user step does). - const disabled = await store.createWorkflowStep({ - name: "Disabled", - description: "off", - prompt: "disabled work", - defaultOn: false, - enabled: false, - }); - // compiled-materialized row (execution detail) → must be ignored entirely. - const compiled = await store.createWorkflowStep({ - name: "Compiled", - description: "materialized", - templateId: "workflow:WF-999", - defaultOn: true, - enabled: true, - }); - - const result = await store.migrateLegacyWorkflowSteps(); - - // 3 user steps converted; nothing previously migrated. - expect(result.migrated).toBe(3); - expect(result.skipped).toBe(0); - expect(result.combinedWorkflowId).toBeTruthy(); - - const defs = await userDefs(); - const fragments = defs.filter((d) => d.kind === "fragment"); - const workflows = defs.filter((d) => d.kind === "workflow"); - - // Exactly 3 fragments (one per user step), exactly 1 combined workflow. - expect(fragments).toHaveLength(3); - expect(workflows).toHaveLength(1); - expect(fragments.map((f) => f.name).sort()).toEqual(["Default On", "Disabled", "Optional"]); - - // Combined workflow: named "Migrated steps", carries the system description, - // and contains ONLY the defaultOn step's user node (plus start/end + seams). - const combined = workflows[0]; - expect(combined.id).toBe(result.combinedWorkflowId); - expect(combined.name).toBe("Migrated steps"); - expect(combined.description).toBe("Converted from your legacy workflow steps"); - const userNodes = combined.ir.nodes.filter( - (n) => n.kind !== "start" && n.kind !== "end" && typeof n.config?.seam !== "string", - ); - expect(userNodes).toHaveLength(1); - expect(userNodes[0].config?.name).toBe("Default On"); - - // Project default points at the combined workflow. - expect(await store.getDefaultWorkflowId()).toBe(combined.id); - - // All 3 user source rows are stamped; the compiled row is untouched. - expect((await store.getWorkflowStep(on.id))?.migratedFragmentId).toBeTruthy(); - expect((await store.getWorkflowStep(optional.id))?.migratedFragmentId).toBeTruthy(); - expect((await store.getWorkflowStep(disabled.id))?.migratedFragmentId).toBeTruthy(); - expect((await store.getWorkflowStep(compiled.id))?.migratedFragmentId).toBeUndefined(); - - // No source records were deleted. - const steps = await store.listWorkflowSteps(); - expect(steps.map((s) => s.id)).toEqual(expect.arrayContaining([on.id, optional.id, disabled.id])); - }); - - it("creates fragments but NO combined workflow and leaves the default unchanged when no step is defaultOn", async () => { - await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: false }); - await store.createWorkflowStep({ name: "B", description: "b", prompt: "b", enabled: false }); - - const result = await store.migrateLegacyWorkflowSteps(); - - expect(result.migrated).toBe(2); - expect(result.combinedWorkflowId).toBeUndefined(); - - const defs = await userDefs(); - expect(defs.filter((d) => d.kind === "fragment")).toHaveLength(2); - expect(defs.filter((d) => d.kind === "workflow")).toHaveLength(0); - expect(await store.getDefaultWorkflowId()).toBeUndefined(); - }); - - it("is idempotent: a second run converts nothing and creates no new definitions", async () => { - await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: true }); - - const first = await store.migrateLegacyWorkflowSteps(); - expect(first.migrated).toBe(1); - const afterFirst = (await userDefs()).length; - - const second = await store.migrateLegacyWorkflowSteps(); - expect(second.migrated).toBe(0); - expect(second.skipped).toBe(1); - expect(second.combinedWorkflowId).toBeUndefined(); - expect((await userDefs()).length).toBe(afterFirst); - }); - - it("does not clobber a pre-existing project default", async () => { - // A user-chosen default workflow exists before migration. - const existing = await store.createWorkflowDefinition({ - name: "My choice", - ir: { - version: "v1", - name: "My choice", - nodes: [ - { id: "start", kind: "start" }, - { id: "end", kind: "end" }, - ], - edges: [{ from: "start", to: "end", condition: "success" }], - }, - kind: "workflow", - }); - await store.setDefaultWorkflowId(existing.id); - - await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: true }); - const result = await store.migrateLegacyWorkflowSteps(); - - // The combined workflow is still created, but the explicit default is kept. - expect(result.combinedWorkflowId).toBeTruthy(); - expect(await store.getDefaultWorkflowId()).toBe(existing.id); - }); - - it("compare-and-set: re-reads the default after the transaction and skips when a concurrent writer set one", async () => { - await store.createWorkflowStep({ name: "A", description: "a", prompt: "a", defaultOn: true }); - - const concurrent = await store.createWorkflowDefinition({ - name: "Concurrent", - ir: { - version: "v1", - name: "Concurrent", - nodes: [ - { id: "start", kind: "start" }, - { id: "end", kind: "end" }, - ], - edges: [{ from: "start", to: "end", condition: "success" }], - }, - kind: "workflow", - }); - - // A project default exists when migration's post-transaction compare-and-set - // re-reads it. Because the set is gated on the re-read (not a pre-transaction - // snapshot), an existing default is observed and never clobbered. - await store.setDefaultWorkflowId(concurrent.id); - - const result = await store.migrateLegacyWorkflowSteps(); - - expect(result.combinedWorkflowId).toBeTruthy(); - expect(result.combinedWorkflowId).not.toBe(concurrent.id); - // The compare-and-set re-read observed the existing default and did NOT clobber it. - expect(await store.getDefaultWorkflowId()).toBe(concurrent.id); - }); - - it("is a no-op with zero user steps", async () => { - const result = await store.migrateLegacyWorkflowSteps(); - expect(result).toEqual({ migrated: 0, skipped: 0, combinedWorkflowId: undefined }); - expect(await userDefs()).toHaveLength(0); - expect(await store.getDefaultWorkflowId()).toBeUndefined(); - }); -}); diff --git a/packages/core/src/__tests__/workflow-steps-table-drop-migration.test.ts b/packages/core/src/__tests__/workflow-steps-table-drop-migration.test.ts new file mode 100644 index 0000000000..47c16092e2 --- /dev/null +++ b/packages/core/src/__tests__/workflow-steps-table-drop-migration.test.ts @@ -0,0 +1,88 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { SCHEMA_VERSION } from "../db.js"; +import { CODE_REVIEW_GROUP_ID } from "../builtin-code-review-group.js"; +import { createTaskStoreTestHarness } from "./store-test-helpers.js"; + +/* +FNXC:WorkflowStepCRUD 2026-06-26-14:00: +U7c cutover (migration 131) DROPs the legacy `workflow_steps` table. Pre/post-merge +workflow steps run graph-native and record into task.workflowStepResults; nothing reads +`workflow_steps` rows at runtime. Migration 130 already normalized legacy compiled-step +enable ids (WS-xxx) in tasks.enabledWorkflowSteps to their built-in optional-group node ids, +so the table holds nothing read at runtime by the time 131 drops it. + +This seed-at-130 test seeds a DB at exactly schemaVersion 130 (the version right before the +cutover) with a populated `workflow_steps` table AND a task whose enabledWorkflowSteps already +holds the normalized graph node id (`code-review`). It then opens the store (replaying ONLY +migration 131) and asserts: + (a) the legacy table is gone — querying it throws and it is absent from sqlite_master; and + (b) the task is intact and still resolves/runs its graph optional-group — its normalized + enable id survives and its workflow selection resolves. +*/ + +describe("Migration 131: drop the legacy workflow_steps table (U7c cutover)", () => { + const harness = createTaskStoreTestHarness(); + + beforeEach(async () => { + await harness.beforeEach(); + }); + + afterEach(async () => { + await harness.afterEach(); + }); + + it("drops a populated workflow_steps table at v130→131 and leaves the task's normalized graph enable id intact", async () => { + await harness.reopenDiskBackedStore(); + const store = harness.store(); + const task = await harness.createTestTask(); + const db = store.getDatabase(); + + // Seed a realistic legacy `workflow_steps` table (as a real <131 DB would carry) with a + // row, then a task whose enabledWorkflowSteps already holds the normalized node id. + db.prepare( + `CREATE TABLE IF NOT EXISTS workflow_steps ( + id TEXT PRIMARY KEY, templateId TEXT, name TEXT NOT NULL, description TEXT NOT NULL, + mode TEXT NOT NULL DEFAULT 'prompt', phase TEXT NOT NULL DEFAULT 'pre-merge', + prompt TEXT NOT NULL DEFAULT '', gateMode TEXT NOT NULL DEFAULT 'advisory', + toolMode TEXT, scriptName TEXT, enabled INTEGER NOT NULL DEFAULT 1, defaultOn INTEGER DEFAULT 0, + modelProvider TEXT, modelId TEXT, migrated_fragment_id TEXT, + createdAt TEXT NOT NULL, updatedAt TEXT NOT NULL + )`, + ).run(); + const now = new Date().toISOString(); + db.prepare( + `INSERT OR REPLACE INTO workflow_steps + (id, templateId, name, description, mode, phase, prompt, gateMode, enabled, defaultOn, createdAt, updatedAt) + VALUES ('WS-001', ?, 'Code Review', 'desc', 'prompt', 'pre-merge', 'x', 'advisory', 1, 1, ?, ?)`, + ).run(CODE_REVIEW_GROUP_ID, now, now); + + db.prepare("UPDATE tasks SET enabledWorkflowSteps = ? WHERE id = ?").run( + JSON.stringify([CODE_REVIEW_GROUP_ID]), + task.id, + ); + + // Stamp the DB at v130 (right before the cutover) so opening it replays ONLY migration 131. + db.prepare("UPDATE __meta SET value = '130' WHERE key = 'schemaVersion'").run(); + + await harness.reopenDiskBackedStore(); + const migratedStore = harness.store(); + const migratedDb = migratedStore.getDatabase(); + + // (a) The cutover dropped the table. + expect(migratedDb.getSchemaVersion()).toBe(SCHEMA_VERSION); + expect( + migratedDb + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'workflow_steps'") + .get(), + ).toBeUndefined(); + // Querying the dropped table now throws (nothing is stranded reading it). + expect(() => migratedDb.prepare("SELECT 1 FROM workflow_steps").get()).toThrow(); + + // (b) The task survives and still resolves/runs its graph optional-group: its normalized + // enable id is intact, so the executor's `enabledWorkflowSteps.includes(node.id)` toggle + // still enables the `code-review` optional-group node. + const migratedTask = await migratedStore.getTask(task.id); + expect(migratedTask.enabledWorkflowSteps).toEqual([CODE_REVIEW_GROUP_ID]); + }); +}); diff --git a/packages/core/src/db-migrate.ts b/packages/core/src/db-migrate.ts index 6322a87e22..c04411938f 100644 --- a/packages/core/src/db-migrate.ts +++ b/packages/core/src/db-migrate.ts @@ -151,56 +151,10 @@ async function migrateConfig(fusionDir: string, db: Database): Promise { new Date().toISOString(), ); - const insertWorkflowStep = db.prepare(` - INSERT OR IGNORE INTO workflow_steps ( - id, - templateId, - name, - description, - mode, - phase, - prompt, - gateMode, - toolMode, - scriptName, - enabled, - defaultOn, - modelProvider, - modelId, - createdAt, - updatedAt - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `); - - for (const step of workflowSteps) { - if (!step?.id || !step.name || !step.description) { - continue; - } - - const mode = step.mode === "script" ? "script" : "prompt"; - const phase = step.phase === "post-merge" ? "post-merge" : "pre-merge"; - const createdAt = step.createdAt || new Date().toISOString(); - const updatedAt = step.updatedAt || createdAt; - - insertWorkflowStep.run( - step.id, - step.templateId ?? null, - step.name, - step.description, - mode, - phase, - mode === "prompt" ? step.prompt || "" : "", - step.gateMode ?? "advisory", - mode === "prompt" ? step.toolMode ?? null : null, - mode === "script" ? step.scriptName ?? null : null, - step.enabled === false ? 0 : 1, - step.defaultOn === undefined ? null : step.defaultOn ? 1 : 0, - mode === "prompt" ? step.modelProvider ?? null : null, - mode === "prompt" ? step.modelId ?? null : null, - createdAt, - updatedAt, - ); - } + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c dropped the `workflow_steps` table. + // Legacy `config.json` workflow steps are preserved in the `config.workflowSteps` JSON + // column above for archival/diagnostic reference, but are NOT imported as table rows — + // workflow steps run graph-native and the table no longer exists in the schema. db.bumpLastModified(); console.log("[migrate] Migrated config.json"); diff --git a/packages/core/src/db.ts b/packages/core/src/db.ts index c5ce148775..44ee3814e4 100644 --- a/packages/core/src/db.ts +++ b/packages/core/src/db.ts @@ -167,7 +167,7 @@ export function isFts5CorruptionError(error: unknown): boolean { // ── Schema Definition ──────────────────────────────────────────────── -const SCHEMA_VERSION = 130; +const SCHEMA_VERSION = 131; const TASKS_FTS_AUTOMERGE = 8; const TASKS_FTS_CRISISMERGE = 16; @@ -401,29 +401,10 @@ CREATE TABLE IF NOT EXISTS distributed_task_id_reservations ( CREATE INDEX IF NOT EXISTS idxDistributedTaskIdReservationsPrefixStatus ON distributed_task_id_reservations(prefix, status); CREATE INDEX IF NOT EXISTS idxDistributedTaskIdReservationsExpiry ON distributed_task_id_reservations(status, expiresAt); --- Workflow step definitions -CREATE TABLE IF NOT EXISTS workflow_steps ( - id TEXT PRIMARY KEY, - templateId TEXT, - name TEXT NOT NULL, - description TEXT NOT NULL, - mode TEXT NOT NULL DEFAULT 'prompt', - phase TEXT NOT NULL DEFAULT 'pre-merge', - prompt TEXT NOT NULL DEFAULT '', - gateMode TEXT NOT NULL DEFAULT 'advisory', - toolMode TEXT, - scriptName TEXT, - enabled INTEGER NOT NULL DEFAULT 1, - defaultOn INTEGER DEFAULT 0, - modelProvider TEXT, - modelId TEXT, - -- (workflow-editor-consolidation U1/U2) when this step has been migrated into a - -- fragment WorkflowDefinition, the fragment's id is stamped here so re-runs of - -- the lazy migration skip already-migrated rows (marker idempotency). - migrated_fragment_id TEXT, - createdAt TEXT NOT NULL, - updatedAt TEXT NOT NULL -); +-- FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c dropped the legacy workflow_steps table. +-- Pre-merge and post-merge workflow steps run graph-native (recorded into +-- task.workflowStepResults); nothing reads workflow_steps rows at runtime. Migration 131 +-- drops the table for upgrading DBs; fresh DBs never create it. See migration 131 below. -- Named workflow definitions authored as WorkflowIr graphs (+ editor layout). -- The ir and layout columns are JSON-encoded TEXT; ir is validated via @@ -4374,6 +4355,10 @@ export class Database { if (version < 77) { this.applyMigration(77, () => { + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c removed `workflow_steps` from SCHEMA_SQL, + // so a DB stamped below this migration can legitimately lack the table — guard the + // column add / backfill (nothing to alter when the table was never created). + if (!this.tableExists("workflow_steps")) return; this.addColumnIfMissing("workflow_steps", "gateMode", "TEXT NOT NULL DEFAULT 'advisory'"); // FN-4368: advisory-by-default for all legacy workflow_steps rows; users opt in to 'gate' via UI. this.db.exec("UPDATE workflow_steps SET gateMode = 'advisory'"); @@ -4790,15 +4775,22 @@ export class Database { // Delete the compiled steps referenced by orphaned selections first, then // the orphaned selection rows themselves. json_each expands the stepIds // JSON array; the WHERE guards against malformed (non-array) stepIds. + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c removed `workflow_steps` from + // SCHEMA_SQL — guard the compiled-step delete when the table is absent; the + // orphaned-selection cleanup still runs (that table always exists). + if (this.tableExists("workflow_steps")) { + this.db.exec(` + DELETE FROM workflow_steps WHERE id IN ( + SELECT je.value + FROM task_workflow_selection sel + JOIN json_each(sel.stepIds) je + WHERE json_valid(sel.stepIds) + AND json_type(sel.stepIds) = 'array' + AND sel.taskId NOT IN (SELECT id FROM tasks) + ); + `); + } this.db.exec(` - DELETE FROM workflow_steps WHERE id IN ( - SELECT je.value - FROM task_workflow_selection sel - JOIN json_each(sel.stepIds) je - WHERE json_valid(sel.stepIds) - AND json_type(sel.stepIds) = 'array' - AND sel.taskId NOT IN (SELECT id FROM tasks) - ); DELETE FROM task_workflow_selection WHERE taskId NOT IN (SELECT id FROM tasks); `); @@ -4880,7 +4872,11 @@ export class Database { if (version < 109) { this.applyMigration(109, () => { this.addColumnIfMissing("workflows", "kind", "TEXT NOT NULL DEFAULT 'workflow'"); - this.addColumnIfMissing("workflow_steps", "migrated_fragment_id", "TEXT"); + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c removed `workflow_steps` from SCHEMA_SQL; + // guard the column add when the table is absent on a table-less seeded DB. + if (this.tableExists("workflow_steps")) { + this.addColumnIfMissing("workflow_steps", "migrated_fragment_id", "TEXT"); + } }); } @@ -5357,6 +5353,11 @@ export class Database { // table is intentionally KEPT (dropped in U7c once all readers are gone). if (version < 130) { this.applyMigration(130, () => { + // FNXC:WorkflowPostMerge 2026-06-26-14:00: U7c removed `workflow_steps` from + // SCHEMA_SQL, so a DB stamped between the table's creation migration and 130 can + // legitimately lack the table (nothing to normalize). Guard the SELECT — absence + // means no legacy compiled-step ids to rewrite, so this migration is a no-op. + if (!this.tableExists("workflow_steps")) return; const optionalGroupNodeIds = new Set([ BROWSER_VERIFICATION_GROUP_ID, CODE_REVIEW_GROUP_ID, @@ -5415,6 +5416,21 @@ export class Database { }); } + // Migration 131: drop the legacy `workflow_steps` table (U7c). + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: + // Pre-merge and post-merge workflow steps run graph-native and record into + // `task.workflowStepResults`. Migration 130 already normalized legacy compiled-step + // enable ids (WS-xxx) in `tasks.enabledWorkflowSteps` to their built-in optional-group + // node ids, so the table holds nothing read at runtime. All store CRUD, the + // workflow-compilation materializer, the merger post-merge path, and the executor + // recovery table read have been removed. Drop the table. Idempotent + // (`DROP TABLE IF EXISTS`); a fresh DB never created it (removed from SCHEMA_SQL). + if (version < 131) { + this.applyMigration(131, () => { + this.db.exec("DROP TABLE IF EXISTS workflow_steps"); + }); + } + } /** diff --git a/packages/core/src/experimental-features.ts b/packages/core/src/experimental-features.ts index ca730591fe..6432afda1d 100644 --- a/packages/core/src/experimental-features.ts +++ b/packages/core/src/experimental-features.ts @@ -12,15 +12,15 @@ FNXC:WorkflowSettings 2026-06-23-21:55: workflowInterpreterDualObserve is no longer user-controllable in Settings. Treat stale persisted true values as inert so upgraded users do not keep running hidden diagnostic shadow observation with no visible off switch. */ /* -FNXC:WorkflowPostMerge 2026-06-26-12:00: -U7b cutover — `graphNativePostMerge` is now DEFAULT-ON. The graph is the single owner -of post-merge execution: a successful merge lets traversal continue to post-merge graph -nodes (optional-group nodes wired off a merge-region success, plus the plain post-merge -nodes that follow a `seam:"merge"` prompt node — e.g. compound-engineering's `document` -step). When this flag is on the legacy merger post-merge path (`runPostMergeWorkflowSteps` -/ `hasEnabledPostMergeWorkflowSteps` in engine/merger.ts) is INERT so post-merge work runs -exactly once via the graph and never double-runs. The flag is retained (not removed) as an -explicit opt-out back to the legacy merger path until U7c deletes the legacy code + table. +FNXC:WorkflowPostMerge 2026-06-26-14:00: +U7b/U7c cutover — `graphNativePostMerge` is DEFAULT-ON and the graph is the SOLE owner of +post-merge execution: a successful merge lets traversal continue to post-merge graph nodes +(optional-group nodes wired off a merge-region success, plus the plain post-merge nodes that +follow a `seam:"merge"` prompt node — e.g. compound-engineering's `document` step). U7c +DELETED the legacy merger post-merge execution path entirely (`runPostMergeWorkflowSteps` / +`hasEnabledPostMergeWorkflowSteps` and the worktree/prompt/script helpers are gone), so there +is no legacy fallback: post-merge work runs exactly once via the graph. The flag still gates +the graph's post-merge nodes (workflow-graph-executor.ts) but no longer toggles a merger path. */ const DEFAULT_ON_EXPERIMENTAL_FEATURES = new Set(["graphNativePostMerge"]); const RETIRED_EXPERIMENTAL_FEATURES = new Set([ diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index 436c344238..d783bf92fb 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -16,7 +16,6 @@ import { patchContainsMovedKey, } from "./moved-settings.js"; import { parseWorkflowIr, serializeWorkflowIr, downgradeIrToV1IfPure } from "./workflow-ir.js"; -import { stepsToWorkflowIr, stepToFragmentIr, layoutForIr } from "./workflow-steps-to-ir.js"; import { resolveAllowedColumns, workflowHasColumn } from "./workflow-transitions.js"; import { extractEffectiveWriteScopeFromPrompt, extractFileScopeTokens, isValidFileScopeEntry } from "./file-scope-classification.js"; @@ -137,9 +136,6 @@ import { type WorkflowColumnsGraduationReport, } from "./workflow-parity.js"; -/** Tags WorkflowStep rows materialized by compiling a workflow so they can be - * filtered out of the user-facing step manager and cleaned up on re-selection. */ -const WORKFLOW_COMPILED_STEP_TEMPLATE_PREFIX = "workflow:"; import { resolveWorktrunkSettings, validateWorktrunkSettings } from "./worktrunk-settings.js"; import { validateLocale } from "./settings-validation.js"; import { normalizeTaskPriority } from "./task-priority.js"; @@ -4217,97 +4213,16 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} return `${Date.now()}-${id}-${sanitized}`; } - private toStoredWorkflowStep(row: { - id: string; - templateId: string | null; - name: string; - description: string; - mode: string; - phase: string | null; - gateMode: string | null; - prompt: string; - toolMode: string | null; - scriptName: string | null; - enabled: number; - defaultOn: number | null; - modelProvider: string | null; - modelId: string | null; - migrated_fragment_id?: string | null; - createdAt: string; - updatedAt: string; - }): import("./types.js").WorkflowStep { - return { - id: row.id, - templateId: row.templateId ?? undefined, - name: row.name, - description: row.description, - mode: row.mode === "script" ? "script" : "prompt", - phase: row.phase === "post-merge" ? "post-merge" : "pre-merge", - gateMode: row.gateMode === "advisory" || row.gateMode === "gate" - ? row.gateMode - : "advisory", - prompt: row.prompt || "", - toolMode: row.toolMode === "coding" || row.toolMode === "readonly" ? row.toolMode : undefined, - scriptName: row.scriptName ?? undefined, - enabled: Boolean(row.enabled), - defaultOn: row.defaultOn === null || row.defaultOn === undefined ? undefined : Boolean(row.defaultOn), - modelProvider: row.modelProvider ?? undefined, - modelId: row.modelId ?? undefined, - migratedFragmentId: row.migrated_fragment_id ?? undefined, - createdAt: row.createdAt, - updatedAt: row.updatedAt, - }; - } - - private getLegacyWorkflowStepSnapshot(id: string, templateId?: string): Record | undefined { - const row = this.db - .prepare("SELECT workflowSteps FROM config WHERE id = 1") - .get() as { workflowSteps?: string | null } | undefined; - const legacySteps = fromJson>>(row?.workflowSteps); - if (!Array.isArray(legacySteps)) { - return undefined; - } - - return legacySteps.find((legacy) => { - if (!legacy || typeof legacy !== "object") return false; - if (legacy.id === id) return true; - return Boolean(templateId && legacy.templateId === templateId); - }); - } - - private applyLegacyWorkflowStepOverrides(step: import("./types.js").WorkflowStep): import("./types.js").WorkflowStep { - const legacy = this.getLegacyWorkflowStepSnapshot(step.id, step.templateId); - if (!legacy) { - return step; - } - - const normalized = { ...step }; - if (!Object.prototype.hasOwnProperty.call(legacy, "mode")) { - normalized.mode = "prompt"; - } - if (!Object.prototype.hasOwnProperty.call(legacy, "phase")) { - normalized.phase = undefined; - } - if (!Object.prototype.hasOwnProperty.call(legacy, "gateMode")) { - normalized.gateMode = "advisory"; - } - - return normalized; - } - /* - FNXC:WorkflowOptionalGroup 2026-06-25-00:00: - U6 deleted the built-in step-template catalog and its template - materializer (`getBuiltInWorkflowTemplate`/`ensureWorkflowStepForTemplate`/ - `toBuiltInWorkflowStep`). `resolveEnabledWorkflowSteps` is now a pure pass-through: - enable ids are trimmed + de-duplicated but otherwise pass through UNCHANGED, keeping - them identity-stable (KTD-6). There is no longer any built-in template to materialize - into a `WS-xxx` row, so the prior `optionalGroupIdSet` collision guard (which kept - built-in group ids out of materialization) is no longer needed and was removed — a - group id like "browser-verification" now passes straight through, exactly matching the + FNXC:WorkflowOptionalGroup 2026-06-26-14:00: + U6 deleted the built-in step-template catalog and its template materializer; U7c dropped + the `workflow_steps` table and its compiled-step materializer entirely. + `resolveEnabledWorkflowSteps` is a pure pass-through: enable ids are trimmed + + de-duplicated but otherwise pass through UNCHANGED, keeping them identity-stable (KTD-6). + A group id like "browser-verification" passes straight through, matching the optional-group node id the executor toggles on `enabledWorkflowSteps.includes(node.id)`. - Plugin (`plugin:`-prefixed) ids also pass through. Workflow-compiled step rows are still - materialized separately via `materializeWorkflowSteps` (unchanged). + Plugin (`plugin:`-prefixed) ids also pass through. There are no longer any materialized + `workflow_steps` rows. */ private async resolveEnabledWorkflowSteps( stepIds?: string[], @@ -4516,26 +4431,20 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} resolvedWorkflowSteps = undefined; } - let task: Task; - try { - task = await this.createTaskWithDistributedReservation(input, { - createTaskWithId: async (taskId) => { - await this.assertNoDependencyCycle(taskId, input.dependencies ?? [], "createTask"); - return this._createTaskInternal( - input, - title, - resolvedWorkflowSteps, - taskId, - { invokeTaskCreatedHook: shouldInvokeTaskCreatedHook && !hasPendingSummarization }, - ); - }, - }); - } catch (err) { - // The task row was never created, so any default-workflow steps we - // materialized above would orphan with no task/selection pointing at them. - this.cleanupOrphanedMaterializedSteps(pendingWorkflowSelection?.stepIds); - throw err; - } + // U7c: selection seeds are optional-group node ids (not materialized + // `workflow_steps` rows), so a failed task creation strands nothing to clean. + const task: Task = await this.createTaskWithDistributedReservation(input, { + createTaskWithId: async (taskId) => { + await this.assertNoDependencyCycle(taskId, input.dependencies ?? [], "createTask"); + return this._createTaskInternal( + input, + title, + resolvedWorkflowSteps, + taskId, + { invokeTaskCreatedHook: shouldInvokeTaskCreatedHook && !hasPendingSummarization }, + ); + }, + }); // Record the inherited workflow selection now that the task row exists. if (pendingWorkflowSelection) { @@ -4711,20 +4620,14 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} resolvedWorkflowSteps = undefined; } - let createdTask: Task; - try { - createdTask = await this._createTaskInternal(input, title, resolvedWorkflowSteps, id, { - createdAt: options.createdAt, - updatedAt: options.updatedAt, - promptOverride: options.prompt, - invokeTaskCreatedHook: options.invokeTaskCreatedHook, - }); - } catch (err) { - // The task row was never created, so any default-workflow steps we - // materialized above would orphan with no task/selection pointing at them. - this.cleanupOrphanedMaterializedSteps(pendingWorkflowSelection?.stepIds); - throw err; - } + // U7c: selection seeds are optional-group node ids (not materialized + // `workflow_steps` rows), so a failed task creation strands nothing to clean. + const createdTask: Task = await this._createTaskInternal(input, title, resolvedWorkflowSteps, id, { + createdAt: options.createdAt, + updatedAt: options.updatedAt, + promptOverride: options.prompt, + invokeTaskCreatedHook: options.invokeTaskCreatedHook, + }); // Record the inherited workflow selection now that the task row exists. if (pendingWorkflowSelection) { @@ -14436,96 +14339,7 @@ ${deps} ${stepsSection}`; } - // ── Workflow Step CRUD Methods ───────────────────────────────────── - - /** - * Create a new workflow step definition. - * Generates a unique ID (WS-001, WS-002, etc.) and stores in the workflow_steps table. - */ - async createWorkflowStep(input: import("./types.js").WorkflowStepInput): Promise { - return this.withConfigLock(async () => { - const counterRow = this.db - .prepare("SELECT nextWorkflowStepId FROM config WHERE id = 1") - .get() as { nextWorkflowStepId?: number } | undefined; - const nextWsId = counterRow?.nextWorkflowStepId || 1; - const id = `WS-${String(nextWsId).padStart(3, "0")}`; - - const mode = input.mode || "prompt"; - const gateMode = input.gateMode || "advisory"; - - // Validate: script mode requires scriptName - if (mode === "script" && !input.scriptName?.trim()) { - throw new Error("Script mode requires a scriptName"); - } - - const now = new Date().toISOString(); - const step: import("./types.js").WorkflowStep = { - id, - templateId: input.templateId, - name: input.name, - description: input.description, - mode, - phase: input.phase || "pre-merge", - gateMode, - prompt: mode === "prompt" ? (input.prompt || "") : "", - toolMode: mode === "prompt" ? (input.toolMode || "readonly") : undefined, - scriptName: mode === "script" ? input.scriptName : undefined, - enabled: input.enabled !== undefined ? input.enabled : true, - defaultOn: input.defaultOn !== undefined ? input.defaultOn : undefined, - modelProvider: mode === "prompt" ? input.modelProvider : undefined, - modelId: mode === "prompt" ? input.modelId : undefined, - migratedFragmentId: input.migratedFragmentId, - createdAt: now, - updatedAt: now, - }; - - this.db.prepare( - `INSERT INTO workflow_steps ( - id, - templateId, - name, - description, - mode, - phase, - gateMode, - prompt, - toolMode, - scriptName, - enabled, - defaultOn, - modelProvider, - modelId, - migrated_fragment_id, - createdAt, - updatedAt - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, - ).run( - step.id, - step.templateId ?? null, - step.name, - step.description, - step.mode, - step.phase || "pre-merge", - step.gateMode, - step.prompt, - step.toolMode ?? null, - step.scriptName ?? null, - step.enabled ? 1 : 0, - step.defaultOn === undefined ? null : step.defaultOn ? 1 : 0, - step.modelProvider ?? null, - step.modelId ?? null, - step.migratedFragmentId ?? null, - step.createdAt, - step.updatedAt, - ); - - const config = await this.readConfig(); - await this.writeConfig(config, { nextWorkflowStepId: nextWsId + 1 }); - this.workflowStepsCache = null; - - return step; - }); - } + // ── Workflow Step palette (plugin templates) ────────────────────────── setPluginWorkflowStepTemplates(templates: Array<{ pluginId: string; template: WorkflowStepTemplate }>): void { this._pluginWorkflowStepTemplates = [...templates]; @@ -14563,278 +14377,45 @@ ${stepsSection}`; }; } + /* + FNXC:WorkflowStepCRUD 2026-06-26-14:00: + U7c dropped the legacy `workflow_steps` table (migration 131). Pre-merge and post-merge + workflow steps run graph-native and record into `task.workflowStepResults`; nothing reads + `workflow_steps` rows at runtime. The store-level table CRUD (`createWorkflowStep`/ + `updateWorkflowStep`/`deleteWorkflowStep`), the workflow-compilation materializer, and + `migrateLegacyWorkflowSteps` have been REMOVED. The plugin step-template PALETTE + (`setPluginWorkflowStepTemplates` / `resolvePluginWorkflowStep`) is RETAINED — it is + in-memory only and never touches the table. `listWorkflowSteps` returns ONLY plugin palette + steps (so `readConfig` and the task-create default-on fallback keep working without the + table), and `getWorkflowStep` is retained but resolves ONLY `plugin:`-prefixed palette ids + (every other id → undefined). Both are the public surface plugins use for their palette. + */ + /** - * List all workflow step definitions from workflow_steps. - * Results are cached and invalidated on create/update/delete. + * List workflow step definitions. Post table-drop (U7c) this returns only the + * in-memory plugin step-template palette; legacy table-backed steps no longer exist. */ async listWorkflowSteps(): Promise { if (this.workflowStepsCache) return this.workflowStepsCache; - const rows = this.db.prepare("SELECT * FROM workflow_steps ORDER BY createdAt ASC").all() as Array<{ - id: string; - templateId: string | null; - name: string; - description: string; - mode: string; - phase: string | null; - prompt: string; - gateMode: string | null; - toolMode: string | null; - scriptName: string | null; - enabled: number; - defaultOn: number | null; - modelProvider: string | null; - modelId: string | null; - createdAt: string; - updatedAt: string; - }>; - const storedSteps = rows - .map((row) => this.applyLegacyWorkflowStepOverrides(this.toStoredWorkflowStep(row))) - // Steps materialized by compiling a workflow are an execution detail; keep - // them out of the user-facing step manager listing. The executor resolves - // them directly via getWorkflowStep, which is unaffected by this filter. - .filter((step) => !step.templateId?.startsWith(WORKFLOW_COMPILED_STEP_TEMPLATE_PREFIX)); const pluginSteps = this._pluginWorkflowStepTemplates .map(({ template }) => this.resolvePluginWorkflowStep(template.id)) .filter((step): step is import("./types.js").WorkflowStep => Boolean(step)); - this.workflowStepsCache = [...storedSteps, ...pluginSteps]; + this.workflowStepsCache = [...pluginSteps]; return this.workflowStepsCache; } /** - * Get a single workflow step by ID. + * Resolve a single workflow step by id. Post table-drop (U7c) only PLUGIN palette + * steps (`plugin::`) resolve — from the in-memory plugin + * step-template registry, never the dropped `workflow_steps` table. Any other id + * (legacy WS-xxx, graph node ids) resolves to `undefined`, which every caller + * treats as "no step". */ async getWorkflowStep(id: string): Promise { - if (id.startsWith("plugin:")) { - const pluginStep = this.resolvePluginWorkflowStep(id); - if (pluginStep) { - return pluginStep; - } - } - - const byId = this.db.prepare("SELECT * FROM workflow_steps WHERE id = ?").get(id) as - | { - id: string; - templateId: string | null; - name: string; - description: string; - mode: string; - phase: string | null; - gateMode: string | null; - prompt: string; - toolMode: string | null; - scriptName: string | null; - enabled: number; - defaultOn: number | null; - modelProvider: string | null; - modelId: string | null; - createdAt: string; - updatedAt: string; - } - | undefined; - if (byId) { - return this.applyLegacyWorkflowStepOverrides(this.toStoredWorkflowStep(byId)); - } - - const byTemplate = this.db - .prepare("SELECT * FROM workflow_steps WHERE templateId = ? ORDER BY createdAt ASC LIMIT 1") - .get(id) as - | { - id: string; - templateId: string | null; - name: string; - description: string; - mode: string; - phase: string | null; - gateMode: string | null; - prompt: string; - toolMode: string | null; - scriptName: string | null; - enabled: number; - defaultOn: number | null; - modelProvider: string | null; - modelId: string | null; - createdAt: string; - updatedAt: string; - } - | undefined; - if (byTemplate) { - return this.applyLegacyWorkflowStepOverrides(this.toStoredWorkflowStep(byTemplate)); - } - - // U6: the built-in step-template catalog was deleted. Built-in quality - // gates (browser-verification, code-review) are now graph optional-group nodes, not - // `workflow_steps` rows, so there is no built-in template to synthesize a step from. - // An id with no stored row resolves to undefined (callers treat that as "no step"). + if (id.startsWith("plugin:")) return this.resolvePluginWorkflowStep(id); return undefined; } - /* - FNXC:WorkflowStepCRUD 2026-06-25-00:00: - U5 removed the `/api/workflow-steps` REST surface (GET/POST/PATCH/DELETE + refine), the - `/workflow-step-templates/:id/create` route, the dead dashboard client mutations, and - (already absent) the Settings management UI. The store-level `workflow_steps` CRUD is - INTENTIONALLY KEPT in full: the table is retained (its drop is U7), `createWorkflowStep` - drives the workflow-compilation materializer (`materializeWorkflowSteps`), - `getWorkflowStep` is consumed by the engine (merger post-merge steps + executor - recovery), `listWorkflowSteps` backs `readConfig`, and `update`/`deleteWorkflowStep` - round-trip the table for those execution/read paths and their store tests. Removing the - store methods belongs with U7's table drop, not the management-surface removal. - */ - /** - * Update a workflow step definition. - * @throws Error if the workflow step is not found - */ - async updateWorkflowStep(id: string, updates: Partial): Promise { - const row = this.db.prepare("SELECT * FROM workflow_steps WHERE id = ?").get(id) as - | { - id: string; - templateId: string | null; - name: string; - description: string; - mode: string; - phase: string | null; - gateMode: string | null; - prompt: string; - toolMode: string | null; - scriptName: string | null; - enabled: number; - defaultOn: number | null; - modelProvider: string | null; - modelId: string | null; - createdAt: string; - updatedAt: string; - } - | undefined; - - if (!row) { - throw new Error(`Workflow step '${id}' not found`); - } - - const step = this.toStoredWorkflowStep(row); - - // Handle mode change - if (updates.mode !== undefined) { - const newMode = updates.mode; - // Validate: script mode requires scriptName - if (newMode === "script" && !updates.scriptName?.trim() && !step.scriptName?.trim()) { - throw new Error("Script mode requires a scriptName"); - } - step.mode = newMode; - // When switching to script mode, clear prompt and model overrides - if (newMode === "script") { - step.prompt = ""; - step.gateMode = step.gateMode || "gate"; - step.toolMode = undefined; - step.modelProvider = undefined; - step.modelId = undefined; - } - // When switching to prompt mode, clear scriptName - if (newMode === "prompt") { - step.scriptName = undefined; - step.gateMode = step.gateMode || "advisory"; - step.toolMode = step.toolMode || "readonly"; - } - } - - if (updates.name !== undefined) step.name = updates.name; - if (updates.description !== undefined) step.description = updates.description; - if (updates.phase !== undefined) step.phase = updates.phase; - if (updates.gateMode !== undefined) step.gateMode = updates.gateMode; - if (updates.prompt !== undefined && step.mode === "prompt") step.prompt = updates.prompt; - if (updates.toolMode !== undefined && step.mode === "prompt") step.toolMode = updates.toolMode; - if (updates.scriptName !== undefined && step.mode === "script") step.scriptName = updates.scriptName; - if (updates.enabled !== undefined) step.enabled = updates.enabled; - if (updates.defaultOn !== undefined) step.defaultOn = updates.defaultOn; - if (step.mode === "script" && !step.scriptName?.trim()) { - throw new Error("Script mode requires a scriptName"); - } - if (step.mode === "prompt") { - if ("modelProvider" in updates) step.modelProvider = updates.modelProvider; - if ("modelId" in updates) step.modelId = updates.modelId; - } - if ("migratedFragmentId" in updates) step.migratedFragmentId = updates.migratedFragmentId; - step.updatedAt = new Date().toISOString(); - - this.db.prepare( - `UPDATE workflow_steps - SET templateId = ?, - name = ?, - description = ?, - mode = ?, - phase = ?, - gateMode = ?, - prompt = ?, - toolMode = ?, - scriptName = ?, - enabled = ?, - defaultOn = ?, - modelProvider = ?, - modelId = ?, - migrated_fragment_id = ?, - updatedAt = ? - WHERE id = ?`, - ).run( - step.templateId ?? null, - step.name, - step.description, - step.mode, - step.phase || "pre-merge", - step.gateMode, - step.prompt, - step.toolMode ?? null, - step.scriptName ?? null, - step.enabled ? 1 : 0, - step.defaultOn === undefined ? null : step.defaultOn ? 1 : 0, - step.modelProvider ?? null, - step.modelId ?? null, - step.migratedFragmentId ?? null, - step.updatedAt, - step.id, - ); - this.db.bumpLastModified(); - this.workflowStepsCache = null; - - return step; - } - - /** - * Delete a workflow step definition. - * Also removes the ID from any tasks that reference it in enabledWorkflowSteps. - * @throws Error if the workflow step is not found - */ - async deleteWorkflowStep(id: string): Promise { - const deleted = this.db.prepare("DELETE FROM workflow_steps WHERE id = ?").run(id) as { - changes?: number; - }; - - if ((deleted.changes || 0) === 0) { - throw new Error(`Workflow step '${id}' not found`); - } - - this.db.bumpLastModified(); - this.workflowStepsCache = null; - - // Clean up references from existing tasks (best-effort, outside config lock) - try { - const tasks = await this.listTasks({ slim: true }); - for (const task of tasks) { - if (task.enabledWorkflowSteps?.includes(id)) { - const updated = task.enabledWorkflowSteps.filter((wsId) => wsId !== id); - // Direct task.json mutation for enabledWorkflowSteps cleanup - await this.withTaskLock(task.id, async () => { - const dir = this.taskDir(task.id); - const t = await this.readTaskJson(dir); - t.enabledWorkflowSteps = updated.length > 0 ? updated : undefined; - t.updatedAt = new Date().toISOString(); - await this.atomicWriteTaskJson(dir, t); - }); - } - } - } catch { - // Best-effort: task cleanup is non-critical - } - } - // ── Workflow definitions (named WorkflowIr graphs) ───────────────────── /** Allocate the next workflow-definition id (WF-001, WF-002, …) using a @@ -15243,24 +14824,12 @@ ${stepsSection}`; // Best-effort: a dangling default falls back gracefully at task creation. } - // Cascade: drop selections referencing this workflow, their materialized - // step rows, and reset the affected tasks' enabled steps. + // Cascade: drop selections referencing this workflow and reset the affected + // tasks' enabled steps. (U7c: no materialized `workflow_steps` rows to delete.) const selections = this.db - .prepare("SELECT taskId, stepIds FROM task_workflow_selection WHERE workflowId = ?") - .all(id) as Array<{ taskId: string; stepIds: string }>; + .prepare("SELECT taskId FROM task_workflow_selection WHERE workflowId = ?") + .all(id) as Array<{ taskId: string }>; for (const row of selections) { - try { - const stepIds = JSON.parse(row.stepIds) as unknown; - if (Array.isArray(stepIds)) { - for (const stepId of stepIds) { - if (typeof stepId === "string") { - this.db.prepare("DELETE FROM workflow_steps WHERE id = ?").run(stepId); - } - } - } - } catch { - // Corrupt stepIds list — still remove the selection row below. - } this.db.prepare("DELETE FROM task_workflow_selection WHERE taskId = ?").run(row.taskId); try { await this.updateTask(row.taskId, { enabledWorkflowSteps: [] }); @@ -15269,7 +14838,6 @@ ${stepsSection}`; // at execution time and are skipped. } } - if (selections.length > 0) this.workflowStepsCache = null; this.db.bumpLastModified(); // U5 (R20) delete reconciliation: re-home each occupant to the default @@ -15753,130 +15321,6 @@ ${stepsSection}`; return definition; } - /** - * Lazy, idempotent migration of legacy user-authored workflow steps into the - * dual workflow-definition representation (U2 / R5 / KTD-3). Runs on first - * editor open per project via `POST /api/workflows/migrate-legacy-steps`. - * - * Policy: - * - Every unmigrated user step (enabled or not, excluding compiled-materialized - * rows) becomes a `kind: "fragment"` definition — the reusable palette piece. - * - The `defaultOn` subset additionally becomes ONE combined `kind: "workflow"` - * definition named "Migrated steps" (these were the steps that ran - * automatically on new tasks); when non-empty and no project default is - * already set, it becomes the project default so new-task behavior is - * preserved. An explicit existing default is never clobbered. - * - Each source row is stamped with `migratedFragmentId` (idempotency marker). - * Source rows are never deleted. - * - * Idempotency: the unmigrated-rows SELECT and the marker stamping happen inside - * a single `transactionImmediate` (write lock acquired BEFORE the SELECT, - * matching `selectTaskWorkflow`'s ordering rationale), so concurrent opens / - * re-runs converge to a single set of definitions. A second run sees zero - * unmigrated rows and returns `{ migrated: 0, skipped: n }`. - */ - async migrateLegacyWorkflowSteps(): Promise<{ - migrated: number; - skipped: number; - combinedWorkflowId?: string; - }> { - // Resolve async prerequisites BEFORE the synchronous transaction: the - // workflow-columns flag (for flag-aware persistence). The project default is - // re-read AFTER the transaction (compare-and-set) so a concurrently-set - // default is never clobbered. - const flagOn = await this.workflowColumnsFlagOn(); - - const result = this.db.transactionImmediate(() => { - // Write lock is now held. Read the raw step rows directly (the cached, - // plugin-merged listWorkflowSteps() is not transaction-scoped). Mirror - // listWorkflowSteps()'s compiled-materialized filter and toStoredWorkflowStep - // mapping so policy decisions match the user-facing step listing. - const rows = this.db - .prepare("SELECT * FROM workflow_steps ORDER BY createdAt ASC") - .all() as Array[0]>; - - const userSteps = rows - .map((row) => this.applyLegacyWorkflowStepOverrides(this.toStoredWorkflowStep(row))) - // Compiled-materialized rows are an execution detail, not user-authored. - .filter((step) => !step.templateId?.startsWith(WORKFLOW_COMPILED_STEP_TEMPLATE_PREFIX)); - - const alreadyMigrated = userSteps.filter((s) => s.migratedFragmentId); - const unmigrated = userSteps.filter((s) => !s.migratedFragmentId); - - if (unmigrated.length === 0) { - return { migrated: 0, skipped: alreadyMigrated.length, combinedWorkflowId: undefined as string | undefined }; - } - - // Every unmigrated user step → a single-node fragment; stamp the source row. - for (const step of unmigrated) { - // parseWorkflowIr runs inside both insertWorkflowDefinitionSync and - // layoutForIr, so compute the fragment IR once and reuse it. - const fragmentIr = stepToFragmentIr(step); - const fragment = this.insertWorkflowDefinitionSync( - { - name: step.name, - description: step.description, - kind: "fragment", - ir: fragmentIr, - layout: layoutForIr(fragmentIr), - }, - flagOn, - ); - this.db - .prepare("UPDATE workflow_steps SET migrated_fragment_id = ?, updatedAt = ? WHERE id = ?") - .run(fragment.id, new Date().toISOString(), step.id); - } - this.workflowStepsCache = null; - this.db.bumpLastModified(); - - // The defaultOn subset → one combined "Migrated steps" workflow. - const defaultOnSteps = unmigrated.filter((s) => s.defaultOn === true); - let combinedWorkflowId: string | undefined; - if (defaultOnSteps.length > 0) { - const ir = stepsToWorkflowIr(defaultOnSteps, "Migrated steps"); - const combined = this.insertWorkflowDefinitionSync( - { - name: "Migrated steps", - description: "Converted from your legacy workflow steps", - kind: "workflow", - ir, - layout: layoutForIr(ir), - }, - flagOn, - ); - combinedWorkflowId = combined.id; - } - - return { migrated: unmigrated.length, skipped: alreadyMigrated.length, combinedWorkflowId }; - }); - - // Set the combined workflow as the project default — only when one was - // created AND no explicit default is already set (don't clobber a user - // choice). Done outside the transaction via the async setter so the project - // default-workflow hooks run. Compare-and-set against the CURRENT default - // (re-read immediately before writing, not the pre-transaction snapshot) so - // a default set concurrently by another writer is never overwritten. If the - // set fails, swallow the error: a missing migrated default is recoverable - // (the user can set one), but throwing here would surface the whole - // migration as failed even though the definitions were written. - if (result.combinedWorkflowId) { - const currentDefaultId = await this.getDefaultWorkflowId(); - if (!currentDefaultId) { - try { - await this.setDefaultWorkflowId(result.combinedWorkflowId); - } catch (err) { - storeLog.warn("Failed to set migrated combined workflow as project default", { - phase: "migrateLegacyWorkflowSteps:set-default", - combinedWorkflowId: result.combinedWorkflowId, - error: err instanceof Error ? err.message : String(err), - }); - } - } - } - - return result; - } - /** Whether a raw workflow CLI command has been approved (trust-on-first-use). * Comparison is on the exact trimmed command string. */ async isWorkflowCliCommandApproved(command: string): Promise { @@ -16126,136 +15570,67 @@ ${stepsSection}`; .run(taskId, workflowId, JSON.stringify(stepIds), new Date().toISOString()); } - /** Delete the WorkflowStep rows previously materialized for a task's selection - * and remove the selection record. Best-effort; safe to call when unset. */ + /* + FNXC:WorkflowStepCRUD 2026-06-26-14:00: + U7c: workflow selection no longer MATERIALIZES legacy `workflow_steps` rows (the table is + dropped). A task's selection records the workflow id plus the set of DEFAULT-ON + `optional-group` node ids (the `enabledWorkflowSteps` toggle keys the graph reads at the + optional-group seam via `enabledWorkflowSteps.includes(node.id)`). The graph runs the + selected workflow's IR directly from `workflowId`; it never reads `selection.stepIds` as + table rows. Compilation is retained ONLY for up-front IR validation (genuinely invalid + graphs still throw before any state is written; interpreter-deferred built-ins are valid). + */ + + /** Remove a task's workflow selection record. Best-effort; safe when unset. */ private removeMaterializedSelection(taskId: string): void { - const existing = this.getTaskWorkflowSelection(taskId); - if (existing) { - for (const stepId of existing.stepIds) { - this.db.prepare("DELETE FROM workflow_steps WHERE id = ?").run(stepId); - } - this.workflowStepsCache = null; - } this.db.prepare("DELETE FROM task_workflow_selection WHERE taskId = ?").run(taskId); } - /** Purge a task's workflow selection and its materialized WorkflowStep rows - * when the task row itself is being physically removed. `task_workflow_selection` - * has no FK to `tasks(id)` (SQLite can't add one to an existing table without a - * rebuild), so deletion must be mirrored here to avoid orphaned selection rows - * and unreclaimable compiled steps. Best-effort and synchronous: unlike - * clearTaskWorkflowSelection it does not touch enabledWorkflowSteps, since the - * owning task row no longer exists. */ + /** Purge a task's workflow selection row when the task row itself is being + * physically removed. `task_workflow_selection` has no FK to `tasks(id)` + * (SQLite can't add one to an existing table without a rebuild), so deletion + * must be mirrored here to avoid orphaned selection rows. */ private purgeTaskWorkflowSelectionRows(taskId: string): void { - const row = this.db - .prepare("SELECT stepIds FROM task_workflow_selection WHERE taskId = ?") - .get(taskId) as { stepIds: string } | undefined; - if (!row) return; - try { - const parsed = JSON.parse(row.stepIds) as unknown; - if (Array.isArray(parsed)) { - for (const stepId of parsed) { - if (typeof stepId === "string") { - this.db.prepare("DELETE FROM workflow_steps WHERE id = ?").run(stepId); - } - } - } - } catch { - // Corrupt stepIds list — still remove the selection row below. - } this.db.prepare("DELETE FROM task_workflow_selection WHERE taskId = ?").run(taskId); - this.workflowStepsCache = null; } - /** Delete a set of freshly materialized WorkflowStep rows that were never - * successfully attached to a task/selection (e.g. the owning task create - * failed). Best-effort; tolerates already-removed ids. */ - private cleanupOrphanedMaterializedSteps(stepIds: string[] | undefined): void { - if (!stepIds || stepIds.length === 0) return; - for (const stepId of stepIds) { - try { - this.db.prepare("DELETE FROM workflow_steps WHERE id = ?").run(stepId); - } catch { - // Best-effort cleanup. - } + /** Validate a workflow's IR by compiling it; throws on genuinely invalid graphs. + * Interpreter-deferred built-ins (optional-group bearing) are valid and tolerated. + * No `workflow_steps` rows are written (U7c). */ + private validateWorkflowCompilable(workflowId: string, def: { ir: WorkflowIr }): void { + try { + compileWorkflowToSteps(def.ir); + } catch (err) { + if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) return; + throw err; } - this.workflowStepsCache = null; } - /** Persist pre-compiled workflow steps as fresh WorkflowStep rows and return - * their ids in execution order. Steps are tagged so they stay out of the - * step manager. Compile via compileWorkflowToSteps before calling. */ - private async materializeWorkflowSteps( - workflowId: string, - inputs: import("./types.js").WorkflowStepInput[], - ): Promise { - const ids: string[] = []; - for (const input of inputs) { - const step = await this.createWorkflowStep({ - ...input, - templateId: `${WORKFLOW_COMPILED_STEP_TEMPLATE_PREFIX}${workflowId}`, - enabled: true, - }); - ids.push(step.id); - } - return ids; - } - - /** Resolve the project-default workflow into materialized step ids, or null - * when no default is set / it is missing / it does not compile. */ + /** Resolve the project-default workflow into the selection seed (workflow id + + * default-on optional-group node ids), or undefined when no default is set / + * it is missing / it is a fragment. */ private async materializeDefaultWorkflowSteps(): Promise<{ workflowId: string; stepIds: string[] } | undefined> { const workflowId = await this.getDefaultWorkflowId(); if (!workflowId) return undefined; const def = await this.getWorkflowDefinition(workflowId); if (!def) return undefined; // KTD-1/R6: a fragment must never act as a project default (it is not a - // selectable workflow); fall back to no default rather than materializing it. + // selectable workflow); fall back to no default. if (def.kind === "fragment") return undefined; - // Compile (and validate) before creating any rows so a non-compilable - // default falls back cleanly with nothing written. Interpreter-deferred - // built-ins are valid selectable workflows but not lowerable to legacy - // WorkflowStep rows, so default materialization falls back to legacy defaults. - // Built-ins that compile to zero steps still record a stepless selection, - // mirroring explicit workflow materialization. - let inputs: import("./types.js").WorkflowStepInput[]; - try { - inputs = compileWorkflowToSteps(def.ir); - } catch (err) { - // FNXC:CodeReviewStep 2026-06-25-15:00: - // Interpreter-deferred built-ins (e.g. builtin:coding/stepwise, which carry - // optional-group nodes) cannot lower to legacy WorkflowStep rows, but they may - // still carry DEFAULT-ON optional groups (e.g. `code-review`) that must be seeded - // into the new task's `enabledWorkflowSteps` for default-on to actually take - // effect — the executor enables a group strictly via - // `enabledWorkflowSteps.includes(node.id)` with no defaultOn fallback. Mirror the - // explicit-workflow path (`materializeExplicitWorkflowSteps`) by recording a - // selection seeded with the default-on group ids instead of bailing to `undefined` - // (which dropped the seeding and silently disabled default-on groups under a - // project-default workflow). - if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) { - return { workflowId, stepIds: resolveDefaultOnOptionalGroupIds(def.ir) }; - } - throw err; - } + // Validate the IR up front (a genuinely non-compilable default propagates and + // is caught by the createTask fallback). Interpreter-deferred built-ins are valid. + this.validateWorkflowCompilable(workflowId, def); // FNXC:WorkflowOptionalGroup 2026-06-21-14:20: seed `enabledWorkflowSteps` - // with the ids of `optional-group` nodes whose `defaultOn` is true, mirroring - // the prior `optionalStep.defaultOn ?? false` precedence (U3, R3). These group - // ids are NOT WorkflowStep rows — they are toggle keys the executor reads at - // the optional-group seam — so they ride alongside the compiled step ids. - const defaultGroupIds = resolveDefaultOnOptionalGroupIds(def.ir); - if (isBuiltinWorkflowId(workflowId) && inputs.length === 0) { - return { workflowId, stepIds: defaultGroupIds }; - } - const stepIds = await this.materializeWorkflowSteps(workflowId, inputs); - return { workflowId, stepIds: [...stepIds, ...defaultGroupIds] }; + // with the ids of `optional-group` nodes whose `defaultOn` is true. These group + // ids are the toggle keys the executor reads at the optional-group seam. + return { workflowId, stepIds: resolveDefaultOnOptionalGroupIds(def.ir) }; } - /** Resolve an EXPLICITLY requested workflow id (U6/R3/KTD-4) into materialized - * step ids for the create-time `workflowId` parameter. Unlike + /** Resolve an EXPLICITLY requested workflow id (U6/R3/KTD-4) into the selection + * seed for the create-time `workflowId` parameter. Unlike * `materializeDefaultWorkflowSteps`, unknown ids and fragments are hard errors - * (thrown BEFORE any task row is created) rather than silent fallbacks, since - * the caller asked for a specific workflow. Compilation happens up front so a - * non-compilable workflow aborts before any rows are written. */ + * (thrown BEFORE any task row is created) since the caller asked for a specific + * workflow. Validation happens up front so a non-compilable workflow aborts. */ private async materializeExplicitWorkflowSteps( workflowId: string, ): Promise<{ workflowId: string; stepIds: string[] }> { @@ -16264,88 +15639,42 @@ ${stepsSection}`; if (def.kind === "fragment") { throw new Error(`Workflow '${workflowId}' is a fragment and cannot be selected for a task`); } - let inputs: import("./types.js").WorkflowStepInput[]; - try { - inputs = compileWorkflowToSteps(def.ir); - } catch (err) { - if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) - return { workflowId, stepIds: resolveDefaultOnOptionalGroupIds(def.ir) }; - throw err; - } - // FNXC:WorkflowOptionalGroup 2026-06-21-14:20: same defaultOn-group seeding as - // the default-workflow path, for an explicitly requested create-time workflow. - const defaultGroupIds = resolveDefaultOnOptionalGroupIds(def.ir); - const stepIds = await this.materializeWorkflowSteps(workflowId, inputs); - return { workflowId, stepIds: [...stepIds, ...defaultGroupIds] }; + this.validateWorkflowCompilable(workflowId, def); + return { workflowId, stepIds: resolveDefaultOnOptionalGroupIds(def.ir) }; } /** - * Select a workflow for a task: compile it when possible, materialize its - * steps, and write their ids into the task's enabledWorkflowSteps. Replaces - * any prior selection (no orphaned steps). Interpreter-deferred workflow IRs - * record the selection with zero materialized steps; genuinely invalid graphs - * still throw before any state is written. + * Select a workflow for a task: validate its IR, then record the selection + * (workflow id + default-on optional-group node ids) and write those ids into + * the task's enabledWorkflowSteps. Replaces any prior selection. Genuinely + * invalid graphs throw before any state is written (U7c: no row materialization). */ async selectTaskWorkflow(taskId: string, workflowId: string): Promise { - // Hold the task lock across the whole sequence (materialize → owner write → - // prior-step cleanup) so it can't interleave with a concurrent select/clear - // or executor updateTask on the same task. updateTaskUnlocked is used inside - // because the per-task lock is non-reentrant. + // Hold the task lock across the whole sequence so it can't interleave with a + // concurrent select/clear or executor updateTask on the same task. + // updateTaskUnlocked is used inside because the per-task lock is non-reentrant. return this.withTaskLock(taskId, async () => { const def = await this.getWorkflowDefinition(workflowId); if (!def) throw new Error(`Workflow '${workflowId}' not found`); // KTD-1/R6: fragments are reusable single-node palette templates, not - // selectable workflows. Reject them from task selection with a clear error - // rather than materializing a degenerate single-step task. + // selectable workflows. if (def.kind === "fragment") { throw new Error(`Workflow '${workflowId}' is a fragment and cannot be selected for a task`); } - // Compile once up front: invalid graphs abort before any mutation, while - // interpreter-deferred graphs keep the selection but materialize no legacy - // WorkflowStep rows. - let inputs: import("./types.js").WorkflowStepInput[]; - try { - inputs = compileWorkflowToSteps(def.ir); - } catch (err) { - if (isBuiltinWorkflowId(workflowId) && isInterpreterDeferredWorkflowCompileError(err)) inputs = []; - else throw err; - } + // Validate once up front: invalid graphs abort before any mutation. + this.validateWorkflowCompilable(workflowId, def); - // Materialize the new steps and point the task at them BEFORE deleting the - // prior selection's rows, so a mid-flight failure never leaves the task - // referencing already-deleted step ids. - const priorSelection = this.getTaskWorkflowSelection(taskId); // U11/KTD-13: capture the OLD field schema (from the prior selection's IR) // before the selection row flips, so we can reconcile existing field values // against the NEW workflow's schema below. const oldFieldDefs = this.resolveTaskCustomFieldDefsSync(taskId); const newFieldDefs: WorkflowFieldDefinition[] = def.ir.version === "v2" ? (def.ir.fields ?? []) : []; - const ids = await this.materializeWorkflowSteps(workflowId, inputs); - try { - await this.updateTaskUnlocked(taskId, { enabledWorkflowSteps: ids }); - this.writeTaskWorkflowSelection(taskId, workflowId, ids); - } catch (err) { - // The owner write (updateTask / selection upsert) failed, so the steps we - // just materialized would orphan with no selection row pointing at them. - // Delete them before propagating; the prior selection is left untouched. - for (const stepId of ids) { - try { - this.db.prepare("DELETE FROM workflow_steps WHERE id = ?").run(stepId); - } catch { - // Best-effort cleanup; surface the original error below. - } - } - this.workflowStepsCache = null; - throw err; - } - - if (priorSelection) { - for (const stepId of priorSelection.stepIds) { - this.db.prepare("DELETE FROM workflow_steps WHERE id = ?").run(stepId); - } - this.workflowStepsCache = null; - } + // Selection seed: default-on optional-group node ids (the graph runs the IR + // directly from workflowId; these ids are the enabledWorkflowSteps toggles). + const ids = resolveDefaultOnOptionalGroupIds(def.ir); + await this.updateTaskUnlocked(taskId, { enabledWorkflowSteps: ids }); + this.writeTaskWorkflowSelection(taskId, workflowId, ids); // U11/KTD-13: reconcile custom field values against the NEW workflow's // schema. Same-id, type-compatible values are kept; incompatible/removed diff --git a/packages/dashboard/app/api/legacy.ts b/packages/dashboard/app/api/legacy.ts index 7d6c476b5f..93226ccc13 100644 --- a/packages/dashboard/app/api/legacy.ts +++ b/packages/dashboard/app/api/legacy.ts @@ -5464,22 +5464,8 @@ export function importWorkflow( }); } -/** Result of the lazy legacy-step migration (U2/R5). `migrated` is the number of - * newly converted user steps; `skipped` the count already migrated; when the - * defaultOn subset was non-empty a combined "Migrated steps" workflow id is set. */ -export interface MigrateLegacyStepsResult { - migrated: number; - skipped: number; - combinedWorkflowId?: string; -} - -/** Run the lazy, idempotent migration of legacy user-authored workflow steps into - * fragments + a combined workflow (U2/R5). Safe to call repeatedly. */ -export function migrateLegacyWorkflowSteps(projectId?: string): Promise { - return api(withProjectId("/workflows/migrate-legacy-steps", projectId), { - method: "POST", - }); -} +// FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c removed migrateLegacyWorkflowSteps and +// MigrateLegacyStepsResult along with the legacy workflow_steps table and its route. /** Result of POST /api/workflows/design (U10/R11). The server validates the * AI-produced IR (parseWorkflowIr), triages compilability (`interpreterOnly`), diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.css b/packages/dashboard/app/components/WorkflowNodeEditor.css index ec27b5b56b..deba3b1459 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.css +++ b/packages/dashboard/app/components/WorkflowNodeEditor.css @@ -114,36 +114,8 @@ Align the embedded workflows header to the shared ViewHeader/Insights metric — cursor: pointer; } -/* U2/R5: one-time legacy-step migration notice banner. */ -.wf-migration-notice { - display: flex; - align-items: center; - justify-content: space-between; - gap: var(--space-sm); - padding: var(--space-sm) var(--space-md); - background: color-mix(in srgb, var(--accent) 12%, transparent); - border-bottom: 1px solid var(--border); - color: var(--text); - font-size: 0.85rem; -} - -.wf-migration-notice-text { - flex: 1; -} - -.wf-migration-notice-dismiss { - display: inline-flex; - align-items: center; - background: transparent; - border: none; - color: var(--text-muted); - cursor: pointer; - flex-shrink: 0; -} - -.wf-migration-notice-dismiss:hover { - color: var(--text); -} +/* FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c removed the .wf-migration-notice banner + styles along with the legacy-step migration notice. */ .wf-editor-close:hover { color: var(--text); @@ -2133,12 +2105,10 @@ Column trait toggles are left-sidebar workflow controls; keep their enabled and } .wf-editor-header, - .wf-migration-notice, .wf-editor-readonly-banner { flex-wrap: wrap; } - .wf-migration-notice, .wf-editor-readonly-banner { gap: var(--space-sm); } diff --git a/packages/dashboard/app/components/WorkflowNodeEditor.tsx b/packages/dashboard/app/components/WorkflowNodeEditor.tsx index 5f5852c292..7698241749 100644 --- a/packages/dashboard/app/components/WorkflowNodeEditor.tsx +++ b/packages/dashboard/app/components/WorkflowNodeEditor.tsx @@ -29,7 +29,6 @@ import { importWorkflow, designWorkflow, ApiRequestError, - migrateLegacyWorkflowSteps, fetchModels, fetchAgents, fetchDiscoveredSkills, @@ -902,16 +901,6 @@ function InnerEditor({ // canvas container (R6) instead of leaving it on a now-removed node. const canvasRef = useRef(null); - // U2/R5: one-time legacy-step migration notice. Shown after the on-open - // migration call converts >0 steps, dismissible, dismissal persisted in - // localStorage (per project when a projectId is available). Guards against - // re-showing across re-opens. - const migrationNoticeStorageKey = useMemo( - () => `fusion:wf-migration-notice-dismissed${projectId ? `:${projectId}` : ""}`, - [projectId], - ); - const [showMigrationNotice, setShowMigrationNotice] = useState(false); - // U5/R10: import affordance state. `importError` renders a PERSISTENT inline // error region (not a toast) for client parse failures and server 4xx // validation failures; `importWarnings` renders non-blocking notes in the same @@ -1119,48 +1108,9 @@ function InnerEditor({ setWorkflowListStageOpen(false); }, [activeId, initialWorkflowId, isMobileMode, workflowListStageOpen]); - // U2/R5: fire the lazy legacy-step migration once on editor open, then reload - // the workflow list so any newly created fragments / "Migrated steps" workflow - // appear. Non-fatal on ANY error (incl. 404 if the route ships in a later - // release — the call is best-effort). When the run converted >0 steps and the - // notice hasn't been dismissed before, surface the one-time notice. - const migrationFiredRef = useRef(false); - useEffect(() => { - if (migrationFiredRef.current) return; - migrationFiredRef.current = true; - let cancelled = false; - void (async () => { - try { - const result = await migrateLegacyWorkflowSteps(projectId); - if (cancelled) return; - if (result.migrated > 0) { - await loadWorkflows(); - if (cancelled) return; - let dismissed = false; - try { - dismissed = localStorage.getItem(migrationNoticeStorageKey) === "1"; - } catch { - // localStorage unavailable (private mode / SSR): treat as not dismissed. - } - if (!dismissed) setShowMigrationNotice(true); - } - } catch { - // Non-fatal: migration is best-effort and tolerates a missing route. - } - })(); - return () => { - cancelled = true; - }; - }, [projectId, loadWorkflows, migrationNoticeStorageKey]); - - const dismissMigrationNotice = useCallback(() => { - setShowMigrationNotice(false); - try { - localStorage.setItem(migrationNoticeStorageKey, "1"); - } catch { - // Best-effort persistence; the in-session dismissal still hides it. - } - }, [migrationNoticeStorageKey]); + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c removed the on-open legacy-step + // migration trigger and its one-time notice. The legacy workflow_steps table was + // dropped; workflow steps run graph-native, so there is nothing to migrate. // U5/R9: export the active workflow as a downloaded JSON envelope. Enabled for // built-ins; the caller gates on `isDirty` (a stale export is impossible @@ -2553,25 +2503,6 @@ function InnerEditor({ ) : null} - {showMigrationNotice ? ( -
- - {t( - "workflows.migrationNotice", - 'Your legacy workflow steps were converted — find them as templates in the palette and as the "Migrated steps" workflow.', - )} - - -
- ) : null}
({ this.status = status; } }, - migrateLegacyWorkflowSteps: vi.fn(), fetchTraits: vi.fn(), fetchStepParsers: vi.fn(), fetchModels: vi.fn(), @@ -85,7 +84,6 @@ import { createWorkflow, deleteWorkflow, fetchModels, - migrateLegacyWorkflowSteps, exportWorkflow, importWorkflow, designWorkflow, @@ -2806,54 +2804,8 @@ describe("WorkflowNodeEditor — U6 empty/onboarding states", () => { }); }); -describe("WorkflowNodeEditor — U2 legacy-step migration notice", () => { - beforeEach(() => { - vi.mocked(fetchWorkflows).mockResolvedValue([]); - vi.mocked(fetchTraits).mockResolvedValue(TRAIT_CATALOG); - vi.mocked(fetchStepParsers).mockResolvedValue(["step-headings", "json-steps"]); - localStorage.clear(); - }); - - afterEach(() => { - cleanup(); - localStorage.clear(); - vi.clearAllMocks(); - }); - - it("shows the one-time notice when migration converted steps", async () => { - vi.mocked(migrateLegacyWorkflowSteps).mockResolvedValue({ migrated: 2, skipped: 0, combinedWorkflowId: "WF-010" }); - render( {}} addToast={() => {}} projectId="p1" />); - expect(await screen.findByTestId("wf-migration-notice")).toBeInTheDocument(); - expect(migrateLegacyWorkflowSteps).toHaveBeenCalledWith("p1"); - }); - - it("dismisses the notice, persisting the dismissal so it stays hidden on re-open", async () => { - vi.mocked(migrateLegacyWorkflowSteps).mockResolvedValue({ migrated: 2, skipped: 0, combinedWorkflowId: "WF-010" }); - const { unmount } = render( - {}} addToast={() => {}} projectId="p1" />, - ); - const notice = await screen.findByTestId("wf-migration-notice"); - expect(notice).toBeInTheDocument(); - - fireEvent.click(screen.getByTestId("wf-migration-notice-dismiss")); - await waitFor(() => expect(screen.queryByTestId("wf-migration-notice")).not.toBeInTheDocument()); - expect(localStorage.getItem("fusion:wf-migration-notice-dismissed:p1")).toBe("1"); - - // Re-open the editor: the persisted dismissal keeps the notice hidden even - // though migration still reports migrated > 0. - unmount(); - render( {}} addToast={() => {}} projectId="p1" />); - await screen.findByTestId("wf-new-workflow"); - expect(screen.queryByTestId("wf-migration-notice")).not.toBeInTheDocument(); - }); - - it("does not show the notice when migration converted nothing", async () => { - vi.mocked(migrateLegacyWorkflowSteps).mockResolvedValue({ migrated: 0, skipped: 3 }); - render( {}} addToast={() => {}} projectId="p1" />); - await screen.findByTestId("wf-new-workflow"); - expect(screen.queryByTestId("wf-migration-notice")).not.toBeInTheDocument(); - }); -}); +// FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c removed the "U2 legacy-step migration +// notice" describe block along with the on-open migration trigger and its notice UI. // ── U5: import/export ─────────────────────────────────────────────────────── @@ -2862,7 +2814,6 @@ describe("WorkflowNodeEditor — U5 import/export", () => { vi.mocked(fetchTraits).mockResolvedValue(TRAIT_CATALOG); vi.mocked(fetchStepParsers).mockResolvedValue(["step-headings", "json-steps"]); vi.mocked(fetchModels).mockResolvedValue({ models: [] }); - vi.mocked(migrateLegacyWorkflowSteps).mockResolvedValue({ migrated: 0, skipped: 0 }); }); afterEach(() => { cleanup(); @@ -3057,7 +3008,6 @@ describe("WorkflowNodeEditor — U9 palette Templates section", () => { vi.mocked(fetchTraits).mockResolvedValue(TRAIT_CATALOG); vi.mocked(fetchStepParsers).mockResolvedValue(["step-headings", "json-steps"]); vi.mocked(fetchModels).mockResolvedValue({ models: [] }); - vi.mocked(migrateLegacyWorkflowSteps).mockResolvedValue({ migrated: 0, skipped: 0 }); vi.mocked(fetchWorkflowStepTemplates).mockResolvedValue({ templates: [] }); vi.mocked(fetchPluginWorkflowStepTemplates).mockResolvedValue({ templates: [] }); try { @@ -3399,7 +3349,6 @@ describe("WorkflowNodeEditor — U10 design-with-AI", () => { vi.mocked(fetchTraits).mockResolvedValue(TRAIT_CATALOG); vi.mocked(fetchStepParsers).mockResolvedValue(["step-headings", "json-steps"]); vi.mocked(fetchModels).mockResolvedValue({ models: [] }); - vi.mocked(migrateLegacyWorkflowSteps).mockResolvedValue({ migrated: 0, skipped: 0 }); }); afterEach(() => { cleanup(); diff --git a/packages/dashboard/src/routes/__tests__/workflow-migrate-route.test.ts b/packages/dashboard/src/routes/__tests__/workflow-migrate-route.test.ts deleted file mode 100644 index 84298071cf..0000000000 --- a/packages/dashboard/src/routes/__tests__/workflow-migrate-route.test.ts +++ /dev/null @@ -1,90 +0,0 @@ -// @vitest-environment node -// -// U2/R5 — HTTP integration coverage for POST /api/workflows/migrate-legacy-steps. -// Exercises the route end-to-end against a REAL TaskStore (no store-method -// mocking — mock-masked dead-wiring learning): the route must invoke the real -// migration seam, persist fragments + a combined workflow, and be idempotent. - -import { describe, it, expect, beforeEach, afterEach } from "vitest"; -import express from "express"; -import { mkdtempSync, rmSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { TaskStore, isBuiltinWorkflowId } from "@fusion/core"; -import { registerWorkflowRoutes } from "../register-workflow-routes.js"; -import { ApiError, sendErrorResponse } from "../../api-error.js"; -import { request } from "../../test-request.js"; - -describe("POST /api/workflows/migrate-legacy-steps (U2/R5)", () => { - let store: TaskStore; - let rootDir: string; - let globalDir: string; - let app: express.Express; - - beforeEach(async () => { - rootDir = mkdtempSync(join(tmpdir(), "wf-migrate-root-")); - globalDir = mkdtempSync(join(tmpdir(), "wf-migrate-global-")); - store = new TaskStore(rootDir, globalDir, { inMemoryDb: true }); - await store.init(); - - app = express(); - app.use(express.json()); - const router = express.Router(); - registerWorkflowRoutes({ - router, - getProjectContext: async () => ({ store, engine: undefined, projectId: undefined }), - rethrowAsApiError: (err: unknown) => { - throw err instanceof ApiError ? err : new ApiError(500, err instanceof Error ? err.message : String(err)); - }, - } as unknown as Parameters[0]); - app.use("/api", router); - app.use((err: unknown, _req: express.Request, res: express.Response, _next: express.NextFunction) => { - if (err instanceof ApiError) sendErrorResponse(res, err.statusCode, err.message, { details: err.details }); - else sendErrorResponse(res, 500, err instanceof Error ? err.message : String(err)); - }); - }); - - afterEach(() => { - store.close(); - rmSync(rootDir, { recursive: true, force: true }); - rmSync(globalDir, { recursive: true, force: true }); - }); - - const post = (path: string) => request(app, "POST", path); - - async function userDefCount() { - return (await store.listWorkflowDefinitions()).filter((w) => !isBuiltinWorkflowId(w.id)).length; - } - - it("migrates legacy steps and returns counts matching the created definitions", async () => { - await store.createWorkflowStep({ name: "On", description: "x", prompt: "p", defaultOn: true }); - await store.createWorkflowStep({ name: "Off", description: "y", prompt: "q", defaultOn: false }); - - const res = await post("/api/workflows/migrate-legacy-steps"); - expect(res.status).toBe(200); - const body = res.body as { migrated: number; skipped: number; combinedWorkflowId?: string }; - expect(body.migrated).toBe(2); - expect(body.skipped).toBe(0); - expect(body.combinedWorkflowId).toBeTruthy(); - - // 2 fragments + 1 combined workflow were actually persisted via the real store. - expect(await userDefCount()).toBe(3); - expect(await store.getDefaultWorkflowId()).toBe(body.combinedWorkflowId); - }); - - it("is idempotent: a second POST converts nothing and creates no new definitions", async () => { - await store.createWorkflowStep({ name: "On", description: "x", prompt: "p", defaultOn: true }); - - const first = (await post("/api/workflows/migrate-legacy-steps")).body as { migrated: number }; - expect(first.migrated).toBe(1); - const afterFirst = await userDefCount(); - - const res = await post("/api/workflows/migrate-legacy-steps"); - expect(res.status).toBe(200); - const body = res.body as { migrated: number; skipped: number; combinedWorkflowId?: string }; - expect(body.migrated).toBe(0); - expect(body.skipped).toBe(1); - expect(body.combinedWorkflowId).toBeUndefined(); - expect(await userDefCount()).toBe(afterFirst); - }); -}); diff --git a/packages/dashboard/src/routes/register-workflow-routes.ts b/packages/dashboard/src/routes/register-workflow-routes.ts index 3c65102d25..8921edcccd 100644 --- a/packages/dashboard/src/routes/register-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-workflow-routes.ts @@ -724,20 +724,10 @@ export function registerWorkflowRoutes(ctx: ApiRoutesContext): void { } }); - // POST /api/workflows/migrate-legacy-steps — Lazy idempotent migration of - // legacy user-authored workflow steps into fragments + a combined "Migrated - // steps" workflow (U2/R5/KTD-3). Fired once per project on first editor open; - // safe to call repeatedly (idempotent via per-row markers). Returns the counts. - router.post("/workflows/migrate-legacy-steps", async (req, res) => { - try { - const { store } = await getProjectContext(req); - const result = await store.migrateLegacyWorkflowSteps(); - res.json(result); - } catch (err: unknown) { - if (err instanceof ApiError) throw err; - rethrowAsApiError(err); - } - }); + // FNXC:WorkflowStepCRUD 2026-06-26-14:00: U7c removed POST + // /api/workflows/migrate-legacy-steps along with the legacy workflow_steps table and its + // store-level migrator. Workflow steps run graph-native; there is no legacy table to + // migrate from. // GET /api/workflows/:id/export — emit a portable, versioned JSON envelope for // a single workflow or fragment (U5/R9/KTD-5). Built-ins are exportable too — diff --git a/packages/engine/src/__tests__/merger-post-merge.test.ts b/packages/engine/src/__tests__/merger-post-merge.test.ts deleted file mode 100644 index 9b319cfd77..0000000000 --- a/packages/engine/src/__tests__/merger-post-merge.test.ts +++ /dev/null @@ -1,1143 +0,0 @@ -import { EventEmitter } from "node:events"; -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; - -// Mock external dependencies -vi.mock("../pi.js", () => ({ - createFnAgent: vi.fn(), - describeModel: vi.fn(() => "mock-provider/mock-model"), - promptWithFallback: vi.fn(async (session, prompt, options) => { - if (options === undefined) { - await session.prompt(prompt); - } else { - await session.prompt(prompt, options); - } - }), - compactSessionContext: vi.fn(), -})); - -// Route async `exec` through the `execSync` mock so existing tests that set up -// mockedExecSync.mockImplementation for verification commands (vitest run, -// pnpm build, etc.) keep working unchanged. `promisify(exec)` in merger.ts -// resolves/rejects based on the callback wired here. -vi.mock("node:child_process", async () => { - const { promisify } = await import("node:util"); - const { EventEmitter } = await import("node:events"); - const execSyncFn = vi.fn(); - const spawnFn = vi.fn((cmd: string, opts?: any) => { - const child = new EventEmitter() as any; - child.stdout = new EventEmitter(); - child.stderr = new EventEmitter(); - child.pid = 12345; - child.exitCode = null; - child.signalCode = null; - child.kill = vi.fn(); - queueMicrotask(() => { - try { - const out = execSyncFn(cmd, opts); - const stdout = out === undefined ? "" : out.toString(); - if (stdout) child.stdout.emit("data", Buffer.from(stdout)); - child.exitCode = 0; - child.emit("close", 0, null); - } catch (err) { - const error = err as { stdout?: string; stderr?: string; status?: number; code?: number }; - const stdout = error?.stdout?.toString?.() ?? ""; - const stderr = error?.stderr?.toString?.() ?? ""; - if (stdout) child.stdout.emit("data", Buffer.from(stdout)); - if (stderr) child.stderr.emit("data", Buffer.from(stderr)); - child.exitCode = error.status ?? error.code ?? 1; - child.emit("close", child.exitCode, null); - } - }); - return child; - }); - const execFn: any = vi.fn((cmd: any, opts: any, cb: any) => { - const callback = typeof opts === "function" ? opts : cb; - try { - const out = execSyncFn(cmd, { stdio: ["pipe", "pipe", "pipe"] }); - const stdout = out === undefined ? "" : out.toString(); - if (typeof callback === "function") callback(null, stdout, ""); - } catch (err: any) { - if (typeof callback === "function") { - callback(err, err?.stdout?.toString?.() ?? "", err?.stderr?.toString?.() ?? ""); - } - } - }); - // Mirror real child_process.exec: promisify resolves to { stdout, stderr }. - execFn[promisify.custom] = (cmd: any, opts?: any) => - new Promise((resolve, reject) => { - execFn(cmd, opts, (err: any, stdout: any, stderr: any) => { - if (err) { - err.stdout = stdout; - err.stderr = stderr; - reject(err); - } else { - resolve({ stdout, stderr }); - } - }); - }); - - // execFile(file, args, opts, cb) — reassemble a shell-equivalent command and - // delegate to execSyncFn so the same mock infrastructure handles both exec and execFile. - const execFileFn: any = vi.fn((file: any, args: any, opts: any, cb: any) => { - // Normalize overloads: (file, args, cb) or (file, args, opts, cb) - const callback = typeof opts === "function" ? opts : cb; - const options = typeof opts === "function" ? {} : opts; - const cmd = [file, ...(Array.isArray(args) ? args : [])].join(" "); - try { - const out = execSyncFn(cmd, { stdio: ["pipe", "pipe", "pipe"], ...options }); - const stdout = out === undefined ? "" : out.toString(); - if (typeof callback === "function") callback(null, stdout, ""); - } catch (err: any) { - if (typeof callback === "function") { - callback(err, err?.stdout?.toString?.() ?? "", err?.stderr?.toString?.() ?? ""); - } - } - }); - execFileFn[promisify.custom] = (file: any, args?: any, opts?: any) => - new Promise((resolve, reject) => { - execFileFn(file, args, opts, (err: any, stdout: any, stderr: any) => { - if (err) { - err.stdout = stdout; - err.stderr = stderr; - reject(err); - } else { - resolve({ stdout, stderr }); - } - }); - }); - - return { execSync: execSyncFn, exec: execFn, execFile: execFileFn, spawn: spawnFn }; -}); - -vi.mock("node:fs", () => ({ - existsSync: vi.fn().mockReturnValue(true), - readFileSync: vi.fn(), -})); - -vi.mock("../rate-limit-retry.js", () => ({ - withRateLimitRetry: (fn: () => Promise) => fn(), -})); - -vi.mock("../context-limit-detector.js", () => ({ - isContextLimitError: vi.fn(), -})); - -import { - aiMergeTask, - pushToRemoteAfterMerge, - findWorktreeUser, - detectResolvableConflicts, - autoResolveFile, - resolveConflicts, - classifyConflict, - getConflictedFiles, - isTrivialWhitespaceConflict, - resolveWithOurs, - resolveWithTheirs, - resolveTrivialWhitespace, - LOCKFILE_PATTERNS, - GENERATED_PATTERNS, - parseDiffStat, - extractFileScope, - validateDiffScope, - shouldSyncDependenciesForMerge, - summarizeVerificationOutput, - inferDefaultTestCommand, - resolveTaskDiffBaseRef, - commitOrAmendMergeWithFixes, - MergeAbortedError, - type ConflictCategory, -} from "../merger.js"; -import { mergerLog } from "../logger.js"; -import { createFnAgent } from "../pi.js"; -import { execSync, exec, spawn } from "node:child_process"; -import * as core from "@fusion/core"; -import { type TaskStore, type Task, type MergeResult, DEFAULT_SETTINGS } from "@fusion/core"; - -const mockedCreateFnAgent = vi.mocked(createFnAgent); -const mockedExecSync = vi.mocked(execSync); -const mockedExec = vi.mocked(exec); -const mockedSpawn = vi.mocked(spawn); -const { existsSync: mockedExistsSyncRaw, readFileSync: mockedReadFileSyncRaw } = await import("node:fs"); -const mockedExistsSync = vi.mocked(mockedExistsSyncRaw); -const mockedReadFileSync = vi.mocked(mockedReadFileSyncRaw); - -function createMockStore(taskOverrides: Partial = {}, allTasks: Task[] = []) { - const baseTask: Task = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - ...taskOverrides, - }; - - return { - getTask: vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }), - listTasks: vi.fn().mockResolvedValue(allTasks), - updateTask: vi.fn().mockResolvedValue(baseTask), - moveTask: vi.fn().mockResolvedValue(baseTask), - logEntry: vi.fn().mockResolvedValue(undefined), - appendAgentLog: vi.fn().mockResolvedValue(undefined), - updateSettings: vi.fn().mockResolvedValue({}), - // FNXC:WorkflowPostMerge 2026-06-26-12:00: U7b flipped graphNativePostMerge DEFAULT-ON, - // which makes the legacy merger post-merge path inert. These legacy-path tests opt OUT - // (flag:false) to keep exercising that code until U7c deletes it. The no-double-run test - // below asserts the default-ON behavior (merger skips post-merge entirely). - getSettings: vi.fn().mockResolvedValue({ - ...DEFAULT_SETTINGS, - mergeIntegrationWorktree: "cwd-main" as const, - experimentalFeatures: { graphNativePostMerge: false }, - }), - getActiveMergingTask: vi.fn().mockReturnValue(null), - emit: vi.fn(), - on: vi.fn(), - clearStaleExecutionStartBranchReferences: vi.fn().mockReturnValue([]), - getVerificationCacheHit: vi.fn().mockReturnValue(null), - recordVerificationCachePass: vi.fn(), - } as unknown as TaskStore; -} - -/** - * Set up execSync to handle the standard merge flow: - * rev-parse, log, diff, merge --squash, diff --cached --quiet (squash check), - * diff --cached (post-agent verify), branch -d - * - * Both `-X ours` and `-X theirs` final-fallback merges return success — the - * default settings strategy is "smart-prefer-main" (-X ours), but a few tests - * still exercise -X theirs explicitly via `mergeConflictStrategy: "smart-prefer-branch"`. - * - * For tests that want the merge to fail after 3 attempts, call - * setupFailingFallbackStrategy() instead. - */ -function setupHappyPathExecSync() { - mockedExecSync.mockImplementation((cmd: any) => { - const cmdStr = String(cmd); - if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123"); - if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123"; - if (cmdStr.includes("git log")) return "- feat: something" as any; - if (cmdStr.includes("merge-base")) return Buffer.from("abc123"); - if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any; - if (cmdStr.includes("merge --squash")) return Buffer.from(""); - if (cmdStr.includes("merge -X theirs --squash") || cmdStr.includes("merge -X ours --squash")) { - return Buffer.from(""); - } - // Post-squash check: --quiet means "did squash stage anything?" → "1" = yes - if (cmdStr.includes("diff --cached --quiet")) return "1" as any; - // Post-agent check: "did agent commit?" → "0" = yes - if (cmdStr.includes("diff --cached")) return "0" as any; - if (cmdStr.includes("show --shortstat")) return "3 files changed, 10 insertions(+), 2 deletions(-)" as any; - if (cmdStr.includes("branch -d") || cmdStr.includes("branch -D")) return Buffer.from(""); - if (cmdStr.includes("worktree remove")) return Buffer.from(""); - return Buffer.from(""); - }); -} - -/** - * Same as setupHappyPathExecSync but makes the final fallback merge fail - * (both `-X theirs` and `-X ours`). Use this for tests that expect the merge - * to throw after 3 attempts fail. - */ -function setupFailingFallbackStrategy() { - mockedExecSync.mockImplementation((cmd: any) => { - const cmdStr = String(cmd); - if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123"); - if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123"; - if (cmdStr.includes("git log")) return "- feat: something" as any; - if (cmdStr.includes("merge-base")) return Buffer.from("abc123"); - if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any; - if (cmdStr.includes("merge --squash")) return Buffer.from(""); - // -X theirs / -X ours should fail for these tests (they expect merge to throw) - if (cmdStr.includes("merge -X theirs --squash") || cmdStr.includes("merge -X ours --squash")) { - const err = new Error("fatal: git merge -X fallback failed with unresolved conflicts"); - err.name = "ExecSyncError"; - throw err; - } - // Post-squash check: --quiet means "did squash stage anything?" → "1" = yes - if (cmdStr.includes("diff --cached --quiet")) return "1" as any; - // Post-agent check: "did agent commit?" → "0" = yes - if (cmdStr.includes("diff --cached")) return "0" as any; - if (cmdStr.includes("show --shortstat")) return "3 files changed, 10 insertions(+), 2 deletions(-)" as any; - if (cmdStr.includes("branch -d") || cmdStr.includes("branch -D")) return Buffer.from(""); - if (cmdStr.includes("worktree remove")) return Buffer.from(""); - return Buffer.from(""); - }); -} - -/** @deprecated Renamed to setupFailingFallbackStrategy. */ -const setupFailingTheirsStrategy = setupFailingFallbackStrategy; - - -describe("aiMergeTask — post-merge workflow steps", () => { - beforeEach(() => { - vi.clearAllMocks(); - mockedExistsSync.mockReturnValue(true); - setupHappyPathExecSync(); - mockedCreateFnAgent.mockResolvedValue({ - session: { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: vi.fn(), - subscribe: vi.fn(), - on: vi.fn(), - state: {}, - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - }, - } as any); - }); - - it("runs post-merge workflow steps after successful merge", async () => { - const store = createMockStore(); - // Add getWorkflowStep to mock - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Notify", - description: "Send notifications after merge", - prompt: "Check the merged code and confirm all is well.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - // Override getTask to include enabledWorkflowSteps - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - const result = await aiMergeTask(store, "/tmp/root", "FN-050"); - - expect(result.merged).toBe(true); - - // getWorkflowStep should have been called for the post-merge step - expect((store as any).getWorkflowStep).toHaveBeenCalledWith("WS-001"); - - const postMergeAgentCall = mockedCreateFnAgent.mock.calls.find( - (c: any) => c[0]?.systemPrompt?.includes("post-merge workflow step agent"), - ); - expect(postMergeAgentCall).toBeDefined(); - expect(postMergeAgentCall?.[0]?.cwd).toMatch(/\.worktrees\/post-merge-FN-050-[a-z0-9]+/); - expect(postMergeAgentCall?.[0]?.cwd).not.toBe("/tmp/root"); - - // Task should still move to done and emit the canonical terminal event after post-merge steps run. - expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done"); - expect(store.emit).toHaveBeenCalledWith( - "task:merged", - expect.objectContaining({ merged: true, task: expect.objectContaining({ id: "FN-050" }) }), - ); - expect((store as any).getWorkflowStep.mock.invocationCallOrder[0]).toBeLessThan( - (store.emit as ReturnType).mock.invocationCallOrder[0], - ); - }); - - it("uses assigned agent runtime model for post-merge prompt step when workflow step has no override", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Notify", - description: "Send notifications after merge", - prompt: "Check merged code.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - assignedAgentId: "agent-001", - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - await aiMergeTask(store, "/tmp/root", "FN-050", { - agentStore: { - listAgents: vi.fn().mockResolvedValue([]), - getAgent: vi.fn().mockResolvedValue({ - id: "agent-001", - runtimeConfig: { - model: "anthropic/claude-3-5-sonnet-20241022", - }, - }), - } as any, - }); - - const postMergeAgentCall = mockedCreateFnAgent.mock.calls.find( - (c: any) => c[0]?.systemPrompt?.includes("post-merge workflow step agent"), - ); - expect(postMergeAgentCall?.[0]?.defaultProvider).toBe("anthropic"); - expect(postMergeAgentCall?.[0]?.defaultModelId).toBe("claude-3-5-sonnet-20241022"); - }); - - it("uses workflow-step model override over assigned agent runtime model", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Notify", - description: "Send notifications after merge", - prompt: "Check merged code.", - phase: "post-merge", - mode: "prompt", - modelProvider: "openai", - modelId: "gpt-4.1-mini", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - assignedAgentId: "agent-001", - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - await aiMergeTask(store, "/tmp/root", "FN-050", { - agentStore: { - listAgents: vi.fn().mockResolvedValue([]), - getAgent: vi.fn().mockResolvedValue({ - id: "agent-001", - runtimeConfig: { - model: "anthropic/claude-3-5-sonnet-20241022", - }, - }), - } as any, - }); - - const postMergeAgentCall = mockedCreateFnAgent.mock.calls.find( - (c: any) => c[0]?.systemPrompt?.includes("post-merge workflow step agent"), - ); - expect(postMergeAgentCall?.[0]?.defaultProvider).toBe("openai"); - expect(postMergeAgentCall?.[0]?.defaultModelId).toBe("gpt-4.1-mini"); - - const modelLogCall = (store.logEntry as ReturnType).mock.calls.find( - (call: any) => String(call[1]).includes("Workflow step 'Post-merge Notify' using model:"), - ); - expect(modelLogCall?.[1]).toContain("(workflow step override)"); - }); - - it("falls back to project default override model when no workflow-step or assigned-agent model is set", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Notify", - description: "Send notifications after merge", - prompt: "Check merged code.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - (store.getSettings as ReturnType).mockResolvedValue({ - ...DEFAULT_SETTINGS, - mergeIntegrationWorktree: "cwd-main" as const, - experimentalFeatures: { graphNativePostMerge: false }, - defaultProviderOverride: "openai", - defaultModelIdOverride: "gpt-4o-mini", - defaultProvider: "anthropic", - defaultModelId: "claude-3-5-haiku-latest", - }); - - await aiMergeTask(store, "/tmp/root", "FN-050"); - - const postMergeAgentCall = mockedCreateFnAgent.mock.calls.find( - (c: any) => c[0]?.systemPrompt?.includes("post-merge workflow step agent"), - ); - expect(postMergeAgentCall?.[0]?.defaultProvider).toBe("openai"); - expect(postMergeAgentCall?.[0]?.defaultModelId).toBe("gpt-4o-mini"); - }); - - it("does not run pre-merge workflow steps in merger", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Pre-merge Check", - description: "Check before merge", - prompt: "Run pre-merge checks.", - phase: "pre-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - await aiMergeTask(store, "/tmp/root", "FN-050"); - - // getWorkflowStep may be called but pre-merge steps should not trigger agent creation - // beyond the merge agent itself. We verify createFnAgent was called only once (merge agent) - // since pre-merge steps are skipped in the merger - const mergeAgentCalls = mockedCreateFnAgent.mock.calls.filter( - (c: any) => c[0]?.systemPrompt?.includes("You are a merge agent") - ); - const postMergeCalls = mockedCreateFnAgent.mock.calls.filter( - (c: any) => c[0]?.systemPrompt?.includes("post-merge") - ); - - // No post-merge agent should be created for a pre-merge step - expect(postMergeCalls).toHaveLength(0); - }); - - it("appends post-merge results to existing pre-merge results", async () => { - const existingPreMergeResults = [{ - workflowStepId: "WS-001", - workflowStepName: "Pre-merge Check", - phase: "pre-merge", - status: "passed", - output: "All good", - }]; - - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-002", - name: "Post-merge Verify", - description: "Verify after merge", - prompt: "Check merged state.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001", "WS-002"], - workflowStepResults: existingPreMergeResults, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - await aiMergeTask(store, "/tmp/root", "FN-050"); - - // Should have called updateTask with workflow results containing both pre and post - const updateCalls = (store.updateTask as ReturnType).mock.calls; - const resultsCall = updateCalls.find((c: any) => - Array.isArray(c[1]?.workflowStepResults) && c[1].workflowStepResults.length > 1 - ); - - if (resultsCall) { - const results = resultsCall[1].workflowStepResults; - // Should contain both pre-merge and post-merge results - expect(results.some((r: any) => r.phase === "pre-merge")).toBe(true); - expect(results.some((r: any) => r.phase === "post-merge")).toBe(true); - } - }); - - it("moves task to done even when post-merge step fails", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Fail", - description: "Will fail", - prompt: "Fail this check.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - // Make the post-merge agent throw - mockedCreateFnAgent.mockImplementation((async (opts: any) => { - if (opts.systemPrompt?.includes("post-merge")) { - return { - session: { - prompt: vi.fn().mockRejectedValue(new Error("Post-merge agent failed")), - dispose: vi.fn(), - subscribe: vi.fn(), - on: vi.fn(), - state: {}, - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - }, - }; - } - return { - session: { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: vi.fn(), - subscribe: vi.fn(), - on: vi.fn(), - state: {}, - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - }, - }; - }) as any); - - const result = await aiMergeTask(store, "/tmp/root", "FN-050"); - - // Merge should succeed regardless of post-merge step failure - expect(result.merged).toBe(true); - expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done"); - }); - - it("runs script-mode post-merge steps", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Build", - description: "Verify build passes", - phase: "post-merge", - mode: "script", - scriptName: "build", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - // Override settings to include scripts - (store.getSettings as ReturnType).mockResolvedValue({ - ...DEFAULT_SETTINGS, - mergeIntegrationWorktree: "cwd-main" as const, - experimentalFeatures: { graphNativePostMerge: false }, - scripts: { build: "pnpm build" }, - }); - - // Mock execSync to handle the script execution - mockedExecSync.mockImplementation((cmd: any) => { - const cmdStr = String(cmd); - if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123"); - if (cmdStr.includes("git log")) return "- feat: something" as any; - if (cmdStr.includes("merge-base")) return Buffer.from("abc123"); - if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any; - if (cmdStr.includes("merge --squash")) return Buffer.from(""); - if (cmdStr.includes("diff --cached --quiet")) return "1" as any; - if (cmdStr.includes("diff --cached")) return "0" as any; - if (cmdStr.includes("branch -d") || cmdStr.includes("branch -D")) return Buffer.from(""); - if (cmdStr.includes("worktree remove")) return Buffer.from(""); - if (cmdStr === "pnpm build") return "Build successful" as any; - return Buffer.from(""); - }); - - const result = await aiMergeTask(store, "/tmp/root", "FN-050"); - - const scriptSpawnCall = mockedSpawn.mock.calls.find((call: any) => String(call[0]) === "pnpm build"); - expect(scriptSpawnCall).toBeDefined(); - expect(scriptSpawnCall?.[2]?.cwd).toMatch(/\.worktrees\/post-merge-FN-050-[a-z0-9]+/); - - expect(result.merged).toBe(true); - expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done"); - }); - - it("creates temporary worktree for post-merge steps", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Notify", - description: "Send notifications after merge", - prompt: "Check the merged code and confirm all is well.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - await aiMergeTask(store, "/tmp/root", "FN-050"); - - const worktreeAddCall = mockedExec.mock.calls.find((call: any) => - String(call[0]).includes("git worktree add") && String(call[0]).includes("post-merge-FN-050-"), - ); - const worktreeRemoveCall = mockedExec.mock.calls.find((call: any) => - String(call[0]).includes("git worktree remove --force") && String(call[0]).includes("post-merge-FN-050-"), - ); - - expect(worktreeAddCall).toBeDefined(); - expect(worktreeRemoveCall).toBeDefined(); - }); - - it("runs configured worktreeInitCommand in the temporary post-merge worktree", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Notify", - description: "Send notifications after merge", - prompt: "Check merged code.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - (store.getSettings as ReturnType).mockResolvedValue({ - ...DEFAULT_SETTINGS, - mergeIntegrationWorktree: "cwd-main" as const, - experimentalFeatures: { graphNativePostMerge: false }, - worktreeInitCommand: "pnpm install", - }); - store.getTask = vi.fn().mockResolvedValue({ - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - prompt: "# test", - }); - - await aiMergeTask(store, "/tmp/root", "FN-050"); - - const initSpawnCall = mockedSpawn.mock.calls.find((call: any) => String(call[0]) === "pnpm install"); - expect(initSpawnCall).toBeDefined(); - expect(initSpawnCall?.[2]?.cwd).toMatch(/\.worktrees\/post-merge-FN-050-[a-z0-9]+/); - expect(store.logEntry).toHaveBeenCalledWith( - "FN-050", - expect.stringContaining("Post-merge worktree init command completed"), - "pnpm install", - ); - }); - - it("skips post-merge worktree init when worktreeInitCommand is not configured", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Notify", - description: "Send notifications after merge", - prompt: "Check merged code.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - store.getTask = vi.fn().mockResolvedValue({ - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - prompt: "# test", - }); - - await aiMergeTask(store, "/tmp/root", "FN-050"); - - const initSpawnCall = mockedSpawn.mock.calls.find((call: any) => String(call[0]) === "pnpm install"); - expect(initSpawnCall).toBeUndefined(); - expect(store.logEntry).not.toHaveBeenCalledWith( - "FN-050", - expect.stringContaining("Post-merge worktree init command completed"), - expect.anything(), - ); - }); - - it("keeps post-merge steps non-fatal when post-merge worktree init fails", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Notify", - description: "Send notifications after merge", - prompt: "Check merged code.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - (store.getSettings as ReturnType).mockResolvedValue({ - ...DEFAULT_SETTINGS, - mergeIntegrationWorktree: "cwd-main" as const, - experimentalFeatures: { graphNativePostMerge: false }, - worktreeInitCommand: "pnpm install", - }); - store.getTask = vi.fn().mockResolvedValue({ - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - prompt: "# test", - }); - mockedSpawn.mockImplementation(((cmd: string) => { - const child = new EventEmitter() as any; - child.stdout = new EventEmitter(); - child.stderr = new EventEmitter(); - child.pid = 12345; - child.exitCode = null; - child.signalCode = null; - child.kill = vi.fn(); - queueMicrotask(() => { - if (cmd === "pnpm install") { - child.stderr.emit("data", Buffer.from("install failed")); - child.exitCode = 1; - child.emit("close", 1, null); - } else { - child.exitCode = 0; - child.emit("close", 0, null); - } - }); - return child; - }) as any); - - const result = await aiMergeTask(store, "/tmp/root", "FN-050"); - - const postMergeAgentCall = mockedCreateFnAgent.mock.calls.find( - (c: any) => c[0]?.systemPrompt?.includes("post-merge workflow step agent"), - ); - expect(postMergeAgentCall).toBeDefined(); - expect(result.merged).toBe(true); - expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done"); - expect(store.logEntry).toHaveBeenCalledWith( - "FN-050", - expect.stringContaining("Post-merge worktree init command failed"), - expect.stringContaining("install failed"), - ); - }); - - it("falls back to rootDir when worktree creation fails", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Notify", - description: "Send notifications after merge", - prompt: "Check the merged code and confirm all is well.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - const baseExecImpl = mockedExecSync.getMockImplementation(); - mockedExecSync.mockImplementation((cmd: any, opts: any) => { - const cmdStr = String(cmd); - if (cmdStr.includes("git worktree add") && cmdStr.includes("post-merge-FN-050-")) { - const error: any = new Error("cannot create worktree"); - error.stderr = "cannot create worktree"; - throw error; - } - return baseExecImpl ? baseExecImpl(cmd, opts) : Buffer.from(""); - }); - - const warnSpy = vi.spyOn(mergerLog, "warn"); - await aiMergeTask(store, "/tmp/root", "FN-050"); - - const postMergeAgentCall = mockedCreateFnAgent.mock.calls.find( - (c: any) => c[0]?.systemPrompt?.includes("post-merge workflow step agent"), - ); - expect(postMergeAgentCall?.[0]?.cwd).toBe("/tmp/root"); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("could not create post-merge worktree — falling back to rootDir")); - expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done"); - }); - - it("cleans up temporary worktree even when post-merge step fails", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Fail", - description: "Will fail", - prompt: "Fail this check.", - phase: "post-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - mockedCreateFnAgent.mockImplementation((async (opts: any) => { - if (opts.systemPrompt?.includes("post-merge")) { - throw new Error("Post-merge agent creation failed"); - } - return { - session: { - prompt: vi.fn().mockResolvedValue(undefined), - dispose: vi.fn(), - subscribe: vi.fn(), - on: vi.fn(), - state: {}, - sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") }, - }, - }; - }) as any); - - await aiMergeTask(store, "/tmp/root", "FN-050"); - - const worktreeRemoveCall = mockedExec.mock.calls.find((call: any) => - String(call[0]).includes("git worktree remove --force") && String(call[0]).includes("post-merge-FN-050-"), - ); - expect(worktreeRemoveCall).toBeDefined(); - expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done"); - }); - - it("does not create post-merge worktree when no post-merge steps exist", async () => { - const store = createMockStore(); - (store as any).getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Pre-merge Check", - description: "Check before merge", - prompt: "Run pre-merge checks.", - phase: "pre-merge", - mode: "prompt", - enabled: true, - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - await aiMergeTask(store, "/tmp/root", "FN-050"); - - const worktreeAddCall = mockedExec.mock.calls.find((call: any) => - String(call[0]).includes("git worktree add") && String(call[0]).includes("post-merge-FN-050-"), - ); - expect(worktreeAddCall).toBeUndefined(); - }); - - /* - FNXC:WorkflowPostMerge 2026-06-26-12:00: - U7b cutover — graph is the SOLE post-merge owner. With graphNativePostMerge default-ON - (no explicit experimentalFeatures, i.e. exactly what production resolves), the merger must - NOT run any post-merge workflow step, create a post-merge worktree, or invoke a post-merge - agent — otherwise the step would DOUBLE-RUN (once here, once via the graph node). This is the - no-double-run proof. - */ - it("graph-native default-ON: merger does NOT run post-merge steps (no double-run)", async () => { - const store = createMockStore(); - // Default-ON: getSettings returns no experimentalFeatures override → flag resolves ON. - (store.getSettings as ReturnType).mockResolvedValue({ - ...DEFAULT_SETTINGS, - mergeIntegrationWorktree: "cwd-main" as const, - }); - const getWorkflowStep = vi.fn().mockResolvedValue({ - id: "WS-001", - name: "Post-merge Notify", - description: "Send notifications after merge", - prompt: "Check the merged code and confirm all is well.", - phase: "post-merge", - mode: "prompt", - enabled: true, - toolMode: "coding", - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }); - (store as any).getWorkflowStep = getWorkflowStep; - - const baseTask = { - id: "FN-050", - title: "Test task", - description: "Test", - column: "in-review", - dependencies: [], - worktree: "/tmp/root/.worktrees/KB-050", - steps: [], - currentStep: 0, - log: [], - enabledWorkflowSteps: ["WS-001"], - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - }; - store.getTask = vi.fn().mockResolvedValue({ ...baseTask, prompt: "# test" }); - - const result = await aiMergeTask(store, "/tmp/root", "FN-050"); - - // Merge still succeeds and the task completes. - expect(result.merged).toBe(true); - expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done"); - - // The legacy post-merge path is fully inert: no post-merge worktree, no post-merge agent. - const worktreeAddCall = mockedExec.mock.calls.find((call: any) => - String(call[0]).includes("git worktree add") && String(call[0]).includes("post-merge-FN-050-"), - ); - expect(worktreeAddCall).toBeUndefined(); - const postMergeAgentCall = mockedCreateFnAgent.mock.calls.find( - (c: any) => c[0]?.systemPrompt?.includes("post-merge"), - ); - expect(postMergeAgentCall).toBeUndefined(); - }); -}); - -// ── Merge Details Collection Tests ───────────────────────────────────── - diff --git a/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts b/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts index 04f2827379..4844a09225 100644 --- a/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts @@ -3,7 +3,6 @@ import { EventEmitter } from "node:events"; import type { Settings, TaskStore, Task } from "@fusion/core"; import { cleanupOrphanedWorktrees } from "../../worktree-pool.js"; import { SelfHealingManager } from "../../self-healing.js"; -import { mergerTestHooks } from "../../merger.js"; import { NativeWorktreeBackend, WorktrunkWorktreeBackend } from "../../worktree-backend.js"; const { execSpy, existsSpy, readdirSpy } = vi.hoisted(() => ({ @@ -23,25 +22,6 @@ vi.mock("node:fs", async (importOriginal) => { }); -function mockWorktreeRemoveFailure(postMergePath: string, porcelainOutput: string): void { - execSpy.mockImplementation((cmd: string, _opts: unknown, cb: (err: any, stdout: string, stderr: string) => void) => { - if (cmd.includes("git worktree remove")) { - const stderr = `fatal: validation failed, cannot remove working tree: '${postMergePath}/.git' is not a .git file, error code 2`; - cb(Object.assign(new Error(stderr), { stderr, status: 2 }), "", stderr); - return; - } - if (cmd === "git worktree prune") { - cb(null, "", ""); - return; - } - if (cmd === "git worktree list --porcelain") { - cb(null, porcelainOutput, ""); - return; - } - cb(null, "", ""); - }); -} - function storeForSelfHealing(settings: Partial, task: Partial): TaskStore & EventEmitter { const emitter = new EventEmitter(); return Object.assign(emitter, { @@ -65,47 +45,6 @@ describe("reliability interactions: worktrunk worktree removal routing", () => { vi.restoreAllMocks(); }); - it("merger post-merge cleanup calls worktrunk backend remove and avoids native git remove", async () => { - const removeSpy = vi.spyOn(WorktrunkWorktreeBackend.prototype, "remove").mockResolvedValue(undefined); - - await mergerTestHooks.removePostMergeWorktree("/repo", "/repo/.worktrees/post", "FN-100", { - worktrunk: { enabled: true, binaryPath: "worktrunk", onFailure: "fail" } as any, - }); - - expect(removeSpy).toHaveBeenCalledWith(expect.objectContaining({ rootDir: "/repo", worktreePath: "/repo/.worktrees/post", taskId: "FN-100" })); - expect(execSpy.mock.calls.some((call) => String(call[0]).includes("git worktree remove"))).toBe(false); - }); - - it("merger post-merge cleanup logs harmless classified temp residue when porcelain is absent after prune", async () => { - const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); - const postMergePath = "/repo/.worktrees/post-merge-FN-343-abcd1234"; - mockWorktreeRemoveFailure(postMergePath, "worktree /repo\nbranch refs/heads/main\n"); - - await mergerTestHooks.removePostMergeWorktree("/repo", postMergePath, "FN-343", {}); - - expect(execSpy.mock.calls.map((call) => String(call[0]))).toEqual([ - `git worktree remove --force "${postMergePath}"`, - "git worktree prune", - "git worktree list --porcelain", - ]); - expect(warnSpy).toHaveBeenCalledWith( - expect.stringContaining("post-merge worktree cleanup classified harmless"), - ); - }); - - it("merger post-merge cleanup keeps still-registered temp worktree failures visible", async () => { - const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); - const postMergePath = "/repo/.worktrees/post-merge-FN-343-abcd1234"; - mockWorktreeRemoveFailure(postMergePath, `worktree /repo\nbranch refs/heads/main\n\nworktree ${postMergePath}\nbranch refs/heads/fusion/fn-343\n`); - - await mergerTestHooks.removePostMergeWorktree("/repo", postMergePath, "FN-343", {}); - - expect(execSpy.mock.calls.map((call) => String(call[0]))).toContain("git worktree list --porcelain"); - expect(warnSpy).toHaveBeenCalledWith( - expect.stringContaining(`failed to remove post-merge worktree ${postMergePath}`), - ); - }); - it("self-healing recover path calls worktrunk backend remove and not native remove", async () => { const removeSpy = vi.spyOn(WorktrunkWorktreeBackend.prototype, "remove").mockResolvedValue(undefined); const task = { diff --git a/packages/engine/src/__tests__/sandbox-wiring-audit.test.ts b/packages/engine/src/__tests__/sandbox-wiring-audit.test.ts index 1dfeeed32c..67a595b95b 100644 --- a/packages/engine/src/__tests__/sandbox-wiring-audit.test.ts +++ b/packages/engine/src/__tests__/sandbox-wiring-audit.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import type { RunAuditEventInput, Routine, RoutineStore, TaskStore } from "@fusion/core"; import { __runConfiguredCommandForTests } from "../executor.js"; -import { __executePostMergeScriptStepForTests } from "../merger.js"; import { createRunAuditor } from "../run-audit.js"; import { RoutineRunner } from "../routine-runner.js"; import { __resetSandboxBackendForTests, __setSandboxBackendForTests } from "../sandbox/index.js"; @@ -44,28 +43,6 @@ describe("sandbox wiring audit emissions", () => { expect(store.events.some((event) => event.domain === "sandbox" && event.mutationType === "sandbox:failure")).toBe(true); }); - it("emits sandbox:run for merger script-mode execution", async () => { - const store = new AuditStoreStub(); - const auditor = createRunAuditor(store as unknown as TaskStore, { - runId: "run-merge-1", - agentId: "merger", - taskId: "FN-4640", - phase: "merge", - }); - - const response = await __executePostMergeScriptStepForTests( - {} as TaskStore, - "FN-4640", - { id: "ws-1", name: "post", type: "script", scriptName: "ok" } as any, - process.cwd(), - { scripts: { ok: "node -e \"process.stdout.write('ok')\"" } } as any, - auditor, - ); - - expect(response.success).toBe(true); - expect(store.events.some((event) => event.domain === "sandbox" && event.mutationType === "sandbox:run")).toBe(true); - }); - it("emits sandbox:prepare and sandbox:run for routine-runner command execution", async () => { __setSandboxBackendForTests({ capabilities: () => ({ id: "native", supportsNetworkPolicy: false, supportsFilesystemPolicy: false, supportsStreaming: true, platform: "any" }), diff --git a/packages/engine/src/__tests__/sandbox-wiring.test.ts b/packages/engine/src/__tests__/sandbox-wiring.test.ts index 654c43a1e4..9c01e64897 100644 --- a/packages/engine/src/__tests__/sandbox-wiring.test.ts +++ b/packages/engine/src/__tests__/sandbox-wiring.test.ts @@ -1,7 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { __runConfiguredCommandForTests } from "../executor.js"; -import { __executePostMergeScriptStepForTests } from "../merger.js"; import { RoutineRunner } from "../routine-runner.js"; import { __resetSandboxBackendForTests, @@ -69,38 +68,6 @@ describe("sandbox wiring", () => { expect(result.spawnError).toBeInstanceOf(Error); }); - it("routes merger executePostMergeScriptStep through sandbox backend", async () => { - const run = vi.fn().mockResolvedValue({ - stdout: "", - stderr: "", - exitCode: 0, - signal: null, - timedOut: false, - bufferExceeded: false, - }); - __setSandboxBackendForTests(makeStub({ run })); - const controller = new AbortController(); - - const result = await __executePostMergeScriptStepForTests( - { updateTask: vi.fn() } as any, - "FN-1", - { scriptName: "post" } as any, - "/tmp/worktree", - { scripts: { post: "echo post" } } as any, - undefined, - controller.signal, - ); - - expect(result.success).toBe(true); - expect(run).toHaveBeenCalledWith("echo post", { - cwd: "/tmp/worktree", - encoding: "utf-8", - timeoutMs: 120_000, - maxBuffer: 10 * 1024 * 1024, - signal: controller.signal, - }); - }); - it("routes routine runner command branch through sandbox backend", async () => { const run = vi.fn().mockResolvedValue({ stdout: "routine", diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index abeb57e92d..82b0b4caa3 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -3715,7 +3715,18 @@ export class TaskExecutor { */ async recoverFailedPreMergeWorkflowStep(task: Task): Promise { try { - const preMergeFailed = (task.workflowStepResults ?? []) + /* + FNXC:WorkflowPostMerge 2026-06-26-14:00: + U7c: gate-ness is now sourced from the recorded `WorkflowStepResult.status`, NOT a + `workflow_steps` table read. The graph executor (workflow-graph-executor.ts) maps a + group outcome to status by gate semantics: a GATE REVISE / hard failure records + `status: "failed"` (blocking), while an ADVISORY REVISE records `status: + "advisory_failure"` (non-blocking). So a pre-merge result with `status === "failed"` + IS by construction a blocking gate failure — the prior `getWorkflowStep(id).gateMode` + lookup was redundant (and after the table drop it returned undefined for graph node + ids anyway). Recovery revives the task from the latest blocking pre-merge failure. + */ + const failed = (task.workflowStepResults ?? []) .filter((r) => (r.phase || "pre-merge") === "pre-merge" && r.status === "failed") .sort((a, b) => { const aTs = Date.parse(a.completedAt || a.startedAt || ""); @@ -3723,18 +3734,6 @@ export class TaskExecutor { return (Number.isFinite(bTs) ? bTs : 0) - (Number.isFinite(aTs) ? aTs : 0); }); - const gateModeCache = new Map(); - const failed: typeof preMergeFailed = []; - for (const result of preMergeFailed) { - let mode = gateModeCache.get(result.workflowStepId); - if (!mode) { - const step = await this.store.getWorkflowStep(result.workflowStepId).catch(() => null); - mode = step?.gateMode || (step?.mode === "script" ? "gate" : "advisory"); - gateModeCache.set(result.workflowStepId, mode); - } - if (mode === "gate") failed.push(result); - } - const target = failed[0]; if (!target) { executorLog.warn(`${task.id}: no failed pre-merge workflow step to recover from`); diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index d4068a7d83..41b687a985 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -64,7 +64,6 @@ import { readInstallMarker, writeInstallMarker, } from "./merge-dependency-sync.js"; -import { resolveTaskWorktreePath } from "./worktree-paths.js"; import { resolveTaskWorkingBranch } from "./worktree-names.js"; import { collectOwnTaskCommitsForRange, @@ -93,8 +92,6 @@ import { type AutostashOutcome, type MergeResult, type MergeDetails, - type WorkflowStep, - type WorkflowStepResult, type Settings, type AgentPromptsConfig, type CanonicalMergeConflictStrategy, @@ -107,8 +104,6 @@ import { type AutostashOrphanRecord, normalizeMergeAdvanceAutoSyncMode, isMergeRequestContractShadowEnabled, - isExperimentalFeatureEnabled, - GRAPH_NATIVE_POST_MERGE_FLAG, } from "@fusion/core"; import { evaluateAutoMergeFactProviders } from "./auto-merge-fact-providers.js"; import { resolveMergePolicy, type MergeFileScopeMode } from "./merge-trait.js"; @@ -140,7 +135,6 @@ import { } from "./merger-squash-audit.js"; import { detectMergeOverlap, restoreBranchWinsFiles } from "./merger-overlap-guard.js"; import { checkDiffVolume, DiffVolumeRegressionError } from "./merger-diff-volume-gate.js"; -import { ReadonlyViolationError, filterCustomToolsForReadonly } from "./workflow-step-tool-policy.js"; import { detectAlreadyLandedOnMain, type AlreadyMergedDetectionStrategy } from "./already-merged-detector.js"; import { decideAutoPrerebase, probeDivergence, runAutoPrerebase } from "./merger-auto-prerebase.js"; import { @@ -339,7 +333,6 @@ const DEPENDENCY_SYNC_TRIGGER_PATTERNS = [ "packages/*/package.json", ]; -const WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS = 4_000; const PULL_REBASE_TIMEOUT_MS = 120_000; const PUSH_TIMEOUT_MS = 60_000; @@ -384,11 +377,6 @@ const MERGE_USER_COMMENTS_MAX_CHARS = 4000; */ export const summarizeVerificationOutputLocal = summarizeVerificationOutput; -function truncateWorkflowScriptOutput(output: string): string { - if (output.length <= WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS) return output; - return `... output truncated to last ${WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS} characters ...\n${output.slice(-WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS)}`; -} - /** Check if a path matches a glob pattern (simple glob support: * and **) */ export function matchGlob(path: string, pattern: string): boolean { // Handle ** which matches across directory boundaries (must do before single *) @@ -540,43 +528,6 @@ export function shouldSyncDependenciesForMerge( ); } -type MergeWorktreeCommandResult = Awaited>; - -const POST_MERGE_INIT_OUTCOME_MAX_CHARS = 2_000; - -function mergeWorktreeCommandErrorMessage(result: { spawnError?: string | Error; timedOut?: boolean; exitCode?: number | null }): string { - if (result.spawnError) return `Failed to start command: ${result.spawnError}`; - if (result.timedOut) return "Command timed out"; - return `Command exited with code ${result.exitCode ?? "unknown"}`; -} - -function truncatePostMergeInitOutput(output: string): string { - if (output.length <= POST_MERGE_INIT_OUTCOME_MAX_CHARS) return output; - return `... output truncated to last ${POST_MERGE_INIT_OUTCOME_MAX_CHARS} chars ...\n${output.slice(-POST_MERGE_INIT_OUTCOME_MAX_CHARS)}`; -} - -function formatPostMergeInitFailureOutcome(initResult: MergeWorktreeCommandResult | undefined, err: unknown): string { - const stderr = initResult?.stderr?.trim(); - if (stderr) return truncatePostMergeInitOutput(stderr); - - const stdout = initResult?.stdout?.trim(); - if (stdout) return truncatePostMergeInitOutput(stdout); - - if (initResult?.spawnError) { - return typeof initResult.spawnError === "string" ? initResult.spawnError : initResult.spawnError.message; - } - - const parts: string[] = []; - if (initResult?.timedOut) parts.push("Command timed out"); - if (initResult?.exitCode !== undefined && initResult.exitCode !== null) parts.push(`exit code: ${initResult.exitCode}`); - if (initResult?.signal) parts.push(`signal: ${initResult.signal}`); - if (parts.length > 0) return parts.join("; "); - - if (err instanceof Error && err.message.trim().length > 0) return err.message; - - const fallback = String(err).trim(); - return fallback.length > 0 ? fallback : "Command failed"; -} async function syncDependenciesForMerge( store: TaskStore, @@ -7396,88 +7347,12 @@ export async function pushToRemoteAfterMerge( } } -/** - * Create a temporary worktree from the current HEAD for isolated post-merge step execution. - * Returns the worktree path, or null if creation fails (graceful fallback to rootDir). - */ -async function createPostMergeWorktree( - rootDir: string, - taskId: string, - settings: Partial, -): Promise { - const randomSuffix = Math.random().toString(36).slice(2, 10); - const postMergeWorktree = resolveTaskWorktreePath(rootDir, settings, `post-merge-${taskId}-${randomSuffix}`); - - try { - await execAsync(`git worktree add ${quoteArg(postMergeWorktree)} HEAD`, { cwd: rootDir }); - return postMergeWorktree; - } catch (err: unknown) { - mergerLog.warn(`${taskId}: failed to create post-merge worktree: ${getCommandErrorMessage(err)}`); - return null; - } -} - -async function runPostMergeWorktreeInitCommand( - store: TaskStore, - taskId: string, - postMergeWorktree: string, - settings: Partial, - audit?: RunAuditor, -): Promise { - const initCommand = getConfiguredWorktreeInitCommand(settings); - if (!initCommand) return; - - const initStartedAt = Date.now(); - let initResult: MergeWorktreeCommandResult | undefined; - try { - initResult = await runConfiguredMergeWorktreeCommand(initCommand, postMergeWorktree, 300_000, undefined, audit); - if (initResult.spawnError || initResult.timedOut || initResult.exitCode !== 0) { - throw new Error(mergeWorktreeCommandErrorMessage(initResult)); - } - await store.logEntry(taskId, `[timing] Post-merge worktree init command completed in ${Date.now() - initStartedAt}ms`, initCommand); - } catch (err: unknown) { - if (err instanceof Error && err.name === "AbortError") { - throw err; - } - await store.logEntry(taskId, `[timing] Post-merge worktree init command failed after ${Date.now() - initStartedAt}ms`); - const message = err instanceof Error ? err.message : String(err); - const outcome = formatPostMergeInitFailureOutcome(initResult, err); - mergerLog.warn(`${taskId}: post-merge worktree init command failed — post-merge workflow steps will still run: ${message}`); - await store.logEntry(taskId, `Post-merge worktree init command failed (post-merge workflow steps will still run): ${message}`, outcome); - } -} - -/** - * Remove a temporary worktree created for post-merge step execution. - * Non-fatal: logs and swallows errors. - */ -async function removePostMergeWorktree( - rootDir: string, - postMergeWorktree: string, - taskId: string, - settings: Partial, - audit?: RunAuditor, -): Promise { - try { - const outcome = await removeWorktree({ - rootDir, - worktreePath: postMergeWorktree, - settings, - taskId, - reason: RemovalReason.MergerPostMerge, - audit, - }); - if ("harmless" in outcome && outcome.harmless) { - mergerLog.warn(`${taskId}: post-merge worktree cleanup classified harmless for ${postMergeWorktree}: ${outcome.message}`); - } - } catch (err: unknown) { - mergerLog.warn(`${taskId}: failed to remove post-merge worktree ${postMergeWorktree}: ${getCommandErrorMessage(err)}`); - } -} - -export const mergerTestHooks = { - removePostMergeWorktree, -}; +/* +FNXC:WorkflowPostMerge 2026-06-26-14:00: +U7c removed the merger-side post-merge execution path entirely (worktree creation + +init-command + prompt/script step execution + isolated-worktree cleanup). Post-merge +workflow steps run exclusively as the workflow graph's own post-merge optional-group node. +*/ /** * AI-powered merge with 3-attempt retry logic when autoResolveConflicts is enabled. @@ -10815,35 +10690,12 @@ export async function aiMergeTask( } } - // 7. Run post-merge workflow steps (in temporary worktree for isolation) - // FNXC:WorkflowPostMerge 2026-06-26-12:00: U7b cutover — when graph-native post-merge - // is active (now default-ON) the GRAPH is the sole post-merge runner; this legacy merger - // path is inert so post-merge steps never double-run. `settings` (project-resolved) gates - // the no-op; an explicit opt-out (`graphNativePostMerge: false`) restores the legacy path. - throwIfAborted(options.signal, taskId); - const hasPostMergeSteps = await hasEnabledPostMergeWorkflowSteps(store, taskId, task.enabledWorkflowSteps, settings); - if (hasPostMergeSteps) { - const postMergeWorktree = await createPostMergeWorktree(rootDir, taskId, settings); - const postMergeCwd = postMergeWorktree || rootDir; - if (postMergeWorktree) { - await runPostMergeWorktreeInitCommand(store, taskId, postMergeWorktree, settings, audit); - mergerLog.log(`${taskId}: running post-merge workflow steps in isolated worktree: ${postMergeWorktree}`); - } else { - mergerLog.warn(`${taskId}: could not create post-merge worktree — falling back to rootDir`); - } - - try { - await runPostMergeWorkflowSteps(store, taskId, rootDir, postMergeCwd, settings, options, audit); - } catch (err: any) { - rethrowIfMergeAborted(err); - mergerLog.error(`${taskId}: post-merge workflow steps error: ${err.message}`); - // Non-fatal — task still moves to done - } finally { - if (postMergeWorktree) { - await removePostMergeWorktree(rootDir, postMergeWorktree, taskId, settings, audit); - } - } - } + // 7. Post-merge workflow steps run graph-native. + // FNXC:WorkflowPostMerge 2026-06-26-14:00: U7c — the legacy merger post-merge execution + // path (worktree creation + prompt/script step execution) has been REMOVED. Post-merge + // workflow steps run exclusively as the workflow graph's own post-merge optional-group + // node, which records into `task.workflowStepResults`. The graph is the single post-merge + // owner; there is no longer a merger-side path to double-run or gate behind a flag. // 8. Clean up worktree throwIfAborted(options.signal, taskId); @@ -12461,163 +12313,6 @@ export function buildMergePrompt(params: MergePromptParams): string { return parts.join("\n"); } -async function hasEnabledPostMergeWorkflowSteps( - store: TaskStore, - taskId: string, - enabledWorkflowSteps: string[] | undefined, - settings?: Settings, -): Promise { - // FNXC:WorkflowPostMerge 2026-06-26-12:00: U7b cutover — graph owns post-merge when the - // flag is on (default). Report "no post-merge steps" so the merger skips worktree creation - // and execution; the graph runs the equivalent post-merge graph node exactly once. - if (isExperimentalFeatureEnabled(settings, GRAPH_NATIVE_POST_MERGE_FLAG)) return false; - if (!enabledWorkflowSteps?.length) return false; - - for (const wsId of enabledWorkflowSteps) { - try { - const ws = await store.getWorkflowStep(wsId); - if (!ws) continue; - const stepPhase = ws.phase || "pre-merge"; - // readonly review steps always run pre-merge to reuse the coding worktree — see FN-2185 post-mortem. - if (stepPhase === "post-merge" && ws.toolMode !== "readonly") { - return true; - } - } catch (err: unknown) { - mergerLog.warn(`${taskId}: failed to inspect workflow step ${wsId} for post-merge phase: ${getCommandErrorMessage(err)}`); - } - } - - return false; -} - -/** - * Run post-merge workflow steps for a task after the merge succeeds. - * Steps execute in an isolated worktree (created from merged HEAD) to prevent - * modifications to the main project directory. Falls back to rootDir if worktree - * creation fails. Failures are logged but do NOT block task completion. - */ -async function runPostMergeWorkflowSteps( - store: TaskStore, - taskId: string, - rootDir: string, - cwd: string, - settings: Settings, - mergeOptions: MergerOptions = {}, - auditor?: RunAuditor, -): Promise { - throwIfAborted(mergeOptions.signal, taskId); - // FNXC:WorkflowPostMerge 2026-06-26-12:00: U7b cutover — defensive no-op (the call site's - // hasEnabledPostMergeWorkflowSteps already gates entry). When graph-native post-merge is - // active the graph is the sole runner; never execute legacy post-merge steps here to avoid - // double-running. Removed entirely in U7c. - if (isExperimentalFeatureEnabled(settings, GRAPH_NATIVE_POST_MERGE_FLAG)) return; - const task = await store.getTask(taskId); - if (!task.enabledWorkflowSteps?.length) return; - - // Get existing pre-merge results to append to - const existingResults: WorkflowStepResult[] = task.workflowStepResults || []; - - for (const wsId of task.enabledWorkflowSteps) { - const ws = await store.getWorkflowStep(wsId); - if (!ws) { - mergerLog.log(`${taskId}: [post-merge] workflow step ${wsId} not found — skipping`); - continue; - } - - // Normalize legacy steps: undefined phase → "pre-merge" - const stepPhase = ws.phase || "pre-merge"; - - // Only run post-merge steps here. - // readonly review steps always run pre-merge to reuse the coding worktree — see FN-2185 post-mortem. - if (stepPhase !== "post-merge" || ws.toolMode === "readonly") continue; - - // Normalize legacy steps without mode to prompt-mode - const stepMode: "prompt" | "script" = ws.mode || "prompt"; - - // Skip validation per mode - if (stepMode === "prompt" && !ws.prompt?.trim()) { - await store.logEntry(taskId, `[post-merge] Workflow step '${ws.name}' has no prompt — skipping`); - existingResults.push({ - workflowStepId: ws.id, - workflowStepName: ws.name, - phase: "post-merge", - status: "skipped", - output: "No prompt configured for this workflow step", - }); - await store.updateTask(taskId, { workflowStepResults: existingResults }); - continue; - } - - if (stepMode === "script" && !ws.scriptName?.trim()) { - await store.logEntry(taskId, `[post-merge] Workflow step '${ws.name}' has no scriptName — skipping`); - existingResults.push({ - workflowStepId: ws.id, - workflowStepName: ws.name, - phase: "post-merge", - status: "skipped", - output: "No scriptName configured for this workflow step", - }); - await store.updateTask(taskId, { workflowStepResults: existingResults }); - continue; - } - - await store.logEntry(taskId, `[post-merge] Starting workflow step: ${ws.name} (${stepMode} mode)`); - mergerLog.log(`${taskId}: [post-merge] running workflow step: ${ws.name} (${stepMode} mode)`); - - const startedAt = new Date().toISOString(); - - try { - const result = stepMode === "script" - ? await executePostMergeScriptStep(store, taskId, ws, cwd, settings, auditor, mergeOptions.signal) - : await executePostMergePromptStep(store, taskId, ws, rootDir, cwd, settings, mergeOptions); - const completedAt = new Date().toISOString(); - - if (result.success) { - await store.logEntry(taskId, `[post-merge] Workflow step completed: ${ws.name}`); - mergerLog.log(`${taskId}: [post-merge] workflow step passed: ${ws.name}`); - existingResults.push({ - workflowStepId: ws.id, - workflowStepName: ws.name, - phase: "post-merge", - status: "passed", - output: result.output, - startedAt, - completedAt, - }); - } else { - // Post-merge failures are logged but do NOT block task completion - await store.logEntry(taskId, `[post-merge] Workflow step failed: ${ws.name}`, result.error || "Unknown error"); - mergerLog.error(`${taskId}: [post-merge] workflow step failed: ${ws.name}; output captured in task log`); - existingResults.push({ - workflowStepId: ws.id, - workflowStepName: ws.name, - phase: "post-merge", - status: "failed", - output: result.error || "Workflow step failed", - startedAt, - completedAt, - }); - } - } catch (err: any) { - const completedAt = new Date().toISOString(); - await store.logEntry(taskId, `[post-merge] Workflow step error: ${ws.name}`, err.message || "Unknown error"); - mergerLog.error(`${taskId}: [post-merge] workflow step error: ${ws.name} — ${err.message}`); - existingResults.push({ - workflowStepId: ws.id, - workflowStepName: ws.name, - phase: "post-merge", - status: "failed", - output: err.message || "Workflow step error", - startedAt, - completedAt, - }); - } - - // Save results after each step (partial results preserved on crash) - await store.updateTask(taskId, { workflowStepResults: existingResults }); - } -} - function getPostMergeScriptSandboxBackend(auditor?: RunAuditor): SandboxBackend { return resolveSandboxBackend({ auditor }); } @@ -12657,228 +12352,6 @@ async function runConfiguredMergeWorktreeCommand( }; } -/** Execute a script-mode post-merge workflow step in the provided execution directory. */ -async function executePostMergeScriptStep( - store: TaskStore, - taskId: string, - workflowStep: WorkflowStep, - cwd: string, - settings: Settings, - auditor?: RunAuditor, - signal?: AbortSignal, -): Promise<{ success: boolean; output?: string; error?: string }> { - const scriptName = workflowStep.scriptName!.trim(); - const scripts = settings.scripts || {}; - const scriptCommand = scripts[scriptName]; - - if (!scriptCommand) { - return { success: false, error: `Script '${scriptName}' not found in project settings` }; - } - - const backend = getPostMergeScriptSandboxBackend(auditor); - const result = await backend.run(scriptCommand, { - cwd, - encoding: "utf-8", - timeoutMs: 120_000, - maxBuffer: 10 * 1024 * 1024, - ...(signal !== undefined && { signal }), - }); - - if (result.exitCode === 0 && !result.signal && !result.timedOut && !result.bufferExceeded && !result.spawnError) { - return { success: true, output: `Script '${scriptName}' completed successfully` }; - } - - const stderr = result.stderr.trim(); - const stdout = result.stdout.trim(); - const parts: string[] = []; - if (result.spawnError) { - parts.push(result.spawnError.message); - } else { - if (result.exitCode !== null) parts.push(`Exit code: ${result.exitCode}`); - if (stdout) parts.push(`stdout: ${truncateWorkflowScriptOutput(stdout)}`); - if (stderr) parts.push(`stderr: ${truncateWorkflowScriptOutput(stderr)}`); - } - if (!parts.length) parts.push("Unknown error"); - return { success: false, error: parts.join("\n") }; -} - -export async function __executePostMergeScriptStepForTests( - store: TaskStore, - taskId: string, - workflowStep: WorkflowStep, - cwd: string, - settings: Settings, - auditor?: RunAuditor, - signal?: AbortSignal, -): Promise<{ success: boolean; output?: string; error?: string }> { - return executePostMergeScriptStep(store, taskId, workflowStep, cwd, settings, auditor, signal); -} - -/** Execute a prompt-mode post-merge workflow step using an AI agent in the provided execution directory. */ -async function executePostMergePromptStep( - store: TaskStore, - taskId: string, - workflowStep: WorkflowStep, - rootDir: string, - cwd: string, - settings: Settings, - mergeOptions: MergerOptions = {}, -): Promise<{ success: boolean; output?: string; error?: string }> { - const toolMode: "coding" | "readonly" = workflowStep.toolMode || "readonly"; - const systemPrompt = `You are a post-merge workflow step agent executing: ${workflowStep.name} - -Task Context: -- Task ID: ${taskId} -- The merge has already been completed successfully. -- You are running in a temporary worktree with the merged code. - -Your role: -- Execute this step exactly as requested. -- Validate outcomes against evidence in the merged tree. -- Report findings in clear, actionable language with file-level references when possible. - -Your Instructions: -${workflowStep.prompt} - -You have access to the file system to review the merged changes. -When your review is complete and everything looks good, simply state your findings. -If issues are found that need attention, describe them clearly and include concrete remediation direction.`; - - const agentLogger = new AgentLogger({ - store, - taskId, - agent: "merger", - persistAgentToolOutput: settings.persistAgentToolOutput, - // Merger agents are task-scoped ephemeral workers. - persistAgentThinkingLog: resolvePersistAgentThinkingLog(settings, { ephemeral: true }), - }); - - try { - // Build skill selection context for post-merge session - let postMergeSkillContext = undefined; - let taskForSkillContext: Awaited> | null = null; - if (mergeOptions.agentStore) { - try { - taskForSkillContext = await store.getTask(taskId); - postMergeSkillContext = await buildSessionSkillContext({ - agentStore: mergeOptions.agentStore, - task: taskForSkillContext, - sessionPurpose: "merger", - projectRootDir: rootDir, - pluginRunner: mergeOptions.pluginRunner, - }); - } catch { - // Graceful fallback - no skill selection - } - } - - const assignedAgentId = taskForSkillContext?.assignedAgentId?.trim(); - const agentStoreWithGetAgent = mergeOptions.agentStore && typeof (mergeOptions.agentStore as { getAgent?: unknown }).getAgent === "function" - ? mergeOptions.agentStore - : null; - const assignedAgent = assignedAgentId && agentStoreWithGetAgent - ? await agentStoreWithGetAgent.getAgent(assignedAgentId).catch(() => null) - : null; - const mergerSessionModel = resolveMergerSessionModel(settings, assignedAgent?.runtimeConfig); - const stepProvider = workflowStep.modelProvider || mergerSessionModel.provider; - const stepModelId = workflowStep.modelId || mergerSessionModel.modelId; - const useOverride = !!(workflowStep.modelProvider && workflowStep.modelId); - - // Post-merge step agents inherit merger instructions - let postMergeInstructions = ""; - if (mergeOptions.agentStore) { - try { - const agents = await mergeOptions.agentStore.listAgents({ role: "merger" }); - for (const agent of agents) { - if (agent.instructionsText || agent.instructionsPath) { - postMergeInstructions = await resolveAgentInstructions(agent, rootDir); - break; - } - } - } catch { - // Graceful fallback - } - } - const postMergeSystemPrompt = buildSystemPromptWithInstructions(systemPrompt, postMergeInstructions); - - const mergerRuntimeHint = extractRuntimeHint(assignedAgent?.runtimeConfig); - const readonlyCustomTools = toolMode === "readonly" - ? filterCustomToolsForReadonly([]) - : { allowed: [] as ToolDefinition[], denied: [] as string[] }; - if (toolMode === "readonly" && readonlyCustomTools.denied.length > 0) { - await store.logEntry( - taskId, - `[readonly-violation] Post-merge workflow step '${workflowStep.name}' dropped denied custom tools: ${readonlyCustomTools.denied.join(", ")}`, - ); - } - const { session } = await createResolvedAgentSession({ - sessionPurpose: "merger", - runtimeHint: mergerRuntimeHint, - pluginRunner: mergeOptions.pluginRunner, - cwd, - systemPrompt: postMergeSystemPrompt, - tools: toolMode, - defaultProvider: stepProvider, - defaultModelId: stepModelId, - fallbackProvider: settings.fallbackProvider, - fallbackModelId: settings.fallbackModelId, - defaultThinkingLevel: settings.defaultThinkingLevel, - runAuditor: createRunAuditor(store, { - runId: generateSyntheticRunId("merge", taskId), - agentId: "merger", - taskId, - phase: "merge", - source: "merger", - }), - settings, - // Skill selection: use assigned agent skills if available, otherwise role fallback - ...(postMergeSkillContext?.skillSelectionContext ? { skillSelection: postMergeSkillContext.skillSelectionContext } : {}), - ...(readonlyCustomTools.allowed.length > 0 ? { customTools: readonlyCustomTools.allowed } : {}), - taskId, - onFallbackModelUsed: createFallbackModelObserver({ - agent: "merger", - label: `post-merge workflow step '${workflowStep.name}'`, - store, - taskId, - }), - }); - - mergerLog.log(`${taskId}: [post-merge] workflow step '${workflowStep.name}' using model ${describeModel(session)}${useOverride ? " (workflow step override)" : ""}`); - await store.logEntry(taskId, `[post-merge] Workflow step '${workflowStep.name}' using model: ${describeModel(session)}${useOverride ? " (workflow step override)" : ""}`); - - let output = ""; - session.subscribe((event) => { - if (event.type === "message_update") { - const msgEvent = event.assistantMessageEvent; - if (msgEvent.type === "text_delta") { - output += msgEvent.delta; - } - } - }); - - await promptWithFallback( - session, - `Execute the post-merge workflow step "${workflowStep.name}" for task ${taskId}.\n\n` + - `Review the merged code in the temporary worktree and evaluate it against your instructions.`, - ); - - checkSessionError(session); - await accumulateSessionTokenUsage(store, taskId, session); - session.dispose(); - await agentLogger.flush(); - - return { success: true, output }; - } catch (err: any) { - await agentLogger.flush(); - if ((err instanceof ReadonlyViolationError) || err?.code === "READONLY_VIOLATION") { - const deniedTool = err?.toolName || "unknown"; - await store.logEntry(taskId, `[readonly-violation] Post-merge workflow step '${workflowStep.name}' attempted denied tool '${deniedTool}'`); - return { success: false, error: `[readonly-violation] ${err?.message ?? "Readonly policy violation"}` }; - } - return { success: false, error: err.message }; - } -} - async function completeTask( store: TaskStore, taskId: string,