diff --git a/frontend/__tests__/conversation-websocket-handler.test.tsx b/frontend/__tests__/conversation-websocket-handler.test.tsx index f0d4e79486..b10dbbcc7b 100644 --- a/frontend/__tests__/conversation-websocket-handler.test.tsx +++ b/frontend/__tests__/conversation-websocket-handler.test.tsx @@ -15,10 +15,11 @@ import { MemoryRouter, Route, Routes } from "react-router"; import { useOptimisticUserMessageStore } from "#/stores/optimistic-user-message-store"; import { useBrowserStore } from "#/stores/browser-store"; import { useCommandStore } from "#/stores/command-store"; +import { useErrorMessageStore } from "#/stores/error-message-store"; import { createMockMessageEvent, createMockUserMessageEvent, - createMockAgentErrorEvent, + createMockConversationErrorEvent, createMockBrowserObservationEvent, createMockBrowserNavigateActionEvent, createMockExecuteBashActionEvent, @@ -51,6 +52,9 @@ afterEach(() => { mswServer.resetHandlers(); // Clean up any React components cleanup(); + // Reset stores to prevent state leakage between tests + useErrorMessageStore.getState().removeErrorMessage(); + useEventStore.getState().clearEvents(); }); afterAll(async () => { @@ -277,16 +281,23 @@ describe("Conversation WebSocket Handler", () => { // 5. Error Handling Tests describe("Error Handling & Recovery", () => { - it("should update error message store on AgentErrorEvent", async () => { - // Create a mock AgentErrorEvent to send through WebSocket - const mockAgentErrorEvent = createMockAgentErrorEvent(); + beforeEach(() => { + // Clear stores before each error handling test to prevent state leakage + useErrorMessageStore.getState().removeErrorMessage(); + useEventStore.getState().clearEvents(); + }); + + it("should update error message store on ConversationErrorEvent", async () => { + // ConversationErrorEvent represents infrastructure/authentication errors + // that should be shown as a banner to the user. + const mockConversationErrorEvent = createMockConversationErrorEvent(); // Set up MSW to send the error event when connection is established mswServer.use( wsLink.addEventListener("connection", ({ client, server }) => { server.connect(); // Send the mock error event after connection - client.send(JSON.stringify(mockAgentErrorEvent)); + client.send(JSON.stringify(mockConversationErrorEvent)); }), ); @@ -299,7 +310,7 @@ describe("Conversation WebSocket Handler", () => { // Wait for connection and error event processing await waitFor(() => { expect(screen.getByTestId("error-message")).toHaveTextContent( - "Failed to execute command: Permission denied", + "Your session has expired. Please log in again.", ); }); }); @@ -438,6 +449,60 @@ describe("Conversation WebSocket Handler", () => { ); }); + it("should clear error message when a successful event is received after a ConversationErrorEvent", async () => { + // This test verifies that error banners disappear when follow-up messages + // are sent and received. Only ConversationErrorEvent sets the error banner, + // and any non-error event should clear it. + const conversationId = "test-conversation-error-clear"; + + // Set up MSW to mock event count API and send events + mswServer.use( + http.get( + `http://localhost:3000/api/conversations/${conversationId}/events/count`, + () => HttpResponse.json(2), + ), + wsLink.addEventListener("connection", ({ client, server }) => { + server.connect(); + + // Send a ConversationErrorEvent first (this sets the error banner) + const mockConversationErrorEvent = createMockConversationErrorEvent(); + client.send(JSON.stringify(mockConversationErrorEvent)); + + // Send a successful (non-error) event immediately after + // This simulates the user sending a follow-up message and receiving a response + const mockSuccessEvent = createMockMessageEvent({ + id: "success-event-after-error", + }); + client.send(JSON.stringify(mockSuccessEvent)); + }), + ); + + // Verify error message store is initially empty + expect(useErrorMessageStore.getState().errorMessage).toBeNull(); + + // Render with WebSocket context (minimal component just to trigger connection) + renderWithWebSocketContext( + , + conversationId, + `http://localhost:3000/api/conversations/${conversationId}`, + ); + + // Wait for connection + await waitFor(() => { + expect(screen.getByTestId("connection-state")).toHaveTextContent( + "OPEN", + ); + }); + + // Wait for both events to be received and error to be cleared + // The error was set by the first event (ConversationErrorEvent), + // then cleared by the second successful event (MessageEvent). + await waitFor(() => { + expect(useEventStore.getState().events.length).toBe(2); + expect(useErrorMessageStore.getState().errorMessage).toBeNull(); + }); + }); + it("should not create duplicate events when WebSocket reconnects with resend_all=true", async () => { const conversationId = "test-conversation-reconnect"; let connectionCount = 0; diff --git a/frontend/src/contexts/conversation-websocket-context.tsx b/frontend/src/contexts/conversation-websocket-context.tsx index 73bd2b365a..3e026db7eb 100644 --- a/frontend/src/contexts/conversation-websocket-context.tsx +++ b/frontend/src/contexts/conversation-websocket-context.tsx @@ -329,16 +329,17 @@ export function ConversationWebSocketProvider({ if (isV1Event(event)) { addEvent(event); - // Handle ConversationErrorEvent specifically + // Handle ConversationErrorEvent specifically - show error banner + // AgentErrorEvent errors are displayed inline in the chat, not as banners if (isConversationErrorEvent(event)) { setErrorMessage(event.detail); + } else { + // Clear error message on any non-ConversationErrorEvent + removeErrorMessage(); } - // Handle AgentErrorEvent specifically + // Track credit limit reached if AgentErrorEvent has budget-related error if (isAgentErrorEvent(event)) { - setErrorMessage(event.error); - - // Track credit limit reached if the error is budget-related if (isBudgetOrCreditError(event.error)) { trackCreditLimitReached({ conversationId: conversationId || "unknown", @@ -417,6 +418,7 @@ export function ConversationWebSocketProvider({ isLoadingHistoryMain, expectedEventCountMain, setErrorMessage, + removeErrorMessage, removeOptimisticUserMessage, queryClient, conversationId, @@ -424,6 +426,7 @@ export function ConversationWebSocketProvider({ appendInput, appendOutput, updateMetricsFromStats, + trackCreditLimitReached, ], ); diff --git a/frontend/src/mocks/mock-ws-helpers.ts b/frontend/src/mocks/mock-ws-helpers.ts index ae4e214943..84bfb33989 100644 --- a/frontend/src/mocks/mock-ws-helpers.ts +++ b/frontend/src/mocks/mock-ws-helpers.ts @@ -7,6 +7,7 @@ import { import { AgentStateChangeObservation } from "#/types/core/observations"; import { MessageEvent } from "#/types/v1/core"; import { AgentErrorEvent } from "#/types/v1/core/events/observation-event"; +import { ConversationErrorEvent } from "#/types/v1/core/events/conversation-state-event"; import { MockSessionMessaage } from "./session-history.mock"; export const generateAgentStateChangeObservation = ( @@ -236,3 +237,19 @@ export const createMockBrowserNavigateActionEvent = ( llm_response_id: "llm-response-789", security_risk: { level: "low" }, }); + +/** + * Creates a mock ConversationErrorEvent for testing conversation-level error handling + * These are infrastructure/authentication errors that should show error banners + */ +export const createMockConversationErrorEvent = ( + overrides: Partial = {}, +): ConversationErrorEvent => ({ + id: "conversation-error-123", + timestamp: new Date().toISOString(), + source: "environment", + kind: "ConversationErrorEvent", + code: "AuthenticationError", + detail: "Your session has expired. Please log in again.", + ...overrides, +}); diff --git a/frontend/src/types/v1/core/events/conversation-state-event.ts b/frontend/src/types/v1/core/events/conversation-state-event.ts index 93679d6671..38105f0ade 100644 --- a/frontend/src/types/v1/core/events/conversation-state-event.ts +++ b/frontend/src/types/v1/core/events/conversation-state-event.ts @@ -62,6 +62,11 @@ export interface ConversationState { } interface ConversationStateUpdateEventBase extends BaseEvent { + /** + * Discriminator field for type guards + */ + kind: "ConversationStateUpdateEvent"; + /** * The source is always "environment" for conversation state update events */ @@ -105,6 +110,11 @@ export type ConversationStateUpdateEvent = // Conversation error event - contains error information export interface ConversationErrorEvent extends BaseEvent { + /** + * Discriminator field for type guards + */ + kind: "ConversationErrorEvent"; + /** * The source is always "environment" for conversation error events */