bxcore: ask_memory's arguments and their shape checks

Co-Authored-By: ornith-1.5-35b-a3b <agent@wntrmute.dev>

crates/bxcore/src/lib.rs

@@ -36,6 +36,7 @@ pub use grant::{Constraints, Grant, Mode};
 pub use hash::{HashError, Sha256, sha256};
 pub use ids::{CallId, Epoch, Hash32, SessionId, Timestamp, ValueError};
 pub use log::{LogRecord, ToolCall, Usage};
+pub use memory::AskMemoryArgs;
 pub use memory::{
     ConsolidateEntry, EntryAction, MemoryAdmin, MemoryAdminReply, MemoryArgs, MemoryBlockReply,
     MemoryCall, MemoryCommand, MemoryFile, RecallArgs, Reindexed, SessionState, SessionStateReply,

crates/bxcore/src/memory.rs

@@ -1,6 +1,7 @@
-//! 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.
+//! 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.
 
 use serde::{Deserialize, Serialize};
 
@@ -11,6 +12,7 @@ use crate::wire::ToolRequest;
 /// The tool names.
 pub const RECALL: &str = "recall";
 pub const MEMORY: &str = "memory";
+pub const ASK_MEMORY: &str = "ask_memory";
 
 /// The most characters a `recall` query may have.
 pub const MAX_QUERY: usize = 500;
@@ -23,6 +25,16 @@ pub const MAX_NOTE: usize = 1_000;
 pub const MAX_ENTRY: usize = 2_200;
 /// 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`. It matches `memoryd`'s rule for
+/// Honcho's names (`honcho::config::MAX_NAME`), so every agent `[[honcho.read]]` allows can be
+/// asked.
+pub const MAX_WORKSPACE_NAME: usize = 512;
+/// The workspace `ask_memory` asks when it names none: the agent's own trusted one.
+pub const OWN_WORKSPACE: &str = "own";
 
 /// The arguments for `recall`.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
@@ -37,6 +49,15 @@ 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>,
+}
+
 /// Which identity file an edit is for.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
@@ -187,6 +208,66 @@ impl RecallArgs {
     }
 }
 
+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
     /// every `consolidate` line has one action and the list is not empty.
@@ -254,8 +335,8 @@ pub struct SessionStateReply {
     pub untrusted: bool,
 }
 
-/// `brokerd` → `memoryd`: a decided `recall` or `memory` call, with the session's state when it
-/// was decided.
+/// `brokerd` → `memoryd`: a decided `recall`, `memory` or `ask_memory` call, with the session's
+/// state when it was decided.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]
 pub struct MemoryCall {

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, 512);
+    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_512_letters_digits_underscores_or_hyphens() {
+    let form = "a workspace name has 1 to 512 letters, digits, _ or -; use own, untrusted, fleet or \
+                another agent's name";
+    for good in [
+        "own",
+        "boxmaker1",
+        "Boxmaker_1-trusted",
+        "x",
+        "-_-",
+        &"a".repeat(512),
+    ] {
+        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(513),
+    ] {
+        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"
+    );
+}