Add bounded review convergence so disputed review gates can resolve independently before requiring operator intervention. - Add staged remediation, arbitration, dispute tooling, and stale-attempt fencing. - Preserve review history and findings while exposing convergence state in core, dashboard, and audit paths. - Add migration, configuration, documentation, changeset, and regression coverage. Files changed: .changeset/fn-149-review-convergence.md | 7 + AGENTS.md | 2 + docs/architecture.md | 4 + docs/dashboard-guide.md | 4 + docs/run-audit.md | 4 + docs/settings-reference.md | 4 + docs/workflow-steps.md | 8 + .../src/__tests__/default-workflow-hooks.test.ts | 13 + packages/core/src/__tests__/plan-approval.test.ts | 21 ++ .../src/__tests__/reopen-semantics-by-role.test.ts | 25 ++ packages/core/src/__tests__/review-severity-gate.test.ts | 18 +- packages/core/src/__tests__/workflow-step-results.test.ts | 52 +++- packages/core/src/config/settings-schema.ts | 6 + packages/core/src/index.gate.ts | 9 + packages/core/src/index.ts | 9 + packages/core/src/planner/plan-approval.ts | 27 +- .../0065_fn_149_review_convergence_stage.sql | 3 + packages/core/src/postgres/schema-applier.ts | 10 + packages/core/src/postgres/schema/project.ts | 2 + packages/core/src/store.ts | 2 +- packages/core/src/task-store/persistence.ts | 4 + packages/core/src/task-store/reset-lifecycle.ts | 2 + packages/core/src/task-store/serialization.ts | 2 + packages/core/src/task-store/task-mutation-ops.ts | 2 +- packages/core/src/task-store/task-row-mappers.ts | 2 +- packages/core/src/task-store/task-update.ts | 4 + packages/core/src/tasks/in-review-stall.ts | 21 ++ packages/core/src/tasks/manual-retry-reset.ts | 2 + packages/core/src/types/settings/settings-scope.ts | 7 + packages/core/src/types/task/task-core.ts | 3 + packages/core/src/types/task/task-review.ts | 2 +- packages/core/src/types/workflow/workflow-steps.ts | 24 +- .../src/workflows/builtin-workflow-settings.ts | 43 +++ .../core/src/workflows/default-workflow-hooks.ts | 10 +- .../core/src/workflows/workflow-step-results.ts | 185 ++++++++++++- .../dashboard/app/components/TaskReviewTab.tsx | 6 +- .../components/__tests__/TaskReviewTab.test.tsx | 6 +- .../dashboard/src/__tests__/routes-tasks.test.ts | 7 + .../src/routes/register-task-workflow-routes.ts | 4 +- .../__tests__/review-arbitration-release.test.ts | 46 ++++ .../src/__tests__/review-arbitration.test.ts | 57 ++++ .../__tests__/review-convergence-context.test.ts | 95 +++++++ .../review-convergence-escalation.test.ts | 135 +++++++++ .../src/__tests__/review-dispute-tool.test.ts | 113 +++++++ .../__tests__/review-finding-supersession.test.ts | 55 ++- .../review-findings-injection.test.ts | 19 +- .../__tests__/review-history-preservation.test.ts | 186 +++++++++++++ .../__tests__/review-ladder-entry-points.test.ts | 304 +++++++++++++++++++++ ...un-audit-sink-health-review-convergence.test.ts | 251 +++++++++++++++++ .../workflow-graph-optional-step-fix.test.ts | 59 ++-- packages/engine/src/execution/replan-target.ts | 6 +- .../clear-terminal-step-failures-for-retry.ts | 8 +- .../src/executor/create-review-dispute-tool.ts | 126 +++++++++ packages/engine/src/executor/deps-bags.ts | 2 +- .../engine/src/executor/execute-workflow-graph.ts | 71 ++++- packages/engine/src/executor/execute-workflow-step.ts | 16 +- packages/engine/src/executor/free-reexports.ts | 1 + packages/engine/src/executor/impl-bindings.ts | 1 + .../engine/src/executor/optional-step-revision.ts | 33 ++- .../src/executor/recover-failed-pre-merge-step.ts | 43 ++- .../request-pre-merge-optional-step-fix.ts | 85 +++--- packages/engine/src/executor/review-arbitration.ts | 233 +++++++++++++++ .../src/executor/review-convergence-ladder.ts | 237 +++++++++++++++ .../route-graph-failure-to-execution-resume.ts | 16 +- .../route-retryable-remediation.ts | 9 +- .../engine/src/executor/run-graph-custom-node.ts | 1 + packages/engine/src/executor/run-implementation.ts | 2 + .../src/executor/task-executor-session-facades.ts | 3 +- .../engine/src/executor/workflow-rerun-bounce.ts | 13 +- .../executor/workflow-step-failure-injection.ts | 23 +- .../engine/src/executor/workflow-step-failures.ts | 16 +- .../engine/src/executor/workflow-step-verdict.ts | 2 + packages/engine/src/self-healing.ts | 5 +- packages/engine/src/util/run-audit.ts | 9 +- .../src/workflows/workflow-graph-executor.ts | 8 +- .../engine/src/worktree/review-diff-fingerprint.ts | 21 +++ .../src/worktree/workspace-review-evidence.ts | 9 +- 77 files changed, 2719 insertions(+), 166 deletions(-) Fusion-Task-Id: FN-149 Fusion-Task-Lineage: 9330df9d-5086-4b40-8daa-66f0ea31cf3e Co-authored-by: Fusion <noreply@runfusion.ai>
234 lines
11 KiB
TypeScript
234 lines
11 KiB
TypeScript
/*
|
|
FNXC:ReviewConvergence 2026-08-22-05:44:
|
|
FN-149 requires a second, independent validator decision after a bounded remediation escalation.
|
|
The arbiter may release only the failed review attempt whose identity it captured; it never uses the
|
|
blanket remediation archive because concurrent review gates may still be blocking.
|
|
*/
|
|
import type { ArbitrationFailureFence, Task, TaskStore, WorkflowReviewFinding, WorkflowStepResult } from "@fusion/core";
|
|
import {
|
|
archiveArbitratedWorkflowStepFailure,
|
|
resolveReviewArbitrationTarget,
|
|
resolveValidatorFallbackModel,
|
|
} from "@fusion/core";
|
|
import { mergeEffectiveSettings } from "../project/effective-settings.js";
|
|
import { reviewStep } from "../execution/reviewer.js";
|
|
import type { EngineRunContext } from "../util/run-audit.js";
|
|
import { emitBoundedRunAudit } from "./emit-bounded-run-audit.js";
|
|
|
|
export type ReviewArbitrationReleaseDeps = {
|
|
store: TaskStore;
|
|
getRunContextFor: (taskId: string) => EngineRunContext | undefined;
|
|
};
|
|
|
|
export type ReviewArbitrationDeps = ReviewArbitrationReleaseDeps & {
|
|
sendTaskBackForFix: (
|
|
task: Task, worktreePath: string, failureFeedback: string, stepName: string, reason: string,
|
|
preserveResumeState: boolean, mergeVerificationFailure: boolean,
|
|
retryPresentation?: { attempt: number; max?: number }, findings?: WorkflowReviewFinding[],
|
|
) => Promise<void>;
|
|
};
|
|
|
|
type ArbitrationDecision = "UPHOLD_REVIEW" | "UPHOLD_IMPLEMENTER" | "SPLIT";
|
|
|
|
function parseArbitrationDecision(value: string): { decision: ArbitrationDecision; notes: string; bindingFindingIds: string[] } | undefined {
|
|
const candidate = value.match(/\{\s*"decision"\s*:\s*"(?:UPHOLD_REVIEW|UPHOLD_IMPLEMENTER|SPLIT)"[\s\S]*\}\s*$/)?.[0];
|
|
if (!candidate) return undefined;
|
|
try {
|
|
const parsed = JSON.parse(candidate) as { decision?: unknown; notes?: unknown; bindingFindingIds?: unknown };
|
|
if (parsed.decision !== "UPHOLD_REVIEW" && parsed.decision !== "UPHOLD_IMPLEMENTER" && parsed.decision !== "SPLIT") return undefined;
|
|
if (typeof parsed.notes !== "string" || !Array.isArray(parsed.bindingFindingIds) || !parsed.bindingFindingIds.every((id) => typeof id === "string")) return undefined;
|
|
return { decision: parsed.decision, notes: parsed.notes, bindingFindingIds: parsed.bindingFindingIds };
|
|
} catch {
|
|
return undefined;
|
|
}
|
|
}
|
|
|
|
function collectGateAttemptHistory(task: Task, workflowStepId: string) {
|
|
const result = task.workflowStepResults?.find((entry) => entry.workflowStepId === workflowStepId);
|
|
if (!result) return [];
|
|
return [...(result.priorAttempts ?? [])].reverse().concat(result).filter((entry) => entry.verdict === "REVISE");
|
|
}
|
|
|
|
function failedFence(result: NonNullable<Task["workflowStepResults"]>[number], decision: ArbitrationDecision, bindingFindingCount: number, notes: string): ArbitrationFailureFence {
|
|
return {
|
|
workflowStepId: result.workflowStepId,
|
|
expectedStartedAt: result.startedAt,
|
|
expectedCompletedAt: result.completedAt,
|
|
expectedVerdict: result.verdict,
|
|
...(result.reviewInputFingerprint ? { expectedReviewInputFingerprint: result.reviewInputFingerprint } : {}),
|
|
decision,
|
|
bindingFindingCount,
|
|
arbitratedAt: new Date().toISOString(),
|
|
arbitrationNotes: notes,
|
|
};
|
|
}
|
|
|
|
/** Apply an already-parsed implementer ruling, refusing every stale or partial attempt. */
|
|
/*
|
|
FNXC:ReviewConvergence 2026-08-22-05:56:
|
|
FN-149 requires an implementer ruling to leave an auditable disposition on disputed findings before
|
|
its exact failed gate is archived. This transformation is deliberately limited to the fenced gate;
|
|
a sibling failed gate must remain byte-identical and merge-blocking.
|
|
*/
|
|
function upholdDisputedFindings(results: WorkflowStepResult[] | undefined, workflowStepId: string): WorkflowStepResult[] | undefined {
|
|
let changed = false;
|
|
const next = results?.map((result) => {
|
|
if (result.workflowStepId !== workflowStepId || !result.findings?.length) return result;
|
|
const findings = result.findings.map((finding) => {
|
|
if (!finding.disputedAt || (finding.resolution !== undefined && finding.resolution !== "open")) return finding;
|
|
changed = true;
|
|
return { ...finding, resolution: "dispute-upheld" as const };
|
|
});
|
|
return findings === result.findings ? result : { ...result, findings };
|
|
});
|
|
return changed ? next : results;
|
|
}
|
|
|
|
function bindingObligations(
|
|
findings: WorkflowReviewFinding[] | undefined,
|
|
bindingFindingIds: readonly string[],
|
|
decision: ArbitrationDecision,
|
|
): WorkflowReviewFinding[] | undefined {
|
|
if (!findings) return undefined;
|
|
// A malformed UPHOLD_REVIEW with no IDs must not erase obligations; a SPLIT with none releases.
|
|
const binding = new Set(bindingFindingIds);
|
|
const survivors = binding.size === 0 && decision === "UPHOLD_REVIEW"
|
|
? findings
|
|
: findings.filter((finding) => binding.has(finding.id));
|
|
return survivors.map((finding) => ({
|
|
...finding,
|
|
disputeRationale: undefined,
|
|
disputedAt: undefined,
|
|
disputeRebuttedAt: undefined,
|
|
}));
|
|
}
|
|
|
|
export async function applyReviewArbitrationRelease(
|
|
deps: ReviewArbitrationReleaseDeps,
|
|
taskId: string,
|
|
fence: ArbitrationFailureFence,
|
|
): Promise<{ applied: boolean; reason?: string }> {
|
|
let outcome: { applied: boolean; reason?: string } = { applied: false };
|
|
await deps.store.updateTaskAtomic(taskId, (task) => {
|
|
const adjudicated = upholdDisputedFindings(task.workflowStepResults, fence.workflowStepId);
|
|
const release = archiveArbitratedWorkflowStepFailure(adjudicated, fence);
|
|
if (!release.applied) {
|
|
outcome = { applied: false, reason: release.reason };
|
|
return null;
|
|
}
|
|
outcome = { applied: true };
|
|
return { workflowStepResults: release.results };
|
|
}, deps.getRunContextFor(taskId));
|
|
const context = deps.getRunContextFor(taskId);
|
|
if (context) await emitBoundedRunAudit(deps.store, {
|
|
taskId, agentId: context.agentId, runId: context.runId, domain: "database",
|
|
mutationType: "task:review-arbitration", target: taskId,
|
|
metadata: {
|
|
workflowStepId: fence.workflowStepId, decision: fence.decision,
|
|
bindingFindingCount: fence.bindingFindingCount, released: outcome.applied,
|
|
outcome: outcome.applied ? "released" : "fenced-stale",
|
|
},
|
|
});
|
|
if (outcome.applied) await deps.store.logEntry(
|
|
taskId,
|
|
"Review arbitration released adjudicated gate",
|
|
`Arbitration released only workflow step '${fence.workflowStepId}' after an implementer ruling.`,
|
|
context,
|
|
);
|
|
return outcome;
|
|
}
|
|
|
|
/** Dispatch one validator arbitration and make its ruling lifecycle-effective. */
|
|
export async function runReviewArbitration(
|
|
deps: ReviewArbitrationDeps,
|
|
task: Task,
|
|
workflowStepId: string,
|
|
stepName: string,
|
|
feedback: string,
|
|
attempt: number,
|
|
max: number | undefined,
|
|
): Promise<"arbitrated" | "declined"> {
|
|
// Never select a display-name match or an unrelated failed gate: the ladder selected this ID.
|
|
const failed = (task.workflowStepResults ?? []).find((result) =>
|
|
result.workflowStepId === workflowStepId && result.status === "failed");
|
|
if (!failed) return "declined";
|
|
const settings = await mergeEffectiveSettings(deps.store, task, await deps.store.getSettings());
|
|
const configuredTarget = resolveReviewArbitrationTarget(settings);
|
|
if (settings.reviewArbitrationEnabled === false) return "declined";
|
|
// A configured arbitration pair owns this third-model lane; otherwise use the validator fallback.
|
|
const fallbackTarget = resolveValidatorFallbackModel(settings);
|
|
const provider = configuredTarget.provider ?? fallbackTarget.provider;
|
|
const modelId = configuredTarget.modelId ?? fallbackTarget.modelId;
|
|
const history = collectGateAttemptHistory(task, failed.workflowStepId);
|
|
const prompt = `${task.prompt ?? ""}\n\n## Review arbitration\nDecide this disagreement using the code and the complete same-gate ledger below. Return exactly one trailing JSON object: {"decision":"UPHOLD_REVIEW"|"UPHOLD_IMPLEMENTER"|"SPLIT","notes":"...","bindingFindingIds":["..."]}.\n\n${JSON.stringify(history)}`;
|
|
let raw: string;
|
|
try {
|
|
const result = await reviewStep(task.worktree ?? process.cwd(), task.id, 0, `Arbitration: ${stepName}`, "code", prompt, undefined, {
|
|
store: deps.store,
|
|
taskId: task.id,
|
|
settings,
|
|
// Task-validator overrides are the highest reviewer-lane precedence, so the configured
|
|
// arbitration pair cannot be shadowed by the ordinary validator model selection.
|
|
...(provider && modelId ? { taskValidatorProvider: provider, taskValidatorModelId: modelId } : {}),
|
|
});
|
|
raw = result.review;
|
|
} catch {
|
|
return "declined";
|
|
}
|
|
const ruling = parseArbitrationDecision(raw);
|
|
if (!ruling) return "declined";
|
|
const fence = failedFence(failed, ruling.decision, ruling.bindingFindingIds.length, ruling.notes);
|
|
if ((ruling.decision === "UPHOLD_IMPLEMENTER" || (ruling.decision === "SPLIT" && ruling.bindingFindingIds.length === 0))) {
|
|
const release = await applyReviewArbitrationRelease(deps, task.id, fence);
|
|
return release.applied ? "arbitrated" : "declined";
|
|
}
|
|
const obligations = bindingObligations(failed.findings, ruling.bindingFindingIds, ruling.decision);
|
|
/*
|
|
FNXC:ReviewConvergence 2026-08-22-05:56:
|
|
A review-upheld or binding SPLIT ruling removes only the dispute annotation from the surviving
|
|
obligations. It does not release their failed gate; the next remediation receives the arbiter's
|
|
binding subset as its must-fix findings.
|
|
*/
|
|
let obligationsApplied = false;
|
|
await deps.store.updateTaskAtomic(task.id, (current) => {
|
|
const currentGate = current.workflowStepResults?.find((entry) => entry.workflowStepId === failed.workflowStepId);
|
|
const fenceStillMatches = currentGate?.status === "failed"
|
|
&& currentGate.startedAt === failed.startedAt
|
|
&& currentGate.completedAt === failed.completedAt
|
|
&& currentGate.verdict === failed.verdict
|
|
&& currentGate.reviewInputFingerprint === failed.reviewInputFingerprint
|
|
&& currentGate.supersededAt == null;
|
|
if (!fenceStillMatches) return null;
|
|
obligationsApplied = true;
|
|
return {
|
|
workflowStepResults: obligations
|
|
? current.workflowStepResults?.map((entry) => entry.workflowStepId === failed.workflowStepId
|
|
? { ...entry, findings: obligations }
|
|
: entry)
|
|
: current.workflowStepResults,
|
|
};
|
|
}, deps.getRunContextFor(task.id));
|
|
/*
|
|
FNXC:ReviewConvergence 2026-08-22-06:06:
|
|
FN-149 fences review-upheld rulings as strictly as implementer releases. An arbiter that read an
|
|
older failed attempt must not bounce a newer gate or inject obsolete obligations into it; a lost
|
|
compare-and-set is a declined, lifecycle-inert ruling rather than a remediation request.
|
|
*/
|
|
if (!obligationsApplied) {
|
|
const context = deps.getRunContextFor(task.id);
|
|
if (context) await emitBoundedRunAudit(deps.store, {
|
|
taskId: task.id, agentId: context.agentId, runId: context.runId, domain: "database",
|
|
mutationType: "task:review-arbitration", target: task.id,
|
|
metadata: {
|
|
workflowStepId: failed.workflowStepId, decision: ruling.decision,
|
|
bindingFindingCount: ruling.bindingFindingIds.length, released: false, outcome: "fenced-stale",
|
|
},
|
|
});
|
|
return "declined";
|
|
}
|
|
await deps.sendTaskBackForFix(task, task.worktree ?? "", feedback, stepName,
|
|
"Review arbitration upheld remaining review obligations", true, false,
|
|
{ attempt: attempt + 1, max }, obligations ?? failed.findings);
|
|
return "arbitrated";
|
|
}
|