diff --git a/crates/bxctl/src/chat.rs b/crates/bxctl/src/chat.rs index 55fccf2..a47ebc0 100644 --- a/crates/bxctl/src/chat.rs +++ b/crates/bxctl/src/chat.rs @@ -58,10 +58,10 @@ pub fn new_session_id() -> SessionId { // The system clock is never before the unix epoch on any machine this runs on. let elapsed = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) - .expect("the system clock is not before the unix epoch"); + .unwrap_or_default(); // "chat--" uses only [a-z0-9-] and stays well under 64 bytes, so this cannot fail. let candidate = format!("chat-{}-{}", elapsed.as_secs(), elapsed.subsec_nanos()); - SessionId::new(&candidate).expect("chat-- is always a valid session id") + SessionId::new(&candidate).unwrap_or_else(|_| SessionId::new("chat-0-0").unwrap_or_default()) } /// Sends one turn and reads the reply. `on_event` sees every event frame as it arrives. diff --git a/crates/bxctl/src/main.rs b/crates/bxctl/src/main.rs index 0639e60..380adc8 100644 --- a/crates/bxctl/src/main.rs +++ b/crates/bxctl/src/main.rs @@ -245,8 +245,9 @@ fn run_interactive(opts: &Options) -> ExitCode { } } Ok(Err(e)) => { + // A failed turn is reported and the loop goes on: the session still exists, so the + // next line resumes it. eprintln!("bxctl: {e}"); - return ExitCode::from(1); } } } diff --git a/crates/bxctl/tests/chat.rs b/crates/bxctl/tests/chat.rs index fa3e845..061a097 100644 --- a/crates/bxctl/tests/chat.rs +++ b/crates/bxctl/tests/chat.rs @@ -57,6 +57,8 @@ fn fake_loopd(events: Vec, end: End) -> FakeLoopd { let id = request.id; let end = if turn.resume && turn.content == "trigger-no-such-session" { End::Error(ErrorCode::NoSuchSession, "session x does not exist") + } else if turn.content == "trigger-turn-limit" { + End::Error(ErrorCode::TurnLimit, "the turn hit a limit") } else { end.clone() }; @@ -417,3 +419,45 @@ fn bad_arguments_print_usage() { .unwrap(); assert_eq!(output.status.code(), Some(2)); } + +/// A failed turn is reported, and the conversation goes on: the session still exists and the +/// next line is a new turn on it. +#[test] +fn interactive_mode_survives_a_failed_turn() { + let fake = fake_loopd( + vec![TurnEvent::Content { + text: "ok".to_string(), + }], + End::Done(TurnDone { + content: "ok".to_string(), + usage: usage(), + }), + ); + let mut child = Command::new(env!("CARGO_BIN_EXE_bxctl")) + .args(["chat", "--socket"]) + .arg(&fake.socket) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap(); + { + let mut stdin = child.stdin.take().unwrap(); + std::io::Write::write_all(&mut stdin, b"first\ntrigger-turn-limit\nthird\n/quit\n") + .unwrap(); + } + let output = child.wait_with_output().unwrap(); + assert!( + output.status.success(), + "a failed turn does not end the chat" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("turn limit: the turn hit a limit"), + "{stderr}" + ); + let turns = fake.turns.lock().unwrap(); + assert_eq!(turns.len(), 3, "the turn after the failure was sent"); + assert!(turns[2].resume, "and it resumed the same session"); + assert_eq!(turns[2].session, turns[0].session); +} diff --git a/crates/loopd/src/baseline.rs b/crates/loopd/src/baseline.rs index d430781..0051a02 100644 --- a/crates/loopd/src/baseline.rs +++ b/crates/loopd/src/baseline.rs @@ -47,12 +47,20 @@ impl Baseline { let mut system = system.trim_end().to_string(); let core = cfg.paths.home.join("memory/core.md"); - if let Ok(text) = std::fs::read_to_string(&core) { - let trimmed = text.trim(); - if !trimmed.is_empty() { - system.push_str("\n\n"); - system.push_str(trimmed); + match std::fs::read_to_string(&core) { + Ok(text) => { + let trimmed = text.trim(); + if !trimmed.is_empty() { + system.push_str("\n\n"); + system.push_str(trimmed); + } } + // A missing file is fine; an unreadable one is reported so the owner is not given a + // session without the memory they curated and no sign of it. + Err(e) if e.kind() != std::io::ErrorKind::NotFound => { + return Err(BaselineError::Read(core.clone(), e)); + } + Err(_) => {} } Ok(Baseline { diff --git a/crates/loopd/src/channel.rs b/crates/loopd/src/channel.rs index 0237e0b..2a1cb01 100644 --- a/crates/loopd/src/channel.rs +++ b/crates/loopd/src/channel.rs @@ -1,6 +1,6 @@ //! The channel server: one turn per connection over the M1 frame protocol. A session runs one //! turn at a time; the busy set keeps a second turn on the same session from starting until the -//! first finishes, and is released before the final frame so a client can send the next turn the +//! first finishes, and is released before every final frame so a client can send the next turn the //! moment it reads the last one. use std::collections::HashSet; @@ -54,9 +54,8 @@ struct Held<'a> { impl Drop for Held<'_> { fn drop(&mut self) { - if let Ok(mut busy) = self.ctx.busy.lock() { - busy.remove(&self.id); - } + let mut busy = self.ctx.busy.lock().unwrap_or_else(|p| p.into_inner()); + busy.remove(&self.id); } } @@ -132,6 +131,7 @@ pub fn handle(stream: UnixStream, ctx: Arc) { match Session::open(&ctx.cfg.paths.home, turn.session.clone()) { Ok(session) => session, Err(e) => { + drop(held); let _ = write_frame( &mut stream, &error_frame(session_code(&e), request.id, e.to_string()), @@ -143,6 +143,7 @@ pub fn handle(stream: UnixStream, ctx: Arc) { let baseline = match Baseline::assemble(&ctx.cfg, &ctx.registry) { Ok(baseline) => baseline, Err(e) => { + drop(held); let _ = write_frame( &mut stream, &error_frame(ErrorCode::Internal, request.id, e.to_string()), @@ -158,6 +159,7 @@ pub fn handle(stream: UnixStream, ctx: Arc) { ) { Ok(session) => session, Err(e) => { + drop(held); let _ = write_frame( &mut stream, &error_frame(session_code(&e), request.id, e.to_string()), diff --git a/crates/loopd/tests/baseline.rs b/crates/loopd/tests/baseline.rs index 7efd4fd..1e70921 100644 --- a/crates/loopd/tests/baseline.rs +++ b/crates/loopd/tests/baseline.rs @@ -232,3 +232,28 @@ fn replay_of_a_prefix_is_a_prefix() { assert_eq!(whole[..part.len()], part[..], "prefix of {n} records"); } } + +/// A core memory file that exists but cannot be read is an error, not silently absent: the owner +/// would otherwise get a session without the memory they curated, and no sign of it. +#[test] +fn an_unreadable_core_memory_file_is_an_error() { + use std::os::unix::fs::PermissionsExt; + if running_as_root() { + return; // root can read anything; the check is meaningless there + } + let home = Home::new(); + let cfg = home.config(Path::new("/tmp/unused.sock")); + home.write("memory/core.md", "secret memory\n"); + let core = home.dir.join("memory/core.md"); + std::fs::set_permissions(&core, std::fs::Permissions::from_mode(0o000)).unwrap(); + let result = Baseline::assemble(&cfg, &Registry::m2b()); + std::fs::set_permissions(&core, std::fs::Permissions::from_mode(0o644)).unwrap(); + let e = result.expect_err("an unreadable core.md must not be ignored"); + assert!(e.to_string().contains("core.md"), "{e}"); +} + +fn running_as_root() -> bool { + std::fs::read_to_string("/proc/self/status") + .map(|s| s.lines().any(|l| l.starts_with("Uid:\t0\t"))) + .unwrap_or(false) +} diff --git a/crates/proto/src/ids.rs b/crates/proto/src/ids.rs index 0b23f4f..b796f3b 100644 --- a/crates/proto/src/ids.rs +++ b/crates/proto/src/ids.rs @@ -42,6 +42,13 @@ impl SessionId { } } +impl Default for SessionId { + fn default() -> Self { + // Not the empty string, which `new` rejects: a fallback must still be a valid id. + SessionId("chat-0-0".to_string()) + } +} + impl TryFrom for SessionId { type Error = ValueError; diff --git a/docs/implementer-log.md b/docs/implementer-log.md index b82de55..74fe20a 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -40,6 +40,7 @@ reviewer adds findings under "Reviews" once per milestone. | M2b/08-loopd-serve | 2026-09-18 | done | 2 | fail | none | Rewrote crates/loopd/src/main.rs into two commands, `selftest` and `serve`, both sharing `run_selftest_check` so the self-test lines are identical. `serve` loads config (exit 1 on failure), removes an existing socket via `channel_socket()` before the self-test, runs the self-test and exits 1 without binding on failure, then creates the socket's parent dir, binds, sets mode 0600 with `std::fs::set_permissions`, prints `serving on`, and calls `channel::serve` with a `Context` from the config, client, `Box::new(FakeTools::new())` and `Registry::m2b()`. Anything else prints both usages and exits 2. The `serve_refuses...` test's "no socket left behind" holds because the socket is removed before the self-test and binding happens only after it passes. First gate run failed on two clippy `collapsible_if` lints; collapsed the two nested `if let` into edition-2024 let-chains and re-ran, which passed. `cargo test -p loopd --test serve` reports 3 passed. | Ornith-1.5-35B-A3B | | M2b/09-bxctl-chat | 2026-09-18 | done | 5 | fail | none | Wrote crates/bxctl/src/chat.rs: `run_turn` (open socket, one `write_frame` with id 1, loop `read_frame` asserting id 1, dispatch final `TurnDone`/`Error` and non-final `TurnEvent` to `on_event`, every other frame `Protocol`); `ChatError` (Connect/Frame/Refused/Protocol) with `source()` returning the io and FrameError; `new_session_id` = `chat--` via two `expect`s (the epoch check and a private-field construction that cannot fail); `Printer` with json mode (one serde_json line per event, no skipping, no escape codes), a dimmed reasoning block opened on the first `Reasoning` and closed on the next non-reasoning event or `end_reasoning`, and every other event kind named exactly. Registered `pub mod chat;` in lib.rs. Rewrote main.rs into a `chat` subcommand: usage + exit 2 for a wrong first arg or unknown flag/missing value/invalid id, `$BOXMAKER_HOME/run/loop/loop.sock` else `/var/lib/boxmaker/...`, `--say` (events to stderr, answer to stdout, resume=true then one retry with resume=false on no_such_session), interactive (create on first turn, resume on the rest, `/quit` stops, the created session id printed once to stdout), `--json` (events to stderr, the TurnDone also to stderr after them, plain answer to stdout). A `Sink` records the first write error so the `on_event` closure (which cannot return a Result) does not lose it. All 11 chat tests pass. Four gate runs before clean: clippy `io_other_error` (switched to `Error::other`), then `redundant_closure` twice (the `other` map and `get_or_insert_with`), then a rustfmt import-order diff./? | Ornith-1.5-35B-A3B | | M2b/10-verify-device | 2026-09-18 | done | 1 | pass | none | No library code. Copied the three given files byte-identical (`cmp` clean): `crates/loopd/tests/device.rs` (replaces the M2a one, its four checks still in it), `Makefile` (only change: `verify-device` now also passes `BOXMAKER_BXCTL`), and `config/system.md`. `make gate` printed `gate: ok` with device at `0 passed; 0 failed; 6 ignored`. `curl http://straylight:11434/health` returned `{"status":"ok"}`. `make verify-device` ran all six checks against the real server in 41.6s, all passed: self-test, capped-thinking block, a four-turn conversation surviving a `loopd` restart with its cache, a request surviving its proxy being killed and restarted, a second turn reusing the first turn's cache, and the baseline fitting the token budget. The baseline is 251 tokens (the brief allows 3000). Ran directly rather than via a subagent: the `delegate` tool returned `Agent "undefined" not found` on every attempt. | Ornith-1.5-35B-A3B | +| M2b/11-review-fixes | 2026-09-18 | done | 1 | pass | a Default impl for SessionId was added to crates/proto/src/ids.rs, which the task did not list | Copied the two given tests (loopd/baseline.rs, bxctl/chat.rs). In channel.rs the busy guard is now dropped before every final frame (the three open/create/assemble session errors, plus the existing turn_done/error path) and Held::drop recovers a poisoned lock with unwrap_or_else(|p| p.into_inner()). baseline::assemble treats a core.md that exists but cannot be read as BaselineError::Read, a missing one still fine (matched on ErrorKind::NotFound). bxctl's interactive loop reports a failed turn and continues instead of exiting 1, and new_session_id has no expect. The prescribed new_session_id fix (unwrap_or_else with a fixed valid id via unwrap_or_default) does not compile without SessionId: Default, and there is no non-panicking way to build a SessionId outside proto, so the fallback default is the valid id "chat-0-0". Staged proto in addition to the listed paths because the build requires it. Gate: ok, 219 tests. | ? | ## Reviews