FN-6634: remove engine merge trait hook registration

Remove the engine-side merge trait hook registration so core remains the sole owner of in-review field effects.

- Stop exporting or registering engine merge-trait onEnter/onExit hooks that collide with core workflow move effects.
- Document the store-owned merge queue handoff and core-owned scheduler-state clearing contract.
- Update merge-trait tests to assert ctx-shaped hook invocation and in-review scheduler state cleanup.
- Add a patch changeset for the published CLI package.

Files changed:
 .changeset/fn-6634-merge-trait-hook-collision.md  |   5 +
 packages/engine/src/__tests__/merge-trait.test.ts | 103 +++++++++-------
 packages/engine/src/index.ts                      |   1 -
 packages/engine/src/merge-trait.ts                | 138 +++++++---------------
 4 files changed, 107 insertions(+), 140 deletions(-)

Fusion-Task-Id: FN-6634

Fusion-Task-Lineage: fe7e2259-e354-4457-afd3-7ff25e5a1504

.changeset/fn-6634-merge-trait-hook-collision.md

@@ -0,0 +1,5 @@
+---
+"@runfusion/fusion": patch
+---
+
+Stop the engine from registering merge-trait hooks that collided with core's in-review field-effects adapter and could crash workflow-column moves.

packages/engine/src/__tests__/merge-trait.test.ts

@@ -40,11 +40,7 @@ import {
   type WorkflowIr,
 } from "@fusion/core";
 
-import {
+import { resolveMergePolicy } from "../merge-trait.js";
-  resolveMergePolicy,
-  registerMergeTraitHooks,
-  __resetMergeTraitRegistrationForTests,
-} from "../merge-trait.js";
 import {
   assertSquashOverlapsFileScope,
   enforceSquashFileScopeInvariant,
@@ -489,52 +485,77 @@ describe("lost-work guard trio is non-configurable (KTD-6 regression)", () => {
   });
 });
 
-// ── 4. merge trait hooks: enqueue (onEnter) drives queue, never inline ───────
+// ── 4. merge.onEnter is the core field-effects hook, invoked as impl(ctx) ─────
-
+//
-describe("merge trait hooks — enqueue-only, queue-driven", () => {
+// Regression for the unhandled rejection in the hold-release sweep
-  beforeEach(() => {
+// (TypeError: Cannot read properties of undefined (reading 'id')). The engine
-    __resetMergeTraitRegistrationForTests();
+// used to register a `mergeOnEnter(store, task)` impl here; the ONLY caller
-    registerMergeTraitHooks();
+// (`applyDefaultWorkflowMoveEffects`) invokes the hook as `impl(ctx)` with a
-  });
+// single `DefaultWorkflowMoveContext` (no store handle), so `task` bound to
-
+// `undefined` and `task.id` threw. The merge hook is now owned by core
-  it("registers real onEnter/onExit impls in the registry (not degraded no-ops)", () => {
+// (`applyInReviewEnterEffects`, registered via `registerDefaultWorkflowHooks`);
-    const onEnter = getTraitRegistry().resolveTraitHook("merge", "onEnter");
+// the enqueue is store-owned on the handoff path. Importing this engine module
-    const onExit = getTraitRegistry().resolveTraitHook("merge", "onExit");
+// must NOT clobber that registration.
-    expect(onEnter.impl).toBeDefined();
+describe("merge.onEnter — core field-effects hook (no engine clobber)", () => {
-    expect(onEnter.warning).toBeUndefined(); // a real impl is registered
+  it("resolves to a real impl that is safe to invoke as impl(ctx) (single arg)", async () => {
-    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 (
+      const onEnter = getTraitRegistry().resolveTraitHook("merge", "onEnter");
-        s: TaskStore,
+      expect(onEnter.impl).toBeDefined();
-        t: { id: string; priority?: string },
+      expect(onEnter.warning).toBeUndefined(); // a real impl, not a degraded no-op
-      ) => Promise<void>;
+
+      // The contract the bug violated: the ONLY caller invokes the hook with a
+      // single `DefaultWorkflowMoveContext` (no store, no second `task` arg). A
+      // `(store, task)`-shaped impl would deref `undefined.id` and throw here.
+      // Order-independent: this guards the signature regardless of which module
+      // registered last.
       const task = await fx.store.getTask(fx.taskId);
-      await onEnter(fx.store, { id: task.id, priority: task.priority });
+      const ctx = {
-      // Exactly one queue entry; the merge itself is NOT performed by the hook.
+        task,
-      expect(fx.peekQueue(fx.taskId)).toBeTruthy();
+        fromColumn: "in-progress",
-      const after = await fx.store.getTask(fx.taskId);
+        toColumn: "in-review",
-      expect(after.column).toBe("in-review"); // hook did not move the card
+        moveSource: "scheduler",
+        bypassGuards: false,
+        movedAt: new Date().toISOString(),
+        settings: undefined,
+        options: {},
+        resetSteps: () => {},
+      };
+      expect(() => (onEnter.impl as (c: unknown) => void)(ctx)).not.toThrow();
     } finally {
       await fx.cleanup();
     }
   });
 
-  it("onEnter is idempotent: re-running (crash-replay) holds exactly one entry", async () => {
+  it("a flag-ON move into in-review does not throw and clears scheduler state", async () => {
     const fx = await makeStoreFixture();
     try {
-      const onEnter = getTraitRegistry().resolveTraitHook("merge", "onEnter").impl as (
+      // Fresh card in in-progress with scheduler dispatch state that MUST be
-        s: TaskStore,
+      // cleared on review entry (else it permanently blocks the merge gate).
-        t: { id: string; priority?: string },
+      const created = await fx.store.createTask({
-      ) => Promise<void>;
+        title: "review-entry",
-      const task = await fx.store.getTask(fx.taskId);
+        description: "x",
-      await onEnter(fx.store, { id: task.id, priority: task.priority });
+        column: "in-progress",
-      await onEnter(fx.store, { id: task.id, priority: task.priority });
+        branch: "fusion/fn-review-entry",
-      expect(fx.queueCount()).toBe(1);
+        baseBranch: "main",
+        steps: [],
+        status: "queued",
+        blockedBy: "SOME-OTHER",
+        overlapBlockedBy: "SOME-OTHER",
+      } as never);
+
+      // The exact wiring that crashed: moveTask → moveTaskInternal →
+      // applyDefaultWorkflowMoveEffects → merge.onEnter, invoked as impl(ctx).
+      const moved = await fx.store.moveTask(created.id, "in-review", {
+        moveSource: "scheduler",
+        allowDirectInReviewMove: true,
+      } as never);
+
+      expect(moved.column).toBe("in-review");
+      // applyInReviewEnterEffects ran: scheduler dispatch state is cleared.
+      expect(moved.status).toBeUndefined();
+      expect(moved.blockedBy).toBeUndefined();
+      expect(moved.overlapBlockedBy).toBeUndefined();
     } finally {
       await fx.cleanup();
     }

packages/engine/src/index.ts

@@ -184,7 +184,6 @@ export {
   type AutostashHandle,
 } from "./merger.js";
 export {
-  registerMergeTraitHooks,
   resolveMergePolicy,
   type ResolvedMergePolicy,
   type MergeFileScopeMode,

packages/engine/src/merge-trait.ts

@@ -3,32 +3,13 @@
  *
  * 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:
+ * capability (KTD-6). This module owns `resolveMergePolicy` — a small
- *
+ * read-through resolver consulted by `merger.ts` at its existing policy-knob
- *   1. The merge trait's hook implementations, registered into core's trait
+ * read sites. When the `workflowColumns` flag is ON it reads the merge-trait
- *      registry via the `registerTraitHookImpl` DI seam (mirrors
+ * config from the task's resolved workflow; otherwise (and when the workflow's
- *      `setCreateFnAgent`):
+ * merge trait carries no config, e.g. the built-in default workflow) it falls
- *        - `onEnter`  → enqueue the task onto the *persisted* merge-request
+ * back to the existing settings knobs (`directMergeCommitStrategy`,
- *          queue (reuse the store's existing enqueue path). It NEVER awaits a
+ * `mergeStrategy`, scope settings) for back-compat.
- *          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
@@ -39,7 +20,6 @@
 
 import {
   isWorkflowColumnsEnabled,
-  registerTraitHookImpl,
   resolveWorkflowIrForTask,
   type DirectMergeCommitStrategy,
   type Settings,
@@ -48,7 +28,6 @@ import {
   type WorkflowIr,
   type WorkflowIrColumn,
 } from "@fusion/core";
-import { mergerLog } from "./logger.js";
 
 // ── Resolved merge policy ────────────────────────────────────────────────────
 
@@ -189,73 +168,36 @@ export async function resolveMergePolicy(
   };
 }
 
-// ── Merge trait hook implementations (DI into core's trait registry) ─────────
+// ── Merge trait hooks: owned by core, NOT this module ────────────────────────
+//
+// The engine deliberately does NOT register `merge` onEnter/onExit impls.
+//
+// `merge.onEnter` is invoked by core's `applyDefaultWorkflowMoveEffects` as
+// `impl(ctx)` with a single `DefaultWorkflowMoveContext` — an in-lock,
+// pre-commit, in-memory field-mutation phase that carries NO store handle. Core
+// registers the correct ctx-shaped impl (`applyInReviewEnterEffects`) via
+// `registerDefaultWorkflowHooks()`; it clears the in-review scheduler state
+// (`status: queued`, `blockedBy`, `overlapBlockedBy`) and mirrors the flag-OFF
+// inline block in `store.ts`.
+//
+// An earlier version of this module registered a `mergeOnEnter(store, task)`
+// impl here that enqueued onto the merge queue. That was wrong on three counts:
+//   1. Signature mismatch — the only caller passes `ctx`, so `store`/`task` bound
+//      to `(ctx, undefined)` and dereferencing `task.id` threw at runtime
+//      (TypeError: cannot read 'id' of undefined) during the hold-release sweep.
+//   2. Slot collision — it clobbered core's field-effects adapter on the
+//      last-write-wins registry, dropping the in-review state clears.
+//   3. Redundant responsibility — the queue enqueue is in-txn and store-owned on
+//      the handoff path (`store.ts` enqueues on `fromHandoff`, shared by both
+//      flag states), and direct non-handoff entry into `in-review` is audited as
+//      a handoff-invariant violation. There is no sanctioned entry into the
+//      merge column that needs a hook-driven enqueue.
+//
+// `merge.onExit` similarly needs no impl: the store dequeues in-lock via the
+// private `dequeueMergeQueueOnColumnExit` on every move (lease-aware), and the
+// ctx move-effects path never invokes `merge.onExit` at all.
 
-/**
+/*
- * onEnter: enqueue the task onto the persisted merge-request queue. NEVER awaits
+FNXC:MergeTrait 2026-06-18-13:05:
- * a merge (KTD-6) — the merge-queue worker loop drives the actual merge and the
+The engine must not register `merge.onEnter` or `merge.onExit` hooks because core owns the ctx-shaped `merge.onEnter` field-effects adapter. Keeping hook registration out of this module prevents last-write-wins registry collisions and preserves in-review scheduler-state clears under workflow columns.
- * 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<Task, "id" | "priority">): Promise<void> {
-  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);
-    mergerLog.warn(`merge enqueue skipped for task ${task.id}: ${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();