Add M1 plan, given tests and fixtures, AGENTS.md and implementer log

Seven task files for the implementing model under docs/plans/M1/, with the test files, byte-exact fixtures, Makefile, deny.toml and gate-script self-test they copy into place. All of it was verified against a private reference implementation: the gate passes after every task in order. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-17 01:22:43 -07:00
parent 19104a9629
commit 3e26c2e3c0
36 changed files with 2129 additions and 2 deletions
@@ -0,0 +1,178 @@
+# M1 task 07: `Decision` in `brokerd`
+
+**Branch:** `m1` (run `git switch m1`; `git status --short` must be empty, otherwise stop)
+**Commit subject:** `Add Decision, decide and the runner stub to brokerd`
+
+## Goal
+
+Put the project's central safety rule into types: a tool can only run if policy produced a
+`Decision`, and nothing outside `brokerd`'s policy module can make one. This task adds the type,
+a policy function that denies everything, and a runner stub. Grants are matched in M3, not here.
+
+## Context
+
+From the design brief: "Authority is encoded in types: a tool cannot execute without a `Decision`
+value that only `brokerd`'s policy module can construct. `Decision` is defined in `brokerd`, has a
+private field and does not implement `Deserialize`, so no other crate or wire message can produce
+one." Also: "No matching grant means deny."
+
+The proof is two `compile_fail` doctests: examples that must **fail** to compile. A third doctest
+uses the same setup through the allowed path and must compile and pass. That third one shows the
+first two fail because of `Decision`'s privacy and not because of a mistake in the example.
+
+## Files
+
+- Create: `crates/brokerd/src/policy.rs`, `crates/brokerd/src/runner.rs`
+- Modify: `crates/brokerd/src/lib.rs`, `docs/implementer-log.md`
+
+## Interfaces
+
+Consumes from `proto`: `ToolRequest`, `ToolResponse`, `DenyReason`, `SessionId`, `CallId`.
+
+Produces:
+
+```rust
+// crates/brokerd/src/policy.rs
+#[derive(Debug)]                        // and nothing else: not Clone, not Serialize, not Deserialize
+pub struct Decision { request: ToolRequest, grant: String }     // both fields private
+
+impl Decision {
+    fn new(request: ToolRequest, grant: String) -> Self;         // private: no `pub`
+    pub fn request(&self) -> &ToolRequest;
+    pub fn grant(&self) -> &str;
+}
+
+/// Until M3 there are no grants, so every request is denied with DenyReason::NoGrant.
+pub fn decide(request: ToolRequest) -> Result<Decision, DenyReason>;
+
+// crates/brokerd/src/runner.rs
+/// Takes the Decision by value, so one decision cannot run a tool twice.
+pub fn run(decision: Decision) -> ToolResponse;   // returns Failed { message: "no tool runner until M3" }
+```
+
+`Decision::new` is not called by non-test code until M3, so the compiler reports it as dead code.
+This is the one place in M1 where a lint attribute is allowed. Put exactly this on `new`:
+
+```rust
+#[cfg_attr(not(test), expect(dead_code, reason = "grant matching arrives in M3"))]
+```
+
+## Steps
+
+- [ ] **1. Write the module documentation first.** `crates/brokerd/src/policy.rs` starts with
+  exactly this doc comment. It contains the three doctests.
+
+````rust
+//! Policy decisions. `Decision` can only be constructed in this module.
+//!
+//! Code outside this module cannot build a `Decision` with a struct literal, because its fields
+//! are private:
+//!
+//! ```compile_fail
+//! let request = proto::ToolRequest {
+//!     session: proto::SessionId::new("s1").unwrap(),
+//!     call: proto::CallId(1),
+//!     tool: "read_file".to_string(),
+//!     arguments: "{}".to_string(),
+//! };
+//! let _ = brokerd::policy::Decision { request, grant: "g".to_string() };
+//! ```
+//!
+//! Nor with the constructor, because it is private to this module:
+//!
+//! ```compile_fail
+//! let request = proto::ToolRequest {
+//!     session: proto::SessionId::new("s1").unwrap(),
+//!     call: proto::CallId(1),
+//!     tool: "read_file".to_string(),
+//!     arguments: "{}".to_string(),
+//! };
+//! let _ = brokerd::policy::Decision::new(request, "g".to_string());
+//! ```
+//!
+//! The same setup compiles when it goes through `decide`, which proves the two examples above
+//! fail because of `Decision` and not because of a mistake in the setup:
+//!
+//! ```
+//! let request = proto::ToolRequest {
+//!     session: proto::SessionId::new("s1").unwrap(),
+//!     call: proto::CallId(1),
+//!     tool: "read_file".to_string(),
+//!     arguments: "{}".to_string(),
+//! };
+//! assert_eq!(brokerd::policy::decide(request).unwrap_err(), proto::DenyReason::NoGrant);
+//! ```
+````
+
+- [ ] **2. Write the unit tests** at the end of `policy.rs`, exactly:
+
+```rust
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use proto::{CallId, SessionId};
+
+    fn request() -> ToolRequest {
+        ToolRequest {
+            session: SessionId::new("s1").unwrap(),
+            call: CallId(1),
+            tool: "read_file".to_string(),
+            arguments: "{}".to_string(),
+        }
+    }
+
+    #[test]
+    fn no_grants_means_deny() {
+        assert_eq!(decide(request()).unwrap_err(), DenyReason::NoGrant);
+    }
+
+    #[test]
+    fn decision_exposes_request_and_grant() {
+        let d = Decision::new(request(), "g1".to_string());
+        assert_eq!(d.request().tool, "read_file");
+        assert_eq!(d.grant(), "g1");
+    }
+}
+```
+
+- [ ] **3. Register the modules and see the tests fail.** Add to `crates/brokerd/src/lib.rs`, below
+  the doc comment:
+
+```rust
+pub mod policy;
+pub mod runner;
+```
+
+Create `runner.rs` as an empty file for now. Run `cargo test -p brokerd`. Expected: it does not
+compile, because `Decision`, `decide`, `ToolRequest` and `DenyReason` are not defined in `policy.rs`.
+
+- [ ] **4. Write `Decision`, `decide` and `run`** as described under Interfaces.
+
+- [ ] **5. See the tests pass.** `cargo test -p brokerd`. Expected: 2 unit tests pass, and under
+  `Doc-tests brokerd` 3 pass, two of them marked `compile fail`.
+
+- [ ] **6. Check that the doctests guard what they claim.** Temporarily change `fn new` to
+  `pub fn new` and run `cargo test -p brokerd --doc`. Expected: one failure saying
+  `Test compiled successfully, but it's marked compile_fail`. Change it back and run the command
+  again. Expected: 3 pass. Write in your log row that you did this check and what you saw.
+
+- [ ] **7. Run the gate.** `make gate`. Expected last line: `gate: ok`.
+
+- [ ] **8. Log and commit.**
+
+```sh
+git add crates/brokerd docs/implementer-log.md
+git commit
+```
+
+## Done when
+
+- `cargo test -p brokerd` passes 2 unit tests and 3 doctests.
+- `Decision` derives only `Debug`, and `new` has no `pub`.
+- `make gate` prints `gate: ok`.
+- `git log --oneline master..m1` shows one commit per task.
+
+## Stop and report if
+
+- The `expect(dead_code)` attribute itself causes a warning or an error.
+- A `compile_fail` doctest passes in step 6 even with `pub fn new`.