diff --git a/bench/admission_burst.py b/bench/admission_burst.py index 105cafc4f..0d6524237 100644 --- a/bench/admission_burst.py +++ b/bench/admission_burst.py @@ -128,7 +128,7 @@ def send(): row['burst_create'] = phase(client, process, model, together( [('create', dict(bot=f'burst{n}', workspace=workspace)) for n in range(bots)])) row['burst_submit'] = phase(client, process, model, together( - [('submit', dict(bot=f'burst{n}', request_id='first', prompt='hold:admission')) + [('submit', dict(bot=f'burst{n}', request_id='first', prompt='hold:admission', delivery='reject')) for n in range(bots)]), turns=bots) started = client.request('stats')['result']['active_turns'] assert started == 2 * bots, started diff --git a/bench/approval_overhead.py b/bench/approval_overhead.py index 652a7678d..97ce5cfaf 100644 --- a/bench/approval_overhead.py +++ b/bench/approval_overhead.py @@ -141,7 +141,7 @@ def send(op, **params): def submit(name): remaining[name] -= 1 pending_submits[send('submit', bot=name, request_id=f'{name}-{remaining[name]}', - prompt=prompt)] = (name, time.monotonic()) + prompt=prompt, delivery='reject')] = (name, time.monotonic()) def answer(message, calls): for call in calls: @@ -291,7 +291,7 @@ def send(op, **params): def submit(name): remaining[name] -= 1 pending_submits[send('submit', bot=name, request_id=f'{name}-{remaining[name]}', - prompt='shell:true')] = (name, time.monotonic()) + prompt='shell:true', delivery='reject')] = (name, time.monotonic()) wall0 = time.monotonic() for name in names: diff --git a/bench/daemon_driver.py b/bench/daemon_driver.py index eed439503..a7557d0fb 100644 --- a/bench/daemon_driver.py +++ b/bench/daemon_driver.py @@ -42,7 +42,7 @@ def submit(self, agent): turn = self.turn_of[agent] self.progress[agent] = [0, 0] self.request('submit', agent, bot=f'b{agent}', request_id=f'{agent}-{turn}', - prompt=prompt(self.config, agent, turn)) + prompt=prompt(self.config, agent, turn), delivery='reject') return {'event': 'turn_start', 'agent': str(agent), 'turn': str(turn)} def handle(self, message): diff --git a/bench/footprint.py b/bench/footprint.py index 0e54a3196..be1cc1a79 100644 --- a/bench/footprint.py +++ b/bench/footprint.py @@ -66,7 +66,7 @@ def rss_mib(): began, cpu = time.monotonic(), cpu_ms(process) turns = [] for first in range(0, bots, BATCH): - for reply in pipelined(client, [('submit', dict(bot=f'b{n}', request_id='one', prompt='hi')) + for reply in pipelined(client, [('submit', dict(bot=f'b{n}', request_id='one', prompt='hi', delivery='reject')) for n in range(first, min(bots, first + BATCH))]): assert 'result' in reply, reply turns.append(reply['result']['turn']) diff --git a/bench/lookout_scale.py b/bench/lookout_scale.py index ad0eda185..89a270709 100644 --- a/bench/lookout_scale.py +++ b/bench/lookout_scale.py @@ -295,7 +295,7 @@ def setup(self): (folder / f'p{p}.lead.json').write_text(json.dumps({'subscriptions': [tasks]})) # One finished turn each lead's messages to its tasks come from: what # a lead writes in its tasks is no news to it, only how they end. - firsts = self.control.many([('submit', {'bot': lead, 'request_id': 'first', 'prompt': 'hi'}) + firsts = self.control.many([('submit', {'bot': lead, 'request_id': 'first', 'prompt': 'hi', 'delivery': 'reject'}) for lead in self.leads()]) self.from_turn = {lead: r['result']['turn'] for lead, r in zip(self.leads(), firsts)} while self.busy_tasks(): @@ -357,7 +357,7 @@ def load(self, seconds, with_lookout=True): # all have ended. while time.monotonic() < stop: batch = [('submit', {'bot': task, 'request_id': f'{self.phase}r{n}-{task}', - 'prompt': f'stream:{count}:{ms}', + 'prompt': f'stream:{count}:{ms}', 'delivery': 'reject', 'from': {'bot': task.split('.')[0] + '.lead', 'turn': self.from_turn[task.split('.')[0] + '.lead']}}) for task in tasks] @@ -450,7 +450,7 @@ def approver(self, gated): raise RuntimeError('approver did not start') sampler = Sampler(process.pid) started = time.monotonic() - self.control.many([('submit', {'bot': name, 'request_id': 'gate', 'prompt': 'shell:true'}) + self.control.many([('submit', {'bot': name, 'request_id': 'gate', 'prompt': 'shell:true', 'delivery': 'reject'}) for name in names]) decided = lambda: [c for l in lines if l.get('event') == 'judged' for c in l['calls']] while len(decided()) < gated and time.monotonic() - started < 300: diff --git a/bench/runtime_client.py b/bench/runtime_client.py index 0a3b4560d..fb67e81eb 100644 --- a/bench/runtime_client.py +++ b/bench/runtime_client.py @@ -84,6 +84,9 @@ def receive(self, predicate, timeout=5): self.saved.append(message) def request(self, op, **params): + if op == 'submit': + # What a busy bot does with it; the daemon supplies none. + params.setdefault('delivery', 'reject') if op == 'create': params.setdefault('model', self.model) params.setdefault('instructions', self.instructions) diff --git a/bench/socket_client.py b/bench/socket_client.py index 6c499e49f..e163cbd1d 100644 --- a/bench/socket_client.py +++ b/bench/socket_client.py @@ -54,6 +54,9 @@ def record(self, event): self.queue.put(event) def request(self, op, **params): + if op == 'submit': + # What a busy bot does with it; the daemon supplies none. + params.setdefault('delivery', 'reject') if op == 'create': # This client's choice for a new bot; the daemon supplies none. params.setdefault('model', 'openai/synthetic-model') diff --git a/client/src/lib.rs b/client/src/lib.rs index ef4142739..bc65bc9fd 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -109,6 +109,47 @@ pub fn default_tools( /// together, so a client refuses a daemon of any other version. pub const PROTOCOL: u64 = 24; +/// Variables naming the terminal a daemon was started in: a terminal +/// multiplexer's place. A client starting the daemon leaves them out of its +/// tool shells (`--shell-exclude`), as a command that used one would land +/// in that terminal, not the bot's. +pub const TERMINAL_PLACES: [&str; 12] = [ + "REX_SESSION", + "REX_BLOCK", + "TMUX", + "TMUX_PANE", + "HERDR_ENV", + "HERDR_WORKSPACE_ID", + "HERDR_TAB_ID", + "HERDR_PANE_ID", + "CMUX_WORKSPACE_ID", + "CMUX_SURFACE_ID", + "CMUX_TAB_ID", + "CMUX_PANEL_ID", +]; + +/// Field names whose printed values are secrets: a client starting the +/// daemon has their values redacted from tool output (`--redact-field`). A +/// name matches a field's end, so `aws_secret_access_key` matches +/// `secret_access_key` and `AccessKeyId` or `NextToken` match nothing. +pub const SECRET_FIELDS: [&str; 12] = [ + "secret", + "secret_key", + "secret_access_key", + "session_token", + "access_token", + "refresh_token", + "id_token", + "auth_token", + "api_key", + "password", + "passwd", + "private_key", +]; + +/// PEM blocks whose bodies are secrets (`--redact-block`). +pub const SECRET_BLOCKS: [&str; 1] = ["PRIVATE KEY"]; + /// Start `command` in a session of its own, for a service that outlives the /// terminal it was started from: closing a Herdr pane, or stopping Herdr, /// ends every process in the pane's session, not only its foreground job. diff --git a/client/src/policy.rs b/client/src/policy.rs index 724d83616..47d346879 100644 --- a/client/src/policy.rs +++ b/client/src/policy.rs @@ -44,10 +44,8 @@ pub fn preamble() -> String { } pub const PREAMBLE: &str = "To delegate a subtask to another agent with its own conversation, run \ -\"$AGENT_BIN\" run --detach --new --agents --bot NAME -- TASK from the shell; it prints a turn handle immediately. \ ---agents gives it this folder's AGENTS.md, skills and memory, so its task need not say where they are. \ -Start NAME with your own name up to its first dot (shop.cart-review for shop.fix-cart), so it is listed with your project's agents. \ -An agent started to review or check something does it itself and starts no agents of its own. \ +\"$AGENT_BIN\" run --detach --new --bot NAME -- TASK from the shell; it prints a turn handle immediately. \ +Add --agents to give it this folder's AGENTS.md, skills and memory. \ Add --model PROVIDER/MODEL to give it one of the models \"$AGENT_BIN\" models lists; without it, it gets yours. \ Continue an existing agent with \"$AGENT_BIN\" run --detach --bot NAME -- TASK. \ To tell an agent something that needs no reply, such as a change of scope it should take into account, \ @@ -58,13 +56,11 @@ Use \"$AGENT_BIN\" fork --source NAME --bot NEW to branch an agent from where it To ask an agent something without disturbing it, mid-turn included, run \ \"$AGENT_BIN\" run --detach --fork NAME --bot NEW --allow '' --expires 15m -- QUESTION and wait on its handle: \ a copy of NAME answers from its history without acting, and is deleted once it has been idle 15 minutes. \ -Ask this way to learn where a busy agent is, or how it would take a steer, before sending one. \ $AGENT_BOT and $AGENT_BOT_ID identify you; $AGENT_PARENT and $AGENT_PARENT_ID, when set, identify the agent that created you. \ To contact that creator, run \ \"$AGENT_BIN\" run --detach --bot \"$AGENT_PARENT\" --bot-id \"$AGENT_PARENT_ID\" -- TASK; \ the identity check refuses a deleted creator or a replacement with the same name. \ -Your final reply already reaches whoever waits on your turn, so contact your creator only to ask something you need, \ -not to report results."; +Your final reply reaches whoever waits on your turn."; /// The prompt-cache lifetime human-facing clients make a bot with: an hour /// for Claude, on Anthropic's API and on Bedrock, where a lead or monitor that @@ -1213,9 +1209,7 @@ mod tests { let sonny = "\"/opt/homebrew/opt/sonny/bin/sonny\" agent"; let text = naming(PREAMBLE, sonny); assert!(!text.contains("AGENT_BIN"), "{text}"); - assert!(text.contains(&format!( - "{sonny} run --detach --new --agents --bot NAME -- TASK" - ))); + assert!(text.contains(&format!("{sonny} run --detach --new --bot NAME -- TASK"))); assert!(text.contains(&format!("{sonny} run --detach --bot \"$AGENT_PARENT\""))); // Without one, the binary the daemon gives every shell. assert_eq!(naming(PREAMBLE, "\"$AGENT_BIN\""), PREAMBLE); diff --git a/docs/CLI.md b/docs/CLI.md index 0e95d43b8..ff280dc27 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -28,8 +28,16 @@ PATTERN`, repeatable, leaves out more: every variable whose name matches, with starts a daemon passes it Codex's `[shell_environment_policy]` from `~/.agent/config.toml`: each name in `exclude`, and `*KEY*`, `*SECRET*` and `*TOKEN*` too with `ignore_default_excludes = false`. As in Codex, those three -are kept by default. A running daemon keeps the excludes it started with, and +are kept by default. It also leaves out the variables that name the +terminal it was started in (`TMUX`, `TMUX_PANE`, Rex's, Herdr's and cmux's +places, `agent_client::TERMINAL_PLACES`), so a bot's command never lands in +that terminal. A running daemon keeps the excludes it started with, and a variable a bot or caller sets with `--env` is still set. +`serve --redact-field NAME` has the value printed after a field named like +NAME (its end, any case, `_` and `-` ignored) redacted from tool output, and +`serve --redact-block LABEL` the body of each PEM block whose header names +LABEL. A client that starts a daemon passes `agent_client::SECRET_FIELDS` +(`password`, `api_key`, `secret_access_key` and the like) and `PRIVATE KEY`. `run` without `--bot` creates a fresh identity. `run --bot NAME` continues an existing bot; add `--new` to create that name. A prompt of `-` reads stdin. diff --git a/docs/DAEMON_MEASUREMENTS.md b/docs/DAEMON_MEASUREMENTS.md index 3f2be9a80..98324f2f1 100644 --- a/docs/DAEMON_MEASUREMENTS.md +++ b/docs/DAEMON_MEASUREMENTS.md @@ -5975,3 +5975,31 @@ probe p50 / max in ms): The probe used to wait behind the whole 64-request queue, mostly the flood's. Now it waits behind one of the flood's requests, and the flood's own rate holds. + +## Opinions move out to clients (#447) + +The daemon no longer strips terminal variables by name, requires `delivery` +on submit, reads a turn's round bound from the bot's `max_rounds`, and runs +its field-name and PEM redaction on the names a client passes at start +(`--redact-field`, `--redact-block`) instead of a constant list. The Perf +workflow's matched screen, base main 96228b5 against the PR head eb96cf9, +on the GitHub runner (2026-10-30): + +| Bench | Measure | Base | Head | Change | +| --- | --- | ---: | ---: | ---: | +| admission | lone submit CPU, ms | 40.231 | 40.158 | -0.2% | +| admission | burst submit p99, ms | 6.308 | 6.049 | -4.1% | +| completion | release to terminal p99, ms | 13.545 | 13.622 | +0.6% | +| footprint | idle RSS, MiB | 19.293 | 19.355 | +0.3% | +| footprint | per idle bot, KiB | 1.402 | 1.452 | +3.6% | +| footprint | RSS after a turn each, MiB | 27.102 | 27.365 | +1.0% | +| footprint | CPU per turn, ms | 1.252 | 1.274 | +1.8% | +| lifecycle | CPU, s | 1.15 | 1.15 | +0.0% | +| lifecycle | p95 turn, ms | 579.367 | 583.056 | +0.6% | +| lifecycle | peak RSS, MiB | 25.621 | 25.662 | +0.2% | + +Store jobs per turn did not change (about 7.5 on both), and nothing passed +the gate's 15%. The benches start the daemon with `serve` and no redact +flags, so the head runs no field scan where the base does. With the flags +the CLI passes, the scan is the base's code over the same twelve names, so +a CLI-started daemon does the base's work per tool result. diff --git a/docs/RUST_PROTOTYPE.md b/docs/RUST_PROTOTYPE.md index 62d9db0ce..d9b2f739f 100644 --- a/docs/RUST_PROTOTYPE.md +++ b/docs/RUST_PROTOTYPE.md @@ -406,6 +406,7 @@ keep-warm when it sets any of them. | `compact_at` (`--compact-at`) | Percent of either context envelope that triggers compaction, and of the byte envelope that triggers tool-result elision first. Estimated completion headroom can advance the byte trigger without reducing the input allowance. | 75 | | `compact_keep` (`--compact-keep`) | Target percent of either context envelope kept verbatim: as newest whole turns by compaction, as newest items by elision. Reduced when pinned context leaves less room. Must be below `compact_at`. | 25 | | `compact_tokens` (`--compact-tokens`) | Input tokens per request that start elision and compaction too, whichever of this and `compact_at` comes first (see [compaction](#compaction)). 0 leaves only `compact_at`; otherwise at least 1,024. | from the model's context window | +| `max_rounds` (`--max-rounds`) | Model calls each turn may make; the last goes with no tool call allowed, and the turn fails `tool_round_limit` past it. | none | | `keep_turns` (`--keep-turns`) | After each of its turns finishes, prune the bot to this many turns' records (see [Retention](#retention)). | none | | `approval_hold_ms` (`--approval-hold`) | Milliseconds a gated call waits live for its verdict before its turn parks (see [tool approval](#tool-approval)). 0 parks at once; at most 3,600,000. | 2,000 | | `max_output_tokens` (`--max-output-tokens`) | Generated tokens per model call, including reasoning, summaries included. Anthropic calls use the model's full output limit (read inside Bedrock ids) unless this is set; set, it is sent as `max_tokens`, at least 2,048 so a legacy thinking budget of 1,024 or more fits beside the answer. Bedrock deducts input plus this bound from quota when a call starts, so a bound near real output buys throughput there. | none | @@ -1489,11 +1490,13 @@ Protocol 8 adds the turn view. ### Delivery modes `delivery` on `submit` says what happens when the bot is busy or the daemon -is at `--max-active`. It is one field with four values; every mode returns +is at `--max-active`. It is one required field with four values, since +what a busy bot does with a message is the client's choice +(`invalid_delivery` without it); every mode returns the turn's id and handle at once, and `wait`, `turns`, and `interrupt` work on the turn unchanged. -- `reject` (default): `bot_busy` while a turn runs or is parked, +- `reject`: `bot_busy` while a turn runs or is parked, `active_agent_limit` when no slot is free. Nothing is written. The `bot_busy` detail says what is in the way (`turn N is running`, or `earlier work is waiting`) and which requests get past it, in this @@ -1623,7 +1626,7 @@ queued work as `bot_busy`. `stats` reports queued and ready turns together as `queued_turns`, a count the storage worker keeps at each transition and recounts at open, so admission and `stats` cost the store nothing. `agent run --delivery MODE` sets the field, defaulting -to `AGENT_DELIVERY` in the client's environment, never in the daemon; with +to `AGENT_DELIVERY` in the client's environment, then `reject`; with `--detach` a peer can hand a busy bot work or a mid-turn message without racing on `bot_busy`. @@ -1864,8 +1867,10 @@ completion, failure, explicit interruption, or parking, and accumulate across resumption. A hard process kill can lose the current segment's unflushed counters. Retries are on by default because they cannot repeat an effect; what they can repeat is billing for a failed attempt, which is recorded as failed usage. Successful -model responses and failures with reported usage share the durable limit of -200 model rounds per turn, including across parking and restart. The last +model responses and failures with reported usage share the bot's durable +limit of model rounds per turn, its `max_rounds` setting (unset, none: the +turn's token budget bounds a runaway instead), including across parking and +restart. The last of them, a retry included when a billed failure made it the last, goes with the bot's tools shown but `tool_choice` `none` (both families), so the model must answer in words, and the turn ends on that reply instead of failing with its work done and unreported; a live @@ -2189,12 +2194,11 @@ omission stated and the full stream is stored as an artifact retrievable through the `artifact` operation. Output is redacted before it is stored or sent, artifacts included, as every tool's result is: each credential the daemon holds, wherever it appears; the value printed after a field name ending in -`secret`, `secret_key`, `secret_access_key`, `session_token`, `access_token`, -`refresh_token`, `id_token`, `auth_token`, `api_key`, `password`, `passwd` or -`private_key` (any case, with or without `_` or `-`, as `NAME=`, `NAME:` or -`"NAME":`), when it is at least 8 bytes of letters and digits both, so a -credential tool's JSON or a credentials file never reaches the model or the -store; and the body of a PEM private key block. Results include separate output, exit code, and +one the daemon was started with (`--redact-field`, any case, with or without +`_` or `-`, as `NAME=`, `NAME:` or `"NAME":`), when it is at least 8 bytes and +mixes letters and digits; and the body of a PEM block whose header names a +`--redact-block` label. Which names count as secrets is the client's +judgement: the CLI passes `agent_client::SECRET_FIELDS` and `PRIVATE KEY`. Results include separate output, exit code, and success status. Nonzero exit is a recorded tool result. A command's return, timeout, or overflow kills its owned process group, so a process it started with `&` does not outlive it; the tool description tells the model so. A @@ -2437,7 +2441,10 @@ Either way the call's `arguments` are `{"input": TEXT}` and its answer is the provider's custom tool output where it has one. The grammar counts toward the 64 KiB. A client's code-running tool is one. -Protocol version 24 adds `item` to `text_delta` and `thinking_delta`, so a +Protocol version 24 makes `delivery` a required `submit` field +(`invalid_delivery` names the four), since the daemon has no opinion of +its own on what a busy bot does with a message. +It adds `item` to `text_delta` and `thinking_delta`, so a client draws each message a call streams apart, as it is kept (see `follow` above). It also reports an unset `cache_ttl` in `inspect`'s `settings` as `null` rather than `"5m"`, so a client tells a lifetime diff --git a/sonny/src/acp.rs b/sonny/src/acp.rs index ddf0115c6..e8cb0e498 100644 --- a/sonny/src/acp.rs +++ b/sonny/src/acp.rs @@ -911,7 +911,7 @@ impl Session { }; let steer = state.running.is_some(); let mut asked = json!({"bot": bound.bot, "request_id": crate::unique("acp"), "prompt": text, - "delivery": if steer { json!("steer") } else { Value::Null }, + "delivery": if steer { "steer" } else { "reject" }, "images": if images.is_empty() { Value::Null } else { json!(images) }}); if !steer && let Some(dir) = &bound.moving { asked["workspace"] = json!(dir); diff --git a/sonny/src/main.rs b/sonny/src/main.rs index d5799cbef..caa9777c1 100644 --- a/sonny/src/main.rs +++ b/sonny/src/main.rs @@ -8470,10 +8470,11 @@ impl App { ) -> Result<(), String> { // A message to a busy bot joins its running turn, on that turn's model. let delivery = match queue { - true => Some("queue"), - false => (join || self.running.is_some()).then_some("steer"), + true => "queue", + false if join || self.running.is_some() => "steer", + false => "reject", }; - let steer = delivery == Some("steer"); + let steer = delivery == "steer"; let model = (!steer).then(|| self.turn_model.clone()).flatten(); // A steer never moves a bot, so a moved lead comes back with the // next message that starts a turn. @@ -8820,7 +8821,7 @@ impl App { .request( "submit", json!({"bot": name, "bot_id": forked["bot_id"], "request_id": format!("{name}.1"), - "prompt": question, "model": self.turn_model, + "prompt": question, "delivery": "reject", "model": self.turn_model, "images": attach::submitted(pictures)}), ) .await @@ -11100,7 +11101,7 @@ mod tests { let shown: Vec = app.pending.iter().map(|l| l.to_string()).collect(); assert_eq!(shown, ["! echo hello", " └ hello"]); let asked = sent.lock().unwrap()[0].clone(); - assert_eq!(asked["delivery"], Value::Null); + assert_eq!(asked["delivery"], "reject"); // Not the person's words: what it printed authorizes nothing. assert_eq!(asked["origin"], "shell"); let prompt = asked["prompt"].as_str().unwrap(); @@ -11194,7 +11195,7 @@ mod tests { ); assert_eq!( (&sent[2]["delivery"], &sent[3]["delivery"]), - (&Value::Null, &json!("steer")) + (&json!("reject"), &json!("steer")) ); assert_eq!(sent[4]["prompt"], "and now?"); assert_eq!(sent[4]["origin"], Value::Null); @@ -11786,7 +11787,7 @@ mod tests { app.input.set("👍 ship it"); app.submit().await; assert_eq!(sent.lock().unwrap()[3]["prompt"], "👍 ship it"); - assert!(sent.lock().unwrap()[3]["delivery"].is_null()); + assert_eq!(sent.lock().unwrap()[3]["delivery"], "reject"); } #[tokio::test] diff --git a/sonny/src/suggest.rs b/sonny/src/suggest.rs index 86c6dda9d..a9cc7bb39 100644 --- a/sonny/src/suggest.rs +++ b/sonny/src/suggest.rs @@ -77,7 +77,7 @@ async fn ask( .request( "submit", json!({"bot": name, "bot_id": id, "request_id": format!("{name}.1"), - "prompt": ASK, "model": model}), + "prompt": ASK, "delivery": "reject", "model": model}), ) .await .ok()?; diff --git a/src/AGENTS.md b/src/AGENTS.md new file mode 100644 index 000000000..4723d430c --- /dev/null +++ b/src/AGENTS.md @@ -0,0 +1,15 @@ +# The daemon and the CLI + +Both are mechanism; clients such as Sonny hold every opinion. Before a change +here, check it against these, and discuss any daemon change with George first: + +- Offer a mechanism the client fills in, never a policy. A default is fine + only where a caller can change it per bot or per call. +- Name no client, terminal or tool it serves (Sonny, Herdr, cmux, git, `.agents`). + A client passes such knowledge in. `tests/daemon_knows_no_client.rs` checks names. +- Write nothing to a model beyond facts and structure: no advice, no style, + no CLI flags in tool descriptions or notes. Model-facing text is the client's. +- No fixed limit that decides how a bot works (rounds, what counts as a + secret, what to keep); capacity bounds that protect the daemon are fine. +- Known exceptions, chosen: the sender line (`hide_senders`), provider-key + stripping and redaction, Codex's `[shell_environment_policy]`. diff --git a/src/cli.rs b/src/cli.rs index d23cf1cdb..1b209cef5 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -21,7 +21,7 @@ const COMMANDS: &[Command] = &[ about: "Send a prompt to a bot, or make one with --new; prints the turn as it runs", usage: "run [OPTIONS] [--] PROMPT...", flags: "--bot --new --fork --allow --expires --detach --delivery --turn --model --workspace --effort --turn-budget-tokens --image --agents --profile --request-id --bot-id --pretty --no-spawn", - settings: "--tools --instructions --instructions-file --budget-tokens --compaction-instructions --compaction-instructions-file --compaction-model --no-compaction --fallback --approval --approve --context-bytes --context-items --note-turns --compact-at --compact-keep --compact-tokens --keep-turns --approval-hold --max-output-tokens --keep-warm --cache-ttl --sandbox --writable --network --hide-senders --unsandboxed", + settings: "--tools --instructions --instructions-file --budget-tokens --compaction-instructions --compaction-instructions-file --compaction-model --no-compaction --fallback --approval --approve --context-bytes --context-items --note-turns --compact-at --compact-keep --compact-tokens --max-rounds --keep-turns --approval-hold --max-output-tokens --keep-warm --cache-ttl --sandbox --writable --network --hide-senders --unsandboxed", startup: true, }, Command { @@ -204,7 +204,7 @@ const COMMANDS: &[Command] = &[ name: "serve", about: "Run the daemon in the foreground; other commands start it for you", usage: "serve --store PATH --provider SPEC... [OPTIONS]", - flags: "--shell-exclude", + flags: "--shell-exclude --redact-field --redact-block", settings: "", startup: true, }, @@ -368,12 +368,24 @@ fn print_flags(flags: &str) { "PATTERN", "Leave variables named like this (* and ? wildcards, any case) out of tool shells; repeatable; clients pass [shell_environment_policy] from ~/.agent/config.toml", ), + "--redact-field" => ( + "NAME", + "Redact the value printed after a field named like this (its end, any case, _ and - ignored) from tool output; repeatable", + ), + "--redact-block" => ( + "LABEL", + "Redact the body of each PEM block whose header names LABEL, such as PRIVATE KEY, from tool output; repeatable", + ), "--port" => ( "N", "Wait for the browser's sign-in on 127.0.0.1:N; default 1455", ), "--context-bytes" => ("N", "A new bot's model context bytes; default 8 MiB"), "--context-items" => ("N", "A new bot's model context items; default 4096"), + "--max-rounds" => ( + "N", + "Model calls each of a new bot's turns may make, the last with no tool call; default unbounded", + ), "--note-turns" => ( "N", "Omitted turns a new bot's context note lists; 0 lists none; default 48", @@ -605,6 +617,8 @@ pub fn prepare(args: Vec) -> Result>> { | "--unsandboxed" | "--env" | "--shell-exclude" + | "--redact-field" + | "--redact-block" ) { return fail_with("usage", format!("{flag} may only be specified once")); diff --git a/src/client.rs b/src/client.rs index 90fe101d5..708f6c4fe 100644 --- a/src/client.rs +++ b/src/client.rs @@ -358,6 +358,7 @@ fn parse(args: &[String]) -> Result { | "--compact-at" | "--compact-keep" | "--compact-tokens" + | "--max-rounds" | "--max-output-tokens" => { let number = value.parse::().map_err(|_| { Error::with("usage", format!("{flag} needs an integer")) @@ -863,6 +864,15 @@ fn connect_or_start(options: &Options) -> Result { for (flag, value) in &options.daemon_flags { command.arg(flag).arg(value); } + for name in agent_client::TERMINAL_PLACES { + command.arg("--shell-exclude").arg(name); + } + for field in agent_client::SECRET_FIELDS { + command.arg("--redact-field").arg(field); + } + for label in agent_client::SECRET_BLOCKS { + command.arg("--redact-block").arg(label); + } for pattern in config()?.shell_environment_policy.excludes() { command.arg("--shell-exclude").arg(pattern); } @@ -1563,7 +1573,7 @@ fn run(options: &Options) -> Result { "workspace":options.workspace.as_ref().and(workspace.as_ref()), "model":if created { Value::Null } else { json!(options.model) }, "effort":if created { Value::Null } else { json!(options.effort) }, - "delivery":options.delivery,"expected_turn":options.turn,"from":from, + "delivery":options.delivery.as_deref().unwrap_or("reject"),"expected_turn":options.turn,"from":from, "budget_tokens":options.turn_budget_tokens}), ) .map_err(|error| ways_past_busy(&bot, error))?; diff --git a/src/main.rs b/src/main.rs index 1f1c5df8b..386dffa58 100644 --- a/src/main.rs +++ b/src/main.rs @@ -139,6 +139,7 @@ fn configuration(args: &[String]) -> Result { let mut idle_exit = None; let mut stall_timeout = None; let mut shell_excludes = Vec::new(); + let (mut redact_fields, mut redact_blocks) = (Vec::new(), Vec::new()); let mut iter = args.iter(); while let Some(flag) = iter.next() { let value = iter @@ -152,6 +153,11 @@ fn configuration(args: &[String]) -> Result { return fail_with("usage", "--shell-exclude needs a name or pattern"); } "--shell-exclude" => shell_excludes.push(value.clone()), + "--redact-field" | "--redact-block" if value.trim().is_empty() => { + return fail_with("usage", format!("{flag} needs a name")); + } + "--redact-field" => redact_fields.push(value.clone()), + "--redact-block" => redact_blocks.push(value.clone()), "--max-processes" | "--max-detached" | "--max-active" @@ -195,5 +201,7 @@ fn configuration(args: &[String]) -> Result { stall_timeout, idle_exit, shell_excludes, + redact_fields, + redact_blocks, }) } diff --git a/src/server/mod.rs b/src/server/mod.rs index 66e6afabb..d4dcc585a 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -750,6 +750,10 @@ pub struct Configuration { /// from the daemon's environment, as Codex's shell environment /// excludes; none by default. pub shell_excludes: Vec, + /// Field names whose values are redacted from tool output. + pub redact_fields: Vec, + /// What the headers of PEM blocks redacted from tool output name. + pub redact_blocks: Vec, } #[derive(Clone, Copy)] @@ -1217,6 +1221,7 @@ pub async fn run(config: Configuration) -> Result<()> { let mut providers = HashMap::new(); let aws_start = agent_runtime::provider::aws::Start::new(); let credentials = agent_runtime::tools::Credentials::default(); + credentials.redact_also(&config.redact_fields, &config.redact_blocks); // Every tool shell inherits the environment the daemon started with, // so the excludes are matched against it once. for (name, _) in std::env::vars_os() { @@ -3911,8 +3916,18 @@ impl Service { if prompt.len() > 256 * 1024 { return fail_with("prompt_limit", "prompt may be up to 262144 bytes"); } + // What a busy bot does with it is the client's to say. let delivery = match delivery.as_deref() { - None => Delivery::Reject, + None => { + return Err(Error::with( + "invalid_delivery", + format!( + "delivery is required: one of {}", + Delivery::NAMES.join(", ") + ), + ) + .facts(json!({"deliveries":Delivery::NAMES}))); + } Some(mode) => Delivery::requested(mode)?, }; if expected_turn.is_some() && delivery != Delivery::Steer { @@ -5183,7 +5198,7 @@ mod tests { workspace: None, model: None, effort: None, - delivery: None, + delivery: Some("reject".into()), expected_turn: None, from: None, origin: None, diff --git a/src/server/turn.rs b/src/server/turn.rs index 217fcf17a..a985e7e0f 100644 --- a/src/server/turn.rs +++ b/src/server/turn.rs @@ -38,7 +38,6 @@ use std::{ }; use tokio::sync::{Notify, mpsc, oneshot, watch}; -pub const MAX_ROUNDS: usize = 200; /// The error a cancelled turn ends with, sent on its cancel channel: a /// client's interrupt, or the daemon shutting down around it. pub const INTERRUPTED: &str = "cancelled"; @@ -1096,7 +1095,8 @@ impl Turn { // Beside the call, a summary holds a copy's call and a request of // its own against the round limit. One a client asked for goes // before the call, as asked, and stays asked until it ends. - let beside = beside.filter(|_| !requested && fits && *model_rounds + 2 < MAX_ROUNDS); + let beside = beside + .filter(|_| !requested && fits && *model_rounds + 2 < self.settings().max_rounds()); let mut spent = Accounting { route: accounting.route.clone(), ..Accounting::default() @@ -1145,7 +1145,8 @@ impl Turn { spent.beside = true; accounting.summary_held = tokens; *slot = Some(Box::pin(async move { - let (before, start) = (record.tokens_used, MAX_ROUNDS - 2); + let (before, start) = + (record.tokens_used, self.settings().max_rounds() - 2); let mut rounds = start; let mut spent = Counted { spent, @@ -1284,7 +1285,7 @@ impl Turn { if let Some(context) = fits(self.fitted_context().await)? { return Ok(Overflow::Fits(Box::new(context))); } - if *model_rounds >= MAX_ROUNDS { + if *model_rounds >= self.settings().max_rounds() { return Ok(Overflow::Stuck); } } @@ -1598,7 +1599,7 @@ impl Turn { if !copied || invalid.code == "compaction_summary_limit" || record.exhausted().is_some() - || *model_rounds >= MAX_ROUNDS + || *model_rounds >= self.settings().max_rounds() { self.compaction_failed(turn, &invalid).await?; return Ok(Summary::Skipped); @@ -2172,7 +2173,7 @@ impl Turn { || *effort == record.effort) }) }); - while model_rounds < MAX_ROUNDS { + while model_rounds < self.settings().max_rounds() { // A refresh the last call left in flight ends here: the next call // reads the cache itself, and the budget counts what it billed. self.settle_refresh(&mut record, accounting).await?; @@ -2350,7 +2351,7 @@ impl Turn { if let Some(error) = record.exhausted() { return Err(error); } - if model_rounds >= MAX_ROUNDS { + if model_rounds >= self.settings().max_rounds() { return fail("tool_round_limit"); } // The view fits the budget alone but not beside what goes ahead @@ -2402,7 +2403,7 @@ impl Turn { // The round limit's last call goes with the tools shown but no // call allowed, so the turn ends on a reply rather than failing // with its work done and unreported. - let tool_calls = model_rounds + 1 < MAX_ROUNDS; + let tool_calls = model_rounds + 1 < self.settings().max_rounds(); if !tool_calls { self.hub .live( @@ -2859,7 +2860,10 @@ impl Turn { // A billed attempt that failed counts as a round: the retry that // is the turn's last allowed call goes without calls, as that // call would have. - if tool_calls && matches!(body, Body::Window(_)) && *model_rounds + 1 >= MAX_ROUNDS { + if tool_calls + && matches!(body, Body::Window(_)) + && *model_rounds + 1 >= self.settings().max_rounds() + { (tool_calls, warm_after) = (false, None); self.hub .live( @@ -3110,9 +3114,10 @@ impl Turn { } // A retry is another billable call: none once failed usage has // spent the bot's or turn's budget, or the rounds are used up. - let capped = record - .exhausted() - .or_else(|| (*model_rounds >= MAX_ROUNDS).then(|| Error::new("tool_round_limit"))); + let capped = record.exhausted().or_else(|| { + (*model_rounds >= self.settings().max_rounds()) + .then(|| Error::new("tool_round_limit")) + }); attempt += 1; let paced = agent_runtime::provider::paced(&error.code); let cap = if paced { @@ -4672,7 +4677,7 @@ fn searches(record: &agent_runtime::store::Bot) -> bool { } fn calls_left(record: &agent_runtime::store::Bot, model_rounds: usize) -> bool { - model_rounds < MAX_ROUNDS && record.exhausted().is_none() + model_rounds < record.settings.max_rounds() && record.exhausted().is_none() } /// Failures of the provider's pace, capacity, or transport, none of which say diff --git a/src/store/db.rs b/src/store/db.rs index a1351c7e8..759b206ce 100644 --- a/src/store/db.rs +++ b/src/store/db.rs @@ -227,6 +227,10 @@ pub struct Settings { /// model without the line saying so after its words. #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub hide_senders: bool, + /// Model calls one of its turns may make, the last with no tool call + /// allowed; none bounds them only by the turn's token budget. + #[serde(skip_serializing_if = "Option::is_none")] + pub max_rounds: Option, /// Command prefixes, each of words, that run outside the sandbox /// without asking, as a Codex rule that allows a prefix runs it /// ([`crate::sandbox::unsandboxed`]). @@ -281,6 +285,10 @@ impl Settings { None => Some(compact_threshold(window)), } } + /// Model calls a turn may make; unset, more than any turn reaches. + pub fn max_rounds(&self) -> usize { + self.max_rounds.unwrap_or(usize::MAX / 4) + } pub fn approval_hold_ms(&self) -> u64 { self.approval_hold_ms.unwrap_or(2000) } @@ -352,6 +360,9 @@ impl Settings { { return refuse("compact_tokens is at least 1024; 0 leaves only compact_at"); } + if self.max_rounds == Some(0) { + return refuse("max_rounds is at least 1"); + } if self.keep_turns == Some(0) { return refuse("keep_turns is at least 1"); } @@ -406,7 +417,7 @@ impl Settings { json!({"context_bytes":self.context_bytes(),"context_items":self.context_items(), "note_turns":self.note_turns(),"compact_at":self.compact_at(), "compact_keep":self.compact_keep(),"compact_tokens":self.compact_tokens, - "keep_turns":self.keep_turns, + "keep_turns":self.keep_turns,"max_rounds":self.max_rounds, "approval_hold_ms":self.approval_hold_ms(),"max_output_tokens":self.max_output_tokens, "keep_warm":self.keep_warm().map_or(0, |after| after.as_secs()), // Null when never set (five minutes), so a client's default can @@ -1256,7 +1267,7 @@ impl CompactionPlan { head.push(b','); } let mut tail = family.user_item(&format!( - "[compaction request] Write the summary of the conversation above now, following your instructions. Keep the summary within {summary_bytes} UTF-8 bytes; be shorter when possible." + "[compaction request] Write the summary of the conversation above now, following your instructions. Keep the summary within {summary_bytes} UTF-8 bytes." ))?; tail.insert(0, b','); Ok((head, tail)) @@ -1266,7 +1277,7 @@ impl CompactionPlan { /// in it, since the copy keeps the bot's own. pub fn request(family: Family, instructions: &str, summary_bytes: usize) -> Result> { let mut request = family.user_item(&format!( - "[compaction request] Write the summary of the conversation above now, following these instructions, without calling tools. Keep the summary within {summary_bytes} UTF-8 bytes; be shorter when possible.\n\n{instructions}" + "[compaction request] Write the summary of the conversation above now, following these instructions, without calling tools. Keep the summary within {summary_bytes} UTF-8 bytes.\n\n{instructions}" ))?; request.insert(0, b','); Ok(request) diff --git a/src/tools.rs b/src/tools.rs index b7c198b9c..d38bff5de 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -113,7 +113,7 @@ impl Tool { "properties":{"text":{"type":"string"}},"required":["text"],"additionalProperties":false}), ), Tool::Shell => ( - "Run a noninteractive bash command (sh where bash is not installed) in the workspace. stdout and stderr are returned separately; each is truncated to a head and tail beyond 64 KiB, with up to 1 MiB of it retained: past that, its first and last 512 KiB, and the command keeps running. Default timeout 120 s, maximum 600 s; a command that times out returns its output so far with timed_out. Every result reports wall_time_ms. Commands run with no terminal: TERM=dumb, NO_COLOR=1, and cat as the pager; AI_AGENT=agent tells tools an agent runs them. When the command returns or times out, every process it started is killed, so `cmd &` does not outlive it. With background=true the command is started and a proc handle is returned immediately; collect its result later with wait. A steer that arrives while a command runs moves it to the background: the result is its proc handle with steered=true, and the command keeps running. A background command runs until it exits, times out, or the daemon stops. With detach=true the command starts in its own session and its pid is returned at once; nothing kills or tracks it, so it outlives the turn and the daemon. Its output is discarded unless the command redirects it to a file. Use detach for a server or service that must keep running, and stop it yourself with kill. With tty=true it runs in a terminal, for a REPL or a program that prompts: the result is what it wrote within yield_time_ms (default 1000) and, while it runs, a proc handle for write_stdin.", + "Run a noninteractive bash command (sh where bash is not installed) in the workspace. stdout and stderr are returned separately; each is truncated to a head and tail beyond 64 KiB, with up to 1 MiB of it retained: past that, its first and last 512 KiB, and the command keeps running. Default timeout 120 s, maximum 600 s; a command that times out returns its output so far with timed_out. Every result reports wall_time_ms. Commands run with no terminal: TERM=dumb, NO_COLOR=1, and cat as the pager; AI_AGENT=agent tells tools an agent runs them. When the command returns or times out, every process it started is killed, so `cmd &` does not outlive it. With background=true the command is started and a proc handle is returned immediately; collect its result later with wait. A steer that arrives while a command runs moves it to the background: the result is its proc handle with steered=true, and the command keeps running. A background command runs until it exits, times out, or the daemon stops. With detach=true the command starts in its own session and its pid is returned at once; nothing kills or tracks it, so it outlives the turn and the daemon. Its output is discarded unless the command redirects it to a file. With tty=true it runs in a terminal, for a REPL or a program that prompts: the result is what it wrote within yield_time_ms (default 1000) and, while it runs, a proc handle for write_stdin.", json!({"type":"object","properties":{"command":{"type":"string"}, "timeout_ms":{"type":"integer","minimum":1,"maximum":MAX_SHELL_TIMEOUT_MS}, "background":{"type":"boolean"},"detach":{"type":"boolean"},"tty":{"type":"boolean"}, @@ -173,7 +173,7 @@ impl Tool { "required":["handle"],"additionalProperties":false}), ), Tool::Wait => ( - "Suspend until every handle resolves, without holding any execution capacity. Handles are 'turn:BOT/N' (a peer agent's turn, printed by run --detach) or 'proc:N' (a background shell). Each result reports the outcome: a peer's status and final text, with before_steer, the last text it wrote before it was steered, when it ended on a reply to a steer; or a process's output and exit code. With timeout_ms, unresolved handles are reported as pending and stay valid for a later wait; 0 polls without waiting. With any: true, the first resolved handle ends the wait and the rest are reported pending. A steer ends the wait at once with steered=true: unresolved handles are reported pending and stay valid, so wait again to keep waiting. To wait for a condition, start a shell with background=true running a loop such as `until CHECK; do sleep 5; done` and wait on its proc handle, rather than checking in repeated calls.", + "Suspend until every handle resolves, without holding any execution capacity. Handles are 'turn:BOT/N' (a peer agent's turn) or 'proc:N' (a background shell). Each result reports the outcome: a peer's status and final text, with before_steer, the last text it wrote before it was steered, when it ended on a reply to a steer; or a process's output and exit code. With timeout_ms, unresolved handles are reported as pending and stay valid for a later wait; 0 polls without waiting. With any: true, the first resolved handle ends the wait and the rest are reported pending. A steer ends the wait at once with steered=true: unresolved handles are reported pending and stay valid, so wait again to keep waiting.", json!({"type":"object","properties":{"handles":{"type":"array","items":{"type":"string"},"minItems":1,"maxItems":64}, "timeout_ms":{"type":"integer","minimum":0,"maximum":MAX_WAIT_TIMEOUT_MS},"any":{"type":"boolean"}}, "required":["handles"],"additionalProperties":false}), @@ -484,6 +484,12 @@ struct CredentialSet { /// of a new token. Refreshes pay for insertion; tool output stays sorted. values: Vec, names: Vec, + /// Field names, lowercased without `_` or `-`, whose values a client + /// had redacted wherever they are printed (`--redact-field`). + fields: Vec, + /// What the header of a PEM block a client had redacted names + /// (`--redact-block`), such as `PRIVATE KEY`. + blocks: Vec, } /// Whether a variable's `name` matches `pattern`: `*` any run of @@ -558,16 +564,27 @@ impl Credentials { .names .clone() } + /// Also redact the values after these field names, matched as a name's + /// end, and the bodies of PEM blocks whose header names one of `blocks`. + pub fn redact_also(&self, fields: &[String], blocks: &[String]) { + let mut set = self.0.write().unwrap_or_else(|e| e.into_inner()); + set.fields = fields.iter().map(|field| field_key(field)).collect(); + set.blocks = blocks.to_vec(); + } pub fn redact(&self, mut text: String) -> String { - { - let set = self.0.read().unwrap_or_else(|e| e.into_inner()); - for credential in &set.values { - if text.contains(&credential.value) { - text = text.replace(&credential.value, REDACTED); - } + let set = self.0.read().unwrap_or_else(|e| e.into_inner()); + for credential in &set.values { + if text.contains(&credential.value) { + text = text.replace(&credential.value, REDACTED); } } - redact_secret_fields(redact_private_keys(text)) + if !set.blocks.is_empty() { + text = redact_blocks(text, &set.blocks); + } + if !set.fields.is_empty() { + text = redact_fields(text, &set.fields); + } + text } /// How many of `text`'s last bytes could begin a credential that more /// output would finish: what a stream holds back until it can redact. @@ -607,30 +624,23 @@ impl Credentials { } const REDACTED: &str = "[REDACTED]"; -/// Field names, lowercased without `_` or `-`, whose values are secrets -/// wherever they are printed: `NAME=VALUE`, `NAME: VALUE`, `"NAME": "VALUE"`. -/// A name matches when it ends with one of these, so `aws_secret_access_key` -/// and `SecretAccessKey` match and `AccessKeyId` or `NextToken` do not. -const SECRET_FIELDS: [&str; 12] = [ - "secret", - "secretkey", - "secretaccesskey", - "sessiontoken", - "accesstoken", - "refreshtoken", - "idtoken", - "authtoken", - "apikey", - "password", - "passwd", - "privatekey", -]; /// Shorter values are flags, placeholders and counts, not secrets; so are /// values without both letters and digits. const SECRET_MIN: usize = 8; -/// Replace the value after each secret field name in `text`. -fn redact_secret_fields(text: String) -> String { +/// A field name as it is matched: lowercased, without `_` or `-`. +fn field_key(name: &str) -> String { + name.chars() + .filter(|c| *c != '_' && *c != '-') + .map(|c| c.to_ascii_lowercase()) + .collect() +} + +/// Replace the value after each of `fields` in `text`, printed as +/// `NAME=VALUE`, `NAME: VALUE` or `"NAME": "VALUE"`. A name matches when it +/// ends with a field, so `aws_secret_access_key` matches `secretaccesskey`. +fn redact_fields(text: String, fields: &[String]) -> String { + let shortest = fields.iter().map(String::len).min().unwrap_or(0); let bytes = text.as_bytes(); let word = |b: u8| b.is_ascii_alphanumeric() || b == b'_' || b == b'-'; let mut spans = Vec::new(); @@ -644,16 +654,12 @@ fn redact_secret_fields(text: String) -> String { while i < bytes.len() && word(bytes[i]) { i += 1; } - // The shortest field is six letters; most words are shorter. - if i - start < 6 { + // A word shorter than every field cannot end with one. + if i - start < shortest { continue; } - let name: String = text[start..i] - .chars() - .filter(|c| *c != '_' && *c != '-') - .map(|c| c.to_ascii_lowercase()) - .collect(); - if !SECRET_FIELDS.iter().any(|field| name.ends_with(field)) { + let name = field_key(&text[start..i]); + if !fields.iter().any(|field| name.ends_with(field.as_str())) { continue; } // The name's closing quote, the separator, and the value's quote. @@ -716,8 +722,9 @@ fn redact_secret_fields(text: String) -> String { out } -/// Replace the body of each PEM private key block in `text`. -fn redact_private_keys(mut text: String) -> String { +/// Replace the body of each PEM block in `text` whose header names one of +/// `blocks`. +fn redact_blocks(mut text: String, blocks: &[String]) -> String { const BEGIN: &str = "-----BEGIN "; const END: &str = "-----END "; let mut from = 0; @@ -735,7 +742,10 @@ fn redact_private_keys(mut text: String) -> String { [b'\n', ..] => dashes + 1, _ => dashes, }; - if !text[begin..header].contains("PRIVATE KEY") { + if !blocks + .iter() + .any(|block| text[begin..header].contains(block.as_str())) + { from = header; continue; } @@ -2122,24 +2132,6 @@ fn sh( process.env_remove("AGENT_PARENT_ID"); // Likewise a set AGENT_EFFORT is this bot's own level. process.env_remove("AGENT_EFFORT"); - // A terminal multiplexer's place names the terminal the daemon was - // started in, not the bot's: a command that used it would land there. - for name in [ - "REX_SESSION", - "REX_BLOCK", - "TMUX", - "TMUX_PANE", - "HERDR_ENV", - "HERDR_WORKSPACE_ID", - "HERDR_TAB_ID", - "HERDR_PANE_ID", - "CMUX_WORKSPACE_ID", - "CMUX_SURFACE_ID", - "CMUX_TAB_ID", - "CMUX_PANEL_ID", - ] { - process.env_remove(name); - } // No command has a terminal: plain output and no pager, as other // harnesses run commands. AI_AGENT tells tools an agent runs them, as // fx sets it. What the caller or bot sets still wins. @@ -2455,8 +2447,12 @@ mod tests { assert_eq!(Registry::new("read").unwrap().names(), ["read"]); } #[test] - fn secrets_are_redacted_by_field_name_and_key_block() { + fn the_fields_and_blocks_a_client_names_are_redacted() { let credentials = Credentials::default(); + let unnamed = "password: fake8fake8fake8\n-----BEGIN RSA PRIVATE KEY-----\nMIIE"; + assert_eq!(credentials.redact(unnamed.to_owned()), unnamed); + let fields = ["secret_access_key", "SessionToken", "api-key", "password"]; + credentials.redact_also(&fields.map(String::from), &["PRIVATE KEY".to_owned()]); let redact = |text: &str| credentials.redact(text.to_owned()); assert_eq!( redact( diff --git a/tests/daemon_knows_no_client.rs b/tests/daemon_knows_no_client.rs new file mode 100644 index 000000000..3f072b719 --- /dev/null +++ b/tests/daemon_knows_no_client.rs @@ -0,0 +1,63 @@ +//! The daemon is mechanism its clients fill in (src/AGENTS.md): no client, +//! terminal or tool it serves is named in its source. A name found here is +//! knowledge that belongs to a client, such as Sonny, which passes it in. +use std::path::{Path, PathBuf}; + +/// The CLI's own files, which are a client: everything else under `src/` is +/// the daemon. +const CLIENT: [&str; 6] = [ + "src/approver.rs", + "src/cli.rs", + "src/client.rs", + "src/client_path.rs", + "src/sign_in.rs", + "src/main.rs", +]; + +/// Names only a client knows, matched as written. +const NAMES: [&str; 12] = [ + "sonny", "Sonny", "herdr", "Herdr", "HERDR", "cmux", "CMUX", "tmux", "TMUX", "REX_", "lookout", + "Lookout", +]; + +fn sources(folder: &Path, found: &mut Vec) { + for entry in std::fs::read_dir(folder).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + sources(&path, found); + } else if path.extension().is_some_and(|e| e == "rs") { + found.push(path); + } + } +} + +#[test] +fn the_daemon_names_no_client_terminal_or_tool() { + let root = Path::new(env!("CARGO_MANIFEST_DIR")); + let mut files = Vec::new(); + sources(&root.join("src"), &mut files); + let mut named = Vec::new(); + for file in files { + let relative = file + .strip_prefix(root) + .unwrap() + .to_string_lossy() + .into_owned(); + if CLIENT.contains(&relative.as_str()) { + continue; + } + let text = std::fs::read_to_string(&file).unwrap(); + // A file's tests may name what they stand in for. + let code = text.split("#[cfg(test)]").next().unwrap_or_default(); + for (line, content) in code.lines().enumerate() { + for name in NAMES.iter().filter(|name| content.contains(*name)) { + named.push(format!("{relative}:{}: {name}", line + 1)); + } + } + } + assert!( + named.is_empty(), + "the daemon names what only a client knows; have the client pass it in:\n{}", + named.join("\n") + ); +} diff --git a/tests/store_contract.rs b/tests/store_contract.rs index 923f8492c..4c4b94958 100644 --- a/tests/store_contract.rs +++ b/tests/store_contract.rs @@ -3223,7 +3223,7 @@ fn a_bot_keeps_its_settings_and_a_fork_copies_them() { assert_eq!( serde_json::to_value(&bob).unwrap()["settings"], json!({"context_bytes":4096,"context_items":4096,"note_turns":48,"compact_at":75, - "compact_keep":25,"compact_tokens":null,"keep_turns":1,"approval_hold_ms":2000,"max_output_tokens":null, + "compact_keep":25,"compact_tokens":null,"keep_turns":1,"max_rounds":null,"approval_hold_ms":2000,"max_output_tokens":null, "keep_warm":0,"cache_ttl":"1h","sandbox":null,"writable":[],"network":false,"hide_senders":false,"unsandboxed":[]}) ); // Retention follows the bot's own setting. diff --git a/tests/test_approvals.py b/tests/test_approvals.py index 1a59d64ce..effa02ce4 100644 --- a/tests/test_approvals.py +++ b/tests/test_approvals.py @@ -905,23 +905,6 @@ def test_a_turn_reads_its_prompts_authors_and_calls(self): self.assertEqual((read['status'], read['workspace'], read['more']), ('finished', str(self.path), False)) self.assertEqual(client.request('prompts', bot='Bob', turn=second, bytes=0)['error'], 'invalid_limit') - def test_the_daemons_own_terminal_never_reaches_a_tool(self): - env = {**clean_env(), 'REX_SESSION': 'session:daemons', 'REX_BLOCK': 'block:daemons', - 'TMUX': '/tmp/tmux-0/default,1,0', 'TMUX_PANE': '%0', 'HERDR_ENV': '1', - 'HERDR_WORKSPACE_ID': 'w1', 'HERDR_TAB_ID': 'w1:t1', 'HERDR_PANE_ID': 'w1:p1', - 'CMUX_WORKSPACE_ID': 'ws-1', 'CMUX_SURFACE_ID': 'sf-1', 'CMUX_TAB_ID': 'ws-1', - 'CMUX_PANEL_ID': 'sf-1'} - client = Client(self.binary, self.path / 'state.sqlite', self.url, 'shell', env=env) - self.addCleanup(client.close) - client.request('create', bot='Bob', workspace=str(self.path)) - turn = client.request('submit', bot='Bob', request_id='a', - prompt='shell:printf "${REX_SESSION-unset}${REX_BLOCK-unset}${TMUX-unset}${TMUX_PANE-unset}' - '${HERDR_ENV-unset}${HERDR_WORKSPACE_ID-unset}${HERDR_TAB_ID-unset}${HERDR_PANE_ID-unset}' - '${CMUX_WORKSPACE_ID-unset}${CMUX_SURFACE_ID-unset}${CMUX_TAB_ID-unset}${CMUX_PANEL_ID-unset}" > seen' - )['result']['turn'] - self.assertEqual(client.finished(turn)['data']['status'], 'completed') - self.assertEqual((self.path / 'seen').read_text(), 'unset' * 12) - @unittest.skipUnless(os.environ.get('AGENT_TEST_RUNTIME') == '1', 'set AGENT_TEST_RUNTIME=1 after a Rust release build') class ApprovalCliTests(ModelFixture): @@ -939,6 +922,21 @@ def agent(self, *args, check=True, env=None): self.assertEqual(result.returncode, 0, result.stderr + result.stdout) return result + def test_the_terminal_a_daemon_started_in_never_reaches_a_tool(self): + # The daemon knows no terminal; the CLI that starts it leaves the + # multiplexers' places out of its tool shells. + env = {'REX_SESSION': 'session:daemons', 'REX_BLOCK': 'block:daemons', + 'TMUX': '/tmp/tmux-0/default,1,0', 'TMUX_PANE': '%0', 'HERDR_ENV': '1', + 'HERDR_WORKSPACE_ID': 'w1', 'HERDR_TAB_ID': 'w1:t1', 'HERDR_PANE_ID': 'w1:p1', + 'CMUX_WORKSPACE_ID': 'ws-1', 'CMUX_SURFACE_ID': 'sf-1', 'CMUX_TAB_ID': 'ws-1', + 'CMUX_PANEL_ID': 'sf-1'} + self.agent('run', *self.common, '--new', '--bot', 'Bob', + 'shell:printf "${REX_SESSION-unset}${REX_BLOCK-unset}${TMUX-unset}${TMUX_PANE-unset}' + '${HERDR_ENV-unset}${HERDR_WORKSPACE_ID-unset}${HERDR_TAB_ID-unset}${HERDR_PANE_ID-unset}' + '${CMUX_WORKSPACE_ID-unset}${CMUX_SURFACE_ID-unset}${CMUX_TAB_ID-unset}${CMUX_PANEL_ID-unset}" > seen', + env=env) + self.assertEqual((self.path / 'seen').read_text(), 'unset' * 12) + def test_manual_mode_gates_every_tool_but_the_bots_own_records(self): submitted = json.loads(self.agent('run', *self.common, '--new', '--bot', 'Bob', '--detach', 'shell:printf ok', env={'AGENT_APPROVAL': 'manual'}).stdout) diff --git a/tests/test_context_eval.py b/tests/test_context_eval.py index 9c620ef59..9f6744573 100644 --- a/tests/test_context_eval.py +++ b/tests/test_context_eval.py @@ -30,7 +30,7 @@ class ContextEvalTests(unittest.TestCase): def test_bots_get_the_clients_current_policy_text(self): # Read from client/src/policy.rs, so an edit there reaches the bench. self.assertTrue(INSTRUCTIONS.startswith('To delegate a subtask')) - self.assertIn('contact your creator only to ask something you need', INSTRUCTIONS) + self.assertIn('Your final reply reaches whoever waits on your turn.', INSTRUCTIONS) self.assertIn('"$AGENT_BIN" run --detach', INSTRUCTIONS) self.assertTrue(COMPACTION.endswith('Reply with the summary only.')) self.assertFalse(any('\\' in text or '\n' in text or ' ' in text for text in (INSTRUCTIONS, COMPACTION))) diff --git a/tests/test_daemon.py b/tests/test_daemon.py index 54c2b4572..cc068888c 100644 --- a/tests/test_daemon.py +++ b/tests/test_daemon.py @@ -262,8 +262,8 @@ def test_admissions_sent_together_answer_in_order_and_start_once(self): bots = [f'b{n}' for n in range(8)] requests = [dict(op='create', bot=bot, workspace=str(self.path), model='openai/synthetic-model', instructions='Test agent.', tools=['echo']) for bot in bots] - requests += [dict(op='submit', bot=bot, request_id='r1', prompt='hi') for bot in bots] - requests += [dict(op='submit', bot='b0', request_id='r1', prompt='hi'), dict(op='stats')] + requests += [dict(op='submit', bot=bot, request_id='r1', prompt='hi', delivery='reject') for bot in bots] + requests += [dict(op='submit', bot='b0', request_id='r1', prompt='hi', delivery='reject'), dict(op='stats')] # One write carries every request: each submission sees the creation # queued before it, and the replies keep request order. together = Connection(client.socket_path) @@ -305,7 +305,7 @@ def test_large_creations_sent_together_all_answer(self): instructions=text, compaction_instructions=text, tools=['echo']) for n in range(32)] lines = [dict(id=n, **r) for n, r in enumerate(creates)] + [dict(id=len(creates), op='stats')] # A bad id is refused after the admission queued before it. - lines.insert(1, dict(id=dict(bad=True), op='submit', bot='big0', request_id='r1', prompt='hi')) + lines.insert(1, dict(id=dict(bad=True), op='submit', bot='big0', request_id='r1', prompt='hi', delivery='reject')) together = Connection(client.socket_path) self.addCleanup(together.close) together.socket.sendall(''.join(json.dumps(line) + '\n' for line in lines).encode()) diff --git a/tests/test_pi_durable.py b/tests/test_pi_durable.py index 5c02eb6dc..f0e1dd23a 100644 --- a/tests/test_pi_durable.py +++ b/tests/test_pi_durable.py @@ -121,7 +121,7 @@ def test_a_request_id_sent_twice_at_once_is_one_turn(self): client.request('create', bot='bob', workspace=directory) for id in (101, 102): client.process.stdin.write(json.dumps({'id': id, 'op': 'submit', 'bot': 'bob', - 'request_id': 'r1', 'prompt': 'hi'}) + '\n') + 'request_id': 'r1', 'prompt': 'hi', 'delivery': 'reject'}) + '\n') client.process.stdin.flush() replies = [client.receive(lambda m, id=id: m.get('id') == id)['result'] for id in (101, 102)] self.assertEqual(replies[0]['turn'], replies[1]['turn']) diff --git a/tests/test_provider_pacing.py b/tests/test_provider_pacing.py index de8111444..a80651790 100644 --- a/tests/test_provider_pacing.py +++ b/tests/test_provider_pacing.py @@ -91,10 +91,10 @@ def setUp(self): self.addCleanup(self.server.shutdown) self.binary = root/'.local/target/release/agent' - def client(self, suffix='', extra=()): + def client(self, suffix='', extra=(), settings=None): client = Client(self.binary, self.path/f'store{suffix}.sqlite', f'http://127.0.0.1:{self.server.server_port}/v1', - extra=extra, settings={'max_output_tokens': 800}) + extra=extra, settings={'max_output_tokens': 800, **(settings or {})}) self.addCleanup(client.close, kill=True) return client @@ -172,11 +172,12 @@ def test_permanent_quota_fails_once_without_pausing_the_pool(self): def test_billed_retries_share_the_durable_turn_round_limit(self): # Exercise the limit ending on a successful tool plan and on a billed # failure. Committed tools still run; no 201st request reaches the model. + # The bound is the bot's own: unset, a turn has none. for mode, retries in (('rounds', 100), ('rounds-failure', 99)): with self.subTest(mode=mode): self.server.calls = 0 self.server.mode = mode - client = self.client(mode) + client = self.client(mode, settings={'max_rounds': 200}) turn = self.submit(client, 'A') done = client.receive(lambda e: e.get('event') == 'turn_finished' and e.get('turn') == turn, timeout=30) diff --git a/tests/test_runtime.py b/tests/test_runtime.py index a3c783e7b..80a5013e0 100644 --- a/tests/test_runtime.py +++ b/tests/test_runtime.py @@ -1685,7 +1685,7 @@ def held_admissions(self, client, count): db.executemany('INSERT INTO burn VALUES (?)', [(n,) for n in range(6000)]) db.execute("CREATE TRIGGER slow_admission AFTER INSERT ON turns WHEN NEW.bot='Slow' " "BEGIN SELECT count(*) FROM burn a, burn b WHERE a.x+b.x>=0; END") - return [{'id': f'submit-{bot}', 'op': 'submit', 'bot': bot, 'request_id': 'held', 'prompt': 'wait'} + return [{'id': f'submit-{bot}', 'op': 'submit', 'bot': bot, 'request_id': 'held', 'prompt': 'wait', 'delivery': 'reject'} for bot in bots] def answered(self, client): diff --git a/tests/test_turn_compaction.py b/tests/test_turn_compaction.py index eb8bd76f9..76e785926 100644 --- a/tests/test_turn_compaction.py +++ b/tests/test_turn_compaction.py @@ -149,7 +149,7 @@ def test_a_summary_beside_the_turn_counts_among_its_rounds(self): # beside that boundary's call is counted among the turn's rounds, # which end at the limit. self.model.timeline, self.model.summary_delay = [], 1.0 - client = self.client(tools='shell', settings={'context_bytes': 65536}) + client = self.client(tools='shell', settings={'context_bytes': 65536, 'max_rounds': 200}) client.request('create', bot='Bob', workspace=str(self.path), tools=['shell'], compaction_instructions='Summarize.') turn = client.request('submit', bot='Bob', request_id='1', prompt='long:250x1')['result']['turn'] @@ -168,7 +168,7 @@ def test_a_billed_retry_that_is_the_last_round_calls_no_tools(self): # The 199th call is billed and fails; its retry is the 200th round, # so it goes without calls and the turn ends on its reply. self.model.fail_billed_at = 199 - client = self.client(tools='shell') + client = self.client(tools='shell', settings={'max_rounds': 200}) client.request('create', bot='Bob', workspace=str(self.path), tools=['shell']) turn = client.request('submit', bot='Bob', request_id='1', prompt='long:250x1')['result']['turn'] ended = client.finished(turn, timeout=90) diff --git a/tests/test_wait.py b/tests/test_wait.py index 443c5a62e..3572369c7 100644 --- a/tests/test_wait.py +++ b/tests/test_wait.py @@ -701,8 +701,9 @@ def test_round_budget_survives_parking_and_daemon_restart(self): self.model.all_streaming.set() self.addCleanup(self.model.release_headers.set) client = self.client('wait') - for bot in ('Anchor', 'Loop'): - client.request('create', bot=bot, workspace=str(self.path)) + # The bound is the bot's own setting. + client.request('create', bot='Anchor', workspace=str(self.path)) + client.request('create', bot='Loop', workspace=str(self.path), settings={'max_rounds': 200}) anchor = client.request('submit', bot='Anchor', request_id='a', prompt='gate')['result'] prompt = 'budget:' + anchor['handle'] turn = client.request('submit', bot='Loop', request_id='loop', prompt=prompt)['result']['turn'] @@ -819,7 +820,7 @@ def looks(at_least): lines = [{'id': f'create-{n}', 'op': 'create', 'bot': f'New{n}', 'workspace': str(self.path), 'model': client.model, 'instructions': client.instructions, 'tools': client.tools} for n in range(8)] - lines.append({'id': 'submit', 'op': 'submit', 'bot': 'Carol', 'request_id': 'late', 'prompt': 'hi'}) + lines.append({'id': 'submit', 'op': 'submit', 'bot': 'Carol', 'request_id': 'late', 'prompt': 'hi', 'delivery': 'reject'}) client.process.stdin.write(''.join(json.dumps(line) + '\n' for line in lines)) client.process.stdin.flush() time.sleep(1)