diff --git a/docs/architecture.md b/docs/architecture.md index 8a5863a9b1..7e1b1d26be 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1800,7 +1800,7 @@ This section preserves the detailed lifecycle/self-healing contracts that were f - **Scheduler overlap priority/age guard (FN-5325)**: with `groupOverlappingFiles=true`, scheduler now defers a lower-priority (or younger same-priority) candidate when an overlapping queued todo task exists, preserving priority→age→task-id order for overlap serialization without preempting in-progress work. If the inversion is against an already-running lower-priority blocker, scheduler still defers the candidate; the per-pairing audit event was removed in FN-6174 due to zero consumers and table bloat. - **Empty-commit refusal + early empty-own-diff finalize (FN-5345/FN-5377)**: Fusion task worktrees install a `prepare-commit-msg` hook that refuses `git commit --allow-empty` and other zero-staged-diff commits, preventing verification-only tasks from manufacturing empty handoff commits that defeat the merger's no-op classifier. The hook allows legitimate empty-tree paths (amend, merge, squash, cherry-pick, revert, rebase). Amend detection tokenizes the parent process command line (`ps -o args=` with `/proc/$PPID/cmdline` fallback for Alpine/busybox) and stops at the first message-supplying flag (`-m`/`-F`/`--message`/`--file`) so a commit message containing the substring `--amend` cannot bypass the guard. In `aiMergeTask`, an early empty-own-diff fast-path runs BEFORE any reuse-handoff acquisition: when integration mode is `reuse-task-worktree`, the branch exists, `git rev-list --count ..` is > 0, and `git diff --quiet ..` exits 0, the task auto-finalizes as no-op with `mergeDetails.noOpMerge: true` and emits `task:auto-recover-finalize-already-on-main` with `reason: "empty-own-diff-early-fast-path"`. The fast-path best-effort removes the stranded worktree (FN-4811 same-task/foreign-owner guard) and deletes the `fusion/` branch so empty-own-diff residuals do not accumulate. This unsticks tasks where a stale empty handoff commit combined with drifted worktree↔branch mapping would otherwise wedge the handoff gate with `registered-branch-mismatch`. The explicit `cwd-integration-branch` mode is unchanged (`cwd-main` remains a deprecated alias normalized to it). `classifyOwnedLandedEvidence` also detects empty-own-diff (aheadCount > 0, zero net diff) and returns `proven-no-op` so downstream self-healing and post-handoff finalize paths benefit too. Additionally, merger's reuse-fallback path now consults `git worktree list --porcelain` before creating a new worktree: extant usable registrations of `fusion/` are reused directly (rather than blindly `git worktree add -f` producing a duplicate registration), and stale registrations are pruned first. The direct-reuse shortcut is guarded by FN-4811 (refuses paths owned by a different task in `activeSessionRegistry`) and FN-4954 (skipped when `recycleWorktrees=true` with a pool attached, so `WorktreePool.acquire` lease bookkeeping stays consistent). Two audit subtypes — `merge:reuse-fallback-pruned-stale-registration` and `merge:reuse-fallback-reused-existing-registration` — replace the prior overloading of `merge:reuse-fallback-new-worktree` for these cases. - **Verified no-op/duplicate executor completion (FN-6275)**: explicit `fn_task_done` may complete with zero branch commits only when the summary starts with a recognized sentinel (`PREMISE STALE:`, `NO-OP:`, `NOOP:`, `DUPLICATE: FN-NNNN ...`, or `REDUNDANT:`) or the task already carries a no-commit contract. The sentinel only relaxes the `no_commits` invariant; `wrong_toplevel`, `wrong_branch`, pending-step/review refusals, and scope-leak guards still run. Accepted sentinel completions persist `noCommitsExpected: true`, write task-log audit details with marker kind/reason/raw summary/run/agent IDs, and add a task timeline activity so the no-code terminal path remains explainable. Ordinary zero-commit implementation completions without a leading sentinel are still refused. -- **In-review branch-binding self-heal (FN-5083)**: `reconcile-in-review-branch-rebind` runs after `reconcile-task-worktree-metadata` and before `reclaim-stale-active-branches`. It restores `task.branch` (and clears `task.worktree` for fresh acquisition) for `in-review` tasks when exactly one case-insensitive `fusion/` candidate branch has unique commits versus the integration base. Ambiguous candidates emit `task:auto-rebind-skipped` (`reason: "ambiguous-candidates"`) and are never auto-resolved. Branch construction across executor/worktree-pool/worktree-acquisition/merger/self-healing canonicalizes to lowercase via `canonicalFusionBranchName`; `fn_task_done` wrong-branch checks now auto-canonicalize case-only mismatches and emit `branch:auto-canonicalize-case`. +- **In-review branch-binding self-heal (FN-5083/FN-6695)**: `reconcile-in-review-branch-rebind` runs after `reconcile-task-worktree-metadata` and before `reclaim-stale-active-branches`. It restores `task.branch` (and clears `task.worktree` for fresh acquisition) for `in-review` tasks when exactly one case-insensitive `fusion/` candidate branch has unique commits versus the integration base. Ambiguous candidates emit `task:auto-rebind-skipped` (`reason: "ambiguous-candidates"`) and are never auto-resolved. Unsafe metadata repair is also skipped with `task:auto-rebind-skipped`: `userPaused` preserves authoritative user intent, and `checkedOutBy` preserves live agent checkout ownership. Branch construction across executor/worktree-pool/worktree-acquisition/merger/self-healing canonicalizes to lowercase via `canonicalFusionBranchName`; `fn_task_done` wrong-branch checks now auto-canonicalize case-only mismatches and emit `branch:auto-canonicalize-case`. - **In-review is terminal-until-merged under `autoMerge: false` (FN-5147)**: when a project sets `settings.autoMerge: false`, `in-review` is the intended resting state until a human merges the PR. No lifecycle-mutating self-healing sweep (`reclaimSelfOwnedBranchConflicts`, `recoverGhostReviewTasks`, `recoverStaleIncompleteReviewTasks`, `recoverInterruptedMergingTasks`, `recoverStuckMergeDeadlocks`, `recoverMissingWorktreeReviewFailures`, `recoverPartialProgressNoTaskDoneFailures`, `recoverCompletionHandoffLimbo`, `recoverPostDoneNonContinuableWedge`, `recoverMergeableReviewTasks`, `recoverMergedReviewTasks`, `recoverAlreadyMergedReviewTasks`, `recoverOrphanOnlyScopeViolations`, `recoverForeignOnlyContaminatedInReviewTasks`, `recoverReviewTasksWithFailedPreMergeSteps`, `finalizeNoOpReviewTasks`, `surfaceInReviewStalls`, `surfaceInReviewStalled`) may move the task out of `in-review`, mark it `paused`/`failed`, or re-enqueue it for execution. Explicit per-task overrides are distinguished by `task.autoMergeProvenance: "user"`; ambiguous legacy rows stamped `autoMerge: true` by the pre-FN-6245 review-entry path are marked `"legacy-stamp"` once and surfaced in run-audit/logs, but are only cleared by the operator-driven `reconcileLegacyAutoMergeStamps({ apply: true })` action. Scoped FN-5819 exception: shared-group members (`branchContext.assignmentMode === "shared"`) are still allowed through the member→`branch_groups.branchName` integration step while `autoMerge` is off; this is a soft pre-integration only and does not permit shared-branch → default-branch promotion. RECONCILE-ONLY sweeps (branch rebind, blocker fan-out, stale-status clears, contamination metadata cleanup, attribution restore, PR refresh, misclassified-failure error clearing) continue to run. - **Auto-merge integration-root default (FN-5279)**: direct auto-merge now defaults `mergeIntegrationWorktree` to `reuse-task-worktree`; merger must pass the reuse handoff gates or emit `merge:reuse-handoff-refused` and leave the task in `in-review` without silently falling back to `cwd-integration-branch` (`cwd-main` remains a deprecated alias normalized to that mode). - **Orphaned execution sweep is observation-only (FN-5337)**: `recoverOrphanedExecutions` only annotates stale in-progress candidates with `task:orphan-detected-no-action` and `[orphan-detected] ... no action (operator-decides)` logs. It must never move `in-progress`/`in-review` backward to `todo` or mutate lease/worktree metadata. Proof-based backward recovery remains exclusively in `recoverInProgressLimbo` (FN-5219), `RestartRecoveryCoordinator`, `recoverMissingWorktreeReviewFailures`, and explicit executor/merger failure paths. Reintroducing lifecycle mutation here requires hard git/session proof gating plus CEO+CTO+PM sign-off. diff --git a/packages/engine/src/__tests__/reliability-interactions/in-review-branch-rebind.test.ts b/packages/engine/src/__tests__/reliability-interactions/in-review-branch-rebind.test.ts index fee1aa2771..30c5484794 100644 --- a/packages/engine/src/__tests__/reliability-interactions/in-review-branch-rebind.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/in-review-branch-rebind.test.ts @@ -53,17 +53,106 @@ describe("FN-5083 reliability interactions: in-review branch rebind", () => { return branch; } + function spyOnRebindAudit(manager: SelfHealingManager) { + return vi.spyOn(manager as any, "emitBranchRebindAuditEvent"); + } + it("rebinds and remains stable on repeated sweeps", async () => { const id = await createTaskInReview("stable rebind"); const branch = await createUniqueFusionBranch(id, "stable"); await store.updateTask(id, { branch: null, worktree: null }); const manager = new SelfHealingManager(store, { rootDir }); + const audit = spyOnRebindAudit(manager); const first = await manager.reconcileInReviewBranchRebind({ includeTaskIds: new Set([id]) }); const second = await manager.reconcileInReviewBranchRebind({ includeTaskIds: new Set([id]) }); + const updated = await store.getTask(id); expect(first.outcomes).toEqual(expect.arrayContaining([expect.objectContaining({ taskId: id, result: "applied", branch })])); - expect(second.outcomes).toEqual(expect.arrayContaining([expect.objectContaining({ taskId: id })])); + expect(updated?.branch).toBe(branch); + expect(audit).toHaveBeenCalledWith(expect.objectContaining({ + taskId: id, + mutationType: "task:auto-rebind-applied", + metadata: expect.objectContaining({ branch, source: "auto-rebind-in-review" }), + })); + expect(second.outcomes).toEqual(expect.arrayContaining([ + expect.objectContaining({ taskId: id, result: "skipped", reason: "binding-intact" }), + ])); + }); + + it("skips auto-rebind when user-paused task intent blocks engine mutation", async () => { + const id = await createTaskInReview("user paused safety gate"); + const branch = await createUniqueFusionBranch(id, "paused"); + const brokenBranch = `fusion/missing-${id.toLowerCase()}`; + await store.updateTask(id, { branch: brokenBranch, worktree: null }); + const originalListTasks = store.listTasks.bind(store); + vi.spyOn(store, "listTasks").mockImplementationOnce(async (opts: any) => { + const tasks = await originalListTasks(opts); + return tasks.map((task: any) => task.id === id ? { ...task, userPaused: true } : task); + }); + + const manager = new SelfHealingManager(store, { rootDir }); + const audit = spyOnRebindAudit(manager); + const result = await manager.reconcileInReviewBranchRebind({ includeTaskIds: new Set([id]) }); + const updated = await store.getTask(id); + + expect(result.outcomes).toEqual(expect.arrayContaining([ + expect.objectContaining({ taskId: id, result: "skipped", reason: "unsafe-to-auto-mutate:user-paused" }), + ])); + expect(updated?.branch).toBe(brokenBranch); + expect(updated?.branch).not.toBe(branch); + expect(audit).toHaveBeenCalledWith(expect.objectContaining({ + taskId: id, + mutationType: "task:auto-rebind-skipped", + metadata: expect.objectContaining({ + reason: "unsafe-to-auto-mutate:user-paused", + branch, + }), + })); + }); + + it("skips auto-rebind when a checked-out task has a live metadata lease", async () => { + const id = await createTaskInReview("checked out safety gate"); + const branch = await createUniqueFusionBranch(id, "checked-out"); + const brokenBranch = `fusion/missing-${id.toLowerCase()}`; + await store.updateTask(id, { branch: brokenBranch, worktree: null, checkedOutBy: "agent-123" }); + + const manager = new SelfHealingManager(store, { rootDir }); + const audit = spyOnRebindAudit(manager); + const result = await manager.reconcileInReviewBranchRebind({ includeTaskIds: new Set([id]) }); + const updated = await store.getTask(id); + + expect(result.outcomes).toEqual(expect.arrayContaining([ + expect.objectContaining({ taskId: id, result: "skipped", reason: "unsafe-to-auto-mutate:checked-out" }), + ])); + expect(updated?.branch).toBe(brokenBranch); + expect(updated?.branch).not.toBe(branch); + expect(audit).toHaveBeenCalledWith(expect.objectContaining({ + taskId: id, + mutationType: "task:auto-rebind-skipped", + metadata: expect.objectContaining({ + reason: "unsafe-to-auto-mutate:checked-out", + branch, + }), + })); + }); + + it("metadata-repairs safe autoMerge false in-review tasks without lifecycle mutation", async () => { + const id = await createTaskInReview("manual merge metadata repair"); + const branch = await createUniqueFusionBranch(id, "manual-merge"); + const mainSha = git(rootDir, "rev-parse main"); + await store.updateTask(id, { branch: null, worktree: null, baseCommitSha: mainSha, autoMerge: false }); + + const manager = new SelfHealingManager(store, { rootDir }); + const result = await manager.reconcileInReviewBranchRebind({ includeTaskIds: new Set([id]) }); + const updated = await store.getTask(id); + + expect(result.outcomes).toEqual(expect.arrayContaining([expect.objectContaining({ taskId: id, result: "applied", branch })])); + expect(updated?.branch).toBe(branch); + expect(updated?.column).toBe("in-review"); + expect(updated?.status).not.toBe("failed"); + expect(updated?.paused).not.toBe(true); + expect(updated?.baseCommitSha).toBe(mainSha); }); it("preserves FN-4962 ordering with metadata reconcile before rebind", async () => { @@ -88,8 +177,48 @@ describe("FN-5083 reliability interactions: in-review branch rebind", () => { const manager = new SelfHealingManager(store, { rootDir }); const result = await manager.reconcileInReviewBranchRebind({ includeTaskIds: new Set([id]) }); + const updated = await store.getTask(id); expect(result.outcomes).toEqual(expect.arrayContaining([expect.objectContaining({ taskId: id, result: "applied", branch })])); + expect(updated?.baseCommitSha).toMatch(/^[0-9a-f]{40}$/); + }); + + it("preserves no-live-branch skip outcome", async () => { + const id = await createTaskInReview("no live branch"); + await store.updateTask(id, { branch: null, worktree: null }); + + const manager = new SelfHealingManager(store, { rootDir }); + const audit = spyOnRebindAudit(manager); + const result = await manager.reconcileInReviewBranchRebind({ includeTaskIds: new Set([id]) }); + + expect(result.outcomes).toEqual(expect.arrayContaining([ + expect.objectContaining({ taskId: id, result: "skipped", reason: "no-live-branch" }), + ])); + expect(audit).toHaveBeenCalledWith(expect.objectContaining({ + taskId: id, + mutationType: "task:auto-rebind-skipped", + metadata: expect.objectContaining({ reason: "no-live-branch" }), + })); + }); + + it("preserves no-unique-work skip outcome", async () => { + const id = await createTaskInReview("no unique work"); + const branch = `fusion/${id.toLowerCase()}`; + git(rootDir, `branch ${branch} main`); + await store.updateTask(id, { branch: null, worktree: null }); + + const manager = new SelfHealingManager(store, { rootDir }); + const audit = spyOnRebindAudit(manager); + const result = await manager.reconcileInReviewBranchRebind({ includeTaskIds: new Set([id]) }); + + expect(result.outcomes).toEqual(expect.arrayContaining([ + expect.objectContaining({ taskId: id, result: "skipped", reason: "no-unique-work" }), + ])); + expect(audit).toHaveBeenCalledWith(expect.objectContaining({ + taskId: id, + mutationType: "task:auto-rebind-skipped", + metadata: expect.objectContaining({ reason: "no-unique-work" }), + })); }); it("skips ambiguous case-variant candidates when filesystem permits both refs", async () => { diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index d5e91b5e02..d4a661630c 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -517,10 +517,24 @@ type RebindOutcome = | { taskId: string; result: "skipped"; - reason: "binding-intact" | "no-live-branch" | "ambiguous-candidates" | "no-unique-work"; + reason: + | "binding-intact" + | "no-live-branch" + | "ambiguous-candidates" + | "no-unique-work" + | "unsafe-to-auto-mutate:user-paused" + | "unsafe-to-auto-mutate:checked-out"; candidates?: Array<{ branch: string; aheadCount: number }>; }; +type AutoRebindSafetyResult = + | { safe: true } + | { + safe: false; + reason: "unsafe-to-auto-mutate:user-paused" | "unsafe-to-auto-mutate:checked-out"; + detail: string; + }; + export type RebindResult = { repaired: number; outcomes: RebindOutcome[] }; interface LandedTaskCommit { @@ -3561,6 +3575,29 @@ export class SelfHealingManager { } } + private assertSafeToAutoRebind(task: Task): AutoRebindSafetyResult { + /* + FNXC:SelfHealingRebind 2026-06-19-12:00: + In-review branch rebind is metadata repair only, but it is still an engine-owned mutation. + Block instead of warn when authoritative user intent or a live checkout is present so recovery never overrides a user pause or rewrites task metadata underneath an active agent lease. + */ + if (task.userPaused === true) { + return { + safe: false, + reason: "unsafe-to-auto-mutate:user-paused", + detail: "task is user-paused; authoritative user intent blocks automatic branch rebind", + }; + } + if (task.checkedOutBy) { + return { + safe: false, + reason: "unsafe-to-auto-mutate:checked-out", + detail: `task is checked out by ${task.checkedOutBy}; automatic branch rebind would mutate metadata under a live lease`, + }; + } + return { safe: true }; + } + private async emitBranchRebindAuditEvent(input: { taskId: string; mutationType: "task:auto-rebind-applied" | "task:auto-rebind-skipped"; @@ -3703,13 +3740,24 @@ export class SelfHealingManager { patch.baseCommitSha = derivedBaseCommit; } } - // TODO(FN-5066): tighten composition once helper API is final. - try { - const maybeAsserting = this as unknown as { assertSafeToAutoMutate?: (opts: unknown) => Promise }; - await maybeAsserting.assertSafeToAutoMutate?.({ taskId: task.id, reason: "in-review-branch-rebind" }); - } catch (assertErr: unknown) { - const message = assertErr instanceof Error ? assertErr.message : String(assertErr); - log.warn(`[self-healing] assertSafeToAutoMutate warning for ${task.id}: ${message}; continuing rebind`); + const safety = this.assertSafeToAutoRebind(task); + if (!safety.safe) { + await this.emitBranchRebindAuditEvent({ + taskId: task.id, + mutationType: "task:auto-rebind-skipped", + metadata: { + taskId: task.id, + reason: safety.reason, + detail: safety.detail, + branch: selected.branch, + aheadCount: selected.aheadCount, + integrationBase, + source: "auto-rebind-in-review", + previousBranch: task.branch ?? null, + }, + }); + result.outcomes.push({ taskId: task.id, result: "skipped", reason: safety.reason }); + continue; } await this.store.updateTask(task.id, patch); await this.emitBranchRebindAuditEvent({