From b8a3f426f6593b52e2e10811971abbb6d6fa8295 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Wed, 26 Aug 2026 02:30:51 +0800 Subject: [PATCH 1/9] Reject environment-only tracing routes (SDK-272) --- src/trace_host.rs | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/trace_host.rs b/src/trace_host.rs index c55e6243..48aec796 100644 --- a/src/trace_host.rs +++ b/src/trace_host.rs @@ -39,6 +39,14 @@ fn session_route(base: &BaseArgs) -> SessionRoute { } } +fn require_saved_trace_profile(profile: Option) -> anyhow::Result { + profile.ok_or_else(|| { + anyhow::anyhow!( + "coding-agent tracing requires a saved Braintrust profile; run `bt login --profile `, then rerun this command with `--profile `" + ) + }) +} + /// Ensure the route carries an organization, the way `resolve_trace_project` /// ensures it carries a project. Tracing has no later opportunity to ask: the /// org is baked into the route before the daemon sees a single event, so a @@ -180,10 +188,7 @@ impl TraceHostServices for BtTraceHost { let token = resolved .api_key .ok_or_else(|| anyhow::anyhow!("selected Braintrust profile has no credential"))?; - let profile = resolved - .profile - .or_else(|| selection.profile.clone()) - .unwrap_or_else(|| "environment".into()); + let profile = require_saved_trace_profile(resolved.profile)?; let expires_at_ms = resolved.is_oauth.then(|| { chrono::Utc::now() .timestamp_millis() @@ -220,3 +225,25 @@ pub fn context(base: BaseArgs) -> TraceHostContext { services: Arc::new(BtTraceHost { base }), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn tracing_rejects_environment_only_auth() { + let error = require_saved_trace_profile(None).unwrap_err(); + assert!(error + .to_string() + .contains("requires a saved Braintrust profile")); + assert!(error.to_string().contains("bt login --profile ")); + } + + #[test] + fn tracing_keeps_the_resolved_saved_profile() { + assert_eq!( + require_saved_trace_profile(Some("test-profile".into())).unwrap(), + "test-profile" + ); + } +} From 7dcdf8841a57b755af103a7a43f840306890b873 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Wed, 26 Aug 2026 02:30:51 +0800 Subject: [PATCH 2/9] Confirm environment API key persistence (SDK-274) --- src/auth.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/auth.rs b/src/auth.rs index 3bf08fad..6c0788a2 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -419,6 +419,10 @@ pub struct LoginArgs { /// Do not try to open a browser automatically #[arg(long)] no_browser: bool, + + /// Persist BRAINTRUST_API_KEY as a saved profile without prompting + #[arg(long, conflicts_with_all = ["oauth", "refresh"])] + save_env_api_key: bool, } #[derive(Debug, Clone, Args)] @@ -1265,6 +1269,8 @@ async fn run_login_set(base: &BaseArgs, args: LoginArgs) -> Result<()> { } } + confirm_environment_api_key_persistence(base, args.save_env_api_key)?; + let interactive = ui::can_prompt(); let api_key = match base.api_key.clone() { @@ -1325,6 +1331,33 @@ async fn run_login_set(base: &BaseArgs, args: LoginArgs) -> Result<()> { ) } +fn confirm_environment_api_key_persistence(base: &BaseArgs, explicitly_allowed: bool) -> Result<()> { + if !environment_api_key_needs_confirmation(base, explicitly_allowed) { + return Ok(()); + } + + let Some(term) = ui::prompt_term() else { + bail!( + "`bt login` would persist BRAINTRUST_API_KEY as a saved profile; pass --save-env-api-key to confirm, or unset BRAINTRUST_API_KEY to choose another login method" + ); + }; + let confirmed = Confirm::new() + .with_prompt("Save BRAINTRUST_API_KEY as a login on this machine?") + .default(false) + .interact_on(&term)?; + if !confirmed { + bail!("login cancelled; BRAINTRUST_API_KEY was not saved"); + } + Ok(()) +} + +fn environment_api_key_needs_confirmation(base: &BaseArgs, explicitly_allowed: bool) -> bool { + matches!( + base.api_key_source, + Some(crate::args::ArgValueSource::EnvVariable) + ) && !explicitly_allowed +} + async fn run_login_oauth(base: &BaseArgs, args: LoginArgs) -> Result<()> { let api_url = base .api_url @@ -3411,6 +3444,17 @@ mod tests { BaseArgs::default() } + #[test] + fn environment_api_keys_require_explicit_persistence_consent() { + let mut base = make_base(); + base.api_key_source = Some(crate::args::ArgValueSource::EnvVariable); + assert!(environment_api_key_needs_confirmation(&base, false)); + assert!(!environment_api_key_needs_confirmation(&base, true)); + + base.api_key_source = Some(crate::args::ArgValueSource::CommandLine); + assert!(!environment_api_key_needs_confirmation(&base, false)); + } + fn auth_config(profile: Option<&str>, org: Option<&str>) -> crate::config::Config { crate::config::Config { profile: profile.map(str::to_string), From b7bfaa3a95a22551e8f5ed67f6a2afd0e65811a4 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Wed, 26 Aug 2026 02:35:15 +0800 Subject: [PATCH 3/9] Improve saved login status output (SDK-270 SDK-275) --- src/auth.rs | 167 ++++++++++++++++++++++++++++++++++++-------------- src/status.rs | 48 ++++++++++++--- 2 files changed, 158 insertions(+), 57 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 6c0788a2..7ef150b8 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1331,7 +1331,10 @@ async fn run_login_set(base: &BaseArgs, args: LoginArgs) -> Result<()> { ) } -fn confirm_environment_api_key_persistence(base: &BaseArgs, explicitly_allowed: bool) -> Result<()> { +fn confirm_environment_api_key_persistence( + base: &BaseArgs, + explicitly_allowed: bool, +) -> Result<()> { if !environment_api_key_needs_confirmation(base, explicitly_allowed) { return Ok(()); } @@ -2081,6 +2084,8 @@ pub struct ProfileVerification { pub user_email: Option, #[serde(skip_serializing_if = "Option::is_none")] pub api_key_hint: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub expires_at: Option, pub status: String, #[serde(skip_serializing_if = "Option::is_none")] pub error: Option, @@ -2117,6 +2122,9 @@ fn build_verification( user_name: jwt_id.as_ref().and_then(|j| j.name.clone()), user_email: jwt_id.as_ref().and_then(|j| j.email.clone()), api_key_hint, + expires_at: (profile.auth_kind == AuthKind::Oauth) + .then_some(profile.oauth_access_expires_at) + .flatten(), status: status_str.to_string(), error, } @@ -2194,38 +2202,96 @@ pub(crate) async fn profile_verifications() -> Result> Ok(verify_all_profiles_from_store(&store).await) } -pub(crate) fn credentials_path() -> Result { +pub(crate) fn profile_metadata_path() -> Result { auth_store_path() } -pub(crate) fn format_verification_line(v: &ProfileVerification) -> String { - let mut parts = vec![v.name.clone(), v.app_url.clone(), v.auth.clone()]; - if let Some(ref api_url) = v.api_url { - parts.push(format!("api: {api_url}")); - } - if let Some(ref org) = v.org { - parts.push(format!("org: {org}")); - } - match v.status.as_str() { - "ok" => { - let id = match (&v.user_name, &v.user_email) { - (Some(name), Some(email)) => Some(format!("{name} ({email})")), - (None, Some(email)) => Some(email.clone()), - _ => v.api_key_hint.clone(), - }; - if let Some(id) = id { - parts.push(id); +pub(crate) fn secret_storage_description() -> Result { + let fallback = secret_store_path()?; + #[cfg(target_os = "macos")] + return Ok(format!( + "macOS Keychain (plaintext fallback: {})", + fallback.display() + )); + #[cfg(target_os = "linux")] + return Ok(format!( + "Secret Service (plaintext fallback: {})", + fallback.display() + )); + #[cfg(not(any(target_os = "macos", target_os = "linux")))] + return Ok(format!("plaintext file: {}", fallback.display())); +} + +pub(crate) fn credential_precedence(base: &BaseArgs) -> String { + if resolve_api_key_override(base).is_some() { + return match base.api_key_source { + Some(crate::args::ArgValueSource::CommandLine) => { + "explicit --api-key overrides saved profiles".into() } - } - "expired" => parts.push("token expired".into()), - "missing" => parts.push("credential missing".into()), - _ => { - if let Some(ref e) = v.error { - parts.push(e.clone()); + Some(crate::args::ArgValueSource::EnvVariable) => { + "BRAINTRUST_API_KEY overrides saved profiles".into() } - } + None => "API key override is active".into(), + }; + } + if let Some(profile) = base + .profile + .as_deref() + .map(str::trim) + .filter(|profile| !profile.is_empty()) + { + return format!("saved profile `{profile}` is selected"); + } + "saved profile selection is automatic".into() +} + +pub(crate) fn format_verification_block(v: &ProfileVerification, selected: bool) -> String { + let identity = match (&v.user_name, &v.user_email, &v.api_key_hint) { + (Some(name), Some(email), _) => Some(format!("{name} <{email}>")), + (None, Some(email), _) => Some(email.clone()), + (_, _, Some(hint)) => Some(hint.clone()), + _ => None, + }; + let status = match v.status.as_str() { + "ok" => "Ready".to_string(), + "expired" => "Needs refresh".to_string(), + "missing" => "Credential missing".to_string(), + _ => v.error.clone().unwrap_or_else(|| "Error".into()), + }; + let mut lines = vec![format!( + "{}{}", + v.name, + if selected { " (selected)" } else { "" } + )]; + lines.push(format!( + " Auth: {}{}", + v.auth, + identity + .as_deref() + .map(|identity| format!(", {identity}")) + .unwrap_or_default() + )); + if let Some(org) = &v.org { + lines.push(format!(" Org: {org}")); + } + lines.push(format!(" App URL: {}", v.app_url)); + if let Some(api_url) = &v.api_url { + lines.push(format!(" API URL: {api_url}")); + } + if let Some(expires_at) = v.expires_at { + let timestamp = chrono::DateTime::::from_timestamp(expires_at as i64, 0) + .map(|value| value.to_rfc3339()) + .unwrap_or_else(|| expires_at.to_string()); + lines.push(format!(" Expires: {timestamp}")); + } + lines.push(format!(" Status: {status}")); + if v.status == "expired" { + lines.push(format!( + " Fix: bt login --refresh --profile {}", + shell_quote_arg(&v.name) + )); } - parts.join(" — ") + lines.join("\n") } async fn fetch_login_orgs(api_key: &str, app_url: &str) -> Result> { @@ -4859,7 +4925,7 @@ mod tests { } #[test] - fn format_verification_line_ok_with_identity() { + fn format_verification_block_ok_with_identity() { let v = ProfileVerification { name: "work".into(), auth: "oauth".into(), @@ -4869,17 +4935,20 @@ mod tests { user_name: Some("Alice".into()), user_email: Some("alice@example.com".into()), api_key_hint: None, + expires_at: Some(1_800_000_000), status: "ok".into(), error: None, }; - assert_eq!( - format_verification_line(&v), - "work — https://app.test.example — oauth — api: https://api.test.example — org: acme — Alice (alice@example.com)" - ); + let block = format_verification_block(&v, true); + assert!(block.contains("work (selected)")); + assert!(block.contains("Auth: oauth, Alice ")); + assert!(block.contains("Org: acme")); + assert!(block.contains("API URL: https://api.test.example")); + assert!(block.contains("Status: Ready")); } #[test] - fn format_verification_line_ok_with_api_key_hint() { + fn format_verification_block_ok_with_api_key_hint() { let v = ProfileVerification { name: "work".into(), auth: "api_key".into(), @@ -4889,17 +4958,18 @@ mod tests { user_name: None, user_email: None, api_key_hint: Some("sk-****zhJwO".into()), + expires_at: None, status: "ok".into(), error: None, }; - assert_eq!( - format_verification_line(&v), - "work — https://app.test.example — api_key — org: acme — sk-****zhJwO" - ); + let block = format_verification_block(&v, false); + assert!(block.contains("Auth: api_key, sk-****zhJwO")); + assert!(block.contains("Org: acme")); + assert!(block.contains("Status: Ready")); } #[test] - fn format_verification_line_expired() { + fn format_verification_block_expired() { let v = ProfileVerification { name: "old".into(), auth: "oauth".into(), @@ -4909,17 +4979,20 @@ mod tests { user_name: None, user_email: None, api_key_hint: None, + expires_at: Some(1_700_000_000), status: "expired".into(), error: None, }; - assert_eq!( - format_verification_line(&v), - "old — https://app.test.example — oauth — token expired" - ); + let block = format_verification_block(&v, true); + assert!(block.contains("old (selected)")); + assert!(block.contains("Auth: oauth")); + assert!(block.contains("Expires: 2023-11-14T22:13:20+00:00")); + assert!(block.contains("Status: Needs refresh")); + assert!(block.contains("bt login --refresh --profile old")); } #[test] - fn format_verification_line_error() { + fn format_verification_block_error() { let v = ProfileVerification { name: "bad".into(), auth: "api_key".into(), @@ -4929,13 +5002,13 @@ mod tests { user_name: None, user_email: None, api_key_hint: None, + expires_at: None, status: "error".into(), error: Some("invalid API key".into()), }; - assert_eq!( - format_verification_line(&v), - "bad — https://app.test.example — api_key — org: corp — invalid API key" - ); + let block = format_verification_block(&v, false); + assert!(block.contains("Org: corp")); + assert!(block.contains("Status: invalid API key")); } #[tokio::test] diff --git a/src/status.rs b/src/status.rs index 79b23952..86966a7e 100644 --- a/src/status.rs +++ b/src/status.rs @@ -141,21 +141,49 @@ pub async fn run(base: BaseArgs, args: StatusArgs) -> Result<()> { } if let Some(profiles) = profiles.as_ref() { + println!("Braintrust CLI status"); + println!("\nActive default context"); + println!(" Organization: {}", org.as_deref().unwrap_or("(unset)")); + println!( + " Project: {}", + project.as_deref().unwrap_or("(unset)") + ); + println!( + " Profile: {}", + profile_info + .as_ref() + .map(|profile| profile.name.as_str()) + .unwrap_or("(none)") + ); + if let Some(profile) = &profile_info { + println!(" Auth: {}", format_auth(profile)); + } + println!( + " Source: {}", + source.as_deref().unwrap_or("automatic") + ); + + println!("\nCredential precedence"); + println!(" {}", auth::credential_precedence(&base)); + + println!("\nSaved login profiles"); if profiles.is_empty() { - eprintln!("No saved profiles. Run `bt login` to create one."); + println!(" No saved profiles. Run `bt login` to create one."); } else { for profile in profiles { - let status = match profile.status.as_str() { - "ok" => crate::ui::CommandStatus::Success, - "expired" => crate::ui::CommandStatus::Warning, - _ => crate::ui::CommandStatus::Error, - }; - crate::ui::print_command_status(status, &auth::format_verification_line(profile)); - } - if let Ok(path) = auth::credentials_path() { - eprintln!("\nCredentials: {}\n", path.display()); + let selected = profile_info + .as_ref() + .is_some_and(|selected| selected.name == profile.name); + println!("\n{}", auth::format_verification_block(profile, selected)); } } + if let Ok(path) = auth::profile_metadata_path() { + println!("\nProfile metadata: {}", path.display()); + } + if let Ok(storage) = auth::secret_storage_description() { + println!("Secret storage: {storage}"); + } + return Ok(()); } if base.verbose { From b0c1e671039fddd62940fa81acfd37467eecdc56 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Wed, 26 Aug 2026 02:37:26 +0800 Subject: [PATCH 4/9] Clarify logout semantics and add remove-all (SDK-276) --- src/auth.rs | 86 ++++++++++++++++++++++++++++++++++++++++++++-------- tests/cli.rs | 68 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 13 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 7ef150b8..10858879 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -427,6 +427,10 @@ pub struct LoginArgs { #[derive(Debug, Clone, Args)] pub struct LogoutArgs { + /// Remove every saved login from this machine + #[arg(long)] + all: bool, + /// Skip confirmation prompt #[arg(long, short = 'f')] force: bool, @@ -1896,17 +1900,7 @@ pub(crate) fn delete_profile(profile_name: &str, force: bool, base_json: bool) - } } - store.profiles.remove(profile_name); - save_auth_store(&store)?; - if let Err(err) = delete_profile_secret(profile_name) { - eprintln!("warning: failed to delete keychain credential for '{profile_name}': {err}"); - } - if let Err(err) = delete_profile_oauth_refresh_token(profile_name) { - eprintln!("warning: failed to delete oauth refresh token for '{profile_name}': {err}"); - } - if let Err(err) = delete_profile_oauth_access_token(profile_name) { - eprintln!("warning: failed to delete oauth access token for '{profile_name}': {err}"); - } + remove_profile_from_store(&mut store, profile_name)?; emit_result( base_json, @@ -1914,13 +1908,30 @@ pub(crate) fn delete_profile(profile_name: &str, force: bool, base_json: bool) - || { ui::print_command_status( ui::CommandStatus::Success, - &format!("Deleted profile '{profile_name}'"), + &format!( + "Removed saved login '{profile_name}' from this machine; the credential was not revoked" + ), ) }, )?; Ok(true) } +fn remove_profile_from_store(store: &mut AuthStore, profile_name: &str) -> Result<()> { + store.profiles.remove(profile_name); + save_auth_store(store)?; + if let Err(err) = delete_profile_secret(profile_name) { + eprintln!("warning: failed to delete keychain credential for '{profile_name}': {err}"); + } + if let Err(err) = delete_profile_oauth_refresh_token(profile_name) { + eprintln!("warning: failed to delete oauth refresh token for '{profile_name}': {err}"); + } + if let Err(err) = delete_profile_oauth_access_token(profile_name) { + eprintln!("warning: failed to delete oauth access token for '{profile_name}': {err}"); + } + Ok(()) +} + pub(crate) fn rename_profile(old_name: &str, new_name: &str, base_json: bool) -> Result<()> { let old_name = old_name.trim(); let new_name = new_name.trim(); @@ -2007,13 +2018,62 @@ pub(crate) fn rename_profile(old_name: &str, new_name: &str, base_json: bool) -> fn run_login_logout(base: BaseArgs, args: LogoutArgs) -> Result<()> { let base_json = base.json; - let store = load_auth_store()?; + let mut store = load_auth_store()?; if store.profiles.is_empty() { return emit_result(base_json, serde_json::json!({ "status": "empty" }), || { println!("No saved profiles.") }); } + if args.all { + if base.login.profile.is_some() { + bail!("--all cannot be combined with --profile"); + } + if !args.force { + let Some(term) = ui::prompt_term() else { + bail!("removing all saved logins requires confirmation; rerun with --force in non-interactive mode"); + }; + let confirmed = Confirm::new() + .with_prompt(format!( + "Remove all {} saved logins from this machine? Credentials will not be revoked.", + store.profiles.len() + )) + .default(false) + .interact_on(&term)?; + if !confirmed { + return emit_result( + base_json, + serde_json::json!({ "status": "cancelled", "results": [] }), + || eprintln!("Cancelled"), + ); + } + } + + let profile_names: Vec = store.profiles.keys().cloned().collect(); + let mut results = Vec::with_capacity(profile_names.len()); + for profile_name in &profile_names { + remove_profile_from_store(&mut store, profile_name)?; + results.push(serde_json::json!({ + "name": profile_name, + "status": "deleted", + "revoked": false, + })); + } + return emit_result( + base_json, + serde_json::json!({ "status": "deleted", "results": results }), + || { + ui::print_command_status( + ui::CommandStatus::Success, + &format!( + "Removed {} saved logins from this machine; credentials were not revoked", + profile_names.len() + ), + ) + }, + ); + } + let profile_name = if let Some(p) = base.login.profile { let p = p.trim().to_string(); if !store.profiles.contains_key(&p) { diff --git a/tests/cli.rs b/tests/cli.rs index cdcbd80d..39b03324 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -333,6 +333,74 @@ fn profiles_delete_removes_metadata_and_credentials() { assert_eq!(config["org"], "test-org"); } +#[cfg(unix)] +#[test] +fn logout_all_removes_every_saved_login_without_revoking_credentials() { + let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); + let fake_bin = tempfile::tempdir().expect("fake bin tempdir"); + write_auth_store( + config_home.path(), + &[ + ("first-profile", "first-org"), + ("second-profile", "second-org"), + ], + ); + write_profile_secrets(config_home.path(), &["first-profile", "second-profile"]); + + let mut cmd = bt_command(); + clear_braintrust_auth_env(&mut cmd); + use_fake_credential_store(&mut cmd, fake_bin.path()); + let output = cmd + .env("HOME", home.path()) + .env("XDG_CONFIG_HOME", config_home.path()) + .args(["logout", "--all", "--force", "--json"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let result: serde_json::Value = + serde_json::from_slice(&output).expect("parse logout JSON output"); + assert_eq!(result["status"], "deleted"); + assert_eq!(result["results"].as_array().unwrap().len(), 2); + assert!(result["results"] + .as_array() + .unwrap() + .iter() + .all(|entry| entry["revoked"] == false)); + + let auth: serde_json::Value = serde_json::from_str( + &fs::read_to_string(config_home.path().join("bt/auth.json")).expect("read auth store"), + ) + .expect("parse auth store"); + assert!(auth["profiles"].as_object().unwrap().is_empty()); + + let secrets: serde_json::Value = serde_json::from_str( + &fs::read_to_string(config_home.path().join("bt/secrets.json")).expect("read secret store"), + ) + .expect("parse secret store"); + assert!(secrets["secrets"].as_object().unwrap().is_empty()); +} + +#[test] +fn logout_all_requires_force_without_a_terminal() { + let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); + write_auth_store(config_home.path(), &[("test-profile", "test-org")]); + + let mut cmd = bt_command(); + clear_braintrust_auth_env(&mut cmd); + cmd.env("HOME", home.path()) + .env("XDG_CONFIG_HOME", config_home.path()) + .args(["logout", "--all"]) + .assert() + .failure() + .stderr(predicate::str::contains( + "rerun with --force in non-interactive mode", + )); +} + #[cfg(unix)] #[test] fn profiles_rename_moves_credentials_and_updates_config() { From b70de838ee89eb61baa3f78d4ea8e418d39d517d Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Wed, 26 Aug 2026 02:42:04 +0800 Subject: [PATCH 5/9] Add effective tracing diagnostics (SDK-273) --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/auth.rs | 27 +++++++ src/trace_host.rs | 176 +++++++++++++++++++++++++++++++++++++++++++++- tests/cli.rs | 86 +++++++++++++++++----- 5 files changed, 273 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f9dab523..93143b52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -594,7 +594,7 @@ dependencies = [ [[package]] name = "bt-daemon" version = "0.1.0" -source = "git+https://github.com/braintrustdata/braintrust-coding-agent-plugins?rev=abd6756ab36776e893c65d2edd96c5968d75e552#abd6756ab36776e893c65d2edd96c5968d75e552" +source = "git+https://github.com/braintrustdata/braintrust-coding-agent-plugins?rev=b4cc5e78b464c6b6cf16cf7fb8b15f3a5e7b72df#b4cc5e78b464c6b6cf16cf7fb8b15f3a5e7b72df" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 18ae65f7..db4e877b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ actix-web = "4.11.0" anyhow = "1.0.89" backoff = { version = "0.4.0", features = ["tokio"] } braintrust-sdk-rust = { git = "https://github.com/braintrustdata/braintrust-sdk-rust", rev = "43ba73edbf5220b57090e049feb094b60a92fcd4" } -bt-daemon = { git = "https://github.com/braintrustdata/braintrust-coding-agent-plugins", rev = "abd6756ab36776e893c65d2edd96c5968d75e552" } +bt-daemon = { git = "https://github.com/braintrustdata/braintrust-coding-agent-plugins", rev = "b4cc5e78b464c6b6cf16cf7fb8b15f3a5e7b72df" } async-trait = "0.1" clap = { version = "4.5.20", features = ["derive", "env"] } crossterm = "0.28.1" diff --git a/src/auth.rs b/src/auth.rs index 10858879..afe2622e 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -2129,6 +2129,33 @@ fn load_credential_for_profile(name: &str, profile: &AuthProfile) -> CredentialL } } +pub(crate) fn diagnose_stored_profile(name: &str) -> Result { + let store = load_auth_store()?; + let profile = store + .profiles + .get(name) + .ok_or_else(|| profile_not_found_err(name, &store))?; + let verification = match load_credential_for_profile(name, profile) { + CredentialLoad::Found(credential) => { + let (identity, hint) = match profile.auth_kind { + AuthKind::Oauth => (Some(decode_jwt_identity(&credential)), None), + AuthKind::ApiKey => (None, profile.api_key_hint.clone()), + }; + build_verification(name, profile, identity, hint, ProfileStatus::Ok) + } + CredentialLoad::Missing => { + build_verification(name, profile, None, None, ProfileStatus::Missing) + } + CredentialLoad::Expired => { + build_verification(name, profile, None, None, ProfileStatus::Expired) + } + CredentialLoad::Error(error) => { + build_verification(name, profile, None, None, ProfileStatus::Error(error)) + } + }; + Ok(verification) +} + #[derive(Debug, Clone, Serialize)] pub struct ProfileVerification { pub name: String, diff --git a/src/trace_host.rs b/src/trace_host.rs index 48aec796..a5b4539c 100644 --- a/src/trace_host.rs +++ b/src/trace_host.rs @@ -10,11 +10,11 @@ use std::sync::Arc; use async_trait::async_trait; use bt_daemon::wire::{AuthSelection, BackendAuth, FlushMode, SessionRoute, TraceDestination}; use bt_daemon::{ - AuthLease, AuthResolveReason, OutputFormat, RouteRequirements, RunHookCommand, + AuthDiagnostic, AuthLease, AuthResolveReason, OutputFormat, RouteRequirements, RunHookCommand, TraceHostContext, TraceHostServices, }; -use crate::args::BaseArgs; +use crate::args::{ArgValueSource, BaseArgs}; #[derive(Clone)] struct BtTraceHost { @@ -47,6 +47,68 @@ fn require_saved_trace_profile(profile: Option) -> anyhow::Result, +) -> AuthDiagnostic { + let expires_at_ms = verification + .expires_at + .and_then(|seconds| i64::try_from(seconds).ok()) + .and_then(|seconds| seconds.checked_mul(1000)); + let (status, error) = match verification.status.as_str() { + "ok" => ("ready", None), + "expired" => ( + "expired", + Some(format!( + "OAuth access token is expired; run `bt login --refresh --profile {}`", + verification.name + )), + ), + "missing" => ( + "error", + Some(format!( + "saved profile credential is missing; rerun `bt login --profile {}`", + verification.name + )), + ), + _ => ( + "error", + Some( + verification + .error + .unwrap_or_else(|| "saved profile is unusable".into()), + ), + ), + }; + AuthDiagnostic { + status: status.into(), + source: source.into(), + kind: Some(verification.auth), + profile: Some(verification.name), + org_name: selected_org.or(verification.org), + expires_at_ms, + error, + } +} + +fn unresolved_auth_diagnostic( + source: &str, + profile: Option, + org_name: Option, + error: impl Into, +) -> AuthDiagnostic { + AuthDiagnostic { + status: "error".into(), + source: source.into(), + kind: None, + profile, + org_name, + expires_at_ms: None, + error: Some(error.into()), + } +} + /// Ensure the route carries an organization, the way `resolve_trace_project` /// ensures it carries a project. Tracing has no later opportunity to ask: the /// org is baked into the route before the daemon sees a single event, so a @@ -206,6 +268,87 @@ impl TraceHostServices for BtTraceHost { expires_at_ms, }) } + + async fn diagnose_auth(&self, selection: &AuthSelection) -> AuthDiagnostic { + let selected_profile = selection + .profile + .clone() + .or_else(|| self.base.profile.clone()); + if let Some(profile) = selected_profile { + if profile == "environment" { + return unresolved_auth_diagnostic( + "legacy_environment_route", + Some(profile), + selection.org_name.clone(), + "profile `environment` is not a saved login; rerun setup with --profile ", + ); + } + return match crate::auth::diagnose_stored_profile(&profile) { + Ok(verification) => profile_auth_diagnostic( + verification, + "saved_profile", + selection.org_name.clone(), + ), + Err(error) => unresolved_auth_diagnostic( + "saved_profile", + Some(profile), + selection.org_name.clone(), + error.to_string(), + ), + }; + } + + if self.base.api_key.is_some() { + let source = match self.base.api_key_source { + Some(ArgValueSource::CommandLine) => "command_line_api_key", + Some(ArgValueSource::EnvVariable) => "environment_api_key", + None => "api_key_override", + }; + return unresolved_auth_diagnostic( + source, + None, + selection.org_name.clone(), + "coding-agent tracing requires a saved Braintrust profile; run `bt login --profile `", + ); + } + + match crate::auth::list_profiles() { + Ok(profiles) if profiles.len() == 1 => { + let profile = &profiles[0].name; + match crate::auth::diagnose_stored_profile(profile) { + Ok(verification) => profile_auth_diagnostic( + verification, + "automatic_saved_profile", + selection.org_name.clone(), + ), + Err(error) => unresolved_auth_diagnostic( + "automatic_saved_profile", + Some(profile.clone()), + selection.org_name.clone(), + error.to_string(), + ), + } + } + Ok(profiles) if profiles.is_empty() => unresolved_auth_diagnostic( + "unresolved", + None, + selection.org_name.clone(), + "no saved Braintrust profile; run `bt login --profile `", + ), + Ok(_) => unresolved_auth_diagnostic( + "unresolved", + None, + selection.org_name.clone(), + "multiple saved profiles exist; pass --profile ", + ), + Err(error) => unresolved_auth_diagnostic( + "saved_profile_store", + None, + selection.org_name.clone(), + error.to_string(), + ), + } + } } pub fn context(base: BaseArgs) -> TraceHostContext { @@ -246,4 +389,33 @@ mod tests { "test-profile" ); } + + #[test] + fn profile_diagnostic_reports_expiry_without_credentials() { + let diagnostic = profile_auth_diagnostic( + crate::auth::ProfileVerification { + name: "work".into(), + auth: "oauth".into(), + app_url: "https://www.braintrust.dev".into(), + api_url: None, + org: Some("acme".into()), + user_name: None, + user_email: None, + api_key_hint: None, + expires_at: Some(1_700_000_000), + status: "expired".into(), + error: None, + }, + "saved_profile", + None, + ); + assert_eq!(diagnostic.status, "expired"); + assert_eq!(diagnostic.kind.as_deref(), Some("oauth")); + assert_eq!(diagnostic.expires_at_ms, Some(1_700_000_000_000)); + assert!(diagnostic + .error + .as_deref() + .unwrap() + .contains("bt login --refresh --profile work")); + } } diff --git a/tests/cli.rs b/tests/cli.rs index 39b03324..283f26dd 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -21,11 +21,14 @@ fn clear_braintrust_auth_env(cmd: &mut Command) { /// Setup, managed run, and import resolve a Braintrust credential and org /// before writing a route, so those tests supply a synthetic one rather than /// depending on whatever auth the ambient environment happens to carry. -fn bt_trace_command() -> Command { +fn bt_trace_command(config_home: &Path, profile: &str, org: &str) -> Command { + write_auth_store(config_home, &[(profile, org)]); + write_profile_secrets(config_home, &[profile]); let mut cmd = bt_command(); clear_braintrust_auth_env(&mut cmd); - cmd.env("BRAINTRUST_API_KEY", "test-api-key") - .env("BRAINTRUST_ORG_NAME", "test-org"); + cmd.env("XDG_CONFIG_HOME", config_home) + .env("BRAINTRUST_PROFILE", profile) + .env("BRAINTRUST_ORG_NAME", org); cmd } @@ -534,7 +537,8 @@ fn trace_help_exposes_user_commands_and_hides_internal_commands() { .args(["trace", "--help"]) .assert() .success() - .stdout(predicate::str::contains("setup")) + .stdout(predicate::str::contains("\n enable")) + .stdout(predicate::str::contains("\n doctor")) .stdout(predicate::str::contains("\n import")) .stdout(predicate::str::contains("\n run")) .stdout(predicate::str::contains("\n daemon").not()) @@ -695,13 +699,15 @@ fn trace_setup_adopts_the_configured_org_without_prompting() { r#"{"installed":[]}"#, ); write_config_org(config_home.path(), "test-org"); + write_auth_store(config_home.path(), &[("test-profile", "test-org")]); + write_profile_secrets(config_home.path(), &["test-profile"]); let mut cmd = bt_command(); clear_braintrust_auth_env(&mut cmd); cmd.env("HOME", home.path()) .env("XDG_CONFIG_HOME", config_home.path()) .env("PATH", bin_dir.path()) - .env("BRAINTRUST_API_KEY", "test-api-key") + .env("BRAINTRUST_PROFILE", "test-profile") .env("AGENT_SETUP_LOG", state_dir.path().join("codex.log")) .env("BT_DAEMON_CONFIG", &config) .args([ @@ -724,6 +730,7 @@ fn trace_setup_adopts_the_configured_org_without_prompting() { #[test] fn trace_run_uses_the_invocation_project_without_changing_setup() { let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); let bin_dir = tempfile::tempdir().expect("bin tempdir"); let state_dir = tempfile::tempdir().expect("state tempdir"); let run_log = state_dir.path().join("run.log"); @@ -731,7 +738,7 @@ fn trace_run_uses_the_invocation_project_without_changing_setup() { let setup_settings = state_dir.path().join("setup-settings.json"); write_run_agent(&bin_dir.path().join("codex")); - bt_trace_command() + bt_trace_command(config_home.path(), "test-profile", "test-org") .env("HOME", home.path()) .env("PATH", bin_dir.path()) .env("AGENT_RUN_LOG", &run_log) @@ -768,6 +775,7 @@ fn trace_run_uses_the_invocation_project_without_changing_setup() { #[test] fn trace_run_opencode_injects_the_npm_plugin_without_changing_global_config() { let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); let bin_dir = tempfile::tempdir().expect("bin tempdir"); let state_dir = tempfile::tempdir().expect("state tempdir"); let run_log = state_dir.path().join("run.log"); @@ -779,7 +787,7 @@ fn trace_run_opencode_injects_the_npm_plugin_without_changing_global_config() { fs::write(&global_config, r#"{"trace_to_braintrust":true}"#).expect("seed global config"); write_run_agent(&bin_dir.path().join("opencode")); - bt_trace_command() + bt_trace_command(config_home.path(), "test-profile", "test-org") .env("HOME", home.path()) .env("OPENCODE_BIN", bin_dir.path().join("opencode")) .env("AGENT_RUN_LOG", &run_log) @@ -821,6 +829,7 @@ fn trace_run_opencode_injects_the_npm_plugin_without_changing_global_config() { #[test] fn trace_run_pi_injects_the_npm_extension_for_only_that_process() { let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); let bin_dir = tempfile::tempdir().expect("bin tempdir"); let state_dir = tempfile::tempdir().expect("state tempdir"); let run_log = state_dir.path().join("run.log"); @@ -831,7 +840,7 @@ fn trace_run_pi_injects_the_npm_extension_for_only_that_process() { fs::write(&global_config, r#"{"trace_to_braintrust":true}"#).expect("seed global config"); write_run_agent(&bin_dir.path().join("pi")); - bt_trace_command() + bt_trace_command(config_home.path(), "test-profile", "test-org") .env("HOME", home.path()) .env("PI_BIN", bin_dir.path().join("pi")) .env("AGENT_RUN_LOG", &run_log) @@ -1005,7 +1014,7 @@ fn trace_setup_codex_installs_plugin_and_preserves_existing_settings() { ) .expect("seed config"); - bt_trace_command() + bt_trace_command(config_home.path(), "test-profile", "test-org") .env("HOME", home.path()) .env("XDG_CONFIG_HOME", config_home.path()) .env("PATH", bin_dir.path()) @@ -1053,13 +1062,14 @@ fn trace_setup_codex_installs_plugin_and_preserves_existing_settings() { #[test] fn trace_setup_claude_installs_plugin_and_writes_selected_project() { let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); let bin_dir = tempfile::tempdir().expect("bin tempdir"); let state_dir = tempfile::tempdir().expect("state tempdir"); let log = state_dir.path().join("claude.log"); let config = state_dir.path().join("config.json"); write_agent_cli(&bin_dir.path().join("claude"), "[]", "[]"); - bt_trace_command() + bt_trace_command(config_home.path(), "test-profile", "test-org") .env("HOME", home.path()) .env("PATH", bin_dir.path()) .env("AGENT_SETUP_LOG", &log) @@ -1098,7 +1108,7 @@ fn trace_setup_opencode_configures_the_npm_plugin_and_selected_route() { write_auth_store(config_home.path(), &[("work", "acme")]); write_profile_secrets(config_home.path(), &["work"]); - bt_trace_command() + bt_trace_command(config_home.path(), "work", "acme") .env("HOME", home.path()) .env("XDG_CONFIG_HOME", config_home.path()) .args([ @@ -1143,7 +1153,7 @@ fn trace_setup_opencode_configures_the_npm_plugin_and_selected_route() { fn trace_setup_honors_global_json() { let home = tempfile::tempdir().expect("home tempdir"); let config_home = tempfile::tempdir().expect("config tempdir"); - let stdout = bt_trace_command() + let stdout = bt_trace_command(config_home.path(), "test-profile", "test-org") .env("HOME", home.path()) .env("XDG_CONFIG_HOME", config_home.path()) .args([ @@ -1161,7 +1171,7 @@ fn trace_setup_honors_global_json() { .clone(); let output: serde_json::Value = serde_json::from_slice(&stdout).expect("trace setup emits JSON"); - assert_eq!(output["command"], "setup"); + assert_eq!(output["command"], "enable"); assert_eq!(output["source"], "opencode"); assert_eq!(output["display_name"], "OpenCode"); assert_eq!(output["restart_required"], true); @@ -1175,16 +1185,60 @@ fn trace_setup_honors_global_json() { ); } +#[cfg(unix)] +#[test] +fn trace_doctor_reports_saved_profile_provenance_without_credentials() { + let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); + let bin_dir = tempfile::tempdir().expect("bin tempdir"); + let log = tempfile::NamedTempFile::new().expect("setup log"); + write_agent_cli( + &bin_dir.path().join("codex"), + r#"{"marketplaces":[]}"#, + r#"{"installed":[]}"#, + ); + + bt_trace_command(config_home.path(), "test-profile", "test-org") + .env("HOME", home.path()) + .env("PATH", bin_dir.path()) + .env("AGENT_SETUP_LOG", log.path()) + .args(["trace", "enable", "codex", "--project", "agent-traces"]) + .assert() + .success(); + + let output = bt_trace_command(config_home.path(), "test-profile", "test-org") + .env("HOME", home.path()) + .args(["trace", "doctor", "codex", "--json"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let doctor: serde_json::Value = + serde_json::from_slice(&output).expect("trace doctor emits JSON"); + assert_eq!(doctor["command"], "doctor"); + assert_eq!(doctor["source"], "codex"); + assert_eq!(doctor["enabled"], true); + assert_eq!(doctor["auth"]["status"], "ready"); + assert_eq!(doctor["auth"]["source"], "saved_profile"); + assert_eq!(doctor["auth"]["kind"], "api_key"); + assert_eq!(doctor["auth"]["profile"], "test-profile"); + assert!(!String::from_utf8(output) + .expect("UTF-8 doctor output") + .contains("test-api-key")); +} + #[cfg(unix)] #[test] fn trace_setup_pi_installs_the_npm_extension_and_selected_route() { let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); let bin_dir = tempfile::tempdir().expect("bin tempdir"); let state_dir = tempfile::tempdir().expect("state tempdir"); let log = state_dir.path().join("pi.log"); write_agent_cli(&bin_dir.path().join("pi"), "{}", "{}"); - bt_trace_command() + bt_trace_command(config_home.path(), "test-profile", "test-org") .env("HOME", home.path()) .env("PATH", bin_dir.path()) .env("AGENT_SETUP_LOG", &log) @@ -1221,7 +1275,7 @@ fn trace_setup_keeps_each_agents_persistent_selection_independent() { r#"{"installed":[]}"#, ); - bt_trace_command() + bt_trace_command(config_home.path(), "test-profile", "test-org") .env("HOME", home.path()) .env("XDG_CONFIG_HOME", config_home.path()) .env("PATH", bin_dir.path()) @@ -1229,7 +1283,7 @@ fn trace_setup_keeps_each_agents_persistent_selection_independent() { .args(["trace", "setup", "codex", "--project", "codex-project"]) .assert() .success(); - bt_trace_command() + bt_trace_command(config_home.path(), "test-profile", "test-org") .env("HOME", home.path()) .env("XDG_CONFIG_HOME", config_home.path()) .args([ From 10fbc44c27ce3fc95aaaa3da63f00051afd4dc50 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Wed, 26 Aug 2026 22:31:30 +0800 Subject: [PATCH 6/9] Limit credential precedence to active overrides (SDK-270) --- src/auth.rs | 16 ++++------------ src/status.rs | 6 ++++-- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index afe2622e..2dba5ed5 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -2309,9 +2309,9 @@ pub(crate) fn secret_storage_description() -> Result { return Ok(format!("plaintext file: {}", fallback.display())); } -pub(crate) fn credential_precedence(base: &BaseArgs) -> String { +pub(crate) fn credential_precedence(base: &BaseArgs) -> Option { if resolve_api_key_override(base).is_some() { - return match base.api_key_source { + return Some(match base.api_key_source { Some(crate::args::ArgValueSource::CommandLine) => { "explicit --api-key overrides saved profiles".into() } @@ -2319,17 +2319,9 @@ pub(crate) fn credential_precedence(base: &BaseArgs) -> String { "BRAINTRUST_API_KEY overrides saved profiles".into() } None => "API key override is active".into(), - }; - } - if let Some(profile) = base - .profile - .as_deref() - .map(str::trim) - .filter(|profile| !profile.is_empty()) - { - return format!("saved profile `{profile}` is selected"); + }); } - "saved profile selection is automatic".into() + None } pub(crate) fn format_verification_block(v: &ProfileVerification, selected: bool) -> String { diff --git a/src/status.rs b/src/status.rs index 86966a7e..a5dea1e3 100644 --- a/src/status.rs +++ b/src/status.rs @@ -163,8 +163,10 @@ pub async fn run(base: BaseArgs, args: StatusArgs) -> Result<()> { source.as_deref().unwrap_or("automatic") ); - println!("\nCredential precedence"); - println!(" {}", auth::credential_precedence(&base)); + if let Some(precedence) = auth::credential_precedence(&base) { + println!("\nCredential precedence"); + println!(" {precedence}"); + } println!("\nSaved login profiles"); if profiles.is_empty() { From 72d201e8f693fcf9b36d2e67ad11df4f80cd3d8f Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Wed, 26 Aug 2026 22:31:30 +0800 Subject: [PATCH 7/9] Preserve invocation environment auth for tracing (SDK-272) --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/trace_host.rs | 187 +++++++++++++++++++++++++++++++++++++++------- tests/cli.rs | 117 ++++++++++++++++++++++++++++- 4 files changed, 275 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 93143b52..c77df401 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -594,7 +594,7 @@ dependencies = [ [[package]] name = "bt-daemon" version = "0.1.0" -source = "git+https://github.com/braintrustdata/braintrust-coding-agent-plugins?rev=b4cc5e78b464c6b6cf16cf7fb8b15f3a5e7b72df#b4cc5e78b464c6b6cf16cf7fb8b15f3a5e7b72df" +source = "git+https://github.com/braintrustdata/braintrust-coding-agent-plugins?rev=f4c9dc0c574ab21bb3999750d592bb4cdced8ff6#f4c9dc0c574ab21bb3999750d592bb4cdced8ff6" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index db4e877b..5f4b448d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ actix-web = "4.11.0" anyhow = "1.0.89" backoff = { version = "0.4.0", features = ["tokio"] } braintrust-sdk-rust = { git = "https://github.com/braintrustdata/braintrust-sdk-rust", rev = "43ba73edbf5220b57090e049feb094b60a92fcd4" } -bt-daemon = { git = "https://github.com/braintrustdata/braintrust-coding-agent-plugins", rev = "b4cc5e78b464c6b6cf16cf7fb8b15f3a5e7b72df" } +bt-daemon = { git = "https://github.com/braintrustdata/braintrust-coding-agent-plugins", rev = "f4c9dc0c574ab21bb3999750d592bb4cdced8ff6" } async-trait = "0.1" clap = { version = "4.5.20", features = ["derive", "env"] } crossterm = "0.28.1" diff --git a/src/trace_host.rs b/src/trace_host.rs index a5b4539c..e1ed4ac2 100644 --- a/src/trace_host.rs +++ b/src/trace_host.rs @@ -8,7 +8,9 @@ use std::ffi::OsString; use std::sync::Arc; use async_trait::async_trait; -use bt_daemon::wire::{AuthSelection, BackendAuth, FlushMode, SessionRoute, TraceDestination}; +use bt_daemon::wire::{ + AuthSelection, AuthSource, BackendAuth, FlushMode, SessionRoute, TraceDestination, +}; use bt_daemon::{ AuthDiagnostic, AuthLease, AuthResolveReason, OutputFormat, RouteRequirements, RunHookCommand, TraceHostContext, TraceHostServices, @@ -22,8 +24,21 @@ struct BtTraceHost { } fn session_route(base: &BaseArgs) -> SessionRoute { + let source = if base.profile.is_some() { + AuthSource::SavedProfile + } else if matches!(base.api_key_source, Some(ArgValueSource::EnvVariable)) + && base + .api_key + .as_deref() + .is_some_and(|key| !key.trim().is_empty()) + { + AuthSource::Environment + } else { + AuthSource::Auto + }; SessionRoute { auth: AuthSelection { + source, profile: base.profile.clone(), org_name: base.org_name.clone(), }, @@ -47,6 +62,25 @@ fn require_saved_trace_profile(profile: Option) -> anyhow::Result anyhow::Result { + base.prefer_profile = true; + let resolved = crate::auth::resolve_auth(&base) + .await + .map_err(|error| anyhow::anyhow!("resolve saved auth: {error}"))?; + let profile = require_saved_trace_profile(resolved.profile).map_err(|_| { + anyhow::anyhow!( + "persistent coding-agent tracing requires a saved Braintrust profile because hooks run in future processes; run `bt login --profile --save-env-api-key`, then rerun with `--profile `" + ) + })?; + base.profile = Some(profile); + base.profile_explicit = true; + base.prefer_profile = true; + if base.org_name.is_none() { + base.org_name = resolved.org_name; + } + Ok(base) +} + fn profile_auth_diagnostic( verification: crate::auth::ProfileVerification, source: &str, @@ -215,11 +249,16 @@ impl TraceHostServices for BtTraceHost { if !requirements.interactive_auth { crate::ui::set_no_input(true); } - let mut base = if requirements.destination_required { - resolve_trace_project(self.base.clone()).await? + let base = if requirements.persistent_auth { + resolve_persistent_trace_auth(self.base.clone()).await? } else { self.base.clone() }; + let mut base = if requirements.destination_required { + resolve_trace_project(base).await? + } else { + base + }; // Hooks tolerate an unresolved org — the daemon accepts their events // without one — but setup, managed run, and import bake the org into a // stored route, so they have to settle it now rather than fail later. @@ -236,10 +275,33 @@ impl TraceHostServices for BtTraceHost { ) -> anyhow::Result { let mut base = self.base.clone(); base.no_input = true; - if let Some(profile) = &selection.profile { - base.profile = Some(profile.clone()); - base.profile_explicit = true; - base.prefer_profile = true; + let selection = selection.clone().canonicalized()?; + match selection.source { + AuthSource::SavedProfile => { + let profile = selection + .profile + .as_ref() + .ok_or_else(|| anyhow::anyhow!("saved-profile auth requires a profile name"))?; + base.profile = Some(profile.clone()); + base.profile_explicit = true; + base.prefer_profile = true; + } + AuthSource::Environment => { + if !matches!(base.api_key_source, Some(ArgValueSource::EnvVariable)) + || base + .api_key + .as_deref() + .is_none_or(|key| key.trim().is_empty()) + { + anyhow::bail!( + "this trace route uses environment auth, but BRAINTRUST_API_KEY is not set" + ); + } + base.profile = None; + base.profile_explicit = false; + base.prefer_profile = false; + } + AuthSource::Auto => {} } if let Some(org_name) = &selection.org_name { base.org_name = Some(org_name.clone()); @@ -250,14 +312,30 @@ impl TraceHostServices for BtTraceHost { let token = resolved .api_key .ok_or_else(|| anyhow::anyhow!("selected Braintrust profile has no credential"))?; - let profile = require_saved_trace_profile(resolved.profile)?; + let canonical_selection = if let Some(profile) = resolved.profile { + AuthSelection { + source: AuthSource::SavedProfile, + profile: Some(profile), + org_name: resolved.org_name.clone(), + } + } else if matches!(base.api_key_source, Some(ArgValueSource::EnvVariable)) { + AuthSelection { + source: AuthSource::Environment, + profile: None, + org_name: resolved.org_name.clone(), + } + } else { + anyhow::bail!( + "invocation-local tracing with an API key requires BRAINTRUST_API_KEY; `--api-key` cannot be forwarded safely to the tracing daemon" + ); + }; let expires_at_ms = resolved.is_oauth.then(|| { chrono::Utc::now() .timestamp_millis() .saturating_add(5 * 60 * 1000) }); Ok(AuthLease { - profile, + selection: canonical_selection, auth: BackendAuth { token, api_url: resolved.api_url, @@ -270,19 +348,39 @@ impl TraceHostServices for BtTraceHost { } async fn diagnose_auth(&self, selection: &AuthSelection) -> AuthDiagnostic { - let selected_profile = selection - .profile - .clone() - .or_else(|| self.base.profile.clone()); - if let Some(profile) = selected_profile { - if profile == "environment" { - return unresolved_auth_diagnostic( - "legacy_environment_route", - Some(profile), + if selection.effective_source() == AuthSource::Environment { + return if matches!(self.base.api_key_source, Some(ArgValueSource::EnvVariable)) + && self + .base + .api_key + .as_deref() + .is_some_and(|key| !key.trim().is_empty()) + { + AuthDiagnostic { + status: "ready".into(), + source: "environment".into(), + kind: Some("api_key".into()), + profile: None, + org_name: selection.org_name.clone(), + expires_at_ms: None, + error: None, + } + } else { + unresolved_auth_diagnostic( + "environment", + None, selection.org_name.clone(), - "profile `environment` is not a saved login; rerun setup with --profile ", - ); - } + "BRAINTRUST_API_KEY is not set in the current process", + ) + }; + } + + let selected_profile = selection.profile.clone().or_else(|| { + (selection.effective_source() == AuthSource::Auto) + .then(|| self.base.profile.clone()) + .flatten() + }); + if let Some(profile) = selected_profile { return match crate::auth::diagnose_stored_profile(&profile) { Ok(verification) => profile_auth_diagnostic( verification, @@ -304,12 +402,15 @@ impl TraceHostServices for BtTraceHost { Some(ArgValueSource::EnvVariable) => "environment_api_key", None => "api_key_override", }; - return unresolved_auth_diagnostic( - source, - None, - selection.org_name.clone(), - "coding-agent tracing requires a saved Braintrust profile; run `bt login --profile `", - ); + return AuthDiagnostic { + status: "ready".into(), + source: source.into(), + kind: Some("api_key".into()), + profile: None, + org_name: selection.org_name.clone(), + expires_at_ms: None, + error: None, + }; } match crate::auth::list_profiles() { @@ -372,9 +473,10 @@ pub fn context(base: BaseArgs) -> TraceHostContext { #[cfg(test)] mod tests { use super::*; + use crate::args::LoginBaseArgs; #[test] - fn tracing_rejects_environment_only_auth() { + fn persistent_tracing_rejects_auth_without_a_saved_profile() { let error = require_saved_trace_profile(None).unwrap_err(); assert!(error .to_string() @@ -418,4 +520,33 @@ mod tests { .unwrap() .contains("bt login --refresh --profile work")); } + + #[tokio::test] + async fn invocation_environment_auth_returns_an_environment_lease() { + let base = BaseArgs { + login: LoginBaseArgs { + api_key: Some("synthetic-api-key".into()), + api_key_source: Some(ArgValueSource::EnvVariable), + ..LoginBaseArgs::default() + }, + org_name: Some("test-org".into()), + ..BaseArgs::default() + }; + let host = BtTraceHost { base }; + let lease = host + .resolve_auth( + &AuthSelection { + source: AuthSource::Environment, + profile: None, + org_name: Some("test-org".into()), + }, + AuthResolveReason::Initial, + ) + .await + .unwrap(); + assert_eq!(lease.selection.source, AuthSource::Environment); + assert_eq!(lease.selection.profile, None); + assert_eq!(lease.auth.token, "synthetic-api-key"); + assert_eq!(lease.auth.org_name.as_deref(), Some("test-org")); + } } diff --git a/tests/cli.rs b/tests/cli.rs index 283f26dd..aa2bdbb3 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -32,6 +32,15 @@ fn bt_trace_command(config_home: &Path, profile: &str, org: &str) -> Command { cmd } +fn bt_trace_environment_command(config_home: &Path) -> Command { + let mut cmd = bt_command(); + clear_braintrust_auth_env(&mut cmd); + cmd.env("XDG_CONFIG_HOME", config_home) + .env("BRAINTRUST_API_KEY", "test-api-key") + .env("BRAINTRUST_ORG_NAME", "test-org"); + cmd +} + fn write_executable(path: &Path) { fs::write(path, "#!/bin/sh\nexit 0\n").expect("write executable"); #[cfg(unix)] @@ -69,7 +78,7 @@ esac fn write_run_agent(path: &Path) { fs::write( path, - "#!/bin/sh\nprintf '%s\\n' \"$*\" > \"$AGENT_RUN_LOG\"\nprintf '%s\\n' \"$BT_TRACE_INVOCATION_SETTINGS\" > \"$AGENT_RUN_SETTINGS\"\nif [ -n \"$AGENT_RUN_CONFIG\" ]; then printf '%s\\n' \"$OPENCODE_CONFIG_CONTENT\" > \"$AGENT_RUN_CONFIG\"; fi\n", + "#!/bin/sh\nprintf '%s\\n' \"$*\" > \"$AGENT_RUN_LOG\"\nprintf '%s\\n' \"$BT_TRACE_INVOCATION_SETTINGS\" > \"$AGENT_RUN_SETTINGS\"\nif [ -n \"$AGENT_RUN_DAEMON_ENV\" ]; then printf '%s\\n%s\\n' \"$BT_DAEMON_SOCKET\" \"$BT_DAEMON_DATA_DIR\" > \"$AGENT_RUN_DAEMON_ENV\"; fi\nif [ -n \"$AGENT_RUN_CONFIG\" ]; then printf '%s\\n' \"$OPENCODE_CONFIG_CONTENT\" > \"$AGENT_RUN_CONFIG\"; fi\n", ) .expect("write fake run agent"); use std::os::unix::fs::PermissionsExt; @@ -285,6 +294,54 @@ fn status_verbose_explicitly_shows_unset_profile() { .stdout(predicate::str::contains("profile: (unset)")); } +#[test] +fn bare_status_does_not_render_the_all_profiles_report() { + let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); + let mut cmd = bt_command(); + clear_braintrust_auth_env(&mut cmd); + cmd.env("HOME", home.path()) + .env("XDG_CONFIG_HOME", config_home.path()) + .args(["status"]) + .assert() + .success() + .stdout(predicate::str::contains("Saved login profiles").not()) + .stdout(predicate::str::contains("Credential precedence").not()) + .stdout(predicate::str::contains("Profile metadata").not()) + .stdout(predicate::str::contains("Secret storage").not()); +} + +#[test] +fn status_all_only_shows_precedence_for_an_active_override() { + let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); + + let mut without_override = bt_command(); + clear_braintrust_auth_env(&mut without_override); + without_override + .env("HOME", home.path()) + .env("XDG_CONFIG_HOME", config_home.path()) + .args(["status", "--all"]) + .assert() + .success() + .stdout(predicate::str::contains("Saved login profiles")) + .stdout(predicate::str::contains("Credential precedence").not()); + + let mut with_override = bt_command(); + clear_braintrust_auth_env(&mut with_override); + with_override + .env("HOME", home.path()) + .env("XDG_CONFIG_HOME", config_home.path()) + .env("BRAINTRUST_API_KEY", "synthetic-api-key") + .args(["status", "--all"]) + .assert() + .success() + .stdout(predicate::str::contains("Credential precedence")) + .stdout(predicate::str::contains( + "BRAINTRUST_API_KEY overrides saved profiles", + )); +} + #[cfg(unix)] #[test] fn profiles_delete_removes_metadata_and_credentials() { @@ -627,10 +684,14 @@ fn trace_commands_require_a_project_non_interactively() { ] { let home = tempfile::tempdir().expect("home tempdir"); let config_home = tempfile::tempdir().expect("config tempdir"); + write_auth_store(config_home.path(), &[("test-profile", "test-org")]); + write_profile_secrets(config_home.path(), &["test-profile"]); let mut cmd = bt_command(); clear_braintrust_auth_env(&mut cmd); cmd.env("HOME", home.path()) .env("XDG_CONFIG_HOME", config_home.path()) + .env("BRAINTRUST_PROFILE", "test-profile") + .env("BRAINTRUST_ORG_NAME", "test-org") .args(args) .assert() .failure() @@ -669,9 +730,21 @@ fn trace_commands_require_an_org_when_the_credential_resolves_none() { let config_home = tempfile::tempdir().expect("config tempdir"); let mut cmd = bt_command(); clear_braintrust_auth_env(&mut cmd); + if args[1] == "setup" { + let auth_dir = config_home.path().join("bt"); + fs::create_dir_all(&auth_dir).expect("create auth dir"); + fs::write( + auth_dir.join("auth.json"), + r#"{"profiles":{"test-profile":{"auth_kind":"api_key"}}}"#, + ) + .expect("write unbound profile"); + write_profile_secrets(config_home.path(), &["test-profile"]); + cmd.env("BRAINTRUST_PROFILE", "test-profile"); + } else { + cmd.env("BRAINTRUST_API_KEY", "test-api-key"); + } cmd.env("HOME", home.path()) .env("XDG_CONFIG_HOME", config_home.path()) - .env("BRAINTRUST_API_KEY", "test-api-key") .args(args) .assert() .failure() @@ -735,14 +808,16 @@ fn trace_run_uses_the_invocation_project_without_changing_setup() { let state_dir = tempfile::tempdir().expect("state tempdir"); let run_log = state_dir.path().join("run.log"); let run_settings = state_dir.path().join("run-settings.json"); + let run_daemon_env = state_dir.path().join("run-daemon-env.txt"); let setup_settings = state_dir.path().join("setup-settings.json"); write_run_agent(&bin_dir.path().join("codex")); - bt_trace_command(config_home.path(), "test-profile", "test-org") + bt_trace_environment_command(config_home.path()) .env("HOME", home.path()) .env("PATH", bin_dir.path()) .env("AGENT_RUN_LOG", &run_log) .env("AGENT_RUN_SETTINGS", &run_settings) + .env("AGENT_RUN_DAEMON_ENV", &run_daemon_env) .env("BT_DAEMON_CONFIG", &setup_settings) .args([ "trace", @@ -765,12 +840,48 @@ fn trace_run_uses_the_invocation_project_without_changing_setup() { settings["route"]["destination"]["project_name"], "invocation-project" ); + assert_eq!(settings["route"]["auth"]["source"], "environment"); + assert!(settings["route"]["auth"].get("profile").is_none()); + let daemon_env = fs::read_to_string(run_daemon_env).expect("read managed daemon environment"); + let mut daemon_env = daemon_env.lines(); + let socket = daemon_env.next().expect("managed daemon socket"); + let data_dir = daemon_env.next().expect("managed daemon data directory"); + assert!(socket.contains("bt-trace-run-")); + assert!(data_dir.contains("bt-trace-run-")); assert!( !setup_settings.exists(), "managed run must not change persistent setup settings" ); } +#[cfg(unix)] +#[test] +fn trace_enable_requires_durable_saved_profile_auth() { + let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); + let bin_dir = tempfile::tempdir().expect("bin tempdir"); + let state_dir = tempfile::tempdir().expect("state tempdir"); + write_agent_cli( + &bin_dir.path().join("codex"), + r#"{"marketplaces":[]}"#, + r#"{"installed":[]}"#, + ); + + bt_trace_environment_command(config_home.path()) + .env("HOME", home.path()) + .env("PATH", bin_dir.path()) + .env("AGENT_SETUP_LOG", state_dir.path().join("codex.log")) + .args(["trace", "enable", "codex", "--project", "agent-traces"]) + .assert() + .failure() + .stderr(predicate::str::contains( + "persistent coding-agent tracing requires a saved Braintrust profile", + )) + .stderr(predicate::str::contains( + "bt login --profile --save-env-api-key", + )); +} + #[cfg(unix)] #[test] fn trace_run_opencode_injects_the_npm_plugin_without_changing_global_config() { From 98f1a388915f8a46f49b719f0c46642dd615a638 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Thu, 27 Aug 2026 22:59:17 +0800 Subject: [PATCH 8/9] Pin bt-daemon to landed SDK-288 changes --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c77df401..d9b75ed5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -594,7 +594,7 @@ dependencies = [ [[package]] name = "bt-daemon" version = "0.1.0" -source = "git+https://github.com/braintrustdata/braintrust-coding-agent-plugins?rev=f4c9dc0c574ab21bb3999750d592bb4cdced8ff6#f4c9dc0c574ab21bb3999750d592bb4cdced8ff6" +source = "git+https://github.com/braintrustdata/braintrust-coding-agent-plugins?rev=face71d8d758363e881e15310ce79d5fcafa2912#face71d8d758363e881e15310ce79d5fcafa2912" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 5f4b448d..f8a80e31 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ actix-web = "4.11.0" anyhow = "1.0.89" backoff = { version = "0.4.0", features = ["tokio"] } braintrust-sdk-rust = { git = "https://github.com/braintrustdata/braintrust-sdk-rust", rev = "43ba73edbf5220b57090e049feb094b60a92fcd4" } -bt-daemon = { git = "https://github.com/braintrustdata/braintrust-coding-agent-plugins", rev = "f4c9dc0c574ab21bb3999750d592bb4cdced8ff6" } +bt-daemon = { git = "https://github.com/braintrustdata/braintrust-coding-agent-plugins", rev = "face71d8d758363e881e15310ce79d5fcafa2912" } async-trait = "0.1" clap = { version = "4.5.20", features = ["derive", "env"] } crossterm = "0.28.1" From f4de4c6b962d436449fe5a9f553ce7f29a06e6f5 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Thu, 27 Aug 2026 23:17:04 +0800 Subject: [PATCH 9/9] Complete tracing setup from available credentials --- src/auth.rs | 64 ++++++++++++++++++++++++++++ src/trace_host.rs | 101 ++++++++++++++++++++++++++++--------------- tests/cli.rs | 106 ++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 229 insertions(+), 42 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 2dba5ed5..392ebd9f 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1365,6 +1365,70 @@ fn environment_api_key_needs_confirmation(base: &BaseArgs, explicitly_allowed: b ) && !explicitly_allowed } +/// Ensure persistent coding-agent tracing has a credential it can resolve in +/// future processes. Unlike ordinary login, `trace enable` is itself an +/// explicit request to persist the credential needed by the installed hooks, +/// so an environment API key does not require a second confirmation flag. +pub(crate) async fn ensure_saved_trace_profile(base: &BaseArgs) -> Result { + let api_key = match base.api_key.clone() { + Some(value) if !value.trim().is_empty() => value, + Some(_) => bail!("api key cannot be empty"), + None if ui::can_prompt() => prompt_api_key()?, + None => bail!( + "coding-agent tracing needs a Braintrust credential; set BRAINTRUST_API_KEY, pass --api-key, or run without --no-input to enter one" + ), + }; + + let app_url = base + .app_url + .clone() + .unwrap_or_else(|| DEFAULT_APP_URL.to_string()); + let login_orgs = fetch_login_orgs(&api_key, &app_url).await?; + let org_constraint = single_org_api_key_constraint(&api_key, &login_orgs); + let store = load_auth_store()?; + let explicit_profile = base + .profile + .as_deref() + .map(str::trim) + .filter(|name| !name.is_empty()); + let (mut profile_name, should_confirm_overwrite) = resolve_api_key_login_profile_name( + explicit_profile, + org_constraint.map(|org| org.name.as_str()), + &app_url, + &store, + )?; + + if should_confirm_overwrite { + if ui::can_prompt() { + confirm_profile_overwrite(&profile_name)?; + } else { + // Never overwrite an unrelated profile just to make setup + // non-interactive. Pick an unused deterministic name and return + // it to the route resolver instead. + profile_name = next_available_profile_name(&profile_name, &store); + } + } + + commit_api_key_profile( + &profile_name, + &api_key, + Some(app_url.clone()), + org_constraint.map(|org| org.name.clone()), + )?; + + Ok(ResolvedAuth { + api_key: Some(api_key), + api_url: base.api_url.clone(), + app_url: Some(app_url), + org_name: base + .org_name + .clone() + .or_else(|| org_constraint.map(|org| org.name.clone())), + is_oauth: false, + profile: Some(profile_name), + }) +} + async fn run_login_oauth(base: &BaseArgs, args: LoginArgs) -> Result<()> { let api_url = base .api_url diff --git a/src/trace_host.rs b/src/trace_host.rs index e1ed4ac2..0241a143 100644 --- a/src/trace_host.rs +++ b/src/trace_host.rs @@ -54,24 +54,29 @@ fn session_route(base: &BaseArgs) -> SessionRoute { } } -fn require_saved_trace_profile(profile: Option) -> anyhow::Result { - profile.ok_or_else(|| { - anyhow::anyhow!( - "coding-agent tracing requires a saved Braintrust profile; run `bt login --profile `, then rerun this command with `--profile `" - ) - }) -} - async fn resolve_persistent_trace_auth(mut base: BaseArgs) -> anyhow::Result { base.prefer_profile = true; - let resolved = crate::auth::resolve_auth(&base) - .await - .map_err(|error| anyhow::anyhow!("resolve saved auth: {error}"))?; - let profile = require_saved_trace_profile(resolved.profile).map_err(|_| { - anyhow::anyhow!( - "persistent coding-agent tracing requires a saved Braintrust profile because hooks run in future processes; run `bt login --profile --save-env-api-key`, then rerun with `--profile `" - ) - })?; + let resolved = match crate::auth::resolve_auth(&base).await { + Ok(resolved) if resolved.profile.is_some() => resolved, + Ok(_) => crate::auth::ensure_saved_trace_profile(&base) + .await + .map_err(|error| anyhow::anyhow!("save tracing login: {error}"))?, + Err(_) + if crate::ui::can_prompt() + || base + .api_key + .as_deref() + .is_some_and(|key| !key.trim().is_empty()) => + { + crate::auth::ensure_saved_trace_profile(&base) + .await + .map_err(|error| anyhow::anyhow!("save tracing login: {error}"))? + } + Err(error) => return Err(anyhow::anyhow!("resolve saved auth: {error}")), + }; + let profile = resolved + .profile + .expect("saved trace profile resolver always returns a profile"); base.profile = Some(profile); base.profile_explicit = true; base.prefer_profile = true; @@ -81,6 +86,46 @@ async fn resolve_persistent_trace_auth(mut base: BaseArgs) -> anyhow::Result anyhow::Result { + match crate::auth::resolve_auth(&base).await { + Ok(resolved) if resolved.api_key.is_some() => { + if let Some(profile) = resolved.profile { + base.profile = Some(profile); + base.profile_explicit = true; + base.prefer_profile = true; + } else if !matches!(base.api_key_source, Some(ArgValueSource::EnvVariable)) { + let resolved = crate::auth::ensure_saved_trace_profile(&base) + .await + .map_err(|error| anyhow::anyhow!("create tracing login: {error}"))?; + base.profile = resolved.profile; + base.profile_explicit = true; + base.prefer_profile = true; + if base.org_name.is_none() { + base.org_name = resolved.org_name; + } + } + if base.org_name.is_none() { + base.org_name = resolved.org_name; + } + Ok(base) + } + Ok(_) | Err(_) if crate::ui::can_prompt() => { + let resolved = crate::auth::ensure_saved_trace_profile(&base) + .await + .map_err(|error| anyhow::anyhow!("create tracing login: {error}"))?; + base.profile = resolved.profile; + base.profile_explicit = true; + base.prefer_profile = true; + if base.org_name.is_none() { + base.org_name = resolved.org_name; + } + Ok(base) + } + Ok(_) => Ok(base), + Err(error) => Err(anyhow::anyhow!("resolve auth: {error}")), + } +} + fn profile_auth_diagnostic( verification: crate::auth::ProfileVerification, source: &str, @@ -251,6 +296,8 @@ impl TraceHostServices for BtTraceHost { } let base = if requirements.persistent_auth { resolve_persistent_trace_auth(self.base.clone()).await? + } else if requirements.interactive_auth { + resolve_invocation_trace_auth(self.base.clone()).await? } else { self.base.clone() }; @@ -285,6 +332,11 @@ impl TraceHostServices for BtTraceHost { base.profile = Some(profile.clone()); base.profile_explicit = true; base.prefer_profile = true; + // The route has already materialized any command-line key as + // this saved profile. Do not let the original CLI override + // displace the selected durable credential on lease renewal. + base.api_key = None; + base.api_key_source = None; } AuthSource::Environment => { if !matches!(base.api_key_source, Some(ArgValueSource::EnvVariable)) @@ -475,23 +527,6 @@ mod tests { use super::*; use crate::args::LoginBaseArgs; - #[test] - fn persistent_tracing_rejects_auth_without_a_saved_profile() { - let error = require_saved_trace_profile(None).unwrap_err(); - assert!(error - .to_string() - .contains("requires a saved Braintrust profile")); - assert!(error.to_string().contains("bt login --profile ")); - } - - #[test] - fn tracing_keeps_the_resolved_saved_profile() { - assert_eq!( - require_saved_trace_profile(Some("test-profile".into())).unwrap(), - "test-profile" - ); - } - #[test] fn profile_diagnostic_reports_expiry_without_credentials() { let diagnostic = profile_auth_diagnostic( diff --git a/tests/cli.rs b/tests/cli.rs index aa2bdbb3..4bcce64d 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1,6 +1,10 @@ use assert_cmd::Command; use predicates::prelude::*; use std::fs; +#[cfg(unix)] +use std::io::{Read, Write}; +#[cfg(unix)] +use std::net::TcpListener; use std::path::Path; fn bt_command() -> Command { @@ -87,6 +91,29 @@ fn write_run_agent(path: &Path) { fs::set_permissions(path, perms).expect("chmod"); } +#[cfg(unix)] +fn serve_login_once() -> (String, std::thread::JoinHandle<()>) { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind login server"); + let address = listener.local_addr().expect("login server address"); + let handle = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept login request"); + let mut request = [0_u8; 4096]; + let size = stream.read(&mut request).expect("read login request"); + let request = String::from_utf8_lossy(&request[..size]); + assert!(request.starts_with("POST /api/apikey/login ")); + + let body = r#"{"org_info":[{"id":"org-1","name":"test-org","api_url":"https://api.braintrust.dev"}]}"#; + write!( + stream, + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + body.len(), + body + ) + .expect("write login response"); + }); + (format!("http://{address}"), handle) +} + fn make_git_repo() -> tempfile::TempDir { let dir = tempfile::tempdir().expect("tempdir"); fs::write(dir.path().join(".git"), "gitdir: /tmp/fake").expect("write .git"); @@ -856,11 +883,56 @@ fn trace_run_uses_the_invocation_project_without_changing_setup() { #[cfg(unix)] #[test] -fn trace_enable_requires_durable_saved_profile_auth() { +fn trace_run_materializes_an_explicit_api_key_and_completes() { let home = tempfile::tempdir().expect("home tempdir"); let config_home = tempfile::tempdir().expect("config tempdir"); let bin_dir = tempfile::tempdir().expect("bin tempdir"); let state_dir = tempfile::tempdir().expect("state tempdir"); + let run_settings = state_dir.path().join("run-settings.json"); + let (app_url, login_server) = serve_login_once(); + write_run_agent(&bin_dir.path().join("codex")); + + let mut cmd = bt_command(); + clear_braintrust_auth_env(&mut cmd); + cmd.env("HOME", home.path()) + .env("XDG_CONFIG_HOME", config_home.path()) + .env("PATH", bin_dir.path()) + .env("BRAINTRUST_ORG_NAME", "test-org") + .env("AGENT_RUN_LOG", state_dir.path().join("run.log")) + .env("AGENT_RUN_SETTINGS", &run_settings) + .args([ + "trace", + "--api-key", + "test-api-key", + "--app-url", + &app_url, + "run", + "codex", + "--project", + "invocation-project", + "--", + "--version", + ]) + .assert() + .success(); + login_server.join().expect("login server thread"); + + let settings: serde_json::Value = + serde_json::from_slice(&fs::read(run_settings).expect("read invocation settings")) + .expect("parse invocation settings"); + assert_eq!(settings["route"]["auth"]["source"], "saved_profile"); + assert_eq!(settings["route"]["auth"]["profile"], "profile"); +} + +#[cfg(unix)] +#[test] +fn trace_enable_persists_environment_auth_and_completes_setup() { + let home = tempfile::tempdir().expect("home tempdir"); + let config_home = tempfile::tempdir().expect("config tempdir"); + let bin_dir = tempfile::tempdir().expect("bin tempdir"); + let state_dir = tempfile::tempdir().expect("state tempdir"); + let daemon_config = state_dir.path().join("daemon.json"); + let (app_url, login_server) = serve_login_once(); write_agent_cli( &bin_dir.path().join("codex"), r#"{"marketplaces":[]}"#, @@ -871,15 +943,31 @@ fn trace_enable_requires_durable_saved_profile_auth() { .env("HOME", home.path()) .env("PATH", bin_dir.path()) .env("AGENT_SETUP_LOG", state_dir.path().join("codex.log")) - .args(["trace", "enable", "codex", "--project", "agent-traces"]) + .env("BT_DAEMON_CONFIG", &daemon_config) + .args([ + "trace", + "--app-url", + &app_url, + "enable", + "codex", + "--project", + "agent-traces", + ]) .assert() - .failure() - .stderr(predicate::str::contains( - "persistent coding-agent tracing requires a saved Braintrust profile", - )) - .stderr(predicate::str::contains( - "bt login --profile --save-env-api-key", - )); + .success(); + login_server.join().expect("login server thread"); + + let auth_store = + fs::read_to_string(config_home.path().join("bt/auth.json")).expect("saved auth profile"); + assert!(auth_store.contains(r#""profile""#)); + let secrets = fs::read_to_string(config_home.path().join("bt/secrets.json")) + .expect("saved profile credential"); + assert!(secrets.contains("test-api-key")); + let settings: serde_json::Value = + serde_json::from_slice(&fs::read(daemon_config).expect("persistent tracing configuration")) + .expect("parse tracing configuration"); + assert_eq!(settings["route"]["auth"]["source"], "saved_profile"); + assert_eq!(settings["route"]["auth"]["profile"], "profile"); } #[cfg(unix)]