boxmaker/docs/plans/M5a/11-brokerd-memory-tools.md

# M5a task 11: `brokerd` knows the memory tools, under built-in grants

**Branch:** `m5a` (run `git switch m5a`; `git status --short` must be empty, otherwise stop)
**Commit subject:** `brokerd: recall and memory, decided under built-in grants`

## Goal

M5a spec, section 8:

- `recall` and `memory` are tools `brokerd` parses and decides, with their shape checks from
  `proto` (task 02).
- Their policy is **built in**: two fixed grants, `builtin-recall` and `builtin-memory`, both
  `auto`, in every set of grants. They are kept apart from the owner's grants (`grants()` lists
  only the owner's; matching sees both through `matchable()`).
- A grant file naming `recall` or `memory` is a grant problem at load, and grant ids starting
  `builtin-` are reserved. So no grant file can turn memory off or put questions in front of it.

Where a decided memory call goes (`memoryd`, not a container) is task 12. Until then, like every
tool, it reaches the configured runtime.

## Files

- Copy (skeleton): `crates/brokerd/src/builtin.rs`
- Copy (tests): `crates/brokerd/tests/memory_tools.rs`, and a **new version** of the given
  `crates/brokerd/tests/grants.rs` (an empty directory now loads the two built-ins, which the old
  test compared against an empty set)
- Modify: `crates/brokerd/src/lib.rs` (`pub mod builtin;` after `pub mod broker;`),
  `crates/brokerd/src/args.rs`, `crates/brokerd/src/grants.rs`,
  `crates/brokerd/src/policy/matching.rs`, `crates/brokerd/src/runner.rs`,
  `crates/brokerd/src/config.rs`, `docs/implementer-log.md`

## The skeleton

`builtin.rs`: `RESERVED`, the two grants' text `RECALL` and `MEMORY`, and the sentence `BUILT_IN`
are written. Fill `grants`.

## `args.rs`, exactly

1. After `use proto::tools::{HttpFetchArgs, ReadFileArgs, ShellArgs, WriteFileArgs};` add
   `use proto::{MemoryArgs, RecallArgs};`.
2. The doc comment of `enum ToolName`, `/// The four tools the broker can dispatch to.`, becomes
   `/// 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).` (two lines), and add the variants
   `Recall,` and `Memory,` after `HttpFetch,`.
3. The doc comment of `ALL`, `/// The four tools, in the order listed in the interface.`, becomes
   `/// The four container tools, in the order listed in the interface.` `ALL` itself is
   unchanged: it stays the four container tools.
4. In `ToolName::parse`, add `"recall" => Some(ToolName::Recall),` and
   `"memory" => Some(ToolName::Memory),` before `_ => None,`. In `as_str`, add
   `ToolName::Recall => "recall",` and `ToolName::Memory => "memory",` after the `HttpFetch` arm.
5. After `as_str`, still inside `impl ToolName`, add:

```rust

    /// Whether `memoryd` runs this tool rather than a container.
    pub fn is_memory(self) -> bool {
        matches!(self, ToolName::Recall | ToolName::Memory)
    }
```

6. In `enum ToolArgs`, add `Recall(RecallArgs),` and `Memory(MemoryArgs),` after
   `HttpFetch(FetchUrl),`. In `ToolArgs::tool`, add `ToolArgs::Recall(_) => ToolName::Recall,`
   and `ToolArgs::Memory(_) => ToolName::Memory,` after the `HttpFetch` arm. In
   `canonical_json`, after the `HttpFetch` arm, add:

```rust
            ToolArgs::Recall(value) => {
                serde_json::to_string(value).unwrap_or_else(|_| "{}".to_string())
            }
            ToolArgs::Memory(value) => {
                serde_json::to_string(value).unwrap_or_else(|_| "{}".to_string())
            }
```

7. In `parse`, after the `ToolName::HttpFetch => { … }` arm, add:

```rust
        ToolName::Recall => {
            let value: RecallArgs = serde_json::from_str(arguments)
                .map_err(|error| ArgsError::Shape(error.to_string()))?;
            value.check().map_err(ArgsError::Shape)?;
            Ok(ToolArgs::Recall(value))
        }
        ToolName::Memory => {
            let value: MemoryArgs = serde_json::from_str(arguments)
                .map_err(|error| ArgsError::Shape(error.to_string()))?;
            value.check().map_err(ArgsError::Shape)?;
            Ok(ToolArgs::Memory(value))
        }
```

## `grants.rs`, exactly

1. In `struct GrantSet`, after `grants: Vec<LoadedGrant>,` add:

```rust
    /// The built-in grants of the memory tools, which every set from `from_grants` has. They are
    /// not the owner's, so `grants` does not list them; matching sees them (`matchable`).
    builtin: Vec<LoadedGrant>,
```

2. At the end of `from_grants`, the block

```rust
        if problems.is_empty() {
            Ok(GrantSet { grants })
        } else {
            Err(problems)
        }
```

   becomes

```rust
        if !problems.is_empty() {
            return Err(problems);
        }
        // The built-ins join only a set that passed: an owner's grant for a memory tool is a
        // problem above, never a replacement.
        let builtin = crate::builtin::grants().map_err(|problem| vec![problem])?;
        Ok(GrantSet { grants, builtin })
```

3. The doc comment of `grants()`, `/// The grants in id order.`, becomes
   `/// The owner's grants in id order.`, and after `grants()` add:

```rust

    /// Every grant a call is matched against: the owner's, then the built-ins.
    pub fn matchable(&self) -> impl Iterator<Item = &LoadedGrant> {
        self.grants.iter().chain(self.builtin.iter())
    }
```

4. In `check_grant`, just before `// Rule 3: the tool must be one of the four; …`, add:

```rust
    // Rule 2b: ids starting `builtin-` are the built-in grants'.
    if id.starts_with(crate::builtin::RESERVED) {
        push(
            problems,
            file.clone(),
            None,
            "grant ids starting builtin- are reserved",
        );
    }

```

   and under the Rule 3 comment add the line `// The memory tools' grants are built in (M5a spec,
   section 8).`; in its `match`, before `Some(tool) => check_tool_constraints(…)`, add:

```rust
        Some(tool) if tool.is_memory() => {
            push(problems, file.clone(), None, crate::builtin::BUILT_IN)
        }
```

## The other three files, exactly

- `policy/matching.rs`, in `match_grants`: `for loaded in grants.grants() {` becomes
  `for loaded in grants.matchable() {`. In `covers`, just before the `ToolArgs::HttpFetch(target)`
  arm, add:

```rust
        // The memory tools take no constraints: their grants are built in (M5a spec, section 8).
        ToolArgs::Recall(_) | ToolArgs::Memory(_) => Some(None),
```

- `runner.rs`, in `run`, after the arm
  `ToolArgs::HttpFetch(_) => (Vec::new(), Some(decision.hosts().to_vec())),` add
  `ToolArgs::Recall(_) | ToolArgs::Memory(_) => (Vec::new(), None),`.
- `config.rs`, in `Runner::time_limit`, after `crate::args::ToolName::HttpFetch => self.http_fetch_ms,` add:

```rust
            // `memoryd` runs these; they never reach a container.
            crate::args::ToolName::Recall | crate::args::ToolName::Memory => self.read_file_ms,
```

## Steps

- [ ] **1. Copy** the skeleton and the two tests; make the edits, file by file, with
  `cargo check -p brokerd` after each.
- [ ] **2. See it fail.** `cargo test -p brokerd --test memory_tools --test grants`: both fail
  (`builtin::grants` is still a `todo!()`).
- [ ] **3. Fill** `builtin::grants`. `cargo fmt --all`.
- [ ] **4. See it pass.** The same command: 4 and 17 passed. Then `cargo test -p brokerd`: every
  suite passes.
- [ ] **5. Run the gate.** `make gate`: `gate: ok`, 4 more tests than before (about 924).
- [ ] **6. Log and commit.** `git add -A crates/brokerd docs/implementer-log.md Cargo.lock && git commit`

## Done when

- `make gate` prints `gate: ok`.

## Stop and report if

- Any other `brokerd` test fails. The given `policy_property` test matches random grants against
  the four container tools; the built-ins must not change what it sees.