From b1fefa52abbc129fc2f96356306861e3d9307241 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:05:43 -0700 Subject: [PATCH 01/20] fix(notifications): backstop wedged rings with a watchdog and pause/abort release A chime that the browser or OS pauses without firing 'ended' held the ring queue for the life of the page, silencing every later bell. Each ring now arms a 5 s safety watchdog (logged at warn as notification_ring_watchdog_fired) that only backstops progression; an Audio 'pause' releases the queue at once and an 'abort' falls back to the built-in tone. The timer is cleared on normal finish and on unmount. Co-Authored-By: Claude Opus 5.5 --- src/hooks/useNotificationSound.ts | 37 +++ .../hooks/useNotificationSound.test.tsx | 228 +++++++++++++++++- 2 files changed, 264 insertions(+), 1 deletion(-) diff --git a/src/hooks/useNotificationSound.ts b/src/hooks/useNotificationSound.ts index 4eb408f45..b7c2f0a7b 100644 --- a/src/hooks/useNotificationSound.ts +++ b/src/hooks/useNotificationSound.ts @@ -1,8 +1,21 @@ import { useCallback, useEffect, useRef } from 'react' import { useAppSelector } from '@/store/hooks' +import { createLogger } from '@/lib/client-logger' + +const log = createLogger('useNotificationSound') const NOTIFICATION_SOUND_SRC = '/your-code-is-ready.mp3' +/** + * Per-ring safety backstop. Progression is driven by completion signals + * (`ended`, `pause`, the fallback tone's `ended`); this timer only releases a + * ring whose completion signal never arrives (e.g. an OS audio interruption + * pauses the element without `ended`), so one dead ring can never silence + * every later bell for the life of the page. The chime is ~1.03 s and the + * fallback tone 0.12 s, so 5 s never cuts an honest ring short. + */ +export const RING_WATCHDOG_MS = 5_000 + /** * Play the built-in fallback tone (the chime file is unusable — a rejected * play() never fires `ended`) and invoke `onDone` when the tone COMPLETES. @@ -63,11 +76,16 @@ export function useNotificationSound() { const queueLengthRef = useRef(0) const ringingRef = useRef(false) const drainRef = useRef<() => void>(() => {}) + const activeWatchdogRef = useRef | null>(null) useEffect(() => { return () => { queueLengthRef.current = 0 ringingRef.current = false + if (activeWatchdogRef.current !== null) { + clearTimeout(activeWatchdogRef.current) + activeWatchdogRef.current = null + } const ctx = audioContextRef.current audioContextRef.current = null @@ -79,9 +97,15 @@ export function useNotificationSound() { const startRing = useCallback(() => { let finished = false + let watchdog: ReturnType | null = null const finish = () => { if (finished) return finished = true + if (watchdog !== null) { + clearTimeout(watchdog) + if (activeWatchdogRef.current === watchdog) activeWatchdogRef.current = null + watchdog = null + } ringingRef.current = false drainRef.current() } @@ -97,6 +121,13 @@ export function useNotificationSound() { if (!playFallbackTone(audioContextRef, finish)) finish() } + watchdog = setTimeout(() => { + if (finished) return + log.warn({ event: 'notification_ring_watchdog_fired', stage: fallbackArmed ? 'fallback' : 'chime' }) + finish() + }, RING_WATCHDOG_MS) + activeWatchdogRef.current = watchdog + try { // A FRESH Audio instance per ring — restarting one shared element would // truncate an in-flight chime. @@ -104,7 +135,13 @@ export function useNotificationSound() { audio.preload = 'auto' audio.volume = 1 audio.addEventListener('ended', finish, { once: true }) + // Per the HTML spec a natural end fires `pause` then `ended`; an + // external pause (OS audio interruption) fires `pause` alone. Either way + // the ring is over — `finish` is idempotent. + audio.addEventListener('pause', finish, { once: true }) audio.addEventListener('error', armFallback, { once: true }) + // An aborted fetch never plays the chime: substitute the tone. + audio.addEventListener('abort', armFallback, { once: true }) const pending = audio.play() if (pending) { pending.catch(armFallback) diff --git a/test/unit/client/hooks/useNotificationSound.test.tsx b/test/unit/client/hooks/useNotificationSound.test.tsx index 5c0d337ed..c0e09711c 100644 --- a/test/unit/client/hooks/useNotificationSound.test.tsx +++ b/test/unit/client/hooks/useNotificationSound.test.tsx @@ -3,9 +3,26 @@ import { renderHook, act, cleanup } from '@testing-library/react' import { configureStore } from '@reduxjs/toolkit' import { Provider } from 'react-redux' import settingsReducer, { defaultSettings } from '@/store/settingsSlice' -import { useNotificationSound } from '@/hooks/useNotificationSound' +import { RING_WATCHDOG_MS, useNotificationSound } from '@/hooks/useNotificationSound' import type { ReactNode } from 'react' +const logMocks = vi.hoisted(() => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), +})) + +vi.mock('@/lib/client-logger', () => ({ + createLogger: () => logMocks, +})) + +function watchdogWarnings() { + return logMocks.warn.mock.calls.filter( + ([entry]) => (entry as { event?: string } | undefined)?.event === 'notification_ring_watchdog_fired', + ) +} + function createStore(soundEnabled: boolean) { return configureStore({ reducer: { @@ -45,6 +62,7 @@ describe('useNotificationSound', () => { let playImpl: () => Promise beforeEach(() => { + logMocks.warn.mockClear() audioInstances = [] playImpl = () => Promise.resolve() AudioSpy = vi.fn(function () { @@ -72,6 +90,7 @@ describe('useNotificationSound', () => { afterEach(() => { cleanup() + vi.useRealTimers() vi.unstubAllGlobals() vi.restoreAllMocks() }) @@ -360,4 +379,211 @@ describe('useNotificationSound', () => { }) expect(toneEndedCallbacks).toHaveLength(0) }) + + describe('ring watchdog backstop', () => { + it('releases a wedged chime that never fires ended: the next ring starts after RING_WATCHDOG_MS and one warn is logged', () => { + vi.useFakeTimers() + const store = createStore(true) + const { result } = renderHook(() => useNotificationSound(), { + wrapper: createWrapper(store), + }) + + act(() => { + result.current.play() + result.current.play() + }) + expect(AudioSpy).toHaveBeenCalledTimes(1) + + // Ring 1 plays but the browser/OS pauses it without `ended` — no + // completion signal ever arrives. Just before the backstop nothing moves. + act(() => { + vi.advanceTimersByTime(RING_WATCHDOG_MS - 1) + }) + expect(AudioSpy).toHaveBeenCalledTimes(1) + expect(watchdogWarnings()).toHaveLength(0) + + act(() => { + vi.advanceTimersByTime(1) + }) + expect(AudioSpy).toHaveBeenCalledTimes(2) + expect(audioInstances[1].play).toHaveBeenCalledTimes(1) + expect(watchdogWarnings()).toHaveLength(1) + expect(watchdogWarnings()[0][0]).toMatchObject({ stage: 'chime' }) + + // A late `ended` from the wedged ring must not double-advance the queue. + act(() => { + audioInstances[0].emit('ended') + }) + expect(AudioSpy).toHaveBeenCalledTimes(2) + }) + + it('clears the watchdog when a ring finishes normally: no backstop fires later', () => { + vi.useFakeTimers() + const store = createStore(true) + const { result } = renderHook(() => useNotificationSound(), { + wrapper: createWrapper(store), + }) + + act(() => { + result.current.play() + }) + act(() => { + vi.advanceTimersByTime(1_000) + audioInstances[0].emit('ended') + }) + expect(vi.getTimerCount()).toBe(0) + + act(() => { + vi.advanceTimersByTime(4_500) + }) + expect(AudioSpy).toHaveBeenCalledTimes(1) + expect(watchdogWarnings()).toHaveLength(0) + }) + + it('does not drain the queue after unmount when a watchdog was pending', () => { + vi.useFakeTimers() + const store = createStore(true) + const { result, unmount } = renderHook(() => useNotificationSound(), { + wrapper: createWrapper(store), + }) + + act(() => { + result.current.play() + result.current.play() + }) + expect(AudioSpy).toHaveBeenCalledTimes(1) + + unmount() + expect(vi.getTimerCount()).toBe(0) + + act(() => { + vi.advanceTimersByTime(RING_WATCHDOG_MS * 2) + }) + expect(AudioSpy).toHaveBeenCalledTimes(1) + expect(watchdogWarnings()).toHaveLength(0) + }) + + it('backstops a fallback tone whose ended never fires', async () => { + vi.useFakeTimers() + class FakeOscillator { + type = 'sine' + frequency = { value: 0 } + connect = vi.fn() + start = vi.fn() + stop = vi.fn() + addEventListener = vi.fn() + } + class FakeAudioContext { + currentTime = 0 + destination = {} + close = vi.fn().mockResolvedValue(undefined) + createOscillator = () => new FakeOscillator() + createGain = () => ({ gain: {}, connect: vi.fn() }) + } + vi.stubGlobal('AudioContext', FakeAudioContext) + playImpl = () => Promise.reject(new Error('chime file failed to load')) + + const store = createStore(true) + const { result } = renderHook(() => useNotificationSound(), { + wrapper: createWrapper(store), + }) + + act(() => { + result.current.play() + result.current.play() + }) + await act(async () => { + await Promise.resolve() + }) + expect(AudioSpy).toHaveBeenCalledTimes(1) + + act(() => { + vi.advanceTimersByTime(RING_WATCHDOG_MS) + }) + expect(AudioSpy).toHaveBeenCalledTimes(2) + expect(watchdogWarnings()).toHaveLength(1) + expect(watchdogWarnings()[0][0]).toMatchObject({ stage: 'fallback' }) + }) + }) + + describe('pause and abort release', () => { + it('a chime paused without ended releases the queue at once', () => { + const store = createStore(true) + const { result } = renderHook(() => useNotificationSound(), { + wrapper: createWrapper(store), + }) + + act(() => { + result.current.play() + result.current.play() + }) + expect(AudioSpy).toHaveBeenCalledTimes(1) + + act(() => { + audioInstances[0].emit('pause') + }) + expect(AudioSpy).toHaveBeenCalledTimes(2) + expect(audioInstances[1].play).toHaveBeenCalledTimes(1) + + // A natural end fires `pause` then `ended`: the trailing `ended` of the + // same ring must not advance the queue a second time. + act(() => { + audioInstances[0].emit('ended') + }) + expect(AudioSpy).toHaveBeenCalledTimes(2) + }) + + it('an aborted chime plays the fallback tone once and the queue continues after it', () => { + const toneEndedCallbacks: Array<() => void> = [] + const oscillatorStarts: number[] = [] + class FakeOscillator { + type = 'sine' + frequency = { value: 0 } + connect = vi.fn() + start = vi.fn(() => { + oscillatorStarts.push(1) + }) + stop = vi.fn() + addEventListener = (type: string, cb: () => void) => { + if (type === 'ended') toneEndedCallbacks.push(cb) + } + } + class FakeAudioContext { + currentTime = 0 + destination = {} + close = vi.fn().mockResolvedValue(undefined) + createOscillator = () => new FakeOscillator() + createGain = () => ({ gain: {}, connect: vi.fn() }) + } + vi.stubGlobal('AudioContext', FakeAudioContext) + + const store = createStore(true) + const { result } = renderHook(() => useNotificationSound(), { + wrapper: createWrapper(store), + }) + + act(() => { + result.current.play() + result.current.play() + }) + + act(() => { + audioInstances[0].emit('abort') + }) + expect(oscillatorStarts).toHaveLength(1) + expect(AudioSpy).toHaveBeenCalledTimes(1) + + // A trailing media error for the same dead ring must not arm a second tone. + act(() => { + audioInstances[0].emit('error') + }) + expect(oscillatorStarts).toHaveLength(1) + + act(() => { + for (const cb of [...toneEndedCallbacks]) cb() + }) + expect(AudioSpy).toHaveBeenCalledTimes(2) + expect(audioInstances[1].play).toHaveBeenCalledTimes(1) + }) + }) }) From 76301ebc1776ef4a065749896894b40eebc68d7d Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:06:48 -0700 Subject: [PATCH 02/20] refactor(notifications): name the ring lifecycle createRingAttempt Move one ring's chime, fallback, pause/abort and watchdog handling into a module-level createRingAttempt with explicit start/dispose, so the hook only owns the queue. Unmount disposes the active ring, which clears its watchdog and guarantees no late completion drains the queue. No behavior change. Co-Authored-By: Claude Opus 5.5 --- src/hooks/useNotificationSound.ts | 133 ++++++++++++++++++------------ 1 file changed, 78 insertions(+), 55 deletions(-) diff --git a/src/hooks/useNotificationSound.ts b/src/hooks/useNotificationSound.ts index b7c2f0a7b..8c56516c7 100644 --- a/src/hooks/useNotificationSound.ts +++ b/src/hooks/useNotificationSound.ts @@ -70,63 +70,55 @@ function playFallbackTone(ctxRef: { current: AudioContext | null }, onDone: () = } } -export function useNotificationSound() { - const soundEnabled = useAppSelector((s) => s.settings.settings.notifications?.soundEnabled ?? true) - const audioContextRef = useRef(null) - const queueLengthRef = useRef(0) - const ringingRef = useRef(false) - const drainRef = useRef<() => void>(() => {}) - const activeWatchdogRef = useRef | null>(null) - - useEffect(() => { - return () => { - queueLengthRef.current = 0 - ringingRef.current = false - if (activeWatchdogRef.current !== null) { - clearTimeout(activeWatchdogRef.current) - activeWatchdogRef.current = null - } - - const ctx = audioContextRef.current - audioContextRef.current = null - if (ctx) { - void ctx.close().catch(() => {}) - } - } - }, []) +type RingAttempt = { + /** Arm the watchdog and play the chime. Call once. */ + start: () => void + /** Silently stop tracking this ring (unmount): clears the watchdog and + * guarantees `onDone` never fires. Idempotent. */ + dispose: () => void +} - const startRing = useCallback(() => { - let finished = false - let watchdog: ReturnType | null = null - const finish = () => { - if (finished) return - finished = true - if (watchdog !== null) { - clearTimeout(watchdog) - if (activeWatchdogRef.current === watchdog) activeWatchdogRef.current = null - watchdog = null - } - ringingRef.current = false - drainRef.current() - } - // Arm the fallback at most once per ring — the chime can fail via the - // play() rejection AND a media `error` event for the same dead ring. - let fallbackArmed = false - const armFallback = () => { - if (fallbackArmed) return - fallbackArmed = true - // A spurious `error` after a successful `ended` must not play an - // audible stray tone for the already-finished ring. - if (finished) return - if (!playFallbackTone(audioContextRef, finish)) finish() +/** + * One ring's lifecycle: play a fresh chime, substitute the fallback tone when + * the chime cannot play, and call `onDone` exactly once when the ring is over — + * on the chime's `ended`/`pause`, on the fallback tone's completion, when no + * completion signal can ever fire, or (backstop only) when RING_WATCHDOG_MS + * elapses without any of those. + */ +function createRingAttempt(ctxRef: { current: AudioContext | null }, onDone: () => void): RingAttempt { + let finished = false + // Arm the fallback at most once per ring — the chime can fail via the + // play() rejection AND a media `error`/`abort` event for the same dead ring. + let fallbackArmed = false + let watchdog: ReturnType | null = null + + const settle = (notify: boolean) => { + if (finished) return + finished = true + if (watchdog !== null) { + clearTimeout(watchdog) + watchdog = null } + if (notify) onDone() + } + const finish = () => settle(true) + + const armFallback = () => { + if (fallbackArmed) return + fallbackArmed = true + // A spurious `error` after a successful `ended` must not play an + // audible stray tone for the already-finished ring. + if (finished) return + if (!playFallbackTone(ctxRef, finish)) finish() + } + const start = () => { watchdog = setTimeout(() => { + watchdog = null if (finished) return log.warn({ event: 'notification_ring_watchdog_fired', stage: fallbackArmed ? 'fallback' : 'chime' }) finish() }, RING_WATCHDOG_MS) - activeWatchdogRef.current = watchdog try { // A FRESH Audio instance per ring — restarting one shared element would @@ -135,8 +127,8 @@ export function useNotificationSound() { audio.preload = 'auto' audio.volume = 1 audio.addEventListener('ended', finish, { once: true }) - // Per the HTML spec a natural end fires `pause` then `ended`; an - // external pause (OS audio interruption) fires `pause` alone. Either way + // Per the HTML spec a natural end fires `pause` then `ended`; an external + // pause (e.g. an OS audio interruption) fires `pause` alone. Either way // the ring is over — `finish` is idempotent. audio.addEventListener('pause', finish, { once: true }) audio.addEventListener('error', armFallback, { once: true }) @@ -149,15 +141,46 @@ export function useNotificationSound() { } catch { armFallback() } + } + + return { start, dispose: () => settle(false) } +} + +export function useNotificationSound() { + const soundEnabled = useAppSelector((s) => s.settings.settings.notifications?.soundEnabled ?? true) + const audioContextRef = useRef(null) + const queueLengthRef = useRef(0) + const activeRingRef = useRef(null) + const drainRef = useRef<() => void>(() => {}) + + useEffect(() => { + return () => { + queueLengthRef.current = 0 + const ring = activeRingRef.current + activeRingRef.current = null + ring?.dispose() + + const ctx = audioContextRef.current + audioContextRef.current = null + if (ctx) { + void ctx.close().catch(() => {}) + } + } }, []) const drain = useCallback(() => { - if (ringingRef.current) return + if (activeRingRef.current) return if (queueLengthRef.current <= 0) return queueLengthRef.current -= 1 - ringingRef.current = true - startRing() - }, [startRing]) + const ring = createRingAttempt(audioContextRef, () => { + if (activeRingRef.current === ring) activeRingRef.current = null + drainRef.current() + }) + // Occupy the slot BEFORE starting: a ring that settles synchronously (no + // audio machinery) re-enters drain from inside start(). + activeRingRef.current = ring + ring.start() + }, []) useEffect(() => { drainRef.current = drain From 11586dcadf29e3e91a49fa418b6e9ee73aaee007 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:08:43 -0700 Subject: [PATCH 03/20] fix(fresh-agent): refresh hidden approvals and questions on reveal without blocking A hidden pane neither refetched nor marked itself dirty for permission and question requests or their cancellations, so after reveal the card could take up to the 3 s busy poll to appear (or a cancelled card lingered). Marking the pane transcript-dirty would be wrong: the blocking "Refreshing conversation" overlay clears only on a newer transcript revision, which an approval never produces. A hidden pane now records that it owes a surface refresh for those events and, on reveal, runs one non-blocking 'reveal-surface' snapshot refresh. A pane that is also transcript-dirty lets the blocking reveal refresh cover it. Co-Authored-By: Claude Opus 5.5 --- src/components/fresh-agent/FreshAgentView.tsx | 27 +++- src/lib/fresh-agent-snapshot-scheduler.ts | 4 +- .../FreshAgentView.hidden-rebind.test.tsx | 124 ++++++++++++++++++ 3 files changed, 152 insertions(+), 3 deletions(-) diff --git a/src/components/fresh-agent/FreshAgentView.tsx b/src/components/fresh-agent/FreshAgentView.tsx index 487cd15c4..701e1a7f4 100644 --- a/src/components/fresh-agent/FreshAgentView.tsx +++ b/src/components/fresh-agent/FreshAgentView.tsx @@ -180,6 +180,15 @@ export const TRANSCRIPT_INVALIDATING_FRESH_AGENT_EVENTS = new Set([ 'freshAgent.rolledBack', 'freshAgent.redone', ]) +// Approval and question lifecycle events change only the pane surface (the +// cards rendered from the snapshot's pendingApprovals/pendingQuestions). A +// hidden pane owes a non-blocking refresh for them on reveal. +export const SURFACE_INVALIDATING_FRESH_AGENT_EVENTS = new Set([ + 'freshAgent.permission.request', + 'freshAgent.permission.cancelled', + 'freshAgent.question.request', + 'freshAgent.question.cancelled', +]) const REVEAL_REFRESH_MAX_WAIT_MS = 15_000 const log = createLogger('FreshAgentView') let nextSnapshotAuditView = 0 @@ -1609,6 +1618,12 @@ export function FreshAgentView({ }, [hidden]) // Snapshot refresh owed at next reveal (set when a reconnect happens hidden). const pendingRevealRefreshRef = useRef(false) + // G26: a NON-blocking surface refresh owed at next reveal, set when an + // approval/question (or its cancellation) arrives while hidden. These do not + // change the transcript, so they must not take the blocking reveal path: + // its overlay clears only on a newer transcript revision, which an approval + // never produces (it would spin for REVEAL_REFRESH_MAX_WAIT_MS, then error). + const surfaceRefreshOwedRef = useRef(false) // Release callback for an in-flight queued CREATE rebind; released by the // freshAgent.created / create-failed ack (or the queue's 10s backstop). const pendingRebindReleaseRef = useRef<(() => void) | null>(null) @@ -1643,6 +1658,7 @@ export function FreshAgentView({ if (previousSnapshotHydrationIdentityRef.current === snapshotHydrationIdentity) return previousSnapshotHydrationIdentityRef.current = snapshotHydrationIdentity snapshotDirtyRef.current = false + surfaceRefreshOwedRef.current = false snapshotDirtyVersionRef.current += 1 snapshotDirtyBaseRevisionRef.current = null revealRefreshVersionRef.current = null @@ -2832,8 +2848,14 @@ export function FreshAgentView({ useEffect(() => { if (hidden) return if (pendingRevealRefreshRef.current) pendingRevealRefreshRef.current = false + if (surfaceRefreshOwedRef.current) { + surfaceRefreshOwedRef.current = false + // A transcript-dirty pane's blocking reveal refresh fetches the same + // snapshot, so it covers the owed surface refresh. + if (!snapshotDirtyRef.current) requestSnapshotRefresh('reveal-surface') + } requestRevealRefresh() - }, [hidden, requestRevealRefresh, snapshotDirty]) + }, [hidden, requestRevealRefresh, requestSnapshotRefresh, snapshotDirty]) // reconcileNotice is a one-shot: visible for 5s, then consumed from the // pane content (a chat pane has no xterm write-notice channel; a timed @@ -3152,6 +3174,9 @@ export function FreshAgentView({ requestRevealRefresh(true) } else if (messageBelongsToPane && isSnapshotInvalidatingFreshAgentEvent(message)) { if (!hiddenRef.current) requestSnapshotRefresh('event') + else if (eventType && SURFACE_INVALIDATING_FRESH_AGENT_EVENTS.has(eventType)) { + surfaceRefreshOwedRef.current = true + } } // kata 1wxv: the requesting-sink ack drives the composer refill; rollback // refusals render their server-pinned message verbatim. Both match on the diff --git a/src/lib/fresh-agent-snapshot-scheduler.ts b/src/lib/fresh-agent-snapshot-scheduler.ts index 3581bd8d1..8513d0356 100644 --- a/src/lib/fresh-agent-snapshot-scheduler.ts +++ b/src/lib/fresh-agent-snapshot-scheduler.ts @@ -18,7 +18,7 @@ import { ApiError } from '@/lib/api' export type SnapshotTrigger = | 'identity' | 'event' | 'send-accepted' | 'materialized' - | 'manual' | 'poll' | 'reconnect' | 'reveal' | 'idle-incomplete' + | 'manual' | 'poll' | 'reconnect' | 'reveal' | 'reveal-surface' | 'idle-incomplete' export type SnapshotOutcome = | { status: 'ok'; value: T } @@ -31,7 +31,7 @@ export const SNAPSHOT_DEBOUNCE_MS = 250 const BACKOFF_BASE_MS = 1_000 const BACKOFF_MAX_MS = 30_000 -const DEBOUNCED_TRIGGERS: ReadonlySet = new Set(['event', 'send-accepted', 'reveal', 'reconnect']) +const DEBOUNCED_TRIGGERS: ReadonlySet = new Set(['event', 'send-accepted', 'reveal', 'reveal-surface', 'reconnect']) export function makeSnapshotKey(input: { sessionType: string; provider: string; threadId: string; cwd?: string; soulId?: string; soulIntentRevision?: number diff --git a/test/unit/client/components/fresh-agent/FreshAgentView.hidden-rebind.test.tsx b/test/unit/client/components/fresh-agent/FreshAgentView.hidden-rebind.test.tsx index 9c3345101..45599041c 100644 --- a/test/unit/client/components/fresh-agent/FreshAgentView.hidden-rebind.test.tsx +++ b/test/unit/client/components/fresh-agent/FreshAgentView.hidden-rebind.test.tsx @@ -398,6 +398,130 @@ describe('FreshAgentView hidden-pane rebind (F8)', () => { expect(screen.getByText('caught up transcript')).toBeInTheDocument() expect(screen.queryByTestId('fresh-agent-stale-transcript')).not.toBeInTheDocument() }) + + describe('hidden approvals and questions owe a non-blocking surface refresh (G26)', () => { + const SESS_SURFACE = '550e8400-e29b-41d4-a716-446655440105' + const bashApproval = { + requestId: 'perm-1', + toolName: 'Bash', + input: { command: 'npm test' }, + } + const question = { + requestId: 'question-1', + questions: [{ + question: 'Which branch should I use?', + header: 'Branch', + options: [{ label: 'main', description: 'The default branch' }], + }], + } + + function surfaceSnapshot(extra: Record = {}) { + return { ...snapshotWithText('current transcript', 5), ...extra } + } + + function emitSurfaceEvent(type: string, extra: Record = {}) { + emitWsMessage({ + type: 'freshAgent.event', + sessionId: SESS_SURFACE, + sessionType: 'freshclaude', + provider: 'claude', + event: { type, sessionId: SESS_SURFACE, ...extra }, + }) + } + + async function mountHiddenSurfacePane(initialSnapshot: Record) { + apiMock.getFreshAgentThreadSnapshot.mockResolvedValueOnce(initialSnapshot) + const paneContent = { ...basePaneContent, sessionId: SESS_SURFACE, status: 'idle' as const } + const view = renderView({ paneContent, hidden: true }) + await act(async () => { await vi.advanceTimersByTimeAsync(500) }) + return { ...view, paneContent } + } + + it('reveals a permission request raised while hidden with one fetch and no blocking overlay', async () => { + const { rerender, paneContent } = await mountHiddenSurfacePane(surfaceSnapshot()) + const callsBeforeRequest = apiMock.getFreshAgentThreadSnapshot.mock.calls.length + expect(screen.queryByRole('alert', { name: 'Permission request for Bash' })).not.toBeInTheDocument() + + emitSurfaceEvent('freshAgent.permission.request', { requestId: 'perm-1', toolName: 'Bash' }) + await act(async () => { await vi.advanceTimersByTimeAsync(500) }) + expect(apiMock.getFreshAgentThreadSnapshot.mock.calls.length).toBe(callsBeforeRequest) + + // Same transcript revision: the approval did not touch the transcript. + apiMock.getFreshAgentThreadSnapshot.mockResolvedValueOnce(surfaceSnapshot({ pendingApprovals: [bashApproval] })) + rerenderView(rerender, { paneContent, hidden: false }) + expect(screen.queryByRole('status', { name: 'Refreshing conversation' })).not.toBeInTheDocument() + await act(async () => { await vi.advanceTimersByTimeAsync(500) }) + + expect(apiMock.getFreshAgentThreadSnapshot.mock.calls.length).toBe(callsBeforeRequest + 1) + expect(screen.getByRole('alert', { name: 'Permission request for Bash' })).toBeInTheDocument() + expect(screen.queryByRole('status', { name: 'Refreshing conversation' })).not.toBeInTheDocument() + expect(screen.queryByTestId('fresh-agent-stale-transcript')).not.toBeInTheDocument() + + // The owed refresh is consumed: hiding and revealing again fetches nothing. + rerenderView(rerender, { paneContent, hidden: true }) + rerenderView(rerender, { paneContent, hidden: false }) + await act(async () => { await vi.advanceTimersByTimeAsync(500) }) + expect(apiMock.getFreshAgentThreadSnapshot.mock.calls.length).toBe(callsBeforeRequest + 1) + }) + + it('reveals a question raised while hidden with one fetch and no blocking overlay', async () => { + const { rerender, paneContent } = await mountHiddenSurfacePane(surfaceSnapshot()) + const callsBeforeRequest = apiMock.getFreshAgentThreadSnapshot.mock.calls.length + + emitSurfaceEvent('freshAgent.question.request', { requestId: 'question-1' }) + await act(async () => { await vi.advanceTimersByTimeAsync(500) }) + expect(apiMock.getFreshAgentThreadSnapshot.mock.calls.length).toBe(callsBeforeRequest) + + apiMock.getFreshAgentThreadSnapshot.mockResolvedValueOnce(surfaceSnapshot({ pendingQuestions: [question] })) + rerenderView(rerender, { paneContent, hidden: false }) + await act(async () => { await vi.advanceTimersByTimeAsync(500) }) + + expect(apiMock.getFreshAgentThreadSnapshot.mock.calls.length).toBe(callsBeforeRequest + 1) + expect(screen.getByRole('region', { name: /^Question from / })).toBeInTheDocument() + expect(screen.queryByRole('status', { name: 'Refreshing conversation' })).not.toBeInTheDocument() + }) + + it('clears a visible approval card that was cancelled while the pane was hidden', async () => { + const { rerender, paneContent } = await mountHiddenSurfacePane(surfaceSnapshot({ pendingApprovals: [bashApproval] })) + expect(screen.getByRole('alert', { name: 'Permission request for Bash' })).toBeInTheDocument() + const callsBeforeCancel = apiMock.getFreshAgentThreadSnapshot.mock.calls.length + + emitSurfaceEvent('freshAgent.permission.cancelled', { requestId: 'perm-1' }) + await act(async () => { await vi.advanceTimersByTimeAsync(500) }) + expect(apiMock.getFreshAgentThreadSnapshot.mock.calls.length).toBe(callsBeforeCancel) + + apiMock.getFreshAgentThreadSnapshot.mockResolvedValueOnce(surfaceSnapshot()) + rerenderView(rerender, { paneContent, hidden: false }) + await act(async () => { await vi.advanceTimersByTimeAsync(500) }) + + expect(apiMock.getFreshAgentThreadSnapshot.mock.calls.length).toBe(callsBeforeCancel + 1) + expect(screen.queryByRole('alert', { name: 'Permission request for Bash' })).not.toBeInTheDocument() + expect(screen.queryByRole('status', { name: 'Refreshing conversation' })).not.toBeInTheDocument() + }) + + it('lets the blocking reveal refresh cover an owed surface refresh when the transcript also changed', async () => { + const { rerender, paneContent } = await mountHiddenSurfacePane(surfaceSnapshot()) + const callsBeforeEvents = apiMock.getFreshAgentThreadSnapshot.mock.calls.length + + emitSurfaceEvent('freshAgent.permission.request', { requestId: 'perm-1', toolName: 'Bash' }) + emitSurfaceEvent('freshAgent.session.changed') + await act(async () => { await vi.advanceTimersByTimeAsync(500) }) + expect(apiMock.getFreshAgentThreadSnapshot.mock.calls.length).toBe(callsBeforeEvents) + + apiMock.getFreshAgentThreadSnapshot.mockResolvedValueOnce({ + ...snapshotWithText('new transcript', 6), + pendingApprovals: [bashApproval], + }) + rerenderView(rerender, { paneContent, hidden: false }) + expect(screen.getByRole('status', { name: 'Refreshing conversation' })).toBeInTheDocument() + await act(async () => { await vi.advanceTimersByTimeAsync(500) }) + + expect(apiMock.getFreshAgentThreadSnapshot.mock.calls.length).toBe(callsBeforeEvents + 1) + expect(screen.getByText('new transcript')).toBeInTheDocument() + expect(screen.getByRole('alert', { name: 'Permission request for Bash' })).toBeInTheDocument() + expect(screen.queryByRole('status', { name: 'Refreshing conversation' })).not.toBeInTheDocument() + }) + }) }) describe('FreshAgentView hidden-pane create rebind (F8)', () => { From a8818c78817a095a524a7b861173c254e8600078 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:11:13 -0700 Subject: [PATCH 04/20] refactor(fresh-agent): extract fresh-agent event classification Move the snapshot-, transcript- and surface-invalidating event sets and their predicates out of FreshAgentView into fresh-agent-event-classes.ts, with one pure classifyHiddenPaneEvent (transcript | surface | none) that the WS handler uses for both the dirty-mark and the owed surface refresh. A table-driven unit test covers every event type. No behavior change. Co-Authored-By: Claude Opus 5.5 --- src/components/fresh-agent/FreshAgentView.tsx | 81 ++----------- .../fresh-agent/fresh-agent-event-classes.ts | 109 ++++++++++++++++++ .../fresh-agent-event-classes.test.ts | 83 +++++++++++++ test/unit/client/lib/fresh-agent-ws.test.ts | 10 +- 4 files changed, 210 insertions(+), 73 deletions(-) create mode 100644 src/components/fresh-agent/fresh-agent-event-classes.ts create mode 100644 test/unit/client/components/fresh-agent/fresh-agent-event-classes.test.ts diff --git a/src/components/fresh-agent/FreshAgentView.tsx b/src/components/fresh-agent/FreshAgentView.tsx index 701e1a7f4..5ae9bf5a9 100644 --- a/src/components/fresh-agent/FreshAgentView.tsx +++ b/src/components/fresh-agent/FreshAgentView.tsx @@ -126,9 +126,15 @@ import { FreshAgentComposer, type FreshAgentComposerHandle } from './FreshAgentC import { FreshAgentDiffPanel } from './FreshAgentDiffPanel' import { FreshAgentSidebar } from './FreshAgentSidebar' import { FreshAgentStatusStrip } from './FreshAgentStatusStrip' +import { + classifyHiddenPaneEvent, + FRESH_AGENT_BUSY_STATUSES, + isSnapshotInvalidatingFreshAgentEvent, + readMessageEventType, +} from './fresh-agent-event-classes' const EARLY_STATES = new Set(['creating', 'starting']) -const BUSY_STATES = new Set(['running', 'compacting']) +const BUSY_STATES = FRESH_AGENT_BUSY_STATUSES // Copy for the stuck-notice card (role="alert") shown while the store carries // the deadman's 'stuck' status; recovery actions live on the card itself. const FRESH_AGENT_STUCK_NOTICE_TEXT = 'Agent appears stuck — no events from the agent process for a while.' @@ -144,51 +150,6 @@ export const FRESH_AGENT_RESERVE_RETRY_FLOOR_MS = 1_000 // up). Exported so tests assert the cap against the real constant. export const IDLE_INCOMPLETE_MAX_RETRIES = 5 const IDLE_INCOMPLETE_RETRY_DELAY_MS = 1_000 -// Exported so tests assert membership against the real constant. -export const SNAPSHOT_INVALIDATING_FRESH_AGENT_EVENTS = new Set([ - 'freshAgent.session.changed', - 'freshAgent.session.snapshot', - 'freshAgent.result', - 'freshAgent.turn.complete', - 'freshAgent.permission.request', - 'freshAgent.permission.cancelled', - 'freshAgent.question.request', - // A provider-cancelled question must also re-drive the snapshot, so the card clears - // even if the freshAgent.question.cancelled fold races (fresh-eyes round-3 F3). - 'freshAgent.question.cancelled', - // kata 1wxv: rollback converges through the invalidating snapshot — the marker - // bucket, the active prefix, and rollback{canRedo,undoneDepth} all ride it. - 'freshAgent.session.rolledBack', - 'freshAgent.session.redone', - 'freshAgent.rolledBack', // the requesting pane's ack also refetches - 'freshAgent.redone', -]) -// These events mean the durable transcript may have changed. Status, approval, -// and question events are deliberately excluded: they update the surrounding -// pane UI, but do not make an already-rendered transcript stale. That keeps a -// hidden pane immediately usable when its last transcript snapshot is still -// current. -export const TRANSCRIPT_INVALIDATING_FRESH_AGENT_EVENTS = new Set([ - 'freshAgent.session.changed', - 'freshAgent.result', - 'freshAgent.turn.complete', - 'freshAgent.error', - 'freshAgent.assistant', - 'freshAgent.stream', - 'freshAgent.session.rolledBack', - 'freshAgent.session.redone', - 'freshAgent.rolledBack', - 'freshAgent.redone', -]) -// Approval and question lifecycle events change only the pane surface (the -// cards rendered from the snapshot's pendingApprovals/pendingQuestions). A -// hidden pane owes a non-blocking refresh for them on reveal. -export const SURFACE_INVALIDATING_FRESH_AGENT_EVENTS = new Set([ - 'freshAgent.permission.request', - 'freshAgent.permission.cancelled', - 'freshAgent.question.request', - 'freshAgent.question.cancelled', -]) const REVEAL_REFRESH_MAX_WAIT_MS = 15_000 const log = createLogger('FreshAgentView') let nextSnapshotAuditView = 0 @@ -797,27 +758,6 @@ function isRecord(value: unknown): value is Record { return !!value && typeof value === 'object' && !Array.isArray(value) } -function readMessageEventType(message: Record): string | undefined { - const event = isRecord(message.event) ? message.event : undefined - return typeof event?.type === 'string' ? event.type : undefined -} - -function isSnapshotInvalidatingFreshAgentEvent(message: Record): boolean { - if (message.type !== 'freshAgent.event') return false - const eventType = readMessageEventType(message) - return Boolean(eventType && SNAPSHOT_INVALIDATING_FRESH_AGENT_EVENTS.has(eventType)) -} - -function isTranscriptInvalidatingFreshAgentEvent(message: Record): boolean { - if (message.type !== 'freshAgent.event') return false - const eventType = readMessageEventType(message) - if (eventType === 'freshAgent.session.snapshot') { - const event = isRecord(message.event) ? message.event : undefined - return typeof event?.status === 'string' && BUSY_STATES.has(event.status) - } - return Boolean(eventType && TRANSCRIPT_INVALIDATING_FRESH_AGENT_EVENTS.has(eventType)) -} - /** * b8ke ext F1: exported for the alias-convergence unit tests (the pure * pane-locator predicate). @@ -3166,7 +3106,8 @@ export function FreshAgentView({ && eventStatus !== undefined && !BUSY_STATES.has(eventStatus) && BUSY_STATES.has(previousStatus) - const transcriptChanged = isTranscriptInvalidatingFreshAgentEvent(message) || busySnapshotSettled + const eventClass = classifyHiddenPaneEvent(message, { busySnapshotSettled }) + const transcriptChanged = eventClass === 'transcript' if (messageBelongsToPane && transcriptChanged && (hiddenRef.current || snapshotDirtyRef.current)) { markSnapshotDirty() } @@ -3174,9 +3115,7 @@ export function FreshAgentView({ requestRevealRefresh(true) } else if (messageBelongsToPane && isSnapshotInvalidatingFreshAgentEvent(message)) { if (!hiddenRef.current) requestSnapshotRefresh('event') - else if (eventType && SURFACE_INVALIDATING_FRESH_AGENT_EVENTS.has(eventType)) { - surfaceRefreshOwedRef.current = true - } + else if (eventClass === 'surface') surfaceRefreshOwedRef.current = true } // kata 1wxv: the requesting-sink ack drives the composer refill; rollback // refusals render their server-pinned message verbatim. Both match on the diff --git a/src/components/fresh-agent/fresh-agent-event-classes.ts b/src/components/fresh-agent/fresh-agent-event-classes.ts new file mode 100644 index 000000000..e1132df28 --- /dev/null +++ b/src/components/fresh-agent/fresh-agent-event-classes.ts @@ -0,0 +1,109 @@ +/** + * Pure classification of fresh-agent WS events by what they make stale in a + * pane. FreshAgentView uses it to decide whether a frame re-drives the + * snapshot, marks a hidden pane's transcript dirty (blocking reveal refresh), + * or only owes a non-blocking surface refresh on reveal. + */ + +export const FRESH_AGENT_BUSY_STATUSES: ReadonlySet = new Set(['running', 'compacting']) + +// Events that re-drive the pane's snapshot while it is visible. +export const SNAPSHOT_INVALIDATING_FRESH_AGENT_EVENTS: ReadonlySet = new Set([ + 'freshAgent.session.changed', + 'freshAgent.session.snapshot', + 'freshAgent.result', + 'freshAgent.turn.complete', + 'freshAgent.permission.request', + 'freshAgent.permission.cancelled', + 'freshAgent.question.request', + // A provider-cancelled question must also re-drive the snapshot, so the card clears + // even if the freshAgent.question.cancelled fold races (fresh-eyes round-3 F3). + 'freshAgent.question.cancelled', + // kata 1wxv: rollback converges through the invalidating snapshot — the marker + // bucket, the active prefix, and rollback{canRedo,undoneDepth} all ride it. + 'freshAgent.session.rolledBack', + 'freshAgent.session.redone', + 'freshAgent.rolledBack', // the requesting pane's ack also refetches + 'freshAgent.redone', +]) + +// These events mean the durable transcript may have changed. Status, approval, +// and question events are deliberately excluded: they update the surrounding +// pane UI, but do not make an already-rendered transcript stale. That keeps a +// hidden pane immediately usable when its last transcript snapshot is still +// current. +export const TRANSCRIPT_INVALIDATING_FRESH_AGENT_EVENTS: ReadonlySet = new Set([ + 'freshAgent.session.changed', + 'freshAgent.result', + 'freshAgent.turn.complete', + 'freshAgent.error', + 'freshAgent.assistant', + 'freshAgent.stream', + 'freshAgent.session.rolledBack', + 'freshAgent.session.redone', + 'freshAgent.rolledBack', + 'freshAgent.redone', +]) + +// Approval and question lifecycle events change only the pane surface (the +// cards rendered from the snapshot's pendingApprovals/pendingQuestions). A +// hidden pane owes a non-blocking refresh for them on reveal; they must not +// take the blocking transcript path, whose overlay clears only on a newer +// transcript revision that an approval never produces. +export const SURFACE_INVALIDATING_FRESH_AGENT_EVENTS: ReadonlySet = new Set([ + 'freshAgent.permission.request', + 'freshAgent.permission.cancelled', + 'freshAgent.question.request', + 'freshAgent.question.cancelled', +]) + +export type HiddenPaneEventClass = 'transcript' | 'surface' | 'none' + +function isRecord(value: unknown): value is Record { + return !!value && typeof value === 'object' && !Array.isArray(value) +} + +function readFreshAgentEvent(message: Record): Record | undefined { + if (message.type !== 'freshAgent.event') return undefined + return isRecord(message.event) ? message.event : undefined +} + +export function readMessageEventType(message: Record): string | undefined { + const event = isRecord(message.event) ? message.event : undefined + return typeof event?.type === 'string' ? event.type : undefined +} + +export function isSnapshotInvalidatingFreshAgentEvent(message: Record): boolean { + const eventType = readFreshAgentEvent(message)?.type + return typeof eventType === 'string' && SNAPSHOT_INVALIDATING_FRESH_AGENT_EVENTS.has(eventType) +} + +export function isTranscriptInvalidatingFreshAgentEvent(message: Record): boolean { + const event = readFreshAgentEvent(message) + const eventType = event?.type + if (eventType === 'freshAgent.session.snapshot') { + return typeof event?.status === 'string' && FRESH_AGENT_BUSY_STATUSES.has(event.status) + } + return typeof eventType === 'string' && TRANSCRIPT_INVALIDATING_FRESH_AGENT_EVENTS.has(eventType) +} + +/** + * What a frame makes stale in its pane: + * - 'transcript': the durable conversation may have changed (a hidden pane + * marks itself dirty and blocks on a fresh revision at reveal). + * - 'surface': only the approval/question cards changed (a hidden pane owes + * one non-blocking refresh at reveal). + * - 'none': nothing a hidden pane needs to refetch. + * + * `busySnapshotSettled` is the caller's observation that a status snapshot + * moved the pane from busy to settled, which ends a turn. + */ +export function classifyHiddenPaneEvent( + message: Record, + { busySnapshotSettled }: { busySnapshotSettled: boolean }, +): HiddenPaneEventClass { + if (isTranscriptInvalidatingFreshAgentEvent(message) || busySnapshotSettled) return 'transcript' + const eventType = readFreshAgentEvent(message)?.type + if (typeof eventType === 'string' && SURFACE_INVALIDATING_FRESH_AGENT_EVENTS.has(eventType)) return 'surface' + return 'none' +} diff --git a/test/unit/client/components/fresh-agent/fresh-agent-event-classes.test.ts b/test/unit/client/components/fresh-agent/fresh-agent-event-classes.test.ts new file mode 100644 index 000000000..e7551d466 --- /dev/null +++ b/test/unit/client/components/fresh-agent/fresh-agent-event-classes.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from 'vitest' +import { + classifyHiddenPaneEvent, + isSnapshotInvalidatingFreshAgentEvent, + type HiddenPaneEventClass, +} from '@/components/fresh-agent/fresh-agent-event-classes' + +function frame(event: Record): Record { + return { type: 'freshAgent.event', sessionId: 's-1', sessionType: 'freshclaude', provider: 'claude', event } +} + +describe('classifyHiddenPaneEvent', () => { + const cases: Array<[string, Record, HiddenPaneEventClass]> = [ + // Transcript: the durable conversation may have changed. + ['session.changed', { type: 'freshAgent.session.changed' }, 'transcript'], + ['result', { type: 'freshAgent.result' }, 'transcript'], + ['turn.complete', { type: 'freshAgent.turn.complete' }, 'transcript'], + ['error', { type: 'freshAgent.error' }, 'transcript'], + ['assistant', { type: 'freshAgent.assistant' }, 'transcript'], + ['stream', { type: 'freshAgent.stream' }, 'transcript'], + ['session.rolledBack', { type: 'freshAgent.session.rolledBack' }, 'transcript'], + ['session.redone', { type: 'freshAgent.session.redone' }, 'transcript'], + ['rolledBack ack', { type: 'freshAgent.rolledBack' }, 'transcript'], + ['redone ack', { type: 'freshAgent.redone' }, 'transcript'], + ['busy snapshot', { type: 'freshAgent.session.snapshot', status: 'running' }, 'transcript'], + ['compacting snapshot', { type: 'freshAgent.session.snapshot', status: 'compacting' }, 'transcript'], + // Surface: only the approval/question cards change. + ['permission.request', { type: 'freshAgent.permission.request' }, 'surface'], + ['permission.cancelled', { type: 'freshAgent.permission.cancelled' }, 'surface'], + ['question.request', { type: 'freshAgent.question.request' }, 'surface'], + ['question.cancelled', { type: 'freshAgent.question.cancelled' }, 'surface'], + // None: status-only frames never stale a hidden pane. + ['idle snapshot', { type: 'freshAgent.session.snapshot', status: 'idle' }, 'none'], + ['status', { type: 'freshAgent.status', status: 'running' }, 'none'], + ['session.metadata', { type: 'freshAgent.session.metadata' }, 'none'], + ['turn.waiting', { type: 'freshAgent.turn.waiting' }, 'none'], + ['missing type', {}, 'none'], + ] + + it.each(cases)('classifies %s', (_label, event, expected) => { + expect(classifyHiddenPaneEvent(frame(event), { busySnapshotSettled: false })).toBe(expected) + }) + + it('classifies a busy-settled idle snapshot as a transcript change', () => { + expect(classifyHiddenPaneEvent( + frame({ type: 'freshAgent.session.snapshot', status: 'idle' }), + { busySnapshotSettled: true }, + )).toBe('transcript') + }) + + it('ignores frames that are not fresh-agent events', () => { + expect(classifyHiddenPaneEvent( + { type: 'freshAgent.created', event: { type: 'freshAgent.permission.request' } }, + { busySnapshotSettled: false }, + )).toBe('none') + }) +}) + +describe('isSnapshotInvalidatingFreshAgentEvent', () => { + it.each([ + 'freshAgent.session.changed', + 'freshAgent.session.snapshot', + 'freshAgent.result', + 'freshAgent.turn.complete', + 'freshAgent.permission.request', + 'freshAgent.permission.cancelled', + 'freshAgent.question.request', + 'freshAgent.question.cancelled', + 'freshAgent.session.rolledBack', + 'freshAgent.session.redone', + 'freshAgent.rolledBack', + 'freshAgent.redone', + ])('re-drives the snapshot for %s', (type) => { + expect(isSnapshotInvalidatingFreshAgentEvent(frame({ type }))).toBe(true) + }) + + it.each(['freshAgent.status', 'freshAgent.stream', 'freshAgent.session.metadata'])( + 'does not re-drive the snapshot for %s', + (type) => { + expect(isSnapshotInvalidatingFreshAgentEvent(frame({ type }))).toBe(false) + }, + ) +}) diff --git a/test/unit/client/lib/fresh-agent-ws.test.ts b/test/unit/client/lib/fresh-agent-ws.test.ts index ff3800b22..9971a1057 100644 --- a/test/unit/client/lib/fresh-agent-ws.test.ts +++ b/test/unit/client/lib/fresh-agent-ws.test.ts @@ -638,8 +638,14 @@ describe('fresh-agent-ws', () => { // The snapshot-driven self-heal path must treat the cancel as invalidating too, so // the card clears even if the fold races (FreshAgentView.tsx's subscription re-query). - const { SNAPSHOT_INVALIDATING_FRESH_AGENT_EVENTS } = await import('@/components/fresh-agent/FreshAgentView') - expect(SNAPSHOT_INVALIDATING_FRESH_AGENT_EVENTS?.has('freshAgent.question.cancelled') ?? false).toBe(true) + const { isSnapshotInvalidatingFreshAgentEvent } = await import('@/components/fresh-agent/fresh-agent-event-classes') + expect(isSnapshotInvalidatingFreshAgentEvent({ + type: 'freshAgent.event', + sessionId, + sessionType: 'freshclaude', + provider: 'claude', + event: { type: 'freshAgent.question.cancelled', sessionId, requestId: 'question-7' }, + })).toBe(true) }) it('handles freshAgent.session.changed without mutating idle status', () => { From b8fab11f06aaa00d950467009fc90ee5a2a46410 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:15:24 -0700 Subject: [PATCH 05/20] fix(fresh-agent): start new and resumed claude panes on the saved default model The model saved for Freshclaude/Kilroy (model dialog or settings popover) was ignored: new panes always used the registry default opus[1m], and the resume path hardcoded the registry default for every fresh provider. One shared resolveFreshAgentPaneModelDefaults now seeds the model for new panes (PaneContainer) and resumed panes (buildResumeContent, used by the sidebar, context menu, handoff, tabs thunk and terminal-to-fresh resume): - claude/kilroy: saved fresh-pane default, then the registry default; the terminal Claude CLI model is deliberately not consulted (decision D1); - codex/opencode: saved default, then the runtime CLI model, then the registry default (unchanged for new panes). Resumed panes start on the saved default (decision D2), which also makes codex resume honor the saved codex default. A no-layout tab restored by TabContent now passes the saved provider settings too. Co-Authored-By: Claude Opus 5.5 --- src/components/TabContent.tsx | 2 + src/components/panes/PaneContainer.tsx | 24 +--- src/lib/fresh-agent-registry.ts | 48 ++++++++ src/lib/session-type-utils.ts | 6 +- .../client/components/TabContent.test.tsx | 48 ++++++++ .../PaneContainer.createContent.test.tsx | 112 ++++++++++++++++++ .../client/lib/session-type-utils.test.ts | 49 ++++++++ test/unit/shared/fresh-agent-registry.test.ts | 38 ++++++ 8 files changed, 307 insertions(+), 20 deletions(-) diff --git a/src/components/TabContent.tsx b/src/components/TabContent.tsx index ac4e3051f..7aefe84dc 100644 --- a/src/components/TabContent.tsx +++ b/src/components/TabContent.tsx @@ -15,6 +15,7 @@ interface TabContentProps { export default function TabContent({ tabId, hidden }: TabContentProps) { const tab = useAppSelector((s) => s.tabs.tabs.find((t) => t.id === tabId)) const defaultNewPane = useAppSelector((s) => s.settings.settings.panes?.defaultNewPane || 'ask') + const freshAgentProviders = useAppSelector((s) => s.settings.settings.freshAgent?.providers) const previousHiddenRef = useRef(hidden) useEffect(() => { @@ -38,6 +39,7 @@ export default function TabContent({ tabId, hidden }: TabContentProps) { sessionId: tabSessionId, cwd: tab.initialCwd, checkpointWorkspace: tab.checkpointWorkspace, + freshAgentProviderSettings: freshAgentProviders?.[resumeSessionType], }) } else if (tab.mode !== 'shell') { defaultContent = { diff --git a/src/components/panes/PaneContainer.tsx b/src/components/panes/PaneContainer.tsx index edb7fa7fc..e6b3b7221 100644 --- a/src/components/panes/PaneContainer.tsx +++ b/src/components/panes/PaneContainer.tsx @@ -14,7 +14,7 @@ import PanePicker, { type PanePickerType } from './PanePicker' import DirectoryPicker from './DirectoryPicker' import { getProviderLabel, isCodingCliProviderName } from '@/lib/coding-cli-utils' import { isFreshAgentProviderName, getFreshAgentProviderConfig } from '@/lib/fresh-agent-provider-utils' -import { normalizeFreshAgentEffort, normalizeFreshAgentModel, resolveFreshAgentPaneCreateEffort, resolveFreshAgentType } from '@/lib/fresh-agent-registry' +import { normalizeFreshAgentEffort, resolveFreshAgentPaneCreateEffort, resolveFreshAgentPaneModelDefaults, resolveFreshAgentType } from '@/lib/fresh-agent-registry' import { clearDraft } from '@/lib/draft-store' import { getTerminalActions } from '@/lib/pane-action-registry' import { renamePaneAfterMirrorReady } from '@/lib/pane-rename' @@ -709,23 +709,11 @@ function PickerWrapper({ ? getFreshAgentProviderConfig(type) : undefined const providerSettings = freshAgentSettings?.providers?.[type] - const providerDefaultModel = typeof providerSettings?.modelSelection?.modelId === 'string' - ? providerSettings.modelSelection.modelId - : undefined - const runtimeProviderConfiguredModel = settings?.codingCli?.providers?.[freshAgentType.runtimeProvider]?.model - const runtimeProviderModel = typeof runtimeProviderConfiguredModel === 'string' - && runtimeProviderConfiguredModel.trim().length > 0 - ? runtimeProviderConfiguredModel - : undefined - const configuredModel = freshAgentType.runtimeProvider === 'codex' || freshAgentType.runtimeProvider === 'opencode' - ? providerDefaultModel - ?? runtimeProviderModel - ?? freshAgentType.defaultModel - : freshAgentType.defaultModel - const model = normalizeFreshAgentModel(freshAgentType.sessionType, freshAgentType.runtimeProvider, configuredModel) ?? configuredModel - const shouldPersistPaneModel = freshAgentType.runtimeProvider !== 'opencode' - || providerDefaultModel !== undefined - || runtimeProviderModel !== undefined + const { model, persistModel: shouldPersistPaneModel } = resolveFreshAgentPaneModelDefaults({ + entry: freshAgentType, + providerSettings, + runtimeProviderModel: settings?.codingCli?.providers?.[freshAgentType.runtimeProvider]?.model, + }) const permissionMode = freshAgentType.settingsVisibility.permissionMode === false ? undefined : providerSettings?.defaultPermissionMode diff --git a/src/lib/fresh-agent-registry.ts b/src/lib/fresh-agent-registry.ts index a1bd0518d..6f3920ad9 100644 --- a/src/lib/fresh-agent-registry.ts +++ b/src/lib/fresh-agent-registry.ts @@ -167,6 +167,54 @@ export type FreshAgentProviderDefaultsForModel = { effort?: string } +/** The saved per-type fresh-pane defaults the model resolver reads. */ +export type FreshAgentPaneModelDefaultsSettings = { + modelSelection?: { modelId?: string } +} + +export type FreshAgentPaneModelDefaults = { + /** The model a new or resumed pane starts on (undefined: provider default). */ + model: string | undefined + /** Whether to stamp `model` into the pane content. An opencode pane with + * nothing configured persists no model, so its provider default applies. */ + persistModel: boolean +} + +function nonBlank(value: string | undefined): string | undefined { + return typeof value === 'string' && value.trim().length > 0 ? value.trim() : undefined +} + +/** + * The one rule for the model a NEW or RESUMED fresh-agent pane starts on. + * An existing pane keeps its own model; this only seeds pane content. + * + * - claude (freshclaude, kilroy): the saved fresh-pane default for the type, + * then the registry default. The terminal Claude CLI model is deliberately + * NOT consulted (decision D1): fresh panes have their own saved default. + * - codex, opencode: the saved fresh-pane default, then the runtime CLI model + * (`codingCli.providers[runtime].model`), then the registry default. + * + * `runtimeProviderModel` is only supplied where the CLI model applies (the + * picker); resume content omits it. + */ +export function resolveFreshAgentPaneModelDefaults(args: { + entry: FreshAgentRegistryEntry + providerSettings?: FreshAgentPaneModelDefaultsSettings + runtimeProviderModel?: string +}): FreshAgentPaneModelDefaults { + const { entry } = args + const saved = nonBlank(args.providerSettings?.modelSelection?.modelId) + const runtimeProviderModel = entry.runtimeProvider === 'claude' + ? undefined + : nonBlank(args.runtimeProviderModel) + const configured = saved ?? runtimeProviderModel ?? entry.defaultModel + const model = normalizeFreshAgentModel(entry.sessionType, entry.runtimeProvider, configured) ?? configured + const persistModel = entry.runtimeProvider !== 'opencode' + || saved !== undefined + || runtimeProviderModel !== undefined + return { model, persistModel } +} + /** * The pane's effective model: the staged pane value first, then the explicit * pane selection, then the persisted provider default, normalized for the diff --git a/src/lib/session-type-utils.ts b/src/lib/session-type-utils.ts index 8c44e4532..b82f90fd3 100644 --- a/src/lib/session-type-utils.ts +++ b/src/lib/session-type-utils.ts @@ -4,7 +4,7 @@ import type { ComponentType } from 'react' import { PROVIDER_ICONS, DefaultProviderIcon } from '@/components/icons/provider-icons' import { isNonShellMode, getProviderLabel } from '@/lib/coding-cli-utils' import { getFreshAgentProviderConfig } from '@/lib/fresh-agent-provider-utils' -import { resolveFreshAgentType } from '@/lib/fresh-agent-registry' +import { resolveFreshAgentPaneModelDefaults, resolveFreshAgentType } from '@/lib/fresh-agent-registry' import type { FreshAgentProviderName, FreshAgentProviderSettings } from '@/lib/fresh-agent-provider-types' import type { CodingCliProviderName } from '@/store/types' import type { @@ -208,6 +208,8 @@ export function buildResumeContent(opts: { const permissionMode = freshAgentType.settingsVisibility.permissionMode === false ? undefined : ps?.defaultPermissionMode ?? freshAgentProviderConfig?.defaultPermissionMode ?? freshAgentType.defaultPermissionMode + // D2: a resumed pane starts on the saved fresh-pane default, like a new one. + const modelDefaults = resolveFreshAgentPaneModelDefaults({ entry: freshAgentType, providerSettings: ps }) return { kind: 'fresh-agent', sessionType: freshAgentType.sessionType, @@ -220,7 +222,7 @@ export function buildResumeContent(opts: { initialCwd: opts.cwd, ...(opts.checkpointWorkspace ? { checkpointWorkspace: opts.checkpointWorkspace } : {}), modelSelection: ps?.modelSelection, - model: freshAgentType.defaultModel, + ...(modelDefaults.persistModel ? { model: modelDefaults.model } : {}), ...(permissionMode ? { permissionMode } : {}), effort: ps?.effort, } diff --git a/test/unit/client/components/TabContent.test.tsx b/test/unit/client/components/TabContent.test.tsx index 27663551b..0d9509066 100644 --- a/test/unit/client/components/TabContent.test.tsx +++ b/test/unit/client/components/TabContent.test.tsx @@ -35,6 +35,7 @@ interface TabConfig { interface StoreOptions { defaultNewPane?: 'ask' | 'shell' | 'browser' | 'editor' + freshAgentProviders?: typeof defaultSettings.freshAgent.providers } function createStore(tabs: TabConfig[], options: StoreOptions = {}) { @@ -44,6 +45,9 @@ function createStore(tabs: TabConfig[], options: StoreOptions = {}) { ...defaultSettings.panes, defaultNewPane: options.defaultNewPane || 'ask', }, + ...(options.freshAgentProviders + ? { freshAgent: { ...defaultSettings.freshAgent, providers: options.freshAgentProviders } } + : {}), } return configureStore({ reducer: { @@ -194,6 +198,50 @@ describe('TabContent', () => { ) }) + it('restores a no-layout fresh-agent tab on the saved fresh-pane default model and permission mode', () => { + const store = createStore([ + { + id: 'tab-1', + mode: 'claude', + sessionRef: { + provider: 'claude', + sessionId: '550e8400-e29b-41d4-a716-446655440000', + }, + sessionMetadataByKey: { + 'claude:550e8400-e29b-41d4-a716-446655440000': { + sessionType: 'freshclaude', + }, + }, + }, + ], { + freshAgentProviders: { + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'sonnet' }, + defaultPermissionMode: 'default', + }, + }, + }) + + render( + + + + ) + + expect(mockPaneLayout).toHaveBeenCalledWith( + expect.objectContaining({ + defaultContent: expect.objectContaining({ + kind: 'fresh-agent', + sessionType: 'freshclaude', + model: 'sonnet', + modelSelection: { kind: 'exact', modelId: 'sonnet' }, + permissionMode: 'default', + }), + }), + expect.anything(), + ) + }) + it('restores fresh-agent default content for shell-mode no-layout tabs using persisted codingCliProvider metadata', () => { const checkpointWorkspace = { path: '/selected/repo', provenance: 'directory-picker' as const } const store = createStore([ diff --git a/test/unit/client/components/panes/PaneContainer.createContent.test.tsx b/test/unit/client/components/panes/PaneContainer.createContent.test.tsx index a2eb83e60..4944a84d0 100644 --- a/test/unit/client/components/panes/PaneContainer.createContent.test.tsx +++ b/test/unit/client/components/panes/PaneContainer.createContent.test.tsx @@ -536,6 +536,118 @@ describe('createContentForType with ext: prefix', () => { }) }) + describe('saved fresh-pane default model (G11)', () => { + async function createClaudeFamilyPane({ + label, + settingsOverrides, + featureFlags = {}, + }: { + label: 'Freshclaude' | 'Kilroy' + settingsOverrides: ServerSettingsPatch + featureFlags?: Record + }) { + const node = createPickerNode('pane-1') + const store = createStore( + { layouts: { 'tab-1': node }, activePane: { 'tab-1': 'pane-1' } }, + [], + settingsOverrides, + { + status: 'ready', + platform: 'linux', + availableClis: { claude: true }, + featureFlags, + }, + ) + + render( + + + , + ) + + fireEvent.click(screen.getByRole('button', { name: label })) + fireEvent.transitionEnd(getPickerContainer()) + const input = await screen.findByLabelText(`Starting directory for ${label}`) + fireEvent.change(input, { target: { value: '/workspace/project' } }) + fireEvent.keyDown(input, { key: 'Enter' }) + + let created: Extract['content'] | undefined + await waitFor(() => { + const paneContent = (store.getState().panes.layouts['tab-1'] as Extract).content + expect(paneContent.kind).toBe('fresh-agent') + created = paneContent + }) + if (!created || created.kind !== 'fresh-agent') throw new Error('expected fresh-agent content') + return created + } + + it('starts a new Freshclaude pane on the saved default model', async () => { + const content = await createClaudeFamilyPane({ + label: 'Freshclaude', + settingsOverrides: { + codingCli: { enabledProviders: ['claude'] }, + freshAgent: { + enabled: true, + providers: { + freshclaude: { modelSelection: { kind: 'exact', modelId: 'sonnet' } }, + }, + }, + }, + }) + + expect(content.model).toBe('sonnet') + expect(content.modelSelection).toEqual({ kind: 'exact', modelId: 'sonnet' }) + }) + + it('starts a new Kilroy pane on the saved Kilroy default model', async () => { + const content = await createClaudeFamilyPane({ + label: 'Kilroy', + featureFlags: { kilroy: true }, + settingsOverrides: { + codingCli: { enabledProviders: ['claude'] }, + freshAgent: { + enabled: true, + providers: { + freshclaude: { modelSelection: { kind: 'exact', modelId: 'opus[1m]' } }, + kilroy: { modelSelection: { kind: 'exact', modelId: 'sonnet' } }, + }, + }, + }, + }) + + expect(content.sessionType).toBe('kilroy') + expect(content.model).toBe('sonnet') + expect(content.modelSelection).toEqual({ kind: 'exact', modelId: 'sonnet' }) + }) + + it('starts on the registry default when no fresh-pane default is saved', async () => { + const content = await createClaudeFamilyPane({ + label: 'Freshclaude', + settingsOverrides: { + codingCli: { enabledProviders: ['claude'] }, + freshAgent: { enabled: true }, + }, + }) + + expect(content.model).toBe('opus[1m]') + }) + + it('does not fall back to the terminal Claude CLI model (decision D1)', async () => { + const content = await createClaudeFamilyPane({ + label: 'Freshclaude', + settingsOverrides: { + codingCli: { + enabledProviders: ['claude'], + providers: { claude: { model: 'haiku' } }, + }, + freshAgent: { enabled: true }, + }, + }) + + expect(content.model).toBe('opus[1m]') + }) + }) + it('creates editor content when the editor option is selected', async () => { const node = createPickerNode('pane-1') const store = createStore( diff --git a/test/unit/client/lib/session-type-utils.test.ts b/test/unit/client/lib/session-type-utils.test.ts index ecef5c0fc..a498ef2be 100644 --- a/test/unit/client/lib/session-type-utils.test.ts +++ b/test/unit/client/lib/session-type-utils.test.ts @@ -235,6 +235,55 @@ describe('buildResumeContent', () => { expect(content.effort).toBe('turbo') }) + describe('resumed panes start on the saved fresh-pane default model (G11, decision D2)', () => { + it('resumes a Freshclaude session on the saved default model', () => { + const content = buildResumeContent({ + sessionType: 'freshclaude', + sessionId: 'abc-123', + freshAgentProviderSettings: { + modelSelection: { kind: 'exact', modelId: 'sonnet' }, + }, + }) + if (content.kind !== 'fresh-agent') throw new Error('expected fresh-agent') + expect(content.model).toBe('sonnet') + expect(content.modelSelection).toEqual({ kind: 'exact', modelId: 'sonnet' }) + }) + + it('resumes a Freshclaude session on the registry default when nothing is saved', () => { + const content = buildResumeContent({ sessionType: 'freshclaude', sessionId: 'abc-123' }) + if (content.kind !== 'fresh-agent') throw new Error('expected fresh-agent') + expect(content.model).toBe('opus[1m]') + }) + + it('resumes a Kilroy session on the saved Kilroy default model', () => { + const saved = buildResumeContent({ + sessionType: 'kilroy', + sessionId: 'xyz-789', + freshAgentProviderSettings: { + modelSelection: { kind: 'exact', modelId: 'sonnet' }, + }, + }) + if (saved.kind !== 'fresh-agent') throw new Error('expected fresh-agent') + expect(saved.model).toBe('sonnet') + + const unsaved = buildResumeContent({ sessionType: 'kilroy', sessionId: 'xyz-789' }) + if (unsaved.kind !== 'fresh-agent') throw new Error('expected fresh-agent') + expect(unsaved.model).toBe('opus[1m]') + }) + + it('resumes a Freshcodex session on the saved Freshcodex default model', () => { + const content = buildResumeContent({ + sessionType: 'freshcodex', + sessionId: 'codex-session-1', + freshAgentProviderSettings: { + modelSelection: { kind: 'exact', modelId: 'gpt-5.6-luna' }, + }, + }) + if (content.kind !== 'fresh-agent') throw new Error('expected fresh-agent') + expect(content.model).toBe('gpt-5.6-luna') + }) + }) + it('does not stamp visual style onto fresh-agent resume content', () => { const content = buildResumeContent({ sessionType: 'freshcodex', diff --git a/test/unit/shared/fresh-agent-registry.test.ts b/test/unit/shared/fresh-agent-registry.test.ts index 74e53c6ec..0b18f67be 100644 --- a/test/unit/shared/fresh-agent-registry.test.ts +++ b/test/unit/shared/fresh-agent-registry.test.ts @@ -2,10 +2,18 @@ import { describe, expect, it } from 'vitest' import { getEffectiveFreshAgentEffort, + resolveFreshAgentPaneModelDefaults, resolveFreshAgentType, resolveFreshAgentPaneCreateEffort, + type FreshAgentRegistryEntry, } from '@/lib/fresh-agent-registry' +function registryEntry(sessionType: string): FreshAgentRegistryEntry { + const entry = resolveFreshAgentType(sessionType) + if (!entry) throw new Error(`no registry entry for ${sessionType}`) + return entry +} + describe('fresh-agent registry', () => { it('keeps kilroy as a hidden claude-backed fresh-agent type', () => { expect(resolveFreshAgentType('kilroy')).toMatchObject({ @@ -142,3 +150,33 @@ describe('getEffectiveFreshAgentEffort', () => { })).toBe('high') }) }) + +describe('resolveFreshAgentPaneModelDefaults', () => { + const saved = (modelId: string) => ({ modelSelection: { kind: 'exact' as const, modelId } }) + + it.each([ + // [label, sessionType, providerSettings, runtimeProviderModel, expected model, expected persistModel] + ['claude: saved default wins', 'freshclaude', saved('sonnet'), undefined, 'sonnet', true], + ['claude: registry default when nothing is saved', 'freshclaude', undefined, undefined, 'opus[1m]', true], + ['claude: a blank saved id falls back to the registry default', 'freshclaude', saved(' '), undefined, 'opus[1m]', true], + ['claude: never falls back to the terminal CLI model (D1)', 'freshclaude', undefined, 'haiku', 'opus[1m]', true], + ['claude: saved default beats the terminal CLI model', 'freshclaude', saved('sonnet'), 'haiku', 'sonnet', true], + ['kilroy: saved default wins', 'kilroy', saved('sonnet'), undefined, 'sonnet', true], + ['kilroy: registry default when nothing is saved', 'kilroy', undefined, undefined, 'opus[1m]', true], + ['codex: saved default wins', 'freshcodex', saved('gpt-5.6-luna'), 'gpt-5.6-terra', 'gpt-5.6-luna', true], + ['codex: runtime CLI model when nothing is saved', 'freshcodex', undefined, 'gpt-5.6-terra', 'gpt-5.6-terra', true], + ['codex: unknown saved model normalizes to the codex default', 'freshcodex', saved('gpt-unknown'), undefined, 'gpt-6-astra', true], + ['codex: registry default when nothing is configured', 'freshcodex', undefined, undefined, 'gpt-6-astra', true], + ['opencode: saved default persists', 'freshopencode', saved('umans/kimi'), undefined, 'umans/kimi', true], + ['opencode: runtime CLI model persists', 'freshopencode', undefined, 'umans/kimi', 'umans/kimi', true], + ['opencode: nothing configured persists no model', 'freshopencode', undefined, undefined, undefined, false], + ] as const)('%s', (_label, sessionType, providerSettings, runtimeProviderModel, model, persistModel) => { + const result = resolveFreshAgentPaneModelDefaults({ + entry: registryEntry(sessionType), + providerSettings, + runtimeProviderModel, + }) + expect(result.model).toBe(model) + expect(result.persistModel).toBe(persistModel) + }) +}) From a1c30231facf993523967f99050eca5fa1ec3691 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:22:34 -0700 Subject: [PATCH 06/20] fix(fresh-agent): carry a saved default model's effort levels into new panes A saved catalog-only Freshclaude/Kilroy default (one absent from the static opus table) started new panes with effort normalized against the opus table, so a model with no effort levels was still sent 'high'. The model dialog and the settings popover now save the levels they already stamp on the pane as freshAgent.providers[type].modelEffortLevels ({modelId, levels}, null clears). The shared resolver returns them only for claude and only when paired with the resolved model, PaneContainer and resume content stamp them as the pane's modelEffortLevels, and resolveFreshAgentPaneCreateEffort clamps against them. The clamp is one extracted clampEffortToStampedLevels shared with getEffectiveFreshAgentEffort. A Rust settings-store test pins that the opaque providers subtree keeps and clears the field. Co-Authored-By: Claude Opus 5.5 --- crates/freshell-server/src/settings_store.rs | 53 +++++++++++++++ shared/settings.ts | 26 ++++++- .../fresh-agent/FreshAgentModelDialog.tsx | 18 +++-- .../fresh-agent/FreshAgentSettingsButton.tsx | 13 +++- src/components/panes/PaneContainer.tsx | 4 +- src/lib/fresh-agent-provider-types.ts | 2 + src/lib/fresh-agent-registry.ts | 63 +++++++++++++---- src/lib/session-type-utils.ts | 1 + src/store/settingsThunks.ts | 6 ++ .../FreshAgentModelDialog.test.tsx | 14 ++++ .../FreshAgentSettingsButton.test.tsx | 14 ++++ .../fresh-agent/FreshAgentView.test.tsx | 50 ++++++++++++++ .../PaneContainer.createContent.test.tsx | 67 +++++++++++++++++++ .../client/lib/session-type-utils.test.ts | 14 ++++ test/unit/client/store/settingsThunks.test.ts | 43 ++++++++++++ test/unit/shared/fresh-agent-registry.test.ts | 52 ++++++++++++++ test/unit/shared/settings.test.ts | 44 ++++++++++++ 17 files changed, 459 insertions(+), 25 deletions(-) diff --git a/crates/freshell-server/src/settings_store.rs b/crates/freshell-server/src/settings_store.rs index 86aab3dfd..3cbe1e99e 100644 --- a/crates/freshell-server/src/settings_store.rs +++ b/crates/freshell-server/src/settings_store.rs @@ -2677,6 +2677,59 @@ mod tests { std::fs::remove_dir_all(&dir).ok(); } + /// G11: the effort levels saved with a fresh-pane default model travel + /// with it through PATCH, GET, config.json and restart, and the client's + /// `null` clear sentinel removes them (the `freshAgent.providers` subtree + /// is opaque to the server and deep-merged). + #[tokio::test] + async fn fresh_agent_provider_model_effort_levels_round_trip_and_clear() { + let dir = std::env::temp_dir().join(format!("frs-settings-{}", uuid_like())); + std::fs::create_dir_all(dir.join(".freshell")).unwrap(); + let store = store_at(&dir); + let levels = json!({ "modelId": "claude-haiku-x", "levels": [] }); + + store + .patch(&json!({ "freshAgent": { "providers": { "freshclaude": { + "modelSelection": { "kind": "exact", "modelId": "claude-haiku-x" }, + "modelEffortLevels": levels, + } } } })) + .await + .unwrap(); + let got = store.get().await; + assert_eq!( + got.fresh_agent.providers["freshclaude"]["modelEffortLevels"], + levels + ); + + // An unrelated provider patch keeps the levels. + store + .patch( + &json!({ "freshAgent": { "providers": { "freshclaude": { "effort": "low" } } } }), + ) + .await + .unwrap(); + let restarted = store_at(&dir); + let after = restarted.get().await; + assert_eq!( + after.fresh_agent.providers["freshclaude"]["modelEffortLevels"], + levels + ); + assert_eq!(after.fresh_agent.providers["freshclaude"]["effort"], "low"); + + // The clear sentinel removes them. + let cleared = restarted + .patch(&json!({ "freshAgent": { "providers": { "freshclaude": { "modelEffortLevels": null } } } })) + .await + .unwrap(); + assert!(cleared.fresh_agent.providers["freshclaude"]["modelEffortLevels"].is_null()); + assert_eq!( + cleared.fresh_agent.providers["freshclaude"]["modelSelection"]["modelId"], + "claude-haiku-x" + ); + + std::fs::remove_dir_all(&dir).ok(); + } + fn valid5() -> Vec { ["claude", "codex", "gemini", "kimi", "opencode"] .iter() diff --git a/shared/settings.ts b/shared/settings.ts index 981b6abd7..a02e4da65 100644 --- a/shared/settings.ts +++ b/shared/settings.ts @@ -141,11 +141,23 @@ export type CodingCliSettings = { mcpServer: boolean } +/** + * The effort levels a fresh-pane default model offers, captured when the + * model was picked (catalog-only claude models are absent from the static + * model table). Paired with the model id so a later model change can never + * pick up the wrong levels; an empty list means the model declares none. + */ +export type FreshAgentModelEffortLevels = { + modelId: string + levels: string[] +} + export type FreshAgentProviderDefaults = { modelSelection?: FreshAgentModelSelection defaultPermissionMode?: string effort?: FreshAgentEffort style?: FreshAgentStyle + modelEffortLevels?: FreshAgentModelEffortLevels } export type ServerSettings = { @@ -746,6 +758,13 @@ function createCodingCliProviderConfigPatchSchema() { .strict() } +const FreshAgentModelEffortLevelsSchema = z + .object({ + modelId: z.string().min(1), + levels: z.array(z.string()), + }) + .strict() + function createFreshAgentProviderDefaultsSchema() { return z .object({ @@ -753,6 +772,7 @@ function createFreshAgentProviderDefaultsSchema() { defaultPermissionMode: z.string().optional(), effort: FreshAgentModelCapabilitiesOpaqueStringSchema.optional(), style: FreshAgentStyleSchema.optional(), + modelEffortLevels: FreshAgentModelEffortLevelsSchema.optional(), }) .strict() } @@ -764,6 +784,7 @@ function createFreshAgentProviderDefaultsPatchSchema() { defaultPermissionMode: z.string().optional(), effort: z.union([FreshAgentModelCapabilitiesOpaqueStringSchema, z.literal('')]).nullable().optional(), style: FreshAgentStyleSchema.optional(), + modelEffortLevels: FreshAgentModelEffortLevelsSchema.nullable().optional(), }) .strict() } @@ -1030,6 +1051,9 @@ function sanitizeFreshAgentServerSettingsPatchInput( if (hasOwn(normalizedProviderPatchInput, 'style')) { normalizedProviderPatch.style = parsed.data.style } + if (hasOwn(normalizedProviderPatchInput, 'modelEffortLevels')) { + normalizedProviderPatch.modelEffortLevels = parsed.data.modelEffortLevels ?? undefined + } providers[providerName] = normalizedProviderPatch } } @@ -1268,7 +1292,7 @@ function normalizeLegacyFreshAgentProviderDefaultsInput( const normalized = pickOwnKeysPreservingUndefined( providerPatch, - ['modelSelection', 'defaultPermissionMode', 'effort', 'style'], + ['modelSelection', 'defaultPermissionMode', 'effort', 'style', 'modelEffortLevels'], ) if ( diff --git a/src/components/fresh-agent/FreshAgentModelDialog.tsx b/src/components/fresh-agent/FreshAgentModelDialog.tsx index 336487887..f50c34cb1 100644 --- a/src/components/fresh-agent/FreshAgentModelDialog.tsx +++ b/src/components/fresh-agent/FreshAgentModelDialog.tsx @@ -345,6 +345,15 @@ export function FreshAgentModelDialog({ // delete). On the wire that means no variant — opencode applies the // model's own provider-side default. const level = highlightedLevelRow.kind === 'level' ? highlightedLevelRow.id : undefined + // Claude providers stamp the switched-to row's known levels (static or + // probed) so effort normalization clamps against THEM — the same idiom + // the settings popover's radio commit uses. An empty stamp is deliberate: + // the selected model declares no levels. The same levels are saved with + // the provider default, paired with the model, so a new pane on this + // default clamps against them too. + const modelEffortLevels = paneContent.provider === 'claude' + ? (model.supportsEffort ? [...model.supportedEffortLevels] : []) + : undefined dispatch(mergePaneContent({ tabId, paneId, @@ -361,13 +370,7 @@ export function FreshAgentModelDialog({ ? { modelLabel: { modelId: model.id, label: model.displayName } } : {}), effort: level, - // Claude providers stamp the switched-to row's known levels (static - // or probed) so post-commit effort normalization clamps against THEM - // — the same idiom the settings popover's radio commit uses. An - // empty stamp is deliberate: the selected model declares no levels. - ...(paneContent.provider === 'claude' - ? { modelEffortLevels: model.supportsEffort ? [...model.supportedEffortLevels] : [] } - : {}), + ...(modelEffortLevels ? { modelEffortLevels } : {}), }, })) // Apply the change to the LIVE session too (fire-and-forget): the server @@ -387,6 +390,7 @@ export function FreshAgentModelDialog({ [sessionType]: { modelSelection: { kind: 'exact', modelId: model.id }, effort: level, + ...(modelEffortLevels ? { modelEffortLevels: { modelId: model.id, levels: modelEffortLevels } } : {}), }, }, }, diff --git a/src/components/fresh-agent/FreshAgentSettingsButton.tsx b/src/components/fresh-agent/FreshAgentSettingsButton.tsx index bb279197d..77debeb56 100644 --- a/src/components/fresh-agent/FreshAgentSettingsButton.tsx +++ b/src/components/fresh-agent/FreshAgentSettingsButton.tsx @@ -188,6 +188,7 @@ export function FreshAgentSettingsButton({ defaultPermissionMode?: string effort?: string style?: FreshAgentStyle + modelEffortLevels?: { modelId: string; levels: string[] } | undefined }) => { void dispatch(saveServerSettingsPatch({ freshAgent: { @@ -363,6 +364,9 @@ export function FreshAgentSettingsButton({ // list entirely — keep the shared static-table // normalizer, unchanged. const nextRow = modelOptions.find((row) => row.value === nextModel) + const nextModelEffortLevels = nextRow?.thinkingEfforts + ? [...nextRow.thinkingEfforts] + : undefined const nextEffort = nextRow ? effortForSwitchedModelRow(nextRow, paneContent.effort) : normalizeFreshAgentEffort( @@ -391,9 +395,7 @@ export function FreshAgentSettingsButton({ // the static table's default-model fallback for // probed-only models. Absent row → field clears // back to static-table normalization. - modelEffortLevels: nextRow?.thinkingEfforts - ? [...nextRow.thinkingEfforts] - : undefined, + modelEffortLevels: nextModelEffortLevels, }, })) // Apply to the LIVE session (fire-and-forget): the @@ -404,9 +406,14 @@ export function FreshAgentSettingsButton({ model: nextModel, ...(nextEffort ? { effort: nextEffort } : {}), }) + // The same levels travel with the saved default, + // paired with the model (absent row → clear). persistProviderDefaults({ modelSelection: { kind: 'exact', modelId: nextModel }, ...(nextEffort ? { effort: nextEffort } : {}), + modelEffortLevels: nextModelEffortLevels + ? { modelId: nextModel, levels: nextModelEffortLevels } + : undefined, }) }} /> diff --git a/src/components/panes/PaneContainer.tsx b/src/components/panes/PaneContainer.tsx index e6b3b7221..9a15f3c06 100644 --- a/src/components/panes/PaneContainer.tsx +++ b/src/components/panes/PaneContainer.tsx @@ -709,7 +709,7 @@ function PickerWrapper({ ? getFreshAgentProviderConfig(type) : undefined const providerSettings = freshAgentSettings?.providers?.[type] - const { model, persistModel: shouldPersistPaneModel } = resolveFreshAgentPaneModelDefaults({ + const { model, persistModel: shouldPersistPaneModel, modelEffortLevels } = resolveFreshAgentPaneModelDefaults({ entry: freshAgentType, providerSettings, runtimeProviderModel: settings?.codingCli?.providers?.[freshAgentType.runtimeProvider]?.model, @@ -728,6 +728,7 @@ function PickerWrapper({ model, providerEffort: normalizeFreshAgentEffortOverride(providerSettings?.effort), fallbackEffort: freshAgentType.defaultEffort, + modelEffortLevels, }) return { kind: 'fresh-agent', @@ -737,6 +738,7 @@ function PickerWrapper({ status: 'creating', modelSelection: normalizeFreshAgentModelSelection(providerSettings?.modelSelection), ...(shouldPersistPaneModel ? { model } : {}), + ...(modelEffortLevels ? { modelEffortLevels } : {}), ...(permissionMode ? { permissionMode } : {}), sandbox: freshAgentType.runtimeProvider === 'codex' ? settings?.codingCli?.providers?.[freshAgentType.runtimeProvider]?.sandbox diff --git a/src/lib/fresh-agent-provider-types.ts b/src/lib/fresh-agent-provider-types.ts index 76760bcc4..67ef0f811 100644 --- a/src/lib/fresh-agent-provider-types.ts +++ b/src/lib/fresh-agent-provider-types.ts @@ -1,6 +1,7 @@ import type { CodingCliProviderName } from '@/lib/coding-cli-types' import type { FreshAgentModelSelection } from '@shared/fresh-agent-model-capabilities' import type { FreshAgentSessionType } from '@shared/fresh-agent' +import type { FreshAgentModelEffortLevels } from '@shared/settings' export type FreshAgentProviderName = Extract @@ -8,6 +9,7 @@ export type FreshAgentProviderSettings = { modelSelection?: FreshAgentModelSelection defaultPermissionMode?: string effort?: string + modelEffortLevels?: FreshAgentModelEffortLevels } export interface FreshAgentProviderConfig { diff --git a/src/lib/fresh-agent-registry.ts b/src/lib/fresh-agent-registry.ts index 6f3920ad9..7b120f910 100644 --- a/src/lib/fresh-agent-registry.ts +++ b/src/lib/fresh-agent-registry.ts @@ -170,6 +170,7 @@ export type FreshAgentProviderDefaultsForModel = { /** The saved per-type fresh-pane defaults the model resolver reads. */ export type FreshAgentPaneModelDefaultsSettings = { modelSelection?: { modelId?: string } + modelEffortLevels?: { modelId: string; levels: string[] } } export type FreshAgentPaneModelDefaults = { @@ -178,6 +179,10 @@ export type FreshAgentPaneModelDefaults = { /** Whether to stamp `model` into the pane content. An opencode pane with * nothing configured persists no model, so its provider default applies. */ persistModel: boolean + /** Claude only: the effort levels saved with `model` (captured when it was + * picked), to stamp as the pane's `modelEffortLevels`. Absent when none were + * saved for exactly this model. */ + modelEffortLevels?: string[] } function nonBlank(value: string | undefined): string | undefined { @@ -212,7 +217,16 @@ export function resolveFreshAgentPaneModelDefaults(args: { const persistModel = entry.runtimeProvider !== 'opencode' || saved !== undefined || runtimeProviderModel !== undefined - return { model, persistModel } + // Levels are saved paired with their model: a mismatched pair (levels for + // another model) is stale and ignored. + const savedLevels = args.providerSettings?.modelEffortLevels + const modelEffortLevels = entry.runtimeProvider === 'claude' + && model !== undefined + && savedLevels?.modelId === model + && Array.isArray(savedLevels.levels) + ? [...savedLevels.levels] + : undefined + return { model, persistModel, ...(modelEffortLevels ? { modelEffortLevels } : {}) } } /** @@ -231,6 +245,32 @@ export function resolveEffectiveFreshAgentModel( return normalizeFreshAgentModel(content.sessionType, content.provider, configured) } +/** + * Probed-only claude models clamp effort against the levels stamped when the + * model was picked: the staged value survives when the stamp knows it, an + * unknown staged value re-clamps to the stamp's first level, and an empty + * stamp (the model declared no levels) clears the effort. + */ +export function clampEffortToStampedLevels(levels: readonly string[], staged: string | undefined): string | undefined { + if (staged && levels.includes(staged)) return staged + return levels.length > 0 ? levels[0] : undefined +} + +/** Whether stamped levels govern this model's effort: claude, a stamp, and a + * model absent from the static menu (static rows keep the static table). */ +function usesStampedEffortLevels( + sessionType: FreshAgentSessionType, + provider: FreshAgentRuntimeProvider, + model: string | undefined, + levels: readonly string[] | undefined, +): levels is readonly string[] { + return provider === 'claude' + && Boolean(model) + && Array.isArray(levels) + && !(FRESH_AGENT_MODEL_OPTIONS_BY_SESSION_TYPE[sessionType] ?? []) + .some((option) => option.value === model) +} + /** * The pane's effective thinking level: the staged pane value first, then the * persisted provider default, normalized against the model's known levels. @@ -250,17 +290,8 @@ export function getEffectiveFreshAgentEffort( providerDefaults?: FreshAgentProviderDefaultsForModel, ): string | undefined { const resolvedModel = resolveEffectiveFreshAgentModel(content, providerDefaults) - if ( - content.provider === 'claude' - && resolvedModel - && Array.isArray(content.modelEffortLevels) - && !(FRESH_AGENT_MODEL_OPTIONS_BY_SESSION_TYPE[content.sessionType] ?? []) - .some((option) => option.value === resolvedModel) - ) { - const levels = content.modelEffortLevels - const staged = content.effort ?? providerDefaults?.effort - if (staged && levels.includes(staged)) return staged - return levels.length > 0 ? levels[0] : undefined + if (usesStampedEffortLevels(content.sessionType, content.provider, resolvedModel, content.modelEffortLevels)) { + return clampEffortToStampedLevels(content.modelEffortLevels, content.effort ?? providerDefaults?.effort) } return normalizeFreshAgentEffort( content.sessionType, @@ -283,8 +314,14 @@ export function resolveFreshAgentPaneCreateEffort(args: { model: string | undefined providerEffort: string | undefined fallbackEffort: string + /** Claude: the levels saved with a probed-only model (see + * resolveFreshAgentPaneModelDefaults); effort clamps against them. */ + modelEffortLevels?: readonly string[] }): string | undefined { - const { sessionType, provider, model, providerEffort, fallbackEffort } = args + const { sessionType, provider, model, providerEffort, fallbackEffort, modelEffortLevels } = args + if (usesStampedEffortLevels(sessionType, provider, model, modelEffortLevels)) { + return clampEffortToStampedLevels(modelEffortLevels, providerEffort ?? fallbackEffort) + } if (provider === 'opencode') { return normalizeFreshAgentEffort(sessionType, provider, model, providerEffort) } diff --git a/src/lib/session-type-utils.ts b/src/lib/session-type-utils.ts index b82f90fd3..a2dd529fe 100644 --- a/src/lib/session-type-utils.ts +++ b/src/lib/session-type-utils.ts @@ -223,6 +223,7 @@ export function buildResumeContent(opts: { ...(opts.checkpointWorkspace ? { checkpointWorkspace: opts.checkpointWorkspace } : {}), modelSelection: ps?.modelSelection, ...(modelDefaults.persistModel ? { model: modelDefaults.model } : {}), + ...(modelDefaults.modelEffortLevels ? { modelEffortLevels: modelDefaults.modelEffortLevels } : {}), ...(permissionMode ? { permissionMode } : {}), effort: ps?.effort, } diff --git a/src/store/settingsThunks.ts b/src/store/settingsThunks.ts index b23672851..ca53a8450 100644 --- a/src/store/settingsThunks.ts +++ b/src/store/settingsThunks.ts @@ -64,6 +64,12 @@ function normalizeFreshAgentProviderPatchForApi( ) { normalizedProviderPatch.effort = null } + if ( + Object.prototype.hasOwnProperty.call(providerPatch, 'modelEffortLevels') + && providerPatch.modelEffortLevels === undefined + ) { + normalizedProviderPatch.modelEffortLevels = null + } return normalizedProviderPatch } diff --git a/test/unit/client/components/fresh-agent/FreshAgentModelDialog.test.tsx b/test/unit/client/components/fresh-agent/FreshAgentModelDialog.test.tsx index 0229afb79..9997be791 100644 --- a/test/unit/client/components/fresh-agent/FreshAgentModelDialog.test.tsx +++ b/test/unit/client/components/fresh-agent/FreshAgentModelDialog.test.tsx @@ -756,12 +756,15 @@ describe('FreshAgentModelDialog (freshclaude)', () => { // would otherwise render their raw id until a probe resolves) expect(content.modelLabel).toEqual({ modelId: 'sonnet', label: 'Sonnet' }) + // The levels travel with the saved default, paired with their model, so + // a new pane on this default clamps effort against them too. expect(saveServerSettingsPatchSpy).toHaveBeenCalledWith({ freshAgent: { providers: { freshclaude: { modelSelection: { kind: 'exact', modelId: 'sonnet' }, effort: 'low', + modelEffortLevels: { modelId: 'sonnet', levels: ['low', 'medium', 'high'] }, }, }, }, @@ -815,6 +818,17 @@ describe('FreshAgentModelDialog (freshclaude)', () => { expect(content.model).toBe('haiku') expect(content.effort).toBeUndefined() expect(content.modelEffortLevels).toEqual([]) + expect(saveServerSettingsPatchSpy).toHaveBeenCalledWith({ + freshAgent: { + providers: { + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'haiku' }, + effort: undefined, + modelEffortLevels: { modelId: 'haiku', levels: [] }, + }, + }, + }, + }) }) it('degrades to the static claude rows when the claude catalog probe fails (dialog stays open, no unavailable notice)', async () => { diff --git a/test/unit/client/components/fresh-agent/FreshAgentSettingsButton.test.tsx b/test/unit/client/components/fresh-agent/FreshAgentSettingsButton.test.tsx index 222c72331..d1f68c253 100644 --- a/test/unit/client/components/fresh-agent/FreshAgentSettingsButton.test.tsx +++ b/test/unit/client/components/fresh-agent/FreshAgentSettingsButton.test.tsx @@ -519,6 +519,20 @@ describe('FreshAgentSettingsButton', () => { expect(configure.sessionId).toBe('thread-settings') expect(configure.provider).toBe('claude') expect(configure.settings).toEqual({ model: 'sonnet', effort: 'alpha' }) + + // The stamped levels are saved with the default model, paired with it, so + // a new pane on this default clamps effort against the same levels. + expect(saveServerSettingsPatchSpy).toHaveBeenCalledWith({ + freshAgent: { + providers: { + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'sonnet' }, + effort: 'alpha', + modelEffortLevels: { modelId: 'sonnet', levels: ['alpha', 'beta'] }, + }, + }, + }, + }) }) it('stamps no modelLabel when the switched-to probed row\'s label echoes its raw id', async () => { getFreshAgentModelCapabilitiesSpy.mockResolvedValue({ diff --git a/test/unit/client/components/fresh-agent/FreshAgentView.test.tsx b/test/unit/client/components/fresh-agent/FreshAgentView.test.tsx index 46758a526..935aac6cc 100644 --- a/test/unit/client/components/fresh-agent/FreshAgentView.test.tsx +++ b/test/unit/client/components/fresh-agent/FreshAgentView.test.tsx @@ -5080,6 +5080,56 @@ describe('FreshAgentView', () => { }) }) + it('creates a Freshclaude pane on a saved catalog-only default with no effort when its saved levels are empty', async () => { + const store = createStore() + store.dispatch(previewServerSettingsPatch({ + freshAgent: { + providers: { + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'claude-haiku-x' }, + modelEffortLevels: { modelId: 'claude-haiku-x', levels: [] }, + effort: 'high', + }, + }, + }, + })) + // The content PaneContainer seeds from that saved default. + store.dispatch(initLayout({ + tabId: 'tab-1', + paneId: 'pane-1', + content: { + kind: 'fresh-agent', + sessionType: 'freshclaude', + provider: 'claude', + createRequestId: 'req-catalog-only-claude-default', + status: 'creating', + modelSelection: { kind: 'exact', modelId: 'claude-haiku-x' }, + model: 'claude-haiku-x', + modelEffortLevels: [], + }, + })) + + render( + + + , + ) + + await waitFor(() => { + expect(sentFreshAgentMessages('freshAgent.create')).toContainEqual(expect.objectContaining({ + type: 'freshAgent.create', + sessionType: 'freshclaude', + provider: 'claude', + model: 'claude-haiku-x', + })) + }) + const create = sentFreshAgentMessages('freshAgent.create') + .find((frame) => frame.requestId === 'req-catalog-only-claude-default') + expect(create).toBeDefined() + // Never the opus table's 'high': the saved model declares no levels. + expect(create?.effort).toBeUndefined() + }) + it('sends Freshopencode messages with modelSelection when pane model is absent', async () => { const store = createStore() store.dispatch(initLayout({ diff --git a/test/unit/client/components/panes/PaneContainer.createContent.test.tsx b/test/unit/client/components/panes/PaneContainer.createContent.test.tsx index 4944a84d0..5d2558891 100644 --- a/test/unit/client/components/panes/PaneContainer.createContent.test.tsx +++ b/test/unit/client/components/panes/PaneContainer.createContent.test.tsx @@ -632,6 +632,73 @@ describe('createContentForType with ext: prefix', () => { expect(content.model).toBe('opus[1m]') }) + it('carries the effort levels saved with a catalog-only default model, so effort clamps against them', async () => { + const content = await createClaudeFamilyPane({ + label: 'Freshclaude', + settingsOverrides: { + codingCli: { enabledProviders: ['claude'] }, + freshAgent: { + enabled: true, + providers: { + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'claude-haiku-x' }, + modelEffortLevels: { modelId: 'claude-haiku-x', levels: [] }, + effort: 'high', + }, + }, + }, + }, + }) + + expect(content.model).toBe('claude-haiku-x') + expect(content.modelEffortLevels).toEqual([]) + // The model declares no effort levels: never the opus table's 'high'. + expect(content.effort).toBeUndefined() + }) + + it('clamps a saved effort the saved catalog-only model does not offer to its first level', async () => { + const content = await createClaudeFamilyPane({ + label: 'Freshclaude', + settingsOverrides: { + codingCli: { enabledProviders: ['claude'] }, + freshAgent: { + enabled: true, + providers: { + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'sonnet' }, + modelEffortLevels: { modelId: 'sonnet', levels: ['alpha', 'beta'] }, + effort: 'max', + }, + }, + }, + }, + }) + + expect(content.modelEffortLevels).toEqual(['alpha', 'beta']) + expect(content.effort).toBe('alpha') + }) + + it('ignores saved effort levels paired with a different model', async () => { + const content = await createClaudeFamilyPane({ + label: 'Freshclaude', + settingsOverrides: { + codingCli: { enabledProviders: ['claude'] }, + freshAgent: { + enabled: true, + providers: { + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'sonnet' }, + modelEffortLevels: { modelId: 'claude-haiku-x', levels: [] }, + }, + }, + }, + }, + }) + + expect(content.model).toBe('sonnet') + expect(content.modelEffortLevels).toBeUndefined() + }) + it('does not fall back to the terminal Claude CLI model (decision D1)', async () => { const content = await createClaudeFamilyPane({ label: 'Freshclaude', diff --git a/test/unit/client/lib/session-type-utils.test.ts b/test/unit/client/lib/session-type-utils.test.ts index a498ef2be..8de980db6 100644 --- a/test/unit/client/lib/session-type-utils.test.ts +++ b/test/unit/client/lib/session-type-utils.test.ts @@ -271,6 +271,20 @@ describe('buildResumeContent', () => { expect(unsaved.model).toBe('opus[1m]') }) + it('carries the effort levels saved with the default model into resumed claude panes', () => { + const content = buildResumeContent({ + sessionType: 'freshclaude', + sessionId: 'abc-123', + freshAgentProviderSettings: { + modelSelection: { kind: 'exact', modelId: 'claude-haiku-x' }, + modelEffortLevels: { modelId: 'claude-haiku-x', levels: [] }, + }, + }) + if (content.kind !== 'fresh-agent') throw new Error('expected fresh-agent') + expect(content.model).toBe('claude-haiku-x') + expect(content.modelEffortLevels).toEqual([]) + }) + it('resumes a Freshcodex session on the saved Freshcodex default model', () => { const content = buildResumeContent({ sessionType: 'freshcodex', diff --git a/test/unit/client/store/settingsThunks.test.ts b/test/unit/client/store/settingsThunks.test.ts index 53f842094..4adf23995 100644 --- a/test/unit/client/store/settingsThunks.test.ts +++ b/test/unit/client/store/settingsThunks.test.ts @@ -449,4 +449,47 @@ describe('settingsThunks', () => { expect(store.getState().settings.settings.freshAgent.providers.freshclaude?.effort).toBeUndefined() expect('agentChat' in store.getState().settings.settings).toBe(false) }) + + it('clears saved fresh-pane effort levels with the API clear sentinel', async () => { + const store = makeStore() + const initialServerSettings = store.getState().settings.serverSettings + store.dispatch(setServerSettings({ + ...initialServerSettings, + freshAgent: { + ...initialServerSettings.freshAgent, + providers: { + ...initialServerSettings.freshAgent.providers, + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'sonnet' }, + modelEffortLevels: { modelId: 'sonnet', levels: ['low', 'medium', 'high'] }, + }, + }, + }, + })) + + apiPatch.mockResolvedValue({}) + + await store.dispatch(saveServerSettingsPatch({ + freshAgent: { + providers: { + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'claude-ghost' }, + modelEffortLevels: undefined, + }, + }, + }, + })) + + expect(apiPatch).toHaveBeenCalledWith('/api/settings', { + freshAgent: { + providers: { + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'claude-ghost' }, + modelEffortLevels: null, + }, + }, + }, + }) + expect(store.getState().settings.settings.freshAgent.providers.freshclaude?.modelEffortLevels).toBeUndefined() + }) }) diff --git a/test/unit/shared/fresh-agent-registry.test.ts b/test/unit/shared/fresh-agent-registry.test.ts index 0b18f67be..59586659e 100644 --- a/test/unit/shared/fresh-agent-registry.test.ts +++ b/test/unit/shared/fresh-agent-registry.test.ts @@ -180,3 +180,55 @@ describe('resolveFreshAgentPaneModelDefaults', () => { expect(result.persistModel).toBe(persistModel) }) }) + +describe('resolveFreshAgentPaneModelDefaults effort levels', () => { + it('returns the saved levels when they are paired with the resolved claude model', () => { + expect(resolveFreshAgentPaneModelDefaults({ + entry: registryEntry('kilroy'), + providerSettings: { + modelSelection: { modelId: 'sonnet' }, + modelEffortLevels: { modelId: 'sonnet', levels: ['low', 'high'] }, + }, + }).modelEffortLevels).toEqual(['low', 'high']) + }) + + it('ignores levels paired with another model', () => { + expect(resolveFreshAgentPaneModelDefaults({ + entry: registryEntry('freshclaude'), + providerSettings: { + modelSelection: { modelId: 'sonnet' }, + modelEffortLevels: { modelId: 'haiku', levels: [] }, + }, + }).modelEffortLevels).toBeUndefined() + }) + + it('ignores levels for non-claude providers', () => { + expect(resolveFreshAgentPaneModelDefaults({ + entry: registryEntry('freshcodex'), + providerSettings: { + modelSelection: { modelId: 'gpt-5.6-luna' }, + modelEffortLevels: { modelId: 'gpt-5.6-luna', levels: ['low'] }, + }, + }).modelEffortLevels).toBeUndefined() + }) +}) + +describe('resolveFreshAgentPaneCreateEffort with stamped levels', () => { + it('clamps a catalog-only claude model against its stamped levels', () => { + const base = { sessionType: 'freshclaude' as const, provider: 'claude' as const, model: 'sonnet', fallbackEffort: 'high' } + expect(resolveFreshAgentPaneCreateEffort({ ...base, providerEffort: 'beta', modelEffortLevels: ['alpha', 'beta'] })).toBe('beta') + expect(resolveFreshAgentPaneCreateEffort({ ...base, providerEffort: 'max', modelEffortLevels: ['alpha', 'beta'] })).toBe('alpha') + expect(resolveFreshAgentPaneCreateEffort({ ...base, providerEffort: undefined, modelEffortLevels: [] })).toBeUndefined() + }) + + it('keeps the static table for the static claude model', () => { + expect(resolveFreshAgentPaneCreateEffort({ + sessionType: 'freshclaude', + provider: 'claude', + model: 'opus[1m]', + providerEffort: 'max', + fallbackEffort: 'high', + modelEffortLevels: ['low'], + })).toBe('max') + }) +}) diff --git a/test/unit/shared/settings.test.ts b/test/unit/shared/settings.test.ts index 31a26dc89..1302c346f 100644 --- a/test/unit/shared/settings.test.ts +++ b/test/unit/shared/settings.test.ts @@ -357,6 +357,50 @@ describe('shared settings contract', () => { }) }) + it('round-trips and clears the effort levels saved with a fresh-pane default model', () => { + const levels = { modelId: 'claude-haiku-x', levels: [] as string[] } + const saved = mergeServerSettings(createDefaultServerSettings({ loggingDebug: false }), { + freshAgent: { + providers: { + freshclaude: { + modelSelection: { kind: 'exact', modelId: 'claude-haiku-x' }, + modelEffortLevels: levels, + }, + }, + }, + }) + expect(saved.freshAgent.providers.freshclaude?.modelEffortLevels).toEqual(levels) + + // A later patch that does not mention the levels keeps them. + const unrelated = mergeServerSettings(saved, { + freshAgent: { providers: { freshclaude: { effort: 'low' } } }, + }) + expect(unrelated.freshAgent.providers.freshclaude?.modelEffortLevels).toEqual(levels) + + // null (the API clear sentinel) removes them. + const cleared = mergeServerSettings(unrelated, { + freshAgent: { providers: { freshclaude: { modelEffortLevels: null } } }, + } as unknown as Parameters[1]) + expect(cleared.freshAgent.providers.freshclaude?.modelEffortLevels).toBeUndefined() + expect(cleared.freshAgent.providers.freshclaude?.modelSelection).toEqual({ kind: 'exact', modelId: 'claude-haiku-x' }) + }) + + it('rejects malformed saved effort levels in the server patch schema', () => { + const schema = buildServerSettingsPatchSchema() + expect(schema.safeParse({ + freshAgent: { providers: { freshclaude: { modelEffortLevels: { modelId: 'sonnet', levels: ['low', 'high'] } } } }, + }).success).toBe(true) + expect(schema.safeParse({ + freshAgent: { providers: { freshclaude: { modelEffortLevels: null } } }, + }).success).toBe(true) + expect(schema.safeParse({ + freshAgent: { providers: { freshclaude: { modelEffortLevels: { modelId: 'sonnet', levels: 'high' } } } }, + }).success).toBe(false) + expect(schema.safeParse({ + freshAgent: { providers: { freshclaude: { modelEffortLevels: { levels: ['high'] } } } }, + }).success).toBe(false) + }) + it('mergeServerSettings preserves runtime CLI providers outside the built-in defaults', () => { const merged = mergeServerSettings(createDefaultServerSettings({ loggingDebug: false }), { codingCli: { From 7ddab9a9977c9dcebeab200903c1eceb341173b5 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:26:04 -0700 Subject: [PATCH 07/20] test(e2e): new and resumed claude panes start on the saved default model Browser e2e for G11 against the real Rust server and the fake Claude-SDK sidecar: the saved Freshclaude/Kilroy default is written through PATCH /api/settings, a picker-created pane's sidecar create carries it, and a pane resumed from the sidebar after the default changed carries the new default (decision D2), asserted on the exact stdin create frames. Co-Authored-By: Claude Opus 5.5 --- .../freshagent-default-model-rust.spec.ts | 338 ++++++++++++++++++ 1 file changed, 338 insertions(+) create mode 100644 test/e2e-browser/specs/freshagent-default-model-rust.spec.ts diff --git a/test/e2e-browser/specs/freshagent-default-model-rust.spec.ts b/test/e2e-browser/specs/freshagent-default-model-rust.spec.ts new file mode 100644 index 000000000..7f29709d3 --- /dev/null +++ b/test/e2e-browser/specs/freshagent-default-model-rust.spec.ts @@ -0,0 +1,338 @@ +/** + * FRESH-PANE DEFAULT MODEL (G11) — PW-RUST e2e. + * + * The model saved for Freshclaude/Kilroy (the model dialog and the settings + * popover save it as `freshAgent.providers[type].modelSelection`) must reach + * the sidecar for: + * 1. a NEW pane created through the picker, and + * 2. a pane created to RESUME a session from the sidebar — which starts on + * the CURRENT saved default, not the model the conversation last used + * (decision D2, matching `claude --resume`). + * + * The spec drives the real Rust server + real browser UI against the + * deterministic fake Claude-SDK sidecar and asserts the EXACT `create` frame + * the server wrote to the sidecar's stdin (FRESHELL_FAKE_STDIN raw audit). + * The saved default is written through the real `PATCH /api/settings` route + * and observed in the client's settings before any pane is created. + * + * Pre-fix, both create frames carried the registry default `opus[1m]`. + * + * CLOUD-LEGALITY: no idle-grace or timing dependencies — every wait is a poll + * on observed state (stdin audit rows, pane layout, sidebar rows). + * + * Helpers are copied, not imported, per this suite's per-spec-ownership + * convention (donor: fresh-agent-control-rust.spec.ts — boot, stdin audit, + * pane bootstrap; auto-title-rust.spec.ts — sidebar row click). + */ +import fs from 'node:fs/promises' +import { existsSync, readFileSync } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { randomUUID } from 'node:crypto' +import { fileURLToPath } from 'node:url' +import type { Page } from '@playwright/test' +import { test, expect } from '../helpers/fixtures.js' +import { RustServer, GEMINI_STRIP_ENV_PREFIXES } from '../helpers/rust-server.js' +import type { E2eServerInfo } from '../helpers/server-fixture-support.js' +import { TestHarness } from '../helpers/test-harness.js' +import { openPanePicker } from '../helpers/pane-picker.js' + +const __filename = fileURLToPath(import.meta.url) +const __dirname = path.dirname(__filename) + +const CLAUDE_SIDECAR_FIXTURE = path.resolve(__dirname, '../fixtures/providers/fake-claude-sdk-sidecar.mjs') + +const CANONICAL_UUID_RE = + /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + +type Flavour = 'freshclaude' | 'kilroy' + +// ── Copied helpers ────────────────────────────────────────────────────────── + +async function selectShellIfPickerShowing(page: Page): Promise { + const picker = page.getByRole('toolbar', { name: /pane type picker/i }).last() + if (!(await picker.isVisible().catch(() => false))) return + for (const name of ['Shell', 'WSL', 'CMD', 'PowerShell', 'Bash']) { + const option = picker.getByRole('button', { name: new RegExp(`^${name}$`, 'i') }) + if (await option.isVisible().catch(() => false)) { + await option.click({ force: true }) + return + } + } +} + +function readJsonl(filePath: string): any[] { + if (!existsSync(filePath)) return [] + return readFileSync(filePath, 'utf8') + .split('\n') + .filter(Boolean) + .map((line) => { + try { + return JSON.parse(line) + } catch { + return null + } + }) + .filter((row) => row !== null) +} + +/** Read the sidecar raw-stdin audit, JSON-parsing each recorded frame. */ +function readStdinFrames(stdinLog: string): any[] { + return readJsonl(stdinLog) + .map((row) => { + try { + return JSON.parse(row.line) + } catch { + return null + } + }) + .filter((frame) => frame !== null) +} + +async function waitForStdinFrame( + stdinLog: string, + pred: (frame: any) => boolean, + what: string, + timeoutMs = 20_000, +): Promise { + let found: any = null + await expect + .poll( + () => { + found = readStdinFrames(stdinLog).find(pred) ?? null + return Boolean(found) + }, + { timeout: timeoutMs, message: `timed out waiting for ${what} in ${stdinLog}` }, + ) + .toBe(true) + return found +} + +function findFreshAgentLeaf(node: any): any { + if (!node) return null + if (node.type === 'leaf' && node.content?.kind === 'fresh-agent') return node + if (node.type === 'split') { + for (const child of node.children ?? []) { + const found = findFreshAgentLeaf(child) + if (found) return found + } + } + return null +} + +async function paneLeaf(harness: TestHarness, tabId: string): Promise { + return findFreshAgentLeaf(await harness.getPaneLayout(tabId)) +} + +async function waitForPaneStatus(harness: TestHarness, tabId: string, wanted: string): Promise { + await expect + .poll(async () => (await paneLeaf(harness, tabId))?.content?.status ?? null, { timeout: 30_000 }) + .toBe(wanted) +} + +/** Save the fresh-pane default model through the real settings route, then + * wait until the client has folded it (settings.updated broadcast). */ +async function saveDefaultModel( + info: E2eServerInfo, + harness: TestHarness, + flavour: Flavour, + modelId: string, +): Promise { + const res = await fetch(`${info.baseUrl}/api/settings`, { + method: 'PATCH', + headers: { 'content-type': 'application/json', 'x-auth-token': info.token }, + body: JSON.stringify({ + freshAgent: { providers: { [flavour]: { modelSelection: { kind: 'exact', modelId } } } }, + }), + }) + expect(res.ok, `PATCH /api/settings failed: ${res.status}`).toBe(true) + await expect + .poll( + async () => (await harness.getSettings())?.freshAgent?.providers?.[flavour]?.modelSelection?.modelId ?? null, + { timeout: 15_000, message: `the client folds the saved ${flavour} default` }, + ) + .toBe(modelId) +} + +async function bootLane(page: Page, flavour: Flavour): Promise<{ + server: RustServer + info: E2eServerInfo + harness: TestHarness + sharedRoot: string + projectDir: string + stdinLog: string +}> { + const sharedRoot = await fs.mkdtemp(path.join(os.tmpdir(), `freshell-default-model-${flavour}-`)) + const projectDir = path.join(sharedRoot, 'project') + await fs.mkdir(projectDir, { recursive: true }) + const stdinLog = path.join(sharedRoot, 'sidecar-stdin.jsonl') + const server = new RustServer({ + env: { + FRESHELL_CLAUDE_SIDECAR: CLAUDE_SIDECAR_FIXTURE, + FRESHELL_CLAUDE_NODE: process.execPath, + FRESHELL_FAKE_PROVIDER: flavour, + FRESHELL_FAKE_STDIN: stdinLog, + FRESHELL_FAKE_EVENTS: path.join(sharedRoot, 'sidecar-events.jsonl'), + ...(flavour === 'kilroy' ? { KILROY_ENABLED: '1' } : {}), + }, + stripEnvPrefixes: flavour === 'kilroy' ? [...GEMINI_STRIP_ENV_PREFIXES] : undefined, + setupHome: async (homeDir) => { + const freshellDir = path.join(homeDir, '.freshell') + await fs.mkdir(freshellDir, { recursive: true }) + await fs.writeFile(path.join(freshellDir, 'config.json'), JSON.stringify({ + version: 1, + settings: { + codingCli: { enabledProviders: ['claude'] }, + freshAgent: { enabled: true }, + // The fixture conversation never gets a title, and the default + // hideEmptySessions drops an untitled row once its pane closes; + // keep it listed so the sidebar can resume it. + sidebar: { hideEmptySessions: false }, + }, + }, null, 2)) + }, + }) + try { + const info = await server.start() + await page.goto(`${info.baseUrl}/?token=${info.token}&e2e=1`) + const harness = new TestHarness(page) + await harness.waitForHarness() + await harness.waitForConnection() + await selectShellIfPickerShowing(page) + await expect(page.locator('.xterm').first()).toBeVisible({ timeout: 30_000 }) + await page.evaluate(() => { + ;(window as any).__FRESHELL_TEST_HARNESS__?.dispatch({ + type: 'connection/setAvailableClis', + payload: { claude: true }, + }) + }) + return { server, info, harness, sharedRoot, projectDir, stdinLog } + } catch (error) { + await server.stop().catch(() => {}) + await fs.rm(sharedRoot, { recursive: true, force: true }).catch(() => {}) + throw error + } +} + +/** Append one user/assistant exchange to the session's CLI transcript + * (`~/.claude/projects//.jsonl`, parentUuid-chained like + * the CLI and the fake sidecar write it). */ +async function appendTranscriptExchange( + homeDir: string, + cwd: string, + sessionId: string, + userText: string, +): Promise { + const file = path.join(homeDir, '.claude', 'projects', cwd.replace(/[^A-Za-z0-9]/g, '-'), `${sessionId}.jsonl`) + await fs.mkdir(path.dirname(file), { recursive: true }) + const existing = existsSync(file) ? readJsonl(file) : [] + let parentUuid: string | null = existing.length > 0 ? existing[existing.length - 1]?.uuid ?? null : null + const lines = (['user', 'assistant'] as const).map((role) => { + const uuid = randomUUID() + const line = { + type: role, + uuid, + parentUuid, + timestamp: new Date().toISOString(), + cwd, + message: { role, content: [{ type: 'text', text: role === 'user' ? userText : 'Fixture reply' }] }, + } + parentUuid = uuid + return JSON.stringify(line) + }) + await fs.appendFile(file, `${lines.join('\n')}\n`) +} + +const LABELS: Record = { freshclaude: 'Freshclaude', kilroy: 'Kilroy' } + +async function runDefaultModelJourney(page: Page, flavour: Flavour): Promise { + const lane = await bootLane(page, flavour) + try { + const { harness, info, stdinLog } = lane + const label = LABELS[flavour] + const tabId = (await harness.getActiveTabId())! + expect(tabId).toBeTruthy() + + // ── 1. A NEW pane starts on the saved default ────────────────────────── + await saveDefaultModel(info, harness, flavour, 'sonnet') + + const picker = await openPanePicker(page) + await picker.getByRole('button', { name: new RegExp(`^${label}$`) }).click({ force: true }) + const directoryInput = page.getByLabel(new RegExp(`^Starting directory for ${label}$`, 'i')) + await expect(directoryInput).toBeVisible({ timeout: 15_000 }) + await directoryInput.fill(lane.projectDir) + await directoryInput.press('Enter') + const paneRoot = page.locator('[data-context="fresh-agent"]').last() + await expect(paneRoot).toBeVisible({ timeout: 15_000 }) + + const freshCreate = await waitForStdinFrame( + stdinLog, + (frame) => frame?.type === 'create' && !frame?.resumeSessionId, + `the new ${label} pane's sidecar create`, + ) + expect(freshCreate.model, 'a new pane starts on the saved default model').toBe('sonnet') + + let durableId = '' + await expect + .poll(async () => { + durableId = (await paneLeaf(harness, tabId))?.content?.sessionRef?.sessionId ?? '' + return durableId + }, { timeout: 30_000 }) + .toMatch(CANONICAL_UUID_RE) + await waitForPaneStatus(harness, tabId, 'idle') + + // A past exchange in the conversation's on-disk transcript (the shape the + // CLI writes), so the session directory keeps listing the conversation + // after its pane closes. This step deliberately does not depend on the + // fake sidecar's turn lanes: the resume path is what is under test. + await appendTranscriptExchange(info.homeDir, lane.projectDir, durableId, 'hello default model') + + const sidebarRow = page.locator(`[data-context="sidebar-session"][data-session-id="${durableId}"]`) + await expect(sidebarRow, 'the session has a sidebar row').toBeVisible({ timeout: 30_000 }) + + // ── 2. A RESUMED pane starts on the CURRENT saved default (D2) ───────── + // Change the saved default AFTER the conversation ran on sonnet: the + // resume must follow the saved default, not the conversation's model. + await saveDefaultModel(info, harness, flavour, 'haiku') + + // Close the pane (kills its live session) so the sidebar click resumes + // the conversation into a NEW pane instead of focusing the open one. + const paneId = (await paneLeaf(harness, tabId))?.id as string + expect(paneId).toBeTruthy() + await page + .locator(`[data-context="pane-header"][data-pane-id="${paneId}"]`) + .getByRole('button', { name: 'Close pane' }) + .click() + await expect + .poll(async () => (await paneLeaf(harness, tabId))?.id ?? null, { timeout: 15_000 }) + .toBeNull() + + await sidebarRow.click() + const resumeCreate = await waitForStdinFrame( + stdinLog, + (frame) => frame?.type === 'create' && frame?.resumeSessionId === durableId, + `the resumed ${label} pane's sidecar create`, + 30_000, + ) + expect(resumeCreate.model, 'a resumed pane starts on the current saved default model').toBe('haiku') + + const resumedLeaf = findFreshAgentLeaf(await harness.getPaneLayout((await harness.getActiveTabId())!)) + expect(resumedLeaf?.content?.sessionType).toBe(flavour) + expect(resumedLeaf?.content?.model).toBe('haiku') + } finally { + await lane.server.stop().catch(() => {}) + await fs.rm(lane.sharedRoot, { recursive: true, force: true }).catch(() => {}) + } +} + +test.describe('fresh-pane default model (G11)', () => { + test.setTimeout(180_000) + + test('freshclaude: new and resumed panes start on the saved default model', async ({ page }) => { + await runDefaultModelJourney(page, 'freshclaude') + }) + + test('kilroy: new and resumed panes start on the saved default model', async ({ page }) => { + await runDefaultModelJourney(page, 'kilroy') + }) +}) From e62dcc1d7f92792e5eef615c4c9d0ab38bee3a45 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:27:56 -0700 Subject: [PATCH 08/20] feat(tabs): shared tab-attention selectors, class helper and indicator Add the building blocks every tab surface will share for the needs-attention mark: memoized selectors for the style-gated union of attentionByTab and watchedCompletionByTab in tab order (tabAttentionSelectors), one tabAttentionClassName owning the emerald, darken and pulse tokens per surface, the 'N other tabs need attention' wording, and a TabAttentionIndicator that renders a polite status (or an aria-hidden dot inside a labelled control). Co-Authored-By: Claude Opus 5.5 --- src/components/TabAttentionIndicator.tsx | 51 +++++++++++++ src/lib/tab-attention.ts | 72 ++++++++++++++++++ src/store/selectors/tabAttentionSelectors.ts | 42 +++++++++++ .../components/TabAttentionIndicator.test.tsx | 34 +++++++++ .../selectors/tabAttentionSelectors.test.ts | 75 +++++++++++++++++++ 5 files changed, 274 insertions(+) create mode 100644 src/components/TabAttentionIndicator.tsx create mode 100644 src/lib/tab-attention.ts create mode 100644 src/store/selectors/tabAttentionSelectors.ts create mode 100644 test/unit/client/components/TabAttentionIndicator.test.tsx create mode 100644 test/unit/client/store/selectors/tabAttentionSelectors.test.ts diff --git a/src/components/TabAttentionIndicator.tsx b/src/components/TabAttentionIndicator.tsx new file mode 100644 index 000000000..561bec605 --- /dev/null +++ b/src/components/TabAttentionIndicator.tsx @@ -0,0 +1,51 @@ +import { cn } from '@/lib/utils' +import { tabAttentionClassName } from '@/lib/tab-attention' +import type { TabAttentionStyle } from '@/store/types' + +interface TabAttentionIndicatorProps { + /** How many tabs carry the mark; nothing renders at 0. Omit for a bare mark. */ + count?: number + /** What the mark means, e.g. "2 other tabs need attention". */ + label: string + /** `dot`: a decorative mark inside a control whose own name/title carries + * the meaning (aria-hidden). `count` (default): a polite live status. */ + variant?: 'count' | 'dot' + style?: TabAttentionStyle + className?: string +} + +/** + * The shared needs-attention mark for tab surfaces (mobile tab strip, its + * chevrons). Visual tokens come from `tabAttentionClassName`, so it follows + * the user's tabAttentionStyle like the desktop tabs. + */ +export function TabAttentionIndicator({ + count, + label, + variant = 'count', + style = 'highlight', + className, +}: TabAttentionIndicatorProps) { + if (count !== undefined && count <= 0) return null + const dotClassName = cn( + 'inline-block h-2 w-2 shrink-0 rounded-full', + tabAttentionClassName({ needsAttention: true, style, surface: 'indicator' }), + ) + + if (variant === 'dot') { + return