From 18bafbd2a08b1ce0d74bd9a056e10f47e530dc5c Mon Sep 17 00:00:00 2001 From: "K. Isom" Date: Tue, 29 Sep 2026 09:36:43 -0700 Subject: [PATCH] memoryd: recall asks Honcho after the local index, outside the state lock Co-Authored-By: ornith-1.5-35b-a3b --- crates/memoryd/src/honcho/search.rs | 16 ++ crates/memoryd/src/serve.rs | 57 +++-- crates/memoryd/tests/honcho_recall_serve.rs | 241 ++++++++++++++++++++ docs/implementer-log.md | 1 + 4 files changed, 295 insertions(+), 20 deletions(-) create mode 100644 crates/memoryd/tests/honcho_recall_serve.rs diff --git a/crates/memoryd/src/honcho/search.rs b/crates/memoryd/src/honcho/search.rs index d04c2b8..f25cb55 100644 --- a/crates/memoryd/src/honcho/search.rs +++ b/crates/memoryd/src/honcho/search.rs @@ -24,6 +24,22 @@ pub const SOURCE_WAIT: Duration = Duration::from_secs(5); /// The line in place of Honcho's items when the query holds a known-prefix secret. pub const SECRET: &str = "honcho: not asked; the query looks like it holds a secret"; +/// What `recall` reads in Honcho: the sources, the owner's peer, and each source's limit +/// (`SOURCE_WAIT` in `memoryd`). +pub struct Reading { + pub readers: Arc, + pub owner: String, + pub wait: Duration, +} + +impl Reading { + /// `recall`'s answer from the local index's items and Honcho's: `search`, then `answer`. + pub fn recall(&self, local: Local, args: &RecallArgs, log: &Log) -> Recalled { + let searched = search(&self.readers, &self.owner, args, self.wait, log); + answer(local, &args.query, args.k, searched) + } +} + /// What the Honcho part found. #[derive(Debug, Clone, PartialEq, Eq)] pub enum Searched { diff --git a/crates/memoryd/src/serve.rs b/crates/memoryd/src/serve.rs index 7d5d691..1aae69c 100644 --- a/crates/memoryd/src/serve.rs +++ b/crates/memoryd/src/serve.rs @@ -19,6 +19,7 @@ use crate::files::Store; use crate::honcho::ledger::{self, LedgerError}; use crate::honcho::mirror::Mirror; use crate::honcho::report; +use crate::honcho::search; use crate::honcho::state; use crate::honcho::turns::TurnSync; use crate::honcho::worker::{self, Wake}; @@ -47,6 +48,8 @@ pub struct Memoryd { wake: Arc, /// With `[honcho]`: decides memory writes into the ledger, under the ledger's own lock. mirror: Option, + /// With `[honcho]`: what `recall` reads beside the local index, outside `state`. + reading: Option, log: Log, } @@ -96,9 +99,12 @@ enum Which { /// under `memory/` (and, the first time, write when sync began), open the index and catch it up, /// bind the three sockets, and start the Honcho worker. The error names what failed. pub fn start(config: Config, log: Log) -> Result { - let opened = match config.honcho_config()? { - Some(honcho) => Some(worker::open(&config, &honcho, &log)?), - None => None, + let (opened, owner) = match config.honcho_config()? { + Some(honcho) => ( + Some(worker::open(&config, &honcho, &log)?), + honcho.owner.clone(), + ), + None => (None, String::new()), }; let store = Store::new(&config.memory_dir()); store.init(&config.memory.soul_starter)?; @@ -119,14 +125,19 @@ pub fn start(config: Config, log: Log) -> Result { (admin_listener, Which::Admin), ]; let wake = Arc::new(Wake::default()); - let (honcho, mirror) = match opened { + let (honcho, mirror, reading) = match opened { Some(opened) => { let sync = Arc::clone(&opened.sync); let mirror = opened.mirror(); + let reading = search::Reading { + readers: Arc::clone(&opened.readers), + owner, + wait: search::SOURCE_WAIT, + }; worker::spawn(opened, &config, Arc::clone(&wake), Arc::clone(&log))?; - (Some(sync), Some(mirror)) + (Some(sync), Some(mirror), Some(reading)) } - None => (None, None), + None => (None, None, None), }; Ok(Running { memoryd: Arc::new(Memoryd { @@ -136,6 +147,7 @@ pub fn start(config: Config, log: Log) -> Result { honcho, wake, mirror, + reading, log, }), listeners, @@ -305,13 +317,13 @@ impl Memoryd { Message::Ok(Empty {}) } - /// A decided call from `brokerd`: `recall` or `memory`, under the one lock. `Failed` carries the - /// sentence for the model. + /// A decided call from `brokerd`: `recall` or `memory`. `memory` runs under the one lock; + /// `recall` takes it for the local index only (`recall`). `Failed` carries the sentence for + /// the model. fn call(&self, call: MemoryCall) -> ToolResponse { - let state = self.lock(); let result = match call.request.tool.as_str() { - bxcore::memory::RECALL => self.recall(&state, &call), - bxcore::memory::MEMORY => self.memory(&state, &call), + bxcore::memory::RECALL => self.recall(&call), + bxcore::memory::MEMORY => self.memory(&self.lock(), &call), _ => { return ToolResponse::Failed { message: "memoryd runs only recall and memory".to_string(), @@ -329,19 +341,24 @@ impl Memoryd { } } - /// `recall`: search, labelled by what it found. - fn recall(&self, state: &State, call: &MemoryCall) -> Result { + /// `recall`: search, labelled by what it found. Without `[honcho]`, the local index under the + /// lock, as before. With it, the local index under the lock, then, with the lock released, + /// Honcho's sources (M5c spec, section 8): a source that hangs holds up no other request. + fn recall(&self, call: &MemoryCall) -> Result { let args: RecallArgs = match serde_json::from_str(&call.request.arguments) { Ok(args) => args, Err(e) => return Err(format!("recall's arguments: {e}")), }; - recall::recall( - &state.index, - &self.store, - &args, - call.request.session.as_str(), - Timestamp::now(), - ) + let session = call.request.session.as_str(); + let Some(reading) = &self.reading else { + let state = self.lock(); + return recall::recall(&state.index, &self.store, &args, session, Timestamp::now()); + }; + let local = { + let state = self.lock(); + recall::local(&state.index, &self.store, &args, session, Timestamp::now())? + }; + Ok(reading.recall(local, &args, &self.log)) } /// `memory`: refused, staged for review, or applied and journalled; then what it appended to diff --git a/crates/memoryd/tests/honcho_recall_serve.rs b/crates/memoryd/tests/honcho_recall_serve.rs new file mode 100644 index 0000000..b28bc25 --- /dev/null +++ b/crates/memoryd/tests/honcho_recall_serve.rs @@ -0,0 +1,241 @@ +//! `memoryd` answering `recall` with `[honcho]` (M5c spec, section 8), through its `brokerd` +//! socket, against a fake Honcho: the local index's hits, then the read workspace's and the +//! fleet's, the answer untrusted when a fleet hit is shown (T58); a hanging source holds up no +//! memory call, because the state lock is released before Honcho is asked; without `[honcho]`, +//! `recall` answers as before. Every wait has a deadline. Do not edit. + +#[path = "support/honcho.rs"] +mod honcho; +#[path = "support/logs.rs"] +mod logs; +#[path = "support/tmp.rs"] +mod tmp; + +use std::os::unix::net::UnixStream; +use std::path::Path; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; + +use bxcore::{ + CallId, DataClass, Envelope, MemoryCall, Message, SessionId, ToolRequest, ToolResponse, + read_frame, write_frame, +}; +use honcho::{FakeHoncho, Mode}; +use logs::{user, write_log}; +use memoryd::config::Config; +use memoryd::serve; +use tmp::TempDir; + +/// `[honcho]` for `url`, reading `boxmaker1`'s workspace, labelled trusted. +fn section(url: &str) -> String { + format!( + "[honcho]\nurl = \"{url}\"\nagent = \"boxmaker0\"\nowner = \"kyle\"\ntrusted = \"boxmaker0-trusted\"\nuntrusted = \"boxmaker0-untrusted\"\nfleet = \"fleet\"\n[[honcho.read]]\nagent = \"boxmaker1\"\nworkspace = \"boxmaker1-trusted\"\ntrust = \"trusted\"\n" + ) +} + +struct Home { + config: Config, + /// Removed when the test ends; declared last, so it outlives what writes into it (T44). + _dir: TempDir, +} + +impl Home { + /// A home whose index holds one line the owner said, `tea time`; with `[honcho]` at `url` + /// when there is one. + fn new(tag: &str, url: Option<&str>) -> Home { + let dir = TempDir::new(tag); + let soul = dir.write("etc/soul.md", "You are Boxmaker.\n"); + write_log(dir.path(), "mm-a", &[user(1, "tea time")]); + let text = format!( + "[paths]\nhome = \"{}\"\n[memory]\nsoul_starter = \"{}\"\n{}", + dir.path().display(), + soul.display(), + url.map(section).unwrap_or_default() + ); + Home { + config: Config::parse(&text).unwrap(), + _dir: dir, + } + } + + /// `serve::start`, run on a thread; its log lines are collected. + fn serve(&self) -> Arc>> { + let lines = Arc::new(Mutex::new(Vec::new())); + let sink = Arc::clone(&lines); + let log: serve::Log = Arc::new(move |l: &str| sink.lock().unwrap().push(l.to_string())); + let running = serve::start(self.config.clone(), log).unwrap(); + std::thread::spawn(move || running.run()); + lines + } +} + +/// One call on `brokerd`'s socket and its answer, within 20 s; how long it took. +fn call(socket: &Path, tool: &str, arguments: &str) -> (ToolResponse, Duration) { + let started = Instant::now(); + let mut stream = UnixStream::connect(socket).unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(20))) + .unwrap(); + let env = Envelope { + v: 1, + id: 3, + r#final: true, + msg: Message::MemoryCall(MemoryCall { + request: ToolRequest { + session: SessionId::new("mm-now").unwrap(), + call: CallId(3), + tool: tool.to_string(), + arguments: arguments.to_string(), + }, + taint: DataClass::Private, + untrusted: false, + }), + }; + write_frame(&mut stream, &env).unwrap(); + match read_frame(&mut stream).unwrap().msg { + Message::ToolResponse(response) => (response, started.elapsed()), + other => panic!("{other:?}"), + } +} + +fn result(response: ToolResponse) -> (String, DataClass, bool) { + match response { + ToolResponse::Result { + content, + class, + untrusted, + .. + } => (content, class, untrusted), + other => panic!("{other:?}"), + } +} + +/// The paths of the fake's search and conclusion-query requests. +fn reads(fake: &FakeHoncho) -> Vec { + fake.requests() + .into_iter() + .map(|r| r.path) + .filter(|p| p.ends_with("/search") || p.ends_with("/conclusions/query")) + .collect() +} + +/// T58, end to end: a fleet hit shown makes `memoryd`'s answer untrusted, as `brokerd` will label +/// the session (task 02); hits only from a workspace labelled trusted do not. +#[test] +fn a_fleet_hit_makes_the_answer_untrusted() { + let fake = FakeHoncho::start(); + let home = Home::new("recall-serve-fleet", Some(&fake.url)); + let _lines = home.serve(); + let socket = home.config.broker_socket(); + let (content, class, untrusted) = + result(call(&socket, "recall", r#"{"query":"tea","k":10}"#).0); + assert!( + content.starts_with("[1] memory · session mm-a, 2026-09-20 14:01 UTC, the owner said · private\n tea time\n[2] memory · boxmaker1: kyle said, "), + "{content}" + ); + assert!( + content.contains("\n[3] memory-tainted · fleet: kyle said, "), + "{content}" + ); + // The conclusions about the owner's peer, `[honcho] owner`, are asked for and shown. + assert!( + content.contains("memory · boxmaker1: a conclusion about kyle, "), + "{content}" + ); + assert_eq!((class, untrusted), (DataClass::Private, true)); + assert!( + !reads(&fake).iter().any(|p| p.contains("boxmaker0")), + "{:?}", + reads(&fake) + ); + fake.set_workspace("fleet", Mode::Missing); + let (content, _, untrusted) = result(call(&socket, "recall", r#"{"query":"tea","k":10}"#).0); + assert!(!content.contains("fleet"), "{content}"); + assert!(!untrusted, "{content}"); +} + +/// `recall` releases the state lock before it asks Honcho: while a source hangs, a memory call is +/// answered at once, and the `recall` answers after the source's limit with its `unavailable` +/// line. +#[test] +fn a_hanging_source_holds_up_no_memory_call() { + let fake = FakeHoncho::start(); + fake.set_workspace("fleet", Mode::Hang); + let home = Home::new("recall-serve-hang", Some(&fake.url)); + let lines = home.serve(); + let socket = home.config.broker_socket(); + let for_recall = socket.clone(); + let recall = std::thread::spawn(move || call(&for_recall, "recall", r#"{"query":"tea"}"#)); + let deadline = Instant::now() + Duration::from_secs(10); + while !reads(&fake) + .iter() + .any(|p| p == "/v3/workspaces/fleet/search") + { + assert!(Instant::now() < deadline, "the fleet was never searched"); + std::thread::sleep(Duration::from_millis(10)); + } + let (answer, took) = call( + &socket, + "memory", + r#"{"action":"note","text":"tea is kept in the blue tin"}"#, + ); + assert!(matches!(answer, ToolResponse::Result { .. }), "{answer:?}"); + assert!( + took < Duration::from_secs(2), + "the memory call waited {took:?}" + ); + assert!(!recall.is_finished(), "the recall is still waiting"); + let (answer, took) = recall.join().unwrap(); + let (content, _, untrusted) = result(answer); + assert!(content.ends_with("\nfleet: unavailable"), "{content}"); + assert!(took < Duration::from_secs(15), "{took:?}"); + assert!(!untrusted, "no fleet hit was shown: {content}"); + assert!( + lines + .lock() + .unwrap() + .iter() + .any(|l| l.starts_with("memoryd: recall: fleet unavailable: ")), + "{:?}", + lines.lock().unwrap() + ); +} + +/// Nothing found anywhere: the sentence local `recall` gives. +#[test] +fn nothing_found_anywhere() { + let fake = FakeHoncho::start(); + let home = Home::new("recall-serve-nothing", Some(&fake.url)); + let _lines = home.serve(); + let (content, class, untrusted) = result( + call( + &home.config.broker_socket(), + "recall", + r#"{"query":"zebra"}"#, + ) + .0, + ); + assert_eq!(content, "(no matches for \"zebra\")"); + assert_eq!((class, untrusted), (DataClass::Private, false)); +} + +/// Without `[honcho]`, `recall` is the local index's, and nothing else is asked. +#[test] +fn without_honcho_recall_answers_as_before() { + let home = Home::new("recall-serve-off", None); + let lines = home.serve(); + let socket = home.config.broker_socket(); + let (content, class, untrusted) = result(call(&socket, "recall", r#"{"query":"tea"}"#).0); + assert_eq!( + content, + "[1] memory · session mm-a, 2026-09-20 14:01 UTC, the owner said · private\n tea time" + ); + assert_eq!((class, untrusted), (DataClass::Private, false)); + let (content, _, _) = result(call(&socket, "recall", r#"{"query":"zebra"}"#).0); + assert_eq!(content, "(no matches for \"zebra\")"); + assert!( + lines.lock().unwrap().is_empty(), + "{:?}", + lines.lock().unwrap() + ); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index e24a36a..c2d3d91 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/09-memoryd-recall-honcho | 2026-09-29 | done | 1 | pass | none | Copied the given test `crates/memoryd/tests/honcho_recall_serve.rs` (new). Step 2 (`cargo test --no-fail-fast -p memoryd --test honcho_recall_serve`) showed exactly the two named failures: `a_fleet_hit_makes_the_answer_untrusted` (only the local hit, no Honcho hit shown) and `a_hanging_source_holds_up_no_memory_call` (`the fleet was never searched`); the other two passed already. Made the edits exactly as the task gives: S1 (`search.rs`, `Reading { readers, owner, wait }` and `Reading::recall` = `search` then `answer`, before the `Searched` doc); E1 (`use crate::honcho::search;`); E2 (`Memoryd` gains `reading: Option` after `mirror`, before `log`); E3 (`start` binds `(opened, owner)`, `owner = honcho.owner.clone()` or `String::new()`); E4 (`start` builds `search::Reading { readers: Arc::clone(&opened.readers), owner, wait: search::SOURCE_WAIT }` and the match yields `(honcho, mirror, reading)`); E5 (`reading` in the `Memoryd { .. }`); E6 (`call` drops the outer lock, `recall` now `self.recall(&call)`, `memory` `self.memory(&self.lock(), &call)`); E7 (`recall` without `[honcho]` answers the local index under the lock as before, with it runs `recall::local` in a lock block then `reading.recall(local, &args, &self.log)` with the lock released). `cargo check -p memoryd` clean, no warning. `cargo test` passes: 4 in `honcho_recall_serve`, 12 in `honcho_search`, 6 in `honcho_serve`, 8 in `serve`, 5 in `serve_sanitize`. `git diff --stat` lists exactly `search.rs` and `serve.rs`; `wc -l crates/memoryd/src/serve.rs` prints 495. `cargo fmt --all` put the `owner,` of E4's `Reading` on its own line (rustfmt-canonical, the task's block was one line); the edits otherwise match. `make gate` printed `gate: ok` with 1434 tests (1433 in `cargo test --workspace`, plus the 1 ignored `end_to_end`; the +4 over the 1430 before is exactly the tests of `honcho_recall_serve.rs`). No new dependency, so `Cargo.lock` unchanged. | ornith-1.5-35b-a3b | | M5c2/08-memoryd-honcho-search | 2026-09-29 | done | 1 | pass | none | Copied the given test `crates/memoryd/tests/honcho_search.rs` (new), a new version of `crates/memoryd/tests/support/honcho.rs` (from `honcho.rs-task08`, replacing the tree's), and the skeleton `crates/memoryd/src/honcho/search.rs` (fill 2 bodies). Step 2 (`cargo build --tests --keep-going -p memoryd`) showed exactly the one named failure: `honcho_search` with one E0432 `unresolved import memoryd::honcho::search`; every other target compiled. Made H1 (`pub mod search;` after `pub mod report;` in `honcho/mod.rs`). Filled its two bodies from their comments, deleting each instruction comment and keeping the doc comment, each from its comment and `cargo check -p memoryd` after: `source` (`search_workspace` with `limit: Some(k)` via `MessageSearch { limit: Some(k), ..MessageSearch::new(query) }`, error `classify(&e)` then `?`; match `&reader.source`, `Source::Agent(peer)` asks `conclusions(reader, peer, query, k)?` then `conclusions(reader, owner, query, k)?` in that order, any other source both `Vec::new()` with no conclusion asked; `items(&reader.label, query, &messages, &about_agent, &about_owner)`); `answer` (`k = usize::try_from(k).unwrap_or(usize::MAX)`; `Secret` -> `(Vec::new(), SECRET.to_string())`, `Asked { found, unavailable: missing }` (field bound `missing`) -> `interleave(local.items.len(), k, found)` and `unavailable(&names)` with `names` the missing names as `Vec<&str>``; `(class, untrusted) = label(&shown)`; `first = local.items.len() + 1`; `parts = local.items`, push `render(first, &shown)` when shown non-empty, `(no matches for "...")` when parts still empty, then `tail` when non-empty; `Recalled { text: parts.join("\n"), class: local.class.max(class), untrusted: local.untrusted || untrusted }`). `cargo check -p memoryd` clean, no warning. `cargo test` passes: 13 in `honcho_client`, 3 in `honcho_keys` (1 ignored), 8 in `honcho_read`, 12 in `honcho_search`, 12 in `honcho_worker`. `git diff --stat` lists exactly `mod.rs` and `support/honcho.rs`; `git status --short` shows `search.rs` and `honcho_search.rs` as new. `make gate` printed `gate: ok` with 1430 tests (1429 in `cargo test --workspace`, plus the 1 ignored `end_to_end`; the +12 over the 1418 before is exactly the tests of `honcho_search.rs`). No new dependency, so `Cargo.lock` unchanged. | ornith-1.5-35b-a3b | | M5c2/07-memoryd-recall-form | 2026-09-29 | done | 1 | pass | one | Copied the two given tests `crates/memoryd/tests/recall_form.rs` (new) and `crates/memoryd/tests/recall.rs` (new version, replacing the tree's). Step 2 (`cargo build --tests --keep-going -p memoryd`) showed exactly the one named failure: `recall_form` with one E0432 `unresolved imports memoryd::recall::Local, item, local, snippet`; every other target compiled. `cargo test --no-fail-fast` on `recall` failed the two named tests `hits_are_tagged_and_labelled` and `long_hits_are_cut`. Made the edits exactly as the task gives: R1 (`use bxcore::sanitize::neutralise;` before the `DataClass` use); R2 (the `Local` struct, `snippet` and `item` inserted after `Recalled`, before the `terms` doc); R3 (`recall` now calls `local` and joins its items or answers `(no matches for "…")`, the body up to `// 1. args.check()?` becomes `local`); R4 (the comment for `local`'s step 4); R5 (`local`'s body builds `items` with `item(&hit.header(i + 1), &snippet(&hit.text))` and returns `Local { items, class, untrusted }`). H1 (dropped `use bxcore::sanitize::neutralise;`); H2 (`use crate::recall::{item, snippet, when}`); H3 (`Item::render` builds the header then calls `item(&header, &self.text)`); H4 (`clean` now calls `snippet`). `cargo check -p memoryd` clean after all edits, no warning. `cargo test` passes: 4 in `recall_form`, 6 in `recall`, 11 in `honcho_recalled`, 2 in `forgery`. `git diff --stat -- crates/memoryd/src` lists only `recall.rs` and `honcho/recalled.rs`; `cargo fmt --all` reordered the `use crate::recall::{when, SNIPPET}` line the task's H2 quoted, so I anchored H2 on the actual `{when, SNIPPET}` line. `make gate` printed `gate: ok` with 1418 tests (1417 in `cargo test --workspace`, plus the 1 ignored `end_to_end`; the +4 over the 1414 before is exactly the tests of `recall_form.rs`). No new dependency, so `Cargo.lock` unchanged. | ornith-1.5-35b-a3b | | M5c2/06-memoryd-honcho-recalled | 2026-09-29 | done | 1 | pass | one | Copied the given test `crates/memoryd/tests/honcho_recalled.rs` and the skeleton `crates/memoryd/src/honcho/recalled.rs` (fill 3 bodies). Made the edits exactly as the task gives: H1 (`pub mod recalled;` after `pub mod read;` in `honcho/mod.rs`); R1 (`when` `pub(crate)` in `recall.rs`). Filled the three bodies from their comments, deleting each instruction comment and keeping the doc comment, `cargo check -p memoryd` after each: `items` (`words(query)`; chain the messages as `(content, message_from(label,m))`, then the agent's conclusions, then the owner's, each as `(content, conclusion_from(label,c))`; keep only when `relevant(&query, content)`, dedupe by the cleaned text `clean(content)` against what is already kept, else `Item { tag: label.tag(), from, class: label.class, untrusted: label.untrusted(), text }`); `interleave` (`room = k.saturating_sub(local)`, round-robin over the sources' `into_iter`, taking the next of each that has one until `room` is filled or a round takes none, so local hits filling `k` leave none); `label` (`fold` from `DataClass::Private` with `DataClass::max`, `untrusted` when any item's is true). After the last, `cargo check` printed no warning. `cargo test -p memoryd --test honcho_recalled --test recall` passes: 11 and 6. `git diff --stat` lists exactly `mod.rs` (1 line) and `recall.rs` (1 line); the two new files are `recalled.rs` and `honcho_recalled.rs`. `make gate` printed `gate: ok` with 1414 tests (1413 in `cargo test --workspace`, plus the 1 ignored `end_to_end`; the +11 over the 1403 before is exactly the tests of `honcho_recalled.rs`). No new dependency, so `Cargo.lock` unchanged. Deviation: the edit/write layer reordered the `use crate::index::{...}` import on line 10 of `recall.rs` to case-insensitive order (`{class_name, class_of, Index}`) and my follow-up edits to that line did not persist, so I restored the rustfmt-canonical order (`{Index, class_name, class_of}`, which matches HEAD) with an in-place perl edit and confirmed `cargo fmt --all` leaves it. | ornith-1.5-35b-a3b |