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

fix(workflow): raise step review timeout default

Browse Source
This commit is contained in:
gsxdsm
2026-07-01 08:11:18 -07:00
parent e4eb8b6190
commit 62c58402ff
14 changed files with 54 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/workflow-step-timeout-default.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Give workflow review steps a longer default timeout.
category: fix
dev: Raises the built-in `workflowStepTimeoutMs` default and engine fallback from 6 minutes to 15 minutes.

1
docs/settings-reference.md
View File

@@ -505,6 +505,7 @@ Default notes:
| `validatorModelId` | `string` | `undefined` | Model ID for plan/code reviewers. |
| `validatorFallbackProvider` | `string` | `undefined` | Fallback provider for reviewers; also used by reviewer UNAVAILABLE/error recovery retry before returning terminal UNAVAILABLE. |
| `validatorFallbackModelId` | `string` | `undefined` | Fallback model ID for reviewers; paired with `validatorFallbackProvider` for reviewer recovery retry. |
| `workflowStepTimeoutMs` | `number` | `900000` | Maximum time in milliseconds a single workflow step may run before it is timed out. |
| `modelPresets` | `ModelPreset[]` | `[]` | Reusable executor/reviewer model presets. |
| `autoSelectModelPreset` | `boolean` | `false` | Auto-select presets by task size. |
| `defaultPresetBySize` | `{ S?: string; M?: string; L?: string }` | `{}` | Mapping for `S`/`M`/`L` → preset ID. |

2
packages/core/src/__tests__/effective-settings-overlay.test.ts
View File

@@ -25,7 +25,7 @@ describe("applyWorkflowSettingsOverlay", () => {
planningFallbackModelId: "workflow-planner-fallback-model",
validatorFallbackProvider: "workflow-validator-fallback",
validatorFallbackModelId: "workflow-validator-fallback-model",
workflowStepTimeoutMs: 360_000,
workflowStepTimeoutMs: 900_000,
runStepsInNewSessions: false,
maxParallelSteps: undefined,
},

6
packages/core/src/__tests__/settings-migration.test.ts
View File

@@ -176,8 +176,8 @@ describe("settings hard-move migration (U4)", () => {
expect((PROJECT_SETTINGS_KEYS as readonly string[]).includes(key)).toBe(false);
}
const effective = await resolveEffectiveSettingsById(resolverStore(store), "builtin:coding", store.getWorkflowSettingsProjectId());
// Declaration defaults: workflowStepTimeoutMs=360000, requirePrApproval=false.
expect(effective.workflowStepTimeoutMs).toBe(360_000);
// Declaration defaults: workflowStepTimeoutMs=900000, requirePrApproval=false.
expect(effective.workflowStepTimeoutMs).toBe(900_000);
expect(effective.requirePrApproval).toBe(false);
});
@@ -233,7 +233,7 @@ describe("settings hard-move migration (U4)", () => {
],
edges: [{ from: "start", to: "end" }],
settings: [
{ id: "workflowStepTimeoutMs", name: "Step timeout", type: "number", default: 360_000 },
{ id: "workflowStepTimeoutMs", name: "Step timeout", type: "number", default: 900_000 },
{ id: "requirePrApproval", name: "Require PR approval", type: "boolean", default: false },
],
},

14
packages/core/src/__tests__/workflow-ir-settings.test.ts
View File

@@ -6,6 +6,7 @@ import {
WorkflowIrError,
} from "../workflow-ir.js";
import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js";
import { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "../builtin-stepwise-final-review-coding-workflow-ir.js";
import { getBuiltinWorkflow } from "../builtin-workflows.js";
import {
BUILTIN_MOVED_WORKFLOW_SETTINGS,
@@ -218,11 +219,12 @@ describe("built-in workflow settings parity anchor (U1, R4)", () => {
expect(declaredIds.has(setting.id)).toBe(true);
}
expect(builtin.settings).toEqual(BUILTIN_WORKFLOW_SETTINGS);
expect(getBuiltinWorkflow("builtin:coding")!.ir).toBe(BUILTIN_CODING_WORKFLOW_IR);
expect(getBuiltinWorkflow("builtin:coding")!.ir).toBe(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR);
expect(getBuiltinWorkflow("builtin:legacy-coding")!.ir).toBe(BUILTIN_CODING_WORKFLOW_IR);
expect((getBuiltinWorkflow("builtin:coding")!.ir as WorkflowIrV2).settings).toEqual(BUILTIN_WORKFLOW_SETTINGS);
});
it("the moved-key catalog has left DEFAULT_PROJECT_SETTINGS (U4 hard-move) and pins its legacy defaults", () => {
it("the moved-key catalog has left DEFAULT_PROJECT_SETTINGS (U4 hard-move) and pins current built-in defaults", () => {
const legacy = DEFAULT_PROJECT_SETTINGS as Record<string, unknown>;
// Post-U4 hard-move: every catalog key has been REMOVED from
// DEFAULT_PROJECT_SETTINGS (the type-vs-schema split keeps the type field but
@@ -230,11 +232,11 @@ describe("built-in workflow settings parity anchor (U1, R4)", () => {
for (const setting of BUILTIN_MOVED_WORKFLOW_SETTINGS) {
expect(Object.prototype.hasOwnProperty.call(legacy, setting.id)).toBe(false);
}
// The declaration defaults are now the single source of truth; pin the legacy
// values explicitly so they can never silently drift from what they were when
// they lived in DEFAULT_PROJECT_SETTINGS.
// The declaration defaults are now the single source of truth; pin current
// built-in values explicitly so they can never silently drift from engine
// read-site fallbacks or workflow editor display.
const expectedDefaults: Record<string, unknown> = {
workflowStepTimeoutMs: 360_000,
workflowStepTimeoutMs: 900_000,
workflowStepScopeEnforcement: "block",
planOnlyScopeLeakEnforcement: "warn",
workflowRevisionForkOnScopeMismatch: true,

2
packages/core/src/__tests__/workflow-settings-e2e.test.ts
View File

@@ -252,7 +252,7 @@ describe("workflow-settings end-to-end journey (U10)", () => {
// The fresh project has declaration defaults (NOT the source project's values).
const freshBefore = await resolveEffectiveSettingsById(resolverStore(store2), "builtin:coding", projectId2);
expect(freshBefore.workflowStepTimeoutMs).toBe(360_000); // legacy/declaration default
expect(freshBefore.workflowStepTimeoutMs).toBe(900_000); // declaration default
expect(freshBefore.requirePrApproval).toBe(false);
// ── Import the v2 export → effective values match the exported project,

10
packages/core/src/__tests__/workflow-settings-resolver.test.ts
View File

@@ -227,7 +227,7 @@ describe("resolveEffectiveSettings (per-task)", () => {
it("no selection → builtin:coding declaration defaults (never throws)", async () => {
const store = makeStore({ selection: {} });
const eff = await resolveEffectiveSettings(store, { id: "t-none" });
expect(eff.workflowStepTimeoutMs).toBe(360_000);
expect(eff.workflowStepTimeoutMs).toBe(900_000);
});
it("missing custom definition degrades to builtin declarations (never throws)", async () => {
@@ -237,13 +237,13 @@ describe("resolveEffectiveSettings (per-task)", () => {
});
const eff = await resolveEffectiveSettings(store, { id: "t1" });
// Degrades to BUILTIN_CODING_WORKFLOW_IR declarations.
expect(eff.workflowStepTimeoutMs).toBe(360_000);
expect(eff.workflowStepTimeoutMs).toBe(900_000);
});
it("selection lookup throwing degrades to builtin declarations", async () => {
const store = makeStore({ selectionThrows: true });
const eff = await resolveEffectiveSettings(store, { id: "t1" });
expect(eff.workflowStepTimeoutMs).toBe(360_000);
expect(eff.workflowStepTimeoutMs).toBe(900_000);
});
it("store value read throwing degrades to declaration defaults", async () => {
@@ -252,7 +252,7 @@ describe("resolveEffectiveSettings (per-task)", () => {
valuesThrows: true,
});
const eff = await resolveEffectiveSettings(store, { id: "t1" });
expect(eff.workflowStepTimeoutMs).toBe(360_000);
expect(eff.workflowStepTimeoutMs).toBe(900_000);
});
it("project-id lookup throwing degrades to declaration defaults (empty stored map)", async () => {
@@ -263,7 +263,7 @@ describe("resolveEffectiveSettings (per-task)", () => {
});
const eff = await resolveEffectiveSettings(store, { id: "t1" });
// The stored 5_000 is unreachable because the project key couldn't be resolved.
expect(eff.workflowStepTimeoutMs).toBe(360_000);
expect(eff.workflowStepTimeoutMs).toBe(900_000);
});
});

10
packages/core/src/__tests__/workflow-settings.test.ts
View File

@@ -33,7 +33,7 @@ const TIMEOUT_DECL: WorkflowSettingDefinition = {
id: "workflowStepTimeoutMs",
name: "Step timeout (ms)",
type: "number",
default: 360_000,
default: 900_000,
};
const FLAG_DECL: WorkflowSettingDefinition = {
id: "runStepsInNewSessions",
@@ -131,7 +131,7 @@ describe("resolveEffectiveSettingValues", () => {
it("falls to the declaration default when unset", () => {
const eff = resolveEffectiveSettingValues([TIMEOUT_DECL], {});
expect(eff).toEqual({ workflowStepTimeoutMs: 360_000 });
expect(eff).toEqual({ workflowStepTimeoutMs: 900_000 });
});
it("drops a stored value that no longer validates (enum→number retype) and uses the default", () => {
@@ -143,7 +143,7 @@ describe("resolveEffectiveSettingValues", () => {
it("drops stored values for ids with no current declaration", () => {
const eff = resolveEffectiveSettingValues([TIMEOUT_DECL], { removedSetting: 7 });
expect(eff).toEqual({ workflowStepTimeoutMs: 360_000 });
expect(eff).toEqual({ workflowStepTimeoutMs: 900_000 });
});
it("omits a setting with neither a valid value nor a default", () => {
@@ -248,7 +248,7 @@ describe("TaskStore.updateWorkflowSettingValues", () => {
const def = await store.getWorkflowDefinition(wfId);
const decls = def!.ir.version === "v2" ? def!.ir.settings : undefined;
expect(resolveEffectiveSettingValues(decls, stored)).toEqual({ workflowStepTimeoutMs: 360_000 });
expect(resolveEffectiveSettingValues(decls, stored)).toEqual({ workflowStepTimeoutMs: 900_000 });
});
it("retype enum→number with a stale stored string: effective resolution drops it, returns default, stored row untouched", async () => {
@@ -305,6 +305,6 @@ describe("TaskStore.updateWorkflowSettingValues", () => {
);
expect(effective.requirePrApproval).toBe(true);
// Untouched built-in keys resolve to their declaration defaults.
expect(effective.workflowStepTimeoutMs).toBe(360_000);
expect(effective.workflowStepTimeoutMs).toBe(900_000);
});
});

20
packages/core/src/builtin-workflow-settings.ts
View File

@@ -6,7 +6,8 @@ import type { WorkflowSettingDefinition } from "./workflow-ir-types.js";
*
* `BUILTIN_MOVED_WORKFLOW_SETTINGS` is the U4 moved-key catalog: keys that
* formerly lived in `DEFAULT_PROJECT_SETTINGS` and are tombstoned by
* `MOVED_SETTINGS_KEYS`. Keep those defaults byte-equal to the legacy literals.
* `MOVED_SETTINGS_KEYS`. Defaults should stay aligned with engine fallback
* literals; intentional product-default changes must update both together.
*
* `BUILTIN_TRIAGE_POLICY_SETTINGS` is workflow-native triage/spec policy. These
* keys never lived in `DEFAULT_PROJECT_SETTINGS`, are NOT part of the U4
@@ -29,12 +30,11 @@ import type { WorkflowSettingDefinition } from "./workflow-ir-types.js";
* (`builtin-coding-workflow-ir.ts`, `builtin-stepwise-coding-workflow-ir.ts`) so
* the catalog has exactly one definition.
*
* Each `default` here MUST be byte-equal to the corresponding literal in
* `DEFAULT_PROJECT_SETTINGS` (`settings-schema.ts`) — this is the parity anchor
* for the U4 hard-move migration. The U1 test
* (`workflow-ir-settings.test.ts`) asserts strict equality against the legacy
* literals. Keys with `undefined` legacy defaults (the per-phase model lanes)
* omit `default` entirely, which round-trips to the same effective value.
* Each `default` here must stay aligned with the corresponding engine read-site
* fallback literal so projects without stored workflow values execute with the
* same policy the Workflow Editor displays. Keys with `undefined` legacy
* defaults (the per-phase model lanes) omit `default` entirely, which
* round-trips to the same effective value.
*
* NOTE: these declarations are inert in U1 — nothing reads them until the
* effective-settings resolver and engine integration land (U3). Adding them does
@@ -51,7 +51,11 @@ export const BUILTIN_MOVED_WORKFLOW_SETTINGS: WorkflowSettingDefinition[] = [
id: "workflowStepTimeoutMs",
name: "Step timeout (ms)",
type: "number",
default: 360_000,
/*
* FNXC:WorkflowReview 2026-07-01-08:09:
* Code Review steps can exceed the old six-minute default on ordinary dashboard changes. Use a fifteen-minute workflow-step default so every project without an override gets enough reviewer time while keeping runaway sessions bounded.
*/
default: 900_000,
description: "Maximum time a single workflow step may run before it is timed out.",
},
{

2
packages/core/src/types.ts
View File

@@ -3945,7 +3945,7 @@ export interface ProjectSettings {
/** Wall-clock timeout (ms) for a single pre-merge workflow step's AI call.
* When a step exceeds this, the session is aborted and the executor is
* given one shot to retry with the configured fallback model before the
* step is reported as failed. Default: 360_000 (6 minutes). */
* step is reported as failed. Default: 900_000 (15 minutes). */
workflowStepTimeoutMs?: number;
/** How pre-merge prompt workflow steps enforce declared File Scope at step end.
* - "block" (default): mark the step failed/revision-requested on off-scope writes

2
packages/dashboard/src/__tests__/task-effective-settings-route.test.ts
View File

@@ -17,7 +17,7 @@ class MockStore extends EventEmitter {
getSettingsFast = vi.fn(async (): Promise<Settings> => ({
defaultProvider: "base-default-provider",
defaultModelId: "base-default-model",
workflowStepTimeoutMs: 360_000,
workflowStepTimeoutMs: 900_000,
runStepsInNewSessions: false,
} as Settings));
getTaskWorkflowSelection = vi.fn((taskId: string) => this.workflowSelections.get(taskId));

14
packages/engine/src/__tests__/effective-settings-merge.test.ts
View File

@@ -8,7 +8,7 @@ const PROJECT = "proj-1";
/** A base settings object with real project values for the keys under test. */
function baseSettings(): Settings {
return {
workflowStepTimeoutMs: 360_000,
workflowStepTimeoutMs: 900_000,
requirePrApproval: false,
runStepsInNewSessions: false,
// A real project value for an absent-default lane — must NOT be clobbered.
@@ -36,8 +36,8 @@ describe("mergeEffectiveSettings (engine entry merge, U3/KTD-3)", () => {
const store = makeStore({ workflowId: "builtin:coding" });
const base = baseSettings();
const merged = await mergeEffectiveSettings(store as any, { id: "t1" }, base);
// Declaration defaults are byte-equal to legacy defaults, so these don't change.
expect(merged.workflowStepTimeoutMs).toBe(360_000);
// Declaration defaults match the engine read-site fallback, so these don't change.
expect(merged.workflowStepTimeoutMs).toBe(900_000);
expect(merged.requirePrApproval).toBe(false);
expect(merged.runStepsInNewSessions).toBe(false);
});
@@ -58,7 +58,7 @@ describe("mergeEffectiveSettings (engine entry merge, U3/KTD-3)", () => {
const base = { ...baseSettings(), verificationFixRetries: 0, workflowStepTimeoutMs: 12_345 } as unknown as Settings;
const merged = await mergeEffectiveSettings(store as any, { id: "t1" }, base);
expect(merged.verificationFixRetries).toBe(0); // not the declaration default (3)
expect(merged.workflowStepTimeoutMs).toBe(12_345); // not the declaration default (360_000)
expect(merged.workflowStepTimeoutMs).toBe(12_345); // not the declaration default (900_000)
});
it("post-migration fill: a declaration default fills when the base lacks the key", async () => {
@@ -66,7 +66,7 @@ describe("mergeEffectiveSettings (engine entry merge, U3/KTD-3)", () => {
// Base lacks workflowStepTimeoutMs (moved key removed from project settings).
const base = { requirePrApproval: false } as unknown as Settings;
const merged = await mergeEffectiveSettings(store as any, { id: "t1" }, base);
expect(merged.workflowStepTimeoutMs).toBe(360_000); // filled from declaration default
expect(merged.workflowStepTimeoutMs).toBe(900_000); // filled from declaration default
});
it("a stored value overrides the base", async () => {
@@ -87,7 +87,7 @@ describe("mergeEffectiveSettings (engine entry merge, U3/KTD-3)", () => {
const store = makeStore({ workflowId: "builtin:coding", values: { workflowStepTimeoutMs: 1 } });
const base = baseSettings();
const merged = await mergeEffectiveSettings(store as any, { id: "t1" }, base);
expect(base.workflowStepTimeoutMs).toBe(360_000);
expect(base.workflowStepTimeoutMs).toBe(900_000);
expect(merged).not.toBe(base);
});
@@ -110,7 +110,7 @@ describe("mergeEffectiveSettings (engine entry merge, U3/KTD-3)", () => {
const merged = await mergeEffectiveSettings(store as any, { id: "t1" }, base);
// resolveEffectiveSettings degrades to builtin declaration defaults even when the
// selection/project throw; the merge stays behavior-inert for the base values.
expect(merged.workflowStepTimeoutMs).toBe(360_000);
expect(merged.workflowStepTimeoutMs).toBe(900_000);
expect(merged.executionProvider).toBe("project-anthropic");
});
});

2
packages/engine/src/__tests__/workflow-settings-fallback-alignment.test.ts
View File

@@ -85,7 +85,7 @@ describe("workflow-settings fallback alignment (KTD-3, item 4)", () => {
// VALUES here are the audited read-site literals; the assertion ties each to
// the declaration default so the table cannot silently drift.
const audited: Record<string, unknown> = {
workflowStepTimeoutMs: 360_000, // executor.ts: ?? 360_000
workflowStepTimeoutMs: 900_000, // executor.ts: ?? 900_000
workflowStepScopeEnforcement: "block", // executor.ts: ?? "block"
planOnlyScopeLeakEnforcement: "warn", // executor.ts: ?? "warn"
workflowRevisionForkOnScopeMismatch: true, // executor.ts: !== false (default-true)

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

@@ -14351,7 +14351,7 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
(c) => c.provider && c.modelId && (c.provider !== primaryProvider || c.modelId !== primaryModelId),
);
const timeoutMs = Math.max(60_000, settings.workflowStepTimeoutMs ?? 360_000);
const timeoutMs = Math.max(60_000, settings.workflowStepTimeoutMs ?? 900_000);
const runOnce = async (
provider: string | undefined,