mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-10 01:29:17 +00:00
331f45e907
* feat(cli): headless runaway-protection guardrails (#4103)
Adds two opt-in run-level budgets and a startup safety warning for
non-interactive / CI / SDK runs. All defaults preserve existing
behavior; the budgets only fire when the user explicitly sets a limit.
Phase 1 — surface unsafe configs and fix doc drift
- New `--yolo`-without-sandbox stderr warning at startup of every
non-interactive run, emitted by `getHeadlessYoloSafetyWarning` in
`packages/cli/src/utils/headlessSafetyWarnings.ts`. Suppressible
via `QWEN_CODE_SUPPRESS_YOLO_WARNING=1` (strict `1`/`true` match so
`=0` / `=false` don't silence it). Strict env match also applied to
the `SANDBOX` check so values like `SANDBOX=0` don't accidentally
bypass the warning.
- Gated on `!config.isInteractive()` at the gemini.tsx call site so
TUI users aren't nagged.
- `docs/users/configuration/settings.md`: corrected
`model.skipLoopDetection` default (`true`, not `false`) and reworded
the `--yolo`/sandbox section — `--yolo` does NOT auto-enable a
sandbox; sandboxing must still be opted into explicitly.
Phase 2 — run-level budgets with distinct exit code
- `--max-wall-time` / `model.maxWallTimeSeconds`: wall-clock duration
for the whole run. Flag accepts `90` (s), `30s`, `5m`, `1h`,
`500ms`. Settings is plain seconds.
- `--max-tool-calls` / `model.maxToolCalls`: cumulative tool
executions (success + failure). Ticked BEFORE each `executeToolCall`
so a budget of N caps the run at exactly N executions.
- New `FatalBudgetExceededError` (exit code 55), distinct from
`FatalTurnLimitedError` (53) and `FatalCancellationError` (130) so
CI scripts can branch on the reason. JSON output mirrors the
`handleMaxTurnsExceededError` / `handleCancellationError` envelope
convention.
- Enforced via `RunBudgetEnforcer` in
`packages/cli/src/utils/runBudget.ts`, wired to the same
`AbortController` as SIGINT so existing cancellation plumbing
carries the abort. A `routeAbort` helper distinguishes budget vs.
SIGINT at the abort-check sites and at the outer catch.
Critical correctness fixes (informed by the #4105 review pass)
- Drain-loop fall-through: the inner drain-item `for await` previously
exited via `finalizeAssistantMessage(); return;`, swallowing a
budget abort that fires during the last drain item and surfacing
exit code 0. Now routes through `routeAbort` so exit 55 is
preserved.
- Settings symmetry: `maxWallTimeSeconds: 0` in settings.json is now
rejected (same as `--max-wall-time 0`); the enforcer treats `<=0`
as "no timer" so silent disable would be a foot-gun.
`validateMaxWallTimeSetting` also rejects `Infinity` / `NaN`.
- `setTimeout` overflow: both parser paths reject durations above
`Math.floor((2^31 - 1) / 1000)s` (~24.8 days). Node clamps
oversized delays to 1ms and fires the timer almost immediately;
fail loud at startup instead.
- First-fence-wins + SIGINT race: `markExceeded` no-ops if the
controller was already aborted by a third party, so a budget tick
arriving after user SIGINT doesn't misattribute the abort to exit
code 55.
- Outer catch re-routes mid-stream `AbortError`s through the budget
handler so users see "Run aborted: …" instead of raw "AbortError".
Tests
- `runBudget.test.ts` (32 tests): parser happy / reject paths,
setting validator, post-increment off-by-one, `maxToolCalls=0`
meaning "disallowed", `-1` meaning unlimited, wall-clock under
fake timers, `stop()` cancels pending timer, idempotent `start()`,
first-fence-wins, SIGINT-race protection.
- `headlessSafetyWarnings.test.ts` (7 tests): YOLO + sandbox / env
matrix; strict-truthy `SANDBOX` check; suppression env.
- Pre-existing suites: `nonInteractiveCli.test.ts` (46),
`gemini.test.tsx` (23), `config/config.test.ts` (220),
`core/utils/errors.test.ts` (12), `core/config/config.test.ts`
(172) all green after picking up the new config getters / CliArgs
fields.
Backward compatibility
- All budgets default to `-1` (unlimited); existing CLI invocations
behave identically.
- New stderr warning only fires in the narrow YOLO-no-sandbox case,
with an explicit suppress env.
- New exit code 55 is purely additive; no existing exit codes change
meaning.
* fix(cli): address audit findings for headless guardrails (#4103, #4502)
Round-1 audit (3 angles × line-by-line + removed-behavior + cross-file)
plus an open-ended design pass surfaced eight correctness issues. This
commit lands all of them; the larger ACP / serve-mode structural items
are documented for follow-up.
Correctness fixes
- headlessSafetyWarnings: `SANDBOX` env check reverted to plain truthy.
The sandbox transport sets `SANDBOX` to `sandbox-exec` (macOS
seatbelt) or the container name (`qwen-code-sandbox`), neither of
which matches `isTruthyEnv`. The PR's strict-`1`/`true` check was
emitting the "no sandbox" warning INSIDE real sandboxes. Match the
rest of the codebase (sandboxConfig.ts, gemini.tsx, Footer.tsx,
prompts.ts, …) which all treat any non-empty value as "sandboxed".
- nonInteractiveCli main-loop abort: add `finalizeAssistantMessage()`
before `routeAbort()`. The drain-item loop already had it (PR #4502
Critical bug #1); the main loop was asymmetric — stream-json
consumers would see an unterminated `message_start` when a budget /
SIGINT abort landed mid-stream.
- nonInteractiveCli drain-loop `routeAbort`: also flush
`flushQueuedNotificationsToSdk(localQueue)` and
`finalizeOneShotMonitors()` before exiting. The old `return`-and-
fall-through path went through the outer holdback loop, which did
this flushing; switching to `routeAbort()` skipped it, so
`task_started` envelopes lost their paired `task_notification`.
- nonInteractiveCli catch handler: emit `adapter.emitResult({...})`
BEFORE `handleBudgetExceededError`, with the budget message as
`errorMessage` when budget tripped. Previously the budget handler
`process.exit(55)`ed before the adapter could emit a terminal
`result` envelope, so STREAM_JSON consumers never saw a stream
terminator on budget exits and hung waiting for one.
- runBudget: new `validateMaxToolCalls` mirrors
`validateMaxWallTimeSetting`. yargs coerces non-numeric flag values
(`--max-tool-calls abc`) to `NaN`, and the enforcer's `>= 0` gate
treats `NaN` and negatives as "no limit", silently disabling the
budget. Reject `NaN`, `Infinity`, fractional, and negative-other-
than-`-1` values at both flag and settings layers. `0` remains
legal (`first tick aborts`), unlike wall-time where 0 is fatal.
- runBudget: new `MIN_WALL_TIME_SECONDS = 1` floor. Previously
`--max-wall-time 500ms` parsed cleanly and aborted on the next
event-loop tick before any model round-trip — almost certainly a
typo (`5m`?) and not a useful guardrail at any rate.
- nonInteractiveCli `tickToolCall`: exempt `ToolNames.STRUCTURED_OUTPUT`.
Under `--json-schema` this is the terminal "I'm done" contract tool,
not real work. Without the exemption a budget-edge completion is
aborted as a false positive (model used N tools then emitted
structured_output as call N+1 → exit 55 instead of success).
- commands/serve.ts: emit the YOLO-no-sandbox warning at daemon
startup when settings.json statically configures
`tools.approvalMode: 'yolo'` with no `tools.sandbox` /
`SANDBOX` env. The daemon can't use `getHeadlessYoloSafetyWarning`
(no Config yet — sessions get their own) so we re-derive the
predicate from settings. Per-session ACP override is documented as
out of scope.
Documentation
- `docs/users/features/headless.md`: new "Scope" subsection under
Run-level budgets explaining (a) `--max-tool-calls` counts top-level
dispatches only — subagent / `agent` tool inner calls are not
counted, (b) `structured_output` is exempt, (c) stream-json input
mode resets budgets per user message, (d) `qwen serve` / ACP
sessions do not currently consult budgets from settings.json.
Tests
- `runBudget.test.ts` grows from 32 → 41 tests: `validateMaxToolCalls`
(NaN / Infinity / negatives / fractional), `parseDurationSeconds`
sub-second rejection, `validateMaxWallTimeSetting` sub-second
rejection.
- `headlessSafetyWarnings.test.ts`: replaced the "still warns when
SANDBOX is 0/false/no" case (which encoded the strict-check bug) with
positive coverage for the real sandbox-set values
(`sandbox-exec`, `qwen-code-sandbox`).
All previously-green suites still green: cli/nonInteractiveCli (46),
cli/gemini.test (23), cli/config/config.test (220), core/utils/errors
(12), core/config/config.test (172). 337 tests across the touched suites.
Won't-fix (out of scope, documented or pre-existing)
- Unpaired `tool_use` in stream-json when a tool is aborted mid-execution
— pre-existing structural gap (SIGINT mid-tool has the same outcome);
PR amplifies it but doesn't introduce it.
- Narrow SIGINT-vs-budget-timer race — already mitigated by
`markExceeded`'s `signal.aborted` check.
- `tickToolCall` increments past abort (cosmetic; only affects the
`observed` value in the error envelope for a pathological caller).
* fix(cli): round-2 audit fixes for headless guardrails (#4103, #4502)
Round-2 audit (after round-1 commit
|
||
|---|---|---|
| .. | ||
| design | ||
| developers | ||
| e2e-tests | ||
| plans | ||
| superpowers/plans | ||
| users | ||
| _meta.ts | ||
| index.md | ||