M5c-2 plan: task 01, ask_memory's arguments

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

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

@@ -0,0 +1,351 @@
+# M5c-2 task 01: `ask_memory`'s arguments and their shape checks
+
+**Branch:** `m5c2`. Check it: `git branch --show-current` prints `m5c2` and `git status --short`
+is empty; otherwise stop. Never switch branch.
+**Commit subject:** `bxcore: ask_memory's arguments and their shape checks`
+**Log row:** the Task cell is `M5c2/01-wire-ask-memory`.
+
+## Goal
+
+M5c-2 adds a core tool, `ask_memory`, that asks Honcho, a memory server shared by a fleet of
+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
+tool), task 07 (`memoryd` answers it).
+
+```json
+{"question":"Who wrote the launch note?","workspaces":["fleet","boxmaker1"]}
+{"question":"What does the owner prefer?"}
+```
+
+- **`question`**: required; 1 to 2,000 characters, counted as characters (`chars().count()`),
+  never bytes; a question of only spaces or newlines is empty.
+- **`workspaces`**: optional; at most two names, none empty, none repeated, each 1 to 100
+  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.
+- 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.)
+
+Each refusal is one sentence that the model is shown after `invalid argument shape: `, so each
+says what to do instead. They are exact; the given tests compare them.
+
+## Every place that changes
+
+This is the complete list.
+
+| Place | What happens there |
+|---|---|
+| `crates/bxcore/src/memory.rs` | the module's doc comment; `ASK_MEMORY`; four constants; `AskMemoryArgs`; its `check` and `asked`; the private `workspace_name`; `MemoryCall`'s doc comment (edits M1 to M6) |
+| `crates/bxcore/src/lib.rs` | one added line exports `AskMemoryArgs` (edit X1) |
+| `crates/bxcore/tests/ask_memory_args.rs` | new, given |
+| everything else | **no change**: not `brokerd` (task 02), not `loopd`, not `memoryd`, no fixture, no `Cargo.toml` |
+
+## Files
+
+- Copy, from `docs/plans/M5c2/files/` (the same path under it): the new test
+  `crates/bxcore/tests/ask_memory_args.rs`.
+- Modify: `crates/bxcore/src/memory.rs`, `crates/bxcore/src/lib.rs`, `docs/implementer-log.md`.
+- Do not touch: any other file.
+
+## Facts you need
+
+- **No wire fixture.** `brokerd` hands a decided memory call to `memoryd` in a `MemoryCall`,
+  whose `request.arguments` is the model's argument text as a string. No message changes, so no
+  fixture under `crates/bxcore/tests/fixtures/` is added or changed. The arguments' own tests
+  hold their JSON inline, as `memory_args.rs` does for `recall` and `memory`.
+- **The default.** `#[serde(default)]` on `workspaces: Vec<String>` reads a missing field as an
+  empty list. An empty list is written out as `"workspaces":[]`: every field is written, as
+  `RecallArgs` writes its default `k`, so that `{"question":"q"}` and
+  `{"question":"q","workspaces":[]}` look the same once `brokerd` writes them out again. So:
+  no `skip_serializing_if`. `asked()` turns the empty list into `["own"]`.
+- **The order of the checks** is fixed, and a given test pins it: the question (empty, then too
+  long), then the number of names, then each name in turn (empty, then its form, then whether an
+  earlier name is the same). A call with several faults is shown the first one.
+- **Names are compared as written**: `fleet` and `Fleet` are two names (a Honcho name is
+  case-sensitive).
+- **Why 100 characters.** `bxcore` has no rule for Honcho's names (`memoryd`'s config has its own,
+  which `bxcore` cannot use: `bxcore` depends on no other crate). The model names a workspace by
+  a short word (`own`, `untrusted`, `fleet`) or an agent's name; 100 characters is plenty and
+  keeps the name short enough to repeat in a refusal.
+- **Characters and bytes in a name.** A name's form allows ASCII only, so its length in bytes
+  (`len()`) is its length in characters; a name with any other character fails the form check
+  whatever its length.
+- **The long strings.** Four `return Err(...)` sentences are longer than a line. `rustfmt` leaves a
+  string literal as it is, and the gate accepts the long line. Do not split a sentence with `\`
+  or `concat!`.
+- The calls and attributes the edits use. This table is complete; `serde` and `serde_json` are
+  already dependencies of `bxcore`, and no crate's source (the Cargo registry,
+  `~/.cargo/registry`) is needed or allowed.
+
+  | Item | What it does |
+  |---|---|
+  | `#[serde(deny_unknown_fields)]` | on `AskMemoryArgs` (M4): an unknown key is refused |
+  | `#[serde(default)]` | on `workspaces` (M4): missing reads as `Vec::new()` |
+  | `str::trim(&self) -> &str`, `str::is_empty` | the empty-question check |
+  | `str::chars(&self).count() -> usize` | the question's length in characters |
+  | `Vec::len`, `slice::iter`, `Iterator::enumerate`, `Iterator::take(i)`, `Iterator::any` | the count and the repeat check: `take(i)` is the names before the `i`th |
+  | `str::bytes(&self)`, `u8::is_ascii_alphanumeric(&self) -> bool`, `Iterator::all` | a name's form |
+  | `String::as_str` | `asked` borrows each name |
+
+## 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 fields in the order shown.
+
+### `crates/bxcore/src/memory.rs`
+
+**M1.** The module's doc comment, the lines
+
+```rust
+//! Memory: the arguments of `recall` and `memory`, their shape checks, and the messages `memoryd`
+//! exchanges with `loopd`, `brokerd` and `bxctl` (M5a spec, sections 5 and 7). Content rules (an
+//! entry matching the file, a file's limit, the check on characters) are `memoryd`'s.
+```
+
+become
+
+```rust
+//! Memory: the arguments of `recall`, `memory` and `ask_memory`, their shape checks, and the
+//! messages `memoryd` exchanges with `loopd`, `brokerd` and `bxctl` (M5a spec, sections 5 and 7;
+//! M5c spec, section 9). Content rules (an entry matching the file, a file's limit, the check on
+//! characters, which workspaces `[honcho]` has) are `memoryd`'s.
+```
+
+**M2.** The line
+
+```rust
+pub const MEMORY: &str = "memory";
+```
+
+becomes
+
+```rust
+pub const MEMORY: &str = "memory";
+pub const ASK_MEMORY: &str = "ask_memory";
+```
+
+**M3.** The lines
+
+```rust
+/// The line that separates entries in `memory.md`, `user.md` and `notes.md`.
+pub const SEPARATOR: &str = "§";
+```
+
+become
+
+```rust
+/// The line that separates entries in `memory.md`, `user.md` and `notes.md`.
+pub const SEPARATOR: &str = "§";
+/// The most characters an `ask_memory` question may have.
+pub const MAX_QUESTION: usize = 2_000;
+/// The most workspaces one `ask_memory` may name: both are asked at once, inside `brokerd`'s wait.
+pub const MAX_WORKSPACES: usize = 2;
+/// The most characters of a workspace name in `ask_memory`.
+pub const MAX_WORKSPACE_NAME: usize = 100;
+/// The workspace `ask_memory` asks when it names none: the agent's own trusted one.
+pub const OWN_WORKSPACE: &str = "own";
+```
+
+**M4.** The lines
+
+```rust
+fn default_k() -> u64 {
+    DEFAULT_K
+}
+```
+
+become
+
+```rust
+fn default_k() -> u64 {
+    DEFAULT_K
+}
+
+/// The arguments for `ask_memory` (M5c spec, section 9). An empty `workspaces` asks `own`.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
+pub struct AskMemoryArgs {
+    pub question: String,
+    #[serde(default)]
+    pub workspaces: Vec<String>,
+}
+```
+
+**M5.** The lines (its spaces are checked: every sentence in the new code has single spaces, and
+is compared letter for letter by the given tests)
+
+```rust
+impl MemoryArgs {
+    /// The shape rules of `memory` (M5a spec, section 5): every text is a well-formed entry, and
+```
+
+become
+
+```rust
+impl AskMemoryArgs {
+    /// The shape rules of `ask_memory` (M5c spec, section 9), in this order: the question, the
+    /// number of names, then each name in turn. Only a name's form is checked here. Each sentence
+    /// reaches the model, so it says what to do instead.
+    pub fn check(&self) -> Result<(), String> {
+        if self.question.trim().is_empty() {
+            return Err("question is empty; write the question to ask".to_string());
+        }
+        let chars = self.question.chars().count();
+        if chars > MAX_QUESTION {
+            return Err(format!(
+                "question has {chars} characters, over the limit of {MAX_QUESTION}; ask a shorter question"
+            ));
+        }
+        let names = self.workspaces.len();
+        if names > MAX_WORKSPACES {
+            return Err(format!(
+                "workspaces names {names}, over the limit of {MAX_WORKSPACES}; ask about the others in another call"
+            ));
+        }
+        for (i, name) in self.workspaces.iter().enumerate() {
+            if name.is_empty() {
+                return Err(
+                    "a workspace name is empty; name a workspace, or leave workspaces out to ask own"
+                        .to_string(),
+                );
+            }
+            if !workspace_name(name) {
+                return Err(format!(
+                    "a workspace name has 1 to {MAX_WORKSPACE_NAME} letters, digits, _ or -; use own, untrusted, fleet or another agent's name"
+                ));
+            }
+            if self.workspaces.iter().take(i).any(|w| w == name) {
+                return Err(format!(
+                    "workspaces names {name} twice; name each workspace once"
+                ));
+            }
+        }
+        Ok(())
+    }
+
+    /// The workspaces to ask: those named, in order, or `own` when none is.
+    pub fn asked(&self) -> Vec<&str> {
+        if self.workspaces.is_empty() {
+            vec![OWN_WORKSPACE]
+        } else {
+            self.workspaces.iter().map(String::as_str).collect()
+        }
+    }
+}
+
+/// A workspace name's form: at most `MAX_WORKSPACE_NAME` characters, each an ASCII letter, a
+/// digit, `_` or `-`, as Honcho's names are. `check` refuses an empty name first.
+fn workspace_name(name: &str) -> bool {
+    name.len() <= MAX_WORKSPACE_NAME
+        && name
+            .bytes()
+            .all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'-')
+}
+
+impl MemoryArgs {
+    /// The shape rules of `memory` (M5a spec, section 5): every text is a well-formed entry, and
+```
+
+**M6.** `MemoryCall`'s doc comment, the lines
+
+```rust
+/// `brokerd` → `memoryd`: a decided `recall` or `memory` call, with the session's state when it
+/// was decided.
+```
+
+become
+
+```rust
+/// `brokerd` → `memoryd`: a decided `recall`, `memory` or `ask_memory` call, with the session's
+/// state when it was decided.
+```
+
+### `crates/bxcore/src/lib.rs`
+
+**X1.** Add one line. The line
+
+```rust
+pub use log::{LogRecord, ToolCall, Usage};
+```
+
+becomes
+
+```rust
+pub use log::{LogRecord, ToolCall, Usage};
+pub use memory::AskMemoryArgs;
+```
+
+Do not change the `pub use memory::{ ... };` block under it.
+
+## Steps
+
+- [ ] **1. Copy the given test.** From the repository root, one command:
+
+  ```sh
+  cp docs/plans/M5c2/files/crates/bxcore/tests/ask_memory_args.rs crates/bxcore/tests/
+  ```
+
+  Then `git status --short` lists one new file, `crates/bxcore/tests/ask_memory_args.rs`.
+- [ ] **2. See it fail.** `cargo build --tests --keep-going -p bxcore`, with a shell timeout of 600000 ms: one
+  test target, `ask_memory_args`, does not compile, with two errors, both `E0432`:
+  ``unresolved import `bxcore::AskMemoryArgs` `` and ``unresolved imports
+  `bxcore::memory::ASK_MEMORY`, `bxcore::memory::MAX_QUESTION`,
+  `bxcore::memory::MAX_WORKSPACE_NAME`, `bxcore::memory::MAX_WORKSPACES`,
+  `bxcore::memory::OWN_WORKSPACE` ``. Every other target compiles.
+- [ ] **3. The edits.** Make edits M1 to M6 and X1, in order. `cargo check -p bxcore` prints no
+  error and no warning.
+- [ ] **4. The tests.** `cargo test -p bxcore --test ask_memory_args --test memory_args
+  --test memory_wire`: every test passes, 8 in `ask_memory_args` and the others as before (6 and
+  5).
+- [ ] **5. Check.** Two commands:
+  - `git diff --stat` lists exactly `crates/bxcore/src/lib.rs` (1 line added) and
+    `crates/bxcore/src/memory.rs`.
+  - `grep -c 'skip_serializing_if' crates/bxcore/src/memory.rs` prints `5`, as before this task:
+    the five on `ConsolidateEntry`; none on `AskMemoryArgs`.
+- [ ] **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: **1372** after this
+  task, 1364 before it (1363 passed in `cargo test --workspace`, plus 1), and the 8 new ones are
+  the tests of `ask_memory_args.rs`. 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/01-wire-ask-memory`. `git status --short` lists `crates/bxcore/src/lib.rs`,
+  `crates/bxcore/src/memory.rs` and your log as modified, and the test as new. Stage with
+  `git add crates/bxcore 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 1372 tests.
+- `git diff HEAD~1 --stat` lists only `crates/bxcore/src/lib.rs`, `crates/bxcore/src/memory.rs`,
+  `crates/bxcore/tests/ask_memory_args.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 when its turn comes, or is there more
+  than once.
+- Step 2 shows a different result: another target failing, or other errors.
+- `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: another crate, a fixture, a `skip_serializing_if`,
+  a check against `[honcho]`. It does not; stop and say why you think so.
+- A test outside `ask_memory_args.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.
+- The gate passes with a count other than 1372.
+- 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 (23 tried, all
+caught): the question checked without `trim`; its length in bytes; its limit moved by one
+either way; the limit on names moved by one, or dropped; the repeat check dropped, comparing a
+name with itself, or ignoring case; the empty-name sentence skipped; `.` allowed in a name, `-`
+refused, non-ASCII letters allowed; the 100-character limit moved by one, or dropped; `asked`
+empty for no names, or out of order; unknown keys accepted; `workspaces` required; the empty list
+left out when written; the names checked before their number; the tool's name; any sentence's
+words.

docs/plans/M5c2/README.md

@@ -89,6 +89,15 @@ trusted workspace within its limit; with rift down, `recall` answers from the lo
 `unavailable` lines. The baseline is measured again against the 3,000-token budget
 (`make verify-device`).
 
+## Counts, as tasks are written
+
+The gate count is the passed total of `cargo test --workspace` plus one, for the ignored
+`end_to_end` test the gate runs on its own. Before task 01 it is 1364 (talos, rustc 1.98.1).
+
+| Task | Gate count after | Mutations tried, caught |
+|---|---|---|
+| 01 | 1372 | 23, 23 |
+
 ## Before hand-over
 
 Every task is worked through `docs/plans/CHECKLIST.md` with a reference in a scratch worktree

docs/plans/M5c2/files/crates/bxcore/tests/ask_memory_args.rs

@@ -0,0 +1,181 @@
+//! The arguments of `ask_memory`: decoding, the default workspace, unknown keys refused, and the
+//! shape checks with the sentences the model is shown (M5c spec, section 9). Do not edit.
+
+use bxcore::AskMemoryArgs;
+use bxcore::memory::{ASK_MEMORY, MAX_QUESTION, MAX_WORKSPACE_NAME, MAX_WORKSPACES, OWN_WORKSPACE};
+
+fn args(json: &str) -> AskMemoryArgs {
+    serde_json::from_str(json).unwrap_or_else(|e| panic!("{json}: {e}"))
+}
+
+fn ask(question: &str, workspaces: &[&str]) -> AskMemoryArgs {
+    AskMemoryArgs {
+        question: question.to_string(),
+        workspaces: workspaces.iter().map(|w| w.to_string()).collect(),
+    }
+}
+
+fn refused(value: &AskMemoryArgs) -> String {
+    value
+        .check()
+        .expect_err(&format!("{value:?} must be refused"))
+}
+
+#[test]
+fn the_name_and_the_limits() {
+    assert_eq!(ASK_MEMORY, "ask_memory");
+    assert_eq!(MAX_QUESTION, 2_000);
+    assert_eq!(MAX_WORKSPACES, 2);
+    assert_eq!(MAX_WORKSPACE_NAME, 100);
+    assert_eq!(OWN_WORKSPACE, "own");
+}
+
+/// No `workspaces`, or an empty list, asks the agent's own workspace; both are written out with
+/// every field, so two spellings of one call read the same in the audit log.
+#[test]
+fn without_workspaces_it_asks_own() {
+    for json in [
+        r#"{"question":"What does the owner prefer?"}"#,
+        r#"{"question":"What does the owner prefer?","workspaces":[]}"#,
+    ] {
+        let value = args(json);
+        assert_eq!(value, ask("What does the owner prefer?", &[]), "{json}");
+        assert_eq!(value.check(), Ok(()), "{json}");
+        assert_eq!(value.asked(), ["own"], "{json}");
+        assert_eq!(
+            serde_json::to_string(&value).unwrap(),
+            r#"{"question":"What does the owner prefer?","workspaces":[]}"#
+        );
+    }
+}
+
+#[test]
+fn named_workspaces_round_trip_exactly_and_are_asked_in_order() {
+    let json = r#"{"question":"Who wrote the launch note?","workspaces":["fleet","boxmaker1"]}"#;
+    let value = args(json);
+    assert_eq!(
+        value,
+        ask("Who wrote the launch note?", &["fleet", "boxmaker1"])
+    );
+    assert_eq!(value.check(), Ok(()));
+    assert_eq!(value.asked(), ["fleet", "boxmaker1"]);
+    assert_eq!(serde_json::to_string(&value).unwrap(), json);
+    let one = args(r#"{"question":"q","workspaces":["untrusted"]}"#);
+    assert_eq!(one.check(), Ok(()));
+    assert_eq!(one.asked(), ["untrusted"]);
+}
+
+/// `reasoning` was in an earlier draft of the spec and is not a field.
+#[test]
+fn unknown_keys_missing_questions_and_wrong_types_are_refused() {
+    for json in [
+        r#"{"question":"q","reasoning":"low"}"#,
+        r#"{"question":"q","workspace":"fleet"}"#,
+        r#"{"question":"q","k":3}"#,
+        r#"{"workspaces":["own"]}"#,
+        r#"{"question":"q","workspaces":"fleet"}"#,
+        r#"{"question":"q","workspaces":[1]}"#,
+        r#"{"question":null}"#,
+    ] {
+        assert!(
+            serde_json::from_str::<AskMemoryArgs>(json).is_err(),
+            "{json}"
+        );
+    }
+}
+
+/// The limit is in characters, not bytes: each `é` is two bytes, each `日` three.
+#[test]
+fn the_question_has_1_to_2000_characters() {
+    assert_eq!(ask(&"é".repeat(2_000), &[]).check(), Ok(()));
+    assert_eq!(ask(&"日".repeat(2_000), &["own"]).check(), Ok(()));
+    assert_eq!(ask("x", &[]).check(), Ok(()));
+    assert_eq!(
+        refused(&ask(&"é".repeat(2_001), &[])),
+        "question has 2001 characters, over the limit of 2000; ask a shorter question"
+    );
+    assert_eq!(
+        refused(&ask(&"x".repeat(5_000), &["own"])),
+        "question has 5000 characters, over the limit of 2000; ask a shorter question"
+    );
+    for empty in ["", " ", " \n\t "] {
+        assert_eq!(
+            refused(&ask(empty, &[])),
+            "question is empty; write the question to ask",
+            "{empty:?}"
+        );
+    }
+}
+
+#[test]
+fn at_most_two_workspaces_each_named_once() {
+    assert_eq!(ask("q", &["own", "fleet"]).check(), Ok(()));
+    assert_eq!(
+        refused(&ask("q", &["own", "untrusted", "fleet"])),
+        "workspaces names 3, over the limit of 2; ask about the others in another call"
+    );
+    assert_eq!(
+        refused(&ask("q", &["fleet", "fleet"])),
+        "workspaces names fleet twice; name each workspace once"
+    );
+    assert_eq!(
+        refused(&ask("q", &["own", "own"])),
+        "workspaces names own twice; name each workspace once"
+    );
+    // Names are compared as written: case is part of a Honcho name.
+    assert_eq!(ask("q", &["fleet", "Fleet"]).check(), Ok(()));
+}
+
+/// Only the form is checked here; whether a name is one of `[honcho]`'s workspaces is `memoryd`'s
+/// check.
+#[test]
+fn a_workspace_name_has_1_to_100_letters_digits_underscores_or_hyphens() {
+    let form = "a workspace name has 1 to 100 letters, digits, _ or -; use own, untrusted, fleet or \
+                another agent's name";
+    for good in [
+        "own",
+        "boxmaker1",
+        "Boxmaker_1-trusted",
+        "x",
+        "-_-",
+        &"a".repeat(100),
+    ] {
+        assert_eq!(ask("q", &[good]).check(), Ok(()), "{good}");
+    }
+    for bad in [
+        " ",
+        "own ",
+        " own",
+        "a.b",
+        "a/b",
+        "a b",
+        "é",
+        "own\n",
+        "a:b",
+        &"a".repeat(101),
+    ] {
+        assert_eq!(refused(&ask("q", &[bad])), form, "{bad:?}");
+        assert_eq!(refused(&ask("q", &["own", bad])), form, "second: {bad:?}");
+    }
+    let empty = "a workspace name is empty; name a workspace, or leave workspaces out to ask own";
+    assert_eq!(refused(&ask("q", &[""])), empty);
+    assert_eq!(refused(&ask("q", &["fleet", ""])), empty);
+}
+
+/// The checks run in a fixed order, so a call with several faults is shown the first: the
+/// question, then the number of names, then each name in turn.
+#[test]
+fn the_first_fault_is_the_one_reported() {
+    assert_eq!(
+        refused(&ask("", &["a", "b", "c"])),
+        "question is empty; write the question to ask"
+    );
+    assert_eq!(
+        refused(&ask("q", &["", "", ""])),
+        "workspaces names 3, over the limit of 2; ask about the others in another call"
+    );
+    assert_eq!(
+        refused(&ask("q", &["", "a.b"])),
+        "a workspace name is empty; name a workspace, or leave workspaces out to ask own"
+    );
+}