diff --git a/apps/desktop-tauri/src-tauri/src/commands/grok_accounts.rs b/apps/desktop-tauri/src-tauri/src/commands/grok_accounts.rs index e71696d5ae9..cfafc57b18f 100644 --- a/apps/desktop-tauri/src-tauri/src/commands/grok_accounts.rs +++ b/apps/desktop-tauri/src-tauri/src/commands/grok_accounts.rs @@ -49,9 +49,8 @@ pub async fn grok_account_add(app: tauri::AppHandle) -> Result<(), String> { .await .map_err(|e| e.to_string())? .map_err(|e| e.to_string())?; - let _credentials = accounts::CREDENTIAL_OPERATION.lock().await; - AccountManager::new() - .and_then(|m| m.import(login)) + accounts::save_login(login) + .await .map_err(|e| e.to_string())?; crate::auto_resume::clear(&app, ProviderId::Grok); changed(&app); diff --git a/apps/desktop-tauri/src-tauri/src/commands/system.rs b/apps/desktop-tauri/src-tauri/src/commands/system.rs index 133a1ab5b05..aa3ed823e97 100644 --- a/apps/desktop-tauri/src-tauri/src/commands/system.rs +++ b/apps/desktop-tauri/src-tauri/src/commands/system.rs @@ -343,6 +343,7 @@ async fn run_cli_provider_login( )), LoginOutcome::LaunchFailed(e) => Err(format!("Failed to launch {display_name} login: {e}")), LoginOutcome::TimedOut => Err(format!("{display_name} login timed out")), + LoginOutcome::Cancelled => Err(format!("{display_name} login cancelled")), LoginOutcome::Failed { status } => Err(format!( "{display_name} login failed with exit code {status}" )), diff --git a/docs/CLI.md b/docs/CLI.md index ae889729809..5d6ff69b52e 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -362,7 +362,7 @@ codexbar config validate | `enable`, `disable` | Turns a provider on or off and saves the setting. | | `set-source` | Saves a provider's usage source (`--source auto\|web\|cli\|oauth`) without changing whether it is enabled. `auto` returns to the default. | | `set-api-key` | Stores a provider API key. `--stdin` reads it from stdin. `--no-enable` stores it without enabling the provider. | -| `login` | Signs in to GitHub Copilot with GitHub's device-code flow. Only `--provider copilot` is supported. See [Sign in to Copilot from the CLI](#sign-in-to-copilot-from-the-cli). | +| `login` | Signs in to GitHub Copilot with GitHub's device-code flow, or to Grok and Kiro through their own CLIs. Other providers are rejected. See [Sign in from the CLI](#sign-in-from-the-cli). | | `path` | Shows the configuration file paths. | | `preferences export`, `preferences import` | Exports or imports portable preferences. See [CONFIGURATION.md](./CONFIGURATION.md#portable-preferences). | | `claude-code-credentials` | `allow`, `deny` or `status`. | @@ -382,14 +382,19 @@ codexbar config validate `config preferences import` changes the saved settings. Restart a running CodexBar afterwards. -### Sign in to Copilot from the CLI +### Sign in from the CLI ```powershell codexbar config login --provider copilot -codexbar config login --provider copilot --json +codexbar config login --provider grok +codexbar config login --provider kiro --json ``` -`config login` starts GitHub's device-code flow for GitHub Copilot. It prints the verification URL and a one-time code, then waits while you approve the code in your browser. The code expires after the lifetime GitHub returns, and the command stops when it does. +`config login` supports three providers: `copilot`, `grok` and `kiro`. The output and exit codes are the same for all three. Each provider's own sign-in differs, as described below. + +#### GitHub Copilot + +`config login --provider copilot` starts GitHub's device-code flow. It prints the verification URL and a one-time code, then waits while you approve the code in your browser. The code expires after the lifetime GitHub returns, and the command stops when it does. Text output is two lines and then a confirmation: @@ -403,35 +408,52 @@ The token is never printed. Once you approve the code, the account is saved the A successful login also turns the Copilot provider on in settings, as the macOS app does. The desktop app's sign-in does not change the enabled state, so enable Copilot there yourself if you use that path. +#### Grok + +`config login --provider grok` runs `grok login --oauth` from Grok Build, the same sign-in the desktop app's Grok account add uses. Grok opens the browser itself, so the CLI prints only `Waiting for the browser sign-in to finish.` (in JSON, an `authorization_pending` line with no URL). Grok's own output is not shown. The sign-in waits up to 5 minutes. + +Once the sign-in finishes, the account is saved to CodexBar's Grok accounts the same way the desktop app saves it. Then the Grok provider is turned on. If the Grok CLI is not installed, the command exits with `8`. Set `CODEXBAR_GROK_CLI_PATH` to a `grok` executable to use that file; the variable is used only when it names an existing file. + +#### Kiro + +`config login --provider kiro` runs `kiro-cli login`. The command first prints `Waiting for the browser sign-in to finish.` (in JSON, an `authorization_pending` line with no URL). kiro-cli then prints a sign-in link and opens the browser. The link is printed as soon as the CLI shows it: on its own line in text mode, or as a second `authorization_pending` line with `verificationURI` in JSON mode. The sign-in waits up to 120 seconds, the same as the desktop sign-in. + +kiro-cli keeps the credential itself. CodexBar does not copy it into its own store. A zero exit code therefore means kiro-cli reported a successful sign-in. Then the Kiro provider is turned on. If kiro-cli is not installed, the command exits with `8`. `CODEXBAR_KIRO_CLI_PATH` selects a specific `kiro-cli` executable, as it does for usage. + +#### JSON events + `--json` prints one JSON object per line, using upstream's field names: | Line | Shape | | --- | --- | -| Code issued | `{"status":"authorization_pending","verificationURI":"https://github.com/login/device","userCode":"ABCD-1234","expiresIn":900}` | -| Approved and saved | `{"status":"authorized"}`. This line is written only after the account is saved and Copilot is enabled. | +| Code issued (Copilot) | `{"status":"authorization_pending","verificationURI":"https://github.com/login/device","userCode":"ABCD-1234","expiresIn":900}` | +| Kiro sign-in link | `{"status":"authorization_pending","verificationURI":""}` | +| Waiting in the browser (Grok; also the first line for Kiro) | `{"status":"authorization_pending"}` | +| Approved and saved | `{"status":"authorized"}`. This line is written only after the credential is saved and the provider is enabled. | | Cancelled (Ctrl-C) | `{"status":"cancelled"}` | -| Failed | `{"status":"failed","reason":"denied","error":"GitHub device login failed: Access denied by user"}`. `reason` is one of the values in the exit code table below. | +| Failed | `{"status":"failed","reason":"denied","error":"GitHub device login failed: Access denied by user"}`. `reason` is one of `denied`, `expired`, `cancelled`, `missing_binary` or `error`. The exit code table below maps each reason to its exit code. | -Text mode prints the URL and code, then `Connected` on success. Errors go to stderr as `Error: `. Ctrl-C ends the login with `Error: Login cancelled.` and exit code `7`. +Text mode prints the URL and code for Copilot. Grok prints only `Waiting for the browser sign-in to finish.` Kiro prints that waiting line, then the sign-in URL once kiro-cli shows it. On success every provider prints `Connected`. Errors go to stderr as `Error: `. Ctrl-C ends the login with `Error: Login cancelled.` and exit code `7`. ### Exit codes | Exit code | Meaning | Final `--json` line | | --- | --- | --- | -| `0` | Signed in. The account is saved and Copilot is enabled. | `{"status":"authorized"}` | -| `1` | Any other failure: network error, GitHub server error, or a settings or token store write error. Usually nothing is saved. If only the settings write that enables Copilot fails, the account is already saved and the error says `Signed in and saved the account, but could not enable Copilot`; running the login again updates that account. | `{"status":"failed","reason":"error",...}` | -| `5` | The user denied the request on GitHub (`access_denied`). Nothing is saved. | `{"status":"failed","reason":"denied",...}` | -| `6` | The device code expired before approval (`expired_token`). Nothing is saved. | `{"status":"failed","reason":"expired",...}` | -| `7` | Stopped with Ctrl-C before the account was saved. Nothing is saved. | `{"status":"cancelled"}` | +| `0` | Signed in. The credential is saved (Copilot and Grok) or kiro-cli reported success (Kiro), and the provider is enabled. | `{"status":"authorized"}` | +| `1` | Any other failure: network error, provider server error, or a settings or store write error. Usually nothing is saved. If only the settings write that enables the provider fails, the sign-in already finished and the error says so: `Signed in and saved the account, but could not enable Copilot` (or `Grok`), or `Signed in, but could not enable Kiro`. Running the login again updates that account. | `{"status":"failed","reason":"error",...}` | +| `5` | The user denied the request on GitHub (`access_denied`). Copilot only. Nothing is saved. | `{"status":"failed","reason":"denied",...}` | +| `6` | The sign-in expired before approval: the GitHub device code (`expired_token`), or the Grok or Kiro sign-in window ran out. Nothing is saved. | `{"status":"failed","reason":"expired",...}` | +| `7` | Stopped with Ctrl-C before the credential was saved. Nothing is saved. | `{"status":"cancelled"}` | +| `8` | The provider's CLI is not installed: `grok` (Grok) or `kiro-cli` (Kiro). Copilot never uses this code. Nothing is saved. | `{"status":"failed","reason":"missing_binary",...}` | | `64` | Invalid arguments, including an unsupported `--provider`. No network call is made. | None; the error goes to stderr | -Only exit code `0` guarantees both that the credential was saved and that Copilot is enabled. Codes `5`, `6`, `7` and `64` mean nothing was saved; code `1` can follow a saved account, as described above. +Only exit code `0` guarantees both that the sign-in finished and that the provider is enabled. For Kiro, kiro-cli keeps the credential, so `0` means kiro-cli reported a successful sign-in. Codes `5`, `6`, `7`, `8` and `64` mean nothing was saved; code `1` can follow a finished sign-in, as described above. Polling follows the interval GitHub returns. Each `slow_down` response adds 5 seconds for the rest of the login. GitHub's `access_denied` and `expired_token` responses end the login with the friendly errors above, and nothing is saved. -Other providers fail before any network call: `Login is only supported for copilot. Use --provider copilot.` +Other providers fail before any network call: `Login is supported for copilot, grok and kiro. Use --provider copilot, grok or kiro.` -The login always signs in on github.com. This port has no GitHub Enterprise host option for the login flow, and the desktop app's sign-in behaves the same way. +The Copilot login always signs in on github.com. This port has no GitHub Enterprise host option for the login flow, and the desktop app's sign-in behaves the same way. For tests and proof runs, `CODEXBAR_GITHUB_BASE_URL` replaces both GitHub roots with one origin. The device-code requests (`/login/device/code`, `/login/oauth/access_token`) and the identity and usage requests (`/user`, `/copilot_internal/user`) all go there. Only `https://` URLs, or loopback `http://` URLs (`127.0.0.1`, `localhost`, `[::1]`), with no path, query or credentials are accepted. Anything else is ignored with a warning, and the default GitHub roots are used. The variable also applies to the desktop app's Copilot sign-in and usage fetches, so unset it for normal use. diff --git a/rust/examples/fake_grok_cli.rs b/rust/examples/fake_grok_cli.rs new file mode 100644 index 00000000000..e61c16818a4 --- /dev/null +++ b/rust/examples/fake_grok_cli.rs @@ -0,0 +1,24 @@ +//! Scripted stand-in for `grok login --oauth --leader-socket `, used by +//! the CLI proof runs. It never writes a login file, so it cannot touch the +//! real Grok account store. +//! +//! The mode comes from the `FAKE_GROK_MODE` environment variable: +//! - `wait` (default): sleeps as if the browser sign-in were still open. +//! - `fail`: exits 3 without signing in. +//! +//! This is a cargo example, not a shipped binary. `cargo test` builds it into +//! `target//examples`. + +use std::process::ExitCode; +use std::thread; +use std::time::Duration; + +fn main() -> ExitCode { + match std::env::var("FAKE_GROK_MODE").unwrap_or_default().as_str() { + "fail" => ExitCode::from(3), + _ => { + thread::sleep(Duration::from_secs(60)); + ExitCode::SUCCESS + } + } +} diff --git a/rust/examples/fake_kiro_cli.rs b/rust/examples/fake_kiro_cli.rs new file mode 100644 index 00000000000..de51fab4606 --- /dev/null +++ b/rust/examples/fake_kiro_cli.rs @@ -0,0 +1,44 @@ +//! Scripted stand-in for `kiro-cli login`, used by the login runner tests and +//! the CLI proof runs. +//! +//! Usage: `fake_kiro_cli login `, or `fake_kiro_cli login` with the mode +//! in the `FAKE_KIRO_MODE` environment variable (what `config login` runs). +//! +//! Modes: +//! - `success`: prints a sign-in URL and the success marker, then exits 0. +//! - `wait`: prints a sign-in URL, then waits for the browser (sleeps). +//! - `silent`: prints nothing and sleeps. +//! - anything else: exits 3. +//! +//! This is a cargo example, not a shipped binary. `cargo test` builds it next to +//! the test binary, so no dependency or PATH setup is needed. + +use std::process::ExitCode; +use std::thread; +use std::time::Duration; + +const SIGN_IN_URL: &str = "https://example.com/device?code=ABCD-1234"; + +fn main() -> ExitCode { + let mode = std::env::args() + .nth(2) + .or_else(|| std::env::var("FAKE_KIRO_MODE").ok()) + .unwrap_or_default(); + match mode.as_str() { + "success" => { + println!("Open {SIGN_IN_URL} to continue"); + println!("Successfully logged in"); + ExitCode::SUCCESS + } + "wait" => { + println!("Open {SIGN_IN_URL} to continue"); + thread::sleep(Duration::from_secs(60)); + ExitCode::SUCCESS + } + "silent" => { + thread::sleep(Duration::from_secs(60)); + ExitCode::SUCCESS + } + _ => ExitCode::from(3), + } +} diff --git a/rust/src/cli/config.rs b/rust/src/cli/config.rs index 781eac16a03..1ccac68703c 100755 --- a/rust/src/cli/config.rs +++ b/rust/src/cli/config.rs @@ -18,6 +18,7 @@ use crate::providers::copilot::login::{ }; use crate::settings::{ApiKeys, ManualCookies, PreferencesDocument, Settings}; +mod cli_login; mod login; pub use login::ConfigLoginError; @@ -86,7 +87,7 @@ pub enum ConfigCommand { #[arg(long = "no-enable")] no_enable: bool, }, - /// Log in to a provider through its own device flow (GitHub Copilot only) + /// Log in to a provider through its own sign-in (GitHub Copilot, Grok, Kiro) Login { #[command(flatten)] target: ConfigProviderArg, @@ -793,38 +794,61 @@ fn resolve_api_key_input(api_key: Option<&str>, read_from_stdin: bool) -> anyhow Ok(value) } -/// Run `config login`. Only GitHub Copilot has a CLI login flow. +/// Run `config login`. GitHub Copilot uses a device code; Grok and Kiro run +/// their own CLI sign-in. +/// +/// Output goes through `std::io::stdout()` rather than a held lock, because +/// the Kiro sign-in link is written from the login thread. async fn login(provider: &str, output: ConfigOutputArgs) -> Result<(), ConfigLoginError> { let id = parse_provider(provider).map_err(ConfigLoginError::usage)?; - if id != ProviderId::Copilot { - return Err(ConfigLoginError::usage( - "Login is only supported for copilot. Use --provider copilot.", - )); - } - let flow = CopilotDeviceFlow::new(); - let api = CopilotApi::new(); - let store = TokenAccountStore::new(); - run_copilot_login( - &flow, - &api, - &store, - output.is_json(), - &mut std::io::stdout().lock(), - login::ctrl_c(), - enable_copilot, - ) - .await - .map(|_| ()) + let json = output.is_json(); + let out = &mut std::io::stdout(); + match id { + ProviderId::Copilot => { + let flow = CopilotDeviceFlow::new(); + let api = CopilotApi::new(); + let store = TokenAccountStore::new(); + run_copilot_login(&flow, &api, &store, json, out, login::ctrl_c(), || { + enable_provider(ProviderId::Copilot) + }) + .await + .map(|_| ()) + } + ProviderId::Grok => cli_login::login_grok(json, out).await, + ProviderId::Kiro => cli_login::login_kiro(json, out).await, + _ => Err(ConfigLoginError::usage( + "Login is supported for copilot, grok and kiro. Use --provider copilot, grok or kiro.", + )), + } } -/// Turns the Copilot provider on after a saved login. Only `config login` -/// does this; the desktop sign-in keeps its current behavior. -fn enable_copilot() -> anyhow::Result<()> { +/// Turns `id` on after a saved login. Only `config login` does this; the +/// desktop sign-in keeps its current behavior. +fn enable_provider(id: ProviderId) -> anyhow::Result<()> { let mut settings = Settings::load(); - settings.enable_provider(ProviderId::Copilot); + settings.enable_provider(id); settings.save() } +/// The error for a sign-in that finished but whose settings write failed. +/// `saved_account` is true when CodexBar itself stored the credential, so the +/// message tells the user the account is already there. +fn enable_failed( + id: ProviderId, + saved_account: bool, + error: impl std::fmt::Display, +) -> ConfigLoginError { + let done = if saved_account { + "Signed in and saved the account" + } else { + "Signed in" + }; + ConfigLoginError::other(format!( + "{done}, but could not enable {}: {error}", + id.display_name() + )) +} + /// Runs the Copilot device login and writes progress to `out`. In JSON mode /// each event is one line; in text mode the URL and code are two lines. /// @@ -846,11 +870,7 @@ async fn run_copilot_login( saved = copilot_steps(flow, api, store, json, out) => saved, }; let result = result.and_then(|mut saved| { - enable().map_err(|error| { - ConfigLoginError::other(format!( - "Signed in and saved the account, but could not enable Copilot: {error}" - )) - })?; + enable().map_err(|error| enable_failed(ProviderId::Copilot, true, error))?; saved.enabled = true; Ok(saved) }); diff --git a/rust/src/cli/config/cli_login.rs b/rust/src/cli/config/cli_login.rs new file mode 100644 index 00000000000..5937e335921 --- /dev/null +++ b/rust/src/cli/config/cli_login.rs @@ -0,0 +1,212 @@ +//! `config login` for the Grok and Kiro CLIs. +//! +//! Each flow runs the provider's own sign-in command and waits for it to +//! finish. Grok's credential is saved to the CodexBar account store, the same +//! way the desktop sign-in saves it. Kiro keeps its credential inside +//! `kiro-cli`, so for Kiro "saved" means the CLI reported a successful +//! sign-in. Neither flow prints a token or the CLI's own output. + +use std::io::{self, Write}; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::time::Duration; + +use super::login::{ + ConfigLoginError, ConfigLoginEvent, LoginFailure, cancelled, ctrl_c, report_outcome, + write_event, +}; +use crate::core::ProviderId; +use crate::login::{LoginOutcome, LoginPhase, run_kiro_login_with}; +use crate::providers::grok::accounts as grok_accounts; + +/// How long a stopped CLI gets to exit before the Ctrl-C path returns. +const STOP_WAIT: Duration = Duration::from_secs(10); + +/// How long a Kiro sign-in may wait for the browser before it is stopped. The +/// desktop sign-in uses the same 120 seconds. +const KIRO_TIMEOUT_SECS: u64 = 120; + +/// Runs `config login --provider grok`. The provider is turned on only after +/// the credential is saved. +pub async fn login_grok(json: bool, out: &mut impl Write) -> Result<(), ConfigLoginError> { + let result = grok_sign_in(json, out).await; + let result = result.and_then(|()| { + super::enable_provider(ProviderId::Grok) + .map_err(|e| super::enable_failed(ProviderId::Grok, true, e)) + }); + report_outcome(json, out, result) +} + +/// Runs `config login --provider kiro`. The provider is turned on only after +/// `kiro-cli` reports a successful sign-in. +pub async fn login_kiro(json: bool, out: &mut impl Write) -> Result<(), ConfigLoginError> { + let result = kiro_sign_in(json, out).await; + let result = result.and_then(|()| { + super::enable_provider(ProviderId::Kiro) + .map_err(|e| super::enable_failed(ProviderId::Kiro, false, e)) + }); + report_outcome(json, out, result) +} + +async fn grok_sign_in(json: bool, out: &mut impl Write) -> Result<(), ConfigLoginError> { + write_event(out, json, &ConfigLoginEvent::pending_browser())?; + grok_accounts::begin_login(); + // Folders left by an earlier crashed login are removed. A failure here + // does not stop this login. + let _cleanup = grok_accounts::cleanup_abandoned_logins(); + + let mut task = tokio::task::spawn_blocking(grok_accounts::login); + let joined = tokio::select! { + biased; + () = ctrl_c() => None, + joined = &mut task => Some(joined), + }; + let Some(joined) = joined else { + grok_accounts::cancel_login(); + // The blocking poll loop sees the flag within about 150 ms and stops + // the CLI. The wait is bounded so a stuck thread cannot hang Ctrl-C. + let _stopped = tokio::time::timeout(STOP_WAIT, task).await; + return Err(cancelled()); + }; + let saved = joined + .map_err(ConfigLoginError::other)? + .map_err(grok_login_error)?; + grok_accounts::save_login(saved) + .await + .map_err(ConfigLoginError::other) +} + +async fn kiro_sign_in(json: bool, out: &mut impl Write) -> Result<(), ConfigLoginError> { + write_event(out, json, &ConfigLoginEvent::pending_browser())?; + + let cancel = Arc::new(AtomicBool::new(false)); + let worker_cancel = Arc::clone(&cancel); + // The sign-in link is printed as soon as the CLI shows it. The callback + // runs on the login thread, so it writes to stdout itself. + let on_link = move |link: &str| { + let mut stdout = io::stdout(); + if let Err(error) = write_event(&mut stdout, json, &ConfigLoginEvent::pending_url(link)) { + tracing::warn!("Could not write the Kiro sign-in link: {error}"); + } + }; + let mut task = tokio::task::spawn_blocking(move || { + run_kiro_login_with( + KIRO_TIMEOUT_SECS, + worker_cancel, + |_: LoginPhase| {}, + on_link, + ) + }); + let joined = tokio::select! { + biased; + () = ctrl_c() => None, + joined = &mut task => Some(joined), + }; + let Some(joined) = joined else { + cancel.store(true, Ordering::SeqCst); + // The watchdog stops the CLI within about 100 ms. The wait is bounded + // so a stuck pipe cannot hang Ctrl-C. + let _stopped = tokio::time::timeout(STOP_WAIT, task).await; + return Err(cancelled()); + }; + let result = joined.map_err(ConfigLoginError::other)?; + kiro_outcome(result.outcome) +} + +/// Maps a Grok sign-in error to a `config login` failure. A missing CLI has +/// its own exit code, and a sign-in that ran out of time counts as expired. +fn grok_login_error(error: io::Error) -> ConfigLoginError { + let failure = match error.kind() { + io::ErrorKind::NotFound => LoginFailure::MissingBinary, + io::ErrorKind::TimedOut => LoginFailure::Expired, + _ => LoginFailure::Other, + }; + ConfigLoginError::new(failure, error.to_string()) +} + +/// Maps the Kiro CLI outcome to a `config login` result. Only a successful +/// sign-in is `Ok`. +fn kiro_outcome(outcome: LoginOutcome) -> Result<(), ConfigLoginError> { + match outcome { + LoginOutcome::Success => Ok(()), + LoginOutcome::MissingBinary => Err(ConfigLoginError::new( + LoginFailure::MissingBinary, + "Kiro CLI (kiro-cli) was not found. Install Kiro CLI, then try again.", + )), + LoginOutcome::TimedOut => Err(ConfigLoginError::new( + LoginFailure::Expired, + "Kiro sign-in timed out. Please try again.", + )), + LoginOutcome::Cancelled => Err(cancelled()), + LoginOutcome::Failed { status } => Err(ConfigLoginError::other(format!( + "Kiro sign-in did not complete (exit code {status})." + ))), + LoginOutcome::LaunchFailed(error) => Err(ConfigLoginError::other(format!( + "Could not start kiro-cli: {error}" + ))), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn grok_missing_binary_maps_to_missing_binary() { + let error = grok_login_error(io::Error::new( + io::ErrorKind::NotFound, + "Grok CLI was not found.", + )); + assert_eq!(error.failure(), LoginFailure::MissingBinary); + assert_eq!(error.to_string(), "Grok CLI was not found."); + } + + #[test] + fn grok_timeout_maps_to_expired() { + let error = grok_login_error(io::Error::new(io::ErrorKind::TimedOut, "timed out")); + assert_eq!(error.failure(), LoginFailure::Expired); + } + + #[test] + fn other_grok_errors_are_generic_failures() { + let error = grok_login_error(io::Error::other("disk full")); + assert_eq!(error.failure(), LoginFailure::Other); + } + + #[test] + fn kiro_success_is_ok() { + assert!(kiro_outcome(LoginOutcome::Success).is_ok()); + } + + #[test] + fn kiro_missing_binary_maps_to_missing_binary() { + let error = kiro_outcome(LoginOutcome::MissingBinary).expect_err("missing"); + assert_eq!(error.failure(), LoginFailure::MissingBinary); + } + + #[test] + fn kiro_timeout_maps_to_expired() { + let error = kiro_outcome(LoginOutcome::TimedOut).expect_err("timed out"); + assert_eq!(error.failure(), LoginFailure::Expired); + } + + #[test] + fn kiro_cancel_maps_to_cancelled() { + let error = kiro_outcome(LoginOutcome::Cancelled).expect_err("cancelled"); + assert_eq!(error.failure(), LoginFailure::Cancelled); + } + + #[test] + fn kiro_nonzero_exit_is_a_generic_failure() { + let error = kiro_outcome(LoginOutcome::Failed { status: 3 }).expect_err("failed"); + assert_eq!(error.failure(), LoginFailure::Other); + assert!(error.to_string().contains("exit code 3")); + } + + #[test] + fn kiro_launch_failure_is_a_generic_failure() { + let error = + kiro_outcome(LoginOutcome::LaunchFailed("denied".to_string())).expect_err("launch"); + assert_eq!(error.failure(), LoginFailure::Other); + } +} diff --git a/rust/src/cli/config/login.rs b/rust/src/cli/config/login.rs index d879ff661a7..b1adcf4a82d 100644 --- a/rust/src/cli/config/login.rs +++ b/rust/src/cli/config/login.rs @@ -140,6 +140,19 @@ impl ConfigLoginEvent { } } + /// `authorization_pending` for a login that shows a sign-in URL but no code. + pub fn pending_url(verification_uri: &str) -> Self { + Self { + verification_uri: Some(verification_uri.to_string()), + ..Self::status("authorization_pending") + } + } + + /// `authorization_pending` for a login that opens the browser itself. + pub fn pending_browser() -> Self { + Self::status("authorization_pending") + } + /// The `failed` event. It carries the failure reason and message only. pub fn failed(error: &ConfigLoginError) -> Self { Self { @@ -150,9 +163,15 @@ impl ConfigLoginEvent { } } +/// The error for a Ctrl-C during a login. Nothing was saved. +pub fn cancelled() -> ConfigLoginError { + ConfigLoginError::new(LoginFailure::Cancelled, "Login cancelled.") +} + /// Writes one event. JSON mode prints one object per line. Text mode prints -/// the URL and code for `authorization_pending` and `Connected` for -/// `authorized`; other events print nothing and their error goes to stderr. +/// the URL (and the code, when there is one) for `authorization_pending`, +/// `Connected` for `authorized`, and nothing for the rest. Failures go to +/// stderr from the caller. pub fn write_event( out: &mut impl Write, json: bool, @@ -161,8 +180,15 @@ pub fn write_event( if json { let line = serde_json::to_string(event).map_err(ConfigLoginError::other)?; writeln!(out, "{line}").map_err(ConfigLoginError::other)?; - } else if let (Some(url), Some(code)) = (&event.verification_uri, &event.user_code) { - writeln!(out, "{url}\n{code}").map_err(ConfigLoginError::other)?; + } else if let Some(url) = &event.verification_uri { + match &event.user_code { + Some(code) => writeln!(out, "{url}\n{code}"), + None => writeln!(out, "{url}"), + } + .map_err(ConfigLoginError::other)?; + } else if event.status == "authorization_pending" { + writeln!(out, "Waiting for the browser sign-in to finish.") + .map_err(ConfigLoginError::other)?; } else if event.status == "authorized" { writeln!(out, "Connected").map_err(ConfigLoginError::other)?; } diff --git a/rust/src/cli/config/tests.rs b/rust/src/cli/config/tests.rs index 7166ab9636f..91fb1cbdd13 100644 --- a/rust/src/cli/config/tests.rs +++ b/rust/src/cli/config/tests.rs @@ -494,11 +494,27 @@ async fn config_login_rejects_other_providers_before_any_io() { .expect_err("cursor has no CLI login"); assert_eq!( error.to_string(), - "Login is only supported for copilot. Use --provider copilot." + "Login is supported for copilot, grok and kiro. Use --provider copilot, grok or kiro." ); assert_eq!(error.exit_code(), crate::cli::exit_codes::USAGE_ERROR); } +#[test] +fn enable_failure_says_whether_codexbar_saved_the_account() { + let grok = super::enable_failed(ProviderId::Grok, true, "settings write failed"); + assert_eq!( + grok.to_string(), + "Signed in and saved the account, but could not enable Grok: settings write failed" + ); + assert_eq!(grok.exit_code(), 1); + + let kiro = super::enable_failed(ProviderId::Kiro, false, "settings write failed"); + assert_eq!( + kiro.to_string(), + "Signed in, but could not enable Kiro: settings write failed" + ); +} + mod copilot_login_output { use std::cell::Cell; use std::future::Future; diff --git a/rust/src/login.rs b/rust/src/login.rs index cedb664b975..08d3e93e17b 100755 --- a/rust/src/login.rs +++ b/rust/src/login.rs @@ -7,8 +7,13 @@ use std::io::{BufRead, BufReader, Read}; #[cfg(windows)] use std::os::windows::process::CommandExt; use std::process::{Child, Command, Stdio}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; +/// How often the watchdog and the exit wait look at the child. +const POLL_INTERVAL: Duration = Duration::from_millis(100); + /// Result of a login attempt #[derive(Debug, Clone)] pub struct LoginResult { @@ -22,9 +27,13 @@ pub struct LoginResult { pub enum LoginOutcome { Success, TimedOut, - Failed { status: i32 }, + Failed { + status: i32, + }, MissingBinary, LaunchFailed(String), + /// The cancel flag was set before the CLI finished. The CLI was stopped. + Cancelled, } /// Phase of the login process @@ -40,6 +49,31 @@ pub enum LoginPhase { pub async fn run_kiro_login(timeout_secs: u64, on_phase: F) -> LoginResult where F: Fn(LoginPhase) + Send + 'static, +{ + run_kiro_login_with( + timeout_secs, + Arc::new(AtomicBool::new(false)), + on_phase, + |_| {}, + ) +} + +/// Run Kiro CLI login with a cancel flag and a callback for the sign-in link. +/// +/// This blocks until the CLI finishes. Callers that need to race a Ctrl-C run +/// it on a blocking thread. Setting `cancel` stops the CLI and returns +/// [`LoginOutcome::Cancelled`]. The CLI is also stopped after `timeout_secs`, +/// even when it prints nothing. `on_link` runs as soon as the CLI prints a URL, +/// before the login finishes. +pub fn run_kiro_login_with( + timeout_secs: u64, + cancel: Arc, + on_phase: F, + on_link: L, +) -> LoginResult +where + F: Fn(LoginPhase) + Send + 'static, + L: Fn(&str) + Send + 'static, { // Use Kiro's own binary resolver which checks well-known Windows install // locations in addition to PATH. @@ -52,26 +86,30 @@ where &binary_path, &["login"], timeout_secs, - on_phase, + &cancel, + &on_phase, + &on_link, &[ "Successfully logged in", "Login successful", "Logged in successfully", ], ) - .await } /// Generic CLI login runner (uses a pre-resolved binary path) -async fn run_cli_login_path( +fn run_cli_login_path( binary_path: &std::path::Path, args: &[&str], timeout_secs: u64, - on_phase: F, + cancel: &Arc, + on_phase: &F, + on_link: &L, success_markers: &[&str], ) -> LoginResult where - F: Fn(LoginPhase) + Send + 'static, + F: Fn(LoginPhase), + L: Fn(&str), { on_phase(LoginPhase::Requesting); @@ -79,18 +117,47 @@ where Ok(c) => c, Err(e) => return launch_failed_result(e), }; + let stdout = child.stdout.take(); + let stderr = child.stderr.take(); - let mut state = CliLoginState::new(timeout_secs, &on_phase, success_markers); + // The watchdog owns a handle to the child so it can stop the CLI on cancel + // or timeout, even while the read below is blocked on a silent pipe. + let child = Arc::new(Mutex::new(child)); + let done = Arc::new(AtomicBool::new(false)); + let timed_out = Arc::new(AtomicBool::new(false)); + spawn_watchdog( + Arc::clone(&child), + Arc::clone(cancel), + Arc::clone(&timed_out), + Arc::clone(&done), + Duration::from_secs(timeout_secs), + ); - if let Some(outcome) = read_login_stream(child.stdout.take(), &mut state) { - return stop_child_with_outcome(&mut child, state, outcome); - } + let mut state = CliLoginState::new(timeout_secs, on_phase, on_link, success_markers); - if let Some(outcome) = read_login_stream(child.stderr.take(), &mut state) { - return stop_child_with_outcome(&mut child, state, outcome); - } + let read_outcome = + read_login_stream(stdout, &mut state).or_else(|| read_login_stream(stderr, &mut state)); + let outcome = match read_outcome { + Some(outcome) => { + kill_child(&child); + outcome + } + None => wait_for_login_exit(&child, on_phase), + }; + done.store(true, Ordering::SeqCst); - wait_for_login_exit(child, state, &on_phase) + // A finished sign-in keeps its result. Otherwise a cancel or a timeout + // explains why the CLI stopped. + let outcome = if matches!(outcome, LoginOutcome::Success) { + outcome + } else if cancel.load(Ordering::SeqCst) { + LoginOutcome::Cancelled + } else if timed_out.load(Ordering::SeqCst) { + LoginOutcome::TimedOut + } else { + outcome + }; + state.into_result(outcome) } fn missing_binary_result(binary: &str) -> LoginResult { @@ -121,6 +188,40 @@ fn spawn_login_process(binary_path: &std::path::Path, args: &[&str]) -> Result>, + cancel: Arc, + timed_out: Arc, + done: Arc, + timeout: Duration, +) { + std::thread::spawn(move || { + let start = Instant::now(); + while !done.load(Ordering::SeqCst) { + if cancel.load(Ordering::SeqCst) { + kill_child(&child); + return; + } + if start.elapsed() > timeout { + timed_out.store(true, Ordering::SeqCst); + kill_child(&child); + return; + } + std::thread::sleep(POLL_INTERVAL); + } + }); +} + +/// Best-effort teardown; the child may already have exited. +fn kill_child(child: &Mutex) { + if let Ok(mut child) = child.lock() { + let _killed = child.kill(); + } +} + struct CliLoginState<'a, F> where F: Fn(LoginPhase), @@ -129,6 +230,7 @@ where auth_link: Option, url_regex: Regex, on_phase: &'a F, + on_link: &'a dyn Fn(&str), success_markers: &'a [&'a str], start: Instant, timeout: Duration, @@ -138,12 +240,18 @@ impl<'a, F> CliLoginState<'a, F> where F: Fn(LoginPhase), { - fn new(timeout_secs: u64, on_phase: &'a F, success_markers: &'a [&'a str]) -> Self { + fn new( + timeout_secs: u64, + on_phase: &'a F, + on_link: &'a dyn Fn(&str), + success_markers: &'a [&'a str], + ) -> Self { Self { output: String::new(), auth_link: None, url_regex: Regex::new(r"https?://[A-Za-z0-9._~:/?#\[\]@!$&'()*+,;=%-]+").unwrap(), on_phase, + on_link, success_markers, start: Instant::now(), timeout: Duration::from_secs(timeout_secs), @@ -180,6 +288,7 @@ where }; self.auth_link = Some(m.as_str().to_string()); + (self.on_link)(m.as_str()); (self.on_phase)(LoginPhase::WaitingBrowser); // Best-effort browser open; the link stays on screen for manual use. let _opened_browser = open::that(m.as_str()); @@ -209,38 +318,124 @@ where .find_map(|line| state.handle_line(&line)) } -fn stop_child_with_outcome( - child: &mut Child, - state: CliLoginState<'_, F>, - outcome: LoginOutcome, -) -> LoginResult +/// Waits for the CLI to exit without holding the lock across the wait, so the +/// watchdog can still kill it. +fn wait_for_login_exit(child: &Mutex, on_phase: &F) -> LoginOutcome where F: Fn(LoginPhase), { - // Best-effort teardown; the child may already have exited. - let _killed = child.kill(); - state.into_result(outcome) -} - -fn wait_for_login_exit( - mut child: Child, - state: CliLoginState<'_, F>, - on_phase: &F, -) -> LoginResult -where - F: Fn(LoginPhase), -{ - match child.wait() { - Ok(status) => { - if status.success() { - on_phase(LoginPhase::Complete); - state.into_result(LoginOutcome::Success) - } else { - state.into_result(LoginOutcome::Failed { + loop { + let status = match child.lock() { + Ok(mut child) => child.try_wait(), + Err(_) => { + return LoginOutcome::LaunchFailed("login process lock poisoned".to_string()); + } + }; + match status { + Ok(Some(status)) => { + if status.success() { + on_phase(LoginPhase::Complete); + return LoginOutcome::Success; + } + return LoginOutcome::Failed { status: status.code().unwrap_or(-1), - }) + }; } + Ok(None) => std::thread::sleep(POLL_INTERVAL), + Err(e) => return LoginOutcome::LaunchFailed(e.to_string()), } - Err(e) => state.into_result(LoginOutcome::LaunchFailed(e.to_string())), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + /// `fake_kiro_cli` is a cargo example. `cargo test` builds it into + /// `target//examples`, next to the `deps` folder of this test binary. + fn fake_kiro_cli() -> PathBuf { + let exe = std::env::current_exe().expect("test binary path"); + let profile_dir = exe + .parent() + .and_then(std::path::Path::parent) + .expect("profile directory"); + profile_dir + .join("examples") + .join(format!("fake_kiro_cli{}", std::env::consts::EXE_SUFFIX)) + } + + fn run( + binary: &std::path::Path, + mode: &str, + timeout_secs: u64, + cancel: Arc, + ) -> LoginResult { + let markers = ["Successfully logged in"]; + run_cli_login_path( + binary, + &["login", mode], + timeout_secs, + &cancel, + &|_: LoginPhase| {}, + &|_: &str| {}, + &markers, + ) + } + + #[test] + fn success_marker_finishes_and_keeps_the_sign_in_link() { + let binary = fake_kiro_cli(); + assert!( + binary.is_file(), + "build the example first: cargo build --manifest-path rust/Cargo.toml --example fake_kiro_cli" + ); + let result = run(&binary, "success", 30, Arc::new(AtomicBool::new(false))); + assert!(matches!(result.outcome, LoginOutcome::Success)); + assert_eq!( + result.auth_link.as_deref(), + Some("https://example.com/device?code=ABCD-1234") + ); + } + + #[test] + fn nonzero_exit_is_a_failure_with_its_status() { + let binary = fake_kiro_cli(); + let result = run(&binary, "fail", 30, Arc::new(AtomicBool::new(false))); + assert!(matches!(result.outcome, LoginOutcome::Failed { status: 3 })); + } + + #[test] + fn cancel_stops_a_cli_that_is_waiting_for_the_browser() { + let binary = fake_kiro_cli(); + let cancel = Arc::new(AtomicBool::new(false)); + let setter = Arc::clone(&cancel); + let started = Instant::now(); + std::thread::spawn(move || { + std::thread::sleep(Duration::from_millis(300)); + setter.store(true, Ordering::SeqCst); + }); + let result = run(&binary, "wait", 30, cancel); + assert!(matches!(result.outcome, LoginOutcome::Cancelled)); + assert!( + started.elapsed() < Duration::from_secs(20), + "cancel must not wait for the CLI" + ); + } + + #[test] + fn a_silent_cli_times_out() { + let binary = fake_kiro_cli(); + let started = Instant::now(); + let result = run(&binary, "silent", 1, Arc::new(AtomicBool::new(false))); + assert!(matches!(result.outcome, LoginOutcome::TimedOut)); + assert!(started.elapsed() < Duration::from_secs(20)); + } + + #[test] + fn a_missing_binary_is_a_launch_failure_not_a_hang() { + let missing = std::env::temp_dir().join("codexbar-no-such-kiro-cli.exe"); + let result = run(&missing, "success", 30, Arc::new(AtomicBool::new(false))); + assert!(matches!(result.outcome, LoginOutcome::LaunchFailed(_))); } } diff --git a/rust/src/providers/grok/accounts.rs b/rust/src/providers/grok/accounts.rs index ccdebeed52a..566f86b02f1 100644 --- a/rust/src/providers/grok/accounts.rs +++ b/rust/src/providers/grok/accounts.rs @@ -18,11 +18,18 @@ use crate::secure_file; use super::text_field; -pub use login::{begin_login, cancel_login, cleanup_abandoned_logins, login}; +pub use login::{begin_login, cancel_login, cleanup_abandoned_logins, executable, login}; pub use orca::select_account as select_orca_account; pub static CREDENTIAL_OPERATION: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); +/// Stores a finished Grok sign-in in the saved-account list. The desktop add +/// flow and `config login --provider grok` both save through here. +pub async fn save_login(login: SavedLogin) -> io::Result<()> { + let _credentials = CREDENTIAL_OPERATION.lock().await; + AccountManager::new()?.import(login) +} + #[derive(Clone, Debug, Serialize)] #[serde(rename_all = "camelCase")] pub struct GrokAccount { diff --git a/rust/src/providers/grok/accounts/login.rs b/rust/src/providers/grok/accounts/login.rs index 72e994fd58b..d6d20ab7d10 100644 --- a/rust/src/providers/grok/accounts/login.rs +++ b/rust/src/providers/grok/accounts/login.rs @@ -22,7 +22,19 @@ pub fn begin_login() { CANCEL.store(false, Ordering::SeqCst); } +/// Explicit path to the Grok CLI. Like `CODEXBAR_KIRO_CLI_PATH`, it is used only +/// when it names an existing file. +const GROK_CLI_PATH_ENV: &str = "CODEXBAR_GROK_CLI_PATH"; + +/// Finds the Grok CLI. A missing binary is `io::ErrorKind::NotFound`, so callers +/// can tell it apart from other failures. pub fn executable() -> io::Result { + if let Ok(raw) = std::env::var(GROK_CLI_PATH_ENV) { + let path = PathBuf::from(raw.trim()); + if !raw.trim().is_empty() && path.is_file() { + return Ok(path); + } + } if let Some(home) = dirs::home_dir() { let native = home.join(if cfg!(windows) { ".grok/bin/grok.exe" @@ -34,7 +46,10 @@ pub fn executable() -> io::Result { } } which::which("grok").map_err(|_| { - io::Error::other("Grok CLI was not found. Install Grok Build, then try again.") + io::Error::new( + io::ErrorKind::NotFound, + "Grok CLI was not found. Install Grok Build, then try again.", + ) }) } @@ -155,11 +170,14 @@ fn wait_for_login(child: &mut Child, home: &Path, timeout: Duration) -> io::Resu if cancelled || start.elapsed() > timeout { let _kill = child.kill(); let _reap = child.wait(); - return Err(io::Error::other(if cancelled { - "Grok sign-in cancelled." + return Err(if cancelled { + io::Error::other("Grok sign-in cancelled.") } else { - "Grok sign-in timed out. Please try again." - })); + io::Error::new( + io::ErrorKind::TimedOut, + "Grok sign-in timed out. Please try again.", + ) + }); } std::thread::sleep(Duration::from_millis(150)); }