From b6ac5f2e51bd362bfe45884d02193fa72d59e721 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 18 Jun 2026 01:27:09 -0700 Subject: [PATCH] FN-6608: bound engine verification runs Add durable engine-level guardrails for verification command timeouts. - Add project-level verificationCommandTimeoutMs settings plumbing and docs. - Enforce configured verification budgets and hard caps in executor and merger verification paths. - Detect marathon verification commands, soft-cap them by default, and require allowFullSuite for explicit full-suite runs. - Cover timeout defaults, marathon detection, and guidance updates with engine/core tests. Files changed: .changeset/fn-6608-verification-bound.md | 5 + docs/settings-reference.md | 1 + docs/testing.md | 2 + .../src/__tests__/settings-consistency.test.ts | 5 + packages/core/src/agent-prompts.ts | 6 +- packages/core/src/settings-schema.ts | 7 +- packages/core/src/types.ts | 6 + .../engine/src/__tests__/executor-core.test.ts | 3 + .../src/__tests__/run-verification-command.test.ts | 176 ++++++++++++++++++++- packages/engine/src/executor.ts | 11 +- packages/engine/src/merger.ts | 9 +- packages/engine/src/run-verification-tool.ts | 142 +++++++++++++++-- packages/engine/src/verification-utils.ts | 13 +- 13 files changed, 360 insertions(+), 26 deletions(-) Fusion-Task-Id: FN-6608 Fusion-Task-Lineage: c593a96d-eb8b-492c-82c7-8943c239f588 --- .changeset/fn-6608-verification-bound.md | 5 + docs/settings-reference.md | 1 + docs/testing.md | 2 + .../__tests__/settings-consistency.test.ts | 5 + packages/core/src/agent-prompts.ts | 6 +- packages/core/src/settings-schema.ts | 7 +- packages/core/src/types.ts | 6 + .../src/__tests__/executor-core.test.ts | 3 + .../run-verification-command.test.ts | 176 +++++++++++++++++- packages/engine/src/executor.ts | 11 +- packages/engine/src/merger.ts | 9 +- packages/engine/src/run-verification-tool.ts | 142 ++++++++++++-- packages/engine/src/verification-utils.ts | 13 +- 13 files changed, 360 insertions(+), 26 deletions(-) create mode 100644 .changeset/fn-6608-verification-bound.md diff --git a/.changeset/fn-6608-verification-bound.md b/.changeset/fn-6608-verification-bound.md new file mode 100644 index 0000000000..fd32d7e6b0 --- /dev/null +++ b/.changeset/fn-6608-verification-bound.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": minor +--- + +Add bounded-by-default verification guardrails: project `verificationCommandTimeoutMs`, marathon command detection, and an explicit `allowFullSuite` escape hatch for full verification runs. diff --git a/docs/settings-reference.md b/docs/settings-reference.md index e843ea377e..13727f31bc 100644 --- a/docs/settings-reference.md +++ b/docs/settings-reference.md @@ -435,6 +435,7 @@ Default notes: | `buildRetryCount` | `number` | `0` | Build retry attempts during merge. | | `verificationFixRetries` | `number` | `3` | In-merge auto-fix retry attempts after deterministic test/build verification failures (0-3). | | `buildTimeoutMs` | `number` | `300000` | Build timeout in milliseconds (5 minutes). | +| `verificationCommandTimeoutMs` | `number` | `undefined` | Optional project-scoped default timeout in milliseconds for executor `fn_run_verification` and configured deterministic test/build verification commands. When unset, `fn_run_verification` keeps its scope defaults (300s package, 900s workspace); when set to a positive value, it overrides both scope defaults while all verification still respects the 1800s hard cap. Set `0` or leave unset to use the legacy scope defaults. Marathon command shapes (`pnpm test`, `pnpm test:full`, `pnpm verify:workspace`, whole-package tests without file filters, and repeat loops) are soft-capped unless the agent explicitly passes `allowFullSuite: true`; opt-in full-suite runs still emit progress heartbeats and obey the hard cap. Project settings override global/default settings via the normal project settings precedence. | | `requirePlanApproval` | `boolean` | `false` | Require manual approval before planning → todo. | | `ephemeralAgentsEnabled` | `boolean` | `true` | Defaults to `true` for both new projects (seeded into `.fusion/fusion.db` on init) and upgrades from pre-FN-4153 projects (falls back to `true` whenever the persisted `config.settings` row omits the key). Users who explicitly set `false` keep that choice. When enabled, Fusion spawns short-lived `executor-FN-XXXX` workers for task execution. When disabled, only permanent executor agents run tasks; the scheduler auto-assigns dispatchable tasks using reporting-chain-aware load balancing, and tasks stay queued until an eligible permanent executor is available. | | `agentProvisioning` | `{ approvalMode?: "always" \| "trusted-only" \| "never"; trustedRoles?: string[]; trustedAgentIds?: string[]; alwaysApproveDelete?: boolean }` | `{}` | Approval policy for `fn_agent_create`/`fn_agent_delete` (`approvalMode` default `trusted-only`, delete approvals default on via `alwaysApproveDelete: true`). | diff --git a/docs/testing.md b/docs/testing.md index 146c74d43e..aece16f658 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -40,6 +40,8 @@ pnpm verify:workspace # deep opt-in verification: lint -> test:full -> build (N `pnpm test:full` runs each package's default test script with capped worker fanout (`FUSION_TEST_TOTAL_WORKERS=4 FUSION_TEST_CONCURRENCY=2 pnpm -r --workspace-concurrency=2 test`). Do not casually raise worker counts; dashboard/jsdom and integration-heavy packages destabilize when oversubscribed. Use `VITEST_MAX_WORKERS=` only for targeted package-level investigation. +Agents running verification through `fn_run_verification` are bounded by default: project `verificationCommandTimeoutMs` when set, otherwise 300s for package scope and 900s for workspace scope, with an 1800s hard cap. Marathon invocations such as root `pnpm test`, `pnpm test:full`, `pnpm verify:workspace`, whole-package tests without file filters, and shell repeat loops are soft-capped unless the agent explicitly passes `allowFullSuite: true`; the escape hatch still emits progress heartbeats and respects the hard cap. Prefer targeted commands such as `pnpm --filter @fusion/ exec vitest run src/path/to/test.ts --silent=passed-only --reporter=dot` before opting into a full run. + ## Fresh-worktree dist bootstrap `pnpm test` auto-runs `scripts/ensure-test-artifacts.mjs` to rebuild missing/stale dist artifacts. Dashboard and `dependency-graph` package lanes auto-bootstrap too. If you hit opaque `Failed to resolve import "./cli-spawn.js"` (or similar), treat it as bootstrap regression against FN-4605 — don't work around with a manual `pnpm build`. diff --git a/packages/core/src/__tests__/settings-consistency.test.ts b/packages/core/src/__tests__/settings-consistency.test.ts index dc829a494f..0b60df72e3 100644 --- a/packages/core/src/__tests__/settings-consistency.test.ts +++ b/packages/core/src/__tests__/settings-consistency.test.ts @@ -76,6 +76,11 @@ describe("settings consistency (U5)", () => { expect(isGlobalSettingsKey(key), `isGlobalSettingsKey('${key}') must be false`).toBe(false); expect(isProjectSettingsKey(key), `isProjectSettingsKey('${key}') must be false`).toBe(false); } + + expect(projectKeys, "verificationCommandTimeoutMs remains a project setting, not a moved workflow setting").toContain("verificationCommandTimeoutMs"); + expect(DEFAULT_PROJECT_SETTINGS.verificationCommandTimeoutMs).toBeUndefined(); + expect(isProjectSettingsKey("verificationCommandTimeoutMs")).toBe(true); + expect(isGlobalSettingsKey("verificationCommandTimeoutMs")).toBe(false); }); it("(d) settings-export v2 global/project section keys never overlap moved keys", async () => { diff --git a/packages/core/src/agent-prompts.ts b/packages/core/src/agent-prompts.ts index 65bb97aed4..e8c9b42b24 100644 --- a/packages/core/src/agent-prompts.ts +++ b/packages/core/src/agent-prompts.ts @@ -196,10 +196,10 @@ Lint, tests, and typecheck are also hard quality gates: ## Verification commands — use fn_run_verification For ALL test/lint/build/typecheck verification, use the \`fn_run_verification\` tool, NOT raw bash. -The tool prevents your session from being killed by the inactivity watchdog during long compiles. +The tool prevents your session from being killed by the inactivity watchdog during long compiles, and verification is time-bounded by default (project \`verificationCommandTimeoutMs\` when set, otherwise 300s package / 900s workspace, hard-capped at 1800s). -- Prefer **package-scoped** verification first: e.g. \`pnpm --filter @fusion/ test\` with \`scope: "package"\`. This is faster and isolated. -- For file-specific package tests, use direct Vitest execution with package-relative paths: \`pnpm --filter @fusion/ exec vitest run src/path/to/test.ts --silent=passed-only --reporter=dot\`. Do not use \`pnpm --filter @fusion/ test -- --run \`; package test scripts can expand into broad quality suites before the filter is applied. +- Prefer **targeted package-scoped** verification first: use direct Vitest execution with package-relative paths: \`pnpm --filter @fusion/ exec vitest run src/path/to/test.ts --silent=passed-only --reporter=dot\`. Do not use \`pnpm --filter @fusion/ test -- --run \`; package test scripts can expand into broad quality suites before the filter is applied. +- Marathon verification invocations (root \`pnpm test\`, \`pnpm test:full\`, \`pnpm verify:workspace\`, whole-package tests with no file filter, and repeat loops) are soft-capped by default. Use \`allowFullSuite: true\` only when the task explicitly requires a genuinely full run; the run still respects the hard timeout and emits progress heartbeats. - Run **workspace-scoped** verification (\`pnpm test\`, \`pnpm lint\`, \`pnpm build\` from root) only when it is explicitly required by the task/workflow or after impacted/package-scoped checks pass and you are doing final integration. - If you need to run \`pnpm install\` (e.g. you added a new package), use \`fn_run_verification\` with \`scope: "workspace"\` and \`timeoutSec: 600\`. - If a verification command times out, do NOT blindly retry — investigate. Check for hung subprocesses, infinite test loops, or tests waiting on missing dependencies. Use \`node_modules/.modules.yaml\` presence to confirm bootstrap.`; diff --git a/packages/core/src/settings-schema.ts b/packages/core/src/settings-schema.ts index 64b7e4e1ac..764ed87f07 100644 --- a/packages/core/src/settings-schema.ts +++ b/packages/core/src/settings-schema.ts @@ -342,9 +342,12 @@ export const DEFAULT_PROJECT_SETTINGS = { // planOnlyScopeLeakEnforcement, workflowRevisionForkOnScopeMismatch, // strictScopeEnforcement, buildRetryCount, verificationFixRetries, // requirePlanApproval) MOVED to workflow settings (U4) — see - // MOVED_SETTINGS_KEYS. `buildTimeoutMs` is NOT moved (no engine reader) and - // stays a plain project setting: + // MOVED_SETTINGS_KEYS. `buildTimeoutMs` and `verificationCommandTimeoutMs` + // are NOT moved and stay plain project settings. Keep verificationCommandTimeoutMs + // undefined so fn_run_verification preserves legacy per-scope defaults until a + // project opts into a single default budget. buildTimeoutMs: 300_000, + verificationCommandTimeoutMs: undefined, ephemeralAgentsEnabled: true, agentProvisioning: {}, sandboxProvisioning: {}, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index d749ed2f30..36b4d4233f 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -3744,6 +3744,12 @@ export interface ProjectSettings { verificationFixRetries?: number; /** Timeout in milliseconds for build commands during merge. Default: 300000 (5 min). */ buildTimeoutMs?: number; + /** + * FNXC:Verification 2026-06-17-14:20: + * Engine verification commands need a durable project-level budget so marathon test runs abort cleanly instead of tripping the stuck detector and requeueing forever. + * When set, this millisecond value overrides both fn_run_verification scope defaults (package 300s, workspace 900s); when unset, the legacy per-scope defaults still apply. + */ + verificationCommandTimeoutMs?: number; /** When enabled, AI-generated task specifications require manual approval * before the task can move from triage to todo. Tasks with approved specs * remain in triage with status "awaiting-approval" until a user approves diff --git a/packages/engine/src/__tests__/executor-core.test.ts b/packages/engine/src/__tests__/executor-core.test.ts index 72f2289d3d..c83c9a2c91 100644 --- a/packages/engine/src/__tests__/executor-core.test.ts +++ b/packages/engine/src/__tests__/executor-core.test.ts @@ -1232,6 +1232,7 @@ describe("Executor verification gate (FN-3345)", () => { expect.anything(), "executor", expect.any(Object), + undefined, ); // Task should move to in-review expect(store.moveTask).toHaveBeenCalledWith("FN-3345", "in-review"); @@ -1409,6 +1410,7 @@ describe("Executor verification gate (FN-3345)", () => { expect.anything(), "executor", expect.any(Object), + undefined, ); // Third call should be build expect(mockedVerification).toHaveBeenNthCalledWith( @@ -1422,6 +1424,7 @@ describe("Executor verification gate (FN-3345)", () => { expect.anything(), "executor", expect.any(Object), + undefined, ); // Task should move to in-review expect(store.moveTask).toHaveBeenCalledWith("FN-3345", "in-review"); diff --git a/packages/engine/src/__tests__/run-verification-command.test.ts b/packages/engine/src/__tests__/run-verification-command.test.ts index 6322d467ca..2e2441e091 100644 --- a/packages/engine/src/__tests__/run-verification-command.test.ts +++ b/packages/engine/src/__tests__/run-verification-command.test.ts @@ -1,7 +1,16 @@ import { describe, it, expect, vi } from "vitest"; import { tmpdir } from "node:os"; import { fileURLToPath } from "node:url"; -import { createRunVerificationTool, runVerificationCommand, normalizeVerificationCommand, type RunVerificationOptions } from "../run-verification-tool.js"; +import { + BOUNDED_VERIFICATION_GUIDANCE, + MARATHON_SOFT_CAP_SEC, + MAX_TIMEOUT_SEC, + createRunVerificationTool, + detectMarathonVerification, + normalizeVerificationCommand, + runVerificationCommand, + type RunVerificationOptions, +} from "../run-verification-tool.js"; // Some tests use platform-appropriate shell syntax. On Windows, sh-style // quoting and pipes through `printf` are different — these tests are skipped @@ -83,6 +92,171 @@ describe("runVerificationCommand", { timeout: 30000 }, () => { }); }); + describe("marathon verification detection", () => { + it.each([ + ["pnpm test", "root workspace test suite"], + ["pnpm -w test", "root workspace test suite"], + ["pnpm test:full", "full workspace verification script"], + ["pnpm verify:workspace", "full workspace verification script"], + ["pnpm --filter @fusion/core test", "whole-package test script"], + ["for i in $(seq 1 20); do pnpm --filter @fusion/core exec vitest run src/foo.test.ts; done", "shell loop repeats"], + ["while true; do pnpm test; done", "shell loop repeats"], + ["seq 1 20 | xargs -I{} pnpm --filter @fusion/core exec vitest run src/foo.test.ts", "seq/xargs pipeline"], + ["pnpm --filter @fusion/core exec vitest run src/a.test.ts && pnpm --filter @fusion/core exec vitest run src/a.test.ts", "&& chain repeats"], + ])("flags marathon command %s", (command, reason) => { + const detection = detectMarathonVerification(command, "workspace"); + + expect(detection.isMarathon).toBe(true); + expect(detection.reason).toContain(reason); + expect(detection.guidance).toContain("allowFullSuite"); + }); + + it.each([ + "pnpm --filter @fusion/core exec vitest run src/__tests__/settings-consistency.test.ts --silent=passed-only --reporter=dot", + "pnpm --filter @fusion/dashboard test -- --run src/__tests__/routes-tasks.test.ts", + "pnpm lint", + "pnpm build", + ])("passes targeted or non-test command %s", (command) => { + expect(detectMarathonVerification(command, "package").isMarathon).toBe(false); + }); + }); + + describe("tool verification budgets and marathon caps", () => { + it("uses the project verification timeout default when provided", async () => { + const onVerificationStart = vi.fn(); + const tool = createRunVerificationTool({ + worktreePath: tempDir, + rootDir: workspaceRoot, + taskId: "FN-6608", + recordActivity: vi.fn(), + verificationCommandTimeoutMs: 1_500, + onVerificationStart, + onVerificationEnd: vi.fn(), + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + }); + + await tool.execute("call-budget", { command: "exit 0", scope: "workspace" }); + + expect(onVerificationStart).toHaveBeenCalledWith(2_000); + }); + + it("falls back to legacy package/workspace defaults when the setting is absent or disabled", async () => { + const packageStart = vi.fn(); + const disabledWorkspaceStart = vi.fn(); + const packageTool = createRunVerificationTool({ + worktreePath: tempDir, + rootDir: workspaceRoot, + taskId: "FN-6608", + recordActivity: vi.fn(), + onVerificationStart: packageStart, + onVerificationEnd: vi.fn(), + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + }); + const disabledTool = createRunVerificationTool({ + worktreePath: tempDir, + rootDir: workspaceRoot, + taskId: "FN-6608", + recordActivity: vi.fn(), + verificationCommandTimeoutMs: 0, + onVerificationStart: disabledWorkspaceStart, + onVerificationEnd: vi.fn(), + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + }); + + await packageTool.execute("call-package-default", { command: "exit 0", scope: "package" }); + await disabledTool.execute("call-workspace-default", { command: "exit 0", scope: "workspace" }); + + expect(packageStart).toHaveBeenCalledWith(300_000); + expect(disabledWorkspaceStart).toHaveBeenCalledWith(900_000); + }); + + it("applies the hard timeout cap to configured defaults and explicit overrides", async () => { + const configuredStart = vi.fn(); + const explicitStart = vi.fn(); + const configuredTool = createRunVerificationTool({ + worktreePath: tempDir, + rootDir: workspaceRoot, + taskId: "FN-6608", + recordActivity: vi.fn(), + verificationCommandTimeoutMs: (MAX_TIMEOUT_SEC + 60) * 1000, + onVerificationStart: configuredStart, + onVerificationEnd: vi.fn(), + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + }); + const explicitTool = createRunVerificationTool({ + worktreePath: tempDir, + rootDir: workspaceRoot, + taskId: "FN-6608", + recordActivity: vi.fn(), + onVerificationStart: explicitStart, + onVerificationEnd: vi.fn(), + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + }); + + await configuredTool.execute("call-configured-cap", { command: "exit 0", scope: "package" }); + await explicitTool.execute("call-explicit-cap", { command: "exit 0", scope: "package", timeoutSec: MAX_TIMEOUT_SEC + 1 }); + + expect(configuredStart).toHaveBeenCalledWith(MAX_TIMEOUT_SEC * 1000); + expect(explicitStart).toHaveBeenCalledWith(MAX_TIMEOUT_SEC * 1000); + }); + + itPosix("reports an actionable timeout without relying on stuck detection", async () => { + const tool = createRunVerificationTool({ + worktreePath: tempDir, + rootDir: workspaceRoot, + taskId: "FN-6608", + recordActivity: vi.fn(), + onVerificationStart: vi.fn(), + onVerificationEnd: vi.fn(), + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + }); + + const result = await tool.execute("call-timeout", { command: "sh -c 'sleep 10 & wait'", scope: "package", timeoutSec: 1 }); + + const text = result.content[0]?.type === "text" ? result.content[0].text : ""; + expect(result.details).toEqual(expect.objectContaining({ success: false, timedOut: true })); + expect(text).toContain("Command timed out after 1s"); + expect(text).toContain(BOUNDED_VERIFICATION_GUIDANCE); + }); + + itPosix("soft-caps marathon commands unless allowFullSuite is provided", async () => { + const cappedStart = vi.fn(); + const allowedStart = vi.fn(); + const recordActivity = vi.fn(); + const command = "pnpm() { echo pulse; }; pnpm test"; + const cappedTool = createRunVerificationTool({ + worktreePath: tempDir, + rootDir: workspaceRoot, + taskId: "FN-6608", + recordActivity: vi.fn(), + onVerificationStart: cappedStart, + onVerificationEnd: vi.fn(), + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + }); + const allowedTool = createRunVerificationTool({ + worktreePath: tempDir, + rootDir: workspaceRoot, + taskId: "FN-6608", + recordActivity, + onVerificationStart: allowedStart, + onVerificationEnd: vi.fn(), + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + }); + + const capped = await cappedTool.execute("call-capped", { command, scope: "workspace", timeoutSec: 600 }); + const allowed = await allowedTool.execute("call-allowed", { command, scope: "workspace", timeoutSec: 600, allowFullSuite: true }); + + const cappedText = capped.content[0]?.type === "text" ? capped.content[0].text : ""; + const allowedText = allowed.content[0]?.type === "text" ? allowed.content[0].text : ""; + expect(cappedStart).toHaveBeenCalledWith(MARATHON_SOFT_CAP_SEC * 1000); + expect(cappedText).toContain("marathon verification detected"); + expect(allowedStart).toHaveBeenCalledWith(600_000); + expect(allowedText).toContain("allowFullSuite=true acknowledged"); + expect(allowed.details).toEqual(expect.objectContaining({ success: true, timedOut: false })); + expect(recordActivity).toHaveBeenCalled(); + }); + }); + describe("tool verification lifecycle callbacks", () => { it("brackets a successful verification run with start and end callbacks", async () => { const onVerificationStart = vi.fn(); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 95528dc9d3..d606f27544 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -1319,10 +1319,10 @@ Lint, tests, and typecheck are also hard quality gates: ## Verification commands — use fn_run_verification For ALL test/lint/build/typecheck verification, use the \`fn_run_verification\` tool, NOT raw bash. -The tool prevents your session from being killed by the inactivity watchdog during long compiles. +The tool prevents your session from being killed by the inactivity watchdog during long compiles, and verification is time-bounded by default (project \`verificationCommandTimeoutMs\` when set, otherwise 300s package / 900s workspace, hard-capped at 1800s). -- Prefer **package-scoped** verification first: e.g. \`pnpm --filter @fusion/ test\` with \`scope: "package"\`. This is faster and isolated. -- For file-specific package tests, use direct Vitest execution with package-relative paths: \`pnpm --filter @fusion/ exec vitest run src/path/to/test.ts --silent=passed-only --reporter=dot\`. Do not use \`pnpm --filter @fusion/ test -- --run \`; package test scripts can expand into broad quality suites before the filter is applied. +- Prefer **targeted package-scoped** verification first: use direct Vitest execution with package-relative paths: \`pnpm --filter @fusion/ exec vitest run src/path/to/test.ts --silent=passed-only --reporter=dot\`. Do not use \`pnpm --filter @fusion/ test -- --run \`; package test scripts can expand into broad quality suites before the filter is applied. +- Marathon verification invocations (root \`pnpm test\`, \`pnpm test:full\`, \`pnpm verify:workspace\`, whole-package tests with no file filter, and repeat loops) are soft-capped by default. Use \`allowFullSuite: true\` only when the task explicitly requires a genuinely full run; the run still respects the hard timeout and emits progress heartbeats. - Run **workspace-scoped** verification (\`pnpm test\`, \`pnpm lint\`, \`pnpm build\` from root) only when it is explicitly required by the task/workflow or after impacted/package-scoped checks pass and you are doing final integration. - If you need to run \`pnpm install\` (e.g. you added a new package), use \`fn_run_verification\` with \`scope: "workspace"\` and \`timeoutSec: 600\`. - If a verification command times out, do NOT blindly retry — investigate. Check for hung subprocesses, infinite test loops, or tests waiting on missing dependencies. Use \`node_modules/.modules.yaml\` presence to confirm bootstrap. @@ -7730,6 +7730,7 @@ export class TaskExecutor { rootDir: this.rootDir, taskId: task.id, recordActivity: () => stuckDetector?.recordActivity(task.id), + verificationCommandTimeoutMs: settings.verificationCommandTimeoutMs, onVerificationStart: (timeoutMs) => stuckDetector?.beginVerification(task.id, timeoutMs), onVerificationEnd: () => stuckDetector?.endVerification(task.id), log: { @@ -11026,7 +11027,7 @@ ${feedback} // Run test command first if configured if (testCommand) { const testResult = await runVerificationCommand( - this.store, worktreePath, task.id, testCommand, "test", undefined, executorLog, "executor", extraEnv, + this.store, worktreePath, task.id, testCommand, "test", undefined, executorLog, "executor", extraEnv, settings.verificationCommandTimeoutMs, ); result.testResult = testResult; @@ -11041,7 +11042,7 @@ ${feedback} // Run build command second if configured if (buildCommand) { const buildResult = await runVerificationCommand( - this.store, worktreePath, task.id, buildCommand, "build", undefined, executorLog, "executor", extraEnv, + this.store, worktreePath, task.id, buildCommand, "build", undefined, executorLog, "executor", extraEnv, settings.verificationCommandTimeoutMs, ); result.buildResult = buildResult; diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index 925708ed80..e04354fe64 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -1414,6 +1414,8 @@ async function runDeterministicVerification( signal?: AbortSignal, ): Promise { const result: VerificationResult = { allPassed: true }; + const settings = await store.getSettings(); + const verificationCommandTimeoutMs = settings.verificationCommandTimeoutMs; // Nothing to verify if (!testCommand && !buildCommand) { @@ -1540,7 +1542,7 @@ async function runDeterministicVerification( failedCommandLabel: "testCommand" | "buildCommand", ): Promise => { const firstAttempt = await runVerificationCommand( - store, rootDir, taskId, command, type, signal, + store, rootDir, taskId, command, type, signal, verificationCommandTimeoutMs, ); if (firstAttempt.success) { return firstAttempt; @@ -1574,7 +1576,7 @@ async function runDeterministicVerification( } const retryAttempt = await runVerificationCommand( - store, rootDir, taskId, command, type, signal, + store, rootDir, taskId, command, type, signal, verificationCommandTimeoutMs, ); if (retryAttempt.success) { result.environmentFault = { @@ -1687,9 +1689,10 @@ async function runVerificationCommand( command: string, type: "test" | "build", signal?: AbortSignal, + timeoutMsOverride?: number, ): Promise { throwIfAborted(signal, taskId); - return runVerificationCommandShared(store, rootDir, taskId, command, type, signal, mergerLog, "merger", VERIFICATION_EXTRA_ENV); + return runVerificationCommandShared(store, rootDir, taskId, command, type, signal, mergerLog, "merger", VERIFICATION_EXTRA_ENV, timeoutMsOverride); } /** diff --git a/packages/engine/src/run-verification-tool.ts b/packages/engine/src/run-verification-tool.ts index fc27f0beba..61fee13a59 100644 --- a/packages/engine/src/run-verification-tool.ts +++ b/packages/engine/src/run-verification-tool.ts @@ -32,9 +32,13 @@ import { executorLog } from "./logger.js"; const MAX_OUTPUT_BYTES = 200 * 1024; // 200 KB const QUIET_HEARTBEAT_INTERVAL_MS = 60_000; // emit synthetic heartbeat after 60s silence const SIGKILL_GRACE_MS = 10_000; -const DEFAULT_TIMEOUT_PACKAGE_SEC = 300; -const DEFAULT_TIMEOUT_WORKSPACE_SEC = 900; -const MAX_TIMEOUT_SEC = 1800; +export const DEFAULT_TIMEOUT_PACKAGE_SEC = 300; +export const DEFAULT_TIMEOUT_WORKSPACE_SEC = 900; +export const MAX_TIMEOUT_SEC = 1800; + +export const BOUNDED_VERIFICATION_GUIDANCE = + "Prefer a bounded targeted command such as `pnpm --filter exec vitest run src/path/to/test.ts --silent=passed-only --reporter=dot` before rerunning broader suites."; +export const MARATHON_SOFT_CAP_SEC = 120; const packageDirCache = new Map(); @@ -82,6 +86,89 @@ function shellSplit(input: string): string[] | null { return tokens; } +function isPnpmToken(token: string): boolean { + return token === "pnpm" || token.endsWith("/pnpm"); +} + +function tokenLooksLikeTestFile(token: string): boolean { + return /\.(test|spec)\.[cm]?[tj]sx?$/.test(token); +} + +function tokenLooksLikeFileScopedVitest(tokens: string[]): boolean { + const vitestIndex = tokens.findIndex((token) => token === "vitest" || token.endsWith("/vitest")); + if (vitestIndex < 0) return false; + const runIndex = tokens.indexOf("run", vitestIndex + 1); + if (runIndex < 0) return false; + return tokens.slice(runIndex + 1).some((token) => !token.startsWith("-") && tokenLooksLikeTestFile(token)); +} + +function tokenLooksLikeForwardedTestFile(tokens: string[]): boolean { + const runIndex = tokens.indexOf("--run"); + if (runIndex < 0) return false; + return tokens.slice(runIndex + 1).some((token) => !token.startsWith("-") && tokenLooksLikeTestFile(token)); +} + +function isRootPnpmTest(tokens: string[]): boolean { + if (tokens.length < 2 || !isPnpmToken(tokens[0])) return false; + const nonFlagTokens = tokens.slice(1).filter((token) => token !== "-w" && token !== "--workspace-root"); + return (nonFlagTokens.length === 1 && nonFlagTokens[0] === "test") + || (nonFlagTokens.length === 2 && nonFlagTokens[0] === "run" && nonFlagTokens[1] === "test"); +} + +export interface MarathonDetection { + isMarathon: boolean; + reason?: string; + guidance: string; +} + +export function detectMarathonVerification(command: string, scope?: "package" | "workspace"): MarathonDetection { + const guidance = `${BOUNDED_VERIFICATION_GUIDANCE} Use allowFullSuite: true only when a genuinely full run is required.`; + const compact = command.replace(/\s+/g, " ").trim(); + const tokens = shellSplit(command) ?? []; + + /* + * FNXC:Verification 2026-06-17-14:48: + * Marathon detection is intentionally token/regex based so it catches the costly invocation shapes that caused stuck-loop requeues without executing shell expansions. + * Positive patterns: root `pnpm test`/`pnpm -w test`, `test:full`, `verify:workspace`, whole-package `pnpm --filter test`, and loop/repeat wrappers around pnpm/npm/vitest test runners. + */ + if (tokens.length > 0 && isRootPnpmTest(tokens)) { + return { isMarathon: true, reason: "root workspace test suite (`pnpm test`) is a marathon verification command", guidance }; + } + + if (/\bpnpm\b(?:\s+[-\w=:@/.]+)*\s+(?:run\s+)?(?:test:full|verify:workspace)\b/.test(compact)) { + return { isMarathon: true, reason: "full workspace verification script is a marathon command", guidance }; + } + + const filterIndex = tokens.findIndex((token) => token === "--filter" || token === "-F"); + if (tokens.length > 0 && isPnpmToken(tokens[0]) && filterIndex >= 0) { + const afterFilter = tokens.slice(filterIndex + 2); + const scriptToken = afterFilter.find((token) => token !== "--"); + const runsTestScript = scriptToken === "test" || (afterFilter[0] === "run" && afterFilter[1] === "test"); + if (runsTestScript && !tokenLooksLikeFileScopedVitest(tokens) && !tokenLooksLikeForwardedTestFile(tokens)) { + return { isMarathon: true, reason: "whole-package test script has no file-scoped vitest run filter", guidance }; + } + } + + if (/\b(for|while)\b[\s\S]*\bdo\b[\s\S]*\b(pnpm|npm|vitest)\b[\s\S]*\b(test|vitest)\b/.test(command)) { + return { isMarathon: true, reason: "shell loop repeats a test runner", guidance }; + } + + if (/\bseq\b[\s\S]*\|[\s\S]*\bxargs\b[\s\S]*\b(pnpm|npm|vitest)\b[\s\S]*\b(test|vitest)\b/.test(command)) { + return { isMarathon: true, reason: "seq/xargs pipeline repeats a test runner", guidance }; + } + + const chainedTestRuns = compact.split(/\s*&&\s*/).filter((part) => /\b(pnpm|npm|vitest)\b.*\b(test|vitest)\b/.test(part)); + if (chainedTestRuns.length > 1) { + return { isMarathon: true, reason: "&& chain repeats test runner invocations", guidance }; + } + + if (scope === "workspace" && /\bpnpm\b\s+(?:run\s+)?test\b/.test(compact) && !tokenLooksLikeFileScopedVitest(tokens)) { + return { isMarathon: true, reason: "workspace-scoped test command is likely a full suite", guidance }; + } + + return { isMarathon: false, guidance }; +} + function shellQuote(value: string): string { if (/^[A-Za-z0-9_@%+=:,./-]+$/.test(value)) return value; return `'${value.replace(/'/g, "'\\''")}'`; @@ -247,7 +334,13 @@ export const runVerificationParams = Type.Object({ timeoutSec: Type.Optional( Type.Number({ description: - "Override the default timeout in seconds. Default: 300 for package scope, 900 for workspace scope. Hard cap: 1800.", + "Override the default timeout in seconds. Default: project verificationCommandTimeoutMs when set, otherwise 300 for package scope and 900 for workspace scope. Hard cap: 1800.", + }), + ), + allowFullSuite: Type.Optional( + Type.Boolean({ + description: + "Explicit opt-in for marathon verification commands such as pnpm test, pnpm test:full, verify:workspace, whole-package tests, or repeat loops. Default: false; still respects the hard timeout.", }), ), expectFailure: Type.Optional( @@ -520,6 +613,8 @@ export interface CreateRunVerificationToolOpts { taskId: string; /** Called on every output line AND on synthetic quiet-interval heartbeats. */ recordActivity: () => void; + /** Project-level default timeout budget in milliseconds. Values <= 0 disable the override and preserve legacy per-scope defaults. */ + verificationCommandTimeoutMs?: number; /** * FNXC:Reliability 2026-06-17-16:12: * FN-6598 brackets fn_run_verification subprocesses so the stuck detector treats bounded, actively running verification as progress instead of no-progress loop churn. @@ -546,21 +641,24 @@ export interface CreateRunVerificationToolOpts { export function createRunVerificationTool( opts: CreateRunVerificationToolOpts, ): ToolDefinition { - const { worktreePath, rootDir, taskId, recordActivity, onVerificationStart, onVerificationEnd, log } = opts; + const { worktreePath, rootDir, taskId, recordActivity, verificationCommandTimeoutMs, onVerificationStart, onVerificationEnd, log } = opts; return { name: "fn_run_verification", label: "Run Verification", description: "Run a verification command (tests, lint, build, typecheck) with timeout and progress " + - "heartbeat protection. Use this instead of bash for any pnpm/npm test/lint/build commands. " + - "Prevents the inactivity watchdog from killing your session during long compiles.", + "heartbeat protection. Verification is bounded by default: project verificationCommandTimeoutMs when set, " + + "otherwise 300s for package scope and 900s for workspace scope, with an 1800s hard cap. " + + "Marathon invocations (pnpm test, test:full, verify:workspace, whole-package tests, repeat loops) " + + "are soft-capped unless allowFullSuite=true is explicitly provided. Use this instead of bash for any " + + "pnpm/npm test/lint/build commands.", parameters: runVerificationParams, execute: async ( _toolCallId: string, params: Static, ) => { - const { command, scope, expectFailure = false } = params; + const { command, scope, allowFullSuite = false, expectFailure = false } = params; const warnings: string[] = []; // ── Scope / command mismatch warning ───────────────────────────────── @@ -583,11 +681,32 @@ export function createRunVerificationTool( } // ── Resolve timeout ─────────────────────────────────────────────────── - const defaultTimeoutSec = + /* + * FNXC:Verification 2026-06-17-14:31: + * Engine-level default verification budgets replace per-task "Verification Bounds" prose. + * A positive project setting overrides both scope defaults; undefined or 0 preserves the legacy package/workspace defaults so existing builds do not silently lose runtime. + */ + const scopeDefaultTimeoutSec = scope === "package" ? DEFAULT_TIMEOUT_PACKAGE_SEC : DEFAULT_TIMEOUT_WORKSPACE_SEC; - const rawTimeoutSec = params.timeoutSec ?? defaultTimeoutSec; + const configuredDefaultTimeoutSec = + typeof verificationCommandTimeoutMs === "number" && verificationCommandTimeoutMs > 0 + ? Math.ceil(verificationCommandTimeoutMs / 1000) + : undefined; + const defaultTimeoutSec = configuredDefaultTimeoutSec ?? scopeDefaultTimeoutSec; + let rawTimeoutSec = params.timeoutSec ?? defaultTimeoutSec; + const marathon = detectMarathonVerification(command, scope); + if (marathon.isMarathon && !allowFullSuite && rawTimeoutSec > MARATHON_SOFT_CAP_SEC) { + const msg = `marathon verification detected (${marathon.reason}); soft-capping timeout to ${MARATHON_SOFT_CAP_SEC}s. ${marathon.guidance}`; + warnings.push(msg); + log.warn(`[fn_run_verification] ${taskId}: ${msg}`); + rawTimeoutSec = MARATHON_SOFT_CAP_SEC; + } else if (marathon.isMarathon && allowFullSuite) { + const msg = `allowFullSuite=true acknowledged for marathon verification (${marathon.reason}); subprocess still sends verification heartbeats and respects the ${MAX_TIMEOUT_SEC}s hard cap.`; + warnings.push(msg); + log.warn(`[fn_run_verification] ${taskId}: ${msg}`); + } const timeoutSec = Math.min(rawTimeoutSec, MAX_TIMEOUT_SEC); const timeoutMs = timeoutSec * 1000; @@ -670,7 +789,8 @@ export function createRunVerificationTool( if (result.timedOut) { lines.push( "\nDo NOT blindly retry — investigate whether subprocesses are hung, " + - "test loops are infinite, or dependencies are missing.", + "test loops are infinite, or dependencies are missing. " + + BOUNDED_VERIFICATION_GUIDANCE, ); } diff --git a/packages/engine/src/verification-utils.ts b/packages/engine/src/verification-utils.ts index 7cd54a68fe..9c0817e74b 100644 --- a/packages/engine/src/verification-utils.ts +++ b/packages/engine/src/verification-utils.ts @@ -10,6 +10,7 @@ import type { SandboxBackend, SandboxRunStreamingOptions, SandboxStreamingResult export const VERIFICATION_COMMAND_MAX_BUFFER = 50 * 1024 * 1024; 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; // ── Types ────────────────────────────────────────────────────────────── @@ -299,6 +300,8 @@ export async function runVerificationCommand( agentLabel?: string, /** Optional extra environment variables to inject into the child process (merged over process.env). */ extraEnv?: NodeJS.ProcessEnv, + /** Optional project-level per-command timeout override in milliseconds. Values <= 0 preserve the legacy default. */ + timeoutMsOverride?: number, ): Promise { const logger = log ?? { log: console.log, error: console.error, warn: console.warn }; const label = (agentLabel ?? "merger") as AgentRole; @@ -323,10 +326,18 @@ export async function runVerificationCommand( }; const verificationStartedAt = Date.now(); + /* + * 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. + */ + const rawTimeoutMs = typeof timeoutMsOverride === "number" && timeoutMsOverride > 0 + ? timeoutMsOverride + : VERIFICATION_COMMAND_TIMEOUT_MS; + const timeoutMs = Math.min(rawTimeoutMs, VERIFICATION_COMMAND_HARD_CAP_MS); try { const { stdout, stderr, bufferOverflow } = await execWithProcessGroup(command, { cwd: rootDir, - timeout: VERIFICATION_COMMAND_TIMEOUT_MS, + timeout: timeoutMs, maxBuffer: VERIFICATION_COMMAND_MAX_BUFFER, signal, ...(extraEnv !== undefined && { env: extraEnv }),