feat(cube-cli): suggest updating the CLI when a request fails on the API

[paveltiunov]

Check List

• Tests have been run in packages where changes have been made if available
• Linter has been run for changed code
• Tests for the changes have been added if not covered yet
• Docs have been added / updated if required

Description of Changes Made

A CLI that lags the API is a common cause of otherwise puzzling API errors, so cube now points at cube update before the user starts digging — but only when this binary really is behind:

$ cube deployments list
error: GET /api/v1/deployments/ failed with 400 Bad Request: "unknown query parameter `limit`"
hint: this request failed on the API side, and Cube CLI 9.9.9 is available — run cube update to upgrade from 1.7.23, then try again

The hint reuses the background release check that already powers the "new release available" notice, so it costs no extra request:

Release check says	Under the error
Newer release exists	hint naming the available version
Already on the latest	nothing — telling a current CLI to update is noise that teaches people to ignore the hint
Couldn't be determined	hedged hint: "may already fix it — run cube update to check"
CUBE_NO_UPDATE_CHECK=1	nothing

How it works:

• New src/error.rs with ApiError, a marker error type for failures that came from an API response (an unsuccessful status, or a body the CLI can't make sense of), plus is_api_error to spot it anywhere in an anyhow error chain. api_bail! mirrors bail! at those sites, so the call sites read as they did before.
• client.rs returns it for every non-success status and for the "web app HTML instead of JSON" case; oauth.rs for the device-authorization, token-poll and refresh endpoints, including responses it can't parse.
• Transport failures (DNS, TLS, connection refused) and local errors (not logged in, API URL is empty) stay plain — an update won't fix those, so they get no hint.
• spawn_check now yields an UpdateCheck outcome (Newer / UpToDate / Disabled / Unknown) instead of a pre-rendered notice string, resolved once and shared by the notice and the hint. On a terminal, when the notice has just announced the release, the hint refers back to it rather than repeating the version and command.
• The check is only awaited when something will read the answer, and the error line prints before that wait, so a slow or blocked api.github.com neither delays scripted runs nor sits between the user and their error.
• Styling goes through owo_colors::Style — colored on a terminal, plain in piped output, since unlike the notice the hint also prints non-interactively (a stale pinned CLI in CI is where the advice pays off).

Tests

cargo test (33 passed), cargo clippy --all-targets -- -D warnings, and cargo fmt --all --check all clean in rust/cube-cli. Tests cover is_api_error (direct, wrapped in context, negative cases), every mapped status plus the HTML case in the client, all four hint outcomes, the color split, and the "don't wait when nobody's listening" predicate — none of them touching env or terminal state.

Behaviour was also verified by running the built binary against a local server returning 400, with fake release endpoints pinned to a newer and an equal version, a socket that accepts and never answers, a closed port, and an unconfigured context — under both a pipe and a pty. Two latency measurements from that harness, against the unresponsive release endpoint:

• piped successful command: 1526 ms → 24 ms
• error line on a failing command: +1521 ms → +20 ms

[claude]

Claude finished @paveltiunov's task in 2m 15s —— View job

---

Review: solid design, one UX issue worth fixing before merge — 2 medium, 2 low

Full review (4 inline comments posted)

• Gather context (read diff + files)
• Check existing review threads (none existed)
• Review code quality / error handling
• Review tests & docs
• Post review

What's good

The marker-error approach is the right shape for this. A few things done well:

• Chain-aware detection. is_api_error walks err.chain() (error.rs:47), so a .context("while listing deployments") added anywhere upstream doesn't lose the classification — and there's a test for exactly that case.
• The blast radius is correctly bounded. I grepped every status check in the crate (is_success() / status() across client.rs, oauth.rs, update.rs, commands/update.rs) and the classification is complete for the Cube API while correctly leaving out the GitHub release lookup at update.rs:74. Suggesting cube update when the release lookup itself failed would be circular — good call.
• Transport vs. response is the right cut line. send_once's request to {url} failed: {e} (client.rs:124) stays plain, so DNS/TLS/connection-refused get no hint.
• OAuth terminal errors stay plain. access_denied and expired_token (oauth.rs:163-166) keep bail! while unparseable poll responses get api_bail! — that distinction is deliberate and correct.
• try_refresh swallows its error (client.rs:252), so refresh() returning an ApiError can't leak a stray hint through the 401 retry path.
• README updated alongside, and the announced flag avoiding a duplicate "run cube update" right under the notice is a nice touch.

Findings

#	Severity	Location	Issue
1	Medium	client.rs:199-205	401/403 (and usually 404) get a cube update hint that contradicts the fix the error itself prescribes
2	Medium	update.rs:138-148	Hint isn't TTY-gated, unlike print_notice — fires in piped/CI output
3	Low	update.rs:167-174	api_error_hint's CUBE_NO_UPDATE_CHECK gate is untested; only hint_text is covered
4	Low	update.rs:152-161	Hint is uncolored while the notice it references uses owo_colors

#1 is the one I'd want addressed. 401/403/404 are the most common API failures a user hits, and none of them are fixed by updating. Attaching the hint there means it mostly fires where it's wrong, which teaches people to skip it in the 400/422/HTML cases where it actually helps. The 401 output is the clearest symptom — the user is told to run cube login and cube update in consecutive lines.

Security / performance

Nothing of concern. ApiError carries only the message that was already being printed, so no new data reaches the terminal. is_api_error is an O(chain-depth) walk on an already-failed path. No new I/O — the hint reuses the existing background check's result rather than issuing another request.

Not verified

I wasn't able to run cargo test / cargo clippy (the command required approval that isn't granted in this environment), so I reviewed statically. The PR description reports 28 tests passing plus clean clippy and fmt. One thing I did check by hand: Client::new(&str, &str) -> Result<Self> (client.rs:51) matches the test helper's call, and finish_response's signature matches the finish helper.

· branch [`claude/cli-error-update-suggestion-xgc9d6`](https://github.com/cube-js/cube/tree/claude/cli-error-update-suggestion-xgc9d6)

[claude]

Claude finished @paveltiunov's task in 1m 40s —— View job

---

Re-review of ede70a8: three of four prior findings addressed well — 1 new medium, 1 prior medium still open

Full review (1 new inline comment)

• Gather context (diff since last review)
• Check existing review threads
• Review the new commit
• Post review

What the new commit fixes

ede70a8 replaces the pre-rendered notice string with an UpdateCheck enum (Newer / UpToDate / Disabled / Unknown) resolved once and shared by both the notice and the hint. That's a better factoring than what I suggested, and it closes three of the four prior findings:

• MUTEX for requests #2 (TTY gating) — resolved by deciding explicitly rather than by symmetry: the hint deliberately prints in non-TTY, and both the doc comment on api_error_hint (update.rs:170-176) and the README say why ("a stale pinned CLI in CI is exactly where the advice pays off"). Reasonable call, and now documented rather than incidental.
• ResultSet pivot methods documentation  #3 (untestable env gate) — hint_for(outcome, announced, color) takes the policy inputs as parameters, so all four outcomes are covered by tests that touch neither env nor terminal state.
• Self reference guard in schema compiler #4 (uncolored hint) — Style::new().yellow() / .bold().cyan(), gated on stderr().is_terminal(), with a test asserting escapes appear only when asked for.

Beyond the four, the commit adds something I didn't ask for and should have: UpToDate now yields no hint at all. Telling someone on the newest release to update is the failure mode that would have made this feature actively annoying, and the three-way split (Newer names the version, Unknown hedges with "may", UpToDate/Disabled stay silent) gets the tone right in each case. The let ... else rewrite of spawn_check is also clearer than the .ok()? chain it replaces.

Findings

#	Severity	Location	Issue	Status
1	Medium	client.rs:199-205	401/403/404 still get a cube update hint that contradicts the fix the error prescribes	prior thread, still open
5	Medium	main.rs:302-305	resolve is now awaited unconditionally — non-TTY runs block up to 1.5 s on the GitHub check for a result they discard	new, inline

#1 is unchanged — client.rs wasn't touched by this commit, so the existing thread still applies as written. It's worth re-reading in light of UpToDate: the new gating means the hint now fires less often overall, but on a genuinely stale CLI a 401 still prints "session expired — run cube login" immediately followed by "run cube update", and only one of those is the fix.

#5 is a side effect of the refactor. Previously the 1500 ms timeout lived inside print_notice, behind its is_terminal early return, so piped runs never waited. Now resolve is awaited before either consumer is consulted — see the inline comment for the one-line guard.

Minor

• Neither the notice nor the hint honours NO_COLOR; owo_colors emits escapes whenever the is_terminal branch is taken. Pre-existing crate-wide behaviour, not introduced here — mentioning only because this commit adds a second colored call site.
• check is None for Update/Completion, which maps to Unknown (main.rs:304) rather than a distinct "not checked". Harmless today since neither command produces an ApiError, so the Unknown hint can't surface from that path.

Not verified

cargo test / cargo clippy still require an approval this environment doesn't grant, so this remains a static review. The five new update.rs tests read as correct against the code, and the color test in particular pins the behaviour that would otherwise be easy to regress. If you'd like these run here, adding Bash(cargo test) / Bash(cargo clippy) to --allowedTools would do it.

· branch [`claude/cli-error-update-suggestion-xgc9d6`](https://github.com/cube-js/cube/tree/claude/cli-error-update-suggestion-xgc9d6)

[claude]

Claude finished @paveltiunov's task in 1m 43s —— View job

---

Re-review of 1a00da8: the perf regression is fixed cleanly — 1 low (new), 1 prior medium still open by author's decision

Full review (1 new inline comment)

• Gather context (diff since last review)
• Check existing review threads
• Review the new commit
• Post review

What the new commit does

1a00da8 moves the "should we even wait?" decision into update::resolve, which now takes Option<JoinHandle> plus api_error and returns UpdateCheck::Unknown without awaiting when neither consumer will read the answer (update.rs:136-160). That closes finding #5 — a piped, successful cube deployments list no longer blocks on api.github.com.

It's better placed than the version I suggested: putting the gate in update.rs keeps both is_terminal checks in the module that owns the policy, so main.rs shrinks to one line rather than growing an IsTerminal import. The wanted(api_error, interactive) split looks over-thin for a two-term boolean, but it earns its keep — a_piped_command_that_succeeded_does_not_wait_on_the_check pins all three interesting combinations without touching terminal state, which is exactly the pattern used for hint_for in the previous commit. Consistent, and the doc comment on resolve states the reasoning rather than leaving it to be re-derived.

The Option handling also folded the old match check { None => Unknown } from main.rs into resolve's let ... else, so the Update/Completion case and the "nobody's listening" case now converge in one place.

Findings

#	Severity	Location	Issue	Status
1	Medium	client.rs:205	401/403/404 classified as API errors	prior thread — author declined, see below
6	Low	main.rs:302	error: output is delayed by the up-to-1.5 s wait on a failing run	new, inline

#1 — your reply stands on its own: gating the hint on UpdateCheck::Newer means a current CLI gets a plain 401, so the contradiction only surfaces for someone who is genuinely stale and hitting an expired session, where mentioning both is defensible. "Any API error" being the product requirement settles it. Leaving the thread unresolved since it's a judgement call you made rather than a defect that disappeared, but I'm not asking for a change.

#6 is small and mostly pre-existing — resolve sits above eprintln!("error: ..."), so when the release check hasn't landed the user waits for GitHub before seeing why their command failed. This commit widens the exposure (piped failing runs now await too, where before this PR they never did). Trivially avoidable by printing the error first; the cost is that the notice then lands below the error instead of above it, so it's your call on layout.

Minor

• Unknown now means three different things: "check failed", "check timed out", and "we never asked". Harmless while the only two consumers are print_notice (TTY-gated) and api_error_hint (error-gated) — the skip condition is exactly the negation of those two — but a third consumer added later would get the Unknown hint text on a path where no check ran. A NotChecked variant would make that unrepresentable if it ever comes up.
• Dropping the un-awaited JoinHandle detaches the task rather than aborting it; the process exits immediately after, so the in-flight request dies with the runtime. No leak.
• NO_COLOR still isn't honoured (crate-wide, pre-existing) — unchanged from the last round.

Not verified

cargo test / cargo clippy remain unavailable here — both invocations were declined by the permission layer, so this is again a static review. Adding Bash(cargo test) / Bash(cargo clippy) to --allowedTools would let me actually run them. Statically, resolve's new Option<JoinHandle<UpdateCheck>> signature matches the single call site at main.rs:302 and check's type at main.rs:279, and wanted is exercised by the new test so it won't trip dead_code.

· branch [`claude/cli-error-update-suggestion-xgc9d6`](https://github.com/cube-js/cube/tree/claude/cli-error-update-suggestion-xgc9d6)

[claude]

Claude finished @paveltiunov's task in 1m 6s —— View job

---

Re-review of 144fee5: clean fix, no new findings — 1 prior medium open by author's decision

Full review (no new inline comments)

• Gather context (diff since last review)
• Check existing review threads
• Review the new commit
• Post review

What the new commit does

144fee5 hoists eprintln!("error: {err:#}") above update::resolve and switches the tail to result.is_err() (main.rs:302-315), so the failure reaches the user before anything touches the release check. That closes finding #6 — the measured 1521 ms → 20 ms against an unresponsive release endpoint matches what the code now does, since the only remaining consumer of the awaited outcome is the hint.

Three things I checked:

• Ownership. if let Err(err) = result moved result; the new &result borrow plus result.is_err() is the minimal correct rewrite. No clone, no change to the exit path.
• Ordering on the other paths. Successful runs are untouched — resolve → print_notice still runs after command output. On a failing run the notice moves below the error, which lands it directly above the hint that says "the newer release above may already fix it" (update.rs:216). The announced wording stays accurate; if anything the adjacency reads better than before.
• Everything on stderr. Error, notice, and hint are all eprintln!, so no interleaving hazard from the reorder.

The comment above the hoist states the reason rather than leaving it to be re-derived, consistent with how resolve and api_error_hint are documented in the earlier commits.

Findings

#	Severity	Location	Issue	Status
1	Medium	client.rs:205	401/403/404 classified as API errors	prior thread — author declined, not re-raised

No new findings. Re-affirmed the one prior thread still applies as written (client.rs is untouched since it was opened); leaving it unresolved because it's your judgement call, not a defect that disappeared. The four earlier threads are resolved.

Minor (not worth a change here)

• On a network where api.github.com is blackholed but the Cube API is reachable — a locked-down CI is the realistic case — every API failure still pays the 1.5 s wait and then prints the Unknown variant ("a newer release may already fix it"). That's the feature's inherent cost on a path that has already failed, and it's now paid after the error is visible, which is the part that mattered.
• NO_COLOR still isn't honoured (crate-wide, pre-existing) — unchanged, and you've flagged it as a separate change.

Not verified

cargo test / cargo clippy were declined by the permission layer again (three invocations, all blocked), so this remains a static review. Adding Bash(cargo test) / Bash(cargo clippy) to --allowedTools would let me actually run them. The commit touches no test-covered logic — main.rs has no tests, and the eight update.rs tests exercise wanted/hint_for, neither of which changed.

· branch [`claude/cli-error-update-suggestion-xgc9d6`](https://github.com/cube-js/cube/tree/claude/cli-error-update-suggestion-xgc9d6)