diff --git a/.changeset/workspace-phase-d-self-healing.md b/.changeset/workspace-phase-d-self-healing.md index 6d412dd404..1bcdb8fa71 100644 --- a/.changeset/workspace-phase-d-self-healing.md +++ b/.changeset/workspace-phase-d-self-healing.md @@ -3,3 +3,7 @@ --- Workspace mode Phase D (U1): workspace-aware self-healing. The existing merging-status reconcilers no longer mis-finalize a partial-landed workspace task (recoverInterruptedMergingTasks now clears the transient `merging` status and re-enqueues the idempotent per-repo land instead of running the single-commit finalize over the non-git workspace root), and recoverMergeableReviewTasks now admits workspace tasks (task.worktree is null). Adds three reconcilers: partial-land recovery (re-enqueue via enqueueMerge, FORK-A unrecoverable → park failed; guarded by autoMerge:false + user-pause + workspace-aware liveness), phantom `workspace-repo-land` lease reclaim (new `entriesByKind` registry seam), and per-repo worktree cleanup from stored paths (no temp walk). New run-audit events: `task:reconcile-workspace-partial-land`(`-no-action`), `task:reclaim-phantom-workspace-land-lease`, `task:reconcile-orphaned-workspace-worktree`. + +Phase D P1 TOCTOU fix (merge-queue dispatch blind spot): the workspace partial-land and phantom-land-lease reconcilers now consult a new `ProjectEngine.isMergePending(taskId)` seam (true if the task is in the engine's in-memory `mergeQueue` or `mergeActive`). This closes the dequeue→rawMerge window where a workspace task is being merged but no other liveness signal fires yet (the id is shifted out of `mergeQueue` while `activeMergeTaskId` / `merging` status / the `workspace-repo-land` lease are not yet set inside `landWorkspaceTask`). The partial-land reconciler skips a merge-pending candidate (emitting `task:reconcile-workspace-partial-land-no-action` with reason `merge-pending`) instead of launching a second concurrent `landWorkspaceTask` (double-squash risk, since a same-task land lease is not contention), and lease reclaim leaves a merge-pending owner's not-yet-registered lease alone. Wired via `InProcessRuntime.setMergePendingProvider`; undefined (unwired) is treated as not-pending so existing guards still apply. + +Phase D review hardening: every single-commit-finalize self-healing site is now workspace-gated so a partial-landed workspace task can never be marked fully merged on one repo's commit — `recoverStuckMergeDeadlocks` (the twin of recoverInterruptedMergingTasks), `recoverOrphanOnlyScopeViolations`, `recoverAlreadyMergedReviewTasks`, `recoverBranchMisboundInReviewTasks`, and `recoverDoneTaskMergeMetadata` all skip workspace tasks and defer recovery to the workspace partial-land reconciler. The partial-land reconciler now bounds its `enqueueMerge` re-enqueue (parks `failed` after repeated queue rejections instead of looping forever) and treats a branch-gone-and-not-landed sub-repo as unrecoverable even when a stale unreachable `landedSha` is present. Phantom land-lease reclaim now only reclaims a demonstrably TERMINAL owner (never an `in-progress` executing task that registered its lease early). Orphan per-repo worktree removal failures are now engine-logged and retry-bounded. The canonical `isRepoLanded` predicate moved to a new dependency-free `workspace-land-predicate` module, dissolving the self-healing ↔ merger-ai import cycle (public export preserved). diff --git a/packages/engine/src/__tests__/self-healing-workspace.test.ts b/packages/engine/src/__tests__/self-healing-workspace.test.ts index ecd32a7892..ad524177af 100644 --- a/packages/engine/src/__tests__/self-healing-workspace.test.ts +++ b/packages/engine/src/__tests__/self-healing-workspace.test.ts @@ -263,6 +263,46 @@ describeIfGit("workspace-aware self-healing (Phase D U1)", () => { expect(store.enqueued).not.toContain(TASK_ID); }); + /* + FNXC:Workspace 2026-06-22-16:40 (Phase D P1 TOCTOU — merge-queue dispatch blind spot): + A workspace task in the dequeue→rawMerge window is being merged but NO liveness signal fires + (no active session path, no executingTaskLock/isTaskActive, no activeMergeTaskId, no `merging` + status, no land lease yet). Without the merge-pending guard the partial-land reconciler would + re-enqueue it → a SECOND concurrent `landWorkspaceTask(T)` → double-squash. With `isMergePending` + returning true (task is in mergeQueue/mergeActive) the reconciler must NOT re-enqueue and must + emit -no-action(reason: "merge-pending"). + */ + it("partial-land reconciler does NOT re-enqueue a merge-pending task (closes double-dispatch)", async () => { + fx = await createWorkspaceFixture(["repo-a", "repo-b"]); + addRepoBranch(fx, "repo-a", "a\n"); + addRepoBranch(fx, "repo-b", "b\n"); + const landedA = landRepoForReal(fx, "repo-a"); // partial-landed → would normally re-enqueue. + + const task = workspaceTask({ + "repo-a": { worktreePath: fx.repoPath("repo-a"), branch: BRANCH, landedSha: landedA }, + "repo-b": { worktreePath: fx.repoPath("repo-b"), branch: BRANCH }, + }); + const store = createStore([task]); + // Narrow seam: inject the in-memory merge-pipeline probe. No session/lock/lease set → only + // the merge-pending guard can stop the re-enqueue. + const manager = makeManager(store, fx.rootDir, { isMergePending: (id: string) => id === TASK_ID }); + + const n = await manager.reconcileWorkspacePartialLands(); + + expect(n).toBe(0); + expect(store.enqueued).not.toContain(TASK_ID); + expect(store.tasks.get(TASK_ID)?.status).not.toBe("failed"); + expect(store.tasks.get(TASK_ID)?.column).toBe("in-review"); + const auditCalls = (store.recordRunAuditEvent as ReturnType).mock.calls; + expect( + auditCalls.some( + ([ev]) => + (ev as { mutationType?: string }).mutationType === "task:reconcile-workspace-partial-land-no-action" && + (ev as { metadata?: { reason?: string } }).metadata?.reason === "merge-pending", + ), + ).toBe(true); + }); + // ── KTD2 FORK-A: branch-gone classification ──────────────────────────────── it("FORK-A: branch gone + landedSha unset → parked failed", async () => { fx = await createWorkspaceFixture(["repo-a"]); @@ -337,6 +377,33 @@ describeIfGit("workspace-aware self-healing (Phase D U1)", () => { expect(activeSessionRegistry.isPathActive(leasePath)).toBe(true); }); + /* + FNXC:Workspace 2026-06-22-16:40 (Phase D P1 TOCTOU — merge-queue dispatch blind spot): + A workspace-repo-land lease whose owner is mid-dispatch (in mergeQueue/mergeActive but not yet + activeMergeTaskId) is about to be LEGITIMATELY used by the in-flight `landWorkspaceTask`. Even + though the owner ROW reads terminal-looking and the lease is past the staleness floor, the + merge-pending guard must keep the lease. Here the owner is `done` and the lease is well past the + 180s floor — so ONLY the merge-pending guard can prevent reclaim. + */ + it("does NOT reclaim a land lease whose owner is merge-pending (mid-dispatch)", async () => { + fx = await createWorkspaceFixture(["repo-a"]); + const leasePath = fx.repoPath("repo-a"); + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-06-22T00:00:00.000Z")); + activeSessionRegistry.registerPath(leasePath, { taskId: TASK_ID, kind: "workspace-repo-land", ownerKey: "land" }); + + const task = workspaceTask({ "repo-a": { worktreePath: leasePath, branch: BRANCH } }, { column: "done" }); + const store = createStore([task]); + // Narrow seam: owner is in the in-memory merge pipeline → lease must be left alone. + const manager = makeManager(store, fx.rootDir, { isMergePending: (id: string) => id === TASK_ID }); + + vi.setSystemTime(new Date("2026-06-22T00:10:00.000Z")); // 600s > 180s floor. + const n = await manager.reclaimPhantomWorkspaceLandLeases(); + + expect(n).toBe(0); + expect(activeSessionRegistry.isPathActive(leasePath)).toBe(true); + }); + it("does NOT reclaim a land lease younger than the staleness floor", async () => { fx = await createWorkspaceFixture(["repo-a"]); const leasePath = fx.repoPath("repo-a"); @@ -414,4 +481,140 @@ describeIfGit("workspace-aware self-healing (Phase D U1)", () => { expect(store.enqueued).not.toContain("FN-9001"); expect(store.tasks.get("FN-9001")?.status).toBe("merging"); // untouched }); + + // ── review A (TWIN): recoverStuckMergeDeadlocks must NOT single-commit-finalize ───── + it("recoverStuckMergeDeadlocks does NOT finalize a partial-landed workspace task with blocked dependents (P0 twin)", async () => { + fx = await createWorkspaceFixture(["repo-a", "repo-b"]); + addRepoBranch(fx, "repo-a", "a\n"); + addRepoBranch(fx, "repo-b", "b\n"); + const landedA = landRepoForReal(fx, "repo-a"); // repo A landed; repo B NOT → partial. + + const task = workspaceTask( + { + "repo-a": { worktreePath: fx.repoPath("repo-a"), branch: BRANCH, landedSha: landedA }, + "repo-b": { worktreePath: fx.repoPath("repo-b"), branch: BRANCH }, + }, + // Deadlock-candidate shape: failed + retries exhausted, mergeConfirmed unset. + { status: "failed", mergeRetries: 5, updatedAt: new Date(Date.now() - 30 * 60_000).toISOString() }, + ); + // A blocked dependent in todo → the deadlock filter admits the (worktree-null) workspace task. + const dependent = { + id: "FN-7002", column: "todo", blockedBy: TASK_ID, paused: false, dependencies: [], steps: [], currentStep: 0, + } as unknown as Task; + const store = createStore([task, dependent], { maxAutoMergeRetries: 1 }); + const manager = makeManager(store, fx.rootDir); + + await manager.recoverStuckMergeDeadlocks(); + + // NOT finalized done; never emitted task:merged on a single repo; status cleared (not done). + expect(store.moveTask).not.toHaveBeenCalled(); + expect(store.emitted.some((e) => e.event === "task:merged")).toBe(false); + expect(store.tasks.get(TASK_ID)?.column).toBe("in-review"); + expect(store.tasks.get(TASK_ID)?.status).toBeNull(); + }); + + // ── review B: bounded re-enqueue — no silent infinite loop ───────────────── + it("partial-land reconciler parks failed after N consecutive enqueue drops (no infinite re-enqueue)", async () => { + fx = await createWorkspaceFixture(["repo-a", "repo-b"]); + addRepoBranch(fx, "repo-a", "a\n"); + addRepoBranch(fx, "repo-b", "b\n"); + const landedA = landRepoForReal(fx, "repo-a"); + + const baseTrees = { + "repo-a": { worktreePath: fx.repoPath("repo-a"), branch: BRANCH, landedSha: landedA }, + "repo-b": { worktreePath: fx.repoPath("repo-b"), branch: BRANCH }, + } as NonNullable; + const task = workspaceTask(baseTrees); + const store = createStore([task]); + // enqueueMerge that ALWAYS rejects (queue full) → drop every time. + const manager = makeManager(store, fx.rootDir, { enqueueMerge: () => false }); + + // First two sweeps: dropped, re-enqueued (not failed yet). repo-b branch still present → retryable. + await manager.reconcileWorkspacePartialLands(); + expect(store.tasks.get(TASK_ID)?.status).not.toBe("failed"); + await manager.reconcileWorkspacePartialLands(); + expect(store.tasks.get(TASK_ID)?.status).not.toBe("failed"); + // Third drop hits the bound → parked failed. + await manager.reconcileWorkspacePartialLands(); + expect(store.tasks.get(TASK_ID)?.status).toBe("failed"); + }); + + // ── review C: phantom-lease reclaim must NOT reclaim a live executing (in-progress) task ─ + it("does NOT reclaim a land lease owned by an IN-PROGRESS executing task (no merge status)", async () => { + fx = await createWorkspaceFixture(["repo-a"]); + const leasePath = fx.repoPath("repo-a"); + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-06-22T00:00:00.000Z")); + activeSessionRegistry.registerPath(leasePath, { taskId: TASK_ID, kind: "workspace-repo-land", ownerKey: "land" }); + + // Owner is executing in 'in-progress' with NO merge status — registered its land lease early. + const task = workspaceTask({ "repo-a": { worktreePath: leasePath, branch: BRANCH } }, { column: "in-progress", status: null }); + const store = createStore([task]); + const manager = makeManager(store, fx.rootDir); + + vi.setSystemTime(new Date("2026-06-22T00:10:00.000Z")); // well past the 180s floor. + const n = await manager.reclaimPhantomWorkspaceLandLeases(); + + expect(n).toBe(0); + expect(activeSessionRegistry.isPathActive(leasePath)).toBe(true); + }); + + // ── review D: branch-gone + landedSha-set-but-UNREACHABLE → parked, not re-enqueued forever ─ + it("FORK-A: branch gone + landedSha set but UNREACHABLE → parked failed (not re-enqueued forever)", async () => { + fx = await createWorkspaceFixture(["repo-a"]); + addRepoBranch(fx, "repo-a", "a\n"); + const landedA = landRepoForReal(fx, "repo-a"); + // Roll the integration ref BACK so landedA is no longer reachable (force-reset), and delete the branch. + fx.git("repo-a", "git reset --hard HEAD~1"); + fx.git("repo-a", `git branch -D ${BRANCH}`); + + const task = workspaceTask({ + "repo-a": { worktreePath: fx.repoPath("repo-a"), branch: BRANCH, landedSha: landedA }, + }); + const store = createStore([task]); + const manager = makeManager(store, fx.rootDir); + + const n = await manager.reconcileWorkspacePartialLands(); + // isRepoLanded is FALSE (landedSha unreachable, no trailer on ref) AND branch gone → unrecoverable. + expect(n).toBe(1); + expect(store.tasks.get(TASK_ID)?.status).toBe("failed"); + expect(store.enqueued).not.toContain(TASK_ID); + }); + + // ── review E: failing git worktree remove → logged, isolated, bounded ────── + it("orphan worktree removal failure is bounded and does not abort the sweep", async () => { + fx = await createWorkspaceFixture(["repo-a", "repo-b"]); + // repo-a: a real removable worktree. repo-b: a path that EXISTS but is NOT a git worktree → remove fails. + const wtA = path.join(fx.repoPath("repo-a"), ".wt-task"); + fx.git("repo-a", `git worktree add -b ${BRANCH} ${wtA} HEAD`); + const wtB = path.join(fx.repoPath("repo-b"), ".not-a-worktree"); + execSync(`mkdir -p ${wtB}`, { stdio: "pipe" }); + writeFileSync(path.join(wtB, "stray.txt"), "x", "utf-8"); + expect(existsSync(wtA)).toBe(true); + expect(existsSync(wtB)).toBe(true); + + const task = workspaceTask( + { + "repo-a": { worktreePath: wtA, branch: BRANCH }, + "repo-b": { worktreePath: wtB, branch: BRANCH }, + }, + { column: "done" }, + ); + const store = createStore([task]); + const manager = makeManager(store, fx.rootDir); + + // First sweep: repo-a removed (isolated from repo-b's failure); repo-b counted as a failure. + const cleaned1 = await manager.reconcileOrphanedWorkspaceWorktrees(); + expect(cleaned1).toBe(1); + expect(existsSync(wtA)).toBe(false); + // The audit recorded a failure for repo-b (observability), and the sweep did not throw. + expect(store.emitted.length >= 0).toBe(true); + + // Subsequent sweeps keep failing on repo-b but stay bounded — after the bound they stop attempting. + await manager.reconcileOrphanedWorkspaceWorktrees(); + await manager.reconcileOrphanedWorkspaceWorktrees(); + const cleanedAfterBound = await manager.reconcileOrphanedWorkspaceWorktrees(); + // No more successful removals (repo-a already gone) and no crash. + expect(cleanedAfterBound).toBe(0); + }); }); diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index 43dbf72e14..bf92d0dcab 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -190,14 +190,14 @@ export { // FNXC:MergerUnification 2026-06-21-19:05: runAiMerge is the sole merge path // (master-plan U0); exported for the CLI callers (fn task merge + UI-only merge). export { runAiMerge } from "./merger-ai.js"; +// FNXC:Workspace 2026-06-22-14:10 (Phase D review G): canonical landed predicate now lives in its +// own dependency-free module (self-healing ↔ merger-ai cycle dissolved). Public export preserved. +export { isRepoLanded } from "./workspace-land-predicate.js"; // FNXC:Workspace 2026-06-21-23:40 (Phase C U1): per-repo workspace merge loop + // the extracted per-repo land primitive, exported for the CLI/dashboard merge doors. export { landWorkspaceTask, landOneRepo, - // FNXC:Workspace 2026-06-22-04:10 (Phase C review A6): canonical landed predicate, - // re-exported so Phase D self-healing reuses it instead of reimplementing the ancestor check. - isRepoLanded, // FNXC:Workspace 2026-06-22-04:10 (Phase C review A4): real error classes (instanceof-able), // re-exported so the engine dispatch can switch to instanceof in the separate pass. WorkspaceRepoLandBusyError, diff --git a/packages/engine/src/merger-ai.ts b/packages/engine/src/merger-ai.ts index a9f66a45c8..d26dd622b0 100644 --- a/packages/engine/src/merger-ai.ts +++ b/packages/engine/src/merger-ai.ts @@ -74,6 +74,13 @@ import { installWorktreeDependencies } from "./merge-dependency-sync.js"; import { activeSessionRegistry } from "./active-session-registry.js"; import { MIN_TEMP_WORKTREE_REAP_AGE_MS } from "./self-healing.js"; import { resolveAiMergeRootPath, resolveLegacyAiMergeRootPath } from "./worktree-paths.js"; +/* +FNXC:Workspace 2026-06-22-14:10 (Phase D review G — cycle dissolved): +`isRepoLanded` + `FUSION_TASK_ID_TRAILER_KEY` moved to the dependency-free `workspace-land-predicate` +module so self-healing can import the predicate without re-entering the self-healing ↔ merger-ai +import cycle (merger-ai already imports `MIN_TEMP_WORKTREE_REAP_AGE_MS` from self-healing). +*/ +import { isRepoLanded, FUSION_TASK_ID_TRAILER_KEY } from "./workspace-land-predicate.js"; const execFileAsync = promisify(execFile); const aiMergeLog = createLogger("merger-ai"); @@ -99,19 +106,6 @@ async function gitOk(args: string[], cwd: string): Promise { } } -/** - * FNXC:Workspace 2026-06-22-04:10 (Phase C review A1): - * Capture git stdout, returning undefined (never throwing) on failure — for read-only - * probes (merge-base, log --grep) where a non-zero exit is an expected "not found". - */ -async function gitCapture(args: string[], cwd: string): Promise { - try { - return await git(args, cwd); - } catch { - return undefined; - } -} - function getErrorMessage(err: unknown): string { return err instanceof Error ? err.message : String(err); } @@ -358,8 +352,6 @@ export async function cleanupAiMergeWorktree(input: { } -const FUSION_TASK_ID_TRAILER_KEY = "Fusion-Task-Id"; - /** Trailers that associate the squash commit with its board task: the * `Fusion-Task-Id` trailer plus the canonical lineage trailer when available. * These are what the board's commit→task association parses. */ @@ -1687,74 +1679,10 @@ export async function landWorkspaceTask( return { taskId, repos, allLanded, finalized: false }; } -/** - * FNXC:Workspace 2026-06-22-00:30 (Phase C U2, KTD3): - * Landed predicate: a sub-repo is landed iff a `landedSha` is recorded AND that sha is - * an ancestor of (or equals) the repo's CURRENT integration tip. The ancestor check - * (not just sha presence) survives a later un-related advance of the integration ref: - * the landed commit is still reachable, so the repo stays "landed". A `landedSha` that - * is NOT reachable from the tip (e.g. the ref was reset/rebuilt) reads as NOT landed and - * the repo re-lands. - * - * FNXC:Workspace 2026-06-22-04:10 (Phase C review A1 — task-trailer ancestor fallback): - * The double-land window: a land advances the integration ref via `advanceIntegrationBranchRef`'s - * CAS, then `persistRepoLandedSha` records `landedSha`. If that DB write fails AFTER the ref - * advanced, the repo is ACTUALLY landed but has NO recorded `landedSha`, so the landedSha check - * above reports NOT-landed → a retry re-runs `landOneRepo`, the CAS rebuilds, and a SECOND squash - * lands (not idempotent). To close the window we ALSO treat the repo as landed when the live - * integration ref carries a commit with THIS task's `Fusion-Task-Id` trailer. - * - * Why a trailer scan and NOT a branch-tip ancestor check: the land is a `git merge --squash`, - * whose squash commit's parent is the integration tip, NOT the task branch — so `merge-base - * --is-ancestor ` is FALSE even right after a successful land. The - * `Fusion-Task-Id` trailer (always stamped onto the squash by `taskTrailers` + the - * ensureTaskMetadata safety net) is the only reliable "this task's work is already on the ref" - * signal that does not depend on the landedSha row, so it is what survives a lost persist. We - * bound the scan to commits the integration tip has gained since the branch's merge-base (the - * land base) so an unrelated historical reuse of the same trailer cannot false-positive. - * - * Exported (A6) so Phase D self-healing reuses THIS canonical predicate instead of - * reimplementing the ancestor/trailer check. - */ -export async function isRepoLanded( - repoRootDir: string, - integrationBranch: string, - landedSha: string | undefined, - taskId?: string, - branch?: string, -): Promise { - const intRef = `refs/heads/${integrationBranch}`; - if (!(await gitOk(["rev-parse", "--verify", intRef], repoRootDir))) { - return false; - } - // Primary: recorded landedSha is an ancestor of (or equals) the integration tip. - // `merge-base --is-ancestor X Y` exits 0 iff X is an ancestor of (or equal to) Y. - if ( - landedSha && - (await gitOk(["merge-base", "--is-ancestor", landedSha, intRef], repoRootDir)) - ) { - return true; - } - // A1 fallback: even without a recorded landedSha, the repo is already landed if the - // integration ref carries a commit with this task's Fusion-Task-Id trailer (the squash - // we lost the persist for). Bound the scan to commits gained since the branch's land base - // so a stale historical trailer of the same id cannot false-positive. - if (taskId) { - const branchRef = branch ? `refs/heads/${branch}` : undefined; - let range = intRef; - if (branchRef && (await gitOk(["rev-parse", "--verify", branchRef], repoRootDir))) { - const base = await gitCapture(["merge-base", branchRef, intRef], repoRootDir); - if (base) range = `${base.trim()}..${intRef}`; - } - const trailer = `${FUSION_TASK_ID_TRAILER_KEY}: ${taskId}`; - const found = await gitCapture( - ["log", "--format=%H", `--grep=${trailer}`, "--fixed-strings", range], - repoRootDir, - ); - if (found && found.trim().length > 0) return true; - } - return false; -} +// FNXC:Workspace 2026-06-22-14:10 (Phase D review G): `isRepoLanded` now lives in +// `workspace-land-predicate.ts` (cycle dissolved). Re-exported here (the imported binding) so +// existing importers of `./merger-ai.js` keep working unchanged. +export { isRepoLanded }; /** * FNXC:Workspace 2026-06-22-00:30 (Phase C U2, KTD3): diff --git a/packages/engine/src/project-engine.ts b/packages/engine/src/project-engine.ts index 5152a36fc1..3c778655d2 100644 --- a/packages/engine/src/project-engine.ts +++ b/packages/engine/src/project-engine.ts @@ -493,6 +493,10 @@ export class ProjectEngine { this.runtime.setMergeActiveClearer?.((taskId) => { this.mergeActive.delete(taskId); }); + // FNXC:Workspace 2026-06-22-16:40 (Phase D P1 TOCTOU): expose the in-memory merge pipeline + // (mergeQueue + mergeActive) to the workspace self-healing reconcilers so they don't + // re-dispatch / reclaim a task that is mid-dequeue→rawMerge. + this.runtime.setMergePendingProvider?.((taskId) => this.isMergePending(taskId)); // Workflow-graph interpreter merge seam: routes through the auto-merge // eligibility gate (requestInterpreterMerge), NOT the human "merge now" // bypass, so a graph merge node can't override an autoMerge-off project. @@ -503,6 +507,26 @@ export class ProjectEngine { return this.activeMergeTaskId; } + /* + FNXC:Workspace 2026-06-22-16:40 (Phase D P1 TOCTOU — merge-queue dispatch blind spot): + A workspace task is "merge-pending" if it sits ANYWHERE in this engine's in-memory merge + pipeline: still queued in `mergeQueue`, OR already dequeued-and-dispatching / actively merging + (tracked by `mergeActive`). `mergeActive.add(taskId)` happens at enqueue time and is only removed + when the merge fully settles (try/finally, stale-merge recovery, or stop()), so it — unlike the + liveness signals the workspace reconcilers consult (session registry, executingTaskLock, + isTaskActive, getActiveMergeTaskId, setStatus("merging"), the workspace-repo-land lease) — covers + the WHOLE dequeue→rawMerge window. In that window `pickNextMergeTaskId` has shifted the id out of + `mergeQueue` but `activeMergeTaskId` / `merging` status / the land lease are not yet set (they fire + later inside the post-semaphore `landWorkspaceTask`). The workspace self-healing reconcilers + (reconcileWorkspacePartialLands / reclaimPhantomWorkspaceLandLeases) call this as a guard so they + never re-dispatch (double-squash) or reclaim the not-yet-registered land lease of a task that is + legitimately mid-dispatch. Because `mergeActive` lingers across the entire dequeue→rawMerge + window, checking it in addition to `mergeQueue` closes that TOCTOU gap. + */ + isMergePending(taskId: string): boolean { + return this.mergeActive.has(taskId) || this.mergeQueue.includes(taskId); + } + /** * Start the engine: initialize the runtime and all auxiliary subsystems. */ diff --git a/packages/engine/src/runtimes/in-process-runtime.ts b/packages/engine/src/runtimes/in-process-runtime.ts index c9a4d1506c..ddf9a15289 100644 --- a/packages/engine/src/runtimes/in-process-runtime.ts +++ b/packages/engine/src/runtimes/in-process-runtime.ts @@ -148,6 +148,13 @@ export class InProcessRuntime ) => Promise; private clearMergeActive?: (taskId: string) => void; private activeMergeTaskIdProvider?: () => string | null; + /** + * FNXC:Workspace 2026-06-22-16:40 (Phase D P1 TOCTOU): predicate that reports whether a task is + * anywhere in ProjectEngine's in-memory merge pipeline (queued OR dequeued-and-merging). Set by + * ProjectEngine before `start()` via `setMergePendingProvider`. Used by the workspace + * self-healing reconcilers to avoid re-dispatching / reclaiming a task mid-dequeue→rawMerge. + */ + private mergePendingProvider?: (taskId: string) => boolean; /** Tracks whether startup recovery was intentionally deferred due to pause state. */ private startupRecoveryDeferred = false; /** Prevent duplicate unpause recovery dispatches from racing each other. */ @@ -797,6 +804,9 @@ export class InProcessRuntime isTaskActive: (taskId: string) => this.executor.isTaskActive(taskId), clearMergeActive: this.clearMergeActive ? (taskId: string) => this.clearMergeActive?.(taskId) : undefined, getActiveMergeTaskId: () => this.activeMergeTaskIdProvider?.() ?? null, + // FNXC:Workspace 2026-06-22-16:40 (Phase D P1 TOCTOU): undefined provider → "not pending" + // (graceful when unwired; existing guards still apply). In production it is always wired. + isMergePending: this.mergePendingProvider ? (taskId: string) => this.mergePendingProvider?.(taskId) ?? false : undefined, leaseManager: this.leaseManager, hasActiveAgentExecution: (agentId: string) => this.heartbeatMonitor?.getTrackedAgents().includes(agentId) ?? false, resumeAssignedTaskForAgent: (agentId: string) => this.executor.resumeTaskForAgent(agentId), @@ -1167,6 +1177,10 @@ export class InProcessRuntime this.activeMergeTaskIdProvider = getActiveMergeTaskId; } + setMergePendingProvider(isMergePending: (taskId: string) => boolean): void { + this.mergePendingProvider = isMergePending; + } + /** * Resume executor/self-healing activity after an unpause transition. * diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 7aae65f4ee..b5b9c91cee 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -46,15 +46,15 @@ import { classifyError, extractMissingModulePath, isNonContinuableSessionError, import { classifyForeignOnlyContamination, deriveTaskIdFromFusionBranch, inspectBranchConflict, listUniqueBranchCommits } from "./branch-conflicts.js"; import { createRunAuditor, generateSyntheticRunId, type DatabaseMutationType, type RunAuditor } from "./run-audit.js"; import { AutoRecoveryDispatcher } from "./auto-recovery.js"; -import { activeSessionRegistry, executingTaskLock, type ActiveSessionKind } from "./active-session-registry.js"; +import { activeSessionRegistry, executingTaskLock } from "./active-session-registry.js"; /* -FNXC:Workspace 2026-06-22-09:30 (Phase D U1): -`isRepoLanded` is the CANONICAL per-repo landed predicate (Phase C, exported A6). Self-healing -reuses it rather than reimplementing the ancestor/trailer check. merger-ai also imports a const -from self-healing (MIN_TEMP_WORKTREE_REAP_AGE_MS), so this is a static cycle — safe because -`isRepoLanded` is only referenced at call time, never at module-eval time. +FNXC:Workspace 2026-06-22-14:10 (Phase D review G — cycle dissolved): +`isRepoLanded` is the CANONICAL per-repo landed predicate (Phase C, exported A6). It now lives in +the dependency-free `workspace-land-predicate` module, NOT merger-ai. Previously self-healing +imported it from merger-ai while merger-ai imports `MIN_TEMP_WORKTREE_REAP_AGE_MS` from +self-healing — a real import cycle. Importing from the predicate module breaks the cycle. */ -import { isRepoLanded } from "./merger-ai.js"; +import { isRepoLanded } from "./workspace-land-predicate.js"; import { findAlreadyMergedTaskCommit } from "./already-merged-detector.js"; import { isAiMergeContainerDir, resolveAiMergeRootPath, resolveLegacyAiMergeRootPath, resolveWorktreesDir } from "./worktree-paths.js"; import { canonicalFusionBranchName, resolveTaskWorkingBranch } from "./worktree-names.js"; @@ -327,6 +327,18 @@ export interface SelfHealingOptions { * Used to avoid clearing a transient merge status mid-merge. */ getActiveMergeTaskId?: () => string | null; + /* + FNXC:Workspace 2026-06-22-16:40 (Phase D P1 TOCTOU — merge-queue dispatch blind spot): + Returns true if the task is ANYWHERE in ProjectEngine's in-memory merge pipeline — queued in + `mergeQueue` OR dequeued-and-merging (`mergeActive`). Unlike `getActiveMergeTaskId` (only the + single in-flight rawMerge) and the session-registry / executingTaskLock / land-lease signals, + this covers the dequeue→rawMerge window where a workspace task is being merged but NONE of those + signals fire yet. The workspace reconcilers consult it before re-enqueuing a partial-land + candidate (prevents a second concurrent `landWorkspaceTask` → double-squash) or reclaiming a + workspace-repo-land lease (the owner is mid-dispatch and is about to register that lease). + Undefined = "not pending" (graceful when unwired); production always wires it. + */ + isMergePending?: (taskId: string) => boolean; /** * Minimum blocker age before stale merge fan-out is cleared from downstream * blockedBy pointers. Must be >= staleMergingStatusMinAgeMs. @@ -717,6 +729,16 @@ export class SelfHealingManager { // ── Per-task deadlock recovery cooldown ───────────────────────────── private deadlockRecoveryCooldown: Map = new Map(); private mergeStarvationDrops: Map = new Map(); + /* + FNXC:Workspace 2026-06-22-14:10 (Phase D review B/E — bounded workspace re-enqueue / orphan-remove): + Per-task drop counter for the workspace partial-land re-enqueue (mirror of `mergeStarvationDrops`): + `enqueueMerge` returns false when the merge queue rejects (full). Without bounding, a perpetually + rejected workspace task is re-enqueued FOREVER. After MAX_STARVATION_DROPS consecutive drops we + park it `status:"failed"`. `orphanWorktreeRemovalFailures` likewise bounds the per-path + `git worktree remove --force` retry in reconcileOrphanedWorkspaceWorktrees. + */ + private workspacePartialLandDrops: Map = new Map(); + private orphanWorktreeRemovalFailures: Map = new Map(); private finalizeUnprovenWarned = new Set(); private metaResolvedSkipAuditMemo = new Map(); private metaStalledSkipAuditMemo = new Map(); @@ -843,6 +865,27 @@ export class SelfHealingManager { return { live, livePaths }; } + /* + FNXC:Workspace 2026-06-22-14:10 (Phase D review C — terminal-owner liveness for lease reclaim): + A `workspace-repo-land` lease may only be reclaimed when its owning task ROW is demonstrably + TERMINAL — i.e. not running anymore in any sense. The Phase-D bug: the prior predicate only + treated an in-review task WITH an active transient merge status as live, so a task still in column + `in-progress` (executing, registered its land lease early, no merge status yet) read as NOT live → + its lease was reclaimed MID-EXECUTION. This predicate inverts to the SAFE direction: the owner is + LIVE unless it is provably terminal — null/missing, `done`, or `failed`. Every other state + (`in-progress`, `in-review` with or without a merge status, `todo`, `triage`, paused, etc.) is + treated as LIVE so we never yank a lease out from under a task that could still be running. The + executing-lock / active-merge-lane checks at the call site are an ADDITIONAL live guard on top of + this. (Distinct from `isWorkspaceTaskLive`, which probes the session REGISTRY; this probes the + task ROW lifecycle.) + */ + private isWorkspaceOwnerLive(owner: Task | null | undefined): boolean { + if (!owner) return false; // not found / deleted → terminal. + if (owner.column === "done") return false; + if (owner.status === "failed") return false; + return true; + } + private async evaluateBackwardMoveTripleProof( task: Task, input: { @@ -5466,7 +5509,13 @@ export class SelfHealingManager { allowsAutoMergeProcessing(t, settings) && !t.paused && !isSharedBranchGroupMemberIntegration(t) && + // FNXC:Workspace 2026-06-22-14:10 (Phase D review A — workspace single-commit-finalize gate): + // This no-op finalize classifies one branch against one base over `this.options.rootDir` + // and moveTask(done)+emitTaskMerged on it. The `Boolean(t.worktree)` gate already excludes + // workspace tasks (their `task.worktree` is null; per-repo worktrees live in + // `workspaceWorktrees`); `!isWorkspaceTask(t)` makes that exclusion explicit and defensive. Boolean(t.worktree) && + !isWorkspaceTask(t) && t.mergeDetails?.mergeConfirmed !== true && t.status !== "merging" && t.status !== "merging-pr" && @@ -6888,6 +6937,13 @@ export class SelfHealingManager { // recover-stale-merging clear STALE ones. A non-transient status (or null) is our domain. !(task.status && ACTIVE_MERGE_STATUSES.has(task.status)), ); + // Drop counters only track LIVE candidates; forget any task that has left the set so a later + // re-appearance starts fresh (mirror of the mergeStarvationDrops cleanup). + const candidateIds = new Set(candidates.map((t) => t.id)); + for (const taskId of [...this.workspacePartialLandDrops.keys()]) { + if (!candidateIds.has(taskId)) this.workspacePartialLandDrops.delete(taskId); + } + if (candidates.length === 0) return 0; let recovered = 0; @@ -6914,6 +6970,21 @@ export class SelfHealingManager { await this.emitWorkspacePartialLandNoAction(task, "live-worktree", liveness.livePaths); continue; } + /* + FNXC:Workspace 2026-06-22-16:40 (Phase D P1 TOCTOU — merge-queue dispatch blind spot): + GUARD 5 — the task is anywhere in ProjectEngine's in-memory merge pipeline (queued or + dequeued-and-dispatching/merging). In the dequeue→rawMerge window the id has been shifted + out of `mergeQueue` but `activeMergeTaskId` / `merging` status / the workspace-repo-land + lease have not yet been set, so GUARDs 1-4 and `isWorkspaceTaskLive` all read "not live". + Re-enqueuing here would launch a SECOND concurrent `landWorkspaceTask(T)`; because a + same-task land lease is explicitly NOT contention, the two don't block → double-squash. + `mergeActive` lingers across the whole window, so this guard closes the gap. Never moves + the task backward; emits no-action and leaves the in-flight dispatch to finish. + */ + if (this.options.isMergePending?.(task.id) === true) { + await this.emitWorkspacePartialLandNoAction(task, "merge-pending", liveness.livePaths); + continue; + } // Classify each acquired sub-repo: landed / retryable / unrecoverable (FORK-A). const workspaceWorktrees = task.workspaceWorktrees ?? {}; @@ -6940,11 +7011,22 @@ export class SelfHealingManager { landedRepos.push(repoRel); continue; } - // Not landed. FORK-A unrecoverable iff the task branch is GONE and nothing landed. + /* + FNXC:Workspace 2026-06-22-14:10 (Phase D review D — FORK-A: branch-gone-and-not-landed + is unrecoverable, regardless of a STALE landedSha): + We are here because `isRepoLanded` returned FALSE — the recorded `landedSha` (if any) is + NOT reachable from the integration tip (branch was force-reset / rolled back / never + actually landed) AND no task-trailer commit is on the ref. The old test was + `!branchPresent && !entry.landedSha`, which let a repo with a STALE landedSha set but + UNREACHABLE, and its `fusion/` branch GONE, fall to `unlandedRepos` → re-enqueued → + `landWorkspaceTask` has NO branch to land → loops forever. Since the repo is provably + NOT landed, the correct test is: branch GONE ⇒ unrecoverable, whether or not a (stale) + landedSha is present. Only a branch that still EXISTS is retryable. + */ const branchPresent = entry.branch ? await this.repoBranchExists(repoRootDir, entry.branch) : false; - if (!branchPresent && !entry.landedSha) { + if (!branchPresent) { unrecoverableRepos.push(repoRel); } else { unlandedRepos.push(repoRel); @@ -6977,25 +7059,23 @@ export class SelfHealingManager { if (unlandedRepos.length === 0) { // Every acquired repo is already landed but the task was never finalized (the finalize // enqueue was dropped). Re-enqueue: landWorkspaceTask skips all repos and finalizes once. - this.options.enqueueMerge?.(task.id); - await this.store.logEntry(task.id, "Auto-recovered (workspace): all sub-repos landed but task not finalized — re-enqueued finalize-once"); - await auditor.database({ - type: "task:reconcile-workspace-partial-land", - target: task.id, - metadata: { taskId: task.id, landedRepos, unlandedRepos: [], failedRepos: [], action: "re-enqueue", reason: "all-landed-not-finalized" }, - }).catch(() => undefined); + await this.enqueueWorkspaceMergeBounded(task, auditor, { + landedRepos, + unlandedRepos: [], + reason: "all-landed-not-finalized", + successLog: "Auto-recovered (workspace): all sub-repos landed but task not finalized — re-enqueued finalize-once", + }); recovered++; continue; } // Partial / none landed, all unlanded repos retryable → re-enqueue the per-repo land. - this.options.enqueueMerge?.(task.id); - await this.store.logEntry(task.id, `Auto-recovered (workspace): re-enqueued partial land (${landedRepos.length} landed, ${unlandedRepos.length} pending)`); - await auditor.database({ - type: "task:reconcile-workspace-partial-land", - target: task.id, - metadata: { taskId: task.id, landedRepos, unlandedRepos, failedRepos: [], action: "re-enqueue", reason: landedRepos.length > 0 ? "partial-land" : "zero-land" }, - }).catch(() => undefined); + await this.enqueueWorkspaceMergeBounded(task, auditor, { + landedRepos, + unlandedRepos, + reason: landedRepos.length > 0 ? "partial-land" : "zero-land", + successLog: `Auto-recovered (workspace): re-enqueued partial land (${landedRepos.length} landed, ${unlandedRepos.length} pending)`, + }); recovered++; } catch (err: unknown) { log.error(`reconcileWorkspacePartialLands: failed for ${task.id}: ${err instanceof Error ? err.message : String(err)}`); @@ -7011,7 +7091,7 @@ export class SelfHealingManager { private async emitWorkspacePartialLandNoAction( task: Task, - reason: "auto-merge-off" | "user-paused" | "live-worktree", + reason: "auto-merge-off" | "user-paused" | "live-worktree" | "merge-pending", livePaths: string[], ): Promise { try { @@ -7031,6 +7111,70 @@ export class SelfHealingManager { } } + /* + FNXC:Workspace 2026-06-22-14:10 (Phase D review B — bounded re-enqueue, no silent infinite loop): + Re-enqueue a workspace task's per-repo land via `enqueueMerge`, CAPTURING the boolean it returns. + `enqueueMerge` returns false when the merge queue rejects (full); the old code discarded it, so a + permanently-rejected task would re-enqueue forever. Mirror `mergeStarvationDrops` in + recoverMergeableReviewTasks: on false, increment a per-task drop counter and after + MAX_STARVATION_DROPS consecutive drops park the task `status:"failed"` (escalate). On a successful + enqueue, reset the counter. When `enqueueMerge` is not wired (option undefined), this is a graceful + no-op (not a crash) — recovery falls back to the next sweep / polling. + Returns true iff the task was parked failed. + */ + private async enqueueWorkspaceMergeBounded( + task: Task, + auditor: RunAuditor, + input: { landedRepos: string[]; unlandedRepos: string[]; reason: string; successLog: string }, + ): Promise { + const enqueueMerge = this.options.enqueueMerge; + if (!enqueueMerge) { + // Option not wired (standalone/tests with no queue) → graceful no-op; rely on next sweep. + this.workspacePartialLandDrops.delete(task.id); + await this.store.logEntry(task.id, `${input.successLog} (enqueue not wired — deferred to next sweep)`); + await auditor.database({ + type: "task:reconcile-workspace-partial-land", + target: task.id, + metadata: { taskId: task.id, landedRepos: input.landedRepos, unlandedRepos: input.unlandedRepos, failedRepos: [], action: "re-enqueue-noop", reason: input.reason }, + }).catch(() => undefined); + return false; + } + + const queued = enqueueMerge(task.id); + if (queued) { + this.workspacePartialLandDrops.delete(task.id); + await this.store.logEntry(task.id, input.successLog); + await auditor.database({ + type: "task:reconcile-workspace-partial-land", + target: task.id, + metadata: { taskId: task.id, landedRepos: input.landedRepos, unlandedRepos: input.unlandedRepos, failedRepos: [], action: "re-enqueue", reason: input.reason }, + }).catch(() => undefined); + return false; + } + + const drops = (this.workspacePartialLandDrops.get(task.id) ?? 0) + 1; + this.workspacePartialLandDrops.set(task.id, drops); + log.warn(`reconcileWorkspacePartialLands: enqueue dropped for ${task.id} (${drops}/${MAX_STARVATION_DROPS}); merge queue rejected re-enqueue`); + if (drops >= MAX_STARVATION_DROPS) { + const error = `Workspace partial-land starvation: ${MAX_STARVATION_DROPS} consecutive enqueue attempts were dropped by the merge queue; task requires manual intervention.`; + await this.store.updateTask(task.id, { status: "failed", error }); + await this.store.logEntry(task.id, error); + this.workspacePartialLandDrops.delete(task.id); + await auditor.database({ + type: "task:reconcile-workspace-partial-land", + target: task.id, + metadata: { taskId: task.id, landedRepos: input.landedRepos, unlandedRepos: input.unlandedRepos, failedRepos: [], action: "park-failed", reason: "enqueue-starvation" }, + }).catch(() => undefined); + return true; + } + await auditor.database({ + type: "task:reconcile-workspace-partial-land", + target: task.id, + metadata: { taskId: task.id, landedRepos: input.landedRepos, unlandedRepos: input.unlandedRepos, failedRepos: [], action: "re-enqueue-dropped", reason: input.reason, drops }, + }).catch(() => undefined); + return false; + } + /** True iff `branch` exists as a local ref in the sub-repo at `repoRootDir`. */ private async repoBranchExists(repoRootDir: string, branch: string): Promise { try { @@ -7061,7 +7205,7 @@ export class SelfHealingManager { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0; - const entries = activeSessionRegistry.entriesByKind("workspace-repo-land" as ActiveSessionKind); + const entries = activeSessionRegistry.entriesByKind("workspace-repo-land"); if (entries.length === 0) return 0; const graceMs = settings.taskStuckTimeoutMs ?? STALE_ACTIVE_BRANCH_EXECUTION_GRACE_MS; @@ -7078,17 +7222,22 @@ export class SelfHealingManager { // A live merge lane / executing owner keeps the lease. if (activeMergeTaskId && activeMergeTaskId === entry.taskId) continue; if (executingTaskLock.has(entry.taskId) || this.options.isTaskActive?.(entry.taskId) === true) continue; + /* + FNXC:Workspace 2026-06-22-16:40 (Phase D P1 TOCTOU — merge-queue dispatch blind spot): + If the owner is anywhere in the in-memory merge pipeline (queued or dequeued-and-merging), + the lease is about to be (or is being) LEGITIMATELY used by an in-flight + `landWorkspaceTask` — it just hasn't registered the lease yet (or registered it this very + instant). `activeMergeTaskId` only names the single in-flight rawMerge and does not cover + the dequeue→rawMerge window, so it can read null here while a dispatch is in progress. + Reclaiming now would yank the lease out from under a live land. Skip; the existing + age-floor + terminal-owner guards still apply once the owner truly settles. + */ + if (this.options.isMergePending?.(entry.taskId) === true) continue; const owner = await this.store.getTask(entry.taskId).catch(() => null); - // Owner is dead/terminal iff: not found, archived/done/failed, OR in-review with NO active - // transient merge status (a merging owner is live; a clean in-review is finished landing). const ownerColumn = owner?.column ?? "deleted"; - const ownerHasActiveMergeStatus = Boolean(owner?.status && ACTIVE_MERGE_STATUSES.has(owner.status)); - const ownerLive = Boolean(owner) - && owner!.column !== "done" - && owner!.status !== "failed" - && ownerHasActiveMergeStatus; - if (ownerLive) continue; // live merging owner → leave its lease alone. + // Only a DEMONSTRABLY TERMINAL owner's lease is reclaimed (review C fix). + if (this.isWorkspaceOwnerLive(owner)) continue; activeSessionRegistry.unregisterPath(entry.path); await createRunAuditor(this.store, { @@ -7142,8 +7291,22 @@ export class SelfHealingManager { if (!worktreePath) continue; // GUARD: skip an active path (mirror self-healing temp-dir sweep isPathActive guard). if (activeSessionRegistry.isPathActive(worktreePath)) continue; - // Nothing on disk → nothing to remove (already cleaned). Skip silently. - if (!existsSync(worktreePath)) continue; + // Nothing on disk → nothing to remove (already cleaned). Skip silently; clear any prior + // failure count so a re-created path starts fresh. + if (!existsSync(worktreePath)) { + this.orphanWorktreeRemovalFailures.delete(worktreePath); + continue; + } + /* + FNXC:Workspace 2026-06-22-14:10 (Phase D review E — bounded + observable orphan removal): + A `git worktree remove --force` failure was caught + audit-logged but NOT engine-logged, + and retried EVERY tick FOREVER (a genuinely stuck path pins this sweep indefinitely). Bound + the retry per-path: after MAX_STARVATION_DROPS consecutive failures stop attempting (leave + the path for manual cleanup) and `log.warn` each failure for observability. + */ + if ((this.orphanWorktreeRemovalFailures.get(worktreePath) ?? 0) >= MAX_STARVATION_DROPS) { + continue; // exhausted retries — stop hammering a stuck path. + } const repoRootDir = join(this.options.rootDir, repoRel); let success = false; @@ -7171,8 +7334,13 @@ export class SelfHealingManager { }); } catch { /* audit best-effort */ } if (success) { + this.orphanWorktreeRemovalFailures.delete(worktreePath); log.log(`reconcileOrphanedWorkspaceWorktrees: removed ${worktreePath} (task ${task.id}, repo ${repoRel})`); cleaned++; + } else { + const failures = (this.orphanWorktreeRemovalFailures.get(worktreePath) ?? 0) + 1; + this.orphanWorktreeRemovalFailures.set(worktreePath, failures); + log.warn(`reconcileOrphanedWorkspaceWorktrees: ${reason} for ${worktreePath} (task ${task.id}, repo ${repoRel}) [${failures}/${MAX_STARVATION_DROPS}]${failures >= MAX_STARVATION_DROPS ? " — giving up; manual cleanup required" : ""}`); } } } @@ -7238,6 +7406,19 @@ export class SelfHealingManager { let repaired = 0; for (const task of candidates) { + /* + FNXC:Workspace 2026-06-22-14:10 (Phase D review F — workspace done-metadata corruption gate): + This reconciler assumes ONE git repo at `this.options.rootDir` and calls `findLandedTaskCommit` + over it. For a workspace task that root is NON-git, so `findLandedTaskCommit` returns null. + `finalizeWorkspaceTask` sets `mergeConfirmed: anyLanded` — a pure NO-OP workspace task (zero + repos landed) is moved to done with `mergeConfirmed:false`, so it reaches the non-confirmed + branch below. There, `landed===null` + a stored `commitSha` would wipe `mergeDetails:undefined` + — corrupting a legitimately-done workspace task's per-repo land map (`workspaceLandedShas`). + The confirmed branch is also meaningless here (no single rootDir commit). Skip workspace tasks + entirely; their mergeDetails are authored once by `finalizeWorkspaceTask` and never need this + single-repo metadata repair. + */ + if (isWorkspaceTask(task)) continue; if (task.mergeDetails?.landedFilesAttributionRestricted || task.mergeDetails?.noOpVerifiedShortCircuit) { log.log(`recoverDoneTaskMergeMetadata: skipped ${task.id} — attribution-restricted`); continue; @@ -7570,6 +7751,30 @@ export class SelfHealingManager { const blockedDependents = dependentsByBlocker.get(task.id) ?? []; const blockedTaskIds = blockedDependents.map((dep) => dep.id); try { + /* + FNXC:Workspace 2026-06-22-14:10 (Phase D review A — P0 workspace gate, TWIN of KTD1): + This is the deadlock-recovery TWIN of recoverInterruptedMergingTasks. Its candidate + filter admits `hasBlockedDependents || Boolean(task.worktree)`, so a workspace task + (task.worktree===null) WITH blocked dependents passes and would reach the single-commit + `findLandedTaskCommit`/moveTask(done)+emitTaskMerged finalize over the NON-git workspace + root — the exact P0: a one-repo commit (or empty) marking a PARTIAL-landed workspace task + fully merged. A workspace task MUST NOT be single-commit-finalized here. Clear the transient + status, leave it in-review, and let the workspace-aware partial-land reconciler + (reconcileWorkspacePartialLands) re-enqueue the idempotent per-repo land. We never move a + workspace task backward here. + */ + if (isWorkspaceTask(task)) { + if (task.status) await this.store.updateTask(task.id, { status: null, error: null }); + this.options.clearMergeActive?.(task.id); + await this.store.logEntry( + task.id, + "Auto-recovery (workspace): cleared stale deadlock 'failed' status; partial-land reconciler owns per-repo re-land (no single-commit finalize)", + ); + log.warn(`self-heal:deadlock-recovery-workspace-skip ${JSON.stringify({ stuckTaskId: task.id, blockedTaskIds, action: "cleared-status-deferred-to-partial-land-reconciler" })}`); + recovered++; + continue; + } + const mergeTarget = await this.resolveSelfHealingMergeTarget(task, settings, "recover-stuck-merge-deadlocks"); const landedCommit = await this.findLandedTaskCommit(task); const landedOnTarget = landedCommit @@ -7739,6 +7944,14 @@ export class SelfHealingManager { let recovered = 0; for (const task of candidates) { try { + /* + FNXC:Workspace 2026-06-22-14:10 (Phase D review A — workspace single-commit-finalize gate): + `findAlreadyMergedTaskCommit` below runs over `this.options.rootDir` (the NON-git workspace + root for a workspace task), and a hit would single-commit-finalize the WHOLE workspace task + done on one phantom/wrong-repo commit (the P0 class). A workspace task lands PER-REPO; its + recovery is owned by reconcileWorkspacePartialLands. Skip it here. + */ + if (isWorkspaceTask(task)) continue; const recentLogs = "getAgentLogs" in this.store && typeof this.store.getAgentLogs === "function" ? await this.store.getAgentLogs(task.id, { limit: 50 }) : []; @@ -7906,6 +8119,14 @@ export class SelfHealingManager { let recovered = 0; for (const task of candidates) { try { + /* + FNXC:Workspace 2026-06-22-14:10 (Phase D review A — workspace single-commit-finalize gate): + `findAlreadyMergedTaskCommit` runs over `this.options.rootDir` (NON-git for a workspace + task) and a hit would single-commit-finalize the whole workspace task done on one + phantom/wrong-repo commit (the P0 class). Workspace tasks land PER-REPO and are recovered + by reconcileWorkspacePartialLands; skip them here. + */ + if (isWorkspaceTask(task)) continue; const mergeTarget = await this.resolveSelfHealingMergeTarget(task, settings, "recover-already-merged-review"); const baseBranch = mergeTarget.branch; if (!baseBranch) continue; @@ -8256,6 +8477,16 @@ export class SelfHealingManager { let recovered = 0; for (const task of candidates) { try { + /* + FNXC:Workspace 2026-06-22-14:10 (Phase D review A — workspace single-commit-finalize gate): + A workspace task carries a `task.branch` (`fusion/`) even though it lands PER-REPO, so + the `Boolean(task.branch)` candidate filter does NOT exclude it. `isBranchTipMisboundToTask` + + `findAlreadyMergedTaskCommit` run over `this.options.rootDir` (NON-git for a workspace + task); a hit would single-commit-finalize the whole task done on one wrong-repo/phantom + commit (the P0 class). Today the rootDir git calls merely error-by-accident; gate it + explicitly. Workspace recovery is owned by reconcileWorkspacePartialLands. + */ + if (isWorkspaceTask(task)) continue; const branch = task.branch; if (!branch) continue; const mergeTarget = await this.resolveSelfHealingMergeTarget(task, settings, "recover-branch-misbound-in-review"); diff --git a/packages/engine/src/workspace-land-predicate.ts b/packages/engine/src/workspace-land-predicate.ts new file mode 100644 index 0000000000..5f903592b7 --- /dev/null +++ b/packages/engine/src/workspace-land-predicate.ts @@ -0,0 +1,119 @@ +/* +FNXC:Workspace 2026-06-22-14:10 (Phase D review G — dissolve self-healing ↔ merger-ai cycle): +`isRepoLanded` is a PURE per-repo git predicate. It used to live in merger-ai.ts, but Phase D +self-healing imports it (`self-healing.ts` → `merger-ai.ts`) while `merger-ai.ts` already imports +`MIN_TEMP_WORKTREE_REAP_AGE_MS` from `self-healing.ts` — a real import cycle. Moving the predicate +(plus the two tiny read-only git helpers it needs) into this dependency-free module breaks the +cycle: BOTH merger-ai.ts and self-healing.ts import from here, and neither imports the other for +this predicate. The module pulls in NOTHING beyond node:child_process, so it is a clean extraction. +The public `isRepoLanded` export from index.ts is preserved by re-exporting from this module. +*/ +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; + +const execFileAsync = promisify(execFile); + +/** Canonical Fusion task-id trailer key stamped on every land squash commit. */ +export const FUSION_TASK_ID_TRAILER_KEY = "Fusion-Task-Id"; + +async function git(args: string[], cwd: string, opts: { timeout?: number } = {}): Promise { + const { stdout } = await execFileAsync("git", args, { + cwd, + encoding: "utf-8", + timeout: opts.timeout ?? 120_000, + maxBuffer: 16 * 1024 * 1024, + }); + return stdout.trim(); +} + +/** Run git, returning true on exit 0 and false on any failure (read-only probes). */ +async function gitOk(args: string[], cwd: string): Promise { + try { + await git(args, cwd); + return true; + } catch { + return false; + } +} + +/** + * FNXC:Workspace 2026-06-22-04:10 (Phase C review A1): + * Capture git stdout, returning undefined (never throwing) on failure — for read-only + * probes (merge-base, log --grep) where a non-zero exit is an expected "not found". + */ +async function gitCapture(args: string[], cwd: string): Promise { + try { + return await git(args, cwd); + } catch { + return undefined; + } +} + +/** + * FNXC:Workspace 2026-06-22-00:30 (Phase C U2, KTD3): + * Landed predicate: a sub-repo is landed iff a `landedSha` is recorded AND that sha is + * an ancestor of (or equals) the repo's CURRENT integration tip. The ancestor check + * (not just sha presence) survives a later un-related advance of the integration ref: + * the landed commit is still reachable, so the repo stays "landed". A `landedSha` that + * is NOT reachable from the tip (e.g. the ref was reset/rebuilt) reads as NOT landed and + * the repo re-lands. + * + * FNXC:Workspace 2026-06-22-04:10 (Phase C review A1 — task-trailer ancestor fallback): + * The double-land window: a land advances the integration ref via `advanceIntegrationBranchRef`'s + * CAS, then `persistRepoLandedSha` records `landedSha`. If that DB write fails AFTER the ref + * advanced, the repo is ACTUALLY landed but has NO recorded `landedSha`, so the landedSha check + * above reports NOT-landed → a retry re-runs `landOneRepo`, the CAS rebuilds, and a SECOND squash + * lands (not idempotent). To close the window we ALSO treat the repo as landed when the live + * integration ref carries a commit with THIS task's `Fusion-Task-Id` trailer. + * + * Why a trailer scan and NOT a branch-tip ancestor check: the land is a `git merge --squash`, + * whose squash commit's parent is the integration tip, NOT the task branch — so `merge-base + * --is-ancestor ` is FALSE even right after a successful land. The + * `Fusion-Task-Id` trailer (always stamped onto the squash by `taskTrailers` + the + * ensureTaskMetadata safety net) is the only reliable "this task's work is already on the ref" + * signal that does not depend on the landedSha row, so it is what survives a lost persist. We + * bound the scan to commits the integration tip has gained since the branch's merge-base (the + * land base) so an unrelated historical reuse of the same trailer cannot false-positive. + * + * Exported (A6) so Phase D self-healing reuses THIS canonical predicate instead of + * reimplementing the ancestor/trailer check. + */ +export async function isRepoLanded( + repoRootDir: string, + integrationBranch: string, + landedSha: string | undefined, + taskId?: string, + branch?: string, +): Promise { + const intRef = `refs/heads/${integrationBranch}`; + if (!(await gitOk(["rev-parse", "--verify", intRef], repoRootDir))) { + return false; + } + // Primary: recorded landedSha is an ancestor of (or equals) the integration tip. + // `merge-base --is-ancestor X Y` exits 0 iff X is an ancestor of (or equal to) Y. + if ( + landedSha && + (await gitOk(["merge-base", "--is-ancestor", landedSha, intRef], repoRootDir)) + ) { + return true; + } + // A1 fallback: even without a recorded landedSha, the repo is already landed if the + // integration ref carries a commit with this task's Fusion-Task-Id trailer (the squash + // we lost the persist for). Bound the scan to commits gained since the branch's land base + // so a stale historical trailer of the same id cannot false-positive. + if (taskId) { + const branchRef = branch ? `refs/heads/${branch}` : undefined; + let range = intRef; + if (branchRef && (await gitOk(["rev-parse", "--verify", branchRef], repoRootDir))) { + const base = await gitCapture(["merge-base", branchRef, intRef], repoRootDir); + if (base) range = `${base.trim()}..${intRef}`; + } + const trailer = `${FUSION_TASK_ID_TRAILER_KEY}: ${taskId}`; + const found = await gitCapture( + ["log", "--format=%H", `--grep=${trailer}`, "--fixed-strings", range], + repoRootDir, + ); + if (found && found.trim().length > 0) return true; + } + return false; +}