diff --git a/README.md b/README.md index c0baf6c..96306a3 100644 --- a/README.md +++ b/README.md @@ -33,15 +33,13 @@ cp target/release/hotdata /usr/local/bin/hotdata ## Connect -Run either of the following (they are equivalent): +Run: ```sh hotdata auth login -# or -hotdata auth ``` -This launches a browser window where you can authorize the CLI to access your Hotdata account. +This launches a browser window where you can authorize the CLI to access your Hotdata account. (Bare `hotdata auth` prints the `auth` subcommand help.) Alternatively, authenticate with an API key using the `--api-key` flag: @@ -62,7 +60,7 @@ API key priority (lowest to highest): config file → `HOTDATA_API_KEY` env var | Command | Subcommands | Description | | :-- | :-- | :-- | -| `auth` | `login`, `status`, `logout` | `login` or bare `auth` opens browser login; `status` / `logout` manage the saved profile | +| `auth` | `login`, `status`, `logout` | `login` opens browser login; `status` / `logout` manage the saved profile | | `workspaces` | `list`, `set` | Manage workspaces | | `connections` | `list`, `create`, `refresh`, `new` | Manage connections | | `databases` | `list`, `create`, `delete`, `tables` | Managed databases (create and load tables via parquet) | diff --git a/skills/hotdata/SKILL.md b/skills/hotdata/SKILL.md index a049bf7..5013197 100644 --- a/skills/hotdata/SKILL.md +++ b/skills/hotdata/SKILL.md @@ -27,10 +27,10 @@ Install all skills with **`hotdata skills install`**. Load specialized skills on ## Authentication -Run **`hotdata auth login`** (or **`hotdata auth`** with no subcommand—same behavior) to authenticate via browser login. Config is stored in `~/.hotdata/config.yml`. +Run **`hotdata auth login`** to authenticate via browser login. Config is stored in `~/.hotdata/config.yml`. API key resolution (lowest to highest priority): -1. Config file (saved by `hotdata auth login` / `hotdata auth`) +1. Config file (saved by `hotdata auth login`) 2. `HOTDATA_API_KEY` environment variable (or `.env` file) 3. `--api-key ` flag (works on any command) @@ -336,8 +336,7 @@ A newer release can be incompatible with the API, so in an **interactive termina ### Auth ``` -hotdata auth login # Browser-based login (same as: hotdata auth) -hotdata auth # Browser-based login (same as: hotdata auth login) +hotdata auth login # Browser-based login hotdata auth status # Check current auth status hotdata auth logout # Remove saved auth for the default profile ``` diff --git a/skills/hotdata/references/WORKFLOWS.md b/skills/hotdata/references/WORKFLOWS.md index d0f0ffe..90cde70 100644 --- a/skills/hotdata/references/WORKFLOWS.md +++ b/skills/hotdata/references/WORKFLOWS.md @@ -34,7 +34,7 @@ End-to-end checklists. Use the linked sections for command detail and guardrails **Skill:** **`hotdata`** (optional **`hotdata-analytics`** for first queries) -1. [ ] `hotdata auth login` (or `hotdata auth`) +1. [ ] `hotdata auth login` 2. [ ] `hotdata workspaces list` → `hotdata workspaces set` if not on the right workspace 3. [ ] `hotdata connections list` — note connection ids and names 4. [ ] (Optional) `hotdata connections create …` — see **`hotdata`** skill **Create a Connection** diff --git a/src/command.rs b/src/command.rs index 0acfc91..65172d2 100644 --- a/src/command.rs +++ b/src/command.rs @@ -263,7 +263,7 @@ pub enum QueryCommands { #[derive(Subcommand)] pub enum AuthCommands { - /// Log in via browser (same as `hotdata auth` with no subcommand) + /// Log in via browser Login, /// Create a new account via browser (defaults to GitHub OAuth) diff --git a/src/config.rs b/src/config.rs index 7ff1662..cf62f6c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -264,7 +264,7 @@ pub fn resolve_workspace_id( .workspaces .first() .map(|w| w.public_id.clone()) - .ok_or_else(|| "no workspace-id provided and no default workspace found. Run 'hotdata auth login' (or 'hotdata auth') or specify --workspace-id.".to_string()) + .ok_or_else(|| "no workspace-id provided and no default workspace found. Run 'hotdata auth login' or specify --workspace-id.".to_string()) } /// Global API key override set via --api-key flag. @@ -283,9 +283,7 @@ pub fn load(profile: &str) -> Result { .map_err(|e| format!("error reading config file: {e}"))?; let config_file: ConfigFile = serde_yaml::from_str(&content).unwrap_or_else(|_| { eprintln!("{}", "error parsing config file.".red()); - eprintln!( - "Run 'hotdata auth login' (or 'hotdata auth') to generate a new config file." - ); + eprintln!("Run 'hotdata auth login' to generate a new config file."); std::process::exit(1); }); config_file diff --git a/src/jwt.rs b/src/jwt.rs index e5b2514..99e1a22 100644 --- a/src/jwt.rs +++ b/src/jwt.rs @@ -12,7 +12,7 @@ //! | Access token valid for > 30 s | return it directly | //! | Access expiring or expired, refresh token valid | call `/o/token/` with `grant_type=refresh_token` | //! | Refresh token dead, `api_key` present | re-mint via `grant_type=api_token` | -//! | Refresh token dead, no `api_key` | return an error — user must `hotdata auth` again | +//! | Refresh token dead, no `api_key` | return an error — user must `hotdata auth login` again | //! //! The raw `hd_...` API token (flow 3 in the design doc) is *never* //! persisted to the session file — it stays in the main config or the @@ -302,7 +302,7 @@ pub fn ensure_access_token( // 0) An explicit identity override (`--api-key`, `HOTDATA_API_KEY`, // or `.env`) is asserting a specific identity for *this invocation*. // The on-disk session may belong to a completely different user - // from a prior `hotdata auth` and must not be reused. Mint fresh + // from a prior `hotdata auth login` and must not be reused. Mint fresh // and deliberately skip persisting so we don't clobber the // interactive session. Surface the real mint error here too — if // the override key is bad, "HTTP 401" is more useful than the @@ -358,7 +358,7 @@ pub fn ensure_access_token( // API token rejected (revoked, expired, or invalid). // Fall through to the re-auth hint — hide the raw HTTP // error from the user; the api.rs caller appends a - // `hotdata auth` hint. + // `hotdata auth login` hint. } } } @@ -435,7 +435,7 @@ impl hotdata::auth::BearerTokenProvider for CliTokenProvider { resolved.map_err(|body| { // Surface as a 401 so `Configuration::resolve_bearer_token` logs the // cause and the request proceeds to a 401 the wrapper shapes into - // the "run hotdata auth" hint (the same end-state as the old + // the "run hotdata auth login" hint (the same end-state as the old // ApiClient refresher returning None). hotdata::auth::TokenExchangeError::Status { status: 401, body } }) @@ -1235,7 +1235,7 @@ mod tests { let (_tmp, _guard) = with_temp_config_dir(); // No session, no api_key fallback -> ensure_access_token errors, the // provider maps it to a 401 so the request proceeds to the wrapper's - // "run hotdata auth" hint. + // "run hotdata auth login" hint. let profile = mock_profile("http://127.0.0.1:1"); let provider = session_provider(&profile, None); match bearer(&provider).unwrap_err() { diff --git a/src/main.rs b/src/main.rs index 31228ab..5132446 100644 --- a/src/main.rs +++ b/src/main.rs @@ -177,7 +177,9 @@ fn main() { // never blocked (see `update::should_check`). let gate_update = !matches!( &cli.command, - None | Some(Commands::Upgrade) | Some(Commands::Completions { .. }) + None | Some(Commands::Upgrade) + | Some(Commands::Completions { .. }) + | Some(Commands::Auth { command: None }) ); if gate_update { update::enforce_latest_or_exit(); @@ -191,10 +193,19 @@ fn main() { } Some(cmd) => match cmd { Commands::Auth { command } => match command { - None | Some(AuthCommands::Login) => auth::login(), + Some(AuthCommands::Login) => auth::login(), Some(AuthCommands::Register { email }) => auth::register(email), Some(AuthCommands::Status) => auth::status("default"), Some(AuthCommands::Logout) => auth::logout("default"), + None => { + use clap::CommandFactory; + let mut cmd = Cli::command(); + cmd.build(); + cmd.find_subcommand_mut("auth") + .unwrap() + .print_help() + .unwrap(); + } }, Commands::Query { sql, diff --git a/src/sdk.rs b/src/sdk.rs index 5401ad6..47b65e4 100644 --- a/src/sdk.rs +++ b/src/sdk.rs @@ -249,7 +249,7 @@ impl ApiError { /// `ApiClient::fail_response`'s formatting. /// /// On a 4xx, re-probe the auth status so a masked 404/403 is upgraded into - /// the "run hotdata auth" hint; otherwise surface the server body. Split out + /// the "run hotdata auth login" hint; otherwise surface the server body. Split out /// from [`exit`](Self::exit) so callers that want to append their own hint /// after the error (e.g. the query cross-source hint) can print, add the /// hint, then exit. @@ -483,7 +483,7 @@ impl Api { eprintln!("{}", format!("error: {e}").red()); eprintln!( "Run {} to log in, or pass --api-key.", - "hotdata auth".cyan() + "hotdata auth login".cyan() ); std::process::exit(1); } @@ -840,7 +840,8 @@ pub fn format_fail_message( if status.is_client_error() && let Some(auth::AuthStatus::Invalid(_)) = auth_status { - return "error: API key is invalid. Run 'hotdata auth login' (or 'hotdata auth') to re-authenticate.".to_string(); + return "error: API key is invalid. Run 'hotdata auth login' to re-authenticate." + .to_string(); } // A 403 ACCESS_DENIED is the allow-list guard rejecting an operation the // credential can't perform — typically a database API token (which is @@ -886,7 +887,7 @@ mod tests { Some(&AuthStatus::Invalid(401)), ); assert!(msg.contains("API key is invalid")); - assert!(msg.contains("hotdata auth login") || msg.contains("hotdata auth")); + assert!(msg.contains("hotdata auth login")); } #[test] @@ -962,7 +963,7 @@ mod tests { msg.to_lowercase().contains("database api token"), "got: {msg}" ); - assert!(msg.contains("hotdata auth"), "got: {msg}"); + assert!(msg.contains("hotdata auth login"), "got: {msg}"); } #[test] diff --git a/tests/auth_no_subcommand_help.rs b/tests/auth_no_subcommand_help.rs new file mode 100644 index 0000000..9e845bf --- /dev/null +++ b/tests/auth_no_subcommand_help.rs @@ -0,0 +1,45 @@ +//! `hotdata auth` with no subcommand prints the `auth` help (listing its +//! subcommands) instead of triggering a browser login — the behavior change +//! from PR #182. +//! +//! Runs fully offline and without credentials: bare `auth` is exempt from the +//! update gate and hits no API. Before the change this arm called +//! `auth::login()`, which prints "Opening browser to log in..." and starts a +//! local callback server; the test asserts that flow is *not* started. + +mod common; + +use common::{DEFAULT_API_URL, unauthenticated_output}; + +#[test] +fn bare_auth_prints_subcommand_help() { + let output = unauthenticated_output(DEFAULT_API_URL, &["auth"]); + + assert!( + output.status.success(), + "`hotdata auth` should exit 0 printing help\n--- stderr ---\n{}", + String::from_utf8_lossy(&output.stderr), + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + + // Clap help block, listing every auth subcommand. + assert!( + stdout.contains("Usage"), + "expected help output with a usage line; got:\n{stdout}" + ); + for sub in ["login", "register", "status", "logout"] { + assert!( + stdout.contains(sub), + "auth help missing `{sub}` subcommand; got:\n{stdout}" + ); + } + + // The login flow must NOT have started. `auth::login()` prints this banner + // before opening a browser / spinning up the callback server. + let combined = format!("{stdout}{}", String::from_utf8_lossy(&output.stderr)); + assert!( + !combined.contains("Opening browser to log in"), + "bare `auth` should print help, not start a login flow; got:\n{combined}" + ); +}