root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(engine): route workflow reviews through validator models (#2533)

Browse Source 
## Summary

- classify review-type workflow steps with the existing review-step
classifier
- resolve their primary, fallback, and thinking-level settings from the
validator model lane
- retain per-step model overrides and executor-purpose workflow-step
tooling
- keep ordinary workflow steps on the execution lane
- make missing-fallback diagnostics identify the correct lane

## Why

Code Review, Plan Review, verification, and inline-review gates were
executed through the implementation model lane merely because they run
inside `executeWorkflowStep()`. That defeats configured reviewer-model
separation and can make the same model implement and validate its own
work.

This changes model selection—not the workflow-step session/tooling
contract—so review steps remain executor-purpose sessions while using
validator lane models.

## Verification

- `FUSION_PG_TEST_SKIP=1 corepack pnpm@10.33.0 --filter @fusion/engine
exec vitest run src/__tests__/executor-workflow-step-model.test.ts` — 14
passed
- `corepack pnpm@10.33.0 --filter @fusion/engine typecheck`
- `corepack pnpm@10.33.0 changeset status --since=origin/main`
- `git diff --check origin/main...HEAD`


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Review-type workflow steps now route through the configured validator
model lane (instead of the execution lane).
* Validator primary/fallback and thinking-level settings are applied
correctly for review steps.
  * Step/task overrides still take priority over lane-based resolution.
* Fallback retry sessions now use the appropriate validator/executor
configuration, with lane-specific fallback guidance when fallback
settings are missing.
* **Tests**
* Expanded executor workflow-step model resolution and routing/fallback
precedence assertions for validator-lane behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Phil Larson
2026-07-29 00:05:04 -07:00
committed by GitHub
parent 9a8fc409ff
commit 72391c90b2
4 changed files with 262 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/route-workflow-reviews-through-validator-lane.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Route review-type workflow steps through validator model lanes instead of the execution model.
category: fix
dev: Review classifiers now select validator primary, fallback, and thinking-level settings while preserving executor runtime semantics and explicit step/task overrides.

20
packages/engine/src/__tests__/executor-test-helpers.ts
View File

@@ -124,8 +124,10 @@ function withSessionDefaults(session: any, options?: { systemPrompt?: unknown })
}
vi.mock("../agent-session-helpers.js", async () => {
const actual = await vi.importActual<typeof import("../agent-session-helpers.js")>("../agent-session-helpers.js");
const { createFnAgent } = await import("../pi.js");
return {
...actual,
createResolvedAgentSession: async (options: any) => {
const result = await createFnAgent(options);
return {
@@ -157,15 +159,26 @@ vi.mock("../agent-session-helpers.js", async () => {
?? (typeof settings?.defaultThinkingLevelOverride === "string" ? settings.defaultThinkingLevelOverride : undefined)
?? (typeof settings?.defaultThinkingLevel === "string" ? settings.defaultThinkingLevel : undefined),
resolveValidatorThinkingLevel: (taskThinkingLevel: string | undefined, settings: Record<string, unknown> | undefined) =>
(typeof settings?.validatorThinkingLevel === "string" ? settings.validatorThinkingLevel : undefined)
?? taskThinkingLevel
taskThinkingLevel
?? (typeof settings?.validatorThinkingLevel === "string" ? settings.validatorThinkingLevel : undefined)
?? (typeof settings?.validatorGlobalThinkingLevel === "string" ? settings.validatorGlobalThinkingLevel : undefined)
?? (typeof (settings?.selectedWorkflowModelLanes as Record<string, unknown> | undefined)?.validatorThinkingLevel === "string"
? (settings?.selectedWorkflowModelLanes as Record<string, unknown>).validatorThinkingLevel as string
: undefined)
?? (typeof settings?.defaultThinkingLevelOverride === "string" ? settings.defaultThinkingLevelOverride : undefined)
?? (typeof settings?.defaultThinkingLevel === "string" ? settings.defaultThinkingLevel : undefined),
resolveValidatorFallbackThinkingLevel: (taskThinkingLevel: string | undefined, settings: Record<string, unknown> | undefined) =>
(typeof settings?.validatorFallbackThinkingLevel === "string" ? settings.validatorFallbackThinkingLevel : undefined)
?? (typeof settings?.fallbackThinkingLevel === "string" ? settings.fallbackThinkingLevel : undefined)
?? (typeof settings?.validatorThinkingLevel === "string" ? settings.validatorThinkingLevel : undefined)
?? (typeof (settings?.selectedWorkflowModelLanes as Record<string, unknown> | undefined)?.validatorFallbackThinkingLevel === "string"
? (settings?.selectedWorkflowModelLanes as Record<string, unknown>).validatorFallbackThinkingLevel as string
: undefined)
?? taskThinkingLevel
?? (typeof settings?.validatorThinkingLevel === "string" ? settings.validatorThinkingLevel : undefined)
?? (typeof settings?.validatorGlobalThinkingLevel === "string" ? settings.validatorGlobalThinkingLevel : undefined)
?? (typeof (settings?.selectedWorkflowModelLanes as Record<string, unknown> | undefined)?.validatorThinkingLevel === "string"
? (settings?.selectedWorkflowModelLanes as Record<string, unknown>).validatorThinkingLevel as string
: undefined)
?? (typeof settings?.defaultThinkingLevelOverride === "string" ? settings.defaultThinkingLevelOverride : undefined)
?? (typeof settings?.defaultThinkingLevel === "string" ? settings.defaultThinkingLevel : undefined),
resolveExecutorSessionModel: (
@@ -197,6 +210,7 @@ vi.mock("../agent-session-helpers.js", async () => {
}
return { provider: undefined, modelId: undefined };
},
};
});
vi.mock("../worktree-names.js", async () => {

187
packages/engine/src/__tests__/executor-workflow-step-model.test.ts
View File

@@ -9,18 +9,26 @@ import {
} from "./executor-test-helpers.js";
type CapturedSession = {
sessionPurpose?: string;
defaultProvider?: string;
defaultModelId?: string;
fallbackProvider?: string;
fallbackModelId?: string;
defaultThinkingLevel?: string;
fallbackThinkingLevel?: string;
};
function captureSession(output = '{"verdict":"APPROVE","notes":""}'): { last?: CapturedSession } {
const holder: { last?: CapturedSession } = {};
mockedCreateFnAgent.mockImplementation(async (opts: any) => {
holder.last = {
sessionPurpose: opts.sessionPurpose,
defaultProvider: opts.defaultProvider,
defaultModelId: opts.defaultModelId,
fallbackProvider: opts.fallbackProvider,
fallbackModelId: opts.fallbackModelId,
defaultThinkingLevel: opts.defaultThinkingLevel,
fallbackThinkingLevel: opts.fallbackThinkingLevel,
};
const listeners: Array<(event: any) => void> = [];
@@ -129,13 +137,22 @@ describe("executor workflow-step model resolution", () => {
const captured = await runStepWithSettings({
executionProvider: "openai",
executionModelId: "gpt-4o",
executionFallbackProvider: "openai",
executionFallbackModelId: "gpt-4o-mini",
executionThinkingLevel: "medium",
fallbackThinkingLevel: "low",
defaultProvider: "anthropic",
defaultModelId: "claude-3-5-sonnet",
});
expect(captured).toMatchObject({
sessionPurpose: "executor",
defaultProvider: "openai",
defaultModelId: "gpt-4o",
fallbackProvider: "openai",
fallbackModelId: "gpt-4o-mini",
defaultThinkingLevel: "medium",
fallbackThinkingLevel: "low",
});
expect(captured).not.toMatchObject({
defaultProvider: "anthropic",
@@ -143,6 +160,176 @@ describe("executor workflow-step model resolution", () => {
});
});
it("routes review-type workflow steps through the validator model lane", async () => {
const captured = await runStepWithSettings(
{
executionProvider: "openai-codex",
executionModelId: "gpt-5.6-terra",
validatorProvider: "openai-codex",
validatorModelId: "gpt-5.6-sol",
validatorFallbackProvider: "openai-codex",
validatorFallbackModelId: "gpt-5.6-flash",
validatorThinkingLevel: "high",
validatorFallbackThinkingLevel: "low",
},
{
step: {
id: "graph:code-review-step",
name: "Code Review",
optionalGroupId: "code-review",
},
},
);
expect(captured).toMatchObject({
sessionPurpose: "executor",
defaultProvider: "openai-codex",
defaultModelId: "gpt-5.6-sol",
fallbackProvider: "openai-codex",
fallbackModelId: "gpt-5.6-flash",
defaultThinkingLevel: "high",
fallbackThinkingLevel: "low",
});
});
it.each([
["inline-fix metadata", { name: "Implementation Check", reviewCanFixInline: true }],
["Code Review group metadata", { name: "Implementation Check", optionalGroupId: "code-review" }],
["Plan Review identity", { id: "graph:plan-review-step", name: "Plan Review" }],
["verification name", { name: "Artifact Verification" }],
])("classifies %s as validator-routed", async (_label, step) => {
await expect(
runStepWithSettings(
{
executionProvider: "executor-provider",
executionModelId: "executor-model",
validatorProvider: "validator-provider",
validatorModelId: "validator-model",
},
{ step },
),
).resolves.toMatchObject({
sessionPurpose: "executor",
defaultProvider: "validator-provider",
defaultModelId: "validator-model",
});
});
it("keeps near-match ordinary names on executor lanes", async () => {
await expect(
runStepWithSettings(
{
executionProvider: "executor-provider",
executionModelId: "executor-model",
validatorProvider: "validator-provider",
validatorModelId: "validator-model",
},
{ step: { name: "Implementation Overview" } },
),
).resolves.toMatchObject({
sessionPurpose: "executor",
defaultProvider: "executor-provider",
defaultModelId: "executor-model",
});
});
it("uses selected-workflow validator lanes after project and global validator lanes fall through", async () => {
await expect(
runStepWithSettings(
{
selectedWorkflowModelLanes: {
validatorProvider: "workflow-validator-provider",
validatorModelId: "workflow-validator-model",
validatorFallbackProvider: "workflow-fallback-provider",
validatorFallbackModelId: "workflow-fallback-model",
validatorFallbackThinkingLevel: "low",
},
defaultProvider: "default-provider",
defaultModelId: "default-model",
},
{ step: { name: "Code Review", optionalGroupId: "code-review" } },
),
).resolves.toMatchObject({
defaultProvider: "workflow-validator-provider",
defaultModelId: "workflow-validator-model",
fallbackProvider: "workflow-fallback-provider",
fallbackModelId: "workflow-fallback-model",
fallbackThinkingLevel: "low",
});
});
it("keeps review step and task overrides ahead of validator-lane settings", async () => {
await expect(
runStepWithSettings(
{
validatorProvider: "project-validator-provider",
validatorModelId: "project-validator-model",
validatorThinkingLevel: "medium",
},
{
task: {
validatorModelProvider: "task-validator-provider",
validatorModelId: "task-validator-model",
validatorThinkingLevel: "high",
thinkingLevel: "low",
},
step: {
name: "Code Review",
optionalGroupId: "code-review",
},
},
),
).resolves.toMatchObject({
defaultProvider: "task-validator-provider",
defaultModelId: "task-validator-model",
defaultThinkingLevel: "high",
});
await expect(
runStepWithSettings(
{
validatorProvider: "project-validator-provider",
validatorModelId: "project-validator-model",
},
{
step: {
name: "Code Review",
optionalGroupId: "code-review",
modelProvider: "step-provider",
modelId: "step-model",
},
},
),
).resolves.toMatchObject({
defaultProvider: "step-provider",
defaultModelId: "step-model",
});
});
it.each([
["provider only", { modelProvider: "partial-provider" }],
["model only", { modelId: "partial-model" }],
])("does not mix a %s step override with the validator lane", async (_label, partialOverride) => {
await expect(
runStepWithSettings(
{
validatorProvider: "validator-provider",
validatorModelId: "validator-model",
},
{
step: {
name: "Code Review",
optionalGroupId: "code-review",
...partialOverride,
},
},
),
).resolves.toMatchObject({
defaultProvider: "validator-provider",
defaultModelId: "validator-model",
});
});
it("keeps step and task overrides ahead of execution-lane settings", async () => {
await expect(
runStepWithSettings(

82
packages/engine/src/executor.ts
View File

@@ -16,7 +16,7 @@ import type { TaskStore, Task, TaskDetail, TaskTokenUsage, StepStatus, Settings,
import { getUnmetSchedulingDependencies } from "./scheduler.js";
import type { ImplementationExit, ImplementationExitReporter } from "./executor/implementation-exit.js";
import { emitWorkflowLifecycleEvent } from "@fusion/core";
import { RetryStormError, serializeRetryStormError, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, resolveWorkflowIrForTask, evaluateForeachMergeProof, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveReboundTarget, resolveLifecycleColumns, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, isLiveSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveMaxConsecutiveToolFailureRetries, resolveConsecutiveToolFailureRetryBackoffMs, resolveConsecutiveToolFailureThreshold, resolveExecutorEscalationTarget, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, DEFAULT_MAX_POST_REVIEW_FIXES, COMPLETION_SUMMARY_NODE_ID, upsertWorkflowStepResult, AWAITING_APPROVAL_PAUSE_REASON, THINKING_LEVELS, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AgentStore, resolveExecutorFallbackModel } from "@fusion/core";
import { RetryStormError, serializeRetryStormError, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, resolveWorkflowIrForTask, evaluateForeachMergeProof, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveReboundTarget, resolveLifecycleColumns, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, isLiveSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveMaxConsecutiveToolFailureRetries, resolveConsecutiveToolFailureRetryBackoffMs, resolveConsecutiveToolFailureThreshold, resolveExecutorEscalationTarget, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, DEFAULT_MAX_POST_REVIEW_FIXES, COMPLETION_SUMMARY_NODE_ID, upsertWorkflowStepResult, AWAITING_APPROVAL_PAUSE_REASON, THINKING_LEVELS, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AgentStore, resolveExecutorFallbackModel, resolveValidatorFallbackModel } from "@fusion/core";
import { finalizeProvenAutoMergeTask } from "./auto-merge-finalization.js";
import { mergeEffectiveSettings } from "./effective-settings.js";
import { generateFeatureVideo, type GenerateFeatureVideoOptions } from "./review-artifacts/feature-video.js";
@@ -106,6 +106,7 @@ import {
createResolvedAgentSession,
extractRuntimeHint,
resolveExecutorSessionModel,
resolveValidatorSessionModel,
resolveExecutorThinkingLevel,
resolveExecutorFallbackThinkingLevel,
resolveValidatorThinkingLevel,
@@ -17535,29 +17536,42 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
},
});
// Determine primary model and an explicit fallback. The workflow step's
// own override takes precedence; otherwise use the canonical executor
// hierarchy: task override → project execution lane → global execution lane
// → project default override → global default. The fallback is the per-step
// override's missing-counterpart settings, then the executor fallback lane,
// which itself falls through to the shared global fallback pair.
// FNXC:ModelResolution 2026-06-25-12:00: FN-7039 requires workflow steps to inherit project execution-lane model settings before default settings so configured Execution models reach step sessions unless the step itself overrides them.
// Determine primary model and an explicit fallback. Review-type workflow
// steps use the validator lane; ordinary workflow prompts use the executor
// lane. A complete per-step override remains authoritative for either lane.
// FNXC:ModelResolution 2026-06-25-12:00: FN-7039 requires ordinary workflow
// steps to inherit project execution-lane model settings before defaults.
// Review gates are independent validation surfaces and must not silently use
// the same implementation model merely because they execute in this method.
const assignedRuntimeConfig = await this.getAssignedAgentRuntimeConfig(task.assignedAgentId);
const executorModel = resolveExecutorSessionModel(
task.modelProvider,
task.modelId,
settings,
assignedRuntimeConfig,
);
const primaryProvider = workflowStep.modelProvider || executorModel.provider;
const primaryModelId = workflowStep.modelId || executorModel.modelId;
const laneModel = isReviewTypeWorkflowStep
? resolveValidatorSessionModel(
task.validatorModelProvider,
task.validatorModelId,
settings,
assignedRuntimeConfig,
)
: resolveExecutorSessionModel(
task.modelProvider,
task.modelId,
settings,
assignedRuntimeConfig,
);
const useOverride = !!(workflowStep.modelProvider && workflowStep.modelId);
const primaryProvider = useOverride ? workflowStep.modelProvider : laneModel.provider;
const primaryModelId = useOverride ? workflowStep.modelId : laneModel.modelId;
const executorFallback = resolveExecutorFallbackModel(settings);
const fallback = executorFallback.provider && executorFallback.modelId
&& (executorFallback.provider !== primaryProvider || executorFallback.modelId !== primaryModelId)
? executorFallback
const workflowFallback = isReviewTypeWorkflowStep
? resolveValidatorFallbackModel(settings)
: resolveExecutorFallbackModel(settings);
const fallback = workflowFallback.provider && workflowFallback.modelId
&& (workflowFallback.provider !== primaryProvider || workflowFallback.modelId !== primaryModelId)
? workflowFallback
: undefined;
const fallbackSettingsHint = isReviewTypeWorkflowStep
? "settings.validatorFallbackProvider/validatorFallbackModelId or fallbackProvider/fallbackModelId"
: "settings.executionFallbackProvider/executionFallbackModelId or fallbackProvider/fallbackModelId";
const fallbackLaneLabel = isReviewTypeWorkflowStep ? "validator" : "executor";
const timeoutMs = Math.max(60_000, settings.workflowStepTimeoutMs ?? 900_000);
@@ -17566,7 +17580,6 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
modelId: string | undefined,
attemptLabel: string,
): Promise<WorkflowStepOutcome> => {
// Workflow step agents inherit executor instructions
const stepInstructions = await this.resolveInstructionsForRole("executor", settings);
const stepSystemPrompt = buildSystemPromptWithInstructions(systemPrompt, stepInstructions);
@@ -17700,16 +17713,23 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
/*
* FNXC:Settings-ThinkingLevel 2026-07-10-00:00:
* WorkflowStep sessions resolve reasoning effort as node/step `thinkingLevel` first, then task override, then settings defaults/lane fallbacks.
* WorkflowStep sessions resolve reasoning effort as node/step `thinkingLevel` first, then the task override for their selected model lane, then settings defaults/lane fallbacks.
*
* FNXC:Settings-ThinkingLevel 2026-07-10-14:20:
* The step's own `fallback` attempt already swaps to a distinct model (validator fallback OR global fallback pair) — it must honor THAT model's fallback thinking level, not silently reuse the primary lane's thinking level. Route by which candidate `fallback.label` actually matched instead of only special-casing `validatorFallback`.
*/
const workflowStepThinkingSource = workflowStep.thinkingLevel ?? task.thinkingLevel;
const workflowStepThinkingSource = workflowStep.thinkingLevel
?? (isReviewTypeWorkflowStep ? task.validatorThinkingLevel ?? task.thinkingLevel : task.thinkingLevel);
const workflowStepThinkingLevel = attemptLabel === "fallback"
? resolveExecutorFallbackThinkingLevel(workflowStepThinkingSource, settings)
: resolveExecutorThinkingLevel(workflowStepThinkingSource, settings);
const workflowStepFallbackThinkingLevel = resolveExecutorFallbackThinkingLevel(workflowStepThinkingSource, settings);
? isReviewTypeWorkflowStep
? resolveValidatorFallbackThinkingLevel(workflowStepThinkingSource, settings)
: resolveExecutorFallbackThinkingLevel(workflowStepThinkingSource, settings)
: isReviewTypeWorkflowStep
? resolveValidatorThinkingLevel(workflowStepThinkingSource, settings)
: resolveExecutorThinkingLevel(workflowStepThinkingSource, settings);
const workflowStepFallbackThinkingLevel = isReviewTypeWorkflowStep
? resolveValidatorFallbackThinkingLevel(workflowStepThinkingSource, settings)
: resolveExecutorFallbackThinkingLevel(workflowStepThinkingSource, settings);
const { session } = await createResolvedAgentSession({
sessionPurpose: "executor",
runtimeHint: workflowRuntimeHint,
@@ -17719,8 +17739,8 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
tools: toolMode,
defaultProvider: provider,
defaultModelId: modelId,
fallbackProvider: executorFallback.provider,
fallbackModelId: executorFallback.modelId,
fallbackProvider: workflowFallback.provider,
fallbackModelId: workflowFallback.modelId,
fallbackThinkingLevel: workflowStepFallbackThinkingLevel,
defaultThinkingLevel: workflowStepThinkingLevel,
runAuditor: createRunAuditor(this.store, this.getRunContextFor(task.id)),
@@ -17970,7 +17990,7 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
if (!retryMalformed) return retryOutcome;
await this.store.logEntry(
task.id,
`Workflow step '${workflowStep.name}' produced malformed output on both the primary attempt and one self-retry — no fallback model configured (set settings.executionFallbackProvider/Id or fallbackProvider/Id)`,
`Workflow step '${workflowStep.name}' produced malformed output on both the primary attempt and one self-retry — no fallback model configured (set ${fallbackSettingsHint})`,
);
return retryOutcome;
}
@@ -17978,12 +17998,12 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
executorLog.warn(`${task.id}: workflow step '${workflowStep.name}' ${reason} and no fallback model is configured`);
await this.store.logEntry(
task.id,
`Workflow step '${workflowStep.name}' ${reason} — no fallback model configured (set settings.executionFallbackProvider/Id or fallbackProvider/Id)`,
`Workflow step '${workflowStep.name}' ${reason} — no fallback model configured (set ${fallbackSettingsHint})`,
);
return primaryOutcome;
}
executorLog.log(`${task.id}: retrying workflow step '${workflowStep.name}' with executor fallback ${fallback.provider}/${fallback.modelId} after primary ${primaryOutcome.timedOut ? "timeout" : "malformed output"}`);
executorLog.log(`${task.id}: retrying workflow step '${workflowStep.name}' with ${fallbackLaneLabel} fallback ${fallback.provider}/${fallback.modelId} after primary ${primaryOutcome.timedOut ? "timeout" : "malformed output"}`);
return runOnce(fallback.provider, fallback.modelId, "fallback");
}