From d2a55b44b246af10877dd487bb31cfe82a5894b8 Mon Sep 17 00:00:00 2001 From: Jack-Khuu Date: Mon, 20 Apr 2026 18:06:47 -0700 Subject: [PATCH 1/4] Override popcorn id with env arg --- README.md | 1 + src/cmd/mod.rs | 111 +++++++++++++++++++++++++++++++++++-------------- 2 files changed, 80 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index ef6bc6e..7a6069d 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ Since we're effectively giving out GPUs for free we rely on either github or dis 1. Register the CLI (Discord recommended): `popcorn register discord` (or `popcorn register github`) 2. To ensure the above worked you can run `cat $HOME/.popcorn.yaml` which should print your client ID which is what will be sent to us on every request +3. To override the submitter ID for local proxy/testing flows, set `POPCORN_SUBMITTER_ID`. Commands that require `cli_id` use this value first, then fall back to `~/.popcorn.yaml`. Sometimes you'll get an error that you're already authenticated despite being unable to submit in which case you can run `popcorn reregister [discord|github]`. diff --git a/src/cmd/mod.rs b/src/cmd/mod.rs index e1b1033..004b2a8 100644 --- a/src/cmd/mod.rs +++ b/src/cmd/mod.rs @@ -40,6 +40,27 @@ fn load_config() -> Result { serde_yaml::from_reader(file).map_err(|e| anyhow!("Failed to parse config file: {}", e)) } +fn submit_cli_id_from_env() -> Option { + std::env::var("POPCORN_SUBMITTER_ID") + .ok() + .filter(|v| !v.trim().is_empty()) +} + +fn resolve_cli_id() -> Result { + if let Some(cli_id) = submit_cli_id_from_env() { + return Ok(cli_id); + } + + let config = load_config()?; + config.cli_id.ok_or_else(|| { + anyhow!( + "cli_id not found in config file ({}). Please run 'popcorn-cli register' first.", + get_config_path() + .map_or_else(|_| "unknown path".to_string(), |p| p.display().to_string()) + ) + }) +} + #[derive(Parser, Debug)] #[command(author, version = env!("CLI_VERSION"), about, long_about = None)] /// Popcorn CLI for GPU Mode competitions. Run `popcorn setup` first in each project so agents use the correct workflow and templates. @@ -183,14 +204,7 @@ pub async fn execute(cli: Cli) -> Result<()> { output, no_tui, }) => { - let config = load_config()?; - let cli_id = config.cli_id.ok_or_else(|| { - anyhow!( - "cli_id not found in config file ({}). Please run 'popcorn-cli register' first.", - get_config_path() - .map_or_else(|_| "unknown path".to_string(), |p| p.display().to_string()) - ) - })?; + let cli_id = resolve_cli_id()?; // Use filepath from Submit command first, fallback to top-level filepath let final_filepath = filepath.or(cli.filepath); @@ -218,14 +232,7 @@ pub async fn execute(cli: Cli) -> Result<()> { } } Some(Commands::Join { code }) => { - let config = load_config()?; - let cli_id = config.cli_id.ok_or_else(|| { - anyhow!( - "cli_id not found in config file ({}). Please run `popcorn register` first.", - get_config_path() - .map_or_else(|_| "unknown path".to_string(), |p| p.display().to_string()) - ) - })?; + let cli_id = resolve_cli_id()?; let client = service::create_client(Some(cli_id))?; let result = service::join_with_invite(&client, &code).await?; let leaderboards = result["leaderboards"] @@ -242,14 +249,7 @@ pub async fn execute(cli: Cli) -> Result<()> { } Some(Commands::Admin { action }) => admin::handle_admin(action).await, Some(Commands::Submissions { action }) => { - let config = load_config()?; - let cli_id = config.cli_id.ok_or_else(|| { - anyhow!( - "cli_id not found in config file ({}). Please run `popcorn register` first.", - get_config_path() - .map_or_else(|_| "unknown path".to_string(), |p| p.display().to_string()) - ) - })?; + let cli_id = resolve_cli_id()?; match action { SubmissionsAction::List { leaderboard, limit } => { @@ -272,14 +272,7 @@ pub async fn execute(cli: Cli) -> Result<()> { // Handle the case where only a filepath is provided (for backward compatibility) if let Some(top_level_filepath) = cli.filepath { - let config = load_config()?; - let cli_id = config.cli_id.ok_or_else(|| { - anyhow!( - "cli_id not found in config file ({}). Please run `popcorn register` first.", - get_config_path() - .map_or_else(|_| "unknown path".to_string(), |p| p.display().to_string()) - ) - })?; + let cli_id = resolve_cli_id()?; // Run TUI with only filepath, no other options submit::run_submit_tui( @@ -299,3 +292,57 @@ pub async fn execute(cli: Cli) -> Result<()> { } } } + +#[cfg(test)] +mod tests { + use super::resolve_cli_id; + use std::env; + use std::fs; + use std::sync::Mutex; + use tempfile::tempdir; + + static ENV_LOCK: Mutex<()> = Mutex::new(()); + + struct EnvGuard { + old_home: Option, + old_submitter: Option, + } + + impl EnvGuard { + fn new() -> Self { + Self { + old_home: env::var("HOME").ok(), + old_submitter: env::var("POPCORN_SUBMITTER_ID").ok(), + } + } + } + + impl Drop for EnvGuard { + fn drop(&mut self) { + match &self.old_home { + Some(v) => env::set_var("HOME", v), + None => env::remove_var("HOME"), + } + match &self.old_submitter { + Some(v) => env::set_var("POPCORN_SUBMITTER_ID", v), + None => env::remove_var("POPCORN_SUBMITTER_ID"), + } + } + } + + #[test] + fn test_resolve_cli_id_prefers_env_over_config() { + let _lock = ENV_LOCK.lock().expect("Failed to lock env mutex"); + let _guard = EnvGuard::new(); + + let temp_home = tempdir().expect("Failed to create temp home dir"); + let config_path = temp_home.path().join(".popcorn.yaml"); + fs::write(config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); + + env::set_var("HOME", temp_home.path()); + env::set_var("POPCORN_SUBMITTER_ID", "env-cli-id"); + + let cli_id = resolve_cli_id().expect("Expected cli_id resolution to succeed"); + assert_eq!(cli_id, "env-cli-id"); + } +} From 40f94927a0caff444a03f6e2f724c78b15099d08 Mon Sep 17 00:00:00 2001 From: Jack-Khuu Date: Wed, 29 Apr 2026 10:11:28 -0700 Subject: [PATCH 2/4] More tests --- src/cmd/mod.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/cmd/mod.rs b/src/cmd/mod.rs index 004b2a8..e2394d5 100644 --- a/src/cmd/mod.rs +++ b/src/cmd/mod.rs @@ -345,4 +345,52 @@ mod tests { let cli_id = resolve_cli_id().expect("Expected cli_id resolution to succeed"); assert_eq!(cli_id, "env-cli-id"); } + + #[test] + fn test_resolve_cli_id_falls_back_to_config() { + let _lock = ENV_LOCK.lock().expect("Failed to lock env mutex"); + let _guard = EnvGuard::new(); + + let temp_home = tempdir().expect("Failed to create temp home dir"); + let config_path = temp_home.path().join(".popcorn.yaml"); + fs::write(config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); + + env::set_var("HOME", temp_home.path()); + env::remove_var("POPCORN_SUBMITTER_ID"); + + let cli_id = resolve_cli_id().expect("Expected cli_id resolution to succeed"); + assert_eq!(cli_id, "config-cli-id"); + } + + #[test] + fn test_resolve_cli_id_ignores_empty_env() { + let _lock = ENV_LOCK.lock().expect("Failed to lock env mutex"); + let _guard = EnvGuard::new(); + + let temp_home = tempdir().expect("Failed to create temp home dir"); + let config_path = temp_home.path().join(".popcorn.yaml"); + fs::write(config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); + + env::set_var("HOME", temp_home.path()); + env::set_var("POPCORN_SUBMITTER_ID", " "); + + let cli_id = resolve_cli_id().expect("Expected cli_id resolution to succeed"); + assert_eq!(cli_id, "config-cli-id"); + } + + #[test] + fn test_resolve_cli_id_errors_when_no_cli_id() { + let _lock = ENV_LOCK.lock().expect("Failed to lock env mutex"); + let _guard = EnvGuard::new(); + + let temp_home = tempdir().expect("Failed to create temp home dir"); + let config_path = temp_home.path().join(".popcorn.yaml"); + fs::write(config_path, "{}\n").expect("Failed to write config"); + + env::set_var("HOME", temp_home.path()); + env::remove_var("POPCORN_SUBMITTER_ID"); + + let err = resolve_cli_id().unwrap_err(); + assert!(err.to_string().contains("cli_id not found")); + } } From 88c50f57cae2dd1e37648a080cb3afeee4c56e6e Mon Sep 17 00:00:00 2001 From: Jack-Khuu Date: Wed, 29 Apr 2026 11:54:46 -0700 Subject: [PATCH 3/4] Fix Windows test failures: skip config-fallback tests on Windows dirs::home_dir() on Windows uses the shell API (SHGetKnownFolderPath), not the HOME env var, so we cannot redirect config lookup in tests. Gate config-fallback tests with #[cfg(not(windows))]. The env var override path is still tested cross-platform. Also recover from poisoned mutex so one test failure doesn't cascade. --- src/cmd/mod.rs | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/cmd/mod.rs b/src/cmd/mod.rs index e2394d5..e0d88f1 100644 --- a/src/cmd/mod.rs +++ b/src/cmd/mod.rs @@ -332,62 +332,62 @@ mod tests { #[test] fn test_resolve_cli_id_prefers_env_over_config() { - let _lock = ENV_LOCK.lock().expect("Failed to lock env mutex"); + let _lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _guard = EnvGuard::new(); - let temp_home = tempdir().expect("Failed to create temp home dir"); - let config_path = temp_home.path().join(".popcorn.yaml"); - fs::write(config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); - - env::set_var("HOME", temp_home.path()); env::set_var("POPCORN_SUBMITTER_ID", "env-cli-id"); let cli_id = resolve_cli_id().expect("Expected cli_id resolution to succeed"); assert_eq!(cli_id, "env-cli-id"); } + // dirs::home_dir() on Windows uses a shell API, not HOME, so config + // redirection via HOME only works on Unix. + #[cfg(not(windows))] #[test] fn test_resolve_cli_id_falls_back_to_config() { - let _lock = ENV_LOCK.lock().expect("Failed to lock env mutex"); + let _lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _guard = EnvGuard::new(); - let temp_home = tempdir().expect("Failed to create temp home dir"); - let config_path = temp_home.path().join(".popcorn.yaml"); - fs::write(config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); + let tmp = tempdir().expect("Failed to create temp dir"); + let config_path = tmp.path().join(".popcorn.yaml"); + fs::write(&config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); - env::set_var("HOME", temp_home.path()); + env::set_var("HOME", tmp.path()); env::remove_var("POPCORN_SUBMITTER_ID"); let cli_id = resolve_cli_id().expect("Expected cli_id resolution to succeed"); assert_eq!(cli_id, "config-cli-id"); } + #[cfg(not(windows))] #[test] fn test_resolve_cli_id_ignores_empty_env() { - let _lock = ENV_LOCK.lock().expect("Failed to lock env mutex"); + let _lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _guard = EnvGuard::new(); - let temp_home = tempdir().expect("Failed to create temp home dir"); - let config_path = temp_home.path().join(".popcorn.yaml"); - fs::write(config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); + let tmp = tempdir().expect("Failed to create temp dir"); + let config_path = tmp.path().join(".popcorn.yaml"); + fs::write(&config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); - env::set_var("HOME", temp_home.path()); + env::set_var("HOME", tmp.path()); env::set_var("POPCORN_SUBMITTER_ID", " "); let cli_id = resolve_cli_id().expect("Expected cli_id resolution to succeed"); assert_eq!(cli_id, "config-cli-id"); } + #[cfg(not(windows))] #[test] fn test_resolve_cli_id_errors_when_no_cli_id() { - let _lock = ENV_LOCK.lock().expect("Failed to lock env mutex"); + let _lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _guard = EnvGuard::new(); - let temp_home = tempdir().expect("Failed to create temp home dir"); - let config_path = temp_home.path().join(".popcorn.yaml"); - fs::write(config_path, "{}\n").expect("Failed to write config"); + let tmp = tempdir().expect("Failed to create temp dir"); + let config_path = tmp.path().join(".popcorn.yaml"); + fs::write(&config_path, "{}\n").expect("Failed to write config"); - env::set_var("HOME", temp_home.path()); + env::set_var("HOME", tmp.path()); env::remove_var("POPCORN_SUBMITTER_ID"); let err = resolve_cli_id().unwrap_err(); From a1b51d55455a33a791c92015809c4af68374a978 Mon Sep 17 00:00:00 2001 From: Jack-Khuu Date: Wed, 29 Apr 2026 12:56:27 -0700 Subject: [PATCH 4/4] Scope imports --- src/cmd/mod.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/cmd/mod.rs b/src/cmd/mod.rs index e0d88f1..9f8b9d3 100644 --- a/src/cmd/mod.rs +++ b/src/cmd/mod.rs @@ -297,9 +297,7 @@ pub async fn execute(cli: Cli) -> Result<()> { mod tests { use super::resolve_cli_id; use std::env; - use std::fs; use std::sync::Mutex; - use tempfile::tempdir; static ENV_LOCK: Mutex<()> = Mutex::new(()); @@ -349,9 +347,9 @@ mod tests { let _lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _guard = EnvGuard::new(); - let tmp = tempdir().expect("Failed to create temp dir"); + let tmp = tempfile::tempdir().expect("Failed to create temp dir"); let config_path = tmp.path().join(".popcorn.yaml"); - fs::write(&config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); + std::fs::write(&config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); env::set_var("HOME", tmp.path()); env::remove_var("POPCORN_SUBMITTER_ID"); @@ -366,9 +364,9 @@ mod tests { let _lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _guard = EnvGuard::new(); - let tmp = tempdir().expect("Failed to create temp dir"); + let tmp = tempfile::tempdir().expect("Failed to create temp dir"); let config_path = tmp.path().join(".popcorn.yaml"); - fs::write(&config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); + std::fs::write(&config_path, "cli_id: config-cli-id\n").expect("Failed to write config"); env::set_var("HOME", tmp.path()); env::set_var("POPCORN_SUBMITTER_ID", " "); @@ -383,9 +381,9 @@ mod tests { let _lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _guard = EnvGuard::new(); - let tmp = tempdir().expect("Failed to create temp dir"); + let tmp = tempfile::tempdir().expect("Failed to create temp dir"); let config_path = tmp.path().join(".popcorn.yaml"); - fs::write(&config_path, "{}\n").expect("Failed to write config"); + std::fs::write(&config_path, "{}\n").expect("Failed to write config"); env::set_var("HOME", tmp.path()); env::remove_var("POPCORN_SUBMITTER_ID");