From 5daa8ec3ec888d23832a98e6dcc478c3f9a1400b Mon Sep 17 00:00:00 2001 From: QuicksilverSlick Date: Mon, 9 Nov 2026 13:43:28 -0600 Subject: [PATCH 1/7] fix(acp): add a dpapi pool entry and stop blanking a folder login's token Every agent's pool is token=env;fallback=, both on the second account, so a limit on it had nowhere to hop. A pool entry can now be label=dpapi:: a long-lived token in a file encrypted for this Windows user, read at each launch and each sign-in check and handed to the child only in its environment. A file that gives no token fails its own entry and the pool moves on. A pool with no login left refuses to start the agent rather than running it on whatever its own environment holds. A folder entry used to send CLAUDE_CODE_OAUTH_TOKEN="", which the CLI reads as signed out. It now sends no token, and the variable is removed where the launch env is applied: the agent spawn and the sign-in check. Claude-Account: russelledeming@gmail.com Co-Authored-By: Claude Opus 5.5 --- crates/buzz-acp/src/acp.rs | 39 ++++- crates/buzz-acp/src/config.rs | 5 +- crates/buzz-acp/src/factory.rs | 97 ++++++++---- crates/buzz-acp/src/factory/alert.rs | 7 +- crates/buzz-acp/src/factory/dpapi.rs | 154 ++++++++++++++++++++ crates/buzz-acp/src/factory/signin_check.rs | 47 ++++-- crates/buzz-acp/src/lib.rs | 18 ++- 7 files changed, 310 insertions(+), 57 deletions(-) create mode 100644 crates/buzz-acp/src/factory/dpapi.rs diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 28978199bfd..e11a8e64d71 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -661,20 +661,26 @@ impl AcpClient { extra_env: &[(String, String)], has_generated_codex_config: bool, ) -> Result { - Self::spawn_with_env(command, args, extra_env, has_generated_codex_config, &[]).await + let none = Ok(Vec::new()); + Self::spawn_with_env(command, args, extra_env, has_generated_codex_config, &none).await } /// Spawn with authoritative launch environment overrides. Unlike persona /// defaults, these values take precedence over the inherited environment. + /// An `Err` is a pool with no login left to start on: nothing is spawned. pub(crate) async fn spawn_with_env( command: &str, args: &[String], extra_env: &[(String, String)], has_generated_codex_config: bool, - launch_env: &[(String, String)], + launch_env: &Result, String>, ) -> Result { use std::process::Stdio; + let launch_env = launch_env + .as_ref() + .map_err(|why| AcpError::Protocol(format!("no login to start on: {why}")))?; + // A launch prefix may change the CLI or environment the adapter sees, // so the version check below could inspect the wrong binary. let launch_prefixed = std::env::var_os(launch::PREFIX_ENV).is_some(); @@ -787,7 +793,7 @@ impl AcpClient { "codex" | "codex-acp" => Some(StandardAdapterKind::Codex), _ => None, }; - cmd.envs(launch_env.iter().cloned()); + crate::factory::apply_launch_env(&mut cmd, launch_env); cmd.env_remove(launch::PREFIX_ENV); let claude_thinking_summaries = standard_adapter == Some(StandardAdapterKind::Claude) && !launch_prefixed @@ -5986,6 +5992,29 @@ mod tests { ); } + /// A folder login's child has no token, even one the persona (or the PC) + /// would hand it. The child says only set or unset, so a failure cannot + /// print a value. + #[tokio::test] + async fn the_real_spawn_removes_a_folder_logins_token() { + const CMD: &str = "if defined CLAUDE_CODE_OAUTH_TOKEN (echo set) else (echo unset)"; + const SH: &str = "[ \"${CLAUDE_CODE_OAUTH_TOKEN+x}\" ] && echo set || echo unset"; + let (shell, flag, show) = if cfg!(windows) { + ("cmd", "/c", CMD) + } else { + ("sh", "-c", SH) + }; + let args = [flag.to_string(), show.to_string()]; + let persona = [("CLAUDE_CODE_OAUTH_TOKEN".into(), "from-persona".into())]; + let folder = Ok(vec![("CLAUDE_CONFIG_DIR".into(), "/logins/one".into())]); + let mut client = AcpClient::spawn_with_env(shell, &args, &persona, false, &folder) + .await + .expect("spawn"); + let seen = client.reader.next().await.expect("a line").expect("read"); + client.shutdown().await; + assert_eq!(seen.trim(), "unset"); + } + #[tokio::test] async fn idle_timeout_fires_on_silent_process() { let mut client = spawn_script("sleep 10").await; @@ -6661,10 +6690,10 @@ sleep 1"#, &[], &[], false, - &[( + &Ok(vec![( "CLAUDE_CODE_EXECUTABLE".into(), cli.to_str().expect("utf8 path").into(), - )], + )]), ) .await .expect("spawn fake claude adapter"); diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index 8e31ffa6327..37aa57abdf6 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -413,7 +413,7 @@ pub struct CliArgs { /// /// One entry may be `label=env` instead of a folder, meaning "this login /// is the agent's own environment, unchanged": no `CLAUDE_CONFIG_DIR` - /// override, so its settings folder stays as it is, and no blanked + /// override, so its settings folder stays as it is, and no removed /// `CLAUDE_CODE_OAUTH_TOKEN`, so its own long-lived `claude setup-token` /// token is the sign-in. An agent with its own token and its folder /// login as the fallback is written: @@ -421,6 +421,9 @@ pub struct CliArgs { /// ```text /// token=env;fallback=C:\Users\PC owner\.buzz-agents\factory\claude /// ``` + /// + /// An entry may also be `label=dpapi:`: a long-lived token in a + /// file encrypted for this Windows user. #[arg(long, env = crate::factory::ACCOUNT_POOL_ENV)] pub claude_account_pool: Option, diff --git a/crates/buzz-acp/src/factory.rs b/crates/buzz-acp/src/factory.rs index b1079ade61a..b3323e095c0 100644 --- a/crates/buzz-acp/src/factory.rs +++ b/crates/buzz-acp/src/factory.rs @@ -17,6 +17,7 @@ use crate::acp::{self, AcpError}; use crate::scope::SessionScope; mod alert; +mod dpapi; mod park_budget; mod paused_state; mod signin_check; @@ -520,10 +521,14 @@ fn card_quote(provider: &str, text: &str) -> String { /// /// One entry may be written `label=env` ([`ENV_ENTRY`]) instead of a folder, /// meaning "this login is the agent's own environment, unchanged": no -/// `CLAUDE_CONFIG_DIR` override and no blanked token, so the agent's own +/// `CLAUDE_CONFIG_DIR` override and no removed token, so the agent's own /// long-lived `claude setup-token` token signs it in. An agent with its own /// token and its folder login as a fallback is written /// `token=env;fallback=…\factory\claude`. +/// +/// An entry may also be `label=dpapi:`: a long-lived token in a file +/// encrypted for this Windows user (see [`dpapi`]), written +/// `token=env;primary=dpapi:…\primary.dpapi;fallback=…\factory\claude`. pub(crate) const ACCOUNT_POOL_ENV: &str = "BUZZ_ACP_CLAUDE_ACCOUNT_POOL"; /// The value that makes a pool entry the agent's own environment rather than @@ -531,10 +536,13 @@ pub(crate) const ACCOUNT_POOL_ENV: &str = "BUZZ_ACP_CLAUDE_ACCOUNT_POOL"; /// still has to be written as a path. pub(crate) const ENV_ENTRY: &str = "env"; +/// What makes a pool entry's value a token file rather than a folder. +const DPAPI_ENTRY: &str = "dpapi:"; + /// The folder a Claude login's own settings and credential live in. const CONFIG_DIR_ENV: &str = "CLAUDE_CONFIG_DIR"; -/// A PC-wide token, blanked for a folder entry so it cannot win over that +/// A PC-wide token, removed for a folder entry so it cannot win over that /// folder's own login. Named so the one test that guards what reaches an /// agent can name it too. pub(super) const OAUTH_TOKEN_ENV: &str = "CLAUDE_CODE_OAUTH_TOKEN"; @@ -542,7 +550,7 @@ pub(super) const OAUTH_TOKEN_ENV: &str = "CLAUDE_CODE_OAUTH_TOKEN"; /// One login of the pool. #[derive(Debug, Clone, PartialEq)] pub(crate) struct Account { - /// The `CLAUDE_CONFIG_DIR` folder, or `None` for a [`ENV_ENTRY`] entry: + /// The `CLAUDE_CONFIG_DIR` folder or `dpapi:` file, or `None` for a [`ENV_ENTRY`] entry: /// the agent's own environment, left exactly as it is. Never shown /// either way, because a folder holds a user path. dir: Option, @@ -660,6 +668,8 @@ pub(crate) struct Factory { /// The account pool; empty when none is configured, which is exactly the /// single-login behaviour. accounts: Vec, + /// What reads a `dpapi:` file: `None` is the real one, only tests set it. + decrypt: Option, /// Which login each agent slot was last started on. started_on: HashMap, /// Consecutive rate-limit failures, for the backoff. A real answer clears it. @@ -784,32 +794,62 @@ impl Factory { } /// Env for starting agent `slot` on the login [`pick`](Self::pick) - /// chose. Empty without a pool. + /// chose. Empty without a pool. A `dpapi:` login whose file gives no + /// token is marked out and the next tried; `Err` when none is left. + pub(crate) fn login_env( + &mut self, + slot: usize, + now: DateTime, + ) -> Result, String> { + let mut dead = String::new(); + // ponytail: `pick` still offers a login when all are out, so a dead + // file ranked first then fails the launch without trying the others, + // which are out too. The slot refill tries again. + for _ in 0..=self.accounts.len() { + let Some(i) = self.pick(now) else { + return Ok(Vec::new()); + }; + match self.account_env(i) { + Ok(env) => { + self.started_on.insert(slot, i); + return Ok(env); + } + Err((_, why)) => { + dead = format!("login {}: {why}", self.at(i)); + tracing::warn!("{dead}; trying the next login"); + self.signin_checked(i, signin_check::SignIn::SignedOut, now); + } + } + } + Err(dead) + } + + /// [`login_env`](Self::login_env) for tests with a login left. + #[cfg(test)] pub(crate) fn launch_env(&mut self, slot: usize, now: DateTime) -> Vec<(String, String)> { - let Some(i) = self.pick(now) else { - return Vec::new(); - }; - self.started_on.insert(slot, i); - self.account_env(i) + self.login_env(slot, now).expect("a login to launch on") } - /// The launch environment for login `i`: its folder, and an empty - /// `CLAUDE_CODE_OAUTH_TOKEN` so a PC-wide token cannot win over the - /// folder's own login. + /// The launch environment for login `i`: a folder entry's + /// `CLAUDE_CONFIG_DIR` (see [`apply_launch_env`] for its token), or a + /// `dpapi:` entry's decrypted token. /// /// Empty for a [`ENV_ENTRY`] entry, which is what makes it the agent's /// own environment unchanged — its own `CLAUDE_CONFIG_DIR` for settings /// and its own long-lived token for the sign-in. The sign-in check asks /// through this same function, so it can never judge a different login /// from the one the adapter would run on. - pub(super) fn account_env(&self, i: usize) -> Vec<(String, String)> { - match self.accounts.get(i).and_then(|a| a.dir.as_deref()) { - Some(dir) => vec![ - (CONFIG_DIR_ENV.into(), dir.to_string()), - (OAUTH_TOKEN_ENV.into(), String::new()), - ], - None => Vec::new(), - } + pub(super) fn account_env(&self, i: usize) -> dpapi::LoginEnv { + let Some(dir) = self.accounts.get(i).and_then(|a| a.dir.as_deref()) else { + return Ok(Vec::new()); + }; + Ok(vec![match dir.strip_prefix(DPAPI_ENTRY) { + Some(file) => ( + OAUTH_TOKEN_ENV.into(), + dpapi::token(file.trim(), self.decrypt)?, + ), + None => (CONFIG_DIR_ENV.into(), dir.to_string()), + }]) } /// Which login agent `slot` was last started on. @@ -1374,6 +1414,16 @@ impl Factory { } } +/// Put a login's launch environment on `cmd`. A folder login has any +/// inherited token removed, so a PC-wide one cannot win over its own login; +/// removed and not blanked, because the Claude CLI reads blank as signed out. +pub(crate) fn apply_launch_env(cmd: &mut tokio::process::Command, env: &[(String, String)]) { + if env.iter().any(|(key, _)| key == CONFIG_DIR_ENV) { + cmd.env_remove(OAUTH_TOKEN_ENV); + } + cmd.envs(env.iter().cloned()); +} + /// The label for a login whose folder name cannot be shown: a home folder is /// named after the OS account, and no record may carry that (spec 3.1). pub(crate) const PRIMARY_LABEL: &str = "primary"; @@ -1972,10 +2022,7 @@ mod tests { } fn env(dir: &str) -> Vec<(String, String)> { - vec![ - ("CLAUDE_CONFIG_DIR".to_string(), dir.to_string()), - ("CLAUDE_CODE_OAUTH_TOKEN".to_string(), String::new()), - ] + vec![("CLAUDE_CONFIG_DIR".to_string(), dir.to_string())] } /// The owner's own pool from 18 Oct: the agent's own long-lived token @@ -1985,7 +2032,7 @@ mod tests { /// A `label=env` entry starts the agent with its environment exactly as /// it is: no `CLAUDE_CONFIG_DIR` override to move its settings, and no /// blanked token, so the agent's own `claude setup-token` token is what - /// signs it in. A folder entry still blanks the token, so a PC-wide one + /// signs it in. A folder entry still removes the token, so a PC-wide one /// cannot win over that folder's own login. #[test] fn an_env_entry_keeps_the_agents_own_environment() { diff --git a/crates/buzz-acp/src/factory/alert.rs b/crates/buzz-acp/src/factory/alert.rs index a51fdaa2bdb..0e7c10ef218 100644 --- a/crates/buzz-acp/src/factory/alert.rs +++ b/crates/buzz-acp/src/factory/alert.rs @@ -236,9 +236,8 @@ mod tests { const TWO_FOLDERS: &str = r"primary=C:\logins\one;spare=C:\logins\two"; /// Everything a pool login is allowed to put in the agent's environment: - /// its own config folder, and a blanked token so a PC-wide one cannot - /// win over the folder's login. Nothing else, and nothing of the - /// harness's own. + /// its own config folder, or a `dpapi:` entry's token. Nothing else, and + /// nothing of the harness's own. const ALLOWED_AGENT_ENV_KEYS: [&str; 2] = ["CLAUDE_CONFIG_DIR", crate::factory::OAUTH_TOKEN_ENV]; @@ -544,7 +543,7 @@ mod tests { let mut factory = Factory::new(pool, "claude-agent-acp"); let mut envs = vec![factory.launch_env(0, now)]; for i in 0..=factory.accounts().len() { - envs.push(factory.account_env(i)); + envs.push(factory.account_env(i).unwrap()); } for env in envs { for (key, value) in env { diff --git a/crates/buzz-acp/src/factory/dpapi.rs b/crates/buzz-acp/src/factory/dpapi.rs new file mode 100644 index 00000000000..debba2c050f --- /dev/null +++ b/crates/buzz-acp/src/factory/dpapi.rs @@ -0,0 +1,154 @@ +//! A login's token from a DPAPI file (`label=dpapi:`): kept nowhere +//! here, and handed to the child only in its environment. + +use super::signin_check::SignIn; + +/// Why a file gave no token: a sign-in check's verdict and fixed words, so +/// neither a value nor anything PowerShell printed can reach a log. +pub(super) type Unread = (SignIn, &'static str); + +/// A login's launch environment, or why its file gave none. +pub(super) type LoginEnv = Result, Unread>; + +/// What reads a token file; a test injects its own. +pub(super) type Decrypt = fn(&str) -> Result; + +/// The token in `file`. Blank is a failure: a failed decrypt prints nothing, +/// and the Claude CLI reads a blank token as signed out. +pub(super) fn token(file: &str, read: Option) -> Result { + match read.unwrap_or(decrypt)(file) { + Ok(token) if token.trim().is_empty() => Err(( + SignIn::SignedOut, + "the file is missing or did not decrypt for this Windows user", + )), + read => read, + } +} + +/// The kit's `lib.mjs` `decrypt` call. The path travels in the child's +/// environment, so it is data to PowerShell and never command text. +#[cfg(windows)] +fn powershell(file: &str) -> tokio::process::Command { + use std::process::Stdio; + const DECRYPT: &str = "$ErrorActionPreference = 'Stop'; \ + $s = (Get-Content -LiteralPath $env:BUZZ_ACP_DPAPI_FILE -Raw).Trim() | ConvertTo-SecureString; \ + [Runtime.InteropServices.Marshal]::PtrToStringBSTR(\ + [Runtime.InteropServices.Marshal]::SecureStringToBSTR($s))"; + let mut cmd = tokio::process::Command::new("powershell.exe"); + cmd.args(["-NoProfile", "-NonInteractive", "-Command", DECRYPT]) + .env("BUZZ_ACP_DPAPI_FILE", file) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + crate::acp::configure_no_window(&mut cmd); + cmd +} + +/// ponytail: blocks the caller and the factory lock for one PowerShell start +/// (about a second, 15 s at worst) at each launch and each 15-minute check. +/// Move it off the lock if launches ever become frequent. +#[cfg(windows)] +fn decrypt(file: &str) -> Result { + use std::io::Read; + use std::time::{Duration, Instant}; + let no_answer = (SignIn::Unknown, "PowerShell did not answer"); + let mut child = powershell(file).into_std().spawn().map_err(|_| no_answer)?; + let deadline = Instant::now() + Duration::from_secs(15); + while !matches!(child.try_wait(), Ok(Some(_))) { + if Instant::now() >= deadline { + let _ = child.kill(); + return Err(no_answer); + } + std::thread::sleep(Duration::from_millis(20)); + } + let mut token = String::new(); + if let Some(mut out) = child.stdout.take() { + let _ = out.read_to_string(&mut token); + } + Ok(token.trim().to_string()) +} + +#[cfg(not(windows))] +fn decrypt(_file: &str) -> Result { + Err((SignIn::SignedOut, "a dpapi: login only works on Windows")) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::factory::{Factory, OAUTH_TOKEN_ENV}; + use chrono::Utc; + + const TOKEN: &str = "fake-token-for-tests"; + const DEAD: Unread = (SignIn::SignedOut, "it did not decrypt"); + const SILENT: Unread = (SignIn::Unknown, "no answer"); + + /// Stands in for DPAPI: no test here reaches PowerShell. + fn fake(file: &str) -> Result { + match file { + "good" => Ok(TOKEN.to_string()), + "blank" => Ok(" ".to_string()), + "hung" => Err(SILENT), + _ => Err(DEAD), + } + } + + fn pool(entries: &str) -> Factory { + let mut f = Factory::new(Some(entries), "claude-agent-acp"); + f.decrypt = Some(fake); + f + } + + fn pair(key: &str, value: &str) -> Vec<(String, String)> { + vec![(key.to_string(), value.to_string())] + } + + #[test] + fn the_order_is_own_token_then_the_file_then_the_folder() { + let now = Utc::now(); + let mut f = pool("token=env;primary=dpapi:good;fallback=C:/f"); + assert!(f.launch_env(0, now).is_empty()); + assert_eq!(f.label(0), "token"); + f.signin_checked(0, SignIn::SignedOut, now); + assert_eq!(f.launch_env(0, now), pair(OAUTH_TOKEN_ENV, TOKEN)); + assert_eq!(f.label(0), "primary"); + assert!(!format!("{f:?}").contains(TOKEN), "the value is not kept"); + f.signin_checked(1, SignIn::SignedOut, now); + assert_eq!(f.launch_env(0, now), pair("CLAUDE_CONFIG_DIR", "C:/f")); + assert_eq!(f.label(0), "fallback"); + } + + /// Missing, blank or silent, a file fails its own entry and the pool + /// moves on. With nothing left the launch fails plainly, naming the + /// entry: never a blank token, never the agent's own environment. + #[test] + fn a_dead_file_moves_on_and_a_pool_of_only_dead_files_refuses() { + let now = Utc::now(); + for file in ["missing", "blank", "hung"] { + let mut f = pool(&format!("primary=dpapi:{file};fallback=C:/f")); + let env = f.launch_env(0, now); + assert_eq!(env, pair("CLAUDE_CONFIG_DIR", "C:/f"), "{file}"); + assert_eq!(f.label(0), "fallback", "{file}"); + assert!(f.accounts()[0].signed_out, "{file}"); + assert_eq!(f.take_alerts().len(), 1, "the owner hears: {file}"); + } + let why = pool("primary=dpapi:missing").login_env(0, now); + assert_eq!(why, Err("login primary: it did not decrypt".to_string())); + } + + /// The check gets a dead file's verdict; a silent PowerShell is no verdict. + #[test] + fn the_check_gets_each_files_own_answer() { + let f = pool("token=env;a=dpapi:missing;b=dpapi:hung;c=dpapi:good"); + let targets = f.signin_check_targets(); + assert_eq!(targets[1].1, Err(DEAD)); + assert_eq!(targets[2].1, Err(SILENT)); + assert_eq!(targets[3].1, Ok(pair(OAUTH_TOKEN_ENV, TOKEN))); + } + + #[cfg(not(windows))] + #[test] + fn off_windows_a_dpapi_entry_fails_plainly() { + assert!(token("good", None).is_err()); + } +} diff --git a/crates/buzz-acp/src/factory/signin_check.rs b/crates/buzz-acp/src/factory/signin_check.rs index 0788e1337af..8390ba523e8 100644 --- a/crates/buzz-acp/src/factory/signin_check.rs +++ b/crates/buzz-acp/src/factory/signin_check.rs @@ -10,7 +10,7 @@ //! So the harness asks first. Every [`CHECK_SECS_DEFAULT`] seconds it runs //! `claude auth status --json` for each login of the pool, in exactly the //! environment the adapter would be launched with — a folder entry's own -//! `CLAUDE_CONFIG_DIR` and blanked token, or, for a `label=env` entry, the +//! `CLAUDE_CONFIG_DIR` and no token, or, for a `label=env` entry, the //! agent's own environment and so its own long-lived token. //! //! ## Why that check @@ -45,7 +45,7 @@ use std::time::Duration; use chrono::{DateTime, Utc}; -use super::{Class, Factory, Failure, Health}; +use super::{dpapi, Class, Factory, Failure, Health}; /// How often each login's sign-in is checked, in seconds; `0` turns the /// check off. @@ -91,13 +91,14 @@ impl Factory { /// environment, unchanged, when there is no pool. /// /// Empty for an adapter that is not Claude: there is no `claude auth - /// status` to ask about a Codex sign-in. - pub(crate) fn signin_check_targets(&self) -> Vec<(usize, Vec<(String, String)>)> { + /// status` to ask about a Codex sign-in. A `dpapi:` login whose file + /// gives no token comes back as `Err`, with nothing for the CLI to judge. + pub(crate) fn signin_check_targets(&self) -> Vec<(usize, dpapi::LoginEnv)> { if self.provider != "claude" { return Vec::new(); } if self.accounts.is_empty() { - return vec![(0, Vec::new())]; + return vec![(0, Ok(Vec::new()))]; } (0..self.accounts.len()) .map(|i| (i, self.account_env(i))) @@ -238,15 +239,11 @@ impl Factory { /// Ask the Claude CLI whether `env` is signed in, spending no usage. /// /// `env` is a login's launch environment: a folder entry's -/// `CLAUDE_CONFIG_DIR` and blanked token, or nothing at all for a +/// `CLAUDE_CONFIG_DIR`, a `dpapi:` entry's token, or nothing at all for a /// `label=env` entry, which leaves the agent's own token in place. pub(crate) async fn probe(env: &[(String, String)]) -> SignIn { let cli = crate::acp::claude_cli(env); - let mut cmd = tokio::process::Command::new(&cli); - cmd.arg("auth") - .arg("status") - .arg("--json") - .envs(env.iter().cloned()); + let mut cmd = status_command(&cli, env); // Any exit status: the real CLI prints `{"loggedIn": false, …}` and // exits 1 when the login is signed out, which is the answer this whole // check exists to hear. A spawn that fails, a timeout and output that @@ -260,6 +257,14 @@ pub(crate) async fn probe(env: &[(String, String)]) -> SignIn { } } +/// `claude auth status --json`, in the login's own environment. +fn status_command(cli: &std::ffi::OsStr, env: &[(String, String)]) -> tokio::process::Command { + let mut cmd = tokio::process::Command::new(cli); + cmd.arg("auth").arg("status").arg("--json"); + super::apply_launch_env(&mut cmd, env); + cmd +} + /// Read `claude auth status --json`. /// /// Only `loggedIn` is read, and only when it really is a boolean. The rest @@ -282,7 +287,7 @@ fn read_status(out: &str) -> SignIn { mod tests { use super::*; use crate::acp::AcpError; - use crate::factory::{classify, Plan, Reporting, PROBE_EVERY}; + use crate::factory::{classify, Plan, Reporting, OAUTH_TOKEN_ENV, PROBE_EVERY}; use crate::scope::SessionScope; use chrono::TimeZone; @@ -321,6 +326,16 @@ mod tests { f } + /// A folder login's check asks with the token removed, never blanked. + #[test] + fn a_folder_login_is_checked_with_the_token_removed() { + let env = pool().account_env(1).unwrap(); + assert!(!env.iter().any(|(key, _)| key == OAUTH_TOKEN_ENV)); + let cmd = status_command("claude".as_ref(), &env); + let mut vars = cmd.as_std().get_envs(); + assert!(vars.any(|var| var == (OAUTH_TOKEN_ENV.as_ref(), None))); + } + /// Only `loggedIn: false` is a verdict. Everything else — a missing /// field, a string where the boolean should be, truncated or empty /// output, an error page — says nothing, because a check that cannot @@ -366,7 +381,7 @@ mod tests { assert!(!f.accounts()[1].signed_out, "the other login is untouched"); // Another login can take the work, so the agent keeps moving. assert_eq!(f.health, Health::Degraded); - assert_eq!(f.launch_env(0, oct18()), f.account_env(1)); + assert_eq!(f.launch_env(0, oct18()), f.account_env(1).unwrap()); let record = f.take_report(oct18()).expect("a STATE record"); assert!(record.contains("state=DEGRADED class=sign_in"), "{record}"); @@ -459,7 +474,7 @@ mod tests { "no Critical card for a login nothing was running on" ); // The work stays exactly where it was. - assert_eq!(f.launch_env(0, oct18()), f.account_env(0)); + assert_eq!(f.launch_env(0, oct18()), f.account_env(0).unwrap()); assert!(!f.wants_restart(0, oct18())); // One truthful line, to the owner, once. assert_eq!( @@ -623,7 +638,7 @@ mod tests { assert_eq!(f.health, Health::Operational); assert_eq!(f.presence(), "online"); assert!(f.wants_restart(0, fixed), "back to the first login"); - assert_eq!(f.launch_env(0, fixed), f.account_env(0)); + assert_eq!(f.launch_env(0, fixed), f.account_env(0).unwrap()); assert_eq!( f.take_alerts(), ["Buzz: all clear — Reset Worker is working normally again."] @@ -642,7 +657,7 @@ mod tests { f.reporting.agent = "Reset Worker".to_string(); assert_eq!( f.signin_check_targets(), - vec![(0, Vec::new())], + vec![(0, Ok(Vec::new()))], "the agent's own environment, unchanged" ); assert!(f.signin_checked(0, SignIn::SignedOut, oct18())); diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index a0b82eac507..7200d931fb2 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -3619,7 +3619,7 @@ async fn run_harness( let cmd = config.agent_command.clone(); let args = config.agent_args.clone(); let env = config.persona_env_vars.clone(); - let launch_env = config.factory.lock().launch_env(idx, chrono::Utc::now()); + let launch_env = config.factory.lock().login_env(idx, chrono::Utc::now()); let has_codex = config.has_generated_codex_config; let observer = observer.clone(); let guard = RespawnGuard::new(idx, respawn_tx.clone()); @@ -6239,7 +6239,13 @@ fn spawn_signin_check(config: &Config, rest: &relay::RestClient) { tokio::spawn(async move { let mut changed = false; for (login, env) in targets { - let found = factory::signin_probe(&env).await; + let found = match env { + Ok(env) => factory::signin_probe(&env).await, + Err((found, why)) => { + tracing::warn!(login, "sign-in check: its token file: {why}"); + found + } + }; tracing::debug!(login, "sign-in check: {found:?}"); changed |= factory .lock() @@ -6960,7 +6966,7 @@ fn recover_panicked_agent( let cmd = config.agent_command.clone(); let args = config.agent_args.clone(); let env = config.persona_env_vars.clone(); - let launch_env = config.factory.lock().launch_env(i, chrono::Utc::now()); + let launch_env = config.factory.lock().login_env(i, chrono::Utc::now()); let has_codex = config.has_generated_codex_config; let guard = RespawnGuard::new(i, respawn_tx.clone()); respawn_tasks.spawn(async move { @@ -7251,7 +7257,7 @@ fn spawn_respawn_task( let args = config.agent_args.clone(); let env = config.persona_env_vars.clone(); let has_codex = config.has_generated_codex_config; - let launch_env = config.factory.lock().launch_env(index, chrono::Utc::now()); + let launch_env = config.factory.lock().login_env(index, chrono::Utc::now()); let guard = RespawnGuard::new(index, respawn_tx.clone()); respawn_tasks.spawn(async move { // Shutdown old agent (reap child, prevent zombie). @@ -7310,7 +7316,7 @@ async fn initialize_agent_pool( // Attempt each spawn under a 60-second timeout; a partial pool is valid. let mut agent_slots: Vec> = Vec::with_capacity(startup.agents as usize); for i in 0..startup.agents as usize { - let launch_env = startup.factory.lock().launch_env(i, chrono::Utc::now()); + let launch_env = startup.factory.lock().login_env(i, chrono::Utc::now()); let spawn_result = AcpClient::spawn_with_env( &startup.command, &startup.args, @@ -7419,7 +7425,7 @@ async fn spawn_and_init( command: &str, args: &[String], extra_env: &[(String, String)], - launch_env: &[(String, String)], + launch_env: &std::result::Result, String>, has_generated_codex_config: bool, agent_index: usize, observer: Option, From ac98a07a75b7a5c3a097c2e8e9f646f47ea3373b Mon Sep 17 00:00:00 2001 From: QuicksilverSlick Date: Mon, 9 Nov 2026 14:31:19 -0600 Subject: [PATCH 2/7] fix(acp): keep the pool string from the agent and pin the launch gates The pool string names the primary's token file, and the agent's tool shell inherited it. It is now removed from the child, beside the Nostr key. Two gates had no test. Making the token removal unconditional, or moving it after the login's own env, stripped every token=env and dpapi login and passed the whole suite. So did loosening the refusal to start with no login left. A real child now shows each kind of login's token, and a pool with no login left starts nothing. Claude-Account: russelledeming@gmail.com Co-Authored-By: Claude Opus 5.5 --- crates/buzz-acp/src/acp.rs | 71 ++++++++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 14 deletions(-) diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index e11a8e64d71..f5763b461a6 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -762,6 +762,8 @@ impl AcpClient { // over the harness-owned agent identity in native shells. cmd.env_remove("GIT_CONFIG_PARAMETERS"); cmd.env_remove("NOSTR_PRIVATE_KEY"); + // It says where the primary's token file is: not for the agent's shell. + cmd.env_remove(crate::factory::ACCOUNT_POOL_ENV); if extra_env.iter().any(|(name, _)| name == "GIT_CONFIG_COUNT") { // Native shells inherit these overrides, while buzz-agent clears // them for MCP. Let both paths use the harness's agent identity. @@ -5992,27 +5994,68 @@ mod tests { ); } - /// A folder login's child has no token, even one the persona (or the PC) - /// would hand it. The child says only set or unset, so a failure cannot - /// print a value. - #[tokio::test] - async fn the_real_spawn_removes_a_folder_logins_token() { - const CMD: &str = "if defined CLAUDE_CODE_OAUTH_TOKEN (echo set) else (echo unset)"; - const SH: &str = "[ \"${CLAUDE_CODE_OAUTH_TOKEN+x}\" ] && echo set || echo unset"; + /// The launch `login` gives a real child, as the child sees `var`. It says + /// only set or unset, so a failure cannot print a value. + async fn child_sees( + var: &str, + persona: &[(String, String)], + login: Result, String>, + ) -> String { let (shell, flag, show) = if cfg!(windows) { - ("cmd", "/c", CMD) + let cmd = format!("if defined {var} (echo set) else (echo unset)"); + ("cmd", "/c", cmd) } else { - ("sh", "-c", SH) + let sh = format!("[ \"${{{var}+x}}\" ] && echo set || echo unset"); + ("sh", "-c", sh) }; - let args = [flag.to_string(), show.to_string()]; - let persona = [("CLAUDE_CODE_OAUTH_TOKEN".into(), "from-persona".into())]; - let folder = Ok(vec![("CLAUDE_CONFIG_DIR".into(), "/logins/one".into())]); - let mut client = AcpClient::spawn_with_env(shell, &args, &persona, false, &folder) + let args = [flag.to_string(), show]; + let mut client = AcpClient::spawn_with_env(shell, &args, persona, false, &login) .await .expect("spawn"); let seen = client.reader.next().await.expect("a line").expect("read"); client.shutdown().await; - assert_eq!(seen.trim(), "unset"); + seen.trim().to_string() + } + + /// A folder login's child has no token, even one the persona (or the PC) + /// would hand it. The agent's own token and a `dpapi:` login keep theirs. + #[tokio::test] + async fn the_real_spawn_gives_each_kind_of_login_its_token() { + const TOKEN: &str = "CLAUDE_CODE_OAUTH_TOKEN"; + let persona = [(TOKEN.to_string(), "from-persona".to_string())]; + let own = Ok(Vec::new()); + assert_eq!(child_sees(TOKEN, &persona, own).await, "set", "env entry"); + let file = Ok(vec![(TOKEN.to_string(), "from-file".to_string())]); + assert_eq!(child_sees(TOKEN, &[], file).await, "set", "dpapi entry"); + let folder = Ok(vec![("CLAUDE_CONFIG_DIR".into(), "/logins/one".into())]); + assert_eq!(child_sees(TOKEN, &persona, folder).await, "unset", "folder"); + } + + /// The pool string says where the primary's token file is, so the agent's + /// tool shell is not handed it. + #[tokio::test] + async fn the_real_spawn_keeps_the_pool_string_from_the_agent() { + let var = crate::factory::ACCOUNT_POOL_ENV; + let persona = [(var.to_string(), "primary=dpapi:C:/x".to_string())]; + assert_eq!(child_sees(var, &persona, Ok(Vec::new())).await, "unset"); + } + + /// With no login left nothing starts, not even on the agent's own + /// environment, which is what a loose refusal would fall through to. + #[tokio::test] + async fn no_login_left_starts_nothing() { + let (shell, flag, show) = if cfg!(windows) { + ("cmd", "/c", "echo started") + } else { + ("sh", "-c", "echo started") + }; + let args = [flag.to_string(), show.to_string()]; + let dead = Err("login primary: dead".to_string()); + let refused = AcpClient::spawn_with_env(shell, &args, &[], false, &dead).await; + assert!( + matches!(&refused, Err(AcpError::Protocol(why)) if why.contains("login primary: dead")), + "nothing may start with no login left" + ); } #[tokio::test] From 001fc8f40b6d6ef9a2e7af43ed5ce28e12f03ed4 Mon Sep 17 00:00:00 2001 From: QuicksilverSlick Date: Mon, 9 Nov 2026 14:33:30 -0600 Subject: [PATCH 3/7] fix(acp): a dpapi login the server refused waits for a new file A real 401 on a dpapi login marked it signed out, and the next sign-in check cleared the mark: claude auth status answers loggedIn:true for any token without asking the server. The work went back onto the revoked token every 15 minutes, with a lost-sign-in text and an all-clear each time, and every agent shares that one file. A real 401 now remembers the file's modified time. A check's yes clears the mark only once the file has changed; a real answer still clears it. A mark set because the file could not be read still clears on a good read. token=env keeps today's behaviour. Claude-Account: russelledeming@gmail.com Co-Authored-By: Claude Opus 5.5 --- crates/buzz-acp/src/factory.rs | 25 ++++++++ crates/buzz-acp/src/factory/dpapi.rs | 69 ++++++++++++++++++++- crates/buzz-acp/src/factory/signin_check.rs | 10 ++- 3 files changed, 101 insertions(+), 3 deletions(-) diff --git a/crates/buzz-acp/src/factory.rs b/crates/buzz-acp/src/factory.rs index b3323e095c0..2bf5a979eaf 100644 --- a/crates/buzz-acp/src/factory.rs +++ b/crates/buzz-acp/src/factory.rs @@ -564,6 +564,18 @@ pub(crate) struct Account { /// timed limit: no clock lifts a lost sign-in, so only a passing /// sign-in check or a real answer on this login clears it. pub signed_out: bool, + /// A `dpapi:` login the server refused: its file's modified time then. + /// `claude auth status` never asks the server, so a check cannot lift + /// that; a new file or a real answer can. + refused: Option, +} + +impl Account { + /// When this login's token file last changed; `None` for any other login. + fn file_stamp(&self) -> Option { + let file = self.dir.as_deref()?.strip_prefix(DPAPI_ENTRY)?.trim(); + std::fs::metadata(file).and_then(|m| m.modified()).ok() + } } /// A limit one login of the pool hit. @@ -742,6 +754,7 @@ impl Factory { label, limited: None, signed_out: false, + refused: None, } }) .collect(); @@ -901,6 +914,7 @@ impl Factory { failure: &Failure, now: DateTime, ) -> Plan { + self.note_refusal(self.on(slot), failure); let (wait, free) = self.begin_limit(self.on(slot), failure, now); if let Some(next) = free { let label = self.accounts[next].label.clone(); @@ -940,9 +954,19 @@ impl Factory { /// while the pool is idle left the dot green and #factory silent until /// some real request happened along and failed (spec 2.2, 4.2). pub(crate) fn record_limit(&mut self, slot: usize, failure: &Failure, now: DateTime) { + self.note_refusal(self.on(slot), failure); self.record_limit_on(self.on(slot), failure, now); } + /// A real turn lost its sign-in on login `on`: see [`Account::refused`]. + fn note_refusal(&mut self, on: usize, failure: &Failure) { + if failure.class == Class::SignIn { + if let Some(account) = self.accounts.get_mut(on) { + account.refused = account.file_stamp(); + } + } + } + /// [`record_limit`](Self::record_limit) for a login named directly /// rather than through the slot running on it: the sign-in check's path, /// which knows which login it asked about and not which slot is on it. @@ -1094,6 +1118,7 @@ impl Factory { { account.limited = None; account.signed_out = false; + account.refused = None; } let settled = if self.preferred(now).is_some_and(|p| p > 0) { Health::Degraded diff --git a/crates/buzz-acp/src/factory/dpapi.rs b/crates/buzz-acp/src/factory/dpapi.rs index debba2c050f..dedbefec03a 100644 --- a/crates/buzz-acp/src/factory/dpapi.rs +++ b/crates/buzz-acp/src/factory/dpapi.rs @@ -76,7 +76,9 @@ fn decrypt(_file: &str) -> Result { #[cfg(test)] mod tests { use super::*; - use crate::factory::{Factory, OAUTH_TOKEN_ENV}; + use crate::acp::AcpError; + use crate::factory::{classify, Factory, OAUTH_TOKEN_ENV}; + use crate::scope::SessionScope; use chrono::Utc; const TOKEN: &str = "fake-token-for-tests"; @@ -86,7 +88,7 @@ mod tests { /// Stands in for DPAPI: no test here reaches PowerShell. fn fake(file: &str) -> Result { match file { - "good" => Ok(TOKEN.to_string()), + good if good.ends_with("good") => Ok(TOKEN.to_string()), "blank" => Ok(" ".to_string()), "hung" => Err(SILENT), _ => Err(DEAD), @@ -146,6 +148,69 @@ mod tests { assert_eq!(targets[3].1, Ok(pair(OAUTH_TOKEN_ENV, TOKEN))); } + fn refuse(f: &mut Factory, now: chrono::DateTime) { + let channel_id = uuid::Uuid::from_u128(7); + let scope = SessionScope::Conversation { channel_id }; + let error = AcpError::agent_error(-32603, "API Error: 401 expired"); + f.plan(0, &scope, &classify(&error, now), now); + } + + /// A server's 401 is about the token, and `claude auth status` never asks + /// the server, so its yes cannot bring the login back. A new file can. + #[test] + fn a_refused_token_stays_out_until_its_file_changes() { + let now = Utc::now(); + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("good"); + std::fs::write(&file, "cipher").unwrap(); + let mut f = pool(&format!("primary=dpapi:{};fallback=C:/f", file.display())); + f.launch_env(0, now); + refuse(&mut f, now); + f.launch_env(0, now); + f.take_alerts(); + assert!(!f.signin_checked(0, SignIn::Valid, now), "nothing changed"); + assert!(f.accounts()[0].signed_out); + assert!(!f.wants_restart(0, now), "the work stays off it"); + assert!(f.take_alerts().is_empty(), "no all-clear to take back"); + + let replaced = std::time::SystemTime::now() + std::time::Duration::from_secs(3600); + let handle = std::fs::File::options().write(true).open(&file).unwrap(); + handle.set_modified(replaced).unwrap(); + assert!(f.signin_checked(0, SignIn::Valid, now), "the new file"); + assert!(!f.accounts()[0].signed_out); + assert!(f.wants_restart(0, now), "back to the first login"); + } + + /// Only a refusal waits for a new file: a real answer ends it, and a file + /// that could not be read says nothing about its token. + #[test] + fn a_real_answer_ends_a_refusal_and_a_good_read_clears_an_unread_file() { + let now = Utc::now(); + let dir = tempfile::tempdir().unwrap(); + for name in ["good", "missing"] { + std::fs::write(dir.path().join(name), "cipher").unwrap(); + } + let good = dir.path().join("good"); + let mut f = pool(&format!("primary=dpapi:{}", good.display())); + f.launch_env(0, now); + refuse(&mut f, now); + f.launch_env(0, now); + f.on_success(0, now); + f.signin_checked(0, SignIn::SignedOut, now); + f.signin_checked(0, SignIn::Valid, now); + assert!(!f.accounts()[0].signed_out, "the refusal is over"); + + let missing = dir.path().join("missing"); + let mut f = pool(&format!( + "primary=dpapi:{};fallback=C:/f", + missing.display() + )); + f.launch_env(0, now); + assert!(f.accounts()[0].signed_out); + f.signin_checked(0, SignIn::Valid, now); + assert!(!f.accounts()[0].signed_out, "a good read clears it"); + } + #[cfg(not(windows))] #[test] fn off_windows_a_dpapi_entry_fails_plainly() { diff --git a/crates/buzz-acp/src/factory/signin_check.rs b/crates/buzz-acp/src/factory/signin_check.rs index 8390ba523e8..392b1ac129c 100644 --- a/crates/buzz-acp/src/factory/signin_check.rs +++ b/crates/buzz-acp/src/factory/signin_check.rs @@ -218,7 +218,15 @@ impl Factory { /// already due. fn signin_restored(&mut self, index: usize, now: DateTime) -> bool { let cleared = match self.accounts.get_mut(index) { - Some(account) => std::mem::take(&mut account.signed_out), + Some(account) + if account.refused.is_some() && account.refused == account.file_stamp() => + { + return false; + } + Some(account) => { + account.refused = None; + std::mem::take(&mut account.signed_out) + } None => false, }; let over = matches!(self.health, Health::Down | Health::Degraded) From c9e8c5e5d0b99026fb73c3f019314d695a021d36 Mon Sep 17 00:00:00 2001 From: QuicksilverSlick Date: Mon, 9 Nov 2026 14:39:04 -0600 Subject: [PATCH 4/7] fix(acp): a token file failure says so, and a silent one is no sign-in A launch that could not read a token file marked the login signed out whatever the reason, and the owner was told "Claude said: not signed in" when Claude was never asked. A PowerShell timeout moved work off a good account as a lost sign-in. The card's remedy was also wrong for a dead file: signing in does not replace it. A silent or unstartable PowerShell now sets the login aside as a timed limit, tried again in 30 minutes, and tells the owner once. A dead file still takes the lost-sign-in path, in its own words: which login's file, what is wrong and that set-account.ps1 is the fix, on the text, the STATE record (class=token_file) and the card. A login already inside a limit keeps it. The sign-in check's handling of a dead file moves to one small function with a test: a file that gave no token is never judged by the CLI. Claude-Account: russelledeming@gmail.com Co-Authored-By: Claude Opus 5.5 --- crates/buzz-acp/src/factory.rs | 39 +++-- crates/buzz-acp/src/factory/dpapi.rs | 156 +++++++++++++++++++- crates/buzz-acp/src/factory/signin_check.rs | 56 +++++-- crates/buzz-acp/src/lib.rs | 11 +- 4 files changed, 229 insertions(+), 33 deletions(-) diff --git a/crates/buzz-acp/src/factory.rs b/crates/buzz-acp/src/factory.rs index 2bf5a979eaf..2767bf688a0 100644 --- a/crates/buzz-acp/src/factory.rs +++ b/crates/buzz-acp/src/factory.rs @@ -26,7 +26,7 @@ use park_budget::{Budget, Parks}; pub(crate) use alert::{send as send_alert, ALERT_CMD_ENV, ALERT_SMS_TO_ENV}; pub(crate) use signin_check::{ - probe as signin_probe, CHECK_SECS_DEFAULT as SIGNIN_CHECK_SECS_DEFAULT, + ask as signin_ask, probe as signin_probe, CHECK_SECS_DEFAULT as SIGNIN_CHECK_SECS_DEFAULT, CHECK_SECS_ENV as SIGNIN_CHECK_SECS_ENV, }; @@ -827,10 +827,10 @@ impl Factory { self.started_on.insert(slot, i); return Ok(env); } - Err((_, why)) => { - dead = format!("login {}: {why}", self.at(i)); + Err(unread) => { + dead = format!("login {}: {}", self.at(i), unread.1); tracing::warn!("{dead}; trying the next login"); - self.signin_checked(i, signin_check::SignIn::SignedOut, now); + self.token_file_failed(i, unread, now); } } } @@ -1194,7 +1194,7 @@ impl Factory { record.push_str(&format!(" incident={incident}")); } if let Some(f) = failure { - record.push_str(&format!("\n{} said: \"{}\"", self.provider_name(), f.text)); + record.push_str(&format!("\n{} said: \"{}\"", self.speaker(f), f.text)); } record.push_str(&format!("\nPlain words: {}", self.plain_words(now))); if !stopped { @@ -1219,6 +1219,27 @@ impl Factory { } } + /// Whose words `failure` carries: a token file's are Buzz's own. + fn speaker(&self, failure: &Failure) -> String { + if failure.slug == dpapi::UNREAD_SLUG { + "Buzz".to_string() + } else { + self.provider_name() + } + } + + /// What brings back the login the current incident is about. + fn sign_in_remedy(&self) -> &'static str { + let on_file = self.accounts.iter().any(|a| { + a.label == self.account && a.dir.as_deref().is_some_and(|d| d.starts_with(DPAPI_ENTRY)) + }); + if on_file { + "its token file is replaced with set-account.ps1" + } else { + "it is signed in again" + } + } + fn who(&self) -> &str { match self.reporting.agent.as_str() { "" => "This agent", @@ -1239,8 +1260,8 @@ impl Factory { recovered." ), (Health::Down, ..) => format!( - "{who} lost its sign-in. Its work is paused until it is signed in again; it \ - keeps checking. {saved}" + "{who} lost its sign-in. Its work is paused until {}; it keeps checking. {saved}", + self.sign_in_remedy() ), (Health::Blocked, Some(Class::SpendCap), _) => format!( "{who}'s work is paused until the spending cap is raised; it checks every 30 \ @@ -1303,13 +1324,13 @@ impl Factory { ( "sign-in", format!("{who} lost its {provider} sign-in"), - "it is signed in again", + self.sign_in_remedy(), ) }; // The time is in the text so a later incident is a new card, not // the same Drop the Bench already holds. let since = chicago_label(self.since.unwrap_or_else(Utc::now)); - let quote = card_quote(&provider, &failure.text); + let quote = card_quote(&self.speaker(failure), &failure.text); let where_now = match moved_to { Some(next) => format!("It is now running on login {next}."), None => "Its requests are parked, not dropped.".to_string(), diff --git a/crates/buzz-acp/src/factory/dpapi.rs b/crates/buzz-acp/src/factory/dpapi.rs index dedbefec03a..c78aea3c621 100644 --- a/crates/buzz-acp/src/factory/dpapi.rs +++ b/crates/buzz-acp/src/factory/dpapi.rs @@ -1,7 +1,13 @@ //! A login's token from a DPAPI file (`label=dpapi:`): kept nowhere //! here, and handed to the child only in its environment. +use chrono::{DateTime, Utc}; + use super::signin_check::SignIn; +use super::{Class, Factory, Failure}; + +/// The STATE record's class for a file that gave no token. +pub(super) const UNREAD_SLUG: &str = "token_file"; /// Why a file gave no token: a sign-in check's verdict and fixed words, so /// neither a value nor anything PowerShell printed can reach a log. @@ -19,7 +25,8 @@ pub(super) fn token(file: &str, read: Option) -> Result match read.unwrap_or(decrypt)(file) { Ok(token) if token.trim().is_empty() => Err(( SignIn::SignedOut, - "the file is missing or did not decrypt for this Windows user", + "the file is missing or did not decrypt for this Windows user; \ + run set-account.ps1 to replace it", )), read => read, } @@ -51,8 +58,19 @@ fn powershell(file: &str) -> tokio::process::Command { fn decrypt(file: &str) -> Result { use std::io::Read; use std::time::{Duration, Instant}; - let no_answer = (SignIn::Unknown, "PowerShell did not answer"); - let mut child = powershell(file).into_std().spawn().map_err(|_| no_answer)?; + let no_answer = ( + SignIn::Unknown, + "PowerShell did not answer in 15 s; it tries again in 30 minutes, and set-account.ps1 \ + replaces the file if it keeps failing", + ); + let wont_start = ( + SignIn::Unknown, + "PowerShell would not start; it tries again in 30 minutes", + ); + let mut child = powershell(file) + .into_std() + .spawn() + .map_err(|_| wont_start)?; let deadline = Instant::now() + Duration::from_secs(15); while !matches!(child.try_wait(), Ok(Some(_))) { if Instant::now() >= deadline { @@ -73,13 +91,54 @@ fn decrypt(_file: &str) -> Result { Err((SignIn::SignedOut, "a dpapi: login only works on Windows")) } +impl Factory { + /// Login `i`'s file gave no token. Claude was never asked, so the owner + /// is told about the file, in words of its own. + /// + /// A file that is dead takes the path a lost sign-in takes. PowerShell + /// not answering says nothing about the token, so that login is set + /// aside as a limit with no reset time: tried again in 30 minutes + /// ([`PROBE_EVERY`](super::PROBE_EVERY)), and no sign-in incident. A + /// login already inside a limit keeps it, and what the record says of it. + pub(super) fn token_file_failed( + &mut self, + i: usize, + (found, why): Unread, + now: DateTime, + ) -> bool { + let text = format!( + "could not read the token file for login {}: {why}", + self.at(i) + ); + let mut failure = Failure { + class: Class::SignIn, + slug: UNREAD_SLUG, + text, + resets_at: None, + }; + if found != SignIn::Unknown { + return self.signin_lost(i, &failure, now); + } + if self.accounts[i] + .limited + .as_ref() + .is_some_and(|l| l.until > now) + { + return false; + } + failure.class = Class::UsageLimit; + self.alert_aside(&format!("{} {}.", self.who(), failure.text)); + self.record_limit_on(i, &failure, now); + true + } +} + #[cfg(test)] mod tests { use super::*; use crate::acp::AcpError; - use crate::factory::{classify, Factory, OAUTH_TOKEN_ENV}; + use crate::factory::{classify, signin_ask as ask, OAUTH_TOKEN_ENV}; use crate::scope::SessionScope; - use chrono::Utc; const TOKEN: &str = "fake-token-for-tests"; const DEAD: Unread = (SignIn::SignedOut, "it did not decrypt"); @@ -120,13 +179,13 @@ mod tests { assert_eq!(f.label(0), "fallback"); } - /// Missing, blank or silent, a file fails its own entry and the pool + /// Missing or blank, a file fails its own entry and the pool /// moves on. With nothing left the launch fails plainly, naming the /// entry: never a blank token, never the agent's own environment. #[test] fn a_dead_file_moves_on_and_a_pool_of_only_dead_files_refuses() { let now = Utc::now(); - for file in ["missing", "blank", "hung"] { + for file in ["missing", "blank"] { let mut f = pool(&format!("primary=dpapi:{file};fallback=C:/f")); let env = f.launch_env(0, now); assert_eq!(env, pair("CLAUDE_CONFIG_DIR", "C:/f"), "{file}"); @@ -211,6 +270,89 @@ mod tests { assert!(!f.accounts()[0].signed_out, "a good read clears it"); } + /// PowerShell not answering says nothing about the sign-in, so the login + /// is set aside for a while and comes back by itself. + #[test] + fn a_silent_file_is_set_aside_and_no_one_says_claude_refused() { + let now = Utc::now(); + let mut f = pool("primary=dpapi:hung;fallback=C:/f"); + assert_eq!(f.launch_env(0, now), pair("CLAUDE_CONFIG_DIR", "C:/f")); + assert!(!f.accounts()[0].signed_out, "Claude was never asked"); + let texts = f.take_alerts(); + assert_eq!(texts.len(), 1, "{texts:?}"); + assert!(texts[0].contains("login primary"), "{texts:?}"); + assert!(texts[0].contains("no answer"), "{texts:?}"); + let record = f.take_report(now).expect("a STATE record"); + assert!(record.contains("class=token_file"), "{record}"); + assert!(!record.contains("Claude said"), "{record}"); + assert!(!f.wants_restart(0, now), "off it for now"); + let until = f.accounts()[0].limited.as_ref().map(|l| l.until); + assert!(!f.token_file_failed(0, SILENT, now), "already set aside"); + assert_eq!(f.accounts()[0].limited.as_ref().map(|l| l.until), until); + let later = now + chrono::Duration::minutes(31); + assert!(f.wants_restart(0, later), "and it tries it again"); + } + + /// A dead file's words name the file and the fix, on the text, the record + /// and the card. Claude never said anything. + #[test] + fn a_dead_files_words_name_the_file_and_the_fix() { + let now = Utc::now(); + let bench = tempfile::tempdir().unwrap(); + let mut f = pool("primary=dpapi:blank;fallback=C:/f"); + f.reporting.bench = Some(bench.path().to_path_buf()); + f.reporting.short = "abcd1234".to_string(); + f.launch_env(0, now); + assert!(f.accounts()[0].signed_out); + let texts = f.take_alerts(); + assert_eq!(texts.len(), 1, "{texts:?}"); + let record = f.take_report(now).expect("a STATE record"); + let card = bench.path().join("inbox/factory/abcd1234-sign-in.json"); + let card = std::fs::read_to_string(card).expect("a card"); + for words in [&texts[0], &record, &card] { + assert!(words.contains("login primary"), "{words}"); + assert!(words.contains("set-account.ps1"), "{words}"); + assert!(!words.contains("Claude said"), "{words}"); + } + assert!(record.contains("class=token_file"), "{record}"); + assert!(card.contains("replaced with set-account.ps1"), "{card}"); + } + + /// A file that gave no token is its own answer: asking the CLI anyway + /// would judge the agent's own environment and read as signed in. + #[tokio::test] + async fn a_dead_file_is_not_asked_about() { + let dead = ask(Err(DEAD), |_| async { panic!("the CLI was asked") }).await; + assert_eq!(dead, Err(DEAD)); + let good = ask(Ok(pair(OAUTH_TOKEN_ENV, TOKEN)), |env| async move { + assert_eq!(env, pair(OAUTH_TOKEN_ENV, TOKEN)); + SignIn::Valid + }) + .await; + assert_eq!(good, Ok(SignIn::Valid)); + } + + /// The check marks a dead file once, in the file's own words, and a + /// silent PowerShell concludes nothing. + #[test] + fn the_check_marks_a_dead_file_and_leaves_a_silent_one() { + let now = Utc::now(); + let mut f = pool("token=env;primary=dpapi:missing"); + f.launch_env(0, now); + assert!(!f.signin_found(1, Err(SILENT), now), "concludes nothing"); + assert!(!f.accounts()[1].signed_out); + assert!(f.take_alerts().is_empty()); + assert!(f.signin_found(1, Err(DEAD), now)); + assert!(f.accounts()[1].signed_out); + let texts = f.take_alerts(); + assert_eq!(texts.len(), 1, "{texts:?}"); + assert!( + texts[0].contains("token file for login primary"), + "{texts:?}" + ); + assert!(!f.signin_found(1, Err(DEAD), now), "said once"); + } + #[cfg(not(windows))] #[test] fn off_windows_a_dpapi_entry_fails_plainly() { diff --git a/crates/buzz-acp/src/factory/signin_check.rs b/crates/buzz-acp/src/factory/signin_check.rs index 392b1ac129c..96f1102d27b 100644 --- a/crates/buzz-acp/src/factory/signin_check.rs +++ b/crates/buzz-acp/src/factory/signin_check.rs @@ -118,10 +118,27 @@ impl Factory { match found { SignIn::Unknown => false, SignIn::Valid => self.signin_restored(index, now), - SignIn::SignedOut => self.signin_lost(index, now), + SignIn::SignedOut => self.signin_lost(index, &check_failure(), now), } } + /// [`signin_checked`](Self::signin_checked), for a check whose login's + /// file may have given it nothing to ask about. No answer from PowerShell + /// concludes nothing, the same as no answer from the CLI. + pub(crate) fn signin_found( + &mut self, + index: usize, + found: Result, + now: DateTime, + ) -> bool { + let (found, why) = match found { + Ok(found) => return self.signin_checked(index, found, now), + Err(unread) => unread, + }; + tracing::warn!(login = index, "sign-in check: its token file: {why}"); + found != SignIn::Unknown && self.token_file_failed(index, (found, why), now) + } + /// The check says login `index` is not signed in, and no request was in /// flight to find that out the hard way. /// @@ -138,7 +155,12 @@ impl Factory { /// /// A login already known to be out changes nothing either way: the check /// runs every 15 minutes and must not card or text on every one of them. - fn signin_lost(&mut self, index: usize, now: DateTime) -> bool { + pub(super) fn signin_lost( + &mut self, + index: usize, + failure: &Failure, + now: DateTime, + ) -> bool { let known = match self.accounts.get(index) { Some(account) => account.signed_out, // No pool: the single login's state is the harness's own. @@ -148,12 +170,12 @@ impl Factory { return false; } if !self.accounts.is_empty() && !self.work_runs_on(index, now) { - return self.spare_signin_lost(index); + return self.spare_signin_lost(index, failure); } if self.health == Health::Blocked { return self.signin_lost_while_parked(index); } - self.record_limit_on(index, &check_failure(), now); + self.record_limit_on(index, failure, now); true } @@ -195,12 +217,16 @@ impl Factory { /// and no all-clear owed afterwards for an outage that never happened. /// The owner still hears once, because a pool quietly down to its last /// working login is how the next outage becomes a surprise. - fn spare_signin_lost(&mut self, index: usize) -> bool { + fn spare_signin_lost(&mut self, index: usize, failure: &Failure) -> bool { self.accounts[index].signed_out = true; - let line = format!( - "{}'s backup sign-in needs signing in again; the agent is unaffected.", - self.who() - ); + let line = if failure.slug == dpapi::UNREAD_SLUG { + format!("{} {}. The agent is unaffected.", self.who(), failure.text) + } else { + format!( + "{}'s backup sign-in needs signing in again; the agent is unaffected.", + self.who() + ) + }; tracing::warn!("{line}"); self.alert_aside(&line); true @@ -244,6 +270,18 @@ impl Factory { } } +/// What `probe` says about a login, unless its file gave no token. Then the +/// CLI is not asked: it would judge the agent's own environment instead. +pub(crate) async fn ask( + env: dpapi::LoginEnv, + probe: impl FnOnce(Vec<(String, String)>) -> F, +) -> Result +where + F: std::future::Future, +{ + Ok(probe(env?).await) +} + /// Ask the Claude CLI whether `env` is signed in, spending no usage. /// /// `env` is a login's launch environment: a folder entry's diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 7200d931fb2..fd4a6bc2d78 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -6239,17 +6239,12 @@ fn spawn_signin_check(config: &Config, rest: &relay::RestClient) { tokio::spawn(async move { let mut changed = false; for (login, env) in targets { - let found = match env { - Ok(env) => factory::signin_probe(&env).await, - Err((found, why)) => { - tracing::warn!(login, "sign-in check: its token file: {why}"); - found - } - }; + let probe = |env: Vec<_>| async move { factory::signin_probe(&env).await }; + let found = factory::signin_ask(env, probe).await; tracing::debug!(login, "sign-in check: {found:?}"); changed |= factory .lock() - .signin_checked(login, found, chrono::Utc::now()); + .signin_found(login, found, chrono::Utc::now()); } if !changed { return; From 3d7175d7ba28ba5b60369b551282d6ebc53dcc14 Mon Sep 17 00:00:00 2001 From: QuicksilverSlick Date: Mon, 9 Nov 2026 14:40:29 -0600 Subject: [PATCH 5/7] fix(acp): send the owner's text from a launch that found a login dead A dead token file at boot, a slot refill or an idle move queued the owner's text, and only a finished turn or a sign-in check that changed something sent it. An idle agent ran on another account and the phone stayed silent. The maintenance beat now sends a text that is waiting. It does nothing when none is, so a STATE record that merely fell due keeps its carrier and #factory keeps its cadence. Claude-Account: russelledeming@gmail.com Co-Authored-By: Claude Opus 5.5 --- crates/buzz-acp/src/factory/alert.rs | 5 ++++ crates/buzz-acp/src/lib.rs | 44 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/crates/buzz-acp/src/factory/alert.rs b/crates/buzz-acp/src/factory/alert.rs index 0e7c10ef218..c9a9ff7e20e 100644 --- a/crates/buzz-acp/src/factory/alert.rs +++ b/crates/buzz-acp/src/factory/alert.rs @@ -113,6 +113,11 @@ impl Factory { pub(crate) fn take_alerts(&mut self) -> Vec { std::mem::take(&mut self.alerts) } + + /// Whether a text is waiting for something to send it. + pub(crate) fn alerts_waiting(&self) -> bool { + !self.alerts.is_empty() + } } /// Run the owner's alert command with `text` as its last argument. diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index fd4a6bc2d78..21c667b4a66 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -3659,6 +3659,8 @@ async fn run_harness( spawn_signin_check(config, &ctx.rest_client); } } + + report_queued_alerts(config, &ctx.rest_client); } // Move an idle agent off a login the pool has given up on, before a @@ -6166,6 +6168,15 @@ fn card_dropped(config: &Config, batch: &FlushBatch, why: &str) { } } +/// Send the owner's texts a launch queued: boot, a slot refill and an idle +/// move have no turn ending to carry them out. Only when one is waiting, so a +/// STATE record that merely fell due keeps its own carrier. +fn report_queued_alerts(config: &Config, rest: &relay::RestClient) { + if config.factory.lock().alerts_waiting() { + report_factory_state(config, rest); + } +} + /// Log the STATE record when the harness's health changed, post it to /// #factory when one is configured (spec 4.2), and text the owner about a /// Critical (spec M2 item 4). Never into a work thread. @@ -15220,6 +15231,39 @@ mod error_outcome_emission_tests { } } + /// A login lost at a launch queues the owner's text with no turn ending + /// to carry it, so the maintenance beat sends it, and takes nothing else. + #[tokio::test] + async fn the_maintenance_beat_sends_a_launchs_text_and_nothing_else() { + let mut config = test_config(); + config.factory = factory::FactoryHandle::new(factory::Factory::new( + Some("/pool/one/claude;/pool/two/claude"), + "claude-agent-acp", + )); + let rest = relay::RestClient { + http: reqwest::Client::new(), + base_url: "http://localhost:0".into(), + keys: nostr::Keys::generate(), + auth_tag_json: None, + }; + let now = chrono::Utc::now(); + let lost = |login| { + config + .factory + .lock() + .signin_checked(login, factory::SignIn::SignedOut, now) + }; + assert!(lost(0)); + assert_eq!(config.factory.lock().take_alerts().len(), 1); + report_queued_alerts(&config, &rest); + let due = config.factory.lock().take_report(now); + assert!(due.is_some(), "no text waiting: the record is left alone"); + + assert!(lost(1)); + report_queued_alerts(&config, &rest); + assert!(config.factory.lock().take_alerts().is_empty(), "sent"); + } + /// With an account pool, a weekly limit on the primary re-runs the same /// batch at once: it stays queued and due, and the agent restarts on the /// second login. From 556f7e8a1051f8b8cbc559e12f129600173eaaf7 Mon Sep 17 00:00:00 2001 From: QuicksilverSlick Date: Mon, 9 Nov 2026 14:41:49 -0600 Subject: [PATCH 6/7] fix(acp): one launch reads each token file once A launch could read the same dead file once per pool entry plus one, under the factory lock on the main loop, so a hung PowerShell held the loop for 30 s or more and the comment's 15 s was low. The loop now stops when the pool offers a login it already failed, and the comments give the real bound: 15 s per dpapi entry. Claude-Account: russelledeming@gmail.com Co-Authored-By: Claude Opus 5.5 --- crates/buzz-acp/src/factory.rs | 15 +++++++-------- crates/buzz-acp/src/factory/dpapi.rs | 20 ++++++++++++++++++-- crates/buzz-acp/src/lib.rs | 9 ++++++--- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/crates/buzz-acp/src/factory.rs b/crates/buzz-acp/src/factory.rs index 2767bf688a0..8deaea0ce24 100644 --- a/crates/buzz-acp/src/factory.rs +++ b/crates/buzz-acp/src/factory.rs @@ -814,27 +814,26 @@ impl Factory { slot: usize, now: DateTime, ) -> Result, String> { - let mut dead = String::new(); + let mut dead = None; + let mut tried = HashSet::new(); // ponytail: `pick` still offers a login when all are out, so a dead // file ranked first then fails the launch without trying the others, // which are out too. The slot refill tries again. - for _ in 0..=self.accounts.len() { - let Some(i) = self.pick(now) else { - return Ok(Vec::new()); - }; + while let Some(i) = self.pick(now).filter(|&i| tried.insert(i)) { match self.account_env(i) { Ok(env) => { self.started_on.insert(slot, i); return Ok(env); } Err(unread) => { - dead = format!("login {}: {}", self.at(i), unread.1); - tracing::warn!("{dead}; trying the next login"); + let why = format!("login {}: {}", self.at(i), unread.1); + tracing::warn!("{why}; trying the next login"); self.token_file_failed(i, unread, now); + dead = Some(why); } } } - Err(dead) + dead.map_or(Ok(Vec::new()), Err) } /// [`login_env`](Self::login_env) for tests with a login left. diff --git a/crates/buzz-acp/src/factory/dpapi.rs b/crates/buzz-acp/src/factory/dpapi.rs index c78aea3c621..7383b8b6f30 100644 --- a/crates/buzz-acp/src/factory/dpapi.rs +++ b/crates/buzz-acp/src/factory/dpapi.rs @@ -52,8 +52,9 @@ fn powershell(file: &str) -> tokio::process::Command { } /// ponytail: blocks the caller and the factory lock for one PowerShell start -/// (about a second, 15 s at worst) at each launch and each 15-minute check. -/// Move it off the lock if launches ever become frequent. +/// (about a second, 15 s at worst) per `dpapi:` entry, at each launch and each +/// 15-minute check, on the main loop. Move it off the lock if launches ever +/// become frequent. #[cfg(windows)] fn decrypt(file: &str) -> Result { use std::io::Read; @@ -139,6 +140,7 @@ mod tests { use crate::acp::AcpError; use crate::factory::{classify, signin_ask as ask, OAUTH_TOKEN_ENV}; use crate::scope::SessionScope; + use std::sync::atomic::{AtomicUsize, Ordering}; const TOKEN: &str = "fake-token-for-tests"; const DEAD: Unread = (SignIn::SignedOut, "it did not decrypt"); @@ -353,6 +355,20 @@ mod tests { assert!(!f.signin_found(1, Err(DEAD), now), "said once"); } + /// One launch reads each file once: it can hold the factory lock for 15 s. + #[test] + fn one_launch_reads_each_file_once() { + static READS: AtomicUsize = AtomicUsize::new(0); + fn counted(_: &str) -> Result { + READS.fetch_add(1, Ordering::SeqCst); + Err(SILENT) + } + let mut f = Factory::new(Some("a=dpapi:x;b=dpapi:y"), "claude-agent-acp"); + f.decrypt = Some(counted); + assert!(f.login_env(0, Utc::now()).is_err()); + assert_eq!(READS.load(Ordering::SeqCst), 2); + } + #[cfg(not(windows))] #[test] fn off_windows_a_dpapi_entry_fails_plainly() { diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 21c667b4a66..8e9ad2aa285 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -3606,7 +3606,8 @@ async fn run_harness( // Slot refill: spawn background tasks for empty slots whose // circuit breaker allows it. spawn_and_init runs off the main - // loop so it never blocks event processing. + // loop so it never blocks event processing; choosing the login + // does not, and reading a `dpapi:` file can take 15 s. for (idx, slot) in crash_history.iter_mut().enumerate() { if pool.slot_alive(idx) || slot.respawn_in_flight { continue; @@ -6236,8 +6237,10 @@ async fn deliver_factory_state( /// Ask each login of the pool whether its sign-in is still good, and act on /// what comes back exactly as a failed turn would (spec M2 item 3). /// -/// Runs off the main loop: every probe spawns a short-lived process. It costs -/// no model usage, and only a definite "not signed in" changes anything. +/// Runs off the main loop: every probe spawns a short-lived process. Only +/// collecting the targets does not, and reading a `dpapi:` file can take 15 s. +/// It costs no model usage, and only a definite "not signed in" changes +/// anything. fn spawn_signin_check(config: &Config, rest: &relay::RestClient) { let factory = config.factory.clone(); let targets = factory.lock().signin_check_targets(); From 2904daf798ea62a1dc6ac0078b3bfe6b7315c62d Mon Sep 17 00:00:00 2001 From: QuicksilverSlick Date: Mon, 9 Nov 2026 14:42:05 -0600 Subject: [PATCH 7/7] fix(acp): a garbled dpapi entry is dead, never a folder login A typo in a dpapi entry (DPAPI: in capitals, a label with a dot or a space, a label over 32 characters) launched the agent with CLAUDE_CONFIG_DIR set to the whole string and any token removed. Nothing named the bad entry. It now fails that entry with a plain reason and the pool moves on. Claude-Account: russelledeming@gmail.com Co-Authored-By: Claude Opus 5.5 --- crates/buzz-acp/src/factory.rs | 14 +++++++++++++- crates/buzz-acp/src/factory/dpapi.rs | 29 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/crates/buzz-acp/src/factory.rs b/crates/buzz-acp/src/factory.rs index 8deaea0ce24..0e7bc7ce3b9 100644 --- a/crates/buzz-acp/src/factory.rs +++ b/crates/buzz-acp/src/factory.rs @@ -1480,18 +1480,30 @@ pub(crate) const PRIMARY_LABEL: &str = "primary"; /// means the agent's own environment, unchanged, so its own long-lived token /// is the login. The label is still the owner's own word, which is how a /// token-based login gets a name that is neither a token nor a path. +/// +/// A `dpapi:` entry that does not parse is kept as one with no file, which +/// fails plainly when it is launched. It must not become a folder login. fn parse_entry(entry: &str) -> (String, Option) { match entry.split_once('=') { Some((label, value)) if is_plain_label(label) && value.trim() == ENV_ENTRY => { (slug(label.trim()), None) } - Some((label, dir)) if is_plain_label(label) && !dir.trim().is_empty() => { + Some((label, dir)) + if is_plain_label(label) && !dir.trim().is_empty() && !garbled_dpapi(dir) => + { (slug(label.trim()), Some(dir.trim().to_string())) } + _ if garbled_dpapi(entry) => (account_label(entry), Some(DPAPI_ENTRY.to_string())), _ => (account_label(entry), Some(entry.to_string())), } } +/// Whether `value` reaches for a `dpapi:` file without being one. +fn garbled_dpapi(value: &str) -> bool { + let value = value.trim(); + !value.starts_with(DPAPI_ENTRY) && value.to_ascii_lowercase().contains(DPAPI_ENTRY) +} + /// Whether `s` is a label the owner wrote rather than the head of a path: at /// most 32 characters of `[A-Za-z0-9_-]`, so a Windows drive or a `~` entry /// never parses as one. diff --git a/crates/buzz-acp/src/factory/dpapi.rs b/crates/buzz-acp/src/factory/dpapi.rs index 7383b8b6f30..bf06e0907e8 100644 --- a/crates/buzz-acp/src/factory/dpapi.rs +++ b/crates/buzz-acp/src/factory/dpapi.rs @@ -22,6 +22,12 @@ pub(super) type Decrypt = fn(&str) -> Result; /// The token in `file`. Blank is a failure: a failed decrypt prints nothing, /// and the Claude CLI reads a blank token as signed out. pub(super) fn token(file: &str, read: Option) -> Result { + if file.is_empty() { + return Err(( + SignIn::SignedOut, + "its pool entry is not written label=dpapi:; correct it in the agent's settings", + )); + } match read.unwrap_or(decrypt)(file) { Ok(token) if token.trim().is_empty() => Err(( SignIn::SignedOut, @@ -369,6 +375,29 @@ mod tests { assert_eq!(READS.load(Ordering::SeqCst), 2); } + /// A typo in a `dpapi:` entry fails that entry and says so. It never + /// becomes a folder login named after the typo. + #[test] + fn a_garbled_dpapi_entry_is_dead_and_never_a_folder() { + let now = Utc::now(); + let long = format!("{}=dpapi:C:/x/p.dpapi", "a".repeat(33)); + for entry in [ + "primary=DPAPI:C:/x/p.dpapi", + "my.login=dpapi:C:/x/p.dpapi", + "primary login=dpapi:C:/x/p.dpapi", + "primary=dpapi:", + long.as_str(), + ] { + let mut f = pool(&format!("{entry};fallback=C:/f")); + f.decrypt = Some(|_| panic!("a garbled entry has no file to read")); + let env = f.launch_env(0, now); + assert_eq!(env, pair("CLAUDE_CONFIG_DIR", "C:/f"), "{entry}"); + assert_eq!(f.label(0), "fallback", "{entry}"); + let texts = f.take_alerts(); + assert!(texts[0].contains("label=dpapi:"), "{entry}: {texts:?}"); + } + } + #[cfg(not(windows))] #[test] fn off_windows_a_dpapi_entry_fails_plainly() {