boxmaker/docs/plans/M5b/01-invalid-arguments-detail.md

# 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<String>`, 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<String>`.
- `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<String>,
    },
}
```

### `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<Hash32>,
}
```

become

```rust
    /// `Some` only for `DeniedByGrant`: the deny grant's file hash.
    pub grant_sha256: Option<Hash32>,
    /// `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<String>,
}
```

**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: <detail>.`).
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.