M5c-2 plan: task 02, brokerd knows ask_memory; the outline split to nine tasks

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

docs/plans/M5c2/01-wire-ask-memory.md

@@ -11,8 +11,8 @@ M5c-2 adds a core tool, `ask_memory`, that asks Honcho, a memory server shared b
 agents, a question about one or two of its workspaces (M5c spec, section 9). This task adds only
 the tool's arguments to `bxcore`, with their shape checks, the way `recall`'s are there
 (`RecallArgs`, its name constant `RECALL`, its limit `MAX_QUERY`, and `RecallArgs::check`). Later
-tasks use them: task 02 (`brokerd` parses the arguments with them), task 03 (`loopd` declares the
+tasks use them: task 02 (`brokerd` parses the arguments with them), task 04 (`loopd` declares the
-tool), task 07 (`memoryd` answers it).
+tool), tasks 08 and 09 (`memoryd` answers it).
 
 ```json
 {"question":"Who wrote the launch note?","workspaces":["fleet","boxmaker1"]}
@@ -24,7 +24,7 @@ tool), task 07 (`memoryd` answers it).
 - **`workspaces`**: optional; at most two names, none empty, none repeated, each 1 to 512
   characters of ASCII letters, digits, `_` and `-`. Missing, or `[]`, means the one workspace
   `own`, the agent's own trusted one. Whether a name is one this agent's `[honcho]` configuration
-  has is **not** checked here: `memoryd` checks that in task 07.
+  has is **not** checked here: `memoryd` checks that in task 08.
 - There is no other field, and an unknown field is refused, as in every argument type we define.
   (An earlier draft of the spec had a `reasoning` field; it was removed, and a given test sends
   it and expects a refusal.)

docs/plans/M5c2/02-brokerd-ask-memory.md

@@ -0,0 +1,462 @@
+# M5c-2 task 02: `brokerd` knows `ask_memory`, a built-in memory tool
+
+**Branch:** `m5c2`. Check it: `git branch --show-current` prints `m5c2` and `git status --short`
+is empty; otherwise stop. Never switch branch.
+**Commit subject:** `brokerd: ask_memory, a built-in memory tool that memoryd runs`
+**Log row:** the Task cell is `M5c2/02-brokerd-ask-memory`.
+
+## Goal
+
+Task 01 gave `bxcore` the arguments of a new tool, `ask_memory` (`bxcore::AskMemoryArgs`, with
+`check()`). This task makes `brokerd` accept it exactly as it accepts `recall` (M5c spec,
+section 9):
+
+- **Its name.** `ToolName::AskMemory`, written `ask_memory`. It is a memory tool:
+  `is_memory()` is true, so `memoryd` runs it, never a container.
+- **Its arguments.** `parse` decodes them into `ToolArgs::AskMemory(AskMemoryArgs)` and runs
+  `check()`; a refusal is `ArgsError::Shape` with `check`'s sentence, as for `recall`. The
+  canonical form is `serde_json::to_string` of the value, as for `recall`.
+- **Its policy is built in.** A third built-in grant, `builtin-ask-memory`, allows it in every
+  session, `secret` and untrusted ones too: `mode = "auto"`, `max_taint = "secret"`,
+  `result_class = "private"`, `untrusted = false`. A question is not written to Honcho (spec,
+  section 8), and the answer's own labels, which `memoryd` gives, raise the session, as `recall`'s
+  do. An owner's grant file naming `ask_memory` is refused with the `BUILT_IN` sentence, which now
+  names the three tools.
+
+`brokerd`'s wait for `memoryd` stays 30 s for every memory tool in this task; task 03 gives
+`ask_memory` its own. Nothing yet asks `brokerd` for `ask_memory`: `loopd` declares it in task 04
+and `memoryd` answers it from task 09. Until then, a call reaching `memoryd` is answered
+`memoryd runs only recall and memory`, a failed result.
+
+## Every place that changes
+
+This is the complete list.
+
+| Place | What happens there |
+|---|---|
+| `crates/brokerd/src/args.rs` | `ToolName::AskMemory`, `ToolArgs::AskMemory`, and their arms in `parse`, `as_str`, `is_memory`, `tool`, `canonical_json` and `parse` (edits A1 to A9) |
+| `crates/brokerd/src/builtin.rs` | the grant `ASK_MEMORY`, its id in `sources`, `BUILT_IN`'s sentence, two comments (edits B1, B2) |
+| `crates/brokerd/src/config.rs`, `Runner::time_limit` | `AskMemory` beside `Recall` and `Memory` (edit C1) |
+| `crates/brokerd/src/policy/matching.rs` | `ToolArgs::AskMemory` takes no constraints, as `Recall` (edit P1) |
+| `crates/brokerd/src/runner.rs` | `ToolArgs::AskMemory` has no mounts and no egress, as `Recall` (edit R1) |
+| `crates/brokerd/src/grants.rs` (rule 3), `container.rs` (`Podman::run`), `memory_port.rs` (`Routed`) | **no change**: each asks `tool.is_memory()`, which edit A5 makes true for `AskMemory` |
+| `crates/brokerd/tests/ask_memory_tool.rs` | new, given |
+| `crates/brokerd/tests/grants.rs`, `memory_tools.rs` | new versions, given: three built-in grants, not two |
+| everything else | **no change**: not `bxcore`, not `loopd`, not `memoryd`, no `Cargo.toml` |
+
+## Files
+
+- Copy, from `docs/plans/M5c2/files/` (the same path under it): the new test
+  `crates/brokerd/tests/ask_memory_tool.rs`, and **new versions** of (they replace the files in
+  the tree) `crates/brokerd/tests/grants.rs` and `crates/brokerd/tests/memory_tools.rs`.
+- Modify: `crates/brokerd/src/args.rs`, `crates/brokerd/src/builtin.rs`,
+  `crates/brokerd/src/config.rs`, `crates/brokerd/src/policy/matching.rs`,
+  `crates/brokerd/src/runner.rs`, `docs/implementer-log.md`.
+- Do not touch: any other file. The test support files under `crates/brokerd/tests/support/` are
+  used as they are.
+
+## Facts you need
+
+- **`AskMemoryArgs`** (task 01, in `bxcore::memory`, exported as `bxcore::AskMemoryArgs`):
+
+  ```rust
+  #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+  #[serde(deny_unknown_fields)]
+  pub struct AskMemoryArgs {
+      pub question: String,
+      #[serde(default)]
+      pub workspaces: Vec<String>,
+  }
+  impl AskMemoryArgs {
+      pub fn check(&self) -> Result<(), String>;
+      pub fn asked(&self) -> Vec<&str>;
+  }
+  ```
+
+  It is used here exactly as `RecallArgs` is: `serde_json::from_str`, then `check()`, each error
+  as `ArgsError::Shape`; `serde_json::to_string` for the canonical form. Its canonical form
+  writes every field: `{"question":"q","workspaces":[]}`.
+- **The order of the built-in grants** is the order of `sources` in `builtin.rs`: `memory`,
+  `recall`, then the new `ask-memory` last. The given `grants.rs` lists them in that order.
+- **Why the grant is `secret` and trusted.** `max_taint = "secret"` lets a `secret` session ask
+  (spec, section 8: a query is not stored by Honcho). `result_class = "private"` and `untrusted =
+  false` are the floor of the answer's label; `memoryd` labels each answer by the workspaces
+  asked, and `brokerd` keeps the stricter of the two (`runner::run`, unchanged). The given tests
+  send an `ask_memory` call through `broker::handle` to a fake `memoryd` that answers `untrusted`
+  or `secret`, and read the session's state afterwards.
+- **The long lines.** The three grant constants are string literals longer than 100 characters;
+  `rustfmt` leaves them as they are, and the gate accepts them, as it does `RECALL` and `MEMORY`
+  today. Do not split them.
+- The given tests:
+  - `ask_memory_tool.rs` (9 tests): the name, `is_memory`, parsing and the canonical form; each
+    refusal with `bxcore`'s sentence and its `detail()`; the built-in grant, its exact text and
+    its hash, allowing the call in a `private`, a `secret` and an untrusted session; an owner's
+    grant file refused with the new `BUILT_IN`; the call routed to `memoryd` with the session's
+    state and canonical arguments; a container runtime refusing it; and, through
+    `broker::handle`, an answer labelled untrusted making the session untrusted, one labelled
+    `secret` raising it to `secret`, and one labelled `public` leaving it `private`.
+  - `grants.rs`: one line changes, the list of built-in ids. `memory_tools.rs`: one line
+    changes, the number of grants `matchable` gives beside one owner grant (4, not 3).
+- The items the edits use. This table is complete; nothing from a crate's source (the Cargo
+  registry, `~/.cargo/registry`) is needed or allowed.
+
+  | Item | Where |
+  |---|---|
+  | `bxcore::AskMemoryArgs`, `AskMemoryArgs::check(&self) -> Result<(), String>` | A1, A9 |
+  | `serde_json::from_str`, `serde_json::to_string` | A8, A9, as the `Recall` arms beside them |
+  | `ArgsError::Shape(String)` | A9 |
+
+## The edits, exactly
+
+Nothing else in these files changes. Each quoted "before" block occurs exactly once in its file
+when its turn comes (make them in order). Keep the variants in the order shown.
+
+### `crates/brokerd/src/args.rs`
+
+**A1.** Add one line. The line
+
+```rust
+use bxcore::tools::{HttpFetchArgs, ReadFileArgs, ShellArgs, WriteFileArgs};
+```
+
+becomes
+
+```rust
+use bxcore::AskMemoryArgs;
+use bxcore::tools::{HttpFetchArgs, ReadFileArgs, ShellArgs, WriteFileArgs};
+```
+
+**A2.** The lines
+
+```rust
+/// The tools the broker can dispatch to: four that run in a container, and the two memory
+/// tools that `memoryd` runs (M5a spec, section 8).
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ToolName {
+    ReadFile,
+    WriteFile,
+    Shell,
+    HttpFetch,
+    Recall,
+    Memory,
+}
+```
+
+become
+
+```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).
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum ToolName {
+    ReadFile,
+    WriteFile,
+    Shell,
+    HttpFetch,
+    Recall,
+    Memory,
+    AskMemory,
+}
+```
+
+**A3.** In `ToolName::parse`, the lines (its spaces are checked: the name is `ask_memory`, with an
+underscore)
+
+```rust
+            "memory" => Some(ToolName::Memory),
+            _ => None,
+```
+
+become
+
+```rust
+            "memory" => Some(ToolName::Memory),
+            "ask_memory" => Some(ToolName::AskMemory),
+            _ => None,
+```
+
+**A4.** In `ToolName::as_str`, the lines (its spaces are checked)
+
+```rust
+            ToolName::Memory => "memory",
+        }
+```
+
+become
+
+```rust
+            ToolName::Memory => "memory",
+            ToolName::AskMemory => "ask_memory",
+        }
+```
+
+**A5.** In `ToolName::is_memory`, the line
+
+```rust
+        matches!(self, ToolName::Recall | ToolName::Memory)
+```
+
+becomes
+
+```rust
+        matches!(
+            self,
+            ToolName::Recall | ToolName::Memory | ToolName::AskMemory
+        )
+```
+
+**A6.** At the end of `enum ToolArgs`, the lines
+
+```rust
+    Recall(RecallArgs),
+    Memory(MemoryArgs),
+}
+```
+
+become
+
+```rust
+    Recall(RecallArgs),
+    Memory(MemoryArgs),
+    AskMemory(AskMemoryArgs),
+}
+```
+
+**A7.** In `ToolArgs::tool`, the line
+
+```rust
+            ToolArgs::Memory(_) => ToolName::Memory,
+```
+
+becomes
+
+```rust
+            ToolArgs::Memory(_) => ToolName::Memory,
+            ToolArgs::AskMemory(_) => ToolName::AskMemory,
+```
+
+**A8.** At the end of `ToolArgs::canonical_json`, the lines
+
+```rust
+            ToolArgs::Memory(value) => {
+                serde_json::to_string(value).unwrap_or_else(|_| "{}".to_string())
+            }
+```
+
+become
+
+```rust
+            ToolArgs::Memory(value) => {
+                serde_json::to_string(value).unwrap_or_else(|_| "{}".to_string())
+            }
+            ToolArgs::AskMemory(value) => {
+                serde_json::to_string(value).unwrap_or_else(|_| "{}".to_string())
+            }
+```
+
+**A9.** At the end of `parse`, the lines
+
+```rust
+            Ok(ToolArgs::Memory(value))
+        }
+```
+
+become
+
+```rust
+            Ok(ToolArgs::Memory(value))
+        }
+        ToolName::AskMemory => {
+            let value: AskMemoryArgs = serde_json::from_str(arguments)
+                .map_err(|error| ArgsError::Shape(error.to_string()))?;
+            value.check().map_err(ArgsError::Shape)?;
+            Ok(ToolArgs::AskMemory(value))
+        }
+```
+
+### `crates/brokerd/src/builtin.rs`
+
+**B1.** The module's doc comment, the lines
+
+```rust
+//! The built-in grants of `recall` and `memory` (M5a spec, section 8). Reading and writing memory
+//! is what an agent is, so no grant file can turn it off or put questions in front of it; the
+//! prompt is protected by `memoryd`, which lets only clean sessions edit identity.
+```
+
+become
+
+```rust
+//! The built-in grants of `recall`, `memory` and `ask_memory` (M5a spec, section 8; M5c spec,
+//! section 9). Reading and writing memory is what an agent is, so no grant file can turn it off or
+//! put questions in front of it; the prompt is protected by `memoryd`, which lets only clean
+//! sessions edit identity.
+```
+
+**B2.** The lines (its spaces are checked: the grant's text is hashed into the audit log, and the
+given test compares it and the sentence letter for letter; every `\n` and `\"` is as shown)
+
+```rust
+pub const MEMORY: &str = "tool = \"memory\"\nmode = \"auto\"\nmax_taint = \"secret\"\nresult_class = \"private\"\nuntrusted = false\n";
+
+/// The sentence for an owner's grant naming a memory tool.
+pub const BUILT_IN: &str = "the policy of recall and memory is built in; remove this grant";
+
+/// The two built-in grants, `builtin-memory` and `builtin-recall`.
+pub fn grants() -> Result<Vec<LoadedGrant>, GrantProblem> {
+    // For each (id, text) of [("builtin-memory", MEMORY), ("builtin-recall", RECALL)]:
+    // `toml::from_str::<Grant>(text)` and `sha256(text.as_bytes())`, each error as a GrantProblem {
+    // file: id, line: None, problem: e.to_string() }; build LoadedGrant { id, grant, sha256 }.
+    // Collect into Result<Vec<LoadedGrant>, GrantProblem> (the first error wins).
+    let sources: &[(&str, &str)] = &[("builtin-memory", MEMORY), ("builtin-recall", RECALL)];
+```
+
+become
+
+```rust
+pub const MEMORY: &str = "tool = \"memory\"\nmode = \"auto\"\nmax_taint = \"secret\"\nresult_class = \"private\"\nuntrusted = false\n";
+pub const ASK_MEMORY: &str = "tool = \"ask_memory\"\nmode = \"auto\"\nmax_taint = \"secret\"\nresult_class = \"private\"\nuntrusted = false\n";
+
+/// The sentence for an owner's grant naming a memory tool.
+pub const BUILT_IN: &str =
+    "the policy of recall, memory and ask_memory is built in; remove this grant";
+
+/// The three built-in grants, `builtin-memory`, `builtin-recall` and `builtin-ask-memory`.
+pub fn grants() -> Result<Vec<LoadedGrant>, GrantProblem> {
+    // For each (id, text) of `sources`:
+    // `toml::from_str::<Grant>(text)` and `sha256(text.as_bytes())`, each error as a GrantProblem {
+    // file: id, line: None, problem: e.to_string() }; build LoadedGrant { id, grant, sha256 }.
+    // Collect into Result<Vec<LoadedGrant>, GrantProblem> (the first error wins).
+    let sources: &[(&str, &str)] = &[
+        ("builtin-memory", MEMORY),
+        ("builtin-recall", RECALL),
+        ("builtin-ask-memory", ASK_MEMORY),
+    ];
+```
+
+The id is `builtin-ask-memory`, with a hyphen; the tool in the grant's text is `ask_memory`, with
+an underscore.
+
+### `crates/brokerd/src/config.rs`
+
+**C1.** In `Runner::time_limit`, the line
+
+```rust
+            crate::args::ToolName::Recall | crate::args::ToolName::Memory => self.read_file_ms,
+```
+
+becomes
+
+```rust
+            crate::args::ToolName::Recall
+            | crate::args::ToolName::Memory
+            | crate::args::ToolName::AskMemory => self.read_file_ms,
+```
+
+### `crates/brokerd/src/policy/matching.rs`
+
+**P1.** The line
+
+```rust
+        ToolArgs::Recall(_) | ToolArgs::Memory(_) => Some(None),
+```
+
+becomes
+
+```rust
+        ToolArgs::Recall(_) | ToolArgs::Memory(_) | ToolArgs::AskMemory(_) => Some(None),
+```
+
+### `crates/brokerd/src/runner.rs`
+
+**R1.** The line
+
+```rust
+        ToolArgs::Recall(_) | ToolArgs::Memory(_) => (Vec::new(), None),
+```
+
+becomes
+
+```rust
+        ToolArgs::Recall(_) | ToolArgs::Memory(_) | ToolArgs::AskMemory(_) => (Vec::new(), None),
+```
+
+## Steps
+
+- [ ] **1. Copy the given tests.** From the repository root, three commands:
+
+  ```sh
+  cp docs/plans/M5c2/files/crates/brokerd/tests/ask_memory_tool.rs crates/brokerd/tests/
+  cp docs/plans/M5c2/files/crates/brokerd/tests/grants.rs crates/brokerd/tests/
+  cp docs/plans/M5c2/files/crates/brokerd/tests/memory_tools.rs crates/brokerd/tests/
+  ```
+
+  Then `git status --short` lists `grants.rs` and `memory_tools.rs` as modified and
+  `ask_memory_tool.rs` as new, all under `crates/brokerd/tests/`.
+- [ ] **2. See them fail.** Two commands, each with a shell timeout of 600000 ms:
+  - `cargo build --tests --keep-going -p brokerd`: one test target, `ask_memory_tool`, does not
+    compile, with 11 errors: one `E0432` (``unresolved import `brokerd::builtin::ASK_MEMORY` ``),
+    one `E0599` (``no variant, associated function, or constant named `AskMemory` found for enum
+    `ToolArgs` ``) and nine `E0599` (the same for enum `ToolName`). Every other target compiles.
+  - `cargo test --no-fail-fast -p brokerd --test grants --test memory_tools`: two tests fail,
+    each on its changed line: `an_empty_directory_is_a_valid_empty_set` in `grants` and
+    `grant_files_cannot_name_the_memory_tools_or_take_a_built_in_id` in `memory_tools`.
+- [ ] **3. The edits.** Make edits A1 to A9, B1, B2, C1, P1 and R1, in order. `cargo check -p
+  brokerd` prints no error and no warning.
+- [ ] **4. The tests.** With a shell timeout of 600000 ms, every test passes: 9 in
+  `ask_memory_tool`, 17 in `grants`, 4 in `memory_tools`, 5 in `memory_port`, 8 in `runner`:
+
+  ```sh
+  cargo test -p brokerd --test ask_memory_tool --test grants --test memory_tools \
+    --test memory_port --test runner
+  ```
+- [ ] **5. Check.** Two commands:
+  - `git diff --stat -- crates/brokerd/src` lists exactly the five files of "Modify" under
+    `crates/brokerd/src`: not `grants.rs`, `container.rs` or `memory_port.rs`.
+  - `git diff --stat -- crates/bxcore crates/loopd crates/memoryd Cargo.toml Cargo.lock` prints
+    nothing.
+- [ ] **6. Run the gate.** `cargo fmt --all`, then `make gate`, with a shell timeout of 600000 ms.
+  Expected last line: `gate: ok`. The test count is the passed total of `cargo test --workspace`
+  plus one, for the ignored `end_to_end` test that the gate runs on its own: **1381** after this
+  task, 1372 before it; the 9 new ones are the tests of `ask_memory_tool.rs` (the new versions of
+  `grants.rs` and `memory_tools.rs` keep their counts). To count, add up the `N passed` of every
+  `test result:` line the gate's `cargo test --workspace` printed, and add 1. Never run the gate
+  again only to count.
+- [ ] **7. Log and commit.** Add your row to `docs/implementer-log.md`; its Task cell is
+  `M5c2/02-brokerd-ask-memory`. `git status --short` lists the five source files, the two
+  replaced tests and your log as modified, and `ask_memory_tool.rs` as new. Stage with
+  `git add crates/brokerd 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 1381 tests.
+- `git diff HEAD~1 --stat` lists only the five source files, the three test files of step 1 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 when its turn comes, or is there more
+  than once.
+- Step 2 shows a different result: other targets not compiling, other errors, or other tests
+  failing.
+- `cargo check` in step 3 prints an error or a warning. Quote it, and stop.
+- A test fails in step 4. Do not change a test: stop, and give the failing tests' names and
+  their messages.
+- The fix seems to need a change anywhere else: `grants.rs`, `container.rs`, `memory_port.rs`,
+  `bxcore`, a test support file, a per-tool wait. It does not; stop and say why you think so.
+- A test outside the three given files 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.
+- The gate passes with a count other than 1381.
+- A comment in an edit does not fit the code under it. Quote it and stop.
+
+## What the given tests pin
+
+Each of these changes to the finished code makes at least one given test fail (17 tried, all
+caught): `ask_memory` not parsed as a name, or written another way; `AskMemory` not a memory
+tool; `check` skipped; `tool()` giving another tool; the canonical form lost; the built-in grant
+missing, under another id, asking instead of allowing, capped at `private`, or labelling its
+results untrusted; `BUILT_IN`'s old sentence; `AskMemory` matching no grant; an owner's grant
+naming it accepted; a container running it; `Routed` sending it to the container runtime;
+`memoryd`'s label dropped, so its answer no longer raises the session.

docs/plans/M5c2/README.md

@@ -73,16 +73,17 @@ Honcho sources and chats outside its state lock, and answers `ask_memory`.
 | # | Delivers | Crates |
 |---|---|---|
 | 00 | (done by the design model, `718e66d`) recordings from rift: workspace search with a limit and with no match, conclusion queries for two peers, and what a missing workspace answers | memoryd fixtures |
-| 01 | Wire: `AskMemoryArgs { question, workspaces }`, its name and its checks (question at most 2,000 characters; one or two workspace names, no repeats); fixtures | bxcore |
+| 01 | Wire: `AskMemoryArgs { question, workspaces }`, its name and its checks (question at most 2,000 characters; one or two workspace names, no repeats, each 1 to 512 characters as `memoryd`'s name rule) | bxcore |
-| 02 | `brokerd`: `ask_memory` as a built-in memory tool (grant, parsing, routing to `memoryd`); a per-tool wait on `memoryd`, 170 s for `ask_memory` and 30 s for the others | brokerd |
+| 02 | `brokerd`: `ask_memory` as a built-in memory tool (name, parsing, the grant `builtin-ask-memory`, owner grants naming it refused, routing to `memoryd`); its answer's labels raise the session | brokerd |
-| 03 | `loopd`: `ask_memory`'s schema as a core tool in a new registry; a per-tool wait on `brokerd`, 180 s for `ask_memory` and `[broker] timeout_ms` for the others; the system prompt's line on memory tools | loopd |
+| 03 | `brokerd`: a per-tool wait on `memoryd`, 170 s for `ask_memory` (`ASK_MEMORY_WAIT`) and 30 s for the others, chosen by `MemoryPort::wait_for` | brokerd |
-| 04 | `memoryd`: read clients, one per `[[honcho.read]]` workspace, the fleet and the agent's own two, with a 5 s limit for searches and 150 s for chat | memoryd |
+| 04 | `loopd`: `ask_memory`'s schema as a core tool in a new registry; a per-tool wait on `brokerd`, 180 s for `ask_memory` and `[broker] timeout_ms` for the others; the system prompt's line on memory tools | loopd |
-| 05 | `memoryd`, pure: Honcho messages and conclusions as `recall` items (tag, source name, class, neutralised, capped), kept only when they share a term with the query, interleaved with the local hits to `k`, the local index first | memoryd |
+| 05 | `memoryd`: read clients, one per `[[honcho.read]]` workspace, the fleet and the agent's own two, with a 5 s limit for searches and 150 s for chat | memoryd |
-| 06 | `memoryd`: `recall`'s Honcho sources, each on its own thread with its own limit, after the local search and outside the lock; `<source>: unavailable`; a query holding a secret sends nothing | memoryd |
+| 06 | `memoryd`, pure: Honcho messages and conclusions as `recall` items (tag, source name, class, neutralised, capped), kept only when they share a term with the query, interleaved with the local hits to `k`, the local index first | memoryd |
-| 07 | `memoryd`: `ask_memory`: workspace names checked against `[honcho]`, both chats at once, each answer tagged and capped, labels, `<workspace>: unavailable`; a question holding a secret sends nothing | memoryd |
+| 07 | `memoryd`: `recall`'s Honcho sources, each on its own thread with its own limit, after the local search and outside the lock; `<source>: unavailable`; a query holding a secret sends nothing | memoryd |
-| 08 | `memoryd` serves `ask_memory` beside `recall` and `memory`, outside the lock; `bxctl`'s usage and the runbook where a new message needs a pointer | memoryd, bxctl |
+| 08 | `memoryd`: `ask_memory`: workspace names checked against `[honcho]`, both chats at once, each answer tagged and capped, labels, `<workspace>: unavailable`; a question holding a secret sends nothing | memoryd |
+| 09 | `memoryd` serves `ask_memory` beside `recall` and `memory`, outside the lock; `bxctl`'s usage and the runbook where a new message needs a pointer | memoryd, bxctl |
 
-After task 08, the design model runs the M5c-2 device check (spec 13) on straylight, rift and the
+After task 09, the design model runs the M5c-2 device check (spec 13) on straylight, rift and the
 owner's Mattermost: a second `memoryd` under its own name writes a fleet note that `recall` finds,
 tagged `memory-tainted`, turning the session untrusted; `ask_memory` answers from the agent's own
 trusted workspace within its limit; with rift down, `recall` answers from the local index with
@@ -97,6 +98,8 @@ The gate count is the passed total of `cargo test --workspace` plus one, for the
 | Task | Gate count after | Mutations tried, caught |
 |---|---|---|
 | 01 | 1372 | 23, 23 |
+| 02 | 1381 | 17, 17 |
+| 03 | 1384 | 10, 10 |
 
 ## Before hand-over
 

docs/plans/M5c2/files/crates/brokerd/tests/ask_memory_tool.rs

@@ -0,0 +1,340 @@
+//! `ask_memory` in `brokerd`: its name and arguments, the built-in grant that allows it in every
+//! session, grant files that name it refused, its calls routed to `memoryd` and never to a
+//! container, and the labels of `memoryd`'s answer raising the session (M5c spec, section 9).
+//! Do not edit.
+
+#[path = "support/build.rs"]
+mod build;
+#[path = "support/client.rs"]
+mod client;
+#[path = "support/fake_podman.rs"]
+mod fake_podman;
+#[path = "support/rig.rs"]
+mod rig;
+#[path = "support/runtime.rs"]
+mod runtime;
+#[path = "support/sink.rs"]
+mod sink;
+#[path = "support/tmp.rs"]
+mod tmp;
+
+use std::os::unix::net::UnixListener;
+use std::path::Path;
+use std::sync::{Arc, mpsc};
+use std::time::Duration;
+
+use brokerd::args::{ArgsError, ToolArgs, ToolName, parse};
+use brokerd::broker::Broker;
+use brokerd::builtin::{ASK_MEMORY, BUILT_IN};
+use brokerd::container::{IN_MEMORYD, Podman};
+use brokerd::grants::load;
+use brokerd::memory_port::{MemoryPort, Routed};
+use brokerd::policy::{Label, SessionState, decide};
+use brokerd::runner::run;
+use build::{allowed, now, private, secret, set};
+use bxcore::{
+    AskMemoryArgs, DataClass, Envelope, MemoryCall, Message, SessionId, ToolResponse, read_frame,
+    sha256, write_frame,
+};
+use fake_podman::{Fake, Lines, serial};
+use rig::Rig;
+use runtime::{Recording, Shared};
+use tmp::TempDir;
+
+/// A fake `memoryd`: answers one call with `answer` and hands back what it was sent. A failed
+/// write is ignored: the test has its answer or has already failed.
+fn fake_memoryd(socket: &Path, answer: ToolResponse) -> mpsc::Receiver<MemoryCall> {
+    let listener = UnixListener::bind(socket).unwrap();
+    let (tx, rx) = mpsc::channel();
+    std::thread::spawn(move || {
+        let Ok((mut stream, _)) = listener.accept() else {
+            return;
+        };
+        let Ok(env) = read_frame(&mut stream) else {
+            return;
+        };
+        let Message::MemoryCall(call) = env.msg else {
+            return;
+        };
+        let _ = tx.send(call);
+        let reply = Envelope {
+            v: 1,
+            id: env.id,
+            r#final: true,
+            msg: Message::ToolResponse(answer),
+        };
+        let _ = write_frame(&mut stream, &reply);
+    });
+    rx
+}
+
+fn answer(class: DataClass, untrusted: bool) -> ToolResponse {
+    ToolResponse::Result {
+        content: "own: memory · The owner prefers UTC.".to_string(),
+        class,
+        untrusted,
+        truncated: false,
+    }
+}
+
+fn routed(socket: &Path, other: &Arc<Recording>) -> Routed {
+    Routed {
+        memory: MemoryPort::new(socket.to_path_buf(), Duration::from_secs(5)),
+        other: Box::new(Shared(Arc::clone(other))),
+    }
+}
+
+const QUESTION: &str = r#"{"question":"What does the owner prefer?"}"#;
+
+#[test]
+fn the_name_and_the_arguments() {
+    assert_eq!(ToolName::parse("ask_memory"), Some(ToolName::AskMemory));
+    assert_eq!(ToolName::AskMemory.as_str(), "ask_memory");
+    assert!(ToolName::AskMemory.is_memory());
+    assert!(
+        !ToolName::ALL.contains(&ToolName::AskMemory),
+        "ALL is the container tools"
+    );
+    let args = parse(
+        ToolName::AskMemory,
+        r#"{ "workspaces": ["fleet", "boxmaker1"], "question": "Who wrote it?" }"#,
+    )
+    .unwrap();
+    assert_eq!(args.tool(), ToolName::AskMemory);
+    assert_eq!(
+        args,
+        ToolArgs::AskMemory(AskMemoryArgs {
+            question: "Who wrote it?".to_string(),
+            workspaces: vec!["fleet".to_string(), "boxmaker1".to_string()],
+        })
+    );
+    assert_eq!(
+        args.canonical_json(),
+        r#"{"question":"Who wrote it?","workspaces":["fleet","boxmaker1"]}"#
+    );
+    let default = parse(ToolName::AskMemory, QUESTION).unwrap();
+    assert_eq!(
+        default.canonical_json(),
+        r#"{"question":"What does the owner prefer?","workspaces":[]}"#,
+        "every field is written, as recall's k is"
+    );
+}
+
+/// The refusals are `bxcore`'s sentences, shown to the model after `invalid argument shape: `.
+#[test]
+fn bad_arguments_are_refused_with_the_checks_sentences() {
+    let long = format!(r#"{{"question":"{}"}}"#, "é".repeat(2_001));
+    for (json, sentence) in [
+        (
+            r#"{"question":"  "}"#,
+            "question is empty; write the question to ask",
+        ),
+        (
+            long.as_str(),
+            "question has 2001 characters, over the limit of 2000; ask a shorter question",
+        ),
+        (
+            r#"{"question":"q","workspaces":["own","untrusted","fleet"]}"#,
+            "workspaces names 3, over the limit of 2; ask about the others in another call",
+        ),
+        (
+            r#"{"question":"q","workspaces":["fleet","fleet"]}"#,
+            "workspaces names fleet twice; name each workspace once",
+        ),
+        (
+            r#"{"question":"q","workspaces":[""]}"#,
+            "a workspace name is empty; name a workspace, or leave workspaces out to ask own",
+        ),
+        (
+            r#"{"question":"q","workspaces":["a.b"]}"#,
+            "a workspace name has 1 to 512 letters, digits, _ or -; use own, untrusted, fleet or \
+             another agent's name",
+        ),
+    ] {
+        let error = parse(ToolName::AskMemory, json).unwrap_err();
+        assert_eq!(error, ArgsError::Shape(sentence.to_string()), "{json}");
+        assert_eq!(
+            error.detail(),
+            format!("invalid argument shape: {sentence}")
+        );
+    }
+    for json in [
+        r#"{"question":"q","reasoning":"low"}"#,
+        r#"{"workspaces":["own"]}"#,
+        "not json",
+    ] {
+        let error = parse(ToolName::AskMemory, json).unwrap_err();
+        assert!(matches!(error, ArgsError::Shape(_)), "{json}: {error:?}");
+    }
+}
+
+/// The built-in grant allows `ask_memory` in every session, `secret` and untrusted ones too: a
+/// question is not written to Honcho (M5c spec, section 8). Its results are labelled `private`
+/// and trusted; `memoryd`'s own labels can only make them stricter.
+#[test]
+fn the_built_in_grant_allows_it_in_every_session() {
+    assert_eq!(
+        ASK_MEMORY,
+        "tool = \"ask_memory\"\nmode = \"auto\"\nmax_taint = \"secret\"\nresult_class = \
+         \"private\"\nuntrusted = false\n"
+    );
+    let grants = set(vec![]);
+    let untrusted = SessionState {
+        taint: DataClass::Secret,
+        untrusted: true,
+    };
+    for state in [private(), secret(), untrusted] {
+        let d = allowed(decide(
+            build::request("ask_memory", QUESTION),
+            &grants,
+            state,
+            now(),
+        ));
+        assert_eq!(d.grant(), "builtin-ask-memory");
+        assert_eq!(d.grant_sha256(), sha256(ASK_MEMORY.as_bytes()).unwrap());
+        assert_eq!(
+            d.label(),
+            Label {
+                class: DataClass::Private,
+                untrusted: false
+            }
+        );
+    }
+    let ids: Vec<&str> = grants.matchable().map(|g| g.id.as_str()).collect();
+    assert_eq!(
+        ids,
+        ["builtin-memory", "builtin-recall", "builtin-ask-memory"]
+    );
+}
+
+#[test]
+fn a_grant_file_cannot_name_it() {
+    assert_eq!(
+        BUILT_IN,
+        "the policy of recall, memory and ask_memory is built in; remove this grant"
+    );
+    let dir = TempDir::new("ask-grants");
+    std::fs::write(
+        dir.path().join("mine.toml"),
+        "tool = \"ask_memory\"\nmode = \"deny\"\nmax_taint = \"secret\"\n",
+    )
+    .unwrap();
+    let problems = load(dir.path()).unwrap_err();
+    let got: Vec<(String, String)> = problems.into_iter().map(|p| (p.file, p.problem)).collect();
+    assert_eq!(got, [("mine.toml".to_string(), BUILT_IN.to_string())]);
+}
+
+#[test]
+fn it_goes_to_memoryd_with_its_canonical_arguments() {
+    let dir = TempDir::new("ask-route");
+    let socket = dir.path().join("memory.sock");
+    let seen = fake_memoryd(&socket, answer(DataClass::Private, false));
+    let other = Recording::answering("container");
+    let asked = r#"{"workspaces":["own","fleet"],"question":"Who wrote it?"}"#;
+    let got = run(
+        allowed(decide(
+            build::request("ask_memory", asked),
+            &set(vec![]),
+            secret(),
+            now(),
+        )),
+        &routed(&socket, &other),
+    );
+    assert_eq!(got, answer(DataClass::Private, false));
+    let call = seen.recv_timeout(Duration::from_secs(5)).unwrap();
+    assert_eq!(call.request.tool, "ask_memory");
+    assert_eq!(
+        call.request.arguments,
+        r#"{"question":"Who wrote it?","workspaces":["own","fleet"]}"#
+    );
+    assert_eq!((call.taint, call.untrusted), (DataClass::Secret, false));
+    assert_eq!(other.count(), 0, "nothing reached the other runtime");
+}
+
+#[test]
+fn a_container_runtime_never_runs_it() {
+    let _s = serial();
+    let fake = Fake::new("ask", "exit 0");
+    let log = Lines::default();
+    let podman = Podman::new(fake.runner(""), fake.dir.join("egress"), log.sink());
+    let got = run(
+        allowed(decide(
+            build::request("ask_memory", QUESTION),
+            &set(vec![]),
+            private(),
+            now(),
+        )),
+        &podman,
+    );
+    assert_eq!(
+        got,
+        ToolResponse::Failed {
+            message: IN_MEMORYD.to_string()
+        }
+    );
+    assert!(fake.calls().is_empty(), "podman was never called");
+}
+
+/// One `ask_memory` call through `broker::handle`, whose runtime sends it to a fake `memoryd`
+/// answering `reply`; the session's state afterwards.
+fn state_after(tag: &str, reply: ToolResponse) -> SessionState {
+    let rig = Rig::new(tag);
+    let socket = rig.dir.path().join("memory.sock");
+    let _seen = fake_memoryd(&socket, reply);
+    let other = Recording::answering("unused");
+    let broker = Arc::new(Broker::new(
+        rig.cfg.clone(),
+        rig.ledger(),
+        Box::new(routed(&socket, &other)),
+        rig.lines.sink(),
+    ));
+    let frames = client::call(&broker, rig::request("s1", 1, "ask_memory", QUESTION));
+    assert!(
+        matches!(
+            frames.as_slice(),
+            [Envelope {
+                msg: Message::ToolResponse(ToolResponse::Result { .. }),
+                ..
+            }]
+        ),
+        "{frames:?}"
+    );
+    rig.state().read(&SessionId::new("s1").unwrap()).unwrap()
+}
+
+/// T58: an answer from a workspace that is not trusted makes a clean session untrusted, as a
+/// `memory-tainted` recall hit does.
+#[test]
+fn an_untrusted_answer_makes_the_session_untrusted() {
+    assert_eq!(
+        state_after("ask-untrusted", answer(DataClass::Private, true)),
+        SessionState {
+            taint: DataClass::Private,
+            untrusted: true
+        }
+    );
+}
+
+/// T58: an answer labelled `secret` raises the session to `secret`.
+#[test]
+fn a_secret_answer_raises_the_session_to_secret() {
+    assert_eq!(
+        state_after("ask-secret", answer(DataClass::Secret, false)),
+        SessionState {
+            taint: DataClass::Secret,
+            untrusted: false
+        }
+    );
+}
+
+/// The grant's label is the floor: a `public` answer still leaves the session `private`.
+#[test]
+fn a_public_answer_leaves_the_session_private() {
+    assert_eq!(
+        state_after("ask-public", answer(DataClass::Public, false)),
+        SessionState {
+            taint: DataClass::Private,
+            untrusted: false
+        }
+    );
+}

docs/plans/M5c2/files/crates/brokerd/tests/grants.rs

@@ -0,0 +1,487 @@
+//! Tests for loading grant files. Do not edit these or the fixtures.
+//!
+//! One case per loading rule in the M3a spec, each with words its problem text must contain, and
+//! the rule that matters most: one invalid file makes the whole set invalid.
+
+#[path = "support/tmp.rs"]
+mod tmp;
+
+use brokerd::grants::{GrantSet, LoadedGrant, RUNBOOK, load, render, valid_id};
+use bxcore::{Constraints, DataClass, Grant, GrantProblem, Hash32, Mode};
+use std::path::{Path, PathBuf};
+use tmp::TempDir;
+
+fn fixture(case: &str) -> PathBuf {
+    Path::new(env!("CARGO_MANIFEST_DIR"))
+        .join("tests/fixtures/grants")
+        .join(case)
+}
+
+const GOOD: &str = "tool = \"read_file\"\nmode = \"auto\"\nmax_taint = \"secret\"\n\n\
+                    [constraints]\npaths = [\"/home/kyle/notes\"]\n";
+
+/// A directory holding one good grant and the given files; returns the problems of loading it.
+fn problems_of(files: &[(&str, &str)]) -> Vec<GrantProblem> {
+    let dir = TempDir::new("grants");
+    dir.write("good.toml", GOOD);
+    for (name, text) in files {
+        dir.write(name, text);
+    }
+    load(dir.path()).expect_err("the set should be invalid")
+}
+
+/// Exactly one problem, in `file`, whose text contains every one of `words`.
+fn one_problem(files: &[(&str, &str)], file: &str, words: &[&str]) -> GrantProblem {
+    let problems = problems_of(files);
+    assert_eq!(problems.len(), 1, "{problems:?}");
+    let p = problems.into_iter().next().unwrap();
+    assert_eq!(p.file, file);
+    for word in words {
+        assert!(p.problem.contains(word), "{:?} lacks {word:?}", p.problem);
+    }
+    p
+}
+
+fn body(tool: &str, mode: &str, rest: &str) -> String {
+    format!("tool = \"{tool}\"\nmode = \"{mode}\"\nmax_taint = \"secret\"\n{rest}")
+}
+
+#[test]
+fn the_valid_fixture_loads_in_id_order_with_file_hashes() {
+    let set = load(&fixture("valid")).unwrap();
+    let ids: Vec<&str> = set.grants().iter().map(|g| g.id.as_str()).collect();
+    assert_eq!(
+        ids,
+        [
+            "fetch-example",
+            "no-fetch-internal",
+            "notes-read",
+            "scratch-write",
+            "shell-bare"
+        ]
+    );
+    let notes = &set.grants()[2];
+    assert_eq!(notes.grant.tool, "read_file");
+    assert_eq!(notes.grant.mode, Mode::Auto);
+    assert!(!notes.grant.untrusted);
+    assert_eq!(notes.grant.constraints.paths, ["/home/kyle/notes"]);
+    let bytes = std::fs::read(fixture("valid").join("notes-read.toml")).unwrap();
+    assert_eq!(notes.sha256, bxcore::sha256(&bytes).unwrap());
+    // Defaults from `bxcore::Grant`.
+    let scratch = &set.grants()[3];
+    assert_eq!(scratch.grant.result_class, DataClass::Private);
+    assert!(scratch.grant.untrusted);
+    assert_eq!(scratch.grant.expires, None);
+}
+
+#[test]
+fn files_that_do_not_end_in_toml_are_ignored() {
+    let dir = TempDir::new("grants");
+    dir.write("good.toml", GOOD);
+    dir.write("good.toml~", "not toml at all {{{");
+    dir.write("good.toml.bak", "not toml at all {{{");
+    dir.write("README.md", "# notes");
+    dir.write("toml", "x");
+    std::fs::create_dir(dir.path().join("archive")).unwrap();
+    let set = load(dir.path()).unwrap();
+    assert_eq!(set.grants().len(), 1);
+    assert_eq!(set.grants()[0].id, "good");
+}
+
+#[test]
+fn an_empty_directory_is_a_valid_empty_set() {
+    let set = load(&fixture("empty")).unwrap();
+    assert!(set.grants().is_empty());
+    let builtin: Vec<&str> = set.matchable().map(|g| g.id.as_str()).collect();
+    assert_eq!(
+        builtin,
+        ["builtin-memory", "builtin-recall", "builtin-ask-memory"],
+        "only the built-ins"
+    );
+}
+
+#[test]
+fn a_missing_directory_is_a_problem_not_an_empty_set() {
+    let missing = fixture("does-not-exist");
+    let problems = load(&missing).unwrap_err();
+    assert_eq!(problems.len(), 1);
+    assert!(problems[0].file.contains("does-not-exist"), "{problems:?}");
+    assert!(problems[0].problem.contains("cannot be read"));
+    // A file where the directory should be is the same.
+    let dir = TempDir::new("grants");
+    let file = dir.write("grants", "x");
+    assert!(load(&file).is_err());
+}
+
+/// The rule the whole design leans on. `fetch-example` alone would allow a fetch that the
+/// mistyped `no-fetch-internal` was written to forbid, so nothing loads at all.
+#[test]
+fn one_invalid_file_makes_the_whole_set_invalid() {
+    let problems = load(&fixture("one-bad")).unwrap_err();
+    assert_eq!(problems.len(), 1, "{problems:?}");
+    assert_eq!(problems[0].file, "no-fetch-internal.toml");
+    assert_eq!(problems[0].line, Some(3));
+    assert!(problems[0].problem.contains("mdoe"), "{problems:?}");
+    // The same directory without the bad file is fine.
+    let dir = TempDir::new("grants");
+    for name in ["notes-read.toml", "fetch-example.toml"] {
+        let text = std::fs::read_to_string(fixture("one-bad").join(name)).unwrap();
+        dir.write(name, &text);
+    }
+    assert_eq!(load(dir.path()).unwrap().grants().len(), 2);
+}
+
+#[test]
+fn every_problem_in_every_file_is_reported() {
+    let problems = load(&fixture("many-bad")).unwrap_err();
+    let got: Vec<(&str, Option<u64>)> =
+        problems.iter().map(|p| (p.file.as_str(), p.line)).collect();
+    assert_eq!(
+        got,
+        [
+            ("a-secret.toml", None),
+            ("a-secret.toml", None),
+            ("b-paths.toml", None),
+            ("b-paths.toml", None),
+            ("b-paths.toml", None),
+            ("b-paths.toml", None),
+            ("c-syntax.toml", Some(3)),
+        ],
+        "{problems:?}"
+    );
+    let all: String = problems
+        .iter()
+        .map(|p| format!("{}\n", p.problem))
+        .collect();
+    for words in [
+        "secrets are not supported until M4",
+        "patterns are not supported",
+        "read_file does not take hosts",
+        "\"notes\" is not a valid absolute path",
+        "\"/home/kyle/../etc\" is not a valid absolute path",
+        "a grant of the whole file system is not supported",
+    ] {
+        assert!(all.contains(words), "missing {words:?} in:\n{all}");
+    }
+}
+
+#[test]
+fn rule_1_unreadable_not_utf8_or_not_a_grant() {
+    one_problem(&[("bad.toml", "tool = ")], "bad.toml", &[]);
+    let p = one_problem(
+        &[("bad.toml", &body("shell", "auto", "colour = \"red\"\n"))],
+        "bad.toml",
+        &["colour"],
+    );
+    assert_eq!(p.line, Some(4));
+    one_problem(
+        &[("bad.toml", "tool = \"shell\"\nmode = \"auto\"\n")],
+        "bad.toml",
+        &["max_taint"],
+    );
+    one_problem(
+        &[("bad.toml", &body("shell", "sometimes", ""))],
+        "bad.toml",
+        &["sometimes"],
+    );
+    one_problem(
+        &[(
+            "bad.toml",
+            &body("shell", "auto", "[constraints]\ncwd = [\"/a\"]\n"),
+        )],
+        "bad.toml",
+        &["cwd"],
+    );
+
+    // Not UTF-8.
+    let dir = TempDir::new("grants");
+    dir.write("good.toml", GOOD);
+    std::fs::write(dir.path().join("latin1.toml"), b"tool = \"caf\xe9\"\n").unwrap();
+    let problems = load(dir.path()).unwrap_err();
+    assert_eq!(problems.len(), 1);
+    assert_eq!(problems[0].file, "latin1.toml");
+    assert!(problems[0].problem.contains("UTF-8"), "{problems:?}");
+
+    // Exists but cannot be read: a directory with a grant's name.
+    let dir = TempDir::new("grants");
+    dir.write("good.toml", GOOD);
+    std::fs::create_dir(dir.path().join("folder.toml")).unwrap();
+    let problems = load(dir.path()).unwrap_err();
+    assert_eq!(problems.len(), 1);
+    assert_eq!(problems[0].file, "folder.toml");
+    assert!(
+        problems[0].problem.contains("cannot be read"),
+        "{problems:?}"
+    );
+}
+
+#[test]
+fn rule_1_a_file_without_read_permission_is_a_problem() {
+    use std::os::unix::fs::PermissionsExt;
+    if tmp::running_as_root("rule_1_a_file_without_read_permission_is_a_problem") {
+        return;
+    }
+    let dir = TempDir::new("grants");
+    dir.write("good.toml", GOOD);
+    let locked = dir.write("locked.toml", GOOD);
+    std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o000)).unwrap();
+    let problems = load(dir.path()).unwrap_err();
+    assert_eq!(problems.len(), 1);
+    assert_eq!(problems[0].file, "locked.toml");
+    assert!(
+        problems[0].problem.contains("cannot be read"),
+        "{problems:?}"
+    );
+}
+
+#[test]
+fn rule_2_the_file_stem_is_the_id() {
+    for id in ["a", "notes-read", "0", "a-1-b", &"x".repeat(64)] {
+        assert!(valid_id(id), "{id:?}");
+    }
+    for id in [
+        "",
+        "Notes",
+        "notes_read",
+        "notes.read",
+        "notes read",
+        ".hidden",
+        &"x".repeat(65),
+    ] {
+        assert!(!valid_id(id), "{id:?}");
+    }
+    one_problem(
+        &[("Bad_Name.toml", GOOD)],
+        "Bad_Name.toml",
+        &["not a valid grant id"],
+    );
+    one_problem(&[(".toml", GOOD)], ".toml", &["not a valid grant id"]);
+    one_problem(&[("a.b.toml", GOOD)], "a.b.toml", &["not a valid grant id"]);
+}
+
+#[test]
+fn rule_3_the_tool_is_one_of_the_four() {
+    for tool in ["echo", "clock", "Read_File", ""] {
+        one_problem(
+            &[("bad.toml", &body(tool, "auto", ""))],
+            "bad.toml",
+            &[
+                "unknown tool",
+                "read_file, write_file, shell and http_fetch",
+            ],
+        );
+    }
+}
+
+#[test]
+fn rules_4_and_5_secrets_and_patterns_are_not_supported() {
+    one_problem(
+        &[("bad.toml", &body("shell", "ask", "secret = \"token\"\n"))],
+        "bad.toml",
+        &["secrets are not supported until M4"],
+    );
+    one_problem(
+        &[(
+            "bad.toml",
+            &body("shell", "ask", "[constraints]\npatterns = [\"^ls\"]\n"),
+        )],
+        "bad.toml",
+        &["patterns are not supported"],
+    );
+    // An empty list is the same as no list.
+    let dir = TempDir::new("grants");
+    dir.write(
+        "ok.toml",
+        &body("shell", "ask", "[constraints]\npatterns = []\nhosts = []\n"),
+    );
+    assert!(load(dir.path()).is_ok());
+}
+
+/// The table of rule 6, cell by cell.
+#[test]
+fn rule_6_each_tool_takes_its_own_constraints() {
+    let paths = "[constraints]\npaths = [\"/a\"]\n";
+    let hosts = "[constraints]\nhosts = [\"example.com\"]\n";
+    let both = "[constraints]\npaths = [\"/a\"]\nhosts = [\"example.com\"]\n";
+    for tool in ["read_file", "write_file"] {
+        one_problem(
+            &[("bad.toml", &body(tool, "auto", ""))],
+            "bad.toml",
+            &[tool, "needs at least one path"],
+        );
+        one_problem(
+            &[("bad.toml", &body(tool, "auto", both))],
+            "bad.toml",
+            &[tool, "does not take hosts"],
+        );
+    }
+    one_problem(
+        &[("bad.toml", &body("shell", "auto", both))],
+        "bad.toml",
+        &["shell does not take hosts"],
+    );
+    one_problem(
+        &[("bad.toml", &body("http_fetch", "auto", ""))],
+        "bad.toml",
+        &["http_fetch needs at least one host"],
+    );
+    one_problem(
+        &[("bad.toml", &body("http_fetch", "auto", both))],
+        "bad.toml",
+        &["http_fetch does not take paths"],
+    );
+    // The allowed cells.
+    let dir = TempDir::new("grants");
+    dir.write("r.toml", &body("read_file", "auto", paths));
+    dir.write("w.toml", &body("write_file", "auto", paths));
+    dir.write("s1.toml", &body("shell", "auto", paths));
+    dir.write("s2.toml", &body("shell", "auto", ""));
+    dir.write("h.toml", &body("http_fetch", "auto", hosts));
+    assert_eq!(load(dir.path()).unwrap().grants().len(), 5);
+    // Two wrong cells in one file are two problems.
+    let wrong = body("http_fetch", "auto", paths);
+    assert_eq!(problems_of(&[("bad.toml", &wrong)]).len(), 2);
+}
+
+#[test]
+fn rule_7_paths_are_valid_absolute_paths_and_never_the_root() {
+    for bad in ["notes", "/a/../b", "/a//b", "/a/./b", "/a/", ""] {
+        let text = body(
+            "shell",
+            "auto",
+            &format!("[constraints]\npaths = [{bad:?}]\n"),
+        );
+        one_problem(
+            &[("bad.toml", &text)],
+            "bad.toml",
+            &["is not a valid absolute path"],
+        );
+    }
+    let root = body(
+        "shell",
+        "auto",
+        "[constraints]\npaths = [\"/home/kyle\", \"/\"]\n",
+    );
+    one_problem(
+        &[("bad.toml", &root)],
+        "bad.toml",
+        &["a grant of the whole file system is not supported"],
+    );
+}
+
+#[test]
+fn rule_8_hosts_are_valid_host_patterns() {
+    for bad in [
+        "Example.com",
+        "example.com:443",
+        "127.0.0.1",
+        "localhost",
+        "*.com",
+        "https://example.com",
+    ] {
+        let text = body(
+            "http_fetch",
+            "auto",
+            &format!("[constraints]\nhosts = [{bad:?}]\n"),
+        );
+        one_problem(
+            &[("bad.toml", &text)],
+            "bad.toml",
+            &[bad, "is not a valid host pattern"],
+        );
+    }
+}
+
+#[test]
+fn rule_9_a_deny_grant_applies_at_every_taint() {
+    for taint in ["public", "private"] {
+        let text = format!("tool = \"shell\"\nmode = \"deny\"\nmax_taint = \"{taint}\"\n");
+        one_problem(
+            &[("bad.toml", &text)],
+            "bad.toml",
+            &["a deny grant must apply at every taint"],
+        );
+    }
+    let dir = TempDir::new("grants");
+    dir.write("no-shell.toml", &body("shell", "deny", ""));
+    assert!(load(dir.path()).is_ok());
+    // The rule is about deny only.
+    dir.write(
+        "ask.toml",
+        "tool = \"shell\"\nmode = \"ask\"\nmax_taint = \"public\"\n",
+    );
+    assert!(load(dir.path()).is_ok());
+}
+
+fn loaded(id: &str, tool: &str, mode: Mode) -> LoadedGrant {
+    LoadedGrant {
+        id: id.to_string(),
+        grant: Grant {
+            tool: tool.to_string(),
+            mode,
+            max_taint: DataClass::Secret,
+            result_class: DataClass::Private,
+            untrusted: true,
+            expires: None,
+            secret: None,
+            constraints: Constraints::default(),
+        },
+        sha256: Hash32::ZERO,
+    }
+}
+
+/// `from_grants` is how tests and the property test build a set without files. It applies the
+/// same value rules, sorts by id, and refuses two grants with one id.
+#[test]
+fn from_grants_applies_the_value_rules() {
+    let set = GrantSet::from_grants(vec![
+        loaded("zz", "shell", Mode::Ask),
+        loaded("aa", "shell", Mode::Deny),
+    ])
+    .unwrap();
+    assert_eq!(set.grants()[0].id, "aa");
+    assert_eq!(set.grants()[1].id, "zz");
+
+    let problems = GrantSet::from_grants(vec![
+        loaded("ok", "shell", Mode::Auto),
+        loaded("no-paths", "read_file", Mode::Auto),
+        loaded("Bad", "shell", Mode::Auto),
+    ])
+    .unwrap_err();
+    let files: Vec<&str> = problems.iter().map(|p| p.file.as_str()).collect();
+    assert_eq!(files, ["Bad.toml", "no-paths.toml"]);
+
+    let twice = GrantSet::from_grants(vec![
+        loaded("same", "shell", Mode::Auto),
+        loaded("same", "shell", Mode::Ask),
+    ])
+    .unwrap_err();
+    assert!(
+        twice[0].problem.contains("two grants have this id"),
+        "{twice:?}"
+    );
+}
+
+#[test]
+fn render_prints_every_problem_and_then_the_runbook_pointer() {
+    let problems = [
+        GrantProblem {
+            file: "a.toml".to_string(),
+            line: Some(3),
+            problem: "unknown field `mdoe`".to_string(),
+        },
+        GrantProblem {
+            file: "Bad_Name.toml".to_string(),
+            line: None,
+            problem: "the file name is not a valid grant id".to_string(),
+        },
+    ];
+    assert_eq!(
+        render(&problems),
+        "a.toml:3: unknown field `mdoe`\n\
+         Bad_Name.toml: the file name is not a valid grant id\n\
+         see docs/runbook.md#grants-invalid\n"
+    );
+    assert_eq!(RUNBOOK, "see docs/runbook.md#grants-invalid");
+    assert!(render(&problems).trim_end().ends_with(RUNBOOK));
+}

docs/plans/M5c2/files/crates/brokerd/tests/memory_tools.rs

@@ -0,0 +1,142 @@
+//! `recall` and `memory` in `brokerd`: their arguments, the built-in grants that allow them in
+//! every session, and grant files that name them refused (M5a spec, section 8). Do not edit.
+
+#[path = "support/build.rs"]
+mod build;
+#[path = "support/tmp.rs"]
+mod tmp;
+
+use brokerd::args::{ToolArgs, ToolName, parse};
+use brokerd::builtin::{BUILT_IN, MEMORY, RECALL};
+use brokerd::grants::{GrantSet, load};
+use brokerd::policy::{Label, Outcome, decide};
+use build::{allowed, grant, now, private, request, secret, set};
+use bxcore::{DataClass, DenyReason, Mode, sha256};
+use tmp::TempDir;
+
+#[test]
+fn the_names_and_arguments_of_the_memory_tools() {
+    assert_eq!(ToolName::parse("recall"), Some(ToolName::Recall));
+    assert_eq!(ToolName::parse("memory"), Some(ToolName::Memory));
+    assert_eq!(
+        (ToolName::Recall.as_str(), ToolName::Memory.as_str()),
+        ("recall", "memory")
+    );
+    assert!(ToolName::Recall.is_memory() && ToolName::Memory.is_memory());
+    assert!(
+        ToolName::ALL.iter().all(|t| !t.is_memory()),
+        "ALL is the container tools"
+    );
+    let recall = parse(ToolName::Recall, r#"{ "k": 3, "query": "launch" }"#).unwrap();
+    assert_eq!(recall.tool(), ToolName::Recall);
+    assert_eq!(recall.canonical_json(), r#"{"query":"launch","k":3}"#);
+    let memory = parse(ToolName::Memory, r#"{"text":"x","action":"note"}"#).unwrap();
+    assert!(matches!(memory, ToolArgs::Memory(_)));
+    assert_eq!(memory.canonical_json(), r#"{"action":"note","text":"x"}"#);
+    for (tool, bad) in [
+        (ToolName::Recall, r#"{"query":""}"#),
+        (ToolName::Recall, r#"{"query":"x","k":11}"#),
+        (
+            ToolName::Memory,
+            r#"{"action":"add","file":"soul","text":"x"}"#,
+        ),
+        (
+            ToolName::Memory,
+            r#"{"action":"add","file":"memory","text":"a\n§\nb"}"#,
+        ),
+        (
+            ToolName::Memory,
+            r#"{"action":"consolidate","file":"memory","entries":[]}"#,
+        ),
+    ] {
+        assert!(parse(tool, bad).is_err(), "{bad}");
+    }
+}
+
+#[test]
+fn both_are_allowed_in_every_session_by_the_built_in_grants() {
+    let grants = set(vec![]);
+    for state in [private(), secret()] {
+        let d = allowed(decide(
+            request("recall", r#"{"query":"x"}"#),
+            &grants,
+            state,
+            now(),
+        ));
+        assert_eq!(d.grant(), "builtin-recall");
+        assert_eq!(d.grant_sha256(), sha256(RECALL.as_bytes()).unwrap());
+        assert_eq!(
+            d.label(),
+            Label {
+                class: DataClass::Private,
+                untrusted: false
+            }
+        );
+        let d = allowed(decide(
+            request("memory", r#"{"action":"note","text":"x"}"#),
+            &grants,
+            state,
+            now(),
+        ));
+        assert_eq!(d.grant(), "builtin-memory");
+        assert_eq!(d.grant_sha256(), sha256(MEMORY.as_bytes()).unwrap());
+    }
+}
+
+#[test]
+fn a_set_without_built_ins_has_no_memory_tools() {
+    let outcome = decide(
+        request("recall", r#"{"query":"x"}"#),
+        &GrantSet::default(),
+        private(),
+        now(),
+    );
+    assert!(
+        matches!(outcome, Outcome::Denied(ref d) if d.reason == DenyReason::NoGrant),
+        "{outcome:?}"
+    );
+}
+
+#[test]
+fn grant_files_cannot_name_the_memory_tools_or_take_a_built_in_id() {
+    let dir = TempDir::new("memory-grants");
+    std::fs::write(
+        dir.path().join("mine.toml"),
+        "tool = \"memory\"\nmode = \"ask\"\nmax_taint = \"secret\"\n",
+    )
+    .unwrap();
+    std::fs::write(
+        dir.path().join("off.toml"),
+        "tool = \"recall\"\nmode = \"deny\"\nmax_taint = \"secret\"\n",
+    )
+    .unwrap();
+    std::fs::write(
+        dir.path().join("builtin-x.toml"),
+        "tool = \"shell\"\nmode = \"ask\"\nmax_taint = \"private\"\n",
+    )
+    .unwrap();
+    let problems = load(dir.path()).unwrap_err();
+    let got: Vec<(String, String)> = problems.into_iter().map(|p| (p.file, p.problem)).collect();
+    assert!(
+        got.contains(&("mine.toml".to_string(), BUILT_IN.to_string())),
+        "{got:?}"
+    );
+    assert!(
+        got.contains(&("off.toml".to_string(), BUILT_IN.to_string())),
+        "{got:?}"
+    );
+    assert!(
+        got.contains(&(
+            "builtin-x.toml".to_string(),
+            "grant ids starting builtin- are reserved".to_string()
+        )),
+        "{got:?}"
+    );
+    let with_shell = set(vec![grant("s", "shell", Mode::Ask)]);
+    assert_eq!(
+        with_shell.grants().len(),
+        1,
+        "the built-ins are not listed as the owner's"
+    );
+    assert_eq!(with_shell.matchable().count(), 4);
+}