feat(FN-7039): graph is sole post-merge owner + migration 130 normalizes enable ids

Makes graph-native post-merge the default and cuts the merger's legacy post-merge
path over so post-merge runs exactly once (via the graph), with migration data prep.

- experimentalFeatures.graphNativePostMerge → default ON; merger
  hasEnabledPostMergeWorkflowSteps/runPostMergeWorkflowSteps become inert when on
  (no double-run; proven by a new no-double-run test). Merger code kept (U7c removes it).
- Migration 130 (SCHEMA_VERSION 129→130) rewrites each task's enabledWorkflowSteps
  entries that are legacy built-in pre-merge workflow_steps row ids → the optional-group
  node id (browser-verification/code-review); dedupes; idempotent; identity-stable;
  leaves node-ids/compiled/custom entries untouched. Table KEPT (U7c drops it).
- Investigation (real DBs): NO custom/plugin post-merge steps exist; the only post-merge
  step is compound-engineering's 'document' graph node — so the merger no-op strands
  nothing. Custom-step re-pointing was verified unnecessary and skipped.

Safe-to-drop in U7c still blocked by live readers: merger post-merge fns, store CRUD,
migrateLegacyWorkflowSteps/readConfig materialization (executor recovery reader is
already null-safe→advisory).

Plan U7b.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-06-26 01:30:24 -07:00
parent 194878dac9
commit 9a2e8a7260
7 changed files with 314 additions and 6 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Post-merge workflow steps now run once via the workflow graph instead of the merger.
category: internal
dev: Flips `experimentalFeatures.graphNativePostMerge` DEFAULT-ON so the graph is the sole post-merge owner; the legacy merger post-merge path (`runPostMergeWorkflowSteps`/`hasEnabledPostMergeWorkflowSteps`) is inert under the flag (kept until U7c). DB migration 130 rewrites legacy compiled `workflow_steps` enable ids (templateId ∈ built-in optional-group ids: browser-verification, code-review) to the graph node ids in tasks' `enabledWorkflowSteps` (idempotent, de-duped). `workflow_steps` table is retained.

View File

@@ -0,0 +1,101 @@
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { SCHEMA_VERSION } from "../db.js";
import { BROWSER_VERIFICATION_GROUP_ID } from "../builtin-browser-verification-group.js";
import { CODE_REVIEW_GROUP_ID } from "../builtin-code-review-group.js";
import { createTaskStoreTestHarness } from "./store-test-helpers.js";
/*
FNXC:WorkflowPostMerge 2026-06-26-12:00:
Migration 130 (U7b post-merge graph-native cutover) — legacy enable-id normalization.
A task's enabledWorkflowSteps must reference GRAPH node ids so the graph enables the right
optional-group node now that the graph is the single post-merge owner. Legacy DBs may hold
compiled `workflow_steps` row ids (WS-xxx) whose templateId is a built-in optional-group node
id (browser-verification / code-review). This test seeds a DB at the old schema (129) with
exactly that legacy shape and asserts init() rewrites the WS-row id to the node id, de-dups,
leaves already-node-id and compiled-workflow entries untouched, and is idempotent across reopen.
*/
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:*)
function insertWorkflowStep(
db: { prepare: (sql: string) => { run: (...args: unknown[]) => unknown } },
args: { id: string; templateId: string; name: string; phase: string },
): void {
const now = new Date().toISOString();
db.prepare(
`INSERT OR REPLACE INTO workflow_steps
(id, templateId, name, description, mode, phase, prompt, gateMode, toolMode, enabled, defaultOn, createdAt, updatedAt)
VALUES (?, ?, ?, ?, 'prompt', ?, 'x', 'advisory', 'coding', 1, 0, ?, ?)`,
).run(args.id, args.templateId, args.name, args.name, args.phase, now, now);
}
describe("Migration 130: post-merge cutover enable-id normalization", () => {
const harness = createTaskStoreTestHarness();
beforeEach(async () => {
await harness.beforeEach();
});
afterEach(async () => {
await harness.afterEach();
});
it("rewrites legacy built-in optional-group WS ids to graph node ids, de-dups, and leaves node-id/compiled entries untouched", async () => {
await harness.reopenDiskBackedStore();
const store = harness.store();
const task = await harness.createTestTask();
const db = store.getDatabase();
// Legacy compiled row for the browser-verification optional group, plus a compiled-workflow
// materialization row (templateId workflow:*) that must NOT be rewritten.
insertWorkflowStep(db as any, { id: WS_BV, templateId: BROWSER_VERIFICATION_GROUP_ID, name: "Browser Verification", phase: "pre-merge" });
insertWorkflowStep(db as any, { id: WS_DOC, templateId: "workflow:builtin:compound-engineering", name: "Document learnings", phase: "post-merge" });
// Legacy enable set: a WS-row id to rewrite, the SAME node id already present (dedup target),
// an already-correct node id, and a compiled-workflow row id (left as-is).
const legacyEnabled = [WS_BV, BROWSER_VERIFICATION_GROUP_ID, CODE_REVIEW_GROUP_ID, WS_DOC];
db.prepare("UPDATE tasks SET enabledWorkflowSteps = ? WHERE id = ?").run(JSON.stringify(legacyEnabled), task.id);
// Roll the DB back to the pre-migration schema so init() replays migration 130.
db.prepare("UPDATE __meta SET value = '129' WHERE key = 'schemaVersion'").run();
await harness.reopenDiskBackedStore();
const migratedDb = harness.store().getDatabase();
expect(migratedDb.getSchemaVersion()).toBe(SCHEMA_VERSION);
const row = migratedDb.prepare("SELECT enabledWorkflowSteps FROM tasks WHERE id = ?").get(task.id) as { enabledWorkflowSteps: string };
const enabled = JSON.parse(row.enabledWorkflowSteps) as string[];
// WS_BV → browser-verification node id; duplicate browser-verification collapsed; code-review
// kept; compiled-workflow row id (WS_DOC) left untouched. Order preserved.
expect(enabled).toEqual([BROWSER_VERIFICATION_GROUP_ID, CODE_REVIEW_GROUP_ID, WS_DOC]);
// No raw WS-row id for a built-in optional group survives.
expect(enabled).not.toContain(WS_BV);
});
it("is idempotent: a second init() makes no further change", async () => {
await harness.reopenDiskBackedStore();
const store = harness.store();
const task = await harness.createTestTask();
const db = store.getDatabase();
insertWorkflowStep(db as any, { id: WS_BV, templateId: BROWSER_VERIFICATION_GROUP_ID, name: "Browser Verification", phase: "pre-merge" });
db.prepare("UPDATE tasks SET enabledWorkflowSteps = ? WHERE id = ?").run(JSON.stringify([WS_BV]), task.id);
db.prepare("UPDATE __meta SET value = '129' WHERE key = 'schemaVersion'").run();
await harness.reopenDiskBackedStore();
const afterFirst = JSON.parse(
(harness.store().getDatabase().prepare("SELECT enabledWorkflowSteps FROM tasks WHERE id = ?").get(task.id) as { enabledWorkflowSteps: string }).enabledWorkflowSteps,
) as string[];
expect(afterFirst).toEqual([BROWSER_VERIFICATION_GROUP_ID]);
// Reopen again (already at SCHEMA_VERSION → migration 130 does not re-run; value is stable).
await harness.reopenDiskBackedStore();
const afterSecond = JSON.parse(
(harness.store().getDatabase().prepare("SELECT enabledWorkflowSteps FROM tasks WHERE id = ?").get(task.id) as { enabledWorkflowSteps: string }).enabledWorkflowSteps,
) as string[];
expect(afterSecond).toEqual([BROWSER_VERIFICATION_GROUP_ID]);
});
});

View File

@@ -17,6 +17,11 @@ import { DEFAULT_PROJECT_SETTINGS } from "./types.js";
import type { PluginOnSchemaInit } from "./plugin-types.js";
import type { SteeringComment, TaskComment } from "./types.js";
import { hasTitleIdDrift, normalizeTitleForTaskId } from "./task-title-id-drift.js";
// FNXC:WorkflowPostMerge 2026-06-26-12:00: built-in optional-group node ids — the stable
// per-task enable keys on the graph. Migration 130 rewrites legacy WS-row ids whose
// templateId is one of these to the node id so the graph enables the right optional group.
import { BROWSER_VERIFICATION_GROUP_ID } from "./builtin-browser-verification-group.js";
import { CODE_REVIEW_GROUP_ID } from "./builtin-code-review-group.js";
// ── Types ────────────────────────────────────────────────────────────
@@ -162,7 +167,7 @@ export function isFts5CorruptionError(error: unknown): boolean {
// ── Schema Definition ────────────────────────────────────────────────
const SCHEMA_VERSION = 129;
const SCHEMA_VERSION = 130;
const TASKS_FTS_AUTOMERGE = 8;
const TASKS_FTS_CRISISMERGE = 16;
@@ -5339,6 +5344,77 @@ export class Database {
});
}
// Migration 130: post-merge graph-native cutover (U7b) — legacy enable-id normalization.
// FNXC:WorkflowPostMerge 2026-06-26-12:00:
// Graph-native post-merge is now default-ON and the graph is the single post-merge owner.
// A task's `enabledWorkflowSteps` must reference GRAPH node ids so the graph enables the
// right optional-group node. Legacy data may still hold compiled `workflow_steps` row ids
// (WS-xxx) whose `templateId` is a built-in optional-group node id (e.g. `browser-verification`,
// `code-review`). Rewrite each such entry to that node id. Entries that are already node ids,
// compiled-workflow materialization rows (templateId `workflow:*`), and custom rows are left
// untouched. Data DML, so wrapped in a transaction; identity-stable + idempotent (a second
// run finds node ids already in place and no WS-row left to rewrite). The `workflow_steps`
// table is intentionally KEPT (dropped in U7c once all readers are gone).
if (version < 130) {
this.applyMigration(130, () => {
const optionalGroupNodeIds = new Set<string>([
BROWSER_VERIFICATION_GROUP_ID,
CODE_REVIEW_GROUP_ID,
]);
// Map every workflow_steps row id → templateId, but only retain rows whose templateId
// is a built-in optional-group node id (the only legacy ids we rewrite).
const wsRows = this.db
.prepare("SELECT id, templateId FROM workflow_steps WHERE templateId IS NOT NULL")
.all() as Array<{ id: string; templateId: string | null }>;
const wsIdToNodeId = new Map<string, string>();
for (const row of wsRows) {
if (row.templateId && optionalGroupNodeIds.has(row.templateId)) {
wsIdToNodeId.set(row.id, row.templateId);
}
}
if (wsIdToNodeId.size === 0) return; // nothing legacy to rewrite
const taskRows = this.db
.prepare("SELECT id, enabledWorkflowSteps FROM tasks WHERE enabledWorkflowSteps IS NOT NULL AND enabledWorkflowSteps NOT IN ('', '[]')")
.all() as Array<{ id: string; enabledWorkflowSteps: string | null }>;
const update = this.db.prepare("UPDATE tasks SET enabledWorkflowSteps = ? WHERE id = ?");
this.db.exec("BEGIN");
try {
for (const task of taskRows) {
let parsed: unknown;
try {
parsed = JSON.parse(task.enabledWorkflowSteps ?? "[]");
} catch {
continue; // corrupt JSON: leave untouched
}
if (!Array.isArray(parsed)) continue;
let changed = false;
const seen = new Set<string>();
const rewritten: string[] = [];
for (const entry of parsed) {
if (typeof entry !== "string") continue;
const mapped = wsIdToNodeId.get(entry);
const next = mapped ?? entry;
if (mapped) changed = true;
if (seen.has(next)) {
// De-dup: rewriting WS-xxx → node id can collide with an already-present node id.
changed = true;
continue;
}
seen.add(next);
rewritten.push(next);
}
if (changed) update.run(JSON.stringify(rewritten), task.id);
}
this.db.exec("COMMIT");
} catch (err) {
this.db.exec("ROLLBACK");
throw err;
}
});
}
}
/**

View File

@@ -11,7 +11,18 @@ workflowGraphExecutor and workflowColumns graduated from Experimental. Runtime g
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.
*/
const DEFAULT_ON_EXPERIMENTAL_FEATURES = new Set<string>();
/*
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.
*/
const DEFAULT_ON_EXPERIMENTAL_FEATURES = new Set<string>(["graphNativePostMerge"]);
const RETIRED_EXPERIMENTAL_FEATURES = new Set<string>([
"workflowInterpreterDualObserve",
]);

View File

@@ -186,9 +186,14 @@ function createMockStore(taskOverrides: Partial<Task> = {}, allTasks: Task[] = [
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(),
@@ -482,6 +487,7 @@ describe("aiMergeTask — post-merge workflow steps", () => {
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
...DEFAULT_SETTINGS,
mergeIntegrationWorktree: "cwd-main" as const,
experimentalFeatures: { graphNativePostMerge: false },
defaultProviderOverride: "openai",
defaultModelIdOverride: "gpt-4o-mini",
defaultProvider: "anthropic",
@@ -695,6 +701,7 @@ describe("aiMergeTask — post-merge workflow steps", () => {
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
...DEFAULT_SETTINGS,
mergeIntegrationWorktree: "cwd-main" as const,
experimentalFeatures: { graphNativePostMerge: false },
scripts: { build: "pnpm build" },
});
@@ -783,6 +790,7 @@ describe("aiMergeTask — post-merge workflow steps", () => {
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
...DEFAULT_SETTINGS,
mergeIntegrationWorktree: "cwd-main" as const,
experimentalFeatures: { graphNativePostMerge: false },
worktreeInitCommand: "pnpm install",
});
store.getTask = vi.fn().mockResolvedValue({
@@ -869,6 +877,7 @@ describe("aiMergeTask — post-merge workflow steps", () => {
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
...DEFAULT_SETTINGS,
mergeIntegrationWorktree: "cwd-main" as const,
experimentalFeatures: { graphNativePostMerge: false },
worktreeInitCommand: "pnpm install",
});
store.getTask = vi.fn().mockResolvedValue({
@@ -1066,6 +1075,68 @@ describe("aiMergeTask — post-merge workflow steps", () => {
);
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<typeof vi.fn>).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 ─────────────────────────────────────

View File

@@ -132,15 +132,20 @@ describe("WorkflowGraphExecutor graph-native post-merge steps", () => {
expect(recorder.results[0].status).toBe("advisory_failure");
});
it("flag OFF (default): the post-merge node is NOT run via the graph and records nothing", async () => {
it("flag explicitly OFF (opt-out): the post-merge node is NOT run via the graph and records nothing", async () => {
const recorder = makeRecorder();
const executor = new WorkflowGraphExecutor({
handlers: { prompt: handler("APPROVE") },
recordWorkflowStepResult: recorder.record,
});
// No experimentalFeatures at all → flag defaults OFF.
const result = await executor.run(taskWith([POST_MERGE_ID]), {}, postMergeIr());
// FNXC:WorkflowPostMerge 2026-06-26-12:00: U7b cutover flipped the DEFAULT to ON, so the
// OFF path is now an explicit opt-out (graphNativePostMerge:false), not the default.
const result = await executor.run(
taskWith([POST_MERGE_ID]),
{ experimentalFeatures: { graphNativePostMerge: false } },
postMergeIr(),
);
expect(result.outcome).toBe("success");
expect(result.visitedNodeIds).toContain("merge");
@@ -148,4 +153,25 @@ describe("WorkflowGraphExecutor graph-native post-merge steps", () => {
expect(result.visitedNodeIds).not.toContain(POST_MERGE_ID);
expect(recorder.results).toHaveLength(0);
});
it("flag DEFAULT (no experimentalFeatures): runs the post-merge node via the graph (default-ON, U7b)", async () => {
const recorder = makeRecorder();
const executor = new WorkflowGraphExecutor({
handlers: { prompt: handler("APPROVE") },
recordWorkflowStepResult: recorder.record,
});
// No experimentalFeatures at all → flag now defaults ON after the U7b cutover.
const result = await executor.run(taskWith([POST_MERGE_ID]), {}, postMergeIr());
expect(result.outcome).toBe("success");
expect(result.visitedNodeIds).toContain("merge");
// Default-ON: the post-merge node IS traversed after the collapsed merge seam.
expect(result.visitedNodeIds.indexOf(POST_MERGE_ID)).toBeGreaterThan(
result.visitedNodeIds.indexOf("merge"),
);
expect(recorder.results).toHaveLength(1);
expect(recorder.results[0].phase).toBe("post-merge");
expect(recorder.results[0].status).toBe("passed");
});
});

View File

@@ -107,6 +107,8 @@ 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";
@@ -10814,8 +10816,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);
const hasPostMergeSteps = await hasEnabledPostMergeWorkflowSteps(store, taskId, task.enabledWorkflowSteps, settings);
if (hasPostMergeSteps) {
const postMergeWorktree = await createPostMergeWorktree(rootDir, taskId, settings);
const postMergeCwd = postMergeWorktree || rootDir;
@@ -12459,7 +12465,12 @@ async function hasEnabledPostMergeWorkflowSteps(
store: TaskStore,
taskId: string,
enabledWorkflowSteps: string[] | undefined,
settings?: Settings,
): Promise<boolean> {
// 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) {
@@ -12495,6 +12506,11 @@ async function runPostMergeWorkflowSteps(
auditor?: RunAuditor,
): Promise<void> {
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;