From 03a184ff1e5190bbd36249c9c2505d60f37f4322 Mon Sep 17 00:00:00 2001 From: bitpshr Date: Fri, 23 Oct 2026 09:08:25 -0400 Subject: [PATCH 1/2] fix: onboard waits longer for the approval and tries an unreachable token endpoint again - The sign-in waits 30 minutes instead of 5. Some people approve 10 to 30 minutes after the page opens, and those clicks reached nothing. - A token request that never connected is sent once more after 1 s; the code never got there, so it's still good. A request that connected is never sent again. When both tries fail to connect, the error page gets `temporarily_unavailable` instead of `server_error`. - A copied key that /v1/user refuses is "couldn't be checked", not "belongs to a different account": a key without user_read is refused there but works otherwise. The reason goes to the worker's log. --- cli/elevenlabs/workflow/loopback.rs | 206 ++++++++++++++++++--- cli/elevenlabs/workflow/onboard/auth.rs | 20 +- cli/elevenlabs/workflow/onboard/connect.rs | 12 +- cli/elevenlabs/workflow/onboard/session.rs | 5 +- cli/elevenlabs/workflow/onboard/tests.rs | 24 ++- 5 files changed, 223 insertions(+), 44 deletions(-) diff --git a/cli/elevenlabs/workflow/loopback.rs b/cli/elevenlabs/workflow/loopback.rs index 046d014..f9e766a 100644 --- a/cli/elevenlabs/workflow/loopback.rs +++ b/cli/elevenlabs/workflow/loopback.rs @@ -13,6 +13,8 @@ //! - A callback for another sign-in (a tab left open by an earlier one) is //! answered with the error page, and the wait goes on: an old tab can't end //! the sign-in the newest one is for. +//! - A token endpoint that can't be reached is tried once more: the code never +//! got there, so it is still good. use std::io::ErrorKind; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; @@ -32,6 +34,8 @@ use tokio::task::JoinSet; const CALLBACK_PATH: &str = "/callback"; /// How long one connection may take to send its request. const READ_TIMEOUT: Duration = Duration::from_secs(10); +/// How long to wait before trying an unreachable token endpoint again. +const EXCHANGE_RETRY_DELAY: Duration = Duration::from_secs(1); /// What the error page says when the login failed after the callback; the /// reason is in the worker's log. const AFTER_CALLBACK_FAILED: &str = @@ -100,10 +104,16 @@ pub(super) async fn sign_in(login: Login<'_>) -> Result { let _ = redirect(&mut browser, login.success_url).await; Ok(token.access_token) } - Err(e) => { - let page = error_page(login.error_url, "server_error", Some(AFTER_CALLBACK_FAILED)); + Err(failure) => { + // Its own OAuth code, so the analytics can tell these apart. + let code = if failure.unreachable { + "temporarily_unavailable" + } else { + "server_error" + }; + let page = error_page(login.error_url, code, Some(AFTER_CALLBACK_FAILED)); let _ = redirect(&mut browser, &page).await; - Err(e) + Err(failure.error) } } } @@ -361,39 +371,64 @@ fn challenge(verifier: &str) -> String { .encode(sha2::Sha256::digest(verifier.as_bytes())) } -/// Trade the code for a token, as `auth login` does. +/// Why a code couldn't be traded for a token. +struct ExchangeFailed { + error: CliError, + /// The token endpoint was never reached, so the code is still unused. + unreachable: bool, +} + +/// Trade the code for a token, as `auth login` does. A token endpoint that +/// can't be reached is tried once more. async fn exchange( token_url: &str, client_id: &str, code: &str, verifier: &str, redirect_uri: &str, -) -> Result { +) -> Result { #[derive(Deserialize)] struct Token { access_token: String, refresh_token: Option, expires_in: Option, } - let failed = |e: String| CliError::Auth(format!("PKCE token exchange failed: {e}")); - let response = reqwest::Client::builder() + let failed = |e: String, unreachable: bool| ExchangeFailed { + error: CliError::Auth(format!("PKCE token exchange failed: {e}")), + unreachable, + }; + let client = reqwest::Client::builder() .connect_timeout(Duration::from_secs(10)) .timeout(Duration::from_secs(30)) .build() - .map_err(|e| failed(e.to_string()))? - .post(token_url) - .form(&[ - ("grant_type", "authorization_code"), - ("code", code), - ("code_verifier", verifier), - ("client_id", client_id), - ("redirect_uri", redirect_uri), - ]) - .send() - .await - .map_err(|e| failed(e.to_string()))?; + .map_err(|e| failed(e.to_string(), false))?; + let send = || { + client + .post(token_url) + .form(&[ + ("grant_type", "authorization_code"), + ("code", code), + ("code_verifier", verifier), + ("client_id", client_id), + ("redirect_uri", redirect_uri), + ]) + .send() + }; + // Only a request that never connected is sent again: once the endpoint + // has the code, a second try could only be refused. + let response = match send().await { + Err(e) if e.is_connect() => { + tokio::time::sleep(EXCHANGE_RETRY_DELAY).await; + send().await + } + sent => sent, + } + .map_err(|e| failed(e.to_string(), e.is_connect()))?; let status = response.status(); - let body = response.text().await.map_err(|e| failed(e.to_string()))?; + let body = response + .text() + .await + .map_err(|e| failed(e.to_string(), false))?; if !status.is_success() { let detail = serde_json::from_str::(&body) .ok() @@ -403,10 +438,10 @@ async fn exchange( .and_then(|d| d.as_str().map(str::to_string)) }) .unwrap_or_else(|| body.chars().take(200).collect()); - return Err(failed(format!("HTTP {status}: {detail}"))); + return Err(failed(format!("HTTP {status}: {detail}"), false)); } let token: Token = - serde_json::from_str(&body).map_err(|e| failed(format!("token response: {e}")))?; + serde_json::from_str(&body).map_err(|e| failed(format!("token response: {e}"), false))?; Ok(TokenBundle::from_token_response( &token.access_token, token.refresh_token.as_deref(), @@ -417,10 +452,10 @@ async fn exchange( #[cfg(test)] mod tests { use std::io::{Read, Write}; - use std::net::{Ipv6Addr, TcpListener, TcpStream}; + use std::net::{Ipv4Addr, Ipv6Addr, TcpListener, TcpStream}; use std::time::Duration; - use super::{bind, classify, wait_for_callback, Callback}; + use super::{bind, classify, exchange, wait_for_callback, Callback, EXCHANGE_RETRY_DELAY}; const ERROR_PAGE: &str = "https://example.test/error"; @@ -634,4 +669,127 @@ mod tests { .to_string(); assert!(err.contains("Timed out waiting"), "{err}"); } + + const CALLBACK: &str = "http://localhost:8484/callback"; + + /// What a stand-in token endpoint does with each request. + #[derive(Clone, Copy)] + enum Answer { + Token, + Refuse, + /// Read the request, then close without a response. + Hang, + } + + /// Answers every connection to `listener` with `answer`; the count is of + /// connections that sent a request. + fn token_endpoint( + listener: tokio::net::TcpListener, + answer: Answer, + ) -> std::sync::Arc { + use std::sync::atomic::Ordering; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + let requests = std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let count = requests.clone(); + tokio::spawn(async move { + while let Ok((mut socket, _)) = listener.accept().await { + let mut buf = [0u8; 4096]; + if !matches!(socket.read(&mut buf).await, Ok(n) if n > 0) { + continue; + } + count.fetch_add(1, Ordering::SeqCst); + let (status, body) = match answer { + Answer::Token => ("200 OK", r#"{"access_token":"token","expires_in":3600}"#), + Answer::Refuse => ("400 Bad Request", r#"{"error":"invalid_grant"}"#), + Answer::Hang => continue, + }; + let response = format!( + "HTTP/1.1 {status}\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{body}", + body.len() + ); + let _ = socket.write_all(response.as_bytes()).await; + } + }); + requests + } + + /// A port nothing listened on a moment ago. Another test may take it + /// before it's used, so the tests below try again with a new one. + fn closed_port() -> u16 { + TcpListener::bind((Ipv4Addr::LOCALHOST, 0)) + .unwrap() + .local_addr() + .unwrap() + .port() + } + + fn token_url(port: u16) -> String { + format!("http://127.0.0.1:{port}/v1/oauth/token") + } + + #[tokio::test] + async fn a_token_endpoint_that_was_unreachable_gets_the_code_on_the_second_try() { + for _ in 0..5 { + let port = closed_port(); + // The endpoint starts listening only after the first try was refused. + let up = tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(300)).await; + let listener = tokio::net::TcpListener::bind((Ipv4Addr::LOCALHOST, port)) + .await + .ok()?; + Some(token_endpoint(listener, Answer::Token)) + }); + let started = std::time::Instant::now(); + let token = exchange(&token_url(port), "client", "code", "verifier", CALLBACK).await; + let Some(requests) = up.await.unwrap() else { + continue; // another test took the port + }; + assert!( + started.elapsed() >= EXCHANGE_RETRY_DELAY, + "it waited, then tried again" + ); + assert_eq!(token.ok().map(|t| t.access_token).as_deref(), Some("token")); + assert_eq!(requests.load(std::sync::atomic::Ordering::SeqCst), 1); + return; + } + panic!("no port stayed free for the test"); + } + + #[tokio::test] + async fn a_token_endpoint_that_stays_unreachable_is_reported_as_unreachable() { + for _ in 0..5 { + let started = std::time::Instant::now(); + let url = token_url(closed_port()); + let Err(failure) = exchange(&url, "client", "code", "verifier", CALLBACK).await else { + continue; // another test took the port + }; + if failure.unreachable { + assert!(started.elapsed() >= EXCHANGE_RETRY_DELAY, "it tried again"); + return; + } + } + panic!("a closed port was never reported as unreachable"); + } + + /// Once the endpoint has the code, sending it again could only be refused, + /// whether the endpoint answered or not. + #[tokio::test] + async fn a_code_that_reached_the_endpoint_is_never_sent_again() { + for answer in [Answer::Refuse, Answer::Hang] { + let listener = tokio::net::TcpListener::bind((Ipv4Addr::LOCALHOST, 0)) + .await + .unwrap(); + let port = listener.local_addr().unwrap().port(); + let requests = token_endpoint(listener, answer); + let Err(failure) = + exchange(&token_url(port), "client", "code", "verifier", CALLBACK).await + else { + panic!("the endpoint never handed out a token"); + }; + assert!(!failure.unreachable); + // Time for a second try to arrive, if there were one. + tokio::time::sleep(EXCHANGE_RETRY_DELAY * 2).await; + assert_eq!(requests.load(std::sync::atomic::Ordering::SeqCst), 1); + } + } } diff --git a/cli/elevenlabs/workflow/onboard/auth.rs b/cli/elevenlabs/workflow/onboard/auth.rs index b40d27a..b9196cb 100644 --- a/cli/elevenlabs/workflow/onboard/auth.rs +++ b/cli/elevenlabs/workflow/onboard/auth.rs @@ -137,23 +137,23 @@ pub(super) fn authorize_params( params } -/// `Err` when the signed-in account itself cannot be identified; `Ok(false)` only -/// when it can and the key does not belong to it. `token` is the sign-in's -/// access token. +/// `Ok(false)` only when the key opens a different account than the sign-in. +/// `Err` when either account can't be identified: a key without `user_read` +/// is refused here, yet may work for everything else. `token` is the +/// sign-in's access token. pub(super) fn key_matches_sign_in( client: &reqwest::Client, base: &str, token: &str, key: &str, ) -> Result { - let unknown = || { - CliError::Other(anyhow::anyhow!( - "the signed-in account could not be identified" - )) - }; + let unknown = |whose: &str| CliError::Other(anyhow::anyhow!("{whose} could not be identified")); let bearer = format!("Bearer {token}"); - let signed_in = account_for(client, base, "authorization", &bearer)?.ok_or_else(unknown)?; - Ok(account_for(client, base, "xi-api-key", key)? == Some(signed_in)) + let signed_in = account_for(client, base, "authorization", &bearer)? + .ok_or_else(|| unknown("the signed-in account"))?; + let owner = account_for(client, base, "xi-api-key", key)? + .ok_or_else(|| unknown("the key's account"))?; + Ok(owner == signed_in) } /// The `user_id` a credential opens; `Ok(None)` when the API rejects it. diff --git a/cli/elevenlabs/workflow/onboard/connect.rs b/cli/elevenlabs/workflow/onboard/connect.rs index e137446..e0281bf 100644 --- a/cli/elevenlabs/workflow/onboard/connect.rs +++ b/cli/elevenlabs/workflow/onboard/connect.rs @@ -206,10 +206,14 @@ pub(super) fn outcome_of( status: "clipboard_invalid", why: "the key on the clipboard belongs to a different account", }, - Err(_) => Outcome::Manual { - status: "could_not_verify", - why: "the key couldn't be checked, so it wasn't stored", - }, + Err(e) => { + // The worker's log is the only place the reason is kept. + eprintln!("The key on the clipboard couldn't be checked: {e}"); + Outcome::Manual { + status: "could_not_verify", + why: "the key couldn't be checked, so it wasn't stored", + } + } }, Clip::Marker(Marker::NotPermitted) => Outcome::NotPermitted, Clip::Marker(Marker::NotAvailable) => Outcome::Manual { diff --git a/cli/elevenlabs/workflow/onboard/session.rs b/cli/elevenlabs/workflow/onboard/session.rs index 7513576..1e596c6 100644 --- a/cli/elevenlabs/workflow/onboard/session.rs +++ b/cli/elevenlabs/workflow/onboard/session.rs @@ -32,8 +32,9 @@ use super::clipboard::Clipboard; use super::env_file; use super::reply::{Next, Reply}; -/// How long the sign-in waits for the approval page, as `auth login` does. -pub(super) const SIGN_IN_MINUTES: u64 = 5; +/// How long the sign-in waits for the approval page. `auth login` waits 5 +/// minutes, but some people approve 10 to 30 minutes after the page opens. +pub(super) const SIGN_IN_MINUTES: u64 = 30; /// How long a new worker may take to take its lock before starting counts as failed. const STARTUP: Duration = Duration::from_secs(10); const POLL: Duration = Duration::from_millis(250); diff --git a/cli/elevenlabs/workflow/onboard/tests.rs b/cli/elevenlabs/workflow/onboard/tests.rs index f634d8f..1baee85 100644 --- a/cli/elevenlabs/workflow/onboard/tests.rs +++ b/cli/elevenlabs/workflow/onboard/tests.rs @@ -931,12 +931,12 @@ fn a_failed_sign_in_says_what_to_do_and_keeps_its_flags() { ); let late = failed( &p, - "Timed out waiting for the OAuth callback after 300s.", + "Timed out waiting for the OAuth callback after 1800s.", false, ) .to_value(); assert_eq!(late["next"], "run"); - assert!(late["say"].as_str().unwrap().contains("5 minutes")); + assert!(late["say"].as_str().unwrap().contains("30 minutes")); let other = failed(&p, "state mismatch", false).to_value(); assert!(other["say"].as_str().unwrap().contains("state mismatch")); assert_eq!(other["details"]["key_status"], "sign_in_failed"); @@ -1189,19 +1189,35 @@ async fn a_clipboard_key_is_checked_with_the_token_the_sign_in_returned() { use wiremock::matchers::{header, method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; const KEY: &str = "sk_0123456789abcdef0123456789abcdef01234567"; + const OTHER_ACCOUNTS_KEY: &str = "sk_1123456789abcdef0123456789abcdef01234567"; + const LIMITED_KEY: &str = "sk_2123456789abcdef0123456789abcdef01234567"; let server = MockServer::start().await; - for (name, value) in [("authorization", "Bearer token"), ("xi-api-key", KEY)] { + for (name, value, user) in [ + ("authorization", "Bearer token", "user"), + ("xi-api-key", KEY, "user"), + ("xi-api-key", OTHER_ACCOUNTS_KEY, "someone-else"), + ] { Mock::given(method("GET")) .and(path("/v1/user")) .and(header(name, value)) .respond_with( - ResponseTemplate::new(200).set_body_json(serde_json::json!({"user_id": "user"})), + ResponseTemplate::new(200).set_body_json(serde_json::json!({"user_id": user})), ) .mount(&server) .await; } + // A key without `user_read` can't read the account. + Mock::given(method("GET")) + .and(path("/v1/user")) + .and(header("xi-api-key", LIMITED_KEY)) + .respond_with(ResponseTemplate::new(403)) + .mount(&server) + .await; let client = reqwest::Client::new(); assert!(key_matches_sign_in(&client, &server.uri(), "token", KEY).unwrap()); + assert!(!key_matches_sign_in(&client, &server.uri(), "token", OTHER_ACCOUNTS_KEY).unwrap()); + // Refused is not another account: it just can't be checked. + assert!(key_matches_sign_in(&client, &server.uri(), "token", LIMITED_KEY).is_err()); // Any other token opens no account here, so nothing vouches for the key. assert!(key_matches_sign_in(&client, &server.uri(), "other", KEY).is_err()); } From 5020e6c1ed0c6a6e88a3aa5d9e10f7f2dc48d69a Mon Sep 17 00:00:00 2001 From: bitpshr Date: Fri, 6 Nov 2026 11:07:57 -0500 Subject: [PATCH 2/2] feat: onboard says how the last sign-in ended and which coding agent ran it --- README.md | 13 +- cli/elevenlabs/workflow/api.rs | 28 +- cli/elevenlabs/workflow/loopback.rs | 47 +++- cli/elevenlabs/workflow/onboard/auth.rs | 39 ++- cli/elevenlabs/workflow/onboard/connect.rs | 49 +++- cli/elevenlabs/workflow/onboard/mod.rs | 31 ++- cli/elevenlabs/workflow/onboard/session.rs | 156 +++++++++++ cli/elevenlabs/workflow/onboard/tests.rs | 287 +++++++++++++++++++-- cli/elevenlabs/workflow/platform.rs | 98 ++++++- cli/elevenlabs/workflow/telemetry.rs | 14 +- 10 files changed, 685 insertions(+), 77 deletions(-) diff --git a/README.md b/README.md index 7598e42..2d76e71 100644 --- a/README.md +++ b/README.md @@ -549,8 +549,17 @@ elevenlabs-cli/1.7.1 os/macos install/npx `os/` is `macos`, `linux`, `windows` or `other`; `env/`, when present, is `ssh`, `container` or `wsl`; `install/` is read from where the binary sits (`npx`, `bunx`, `pnpm-dlx`, `yarn-dlx`, `npm`, `homebrew`, `scoop`, `installer` or -`other`). Nothing else about the machine is sent: no paths, hostnames or ids. -`onboard` puts the same values on the sign-in's approval link. +`other`); `agent/`, when a coding agent ran the command, is `claude-code`, +`claude-desktop`, `cursor`, `codex`, `gemini-cli` or `vscode`, read from a +variable the agent sets. Nothing else about the machine is sent: no paths, +hostnames or ids. `onboard` puts the same values on the sign-in's approval link. + +`onboard` also says how the project's last sign-in ended, when it ended in the +past day: `signin/` on its requests, and `cli_prev` on the next +approval link, where `` is a fixed word such as `timed-out`, +`stopped` or `no-key`. The link also carries `cli_prev_id`, the start of a +SHA-256 hash of that sign-in's `state`, so it can be matched to its approval +page; the state itself is never sent. To turn it off, pass `--telemetry off` to any command, or set `ELEVENLABS_TELEMETRY=off` (or `DO_NOT_TRACK=1`) to turn it off everywhere. diff --git a/cli/elevenlabs/workflow/api.rs b/cli/elevenlabs/workflow/api.rs index f21ef45..8785be1 100644 --- a/cli/elevenlabs/workflow/api.rs +++ b/cli/elevenlabs/workflow/api.rs @@ -81,9 +81,19 @@ pub(super) fn suffix_env_key() -> String { } pub fn command_scope(command: &'static str) -> CommandScope { + command_scope_with(command, None) +} + +/// [`command_scope`], with one more fixed tag after the command's (`onboard` +/// adds how the last sign-in ended). Built from fixed words by the caller, +/// like the command tag. +pub fn command_scope_with(command: &'static str, extra: Option) -> CommandScope { let key = suffix_env_key(); let previous = std::env::var(&key).ok(); - let tag = format!("cmd/{command}"); + let tag = match extra { + Some(extra) => format!("cmd/{command} {extra}"), + None => format!("cmd/{command}"), + }; let combined = match previous.as_deref().map(str::trim).filter(|s| !s.is_empty()) { Some(existing) => format!("{existing} {tag}"), None => tag, @@ -629,6 +639,22 @@ mod tests { }); } + #[test] + fn scope_puts_an_extra_tag_after_the_command() { + with_env(|key| { + std::env::set_var(key, "os/macos install/npx"); + { + let _scope = command_scope_with("onboard.check", Some("signin/timed-out".into())); + let tag = std::env::var(key).unwrap(); + assert_eq!( + tag, + "os/macos install/npx cmd/onboard.check signin/timed-out" + ); + } + assert_eq!(std::env::var(key).as_deref(), Ok("os/macos install/npx")); + }); + } + #[test] fn scope_ignores_a_blank_existing_value() { with_env(|key| { diff --git a/cli/elevenlabs/workflow/loopback.rs b/cli/elevenlabs/workflow/loopback.rs index f9e766a..07a7331 100644 --- a/cli/elevenlabs/workflow/loopback.rs +++ b/cli/elevenlabs/workflow/loopback.rs @@ -393,15 +393,16 @@ async fn exchange( refresh_token: Option, expires_in: Option, } - let failed = |e: String, unreachable: bool| ExchangeFailed { - error: CliError::Auth(format!("PKCE token exchange failed: {e}")), - unreachable, + // `kind` names the failure for `onboard`'s report (`onboard::connect::sign_in_failure`). + let failed = |kind: &str, e: String| ExchangeFailed { + error: CliError::Auth(format!("PKCE token exchange failed [{kind}]: {e}")), + unreachable: kind == "connect", }; let client = reqwest::Client::builder() .connect_timeout(Duration::from_secs(10)) .timeout(Duration::from_secs(30)) .build() - .map_err(|e| failed(e.to_string(), false))?; + .map_err(|e| failed("request", e.to_string()))?; let send = || { client .post(token_url) @@ -423,12 +424,22 @@ async fn exchange( } sent => sent, } - .map_err(|e| failed(e.to_string(), e.is_connect()))?; + .map_err(|e| { + // A connect that timed out never reached the endpoint either. + let kind = if e.is_connect() { + "connect" + } else if e.is_timeout() { + "timeout" + } else { + "request" + }; + failed(kind, e.to_string()) + })?; let status = response.status(); let body = response .text() .await - .map_err(|e| failed(e.to_string(), false))?; + .map_err(|e| failed("response", e.to_string()))?; if !status.is_success() { let detail = serde_json::from_str::(&body) .ok() @@ -438,10 +449,15 @@ async fn exchange( .and_then(|d| d.as_str().map(str::to_string)) }) .unwrap_or_else(|| body.chars().take(200).collect()); - return Err(failed(format!("HTTP {status}: {detail}"), false)); + let kind = if status.is_server_error() { + "server" + } else { + "refused" + }; + return Err(failed(kind, format!("HTTP {status}: {detail}"))); } - let token: Token = - serde_json::from_str(&body).map_err(|e| failed(format!("token response: {e}"), false))?; + let token: Token = serde_json::from_str(&body) + .map_err(|e| failed("response", format!("token response: {e}")))?; Ok(TokenBundle::from_token_response( &token.access_token, token.refresh_token.as_deref(), @@ -765,6 +781,12 @@ mod tests { }; if failure.unreachable { assert!(started.elapsed() >= EXCHANGE_RETRY_DELAY, "it tried again"); + // `onboard` reports it as `exchange-connect`. + assert!( + failure.error.to_string().contains("[connect]"), + "{}", + failure.error + ); return; } } @@ -787,6 +809,13 @@ mod tests { panic!("the endpoint never handed out a token"); }; assert!(!failure.unreachable); + if let Answer::Refuse = answer { + assert!( + failure.error.to_string().contains("[refused]"), + "{}", + failure.error + ); + } // Time for a second try to arrive, if there were one. tokio::time::sleep(EXCHANGE_RETRY_DELAY * 2).await; assert_eq!(requests.load(std::sync::atomic::Ordering::SeqCst), 1); diff --git a/cli/elevenlabs/workflow/onboard/auth.rs b/cli/elevenlabs/workflow/onboard/auth.rs index b9196cb..f3932e4 100644 --- a/cli/elevenlabs/workflow/onboard/auth.rs +++ b/cli/elevenlabs/workflow/onboard/auth.rs @@ -14,7 +14,7 @@ use super::super::platform; use super::super::say::run_async as block_on; use super::super::settings; use super::super::telemetry; -use super::session::SIGN_IN_MINUTES; +use super::session::{LastSignIn, SIGN_IN_MINUTES}; pub(super) const CLI_NAME: &str = "elevenlabs"; @@ -87,11 +87,14 @@ fn parse_sign_in(main_rs: &str) -> Result { }) } -/// Sign in through the browser, asking the page for a key. Returns the access token. +/// Sign in through the browser, asking the page for a key. `previous` is how +/// the last sign-in for this project ended, for the page to record. Returns +/// the access token. pub(super) fn sign_in( ctx: &AppContext, authorize_url: &str, no_browser: bool, + previous: Option<&LastSignIn>, ) -> Result { let config = sign_in_config()?; // A local --base-url also moves the token endpoint, for testing against a local backend. @@ -100,9 +103,13 @@ pub(super) fn sign_in( .and_then(loopback) .unwrap_or_else(|| config.token_url.trim_end_matches("/v1/oauth/token").into()); let token_url = format!("{oauth_base}/v1/oauth/token"); + let on = telemetry::enabled(); let params = authorize_params( - telemetry::enabled().then(|| (platform::detect(), platform::install_source())), + on.then(|| (platform::detect(), platform::install_source())), + on.then(platform::agent).flatten(), + previous.filter(|_| on), ); + let params: Vec<(&str, &str)> = params.iter().map(|(k, v)| (*k, v.as_str())).collect(); block_on(loopback::sign_in(loopback::Login { cli_name: CLI_NAME, scheme: &config.scheme, @@ -121,18 +128,30 @@ pub(super) fn sign_in( } /// What the sign-in adds to the approval link: the page should hand over a -/// key, and, with telemetry on, where the CLI runs and how it was installed, -/// which the page records for a sign-in that never gets to make an API call. +/// key, and, with telemetry on, where the CLI runs, how it was installed, +/// which coding agent ran it and how this project's last sign-in ended. The +/// page records them for a sign-in that never gets to make an API call. pub(super) fn authorize_params( platform: Option<((&'static str, Option<&'static str>), &'static str)>, -) -> Vec<(&'static str, &'static str)> { - let mut params = vec![("key_handoff", "clipboard")]; + agent: Option<&'static str>, + previous: Option<&LastSignIn>, +) -> Vec<(&'static str, String)> { + let mut params = vec![("key_handoff", "clipboard".to_string())]; if let Some(((os, env), install)) = platform { - params.push(("cli_os", os)); + params.push(("cli_os", os.into())); if let Some(env) = env { - params.push(("cli_env", env)); + params.push(("cli_env", env.into())); + } + params.push(("cli_install", install.into())); + } + if let Some(agent) = agent { + params.push(("cli_agent", agent.into())); + } + if let Some(previous) = previous { + params.push(("cli_prev", previous.outcome.into())); + if let Some(id) = &previous.id { + params.push(("cli_prev_id", id.clone())); } - params.push(("cli_install", install)); } params } diff --git a/cli/elevenlabs/workflow/onboard/connect.rs b/cli/elevenlabs/workflow/onboard/connect.rs index e0281bf..ad41d37 100644 --- a/cli/elevenlabs/workflow/onboard/connect.rs +++ b/cli/elevenlabs/workflow/onboard/connect.rs @@ -17,7 +17,7 @@ use super::env_file::{self, API_KEY_ENV}; use super::manual::{self, NO_BROWSER}; use super::project::{last4, shell_arg, KeySource, Project}; use super::reply::{command, Next, Reply}; -use super::session::{Session, LAUNCH_FLAG, SIGN_IN_MINUTES, WORKER_FLAG}; +use super::session::{LastSignIn, Session, LAUNCH_FLAG, SIGN_IN_MINUTES, WORKER_FLAG}; /// How long one `connect` waits for the approval before handing back to the /// agent; well under the time agents let a command run. @@ -46,9 +46,15 @@ pub(super) fn handle( for key in super::added_since_startup() { std::env::remove_var(key); } - let _scope = super::super::api::command_scope("onboard.connect"); - let reply = sign_in_and_store(matches, ctx, &project, || session.signed_in(&lock)) - .unwrap_or_else(|e| failed(&project, &e.to_string(), replace)); + let previous = session.previous(); + let _scope = super::super::api::command_scope_with( + "onboard.connect", + super::sign_in_tag(previous.as_ref()), + ); + let reply = sign_in_and_store(matches, ctx, &project, previous.as_ref(), || { + session.signed_in(&lock) + }) + .unwrap_or_else(|e| failed(&project, &e.to_string(), replace)); session.finish(&lock, &reply.to_value())?; return Ok(None); } @@ -241,10 +247,11 @@ fn sign_in_and_store( matches: &clap::ArgMatches, ctx: &AppContext, project: &Project, + previous: Option<&LastSignIn>, on_signed_in: impl FnOnce(), ) -> Result { let client = ctx.http_config().build_client()?; - let token = match auth::sign_in(ctx, &authorize_url(matches)?, project.no_browser) { + let token = match auth::sign_in(ctx, &authorize_url(matches)?, project.no_browser, previous) { Ok(token) => token, Err(error) => { // The page may have copied a key before the callback failed; nothing @@ -369,5 +376,35 @@ pub(super) fn failed(project: &Project, error: &str, replace: bool) -> Reply { ), ) }; - reply.run(&retry).detail("key_status", "sign_in_failed") + reply + .run(&retry) + .detail("key_status", "sign_in_failed") + .detail("signin", sign_in_failure(error)) +} + +/// What ended a sign-in before the clipboard step, as the label the next +/// sign-in reports (see `Session::last_sign_in`). +pub(super) fn sign_in_failure(error: &str) -> &'static str { + if error.contains("returned error: access_denied") { + return "declined"; + } + if error.contains("Timed out waiting") { + return "timed-out"; + } + if error.contains("Could not listen for the browser") { + return "port-busy"; + } + // `loopback::exchange` names the failure in brackets. + let kind = error + .split_once("PKCE token exchange failed [") + .and_then(|(_, rest)| rest.split_once(']')) + .map(|(kind, _)| kind); + match kind { + Some("connect") => "exchange-connect", + Some("timeout") => "exchange-timeout", + Some("refused") => "exchange-refused", + Some("server") => "exchange-server", + Some(_) => "exchange-other", + None => "sign-in-error", + } } diff --git a/cli/elevenlabs/workflow/onboard/mod.rs b/cli/elevenlabs/workflow/onboard/mod.rs index 72af8d0..32b9197 100644 --- a/cli/elevenlabs/workflow/onboard/mod.rs +++ b/cli/elevenlabs/workflow/onboard/mod.rs @@ -37,12 +37,17 @@ fn dispatch(matches: &clap::ArgMatches, ctx: &AppContext) -> Result<(), CliError let (name, sub) = matches .subcommand() .ok_or_else(|| CliError::Validation("Start with `elevenlabs onboard init`.".into()))?; - let _scope = api::command_scope(match name { - "init" => "onboard.init", - "connect" => "onboard.connect", - "check" => "onboard.check", - _ => "onboard.test", - }); + // The worker reports the sign-in before its own (see `connect::handle`). + let last = (name != "connect").then(last_sign_in).flatten(); + let _scope = api::command_scope_with( + match name { + "init" => "onboard.init", + "connect" => "onboard.connect", + "check" => "onboard.check", + _ => "onboard.test", + }, + sign_in_tag(last.as_ref()), + ); let reply = match name { "init" => Some(init::handle(sub, ctx)?.to_value()), "connect" => connect::handle(sub, ctx)?, @@ -55,6 +60,20 @@ fn dispatch(matches: &clap::ArgMatches, ctx: &AppContext) -> Result<(), CliError } } +/// `signin/` with telemetry on, so a request after a failed sign-in +/// says how it ended, even when the developer then adds a key by hand. +fn sign_in_tag(sign_in: Option<&session::LastSignIn>) -> Option { + sign_in + .filter(|_| super::telemetry::enabled()) + .map(|sign_in| format!("signin/{}", sign_in.outcome)) +} + +/// How the sign-in for the project in the working folder last ended. +fn last_sign_in() -> Option { + let dir = std::env::current_dir().ok()?; + session::Session::for_project(&dir).ok()?.last_sign_in() +} + /// `--env-path`, not `--env-file`: through npx, Node takes `--env-file` for itself (for any /// package), so the CLI never sees it and Node fails when the file doesn't exist yet. The old /// name still works when the CLI is run directly. diff --git a/cli/elevenlabs/workflow/onboard/session.rs b/cli/elevenlabs/workflow/onboard/session.rs index 1e596c6..f8e7827 100644 --- a/cli/elevenlabs/workflow/onboard/session.rs +++ b/cli/elevenlabs/workflow/onboard/session.rs @@ -42,6 +42,78 @@ const POLL: Duration = Duration::from_millis(250); pub(super) const WORKER_FLAG: &str = "worker"; pub(super) const LAUNCH_FLAG: &str = "launch-worker"; +/// How long a finished sign-in's outcome is still reported. +const REPORT_FOR: Duration = Duration::from_secs(24 * 60 * 60); +/// Where [`Session::start`] keeps how the sign-in before the new one ended. +const PREVIOUS: &str = "previous.json"; + +/// Every label a sign-in's outcome can have (see [`outcome_label`]): fixed +/// words, so nothing the developer typed or the machine holds is reported. +pub(super) const OUTCOMES: &[&str] = &[ + "created", + "not-permitted", + "not-available", + "page-failed", + "wrong-account", + "unverified", + "no-key", + "clipboard-error", + "replaced", + "restarted", + "stopped", + "stopped-after", + "declined", + "timed-out", + "port-busy", + "exchange-connect", + "exchange-timeout", + "exchange-refused", + "exchange-server", + "exchange-other", + "sign-in-error", + "other", +]; + +/// How a sign-in ended: one of [`OUTCOMES`], and a hash of its `state` (see +/// [`state_id`]) so the approval page's record of the same sign-in can be found. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct LastSignIn { + pub outcome: &'static str, + pub id: Option, +} + +/// The label for a worker's reply: its `key_status`, and for a sign-in that +/// failed before the clipboard step, its `signin` detail (see `connect::failed`). +pub(super) fn outcome_label(key_status: Option<&str>, signin: Option<&str>) -> &'static str { + match key_status.unwrap_or_default() { + "created" => "created", + "not_permitted" => "not-permitted", + "not_available" => "not-available", + "failed" => "page-failed", + "clipboard_invalid" => "wrong-account", + "could_not_verify" => "unverified", + "no_key" => "no-key", + "clipboard_unreadable" => "clipboard-error", + "sign_in_replaced" => "replaced", + "sign_in_failed" => signin.and_then(known_outcome).unwrap_or("sign-in-error"), + _ => "other", + } +} + +fn known_outcome(label: &str) -> Option<&'static str> { + OUTCOMES.iter().copied().find(|known| *known == label) +} + +/// A sign-in's id: the start of the SHA-256 of its `state`, in hex. The +/// approval page computes the same from its link; the state never leaves. +pub(super) fn state_id(state: &str) -> String { + Sha256::digest(state.as_bytes()) + .iter() + .take(8) + .map(|b| format!("{b:02x}")) + .collect() +} + pub(super) struct Session { dir: PathBuf, } @@ -115,13 +187,16 @@ impl Session { let claim_path = root.join("claim.lock"); let claim = open_lock(&claim_path)?; claim.lock().map_err(io(&claim_path))?; + let mut restarted = false; if self.running() { let same = std::fs::read_to_string(self.path("request")).is_ok_and(|r| r == request); if same || self.path("signed-in").exists() { return Ok(()); } self.stop(); + restarted = true; } + self.keep_last_sign_in(restarted); for stale in ["result.json", "worker.log", "worker.pid", "signed-in"] { let _ = std::fs::remove_file(self.path(stale)); } @@ -245,6 +320,78 @@ impl Session { } } + /// How this project's last sign-in ended, if it ended in the last day. + /// `None` while one is running, or when there was none. + pub fn last_sign_in(&self) -> Option { + if self.running() { + return None; + } + let outcome = match self.result() { + Some(result) if recent(&self.path("result.json")) => outcome_label( + result["details"]["key_status"].as_str(), + result["details"]["signin"].as_str(), + ), + Some(_) => return None, + // A worker that took its lock but left no reply was stopped: by + // the agent, the OS or a crash, before or after the browser step. + None if recent(&self.path("worker.pid")) => { + if self.path("signed-in").exists() { + "stopped-after" + } else { + "stopped" + } + } + None => return None, + }; + Some(LastSignIn { + outcome, + id: self.sign_in_id(), + }) + } + + /// The sign-in before the current one, as [`Self::start`] kept it. + pub fn previous(&self) -> Option { + let text = std::fs::read_to_string(self.path(PREVIOUS)).ok()?; + let kept: Value = serde_json::from_str(&text).ok()?; + Some(LastSignIn { + outcome: kept["outcome"].as_str().and_then(known_outcome)?, + id: kept["id"] + .as_str() + .filter(|id| id.len() == 16 && id.bytes().all(|b| b.is_ascii_hexdigit())) + .map(str::to_string), + }) + } + + /// Before a new sign-in clears this one's files, keep how it ended for + /// the new one to report. One stopped here for another request left no reply. + pub(super) fn keep_last_sign_in(&self, restarted: bool) { + let last = if restarted { + Some(LastSignIn { + outcome: "restarted", + id: self.sign_in_id(), + }) + } else { + self.last_sign_in() + }; + let path = self.path(PREVIOUS); + match last { + Some(last) => { + let kept = serde_json::json!({ "outcome": last.outcome, "id": last.id }); + let _ = env_file::write(&path, &kept.to_string()); + } + None => { + let _ = std::fs::remove_file(&path); + } + } + } + + /// [`state_id`] of the sign-in whose approval link the worker logged. + fn sign_in_id(&self) -> Option { + let url = reqwest::Url::parse(&self.url()?).ok()?; + let (_, state) = url.query_pairs().find(|(name, _)| name == "state")?; + Some(state_id(&state)) + } + /// The approval link the worker printed (the sign-in flow logs ` URL: ` before it waits). pub fn url(&self) -> Option { let log = std::fs::read_to_string(self.path("worker.log")).ok()?; @@ -255,6 +402,15 @@ impl Session { } } +/// Written less than [`REPORT_FOR`] ago. +fn recent(path: &Path) -> bool { + std::fs::metadata(path) + .and_then(|meta| meta.modified()) + .ok() + .and_then(|at| at.elapsed().ok()) + .is_some_and(|age| age < REPORT_FOR) +} + fn open_lock(path: &Path) -> Result { OpenOptions::new() .create(true) diff --git a/cli/elevenlabs/workflow/onboard/tests.rs b/cli/elevenlabs/workflow/onboard/tests.rs index 1baee85..73356e6 100644 --- a/cli/elevenlabs/workflow/onboard/tests.rs +++ b/cli/elevenlabs/workflow/onboard/tests.rs @@ -268,7 +268,10 @@ fn every_step_parses_its_flags_and_bare_onboard_is_refused() { assert_eq!(opt_string(sub, "env-file").as_deref(), Some(".env.dev")); assert!(sub.get_flag("no-browser")); let m = parse(&["onboard", "check", "--env-path", "config/.env"]).unwrap(); - assert_eq!(opt_string(m.subcommand().unwrap().1, "env-file").as_deref(), Some("config/.env")); + assert_eq!( + opt_string(m.subcommand().unwrap().1, "env-file").as_deref(), + Some("config/.env") + ); assert!( parse(&["onboard", "connect", "--product", "music"]).is_err(), "one key for every product, so no product to name" @@ -355,7 +358,10 @@ fn loading_the_key_is_an_instruction_for_the_app_at_hand() { assert_eq!(pkg(r#""next":"15""#), "Next.js reads it on its own."); // Vite and what's built on it don't put env files in `process.env`, where the SDK looks. let vite = pkg(r#""vite":"7""#); - assert!(vite.contains("`loadEnv(mode, process.cwd(), \"\")` in `vite.config`"), "{vite}"); + assert!( + vite.contains("`loadEnv(mode, process.cwd(), \"\")` in `vite.config`"), + "{vite}" + ); assert!(pkg(r#""astro":"5""#).contains("`apiKey: import.meta.env.ELEVENLABS_API_KEY`")); assert!(pkg(r#""@sveltejs/kit":"2""#).contains("`$env/static/private`")); let empty = tempfile::tempdir().unwrap(); @@ -373,41 +379,77 @@ fn check_knows_whether_the_app_loads_the_key_file() { p.loads_key_file(&p.env_file) }; write("package.json", r#"{"scripts":{"start":"node index.js"}}"#); - write("index.js", "import { ElevenLabsClient } from \"@elevenlabs/elevenlabs-js\";\n"); + write( + "index.js", + "import { ElevenLabsClient } from \"@elevenlabs/elevenlabs-js\";\n", + ); assert!(!loads(), "plain Node with nothing loading .env.local"); - write("package.json", r#"{"scripts":{"start":"node --env-file=.env.local index.js"}}"#); + write( + "package.json", + r#"{"scripts":{"start":"node --env-file=.env.local index.js"}}"#, + ); assert!(loads(), "the start script loads it"); - write("package.json", r#"{"scripts":{"start":"node --env-file=\".env.local\" index.js"}}"#); + write( + "package.json", + r#"{"scripts":{"start":"node --env-file=\".env.local\" index.js"}}"#, + ); assert!(loads(), "quoted"); - write("package.json", r#"{"scripts":{"start":"node --env-file=.env index.js"}}"#); + write( + "package.json", + r#"{"scripts":{"start":"node --env-file=.env index.js"}}"#, + ); assert!(!loads(), "another file"); // A monorepo package names the root's key file by its own relative path. std::fs::create_dir_all(dir.path().join("apps/api")).unwrap(); - write("package.json", r#"{"private":true,"workspaces":["apps/*"]}"#); - write("apps/api/package.json", r#"{"scripts":{"start":"node --env-file=../../.env.local server.js"}}"#); + write( + "package.json", + r#"{"private":true,"workspaces":["apps/*"]}"#, + ); + write( + "apps/api/package.json", + r#"{"scripts":{"start":"node --env-file=../../.env.local server.js"}}"#, + ); assert!(loads(), "from a package in a subfolder"); - write("apps/api/package.json", r#"{"scripts":{"start":"node --env-file=..\\..\\.env.local server.js"}}"#); + write( + "apps/api/package.json", + r#"{"scripts":{"start":"node --env-file=..\\..\\.env.local server.js"}}"#, + ); assert!(loads(), "a Windows path"); - write("apps/api/package.json", r#"{"scripts":{"start":"node --env-file=../../.env.local.example server.js"}}"#); + write( + "apps/api/package.json", + r#"{"scripts":{"start":"node --env-file=../../.env.local.example server.js"}}"#, + ); assert!(!loads(), "a file with a longer name"); std::fs::remove_dir_all(dir.path().join("apps")).unwrap(); write("package.json", r#"{"scripts":{"start":"node index.js"}}"#); write("index.js", "import \"dotenv/config\";\n"); assert!(!loads(), "dotenv with no path reads .env, not .env.local"); - write("index.js", "require(\"dotenv\").config({ path: \".env.local\" });\n"); + write( + "index.js", + "require(\"dotenv\").config({ path: \".env.local\" });\n", + ); assert!(loads(), "dotenv given the file"); write("index.js", ""); - write("package.json", r#"{"scripts":{"start":"env-cmd -f .env.local node index.js"}}"#); + write( + "package.json", + r#"{"scripts":{"start":"env-cmd -f .env.local node index.js"}}"#, + ); assert!(loads(), "env-cmd given the file"); write("package.json", r#"{"scripts":{"start":"sh start.sh"}}"#); - write("start.sh", "set -a; source .env.local; set +a\nnode index.js\n"); + write( + "start.sh", + "set -a; source .env.local; set +a\nnode index.js\n", + ); assert!(loads(), "a start script that sources it"); std::fs::remove_file(dir.path().join("start.sh")).unwrap(); assert!(!loads()); write("bun.lock", ""); assert!(loads(), "Bun reads it on its own"); std::fs::remove_file(dir.path().join("bun.lock")).unwrap(); - write("vite.config.ts", "const env = loadEnv(mode, process.cwd(), \"\");\n"); + write( + "vite.config.ts", + "const env = loadEnv(mode, process.cwd(), \"\");\n", + ); assert!(loads(), "Vite's loadEnv"); std::fs::remove_file(dir.path().join("vite.config.ts")).unwrap(); write("package.json", r#"{"dependencies":{"next":"15"}}"#); @@ -415,10 +457,18 @@ fn check_knows_whether_the_app_loads_the_key_file() { let py = tempfile::tempdir().unwrap(); std::fs::write(py.path().join("requirements.txt"), "elevenlabs\n").unwrap(); - std::fs::write(py.path().join("app.py"), "from elevenlabs.client import ElevenLabs\n").unwrap(); + std::fs::write( + py.path().join("app.py"), + "from elevenlabs.client import ElevenLabs\n", + ) + .unwrap(); let p = project(py.path()); assert!(!p.loads_key_file(&p.env_file), "nothing loads .env"); - std::fs::write(py.path().join("app.py"), "from dotenv import load_dotenv\nload_dotenv()\n").unwrap(); + std::fs::write( + py.path().join("app.py"), + "from dotenv import load_dotenv\nload_dotenv()\n", + ) + .unwrap(); assert!(p.loads_key_file(&p.env_file), "python-dotenv"); } @@ -853,9 +903,15 @@ fn the_question_and_the_code_name_no_product() { ); // Right after the answer: the download first, unless they only want the SDK. let code = code["instruction"].as_str().unwrap(); - assert!(code.contains("Otherwise first run `details.fetch_skills`"), "{code}"); + assert!( + code.contains("Otherwise first run `details.fetch_skills`"), + "{code}" + ); // Outside the project, where some agents' file search doesn't look. - assert!(code.contains("with shell commands like `ls` and `cat`"), "{code}"); + assert!( + code.contains("with shell commands like `ls` and `cat`"), + "{code}" + ); } #[test] @@ -1222,28 +1278,209 @@ async fn a_clipboard_key_is_checked_with_the_token_the_sign_in_returned() { assert!(key_matches_sign_in(&client, &server.uri(), "other", KEY).is_err()); } +fn pairs(expected: &[(&'static str, &str)]) -> Vec<(&'static str, String)> { + expected.iter().map(|(k, v)| (*k, v.to_string())).collect() +} + #[test] fn the_approval_link_asks_for_a_key_and_says_where_the_cli_runs() { assert_eq!( - authorize_params(Some((("linux", Some("wsl")), "npx"))), - [ + authorize_params(Some((("linux", Some("wsl")), "npx")), None, None), + pairs(&[ ("key_handoff", "clipboard"), ("cli_os", "linux"), ("cli_env", "wsl"), ("cli_install", "npx") - ] + ]) ); assert_eq!( - authorize_params(Some((("windows", None), "scoop"))), - [ + authorize_params(Some((("windows", None), "scoop")), None, None), + pairs(&[ ("key_handoff", "clipboard"), ("cli_os", "windows"), ("cli_install", "scoop") - ] + ]) + ); +} + +#[test] +fn the_approval_link_says_which_agent_ran_it_and_how_the_last_sign_in_ended() { + use super::session::LastSignIn; + let last = LastSignIn { + outcome: "timed-out", + id: Some("ba7816bf8f01cfea".into()), + }; + assert_eq!( + authorize_params( + Some((("macos", None), "npx")), + Some("claude-code"), + Some(&last) + ), + pairs(&[ + ("key_handoff", "clipboard"), + ("cli_os", "macos"), + ("cli_install", "npx"), + ("cli_agent", "claude-code"), + ("cli_prev", "timed-out"), + ("cli_prev_id", "ba7816bf8f01cfea") + ]) + ); + let unknown_link = LastSignIn { + outcome: "stopped", + id: None, + }; + assert_eq!( + authorize_params(None, None, Some(&unknown_link)), + pairs(&[("key_handoff", "clipboard"), ("cli_prev", "stopped")]) ); } #[test] fn with_telemetry_off_the_approval_link_only_asks_for_a_key() { - assert_eq!(authorize_params(None), [("key_handoff", "clipboard")]); + assert_eq!( + authorize_params(None, None, None), + pairs(&[("key_handoff", "clipboard")]) + ); +} + +#[test] +fn a_failed_sign_in_is_named_by_what_ended_it() { + use super::connect::sign_in_failure; + let cases = [ + ( + "Authorization server returned error: access_denied (declined)", + "declined", + ), + ("Timed out waiting for the browser after 300s", "timed-out"), + ( + "Could not listen for the browser on any sign-in port (8484: in use)", + "port-busy", + ), + ( + "PKCE token exchange failed [connect]: error sending request for url", + "exchange-connect", + ), + ( + "PKCE token exchange failed [timeout]: operation timed out", + "exchange-timeout", + ), + ( + "PKCE token exchange failed [refused]: HTTP 400 Bad Request: Invalid authorization code", + "exchange-refused", + ), + ( + "PKCE token exchange failed [server]: HTTP 502 Bad Gateway: ", + "exchange-server", + ), + ( + "PKCE token exchange failed [response]: token response: missing field", + "exchange-other", + ), + ("something else entirely", "sign-in-error"), + ]; + for (error, want) in cases { + assert_eq!(sign_in_failure(error), want, "{error}"); + } +} + +#[test] +fn a_reply_is_reported_by_a_fixed_label() { + use super::session::{outcome_label, OUTCOMES}; + let cases = [ + (Some("created"), None, "created"), + (Some("no_key"), None, "no-key"), + (Some("clipboard_unreadable"), None, "clipboard-error"), + (Some("clipboard_invalid"), None, "wrong-account"), + (Some("could_not_verify"), None, "unverified"), + (Some("failed"), None, "page-failed"), + (Some("sign_in_replaced"), None, "replaced"), + (Some("sign_in_failed"), Some("timed-out"), "timed-out"), + // Only known labels pass through. + ( + Some("sign_in_failed"), + Some("C:\\Users\\me"), + "sign-in-error", + ), + (Some("sign_in_failed"), None, "sign-in-error"), + (Some("something_new"), None, "other"), + (None, None, "other"), + ]; + for (status, signin, want) in cases { + assert_eq!(outcome_label(status, signin), want, "{status:?} {signin:?}"); + } + // Every label fits the API's tag pattern, `[a-z-]{1,16}`. + for label in OUTCOMES { + assert!( + (1..=16).contains(&label.len()) + && label.bytes().all(|b| b.is_ascii_lowercase() || b == b'-'), + "{label}" + ); + } +} + +#[test] +fn the_last_sign_in_is_its_reply_or_a_stopped_worker() { + use super::session::{LastSignIn, Session}; + let dir = tempfile::tempdir().unwrap(); + let s = Session::in_dir(dir.path().to_path_buf()); + assert_eq!(s.last_sign_in(), None, "no sign-in yet"); + + // A worker that logged its link and was stopped before replying. + std::fs::write(dir.path().join("worker.pid"), "4242").unwrap(); + std::fs::write( + dir.path().join("worker.log"), + "Opening browser to authenticate…\n URL: https://elevenlabs.io/app/oauth/authorize?client_id=c&state=abc&key_handoff=clipboard\n", + ) + .unwrap(); + let stopped = LastSignIn { + outcome: "stopped", + // The first 8 bytes of SHA-256("abc"). + id: Some("ba7816bf8f01cfea".into()), + }; + assert_eq!(s.last_sign_in(), Some(stopped)); + std::fs::write(dir.path().join("signed-in"), "").unwrap(); + assert_eq!(s.last_sign_in().unwrap().outcome, "stopped-after"); + + // A worker that replied. + std::fs::write( + dir.path().join("result.json"), + r#"{"details":{"key_status":"sign_in_failed","signin":"exchange-refused"}}"#, + ) + .unwrap(); + assert_eq!(s.last_sign_in().unwrap().outcome, "exchange-refused"); + + // One still running reports nothing yet. + let lock = s.take().unwrap().unwrap(); + assert_eq!(s.last_sign_in(), None); + drop(lock); +} + +#[test] +fn a_new_sign_in_keeps_how_the_last_one_ended() { + use super::session::Session; + let dir = tempfile::tempdir().unwrap(); + let s = Session::in_dir(dir.path().to_path_buf()); + std::fs::write( + dir.path().join("result.json"), + r#"{"details":{"key_status":"no_key"}}"#, + ) + .unwrap(); + s.keep_last_sign_in(false); + assert_eq!(s.previous().unwrap().outcome, "no-key"); + s.keep_last_sign_in(true); + assert_eq!( + s.previous().unwrap().outcome, + "restarted", + "one this command stopped for another request left no reply" + ); + std::fs::remove_file(dir.path().join("result.json")).unwrap(); + s.keep_last_sign_in(false); + assert_eq!(s.previous(), None, "nothing to report, nothing kept"); + // A kept file that was tampered with reports nothing. + std::fs::write( + dir.path().join("previous.json"), + r#"{"outcome":"C:\\Users\\me","id":"not-hex"}"#, + ) + .unwrap(); + assert_eq!(s.previous(), None); } diff --git a/cli/elevenlabs/workflow/platform.rs b/cli/elevenlabs/workflow/platform.rs index 17e9e29..9e12e7a 100644 --- a/cli/elevenlabs/workflow/platform.rs +++ b/cli/elevenlabs/workflow/platform.rs @@ -1,20 +1,56 @@ //! Roughly where the CLI runs: the OS, whether it is over SSH, in a -//! container or in WSL, and how it was installed. The API is told in the -//! User-Agent (see [`super::telemetry`]), so sign-in and request problems -//! can be counted per platform, and npx runs told apart from installs. -//! Nothing else about the machine is sent, and `--telemetry off` sends none -//! of it. +//! container or in WSL, how it was installed, and which coding agent ran it. +//! The API is told in the User-Agent (see [`super::telemetry`]), so sign-in +//! and request problems can be counted per platform and agent, and npx runs +//! told apart from installs. Nothing else about the machine is sent, and +//! `--telemetry off` sends none of it. use std::path::Path; -/// The prefixes of the tags below, so an inherited User-Agent suffix can be -/// cleared of them (see [`super::telemetry`]). -pub(super) const TAG_PREFIXES: &[&str] = &["os/", "env/", "install/"]; +/// The prefixes of the tags below, and of the `signin/` tag `onboard` adds, +/// so an inherited User-Agent suffix can be cleared of them (see +/// [`super::telemetry`]). +pub(super) const TAG_PREFIXES: &[&str] = &["os/", "env/", "install/", "agent/", "signin/"]; -/// `os/`, `env/` when one applies, then `install/`: the -/// User-Agent tags. +/// `os/`, `env/` when one applies, `install/`, then +/// `agent/` when a coding agent ran this: the User-Agent tags. pub(super) fn tags() -> String { - tags_for(detect(), install_source()) + let tags = tags_for(detect(), install_source()); + match agent() { + Some(agent) => format!("{tags} agent/{agent}"), + None => tags, + } +} + +/// The coding agent running this command, from a variable it sets for the +/// commands it runs. Only the label leaves the machine, never a value. +pub(super) fn agent() -> Option<&'static str> { + agent_for(&|name| std::env::var(name).ok()) +} + +/// Checked in this order: Cursor and VS Code forks also set `TERM_PROGRAM=vscode`. +pub(super) fn agent_for(var: &dyn Fn(&str) -> Option) -> Option<&'static str> { + let set = |name: &str| var(name).is_some_and(|v| !v.trim().is_empty()); + if set("CLAUDECODE") { + // Claude Code in the desktop app is told apart: its built-in browser + // can't write the clipboard the key travels on. + let desktop = var("CLAUDE_CODE_ENTRYPOINT").as_deref() == Some("claude-desktop"); + Some(if desktop { + "claude-desktop" + } else { + "claude-code" + }) + } else if set("GEMINI_CLI") { + Some("gemini-cli") + } else if set("CODEX_SANDBOX") || set("CODEX_SANDBOX_NETWORK_DISABLED") { + Some("codex") + } else if set("CURSOR_AGENT") || set("CURSOR_TRACE_ID") { + Some("cursor") + } else if var("TERM_PROGRAM").as_deref() == Some("vscode") { + Some("vscode") + } else { + None + } } pub(super) fn tags_for((os, env): (&str, Option<&str>), install: &str) -> String { @@ -114,7 +150,7 @@ pub(super) fn install_source_for(path: &str) -> &'static str { #[cfg(test)] mod tests { - use super::{detect_for, install_source_for, tags_for}; + use super::{agent_for, detect_for, install_source_for, tags_for}; fn vars(set: &'static [&'static str]) -> impl Fn(&str) -> Option { move |name| set.contains(&name).then(|| "1".to_string()) @@ -211,4 +247,42 @@ mod tests { "os/linux env/ssh install/homebrew" ); } + + #[test] + fn the_coding_agent_from_the_variables_it_sets() { + type Vars = &'static [(&'static str, &'static str)]; + let with = |set: Vars| { + move |name: &str| { + set.iter() + .find(|(k, _)| *k == name) + .map(|(_, v)| v.to_string()) + } + }; + let cases: [(Vars, Option<&str>); 9] = [ + (&[("CLAUDECODE", "1")], Some("claude-code")), + ( + &[("CLAUDECODE", "1"), ("CLAUDE_CODE_ENTRYPOINT", "cli")], + Some("claude-code"), + ), + ( + &[ + ("CLAUDECODE", "1"), + ("CLAUDE_CODE_ENTRYPOINT", "claude-desktop"), + ], + Some("claude-desktop"), + ), + (&[("GEMINI_CLI", "1")], Some("gemini-cli")), + (&[("CODEX_SANDBOX", "seatbelt")], Some("codex")), + ( + &[("CURSOR_TRACE_ID", "abc"), ("TERM_PROGRAM", "vscode")], + Some("cursor"), + ), + (&[("TERM_PROGRAM", "vscode")], Some("vscode")), + (&[("TERM_PROGRAM", "iTerm.app")], None), + (&[("CLAUDECODE", " ")], None), + ]; + for (set, want) in cases { + assert_eq!(agent_for(&with(set)), want, "{set:?}"); + } + } } diff --git a/cli/elevenlabs/workflow/telemetry.rs b/cli/elevenlabs/workflow/telemetry.rs index fcc9fad..1f9e0c0 100644 --- a/cli/elevenlabs/workflow/telemetry.rs +++ b/cli/elevenlabs/workflow/telemetry.rs @@ -1,7 +1,9 @@ -//! Anonymous usage telemetry: where the CLI runs and how it was installed -//! (see [`super::platform`]), carried as User-Agent tags on every request so -//! the API's `server_cli_request` analytics can count, say, npx runs of -//! `onboard` against installed ones. +//! Anonymous usage telemetry: where the CLI runs, how it was installed and +//! which coding agent ran it (see [`super::platform`]), carried as User-Agent +//! tags on every request so the API's `server_cli_request` analytics can +//! count, say, npx runs of `onboard` against installed ones. `onboard` also +//! says how the project's last sign-in ended (`signin/`, a fixed +//! word), so a sign-in that never reached the API can still be explained. //! //! On by default. `--telemetry off` on any command, `ELEVENLABS_TELEMETRY=off`, //! or `DO_NOT_TRACK=1` turns it off, and then neither the User-Agent nor the @@ -118,8 +120,8 @@ fn resolve() { } const TELEMETRY_HELP: &str = "on or off. Whether requests say, anonymously, which OS the CLI \ - runs on and how it was installed (npx, npm, Homebrew, ...). Overrides ELEVENLABS_TELEMETRY; \ - DO_NOT_TRACK=1 also turns it off."; + runs on, how it was installed (npx, npm, Homebrew, ...), which coding agent ran it, and how \ + the last sign-in ended. Overrides ELEVENLABS_TELEMETRY; DO_NOT_TRACK=1 also turns it off."; /// Resolve telemetry and register `--telemetry` so clap accepts it on every /// command. Its value is never sent: `Explicit` with no opt-ins means the