From 5578d90e58906a845364ff7466dac43aa0201aeb Mon Sep 17 00:00:00 2001 From: phantom5099 <1011668688@qq.com> Date: Wed, 10 Jun 2026 21:31:05 +0800 Subject: [PATCH 1/2] delete core-scope and propagate the error upward --- packages/codingcode/src/client/direct.ts | 2 +- packages/codingcode/src/client/http.ts | 3 ++- .../src/client/http/agent-runtime.ts | 3 ++- packages/codingcode/src/client/types.ts | 2 +- packages/codingcode/src/core/result.ts | 6 ----- packages/codingcode/src/core/scope.ts | 10 -------- packages/codingcode/src/server/adapter.ts | 2 +- packages/codingcode/src/server/handler.ts | 2 ++ .../codingcode/test/client/direct.test.ts | 22 ++++++++++++++++ .../test/client/http/agent-runtime.test.ts | 14 +++++------ .../codingcode/test/server/adapter.test.ts | 1 + .../codingcode/test/server/handler.test.ts | 25 ++++++++++++++++--- packages/desktop/shared/types.ts | 2 +- packages/desktop/src/hooks/useAgent.ts | 9 +++++-- packages/tui/src/hooks/useAgentRunner.ts | 2 +- 15 files changed, 70 insertions(+), 35 deletions(-) delete mode 100644 packages/codingcode/src/core/scope.ts diff --git a/packages/codingcode/src/client/direct.ts b/packages/codingcode/src/client/direct.ts index e70bc53..c62fd9b 100644 --- a/packages/codingcode/src/client/direct.ts +++ b/packages/codingcode/src/client/direct.ts @@ -47,7 +47,7 @@ export async function* agentEventToStreamChunk( yield { type: 'approval_request', id: event.id, tool: event.tool, args: event.args }; break; case 'Error': - yield { type: 'error', message: event.error.message ?? String(event.error) }; + yield { type: 'error', message: event.error.message ?? String(event.error), code: event.error.code }; break; case 'Done': yield { type: 'done' }; diff --git a/packages/codingcode/src/client/http.ts b/packages/codingcode/src/client/http.ts index 40db6e1..59ed3fd 100644 --- a/packages/codingcode/src/client/http.ts +++ b/packages/codingcode/src/client/http.ts @@ -87,7 +87,8 @@ export async function createHttpClient(serverUrl: string): Promise }; break; case 'error': - throw new Error(data.message as string); + yield { type: 'error', message: data.message as string, code: data.code as string }; + return; case 'done': break; case 'complete': diff --git a/packages/codingcode/src/client/http/agent-runtime.ts b/packages/codingcode/src/client/http/agent-runtime.ts index 07dc3c8..9cc5cb6 100644 --- a/packages/codingcode/src/client/http/agent-runtime.ts +++ b/packages/codingcode/src/client/http/agent-runtime.ts @@ -108,7 +108,8 @@ export function createHttpAgentClient( }; break; case 'error': - throw new Error(data.message as string); + yield { type: 'error', message: data.message as string, code: data.code as string }; + return; case 'done': break; case 'complete': diff --git a/packages/codingcode/src/client/types.ts b/packages/codingcode/src/client/types.ts index 7f7050a..efe0cbc 100644 --- a/packages/codingcode/src/client/types.ts +++ b/packages/codingcode/src/client/types.ts @@ -18,7 +18,7 @@ export type StreamChunk = | { type: 'tool_start'; id: string; name: string; args: Record } | { type: 'tool_result'; id: string; name: string; output: string; ok: boolean } | { type: 'tool_denied'; id: string; name: string; reason: string } - | { type: 'error'; message: string } + | { type: 'error'; message: string; code: string } | { type: 'done' } | { type: 'todo_update'; items: ReadonlyArray<{ step: string; status: string }> } | { type: 'usage'; prompt: number; completion: number; total: number } diff --git a/packages/codingcode/src/core/result.ts b/packages/codingcode/src/core/result.ts index 8ed4f4d..8e437fe 100644 --- a/packages/codingcode/src/core/result.ts +++ b/packages/codingcode/src/core/result.ts @@ -3,10 +3,4 @@ export type Result = { ok: true; value: T } | { ok: false; error: export const Result = { ok: (value: T): Result => ({ ok: true, value }), err: (error: E): Result => ({ ok: false, error }), - - map: (r: Result, fn: (v: T) => U): Result => - r.ok ? Result.ok(fn(r.value)) : r, - - flatMap: (r: Result, fn: (v: T) => Result): Result => - r.ok ? fn(r.value) : r, }; diff --git a/packages/codingcode/src/core/scope.ts b/packages/codingcode/src/core/scope.ts deleted file mode 100644 index c771b13..0000000 --- a/packages/codingcode/src/core/scope.ts +++ /dev/null @@ -1,10 +0,0 @@ -export interface ProjectScope { - projectPath: string; -} - -export interface SessionScope { - projectPath: string; - sessionId: string; -} - -export type Scope = ProjectScope | SessionScope; diff --git a/packages/codingcode/src/server/adapter.ts b/packages/codingcode/src/server/adapter.ts index 167a63b..0ac06c2 100644 --- a/packages/codingcode/src/server/adapter.ts +++ b/packages/codingcode/src/server/adapter.ts @@ -23,7 +23,7 @@ export function agentEventToSseEvent(event: AgentEvent): SseEvent | null { case 'ToolDenied': return { type: 'tool_denied', id: event.id, name: event.name, reason: event.reason }; case 'Error': - return { type: 'error', message: event.error.message ?? String(event.error) }; + return { type: 'error', message: event.error.message ?? String(event.error), code: event.error.code }; case 'Done': return { type: 'done' }; case 'TodoUpdate': diff --git a/packages/codingcode/src/server/handler.ts b/packages/codingcode/src/server/handler.ts index 1a96732..7e0a53c 100644 --- a/packages/codingcode/src/server/handler.ts +++ b/packages/codingcode/src/server/handler.ts @@ -1,6 +1,7 @@ import type { Context } from 'hono'; import { registerEmitter, unregisterEmitter } from '../approval/async-confirm.js'; import type { SseEvent } from './adapter.js'; +import { AgentError } from '../core/error.js'; export function sseHandler( createGenerator: () => AsyncGenerator, @@ -34,6 +35,7 @@ export function sseHandler( enqueue({ type: 'error', message: e instanceof Error ? e.message : String(e), + ...(e instanceof AgentError ? { code: e.code } : {}), }); } finally { unregisterEmitter(sessionId); diff --git a/packages/codingcode/test/client/direct.test.ts b/packages/codingcode/test/client/direct.test.ts index f20750b..e2a4e67 100644 --- a/packages/codingcode/test/client/direct.test.ts +++ b/packages/codingcode/test/client/direct.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from 'vitest'; import { createDirectClient, agentEventToStreamChunk } from '../../src/client/direct.js'; import { registerEmitter, unregisterEmitter } from '../../src/approval/async-confirm.js'; +import { AgentError } from '../../src/core/error.js'; const noopLlm = { completeStream: () => ({ @@ -99,4 +100,25 @@ describe('agentEventToStreamChunk - approval interleaving', () => { { type: 'usage', prompt: 1000, completion: 500, total: 1500 }, ]); }); + + it('yields error chunk with code from AgentError', async () => { + async function* source() { + yield { _tag: 'Error' as const, error: AgentError.toolExecutionFailed('bash', 'EACCES') }; + yield { _tag: 'Done' as const, content: '' }; + } + + const chunks: any[] = []; + for await (const chunk of agentEventToStreamChunk(source())) { + chunks.push(chunk); + } + + expect(chunks).toEqual([ + { + type: 'error', + message: expect.stringContaining('bash'), + code: 'TOOL_EXECUTION_FAILED', + }, + { type: 'done' }, + ]); + }); }); diff --git a/packages/codingcode/test/client/http/agent-runtime.test.ts b/packages/codingcode/test/client/http/agent-runtime.test.ts index c00e4e4..157277d 100644 --- a/packages/codingcode/test/client/http/agent-runtime.test.ts +++ b/packages/codingcode/test/client/http/agent-runtime.test.ts @@ -91,21 +91,21 @@ describe('createHttpAgentClient.sendMessage', () => { fetchSpy.mockRestore(); }); - it('throws on error event', async () => { + it('yields error event instead of throwing', async () => { const fetchSpy = vi .spyOn(globalThis, 'fetch') .mockResolvedValue( - createSseResponse([JSON.stringify({ type: 'error', message: 'something broke' })]) + createSseResponse([JSON.stringify({ type: 'error', message: 'something broke', code: 'LLM_FAILED' })]) ); const request = createRequestHelpers('http://localhost:8080'); const client = createHttpAgentClient('http://localhost:8080', request); - await expect(async () => { - for await (const _ of client.sendMessage('hi', { sessionId: 's', cwd: '/tmp' })) { - // consume - } - }).rejects.toThrow('something broke'); + const chunks: Array<{ type: string }> = []; + for await (const c of client.sendMessage('hi', { sessionId: 's', cwd: '/tmp' })) { + chunks.push(c); + } + expect(chunks).toEqual([{ type: 'error', message: 'something broke', code: 'LLM_FAILED' }]); fetchSpy.mockRestore(); }); diff --git a/packages/codingcode/test/server/adapter.test.ts b/packages/codingcode/test/server/adapter.test.ts index 7559fd0..16102c2 100644 --- a/packages/codingcode/test/server/adapter.test.ts +++ b/packages/codingcode/test/server/adapter.test.ts @@ -54,6 +54,7 @@ describe('agentEventToSseEvent', () => { expect(agentEventToSseEvent({ _tag: 'Error', error: err })).toEqual({ type: 'error', message: err.message, + code: 'LLM_FAILED', }); }); diff --git a/packages/codingcode/test/server/handler.test.ts b/packages/codingcode/test/server/handler.test.ts index 09057e6..067d21b 100644 --- a/packages/codingcode/test/server/handler.test.ts +++ b/packages/codingcode/test/server/handler.test.ts @@ -11,6 +11,7 @@ import { McpService } from '../../src/mcp/index.js'; import { Result } from '../../src/core/result.js'; import { CheckpointService } from '../../src/checkpoint/checkpoint-service.js'; import { ToolSearchService } from '../../src/tools/tool-search-service.js'; +import { AgentError } from '../../src/core/error.js'; const mockState = { sessionId: 'test-session', @@ -361,16 +362,34 @@ describe('sseHandler + sendMessage integration', () => { expect(textEvent!.text).toContain('[Using:'); }); - it('should send error event when factory throws', async () => { + it('should preserve AgentError code in catch', async () => { const handler = sseHandler( async function* () { - throw new Error('boom'); + throw AgentError.toolNotFound('myTool'); }, { sessionId: 'test' } ); const response = await handler({} as any); const { events } = await readSSEStream(response); - expect(events.some((e: any) => e.type === 'error')).toBe(true); + const errorEvent = events.find((e: any) => e.type === 'error'); + expect(errorEvent).toBeDefined(); + expect(errorEvent.code).toBe('TOOL_NOT_FOUND'); + expect(errorEvent.message).toContain('myTool'); + }); + + it('should not include code for plain Error in catch', async () => { + const handler = sseHandler( + async function* () { + throw new Error('plain error'); + }, + { sessionId: 'test' } + ); + const response = await handler({} as any); + const { events } = await readSSEStream(response); + + const errorEvent = events.find((e: any) => e.type === 'error'); + expect(errorEvent).toBeDefined(); + expect(errorEvent.code).toBeUndefined(); }); }); diff --git a/packages/desktop/shared/types.ts b/packages/desktop/shared/types.ts index c79437e..8525ce6 100644 --- a/packages/desktop/shared/types.ts +++ b/packages/desktop/shared/types.ts @@ -22,7 +22,7 @@ export type Item = insertions?: number; deletions?: number; } - | { id: string; type: 'error'; message: string }; + | { id: string; type: 'error'; message: string; code?: string }; export type TodoStatus = 'pending' | 'in_progress' | 'completed'; diff --git a/packages/desktop/src/hooks/useAgent.ts b/packages/desktop/src/hooks/useAgent.ts index 6eede4c..bc2f6b3 100644 --- a/packages/desktop/src/hooks/useAgent.ts +++ b/packages/desktop/src/hooks/useAgent.ts @@ -182,7 +182,7 @@ export function useAgentCore() { status: 'rejected', }; case 'error': - return { id: randomId(), type: 'error', message: event.message }; + return { id: randomId(), type: 'error', message: event.message, code: event.code }; case 'todo_update': applyTodoUpdate(threadId, event.items as any); return null; @@ -257,9 +257,14 @@ export function useAgentCore() { signal: controller.signal, }); + let hasError = false; for await (const event of stream) { if (event.type === 'session_id') continue; + if (event.type === 'error') { + hasError = true; + } + const item = streamChunkToItem(event, threadId, assistantMessageId, turnId); if (item) { applyChunk(threadId, turnId, item); @@ -274,7 +279,7 @@ export function useAgentCore() { } } - completeTurn(threadId, turnId, 'completed'); + completeTurn(threadId, turnId, hasError ? 'error' : 'completed'); } catch (err: any) { const msg = err instanceof ApiError ? (err.body?.message ?? err.message) : String(err); applyChunk(threadId, turnId, { id: randomId(), type: 'error', message: msg }); diff --git a/packages/tui/src/hooks/useAgentRunner.ts b/packages/tui/src/hooks/useAgentRunner.ts index 45b6746..c74f415 100644 --- a/packages/tui/src/hooks/useAgentRunner.ts +++ b/packages/tui/src/hooks/useAgentRunner.ts @@ -119,7 +119,7 @@ export function useAgentRunner(runner: (input: string) => AsyncGenerator Date: Wed, 10 Jun 2026 22:20:50 +0800 Subject: [PATCH 2/2] delete promptUser --- .../codingcode/src/approval/confirmation.ts | 82 ------------------- packages/codingcode/src/approval/index.ts | 4 +- packages/codingcode/src/approval/pipeline.ts | 41 +++++----- packages/codingcode/src/approval/presets.ts | 2 +- .../codingcode/test/approval/pipeline.test.ts | 14 +--- packages/desktop/electron/main.ts | 2 - 6 files changed, 24 insertions(+), 121 deletions(-) diff --git a/packages/codingcode/src/approval/confirmation.ts b/packages/codingcode/src/approval/confirmation.ts index 48f1c71..0dfaa89 100644 --- a/packages/codingcode/src/approval/confirmation.ts +++ b/packages/codingcode/src/approval/confirmation.ts @@ -1,4 +1,3 @@ -import * as readline from 'node:readline'; import { Effect } from 'effect'; import type { PermissionRule } from './types.js'; import { ApprovalWaitService } from './async-confirm.js'; @@ -9,87 +8,6 @@ export type ConfirmResult = | { type: 'always'; rule: PermissionRule } | { type: 'never'; rule: PermissionRule }; -async function promptUser(question: string): Promise { - const rl = readline.createInterface({ - input: process.stdin, - output: process.stdout, - terminal: true, - }); - try { - return await new Promise((resolve) => { - rl.question(question, (ans) => resolve(ans.trim().toLowerCase())); - }); - } finally { - rl.close(); - } -} - -function buildResult(answer: string, tool: string): ConfirmResult { - switch (answer) { - case 'y': - return { type: 'allow' }; - case 'n': - return { type: 'deny' }; - case 'a': - return { - type: 'always', - rule: { - id: `user-allow-${tool}-${Date.now()}`, - action: 'allow', - toolPattern: tool, - reason: 'User always allows', - source: 'user', - }, - }; - case 'r': - return { - type: 'never', - rule: { - id: `user-deny-${tool}-${Date.now()}`, - action: 'deny', - toolPattern: tool, - reason: 'User never allows', - source: 'user', - }, - }; - default: - return { type: 'deny' }; - } -} - -export function userConfirm( - tool: string, - args: Record, - mode: 'interactive' | 'default-deny' = 'default-deny' -): Effect.Effect { - if (mode === 'default-deny') { - return Effect.succeed({ type: 'deny' } as ConfirmResult); - } - - return Effect.gen(function* () { - const serializedArgs = Object.entries(args) - .map(([k, v]) => ` ${k}: ${String(v).slice(0, 200)}`) - .join('\n'); - - const question = `\n[Approval] Tool "${tool}" wants to run:\n${serializedArgs}\nAllow? (Y)es / (N)o / (A)lways / Neve_r / (V)iew full: `; - - const answer = yield* Effect.promise(() => promptUser(question)); - - if (answer === 'v') { - console.log('\nFull arguments:', JSON.stringify(args, null, 2)); - const result = yield* userConfirm(tool, args, 'interactive'); - return result; - } - - return buildResult(answer, tool); - }); -} - -/** - * Async confirmation via SSE: sends an approval request to the TUI client - * and waits for the response via Effect.async + Deferred. - * @param waitSvc injected as parameter to keep R channel clean. - */ export function userConfirmAsync( tool: string, args: Record, diff --git a/packages/codingcode/src/approval/index.ts b/packages/codingcode/src/approval/index.ts index 2c71b74..9a70929 100644 --- a/packages/codingcode/src/approval/index.ts +++ b/packages/codingcode/src/approval/index.ts @@ -34,7 +34,7 @@ export class ApprovalService extends Effect.Service()('Approval if (result && result.decision === 'continue') { return null; } - return result as any; + return result; }), recordAudit: (entry) => @@ -71,7 +71,6 @@ export class ApprovalService extends Effect.Service()('Approval destructiveTools: destTools, permissionMode: currentPermMode, hooks: buildPipelineHooks(), - interactive: process.stdin.isTTY ?? false, asyncConfirm: hasEmitter(request.sessionId), asyncConfirmService: approvalWait, onAlways: (rule) => engine.addRule(rule), @@ -142,7 +141,6 @@ export class ApprovalService extends Effect.Service()('Approval destructiveTools, permissionMode: _globalPermissionMode, hooks: buildPipelineHooks(), - interactive: process.stdin.isTTY ?? false, asyncConfirm: hasEmitter(request.sessionId), asyncConfirmService: approvalWait, onAlways: (rule) => ruleEngine.addRule(rule), diff --git a/packages/codingcode/src/approval/pipeline.ts b/packages/codingcode/src/approval/pipeline.ts index a591f5c..f3da50b 100644 --- a/packages/codingcode/src/approval/pipeline.ts +++ b/packages/codingcode/src/approval/pipeline.ts @@ -1,19 +1,16 @@ import { Effect } from 'effect'; import type { ApprovalDecision, PermissionMode, PermissionRule, ToolCallRequest } from './types.js'; import type { RuleEngine } from './rule-engine.js'; -import { userConfirm, userConfirmAsync } from './confirmation.js'; +import { userConfirmAsync } from './confirmation.js'; import type { ApprovalWaitService } from './async-confirm.js'; +import type { HookDecision } from '../hooks/registry.js'; export interface PipelineHooks { /** Emit decision from PreToolUse hooks (Layer 4). Returns first non-null HookDecision or null. */ emitPreToolUseDecision: (payload: { toolName: string; args: Record; - }) => Effect.Effect<{ - decision?: 'allow' | 'deny' | 'ask'; - reason?: string; - modifiedInput?: Record; - } | null>; + }) => Effect.Effect; /** Record audit log for the final decision (Layer 6). */ recordAudit: (entry: { tool: string; @@ -29,8 +26,6 @@ export interface PipelineOptions { destructiveTools: Set; permissionMode: PermissionMode; hooks: PipelineHooks; - /** Whether TTY is available for interactive confirmation. */ - interactive: boolean; /** Use async SSE-based confirmation instead of blocking readline. */ asyncConfirm?: boolean; /** Service for async confirmation (injected to keep R clean). */ @@ -132,19 +127,23 @@ export function runPipeline( // Layer 5: User Confirmation { layers.push(LAYER_NAMES[4]); - const confirmResult = yield* opts.asyncConfirm && opts.asyncConfirmService - ? userConfirmAsync( - request.tool, - request.input, - opts.asyncConfirmService, - opts.sessionId, - opts.callId - ) - : userConfirm( - request.tool, - request.input, - opts.interactive ? 'interactive' : 'default-deny' - ); + if (!opts.asyncConfirm || !opts.asyncConfirmService) { + const result: ApprovalDecision = { + type: 'deny', + reason: 'Approval required but no UI available', + source: 'system', + }; + const final = yield* layer6Audit(request, result, layers, opts); + return final; + } + + const confirmResult = yield* userConfirmAsync( + request.tool, + request.input, + opts.asyncConfirmService, + opts.sessionId, + opts.callId + ); let result: ApprovalDecision; switch (confirmResult.type) { diff --git a/packages/codingcode/src/approval/presets.ts b/packages/codingcode/src/approval/presets.ts index 34e7453..aa908d1 100644 --- a/packages/codingcode/src/approval/presets.ts +++ b/packages/codingcode/src/approval/presets.ts @@ -90,7 +90,7 @@ export const READONLY_TOOL_NAMES: string[] = [ 'search_files', 'fetch_url', 'web_search', - 'tool_search', + 'dispatch_agent', 'todo_write', ]; diff --git a/packages/codingcode/test/approval/pipeline.test.ts b/packages/codingcode/test/approval/pipeline.test.ts index b71c5cd..423f850 100644 --- a/packages/codingcode/test/approval/pipeline.test.ts +++ b/packages/codingcode/test/approval/pipeline.test.ts @@ -26,7 +26,6 @@ describe('Approval Pipeline', () => { destructiveTools: new Set(), permissionMode: 'default', hooks: mockHooks, - interactive: false, sessionId: 'test', } ) @@ -45,7 +44,6 @@ describe('Approval Pipeline', () => { destructiveTools: new Set(), permissionMode: 'default', hooks: mockHooks, - interactive: false, sessionId: 'test', } ) @@ -64,7 +62,6 @@ describe('Approval Pipeline', () => { destructiveTools: new Set(['Bash']), permissionMode: 'plan', hooks: mockHooks, - interactive: false, sessionId: 'test', } ) @@ -83,7 +80,6 @@ describe('Approval Pipeline', () => { destructiveTools: new Set(), permissionMode: 'plan', hooks: mockHooks, - interactive: false, sessionId: 'test', } ) @@ -101,7 +97,6 @@ describe('Approval Pipeline', () => { destructiveTools: new Set(['Bash']), permissionMode: 'bypass', hooks: mockHooks, - interactive: false, sessionId: 'test', } ) @@ -120,7 +115,6 @@ describe('Approval Pipeline', () => { destructiveTools: new Set(['Bash', 'execute_command']), permissionMode: 'acceptEdits', hooks: mockHooks, - interactive: false, sessionId: 'test', } ) @@ -138,14 +132,13 @@ describe('Approval Pipeline', () => { destructiveTools: new Set(['Bash', 'execute_command']), permissionMode: 'acceptEdits', hooks: mockHooks, - interactive: false, sessionId: 'test', } ) ); - // Should continue to user confirmation layer (which returns deny in non-interactive mode) + // Destructive tool in acceptEdits mode with no UI available → system deny expect(decision.type).toBe('deny'); - expect((decision as any).source).toBe('user-confirm'); + expect((decision as any).source).toBe('system'); }); it('Layer 4: PreToolUse hook can deny (non-readonly tool)', async () => { @@ -163,7 +156,6 @@ describe('Approval Pipeline', () => { destructiveTools: new Set(['Bash']), permissionMode: 'default', hooks: hooksWithDeny, - interactive: false, sessionId: 'test', } ) @@ -186,7 +178,6 @@ describe('Approval Pipeline', () => { destructiveTools: new Set(['Bash']), permissionMode: 'default', hooks: hooksWithAllow, - interactive: false, sessionId: 'test', } ) @@ -213,7 +204,6 @@ describe('Approval Pipeline', () => { destructiveTools: new Set(), permissionMode: 'default', hooks: hooksWithAudit, - interactive: false, sessionId: 'test', } ) diff --git a/packages/desktop/electron/main.ts b/packages/desktop/electron/main.ts index e1eae92..19bfbbe 100644 --- a/packages/desktop/electron/main.ts +++ b/packages/desktop/electron/main.ts @@ -20,8 +20,6 @@ function createWindow(apiPort: number): BrowserWindow { const win = new BrowserWindow({ width: 1280, height: 800, - minWidth: 900, - minHeight: 600, backgroundColor: '#1e1e1e', titleBarStyle: 'hidden', trafficLightPosition: { x: 12, y: 12 },