diff --git a/docs/users/configuration/settings.md b/docs/users/configuration/settings.md index 48d26e8ef77..bc947d430fd 100644 --- a/docs/users/configuration/settings.md +++ b/docs/users/configuration/settings.md @@ -143,10 +143,12 @@ Settings are organized into categories. Most settings should be placed within th | -------------------------------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------- | | `model.name` | string | The Qwen model to use for conversations. | `undefined` | | `model.maxSessionTurns` | number | Maximum number of user/model/tool turns to keep in a session. -1 means unlimited. | `-1` | +| `model.maxWallTimeSeconds` | number | Wall-clock budget for headless / unattended runs, in seconds. `-1` means unlimited. Overridable per-invocation via `--max-wall-time`, which requires a positive duration (`90`, `30s`, `5m`, `1h`, `1.5h`); the minimum is 1 second — sub-second values (`500ms`, `0.5`) are rejected as typos. Omit the flag to fall back to this setting. Aborts with exit code 55 when exceeded. | `-1` | +| `model.maxToolCalls` | number | Cumulative tool-call budget for a run (counts every executed tool, success or failure; `structured_output` under `--json-schema` is exempt). `-1` means unlimited; `0` means "no tool calls allowed". Capped at 1,000,000 to catch typos. Overridable via `--max-tool-calls`. Aborts with exit code 55 when exceeded. | `-1` | | `model.generationConfig` | object | Advanced overrides passed to the underlying content generator. Supports request controls such as `timeout`, `maxRetries`, `enableCacheControl`, `splitToolMedia` (set `true` for strict OpenAI-compatible servers like LM Studio that reject non-text content on `role: "tool"` messages — splits media into a follow-up user message), `contextWindowSize` (override model's context window size), `modalities` (override auto-detected input modalities), `customHeaders` (custom HTTP headers for API requests), and `extra_body` (additional body parameters for OpenAI-compatible API requests only), along with fine-tuning knobs under `samplingParams` (for example `temperature`, `top_p`, `max_tokens`). Leave unset to rely on provider defaults. | `undefined` | | `model.chatCompression.contextPercentageThreshold` | number | Sets the threshold for chat history compression as a percentage of the model's total token limit. This is a value between 0 and 1 that applies to both automatic compression and the manual `/compress` command. For example, a value of `0.6` will trigger compression when the chat history exceeds 60% of the token limit. Use `0` to disable compression entirely. | `0.7` | | `model.skipNextSpeakerCheck` | boolean | Skip the next speaker check. | `false` | -| `model.skipLoopDetection` | boolean | Disables loop detection checks. Loop detection prevents infinite loops in AI responses but can generate false positives that interrupt legitimate workflows. Enable this option if you experience frequent false positive loop detection interruptions. | `false` | +| `model.skipLoopDetection` | boolean | Disables streaming loop detection checks. Defaults to `true` (loop detection is skipped) to avoid false positives interrupting legitimate workflows. Set to `false` to re-enable streaming loop detection — useful as a guardrail in headless / non-interactive runs where stuck repetition can otherwise waste budget. | `true` | | `model.skipStartupContext` | boolean | Skips sending the startup workspace context (environment summary and acknowledgement) at the beginning of each session. Enable this if you prefer to provide context manually or want to save tokens on startup. | `false` | | `model.enableOpenAILogging` | boolean | Enables logging of OpenAI API calls for debugging and analysis. When enabled, API requests and responses are logged to JSON files. | `false` | | `model.openAILoggingDir` | string | Custom directory path for OpenAI API logs. If not specified, defaults to `logs/openai` in the current working directory. Supports absolute paths, relative paths (resolved from current working directory), and `~` expansion (home directory). | `undefined` | @@ -703,7 +705,9 @@ Qwen Code can execute potentially unsafe operations (like shell commands and fil - Using `--sandbox` or `-s` flag. - Setting `QWEN_SANDBOX` environment variable. -- Sandbox is enabled when using `--yolo` or `--approval-mode=yolo` by default. +- Setting `tools.sandbox` in settings. + +> ⚠️ **`--yolo` does _not_ automatically enable a sandbox.** YOLO mode only auto-approves tool calls; sandboxing must still be opted into via `--sandbox`, `QWEN_SANDBOX`, or `tools.sandbox`. In headless / non-interactive runs with `--yolo` (or `--approval-mode=yolo`) and no sandbox, the model can execute shell, write, and edit tools at the current process's privilege level — Qwen Code prints a warning to stderr in that case. Suppress with `QWEN_CODE_SUPPRESS_YOLO_WARNING=1` once you've reviewed the trade-off. By default, it uses a pre-built `qwen-code-sandbox` Docker image. diff --git a/docs/users/features/headless.md b/docs/users/features/headless.md index e6e0492d5ce..6dad885ec53 100644 --- a/docs/users/features/headless.md +++ b/docs/users/features/headless.md @@ -238,9 +238,41 @@ Key command-line options for headless usage: | `--approval-mode` | Set approval mode | `qwen -p "query" --approval-mode auto_edit` | | `--continue` | Resume the most recent session for this project | `qwen --continue -p "Pick up where we left off"` | | `--resume [sessionId]` | Resume a specific session (or choose interactively) | `qwen --resume 123e... -p "Finish the refactor"` | +| `--max-session-turns` | Cap the number of user/model/tool turns in the run | `qwen -p "..." --max-session-turns 30` | +| `--max-wall-time` | Wall-clock budget; accepts `90` (s), `30s`, `5m`, `1h`, `1.5h` | `qwen -p "..." --max-wall-time 10m` | +| `--max-tool-calls` | Cumulative tool-call budget for the run | `qwen -p "..." --max-tool-calls 50` | For complete details on all available configuration options, settings files, and environment variables, see the [Configuration Guide](../configuration/settings). +## Safety in unattended runs + +Headless / CI runs combined with `--yolo` (or `--approval-mode=yolo`) auto-approve every tool call, including `shell`, `write`, and `edit`. **`--yolo` does not enable a sandbox** — those tools run at the host process's privilege level. When Qwen Code detects this combination with no sandbox configured, it prints a one-line warning to stderr at startup. Suppress the warning with `QWEN_CODE_SUPPRESS_YOLO_WARNING=1` once you've reviewed the trade-off. + +### Run-level budgets + +Qwen Code can abort an unattended run when it crosses one of the following thresholds. Each is `-1` (unlimited) by default; setting any one is enough to bound runaway behavior. They are enforced cooperatively against the same `AbortController` that already carries SIGINT, so a budget abort emits a structured `FatalBudgetExceededError` (exit code **55**) — distinct from the turn-cap exit code 53 and SIGINT's 130 so CI scripts can branch on the reason. + +| Flag | Settings key | What it bounds | +| --------------------- | -------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--max-wall-time` | `model.maxWallTimeSeconds` | Wall-clock duration of the whole run. Flag accepts `90` (s), `30s`, `5m`, `1h`, `1.5h` (fractional units supported). Minimum 1s — sub-second values are rejected as typos. Settings is seconds. | +| `--max-tool-calls` | `model.maxToolCalls` | Cumulative top-level tool calls dispatched by the main run loop (counts successes _and_ failures — the model still consumes tokens on errors). See "Scope" below for subagent / structured-output exemptions. | +| `--max-session-turns` | `model.maxSessionTurns` | Number of user/model/tool turns; pre-existing. Exits with code 53 on overrun (distinct from budget exit 55). | + +#### Scope + +- **`--max-tool-calls` counts top-level dispatches only.** When the model calls the `agent` tool, the dispatch counts as **1**; inner tool calls performed by the spawned subagent are **not** counted. A model that funnels work through subagents can do unbounded inner work under a small top-level budget. Combine with `--exclude-tools agent` if you need a tighter cap. +- **`structured_output` is exempt from `--max-tool-calls`.** Under `--json-schema`, the model's terminal `structured_output` call is the "I'm done" contract, not real work — it doesn't count against `--max-tool-calls` so a budget-edge completion isn't aborted as a false positive. The exemption is unconditional (including failed Ajv validations), so a model stuck in a malformed-output retry loop is NOT bounded by `--max-tool-calls`; combine with `--max-session-turns` or `--max-wall-time` to cap retries. +- **`structured_output` is NOT exempt from `--max-session-turns`.** That counter is pre-existing and bumps for every turn including the terminal contract. Size `--max-session-turns` to `N+1` if you want to allow `N` real-work turns under `--json-schema`. +- **Single-shot vs `--input-format stream-json`:** in stream-json input mode the daemon resets the budget counters at the start of every user message; the budget is per-message, not per-process. +- **`qwen serve` / ACP sessions:** the daemon ACP session path does NOT currently consult `--max-wall-time` / `--max-tool-calls` from settings.json. These budgets only apply to single-shot `qwen -p` runs and to `--input-format stream-json` sessions. (`qwen serve` does emit the YOLO-no-sandbox warning at boot if `tools.approvalMode: 'yolo'` is set in settings.) + +### Recommended combinations + +- **Trusted, isolated environment (ephemeral CI runner, container):** `qwen -p "..." --yolo --max-session-turns N --max-wall-time 10m --output-format json`. Pin a turn budget and a wall-clock budget so a stuck agent can't burn through your CI minutes, and capture `--output-format json` for post-run usage / tool-call auditing. +- **Local machine or shared infra:** also pass `--sandbox` (or set `QWEN_SANDBOX=1`) so shell / write / edit tools run inside the sandbox image. +- **Long-running CI with retry-on-rate-limit:** combine `QWEN_CODE_UNATTENDED_RETRY=1` with `--max-wall-time`. The retry env keeps the run alive past transient 429 / 529 responses; the wall-clock budget ensures a persistently-failing provider can't extend the job indefinitely. +- **Bounded auditing / exploration:** for read-only tasks, `--max-tool-calls 25` caps how aggressively the model can grep / read. Combine with `--exclude-tools shell,write,edit` to make the bound meaningful. + ## Examples ### Code review diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index dae40a9285f..3e372e982b7 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -12,7 +12,12 @@ import type { Argv, CommandModule } from 'yargs'; // handler below so it only loads when the user actually runs `qwen serve`. import { writeStderrLine } from '../utils/stdioHelpers.js'; import { DEFAULT_RING_SIZE } from '../serve/eventBus.js'; -import { MCP_BUDGET_WARN_FRACTION } from '@qwen-code/qwen-code-core'; +import { + ApprovalMode, + MCP_BUDGET_WARN_FRACTION, +} from '@qwen-code/qwen-code-core'; +import { loadSettings } from '../config/settings.js'; +import { HEADLESS_YOLO_NO_SANDBOX_WARNING } from '../utils/headlessSafetyWarnings.js'; /** * Pause the current async function indefinitely. Used after the daemon @@ -203,6 +208,37 @@ export const serveCommand: CommandModule = { ); } + // Emit the headless-YOLO safety warning at daemon startup if + // settings.json statically configures yolo + no sandbox. We can't + // use `getHeadlessYoloSafetyWarning(config)` here because the daemon + // hasn't constructed a `Config` yet — sessions get their own — so + // we re-derive the predicate from the same settings.json the + // sessions will load. Per-session override (the ACP client flipping + // approval mode mid-session) is out of scope here; this warns about + // a deployment that's wide-open at boot. Suppress with + // QWEN_CODE_SUPPRESS_YOLO_WARNING=1. + try { + const loaded = loadSettings(argv.workspace ?? process.cwd()); + const merged = loaded.merged; + const approvalMode = merged.tools?.approvalMode; + const sandbox = merged.tools?.sandbox; + const sandboxEnv = process.env['SANDBOX']; + const suppress = process.env['QWEN_CODE_SUPPRESS_YOLO_WARNING']; + const suppressed = suppress === '1' || suppress === 'true'; + if ( + approvalMode === ApprovalMode.YOLO && + !sandbox && + !sandboxEnv && + !suppressed + ) { + writeStderrLine(HEADLESS_YOLO_NO_SANDBOX_WARNING); + } + } catch { + // Settings load can fail (corrupt JSON, etc.); don't block + // daemon startup just to emit a warning — the existing settings + // path will report the same error to the user via Session. + } + // Lazy-load the serve module so non-serve invocations don't pay for // express + body-parser + qs in their startup path. const { runQwenServe } = await import('../serve/index.js'); diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index baea941922e..cc328924500 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -75,6 +75,11 @@ export function isValidSessionId(value: string): boolean { import { isWorkspaceTrusted } from './trustedFolders.js'; import { writeStderrLine } from '../utils/stdioHelpers.js'; +import { + parseDurationSeconds, + validateMaxToolCalls, + validateMaxWallTimeSetting, +} from '../utils/runBudget.js'; const debugLogger = createDebugLogger('CONFIG'); @@ -171,6 +176,8 @@ export interface CliArgs { /** Internal: preserve the outer session ID when relaunching in a sandbox */ sandboxSessionId?: string | undefined; maxSessionTurns: number | undefined; + maxWallTime: string | undefined; + maxToolCalls: number | undefined; coreTools: string[] | undefined; excludeTools: string[] | undefined; disabledSlashCommands: string[] | undefined; @@ -828,6 +835,16 @@ export async function parseArguments(): Promise { type: 'number', description: 'Maximum number of session turns', }) + .option('max-wall-time', { + type: 'string', + description: + 'Run-level wall-clock budget for headless / unattended runs. Accepts seconds (e.g. `90`), or a duration string with unit (e.g. `30s`, `5m`, `1h`, `1.5h`). Minimum 1s — sub-second values (`500ms`, `0.5`) are rejected as typos; max ~24 days. Aborts the run with exit code 55 when exceeded.', + }) + .option('max-tool-calls', { + type: 'number', + description: + 'Maximum cumulative tool calls executed during the run (success or failure; `structured_output` under --json-schema is exempt). Aborts with exit code 55 when exceeded. -1 / unset means no limit; 0 means "no tool calls allowed" (first call aborts). Capped at 1,000,000 to catch typos.', + }) .option('core-tools', { type: 'array', string: true, @@ -1116,6 +1133,65 @@ export async function loadHierarchicalGeminiMemory( ); } +/** + * Resolves the wall-clock budget for a run. Returns seconds (`-1` = + * unlimited). Order of precedence: `--max-wall-time` flag, then + * `model.maxWallTimeSeconds` from settings, else unlimited. + * + * The CLI flag is a duration string (`30s` / `5m` / `1h` / `90`); the + * settings entry is a plain number of seconds (parity with + * `model.maxSessionTurns`). Both layers reject `0` and out-of-range + * values up front — a typo in a CI guardrail should fail loud at startup, + * not silently disable the budget. + */ +function resolveMaxWallTimeSeconds(argv: CliArgs, settings: Settings): number { + if (argv.maxWallTime !== undefined && argv.maxWallTime !== null) { + try { + return parseDurationSeconds(String(argv.maxWallTime)); + } catch (err) { + throw new Error(`--max-wall-time: ${(err as Error).message}`); + } + } + const fromSettings = settings.model?.maxWallTimeSeconds; + if (typeof fromSettings === 'number') { + try { + return validateMaxWallTimeSetting(fromSettings); + } catch (err) { + throw new Error(`settings.json: ${(err as Error).message}`); + } + } + return -1; +} + +/** + * Resolves the tool-call budget for a run. Returns the validated count + * (`-1` = unlimited). Order of precedence: `--max-tool-calls` flag, then + * `model.maxToolCalls` from settings, else unlimited. + * + * Symmetric with `resolveMaxWallTimeSeconds`: yargs accepts `NaN` from + * non-numeric flag values, and the enforcer's `>= 0` gate would silently + * disable the budget for `NaN` / negatives. Validate up front so a typo + * in a CI guardrail fails loudly. + */ +function resolveMaxToolCalls(argv: CliArgs, settings: Settings): number { + if (argv.maxToolCalls !== undefined && argv.maxToolCalls !== null) { + try { + return validateMaxToolCalls(argv.maxToolCalls); + } catch (err) { + throw new Error(`--max-tool-calls: ${(err as Error).message}`); + } + } + const fromSettings = settings.model?.maxToolCalls; + if (typeof fromSettings === 'number') { + try { + return validateMaxToolCalls(fromSettings); + } catch (err) { + throw new Error(`settings.json: ${(err as Error).message}`); + } + } + return -1; +} + export function isDebugMode(argv: CliArgs): boolean { return ( argv.debug || @@ -1731,6 +1807,8 @@ export async function loadCliConfig( sessionTokenLimit: settings.model?.sessionTokenLimit ?? -1, maxSessionTurns: argv.maxSessionTurns ?? settings.model?.maxSessionTurns ?? -1, + maxWallTimeSeconds: resolveMaxWallTimeSeconds(argv, settings), + maxToolCalls: resolveMaxToolCalls(argv, settings), experimentalZedIntegration: argv.acp || argv.experimentalAcp || false, cronEnabled: settings.experimental?.cron ?? false, emitToolUseSummaries: settings.experimental?.emitToolUseSummaries ?? true, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 46726450ae8..c460b488c38 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1076,6 +1076,26 @@ const SETTINGS_SCHEMA = { 'Maximum number of user/model/tool turns to keep in a session. -1 means unlimited.', showInDialog: false, }, + maxWallTimeSeconds: { + type: 'number', + label: 'Max Wall-Clock Time (seconds)', + category: 'Model', + requiresRestart: false, + default: -1, + description: + 'Run-level wall-clock budget for headless / unattended runs, in seconds. -1 means unlimited; otherwise must be in [1, ~2,147,483] (sub-second values and values above ~24 days are rejected as typos). Overridable per-invocation via --max-wall-time (which also accepts duration suffixes like 5m, 1.5h).', + showInDialog: false, + }, + maxToolCalls: { + type: 'number', + label: 'Max Tool Calls', + category: 'Model', + requiresRestart: false, + default: -1, + description: + 'Cumulative tool-call budget for a run (counts every executed tool, success or failure; structured_output under --json-schema is exempt). -1 means unlimited; 0 means "no tool calls allowed" (first call aborts). Capped at 1,000,000 to catch typos. Overridable via --max-tool-calls.', + showInDialog: false, + }, chatCompression: { type: 'object', label: 'Chat Compression', @@ -1109,7 +1129,8 @@ const SETTINGS_SCHEMA = { category: 'Model', requiresRestart: false, default: true, - description: 'Disable all loop detection checks (streaming and LLM).', + description: + 'Skip streaming loop detection. Defaults to true to avoid false-positive interruptions; set to false to re-enable as an unattended-run guardrail.', showInDialog: false, }, skipStartupContext: { diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 50ea0850315..28e5337851b 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -24,7 +24,7 @@ import type { CliArgs } from './config/config.js'; import { type LoadedSettings } from './config/settings.js'; import { appEvents, AppEvent } from './utils/events.js'; import type { Config } from '@qwen-code/qwen-code-core'; -import { OutputFormat } from '@qwen-code/qwen-code-core'; +import { ApprovalMode, OutputFormat } from '@qwen-code/qwen-code-core'; const mockWriteStderrLine = vi.hoisted(() => vi.fn()); @@ -181,6 +181,7 @@ describe('gemini.tsx main function', () => { isInteractive: () => false, getQuestion: () => '', getSandbox: () => false, + getApprovalMode: () => ApprovalMode.DEFAULT, getDebugMode: () => false, getListExtensions: () => false, getMcpServers: () => ({}), @@ -260,6 +261,7 @@ describe('gemini.tsx main function', () => { isInteractive: () => false, getQuestion: () => 'bare prompt', getSandbox: () => false, + getApprovalMode: () => ApprovalMode.DEFAULT, getDebugMode: () => false, getListExtensions: () => false, getMcpServers: () => ({}), @@ -569,6 +571,7 @@ describe('gemini.tsx main function', () => { isInteractive: () => false, getQuestion: () => ' hello stream ', getSandbox: () => false, + getApprovalMode: () => ApprovalMode.DEFAULT, getDebugMode: () => false, getListExtensions: () => false, getMcpServers: () => ({}), @@ -773,6 +776,8 @@ describe('gemini.tsx main function kitty protocol', () => { disabledSlashCommands: undefined, authType: undefined, maxSessionTurns: undefined, + maxWallTime: undefined, + maxToolCalls: undefined, experimentalLsp: undefined, channel: undefined, chatRecording: undefined, diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 94061489e9b..7b6df42b510 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -79,6 +79,7 @@ import { getStartupWarnings } from './utils/startupWarnings.js'; import { getUserStartupWarnings } from './utils/userStartupWarnings.js'; import { getCliVersion } from './utils/version.js'; import { writeStderrLine } from './utils/stdioHelpers.js'; +import { getHeadlessYoloSafetyWarning } from './utils/headlessSafetyWarnings.js'; import { computeWindowTitle } from './utils/windowTitle.js'; import { startEarlyInputCapture, @@ -822,6 +823,17 @@ export async function main() { } } + // Headless + YOLO without a sandbox lets the model auto-approve and + // execute shell / write / edit tools at the current process's + // privilege level. Emit a one-line stderr warning so unattended runs + // have at least an observable signal. Interactive runs are excluded + // because the user is at the keyboard and the TUI shows approval + // state directly. See issue #4103. + if (!config.isInteractive()) { + const yoloWarning = getHeadlessYoloSafetyWarning(config); + if (yoloWarning) writeStderrLine(yoloWarning); + } + // For non-stream-json mode, initialize config here. Stream-json defers // `config.initialize()` to inside `Session.ensureConfigInitialized` // because the initial control_request may register SDK MCP servers diff --git a/packages/cli/src/nonInteractiveCli.test.ts b/packages/cli/src/nonInteractiveCli.test.ts index 51d148a33d8..3892884de7e 100644 --- a/packages/cli/src/nonInteractiveCli.test.ts +++ b/packages/cli/src/nonInteractiveCli.test.ts @@ -174,6 +174,8 @@ describe('runNonInteractive', () => { getGeminiClient: vi.fn().mockReturnValue(mockGeminiClient), getToolRegistry: vi.fn().mockReturnValue(mockToolRegistry), getMaxSessionTurns: vi.fn().mockReturnValue(10), + getMaxWallTimeSeconds: vi.fn().mockReturnValue(-1), + getMaxToolCalls: vi.fn().mockReturnValue(-1), getProjectRoot: vi.fn().mockReturnValue('/test/project'), getTargetDir: vi.fn().mockReturnValue('/test/project'), getMcpServers: vi.fn().mockReturnValue(undefined), diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index 3c1125a15d0..14f00dfa024 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -43,7 +43,9 @@ import { handleToolError, handleCancellationError, handleMaxTurnsExceededError, + handleBudgetExceededError, } from './utils/errors.js'; +import { RunBudgetEnforcer } from './utils/runBudget.js'; const debugLogger = createDebugLogger('NON_INTERACTIVE_CLI'); @@ -220,6 +222,44 @@ export async function runNonInteractive( const geminiClient = config.getGeminiClient(); const abortController = options.abortController ?? new AbortController(); + // Run-level budget enforcement for headless / unattended runs + // (issue #4103). Tied to the same abortController as user-initiated + // SIGINT so the existing cancellation plumbing carries the abort; + // `routeAbort` below interprets the reason so the user sees + // "budget exceeded" instead of a generic "cancelled" envelope. + const budgetEnforcer = new RunBudgetEnforcer( + { + maxWallTimeSeconds: config.getMaxWallTimeSeconds(), + maxToolCalls: config.getMaxToolCalls(), + }, + abortController, + ); + budgetEnforcer.start(); + + /** + * Called at every abort-detection site in place of + * `handleCancellationError` directly. If a budget tripped, surface the + * structured budget error (exit 55); otherwise fall through to the + * SIGINT / user-cancel path (exit 130) so existing behavior is + * preserved. Both branches call into `process.exit(...)` so the + * `unreachable` throw is only present to keep the type-checker honest. + */ + const routeAbort = async (): Promise => { + const exceeded = budgetEnforcer.getExceeded(); + if (exceeded) { + await handleBudgetExceededError(config, exceeded); + // Explicit unreachable — `handleBudgetExceededError` is `never` + // in production (it calls `process.exit`). If a test stubs + // `process.exit` or a future refactor makes the handler + // resumable, this throw carries the original budget message + // so the outer catch's `errorMessage` field stays actionable + // (vs. a useless literal "unreachable"). + throw new Error(exceeded.message); + } + await handleCancellationError(config); + throw new Error('Operation cancelled.'); + }; + interface LocalQueueItem { displayText: string; modelText: string; @@ -613,6 +653,34 @@ export async function runNonInteractive( ) : createToolProgressHandler(requestInfo, adapter); + // Tick BEFORE the call so that --max-tool-calls=N caps the run + // at exactly N executions: the (N+1)th tick aborts before the + // tool runs. Ticking after would let the (N+1)th tool execute + // and only then abort. See issue #4103. + // + // Exempt `structured_output` ONLY when `--json-schema` is + // active: under --json-schema this is the terminal "I'm done" + // contract tool, not real work, and counting it would abort + // an otherwise-valid completion at the budget edge (budget=3, + // model used 3 tools then emits structured_output as call #4 + // → exit 55 instead of success). Guarding on + // `getJsonSchema()` keeps the exemption tied to the feature + // that owns the tool name — an MCP server that registers an + // unrelated tool literally named `structured_output` would + // otherwise inherit a free pass. + // + // Caveat: failed structured_output calls (Ajv validation + // failure) also skip the tick, so a model stuck in a + // validation-retry loop is not bounded by --max-tool-calls. + // Documented in docs/users/features/headless.md → "Scope". + // Combine with --max-session-turns or --max-wall-time. + const isStructuredOutputExempt = + requestInfo.name === ToolNames.STRUCTURED_OUTPUT && + config.getJsonSchema?.() !== undefined; + if (!isStructuredOutputExempt) { + budgetEnforcer.tickToolCall(); + } + if (abortController.signal.aborted) await routeAbort(); const toolResponse = await executeToolCall( config, requestInfo, @@ -749,7 +817,12 @@ export async function runNonInteractive( for await (const event of responseStream) { if (abortController.signal.aborted) { - await handleCancellationError(config); + // Pair the startAssistantMessage() above so stream-json mode + // doesn't leave an unterminated message_start when a budget / + // SIGINT abort lands mid-stream. Symmetric with the drain-item + // loop fix below. + adapter.finalizeAssistantMessage(); + await routeAbort(); } // Use adapter for all event processing adapter.processEvent(event); @@ -863,10 +936,24 @@ export async function runNonInteractive( for await (const event of itemStream) { if (abortController.signal.aborted) { - // Pair the startAssistantMessage() above so stream-json mode doesn't - // leave an unterminated message_start. + // Pair the startAssistantMessage() above so stream-json + // mode doesn't leave an unterminated message_start, then + // route through `routeAbort` so a budget overrun in the + // final drain item surfaces as exit code 55 instead of + // being silently swallowed by the outer success path + // (drain-loop fall-through; see issue #4103 review). + // + // Also flush queued task notifications and finalize + // one-shot monitors here. Previously this site used a + // bare `return` and let control fall through to the + // outer holdback loop, which did the flushing before + // exiting; routing through `routeAbort` skips that + // path, so we re-do it inline to preserve the + // task_started↔task_notification pairing invariant. adapter.finalizeAssistantMessage(); - return; + flushQueuedNotificationsToSdk(localQueue); + finalizeOneShotMonitors(); + await routeAbort(); } adapter.processEvent(event); if (event.type === GeminiEventType.ToolCallRequest) { @@ -1018,12 +1105,12 @@ export async function runNonInteractive( while (true) { if (abortController.signal.aborted) { registry.abortAll(); - // Flush queued terminal notifications before handleCancellationError - // exits so stream-json consumers always see a task_notification paired - // with every task_started. + // Flush queued terminal notifications before routeAbort + // exits so stream-json consumers always see a task_notification + // paired with every task_started. flushQueuedNotificationsToSdk(localQueue); finalizeOneShotMonitors(); - await handleCancellationError(config); + await routeAbort(); } // Once we enter the final holdback loop, monitor events should no // longer extend one-shot runtime. Already-queued events still drain @@ -1130,8 +1217,22 @@ export async function runNonInteractive( flushQueuedNotificationsToSdk(localQueue); finalizeOneShotMonitors(); + // If a run-level budget tripped during an awaited stream / tool + // call, the underlying fetch's AbortError lands here before our + // explicit `routeAbort` sites can fire. Capture the reason so we + // can (a) include the friendly "Run aborted: …" message in the + // adapter's terminal result envelope (STREAM_JSON consumers + // depend on that envelope to close the stream cleanly) and (b) + // exit with the budget handler's exit code 55 instead of the + // generic `handleError` exit code 1 from a raw "AbortError". + const budgetExceeded = budgetEnforcer.getExceeded(); + // For JSON and STREAM_JSON modes, compute usage from metrics - const message = error instanceof Error ? error.message : String(error); + const message = budgetExceeded + ? budgetExceeded.message + : error instanceof Error + ? error.message + : String(error); const metrics = uiTelemetryService.getMetrics(); const usage = computeUsageFromMetrics(metrics); // Get stats for JSON format output @@ -1152,18 +1253,43 @@ export async function runNonInteractive( outputFormat === OutputFormat.TEXT && isAlreadyReportedError; if (!skipAdapterEmit) { - adapter.emitResult({ - isError: true, - durationMs: Date.now() - startTime, - apiDurationMs: totalApiDurationMs, - numTurns: turnCount, - errorMessage: message, - usage, - stats, - }); + // Wrap in try/catch: emitResult eventually hits stdout.write, which + // can throw on EPIPE / ERR_STREAM_WRITE_AFTER_END when a piped + // consumer closes early (`qwen -p ... | head -n 1` is the common + // case). Letting that throw bubble out skips `handleBudgetExceededError` + // / `handleError` below, dropping the documented exit code 55 + // contract — precisely when stdout is in trouble. Best-effort emit + // and continue to the exit handler. + try { + adapter.emitResult({ + isError: true, + durationMs: Date.now() - startTime, + apiDurationMs: totalApiDurationMs, + numTurns: turnCount, + errorMessage: message, + usage, + stats, + }); + } catch (emitErr) { + debugLogger.error( + `Failed to emit terminal result envelope: ${ + emitErr instanceof Error ? emitErr.message : String(emitErr) + }`, + ); + } + } + if (budgetExceeded) { + // Always exit AFTER emitResult so STREAM_JSON / JSON consumers + // see a terminal result envelope before the process dies. + await handleBudgetExceededError(config, budgetExceeded); } await handleError(error, config); } finally { + // Cancel the wall-clock timer so it doesn't fire after a successful + // run completes — important for callers (e.g. the `qwen serve` + // daemon, SDK) that reuse a single process across many runs. + budgetEnforcer.stop(); + const reg = config.getBackgroundTaskRegistry(); reg.setNotificationCallback(undefined); reg.setRegisterCallback(undefined); diff --git a/packages/cli/src/utils/errors.ts b/packages/cli/src/utils/errors.ts index 6e4e4a5ebad..82afca4a64b 100644 --- a/packages/cli/src/utils/errors.ts +++ b/packages/cli/src/utils/errors.ts @@ -11,9 +11,11 @@ import { parseAndFormatApiError, FatalTurnLimitedError, FatalCancellationError, + FatalBudgetExceededError, ToolErrorType, createDebugLogger, } from '@qwen-code/qwen-code-core'; +import type { BudgetExceeded } from './runBudget.js'; import { runExitCleanup } from './cleanup.js'; import { writeStderrLine } from './stdioHelpers.js'; @@ -278,3 +280,31 @@ export async function handleMaxTurnsExceededError( } return exitAfterCleanup(maxTurnsError.exitCode); } + +/** + * Emits the structured "run aborted by budget" error and exits. Used by + * the non-interactive run loop when `--max-wall-time` or `--max-tool-calls` + * fires (see `RunBudgetEnforcer`). Exit code is 55, distinct from the + * turn-cap exit code 53 and SIGINT's 130 so CI scripts can branch on the + * reason. + * + * The output shape intentionally mirrors `handleMaxTurnsExceededError` / + * `handleCancellationError`: structured JSON only on `OutputFormat.JSON` + * and plain stderr for everything else (incl. STREAM_JSON). Emitting a + * structured envelope on STREAM_JSON too is a real gap, but it's a + * codebase-wide convention question that affects cancel / max-turns + * equally, not a budget-specific decision. + */ +export async function handleBudgetExceededError( + config: Config, + exceeded: BudgetExceeded, +): Promise { + const fatal = new FatalBudgetExceededError(exceeded.message); + if (config.getOutputFormat() === OutputFormat.JSON) { + const formatter = new JsonFormatter(); + writeStderrLine(formatter.formatError(fatal, fatal.exitCode)); + } else { + writeStderrLine(fatal.message); + } + return exitAfterCleanup(fatal.exitCode); +} diff --git a/packages/cli/src/utils/headlessSafetyWarnings.test.ts b/packages/cli/src/utils/headlessSafetyWarnings.test.ts new file mode 100644 index 00000000000..35a573335b9 --- /dev/null +++ b/packages/cli/src/utils/headlessSafetyWarnings.test.ts @@ -0,0 +1,96 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { ApprovalMode, type Config } from '@qwen-code/qwen-code-core'; +import { + HEADLESS_YOLO_NO_SANDBOX_WARNING, + getHeadlessYoloSafetyWarning, +} from './headlessSafetyWarnings.js'; + +function makeConfig( + approvalMode: ApprovalMode, + sandbox: unknown, +): Pick { + return { + getApprovalMode: () => approvalMode, + // Real return type is `SandboxConfig | undefined`; the warning policy + // only cares about truthiness so the tests model it as such. + getSandbox: () => sandbox as ReturnType, + }; +} + +describe('getHeadlessYoloSafetyWarning', () => { + it('warns when approval mode is YOLO and no sandbox is configured', () => { + const cfg = makeConfig(ApprovalMode.YOLO, undefined); + expect(getHeadlessYoloSafetyWarning(cfg, {})).toBe( + HEADLESS_YOLO_NO_SANDBOX_WARNING, + ); + }); + + it('does not warn when approval mode is not YOLO', () => { + const cfg = makeConfig(ApprovalMode.DEFAULT, undefined); + expect(getHeadlessYoloSafetyWarning(cfg, {})).toBeNull(); + }); + + it('does not warn when a sandbox is configured', () => { + const cfg = makeConfig(ApprovalMode.YOLO, { + command: 'docker', + image: 'qwen-code-sandbox', + }); + expect(getHeadlessYoloSafetyWarning(cfg, {})).toBeNull(); + }); + + it('does not warn when SANDBOX env is set to the value the sandbox transport actually writes', () => { + const cfg = makeConfig(ApprovalMode.YOLO, undefined); + // macOS seatbelt + expect( + getHeadlessYoloSafetyWarning(cfg, { SANDBOX: 'sandbox-exec' }), + ).toBeNull(); + // Docker / Podman container name + expect( + getHeadlessYoloSafetyWarning(cfg, { SANDBOX: 'qwen-code-sandbox' }), + ).toBeNull(); + // Generic truthy values + expect(getHeadlessYoloSafetyWarning(cfg, { SANDBOX: '1' })).toBeNull(); + expect(getHeadlessYoloSafetyWarning(cfg, { SANDBOX: 'true' })).toBeNull(); + }); + + it('warns when SANDBOX env is unset or empty string', () => { + const cfg = makeConfig(ApprovalMode.YOLO, undefined); + expect(getHeadlessYoloSafetyWarning(cfg, {})).toBe( + HEADLESS_YOLO_NO_SANDBOX_WARNING, + ); + expect(getHeadlessYoloSafetyWarning(cfg, { SANDBOX: '' })).toBe( + HEADLESS_YOLO_NO_SANDBOX_WARNING, + ); + }); + + it('respects the explicit suppression env var when set to 1 or true', () => { + const cfg = makeConfig(ApprovalMode.YOLO, undefined); + expect( + getHeadlessYoloSafetyWarning(cfg, { + QWEN_CODE_SUPPRESS_YOLO_WARNING: '1', + }), + ).toBeNull(); + expect( + getHeadlessYoloSafetyWarning(cfg, { + QWEN_CODE_SUPPRESS_YOLO_WARNING: 'true', + }), + ).toBeNull(); + }); + + it('does NOT suppress when QWEN_CODE_SUPPRESS_YOLO_WARNING is 0 / false / empty', () => { + const cfg = makeConfig(ApprovalMode.YOLO, undefined); + for (const val of ['0', 'false', '', 'no']) { + expect( + getHeadlessYoloSafetyWarning(cfg, { + QWEN_CODE_SUPPRESS_YOLO_WARNING: val, + }), + ).toBe(HEADLESS_YOLO_NO_SANDBOX_WARNING); + } + }); +}); diff --git a/packages/cli/src/utils/headlessSafetyWarnings.ts b/packages/cli/src/utils/headlessSafetyWarnings.ts new file mode 100644 index 00000000000..34f931b4b6a --- /dev/null +++ b/packages/cli/src/utils/headlessSafetyWarnings.ts @@ -0,0 +1,48 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { ApprovalMode, type Config } from '@qwen-code/qwen-code-core'; + +export const HEADLESS_YOLO_NO_SANDBOX_WARNING = + 'Warning: running headless with --yolo / approval-mode=yolo and no sandbox. ' + + "All tool calls (shell, write, edit) auto-execute at this process's privilege level. " + + 'Enable a sandbox via --sandbox / QWEN_SANDBOX, or set ' + + 'QWEN_CODE_SUPPRESS_YOLO_WARNING=1 to silence this notice.'; + +/** + * Returns a warning line to emit when running in YOLO without a sandbox in a + * non-interactive run, or `null` when no warning is warranted: sandbox is + * configured, we're already inside a sandbox, approval mode is not YOLO, or + * the user explicitly suppressed the notice. + * + * The call site (gemini.tsx) is responsible for gating on + * `!config.isInteractive()` — this helper deliberately ignores interactivity + * so it stays pure and unit-testable. + * + * The `env` argument is injectable for tests; production callers omit it and + * fall through to `process.env`. + */ +export function getHeadlessYoloSafetyWarning( + config: Pick, + env: NodeJS.ProcessEnv = process.env, +): string | null { + if (config.getApprovalMode() !== ApprovalMode.YOLO) return null; + if (config.getSandbox()) return null; + // `SANDBOX` is set by the sandbox transport itself: macOS seatbelt sets + // it to `sandbox-exec`, Docker/Podman to the container name (e.g. + // `qwen-code-sandbox`). Match the rest of the codebase + // (sandboxConfig.ts, gemini.tsx, Footer.tsx, prompts.ts, …) which all + // treat any non-empty value as "inside a sandbox". A strict 1/true + // check here misfires inside real sandboxes, where the helper would + // wrongly emit a "no sandbox" warning despite the run being contained. + if (env['SANDBOX']) return null; + if (isTruthyEnv(env['QWEN_CODE_SUPPRESS_YOLO_WARNING'])) return null; + return HEADLESS_YOLO_NO_SANDBOX_WARNING; +} + +function isTruthyEnv(val: string | undefined): boolean { + return val === '1' || val === 'true'; +} diff --git a/packages/cli/src/utils/runBudget.test.ts b/packages/cli/src/utils/runBudget.test.ts new file mode 100644 index 00000000000..6fa44823bb8 --- /dev/null +++ b/packages/cli/src/utils/runBudget.test.ts @@ -0,0 +1,227 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { + RunBudgetEnforcer, + parseDurationSeconds, + validateMaxToolCalls, + validateMaxWallTimeSetting, +} from './runBudget.js'; + +describe('parseDurationSeconds', () => { + it.each([ + ['90', 90], + ['90s', 90], + ['30S', 30], + [' 45 ', 45], + ['5m', 300], + ['1h', 3600], + ['1.5h', 5400], + ['1s', 1], + ])('parses %s as %d seconds', (input, expected) => { + expect(parseDurationSeconds(input)).toBeCloseTo(expected); + }); + + it.each(['', 'abc', '10x', '-5', '5 m s', 'NaN', '0', '0s', '0ms'])( + 'rejects invalid / non-positive input %s', + (input) => { + expect(() => parseDurationSeconds(input)).toThrow(); + }, + ); + + it('rejects sub-second budgets — they fire before any model round-trip', () => { + // Previously a tiny budget like `500ms` parsed cleanly and immediately + // aborted the run on the next event-loop tick. That's a typo, not a + // useful guardrail. + expect(() => parseDurationSeconds('500ms')).toThrow(/minimum/i); + expect(() => parseDurationSeconds('1ms')).toThrow(/minimum/i); + expect(() => parseDurationSeconds('0.5')).toThrow(/minimum/i); + }); + + it('rejects values larger than Node.js can safely time out on', () => { + // The regex doesn't accept `d`, so `100d` fails as a format error; + // `2400h` parses but exceeds MAX_WALL_TIME_SECONDS (~24.8d). + expect(() => parseDurationSeconds('100d')).toThrow(); + expect(() => parseDurationSeconds('2400h')).toThrow(); + }); +}); + +describe('validateMaxWallTimeSetting', () => { + it('accepts -1 (unlimited sentinel)', () => { + expect(validateMaxWallTimeSetting(-1)).toBe(-1); + }); + + it('accepts positive numbers at or above the 1s floor', () => { + expect(validateMaxWallTimeSetting(60)).toBe(60); + expect(validateMaxWallTimeSetting(1)).toBe(1); + }); + + it('rejects 0 (mirrors CLI flag behavior — 0 is a foot-gun)', () => { + expect(() => validateMaxWallTimeSetting(0)).toThrow(); + }); + + it('rejects sub-second values', () => { + expect(() => validateMaxWallTimeSetting(0.5)).toThrow(/minimum/i); + expect(() => validateMaxWallTimeSetting(0.001)).toThrow(/minimum/i); + }); + + it('rejects negatives other than -1', () => { + expect(() => validateMaxWallTimeSetting(-2)).toThrow(); + }); + + it('rejects Infinity and NaN', () => { + expect(() => + validateMaxWallTimeSetting(Number.POSITIVE_INFINITY), + ).toThrow(); + expect(() => validateMaxWallTimeSetting(Number.NaN)).toThrow(); + }); + + it('rejects values larger than the Node.js timeout ceiling', () => { + expect(() => validateMaxWallTimeSetting(3_000_000)).toThrow(); + }); +}); + +describe('validateMaxToolCalls', () => { + it('accepts -1 (unlimited sentinel)', () => { + expect(validateMaxToolCalls(-1)).toBe(-1); + }); + + it('accepts 0 (no-tool-calls-allowed sentinel)', () => { + // Asymmetric with wall-time where 0 is fatal — for tool-calls, 0 means + // "the first tick aborts", which is a legitimate "model must answer + // without invoking tools" mode. + expect(validateMaxToolCalls(0)).toBe(0); + }); + + it('accepts positive integers', () => { + expect(validateMaxToolCalls(5)).toBe(5); + expect(validateMaxToolCalls(1000)).toBe(1000); + }); + + it('rejects NaN — yargs coerces non-numeric flag values to NaN', () => { + // `qwen -p '...' --max-tool-calls abc` would otherwise silently + // disable the budget; the >= 0 gate in tickToolCall is false for NaN. + expect(() => validateMaxToolCalls(Number.NaN)).toThrow(); + }); + + it('rejects Infinity', () => { + expect(() => validateMaxToolCalls(Number.POSITIVE_INFINITY)).toThrow(); + }); + + it('rejects negatives other than -1', () => { + // `--max-tool-calls=-5` (typo for `5`) would otherwise silently + // disable the budget — the exact foot-gun the wall-time validator + // was built to prevent. + expect(() => validateMaxToolCalls(-5)).toThrow(); + expect(() => validateMaxToolCalls(-2)).toThrow(); + }); + + it('rejects fractional values', () => { + expect(() => validateMaxToolCalls(1.5)).toThrow(); + expect(() => validateMaxToolCalls(0.5)).toThrow(); + }); + + it('rejects values above the 1_000_000 ceiling (likely typo)', () => { + // `1e10` (10_000_000_000) parses as a valid integer in JS, would + // pass the `>= 0` gate forever, and silently disable the budget. + // Fail loud at startup. + expect(() => validateMaxToolCalls(1_000_001)).toThrow(); + expect(() => validateMaxToolCalls(1e10)).toThrow(); + }); +}); + +describe('RunBudgetEnforcer', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + afterEach(() => { + vi.useRealTimers(); + }); + + it('allows up to maxToolCalls calls, aborts on the (N+1)th', () => { + const ac = new AbortController(); + const enforcer = new RunBudgetEnforcer({ maxToolCalls: 1 }, ac); + enforcer.tickToolCall(); + expect(ac.signal.aborted).toBe(false); + enforcer.tickToolCall(); + expect(ac.signal.aborted).toBe(true); + const exceeded = enforcer.getExceeded(); + expect(exceeded?.kind).toBe('tool-calls'); + expect(exceeded?.limit).toBe(1); + expect(exceeded?.observed).toBe(2); + }); + + it('treats maxToolCalls=0 as "no tool calls allowed"', () => { + const ac = new AbortController(); + const enforcer = new RunBudgetEnforcer({ maxToolCalls: 0 }, ac); + enforcer.tickToolCall(); + expect(ac.signal.aborted).toBe(true); + expect(enforcer.getExceeded()?.kind).toBe('tool-calls'); + }); + + it('does not enforce when budget is -1 (unlimited)', () => { + const ac = new AbortController(); + const enforcer = new RunBudgetEnforcer({ maxToolCalls: -1 }, ac); + for (let i = 0; i < 50; i++) enforcer.tickToolCall(); + expect(ac.signal.aborted).toBe(false); + expect(enforcer.getExceeded()).toBeNull(); + }); + + it('fires wall-clock abort after maxWallTimeSeconds', () => { + const ac = new AbortController(); + const enforcer = new RunBudgetEnforcer({ maxWallTimeSeconds: 5 }, ac); + enforcer.start(); + vi.advanceTimersByTime(4999); + expect(ac.signal.aborted).toBe(false); + vi.advanceTimersByTime(2); + expect(ac.signal.aborted).toBe(true); + expect(enforcer.getExceeded()?.kind).toBe('wall-time'); + }); + + it('stop() cancels a pending wall-clock timer', () => { + const ac = new AbortController(); + const enforcer = new RunBudgetEnforcer({ maxWallTimeSeconds: 1 }, ac); + enforcer.start(); + enforcer.stop(); + vi.advanceTimersByTime(10_000); + expect(ac.signal.aborted).toBe(false); + expect(enforcer.getExceeded()).toBeNull(); + }); + + it('first-fence-wins: a later overrun does not clobber the original reason', () => { + const ac = new AbortController(); + const enforcer = new RunBudgetEnforcer( + { maxToolCalls: 0, maxWallTimeSeconds: 1 }, + ac, + ); + enforcer.start(); + enforcer.tickToolCall(); + vi.advanceTimersByTime(2000); + expect(enforcer.getExceeded()?.kind).toBe('tool-calls'); + }); + + it('does not record a budget reason when the controller was already aborted by a third party (SIGINT race)', () => { + const ac = new AbortController(); + const enforcer = new RunBudgetEnforcer({ maxToolCalls: 0 }, ac); + // Simulate SIGINT landing first: the shared abortController already + // fired before any budget tick. The enforcer must not retroactively + // claim the abort as a budget overrun. + ac.abort(); + enforcer.tickToolCall(); + expect(enforcer.getExceeded()).toBeNull(); + }); + + it('start() is idempotent — only one wall-clock timer is armed', () => { + const ac = new AbortController(); + const enforcer = new RunBudgetEnforcer({ maxWallTimeSeconds: 5 }, ac); + enforcer.start(); + enforcer.start(); + vi.advanceTimersByTime(5_001); + expect(ac.signal.aborted).toBe(true); + expect(enforcer.getExceeded()?.kind).toBe('wall-time'); + }); +}); diff --git a/packages/cli/src/utils/runBudget.ts b/packages/cli/src/utils/runBudget.ts new file mode 100644 index 00000000000..7af7414f5fe --- /dev/null +++ b/packages/cli/src/utils/runBudget.ts @@ -0,0 +1,305 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Run-level budget enforcement for headless / non-interactive Qwen Code + * sessions. See issue QwenLM/qwen-code#4103. + * + * Two budgets are enforced today: + * - `--max-wall-time` / `model.maxWallTimeSeconds` — clock-time guardrail + * for long-running unattended runs. + * - `--max-tool-calls` / `model.maxToolCalls` — bounds the cumulative + * number of tool executions (success or failure). + * + * `tickToolCall()` is invoked **before** each `executeToolCall` so that a + * budget of N caps the run at exactly N executions — the (N+1)th tick + * aborts before the work is performed. The wall-clock timer is started via + * `start()` and torn down by `stop()`. When any limit is exceeded the + * enforcer aborts the run via the shared `AbortController` and records the + * reason so the caller can emit a structured error envelope. + */ + +export type BudgetKind = 'wall-time' | 'tool-calls'; + +export interface BudgetExceeded { + kind: BudgetKind; + limit: number; + /** Observed value at the moment the budget was exceeded. */ + observed: number; + /** Human-readable message suitable for stderr / structured error output. */ + message: string; +} + +export interface RunBudgetOptions { + /** + * Wall-clock budget in seconds. Non-positive (`-1`, `0`, undefined) + * disables the budget; the CLI parser rejects `0` at the input layer so + * this enforcer never sees a legitimate "zero seconds" value. + */ + maxWallTimeSeconds?: number; + /** + * Max cumulative tool calls. `-1` / `undefined` disables; `0` is a valid + * budget meaning "no tool calls allowed" (the first tick aborts). + */ + maxToolCalls?: number; +} + +const SECOND = 1000; +/** + * Node clamps `setTimeout` delays >= 2^31 to 1 ms, which would fire the + * timer almost immediately. Reject upstream so a user typing `--max-wall-time + * 100d` gets a clear error instead of a confusing instant abort. + */ +const MAX_TIMEOUT_MS = 2_147_483_647; +const MAX_WALL_TIME_SECONDS = Math.floor(MAX_TIMEOUT_MS / SECOND); +/** + * Wall-clock budgets below 1s are almost always a typo (someone meant `1m` + * or `1h`); accepting them silently produces a run that aborts on the next + * event-loop tick before any model request returns. Round-trip latency to + * any reasonable LLM is multiple seconds, so a sub-second budget is also + * not a meaningful guardrail. Reject loudly. + */ +const MIN_WALL_TIME_SECONDS = 1; + +/** + * Parses a duration string used by `--max-wall-time`. + * + * Accepted forms (all must resolve to a duration in + * `[MIN_WALL_TIME_SECONDS, MAX_WALL_TIME_SECONDS]`): + * - plain number (interpreted as seconds): `"90"` → 90 + * - suffixed: `"30s"`, `"5m"`, `"1h"`, `"1.5h"`, `"3600s"` + * - `ms` suffix is syntactically accepted but rejected at the floor + * unless the value resolves to `>= 1s` (e.g. `"1000ms"` is legal, + * `"500ms"` is not) + * - case-insensitive suffix; whitespace tolerated + * + * Returns the duration in **seconds** for parity with `maxWallTimeSeconds` + * in settings.json. + * + * Throws on garbage input, on negative values (regex-rejected — no sign + * allowed), on zero, on sub-second values below `MIN_WALL_TIME_SECONDS`, + * and on values above `MAX_WALL_TIME_SECONDS`. A typo in a CI budget flag + * should fail loud at startup, not silently disable (or instant-fire) the + * guardrail. + */ +export function parseDurationSeconds(input: string): number { + const trimmed = input.trim().toLowerCase(); + if (trimmed.length === 0) { + throw new Error('Invalid duration: empty string'); + } + // The regex disallows a leading sign, so negatives short-circuit on + // structural mismatch — no explicit `< 0` check needed. + const match = /^(\d+(?:\.\d+)?)\s*(ms|s|m|h)?$/.exec(trimmed); + if (!match) { + throw new Error( + `Invalid duration "${input}". Use a positive number of seconds (e.g. 90) or a duration with unit (e.g. 30s, 5m, 1h, 500ms).`, + ); + } + const value = Number.parseFloat(match[1]); + const unit = match[2] ?? 's'; + let seconds: number; + switch (unit) { + case 'ms': + seconds = value / 1000; + break; + case 's': + seconds = value; + break; + case 'm': + seconds = value * 60; + break; + case 'h': + seconds = value * 3600; + break; + default: + // Unreachable given the regex, but keeps the type-checker honest. + throw new Error(`Invalid duration unit "${unit}"`); + } + if (seconds <= 0) { + throw new Error( + `Invalid duration "${input}": must be greater than zero. Omit the flag entirely if you don't want a wall-clock budget.`, + ); + } + if (seconds < MIN_WALL_TIME_SECONDS) { + // Only suggest a "did you mean" rewrite when the user actually + // used the `ms` suffix — for bare sub-second inputs like `0.5` or + // `0.5s`, the rewrite would be a no-op ("did you mean 0.5s?") and + // just confuses the error. + const hint = /ms\b/i.test(trimmed) + ? ` (probably a typo — did you mean ${input.replace(/ms\b/i, 's')}?)` + : ''; + throw new Error( + `Invalid duration "${input}": below the ${MIN_WALL_TIME_SECONDS}s minimum${hint}. Sub-second wall-clock budgets fire before any model round-trip can complete.`, + ); + } + if (seconds > MAX_WALL_TIME_SECONDS) { + throw new Error( + `Invalid duration "${input}": exceeds the maximum supported wall-clock budget (${MAX_WALL_TIME_SECONDS}s ≈ 24 days). Use a smaller value.`, + ); + } + return seconds; +} + +/** + * Validates a `maxWallTimeSeconds` value sourced from settings.json + * (as opposed to the CLI flag, which goes through `parseDurationSeconds`). + * + * The settings entry is a plain number, so the CLI's parser doesn't run. + * Mirror the same rejection rules here so `maxWallTimeSeconds: 0` in + * settings.json doesn't silently disable the budget (the enforcer treats + * `<= 0` as "no timer") while the equivalent `--max-wall-time 0` flag is + * fatal. Asymmetry would be a foot-gun. + * + * Returns the validated value, or `-1` for the "unlimited" sentinel. + */ +export function validateMaxWallTimeSetting(value: number): number { + if (value === -1) return -1; + if (!Number.isFinite(value)) { + throw new Error( + `model.maxWallTimeSeconds must be a finite number; got ${value}.`, + ); + } + if (value <= 0) { + throw new Error( + `model.maxWallTimeSeconds must be > 0 (or -1 for unlimited); got ${value}. ` + + `Use -1 to disable, not 0.`, + ); + } + if (value < MIN_WALL_TIME_SECONDS) { + throw new Error( + `model.maxWallTimeSeconds ${value} is below the ${MIN_WALL_TIME_SECONDS}s minimum. Sub-second budgets fire before any model round-trip can complete.`, + ); + } + if (value > MAX_WALL_TIME_SECONDS) { + throw new Error( + `model.maxWallTimeSeconds ${value} exceeds the maximum supported wall-clock budget (${MAX_WALL_TIME_SECONDS}s ≈ 24 days).`, + ); + } + return value; +} + +/** + * Upper bound for `maxToolCalls`. Above this, a value is almost certainly + * a typo (`1e10` meant `1e1`, or a misplaced zero): no realistic run + * executes a billion tool calls, and `tickToolCall`'s `>` gate would + * functionally never trip. Same fail-loud philosophy as `MAX_WALL_TIME_SECONDS`. + */ +const MAX_TOOL_CALLS = 1_000_000; + +/** + * Validates a `maxToolCalls` value sourced from either the `--max-tool-calls` + * CLI flag or `model.maxToolCalls` in settings.json. Mirrors + * `validateMaxWallTimeSetting`: the enforcer treats anything `< 0` as "no + * limit", so any non-`-1` negative would silently disable the budget. Reject + * up front to keep the fail-loud philosophy symmetric across all budgets. + * + * `0` IS legal here — it means "no tool calls allowed; first tick aborts" + * (asymmetric with wall-time where 0 is fatal). Documented in the schema. + */ +export function validateMaxToolCalls(value: number): number { + if (value === -1) return -1; + if (!Number.isFinite(value)) { + throw new Error(`maxToolCalls must be a finite number; got ${value}.`); + } + if (!Number.isInteger(value)) { + throw new Error( + `maxToolCalls must be an integer (or -1 for unlimited); got ${value}.`, + ); + } + if (value < 0) { + throw new Error( + `maxToolCalls must be >= 0 (or -1 for unlimited); got ${value}. Use -1 to disable, not a negative number.`, + ); + } + if (value > MAX_TOOL_CALLS) { + throw new Error( + `maxToolCalls ${value} exceeds the supported ceiling (${MAX_TOOL_CALLS}). Likely a typo — use a smaller value or -1 for unlimited.`, + ); + } + return value; +} + +export class RunBudgetEnforcer { + private readonly maxWallTimeSeconds: number; + private readonly maxToolCalls: number; + private readonly abortController: AbortController; + private wallTimer: ReturnType | null = null; + private toolCallCount = 0; + private exceeded: BudgetExceeded | null = null; + + constructor(opts: RunBudgetOptions, abortController: AbortController) { + this.maxWallTimeSeconds = opts.maxWallTimeSeconds ?? -1; + this.maxToolCalls = opts.maxToolCalls ?? -1; + this.abortController = abortController; + } + + /** + * Starts the wall-clock timer (if configured). Idempotent so callers + * don't need to thread "did I already start?" state. + */ + start(): void { + if (this.wallTimer !== null) return; + if (this.maxWallTimeSeconds <= 0) return; + this.wallTimer = setTimeout(() => { + this.markExceeded({ + kind: 'wall-time', + limit: this.maxWallTimeSeconds, + observed: this.maxWallTimeSeconds, + message: `Run aborted: wall-clock budget of ${this.maxWallTimeSeconds}s exceeded (--max-wall-time).`, + }); + }, this.maxWallTimeSeconds * SECOND); + // Don't keep the event loop alive solely for the timeout — once the + // main loop exits naturally we want the process to exit too. + (this.wallTimer as NodeJS.Timeout).unref?.(); + } + + /** Records one tool execution and enforces `maxToolCalls`. */ + tickToolCall(): void { + this.toolCallCount += 1; + if (this.maxToolCalls >= 0 && this.toolCallCount > this.maxToolCalls) { + this.markExceeded({ + kind: 'tool-calls', + limit: this.maxToolCalls, + observed: this.toolCallCount, + message: `Run aborted: tool-call budget of ${this.maxToolCalls} exceeded (--max-tool-calls); observed ${this.toolCallCount}.`, + }); + } + } + + /** + * Returns the budget-exceeded record if one fired, else null. The + * non-interactive loop checks this after `abortController.signal` + * fires to distinguish "budget abort" from "user SIGINT" so it can + * emit a structured-error envelope with the right reason. + */ + getExceeded(): BudgetExceeded | null { + return this.exceeded; + } + + /** Cancels the wall-clock timer. Safe to call multiple times. */ + stop(): void { + if (this.wallTimer !== null) { + clearTimeout(this.wallTimer); + this.wallTimer = null; + } + } + + private markExceeded(record: BudgetExceeded): void { + // First fence wins — once one budget has been recorded, subsequent + // overruns (e.g. an in-flight tool finishing after wall-time fired) + // don't clobber the original reason. + if (this.exceeded !== null) return; + // If the abort already happened from a different source (SIGINT, an + // external `options.abortController` shared with a parent), don't + // claim it as a budget event — otherwise the caller would emit exit + // code 55 ("budget exceeded") when the real cause was user + // cancellation (130). + if (this.abortController.signal.aborted) return; + this.exceeded = record; + this.stop(); + this.abortController.abort(); + } +} diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 733f8bab3e8..500f0dba5bd 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -593,6 +593,19 @@ export interface ConfigParameters { model?: string; outputLanguageFilePath?: string; maxSessionTurns?: number; + /** + * Wall-clock budget for an unattended run, in seconds. `-1` (default) + * means no limit. Enforced by the CLI's non-interactive run loop — + * see `RunBudgetEnforcer` in `packages/cli/src/utils/runBudget.ts`. + * Issue: QwenLM/qwen-code#4103. + */ + maxWallTimeSeconds?: number; + /** + * Cumulative tool-call budget across the entire run. `-1` means no + * limit. Counts every `executeToolCall` invocation (incl. failed + * tools, since the model is still consuming tokens reading the error). + */ + maxToolCalls?: number; clearContextOnIdle?: ClearContextOnIdleSettings; sessionTokenLimit?: number; experimentalZedIntegration?: boolean; @@ -884,6 +897,8 @@ export class Config { private ideMode: boolean; private readonly maxSessionTurns: number; + private readonly maxWallTimeSeconds: number; + private readonly maxToolCalls: number; private readonly clearContextOnIdle: ClearContextOnIdleSettings; private readonly sessionTokenLimit: number; private readonly listExtensions: boolean; @@ -1047,6 +1062,8 @@ export class Config { this.fileDiscoveryService = params.fileDiscoveryService ?? null; this.bugCommand = params.bugCommand; this.maxSessionTurns = params.maxSessionTurns ?? -1; + this.maxWallTimeSeconds = params.maxWallTimeSeconds ?? -1; + this.maxToolCalls = params.maxToolCalls ?? -1; this.clearContextOnIdle = { toolResultsThresholdMinutes: params.clearContextOnIdle?.toolResultsThresholdMinutes ?? 60, @@ -2164,6 +2181,14 @@ export class Config { return this.maxSessionTurns; } + getMaxWallTimeSeconds(): number { + return this.maxWallTimeSeconds; + } + + getMaxToolCalls(): number { + return this.maxToolCalls; + } + getClearContextOnIdle(): ClearContextOnIdleSettings { return this.clearContextOnIdle; } diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index f82734cd476..1791b478c58 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -181,6 +181,18 @@ export class FatalToolExecutionError extends FatalError { super(message, 54); } } +/** + * Raised when a headless / unattended run exceeds a configured budget + * (`--max-wall-time`, `--max-tool-calls`). Distinct exit code from + * `FatalTurnLimitedError` (53) so CI scripts can branch on + * "run exhausted its budget" vs. "run hit the turn cap." See issue + * QwenLM/qwen-code#4103. + */ +export class FatalBudgetExceededError extends FatalError { + constructor(message: string) { + super(message, 55); + } +} export class FatalCancellationError extends FatalError { constructor(message: string) { super(message, 130); // Standard exit code for SIGINT diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index 9ea83878b01..6167eb25dd3 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -458,6 +458,16 @@ "type": "number", "default": -1 }, + "maxWallTimeSeconds": { + "description": "Run-level wall-clock budget for headless / unattended runs, in seconds. -1 means unlimited; otherwise must be in [1, ~2,147,483] (sub-second values and values above ~24 days are rejected as typos). Overridable per-invocation via --max-wall-time (which also accepts duration suffixes like 5m, 1.5h).", + "type": "number", + "default": -1 + }, + "maxToolCalls": { + "description": "Cumulative tool-call budget for a run (counts every executed tool, success or failure; structured_output under --json-schema is exempt). -1 means unlimited; 0 means \"no tool calls allowed\" (first call aborts). Capped at 1,000,000 to catch typos. Overridable via --max-tool-calls.", + "type": "number", + "default": -1 + }, "chatCompression": { "description": "Chat compression settings.", "type": "object", @@ -473,7 +483,7 @@ "default": true }, "skipLoopDetection": { - "description": "Disable all loop detection checks (streaming and LLM).", + "description": "Skip streaming loop detection. Defaults to true to avoid false-positive interruptions; set to false to re-enable as an unattended-run guardrail.", "type": "boolean", "default": true },