Skip to content

Commit 908f0ac

Browse files
brandonkachenbrandonkachen
committed
fix: improve error handling and testing across CLI and SDK
- Add null check for loginUrl in login modal - Enhance timer tests with JSDOM setup and async handling - Make onBeforeMessageSend required and add auto-scroll on errors - Update health check endpoint to use website URL with JSON validation
1 parent 53796a2 commit 908f0ac

File tree

4 files changed

+176
-30
lines changed

4 files changed

+176
-30
lines changed

‎cli/src/components/login-modal.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,9 @@ export const LoginModal = ({
252252

253253
// Handle login URL activation
254254
const handleActivateLoginUrl = useCallback(async () => {
255+
if (!loginUrl) {
256+
return
257+
}
255258
try {
256259
await open(loginUrl)
257260
} catch (err) {

‎cli/src/hooks/__tests__/use-send-message-timer.test.ts‎

Lines changed: 144 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,53 @@
1-
import { renderHook, waitFor } from '@testing-library/react'
1+
import { renderHook, waitFor, act } from '@testing-library/react'
2+
import { JSDOM } from 'jsdom'
23
import { mock, spyOn } from 'bun:test'
34
import { describe, test, expect, beforeEach, afterEach } from 'bun:test'
45

56
import { useSendMessage } from '../use-send-message'
67
import * as codebuffClient from '../../utils/codebuff-client'
78
import * as loadAgentDefs from '../../utils/load-agent-definitions'
9+
import * as localAgentRegistry from '../../utils/local-agent-registry'
810
import { logger } from '../../utils/logger'
911

1012
// Type for logger call arguments
1113
type LoggerInfoCall = [data: Record<string, any>, message: string]
1214

13-
describe('useSendMessage timer', () => {
15+
const timerDescribe =
16+
process.env.SKIP_TIMER_TESTS === '1' ? describe.skip : describe
17+
18+
if (typeof document === 'undefined') {
19+
const dom = new JSDOM('<!doctype html><html><body></body></html>')
20+
const { window } = dom
21+
22+
const globalWindow = window as unknown as Window & typeof globalThis
23+
24+
;(globalThis as any).window = globalWindow
25+
;(globalThis as any).document = globalWindow.document
26+
27+
if (typeof (globalThis as any).navigator === 'undefined') {
28+
Object.defineProperty(globalThis, 'navigator', {
29+
value: globalWindow.navigator,
30+
configurable: true,
31+
})
32+
}
33+
34+
const descriptors = Object.getOwnPropertyDescriptors(globalWindow)
35+
for (const [key, descriptor] of Object.entries(descriptors)) {
36+
if (typeof (globalThis as any)[key] === 'undefined') {
37+
Object.defineProperty(globalThis, key, descriptor)
38+
}
39+
}
40+
41+
if (typeof globalThis.requestAnimationFrame === 'undefined') {
42+
;(globalThis as any).requestAnimationFrame = (cb: FrameRequestCallback) =>
43+
setTimeout(cb, 0)
44+
}
45+
if (typeof globalThis.cancelAnimationFrame === 'undefined') {
46+
;(globalThis as any).cancelAnimationFrame = (id: number) => clearTimeout(id)
47+
}
48+
}
49+
50+
timerDescribe('useSendMessage timer', () => {
1451
let mockSetMessages: ReturnType<typeof mock>
1552
let mockSetFocusedAgentId: ReturnType<typeof mock>
1653
let mockSetInputFocused: ReturnType<typeof mock>
@@ -29,6 +66,7 @@ describe('useSendMessage timer', () => {
2966
let activeSubagentsRef: React.MutableRefObject<Set<string>>
3067
let isChainInProgressRef: React.MutableRefObject<boolean>
3168
let abortControllerRef: React.MutableRefObject<AbortController | null>
69+
let onBeforeMessageSend: ReturnType<typeof mock>
3270

3371
beforeEach(() => {
3472
// Setup state setter mocks
@@ -66,13 +104,18 @@ describe('useSendMessage timer', () => {
66104
activeSubagentsRef = { current: new Set() }
67105
isChainInProgressRef = { current: false }
68106
abortControllerRef = { current: null }
107+
onBeforeMessageSend = mock(async () => ({ success: true, errors: [] }))
69108

70109
// Spy on external module functions
71110
spyOn(codebuffClient, 'getCodebuffClient').mockReturnValue({
72111
run: mock(async () => ({ credits: 100 })),
73112
} as any)
74113
spyOn(codebuffClient, 'formatToolOutput').mockReturnValue('formatted output')
75114
spyOn(loadAgentDefs, 'loadAgentDefinitions').mockReturnValue([])
115+
spyOn(localAgentRegistry, 'getLoadedAgentsData').mockReturnValue({
116+
agents: [],
117+
agentsDir: '',
118+
})
76119
spyOn(logger, 'info').mockImplementation(() => {})
77120
spyOn(logger, 'error').mockImplementation(() => {})
78121
spyOn(logger, 'warn').mockImplementation(() => {})
@@ -102,18 +145,17 @@ describe('useSendMessage timer', () => {
102145
setIsStreaming: mockSetIsStreaming,
103146
setCanProcessQueue: mockSetCanProcessQueue,
104147
abortControllerRef,
105-
mainAgentTimer: {
106-
start: mockSetMainAgentStreamStartTime.bind(null, Date.now()),
107-
stop: mockSetMainAgentStreamStartTime.bind(null, null),
108-
elapsedSeconds: 0,
109-
startTime: null,
110-
},
148+
onBeforeMessageSend,
111149
scrollToLatest: mockScrollToLatest,
112150
availableWidth: 80,
113151
}),
114152
)
115153

116-
await result.current.sendMessage('test message', { agentMode: 'FAST' })
154+
await waitFor(() => expect(result.current).toBeTruthy())
155+
156+
await act(async () => {
157+
await result.current!.sendMessage('test message', { agentMode: 'FAST' })
158+
})
117159

118160
await waitFor(() => {
119161
const loggerInfoSpy = logger.info as ReturnType<typeof spyOn>
@@ -165,18 +207,17 @@ describe('useSendMessage timer', () => {
165207
setIsStreaming: mockSetIsStreaming,
166208
setCanProcessQueue: mockSetCanProcessQueue,
167209
abortControllerRef,
168-
mainAgentTimer: {
169-
start: mockSetMainAgentStreamStartTime.bind(null, Date.now()),
170-
stop: mockSetMainAgentStreamStartTime.bind(null, null),
171-
elapsedSeconds: 0,
172-
startTime: null,
173-
},
210+
onBeforeMessageSend,
174211
scrollToLatest: mockScrollToLatest,
175212
availableWidth: 80,
176213
}),
177214
)
178215

179-
await result.current.sendMessage('test message', { agentMode: 'FAST' })
216+
await waitFor(() => expect(result.current).toBeTruthy())
217+
218+
await act(async () => {
219+
await result.current!.sendMessage('test message', { agentMode: 'FAST' })
220+
})
180221

181222
await waitFor(() => {
182223
const loggerInfoSpy = logger.info as ReturnType<typeof spyOn>
@@ -220,18 +261,17 @@ describe('useSendMessage timer', () => {
220261
setIsStreaming: mockSetIsStreaming,
221262
setCanProcessQueue: mockSetCanProcessQueue,
222263
abortControllerRef,
223-
mainAgentTimer: {
224-
start: mockSetMainAgentStreamStartTime.bind(null, Date.now()),
225-
stop: mockSetMainAgentStreamStartTime.bind(null, null),
226-
elapsedSeconds: 0,
227-
startTime: null,
228-
},
264+
onBeforeMessageSend,
229265
scrollToLatest: mockScrollToLatest,
230266
availableWidth: 80,
231267
}),
232268
)
233269

234-
await result.current.sendMessage('test message', { agentMode: 'FAST' })
270+
await waitFor(() => expect(result.current).toBeTruthy())
271+
272+
await act(async () => {
273+
await result.current!.sendMessage('test message', { agentMode: 'FAST' })
274+
})
235275

236276
await waitFor(() => {
237277
// Find the setMessages call that marks completion
@@ -242,16 +282,94 @@ describe('useSendMessage timer', () => {
242282
const testMessages = [
243283
{
244284
id: 'ai-123',
245-
variant: 'ai',
285+
variant: 'ai' as const,
246286
content: '',
247287
blocks: [],
288+
timestamp: '0',
289+
metadata: { completionTimeSeconds: 12.34 } as Record<
290+
string,
291+
unknown
292+
>,
248293
},
249294
]
250-
const result = fn(testMessages)
251-
return result.some((msg: any) => msg.isComplete && msg.completionTime)
295+
296+
fn(testMessages as any)
297+
const metadata = (testMessages[0] as any).metadata
298+
return Boolean(metadata && 'completionTimeSeconds' in metadata)
252299
})
253300

254301
expect(completionCall).toBeDefined()
302+
303+
if (completionCall) {
304+
const fn = completionCall[0] as (messages: any[]) => any[]
305+
const testMessages = [
306+
{
307+
id: 'ai-456',
308+
variant: 'ai' as const,
309+
content: '',
310+
blocks: [],
311+
timestamp: '0',
312+
metadata: {} as Record<string, unknown>,
313+
},
314+
]
315+
316+
fn(testMessages as any)
317+
318+
const metadata = (testMessages[0] as any).metadata
319+
expect(metadata).toBeDefined()
320+
expect(typeof metadata?.completionTimeSeconds).toBe('number')
321+
expect((metadata?.completionTimeSeconds ?? 0)).toBeGreaterThanOrEqual(0)
322+
}
323+
})
324+
})
325+
326+
test('scrolls to latest when validation errors occur', async () => {
327+
const validationErrors = [
328+
{ id: 'agent-1', message: 'Field is required' },
329+
]
330+
onBeforeMessageSend.mockResolvedValue({ success: false, errors: validationErrors })
331+
332+
const { result } = renderHook(() =>
333+
useSendMessage({
334+
setMessages: mockSetMessages,
335+
setFocusedAgentId: mockSetFocusedAgentId,
336+
setInputFocused: mockSetInputFocused,
337+
inputRef,
338+
setStreamingAgents: mockSetStreamingAgents,
339+
setCollapsedAgents: mockSetCollapsedAgents,
340+
activeSubagentsRef,
341+
isChainInProgressRef,
342+
setActiveSubagents: mockSetActiveSubagents,
343+
setIsChainInProgress: mockSetIsChainInProgress,
344+
setIsWaitingForResponse: mockSetIsWaitingForResponse,
345+
startStreaming: mockStartStreaming,
346+
stopStreaming: mockStopStreaming,
347+
setIsStreaming: mockSetIsStreaming,
348+
setCanProcessQueue: mockSetCanProcessQueue,
349+
abortControllerRef,
350+
onBeforeMessageSend,
351+
mainAgentTimer: {
352+
start: mockSetMainAgentStreamStartTime.bind(null, Date.now()),
353+
stop: mockSetMainAgentStreamStartTime.bind(null, null),
354+
elapsedSeconds: 0,
355+
startTime: null,
356+
},
357+
scrollToLatest: mockScrollToLatest,
358+
availableWidth: 80,
359+
}),
360+
)
361+
362+
await waitFor(() => expect(result.current).toBeTruthy())
363+
364+
await act(async () => {
365+
await result.current!.sendMessage('test message', { agentMode: 'FAST' })
366+
})
367+
368+
await waitFor(() => {
369+
expect(mockScrollToLatest.mock.calls.length).toBeGreaterThanOrEqual(1)
370+
})
371+
await waitFor(() => {
372+
expect(mockScrollToLatest.mock.calls.length).toBeGreaterThanOrEqual(2)
255373
})
256374
})
257375
})

‎cli/src/hooks/use-send-message.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,10 @@ interface UseSendMessageOptions {
100100
setCanProcessQueue: (can: boolean) => void
101101
abortControllerRef: React.MutableRefObject<AbortController | null>
102102
agentId?: string
103-
onBeforeMessageSend?: () => Promise<{ success: boolean; errors: Array<{ id: string; message: string }> }>
103+
onBeforeMessageSend: () => Promise<{
104+
success: boolean
105+
errors: Array<{ id: string; message: string }>
106+
}>
104107
mainAgentTimer: ElapsedTimeTracker
105108
scrollToLatest: () => void
106109
availableWidth?: number
@@ -305,6 +308,7 @@ export const useSendMessage = ({
305308

306309
applyMessageUpdate((prev) => [...prev, errorMessage])
307310
await yieldToEventLoop()
311+
setTimeout(() => scrollToLatest(), 0)
308312

309313
return
310314
}
@@ -320,6 +324,7 @@ export const useSendMessage = ({
320324

321325
applyMessageUpdate((prev) => [...prev, errorMessage])
322326
await yieldToEventLoop()
327+
setTimeout(() => scrollToLatest(), 0)
323328

324329
return
325330
}

‎sdk/src/client.ts‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { BACKEND_URL } from './constants'
1+
import { BACKEND_URL, WEBSITE_URL } from './constants'
22
import { run } from './run'
33
import { API_KEY_ENV_VAR } from '../../common/src/old-constants'
44

@@ -63,11 +63,31 @@ export class CodebuffClient {
6363
*/
6464
public async checkConnection(): Promise<boolean> {
6565
try {
66-
const response = await fetch(`${BACKEND_URL}/healthz`, {
66+
const response = await fetch(`${WEBSITE_URL}/api/healthz`, {
6767
method: 'GET',
6868
signal: AbortSignal.timeout(5000), // 5 second timeout
6969
})
70-
return response.ok && (await response.text()) === 'ok'
70+
if (!response.ok) {
71+
return false
72+
}
73+
74+
let result: unknown
75+
try {
76+
result = await response.json()
77+
} catch {
78+
return false
79+
}
80+
81+
if (
82+
typeof result === 'object' &&
83+
result !== null &&
84+
'status' in result &&
85+
(result as { status?: unknown }).status === 'ok'
86+
) {
87+
return true
88+
}
89+
90+
return false
7191
} catch (error) {
7292
return false
7393
}

0 commit comments

Comments
 (0)