diff --git a/.changeset/fn-9060-workspace-zero-acquire.md b/.changeset/fn-9060-workspace-zero-acquire.md new file mode 100644 index 0000000000..94f03735b6 --- /dev/null +++ b/.changeset/fn-9060-workspace-zero-acquire.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Workspace tasks with no acquired sub-repo now complete or fail review consistently. +category: fix +dev: Uses classifyWorkspaceZeroAcquire and the retryable review seam flag to avoid deterministic retry exhaustion. diff --git a/docs/workflow-steps.md b/docs/workflow-steps.md index 68169b1c05..c290273d40 100644 --- a/docs/workflow-steps.md +++ b/docs/workflow-steps.md @@ -372,7 +372,7 @@ Parallelism is opt-in *per step by the planner*, not asserted by the workflow au #### `step-review` node & rework edges -`step-review` (`{ type: "plan" | "code", model? }`, legal only inside a foreach template) runs the reviewer against the current instance's step and maps the verdict to outcome edges: `outcome:approve` (marks the step done), `outcome:revise` (typically a rework edge — revise in place, no reset), `outcome:rethink` (a rework edge whose traversal first triggers reset-to-baseline: git reset + session rewind + step→pending), `outcome:unavailable` (bounded retry then route). The validator requires `approve` and `revise` routed; `rethink` defaults to the revise target with reset semantics. Verdict authority is single-writer — review nodes inside `split` branches are advisory-only. +`step-review` (`{ type: "plan" | "code", model? }`, legal only inside a foreach template) runs the reviewer against the current instance's step and maps the verdict to outcome edges: `outcome:approve` (marks the step done), `outcome:revise` (typically a rework edge — revise in place, no reset), `outcome:rethink` (a rework edge whose traversal first triggers reset-to-baseline: git reset + session rewind + step→pending), `outcome:unavailable` (transient unavailability retries within its bounded budget before routing; deterministic unavailability routes immediately). The validator requires `approve` and `revise` routed; `rethink` defaults to the revise target with reset semantics. Verdict authority is single-writer — review nodes inside `split` branches are advisory-only. `rework` edges (`edge.kind: "rework"`) are the **only legal cycles**: a loop-back within one foreach instance, bounded by `maxReworkCycles`. Exhaustion emits `outcome:rework-exhausted` (validator requires it routed — escalation, hold, or failure; defaults to failure). Non-rework cycles still throw. diff --git a/packages/engine/src/__tests__/executor-workspace-taskdone.test.ts b/packages/engine/src/__tests__/executor-workspace-taskdone.test.ts index 186e58022b..fb48e1c39c 100644 --- a/packages/engine/src/__tests__/executor-workspace-taskdone.test.ts +++ b/packages/engine/src/__tests__/executor-workspace-taskdone.test.ts @@ -237,6 +237,20 @@ describeIfGit("U2 KTD4 — per-repo scope-leak guard in fn_task_done", () => { }); }); +describe("FN-9060 — zero-acquire workspace completion invariant", () => { + it("refuses unproven zero-acquire work but accepts explicit commit-free completion", async () => { + const store = createStore(["src/**"]); + const executor = new TaskExecutor(store, "/tmp/workspace-root"); + (executor as any).workspaceConfig = { repos: ["repo-a"] } as WorkspaceConfig; + const unproven = await (executor as any).verifyWorktreeInvariants(makeTask({ workspaceWorktrees: {} })); + expect(unproven).toMatchObject({ ok: false, reason: "no_commits", observed: "0 acquired sub-repo worktrees" }); + expect((unproven as any).expected).toContain("fn_acquire_repo_worktree"); + + const eligible = await (executor as any).verifyWorktreeInvariants(makeTask({ workspaceWorktrees: {}, noCommitsExpected: true })); + expect(eligible).toEqual({ ok: true }); + }); +}); + describeIfGit("U2 KTD4 — per-repo worktree-invariant verify in fn_task_done", () => { let fx: WorkspaceFixture; afterEach(() => fx?.cleanup()); diff --git a/packages/engine/src/__tests__/executor-workspace.test.ts b/packages/engine/src/__tests__/executor-workspace.test.ts index 2046ddc2c1..0d3a01f71d 100644 --- a/packages/engine/src/__tests__/executor-workspace.test.ts +++ b/packages/engine/src/__tests__/executor-workspace.test.ts @@ -19,6 +19,7 @@ function createStore(overrides: Partial> = {}): TaskStor updateTask: vi.fn().mockResolvedValue(undefined), logEntry: vi.fn().mockResolvedValue(undefined), getSettings: vi.fn().mockResolvedValue({ autoMerge: false }), + getTask: vi.fn().mockResolvedValue(undefined), on: emitter.on.bind(emitter), ...overrides, }) as unknown as TaskStore & EventEmitter; @@ -256,17 +257,16 @@ describeIfGit("U1 KTD1 — verifyWorktreeInvariants gated off in workspace mode" let fx: WorkspaceFixture; afterEach(() => fx?.cleanup()); - it("returns ok for a zero-acquire workspace task (no task.worktree) so fn_task_done does not requeue", async () => { + it("refuses an unproven zero-acquire workspace task so fn_task_done can requeue for acquisition", async () => { fx = await createWorkspaceFixture(); const store = createStore(); const executor = new TaskExecutor(store, fx.rootDir); (executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig; - // A workspace task that acquired ZERO sub-repos has no task.worktree and no - // tracked paths. The singular invariant would otherwise refuse on - // "missing task.worktree"; in workspace mode it is gated OFF. + // A workspace task that acquired ZERO sub-repos has no task.worktree. It must + // acquire one before claiming completion unless it proves commit-free intent. const result = await (executor as any).verifyWorktreeInvariants(makeTask("FN-WS-1", { worktree: undefined })); - expect(result).toEqual({ ok: true }); + expect(result).toMatchObject({ ok: false, reason: "no_commits" }); }); it("non-workspace task with no worktree still fails the invariant (regression: gate is workspace-only)", async () => { diff --git a/packages/engine/src/__tests__/reviewer-workspace.test.ts b/packages/engine/src/__tests__/reviewer-workspace.test.ts index d04a68cc12..1375664d4b 100644 --- a/packages/engine/src/__tests__/reviewer-workspace.test.ts +++ b/packages/engine/src/__tests__/reviewer-workspace.test.ts @@ -176,12 +176,25 @@ describe("U2 KTD3 — reviewWorkspacePerRepo conjunction + tagging (the shared l expect(result.summary).toMatch(/^repo-a:/); }); - it("zero-acquire workspace task → UNAVAILABLE (caller routes; no fabricated APPROVE)", async () => { + it("unproven zero-acquire workspace task → non-retryable UNAVAILABLE without invoking a reviewer", async () => { const task = makeTask({ workspaceWorktrees: {} }); const executor = workspaceExecutor(makeStore(task)); const invoke = vi.fn(); const result = await (executor as any).reviewWorkspacePerRepo(task, invoke); expect(result.verdict).toBe("UNAVAILABLE"); + expect(result.retryable).toBe(false); + expect(result.review).toContain("re-invocation cannot change"); + expect(invoke).not.toHaveBeenCalled(); + }); + + it("commit-free zero-acquire workspace task approves honestly without invoking a reviewer", async () => { + const task = makeTask({ workspaceWorktrees: {}, noCommitsExpected: true }); + const executor = workspaceExecutor(makeStore(task)); + const invoke = vi.fn(); + const result = await (executor as any).reviewWorkspacePerRepo(task, invoke); + expect(result.verdict).toBe("APPROVE"); + expect(result.review).toContain("no diff was reviewed"); + expect(result.review).toContain("explicit noCommitsExpected=true"); expect(invoke).not.toHaveBeenCalled(); }); }); @@ -206,6 +219,28 @@ describe("U2 KTD3 — step-inversion review seam (executor.ts:5668) loops per su expect(result.verdict).toBe("APPROVE"); }); + it("preserves fn_task_done's persisted no-op eligibility through the production step-review seam", async () => { + // fn_task_done persists this flag before it schedules the graph handoff; the + // later review must not reclassify the same zero-acquire task as unproven. + const task = makeTask({ + workspaceWorktrees: {}, + noCommitsExpected: true, + summary: "PREMISE STALE: implementation already exists on HEAD", + }); + const store = makeStore(task); + const executor = workspaceExecutor(store); + const seams = executor.createAuthoritativeWorkflowSeams({ autoMerge: false } as any); + const context = { + [FOREACH_ACTIVE_CONTEXT_KEY]: { stepIndex: 1, worktreePath: ROOT, baselineSha: "base" }, + } as any; + + const result = await seams.stepReview!(task as any, context, { type: "code", advisory: false } as any); + + expect(result.verdict).toBe("APPROVE"); + expect(result.review).toContain("no diff was reviewed"); + expect(mockedReviewStep).not.toHaveBeenCalled(); + }); + it("passes unified user comments and legacy steering into workflow graph stepReview", async () => { const task = makeTask({ worktree: WT_A, diff --git a/packages/engine/src/__tests__/workflow-step-review.test.ts b/packages/engine/src/__tests__/workflow-step-review.test.ts index 7625a383b2..e8d3eba99e 100644 --- a/packages/engine/src/__tests__/workflow-step-review.test.ts +++ b/packages/engine/src/__tests__/workflow-step-review.test.ts @@ -221,6 +221,20 @@ describe("WorkflowGraphExecutor step-review (U5)", () => { expect(result.outcome).toBe("success"); }); + it("deterministic UNAVAILABLE routes unavailable after one attempt without changing the verdict", async () => { + const stepReview = vi.fn(async (): Promise => ({ verdict: "UNAVAILABLE", retryable: false })); + const seams = baseSeams({ + stepExecute: async () => ({ outcome: "success", value: "step-done" }), + stepReview, + }); + const executor = new WorkflowGraphExecutor({ seams }); + const result = await executor.run(taskWithSteps(1), settingsOn(), reviewForeachIr()); + + expect(stepReview).toHaveBeenCalledTimes(1); + // No unavailable edge means template exit, but no authoritative APPROVE marked the step done. + expect(result.outcome).toBe("success"); + }); + it("persists the verdict into the instance row", async () => { const saved: WorkflowStepInstanceState[] = []; const seams = baseSeams({ diff --git a/packages/engine/src/__tests__/workspace-zero-acquire.test.ts b/packages/engine/src/__tests__/workspace-zero-acquire.test.ts new file mode 100644 index 0000000000..b2582cecbc --- /dev/null +++ b/packages/engine/src/__tests__/workspace-zero-acquire.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from "vitest"; +import type { Task } from "@fusion/core"; +import { classifyWorkspaceZeroAcquire } from "../executor/workspace-zero-acquire.js"; + +function task(overrides: Partial = {}): Task { + return { + id: "FN-ZERO", + title: "coordination", + description: "", + column: "in-progress", + dependencies: [], + steps: [{ name: "complete", status: "done" }], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + ...overrides, + } as Task; +} + +const coordinationPrompt = `# Task: coordination +## Review Level: 1 Plan Only +## Mission +Coordinate routing. Do not change product source. +## File Scope +- task documents`; + +const promptDerived = `# Task: audit +## Review Level: 1 Plan Only +## Mission +Audit evidence; no source changes expected. +## File Scope +- task documents`; + +describe("classifyWorkspaceZeroAcquire", () => { + it("is not applicable outside workspace mode or after one or more repos were acquired", () => { + expect(classifyWorkspaceZeroAcquire(task(), { workspaceMode: false })).toEqual({ kind: "not-applicable" }); + expect(classifyWorkspaceZeroAcquire(task({ workspaceWorktrees: { a: { worktreePath: "/a", branch: "fusion/fn-zero" } } as any }), { workspaceMode: true })).toEqual({ kind: "not-applicable" }); + expect(classifyWorkspaceZeroAcquire(task({ workspaceWorktrees: { + a: { worktreePath: "/a", branch: "fusion/fn-zero" }, b: { worktreePath: "/b", branch: "fusion/fn-zero" }, + } as any }), { workspaceMode: true })).toEqual({ kind: "not-applicable" }); + }); + + it.each([undefined, {}])("treats %j workspace worktrees as the same unproven zero-acquire state", (workspaceWorktrees) => { + expect(classifyWorkspaceZeroAcquire(task({ workspaceWorktrees } as Partial), { workspaceMode: true })) + .toEqual({ kind: "unproven" }); + }); + + it("accepts every established commit-free eligibility source", () => { + expect(classifyWorkspaceZeroAcquire(task({ noCommitsExpected: true }), { workspaceMode: true })) + .toEqual({ kind: "commit-free-eligible", reason: "explicit noCommitsExpected=true" }); + expect(classifyWorkspaceZeroAcquire(task({ prompt: coordinationPrompt }), { workspaceMode: true })) + .toEqual({ kind: "commit-free-eligible", reason: "prompt-derived coordination-only no-source scope" }); + expect(classifyWorkspaceZeroAcquire(task({ title: "audit", prompt: promptDerived, description: "Operational routing evidence" }), { workspaceMode: true })) + .toEqual({ kind: "commit-free-eligible", reason: "prompt/source metadata derived operational no-commit contract" }); + expect(classifyWorkspaceZeroAcquire(task(), { workspaceMode: true, noOpCompletion: true, noOpCompletionReason: "verified PREMISE STALE completion sentinel" })) + .toEqual({ kind: "commit-free-eligible", reason: "verified PREMISE STALE completion sentinel" }); + }); +}); diff --git a/packages/engine/src/execution/reviewer.ts b/packages/engine/src/execution/reviewer.ts index 9246ed00f6..0adce3ac64 100644 --- a/packages/engine/src/execution/reviewer.ts +++ b/packages/engine/src/execution/reviewer.ts @@ -81,6 +81,13 @@ export interface ReviewResult { verdict: ReviewVerdict; review: string; summary: string; + /** + * FNXC:WorkflowReviewGates 2026-08-15-04:21: + * Only deterministic verdicts whose inputs cannot change on another invocation + * may disable the step-review retry loop. Provider failures still throw + * ReviewerProviderError rather than being converted into UNAVAILABLE. + */ + retryable?: boolean; } export interface ReviewOptions { diff --git a/packages/engine/src/executor/create-authoritative-workflow-seams.ts b/packages/engine/src/executor/create-authoritative-workflow-seams.ts index cecfcd6cdd..1de3cef423 100644 --- a/packages/engine/src/executor/create-authoritative-workflow-seams.ts +++ b/packages/engine/src/executor/create-authoritative-workflow-seams.ts @@ -28,7 +28,7 @@ import { resolveValidatorThinkingLevel, resolveValidatorFallbackThinkingLevel, } from "../agents/agent-session-helpers.js"; -import type { ReviewVerdict } from "../execution/reviewer.js"; +import type { ReviewResult } from "../execution/reviewer.js"; import { buildReviewUnavailableMessage, buildPlanVerifiedMessage, @@ -420,19 +420,35 @@ export function createAuthoritativeWorkflowSeams( onSessionEnded: (s) => deps.unregisterSubagentSession(seamTask.id, s), }, }); - const runForCwd = (cwd: string): Promise<{ verdict: ReviewVerdict; review: string; summary: string }> => { + const runForCwd = (cwd: string): Promise => { const invoke = () => invokeReviewerForCwd(cwd); return sem ? sem.runNested(invoke) : invoke(); }; const workspaceConfig = deps.ensureWorkspaceConfig ? await deps.ensureWorkspaceConfig() : deps.workspaceConfig; + /* + FNXC:Workspace 2026-08-15-04:31: + Only step-review nodes use this per-repo aggregation seam. Prompt-node + code-review and plan-review groups remain intentionally out of scope; + extending their workspace awareness requires separate review authority work. + */ const invokeReviewer = () => workspaceConfig && reviewCwd === worktreePath - ? deps.reviewWorkspacePerRepo(detail, (cwd: string) => runForCwd(cwd)) + ? deps.reviewWorkspacePerRepo(detail, (cwd: string) => runForCwd(cwd), { + /* + FNXC:Workspace 2026-08-15-04:49: + fn_task_done persists an accepted no-op sentinel as noCommitsExpected + before scheduling this review handoff. Carry that durable provenance + into the shared classifier so workspace review agrees with completion + even when the task acquired no sub-repo worktree. + */ + noOpCompletion: detail.noCommitsExpected === true, + noOpCompletionReason: "verified no-op completion persisted by fn_task_done", + }) : runForCwd(reviewCwd); - let review: { verdict: ReviewVerdict; review: string; summary: string }; + let review: ReviewResult; try { review = await invokeReviewer(); } catch (err) { @@ -481,7 +497,7 @@ export function createAuthoritativeWorkflowSeams( } } - return { verdict: review.verdict, review: review.review, summary: review.summary }; + return { verdict: review.verdict, review: review.review, summary: review.summary, retryable: review.retryable }; }, }; } diff --git a/packages/engine/src/executor/workspace-review-per-repo.ts b/packages/engine/src/executor/workspace-review-per-repo.ts index 5174b576e4..7209110be4 100644 --- a/packages/engine/src/executor/workspace-review-per-repo.ts +++ b/packages/engine/src/executor/workspace-review-per-repo.ts @@ -11,8 +11,8 @@ * own `invokeForCwd(cwd)` once per acquired worktree (cwd = repo.worktreePath) and aggregates the repo-tagged * verdicts as a CONJUNCTION — the task is "reviewed" only if EVERY repo passes; the FIRST non-APPROVE repo's * verdict becomes the aggregate verdict (mirroring verifyWorktreeInvariants' first-failing-repo return), and its - * findings are repo-tagged. A zero-acquire workspace task (empty map) returns UNAVAILABLE so the caller routes it - * rather than fabricating an APPROVE. + * findings are repo-tagged. A zero-acquire workspace task is classified with the completion invariant: proven + * commit-free work approves honestly, while unproven work returns non-retryable UNAVAILABLE. * * Verdict severity for the conjunction: any RETHINK/REVISE/UNAVAILABLE fails the whole review; only all-APPROVE * (or all-skipped UNAVAILABLE-advisory, handled by the caller) approves. We surface the first failing repo's exact @@ -21,6 +21,7 @@ */ import type { Task } from "@fusion/core"; import type { ReviewResult } from "../execution/reviewer.js"; +import { classifyWorkspaceZeroAcquire, type WorkspaceZeroAcquireOptions } from "./workspace-zero-acquire.js"; export async function reviewWorkspacePerRepo( // FNXC:Workspace 2026-06-21-15:00: F7 — drop the dead `repoRel` callback param. @@ -29,18 +30,37 @@ export async function reviewWorkspacePerRepo( // (Phase C). The loop below still tags findings with `repoRel` from its own iteration key. task: Task, invokeForCwd: (cwd: string) => Promise, + options: Omit & { workspaceMode?: boolean } = {}, ): Promise { const workspaceWorktrees = task.workspaceWorktrees ?? {}; // FNXC:Workspace 2026-06-21-15:00: F6 — sort repo keys so the reported FIRST failing repo is // deterministic across runs/rehydrate. const repoKeys = Object.keys(workspaceWorktrees).sort(); if (repoKeys.length === 0) { - // No acquired worktree — surface UNAVAILABLE so the caller routes it rather than - // fabricating an authoritative APPROVE for an un-reviewable workspace task. + /* + FNXC:Workspace 2026-08-15-04:21: + This is the review-side consumer of classifyWorkspaceZeroAcquire. A proven + commit-free task has no diff to inspect and may approve honestly; an unproven + empty map remains unavailable, but re-invoking cannot acquire a repo, so it is + explicitly non-retryable rather than burning the review retry budget. + */ + const zeroAcquire = classifyWorkspaceZeroAcquire(task, { + workspaceMode: options.workspaceMode ?? true, + noOpCompletion: options.noOpCompletion, + noOpCompletionReason: options.noOpCompletionReason, + }); + if (zeroAcquire.kind === "commit-free-eligible") { + return { + verdict: "APPROVE", + review: `No sub-repo worktree was acquired; no diff was reviewed because this workspace task is commit-free eligible (${zeroAcquire.reason}).`, + summary: `APPROVE: no sub-repo worktree acquired (${zeroAcquire.reason})`, + }; + } return { verdict: "UNAVAILABLE", - review: "No acquired sub-repo worktree to review (workspace task with zero worktrees).", - summary: "Skipped: no sub-repo worktree", + retryable: false, + review: "No acquired sub-repo worktree to review; re-invocation cannot change this unproven zero-acquire workspace verdict.", + summary: "Unavailable: no sub-repo worktree acquired", }; } diff --git a/packages/engine/src/executor/workspace-zero-acquire.ts b/packages/engine/src/executor/workspace-zero-acquire.ts new file mode 100644 index 0000000000..022dd3b161 --- /dev/null +++ b/packages/engine/src/executor/workspace-zero-acquire.ts @@ -0,0 +1,53 @@ +import type { Task } from "@fusion/core"; +import { getNoCommitEligibilityReason } from "./no-commit-eligibility.js"; +import { evaluatePromptDerivedNoCommitEligibility } from "./prompt-derived-eligibility.js"; + +export type WorkspaceZeroAcquireClassification = + | { kind: "not-applicable" } + | { kind: "commit-free-eligible"; reason: string } + | { kind: "unproven" }; + +export type WorkspaceZeroAcquireOptions = { + workspaceMode: boolean; + noOpCompletion?: boolean; + noOpCompletionReason?: string; +}; + +/** + * FNXC:Workspace 2026-08-15-04:21: + * Completion verification and per-repo review must classify the same empty + * workspace map identically. Previously one accepted it vacuously while the + * other returned UNAVAILABLE forever; centralizing the predicate prevents those + * lifecycle ends from drifting apart again. + */ +export function classifyWorkspaceZeroAcquire( + task: Task, + options: WorkspaceZeroAcquireOptions, +): WorkspaceZeroAcquireClassification { + if (!options.workspaceMode || Object.keys(task.workspaceWorktrees ?? {}).length > 0) { + return { kind: "not-applicable" }; + } + + const explicitReason = getNoCommitEligibilityReason(task); + if (explicitReason) return { kind: "commit-free-eligible", reason: explicitReason }; + + if (options.noOpCompletion) { + return { + kind: "commit-free-eligible", + reason: options.noOpCompletionReason ?? "verified no-op/duplicate completion sentinel", + }; + } + + const prompt = typeof (task as Task & { prompt?: unknown }).prompt === "string" + ? (task as Task & { prompt: string }).prompt + : ""; + const promptEligibility = evaluatePromptDerivedNoCommitEligibility(task, prompt); + if (promptEligibility.eligible) { + return { + kind: "commit-free-eligible", + reason: promptEligibility.reason ?? "prompt-derived no-commit eligibility", + }; + } + + return { kind: "unproven" }; +} diff --git a/packages/engine/src/executor/worktree-verify-invariants.ts b/packages/engine/src/executor/worktree-verify-invariants.ts index 19b3196325..c23f35c068 100644 --- a/packages/engine/src/executor/worktree-verify-invariants.ts +++ b/packages/engine/src/executor/worktree-verify-invariants.ts @@ -21,6 +21,7 @@ import { executorLog } from "../logger.js"; import { createRunAuditor, type EngineRunContext } from "../util/run-audit.js"; import { canonicalizePath } from "./session-worktree-paths.js"; import { resolveDiffBaseRef } from "./worktree-git-refs.js"; +import { classifyWorkspaceZeroAcquire } from "./workspace-zero-acquire.js"; import { evaluatePromptDerivedNoCommitEligibility } from "./prompt-derived-eligibility.js"; import { getNoCommitEligibilityReason } from "./no-commit-eligibility.js"; import { resolveAuthoritativeExternalExecutionRoute } from "./resolve-authoritative-external-execution-route.js"; @@ -57,14 +58,29 @@ export async function verifyWorktreeInvariants( ? await deps.ensureWorkspaceConfig() : deps.workspaceConfig; const settings = await deps.store.getSettings(); - // FNXC:Workspace 2026-06-21-23:30: KTD2 — un-stubbed per-repo worktree-invariant verification. - // Phase A returned a flat {ok:true} stub here (no root worktree to verify against the non-git root). Phase B iterates every `task.workspaceWorktrees` entry, asserting (a) the sub-repo worktree's git toplevel matches the recorded repo.worktreePath and (b) its HEAD is on the recorded `fusion/` branch (repo.branch). The result union is PRESERVED EXACTLY — `{ok:true} | {ok:false; reason:'wrong_toplevel'|'wrong_branch'|'no_commits'; observed; expected}` — because the :10889 consumer switches on `reason` to drive requeue/handoff (:10894-10936). We ADD an optional `repo` field to the failure shape (purely additive; the consumer only reads reason/observed/expected) and return the FIRST failing repo. A zero-acquire workspace task (empty map) verifies vacuously → {ok:true}, matching Phase A so fn_task_done does not requeue it. + // FNXC:Workspace 2026-08-15-04:21: + // Per-repo review consumes the same zero-acquire map. Classify it before the + // loop so fn_task_done accepts only a proven commit-free task instead of + // vacuously accepting work that review must honestly leave unavailable. if (workspaceConfig) { const workspaceWorktrees = task.workspaceWorktrees ?? {}; - // FNXC:Workspace 2026-06-22-00:00: KTD2 — resolve the SAME task-wide no-commit eligibility the singular path - // uses (getNoCommitEligibilityReason / no-op-completion sentinel / prompt-derived), once, before the per-repo - // loop. When eligible (Plan-Only, verified no-op, etc.) the per-repo no_commits guard below is skipped so an - // intentionally commit-free workspace task is not blocked from completion. + const zeroAcquire = classifyWorkspaceZeroAcquire(task, { + workspaceMode: true, + noOpCompletion: options?.noOpCompletion, + noOpCompletionReason: options?.noOpCompletionReason, + }); + if (zeroAcquire.kind === "commit-free-eligible") { + executorLog.debug(`${task.id}: workspace fn_task_done zero-acquire accepted (${zeroAcquire.reason})`); + return { ok: true }; + } + if (zeroAcquire.kind === "unproven") { + return { + ok: false, + reason: "no_commits", + observed: "0 acquired sub-repo worktrees", + expected: "at least one acquired sub-repo worktree (fn_acquire_repo_worktree), or a no-op completion sentinel / noCommitsExpected", + }; + } const workspacePromptContent = (task as Task & { prompt?: unknown }).prompt; const workspacePromptEligibility = evaluatePromptDerivedNoCommitEligibility( task, @@ -78,9 +94,6 @@ export async function verifyWorktreeInvariants( (workspacePromptEligibility.eligible ? workspacePromptEligibility.reason ?? "prompt-derived no-commit eligibility" : null); - if (workspaceNoCommitEligibilityReason) { - executorLog.debug(`${task.id}: workspace fn_task_done no_commits guard skipped (${workspaceNoCommitEligibilityReason})`); - } // FNXC:Workspace 2026-06-21-15:00: F6 — iterate sorted repo keys so the FIRST failing repo // returned here is deterministic across runs/rehydrate (the value is surfaced to the operator). const commitCounts: string[] = []; diff --git a/packages/engine/src/workflows/workflow-node-handlers.ts b/packages/engine/src/workflows/workflow-node-handlers.ts index 84cbd10a14..eb0dadbe95 100644 --- a/packages/engine/src/workflows/workflow-node-handlers.ts +++ b/packages/engine/src/workflows/workflow-node-handlers.ts @@ -136,6 +136,7 @@ export interface StepReviewSeamResult { verdict: "APPROVE" | "REVISE" | "RETHINK" | "UNAVAILABLE"; review?: string; summary?: string; + retryable?: boolean; } /** The reserved context key carrying the active foreach instance (KTD-3, U3). @@ -488,9 +489,12 @@ export function createPrimitivePromptLikeHandler( }; } -/** Per-step-review-node cap on UNAVAILABLE retries before routing the - * `outcome:unavailable` edge (KTD-4 — mirrors the in-session - * `planSpecUnavailableCounts` limiter posture, executor.ts ~7297). */ +/* +FNXC:WorkflowReviewGates 2026-08-15-04:21: +This cap is reserved for transient reviewer unavailability. A deterministic +UNAVAILABLE such as an unproven zero-acquire workspace map cannot change by +retrying, so handlers route it after one attempt without consuming this budget. +*/ const STEP_REVIEW_UNAVAILABLE_RETRY_CAP = 2; /** Resolve a step-review node's config (KTD-4). Defaults `type` to `code` (the @@ -551,7 +555,7 @@ export function createStepReviewHandler(seams: WorkflowLegacySeams): WorkflowNod let result: StepReviewSeamResult = { verdict: "UNAVAILABLE" }; for (let attempt = 0; attempt <= STEP_REVIEW_UNAVAILABLE_RETRY_CAP; attempt++) { result = await seams.stepReview(ctx.task, ctx.context, config); - if (result.verdict !== "UNAVAILABLE") break; + if (result.verdict !== "UNAVAILABLE" || result.retryable === false) break; } // Persist the verdict onto the active context so the foreach sub-walk writes @@ -612,7 +616,7 @@ export function createPrimitiveStepReviewHandler(primitives: WorkflowRuntimePrim } primitivePatch = primitiveResult.contextPatch; result = primitiveResult.data ?? { verdict: "UNAVAILABLE" as const }; - if (result.verdict !== "UNAVAILABLE") break; + if (result.verdict !== "UNAVAILABLE" || result.retryable === false) break; } if (!advisory) {