fix(review): workspace-merge park must use status:'failed' to avoid re-enqueue loop (U0)

Post-fix verification review (correctness + adversarial + reliability, unanimous
P0) found that the earlier retry-burn fix introduced an infinite loop: parking a
WorkspaceTaskMergeError task with status:null + mergeRetries:0 passes every
auto-merge eligibility gate (canMergeTask short-circuits only on status==='failed'),
so the cooldown sweep re-enqueues it every tick → guard re-throws → re-park, forever.

- Park with status:'failed' (keep mergeRetries:0). canMergeTask now blocks the
  auto-sweep; a human's manual merge still works because it flows through the
  manual-resolver branch (rejectMergeResolvers), which bypasses canMergeTask — so
  'failed' does not block manual retry (the original comment's worry was wrong).
- Detect the error via `err instanceof Error && err.name === "WorkspaceTaskMergeError"`,
  matching the VerificationError/MergeAbortedError convention and bundle-safe across
  the @fusion/core→@fusion/engine boundary (drops the now-unused class import).
- Document that the dispatch door guard is a fast-fail only; the unconditional
  chokepoint guard inside runAiMerge is the authoritative enforcement.
- Add a regression test asserting the auto-merge park sets status:'failed' (not null).

Gate green: lint, typecheck, build, test:gate (649+58), project-engine (81).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-06-21 20:29:46 -07:00
parent 7240b77c67
commit 316d2659b8
2 changed files with 57 additions and 10 deletions

View File

@@ -1318,6 +1318,44 @@ describe("ProjectEngine U0 merge unification dispatch", () => {
expect(mocks.runAiMerge).not.toHaveBeenCalled();
await engine.stop();
});
// Regression: the auto-merge park for a WorkspaceTaskMergeError must set status:"failed",
// not status:null. status:null + mergeRetries:0 passes every eligibility gate, so the
// cooldown sweep re-enqueues the task every tick → tight re-throw/re-park loop. status:"failed"
// makes canMergeTask short-circuit; manual retry still works (it bypasses canMergeTask).
it("R7 auto-merge park: workspace task is parked status:'failed' so it is not re-enqueued", async () => {
const mockStore = createMockStore({ ...baseSettings, autoMerge: true });
mockStore.store.getTask.mockResolvedValue({
id: "FN-WS-AUTO",
column: "in-review",
paused: false,
mergeRetries: 0,
status: "queued",
workspaceWorktrees: {
"repo-a": { worktreePath: "/tmp/a", branch: "fusion/fn-ws-a" },
},
} as any);
mocks.currentStore = mockStore.store;
const engine = createEngine();
await engine.start();
// Auto-merge path (no manual resolver): the R7 door guard throws before runAiMerge,
// and the dispatch catch parks the task.
engine.enqueueMerge("FN-WS-AUTO");
await vi.waitFor(() => {
expect(mockStore.store.updateTask).toHaveBeenCalledWith(
"FN-WS-AUTO",
expect.objectContaining({ status: "failed", mergeRetries: 0 }),
);
});
expect(mocks.runAiMerge).not.toHaveBeenCalled();
// Guard against regression to the re-enqueue loop (status:null park):
expect(mockStore.store.updateTask).not.toHaveBeenCalledWith(
"FN-WS-AUTO",
expect.objectContaining({ status: null }),
);
await engine.stop();
});
});
describe("ProjectEngine merge queue priority ordering", () => {

View File

@@ -13,7 +13,7 @@ import type {
ResearchSynthesisRequest,
ResearchSynthesisResult,
} from "@fusion/core";
import { allowsAutoMergeProcessing, assertNotWorkspaceTaskMerge, compareTasksByPriorityThenAgeAndId, getTaskHardMergeBlocker, isSharedBranchGroupMemberIntegration, normalizeMergerMode, resolveMaxAutoMergeRetries, sortTasksByPriorityThenAgeAndId, WorkspaceTaskMergeError } from "@fusion/core";
import { allowsAutoMergeProcessing, assertNotWorkspaceTaskMerge, compareTasksByPriorityThenAgeAndId, getTaskHardMergeBlocker, isSharedBranchGroupMemberIntegration, normalizeMergerMode, resolveMaxAutoMergeRetries, sortTasksByPriorityThenAgeAndId } from "@fusion/core";
import { execFile } from "node:child_process";
import { promisify } from "node:util";
import { InProcessRuntime } from "./runtimes/in-process-runtime.js";
@@ -2287,11 +2287,15 @@ export class ProjectEngine {
this.activeMergeSession = session;
},
};
// FNXC:Workspace 2026-06-21-19:05:
// FNXC:Workspace 2026-06-21-19:40:
// R7 merge-boundary guard (master-plan U0). Reject workspace-mode
// tasks BEFORE any git work — they need the per-repo merge loop that
// lands in master-plan U6 (which removes this guard). Load the task
// here so the dispatch shares the one predicate in @fusion/core.
// This door is a FAST-FAIL only: a getTask failure is swallowed to null
// and the guard is skipped, but the unconditional chokepoint guard inside
// runAiMerge (which re-reads the task) is the authoritative enforcement,
// so a transient read failure here cannot let a workspace task reach git work.
const mergeTask = await store.getTask(taskId).catch(() => null);
if (mergeTask) assertNotWorkspaceTaskMerge(mergeTask);
@@ -2358,19 +2362,24 @@ export class ProjectEngine {
continue;
}
// FNXC:Workspace 2026-06-21-19:05:
// FNXC:Workspace 2026-06-21-19:40:
// R7 workspace merge-boundary park (master-plan U0). A WorkspaceTaskMergeError
// is a PERMANENT config error (workspace task hit a merge door before the
// per-repo merge loop exists — master-plan U6), NOT a transient merge failure.
// Park the task WITHOUT burning mergeRetries (set to 0) so a human can manually
// retry after addressing the config; the default failed-path below would
// otherwise pin mergeRetries to the cap and permanently block manual retry.
// Park with status:"failed" so the auto-merge cooldown sweep STOPS re-attempting:
// `canMergeTask` short-circuits on status==="failed". (Parking with status:null +
// mergeRetries:0 passes every eligibility gate, so the sweep re-enqueues every tick
// → tight WorkspaceTaskMergeError re-throw/re-park loop.) Keep mergeRetries:0 (not
// the cap) so a human's manual merge after the config is addressed is not blocked by
// exhausted retries — and manual merge flows through the manual-resolver branch
// (rejectMergeResolvers), which bypasses canMergeTask, so "failed" never blocks it.
// Detect by err.name (matches the VerificationError/MergeAbortedError convention and
// is robust across the @fusion/core→@fusion/engine package boundary).
const isWorkspaceMergeError =
err instanceof WorkspaceTaskMergeError
|| (err as { name?: string } | null)?.name === "WorkspaceTaskMergeError";
err instanceof Error && err.name === "WorkspaceTaskMergeError";
if (isWorkspaceMergeError) {
runtimeLog.error(
`${hasManualResolver ? "Manual" : "Auto"}-merge blocked for ${taskId}: workspace-mode tasks cannot merge until per-repo merge support (master-plan U6) lands; parking without burning mergeRetries so a human can retry after the config is addressed: ${errorMsg}`,
`${hasManualResolver ? "Manual" : "Auto"}-merge blocked for ${taskId}: workspace-mode tasks cannot merge until per-repo merge support (master-plan U6) lands; parking as failed (manual retry still works) without exhausting mergeRetries: ${errorMsg}`,
);
await store
.logEntry(taskId, `Merge blocked: ${errorMsg}`, "WorkspaceTaskMergeError")
@@ -2379,7 +2388,7 @@ export class ProjectEngine {
this.rejectMergeResolvers(taskId, err instanceof Error ? err : new Error(errorMsg));
} else {
await store
.updateTask(taskId, { status: null, mergeRetries: 0, error: errorMsg })
.updateTask(taskId, { status: "failed", mergeRetries: 0, error: errorMsg })
.catch(() => undefined);
}
continue;