fix(ui): 060 keep session-list refresh icon visibly spinning on active session

[omercnet]

Summary

• The session-list refresh icon never visibly animated when clicked on the currently active session, even though it animated correctly on every non-active session. Root cause was a force: true-ignoring short-circuit in loadMessages plus the absence of any minimum-visible-duration floor on the spinner.
• Replaces the buggy isLoading short-circuit with an in-flight promise registry so force: true reliably awaits any prior load and then refetches, instead of silently no-opping.
• Adds a small pure helper withMinimumDuration (default 450 ms) wrapping the reload await so the spinner is guaranteed perceptible regardless of how fast the work resolves.
• Adds a node:test-based regression test that locks in the spinner-duration floor across six behavioral cases.

Root Cause

packages/ui/src/stores/session-api.ts#loadMessages had three early-return checks at the top:

1. force: true invalidates the messagesLoaded set.
2. alreadyLoaded && !force returns.
3. isLoading returns — and this check ignored force: true.

Whenever the target session was already in the loadingMessages set, loadMessages resolved in the same microtask without doing any work. The currently active session is the only session reliably in that set at click time, because:

• packages/ui/src/components/session/session-view.tsx triggers a loadMessages call inside a createEffect whenever the active session mounts or changes.
• packages/ui/src/stores/session-events.ts fires a force: true reload after every compaction event.
• Any in-flight refetch triggered by SSE hydration leaves the session flagged as loading until the round-trip returns.

Non-active sessions are not in loadingMessages, so they hit the real fetch path and the spinner stayed visible for the duration of the network round-trip by accident. The active session got the silent no-op, handleReloadSession's finally block cleared reloadingSessionIds in the same microtask, and the <Show> in session-list.tsx never paid out a frame with the animated icon.

CSS was ruled out as a contributor — .session-item-active only changes background-color, color, font-weight, box-shadow; nothing overrides animate-spin or the shared @keyframes spin.

What Changed

packages/ui/src/stores/session-api.ts (+61 / -3)

• Add a module-scoped inFlightLoadMessages map keyed by ${instanceId}:${sessionId}.
• Replace the isLoading short-circuit with an in-flight-aware branch:
  • Non-force callers continue to dedupe by awaiting the shared promise and returning.
  • force: true callers wait for the prior load to settle (await existing.catch(() => {})), then fall through to run a fresh fetch.
• Register the new fetch's promise in the map before the first await; resolve / reject it from the existing try/catch/finally and clear the entry under a value-identity check so a slow finally cannot erase a newer registration.
• Same fix incidentally repairs packages/ui/src/stores/session-events.ts line 622 (post-compaction reload), which previously dropped under the same defect and could leave the UI showing stale messages until the next user action.

packages/ui/src/components/session-list.tsx (+11 / -1)

• Import the new helper.
• Wrap the reload await in withMinimumDuration(loadMessages(...), MIN_RELOAD_SPINNER_MS) so the spinner is held for at least 450 ms regardless of how fast the underlying work resolves. Protects the UX against any future fast path (cached responses, no-op early returns, localhost dev servers) that could otherwise flash the spinner imperceptibly.

packages/ui/src/lib/min-duration.ts (new)

• Pure helper module. Exports withMinimumDuration<T>(work, minMs, deps?) and MIN_RELOAD_SPINNER_MS = 450.
• now and delay are injectable so the helper is unit-testable without sleeping real time.
• Both success and failure paths honour the floor so a failed reload still shows the spinner long enough to be perceived before the error toast replaces it.
• Zero internal imports so it is reachable from the project's node:test runner.

packages/ui/src/lib/min-duration.test.ts (new, regression test)

• Six node:test cases exercising fast resolve, slow resolve, fast reject, slow reject, non-positive minMs, and the default-constant bounds.
• Uses an injected virtual clock so the suite finishes in under 5 ms of real time.
• Runnable via node --experimental-strip-types --test src/lib/min-duration.test.ts from packages/ui/, matching the existing pattern in packages/ui/src/components/tool-call/question-active.test.ts.

Verification

Check	Command	Result
Regression test	node --experimental-strip-types --test src/lib/min-duration.test.ts (from packages/ui/)	6/6 pass
Typecheck	npx tsc --noEmit -p tsconfig.json (from packages/ui/)	exit 0
Build	npx vite build (from packages/ui/)	✓ built in 19.81s, exit 0

No SCR required — loadMessages' public contract is being clarified to match its documented intent (force: true means "actually refetch"), not redefined. No copy, no styling, no public API changes.

Scope Notes

• File-length warnings (informational, no refactor in this PR): packages/ui/src/components/session-list.tsx is now 851 lines and packages/ui/src/stores/session-api.ts is now 975 lines, both above the 800-line warning threshold from AGENTS.md.
• The investigation and fix are tracked in tasks/todo/060-session-list-refresh-icon-animation.md.

[shantur]

• Adds a small pure helper withMinimumDuration (default 450 ms) wrapping the reload await so the spinner is guaranteed perceptible regardless of how fast the work resolves.

I don't think that is the correct approach. The sessions are reloaded really quickly hence the spinner isn't shown.
If you need a better feedback that session is reloaded, we can add a toast.

[omercnet]

Thanks for the quick look! I want to make sure we're talking about the same defect before deciding on the fix shape — I think the comment may be reacting to the secondary part of the change rather than the primary one.

What I'm seeing in the code (not a UX hunch — a code path)

The reported bug is "refresh icon doesn't animate on the currently active session." Non-active sessions animate fine. That asymmetry is the smoking gun. Walking the code:

packages/ui/src/stores/session-api.ts — loadMessages has three early-return checks at the top:

if (force) { /* invalidate messagesLoaded set */ }
if (alreadyLoaded && !force) return
const isLoading = loading().loadingMessages.get(instanceId)?.has(sessionId)
if (isLoading) return    // ← ignores force:true

The third check ignores force: true. Whenever the target session is already in loadingMessages, loadMessages resolves in the same microtask without doing any HTTP work at all — it does not just "reload really quickly," it does not reload.

The currently active session is the only session reliably in loadingMessages at click time, because:

• packages/ui/src/components/session/session-view.tsx:196 triggers loadMessages(...) inside a createEffect whenever the active session mounts/changes.
• packages/ui/src/stores/session-events.ts:622 fires loadMessages(..., { force: true }) after every compaction event.
• Any in-flight SSE hydration leaves the session flagged as loading until the round-trip returns.

Non-active sessions are not in loadingMessages, so they go down the real fetch path. The spinner stays visible for the duration of that round-trip — by accident. The active session gets the silent no-op, handleReloadSession's finally block clears reloadingSessionIds in the same microtask, and the <Show> never paints a frame with the animated icon.

So the user-visible symptom (no spinner on active) is downstream of a logic bug: the reload button doesn't actually reload anything when clicked on the active session. A toast saying "Session reloaded" would announce work that didn't happen.

The two parts of this PR are not equivalent

1. session-api.ts in-flight registry — the actual fix. Makes force: true await any prior load and then run a fresh fetch, so the refresh button refetches even when the session is already in flight. Same change also repairs the silent-drop in the post-compaction reload path (session-events.ts:622), which today can leave the UI showing stale messages until the next user action.

2. withMinimumDuration(450ms) wrapper in session-list.tsx — a secondary UX guard. Even after fix (1), there are still fast paths where the reload genuinely takes <50ms (e.g. cached responses, no-op early returns from other code paths, localhost dev). The wrapper guarantees the spinner is perceptible in those cases too.

I hear you that 450ms of forced spinner is a perceptible delay. I'm completely fine dropping the wrapper if you'd rather keep this PR minimal — the session-api fix alone closes the reported bug because, with the in-flight defect fixed, a force: true reload on the active session will actually do its HTTP round-trip, which is more than long enough to render the spinner. A toast could be added later as a separate UX improvement if you decide that's the better signal.

Can you confirm which way you'd like me to take this?

• (a) Keep the session-api fix only — drop the withMinimumDuration helper, its test, and the session-list.tsx wrapper. Smallest possible PR for the reported bug.
• (b) Keep the session-api fix and replace the wrapper with a "Session reloaded" toast on success.
• (c) Keep the PR as-is — wrapper is cheap insurance against future fast paths and the regression test locks it in.

I'd lean (a) — let the real fix stand on its own, and treat any future "spinner too fast" issue as its own UX ticket if it ever materializes. Happy to push that revision as soon as you confirm.