diff --git a/crates/gatewayd/src/approvals/mod.rs b/crates/gatewayd/src/approvals/mod.rs index b204a7e..2b00a83 100644 --- a/crates/gatewayd/src/approvals/mod.rs +++ b/crates/gatewayd/src/approvals/mod.rs @@ -3,7 +3,7 @@ pub mod text; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::mpsc::Sender; use std::sync::{Mutex, MutexGuard}; @@ -44,6 +44,8 @@ pub enum Judged { #[derive(Debug, Default)] pub struct Registry { posted: Mutex>, + /// Every approval ever posted here, answered or not: for the notice. + ever: Mutex>, } impl Registry { @@ -56,6 +58,10 @@ impl Registry { } pub fn add(&self, posted: Posted) { + self.ever + .lock() + .unwrap_or_else(|p| p.into_inner()) + .insert(posted.approval); self.lock().insert(posted.approval, posted); } @@ -73,6 +79,14 @@ impl Registry { self.lock().values().map(|p| p.session.clone()).collect() } + /// Was approval `id` ever posted here? + pub fn was_posted(&self, id: u64) -> bool { + self.ever + .lock() + .unwrap_or_else(|p| p.into_inner()) + .contains(&id) + } + /// Judge a command from `user`, posted as `post` at `at` in the thread `root`, in the order of /// the spec's section 7. An accepted answer is taken out and sent; nothing else changes. pub fn judge( diff --git a/crates/gatewayd/src/deliver.rs b/crates/gatewayd/src/deliver.rs index 91ab7f3..c1ec3e9 100644 --- a/crates/gatewayd/src/deliver.rs +++ b/crates/gatewayd/src/deliver.rs @@ -3,6 +3,7 @@ use std::os::unix::net::UnixStream; use std::path::Path; +use std::time::{Duration, Instant}; use proto::{ Envelope, ErrorCode, Message, PROTOCOL_VERSION, Turn, TurnEvent, WireError, read_frame, @@ -40,10 +41,36 @@ pub enum Outcome { pub fn approval_text(approval: u64) -> String { format!( - "waiting for approval {approval}: approve or deny it with `bxctl` (Mattermost approvals arrive in M4b)" + "waiting for approval {approval}: brokerd has not sent it here; answer it with bxctl approvals" ) } +/// How long a turn waits for `brokerd` to bring its approval here before posting the notice. +pub const NOTICE_WAIT: Duration = Duration::from_secs(10); + +/// Whether an approval was brought here (and so is posted already), and how long to wait for it. +pub struct Notice<'a> { + pub posted: &'a dyn Fn(u64) -> bool, + pub wait: Duration, +} + +impl Notice<'_> { + fn arrives(&self, id: u64) -> bool { + let until = Instant::now() + self.wait; + loop { + if (self.posted)(id) { + return true; + } + if Instant::now() >= until { + return false; + } + std::thread::sleep( + Duration::from_millis(50).min(until.saturating_duration_since(Instant::now())), + ); + } + } +} + /// "Error: : "; the detail carries `loopd`'s runbook pointer when there is one. pub fn error_text(error: &WireError) -> String { // "Error: : ", where is the snake_case name serde gives the ErrorCode @@ -173,14 +200,23 @@ fn post(poster: &dyn Poster, thread: &Thread, text: String, log: &dyn Fn(&str)) } /// Run a batch's turn and post what comes of it in its thread. A post that fails is logged. -pub fn deliver(poster: &dyn Poster, socket: &Path, batch: &Batch, log: &dyn Fn(&str)) { - // Post in the batch thread. An ApprovalPending event posts `approval_text(approval)` at once. - // Then: Answer -> every part of `split_answer`, in order; Refused -> `error_text`; +pub fn deliver( + poster: &dyn Poster, + socket: &Path, + batch: &Batch, + log: &dyn Fn(&str), + notice: &Notice, +) { + // Post in the batch thread. An ApprovalPending event waits up to `notice.wait` for `brokerd` to + // bring the approval through `gateway.sock`; only when it does not, it posts the notice. Then: + // Answer -> every part of `split_answer`, in order; Refused -> `error_text`; // LoopDown(why) -> log "gatewayd: : " and post LOOP_DOWN. A post that fails is // logged, exactly "gatewayd: cannot post in (thread ): ", and the rest // goes on. let outcome = run_turn(socket, batch, &mut |event| { - if let TurnEvent::ApprovalPending { approval, .. } = event { + if let TurnEvent::ApprovalPending { approval, .. } = event + && !notice.arrives(*approval) + { post(poster, &batch.thread, approval_text(*approval), log); } }); diff --git a/crates/gatewayd/src/serve/approvals.rs b/crates/gatewayd/src/serve/approvals.rs index 1c756d4..27ea071 100644 --- a/crates/gatewayd/src/serve/approvals.rs +++ b/crates/gatewayd/src/serve/approvals.rs @@ -141,7 +141,7 @@ impl Gateway { Ending::NoLonger => Ended::NoLonger, Ending::Answered { by, - reason: _, + reason: refusal, outcome, } => { let by = match self.client.username(&by) { @@ -151,7 +151,10 @@ impl Gateway { match outcome { GatewayOutcome::Runs {} => Ended::Runs { by }, GatewayOutcome::Denied { reason } => Ended::Denied { by, reason }, - GatewayOutcome::Refused {} => Ended::Refused { by, reason: None }, + GatewayOutcome::Refused {} => Ended::Refused { + by, + reason: refusal, + }, GatewayOutcome::Gone {} => Ended::NoLonger, } } diff --git a/crates/gatewayd/src/serve/handle.rs b/crates/gatewayd/src/serve/handle.rs index 2d00adc..3ad1177 100644 --- a/crates/gatewayd/src/serve/handle.rs +++ b/crates/gatewayd/src/serve/handle.rs @@ -4,10 +4,11 @@ use std::sync::Arc; use std::time::{SystemTime, UNIX_EPOCH}; -use crate::deliver::{LOOP_DOWN, deliver}; +use crate::approvals::Judged; +use crate::deliver::{LOOP_DOWN, Notice, deliver}; use crate::mm::{Post, typing}; use crate::serve::{Gateway, Stop}; -use crate::sessions::{BUSY, Batch, Ignored, M4B_COMMAND, Pushed, Route}; +use crate::sessions::{BUSY, Batch, Ignored, Pushed, Route}; use crate::state::InFlight; use crate::ws::WsError; use crate::ws::conn::Ws; @@ -81,8 +82,26 @@ impl Gateway { } Route::Ignore(_) => {} Route::Reply { thread, text } => self.post(&thread.channel, &thread.root, &text), - // Until the approvals module is wired in (task 15), M4a's notice. - Route::Approval { thread, .. } => self.post(&thread.channel, &thread.root, M4B_COMMAND), + Route::Approval { + thread, + command, + user, + post: command_post, + at, + } => { + let approvers = self.config.approvers(); + let judged = self.registry.judge( + &command, + &user, + &thread.root, + &command_post, + at, + approvers, + ); + if let Judged::Reply(text) = judged { + self.post(&thread.channel, &thread.root, &text); + } + } Route::Queue(message) => { if message.joins_thread { self.state.join_thread(&message.thread.root)?; @@ -117,6 +136,7 @@ impl Gateway { channel: batch.thread.channel.clone(), root: batch.thread.root.clone(), })?; + let (registry, wait) = (Arc::clone(&self.registry), self.notice); let (client, socket, log, done) = ( Arc::clone(&self.client), self.loop_socket.clone(), @@ -125,7 +145,12 @@ impl Gateway { ); let (session, thread) = (batch.session.clone(), batch.thread.clone()); let spawned = std::thread::Builder::new().spawn(move || { - deliver(client.as_ref(), &socket, &batch, &|line| log(line)); + let posted = |id: u64| registry.was_posted(id); + let notice = Notice { + posted: &posted, + wait, + }; + deliver(client.as_ref(), &socket, &batch, &|line| log(line), ¬ice); let _ = done.send(batch.session); }); if let Err(e) = spawned { @@ -156,9 +181,14 @@ impl Gateway { /// Show this bot as typing in every thread with a turn running. pub(super) fn typing(&mut self, ws: &mut Ws) -> Result<(), WsError> { - // `for thread in self.queues.threads() { self.seq += 1; - // ws.send_text(&typing(self.seq, &thread.channel, &thread.root))?; }` then Ok(()). + // `let waiting = self.registry.waiting_sessions();` then for each thread, none that waits + // for its approval: A turn waiting for its approval is not working: no typing in its thread. + // Otherwise `self.seq += 1; ws.send_text(...)`; then Ok(()). + let waiting = self.registry.waiting_sessions(); for thread in self.queues.threads() { + if waiting.contains(&format!("mm-{}", thread.root)) { + continue; + } self.seq += 1; ws.send_text(&typing(self.seq, &thread.channel, &thread.root))?; } diff --git a/crates/gatewayd/src/serve/mod.rs b/crates/gatewayd/src/serve/mod.rs index 51eb69b..1b24220 100644 --- a/crates/gatewayd/src/serve/mod.rs +++ b/crates/gatewayd/src/serve/mod.rs @@ -77,6 +77,8 @@ pub struct Tuning { pub poll: Duration, /// Connecting and each REST read. pub rest_timeout: Duration, + /// How long a turn waits for its approval to come through `gateway.sock` before the notice. + pub notice: Duration, } impl Default for Tuning { @@ -86,6 +88,7 @@ impl Default for Tuning { backoff: secs.to_vec(), poll: Duration::from_millis(200), rest_timeout: Duration::from_secs(30), + notice: crate::deliver::NOTICE_WAIT, } } } @@ -112,6 +115,8 @@ pub(crate) struct Gateway { gateway_rx: Receiver, /// The last post of each approval posted here and not yet ended. open: HashMap, + /// How long a turn waits for its approval to come through `gateway.sock` before the notice. + notice: Duration, } /// The wait before attempt `n` (from 0) after a loss. @@ -167,7 +172,7 @@ pub fn run(config: Config, token: Secret, tuning: Tuning, log: Log, stop: &Atomi return Stop::Start(format!("cannot start the gateway.sock thread: {e}")); } let client = Arc::new(Client::new(connector, token, tuning.rest_timeout)); - let mut g = Gateway::new(config, client, state, log, gateway_rx); + let mut g = Gateway::new(config, client, state, log, gateway_rx, tuning.notice); let mut failures = 0; loop { if stop.load(Ordering::SeqCst) { @@ -235,6 +240,7 @@ impl Gateway { state: State, log: Log, gateway_rx: Receiver, + notice: Duration, ) -> Gateway { let (done_tx, done_rx) = mpsc::channel(); let limit = usize::try_from(config.limits.queue).unwrap_or(usize::MAX); @@ -258,6 +264,7 @@ impl Gateway { registry: Arc::new(Registry::new()), gateway_rx, open: HashMap::new(), + notice, } } diff --git a/crates/gatewayd/src/sessions.rs b/crates/gatewayd/src/sessions.rs index ad61878..79b1ec0 100644 --- a/crates/gatewayd/src/sessions.rs +++ b/crates/gatewayd/src/sessions.rs @@ -7,7 +7,6 @@ use proto::SessionId; use crate::mm::Post; -pub const M4B_COMMAND: &str = "approvals over Mattermost arrive in M4b; use `bxctl approvals`"; pub const UNKNOWN_COMMAND: &str = "unknown command; the commands are !approve and !deny"; pub const BUSY: &str = "busy: too many messages are waiting in this conversation"; diff --git a/crates/gatewayd/tests/deliver.rs b/crates/gatewayd/tests/deliver.rs index 642e981..02ffb64 100644 --- a/crates/gatewayd/tests/deliver.rs +++ b/crates/gatewayd/tests/deliver.rs @@ -1,6 +1,6 @@ //! A turn on `loop.sock` and its answer in the thread, against a fake `loopd`: the answer, long //! answers, approvals, errors, an unknown session, and a loop that is not there (M4a spec, section -//! 8). Do not edit. +//! 8; M4b spec, section 7, for the notice). Do not edit. #[path = "support/fake_loop.rs"] mod fake_loop; @@ -11,7 +11,7 @@ use std::sync::Mutex; use std::time::Duration; use fake_loop::{Reply, done, error, event, serve_loop}; -use gatewayd::deliver::{EMPTY_ANSWER, LOOP_DOWN, MAX_POST, Poster, deliver, split_answer}; +use gatewayd::deliver::{EMPTY_ANSWER, LOOP_DOWN, MAX_POST, Notice, Poster, deliver, split_answer}; use gatewayd::mm::MmError; use gatewayd::sessions::{Batch, Thread}; use proto::{DataClass, ErrorCode, SessionId, Timestamp, TurnEvent}; @@ -64,9 +64,18 @@ fn batch(resume: bool, text: &str) -> Batch { fn run(dir: &TempDir, poster: &Record, batch: &Batch) -> Vec { let log = Mutex::new(Vec::new()); - deliver(poster, &dir.path().join("loop.sock"), batch, &|line| { - log.lock().unwrap().push(line.to_string()) - }); + let never = |_: u64| false; + let notice = Notice { + posted: &never, + wait: Duration::ZERO, + }; + deliver( + poster, + &dir.path().join("loop.sock"), + batch, + &|line| log.lock().unwrap().push(line.to_string()), + ¬ice, + ); log.into_inner().unwrap() } @@ -127,7 +136,7 @@ fn an_approval_is_announced_once_before_the_answer() { assert_eq!( poster.texts(), [ - "waiting for approval 42: approve or deny it with `bxctl` (Mattermost approvals arrive in M4b)", + "waiting for approval 42: brokerd has not sent it here; answer it with bxctl approvals", "done" ] ); diff --git a/crates/gatewayd/tests/deliver_notice.rs b/crates/gatewayd/tests/deliver_notice.rs new file mode 100644 index 0000000..1fdf0cb --- /dev/null +++ b/crates/gatewayd/tests/deliver_notice.rs @@ -0,0 +1,95 @@ +//! The notice for an approval that `brokerd` did not bring: posted only when it has not arrived +//! within the wait (M4b spec, section 7). Do not edit. + +#[path = "support/fake_loop.rs"] +mod fake_loop; +#[path = "support/tmp.rs"] +mod tmp; + +use std::sync::Mutex; +use std::time::{Duration, Instant}; + +use fake_loop::{done, event, serve_loop}; +use gatewayd::deliver::{Notice, Poster, deliver}; +use gatewayd::mm::MmError; +use gatewayd::sessions::{Batch, Thread}; +use proto::{SessionId, Timestamp, TurnEvent}; +use tmp::TempDir; + +const DM: &str = "d0000000000000000000000000"; +const ROOT: &str = "r0000000000000000000000000"; +const NOTICE: &str = + "waiting for approval 42: brokerd has not sent it here; answer it with bxctl approvals"; + +#[derive(Default)] +struct Record(Mutex>); + +impl Poster for Record { + fn post(&self, _: &str, _: &str, text: &str) -> Result<(), MmError> { + self.0.lock().unwrap().push(text.to_string()); + Ok(()) + } +} + +/// A turn whose loop says approval 42 is pending, then answers; what was posted, and how long it took. +fn turn(tag: &str, posted: &dyn Fn(u64) -> bool, wait: Duration) -> (Vec, Duration) { + let dir = TempDir::new(tag); + let socket = dir.path().join("loop.sock"); + let expires = Timestamp::parse("2026-09-24T08:20:00.000Z").unwrap(); + let _turns = serve_loop(&socket, move |_, _| { + vec![ + event(TurnEvent::ApprovalPending { + approval: 42, + tool: "read_file".to_string(), + expires, + }), + done("done"), + ] + }); + let batch = Batch { + session: SessionId::new(&format!("mm-{ROOT}")).unwrap(), + thread: Thread { + channel: DM.to_string(), + root: ROOT.to_string(), + }, + resume: false, + text: "go".to_string(), + }; + let poster = Record::default(); + let started = Instant::now(); + deliver(&poster, &socket, &batch, &|_| {}, &Notice { posted, wait }); + let took = started.elapsed(); + (poster.0.into_inner().unwrap(), took) +} + +#[test] +fn brought_already_no_notice_and_no_wait() { + let (posts, took) = turn("notice-there", &|id| id == 42, Duration::from_secs(5)); + assert_eq!(posts, ["done"]); + assert!(took < Duration::from_secs(1), "{took:?}"); +} + +#[test] +fn brought_during_the_wait_no_notice() { + let started = Instant::now(); + let later = move |id: u64| id == 42 && started.elapsed() > Duration::from_millis(300); + let (posts, took) = turn("notice-later", &later, Duration::from_secs(5)); + assert_eq!(posts, ["done"]); + assert!(took < Duration::from_secs(2), "{took:?}"); +} + +#[test] +fn never_brought_the_notice_after_the_wait() { + let (posts, took) = turn("notice-never", &|_| false, Duration::from_millis(400)); + assert_eq!(posts, [NOTICE, "done"]); + assert!( + took >= Duration::from_millis(400) && took < Duration::from_secs(3), + "{took:?}" + ); +} + +#[test] +fn another_approval_brought_does_not_count() { + let (posts, _) = turn("notice-other", &|id| id == 7, Duration::from_millis(200)); + assert_eq!(posts, [NOTICE, "done"]); +} diff --git a/crates/gatewayd/tests/serve.rs b/crates/gatewayd/tests/serve.rs index e5e1b1c..6227145 100644 --- a/crates/gatewayd/tests/serve.rs +++ b/crates/gatewayd/tests/serve.rs @@ -17,7 +17,7 @@ use fake_loop::{done, serve_loop}; use fake_mm::{BOT, BOT_NAME, DM, EVE, EVE_DM, FakeMm, KYLE, id, post}; use gateway::{OTHER, SHARED, WAIT, config, loop_dir, read_state, start, up}; use gatewayd::serve::Stop; -use gatewayd::sessions::{BUSY, M4B_COMMAND}; +use gatewayd::sessions::BUSY; use tmp::TempDir; #[test] @@ -204,7 +204,11 @@ fn commands_are_answered_without_a_turn() { ws.posted(&post(&p1, KYLE, DM, "", "!approve 3", 5), "D"); assert_eq!( mm.wait_posts(1, WAIT), - [(DM.to_string(), p1, M4B_COMMAND.to_string())] + [( + DM.to_string(), + p1, + "approval 3 is not pending here".to_string() + )] ); assert!(turns.recv_timeout(Duration::from_millis(200)).is_err()); } diff --git a/crates/gatewayd/tests/serve_answers.rs b/crates/gatewayd/tests/serve_answers.rs new file mode 100644 index 0000000..0a343e7 --- /dev/null +++ b/crates/gatewayd/tests/serve_answers.rs @@ -0,0 +1,248 @@ +//! Approvals answered from Mattermost end to end: `!approve` and `!deny` from an approver in the +//! approval's thread reach `brokerd`, the post shows what came of it, typing pauses while the turn +//! waits, and everything else is answered with its reason; the notice comes only when `brokerd` +//! never brought the approval (M4b spec, section 7). Do not edit. + +#[path = "support/brokerd_client.rs"] +mod brokerd_client; +#[path = "support/fake_loop.rs"] +mod fake_loop; +#[path = "support/fake_mm.rs"] +mod fake_mm; +#[path = "support/gateway.rs"] +mod gateway; +#[path = "support/tmp.rs"] +mod tmp; + +use std::path::PathBuf; +use std::sync::{Mutex, mpsc}; +use std::time::{Duration, Instant}; + +use brokerd_client::{Asking, ask, pending}; +use fake_loop::{done, event, serve_loop}; +use fake_mm::{DM, EVE, FakeMm, KYLE, WsPeer, id, post}; +use gateway::{Running, WAIT, loop_dir, start}; +use gatewayd::approvals::NOT_APPROVER; +use gatewayd::config::Config; +use proto::{GatewayOutcome, Timestamp, Turn, TurnEvent}; +use tmp::TempDir; + +/// KYLE and EVE may talk; only KYLE approves. +fn config(home: &TempDir, url: &str) -> Config { + let text = format!( + "[mattermost]\nurl = \"{url}\"\n[secrets.mattermost_token]\nenv = \"NOT_READ_BY_RUN\"\n[allow]\nusers = [\"{KYLE}\", \"{EVE}\"]\n[approvals]\nusers = [\"{KYLE}\"]\n[paths]\nhome = \"{}\"\n[limits]\ntyping_every_ms = 100\n", + home.path().display() + ); + Config::parse(&text).unwrap() +} + +/// A loop whose first turn waits for `go`, then sends `before` and answers "done". +fn held(home: &TempDir, before: Vec) -> (mpsc::Receiver, mpsc::Sender<()>) { + loop_dir(home); + let (go_tx, go_rx) = mpsc::channel::<()>(); + let go = Mutex::new(go_rx); + let before = Mutex::new(Some(before)); + let turns = serve_loop(&home.path().join("run/loop/loop.sock"), move |_, _| { + let _ = go.lock().unwrap().recv_timeout(Duration::from_secs(20)); + let mut replies: Vec<_> = before + .lock() + .unwrap() + .take() + .unwrap_or_default() + .into_iter() + .map(event) + .collect(); + replies.push(done("done")); + replies + }); + (turns, go_tx) +} + +struct Stage { + _home: TempDir, + mm: FakeMm, + _running: Running, + ws: WsPeer, + go: mpsc::Sender<()>, + socket: PathBuf, + root: String, +} + +/// gatewayd up, a direct message from KYLE whose turn is held, and the approval's socket. +fn stage(tag: &str, before: Vec) -> Stage { + let home = TempDir::new(tag); + let (turns, go) = held(&home, before); + let mm = FakeMm::start(); + let running = start(config(&home, &mm.url())); + let mut ws = mm.next_ws(WAIT); + running.wait_log("gatewayd: connected to "); + let root = id('p', 1); + ws.posted(&post(&root, KYLE, DM, "", "read my notes", 5), "D"); + turns.recv_timeout(WAIT).expect("the turn started"); + let socket = home.path().join("run/gateway/gateway.sock"); + Stage { + _home: home, + mm, + _running: running, + ws, + go, + socket, + root, + } +} + +impl Stage { + fn ask(&self, approval: u64) -> Asking { + ask( + &self.socket, + pending(approval, &format!("mm-{}", self.root), r#"{"path":"/n/a"}"#), + ) + .unwrap() + } + + fn command(&mut self, n: u32, user: &str, root: &str, text: &str, at: i64) { + self.ws + .posted(&post(&id('c', n), user, DM, root, text, at), "D"); + } + + /// The text posted in the thread after the approval, within five seconds. + fn replies(&self, after: usize) -> Vec { + let posts = self.mm.wait_posts(after + 1, WAIT); + posts[after..].iter().map(|(_, _, t)| t.clone()).collect() + } +} + +fn wait_until(what: &str, mut done: impl FnMut() -> bool) { + let until = Instant::now() + WAIT; + while !done() { + assert!(Instant::now() < until, "never happened: {what}"); + std::thread::sleep(Duration::from_millis(20)); + } +} + +#[test] +fn approved_by_the_approver_the_post_says_it_runs_and_typing_pauses_meanwhile() { + let mut s = stage("ans-approve", vec![]); + s.ws.typing_within(Duration::from_millis(300)); + let mut asking = s.ask(42); + s.ws.typing_within(Duration::from_millis(200)); + assert!( + s.ws.typing_within(Duration::from_millis(400)).is_empty(), + "no typing while it waits for approval" + ); + let root = s.root.clone(); + s.command(1, KYLE, &root, "!approve 42", 5); + let reply = asking.answer(WAIT).expect("the answer reaches brokerd"); + assert_eq!( + ( + reply.approval, + reply.approved, + reply.by.as_str(), + reply.post.clone(), + reply.reason.clone() + ), + (42, true, KYLE, id('c', 1), None) + ); + asking.outcome(GatewayOutcome::Runs {}); + let edits = s.mm.wait_edits(1, WAIT); + assert_eq!(edits[0].0, FakeMm::post_id(0)); + assert!( + edits[0] + .1 + .ends_with("\n\n✅ Approved by @kyle: the call runs."), + "{}", + edits[0].1 + ); + assert_eq!(s.mm.posts().len(), 1, "nothing else posted in the thread"); + wait_until("typing again", || { + !s.ws.typing_within(Duration::from_millis(300)).is_empty() + }); + s.go.send(()).unwrap(); +} + +#[test] +fn refused_with_a_reason_the_post_says_so() { + let mut s = stage("ans-deny", vec![]); + let mut asking = s.ask(42); + let root = s.root.clone(); + s.command(1, KYLE, &root, "@boxmaker-straylight !deny 42 not now", 5); + let reply = asking.answer(WAIT).expect("the answer reaches brokerd"); + assert_eq!( + (reply.approved, reply.reason.as_deref()), + (false, Some("not now")) + ); + asking.outcome(GatewayOutcome::Refused {}); + let edits = s.mm.wait_edits(1, WAIT); + assert!( + edits[0].1.ends_with("\n\n❌ Refused by @kyle: `not now`."), + "{}", + edits[0].1 + ); + s.go.send(()).unwrap(); +} + +#[test] +fn nobody_else_and_nothing_else_answers_it() { + let mut s = stage("ans-refused", vec![]); + let mut asking = s.ask(42); + let root = s.root.clone(); + let elsewhere = id('p', 9); + s.command(1, EVE, &root, "!approve 42", 5); + s.command(2, KYLE, &elsewhere, "!approve 42", 6); + s.command(3, KYLE, &root, "!approve 42", 1); + s.command(4, KYLE, &root, "!approve 7", 7); + s.command(5, KYLE, &root, "!approve 42 please", 8); + assert_eq!( + s.replies(1)[..5], + [ + NOT_APPROVER.to_string(), + "approval 42 is answered in its own thread".to_string(), + "approval 42 was answered before it was shown in full".to_string(), + "approval 7 is not pending here".to_string(), + "use !approve or !deny ".to_string(), + ] + ); + assert_eq!( + asking.answer(Duration::from_millis(300)), + None, + "nothing reached brokerd" + ); + s.command(6, KYLE, &root, "!approve 42", 9); + assert!( + asking.answer(WAIT).is_some(), + "the approver, in the thread, after the post" + ); + asking.outcome(GatewayOutcome::Runs {}); + s.go.send(()).unwrap(); +} + +#[test] +fn the_notice_only_when_brokerd_never_brought_it() { + let expires = Timestamp::parse("2026-09-24T08:20:00.000Z").unwrap(); + let pending_event = TurnEvent::ApprovalPending { + approval: 42, + tool: "read_file".to_string(), + expires, + }; + let s = stage("ans-notice", vec![pending_event.clone()]); + s.go.send(()).unwrap(); + let posts = s.mm.wait_posts(2, WAIT); + let texts: Vec<&str> = posts.iter().map(|(_, _, t)| t.as_str()).collect(); + assert_eq!( + texts, + [ + "waiting for approval 42: brokerd has not sent it here; answer it with bxctl approvals", + "done" + ] + ); + + let s = stage("ans-no-notice", vec![pending_event]); + let asking = s.ask(42); + s.go.send(()).unwrap(); + let posts = s.mm.wait_posts(2, WAIT); + assert!(posts[0].2.starts_with("**Approval 42**"), "{posts:?}"); + assert_eq!(posts[1].2, "done"); + std::thread::sleep(Duration::from_millis(500)); + assert_eq!(s.mm.posts().len(), 2, "no notice: {:?}", s.mm.posts()); + asking.close(); +} diff --git a/crates/gatewayd/tests/support/gateway.rs b/crates/gatewayd/tests/support/gateway.rs index 6af0432..0e2c94e 100644 --- a/crates/gatewayd/tests/support/gateway.rs +++ b/crates/gatewayd/tests/support/gateway.rs @@ -98,6 +98,7 @@ pub fn start(config: Config) -> Running { backoff: vec![Duration::from_millis(50)], poll: Duration::from_millis(20), rest_timeout: WAIT, + notice: Duration::from_millis(300), }; let (s, l) = (Arc::clone(&stop), Arc::clone(&log)); let handle = std::thread::spawn(move || { diff --git a/docs/implementer-log.md b/docs/implementer-log.md index d571baf..4067413 100644 --- a/docs/implementer-log.md +++ b/docs/implementer-log.md @@ -6,6 +6,7 @@ reviewer adds findings under "Reviews" once per milestone. | Task | Date | Status | Gate runs | First gate | Deviations | Notes | Model | |---|---|---|---|---|---|---|---| +| M4b/15-gatewayd-answers | 2026-09-24 | done | 1 | pass | none | Copied `serve_answers.rs` and `deliver_notice.rs` (new), and new versions of `deliver.rs`, `serve.rs` (`!approve 3` now "approval 3 is not pending here") and `support/gateway.rs` (`Tuning` gains `notice`) into `crates/gatewayd/tests/`. `approvals/mod.rs`: added `ever: Mutex>` (doc "Every approval ever posted here, answered or not: for the notice.", `HashSet` imported beside `HashMap`); `add` inserts `posted.approval` into `ever` before into `posted`, and added `was_posted`. `deliver.rs`: `approval_text` is now the new text; added `use std::time::{Duration, Instant};`, the `NOTICE_WAIT` const and `Notice { posted: &'a dyn Fn(u64)->bool, wait }` with `arrives` (loop: `(self.posted)(id)` -> true, past `Instant::now()+wait` taken once -> false, else sleep `50ms.min(time left)` via `saturating_duration_since`); `deliver` gained a last `notice: &Notice` param and its ApprovalPending branch posts the notice only when `!notice.arrives(*approval)`. `serve/mod.rs`: `Tuning.notice` (doc, `crate::deliver::NOTICE_WAIT` in Default); `Gateway.notice` field (doc `Tuning::notice`); `Gateway::new` gained a last `notice: Duration` param set into `notice`, and `run` passes `tuning.notice`. `serve/handle.rs`: `Route::Approval` arm now calls `self.registry.judge(&command,&user,&thread.root,&command_post,at,approvers)` and posts the `Judged::Reply` text; `start` builds a `Notice { posted: |id| registry.was_posted(id), wait: self.notice }` and passes it to `deliver`; `typing` takes `self.registry.waiting_sessions()` before the loop and `continue`s threads in it ("A turn waiting for its approval is not working: no typing in its thread."); imported `Judged` and `Notice`, dropped `M4B_COMMAND`. `sessions.rs`: deleted `M4B_COMMAND`. `serve/approvals.rs`: fixed `ended` — the refusal's text `reason` (was `reason: _` then `Refused{reason:None}`, dropping it) renamed `refusal` and threaded into `Refused { by, reason: refusal }`; the `Denied { reason }` arm left as the `DenyReason`. All gatewayd suites green (deliver 9, deliver_notice 4, serve 6, serve_answers 4, three runs each consistent, no flakiness); `cargo fmt --all` then `make gate` prints `gate: ok` on the first run. | ? | | M4b/14-gatewayd-serve-approvals | 2026-09-24 | done | 1 | pass | none | Copied the skeleton `approvals.rs`, `serve_approvals.rs`, the new `support/brokerd_client.rs`, and a new `support/fake_mm.rs` (which gains `edits`/`wait_edits`, `username` via `GET /api/v4/users/{user}`, and `refuse_posts` returning 400 on `POST /api/v4/posts`). In `serve/mod.rs`: added `mod approvals;` before `mod handle;` and the `HashMap`, `Registry` and `gateway::{self, ToLoop}` imports; the three `registry`/`gateway_rx`/`open` fields on `Gateway`; `Gateway::new` gained a `gateway_rx: Receiver` last parameter and set `registry`, `gateway_rx`, `open`; `run` now `listen`s `gateway.sock` and `serve`s it on its own thread before `Client::new`, returning `Stop::Start` on either failure and passing `gateway_rx` to `Gateway::new`; `connected` calls `self.mark_left_behind()?` after the `take_in_flight` loop; `event_loop` calls `self.gateway_requests()?` after `self.finished()?`. Filled the four `todo!()`s in `approvals.rs`: `place` (`turn_of` the session's in-flight turn, else the first approver's `direct_channel` as a new empty thread, each error string as given); `show` was already written; `ended` removes from the registry and state, returns when there is no `open`, maps `Ending::NoLonger`/`Answered` to the `Ended` (the `by` run through `username` as `@` or left the id, and `outcome` → `Runs`/`Denied`/`Refused{reason:None}`/`Gone`); `mark_left_behind` edits each left-behind approval's last post with the title and the `Restarted` line; `mark` patches the post and logs a failure. The one clippy warning was the unused `reason` of `Ending::Answered` (the real reason comes from `outcome`), ignored with `reason: _`. All five `serve_approvals` tests pass three times; the whole gatewayd suite is green; `cargo fmt --all` then `make gate` prints `gate: ok` on the first run. | ? | | M4b/13-gatewayd-gateway-sock | 2026-09-24 | done | 1 | pass | none | Copied `docs/plans/M4b/files/crates/gatewayd/src/gateway.rs` to `crates/gatewayd/src/` and `docs/plans/M4b/files/crates/gatewayd/tests/gateway_sock.rs` to `crates/gatewayd/tests/`, then added `pub mod gateway;` to `lib.rs` after `pub mod deliver;`. Filled the four `todo!()`s the skeleton left in `gateway.rs`: `end` (send `ToLoop::Ended { approval, ending }` to the loop, the only thing that touches the loop); `frames` (`try_clone` the stream, spawn a reader thread that hands whole frames over an mpsc channel so the connection waits with a timeout without losing half a frame, `Ok(rx)` — the same as brokerd's courier); `listen` (the parent dir made 0700 via a recursive `DirBuilder` then `set_permissions`, a stale socket removed, anything else left in place as "cannot listen on : something else is there", bind, then the socket made 0600, with the spec's error strings and paths via `.display()`); and `serve` (spawn an accept loop that skips `incoming()` errors and starts a `connection` thread per stream, logging a spawn failure as "gatewayd: cannot start a thread for an approval: ", keeping a separate `log` clone for that message). The skeleton's `ToLoop` was already complete with `Ended`, so it needed no change. All six `gateway_sock` tests pass three times running in ~1 s; `make gate` prints `gate: ok` on the first run. | ? | | M4b/12-gatewayd-state-approvals | 2026-09-24 | done | 1 | pass | none | Copied `docs/plans/M4b/files/crates/gatewayd/tests/state_approvals.rs` into `crates/gatewayd/tests/`. In `state.rs`: added the `#[serde(default)] approvals: Vec` field after `in_flight` (optional so an M4a state file loads unchanged) and the `ApprovalPosts { approval, channel, posts }` struct (`#[serde(deny_unknown_fields)`) after `struct StateFile`; in `problem` the `in_flight` loop now has a loop over `approvals` requiring a `valid_id` channel and at least one `valid_id` post, else "approval {}'s posts are not valid"; added `turn_of` (find the `in_flight` entry of a session, cloned), `add_approval` (replace any entry of the same approval, push, save), `remove_approval` (retain by approval, save only when one was removed) and `take_approvals` (`std::mem::take`, save only when there were some, return them) at the end of `impl State`. `cargo fmt --all` then `cargo test -p gatewayd --test state_approvals --test state --test serve_restart` green (4, 4 and 7 passed); `make gate` prints `gate: ok` on the first run. | ? |