[codex] Fix freebuff model picker enter (#545)

Co-authored-by: James Grugett <jahooma@gmail.com>

Author: brandonkachen

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'
 
@@ -173,30 +177,32 @@ export const FreebuffModelSelector: React.FC = () => {
           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],
     ),
   )
 
@@ -219,7 +225,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

cli/src/components/waiting-room-screen.tsx

@@ -173,7 +173,7 @@ export const WaitingRoomScreen: React.FC<WaitingRoomScreenProps> = ({
             maxWidth: contentMaxWidth,
           }}
         >
-          {error && !session && (
+          {error && (!session || session.status === 'none') && (
             <text style={{ fg: theme.secondary, wrapMode: 'word' }}>
               ⚠ {error}
             </text>

cli/src/hooks/use-freebuff-session.ts

@@ -376,6 +376,7 @@ export function useFreebuffSession(): UseFreebuffSessionResult {
     let abortController = new AbortController()
     let timer: ReturnType<typeof setTimeout> | 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

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()
+  })
+})

cli/src/utils/freebuff-model-navigation.ts

@@ -0,0 +1,37 @@
+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
+  // 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
+    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 (!isSelectable(targetId) || targetId === committedId) return null
+  return targetId
+}