From cd7fef7ad78f9ef51647d4f4711fd169999ca024 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 23 Sep 2026 10:15:17 -0400 Subject: [PATCH] feat(agentd): offer a context only the sign-ins it saw A held sign-in is offered to a context once its report names the request. Anyone can push a request that names an account, so one the poll delivers waits unshown until it expires. Show and approve by URL still find it. Co-Authored-By: Claude Opus 5.5 (1M context) Change-Id: I3bd6656b488dda1b1f7e8e2b9cf1014942a199cc --- crates/didbot-agentd/src/pending.rs | 60 +++++++++++++- crates/didbot-agentd/src/poll.rs | 7 +- crates/didbot-agentd/src/protocol.rs | 29 ++++--- crates/didbot-agentd/src/serve.rs | 117 ++++++++++++++++++++++----- 4 files changed, 174 insertions(+), 39 deletions(-) diff --git a/crates/didbot-agentd/src/pending.rs b/crates/didbot-agentd/src/pending.rs index c99a0d16..4dce7f56 100644 --- a/crates/didbot-agentd/src/pending.rs +++ b/crates/didbot-agentd/src/pending.rs @@ -10,6 +10,10 @@ //! by the context that owns the account. An `approve` arrives with nothing //! but a token, so records are also found by that — and that lookup is what //! decides which account the daemon approves as. The caller never says. +//! +//! Anyone can push a request that names an account, so a record the poll +//! delivers is held unseen. A `report` is answered only with the records its +//! context has reported seeing. use std::collections::HashMap; @@ -25,6 +29,8 @@ pub struct Holding { pub key: Key, /// The record itself. pub record: Record, + /// Whether that context reported seeing this request's `request_uri`. + pub seen: bool, } /// Every decision this daemon is holding, by the request it is about. @@ -44,16 +50,41 @@ impl Held { /// A record that has already been settled or has already expired is not /// held: there is no choice left in it, and putting one in front of an /// agent would be asking for a decision that cannot be made. + /// + /// A new record is held unseen. One held again for the same context keeps + /// its mark. pub fn keep(&mut self, key: Key, record: Record, now: OffsetDateTime) -> bool { if record.settled() || expired(&record, now) { self.by_request.remove(&record.request_uri); return false; } + let seen = self + .by_request + .get(&record.request_uri) + .is_some_and(|holding| holding.key == key && holding.seen); self.by_request - .insert(record.request_uri.clone(), Holding { key, record }); + .insert(record.request_uri.clone(), Holding { key, record, seen }); true } + /// Mark the records one context reported seeing, and say how many were + /// newly marked. + /// + /// Only records held for that context are marked. A context that names a + /// request for another context's account leaves it unseen for both. + pub fn saw(&mut self, key: &Key, request_uris: &[String]) -> usize { + let mut marked = 0; + for request_uri in request_uris { + if let Some(holding) = self.by_request.get_mut(request_uri) { + if &holding.key == key && !holding.seen { + holding.seen = true; + marked += 1; + } + } + } + marked + } + /// Stop holding one record, whatever became of it. pub fn forget(&mut self, request_uri: &str) -> Option { self.by_request.remove(request_uri) @@ -67,7 +98,8 @@ impl Held { before - self.by_request.len() } - /// What one context is holding, oldest request first. + /// What one context is offered, oldest request first: the records held + /// for it that it reported seeing. /// /// A context is answered with its own and nothing else: two agents on /// this machine are two accounts, and one has no business being offered @@ -76,7 +108,7 @@ impl Held { let mut found: Vec<_> = self .by_request .values() - .filter(|holding| &holding.key == key) + .filter(|holding| &holding.key == key && holding.seen) .map(|holding| holding.record.clone()) .collect(); found.sort_by(|a, b| a.request_uri.cmp(&b.request_uri)); @@ -185,6 +217,8 @@ mod tests { record("r2", "2026-09-09T12:04:00Z", Some("k2")), now, ); + assert_eq!(held.saw(&key("a1"), &["r1".into(), "r2".into()]), 1); + assert_eq!(held.saw(&key("a2"), &["r2".into()]), 1); let mine = held.for_context(&key("a1")); assert_eq!(mine.len(), 1); @@ -192,6 +226,26 @@ mod tests { assert_eq!(held.all().len(), 2); } + #[test] + fn a_record_delivered_again_keeps_its_mark() { + let mut held = Held::new(); + let now = at("2026-09-09T12:00:00Z"); + held.keep( + key("a1"), + record("r1", "2026-09-09T12:04:00Z", Some("k1")), + now, + ); + held.saw(&key("a1"), &["r1".into()]); + + // A poll that starts again from no cursor delivers it once more. + held.keep( + key("a1"), + record("r1", "2026-09-09T12:04:00Z", Some("k1")), + now, + ); + assert_eq!(held.for_context(&key("a1")).len(), 1); + } + #[test] fn a_token_names_the_account_that_will_approve() { let mut held = Held::new(); diff --git a/crates/didbot-agentd/src/poll.rs b/crates/didbot-agentd/src/poll.rs index cc032d22..b958aab7 100644 --- a/crates/didbot-agentd/src/poll.rs +++ b/crates/didbot-agentd/src/poll.rs @@ -7,9 +7,10 @@ //! last answer gave it. //! //! Nothing here pushes anything at anyone. The hook still starts every -//! exchange over the socket, and what this task has collected rides the next -//! answer. An agent therefore learns of a sign-in at its next tool call, -//! which is the latency this design accepts rather than the one it hides. +//! exchange over the socket, and what this task has collected rides the +//! answer to a report once its context has reported seeing it. An agent +//! therefore learns of a sign-in at a tool call, which is the latency this +//! design accepts rather than the one it hides. use std::sync::Arc; use std::time::Duration; diff --git a/crates/didbot-agentd/src/protocol.rs b/crates/didbot-agentd/src/protocol.rs index fec65fe2..0f68787e 100644 --- a/crates/didbot-agentd/src/protocol.rs +++ b/crates/didbot-agentd/src/protocol.rs @@ -135,13 +135,12 @@ pub struct Report { /// The harness's identifier for this particular tool call. #[serde(default, skip_serializing_if = "Option::is_none")] pub call: Option, - /// Pushed requests the adapter saw go past in tool output. + /// Pushed requests the adapter saw this context handle, by `request_uri`. /// - /// The fast path, and only that. [`Answer::pending`] is filled by what - /// the daemon has already been told; this is for the case where a client - /// printed its authorize URL in the very tool call being reported, which - /// is sooner than a poll can have finished. The daemon fetches any of - /// these it is not already holding. + /// This is what the daemon may show. Anyone can push a request that + /// names an account, so [`Answer::pending`] carries a held request only + /// once a report from its context has named it. The daemon fetches any of + /// these it is not already holding, which is sooner than a poll. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub seen_request_uris: Vec, } @@ -150,10 +149,10 @@ pub struct Report { /// /// Names a decision, and never an account. Either the one-time `token` from a /// decision the daemon already showed it, or the `url` of an authorize page -/// for one it has not -- the manual path, for a request no poll had delivered -/// and no tool output was seen to carry. Both end in the same place: the -/// daemon resolves them to a record it holds and acts as the account that -/// record names, never as an account the caller asked for. +/// for one it has not -- the manual path, which acts on a request whether or +/// not a report named it. Both end in the same place: the daemon resolves +/// them to a record it holds and acts as the account that record names, never +/// as an account the caller asked for. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Approve { /// The wire version this message was written against. @@ -191,11 +190,11 @@ pub struct Decline { /// A caller asking about one decision by name, rather than waiting to be told. /// -/// The daemon hears about a sign-in by polling, and an adapter can hand it a -/// `request_uri` it saw go past in tool output. Both can miss: a client that -/// printed its URL where no hook read it, or a poll that has not come back -/// yet. This is that same lookup made deliberate -- asked for by an agent -/// holding the URL, rather than by the daemon noticing it. +/// A report shows a context only the sign-ins it named, and that can miss: a +/// client that printed its URL where no hook read it. This is the same +/// lookup made deliberate -- asked for by an agent holding the URL, rather +/// than by the daemon noticing it -- and it finds a request whether or not a +/// report named it. /// /// It is a fetch, not a way around anything. The record still comes from /// `bot.did.getAuthorization` under an account's own credential, and the diff --git a/crates/didbot-agentd/src/serve.rs b/crates/didbot-agentd/src/serve.rs index f6bbde8b..b0bc6cab 100644 --- a/crates/didbot-agentd/src/serve.rs +++ b/crates/didbot-agentd/src/serve.rs @@ -5,8 +5,8 @@ //! own task and the store is shared. //! //! Sign-in decisions ride these exchanges rather than getting a channel of -//! their own: [`crate::poll`] collects them in the background, and whatever -//! is waiting for the reporting context goes out on the next answer. So this +//! their own: [`crate::poll`] collects them in the background, and those the +//! reporting context has reported seeing go out on the next answer. So this //! stays what it was -- one line in, one line out, no session -- and the //! agent hears about a sign-in on its next tool call. @@ -336,8 +336,8 @@ impl Daemon { /// Decide what one report is owed, and mint if that is a name. /// - /// Every answer also carries whatever sign-in decisions are waiting for - /// the context that reported, which is how an agent hears about one at + /// Every answer also carries the waiting sign-in decisions that the + /// context's reports have named, which is how an agent hears about one at /// all: nothing here opens a connection to the adapter. /// /// For a caller whose delivery is this call returning. The socket path @@ -513,11 +513,14 @@ impl Daemon { } } - /// Fetch decisions the adapter saw go past that no poll has delivered. + /// Mark the decisions a context reported seeing, fetching any that no + /// poll has delivered. /// - /// The fast path for the tool call currently being reported: a client - /// that printed its authorize URL a moment ago may not be in a poll's - /// answer yet. Anything already held costs nothing. + /// This list is what the daemon may show the context: a held record is + /// offered only once a report from its context names it. Fetching is + /// also the fast path for a client that printed its authorize URL a + /// moment ago, before a poll delivered it. Anything already held costs + /// nothing. async fn fetch_seen(&self, key: &Key, seen: &[String]) { if seen.is_empty() { return; @@ -540,19 +543,27 @@ impl Daemon { for request_uri in seen { if let Err(err) = self.fetch_one(key, &account, request_uri).await { - // Not the caller's trouble: the report itself succeeded, and - // a decision that could not be fetched arrives on the next - // poll or not at all. + // Not the caller's trouble: the report itself succeeded. A + // decision that could not be fetched is not shown, and the + // manual path can still find it by its URL. debug!(%request_uri, error = %err, "could not fetch a request the adapter saw"); } } + let marked = self.held.lock().await.saw(key, seen); + if marked > 0 { + debug!( + did = %account.did, marked, + "a context reported sign-ins it saw, so they are offered to it" + ); + } } /// Fetch one record as one account and hold it for that account's context. /// - /// The whole of the fast path, and of the manual one behind + /// Shared by a report's fetch and the manual path behind /// [`crate::protocol::Show`]: the two differ only in how they arrive at - /// an account to ask as. A record already held costs nothing. + /// an account to ask as. It holds the record unseen, and a record already + /// held costs nothing. async fn fetch_one( &self, key: &Key, @@ -1039,8 +1050,8 @@ mod tests { /// identity it already issued, and follows that account's sign-ins /// again. Without the file behind the store the account exists at the /// server, the daemon will not hand it back, and the context runs to its - /// end with none. Without the poll, an app's sign-in for it never - /// reaches the agent. + /// end with none. Without the poll, the daemon holds none of that + /// account's sign-ins until a report names one. #[tokio::test] async fn a_restart_serves_the_same_identity_and_follows_its_sign_ins() { let double = double::start().await; @@ -1088,7 +1099,9 @@ mod tests { double.state.presented() ); - let answer = again.consider(report(Observed::Acted, Some("a-1"))).await; + let mut seen = report(Observed::Acted, Some("a-1")); + seen.seen_request_uris = vec!["r1".into()]; + let answer = again.consider(seen).await; serving.abort(); assert!(answer.trouble.is_none(), "{answer:?}"); let pending = answer.pending.expect("the sign-in reached the context"); @@ -1392,20 +1405,31 @@ mod tests { record } + /// Anyone can push a request that names an agent's account, so the poll + /// delivering one says nothing about whether the agent started it. The + /// context is offered it once a report from that context names it. #[tokio::test] - async fn a_decision_the_server_has_reaches_the_context_it_belongs_to() { + async fn a_decision_the_poll_delivered_is_offered_once_its_context_names_it() { 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 unseen = daemon.consider(report(Observed::Acted, Some("a-1"))).await; + assert!(unseen.pending.is_none(), "{unseen:?}"); - let answer = daemon.consider(report(Observed::Acted, Some("a-1"))).await; + let mut seen = report(Observed::Noted, Some("a-1")); + seen.seen_request_uris = vec!["r1".into()]; + let answer = daemon.consider(seen).await; let pending = answer.pending.expect("the decision"); assert_eq!(pending.len(), 1); assert_eq!(pending[0].token.as_deref(), Some("k1")); assert!(pending[0].first_time); + // It stays offered on later calls, which name nothing. + let later = daemon.consider(report(Observed::Acted, Some("a-1"))).await; + assert_eq!(later.pending.map(|pending| pending.len()), Some(1)); + // And the poll presented the account's own credential to get it. assert!( double.state.presented().contains(&"agent-token".to_owned()), @@ -1418,6 +1442,32 @@ mod tests { assert!(other.pending.is_none()); } + /// A context that names another context's request marks nothing: the + /// request is still unseen by the context it belongs to, and it is never + /// the namer's to be offered. + #[tokio::test] + async fn a_request_one_context_saw_is_not_offered_to_another() { + 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()); + daemon.consider(report(Observed::Began, Some("a-2"))).await; + + let mut named_by_other = report(Observed::Noted, Some("a-2")); + named_by_other.seen_request_uris = vec!["r1".into()]; + let other = daemon.consider(named_by_other).await; + assert!(other.pending.is_none(), "{other:?}"); + let mine = daemon.consider(report(Observed::Acted, Some("a-1"))).await; + assert!(mine.pending.is_none(), "{mine:?}"); + + let mut named_by_mine = report(Observed::Noted, Some("a-1")); + named_by_mine.seen_request_uris = vec!["r1".into()]; + let mine = daemon.consider(named_by_mine).await; + assert_eq!(mine.pending.map(|pending| pending.len()), Some(1)); + let other = daemon.consider(report(Observed::Acted, Some("a-2"))).await; + assert!(other.pending.is_none(), "{other:?}"); + } + #[tokio::test] async fn one_whose_moment_has_passed_is_never_offered() { let double = double::start().await; @@ -1593,6 +1643,37 @@ mod tests { until!("the code arrived", !double.state.delivered().is_empty()); } + /// Handing over the URL is the agent saying it saw the request, so the + /// manual path acts on a held one that no report named. + #[tokio::test] + async fn a_held_request_no_report_named_can_still_be_approved_by_its_url() { + 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 unseen = daemon.consider(report(Observed::Acted, Some("a-1"))).await; + assert!(unseen.pending.is_none(), "{unseen:?}"); + + let shown = daemon + .show(crate::protocol::Show { + version: VERSION, + url: "r1".into(), + }) + .await; + assert_eq!(shown.pending.map(|pending| pending.len()), Some(1)); + + let answer = daemon + .approve(Approve { + version: VERSION, + token: None, + url: Some(format!("{}/oauth/authorize?request_uri=r1", double.origin)), + }) + .await; + assert!(answer.trouble.is_none(), "{answer:?}"); + assert_eq!(double.state.approved(), vec!["k1".to_owned()]); + until!("the code arrived", !double.state.delivered().is_empty()); + } + #[tokio::test] async fn a_bare_request_uri_needs_no_url_at_all() { let double = double::start().await; -- 2.51.2