From c6485eee124e899f5b33477750d3cb8a5b4cdc82 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 23 Sep 2026 10:58:23 -0400 Subject: [PATCH] feat(agentd)!: list only the sign-ins a context reported, unless --all didbot oauth pending now lists what some context here reported seeing. The pending ask takes an optional all flag and its answer an optional unseen list, each left off when empty. --all prints the unseen ones, each marked not-seen-by-any-agent-here. Co-Authored-By: Claude Opus 5.5 (1M context) Change-Id: I337f345e9c5a55505b59d84a524d5dad79390153 --- .../fixtures/protocol/answer-pending-all.json | 38 +++++++++ .../fixtures/protocol/ask-pending-all.json | 5 ++ crates/didbot-agentd/src/bin/didbot-oauth.rs | 81 ++++++++++++++++--- crates/didbot-agentd/src/protocol.rs | 41 ++++++++-- crates/didbot-agentd/src/serve.rs | 61 ++++++++++++-- crates/didbot-agentd/tests/fixtures.rs | 37 ++++++++- 6 files changed, 237 insertions(+), 26 deletions(-) create mode 100644 crates/didbot-agentd/fixtures/protocol/answer-pending-all.json create mode 100644 crates/didbot-agentd/fixtures/protocol/ask-pending-all.json diff --git a/crates/didbot-agentd/fixtures/protocol/answer-pending-all.json b/crates/didbot-agentd/fixtures/protocol/answer-pending-all.json new file mode 100644 index 00000000..ae9f1bd5 --- /dev/null +++ b/crates/didbot-agentd/fixtures/protocol/answer-pending-all.json @@ -0,0 +1,38 @@ +{ + "pending": [ + { + "clientOrigin": "https://client.example", + "cut": [ + "repo:app.example.note" + ], + "expiresAt": "2026-09-22T12:34:56Z", + "firstTime": true, + "granted": [ + "atproto" + ], + "requested": [ + "atproto", + "repo:app.example.note" + ], + "rule": "scope-ceiling", + "token": "k-3f9a", + "verdict": "narrow" + } + ], + "unseen": [ + { + "clientOrigin": "https://stranger.example", + "expiresAt": "2026-09-22T12:35:10Z", + "firstTime": true, + "granted": [ + "atproto" + ], + "requested": [ + "atproto" + ], + "token": "k-7c21", + "verdict": "allow" + } + ], + "version": 2 +} diff --git a/crates/didbot-agentd/fixtures/protocol/ask-pending-all.json b/crates/didbot-agentd/fixtures/protocol/ask-pending-all.json new file mode 100644 index 00000000..f88ebeea --- /dev/null +++ b/crates/didbot-agentd/fixtures/protocol/ask-pending-all.json @@ -0,0 +1,5 @@ +{ + "all": true, + "asks": "pending", + "version": 2 +} diff --git a/crates/didbot-agentd/src/bin/didbot-oauth.rs b/crates/didbot-agentd/src/bin/didbot-oauth.rs index 04dd302e..adda809c 100644 --- a/crates/didbot-agentd/src/bin/didbot-oauth.rs +++ b/crates/didbot-agentd/src/bin/didbot-oauth.rs @@ -20,17 +20,18 @@ use didbot_agentd::cli::{ask, choose, flag, one_line, positional, printable, soc use didbot_agentd::decisions::Record; use didbot_agentd::direct::{Direct, Named, TokenFlag}; use didbot_agentd::protocol::{ - Approve, DecisionForAccount, Decline, Message, Pending, Show, VERSION, + Answer, Approve, DecisionForAccount, Decline, Message, Pending, Show, VERSION, }; const USAGE: &str = "\ -didbot oauth pending what has asked to sign in as you +didbot oauth pending sign-ins an agent here reported seeing +didbot oauth pending --all and the ones none did, each marked didbot oauth approve let one of them in didbot oauth decline [--reason WHY] turn one of them down -When a sign-in has not reached you -- no hook saw the client print its URL, -or the poll has not come back yet -- name it by that URL instead: +When a sign-in has not reached you -- no hook saw the client print its URL -- +name it by that URL instead: didbot oauth show look one up and print it didbot oauth approve --url let that one in @@ -42,9 +43,10 @@ own. A token names one request: it is single use and it is not an account. With no daemon on this host -- a CI job that provisioned itself, or a host handed a token by an operator -- set DIDBOT_PDS and DIDBOT_ACCOUNT_TOKEN (or DIDBOT_ACCOUNT_TOKEN_FILE) and the same commands talk to that server as that one -account. --direct insists on it rather than trying the socket first. Run the -binary by name, didbot-oauth, for that: `didbot oauth` hands nothing from the -environment's credentials to what it runs. +account, where pending lists every sign-in waiting for it. --direct insists on +it rather than trying the socket first. Run the binary by name, didbot-oauth, +for that: `didbot oauth` hands nothing from the environment's credentials to +what it runs. An account whose document names an OpenID Connect identity signs in with a token from that issuer instead, at the server DIDBOT_PDS names: @@ -129,7 +131,7 @@ fn main() -> ExitCode { } match command { - Some("pending") => pending(), + Some("pending") => pending(&args[1..]), Some("show") => show(&args[1..]), Some("approve") => approve(&args[1..]), Some("decline") => decline(&args[1..]), @@ -148,15 +150,21 @@ fn main() -> ExitCode { } } -fn pending() -> ExitCode { - match ask(&Message::Pending(Pending { version: VERSION })) { +/// What has asked to sign in: the sign-ins an agent here reported seeing, +/// and with `--all` the rest. +fn pending(args: &[String]) -> ExitCode { + let all = args.iter().any(|arg| arg == "--all"); + match ask(&Message::Pending(Pending { + version: VERSION, + all, + })) { Ok(answer) => { - if let Some(trouble) = answer.trouble { + if let Some(trouble) = &answer.trouble { eprintln!("didbot oauth pending: {trouble}"); return ExitCode::FAILURE; } - for decision in answer.pending.unwrap_or_default() { - println!("{}", one_line(&decision)); + for line in pending_lines(&answer) { + println!("{line}"); } ExitCode::SUCCESS } @@ -167,6 +175,22 @@ fn pending() -> ExitCode { } } +/// What starts the line of a sign-in no agent here reported seeing. +const UNSEEN: &str = "not-seen-by-any-agent-here"; + +/// The lines `pending` prints: the sign-ins an agent here reported seeing, +/// then the ones none did, each marked first so it is read before anything +/// else on its line. +fn pending_lines(answer: &Answer) -> Vec { + let seen = answer.pending.iter().flatten().map(one_line); + let unseen = answer + .unseen + .iter() + .flatten() + .map(|decision| format!("{UNSEEN} {}", one_line(decision))); + seen.chain(unseen).collect() +} + /// Look one decision up by the URL a client printed, and print it. fn show(args: &[String]) -> ExitCode { let named = positional(args) @@ -435,12 +459,43 @@ mod tests { "{command} is not in the usage" ); } + assert!(USAGE.contains("didbot oauth pending --all"), "{USAGE}"); // And nothing here asks for an account, which is the property the // whole design turns on. assert!(!USAGE.contains("--as"), "{USAGE}"); assert!(!USAGE.contains("did:"), "{USAGE}"); } + /// A sign-in no agent here reported comes after the rest, marked first + /// on its line, with its token still last to copy. + #[test] + fn a_sign_in_no_agent_here_reported_is_marked_first_on_its_line() { + let decision = |origin: &str, token: &str| DecisionForAccount { + token: Some(token.into()), + client_origin: origin.into(), + first_time: true, + requested: vec!["atproto".into()], + granted: vec!["atproto".into()], + cut: Vec::new(), + rule: None, + reason: None, + verdict: "allow".into(), + expires_at: "2026-09-23T12:04:00Z".into(), + }; + let answer = Answer::quiet() + .and_pending(vec![decision("https://app.example", "k1")]) + .and_unseen(vec![decision("https://stranger.example", "k2")]); + + let lines = pending_lines(&answer); + assert_eq!(lines.len(), 2, "{lines:?}"); + assert!(lines[0].starts_with("https://app.example "), "{lines:?}"); + assert!( + lines[1].starts_with(&format!("{UNSEEN} https://stranger.example ")), + "{lines:?}" + ); + assert!(lines[1].ends_with(" token=k2"), "{lines:?}"); + } + #[test] fn a_decline_reads_its_token_and_its_reason_from_one_line() { // The shape `didbot oauth decline --reason WHY` produces, and diff --git a/crates/didbot-agentd/src/protocol.rs b/crates/didbot-agentd/src/protocol.rs index 0f68787e..ad9c0505 100644 --- a/crates/didbot-agentd/src/protocol.rs +++ b/crates/didbot-agentd/src/protocol.rs @@ -26,6 +26,9 @@ use serde::{Deserialize, Serialize}; /// sent is still read and answered — see `serve::Daemon::consider`, which /// refuses a version newer than its own and nothing else — and an adapter /// still on version 1 finds the fields it knows where they were. +/// +/// A field added within a version is optional and left off when empty, so a +/// component that predates it reads the message as it always did. pub const VERSION: u32 = 2; /// The longest request line the daemon reads, in bytes, newline included. @@ -210,15 +213,20 @@ pub struct Show { /// A caller asking what the daemon is holding. /// -/// Answered with everything, not with one context's share. A `report` names -/// the context it is about and is answered with that context's decisions -/// only; this arrives from a command line, which names nothing the daemon -/// can check — and the ceiling on that is the user account, as -/// [`crate::socket`] already sets out. +/// Answered for every context, not one. A `report` names the context it is +/// about and is answered with that context's decisions only; this arrives +/// from a command line, which names nothing the daemon can check — and the +/// ceiling on that is the user account, as [`crate::socket`] already sets +/// out. [`Answer::pending`] carries the sign-ins their context reported +/// seeing, and [`Pending::all`] asks for the rest in [`Answer::unseen`]. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Pending { /// The wire version this message was written against. pub version: u32, + /// Whether to answer with the sign-ins their context has not reported + /// seeing, too. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub all: bool, } /// One sign-in decision, as an agent is shown it. @@ -286,9 +294,15 @@ pub struct Answer { /// Why there is nothing to report, when there should have been. #[serde(default, skip_serializing_if = "Option::is_none")] pub trouble: Option, - /// Sign-ins waiting on a decision, for the context that asked. + /// Sign-ins waiting on a decision: a reporting context's own that it + /// reported seeing, every one its context reported for a [`Pending`] ask, + /// or the one a [`Show`] named. #[serde(default, skip_serializing_if = "Option::is_none")] pub pending: Option>, + /// Sign-ins waiting that their context has not reported seeing, carried + /// only for a [`Pending`] ask that sets [`Pending::all`]. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub unseen: Option>, /// What the approved login may do now: the request as the ceiling grants /// it, which is less than was asked for while the ceiling narrows it. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -307,6 +321,7 @@ impl Answer { done: None, trouble: None, pending: None, + unseen: None, granted: None, requested: None, } @@ -349,6 +364,16 @@ impl Answer { self } + /// Carry the sign-ins their context has not reported seeing, left off + /// when there are none, as [`Answer::and_pending`] does. + #[must_use] + pub fn and_unseen(mut self, unseen: Vec) -> Self { + if !unseen.is_empty() { + self.unseen = Some(unseen); + } + self + } + /// Carry what an approval's client requested and what it was granted. #[must_use] pub fn and_granted(mut self, requested: Vec, granted: Vec) -> Self { @@ -499,6 +524,10 @@ mod tests { let Message::Pending(pending) = message else { panic!("a question"); }; + assert!( + !pending.all, + "a line without the flag asks for what was seen" + ); assert_eq!( serde_json::to_string(&Message::Pending(pending)).unwrap(), line diff --git a/crates/didbot-agentd/src/serve.rs b/crates/didbot-agentd/src/serve.rs index 87e5aa89..9d9343e4 100644 --- a/crates/didbot-agentd/src/serve.rs +++ b/crates/didbot-agentd/src/serve.rs @@ -178,15 +178,25 @@ impl Daemon { } } - /// Everything this daemon is holding, for a caller that cannot say which - /// context it is. See [`crate::protocol::Pending`]. + /// What this daemon is holding, for a caller that cannot say which + /// context it is: the sign-ins their context reported seeing, and the + /// rest only when asked for all. See [`crate::protocol::Pending`]. pub async fn pending(&self, pending: crate::protocol::Pending) -> Answer { if let Some(refusal) = too_new(pending.version) { return refusal; } - self.held.lock().await.sweep(OffsetDateTime::now_utc()); - let held = self.held.lock().await.all(); - Answer::quiet().and_pending(held.iter().map(DecisionForAccount::from).collect()) + let mut held = self.held.lock().await; + held.sweep(OffsetDateTime::now_utc()); + let seen = held.seen(); + let unseen = if pending.all { + held.unseen() + } else { + Vec::new() + }; + drop(held); + Answer::quiet() + .and_pending(seen.iter().map(DecisionForAccount::from).collect()) + .and_unseen(unseen.iter().map(DecisionForAccount::from).collect()) } /// Redeem an approval token and hand the client its code. @@ -1163,6 +1173,7 @@ mod tests { let mut stream = UnixStream::connect(&path).await.unwrap(); let mut line = serde_json::to_vec(&Message::Pending(crate::protocol::Pending { version: VERSION, + all: false, })) .unwrap(); line.push(b'\n'); @@ -1860,13 +1871,51 @@ mod tests { double.state.offer(offered(&double, "r1", "k1")); let (daemon, _scratch) = daemon_with(&double).await; until!("the poll found it", !daemon.holding().await.is_empty()); + let mut seen = report(Observed::Noted, Some("a-1")); + seen.seen_request_uris = vec!["r1".into()]; + daemon.consider(seen).await; let answer = daemon - .pending(crate::protocol::Pending { version: VERSION }) + .pending(crate::protocol::Pending { + version: VERSION, + all: false, + }) .await; let pending = answer.pending.expect("the decision"); assert_eq!(pending.len(), 1); assert_eq!(pending[0].client_origin, double.origin); + assert!(answer.unseen.is_none()); + } + + /// The command line names no context, so it is answered with what some + /// context here reported. A sign-in nobody here reported is listed only + /// when asked for all, and apart from the rest. + #[tokio::test] + async fn asking_what_is_held_leaves_out_what_no_context_reported() { + let double = double::start().await; + double.state.offer(offered(&double, "r1", "k1")); + let (daemon, _scratch) = daemon_with(&double).await; + until!("the poll found it", !daemon.holding().await.is_empty()); + + let seen_only = daemon + .pending(crate::protocol::Pending { + version: VERSION, + all: false, + }) + .await; + assert!(seen_only.pending.is_none(), "{seen_only:?}"); + assert!(seen_only.unseen.is_none(), "{seen_only:?}"); + + let all = daemon + .pending(crate::protocol::Pending { + version: VERSION, + all: true, + }) + .await; + assert!(all.pending.is_none(), "{all:?}"); + let unseen = all.unseen.expect("the unseen decision"); + assert_eq!(unseen.len(), 1); + assert_eq!(unseen[0].token.as_deref(), Some("k1")); } /// Whatever the harness invents, the name is a DNS label the server diff --git a/crates/didbot-agentd/tests/fixtures.rs b/crates/didbot-agentd/tests/fixtures.rs index cd08dcd5..04b3e7bc 100644 --- a/crates/didbot-agentd/tests/fixtures.rs +++ b/crates/didbot-agentd/tests/fixtures.rs @@ -85,6 +85,22 @@ fn narrowed_decision() -> DecisionForAccount { } } +/// One that no context reported seeing, as an ask for all lists it. +fn unreported_decision() -> DecisionForAccount { + DecisionForAccount { + token: Some("k-7c21".into()), + client_origin: "https://stranger.example".into(), + first_time: true, + requested: vec!["atproto".into()], + granted: vec!["atproto".into()], + cut: Vec::new(), + rule: None, + reason: None, + verdict: "allow".into(), + expires_at: "2026-09-22T12:35:10Z".into(), + } +} + /// Every message and every answer this crate can put on the wire. /// /// A variant with two legal ways of naming what it acts on gets one of each, @@ -134,7 +150,20 @@ fn everything() -> BTreeMap<&'static str, Value> { reason: Some("not something I asked for".into()), }), ), - ("ask-pending", Message::Pending(Pending { version: VERSION })), + ( + "ask-pending", + Message::Pending(Pending { + version: VERSION, + all: false, + }), + ), + ( + "ask-pending-all", + Message::Pending(Pending { + version: VERSION, + all: true, + }), + ), ( "ask-show", Message::Show(Show { @@ -157,6 +186,12 @@ fn everything() -> BTreeMap<&'static str, Value> { "answer-pending", Answer::quiet().and_pending(vec![narrowed_decision()]), ), + ( + "answer-pending-all", + Answer::quiet() + .and_pending(vec![narrowed_decision()]) + .and_unseen(vec![unreported_decision()]), + ), ( "answer-done", Answer::done("signed in to https://client.example as did:web:explorer-3f9a1c2d4e5f6071.pds.example") -- 2.51.2