From 1ceaa36b9b53179749ff9d43c837d22a46a90604 Mon Sep 17 00:00:00 2001 From: "K. Isom" Date: Sun, 20 Sep 2026 17:42:16 -0700 Subject: [PATCH] Give loopd's tool port approvals, its own clock and plain denials Implemented-By: OpenCode session (model recorded in docs/implementer-log.md) --- crates/loopd/src/main.rs | 2 +- crates/loopd/src/tools.rs | 147 ++++++++++++++++++-- crates/loopd/src/turn.rs | 29 ++-- crates/loopd/tests/support/mod.rs | 41 +++++- crates/loopd/tests/tools.rs | 188 +++++++++++++++++++++---- crates/loopd/tests/turn.rs | 25 +++- crates/loopd/tests/turn_broker.rs | 222 ++++++++++++++++++++++++++++++ docs/implementer-log.md | 1 + 8 files changed, 600 insertions(+), 55 deletions(-) create mode 100644 crates/loopd/tests/turn_broker.rs diff --git a/crates/loopd/src/main.rs b/crates/loopd/src/main.rs index bfa0853..a6aefd9 100644 --- a/crates/loopd/src/main.rs +++ b/crates/loopd/src/main.rs @@ -108,7 +108,7 @@ fn run_serve(path: &str) -> ExitCode { cfg, client, Box::new(FakeTools::new()), - Registry::m2b(), + Registry::m3a(), )); if let Err(e) = channel::serve(listener, ctx) { eprintln!("loopd: the channel server stopped: {e}"); diff --git a/crates/loopd/src/tools.rs b/crates/loopd/src/tools.rs index 1210a43..9c1de3c 100644 --- a/crates/loopd/src/tools.rs +++ b/crates/loopd/src/tools.rs @@ -7,7 +7,20 @@ use crate::llama::ToolSchema; /// The seam every tool call goes through. `loopd` answers `find_tool` and `call_tool` itself; /// anything else, including grants, is passed to the port (`brokerd`). pub trait ToolPort: Send + Sync { - fn call(&self, request: &proto::ToolRequest) -> proto::ToolResponse; + /// Returns the final answer, never `PendingApproval`. If the broker says the call is waiting + /// for the owner, the port calls `on_pending` once and goes on waiting. + fn call( + &self, + request: &proto::ToolRequest, + on_pending: &mut dyn FnMut(&Pending), + ) -> proto::ToolResponse; +} + +/// One waiting call, as the broker reports it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Pending { + pub approval: u64, + pub expires: proto::Timestamp, } #[derive(Debug, Clone, PartialEq)] @@ -26,7 +39,9 @@ impl Registry { Registry { entries } } - /// The tool set for the M2b epoch: `clock` (core) and `echo` (not core). + /// The tool set for the M2b epoch: `clock` (core) and `echo` (not core). This is the test + /// registry: `echo` exists only in `FakeTools` and the recorded conversations, and `loopd + /// serve` never uses it. pub fn m2b() -> Registry { Registry::new(vec![ Entry { @@ -40,6 +55,33 @@ impl Registry { ]) } + /// The tool set for the M3a epoch: `clock` (core), then the four tools `brokerd` decides on. + /// The core entries match `m2b()`, so the tools array is unchanged. + pub fn m3a() -> Registry { + Registry::new(vec![ + Entry { + schema: clock_schema(), + core: true, + }, + Entry { + schema: read_file_schema(), + core: false, + }, + Entry { + schema: write_file_schema(), + core: false, + }, + Entry { + schema: shell_schema(), + core: false, + }, + Entry { + schema: http_fetch_schema(), + core: false, + }, + ]) + } + /// The fixed tools array: core entries, then `find_tool`, then `call_tool`. pub fn core_schemas(&self) -> Vec { let mut schemas: Vec = self @@ -76,6 +118,7 @@ impl Registry { pub const FIND_TOOL: &str = "find_tool"; pub const CALL_TOOL: &str = "call_tool"; +pub const CLOCK: &str = "clock"; pub fn clock_schema() -> ToolSchema { ToolSchema { @@ -102,6 +145,65 @@ pub fn echo_schema() -> ToolSchema { } } +pub fn read_file_schema() -> ToolSchema { + ToolSchema { + name: "read_file".to_string(), + description: "Read a text file. Needs a grant from the owner for the path.".to_string(), + parameters: serde_json::json!({ + "type": "object", + "properties": { + "path": { "type": "string", "description": "The absolute path of the file." } + }, + "required": ["path"] + }), + } +} + +pub fn write_file_schema() -> ToolSchema { + ToolSchema { + name: "write_file".to_string(), + description: "Write a text file, replacing it. Needs a grant from the owner for the path." + .to_string(), + parameters: serde_json::json!({ + "type": "object", + "properties": { + "path": { "type": "string", "description": "The absolute path of the file." }, + "content": { "type": "string", "description": "The whole new content of the file." } + }, + "required": ["path", "content"] + }), + } +} + +pub fn shell_schema() -> ToolSchema { + ToolSchema { + name: "shell".to_string(), + description: "Run a shell command in a sandbox. Needs a grant from the owner.".to_string(), + parameters: serde_json::json!({ + "type": "object", + "properties": { + "command": { "type": "string", "description": "The command line to run." }, + "cwd": { "type": "string", "description": "The absolute path of the directory to run it in." } + }, + "required": ["command"] + }), + } +} + +pub fn http_fetch_schema() -> ToolSchema { + ToolSchema { + name: "http_fetch".to_string(), + description: "Fetch an https URL. Needs a grant from the owner for the host.".to_string(), + parameters: serde_json::json!({ + "type": "object", + "properties": { + "url": { "type": "string", "description": "The URL. It must start with https://" } + }, + "required": ["url"] + }), + } +} + fn find_tool_schema() -> ToolSchema { ToolSchema { name: "find_tool".to_string(), @@ -157,6 +259,9 @@ pub fn dispatch(registry: &Registry, name: &str, arguments: &str) -> Dispatch { if name == CALL_TOOL { return dispatch_call_tool(registry, arguments); } + if name == CLOCK { + return Dispatch::Local(proto::Timestamp::now().to_rfc3339()); + } Dispatch::Port { tool: name.to_string(), arguments: arguments.to_string(), @@ -224,6 +329,32 @@ pub fn cap_result(text: &str, cap: usize) -> (String, bool) { (result, true) } +/// The one fixed sentence a denial reaches the model as, named by its reason. +pub fn denial_text(reason: proto::DenyReason) -> &'static str { + match reason { + proto::DenyReason::NoGrant => "Denied: no grant allows this call.", + proto::DenyReason::GrantExpired => "Denied: the grant for this call has expired.", + proto::DenyReason::TaintTooHigh => { + "Denied: this session has seen data too sensitive for this call." + } + proto::DenyReason::DeniedByGrant => "Denied: a grant forbids this call.", + proto::DenyReason::ApprovalRefused => "Denied: the owner refused this call.", + proto::DenyReason::ApprovalExpired => { + "Denied: the approval request expired without an answer." + } + proto::DenyReason::InvalidArguments => "Denied: the arguments are not valid for this tool.", + proto::DenyReason::GrantsInvalid => { + "Denied: the grant files have an error; the owner has been told." + } + proto::DenyReason::AuditUnavailable => { + "Denied: the audit log cannot be written; the owner has been told." + } + proto::DenyReason::StateUnreadable => { + "Denied: this session's broker state is damaged; the owner has been told." + } + } +} + pub struct FakeTools { calls: Mutex>, } @@ -247,19 +378,17 @@ impl FakeTools { } impl ToolPort for FakeTools { - fn call(&self, request: &proto::ToolRequest) -> proto::ToolResponse { + fn call( + &self, + request: &proto::ToolRequest, + _on_pending: &mut dyn FnMut(&Pending), + ) -> proto::ToolResponse { self.calls .lock() .unwrap_or_else(|p| p.into_inner()) .push(request.clone()); match request.tool.as_str() { - "clock" => proto::ToolResponse::Result { - content: proto::Timestamp::now().to_rfc3339(), - class: proto::DataClass::Public, - untrusted: false, - truncated: false, - }, "echo" => match serde_json::from_str::(&request.arguments) { Ok(serde_json::Value::Object(obj)) => { match obj.get("text").and_then(|v| v.as_str()) { diff --git a/crates/loopd/src/turn.rs b/crates/loopd/src/turn.rs index a12d1b2..1c678c0 100644 --- a/crates/loopd/src/turn.rs +++ b/crates/loopd/src/turn.rs @@ -7,7 +7,7 @@ use crate::config::Config; use crate::llama::info::{CacheOutcome, cache_outcome}; use crate::llama::{ChatEvent, ChatRequest, Client, InferError}; use crate::session::{Session, SessionError}; -use crate::tools::{Registry, ToolPort, cap_result, dispatch}; +use crate::tools::{Registry, ToolPort, cap_result, denial_text, dispatch}; use proto::{CallId, DataClass, LogRecord, Timestamp, ToolRequest, ToolResponse, TurnEvent, Usage}; /// What one turn can fail with, beyond the inference server. @@ -230,7 +230,7 @@ pub fn run_turn( false, ) } else { - run_call(rt, session, call, call_id) + run_call(rt, session, call, call_id, on_event) }; let (text, truncated) = cap_result(&text, rt.cfg.r#loop.tool_result_cap); @@ -260,6 +260,7 @@ fn run_call( session: &Session, call: &proto::ToolCall, call_id: CallId, + on_event: &mut dyn FnMut(&TurnEvent), ) -> (String, DataClass, bool) { match dispatch(rt.registry, &call.name, &call.arguments) { crate::tools::Dispatch::Local(text) => (text, DataClass::Public, false), @@ -270,7 +271,14 @@ fn run_call( tool, arguments, }; - match rt.port.call(&request) { + let response = rt.port.call(&request, &mut |pending| { + on_event(&TurnEvent::ApprovalPending { + approval: pending.approval, + tool: request.tool.clone(), + expires: pending.expires, + }); + }); + match response { ToolResponse::Result { content, class, @@ -282,14 +290,15 @@ fn run_call( DataClass::Public, false, ), - ToolResponse::Denied { reason } => ( - format!("The call was denied: {reason:?}"), - DataClass::Public, - false, - ), + ToolResponse::Denied { reason } => { + on_event(&TurnEvent::ToolDenied { + name: request.tool.clone(), + reason, + }); + (denial_text(reason).to_string(), DataClass::Public, false) + } ToolResponse::PendingApproval { .. } => ( - "This version of loopd cannot wait for approval; the call was rejected." - .to_string(), + "The tool failed: the tool broker gave no final answer".to_string(), DataClass::Public, false, ), diff --git a/crates/loopd/tests/support/mod.rs b/crates/loopd/tests/support/mod.rs index 5ba2d1c..5a62324 100644 --- a/crates/loopd/tests/support/mod.rs +++ b/crates/loopd/tests/support/mod.rs @@ -131,14 +131,43 @@ pub fn ok_result(content: &str) -> proto::ToolResponse { } } +pub fn pending(approval: u64, expires: &str) -> proto::ToolResponse { + proto::ToolResponse::PendingApproval { + approval, + expires: proto::Timestamp::parse(expires).unwrap(), + } +} + impl loopd::tools::ToolPort for ScriptedPort { - fn call(&self, request: &proto::ToolRequest) -> proto::ToolResponse { + /// A scripted `PendingApproval` is reported through `on_pending`, as the real port does with + /// the broker's pending frame, and the reply after it is the answer. Use + /// `ReturnsPendingPort` for a port that breaks the rule and returns one. + fn call( + &self, + request: &proto::ToolRequest, + on_pending: &mut dyn FnMut(&loopd::tools::Pending), + ) -> proto::ToolResponse { self.calls.lock().unwrap().push(request.clone()); - self.replies - .lock() - .unwrap() - .pop_front() - .unwrap_or_else(|| ok_result("scripted")) + let mut replies = self.replies.lock().unwrap(); + let mut reply = replies.pop_front(); + if let Some(proto::ToolResponse::PendingApproval { approval, expires }) = reply { + on_pending(&loopd::tools::Pending { approval, expires }); + reply = replies.pop_front(); + } + reply.unwrap_or_else(|| ok_result("scripted")) + } +} + +/// A port that breaks the rule: it returns a pending frame as its final answer. +pub struct ReturnsPendingPort; + +impl loopd::tools::ToolPort for ReturnsPendingPort { + fn call( + &self, + _request: &proto::ToolRequest, + _on_pending: &mut dyn FnMut(&loopd::tools::Pending), + ) -> proto::ToolResponse { + pending(1, "2026-09-18T12:00:00.000Z") } } diff --git a/crates/loopd/tests/tools.rs b/crates/loopd/tests/tools.rs index c4721bb..1e450ac 100644 --- a/crates/loopd/tests/tools.rs +++ b/crates/loopd/tests/tools.rs @@ -1,9 +1,12 @@ -//! Tests for the registry, dispatch, the result cap and the fake tools. Do not edit. +//! Tests for the registry, dispatch, the denial sentences, the result cap and the fake tools. +//! Do not edit. mod support; -use loopd::tools::{Dispatch, FakeTools, Registry, ToolPort, cap_result, dispatch}; -use proto::{CallId, SessionId, ToolRequest, ToolResponse}; +use loopd::tools::{ + Dispatch, FakeTools, Pending, Registry, ToolPort, cap_result, denial_text, dispatch, +}; +use proto::{CallId, DenyReason, SessionId, ToolRequest, ToolResponse}; fn req(tool: &str, arguments: &str) -> ToolRequest { ToolRequest { @@ -105,17 +108,151 @@ fn call_tool_unwraps_a_known_non_core_tool_and_nothing_else() { #[test] fn any_other_tool_goes_to_the_port_as_it_is() { let r = Registry::m2b(); - let want = Dispatch::Port { - tool: "clock".to_string(), - arguments: "{}".to_string(), - }; - assert_eq!(dispatch(&r, "clock", "{}"), want); // Even one the registry does not know: the port (brokerd) decides, not loopd. let want = Dispatch::Port { tool: "read_file".to_string(), arguments: r#"{"path":"/x"}"#.to_string(), }; assert_eq!(dispatch(&r, "read_file", r#"{"path":"/x"}"#), want); + let want = Dispatch::Port { + tool: "weather".to_string(), + arguments: "not json".to_string(), + }; + assert_eq!(dispatch(&r, "weather", "not json"), want); +} + +#[test] +fn the_clock_is_answered_locally_whatever_its_arguments() { + for registry in [Registry::m2b(), Registry::m3a(), Registry::new(vec![])] { + for arguments in ["{}", r#"{"zone":"UTC"}"#, "not json", ""] { + let before = proto::Timestamp::now(); + match dispatch(®istry, "clock", arguments) { + Dispatch::Local(text) => { + let time = proto::Timestamp::parse(&text) + .unwrap_or_else(|e| panic!("an RFC 3339 time, got {text:?}: {e:?}")); + assert!(time >= before, "{text}"); + assert!(text.ends_with('Z'), "UTC: {text}"); + } + other => panic!("{arguments:?}: {other:?}, the clock must not reach the port"), + } + } + } +} + +#[test] +fn the_m3a_registry_has_the_same_core_and_the_four_broker_tools() { + let r = Registry::m3a(); + assert_eq!( + r.core_schemas(), + Registry::m2b().core_schemas(), + "the tools array is part of the baseline: it must not change" + ); + let names = |q: &str| r.find(q).iter().map(|s| s.name.clone()).collect::>(); + assert_eq!(names("file"), ["read_file", "write_file"]); + assert_eq!(names("shell"), ["shell"]); + assert_eq!(names("fetch"), ["http_fetch"]); + assert_eq!(names("https"), ["http_fetch"]); + assert!(names("echo").is_empty(), "echo is a test tool only"); + + // The argument schemas are section 3's table: exactly these properties, all strings. + let table: [(&str, &[&str], &[&str]); 4] = [ + ("read_file", &["path"], &["path"]), + ("write_file", &["content", "path"], &["path", "content"]), + ("shell", &["command", "cwd"], &["command"]), + ("http_fetch", &["url"], &["url"]), + ]; + for (name, properties, required) in table { + let entry = r.get(name).unwrap_or_else(|| panic!("{name} is missing")); + assert!(!entry.core, "{name} is found with find_tool, not declared"); + let p = &entry.schema.parameters; + assert_eq!(p["type"], "object", "{name}"); + let mut got: Vec<&str> = p["properties"] + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect(); + got.sort_unstable(); + assert_eq!(got, properties, "{name}: properties"); + for property in properties { + assert_eq!( + p["properties"][property]["type"], "string", + "{name}.{property}" + ); + assert!( + p["properties"][property]["description"].is_string(), + "{name}.{property} needs a description" + ); + } + assert_eq!( + p["required"], + serde_json::json!(required), + "{name}: required" + ); + } + + // call_tool lets the four through to the port and nothing else. + match dispatch( + &r, + "call_tool", + r#"{"name":"shell","arguments":{"command":"ls"}}"#, + ) { + Dispatch::Port { tool, arguments } => { + assert_eq!(tool, "shell"); + assert_eq!(arguments, r#"{"command":"ls"}"#); + } + other => panic!("{other:?}"), + } + assert!(matches!( + dispatch(&r, "call_tool", r#"{"name":"echo","arguments":{}}"#), + Dispatch::Local(t) if t.contains("No tool named \"echo\"") + )); +} + +#[test] +fn every_deny_reason_has_its_sentence() { + let table = [ + (DenyReason::NoGrant, "Denied: no grant allows this call."), + ( + DenyReason::GrantExpired, + "Denied: the grant for this call has expired.", + ), + ( + DenyReason::TaintTooHigh, + "Denied: this session has seen data too sensitive for this call.", + ), + ( + DenyReason::DeniedByGrant, + "Denied: a grant forbids this call.", + ), + ( + DenyReason::ApprovalRefused, + "Denied: the owner refused this call.", + ), + ( + DenyReason::ApprovalExpired, + "Denied: the approval request expired without an answer.", + ), + ( + DenyReason::InvalidArguments, + "Denied: the arguments are not valid for this tool.", + ), + ( + DenyReason::GrantsInvalid, + "Denied: the grant files have an error; the owner has been told.", + ), + ( + DenyReason::AuditUnavailable, + "Denied: the audit log cannot be written; the owner has been told.", + ), + ( + DenyReason::StateUnreadable, + "Denied: this session's broker state is damaged; the owner has been told.", + ), + ]; + for (reason, want) in table { + assert_eq!(denial_text(reason), want, "{reason:?}"); + } } #[test] @@ -134,36 +271,37 @@ fn results_are_cut_on_a_character_boundary_and_marked() { } #[test] -fn fake_tools_answer_clock_and_echo_and_record_calls() { +fn fake_tools_answer_echo_deny_the_rest_and_record_calls() { let fake = FakeTools::new(); - match fake.call(&req("clock", "{}")) { + let mut seen = 0; + let mut on_pending = |_: &Pending| seen += 1; + match fake.call(&req("echo", r#"{"text":"box"}"#), &mut on_pending) { ToolResponse::Result { content, class, untrusted, truncated, } => { - assert!( - proto::Timestamp::parse(&content).is_ok(), - "an RFC 3339 time: {content}" - ); + assert_eq!(content, "box"); assert_eq!(class, proto::DataClass::Public); assert!(!untrusted && !truncated); } other => panic!("{other:?}"), } - match fake.call(&req("echo", r#"{"text":"box"}"#)) { - ToolResponse::Result { content, .. } => assert_eq!(content, "box"), - other => panic!("{other:?}"), - } assert!(matches!( - fake.call(&req("echo", r#"{"tex":"box"}"#)), + fake.call(&req("echo", r#"{"tex":"box"}"#), &mut on_pending), ToolResponse::Failed { .. } )); - assert!(matches!( - fake.call(&req("weather", "{}")), - ToolResponse::Denied { .. } - )); - assert_eq!(fake.calls().len(), 4); - assert_eq!(fake.calls()[1].tool, "echo"); + for tool in ["weather", "read_file", "clock"] { + assert_eq!( + fake.call(&req(tool, "{}"), &mut on_pending), + ToolResponse::Denied { + reason: DenyReason::NoGrant + }, + "{tool}: the clock is loopd's own now, not the port's" + ); + } + assert_eq!(seen, 0, "the fake never asks for approval"); + assert_eq!(fake.calls().len(), 5); + assert_eq!(fake.calls()[0].tool, "echo"); } diff --git a/crates/loopd/tests/turn.rs b/crates/loopd/tests/turn.rs index 110577e..7433ebe 100644 --- a/crates/loopd/tests/turn.rs +++ b/crates/loopd/tests/turn.rs @@ -1,5 +1,6 @@ //! Tests for one turn: record sequences and tool dispatch. Do not edit. -//! The limits and the append-only property are in `limits.rs`. +//! The limits and the append-only property are in `limits.rs`; denials and approvals are in +//! `turn_broker.rs`. mod support; #[path = "support/turn.rs"] @@ -264,7 +265,7 @@ fn the_result_cap_applies_when_appended() { } #[test] -fn tool_failures_and_denials_become_results_the_model_can_read() { +fn a_tool_failure_becomes_a_result_the_model_can_read() { let s = setup(vec![ToolResponse::Failed { message: "disk on fire".to_string(), }]); @@ -273,8 +274,24 @@ fn tool_failures_and_denials_become_results_the_model_can_read() { vec![Reply::fixture("tool_call"), Reply::fixture("plain")], ); let mut session = s.session("a"); - assert!(s.turn(&mut session, "x").0.is_ok()); + let (result, events) = s.turn(&mut session, "x"); + assert!(result.is_ok(), "{result:?}"); + match &session.records()[4] { + LogRecord::ToolResult { + content, + class, + untrusted, + .. + } => { + assert_eq!(content, "The tool failed: disk on fire"); + assert_eq!((*class, *untrusted), (DataClass::Public, false)); + } + other => panic!("{other:?}"), + } assert!( - matches!(&session.records()[4], LogRecord::ToolResult { content, class: DataClass::Public, .. } if content.contains("disk on fire")) + !events + .iter() + .any(|e| matches!(e, TurnEvent::ToolDenied { .. })), + "a failure is not a denial" ); } diff --git a/crates/loopd/tests/turn_broker.rs b/crates/loopd/tests/turn_broker.rs new file mode 100644 index 0000000..ba43fce --- /dev/null +++ b/crates/loopd/tests/turn_broker.rs @@ -0,0 +1,222 @@ +//! Tests for what the turn loop does with the broker's answers: denials, pending approvals, and +//! a port that misbehaves. Do not edit. + +mod support; +#[path = "support/turn.rs"] +mod turn_support; + +use proto::{DataClass, DenyReason, LogRecord, Timestamp, ToolResponse, TurnEvent}; +use support::{Reply, ReturnsPendingPort, ok_result, pending}; +use turn_support::{setup, types}; + +const CHAT: &str = "/v1/chat/completions"; + +/// The tool events of a turn, in order, as short strings. +fn tool_events(events: &[TurnEvent]) -> Vec { + events + .iter() + .filter_map(|e| match e { + TurnEvent::ToolCallStarted { name } => Some(format!("started {name}")), + TurnEvent::ApprovalPending { approval, tool, .. } => { + Some(format!("pending {approval} {tool}")) + } + TurnEvent::ToolDenied { name, reason } => Some(format!("denied {name} {reason:?}")), + TurnEvent::ToolResult { name, .. } => Some(format!("result {name}")), + _ => None, + }) + .collect() +} + +#[test] +fn a_denial_is_a_fixed_sentence_for_the_model_and_an_event_for_the_owner() { + let s = setup(vec![ToolResponse::Denied { + reason: DenyReason::TaintTooHigh, + }]); + s.server.route( + CHAT, + vec![Reply::fixture("tool_call"), Reply::fixture("plain")], + ); + let mut session = s.session("a"); + let (result, events) = s.turn(&mut session, "x"); + assert!( + result.is_ok(), + "the turn goes on after a denial: {result:?}" + ); + match &session.records()[4] { + LogRecord::ToolResult { + content, + class, + untrusted, + truncated, + .. + } => { + assert_eq!( + content, + "Denied: this session has seen data too sensitive for this call." + ); + assert_eq!( + (*class, *untrusted, *truncated), + (DataClass::Public, false, false) + ); + } + other => panic!("{other:?}"), + } + assert_eq!( + tool_events(&events), + [ + "started read_file", + "denied read_file TaintTooHigh", + "result read_file" + ], + "the denial comes before the result" + ); + // The model reads the sentence in the next request. + let m2 = s.server.requests_to(CHAT)[1].json(); + assert_eq!( + m2["messages"][3]["content"], + "Denied: this session has seen data too sensitive for this call." + ); +} + +#[test] +fn a_denied_call_tool_names_the_target_tool_in_the_denial() { + let s = setup(vec![ToolResponse::Denied { + reason: DenyReason::NoGrant, + }]); + s.server.route( + CHAT, + vec![Reply::fixture("call_tool"), Reply::fixture("plain")], + ); + let mut session = s.session("a"); + let (result, events) = s.turn(&mut session, "echo box"); + assert!(result.is_ok(), "{result:?}"); + assert_eq!( + tool_events(&events), + [ + "started call_tool", + "denied echo NoGrant", + "result call_tool" + ], + "the owner writes grants for `echo`, not for `call_tool`" + ); + assert!( + matches!(&session.records()[4], LogRecord::ToolResult { content, .. } if content == "Denied: no grant allows this call.") + ); +} + +#[test] +fn a_pending_approval_is_an_event_and_the_answer_after_it_is_the_result() { + let s = setup(vec![ + pending(41, "2026-09-18T12:15:00.000Z"), + ok_result("straylight\n"), + ]); + s.server.route( + CHAT, + vec![Reply::fixture("tool_call"), Reply::fixture("plain")], + ); + let mut session = s.session("a"); + let (result, events) = s.turn(&mut session, "x"); + assert!(result.is_ok(), "{result:?}"); + assert_eq!( + tool_events(&events), + [ + "started read_file", + "pending 41 read_file", + "result read_file" + ] + ); + let expires = events.iter().find_map(|e| match e { + TurnEvent::ApprovalPending { expires, .. } => Some(*expires), + _ => None, + }); + assert_eq!( + expires, + Some(Timestamp::parse("2026-09-18T12:15:00.000Z").unwrap()) + ); + assert_eq!(s.port.calls().len(), 1, "one call, however long it waited"); + assert_eq!( + types(session.records()), + [ + "start", + "user", + "assistant", + "usage", + "tool_result", + "assistant", + "usage" + ], + "waiting writes nothing to the log" + ); + assert!( + matches!(&session.records()[4], LogRecord::ToolResult { content, .. } if content == "straylight\n") + ); +} + +#[test] +fn a_pending_approval_that_ends_in_a_refusal() { + let s = setup(vec![ + pending(7, "2026-09-18T12:15:00.000Z"), + ToolResponse::Denied { + reason: DenyReason::ApprovalRefused, + }, + ]); + s.server.route( + CHAT, + vec![Reply::fixture("tool_call"), Reply::fixture("plain")], + ); + let mut session = s.session("a"); + let (result, events) = s.turn(&mut session, "x"); + assert!(result.is_ok(), "{result:?}"); + assert_eq!( + tool_events(&events), + [ + "started read_file", + "pending 7 read_file", + "denied read_file ApprovalRefused", + "result read_file" + ] + ); + assert!( + matches!(&session.records()[4], LogRecord::ToolResult { content, .. } if content == "Denied: the owner refused this call.") + ); +} + +#[test] +fn a_port_that_returns_a_pending_frame_as_its_answer_is_a_failure_not_a_decision() { + let s = setup(vec![]); + s.server.route( + CHAT, + vec![Reply::fixture("tool_call"), Reply::fixture("plain")], + ); + let mut session = s.session("a"); + let runtime = loopd::turn::Runtime { + cfg: &s.cfg, + client: &s.client, + port: &ReturnsPendingPort, + registry: &s.registry, + }; + let mut events = Vec::new(); + let result = + loopd::turn::run_turn(&mut session, &runtime, "x", &mut |e| events.push(e.clone())); + assert!(result.is_ok(), "{result:?}"); + match &session.records()[4] { + LogRecord::ToolResult { + content, + class, + untrusted, + .. + } => { + assert_eq!( + content, + "The tool failed: the tool broker gave no final answer" + ); + assert_eq!((*class, *untrusted), (DataClass::Public, false)); + } + other => panic!("{other:?}"), + } + assert_eq!( + tool_events(&events), + ["started read_file", "result read_file"], + "neither pending nor denied: the port said neither" + ); +} diff --git a/docs/implementer-log.md b/docs/implementer-log.md index ec0621a..8a8126c 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -57,6 +57,7 @@ reviewer adds findings under "Reviews" once per milestone. | M3a/11-brokerd-approvals | 2026-09-19 | done | 1 | pass | none | Wrote crates/brokerd/src/approvals.rs: Verdict { Run(Box), Denied(DenyReason) } (boxed for clippy's large_enum_variant), Entry { info, ask, reply: Sender }, and Table { entries: Mutex> } with a single private `lock()` helper that takes the mutex and recovers a poisoned guard with `unwrap_or_else(|p| p.into_inner())`. `insert` makes a channel and stores the Entry under `info.approval` returning the receiver; `take` removes under the lock and returns the Entry (so the non-Clone `Ask` is not cloned); `take_expired` holds one lock, collects the ids where `now >= expires` (BTreeMap `values()` already yields id order, so no per-id lock to race), removes each, returns them in id order; `list` clones every `info` in id order. No method sends on `reply`. 7 approvals tests pass five runs in a row; `make gate` prints `gate: ok`. | ? | Wrote `crates/brokerd/src/runner.rs`: `Mount` (path/writable), `RunSpec` (four private fields in the given order, no constructor, no Clone, four getters, `egress` as `as_deref`), `RunOutput`, `RunError` (Failed/Unavailable), `Runtime` (Send+Sync), `Refusing` (every call `Err(Unavailable(REFUSING))`), and `run(decision, &dyn Runtime)`. `run` takes `decision.label()` first, builds the spec (`tool = args().tool()`, `arguments = args().clone()`), then the mounts/egress table is one `match` on the `ToolArgs` variant with no `_` arm: ReadFile mounts the matched path read-only, WriteFile writable, Shell mounts every grant path in order writable, HttpFetch mounts nothing and carries `Some(hosts())`; only HttpFetch has network. `Ok(output)` -> `ToolResponse::Result` with the label's class/untrusted; `Err(Failed|Unavailable(m))` -> `Failed { message: m }` unchanged. Added the two module doctests (compile_fail proving the fields private, compiling through proving `tool()` public). 8 runner tests pass; 9 doctests pass; `make gate` prints `gate: ok`. Proved the compile_fail doctest has teeth: making the four RunSpec fields `pub` made it fail ("test compiled"), reverting to private made it pass again. | ? | | M3a/14-brokerd-admin | 2026-09-20 | done | 1 | pass | none | Wrote crates/brokerd/src/admin.rs (129 lines): BY ("bxctl") and REFUSAL_INTERNAL ("the refusal could not be recorded; the call is denied; see docs/runbook.md#audit-unavailable"); answer (grants as they are now, ledger.answer(ask, info.approval, answer, &grants, now) -> Answered { verdict, outcome }, reply.send(verdict) logging "brokerd: approval {id} was answered after its requester had gone" on Err, return outcome); expire_due (take_expired(now), answer(.., Answer::Expired, now) each, return count); handle (read_request None -> return; by kind: Approvals -> ApprovalList{table().list()}, Approve/Refuse -> table().take(approval) None -> NoSuchApproval "approval {id} is not pending" else answer and ApproveResult{outcome}/Ok(Empty {}), a Refuse whose outcome is not Denied{ApprovalRefused} -> Internal with REFUSAL_INTERNAL, CheckGrants -> grants::load(cfg.paths.grants) Err's list or empty -> GrantsReport{problems}, anything else -> forbid on admin.sock); copied crates/brokerd/tests/admin.rs byte-identical (support/ already identical), added pub mod admin; to lib.rs. All twelve admin tests pass five runs in a row; make gate prints gate: ok. Two compiler-necessary deviations from the interface shorthand: handle takes `mut stream` (the task wrote `stream`, but read_request/forbid/send take &mut, and `mut` is not part of the fn(UnixStream,&Broker) type the test passes); Answer::Approved/Refused use `Some(BY.to_string())` because ledger::Answer::by is Option. | ? | | M3a/15-brokerd-serve | 2026-09-20 | done | 2 | fail | none | Copied crates/brokerd/tests/serve.rs byte-identical. Wrote crates/brokerd/src/serve.rs: ServeError { Audit(AuditError), Dir(PathBuf, io::Error), Socket(PathBuf, io::Error) } with Display (Audit is the audit error's text; Dir/Socket are "cannot prepare"/"cannot listen on" {path}.display() + ": {e}") and source() returning the inner error for all three; Started { pub broker, pub recovered, pub accepted, private tools/admin }; start opens the audit lock first so nothing is made on failure, binds both sockets in order, and builds the ledger and broker each through its own box over the one log Arc; listen makes the socket's dir at 0700, sets 0700 on it always (even when pre-existing), removes a stale socket (NotFound ignored), binds, sets 0600 on the socket; run spawns the once-a-second expiry thread and one accept thread per socket (a handler per stream, the first listener error sent on an mpsc channel, run returns it) — serve is the only module that starts threads; main.rs parses `serve --config [--accept-break]` (exit 2 on a bad list), reads config before touching disk (exit 1), routes NothingToAccept to exit 2 and every other start error to exit 1, prints the recovered/accepted/serving notices to stderr. Added `pub mod serve;` to lib.rs. All 9 serve tests plus the rest of brokerd pass; `make gate` prints `gate: ok`. First `make gate` failed at `cargo fmt --all --check` (edits landed after fmt); passed on the second run after re-running `cargo fmt`. | ? | +| M3a/16-loopd-tools | 2026-09-20 | done | 1 | pass | none | Added `Pending { approval, expires }` and the `ToolPort::call` two-argument form (`on_pending`) in tools.rs; `Registry::m3a()` with clock (core) then read_file/write_file/shell/http_fetch (all core: false), `m2b()` retitled the test registry; the four broker schemas (`read_file_schema`/`write_file_schema`/`shell_schema`/`http_fetch_schema`) with the exact descriptions and required lists; `CLOCK` const and the clock arm in `dispatch` returning `Timestamp::now()` locally; `denial_text` covering all ten DenyReason variants with no `_` arm; `FakeTools::call` took `_on_pending` and lost its clock arm so clock falls through to Denied{NoGrant}. turn.rs: `run_call` gained an `on_event` param, the Dispatch::Port arm calls `port.call` with the on_pending closure that emits `ApprovalPending` (named by `request.tool`) and the four response arms (Result/Failed/Denied emits `ToolDenied` by `request.tool` then `denial_text`, PendingApproval the fixed "no final answer" text); main.rs `Registry::m3a()`. Copied tests/tools.rs, turn.rs, turn_broker.rs, tests/support/mod.rs byte-identical. tools 10, turn 6, turn_broker 5, limits 9, channel 6; `make gate` prints `gate: ok`. | ? | ## Reviews