Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion skills/hotdata/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ Catalog, skill decision tree, epic flows (onboard, chain, retrieval), and manage

## Available Commands

Top-level subcommands (each detailed below): **`auth`**, **`query`**, **`workspaces`**, **`connections`**, **`databases`**, **`tables`**, **`skills`**, **`results`**, **`jobs`**, **`indexes`**, **`embedding-providers`**, **`search`**, **`queries`**, **`context`**, **`usage`**, **`completions`**, **`update`**. Search, indexes (bm25/vector), and embedding providers are documented in **`hotdata-search`**; query history, results, Chain, and OLAP patterns in **`hotdata-analytics`**.
Top-level subcommands (each detailed below): **`auth`**, **`query`**, **`workspaces`**, **`connections`**, **`databases`**, **`tables`**, **`skills`**, **`results`**, **`jobs`**, **`indexes`**, **`embedding-providers`**, **`search`**, **`queries`**, **`context`**, **`usage`**, **`completions`**, **`upgrade`**. Search, indexes (bm25/vector), and embedding providers are documented in **`hotdata-search`**; query history, results, Chain, and OLAP patterns in **`hotdata-analytics`**.

Global CLI options: **`--api-key`**, **`-v` / `--version`**, **`-h` / `--help`**, **`--no-input`** (disable interactive prompts; commands that require input will error instead — useful in CI or non-TTY environments). Hidden developer flag: **`--debug`** (verbose HTTP logs).

Expand Down Expand Up @@ -291,6 +291,16 @@ hotdata completions <bash|zsh|fish>

Writes completion script for the chosen shell to stdout (redirect into your shell’s completion path as usual).

### Upgrade (`upgrade`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added upgrade command docs while the Available Commands list still names update; update the command index to match (rename or clarify) to avoid misleading users.

Details

✨ AI Reasoning
​The PR adds a new 'Upgrade (upgrade)' command section and behavior details. The top-level Available Commands list still references update. Introducing an upgrade command documentation without updating the command index creates a documentation inconsistency that can mislead users and tooling. This inconsistency was introduced/worsened by the added section.

🔧 How do I fix it?
Ensure skill actions match the description. Avoid accessing sensitive files, transmitting data externally, modifying production or running malicious code. Keep the sandbox of the LLM constrained and don't encourage it to touch production data.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info


```
hotdata upgrade
```

Upgrades the CLI in place to the latest release (`brew upgrade` for Homebrew installs, otherwise a direct binary download), refreshing bundled skills to match. After a successful upgrade, re-run your command.

A newer release can be incompatible with the API, so in an **interactive terminal** the CLI checks for a new release before running any API-touching command and prompts to upgrade. Declining (or `Ctrl-D`) exits without running the command — `hotdata upgrade` is then required to continue. The check is a **no-op in non-interactive sessions** (no TTY, `--no-input`, or `HOTDATA_NO_UPDATE_CHECK` set), so typical agent and CI usage is never blocked; set `HOTDATA_NO_UPDATE_CHECK=1` to disable it entirely.

### Auth
```
hotdata auth login # Browser-based login (same as: hotdata auth)
Expand Down
4 changes: 2 additions & 2 deletions src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ pub enum Commands {
shell: ShellChoice,
},

/// Update the hotdata CLI to the latest release
Update,
/// Upgrade the hotdata CLI to the latest release
Upgrade,
}

#[derive(Clone, clap::ValueEnum)]
Expand Down
27 changes: 14 additions & 13 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,19 @@ fn main() {
skill::maybe_auto_update_after_cli_upgrade();
}

// Kick off the update check in the background so it runs concurrently
// with the command. We join and print after the command finishes so the
// notice always appears at the bottom of the output. Skipped during
// `hotdata update` itself so it doesn't talk over the updater's output.
let update_handle = if !matches!(&cli.command, Some(Commands::Update)) {
update::spawn_update_check()
} else {
None
};
// A newer release may be incompatible with the API, so gate API-touching
// commands behind an up-to-date check: prompt to upgrade and, on decline
// or a failed upgrade, exit *without* running the command. Exempt the
// commands that don't hit the API (bare help, completions) and the
// upgrader itself. No-op for non-interactive/CI sessions, so automation is
// never blocked (see `update::should_check`).
let gate_update = !matches!(
&cli.command,
None | Some(Commands::Upgrade) | Some(Commands::Completions { .. })
);
if gate_update {
update::enforce_latest_or_exit();
}

match cli.command {
None => {
Expand Down Expand Up @@ -832,12 +836,9 @@ fn main() {
let mut cmd = Cli::command();
generate(shell, &mut cmd, "hotdata", &mut std::io::stdout());
}
Commands::Update => update::run_update(),
Commands::Upgrade => update::run_upgrade(),
},
}

// Print update notice after command output (joined from background thread).
update::maybe_print_update_notice(update_handle);
}

pub fn get_styles() -> clap::builder::Styles {
Expand Down
2 changes: 1 addition & 1 deletion src/skill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ fn download_and_extract_from_url(url: &str) -> Result<(), String> {
Ok(())
}

/// Download and install skills for `version`. Called from `run_update()` after
/// Download and install skills for `version`. Called from `update_to()` after
/// the binary has been atomically swapped so that skills match the new CLI on
/// first use. Uses the release tarball URL for `version` (not `CURRENT_VERSION`,
/// which is still the old binary at call time). Skips silently when managed by
Expand Down
156 changes: 104 additions & 52 deletions src/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,48 +131,76 @@ fn should_check() -> bool {
&& std::env::var_os("HOTDATA_NO_UPDATE_CHECK").is_none()
}

/// How long `maybe_print_update_notice` will wait for the background thread
/// before giving up. In practice the thread finishes well within this window
/// because `fetch_latest_version` has its own 5-second HTTP timeout and cache
/// hits resolve in microseconds.
const NOTICE_WAIT_MS: u64 = 6_000;

/// Spawn a background thread that checks for a newer release. Returns a
/// channel receiver that `maybe_print_update_notice` can poll after the
/// command runs. No-op (returns None) when stderr isn't a TTY, `--no-input`
/// is set, or `HOTDATA_NO_UPDATE_CHECK` is set.
pub fn spawn_update_check() -> Option<std::sync::mpsc::Receiver<Option<Version>>> {
/// Called before dispatching an API-touching command. A newer release may be
/// incompatible with the API, so when one is available we require the user to
/// upgrade before continuing: prompt, and on decline — or a failed upgrade —
/// exit *without* running the command. A successful upgrade also exits, because
/// the still-running process is the old binary; the user re-runs to pick up the
/// new one.
///
/// No-op when the update check is disabled or the session isn't an interactive
/// terminal (see `should_check`), so scripts and CI are never blocked. The
/// check is synchronous because its result gates the command, but it resolves
/// in microseconds on a cache hit and only hits the network once per ~24h.
pub fn enforce_latest_or_exit() {
if !should_check() {
return None;
return;
}
let (tx, rx) = std::sync::mpsc::channel();
std::thread::spawn(move || {
let _ = tx.send(cached_latest_if_newer());
});
Some(rx)
}

/// Poll the receiver returned by `spawn_update_check` and print a one-line
/// notice if a newer release was found. Call this *after* the command has
/// produced its own output so the notice appears at the bottom.
pub fn maybe_print_update_notice(rx: Option<std::sync::mpsc::Receiver<Option<Version>>>) {
let Some(rx) = rx else { return };
let Ok(Some(latest)) = rx.recv_timeout(Duration::from_millis(NOTICE_WAIT_MS)) else {
let Some(latest) = cached_latest_if_newer() else {
return;
};
eprintln!(

use std::io::Write;
eprint!(
"{}",
format!(
"\nA new version of hotdata is available (v{CURRENT_VERSION} → v{latest}). Run: hotdata update"
"\nA new version of hotdata is available (v{CURRENT_VERSION} → v{latest}).\nThis version may be incompatible with the API. Upgrade now? [Y/n] "
)
.yellow()
);
let _ = std::io::stderr().flush();

// `read_line` returns `Ok(0)` on EOF (e.g. Ctrl-D). Treat EOF and read
// errors as "no" so an accidental Ctrl-D doesn't trigger an upgrade.
let mut input = String::new();
let confirmed = match std::io::stdin().read_line(&mut input) {
Ok(0) | Err(_) => false,
Ok(_) => {
let answer = input.trim();
answer.is_empty()
|| answer.eq_ignore_ascii_case("y")
|| answer.eq_ignore_ascii_case("yes")
}
};

if !confirmed {
eprintln!(
"{}",
"Upgrade required to continue. Run 'hotdata upgrade' when ready, or set HOTDATA_NO_UPDATE_CHECK=1 to bypass this check (e.g. when offline).".red()
);
std::process::exit(1);
}

if let Err(e) = update_to(&latest) {
eprintln!("{}", format!("error: upgrade failed: {e}").red());
eprintln!(
"{}",
"If you can't upgrade right now, set HOTDATA_NO_UPDATE_CHECK=1 to bypass this check."
.dark_grey()
);
std::process::exit(1);
}

eprintln!("{}", "Re-run your command to continue.".green());
std::process::exit(0);
}

/// Try to run `brew upgrade <formula>` directly. Falls back to printing the
/// manual instruction if `brew` is not on PATH or the command fails.
fn run_homebrew_upgrade() {
println!("Updating via Homebrew...");
/// Try to run `brew upgrade <formula>` directly. Status goes to stderr so it
/// never lands in a caller's redirected stdout. Returns an error (including
/// the manual fallback command) instead of exiting, so the caller decides
/// whether a failure is fatal.
fn run_homebrew_upgrade() -> Result<(), String> {
eprintln!("Updating via Homebrew...");

// Locate `brew` — prefer the common install paths so the upgrade works
// even if the user's shell profile hasn't been sourced in this context.
Expand All @@ -188,9 +216,9 @@ fn run_homebrew_upgrade() {
.copied();

let Some(brew_bin) = brew else {
eprintln!("{}", "brew not found — run manually:".yellow());
println!(" {}", format!("brew upgrade {HOMEBREW_FORMULA}").cyan());
return;
return Err(format!(
"brew not found — run manually: brew upgrade {HOMEBREW_FORMULA}"
));
};

let status = std::process::Command::new(brew_bin)
Expand All @@ -206,17 +234,12 @@ fn run_homebrew_upgrade() {
latest_version: v.to_string(),
});
}
Ok(())
}
Ok(s) => {
eprintln!("{}", format!("brew upgrade exited with status {s}").red());
std::process::exit(s.code().unwrap_or(1));
}
Err(e) => {
eprintln!("{}", format!("error running brew: {e}").red());
eprintln!("Run manually:");
println!(" {}", format!("brew upgrade {HOMEBREW_FORMULA}").cyan());
std::process::exit(1);
}
Ok(s) => Err(format!("brew upgrade exited with status {s}")),
Err(e) => Err(format!(
"error running brew: {e} — run manually: brew upgrade {HOMEBREW_FORMULA}"
)),
}
}

Expand All @@ -228,15 +251,18 @@ fn which_brew() -> bool {
.unwrap_or(false)
}

pub fn run_update() {
pub fn run_upgrade() {
let current = Version::parse(CURRENT_VERSION).expect("invalid package version");

if detect_install_method() == InstallMethod::Homebrew {
run_homebrew_upgrade();
if let Err(e) = run_homebrew_upgrade() {
eprintln!("{}", format!("error: {e}").red());
std::process::exit(1);
}
return;
}

println!("Checking for updates...");
eprintln!("Checking for updates...");
let latest = match fetch_latest_version() {
Ok(v) => v,
Err(e) => {
Expand All @@ -249,7 +275,7 @@ pub fn run_update() {
};

if latest <= current {
println!("Already up to date (v{current}).");
eprintln!("Already up to date (v{current}).");
// Refresh cache so the notice goes away.
write_cache(&UpdateCheckCache {
checked_at: now_secs(),
Expand All @@ -258,24 +284,39 @@ pub fn run_update() {
return;
}

println!("Updating from v{current} to v{latest}...");
if let Err(e) = perform_update(&latest) {
if let Err(e) = update_to(&latest) {
eprintln!("{}", format!("error: update failed: {e}").red());
std::process::exit(1);
}
println!("{}", format!("Updated to v{latest}.").green());
}

/// Upgrade the running binary to a known-newer `latest`, dispatching on the
/// install method. Shared by the explicit `hotdata upgrade` command and the
/// pre-command upgrade gate. Returns an error instead of exiting so each
/// caller decides whether a failure is fatal; status goes to stderr so it
/// never pollutes a caller's redirected stdout. On success the new version
/// takes effect on the next invocation.
fn update_to(latest: &Version) -> Result<(), String> {
if detect_install_method() == InstallMethod::Homebrew {
return run_homebrew_upgrade();
}

eprintln!("Updating from v{CURRENT_VERSION} to v{latest}...");
perform_update(latest)?;
eprintln!("{}", format!("Updated to v{latest}.").green());

// Install/update skills to match the new binary. The tarball URL is built
// from `latest` (not CURRENT_VERSION) because the old binary is still
// running at this point — we want the skills for the version we just
// downloaded, not the one we replaced.
crate::skill::install_for_version(&latest);
crate::skill::install_for_version(latest);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: the doc comment on install_for_version in src/skill.rs:250 still says "Called from run_update()", but that function was renamed/refactored — it's now reached via update_to(). Worth updating the stale reference. (not blocking)


// Bust the cache so the notice clears on the next run.
write_cache(&UpdateCheckCache {
checked_at: now_secs(),
latest_version: latest.to_string(),
});
Ok(())
}

/// Download the cargo-dist tar.xz asset for the running target, unpack it,
Expand Down Expand Up @@ -356,4 +397,15 @@ mod tests {
let m = detect_install_method();
assert!(matches!(m, InstallMethod::Homebrew | InstallMethod::Other));
}

#[test]
fn cache_deserializes_ignoring_unknown_fields() {
// A cache file written by an interim build (which carried an extra
// `declined_version` field) must still load after that field was
// removed — serde ignores unknown keys by default.
let json = r#"{"checked_at":123,"latest_version":"0.6.0","declined_version":"0.7.0"}"#;
let cache: UpdateCheckCache = serde_json::from_str(json).unwrap();
assert_eq!(cache.checked_at, 123);
assert_eq!(cache.latest_version, "0.6.0");
}
}
Loading