diff --git a/Cargo.lock b/Cargo.lock index 2974bc6..29acf90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -296,6 +296,21 @@ dependencies = [ "serde_json 1.0.149", ] +[[package]] +name = "assert_cmd" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a686bbee5efb88a82df0621b236e74d925f470e5445d3220a5648b892ec99c9" +dependencies = [ + "anstyle", + "bstr", + "libc", + "predicates", + "predicates-core", + "predicates-tree", + "wait-timeout", +] + [[package]] name = "async-compression" version = "0.4.41" @@ -435,12 +450,24 @@ dependencies = [ "alloc-stdlib", ] +[[package]] +name = "bstr" +version = "1.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63044e1ae8e69f3b5a92c736ca6269b8d12fa7efe39bf34ddb06d102cf0e2cab" +dependencies = [ + "memchr", + "regex-automata", + "serde", +] + [[package]] name = "bt" version = "0.2.0" dependencies = [ "actix-web", "anyhow", + "assert_cmd", "backoff", "base64 0.22.1", "braintrust-sdk-rust", @@ -457,6 +484,7 @@ dependencies = [ "oauth2", "open", "pathdiff", + "predicates", "ratatui", "regex", "reqwest 0.12.28", @@ -864,6 +892,12 @@ dependencies = [ "zeroize", ] +[[package]] +name = "difflib" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6184e33543162437515c2e2b48714794e37845ec9851711914eec9d308f6ebe8" + [[package]] name = "digest" version = "0.10.7" @@ -992,6 +1026,15 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "float-cmp" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b09cf3155332e944990140d967ff5eceb70df778b34f77d8075db46e4704e6d8" +dependencies = [ + "num-traits", +] + [[package]] name = "fnv" version = "1.0.7" @@ -1855,6 +1898,12 @@ version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d87ecb2933e8aeadb3e3a02b828fed80a7528047e68b4f424523a0981a3a084" +[[package]] +name = "normalize-line-endings" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" + [[package]] name = "num-conv" version = "0.2.0" @@ -2061,6 +2110,36 @@ dependencies = [ "zerocopy", ] +[[package]] +name = "predicates" +version = "3.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ada8f2932f28a27ee7b70dd6c1c39ea0675c55a36879ab92f3a715eaa1e63cfe" +dependencies = [ + "anstyle", + "difflib", + "float-cmp", + "normalize-line-endings", + "predicates-core", + "regex", +] + +[[package]] +name = "predicates-core" +version = "1.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cad38746f3166b4031b1a0d39ad9f954dd291e7854fcc0eed52ee41a0b50d144" + +[[package]] +name = "predicates-tree" +version = "1.0.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d0de1b847b39c8131db0467e9df1ff60e6d0562ab8e9a16e568ad0fdb372e2f2" +dependencies = [ + "predicates-core", + "termtree", +] + [[package]] name = "prettyplease" version = "0.2.37" @@ -3001,6 +3080,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "termtree" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683" + [[package]] name = "thiserror" version = "1.0.69" @@ -3400,6 +3485,15 @@ dependencies = [ "memchr", ] +[[package]] +name = "wait-timeout" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ac3b126d3914f9849036f826e054cbabdc8519970b8998ddaf3b5bd3c65f11" +dependencies = [ + "libc", +] + [[package]] name = "want" version = "0.3.1" diff --git a/Cargo.toml b/Cargo.toml index 44aa474..4eff1aa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,3 +69,5 @@ install-success-msg = "" [dev-dependencies] tempfile = "3" +assert_cmd = "2.2.0" +predicates = "3.1.4" diff --git a/src/auth.rs b/src/auth.rs index 29a70be..32814f2 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -289,6 +289,51 @@ pub async fn run(base: BaseArgs, args: AuthArgs) -> Result<()> { } } +pub async fn login_read_only(base: &BaseArgs) -> Result { + if !has_cached_project_id() { + return login(base).await; + } + + let ctx = fast_login(base).await?; + if ctx.login.org_name.trim().is_empty() { + login(base).await + } else { + Ok(ctx) + } +} + +/// Build login context from stored auth without forcing a login validation request. +/// Use for read-oriented flows where downstream API calls can surface auth errors. +pub async fn fast_login(base: &BaseArgs) -> Result { + maybe_warn_api_key_override(base); + let auth = resolve_auth(base).await?; + let api_key = auth.api_key.clone().ok_or_else(|| { + anyhow::anyhow!( + "no login credentials found; set BRAINTRUST_API_KEY, pass --api-key, or run `bt auth login`" + ) + })?; + let org_name = auth.org_name.clone().unwrap_or_default(); + let api_url = auth + .api_url + .clone() + .unwrap_or_else(|| DEFAULT_API_URL.to_string()); + let app_url = auth + .app_url + .clone() + .unwrap_or_else(|| DEFAULT_APP_URL.to_string()); + + Ok(LoginContext { + login: LoginState { + api_key, + org_id: String::new(), + org_name, + api_url: Some(api_url.clone()), + }, + api_url, + app_url, + }) +} + pub async fn login(base: &BaseArgs) -> Result { maybe_warn_api_key_override(base); let auth = resolve_auth(base).await?; @@ -354,6 +399,13 @@ pub async fn login(base: &BaseArgs) -> Result { }) } +fn has_cached_project_id() -> bool { + crate::config::load() + .ok() + .and_then(|config| config.project_id) + .is_some_and(|project_id| !project_id.trim().is_empty()) +} + fn maybe_warn_api_key_override(base: &BaseArgs) { if base.json || !std::io::stderr().is_terminal() { return; @@ -2539,8 +2591,18 @@ fn auth_store_path() -> Result { #[cfg(test)] mod tests { + use futures_util::lock::Mutex; + use tempfile::TempDir; + use super::*; - use std::time::{SystemTime, UNIX_EPOCH}; + use std::{ + env, + ffi::OsString, + fs, + path::PathBuf, + sync::OnceLock, + time::{SystemTime, UNIX_EPOCH}, + }; fn make_base() -> BaseArgs { BaseArgs { @@ -2559,6 +2621,128 @@ mod tests { } } + fn env_test_lock() -> &'static Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + } + + fn setup_global_config(project_id: Option<&str>, org: Option<&str>) { + let cfg = crate::config::Config { + org: org.map(str::to_string), + project_id: project_id.map(str::to_string), + ..crate::config::Config::default() + }; + + crate::config::save_global(&cfg).expect("save global config"); + } + + fn setup_auth_store_profiles(profiles: &[(&str, &str, &str, &str)]) { + let mut store = AuthStore::default(); + for (profile_name, org_name, api_url, app_url) in profiles { + store.profiles.insert( + (*profile_name).to_string(), + AuthProfile { + auth_kind: AuthKind::ApiKey, + api_url: Some((*api_url).to_string()), + app_url: Some((*app_url).to_string()), + org_name: Some((*org_name).to_string()), + oauth_client_id: None, + oauth_access_expires_at: None, + user_name: None, + email: None, + api_key_hint: None, + }, + ); + } + + save_auth_store(&store).expect("save auth store"); + } + + fn assert_err_contains(result: Result, expected_substring: &str) { + match result { + Ok(_) => panic!("expected error containing '{expected_substring}'"), + Err(err) => { + let msg = err.to_string(); + assert!( + msg.contains(expected_substring), + "expected error to contain '{expected_substring}', got '{msg}'", + ); + } + } + } + + fn assert_invalid_api_url(result: Result) { + assert_err_contains(result, "invalid api_url"); + } + + fn restore_env_var(key: &str, previous: Option) { + match previous { + Some(value) => env::set_var(key, value), + None => env::remove_var(key), + } + } + + fn base_args_for_path_probe(org_name: Option<&str>) -> BaseArgs { + let mut base = make_base(); + base.api_key = Some("test-api-key".into()); + base.api_url = Some("not-a-valid-url".into()); + base.app_url = Some("https://app.example.test".into()); + base.org_name = org_name.map(|s| s.into()); + base + } + + struct TestEnv { + _guard: futures_util::lock::MutexGuard<'static, ()>, + _config_dir: TempDir, + _cwd_dir: TempDir, + previous_cwd: PathBuf, + previous_xdg_config_home: Option, + previous_appdata: Option, + } + + impl TestEnv { + async fn new(project_id: Option<&str>, org: Option<&str>) -> Self { + let guard = env_test_lock().lock().await; + let previous_cwd = env::current_dir().expect("read current dir"); + let cwd_dir = TempDir::new().expect("create temp cwd"); + // Prevent config::local_path from traversing parent directories. + fs::create_dir(cwd_dir.path().join(".git")).expect("create .git marker"); + env::set_current_dir(cwd_dir.path()).expect("set test current dir"); + + let previous_xdg_config_home = env::var_os("XDG_CONFIG_HOME"); + let previous_appdata = env::var_os("APPDATA"); + let config_dir = TempDir::new().expect("create temp config dir"); + env::set_var("XDG_CONFIG_HOME", config_dir.path()); + env::set_var("APPDATA", config_dir.path()); + setup_global_config(project_id, org); + Self { + _guard: guard, + _config_dir: config_dir, + _cwd_dir: cwd_dir, + previous_cwd, + previous_xdg_config_home, + previous_appdata, + } + } + + async fn login_read_only_with_base(&self, base: BaseArgs) -> Result { + login_read_only(&base).await + } + + async fn login_read_only_probe(&self, org_name: Option<&str>) -> Result { + self.login_read_only_with_base(base_args_for_path_probe(org_name)) + .await + } + } + + impl Drop for TestEnv { + fn drop(&mut self) { + env::set_current_dir(&self.previous_cwd).expect("restore current dir"); + restore_env_var("XDG_CONFIG_HOME", self.previous_xdg_config_home.clone()); + restore_env_var("APPDATA", self.previous_appdata.clone()); + } + } + #[test] fn default_app_url_is_www() { assert_eq!(DEFAULT_APP_URL, "https://www.braintrust.dev"); @@ -3016,4 +3200,99 @@ mod tests { "bad — api_key — org: corp — invalid API key" ); } + + #[tokio::test] + async fn login_read_only_no_cached_project_id_uses_validated_login_path() { + let env = TestEnv::new(None, None).await; + assert_invalid_api_url(env.login_read_only_probe(Some("acme")).await); + } + + #[tokio::test] + async fn login_read_only_cached_project_id_and_org_uses_fast_path() { + let env = TestEnv::new(Some("proj_123"), None).await; + let ctx = env + .login_read_only_probe(Some("acme")) + .await + .expect("fast path should succeed"); + + assert_eq!(ctx.login.org_name, "acme"); + assert_eq!(ctx.login.org_id, ""); + assert_eq!(ctx.api_url, "not-a-valid-url"); + } + + #[tokio::test] + async fn login_read_only_cached_project_id_but_missing_org_falls_back_to_login() { + let env = TestEnv::new(Some("proj_123"), None).await; + assert_invalid_api_url(env.login_read_only_probe(None).await); + } + + #[tokio::test] + async fn login_read_only_cached_project_id_but_whitespace_org_falls_back_to_login() { + let env = TestEnv::new(Some("proj_123"), None).await; + assert_invalid_api_url(env.login_read_only_probe(Some(" ")).await); + } + + #[tokio::test] + async fn login_read_only_whitespace_project_id_is_treated_as_not_cached() { + let env = TestEnv::new(Some(" "), None).await; // has_cached_project_id => false + assert_invalid_api_url(env.login_read_only_probe(Some("acme")).await); + } + + #[tokio::test] + async fn login_read_only_cached_project_id_and_config_org_uses_fast_path() { + let env = TestEnv::new(Some("proj_123"), Some("acme-org")).await; + setup_auth_store_profiles(&[ + ( + "acme-profile", + "acme-org", + "https://api.acme.example", + "https://www.acme.example", + ), + ( + "other-profile", + "other-org", + "https://api.other.example", + "https://www.other.example", + ), + ]); + save_profile_secret_plaintext("acme-profile", "acme-secret").expect("save acme secret"); + save_profile_secret_plaintext("other-profile", "other-secret").expect("save other secret"); + + let ctx = env + .login_read_only_with_base(make_base()) + .await + .expect("fast path should succeed with cfg org"); + + assert_eq!(ctx.login.api_key, "acme-secret"); + assert_eq!(ctx.login.org_name, "acme-org"); + assert_eq!(ctx.api_url, "https://api.acme.example"); + assert_eq!(ctx.app_url, "https://www.acme.example"); + } + + #[tokio::test] + async fn login_read_only_cached_project_id_and_org_uses_default_urls() { + let env = TestEnv::new(Some("proj_123"), None).await; + let mut base = make_base(); + base.api_key = Some("test-api-key".into()); + base.org_name = Some("acme".into()); + + let ctx = env + .login_read_only_with_base(base) + .await + .expect("fast path should succeed"); + + assert_eq!(ctx.login.org_name, "acme"); + assert_eq!(ctx.api_url, DEFAULT_API_URL); + assert_eq!(ctx.app_url, DEFAULT_APP_URL); + } + + #[tokio::test] + async fn login_read_only_cached_project_id_missing_api_key_returns_helpful_error() { + let env = TestEnv::new(Some("proj_123"), None).await; + let mut base = make_base(); + base.org_name = Some("acme".into()); + + let result = env.login_read_only_with_base(base).await; + assert_err_contains(result, "no login credentials found"); + } } diff --git a/src/experiments/mod.rs b/src/experiments/mod.rs index 8218073..33f4cfd 100644 --- a/src/experiments/mod.rs +++ b/src/experiments/mod.rs @@ -1,13 +1,11 @@ -use anyhow::{anyhow, bail, Result}; +use anyhow::{bail, Result}; use clap::{Args, Subcommand}; use crate::{ args::BaseArgs, - auth::login, - config, http::ApiClient, - projects::api::{get_project_by_name, Project}, - ui::{self, is_interactive, select_project_interactive, with_spinner}, + projects::context::{resolve_project_context, ProjectContext}, + ui::{self, with_spinner}, }; mod api; @@ -17,11 +15,7 @@ mod view; use api::{self as experiments_api, Experiment}; -pub(crate) struct ResolvedContext { - pub client: ApiClient, - pub app_url: String, - pub project: Project, -} +pub(crate) type ResolvedContext = ProjectContext; #[derive(Debug, Clone, Args)] #[command(after_help = "\ @@ -112,28 +106,56 @@ pub(crate) async fn select_experiment_interactive( } pub async fn run(base: BaseArgs, args: ExperimentsArgs) -> Result<()> { - let auth = login(&base).await?; - let client = ApiClient::new(&auth)?; - let config_project = config::load().ok().and_then(|c| c.project); - let project_name = match base.project.as_deref().or(config_project.as_deref()) { - Some(p) => p.to_string(), - None if is_interactive() => select_project_interactive(&client, None, None).await?, - None => anyhow::bail!("--project required (or set BRAINTRUST_DEFAULT_PROJECT)"), - }; - - let project = get_project_by_name(&client, &project_name) - .await? - .ok_or_else(|| anyhow!("project '{project_name}' not found"))?; - - let ctx = ResolvedContext { - client, - app_url: auth.app_url, - project, - }; - match args.command { - None | Some(ExperimentsCommands::List) => list::run(&ctx, base.json).await, - Some(ExperimentsCommands::View(v)) => view::run(&ctx, v.name(), base.json, v.web).await, - Some(ExperimentsCommands::Delete(d)) => delete::run(&ctx, d.name(), d.force).await, + None | Some(ExperimentsCommands::List) => { + let ctx = resolve_project_context(&base, true).await?; + list::run(&ctx, base.json).await + } + Some(ExperimentsCommands::View(v)) => { + let ctx = resolve_project_context(&base, true).await?; + view::run(&ctx, v.name(), base.json, v.web).await + } + Some(ExperimentsCommands::Delete(d)) => { + let ctx = resolve_project_context(&base, false).await?; + delete::run(&ctx, d.name(), d.force).await + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn experiments_command_is_read_only(command: Option<&ExperimentsCommands>) -> bool { + matches!( + command, + None | Some(ExperimentsCommands::List) | Some(ExperimentsCommands::View(_)) + ) + } + + #[test] + fn experiments_routes_list_and_view_to_read_only_auth() { + assert!(experiments_command_is_read_only(None)); + assert!(experiments_command_is_read_only(Some( + &ExperimentsCommands::List + ))); + assert!(experiments_command_is_read_only(Some( + &ExperimentsCommands::View(ViewArgs { + name_positional: Some("my-experiment".to_string()), + name_flag: None, + web: false, + }) + ))); + } + + #[test] + fn experiments_routes_delete_to_validated_auth() { + assert!(!experiments_command_is_read_only(Some( + &ExperimentsCommands::Delete(DeleteArgs { + name_positional: Some("my-experiment".to_string()), + name_flag: None, + force: true, + }) + ))); } } diff --git a/src/functions/mod.rs b/src/functions/mod.rs index 3596449..c9d5c43 100644 --- a/src/functions/mod.rs +++ b/src/functions/mod.rs @@ -1,13 +1,11 @@ -use anyhow::{anyhow, bail, Result}; +use anyhow::{bail, Result}; use clap::{Args, Subcommand, ValueEnum}; use crate::{ args::BaseArgs, - auth::login, - config, http::ApiClient, - projects::api::{get_project_by_name, Project}, - ui::{self, is_interactive, select_project_interactive, with_spinner}, + projects::context::{resolve_project_context, ProjectContext}, + ui::{self, with_spinner}, }; pub(crate) mod api; @@ -245,30 +243,7 @@ impl DeleteArgs { // --- Resolved context --- -pub(crate) struct ResolvedContext { - pub client: ApiClient, - pub app_url: String, - pub project: Project, -} - -async fn resolve_context(base: &BaseArgs) -> Result { - let ctx = login(base).await?; - let client = ApiClient::new(&ctx)?; - let config_project = config::load().ok().and_then(|c| c.project); - let project_name = match base.project.as_deref().or(config_project.as_deref()) { - Some(p) => p.to_string(), - None if is_interactive() => select_project_interactive(&client, None, None).await?, - None => anyhow::bail!("--project required (or set BRAINTRUST_DEFAULT_PROJECT)"), - }; - let project = get_project_by_name(&client, &project_name) - .await? - .ok_or_else(|| anyhow!("project '{project_name}' not found"))?; - Ok(ResolvedContext { - client, - app_url: ctx.app_url, - project, - }) -} +pub(crate) type ResolvedContext = ProjectContext; // --- Interactive selection --- @@ -309,39 +284,146 @@ pub(crate) async fn select_function_interactive( // --- Entry points --- pub async fn run_typed(base: BaseArgs, args: FunctionArgs, kind: FunctionTypeFilter) -> Result<()> { - let ctx = resolve_context(&base).await?; let ft = Some(kind); match args.command { - None | Some(FunctionCommands::List) => list::run(&ctx, base.json, ft).await, + None | Some(FunctionCommands::List) => { + let ctx = resolve_project_context(&base, true).await?; + list::run(&ctx, base.json, ft).await + } Some(FunctionCommands::View(v)) => { + let ctx = resolve_project_context(&base, true).await?; view::run(&ctx, v.slug(), base.json, v.web, v.verbose, ft).await } - Some(FunctionCommands::Delete(d)) => delete::run(&ctx, d.slug(), d.force, ft).await, - Some(FunctionCommands::Invoke(i)) => invoke::run(&ctx, &i, base.json, ft).await, + Some(FunctionCommands::Delete(d)) => { + let ctx = resolve_project_context(&base, false).await?; + delete::run(&ctx, d.slug(), d.force, ft).await + } + Some(FunctionCommands::Invoke(i)) => { + let ctx = resolve_project_context(&base, false).await?; + invoke::run(&ctx, &i, base.json, ft).await + } } } pub async fn run(base: BaseArgs, args: FunctionsArgs) -> Result<()> { - let ctx = resolve_context(&base).await?; + let default_function_type = args.function_type; match args.command { - None => list::run(&ctx, base.json, args.function_type).await, - Some(FunctionsCommands::List(ref la)) => list::run(&ctx, base.json, la.function_type).await, + None => { + let ctx = resolve_project_context(&base, true).await?; + list::run(&ctx, base.json, default_function_type).await + } + Some(FunctionsCommands::List(la)) => { + let ctx = resolve_project_context(&base, true).await?; + list::run(&ctx, base.json, la.function_type).await + } Some(FunctionsCommands::View(v)) => { + let ctx = resolve_project_context(&base, true).await?; view::run( &ctx, v.slug(), base.json, v.web, v.verbose, - args.function_type, + default_function_type, ) .await } Some(FunctionsCommands::Delete(d)) => { + let ctx = resolve_project_context(&base, false).await?; delete::run(&ctx, d.slug(), d.force, d.function_type).await } Some(FunctionsCommands::Invoke(i)) => { + let ctx = resolve_project_context(&base, false).await?; invoke::run(&ctx, &i.inner, base.json, i.function_type).await } } } + +#[cfg(test)] +mod tests { + use super::*; + use clap::Parser; + + #[derive(Debug, Parser)] + struct FunctionArgsHarness { + #[command(flatten)] + args: FunctionArgs, + } + + #[derive(Debug, Parser)] + struct FunctionsArgsHarness { + #[command(flatten)] + args: FunctionsArgs, + } + + fn function_command_is_read_only(command: Option<&FunctionCommands>) -> bool { + matches!( + command, + None | Some(FunctionCommands::List) | Some(FunctionCommands::View(_)) + ) + } + + fn functions_command_is_read_only(command: Option<&FunctionsCommands>) -> bool { + matches!( + command, + None | Some(FunctionsCommands::List(_)) | Some(FunctionsCommands::View(_)) + ) + } + + #[test] + fn typed_function_commands_map_to_expected_auth_mode() { + let parsed = FunctionArgsHarness::try_parse_from(["bt-tools"]).expect("parse"); + assert!(function_command_is_read_only(parsed.args.command.as_ref())); + + let parsed = FunctionArgsHarness::try_parse_from(["bt-tools", "list"]).expect("parse"); + assert!(function_command_is_read_only(parsed.args.command.as_ref())); + + let parsed = FunctionArgsHarness::try_parse_from(["bt-tools", "view", "--slug", "my-tool"]) + .expect("parse"); + assert!(function_command_is_read_only(parsed.args.command.as_ref())); + + let parsed = FunctionArgsHarness::try_parse_from([ + "bt-tools", "delete", "--slug", "my-tool", "--force", + ]) + .expect("parse"); + assert!(!function_command_is_read_only(parsed.args.command.as_ref())); + + let parsed = + FunctionArgsHarness::try_parse_from(["bt-tools", "invoke", "--slug", "my-tool"]) + .expect("parse"); + assert!(!function_command_is_read_only(parsed.args.command.as_ref())); + } + + #[test] + fn functions_commands_map_to_expected_auth_mode() { + let parsed = FunctionsArgsHarness::try_parse_from(["bt-functions"]).expect("parse"); + assert!(functions_command_is_read_only(parsed.args.command.as_ref())); + + let parsed = FunctionsArgsHarness::try_parse_from(["bt-functions", "list"]).expect("parse"); + assert!(functions_command_is_read_only(parsed.args.command.as_ref())); + + let parsed = + FunctionsArgsHarness::try_parse_from(["bt-functions", "view", "--slug", "my-fn"]) + .expect("parse"); + assert!(functions_command_is_read_only(parsed.args.command.as_ref())); + + let parsed = FunctionsArgsHarness::try_parse_from([ + "bt-functions", + "delete", + "--slug", + "my-fn", + "--force", + ]) + .expect("parse"); + assert!(!functions_command_is_read_only( + parsed.args.command.as_ref() + )); + + let parsed = + FunctionsArgsHarness::try_parse_from(["bt-functions", "invoke", "--slug", "my-fn"]) + .expect("parse"); + assert!(!functions_command_is_read_only( + parsed.args.command.as_ref() + )); + } +} diff --git a/src/projects/context.rs b/src/projects/context.rs new file mode 100644 index 0000000..e635267 --- /dev/null +++ b/src/projects/context.rs @@ -0,0 +1,44 @@ +use anyhow::{anyhow, bail, Result}; + +use crate::{ + args::BaseArgs, + auth::{login, login_read_only}, + config, + http::ApiClient, + ui::{is_interactive, select_project_interactive}, +}; + +use super::api::{get_project_by_name, Project}; + +pub(crate) struct ProjectContext { + pub client: ApiClient, + pub app_url: String, + pub project: Project, +} + +pub(crate) async fn resolve_project_context( + base: &BaseArgs, + read_only: bool, +) -> Result { + let auth = if read_only { + login_read_only(base).await? + } else { + login(base).await? + }; + let client = ApiClient::new(&auth)?; + let config_project = config::load().ok().and_then(|c| c.project); + let project_name = match base.project.as_deref().or(config_project.as_deref()) { + Some(p) => p.to_string(), + None if is_interactive() => select_project_interactive(&client, None, None).await?, + None => bail!("--project required (or set BRAINTRUST_DEFAULT_PROJECT)"), + }; + let project = get_project_by_name(&client, &project_name) + .await? + .ok_or_else(|| anyhow!("project '{project_name}' not found"))?; + + Ok(ProjectContext { + client, + app_url: auth.app_url, + project, + }) +} diff --git a/src/projects/mod.rs b/src/projects/mod.rs index 10fd85d..2ee1dd8 100644 --- a/src/projects/mod.rs +++ b/src/projects/mod.rs @@ -6,6 +6,7 @@ use crate::auth::login; use crate::http::ApiClient; pub(crate) mod api; +pub(crate) mod context; mod create; mod delete; mod list; diff --git a/src/prompts/mod.rs b/src/prompts/mod.rs index 358ea6e..0a87fde 100644 --- a/src/prompts/mod.rs +++ b/src/prompts/mod.rs @@ -1,19 +1,12 @@ -use anyhow::{anyhow, Result}; +use anyhow::Result; use clap::{Args, Subcommand}; use crate::{ args::BaseArgs, - auth::login, - http::ApiClient, - projects::api::{get_project_by_name, Project}, - ui::{is_interactive, select_project_interactive}, + projects::context::{resolve_project_context, ProjectContext}, }; -pub(crate) struct ResolvedContext { - pub client: ApiClient, - pub app_url: String, - pub project: Project, -} +pub(crate) type ResolvedContext = ProjectContext; mod api; mod delete; @@ -93,32 +86,55 @@ impl DeleteArgs { } pub async fn run(base: BaseArgs, args: PromptsArgs) -> Result<()> { - let auth = login(&base).await?; - let client = ApiClient::new(&auth)?; - let project_name = match base - .project - .or_else(|| crate::config::load().ok().and_then(|c| c.project)) - { - Some(p) => p, - None if is_interactive() => select_project_interactive(&client, None, None).await?, - None => anyhow::bail!("--project required (or set BRAINTRUST_DEFAULT_PROJECT)"), - }; - - let project = get_project_by_name(&client, &project_name) - .await? - .ok_or_else(|| anyhow!("project '{project_name}' not found"))?; - - let ctx = ResolvedContext { - client, - app_url: auth.app_url, - project, - }; - match args.command { - None | Some(PromptsCommands::List) => list::run(&ctx, base.json).await, + None | Some(PromptsCommands::List) => { + let ctx = resolve_project_context(&base, true).await?; + list::run(&ctx, base.json).await + } Some(PromptsCommands::View(p)) => { + let ctx = resolve_project_context(&base, true).await?; view::run(&ctx, p.slug(), base.json, p.web, p.verbose).await } - Some(PromptsCommands::Delete(p)) => delete::run(&ctx, p.slug(), p.force).await, + Some(PromptsCommands::Delete(p)) => { + let ctx = resolve_project_context(&base, false).await?; + delete::run(&ctx, p.slug(), p.force).await + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn prompts_command_is_read_only(command: Option<&PromptsCommands>) -> bool { + matches!( + command, + None | Some(PromptsCommands::List) | Some(PromptsCommands::View(_)) + ) + } + + #[test] + fn prompts_routes_list_and_view_to_read_only_auth() { + assert!(prompts_command_is_read_only(None)); + assert!(prompts_command_is_read_only(Some(&PromptsCommands::List))); + assert!(prompts_command_is_read_only(Some(&PromptsCommands::View( + ViewArgs { + slug_positional: Some("my-prompt".to_string()), + slug_flag: None, + web: false, + verbose: false, + } + )))); + } + + #[test] + fn prompts_routes_delete_to_validated_auth() { + assert!(!prompts_command_is_read_only(Some( + &PromptsCommands::Delete(DeleteArgs { + slug_positional: Some("my-prompt".to_string()), + slug_flag: None, + force: true, + }) + ))); } } diff --git a/src/switch.rs b/src/switch.rs index cc9bf3a..cbbd93a 100644 --- a/src/switch.rs +++ b/src/switch.rs @@ -110,16 +110,23 @@ pub async fn run(base: BaseArgs, args: SwitchArgs) -> Result<()> { let ctx = login(&login_base).await?; let client = ApiClient::new(&ctx)?; - let org_name = client.org_name(); + let org_name = client.org_name().to_string(); - let project_name = match resolved_project { - Some(p) => Some(validate_or_create_project(&client, &p).await?), + let project = match resolved_project { + Some(p) => validate_or_create_project(&client, &p).await?, None => { if !is_interactive() { bail!("target required. Use: bt switch or bt switch /"); } interactive = true; - Some(select_project_interactive(&client, None, current_cfg.project.as_deref()).await?) + let selected_name = + select_project_interactive(&client, None, current_cfg.project.as_deref()).await?; + with_spinner( + "Loading project...", + api::get_project_by_name(&client, &selected_name), + ) + .await? + .ok_or_else(|| anyhow::anyhow!("project '{selected_name}' not found"))? } }; @@ -138,15 +145,11 @@ pub async fn run(base: BaseArgs, args: SwitchArgs) -> Result<()> { }; let mut cfg = config::load_file(&path); - cfg.org = Some(org_name.to_string()); - cfg.project = project_name.clone(); + apply_switch_config(&mut cfg, &org_name, &project); config::save_file(&path, &cfg) .context(format!("Could not save config to {}", path.display()))?; - let display = match &project_name { - Some(p) => format!("{org_name}/{p}"), - None => org_name.to_string(), - }; + let display = format!("{org_name}/{}", project.name); print_command_status(CommandStatus::Success, &format!("Switched to {display}")); if args.verbose { eprintln!("Wrote to {}", path.display()); @@ -204,11 +207,11 @@ fn select_scope() -> Result { } } -async fn validate_or_create_project(client: &ApiClient, name: &str) -> Result { +async fn validate_or_create_project(client: &ApiClient, name: &str) -> Result { let exists = with_spinner("Loading project...", api::get_project_by_name(client, name)).await?; - if exists.is_some() { - return Ok(name.to_string()); + if let Some(project) = exists { + return Ok(project); } if !is_interactive() { @@ -221,13 +224,18 @@ async fn validate_or_create_project(client: &ApiClient, name: &str) -> Result>(message: &str, fut: F) -> T { if !std::io::stderr().is_terminal() || !animations_enabled() || is_quiet() { return fut.await;