fix(engine): make merge/step verification timeout scope-aware (#1771)
## Problem Fusion's verification gate (the merger, and the executor's per-step auto-gate) runs the project's configured `testCommand`/`buildCommand` bounded by a **flat 10-minute** default (`VERIFICATION_COMMAND_TIMEOUT_MS = 600_000`) when no `verificationCommandTimeoutMs` override is set. A **workspace-scoped** command (a full suite that legitimately takes ~10+ min) hits that wall and is killed as an **infra `timedOut`** — blocking the merge — even though nothing is actually hung. Meanwhile a **package-scoped** command got a too-generous bound. The `fn_run_verification` tool already derived its default from scope (300s/900s); the merger/executor did not. ## Change Make the shared `runVerificationCommand` (used by both the merger and the executor auto-gate) **scope-aware**, mirroring the tool: - **package-scoped** (`pnpm --filter`/`-F …`) → **300s** - **workspace-scoped** (root command like `pnpm test`) → **900s** An explicit project `verificationCommandTimeoutMs` still overrides, and the 30-min hard cap (`VERIFICATION_COMMAND_HARD_CAP_MS`) still clamps. New `classifyVerificationScope` / `defaultVerificationTimeoutMs` helpers mirror `run-verification-tool`'s `DEFAULT_TIMEOUT_PACKAGE_SEC` (300) / `DEFAULT_TIMEOUT_WORKSPACE_SEC` (900). ## Verification - `verification-utils.test.ts` (new scope-classification + default cases), `run-verification-command.test.ts`, `merger-verification.test.ts` — **143 tests pass**. - Lint clean. `patch` changeset added. Note: a workspace command needing >900s should be **scoped** (FN-5048 / the bounded-verification guidance), or set `verificationCommandTimeoutMs` explicitly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- stage-review-badge-begin --> --- <a href="https://stagereview.app/Runfusion/Fusion/pull/1771"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://stagereview.app/assets/gh-open-in-stage-dark.svg"> <img src="https://stagereview.app/assets/gh-open-in-stage-light.svg" alt="Open in Stage"> </picture> </a> <!-- stage-review-badge-end --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Verification (merge/step gate) timeouts are now scope-aware: package-scoped commands default to 300s and workspace-scoped commands default to 900s. * If a custom timeout is provided, it still overrides the default, while the safety hard cap remains enforced. * **Bug Fixes** * Prevents verification jobs from using an overly generic fixed timeout, reducing unnecessary early timeouts or excessive waits. * **Tests** * Expanded coverage to validate scope detection and the new default timeout behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
7
.changeset/scope-aware-verification-timeout.md
Normal file
7
.changeset/scope-aware-verification-timeout.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Verification (merge/step gate) timeout now scales with command scope instead of a flat 10 minutes.
|
||||
category: fix
|
||||
dev: verification-utils runVerificationCommand derives its default from the command — package-scoped (pnpm --filter/-F) gets 300s, workspace-scoped gets 900s — matching fn_run_verification (DEFAULT_TIMEOUT_PACKAGE_SEC/WORKSPACE_SEC). Project verificationCommandTimeoutMs still overrides; the 1800s hard cap still applies. Fixes workspace-scoped suites being killed as a 10-min infra timeout during merge/step verification.
|
||||
@@ -10,9 +10,9 @@ import {
|
||||
} from "../sandbox/index.js";
|
||||
import { defaultShell } from "../shell-utils.js";
|
||||
import {
|
||||
defaultVerificationTimeoutMs,
|
||||
runVerificationCommand,
|
||||
VERIFICATION_COMMAND_MAX_BUFFER,
|
||||
VERIFICATION_COMMAND_TIMEOUT_MS,
|
||||
} from "../verification-utils.js";
|
||||
|
||||
function makeStub(overrides: Partial<SandboxBackend> = {}): SandboxBackend {
|
||||
@@ -159,7 +159,9 @@ describe("sandbox wiring", () => {
|
||||
expect(runStreaming).toHaveBeenCalledTimes(1);
|
||||
expect(runStreaming).toHaveBeenCalledWith("echo ok", {
|
||||
cwd: "/tmp/project",
|
||||
timeout: VERIFICATION_COMMAND_TIMEOUT_MS,
|
||||
// FNXC:Verification 2026-06-25-14:05: the default budget is now scope-aware;
|
||||
// "echo ok" has no pnpm --filter so it resolves to the workspace default (900s).
|
||||
timeout: defaultVerificationTimeoutMs("echo ok"),
|
||||
maxBuffer: VERIFICATION_COMMAND_MAX_BUFFER,
|
||||
signal: undefined,
|
||||
env: { FOO: "1" },
|
||||
|
||||
@@ -3,11 +3,39 @@ import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { setTimeout as delay } from "node:timers/promises";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { detectMissingWorkspaceEntry, execWithProcessGroup } from "../verification-utils.js";
|
||||
import {
|
||||
classifyVerificationScope,
|
||||
defaultVerificationTimeoutMs,
|
||||
detectMissingWorkspaceEntry,
|
||||
execWithProcessGroup,
|
||||
VERIFICATION_TIMEOUT_PACKAGE_MS,
|
||||
VERIFICATION_TIMEOUT_WORKSPACE_MS,
|
||||
} from "../verification-utils.js";
|
||||
|
||||
const onPosix = process.platform !== "win32";
|
||||
const itPosix = onPosix ? it : it.skip;
|
||||
|
||||
describe("scope-aware verification default timeout", () => {
|
||||
it("classifies pnpm --filter / -F commands as package-scoped", () => {
|
||||
expect(classifyVerificationScope("pnpm --filter @fusion/dashboard test")).toBe("package");
|
||||
expect(classifyVerificationScope("pnpm -w --filter @fusion/core exec vitest run src/a.test.ts")).toBe("package");
|
||||
expect(classifyVerificationScope("pnpm -F @runfusion/fusion test")).toBe("package");
|
||||
});
|
||||
|
||||
it("classifies root-level commands as workspace-scoped", () => {
|
||||
expect(classifyVerificationScope("pnpm test")).toBe("workspace");
|
||||
expect(classifyVerificationScope("pnpm test:full")).toBe("workspace");
|
||||
expect(classifyVerificationScope("npm run verify")).toBe("workspace");
|
||||
});
|
||||
|
||||
it("derives the default budget from scope (package 300s, workspace 900s)", () => {
|
||||
expect(defaultVerificationTimeoutMs("pnpm --filter @fusion/dashboard test")).toBe(VERIFICATION_TIMEOUT_PACKAGE_MS);
|
||||
expect(VERIFICATION_TIMEOUT_PACKAGE_MS).toBe(300_000);
|
||||
expect(defaultVerificationTimeoutMs("pnpm test")).toBe(VERIFICATION_TIMEOUT_WORKSPACE_MS);
|
||||
expect(VERIFICATION_TIMEOUT_WORKSPACE_MS).toBe(900_000);
|
||||
});
|
||||
});
|
||||
|
||||
describe("execWithProcessGroup", { timeout: 10_000 }, () => {
|
||||
let tempDir: string;
|
||||
|
||||
|
||||
@@ -9,10 +9,50 @@ import type { SandboxBackend, SandboxRunStreamingOptions, SandboxStreamingResult
|
||||
// ── Constants ──────────────────────────────────────────────────────────
|
||||
|
||||
export const VERIFICATION_COMMAND_MAX_BUFFER = 50 * 1024 * 1024;
|
||||
/**
|
||||
* Legacy flat default. Retained for back-compat; the merger/executor gate now
|
||||
* derives its default from command scope (see VERIFICATION_TIMEOUT_*_MS below).
|
||||
*/
|
||||
export const VERIFICATION_COMMAND_TIMEOUT_MS = 600_000;
|
||||
export const VERIFICATION_COMMAND_HARD_CAP_MS = 1_800_000;
|
||||
export const VERIFICATION_LOG_MAX_CHARS = 20_000;
|
||||
|
||||
/*
|
||||
FNXC:Verification 2026-06-25-13:55:
|
||||
The merger/executor verification gate used a flat 10-min default for ANY configured
|
||||
test/build command, while the fn_run_verification tool already derived its default
|
||||
from command scope. A workspace-scoped command (a full suite, ~10+ min) hit the flat
|
||||
10-min wall and was killed as an infra timeout; a package-scoped command got a too-
|
||||
generous bound. Make the shared runner scope-aware to match the tool: a package-scoped
|
||||
command (pnpm --filter/-F ...) defaults to 300s, anything else (root/workspace command)
|
||||
to 900s. An explicit project verificationCommandTimeoutMs still overrides, and the 30-min
|
||||
hard cap (VERIFICATION_COMMAND_HARD_CAP_MS) still clamps the result. These mirror
|
||||
run-verification-tool's DEFAULT_TIMEOUT_PACKAGE_SEC (300) / DEFAULT_TIMEOUT_WORKSPACE_SEC (900).
|
||||
*/
|
||||
export const VERIFICATION_TIMEOUT_PACKAGE_MS = 300_000;
|
||||
export const VERIFICATION_TIMEOUT_WORKSPACE_MS = 900_000;
|
||||
|
||||
/**
|
||||
* Classify a configured verification command by scope. A command that targets a
|
||||
* single workspace package via pnpm's `--filter`/`-F` is "package"-scoped; every
|
||||
* other shape (a root-level command such as `pnpm test`) is "workspace"-scoped.
|
||||
*/
|
||||
export function classifyVerificationScope(command: string): "package" | "workspace" {
|
||||
const tokens = command.split(/\s+/).filter(Boolean);
|
||||
return tokens.some((token) => token === "--filter" || token === "-F") ? "package" : "workspace";
|
||||
}
|
||||
|
||||
/**
|
||||
* The default per-command verification budget for a command when no explicit
|
||||
* project `verificationCommandTimeoutMs` override is provided — scope-aware,
|
||||
* matching the fn_run_verification tool.
|
||||
*/
|
||||
export function defaultVerificationTimeoutMs(command: string): number {
|
||||
return classifyVerificationScope(command) === "package"
|
||||
? VERIFICATION_TIMEOUT_PACKAGE_MS
|
||||
: VERIFICATION_TIMEOUT_WORKSPACE_MS;
|
||||
}
|
||||
|
||||
// ── Types ──────────────────────────────────────────────────────────────
|
||||
|
||||
/** Result of running a single verification command */
|
||||
@@ -359,10 +399,12 @@ export async function runVerificationCommand(
|
||||
/*
|
||||
* FNXC:Verification 2026-06-17-14:38:
|
||||
* Configured test/build commands share the same project verification budget as fn_run_verification so merge/step verification cannot run marathon subprocesses outside the engine-level guardrail.
|
||||
* FNXC:Verification 2026-06-25-13:55:
|
||||
* The default is now scope-aware (defaultVerificationTimeoutMs): package-scoped commands get 300s, workspace-scoped 900s — matching the tool — instead of a flat 10-min budget that killed workspace-scoped suites as infra timeouts.
|
||||
*/
|
||||
const rawTimeoutMs = typeof timeoutMsOverride === "number" && timeoutMsOverride > 0
|
||||
? timeoutMsOverride
|
||||
: VERIFICATION_COMMAND_TIMEOUT_MS;
|
||||
: defaultVerificationTimeoutMs(command);
|
||||
const timeoutMs = Math.min(rawTimeoutMs, VERIFICATION_COMMAND_HARD_CAP_MS);
|
||||
try {
|
||||
const { stdout, stderr, bufferOverflow } = await execWithProcessGroup(
|
||||
|
||||
Reference in New Issue
Block a user