fix: reliable generated-image display + stop client remount storm (#927) (#934)

* test: reproduce issue #927

* fix(hub): cache generated images + raise socket buffer cap (closes #927)

Generated-image display was slow and could silently fail:

- The /generated-images route sent `Cache-Control: no-store`, so every
  card remount (session switch, scroll, reload) re-ran the full HTTP ->
  socket.io RPC -> base64 round-trip. The bytes for an imageId are
  immutable, so serve them `private, max-age=31536000, immutable` + ETag.
- socket.io / bun-engine `maxHttpBufferSize` was left at the 1 MB default,
  while the MCP tool accepts images up to 25 MB. The base64 CLI -> hub ack
  frame for anything above ~750 KB raw exceeded the cap and was dropped.
  Raise the buffer to comfortably carry the largest allowed image.

via [HAPI](https://hapi.run)

Co-Authored-By: HAPI <noreply@hapi.run>

* fix(hub): short-circuit generated-image revalidation with a 304 (#927)

The imageId is an immutable content fingerprint, so use it as the ETag and
answer If-None-Match with 304 before issuing the readGeneratedImage RPC.
This makes the ETag actually useful: revalidation now skips the CLI socket
round-trip entirely, and still serves correctly even after the image was
evicted from the CLI's in-memory store.

via [HAPI](https://hapi.run)

Co-Authored-By: HAPI <noreply@hapi.run>

* fix(web): stabilize ApiClient identity across token refresh (#927)

The ApiClient was rebuilt whenever the token value changed (useMemo dep),
even though every request already reads the live token via getToken/tokenRef.
On a flaky/remote connection, repeated 401s -> onUnauthorized -> forced
refresh churned `api`'s identity, which remounts everything keyed on it:
VoiceBackendSession ([props.api]) -> Voice re-register spam, and
GeneratedImageCard ([ctx.api]) -> per-image refetch storm, feeding a
render avalanche. Depend on auth presence (hasToken) instead.

Reproduced with a useAuth hook test (red->green); full web suite stays green.

via [HAPI](https://hapi.run)

Co-Authored-By: HAPI <noreply@hapi.run>

---------

Co-authored-by: HAPI <noreply@hapi.run>

hub/src/socket/server.ts

@@ -9,6 +9,7 @@ import { parseAccessToken } from '../utils/accessToken'
 import { registerCliHandlers } from './handlers/cli'
 import { registerTerminalHandlers } from './handlers/terminal'
 import { RpcRegistry } from './rpcRegistry'
+import { SOCKET_MAX_HTTP_BUFFER_SIZE } from './socketLimits'
 import type { SyncEvent } from '../sync/syncEngine'
 import { TerminalRegistry } from './terminalRegistry'
 import type { CliSocketWithData, SocketData, SocketServer } from './socketTypes'
@@ -62,12 +63,14 @@ export function createSocketServer(deps: SocketServerDeps): {
     }
 
     const io = new Server<DefaultEventsMap, DefaultEventsMap, DefaultEventsMap, SocketData>({
-        cors: corsOptions
+        cors: corsOptions,
+        maxHttpBufferSize: SOCKET_MAX_HTTP_BUFFER_SIZE
     })
 
     const engine = new Engine({
         path: '/socket.io/',
         cors: corsOptions,
+        maxHttpBufferSize: SOCKET_MAX_HTTP_BUFFER_SIZE,
         allowRequest: async (req) => {
             const origin = req.headers.get('origin')
             if (!origin || allowAllOrigins || corsOrigins.includes(origin)) {

hub/src/socket/socketLimits.test.ts

@@ -0,0 +1,14 @@
+import { describe, expect, it } from 'bun:test'
+import { MAX_GENERATED_IMAGE_BYTES, SOCKET_MAX_HTTP_BUFFER_SIZE } from './socketLimits'
+
+describe('socket limits', () => {
+    it('buffer size can carry the largest generated image after base64 + JSON-RPC framing', () => {
+        // Generated images cross the /cli socket as a base64 string inside a JSON-RPC envelope.
+        // base64 inflates the payload by ~4/3; the engine default (1e6) silently drops anything
+        // above ~750 KB raw (issue #927). The buffer must exceed the largest allowed image.
+        const base64Bytes = Math.ceil(MAX_GENERATED_IMAGE_BYTES / 3) * 4
+        expect(SOCKET_MAX_HTTP_BUFFER_SIZE).toBeGreaterThan(base64Bytes)
+        // and must be well above the 1 MB engine default that caused the regression
+        expect(SOCKET_MAX_HTTP_BUFFER_SIZE).toBeGreaterThan(1e6)
+    })
+})

hub/src/socket/socketLimits.ts

@@ -0,0 +1,10 @@
+// The largest generated image the CLI will serve inline. Must stay in sync with the CLI-side
+// limits in cli/src/claude/utils/startHappyServer.ts and cli/src/modules/common/generatedImages.ts.
+export const MAX_GENERATED_IMAGE_BYTES = 25 * 1024 * 1024
+
+// Generated images (and other large RPC payloads) cross the /cli socket as a base64 string wrapped
+// in a JSON-RPC envelope, which inflates the payload by ~4/3. The engine.io default of 1e6 bytes
+// silently drops the CLI -> hub ack frame for any image above ~750 KB raw, so a 25 MB image that
+// the MCP tool happily accepts can never reach the browser (issue #927). Size the buffer to carry
+// the largest allowed image after base64 + framing, with headroom.
+export const SOCKET_MAX_HTTP_BUFFER_SIZE = 48 * 1024 * 1024

hub/src/web/routes/git.test.ts

@@ -0,0 +1,61 @@
+import { describe, expect, it } from 'bun:test'
+import { Hono } from 'hono'
+import type { Session, SyncEngine } from '../../sync/syncEngine'
+import type { WebAppEnv } from '../middleware/auth'
+import { createGitRoutes } from './git'
+
+function buildApp(engine: Partial<SyncEngine>): Hono<WebAppEnv> {
+    const app = new Hono<WebAppEnv>()
+    app.use('*', async (c, next) => {
+        c.set('namespace', 'default')
+        await next()
+    })
+    app.route('/api', createGitRoutes(() => engine as SyncEngine))
+    return app
+}
+
+describe('generated images route', () => {
+    it('serves generated images with an immutable cache header instead of no-store', async () => {
+        const pngBytes = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])
+        const session = { id: 'session-1', namespace: 'default', active: true } as unknown as Session
+        const engine = {
+            resolveSessionAccess: () => ({ ok: true as const, sessionId: 'session-1', session }),
+            readGeneratedImage: async () => ({
+                success: true,
+                content: pngBytes.toString('base64'),
+                mimeType: 'image/png',
+                fileName: 'shot.png'
+            })
+        } as unknown as Partial<SyncEngine>
+
+        const response = await buildApp(engine).request('/api/sessions/session-1/generated-images/img-1')
+
+        expect(response.status).toBe(200)
+        const cacheControl = response.headers.get('cache-control') ?? ''
+        // Generated images are content-addressed by an immutable random id, so they must be
+        // cacheable; `no-store` forces a full RPC round-trip on every remount (issue #927).
+        expect(cacheControl).toContain('immutable')
+        expect(cacheControl).not.toContain('no-store')
+        expect(response.headers.get('etag')).toBe('"img-1"')
+    })
+
+    it('returns 304 without an RPC round-trip when If-None-Match matches', async () => {
+        const session = { id: 'session-1', namespace: 'default', active: true } as unknown as Session
+        let rpcCalls = 0
+        const engine = {
+            resolveSessionAccess: () => ({ ok: true as const, sessionId: 'session-1', session }),
+            readGeneratedImage: async () => {
+                rpcCalls += 1
+                return { success: true, content: '', mimeType: 'image/png', fileName: 'shot.png' }
+            }
+        } as unknown as Partial<SyncEngine>
+
+        const response = await buildApp(engine).request('/api/sessions/session-1/generated-images/img-1', {
+            headers: { 'if-none-match': '"img-1"' }
+        })
+
+        expect(response.status).toBe(304)
+        // The whole point: a cache hit must not touch the CLI over the socket.
+        expect(rpcCalls).toBe(0)
+    })
+})

hub/src/web/routes/git.ts

@@ -35,6 +35,21 @@ async function runRpc<T>(fn: () => Promise<T>): Promise<T | { success: false; er
     }
 }
 
+// Generated-image bytes for a given id never change, so they are cached for a year as immutable.
+const GENERATED_IMAGE_CACHE_CONTROL = 'private, max-age=31536000, immutable'
+
+// Weak comparison of an If-None-Match header against our ETag (handles lists, `*`, and W/ prefixes).
+function ifNoneMatchMatches(header: string | undefined, etag: string): boolean {
+    if (!header) {
+        return false
+    }
+    const normalized = etag.replace(/^W\//, '')
+    return header.split(',').some((candidate) => {
+        const trimmed = candidate.trim()
+        return trimmed === '*' || trimmed.replace(/^W\//, '') === normalized
+    })
+}
+
 export function createGitRoutes(getSyncEngine: () => SyncEngine | null): Hono<WebAppEnv> {
     const app = new Hono<WebAppEnv>()
 
@@ -150,16 +165,31 @@ export function createGitRoutes(getSyncEngine: () => SyncEngine | null): Hono<We
             return c.json({ error: 'Invalid generated image id' }, 400)
         }
 
+        // The id is an immutable content fingerprint, so it doubles as the ETag. If the client
+        // already holds it, answer 304 *before* the RPC so revalidation skips the CLI round-trip
+        // entirely (and still works even if the image was evicted from CLI memory). Issue #927.
+        const etag = `"${parsed.data.imageId}"`
+        if (ifNoneMatchMatches(c.req.header('if-none-match'), etag)) {
+            return c.body(null, 304, {
+                'Cache-Control': GENERATED_IMAGE_CACHE_CONTROL,
+                ETag: etag
+            })
+        }
+
         const result = await runRpc(() => engine.readGeneratedImage(sessionResult.sessionId, parsed.data.imageId))
         if (!result.success || !result.content) {
             return c.json({ success: false, error: result.error ?? 'Generated image not found' }, 404)
         }
 
         const bytes = Uint8Array.from(Buffer.from(result.content, 'base64'))
+        // Generated images are content-addressed by an immutable random id, so the bytes for a
+        // given id never change. Cache aggressively so remounts/scroll/session reopen don't
+        // re-run the full HTTP -> socket.io RPC -> base64 round-trip every time (issue #927).
         return c.body(bytes, 200, {
             'Content-Type': result.mimeType ?? 'application/octet-stream',
             'Content-Disposition': `inline; filename="${encodeURIComponent(result.fileName ?? 'generated-image')}"`,
-            'Cache-Control': 'no-store'
+            'Cache-Control': GENERATED_IMAGE_CACHE_CONTROL,
+            ETag: etag
         })
     })
 

web/src/hooks/useAuth.test.tsx

@@ -0,0 +1,79 @@
+import { act, renderHook, waitFor } from '@testing-library/react'
+import { describe, expect, it, vi } from 'vitest'
+
+// Mock the network layer so we can drive token refreshes deterministically.
+// The real ApiClient reads the live token via `getToken`, so the mock records the
+// constructor options (including onUnauthorized) and hands out incrementing tokens.
+const h = vi.hoisted(() => {
+    let idSeq = 0
+    let authCount = 0
+    class MockApiClient {
+        token: string
+        options: { getToken?: () => string | null; onUnauthorized?: () => unknown; baseUrl?: string } | undefined
+        readonly id: number
+        constructor(token: string, options?: MockApiClient['options']) {
+            this.token = token
+            this.options = options
+            this.id = ++idSeq
+        }
+        async authenticate(): Promise<{ token: string; user: { id: string } }> {
+            authCount += 1
+            return { token: `token-${authCount}`, user: { id: 'u1' } }
+        }
+    }
+    class MockApiError extends Error {
+        status: number
+        code?: string
+        constructor(message: string, status = 401, code?: string) {
+            super(message)
+            this.status = status
+            this.code = code
+        }
+    }
+    return { MockApiClient, MockApiError }
+})
+
+vi.mock('@/api/client', () => ({ ApiClient: h.MockApiClient, ApiError: h.MockApiError }))
+
+// Imported after the mock is registered (vi.mock is hoisted).
+import { useAuth } from '@/hooks/useAuth'
+
+type ApiWithOptions = {
+    id: number
+    options?: { getToken?: () => string | null; onUnauthorized?: () => unknown }
+}
+
+describe('useAuth — api identity stability across token refresh (issue #927)', () => {
+    it('keeps the same ApiClient instance when the token refreshes', async () => {
+        // Stable authSource reference, exactly like the real caller (useAuthSource holds it in
+        // useState). This isolates the bug under test: a *token* refresh, not a source change.
+        const authSource = { type: 'accessToken' as const, token: 'seed' }
+        const { result } = renderHook(() => useAuth(authSource, 'http://hub.test'))
+
+        // Initial authenticate resolves and sets the first token.
+        await waitFor(() => expect(result.current.api).not.toBeNull())
+        const api1 = result.current.api as unknown as ApiWithOptions
+        const token1 = result.current.token
+        expect(token1).toBe('token-1')
+
+        // Drive the exact real-world trigger: a 401 invokes onUnauthorized,
+        // which force-refreshes the token (this is what the flaky remote network does).
+        await act(async () => {
+            await api1.options?.onUnauthorized?.()
+        })
+
+        // The token did advance...
+        expect(result.current.token).toBe('token-2')
+        expect(result.current.token).not.toBe(token1)
+
+        // ...but recreating the client was unnecessary: the OLD instance already serves
+        // the fresh token via getToken, so nothing downstream needed a new `api` reference.
+        expect(api1.options?.getToken?.()).toBe(result.current.token)
+
+        // DESIRED: `api` stays referentially stable across a refresh, so effects keyed on
+        // `api` (VoiceBackendSession `[props.api]`, GeneratedImageCard `[ctx.api, ...]`) do
+        // NOT re-run / remount. On current code `api` is rebuilt because `token` is a useMemo
+        // dep, which drives the Voice-remount spam + per-image refetch storm. This fails today.
+        expect(result.current.api).toBe(api1 as unknown as typeof result.current.api)
+    })
+})

web/src/hooks/useAuth.ts

@@ -157,15 +157,21 @@ export function useAuth(authSource: AuthSource | null, baseUrl: string): {
         }
     }, [baseUrl])
 
+    // Keep the ApiClient referentially stable across token *refreshes*: the client always reads
+    // the live token via getToken (tokenRef), so it never needs rebuilding when the token value
+    // changes — only when auth presence toggles (login/logout). Rebuilding on every refresh churns
+    // `api`'s identity, which remounts everything keyed on it (VoiceBackendSession `[props.api]`,
+    // GeneratedImageCard `[ctx.api, ...]`) and drives the remount/refetch storm. Issue #927.
+    const hasToken = token !== null
     const api = useMemo(() => (
-        token
-            ? new ApiClient(token, {
+        hasToken
+            ? new ApiClient(tokenRef.current ?? '', {
                 baseUrl,
                 getToken: () => tokenRef.current,
                 onUnauthorized: () => refreshAuth({ force: true })
             })
             : null
-    ), [baseUrl, refreshAuth, token])
+    ), [baseUrl, refreshAuth, hasToken])
 
     useEffect(() => {
         let isCancelled = false