diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 28978199bfd..f5763b461a6 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(); @@ -756,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. @@ -787,7 +795,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 +5994,70 @@ mod tests { ); } + /// 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) { + let cmd = format!("if defined {var} (echo set) else (echo unset)"); + ("cmd", "/c", cmd) + } else { + let sh = format!("[ \"${{{var}+x}}\" ] && echo set || echo unset"); + ("sh", "-c", sh) + }; + 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; + 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] async fn idle_timeout_fires_on_silent_process() { let mut client = spawn_script("sleep 10").await; @@ -6661,10 +6733,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..0e7bc7ce3b9 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; @@ -25,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, }; @@ -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, @@ -556,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. @@ -660,6 +680,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. @@ -732,6 +754,7 @@ impl Factory { label, limited: None, signed_out: false, + refused: None, } }) .collect(); @@ -784,32 +807,61 @@ 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 = 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. + 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) => { + 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); + } + } + } + dead.map_or(Ok(Vec::new()), Err) + } + + /// [`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. @@ -861,6 +913,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(); @@ -900,9 +953,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. @@ -1054,6 +1117,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 @@ -1129,7 +1193,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 { @@ -1154,6 +1218,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", @@ -1174,8 +1259,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 \ @@ -1238,13 +1323,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(), @@ -1374,6 +1459,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"; @@ -1385,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. @@ -1972,10 +2079,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 +2089,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..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. @@ -236,9 +241,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 +548,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..bf06e0907e8 --- /dev/null +++ b/crates/buzz-acp/src/factory/dpapi.rs @@ -0,0 +1,406 @@ +//! 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. +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 { + 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, + "the file is missing or did not decrypt for this Windows user; \ + run set-account.ps1 to replace it", + )), + 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) 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; + use std::time::{Duration, Instant}; + 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 { + 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")) +} + +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, 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"); + const SILENT: Unread = (SignIn::Unknown, "no answer"); + + /// Stands in for DPAPI: no test here reaches PowerShell. + fn fake(file: &str) -> Result { + match file { + good if good.ends_with("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 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"] { + 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))); + } + + 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"); + } + + /// 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"); + } + + /// 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); + } + + /// 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() { + 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..96f1102d27b 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))) @@ -117,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. /// @@ -137,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. @@ -147,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 } @@ -194,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 @@ -217,7 +244,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) @@ -235,18 +270,26 @@ 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 -/// `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 +303,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 +333,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 +372,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 +427,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 +520,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 +684,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 +703,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..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; @@ -3619,7 +3620,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()); @@ -3659,6 +3660,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 +6169,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. @@ -6225,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(); @@ -6239,11 +6253,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 = factory::signin_probe(&env).await; + 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; @@ -6960,7 +6975,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 +7266,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 +7325,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 +7434,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, @@ -15219,6 +15234,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.