Skip to content

feat(cli): nudge raven upgrade in the tui status bar when behind - #220

Merged
0xKT merged 3 commits into
mainfrom
feat/tui_update_notice
Jul 29, 2026
Merged

feat(cli): nudge raven upgrade in the tui status bar when behind#220
0xKT merged 3 commits into
mainfrom
feat/tui_update_notice

Conversation

@arelchan

Copy link
Copy Markdown
Contributor

Summary

The TUI already read update_behind / update_command out of the session init
bundle, but nothing on the Python side ever filled those fields, so the hint
never appeared. This wires up the missing half and moves where it shows.

The check

raven/cli/update_notice.py compares the running version against a small
cache in ~/.raven/update_check.json:

  • raven tui calls maybe_refresh_async() once per launch. It spawns a daemon
    thread only when the cache is missing or older than 24h, so a normal launch
    touches the network at most once a day and never blocks startup.
  • The gateway only reads that cache when it builds the session info bundle
    (_default_session_info). Reading is pure and fast; no network on the
    session-create path, which is why the check is cached rather than live.
  • Every failure path is swallowed: no cache, unparseable version, corrupt JSON,
    network error, rate limit. An update hint must never break startup.

The tradeoff is staleness by one launch: the first launch after a release
lands refreshes the cache, and the hint shows on the next launch.

Where it shows

The status bar's right slot, in the warn color, replacing the cwd/branch label
while an update is available. No extra line, no layout shift.

The old banner block in branding.tsx is removed. The banner is the first
thing on screen at startup and an upgrade nudge does not deserve that weight.
update_behind is now a flag rather than a commit count, so the text is
version-agnostic (Update available - run raven upgrade) instead of the old
N commits behind.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

uv run ruff check raven/ tests/                  # All checks passed
uv run ruff format --check raven/ tests/         # 760 files already formatted
uv run pytest tests/test_cli_update_notice.py \
  tests/test_cli_tui_commands.py \
  tests/test_tui_rpc_session.py -q               # 95 passed
cd ui-tui && npm run type-check                  # clean
cd ui-tui && npm test                            # 904 passed | 3 skipped
cd ui-tui && npm run lint                        # 0 errors
cd ui-tui && npx prettier --check "src/**/*.{ts,tsx}"   # clean

New tests cover the cache read/write and refresh throttle
(tests/test_cli_update_notice.py), the status-bar slot swap
(ui-tui/src/__tests__/statusRule.test.tsx), and the banner no longer
rendering the block (branding.test.tsx).

Also exercised the five edge cases by hand: absent cache, newer cached
release, cache equal to current, unparseable version string, and a corrupt
cache file. Only the second one returns a notice; the rest return None.

The lint warning reported on appChrome.tsx (react-hooks/exhaustive-deps,
line 275) is pre-existing on main and untouched by this change.

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

The GitHub releases fetch reuses upgrade_commands._fetch_latest_release, so
no new network surface or credential handling. The cache file holds a version
string and a timestamp only.

Wire-compatible: update_behind / update_command were already optional
fields in the info bundle, and a client that never receives them behaves as
before. Rollback is reverting this commit; the feature degrades to its current
state, which is showing nothing.

To see it locally:

printf '{"latest_version":"9.9.9","checked_at":%s}' "$(date +%s)" > ~/.raven/update_check.json

Related Issues

N/A

@0xKT 0xKT left a comment

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.

Review of ce56583, merged onto main 16719d0: clean merge, full unit suite green locally (4621 passed). Note the green checks on this PR predate #224, which switched CI to the full unit suite, so a rebase and re-run is needed regardless.

Items are ID severity. Each is independent; reply with the ID when addressed.

B1 blocking: unit tests now hit the GitHub API and write the real $HOME

  • Where: raven/cli/tui_commands.py:997-999, tests/test_cli_tui_commands.py:505
  • What: test_tui_announces_log_path_only_on_abnormal_exit (6 cases) mocks find_node, so control flow reaches maybe_refresh_async() with nothing stubbing it. Under a throwaway HOME those cases write $HOME/.raven/update_check.json with a real fetched latest_version. All failures are swallowed, so the test stays green either way. Since #224 the CI unit job runs the full suite.
  • Fix: autouse fixture redirecting update_notice._CACHE_PATH to tmp_path, mirroring tests/conftest.py:30-52 for model_catalog_cache._CACHE_PATH. That also isolates tests/test_tui_rpc_session.py, whose session_create({}) reads the real cache today.
  • Do not: patch tui_commands.maybe_refresh_async. The import at tui_commands.py:997 is function-local, so there is no module attribute to replace.

B2 blocking: a valid-JSON, non-object cache file raises instead of being swallowed

  • Where: raven/cli/update_notice.py:37-40, raising at :77 and :93
  • What: _read_cache returns whatever json.loads yields with no dict check. A cache holding "0.2.0", [1] or 42 reaches cache.get(...) and raises AttributeError inside tui() (raven tui fails to launch) and inside _default_session_info (session.create fails). Both reproduced. Reaching it takes a hand-edited file, which the "To see it locally" snippet invites, and the module contract is that the hint never breaks startup.
  • Fix: isinstance(parsed, dict) guard in _read_cache.

S1 should-fix: the nudge points at a command that always fails for editable and pip installs

  • Where: raven/cli/update_notice.py:96-100 vs raven/cli/upgrade_commands.py:440-449
  • What: raven upgrade raises for editable installs and non-uv-tool installs and exits 1, but update_notice() only compares versions, so those users get a permanent warn-colored nudge to a failing command.
  • Fix: gate the notice on upgrade_commands._is_uv_tool_install() (:356), wrapped in try/except since _uv_tool_target raises UpgradeError on a malformed receipt.

S2 should-fix: the failure path never backs off, and its comment says otherwise

  • Where: raven/cli/update_notice.py:56-65
  • What: _refresh writes checked_at only on success, so while offline, rate-limited, or whenever the latest release is a draft or prerelease (_parse_release_payload raises), every launch spawns a thread and refetches. The comment at :64 claims "try again next TTL".
  • Fix: write checked_at on failure too, keeping the previous latest_version.

S3 should-fix: the cache path bypasses the cache-dir helper

  • Where: raven/cli/update_notice.py:24
  • What: hardcodes Path.home() / ".raven", so a non-default instance set via set_config_path() writes back into the default home.
  • Fix: paths.get_cache_dir() (raven/config/paths.py:37), the designated disposable-cache dir, with precedent at raven/token_wise/model_catalog_cache.py:39. It also removes the manual mkdir.

N1 minor: duplicate version parser

update_notice.py:27,30 reimplements upgrade_commands.py:21,178. The looser semantics (prefix match, return None) look deliberate, but the module already imports _fetch_latest_release from that file, so one shared lenient parser keeps a single source of truth.

N2 minor: dead and missing demo fixtures

ui-tui/src/demo/gallery.tsx:102-103 still seeds update_behind: 2 into the SessionPanel demo, dead now that the banner block is gone, while the StatusRule demos and stubGatewayFixtures.ts:32 never exercise the new nudge state.

N3 minor: no opt-out for the launch-time network call

The fetch sends the raven version in the User-Agent (upgrade_commands.py:234-239). An env-var escape hatch is conventional for update checks.

N4 minor: update_behind is a flag in a count-shaped field

Nothing on main ever populated it (stubGatewayFixtures.ts:32 is null), so renaming it to a boolean update_available is free now and not later.

Verified fine, no action: the status bar stays on one line at narrow widths (checked down to cols=44, where Yoga shrinks and truncates both slots), and the RPC server is only constructed inside the raven tui process, so the refresher and the reader always share a process.

arelchan and others added 2 commits July 28, 2026 20:32
The TUI already read update_behind / update_command out of the session init
bundle, but nothing ever filled those fields, so the hint never appeared.

Add raven/cli/update_notice.py: a cached check against the GitHub releases
API. The live call is too slow for the session-create path, so `raven tui`
refreshes ~/.raven/update_check.json in a daemon thread at most once a day
and the gateway only reads that cache while building the info bundle. Every
failure path is swallowed -- an update hint must never break startup. The
cost is that a freshly published release surfaces on the next launch.

Show it in the status bar's right slot, replacing the cwd/branch label, in
the warn color. The old banner block is removed: the banner is the first
thing on screen and an upgrade hint does not deserve that weight.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Review follow-ups on the startup nudge.

Tests no longer touch the network or the real home: an autouse fixture
redirects the cache path and sets RAVEN_NO_UPDATE_CHECK for the suite.
Redirecting the path alone leaves the cache absent, which is exactly the
state that spawns the fetch, so six tui cases fetched a release on every
run since CI went to the full suite.

A cache file holding valid JSON that is not an object no longer raises
out of `raven tui` or session.create -- _read_cache rejects non-dicts.

The nudge is gated on an upgradable install, so editable and non-uv-tool
users stop being pointed at a command that always exits 1. A failed
refresh now stamps checked_at too, so offline, rate-limited and
draft-release launches back off for a full TTL instead of refetching
every time.

The cache moves to paths.get_cache_dir(), resolved per call so
set_config_path() is honoured, and the duplicate version parser
delegates to upgrade_commands.

Also adds a RAVEN_NO_UPDATE_CHECK opt-out, renames update_behind to the
boolean update_available it always was, and gives the demo gallery a
StatusRule nudge state in place of the dead SessionPanel seed.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@arelchan
arelchan force-pushed the feat/tui_update_notice branch from ce56583 to 1c581e6 Compare July 28, 2026 13:26
@arelchan

Copy link
Copy Markdown
Contributor Author

All nine items addressed, rebased onto 16719d0. Thanks for catching B1 and B2 -- both were real and both are the kind of thing a swallowed-exception module hides well.

B1 fixed -- unit tests no longer fetch or write the real home

tests/conftest.py gains an autouse fixture next to _no_openrouter_network. It does two things: redirects the cache path to tmp_path, and sets the new RAVEN_NO_UPDATE_CHECK=1 for the whole suite.

The redirect alone is not enough. Under a temp path the cache is absent, which is exactly the state that spawns the fetch thread, so the six test_tui_announces_log_path_only_on_abnormal_exit cases would still hit the GitHub API on every CI run. Opting out by env kills the fetch and the write.

tests/test_cli_update_notice.py is the one place that clears the flag, so the module is still covered end to end. And as you noted, tui_commands.maybe_refresh_async is a function-local import with no module attribute to patch -- nothing does that.

B2 fixed -- non-object cache no longer raises

isinstance(parsed, dict) guard in _read_cache. Parametrized over "0.2.0" / [1] / 42 / null, asserting both update_notice() and maybe_refresh_async() stay quiet.

S1 fixed -- no nudge when raven upgrade cannot work

update_notice() now gates on upgrade_commands._is_uv_tool_install() behind try/except, so a malformed uv receipt reads as "not upgradable" rather than propagating UpgradeError. Verified on this checkout, which is an editable install: cache says 9.9.9, notice is None.

S2 fixed -- the failure path backs off

_refresh now stamps checked_at on failure too, keeping the previous latest_version. So offline, rate-limited and draft/prerelease all back off for a full TTL instead of refetching every launch. The comment that claimed this is now true. Covered by a test that makes the fetch raise and asserts the version survives and checked_at moves.

S3 fixed -- cache lives in the cache dir

paths.get_cache_dir() / "update_check.json", resolved per call rather than at import so set_config_path() is honoured. The manual mkdir is gone with it.

N1 fixed -- one version parser

update_notice._version_key now delegates to upgrade_commands._version_key and converts its UpgradeError into None. The lenient semantics are kept where they belong (a version we cannot read means no notice), with a single grammar.

N2 fixed -- demo fixtures

Removed the dead update_behind seed from the SessionPanel demo and added a StatusRule -- update available demo so the right-slot takeover is visible in the gallery.

N3 fixed -- opt-out

RAVEN_NO_UPDATE_CHECK=1 (also accepts true / yes / on) skips both the fetch and the hint. Documented in the module docstring.

N4 fixed -- flag-shaped field

update_behind?: number | null is now update_available?: boolean | null across session.py, types.ts, appLayout.tsx, the demo gallery, stubGatewayFixtures.ts and branding.test.tsx. Nothing on main populated the old name, so this was free.

Verification

uv run ruff check raven/ tests/            # All checks passed
uv run ruff format --check raven/ tests/   # 763 files already formatted
uv run pytest tests/ --ignore=tests/integration   # 4629 passed
cd ui-tui && npm run type-check            # clean
cd ui-tui && npm test                      # 904 passed | 3 skipped
cd ui-tui && npm run lint                  # 0 errors
cd ui-tui && npx prettier --check "src/**/*.{ts,tsx}"   # clean

A first full-suite run showed two failures; a clean re-run of the same tree came back 4629 passed, and both are
pre-existing and unrelated -- the first also reproduces with this branch's changes stashed:

  • tests/test_cli_theme.py::test_bold_accent_renders_styled_not_bare fails whenever TERM and COLORTERM are unset, which is the case in a non-tty shell: rich falls back to the ANSI palette and emits \x1b[1;93m instead of truecolor. Passes with COLORTERM=truecolor, and the whole file passes standalone under that. Probably worth pinning the console's color_system in the test rather than inheriting the environment.
  • tests/test_default_context_engine.py::TestTwoTrackConcurrency::test_skill_and_memory_run_concurrently asserts elapsed < 0.15 for two 0.10s stubs. It flaked once under full-suite CPU load and passes standalone (20/20).

@0xKT

0xKT commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Re-reviewed 1c581e6 on the merged state. B2, S1, S2, S3, N1, N2, N3, N4 all verified fixed -- S1 checked against an editable checkout (_is_uv_tool_install() false, 9.9.9 cached, notice None), N3 checked with RAVEN_NO_UPDATE_CHECK=yes. Thanks for the RAVEN_NO_UPDATE_CHECK addition, and you are right that redirecting the cache path alone would not have been enough.

B1 is not closed yet, and there are two new minors.

B1 still open blocking: the suite still fetches and still writes the real home

  • Where: tests/test_cli_update_notice.py:70-76 (test_non_object_cache_is_ignored)
  • What: the cache fixture clears RAVEN_NO_UPDATE_CHECK, and line 76 calls un.maybe_refresh_async() bare. A non-object payload reads back as None, which is the "cache absent" state, so a real daemon thread spawns and _refresh() performs the GitHub fetch -- once per parametrized case. The write lands on the real path rather than the fixture's tmp_path because the thread reaches _write_cache() after monkeypatch teardown has already restored _cache_path.
  • Evidence: full unit suite on the merged tree under a throwaway HOME, 4629 passed, and afterwards $HOME/.raven/cache/update_check.json contains {"checked_at": ..., "latest_version": "0.1.9"} -- a real fetched version. Reproduced deterministically by replaying the same sequence (patch _cache_path, call maybe_refresh_async(), restore _cache_path, join the thread): the temp file stays non-object and the real path gets written.
  • Why it passed locally: running that file alone does not show it. The process exits in under a second and the daemon thread is killed before the request completes; it needs a run long enough for the HTTP call to land, which the full suite is.
  • Fix: stub threading.Thread before the bare call, the same way lines 91, 99, 107 and 115 already do with _FakeThread. The assertion only needs "does not raise", not a real thread.

N5 minor: the refresh path is not gated the way the notice is

_upgrade_command_works() gates update_notice() (update_notice.py:161) but not maybe_refresh_async() (:102-120), so editable and pip installs still hit the GitHub API every TTL for a notice they can never see. Gating the refresh too would make S1 free of network cost as well.

N6 minor: delegation tightened the version grammar

upgrade_commands._version_key uses fullmatch, so _version_key("0.1.9.dev1") and ("0.2.0rc1") now return None where the previous prefix match parsed them. v0.2.0 still parses, and refusing to notice is the safe direction, but anyone on a uv tool install raven==X.Y.Zrc1 build silently stops getting the hint. Worth a deliberate call rather than a side effect.

The non-object-cache test called maybe_refresh_async() bare. A non-object
payload reads back as "no cache", which is the state that spawns the
refresh thread, so each parametrized case fetched a release and wrote the
result after monkeypatch had already restored the cache path -- landing on
the real home rather than tmp_path. Running the file alone hid it: the
process exits before the request completes. Stub the thread, since the
assertion only needs "does not raise".

Gate maybe_refresh_async on an upgradable install too. Editable and pip
installs cannot act on the result, so they were paying for a fetch every
TTL for a hint update_notice() already suppresses.

Normalise a prerelease version to the release it builds on before the
strict parser sees it. Delegating to upgrade_commands._version_key brought
fullmatch semantics with it, which silently dropped the hint for anyone on
an rc or dev build. The suffix is discarded rather than ordered, so an rc
of the latest release compares equal to it and is not nagged.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@arelchan

Copy link
Copy Markdown
Contributor Author

0f071fb. B1 closed, N5 and N6 done. Your diagnosis of B1 was exactly right and I had verified it the way that hides it -- running the file alone, where the process exits before the request lands.

B1 closed

monkeypatch.setattr("threading.Thread", ...) before the bare maybe_refresh_async() in test_non_object_cache_is_ignored, matching what the four refresh tests already do. The assertion only ever needed "does not raise".

Reproduced the leak first, without touching the real cache, by standing in a second temp file for $HOME: patch _cache_path to A, write a non-object payload, call maybe_refresh_async(), restore _cache_path to B, join the daemon thread. A stayed non-object and B came back with {"checked_at": ..., "latest_version": "0.1.9"} -- a real fetched version, landing after teardown, exactly as you described.

Then verified the fix the way you found it: deleted ~/.raven/cache/update_check.json, ran the full unit suite (4634 passed), slept to give any stray thread time to write, and the path was still absent. Same check after the three targeted files: absent.

N5 done -- the refresh is gated like the notice

maybe_refresh_async now returns early unless _upgrade_command_works(). Editable and pip installs stop paying for a fetch every TTL for a hint they can never be shown. _is_uv_tool_install() measures 0.07 ms cold on this machine (an is_file() plus a small TOML read), so the launch path does not notice it. Covered by test_refresh_skipped_when_upgrade_command_would_fail.

N6 -- deliberate call: normalise, do not order

You are right that delegation tightened the grammar and that this deserves a decision rather than a side effect.

Decision: a prerelease is reduced to the release it builds on before the single strict parser sees it, so 0.1.9rc1, 0.1.9.dev1 and 0.1.9+local all compare as 0.1.9 and keep getting the hint. _release_prefix() is a normaliser, not a second grammar -- upgrade_commands._version_key stays the only place the shape of a version is defined, so N1 holds.

The suffix is discarded rather than ordered: 0.2.0rc1 against a latest of 0.2.0 compares equal and produces no notice. Nagging someone to "upgrade" to the release they are deliberately testing an rc of seemed worse than the small loss of precision. Both cases are pinned by tests.

Verification

uv run ruff check raven/ tests/            # All checks passed
uv run ruff format --check raven/ tests/   # 763 files already formatted
uv run pytest tests/ --ignore=tests/integration   # 4634 passed
cd ui-tui && npm run type-check            # clean
cd ui-tui && npm test                      # 904 passed | 3 skipped
cd ui-tui && npm run lint                  # 0 errors
cd ui-tui && npx prettier --check "src/**/*.{ts,tsx}"   # clean

Ran in a separate worktree with its own environment, since another change is in flight on this checkout. Note that a plain uv sync there misses the optional extras and produces 25 unrelated failures (slack_sdk, boxlite); --all-extras --all-groups matches CI.

Still not verified, for the record

The positive path -- a real uv-tool install with a newer release actually rendering the hint -- has only been exercised with _upgrade_command_works stubbed to True. This checkout is editable, so S1 correctly suppresses it here. install.sh uses uv tool install, so real users hit the gated-on branch, but nobody has watched the status bar light up on a genuine install. If that matters before merge, the check is:

printf '{"latest_version":"9.9.9","checked_at":%s}' "$(date +%s)" > ~/.raven/cache/update_check.json
raven tui

Also worth noting: nothing asserts that raven tui still calls maybe_refresh_async(). Per your earlier note the import is function-local, so there is no module attribute to patch, and I did not want to restructure production code to make it observable. Deleting that line would keep the suite green.

@0xKT
0xKT self-requested a review July 29, 2026 07:01

@0xKT 0xKT left a comment

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.

Verified 0f071fb on the merged state. All eleven items closed, each checked rather than taken on report:

  • B1: full unit suite under a throwaway HOME, 4634 passed, and afterwards the cache directory does not exist at all. The previous revision left $HOME/.raven/cache/update_check.json there with a real fetched version.
  • N5 doubles as a second line of defence: CI installs raven editable, so _upgrade_command_works() is false and the refresh never spawns.
  • N6: swept the normaliser. 0.2.0rc1 / 0.1.9.dev1 / 0.1.9+local / v0.2.0 all reduce correctly, garbage / 1.2 / .1.2 all read as unparseable. Dropping the suffix instead of ordering it is the right call.
  • S1 checked against an editable checkout, N3 with RAVEN_NO_UPDATE_CHECK=yes, and the update_available field name matches on both sides of the seam.

Approving. Three coverage gaps stay open by agreement; the maintainer is doing a test-organisation pass separately and these fold into it.

  1. Nothing asserts the session bundle carries update_available / update_command. tests/test_tui_rpc_session.py is untouched by this PR, so deleting the two lines at session.py:159 or misspelling the key leaves the whole suite green and the TUI showing nothing, which is exactly the pre-PR state this change exists to fix. That seam has broken once already.

  2. Nothing asserts raven tui still calls maybe_refresh_async(). One correction on the reasoning: a function-local import resolves the attribute on the source module at call time, so monkeypatch.setattr(update_notice, "maybe_refresh_async", ...) does observe the call. Verified with a throwaway test against the real tui_app invocation; no production restructuring needed.

  3. The positive path has never run on a real uv-tool install, as you noted. Worth one manual look at some point, since every dev checkout is editable and S1 correctly suppresses the hint there, so nobody stumbles onto it by accident.

@0xKT
0xKT merged commit 7babed9 into main Jul 29, 2026
9 checks passed
@0xKT
0xKT deleted the feat/tui_update_notice branch July 29, 2026 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants