fix(#4842): stop lock-held CLI cache rebuild stalls during streaming

[rodboev]

Thinking Path

• #4889 already removed the per-row cron sidecar I/O slice, and #4949 already shipped the single-profile streaming-freeze path in current api/models.py.
• The remaining get_cli_sessions() stall was in the follower path after invalidation: a cold follower could wait forever, and a clear during rebuild could leave a joiner with no valid cache entry and fall through to [].
• This keeps the shipped cache-key and payload semantics intact, then tightens the singleflight follow path so callers either reuse fresh cache, reuse still-valid stale rows, or do a bounded fallback rebuild.

What Changed

• api/models.py: split the rebuild path into shared helpers, cap cold followers at a 0.25s wait before an independent rebuild, make invalidated joiners loop back and reclaim rebuild ownership after the old inflight slot drops, reuse fresh cache entries when available, remove the dead post-return fallback block, repair the warning log string, and keep the all-profiles branch on the shared cache path.
• tests/test_issue4842_cli_cache_streaming.py: add focused regressions for the cold-follower timeout fallback and the clear-during-rebuild joiner that used to return [], alongside the existing streaming and invalidation coverage.

Why It Matters

This keeps /api/sessions responsive on slow or wedged rebuilds, it closes the silent empty-sidebar case after clear_cli_sessions_cache(), and it preserves the all-profiles scan behavior that pulls named-profile CLI sessions into the aggregate view. Sidebar rows stay consistent, but followers no longer hang indefinitely or drop CLI and cron sessions.

Verification

• pytest tests/test_issue4842_cli_cache_streaming.py tests/test_cli_sessions_cache_fingerprint.py -v --timeout=60
• 11 passed, 1 warning in 6.30s

Full-suite CI context, not a required local check unless requested: pytest tests/ -v --timeout=60.

Upstream

Closes #4842.

Attribution: the reporter's one-session traces and lock diagnosis on #4842 narrowed this fix to the CLI cache rebuild path after the earlier shipped slices in #4889 and #4949.

Model Used

GPT 5.5 via Codex CLI

[greptile-apps]

Greptile Summary

This PR updates the CLI sessions cache rebuild flow. The main changes are:

• Adds per-key in-flight rebuild tracking.
• Adds cache invalidation stamps for explicit clears.
• Reuses fresh or still-valid stale rows while rebuilds are running.
• Adds bounded cold-follower fallback behavior.
• Adds focused cache and streaming regression tests.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.
• The invalidation stamp check prevents cleared cache entries from being restored.
• The added tests cover the main rebuild and clear-during-rebuild paths.

Important Files Changed

Filename	Overview
api/models.py	Refactors CLI session cache rebuilds around shared in-flight, invalidation, and cache-store helpers.
tests/test_issue4842_cli_cache_streaming.py	Adds concurrency tests for stale reuse, follower fallback, cache clears during rebuilds, and stale-store rejection.

_{Reviews (6): Last reviewed commit: "fix(cli): restore all-profiles session s..." | Re-trigger Greptile}

[nesquena-hermes]

Strong work — the singleflight + stale-while-revalidate shape is right, and the dual gate confirmed the core concurrency is sound: owner-exception wakeup (try/finally always reaches _cli_sessions_cache_done → event.set()), claim atomicity (slot is owner-exclusive, is event guard pops only your own event), clear-during-rebuild lost-update, and lock ordering all trace clean. Moving the rebuild out of _CLI_SESSIONS_CACHE_LOCK is the correct fix for the #4842 stall. Two fixes before merge, one of them a real silent data-drop (Codex reproduced it in-process):

🔴 MUST-FIX 1 — follower can return [] and silently drop sidebar rows (api/models.py:4297)

In _reload_cli_sessions_after_inflight() non-owner branch: when clear_cli_sessions_cache() fires during the owner's rebuild, the owner correctly skips caching (invalidation stamp changed) — but the follower then finds no valid cached entry AND its own stale_sessions is now stamp-invalid (the clear bumped the version), so it falls through to return []. Reproduced in-process: owner returned fresh rows, the follower returned [], load count stayed 1 — the follower's /api/sessions response silently loses the CLI/cron sidebar rows (also hits direct callers like gateway session snapshots / import metadata).
Fix: don't return [] there — after the inflight owner's event fires and there's no valid cache and stale is invalid, re-claim and run a post-invalidation rebuild (loop back through claim after the old inflight slot is popped), caching only if the stamp still matches. This matches the route-level pattern.

🔴 MUST-FIX 2 — unbounded cold-follower wait (api/models.py:4280 and :4948)

When there are no stale rows (cold cache, or right after a clear() popped the entry), the follower waits with event.wait() / event.wait(None) — unbounded. A wedged owner (hung SQLite/mount) blocks every no-stale /api/sessions + CLI-session caller indefinitely, instead of degrading to an independent rebuild like the route-level singleflight (api/routes.py:2368-2399, ~_SESSIONS_CACHE_WAIT_SECONDS).
Fix: give the cold/no-stale wait a finite timeout (~0.25s, mirror the route-level constant), then retry the cache read and run a fallback rebuild outside the inflight slot if still empty.

🟡 NIT (quick) — api/models.py:4305 log string em-dash is mojibake

"... failed â€" check ..." — the bytes are corrupted (c3 a2 e2 82 ac e2 80 9d) vs the correct — on the twin line. Garbage in operator logs; restore the —. (Opus also flagged an unreachable post-return block around :4979-4997 — please drop it if dead.)

Once MUST-FIX 1+2 land (and the nit), I'll re-run the full Codex+Opus+suite and fast-track it — this is the #4842 layer we want. The 293 lines of concurrency tests are great; please add one that reproduces the clear-during-rebuild []-drop (MUST-FIX 1) and one for the bounded cold-wait timeout fallback.

[rodboev]

I tightened the follower side of get_cli_sessions() in api/models.py.

1. Cold followers now wait 0.25s max, then do their own rebuild if the owner still has not produced a cache entry.
2. If clear_cli_sessions_cache() invalidates a joiner during the owner's rebuild, the joiner now loops back, reclaims the rebuild after the old inflight slot drops, and returns fresh rows instead of [].
3. I removed the dead post-return fallback and fixed the warning log string.
4. Added focused regressions for both failure modes in tests/test_issue4842_cli_cache_streaming.py.

[nesquena-hermes]

Shipped in v0.51.671 (Release YA, just deployed) — thanks @rodboev! Closes #4842. The CLI/cron sidebar rebuild now runs outside the cache lock (stale-while-revalidate + singleflight), so a cron-heavy rebuild no longer serializes every /api/sessions poll. Both bounce items resolved: Codex reproduced the prior clear-during-rebuild race and confirmed the follower now re-claims + returns fresh rows (not []); all waits bounded. Gate: Codex SAFE + Opus SHIP IT, suite 10653. Completes the #4842 chain on top of #4889 + #4908/#4949. Verified on prod.