kyle/boxmaker
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

M5c-2 plan: task 09, recall asks Honcho outside the state lock

Browse Source 
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
kyle
2026-09-28 19:58:04 -07:00
co-authored by Claude Opus 5.5
parent d41c41eb4a
commit 30538b0243
3 changed files with 600 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plans/M5c2/09-memoryd-recall-honcho.md
+358
View File
@@ -0,0 +1,358 @@
# M5c-2 task 09: `recall` asks Honcho, outside the state lock
**Branch:** `m5c2`. Check it: `git branch --show-current` prints `m5c2` and `git status --short`
is empty; otherwise stop. Never switch branch.
**Commit subject:** `memoryd: recall asks Honcho after the local index, outside the state lock`
**Log row:** the Task cell is `M5c2/09-memoryd-recall-honcho`.
## Goal
Task 08 wrote `recall`'s Honcho part (`honcho/search.rs`): `search` asks every source at once,
each with its own limit, and `answer` makes the answer from the local index's items and
Honcho's. This task makes `memoryd`'s `recall` use it (M5c spec, section 8).
- **With `[honcho]`**, a `recall` call:
1. takes the state lock, runs `recall::local` (task 07: the search, the recall log, the local
items and their label), and releases the lock;
2. then, with no lock held, runs the Honcho part and makes the answer: `search::Reading::recall`,
a new method that is `search` then `answer`.
So a source that hangs for its 5 s holds up no `memory` call, no memory block and no other
`recall`: before this task, `call` held the lock for the whole of any call.
- **Without `[honcho]`**, `recall` is exactly as before: `recall::recall` under the lock.
- **`memory`** calls are unchanged: under the lock for the whole call.
- **At start**, with `[honcho]`, `start` keeps what `recall` reads, `search::Reading { readers,
owner, wait }`: the readers task 05 built (`opened.readers`, taken before the worker's thread
takes the rest of `Opened`), the owner's peer from `[honcho]` (`honcho.owner`), and
`search::SOURCE_WAIT` (5 s).
`serve.rs` is near the 500-line limit (495 lines after this task), so `Reading` and its method go
into `search.rs`, not `serve.rs`.
## Every place that changes
This is the complete list.
| Place | What happens there |
|---|---|
| `crates/memoryd/src/honcho/search.rs` | `Reading` and `Reading::recall` (edit S1) |
| `crates/memoryd/src/serve.rs` | a `use` line; `Memoryd::reading`; `start` builds it; `call` and `recall` (edits E1 to E7) |
| `crates/memoryd/tests/honcho_recall_serve.rs` | new, given |
| everything else | **no change**: not `recall.rs`, not `start.rs`, not the worker, no `Cargo.toml` |
## Files
- Copy, from `docs/plans/M5c2/files/` (the same path under it): the new test
`crates/memoryd/tests/honcho_recall_serve.rs`.
- Modify: `crates/memoryd/src/honcho/search.rs`, `crates/memoryd/src/serve.rs`,
`docs/implementer-log.md`.
- Do not touch: any other file.
## Facts you need
- **`serve.rs` today.** `start` opens the Honcho side (`worker::open`, returning `Opened`) when
`config.honcho_config()?` is `Some(honcho)`, and later moves `opened` into the worker's thread
(`worker::spawn`). `Opened::readers` is an `Arc<Readers>` (task 05). `call` takes the state
lock (`self.lock()`) first, then runs `recall` or `memory` with it.
- **Taking the lock for a block only**: `let local = { let state = self.lock(); … };` releases
the lock at the block's closing brace, when `state` is dropped. The Honcho part runs after it.
- **`let … else`**: `let Some(reading) = &self.reading else { …; return …; };` runs the `else`
block (which must return) when there is no `[honcho]`.
- **What `recall` answers with `[honcho]`** (task 08's `answer`): the local items, then Honcho's
up to `k` in all, numbered after them, then any `{source}: unavailable` line;
`(no matches for "…")` when there is nothing. A failed or late source is logged by
`search`, never with the query.
- **The given test** `honcho_recall_serve.rs` (4 tests; about 5 s), through `memoryd`'s `brokerd`
socket, against the fake Honcho (task 08's version):
- a local hit, then `boxmaker1`'s (labelled trusted, among them a conclusion about the owner's
peer, `kyle`) and the fleet's; the answer untrusted because a fleet hit is shown (T58); the
agent's own workspaces never searched; with the fleet missing, no fleet hit and a trusted
answer;
- while the fleet hangs, a `memory` call answered within 2 s and the `recall` still waiting;
then the `recall` answered with `fleet: unavailable`, and the line logged;
- nothing found anywhere: `(no matches for "zebra")`;
- without `[honcho]`: the local answer exactly as before, and nothing logged.
- The items the edits use. This table is complete.
| Item | Where |
|---|---|
| `search::search`, `search::answer`, `search::SOURCE_WAIT` (task 08) | S1, E4 |
| `recall::local`, `recall::recall` (task 07) | E7 |
| `Arc::clone` | E4 |
## 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/memoryd/src/honcho/search.rs`
**S1.** The line
```rust
/// What the Honcho part found.
```
becomes
```rust
/// 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<Readers>,
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.
```
### `crates/memoryd/src/serve.rs`
**E1.** Add one line. The line
```rust
use crate::honcho::report;
```
becomes
```rust
use crate::honcho::report;
use crate::honcho::search;
```
**E2.** In `struct Memoryd`, the lines
```rust
mirror: Option<Mirror>,
log: Log,
}
```
become
```rust
mirror: Option<Mirror>,
/// With `[honcho]`: what `recall` reads beside the local index, outside `state`.
reading: Option<search::Reading>,
log: Log,
}
```
**E3.** In `start`, the lines
```rust
let opened = match config.honcho_config()? {
Some(honcho) => Some(worker::open(&config, &honcho, &log)?),
None => None,
};
```
become
```rust
let (opened, owner) = match config.honcho_config()? {
Some(honcho) => (
Some(worker::open(&config, &honcho, &log)?),
honcho.owner.clone(),
),
None => (None, String::new()),
};
```
**E4.** In `start`, the lines
```rust
let (honcho, mirror) = match opened {
Some(opened) => {
let sync = Arc::clone(&opened.sync);
let mirror = opened.mirror();
worker::spawn(opened, &config, Arc::clone(&wake), Arc::clone(&log))?;
(Some(sync), Some(mirror))
}
None => (None, None),
};
```
become
```rust
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(reading))
}
None => (None, None, None),
};
```
**E5.** In `start`, the lines
```rust
wake,
mirror,
log,
}),
```
become
```rust
wake,
mirror,
reading,
log,
}),
```
**E6.** The lines
```rust
/// A decided call from `brokerd`: `recall` or `memory`, under the one lock. `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),
```
become
```rust
/// 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 result = match call.request.tool.as_str() {
bxcore::memory::RECALL => self.recall(&call),
bxcore::memory::MEMORY => self.memory(&self.lock(), &call),
```
**E7.** The lines
```rust
/// `recall`: search, labelled by what it found.
fn recall(&self, state: &State, call: &MemoryCall) -> Result<Recalled, String> {
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(),
)
}
```
become
```rust
/// `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<Recalled, String> {
let args: RecallArgs = match serde_json::from_str(&call.request.arguments) {
Ok(args) => args,
Err(e) => return Err(format!("recall's arguments: {e}")),
};
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))
}
```
## Steps
- [ ] **1. Copy the given test.** From the repository root, one command:
```sh
cp docs/plans/M5c2/files/crates/memoryd/tests/honcho_recall_serve.rs crates/memoryd/tests/
```
- [ ] **2. See it fail.** `cargo test --no-fail-fast -p memoryd --test honcho_recall_serve`, with
a shell timeout of 600000 ms: it compiles, and two tests fail:
`a_fleet_hit_makes_the_answer_untrusted` (no Honcho hit is shown yet) and
`a_hanging_source_holds_up_no_memory_call` (the fleet is never searched: ``the fleet was never
searched``). The other two pass already.
- [ ] **3. The edits.** Make edits S1 and E1 to E7, in order. `cargo check -p memoryd` prints no
error and no warning.
- [ ] **4. The tests.** With a shell timeout of 600000 ms, every test passes: 4 in
`honcho_recall_serve`, and as before 12 in `honcho_search`, 6 in `honcho_serve`, 8 in `serve`
and 5 in `serve_sanitize`:
```sh
cargo test -p memoryd --test honcho_recall_serve --test honcho_serve --test serve \
--test serve_sanitize --test honcho_search
```
- [ ] **5. Check.** Two commands:
- `git diff --stat` lists exactly `crates/memoryd/src/honcho/search.rs` and
`crates/memoryd/src/serve.rs`.
- `wc -l crates/memoryd/src/serve.rs` prints 495.
- [ ] **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: **1434** after this
task, 1430 before it; the 4 new ones are the tests of `honcho_recall_serve.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/09-memoryd-recall-honcho`. Stage with
`git add crates/memoryd 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 1434 tests.
- `git diff HEAD~1 --stat` lists only `search.rs`, `serve.rs`, `honcho_recall_serve.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: a compile error, or other tests failing.
- `cargo check` in step 3 prints an error or a warning. Quote it, and stop.
- A test fails in step 4. Do not change a test: stop, and give the failing tests' names and
their messages.
- The fix seems to need a change anywhere else: `recall.rs`, `start.rs`, `read.rs`, the worker,
a configuration key for the limit. It does not; stop and say why you think so.
- `serve.rs` passes 500 lines, or the gate's line check fails.
- A test outside the given file 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 1434.
- 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 (8 tried, all
caught): the state lock held across the Honcho part; Honcho not asked though configured; no
`Reading` built at start; the agent's peer passed as the owner's; the path without `[honcho]`
going through the Honcho answer; `Reading::recall` dropping Honcho's label; a zero limit; the
local items left out of the answer.
docs/plans/M5c2/README.md
+1
View File
@@ -107,6 +107,7 @@ The gate count is the passed total of `cargo test --workspace` plus one, for the
| 06 | 1414 | 22, 22 | | 06 | 1414 | 22, 22 |
| 07 | 1418 | 15, 15 | | 07 | 1418 | 15, 15 |
| 08 | 1430 | 19, 19 | | 08 | 1430 | 19, 19 |
| 09 | 1434 | 8, 8 |
**A flaky test, fixed before the run:** `bxctl`'s `chat_approvals` test **A flaky test, fixed before the run:** `bxctl`'s `chat_approvals` test
`json_prints_only_json_and_never_asks_brokerd` failed once in 5 loaded runs of the whole workspace `json_prints_only_json_and_never_asks_brokerd` failed once in 5 loaded runs of the whole workspace
docs/plans/M5c2/files/crates/memoryd/tests/honcho_recall_serve.rs
+241
View File
@@ -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<Mutex<Vec<String>>> {
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<String> {
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()
);
}