Skip to content

Commit e3a4e71

Browse files
Fix freebuff stale session takeover (#605)
Co-authored-by: James Grugett <jahooma@gmail.com>
1 parent ab65f2e commit e3a4e71

4 files changed

Lines changed: 169 additions & 11 deletions

File tree

cli/src/hooks/use-exit-handler.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { useCallback, useEffect, useRef, useState } from 'react'
33
import { getCurrentChatId } from '../project-files'
44
import { flushAnalytics } from '../utils/analytics'
55
import { IS_FREEBUFF } from '../utils/constants'
6+
import { exitFreebuffCleanly } from '../utils/freebuff-exit'
67
import { withTimeout } from '../utils/terminal-color-detection'
78

89
import type { InputValue } from '../types/store'
@@ -38,6 +39,19 @@ function setupExitMessageHandler() {
3839
})
3940
}
4041

42+
function exitCli(): void {
43+
if (IS_FREEBUFF) {
44+
void exitFreebuffCleanly()
45+
return
46+
}
47+
48+
withTimeout(flushAnalytics(), EXIT_FLUSH_TIMEOUT_MS, undefined).finally(
49+
() => {
50+
process.exit(0)
51+
},
52+
)
53+
}
54+
4155
export const useExitHandler = ({
4256
inputValue,
4357
setInputValue,
@@ -70,9 +84,7 @@ export const useExitHandler = ({
7084
exitWarningTimeoutRef.current = null
7185
}
7286

73-
withTimeout(flushAnalytics(), EXIT_FLUSH_TIMEOUT_MS, undefined).then(() => {
74-
process.exit(0)
75-
})
87+
exitCli()
7688
return true
7789
}, [inputValue, setInputValue, nextCtrlCWillExit])
7890

@@ -83,11 +95,7 @@ export const useExitHandler = ({
8395
exitWarningTimeoutRef.current = null
8496
}
8597

86-
withTimeout(flushAnalytics(), EXIT_FLUSH_TIMEOUT_MS, undefined).finally(
87-
() => {
88-
process.exit(0)
89-
},
90-
)
98+
exitCli()
9199
}
92100

93101
process.on('SIGINT', handleSigint)

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ import {
1212
import { useFreebuffSessionStore } from '../state/freebuff-session-store'
1313
import { getAuthTokenDetails } from '../utils/auth'
1414
import { IS_FREEBUFF } from '../utils/constants'
15+
import {
16+
isFreebuffInstanceOwnedByDeadLocalProcess,
17+
recordFreebuffInstanceOwner,
18+
} from '../utils/freebuff-instance-owner'
1519
import { logger } from '../utils/logger'
1620
import { saveFreebuffModelPreference } from '../utils/settings'
1721

@@ -363,9 +367,9 @@ interface UseFreebuffSessionResult {
363367
* Manages the freebuff waiting-room session lifecycle:
364368
* - GET on mount to probe state (no auto-join; the user picks a model in
365369
* the landing screen, which calls joinFreebuffQueue)
366-
* - if the probe sees an existing seat, asks before POSTing to take over
367-
* (rotates the instance id so any other CLI on the same account is
368-
* superseded)
370+
* - if the probe sees an existing seat, auto-takes-over when the prior
371+
* local owner process is gone; otherwise asks before POSTing to rotate
372+
* the instance id so any other CLI on the same account is superseded
369373
* - polls GET while queued (fast) or active (slow) to keep state fresh
370374
* - re-POSTs on explicit refresh (chat gate rejected us, user switched
371375
* models, user rejoined after ending)
@@ -406,6 +410,9 @@ export function useFreebuffSession(): UseFreebuffSessionResult {
406410
let nextMethod: 'GET' | 'POST' = 'GET'
407411

408412
const apply = (next: FreebuffSessionResponse) => {
413+
if (next.status === 'queued' || next.status === 'active') {
414+
recordFreebuffInstanceOwner(next.instanceId)
415+
}
409416
setSession(next)
410417
setError(null)
411418
previousStatus = next.status
@@ -479,6 +486,14 @@ export function useFreebuffSession(): UseFreebuffSessionResult {
479486
(next.status === 'queued' || next.status === 'active')
480487
) {
481488
useFreebuffModelStore.getState().setSelectedModel(next.model)
489+
// A fast restart after Ctrl+C can observe the old server row before
490+
// best-effort DELETE lands. If the row belongs to a dead local
491+
// process, silently do the same POST as the Take over button.
492+
if (isFreebuffInstanceOwnedByDeadLocalProcess(next.instanceId)) {
493+
nextMethod = 'POST'
494+
schedule(0)
495+
return
496+
}
482497
apply({ status: 'takeover_prompt', model: next.model })
483498
return
484499
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import fs from 'fs'
2+
import os from 'os'
3+
import path from 'path'
4+
5+
import { afterEach, beforeEach, describe, expect, test } from 'bun:test'
6+
7+
import { ensureCliTestEnv } from '../../__tests__/test-utils'
8+
9+
const OWNER_FILE = 'freebuff-instance-owner.json'
10+
11+
ensureCliTestEnv()
12+
13+
const { getConfigDir } = await import('../auth')
14+
const {
15+
isFreebuffInstanceOwnedByDeadLocalProcess,
16+
recordFreebuffInstanceOwner,
17+
} = await import('../freebuff-instance-owner')
18+
19+
describe('freebuff instance owner', () => {
20+
let originalHome: string | undefined
21+
let tempHome: string
22+
23+
const ownerPath = () => path.join(getConfigDir(), OWNER_FILE)
24+
25+
beforeEach(() => {
26+
originalHome = process.env.HOME
27+
tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'freebuff-owner-'))
28+
process.env.HOME = tempHome
29+
})
30+
31+
afterEach(() => {
32+
if (originalHome === undefined) {
33+
delete process.env.HOME
34+
} else {
35+
process.env.HOME = originalHome
36+
}
37+
fs.rmSync(tempHome, { recursive: true, force: true })
38+
})
39+
40+
test('does not classify the current process as dead', () => {
41+
recordFreebuffInstanceOwner('inst-current')
42+
43+
expect(
44+
isFreebuffInstanceOwnedByDeadLocalProcess('inst-current'),
45+
).toBe(false)
46+
})
47+
48+
test('classifies a matching owner with a dead pid as dead', () => {
49+
fs.mkdirSync(getConfigDir(), { recursive: true })
50+
fs.writeFileSync(
51+
ownerPath(),
52+
JSON.stringify({ instanceId: 'inst-dead', pid: 2_147_483_647 }),
53+
)
54+
55+
expect(isFreebuffInstanceOwnedByDeadLocalProcess('inst-dead')).toBe(true)
56+
})
57+
58+
test('ignores a dead pid for a different instance id', () => {
59+
fs.mkdirSync(getConfigDir(), { recursive: true })
60+
fs.writeFileSync(
61+
ownerPath(),
62+
JSON.stringify({ instanceId: 'inst-other', pid: 2_147_483_647 }),
63+
)
64+
65+
expect(
66+
isFreebuffInstanceOwnedByDeadLocalProcess('inst-current'),
67+
).toBe(false)
68+
})
69+
})
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import fs from 'fs'
2+
import path from 'path'
3+
4+
import { getConfigDir } from './auth'
5+
import { logger } from './logger'
6+
7+
interface FreebuffInstanceOwner {
8+
instanceId: string
9+
pid: number
10+
}
11+
12+
const OWNER_FILE = 'freebuff-instance-owner.json'
13+
14+
const getOwnerPath = (): string => path.join(getConfigDir(), OWNER_FILE)
15+
16+
function readOwner(): FreebuffInstanceOwner | null {
17+
try {
18+
const raw = fs.readFileSync(getOwnerPath(), 'utf8')
19+
const parsed = JSON.parse(raw) as Partial<FreebuffInstanceOwner>
20+
if (
21+
typeof parsed.instanceId !== 'string' ||
22+
typeof parsed.pid !== 'number'
23+
) {
24+
return null
25+
}
26+
return {
27+
instanceId: parsed.instanceId,
28+
pid: parsed.pid,
29+
}
30+
} catch {
31+
return null
32+
}
33+
}
34+
35+
function isProcessRunning(pid: number): boolean {
36+
if (!Number.isInteger(pid) || pid <= 0) return false
37+
try {
38+
process.kill(pid, 0)
39+
return true
40+
} catch (error) {
41+
return (error as NodeJS.ErrnoException).code === 'EPERM'
42+
}
43+
}
44+
45+
export function recordFreebuffInstanceOwner(instanceId: string): void {
46+
try {
47+
fs.mkdirSync(getConfigDir(), { recursive: true })
48+
fs.writeFileSync(
49+
getOwnerPath(),
50+
JSON.stringify({ instanceId, pid: process.pid }, null, 2),
51+
)
52+
} catch (error) {
53+
logger.debug(
54+
{ error: error instanceof Error ? error.message : String(error) },
55+
'[freebuff-session] Failed to record local owner',
56+
)
57+
}
58+
}
59+
60+
export function isFreebuffInstanceOwnedByDeadLocalProcess(
61+
instanceId: string,
62+
): boolean {
63+
const owner = readOwner()
64+
if (!owner || owner.instanceId !== instanceId) return false
65+
return !isProcessRunning(owner.pid)
66+
}

0 commit comments

Comments
 (0)