# M5b task 01: an invalid call is denied with its reason **Branch:** `m5b` (run `git switch m5b`; `git status --short` must be empty, otherwise stop) **Commit subject:** `bxcore, brokerd, loopd: an invalid-arguments denial says why` ## Goal `brokerd` refuses a tool call whose arguments do not parse (`invalid_arguments`). Its parser explains why (`ArgsError`), but `policy::decide` throws the explanation away, so the model reads only a fixed sentence and has to guess. On the device (2026-09-25) it spent two calls finding that `note` takes no `file`. After this task the explanation reaches the model (M5b spec, section 4): | The model's call to `memory` | What it reads back | |---|---| | `{"action":"note","file":"notes.md","text":"x"}`, before | `Denied: the arguments are not valid for this tool.` | | the same, after | ``Denied: the arguments are not valid for this tool: invalid argument shape: unknown field `file`, expected `text`.`` | Four parts: 1. **`bxcore`**: `ToolResponse::Denied` gains a field `detail: Option`, written only when it is `Some`. A denial without a detail has exactly the bytes it had before, so every existing fixture stays valid. One new fixture has a detail. 2. **`brokerd`**: `ArgsError` gains `detail()`: its `Display` text on one line (each control character, and U+2028 and U+2029, becomes a space), cut to 300 characters. `policy::Denial` gains `detail`, set by `decide` for `InvalidArguments` only. The ledger passes the whole `Denial` on (`Decided::Denied(Denial)`), and `broker.sock` sends its detail. 3. **`loopd`**: a new `denial_message(reason, detail)` puts the detail after the sentence, before its full stop. Without a detail it is exactly today's sentence. 4. **Unchanged**: the audit record (`DecisionRecord::Denied { reason }` has no detail: the record holds the arguments already), `TurnEvent::ToolDenied`, and every text `gatewayd` and `bxctl` show the owner. Do not touch `crates/gatewayd`, `crates/bxctl` or `crates/bxcore/src/audit.rs`. ## Every place `ToolResponse::Denied` appears This is the complete list. Each one is covered by an edit below or by a given file. | Place | What happens there | |---|---| | `crates/bxcore/src/wire.rs`, the enum | gains `detail` (edit 1) | | `crates/brokerd/src/broker.rs`, `handle`, step 3a | sends the `Denial`'s detail (edit B1) | | `crates/brokerd/src/broker.rs`, `pending`, `Verdict::Denied` and `Err(reason)` | `detail: None`: an approval's denial has no detail (edits B2, B3) | | `crates/brokerd/src/ledger/answer.rs`, `finish` | a pattern with `..`; no change | | `crates/loopd/src/turn.rs`, `run_call` | reads `detail` and calls `denial_message` (edit T2) | | `crates/loopd/src/tools.rs`, `FakeTools::call` | `detail: None` (edit L2) | | `crates/loopd/src/broker_port.rs` | a pattern with `..`; no change | | tests in `bxcore`, `brokerd`, `loopd` | new versions are given; `brokerd/tests/broker_sequence.rs` uses `..` and is not changed | ## Files - Copy, from `docs/plans/M5b/files/` (the same path under it): - new: `crates/bxcore/tests/fixtures/wire/tool_response_denied_detail.json`, `crates/brokerd/tests/invalid_detail.rs` - **new versions** of: `crates/bxcore/tests/wire.rs`, `crates/bxcore/tests/strict.rs`; `crates/brokerd/tests/admin.rs`, `broker.rs`, `broker_pending.rs`, `courier.rs`, `ledger.rs`, `serve.rs`; `crates/loopd/tests/broker_port.rs`, `tools.rs`, `turn_broker.rs`. Your `broker.rs` is the given `broker.rs-task01`: the given `broker.rs` is task 09's. Your `serve.rs` is the given `serve.rs-task01`: the given `serve.rs` is task 13's. - Modify: `crates/bxcore/src/wire.rs`, `crates/brokerd/src/args.rs`, `crates/brokerd/src/policy/mod.rs`, `crates/brokerd/src/ledger/mod.rs`, `crates/brokerd/src/broker.rs`, `crates/loopd/src/tools.rs`, `crates/loopd/src/turn.rs`, `docs/implementer-log.md` ## Facts you need - `serde`: `#[serde(default, skip_serializing_if = "Option::is_none")]` on a field of a variant of this enum (internally tagged, `deny_unknown_fields`) does what it says: a missing `detail` reads as `None`, `None` is not written, and an unknown key is still refused. Checked against every fixture; `strict.rs` walks the new one too. - `char::is_control(self) -> bool` is true for Unicode's control characters only: U+0000 to U+001F and U+007F to U+009F (tab, line feed, carriage return, escape and NEL among them). It is false for U+2028 (line separator) and U+2029 (paragraph separator), so `detail` names those two. - The 300 is characters (`chars()`), not bytes, so a cut never splits a character. - `str::strip_suffix(&self, '.') -> Option<&str>`; `Option::as_deref(&self) -> Option<&str>` for an `Option`. - `Denial { detail: Some(..), ..Denial::new(reason) }` is Rust's struct update syntax: the other fields come from `Denial::new`. - `crates/brokerd/src/ledger/mod.rs` already imports `Denial` and `DenyReason`; keep both. ## The edits, exactly Nothing else in these files changes. Each quoted "before" block occurs exactly once in its file. ### `crates/bxcore/src/wire.rs` **1.** In `enum ToolResponse`, the lines ```rust Denied { reason: DenyReason, }, } ``` become ```rust Denied { reason: DenyReason, /// Why the arguments were not valid, for `invalid_arguments` only. Written only when /// present, so a denial without one has the same bytes as before M5b. #[serde(default, skip_serializing_if = "Option::is_none")] detail: Option, }, } ``` ### `crates/brokerd/src/args.rs` **A1.** The lines ```rust /// Maximum length, in bytes, of a URL. pub const MAX_URL: usize = 2048; ``` become ```rust /// Maximum length, in bytes, of a URL. pub const MAX_URL: usize = 2048; /// Maximum length, in characters, of the detail an invalid call is denied with. pub const MAX_DETAIL: usize = 300; ``` **A2.** The line ```rust impl std::error::Error for ArgsError { ``` becomes ```rust impl ArgsError { /// The error as the model is shown it: the `Display` text on one line, each control character /// and each Unicode line or paragraph separator replaced by a space, cut to `MAX_DETAIL` /// characters. The text holds the model's own arguments, which may be long or hold newlines. pub fn detail(&self) -> String { self.to_string() .chars() .map(|c| { if c.is_control() || c == '\u{2028}' || c == '\u{2029}' { ' ' } else { c } }) .take(MAX_DETAIL) .collect() } } impl std::error::Error for ArgsError { ``` ### `crates/brokerd/src/policy/mod.rs` **P1.** In `struct Denial`, the lines ```rust /// `Some` only for `DeniedByGrant`: the deny grant's file hash. pub grant_sha256: Option, } ``` become ```rust /// `Some` only for `DeniedByGrant`: the deny grant's file hash. pub grant_sha256: Option, /// `Some` only for `InvalidArguments`: why, from `ArgsError::detail`. It goes to the model in /// the `ToolResponse`, never into the audit record, which holds the arguments already. pub detail: Option, } ``` **P2.** In `Denial::new`, the lines ```rust grant: None, grant_sha256: None, } ``` become ```rust grant: None, grant_sha256: None, detail: None, } ``` **P3.** In `decide`, the lines ```rust // 2. Malformed arguments are refused before matching, whatever the grants say. let Ok(args) = args::parse(tool, &request.arguments) else { return Outcome::Denied(Denial::new(DenyReason::InvalidArguments)); }; ``` become ```rust // 2. Malformed arguments are refused before matching, whatever the grants say, and the // model is told why, so that it can correct the call. let args = match args::parse(tool, &request.arguments) { Ok(args) => args, Err(error) => { return Outcome::Denied(Denial { detail: Some(error.detail()), ..Denial::new(DenyReason::InvalidArguments) }); } }; ``` **P4.** In `decide`, the lines ```rust Mode::Deny => Outcome::Denied(Denial { reason: DenyReason::DeniedByGrant, grant: Some(winner.id.clone()), grant_sha256: Some(winner.grant_sha256), }), ``` become ```rust Mode::Deny => Outcome::Denied(Denial { reason: DenyReason::DeniedByGrant, grant: Some(winner.id.clone()), grant_sha256: Some(winner.grant_sha256), detail: None, }), ``` **P5.** In `redecide`, the lines ```rust Mode::Deny => Err(Denial { reason: DenyReason::DeniedByGrant, grant: Some(winner.id.clone()), grant_sha256: Some(winner.grant_sha256), }), ``` become ```rust Mode::Deny => Err(Denial { reason: DenyReason::DeniedByGrant, grant: Some(winner.id.clone()), grant_sha256: Some(winner.grant_sha256), detail: None, }), ``` ### `crates/brokerd/src/ledger/mod.rs` **D1.** The lines ```rust /// How a call was decided. #[derive(Debug)] pub enum Decided { ``` become ```rust /// How a call was decided. A denial carries the policy's `Denial`, so an invalid call keeps its /// detail on the way to the answer. #[derive(Debug)] pub enum Decided { ``` **D2.** At the end of `enum Decided`, the lines ```rust Denied(DenyReason), } ``` become ```rust Denied(Denial), } ``` **D3.** In `Ledger::decide`, the line ```rust None => return Decided::Denied(DenyReason::AuditUnavailable), ``` becomes ```rust None => return Decided::Denied(Denial::new(DenyReason::AuditUnavailable)), ``` **D4.** The line ```rust RecordPayload::Simple(reason) => Decided::Denied(reason), ``` becomes ```rust RecordPayload::Simple(reason) => Decided::Denied(Denial::new(reason)), ``` **D5.** The line ```rust RecordPayload::Denial(denial) => Decided::Denied(denial.reason), ``` becomes ```rust RecordPayload::Denial(denial) => Decided::Denied(denial), ``` **D6.** The line ```rust Err(()) => Decided::Denied(DenyReason::AuditUnavailable), ``` becomes ```rust Err(()) => Decided::Denied(Denial::new(DenyReason::AuditUnavailable)), ``` ### `crates/brokerd/src/broker.rs` **B1.** In `handle`, the line ```rust Decided::Denied(reason) => Some(ToolResponse::Denied { reason }), // 3a. denied ``` becomes ```rust Decided::Denied(denial) => Some(ToolResponse::Denied { reason: denial.reason, detail: denial.detail, }), // 3a. denied ``` **B2.** In `pending`, the line ```rust Verdict::Denied(reason) => Some(ToolResponse::Denied { reason }), ``` becomes ```rust Verdict::Denied(reason) => Some(ToolResponse::Denied { reason, detail: None, }), ``` **B3.** At the end of `pending`, the line ```rust Err(reason) => Some(ToolResponse::Denied { reason }), ``` becomes ```rust Err(reason) => Some(ToolResponse::Denied { reason, detail: None, }), ``` ### `crates/loopd/src/tools.rs` **L1.** The line ```rust pub struct FakeTools { ``` becomes ```rust /// What a denial reaches the model as: the reason's sentence, and the broker's detail, if it sent /// one, after a colon (`Denied: the arguments are not valid for this tool: .`). pub fn denial_message(reason: bxcore::DenyReason, detail: Option<&str>) -> String { let sentence = denial_text(reason); match detail { None => sentence.to_string(), Some(detail) => { let stem = sentence.strip_suffix('.').unwrap_or(sentence); format!("{stem}: {detail}.") } } } pub struct FakeTools { ``` `denial_text` itself does not change. **L2.** In `FakeTools::call`, the lines ```rust _ => bxcore::ToolResponse::Denied { reason: bxcore::DenyReason::NoGrant, }, ``` become ```rust _ => bxcore::ToolResponse::Denied { reason: bxcore::DenyReason::NoGrant, detail: None, }, ``` ### `crates/loopd/src/turn.rs` **T1.** The line ```rust use crate::tools::{Registry, ToolPort, cap_result, denial_text, dispatch}; ``` becomes ```rust use crate::tools::{Registry, ToolPort, cap_result, denial_message, dispatch}; ``` **T2.** In `run_call`, the lines ```rust ToolResponse::Denied { reason } => { on_event(&TurnEvent::ToolDenied { name: request.tool.clone(), reason, }); (denial_text(reason).to_string(), DataClass::Public, false) } ``` become ```rust ToolResponse::Denied { reason, detail } => { on_event(&TurnEvent::ToolDenied { name: request.tool.clone(), reason, }); ( denial_message(reason, detail.as_deref()), DataClass::Public, false, ) } ``` The event keeps only the reason: the owner's texts do not change. ## Steps - [ ] **1. Copy the given files.** From the repository root, thirteen commands: ```sh cp docs/plans/M5b/files/crates/bxcore/tests/fixtures/wire/tool_response_denied_detail.json \ crates/bxcore/tests/fixtures/wire/ cp docs/plans/M5b/files/crates/bxcore/tests/wire.rs \ crates/bxcore/tests/ cp docs/plans/M5b/files/crates/bxcore/tests/strict.rs \ crates/bxcore/tests/ cp docs/plans/M5b/files/crates/brokerd/tests/invalid_detail.rs \ crates/brokerd/tests/ cp docs/plans/M5b/files/crates/brokerd/tests/admin.rs \ crates/brokerd/tests/ cp docs/plans/M5b/files/crates/brokerd/tests/broker.rs-task01 \ crates/brokerd/tests/broker.rs cp docs/plans/M5b/files/crates/brokerd/tests/broker_pending.rs \ crates/brokerd/tests/ cp docs/plans/M5b/files/crates/brokerd/tests/courier.rs \ crates/brokerd/tests/ cp docs/plans/M5b/files/crates/brokerd/tests/ledger.rs \ crates/brokerd/tests/ cp docs/plans/M5b/files/crates/brokerd/tests/serve.rs-task01 \ crates/brokerd/tests/serve.rs cp docs/plans/M5b/files/crates/loopd/tests/broker_port.rs \ crates/loopd/tests/ cp docs/plans/M5b/files/crates/loopd/tests/tools.rs \ crates/loopd/tests/ cp docs/plans/M5b/files/crates/loopd/tests/turn_broker.rs \ crates/loopd/tests/ ``` Then `git status --short` lists 11 modified files and 2 new ones, all under `crates/*/tests/`. - [ ] **2. See them fail.** - `cargo test -p bxcore --test strict`: one test fails, `envelopes_reject_unknown_keys_at_every_depth`, with `tool_response_denied_detail.json: fixture must decode`. - `cargo test -p bxcore --test wire`: does not compile, with two errors `E0559` (``variant `bxcore::ToolResponse::Denied` has no field named `detail` ``). - `cargo build --tests --keep-going -p brokerd -p loopd`: ten test targets do not compile: `admin`, `broker`, `broker_pending`, `courier`, `invalid_detail`, `ledger` and `serve` in `brokerd`; `broker_port`, `tools` and `turn_broker` in `loopd`. Every error is about a name this task adds: `E0559` (no field named `detail` on `ToolResponse::Denied`), `E0609` (no field `detail` on `Denial`), `E0599` (no method named `detail` for `ArgsError`), `E0432` (unresolved import `MAX_DETAIL` or `denial_message`), or `E0308` (mismatched types, in `ledger.rs`, where `Decided::Denied` will hold a `Denial`). - [ ] **3. `bxcore`.** Make edit 1. `cargo test -p bxcore`: every suite passes. - [ ] **4. `brokerd`.** Make edits A1, A2, P1 to P5, D1 to D6, and B1 to B3. `cargo test -p brokerd`: every suite passes, among them the eight tests of `invalid_detail`. - [ ] **5. `loopd`.** Make edits L1, L2, T1 and T2. `cargo test -p loopd`: every suite passes. - [ ] **6. Check.** `grep -rn "Decided::Denied(DenyReason" crates/` prints nothing, and `git diff --stat -- crates/gatewayd crates/bxctl crates/bxcore/src/audit.rs` prints nothing. - [ ] **7. Run the gate.** `cargo fmt --all`, then `make gate`, with a shell timeout of 600000 ms. Expected last line: `gate: ok`, with 956 tests: 944 before this task, plus 12 new ones (two in `bxcore/tests/wire.rs`, eight in `brokerd/tests/invalid_detail.rs`, one in `loopd/tests/tools.rs`, one in `loopd/tests/turn_broker.rs`). - [ ] **8. Log and commit.** `git add crates/bxcore crates/brokerd crates/loopd docs/implementer-log.md Cargo.lock`, then commit as `AGENTS.md` says. `Cargo.lock` does not change in this task; the `git add` is harmless. ## Done when - `make gate` prints `gate: ok` with 956 tests. - `grep -rn "Decided::Denied(DenyReason" crates/` prints nothing. ## Stop and report if - Any quoted "before" block is not in its file exactly as shown, or is there more than once. - Step 2 shows a different result: another test failing in `strict`, or an error of another kind. - After step 3, 4 or 5, a test fails. Do not change a test or a fixture. - A test in `gatewayd`, `bxctl`, `memoryd` or `toolkit` fails in the gate: check once whether it fails without your change, then stop and report both results. - The gate passes with a count other than 956. - A comment in an edit does not fit the code under it. Quote it and stop.