Przeglądaj źródła

fix(tools): give the unknown-tool failure a structured error code

The registry's unknown-tool branch returned isError text with no { name, code },
so a model-requested unknown tool logged an unroutable tool/result — a gap in
exactly the taxonomy this PR adds. Introduce ToolNotFoundError (HarnessError,
code UNKNOWN_TOOL) and route the unknown-tool case through the same catch as a
tool-thrown error, so both failure classes surface structured error metadata
from one path. Addresses PR review finding.
Tianyi Cui 3 miesięcy temu
rodzic
commit
bed91b46d5
2 zmienionych plików z 35 dodań i 12 usunięć
  1. 21 11
      packages/tools/src/index.ts
  2. 14 1
      packages/tools/tests/tools.spec.ts

+ 21 - 11
packages/tools/src/index.ts

@@ -72,6 +72,19 @@ export interface ToolErrorInfo {
   code: string
 }
 
+/**
+ * Thrown (internally) when the model requests a tool that isn't registered.
+ * Extends {@link HarnessError} (`code: 'UNKNOWN_TOOL'`) so an unknown-tool
+ * failure is as routable as a tool-thrown one — retry/sandbox/replay code can
+ * distinguish it from a tool body's own error.
+ */
+export class ToolNotFoundError extends HarnessError {
+  constructor(public readonly toolName: string) {
+    super(`unknown tool "${toolName}"`, 'UNKNOWN_TOOL')
+    this.name = 'ToolNotFoundError'
+  }
+}
+
 /** The outcome of one tool call. */
 export interface ToolExecutionResult {
   callId: CallId
@@ -160,21 +173,18 @@ export class ToolRegistry extends Service {
 
   /**
    * Execute one tool call through the `tools/execute` waterfall. If the tool
-   * is not registered, returns an `isError` result immediately (no waterfall).
-   * If the tool throws, the error is caught and returned as an `isError` result
-   * so the loop never sees an uncaught exception from a tool.
+   * is not registered, the result is an `isError` carrying a `UNKNOWN_TOOL`
+   * structured error. If the tool throws, the error is caught and returned as
+   * an `isError` result so the loop never sees an uncaught exception; a thrown
+   * {@link HarnessError} surfaces its `{ name, code }` on the result.
    */
   execute(exec: ToolExecution): Promise<ToolExecutionResult> {
     return this.ctx.waterfall(this, 'tools/execute', exec, async (): Promise<ToolExecutionResult> => {
-      const tool = this.store.get(exec.name)
-      if (!tool) {
-        return {
-          callId: exec.callId,
-          content: [{ type: 'text', text: `Error: unknown tool "${exec.name}"` }],
-          isError: true,
-        }
-      }
       try {
+        const tool = this.store.get(exec.name)
+        // Unknown tool routes through the same catch as a tool-thrown error, so
+        // both failure classes get structured `{ name, code }` from one path.
+        if (!tool) throw new ToolNotFoundError(exec.name)
         const content = await tool.execute(exec.arguments, exec)
         return { callId: exec.callId, content, isError: false }
       } catch (error: unknown) {

+ 14 - 1
packages/tools/tests/tools.spec.ts

@@ -3,7 +3,7 @@ import { Context } from 'cordis'
 import { CallId } from '@deepseek-ai/dsh-llm'
 import SystemPrompt from '@deepseek-ai/dsh-system-prompt'
 import ToolRegistry, {
-  defineTool, schemaSpecToJsonSchema, validateArgs, ToolArgsError,
+  defineTool, schemaSpecToJsonSchema, validateArgs, ToolArgsError, ToolNotFoundError,
   type InferArgs, type SchemaSpec, type ToolExecutionResult,
 } from '@deepseek-ai/dsh-tools'
 
@@ -60,12 +60,25 @@ describe('ToolRegistry', () => {
 
     const unknown = await ctx.tools.execute({ callId: CallId('c1'), name: 'nope', arguments: {} })
     expect(unknown.isError).toBe(true)
+    expect(unknown.content[0]).toMatchObject({ text: 'Error: unknown tool "nope"' })
+    // An unknown tool is a routable failure class, same as a tool-thrown one.
+    expect(unknown.error).toEqual({ name: 'ToolNotFoundError', code: 'UNKNOWN_TOOL' })
 
     const thrown = await ctx.tools.execute({ callId: CallId('c2'), name: 'boom', arguments: {} })
     expect(thrown.isError).toBe(true)
     expect(thrown.content[0]).toMatchObject({ text: 'Error: exploded' })
   })
 
+  it('ToolNotFoundError carries the tool name and a stable code', async () => {
+    const { HarnessError } = await import('@deepseek-ai/dsh-llm')
+    const err = new ToolNotFoundError('ghost')
+    expect(err).toBeInstanceOf(HarnessError)
+    expect(err.name).toBe('ToolNotFoundError')
+    expect(err.code).toBe('UNKNOWN_TOOL')
+    expect(err.toolName).toBe('ghost')
+    expect(err.message).toBe('unknown tool "ghost"')
+  })
+
   it('lets tools/execute waterfall listeners veto a call (permission pattern)', async () => {
     const ctx = await setup()
     ctx.tools.register(echoTool)