brokerd: answers carry the Mattermost post; Table::contains

Implemented-By: OpenCode session (model recorded in docs/implementer-log.md)
This commit is contained in:
2026-09-24 04:15:11 -07:00
parent a5c37d8756
commit a00711e379
8 changed files with 172 additions and 27 deletions
+2
View File
@@ -80,6 +80,7 @@ pub fn handle(mut stream: UnixStream, broker: &Broker) {
entry, entry,
Answer::Approved { Answer::Approved {
by: Some(BY.to_string()), by: Some(BY.to_string()),
post: None,
}, },
now, now,
); );
@@ -96,6 +97,7 @@ pub fn handle(mut stream: UnixStream, broker: &Broker) {
entry, entry,
Answer::Refused { Answer::Refused {
by: Some(BY.to_string()), by: Some(BY.to_string()),
post: None,
reason, reason,
}, },
now, now,
+29 -1
View File
@@ -2,7 +2,7 @@
//! gone. In memory only. //! gone. In memory only.
use crate::policy::{Ask, Decision}; use crate::policy::{Ask, Decision};
use proto::{DenyReason, PendingApproval, Timestamp}; use proto::{DecisionRecord, DenyReason, PendingApproval, Timestamp};
use std::collections::BTreeMap; use std::collections::BTreeMap;
use std::sync::mpsc::{Receiver, Sender, channel}; use std::sync::mpsc::{Receiver, Sender, channel};
use std::sync::{Mutex, MutexGuard}; use std::sync::{Mutex, MutexGuard};
@@ -14,6 +14,29 @@ pub enum Verdict {
Denied(DenyReason), Denied(DenyReason),
} }
/// An owner's answer to a pending call.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Answer {
/// `by` is "bxctl" or a Mattermost user id; `post` the Mattermost post that answered.
Approved {
by: Option<String>,
post: Option<String>,
},
Refused {
by: Option<String>,
post: Option<String>,
reason: Option<String>,
},
Expired,
}
/// What an approval leaves: the verdict (run or deny) and the record written.
#[derive(Debug)]
pub struct Answered {
pub verdict: Verdict,
pub outcome: DecisionRecord,
}
#[derive(Debug)] #[derive(Debug)]
pub struct Entry { pub struct Entry {
pub info: PendingApproval, pub info: PendingApproval,
@@ -52,6 +75,11 @@ impl Table {
receiver receiver
} }
/// Is approval `id` still pending? Only a hint: it may be taken the moment after.
pub fn contains(&self, id: u64) -> bool {
self.lock().contains_key(&id)
}
pub fn take(&self, id: u64) -> Option<Entry> { pub fn take(&self, id: u64) -> Option<Entry> {
self.lock().remove(&id) self.lock().remove(&id)
} }
+13 -25
View File
@@ -10,6 +10,7 @@ use proto::{
}; };
use crate::approvals::Verdict; use crate::approvals::Verdict;
pub use crate::approvals::{Answer, Answered};
use crate::audit::{AuditError, Writer}; use crate::audit::{AuditError, Writer};
use crate::grants::GrantSet; use crate::grants::GrantSet;
use crate::policy::{Ask, Denial, Label, SessionState}; use crate::policy::{Ask, Denial, Label, SessionState};
@@ -82,26 +83,6 @@ enum RecordPayload {
Simple(DenyReason), Simple(DenyReason),
} }
/// An owner's answer to a pending call.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Answer {
Approved {
by: Option<String>,
},
Refused {
by: Option<String>,
reason: Option<String>,
},
Expired,
}
/// What an approval leaves: the verdict (run or deny) and the record written.
#[derive(Debug)]
pub struct Answered {
pub verdict: Verdict,
pub outcome: DecisionRecord,
}
struct Inner { struct Inner {
audit: Box<dyn AuditSink>, audit: Box<dyn AuditSink>,
state: StateStore, state: StateStore,
@@ -339,10 +320,17 @@ impl Ledger {
}; };
// 3. The answer and its fields. // 3. The answer and its fields.
let (answer, by, reason) = match &answer { let (answer, by, post, reason) = match &answer {
Answer::Approved { by } => (ApprovalAnswer::Approved, by.clone(), None), Answer::Approved { by, post } => {
Answer::Refused { by, reason } => (ApprovalAnswer::Refused, by.clone(), reason.clone()), (ApprovalAnswer::Approved, by.clone(), post.clone(), None)
Answer::Expired => (ApprovalAnswer::Expired, None, None), }
Answer::Refused { by, post, reason } => (
ApprovalAnswer::Refused,
by.clone(),
post.clone(),
reason.clone(),
),
Answer::Expired => (ApprovalAnswer::Expired, None, None, None),
}; };
// 4. The re-decision (approved only); every other answer is a denial. // 4. The re-decision (approved only); every other answer is a denial.
@@ -382,7 +370,7 @@ impl Ledger {
decision, decision,
answer, answer,
by, by,
post: None, post,
reason, reason,
outcome: outcome.clone(), outcome: outcome.clone(),
grant, grant,
+2
View File
@@ -73,6 +73,7 @@ fn answer_entry(
fn approved() -> Answer { fn approved() -> Answer {
Answer::Approved { Answer::Approved {
by: Some("bxctl".to_string()), by: Some("bxctl".to_string()),
post: None,
} }
} }
@@ -166,6 +167,7 @@ fn a_denied_verdict_reaches_the_requester_and_nothing_runs() {
let (mut stream, approval, _) = start(&broker, r#"{"path":"/n/a"}"#); let (mut stream, approval, _) = start(&broker, r#"{"path":"/n/a"}"#);
let refused = Answer::Refused { let refused = Answer::Refused {
by: Some("bxctl".to_string()), by: Some("bxctl".to_string()),
post: None,
reason: None, reason: None,
}; };
answer_by_hand(&broker, approval, refused); answer_by_hand(&broker, approval, refused);
+5 -1
View File
@@ -177,10 +177,14 @@ fn approved_and_refused_calls_run_only_after_an_approval() {
continue; continue;
}; };
let answer = if item.approval % 2 == 0 { let answer = if item.approval % 2 == 0 {
Answer::Approved { by: None } Answer::Approved {
by: None,
post: None,
}
} else { } else {
Answer::Refused { Answer::Refused {
by: None, by: None,
post: None,
reason: None, reason: None,
} }
}; };
+2
View File
@@ -33,6 +33,7 @@ fn pending(ledger: &Ledger) -> Ask {
fn bxctl() -> Answer { fn bxctl() -> Answer {
Answer::Approved { Answer::Approved {
by: Some("bxctl".to_string()), by: Some("bxctl".to_string()),
post: None,
} }
} }
@@ -206,6 +207,7 @@ fn a_refusal_is_recorded_with_the_owners_reason() {
let ask = pending(&ledger); let ask = pending(&ledger);
let refused = Answer::Refused { let refused = Answer::Refused {
by: Some("bxctl".to_string()), by: Some("bxctl".to_string()),
post: None,
reason: Some("not now".to_string()), reason: Some("not now".to_string()),
}; };
let answered = ledger.answer(ask, 0, refused, &asking(), now()); let answered = ledger.answer(ask, 0, refused, &asking(), now());
+118
View File
@@ -0,0 +1,118 @@
//! An answer from Mattermost records who answered and in which post, approved or refused; the
//! table says whether an approval is still pending (M4b spec, section 5). Do not edit.
#[path = "support/build.rs"]
mod build;
#[path = "support/rig.rs"]
mod rig;
#[path = "support/sink.rs"]
mod sink;
#[path = "support/tmp.rs"]
mod tmp;
use brokerd::approvals::Table;
use brokerd::ledger::{Answer, Decided, Grants, Ledger};
use brokerd::policy::Ask;
use build::{grant, now, read, set};
use proto::{ApprovalAnswer, AuditEvent, CallId, DataClass, Mode, PendingApproval, SessionId};
use rig::Rig;
const KYLE: &str = "k0000000000000000000000000";
const POST: &str = "c0000000000000000000000001";
fn asking() -> Grants {
Ok(set(vec![grant("n", "read_file", Mode::Ask).paths(&["/n"])]))
}
fn pending(ledger: &Ledger) -> Ask {
match ledger.decide(read("/n/deep/a"), &asking(), now()) {
Decided::Ask { ask, seq: 0, .. } => ask,
other => panic!("{other:?}"),
}
}
/// The one `Approval` record's (answer, by, post, reason).
fn recorded(
rig: &Rig,
) -> (
ApprovalAnswer,
Option<String>,
Option<String>,
Option<String>,
) {
let events = rig.events();
match &events[1] {
AuditEvent::Approval {
answer,
by,
post,
reason,
..
} => (*answer, by.clone(), post.clone(), reason.clone()),
other => panic!("{other:?}"),
}
}
#[test]
fn an_approval_from_mattermost_names_the_user_and_the_post() {
let rig = Rig::new("post-approve");
let ledger = rig.ledger();
let answer = Answer::Approved {
by: Some(KYLE.to_string()),
post: Some(POST.to_string()),
};
ledger.answer(pending(&ledger), 0, answer, &asking(), now());
assert_eq!(
recorded(&rig),
(
ApprovalAnswer::Approved,
Some(KYLE.to_string()),
Some(POST.to_string()),
None
)
);
}
#[test]
fn a_refusal_from_mattermost_names_them_and_keeps_the_reason() {
let rig = Rig::new("post-refuse");
let ledger = rig.ledger();
let answer = Answer::Refused {
by: Some(KYLE.to_string()),
post: Some(POST.to_string()),
reason: Some("not on a Friday".to_string()),
};
ledger.answer(pending(&ledger), 0, answer, &asking(), now());
assert_eq!(
recorded(&rig),
(
ApprovalAnswer::Refused,
Some(KYLE.to_string()),
Some(POST.to_string()),
Some("not on a Friday".to_string())
)
);
}
#[test]
fn the_table_says_what_is_pending() {
let rig = Rig::new("post-table");
let ledger = rig.ledger();
let table = Table::new();
assert!(!table.contains(0));
let info = PendingApproval {
approval: 0,
session: SessionId::new("s1").unwrap(),
call: CallId(1),
tool: "read_file".to_string(),
arguments: "{}".to_string(),
grant: "n".to_string(),
taint: DataClass::Private,
created: now(),
expires: now(),
};
let _verdict = table.insert(info, pending(&ledger));
assert!(table.contains(0) && !table.contains(1));
assert!(table.take(0).is_some());
assert!(!table.contains(0));
}
+1
View File
@@ -6,6 +6,7 @@ reviewer adds findings under "Reviews" once per milestone.
| Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model | | Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model |
|---|---|---|---|---|---|---|---| |---|---|---|---|---|---|---|---|
| M4b/04-brokerd-answer-post | 2026-09-24 | done | 1 | pass | none | Moved `enum Answer` and `struct Answered` from `ledger.rs` into `approvals.rs` before `pub struct Entry` (with doc comments), added `pub use crate::approvals::{Answer, Answered};` next to the `Verdict` use and `DecisionRecord` to the `use proto` line in `approvals.rs`. Added the `post` field to `Answer::Approved`/`Answer::Refused`, threaded it through `ledger::answer` (step 3 binds `post`, step 6 writes it into `AuditEvent::Approval` instead of `None`), and gave the two `bxctl` answers in `admin.rs` `post: None`. Added `Table::contains` before `take`. Copied `ledger_post.rs`, `broker_pending.rs`, `broker_sequence.rs` and `ledger_answer.rs` into `crates/brokerd/tests/`; every suite passes, `ledger_post` 3 passed, `ledger.rs` is 487 lines, and `make gate` prints `gate: ok` on the first run. | ? |
| M4b/03-bxctl-deny | 2026-09-24 | done | 1 | pass | none | Copied `docs/plans/M4b/files/crates/bxctl/tests/cli_deny.rs` into `crates/bxctl/tests/`. In `parse`, `"refuse" => parse_refuse(rest, home)` became `"refuse" | "deny" => parse_refuse(rest, home)`, so `deny` produces the same `Command::Refuse` as `refuse` for every argument list including the error cases. Added a blank line and the two-line `bxctl deny <id> [--reason TEXT] [--admin-socket PATH]` / `Another name for refuse.` block to `USAGE` after the `refuse` block, keeping the two-space and six-space indents. The copied test's `deny_parses_as_refuse` and `usage_names_deny` both failed before the changes and pass after; `cargo test -p bxctl` is green and `make gate` prints `gate: ok` on the first run. | ? | | M4b/03-bxctl-deny | 2026-09-24 | done | 1 | pass | none | Copied `docs/plans/M4b/files/crates/bxctl/tests/cli_deny.rs` into `crates/bxctl/tests/`. In `parse`, `"refuse" => parse_refuse(rest, home)` became `"refuse" | "deny" => parse_refuse(rest, home)`, so `deny` produces the same `Command::Refuse` as `refuse` for every argument list including the error cases. Added a blank line and the two-line `bxctl deny <id> [--reason TEXT] [--admin-socket PATH]` / `Another name for refuse.` block to `USAGE` after the `refuse` block, keeping the two-space and six-space indents. The copied test's `deny_parses_as_refuse` and `usage_names_deny` both failed before the changes and pass after; `cargo test -p bxctl` is green and `make gate` prints `gate: ok` on the first run. | ? |
| M4b/02-proto-gateway-wire | 2026-09-24 | done | 2 | fail | none | Copied `gateway.rs`, `gateway_wire.rs` and the eight `gateway_*.json` fixtures into place; added `pub mod gateway;` after `pub mod frame;` and the re-export to `lib.rs`, the four `Message` variants plus the `use` to `wire.rs`, and the four `kind` arms (changing "fourteen" to "eighteen") in `broker.rs`. Filled `is_mattermost_id` (`id.len() == 26 && every byte a-z0-9`) and `ApprovalReply::check` in the given order: `by`, `post`, then a reason over `MAX_REASON` chars by `chars().count()`. All 3 `gateway_wire` tests pass; the first `make gate` failed on clippy `collapsible_if`, fixed by collapsing the nested `if let` into a let-chain guard, and the second prints `gate: ok`. | ? | | M4b/02-proto-gateway-wire | 2026-09-24 | done | 2 | fail | none | Copied `gateway.rs`, `gateway_wire.rs` and the eight `gateway_*.json` fixtures into place; added `pub mod gateway;` after `pub mod frame;` and the re-export to `lib.rs`, the four `Message` variants plus the `use` to `wire.rs`, and the four `kind` arms (changing "fourteen" to "eighteen") in `broker.rs`. Filled `is_mattermost_id` (`id.len() == 26 && every byte a-z0-9`) and `ApprovalReply::check` in the given order: `by`, `post`, then a reason over `MAX_REASON` chars by `chars().count()`. All 3 `gateway_wire` tests pass; the first `make gate` failed on clippy `collapsible_if`, fixed by collapsing the nested `if let` into a let-chain guard, and the second prints `gate: ok`. | ? |
| M4b/01-proto-escape | 2026-09-24 | done | 1 | pass | none | Copied `docs/plans/M4b/files/crates/proto/src/escape.rs`, `bxctl/src/escape.rs` and `proto/tests/escape.rs` into place, and added `pub mod escape;` to `proto/src/lib.rs` after `pub mod class;`. The bxctl file is now the one-line re-export `pub use proto::escape::{escape_json_text, escape_model_text};`, so no bxctl source or test changed and behaviour is identical. `cargo test -p proto --test escape` reports 8 passed; every bxctl suite passes unchanged (the moved escape test now runs against `proto::escape`); `cargo fmt --all` then `make gate` prints `gate: ok` on the first run. | ? | | M4b/01-proto-escape | 2026-09-24 | done | 1 | pass | none | Copied `docs/plans/M4b/files/crates/proto/src/escape.rs`, `bxctl/src/escape.rs` and `proto/tests/escape.rs` into place, and added `pub mod escape;` to `proto/src/lib.rs` after `pub mod class;`. The bxctl file is now the one-line re-export `pub use proto::escape::{escape_json_text, escape_model_text};`, so no bxctl source or test changed and behaviour is identical. `cargo test -p proto --test escape` reports 8 passed; every bxctl suite passes unchanged (the moved escape test now runs against `proto::escape`); `cargo fmt --all` then `make gate` prints `gate: ok` on the first run. | ? |