diff --git a/crates/brokerd/src/memory_port.rs b/crates/brokerd/src/memory_port.rs index 8468c3b..f41cc79 100644 --- a/crates/brokerd/src/memory_port.rs +++ b/crates/brokerd/src/memory_port.rs @@ -1,6 +1,7 @@ -//! Where decided `recall` and `memory` calls go: to `memoryd` over its `brokerd` socket, with the -//! session's state, instead of a container (M5a spec, section 8). `Routed` sends every other call -//! to the configured runtime. +//! Where decided `recall`, `memory` and `ask_memory` calls go: to `memoryd` over its `brokerd` +//! socket, with the session's state, instead of a container (M5a spec, section 8). `Routed` sends +//! every other call to the configured runtime. `ask_memory` has a wait of its own (M5c spec, +//! section 9). use std::os::unix::net::UnixStream; use std::path::PathBuf; @@ -11,6 +12,7 @@ use bxcore::{ write_frame, }; +use crate::args::ToolName; use crate::policy::Label; use crate::runner::{RunError, RunOutput, RunSpec, Runtime}; @@ -18,27 +20,52 @@ use crate::runner::{RunError, RunOutput, RunSpec, Runtime}; pub const UNREACHABLE: &str = "memory is unavailable: memoryd did not answer (see docs/runbook.md#memoryd-unreachable)"; -/// How long `brokerd` waits for `memoryd` to answer one call. +/// How long `brokerd` waits for `memoryd` to answer one `recall` or `memory` call. pub const MEMORY_WAIT: Duration = Duration::from_secs(30); +/// How long it waits for one `ask_memory` call: `memoryd` asks Honcho's chat about each +/// workspace, both at once, with 150 s each (M5c spec, section 9). +pub const ASK_MEMORY_WAIT: Duration = Duration::from_secs(170); /// A client of `memoryd`'s `brokerd` socket. One connection per call. pub struct MemoryPort { socket: PathBuf, wait: Duration, + ask_wait: Duration, } impl MemoryPort { + /// A port that waits `wait` for `recall` and `memory`, and `ASK_MEMORY_WAIT` for `ask_memory`. pub fn new(socket: PathBuf, wait: Duration) -> MemoryPort { - MemoryPort { socket, wait } + MemoryPort { + socket, + wait, + ask_wait: ASK_MEMORY_WAIT, + } + } + + /// The same port, waiting `wait` for `ask_memory`. The tests use a short one. + pub fn with_ask_wait(self, wait: Duration) -> MemoryPort { + MemoryPort { + ask_wait: wait, + ..self + } + } + + /// How long one call of `tool` waits for `memoryd`, to write the call and to read the answer. + pub fn wait_for(&self, tool: ToolName) -> Duration { + match tool { + ToolName::AskMemory => self.ask_wait, + _ => self.wait, + } } fn ask(&self, spec: &RunSpec) -> Option { // `None` at the first thing that fails. 1. A MemoryCall: request with the spec's session, // call, `spec.tool().as_str()`, and `spec.arguments().canonical_json()`; taint and // untrusted from `spec.state()`. 2. `UnixStream::connect(&self.socket)`, then read and - // write timeouts of `self.wait`. 3. `write_frame` an Envelope { v: PROTOCOL_VERSION, id: - // spec.call().0, final: true, msg: Message::MemoryCall(call) }. 4. `read_frame`; a - // Message::ToolResponse is Some(it), any other message None. + // write timeouts of `self.wait_for(spec.tool())`. 3. `write_frame` an Envelope { v: + // PROTOCOL_VERSION, id: spec.call().0, final: true, msg: Message::MemoryCall(call) }. 4. + // `read_frame`; a Message::ToolResponse is Some(it), any other message None. let call = MemoryCall { request: ToolRequest { session: spec.session().clone(), @@ -49,9 +76,10 @@ impl MemoryPort { taint: spec.state().taint, untrusted: spec.state().untrusted, }; + let wait = self.wait_for(spec.tool()); let mut stream = UnixStream::connect(&self.socket).ok()?; - stream.set_read_timeout(Some(self.wait)).ok()?; - stream.set_write_timeout(Some(self.wait)).ok()?; + stream.set_read_timeout(Some(wait)).ok()?; + stream.set_write_timeout(Some(wait)).ok()?; write_frame( &mut stream, &Envelope { diff --git a/crates/brokerd/tests/memory_wait.rs b/crates/brokerd/tests/memory_wait.rs new file mode 100644 index 0000000..2c1c2fa --- /dev/null +++ b/crates/brokerd/tests/memory_wait.rs @@ -0,0 +1,131 @@ +//! How long `brokerd` waits for `memoryd`, per tool: 30 s for `recall` and `memory`, 170 s for +//! `ask_memory`, whose two Honcho chats have 150 s each (M5c spec, sections 9 and 11). The waits +//! are shortened here with `with_ask_wait` and `new`'s own wait, so no test waits long. Do not +//! edit. + +#[path = "support/build.rs"] +mod build; +#[path = "support/tmp.rs"] +mod tmp; + +use std::os::unix::net::UnixListener; +use std::path::Path; +use std::time::Duration; + +use brokerd::args::ToolName; +use brokerd::memory_port::{ASK_MEMORY_WAIT, MEMORY_WAIT, MemoryPort, UNREACHABLE}; +use brokerd::policy::decide; +use brokerd::runner::run; +use build::{allowed, now, private, request, set}; +use bxcore::{DataClass, Envelope, Message, ToolRequest, ToolResponse, read_frame, write_frame}; +use tmp::TempDir; + +/// How long the slow fake `memoryd` takes to answer. +const SLOW: Duration = Duration::from_millis(1_500); +/// A wait shorter than `SLOW`, and one far longer. +const SHORT: Duration = Duration::from_millis(300); +const LONG: Duration = Duration::from_secs(20); + +fn found() -> ToolResponse { + ToolResponse::Result { + content: "own: memory · The owner prefers UTC.".to_string(), + class: DataClass::Private, + untrusted: false, + truncated: false, + } +} + +/// A fake `memoryd` that answers one call after `SLOW`. A failed write is ignored: the port has +/// stopped waiting, which is what some tests expect. +fn slow_memoryd(socket: &Path) { + let listener = UnixListener::bind(socket).unwrap(); + std::thread::spawn(move || { + let Ok((mut stream, _)) = listener.accept() else { + return; + }; + let Ok(env) = read_frame(&mut stream) else { + return; + }; + std::thread::sleep(SLOW); + let reply = Envelope { + v: 1, + id: env.id, + r#final: true, + msg: Message::ToolResponse(found()), + }; + let _ = write_frame(&mut stream, &reply); + }); +} + +/// One allowed call of `req` through `port`, to a slow fake `memoryd` of its own. +fn ask( + dir: &TempDir, + name: &str, + port: impl Fn(&Path) -> MemoryPort, + req: ToolRequest, +) -> ToolResponse { + let socket = dir.path().join(name); + slow_memoryd(&socket); + let decision = allowed(decide(req, &set(vec![]), private(), now())); + run(decision, &port(&socket)) +} + +fn unreachable() -> ToolResponse { + ToolResponse::Failed { + message: UNREACHABLE.to_string(), + } +} + +const QUESTION: &str = r#"{"question":"What does the owner prefer?"}"#; +const QUERY: &str = r#"{"query":"owner"}"#; + +#[test] +fn the_waits_and_which_tool_gets_which() { + assert_eq!(MEMORY_WAIT, Duration::from_secs(30)); + assert_eq!(ASK_MEMORY_WAIT, Duration::from_secs(170)); + let port = MemoryPort::new("memory.sock".into(), MEMORY_WAIT); + assert_eq!(port.wait_for(ToolName::Recall), MEMORY_WAIT); + assert_eq!(port.wait_for(ToolName::Memory), MEMORY_WAIT); + assert_eq!(port.wait_for(ToolName::AskMemory), ASK_MEMORY_WAIT); + let port = MemoryPort::new("memory.sock".into(), SHORT); + assert_eq!(port.wait_for(ToolName::Recall), SHORT); + assert_eq!(port.wait_for(ToolName::Memory), SHORT); + assert_eq!(port.wait_for(ToolName::AskMemory), ASK_MEMORY_WAIT); + let port = MemoryPort::new("memory.sock".into(), MEMORY_WAIT).with_ask_wait(LONG); + assert_eq!(port.wait_for(ToolName::Recall), MEMORY_WAIT); + assert_eq!(port.wait_for(ToolName::Memory), MEMORY_WAIT); + assert_eq!(port.wait_for(ToolName::AskMemory), LONG); +} + +/// A slow answer reaches `ask_memory`, whose wait is long, and not `recall` or `memory`, whose +/// wait is short: the wait is chosen by the call's tool. +#[test] +fn a_slow_answer_reaches_ask_memory_only_when_its_wait_is_longer() { + let dir = TempDir::new("wait-ask"); + let port = |socket: &Path| MemoryPort::new(socket.to_path_buf(), SHORT).with_ask_wait(LONG); + assert_eq!( + ask(&dir, "a.sock", port, request("ask_memory", QUESTION)), + found() + ); + assert_eq!( + ask(&dir, "r.sock", port, request("recall", QUERY)), + unreachable() + ); + let note = r#"{"action":"note","text":"x"}"#; + assert_eq!( + ask(&dir, "m.sock", port, request("memory", note)), + unreachable() + ); +} + +/// And the other way round: a short wait for `ask_memory` gives up, while `recall` waits. +#[test] +fn a_slow_answer_reaches_recall_only_when_its_wait_is_longer() { + let dir = TempDir::new("wait-recall"); + let port = |socket: &Path| MemoryPort::new(socket.to_path_buf(), LONG).with_ask_wait(SHORT); + assert_eq!(ask(&dir, "r.sock", port, request("recall", QUERY)), found()); + assert_eq!( + ask(&dir, "a.sock", port, request("ask_memory", QUESTION)), + unreachable() + ); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 10577e5..e5e10be 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/03-brokerd-ask-memory-wait | 2026-09-28 | done | 1 | pass | none | Copied the given test `crates/brokerd/tests/memory_wait.rs` from `docs/plans/M5c2/files/crates/brokerd/tests/`. Step 2 (`cargo build --tests --keep-going -p brokerd`) showed exactly the 10 named failures: one E0432 `unresolved import brokerd::memory_port::ASK_MEMORY_WAIT`, six E0599 for `wait_for`, three E0599 for `with_ask_wait`; every other target compiled. Made edits W1 (module doc naming `ask_memory` and the `ask_memory` wait of its own), W2 (`use crate::args::ToolName`), W3 (`MEMORY_WAIT` doc now says `recall` or `memory`; added `ASK_MEMORY_WAIT` 170 s after it with no blank line between, `MemoryPort` gains `ask_wait` last, `new` sets `ask_wait: ASK_MEMORY_WAIT`, `with_ask_wait` via struct update, `wait_for` matches `AskMemory` -> `ask_wait` else `wait`), W4 (ask comment `self.wait_for(spec.tool())`), W5 (`ask` binds `wait = self.wait_for(spec.tool())` and uses it for both timeouts). `cargo check -p brokerd` clean. `cargo test -p brokerd --test memory_wait --test memory_port --test ask_memory_tool` passes: 3 in `memory_wait`, 5 in `memory_port`, 9 in `ask_memory_tool`. `git diff --stat` lists only `memory_port.rs`; `grep -n 'MEMORY_WAIT' crates/brokerd/src/main.rs` prints the `use` line and `MemoryPort::new(cfg.memory_socket(), MEMORY_WAIT)`. `make gate` printed `gate: ok` with 1384 tests (1383 in `cargo test --workspace`, plus the 1 ignored `end_to_end`; the +3 over the 1381 before is exactly the 3 new `memory_wait` tests). No new dependency, so `Cargo.lock` unchanged. | ornith-1.5-35b-a3b | | M5c2/02-brokerd-ask-memory | 2026-09-28 | done | 1 | pass | none | Copied the given tests from `docs/plans/M5c2/files/crates/brokerd/tests/`: `ask_memory_tool.rs` (new) and new versions of `grants.rs` and `memory_tools.rs`. Step 2 (`cargo build --tests --keep-going -p brokerd`) showed exactly the 11 named failures: one E0432 `unresolved import brokerd::builtin::ASK_MEMORY`, one E0599 for `AskMemory` on `ToolArgs`, nine E0599 for `AskMemory` on `ToolName`; every other target compiled. `cargo test --no-fail-fast` on `grants` and `memory_tools` failed the two named lines (`an_empty_directory_is_a_valid_empty_set`, `grant_files_cannot_name_the_memory_tools_or_take_a_built_in_id`). Made edits A1 (`use bxcore::AskMemoryArgs`), A2 (`ToolName::AskMemory`), A3 (`ask_memory` in `parse`), A4 (`ask_memory` in `as_str`), A5 (`is_memory`), A6 (`ToolArgs::AskMemory`), A7 (`tool()`), A8 (`canonical_json`), A9 (`parse` decodes then runs `check`); B1 (module doc naming the three tools), B2 (`ASK_MEMORY` grant text, `BUILT_IN` sentence naming the three, `sources` with `builtin-ask-memory` last), C1 (`time_limit`), P1 (`matching` takes no constraints), R1 (`runner` no mounts/egress). `cargo check -p brokerd` clean. `cargo test` passes: 9 in `ask_memory_tool`, 17 in `grants`, 4 in `memory_tools`, 5 in `memory_port`, 8 in `runner`. `git diff --stat -- crates/brokerd/src` lists only the five files, not `grants.rs`/`container.rs`/`memory_port.rs`; the other-crates diff is empty. `make gate` printed `gate: ok` with 1381 tests (1380 in `cargo test --workspace`, plus the 1 ignored `end_to_end`; the +9 over the 1372 before is exactly the 9 new `ask_memory_tool` tests). No new dependency, so `Cargo.lock` unchanged. | ornith-1.5-35b-a3b | | 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 |