From e8568edf7e897efceb7c97ee1eef6d16b05fcc76 Mon Sep 17 00:00:00 2001 From: "K. Isom" Date: Sat, 19 Sep 2026 00:28:46 -0700 Subject: [PATCH] Add the admin messages, approval ids as numbers, and two turn events Implemented-By: OpenCode session (model recorded in docs/implementer-log.md) --- crates/bxctl/src/chat.rs | 4 + crates/proto/src/lib.rs | 5 +- crates/proto/src/wire.rs | 85 +++++- crates/proto/tests/admin_wire.rs | 257 ++++++++++++++++++ .../tests/fixtures/wire/approval_list.json | 1 + .../fixtures/wire/approval_list_empty.json | 1 + .../proto/tests/fixtures/wire/approvals.json | 1 + crates/proto/tests/fixtures/wire/approve.json | 1 + .../fixtures/wire/approve_result_allowed.json | 1 + .../fixtures/wire/approve_result_denied.json | 1 + .../tests/fixtures/wire/check_grants.json | 1 + .../tests/fixtures/wire/error_forbidden.json | 1 + .../fixtures/wire/error_no_such_approval.json | 1 + .../tests/fixtures/wire/grants_report.json | 1 + .../tests/fixtures/wire/grants_report_ok.json | 1 + crates/proto/tests/fixtures/wire/ok.json | 1 + crates/proto/tests/fixtures/wire/refuse.json | 1 + .../tests/fixtures/wire/refuse_no_reason.json | 1 + .../fixtures/wire/tool_response_pending.json | 2 +- .../wire/turn_event_approval_pending.json | 1 + .../fixtures/wire/turn_event_tool_denied.json | 1 + crates/proto/tests/strict.rs | 26 +- crates/proto/tests/turn_wire.rs | 36 ++- crates/proto/tests/wire.rs | 19 +- docs/implementer-log.md | 1 + 25 files changed, 441 insertions(+), 10 deletions(-) create mode 100644 crates/proto/tests/admin_wire.rs create mode 100644 crates/proto/tests/fixtures/wire/approval_list.json create mode 100644 crates/proto/tests/fixtures/wire/approval_list_empty.json create mode 100644 crates/proto/tests/fixtures/wire/approvals.json create mode 100644 crates/proto/tests/fixtures/wire/approve.json create mode 100644 crates/proto/tests/fixtures/wire/approve_result_allowed.json create mode 100644 crates/proto/tests/fixtures/wire/approve_result_denied.json create mode 100644 crates/proto/tests/fixtures/wire/check_grants.json create mode 100644 crates/proto/tests/fixtures/wire/error_forbidden.json create mode 100644 crates/proto/tests/fixtures/wire/error_no_such_approval.json create mode 100644 crates/proto/tests/fixtures/wire/grants_report.json create mode 100644 crates/proto/tests/fixtures/wire/grants_report_ok.json create mode 100644 crates/proto/tests/fixtures/wire/ok.json create mode 100644 crates/proto/tests/fixtures/wire/refuse.json create mode 100644 crates/proto/tests/fixtures/wire/refuse_no_reason.json create mode 100644 crates/proto/tests/fixtures/wire/turn_event_approval_pending.json create mode 100644 crates/proto/tests/fixtures/wire/turn_event_tool_denied.json diff --git a/crates/bxctl/src/chat.rs b/crates/bxctl/src/chat.rs index a47ebc0..1112a5f 100644 --- a/crates/bxctl/src/chat.rs +++ b/crates/bxctl/src/chat.rs @@ -51,6 +51,8 @@ fn code_name(code: ErrorCode) -> &'static str { ErrorCode::NoSuchSession => "no such session", ErrorCode::SessionExists => "session exists", ErrorCode::Inference => "inference", + ErrorCode::Forbidden => "forbidden", + ErrorCode::NoSuchApproval => "no such approval", } } @@ -193,6 +195,8 @@ impl Printer { writeln!(out, "[cache loss: {got} of {expected}]")?; } TurnEvent::Queued { .. } | TurnEvent::Progress { .. } => {} + // Printed from task 20 on; until then these events are not sent. + TurnEvent::ApprovalPending { .. } | TurnEvent::ToolDenied { .. } => {} } out.flush() } diff --git a/crates/proto/src/lib.rs b/crates/proto/src/lib.rs index 1025f3e..e6db5f6 100644 --- a/crates/proto/src/lib.rs +++ b/crates/proto/src/lib.rs @@ -17,6 +17,7 @@ pub use hash::{HashError, Sha256, sha256}; pub use ids::{CallId, Epoch, Hash32, SessionId, Timestamp, ValueError}; pub use log::{LogRecord, ToolCall, Usage}; pub use wire::{ - DenyReason, Envelope, ErrorCode, Message, PROTOCOL_VERSION, ToolRequest, ToolResponse, Turn, - TurnDone, TurnEvent, WireError, + ApprovalList, Approve, ApproveResult, DenyReason, Empty, Envelope, ErrorCode, GrantProblem, + GrantsReport, Message, PROTOCOL_VERSION, PendingApproval, Refuse, ToolRequest, ToolResponse, + Turn, TurnDone, TurnEvent, WireError, }; diff --git a/crates/proto/src/wire.rs b/crates/proto/src/wire.rs index 5bf37f9..354d814 100644 --- a/crates/proto/src/wire.rs +++ b/crates/proto/src/wire.rs @@ -1,6 +1,6 @@ use serde::{Deserialize, Serialize}; -use crate::{CallId, DataClass, SessionId, Timestamp, Usage}; +use crate::{CallId, DataClass, DecisionRecord, SessionId, Timestamp, Usage}; pub const PROTOCOL_VERSION: u32 = 1; @@ -27,6 +27,14 @@ pub enum Message { Turn(Turn), TurnEvent(TurnEvent), TurnDone(TurnDone), + Approvals(Empty), + ApprovalList(ApprovalList), + Approve(Approve), + ApproveResult(ApproveResult), + Refuse(Refuse), + Ok(Empty), + CheckGrants(Empty), + GrantsReport(GrantsReport), } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -49,6 +57,8 @@ pub enum ErrorCode { NoSuchSession, SessionExists, Inference, + Forbidden, + NoSuchApproval, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -64,7 +74,7 @@ pub struct ToolRequest { #[serde(tag = "status", deny_unknown_fields, rename_all = "snake_case")] pub enum ToolResponse { PendingApproval { - approval: String, + approval: u64, expires: Timestamp, }, Result { @@ -90,6 +100,10 @@ pub enum DenyReason { DeniedByGrant, ApprovalRefused, ApprovalExpired, + GrantsInvalid, + AuditUnavailable, + InvalidArguments, + StateUnreadable, } // JSON: {"kind":"turn","body":{"session":"…","content":"…","resume":false}} @@ -142,6 +156,15 @@ pub enum TurnEvent { expected: u64, got: u64, }, + ApprovalPending { + approval: u64, + tool: String, + expires: Timestamp, + }, + ToolDenied { + name: String, + reason: DenyReason, + }, } // JSON: {"kind":"turn_done","body":{"content":"…","usage":{…}}} @@ -151,3 +174,61 @@ pub struct TurnDone { pub content: String, pub usage: Usage, } + +// The admin messages on admin.sock. JSON: {"kind":"approvals","body":{}} +#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Empty {} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct PendingApproval { + pub approval: u64, + pub session: SessionId, + pub call: CallId, + pub tool: String, + pub arguments: String, + pub grant: String, + pub taint: DataClass, + pub created: Timestamp, + pub expires: Timestamp, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ApprovalList { + pub items: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Approve { + pub approval: u64, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ApproveResult { + pub outcome: DecisionRecord, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Refuse { + pub approval: u64, + pub reason: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct GrantProblem { + pub file: String, + pub line: Option, + pub problem: String, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct GrantsReport { + pub problems: Vec, +} diff --git a/crates/proto/tests/admin_wire.rs b/crates/proto/tests/admin_wire.rs new file mode 100644 index 0000000..d58ebdf --- /dev/null +++ b/crates/proto/tests/admin_wire.rs @@ -0,0 +1,257 @@ +//! Tests for the admin messages of `admin.sock` and the two new error codes, against byte-exact +//! fixtures. Do not edit these or the fixtures. + +use proto::{ + ApprovalList, Approve, ApproveResult, CallId, DataClass, DecisionRecord, DenyReason, Empty, + Envelope, ErrorCode, GrantProblem, GrantsReport, Message, PendingApproval, Refuse, SessionId, + Timestamp, WireError, +}; + +fn fixture(name: &str) -> String { + let path = format!("{}/tests/fixtures/wire/{name}", env!("CARGO_MANIFEST_DIR")); + let text = std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("{path}: {e}")); + text.trim_end_matches('\n').to_string() +} + +/// The fixture must decode to `want`, and `want` must encode to exactly the fixture's bytes. +fn check(name: &str, id: u64, msg: Message) { + let want = Envelope { + v: 1, + id, + r#final: true, + msg, + }; + let text = fixture(name); + let got: Envelope = serde_json::from_str(&text).unwrap_or_else(|e| panic!("{name}: {e}")); + assert_eq!(got, want, "{name}: decoded value"); + assert_eq!( + serde_json::to_string(&want).unwrap(), + text, + "{name}: encoded bytes" + ); +} + +fn pending() -> PendingApproval { + PendingApproval { + approval: 41, + session: SessionId::new("chat-1789700000-42").unwrap(), + call: CallId(3), + tool: "shell".to_string(), + arguments: r#"{"command":"rm -rf /home/kyle/scratch/build","cwd":"/home/kyle/scratch"}"# + .to_string(), + grant: "shell-scratch".to_string(), + taint: DataClass::Private, + created: Timestamp::parse("2026-09-18T08:05:00.000Z").unwrap(), + expires: Timestamp::parse("2026-09-18T08:20:00.000Z").unwrap(), + } +} + +#[test] +fn the_three_requests_with_an_empty_body() { + check("approvals.json", 5, Message::Approvals(Empty {})); + check("check_grants.json", 8, Message::CheckGrants(Empty {})); + check("ok.json", 7, Message::Ok(Empty {})); +} + +#[test] +fn approval_list() { + let list = ApprovalList { + items: vec![pending()], + }; + check("approval_list.json", 5, Message::ApprovalList(list)); + let empty = ApprovalList { items: Vec::new() }; + check("approval_list_empty.json", 5, Message::ApprovalList(empty)); +} + +#[test] +fn approve_and_its_result() { + check( + "approve.json", + 6, + Message::Approve(Approve { approval: 41 }), + ); + check( + "approve_result_allowed.json", + 6, + Message::ApproveResult(ApproveResult { + outcome: DecisionRecord::Allowed {}, + }), + ); + check( + "approve_result_denied.json", + 6, + Message::ApproveResult(ApproveResult { + outcome: DecisionRecord::Denied { + reason: DenyReason::NoGrant, + }, + }), + ); +} + +#[test] +fn refuse_with_and_without_a_reason() { + check( + "refuse.json", + 7, + Message::Refuse(Refuse { + approval: 41, + reason: Some("not while I am away".to_string()), + }), + ); + check( + "refuse_no_reason.json", + 7, + Message::Refuse(Refuse { + approval: 41, + reason: None, + }), + ); +} + +#[test] +fn grants_report() { + let report = GrantsReport { + problems: vec![ + GrantProblem { + file: "notes-read.toml".to_string(), + line: Some(3), + problem: "unknown field `mdoe`".to_string(), + }, + GrantProblem { + file: "Bad_Name.toml".to_string(), + line: None, + problem: "the file name is not a valid grant id".to_string(), + }, + ], + }; + check("grants_report.json", 8, Message::GrantsReport(report)); + let ok = GrantsReport { + problems: Vec::new(), + }; + check("grants_report_ok.json", 8, Message::GrantsReport(ok)); +} + +#[test] +fn the_two_new_error_codes() { + check( + "error_forbidden.json", + 9, + Message::Error(WireError { + code: ErrorCode::Forbidden, + detail: "approve is not accepted on broker.sock".to_string(), + }), + ); + check( + "error_no_such_approval.json", + 6, + Message::Error(WireError { + code: ErrorCode::NoSuchApproval, + detail: "41".to_string(), + }), + ); +} + +/// An empty body is an object with no keys: not `null`, not a missing body, not an object with a +/// key in it. +#[test] +fn an_empty_body_must_be_an_empty_object() { + let good = fixture("approvals.json"); + assert!(serde_json::from_str::(&good).is_ok()); + let bad = [ + good.replacen("\"body\":{}", "\"body\":null", 1), + good.replacen(",\"body\":{}", "", 1), + good.replacen("\"body\":{}", "\"body\":{\"all\":true}", 1), + ]; + for text in bad { + assert_ne!(text, good); + assert!( + serde_json::from_str::(&text).is_err(), + "accepted {text}" + ); + } +} + +/// `reason` and `line` may be null but may not be left out: every field is always written, so a +/// reader never has to guess what a missing one means. +#[test] +fn optional_fields_are_null_not_absent() { + let refuse = fixture("refuse_no_reason.json"); + let cut = refuse.replacen(",\"reason\":null", "", 1); + assert_ne!(cut, refuse); + assert!( + serde_json::from_str::(&cut).is_ok(), + "serde reads a missing Option as None; this documents it" + ); + assert!( + serde_json::to_string(&Refuse { + approval: 1, + reason: None + }) + .unwrap() + .contains("\"reason\":null") + ); + assert!( + serde_json::to_string(&GrantProblem { + file: "a.toml".to_string(), + line: None, + problem: "x".to_string() + }) + .unwrap() + .contains("\"line\":null") + ); +} + +/// An outcome is strict too. serde does not apply `deny_unknown_fields` to unit variants such as +/// `allowed`, so `DecisionRecord` must not rely on the derive for it. +#[test] +fn an_outcome_rejects_unknown_and_misplaced_fields() { + for good in [ + r#"{"outcome":"allowed"}"#, + r#"{"outcome":"ask"}"#, + r#"{"outcome":"denied","reason":"no_grant"}"#, + ] { + let value: DecisionRecord = serde_json::from_str(good).unwrap(); + assert_eq!(serde_json::to_string(&value).unwrap(), good); + } + for bad in [ + r#"{"outcome":"allowed","zz":1}"#, + r#"{"outcome":"ask","zz":1}"#, + r#"{"outcome":"denied","reason":"no_grant","zz":1}"#, + r#"{"outcome":"allowed","reason":"no_grant"}"#, + r#"{"outcome":"ask","reason":null,"zz":1}"#, + r#"{"outcome":"allowed","reason":null}"#, + r#"{"outcome":"denied"}"#, + r#"{"outcome":"approved"}"#, + r#"{"reason":"no_grant"}"#, + ] { + assert!( + serde_json::from_str::(bad).is_err(), + "accepted {bad}" + ); + } +} + +#[test] +fn pending_approvals_reject_bad_values() { + let good = fixture("approval_list.json"); + assert!(serde_json::from_str::(&good).is_ok()); + let bad = [ + // an approval id is a number + good.replacen("\"approval\":41", "\"approval\":\"41\"", 1), + // the session id is validated + good.replacen("chat-1789700000-42", "../etc", 1), + // the taint is one of the three classes + good.replacen("\"taint\":\"private\"", "\"taint\":\"internal\"", 1), + // a field is missing + good.replacen("\"grant\":\"shell-scratch\",", "", 1), + // a field nobody defined + good.replacen("\"grant\":", "\"note\":1,\"grant\":", 1), + ]; + for text in bad { + assert_ne!(text, good); + assert!( + serde_json::from_str::(&text).is_err(), + "accepted {text}" + ); + } +} diff --git a/crates/proto/tests/fixtures/wire/approval_list.json b/crates/proto/tests/fixtures/wire/approval_list.json new file mode 100644 index 0000000..cd43c81 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/approval_list.json @@ -0,0 +1 @@ +{"v":1,"id":5,"final":true,"msg":{"kind":"approval_list","body":{"items":[{"approval":41,"session":"chat-1789700000-42","call":3,"tool":"shell","arguments":"{\"command\":\"rm -rf /home/kyle/scratch/build\",\"cwd\":\"/home/kyle/scratch\"}","grant":"shell-scratch","taint":"private","created":"2026-09-18T08:05:00.000Z","expires":"2026-09-18T08:20:00.000Z"}]}}} diff --git a/crates/proto/tests/fixtures/wire/approval_list_empty.json b/crates/proto/tests/fixtures/wire/approval_list_empty.json new file mode 100644 index 0000000..81da4a9 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/approval_list_empty.json @@ -0,0 +1 @@ +{"v":1,"id":5,"final":true,"msg":{"kind":"approval_list","body":{"items":[]}}} diff --git a/crates/proto/tests/fixtures/wire/approvals.json b/crates/proto/tests/fixtures/wire/approvals.json new file mode 100644 index 0000000..012a021 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/approvals.json @@ -0,0 +1 @@ +{"v":1,"id":5,"final":true,"msg":{"kind":"approvals","body":{}}} diff --git a/crates/proto/tests/fixtures/wire/approve.json b/crates/proto/tests/fixtures/wire/approve.json new file mode 100644 index 0000000..2010b9f --- /dev/null +++ b/crates/proto/tests/fixtures/wire/approve.json @@ -0,0 +1 @@ +{"v":1,"id":6,"final":true,"msg":{"kind":"approve","body":{"approval":41}}} diff --git a/crates/proto/tests/fixtures/wire/approve_result_allowed.json b/crates/proto/tests/fixtures/wire/approve_result_allowed.json new file mode 100644 index 0000000..467b6e4 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/approve_result_allowed.json @@ -0,0 +1 @@ +{"v":1,"id":6,"final":true,"msg":{"kind":"approve_result","body":{"outcome":{"outcome":"allowed"}}}} diff --git a/crates/proto/tests/fixtures/wire/approve_result_denied.json b/crates/proto/tests/fixtures/wire/approve_result_denied.json new file mode 100644 index 0000000..5497161 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/approve_result_denied.json @@ -0,0 +1 @@ +{"v":1,"id":6,"final":true,"msg":{"kind":"approve_result","body":{"outcome":{"outcome":"denied","reason":"no_grant"}}}} diff --git a/crates/proto/tests/fixtures/wire/check_grants.json b/crates/proto/tests/fixtures/wire/check_grants.json new file mode 100644 index 0000000..6471af8 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/check_grants.json @@ -0,0 +1 @@ +{"v":1,"id":8,"final":true,"msg":{"kind":"check_grants","body":{}}} diff --git a/crates/proto/tests/fixtures/wire/error_forbidden.json b/crates/proto/tests/fixtures/wire/error_forbidden.json new file mode 100644 index 0000000..601f644 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/error_forbidden.json @@ -0,0 +1 @@ +{"v":1,"id":9,"final":true,"msg":{"kind":"error","body":{"code":"forbidden","detail":"approve is not accepted on broker.sock"}}} diff --git a/crates/proto/tests/fixtures/wire/error_no_such_approval.json b/crates/proto/tests/fixtures/wire/error_no_such_approval.json new file mode 100644 index 0000000..9bbeca8 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/error_no_such_approval.json @@ -0,0 +1 @@ +{"v":1,"id":6,"final":true,"msg":{"kind":"error","body":{"code":"no_such_approval","detail":"41"}}} diff --git a/crates/proto/tests/fixtures/wire/grants_report.json b/crates/proto/tests/fixtures/wire/grants_report.json new file mode 100644 index 0000000..a46303f --- /dev/null +++ b/crates/proto/tests/fixtures/wire/grants_report.json @@ -0,0 +1 @@ +{"v":1,"id":8,"final":true,"msg":{"kind":"grants_report","body":{"problems":[{"file":"notes-read.toml","line":3,"problem":"unknown field `mdoe`"},{"file":"Bad_Name.toml","line":null,"problem":"the file name is not a valid grant id"}]}}} diff --git a/crates/proto/tests/fixtures/wire/grants_report_ok.json b/crates/proto/tests/fixtures/wire/grants_report_ok.json new file mode 100644 index 0000000..af1dcc2 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/grants_report_ok.json @@ -0,0 +1 @@ +{"v":1,"id":8,"final":true,"msg":{"kind":"grants_report","body":{"problems":[]}}} diff --git a/crates/proto/tests/fixtures/wire/ok.json b/crates/proto/tests/fixtures/wire/ok.json new file mode 100644 index 0000000..da17c30 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/ok.json @@ -0,0 +1 @@ +{"v":1,"id":7,"final":true,"msg":{"kind":"ok","body":{}}} diff --git a/crates/proto/tests/fixtures/wire/refuse.json b/crates/proto/tests/fixtures/wire/refuse.json new file mode 100644 index 0000000..99cedfd --- /dev/null +++ b/crates/proto/tests/fixtures/wire/refuse.json @@ -0,0 +1 @@ +{"v":1,"id":7,"final":true,"msg":{"kind":"refuse","body":{"approval":41,"reason":"not while I am away"}}} diff --git a/crates/proto/tests/fixtures/wire/refuse_no_reason.json b/crates/proto/tests/fixtures/wire/refuse_no_reason.json new file mode 100644 index 0000000..4c63789 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/refuse_no_reason.json @@ -0,0 +1 @@ +{"v":1,"id":7,"final":true,"msg":{"kind":"refuse","body":{"approval":41,"reason":null}}} diff --git a/crates/proto/tests/fixtures/wire/tool_response_pending.json b/crates/proto/tests/fixtures/wire/tool_response_pending.json index a1148c9..67cb898 100644 --- a/crates/proto/tests/fixtures/wire/tool_response_pending.json +++ b/crates/proto/tests/fixtures/wire/tool_response_pending.json @@ -1 +1 @@ -{"v":1,"id":7,"final":false,"msg":{"kind":"tool_response","body":{"status":"pending_approval","approval":"ap-0001","expires":"2026-09-17T08:35:00.000Z"}}} +{"v":1,"id":7,"final":false,"msg":{"kind":"tool_response","body":{"status":"pending_approval","approval":41,"expires":"2026-09-17T08:35:00.000Z"}}} diff --git a/crates/proto/tests/fixtures/wire/turn_event_approval_pending.json b/crates/proto/tests/fixtures/wire/turn_event_approval_pending.json new file mode 100644 index 0000000..3fe22e6 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/turn_event_approval_pending.json @@ -0,0 +1 @@ +{"v":1,"id":3,"final":false,"msg":{"kind":"turn_event","body":{"event":"approval_pending","approval":41,"tool":"shell","expires":"2026-09-18T08:20:00.000Z"}}} diff --git a/crates/proto/tests/fixtures/wire/turn_event_tool_denied.json b/crates/proto/tests/fixtures/wire/turn_event_tool_denied.json new file mode 100644 index 0000000..1622ad6 --- /dev/null +++ b/crates/proto/tests/fixtures/wire/turn_event_tool_denied.json @@ -0,0 +1 @@ +{"v":1,"id":3,"final":false,"msg":{"kind":"turn_event","body":{"event":"tool_denied","name":"read_file","reason":"no_grant"}}} diff --git a/crates/proto/tests/strict.rs b/crates/proto/tests/strict.rs index 6509ea2..09178b9 100644 --- a/crates/proto/tests/strict.rs +++ b/crates/proto/tests/strict.rs @@ -75,9 +75,31 @@ fn envelopes_reject_unknown_keys_at_every_depth() { "turn_event_retrying.json", "turn_done.json", "error_session_full.json", + "approvals.json", + "approval_list.json", + "approval_list_empty.json", + "approve.json", + "approve_result_allowed.json", + "approve_result_denied.json", + "refuse.json", + "refuse_no_reason.json", + "ok.json", + "check_grants.json", + "grants_report.json", + "grants_report_ok.json", + "error_forbidden.json", + "error_no_such_approval.json", + "turn_event_approval_pending.json", + "turn_event_tool_denied.json", ] { - // Envelope, msg and body: three objects; turn_done also has a usage object. - let want = if name == "turn_done.json" { 4 } else { 3 }; + // Envelope, msg and body: three objects. Some bodies hold more: turn_done a usage + // object, approval_list one item, approve_result an outcome, grants_report two problems. + let want = match name { + "turn_done.json" | "approval_list.json" => 4, + "approve_result_allowed.json" | "approve_result_denied.json" => 4, + "grants_report.json" => 5, + _ => 3, + }; assert_eq!( check::(name, &fixture(&format!("wire/{name}"))), want, diff --git a/crates/proto/tests/turn_wire.rs b/crates/proto/tests/turn_wire.rs index abe35f2..835fbf9 100644 --- a/crates/proto/tests/turn_wire.rs +++ b/crates/proto/tests/turn_wire.rs @@ -1,8 +1,8 @@ //! Tests for the channel messages and the usage record, against byte-exact fixtures. Do not edit. use proto::{ - CallId, DataClass, Envelope, Epoch, ErrorCode, Hash32, LogRecord, Message, SessionId, - Timestamp, ToolCall, Turn, TurnDone, TurnEvent, Usage, WireError, + CallId, DataClass, DenyReason, Envelope, Epoch, ErrorCode, Hash32, LogRecord, Message, + SessionId, Timestamp, ToolCall, Turn, TurnDone, TurnEvent, Usage, WireError, }; fn fixture(kind: &str, name: &str) -> String { @@ -92,6 +92,29 @@ fn turn_events() { }), ), ); + check( + "turn_event_approval_pending.json", + env( + 3, + false, + Message::TurnEvent(TurnEvent::ApprovalPending { + approval: 41, + tool: "shell".to_string(), + expires: Timestamp::parse("2026-09-18T08:20:00.000Z").unwrap(), + }), + ), + ); + check( + "turn_event_tool_denied.json", + env( + 3, + false, + Message::TurnEvent(TurnEvent::ToolDenied { + name: "read_file".to_string(), + reason: DenyReason::NoGrant, + }), + ), + ); } #[test] @@ -165,6 +188,15 @@ fn every_turn_event_kind_round_trips() { expected: 500, got: 20, }, + TurnEvent::ApprovalPending { + approval: u64::MAX, + tool: "http_fetch".to_string(), + expires: Timestamp::parse("2026-09-18T08:20:00.000Z").unwrap(), + }, + TurnEvent::ToolDenied { + name: "shell".to_string(), + reason: DenyReason::StateUnreadable, + }, ]; for event in all { let text = serde_json::to_string(&event).unwrap(); diff --git a/crates/proto/tests/wire.rs b/crates/proto/tests/wire.rs index c8df146..b254fb9 100644 --- a/crates/proto/tests/wire.rs +++ b/crates/proto/tests/wire.rs @@ -54,12 +54,23 @@ fn tool_request() { #[test] fn tool_response_pending() { let body = ToolResponse::PendingApproval { - approval: "ap-0001".to_string(), + approval: 41, expires: Timestamp::parse("2026-09-17T08:35:00.000Z").unwrap(), }; check("tool_response_pending.json", response(7, false, body)); } +#[test] +fn an_approval_id_is_a_number_not_a_string() { + let good = fixture("tool_response_pending.json"); + let bad = good.replacen("\"approval\":41", "\"approval\":\"41\"", 1); + assert_ne!(good, bad); + assert!(serde_json::from_str::(&good).is_ok()); + assert!(serde_json::from_str::(&bad).is_err()); + let negative = good.replacen("\"approval\":41", "\"approval\":-1", 1); + assert!(serde_json::from_str::(&negative).is_err()); +} + #[test] fn tool_response_result() { let body = ToolResponse::Result { @@ -113,6 +124,10 @@ fn deny_reasons_and_error_codes_are_snake_case() { (DenyReason::DeniedByGrant, "denied_by_grant"), (DenyReason::ApprovalRefused, "approval_refused"), (DenyReason::ApprovalExpired, "approval_expired"), + (DenyReason::GrantsInvalid, "grants_invalid"), + (DenyReason::AuditUnavailable, "audit_unavailable"), + (DenyReason::InvalidArguments, "invalid_arguments"), + (DenyReason::StateUnreadable, "state_unreadable"), ]; for (value, text) in reasons { assert_eq!( @@ -125,6 +140,8 @@ fn deny_reasons_and_error_codes_are_snake_case() { (ErrorCode::BadVersion, "bad_version"), (ErrorCode::BadMessage, "bad_message"), (ErrorCode::Internal, "internal"), + (ErrorCode::Forbidden, "forbidden"), + (ErrorCode::NoSuchApproval, "no_such_approval"), ]; for (value, text) in codes { assert_eq!( diff --git a/docs/implementer-log.md b/docs/implementer-log.md index 1207ead..21af6a3 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -42,6 +42,7 @@ reviewer adds findings under "Reviews" once per milestone. | 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 | | M3a/01-proto-audit-types | 2026-09-19 | stopped | 1 | fail | none | The audit types were implemented exactly as the task specifies in audit.rs and lib.rs and the two tests copied; `records` passes (3 passed) and the audit portion of `strict` passes. `make gate` cannot pass: the task's `strict.rs` walks 28 wire fixtures but 16 (approvals/approval_list/approve/refuse/ok/grants_report/turn_event_* and friends) do not exist on the m3a branch and are created by task 02 ("leave wire.rs alone: task 02 changes it"). The `envelopes_reject_unknown_keys_at_every_depth` test fails on the missing `approvals.json`, so the gate fails. The branch was healthy at start (master's `strict` = 5 passed); the block is the task's new `strict.rs` requiring later fixtures. Reverted audit.rs/lib.rs/tests for a clean tree and committed only this row. A later session that has the wire fixtures (or a `strict.rs` scoped to task 01) can finish it. 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. | Ornith | +| M3a/02-proto-admin-wire | 2026-09-22 | done | 1 | pass | none | Added four DenyReason (GrantsInvalid, AuditUnavailable, InvalidArguments, StateUnreadable), two ErrorCode (Forbidden, NoSuchApproval), approval ids as u64 in ToolResponse::PendingApproval and TurnEvent::ApprovalPending, TurnEvent::ApprovalPending and ToolDenied, and the eight admin types (Empty {}, PendingApproval, ApprovalList, Approve, ApproveResult, Refuse, GrantProblem, GrantsReport) with deny_unknown_fields; re-exported from lib.rs; added the two required match arms in bxctl chat.rs. Copied four test files and 17 wire fixtures byte-identical. wire 10, turn_wire 5, admin_wire 10, strict 5 passed; `make gate` prints `gate: ok`. | OpenCode | | M3a/01-proto-audit-types | 2026-09-22 | done | 1 | pass | none | Finished the blocked task. `audit.rs` now holds the chained shapes: `DecisionRecord` (`Allowed {}`, `Ask {}`, `Denied { reason }`), `ApprovalAnswer`, `ResultStatus`, `AuditEvent` (Decision/Approval/Result/Recovery/AcceptedBreak), and `AuditRecord { seq, time, prev, event }`; `lib.rs` re-exports the five names. All `Option`s emit as `null` (no `skip_serializing_if`); `deny_unknown_fields` on all three object enums/struct. Tests copied from `docs/plans/M3a/files/`: `records` 3 passed, `strict` 5 passed. Proved the brace rule has teeth: with `Allowed`/`Ask` as unit variants, `audit_records_reject_unknown_keys_at_every_depth` accepted `{"outcome":"allowed","zz_unknown":true}` and failed; braces restored, it passes again. NOTE: `docs/plans/M3a/files/crates/proto/tests/strict.rs` was already locally modified in the working tree (the committed version walks 16 wire fixtures that do not exist on m3a and are created by task 02) — I copied it as-is from the path, which is why `strict` is 5 passed; I did not touch any other protected file. `git status` was not empty at start because of that pre-existing modification, which I left uncommitted and unstaged. | OpenCode |