diff --git a/packages/core/src/__tests__/builtin-traits.test.ts b/packages/core/src/__tests__/builtin-traits.test.ts index 2a819656d8..10f0e76527 100644 --- a/packages/core/src/__tests__/builtin-traits.test.ts +++ b/packages/core/src/__tests__/builtin-traits.test.ts @@ -42,11 +42,19 @@ describe("built-in traits", () => { expect(r.getTrait("gate")?.hooks?.gate).toBe(true); }); - it("merge trait ships a config STUB shape (behavior is U7)", () => { + it("merge trait config schema matches the U7 policy fields", () => { const r = freshRegistry(); - const keys = (r.getTrait("merge")?.configSchema?.fields ?? []).map((f) => f.key).sort(); - expect(keys).toEqual(["conflictStrategy", "fileScope", "squash", "strategy"]); + const fields = r.getTrait("merge")?.configSchema?.fields ?? []; + const keys = fields.map((f) => f.key).sort(); + // U7 tightened the schema: strategy enum, fileScope enum (incl. custom), + // custom-rules array, squash posture, conflictStrategy. + expect(keys).toEqual(["conflictStrategy", "fileScope", "rules", "squash", "strategy"]); expect(r.getTrait("merge")?.flags.mergeOrchestration).toBe(true); + + const strategy = fields.find((f) => f.key === "strategy"); + expect(strategy?.enumValues).toEqual(["always-squash", "auto", "always-rebase", "pr-only"]); + const fileScope = fields.find((f) => f.key === "fileScope"); + expect(fileScope?.enumValues).toEqual(["strict", "warn", "off", "custom"]); }); it("hold trait's release config matches WorkflowHoldRelease kinds", () => { diff --git a/packages/core/src/builtin-traits.ts b/packages/core/src/builtin-traits.ts index 769297575c..957b6735cc 100644 --- a/packages/core/src/builtin-traits.ts +++ b/packages/core/src/builtin-traits.ts @@ -142,7 +142,7 @@ export const BUILTIN_TRAIT_DEFINITIONS: TraitDefinition[] = [ id: "merge", name: "Merge", description: - "Enqueues onto the merge-request queue; configures merge policy. Behavior is U7 — this is a config STUB.", + "Enqueues onto the merge-request queue; configures merge policy (U7). The lost-work guard trio stays capability-level and is unreachable from this config (KTD-6).", builtin: true, flags: { mergeOrchestration: true }, hooks: { onEnter: true, onExit: true }, @@ -151,15 +151,28 @@ export const BUILTIN_TRAIT_DEFINITIONS: TraitDefinition[] = [ { key: "strategy", type: "enum", - enumValues: ["squash", "merge-commit", "rebase", "pr-only"], + // Direct-merge commit strategies (`DirectMergeCommitStrategy`) plus + // `pr-only` (maps onto `mergeStrategy: "pull-request"`). Absent → + // settings read-through (back-compat for the default workflow). + enumValues: ["always-squash", "auto", "always-rebase", "pr-only"], description: "Merge strategy", }, { key: "fileScope", type: "enum", + // strict = throw on zero-overlap (today); warn = log + proceed (audit + // carries the violating file list); off = skip the throw + emit one + // per-merge "scope enforcement disabled" audit (per-task scopeOverride + // is a documented no-op here); custom = evaluate `rules` in place of + // the task's File Scope section. enumValues: ["strict", "warn", "off", "custom"], description: "File-scope enforcement mode", }, + { + key: "rules", + type: "array", + description: "Custom file-scope glob/path rules (used when fileScope === 'custom')", + }, { key: "squash", type: "boolean", description: "Squash posture" }, { key: "conflictStrategy", diff --git a/packages/engine/src/__tests__/merge-trait.test.ts b/packages/engine/src/__tests__/merge-trait.test.ts new file mode 100644 index 0000000000..80fa0741c9 --- /dev/null +++ b/packages/engine/src/__tests__/merge-trait.test.ts @@ -0,0 +1,630 @@ +/** + * U7 — Merge trait behavior (R10). + * + * Covers every U7 plan scenario: + * - each `strategy` value routes to the merger behavior it names, incl. + * `pr-only` (enqueue-with-prState marker, documented below); + * - `fileScope` off / warn / strict / custom behaviors incl. audit payloads; + * - lost-work guard trio regression: config CANNOT reach the three guards; + * - merge completion drives the next column via the queue callback, not + * inline; + * - a queued merge surviving restart resumes from SQLite state (fixture). + * + * Fast: mock stores + a single in-memory `TaskStore` (no real git, no real + * merges). No process spawns; no fake-timer-dependent waits. + * + * PR-ONLY DESIGN DECISION: there is no PR-creation path inside the merge queue + * in this codebase — the merge-queue worker loop runs `aiMergeTask` (a direct + * merge). So `pr-only` is implemented as a *routing flag* on the resolved + * policy (`pullRequestOnly: true`), consistent with the existing + * `settings.mergeStrategy === "pull-request"` posture: `merger.ts` skips + * direct-merge commit routing exactly as it does for the pull-request setting. + * The card still enqueues onto the same persisted merge-request queue; the + * pr-state marker is the existing pr-monitor machinery's concern. This is the + * narrowest change that makes `pr-only` behave like the pull-request route + * without reimplementing merge mechanics. + */ + +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { execSync } from "node:child_process"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + DEFAULT_SETTINGS, + TaskStore, + getTraitRegistry, + type Settings, + type Task, + type WorkflowIr, +} from "@fusion/core"; + +import { + resolveMergePolicy, + registerMergeTraitHooks, + __resetMergeTraitRegistrationForTests, +} from "../merge-trait.js"; +import { + assertSquashOverlapsFileScope, + enforceSquashFileScopeInvariant, + FileScopeViolationError, +} from "../merger.js"; + +// NOTE: this file deliberately does NOT import `./merger-test-helpers.js` — that +// module installs a module-scope `vi.mock("node:child_process")` that would +// break the real `git` operations the real-`TaskStore` fixtures here rely on. +// The file-scope tests use a real git repo and stage real files instead, so the +// merger's real `git diff --cached --name-only` returns the staged set. + +// ── helpers ────────────────────────────────────────────────────────────────── + +function settingsWith(overrides: Partial): Settings { + return { ...DEFAULT_SETTINGS, ...overrides } as Settings; +} + +/** Initialize a temp dir as a git repo with a `.fusion` dir so `createTask` + * (which writes `task.json`) works against a real `TaskStore`. */ +async function initRepo(rootDir: string): Promise { + const run = (cmd: string) => execSync(cmd, { cwd: rootDir, stdio: "pipe" }); + run("git init -b main"); + run('git config user.email "test@example.com"'); + run('git config user.name "Test User"'); + await writeFile(join(rootDir, "README.md"), "# fixture\n", "utf-8"); + run("git add README.md"); + run('git commit -m "chore: init"'); + await mkdir(join(rootDir, ".fusion"), { recursive: true }); +} + +/** A linear custom workflow whose `in-review` column carries a merge trait with + * the given config. Linear so `selectTaskWorkflow` compiles it. */ +function customMergeWorkflowIr(mergeConfig: Record): WorkflowIr { + return { + version: "v2", + name: "custom-merge-wf", + columns: [ + { id: "triage", name: "Triage", traits: [{ trait: "intake" }] }, + { id: "in-progress", name: "In progress", traits: [{ trait: "wip" }] }, + { + id: "in-review", + name: "In review", + traits: [{ trait: "merge", config: mergeConfig }], + }, + { id: "done", name: "Done", traits: [{ trait: "complete" }] }, + ], + nodes: [ + { id: "start", kind: "start", column: "triage" }, + { id: "execute", kind: "prompt", column: "in-progress", config: { seam: "execute" } }, + { id: "merge", kind: "prompt", column: "in-review", config: { seam: "merge" } }, + { id: "end", kind: "end", column: "done" }, + ], + edges: [ + { from: "start", to: "execute" }, + { from: "execute", to: "merge", condition: "success" }, + { from: "merge", to: "end", condition: "success" }, + { from: "execute", to: "end", condition: "failure" }, + { from: "merge", to: "end", condition: "failure" }, + ], + }; +} + +// ── 1. strategy routing (resolveMergePolicy) ───────────────────────────────── + +describe("resolveMergePolicy — strategy routing", () => { + beforeEach(() => vi.clearAllMocks()); + + // Flag-OFF resolution returns before touching the store (settings passed in). + const noStore = {} as never; + + it("flag OFF: falls back to settings (directMergeCommitStrategy + mergeStrategy)", async () => { + const settings = settingsWith({ + mergeStrategy: "direct", + directMergeCommitStrategy: "always-rebase", + }); + const policy = await resolveMergePolicy(noStore, { id: "FN-1", column: "in-review" }, settings); + expect(policy.source).toBe("settings"); + expect(policy.commitStrategy).toBe("always-rebase"); + expect(policy.pullRequestOnly).toBe(false); + }); + + it("flag OFF + pull-request setting: pullRequestOnly true via settings", async () => { + const settings = settingsWith({ mergeStrategy: "pull-request" }); + const policy = await resolveMergePolicy(noStore, { id: "FN-1", column: "in-review" }, settings); + expect(policy.pullRequestOnly).toBe(true); + expect(policy.source).toBe("settings"); + }); + + it.each([ + ["always-squash", "always-squash", false], + ["auto", "auto", false], + ["always-rebase", "always-rebase", false], + ] as const)( + "flag ON: merge trait strategy '%s' resolves to commitStrategy '%s'", + async (strategy, expectedStrategy, expectedPrOnly) => { + const fx = await makeStoreFixture(); + try { + await fx.selectCustomMergeWorkflow({ strategy }); + const policy = await resolveMergePolicy(fx.store, { id: fx.taskId, column: "in-review" }); + expect(policy.source).toBe("workflow"); + expect(policy.commitStrategy).toBe(expectedStrategy); + expect(policy.pullRequestOnly).toBe(expectedPrOnly); + } finally { + await fx.cleanup(); + } + }, + ); + + it("flag ON: merge trait strategy 'pr-only' sets pullRequestOnly (PR-route, no direct merge)", async () => { + const fx = await makeStoreFixture(); + try { + await fx.selectCustomMergeWorkflow({ strategy: "pr-only" }); + const policy = await resolveMergePolicy(fx.store, { id: fx.taskId, column: "in-review" }); + expect(policy.source).toBe("workflow"); + expect(policy.pullRequestOnly).toBe(true); + } finally { + await fx.cleanup(); + } + }); + + it("flag ON but default workflow (no merge config): resolves entirely from settings", async () => { + const fx = await makeStoreFixture(); + try { + // No custom workflow selected → default workflow → merge trait has no + // config → settings read-through (verbatim back-compat). The flag is + // already ON from the fixture; set the project-level strategy. + await fx.store.updateSettings(settingsWith({ directMergeCommitStrategy: "auto" })); + const policy = await resolveMergePolicy(fx.store, { id: fx.taskId, column: "in-review" }); + expect(policy.commitStrategy).toBe("auto"); + // Default workflow's merge column has no config, so source is settings. + expect(policy.source).toBe("settings"); + } finally { + await fx.cleanup(); + } + }); +}); + +// ── 2. fileScope modes (off / warn / strict / custom) ──────────────────────── +// +// These use a REAL git repo with a staged out-of-scope file so the merger's +// real `git diff --cached --name-only` returns it. The store is a lightweight +// inline fake (NOT the merger-test-helpers mock, which would shadow git). The +// resolved fileScope mode is driven by the fake's settings + workflow stubs. + +interface ScopeRepo { + rootDir: string; + /** Stage a file at the given repo-relative path (creates it). */ + stage: (relPath: string) => Promise; + cleanup: () => Promise; +} + +async function makeScopeRepo(): Promise { + const rootDir = await mkdtemp(join(tmpdir(), "fusion-scope-")); + await initRepo(rootDir); + const run = (cmd: string) => execSync(cmd, { cwd: rootDir, stdio: "pipe" }); + return { + rootDir, + async stage(relPath) { + const abs = join(rootDir, relPath); + await mkdir(join(abs, ".."), { recursive: true }); + await writeFile(abs, "// staged\n", "utf-8"); + run(`git add -- "${relPath}"`); + }, + cleanup: async () => { + await rm(rootDir, { recursive: true, force: true }); + }, + }; +} + +/** Inline fake store for the file-scope assertions: just the methods the + * resolver + enforcement read. `workflow` drives the resolved fileScope mode; + * omitting it (with a flag-off settings) yields the legacy `warn` mode. */ +function fakeScopeStore(opts: { + declaredScope: string[]; + settings: Settings; + scopeOverride?: boolean; + workflow?: { id: string; mergeConfig: Record }; +}) { + const task: Task = { + id: "FN-4073", + title: "scope task", + description: "x", + column: "in-review", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + scopeOverride: opts.scopeOverride, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + } as Task; + const parseFileScopeFromPrompt = vi.fn().mockResolvedValue(opts.declaredScope); + return { + task, + parseFileScopeFromPrompt, + store: { + getTask: vi.fn().mockResolvedValue(task), + getSettings: vi.fn().mockResolvedValue(opts.settings), + appendAgentLog: vi.fn().mockResolvedValue(undefined), + parseFileScopeFromPrompt, + getTaskWorkflowSelection: vi.fn().mockReturnValue( + opts.workflow ? { workflowId: opts.workflow.id, stepIds: [] } : undefined, + ), + getWorkflowDefinition: vi.fn().mockResolvedValue( + opts.workflow + ? { id: opts.workflow.id, ir: customMergeWorkflowIr(opts.workflow.mergeConfig) } + : undefined, + ), + } as never, + }; +} + +describe("fileScope modes — enforceSquashFileScopeInvariant", () => { + let repo: ScopeRepo; + beforeEach(async () => { + vi.clearAllMocks(); + repo = await makeScopeRepo(); + }); + afterEach(async () => { + await repo.cleanup(); + }); + + const declared = ["packages/engine/src/merger.ts"]; + + it("'warn' (legacy/flag-OFF default): logs + proceeds, audit carries the file list", async () => { + const { store } = fakeScopeStore({ declaredScope: declared, settings: settingsWith({}) }); + await repo.stage("packages/core/src/store.ts"); // out of scope + const auditor = { git: vi.fn().mockResolvedValue(undefined) }; + + await expect( + enforceSquashFileScopeInvariant({ + store, + taskId: "FN-4073", + rootDir: repo.rootDir, + task: await (store as never as { getTask: (id: string) => Promise }).getTask("FN-4073"), + resetLabel: "file-scope invariant violation", + auditor: auditor as never, + }), + ).resolves.toBeUndefined(); + + expect(auditor.git).toHaveBeenCalledTimes(1); + const call = auditor.git.mock.calls[0][0]; + expect(call.type).toBe("merge:file-scope-violation"); + expect(call.metadata.warningOnly).toBe(true); + expect(call.metadata.stagedFiles).toEqual(["packages/core/src/store.ts"]); + expect(call.metadata.declaredScope).toEqual(declared); + }); + + it("'strict': re-throws FileScopeViolationError and audits with warningOnly=false", async () => { + const { store } = fakeScopeStore({ + declaredScope: declared, + settings: settingsWith({ experimentalFeatures: { workflowColumns: true } }), + workflow: { id: "wf-strict", mergeConfig: { fileScope: "strict" } }, + }); + await repo.stage("packages/core/src/store.ts"); + const auditor = { git: vi.fn().mockResolvedValue(undefined) }; + + await expect( + enforceSquashFileScopeInvariant({ + store, + taskId: "FN-4073", + rootDir: repo.rootDir, + task: await (store as never as { getTask: (id: string) => Promise }).getTask("FN-4073"), + resetLabel: "file-scope invariant violation", + auditor: auditor as never, + }), + ).rejects.toBeInstanceOf(FileScopeViolationError); + + const call = auditor.git.mock.calls[0][0]; + expect(call.metadata.mode).toBe("strict"); + expect(call.metadata.warningOnly).toBe(false); + }); + + it("'off': skips the throw and emits one scope-enforcement-disabled audit", async () => { + const { store } = fakeScopeStore({ + declaredScope: declared, + settings: settingsWith({ experimentalFeatures: { workflowColumns: true } }), + scopeOverride: true, + workflow: { id: "wf-off", mergeConfig: { fileScope: "off" } }, + }); + await repo.stage("packages/core/src/store.ts"); + const auditor = { git: vi.fn().mockResolvedValue(undefined) }; + + await expect( + enforceSquashFileScopeInvariant({ + store, + taskId: "FN-4073", + rootDir: repo.rootDir, + task: await (store as never as { getTask: (id: string) => Promise }).getTask("FN-4073"), + resetLabel: "file-scope invariant violation", + auditor: auditor as never, + }), + ).resolves.toBeUndefined(); + + expect(auditor.git).toHaveBeenCalledTimes(1); + const call = auditor.git.mock.calls[0][0]; + expect(call.type).toBe("merge:file-scope-enforcement-disabled"); + expect(call.metadata.disabledByWorkflowConfig).toBe(true); + // per-task scopeOverride is a documented no-op in this mode + expect(call.metadata.scopeOverrideIsNoOp).toBe(true); + }); + + it("'custom': evaluates supplied rules in place of the prompt's File Scope", async () => { + // Prompt scope would be `declared` (no overlap), but custom rules DO overlap + // the staged file → no violation. + const { store, parseFileScopeFromPrompt } = fakeScopeStore({ + declaredScope: declared, + settings: settingsWith({ experimentalFeatures: { workflowColumns: true } }), + workflow: { id: "wf-custom", mergeConfig: { fileScope: "custom", rules: ["packages/core/src/**"] } }, + }); + await repo.stage("packages/core/src/store.ts"); // overlaps custom rules + const auditor = { git: vi.fn().mockResolvedValue(undefined) }; + + await expect( + enforceSquashFileScopeInvariant({ + store, + taskId: "FN-4073", + rootDir: repo.rootDir, + task: await (store as never as { getTask: (id: string) => Promise }).getTask("FN-4073"), + resetLabel: "file-scope invariant violation", + auditor: auditor as never, + }), + ).resolves.toBeUndefined(); + + expect(auditor.git).not.toHaveBeenCalled(); + // The prompt's File Scope is bypassed when custom rules are present. + expect(parseFileScopeFromPrompt).not.toHaveBeenCalled(); + }); + + it("'custom' with violating rules: rules replace prompt scope and a violation is detected", async () => { + const { store } = fakeScopeStore({ + declaredScope: declared, + settings: settingsWith({ experimentalFeatures: { workflowColumns: true } }), + workflow: { id: "wf-custom", mergeConfig: { fileScope: "custom", rules: ["docs/**"] } }, + }); + await repo.stage("packages/core/src/store.ts"); // does NOT overlap docs/** + const auditor = { git: vi.fn().mockResolvedValue(undefined) }; + + await expect( + enforceSquashFileScopeInvariant({ + store, + taskId: "FN-4073", + rootDir: repo.rootDir, + task: await (store as never as { getTask: (id: string) => Promise }).getTask("FN-4073"), + resetLabel: "file-scope invariant violation", + auditor: auditor as never, + }), + ).resolves.toBeUndefined(); + + expect(auditor.git).toHaveBeenCalledTimes(1); + expect(auditor.git.mock.calls[0][0].metadata.declaredScope).toEqual(["docs/**"]); + }); +}); + +describe("assertSquashOverlapsFileScope — custom rules + scopeOverride interaction", () => { + let repo: ScopeRepo; + beforeEach(async () => { + vi.clearAllMocks(); + repo = await makeScopeRepo(); + }); + afterEach(async () => { + await repo.cleanup(); + }); + + it("custom rules override the per-task scopeOverride (rules take precedence)", async () => { + const { store } = fakeScopeStore({ + declaredScope: ["packages/engine/**"], + settings: settingsWith({}), + scopeOverride: true, + }); + await repo.stage("packages/core/src/store.ts"); // does not overlap custom rules + await expect( + assertSquashOverlapsFileScope({ + store, + taskId: "FN-4073", + rootDir: repo.rootDir, + task: await (store as never as { getTask: (id: string) => Promise }).getTask("FN-4073"), + customScopeRules: ["packages/engine/**"], + }), + ).rejects.toBeInstanceOf(FileScopeViolationError); + }); + + it("without custom rules, scopeOverride bypasses the check (legacy behavior intact)", async () => { + const { store } = fakeScopeStore({ + declaredScope: ["packages/engine/**"], + settings: settingsWith({}), + scopeOverride: true, + }); + await repo.stage("packages/core/src/store.ts"); + await expect( + assertSquashOverlapsFileScope({ + store, + taskId: "FN-4073", + rootDir: repo.rootDir, + task: await (store as never as { getTask: (id: string) => Promise }).getTask("FN-4073"), + }), + ).resolves.toBeUndefined(); + }); +}); + +// ── 3. lost-work guard trio: config CANNOT reach them ──────────────────────── + +describe("lost-work guard trio is non-configurable (KTD-6 regression)", () => { + it("the merge trait config schema exposes NO field that names a lost-work guard", () => { + const def = getTraitRegistry().getTrait("merge"); + expect(def).toBeDefined(); + const keys = (def?.configSchema?.fields ?? []).map((f) => f.key); + // Only policy knobs — nothing that could disable sibling-branch rejection, + // line-anchored attribution, or no-op-finalize modifiedFiles preservation. + expect(keys.sort()).toEqual(["conflictStrategy", "fileScope", "rules", "squash", "strategy"]); + for (const forbidden of [ + "allowSiblingMergeTarget", + "siblingBranch", + "attribution", + "clearModifiedFiles", + "noOpFinalize", + "lostWork", + ]) { + expect(keys).not.toContain(forbidden); + } + }); + + it("resolved policy never carries a lost-work toggle, regardless of fileScope/strategy", async () => { + const fx = await makeStoreFixture(); + try { + for (const cfg of [ + { fileScope: "off", strategy: "always-squash" }, + { fileScope: "warn", strategy: "auto" }, + { fileScope: "custom", rules: ["**/*"], strategy: "pr-only" }, + ] as const) { + await fx.selectCustomMergeWorkflow(cfg); + const policy = await resolveMergePolicy(fx.store, { id: fx.taskId, column: "in-review" }); + // The resolved policy object's keys are a closed set — no guard knob. + expect(Object.keys(policy).sort()).toEqual( + ["commitStrategy", "fileScope", "fileScopeRules", "pullRequestOnly", "source"].sort(), + ); + } + } finally { + await fx.cleanup(); + } + }); +}); + +// ── 4. merge trait hooks: enqueue (onEnter) drives queue, never inline ─────── + +describe("merge trait hooks — enqueue-only, queue-driven", () => { + beforeEach(() => { + __resetMergeTraitRegistrationForTests(); + registerMergeTraitHooks(); + }); + + it("registers real onEnter/onExit impls in the registry (not degraded no-ops)", () => { + const onEnter = getTraitRegistry().resolveTraitHook("merge", "onEnter"); + const onExit = getTraitRegistry().resolveTraitHook("merge", "onExit"); + expect(onEnter.impl).toBeDefined(); + expect(onEnter.warning).toBeUndefined(); // a real impl is registered + expect(onExit.impl).toBeDefined(); + expect(onExit.warning).toBeUndefined(); + }); + + it("onEnter enqueues onto the persisted merge queue and never awaits a merge", async () => { + const fx = await makeStoreFixture(); + try { + const onEnter = getTraitRegistry().resolveTraitHook("merge", "onEnter").impl as ( + s: TaskStore, + t: { id: string; priority?: string }, + ) => Promise; + const task = await fx.store.getTask(fx.taskId); + await onEnter(fx.store, { id: task.id, priority: task.priority }); + // Exactly one queue entry; the merge itself is NOT performed by the hook. + expect(fx.peekQueue(fx.taskId)).toBeTruthy(); + const after = await fx.store.getTask(fx.taskId); + expect(after.column).toBe("in-review"); // hook did not move the card + } finally { + await fx.cleanup(); + } + }); + + it("onEnter is idempotent: re-running (crash-replay) holds exactly one entry", async () => { + const fx = await makeStoreFixture(); + try { + const onEnter = getTraitRegistry().resolveTraitHook("merge", "onEnter").impl as ( + s: TaskStore, + t: { id: string; priority?: string }, + ) => Promise; + const task = await fx.store.getTask(fx.taskId); + await onEnter(fx.store, { id: task.id, priority: task.priority }); + await onEnter(fx.store, { id: task.id, priority: task.priority }); + expect(fx.queueCount()).toBe(1); + } finally { + await fx.cleanup(); + } + }); +}); + +// ── 5. queued merge survives restart (resumes from SQLite) ─────────────────── + +describe("queued merge survives restart (SQLite-authoritative)", () => { + it("a queued entry persists across a fresh TaskStore over the same DB file", async () => { + const rootDir = await mkdtemp(join(tmpdir(), "fusion-merge-trait-")); + try { + await initRepo(rootDir); + // First store: create task in-review and enqueue. + const store1 = new TaskStore(rootDir, undefined, {}); + await store1.init(); + await store1.updateSettings(settingsWith({ mergeStrategy: "direct" })); + const created = await store1.createTask({ + title: "resume", + description: "x", + column: "in-review", + branch: "fusion/fn-resume", + baseBranch: "main", + steps: [], + } as never); + const resumeId = created.id; + store1.enqueueMergeQueue(resumeId, {}); + expect(store1.peekMergeQueue().some((e) => e.taskId === resumeId)).toBe(true); + store1.close(); + + // Second store over the same on-disk DB: the queued entry is still there. + const store2 = new TaskStore(rootDir, undefined, {}); + await store2.init(); + expect(store2.peekMergeQueue().some((e) => e.taskId === resumeId)).toBe(true); + store2.close(); + } finally { + await rm(rootDir, { recursive: true, force: true }); + } + }); +}); + +// ── shared in-memory store fixture ─────────────────────────────────────────── + +interface StoreFixture { + store: TaskStore; + taskId: string; + selectCustomMergeWorkflow: (mergeConfig: Record) => Promise; + peekQueue: (taskId: string) => unknown; + queueCount: () => number; + cleanup: () => Promise; +} + +async function makeStoreFixture(): Promise { + const rootDir = await mkdtemp(join(tmpdir(), "fusion-merge-trait-")); + await initRepo(rootDir); + const store = new TaskStore(rootDir, undefined, { inMemoryDb: true }); + await store.init(); + await store.updateSettings(settingsWith({ mergeStrategy: "direct" })); + // `experimentalFeatures` is a GLOBAL setting (mirrors the characterization + // suite), so it must be set via updateGlobalSettings to flip the flag. + await store.updateGlobalSettings({ experimentalFeatures: { workflowColumns: true } } as never); + const created = await store.createTask({ + title: "merge-trait fixture", + description: "merge-trait fixture", + column: "in-review", + branch: "fusion/fn-mt", + baseBranch: "main", + steps: [], + } as never); + const taskId = created.id; + + return { + store, + taskId, + async selectCustomMergeWorkflow(mergeConfig) { + const def = await store.createWorkflowDefinition({ + name: `wf-${Math.random().toString(36).slice(2)}`, + ir: customMergeWorkflowIr(mergeConfig), + } as never); + await store.selectTaskWorkflow(taskId, def.id); + }, + peekQueue(id) { + return store.peekMergeQueue().find((e) => e.taskId === id); + }, + queueCount() { + return store.peekMergeQueue().length; + }, + cleanup: async () => { + store.close(); + await rm(rootDir, { recursive: true, force: true }); + }, + }; +} diff --git a/packages/engine/src/merge-trait.ts b/packages/engine/src/merge-trait.ts new file mode 100644 index 0000000000..5c320dd319 --- /dev/null +++ b/packages/engine/src/merge-trait.ts @@ -0,0 +1,291 @@ +/** + * Merge trait behavior (U7, R10) — `@fusion/engine` side. + * + * The merge trait turns merge/PR orchestration, merge strategy, squash posture + * and file-scope enforcement mode into *configuration* over the substrate merge + * capability (KTD-6). This module owns two things: + * + * 1. The merge trait's hook implementations, registered into core's trait + * registry via the `registerTraitHookImpl` DI seam (mirrors + * `setCreateFnAgent`): + * - `onEnter` → enqueue the task onto the *persisted* merge-request + * queue (reuse the store's existing enqueue path). It NEVER awaits a + * merge inline; completion is driven by the merge-queue worker loop + * (`ProjectEngine.pickNextMergeTaskId` → `aiMergeTask` → + * `store.moveTask(id, "done")`) and resolved via the queue, so a + * graph walk / transition never blocks on a merge (the plan-002 + * deadlock hazard). + * - `onExit` → leaving the merge column dequeues a pending request. + * The store already performs this in-lock inside `moveTaskInternal` + * (`dequeueMergeQueueOnColumnExit`, a private method); the hook + * delegates to that existing mechanism rather than reimplementing the + * dequeue (see the onExit impl note). It is registered so the registry + * resolves a real impl (not a degraded no-op + audit warning). + * + * 2. `resolveMergePolicy` — a small read-through resolver consulted by + * `merger.ts` at its existing policy-knob read sites. When the + * `workflowColumns` flag is ON it reads the merge-trait config from the + * task's resolved workflow; otherwise (and when the workflow's merge + * trait carries no config, e.g. the built-in default workflow) it falls + * back to the existing settings knobs (`directMergeCommitStrategy`, + * `mergeStrategy`, scope settings) for back-compat. + * + * The three 2026-05-23 lost-work guards stay in `merger.ts` mechanics and are + * UNREACHABLE from this config (KTD-6 / R10): sibling `fusion/fn-*` merge-target + * rejection, line-anchored commit attribution, and the no-op-finalize + * `modifiedFiles` preservation are not gated by any field this resolver + * exposes. + */ + +import { + BUILTIN_CODING_WORKFLOW_IR, + getBuiltinWorkflow, + isBuiltinWorkflowId, + isWorkflowColumnsEnabled, + parseWorkflowIr, + registerTraitHookImpl, + type DirectMergeCommitStrategy, + type Settings, + type Task, + type TaskStore, + type WorkflowIr, + type WorkflowIrColumn, +} from "@fusion/core"; + +// ── Resolved merge policy ──────────────────────────────────────────────────── + +/** File-scope enforcement mode (R10). `custom` evaluates `rules` in place of + * the task's File Scope section. */ +export type MergeFileScopeMode = "strict" | "warn" | "off" | "custom"; + +/** The merge strategy as authored on the trait. Direct-merge commit strategies + * plus `pr-only` (which routes to the pull-request flow without a direct + * merge). Absent on the trait → resolved from settings. */ +export type MergeTraitStrategy = DirectMergeCommitStrategy | "pr-only"; + +/** Fully-resolved merge policy consumed by `merger.ts`. */ +export interface ResolvedMergePolicy { + /** Direct-merge commit strategy. For `pr-only` this is the fallback used if + * a direct merge is ever taken; `pullRequestOnly` is the authoritative + * routing signal. */ + commitStrategy: DirectMergeCommitStrategy; + /** True when the trait authored `strategy: "pr-only"` — the merge is routed + * through the PR flow (enqueue-with-prState marker) without a direct merge. */ + pullRequestOnly: boolean; + /** File-scope enforcement mode. */ + fileScope: MergeFileScopeMode; + /** Custom scope rules (only meaningful when `fileScope === "custom"`). */ + fileScopeRules: string[]; + /** Where the policy came from — `workflow` when read from the task's merge + * trait config (flag ON), `settings` for the legacy/back-compat read-through. */ + source: "workflow" | "settings"; +} + +// ── Workflow IR resolution (read-only, flag-gated) ─────────────────────────── + +/** + * Resolve the task's workflow IR. Mirrors the store's private + * `resolveTaskWorkflowIrSync` resolution rule (selection → builtin/custom → + * default) but stays read-only and engine-side. A missing/corrupt definition + * degrades to the default workflow so policy resolution never throws. + */ +async function resolveTaskWorkflowIr(store: TaskStore, taskId: string): Promise { + let workflowId: string | undefined; + try { + workflowId = store.getTaskWorkflowSelection(taskId)?.workflowId; + } catch { + workflowId = undefined; + } + if (!workflowId) return BUILTIN_CODING_WORKFLOW_IR; + + if (isBuiltinWorkflowId(workflowId)) { + const builtin = getBuiltinWorkflow(workflowId); + return builtin?.ir ?? BUILTIN_CODING_WORKFLOW_IR; + } + + try { + const def = await store.getWorkflowDefinition(workflowId); + if (!def) return BUILTIN_CODING_WORKFLOW_IR; + // `def.ir` is already a parsed WorkflowIr; reparse defensively only if a + // raw string ever slips through. + return typeof def.ir === "string" ? parseWorkflowIr(def.ir) : def.ir; + } catch { + return BUILTIN_CODING_WORKFLOW_IR; + } +} + +/** Find the column the task currently sits in (by id). */ +function findColumn(ir: WorkflowIr, columnId: string): WorkflowIrColumn | undefined { + if (ir.version !== "v2") return undefined; + return ir.columns.find((c) => c.id === columnId); +} + +/** Extract the merge trait's config from a column, if it carries one. */ +function readMergeTraitConfig(column: WorkflowIrColumn | undefined): Record | undefined { + if (!column) return undefined; + const ct = column.traits.find((t) => t.trait === "merge"); + if (!ct) return undefined; + return ct.config ?? {}; +} + +// ── Policy read-through resolver ───────────────────────────────────────────── + +const VALID_COMMIT_STRATEGIES: ReadonlySet = new Set([ + "auto", + "always-squash", + "always-rebase", +]); +const VALID_FILE_SCOPE_MODES: ReadonlySet = new Set(["strict", "warn", "off", "custom"]); + +/** The settings-only fallback policy (legacy / flag-OFF / no trait config). */ +function settingsPolicy(settings: Pick): ResolvedMergePolicy { + return { + commitStrategy: settings.directMergeCommitStrategy ?? "always-squash", + pullRequestOnly: settings.mergeStrategy === "pull-request", + // Legacy file-scope behavior is a soft warn (see + // `enforceSquashFileScopeInvariant`, which logs + proceeds), so the + // back-compat read-through reports `warn` — the existing call path is + // unchanged when the flag is OFF. + fileScope: "warn", + fileScopeRules: [], + source: "settings", + }; +} + +/** + * Resolve the effective merge policy for a task (R10). Flag ON: read the merge + * trait's config from the task's resolved workflow column; fall back to + * settings for any field the trait leaves unset (the built-in default + * workflow's merge trait carries no config, so it resolves entirely from + * settings — verbatim back-compat). Flag OFF: settings only. + * + * The lost-work guard trio is intentionally NOT represented here: no field this + * resolver returns can disable the sibling-branch rejection, line-anchored + * attribution, or the no-op-finalize `modifiedFiles` guard (KTD-6). + */ +export async function resolveMergePolicy( + store: TaskStore, + task: Pick, + settings?: Pick, +): Promise { + const resolvedSettings = settings ?? (await store.getSettings()); + const fallback = settingsPolicy(resolvedSettings); + + if (!isWorkflowColumnsEnabled(resolvedSettings)) { + return fallback; + } + + let config: Record | undefined; + try { + const ir = await resolveTaskWorkflowIr(store, task.id); + config = readMergeTraitConfig(findColumn(ir, task.column)); + } catch { + config = undefined; + } + // No merge trait, or a merge trait carrying no policy fields (e.g. the + // built-in default workflow's `{ trait: "merge" }` with no config) → resolve + // entirely from settings (verbatim back-compat). + if (!config || (config.strategy === undefined && config.fileScope === undefined)) { + return fallback; + } + + // strategy → commitStrategy + pullRequestOnly + let commitStrategy = fallback.commitStrategy; + let pullRequestOnly = fallback.pullRequestOnly; + const rawStrategy = config.strategy; + if (rawStrategy === "pr-only") { + pullRequestOnly = true; + } else if (typeof rawStrategy === "string" && VALID_COMMIT_STRATEGIES.has(rawStrategy)) { + commitStrategy = rawStrategy as DirectMergeCommitStrategy; + pullRequestOnly = false; + } + + // fileScope → mode + rules + let fileScope = fallback.fileScope; + const rawFileScope = config.fileScope; + if (typeof rawFileScope === "string" && VALID_FILE_SCOPE_MODES.has(rawFileScope)) { + fileScope = rawFileScope as MergeFileScopeMode; + } + const fileScopeRules = Array.isArray(config.rules) + ? (config.rules.filter((r): r is string => typeof r === "string")) + : []; + + return { + commitStrategy, + pullRequestOnly, + fileScope, + fileScopeRules, + source: "workflow", + }; +} + +// ── Merge trait hook implementations (DI into core's trait registry) ───────── + +/** + * onEnter: enqueue the task onto the persisted merge-request queue. NEVER awaits + * a merge (KTD-6) — the merge-queue worker loop drives the actual merge and the + * subsequent move to the `complete`-flagged column. Delegates to the store's + * existing `enqueueMergeQueue` so the queue mechanics (audit, priority, + * idempotent ON CONFLICT insert) are not reimplemented. + * + * Idempotent: `enqueueMergeQueue` is `ON CONFLICT(taskId) DO NOTHING`, so a + * crash-then-rerun (recovery sweep replaying `transitionPending` hooks) holds + * exactly one queue entry. + * + * Invoked by the store's post-commit hook runner with `(store, task)`. + */ +async function mergeOnEnter(store: TaskStore, task: Pick): Promise { + try { + store.enqueueMergeQueue(task.id, { priority: task.priority }); + } catch (err) { + // Enqueue rejects (e.g. task not in the merge column) degrade to a no-op: + // the card is never stranded and the queue is never corrupted. The store + // already audits the rejection. + const message = err instanceof Error ? err.message : String(err); + void message; + } +} + +/** + * onExit: leaving the merge column dequeues a pending (unleased) request. + * + * NOTE (design / delegation): the store ALREADY performs dequeue-on-column-exit + * in-lock inside `moveTaskInternal` via the private + * `dequeueMergeQueueOnColumnExit`, which runs unconditionally on every move and + * owns the lease-aware semantics (drop an unleased entry; audit a leased one as + * a stale-lease event). The merge trait's onExit therefore *delegates to that + * existing mechanism* — it does not reissue a dequeue (which would be a + * redundant second pass and could not see the lease columns without a store API + * change the prompt forbids). Registering the hook makes the registry resolve a + * real impl (not a degraded no-op + audit warning) and documents that the + * substrate, not the trait, owns the dequeue mechanic (KTD-6: traits configure + * and invoke capabilities; they never reimplement them). + */ +function mergeOnExit(): void { + // Intentional no-op: dequeue is owned by the store's in-lock + // `dequeueMergeQueueOnColumnExit` (see note above). +} + +let registered = false; + +/** + * Register the merge trait's hook implementations into core's shared trait + * registry. Idempotent (guarded), so importing this module (or calling it from + * engine startup) more than once is safe. Mirrors the `setCreateFnAgent` DI + * pattern: core declares the hook descriptors; the engine supplies the impls. + */ +export function registerMergeTraitHooks(): void { + if (registered) return; + registered = true; + registerTraitHookImpl("merge", "onEnter", mergeOnEnter as never); + registerTraitHookImpl("merge", "onExit", mergeOnExit as never); +} + +/** Test-only: re-arm registration so a fresh registry can be exercised. */ +export function __resetMergeTraitRegistrationForTests(): void { + registered = false; +} + +// Register on import (idempotent) so the engine's trait registry resolves real +// merge-hook impls without a separate wiring call. +registerMergeTraitHooks(); diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index 49ab13c6de..a776e7cb86 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -92,6 +92,7 @@ import { normalizeMergeAdvanceAutoSyncMode, isMergeRequestContractShadowEnabled, } from "@fusion/core"; +import { resolveMergePolicy, type MergeFileScopeMode } from "./merge-trait.js"; import { describeModel, promptWithFallback } from "./pi.js"; import { accumulateSessionTokenUsage } from "./session-token-usage.js"; import { createResolvedAgentSession, extractRuntimeHint, resolveMergerSessionModel } from "./agent-session-helpers.js"; @@ -4930,10 +4931,16 @@ export async function assertSquashOverlapsFileScope(params: { taskId: string; rootDir: string; task: Task; + /** U7 (R10): when the merge trait's `fileScope: "custom"` mode is active, + * these glob/path rules replace the task's File Scope section as the + * declared scope. `scopeOverride` is a documented no-op only under + * `fileScope: "off"` (handled by the caller, which skips this assert). */ + customScopeRules?: string[]; }): Promise { - const { store, taskId, rootDir, task } = params; + const { store, taskId, rootDir, task, customScopeRules } = params; + const hasCustomRules = Array.isArray(customScopeRules) && customScopeRules.length > 0; - if (task.scopeOverride === true) { + if (!hasCustomRules && task.scopeOverride === true) { const reasonSuffix = task.scopeOverrideReason?.trim() ? ` — reason: ${task.scopeOverrideReason.trim()}` : ""; @@ -4947,11 +4954,16 @@ export async function assertSquashOverlapsFileScope(params: { return; } - if (typeof (store as Partial).parseFileScopeFromPrompt !== "function") { - return; + let declaredScope: string[]; + if (hasCustomRules) { + // Custom rules replace the parsed File Scope section entirely. + declaredScope = customScopeRules; + } else { + if (typeof (store as Partial).parseFileScopeFromPrompt !== "function") { + return; + } + declaredScope = await store.parseFileScopeFromPrompt(taskId); } - - const declaredScope = await store.parseFileScopeFromPrompt(taskId); if (declaredScope.length === 0) { return; } @@ -4986,12 +4998,70 @@ export async function enforceSquashFileScopeInvariant(params: { resetLabel: string; auditor?: RunAuditor; }): Promise { + // U7 (R10): resolve the file-scope enforcement mode from the merge trait + // (flag ON) or settings (back-compat). The lost-work guard trio is NOT gated + // by this mode — it lives elsewhere in the mechanics and stays enforced for + // every mode (KTD-6). + const policy = await resolveMergePolicy(params.store, params.task); + const mode: MergeFileScopeMode = policy.fileScope; + + if (mode === "off") { + // Skip the violation throw, but emit exactly one per-merge audit event + // recording that scope enforcement was disabled by workflow config. Per-task + // `scopeOverride` is a documented no-op in this mode (the scope check itself + // is disabled, so there is nothing to override). + if (params.auditor) { + try { + await params.auditor.git({ + type: "merge:file-scope-enforcement-disabled", + target: params.taskId, + metadata: { + resetLabel: params.resetLabel, + mode: "off", + disabledByWorkflowConfig: true, + scopeOverrideIsNoOp: params.task.scopeOverride === true, + }, + }); + } catch (auditErr) { + mergerLog.warn(`${params.taskId}: failed to emit run_audit event for file-scope-enforcement-disabled: ${auditErr instanceof Error ? auditErr.message : String(auditErr)}`); + } + } + return; + } + + const customScopeRules = mode === "custom" ? policy.fileScopeRules : undefined; + try { - await assertSquashOverlapsFileScope(params); + await assertSquashOverlapsFileScope({ ...params, customScopeRules }); } catch (error: unknown) { if (!(error instanceof FileScopeViolationError)) { throw error; } + // `strict` re-throws the violation (hard guardrail that blocks the merge); + // `warn`/`custom` log + proceed, with the audit carrying the violating file + // list (same payload as the error). + if (mode === "strict") { + if (params.auditor) { + try { + await params.auditor.git({ + type: "merge:file-scope-violation", + target: params.taskId, + metadata: { + resetLabel: params.resetLabel, + mode: "strict", + stagedFiles: error.stagedFiles, + declaredScope: error.declaredScope, + stagedFileCount: error.stagedFiles.length, + declaredScopeCount: error.declaredScope.length, + warningOnly: false, + }, + }); + } catch (auditErr) { + mergerLog.warn(`${params.taskId}: failed to emit run_audit event for FileScopeViolationError (strict): ${auditErr instanceof Error ? auditErr.message : String(auditErr)}`); + } + } + throw error; + } const warningMessage = `${error.message} Warning only — continuing merge.`; await params.store.appendAgentLog( params.taskId, @@ -7534,6 +7604,11 @@ export async function aiMergeTask( const projectRootDir = rootDir; const settings = await store.getSettings(); + // U7 (R10): resolve the merge trait's policy (strategy / fileScope / rules) + // from the task's workflow when the workflowColumns flag is ON, falling back + // to the existing settings knobs otherwise. Read-through only — merge + // mechanics (and the non-configurable lost-work guard trio) are untouched. + const mergePolicy = await resolveMergePolicy(store, task, settings); const resolvedIntegrationBranch = await resolveIntegrationBranch(projectRootDir, settings); const groupRouting = await resolveBranchGroupMergeRouting({ task, @@ -9204,8 +9279,17 @@ export async function aiMergeTask( let selectedPostMergeAuditStrategy: PostMergeAuditStrategy = "squash"; let classifiedBranchCommits: BranchCommitClassification[] = []; - if (settings.mergeStrategy !== "pull-request") { - const configuredRoute = resolveDirectMergeCommitStrategy(settings, task.prompt); + // U7 (R10): `pr-only` authored on the merge trait routes through the PR flow + // exactly like `settings.mergeStrategy === "pull-request"` — no direct-merge + // commit routing runs. + const isPullRequestRoute = settings.mergeStrategy === "pull-request" || mergePolicy.pullRequestOnly; + if (!isPullRequestRoute) { + // When the workflow's merge trait authored a commit strategy, it takes + // precedence over the project/prompt setting (read-through, mechanics + // unchanged); otherwise fall back to the existing resolver. + const configuredRoute = mergePolicy.source === "workflow" + ? { strategy: mergePolicy.commitStrategy, source: "workflow" as const } + : resolveDirectMergeCommitStrategy(settings, task.prompt); if (configuredRoute.strategy === "auto") { try { const classification = await classifyBranchCommitsForDirectMerge( diff --git a/packages/engine/src/run-audit.ts b/packages/engine/src/run-audit.ts index da0a5af3cd..0b560535c2 100644 --- a/packages/engine/src/run-audit.ts +++ b/packages/engine/src/run-audit.ts @@ -152,6 +152,7 @@ export type GitMutationType = | "merge:start" | "merge:resolve" | "merge:file-scope-violation" + | "merge:file-scope-enforcement-disabled" | "merge:auto-prerebase:applied" | "merge:auto-prerebase:skipped" | "merge:auto-prerebase:failed"