fix(FN-4962): address step 4 review feedback
Fusion-Task-Id: FN-4962 Fusion-Task-Lineage: de4ce54b-c5e4-40f9-a048-677adac3e1a0
This commit is contained in:
committed by
gsxdsm
parent
d13ee7f526
commit
d5ed2cd2b4
5
.changeset/FN-4962-stale-worktree-metadata-reconcile.md
Normal file
5
.changeset/FN-4962-stale-worktree-metadata-reconcile.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Reconcile stale task worktree/branch metadata after orphan recovery so the dashboard Changes view shows the correct diffs.
|
||||
@@ -170,6 +170,7 @@ Detailed mechanism logs live in `docs/architecture.md` and `docs/design/`. The c
|
||||
|
||||
- **Orphan `fusion/*` branches**: prune-or-rescue, never force-delete. Subsumed branches pruned; unique-commit branches rescued into triage tasks.
|
||||
- **Stale active branches**: self-healing's `reclaim-stale-active-branches` stage prunes a `fusion/<task-id>` branch with zero unique commits when no usable worktree mapping exists, then clears `task.branch`/`task.worktree`/`task.baseCommitSha`.
|
||||
- **Worktree metadata reconcile ordering (FN-4962)**: `reconcile-task-worktree-metadata` must run before `reclaim-stale-active-branches`; stale `task.worktree` metadata is rebound to live `fusion/<task-id>` worktrees when present (`task:auto-recover-worktree-metadata-rebound`) or cleared (`task:auto-recover-worktree-metadata-cleared`) when absent.
|
||||
- **Completion fan-out is synchronous**: `SelfHealingManager.reconcileCompletedTask()` runs on `in-review → done`. Downstream stale `blockedBy` links and residual `fusion/<task-id>` branch/worktree artifacts are reconciled immediately, not on a periodic sweep.
|
||||
- **In-review stall deadlock**: identical stalls (same code + reason) repeated past `inReviewStallDeadlockThreshold` (default 3) auto-pause with `pausedReason: "in-review-stall-deadlock"` and `status: "failed"`.
|
||||
- **Restart recovery**: `RestartRecoveryCoordinator` classifies interrupted `in-progress` runs. Unusable-worktree session-start failures (`missing`, `incomplete`, `unregistered git worktree`) are recoverable; retries are capped at `MAX_WORKTREE_SESSION_RETRIES=3` before escalating.
|
||||
|
||||
@@ -668,6 +668,7 @@ When stuck-kill retries are exhausted, `checkStuckBudget()` marks the task `stat
|
||||
- `recoverMissingWorktreeReviewFailures()` is a narrow failed-review recovery: only `status: "failed"` `in-review` tasks with the explicit session-start signature `Refusing to start coding agent in missing worktree:` (from `assertValidWorktreeSession()`) are requeued. Recovery clears stale session metadata (`worktree`, `branch`, `sessionFile`, transient failure state), preserves valid step progress/retry counters, logs the auto-recovery reason, and moves the task back to `todo` for a clean retry.
|
||||
- `recoverMergeableReviewTasks()` only re-enqueues truly eligible tasks; retry-exhausted review tasks are skipped to avoid re-enqueue/no-op loops that keep refreshing `updatedAt`.
|
||||
- `recoverAlreadyMergedReviewTasks()` auto-finalizes retry-exhausted `in-review` tasks when self-healing can prove their work already landed on the merge target. On this landed-content path it clears soft blockers (`paused`, stale `status: "failed"`, and residual `error`) before moving to `done`; true hard blockers (for example incomplete steps, awaiting-user-review, or failed pre-merge workflow steps) still park the task in stable `in-review/failed` state with a blocker error instead of entering an auto-finalize loop.
|
||||
- `reconcileTaskWorktreeMetadata()` (FN-4962) reconciles stale `task.worktree`/`task.branch` rows against authoritative `git worktree list --porcelain` branch mappings during startup recovery, periodic maintenance, and completion fan-out. The stage must run before `reclaim-stale-active-branches`: stale rows rebound to live `fusion/<id>` worktrees emit `task:auto-recover-worktree-metadata-rebound`; stale rows with no live branch mapping are nulled (`worktree=null`, `branch=null`, `baseCommitSha` unchanged) and emit `task:auto-recover-worktree-metadata-cleared`.
|
||||
|
||||
##### Orphan-only scope-violation auto-recovery
|
||||
`recoverOrphanOnlyScopeViolations()` handles the narrow FN-4350 shape without weakening the file-scope invariant: it runs only when all of these predicates hold — task is `column === "in-review"`; task is failed (`status === "failed"`, with engine/global pause both off); error evidence is a FileScopeViolation (`tool_error` agent-log payload from `formatFileScopeViolationAgentLog`, with `task.error` prefix fallback); `task.scopeOverride !== true`; task is not actively executing and `mergeDetails.mergeConfirmed !== true`. It then verifies the task's specific work is already on `main` using `findAlreadyMergedTaskCommit` (Fusion-Task-Id trailer / ancestry / patch-id / tree-equality proof). Only when staged files are orphan-only (no declared-scope overlap after excluding `.changeset/*`) and main-branch proof is positive does it finalize as a no-op (`resolutionStrategy: "orphan-discard-no-op"`), append an explicit auto-recovery log line, and tear down the task worktree so orphan staging is discarded.
|
||||
@@ -697,6 +698,11 @@ Guardrails: this routine does **not** retry merges, does **not** apply to mixed/
|
||||
### Observability and reflection
|
||||
- `AgentLogger` (`agent-logger.ts`) — structured per-agent run logging
|
||||
- `RunAudit` (`run-audit.ts`) — mutation audit tracking (DB/git/filesystem)
|
||||
|
||||
#### Key diagnostic points (log subsystem tags)
|
||||
- `[self-healing]` — startup/maintenance recovery pass outcomes.
|
||||
- `[worktree-metadata-reconcile]` — FN-4962 stale `task.worktree`/`task.branch` rebind-or-clear decisions and audit emission failures.
|
||||
- `[scheduler]`, `[executor]`, `[merger]` — core execution/dispatch/merge lanes.
|
||||
- `Notifier` (`notifier.ts`) — legacy ntfy compatibility shim (`NtfyNotifier`) plus shared ntfy helpers
|
||||
- Runtime ownership: `NtfyNotifier` no longer owns an independent task-lifecycle listener graph; `ProjectEngine` injects the canonical `NotificationService` instance so task lifecycle notifications (`task:moved`, `task:updated`, `task:merged`) are emitted through a single path.
|
||||
- Merge dedup safety: `ProjectEngine.start()` is idempotent, so repeated start calls do not wire a second `NotificationService`/`NtfyNotifier` pair. A successful merge therefore emits exactly one canonical `merged` ntfy lifecycle notification per task.
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { EventEmitter } from "node:events";
|
||||
import type { Task, TaskStore } from "@fusion/core";
|
||||
import { SelfHealingManager } from "../../self-healing.js";
|
||||
@@ -71,6 +73,19 @@ describe("reliability interactions: worktree metadata reconcile", () => {
|
||||
expect((store as any).updateTask).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("runs reconcile before reclaim-stale-active-branches in maintenance ordering", () => {
|
||||
const selfHealingPath = fileURLToPath(new URL("../../self-healing.ts", import.meta.url));
|
||||
const source = readFileSync(selfHealingPath, "utf8");
|
||||
const maintenanceSlice = source.slice(
|
||||
source.indexOf("const batch2Fns:"),
|
||||
source.indexOf("for (const fn of batch2Fns)"),
|
||||
);
|
||||
|
||||
expect(maintenanceSlice.indexOf('"reconcile-task-worktree-metadata"')).toBeLessThan(
|
||||
maintenanceSlice.indexOf('"reclaim-stale-active-branches"'),
|
||||
);
|
||||
});
|
||||
|
||||
it("skips done tasks during periodic reconcile (completion fan-out owns done lifecycle)", async () => {
|
||||
const store = makeStore([task("FN-3", { column: "done", worktree: "/missing", branch: null })]);
|
||||
vi.spyOn(worktreePoolModule, "getRegisteredWorktreeBranchMap").mockResolvedValue(new Map([["fusion/fn-3", "/live"]]));
|
||||
|
||||
@@ -2379,7 +2379,7 @@ export class SelfHealingManager {
|
||||
newBranch: normalizedBranch,
|
||||
});
|
||||
worktreeMetadataReconcileLog.log(
|
||||
`[worktree-metadata-reconcile] rebound ${task.id}: ${previousWorktree} -> ${liveWorktree} (${previousBranch ?? "<none>"} -> ${normalizedBranch})`,
|
||||
`rebound ${task.id}: ${previousWorktree} -> ${liveWorktree} (${previousBranch ?? "<none>"} -> ${normalizedBranch})`,
|
||||
);
|
||||
repaired++;
|
||||
continue;
|
||||
@@ -2395,7 +2395,7 @@ export class SelfHealingManager {
|
||||
newBranch: null,
|
||||
});
|
||||
worktreeMetadataReconcileLog.log(
|
||||
`[worktree-metadata-reconcile] cleared ${task.id}: ${previousWorktree} (${previousBranch ?? "<none>"})`,
|
||||
`cleared ${task.id}: ${previousWorktree} (${previousBranch ?? "<none>"})`,
|
||||
);
|
||||
repaired++;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user