Skip to content

Commit 79715fd

Browse files
committed
Fix stuck model switches from false end-failure in session takeover
The model_locked takeover only released rows that were active on the locked model, so ended-within-grace rows (the stale rows the branch exists for) and already-released rows produced an explanation claiming the end had failed when no DELETE was ever attempted, locking the user out of their pick for the session hour (#1298). Route the follow-up GET through a pure planner that separates release (live row attributable to the lock) from retry (row already gone, re-POST the consumed pick once) from explain (real read/delete failure or an unattributable row). Grace-window DELETE replays the refund receipt idempotently via the instance id.
1 parent 529f7c0 commit 79715fd

3 files changed

Lines changed: 96 additions & 9 deletions

File tree

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

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
holdsLiveFreebuffSlot,
4343
isFreebuffSessionTimeoutError,
4444
mergeCompactActiveSession,
45+
planModelLockedSwitch,
4546
} from '../utils/freebuff-session-api'
4647
import {
4748
failedPollDelayMs,
@@ -589,25 +590,30 @@ export function useFreebuffSession(): UseFreebuffSessionResult {
589590
const current = getFreebuffModel(next.currentModel).displayName
590591
const requested = getFreebuffModel(explicitPickModel).displayName
591592
let released = false
593+
let lockRaced = false
592594
try {
593595
const held = await callFreebuffSession('GET', token, {
594596
signal: fetchController.signal,
595597
})
596598
if (
597599
!cancelled &&
598600
!fetchController.signal.aborted &&
599-
generation === restartGeneration &&
600-
held.status === 'active' &&
601-
held.model === next.currentModel
601+
generation === restartGeneration
602602
) {
603-
await useFreebuffSessionStore
604-
.getState()
605-
.releaseSlot(held, fetchController.signal)
606-
released = true
603+
const action = planModelLockedSwitch(held, next.currentModel)
604+
if (action === 'release') {
605+
await useFreebuffSessionStore
606+
.getState()
607+
.releaseSlot(held, fetchController.signal)
608+
released = true
609+
} else if (action === 'retry') {
610+
lockRaced = true
611+
}
607612
}
608613
} catch {
609-
// DELETE failed — fall through to the revert-with-explanation
610-
// path below rather than stranding the user mid-switch.
614+
// Reading or deleting the held row failed — fall through to the
615+
// revert-with-explanation path below rather than stranding the
616+
// user mid-switch.
611617
}
612618
if (
613619
cancelled ||
@@ -629,6 +635,16 @@ export function useFreebuffSession(): UseFreebuffSessionResult {
629635
schedule(0)
630636
return
631637
}
638+
if (lockRaced) {
639+
// The GET found no row left to release: the lock raced and
640+
// released itself. Re-POST rather than reporting an end that
641+
// was never needed (#1298). The marker is consumed, so a
642+
// second model_locked takes the revert branch — one retry,
643+
// no loop.
644+
nextMethod = 'POST'
645+
schedule(0)
646+
return
647+
}
632648
useChatStore
633649
.getState()
634650
.setMessages((prev) => [

cli/src/utils/__tests__/freebuff-session-api.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@ import {
1010
classifyFreebuffSessionRequestFailure,
1111
FreebuffSessionRequestError,
1212
mergeCompactActiveSession,
13+
planModelLockedSwitch,
1314
} from '../freebuff-session-api'
1415

16+
import type { FreebuffSessionServerResponse } from '@codebuff/common/types/freebuff-session'
17+
1518
let fetchSpy: ReturnType<typeof spyOn> | undefined
1619

1720
afterEach(() => {
@@ -230,3 +233,45 @@ test('DELETE sends the held instance and preserves the server refund receipt', a
230233
'held-cli',
231234
)
232235
})
236+
237+
const heldActive = (model: string): FreebuffSessionServerResponse => ({
238+
status: 'active',
239+
accessTier: 'full',
240+
instanceId: 'inst-held',
241+
model,
242+
admittedAt: '2026-09-09T00:00:00.000Z',
243+
expiresAt: '2026-09-09T01:00:00.000Z',
244+
remainingMs: 60_000,
245+
})
246+
247+
test('a deliberate pick releases the row that holds the lock', () => {
248+
expect(planModelLockedSwitch(heldActive('x/mimo'), 'x/mimo')).toBe('release')
249+
})
250+
251+
test('an ended row still inside the grace window is released (#1298)', () => {
252+
expect(
253+
planModelLockedSwitch(
254+
{ status: 'ended', instanceId: 'inst-held' },
255+
'x/mimo',
256+
),
257+
).toBe('release')
258+
})
259+
260+
test('an ended row past grace means nothing to end: retry the pick', () => {
261+
expect(planModelLockedSwitch({ status: 'ended' }, 'x/mimo')).toBe('retry')
262+
})
263+
264+
test('a swept row means nothing to end: retry the pick', () => {
265+
expect(planModelLockedSwitch({ status: 'none' }, 'x/mimo')).toBe('retry')
266+
})
267+
268+
test('a different model than the lock named is never deleted', () => {
269+
expect(planModelLockedSwitch(heldActive('x/other'), 'x/mimo')).toBe('explain')
270+
})
271+
272+
test('no readable row explains instead of claiming a failed end', () => {
273+
expect(planModelLockedSwitch(undefined, 'x/mimo')).toBe('explain')
274+
expect(planModelLockedSwitch({ status: 'superseded' }, 'x/mimo')).toBe(
275+
'explain',
276+
)
277+
})

cli/src/utils/freebuff-session-api.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,3 +227,29 @@ export function holdsLiveFreebuffSlot(
227227
(current.status === 'ended' && Boolean(current.instanceId))
228228
)
229229
}
230+
231+
/** What the `model_locked` takeover should do with the row a follow-up GET
232+
* returned. One function asks this so "we tried to end it and failed" stays
233+
* distinguishable from "there was nothing to end":
234+
* - 'release': the row still holds a slot — active on the locked model, or
235+
* `ended` inside the grace window (the stale row from a crashed CLI; its
236+
* DELETE replays the refund receipt idempotently via the instance id).
237+
* - 'retry': no row is left to release (#1298: the lock raced and released
238+
* itself) — re-POST instead of reporting a failed end.
239+
* - 'explain': anything we cannot attribute to this lock, including a row
240+
* that arrived under an unexpected status — never delete it blindly.
241+
*/
242+
export type ModelLockedSwitchAction = 'release' | 'retry' | 'explain'
243+
244+
export function planModelLockedSwitch(
245+
held: FreebuffSessionServerResponse | undefined,
246+
lockedModel: string,
247+
): ModelLockedSwitchAction {
248+
if (!held) return 'explain'
249+
if (held.status === 'none') return 'retry'
250+
if (held.status === 'ended') {
251+
return held.instanceId ? 'release' : 'retry'
252+
}
253+
if (held.status === 'active' && held.model === lockedModel) return 'release'
254+
return 'explain'
255+
}

0 commit comments

Comments
 (0)