diff --git a/cli/src/agent/permissionAdapter.ts b/cli/src/agent/permissionAdapter.ts index 8b43830c..ac5ff544 100644 --- a/cli/src/agent/permissionAdapter.ts +++ b/cli/src/agent/permissionAdapter.ts @@ -2,7 +2,7 @@ import type { AgentBackend, PermissionRequest, PermissionResponse } from './type import type { AgentState, SessionPermissionMode } from '@/api/types'; import type { ApiSessionClient } from '@/api/apiSession'; import { logger } from '@/ui/logger'; -import { deriveToolName } from '@/agent/utils'; +import { deriveToolInput, deriveToolName } from '@/agent/utils'; import { RPC_METHODS } from '@hapi/protocol/rpcMethods'; import { resolveToolAutoApprovalDecision, @@ -15,13 +15,6 @@ interface PermissionResponseMessage { decision?: 'approved' | 'approved_for_session' | 'denied' | 'abort'; } -function deriveToolInput(request: PermissionRequest): unknown { - if (request.rawInput !== undefined) { - return request.rawInput; - } - return request.rawOutput; -} - function pickOptionId( request: PermissionRequest, preferredKinds: string[], diff --git a/cli/src/agent/permissionJson.test.ts b/cli/src/agent/permissionJson.test.ts new file mode 100644 index 00000000..263e1621 --- /dev/null +++ b/cli/src/agent/permissionJson.test.ts @@ -0,0 +1,146 @@ +import { describe, expect, it, vi } from 'vitest' +import { AgentStateSchema, SessionSchema, type AgentStateRequest, type AgentStateCompletedRequest } from '@hapi/protocol/schemas' +import { RPC_METHODS } from '@hapi/protocol/rpcMethods' +import type { AgentBackend, PermissionRequest } from './types' + +const transport = vi.hoisted(() => ({ + version: 0, + snapshots: [] as Array<{ requests: Record; completedRequests: Record }>, + debug: vi.fn() +})) + +vi.mock('@/ui/logger', () => ({ logger: { debug: transport.debug } })) +vi.mock('socket.io-client', () => ({ + io: () => { + const socket = { + connected: true, + on: () => socket, + off: () => socket, + emit: () => socket, + connect: () => socket, + disconnect: () => socket, + timeout: () => ({ emitWithAck: async () => ({}) }), + emitWithAck: async (event: string, payload: { + expectedVersion: number + agentState: unknown + }) => { + expect(event).toBe('update-state') + expect(payload.expectedVersion).toBe(transport.version) + // Model the JSON boundary and successful versioned storage ACK. + const wire = JSON.parse(JSON.stringify(payload.agentState)) + transport.snapshots.push(wire) + return JSON.parse(JSON.stringify({ + result: 'success', version: ++transport.version, agentState: wire + })) + } + } + return Object.assign(socket, { volatile: socket }) + } +})) + +import { ApiSessionClient } from '@/api/apiSession' +import { PermissionAdapter } from './permissionAdapter' +import { AcpPermissionHandler } from '@/modules/common/permission/AcpPermissionHandler' +import { CopilotPermissionHandler } from '@/copilot/utils/permissionHandler' +import { GrokPermissionHandler } from '@/grok/utils/permissionHandler' +import { OpencodePermissionHandler } from '@/opencode/utils/permissionHandler' + +describe.each([ + PermissionAdapter, AcpPermissionHandler, CopilotPermissionHandler, + GrokPermissionHandler, OpencodePermissionHandler +].map(Handler => ({ name: Handler.name, Handler })))('$name JSON permission state', ({ Handler }) => { + + it.each([ + { label: 'missing input auto-approval', rawInput: undefined, rawOutput: undefined, expected: null, autoApprove: true }, + { label: 'missing input', rawInput: undefined, rawOutput: undefined, expected: null }, + { label: 'missing input denial', rawInput: undefined, rawOutput: undefined, expected: null, decision: 'denied' }, + { label: 'missing input abort', rawInput: undefined, rawOutput: undefined, expected: null, decision: 'abort' }, + { label: 'explicit null', rawInput: null, rawOutput: { output: true }, expected: null }, + { label: 'input object', rawInput: { path: 'report.md' }, rawOutput: undefined, expected: { path: 'report.md' } }, + { label: 'output fallback', rawInput: undefined, rawOutput: { output: true }, expected: { output: true } }, + ...[false, 0, ''].map(value => ({ label: `falsy input ${JSON.stringify(value)}`, rawInput: value, rawOutput: 'fallback', expected: value })) + ])('$label: retain three requests and complete each decision', async (testCase) => { + const { rawInput, rawOutput, expected } = testCase + const autoApprove = 'autoApprove' in testCase && testCase.autoApprove + const decision = 'decision' in testCase ? testCase.decision : 'approved' + transport.version = 0 + transport.snapshots = [] + transport.debug.mockClear() + const client = new ApiSessionClient('isolated-repro-token', SessionSchema.parse({ + id: '11111111-1111-4111-8111-111111111111', namespace: 'repro', + seq: 0, createdAt: 1, updatedAt: 1, active: true, activeAt: 1, + metadata: null, metadataVersion: 0, + agentState: { requests: {}, completedRequests: {} }, agentStateVersion: 0, + thinking: true, thinkingAt: 1, todos: [], model: null, + modelReasoningEffort: null, effort: null, serviceTier: null + })) + let emitPermission: ((request: PermissionRequest) => void) | undefined + const respondToPermission = vi.fn(async () => {}) + const backend: AgentBackend = { + async initialize() {}, + async newSession() { return 'repro' }, + async prompt() {}, + async cancelPrompt() {}, + async disconnect() {}, + respondToPermission, + onPermissionRequest(handler) { emitPermission = handler } + } + new Handler(client, backend, () => 'default') + const ids = ['first', 'second', 'third'] + try { + for (const id of ids) { + if (!emitPermission) throw new Error('Permission listener not registered') + emitPermission({ + id, sessionId: 'repro', toolCallId: id, title: autoApprove ? 'hapi_change_title' : 'Write', rawInput, rawOutput, + options: [ + { optionId: 'allow-once', name: 'Allow once', kind: 'allow_once' }, + { optionId: 'reject-once', name: 'Reject once', kind: 'reject_once' } + ] + }) + } + if (autoApprove) { + await vi.waitFor(() => expect(respondToPermission).toHaveBeenCalledTimes(3)) + expect(await client.flush({ timeoutMs: 1000 })).toBe(true) + } else { + expect(await client.flush({ timeoutMs: 1000 })).toBe(true) + expect(respondToPermission).not.toHaveBeenCalled() + const pending = transport.snapshots.at(-1)! + const pendingParsed = AgentStateSchema.safeParse(pending) + expect.soft(pendingParsed.success, 'pending state survives JSON').toBe(true) + expect.soft(Object.keys(pending.requests), 'all requests remain visible').toEqual(ids) + + expect.soft(Object.values(pending.requests).map(request => request.arguments)).toEqual(ids.map(() => expected)) + + // Each decision must leave the other requests pending. + for (const id of ids) { + await client.rpcHandlerManager.handleRequest({ + method: `${client.sessionId}:${RPC_METHODS.Permission}`, + params: JSON.stringify({ id, approved: decision === 'approved' || (decision === 'abort' && id !== 'third'), + decision: decision === 'abort' && id !== 'third' ? 'approved' : decision }) + }) + expect(await client.flush({ timeoutMs: 1000 })).toBe(true) + expect.soft(Object.keys(transport.snapshots.at(-1)!.requests)).toEqual(ids.slice(ids.indexOf(id) + 1)) + expect(respondToPermission.mock.calls.map(call => call[1].id)).toEqual(ids.slice(0, ids.indexOf(id) + 1)) + } + } + expect(respondToPermission.mock.calls.map(call => call[1].id)).toEqual(ids) + for (const call of respondToPermission.mock.calls) { + expect(call[2]).toEqual(decision === 'abort' && call[1].id === 'third' + ? { outcome: 'cancelled' } + : { outcome: 'selected', optionId: decision === 'denied' ? 'reject-once' : 'allow-once' }) + } + const completed = transport.snapshots.at(-1)! + const completedParsed = AgentStateSchema.safeParse(completed) + expect.soft(completedParsed.success, 'completed state survives JSON').toBe(true) + expect.soft(Object.keys(completed.completedRequests), 'all decisions retained').toEqual(ids) + const invalidAcks = transport.debug.mock.calls.filter(call => + String(call[0]).includes('Ignoring invalid agentState value from ack') + ).length + expect(invalidAcks).toBe(0) + expect(Object.values(completed.completedRequests).map(request => request.arguments)).toEqual(ids.map(() => expected)) + expect(transport.snapshots.every(snapshot => AgentStateSchema.safeParse(snapshot).success)).toBe(true) + } finally { + client.close() + } + }) +}) diff --git a/cli/src/agent/utils.ts b/cli/src/agent/utils.ts index 110cba3c..ceffa44a 100644 --- a/cli/src/agent/utils.ts +++ b/cli/src/agent/utils.ts @@ -1,4 +1,9 @@ import { isObject } from '@hapi/protocol'; +import type { PermissionRequest } from './types'; + +export function deriveToolInput(request: PermissionRequest): unknown { + return request.rawInput !== undefined ? request.rawInput : request.rawOutput ?? null; +} type ToolNameSource = 'title' | 'raw_input_name' | 'raw_input_tool' | 'kind' | 'default'; diff --git a/cli/src/copilot/utils/permissionHandler.ts b/cli/src/copilot/utils/permissionHandler.ts index 6268bea9..fce7ed5b 100644 --- a/cli/src/copilot/utils/permissionHandler.ts +++ b/cli/src/copilot/utils/permissionHandler.ts @@ -1,7 +1,7 @@ import type { ApiSessionClient } from '@/api/apiSession'; import type { AgentBackend, PermissionRequest, PermissionResponse } from '@/agent/types'; import type { CopilotPermissionMode } from '@hapi/protocol/types'; -import { deriveToolName } from '@/agent/utils'; +import { deriveToolInput, deriveToolName } from '@/agent/utils'; import { logger } from '@/ui/logger'; import { BasePermissionHandler, @@ -26,13 +26,6 @@ const READ_ONLY_TOOL_NAMES = new Set([ 'list files' ]); -function deriveToolInput(request: PermissionRequest): unknown { - if (request.rawInput !== undefined) { - return request.rawInput; - } - return request.rawOutput; -} - function pickOptionId( request: PermissionRequest, preferredKinds: string[], diff --git a/cli/src/grok/utils/permissionHandler.ts b/cli/src/grok/utils/permissionHandler.ts index e1b31bca..f2323f23 100644 --- a/cli/src/grok/utils/permissionHandler.ts +++ b/cli/src/grok/utils/permissionHandler.ts @@ -1,7 +1,7 @@ import type { ApiSessionClient } from '@/api/apiSession' import type { AgentBackend, PermissionRequest, PermissionResponse } from '@/agent/types' import type { GrokPermissionMode } from '@hapi/protocol/types' -import { deriveToolName } from '@/agent/utils' +import { deriveToolInput, deriveToolName } from '@/agent/utils' import { logger } from '@/ui/logger' import { BasePermissionHandler, @@ -17,10 +17,6 @@ interface PermissionResponseMessage { reason?: string } -function deriveToolInput(request: PermissionRequest): unknown { - return request.rawInput !== undefined ? request.rawInput : request.rawOutput -} - function pickOptionId( request: PermissionRequest, preferredKinds: string[], diff --git a/cli/src/modules/common/permission/AcpPermissionHandler.ts b/cli/src/modules/common/permission/AcpPermissionHandler.ts index 11650c8a..40d65725 100644 --- a/cli/src/modules/common/permission/AcpPermissionHandler.ts +++ b/cli/src/modules/common/permission/AcpPermissionHandler.ts @@ -1,7 +1,7 @@ import type { ApiSessionClient } from '@/api/apiSession' import type { AgentBackend, PermissionRequest, PermissionResponse } from '@/agent/types' import type { PermissionMode } from '@hapi/protocol/types' -import { deriveToolName } from '@/agent/utils' +import { deriveToolInput, deriveToolName } from '@/agent/utils' import { logger } from '@/ui/logger' import { BasePermissionHandler, @@ -17,10 +17,6 @@ interface PermissionResponseMessage { reason?: string } -function deriveToolInput(request: PermissionRequest): unknown { - return request.rawInput !== undefined ? request.rawInput : request.rawOutput -} - function pickOptionId(request: PermissionRequest, preferredKinds: string[]): string | null { for (const kind of preferredKinds) { const match = request.options.find((option) => option.kind === kind) diff --git a/cli/src/opencode/utils/permissionHandler.ts b/cli/src/opencode/utils/permissionHandler.ts index cad1b18e..c072da03 100644 --- a/cli/src/opencode/utils/permissionHandler.ts +++ b/cli/src/opencode/utils/permissionHandler.ts @@ -1,7 +1,7 @@ import type { ApiSessionClient } from '@/api/apiSession'; import type { AgentBackend, PermissionRequest, PermissionResponse } from '@/agent/types'; import type { OpencodePermissionMode } from '@hapi/protocol/types'; -import { deriveToolName } from '@/agent/utils'; +import { deriveToolInput, deriveToolName } from '@/agent/utils'; import { logger } from '@/ui/logger'; import { BasePermissionHandler, @@ -17,13 +17,6 @@ interface PermissionResponseMessage { reason?: string; } -function deriveToolInput(request: PermissionRequest): unknown { - if (request.rawInput !== undefined) { - return request.rawInput; - } - return request.rawOutput; -} - function pickOptionId( request: PermissionRequest, preferredKinds: string[],