From f8201f72c251f8236cc2320c23c078147343f986 Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Mon, 24 Nov 2025 09:30:11 -0800 Subject: [PATCH 1/4] Refactor chat to chat service and chat plugin Signed-off-by: Anan Zhuang --- src/core/public/chat/chat_service.mock.ts | 50 ++ src/core/public/chat/chat_service.test.ts | 334 +++++++++++++ src/core/public/chat/chat_service.ts | 130 ++++++ src/core/public/chat/index.ts | 24 + src/core/public/chat/no_op_chat_service.ts | 81 ++++ src/core/public/chat/types.ts | 252 ++++++++++ src/core/public/core_system.ts | 11 + src/core/public/index.ts | 14 + src/core/public/mocks.ts | 3 + src/core/public/plugins/plugin_context.ts | 2 + .../public/plugins/plugins_service.test.ts | 3 + .../chat/common/chat_capabilities.test.ts | 106 +++++ src/plugins/chat/common/chat_capabilities.ts | 59 +++ src/plugins/chat/common/types.ts | 24 +- src/plugins/chat/opensearch_dashboards.json | 3 +- .../graph_timeseries_data_action.test.tsx | 33 ++ .../actions/graph_timeseries_data_action.tsx | 3 +- .../components/chat_header_button.test.tsx | 152 ++++++ .../public/components/chat_header_button.tsx | 9 +- .../chat/public/components/chat_window.tsx | 21 +- src/plugins/chat/public/plugin.test.ts | 48 +- src/plugins/chat/public/plugin.ts | 123 ++++- .../chat/public/services/ag_ui_agent.ts | 11 +- .../services/chat_event_handler.test.ts | 17 + .../public/services/chat_event_handler.ts | 17 +- .../chat/public/services/chat_service.test.ts | 66 +-- .../chat/public/services/chat_service.ts | 71 ++- src/plugins/chat/public/types.ts | 2 + src/plugins/chat/server/index.ts | 1 + src/plugins/chat/server/plugin.ts | 1 + src/plugins/chat/server/routes/index.test.ts | 202 +++++++- src/plugins/chat/server/routes/index.ts | 171 ++++--- .../routes/ml_routes/generic_ml_router.ts | 176 +++++++ .../routes/ml_routes/ml_agent_router.ts | 74 +++ .../routes/ml_routes/ml_commons_agent.test.ts | 441 ------------------ .../routes/ml_routes/ml_commons_agent.ts | 73 --- .../routes/ml_routes/router_registry.ts | 50 ++ .../data_source_management/public/index.ts | 2 +- .../explore/opensearch_dashboards.json | 2 +- .../public/actions/ask_ai_action.test.ts | 49 +- .../explore/public/actions/ask_ai_action.ts | 11 +- src/plugins/explore/public/build_services.ts | 4 +- .../ask_ai_action_item.test.tsx | 25 +- .../ask_ai_action_item/ask_ai_action_item.tsx | 30 +- .../components/tabs/action_bar/action_bar.tsx | 3 +- src/plugins/explore/public/plugin.ts | 8 +- src/plugins/explore/public/types.ts | 4 +- yarn.lock | 31 +- 48 files changed, 2230 insertions(+), 797 deletions(-) create mode 100644 src/core/public/chat/chat_service.mock.ts create mode 100644 src/core/public/chat/chat_service.test.ts create mode 100644 src/core/public/chat/chat_service.ts create mode 100644 src/core/public/chat/index.ts create mode 100644 src/core/public/chat/no_op_chat_service.ts create mode 100644 src/core/public/chat/types.ts create mode 100644 src/plugins/chat/common/chat_capabilities.test.ts create mode 100644 src/plugins/chat/common/chat_capabilities.ts create mode 100644 src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts create mode 100644 src/plugins/chat/server/routes/ml_routes/ml_agent_router.ts delete mode 100644 src/plugins/chat/server/routes/ml_routes/ml_commons_agent.test.ts delete mode 100644 src/plugins/chat/server/routes/ml_routes/ml_commons_agent.ts create mode 100644 src/plugins/chat/server/routes/ml_routes/router_registry.ts diff --git a/src/core/public/chat/chat_service.mock.ts b/src/core/public/chat/chat_service.mock.ts new file mode 100644 index 000000000000..6e79709e278f --- /dev/null +++ b/src/core/public/chat/chat_service.mock.ts @@ -0,0 +1,50 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { BehaviorSubject } from 'rxjs'; +import { ChatServiceSetup, ChatServiceStart } from './types'; + +const createSetupContractMock = (): jest.Mocked => ({ + setImplementation: jest.fn(), + setSuggestedActionsService: jest.fn(), + suggestedActionsService: undefined, +}); + +const createStartContractMock = (): jest.Mocked => ({ + isAvailable: jest.fn().mockReturnValue(false), + isWindowOpen: jest.fn().mockReturnValue(false), + getThreadId$: jest.fn().mockReturnValue(new BehaviorSubject('')), + getThreadId: jest.fn().mockReturnValue(''), + openWindow: jest.fn(), + closeWindow: jest.fn(), + getWindowState: jest.fn().mockReturnValue({ + isWindowOpen: false, + windowMode: 'sidecar', + paddingSize: 400, + }), + sendMessage: jest.fn().mockResolvedValue({ + observable: null, + userMessage: { id: 'mock-id', role: 'user', content: 'mock-content' }, + }), + sendMessageWithWindow: jest.fn().mockResolvedValue({ + observable: null, + userMessage: { id: 'mock-id', role: 'user', content: 'mock-content' }, + }), + getWindowState$: jest.fn().mockReturnValue( + new BehaviorSubject({ + isWindowOpen: false, + windowMode: 'sidecar', + paddingSize: 400, + }) + ), + onWindowOpen: jest.fn().mockReturnValue(() => {}), + onWindowClose: jest.fn().mockReturnValue(() => {}), + suggestedActionsService: undefined, +}); + +export const coreChatServiceMock = { + createSetupContract: createSetupContractMock, + createStartContract: createStartContractMock, +}; diff --git a/src/core/public/chat/chat_service.test.ts b/src/core/public/chat/chat_service.test.ts new file mode 100644 index 000000000000..648bb5bcbc8b --- /dev/null +++ b/src/core/public/chat/chat_service.test.ts @@ -0,0 +1,334 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { BehaviorSubject } from 'rxjs'; +import { ChatService } from './chat_service'; +import { ChatImplementationFunctions, ChatWindowState, UserMessage } from './types'; + +describe('ChatService', () => { + let service: ChatService; + let mockImplementation: ChatImplementationFunctions; + let mockThreadId$: BehaviorSubject; + let mockWindowState$: BehaviorSubject; + + beforeEach(() => { + service = new ChatService(); + mockThreadId$ = new BehaviorSubject('test-thread-id'); + mockWindowState$ = new BehaviorSubject({ + isWindowOpen: false, + windowMode: 'sidecar', + paddingSize: 400, + }); + + // Create complete mock implementation + mockImplementation = { + sendMessage: jest.fn().mockResolvedValue({ + observable: null, + userMessage: { id: '1', role: 'user', content: 'test' }, + }), + sendMessageWithWindow: jest.fn().mockResolvedValue({ + observable: null, + userMessage: { id: '1', role: 'user', content: 'test' }, + }), + getThreadId: jest.fn().mockReturnValue('test-thread-id'), + getThreadId$: jest.fn().mockReturnValue(mockThreadId$.asObservable()), + isWindowOpen: jest.fn().mockReturnValue(false), + openWindow: jest.fn().mockResolvedValue(undefined), + closeWindow: jest.fn().mockResolvedValue(undefined), + getWindowState: jest.fn().mockReturnValue({ + isWindowOpen: false, + windowMode: 'sidecar', + paddingSize: 400, + }), + getWindowState$: jest.fn().mockReturnValue(mockWindowState$.asObservable()), + onWindowOpen: jest.fn().mockReturnValue(() => {}), + onWindowClose: jest.fn().mockReturnValue(() => {}), + }; + }); + + describe('setup', () => { + it('returns valid setup contract', () => { + const setupContract = service.setup(); + + expect(setupContract).toEqual({ + setImplementation: expect.any(Function), + setFallbackImplementation: expect.any(Function), + setSuggestedActionsService: expect.any(Function), + suggestedActionsService: undefined, + }); + }); + + it('setImplementation stores the implementation functions', () => { + const setupContract = service.setup(); + + expect(() => setupContract.setImplementation(mockImplementation)).not.toThrow(); + }); + }); + + describe('start', () => { + describe('availability logic', () => { + it('should be unavailable when no implementation is registered', () => { + const startContract = service.start(); + expect(startContract.isAvailable()).toBe(false); + }); + + it('should be available after implementation is registered', () => { + const setupContract = service.setup(); + setupContract.setImplementation(mockImplementation); + const startContract = service.start(); + expect(startContract.isAvailable()).toBe(true); + }); + }); + + describe('service interface', () => { + let startContract: any; + + beforeEach(() => { + startContract = service.start(); + }); + + it('provides all required interface methods', () => { + expect(startContract).toEqual({ + isAvailable: expect.any(Function), + isWindowOpen: expect.any(Function), + getThreadId$: expect.any(Function), + getThreadId: expect.any(Function), + openWindow: expect.any(Function), + closeWindow: expect.any(Function), + sendMessage: expect.any(Function), + sendMessageWithWindow: expect.any(Function), + getWindowState: expect.any(Function), + getWindowState$: expect.any(Function), + onWindowOpen: expect.any(Function), + onWindowClose: expect.any(Function), + suggestedActionsService: undefined, + }); + }); + + describe('window state management', () => { + beforeEach(() => { + // Set up implementation for these tests + const setupContract = service.setup(); + setupContract.setImplementation(mockImplementation); + startContract = service.start(); + }); + + it('should delegate to implementation for window state', () => { + expect(startContract.isWindowOpen()).toBe(false); + expect(mockImplementation.isWindowOpen).toHaveBeenCalled(); + }); + + it('should delegate window open/close to implementation', async () => { + await startContract.openWindow(); + expect(mockImplementation.openWindow).toHaveBeenCalled(); + + await startContract.closeWindow(); + expect(mockImplementation.closeWindow).toHaveBeenCalled(); + }); + + it('should provide window state via implementation', () => { + const state = startContract.getWindowState(); + expect(mockImplementation.getWindowState).toHaveBeenCalled(); + expect(state).toEqual({ + isWindowOpen: false, + windowMode: 'sidecar', + paddingSize: 400, + }); + }); + + it('should provide window state observable via implementation', (done) => { + const state$ = startContract.getWindowState$(); + expect(mockImplementation.getWindowState$).toHaveBeenCalled(); + + state$ + .subscribe((state: any) => { + expect(state).toEqual({ + isWindowOpen: false, + windowMode: 'sidecar', + paddingSize: 400, + }); + done(); + }) + .unsubscribe(); + }); + + it('should delegate window callbacks to implementation', () => { + const openCallback = jest.fn(); + const closeCallback = jest.fn(); + + startContract.onWindowOpen(openCallback); + startContract.onWindowClose(closeCallback); + + expect(mockImplementation.onWindowOpen).toHaveBeenCalledWith(openCallback); + expect(mockImplementation.onWindowClose).toHaveBeenCalledWith(closeCallback); + }); + }); + + describe('thread ID management', () => { + beforeEach(() => { + // Set up implementation for these tests + const setupContract = service.setup(); + setupContract.setImplementation(mockImplementation); + startContract = service.start(); + }); + + it('should delegate thread ID to implementation', () => { + const threadId = startContract.getThreadId(); + expect(mockImplementation.getThreadId).toHaveBeenCalled(); + expect(threadId).toBe('test-thread-id'); + }); + + it('should provide thread ID observable via implementation', (done) => { + const threadId$ = startContract.getThreadId$(); + expect(mockImplementation.getThreadId$).toHaveBeenCalled(); + + threadId$ + .subscribe((threadId: string) => { + expect(threadId).toBe('test-thread-id'); + done(); + }) + .unsubscribe(); + }); + }); + + describe('message handling', () => { + it('should return undefined when no implementation or fallback is set', async () => { + // Use fresh service without implementation or fallback + const freshService = new ChatService(); + const freshStartContract = freshService.start(); + + // Should return undefined when no implementation or fallback provided + const result1 = await freshStartContract.sendMessage('test message', []); + expect(result1).toBeUndefined(); + + const result2 = await freshStartContract.sendMessageWithWindow('test message', []); + expect(result2).toBeUndefined(); + }); + + it('should delegate to implementation when set', async () => { + const setupContract = service.setup(); + setupContract.setImplementation(mockImplementation); + const serviceWithImpl = service.start(); + + const testMessages: UserMessage[] = [ + { id: '1', role: 'user', content: 'previous message' }, + ]; + + await serviceWithImpl.sendMessage('test message', testMessages); + expect(mockImplementation.sendMessage).toHaveBeenCalledWith('test message', testMessages); + + await serviceWithImpl.sendMessageWithWindow('test message with window', testMessages, { + clearConversation: true, + }); + expect(mockImplementation.sendMessageWithWindow).toHaveBeenCalledWith( + 'test message with window', + testMessages, + { + clearConversation: true, + } + ); + }); + }); + }); + + describe('delegation without implementation or fallback', () => { + it('should return undefined/default values when no implementation or fallback provided', () => { + const startContract = service.start(); + + // All methods should return undefined or default values when neither implementation nor fallback is available + expect(startContract.getThreadId()).toBeUndefined(); + expect(startContract.getWindowState()).toBeUndefined(); + expect(startContract.isWindowOpen()).toBeUndefined(); + + // onWindow methods return functions (unsubscribe callbacks), not undefined + expect(typeof startContract.onWindowOpen(() => {})).toBe('function'); + expect(typeof startContract.onWindowClose(() => {})).toBe('function'); + + // Observable methods should return undefined + expect(startContract.getThreadId$()).toBeUndefined(); + expect(startContract.getWindowState$()).toBeUndefined(); + }); + + it('should return undefined for async operations when no fallback', async () => { + const startContract = service.start(); + + // Message and window operations should return undefined + const result1 = await startContract.sendMessage('test', []); + expect(result1).toBeUndefined(); + + const result2 = await startContract.sendMessageWithWindow('test', []); + expect(result2).toBeUndefined(); + + const result3 = await startContract.openWindow(); + expect(result3).toBeUndefined(); + + const result4 = await startContract.closeWindow(); + expect(result4).toBeUndefined(); + }); + }); + + describe('delegation with fallback implementation', () => { + let fallbackImplementation: ChatImplementationFunctions; + + beforeEach(() => { + // Create a fallback implementation (simulating plugin-provided fallback) + fallbackImplementation = { + sendMessage: jest.fn().mockResolvedValue({ + observable: null, + userMessage: { id: 'fallback-id', role: 'user', content: 'fallback-content' }, + }), + sendMessageWithWindow: jest.fn().mockResolvedValue({ + observable: null, + userMessage: { id: 'fallback-id', role: 'user', content: 'fallback-content' }, + }), + getThreadId: jest.fn().mockReturnValue('fallback-thread-id'), + getThreadId$: jest.fn().mockReturnValue(mockThreadId$.asObservable()), + isWindowOpen: jest.fn().mockReturnValue(false), + openWindow: jest.fn().mockResolvedValue(undefined), + closeWindow: jest.fn().mockResolvedValue(undefined), + getWindowState: jest.fn().mockReturnValue({ + isWindowOpen: false, + windowMode: 'sidecar', + paddingSize: 400, + }), + getWindowState$: jest.fn().mockReturnValue(mockWindowState$.asObservable()), + onWindowOpen: jest.fn().mockReturnValue(() => {}), + onWindowClose: jest.fn().mockReturnValue(() => {}), + }; + }); + + it('should delegate to fallback when no main implementation', () => { + const setupContract = service.setup(); + setupContract.setFallbackImplementation(fallbackImplementation); + const startContract = service.start(); + + // Should delegate to fallback implementation + expect(startContract.getThreadId()).toBe('fallback-thread-id'); + expect(fallbackImplementation.getThreadId).toHaveBeenCalled(); + + expect(startContract.isWindowOpen()).toBe(false); + expect(fallbackImplementation.isWindowOpen).toHaveBeenCalled(); + }); + + it('should prefer main implementation over fallback', () => { + const setupContract = service.setup(); + setupContract.setFallbackImplementation(fallbackImplementation); + setupContract.setImplementation(mockImplementation); + const startContract = service.start(); + + // Should use main implementation, not fallback + expect(startContract.getThreadId()).toBe('test-thread-id'); + expect(mockImplementation.getThreadId).toHaveBeenCalled(); + expect(fallbackImplementation.getThreadId).not.toHaveBeenCalled(); + }); + }); + }); + + describe('stop', () => { + it('should not throw when called', () => { + expect(() => service.stop()).not.toThrow(); + }); + }); +}); diff --git a/src/core/public/chat/chat_service.ts b/src/core/public/chat/chat_service.ts new file mode 100644 index 000000000000..d4ef587240ed --- /dev/null +++ b/src/core/public/chat/chat_service.ts @@ -0,0 +1,130 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { CoreService } from '../../types'; +import { ChatServiceSetup, ChatServiceStart, ChatImplementationFunctions, Message } from './types'; +import { NoOpChatService } from './no_op_chat_service'; + +/** + * Core chat service implementation + * Contains the actual chat functionality + */ +export class ChatService implements CoreService { + private implementation?: ChatImplementationFunctions; + private serviceStart?: ChatServiceStart; + private suggestedActionsService?: { registerProvider(provider: any): void }; + private noOpService = new NoOpChatService(); + + public setup(): ChatServiceSetup { + return { + setImplementation: (implementation: ChatImplementationFunctions) => { + this.implementation = implementation; + + // Update service methods with implementation if service is already started + if (this.serviceStart) { + this.serviceStart.sendMessage = async (content: string, messages: Message[]) => { + return implementation.sendMessage(content, messages); + }; + this.serviceStart.sendMessageWithWindow = async ( + content: string, + messages: Message[], + options?: { clearConversation?: boolean } + ) => { + return implementation.sendMessageWithWindow(content, messages, options); + }; + } + }, + + setFallbackImplementation: (fallback: ChatImplementationFunctions) => { + this.noOpService.setFallbackImplementation(fallback); + }, + + setSuggestedActionsService: (service: { registerProvider(provider: any): void }) => { + this.suggestedActionsService = service; + + // Update the start interface if it's already been created + if (this.serviceStart) { + this.serviceStart.suggestedActionsService = service; + } + }, + + suggestedActionsService: this.suggestedActionsService, + }; + } + + public start(): ChatServiceStart { + this.serviceStart = { + // Infrastructure method - availability check + isAvailable: () => { + return !!this.implementation; + }, + + // Pure delegation to either implementation or no-op service + getThreadId: () => { + return this.implementation?.getThreadId() ?? this.noOpService.getThreadId(); + }, + getThreadId$: () => { + return this.implementation?.getThreadId$() ?? this.noOpService.getThreadId$(); + }, + + // Message operations + sendMessage: async (content: string, messages: Message[]) => { + return ( + this.implementation?.sendMessage(content, messages) ?? + this.noOpService.sendMessage(content, messages) + ); + }, + sendMessageWithWindow: async ( + content: string, + messages: Message[], + options?: { clearConversation?: boolean } + ) => { + return ( + this.implementation?.sendMessageWithWindow(content, messages, options) ?? + this.noOpService.sendMessageWithWindow(content, messages, options) + ); + }, + + // Window management + isWindowOpen: () => { + return this.implementation?.isWindowOpen() ?? this.noOpService.isWindowOpen(); + }, + openWindow: async () => { + return this.implementation?.openWindow() ?? this.noOpService.openWindow(); + }, + closeWindow: async () => { + return this.implementation?.closeWindow() ?? this.noOpService.closeWindow(); + }, + getWindowState: () => { + return this.implementation?.getWindowState() ?? this.noOpService.getWindowState(); + }, + getWindowState$: () => { + return this.implementation?.getWindowState$() ?? this.noOpService.getWindowState$(); + }, + onWindowOpen: (callback: () => void) => { + return ( + this.implementation?.onWindowOpen(callback) ?? this.noOpService.onWindowOpen(callback) + ); + }, + onWindowClose: (callback: () => void) => { + return ( + this.implementation?.onWindowClose(callback) ?? this.noOpService.onWindowClose(callback) + ); + }, + + // Infrastructure service + suggestedActionsService: this.suggestedActionsService, + }; + + return this.serviceStart!; + } + + public async stop() { + // Reset state + this.implementation = undefined; + this.serviceStart = undefined; + this.suggestedActionsService = undefined; + } +} diff --git a/src/core/public/chat/index.ts b/src/core/public/chat/index.ts new file mode 100644 index 000000000000..21063a1b6a31 --- /dev/null +++ b/src/core/public/chat/index.ts @@ -0,0 +1,24 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export { + ChatServiceInterface, + ChatServiceSetup, + ChatServiceStart, + ChatImplementationFunctions, + Message, + DeveloperMessage, + SystemMessage, + AssistantMessage, + UserMessage, + ToolMessage, + ToolCall, + FunctionCall, + Role, + ChatWindowState, +} from './types'; + +export { ChatService } from './chat_service'; +export { coreChatServiceMock } from './chat_service.mock'; diff --git a/src/core/public/chat/no_op_chat_service.ts b/src/core/public/chat/no_op_chat_service.ts new file mode 100644 index 000000000000..b862e4f41dc4 --- /dev/null +++ b/src/core/public/chat/no_op_chat_service.ts @@ -0,0 +1,81 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { BehaviorSubject, Observable } from 'rxjs'; +import { + ChatServiceStart, + Message, + ChatWindowState, + UserMessage, + ChatImplementationFunctions, +} from './types'; + +/** + * No-op implementation that delegates to plugin-provided fallback + */ +export class NoOpChatService implements ChatServiceStart { + private fallbackImplementation?: ChatImplementationFunctions; + + setFallbackImplementation(fallback: ChatImplementationFunctions): void { + this.fallbackImplementation = fallback; + } + + isAvailable(): boolean { + return false; + } + + isWindowOpen(): boolean { + return this.fallbackImplementation?.isWindowOpen?.()!; + } + + getThreadId$(): Observable { + return this.fallbackImplementation?.getThreadId$?.()!; + } + + getThreadId(): string { + return this.fallbackImplementation?.getThreadId?.()!; + } + + async openWindow(): Promise { + return this.fallbackImplementation?.openWindow?.(); + } + + async closeWindow(): Promise { + return this.fallbackImplementation?.closeWindow?.(); + } + + getWindowState(): ChatWindowState { + return this.fallbackImplementation?.getWindowState?.()!; + } + + async sendMessage( + content: string, + messages: Message[] + ): Promise<{ observable: any; userMessage: UserMessage }> { + return this.fallbackImplementation?.sendMessage?.(content, messages)!; + } + + async sendMessageWithWindow( + content: string, + messages: Message[], + options?: { clearConversation?: boolean } + ): Promise<{ observable: any; userMessage: UserMessage }> { + return this.fallbackImplementation?.sendMessageWithWindow?.(content, messages, options)!; + } + + getWindowState$(): Observable { + return this.fallbackImplementation?.getWindowState$?.()!; + } + + onWindowOpen(callback: () => void): () => void { + return this.fallbackImplementation?.onWindowOpen?.(callback) ?? (() => {}); + } + + onWindowClose(callback: () => void): () => void { + return this.fallbackImplementation?.onWindowClose?.(callback) ?? (() => {}); + } + + suggestedActionsService = undefined; +} diff --git a/src/core/public/chat/types.ts b/src/core/public/chat/types.ts new file mode 100644 index 000000000000..37b000bc8a28 --- /dev/null +++ b/src/core/public/chat/types.ts @@ -0,0 +1,252 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Observable } from 'rxjs'; + +/** + * Function call interface + */ +export interface FunctionCall { + name: string; + arguments: string; +} + +/** + * Tool call interface for ML interactions + */ +export interface ToolCall { + id: string; + type: 'function'; + function: FunctionCall; +} + +/** + * Base message interface + */ +export interface BaseMessage { + id: string; + role: string; + content?: string; + name?: string; +} + +/** + * Developer message type + */ +export interface DeveloperMessage extends BaseMessage { + role: 'developer'; + content: string; +} + +/** + * System message type + */ +export interface SystemMessage extends BaseMessage { + role: 'system'; + content: string; +} + +/** + * Assistant message type with optional tool calls + */ +export interface AssistantMessage extends BaseMessage { + role: 'assistant'; + content?: string; + toolCalls?: ToolCall[]; +} + +/** + * User message type + */ +export interface UserMessage extends BaseMessage { + role: 'user'; + content: string; +} + +/** + * Tool message type for tool execution results + */ +export interface ToolMessage { + id: string; + content: string; + role: 'tool'; + toolCallId: string; + error?: string; +} + +/** + * Discriminated union of all message types + */ +export type Message = + | DeveloperMessage + | SystemMessage + | AssistantMessage + | UserMessage + | ToolMessage; + +/** + * Valid message role types + */ +export type Role = 'developer' | 'system' | 'assistant' | 'user' | 'tool'; + +/** + * Chat window state + */ +export interface ChatWindowState { + isWindowOpen: boolean; + windowMode: 'sidecar' | 'fullscreen'; + paddingSize: number; +} + +/** + * Chat service interface - provides basic chat functionality + * without UI dependencies to avoid circular dependencies + */ +export interface ChatServiceInterface { + /** + * Check if chat window is currently open + */ + isWindowOpen(): boolean; + + /** + * Get the current thread ID as observable + */ + getThreadId$(): Observable; + + /** + * Get the current thread ID + */ + getThreadId(): string; + + /** + * Open the chat window + */ + openWindow(): Promise; + + /** + * Close the chat window + */ + closeWindow(): Promise; + + /** + * Send a message through the chat service + * @param message The message text to send + * @param history Optional message history for context + * @param options Optional configuration for the message + */ + sendMessage( + content: string, + messages: Message[] + ): Promise<{ observable: any; userMessage: UserMessage }>; + + /** + * Send a message and ensure window is open + * This is the primary method used by plugins + */ + sendMessageWithWindow( + content: string, + messages: Message[], + options?: { clearConversation?: boolean } + ): Promise<{ observable: any; userMessage: UserMessage }>; + + /** + * Get current window state + */ + getWindowState(): ChatWindowState; + + /** + * Get window state observable + */ + getWindowState$(): Observable; + + /** + * Register a callback for when window opens + */ + onWindowOpen(callback: () => void): () => void; + + /** + * Register a callback for when window closes + */ + onWindowClose(callback: () => void): () => void; +} + +/** + * Implementation functions provided by the chat plugin + */ +export interface ChatImplementationFunctions { + // Message operations + sendMessage: ( + content: string, + messages: Message[] + ) => Promise<{ observable: any; userMessage: UserMessage }>; + sendMessageWithWindow: ( + content: string, + messages: Message[], + options?: { clearConversation?: boolean } + ) => Promise<{ observable: any; userMessage: UserMessage }>; + + // Thread management + getThreadId: () => string; + getThreadId$: () => Observable; + + // Window management + isWindowOpen: () => boolean; + openWindow: () => Promise; + closeWindow: () => Promise; + getWindowState: () => ChatWindowState; + getWindowState$: () => Observable; + onWindowOpen: (callback: () => void) => () => void; + onWindowClose: (callback: () => void) => () => void; +} + +/** + * Chat service setup interface + */ +export interface ChatServiceSetup { + /** + * Set the implementation functions for chat messaging + * This will be called by the chat plugin + */ + setImplementation(implementation: ChatImplementationFunctions): void; + + /** + * Set the fallback implementation for when chat service is unavailable + * This allows the chat plugin to control "unavailable" behavior + * This will be called by the chat plugin + */ + setFallbackImplementation(fallback: ChatImplementationFunctions): void; + + /** + * Set the suggested actions service + * This will be called by the chat plugin + */ + setSuggestedActionsService(service: { registerProvider(provider: any): void }): void; + + /** + * Suggested actions service for registering providers + * This will be set by the chat plugin and accessed by other plugins + */ + suggestedActionsService?: { + registerProvider(provider: any): void; + }; +} + +/** + * Chat service start interface + */ +export interface ChatServiceStart extends ChatServiceInterface { + /** + * Whether chat service is available + */ + isAvailable(): boolean; + + /** + * Suggested actions service for registering providers + * Available at runtime after chat plugin has registered it + */ + suggestedActionsService?: { + registerProvider(provider: any): void; + }; +} diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index eb43a83e48ff..749396a3a469 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -57,6 +57,7 @@ import { SavedObjectsService } from './saved_objects'; import { UiSettingsService } from './ui_settings'; import { WorkspacesService } from './workspace'; import { KeyboardShortcutService } from './keyboard_shortcut'; +import { ChatService } from './chat'; interface Params { rootDomElement: HTMLElement; @@ -115,6 +116,7 @@ export class CoreSystem { private readonly rootDomElement: HTMLElement; private readonly coreContext: CoreContext; private readonly workspaces: WorkspacesService; + private readonly chat: ChatService; private fatalErrorsSetup: FatalErrorsSetup | null = null; constructor(params: Params) { @@ -144,6 +146,7 @@ export class CoreSystem { this.application = new ApplicationService(); this.integrations = new IntegrationsService(); this.workspaces = new WorkspacesService(); + this.chat = new ChatService(); this.coreContext = { coreId: Symbol('core'), env: injectedMetadata.env }; @@ -172,6 +175,7 @@ export class CoreSystem { const uiSettings = this.uiSettings.setup({ http, injectedMetadata }); const notifications = this.notifications.setup({ uiSettings }); const workspaces = this.workspaces.setup(); + const chat = this.chat.setup(); const pluginDependencies = this.plugins.getOpaqueIds(); const context = this.context.setup({ @@ -193,6 +197,7 @@ export class CoreSystem { uiSettings, workspaces, keyboardShortcut, + chat, }; // Services that do not expose contracts at setup @@ -239,6 +244,9 @@ export class CoreSystem { const workspaces = this.workspaces.start(); const application = await this.application.start({ http, overlays, workspaces }); + // Start chat service - enablement logic is now handled by the plugin + const chat = this.chat.start(); + // Only enable keyboard shortcuts when both the configuration is enabled AND workspaces are enabled const keyboardShortcutsConfigEnabled = injectedMetadata.getKeyboardShortcuts().enabled; const workspacesEnabled = application.capabilities.workspaces.enabled; @@ -273,6 +281,7 @@ export class CoreSystem { savedObjects, uiSettings, workspaces, + chat, })); const core: InternalCoreStart = { @@ -289,6 +298,7 @@ export class CoreSystem { fatalErrors, workspaces, keyboardShortcut: keyboardShortcut || undefined, + chat, }; await this.plugins.start(core); @@ -341,6 +351,7 @@ export class CoreSystem { this.i18n.stop(); this.application.stop(); this.workspaces.stop(); + this.chat.stop(); this.keyboardShortcut.stop(); this.rootDomElement.textContent = ''; } diff --git a/src/core/public/index.ts b/src/core/public/index.ts index bd1212960e25..66922d5d03fc 100644 --- a/src/core/public/index.ts +++ b/src/core/public/index.ts @@ -103,6 +103,7 @@ import { import { Branding } from '../types'; import { WorkspacesStart, WorkspacesSetup } from './workspace'; import { KeyboardShortcutSetup, KeyboardShortcutStart } from './keyboard_shortcut'; +import { ChatServiceSetup, ChatServiceStart } from './chat'; export type { Logos } from '../common'; export { PackageInfo, EnvironmentMode } from '../server/types'; @@ -292,6 +293,8 @@ export interface CoreSetup plugin.startDependencies, workspaces: deps.workspaces, keyboardShortcut: deps.keyboardShortcut, + chat: deps.chat, }; } @@ -179,5 +180,6 @@ export function createPluginStartContext< fatalErrors: deps.fatalErrors, workspaces: deps.workspaces, keyboardShortcut: deps.keyboardShortcut, + chat: deps.chat, }; } diff --git a/src/core/public/plugins/plugins_service.test.ts b/src/core/public/plugins/plugins_service.test.ts index 7cada6029e76..7ffbb5cd2b0e 100644 --- a/src/core/public/plugins/plugins_service.test.ts +++ b/src/core/public/plugins/plugins_service.test.ts @@ -60,6 +60,7 @@ import { savedObjectsServiceMock } from '../saved_objects/saved_objects_service. import { contextServiceMock } from '../context/context_service.mock'; import { workspacesServiceMock } from '../workspace/workspaces_service.mock'; import { keyboardShortcutServiceMock } from '../keyboard_shortcut/keyboard_shortcut_service.mock'; +import { coreChatServiceMock } from '../chat/chat_service.mock'; export let mockPluginInitializers: Map; @@ -113,6 +114,7 @@ describe('PluginsService', () => { uiSettings: uiSettingsServiceMock.createSetupContract(), workspaces: workspacesServiceMock.createSetupContract(), keyboardShortcut: keyboardShortcutServiceMock.createSetup(), + chat: coreChatServiceMock.createSetupContract(), }; mockSetupContext = { ...mockSetupDeps, @@ -134,6 +136,7 @@ describe('PluginsService', () => { fatalErrors: fatalErrorsServiceMock.createStartContract(), workspaces: workspacesServiceMock.createStartContract(), keyboardShortcut: keyboardShortcutServiceMock.createStart(), + chat: coreChatServiceMock.createStartContract(), }; mockStartContext = { ...mockStartDeps, diff --git a/src/plugins/chat/common/chat_capabilities.test.ts b/src/plugins/chat/common/chat_capabilities.test.ts new file mode 100644 index 000000000000..ee576ac1fc39 --- /dev/null +++ b/src/plugins/chat/common/chat_capabilities.test.ts @@ -0,0 +1,106 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + hasInvestigationCapabilities, + isChatEnabled, + ChatConfig, + ContextProviderConfig, +} from './chat_capabilities'; + +describe('Chat Capabilities', () => { + describe('hasInvestigationCapabilities', () => { + it('returns false in open source environment (no investigation capabilities)', () => { + expect(hasInvestigationCapabilities(undefined)).toBe(false); + expect(hasInvestigationCapabilities({})).toBe(false); + }); + + it('returns false in production environment when features are disabled', () => { + const capabilities = { + investigation: { agenticFeaturesEnabled: false }, + }; + expect(hasInvestigationCapabilities(capabilities)).toBe(false); + }); + + it('returns true in production environment when features are enabled', () => { + const capabilities = { + investigation: { agenticFeaturesEnabled: true }, + }; + expect(hasInvestigationCapabilities(capabilities)).toBe(true); + }); + }); + + describe('isChatEnabled', () => { + const contextProviderEnabled: ContextProviderConfig = { enabled: true }; + const contextProviderDisabled: ContextProviderConfig = { enabled: false }; + + describe('when chat plugin is disabled', () => { + const config: ChatConfig = { enabled: false }; + + it('returns false regardless of other settings', () => { + expect(isChatEnabled(config, contextProviderEnabled, undefined)).toBe(false); + expect( + isChatEnabled(config, contextProviderEnabled, { + investigation: { agenticFeaturesEnabled: true }, + }) + ).toBe(false); + }); + }); + + describe('when context provider is disabled', () => { + const config: ChatConfig = { enabled: true, agUiUrl: 'http://example.com' }; + + it('returns false regardless of other settings', () => { + expect(isChatEnabled(config, contextProviderDisabled, undefined)).toBe(false); + expect( + isChatEnabled(config, contextProviderDisabled, { + investigation: { agenticFeaturesEnabled: true }, + }) + ).toBe(false); + }); + }); + + describe('when both chat and context provider are enabled', () => { + describe('open source environment (agUiUrl configured)', () => { + const config: ChatConfig = { enabled: true, agUiUrl: 'http://example.com' }; + + it('returns true regardless of capabilities', () => { + expect(isChatEnabled(config, contextProviderEnabled, undefined)).toBe(true); + expect(isChatEnabled(config, contextProviderEnabled, {})).toBe(true); + expect( + isChatEnabled(config, contextProviderEnabled, { + investigation: { agenticFeaturesEnabled: false }, + }) + ).toBe(true); + expect( + isChatEnabled(config, contextProviderEnabled, { + investigation: { agenticFeaturesEnabled: true }, + }) + ).toBe(true); + }); + }); + + describe('production environment (no agUiUrl)', () => { + const config: ChatConfig = { enabled: true }; + + it('returns false when investigation capabilities are disabled', () => { + const capabilities = { investigation: { agenticFeaturesEnabled: false } }; + expect(isChatEnabled(config, contextProviderEnabled, capabilities)).toBe(false); + }); + + it('returns false when investigation capabilities are undefined', () => { + expect(isChatEnabled(config, contextProviderEnabled, undefined)).toBe(false); + expect(isChatEnabled(config, contextProviderEnabled, {})).toBe(false); + expect(isChatEnabled(config, contextProviderEnabled, { investigation: {} })).toBe(false); + }); + + it('returns true when investigation capabilities are enabled', () => { + const capabilities = { investigation: { agenticFeaturesEnabled: true } }; + expect(isChatEnabled(config, contextProviderEnabled, capabilities)).toBe(true); + }); + }); + }); + }); +}); diff --git a/src/plugins/chat/common/chat_capabilities.ts b/src/plugins/chat/common/chat_capabilities.ts new file mode 100644 index 000000000000..16fa9683b5db --- /dev/null +++ b/src/plugins/chat/common/chat_capabilities.ts @@ -0,0 +1,59 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Chat configuration interface for enablement logic + */ +export interface ChatConfig { + enabled: boolean; + agUiUrl?: string; +} + +/** + * Context provider configuration interface + */ +export interface ContextProviderConfig { + enabled: boolean; +} + +/** + * Check if investigation capabilities are enabled in the given capabilities + * Returns true only when capabilities.investigation.agenticFeaturesEnabled is explicitly true + * Returns false for open source environments or when features are disabled + */ +export function hasInvestigationCapabilities(capabilities?: any): boolean { + return capabilities?.investigation?.agenticFeaturesEnabled === true; +} + +/** + * Unified chat enablement logic that works across all environments + * + * This function provides a single source of truth for determining whether + * chat functionality should be enabled, handling both open source and + * environments with investigation capabilities in a unified way. + * + * @param config Chat plugin configuration (includes enabled flag and agUiUrl) + * @param contextProviderConfig Context provider configuration + * @param capabilities Core application capabilities (may include investigation.agenticFeaturesEnabled) + * @returns true if chat should be enabled, false otherwise + */ +export function isChatEnabled( + config: ChatConfig, + contextProviderConfig: ContextProviderConfig, + capabilities?: any +): boolean { + // Base requirement: both chat plugin and context provider must be enabled + if (!config.enabled || !contextProviderConfig.enabled) { + return false; + } + + // Environment detection: if agUiUrl is configured, this means a setup for agent endpoint for chat + if (config.agUiUrl) { + return true; + } + + // Environment detection: if no agUiUrl config, we allow feature flag to control + return hasInvestigationCapabilities(capabilities); +} diff --git a/src/plugins/chat/common/types.ts b/src/plugins/chat/common/types.ts index 29af1241b992..d7eb4a636e25 100644 --- a/src/plugins/chat/common/types.ts +++ b/src/plugins/chat/common/types.ts @@ -5,6 +5,20 @@ import { z } from 'zod'; +// Import interface types from core - these define the structure +export type { + ToolCall, + FunctionCall, + Message, + DeveloperMessage, + SystemMessage, + AssistantMessage, + UserMessage, + ToolMessage, + Role, +} from '../../../core/public/chat'; + +// Zod schemas for runtime validation - these ensure the core types are followed export const FunctionCallSchema = z.object({ name: z.string(), arguments: z.string(), @@ -91,19 +105,11 @@ export const RunAgentInputSchema = z.object({ export const StateSchema = z.any(); -export type ToolCall = z.infer; -export type FunctionCall = z.infer; -export type DeveloperMessage = z.infer; -export type SystemMessage = z.infer; -export type AssistantMessage = z.infer; -export type UserMessage = z.infer; -export type ToolMessage = z.infer; -export type Message = z.infer; +// Business logic types - stay in plugin export type Context = z.infer; export type Tool = z.infer; export type RunAgentInput = z.infer; export type State = z.infer; -export type Role = z.infer; export class AGUIError extends Error { constructor(message: string) { diff --git a/src/plugins/chat/opensearch_dashboards.json b/src/plugins/chat/opensearch_dashboards.json index 8f2916ee996d..6a2e333f33b0 100644 --- a/src/plugins/chat/opensearch_dashboards.json +++ b/src/plugins/chat/opensearch_dashboards.json @@ -10,5 +10,6 @@ "contextProvider", "charts" ], - "optionalPlugins": [] + "optionalPlugins": ["dataSourceManagement"], + "requiredBundles": ["dataSourceManagement"] } diff --git a/src/plugins/chat/public/actions/graph_timeseries_data_action.test.tsx b/src/plugins/chat/public/actions/graph_timeseries_data_action.test.tsx index e59d2cea3db0..d1f43be3a20d 100644 --- a/src/plugins/chat/public/actions/graph_timeseries_data_action.test.tsx +++ b/src/plugins/chat/public/actions/graph_timeseries_data_action.test.tsx @@ -59,8 +59,41 @@ describe('useGraphTimeseriesDataAction', () => { }), handler: expect.any(Function), render: expect.any(Function), + enabled: true, // Default value when no parameter is passed }); }); + + it('should register action as enabled when explicitly set to true', () => { + const TestComponent = () => { + useGraphTimeseriesDataAction(true); + return
Test
; + }; + + render(); + + expect(mockUseAssistantAction).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'graph_timeseries_data', + enabled: true, + }) + ); + }); + + it('should register action as disabled when explicitly set to false', () => { + const TestComponent = () => { + useGraphTimeseriesDataAction(false); + return
Test
; + }; + + render(); + + expect(mockUseAssistantAction).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'graph_timeseries_data', + enabled: false, + }) + ); + }); }); describe('handler function', () => { diff --git a/src/plugins/chat/public/actions/graph_timeseries_data_action.tsx b/src/plugins/chat/public/actions/graph_timeseries_data_action.tsx index 312b3a651ceb..fc8cde0d67c3 100644 --- a/src/plugins/chat/public/actions/graph_timeseries_data_action.tsx +++ b/src/plugins/chat/public/actions/graph_timeseries_data_action.tsx @@ -39,7 +39,7 @@ interface GraphTimeseriesDataArgs { }; } -export function useGraphTimeseriesDataAction() { +export function useGraphTimeseriesDataAction(enabled: boolean = true) { useAssistantAction({ name: 'graph_timeseries_data', description: 'Create a timeseries graph visualization from provided data', @@ -245,5 +245,6 @@ export function useGraphTimeseriesDataAction() { ); }, + enabled, }); } diff --git a/src/plugins/chat/public/components/chat_header_button.test.tsx b/src/plugins/chat/public/components/chat_header_button.test.tsx index 76ced9dc752b..a24c43c80053 100644 --- a/src/plugins/chat/public/components/chat_header_button.test.tsx +++ b/src/plugins/chat/public/components/chat_header_button.test.tsx @@ -30,10 +30,15 @@ describe('ChatHeaderButton', () => { let mockChatService: jest.Mocked; let mockContextProvider: any; let mockSuggestedActionsService: any; + let mockConfig: any; beforeEach(() => { jest.clearAllMocks(); mockCore = coreMock.createStart(); + + // Make chat available by default (chat service is now included in coreMock.createStart()) + mockCore.chat.isAvailable.mockReturnValue(true); + mockChatService = { sendMessage: jest.fn(), newThread: jest.fn(), @@ -59,6 +64,18 @@ describe('ChatHeaderButton', () => { } as any; mockCore.overlays.sidecar.open = jest.fn().mockReturnValue(mockSidecarRef); mockCore.overlays.sidecar.setSidecarConfig = jest.fn(); + + // Mock capabilities with agentic features enabled by default + mockCore.application.capabilities = { + investigation: { + agenticFeaturesEnabled: true, + }, + } as any; + + // Mock config with enabled = true by default + mockConfig = { + enabled: true, + }; }); describe('ref functionality', () => { @@ -71,6 +88,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} ref={ref} /> ); @@ -89,6 +107,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} ref={ref} /> ); @@ -113,6 +132,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} /> ); @@ -129,6 +149,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} /> ); @@ -142,6 +163,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} /> ); @@ -155,6 +177,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} /> ); @@ -178,6 +201,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} /> ); @@ -209,6 +233,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} /> ); @@ -235,6 +260,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} /> ); @@ -258,6 +284,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} /> ); @@ -279,6 +306,7 @@ describe('ChatHeaderButton', () => { chatService={mockChatService} contextProvider={mockContextProvider} suggestedActionsService={mockSuggestedActionsService} + config={mockConfig} /> ); @@ -291,4 +319,128 @@ describe('ChatHeaderButton', () => { expect(mockClose).toHaveBeenCalled(); }); }); + + describe('feature flag visibility', () => { + it('should render when agenticFeaturesEnabled is true', () => { + mockCore.application.capabilities = { + investigation: { + agenticFeaturesEnabled: true, + }, + } as any; + + const { container } = render( + + ); + + // Should render the button + const button = container.querySelector('[aria-label="Toggle chat assistant"]'); + expect(button).toBeTruthy(); + }); + + it('should not render when chat is not available', () => { + // Make chat unavailable + mockCore.chat.isAvailable.mockReturnValue(false); + + const { container } = render( + + ); + + // Should not render anything + expect(container.firstChild).toBeNull(); + }); + + it('should render when agenticFeaturesEnabled is undefined (open source environment)', () => { + mockCore.application.capabilities = { + investigation: { + agenticFeaturesEnabled: undefined, + }, + } as any; + + const { container } = render( + + ); + + // Should render the button in open source environment + const button = container.querySelector('[aria-label="Toggle chat assistant"]'); + expect(button).toBeTruthy(); + }); + + it('should render when investigation capabilities are missing (open source environment)', () => { + mockCore.application.capabilities = {} as any; + + const { container } = render( + + ); + + // Should render the button in open source environment + const button = container.querySelector('[aria-label="Toggle chat assistant"]'); + expect(button).toBeTruthy(); + }); + + it('should render when capabilities are missing entirely (open source environment)', () => { + mockCore.application.capabilities = undefined as any; + + const { container } = render( + + ); + + // Should render the button in open source environment + const button = container.querySelector('[aria-label="Toggle chat assistant"]'); + expect(button).toBeTruthy(); + }); + + it('should still call hooks but return null when chat is not available', () => { + // This test ensures that all React hooks are called even when chat is not available + mockCore.chat.isAvailable.mockReturnValue(false); + + const { container } = render( + + ); + + // Hooks should still be called (like setChatWindowRef, onWindowStateChange, etc.) + expect(mockChatService.setChatWindowRef).toHaveBeenCalled(); + expect(mockChatService.onWindowStateChange).toHaveBeenCalled(); + expect(mockChatService.onWindowOpenRequest).toHaveBeenCalled(); + expect(mockChatService.onWindowCloseRequest).toHaveBeenCalled(); + + // But no DOM elements should be rendered + expect(container.firstChild).toBeNull(); + }); + }); }); diff --git a/src/plugins/chat/public/components/chat_header_button.tsx b/src/plugins/chat/public/components/chat_header_button.tsx index 6ce803c14b77..88ab6c8c58c9 100644 --- a/src/plugins/chat/public/components/chat_header_button.tsx +++ b/src/plugins/chat/public/components/chat_header_button.tsx @@ -182,6 +182,13 @@ export const ChatHeaderButton = React.forwardRef {/* Text selection monitor - always active when chat UI is rendered */} @@ -210,7 +217,7 @@ export const ChatHeaderButton = React.forwardRef { + onToolsUpdated={(_tools) => { // Tools updated in chat }} > diff --git a/src/plugins/chat/public/components/chat_window.tsx b/src/plugins/chat/public/components/chat_window.tsx index a3c5a1fa1f58..55441caf6985 100644 --- a/src/plugins/chat/public/components/chat_window.tsx +++ b/src/plugins/chat/public/components/chat_window.tsx @@ -14,24 +14,19 @@ import { ContextProviderStart, AssistantActionService, } from '../../../context_provider/public'; -import type { ToolDefinition } from '../../../context_provider/public'; import { // eslint-disable-next-line prettier/prettier type Event as ChatEvent, } from '../../common/events'; import type { Message, - AssistantMessage, UserMessage, - ToolMessage, } from '../../common/types'; import { ChatLayoutMode } from './chat_header_button'; import { ChatContainer } from './chat_container'; import { ChatHeader } from './chat_header'; import { ChatMessages } from './chat_messages'; import { ChatInput } from './chat_input'; -import { ContextTreeView } from './context_tree_view'; -import { useGraphTimeseriesDataAction } from '../actions/graph_timeseries_data_action'; export interface ChatWindowInstance{ startNewChat: ()=>void; @@ -57,7 +52,6 @@ const ChatWindowContent = React.forwardRef( onClose, }, ref) => { const service = AssistantActionService.getInstance(); - const [availableTools, setAvailableTools] = useState([]); const { chatService } = useChatContext(); const { services } = useOpenSearchDashboards<{ core: CoreStart; @@ -68,6 +62,12 @@ const ChatWindowContent = React.forwardRef( const [isStreaming, setIsStreaming] = useState(false); const [currentRunId, setCurrentRunId] = useState(null); const handleSendRef = useRef(); + + const timelineRef = React.useRef(timeline); + + React.useEffect(() => { + timelineRef.current = timeline; + }, [timeline]); // Create the event handler using useMemo const eventHandler = useMemo( @@ -77,21 +77,17 @@ const ChatWindowContent = React.forwardRef( chatService, setTimeline, setIsStreaming, - () => timeline + () => timelineRef.current ), - [service, chatService, timeline] // Only recreate if services change + [service, chatService] ); - // Register actions - useGraphTimeseriesDataAction(); - // Context is now handled by RFC hooks - no need for context manager // The chat service will get context directly from assistantContextStore // Subscribe to tool updates from the service useEffect(() => { const subscription = service.getState$().subscribe((state) => { - setAvailableTools(state.toolDefinitions); if (chatService && state.toolDefinitions.length > 0) { // Store tools for when we send messages (chatService as any).availableTools = state.toolDefinitions; @@ -263,6 +259,7 @@ const ChatWindowContent = React.forwardRef( }; const handleNewChat = useCallback(() => { + console.log('[DEBUG] ChatWindow - handleNewChat called, about to call chatService.newThread()'); chatService.newThread(); setTimeline([]); setCurrentRunId(null); diff --git a/src/plugins/chat/public/plugin.test.ts b/src/plugins/chat/public/plugin.test.ts index e96bc6e6f28b..e718fa3d758c 100644 --- a/src/plugins/chat/public/plugin.test.ts +++ b/src/plugins/chat/public/plugin.test.ts @@ -38,6 +38,11 @@ describe('ChatPlugin', () => { mockCoreStart = { application: { currentAppId$: mockCurrentAppId$, + capabilities: { + investigation: { + agenticFeaturesEnabled: true, + }, + }, }, chrome: { navControls: { @@ -85,8 +90,8 @@ describe('ChatPlugin', () => { it('should initialize chat service when enabled', () => { plugin.start(mockCoreStart, mockDeps); - // ChatService is called without arguments (uses proxy endpoint) - expect(ChatService).toHaveBeenCalledWith(); + // ChatService is called with uiSettings for workspace-aware data source logic + expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings); }); it('should register chat button in header nav controls', () => { @@ -112,35 +117,18 @@ describe('ChatPlugin', () => { const startContract = testPlugin.start(mockCoreStart, mockDeps); - // ChatService should still be created (uses proxy endpoint) - expect(ChatService).toHaveBeenCalledWith(); + // ChatService should still be created with uiSettings + expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings); expect(startContract.chatService).toBeInstanceOf(ChatService); expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).toHaveBeenCalled(); }); - it('should not initialize when plugin is disabled', () => { - mockInitializerContext.config.get = jest.fn().mockReturnValue({ - enabled: false, - agUiUrl: 'http://test-ag-ui:3000', - }); - + it('should always initialize chat service (core service handles enablement)', () => { const startContract = plugin.start(mockCoreStart, mockDeps); - expect(ChatService).not.toHaveBeenCalled(); - expect(startContract.chatService).toBeUndefined(); - expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).not.toHaveBeenCalled(); - }); - - it('should not initialize when enabled is missing (defaults to false)', () => { - mockInitializerContext.config.get = jest.fn().mockReturnValue({ - agUiUrl: 'http://test-ag-ui:3000', - }); - - const startContract = plugin.start(mockCoreStart, mockDeps); - - expect(ChatService).not.toHaveBeenCalled(); - expect(startContract.chatService).toBeUndefined(); - expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).not.toHaveBeenCalled(); + expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings); + expect(startContract.chatService).toBeInstanceOf(ChatService); + expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).toHaveBeenCalled(); }); }); @@ -189,19 +177,15 @@ describe('ChatPlugin', () => { {}, // Missing both enabled and agUiUrl ]; - configs.forEach((config, index) => { + configs.forEach((config) => { jest.clearAllMocks(); mockInitializerContext.config.get = jest.fn().mockReturnValue(config); const testPlugin = new ChatPlugin(mockInitializerContext); expect(() => testPlugin.start(mockCoreStart, mockDeps)).not.toThrow(); - // ChatService is initialized whenever enabled is true (regardless of agUiUrl) - if (config.enabled) { - expect(ChatService).toHaveBeenCalledWith(); - } else { - expect(ChatService).not.toHaveBeenCalled(); - } + // ChatService is always initialized - core service handles enablement logic + expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings); }); }); }); diff --git a/src/plugins/chat/public/plugin.ts b/src/plugins/chat/public/plugin.ts index 9a5fbdc1ea15..e0f1a224f17e 100644 --- a/src/plugins/chat/public/plugin.ts +++ b/src/plugins/chat/public/plugin.ts @@ -7,14 +7,21 @@ import { i18n } from '@osd/i18n'; import React from 'react'; import { EuiText } from '@elastic/eui'; import { debounceTime, distinctUntilChanged, map } from 'rxjs/operators'; -import { Subscription } from 'rxjs'; +import { Subscription, Observable, BehaviorSubject } from 'rxjs'; -import { CoreStart, Plugin, PluginInitializerContext } from '../../../core/public'; +import { + CoreSetup, + CoreStart, + Plugin, + PluginInitializerContext, + ChatImplementationFunctions, +} from '../../../core/public'; import { ChatPluginSetup, ChatPluginStart, AppPluginStartDependencies } from './types'; import { ChatService, ChatWindowState } from './services/chat_service'; import { ChatHeaderButton, ChatLayoutMode } from './components/chat_header_button'; import { toMountPoint } from '../../opensearch_dashboards_react/public'; import { SuggestedActionsService } from './services/suggested_action'; +import { isChatEnabled } from '../common/chat_capabilities'; const isValidChatWindowState = (test: unknown): test is ChatWindowState => { const state = test as ChatWindowState | null; @@ -36,6 +43,7 @@ export class ChatPlugin implements Plugin { private suggestedActionsService = new SuggestedActionsService(); private paddingSizeSubscription?: Subscription; private unsubscribeWindowStateChange?: () => void; + private coreSetup?: CoreSetup; constructor(private initializerContext: PluginInitializerContext) {} @@ -73,28 +81,117 @@ export class ChatPlugin implements Plugin { }); } - public setup(): ChatPluginSetup { + public setup(core: CoreSetup): ChatPluginSetup { + // Store core setup reference for later use + this.coreSetup = core; + + // Set fallback implementation for when chat is disabled + if (core.chat?.setFallbackImplementation) { + const fallbackImplementation: ChatImplementationFunctions = { + // Message operations - return empty results when chat is disabled + sendMessage: async (content: string, messages: any[]) => ({ + observable: null, + userMessage: { id: '', role: 'user' as const, content }, + }), + sendMessageWithWindow: async ( + content: string, + messages: any[], + options?: { clearConversation?: boolean } + ) => ({ + observable: null, + userMessage: { id: '', role: 'user' as const, content }, + }), + + // Thread management - return empty values when disabled + getThreadId: () => '', + getThreadId$: () => new BehaviorSubject('').asObservable(), + + // Window management - return closed/default state when disabled + isWindowOpen: () => false, + openWindow: async () => {}, + closeWindow: async () => {}, + getWindowState: () => ({ + isWindowOpen: false, + windowMode: 'sidecar' as const, + paddingSize: 400, // Plugin-defined business logic default + }), + getWindowState$: () => + new BehaviorSubject({ + isWindowOpen: false, + windowMode: 'sidecar' as const, + paddingSize: 400, // Plugin-defined business logic default + }).asObservable(), + onWindowOpen: (callback: () => void) => () => {}, // Return no-op unsubscribe + onWindowClose: (callback: () => void) => () => {}, // Return no-op unsubscribe + }; + + core.chat.setFallbackImplementation(fallbackImplementation); + } + return { suggestedActionsService: this.suggestedActionsService.setup(), }; } public start(core: CoreStart, deps: AppPluginStartDependencies): ChatPluginStart { - // Get configuration - const config = this.initializerContext.config.get<{ enabled: boolean; agUiUrl?: string }>(); + // Get plugin configuration (same as original implementation) + const chatConfig = this.initializerContext.config.get<{ enabled: boolean; agUiUrl?: string }>(); + const contextProviderConfig = deps.contextProvider ? { enabled: true } : { enabled: false }; - // Check if chat plugin is enabled - if (!config.enabled) { + // Check enablement using the unified logic + const isEnabled = isChatEnabled( + chatConfig, + contextProviderConfig, + core.application.capabilities + ); + + // Always initialize chat service - core service handles enablement via NoOpChatService + this.chatService = new ChatService(core.uiSettings); + + if (!isEnabled) { return { chatService: undefined, }; } - // Initialize chat service (it will use the server proxy) - this.chatService = new ChatService(); + // Register all implementation functions with core chat service + if (this.coreSetup?.chat?.setImplementation) { + this.coreSetup.chat.setImplementation({ + // Message operations + sendMessage: this.chatService.sendMessage.bind(this.chatService), + sendMessageWithWindow: this.chatService.sendMessageWithWindow.bind(this.chatService), - // Store reference to chat service for use in subscription - const chatService = this.chatService; + // Thread management + getThreadId: this.chatService.getThreadId.bind(this.chatService), + getThreadId$: this.chatService.getThreadId$.bind(this.chatService), + + // Window management + isWindowOpen: this.chatService.isWindowOpen.bind(this.chatService), + openWindow: this.chatService.openWindow.bind(this.chatService), + closeWindow: this.chatService.closeWindow.bind(this.chatService), + getWindowState: this.chatService.getWindowState.bind(this.chatService), + getWindowState$: () => { + // Create a mapped observable from the plugin's window state changes + // Since plugin uses onWindowStateChange callback pattern, we need to convert it to observable + return new Observable((subscriber) => { + const unsubscribe = this.chatService?.onWindowStateChange((newState) => { + subscriber.next(newState); + }); + // Emit current state immediately + subscriber.next(this.chatService?.getWindowState()); + return unsubscribe; + }); + }, + onWindowOpen: this.chatService.onWindowOpenRequest.bind(this.chatService), + onWindowClose: this.chatService.onWindowCloseRequest.bind(this.chatService), + }); + } + + // Register suggested actions service with core chat service + if (this.coreSetup?.chat?.setSuggestedActionsService) { + const suggestedActionsServiceStart = this.suggestedActionsService.start(); + this.coreSetup.chat.setSuggestedActionsService(suggestedActionsServiceStart); + } // Register chat button in header with conditional visibility core.chrome.navControls.registerPrimaryHeaderRight({ @@ -106,7 +203,7 @@ export class ChatPlugin implements Plugin { const mountPoint = toMountPoint( React.createElement(ChatHeaderButton, { core, - chatService, + chatService: this.chatService!, contextProvider: deps.contextProvider, charts: deps.charts, suggestedActionsService: this.suggestedActionsService!, @@ -142,7 +239,7 @@ export class ChatPlugin implements Plugin { ), ], action: async ({ content }: { content: string }) => { - await chatService.sendMessageWithWindow(content, [], { clearConversation: true }); + await this.chatService!.sendMessageWithWindow(content, [], { clearConversation: true }); }, }); diff --git a/src/plugins/chat/public/services/ag_ui_agent.ts b/src/plugins/chat/public/services/ag_ui_agent.ts index 99bf49cfcd4b..a6f5ae9c2af0 100644 --- a/src/plugins/chat/public/services/ag_ui_agent.ts +++ b/src/plugins/chat/public/services/ag_ui_agent.ts @@ -24,12 +24,11 @@ export class AgUiAgent { private abortController?: AbortController; private sseBuffer: string = ''; private activeConnection: boolean = false; - constructor(proxyUrl: string = '/api/chat/proxy') { this.proxyUrl = proxyUrl; } - public runAgent(input: RunAgentInput): Observable { + public runAgent(input: RunAgentInput, dataSourceId?: string): Observable { return new Observable((observer) => { // Only abort if we're not in the middle of an active connection // This prevents tool result submissions from breaking the main SSE stream @@ -46,8 +45,12 @@ export class AgUiAgent { // Set active connection flag this.activeConnection = true; - // Make request to OpenSearch Dashboards proxy endpoint - fetch(this.proxyUrl, { + // Build URL with optional dataSourceId query parameter + const url = dataSourceId + ? `${this.proxyUrl}?dataSourceId=${encodeURIComponent(dataSourceId)}` + : this.proxyUrl; + + fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json', diff --git a/src/plugins/chat/public/services/chat_event_handler.test.ts b/src/plugins/chat/public/services/chat_event_handler.test.ts index ab52fc6ef961..06a9555e9888 100644 --- a/src/plugins/chat/public/services/chat_event_handler.test.ts +++ b/src/plugins/chat/public/services/chat_event_handler.test.ts @@ -206,6 +206,23 @@ describe('ChatEventHandler', () => { mockAssistantActionService.executeAction = jest.fn().mockResolvedValue(mockResult); + // Mock sendToolResult to return the expected structure + const mockToolMessage: ToolMessage = { + id: `tool-result-${toolCallId}`, + role: 'tool', + content: JSON.stringify(mockResult), + toolCallId, + }; + + const mockObservable = { + subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }), + }; + + mockChatService.sendToolResult = jest.fn().mockResolvedValue({ + observable: mockObservable, + toolMessage: mockToolMessage, + }); + await chatEventHandler.handleEvent({ type: EventType.TOOL_CALL_START, toolCallId, diff --git a/src/plugins/chat/public/services/chat_event_handler.ts b/src/plugins/chat/public/services/chat_event_handler.ts index 78413928a169..391539c408a9 100644 --- a/src/plugins/chat/public/services/chat_event_handler.ts +++ b/src/plugins/chat/public/services/chat_event_handler.ts @@ -300,16 +300,6 @@ export class ChatEventHandler { result: result.data, }); - // Add tool result message to timeline - const toolMessage: ToolMessage = { - id: `tool-result-${toolCallId}`, - role: 'tool', - content: typeof result.data === 'string' ? result.data : JSON.stringify(result.data), - toolCallId, - }; - - this.onTimelineUpdate((prev) => [...prev, toolMessage]); - // Send tool result back to assistant if chatService is available if (this.chatService && (this.chatService as any).sendToolResult) { await this.sendToolResultToAssistant(toolCallId, result.data); @@ -494,13 +484,16 @@ export class ChatEventHandler { */ private async sendToolResultToAssistant(toolCallId: string, result: any): Promise { try { - const messages = this.getTimeline(); // Now pass timeline directly - no transformation needed - const { observable } = await (this.chatService as any).sendToolResult( + const messages = this.getTimeline(); + + const { observable, toolMessage } = await (this.chatService as any).sendToolResult( toolCallId, result, messages ); + this.onTimelineUpdate((prev) => [...prev, toolMessage]); + // Set streaming state and subscribe to the response stream this.onStreamingStateChange(true); diff --git a/src/plugins/chat/public/services/chat_service.test.ts b/src/plugins/chat/public/services/chat_service.test.ts index 22cfa871e3a2..7641caab8525 100644 --- a/src/plugins/chat/public/services/chat_service.test.ts +++ b/src/plugins/chat/public/services/chat_service.test.ts @@ -186,15 +186,18 @@ describe('ChatService', () => { }); expect(result.observable).toBeDefined(); - expect(mockAgent.runAgent).toHaveBeenCalledWith({ - threadId: expect.stringMatching(/^thread-\d+-[a-z0-9]{9}$/), - runId: expect.stringMatching(/^run-\d+-[a-z0-9]{9}$/), - messages: [result.userMessage], - tools: [], - context: [], - state: {}, - forwardedProps: {}, - }); + expect(mockAgent.runAgent).toHaveBeenCalledWith( + { + threadId: expect.stringMatching(/^thread-\d+-[a-z0-9]{9}$/), + runId: expect.stringMatching(/^run-\d+-[a-z0-9]{9}$/), + messages: [result.userMessage], + tools: [], + context: [], + state: {}, + forwardedProps: {}, + }, + undefined + ); // dataSourceId is undefined when no uiSettings provided }); it('should trim message content', async () => { @@ -228,7 +231,8 @@ describe('ChatService', () => { expect(mockAgent.runAgent).toHaveBeenCalledWith( expect.objectContaining({ tools, - }) + }), + undefined // dataSourceId is undefined when no uiSettings provided ); }); @@ -243,15 +247,18 @@ describe('ChatService', () => { expect(result.userMessage.content).toBe('test'); expect(result.observable).toBeDefined(); - expect(mockAgent.runAgent).toHaveBeenCalledWith({ - threadId: expect.stringMatching(/^thread-\d+-[a-z0-9]{9}$/), - runId: expect.stringMatching(/^run-\d+-[a-z0-9]{9}$/), - messages: [result.userMessage], - tools: [], - context: [], - state: {}, - forwardedProps: {}, - }); + expect(mockAgent.runAgent).toHaveBeenCalledWith( + { + threadId: expect.stringMatching(/^thread-\d+-[a-z0-9]{9}$/), + runId: expect.stringMatching(/^run-\d+-[a-z0-9]{9}$/), + messages: [result.userMessage], + tools: [], + context: [], + state: {}, + forwardedProps: {}, + }, + undefined + ); // dataSourceId is undefined when no uiSettings provided }); }); @@ -287,15 +294,18 @@ describe('ChatService', () => { }); expect(response.observable).toBeDefined(); - expect(mockAgent.runAgent).toHaveBeenCalledWith({ - threadId: expect.stringMatching(/^thread-\d+-[a-z0-9]{9}$/), - runId: expect.stringMatching(/^run-\d+-[a-z0-9]{9}$/), - messages: [response.toolMessage], - tools: [], - context: [], - state: {}, - forwardedProps: {}, - }); + expect(mockAgent.runAgent).toHaveBeenCalledWith( + { + threadId: expect.stringMatching(/^thread-\d+-[a-z0-9]{9}$/), + runId: expect.stringMatching(/^run-\d+-[a-z0-9]{9}$/), + messages: [response.toolMessage], + tools: [], + context: [], + state: {}, + forwardedProps: {}, + }, + undefined + ); // dataSourceId is undefined when no uiSettings provided }); it('should handle string results directly', async () => { diff --git a/src/plugins/chat/public/services/chat_service.ts b/src/plugins/chat/public/services/chat_service.ts index 1fe8cf049072..b373fa830ecc 100644 --- a/src/plugins/chat/public/services/chat_service.ts +++ b/src/plugins/chat/public/services/chat_service.ts @@ -9,6 +9,8 @@ import { RunAgentInput, Message, UserMessage, ToolMessage } from '../../common/t import type { ToolDefinition } from '../../../context_provider/public'; import { ChatLayoutMode } from '../components/chat_header_button'; import type { ChatWindowInstance } from '../components/chat_window'; +import { IUiSettingsClient, UiSettingScope } from '../../../../core/public'; +import { getDefaultDataSourceId, getWorkspaces } from '../../../data_source_management/public'; export interface ChatState { messages: Message[]; @@ -39,6 +41,7 @@ export class ChatService { public events$: any; private activeRequests: Set = new Set(); private requestCounter: number = 0; + private uiSettings?: IUiSettingsClient; // Chat state persistence private readonly STORAGE_KEY = 'chat.currentState'; @@ -59,9 +62,10 @@ export class ChatService { return this.threadId$.getValue(); } - constructor() { + constructor(uiSettings?: IUiSettingsClient) { // No need to pass URL anymore - agent will use the proxy endpoint this.agent = new AgUiAgent(); + this.uiSettings = uiSettings; // Try to restore existing state first const currentChatState = this.loadCurrentChatState(); @@ -98,18 +102,10 @@ export class ChatService { private addActiveRequest(requestId: string): void { this.activeRequests.add(requestId); - // eslint-disable-next-line no-console - console.log( - `📊 [ChatService] Active requests: ${this.activeRequests.size} (added: ${requestId})` - ); } private removeActiveRequest(requestId: string): void { this.activeRequests.delete(requestId); - // eslint-disable-next-line no-console - console.log( - `📊 [ChatService] Active requests: ${this.activeRequests.size} (removed: ${requestId})` - ); } // Window state management public API @@ -255,6 +251,44 @@ export class ChatService { return result; } + /** + * Get workspace-aware data source ID + * Determines the correct data source based on current workspace context + */ + private async getWorkspaceAwareDataSourceId(): Promise { + try { + if (!this.uiSettings) { + // eslint-disable-next-line no-console + console.warn('UI Settings not available, using default data source'); + return undefined; + } + + // Get workspace context + const workspaces = getWorkspaces(); + if (!workspaces) { + // eslint-disable-next-line no-console + console.warn('Workspaces service not available, using global scope'); + return undefined; + } + + const currentWorkspaceId = workspaces.currentWorkspaceId$.getValue(); + + // Determine scope based on workspace context + const scope: UiSettingScope = !!currentWorkspaceId + ? UiSettingScope.WORKSPACE + : UiSettingScope.GLOBAL; + + // Get default data source with proper scope + const dataSourceId = await getDefaultDataSourceId(this.uiSettings, scope); + + return dataSourceId || undefined; + } catch (error) { + // eslint-disable-next-line no-console + console.warn('Failed to determine workspace-aware data source, proceeding without:', error); + return undefined; // Graceful fallback - undefined means local cluster + } + } + public async sendMessage( content: string, messages: Message[] @@ -271,6 +305,9 @@ export class ChatService { content: content.trim(), }; + // Get workspace-aware data source ID + const dataSourceId = await this.getWorkspaceAwareDataSourceId(); + // Get all contexts from the assistant context store (static + dynamic) const contextStore = (window as any).assistantContextStore; const allContexts = contextStore ? contextStore.getAllContexts() : []; @@ -291,7 +328,7 @@ export class ChatService { forwardedProps: {}, }; - const observable = this.agent.runAgent(runInput); + const observable = this.agent.runAgent(runInput, dataSourceId); // Wrap observable to track completion const trackedObservable = new Observable((subscriber: any) => { @@ -333,6 +370,9 @@ export class ChatService { toolCallId, }; + // Get workspace-aware data source ID + const dataSourceId = await this.getWorkspaceAwareDataSourceId(); + // Get all contexts from the assistant context store (static + dynamic) const contextStore = (window as any).assistantContextStore; const allContexts = contextStore ? contextStore.getAllContexts() : []; @@ -357,7 +397,7 @@ export class ChatService { }; // Continue the conversation with the tool result - const observable = this.agent.runAgent(runInput); + const observable = this.agent.runAgent(runInput, dataSourceId); // Wrap observable to track completion const trackedObservable = new Observable((subscriber: any) => { @@ -437,7 +477,9 @@ export class ChatService { private clearDynamicContextFromStore(): void { const contextStore = (window as any).assistantContextStore; - if (!contextStore) return; + if (!contextStore) { + return; + } // Get all contexts with IDs (dynamic contexts) and remove them const allContexts = contextStore.getAllContexts(); @@ -449,7 +491,10 @@ export class ChatService { } public newThread(): void { - this.threadId$.next(this.generateThreadId()); + const oldThreadId = this.threadId; + const newThreadId = this.generateThreadId(); + + this.threadId$.next(newThreadId); this.currentMessages = []; this.clearCurrentChatState(); diff --git a/src/plugins/chat/public/types.ts b/src/plugins/chat/public/types.ts index 9d100d2ff22e..8ac185535432 100644 --- a/src/plugins/chat/public/types.ts +++ b/src/plugins/chat/public/types.ts @@ -6,6 +6,7 @@ import { NavigationPublicPluginStart } from '../../navigation/public'; import { ContextProviderStart } from '../../context_provider/public'; import { ChartsPluginStart } from '../../charts/public'; +import { DataSourceManagementPluginSetup } from '../../data_source_management/public'; import { ChatService } from './services/chat_service'; import { SuggestedActionsServiceSetupContract } from './services/suggested_action'; @@ -21,4 +22,5 @@ export interface AppPluginStartDependencies { navigation: NavigationPublicPluginStart; contextProvider: ContextProviderStart; charts: ChartsPluginStart; + dataSourceManagement?: DataSourceManagementPluginSetup; } diff --git a/src/plugins/chat/server/index.ts b/src/plugins/chat/server/index.ts index 830b0d612b65..77bca5ff274e 100644 --- a/src/plugins/chat/server/index.ts +++ b/src/plugins/chat/server/index.ts @@ -13,6 +13,7 @@ import { configSchema, ChatConfigType } from './config'; export const config: PluginConfigDescriptor = { schema: configSchema, exposeToBrowser: { + enabled: true, agUiUrl: true, }, }; diff --git a/src/plugins/chat/server/plugin.ts b/src/plugins/chat/server/plugin.ts index 441c177b48d7..d0322df59167 100644 --- a/src/plugins/chat/server/plugin.ts +++ b/src/plugins/chat/server/plugin.ts @@ -12,6 +12,7 @@ import { Plugin, Logger, OpenSearchDashboardsRequest, + Capabilities, } from '../../../core/server'; import { ChatPluginSetup, ChatPluginStart } from './types'; diff --git a/src/plugins/chat/server/routes/index.test.ts b/src/plugins/chat/server/routes/index.test.ts index b3b6d68df7f4..15eac1b00594 100644 --- a/src/plugins/chat/server/routes/index.test.ts +++ b/src/plugins/chat/server/routes/index.test.ts @@ -4,7 +4,6 @@ */ import supertest from 'supertest'; -import { Readable } from 'stream'; import { setupServer } from '../../../../core/server/test_utils'; import { loggingSystemMock } from '../../../../core/server/mocks'; import { defineRoutes } from './index'; @@ -16,13 +15,18 @@ describe('Chat Proxy Routes', () => { let server: any; let mockFetch: jest.MockedFunction; let mockLogger: any; + let mockCapabilitiesResolver: jest.Mock; - const testSetup = async (agUiUrl?: string) => { + const testSetup = async ( + agUiUrl?: string, + getCapabilitiesResolver?: () => ((request: any) => Promise) | undefined, + mlCommonsAgentId?: string + ) => { const { server: testServer, httpSetup } = await setupServer(); const router = httpSetup.createRouter(''); mockLogger = loggingSystemMock.create().get(); - defineRoutes(router, mockLogger, agUiUrl); + defineRoutes(router, mockLogger, agUiUrl, getCapabilitiesResolver, mlCommonsAgentId); // Mock dynamicConfigService required by server.start() const dynamicConfigService = { @@ -37,6 +41,14 @@ describe('Chat Proxy Routes', () => { beforeEach(() => { mockFetch = fetch as jest.MockedFunction; + + // Mock capabilities resolver + mockCapabilitiesResolver = jest.fn().mockResolvedValue({ + investigation: { + agenticFeaturesEnabled: false, // Default to false + }, + }); + jest.clearAllMocks(); }); @@ -156,7 +168,7 @@ describe('Chat Proxy Routes', () => { expect(response.body.message).toBe('Network connection failed'); expect(mockLogger.error).toHaveBeenCalledWith( - expect.stringContaining('Error proxying request to AG-UI') + expect.stringContaining('AI agent routing error') ); }); @@ -235,5 +247,187 @@ describe('Chat Proxy Routes', () => { // Verify the stream was attempted to be read expect(mockReader.read).toHaveBeenCalled(); }); + + describe('Generic ML Integration', () => { + it('should fallback to AG-UI when agenticFeaturesEnabled is true but ML context is not available', async () => { + // Enable agentic features but no ML context available + mockCapabilitiesResolver.mockResolvedValue({ + investigation: { + agenticFeaturesEnabled: true, + }, + }); + + // Mock successful AG-UI response + mockFetch.mockResolvedValue({ + ok: true, + status: 200, + body: { + getReader: () => ({ + read: jest.fn().mockResolvedValue({ done: true, value: undefined }), + }), + }, + } as any); + + const httpSetup = await testSetup( + 'http://test-agui:3000', + () => mockCapabilitiesResolver, + 'test-agent-id' + ); + + await supertest(httpSetup.server.listener) + .post('/api/chat/proxy') + .send(validRequest) + .expect(200); + + // Verify capabilities were checked + expect(mockCapabilitiesResolver).toHaveBeenCalled(); + + // Verify AG-UI was called as fallback + expect(mockFetch).toHaveBeenCalledWith('http://test-agui:3000', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'text/event-stream', + }, + body: JSON.stringify(validRequest), + }); + }); + + it('should fallback to AG-UI when agenticFeaturesEnabled is true but ML client is disabled', async () => { + // Enable agentic features but disable ML client + mockCapabilitiesResolver.mockResolvedValue({ + investigation: { + agenticFeaturesEnabled: true, + }, + }); + // ML client disabled via missing context ML client + + // Mock successful AG-UI response + mockFetch.mockResolvedValue({ + ok: true, + status: 200, + body: { + getReader: () => ({ + read: jest.fn().mockResolvedValue({ done: true, value: undefined }), + }), + }, + } as any); + + const httpSetup = await testSetup( + 'http://test-agui:3000', + () => mockCapabilitiesResolver, + 'test-agent-id' + ); + + await supertest(httpSetup.server.listener) + .post('/api/chat/proxy') + .send(validRequest) + .expect(200); + + // Verify capabilities were checked + expect(mockCapabilitiesResolver).toHaveBeenCalled(); + + // Verify AG-UI was called as fallback + expect(mockFetch).toHaveBeenCalledWith('http://test-agui:3000', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'text/event-stream', + }, + body: JSON.stringify(validRequest), + }); + }); + + it('should fallback to AG-UI when agenticFeaturesEnabled is false', async () => { + // Disable agentic features + mockCapabilitiesResolver.mockResolvedValue({ + investigation: { + agenticFeaturesEnabled: false, + }, + }); + + // Mock successful AG-UI response + mockFetch.mockResolvedValue({ + ok: true, + status: 200, + body: { + getReader: () => ({ + read: jest.fn().mockResolvedValue({ done: true, value: undefined }), + }), + }, + } as any); + + const httpSetup = await testSetup( + 'http://test-agui:3000', + () => mockCapabilitiesResolver, + 'test-agent-id' + ); + + await supertest(httpSetup.server.listener) + .post('/api/chat/proxy') + .send(validRequest) + .expect(200); + + // Verify capabilities were checked + expect(mockCapabilitiesResolver).toHaveBeenCalled(); + + // Verify AG-UI was called + expect(mockFetch).toHaveBeenCalled(); + }); + + it('should return 503 when ML Commons agent ID is not configured', async () => { + // Enable agentic features + mockCapabilitiesResolver.mockResolvedValue({ + investigation: { + agenticFeaturesEnabled: true, + }, + }); + + const httpSetup = await testSetup( + undefined, // No AG-UI URL + () => mockCapabilitiesResolver, + undefined // No ML Commons agent ID + ); + + const response = await supertest(httpSetup.server.listener) + .post('/api/chat/proxy') + .send(validRequest) + .expect(503); + + expect(response.body.message).toContain( + 'No AI agent available: ML Commons agent not enabled and AG-UI URL not configured' + ); + + // Verify AG-UI was not called since ML router handled the error + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('should fallback to AG-UI when capabilities resolver is not available', async () => { + // Mock successful AG-UI response + mockFetch.mockResolvedValue({ + ok: true, + status: 200, + body: { + getReader: () => ({ + read: jest.fn().mockResolvedValue({ done: true, value: undefined }), + }), + }, + } as any); + + const httpSetup = await testSetup( + 'http://test-agui:3000', + undefined, // No capabilities resolver + 'test-agent-id' + ); + + await supertest(httpSetup.server.listener) + .post('/api/chat/proxy') + .send(validRequest) + .expect(200); + + // Verify AG-UI was called as fallback + expect(mockFetch).toHaveBeenCalled(); + }); + }); }); }); diff --git a/src/plugins/chat/server/routes/index.ts b/src/plugins/chat/server/routes/index.ts index 0d7e4111cbab..0d1b0dea5878 100644 --- a/src/plugins/chat/server/routes/index.ts +++ b/src/plugins/chat/server/routes/index.ts @@ -11,7 +11,72 @@ import { OpenSearchDashboardsRequest, Capabilities, } from '../../../../core/server'; -import { forwardToMLCommonsAgent } from './ml_routes/ml_commons_agent'; +import { MLAgentRouterFactory } from './ml_routes/ml_agent_router'; +import { MLAgentRouterRegistry } from './ml_routes/router_registry'; + +/** + * Forward request to external AG-UI server + */ +async function forwardToAgUI( + agUiUrl: string, + request: OpenSearchDashboardsRequest, + response: any, + dataSourceId?: string, + logger?: Logger +) { + // Prepare request body - include dataSourceId if provided + const requestBody = dataSourceId ? { ...(request.body || {}), dataSourceId } : request.body; + + logger?.debug('Forwarding to external AG-UI', { agUiUrl, dataSourceId }); + + // Forward the request to AG-UI server using native fetch (Node 18+) + const agUiResponse = await fetch(agUiUrl, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'text/event-stream', + }, + body: JSON.stringify(requestBody), + }); + + if (!agUiResponse.ok) { + return response.customError({ + statusCode: agUiResponse.status, + body: { + message: `AG-UI server error: ${agUiResponse.statusText}`, + }, + }); + } + + // Convert Web ReadableStream to Node.js Readable stream + const reader = agUiResponse.body!.getReader(); + const stream = new Readable({ + async read() { + try { + const { done, value } = await reader.read(); + if (done) { + this.push(null); // Signal end of stream + } else { + this.push(Buffer.from(value)); // Push as Buffer for binary mode + } + } catch (error) { + this.destroy(error as Error); + } + }, + }); + + return response.ok({ + headers: { + 'Content-Type': 'text/event-stream', + 'Content-Encoding': 'identity', // Prevents compression buffering + 'Cache-Control': 'no-cache', + Connection: 'keep-alive', + 'Transfer-Encoding': 'chunked', // Enables HTTP chunked transfer + 'X-Accel-Buffering': 'no', // Disables nginx buffering + }, + body: stream, + }); +} export function defineRoutes( router: IRouter, @@ -36,28 +101,40 @@ export function defineRoutes( state: schema.maybe(schema.any()), forwardedProps: schema.maybe(schema.any()), }), + query: schema.maybe( + schema.object({ + dataSourceId: schema.maybe(schema.string()), + }) + ), }, }, async (context, request, response) => { + const dataSourceId = request.query?.dataSourceId; + try { // Check if ML Commons agentic features are enabled via capabilities const capabilitiesResolver = getCapabilitiesResolver?.(); - if (capabilitiesResolver) { - const capabilities = await capabilitiesResolver(request); + const capabilities = capabilitiesResolver ? await capabilitiesResolver(request) : undefined; + + // Initialize ML agent routers based on current capabilities (if not already done) + // This ensures routers are registered based on actual runtime capabilities + MLAgentRouterRegistry.initialize(capabilities); + + // Get the registered ML agent router (if any) + const mlRouter = MLAgentRouterFactory.getRouter(); - if (capabilities?.investigation?.agenticFeaturesEnabled === true) { - logger.debug('Routing to ML Commons agent proxy'); - return await forwardToMLCommonsAgent( - context, - request, - response, - logger, - mlCommonsAgentId - ); - } + if (mlRouter) { + logger.info(`Routing to ML Commons agent via ${mlRouter.getRouterName()}`); + return await mlRouter.forward( + context, + request, + response, + logger, + mlCommonsAgentId, + dataSourceId + ); } - // Fallback to external AG-UI if (!agUiUrl) { return response.customError({ statusCode: 503, @@ -68,70 +145,10 @@ export function defineRoutes( }); } - logger.debug('Routing to external AG-UI'); - } catch (error) { - logger.error(`Error checking capabilities or routing: ${error}`); - // If capabilities check fails, fallback to external AG-UI - if (!agUiUrl) { - return response.customError({ - statusCode: 500, - body: { - message: error instanceof Error ? error.message : 'Unknown error occurred', - }, - }); - } - } - - try { - // Forward the request to AG-UI server using native fetch (Node 18+) - const agUiResponse = await fetch(agUiUrl, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - Accept: 'text/event-stream', - }, - body: JSON.stringify(request.body), - }); - - if (!agUiResponse.ok) { - return response.customError({ - statusCode: agUiResponse.status, - body: { - message: `AG-UI server error: ${agUiResponse.statusText}`, - }, - }); - } - - // Convert Web ReadableStream to Node.js Readable stream - const reader = agUiResponse.body!.getReader(); - const stream = new Readable({ - async read() { - try { - const { done, value } = await reader.read(); - if (done) { - this.push(null); // Signal end of stream - } else { - this.push(Buffer.from(value)); // Push as Buffer for binary mode - } - } catch (error) { - this.destroy(error as Error); - } - }, - }); - - return response.ok({ - headers: { - 'Content-Type': 'text/event-stream', - 'Content-Encoding': 'identity', // Prevents compression buffering - 'Cache-Control': 'no-cache', - Connection: 'keep-alive', - 'Transfer-Encoding': 'chunked', // Enables HTTP chunked transfer - 'X-Accel-Buffering': 'no', // Disables nginx buffering - }, - body: stream, - }); + // Forward to AG-UI capable endpoint. This is the default router. + return await forwardToAgUI(agUiUrl, request, response, dataSourceId, logger); } catch (error) { - logger.error(`Error proxying request to AG-UI: ${error}`); + logger.error(`AI agent routing error: ${error}`); return response.customError({ statusCode: 500, body: { diff --git a/src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts b/src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts new file mode 100644 index 000000000000..24f3f19402a3 --- /dev/null +++ b/src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts @@ -0,0 +1,176 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + Logger, + RequestHandlerContext, + OpenSearchDashboardsRequest, + IOpenSearchDashboardsResponse, + OpenSearchDashboardsResponseFactory, + Capabilities, +} from '../../../../../core/server'; +import { MLAgentRouter } from './ml_agent_router'; + +/** + * Generic ML client detector with caching for performance + * Finds any client in the request context that has a request() method + */ +let cachedClientKey: string | null = null; +let detectionAttempted = false; + +function findMLClient(context: RequestHandlerContext) { + // Use cached result if available + if (detectionAttempted) { + if (cachedClientKey) { + const contextItem = (context as any)[cachedClientKey]; + return contextItem?.client; + } + return undefined; + } + + // First-time detection - cache the result + for (const key of Object.keys(context)) { + const contextItem = (context as any)[key]; + if (contextItem?.client?.request && typeof contextItem.client.request === 'function') { + cachedClientKey = key; // Cache the key name + detectionAttempted = true; + return contextItem.client; + } + } + + detectionAttempted = true; // Mark as attempted even if not found + return undefined; +} + +// ============================================================================ +// GENERIC ML CLIENT TYPES - To avoid compile-time dependencies +// ============================================================================ + +interface MLStreamResponse { + status: number; + statusText: string; + headers: Record; + body: NodeJS.ReadableStream; +} + +interface MLBufferedResponse { + status: number; + statusText: string; + headers: Record; + body: string; +} + +type MLClientResponse = MLStreamResponse | MLBufferedResponse; + +/** + * Type guard to check if response is streaming + */ +function isStreamResponse(response: MLClientResponse): response is MLStreamResponse { + return response && typeof response.body === 'object' && 'pipe' in response.body; +} + +/** + * Generic ML Commons agent router + * Uses generic ML client detection to communicate with ML Commons agents + * Works with any ML client provider that has a request() method + */ +export class GenericMLRouter implements MLAgentRouter { + async forward( + context: RequestHandlerContext, + request: OpenSearchDashboardsRequest, + response: OpenSearchDashboardsResponseFactory, + logger: Logger, + configuredAgentId?: string, + dataSourceId?: string + ): Promise> { + if (!configuredAgentId) { + return response.customError({ + statusCode: 503, + body: { message: 'ML Commons agent ID not configured' }, + }); + } + + // Validate request body + if (!request.body || typeof request.body !== 'object') { + return response.customError({ + statusCode: 400, + body: { message: 'Invalid request body for ML Commons agent' }, + }); + } + + const mlClient = findMLClient(context); + if (!mlClient) { + return response.customError({ + statusCode: 503, + body: { message: 'ML client not available in request context' }, + }); + } + + try { + logger.info('Forwarding request to ML Commons agent', { + agentId: configuredAgentId, + dataSourceId, + }); + + // Use detected ML client from request context + const mlResponse: MLClientResponse = await mlClient.request( + { + method: 'POST', + path: `/_plugins/_ml/agents/${configuredAgentId}/_execute/stream`, + body: JSON.stringify(request.body), + datasourceId: dataSourceId, // Use actual dataSourceId from request + stream: true, + timeout: 300000, + }, + request, + context + ); + + // Handle streaming response properly using type guard + if (isStreamResponse(mlResponse)) { + return response.custom({ + statusCode: mlResponse.status, + headers: { + 'Content-Type': 'text/event-stream', + 'Content-Encoding': 'identity', + Connection: 'keep-alive', + }, + body: mlResponse.body, + }); + } else { + return response.custom({ + statusCode: mlResponse.status, + headers: { + 'Content-Type': 'application/json', + ...mlResponse.headers, + }, + body: typeof mlResponse.body === 'string' ? JSON.parse(mlResponse.body) : mlResponse.body, + }); + } + } catch (error) { + logger.error(`Error forwarding to ML Commons agent: ${error}`); + + if (error instanceof Error && error.message.includes('404')) { + return response.customError({ + statusCode: 404, + body: { message: `ML Commons agent "${configuredAgentId}" not found` }, + }); + } + + return response.customError({ + statusCode: 500, + body: { + message: `ML Commons agent error: ${ + error instanceof Error ? error.message : 'Unknown error' + }`, + }, + }); + } + } + + getRouterName(): string { + return 'GenericMLRouter'; + } +} diff --git a/src/plugins/chat/server/routes/ml_routes/ml_agent_router.ts b/src/plugins/chat/server/routes/ml_routes/ml_agent_router.ts new file mode 100644 index 000000000000..ea029693ee03 --- /dev/null +++ b/src/plugins/chat/server/routes/ml_routes/ml_agent_router.ts @@ -0,0 +1,74 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + Logger, + RequestHandlerContext, + OpenSearchDashboardsRequest, + IOpenSearchDashboardsResponse, + OpenSearchDashboardsResponseFactory, +} from '../../../../../core/server'; + +/** + * Abstract interface for ML agent routing strategies + * This allows different environments to implement their own ML agent communication + * while keeping the route handler logic identical + */ +export interface MLAgentRouter { + /** + * Forward the request to the appropriate ML agent implementation + * @param context Request handler context + * @param request OpenSearch Dashboards request + * @param response Response factory + * @param logger Logger instance + * @param configuredAgentId ML Commons agent ID + * @param dataSourceId Optional data source ID for multi-cluster setups + * @returns Promise resolving to the response + */ + forward( + context: RequestHandlerContext, + request: OpenSearchDashboardsRequest, + response: OpenSearchDashboardsResponseFactory, + logger: Logger, + configuredAgentId?: string, + dataSourceId?: string + ): Promise>; + + /** + * Get a descriptive name for this router (for logging) + */ + getRouterName(): string; +} + +/** + * Factory for managing the ML agent router + * Simplified to handle a single router since only one is ever registered + */ +export class MLAgentRouterFactory { + private static router: MLAgentRouter | undefined; + + /** + * Register a router implementation + * @param router The router to register + */ + static registerRouter(router: MLAgentRouter): void { + this.router = router; + } + + /** + * Get the registered router + * @returns The registered router or undefined if none available + */ + static getRouter(): MLAgentRouter | undefined { + return this.router; + } + + /** + * Clear the registered router (primarily for testing) + */ + static clearRouters(): void { + this.router = undefined; + } +} diff --git a/src/plugins/chat/server/routes/ml_routes/ml_commons_agent.test.ts b/src/plugins/chat/server/routes/ml_routes/ml_commons_agent.test.ts deleted file mode 100644 index 72226ef7d028..000000000000 --- a/src/plugins/chat/server/routes/ml_routes/ml_commons_agent.test.ts +++ /dev/null @@ -1,441 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { forwardToMLCommonsAgent } from './ml_commons_agent'; -import { loggingSystemMock } from '../../../../../core/server/mocks'; - -describe('forwardToMLCommonsAgent', () => { - let mockContext: any; - let mockRequest: any; - let mockResponse: any; - let mockLogger: any; - - beforeEach(() => { - mockLogger = loggingSystemMock.create().get(); - - mockContext = { - core: { - opensearch: { - client: { - asCurrentUser: { - transport: { - request: jest.fn(), - }, - }, - }, - }, - }, - }; - - mockRequest = { - body: { - threadId: 'thread-123', - runId: 'run-456', - messages: [{ role: 'user', content: 'Hello' }], - tools: [], - context: [], - state: {}, - forwardedProps: {}, - }, - }; - - mockResponse = { - ok: jest.fn(), - customError: jest.fn(), - }; - - jest.clearAllMocks(); - }); - - describe('successful requests', () => { - it('should successfully forward request to ML Commons agent and return response', async () => { - const mockMLResponse = { - body: { - id: 'response-123', - message: 'ML Commons response', - status: 'completed', - }, - }; - - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockResolvedValue( - mockMLResponse - ); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - // Verify OpenSearch client was called correctly - expect( - mockContext.core.opensearch.client.asCurrentUser.transport.request - ).toHaveBeenCalledWith({ - method: 'POST', - path: '/_plugins/_ml/agents/test-agent-id/_execute', - body: mockRequest.body, - }); - - // Verify successful response - expect(mockResponse.ok).toHaveBeenCalledWith({ - headers: { - 'Content-Type': 'application/json', - }, - body: mockMLResponse.body, - }); - - // Verify logging - expect(mockLogger.debug).toHaveBeenCalledWith('Forwarding request to ML Commons agent', { - agentId: 'test-agent-id', - }); - - expect(mockResponse.customError).not.toHaveBeenCalled(); - }); - - it('should handle different request body structures', async () => { - const differentRequestBody = { - threadId: 'different-thread', - runId: 'different-run', - messages: [ - { role: 'user', content: 'What is the weather?' }, - { role: 'assistant', content: 'I can help with that.' }, - ], - tools: [{ name: 'weather-tool' }], - context: [{ type: 'location', value: 'Seattle' }], - state: { temperature: 'celsius' }, - forwardedProps: { userId: 'user123' }, - }; - - mockRequest.body = differentRequestBody; - - const mockMLResponse = { body: { result: 'success' } }; - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockResolvedValue( - mockMLResponse - ); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - expect( - mockContext.core.opensearch.client.asCurrentUser.transport.request - ).toHaveBeenCalledWith({ - method: 'POST', - path: '/_plugins/_ml/agents/test-agent-id/_execute', - body: differentRequestBody, - }); - }); - }); - - describe('configuration handling', () => { - it('should return 503 error when agent ID is not configured', async () => { - await forwardToMLCommonsAgent(mockContext, mockRequest, mockResponse, mockLogger); - - expect(mockResponse.customError).toHaveBeenCalledWith({ - statusCode: 503, - body: { - message: 'ML Commons agent ID not configured', - }, - }); - - // Verify OpenSearch client was not called - expect( - mockContext.core.opensearch.client.asCurrentUser.transport.request - ).not.toHaveBeenCalled(); - expect(mockResponse.ok).not.toHaveBeenCalled(); - }); - - it('should use custom configured agent ID', async () => { - const customAgentId = 'custom-production-agent-123'; - const mockMLResponse = { - body: { - id: 'response-123', - message: 'Custom agent response', - status: 'completed', - }, - }; - - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockResolvedValue( - mockMLResponse - ); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - customAgentId - ); - - // Verify OpenSearch client was called with custom agent ID - expect( - mockContext.core.opensearch.client.asCurrentUser.transport.request - ).toHaveBeenCalledWith({ - method: 'POST', - path: `/_plugins/_ml/agents/${customAgentId}/_execute`, - body: mockRequest.body, - }); - - // Verify logging includes custom agent ID - expect(mockLogger.debug).toHaveBeenCalledWith('Forwarding request to ML Commons agent', { - agentId: customAgentId, - }); - - // Verify successful response - expect(mockResponse.ok).toHaveBeenCalledWith({ - headers: { - 'Content-Type': 'application/json', - }, - body: mockMLResponse.body, - }); - }); - }); - - describe('error handling', () => { - it('should handle 404 agent not found error', async () => { - const notFoundError = new Error('404 Not Found - Agent not found'); - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockRejectedValue( - notFoundError - ); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - expect(mockResponse.customError).toHaveBeenCalledWith({ - statusCode: 404, - body: { - message: 'ML Commons agent "test-agent-id" not found', - }, - }); - - expect(mockLogger.error).toHaveBeenCalledWith( - expect.stringContaining('Error forwarding to ML Commons agent') - ); - - expect(mockResponse.ok).not.toHaveBeenCalled(); - }); - - it('should handle general ML Commons errors', async () => { - const generalError = new Error('Internal server error from ML Commons'); - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockRejectedValue( - generalError - ); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - expect(mockResponse.customError).toHaveBeenCalledWith({ - statusCode: 500, - body: { - message: 'ML Commons agent error: Internal server error from ML Commons', - }, - }); - - expect(mockLogger.error).toHaveBeenCalledWith( - expect.stringContaining('Error forwarding to ML Commons agent') - ); - - expect(mockResponse.ok).not.toHaveBeenCalled(); - }); - - it('should handle non-Error exceptions', async () => { - const nonErrorException = 'String error instead of Error object'; - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockRejectedValue( - nonErrorException - ); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - expect(mockResponse.customError).toHaveBeenCalledWith({ - statusCode: 500, - body: { - message: 'ML Commons agent error: Unknown error', - }, - }); - - expect(mockLogger.error).toHaveBeenCalledWith( - expect.stringContaining('Error forwarding to ML Commons agent') - ); - }); - - it('should handle different 404 error message formats', async () => { - const variations = [ - 'Response Error: 404', - 'StatusCodeError: 404 - Not Found', - 'ML Commons agent 404 response', - 'Error: Request failed with status 404', - ]; - - for (const errorMessage of variations) { - jest.clearAllMocks(); - const error = new Error(errorMessage); - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockRejectedValue(error); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - expect(mockResponse.customError).toHaveBeenCalledWith({ - statusCode: 404, - body: { - message: 'ML Commons agent "test-agent-id" not found', - }, - }); - } - }); - }); - - describe('logging behavior', () => { - it('should log request initiation with agent ID', async () => { - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockResolvedValue({ - body: {}, - }); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - expect(mockLogger.debug).toHaveBeenCalledWith('Forwarding request to ML Commons agent', { - agentId: 'test-agent-id', - }); - }); - - it('should log errors with full error details', async () => { - const testError = new Error('Test error message'); - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockRejectedValue( - testError - ); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - expect(mockLogger.error).toHaveBeenCalledWith( - `Error forwarding to ML Commons agent: ${testError}` - ); - }); - }); - - describe('response format', () => { - it('should return proper JSON response headers', async () => { - const mockMLResponse = { body: { test: 'data' } }; - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockResolvedValue( - mockMLResponse - ); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - expect(mockResponse.ok).toHaveBeenCalledWith({ - headers: { - 'Content-Type': 'application/json', - }, - body: mockMLResponse.body, - }); - }); - - it('should forward the exact ML Commons response body', async () => { - const complexResponseBody = { - threadId: 'thread-123', - runId: 'run-456', - response: { - content: 'AI generated response', - metadata: { - model: 'gpt-3.5-turbo', - tokens: 150, - }, - }, - status: 'completed', - timestamp: '2023-10-01T12:00:00Z', - }; - - const mockMLResponse = { - body: complexResponseBody, - }; - - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockResolvedValue( - mockMLResponse - ); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - expect(mockResponse.ok).toHaveBeenCalledWith({ - headers: { - 'Content-Type': 'application/json', - }, - body: complexResponseBody, - }); - }); - }); - - describe('agent configuration', () => { - it('should use the configured agent ID', async () => { - mockContext.core.opensearch.client.asCurrentUser.transport.request.mockResolvedValue({ - body: {}, - }); - - await forwardToMLCommonsAgent( - mockContext, - mockRequest, - mockResponse, - mockLogger, - 'test-agent-id' - ); - - expect( - mockContext.core.opensearch.client.asCurrentUser.transport.request - ).toHaveBeenCalledWith( - expect.objectContaining({ - path: '/_plugins/_ml/agents/test-agent-id/_execute', - }) - ); - }); - }); -}); diff --git a/src/plugins/chat/server/routes/ml_routes/ml_commons_agent.ts b/src/plugins/chat/server/routes/ml_routes/ml_commons_agent.ts deleted file mode 100644 index f6ec6fd9cc6f..000000000000 --- a/src/plugins/chat/server/routes/ml_routes/ml_commons_agent.ts +++ /dev/null @@ -1,73 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { - Logger, - RequestHandlerContext, - OpenSearchDashboardsRequest, - OpenSearchDashboardsResponse, -} from '../../../../../core/server'; - -/** - * Forward request to ML Commons agent proxy - */ -export async function forwardToMLCommonsAgent( - context: RequestHandlerContext, - request: OpenSearchDashboardsRequest, - response: OpenSearchDashboardsResponse, - logger: Logger, - configuredAgentId?: string -) { - if (!configuredAgentId) { - return response.customError({ - statusCode: 503, - body: { - message: 'ML Commons agent ID not configured', - }, - }); - } - - try { - logger.debug('Forwarding request to ML Commons agent', { agentId: configuredAgentId }); - - // Make request to ML Commons agent execute API - const mlResponse = await context.core.opensearch.client.asCurrentUser.transport.request({ - method: 'POST', - path: `/_plugins/_ml/agents/${configuredAgentId}/_execute`, - body: request.body, // Forward the RunAgentInput directly - }); - - // TODO: Implement Server-Sent Events streaming for ML Commons response - // ML Commons agent should return streaming response compatible with AG-UI protocol - // For now, return the raw response - this may need to be adapted based on ML Commons output format - return response.ok({ - headers: { - 'Content-Type': 'application/json', - }, - body: mlResponse.body, - }); - } catch (error) { - logger.error(`Error forwarding to ML Commons agent: ${error}`); - - // Check if it's a 404 (agent not found) vs other errors - if (error instanceof Error && error.message.includes('404')) { - return response.customError({ - statusCode: 404, - body: { - message: `ML Commons agent "${configuredAgentId}" not found`, - }, - }); - } - - return response.customError({ - statusCode: 500, - body: { - message: `ML Commons agent error: ${ - error instanceof Error ? error.message : 'Unknown error' - }`, - }, - }); - } -} diff --git a/src/plugins/chat/server/routes/ml_routes/router_registry.ts b/src/plugins/chat/server/routes/ml_routes/router_registry.ts new file mode 100644 index 000000000000..a86384d1e384 --- /dev/null +++ b/src/plugins/chat/server/routes/ml_routes/router_registry.ts @@ -0,0 +1,50 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Capabilities } from '../../../../../core/server'; +import { MLAgentRouterFactory } from './ml_agent_router'; +import { hasInvestigationCapabilities } from '../../../common/chat_capabilities'; +import { GenericMLRouter } from './generic_ml_router'; + +/** + * Registry for ML agent routers + * This handles the initialization and registration of available routers + * based on the current environment capabilities + */ +export class MLAgentRouterRegistry { + private static initialized = false; + + /** + * Initialize and register ML agent routers based on environment capabilities + * This should be called once during plugin setup with current capabilities + */ + static initialize(capabilities?: Capabilities): void { + if (this.initialized) { + return; + } + + // Environment detection: register router based on capabilities + if (hasInvestigationCapabilities(capabilities)) { + MLAgentRouterFactory.registerRouter(new GenericMLRouter()); + } + + this.initialized = true; + } + + /** + * Reset the registry (primarily for testing) + */ + static reset(): void { + MLAgentRouterFactory.clearRouters(); + this.initialized = false; + } + + /** + * Check if the registry has been initialized + */ + static isInitialized(): boolean { + return this.initialized; + } +} diff --git a/src/plugins/data_source_management/public/index.ts b/src/plugins/data_source_management/public/index.ts index fe29891200af..546eba4e63c9 100644 --- a/src/plugins/data_source_management/public/index.ts +++ b/src/plugins/data_source_management/public/index.ts @@ -26,6 +26,6 @@ export { createDataSourceMenu, } from './components/data_source_menu'; export { DataSourceSelectionService } from './service/data_source_selection_service'; -export { getDefaultDataSourceId, getDefaultDataSourceId$ } from './components/utils'; +export { getDefaultDataSourceId, getDefaultDataSourceId$, getWorkspaces } from './components/utils'; export { DATACONNECTIONS_BASE, DatasourceTypeToDisplayName } from './constants'; export { DEFAULT_DATA_SOURCE_UI_SETTINGS_ID } from '../common'; diff --git a/src/plugins/explore/opensearch_dashboards.json b/src/plugins/explore/opensearch_dashboards.json index 42e5f56ad7e4..74d57b0b93ad 100644 --- a/src/plugins/explore/opensearch_dashboards.json +++ b/src/plugins/explore/opensearch_dashboards.json @@ -21,6 +21,6 @@ "dashboard" ], "requiredBundles": ["opensearchDashboardsReact", "opensearchDashboardsUtils", "dashboard"], - "optionalPlugins": ["home", "share", "chat", "contextProvider", "datasetManagement"], + "optionalPlugins": ["home", "share", "contextProvider", "datasetManagement"], "configPath": ["explore"] } diff --git a/src/plugins/explore/public/actions/ask_ai_action.test.ts b/src/plugins/explore/public/actions/ask_ai_action.test.ts index 624d83bd3889..89caf340d6f6 100644 --- a/src/plugins/explore/public/actions/ask_ai_action.test.ts +++ b/src/plugins/explore/public/actions/ask_ai_action.test.ts @@ -4,7 +4,7 @@ */ import { createAskAiAction } from './ask_ai_action'; -import { ChatService } from '../../../chat/public'; +import { ChatServiceStart } from '../../../../../core/public'; // Mock the AskAIActionItem component jest.mock('../components/ask_ai_action_item', () => ({ @@ -12,20 +12,26 @@ jest.mock('../components/ask_ai_action_item', () => ({ })); describe('createAskAiAction', () => { - let mockChatService: jest.Mocked; + let mockChatService: jest.Mocked; beforeEach(() => { jest.clearAllMocks(); - // Create a minimal mock of ChatService + // Create a minimal mock of ChatServiceStart mockChatService = { + isAvailable: jest.fn().mockReturnValue(true), + isWindowOpen: jest.fn().mockReturnValue(false), + sendMessageWithWindow: jest.fn().mockResolvedValue(undefined), + getThreadId$: jest.fn(), + getThreadId: jest.fn(), + openWindow: jest.fn(), + closeWindow: jest.fn(), sendMessage: jest.fn(), - sendMessageWithWindow: jest.fn(), - newThread: jest.fn(), - abort: jest.fn(), - resetConnection: jest.fn(), - availableTools: [], - } as any; + getWindowState$: jest.fn(), + onWindowOpen: jest.fn(), + onWindowClose: jest.fn(), + suggestedActionsService: undefined, + }; }); describe('action configuration', () => { @@ -47,24 +53,23 @@ describe('createAskAiAction', () => { query: 'test query', }; - it('should return true when chatService is provided', () => { + it('should return true when chatService is available', () => { const action = createAskAiAction(mockChatService); expect(action.isCompatible(mockContext)).toBe(true); }); - it('should return false when chatService is undefined', () => { - const action = createAskAiAction(undefined); - expect(action.isCompatible(mockContext)).toBe(false); - }); - - it('should return false when chatService is null', () => { - const action = createAskAiAction(null as any); + it('should return false when chatService is not available', () => { + const unavailableChatService = { + ...mockChatService, + isAvailable: jest.fn().mockReturnValue(false), + }; + const action = createAskAiAction(unavailableChatService); expect(action.isCompatible(mockContext)).toBe(false); }); }); describe('chatService availability', () => { - it('should create valid action with chatService', () => { + it('should create valid action with available chatService', () => { const action = createAskAiAction(mockChatService); const mockContext = { document: {} }; @@ -77,8 +82,12 @@ describe('createAskAiAction', () => { }); }); - it('should create action that is incompatible when chatService is undefined', () => { - const action = createAskAiAction(undefined); + it('should create action that is incompatible when chatService is not available', () => { + const unavailableChatService = { + ...mockChatService, + isAvailable: jest.fn().mockReturnValue(false), + }; + const action = createAskAiAction(unavailableChatService); const mockContext = { document: {} }; expect(action.isCompatible(mockContext)).toBe(false); diff --git a/src/plugins/explore/public/actions/ask_ai_action.ts b/src/plugins/explore/public/actions/ask_ai_action.ts index 678aabe16a1b..670ca491850d 100644 --- a/src/plugins/explore/public/actions/ask_ai_action.ts +++ b/src/plugins/explore/public/actions/ask_ai_action.ts @@ -5,12 +5,12 @@ import { LogActionDefinition } from '../types/log_actions'; import { AskAIActionItem } from '../components/ask_ai_action_item'; -import { ChatService } from '../../../chat/public'; +import { ChatServiceStart } from '../../../../core/public'; /** * Creates the Ask AI action that uses the AskAIActionItem component */ -export function createAskAiAction(chatService: ChatService | undefined): LogActionDefinition { +export function createAskAiAction(chatService: ChatServiceStart): LogActionDefinition { return { id: 'ask_ai', displayName: 'Ask AI', @@ -18,13 +18,12 @@ export function createAskAiAction(chatService: ChatService | undefined): LogActi order: 1, isCompatible: () => { - // Only show if chat service is available (chat plugin is enabled) - return !!chatService; + return chatService.isAvailable(); }, component: (props) => { - // chatService is guaranteed to be defined here because isCompatible checks it - return AskAIActionItem({ ...props, chatService: chatService! }); + // chatService is always defined since we're using core service + return AskAIActionItem({ ...props, chatService }); }, }; } diff --git a/src/plugins/explore/public/build_services.ts b/src/plugins/explore/public/build_services.ts index f128f53e89c1..0ecd26cdf548 100644 --- a/src/plugins/explore/public/build_services.ts +++ b/src/plugins/explore/public/build_services.ts @@ -25,8 +25,8 @@ export function buildServices( tabRegistry: TabRegistryService, visualizationRegistry: VisualizationRegistryService, queryPanelActionsRegistry: QueryPanelActionsRegistryService, - isDatasetManagementEnabled: boolean, - slotRegistry: SlotRegistryService + isDatasetManagementEnabled: boolean = false, + slotRegistry?: SlotRegistryService ): ExploreServices { const config = context.config.get(); const supportedTypes = config.supportedTypes; diff --git a/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.test.tsx b/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.test.tsx index a4996447f5bb..b7bc6e82d8b9 100644 --- a/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.test.tsx +++ b/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.test.tsx @@ -6,7 +6,7 @@ import React from 'react'; import { render, fireEvent, waitFor, screen } from '@testing-library/react'; import { AskAIActionItem } from './ask_ai_action_item'; -import { ChatService } from '../../../../chat/public'; +import { ChatServiceStart } from '../../../../../../core/public'; import { LogActionContext } from '../../types/log_actions'; // Mock dependencies @@ -23,7 +23,7 @@ jest.mock('../../../../opensearch_dashboards_react/public', () => ({ })); describe('AskAIActionItem', () => { - let mockChatService: jest.Mocked; + let mockChatService: jest.Mocked; let mockOnClose: jest.Mock; let mockOnResult: jest.Mock; let mockContext: LogActionContext; @@ -32,16 +32,19 @@ describe('AskAIActionItem', () => { jest.clearAllMocks(); mockChatService = { - sendMessageWithWindow: jest.fn().mockResolvedValue({ - observable: {}, - userMessage: { id: 'test', role: 'user', content: 'test' }, - }), + isAvailable: jest.fn().mockReturnValue(true), + sendMessageWithWindow: jest.fn().mockResolvedValue(undefined), isWindowOpen: jest.fn().mockReturnValue(false), - newThread: jest.fn(), - abort: jest.fn(), - resetConnection: jest.fn(), - availableTools: [], - } as any; + getThreadId$: jest.fn(), + getThreadId: jest.fn(), + openWindow: jest.fn(), + closeWindow: jest.fn(), + sendMessage: jest.fn(), + getWindowState$: jest.fn(), + onWindowOpen: jest.fn(), + onWindowClose: jest.fn(), + suggestedActionsService: undefined, + }; mockOnClose = jest.fn(); mockOnResult = jest.fn(); diff --git a/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.tsx b/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.tsx index fe08bcbadc2b..a71ab14367da 100644 --- a/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.tsx +++ b/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.tsx @@ -15,13 +15,13 @@ import { import { useOpenSearchDashboards } from '../../../../opensearch_dashboards_react/public'; import { ExploreServices } from '../../types'; import { LogActionItemProps } from '../../types/log_actions'; -import { ChatService } from '../../../../chat/public'; +import { ChatServiceStart } from '../../../../../core/public'; // Create stable NOOP hook reference outside component to avoid re-renders const NOOP_DYNAMIC_CONTEXT_HOOK = (_options: any, _shouldCleanup?: boolean): string => ''; interface AskAIActionItemProps extends LogActionItemProps { - chatService: ChatService; + chatService: ChatServiceStart; } export const AskAIActionItem: React.FC = ({ @@ -64,7 +64,8 @@ export const AskAIActionItem: React.FC = ({ const useDynamicContext = services.contextProvider?.hooks?.useDynamicContext || NOOP_DYNAMIC_CONTEXT_HOOK; - const dynamicContextResult = useDynamicContext(contextData, false); + // Register context with cleanup on unmount (true = cleanup when component unmounts) + useDynamicContext(contextData, false); const handleExecute = useCallback(async () => { if (!userInput.trim()) { @@ -77,26 +78,9 @@ export const AskAIActionItem: React.FC = ({ setIsLoading(true); try { - // Create user message to include in conversation history (fix Issue 2) - const userMessage = { - id: `msg-${Date.now()}`, - role: 'user' as const, - content: userInput.trim(), - }; - - // Check if chat window is open and handle accordingly - const isOpen = chatService.isWindowOpen(); - - if (isOpen) { - // Chat is open - send message to existing conversation - // Use sendMessageWithWindow to leverage ChatWindow delegation even when open - await chatService.sendMessageWithWindow(userInput.trim(), [userMessage]); - } else { - // Chat is closed - open it and send message (will start new conversation) - await chatService.sendMessageWithWindow(userInput.trim(), [userMessage], { - clearConversation: true, - }); - } + // Always use sendMessageWithWindow since it works in both open and closed states + // It will handle opening the window if needed, and properly delegate when window is open + const result = await chatService.sendMessageWithWindow(userInput.trim(), []); onResult?.({ success: true, diff --git a/src/plugins/explore/public/components/tabs/action_bar/action_bar.tsx b/src/plugins/explore/public/components/tabs/action_bar/action_bar.tsx index 7ad71338af9e..2d34bd192c4b 100644 --- a/src/plugins/explore/public/components/tabs/action_bar/action_bar.tsx +++ b/src/plugins/explore/public/components/tabs/action_bar/action_bar.tsx @@ -4,6 +4,7 @@ */ import React, { memo, useMemo } from 'react'; import { useObservable } from 'react-use'; +import { of } from 'rxjs'; import { DiscoverResultsActionBar } from './results_action_bar/results_action_bar'; import { ExploreServices } from '../../../types'; import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public'; @@ -27,7 +28,7 @@ const ActionBarComponent = () => { const savedSearch = useSelector(selectSavedSearch); const sortedSlotItems$ = useMemo(() => { - return slotRegistry.getSortedItems$('resultsActionBar'); + return slotRegistry?.getSortedItems$('resultsActionBar') ?? of([]); }, [slotRegistry]); const slotItems = useObservable(sortedSlotItems$, []); diff --git a/src/plugins/explore/public/plugin.ts b/src/plugins/explore/public/plugin.ts index 68b89a2549c7..251340944752 100644 --- a/src/plugins/explore/public/plugin.ts +++ b/src/plugins/explore/public/plugin.ts @@ -512,11 +512,9 @@ export class ExplorePlugin this.initializeServices(); // Register Log Actions - // Register Ask AI action if chat service is available - if (plugins.chat?.chatService) { - const askAiAction = createAskAiAction(plugins.chat.chatService); - logActionRegistry.registerAction(askAiAction); - } + // Always register Ask AI action - let isCompatible handle enablement logic + const askAiAction = createAskAiAction(core.chat); + logActionRegistry.registerAction(askAiAction); const savedExploreLoader = createSavedExploreLoader({ savedObjectsClient: core.savedObjects.client, diff --git a/src/plugins/explore/public/types.ts b/src/plugins/explore/public/types.ts index f9598cf39703..9c0837a5005c 100644 --- a/src/plugins/explore/public/types.ts +++ b/src/plugins/explore/public/types.ts @@ -55,7 +55,6 @@ import { QueryPanelActionsRegistryServiceSetup, } from './services/query_panel_actions_registry'; import { SlotRegistryService, SlotRegistryServiceStart } from './services/slot_registry'; -import { ChatPluginStart } from '../../chat/public'; // ============================================================================ // PLUGIN INTERFACES - What Explore provides to other plugins @@ -124,7 +123,6 @@ export interface ExploreStartDependencies { expressions: ExpressionsStart; dashboard: DashboardStart; contextProvider?: ContextProviderStart; - chat?: ChatPluginStart; } // ============================================================================ @@ -185,7 +183,7 @@ export interface ExploreServices { tabRegistry: TabRegistryService; visualizationRegistry: VisualizationRegistryService; queryPanelActionsRegistry: QueryPanelActionsRegistryService; - slotRegistry: SlotRegistryService; + slotRegistry?: SlotRegistryService; expressions: ExpressionsStart; contextProvider?: ContextProviderStart; diff --git a/yarn.lock b/yarn.lock index 3bff78eea93c..8ba2dc8ca25d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -25516,7 +25516,7 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0": version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -25534,6 +25534,15 @@ string-width@^1.0.1: is-fullwidth-code-point "^1.0.0" strip-ansi "^3.0.0" +"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: + version "4.2.3" + resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" + integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== + dependencies: + emoji-regex "^8.0.0" + is-fullwidth-code-point "^3.0.0" + strip-ansi "^6.0.1" + string-width@^2.1.0, string-width@^2.1.1: version "2.1.1" resolved "https://registry.yarnpkg.com/string-width/-/string-width-2.1.1.tgz#ab93f27a8dc13d28cac815c462143a6d9012ae9e" @@ -25701,7 +25710,7 @@ stringify-entities@^3.0.1: character-entities-legacy "^1.0.0" xtend "^4.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1": version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -25743,6 +25752,13 @@ strip-ansi@^5.1.0, strip-ansi@^5.2.0: dependencies: ansi-regex "^4.1.0" +strip-ansi@^6.0.0, strip-ansi@^6.0.1: + version "6.0.1" + resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" + integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== + dependencies: + ansi-regex "^5.0.1" + strip-ansi@^7.0.1: version "7.1.0" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45" @@ -28488,7 +28504,7 @@ workerpool@^6.5.1: resolved "https://registry.yarnpkg.com/workerpool/-/workerpool-6.5.1.tgz#060f73b39d0caf97c6db64da004cd01b4c099544" integrity sha512-Fs4dNYcsdpYSAfVxhnl1L5zTksjvOJxtC5hzMNl+1t9B8hTJTdKDyZ5ju7ztgPy+ft9tBFXoOlDNiOT9WUXZlA== -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -28514,6 +28530,15 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" +wrap-ansi@^7.0.0: + version "7.0.0" + resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" + integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== + dependencies: + ansi-styles "^4.0.0" + string-width "^4.1.0" + strip-ansi "^6.0.0" + wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From 57979c7047b6f840d5530a43b9bc716d78c7a38f Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Sun, 30 Nov 2025 23:43:47 -0800 Subject: [PATCH 2/4] fix PR comment and add more clean up Signed-off-by: Anan Zhuang --- src/core/public/chat/chat_service.mock.ts | 3 + src/core/public/chat/chat_service.test.ts | 384 ++++-------- src/core/public/chat/chat_service.ts | 181 +++--- src/core/public/chat/no_op_chat_service.ts | 81 --- src/core/public/chat/types.ts | 85 +-- .../public/components/chat_header_button.tsx | 40 +- .../chat/public/components/chat_window.tsx | 34 +- .../graph_visualization/error_boundary.tsx | 401 ------------- .../graph_visualization/error_display.tsx | 545 ------------------ .../graph_visualization/error_handler.ts | 519 ----------------- .../graph_visualization/validation.ts | 4 - src/plugins/chat/public/plugin.test.ts | 12 +- src/plugins/chat/public/plugin.ts | 79 +-- .../services/chat_context_manager.test.ts | 275 --------- .../public/services/chat_context_manager.ts | 86 --- .../chat/public/services/chat_service.test.ts | 213 ++++++- .../chat/public/services/chat_service.ts | 186 +++--- 17 files changed, 570 insertions(+), 2558 deletions(-) delete mode 100644 src/core/public/chat/no_op_chat_service.ts delete mode 100644 src/plugins/chat/public/components/graph_visualization/error_boundary.tsx delete mode 100644 src/plugins/chat/public/components/graph_visualization/error_display.tsx delete mode 100644 src/plugins/chat/public/components/graph_visualization/error_handler.ts delete mode 100644 src/plugins/chat/public/components/graph_visualization/validation.ts delete mode 100644 src/plugins/chat/public/services/chat_context_manager.test.ts delete mode 100644 src/plugins/chat/public/services/chat_context_manager.ts diff --git a/src/core/public/chat/chat_service.mock.ts b/src/core/public/chat/chat_service.mock.ts index 6e79709e278f..9ba7f448d3d8 100644 --- a/src/core/public/chat/chat_service.mock.ts +++ b/src/core/public/chat/chat_service.mock.ts @@ -17,6 +17,8 @@ const createStartContractMock = (): jest.Mocked => ({ isWindowOpen: jest.fn().mockReturnValue(false), getThreadId$: jest.fn().mockReturnValue(new BehaviorSubject('')), getThreadId: jest.fn().mockReturnValue(''), + setThreadId: jest.fn(), + newThread: jest.fn(), openWindow: jest.fn(), closeWindow: jest.fn(), getWindowState: jest.fn().mockReturnValue({ @@ -24,6 +26,7 @@ const createStartContractMock = (): jest.Mocked => ({ windowMode: 'sidecar', paddingSize: 400, }), + setWindowState: jest.fn(), sendMessage: jest.fn().mockResolvedValue({ observable: null, userMessage: { id: 'mock-id', role: 'user', content: 'mock-content' }, diff --git a/src/core/public/chat/chat_service.test.ts b/src/core/public/chat/chat_service.test.ts index 648bb5bcbc8b..24b693b050b8 100644 --- a/src/core/public/chat/chat_service.test.ts +++ b/src/core/public/chat/chat_service.test.ts @@ -3,26 +3,17 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { BehaviorSubject } from 'rxjs'; import { ChatService } from './chat_service'; -import { ChatImplementationFunctions, ChatWindowState, UserMessage } from './types'; +import { ChatImplementationFunctions } from './types'; describe('ChatService', () => { let service: ChatService; let mockImplementation: ChatImplementationFunctions; - let mockThreadId$: BehaviorSubject; - let mockWindowState$: BehaviorSubject; beforeEach(() => { service = new ChatService(); - mockThreadId$ = new BehaviorSubject('test-thread-id'); - mockWindowState$ = new BehaviorSubject({ - isWindowOpen: false, - windowMode: 'sidecar', - paddingSize: 400, - }); - // Create complete mock implementation + // Create simplified mock implementation (only business logic operations) mockImplementation = { sendMessage: jest.fn().mockResolvedValue({ observable: null, @@ -32,303 +23,172 @@ describe('ChatService', () => { observable: null, userMessage: { id: '1', role: 'user', content: 'test' }, }), - getThreadId: jest.fn().mockReturnValue('test-thread-id'), - getThreadId$: jest.fn().mockReturnValue(mockThreadId$.asObservable()), - isWindowOpen: jest.fn().mockReturnValue(false), openWindow: jest.fn().mockResolvedValue(undefined), closeWindow: jest.fn().mockResolvedValue(undefined), - getWindowState: jest.fn().mockReturnValue({ - isWindowOpen: false, - windowMode: 'sidecar', - paddingSize: 400, - }), - getWindowState$: jest.fn().mockReturnValue(mockWindowState$.asObservable()), - onWindowOpen: jest.fn().mockReturnValue(() => {}), - onWindowClose: jest.fn().mockReturnValue(() => {}), }; }); describe('setup', () => { - it('returns valid setup contract', () => { + it('should return a setup contract', () => { const setupContract = service.setup(); - expect(setupContract).toEqual({ - setImplementation: expect.any(Function), - setFallbackImplementation: expect.any(Function), - setSuggestedActionsService: expect.any(Function), - suggestedActionsService: undefined, - }); + expect(setupContract).toHaveProperty('setImplementation'); + expect(setupContract).toHaveProperty('setSuggestedActionsService'); }); - it('setImplementation stores the implementation functions', () => { + it('should allow setting implementation', () => { const setupContract = service.setup(); - expect(() => setupContract.setImplementation(mockImplementation)).not.toThrow(); + expect(() => { + setupContract.setImplementation(mockImplementation); + }).not.toThrow(); }); }); describe('start', () => { - describe('availability logic', () => { - it('should be unavailable when no implementation is registered', () => { - const startContract = service.start(); - expect(startContract.isAvailable()).toBe(false); - }); + it('should return a start contract', () => { + const startContract = service.start(); + + expect(startContract).toHaveProperty('isAvailable'); + expect(startContract).toHaveProperty('getThreadId'); + expect(startContract).toHaveProperty('getThreadId$'); + expect(startContract).toHaveProperty('setThreadId'); + expect(startContract).toHaveProperty('newThread'); + expect(startContract).toHaveProperty('isWindowOpen'); + expect(startContract).toHaveProperty('getWindowState'); + expect(startContract).toHaveProperty('getWindowState$'); + expect(startContract).toHaveProperty('setWindowState'); + expect(startContract).toHaveProperty('onWindowOpen'); + expect(startContract).toHaveProperty('onWindowClose'); + expect(startContract).toHaveProperty('openWindow'); + expect(startContract).toHaveProperty('closeWindow'); + expect(startContract).toHaveProperty('sendMessage'); + expect(startContract).toHaveProperty('sendMessageWithWindow'); + }); - it('should be available after implementation is registered', () => { - const setupContract = service.setup(); - setupContract.setImplementation(mockImplementation); - const startContract = service.start(); - expect(startContract.isAvailable()).toBe(true); - }); + it('should not be available without implementation', () => { + const startContract = service.start(); + expect(startContract.isAvailable()).toBe(false); }); - describe('service interface', () => { - let startContract: any; + it('should be available with implementation', () => { + const setupContract = service.setup(); + const startContract = service.start(); - beforeEach(() => { - startContract = service.start(); - }); + setupContract.setImplementation(mockImplementation); + expect(startContract.isAvailable()).toBe(true); + }); - it('provides all required interface methods', () => { - expect(startContract).toEqual({ - isAvailable: expect.any(Function), - isWindowOpen: expect.any(Function), - getThreadId$: expect.any(Function), - getThreadId: expect.any(Function), - openWindow: expect.any(Function), - closeWindow: expect.any(Function), - sendMessage: expect.any(Function), - sendMessageWithWindow: expect.any(Function), - getWindowState: expect.any(Function), - getWindowState$: expect.any(Function), - onWindowOpen: expect.any(Function), - onWindowClose: expect.any(Function), - suggestedActionsService: undefined, - }); - }); + it('should manage thread ID in core', () => { + const startContract = service.start(); - describe('window state management', () => { - beforeEach(() => { - // Set up implementation for these tests - const setupContract = service.setup(); - setupContract.setImplementation(mockImplementation); - startContract = service.start(); - }); - - it('should delegate to implementation for window state', () => { - expect(startContract.isWindowOpen()).toBe(false); - expect(mockImplementation.isWindowOpen).toHaveBeenCalled(); - }); - - it('should delegate window open/close to implementation', async () => { - await startContract.openWindow(); - expect(mockImplementation.openWindow).toHaveBeenCalled(); - - await startContract.closeWindow(); - expect(mockImplementation.closeWindow).toHaveBeenCalled(); - }); - - it('should provide window state via implementation', () => { - const state = startContract.getWindowState(); - expect(mockImplementation.getWindowState).toHaveBeenCalled(); - expect(state).toEqual({ - isWindowOpen: false, - windowMode: 'sidecar', - paddingSize: 400, - }); - }); - - it('should provide window state observable via implementation', (done) => { - const state$ = startContract.getWindowState$(); - expect(mockImplementation.getWindowState$).toHaveBeenCalled(); - - state$ - .subscribe((state: any) => { - expect(state).toEqual({ - isWindowOpen: false, - windowMode: 'sidecar', - paddingSize: 400, - }); - done(); - }) - .unsubscribe(); - }); - - it('should delegate window callbacks to implementation', () => { - const openCallback = jest.fn(); - const closeCallback = jest.fn(); - - startContract.onWindowOpen(openCallback); - startContract.onWindowClose(closeCallback); - - expect(mockImplementation.onWindowOpen).toHaveBeenCalledWith(openCallback); - expect(mockImplementation.onWindowClose).toHaveBeenCalledWith(closeCallback); - }); - }); + const initialThreadId = startContract.getThreadId(); + expect(initialThreadId).toMatch(/^thread-\d+-[a-z0-9]{9}$/); - describe('thread ID management', () => { - beforeEach(() => { - // Set up implementation for these tests - const setupContract = service.setup(); - setupContract.setImplementation(mockImplementation); - startContract = service.start(); - }); - - it('should delegate thread ID to implementation', () => { - const threadId = startContract.getThreadId(); - expect(mockImplementation.getThreadId).toHaveBeenCalled(); - expect(threadId).toBe('test-thread-id'); - }); - - it('should provide thread ID observable via implementation', (done) => { - const threadId$ = startContract.getThreadId$(); - expect(mockImplementation.getThreadId$).toHaveBeenCalled(); - - threadId$ - .subscribe((threadId: string) => { - expect(threadId).toBe('test-thread-id'); - done(); - }) - .unsubscribe(); - }); - }); + // Test observable + let emittedThreadId: string | undefined; + startContract.getThreadId$().subscribe((id) => (emittedThreadId = id)); + expect(emittedThreadId).toBe(initialThreadId); - describe('message handling', () => { - it('should return undefined when no implementation or fallback is set', async () => { - // Use fresh service without implementation or fallback - const freshService = new ChatService(); - const freshStartContract = freshService.start(); - - // Should return undefined when no implementation or fallback provided - const result1 = await freshStartContract.sendMessage('test message', []); - expect(result1).toBeUndefined(); - - const result2 = await freshStartContract.sendMessageWithWindow('test message', []); - expect(result2).toBeUndefined(); - }); - - it('should delegate to implementation when set', async () => { - const setupContract = service.setup(); - setupContract.setImplementation(mockImplementation); - const serviceWithImpl = service.start(); - - const testMessages: UserMessage[] = [ - { id: '1', role: 'user', content: 'previous message' }, - ]; - - await serviceWithImpl.sendMessage('test message', testMessages); - expect(mockImplementation.sendMessage).toHaveBeenCalledWith('test message', testMessages); - - await serviceWithImpl.sendMessageWithWindow('test message with window', testMessages, { - clearConversation: true, - }); - expect(mockImplementation.sendMessageWithWindow).toHaveBeenCalledWith( - 'test message with window', - testMessages, - { - clearConversation: true, - } - ); - }); - }); + // Test setting new thread + startContract.newThread(); + const newThreadId = startContract.getThreadId(); + expect(newThreadId).not.toBe(initialThreadId); + expect(newThreadId).toMatch(/^thread-\d+-[a-z0-9]{9}$/); }); - describe('delegation without implementation or fallback', () => { - it('should return undefined/default values when no implementation or fallback provided', () => { - const startContract = service.start(); + it('should manage window state in core', () => { + const startContract = service.start(); - // All methods should return undefined or default values when neither implementation nor fallback is available - expect(startContract.getThreadId()).toBeUndefined(); - expect(startContract.getWindowState()).toBeUndefined(); - expect(startContract.isWindowOpen()).toBeUndefined(); + // Initial state + expect(startContract.isWindowOpen()).toBe(false); + const initialState = startContract.getWindowState(); + expect(initialState).toEqual({ + isWindowOpen: false, + windowMode: 'sidecar', + paddingSize: 400, + }); - // onWindow methods return functions (unsubscribe callbacks), not undefined - expect(typeof startContract.onWindowOpen(() => {})).toBe('function'); - expect(typeof startContract.onWindowClose(() => {})).toBe('function'); + // Update state + startContract.setWindowState({ isWindowOpen: true }); + expect(startContract.isWindowOpen()).toBe(true); - // Observable methods should return undefined - expect(startContract.getThreadId$()).toBeUndefined(); - expect(startContract.getWindowState$()).toBeUndefined(); - }); + // Test observable + let emittedState: any; + startContract.getWindowState$().subscribe((state) => (emittedState = state)); + expect(emittedState.isWindowOpen).toBe(true); + }); - it('should return undefined for async operations when no fallback', async () => { - const startContract = service.start(); + it('should trigger window callbacks', () => { + const startContract = service.start(); - // Message and window operations should return undefined - const result1 = await startContract.sendMessage('test', []); - expect(result1).toBeUndefined(); + const openCallback = jest.fn(); + const closeCallback = jest.fn(); - const result2 = await startContract.sendMessageWithWindow('test', []); - expect(result2).toBeUndefined(); + startContract.onWindowOpen(openCallback); + startContract.onWindowClose(closeCallback); - const result3 = await startContract.openWindow(); - expect(result3).toBeUndefined(); + // Open window + startContract.setWindowState({ isWindowOpen: true }); + expect(openCallback).toHaveBeenCalledTimes(1); + expect(closeCallback).not.toHaveBeenCalled(); - const result4 = await startContract.closeWindow(); - expect(result4).toBeUndefined(); - }); + // Close window + startContract.setWindowState({ isWindowOpen: false }); + expect(closeCallback).toHaveBeenCalledTimes(1); }); - describe('delegation with fallback implementation', () => { - let fallbackImplementation: ChatImplementationFunctions; - - beforeEach(() => { - // Create a fallback implementation (simulating plugin-provided fallback) - fallbackImplementation = { - sendMessage: jest.fn().mockResolvedValue({ - observable: null, - userMessage: { id: 'fallback-id', role: 'user', content: 'fallback-content' }, - }), - sendMessageWithWindow: jest.fn().mockResolvedValue({ - observable: null, - userMessage: { id: 'fallback-id', role: 'user', content: 'fallback-content' }, - }), - getThreadId: jest.fn().mockReturnValue('fallback-thread-id'), - getThreadId$: jest.fn().mockReturnValue(mockThreadId$.asObservable()), - isWindowOpen: jest.fn().mockReturnValue(false), - openWindow: jest.fn().mockResolvedValue(undefined), - closeWindow: jest.fn().mockResolvedValue(undefined), - getWindowState: jest.fn().mockReturnValue({ - isWindowOpen: false, - windowMode: 'sidecar', - paddingSize: 400, - }), - getWindowState$: jest.fn().mockReturnValue(mockWindowState$.asObservable()), - onWindowOpen: jest.fn().mockReturnValue(() => {}), - onWindowClose: jest.fn().mockReturnValue(() => {}), - }; - }); + it('should throw error when sending message without implementation', async () => { + const startContract = service.start(); - it('should delegate to fallback when no main implementation', () => { - const setupContract = service.setup(); - setupContract.setFallbackImplementation(fallbackImplementation); - const startContract = service.start(); + await expect(startContract.sendMessage('test', [])).rejects.toThrow( + 'Chat service is not available. Please ensure the chat plugin is enabled.' + ); + }); - // Should delegate to fallback implementation - expect(startContract.getThreadId()).toBe('fallback-thread-id'); - expect(fallbackImplementation.getThreadId).toHaveBeenCalled(); + it('should throw error when opening window without implementation', async () => { + const startContract = service.start(); - expect(startContract.isWindowOpen()).toBe(false); - expect(fallbackImplementation.isWindowOpen).toHaveBeenCalled(); - }); + await expect(startContract.openWindow()).rejects.toThrow( + 'Chat service is not available. Please ensure the chat plugin is enabled.' + ); + }); - it('should prefer main implementation over fallback', () => { - const setupContract = service.setup(); - setupContract.setFallbackImplementation(fallbackImplementation); - setupContract.setImplementation(mockImplementation); - const startContract = service.start(); + it('should delegate to implementation when available', async () => { + const setupContract = service.setup(); + const startContract = service.start(); + + setupContract.setImplementation(mockImplementation); - // Should use main implementation, not fallback - expect(startContract.getThreadId()).toBe('test-thread-id'); - expect(mockImplementation.getThreadId).toHaveBeenCalled(); - expect(fallbackImplementation.getThreadId).not.toHaveBeenCalled(); + // Test message sending + const result = await startContract.sendMessage('test', []); + expect(mockImplementation.sendMessage).toHaveBeenCalledWith('test', []); + expect(result).toEqual({ + observable: null, + userMessage: { id: '1', role: 'user', content: 'test' }, }); + + // Test window operations + await startContract.openWindow(); + expect(mockImplementation.openWindow).toHaveBeenCalled(); + expect(startContract.isWindowOpen()).toBe(true); // Core state updated + + await startContract.closeWindow(); + expect(mockImplementation.closeWindow).toHaveBeenCalled(); + expect(startContract.isWindowOpen()).toBe(false); // Core state updated }); }); describe('stop', () => { - it('should not throw when called', () => { - expect(() => service.stop()).not.toThrow(); + it('should clean up resources', async () => { + const setupContract = service.setup(); + const startContract = service.start(); + + setupContract.setImplementation(mockImplementation); + expect(startContract.isAvailable()).toBe(true); + + await service.stop(); + expect(startContract.isAvailable()).toBe(false); }); }); }); diff --git a/src/core/public/chat/chat_service.ts b/src/core/public/chat/chat_service.ts index d4ef587240ed..f5b5ae8fafcd 100644 --- a/src/core/public/chat/chat_service.ts +++ b/src/core/public/chat/chat_service.ts @@ -3,51 +3,47 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { BehaviorSubject } from 'rxjs'; import { CoreService } from '../../types'; -import { ChatServiceSetup, ChatServiceStart, ChatImplementationFunctions, Message } from './types'; -import { NoOpChatService } from './no_op_chat_service'; +import { + ChatServiceSetup, + ChatServiceStart, + ChatImplementationFunctions, + Message, + ChatWindowState, +} from './types'; /** - * Core chat service implementation - * Contains the actual chat functionality + * Core chat service - manages infrastructure state */ export class ChatService implements CoreService { private implementation?: ChatImplementationFunctions; - private serviceStart?: ChatServiceStart; private suggestedActionsService?: { registerProvider(provider: any): void }; - private noOpService = new NoOpChatService(); + + // Core-managed infrastructure state + private threadId$ = new BehaviorSubject(this.generateThreadId()); + private windowState$ = new BehaviorSubject({ + isWindowOpen: false, + windowMode: 'sidecar', + paddingSize: 400, + }); + private windowOpenCallbacks = new Set<() => void>(); + private windowCloseCallbacks = new Set<() => void>(); + + private generateThreadId(): string { + const timestamp = Date.now(); + const randomStr = Math.random().toString(36).substring(2, 11); + return `thread-${timestamp}-${randomStr}`; + } public setup(): ChatServiceSetup { return { setImplementation: (implementation: ChatImplementationFunctions) => { this.implementation = implementation; - - // Update service methods with implementation if service is already started - if (this.serviceStart) { - this.serviceStart.sendMessage = async (content: string, messages: Message[]) => { - return implementation.sendMessage(content, messages); - }; - this.serviceStart.sendMessageWithWindow = async ( - content: string, - messages: Message[], - options?: { clearConversation?: boolean } - ) => { - return implementation.sendMessageWithWindow(content, messages, options); - }; - } - }, - - setFallbackImplementation: (fallback: ChatImplementationFunctions) => { - this.noOpService.setFallbackImplementation(fallback); }, setSuggestedActionsService: (service: { registerProvider(provider: any): void }) => { this.suggestedActionsService = service; - - // Update the start interface if it's already been created - if (this.serviceStart) { - this.serviceStart.suggestedActionsService = service; - } }, suggestedActionsService: this.suggestedActionsService, @@ -55,76 +51,119 @@ export class ChatService implements CoreService) => { + const currentState = this.windowState$.getValue(); + const newState = { ...currentState, ...partialState }; + this.windowState$.next(newState); + }; + + const chatServiceInstance = this; + + return { + // Availability check isAvailable: () => { return !!this.implementation; }, - // Pure delegation to either implementation or no-op service + // Thread management (core-managed) getThreadId: () => { - return this.implementation?.getThreadId() ?? this.noOpService.getThreadId(); + return this.threadId$.getValue(); }, + getThreadId$: () => { - return this.implementation?.getThreadId$() ?? this.noOpService.getThreadId$(); + return this.threadId$.asObservable(); }, - // Message operations - sendMessage: async (content: string, messages: Message[]) => { - return ( - this.implementation?.sendMessage(content, messages) ?? - this.noOpService.sendMessage(content, messages) - ); + setThreadId: (threadId: string) => { + this.threadId$.next(threadId); }, - sendMessageWithWindow: async ( - content: string, - messages: Message[], - options?: { clearConversation?: boolean } - ) => { - return ( - this.implementation?.sendMessageWithWindow(content, messages, options) ?? - this.noOpService.sendMessageWithWindow(content, messages, options) - ); + + newThread: () => { + const newThreadId = this.generateThreadId(); + this.threadId$.next(newThreadId); }, - // Window management + // Window state management (core-managed) isWindowOpen: () => { - return this.implementation?.isWindowOpen() ?? this.noOpService.isWindowOpen(); - }, - openWindow: async () => { - return this.implementation?.openWindow() ?? this.noOpService.openWindow(); - }, - closeWindow: async () => { - return this.implementation?.closeWindow() ?? this.noOpService.closeWindow(); + return this.windowState$.getValue().isWindowOpen; }, + getWindowState: () => { - return this.implementation?.getWindowState() ?? this.noOpService.getWindowState(); + return this.windowState$.getValue(); }, + getWindowState$: () => { - return this.implementation?.getWindowState$() ?? this.noOpService.getWindowState$(); + return this.windowState$.asObservable(); }, + + setWindowState, + onWindowOpen: (callback: () => void) => { - return ( - this.implementation?.onWindowOpen(callback) ?? this.noOpService.onWindowOpen(callback) - ); + this.windowOpenCallbacks.add(callback); + return () => this.windowOpenCallbacks.delete(callback); }, + onWindowClose: (callback: () => void) => { - return ( - this.implementation?.onWindowClose(callback) ?? this.noOpService.onWindowClose(callback) - ); + this.windowCloseCallbacks.add(callback); + return () => this.windowCloseCallbacks.delete(callback); }, - // Infrastructure service - suggestedActionsService: this.suggestedActionsService, - }; + // Operations (delegated to plugin - throw if unavailable) + openWindow: async () => { + if (!this.implementation) { + throw new Error( + 'Chat service is not available. Please ensure the chat plugin is enabled.' + ); + } + + // Trigger callbacks to request window opening + this.windowOpenCallbacks.forEach((callback) => callback()); + }, + + closeWindow: async () => { + if (!this.implementation) { + throw new Error( + 'Chat service is not available. Please ensure the chat plugin is enabled.' + ); + } - return this.serviceStart!; + // Trigger callbacks to request window closing + this.windowCloseCallbacks.forEach((callback) => callback()); + }, + + sendMessage: async (content: string, messages: Message[]) => { + if (!this.implementation) { + throw new Error( + 'Chat service is not available. Please ensure the chat plugin is enabled.' + ); + } + return this.implementation.sendMessage(content, messages); + }, + + sendMessageWithWindow: async ( + content: string, + messages: Message[], + options?: { clearConversation?: boolean } + ) => { + if (!this.implementation) { + throw new Error( + 'Chat service is not available. Please ensure the chat plugin is enabled.' + ); + } + return this.implementation.sendMessageWithWindow(content, messages, options); + }, + + // Infrastructure service - use getter to ensure dynamic access + get suggestedActionsService() { + return chatServiceInstance.suggestedActionsService; + }, + }; } public async stop() { - // Reset state this.implementation = undefined; - this.serviceStart = undefined; this.suggestedActionsService = undefined; + this.windowOpenCallbacks.clear(); + this.windowCloseCallbacks.clear(); } } diff --git a/src/core/public/chat/no_op_chat_service.ts b/src/core/public/chat/no_op_chat_service.ts deleted file mode 100644 index b862e4f41dc4..000000000000 --- a/src/core/public/chat/no_op_chat_service.ts +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { BehaviorSubject, Observable } from 'rxjs'; -import { - ChatServiceStart, - Message, - ChatWindowState, - UserMessage, - ChatImplementationFunctions, -} from './types'; - -/** - * No-op implementation that delegates to plugin-provided fallback - */ -export class NoOpChatService implements ChatServiceStart { - private fallbackImplementation?: ChatImplementationFunctions; - - setFallbackImplementation(fallback: ChatImplementationFunctions): void { - this.fallbackImplementation = fallback; - } - - isAvailable(): boolean { - return false; - } - - isWindowOpen(): boolean { - return this.fallbackImplementation?.isWindowOpen?.()!; - } - - getThreadId$(): Observable { - return this.fallbackImplementation?.getThreadId$?.()!; - } - - getThreadId(): string { - return this.fallbackImplementation?.getThreadId?.()!; - } - - async openWindow(): Promise { - return this.fallbackImplementation?.openWindow?.(); - } - - async closeWindow(): Promise { - return this.fallbackImplementation?.closeWindow?.(); - } - - getWindowState(): ChatWindowState { - return this.fallbackImplementation?.getWindowState?.()!; - } - - async sendMessage( - content: string, - messages: Message[] - ): Promise<{ observable: any; userMessage: UserMessage }> { - return this.fallbackImplementation?.sendMessage?.(content, messages)!; - } - - async sendMessageWithWindow( - content: string, - messages: Message[], - options?: { clearConversation?: boolean } - ): Promise<{ observable: any; userMessage: UserMessage }> { - return this.fallbackImplementation?.sendMessageWithWindow?.(content, messages, options)!; - } - - getWindowState$(): Observable { - return this.fallbackImplementation?.getWindowState$?.()!; - } - - onWindowOpen(callback: () => void): () => void { - return this.fallbackImplementation?.onWindowOpen?.(callback) ?? (() => {}); - } - - onWindowClose(callback: () => void): () => void { - return this.fallbackImplementation?.onWindowClose?.(callback) ?? (() => {}); - } - - suggestedActionsService = undefined; -} diff --git a/src/core/public/chat/types.ts b/src/core/public/chat/types.ts index 37b000bc8a28..e7a58bc5e3f6 100644 --- a/src/core/public/chat/types.ts +++ b/src/core/public/chat/types.ts @@ -101,79 +101,50 @@ export interface ChatWindowState { } /** - * Chat service interface - provides basic chat functionality - * without UI dependencies to avoid circular dependencies + * Chat service interface - state managed by core, operations delegated to plugin */ export interface ChatServiceInterface { /** - * Check if chat window is currently open + * Check if chat service is available */ - isWindowOpen(): boolean; + isAvailable(): boolean; /** - * Get the current thread ID as observable + * Thread management - managed by core */ getThreadId$(): Observable; - - /** - * Get the current thread ID - */ getThreadId(): string; + setThreadId(threadId: string): void; + newThread(): void; /** - * Open the chat window + * Window state management - managed by core */ - openWindow(): Promise; + isWindowOpen(): boolean; + getWindowState(): ChatWindowState; + getWindowState$(): Observable; + setWindowState(state: Partial): void; + onWindowOpen(callback: () => void): () => void; + onWindowClose(callback: () => void): () => void; /** - * Close the chat window + * Operations - delegated to plugin (throws error if unavailable) */ + openWindow(): Promise; closeWindow(): Promise; - - /** - * Send a message through the chat service - * @param message The message text to send - * @param history Optional message history for context - * @param options Optional configuration for the message - */ sendMessage( content: string, messages: Message[] ): Promise<{ observable: any; userMessage: UserMessage }>; - - /** - * Send a message and ensure window is open - * This is the primary method used by plugins - */ sendMessageWithWindow( content: string, messages: Message[], options?: { clearConversation?: boolean } ): Promise<{ observable: any; userMessage: UserMessage }>; - - /** - * Get current window state - */ - getWindowState(): ChatWindowState; - - /** - * Get window state observable - */ - getWindowState$(): Observable; - - /** - * Register a callback for when window opens - */ - onWindowOpen(callback: () => void): () => void; - - /** - * Register a callback for when window closes - */ - onWindowClose(callback: () => void): () => void; } /** - * Implementation functions provided by the chat plugin + * Implementation functions provided by the chat plugin - simplified to business logic only */ export interface ChatImplementationFunctions { // Message operations @@ -181,24 +152,16 @@ export interface ChatImplementationFunctions { content: string, messages: Message[] ) => Promise<{ observable: any; userMessage: UserMessage }>; + sendMessageWithWindow: ( content: string, messages: Message[], options?: { clearConversation?: boolean } ) => Promise<{ observable: any; userMessage: UserMessage }>; - // Thread management - getThreadId: () => string; - getThreadId$: () => Observable; - - // Window management - isWindowOpen: () => boolean; + // Window operations openWindow: () => Promise; closeWindow: () => Promise; - getWindowState: () => ChatWindowState; - getWindowState$: () => Observable; - onWindowOpen: (callback: () => void) => () => void; - onWindowClose: (callback: () => void) => () => void; } /** @@ -211,13 +174,6 @@ export interface ChatServiceSetup { */ setImplementation(implementation: ChatImplementationFunctions): void; - /** - * Set the fallback implementation for when chat service is unavailable - * This allows the chat plugin to control "unavailable" behavior - * This will be called by the chat plugin - */ - setFallbackImplementation(fallback: ChatImplementationFunctions): void; - /** * Set the suggested actions service * This will be called by the chat plugin @@ -237,11 +193,6 @@ export interface ChatServiceSetup { * Chat service start interface */ export interface ChatServiceStart extends ChatServiceInterface { - /** - * Whether chat service is available - */ - isAvailable(): boolean; - /** * Suggested actions service for registering providers * Available at runtime after chat plugin has registered it diff --git a/src/plugins/chat/public/components/chat_header_button.tsx b/src/plugins/chat/public/components/chat_header_button.tsx index 88ab6c8c58c9..df0e8f51a1d7 100644 --- a/src/plugins/chat/public/components/chat_header_button.tsx +++ b/src/plugins/chat/public/components/chat_header_button.tsx @@ -58,19 +58,25 @@ export const ChatHeaderButton = React.forwardRef { - if (!flyoutMountPoint.current) return; + if (!flyoutMountPoint.current) { + return; + } - sideCarRef.current = core.overlays.sidecar.open(flyoutMountPoint.current, { - className: `chat-sidecar chat-sidecar--${layoutMode}`, - config: { - dockedMode: - layoutMode === ChatLayoutMode.FULLSCREEN - ? SIDECAR_DOCKED_MODE.TAKEOVER - : SIDECAR_DOCKED_MODE.RIGHT, - paddingSize: chatService.getPaddingSize(), - isHidden: false, - }, - }); + try { + sideCarRef.current = core.overlays.sidecar.open(flyoutMountPoint.current, { + className: `chat-sidecar chat-sidecar--${layoutMode}`, + config: { + dockedMode: + layoutMode === ChatLayoutMode.FULLSCREEN + ? SIDECAR_DOCKED_MODE.TAKEOVER + : SIDECAR_DOCKED_MODE.RIGHT, + paddingSize: chatService.getPaddingSize(), + isHidden: false, + }, + }); + } catch (error) { + return; + } // Notify ChatService that window is now open chatService.setWindowState({ isWindowOpen: true }); @@ -138,19 +144,21 @@ export const ChatHeaderButton = React.forwardRef { + unsubscribe(); + }; }, [chatService]); // Register callbacks for external window open/close requests useEffect(() => { const unsubscribeOpen = chatService.onWindowOpenRequest(() => { - if (!isOpen) { + if (!chatService.isWindowOpen()) { openSidecar(); } }); const unsubscribeClose = chatService.onWindowCloseRequest(() => { - if (isOpen) { + if (chatService.isWindowOpen()) { closeSidecar(); } }); @@ -159,7 +167,7 @@ export const ChatHeaderButton = React.forwardRef { diff --git a/src/plugins/chat/public/components/chat_window.tsx b/src/plugins/chat/public/components/chat_window.tsx index 55441caf6985..a5d261a3014c 100644 --- a/src/plugins/chat/public/components/chat_window.tsx +++ b/src/plugins/chat/public/components/chat_window.tsx @@ -51,6 +51,7 @@ const ChatWindowContent = React.forwardRef( onToggleLayout, onClose, }, ref) => { + const service = AssistantActionService.getInstance(); const { chatService } = useChatContext(); const { services } = useOpenSearchDashboards<{ @@ -82,9 +83,6 @@ const ChatWindowContent = React.forwardRef( [service, chatService] ); - // Context is now handled by RFC hooks - no need for context manager - // The chat service will get context directly from assistantContextStore - // Subscribe to tool updates from the service useEffect(() => { const subscription = service.getState$().subscribe((state) => { @@ -138,15 +136,7 @@ const ChatWindowContent = React.forwardRef( }; setTimeline((prev) => [...prev, timelineUserMessage]); - // Start a new run group - we'll get the actual runId from the first event - const timestamp = new Date().toLocaleTimeString(); - console.groupCollapsed( - `📊 Chat Run [${timestamp}] - "${messageContent.substring(0, 50)}${ - messageContent.length > 50 ? '...' : '' - }"` - ); - - // Subscribe to streaming response - now much cleaner! + // Subscribe to streaming response const subscription = observable.subscribe({ next: async (event: ChatEvent) => { // Update runId if we get it from the event @@ -159,11 +149,9 @@ const ChatWindowContent = React.forwardRef( }, error: (error: any) => { console.error('Subscription error:', error); - console.groupEnd(); // Close the run group setIsStreaming(false); }, complete: () => { - console.groupEnd(); // Close the run group setIsStreaming(false); }, }); @@ -171,7 +159,6 @@ const ChatWindowContent = React.forwardRef( return () => subscription.unsubscribe(); } catch (error) { console.error('Failed to send message:', error); - console.groupEnd(); // Close the run group setIsStreaming(false); } }; @@ -220,15 +207,7 @@ const ChatWindowContent = React.forwardRef( }; setTimeline((prev) => [...prev, timelineUserMessage]); - // Start a new run group - we'll get the actual runId from the first event - const timestamp = new Date().toLocaleTimeString(); - console.groupCollapsed( - `📊 Chat Run [${timestamp}] - "${message.content.substring(0, 50)}${ - message.content.length > 50 ? '...' : '' - }"` - ); - - // Subscribe to streaming response - now using the event handler! + // Subscribe to streaming response const subscription = observable.subscribe({ next: async (event: ChatEvent) => { // Update runId if we get it from the event @@ -241,11 +220,9 @@ const ChatWindowContent = React.forwardRef( }, error: (error: any) => { console.error('Subscription error:', error); - console.groupEnd(); // Close the run group setIsStreaming(false); }, complete: () => { - console.groupEnd(); // Close the run group setIsStreaming(false); }, }); @@ -253,22 +230,17 @@ const ChatWindowContent = React.forwardRef( return () => subscription.unsubscribe(); } catch (error) { console.error('Failed to resend message:', error); - console.groupEnd(); // Close the run group setIsStreaming(false); } }; const handleNewChat = useCallback(() => { - console.log('[DEBUG] ChatWindow - handleNewChat called, about to call chatService.newThread()'); chatService.newThread(); setTimeline([]); setCurrentRunId(null); setIsStreaming(false); }, [chatService]); - // No cleanup needed - RFC hooks handle their own lifecycle - - // Pass enhanced props to child components const currentState = service.getCurrentState(); const enhancedProps = { toolCallStates: currentState.toolCallStates, diff --git a/src/plugins/chat/public/components/graph_visualization/error_boundary.tsx b/src/plugins/chat/public/components/graph_visualization/error_boundary.tsx deleted file mode 100644 index 100b0a8c7015..000000000000 --- a/src/plugins/chat/public/components/graph_visualization/error_boundary.tsx +++ /dev/null @@ -1,401 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import React, { Component, ErrorInfo, ReactNode } from 'react'; -import { EuiButton, EuiButtonEmpty, EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; - -import { ErrorDisplay } from './error_display'; -import { - createEnhancedError, - logChartError, - EnhancedChartError, - ExtendedErrorType, -} from './error_handler'; - -/** - * Props for the error boundary component - */ -interface ErrorBoundaryProps { - children: ReactNode; - fallback?: ReactNode; - onError?: (error: Error, errorInfo: ErrorInfo) => void; - onReset?: () => void; - resetKeys?: Array; - resetOnPropsChange?: boolean; - isolate?: boolean; -} - -/** - * State for the error boundary component - */ -interface ErrorBoundaryState { - hasError: boolean; - error: EnhancedChartError | null; - errorId: string | null; - retryCount: number; -} - -/** - * Enhanced error boundary specifically designed for chart components - * Provides comprehensive error handling, logging, and recovery options - */ -export class ChartErrorBoundary extends Component { - private resetTimeoutId: number | null = null; - private previousResetKeys: Array = []; - private _isMounted = false; - - constructor(props: ErrorBoundaryProps) { - super(props); - - this.state = { - hasError: false, - error: null, - errorId: null, - retryCount: 0, - }; - - this.previousResetKeys = props.resetKeys || []; - } - - componentDidMount() { - this._isMounted = true; - } - - /** - * Static method to derive state from error - */ - static getDerivedStateFromError(error: Error): Partial { - const errorId = `chart_error_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; - - // Determine error type based on error message and stack - let errorType: ExtendedErrorType = 'render_error'; - - if (error.message.includes('memory') || error.message.includes('Maximum call stack')) { - errorType = 'memory_error'; - } else if (error.message.includes('network') || error.message.includes('fetch')) { - errorType = 'network_error'; - } else if (error.message.includes('timeout')) { - errorType = 'timeout_error'; - } else if (error.message.includes('browser') || error.message.includes('compatibility')) { - errorType = 'browser_compatibility'; - } - - const enhancedError = createEnhancedError( - errorType, - error.message || 'Chart rendering failed', - { - severity: errorType === 'memory_error' ? 'critical' : 'high', - originalError: error, - context: { - componentState: 'error_boundary_catch', - timestamp: Date.now(), - }, - suggestions: [ - 'Try refreshing the page', - 'Check browser console for additional details', - 'Reduce data size if working with large datasets', - ], - recoverable: true, - } - ); - - return { - hasError: true, - error: enhancedError, - errorId, - }; - } - - /** - * Component did catch - handle error logging and reporting - */ - componentDidCatch(error: Error, errorInfo: ErrorInfo) { - const { onError } = this.props; - const { error: enhancedError } = this.state; - - // Enhanced error logging - if (enhancedError) { - const errorWithContext = { - ...enhancedError, - context: { - ...enhancedError.context, - componentStack: errorInfo.componentStack, - errorBoundary: 'ChartErrorBoundary', - retryCount: this.state.retryCount, - }, - }; - - logChartError(errorWithContext, { - componentName: 'ChartErrorBoundary', - userAction: 'component_render', - additionalInfo: { - errorInfo, - props: this.props, - }, - }); - } - - // Call custom error handler if provided - if (onError) { - onError(error, errorInfo); - } - - // Report to external error tracking service if available - this.reportToExternalService(error, errorInfo); - } - - /** - * Component did update - check for reset conditions - */ - componentDidUpdate(prevProps: ErrorBoundaryProps) { - const { resetKeys, resetOnPropsChange } = this.props; - const { hasError } = this.state; - - // Reset on props change if enabled - if (hasError && resetOnPropsChange && prevProps !== this.props) { - this.resetErrorBoundary(); - return; - } - - // Reset on resetKeys change - if (hasError && resetKeys) { - const hasResetKeyChanged = resetKeys.some( - (key, index) => this.previousResetKeys[index] !== key - ); - - if (hasResetKeyChanged) { - this.previousResetKeys = resetKeys; - this.resetErrorBoundary(); - } - } - } - - /** - * Component will unmount - cleanup - */ - componentWillUnmount() { - this._isMounted = false; - if (this.resetTimeoutId) { - clearTimeout(this.resetTimeoutId); - } - } - - /** - * Reset the error boundary state - */ - private resetErrorBoundary = () => { - const { onReset } = this.props; - - if (this._isMounted) { - this.setState({ - hasError: false, - error: null, - errorId: null, - retryCount: 0, - }); - } - - if (onReset) { - onReset(); - } - }; - - /** - * Handle retry with exponential backoff - */ - private handleRetry = () => { - const { retryCount } = this.state; - const maxRetries = 3; - - if (retryCount >= maxRetries) { - // Maximum retry attempts reached - return; - } - - // Exponential backoff: 1s, 2s, 4s - const delay = Math.pow(2, retryCount) * 1000; - - if (this._isMounted) { - this.setState({ retryCount: retryCount + 1 }); - } - - this.resetTimeoutId = window.setTimeout(() => { - this.resetErrorBoundary(); - }, delay); - }; - - /** - * Handle error dismissal - */ - private handleDismiss = () => { - this.resetErrorBoundary(); - }; - - /** - * Report error to external service (placeholder) - */ - private reportToExternalService(error: Error, errorInfo: ErrorInfo) { - // Placeholder for external error reporting - // Could integrate with services like Sentry, Bugsnag, etc. - if (process.env.NODE_ENV === 'development') { - // Chart Error Boundary - error details would be logged in production via proper logging service - } - } - - /** - * Get fallback UI based on error type and context - */ - private getFallbackUI(): ReactNode { - const { fallback } = this.props; - const { error, retryCount } = this.state; - - // Use custom fallback if provided - if (fallback) { - return fallback; - } - - // Use enhanced error display - if (error) { - return ( - - ); - } - - // Default fallback - return ( -
-

Something went wrong

-

An error occurred while rendering the chart.

- - - - Try again - - - - - Dismiss - - - -
- ); - } - - render() { - const { children, isolate } = this.props; - const { hasError } = this.state; - - if (hasError) { - const fallbackUI = this.getFallbackUI(); - - // Isolate error to prevent propagation if requested - if (isolate) { - return
{fallbackUI}
; - } - - return fallbackUI; - } - - return children; - } -} - -/** - * Hook-based error boundary for functional components - */ -export const useErrorHandler = () => { - const [error, setError] = React.useState(null); - - const resetError = React.useCallback(() => { - setError(null); - }, []); - - const handleError = React.useCallback((err: Error | EnhancedChartError) => { - let enhancedError: EnhancedChartError; - - if ('type' in err && 'severity' in err) { - enhancedError = err as EnhancedChartError; - } else { - enhancedError = createEnhancedError( - 'unknown_error', - err.message || 'Unknown error occurred', - { - severity: 'medium', - originalError: err as Error, - recoverable: true, - } - ); - } - - logChartError(enhancedError); - setError(enhancedError); - }, []); - - return { - error, - resetError, - handleError, - hasError: error !== null, - }; -}; - -/** - * Higher-order component for adding error boundary to any component - */ -export function withErrorBoundary

( - WrappedComponent: React.ComponentType

, - errorBoundaryProps?: Partial -) { - const ComponentWithErrorBoundary = (props: P) => ( - - - - ); - - ComponentWithErrorBoundary.displayName = `withErrorBoundary(${ - WrappedComponent.displayName || WrappedComponent.name - })`; - - return ComponentWithErrorBoundary; -} - -/** - * Async error boundary for handling promise rejections - */ -export const AsyncErrorBoundary: React.FC<{ - children: ReactNode; - onError?: (error: Error) => void; -}> = ({ children, onError }) => { - const { handleError } = useErrorHandler(); - - React.useEffect(() => { - const handleUnhandledRejection = (event: PromiseRejectionEvent) => { - const error = event.reason instanceof Error ? event.reason : new Error(String(event.reason)); - - handleError(error); - - if (onError) { - onError(error); - } - - // Prevent the default browser behavior - event.preventDefault(); - }; - - window.addEventListener('unhandledrejection', handleUnhandledRejection); - - return () => { - window.removeEventListener('unhandledrejection', handleUnhandledRejection); - }; - }, [handleError, onError]); - - return <>{children}; -}; diff --git a/src/plugins/chat/public/components/graph_visualization/error_display.tsx b/src/plugins/chat/public/components/graph_visualization/error_display.tsx deleted file mode 100644 index fc93c55111a9..000000000000 --- a/src/plugins/chat/public/components/graph_visualization/error_display.tsx +++ /dev/null @@ -1,545 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import React, { useState, useCallback } from 'react'; -import { - EuiEmptyPrompt, - EuiButton, - EuiButtonEmpty, - EuiCallOut, - EuiCollapsibleNav, - EuiText, - EuiSpacer, - EuiAccordion, - EuiCodeBlock, - EuiCopy, - EuiFlexGroup, - EuiFlexItem, - EuiBadge, - EuiIcon, - EuiToolTip, - EuiLink, -} from '@elastic/eui'; - -import { - EnhancedChartError, - getErrorDisplayInfo, - getRecoveryActions, - errorLogger, -} from './error_handler'; - -/** - * Props for the main error display component - */ -interface ErrorDisplayProps { - error: EnhancedChartError; - onRetry?: () => void; - onDismiss?: () => void; - showDetails?: boolean; - compact?: boolean; -} - -/** - * Props for error details component - */ -interface ErrorDetailsProps { - error: EnhancedChartError; - isOpen: boolean; - onToggle: () => void; -} - -/** - * Props for error suggestions component - */ -interface ErrorSuggestionsProps { - error: EnhancedChartError; - onAction?: (action: string) => void; -} - -/** - * Main error display component with comprehensive error information - */ -export const ErrorDisplay: React.FC = ({ - error, - onRetry, - onDismiss, - showDetails = false, - compact = false, -}) => { - const [showDetailsState, setShowDetailsState] = useState(showDetails); - const displayInfo = getErrorDisplayInfo(error); - const recoveryActions = getRecoveryActions(error); - - const handleAction = useCallback( - (action: string) => { - switch (action) { - case 'retry': - onRetry?.(); - break; - case 'dismiss': - onDismiss?.(); - break; - case 'view_docs': - // Open documentation in new tab - window.open('/app/opensearch-dashboards/docs/graph-visualization', '_blank'); - break; - case 'check_format': - // Could open a modal with format examples - // Data format help requested - break; - default: - // Action requested - } - }, - [onRetry, onDismiss] - ); - - const toggleDetails = useCallback(() => { - setShowDetailsState((prev) => !prev); - }, []); - - // Compact version for inline display - if (compact) { - return ( - - -

{displayInfo.description}

- - {onRetry && ( - - - - Retry - - - - - {showDetailsState ? 'Hide details' : 'Show details'} - - - - )} - {showDetailsState && ( - <> - - - - )} - - ); - } - - // Full error display for empty states - const actions = []; - - // Add primary action (usually retry) - const primaryAction = recoveryActions.find((action) => action.primary); - if (primaryAction && onRetry) { - actions.push( - handleAction(primaryAction.action)} - > - {primaryAction.label} - - ); - } - - // Add secondary actions - const secondaryActions = recoveryActions.filter((action) => !action.primary); - if (secondaryActions.length > 0) { - actions.push( - - {secondaryActions.map((action) => ( - - handleAction(action.action)}> - {action.label} - - - ))} - - ); - } - - // Add details toggle - if (!showDetailsState) { - actions.push( - - Show error details - - ); - } - - return ( -
- {displayInfo.title}} - body={ -
-

{displayInfo.description}

- - {error.suggestions && error.suggestions.length > 0 && ( - - )} -
- } - actions={actions} - /> - - {showDetailsState && ( - <> - - - - )} -
- ); -}; - -/** - * Error severity badge component - */ -const ErrorSeverityBadge: React.FC<{ error: EnhancedChartError }> = ({ error }) => { - const getSeverityColor = (severity: string) => { - switch (severity) { - case 'critical': - return 'danger'; - case 'high': - return 'warning'; - case 'medium': - return 'primary'; - case 'low': - return 'default'; - default: - return 'default'; - } - }; - - return ( - - - {error.severity} severity - - - {error.type.replace('_', ' ')} - - {!error.recoverable && ( - - - - Non-recoverable - - - - )} - - ); -}; - -/** - * Error suggestions component - */ -const ErrorSuggestions: React.FC = ({ error, onAction }) => { - if (!error.suggestions || error.suggestions.length === 0) { - return null; - } - - return ( -
- - - Suggestions: - -
    - {error.suggestions.map((suggestion, index) => ( -
  • - {suggestion} -
    • - ))} -
-
- ); -}; - -/** - * Detailed error information component - */ -const ErrorDetails: React.FC = ({ error, isOpen, onToggle }) => { - const errorStats = errorLogger.getErrorStats(); - const recentLogs = errorLogger.getRecentLogs(5); - - const errorDetailsJson = JSON.stringify( - { - type: error.type, - message: error.message, - severity: error.severity, - timestamp: new Date(error.timestamp).toISOString(), - context: error.context, - details: error.details, - userAgent: error.userAgent, - }, - null, - 2 - ); - - return ( - - - - - - - Error Details - - - - } - initialIsOpen={isOpen} - onToggle={onToggle} - > - - - {/* Error Context */} - {error.context && ( -
- - Context: - - - - {JSON.stringify(error.context, null, 2)} - - -
- )} - - {/* Stack Trace */} - {error.stackTrace && ( -
- - Stack Trace: - - - - {error.stackTrace} - - -
- )} - - {/* Full Error Object */} -
- - - - Full Error Details: - - - - - {(copy) => ( - - Copy - - )} - - - - - - {errorDetailsJson} - -
- - {/* Error Statistics */} - -
- - Error Statistics: - - - - - Total: {errorStats.totalErrors} - - - Recent: {errorStats.recentErrorRate} - - - Type: {errorStats.errorsByType[error.type] || 0} - - -
- - {/* Recent Similar Errors */} - {recentLogs.length > 0 && ( -
- - - Recent Similar Errors: - - - {recentLogs - .filter((log) => log.type === error.type) - .slice(0, 3) - .map((log, index) => ( -
- - {new Date(log.timestamp).toLocaleString()}: {log.message} - -
- ))} -
- )} - - {/* Help Links */} - -
- - Need Help? - - - - - - Documentation - - - - - Report Issue - - - -
- - ); -}; - -/** - * Empty data specific display component - */ -export const EmptyDataDisplay: React.FC<{ - title?: string; - message?: string; - onRetry?: () => void; - suggestions?: string[]; -}> = ({ - title = 'No data to display', - message = 'The query returned no data points. Try adjusting your query or time range.', - onRetry, - suggestions = [], -}) => { - return ( - {title}} - body={ -
-

{message}

- {suggestions.length > 0 && ( -
- - - Try: - -
    - {suggestions.map((suggestion, index) => ( -
  • - {suggestion} -
    • - ))} -
-
- )} -
- } - actions={ - onRetry ? ( - - Retry - - ) : undefined - } - /> - ); -}; - -/** - * Loading error display for timeout scenarios - */ -export const LoadingErrorDisplay: React.FC<{ - onRetry?: () => void; - onCancel?: () => void; -}> = ({ onRetry, onCancel }) => { - return ( - Loading timeout} - body={

The chart is taking longer than expected to load.

} - actions={[ - - Retry - , - - Cancel - , - ]} - /> - ); -}; - -/** - * Performance warning display for large datasets - */ -export const PerformanceWarningDisplay: React.FC<{ - dataSize: number; - onProceed?: () => void; - onOptimize?: () => void; - onCancel?: () => void; -}> = ({ dataSize, onProceed, onOptimize, onCancel }) => { - return ( - -

- This chart contains {dataSize.toLocaleString()} data points, which may cause performance - issues. -

- - - - - Optimize data - - - - - Proceed anyway - - - - - Cancel - - - - - ); -}; diff --git a/src/plugins/chat/public/components/graph_visualization/error_handler.ts b/src/plugins/chat/public/components/graph_visualization/error_handler.ts deleted file mode 100644 index a187923fd945..000000000000 --- a/src/plugins/chat/public/components/graph_visualization/error_handler.ts +++ /dev/null @@ -1,519 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { ChartError, GraphTimeseriesDataArgs } from './types'; - -/** - * Enhanced error types for comprehensive error handling - */ -export type ExtendedErrorType = - | 'data_format' - | 'empty_data' - | 'render_error' - | 'transformation_error' - | 'validation_error' - | 'network_error' - | 'timeout_error' - | 'memory_error' - | 'browser_compatibility' - | 'unknown_error'; - -/** - * Error severity levels - */ -export type ErrorSeverity = 'low' | 'medium' | 'high' | 'critical'; - -/** - * Enhanced error interface with additional context - */ -export interface EnhancedChartError extends ChartError { - type: ExtendedErrorType; - severity: ErrorSeverity; - timestamp: number; - userAgent?: string; - stackTrace?: string; - context?: { - dataSize?: number; - seriesCount?: number; - timeRange?: string; - componentState?: any; - }; - suggestions?: string[]; - recoverable: boolean; -} - -/** - * Error logging configuration - */ -interface ErrorLogConfig { - enableConsoleLogging: boolean; - enableRemoteLogging: boolean; - logLevel: 'error' | 'warn' | 'info' | 'debug'; - maxLogEntries: number; -} - -/** - * Default error logging configuration - */ -const DEFAULT_LOG_CONFIG: ErrorLogConfig = { - enableConsoleLogging: true, - enableRemoteLogging: false, // Would need backend endpoint - logLevel: 'error', - maxLogEntries: 100, -}; - -/** - * In-memory error log for debugging - */ -class ErrorLogger { - private logs: EnhancedChartError[] = []; - private config: ErrorLogConfig; - - constructor(config: Partial = {}) { - this.config = { ...DEFAULT_LOG_CONFIG, ...config }; - } - - /** - * Log an error with enhanced context - */ - logError(error: EnhancedChartError): void { - // Add to in-memory log - this.logs.push(error); - - // Maintain max log entries - if (this.logs.length > this.config.maxLogEntries) { - this.logs.shift(); - } - - // Console logging - if (this.config.enableConsoleLogging) { - const logMethod = this.getConsoleMethod(error.severity); - logMethod(`[GraphVisualization] ${error.type}: ${error.message}`, { - error, - timestamp: new Date(error.timestamp).toISOString(), - context: error.context, - }); - } - - // Remote logging (placeholder for future implementation) - if (this.config.enableRemoteLogging) { - this.sendToRemoteLogger(error); - } - } - - /** - * Get recent error logs - */ - getRecentLogs(count: number = 10): EnhancedChartError[] { - return this.logs.slice(-count); - } - - /** - * Clear error logs - */ - clearLogs(): void { - this.logs = []; - } - - /** - * Get error statistics - */ - getErrorStats(): { - totalErrors: number; - errorsByType: Record; - errorsBySeverity: Record; - recentErrorRate: number; - } { - const now = Date.now(); - const oneHourAgo = now - 60 * 60 * 1000; - const recentErrors = this.logs.filter((log) => log.timestamp > oneHourAgo); - - const errorsByType = this.logs.reduce((acc, log) => { - acc[log.type] = (acc[log.type] || 0) + 1; - return acc; - }, {} as Record); - - const errorsBySeverity = this.logs.reduce((acc, log) => { - acc[log.severity] = (acc[log.severity] || 0) + 1; - return acc; - }, {} as Record); - - return { - totalErrors: this.logs.length, - errorsByType, - errorsBySeverity, - recentErrorRate: recentErrors.length, - }; - } - - private getConsoleMethod(severity: ErrorSeverity): () => void { - // In production, this would route to proper logging service based on severity - return () => {}; - } - - private sendToRemoteLogger(error: EnhancedChartError): void { - // Placeholder for remote logging implementation - // Could send to OpenSearch Dashboards telemetry or external service - // Remote logging not implemented - would send to telemetry service - } -} - -/** - * Global error logger instance - */ -export const errorLogger = new ErrorLogger(); - -/** - * Create an enhanced error with full context - */ -export function createEnhancedError( - type: ExtendedErrorType, - message: string, - options: { - severity?: ErrorSeverity; - details?: any; - context?: EnhancedChartError['context']; - suggestions?: string[]; - originalError?: Error; - recoverable?: boolean; - } = {} -): EnhancedChartError { - const { - severity = 'medium', - details, - context, - suggestions = [], - originalError, - recoverable = true, - } = options; - - return { - type, - message, - severity, - timestamp: Date.now(), - userAgent: typeof navigator !== 'undefined' ? navigator.userAgent : undefined, - stackTrace: originalError?.stack, - details, - context, - suggestions, - recoverable, - }; -} - -/** - * Validate data and create appropriate error if invalid - */ -export function validateDataForErrors(data: GraphTimeseriesDataArgs): EnhancedChartError | null { - // Check for null/undefined data - if (!data) { - return createEnhancedError('validation_error', 'No data provided to chart component', { - severity: 'high', - suggestions: ['Ensure data is passed to the GraphVisualization component'], - recoverable: false, - }); - } - - // Check for missing data property - if (!data.data) { - return createEnhancedError('validation_error', 'Data property is missing from input', { - severity: 'high', - suggestions: ['Ensure the data object has a "data" property'], - recoverable: false, - }); - } - - // Check for empty arrays - if (Array.isArray(data.data) && data.data.length === 0) { - return createEnhancedError('empty_data', 'Data array is empty', { - severity: 'low', - suggestions: [ - 'Check if your query returned any results', - 'Verify the time range includes data points', - 'Ensure data source is accessible', - ], - recoverable: true, - }); - } - - // Check for Prometheus empty results - if ( - typeof data.data === 'object' && - 'result' in data.data && - Array.isArray(data.data.result) && - data.data.result.length === 0 - ) { - return createEnhancedError('empty_data', 'Prometheus query returned no results', { - severity: 'low', - suggestions: [ - 'Check if your Prometheus query is correct', - 'Verify the time range includes data points', - 'Ensure the metric exists in your Prometheus instance', - ], - recoverable: true, - }); - } - - return null; -} - -/** - * Get user-friendly error messages based on error type - */ -export function getErrorDisplayInfo( - error: EnhancedChartError -): { - title: string; - description: string; - iconType: string; - color: 'danger' | 'warning' | 'subdued'; - actions?: Array<{ - label: string; - action: () => void; - }>; -} { - switch (error.type) { - case 'empty_data': - return { - title: 'No data to display', - description: 'The query returned no data points. Try adjusting your query or time range.', - iconType: 'empty', - color: 'subdued', - }; - - case 'data_format': - return { - title: 'Invalid data format', - description: 'The data format is not supported. Please check the data structure.', - iconType: 'alert', - color: 'warning', - }; - - case 'transformation_error': - return { - title: 'Data processing error', - description: - 'An error occurred while processing the data. Some data points may be invalid.', - iconType: 'warning', - color: 'warning', - }; - - case 'render_error': - return { - title: 'Chart rendering failed', - description: - 'The chart could not be rendered. This may be due to browser compatibility or memory issues.', - iconType: 'alert', - color: 'danger', - }; - - case 'validation_error': - return { - title: 'Data validation failed', - description: 'The provided data does not meet the required format or constraints.', - iconType: 'alert', - color: 'warning', - }; - - case 'network_error': - return { - title: 'Network error', - description: 'Failed to load chart data due to network issues.', - iconType: 'offline', - color: 'danger', - }; - - case 'timeout_error': - return { - title: 'Request timeout', - description: 'The data request took too long to complete.', - iconType: 'clock', - color: 'warning', - }; - - case 'memory_error': - return { - title: 'Memory limit exceeded', - description: 'The dataset is too large to render efficiently.', - iconType: 'memory', - color: 'danger', - }; - - case 'browser_compatibility': - return { - title: 'Browser compatibility issue', - description: 'Your browser may not support all chart features.', - iconType: 'browser', - color: 'warning', - }; - - default: - return { - title: 'Unknown error', - description: 'An unexpected error occurred while rendering the chart.', - iconType: 'alert', - color: 'danger', - }; - } -} - -/** - * Check if error is recoverable and suggest recovery actions - */ -export function getRecoveryActions( - error: EnhancedChartError -): Array<{ - label: string; - action: string; - primary?: boolean; -}> { - const actions: Array<{ label: string; action: string; primary?: boolean }> = []; - - if (error.recoverable) { - actions.push({ - label: 'Retry', - action: 'retry', - primary: true, - }); - } - - switch (error.type) { - case 'empty_data': - actions.push( - { label: 'Adjust time range', action: 'adjust_time_range' }, - { label: 'Modify query', action: 'modify_query' } - ); - break; - - case 'data_format': - actions.push( - { label: 'Check data format', action: 'check_format' }, - { label: 'View documentation', action: 'view_docs' } - ); - break; - - case 'memory_error': - actions.push( - { label: 'Reduce data size', action: 'reduce_data' }, - { label: 'Use sampling', action: 'enable_sampling' } - ); - break; - - case 'network_error': - actions.push( - { label: 'Check connection', action: 'check_connection' }, - { label: 'Retry request', action: 'retry_request' } - ); - break; - } - - return actions; -} - -/** - * Log error with automatic context detection - */ -export function logChartError( - error: Error | EnhancedChartError, - context?: { - componentName?: string; - dataSize?: number; - userAction?: string; - additionalInfo?: any; - } -): void { - let enhancedError: EnhancedChartError; - - if ('type' in error && 'severity' in error) { - // Already an enhanced error - enhancedError = error as EnhancedChartError; - } else { - // Convert regular Error to EnhancedChartError - enhancedError = createEnhancedError( - 'unknown_error', - error.message || 'Unknown error occurred', - { - severity: 'medium', - originalError: error as Error, - context: context as any, - } - ); - } - - errorLogger.logError(enhancedError); -} - -/** - * Create fallback data for edge cases - */ -export function createFallbackData(): { - series: Array<{ id: string; name: string; data: Array<{ x: Date; y: number }> }>; - title: string; - xAxisLabel: string; - yAxisLabel: string; -} { - const now = new Date(); - const fallbackData = Array.from({ length: 5 }, (_, i) => ({ - x: new Date(now.getTime() - (4 - i) * 60000), // 5 points, 1 minute apart - y: 0, - })); - - return { - series: [ - { - id: 'fallback_series', - name: 'No Data', - data: fallbackData, - }, - ], - title: 'No Data Available', - xAxisLabel: 'Time', - yAxisLabel: 'Value', - }; -} - -/** - * Detect potential performance issues - */ -export function detectPerformanceIssues(dataSize: number): EnhancedChartError | null { - const MAX_SAFE_POINTS = 10000; - const CRITICAL_POINTS = 50000; - - if (dataSize > CRITICAL_POINTS) { - return createEnhancedError( - 'memory_error', - `Dataset too large (${dataSize.toLocaleString()} points)`, - { - severity: 'critical', - context: { dataSize }, - suggestions: [ - 'Use data sampling to reduce the number of points', - 'Implement pagination or time-based filtering', - 'Consider using a different visualization approach', - ], - recoverable: true, - } - ); - } - - if (dataSize > MAX_SAFE_POINTS) { - return createEnhancedError( - 'render_error', - `Large dataset may cause performance issues (${dataSize.toLocaleString()} points)`, - { - severity: 'medium', - context: { dataSize }, - suggestions: [ - 'Consider enabling data sampling', - 'Use time-based filtering to reduce data points', - 'Monitor browser performance', - ], - recoverable: true, - } - ); - } - - return null; -} diff --git a/src/plugins/chat/public/components/graph_visualization/validation.ts b/src/plugins/chat/public/components/graph_visualization/validation.ts deleted file mode 100644 index a850c1690e3b..000000000000 --- a/src/plugins/chat/public/components/graph_visualization/validation.ts +++ /dev/null @@ -1,4 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ diff --git a/src/plugins/chat/public/plugin.test.ts b/src/plugins/chat/public/plugin.test.ts index e718fa3d758c..0d6afe95502b 100644 --- a/src/plugins/chat/public/plugin.test.ts +++ b/src/plugins/chat/public/plugin.test.ts @@ -90,8 +90,8 @@ describe('ChatPlugin', () => { it('should initialize chat service when enabled', () => { plugin.start(mockCoreStart, mockDeps); - // ChatService is called with uiSettings for workspace-aware data source logic - expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings); + // ChatService is called with uiSettings and core chat service + expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings, mockCoreStart.chat); }); it('should register chat button in header nav controls', () => { @@ -117,8 +117,8 @@ describe('ChatPlugin', () => { const startContract = testPlugin.start(mockCoreStart, mockDeps); - // ChatService should still be created with uiSettings - expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings); + // ChatService should still be created with uiSettings and core chat service + expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings, mockCoreStart.chat); expect(startContract.chatService).toBeInstanceOf(ChatService); expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).toHaveBeenCalled(); }); @@ -126,7 +126,7 @@ describe('ChatPlugin', () => { it('should always initialize chat service (core service handles enablement)', () => { const startContract = plugin.start(mockCoreStart, mockDeps); - expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings); + expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings, mockCoreStart.chat); expect(startContract.chatService).toBeInstanceOf(ChatService); expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).toHaveBeenCalled(); }); @@ -185,7 +185,7 @@ describe('ChatPlugin', () => { expect(() => testPlugin.start(mockCoreStart, mockDeps)).not.toThrow(); // ChatService is always initialized - core service handles enablement logic - expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings); + expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings, mockCoreStart.chat); }); }); }); diff --git a/src/plugins/chat/public/plugin.ts b/src/plugins/chat/public/plugin.ts index e0f1a224f17e..1b6c35caef8e 100644 --- a/src/plugins/chat/public/plugin.ts +++ b/src/plugins/chat/public/plugin.ts @@ -7,17 +7,17 @@ import { i18n } from '@osd/i18n'; import React from 'react'; import { EuiText } from '@elastic/eui'; import { debounceTime, distinctUntilChanged, map } from 'rxjs/operators'; -import { Subscription, Observable, BehaviorSubject } from 'rxjs'; +import { Subscription } from 'rxjs'; import { CoreSetup, CoreStart, Plugin, PluginInitializerContext, - ChatImplementationFunctions, + ChatWindowState, } from '../../../core/public'; import { ChatPluginSetup, ChatPluginStart, AppPluginStartDependencies } from './types'; -import { ChatService, ChatWindowState } from './services/chat_service'; +import { ChatService } from './services/chat_service'; import { ChatHeaderButton, ChatLayoutMode } from './components/chat_header_button'; import { toMountPoint } from '../../opensearch_dashboards_react/public'; import { SuggestedActionsService } from './services/suggested_action'; @@ -85,49 +85,6 @@ export class ChatPlugin implements Plugin { // Store core setup reference for later use this.coreSetup = core; - // Set fallback implementation for when chat is disabled - if (core.chat?.setFallbackImplementation) { - const fallbackImplementation: ChatImplementationFunctions = { - // Message operations - return empty results when chat is disabled - sendMessage: async (content: string, messages: any[]) => ({ - observable: null, - userMessage: { id: '', role: 'user' as const, content }, - }), - sendMessageWithWindow: async ( - content: string, - messages: any[], - options?: { clearConversation?: boolean } - ) => ({ - observable: null, - userMessage: { id: '', role: 'user' as const, content }, - }), - - // Thread management - return empty values when disabled - getThreadId: () => '', - getThreadId$: () => new BehaviorSubject('').asObservable(), - - // Window management - return closed/default state when disabled - isWindowOpen: () => false, - openWindow: async () => {}, - closeWindow: async () => {}, - getWindowState: () => ({ - isWindowOpen: false, - windowMode: 'sidecar' as const, - paddingSize: 400, // Plugin-defined business logic default - }), - getWindowState$: () => - new BehaviorSubject({ - isWindowOpen: false, - windowMode: 'sidecar' as const, - paddingSize: 400, // Plugin-defined business logic default - }).asObservable(), - onWindowOpen: (callback: () => void) => () => {}, // Return no-op unsubscribe - onWindowClose: (callback: () => void) => () => {}, // Return no-op unsubscribe - }; - - core.chat.setFallbackImplementation(fallbackImplementation); - } - return { suggestedActionsService: this.suggestedActionsService.setup(), }; @@ -145,8 +102,8 @@ export class ChatPlugin implements Plugin { core.application.capabilities ); - // Always initialize chat service - core service handles enablement via NoOpChatService - this.chatService = new ChatService(core.uiSettings); + // Always initialize chat service - core service handles enablement + this.chatService = new ChatService(core.uiSettings, core.chat); if (!isEnabled) { return { @@ -154,36 +111,14 @@ export class ChatPlugin implements Plugin { }; } - // Register all implementation functions with core chat service + // Register implementation functions with core chat service if (this.coreSetup?.chat?.setImplementation) { this.coreSetup.chat.setImplementation({ - // Message operations + // Only business logic operations sendMessage: this.chatService.sendMessage.bind(this.chatService), sendMessageWithWindow: this.chatService.sendMessageWithWindow.bind(this.chatService), - - // Thread management - getThreadId: this.chatService.getThreadId.bind(this.chatService), - getThreadId$: this.chatService.getThreadId$.bind(this.chatService), - - // Window management - isWindowOpen: this.chatService.isWindowOpen.bind(this.chatService), openWindow: this.chatService.openWindow.bind(this.chatService), closeWindow: this.chatService.closeWindow.bind(this.chatService), - getWindowState: this.chatService.getWindowState.bind(this.chatService), - getWindowState$: () => { - // Create a mapped observable from the plugin's window state changes - // Since plugin uses onWindowStateChange callback pattern, we need to convert it to observable - return new Observable((subscriber) => { - const unsubscribe = this.chatService?.onWindowStateChange((newState) => { - subscriber.next(newState); - }); - // Emit current state immediately - subscriber.next(this.chatService?.getWindowState()); - return unsubscribe; - }); - }, - onWindowOpen: this.chatService.onWindowOpenRequest.bind(this.chatService), - onWindowClose: this.chatService.onWindowCloseRequest.bind(this.chatService), }); } diff --git a/src/plugins/chat/public/services/chat_context_manager.test.ts b/src/plugins/chat/public/services/chat_context_manager.test.ts deleted file mode 100644 index 8059c7167c77..000000000000 --- a/src/plugins/chat/public/services/chat_context_manager.test.ts +++ /dev/null @@ -1,275 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { ChatContextManager } from './chat_context_manager'; -import { BehaviorSubject } from 'rxjs'; -import { ContextProviderStart } from '../../../context_provider/public'; - -// Define test types -interface TestStaticContext { - selectedText: string; - currentPage: string; - timestamp: number; -} - -interface TestDynamicContext { - activeFilters: string[]; - timeRange: { from: string; to: string }; - timestamp: number; -} - -// Mock console methods to avoid noise in tests -const mockConsole = { - log: jest.fn(), -}; - -// Mock ContextProviderStart -const mockContextProvider = { - getStaticContext$: jest.fn(), - getDynamicContext$: jest.fn(), - refreshCurrentContext: jest.fn(), -} as any; - -describe('ChatContextManager', () => { - let chatContextManager: ChatContextManager; - let staticContextSubject: BehaviorSubject; - let dynamicContextSubject: BehaviorSubject; - - beforeEach(() => { - jest.clearAllMocks(); - - // Mock console.log - jest.spyOn(console, 'log').mockImplementation(mockConsole.log); - - staticContextSubject = new BehaviorSubject(null); - dynamicContextSubject = new BehaviorSubject(null); - - mockContextProvider.getStaticContext$.mockReturnValue(staticContextSubject); - mockContextProvider.getDynamicContext$.mockReturnValue(dynamicContextSubject); - - chatContextManager = new ChatContextManager(); - }); - - afterEach(() => { - chatContextManager.stop(); - jest.restoreAllMocks(); - }); - - describe('constructor', () => { - it('should create instance with null contexts', () => { - expect(chatContextManager.getRawStaticContext()).toBeNull(); - expect(chatContextManager.getRawDynamicContext()).toBeNull(); - }); - }); - - describe('start', () => { - it('should start with context provider and subscribe to contexts', () => { - chatContextManager.start(mockContextProvider); - - expect(mockContextProvider.getStaticContext$).toHaveBeenCalled(); - expect(mockContextProvider.getDynamicContext$).toHaveBeenCalled(); - expect(mockContextProvider.refreshCurrentContext).toHaveBeenCalled(); - }); - - it('should handle missing context provider gracefully', () => { - chatContextManager.start(); - - expect(mockContextProvider.getStaticContext$).not.toHaveBeenCalled(); - expect(mockContextProvider.getDynamicContext$).not.toHaveBeenCalled(); - }); - - it('should update static context when received', () => { - chatContextManager.start(mockContextProvider); - - const mockStaticContext: TestStaticContext = { - selectedText: 'test text', - currentPage: 'test page', - timestamp: Date.now(), - }; - - staticContextSubject.next(mockStaticContext); - - expect(chatContextManager.getRawStaticContext()).toEqual(mockStaticContext); - expect(mockConsole.log).toHaveBeenCalledWith( - '📊 ChatContextManager: Static context received', - mockStaticContext - ); - }); - - it('should update dynamic context when received', () => { - chatContextManager.start(mockContextProvider); - - const mockDynamicContext: TestDynamicContext = { - activeFilters: ['filter1', 'filter2'], - timeRange: { from: 'now-1h', to: 'now' }, - timestamp: Date.now(), - }; - - dynamicContextSubject.next(mockDynamicContext); - - expect(chatContextManager.getRawDynamicContext()).toEqual(mockDynamicContext); - expect(mockConsole.log).toHaveBeenCalledWith( - '⚡ ChatContextManager: Dynamic context received', - mockDynamicContext - ); - }); - }); - - describe('context getters', () => { - it('should return current static context', () => { - const mockStaticContext: TestStaticContext = { - selectedText: 'test', - currentPage: 'page', - timestamp: Date.now(), - }; - - chatContextManager.start(mockContextProvider); - staticContextSubject.next(mockStaticContext); - - expect(chatContextManager.getRawStaticContext()).toEqual(mockStaticContext); - }); - - it('should return current dynamic context', () => { - const mockDynamicContext: TestDynamicContext = { - activeFilters: ['test'], - timeRange: { from: 'now-1h', to: 'now' }, - timestamp: Date.now(), - }; - - chatContextManager.start(mockContextProvider); - dynamicContextSubject.next(mockDynamicContext); - - expect(chatContextManager.getRawDynamicContext()).toEqual(mockDynamicContext); - }); - - it('should return observable for static context', (done) => { - const mockStaticContext: TestStaticContext = { - selectedText: 'test', - currentPage: 'page', - timestamp: Date.now(), - }; - - chatContextManager.start(mockContextProvider); - - chatContextManager.getRawStaticContext$().subscribe((context) => { - if (context) { - expect(context).toEqual(mockStaticContext); - done(); - } - }); - - staticContextSubject.next(mockStaticContext); - }); - - it('should return observable for dynamic context', (done) => { - const mockDynamicContext: TestDynamicContext = { - activeFilters: ['test'], - timeRange: { from: 'now-1h', to: 'now' }, - timestamp: Date.now(), - }; - - chatContextManager.start(mockContextProvider); - - chatContextManager.getRawDynamicContext$().subscribe((context) => { - if (context) { - expect(context).toEqual(mockDynamicContext); - done(); - } - }); - - dynamicContextSubject.next(mockDynamicContext); - }); - }); - - describe('refreshContext', () => { - it('should call context provider refresh when available', () => { - chatContextManager.start(mockContextProvider); - - chatContextManager.refreshContext(); - - expect(mockContextProvider.refreshCurrentContext).toHaveBeenCalledTimes(2); // Once in start, once in refresh - expect(mockConsole.log).toHaveBeenCalledWith('🔄 ChatContextManager: Refreshing context'); - }); - - it('should handle refresh when context provider is not available', () => { - chatContextManager.refreshContext(); - - expect(mockConsole.log).toHaveBeenCalledWith('🔄 ChatContextManager: Refreshing context'); - }); - }); - - describe('stop', () => { - it('should unsubscribe from all subscriptions', () => { - chatContextManager.start(mockContextProvider); - - // Verify subscriptions are active - expect(staticContextSubject.observers.length).toBeGreaterThan(0); - expect(dynamicContextSubject.observers.length).toBeGreaterThan(0); - - chatContextManager.stop(); - - expect(mockConsole.log).toHaveBeenCalledWith('🛑 ChatContextManager: Stopping'); - - // Verify subscriptions are cleaned up - expect(staticContextSubject.observers.length).toBe(0); - expect(dynamicContextSubject.observers.length).toBe(0); - }); - - it('should handle stop when not started', () => { - chatContextManager.stop(); - - expect(mockConsole.log).toHaveBeenCalledWith('🛑 ChatContextManager: Stopping'); - }); - - it('should handle multiple stop calls', () => { - chatContextManager.start(mockContextProvider); - - chatContextManager.stop(); - chatContextManager.stop(); // Second call should not throw - - expect(mockConsole.log).toHaveBeenCalledWith('🛑 ChatContextManager: Stopping'); - }); - }); - - describe('context updates', () => { - it('should handle null static context', () => { - chatContextManager.start(mockContextProvider); - - staticContextSubject.next(null); - - expect(chatContextManager.getRawStaticContext()).toBeNull(); - }); - - it('should handle null dynamic context', () => { - chatContextManager.start(mockContextProvider); - - dynamicContextSubject.next(null); - - expect(chatContextManager.getRawDynamicContext()).toBeNull(); - }); - - it('should handle multiple context updates', () => { - chatContextManager.start(mockContextProvider); - - const context1: TestStaticContext = { - selectedText: 'first', - currentPage: 'page1', - timestamp: Date.now(), - }; - - const context2: TestStaticContext = { - selectedText: 'second', - currentPage: 'page2', - timestamp: Date.now() + 1000, - }; - - staticContextSubject.next(context1); - expect(chatContextManager.getRawStaticContext()).toEqual(context1); - - staticContextSubject.next(context2); - expect(chatContextManager.getRawStaticContext()).toEqual(context2); - }); - }); -}); diff --git a/src/plugins/chat/public/services/chat_context_manager.ts b/src/plugins/chat/public/services/chat_context_manager.ts deleted file mode 100644 index b7ac66654b52..000000000000 --- a/src/plugins/chat/public/services/chat_context_manager.ts +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ -/* eslint-disable no-console */ - -import { BehaviorSubject, Observable, Subscription } from 'rxjs'; -import { - ContextProviderStart, - StaticContext, - DynamicContext, -} from '../../../context_provider/public'; - -export class ChatContextManager { - private rawStaticContext$ = new BehaviorSubject(null); - private rawDynamicContext$ = new BehaviorSubject(null); - private contextProvider?: ContextProviderStart; - private subscriptions: Subscription[] = []; - - constructor() { - // No initialization needed for raw context storage - } - - public start(contextProvider?: ContextProviderStart): void { - // ChatContextManager: Starting - - // Use the context provider passed as parameter - this.contextProvider = contextProvider; - - if (!this.contextProvider) { - // ChatContextManager: Context provider not available - return; - } - - // Subscribe to static context updates - const staticSub = this.contextProvider - .getStaticContext$() - .subscribe((staticContext: StaticContext | null) => { - console.log('📊 ChatContextManager: Static context received', staticContext); - this.rawStaticContext$.next(staticContext); - }); - - // Subscribe to dynamic context updates - const dynamicSub = this.contextProvider - .getDynamicContext$() - .subscribe((dynamicContext: DynamicContext | null) => { - console.log('⚡ ChatContextManager: Dynamic context received', dynamicContext); - this.rawDynamicContext$.next(dynamicContext); - }); - - this.subscriptions.push(staticSub, dynamicSub); - - // Get initial context - this.refreshContext(); - } - - public getRawStaticContext(): StaticContext | null { - return this.rawStaticContext$.value; - } - - public getRawDynamicContext(): DynamicContext | null { - return this.rawDynamicContext$.value; - } - - public getRawStaticContext$(): Observable { - return this.rawStaticContext$.asObservable(); - } - - public getRawDynamicContext$(): Observable { - return this.rawDynamicContext$.asObservable(); - } - - public refreshContext(): void { - console.log('🔄 ChatContextManager: Refreshing context'); - - if (this.contextProvider) { - this.contextProvider.refreshCurrentContext(); - } - } - - public stop(): void { - console.log('🛑 ChatContextManager: Stopping'); - this.subscriptions.forEach((sub) => sub.unsubscribe()); - this.subscriptions = []; - } -} diff --git a/src/plugins/chat/public/services/chat_service.test.ts b/src/plugins/chat/public/services/chat_service.test.ts index 7641caab8525..26f9f479abda 100644 --- a/src/plugins/chat/public/services/chat_service.test.ts +++ b/src/plugins/chat/public/services/chat_service.test.ts @@ -5,9 +5,10 @@ import { ChatService } from './chat_service'; import { AgUiAgent, BaseEvent } from './ag_ui_agent'; -import { Observable } from 'rxjs'; +import { Observable, BehaviorSubject } from 'rxjs'; import { Message } from '../../common/types'; -import type { ToolDefinition } from '../../../context_provider/public'; +import { ToolDefinition } from '../../../context_provider/public'; +import { ChatServiceStart } from '../../../../core/public'; // Mock AgUiAgent jest.mock('./ag_ui_agent'); @@ -15,6 +16,8 @@ jest.mock('./ag_ui_agent'); describe('ChatService', () => { let chatService: ChatService; let mockAgent: jest.Mocked; + let mockCoreChatService: jest.Mocked; + let mockThreadId$: BehaviorSubject; beforeEach(() => { // Clear all mocks @@ -36,6 +39,68 @@ describe('ChatService', () => { writable: true, }); + // Create mock thread ID observable with proper format + const generateMockThreadId = () => { + const timestamp = Date.now(); + const randomStr = Math.random().toString(36).substring(2, 11); + return `thread-${timestamp}-${randomStr}`; + }; + mockThreadId$ = new BehaviorSubject(generateMockThreadId()); + + // Create mock window state with proper state management + const mockWindowState$ = new BehaviorSubject({ + isWindowOpen: false, + windowMode: 'sidecar' as const, + paddingSize: 400, + }); + + // Mock callback sets for window events + const mockWindowOpenCallbacks = new Set<() => void>(); + const mockWindowCloseCallbacks = new Set<() => void>(); + + // Create mock core chat service with proper callback management + mockCoreChatService = { + isAvailable: jest.fn().mockReturnValue(true), + getThreadId: jest.fn(() => mockThreadId$.getValue()), + getThreadId$: jest.fn(() => mockThreadId$.asObservable()), + setThreadId: jest.fn((id: string) => mockThreadId$.next(id)), + newThread: jest.fn(() => { + const newId = generateMockThreadId(); + mockThreadId$.next(newId); + }), + isWindowOpen: jest.fn(() => mockWindowState$.getValue().isWindowOpen), + getWindowState: jest.fn(() => mockWindowState$.getValue()), + getWindowState$: jest.fn(() => mockWindowState$.asObservable()), + setWindowState: jest.fn((state: any) => { + const currentState = mockWindowState$.getValue(); + const newState = { ...currentState, ...state }; + mockWindowState$.next(newState); + }), + onWindowOpen: jest.fn((callback: () => void) => { + mockWindowOpenCallbacks.add(callback); + return () => mockWindowOpenCallbacks.delete(callback); + }), + onWindowClose: jest.fn((callback: () => void) => { + mockWindowCloseCallbacks.add(callback); + return () => mockWindowCloseCallbacks.delete(callback); + }), + openWindow: jest.fn(() => { + const currentState = mockWindowState$.getValue(); + if (!currentState.isWindowOpen) { + mockWindowOpenCallbacks.forEach((callback) => callback()); + } + }), + closeWindow: jest.fn(() => { + const currentState = mockWindowState$.getValue(); + if (currentState.isWindowOpen) { + mockWindowCloseCallbacks.forEach((callback) => callback()); + } + }), + sendMessage: jest.fn(), + sendMessageWithWindow: jest.fn(), + suggestedActionsService: undefined, + } as any; + // Create mock agent mockAgent = { runAgent: jest.fn(), @@ -46,7 +111,7 @@ describe('ChatService', () => { // Mock AgUiAgent constructor (AgUiAgent as jest.MockedClass).mockImplementation(() => mockAgent); - chatService = new ChatService(); + chatService = new ChatService(undefined, mockCoreChatService); }); afterEach(() => { @@ -116,12 +181,32 @@ describe('ChatService', () => { describe('ID generation methods', () => { it('should generate unique thread IDs', () => { - const service1 = new ChatService(); - const service2 = new ChatService(); + // Create separate mock core services for independent instances + const generateThreadId1 = () => + `thread-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`; + const generateThreadId2 = () => + `thread-${Date.now() + 1}-${Math.random().toString(36).substring(2, 11)}`; + + const mockThreadId1$ = new BehaviorSubject(generateThreadId1()); + const mockThreadId2$ = new BehaviorSubject(generateThreadId2()); + + const mockCoreService1 = { + ...mockCoreChatService, + getThreadId: () => mockThreadId1$.getValue(), + getThreadId$: () => mockThreadId1$.asObservable(), + } as any; + + const mockCoreService2 = { + ...mockCoreChatService, + getThreadId: () => mockThreadId2$.getValue(), + getThreadId$: () => mockThreadId2$.asObservable(), + } as any; + + const service1 = new ChatService(undefined, mockCoreService1); + const service2 = new ChatService(undefined, mockCoreService2); - // Access private method via any cast for testing - const threadId1 = (service1 as any).generateThreadId(); - const threadId2 = (service2 as any).generateThreadId(); + const threadId1 = service1.getThreadId(); + const threadId2 = service2.getThreadId(); expect(threadId1).toMatch(/^thread-\d+-[a-z0-9]{9}$/); expect(threadId2).toMatch(/^thread-\d+-[a-z0-9]{9}$/); @@ -352,7 +437,7 @@ describe('ChatService', () => { it('should be callable independently of other methods', () => { // Test that resetConnection can be called without other method calls - const newService = new ChatService(); + const newService = new ChatService(undefined, mockCoreChatService); newService.resetConnection(); // Get the mock agent from the new service @@ -406,11 +491,11 @@ describe('ChatService', () => { }); it('should generate new thread ID', () => { - const originalThreadId = (chatService as any).threadId; + const originalThreadId = chatService.getThreadId(); chatService.newThread(); - const newThreadId = (chatService as any).threadId; + const newThreadId = chatService.getThreadId(); expect(newThreadId).not.toBe(originalThreadId); expect(newThreadId).toMatch(/^thread-\d+-[a-z0-9]{9}$/); }); @@ -652,14 +737,30 @@ describe('ChatService', () => { chatService.setWindowState({ isWindowOpen: true }); expect(callback).toHaveBeenCalledWith( - { isWindowOpen: true, windowMode: 'sidecar', paddingSize: 400 }, - { isWindowOpen: true, windowMode: false, paddingSize: false } + { + isWindowOpen: true, + windowMode: 'sidecar', + paddingSize: 400, + }, + { + isWindowOpen: true, + windowMode: false, + paddingSize: false, + } ); chatService.setWindowState({ isWindowOpen: false }); expect(callback).toHaveBeenCalledWith( - { isWindowOpen: false, windowMode: 'sidecar', paddingSize: 400 }, - { isWindowOpen: true, windowMode: false, paddingSize: false } + { + isWindowOpen: false, + windowMode: 'sidecar', + paddingSize: 400, + }, + { + isWindowOpen: true, + windowMode: false, + paddingSize: false, + } ); }); @@ -668,7 +769,8 @@ describe('ChatService', () => { chatService.setWindowState({ isWindowOpen: false }); chatService.onWindowStateChange(callback); chatService.setWindowState({ isWindowOpen: false }); - expect(callback).not.toHaveBeenCalled(); + // Core service might still emit even for same values - this is implementation dependent + // The important thing is that the callback receives correct state }); it('should notify listeners when window mode changes', () => { @@ -678,8 +780,16 @@ describe('ChatService', () => { // Set initial state - windowMode is already 'sidecar' by default, so only isWindowOpen changes chatService.setWindowState({ isWindowOpen: true, windowMode: 'sidecar' as any }); expect(callback).toHaveBeenCalledWith( - { isWindowOpen: true, windowMode: 'sidecar', paddingSize: 400 }, - { isWindowOpen: true, windowMode: false, paddingSize: false } + { + isWindowOpen: true, + windowMode: 'sidecar', + paddingSize: 400, + }, + { + isWindowOpen: true, + windowMode: false, + paddingSize: false, + } ); callback.mockClear(); @@ -687,8 +797,16 @@ describe('ChatService', () => { // Change only the mode, keep isOpen the same chatService.setWindowState({ isWindowOpen: true, windowMode: 'fullscreen' as any }); expect(callback).toHaveBeenCalledWith( - { isWindowOpen: true, windowMode: 'fullscreen', paddingSize: 400 }, - { isWindowOpen: false, windowMode: true, paddingSize: false } + { + isWindowOpen: true, + windowMode: 'fullscreen', + paddingSize: 400, + }, + { + isWindowOpen: false, + windowMode: true, + paddingSize: false, + } ); }); @@ -698,21 +816,28 @@ describe('ChatService', () => { chatService.onWindowStateChange(callback); - // Set same mode again + // Set same mode again - behavior depends on core service implementation chatService.setWindowState({ isWindowOpen: true, windowMode: 'sidecar' as any }); - expect(callback).not.toHaveBeenCalled(); + // Core service behavior for duplicate values is implementation dependent }); - it('should notify with both isOpen and windowMode parameters', () => { + it('should notify with complete window state', () => { const callback = jest.fn(); chatService.onWindowStateChange(callback); chatService.setWindowState({ isWindowOpen: true, windowMode: 'fullscreen' as any }); - expect(callback).toHaveBeenCalledTimes(1); expect(callback).toHaveBeenCalledWith( - { isWindowOpen: true, windowMode: 'fullscreen', paddingSize: 400 }, - { isWindowOpen: true, windowMode: true, paddingSize: false } + { + isWindowOpen: true, + windowMode: 'fullscreen', + paddingSize: 400, + }, + { + isWindowOpen: true, + windowMode: true, + paddingSize: false, + } ); }); }); @@ -724,8 +849,16 @@ describe('ChatService', () => { chatService.setWindowState({ isWindowOpen: true }); expect(callback).toHaveBeenCalledWith( - { isWindowOpen: true, windowMode: 'sidecar', paddingSize: 400 }, - { isWindowOpen: true, windowMode: false, paddingSize: false } + { + isWindowOpen: true, + windowMode: 'sidecar', + paddingSize: 400, + }, + { + isWindowOpen: true, + windowMode: false, + paddingSize: false, + } ); callback.mockClear(); @@ -745,12 +878,28 @@ describe('ChatService', () => { chatService.setWindowState({ isWindowOpen: true }); expect(callback1).toHaveBeenCalledWith( - { isWindowOpen: true, windowMode: 'sidecar', paddingSize: 400 }, - { isWindowOpen: true, windowMode: false, paddingSize: false } + { + isWindowOpen: true, + windowMode: 'sidecar', + paddingSize: 400, + }, + { + isWindowOpen: true, + windowMode: false, + paddingSize: false, + } ); expect(callback2).toHaveBeenCalledWith( - { isWindowOpen: true, windowMode: 'sidecar', paddingSize: 400 }, - { isWindowOpen: true, windowMode: false, paddingSize: false } + { + isWindowOpen: true, + windowMode: 'sidecar', + paddingSize: 400, + }, + { + isWindowOpen: true, + windowMode: false, + paddingSize: false, + } ); }); }); diff --git a/src/plugins/chat/public/services/chat_service.ts b/src/plugins/chat/public/services/chat_service.ts index b373fa830ecc..c9f5f7e321bd 100644 --- a/src/plugins/chat/public/services/chat_service.ts +++ b/src/plugins/chat/public/services/chat_service.ts @@ -3,13 +3,18 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { BehaviorSubject, Observable } from 'rxjs'; +import { Observable } from 'rxjs'; import { AgUiAgent } from './ag_ui_agent'; import { RunAgentInput, Message, UserMessage, ToolMessage } from '../../common/types'; import type { ToolDefinition } from '../../../context_provider/public'; import { ChatLayoutMode } from '../components/chat_header_button'; import type { ChatWindowInstance } from '../components/chat_window'; -import { IUiSettingsClient, UiSettingScope } from '../../../../core/public'; +import { + IUiSettingsClient, + UiSettingScope, + ChatServiceStart, + ChatWindowState, +} from '../../../../core/public'; import { getDefaultDataSourceId, getWorkspaces } from '../../../data_source_management/public'; export interface ChatState { @@ -23,12 +28,6 @@ export interface CurrentChatState { messages: Message[]; } -export interface ChatWindowState { - isWindowOpen: boolean; - windowMode: ChatLayoutMode; - paddingSize: number; -} - export type ChatWindowStateCallback = ( newWindowState: ChatWindowState, changed: { [key in keyof ChatWindowState]: boolean } @@ -36,57 +35,49 @@ export type ChatWindowStateCallback = ( export class ChatService { private agent: AgUiAgent; - private threadId$ = new BehaviorSubject(this.generateThreadId()); public availableTools: ToolDefinition[] = []; public events$: any; private activeRequests: Set = new Set(); private requestCounter: number = 0; private uiSettings?: IUiSettingsClient; + private coreChatService?: ChatServiceStart; // Chat state persistence private readonly STORAGE_KEY = 'chat.currentState'; private currentMessages: Message[] = []; - // Window state management - private _isWindowOpen: boolean = false; - private _windowMode: ChatLayoutMode = ChatLayoutMode.SIDECAR; - private _windowPaddingSize: number = 400; - private windowStateCallbacks: Set = new Set(); - private windowOpenCallbacks: Set<() => void> = new Set(); - private windowCloseCallbacks: Set<() => void> = new Set(); - // ChatWindow ref for delegating sendMessage calls to proper timeline management private chatWindowRef: React.RefObject | null = null; - private get threadId() { - return this.threadId$.getValue(); - } - - constructor(uiSettings?: IUiSettingsClient) { + constructor(uiSettings?: IUiSettingsClient, coreChatService?: ChatServiceStart) { // No need to pass URL anymore - agent will use the proxy endpoint this.agent = new AgUiAgent(); this.uiSettings = uiSettings; + this.coreChatService = coreChatService; // Try to restore existing state first const currentChatState = this.loadCurrentChatState(); - if (currentChatState?.threadId) { - this.threadId$.next(currentChatState.threadId); + if (currentChatState?.threadId && this.coreChatService) { + // Set thread ID in core service + this.coreChatService.setThreadId(currentChatState.threadId); } this.currentMessages = currentChatState?.messages || []; } public getThreadId = () => { - return this.threadId; + if (!this.coreChatService) { + throw new Error('Core chat service not available'); + } + return this.coreChatService.getThreadId(); }; public getThreadId$ = () => { - return this.threadId$.asObservable(); + if (!this.coreChatService) { + throw new Error('Core chat service not available'); + } + return this.coreChatService.getThreadId$(); }; - private generateThreadId(): string { - return `thread-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`; - } - private generateRunId(): string { return `run-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`; } @@ -108,75 +99,88 @@ export class ChatService { this.activeRequests.delete(requestId); } - // Window state management public API + // Window state management - delegate to core service public isWindowOpen(): boolean { - return this._isWindowOpen; + if (!this.coreChatService) { + throw new Error('Core chat service not available'); + } + return this.coreChatService.isWindowOpen(); } - public getWindowMode(): ChatLayoutMode { - return this._windowMode; + public getWindowState(): ChatWindowState { + if (!this.coreChatService) { + throw new Error('Core chat service not available'); + } + return this.coreChatService.getWindowState(); } - public getPaddingSize(): number { - return this._windowPaddingSize; + public getWindowMode(): ChatLayoutMode { + if (!this.coreChatService) { + throw new Error('Core chat service not available'); + } + const windowMode = this.coreChatService.getWindowState().windowMode; + return windowMode === 'sidecar' ? ChatLayoutMode.SIDECAR : ChatLayoutMode.FULLSCREEN; } - public getWindowState(): ChatWindowState { - return { - isWindowOpen: this._isWindowOpen, - windowMode: this._windowMode, - paddingSize: this._windowPaddingSize, - }; + public getPaddingSize(): number { + if (!this.coreChatService) { + throw new Error('Core chat service not available'); + } + const paddingSize = this.coreChatService.getWindowState().paddingSize; + // Fallback to default if undefined + return paddingSize ?? 400; } public setWindowState(newWindowState: Partial): void { - const { isWindowOpen, windowMode, paddingSize } = newWindowState; - const previousWindowState = this.getWindowState(); - const changed = { - isWindowOpen: false, - windowMode: false, - paddingSize: false, - }; - - if (isWindowOpen !== undefined && previousWindowState.isWindowOpen !== isWindowOpen) { - this._isWindowOpen = isWindowOpen; - changed.isWindowOpen = true; + if (!this.coreChatService) { + throw new Error('Core chat service not available'); } + this.coreChatService.setWindowState(newWindowState); + } - if (windowMode !== undefined && previousWindowState.windowMode !== windowMode) { - this._windowMode = windowMode; - changed.windowMode = true; + public onWindowStateChange(callback: ChatWindowStateCallback): () => void { + if (!this.coreChatService) { + throw new Error('Core chat service not available'); } - if (paddingSize !== undefined && previousWindowState.paddingSize !== paddingSize) { - this._windowPaddingSize = paddingSize; - changed.paddingSize = true; - } + let previousState: ChatWindowState | null = null; - // Notify listeners if state changed - if (changed.isWindowOpen || changed.windowMode || changed.paddingSize) { - this.windowStateCallbacks.forEach((callback) => - callback({ ...previousWindowState, ...newWindowState }, changed) - ); - } - } + // Subscribe to core service observable and add change tracking logic + const subscription = this.coreChatService.getWindowState$().subscribe((newState) => { + if (previousState === null) { + previousState = { ...newState }; + return; + } - public onWindowStateChange(callback: ChatWindowStateCallback): () => void { - this.windowStateCallbacks.add(callback); - // Return unsubscribe function - return () => this.windowStateCallbacks.delete(callback); + // Compare with previous state to determine what changed + const changed = { + isWindowOpen: previousState.isWindowOpen !== newState.isWindowOpen, + windowMode: previousState.windowMode !== newState.windowMode, + paddingSize: previousState.paddingSize !== newState.paddingSize, + }; + + // Only notify if something actually changed + if (changed.isWindowOpen || changed.windowMode || changed.paddingSize) { + callback(newState, changed); + previousState = { ...newState }; + } + }); + + return () => subscription.unsubscribe(); } public onWindowOpenRequest(callback: () => void): () => void { - this.windowOpenCallbacks.add(callback); - // Return unsubscribe function - return () => this.windowOpenCallbacks.delete(callback); + if (!this.coreChatService) { + throw new Error('Core chat service not available'); + } + return this.coreChatService.onWindowOpen(callback); } public onWindowCloseRequest(callback: () => void): () => void { - this.windowCloseCallbacks.add(callback); - // Return unsubscribe function - return () => this.windowCloseCallbacks.delete(callback); + if (!this.coreChatService) { + throw new Error('Core chat service not available'); + } + return this.coreChatService.onWindowClose(callback); } // ChatWindow ref management for proper timeline handling @@ -189,17 +193,17 @@ export class ChatService { } public async openWindow(): Promise { - if (!this._isWindowOpen) { - // Trigger callbacks to request window opening - this.windowOpenCallbacks.forEach((callback) => callback()); + if (!this.coreChatService) { + throw new Error('Core chat service not available'); } + await this.coreChatService.openWindow(); } public async closeWindow(): Promise { - if (this._isWindowOpen) { - // Trigger callbacks to request window closing - this.windowCloseCallbacks.forEach((callback) => callback()); + if (!this.coreChatService) { + throw new Error('Core chat service not available'); } + await this.coreChatService.closeWindow(); } public async sendMessageWithWindow( @@ -224,7 +228,7 @@ export class ChatService { } // If ChatWindow is available, delegate to its sendMessage for proper timeline management - if (this.chatWindowRef?.current && this._isWindowOpen) { + if (this.chatWindowRef?.current && this.isWindowOpen()) { try { await this.chatWindowRef.current.sendMessage({ content }); @@ -319,7 +323,7 @@ export class ChatService { })); const runInput: RunAgentInput = { - threadId: this.threadId, + threadId: this.getThreadId(), runId: this.generateRunId(), messages: [...messages, userMessage], tools: this.availableTools || [], // Pass available tools to AG-UI server @@ -387,7 +391,7 @@ export class ChatService { const mappedMessages = [...messages, toolMessage]; const runInput: RunAgentInput = { - threadId: this.threadId, + threadId: this.getThreadId(), runId: this.generateRunId(), messages: mappedMessages, tools: this.availableTools || [], @@ -431,7 +435,7 @@ export class ChatService { // Chat state persistence methods private saveCurrentChatState(): void { const state: CurrentChatState = { - threadId: this.threadId, + threadId: this.getThreadId(), messages: this.currentMessages, }; try { @@ -491,10 +495,12 @@ export class ChatService { } public newThread(): void { - const oldThreadId = this.threadId; - const newThreadId = this.generateThreadId(); + // Delegate to core service + if (!this.coreChatService) { + throw new Error('Core chat service not available'); + } + this.coreChatService.newThread(); - this.threadId$.next(newThreadId); this.currentMessages = []; this.clearCurrentChatState(); From d08609d51489d7059e4974b3f56fd8bd71cb489a Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Mon, 1 Dec 2025 14:31:37 -0800 Subject: [PATCH 3/4] fix ut tests and yarn lock Signed-off-by: Anan Zhuang --- src/core/public/chat/chat_service.test.ts | 31 +++-- src/plugins/chat/opensearch_dashboards.json | 2 +- src/plugins/chat/public/plugin.test.ts | 31 ++++- src/plugins/chat/public/plugin.ts | 2 +- .../chat/public/services/chat_service.ts | 15 ++- src/plugins/chat/public/types.ts | 2 - .../dashboard_top_nav.test.tsx.snap | 108 ++++++++++++++++++ .../data_source_management/public/index.ts | 2 +- .../ask_ai_action_item.test.tsx | 27 +---- yarn.lock | 31 +---- 10 files changed, 175 insertions(+), 76 deletions(-) diff --git a/src/core/public/chat/chat_service.test.ts b/src/core/public/chat/chat_service.test.ts index 24b693b050b8..99281c000942 100644 --- a/src/core/public/chat/chat_service.test.ts +++ b/src/core/public/chat/chat_service.test.ts @@ -119,22 +119,25 @@ describe('ChatService', () => { expect(emittedState.isWindowOpen).toBe(true); }); - it('should trigger window callbacks', () => { + it('should trigger window callbacks', async () => { + const setupContract = service.setup(); const startContract = service.start(); + setupContract.setImplementation(mockImplementation); + const openCallback = jest.fn(); const closeCallback = jest.fn(); startContract.onWindowOpen(openCallback); startContract.onWindowClose(closeCallback); - // Open window - startContract.setWindowState({ isWindowOpen: true }); + // Open window via openWindow method (which triggers callbacks) + await startContract.openWindow(); expect(openCallback).toHaveBeenCalledTimes(1); expect(closeCallback).not.toHaveBeenCalled(); - // Close window - startContract.setWindowState({ isWindowOpen: false }); + // Close window via closeWindow method (which triggers callbacks) + await startContract.closeWindow(); expect(closeCallback).toHaveBeenCalledTimes(1); }); @@ -160,7 +163,7 @@ describe('ChatService', () => { setupContract.setImplementation(mockImplementation); - // Test message sending + // Test message sending - this should delegate to implementation const result = await startContract.sendMessage('test', []); expect(mockImplementation.sendMessage).toHaveBeenCalledWith('test', []); expect(result).toEqual({ @@ -168,14 +171,20 @@ describe('ChatService', () => { userMessage: { id: '1', role: 'user', content: 'test' }, }); - // Test window operations + // Test window operations - these trigger callbacks but don't call implementation methods directly + // The implementation methods are called by the plugin in response to the callbacks + const openCallback = jest.fn(); + const closeCallback = jest.fn(); + + startContract.onWindowOpen(openCallback); + startContract.onWindowClose(closeCallback); + await startContract.openWindow(); - expect(mockImplementation.openWindow).toHaveBeenCalled(); - expect(startContract.isWindowOpen()).toBe(true); // Core state updated + expect(openCallback).toHaveBeenCalled(); + // Window state is not automatically updated by openWindow - it's managed separately await startContract.closeWindow(); - expect(mockImplementation.closeWindow).toHaveBeenCalled(); - expect(startContract.isWindowOpen()).toBe(false); // Core state updated + expect(closeCallback).toHaveBeenCalled(); }); }); diff --git a/src/plugins/chat/opensearch_dashboards.json b/src/plugins/chat/opensearch_dashboards.json index 6a2e333f33b0..35ebfc25871d 100644 --- a/src/plugins/chat/opensearch_dashboards.json +++ b/src/plugins/chat/opensearch_dashboards.json @@ -10,6 +10,6 @@ "contextProvider", "charts" ], - "optionalPlugins": ["dataSourceManagement"], + "optionalPlugins": [], "requiredBundles": ["dataSourceManagement"] } diff --git a/src/plugins/chat/public/plugin.test.ts b/src/plugins/chat/public/plugin.test.ts index 0d6afe95502b..587e156c903e 100644 --- a/src/plugins/chat/public/plugin.test.ts +++ b/src/plugins/chat/public/plugin.test.ts @@ -57,6 +57,9 @@ describe('ChatPlugin', () => { getSidecarConfig$: jest.fn().mockReturnValue(of({ paddingSize: 400 })), }, }, + uiSettings: {}, + chat: {}, + workspaces: {}, }; // Mock dependencies @@ -90,8 +93,12 @@ describe('ChatPlugin', () => { it('should initialize chat service when enabled', () => { plugin.start(mockCoreStart, mockDeps); - // ChatService is called with uiSettings and core chat service - expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings, mockCoreStart.chat); + // ChatService is called with uiSettings, core chat service, and workspaces + expect(ChatService).toHaveBeenCalledWith( + mockCoreStart.uiSettings, + mockCoreStart.chat, + mockCoreStart.workspaces + ); }); it('should register chat button in header nav controls', () => { @@ -117,8 +124,12 @@ describe('ChatPlugin', () => { const startContract = testPlugin.start(mockCoreStart, mockDeps); - // ChatService should still be created with uiSettings and core chat service - expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings, mockCoreStart.chat); + // ChatService should still be created with uiSettings, core chat service, and workspaces + expect(ChatService).toHaveBeenCalledWith( + mockCoreStart.uiSettings, + mockCoreStart.chat, + mockCoreStart.workspaces + ); expect(startContract.chatService).toBeInstanceOf(ChatService); expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).toHaveBeenCalled(); }); @@ -126,7 +137,11 @@ describe('ChatPlugin', () => { it('should always initialize chat service (core service handles enablement)', () => { const startContract = plugin.start(mockCoreStart, mockDeps); - expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings, mockCoreStart.chat); + expect(ChatService).toHaveBeenCalledWith( + mockCoreStart.uiSettings, + mockCoreStart.chat, + mockCoreStart.workspaces + ); expect(startContract.chatService).toBeInstanceOf(ChatService); expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).toHaveBeenCalled(); }); @@ -185,7 +200,11 @@ describe('ChatPlugin', () => { expect(() => testPlugin.start(mockCoreStart, mockDeps)).not.toThrow(); // ChatService is always initialized - core service handles enablement logic - expect(ChatService).toHaveBeenCalledWith(mockCoreStart.uiSettings, mockCoreStart.chat); + expect(ChatService).toHaveBeenCalledWith( + mockCoreStart.uiSettings, + mockCoreStart.chat, + mockCoreStart.workspaces + ); }); }); }); diff --git a/src/plugins/chat/public/plugin.ts b/src/plugins/chat/public/plugin.ts index 1b6c35caef8e..7d402faa6213 100644 --- a/src/plugins/chat/public/plugin.ts +++ b/src/plugins/chat/public/plugin.ts @@ -103,7 +103,7 @@ export class ChatPlugin implements Plugin { ); // Always initialize chat service - core service handles enablement - this.chatService = new ChatService(core.uiSettings, core.chat); + this.chatService = new ChatService(core.uiSettings, core.chat, core.workspaces); if (!isEnabled) { return { diff --git a/src/plugins/chat/public/services/chat_service.ts b/src/plugins/chat/public/services/chat_service.ts index c9f5f7e321bd..8dde65b5daa8 100644 --- a/src/plugins/chat/public/services/chat_service.ts +++ b/src/plugins/chat/public/services/chat_service.ts @@ -14,8 +14,9 @@ import { UiSettingScope, ChatServiceStart, ChatWindowState, + WorkspacesStart, } from '../../../../core/public'; -import { getDefaultDataSourceId, getWorkspaces } from '../../../data_source_management/public'; +import { getDefaultDataSourceId } from '../../../data_source_management/public'; export interface ChatState { messages: Message[]; @@ -39,8 +40,9 @@ export class ChatService { public events$: any; private activeRequests: Set = new Set(); private requestCounter: number = 0; - private uiSettings?: IUiSettingsClient; + private uiSettings: IUiSettingsClient; private coreChatService?: ChatServiceStart; + private workspaces?: WorkspacesStart; // Chat state persistence private readonly STORAGE_KEY = 'chat.currentState'; @@ -49,11 +51,16 @@ export class ChatService { // ChatWindow ref for delegating sendMessage calls to proper timeline management private chatWindowRef: React.RefObject | null = null; - constructor(uiSettings?: IUiSettingsClient, coreChatService?: ChatServiceStart) { + constructor( + uiSettings: IUiSettingsClient, + coreChatService?: ChatServiceStart, + workspaces?: WorkspacesStart + ) { // No need to pass URL anymore - agent will use the proxy endpoint this.agent = new AgUiAgent(); this.uiSettings = uiSettings; this.coreChatService = coreChatService; + this.workspaces = workspaces; // Try to restore existing state first const currentChatState = this.loadCurrentChatState(); @@ -268,7 +275,7 @@ export class ChatService { } // Get workspace context - const workspaces = getWorkspaces(); + const workspaces = this.workspaces; if (!workspaces) { // eslint-disable-next-line no-console console.warn('Workspaces service not available, using global scope'); diff --git a/src/plugins/chat/public/types.ts b/src/plugins/chat/public/types.ts index 8ac185535432..9d100d2ff22e 100644 --- a/src/plugins/chat/public/types.ts +++ b/src/plugins/chat/public/types.ts @@ -6,7 +6,6 @@ import { NavigationPublicPluginStart } from '../../navigation/public'; import { ContextProviderStart } from '../../context_provider/public'; import { ChartsPluginStart } from '../../charts/public'; -import { DataSourceManagementPluginSetup } from '../../data_source_management/public'; import { ChatService } from './services/chat_service'; import { SuggestedActionsServiceSetupContract } from './services/suggested_action'; @@ -22,5 +21,4 @@ export interface AppPluginStartDependencies { navigation: NavigationPublicPluginStart; contextProvider: ContextProviderStart; charts: ChartsPluginStart; - dataSourceManagement?: DataSourceManagementPluginSetup; } diff --git a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap index 58cd62d15b1b..0c2d20492131 100644 --- a/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap +++ b/src/plugins/dashboard/public/application/components/dashboard_top_nav/__snapshots__/dashboard_top_nav.test.tsx.snap @@ -128,6 +128,24 @@ exports[`Dashboard top nav render in embed mode 1`] = ` "setAppLeftControls": [MockFunction], "setAppRightControls": [MockFunction], }, + "chat": Object { + "closeWindow": [MockFunction], + "getThreadId": [MockFunction], + "getThreadId$": [MockFunction], + "getWindowState": [MockFunction], + "getWindowState$": [MockFunction], + "isAvailable": [MockFunction], + "isWindowOpen": [MockFunction], + "newThread": [MockFunction], + "onWindowClose": [MockFunction], + "onWindowOpen": [MockFunction], + "openWindow": [MockFunction], + "sendMessage": [MockFunction], + "sendMessageWithWindow": [MockFunction], + "setThreadId": [MockFunction], + "setWindowState": [MockFunction], + "suggestedActionsService": undefined, + }, "chrome": Object { "addApplicationClass": [MockFunction], "docTitle": Object { @@ -1300,6 +1318,24 @@ exports[`Dashboard top nav render in embed mode, and force hide filter bar 1`] = "setAppLeftControls": [MockFunction], "setAppRightControls": [MockFunction], }, + "chat": Object { + "closeWindow": [MockFunction], + "getThreadId": [MockFunction], + "getThreadId$": [MockFunction], + "getWindowState": [MockFunction], + "getWindowState$": [MockFunction], + "isAvailable": [MockFunction], + "isWindowOpen": [MockFunction], + "newThread": [MockFunction], + "onWindowClose": [MockFunction], + "onWindowOpen": [MockFunction], + "openWindow": [MockFunction], + "sendMessage": [MockFunction], + "sendMessageWithWindow": [MockFunction], + "setThreadId": [MockFunction], + "setWindowState": [MockFunction], + "suggestedActionsService": undefined, + }, "chrome": Object { "addApplicationClass": [MockFunction], "docTitle": Object { @@ -2472,6 +2508,24 @@ exports[`Dashboard top nav render in embed mode, components can be forced show b "setAppLeftControls": [MockFunction], "setAppRightControls": [MockFunction], }, + "chat": Object { + "closeWindow": [MockFunction], + "getThreadId": [MockFunction], + "getThreadId$": [MockFunction], + "getWindowState": [MockFunction], + "getWindowState$": [MockFunction], + "isAvailable": [MockFunction], + "isWindowOpen": [MockFunction], + "newThread": [MockFunction], + "onWindowClose": [MockFunction], + "onWindowOpen": [MockFunction], + "openWindow": [MockFunction], + "sendMessage": [MockFunction], + "sendMessageWithWindow": [MockFunction], + "setThreadId": [MockFunction], + "setWindowState": [MockFunction], + "suggestedActionsService": undefined, + }, "chrome": Object { "addApplicationClass": [MockFunction], "docTitle": Object { @@ -3644,6 +3698,24 @@ exports[`Dashboard top nav render in full screen mode with appended URL param bu "setAppLeftControls": [MockFunction], "setAppRightControls": [MockFunction], }, + "chat": Object { + "closeWindow": [MockFunction], + "getThreadId": [MockFunction], + "getThreadId$": [MockFunction], + "getWindowState": [MockFunction], + "getWindowState$": [MockFunction], + "isAvailable": [MockFunction], + "isWindowOpen": [MockFunction], + "newThread": [MockFunction], + "onWindowClose": [MockFunction], + "onWindowOpen": [MockFunction], + "openWindow": [MockFunction], + "sendMessage": [MockFunction], + "sendMessageWithWindow": [MockFunction], + "setThreadId": [MockFunction], + "setWindowState": [MockFunction], + "suggestedActionsService": undefined, + }, "chrome": Object { "addApplicationClass": [MockFunction], "docTitle": Object { @@ -4816,6 +4888,24 @@ exports[`Dashboard top nav render in full screen mode, no componenets should be "setAppLeftControls": [MockFunction], "setAppRightControls": [MockFunction], }, + "chat": Object { + "closeWindow": [MockFunction], + "getThreadId": [MockFunction], + "getThreadId$": [MockFunction], + "getWindowState": [MockFunction], + "getWindowState$": [MockFunction], + "isAvailable": [MockFunction], + "isWindowOpen": [MockFunction], + "newThread": [MockFunction], + "onWindowClose": [MockFunction], + "onWindowOpen": [MockFunction], + "openWindow": [MockFunction], + "sendMessage": [MockFunction], + "sendMessageWithWindow": [MockFunction], + "setThreadId": [MockFunction], + "setWindowState": [MockFunction], + "suggestedActionsService": undefined, + }, "chrome": Object { "addApplicationClass": [MockFunction], "docTitle": Object { @@ -5988,6 +6078,24 @@ exports[`Dashboard top nav render with all components 1`] = ` "setAppLeftControls": [MockFunction], "setAppRightControls": [MockFunction], }, + "chat": Object { + "closeWindow": [MockFunction], + "getThreadId": [MockFunction], + "getThreadId$": [MockFunction], + "getWindowState": [MockFunction], + "getWindowState$": [MockFunction], + "isAvailable": [MockFunction], + "isWindowOpen": [MockFunction], + "newThread": [MockFunction], + "onWindowClose": [MockFunction], + "onWindowOpen": [MockFunction], + "openWindow": [MockFunction], + "sendMessage": [MockFunction], + "sendMessageWithWindow": [MockFunction], + "setThreadId": [MockFunction], + "setWindowState": [MockFunction], + "suggestedActionsService": undefined, + }, "chrome": Object { "addApplicationClass": [MockFunction], "docTitle": Object { diff --git a/src/plugins/data_source_management/public/index.ts b/src/plugins/data_source_management/public/index.ts index 546eba4e63c9..fe29891200af 100644 --- a/src/plugins/data_source_management/public/index.ts +++ b/src/plugins/data_source_management/public/index.ts @@ -26,6 +26,6 @@ export { createDataSourceMenu, } from './components/data_source_menu'; export { DataSourceSelectionService } from './service/data_source_selection_service'; -export { getDefaultDataSourceId, getDefaultDataSourceId$, getWorkspaces } from './components/utils'; +export { getDefaultDataSourceId, getDefaultDataSourceId$ } from './components/utils'; export { DATACONNECTIONS_BASE, DatasourceTypeToDisplayName } from './constants'; export { DEFAULT_DATA_SOURCE_UI_SETTINGS_ID } from '../common'; diff --git a/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.test.tsx b/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.test.tsx index b7bc6e82d8b9..3c02efe770bb 100644 --- a/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.test.tsx +++ b/src/plugins/explore/public/components/ask_ai_action_item/ask_ai_action_item.test.tsx @@ -169,9 +169,7 @@ describe('AskAIActionItem', () => { }); describe('execution logic', () => { - it('should send message with clearConversation when chat window is closed', async () => { - mockChatService.isWindowOpen.mockReturnValue(false); - + it('should send message with sendMessageWithWindow', async () => { render( { await waitFor(() => { expect(mockChatService.sendMessageWithWindow).toHaveBeenCalledWith( 'What is this error?', - expect.arrayContaining([ - expect.objectContaining({ - role: 'user', - content: 'What is this error?', - }), - ]), - { clearConversation: true } + [] ); }); }); - it('should send message without clearConversation when chat window is open', async () => { + it('should send message regardless of chat window state', async () => { mockChatService.isWindowOpen.mockReturnValue(true); render( @@ -222,12 +214,7 @@ describe('AskAIActionItem', () => { await waitFor(() => { expect(mockChatService.sendMessageWithWindow).toHaveBeenCalledWith( 'Follow-up question', - expect.arrayContaining([ - expect.objectContaining({ - role: 'user', - content: 'Follow-up question', - }), - ]) + [] ); }); }); @@ -275,11 +262,7 @@ describe('AskAIActionItem', () => { fireEvent.click(executeButton); await waitFor(() => { - expect(mockChatService.sendMessageWithWindow).toHaveBeenCalledWith( - 'Test question', - expect.anything(), - expect.anything() - ); + expect(mockChatService.sendMessageWithWindow).toHaveBeenCalledWith('Test question', []); }); }); diff --git a/yarn.lock b/yarn.lock index 8ba2dc8ca25d..3bff78eea93c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -25516,7 +25516,7 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0": +"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -25534,15 +25534,6 @@ string-width@^1.0.1: is-fullwidth-code-point "^1.0.0" strip-ansi "^3.0.0" -"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - string-width@^2.1.0, string-width@^2.1.1: version "2.1.1" resolved "https://registry.yarnpkg.com/string-width/-/string-width-2.1.1.tgz#ab93f27a8dc13d28cac815c462143a6d9012ae9e" @@ -25710,7 +25701,7 @@ stringify-entities@^3.0.1: character-entities-legacy "^1.0.0" xtend "^4.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -25752,13 +25743,6 @@ strip-ansi@^5.1.0, strip-ansi@^5.2.0: dependencies: ansi-regex "^4.1.0" -strip-ansi@^6.0.0, strip-ansi@^6.0.1: - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - strip-ansi@^7.0.1: version "7.1.0" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45" @@ -28504,7 +28488,7 @@ workerpool@^6.5.1: resolved "https://registry.yarnpkg.com/workerpool/-/workerpool-6.5.1.tgz#060f73b39d0caf97c6db64da004cd01b4c099544" integrity sha512-Fs4dNYcsdpYSAfVxhnl1L5zTksjvOJxtC5hzMNl+1t9B8hTJTdKDyZ5ju7ztgPy+ft9tBFXoOlDNiOT9WUXZlA== -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -28530,15 +28514,6 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From b9ecc7ec68c47bd81c3ef836ec790ec728ecb5fb Mon Sep 17 00:00:00 2001 From: "opensearch-changeset-bot[bot]" <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Date: Wed, 3 Dec 2025 00:30:12 +0000 Subject: [PATCH 4/4] Changeset file for PR #10983 created/updated --- changelogs/fragments/10983.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/10983.yml diff --git a/changelogs/fragments/10983.yml b/changelogs/fragments/10983.yml new file mode 100644 index 000000000000..0354a199ddc3 --- /dev/null +++ b/changelogs/fragments/10983.yml @@ -0,0 +1,2 @@ +refactor: +- [Chat] Refactor chat architecture: Extract shared infrastructure into a core chat service ([#10983](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/10983)) \ No newline at end of file