From 7c283c7880a598e3d855ae4c008622a44b344164 Mon Sep 17 00:00:00 2001 From: James Grugett Date: Fri, 24 Apr 2026 23:07:26 -0700 Subject: [PATCH 1/2] Fix freebuff model picker enter --- .../components/freebuff-model-selector.tsx | 43 +++++---- cli/src/components/waiting-room-screen.tsx | 2 +- cli/src/hooks/use-freebuff-session.ts | 11 ++- .../freebuff-model-navigation.test.ts | 93 +++++++++++++++++++ cli/src/utils/freebuff-model-navigation.ts | 36 +++++++ 5 files changed, 165 insertions(+), 20 deletions(-) create mode 100644 cli/src/utils/__tests__/freebuff-model-navigation.test.ts create mode 100644 cli/src/utils/freebuff-model-navigation.ts diff --git a/cli/src/components/freebuff-model-selector.tsx b/cli/src/components/freebuff-model-selector.tsx index 0850a0bd7..5db6551a5 100644 --- a/cli/src/components/freebuff-model-selector.tsx +++ b/cli/src/components/freebuff-model-selector.tsx @@ -17,6 +17,10 @@ import { useFreebuffModelStore } from '../state/freebuff-model-store' import { useFreebuffSessionStore } from '../state/freebuff-session-store' import { useTerminalDimensions } from '../hooks/use-terminal-dimensions' import { useTheme } from '../hooks/use-theme' +import { + nextSelectableFreebuffModelId, + resolveFreebuffModelCommitTarget, +} from '../utils/freebuff-model-navigation' import type { KeyEvent } from '@opentui/core' @@ -169,30 +173,32 @@ export const FreebuffModelSelector: React.FC = () => { const isCommit = name === 'return' || name === 'enter' || name === 'space' if (!isForward && !isBackward && !isCommit) return if (isCommit) { - if ( - focusedId !== committedModelId && - isFreebuffModelAvailable(focusedId, new Date(now)) - ) { + const targetId = resolveFreebuffModelCommitTarget({ + focusedId, + selectedId: selectedModel, + committedId: committedModelId, + isSelectable: (modelId) => + isFreebuffModelAvailable(modelId, new Date(now)), + }) + if (targetId) { key.preventDefault?.() - pick(focusedId) + pick(targetId) } return } - const currentIdx = FREEBUFF_MODEL_SELECTOR_MODELS.findIndex( - (m) => m.id === focusedId, - ) - if (currentIdx === -1) return - const len = FREEBUFF_MODEL_SELECTOR_MODELS.length - const nextIdx = isForward - ? (currentIdx + 1) % len - : (currentIdx - 1 + len) % len - const target = FREEBUFF_MODEL_SELECTOR_MODELS[nextIdx] - if (target) { + const targetId = nextSelectableFreebuffModelId({ + modelIds: FREEBUFF_MODEL_SELECTOR_MODELS.map((model) => model.id), + focusedId, + direction: isForward ? 'forward' : 'backward', + isSelectable: (modelId) => + isFreebuffModelAvailable(modelId, new Date(now)), + }) + if (targetId) { key.preventDefault?.() - setFocusedId(target.id) + setFocusedId(targetId) } }, - [pending, pick, focusedId, committedModelId, now], + [pending, pick, focusedId, selectedModel, committedModelId, now], ), ) @@ -215,7 +221,8 @@ export const FreebuffModelSelector: React.FC = () => { // 'Selected' means the dot is filled and the label is bold. On the // landing screen ('none') this tracks the pre-focused pick; on the // queued screen it tracks the model the server has us on. Either - // way, selectedModel reflects the intent of "what Enter commits to." + // way, selectedModel is the safe fallback if focus ever lands on a + // closed row (for example when deployment hours change). const isSelected = model.id === selectedModel const isHovered = hoveredId === model.id const isFocused = focusedId === model.id && !isSelected diff --git a/cli/src/components/waiting-room-screen.tsx b/cli/src/components/waiting-room-screen.tsx index f2a09022e..2bbee6c71 100644 --- a/cli/src/components/waiting-room-screen.tsx +++ b/cli/src/components/waiting-room-screen.tsx @@ -173,7 +173,7 @@ export const WaitingRoomScreen: React.FC = ({ maxWidth: contentMaxWidth, }} > - {error && !session && ( + {error && (!session || session.status === 'none') && ( ⚠ {error} diff --git a/cli/src/hooks/use-freebuff-session.ts b/cli/src/hooks/use-freebuff-session.ts index b7a91eb1e..19f21ecaa 100644 --- a/cli/src/hooks/use-freebuff-session.ts +++ b/cli/src/hooks/use-freebuff-session.ts @@ -376,6 +376,7 @@ export function useFreebuffSession(): UseFreebuffSessionResult { let abortController = new AbortController() let timer: ReturnType | null = null let previousStatus: FreebuffSessionResponse['status'] | null = null + let restartGeneration = 0 // Method for the NEXT tick. GET is read-only; POST claims/rotates a seat. // Startup is GET (probe before committing). After any POST completes we // flip back to GET. refresh() sets it to 'POST' for explicit join/rejoin; @@ -489,6 +490,7 @@ export function useFreebuffSession(): UseFreebuffSessionResult { controller = { restart: async (mode) => { + const generation = ++restartGeneration clearTimer() // Abort any in-flight fetch so it can't race us and overwrite state. abortController.abort() @@ -498,6 +500,7 @@ export function useFreebuffSession(): UseFreebuffSessionResult { // doesn't bounce a 'landing' restart straight back to 'ended'. previousStatus = null if (mode === 'landing') { + nextMethod = 'GET' // Land on the picker immediately. We can't go through the normal // tick/apply path because a server-side row that hasn't been // swept yet would trip the startup-takeover branch into an @@ -511,7 +514,13 @@ export function useFreebuffSession(): UseFreebuffSessionResult { const fetchController = abortController callSession('GET', token, { signal: fetchController.signal }) .then((response) => { - if (cancelled || fetchController.signal.aborted) return + if ( + cancelled || + fetchController.signal.aborted || + generation !== restartGeneration + ) { + return + } const depths = response.status === 'none' || response.status === 'queued' ? response.queueDepthByModel diff --git a/cli/src/utils/__tests__/freebuff-model-navigation.test.ts b/cli/src/utils/__tests__/freebuff-model-navigation.test.ts new file mode 100644 index 000000000..4723245ba --- /dev/null +++ b/cli/src/utils/__tests__/freebuff-model-navigation.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, test } from 'bun:test' + +import { + nextSelectableFreebuffModelId, + resolveFreebuffModelCommitTarget, +} from '../freebuff-model-navigation' + +describe('nextSelectableFreebuffModelId', () => { + test('skips unavailable models when moving forward', () => { + const modelIds = ['glm', 'minimax'] + + expect( + nextSelectableFreebuffModelId({ + modelIds, + focusedId: 'minimax', + direction: 'forward', + isSelectable: (id) => id !== 'glm', + }), + ).toBe('minimax') + }) + + test('skips unavailable models when moving backward', () => { + const modelIds = ['glm', 'minimax'] + + expect( + nextSelectableFreebuffModelId({ + modelIds, + focusedId: 'minimax', + direction: 'backward', + isSelectable: (id) => id !== 'glm', + }), + ).toBe('minimax') + }) + + test('moves to the next available model when more than one is selectable', () => { + const modelIds = ['glm', 'minimax', 'other'] + + expect( + nextSelectableFreebuffModelId({ + modelIds, + focusedId: 'minimax', + direction: 'forward', + isSelectable: (id) => id !== 'glm', + }), + ).toBe('other') + }) + + test('returns null when no selectable model exists', () => { + expect( + nextSelectableFreebuffModelId({ + modelIds: ['glm'], + focusedId: 'glm', + direction: 'forward', + isSelectable: () => false, + }), + ).toBeNull() + }) +}) + +describe('resolveFreebuffModelCommitTarget', () => { + test('falls back to the selected model when focus is on a closed model', () => { + expect( + resolveFreebuffModelCommitTarget({ + focusedId: 'glm', + selectedId: 'minimax', + committedId: null, + isSelectable: (id) => id !== 'glm', + }), + ).toBe('minimax') + }) + + test('commits the focused model when it is selectable', () => { + expect( + resolveFreebuffModelCommitTarget({ + focusedId: 'minimax', + selectedId: 'glm', + committedId: null, + isSelectable: (id) => id === 'minimax', + }), + ).toBe('minimax') + }) + + test('returns null when the target is already committed', () => { + expect( + resolveFreebuffModelCommitTarget({ + focusedId: 'minimax', + selectedId: 'minimax', + committedId: 'minimax', + isSelectable: () => true, + }), + ).toBeNull() + }) +}) diff --git a/cli/src/utils/freebuff-model-navigation.ts b/cli/src/utils/freebuff-model-navigation.ts new file mode 100644 index 000000000..402319cec --- /dev/null +++ b/cli/src/utils/freebuff-model-navigation.ts @@ -0,0 +1,36 @@ +export function nextSelectableFreebuffModelId(params: { + modelIds: readonly string[] + focusedId: string + direction: 'forward' | 'backward' + isSelectable: (modelId: string) => boolean +}): string | null { + const { modelIds, focusedId, direction, isSelectable } = params + if (modelIds.length === 0) return null + + const currentIdx = modelIds.indexOf(focusedId) + if (currentIdx === -1) return null + + const step = direction === 'forward' ? 1 : -1 + for (let offset = 1; offset <= modelIds.length; offset++) { + const idx = + (currentIdx + step * offset + modelIds.length) % modelIds.length + const candidate = modelIds[idx] + if (isSelectable(candidate)) return candidate + } + + return null +} + +export function resolveFreebuffModelCommitTarget(params: { + focusedId: string + selectedId: string + committedId: string | null + isSelectable: (modelId: string) => boolean +}): string | null { + const { focusedId, selectedId, committedId, isSelectable } = params + const targetId = isSelectable(focusedId) ? focusedId : selectedId + + if (targetId === committedId) return null + if (!isSelectable(targetId)) return null + return targetId +} From 958892f4f3d5704f55eae776cd08d7c486148607 Mon Sep 17 00:00:00 2001 From: James Grugett Date: Fri, 24 Apr 2026 23:43:09 -0700 Subject: [PATCH 2/2] Address freebuff picker review comments --- cli/src/utils/freebuff-model-navigation.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/src/utils/freebuff-model-navigation.ts b/cli/src/utils/freebuff-model-navigation.ts index 402319cec..eef067d5c 100644 --- a/cli/src/utils/freebuff-model-navigation.ts +++ b/cli/src/utils/freebuff-model-navigation.ts @@ -11,6 +11,8 @@ export function nextSelectableFreebuffModelId(params: { if (currentIdx === -1) return null const step = direction === 'forward' ? 1 : -1 + // Include a full wrap back to the current item so arrows stay on the same + // selectable model when every peer is unavailable. for (let offset = 1; offset <= modelIds.length; offset++) { const idx = (currentIdx + step * offset + modelIds.length) % modelIds.length @@ -30,7 +32,6 @@ export function resolveFreebuffModelCommitTarget(params: { const { focusedId, selectedId, committedId, isSelectable } = params const targetId = isSelectable(focusedId) ? focusedId : selectedId - if (targetId === committedId) return null - if (!isSelectable(targetId)) return null + if (!isSelectable(targetId) || targetId === committedId) return null return targetId }