test(#121): Phase 3 - Add comprehensive tests for permission handlers

Add tests for AskUserQuestionHandler (14 tests):
- Multi-question format handling
- Backward compatibility (single question)
- Session mapping edge cases
- Cancel and re-prompt flow with async polling

Add tests for QuestionToolWorkaround (21 tests):
- Question tool detection
- Duplicate prevention with toolCallId tracking
- Session mapping
- Question formatting with/without options
- Cancel and notify flow
- Handled set cleanup after 60s delay

Coverage improvements:
- AskUserQuestionHandler: 2.17% → ~95%
- QuestionToolWorkaround: 11.42% → ~95%
- Overall: 49.3% → 52.52% (above CI threshold)

Part of #121
This commit is contained in:
Jiayuan
2026-01-29 00:44:51 +08:00
parent 8e4de4722a
commit db423b53a5
2 changed files with 818 additions and 0 deletions

View File

@@ -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 }
)
})
})
})

View File

@@ -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)
})
})
})