From ccca0949ae1e7fa38c92e24a861c20e94b07f5c1 Mon Sep 17 00:00:00 2001 From: nrslib <38722970+nrslib@users.noreply.github.com> Date: Wed, 11 Feb 2026 11:31:38 +0900 Subject: [PATCH] fix: opencode permission and tool wiring for edit execution --- src/__tests__/opencode-client-cleanup.test.ts | 209 +++++++++++++++++- src/__tests__/opencode-types.test.ts | 56 ++++- src/infra/opencode/client.ts | 96 ++++++-- src/infra/opencode/types.ts | 134 +++++++++++ src/infra/providers/opencode.ts | 1 + 5 files changed, 466 insertions(+), 30 deletions(-) diff --git a/src/__tests__/opencode-client-cleanup.test.ts b/src/__tests__/opencode-client-cleanup.test.ts index 3081682..673398e 100644 --- a/src/__tests__/opencode-client-cleanup.test.ts +++ b/src/__tests__/opencode-client-cleanup.test.ts @@ -290,10 +290,13 @@ describe('OpenCodeClient stream cleanup', () => { expect(result.status).toBe('error'); expect(result.content).toContain('no question handler'); - expect(questionReject).toHaveBeenCalledWith({ - requestID: 'q-1', - directory: '/tmp', - }); + expect(questionReject).toHaveBeenCalledWith( + { + requestID: 'q-1', + directory: '/tmp', + }, + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); }); it('should answer question.asked when handler is configured', async () => { @@ -350,10 +353,200 @@ describe('OpenCodeClient stream cleanup', () => { }); expect(result.status).toBe('done'); - expect(questionReply).toHaveBeenCalledWith({ - requestID: 'q-2', - directory: '/tmp', - answers: [['A']], + expect(questionReply).toHaveBeenCalledWith( + { + requestID: 'q-2', + directory: '/tmp', + answers: [['A']], + }, + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); + }); + + it('should pass mapped tools to promptAsync when allowedTools is set', async () => { + const { OpenCodeClient } = await import('../infra/opencode/client.js'); + const stream = new MockEventStream([ + { + type: 'message.updated', + properties: { + info: { + sessionID: 'session-tools', + role: 'assistant', + time: { created: Date.now(), completed: Date.now() + 1 }, + }, + }, + }, + ]); + + const promptAsync = vi.fn().mockResolvedValue(undefined); + const sessionCreate = vi.fn().mockResolvedValue({ data: { id: 'session-tools' } }); + const disposeInstance = vi.fn().mockResolvedValue({ data: {} }); + const subscribe = vi.fn().mockResolvedValue({ stream }); + + createOpencodeMock.mockResolvedValue({ + client: { + instance: { dispose: disposeInstance }, + session: { create: sessionCreate, promptAsync }, + event: { subscribe }, + permission: { reply: vi.fn() }, + }, + server: { close: vi.fn() }, }); + + const client = new OpenCodeClient(); + const result = await client.call('coder', 'hello', { + cwd: '/tmp', + model: 'opencode/big-pickle', + allowedTools: ['Read', 'Edit', 'Bash', 'WebSearch', 'WebFetch', 'mcp__github__search'], + }); + + expect(result.status).toBe('done'); + expect(promptAsync).toHaveBeenCalledWith( + expect.objectContaining({ + tools: { + read: true, + edit: true, + bash: true, + websearch: true, + webfetch: true, + mcp__github__search: true, + }, + }), + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); + }); + + it('should configure allow permissions for edit mode', async () => { + const { OpenCodeClient } = await import('../infra/opencode/client.js'); + const stream = new MockEventStream([ + { + type: 'message.updated', + properties: { + info: { + sessionID: 'session-perm', + role: 'assistant', + time: { created: Date.now(), completed: Date.now() + 1 }, + }, + }, + }, + ]); + + const promptAsync = vi.fn().mockResolvedValue(undefined); + const sessionCreate = vi.fn().mockResolvedValue({ data: { id: 'session-perm' } }); + const disposeInstance = vi.fn().mockResolvedValue({ data: {} }); + const subscribe = vi.fn().mockResolvedValue({ stream }); + + createOpencodeMock.mockResolvedValue({ + client: { + instance: { dispose: disposeInstance }, + session: { create: sessionCreate, promptAsync }, + event: { subscribe }, + permission: { reply: vi.fn() }, + }, + server: { close: vi.fn() }, + }); + + const client = new OpenCodeClient(); + await client.call('coder', 'hello', { + cwd: '/tmp', + model: 'opencode/big-pickle', + permissionMode: 'edit', + }); + + const createCallArgs = createOpencodeMock.mock.calls[0]?.[0] as { config?: Record }; + const permission = createCallArgs.config?.permission as Record; + expect(permission.read).toBe('allow'); + expect(permission.edit).toBe('allow'); + expect(permission.write).toBe('allow'); + expect(permission.bash).toBe('allow'); + expect(permission.question).toBe('deny'); + }); + + it('should pass permission ruleset to session.create', async () => { + const { OpenCodeClient } = await import('../infra/opencode/client.js'); + const stream = new MockEventStream([ + { + type: 'message.updated', + properties: { + info: { + sessionID: 'session-ruleset', + role: 'assistant', + time: { created: Date.now(), completed: Date.now() + 1 }, + }, + }, + }, + ]); + + const promptAsync = vi.fn().mockResolvedValue(undefined); + const sessionCreate = vi.fn().mockResolvedValue({ data: { id: 'session-ruleset' } }); + const disposeInstance = vi.fn().mockResolvedValue({ data: {} }); + const subscribe = vi.fn().mockResolvedValue({ stream }); + + createOpencodeMock.mockResolvedValue({ + client: { + instance: { dispose: disposeInstance }, + session: { create: sessionCreate, promptAsync }, + event: { subscribe }, + permission: { reply: vi.fn() }, + }, + server: { close: vi.fn() }, + }); + + const client = new OpenCodeClient(); + await client.call('coder', 'hello', { + cwd: '/tmp', + model: 'opencode/big-pickle', + permissionMode: 'edit', + }); + + expect(sessionCreate).toHaveBeenCalledWith(expect.objectContaining({ + directory: '/tmp', + permission: expect.arrayContaining([ + expect.objectContaining({ permission: 'edit', action: 'allow' }), + expect.objectContaining({ permission: 'question', action: 'deny' }), + ]), + })); + }); + + it('should fail fast when permission reply times out', async () => { + const { OpenCodeClient } = await import('../infra/opencode/client.js'); + const stream = new MockEventStream([ + { + type: 'permission.asked', + properties: { + id: 'perm-1', + sessionID: 'session-perm-timeout', + }, + }, + ]); + + const promptAsync = vi.fn().mockResolvedValue(undefined); + const sessionCreate = vi.fn().mockResolvedValue({ data: { id: 'session-perm-timeout' } }); + const disposeInstance = vi.fn().mockResolvedValue({ data: {} }); + const subscribe = vi.fn().mockResolvedValue({ stream }); + const permissionReply = vi.fn().mockImplementation(() => new Promise(() => {})); + + createOpencodeMock.mockResolvedValue({ + client: { + instance: { dispose: disposeInstance }, + session: { create: sessionCreate, promptAsync }, + event: { subscribe }, + permission: { reply: permissionReply }, + }, + server: { close: vi.fn() }, + }); + + const client = new OpenCodeClient(); + const result = await Promise.race([ + client.call('coder', 'hello', { + cwd: '/tmp', + model: 'opencode/big-pickle', + permissionMode: 'edit', + }), + new Promise((_, reject) => setTimeout(() => reject(new Error('timed out')), 8000)), + ]); + + expect(result.status).toBe('error'); + expect(result.content).toContain('permission reply timed out'); }); }); diff --git a/src/__tests__/opencode-types.test.ts b/src/__tests__/opencode-types.test.ts index 6251b8d..e8ad9b3 100644 --- a/src/__tests__/opencode-types.test.ts +++ b/src/__tests__/opencode-types.test.ts @@ -3,7 +3,12 @@ */ import { describe, it, expect } from 'vitest'; -import { mapToOpenCodePermissionReply } from '../infra/opencode/types.js'; +import { + buildOpenCodePermissionConfig, + buildOpenCodePermissionRuleset, + mapToOpenCodePermissionReply, + mapToOpenCodeTools, +} from '../infra/opencode/types.js'; import type { PermissionMode } from '../core/models/index.js'; describe('mapToOpenCodePermissionReply', () => { @@ -28,3 +33,52 @@ describe('mapToOpenCodePermissionReply', () => { }); }); }); + +describe('mapToOpenCodeTools', () => { + it('should map built-in tool names to OpenCode tool IDs', () => { + expect(mapToOpenCodeTools(['Read', 'Edit', 'Bash', 'WebSearch', 'WebFetch'])).toEqual({ + read: true, + edit: true, + bash: true, + websearch: true, + webfetch: true, + }); + }); + + it('should keep unknown tool names as-is', () => { + expect(mapToOpenCodeTools(['mcp__github__search', 'custom_tool'])).toEqual({ + mcp__github__search: true, + custom_tool: true, + }); + }); + + it('should return undefined when tools are not provided', () => { + expect(mapToOpenCodeTools(undefined)).toBeUndefined(); + expect(mapToOpenCodeTools([])).toBeUndefined(); + }); +}); + +describe('OpenCode permissions', () => { + it('should build allow config for full mode', () => { + expect(buildOpenCodePermissionConfig('full')).toBe('allow'); + }); + + it('should build deny config for readonly mode', () => { + expect(buildOpenCodePermissionConfig('readonly')).toBe('deny'); + }); + + it('should build ruleset for edit mode', () => { + const ruleset = buildOpenCodePermissionRuleset('edit'); + expect(ruleset.length).toBeGreaterThan(0); + expect(ruleset.find((rule) => rule.permission === 'edit')).toEqual({ + permission: 'edit', + pattern: '**', + action: 'allow', + }); + expect(ruleset.find((rule) => rule.permission === 'question')).toEqual({ + permission: 'question', + pattern: '**', + action: 'deny', + }); + }); +}); diff --git a/src/infra/opencode/client.ts b/src/infra/opencode/client.ts index 3d266fd..cb6271b 100644 --- a/src/infra/opencode/client.ts +++ b/src/infra/opencode/client.ts @@ -10,7 +10,13 @@ import { createServer } from 'node:net'; import type { AgentResponse } from '../../core/models/index.js'; import { createLogger, getErrorMessage } from '../../shared/utils/index.js'; import { parseProviderModel } from '../../shared/utils/providerModel.js'; -import { mapToOpenCodePermissionReply, type OpenCodeCallOptions } from './types.js'; +import { + buildOpenCodePermissionConfig, + buildOpenCodePermissionRuleset, + mapToOpenCodePermissionReply, + mapToOpenCodeTools, + type OpenCodeCallOptions, +} from './types.js'; import { type OpenCodeStreamEvent, type OpenCodePart, @@ -29,6 +35,7 @@ const OPENCODE_STREAM_IDLE_TIMEOUT_MS = 10 * 60 * 1000; const OPENCODE_STREAM_ABORTED_MESSAGE = 'OpenCode execution aborted'; const OPENCODE_RETRY_MAX_ATTEMPTS = 3; const OPENCODE_RETRY_BASE_DELAY_MS = 250; +const OPENCODE_INTERACTION_TIMEOUT_MS = 5000; const OPENCODE_RETRYABLE_ERROR_PATTERNS = [ 'stream disconnected before completion', 'transport error', @@ -41,6 +48,31 @@ const OPENCODE_RETRYABLE_ERROR_PATTERNS = [ 'failed to start server on port', ]; +async function withTimeout( + operation: (signal: AbortSignal) => Promise, + timeoutMs: number, + timeoutErrorMessage: string, +): Promise { + const controller = new AbortController(); + let timeoutId: ReturnType | undefined; + const timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout(() => { + controller.abort(); + reject(new Error(timeoutErrorMessage)); + }, timeoutMs); + }); + try { + return await Promise.race([ + operation(controller.signal), + timeoutPromise, + ]); + } finally { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } + } +} + function extractOpenCodeErrorMessage(error: unknown): string | undefined { if (!error || typeof error !== 'object') { return undefined; @@ -256,10 +288,11 @@ export class OpenCodeClient { const parsedModel = parseProviderModel(options.model, 'OpenCode model'); const fullModel = `${parsedModel.providerID}/${parsedModel.modelID}`; const port = await getFreePort(); + const permission = buildOpenCodePermissionConfig(options.permissionMode); const config = { model: fullModel, small_model: fullModel, - permission: { question: 'deny' as const }, + permission, ...(options.opencodeApiKey ? { provider: { opencode: { options: { apiKey: options.opencodeApiKey } } } } : {}), @@ -274,7 +307,10 @@ export class OpenCodeClient { const sessionResult = options.sessionId ? { data: { id: options.sessionId } } - : await client.session.create({ directory: options.cwd }); + : await client.session.create({ + directory: options.cwd, + permission: buildOpenCodePermissionRuleset(options.permissionMode), + }); const sessionId = sessionResult.data?.id; if (!sessionId) { @@ -286,11 +322,13 @@ export class OpenCodeClient { ); resetIdleTimeout(); + const tools = mapToOpenCodeTools(options.allowedTools); await client.session.promptAsync( { sessionID: sessionId, directory: options.cwd, model: parsedModel, + ...(tools ? { tools } : {}), parts: [{ type: 'text' as const, text: fullPrompt }], }, { signal: streamAbortController.signal }, @@ -348,11 +386,15 @@ export class OpenCodeClient { const reply = options.permissionMode ? mapToOpenCodePermissionReply(options.permissionMode) : 'once'; - await client.permission.reply({ - requestID: permProps.id, - directory: options.cwd, - reply, - }); + await withTimeout( + (signal) => client.permission.reply({ + requestID: permProps.id, + directory: options.cwd, + reply, + }, { signal }), + OPENCODE_INTERACTION_TIMEOUT_MS, + 'OpenCode permission reply timed out', + ); } continue; } @@ -361,10 +403,14 @@ export class OpenCodeClient { const questionProps = sseEvent.properties as OpenCodeQuestionAskedProperties; if (questionProps.sessionID === sessionId) { if (!options.onAskUserQuestion) { - await client.question.reject({ - requestID: questionProps.id, - directory: options.cwd, - }); + await withTimeout( + (signal) => client.question.reject({ + requestID: questionProps.id, + directory: options.cwd, + }, { signal }), + OPENCODE_INTERACTION_TIMEOUT_MS, + 'OpenCode question reject timed out', + ); success = false; failureMessage = 'OpenCode asked a question, but no question handler is configured'; break; @@ -372,16 +418,24 @@ export class OpenCodeClient { try { const answers = await options.onAskUserQuestion(toQuestionInput(questionProps)); - await client.question.reply({ - requestID: questionProps.id, - directory: options.cwd, - answers: toQuestionAnswers(questionProps, answers), - }); + await withTimeout( + (signal) => client.question.reply({ + requestID: questionProps.id, + directory: options.cwd, + answers: toQuestionAnswers(questionProps, answers), + }, { signal }), + OPENCODE_INTERACTION_TIMEOUT_MS, + 'OpenCode question reply timed out', + ); } catch { - await client.question.reject({ - requestID: questionProps.id, - directory: options.cwd, - }); + await withTimeout( + (signal) => client.question.reject({ + requestID: questionProps.id, + directory: options.cwd, + }, { signal }), + OPENCODE_INTERACTION_TIMEOUT_MS, + 'OpenCode question reject timed out', + ); success = false; failureMessage = 'OpenCode question handling failed'; break; diff --git a/src/infra/opencode/types.ts b/src/infra/opencode/types.ts index d25816d..490561d 100644 --- a/src/infra/opencode/types.ts +++ b/src/infra/opencode/types.ts @@ -8,6 +8,7 @@ import type { PermissionMode } from '../../core/models/index.js'; /** OpenCode permission reply values */ export type OpenCodePermissionReply = 'once' | 'always' | 'reject'; +export type OpenCodePermissionAction = 'ask' | 'allow' | 'deny'; /** Map TAKT PermissionMode to OpenCode permission reply */ export function mapToOpenCodePermissionReply(mode: PermissionMode): OpenCodePermissionReply { @@ -19,6 +20,138 @@ export function mapToOpenCodePermissionReply(mode: PermissionMode): OpenCodePerm return mapping[mode]; } +const OPEN_CODE_PERMISSION_KEYS = [ + 'read', + 'glob', + 'grep', + 'edit', + 'write', + 'bash', + 'task', + 'websearch', + 'webfetch', + 'question', +] as const; + +export type OpenCodePermissionKey = typeof OPEN_CODE_PERMISSION_KEYS[number]; + +export type OpenCodePermissionMap = Record; + +function buildPermissionMap(mode?: PermissionMode): OpenCodePermissionMap { + const allDeny: OpenCodePermissionMap = { + read: 'deny', + glob: 'deny', + grep: 'deny', + edit: 'deny', + write: 'deny', + bash: 'deny', + task: 'deny', + websearch: 'deny', + webfetch: 'deny', + question: 'deny', + }; + + if (mode === 'readonly') return allDeny; + + if (mode === 'full') { + return { + ...allDeny, + read: 'allow', + glob: 'allow', + grep: 'allow', + edit: 'allow', + write: 'allow', + bash: 'allow', + task: 'allow', + websearch: 'allow', + webfetch: 'allow', + question: 'allow', + }; + } + + if (mode === 'edit') { + return { + ...allDeny, + read: 'allow', + glob: 'allow', + grep: 'allow', + edit: 'allow', + write: 'allow', + bash: 'allow', + task: 'allow', + websearch: 'allow', + webfetch: 'allow', + question: 'deny', + }; + } + + return { + ...allDeny, + read: 'ask', + glob: 'ask', + grep: 'ask', + edit: 'ask', + write: 'ask', + bash: 'ask', + task: 'ask', + websearch: 'ask', + webfetch: 'ask', + question: 'deny', + }; +} + +export function buildOpenCodePermissionConfig(mode?: PermissionMode): OpenCodePermissionAction | Record { + if (mode === 'readonly') return 'deny'; + if (mode === 'full') return 'allow'; + return buildPermissionMap(mode); +} + +export function buildOpenCodePermissionRuleset(mode?: PermissionMode): Array<{ permission: string; pattern: string; action: OpenCodePermissionAction }> { + const permissionMap = buildPermissionMap(mode); + return OPEN_CODE_PERMISSION_KEYS.map((permission) => ({ + permission, + pattern: '**', + action: permissionMap[permission], + })); +} + +const BUILTIN_TOOL_MAP: Record = { + Read: 'read', + Glob: 'glob', + Grep: 'grep', + Edit: 'edit', + Write: 'write', + Bash: 'bash', + WebSearch: 'websearch', + WebFetch: 'webfetch', +}; + +export function mapToOpenCodeTools(allowedTools?: string[]): Record | undefined { + if (!allowedTools || allowedTools.length === 0) { + return undefined; + } + + const mapped = new Set(); + for (const tool of allowedTools) { + const normalized = tool.trim(); + if (!normalized) { + continue; + } + const mappedTool = BUILTIN_TOOL_MAP[normalized] ?? normalized; + mapped.add(mappedTool); + } + + if (mapped.size === 0) { + return undefined; + } + + const tools: Record = {}; + for (const tool of mapped) { + tools[tool] = true; + } + return tools; +} + /** Options for calling OpenCode */ export interface OpenCodeCallOptions { cwd: string; @@ -26,6 +159,7 @@ export interface OpenCodeCallOptions { sessionId?: string; model: string; systemPrompt?: string; + allowedTools?: string[]; /** Permission mode for automatic permission handling */ permissionMode?: PermissionMode; /** Enable streaming mode with callback (best-effort) */ diff --git a/src/infra/providers/opencode.ts b/src/infra/providers/opencode.ts index d5df0aa..19e9798 100644 --- a/src/infra/providers/opencode.ts +++ b/src/infra/providers/opencode.ts @@ -17,6 +17,7 @@ function toOpenCodeOptions(options: ProviderCallOptions): OpenCodeCallOptions { abortSignal: options.abortSignal, sessionId: options.sessionId, model: options.model, + allowedTools: options.allowedTools, permissionMode: options.permissionMode, onStream: options.onStream, onAskUserQuestion: options.onAskUserQuestion,