diff --git a/crates/bxcore/src/lib.rs b/crates/bxcore/src/lib.rs index 0e8b24f..c02758d 100644 --- a/crates/bxcore/src/lib.rs +++ b/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, diff --git a/crates/bxcore/src/memory.rs b/crates/bxcore/src/memory.rs index b0cb9e5..2e9d77b 100644 --- a/crates/bxcore/src/memory.rs +++ b/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, +} + /// 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 { diff --git a/crates/bxcore/tests/ask_memory_args.rs b/crates/bxcore/tests/ask_memory_args.rs new file mode 100644 index 0000000..bee4eac --- /dev/null +++ b/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::(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" + ); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 06364f8..0035552 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -6,6 +6,7 @@ reviewer adds findings under "Reviews" once per milestone. | Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model | |---|---|---|---|---|---|---|---| +| M5c2/01-wire-ask-memory | 2026-09-28 | done | 2 | pass | none | Copied the given test `crates/bxcore/tests/ask_memory_args.rs` from `docs/plans/M5c2/files/`. Step 2 (`cargo build --tests --keep-going -p bxcore`) showed exactly the two named failures: E0432 `unresolved import bxcore::AskMemoryArgs` and E0432 for `bxcore::memory::ASK_MEMORY`, `MAX_QUESTION`, `MAX_WORKSPACE_NAME`, `MAX_WORKSPACES`, `OWN_WORKSPACE`; every other target compiled. Made the edits exactly as the task gives: M1 (module doc naming `ask_memory` and the `[honcho]` content rule), M2 (`ASK_MEMORY`), M3 (`MAX_QUESTION` 2000, `MAX_WORKSPACES` 2, `MAX_WORKSPACE_NAME` 512, `OWN_WORKSPACE` "own"), M4 (`AskMemoryArgs { question, workspaces }` with `deny_unknown_fields`, `workspaces` `serde(default)`), M5 (`check` in the fixed order — the question empty then its length in characters, the number of names, then each name empty/form/repeat — and `asked` returning `["own"]` for an empty list), the private `workspace_name`, M6 (`MemoryCall` doc), X1 (`pub use memory::AskMemoryArgs` in lib.rs after the `log` line, leaving the `pub use memory::{...}` block). No `skip_serializing_if` on `AskMemoryArgs`; `grep -c 'skip_serializing_if'` prints 5. `cargo check -p bxcore` clean. `cargo test` passes: 8 in `ask_memory_args`, 6 in `memory_args`, 5 in `memory_wire`. `git diff --stat` lists only lib.rs (1 added) and memory.rs. `make gate` printed `gate: ok` with 1372 tests (1371 in `cargo test --workspace`, plus the 1 ignored `end_to_end` the gate runs separately; the +8 over the 1364 before is exactly the 8 new `ask_memory_args`). No new dependency, so `Cargo.lock` unchanged. | ornith-1.5-35b-a3b | | M5c1/15-memoryd-honcho-began | 2026-09-28 | done | 2 | pass | none | Copied the given tests from `docs/plans/M5c1/files/crates/memoryd/tests/`: the three new `honcho_ledger_began.rs` (5), `honcho_decide_began.rs` (4), `honcho_began_serve.rs` (2) and new versions of `honcho_ledger.rs`, `honcho_ledger_file.rs`, `honcho_decide.rs`, `honcho_turns.rs`, `honcho_naming.rs`, `honcho_serve.rs`, `honcho_mirror.rs`, `honcho_mirror_serve.rs`, `honcho_edits.rs`, `honcho_worker_more.rs`, `honcho_sync.rs` and the support `ledger.rs`, `turns.rs`, `worker.rs` (14 modified, 3 new). Step 2 (`cargo build --tests --keep-going -p memoryd`) showed the 14 named targets not compiling: E0061 (`decide` 5 args, `conclusion` 3), E0432 (`BEFORE_SYNC`), E0559 (`Unconclude` no `peer`), E0599 (no `began`/`begin`/`BeganNotFirst`/`Began`); every other target compiled. Made the edits exactly as the task gives: L1-L13 in `ledger.rs` (`Unconclude.peer`; `Record::Began`; the `BeganNotFirst`/`NotBegun` errors and their two Display lines; the `began`/`records` fields and init, the `first_not_began` check in `open`, the new `began()`, `conclusion` by peer, `check` refusing a second `Began`, `apply` counting records and setting `began`); D1-D5 in `decide.rs` (`Timestamp` import, `BEFORE_SYNC`, the doc, the `began` parameter, the first check on the `User` record's time); T1 (`turns.rs` passing `ledger().began()`); M1 (`mirror.rs` `Unconclude`'s `peer`); W1 (`worker.rs` `conclusion` with `peer`); A1-A2 (`start.rs` imports and `Opened::begin`, writing `Began` once when `began().is_none()`); S1-S2 (`serve.rs` doc and `opened.begin()` after `store.init`, before the sockets). `cargo check -p memoryd` clean after all edits; `cargo test -p memoryd` every suite passes; the other-crates `git diff --stat` is empty. `make gate` printed `gate: ok` with 1345 tests (1344 in `cargo test --workspace`, plus the 1 ignored `end_to_end` the gate runs separately; the +13 over baseline is exactly the 11 in the three new files, 1 more in `honcho_mirror.rs`, 1 more in `honcho_edits.rs`). One `loopd` `pointers` test failed once in a manual full-workspace run under load (`6 passed; 1 failed`) but passes in isolation and in a clean run without my change, and both `make gate` runs printed `gate: ok`: it is a load-dependent flake, not this task (loopd is untouched, the edit is all in memoryd). | ornith-1.5-35b-a3b | | M5c1/14-bxctl-memory-sync | 2026-09-28 | done | 1 | pass | none | Copied the two given tests from `docs/plans/M5c1/files/`: `crates/memoryd/tests/honcho_sync.rs` (from `honcho_sync.rs-task14`) and `crates/bxctl/tests/cli_memory_sync.rs`, and the skeleton `crates/memoryd/src/honcho/report.rs`. Step 2 showed exactly the named failures: `honcho_sync` with 5 (E0432 for `memoryd::honcho::report`, E0432 for `...::ledger::Failure`/`label`, two E0599 for `failures`, one E0599 for the missing `Sync`); `cli_memory_sync` with 6 (all the missing `Sync`). Made X1 (`MemoryCommand::Sync`), B1-B3 (`memory sync` parsed as `Ask::Admin(MemoryCommand::Sync, String::new())`, the module doc), U1 (the usage line), L1-L9 (`Failure { item, time, status, kind }`; `State::Failed { time, status, kind }`; the `fails: Vec` field, kept in `apply` on every `Failed` record; `failures()` oldest first via `filter_map`; renamed private `describe` to public `label`); R1 (`use crate::honcho::report`; `Mirror::report` under the ledger's lock); H1 (`pub mod report;`); S1/S2 (`Sync` answered with `mirror.report()` or `NOT_CONFIGURED`). Copied the skeleton. The task's U1 code block rendered every line with one extra leading space, but the actual `USAGE` literal uses two spaces before a command and six before its explanation (as `cli::usage_explains_the_speaker_flag` enforces), so I matched those. `cargo check -p memoryd` clean after filling the body: `counts()` for the first line, then when `failures()` is non-empty push `"\nfailed, oldest first:"` and each `format!("\n {} {} {}", time.to_rfc3339(), label(&item), kind)` with ` (HTTP {status})` appended when `status` is `Some`. `cargo test` all pass: `honcho_sync` 4, `honcho_ledger` 12, `honcho_ledger_file` 12, `honcho_worker` 12, `cli_memory_sync` 4, every other suite unchanged. `grep -n 'MemoryCommand::Sync'` prints two lines, `git diff --stat -- crates/loopd crates/brokerd crates/gatewayd Cargo.toml Cargo.lock` prints nothing. `make gate` printed `gate: ok` with 1332 tests (1331 in `cargo test --workspace`, plus the 1 ignored `end_to_end` the gate runs separately). No new dependency, so `Cargo.lock` is unchanged. | ornith-1.5-35b-a3b | | M5c1/13-memoryd-honcho-edits | 2026-09-28 | done | 1 | pass | none | Copied the given files from `docs/plans/M5c1/files/`: the test `crates/memoryd/tests/honcho_edits.rs` (from `honcho_edits.rs-task13`), new versions of `crates/memoryd/tests/honcho_worker_more.rs` (from `honcho_worker_more.rs-task13`) and `tests/support/honcho.rs` (from `support/honcho.rs`), and the skeletons `crates/memoryd/src/honcho/edits.rs` and `crates/memoryd/src/honcho/start.rs` (given whole). Step 2 showed exactly the named failure: `honcho_edits` with one E0432 (`unresolved import memoryd::honcho::edits`); every other target compiled; `honcho_worker_more` 10, `honcho_client` 13, `honcho_worker` 12 passed. Made H1 (`pub mod edits;` after `pub mod decide;` in `honcho/mod.rs`), H2 (`pub mod start;` after `pub mod naming;`), C1 (`owner: String` field in `Clients`), C2 (`owner: config.owner.clone()` in `Clients::new`) and C3 (`ensure_memory_session`: ensure workspace, the agent's peer, the owner's, then the `_memory` session with both peers `observe_me(false) observe_others(false)`); copied the two skeletons. `cargo check -p memoryd` gave 22 warnings (six unused imports, sixteen unused variables, exactly the task named). Filled its five bodies in the order of the file, deleting each instruction comment and keeping the doc comments, each from its comment and `cargo check` after: `memory_session` (`format!("{}{MEMORY_SUFFIX}", session.as_str())`); `conclude` (`memory_session`, `ensure_memory_session`, `ConclusionCreate::new(neutralise(text), peer, peer).with_session_id(name)` in `ConclusionBatchCreate::new(vec![..])`, `create_conclusions` -> the ids); `unconclude` (`Some(id)` -> `delete_conclusion` then `vec![id]`, `None` -> `Vec::new()`); `fleet_batch` (`Map` metadata `agent`/`boxmaker_session`/`edit`, `parts(neutralise(text))` each `MessageCreate::new(part, agent)` with the metadata); `fleet` (`HonchoSession { name: session id, speakers: [] }`, `ensure_session(Fleet, ...)`, `add_messages` -> the ids). `cargo check` clean. Made W1 (imports: drop `Config`/`Timeouts`/`HonchoConfig`/`mirror`/`state`/`LEDGER`/`read_log`/`StartError`, add `edits`, re-export `start::{Opened, open, spawn}`); W2 (`Job` enum `Turn`/`Conclude`/`Unconclude`/`Fleet`, `Next` holds `item`/`workspace`/`job`); W3 (`label` naming a turn or a part); W4 and W5 (the two log lines use `label(&next.item)`); W6 (`next` matching every `(Item, Action)` pair into a `Job`, `send` dispatching and `send_turn` split out); W7 (the start code removed, it moves to `start.rs`). `cargo check -p memoryd` clean, `worker.rs` 486 lines. Step 6 all pass: `honcho_edits` 10, `honcho_worker_more` 10, `honcho_worker` 12, `honcho_client` 13, `honcho_keys` 3, `honcho_mirror_serve` 6, `honcho_serve` 6. Step 7: `fn open`/`fn spawn` both in `start.rs`, no `Action::Retract` in `worker.rs`, the other-crates `git diff --stat` empty. `cargo fmt --all`; `make gate` printed `gate: ok` with 1324 tests (1313 in `cargo test --workspace`, plus the 10 in `honcho_edits.rs` and the 1 ignored `end_to_end` the gate runs separately). No new dependency, so `Cargo.lock` is unchanged. | ornith-1.5-35b-a3b |