fix: address code review feedback from post-release audit (#106)

* fix: address code review feedback from post-release audit

- query.rs: remove dead AsyncResponse.status field (was the source of
  the dead_code compiler warning; field was printed pre-polling but is
  unused now that the loop reads QueryRunResponse.status)
- query.rs: reorder polling loop — poll first, then check deadline and
  sleep; eliminates the mandatory 500ms delay before the first check and
  ensures timeout is detected without an extra sleep cycle
- skill.rs: add 120s timeout to download_and_extract_from_url HTTP
  client, matching the timeout used in perform_update; prevents an
  indefinite hang during hotdata update if the skills download stalls
- main.rs: replace chained .unwrap() with .expect() in the databases
  tables help path for clearer panic messages if subcommand names change

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(update): execute brew upgrade directly for Homebrew installs

Instead of printing instructions and returning, run_update() now shells
out to `brew upgrade <formula>` when a Homebrew install is detected.
brew is located by checking common install paths (/opt/homebrew/bin,
/usr/local/bin) before falling back to PATH. If brew is not found or
the upgrade fails, falls back to printing the manual command.

On success, busts the update-available cache so the notice clears on
the next run, matching the behaviour of the non-Homebrew upgrade path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Eddie A Tejeda <669988+eddietejeda@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: eddietejeda

src/main.rs

@@ -484,11 +484,11 @@ fn main() {
                                     let mut cmd = Cli::command();
                                     cmd.build();
                                     cmd.find_subcommand_mut("databases")
-                                        .unwrap()
+                                        .expect("databases subcommand not found")
                                         .find_subcommand_mut("tables")
-                                        .unwrap()
+                                        .expect("tables subcommand not found")
                                         .print_help()
-                                        .unwrap();
+                                        .expect("failed to print help");
                                 }
                             }
                         },

src/query.rs

@@ -17,7 +17,6 @@ pub struct QueryResponse {
 #[derive(Deserialize)]
 struct AsyncResponse {
     query_run_id: String,
-    status: String,
 }
 
 #[derive(Deserialize)]
@@ -184,17 +183,6 @@ pub fn execute(
         let deadline = std::time::Instant::now() + std::time::Duration::from_secs(300);
 
         loop {
-            std::thread::sleep(std::time::Duration::from_millis(500));
-            if std::time::Instant::now() > deadline {
-                spinner.finish_and_clear();
-                use crossterm::style::Stylize;
-                eprintln!("{}", "query timed out after 5 minutes".red());
-                eprintln!(
-                    "{}",
-                    format!("Check status with: hotdata query status {run_id}").dark_grey()
-                );
-                std::process::exit(1);
-            }
             let run: QueryRunResponse = api.get(&format!("/query-runs/{run_id}"));
             match run.status.as_str() {
                 "succeeded" => {
@@ -218,7 +206,7 @@ pub fn execute(
                     eprintln!("{}", format!("query failed: {err}").red());
                     std::process::exit(1);
                 }
-                "running" | "queued" | "pending" => continue,
+                "running" | "queued" | "pending" => {}
                 status => {
                     spinner.finish_and_clear();
                     use crossterm::style::Stylize;
@@ -230,6 +218,17 @@ pub fn execute(
                     std::process::exit(2);
                 }
             }
+            if std::time::Instant::now() > deadline {
+                spinner.finish_and_clear();
+                use crossterm::style::Stylize;
+                eprintln!("{}", "query timed out after 5 minutes".red());
+                eprintln!(
+                    "{}",
+                    format!("Check status with: hotdata query status {run_id}").dark_grey()
+                );
+                std::process::exit(1);
+            }
+            std::thread::sleep(std::time::Duration::from_millis(500));
         }
     }
 

src/skill.rs

@@ -205,7 +205,10 @@ fn download_and_extract_from_url(url: &str) -> Result<(), String> {
     // `resp.text()` and would corrupt the gzip stream). Log the
     // request line manually so `--debug` still shows the URL.
     crate::util::debug_request("GET", url, &[], None);
-    let client = reqwest::blocking::Client::new();
+    let client = reqwest::blocking::Client::builder()
+        .timeout(std::time::Duration::from_secs(120))
+        .build()
+        .map_err(|e| format!("error creating HTTP client: {e}"))?;
     let resp = client
         .get(url)
         .send()

src/update.rs

@@ -164,12 +164,76 @@ pub fn maybe_print_update_notice(rx: Option<std::sync::mpsc::Receiver<Option<Ver
     );
 }
 
+/// 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...");
+
+    // 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.
+    let brew = ["brew", "/opt/homebrew/bin/brew", "/usr/local/bin/brew"]
+        .iter()
+        .find(|&&p| {
+            if p == "brew" {
+                which_brew()
+            } else {
+                std::path::Path::new(p).exists()
+            }
+        })
+        .copied();
+
+    let Some(brew_bin) = brew else {
+        eprintln!(
+            "{}",
+            "brew not found — run manually:".yellow()
+        );
+        println!("  {}", format!("brew upgrade {HOMEBREW_FORMULA}").cyan());
+        return;
+    };
+
+    let status = std::process::Command::new(brew_bin)
+        .args(["upgrade", HOMEBREW_FORMULA])
+        .status();
+
+    match status {
+        Ok(s) if s.success() => {
+            // Cache bust so the update notice clears on the next run.
+            if let Ok(v) = fetch_latest_version() {
+                write_cache(&UpdateCheckCache {
+                    checked_at: now_secs(),
+                    latest_version: v.to_string(),
+                });
+            }
+        }
+        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);
+        }
+    }
+}
+
+fn which_brew() -> bool {
+    std::process::Command::new("which")
+        .arg("brew")
+        .output()
+        .map(|o| o.status.success())
+        .unwrap_or(false)
+}
+
 pub fn run_update() {
     let current = Version::parse(CURRENT_VERSION).expect("invalid package version");
 
     if detect_install_method() == InstallMethod::Homebrew {
-        println!("hotdata was installed via Homebrew. Update with:");
-        println!("  {}", format!("brew upgrade {HOMEBREW_FORMULA}").cyan());
+        run_homebrew_upgrade();
         return;
     }