From 50e9cc20376305deef77ef23db84956e1035d67f Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Sat, 3 Oct 2026 21:39:31 -0700 Subject: [PATCH] docs: M6b implementation plan Six closed tasks for consult: registry, CONSULT_WAIT, [consult]/[secrets], ToolName and grants, Decision body, loopback HTTP. Reference is the git bundle ~/boxmaker-refs/m6b-ref.bundle at 5fb05f0. Gate after task 06 is 1611. Co-Authored-By: Grok 4.6 --- docs/plans/M6b/01-loopd-consult-registry.md | 179 ++++++++ docs/plans/M6b/02-loopd-consult-wait.md | 205 +++++++++ docs/plans/M6b/03-brokerd-consult-config.md | 426 ++++++++++++++++++ docs/plans/M6b/04-brokerd-consult-tool.md | 399 ++++++++++++++++ .../M6b/05-brokerd-consult-audit-body.md | 408 +++++++++++++++++ docs/plans/M6b/06-brokerd-consult-http.md | 425 +++++++++++++++++ docs/plans/M6b/README.md | 126 ++++++ .../crates/brokerd/tests/config_consult.rs | 84 ++++ .../crates/brokerd/tests/consult_allow.rs | 185 ++++++++ .../crates/brokerd/tests/consult_deny.rs | 168 +++++++ .../crates/brokerd/tests/consult_secret.rs | 82 ++++ .../crates/brokerd/tests/grants_consult.rs | 76 ++++ .../loopd/tests/broker_port_consult_wait.rs | 22 + .../crates/loopd/tests/consult_registry.rs | 58 +++ docs/plans/M6b/files/runbook-consult.md | 44 ++ 15 files changed, 2887 insertions(+) create mode 100644 docs/plans/M6b/01-loopd-consult-registry.md create mode 100644 docs/plans/M6b/02-loopd-consult-wait.md create mode 100644 docs/plans/M6b/03-brokerd-consult-config.md create mode 100644 docs/plans/M6b/04-brokerd-consult-tool.md create mode 100644 docs/plans/M6b/05-brokerd-consult-audit-body.md create mode 100644 docs/plans/M6b/06-brokerd-consult-http.md create mode 100644 docs/plans/M6b/README.md create mode 100644 docs/plans/M6b/files/crates/brokerd/tests/config_consult.rs create mode 100644 docs/plans/M6b/files/crates/brokerd/tests/consult_allow.rs create mode 100644 docs/plans/M6b/files/crates/brokerd/tests/consult_deny.rs create mode 100644 docs/plans/M6b/files/crates/brokerd/tests/consult_secret.rs create mode 100644 docs/plans/M6b/files/crates/brokerd/tests/grants_consult.rs create mode 100644 docs/plans/M6b/files/crates/loopd/tests/broker_port_consult_wait.rs create mode 100644 docs/plans/M6b/files/crates/loopd/tests/consult_registry.rs create mode 100644 docs/plans/M6b/files/runbook-consult.md diff --git a/docs/plans/M6b/01-loopd-consult-registry.md b/docs/plans/M6b/01-loopd-consult-registry.md new file mode 100644 index 0000000..3eeef9e --- /dev/null +++ b/docs/plans/M6b/01-loopd-consult-registry.md @@ -0,0 +1,179 @@ +# M6b task 01: `consult` in the M5c registry, not core + +**Branch:** `m6b`. Check it: `git branch --show-current` prints `m6b` and `git status --short` +is empty; otherwise stop. Never switch branch. +**Commit subject:** `loopd: consult in the M5c registry, not core` +**Log row:** the Task cell is `M6b/01-loopd-consult-registry`. + +**First action.** Run this before any Read of crate source. Do not `ls` `crates/loopd/tests/`. +Do not Read `tests/support/`, `turn.rs`, `session.rs`, `docs/implementer-log.md` (except the +first 12 lines at the log step). The signatures below are complete. + +```sh +cp docs/plans/M6b/files/crates/loopd/tests/consult_registry.rs crates/loopd/tests/ +``` + +## Goal + +`consult` is in `Registry::m5c()`, not core. `find_tool` finds it. A new session's epoch +`tools` array stays `clock`, `recall`, `memory`, `ask_memory`, `find_tool`, `call_tool` +(spec 4). `loopd` does not answer the call: `dispatch` already Ports every non-core name +other than `subagent` to `brokerd`. + +## Every place that changes + +| Place | What happens there | +|---|---| +| `crates/loopd/src/consult.rs` | new: `CONSULT`, `schema()` | +| `crates/loopd/src/lib.rs` | `pub mod consult;` | +| `crates/loopd/src/tools.rs` | `m5c()` pushes the non-core entry after `subagent` | +| `crates/loopd/tests/consult_registry.rs` | new, given | +| everything else | **no change**: not `dispatch`, not `broker_port.rs` | + +## Files + +- Copy, from `docs/plans/M6b/files/` (the same path under it): the new test + `crates/loopd/tests/consult_registry.rs`. +- Create: `crates/loopd/src/consult.rs` (the module below). +- Modify: `crates/loopd/src/lib.rs`, `crates/loopd/src/tools.rs`, + `docs/implementer-log.md`. +- Do not touch: any other file. + +At the log-and-commit step, read only the first 12 lines of `docs/implementer-log.md`. + +## Facts you need + +- **Not core.** `core_schemas()` already filters `e.core`. A non-core `Entry` is found by + `find` and by `get`, and is absent from `Baseline.tools`. +- **`m5c()` only.** Do not add the entry to `m2b`, `m3a`, or `m5a`. `loopd serve` uses `m5c`. +- **`push`, after `subagent`.** `ask_memory` stays at index 3. `consult` is last among the + non-core names. +- rustfmt `mod` order: `consult` after `config` / before `cron` as rustfmt decides. +- Same-crate modules may import each other. This task's module imports nothing from + `turn` or `tools`. +- The items the edits use. This table is complete. + + | Item | Signature | + |---|---| + | `ToolSchema` | `{ name: String, description: String, parameters: serde_json::Value }` in `crate::llama` | + | `Entry` | `{ schema: ToolSchema, core: bool }` | + | `Registry::m5c` | `m5c() -> Registry` | + | `Registry::get` | `get(&self, name: &str) -> Option<&Entry>` | + | `Registry::find` | `find(&self, query: &str) -> Vec<&ToolSchema>` | + | `Registry::core_schemas` | `core_schemas(&self) -> Vec` | + +## The edits, exactly + +### `crates/loopd/src/consult.rs` + +Write this module. Do not add a wait constant, a parse, or a runtime (later tasks). + +```rust +//! The `consult` tool: `brokerd` decides it and POSTs an allowlisted body (M6 spec, section 4). +//! It is not core; the parent finds it with `find_tool` and calls it through `call_tool`. + +pub const CONSULT: &str = "consult"; + +pub fn schema() -> crate::llama::ToolSchema { + crate::llama::ToolSchema { + name: CONSULT.to_string(), + description: "Ask a remote model a question. The session history is not sent. Needs a grant from the owner.".to_string(), + parameters: serde_json::json!({ + "type": "object", + "properties": { + "question": { + "type": "string", + "description": "The question to send. Nothing else of the session is included." + } + }, + "required": ["question"] + }), + } +} +``` + +### `crates/loopd/src/lib.rs` + +**L1.** After `pub mod config;` add `pub mod consult;`. rustfmt decides the final order. + +### `crates/loopd/src/tools.rs` + +**R1.** The lines (exactly once, inside `m5c`) + +```rust + entries.push(Entry { + schema: crate::subagent::schema(), + core: false, + }); + Registry::new(entries) +``` + +become + +```rust + entries.push(Entry { + schema: crate::subagent::schema(), + core: false, + }); + entries.push(Entry { + schema: crate::consult::schema(), + core: false, + }); + Registry::new(entries) +``` + +## Steps + +- [ ] **1. Copy the given test.** + + ```sh + cp docs/plans/M6b/files/crates/loopd/tests/consult_registry.rs crates/loopd/tests/ + ``` + +- [ ] **2. See it fail.** `cargo build --tests --keep-going -p loopd`, with a shell timeout of + 600000 ms: test target `consult_registry` does not compile (`E0432` unresolved + `loopd::consult`). Every other target compiles. +- [ ] **3. The edits.** Write `consult.rs`, L1, R1. `cargo check -p loopd` prints no error + and no warning. `wc -l crates/loopd/src/consult.rs` and `wc -l crates/loopd/src/tools.rs` + each ≤ 500. +- [ ] **4. The tests.** `cargo test -p loopd --test consult_registry --test subagent_registry + --test ask_memory_tool --test tools --test baseline`: every test passes, 2 in + `consult_registry`. +- [ ] **5. Check.** `git diff --stat` lists exactly `crates/loopd/src/consult.rs`, + `crates/loopd/src/lib.rs`, `crates/loopd/src/tools.rs`. `git status --short` also shows + the new test as untracked. +- [ ] **6. Run the gate.** `cargo fmt --all`, then `make gate`, with a shell timeout of + 600000 ms. Expected last line: `gate: ok`. That is done counting: **1592** (1590 before, + plus the 2 of `consult_registry.rs`). After `gate: ok`, do not add up `test result:` + lines and do not run `cargo test --workspace`. +- [ ] **7. Log and commit.** Add your row to `docs/implementer-log.md`; its Task cell is + `M6b/01-loopd-consult-registry`. Read only the first 12 lines of that file; insert the + row under the header. Stage with + `git add crates/loopd docs/implementer-log.md Cargo.lock`, then commit as `AGENTS.md` + says. `Cargo.lock` does not change in this task; the `git add` is harmless. + +## Done when + +- `make gate` prints `gate: ok` with 1592 tests. +- `git diff HEAD~1 --stat` lists only `crates/loopd/src/consult.rs`, + `crates/loopd/src/lib.rs`, `crates/loopd/src/tools.rs`, + `crates/loopd/tests/consult_registry.rs` and `docs/implementer-log.md`. + +## Stop and report if + +- The branch check at the top fails. +- A "before" block is not in its file exactly as shown, or is there more than once. +- Step 2 shows a different result: the tests compile, or a different crate fails. +- `cargo check` in step 3 prints an error or a warning. Quote it, and stop. +- A test fails in step 4. Do not change the test: stop, and give the failing tests' names + and their messages. +- The fix seems to need a change anywhere else. It does not; stop and say why you think so. +- A test outside `consult_registry.rs` fails in the gate: check once whether it fails + without your change (`git stash -u`, run it, `git stash pop`), then stop and report both + results. + +## What the given tests pin + +`consult` is in `m5c()`, `core` is false, `schema()` matches the entry, `find("consult")` +and `find("remote")` both hit it, `subagent` is still present, and a baseline assembled +from `m5c()` still lists only the six core names in `tools`. diff --git a/docs/plans/M6b/02-loopd-consult-wait.md b/docs/plans/M6b/02-loopd-consult-wait.md new file mode 100644 index 0000000..9abea54 --- /dev/null +++ b/docs/plans/M6b/02-loopd-consult-wait.md @@ -0,0 +1,205 @@ +# M6b task 02: `CONSULT_WAIT` through `timeout_for` + +**Branch:** `m6b`. Check it: `git branch --show-current` prints `m6b` and `git status --short` +is empty; otherwise stop. Never switch branch. +**Commit subject:** `loopd: consult waits one hundred and eighty seconds` +**Log row:** the Task cell is `M6b/02-loopd-consult-wait`. + +**First action.** Run this before any Read of crate source. Do not `ls` `crates/loopd/tests/`. +Do not Read `tests/support/`, `config.rs`, `docs/implementer-log.md` (except the first 12 +lines at the log step). Do not edit `crates/loopd/tests/broker_port_ask_wait.rs` or +`crates/loopd/src/config.rs`. The quoted blocks are the only edits. + +```sh +cp docs/plans/M6b/files/crates/loopd/tests/broker_port_consult_wait.rs crates/loopd/tests/ +``` + +## Goal + +`BrokerPort::timeout_for("consult")` is 180 s, or `[broker] timeout_ms` if that is longer +(spec 4.3). The default `[broker] timeout_ms` stays 120_000. `ask_memory` is unchanged. + +No test sleeps (T27). This task does not talk to `brokerd`. + +## Every place that changes + +| Place | What happens there | +|---|---| +| `crates/loopd/src/broker_port.rs` | `CONSULT_WAIT`; `consult_timeout` on `new` and `with_log`; `timeout_for` arm | +| `crates/loopd/tests/broker_port_consult_wait.rs` | new, given | +| everything else | **no change**: not `config.rs`, not `broker_port_ask_wait.rs` | + +## Files + +- Copy, from `docs/plans/M6b/files/` (the same path under it): the new test + `crates/loopd/tests/broker_port_consult_wait.rs`. +- Modify: `crates/loopd/src/broker_port.rs`, `docs/implementer-log.md`. +- Do not touch: any other file. Do not edit `crates/loopd/tests/broker_port_ask_wait.rs` + (`the_wait_for_each_tool` does not list `consult`; leaving it unedited is required). + Do not change the default `timeout_ms: 120_000` in `crates/loopd/src/config.rs`. + +## Facts you need + +- **Floor, not a replacement.** `timeout_for("consult")` is + `self.timeout.max(self.consult_timeout)`. A configured 300 s wait stays 300 s. +- **Both constructors.** `new` and `with_log` each name `consult_timeout: CONSULT_WAIT`. + `with_ask_wait` and `with_pending_cap` use `..self` and need no edit. +- **`ASK_MEMORY_WAIT` stays.** Do not change its value or its arm. +- The items the edits use. This table is complete. + + | Item | Signature | + |---|---| + | `BrokerPort::new` | `new(socket: PathBuf, timeout: Duration) -> BrokerPort` | + | `BrokerPort::with_log` | `with_log(socket: PathBuf, timeout: Duration, log: Box) -> BrokerPort` | + | `BrokerPort::timeout_for` | `timeout_for(&self, tool: &str) -> Duration` | + | `crate::consult::CONSULT` | `&str`, `"consult"` (task 01) | + | `crate::memory::ASK_MEMORY` | `&str`, `"ask_memory"` | + +## The edits, exactly + +Each quoted "before" block occurs exactly once after task 01. + +### `crates/loopd/src/broker_port.rs` + +**W1.** After `pub const ASK_MEMORY_WAIT: Duration = Duration::from_secs(180);` add: + +```rust +/// The longest `BrokerPort` waits for a `consult` answer, unless `[broker] timeout_ms` is +/// longer: `brokerd`'s `[consult] timeout_ms` defaults to 180 s (M6 spec, section 4.3). +pub const CONSULT_WAIT: Duration = Duration::from_secs(180); +``` + +**W2.** The struct fields + +```rust + socket: PathBuf, + timeout: Duration, + ask_timeout: Duration, + pending_cap: Duration, + log: Box, +``` + +become + +```rust + socket: PathBuf, + timeout: Duration, + ask_timeout: Duration, + consult_timeout: Duration, + pending_cap: Duration, + log: Box, +``` + +**W3.** Inside `new`, the fields + +```rust + socket, + timeout, + ask_timeout: ASK_MEMORY_WAIT, + pending_cap: MAX_PENDING_WAIT, + log: Box::new(|l| eprintln!("{l}")), +``` + +become + +```rust + socket, + timeout, + ask_timeout: ASK_MEMORY_WAIT, + consult_timeout: CONSULT_WAIT, + pending_cap: MAX_PENDING_WAIT, + log: Box::new(|l| eprintln!("{l}")), +``` + +**W4.** The comment and body of `timeout_for` + +```rust + /// The wait for one call of `tool`, used wherever the port waits for that call: the timeout, + /// or for `ask_memory` the longer of the timeout and its own wait. + pub fn timeout_for(&self, tool: &str) -> Duration { + if tool == crate::memory::ASK_MEMORY { + self.timeout.max(self.ask_timeout) + } else { + self.timeout + } + } +``` + +become + +```rust + /// The wait for one call of `tool`, used wherever the port waits for that call: the timeout, + /// or for `ask_memory` and `consult` the longer of the timeout and that tool's own wait. + pub fn timeout_for(&self, tool: &str) -> Duration { + if tool == crate::memory::ASK_MEMORY { + self.timeout.max(self.ask_timeout) + } else if tool == crate::consult::CONSULT { + self.timeout.max(self.consult_timeout) + } else { + self.timeout + } + } +``` + +**W5.** Inside `with_log`, the fields + +```rust + socket, + timeout, + ask_timeout: ASK_MEMORY_WAIT, + pending_cap: MAX_PENDING_WAIT, + log, +``` + +become + +```rust + socket, + timeout, + ask_timeout: ASK_MEMORY_WAIT, + consult_timeout: CONSULT_WAIT, + pending_cap: MAX_PENDING_WAIT, + log, +``` + +## Steps + +- [ ] **1. Copy the given test.** + + ```sh + cp docs/plans/M6b/files/crates/loopd/tests/broker_port_consult_wait.rs crates/loopd/tests/ + ``` + +- [ ] **2. See it fail.** `cargo test -p loopd --test broker_port_consult_wait`, timeout + 600000 ms: the test target does not compile (`E0432` unresolved `CONSULT_WAIT`). +- [ ] **3. The edits.** W1–W5. `cargo check -p loopd` prints no error and no warning. + `wc -l crates/loopd/src/broker_port.rs` ≤ 500. +- [ ] **4. The tests.** `cargo test -p loopd --test broker_port_consult_wait --test + broker_port_ask_wait`: every test passes, 1 in `broker_port_consult_wait`. + `the_wait_for_each_tool` still passes. +- [ ] **5. Run the gate.** `cargo fmt --all`, then `make gate`, timeout 600000 ms. Last + line `gate: ok`. Test count **1593** (1592 before, plus 1). After `gate: ok`, do not add + up `test result:` lines. +- [ ] **6. Log and commit.** Task cell `M6b/02-loopd-consult-wait`. Read only the first 12 + lines of `docs/implementer-log.md`; insert the row under the header. Stage with + `git add crates/loopd docs/implementer-log.md Cargo.lock`, then commit as `AGENTS.md` + says. + +## Done when + +- `make gate` prints `gate: ok` with 1593 tests. +- `git diff HEAD~1 --stat` lists only `crates/loopd/src/broker_port.rs`, + `crates/loopd/tests/broker_port_consult_wait.rs` and the log. + +## Stop and report if + +- The branch check fails; a "before" block is missing or duplicated; step 2 compiles; you + edit `broker_port_ask_wait.rs` or `config.rs`; `cargo check` warns; `the_wait_for_each_tool` + fails; a test outside the named files fails in the gate (`git stash -u`, rerun, + `git stash pop`, report both). + +## What the given tests pin + +`CONSULT_WAIT` is 180 s. `timeout_for("consult")` is 180 s when the port timeout is 120 s, +and 300 s when the port timeout is 300 s. `timeout_for("recall")` stays the port timeout. +`with_log` matches `new`. diff --git a/docs/plans/M6b/03-brokerd-consult-config.md b/docs/plans/M6b/03-brokerd-consult-config.md new file mode 100644 index 0000000..3519f31 --- /dev/null +++ b/docs/plans/M6b/03-brokerd-consult-config.md @@ -0,0 +1,426 @@ +# M6b task 03: `[consult]` and `[secrets]`, start fails closed + +**Branch:** `m6b`. Check it: `git branch --show-current` prints `m6b` and `git status --short` +is empty; otherwise stop. Never switch branch. +**Commit subject:** `brokerd: consult config, secrets, loopback URL` +**Log row:** the Task cell is `M6b/03-brokerd-consult-config`. + +**First action.** Run this before any Read of crate source. Do not `ls` +`crates/brokerd/tests/`. Do not Read `args.rs`, `broker.rs`, `ledger/`, +`docs/implementer-log.md` (except the first 12 lines at the log step). Do not edit +`crates/brokerd/tests/config.rs`. The signatures below are complete. + +```sh +cp docs/plans/M6b/files/crates/brokerd/tests/config_consult.rs crates/brokerd/tests/ +cp docs/plans/M6b/files/crates/brokerd/tests/consult_secret.rs crates/brokerd/tests/ +``` + +Then append the runbook fragment (step 1 of Steps names the exact command). + +## Goal + +`brokerd.toml` may have `[consult]` and `[secrets.]`. A present `[consult]` is +validated at load: loopback `http://` only, non-empty model and secret, `timeout_ms` ≥ 1, +temperature from 0 to 0.6 inclusive. Temperature is stored as `temperature_milli: u16` so +`Config` stays `Eq`. No `[consult]` is `None` and an empty `[secrets]` map. Loading the +named secret at start is `load_secret`; a missing or unreadable secret is +`consult-secret-unreadable` and `brokerd` must not start. This task does not POST, parse +tool arguments, or change `ToolName`. + +## Every place that changes + +| Place | What happens there | +|---|---| +| `crates/brokerd/Cargo.toml` | `bxcore` enables feature `secrets` | +| `crates/brokerd/src/consult.rs` | new: `Consult`, `validate`, `parse_loopback_http`, `load_secret` | +| `crates/brokerd/src/lib.rs` | `pub mod consult;` | +| `crates/brokerd/src/config.rs` | `consult`, `secrets`; validate at load | +| `crates/brokerd/tests/support/rig.rs` | the `Config { }` literal names the new fields | +| `docs/runbook.md` | append both consult entries | +| `crates/brokerd/tests/config_consult.rs` | new, given | +| `crates/brokerd/tests/consult_secret.rs` | new, given | +| everything else | **no change**: not `ToolName`, not `main.rs`, not `ledger` | + +## Files + +- Copy, from `docs/plans/M6b/files/` (the same path under it): the new tests + `crates/brokerd/tests/config_consult.rs` and + `crates/brokerd/tests/consult_secret.rs`. +- Append, from `docs/plans/M6b/files/runbook-consult.md`, to `docs/runbook.md`. Both + headings go in now so `scripts/check-runbook.sh` accepts `SECRET_POINTER`; task 05 + adds the `consult-not-configured` pointer in code. +- Create: `crates/brokerd/src/consult.rs` (the module below). +- Modify: `crates/brokerd/Cargo.toml`, `crates/brokerd/src/lib.rs`, + `crates/brokerd/src/config.rs`, `crates/brokerd/tests/support/rig.rs`, + `docs/runbook.md`, `docs/implementer-log.md`. +- Do not touch: any other file. Do not edit `crates/brokerd/tests/config.rs`. + After the Cargo.toml change, run `cargo build` once so `Cargo.lock` updates. + +## Facts you need + +- **No vendor names.** The URL, model and secret name are configuration. The module must + not mention a cloud provider. +- **Loopback HTTP only.** `parse_loopback_http` accepts `http://127.0.0.1` or + `http://localhost`, optional port 1–65535 and path. `https://`, a non-loopback host, a + userinfo (`@`), or port `0` is an error. Default port is 80. A URL with no path uses + `"/"`. +- **Temperature.** TOML `temperature` is an `f64` in 0..=0.6, or the integer `0`. Any + other integer is invalid (`must be from 0 to 0.6`). Store `(v * 1000.0).round()` as + `u16` via `format!("{milli:.0}").parse::()`. Default 600 (0.6). `Config` stays + `Eq`; do not store `f64` on `Consult`. +- **`#[serde(default)]` on the new `Config` fields.** `an_empty_file_gets_every_default` + in `config.rs` compares a loaded empty file to `Config::default()`. Missing tables are + `consult: None` and `secrets: {}`. +- **The only `Config { }` literal** is `crates/brokerd/tests/support/rig.rs`. Add + `consult: None, secrets: Default::default()`. Other tests load TOML. +- **Secrets.** Same `SecretSpec` as `gatewayd`: exactly one of `credential` / `env` / + `file`. `load` is `bxcore::secrets::load`. Role string `"brokerd"`. Load the consult + secret only when `[consult]` is present; absent table is `Ok(None)`. +- **Do not add `ToolArgs`, `QUESTION_CAP`, `NOT_CONFIGURED`, a transport, or HTTP.** + Those are later tasks. An unused constant fails clippy. +- The items the edits use. This table is complete. + + | Item | Signature | + |---|---| + | `SecretSpec` | `{ credential: Option, env: Option, file: Option }` in `bxcore::secrets` | + | `SecretSpec::source` | `source(&self) -> Result` | + | `bxcore::secrets::load` | `load(role: &str, name: &str, source: &SecretSource, env: &dyn Fn(&str) -> Option) -> Result` | + | `Loaded` | `{ secret: Secret, warning: Option }` | + | `SecretError` | `Display` names the secret and `docs/runbook.md#secret-unavailable` | + | `Config::load` | `load(path: &Path) -> Result` | + | `ConfigError::Invalid` | `Invalid(PathBuf, String)` | + | `ConfigError::Parse` | `Parse(PathBuf, toml::de::Error)` | + +## The edits, exactly + +### `crates/brokerd/Cargo.toml` + +**C1.** The line + +```toml +bxcore.workspace = true +``` + +becomes + +```toml +bxcore = { workspace = true, features = ["secrets"] } +``` + +Then `cargo build` once (timeout 600000 ms) so `Cargo.lock` records the feature. + +### `crates/brokerd/src/lib.rs` + +**L1.** After `pub mod config;` add `pub mod consult;`. rustfmt decides the final order. + +### `crates/brokerd/src/consult.rs` + +Write this module. Do not add `ConsultArgs`, `wire_body`, `ConsultTransport`, or +`ConsultRuntime`. + +```rust +//! `consult`: an allowlisted chat-completions POST decided by the grant's `max_taint`. +//! Nothing here names a vendor (M6 spec, section 4). + +use std::ffi::OsString; +use std::time::Duration; + +use bxcore::secrets::{Loaded, SecretError, SecretSpec}; +use serde::{Deserialize, Deserializer}; + +use crate::config::Config; + +pub const CONSULT: &str = "consult"; +/// Default wait for the remote completion, milliseconds. +pub const DEFAULT_TIMEOUT_MS: u64 = 180_000; +/// Default sampling temperature as thousandths (0.6). +pub const DEFAULT_TEMPERATURE_MILLI: u16 = 600; +/// Start failure when the named secret cannot be read. +pub const SECRET_POINTER: &str = "see docs/runbook.md#consult-secret-unreadable"; + +/// `[consult]` in `brokerd.toml`. Unknown keys are errors. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Consult { + pub url: String, + pub model: String, + #[serde( + rename = "temperature", + default = "default_temperature_milli", + deserialize_with = "de_temperature_milli" + )] + pub temperature_milli: u16, + #[serde(default = "default_timeout_ms")] + pub timeout_ms: u64, + pub secret: String, +} + +fn default_temperature_milli() -> u16 { + DEFAULT_TEMPERATURE_MILLI +} + +fn default_timeout_ms() -> u64 { + DEFAULT_TIMEOUT_MS +} + +fn de_temperature_milli<'de, D: Deserializer<'de>>(deserializer: D) -> Result { + #[derive(Deserialize)] + #[serde(untagged)] + enum Num { + F(f64), + I(i64), + } + let v = match Num::deserialize(deserializer).map_err(serde::de::Error::custom)? { + Num::F(f) => f, + Num::I(0) => 0.0, + Num::I(_) => { + return Err(serde::de::Error::custom( + "[consult] temperature must be from 0 to 0.6", + )); + } + }; + if !v.is_finite() || v < 0.0 || v > 0.6 { + return Err(serde::de::Error::custom( + "[consult] temperature must be from 0 to 0.6", + )); + } + let milli = (v * 1000.0).round(); + let text = format!("{milli:.0}"); + text.parse::().map_err(serde::de::Error::custom) +} + +impl Consult { + /// The temperature written in the JSON body. + pub fn temperature(&self) -> f64 { + f64::from(self.temperature_milli) / 1000.0 + } + + pub fn timeout(&self) -> Duration { + Duration::from_millis(self.timeout_ms) + } +} + +/// Why a `[consult]` table cannot be used. +pub fn validate(consult: &Consult) -> Result<(), String> { + parse_loopback_http(&consult.url).map(|_| ())?; + if consult.model.is_empty() { + return Err("[consult] model must not be empty".to_string()); + } + if consult.timeout_ms == 0 { + return Err("[consult] timeout_ms must be at least 1".to_string()); + } + if consult.secret.is_empty() { + return Err("[consult] secret must not be empty".to_string()); + } + if consult.temperature_milli > DEFAULT_TEMPERATURE_MILLI { + return Err("[consult] temperature must be from 0 to 0.6".to_string()); + } + Ok(()) +} + +/// Host, port and path of a loopback `http://` URL. M6 stays on loopback. +pub fn parse_loopback_http(url: &str) -> Result<(String, u16, String), String> { + let rest = match url.strip_prefix("http://") { + Some(rest) => rest, + None => { + return Err("[consult] url must be http://127.0.0.1 or http://localhost".to_string()); + } + }; + if rest.contains('@') { + return Err("[consult] url must not include a userinfo".to_string()); + } + let (hostport, path) = match rest.find('/') { + Some(i) => match rest.split_at_checked(i) { + Some(pair) => pair, + None => { + return Err("[consult] url is not a valid http URL".to_string()); + } + }, + None => (rest, "/"), + }; + let path = if path.is_empty() { "/" } else { path }; + let (host, port) = match hostport.split_once(':') { + Some((host, port)) => { + let port: u16 = port + .parse() + .map_err(|_| "[consult] url port is not a number".to_string())?; + if port == 0 { + return Err("[consult] url port must be from 1 to 65535".to_string()); + } + (host, port) + } + None => (hostport, 80), + }; + if host != "127.0.0.1" && host != "localhost" { + return Err("[consult] url must be http://127.0.0.1 or http://localhost".to_string()); + } + Ok((host.to_string(), port, path.to_string())) +} + +/// Why `brokerd` will not start with `[consult]` set. +#[derive(Debug)] +pub enum SecretStartError { + Missing(String), + Spec(String, String), + Load(SecretError), +} + +impl std::fmt::Display for SecretStartError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + SecretStartError::Missing(name) => { + write!(f, "secret {name} is not in [secrets]; {SECRET_POINTER}") + } + SecretStartError::Spec(name, why) => { + write!(f, "[secrets.{name}]: {why}; {SECRET_POINTER}") + } + SecretStartError::Load(err) => write!(f, "{err}\n{SECRET_POINTER}"), + } + } +} + +impl std::error::Error for SecretStartError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + SecretStartError::Load(err) => Some(err), + SecretStartError::Missing(_) | SecretStartError::Spec(_, _) => None, + } + } +} + +/// Load the bearer for `[consult]`, or `Ok(None)` when the table is absent. +pub fn load_secret( + cfg: &Config, + env: &dyn Fn(&str) -> Option, +) -> Result, SecretStartError> { + let Some(consult) = &cfg.consult else { + return Ok(None); + }; + let spec: &SecretSpec = match cfg.secrets.get(&consult.secret) { + Some(spec) => spec, + None => return Err(SecretStartError::Missing(consult.secret.clone())), + }; + let source = spec + .source() + .map_err(|why| SecretStartError::Spec(consult.secret.clone(), why))?; + let loaded = bxcore::secrets::load("brokerd", &consult.secret, &source, env) + .map_err(SecretStartError::Load)?; + Ok(Some(loaded)) +} +``` + +### `crates/brokerd/src/config.rs` + +**F1.** The imports + +```rust +use serde::Deserialize; +use std::path::{Path, PathBuf}; +``` + +become + +```rust +use serde::Deserialize; +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +use bxcore::secrets::SecretSpec; +``` + +**F2.** After `pub memory: Memory,` (the last field of `Config`) add: + +```rust + #[serde(default)] + pub consult: Option, + #[serde(default)] + pub secrets: BTreeMap, +``` + +**F3.** In `Config::load`, immediately before `Ok(config)`, add: + +```rust + if let Some(consult) = &config.consult + && let Err(why) = crate::consult::validate(consult) + { + return Err(ConfigError::Invalid(path.to_path_buf(), why)); + } +``` + +Do not change `time_limit` (no `ToolName::Consult` yet). + +### `crates/brokerd/tests/support/rig.rs` + +**R1.** The `Config {` fields ending + +```rust + gateway: None, + memory: Default::default(), + }; +``` + +become + +```rust + gateway: None, + memory: Default::default(), + consult: None, + secrets: Default::default(), + }; +``` + +### `docs/runbook.md` + +**B1.** Append the file `docs/plans/M6b/files/runbook-consult.md` as-is (both `##` +headings) after the last line of `docs/runbook.md`. + +## Steps + +- [ ] **1. Copy the given tests and append the runbook.** + + ```sh + cp docs/plans/M6b/files/crates/brokerd/tests/config_consult.rs crates/brokerd/tests/ + cp docs/plans/M6b/files/crates/brokerd/tests/consult_secret.rs crates/brokerd/tests/ + cat docs/plans/M6b/files/runbook-consult.md >> docs/runbook.md + ``` + +- [ ] **2. See them fail.** `cargo test -p brokerd --test config_consult --test + consult_secret`, timeout 600000 ms: both targets do not compile (`E0432` unresolved + `brokerd::consult`). +- [ ] **3. The edits.** C1 (then `cargo build`), L1, write `consult.rs`, F1–F3, R1, B1. + `cargo check -p brokerd` prints no error and no warning. `wc -l + crates/brokerd/src/consult.rs` and `wc -l crates/brokerd/src/config.rs` each ≤ 500. +- [ ] **4. The tests.** `cargo test -p brokerd --test config_consult --test consult_secret + --test config`: every test passes, 6 in `config_consult`, 3 in `consult_secret`. + `an_empty_file_gets_every_default` still passes. +- [ ] **5. Run the gate.** `cargo fmt --all`, then `make gate`, timeout 600000 ms. Last + line `gate: ok`. Test count **1602** (1593 before, plus 9). After `gate: ok`, do not add + up `test result:` lines. +- [ ] **6. Log and commit.** Task cell `M6b/03-brokerd-consult-config`. Read only the first + 12 lines of `docs/implementer-log.md`; insert the row under the header. Stage with + `git add crates/brokerd Cargo.lock docs/runbook.md docs/implementer-log.md`, then + commit as `AGENTS.md` says. + +## Done when + +- `make gate` prints `gate: ok` with 1602 tests. +- `git diff HEAD~1 --stat` lists only `crates/brokerd/Cargo.toml`, + `crates/brokerd/src/consult.rs`, `crates/brokerd/src/lib.rs`, + `crates/brokerd/src/config.rs`, `crates/brokerd/tests/support/rig.rs`, + `crates/brokerd/tests/config_consult.rs`, `crates/brokerd/tests/consult_secret.rs`, + `docs/runbook.md`, the log, and `Cargo.lock` if it changed. + +## Stop and report if + +- The branch check fails; a "before" block is missing or duplicated; step 2 compiles; you + edit `tests/config.rs` or add `ToolName::Consult`; `cargo check` warns; existing + `config` tests fail; a test outside the named files fails in the gate (`git stash -u`, + rerun, `git stash pop`, report both). + +## What the given tests pin + +No `[consult]` is `None` with empty secrets. A table loads url/model/secret and defaults +temperature 0.6 / timeout 180_000. `temperature = 0.7` is parse-invalid. A non-loopback +URL is `ConfigError::Invalid`. An unknown `[consult]` key is parse-invalid. `[secrets.consult]` +loads. A missing secrets entry and an unset env secret both name +`consult-secret-unreadable`; a present env secret loads. diff --git a/docs/plans/M6b/04-brokerd-consult-tool.md b/docs/plans/M6b/04-brokerd-consult-tool.md new file mode 100644 index 0000000..ce5856a --- /dev/null +++ b/docs/plans/M6b/04-brokerd-consult-tool.md @@ -0,0 +1,399 @@ +# M6b task 04: `ToolName::Consult`, parse, grants + +**Branch:** `m6b`. Check it: `git branch --show-current` prints `m6b` and `git status --short` +is empty; otherwise stop. Never switch branch. +**Commit subject:** `brokerd: consult tool name, parse, grants` +**Log row:** the Task cell is `M6b/04-brokerd-consult-tool`. + +**First action.** Run this before any Read of crate source. Do not `ls` +`crates/brokerd/tests/`. Do not Read `broker.rs`, `ledger/`, `main.rs`, +`docs/implementer-log.md` (except the first 12 lines at the log step). Do not edit +`crates/brokerd/tests/grants.rs` (its `rule_3` unknown-tool sentence stays the four +container tools). The signatures below are complete. + +```sh +cp docs/plans/M6b/files/crates/brokerd/tests/grants_consult.rs crates/brokerd/tests/ +``` + +## Goal + +`consult` is a `ToolName`. `parse` reads `{"question":…}` with `deny_unknown_fields` and +caps `question` at 16_384 bytes on a character boundary. A grant with empty constraints +loads (`untrusted` still defaults true). Paths or hosts on a consult grant are invalid. +`covers` treats consult like the memory tools (`Some(None)`). `ALL` stays the four +container tools. `is_memory` is false. Unknown tool names still fail with the four-tool +sentence. + +## Every place that changes + +Adding `ToolName::Consult` makes every exhaustive `match` on `ToolName` or `ToolArgs` +fail to compile until it has an arm. List each match and fill it. + +| Place | What happens there | +|---|---| +| `crates/brokerd/src/consult.rs` | `ConsultArgs`, `QUESTION_CAP`, `cap_bytes`, `question_of`, `wire_body` | +| `crates/brokerd/src/args.rs` | `Consult` on `ToolName` and `ToolArgs`; `parse` | +| `crates/brokerd/src/config.rs` | `time_limit` arm (uses `read_file_ms`; consult never hits a container) | +| `crates/brokerd/src/policy/matching.rs` | `covers`: `Some(None)` | +| `crates/brokerd/src/grants.rs` | `check_tool_constraints`: no paths, no hosts | +| `crates/brokerd/src/runner.rs` | mounts none, egress none | +| `crates/brokerd/src/container.rs` | refuse `Consult` with `IN_CONSULT` | +| `crates/brokerd/tests/grants_consult.rs` | new, given | +| everything else | **no change**: not `ledger`, not `broker.rs`, not `tests/grants.rs` | + +`memory_port::wait_for` already has `_ =>` and needs no edit. + +## Files + +- Copy, from `docs/plans/M6b/files/` (the same path under it): the new test + `crates/brokerd/tests/grants_consult.rs`. +- Modify: `crates/brokerd/src/consult.rs`, `crates/brokerd/src/args.rs`, + `crates/brokerd/src/config.rs`, `crates/brokerd/src/policy/matching.rs`, + `crates/brokerd/src/grants.rs`, `crates/brokerd/src/runner.rs`, + `crates/brokerd/src/container.rs`, `docs/implementer-log.md`. +- Do not touch: any other file. Do not edit `crates/brokerd/tests/grants.rs`. + +## Facts you need + +- **Unknown keys.** `ConsultArgs` has `#[serde(deny_unknown_fields)]`. `parse` maps a + serde error to `ArgsError::Shape`. The given test accepts a detail that contains + `unknown` or `history`. +- **Cap on parse.** `cap_bytes(&value.question, QUESTION_CAP).0` is the stored question. + `floor_char_boundary` then `.get(..boundary)`. +- **`wire_body`** is used by task 05's tests; write it now so those tests can call it. + No history, no `tools` key. When `consult` is `None`, model is `""` and temperature is + 0.6. +- **`ALL` is `[ToolName; 4]`.** Do not put `Consult` in it. +- **Rule 3.** `ToolName::parse("consult")` is `Some`, so a consult grant is not "unknown + tool". The unknown-tool sentence in `grants.rs` still lists the four container tools. + Do not edit that string. +- The items the edits use. This table is complete. + + | Item | Signature | + |---|---| + | `ToolName::parse` | `parse(name: &str) -> Option` | + | `ToolName::as_str` | `as_str(self) -> &'static str` | + | `ToolName::is_memory` | `is_memory(self) -> bool` | + | `ToolName::ALL` | `[ToolName; 4]` | + | `parse` | `parse(tool: ToolName, arguments: &str) -> Result` | + | `ArgsError::Shape` | `Shape(String)` | + | `ArgsError::detail` | `detail(&self) -> String` | + | `Grant.untrusted` | defaults true | + | `str::floor_char_boundary` | `floor_char_boundary(&self, index: usize) -> usize` (Rust 1.98) | + +## The edits, exactly + +Each quoted "before" block occurs exactly once after task 03. + +### `crates/brokerd/src/consult.rs` + +**A1.** Add `Serialize` to the serde import: + +```rust +use serde::{Deserialize, Deserializer}; +``` + +becomes + +```rust +use serde::{Deserialize, Deserializer, Serialize}; +``` + +**A2.** After `pub const CONSULT: &str = "consult";` add: + +```rust +/// `question` and the answer are cut to this many bytes, on a character boundary. +pub const QUESTION_CAP: usize = 16_384; +``` + +**A3.** After `parse_loopback_http` (after its closing `}`, before `SecretStartError`) add: + +```rust +/// The model's `consult` arguments. Unknown keys are errors. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ConsultArgs { + pub question: String, +} + +/// Cut `text` to `cap` bytes on a character boundary. +pub fn cap_bytes(text: &str, cap: usize) -> (String, bool) { + if text.len() <= cap { + return (text.to_string(), false); + } + let boundary = text.floor_char_boundary(cap); + match text.get(..boundary) { + Some(cut) => (cut.to_string(), true), + None => (String::new(), true), + } +} + +/// The `question` field of `arguments`, capped, or empty when it is missing or not a string. +pub fn question_of(arguments: &str) -> String { + let extracted = match serde_json::from_str::(arguments) { + Ok(serde_json::Value::Object(obj)) => obj + .get("question") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(), + _ => String::new(), + }; + cap_bytes(&extracted, QUESTION_CAP).0 +} + +/// The exact JSON body `brokerd` POSTs (and records). No history, no tools. +pub fn wire_body(consult: Option<&Consult>, arguments: &str) -> String { + let model = consult.map(|c| c.model.as_str()).unwrap_or(""); + let temperature = consult + .map(Consult::temperature) + .unwrap_or(f64::from(DEFAULT_TEMPERATURE_MILLI) / 1000.0); + wire_body_parts(model, temperature, &question_of(arguments)) +} + +fn wire_body_parts(model: &str, temperature: f64, question: &str) -> String { + serde_json::json!({ + "model": model, + "temperature": temperature, + "messages": [{"role": "user", "content": question}], + }) + .to_string() +} +``` + +### `crates/brokerd/src/args.rs` + +**N1.** After the `use bxcore::{MemoryArgs, RecallArgs};` line add: + +```rust +use crate::consult::{ConsultArgs, QUESTION_CAP, cap_bytes}; +``` + +**N2.** The comment on `ToolName` + +```rust +/// The tools the broker can dispatch to: four that run in a container, and the three memory +/// tools that `memoryd` runs (M5a spec, section 8; M5c spec, section 9). +``` + +becomes + +```rust +/// The tools the broker can dispatch to: four that run in a container, the three memory +/// tools that `memoryd` runs, and `consult` (M6 spec, section 4). +``` + +**N3.** After `AskMemory,` in `ToolName` add `Consult,`. + +**N4.** In `ToolName::parse`, after `"ask_memory" => Some(ToolName::AskMemory),` add +`"consult" => Some(ToolName::Consult),`. + +**N5.** In `as_str`, after `ToolName::AskMemory => "ask_memory",` add +`ToolName::Consult => "consult",`. + +**N6.** After `AskMemory(AskMemoryArgs),` in `ToolArgs` add: + +```rust + Consult { + question: String, + }, +``` + +**N7.** In `tool()`, after `ToolArgs::AskMemory(_) => ToolName::AskMemory,` add +`ToolArgs::Consult { .. } => ToolName::Consult,`. + +**N8.** In `canonical_json()`, after the `AskMemory` arm add: + +```rust + ToolArgs::Consult { question } => { + let value = ConsultArgs { + question: question.clone(), + }; + serde_json::to_string(&value).unwrap_or_else(|_| "{}".to_string()) + } +``` + +**N9.** In `parse()`, after the `AskMemory` arm add: + +```rust + ToolName::Consult => { + let value: ConsultArgs = serde_json::from_str(arguments) + .map_err(|error| ArgsError::Shape(error.to_string()))?; + let question = cap_bytes(&value.question, QUESTION_CAP).0; + Ok(ToolArgs::Consult { question }) + } +``` + +Do not add `Consult` to `is_memory`. Do not put it in `ALL`. + +### `crates/brokerd/src/config.rs` + +**T1.** The `time_limit` arm + +```rust + crate::args::ToolName::Recall + | crate::args::ToolName::Memory + | crate::args::ToolName::AskMemory => self.read_file_ms, +``` + +becomes + +```rust + crate::args::ToolName::Recall + | crate::args::ToolName::Memory + | crate::args::ToolName::AskMemory + | crate::args::ToolName::Consult => self.read_file_ms, +``` + +### `crates/brokerd/src/policy/matching.rs` + +**M1.** The `covers` arm + +```rust + ToolArgs::Recall(_) | ToolArgs::Memory(_) | ToolArgs::AskMemory(_) => Some(None), +``` + +becomes + +```rust + ToolArgs::Recall(_) + | ToolArgs::Memory(_) + | ToolArgs::AskMemory(_) + | ToolArgs::Consult { .. } => Some(None), +``` + +### `crates/brokerd/src/grants.rs` + +**G1.** The end of `check_tool_constraints` (exactly once) + +```rust + "http_fetch does not take paths", + ); + } + } + _ => {} + } +} +``` + +becomes + +```rust + "http_fetch does not take paths", + ); + } + } + "consult" => { + if !grant.constraints.paths.is_empty() { + push( + problems, + file.to_string(), + None, + "consult does not take paths".to_string(), + ); + } + if !grant.constraints.hosts.is_empty() { + push( + problems, + file.to_string(), + None, + "consult does not take hosts".to_string(), + ); + } + } + _ => {} + } +} +``` + +Do not edit the unknown-tool sentence. + +### `crates/brokerd/src/runner.rs` + +**U1.** The mounts arm + +```rust + ToolArgs::Recall(_) | ToolArgs::Memory(_) | ToolArgs::AskMemory(_) => (Vec::new(), None), +``` + +becomes + +```rust + ToolArgs::Recall(_) + | ToolArgs::Memory(_) + | ToolArgs::AskMemory(_) + | ToolArgs::Consult { .. } => (Vec::new(), None), +``` + +### `crates/brokerd/src/container.rs` + +**P1.** After `pub const IN_MEMORYD: &str = "the memory tools run in memoryd, not in a container";` add: + +```rust +/// A `consult` that reached the container runtime: fixed, it reaches the model. +pub const IN_CONSULT: &str = "consult runs in brokerd, not in a container"; +``` + +**P2.** The comment and body at the start of `Podman::run` + +```rust + // `Routed` sends the memory tools to `memoryd`; one here is refused, never run. + if spec.tool().is_memory() { + return Err(RunError::Unavailable(IN_MEMORYD.to_string())); + } +``` + +become + +```rust + // `Routed` sends the memory tools to `memoryd`; `ConsultStack` runs `consult`. + if spec.tool().is_memory() { + return Err(RunError::Unavailable(IN_MEMORYD.to_string())); + } + if spec.tool() == crate::args::ToolName::Consult { + return Err(RunError::Unavailable(IN_CONSULT.to_string())); + } +``` + +## Steps + +- [ ] **1. Copy the given test.** + + ```sh + cp docs/plans/M6b/files/crates/brokerd/tests/grants_consult.rs crates/brokerd/tests/ + ``` + +- [ ] **2. See it fail.** `cargo test -p brokerd --test grants_consult`, timeout 600000 + ms: the target does not compile (`E0599` / unresolved `ToolName::Consult` or + `ConsultArgs`). +- [ ] **3. The edits.** A1–A3, N1–N9, T1, M1, G1, U1, P1–P2. After each file, + `cargo check -p brokerd`. When it prints no error and no warning, go on. `wc -l` of + `consult.rs`, `args.rs`, `grants.rs`, `container.rs` each ≤ 500. +- [ ] **4. The tests.** `cargo test -p brokerd --test grants_consult --test grants --test + args`: every test passes, 4 in `grants_consult`. The existing unknown-tool case in + `grants.rs` still names the four container tools. +- [ ] **5. Run the gate.** `cargo fmt --all`, then `make gate`, timeout 600000 ms. Last + line `gate: ok`. Test count **1606** (1602 before, plus 4). After `gate: ok`, do not add + up `test result:` lines. +- [ ] **6. Log and commit.** Task cell `M6b/04-brokerd-consult-tool`. Read only the first + 12 lines of `docs/implementer-log.md`; insert the row under the header. Stage with + `git add crates/brokerd docs/implementer-log.md Cargo.lock`, then commit as `AGENTS.md` + says. + +## Done when + +- `make gate` prints `gate: ok` with 1606 tests. +- `git diff HEAD~1 --stat` lists only the files in the Files section plus the log. + +## Stop and report if + +- The branch check fails; a "before" block is missing or duplicated; step 2 compiles; you + edit `tests/grants.rs`; you add `Consult` to `ALL` or `is_memory`; `cargo check` warns; + an existing `grants` or `args` test fails; a test outside the named files fails in the + gate (`git stash -u`, rerun, `git stash pop`, report both). + +## What the given tests pin + +`ToolName::parse("consult")` is `Consult`; `as_str` is `"consult"`; it is not memory and +not in `ALL`. `parse` of `{"question":"hello"}` yields that question. A `history` key is +invalid-arguments. An empty-constraints consult grant loads with `untrusted` true. Paths +or hosts on the grant are rejected with `does not take paths` / `does not take hosts`. diff --git a/docs/plans/M6b/05-brokerd-consult-audit-body.md b/docs/plans/M6b/05-brokerd-consult-audit-body.md new file mode 100644 index 0000000..0bb0216 --- /dev/null +++ b/docs/plans/M6b/05-brokerd-consult-audit-body.md @@ -0,0 +1,408 @@ +# M6b task 05: Decision `body`; denials still record it + +**Branch:** `m6b`. Check it: `git branch --show-current` prints `m6b` and `git status --short` +is empty; otherwise stop. Never switch branch. +**Commit subject:** `brokerd: consult decisions record the body` +**Log row:** the Task cell is `M6b/05-brokerd-consult-audit-body`. + +**First action.** Run this before any Read of crate source. Do not `ls` +`crates/brokerd/tests/`. Do not Read `main.rs`, `consult_http`, `docs/implementer-log.md` +(except the first 12 lines at the log step). Do not edit `docs/runbook.md` (task 03 +already appended `## consult-not-configured`). The signatures below are complete. + +```sh +cp docs/plans/M6b/files/crates/brokerd/tests/consult_deny.rs crates/brokerd/tests/ +``` + +## Goal + +Every `consult` decision records the JSON body that would be POSTed, including a denial +(spec 4.4). `AuditEvent::Decision` gains optional `body` with +`#[serde(default, skip_serializing_if = "Option::is_none")]`. Other tools leave it unset. +No `[consult]` is `decide_forced` with `InvalidArguments` and `NOT_CONFIGURED` (the +runbook pointer). A grant that does not cover the session's taint is `TaintTooHigh` and +still records the body. Unknown keys are invalid-arguments. Nothing is POSTed on a +denial (`Recording` stays at count 0). + +## Every place that changes + +Adding `body` to `AuditEvent::Decision` makes every struct literal of that variant fail +to compile until it names the field. List each constructor. Matches that use `..` need +no edit. + +| Place | What happens there | +|---|---| +| `crates/loopd` / other crates | **no change** except the constructors below | +| `crates/bxcore/src/audit.rs` | `body: Option` on `Decision` only | +| `crates/bxcore/tests/records.rs` | three `Decision { }` literals: `body: None` | +| `crates/brokerd/tests/ledger.rs` | one `Decision { }` literal: `body: None` | +| `crates/brokerd/tests/support/audit_dir.rs` | `denied()`: `body: None` | +| `crates/brokerd/src/ledger/mod.rs` | `decide_with_body`, `decide_forced`, `decide_inner`; event includes `body` | +| `crates/brokerd/src/consult.rs` | `NOT_CONFIGURED` | +| `crates/brokerd/src/broker.rs` | consult uses `wire_body` then `decide_forced` / `decide_with_body` | +| `crates/brokerd/tests/consult_deny.rs` | new, given | +| everything else | **no change**: not `main.rs`, not HTTP, not `docs/runbook.md` | + +`loopd/tests/end_to_end.rs` matches `Decision` with `..` and needs no edit. Do not add +`body` to `AuditEvent::Approval`. + +## Files + +- Copy, from `docs/plans/M6b/files/` (the same path under it): the new test + `crates/brokerd/tests/consult_deny.rs`. +- Modify: `crates/bxcore/src/audit.rs`, `crates/bxcore/tests/records.rs`, + `crates/brokerd/src/ledger/mod.rs`, `crates/brokerd/src/consult.rs`, + `crates/brokerd/src/broker.rs`, `crates/brokerd/tests/ledger.rs`, + `crates/brokerd/tests/support/audit_dir.rs`, `docs/implementer-log.md`. +- Do not touch: any other file. Do not edit `docs/runbook.md`. + +## Facts you need + +- **Serde.** `body` is last on `Decision`. `#[serde(default, skip_serializing_if = "Option::is_none")]`. + Existing JSONL fixtures without `body` still decode. A `None` still equals a decoded + record that omitted the field. +- **`decide` stays.** Existing callers keep `decide(...)`. Consult goes through + `decide_with_body` or `decide_forced`. `decide` is `decide_inner(..., None, None)`. +- **`decide_forced` does not match grants.** The outcome is the given `Denial`. The + noconfig test's rig has no consult grant; matching would be `NoGrant`, not + `InvalidArguments`. +- **`NOT_CONFIGURED`** is exactly + `consult is not configured; see docs/runbook.md#consult-not-configured` (no trailing + period). `scripts/check-runbook.sh` reads this pointer; the heading already exists. +- **`Denial`** is `crate::policy::Denial { reason, grant, grant_sha256, detail }`. +- **Wire body on noconfig.** `wire_body(None, arguments)`: empty model, temperature 0.6, + one user message. +- The items the edits use. This table is complete. + + | Item | Signature | + |---|---| + | `consult::wire_body` | `wire_body(consult: Option<&Consult>, arguments: &str) -> String` | + | `consult::CONSULT` | `&str`, `"consult"` | + | `Ledger::decide` | `decide(&self, request: ToolRequest, grants: &Grants, now: Timestamp) -> Decided` | + | `policy::Denial` | `{ reason: DenyReason, grant: Option, grant_sha256: Option, detail: Option }` | + | `DenyReason::InvalidArguments` | in `bxcore` | + | `DenyReason::TaintTooHigh` | in `bxcore` | + | `Broker::cfg` | `cfg(&self) -> &Config` | + | `Broker::ledger` | `ledger(&self) -> &Ledger` | + | `Broker::grants` | as used in `handle` already | + +## The edits, exactly + +Each quoted "before" block occurs exactly once after task 04. + +### `crates/bxcore/src/audit.rs` + +**E1.** The `Decision` fields ending + +```rust + taint: DataClass, + untrusted: bool, + }, + Approval { +``` + +become + +```rust + taint: DataClass, + untrusted: bool, + /// The JSON body `consult` would send (or sent). Other tools leave this unset. + #[serde(default, skip_serializing_if = "Option::is_none")] + body: Option, + }, + Approval { +``` + +Do not add `body` to `Approval`. + +### `crates/bxcore/tests/records.rs` + +**E2, E3, E4.** Each of the three `AuditEvent::Decision {` literals gains `body: None,` +after `untrusted: …,`. They are: + +1. `tool: "read_file"` / `untrusted: false,` then `},` of that event +2. `tool: "shell"` / `untrusted: true,` then `},` of that event +3. `tool: "rm_rf"` / `untrusted: false,` then `},` of that event + +Each `untrusted: false,` / `untrusted: true,` that is the last field of a `Decision` +becomes that line plus `body: None,`. Do not add it to `Approval` or `Result`. + +### `crates/brokerd/tests/ledger.rs` + +**E5.** In `an_allowed_call_is_recorded_in_full`, after `untrusted: false,` of the +`AuditEvent::Decision` add `body: None,`. + +### `crates/brokerd/tests/support/audit_dir.rs` + +**E6.** In `denied()`, after `untrusted: false,` add `body: None,`. + +### `crates/brokerd/src/consult.rs` + +**N1.** Immediately before `pub const SECRET_POINTER: &str = "see docs/runbook.md#consult-secret-unreadable";` insert: + +```rust +/// Denied when `brokerd.toml` has no `[consult]` table. +pub const NOT_CONFIGURED: &str = + "consult is not configured; see docs/runbook.md#consult-not-configured"; +``` + +### `crates/brokerd/src/ledger/mod.rs` + +**D1.** Replace the start of `decide` + +```rust + /// Decide and record a call. + pub fn decide(&self, request: ToolRequest, grants: &Grants, now: Timestamp) -> Decided { + let session = request.session.clone(); +``` + +with + +```rust + /// Decide and record a call. + pub fn decide(&self, request: ToolRequest, grants: &Grants, now: Timestamp) -> Decided { + self.decide_inner(request, grants, now, None, None) + } + + /// Decide and record a call, storing `body` on the decision (consult). + pub fn decide_with_body( + &self, + request: ToolRequest, + grants: &Grants, + now: Timestamp, + body: Option, + ) -> Decided { + self.decide_inner(request, grants, now, body, None) + } + + /// Record `forced` as the denial, with `body`, without matching grants. + pub fn decide_forced( + &self, + request: ToolRequest, + grants: &Grants, + now: Timestamp, + body: Option, + forced: crate::policy::Denial, + ) -> Decided { + self.decide_inner(request, grants, now, body, Some(forced)) + } + + fn decide_inner( + &self, + request: ToolRequest, + grants: &Grants, + now: Timestamp, + body: Option, + forced: Option, + ) -> Decided { + let session = request.session.clone(); +``` + +**D2.** The outcome assignment. The lines + +```rust + let (outcome, grant, grant_sha256, payload) = match grants { + Err(_) => ( + DecisionRecord::Denied { + reason: DenyReason::GrantsInvalid, + }, + None, + None, + RecordPayload::Simple(DenyReason::GrantsInvalid), + ), + Ok(set) => match state { + Err(()) => ( + DecisionRecord::Denied { + reason: DenyReason::StateUnreadable, + }, + None, + None, + RecordPayload::Simple(DenyReason::StateUnreadable), + ), + Ok(state) => match crate::policy::decide(request, set, state, now) { + crate::policy::Outcome::Allowed(decision) => ( + DecisionRecord::Allowed {}, + Some(decision.grant().to_string()), + Some(decision.grant_sha256()), + RecordPayload::Allowed(decision), + ), + crate::policy::Outcome::Ask(ask) => ( + DecisionRecord::Ask {}, + Some(ask.grant().to_string()), + Some(ask.grant_sha256()), + RecordPayload::Ask(ask), + ), + crate::policy::Outcome::Denied(denial) => ( + DecisionRecord::Denied { + reason: denial.reason, + }, + denial.grant.clone(), + denial.grant_sha256, + RecordPayload::Denial(denial), + ), + }, + }, + }; +``` + +become + +```rust + let (outcome, grant, grant_sha256, payload) = if let Some(denial) = forced { + ( + DecisionRecord::Denied { + reason: denial.reason, + }, + denial.grant.clone(), + denial.grant_sha256, + RecordPayload::Denial(denial), + ) + } else { + match grants { + Err(_) => ( + DecisionRecord::Denied { + reason: DenyReason::GrantsInvalid, + }, + None, + None, + RecordPayload::Simple(DenyReason::GrantsInvalid), + ), + Ok(set) => match state { + Err(()) => ( + DecisionRecord::Denied { + reason: DenyReason::StateUnreadable, + }, + None, + None, + RecordPayload::Simple(DenyReason::StateUnreadable), + ), + Ok(state) => match crate::policy::decide(request, set, state, now) { + crate::policy::Outcome::Allowed(decision) => ( + DecisionRecord::Allowed {}, + Some(decision.grant().to_string()), + Some(decision.grant_sha256()), + RecordPayload::Allowed(decision), + ), + crate::policy::Outcome::Ask(ask) => ( + DecisionRecord::Ask {}, + Some(ask.grant().to_string()), + Some(ask.grant_sha256()), + RecordPayload::Ask(ask), + ), + crate::policy::Outcome::Denied(denial) => ( + DecisionRecord::Denied { + reason: denial.reason, + }, + denial.grant.clone(), + denial.grant_sha256, + RecordPayload::Denial(denial), + ), + }, + }, + } + }; +``` + +**D3.** The event fields ending + +```rust + taint: recorded.taint, + untrusted: recorded.untrusted, + }; +``` + +become + +```rust + taint: recorded.taint, + untrusted: recorded.untrusted, + body, + }; +``` + +### `crates/brokerd/src/broker.rs` + +**H1.** The lines + +```rust + let now = Timestamp::now(); + let grants = broker.grants(); + let answer = match broker.ledger().decide(request, &grants, now) { +``` + +become + +```rust + let now = Timestamp::now(); + let grants = broker.grants(); + let decided = if request.tool == crate::consult::CONSULT { + let body = crate::consult::wire_body(broker.cfg().consult.as_ref(), &request.arguments); + if broker.cfg().consult.is_none() { + broker.ledger().decide_forced( + request, + &grants, + now, + Some(body), + crate::policy::Denial { + reason: bxcore::DenyReason::InvalidArguments, + grant: None, + grant_sha256: None, + detail: Some(crate::consult::NOT_CONFIGURED.to_string()), + }, + ) + } else { + broker + .ledger() + .decide_with_body(request, &grants, now, Some(body)) + } + } else { + broker.ledger().decide(request, &grants, now) + }; + let answer = match decided { +``` + +## Steps + +- [ ] **1. Copy the given test.** + + ```sh + cp docs/plans/M6b/files/crates/brokerd/tests/consult_deny.rs crates/brokerd/tests/ + ``` + +- [ ] **2. See it fail.** `cargo test -p brokerd --test consult_deny`, timeout 600000 ms: + the target does not compile (`E0560` / no field `body` on `AuditEvent::Decision`, or + unresolved `NOT_CONFIGURED`). +- [ ] **3. The edits.** E1–E6, N1, D1–D3, H1. `cargo check -p bxcore -p brokerd` prints no + error and no warning. `wc -l crates/brokerd/src/ledger/mod.rs` and + `wc -l crates/brokerd/src/broker.rs` each ≤ 500. +- [ ] **4. The tests.** `cargo test -p brokerd --test consult_deny --test ledger --test + audit` and `cargo test -p bxcore --test records`: every test passes, 3 in + `consult_deny`. Existing Decision equality tests still pass. +- [ ] **5. Run the gate.** `cargo fmt --all`, then `make gate`, timeout 600000 ms. Last + line `gate: ok`. Test count **1609** (1606 before, plus 3). After `gate: ok`, do not add + up `test result:` lines. +- [ ] **6. Log and commit.** Task cell `M6b/05-brokerd-consult-audit-body`. Read only the + first 12 lines of `docs/implementer-log.md`; insert the row under the header. Stage with + `git add crates/bxcore crates/brokerd docs/implementer-log.md Cargo.lock`, then commit + as `AGENTS.md` says. + +## Done when + +- `make gate` prints `gate: ok` with 1609 tests. +- `git diff HEAD~1 --stat` lists only the files in the Files section plus the log. + +## Stop and report if + +- The branch check fails; a "before" block is missing or duplicated (in particular E1 + matching `Approval` instead of `Decision`); step 2 compiles; you edit `docs/runbook.md` + or `main.rs`; you add `body` to `Approval`; `cargo check` warns; an existing ledger or + records test fails; a test outside the named files fails in the gate (`git stash -u`, + rerun, `git stash pop`, report both). + +## What the given tests pin + +No `[consult]` is `InvalidArguments` naming `consult-not-configured` and `docs/runbook.md`; +the audit `body` equals `wire_body(None, question)` and has no `tools`. A secret session +against a private consult grant is `TaintTooHigh` with no detail; the body has +`"model":"test-model"` and one message. An unknown `history` key is `InvalidArguments`. +Nothing reaches the runtime (`count() == 0`). diff --git a/docs/plans/M6b/06-brokerd-consult-http.md b/docs/plans/M6b/06-brokerd-consult-http.md new file mode 100644 index 0000000..9aeb2f7 --- /dev/null +++ b/docs/plans/M6b/06-brokerd-consult-http.md @@ -0,0 +1,425 @@ +# M6b task 06: allowlisted POST; audit body equals the bytes + +**Branch:** `m6b`. Check it: `git branch --show-current` prints `m6b` and `git status --short` +is empty; otherwise stop. Never switch branch. +**Commit subject:** `brokerd: consult posts the allowlisted body` +**Log row:** the Task cell is `M6b/06-brokerd-consult-http`. + +**First action.** Run this before any Read of crate source. Do not `ls` +`crates/brokerd/tests/`. Do not Read `podman.rs`, `docs/implementer-log.md` (except the +first 12 lines at the log step). The given test injects an in-process fake; do not open a +socket in the tests and do not reach the network (T27). + +```sh +cp docs/plans/M6b/files/crates/brokerd/tests/consult_allow.rs crates/brokerd/tests/ +``` + +## Goal + +An allowed `consult` POSTs the allowlisted JSON (`wire_body`) through `ConsultTransport`. +Tests inject a fake; production uses loopback HTTP/1.1 (`HttpTransport`). The audit +`body` equals the posted bytes. `messages` has one user item and no `tools`. The tool +result is `choices[0].message.content`, tag-neutralised, then capped. `ConsultStack` +sends `Consult` to that runtime and everything else to `inner`, so existing `Routed` +tests stay unedited. `main` loads the secret after `Config::load` and wraps `Routed` +when `[consult]` and the secret are both present. + +## Every place that changes + +| Place | What happens there | +|---|---| +| `crates/brokerd/src/consult.rs` | `ConsultTransport`, `ConsultRuntime`, `ConsultStack`, `parse_message_content` | +| `crates/brokerd/src/consult_http.rs` | new: `HttpTransport` | +| `crates/brokerd/src/lib.rs` | `pub mod consult_http;` | +| `crates/brokerd/src/main.rs` | `load_secret`; wrap `Routed` in `ConsultStack` | +| `crates/brokerd/tests/consult_allow.rs` | new, given | +| everything else | **no change**: not `ledger`, not `handle`'s decide path | + +## Files + +- Copy, from `docs/plans/M6b/files/` (the same path under it): the new test + `crates/brokerd/tests/consult_allow.rs`. +- Create: `crates/brokerd/src/consult_http.rs` (the module below). +- Modify: `crates/brokerd/src/consult.rs`, `crates/brokerd/src/lib.rs`, + `crates/brokerd/src/main.rs`, `docs/implementer-log.md`. +- Do not touch: any other file. + +The given test holds `mod runtime` even though it does not name `Recording`: +`support/client.rs` imports `crate::runtime`. Leave that module in the test. + +## Facts you need + +- **`Runtime::run_labelled` default calls `run`.** Implement both: `run` forwards to + `run_labelled` and drops the label. Implementing only `run_labelled` recurses. +- **No `choices[0]` index.** `as_array().and_then(|a| a.first())`. A missing content is + `"the response has no content"`. +- **Neutralise then cap.** `bxcore::sanitize::neutralise` then `cap_bytes(..., QUESTION_CAP)`. + The given test builds the think tag with `concat!("<", "think", ">")` and asserts + `<` in the result. Do not write that tag whole anywhere (T33). +- **Label.** `run_labelled` returns `(RunOutput { content, truncated }, None)`. Result + class comes from the grant, as today. +- **HTTP/1.1.** `TcpStream` to the loopback host:port from `parse_loopback_http`. Headers: + `POST {path} HTTP/1.1`, `Host`, `Authorization: Bearer {bearer}`, + `Content-Type: application/json`, `Content-Length`, `Connection: close`. Response cap + 262_144 bytes. Status must contain ` 200 ` or end with ` 200`. HTTPS is later. +- **`load_secret` in `main`** after `Config::load`, before the instance lock. Print + `{e}` and exit 1 on `Err`. Print a `warning` if `Loaded` has one. Role is already + `"brokerd"` inside `load_secret`. +- **Wrap after `Routed`.** `ConsultStack { consult: ConsultRuntime::new(consult, + loaded.secret, HttpTransport), inner: runtime }` when both `cfg.consult` and the + loaded secret are `Some`. Otherwise keep `runtime`. +- The items the edits use. This table is complete. + + | Item | Signature | + |---|---| + | `Runtime::run` | `run(&self, spec: &RunSpec) -> Result` | + | `Runtime::run_labelled` | `run_labelled(&self, spec: &RunSpec) -> Result<(RunOutput, Option