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

packages/ui/src/components/session-list.tsx

@@ -6,6 +6,7 @@ import { Bot, User, Copy, Trash2, Pencil, ShieldAlert, ChevronDown, Search, Squa
 import KeyboardHint from "./keyboard-hint"
 import SessionRenameDialog from "./session-rename-dialog"
 import { keyboardRegistry } from "../lib/keyboard-registry"
+import { MIN_RELOAD_SPINNER_MS, withMinimumDuration } from "../lib/min-duration"
 import { showToastNotification } from "../lib/notifications"
 import { useI18n } from "../lib/i18n"
 import { showConfirmDialog } from "../stores/alerts"
@@ -237,7 +238,16 @@ const SessionList: Component<SessionListProps> = (props) => {
     })
 
     try {
-      await loadMessages(props.instanceId, sessionId, { force: true })
+      // Hold the spinner for at least MIN_RELOAD_SPINNER_MS so the user can
+      // perceive the animation even when the underlying reload resolves
+      // very quickly (e.g. cached responses, or the in-flight dedupe path
+      // in loadMessages reusing an already-running fetch). Without this
+      // floor, the spinner can flash imperceptibly on the currently active
+      // session — task 060.
+      await withMinimumDuration(
+        loadMessages(props.instanceId, sessionId, { force: true }),
+        MIN_RELOAD_SPINNER_MS,
+      )
     } catch (error) {
       log.error(`Failed to reload session ${sessionId}:`, error)
       showToastNotification({ message: t("sessionList.reload.error"), variant: "error" })

packages/ui/src/lib/min-duration.test.ts

@@ -0,0 +1,176 @@
+import assert from "node:assert/strict"
+import { describe, it } from "node:test"
+
+import { MIN_RELOAD_SPINNER_MS, withMinimumDuration } from "./min-duration.ts"
+
+/**
+ * Regression tests for the bug "session list refresh icon does not animate
+ * for the currently active session" (task 060).
+ *
+ * The user-visible symptom is that clicking the reload icon on the active
+ * session shows no spinner because the underlying `loadMessages(..., { force: true })`
+ * can resolve in the same microtask (silent early-return on in-flight reload).
+ * `withMinimumDuration` is the floor that guarantees the spinner remains
+ * visible long enough to be perceived regardless of how fast the work resolves.
+ *
+ * These tests exercise the pure helper with injectable time and delay so
+ * they do not depend on real wall-clock time. They use the existing
+ * `node:test` runner (no vitest in this repo) and are wired to follow the
+ * same pattern as `packages/ui/src/components/tool-call/question-active.test.ts`.
+ */
+
+interface Frame {
+  resolve: () => void
+  scheduledAt: number
+}
+
+/**
+ * Minimal virtual clock. `advance(ms)` moves time forward and resolves any
+ * delays whose deadlines have been reached, in insertion order, so callers
+ * can deterministically drive the helper.
+ */
+function createVirtualClock(start = 1_000_000) {
+  let nowMs = start
+  const frames: Array<Frame & { deadline: number }> = []
+
+  function now(): number {
+    return nowMs
+  }
+
+  function delay(ms: number): Promise<void> {
+    return new Promise<void>((resolve) => {
+      frames.push({ resolve, scheduledAt: nowMs, deadline: nowMs + ms })
+    })
+  }
+
+  async function advance(ms: number): Promise<void> {
+    nowMs += ms
+    // Flush any frames whose deadline is now in the past, in deadline order.
+    frames.sort((a, b) => a.deadline - b.deadline)
+    while (frames.length > 0 && frames[0]!.deadline <= nowMs) {
+      const frame = frames.shift()!
+      frame.resolve()
+      // Yield to the microtask queue so awaiters can observe the resolution
+      // before we continue advancing.
+      await Promise.resolve()
+    }
+  }
+
+  return { now, delay, advance, pending: () => frames.length }
+}
+
+describe("withMinimumDuration", () => {
+  it("holds a fast-resolving promise until the minimum duration has elapsed", async () => {
+    const clock = createVirtualClock()
+
+    const work = Promise.resolve("done")
+    const wrapped = withMinimumDuration(work, 450, { now: clock.now, delay: clock.delay })
+
+    // The inner work has already resolved, so the helper has scheduled a
+    // delay for the remaining 450ms. Advancing time by less than that must
+    // not settle the wrapper.
+    await clock.advance(449)
+    let settled = false
+    void wrapped.then(
+      () => {
+        settled = true
+      },
+      () => {
+        settled = true
+      },
+    )
+    // Give the microtask queue a chance to observe any incorrect early-resolve.
+    await Promise.resolve()
+    await Promise.resolve()
+    assert.equal(settled, false, "wrapper resolved before minimum duration elapsed")
+
+    // Push time over the threshold; the wrapper must now resolve with the
+    // original value.
+    await clock.advance(1)
+    const value = await wrapped
+    assert.equal(value, "done")
+  })
+
+  it("forwards a slow-resolving promise immediately once it resolves (no extra delay)", async () => {
+    const clock = createVirtualClock()
+
+    let resolveWork: ((value: string) => void) | null = null
+    const work = new Promise<string>((resolve) => {
+      resolveWork = resolve
+    })
+    const wrapped = withMinimumDuration(work, 450, { now: clock.now, delay: clock.delay })
+
+    // Advance well past the minimum-duration threshold while the work is
+    // still pending.
+    await clock.advance(900)
+
+    // Now resolve the work. The wrapper must surface the value without
+    // scheduling any additional delay (elapsed > minMs at this point).
+    resolveWork!("late-done")
+    const value = await wrapped
+    assert.equal(value, "late-done")
+    assert.equal(clock.pending(), 0, "no pending delays should remain")
+  })
+
+  it("propagates a fast rejection but holds it until the minimum duration has elapsed", async () => {
+    const clock = createVirtualClock()
+
+    const failure = new Error("boom")
+    const work = Promise.reject(failure)
+    const wrapped = withMinimumDuration(work, 450, { now: clock.now, delay: clock.delay })
+
+    // Attach a catch handler immediately so the unhandled-rejection guard
+    // does not abort the test runner. The handler must not see the
+    // rejection before the minimum duration has elapsed.
+    let observed: unknown = null
+    const observation = wrapped.catch((err) => {
+      observed = err
+    })
+
+    await clock.advance(449)
+    await Promise.resolve()
+    await Promise.resolve()
+    assert.equal(observed, null, "rejection surfaced before minimum duration elapsed")
+
+    await clock.advance(1)
+    await observation
+    assert.equal(observed, failure)
+  })
+
+  it("propagates a slow rejection immediately once the work rejects (no extra delay)", async () => {
+    const clock = createVirtualClock()
+
+    let rejectWork: ((reason: unknown) => void) | null = null
+    const work = new Promise<string>((_resolve, reject) => {
+      rejectWork = reject
+    })
+    const wrapped = withMinimumDuration(work, 450, { now: clock.now, delay: clock.delay })
+
+    await clock.advance(900)
+    const failure = new Error("late-boom")
+    rejectWork!(failure)
+
+    let observed: unknown = null
+    await wrapped.catch((err) => {
+      observed = err
+    })
+    assert.equal(observed, failure)
+    assert.equal(clock.pending(), 0, "no pending delays should remain")
+  })
+
+  it("returns the underlying promise unchanged when minMs is non-positive", async () => {
+    const clock = createVirtualClock()
+    const work = Promise.resolve(42)
+    const wrapped = withMinimumDuration(work, 0, { now: clock.now, delay: clock.delay })
+    assert.equal(await wrapped, 42)
+    assert.equal(clock.pending(), 0, "no delays should be scheduled when minMs <= 0")
+  })
+
+  it("exports a default minimum duration suitable for the reload spinner", () => {
+    // Codified to lock in a UX-tuned value; bumping requires a deliberate
+    // edit to the constant.
+    assert.equal(typeof MIN_RELOAD_SPINNER_MS, "number")
+    assert.ok(MIN_RELOAD_SPINNER_MS >= 300, "minimum should not feel like a flicker")
+    assert.ok(MIN_RELOAD_SPINNER_MS <= 1000, "minimum should not feel sluggish")
+  })
+})

packages/ui/src/lib/min-duration.ts

@@ -0,0 +1,91 @@
+/**
+ * Minimum visible duration helper.
+ *
+ * Wraps an async `work` promise so the resolved/rejected promise it returns
+ * never settles earlier than `minMs` after invocation. This is used to keep
+ * short-lived spinners visible long enough for the user to perceive them,
+ * even when the underlying work resolves in the same microtask (e.g. cached
+ * loads, dedupe early-returns, localhost dev servers).
+ *
+ * The minimum-duration guard applies to BOTH success and failure paths: a
+ * failed reload should still show the spinner long enough to be perceptible
+ * before any error toast replaces it.
+ *
+ * The `now` and `delay` dependencies are injectable so the helper can be
+ * unit-tested deterministically without sleeping real time. By default they
+ * use `Date.now()` and `setTimeout`.
+ *
+ * Concurrent invocations on the same caller-supplied `work` promise are
+ * intentionally not de-duped here; the caller controls scheduling. This
+ * helper is a thin timing wrapper, nothing more.
+ */
+
+/**
+ * Default minimum visible duration for the session-list reload spinner, in
+ * milliseconds. Tuned to feel responsive but visible — values under ~400ms
+ * tend to feel like a flicker.
+ */
+export const MIN_RELOAD_SPINNER_MS = 450
+
+export interface WithMinimumDurationDeps {
+  /** Returns the current timestamp in milliseconds. Defaults to `Date.now`. */
+  now?: () => number
+  /** Resolves after the given number of milliseconds. Defaults to `setTimeout`. */
+  delay?: (ms: number) => Promise<void>
+}
+
+function defaultDelay(ms: number): Promise<void> {
+  return new Promise((resolve) => {
+    setTimeout(resolve, ms)
+  })
+}
+
+/**
+ * Returns a promise that mirrors `work`'s outcome (value or error) but is
+ * guaranteed not to settle before `minMs` have elapsed since the call.
+ *
+ * When `work` resolves before `minMs`, the result is held until `minMs` is
+ * reached. When `work` resolves after `minMs`, the result is forwarded
+ * immediately. The same applies to rejections.
+ */
+export async function withMinimumDuration<T>(
+  work: Promise<T>,
+  minMs: number,
+  deps?: WithMinimumDurationDeps,
+): Promise<T> {
+  const now = deps?.now ?? Date.now
+  const delay = deps?.delay ?? defaultDelay
+
+  // Guard against pathological inputs — never delay if asked for a
+  // non-positive minimum.
+  if (!(minMs > 0)) {
+    return work
+  }
+
+  const startedAt = now()
+
+  // Settle the work and the elapsed-time guard independently so the wrapper
+  // mirrors the work's outcome (value or error) while still enforcing the
+  // floor on perceived duration.
+  let workValue: T
+  let workError: unknown
+  let workSucceeded = false
+
+  try {
+    workValue = await work
+    workSucceeded = true
+  } catch (error) {
+    workError = error
+  }
+
+  const elapsed = now() - startedAt
+  const remaining = minMs - elapsed
+  if (remaining > 0) {
+    await delay(remaining)
+  }
+
+  if (workSucceeded) {
+    return workValue!
+  }
+  throw workError
+}

packages/ui/src/stores/session-api.ts

@@ -700,6 +700,25 @@ async function fetchProviders(instanceId: string): Promise<void> {
   }
 }
 
+/**
+ * Tracks the in-flight `loadMessages` promise for each `(instanceId, sessionId)`
+ * pair. Used to de-duplicate concurrent calls and — critically — to ensure
+ * that `force: true` callers wait for any in-flight load to finish before
+ * starting their own fresh fetch, rather than silently early-returning.
+ *
+ * Without this registry, a `force: true` reload triggered on the currently
+ * active session (which is the only session reliably in `loadingMessages`
+ * at click time, because of the active-session load effect in `session-view`
+ * and post-compaction reloads in `session-events`) would no-op via the
+ * `isLoading` short-circuit. That made the session-list refresh icon appear
+ * to never animate for the active session (task 060).
+ */
+const inFlightLoadMessages = new Map<string, Promise<void>>()
+
+function loadMessagesKey(instanceId: string, sessionId: string): string {
+  return `${instanceId}:${sessionId}`
+}
+
 async function loadMessages(
   instanceId: string,
   sessionId: string,
@@ -725,9 +744,22 @@ async function loadMessages(
     return
   }
 
-  const isLoading = loading().loadingMessages.get(instanceId)?.has(sessionId)
-  if (isLoading) {
-    return
+  // If a load is already in flight for this session, share its promise.
+  // Non-force callers reuse the result; force callers wait for it to finish
+  // (or fail) and then fall through to start a fresh fetch. This replaces
+  // an earlier short-circuit that silently dropped force:true reloads when
+  // any prior load was in flight — see task 060.
+  const inFlightKey = loadMessagesKey(instanceId, sessionId)
+  const existing = inFlightLoadMessages.get(inFlightKey)
+  if (existing) {
+    if (!force) {
+      await existing
+      return
+    }
+    // Wait for the prior load to settle (success or failure) before
+    // starting a fresh fetch. Swallow the prior error so the new attempt
+    // still runs; the new attempt's outcome is what the caller sees.
+    await existing.catch(() => {})
   }
 
   const instance = instances().get(instanceId)
@@ -758,6 +790,22 @@ async function loadMessages(
     return next
   })
 
+  // Register the work as the new in-flight load before performing any
+  // awaits, so concurrent callers (including subsequent force:true clicks)
+  // observe it and queue against it instead of starting parallel fetches.
+  // The work promise is unregistered in the `finally` block below.
+  let resolveInFlight!: () => void
+  let rejectInFlight!: (reason: unknown) => void
+  const inFlight = new Promise<void>((resolve, reject) => {
+    resolveInFlight = resolve
+    rejectInFlight = reject
+  })
+  inFlightLoadMessages.set(inFlightKey, inFlight)
+  // Avoid unhandled-rejection warnings if no concurrent caller awaits this
+  // shared promise. The original work's error is still surfaced to the
+  // direct caller via the `throw` in the catch block below.
+  inFlight.catch(() => {})
+
   try {
     log.info(`[HTTP] GET /session.${"messages"} for instance ${instanceId}`, { sessionId })
     const apiMessages = await requestData<any[]>(
@@ -876,6 +924,7 @@ async function loadMessages(
 
   } catch (error) {
     log.error("Failed to load messages:", error)
+    rejectInFlight(error)
     throw error
   } finally {
     setLoading((prev) => {
@@ -886,6 +935,15 @@ async function loadMessages(
       }
       return next
     })
+    // Always clear our entry from the registry. Only clear if the value
+    // is still ours — a slow finally could otherwise erase a newer
+    // entry registered by an immediately-following call.
+    if (inFlightLoadMessages.get(inFlightKey) === inFlight) {
+      inFlightLoadMessages.delete(inFlightKey)
+    }
+    // Resolve the shared promise so any concurrent waiters (which already
+    // attached their own .catch above for the error case) unblock cleanly.
+    resolveInFlight()
   }
 
   updateSessionInfo(instanceId, sessionId)