Run read-only `shell` calls concurrently (conditionally downgrade from Risky)

[edwin-zvs]

Summary

shell is classified ToolRisk::Risky, so when a model turn emits several shell calls they execute sequentially. Only Safe tools are fanned out concurrently (futures::future::join_all); Risky tools run one-at-a-time through the approval gate. As a result, batched/parallel read-only shell calls (e.g. inspecting several files at once) can never run concurrently — even in unsafe_auto. This proposes letting read-only shell invocations be treated as Safe so they execute concurrently, mirroring the read-only inspection tools that already fan out.

Where this lives

• Per-turn partition + the two execution paths — crates/adapter-zarvis/src/agent.rs:
  • Safe bucket → join_all (concurrent), ~L797–819
  • Risky bucket → sequential for loop via run_one_tool (approval gate), ~L841–870
• shell risk = Risky — crates/adapter-zarvis/src/tools/shell.rs
• effective_risk's auto-approve downgrade only matches edit_file (path-scoped), never shell — crates/adapter-zarvis/src/tools/mod.rs (auto_approve_covers)
• unsafe_auto removes the approval prompt but does not change the Safe/Risky classification, so shell still serializes there too.

Motivation

Benchmarking zarvis vs the Codex CLI (same model + backend, only the harness differs), codex parallelizes ~27% of its turns — multiple exec_command reads at once — while zarvis stays ~1 tool/turn. There are two independent gaps:

1. the model rarely emits parallel calls under zarvis (separate, prompt/tool-surface issue), and
2. even when it does, zarvis serializes them because shell is Risky.

This issue is about (2), the execution side. Prior art: openai/codex PR #10505 ("Enable parallel shell tools") marks its shell tools supports_parallel_tool_calls = true so its executor runs them concurrently.

Proposal (options, not mutually exclusive)

1. Read-only command detection (recommended): treat a shell call as Safe only when its command is provably side-effect-free — an allowlist of cat/sed -n/rg/grep/ls/find/head/tail/wc/git log|diff|show, with no redirects, no $(...)/backticks, no ;/&&/| into a mutator. Default to Risky on any doubt.
2. Explicit read_only: true arg the model sets; only those fan out. Simpler than parsing, but trusts the model.
3. Reintroduce a dedicated Safe read tool — but zarvis: codex-style minimal tool surface (shell + edit_file + write_stdin) #329 deliberately removed read_file/list_dir for the codex-style minimal surface, so this partly reverses that.

Risks / why shell is Risky today

• shell runs arbitrary commands; concurrent mutating shell can race (a build vs rm, two writers to one file) and can't be gated one-by-one for approval. Any downgrade must be conservative — only provably read-only commands, default Risky.
• Naive parsing is fooled by command substitution, redirects, and chaining; the allowlist must reject those outright.
• Need a concurrency cap and sane interleaving of concurrent output.
• A blanket downgrade of shell to Safe is a non-starter — it would bypass approval for destructive commands in manual/auto_review.

Acceptance

• Independent read-only shell calls in one turn execute concurrently (duration test in the spirit of codex's tool_parallelism suite).
• Any non-read-only or ambiguous command stays Risky and serial.
• No change to approval semantics for mutating commands.

Note

This is the execution-side enabler only; it pays off in combination with the model actually emitting parallel shell calls, which under zarvis it currently mostly doesn't. Tracked/most valuable alongside that separate investigation.

🤖 Generated with Claude Code