diff --git a/CHANGELOG.md b/CHANGELOG.md index a0e74f6d..b3c97e5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,51 @@ 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 | Full-auto flag | + | --- | --- | + | `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, 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` + 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. 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. + ## [0.32.0] - 2026-09-04 ### Added diff --git a/README.md b/README.md index 1a564301..ef446a0f 100644 --- a/README.md +++ b/README.md @@ -344,11 +344,13 @@ 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, 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. @@ -357,8 +359,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..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. @@ -461,6 +462,58 @@ 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 | Full-auto flag | +| --- | --- | +| `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 +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, 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 +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..2b580ff5 100644 --- a/rust/aid/src/rewrite.rs +++ b/rust/aid/src/rewrite.rs @@ -47,10 +47,26 @@ 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, 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 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 @@ -111,7 +127,7 @@ const AGENTS: &[(&str, Agent)] = &[ ( "codex", Agent { - command: &["codex"], + command: &["codex", "--dangerously-bypass-approvals-and-sandbox"], prompt_flags: &[], env: &[], remote_control: None, @@ -120,7 +136,7 @@ const AGENTS: &[(&str, Agent)] = &[ ( "gemini", Agent { - command: &["gemini"], + command: &["gemini", "--yolo"], prompt_flags: &["--prompt-interactive"], env: &[], remote_control: None, @@ -1045,14 +1061,60 @@ 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`, 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!( + named, known, + "an agent was added or removed without saying how it starts unattended" + ); + + for (agent, flag) in expected { + // 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}" + ); + } + } + } + #[test] fn a_prompt_is_one_argument_however_it_is_spelled() { // Python's `shlex.quote` spelling, byte for byte: the payload travels in @@ -1419,11 +1481,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 +1891,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..406cc8c5 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,112 @@ 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. +/// +/// 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!( + named, + ["claude", "codex", "gemini"], + "docs/cli.md's full-auto table names {named:?}, not one row per agent" + ); + + 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(); + + // 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 + .split_whitespace() + .any(|word| word.trim_matches('\'') == 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. +/// +/// 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 + .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() + .strip_prefix('`')? + .strip_suffix('`')? + .to_owned(); + if agent.split_whitespace().count() != 1 { + return None; + } + let flag = cells + .next()? + .split_whitespace() + .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)) + }) + .collect() +}