From 6b18b3ebc8c11a712324483ef7da01c260b10c57 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Mon, 7 Sep 2026 12:10:38 +0000 Subject: [PATCH 1/2] Start every agent in full auto, not just claude `aid --claude` has run with `--dangerously-skip-permissions` since the Python build. `aid --codex` and `aid --gemini` ran their CLIs bare, so codex stopped at its first approval prompt and gemini at its first tool call, and an `aid owner/repo fix the bug` on either opened a workspace, printed a question and waited for somebody who had already walked away. The reason for claude's flag was never claude-specific. It is that the agent is already inside a disposable container holding only this repo, so the per-tool prompts buy nothing, and that is true of every row in the table. So the table now carries a full-auto flag per agent: `--dangerously-skip-permissions` for claude, `--dangerously-bypass-approvals-and-sandbox` for codex, `--yolo` for gemini. **codex gets the bypass and not `--full-auto`.** That is the trap here, because `--full-auto` reads like the answer: it approves every action, and it keeps codex's own sandbox, which is workspace-write with the network off. `gh`, `cargo fetch` and `pip install` would fail inside a container that has a network and a checkout the agent is meant to be able to push from. The container is already the sandbox, so a second one nested inside it removes exactly the capabilities `dl` provisioned. Held as a rule, not as three assertions. `every_agent_starts_in_full_auto` reads `AGENTS` and asserts its own length against the list of spellings it expects, so an agent added without a flag fails there rather than in somebody's unattended run. The spellings are named rather than pattern-matched, because `--full-auto` matches any pattern loose enough to cover three CLIs and picking it would be a silent regression. The full-auto table added to docs/cli.md is a second hand-maintained copy of that fact, so it gets the guard this repository requires beside one: `the_full_auto_section_names_the_flags_each_agent_is_actually_started_with` parses the table under the heading and diffs each row against the command the devpod shim receives, which is a launch rather than another copy of the list. Verified to bite by rewriting the codex row to `--full-auto` and watching it fail. The README paragraph that said the trade applied "to `aid` starting `claude` and nothing else" is now wrong and says so. Closes #584 --- CHANGELOG.md | 38 ++++++++++++++++ README.md | 18 +++++--- docs/cli.md | 43 ++++++++++++++++++ rust/aid/src/rewrite.rs | 74 ++++++++++++++++++++++++++----- rust/aid/tests/rewrite.rs | 93 +++++++++++++++++++++++++++++++++++++-- 5 files changed, 244 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0e74f6d..8f1059c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **`aid --codex` and `aid --gemini` now start in full auto, which only + `aid --claude` ever did.** The other two ran their CLIs bare, so codex stopped at + its first approval prompt and gemini at its first tool call: an + `aid owner/repo fix the bug` opened a workspace, printed a question and waited for + somebody who had already walked away. The reason for claude's + `--dangerously-skip-permissions` was never claude-specific. It is that the agent + is already inside a disposable container holding only this repo, so the per-tool + prompts buy nothing and stall the run, and that is true of every agent `aid` + starts. + + One rule, three spellings, because each CLI has its own: + + | Agent | What `aid` runs | + | --- | --- | + | `claude` | `claude --dangerously-skip-permissions`, with `IS_SANDBOX=1` beside it | + | `codex` | `codex --dangerously-bypass-approvals-and-sandbox` | + | `gemini` | `gemini --yolo` | + + **codex gets the bypass and not its `--full-auto`**, which is the trap in this + change. `--full-auto` reads like the answer: it approves every action. It also + keeps codex's own sandbox, which is workspace-write with the network off, so `gh`, + `cargo fetch` and `pip install` would fail inside a container that has a network + and a checkout the agent is meant to be able to push from. The container is + already the sandbox; nesting a second one inside it subtracts exactly the + capabilities `dl` went to the trouble of provisioning. + + Held as a rule rather than as three assertions. `every_agent_starts_in_full_auto` + reads the agent table and fails on a row that has no flag, so an agent added later + without one breaks a test instead of stopping somebody's unattended run. The + full-auto table now in `docs/cli.md` is a second hand-maintained copy of that + fact, so `the_full_auto_section_names_the_flags_each_agent_is_actually_started_with` + diffs the page against the command devpod receives. + + Unchanged: a command you typed yourself. `dl -- codex` runs codex, exactly as + written, with nothing added. + ## [0.32.0] - 2026-09-04 ### Added diff --git a/README.md b/README.md index 1a564301..c0274064 100644 --- a/README.md +++ b/README.md @@ -344,11 +344,15 @@ the question and launches one-shot, so scripts behave as they always have. | `--devcontainer ` | Passed through to `dl` | | `--claude-profile ` | Passed through to `dl`: which host Claude login to forward. Not the claude.ai account the container's `claude` is paired to for Remote Control | -**The trade, stated plainly.** `claude` starts with `--dangerously-skip-permissions`, because the -agent is already inside a disposable container holding only this repo, and the per-tool prompts -would stall an unattended run. `IS_SANDBOX=1` rides along because `claude` otherwise refuses that -flag under `uid 0`, and devcontainers that run as root are ordinary. That variable is scoped to -the agent process and is not exported into your shell. +**The trade, stated plainly.** Every agent starts in full auto, because it is already inside a +disposable container holding only this repo and the per-tool prompts would stall an unattended +run. Each CLI spells that its own way: `claude --dangerously-skip-permissions`, +`codex --dangerously-bypass-approvals-and-sandbox`, `gemini --yolo`. codex gets the bypass rather +than its `--full-auto`, which approves every action but keeps codex's own sandbox, and that +sandbox has no network, so `gh` and `cargo fetch` would fail inside a container that has one. +`IS_SANDBOX=1` rides along for `claude` because it otherwise refuses its flag under `uid 0`, and +devcontainers that run as root are ordinary. That variable is scoped to the agent process and is +not exported into your shell. The agent cannot reach your host, but it can rewrite the checkout it is in. Review an `aid` workspace before pushing rather than treating it as a sandbox that will stop the agent for you. @@ -357,8 +361,8 @@ session is also drivable from the claude.ai account signed in inside the contain `DEVLAUNCH_AID_REMOTE_CONTROL=0` turns that off for every launch; [docs/cli.md](docs/cli.md) has the rest. -This applies to `aid` starting `claude` and nothing else. `--codex` and `--gemini` are unaffected, -and `dl -- claude` runs exactly what you typed. +This applies to agents `aid` starts, and nothing else. `dl -- claude` runs exactly what you +typed. The agent's CLI has to be in the container already. `aid` runs it; it does not install it. diff --git a/docs/cli.md b/docs/cli.md index d95e4f72..2fc7553f 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -461,6 +461,49 @@ strings `render.rs` and `lib.rs` own, so `the_force_placement_section_quotes_the_refusals_it_says_it_does` reads this section back and diffs them against what the binary prints. +## Full auto: every agent, every launch + +`aid` exists to hand a repo to an agent and walk away, so every agent it starts is +started in that agent's full auto mode. There is no flag to type and no flag to +type differently per agent: + +| Agent | What `aid` runs | +| --- | --- | +| `claude` | `claude --dangerously-skip-permissions`, with `IS_SANDBOX=1` beside it | +| `codex` | `codex --dangerously-bypass-approvals-and-sandbox` | +| `gemini` | `gemini --yolo` | + +One rule, three spellings, and the table in `rust/aid/src/rewrite.rs` is where they +live. `every_agent_starts_in_full_auto` holds the rule against that table rather +than against one row of it, so an agent added without its flag fails a test instead +of stopping someone's unattended run to ask about its first edit. + +Two of those rows have a reason worth reading. + +**codex gets the bypass, not `--full-auto`.** codex offers both and only one of them +is this. `--full-auto` approves every action but keeps codex's own sandbox, which is +workspace write with the network off. That would break `gh`, `cargo fetch` and +`pip install` inside a container that has a network and a checkout the agent is +meant to be able to push from. The container is already the sandbox, so a second one +nested inside it subtracts exactly the capabilities `dl` went to the trouble of +provisioning. + +**`IS_SANDBOX=1` is what makes claude's flag usable.** claude refuses +`--dangerously-skip-permissions` outright under `uid 0`, exiting 1 with "cannot be +used with root/sudo privileges", and devcontainers that run as root are ordinary. +The variable is claude's own way of being told the refusal is answering for a +machine that is not there. It is scoped to the agent process and is not exported +into your shell. + +What this buys and what it costs is the same sentence: the agent will not stop to +ask, so an `aid owner/repo fix the bug` can run to the end unattended, and it can +also rewrite the checkout it is in without asking. It cannot reach your host. Review +an `aid` workspace before pushing rather than treating it as something that will +stop the agent for you. + +None of this reaches a command you typed yourself. `dl -- claude` runs claude, +exactly as written, with no flags added and no variables set. + ## Remote Control: every `aid` session, on your phone too Every `aid` launch of claude starts with Claude Code's Remote Control on. There is no diff --git a/rust/aid/src/rewrite.rs b/rust/aid/src/rewrite.rs index e804cf36..96544a1d 100644 --- a/rust/aid/src/rewrite.rs +++ b/rust/aid/src/rewrite.rs @@ -47,10 +47,23 @@ pub(crate) const AGENT_ENV_VAR: &str = "DEVLAUNCH_AID_AGENT"; /// quoted argument; each of these CLIs takes an initial prompt that way and then /// drops into its interactive session. /// -/// claude is started with `--dangerously-skip-permissions`: the whole point of a dl -/// workspace is that the agent is already inside a disposable container with only -/// this repo in it, so the per-tool prompts it would otherwise ask on the host buy -/// nothing and stop an unattended `aid owner/repo fix the bug` dead. +/// **Every agent here is started in its own full-auto mode**, because the whole +/// point of a dl workspace is that the agent is already inside a disposable +/// container with only this repo in it, so the per-tool prompts it would otherwise +/// ask on the host buy nothing and stop an unattended `aid owner/repo fix the bug` +/// dead. The flag is per-CLI and each one spells it its own way: +/// `--dangerously-skip-permissions` for claude, +/// `--dangerously-bypass-approvals-and-sandbox` for codex, `--yolo` for gemini. +/// Held by `every_agent_starts_in_full_auto`. +/// +/// codex is the one worth spelling out, because it offers two modes and only one of +/// them is this one. `--full-auto` reads like the answer and is not: it approves +/// every action but keeps codex's own sandbox, which is workspace-write with the +/// network off, so `gh`, `cargo fetch` and `pip install` fail inside a container +/// that has a network and a checkout the agent is meant to be able to push from. +/// The bypass flag is the analogue of claude's, and it is the honest one here: the +/// container is the sandbox, so a second one nested inside it subtracts exactly the +/// capabilities dl went to the trouble of provisioning. /// /// `IS_SANDBOX=1` is what makes that flag usable at all here. claude refuses it /// outright under uid 0 — "cannot be used with root/sudo privileges", exit 1 — and @@ -111,7 +124,7 @@ const AGENTS: &[(&str, Agent)] = &[ ( "codex", Agent { - command: &["codex"], + command: &["codex", "--dangerously-bypass-approvals-and-sandbox"], prompt_flags: &[], env: &[], remote_control: None, @@ -120,7 +133,7 @@ const AGENTS: &[(&str, Agent)] = &[ ( "gemini", Agent { - command: &["gemini"], + command: &["gemini", "--yolo"], prompt_flags: &["--prompt-interactive"], env: &[], remote_control: None, @@ -1045,12 +1058,47 @@ mod tests { fn gemini_gets_its_interactive_flag_only_beside_a_prompt() { assert_eq!( build_agent_command("gemini", "hi", None).as_deref(), - Some("gemini --prompt-interactive hi") + Some("gemini --yolo --prompt-interactive hi") ); assert_eq!( build_agent_command("gemini", "", None).as_deref(), - Some("gemini") + Some("gemini --yolo") + ); + } + + #[test] + fn every_agent_starts_in_full_auto() { + // The rule the table's own doc comment states, held against the table + // rather than against one row of it, so an agent added without its + // full-auto flag fails here instead of stopping an unattended run to ask + // about its first edit. Each CLI spells the flag its own way, so the + // spellings are named rather than pattern-matched -- codex in particular + // has a second mode, `--full-auto`, that approves everything but keeps its + // own network-less sandbox, and picking that one would be a silent + // regression rather than a visible one. + let expected = [ + ("claude", "--dangerously-skip-permissions"), + ("codex", "--dangerously-bypass-approvals-and-sandbox"), + ("gemini", "--yolo"), + ]; + assert_eq!( + expected.len(), + AGENTS.len(), + "an agent was added or removed without saying how it starts unattended" ); + for (agent, flag) in expected { + let command = build_agent_command(agent, "hi", None).expect("a known agent"); + assert!( + command.contains(flag), + "{agent} is not in full auto: {command}" + ); + assert!( + build_agent_command(agent, "", None) + .expect("a known agent") + .contains(flag), + "{agent} is not in full auto without a prompt" + ); + } } #[test] @@ -1419,11 +1467,15 @@ mod tests { } assert_eq!( build_dl_args(&parsed(&["--codex", "owner/repo", "hi"])).expect("a known agent"), - ["owner/repo", "--", "codex hi"] + [ + "owner/repo", + "--", + "codex --dangerously-bypass-approvals-and-sandbox hi" + ] ); assert_eq!( build_dl_args(&parsed(&["--gemini", "owner/repo", "hi"])).expect("a known agent"), - ["owner/repo", "--", "gemini --prompt-interactive hi"] + ["owner/repo", "--", "gemini --yolo --prompt-interactive hi"] ); // And the same through the variable, which is how somebody who set it once // launches every line. @@ -1825,7 +1877,7 @@ mod tests { [ "owner/repo", "--", - "gemini --prompt-interactive 'explain this'" + "gemini --yolo --prompt-interactive 'explain this'" ] ); } diff --git a/rust/aid/tests/rewrite.rs b/rust/aid/tests/rewrite.rs index e4eacc26..1cdbdf60 100644 --- a/rust/aid/tests/rewrite.rs +++ b/rust/aid/tests/rewrite.rs @@ -432,13 +432,16 @@ fn the_detached_cache_refresh_reaches_dl_through_aids_own_name() { #[test] fn each_agent_is_started_the_way_its_own_cli_takes_a_prompt() { // gemini's initial prompt is a flag that is a syntax error without one, so the - // flag only appears beside a prompt; codex takes neither a flag nor a variable. + // flag only appears beside a prompt; codex takes no prompt flag and no + // variable. Each line also carries the agent's full-auto flag, which is the + // spelling of one rule in three CLIs and is held as a rule by + // `every_agent_starts_in_full_auto` (rust/aid/src/rewrite.rs). let world = World::with(&["--warm"]); world.aid(&["--gemini", MAIN, "explain", "this"]).exited(0); assert_eq!( world.devpod_calls().last().expect("a session"), &format!( - "devpod ssh {MAIN} --command bash -lc 'gemini --prompt-interactive '\"'\"'explain this'\"'\"''" + "devpod ssh {MAIN} --command bash -lc 'gemini --yolo --prompt-interactive '\"'\"'explain this'\"'\"''" ) ); @@ -446,14 +449,16 @@ fn each_agent_is_started_the_way_its_own_cli_takes_a_prompt() { bare.aid(&["--gemini", MAIN]).exited(0); assert_eq!( bare.devpod_calls().last().expect("a session"), - &format!("devpod ssh {MAIN} --command bash -lc gemini") + &format!("devpod ssh {MAIN} --command bash -lc 'gemini --yolo'") ); let codex = World::with(&["--warm"]); codex.aid(&["--codex", MAIN, "hi"]).exited(0); assert_eq!( codex.devpod_calls().last().expect("a session"), - &format!("devpod ssh {MAIN} --command bash -lc 'codex hi'") + &format!( + "devpod ssh {MAIN} --command bash -lc 'codex --dangerously-bypass-approvals-and-sandbox hi'" + ) ); } @@ -560,3 +565,83 @@ fn a_remote_agent_that_failed_ends_with_the_agents_status() { let world = World::with(&["--warm", "--remote-exit"]); world.aid(&[MAIN, "boom"]).exited(130); } + +/// The full-auto table `docs/cli.md` prints, held to the lines `aid` actually runs. +/// +/// The "Full auto: every agent, every launch" section writes each agent's flag into +/// a table. That is a hand-maintained copy of a fact owned by `rewrite.rs`'s agent +/// table, and this repository allows a second copy only with a test beside it that +/// diffs it against the first. `every_agent_starts_in_full_auto` in +/// `rust/aid/src/rewrite.rs` is not that test: it pins the *behaviour*, and would +/// still pass with a flag changed and the page left naming the old one. +/// +/// Diffed against the command as devpod receives it, rather than against the source +/// table, so the page is checked against what a launch does and not against another +/// copy of the same list. Every row is required to name an agent this build knows, +/// which is what stops the guard passing on a table that has quietly lost a row. +#[test] +fn the_full_auto_section_names_the_flags_each_agent_is_actually_started_with() { + let doc = std::fs::read_to_string(repo_root().join("docs/cli.md")).expect("docs/cli.md"); + let rows = full_auto_rows(&doc); + assert_eq!( + rows.len(), + 3, + "docs/cli.md's full-auto table has {} rows, not one per agent: {rows:?}", + rows.len() + ); + + for (agent, flag) in rows { + let world = World::with(&["--warm"]); + world.aid(&[&format!("--{agent}"), MAIN]).exited(0); + let command = world + .devpod_calls() + .last() + .unwrap_or_else(|| panic!("`aid --{agent}` opened no session")) + .clone(); + + assert!( + command.contains(&flag), + "docs/cli.md says `{agent}` runs with {flag:?}; the launch is {command:?}" + ); + } +} + +/// The `(agent, flag)` pairs the full-auto table states. +/// +/// Matched on the heading rather than on a phrase under it, so the prose around the +/// table stays free to be rewritten while this test keeps pointing at one span. A +/// missing heading says so rather than yielding an empty section that every +/// assertion passes over. +/// +/// The agent is the first cell's single backticked word; the flag is the first +/// `--word` in the second cell, which is the whole of what the copy has to get +/// right. The rest of the cell is prose and is deliberately not read. +fn full_auto_rows(document: &str) -> Vec<(String, String)> { + const HEADING: &str = "## Full auto: every agent, every launch"; + let start = document + .find(HEADING) + .unwrap_or_else(|| panic!("docs/cli.md no longer has a '{HEADING}' section")); + let rest = &document[start + HEADING.len()..]; + let section = match rest.find("\n## ") { + Some(end) => &rest[..end], + None => rest, + }; + + section + .lines() + .filter_map(|line| { + let mut cells = line.trim().strip_prefix('|')?.split('|'); + let agent = cells.next()?.trim().trim_matches('`').to_owned(); + if !["claude", "codex", "gemini"].contains(&agent.as_str()) { + return None; + } + let flag = cells + .next()? + .split_whitespace() + .map(|word| word.trim_end_matches(|c: char| !c.is_ascii_alphanumeric())) + .find(|word| word.starts_with("--"))? + .to_owned(); + Some((agent, flag)) + }) + .collect() +} From 659428b5138b23fb99ba910899017718f5a73a5b Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Mon, 7 Sep 2026 12:26:58 +0000 Subject: [PATCH 2/2] Close the holes review found in both full-auto guards Four findings from Sourcery and a fresh-context review of 6b18b3e, none in the behaviour and all in what was written to hold it. **Both guards counted rows where they had to compare names.** `every_agent_starts_in_full_auto` asserted `expected.len()` against `AGENTS.len()`, which never checks that `expected` *covers* the table: dropping claude's flag and duplicating codex's row is still three rows beside a three-row table, and the review demonstrated it passing with claude launching bare. The docs guard had the same shape, and Sourcery broke it on sight. Both now diff the sorted set of names, which subsumes the count. **Both compared flags by substring.** Every truncation of a flag is a substring of it and so is every flag that merely starts with one, so a docs table reflowed down to `--d` passed, and `command: &["gemini", "--yolo-dry-run"]` passed a `contains("--yolo")` while asking gemini for the opposite of full auto. Both now compare whole argv words. This is the trap `the_force_placement_section_quotes_the_refusals_it_says_it_does` already recorded for the same reason, and the comments now cite each other. `full_auto_rows`'s `["claude", "codex", "gemini"]` allow-list was itself a fourth copy of the agent list, and worse than a copy: it *skipped* any row naming a fourth agent, so that row's flag went unchecked. Rows are now recognised by shape, a single backticked word in the first cell, and an unknown name reaches the caller and fails at the launch. The backticks are what separate a row from the `| --- |` separator, whose cells begin `--` and parsed as an agent named `---`. **`--full-auto` does not approve every action, and saying it did conceded the argument.** It is an approval policy plus a sandbox, not an absence of approvals, so it still escalates to a person (openai/codex#6522, openai/codex#4847). That is the *strongest* reason not to use it here, because stopping to ask is the whole of what #584 is about, and the prose had thrown it away and kept only the sandbox half. Corrected in the table's doc comment, docs/cli.md, the changelog and the README. Only `--dangerously-bypass-approvals-and-sandbox` sets the policy to never ask. **The README was a third hand-maintained copy of the flags with no guard on it.** The review made all three wrong and both suites stayed green, which is fair: the PR invoked the standing rule to justify a guard for docs/cli.md and then left the same fact next door unguarded. Rather than add a fourth guard, the README now states the rule and links the docs table for the spellings, keeping claude's flag because the `IS_SANDBOX=1` paragraph is about it. That is what "README orients, docs explains" asks for anyway, and the new anchor is checked by test_docs_links. Also: the docs table was headed "What `aid` runs" while showing less than a launch does, since a default claude line also carries `CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1` and `--remote-control=`. It is headed "Full-auto flag" now and says what it omits. docs/cli.md's opening paragraph enumerates the page's sections and had not been extended for the new one. Not verifiable here and stated rather than claimed: neither codex nor gemini is installed in this container, so `gemini --yolo --prompt-interactive ` is checked as a string the fake devpod records and not as a process that ran. --- CHANGELOG.md | 35 ++++++++++++++---------- README.md | 12 ++++----- docs/cli.md | 24 ++++++++++++----- rust/aid/src/rewrite.rs | 56 ++++++++++++++++++++++++--------------- rust/aid/tests/rewrite.rs | 49 +++++++++++++++++++++++++++------- 5 files changed, 117 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f1059c9..b3c97e5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,26 +21,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 One rule, three spellings, because each CLI has its own: - | Agent | What `aid` runs | + | Agent | Full-auto flag | | --- | --- | - | `claude` | `claude --dangerously-skip-permissions`, with `IS_SANDBOX=1` beside it | - | `codex` | `codex --dangerously-bypass-approvals-and-sandbox` | - | `gemini` | `gemini --yolo` | + | `claude` | `--dangerously-skip-permissions`, with `IS_SANDBOX=1` beside it | + | `codex` | `--dangerously-bypass-approvals-and-sandbox` | + | `gemini` | `--yolo` | **codex gets the bypass and not its `--full-auto`**, which is the trap in this - change. `--full-auto` reads like the answer: it approves every action. It also - keeps codex's own sandbox, which is workspace-write with the network off, so `gh`, - `cargo fetch` and `pip install` would fail inside a container that has a network - and a checkout the agent is meant to be able to push from. The container is - already the sandbox; nesting a second one inside it subtracts exactly the - capabilities `dl` went to the trouble of provisioning. + change, for two reasons of which the first is the one that matters: `--full-auto` + still escalates to a person. It is an approval policy plus a sandbox rather than an + absence of approvals, so an unattended run stops and asks, which is the whole of + the failure being fixed here. Only `--dangerously-bypass-approvals-and-sandbox` + sets the policy to never ask. The second reason is the sandbox `--full-auto` keeps: + workspace-write with the network off, so `gh`, `cargo fetch` and `pip install` + would fail inside a container that has a network and a checkout the agent is meant + to be able to push from. The container is already the sandbox; nesting a second one + inside it subtracts exactly the capabilities `dl` went to the trouble of + provisioning. Held as a rule rather than as three assertions. `every_agent_starts_in_full_auto` - reads the agent table and fails on a row that has no flag, so an agent added later - without one breaks a test instead of stopping somebody's unattended run. The - full-auto table now in `docs/cli.md` is a second hand-maintained copy of that + diffs the *names* it expects against the agent table's keys, so an agent added + later without a flag breaks a test instead of stopping somebody's unattended run. + The full-auto table now in `docs/cli.md` is a second hand-maintained copy of that fact, so `the_full_auto_section_names_the_flags_each_agent_is_actually_started_with` - diffs the page against the command devpod receives. + diffs the page against the command devpod receives. Both compare flags as whole + argv words: every truncation of a flag is a substring of it, and so is every flag + that merely starts with one, so `--yolo-dry-run` satisfied a substring test while + asking gemini for the opposite of what the page promised. Unchanged: a command you typed yourself. `dl -- codex` runs codex, exactly as written, with nothing added. diff --git a/README.md b/README.md index c0274064..ef446a0f 100644 --- a/README.md +++ b/README.md @@ -346,13 +346,11 @@ the question and launches one-shot, so scripts behave as they always have. **The trade, stated plainly.** Every agent starts in full auto, because it is already inside a disposable container holding only this repo and the per-tool prompts would stall an unattended -run. Each CLI spells that its own way: `claude --dangerously-skip-permissions`, -`codex --dangerously-bypass-approvals-and-sandbox`, `gemini --yolo`. codex gets the bypass rather -than its `--full-auto`, which approves every action but keeps codex's own sandbox, and that -sandbox has no network, so `gh` and `cargo fetch` would fail inside a container that has one. -`IS_SANDBOX=1` rides along for `claude` because it otherwise refuses its flag under `uid 0`, and -devcontainers that run as root are ordinary. That variable is scoped to the agent process and is -not exported into your shell. +run. Each CLI spells that its own way, and +[docs/cli.md](docs/cli.md#full-auto-every-agent-every-launch) has the flag per agent. `claude`'s +is `--dangerously-skip-permissions`, with `IS_SANDBOX=1` beside it because it otherwise refuses +that flag under `uid 0` and devcontainers that run as root are ordinary. That variable is scoped +to the agent process and is not exported into your shell. The agent cannot reach your host, but it can rewrite the checkout it is in. Review an `aid` workspace before pushing rather than treating it as a sandbox that will stop the agent for you. diff --git a/docs/cli.md b/docs/cli.md index 2fc7553f..bd73dc68 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -4,7 +4,8 @@ selector decides what you picked, which verb refreshes git state and which only touch the container, which commands get a terminal, what `--rm` promises and where it stops, which exits fire it, the spellings that were retired and what they say -now, what `aid`'s Remote Control default starts and how to turn it off, what +now, which full-auto flag `aid` starts each agent with and why codex gets the one it +gets, what `aid`'s Remote Control default starts and how to turn it off, what `kill` does to a workspace that will not answer, and what happens when devpod is missing, will not answer, or injects the wrong agent binary. @@ -467,11 +468,16 @@ section back and diffs them against what the binary prints. started in that agent's full auto mode. There is no flag to type and no flag to type differently per agent: -| Agent | What `aid` runs | +| Agent | Full-auto flag | | --- | --- | -| `claude` | `claude --dangerously-skip-permissions`, with `IS_SANDBOX=1` beside it | -| `codex` | `codex --dangerously-bypass-approvals-and-sandbox` | -| `gemini` | `gemini --yolo` | +| `claude` | `--dangerously-skip-permissions`, with `IS_SANDBOX=1` beside it | +| `codex` | `--dangerously-bypass-approvals-and-sandbox` | +| `gemini` | `--yolo` | + +The flag and not the whole command line, which is longer than one column: a default +`claude` launch also carries `CLAUDE_CODE_DISABLE_TERMINAL_TITLE=1` and a +`--remote-control=`, and gemini takes its initial prompt through +`--prompt-interactive`. One rule, three spellings, and the table in `rust/aid/src/rewrite.rs` is where they live. `every_agent_starts_in_full_auto` holds the rule against that table rather @@ -481,8 +487,12 @@ of stopping someone's unattended run to ask about its first edit. Two of those rows have a reason worth reading. **codex gets the bypass, not `--full-auto`.** codex offers both and only one of them -is this. `--full-auto` approves every action but keeps codex's own sandbox, which is -workspace write with the network off. That would break `gh`, `cargo fetch` and +is this, for two reasons of which the first is the one that matters: `--full-auto` +still escalates to a person. It is an approval policy plus a sandbox rather than an +absence of approvals, so an unattended run stops and asks, which is the whole of +what this rule exists to prevent. Only `--dangerously-bypass-approvals-and-sandbox` +sets the policy to never ask. The second reason is the sandbox `--full-auto` keeps: +workspace write with the network off, which would break `gh`, `cargo fetch` and `pip install` inside a container that has a network and a checkout the agent is meant to be able to push from. The container is already the sandbox, so a second one nested inside it subtracts exactly the capabilities `dl` went to the trouble of diff --git a/rust/aid/src/rewrite.rs b/rust/aid/src/rewrite.rs index 96544a1d..2b580ff5 100644 --- a/rust/aid/src/rewrite.rs +++ b/rust/aid/src/rewrite.rs @@ -57,13 +57,16 @@ pub(crate) const AGENT_ENV_VAR: &str = "DEVLAUNCH_AID_AGENT"; /// Held by `every_agent_starts_in_full_auto`. /// /// codex is the one worth spelling out, because it offers two modes and only one of -/// them is this one. `--full-auto` reads like the answer and is not: it approves -/// every action but keeps codex's own sandbox, which is workspace-write with the +/// them is this one. `--full-auto` reads like the answer and is not, for two +/// reasons and the first is the one that matters: it *still escalates to a person*. +/// It is an approval policy plus a sandbox, not an absence of approvals, so an +/// unattended run stops and asks -- which is exactly the failure this rule exists to +/// prevent. Only `--dangerously-bypass-approvals-and-sandbox` sets the policy to +/// never ask. The second reason is the sandbox it keeps: workspace-write with the /// network off, so `gh`, `cargo fetch` and `pip install` fail inside a container /// that has a network and a checkout the agent is meant to be able to push from. -/// The bypass flag is the analogue of claude's, and it is the honest one here: the -/// container is the sandbox, so a second one nested inside it subtracts exactly the -/// capabilities dl went to the trouble of provisioning. +/// The container is already the sandbox, so a second one nested inside it subtracts +/// exactly the capabilities dl went to the trouble of provisioning. /// /// `IS_SANDBOX=1` is what makes that flag usable at all here. claude refuses it /// outright under uid 0 — "cannot be used with root/sudo privileges", exit 1 — and @@ -1073,31 +1076,42 @@ mod tests { // full-auto flag fails here instead of stopping an unattended run to ask // about its first edit. Each CLI spells the flag its own way, so the // spellings are named rather than pattern-matched -- codex in particular - // has a second mode, `--full-auto`, that approves everything but keeps its - // own network-less sandbox, and picking that one would be a silent - // regression rather than a visible one. + // has a second mode, `--full-auto`, which still escalates to a person, and + // picking it would be a silent regression rather than a visible one. let expected = [ ("claude", "--dangerously-skip-permissions"), ("codex", "--dangerously-bypass-approvals-and-sandbox"), ("gemini", "--yolo"), ]; + + // The *names* against the table's keys, not `expected.len()` against + // `AGENTS.len()`, which was the first shape of this and let an agent through + // unchecked: `expected` holding claude, codex, codex is three rows long + // beside a three-row table while claude is never launched at all. Reviewed + // into existence -- with the count, dropping claude's flag and duplicating + // codex's row passed here. + let mut named: Vec<&str> = expected.iter().map(|(agent, _)| *agent).collect(); + named.sort_unstable(); + let mut known: Vec<&str> = AGENTS.iter().map(|(name, _)| *name).collect(); + known.sort_unstable(); assert_eq!( - expected.len(), - AGENTS.len(), + named, known, "an agent was added or removed without saying how it starts unattended" ); + for (agent, flag) in expected { - let command = build_agent_command(agent, "hi", None).expect("a known agent"); - assert!( - command.contains(flag), - "{agent} is not in full auto: {command}" - ); - assert!( - build_agent_command(agent, "", None) - .expect("a known agent") - .contains(flag), - "{agent} is not in full auto without a prompt" - ); + // As a whole argv word. `contains` is the same trap the docs guard in + // `aid/tests/rewrite.rs` records: every truncation of a flag is a + // substring of it, and so is every flag that merely starts with one, so + // a table entry reading `--yolo-dry-run` satisfied a `contains("--yolo")` + // while asking gemini for the opposite of full auto. + for prompt in ["hi", ""] { + let command = build_agent_command(agent, prompt, None).expect("a known agent"); + assert!( + command.split_whitespace().any(|word| word == flag), + "{agent} is not in full auto (prompt {prompt:?}): {command}" + ); + } } } diff --git a/rust/aid/tests/rewrite.rs b/rust/aid/tests/rewrite.rs index 1cdbdf60..406cc8c5 100644 --- a/rust/aid/tests/rewrite.rs +++ b/rust/aid/tests/rewrite.rs @@ -577,17 +577,24 @@ fn a_remote_agent_that_failed_ends_with_the_agents_status() { /// /// Diffed against the command as devpod receives it, rather than against the source /// table, so the page is checked against what a launch does and not against another -/// copy of the same list. Every row is required to name an agent this build knows, -/// which is what stops the guard passing on a table that has quietly lost a row. +/// copy of the same list. +/// +/// The set of names is asserted, not the count. Counting was the first shape of this +/// guard and Sourcery broke it on sight: three rows reading claude, claude, codex +/// satisfy a length check while the page has quietly lost gemini. The names the +/// table has to carry are therefore spelled out here, and every row is launched, so +/// a row naming an agent this build has never heard of fails at the launch rather +/// than being skipped as unrecognised. #[test] fn the_full_auto_section_names_the_flags_each_agent_is_actually_started_with() { let doc = std::fs::read_to_string(repo_root().join("docs/cli.md")).expect("docs/cli.md"); let rows = full_auto_rows(&doc); + let mut named: Vec<&str> = rows.iter().map(|(agent, _)| agent.as_str()).collect(); + named.sort_unstable(); assert_eq!( - rows.len(), - 3, - "docs/cli.md's full-auto table has {} rows, not one per agent: {rows:?}", - rows.len() + named, + ["claude", "codex", "gemini"], + "docs/cli.md's full-auto table names {named:?}, not one row per agent" ); for (agent, flag) in rows { @@ -599,8 +606,15 @@ fn the_full_auto_section_names_the_flags_each_agent_is_actually_started_with() { .unwrap_or_else(|| panic!("`aid --{agent}` opened no session")) .clone(); + // As a whole word and not by `contains`, which is the same trap + // `the_force_placement_section_quotes_the_refusals_it_says_it_does` records: + // every truncation of a flag is a substring of it, so a table reflowed or + // mistyped down to `--d` would satisfy a substring test against a launch + // that says `--dangerously-bypass-approvals-and-sandbox`. assert!( - command.contains(&flag), + command + .split_whitespace() + .any(|word| word.trim_matches('\'') == flag), "docs/cli.md says `{agent}` runs with {flag:?}; the launch is {command:?}" ); } @@ -616,6 +630,13 @@ fn the_full_auto_section_names_the_flags_each_agent_is_actually_started_with() { /// The agent is the first cell's single backticked word; the flag is the first /// `--word` in the second cell, which is the whole of what the copy has to get /// right. The rest of the cell is prose and is deliberately not read. +/// +/// Requiring the backticks is what separates a row from the two lines every markdown +/// table starts with. `| Agent |` is not backticked and the `| --- |` separator is +/// not either, which matters more than it looks: the separator's cells begin `--` +/// and would otherwise parse as an agent named `---` asking for a flag named `---`. +/// No allow-list of names is applied here on purpose, so an unknown name reaches the +/// caller and fails there. fn full_auto_rows(document: &str) -> Vec<(String, String)> { const HEADING: &str = "## Full auto: every agent, every launch"; let start = document @@ -631,14 +652,22 @@ fn full_auto_rows(document: &str) -> Vec<(String, String)> { .lines() .filter_map(|line| { let mut cells = line.trim().strip_prefix('|')?.split('|'); - let agent = cells.next()?.trim().trim_matches('`').to_owned(); - if !["claude", "codex", "gemini"].contains(&agent.as_str()) { + let agent = cells + .next()? + .trim() + .strip_prefix('`')? + .strip_suffix('`')? + .to_owned(); + if agent.split_whitespace().count() != 1 { return None; } let flag = cells .next()? .split_whitespace() - .map(|word| word.trim_end_matches(|c: char| !c.is_ascii_alphanumeric())) + .map(|word| { + word.trim_start_matches('`') + .trim_end_matches(|c: char| !c.is_ascii_alphanumeric()) + }) .find(|word| word.starts_with("--"))? .to_owned(); Some((agent, flag))