diff --git a/.changeset/worktree-base-refresh-never-blocks-execution.md b/.changeset/worktree-base-refresh-never-blocks-execution.md new file mode 100644 index 0000000000..579321722e --- /dev/null +++ b/.changeset/worktree-base-refresh-never-blocks-execution.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Stop failing tasks over a stale worktree base — refreshing the base no longer blocks execution. +category: fix +dev: `refreshReusedWorktreeBase` now returns `executionSafe: true` with `skipped: true` for dirty checkouts, own-commit rebase conflicts, unresolvable bases, and compensated persistence failures; only an unproven tree (failed compensation) still throws `WorktreeBaseRefreshError`. The dirty-tree check moved after the up-to-date check so a worktree already on the current base is never refused. `execute()` now catches the residual throw and routes it into the shared bounded non-parking hold (`holdForWorktreeBaseRefresh`), which the graph-failure lane also uses. New git run-audit type `worktree:base-refresh-skipped` separates a declined refresh from a genuine block. diff --git a/packages/engine/src/__tests__/reliability-interactions/secrets-env-materialization.test.ts b/packages/engine/src/__tests__/reliability-interactions/secrets-env-materialization.test.ts index 6151bb6a9d..e8e1f86047 100644 --- a/packages/engine/src/__tests__/reliability-interactions/secrets-env-materialization.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/secrets-env-materialization.test.ts @@ -78,8 +78,21 @@ describe("reliability interactions: secrets env materialization", () => { expect(existsSync(join(worktree, ".fusion-secrets-env.fingerprint"))).toBe(false); expect(execFileSync("git", ["status", "--porcelain"], { cwd: worktree, encoding: "utf8" })).toBe(""); + /* + FNXC:SecretsEnvMaterialization 2026-08-09-23:49: + Sidecar adoption must not launder REAL dirt into a clean verdict — the refresh still has to SEE it. Advance + main first so a git mutation is genuinely required, which is the only situation where the working tree is + consulted at all. Per FNXC:WorktreeBaseRefresh 2026-08-09-23:49 the dirt now declines the refresh instead of + refusing execution, and the assertion that matters is that the agent's uncommitted work survives untouched. + */ + writeFileSync(join(root, "advance.txt"), "C1\n"); + execFileSync("git", ["add", "advance.txt"], { cwd: root }); + execFileSync("git", ["commit", "-m", "C1"], { cwd: root }); + writeFileSync(join(worktree, "unrelated.txt"), "dirt\n"); - await expect(refreshReusedWorktreeBase({ task: { id: "FN-1", baseCommitSha: base } as any, rootDir: root, worktreePath: worktree, store, settings: {} })).resolves.toMatchObject({ kind: "dirty-worktree", executionSafe: false }); + await expect(refreshReusedWorktreeBase({ task: { id: "FN-1", baseCommitSha: base } as any, rootDir: root, worktreePath: worktree, store, settings: {} })).resolves.toMatchObject({ kind: "dirty-worktree", executionSafe: true, skipped: true }); + expect(execFileSync("git", ["rev-parse", "HEAD"], { cwd: worktree, encoding: "utf8" }).trim()).toBe(base); + expect(readFileSync(join(worktree, "unrelated.txt"), "utf8")).toBe("dirt\n"); }); it("orphan reap reclaims orphaned env artifacts", async () => { diff --git a/packages/engine/src/__tests__/worktree-acquisition.test.ts b/packages/engine/src/__tests__/worktree-acquisition.test.ts index 5255fd3dd5..d0968e934e 100644 --- a/packages/engine/src/__tests__/worktree-acquisition.test.ts +++ b/packages/engine/src/__tests__/worktree-acquisition.test.ts @@ -533,11 +533,21 @@ describe("acquireTaskWorktree", () => { expect(git(worktreePath, "git rev-parse HEAD")).toBe(staleBase); }); + /* + FNXC:WorktreeBaseRefresh 2026-08-09-23:49: + The invariant here is the RESOURCE-HYGIENE ordering — the task binding is cleared before the worktree goes + back to the pool — not the refusal policy that used to trigger it. A dirty checkout no longer fails base + refresh (it declines and executes on the existing base), and in production a pooled worktree never reaches + the refresh dirty anyway: `prepareForTask` runs `git checkout -- .` + `git clean -fd` first, so the old + fixture's dirt only survived because the pool is mocked here. Drive the ordering through the secrets-record + reconciliation refusal instead, which is a secrets-safety gate and remains unconditionally blocking. + */ it("clears a pooled task binding before releasing a worktree that fails base refresh", async () => { const rootDir = makeRepo(); const pooledPath = join(rootDir, ".worktrees", "pooled-fn-4-dirty"); git(rootDir, `git worktree add -b fusion/fn-4-dirty ${JSON.stringify(pooledPath)}`); - writeFileSync(join(pooledPath, "uncommitted.ts"), "dirty\n", "utf-8"); + // A malformed root secrets-env record: reconciliation cannot prove the checkout is safe to hand an agent. + writeFileSync(join(pooledPath, ".fusion-secrets-env.fingerprint"), "not-a-valid-record\n", "utf-8"); const pool = { acquire: vi.fn().mockReturnValue(pooledPath), prepareForTask: vi.fn().mockResolvedValue({ diff --git a/packages/engine/src/__tests__/worktree-base-refresh-never-blocks-execution.test.ts b/packages/engine/src/__tests__/worktree-base-refresh-never-blocks-execution.test.ts new file mode 100644 index 0000000000..eb74aa95e4 --- /dev/null +++ b/packages/engine/src/__tests__/worktree-base-refresh-never-blocks-execution.test.ts @@ -0,0 +1,163 @@ +/* +FNXC:WorktreeBaseRefresh 2026-08-09-23:49 (a stale base is never an execution failure — regression): + +Reported symptom: Fusion paged the operator with "** needs operator action** — The task entered a terminal +failed state" many times a day, rising to 17 on 2026-08-09. Run-audit over 2026-08-01..09 showed 99 of 136 +execution failures were pre-session worktree base-refresh refusals (74 `dirty-worktree`, 25 +`stale-base-conflict`), and 82 of them landed within five minutes of "Task marked done by agent" — they were +code-review-remediation re-entries into `execute()` on the task's OWN warm worktree. The refusal threw +`WorktreeBaseRefreshError` out of `acquireTaskWorktree`, and the `execute()` call site had no catch, so it fell +through every classifier to the generic terminal sink and parked the task `failed`. The bounded non-parking lane +built for exactly this fired 0 times, because it only ever saw refusals published as typed graph node values and +no code node enables `refreshStaleBase`. + +Invariant under test, across every refusal shape observed in production: refreshing the base is an optimization, +so a refresh that cannot proceed DECLINES and lets the session run on the existing base — it never blocks +execution and never destroys the agent's uncommitted work. The merge lane still rebases with conflict +resolution before landing, which is why dispatch-time rebase (which has no conflict resolution) must not be +authoritative. Only an UNPROVEN tree — compensation failed, so a half-rebased checkout may be on disk — may +still refuse, because handing an agent that checkout would corrupt real work. +*/ +import { execFileSync } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { acquireTaskWorktree, WorktreeBaseRefreshError } from "../worktree/worktree-acquisition.js"; +import { refreshReusedWorktreeBase } from "../worktree-base-refresh.js"; + +const paths: string[] = []; +const git = (cwd: string, args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8" }).trim(); + +afterEach(() => paths.splice(0).forEach((path) => rmSync(path, { recursive: true, force: true }))); + +/** + * A task planned at C0 on its own branch while the integration branch advanced to C1 — the shape every + * production refusal had. + */ +function reusedWorktree() { + const root = mkdtempSync(join(tmpdir(), "fn-base-refresh-")); + paths.push(root); + git(root, ["init", "-b", "main"]); + git(root, ["config", "user.email", "test@example.com"]); + git(root, ["config", "user.name", "Test"]); + writeFileSync(join(root, "shared.ts"), "export const shared = 'main-C0';\n"); + git(root, ["add", "shared.ts"]); + git(root, ["commit", "-m", "C0"]); + const c0 = git(root, ["rev-parse", "HEAD"]); + + const worktree = join(root, "linked"); + git(root, ["worktree", "add", "-b", "fusion/fn-1", worktree, c0]); + + // Integration advances under the task. + writeFileSync(join(root, "shared.ts"), "export const shared = 'main-C1';\n"); + git(root, ["add", "shared.ts"]); + git(root, ["commit", "-m", "C1"]); + + return { root, worktree, c0, c1: git(root, ["rev-parse", "HEAD"]) }; +} + +function acquire(root: string, worktree: string, baseCommitSha: string) { + const store = { updateTask: vi.fn().mockResolvedValue(undefined), logEntry: vi.fn().mockResolvedValue(undefined) } as any; + const promise = acquireTaskWorktree({ + task: { id: "FN-1", title: "reuse", description: "", branch: "fusion/fn-1", worktree, baseCommitSha } as any, + rootDir: root, + store, + settings: {} as any, + refreshStaleBase: true, + createWorktree: vi.fn(), + logger: { log: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + }); + return { store, promise }; +} + +describe("worktree base refresh never blocks execution", () => { + it("proceeds with uncommitted agent work intact instead of refusing (the 74 dirty-worktree parks)", async () => { + const { root, worktree, c0 } = reusedWorktree(); + // The remediation re-entry shape: the previous session left edits in the task's own warm checkout. + writeFileSync(join(worktree, "in-flight.ts"), "export const inFlight = true;\n"); + + const { store, promise } = acquire(root, worktree, c0); + await expect(promise).resolves.toMatchObject({ + source: "existing", + baseRefresh: { kind: "dirty-worktree", executionSafe: true, skipped: true }, + }); + + // The work the agent is mid-way through must survive untouched, on its original base. + expect(readFileSync(join(worktree, "in-flight.ts"), "utf8")).toBe("export const inFlight = true;\n"); + expect(git(worktree, ["rev-parse", "HEAD"])).toBe(c0); + // Operators still see WHY the base is stale — as a skip, not an execution failure. + const logged = store.logEntry.mock.calls.map(([, message]: [string, string]) => message).join("\n"); + expect(logged).toContain("Worktree base refresh skipped (dirty-worktree)"); + expect(logged).not.toContain("blocked execution"); + }); + + it("keeps the local base when the task's own commits conflict with the moved base (the 25 stale-base-conflict parks)", async () => { + const { root, worktree, c0, c1 } = reusedWorktree(); + // Task commits C2 touching the same file main advanced → rebase onto C1 must conflict. + writeFileSync(join(worktree, "shared.ts"), "export const shared = 'task-C2';\n"); + git(worktree, ["add", "shared.ts"]); + git(worktree, ["commit", "-m", "C2"]); + const c2 = git(worktree, ["rev-parse", "HEAD"]); + + const { promise } = acquire(root, worktree, c0); + await expect(promise).resolves.toMatchObject({ + source: "existing", + baseRefresh: { kind: "stale-base-conflict", executionSafe: true, skipped: true }, + }); + + // Compensated back to the task's own tip: no rebase in progress, no conflict markers, no lost commit. + expect(git(worktree, ["rev-parse", "HEAD"])).toBe(c2); + expect(git(worktree, ["status", "--porcelain"])).toBe(""); + expect(readFileSync(join(worktree, "shared.ts"), "utf8")).toBe("export const shared = 'task-C2';\n"); + expect(c2).not.toBe(c1); + }); + + it("still advances a clean worktree with no own commits — the optimization is preserved, not disabled", async () => { + const { root, worktree, c0, c1 } = reusedWorktree(); + + const { store, promise } = acquire(root, worktree, c0); + await expect(promise).resolves.toMatchObject({ + baseRefresh: { kind: "reset-to-base", executionSafe: true }, + }); + + expect(git(worktree, ["rev-parse", "HEAD"])).toBe(c1); + expect(store.updateTask).toHaveBeenCalledWith("FN-1", { baseCommitSha: c1 }); + }); + + /* + The residual blocking case. It must remain reachable: an unproven tree is the one state where running a + session really would corrupt work, and the executor routes this throw into the bounded non-parking hold + rather than the terminal sink. + */ + it("still refuses execution when compensation cannot prove the tree was restored", async () => { + const { root, worktree, c0 } = reusedWorktree(); + + // A clean advance reaches the baseline write; failing it AND destroying the checkout in the same step makes + // the restore unprovable, which is the only state that may still refuse a session. + const store = { + updateTask: vi.fn().mockImplementation(async () => { + rmSync(worktree, { recursive: true, force: true }); + throw new Error("database unavailable"); + }), + logEntry: vi.fn().mockResolvedValue(undefined), + } as any; + + const result = await refreshReusedWorktreeBase({ + task: { id: "FN-1", baseCommitSha: c0 } as any, + rootDir: root, + worktreePath: worktree, + store, + settings: {}, + }); + + expect(result).toMatchObject({ kind: "base-reconciliation-required", executionSafe: false }); + expect(result.skipped).toBeFalsy(); + }); + + it("carries the refusal kind on the thrown error so the executor can hold rather than park", async () => { + const error = new WorktreeBaseRefreshError({ kind: "base-reconciliation-required", executionSafe: false }); + expect(error).toBeInstanceOf(Error); + expect(error.refresh.kind).toBe("base-reconciliation-required"); + }); +}); diff --git a/packages/engine/src/__tests__/worktree-base-refresh.test.ts b/packages/engine/src/__tests__/worktree-base-refresh.test.ts index 0e808f3836..09e2bfa552 100644 --- a/packages/engine/src/__tests__/worktree-base-refresh.test.ts +++ b/packages/engine/src/__tests__/worktree-base-refresh.test.ts @@ -77,14 +77,120 @@ describe("refreshReusedWorktreeBase", () => { expect(store.updateTask).toHaveBeenCalledWith("FN-1", { baseCommitSha: c1 }); }); - it("blocks a dirty checkout without changing the durable baseline", async () => { + it("skips a dirty checkout without changing the durable baseline, and stays execution-safe", async () => { const { root, worktree, c0 } = fixture(); writeFileSync(join(worktree, "dirty.txt"), "keep me\n"); const store = { updateTask: vi.fn().mockResolvedValue(undefined) } as any; const result = await refreshReusedWorktreeBase({ task: { id: "FN-1", baseCommitSha: c0 } as any, rootDir: root, worktreePath: worktree, store, settings: {}, }); - expect(result.kind).toBe("dirty-worktree"); + expect(result).toMatchObject({ kind: "dirty-worktree", executionSafe: true, skipped: true }); expect(store.updateTask).not.toHaveBeenCalled(); + expect(git(worktree, "rev-parse HEAD")).toBe(c0); + expect(git(worktree, "status --porcelain")).toContain("dirty.txt"); + }); + + /* + FNXC:WorktreeBaseRefreshTests 2026-08-09-23:49: + The invariant, asserted across every surface that produced an execution refusal in production: a refresh that + cannot proceed leaves the checkout untouched and lets the session run. Between 2026-08-01 and 2026-08-09 the + blocking form parked 99 tasks `failed` — 74 dirty-worktree, 25 own-commit rebase conflicts — for a base that + the merge lane rebases with conflict resolution anyway. Only an UNPROVEN tree may still refuse. + */ + describe("never blocks execution for a recoverable base state", () => { + it("keeps the local base when rebasing own commits onto a moved base conflicts", async () => { + const { root, worktree, c0, c1 } = fixture(); + // Same file edited on both sides of the fork → the rebase onto C1 must conflict. + writeFileSync(join(worktree, "scaffold.ts"), "export const scaffold = 'task-side';\n"); + git(worktree, "add scaffold.ts && git commit -m C2"); + const c2 = git(worktree, "rev-parse HEAD"); + const store = { updateTask: vi.fn().mockResolvedValue(undefined) } as any; + + const result = await refreshReusedWorktreeBase({ + task: { id: "FN-1", baseCommitSha: c0 } as any, rootDir: root, worktreePath: worktree, store, settings: {}, + }); + + expect(result).toMatchObject({ kind: "stale-base-conflict", executionSafe: true, skipped: true }); + // Compensated back to the task's own tip, with no rebase left in progress and no work lost. + expect(git(worktree, "rev-parse HEAD")).toBe(c2); + expect(git(worktree, "status --porcelain")).toBe(""); + expect(store.updateTask).not.toHaveBeenCalled(); + expect(c2).not.toBe(c1); + }); + + it("runs on the existing base when the base commit cannot be resolved", async () => { + const { worktree, c0 } = fixture(); + // A repo with no `main` commit to resolve: `rev-parse main^{commit}` fails, so the probe itself is unusable. + const emptyRoot = mkdtempSync(join(tmpdir(), "fn-8693-empty-")); + paths.push(emptyRoot); + git(emptyRoot, "init -b main"); + const store = { updateTask: vi.fn().mockResolvedValue(undefined) } as any; + + const result = await refreshReusedWorktreeBase({ + task: { id: "FN-1", baseCommitSha: c0 } as any, + rootDir: emptyRoot, + worktreePath: worktree, + store, + settings: {}, + }); + + expect(result).toMatchObject({ kind: "base-unresolvable", executionSafe: true, skipped: true }); + expect(git(worktree, "rev-parse HEAD")).toBe(c0); + expect(store.updateTask).not.toHaveBeenCalled(); + }); + + it("treats a worktrunk-managed checkout as safe rather than refusing execution", async () => { + const { root, worktree, c0 } = fixture(); + const store = { updateTask: vi.fn().mockResolvedValue(undefined) } as any; + const result = await refreshReusedWorktreeBase({ + task: { id: "FN-1", baseCommitSha: c0 } as any, + rootDir: root, + worktreePath: worktree, + store, + settings: { worktrunk: { enabled: true } } as any, + }); + expect(result).toMatchObject({ kind: "worktrunk-refresh-unsupported", executionSafe: true, skipped: true }); + }); + + /* + The ordering defect on its own: HEAD already contains the integration tip, so no git command would run and + uncommitted work is irrelevant — yet the dirty check ran first and refused execution anyway. + */ + it("reports up-to-date for a dirty checkout that already sits on the current base", async () => { + const { root, worktree, c1 } = fixture(); + git(worktree, `reset --hard ${c1}`); + writeFileSync(join(worktree, "wip.ts"), "export const wip = true;\n"); + const store = { updateTask: vi.fn().mockResolvedValue(undefined) } as any; + + const result = await refreshReusedWorktreeBase({ + task: { id: "FN-1", baseCommitSha: c1 } as any, rootDir: root, worktreePath: worktree, store, settings: {}, + }); + + expect(result).toMatchObject({ kind: "up-to-date", executionSafe: true }); + expect(result.skipped).toBeUndefined(); + expect(git(worktree, "status --porcelain")).toContain("wip.ts"); + }); + + it("still proceeds when only the durable baseline write fails on an already-current head", async () => { + const { root, worktree, c0, c1 } = fixture(); + git(worktree, `reset --hard ${c1}`); + const store = { updateTask: vi.fn().mockRejectedValue(new Error("database unavailable")) } as any; + const result = await refreshReusedWorktreeBase({ + task: { id: "FN-1", baseCommitSha: c0 } as any, rootDir: root, worktreePath: worktree, store, settings: {}, + }); + // No git command ran, so HEAD is the verified-current tip — a failed baseline write must not block a session. + expect(result).toMatchObject({ executionSafe: true, skipped: true }); + expect(git(worktree, "rev-parse HEAD")).toBe(c1); + }); + + it("keeps the compensated persistence failure execution-safe", async () => { + const { root, worktree, c0 } = fixture(); + const store = { updateTask: vi.fn().mockRejectedValue(new Error("database unavailable")) } as any; + const result = await refreshReusedWorktreeBase({ + task: { id: "FN-1", baseCommitSha: c0 } as any, rootDir: root, worktreePath: worktree, store, settings: {}, + }); + expect(result).toMatchObject({ kind: "base-persistence-failed-compensated", executionSafe: true, skipped: true }); + expect(git(worktree, "rev-parse HEAD")).toBe(c0); + }); }); }); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 48045d3915..f1a0efd965 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -250,7 +250,7 @@ import { type RunTaskStepResult, } from "./execution/step-runner.js"; // FNXC:MergerUnification 2026-06-21-19:05: the foundation branch imported `acquireWorkspaceRepoWorktree` here but never used it in executor.ts (the agent tool wraps it via agent-tools.ts), which fails lint on the inherited base. Removed until master-plan U1 re-adds it together with its per-repo acquisition usage. -import { acquireTaskWorktree, type AcquireTaskWorktreeResult } from "./worktree/worktree-acquisition.js"; +import { acquireTaskWorktree, type AcquireTaskWorktreeResult, WorktreeBaseRefreshError } from "./worktree/worktree-acquisition.js"; import { resolveCapturedBaseCommitSha } from "./execution/base-commit-capture.js"; import { installTaskWorktreeIdentityGuard } from "./worktree/worktree-hooks.js"; import { @@ -11400,6 +11400,52 @@ export class TaskExecutor { ]).has(this.graphFailureValue(result) ?? ""); } + /* + FNXC:WorktreeBaseRefresh 2026-08-09-23:49: + The single bounded, NON-PARKING lane for every base-refresh refusal, whichever way it arrives — as a typed + graph failure value or as a `WorktreeBaseRefreshError` thrown by acquisition inside `execute()`. Previously + only the graph path had a lane and the thrown path fell to the terminal sink, so the recovery that existed on + paper never ran. A refusal is a pre-session checkout state that a later acquisition can clear once git state + changes; it must never consume a provider retry budget, mislabel itself as a plan defect, or park the task. + Exhaustion deliberately leaves the task held and cleanly dispatchable rather than failed. + */ + private async holdForWorktreeBaseRefresh(task: Task, refusal: WorktreeBaseRefreshError | string): Promise { + const refreshKind = typeof refusal === "string" ? refusal : refusal.refresh.kind; + const live = await this.store.getTask(task.id).catch(() => null); + const priorRetries = live?.graphResumeRetryCount ?? 0; + if (priorRetries >= MAX_TRANSIENT_GRAPH_RESUME_RETRIES) { + await this.store.logEntry( + task.id, + `Worktree base refresh remains blocked (${refreshKind}) — retry budget exhausted; task remains held`, + undefined, + this.getRunContextFor(task.id), + ); + return; + } + const nextRetries = priorRetries + 1; + await this.store.logEntry( + task.id, + `Worktree base refresh blocked execution (${refreshKind}) — retrying in place (${nextRetries}/${MAX_TRANSIENT_GRAPH_RESUME_RETRIES})`, + undefined, + this.getRunContextFor(task.id), + ); + await this.store.updateTask(task.id, { graphResumeRetryCount: nextRetries }, this.getRunContextFor(task.id)); + /* + A refusal is not a failure state: clear any stale park so the card never shows `failed` while it is simply + waiting for a clean checkout — that badge is what paged the operator 99 times. + */ + if (live && (live.status != null || live.error != null)) { + await this.store.updateTask(task.id, { status: null, error: null }, this.getRunContextFor(task.id)); + } + const resume = live ?? task; + const handle = setTimeout(() => { + this.execute(resume).catch((err) => + executorLog.error(`Failed worktree base refresh retry for ${task.id}:`, err), + ); + }, TRANSIENT_GRAPH_RESUME_RETRY_BACKOFF_MS); + handle.unref?.(); + } + /* FNXC:SessionContention 2026-07-25-21:30 (self-recovering wait — the task is never parked): Retry the graph in place on an exponential backoff while the holder finishes. The counter is @@ -12614,28 +12660,9 @@ export class TaskExecutor { in the task log. Exhaustion deliberately leaves the task held for a later clean acquisition. */ if (this.isWorktreeBaseRefreshGraphFailure(result)) { - const refreshKind = this.graphFailureValue(result)!; - const priorRetries = live.graphResumeRetryCount ?? 0; - if (priorRetries < MAX_TRANSIENT_GRAPH_RESUME_RETRIES) { - const nextRetries = priorRetries + 1; - const message = `Worktree base refresh blocked execution (${refreshKind}) — retrying in place (${nextRetries}/${MAX_TRANSIENT_GRAPH_RESUME_RETRIES})`; - await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id)); - await this.store.updateTask(task.id, { graphResumeRetryCount: nextRetries }, this.getRunContextFor(task.id)); - const scheduleRetry = () => { - this.execute(live).catch((err) => - executorLog.error(`Failed worktree base refresh retry for ${task.id}:`, err), - ); - }; - const handle = setTimeout(scheduleRetry, TRANSIENT_GRAPH_RESUME_RETRY_BACKOFF_MS); - handle.unref?.(); - } else { - await this.store.logEntry( - task.id, - `Worktree base refresh remains blocked (${refreshKind}) — retry budget exhausted; task remains held`, - undefined, - this.getRunContextFor(task.id), - ); - } + // FNXC:WorktreeBaseRefresh 2026-08-09-23:49: one shared lane with the thrown-error path, so the two + // entry points cannot drift in retry budget, park-clearing, or log wording. + await this.holdForWorktreeBaseRefresh(live, this.graphFailureValue(result)!); await this.persistTokenUsage(task.id); return; } @@ -16609,6 +16636,20 @@ export class TaskExecutor { // Dependency added mid-execution — discard worktree and move to triage this.depAborted.delete(task.id); await this.handleDepAbortCleanup(task.id, worktreePath); + } else if (err instanceof WorktreeBaseRefreshError) { + /* + FNXC:WorktreeBaseRefresh 2026-08-09-23:49: + Classified FIRST among error types, because acquisition throws this BEFORE any session starts and the + generic sink below would park the task `failed` and page the operator for a pre-session checkout state. + The graph lane at `isWorktreeBaseRefreshGraphFailure` only sees refusals published as typed node values, + and no code node enables `refreshStaleBase` — so between 2026-08-01 and 2026-08-09 it fired 0 times while + 99 refusals reached the terminal sink. Route the throw into the same bounded, non-parking retry instead. + Post-fix this is reachable only for an UNPROVEN tree (compensation failed), which a later acquisition can + still repair once git state changes, so it must stay a retry rather than a terminal failure. + */ + await this.holdForWorktreeBaseRefresh(task, err); + await this.persistTokenUsage(task.id); + return; } else if (isInvalidAssistantContinuationErrorMessage(errorMessage)) { /* FNXC:PostDoneContinuation 2026-07-16-11:57: diff --git a/packages/engine/src/util/run-audit.ts b/packages/engine/src/util/run-audit.ts index 005fe53860..cb4b74bc36 100644 --- a/packages/engine/src/util/run-audit.ts +++ b/packages/engine/src/util/run-audit.ts @@ -102,7 +102,14 @@ export type GitMutationType = * distinguish a mechanical C1 advance, a typed block/conflict, compensated persistence failure, * and stateless C0-to-C1 reconciliation without recording command output or branch prose. */ + /* + * FNXC:WorktreeBaseRefresh 2026-08-09-23:49: + * `-skipped` is the ordinary declined-refresh outcome (dirty tree, own-commit conflict, unresolvable base): + * the checkout kept its existing base and execution continued. `-blocked` is now reserved for the rare + * unproven tree that genuinely refused execution, so the two are no longer conflated in operator triage. + */ | "worktree:base-refreshed" + | "worktree:base-refresh-skipped" | "worktree:base-refresh-blocked" | "worktree:base-refresh-conflict" | "worktree:base-refresh-persistence-failed-compensated" diff --git a/packages/engine/src/worktree-base-refresh.ts b/packages/engine/src/worktree-base-refresh.ts index d551da2f84..97ba98ce39 100644 --- a/packages/engine/src/worktree-base-refresh.ts +++ b/packages/engine/src/worktree-base-refresh.ts @@ -20,8 +20,15 @@ export type WorktreeBaseRefreshKind = | "base-reconciliation-required"; export interface WorktreeBaseRefreshResult { + /** + * True when the checkout is at a known-good commit and a session may run there. + * FNXC:WorktreeBaseRefresh 2026-08-09-23:49: this is a TREE-INTEGRITY verdict, not a base-freshness one. + * A skipped refresh is execution-safe; only an unproven tree (failed compensation) is not. + */ kind: WorktreeBaseRefreshKind; executionSafe: boolean; + /** True when the refresh was declined and the worktree kept its existing base untouched. */ + skipped?: boolean; integrationBranch?: string; baseSha?: string; originalHead?: string; @@ -78,16 +85,32 @@ async function compensate(cwd: string, originalHead: string): Promise { FNXC:WorktreeBaseRefresh 2026-08-01-16:04: Planning deliberately creates an isolated task worktree, but execution may begin after dependencies land. Only refresh-enabled execution reuse reaches this primitive: planning, review, gates, and merge retain their -existing worktrees. The primitive refuses dirty, unresolved, worktrunk, conflict, git, persistence, and -reconciliation-unknown states with typed non-execution outcomes. It resolves integration C1 independently -on every reuse and aligns durable baseCommitSha to C1; a rebased own-commit HEAD C2 is preserved and is never -stored as the baseline. Git mutations are compensated back to C0 when baseline persistence fails; a later -process can statelessly reconcile C0 versus clean C1/C2 from durable metadata and git proof alone. +existing worktrees. It resolves integration C1 independently on every reuse and aligns durable baseCommitSha +to C1; a rebased own-commit HEAD C2 is preserved and is never stored as the baseline. Git mutations are +compensated back to C0 when baseline persistence fails; a later process can statelessly reconcile C0 versus +clean C1/C2 from durable metadata and git proof alone. + +FNXC:WorktreeBaseRefresh 2026-08-09-23:49: +Refreshing the base is an OPTIMIZATION (start on landed dependency commits), never a correctness gate — so it +must never block execution. The original design refused dirty/conflicting/unresolvable states with +`executionSafe: false`, and every such refusal reached the executor's terminal sink: 99 tasks were parked +`failed` and paged the operator between 2026-08-01 and 2026-08-09 (74 dirty-worktree, 25 stale-base-conflict), +while the bounded non-parking lane built for them fired 0 times. 82 of the 99 landed within five minutes of +"Task marked done by agent" — they were code-review-remediation re-entries into `execute()` on the task's own +warm worktree, i.e. exactly the checkout the refresh must leave alone. + +`executionSafe` now means "the tree is at a known-good commit with the session's work intact", not "the base is +current". Every outcome that leaves the tree provably at C0/C1/C2 is safe and execution proceeds on whatever +base it has; only an UNPROVEN tree (compensation failed, so a half-rebased checkout may be on disk) still +blocks, because running an agent there would corrupt real work. This matches the policy already used when a +FRESH worktree cannot rebase onto origin/main: keep the local base and let the merge-time rebase retry with +conflict resolution — the merge lane owns conflict arbitration and deliberately leaves `refreshStaleBase` off +(see merger.ts). Dispatch-time rebase had no conflict resolution at all, so it could only ever fail. */ export async function refreshReusedWorktreeBase(input: RefreshReusedWorktreeBaseInput): Promise { const { task, rootDir, worktreePath, store, settings, audit, logger } = input; if (settings.worktrunk?.enabled) { - return { kind: "worktrunk-refresh-unsupported", executionSafe: false, durableBaseSha: task.baseCommitSha ?? null }; + return { kind: "worktrunk-refresh-unsupported", executionSafe: true, skipped: true, durableBaseSha: task.baseCommitSha ?? null }; } let integrationBranch: string; @@ -102,11 +125,21 @@ export async function refreshReusedWorktreeBase(input: RefreshReusedWorktreeBase git(worktreePath, "status --porcelain"), ]); } catch (error) { - return { kind: "base-unresolvable", executionSafe: false, durableBaseSha: task.baseCommitSha ?? null, detail: error instanceof Error ? error.message : String(error) }; + /* + FNXC:WorktreeBaseRefresh 2026-08-09-23:49: + Nothing was mutated, so the checkout is untouched and safe to run on its existing base. A genuinely broken + worktree still fails at session start and routes to the unusable-worktree recovery, which owns that repair. + */ + return { kind: "base-unresolvable", executionSafe: true, skipped: true, durableBaseSha: task.baseCommitSha ?? null, detail: error instanceof Error ? error.message : String(error) }; } const common = { integrationBranch, baseSha, originalHead, durableBaseSha: task.baseCommitSha ?? null }; - if (dirty) return { kind: "dirty-worktree", executionSafe: false, observedHead: originalHead, ...common }; + /* + FNXC:WorktreeBaseRefresh 2026-08-09-23:49: + Decide whether a git mutation is even needed BEFORE consulting the working tree. The original order refused + every dirty checkout up front, so a worktree already sitting on the current base — where the refresh is a + no-op and uncommitted work is irrelevant — was still blocked from executing. + */ const baselineMatches = task.baseCommitSha === baseSha; const headCurrent = await isAncestor(worktreePath, baseSha, originalHead); if (headCurrent) { @@ -116,15 +149,27 @@ export async function refreshReusedWorktreeBase(input: RefreshReusedWorktreeBase await audit?.git?.({ type: "worktree:base-refresh-reconciled", target: worktreePath, metadata: { taskId: task.id, outcome: "reconciled" } }); return { kind: "up-to-date", executionSafe: true, observedHead: originalHead, ...common }; } catch (error) { - return { kind: "base-reconciliation-required", executionSafe: false, observedHead: originalHead, ...common, detail: error instanceof Error ? error.message : String(error) }; + /* + FNXC:WorktreeBaseRefresh 2026-08-09-23:49: + Only the durable baseline write failed; no git command ran, so HEAD is still the verified-current C1. + Execution proceeds and a later reuse re-reconciles the baseline statelessly from git proof. + */ + return { kind: "base-reconciliation-required", executionSafe: true, skipped: true, observedHead: originalHead, ...common, detail: error instanceof Error ? error.message : String(error) }; } } + /* + FNXC:WorktreeBaseRefresh 2026-08-09-23:49: + A mutation is required from here on, so uncommitted work now matters: `reset --hard`/`rebase` would destroy + the session's in-flight edits. Decline and keep the existing base — never move ground under live work. + */ + if (dirty) return { kind: "dirty-worktree", executionSafe: true, skipped: true, observedHead: originalHead, ...common }; + let mergeBase: string; try { mergeBase = await git(worktreePath, `merge-base HEAD ${quote(baseSha)}`); } catch (error) { - return { kind: "git-refresh-failed", executionSafe: false, observedHead: originalHead, ...common, detail: error instanceof Error ? error.message : String(error) }; + return { kind: "git-refresh-failed", executionSafe: true, skipped: true, observedHead: originalHead, ...common, detail: error instanceof Error ? error.message : String(error) }; } let kind: "reset-to-base" | "rebased"; try { @@ -137,12 +182,34 @@ export async function refreshReusedWorktreeBase(input: RefreshReusedWorktreeBase await git(worktreePath, `rebase ${quote(baseSha)}`); kind = "rebased"; } catch (error) { + /* + FNXC:WorktreeBaseRefresh 2026-08-09-23:49: + A conflict rebasing the task's OWN commits onto a moved integration tip is expected on a busy main, and + dispatch has no conflict resolution to offer. Once compensation proves the tree is back at C0 the work is + intact, so keep the local base and defer to the merge-time rebase, which does have AI conflict arbitration. + Only a FAILED compensation still blocks: a half-rebased checkout is unproven and unsafe to hand an agent. + */ const restored = await compensate(worktreePath, originalHead); - return { kind: restored ? "stale-base-conflict" : "base-reconciliation-required", executionSafe: false, observedHead: originalHead, ...common, detail: error instanceof Error ? error.message : String(error) }; + return { + kind: restored ? "stale-base-conflict" : "base-reconciliation-required", + executionSafe: restored, + skipped: restored, + observedHead: originalHead, + ...common, + detail: error instanceof Error ? error.message : String(error), + }; } } } catch (error) { - return { kind: "git-refresh-failed", executionSafe: false, observedHead: originalHead, ...common, detail: error instanceof Error ? error.message : String(error) }; + const restored = await compensate(worktreePath, originalHead); + return { + kind: restored ? "git-refresh-failed" : "base-reconciliation-required", + executionSafe: restored, + skipped: restored, + observedHead: originalHead, + ...common, + detail: error instanceof Error ? error.message : String(error), + }; } const observedHead = await git(worktreePath, "rev-parse HEAD").catch(() => undefined); @@ -153,6 +220,7 @@ export async function refreshReusedWorktreeBase(input: RefreshReusedWorktreeBase } catch (error) { const restored = await compensate(worktreePath, originalHead); await audit?.git?.({ type: restored ? "worktree:base-refresh-persistence-failed-compensated" : "worktree:base-refresh-blocked", target: worktreePath, metadata: { taskId: task.id, outcome: restored ? "compensated" : "reconciliation-required" } }).catch(() => undefined); - return { kind: restored ? "base-persistence-failed-compensated" : "base-reconciliation-required", executionSafe: false, observedHead, ...common, detail: error instanceof Error ? error.message : String(error) }; + // FNXC:WorktreeBaseRefresh 2026-08-09-23:49: a proven restore to C0 leaves the work intact — run on the old base. + return { kind: restored ? "base-persistence-failed-compensated" : "base-reconciliation-required", executionSafe: restored, skipped: restored, observedHead, ...common, detail: error instanceof Error ? error.message : String(error) }; } } diff --git a/packages/engine/src/worktree/worktree-acquisition.ts b/packages/engine/src/worktree/worktree-acquisition.ts index b0a30c9287..90ae17c427 100644 --- a/packages/engine/src/worktree/worktree-acquisition.ts +++ b/packages/engine/src/worktree/worktree-acquisition.ts @@ -269,8 +269,22 @@ export async function acquireTaskWorktree(opts: AcquireTaskWorktreeOptions): Pro ? { ...settings, worktrunk: { ...settings.worktrunk, enabled: false } } : settings; const refresh = await refreshReusedWorktreeBase({ task, rootDir, worktreePath: path, store, settings: refreshSettings, audit, logger }); + /* + FNXC:WorktreeBaseRefresh 2026-08-09-23:49: + A declined refresh is an unremarkable outcome, not an execution failure: the checkout is intact and the + merge lane still rebases with conflict resolution before landing. Record it as a skip so the base staleness + stays observable without parking the task or paging the operator. + + This matters more since the refresh was extended to freshly reacquired and pooled worktrees: that widened + where a fail-closed refusal could fire, and every one of them reached the executor's terminal sink. + */ + if (refresh.skipped) { + await audit?.git({ type: refresh.kind === "stale-base-conflict" ? "worktree:base-refresh-conflict" : "worktree:base-refresh-skipped", target: path, metadata: { taskId: task.id, outcome: refresh.kind } }); + await store.logEntry(task.id, `Worktree base refresh skipped (${refresh.kind}) — kept local base; the merge-time rebase will retry with conflict resolution`, refresh.detail, runContext); + return refresh; + } if (!refresh.executionSafe) { - await audit?.git({ type: refresh.kind === "stale-base-conflict" ? "worktree:base-refresh-conflict" : "worktree:base-refresh-blocked", target: path, metadata: { taskId: task.id, outcome: refresh.kind } }); + await audit?.git({ type: "worktree:base-refresh-blocked", target: path, metadata: { taskId: task.id, outcome: refresh.kind } }); await store.logEntry(task.id, `Worktree base refresh blocked execution (${refresh.kind})`, refresh.detail, runContext); throw new WorktreeBaseRefreshError(refresh); }