diff --git a/tests/unit/main/permission/AskUserQuestionHandler.test.ts b/tests/unit/main/permission/AskUserQuestionHandler.test.ts new file mode 100644 index 000000000..3d78dd1b4 --- /dev/null +++ b/tests/unit/main/permission/AskUserQuestionHandler.test.ts @@ -0,0 +1,355 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' + +import { AskUserQuestionHandler } from '../../../../src/main/permission/AskUserQuestionHandler' +import type { Conductor } from '../../../../src/main/conductor/Conductor' +import type { G3HandlerParams } from '../../../../src/main/permission/types' + +describe('AskUserQuestionHandler', () => { + let handler: AskUserQuestionHandler + let mockConductor: Conductor + const originalConsoleWarn = console.warn + const originalConsoleLog = console.log + const originalConsoleError = console.error + const originalSetImmediate = global.setImmediate + + beforeEach(() => { + vi.useFakeTimers() + + // Mock console methods + console.warn = vi.fn() + console.log = vi.fn() + console.error = vi.fn() + + // Mock setImmediate to schedule callback + global.setImmediate = vi.fn((callback: (...args: unknown[]) => void) => { + return setTimeout(callback, 0) as unknown as NodeJS.Immediate + }) as unknown as typeof global.setImmediate + + mockConductor = { + getMulticaSessionIdByAcp: vi.fn().mockReturnValue('multica-session-123'), + addPendingAnswer: vi.fn(), + cancelRequest: vi.fn().mockResolvedValue(undefined), + isSessionProcessing: vi.fn().mockReturnValue(false), + sendPrompt: vi.fn().mockResolvedValue(undefined) + } as unknown as Conductor + + handler = new AskUserQuestionHandler(mockConductor) + }) + + afterEach(() => { + vi.useRealTimers() + console.warn = originalConsoleWarn + console.log = originalConsoleLog + console.error = originalConsoleError + global.setImmediate = originalSetImmediate + vi.clearAllMocks() + }) + + describe('handle - multi-question format', () => { + it('should store multiple answers and re-prompt with formatted text', async () => { + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { rawInput: {} }, + responseData: { + answers: [ + { question: 'What is your name?', answer: 'John' }, + { question: 'What is your age?', answer: '30' } + ] + } + } + + await handler.handle(params) + + // Should map ACP session to Multica session + expect(mockConductor.getMulticaSessionIdByAcp).toHaveBeenCalledWith('acp-session-456') + + // Should store each answer + expect(mockConductor.addPendingAnswer).toHaveBeenCalledTimes(2) + expect(mockConductor.addPendingAnswer).toHaveBeenNthCalledWith( + 1, + 'multica-session-123', + 'What is your name?', + 'John' + ) + expect(mockConductor.addPendingAnswer).toHaveBeenNthCalledWith( + 2, + 'multica-session-123', + 'What is your age?', + '30' + ) + + // Should log the processing + expect(console.log).toHaveBeenCalledWith(expect.stringContaining('Processing 2 answers')) + }) + + it('should handle single answer in multi-question format', async () => { + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { rawInput: {} }, + responseData: { + answers: [{ question: 'What is your name?', answer: 'John' }] + } + } + + await handler.handle(params) + + expect(mockConductor.addPendingAnswer).toHaveBeenCalledTimes(1) + expect(mockConductor.addPendingAnswer).toHaveBeenCalledWith( + 'multica-session-123', + 'What is your name?', + 'John' + ) + }) + + it('should handle empty answers array', async () => { + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { rawInput: {} }, + responseData: { + answers: [] + } + } + + await handler.handle(params) + + // Should not add any answers, should fall through to backward compatibility + expect(mockConductor.addPendingAnswer).not.toHaveBeenCalled() + }) + }) + + describe('handle - backward compatibility (single question)', () => { + it('should handle single question with selectedOption', async () => { + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { + rawInput: { + questions: [{ question: 'Do you agree?' }] + } + }, + responseData: { + selectedOption: 'Yes' + } + } + + await handler.handle(params) + + expect(mockConductor.addPendingAnswer).toHaveBeenCalledWith( + 'multica-session-123', + 'Do you agree?', + 'Yes' + ) + }) + + it('should handle single question with selectedOptions array', async () => { + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { + rawInput: { + questions: [{ question: 'Select colors:' }] + } + }, + responseData: { + selectedOptions: ['Red', 'Blue'] + } + } + + await handler.handle(params) + + expect(mockConductor.addPendingAnswer).toHaveBeenCalledWith( + 'multica-session-123', + 'Select colors:', + 'Red, Blue' + ) + }) + + it('should handle single question with customText', async () => { + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { + rawInput: { + questions: [{ question: 'Enter your feedback:' }] + } + }, + responseData: { + customText: 'Great service!' + } + } + + await handler.handle(params) + + expect(mockConductor.addPendingAnswer).toHaveBeenCalledWith( + 'multica-session-123', + 'Enter your feedback:', + 'Great service!' + ) + }) + + it('should return early if no questions in rawInput', async () => { + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { rawInput: {} }, + responseData: {} + } + + await handler.handle(params) + + expect(mockConductor.addPendingAnswer).not.toHaveBeenCalled() + }) + + it('should return early if no answer provided', async () => { + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { + rawInput: { + questions: [{ question: 'Do you agree?' }] + } + }, + responseData: {} + } + + await handler.handle(params) + + expect(mockConductor.addPendingAnswer).not.toHaveBeenCalled() + }) + }) + + describe('handle - session mapping', () => { + it('should warn and return if no Multica session found', async () => { + vi.mocked(mockConductor.getMulticaSessionIdByAcp).mockReturnValue(null) + + const params: G3HandlerParams = { + acpSessionId: 'unknown-acp-session', + toolCall: { rawInput: {} }, + responseData: { answers: [{ question: 'Q?', answer: 'A' }] } + } + + await handler.handle(params) + + expect(console.warn).toHaveBeenCalledWith( + expect.stringContaining('Could not find Multica session ID') + ) + expect(mockConductor.addPendingAnswer).not.toHaveBeenCalled() + }) + }) + + describe('handle - cancel and re-prompt flow', () => { + it('should cancel current turn and re-prompt with answer', async () => { + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { rawInput: {} }, + responseData: { + answers: [{ question: 'What?', answer: 'Answer text' }] + } + } + + await handler.handle(params) + + // Run setImmediate callbacks + await vi.runAllTimersAsync() + + // Should cancel the request + expect(mockConductor.cancelRequest).toHaveBeenCalledWith('multica-session-123') + + // Should send re-prompt with internal flag + expect(mockConductor.sendPrompt).toHaveBeenCalledWith( + 'multica-session-123', + [{ type: 'text', text: 'Answer text' }], + { internal: true } + ) + }) + + it('should poll while session is processing', async () => { + // Session is processing initially, then stops after a few polls + let pollCount = 0 + vi.mocked(mockConductor.isSessionProcessing).mockImplementation(() => { + pollCount++ + return pollCount < 3 // Stop processing after 2 polls + }) + + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { rawInput: {} }, + responseData: { + answers: [{ question: 'What?', answer: 'Yes' }] + } + } + + await handler.handle(params) + + // Run all timers to complete the async flow + await vi.runAllTimersAsync() + + // Should have checked processing state multiple times + expect(mockConductor.isSessionProcessing).toHaveBeenCalledTimes(3) + expect(mockConductor.sendPrompt).toHaveBeenCalled() + }) + + it('should handle timeout waiting for cancel', async () => { + // Session always processing (timeout scenario after 2s = 20 polls at 100ms each) + vi.mocked(mockConductor.isSessionProcessing).mockReturnValue(true) + + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { rawInput: {} }, + responseData: { + answers: [{ question: 'What?', answer: 'Yes' }] + } + } + + await handler.handle(params) + + // Advance timers to trigger timeout (maxWaitMs = 2000, pollIntervalMs = 100) + await vi.advanceTimersByTimeAsync(2500) + + // Should still proceed after timeout + expect(mockConductor.sendPrompt).toHaveBeenCalled() + }) + + it('should log error if cancel/re-prompt fails', async () => { + vi.mocked(mockConductor.cancelRequest).mockRejectedValue(new Error('Cancel failed')) + + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { rawInput: {} }, + responseData: { + answers: [{ question: 'What?', answer: 'Yes' }] + } + } + + await handler.handle(params) + + // Run all timers to complete the async flow + await vi.runAllTimersAsync() + + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('Error during cancel/re-prompt'), + expect.any(Error) + ) + }) + + it('should format multiple answers for re-prompt', async () => { + const params: G3HandlerParams = { + acpSessionId: 'acp-session-456', + toolCall: { rawInput: {} }, + responseData: { + answers: [ + { question: 'Name?', answer: 'John' }, + { question: 'Age?', answer: '30' } + ] + } + } + + await handler.handle(params) + + // Run all timers to complete the async flow + await vi.runAllTimersAsync() + + const expectedText = 'Q1: Name?\nA1: John\n\nQ2: Age?\nA2: 30' + expect(mockConductor.sendPrompt).toHaveBeenCalledWith( + 'multica-session-123', + [{ type: 'text', text: expectedText }], + { internal: true } + ) + }) + }) +}) diff --git a/tests/unit/main/permission/QuestionToolWorkaround.test.ts b/tests/unit/main/permission/QuestionToolWorkaround.test.ts new file mode 100644 index 000000000..2afd9f6ea --- /dev/null +++ b/tests/unit/main/permission/QuestionToolWorkaround.test.ts @@ -0,0 +1,463 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' + +import { QuestionToolWorkaround } from '../../../../src/main/permission/QuestionToolWorkaround' +import type { Conductor } from '../../../../src/main/conductor/Conductor' +import type { QuestionToolUpdate } from '../../../../src/main/permission/types' + +describe('QuestionToolWorkaround', () => { + let workaround: QuestionToolWorkaround + let mockConductor: Conductor + const originalConsoleLog = console.log + const originalConsoleError = console.error + const originalSetImmediate = global.setImmediate + const originalSetTimeout = global.setTimeout + + beforeEach(() => { + vi.useFakeTimers() + + // Mock console methods + console.log = vi.fn() + console.error = vi.fn() + + // Mock setImmediate to execute callback immediately + global.setImmediate = vi.fn((callback: (...args: unknown[]) => void) => { + callback() + return {} as NodeJS.Immediate + }) as unknown as typeof global.setImmediate + + mockConductor = { + getMulticaSessionIdByAcp: vi.fn().mockReturnValue('multica-session-123'), + cancelRequest: vi.fn().mockResolvedValue(undefined), + sendPrompt: vi.fn().mockResolvedValue(undefined) + } as unknown as Conductor + + workaround = new QuestionToolWorkaround(mockConductor) + }) + + afterEach(() => { + vi.useRealTimers() + console.log = originalConsoleLog + console.error = originalConsoleError + global.setImmediate = originalSetImmediate + global.setTimeout = originalSetTimeout + vi.clearAllMocks() + }) + + describe('handleToolUpdate - detection', () => { + it('should detect question tool in progress', async () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { + questions: [{ question: 'What is your name?' }] + } + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(mockConductor.getMulticaSessionIdByAcp).toHaveBeenCalledWith('acp-session-456') + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Tool detected (will hang forever)') + ) + }) + + it('should ignore non-question tools', () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'bash', + status: 'in_progress' + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(mockConductor.getMulticaSessionIdByAcp).not.toHaveBeenCalled() + }) + + it('should ignore question tool with non-in_progress status', () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'completed' + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(mockConductor.getMulticaSessionIdByAcp).not.toHaveBeenCalled() + }) + + it('should ignore non-tool_call_update events', () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'other_event', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress' + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(mockConductor.getMulticaSessionIdByAcp).not.toHaveBeenCalled() + }) + + it('should handle missing title', () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + status: 'in_progress' + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(mockConductor.getMulticaSessionIdByAcp).not.toHaveBeenCalled() + }) + + it('should handle missing status', () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question' + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(mockConductor.getMulticaSessionIdByAcp).not.toHaveBeenCalled() + }) + }) + + describe('handleToolUpdate - duplicate prevention', () => { + it('should skip already handled toolCallIds', () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { questions: [{ question: 'Q1?' }] } + } + + // First call + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + expect(mockConductor.cancelRequest).toHaveBeenCalledTimes(1) + + // Second call with same toolCallId + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + expect(mockConductor.cancelRequest).toHaveBeenCalledTimes(1) + + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Already handled toolCallId=tool-123') + ) + }) + + it('should handle different toolCallIds separately', () => { + const update1: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { questions: [{ question: 'Q1?' }] } + } + + const update2: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-456', + title: 'question', + status: 'in_progress', + rawInput: { questions: [{ question: 'Q2?' }] } + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update: update1 }) + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update: update2 }) + + expect(mockConductor.cancelRequest).toHaveBeenCalledTimes(2) + }) + + it('should handle missing toolCallId (still processes)', () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + title: 'question', + status: 'in_progress', + rawInput: { questions: [{ question: 'Q?' }] } + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + // Should still process even without toolCallId + expect(mockConductor.cancelRequest).toHaveBeenCalled() + }) + }) + + describe('handleToolUpdate - session mapping', () => { + it('should return early if no Multica session found', () => { + vi.mocked(mockConductor.getMulticaSessionIdByAcp).mockReturnValue(null) + + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { questions: [{ question: 'Q?' }] } + } + + workaround.handleToolUpdate({ sessionId: 'unknown-acp-session', update }) + + expect(mockConductor.cancelRequest).not.toHaveBeenCalled() + }) + }) + + describe('handleToolUpdate - question formatting', () => { + it('should format single question without options', async () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { + questions: [{ question: 'What is your name?' }] + } + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(console.log).toHaveBeenCalledWith( + '[Question] Original questions:', + '1. What is your name?' + ) + }) + + it('should format question with options', async () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { + questions: [ + { + question: 'Select a color:', + options: [ + { label: 'Red', description: 'The color red' }, + { label: 'Blue', description: 'The color blue' } + ] + } + ] + } + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(console.log).toHaveBeenCalledWith( + '[Question] Original questions:', + '1. Select a color:\n Options: Red, Blue' + ) + }) + + it('should format multiple questions', async () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { + questions: [ + { question: 'What is your name?' }, + { question: 'What is your age?', options: [{ label: '18-25' }, { label: '26-35' }] } + ] + } + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(console.log).toHaveBeenCalledWith( + '[Question] Original questions:', + '1. What is your name?\n2. What is your age?\n Options: 18-25, 26-35' + ) + }) + + it('should handle missing questions array', async () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: {} + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(console.log).toHaveBeenCalledWith('[Question] Original questions:', '') + }) + + it('should handle missing rawInput', async () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress' + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(console.log).toHaveBeenCalledWith('[Question] Original questions:', '') + }) + }) + + describe('handleToolUpdate - cancel and notify flow', () => { + it('should cancel request and send notification', async () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { + questions: [{ question: 'What is your name?' }] + } + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + // Run timers to execute setImmediate and setTimeout callbacks + await vi.runAllTimersAsync() + + expect(mockConductor.cancelRequest).toHaveBeenCalledWith('multica-session-123') + + const expectedPrompt = `The "question" tool is not available in this environment. You tried to ask: + +1. What is your name? + +Please ask these questions directly in the conversation (as plain text) so the user can respond.` + + expect(mockConductor.sendPrompt).toHaveBeenCalledWith( + 'multica-session-123', + [{ type: 'text', text: expectedPrompt }], + { internal: true } + ) + + expect(console.log).toHaveBeenCalledWith( + '[Question] Agent notified internally to ask directly' + ) + }) + + it('should use generic prompt when no questions provided', async () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: {} + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + // Run timers to execute setImmediate and setTimeout callbacks + await vi.runAllTimersAsync() + + const expectedPrompt = + 'The "question" tool is not available in this environment. Please ask your question directly in the conversation instead of using the question tool.' + + expect(mockConductor.sendPrompt).toHaveBeenCalledWith( + 'multica-session-123', + [{ type: 'text', text: expectedPrompt }], + { internal: true } + ) + }) + + it('should wait for cancel to settle before re-prompting', async () => { + // Use real timers for this test + vi.useRealTimers() + + let setImmediateCallback: (() => void) | null = null + global.setImmediate = vi.fn((callback: (...args: unknown[]) => void) => { + setImmediateCallback = callback as () => void + return {} as NodeJS.Immediate + }) as unknown as typeof global.setImmediate + + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { questions: [{ question: 'Q?' }] } + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + expect(setImmediateCallback).not.toBeNull() + + // Execute the async flow + if (setImmediateCallback) { + await setImmediateCallback() + } + + // Cancel should be called before sendPrompt + expect(mockConductor.cancelRequest).toHaveBeenCalledBefore(mockConductor.sendPrompt) + }) + + it('should log error if handling fails', async () => { + vi.mocked(mockConductor.cancelRequest).mockRejectedValue(new Error('Cancel failed')) + + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { questions: [{ question: 'Q?' }] } + } + + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + + // Run timers to execute setImmediate and setTimeout callbacks + await vi.runAllTimersAsync() + + expect(console.error).toHaveBeenCalledWith( + '[Question] Failed to handle question tool:', + expect.any(Error) + ) + }) + }) + + describe('handleToolUpdate - handled set cleanup', () => { + it('should clean up handled toolCallIds after delay', () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { questions: [{ question: 'Q?' }] } + } + + // First call + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + expect(mockConductor.cancelRequest).toHaveBeenCalledTimes(1) + + // Advance timers by 60 seconds (cleanup happens at 60s) + vi.advanceTimersByTime(60000) + + // Second call should process again after cleanup + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + expect(mockConductor.cancelRequest).toHaveBeenCalledTimes(2) + }) + + it('should not clean up before delay', () => { + const update: QuestionToolUpdate = { + sessionUpdate: 'tool_call_update', + toolCallId: 'tool-123', + title: 'question', + status: 'in_progress', + rawInput: { questions: [{ question: 'Q?' }] } + } + + // First call + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + expect(mockConductor.cancelRequest).toHaveBeenCalledTimes(1) + + // Advance timers by 59 seconds (just before cleanup) + vi.advanceTimersByTime(59000) + + // Second call should still be skipped + workaround.handleToolUpdate({ sessionId: 'acp-session-456', update }) + expect(mockConductor.cancelRequest).toHaveBeenCalledTimes(1) + }) + }) +})