From 56b8b10d46088c73963808f4ea1a2ca987ed6e04 Mon Sep 17 00:00:00 2001 From: mesalogo Date: Mon, 5 Oct 2026 15:20:17 +0800 Subject: [PATCH] fix: let direct models continue after tool execution failures Return ordinary tool exceptions as paired error results so the model can decide how to recover. Preserve cancellation, existing recovery guidance, and failed tool activities without adding automatic retries or retry limits. Validate all three text protocols with real process execution and cover bounded errors and cancellation. Focused tests: 161 passed, 1 skipped; typecheck and lint passed. Full suite: 6230 passed, 87 skipped, 1 Obsidian temporary-directory assertion failed. Release note: Direct-model conversations can continue after a tool fails, allowing the model to use the reported error to choose its next step. --- docs/features/direct-model-agent/progress.md | 19 +- .../direct-model-agent/technical-design.md | 7 +- docs/features/unified-execution/progress.md | 5 + src/main/agent/model-runtime.test.ts | 212 ++++++++++++++++++ src/main/agent/model-runtime.ts | 32 +-- 5 files changed, 259 insertions(+), 16 deletions(-) diff --git a/docs/features/direct-model-agent/progress.md b/docs/features/direct-model-agent/progress.md index 9037b8fb..8d5e48b1 100644 --- a/docs/features/direct-model-agent/progress.md +++ b/docs/features/direct-model-agent/progress.md @@ -10,7 +10,24 @@ | 日期 | 2026-09-09 | | 关联入口 | [直连模型 Agent 能力](./README.md) | -## 当前结论 +## 2026-10-05 工具执行异常回传 + +- 普通工具执行异常也返回模型，保留失败活动与错误详情，由模型决定后续行动；规则见 + [失败恢复](./technical-design.md#读取与搜索失败恢复)。请求取消与 `AbortError` 仍停止循环。 +- 新增三协议回归使用真实 Provider、进程服务和文件系统：无效 cwd 产生 `ENOENT`， + 下一次模拟模型请求收到对应失败结果，再调用同一工具执行真实命令成功。另覆盖错误详情 + 有界处理和取消；未使用真实外部模型，不作为桌面 UI 或真实模型自主决策验收。 +- 修复验证中发现 `DOMException` 不一定满足 `instanceof Error`，改为按错误名称识别取消； + 取消测试在意外后续模型请求时立即失败，避免模拟响应无限重复耗尽内存。 +- `model-runtime.test.ts`、`model-tool-provider.test.ts`、`direct-model-process-service.test.ts`： + 161 项通过、1 项跳过。`npm run typecheck`、`npm run lint` 通过。 +- 全量 `npm test`：6,230 项通过、87 项跳过、1 项失败；失败为 + `obsidian-electron.test.ts:29` 要求临时目录在仓库外，与本次按仓库规范设置 + `TEMP`/`TMP` 到仓库 `temp/` 冲突。未修改该测试，全量不能记为通过。 +- 托管 SSH 使用远端 OpenCode/Continue 的工具循环，不运行本次修改的 `ModelAgentRuntime`； + 不涉及 gbagent 生产路径。临时目录创建、保留和清理逻辑未修改。 + +## 既有本机验收结论 本机生产路径已经完成源码接线：直连模型 Execute 可运行平台 Shell，Ask/Execute 均可按 父模式委派单层编程 Subagent；长输出可通过 `output_read` 续读。OpenCode、Continue、 diff --git a/docs/features/direct-model-agent/technical-design.md b/docs/features/direct-model-agent/technical-design.md index e07a9f0f..e7409e1a 100644 --- a/docs/features/direct-model-agent/technical-design.md +++ b/docs/features/direct-model-agent/technical-design.md @@ -98,8 +98,11 @@ type BuiltinModelToolSummary = { `EISDIR`、`EACCES`、`EPERM`、`ELOOP`、`ENAMETOOLONG` 等已列明的文件系统错误。 Runtime 向模型返回 `{ ok: false, recoverable: true, error, nextAction }` 的 JSON 文本工具结果， 工具活动记为 `recoverable`，继续工具循环，由模型修正路径、分页参数、glob 或搜索表达式； -提示不得原样重复失败参数。取消、远端断线、失效工作区、缺失 rg 可执行文件及未分类的内部 -错误继续向外传播，不转为可恢复结果。 +提示不得原样重复失败参数。其他工具执行异常（包括进程启动或输出文件的 `ENOENT`、 +远端断线、失效工作区、缺失 rg 可执行文件）由 Runtime 返回 `{ ok: false, error }`， +工具活动记为 `failed`，错误详情使用已有有界处理并保留 cause 信息。失败结果继续进入 +下一次模型请求，由模型决定重试、调整方法或说明失败；不自动重放工具，也不增加重试次数限制。 +Anthropic 的失败结果同时设置 `is_error: true`。请求取消及 `AbortError` 仍向外传播并停止循环。 `workspace_read_text` 仍只接受工作区内相对路径。搜索原样接受 rg 参数及账号可访问的路径， 不增加逐工具授权；保留 `--no-config` 默认参数、原生退出码、取消和分页，不提升系统权限。 diff --git a/docs/features/unified-execution/progress.md b/docs/features/unified-execution/progress.md index 844a0874..876daa51 100644 --- a/docs/features/unified-execution/progress.md +++ b/docs/features/unified-execution/progress.md @@ -259,3 +259,8 @@ SSH 专家保留父请求已解析的 OpenCode 或 Continue 选择，通过现 `deai-writing` 扫描九份文档，阻断项为 0。复核项主要为未验证状态、数据保留条件和字段枚举；结合技术文档语境保留这些限定，不将迁移计划改写为已完成事实。 `git diff --check -- docs/features/unified-execution docs/features/assistant-workbar/README.md` 通过已跟踪改动的空白检查。新建文档另经内容与链接核对；该命令不会把未跟踪文件当作已验证差异。文档检查不替代运行时验收，后者按用户故事与技术设计执行。 + +## 2026-10-05 直连模型工具错误回传 + +普通工具执行失败不再直接终止直连模型循环，错误结果继续交给模型处理，取消行为保留。 +实现、三协议回归及全量测试边界见[直连模型修复记录](../direct-model-agent/progress.md#2026-10-05-工具执行异常回传)。 diff --git a/src/main/agent/model-runtime.test.ts b/src/main/agent/model-runtime.test.ts index ad9c87ed..fea4ec54 100644 --- a/src/main/agent/model-runtime.test.ts +++ b/src/main/agent/model-runtime.test.ts @@ -4017,6 +4017,218 @@ describe('ModelAgentRuntime', () => { expect(events.at(-1)).toMatchObject({ type: 'done' }) }) + it.each(['openai-chat-completions', 'openai-responses', 'anthropic-messages'] as const)( + 'returns ordinary ENOENT to %s and lets the model retry the same production tool', + async (protocol) => { + const tempRoot = join(process.cwd(), 'temp') + await mkdir(tempRoot, { recursive: true }) + const workspace = await mkdtemp(join(tempRoot, 'model-tool-enoent-')) + const workspaceAccess = new LocalWorkspaceAccess(workspace) + const processService = new LocalDirectModelProcessService() + const toolProvider = new ModelToolProvider( + workspaceAccess, [], undefined, undefined, false, { processService } + ) + const marker = 'process-retry-succeeded' + const calls = ['missing-cwd', '.'].map((cwd, index) => ({ + id: `process-${index}`, + name: 'process_execute', + arguments: { command: `echo ${marker}`, cwd, timeoutMs: 10_000 } + })) + const responses = calls.map((call) => { + if (protocol === 'anthropic-messages') { + return { content: [{ type: 'tool_use', id: call.id, name: call.name, input: call.arguments }], stop_reason: 'tool_use' } + } + if (protocol === 'openai-responses') { + return { id: `response-${call.id}`, output: [{ + type: 'function_call', id: `fc-${call.id}`, call_id: call.id, + name: call.name, arguments: JSON.stringify(call.arguments) + }] } + } + return { choices: [{ message: { role: 'assistant', content: null, tool_calls: [{ + id: call.id, type: 'function', + function: { name: call.name, arguments: JSON.stringify(call.arguments) } + }] } }] } + }) + const fetcher = vi.fn(async () => { + const response = responses.shift() + if (response) return Response.json(response) + if (protocol === 'anthropic-messages') { + return Response.json({ content: [{ type: 'text', text: 'Command completed.' }], stop_reason: 'end_turn' }) + } + if (protocol === 'openai-responses') { + return Response.json({ id: 'response-final', output: [{ type: 'message', role: 'assistant', content: [{ type: 'output_text', text: 'Command completed.' }] }] }) + } + return Response.json({ choices: [{ message: { role: 'assistant', content: 'Command completed.' } }] }) + }) + const runtime = new ModelAgentRuntime({ + baseUrl: 'https://example.test/v1', model: 'test-model', + protocol, authentication: 'none', fetcher, toolProvider, workspaceAccess + }) + const events: RuntimeEvent[] = [] + try { + for await (const event of runtime.run({ + requestId: crypto.randomUUID(), conversationId: `enoent-${protocol}`, + prompt: 'Run the command and correct the working directory if needed.' + }, new AbortController().signal)) events.push(event) + + expect(fetcher).toHaveBeenCalledTimes(3) + let failure: unknown + for (const requestIndex of [1, 2]) { + const body = JSON.parse(String(fetcher.mock.calls[requestIndex]![1]!.body)) + const entries = (body.input ?? body.messages) as Array> + for (let callIndex = 0; callIndex < requestIndex; callIndex++) { + const call = calls[callIndex]! + let text: string + if (protocol === 'anthropic-messages') { + const blocks = entries.flatMap((entry) => Array.isArray(entry.content) + ? entry.content as Array> : []) + expect(blocks.filter((block) => block.type === 'tool_use' && block.id === call.id)).toEqual([ + expect.objectContaining({ name: call.name, input: call.arguments }) + ]) + const results = blocks.filter((block) => block.type === 'tool_result' && block.tool_use_id === call.id) + expect(results).toHaveLength(1) + expect(results[0]!.is_error).toBe(callIndex === 0 ? true : undefined) + text = (results[0]!.content as Array<{ text: string }>)[0]!.text + } else if (protocol === 'openai-responses') { + expect(entries.filter((entry) => entry.type === 'function_call' && entry.call_id === call.id)).toEqual([ + expect.objectContaining({ name: call.name, arguments: JSON.stringify(call.arguments) }) + ]) + const results = entries.filter((entry) => entry.type === 'function_call_output' && entry.call_id === call.id) + expect(results).toHaveLength(1) + text = (results[0]!.output as Array<{ text: string }>)[0]!.text + } else { + const toolCalls = entries.flatMap((entry) => Array.isArray(entry.tool_calls) + ? entry.tool_calls as Array> : []) + expect(toolCalls.filter((entry) => entry.id === call.id)).toEqual([ + expect.objectContaining({ function: { name: call.name, arguments: JSON.stringify(call.arguments) } }) + ]) + const results = entries.filter((entry) => entry.role === 'tool' && entry.tool_call_id === call.id) + expect(results).toHaveLength(1) + text = results[0]!.content as string + } + const result = JSON.parse(text) + if (callIndex === 0) { + expect(result).toEqual({ ok: false, error: expect.stringContaining('ENOENT') }) + expect(result.error).toContain('missing-cwd') + expect(result.error.length).toBeLessThanOrEqual(2_000) + if (requestIndex === 1) failure = result + else expect(result).toEqual(failure) + } else { + expect(result).toMatchObject({ exitCode: 0, stdout: expect.stringContaining(marker), stderr: '' }) + } + } + } + const toolEvents = events.filter((event) => event.type === 'tool') + expect(toolEvents.map((event) => [event.callId, event.state])).toEqual([ + ['process-0', 'pending'], ['process-0', 'running'], ['process-0', 'failed'], + ['process-1', 'pending'], ['process-1', 'running'], ['process-1', 'completed'] + ]) + expect(toolEvents[2]).toMatchObject({ error: (failure as { error: string }).error }) + expect(events).toContainEqual(expect.objectContaining({ type: 'text', delta: 'Command completed.' })) + expect(events.at(-1)).toMatchObject({ type: 'done' }) + } finally { + try { + await runtime.dispose() + await processService.dispose() + } finally { + await rm(workspace, { recursive: true, force: true }) + } + } + } + ) + + it('returns bounded ordinary tool error detail without control characters or a stack', async () => { + const error = new Error(`ENOENT\u0000: missing cwd ${'x'.repeat(4_000)} END-OF-DETAIL`) + error.stack = 'PRIVATE-STACK-MARKER' + const toolProvider = createToolProvider({ + callTool: vi.fn(async () => { throw error }) + }) + const fetcher = vi.fn() + .mockResolvedValueOnce(Response.json({ choices: [{ message: { + role: 'assistant', content: null, tool_calls: [{ + id: 'bounded-error', type: 'function', + function: { name: 'workspace_read_text', arguments: '{"path":"README.md"}' } + }] + } }] })) + .mockResolvedValueOnce(Response.json({ choices: [{ message: { + role: 'assistant', content: 'Reported failure.' + } }] })) + const runtime = new ModelAgentRuntime({ + baseUrl: 'https://example.test/v1', model: 'test-model', + protocol: 'openai-chat-completions', authentication: 'none', fetcher, toolProvider + }) + const events: RuntimeEvent[] = [] + try { + for await (const event of runtime.run({ + requestId: crypto.randomUUID(), conversationId: 'bounded-tool-error', + prompt: 'Read the file.' + }, new AbortController().signal)) events.push(event) + + expect(fetcher).toHaveBeenCalledTimes(2) + const body = JSON.parse(String(fetcher.mock.calls[1]![1]!.body)) as { + messages: Array<{ role: string; tool_call_id?: string; content: string }> + } + const results = body.messages.filter((message) => message.role === 'tool') + expect(results).toHaveLength(1) + expect(results[0]!.tool_call_id).toBe('bounded-error') + const result = JSON.parse(results[0]!.content) + expect(result).toEqual({ ok: false, error: expect.stringContaining('ENOENT: missing cwd') }) + expect(result.error.length).toBeLessThanOrEqual(2_000) + expect(result.error).not.toContain('\u0000') + expect(result.error).not.toContain('END-OF-DETAIL') + expect(result.error).not.toContain('PRIVATE-STACK-MARKER') + expect(events.filter((event) => event.type === 'tool').map((event) => event.state)) + .toEqual(['pending', 'running', 'failed']) + expect(events).toContainEqual(expect.objectContaining({ + type: 'tool', callId: 'bounded-error', state: 'failed', error: result.error + })) + expect(events.at(-1)).toMatchObject({ type: 'done' }) + } finally { + await runtime.dispose() + } + }) + + it.each(['AbortSignal', 'AbortError'] as const)( + 'propagates %s during tool execution without sending a failure result to the model', + async (cancellation) => { + const controller = new AbortController() + const error = cancellation === 'AbortError' + ? new DOMException('Tool cancelled', 'AbortError') + : new Error('Tool cancelled by user') + const toolProvider = createToolProvider({ + callTool: vi.fn(async () => { + if (cancellation === 'AbortSignal') controller.abort(error) + throw error + }) + }) + const fetcher = vi.fn().mockResolvedValueOnce(Response.json({ choices: [{ message: { + role: 'assistant', content: null, tool_calls: [{ + id: 'cancelled-call', type: 'function', + function: { name: 'workspace_read_text', arguments: '{"path":"README.md"}' } + }] + } }] })).mockRejectedValue(new Error('Unexpected model request after cancellation')) + const runtime = new ModelAgentRuntime({ + baseUrl: 'https://example.test/v1', model: 'test-model', + protocol: 'openai-chat-completions', authentication: 'none', fetcher, toolProvider + }) + const events: RuntimeEvent[] = [] + try { + const consume = async () => { + for await (const event of runtime.run({ + requestId: crypto.randomUUID(), conversationId: `tool-cancellation-${cancellation}`, + prompt: 'Read the file.' + }, controller.signal)) events.push(event) + } + await expect(consume()).rejects.toBe(error) + expect(toolProvider.callTool).toHaveBeenCalledOnce() + expect(fetcher).toHaveBeenCalledOnce() + expect(events).not.toContainEqual(expect.objectContaining({ type: 'done' })) + } finally { + await runtime.dispose() + } + } + ) + it('returns recoverable tool failures to the model instead of aborting the run', async () => { const responses = [ { diff --git a/src/main/agent/model-runtime.ts b/src/main/agent/model-runtime.ts index a3b275de..1705abc2 100644 --- a/src/main/agent/model-runtime.ts +++ b/src/main/agent/model-runtime.ts @@ -882,14 +882,18 @@ function getToolResultPreview(parts: ModelToolResultPart[]): string { .slice(0, 16_000) } -function createRecoverableToolErrorResult( - error: RecoverableModelToolError +function createToolErrorResult( + error: unknown ): ModelToolResult { const text = JSON.stringify({ ok: false, - recoverable: true, - error: error.message.slice(0, 1_000), - nextAction: error.nextAction.slice(0, 1_000) + ...(error instanceof RecoverableModelToolError + ? { + recoverable: true, + error: error.message.slice(0, 1_000), + nextAction: error.nextAction.slice(0, 1_000) + } + : { error: safeToolErrorDetail(error) ?? 'Tool execution failed' }) }) return { parts: [{ type: 'text', text }], @@ -3811,6 +3815,14 @@ export class ModelAgentRuntime implements AgentRuntime { } result = step.value } catch (error) { + signal.throwIfAborted() + if (typeof error === 'object' && error !== null && ( + ('name' in error && error.name === 'AbortError') || + ('cause' in error && typeof error.cause === 'object' && error.cause !== null && + 'name' in error.cause && error.cause.name === 'AbortError') + )) { + throw error + } const recoverable = error instanceof RecoverableModelToolError const detail = safeToolErrorDetail(error) if (showToolActivity) { @@ -3828,14 +3840,8 @@ export class ModelAgentRuntime implements AgentRuntime { ...(detail ? { error: detail } : {}) } } - if (recoverable) { - result = createRecoverableToolErrorResult(error) - toolFailed = true - } else { - throw new Error(`工具「${displayName}」执行失败`, { - cause: error - }) - } + result = createToolErrorResult(error) + toolFailed = true } roundContextBytes += validateToolResult(result) if (responses) {