fix(review): Phase D self-healing hardening — finalize-site audit, lease/TOCTOU safety
4-persona review of the Phase-D workspace self-healing. The headline: the P0 single-commit-finalize guard had to be applied across ALL surfaces, not just the one reconciler U1 patched (FN-5893). Finalize-site audit (A): gated every site where a workspace task could be single-commit-finalized on one repo's commit — recoverStuckMergeDeadlocks (the twin of the U1-patched reconciler, reachable via blocked-dependents), recoverOrphanOnlyScopeViolations, recoverAlreadyMergedReviewTasks, recoverBranchMisboundInReviewTasks (workspace tasks carry task.branch so the Boolean(branch) filter didn't exclude them), plus a defensive filter on finalizeNoOpReviewTasks. recoverMergedReviewTasks confirmed safe (mergeConfirmed gate). Each is an isWorkspaceTask early-skip; single-repo behavior unchanged. Reliability/concurrency: - The partial-land reconciler now captures enqueueMerge's boolean and bounds re-enqueues (mergeStarvationDrops → fail after N) instead of looping silently forever on a full queue. - The phantom-lease reclaim only acts on a terminal owner (null/done/failed) — it no longer reclaims the lease of an in-progress executing task that registered it early (shared isWorkspaceOwnerLive predicate). - A new isMergePending(taskId) = mergeActive ∪ mergeQueue seam (exposed from ProjectEngine, wired through the runtime) guards both reconcilers against the merge-queue dispatch window — a task dequeued-but-not-yet-merging is no longer re-enqueued (which, since a same-task land lease isn't contention, could have caused a concurrent double-squash). - FORK-A: a repo whose branch is gone and which isn't landed is parked, not re-enqueued forever. Orphan-worktree removal failures log.warn + bound. recoverDoneTaskMergeMetadata skips workspace tasks. Maintainability: dissolved the self-healing↔merger-ai import cycle by moving isRepoLanded into a dependency-free workspace-land-predicate.ts; removed a redundant cast. Gate green: build, typecheck, lint, test:gate (649+58); self-healing + e2e + project-engine + merger 724. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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).
|
||||
|
||||
@@ -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<typeof vi.fn>).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<Task["workspaceWorktrees"]>;
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<boolean> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<string | undefined> {
|
||||
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 <branch> <integration>` 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<boolean> {
|
||||
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):
|
||||
|
||||
@@ -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.
|
||||
*/
|
||||
|
||||
@@ -148,6 +148,13 @@ export class InProcessRuntime
|
||||
) => Promise<import("@fusion/core").MergeResult>;
|
||||
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.
|
||||
*
|
||||
|
||||
@@ -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<string, number> = new Map();
|
||||
private mergeStarvationDrops: Map<string, number> = 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<string, number> = new Map();
|
||||
private orphanWorktreeRemovalFailures: Map<string, number> = new Map();
|
||||
private finalizeUnprovenWarned = new Set<string>();
|
||||
private metaResolvedSkipAuditMemo = new Map<string, string>();
|
||||
private metaStalledSkipAuditMemo = new Map<string, string>();
|
||||
@@ -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/<id>` 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<void> {
|
||||
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<boolean> {
|
||||
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<boolean> {
|
||||
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/<id>`) 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");
|
||||
|
||||
119
packages/engine/src/workspace-land-predicate.ts
Normal file
119
packages/engine/src/workspace-land-predicate.ts
Normal file
@@ -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<string> {
|
||||
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<boolean> {
|
||||
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<string | undefined> {
|
||||
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 <branch> <integration>` 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<boolean> {
|
||||
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;
|
||||
}
|
||||
Reference in New Issue
Block a user