FN-9060: reconcile zero-acquire workspace completion reviews
Align workspace completion and per-repo review behavior for tasks with no acquired worktrees. - Centralize zero-acquire classification for completion and review paths. - Approve proven commit-free tasks and immediately route deterministic unavailable reviews. - Add regression coverage, workflow documentation, and a patch changeset. Files changed: .changeset/fn-9060-workspace-zero-acquire.md | 7 +++ docs/workflow-steps.md | 2 +- .../__tests__/executor-workspace-taskdone.test.ts | 14 +++++ .../src/__tests__/executor-workspace.test.ts | 10 ++-- .../src/__tests__/reviewer-workspace.test.ts | 37 +++++++++++++- .../src/__tests__/workflow-step-review.test.ts | 14 +++++ .../src/__tests__/workspace-zero-acquire.test.ts | 59 ++++++++++++++++++++++ packages/engine/src/execution/reviewer.ts | 7 +++ .../create-authoritative-workflow-seams.ts | 26 ++++++++-- .../src/executor/workspace-review-per-repo.ts | 32 +++++++++--- .../engine/src/executor/workspace-zero-acquire.ts | 53 +++++++++++++++++++ .../engine/src/executor/worktree-verify-invariants.ts | 31 ++++++++---- .../engine/src/workflows/workflow-node-handlers.ts | 14 +++-- 13 files changed, 274 insertions(+), 32 deletions(-) Fusion-Task-Id: FN-9060 Fusion-Task-Lineage: eb88f881-7db2-49d2-b79a-796eacd4fa40 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-9060-workspace-zero-acquire.md
Normal file
7
.changeset/fn-9060-workspace-zero-acquire.md
Normal file
@@ -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.
|
||||||
@@ -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` 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.
|
`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.
|
||||||
|
|
||||||
|
|||||||
@@ -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", () => {
|
describeIfGit("U2 KTD4 — per-repo worktree-invariant verify in fn_task_done", () => {
|
||||||
let fx: WorkspaceFixture;
|
let fx: WorkspaceFixture;
|
||||||
afterEach(() => fx?.cleanup());
|
afterEach(() => fx?.cleanup());
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ function createStore(overrides: Partial<Record<string, unknown>> = {}): TaskStor
|
|||||||
updateTask: vi.fn().mockResolvedValue(undefined),
|
updateTask: vi.fn().mockResolvedValue(undefined),
|
||||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||||
getSettings: vi.fn().mockResolvedValue({ autoMerge: false }),
|
getSettings: vi.fn().mockResolvedValue({ autoMerge: false }),
|
||||||
|
getTask: vi.fn().mockResolvedValue(undefined),
|
||||||
on: emitter.on.bind(emitter),
|
on: emitter.on.bind(emitter),
|
||||||
...overrides,
|
...overrides,
|
||||||
}) as unknown as TaskStore & EventEmitter;
|
}) as unknown as TaskStore & EventEmitter;
|
||||||
@@ -256,17 +257,16 @@ describeIfGit("U1 KTD1 — verifyWorktreeInvariants gated off in workspace mode"
|
|||||||
let fx: WorkspaceFixture;
|
let fx: WorkspaceFixture;
|
||||||
afterEach(() => fx?.cleanup());
|
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();
|
fx = await createWorkspaceFixture();
|
||||||
const store = createStore();
|
const store = createStore();
|
||||||
const executor = new TaskExecutor(store, fx.rootDir);
|
const executor = new TaskExecutor(store, fx.rootDir);
|
||||||
(executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig;
|
(executor as any).workspaceConfig = { repos: fx.repos } as WorkspaceConfig;
|
||||||
|
|
||||||
// A workspace task that acquired ZERO sub-repos has no task.worktree and no
|
// A workspace task that acquired ZERO sub-repos has no task.worktree. It must
|
||||||
// tracked paths. The singular invariant would otherwise refuse on
|
// acquire one before claiming completion unless it proves commit-free intent.
|
||||||
// "missing task.worktree"; in workspace mode it is gated OFF.
|
|
||||||
const result = await (executor as any).verifyWorktreeInvariants(makeTask("FN-WS-1", { worktree: undefined }));
|
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 () => {
|
it("non-workspace task with no worktree still fails the invariant (regression: gate is workspace-only)", async () => {
|
||||||
|
|||||||
@@ -176,12 +176,25 @@ describe("U2 KTD3 — reviewWorkspacePerRepo conjunction + tagging (the shared l
|
|||||||
expect(result.summary).toMatch(/^repo-a:/);
|
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 task = makeTask({ workspaceWorktrees: {} });
|
||||||
const executor = workspaceExecutor(makeStore(task));
|
const executor = workspaceExecutor(makeStore(task));
|
||||||
const invoke = vi.fn();
|
const invoke = vi.fn();
|
||||||
const result = await (executor as any).reviewWorkspacePerRepo(task, invoke);
|
const result = await (executor as any).reviewWorkspacePerRepo(task, invoke);
|
||||||
expect(result.verdict).toBe("UNAVAILABLE");
|
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();
|
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");
|
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 () => {
|
it("passes unified user comments and legacy steering into workflow graph stepReview", async () => {
|
||||||
const task = makeTask({
|
const task = makeTask({
|
||||||
worktree: WT_A,
|
worktree: WT_A,
|
||||||
|
|||||||
@@ -221,6 +221,20 @@ describe("WorkflowGraphExecutor step-review (U5)", () => {
|
|||||||
expect(result.outcome).toBe("success");
|
expect(result.outcome).toBe("success");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("deterministic UNAVAILABLE routes unavailable after one attempt without changing the verdict", async () => {
|
||||||
|
const stepReview = vi.fn(async (): Promise<StepReviewSeamResult> => ({ 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 () => {
|
it("persists the verdict into the instance row", async () => {
|
||||||
const saved: WorkflowStepInstanceState[] = [];
|
const saved: WorkflowStepInstanceState[] = [];
|
||||||
const seams = baseSeams({
|
const seams = baseSeams({
|
||||||
|
|||||||
59
packages/engine/src/__tests__/workspace-zero-acquire.test.ts
Normal file
59
packages/engine/src/__tests__/workspace-zero-acquire.test.ts
Normal file
@@ -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> = {}): 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<Task>), { 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" });
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -81,6 +81,13 @@ export interface ReviewResult {
|
|||||||
verdict: ReviewVerdict;
|
verdict: ReviewVerdict;
|
||||||
review: string;
|
review: string;
|
||||||
summary: 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 {
|
export interface ReviewOptions {
|
||||||
|
|||||||
@@ -28,7 +28,7 @@ import {
|
|||||||
resolveValidatorThinkingLevel,
|
resolveValidatorThinkingLevel,
|
||||||
resolveValidatorFallbackThinkingLevel,
|
resolveValidatorFallbackThinkingLevel,
|
||||||
} from "../agents/agent-session-helpers.js";
|
} from "../agents/agent-session-helpers.js";
|
||||||
import type { ReviewVerdict } from "../execution/reviewer.js";
|
import type { ReviewResult } from "../execution/reviewer.js";
|
||||||
import {
|
import {
|
||||||
buildReviewUnavailableMessage,
|
buildReviewUnavailableMessage,
|
||||||
buildPlanVerifiedMessage,
|
buildPlanVerifiedMessage,
|
||||||
@@ -420,19 +420,35 @@ export function createAuthoritativeWorkflowSeams(
|
|||||||
onSessionEnded: (s) => deps.unregisterSubagentSession(seamTask.id, s),
|
onSessionEnded: (s) => deps.unregisterSubagentSession(seamTask.id, s),
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
const runForCwd = (cwd: string): Promise<{ verdict: ReviewVerdict; review: string; summary: string }> => {
|
const runForCwd = (cwd: string): Promise<ReviewResult> => {
|
||||||
const invoke = () => invokeReviewerForCwd(cwd);
|
const invoke = () => invokeReviewerForCwd(cwd);
|
||||||
return sem ? sem.runNested(invoke) : invoke();
|
return sem ? sem.runNested(invoke) : invoke();
|
||||||
};
|
};
|
||||||
const workspaceConfig = deps.ensureWorkspaceConfig
|
const workspaceConfig = deps.ensureWorkspaceConfig
|
||||||
? await deps.ensureWorkspaceConfig()
|
? await deps.ensureWorkspaceConfig()
|
||||||
: deps.workspaceConfig;
|
: 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 = () =>
|
const invokeReviewer = () =>
|
||||||
workspaceConfig && reviewCwd === worktreePath
|
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);
|
: runForCwd(reviewCwd);
|
||||||
|
|
||||||
let review: { verdict: ReviewVerdict; review: string; summary: string };
|
let review: ReviewResult;
|
||||||
try {
|
try {
|
||||||
review = await invokeReviewer();
|
review = await invokeReviewer();
|
||||||
} catch (err) {
|
} 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 };
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -11,8 +11,8 @@
|
|||||||
* own `invokeForCwd(cwd)` once per acquired worktree (cwd = repo.worktreePath) and aggregates the repo-tagged
|
* 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
|
* 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
|
* 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
|
* findings are repo-tagged. A zero-acquire workspace task is classified with the completion invariant: proven
|
||||||
* rather than fabricating an APPROVE.
|
* 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
|
* 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
|
* (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 { Task } from "@fusion/core";
|
||||||
import type { ReviewResult } from "../execution/reviewer.js";
|
import type { ReviewResult } from "../execution/reviewer.js";
|
||||||
|
import { classifyWorkspaceZeroAcquire, type WorkspaceZeroAcquireOptions } from "./workspace-zero-acquire.js";
|
||||||
|
|
||||||
export async function reviewWorkspacePerRepo(
|
export async function reviewWorkspacePerRepo(
|
||||||
// FNXC:Workspace 2026-06-21-15:00: F7 — drop the dead `repoRel` callback param.
|
// 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.
|
// (Phase C). The loop below still tags findings with `repoRel` from its own iteration key.
|
||||||
task: Task,
|
task: Task,
|
||||||
invokeForCwd: (cwd: string) => Promise<ReviewResult>,
|
invokeForCwd: (cwd: string) => Promise<ReviewResult>,
|
||||||
|
options: Omit<WorkspaceZeroAcquireOptions, "workspaceMode"> & { workspaceMode?: boolean } = {},
|
||||||
): Promise<ReviewResult> {
|
): Promise<ReviewResult> {
|
||||||
const workspaceWorktrees = task.workspaceWorktrees ?? {};
|
const workspaceWorktrees = task.workspaceWorktrees ?? {};
|
||||||
// FNXC:Workspace 2026-06-21-15:00: F6 — sort repo keys so the reported FIRST failing repo is
|
// FNXC:Workspace 2026-06-21-15:00: F6 — sort repo keys so the reported FIRST failing repo is
|
||||||
// deterministic across runs/rehydrate.
|
// deterministic across runs/rehydrate.
|
||||||
const repoKeys = Object.keys(workspaceWorktrees).sort();
|
const repoKeys = Object.keys(workspaceWorktrees).sort();
|
||||||
if (repoKeys.length === 0) {
|
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 {
|
return {
|
||||||
verdict: "UNAVAILABLE",
|
verdict: "UNAVAILABLE",
|
||||||
review: "No acquired sub-repo worktree to review (workspace task with zero worktrees).",
|
retryable: false,
|
||||||
summary: "Skipped: no sub-repo worktree",
|
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",
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
53
packages/engine/src/executor/workspace-zero-acquire.ts
Normal file
53
packages/engine/src/executor/workspace-zero-acquire.ts
Normal file
@@ -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" };
|
||||||
|
}
|
||||||
@@ -21,6 +21,7 @@ import { executorLog } from "../logger.js";
|
|||||||
import { createRunAuditor, type EngineRunContext } from "../util/run-audit.js";
|
import { createRunAuditor, type EngineRunContext } from "../util/run-audit.js";
|
||||||
import { canonicalizePath } from "./session-worktree-paths.js";
|
import { canonicalizePath } from "./session-worktree-paths.js";
|
||||||
import { resolveDiffBaseRef } from "./worktree-git-refs.js";
|
import { resolveDiffBaseRef } from "./worktree-git-refs.js";
|
||||||
|
import { classifyWorkspaceZeroAcquire } from "./workspace-zero-acquire.js";
|
||||||
import { evaluatePromptDerivedNoCommitEligibility } from "./prompt-derived-eligibility.js";
|
import { evaluatePromptDerivedNoCommitEligibility } from "./prompt-derived-eligibility.js";
|
||||||
import { getNoCommitEligibilityReason } from "./no-commit-eligibility.js";
|
import { getNoCommitEligibilityReason } from "./no-commit-eligibility.js";
|
||||||
import { resolveAuthoritativeExternalExecutionRoute } from "./resolve-authoritative-external-execution-route.js";
|
import { resolveAuthoritativeExternalExecutionRoute } from "./resolve-authoritative-external-execution-route.js";
|
||||||
@@ -57,14 +58,29 @@ export async function verifyWorktreeInvariants(
|
|||||||
? await deps.ensureWorkspaceConfig()
|
? await deps.ensureWorkspaceConfig()
|
||||||
: deps.workspaceConfig;
|
: deps.workspaceConfig;
|
||||||
const settings = await deps.store.getSettings();
|
const settings = await deps.store.getSettings();
|
||||||
// FNXC:Workspace 2026-06-21-23:30: KTD2 — un-stubbed per-repo worktree-invariant verification.
|
// FNXC:Workspace 2026-08-15-04:21:
|
||||||
// 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/<id>` 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.
|
// 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) {
|
if (workspaceConfig) {
|
||||||
const workspaceWorktrees = task.workspaceWorktrees ?? {};
|
const workspaceWorktrees = task.workspaceWorktrees ?? {};
|
||||||
// FNXC:Workspace 2026-06-22-00:00: KTD2 — resolve the SAME task-wide no-commit eligibility the singular path
|
const zeroAcquire = classifyWorkspaceZeroAcquire(task, {
|
||||||
// uses (getNoCommitEligibilityReason / no-op-completion sentinel / prompt-derived), once, before the per-repo
|
workspaceMode: true,
|
||||||
// loop. When eligible (Plan-Only, verified no-op, etc.) the per-repo no_commits guard below is skipped so an
|
noOpCompletion: options?.noOpCompletion,
|
||||||
// intentionally commit-free workspace task is not blocked from completion.
|
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 workspacePromptContent = (task as Task & { prompt?: unknown }).prompt;
|
||||||
const workspacePromptEligibility = evaluatePromptDerivedNoCommitEligibility(
|
const workspacePromptEligibility = evaluatePromptDerivedNoCommitEligibility(
|
||||||
task,
|
task,
|
||||||
@@ -78,9 +94,6 @@ export async function verifyWorktreeInvariants(
|
|||||||
(workspacePromptEligibility.eligible
|
(workspacePromptEligibility.eligible
|
||||||
? workspacePromptEligibility.reason ?? "prompt-derived no-commit eligibility"
|
? workspacePromptEligibility.reason ?? "prompt-derived no-commit eligibility"
|
||||||
: null);
|
: 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
|
// 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).
|
// returned here is deterministic across runs/rehydrate (the value is surfaced to the operator).
|
||||||
const commitCounts: string[] = [];
|
const commitCounts: string[] = [];
|
||||||
|
|||||||
@@ -136,6 +136,7 @@ export interface StepReviewSeamResult {
|
|||||||
verdict: "APPROVE" | "REVISE" | "RETHINK" | "UNAVAILABLE";
|
verdict: "APPROVE" | "REVISE" | "RETHINK" | "UNAVAILABLE";
|
||||||
review?: string;
|
review?: string;
|
||||||
summary?: string;
|
summary?: string;
|
||||||
|
retryable?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** The reserved context key carrying the active foreach instance (KTD-3, U3).
|
/** 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
|
FNXC:WorkflowReviewGates 2026-08-15-04:21:
|
||||||
* `planSpecUnavailableCounts` limiter posture, executor.ts ~7297). */
|
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;
|
const STEP_REVIEW_UNAVAILABLE_RETRY_CAP = 2;
|
||||||
|
|
||||||
/** Resolve a step-review node's config (KTD-4). Defaults `type` to `code` (the
|
/** 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" };
|
let result: StepReviewSeamResult = { verdict: "UNAVAILABLE" };
|
||||||
for (let attempt = 0; attempt <= STEP_REVIEW_UNAVAILABLE_RETRY_CAP; attempt++) {
|
for (let attempt = 0; attempt <= STEP_REVIEW_UNAVAILABLE_RETRY_CAP; attempt++) {
|
||||||
result = await seams.stepReview(ctx.task, ctx.context, config);
|
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
|
// 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;
|
primitivePatch = primitiveResult.contextPatch;
|
||||||
result = primitiveResult.data ?? { verdict: "UNAVAILABLE" as const };
|
result = primitiveResult.data ?? { verdict: "UNAVAILABLE" as const };
|
||||||
if (result.verdict !== "UNAVAILABLE") break;
|
if (result.verdict !== "UNAVAILABLE" || result.retryable === false) break;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!advisory) {
|
if (!advisory) {
|
||||||
|
|||||||
Reference in New Issue
Block a user