diff --git a/packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/chat-error-suppression.test.ts b/packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/chat-error-suppression.test.ts new file mode 100644 index 000000000..de6f1707d --- /dev/null +++ b/packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/chat-error-suppression.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from 'bun:test' +import { + computeVisibleChatError, + isStaleErrorMarkerStillCurrent, +} from './chat-error-suppression' + +describe('computeVisibleChatError', () => { + it('returns undefined when there is no chat error', () => { + expect( + computeVisibleChatError({ chatError: undefined, staleErrorMarker: null }), + ).toBeUndefined() + }) + + it('returns the chat error when no marker is set', () => { + const error = new Error('Credits exhausted') + expect( + computeVisibleChatError({ chatError: error, staleErrorMarker: null }), + ).toBe(error) + }) + + it('suppresses the chat error when its reference matches the marker', () => { + const error = new Error('Invalid Authentication') + expect( + computeVisibleChatError({ chatError: error, staleErrorMarker: error }), + ).toBeUndefined() + }) + + it('shows a fresh error reference even when a different one is suppressed', () => { + const oldError = new Error('Credits exhausted') + const newError = new Error('Failed to parse JSON') + expect( + computeVisibleChatError({ + chatError: newError, + staleErrorMarker: oldError, + }), + ).toBe(newError) + }) + + it('treats structurally identical errors as distinct references', () => { + // Two Error objects with the same message are still different references — + // the suppression key is identity, not message equality. + const first = new Error('Credits exhausted') + const second = new Error('Credits exhausted') + expect( + computeVisibleChatError({ chatError: second, staleErrorMarker: first }), + ).toBe(second) + }) +}) + +describe('isStaleErrorMarkerStillCurrent', () => { + it('is false when no marker is stored', () => { + expect(isStaleErrorMarkerStillCurrent(undefined, null)).toBe(false) + expect(isStaleErrorMarkerStillCurrent(new Error('x'), null)).toBe(false) + }) + + it('is false when the marker exists but the live error has cleared', () => { + const marker = new Error('Daily limit reached') + expect(isStaleErrorMarkerStillCurrent(undefined, marker)).toBe(false) + }) + + it('is true when the live error still references the marker', () => { + const marker = new Error('Daily limit reached') + expect(isStaleErrorMarkerStillCurrent(marker, marker)).toBe(true) + }) + + it('is false when the live error is a different reference', () => { + const marker = new Error('Daily limit reached') + const live = new Error('Daily limit reached') + expect(isStaleErrorMarkerStillCurrent(live, marker)).toBe(false) + }) +}) diff --git a/packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/chat-error-suppression.ts b/packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/chat-error-suppression.ts new file mode 100644 index 000000000..4d61dadaf --- /dev/null +++ b/packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/chat-error-suppression.ts @@ -0,0 +1,37 @@ +/** + * `useChat` from `@ai-sdk/react` keeps its `error` populated until a subsequent + * request succeeds — there is no public clearError. When the user switches + * provider or resets the conversation after a failure (e.g. credits exhausted, + * invalid auth, malformed response) the error banner persists and gives the + * impression the new provider also failed. Closes #862. + * + * The fix records the *reference* of the error at the moment the user takes a + * recovery action; while `chatError` still points at that same reference we + * treat it as stale and hide it from the UI. The next distinct error or a + * cleared error invalidates the marker and the banner behaves normally again. + */ +export interface ChatErrorVisibilityInput { + chatError: Error | undefined + staleErrorMarker: Error | null +} + +export function computeVisibleChatError( + input: ChatErrorVisibilityInput, +): Error | undefined { + if (!input.chatError) return undefined + if (input.chatError === input.staleErrorMarker) return undefined + return input.chatError +} + +/** + * Whether a stored marker is still relevant for the live error. Used by the + * effect that drops the marker once `useChat` produces a different error + * reference (new failure) or clears its error (successful request). + */ +export function isStaleErrorMarkerStillCurrent( + chatError: Error | undefined, + staleErrorMarker: Error | null, +): boolean { + if (!staleErrorMarker) return false + return chatError === staleErrorMarker +} diff --git a/packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/useChatSession.ts b/packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/useChatSession.ts index 11eded179..27a5e8f84 100644 --- a/packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/useChatSession.ts +++ b/packages/browseros-agent/apps/agent/entrypoints/sidepanel/index/useChatSession.ts @@ -49,6 +49,10 @@ import { toolApprovalConfigStorage, } from '@/lib/tool-approvals/storage' import { selectedWorkspaceStorage } from '@/lib/workspace/workspace-storage' +import { + computeVisibleChatError, + isStaleErrorMarkerStillCurrent, +} from './chat-error-suppression' import type { ChatMode } from './chatTypes' import { GetConversationWithMessagesDocument } from './graphql/chatSessionDocument' import { toLlmProviderConfig } from './sidepanel-chat-targets' @@ -534,6 +538,17 @@ export const useChatSession = (options?: ChatSessionOptions) => { syncExecutionHistory(messages, status) }, [messages, status, syncExecutionHistory]) + // Suppress useChat's error after the user switches provider or resets the + // conversation to recover from a failure. Drops the marker once useChat + // produces a different error reference or clears it on its own. See + // chat-error-suppression.ts for the why. + const [staleErrorMarker, setStaleErrorMarker] = useState(null) + useEffect(() => { + if (!isStaleErrorMarkerStillCurrent(chatError, staleErrorMarker)) { + setStaleErrorMarker(null) + } + }, [chatError, staleErrorMarker]) + // Save conversation only after streaming completes — not on every token const previousStatusRef = useRef(status) // biome-ignore lint/correctness/useExhaustiveDependencies: only save when streaming finishes @@ -785,13 +800,19 @@ export const useChatSession = (options?: ChatSessionOptions) => { }) if (target.kind === 'llm') setDefaultProvider(target.provider.id) - if ( + const providerActuallyChanged = previousTarget && - (previousTarget.kind !== target.kind || - previousTarget.id !== target.id) && - messagesRef.current.length > 0 - ) { - resetConversationState() + (previousTarget.kind !== target.kind || previousTarget.id !== target.id) + + if (providerActuallyChanged) { + // Closes #862: suppress the previous provider's error so the user can + // retry on the new one without the stale banner. Done independently of + // resetConversationState so it also covers the empty-conversation case + // (e.g. auth error on the very first send produces no assistant turn). + if (chatError) setStaleErrorMarker(chatError) + if (messagesRef.current.length > 0) { + resetConversationState() + } } } @@ -806,6 +827,9 @@ export const useChatSession = (options?: ChatSessionOptions) => { const resetConversation = () => { track(CONVERSATION_RESET_EVENT, { message_count: messages.length }) + // Closes #862: hide any error from the just-ended conversation so the + // fresh session starts with a clean slate. + if (chatError) setStaleErrorMarker(chatError) resetConversationState() } @@ -825,7 +849,7 @@ export const useChatSession = (options?: ChatSessionOptions) => { isSyncing: !isIntegrationsSynced, isRestoringConversation, agentUrlError, - chatError, + chatError: computeVisibleChatError({ chatError, staleErrorMarker }), handleSelectProvider, getActionForMessage, resetConversation,