diff --git a/docs/adr/0015-structured-error-taxonomy.md b/docs/adr/0015-structured-error-taxonomy.md new file mode 100644 index 0000000000..6eb3c860b7 --- /dev/null +++ b/docs/adr/0015-structured-error-taxonomy.md @@ -0,0 +1,24 @@ +# ADR 0015: Structured error taxonomy + +Status: accepted (2026-06-14) + +## Context + +Failures crossed seams as bare strings. A tool error flattened to a text block — name, code, and stack lost — so a future sandbox/retry plugin couldn't tell ENOENT from EACCES, and the model got less actionable feedback than it could. A non-Error throw degraded further: the loop wrapped it in `new Error(String(x))`, dropping any code. And `LlmError` was the only typed error in the system, with no shared base, so there was nothing for a consumer to `instanceof` against generically. + +This is the last of the RFC 005 pieces and the one the user was most skeptical of, so it was deliberately built **last and in isolation**: the earlier PRs (arg validation, dev invariants) threw plain `Error`s with a `code` field, decoupled from any shared base, so this change is a pure upgrade and is independently revertible without unpicking them. + +## Decision + +A single `HarnessError extends Error` base in `dsh-llm` (the leaf package every other imports — no new dependency edge): a stable `code` distinct from `message`, `cause` chaining via `ErrorOptions`, and `name` defaulting to the subclass. `isHarnessError` narrows at seams. + +- `LlmError`, `ToolArgsError` (dsh-tools), and `InvariantError` (dsh-invariants) now extend it, keeping their existing codes. +- `ToolExecutionResult` gains optional `error: { name, code }`, populated in the registry's catch when the thrown value is a `HarnessError`. The agent loop forwards it onto the `tool/result` session event (which gained the same optional field), so the structured failure survives into the log for retry/sandbox plugins and replay. The model-facing text block is unchanged. +- The loop's `toError` wraps a non-Error throw in a `HarnessError` (`code: 'UNKNOWN'`, original chained as `cause`) instead of a bare `Error`, so even a bad throw carries a routable code into the session `error` event (which already surfaced `code`). + +## Consequences + +- Errors are machine-routable end-to-end: a plugin can branch on `error.code` rather than substring-matching a message. +- One base class is imported widely, but it lives in the package everyone already depends on, so the cost is a single import, not a new edge. +- `deriveMessages` does not surface `error` into model history — the model still sees the text block; the structured field is for code and replay. +- Reverting this PR returns the earlier errors to plain `Error`+`code` form; nothing else in the stack depends on the shared base. diff --git a/docs/adr/README.md b/docs/adr/README.md index eef1bfe957..963e8f47bd 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -26,3 +26,4 @@ Do NOT write an ADR for: a mechanical or local choice (a variable name, a one-fi | [0012](0012-dev-invariants-over-deep-readonly.md) | Dev-mode invariants over compile-time deep-readonly | accepted | | [0013](0013-property-based-testing.md) | Property-based testing for protocol-shaped code | accepted | | [0014](0014-doc-sync-enforcement.md) | Doc-sync enforcement (doc code blocks + event taxonomy) | accepted | +| [0015](0015-structured-error-taxonomy.md) | Structured error taxonomy (HarnessError base) | accepted | diff --git a/docs/rfc/005-runtime-validation-and-error-taxonomy.md b/docs/rfc/005-runtime-validation-and-error-taxonomy.md index c3382abe22..47be3437bb 100644 --- a/docs/rfc/005-runtime-validation-and-error-taxonomy.md +++ b/docs/rfc/005-runtime-validation-and-error-taxonomy.md @@ -1,6 +1,6 @@ # RFC 005: Runtime validation at the model boundary, error taxonomy, dev-mode invariants -Status: partially implemented — part 1 (arg validation) → [ADR 0011](../adr/0011-runtime-arg-validation.md); part 3 (dev invariants) → [ADR 0012](../adr/0012-dev-invariants-over-deep-readonly.md); part 2 (error taxonomy) in progress +Status: implemented — part 1 (arg validation) → [ADR 0011](../adr/0011-runtime-arg-validation.md); part 3 (dev invariants) → [ADR 0012](../adr/0012-dev-invariants-over-deep-readonly.md); part 2 (error taxonomy) → [ADR 0015](../adr/0015-structured-error-taxonomy.md) ## Problem diff --git a/docs/rfc/README.md b/docs/rfc/README.md index c763e09024..64992d1e93 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -8,7 +8,7 @@ Proposals for substantial future work — reviewed before implementation, unlike | [002](002-mutation-testing.md) | Mutation testing as the coverage counterweight | proposed | | [003](003-deterministic-and-stress-testing.md) | Deterministic tests + replay invariant fixture + race stress | proposed | | [004](004-architectural-conformance.md) | Architectural rules: dependency-cruiser, adapter conformance kit | proposed | -| [005](005-runtime-validation-and-error-taxonomy.md) | Runtime arg validation, structured error taxonomy, dev-mode invariants | partially implemented | +| [005](005-runtime-validation-and-error-taxonomy.md) | Runtime arg validation, structured error taxonomy, dev-mode invariants | implemented | | [006](006-doc-sync-and-api-reports.md) | Doc-sync enforcement and API extractor reports | implemented (pts 1-2; pt 3 deferred) | | [007](007-supply-chain-and-vendor-drift.md) | Supply chain checks and vendor drift verification | proposed | | [008](008-immutable-public-surfaces.md) | Deep-readonly public surfaces | implemented (revised) | diff --git a/packages/agent-loop/src/loop.ts b/packages/agent-loop/src/loop.ts index 541740cfe7..799a1c164d 100644 --- a/packages/agent-loop/src/loop.ts +++ b/packages/agent-loop/src/loop.ts @@ -9,7 +9,7 @@ import type { Context } from 'cordis' import type { FinishReason, GenerateOptions, Message } from '@deepseek-ai/dsh-llm' -import { BlockAssembler } from '@deepseek-ai/dsh-llm' +import { BlockAssembler, HarnessError } from '@deepseek-ai/dsh-llm' import type { Session, TurnEndReason, TurnTrigger } from '@deepseek-ai/dsh-session' import { renderPrompt } from '@deepseek-ai/dsh-system-prompt' import type {} from '@deepseek-ai/dsh-tools' @@ -18,9 +18,15 @@ import type { LoopAgent } from './agent.ts' /** An Error with an optional machine-readable code (e.g., from LlmError or a throwing plugin). */ type CodedError = Error & { code?: string } -/** Normalize an arbitrary thrown value into a (possibly coded) Error. */ +/** + * Normalize an arbitrary thrown value into a coded Error. A real Error passes + * through (its `code`, if any, is preserved by {@link errorData}); a non-Error + * throw is wrapped in a {@link HarnessError} with code `UNKNOWN` and the + * original value chained as `cause`, so a bad throw still carries a routable + * code instead of degrading to a bare message. + */ function toError(error: unknown): CodedError { - return error instanceof Error ? error : new Error(String(error)) + return error instanceof Error ? error : new HarnessError(String(error), 'UNKNOWN', { cause: error }) } /** @@ -178,7 +184,7 @@ async function runTurn(ctx: Context, agent: LoopAgent, handle: LoopHandle, turn: try { stepOutcome = await runStep(ctx, agent, turn, step, abort.signal) } catch (error: unknown) { - stepOutcome = { error: error instanceof Error ? error : new Error(String(error)) } + stepOutcome = { error: toError(error) } } finally { handle.setAbort(undefined) } @@ -345,6 +351,7 @@ async function runStep( callId: result.callId, content: result.content, isError: result.isError, + ...result.error ? { error: result.error } : {}, }) // signal CAN flip during the await above (abort() inside a tool); // the analyzer can't see through the await boundary. diff --git a/packages/agent-loop/tests/coverage-edges.spec.ts b/packages/agent-loop/tests/coverage-edges.spec.ts index 4d1f03c01e..260c521de1 100644 --- a/packages/agent-loop/tests/coverage-edges.spec.ts +++ b/packages/agent-loop/tests/coverage-edges.spec.ts @@ -6,7 +6,7 @@ import SystemPrompt from '@deepseek-ai/dsh-system-prompt' import ToolRegistry, { defineTool } from '@deepseek-ai/dsh-tools' import AgentRegistry from '@deepseek-ai/dsh-agent' import AgentLoop, { LoopAgent } from '@deepseek-ai/dsh-agent-loop' -import { MockAdapter, textResponse } from './mock-adapter.ts' +import { MockAdapter, textResponse, toolCallResponse } from './mock-adapter.ts' async function harness(adapter: MockAdapter) { const ctx = new Context() @@ -177,6 +177,10 @@ describe('toError normalization', () => { await waitForIdle(ctx, agent) expect(errors).toHaveLength(1) expect(errors[0]!.message).toBe('naked string error') + // A non-Error throw is wrapped in a HarnessError with code UNKNOWN, so the + // session error event carries a routable code instead of degrading. + const errorEvent = agent.session.events.find(e => e.type === 'error') + expect(errorEvent?.type === 'error' && errorEvent.data.code).toBe('UNKNOWN') }) it('normalizes non-Error throws from agent/request waterfall via inline toError in runStep catch', async () => { @@ -201,6 +205,8 @@ describe('toError normalization', () => { expect(errors).toHaveLength(1) // String() of { code: 500 } is '[object Object]' expect(errors[0]!.message).toBe('[object Object]') + const errorEvent = agent.session.events.find(e => e.type === 'error') + expect(errorEvent?.type === 'error' && errorEvent.data.code).toBe('UNKNOWN') }) }) @@ -259,3 +265,33 @@ describe('disposed vs aborted branching', () => { expect(reasons).toContainEqual({ kind: 'disposed' }) }) }) + +describe('structured tool error propagation (RFC 005 pt 2)', () => { + it('forwards a tool HarnessError onto the tool/result session event', async () => { + const { HarnessError } = await import('@deepseek-ai/dsh-llm') + // First model turn calls the tool; second turn (after the tool result is + // fed back) ends with plain text so the loop settles. + const adapter = new MockAdapter([ + toolCallResponse('c1', 'boom', {}), + textResponse('done'), + ]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create('a1', { model: 'mock' }) + ctx.tools.register(defineTool({ + name: 'boom', + description: 'always fails', + parameters: {}, + async execute() { + throw new HarnessError('exploded', 'BOOM') + }, + })) + + send(agent, 'go') + await waitForIdle(ctx, agent) + + const toolResult = agent.session.events.find(e => e.type === 'tool/result') + expect(toolResult?.type === 'tool/result' && toolResult.data.isError).toBe(true) + expect(toolResult?.type === 'tool/result' && toolResult.data.error) + .toEqual({ name: 'HarnessError', code: 'BOOM' }) + }) +}) diff --git a/packages/invariants/package.json b/packages/invariants/package.json index 5e8bc76d91..646114b68c 100644 --- a/packages/invariants/package.json +++ b/packages/invariants/package.json @@ -21,6 +21,7 @@ "license": "BSD-3-Clause", "peerDependencies": { "@deepseek-ai/dsh-agent": "^0.0.1", + "@deepseek-ai/dsh-llm": "^0.0.1", "@deepseek-ai/dsh-session": "^0.0.1", "cordis": "^4.0.0-rc.6" }, diff --git a/packages/invariants/src/index.ts b/packages/invariants/src/index.ts index c5d9b30d00..fa72788ae4 100644 --- a/packages/invariants/src/index.ts +++ b/packages/invariants/src/index.ts @@ -20,6 +20,7 @@ */ import type { Context } from 'cordis' +import { HarnessError } from '@deepseek-ai/dsh-llm' import type { Agent, AgentStatus } from '@deepseek-ai/dsh-agent' import type { Session, SessionEvent } from '@deepseek-ai/dsh-session' @@ -27,13 +28,13 @@ export const name = 'invariants' export const inject = ['sessions'] /** - * Thrown when a harness event-contract invariant is violated. Plain `Error` - * with a `code` for now; a later change promotes the harness error taxonomy. + * Thrown when a harness event-contract invariant is violated. Extends + * {@link HarnessError} (`code: 'INVARIANT'`) so a violation is routable like + * any other harness failure. */ -export class InvariantError extends Error { - readonly code = 'INVARIANT' +export class InvariantError extends HarnessError { constructor(message: string) { - super(`invariant violated: ${message}`) + super(`invariant violated: ${message}`, 'INVARIANT') this.name = 'InvariantError' } } diff --git a/packages/llm/README.md b/packages/llm/README.md index 22a85bdf0d..9012c8425c 100644 --- a/packages/llm/README.md +++ b/packages/llm/README.md @@ -38,7 +38,8 @@ Streaming is a raw chunk protocol (`block-start`, `text-delta`, `reasoning-delta - `LlmAdapter` — abstract base class for provider adapters. The only required method is `stream()`. - `BlockAssembler` — incrementally assembles raw chunks into complete content blocks and an assistant message. Used by the agent loop (raw chunks for replay + assembled for history) and by `streamBlocks()`/`generate()`. -- `LlmError` — typed error with a `code` string (`NO_ADAPTER`, `DUPLICATE_ADAPTER`, and adapter codes like `AUTH`/`RATE_LIMIT`) and an optional numeric `status` when the failure came from a non-2xx provider response. +- `HarnessError` — base class for the harness error taxonomy: a stable `code` string (distinct from the human `message`) plus `cause` chaining. Lives here, in the leaf package every other imports, so a single base is shared without a new dependency edge. Per-package errors (`LlmError`, `ToolArgsError`, `InvariantError`, …) extend it. `isHarnessError(value)` narrows at seams. +- `LlmError` — extends `HarnessError`; `code` string (`NO_ADAPTER`, `DUPLICATE_ADAPTER`, and adapter codes like `AUTH`/`RATE_LIMIT`) plus an optional numeric `status` when the failure came from a non-2xx provider response. ### Real adapters diff --git a/packages/llm/src/error.ts b/packages/llm/src/error.ts new file mode 100644 index 0000000000..eca91c8d17 --- /dev/null +++ b/packages/llm/src/error.ts @@ -0,0 +1,33 @@ +/** + * The harness error taxonomy: one base class so failures carry a stable, + * machine-routable `code` and chain their `cause`, instead of flattening to a + * bare message string. Per-package errors extend {@link HarnessError}; the + * tool layer surfaces `{ name, code }` on results and the session `tool/result` + * event so retry/sandbox plugins and replay can distinguish failure classes. + * + * Lives in dsh-llm (the leaf package every other imports) so a single base is + * shared without a new dependency edge. See ADR 0015. + * + * @module @deepseek-ai/dsh-llm/error + */ + +/** + * Base class for all harness errors. Carries a `code` (stable, programmatic — + * e.g. `NO_ADAPTER`, `INVALID_ARGS`, `INVARIANT`) distinct from the + * human-readable `message`, and supports `cause` chaining via the standard + * `ErrorOptions`. `name` defaults to the subclass constructor name. + */ +export class HarnessError extends Error { + readonly code: string + + constructor(message: string, code: string, options?: ErrorOptions) { + super(message, options) + this.code = code + this.name = new.target.name + } +} + +/** Narrow an arbitrary thrown value to a HarnessError (for `instanceof` at seams). */ +export function isHarnessError(value: unknown): value is HarnessError { + return value instanceof HarnessError +} diff --git a/packages/llm/src/index.ts b/packages/llm/src/index.ts index 2e53caad1e..268289f465 100644 --- a/packages/llm/src/index.ts +++ b/packages/llm/src/index.ts @@ -9,9 +9,11 @@ import { Context, Service } from 'cordis' import type { ContentBlock, GenerateOptions, GenerateResult, StreamChunk } from './types.ts' import { BlockAssembler } from './assembler.ts' +import { HarnessError } from './error.ts' export * from './brand.ts' export * from './never.ts' +export * from './error.ts' export * from './types.ts' export { BlockAssembler } from './assembler.ts' @@ -31,14 +33,14 @@ declare module 'cordis' { } /** - * Typed error for LLM-related failures. The `code` string enables programmatic - * handling (e.g. `AUTH`, `RATE_LIMIT`, `NO_ADAPTER`); `status` carries the HTTP - * status when the error originated from a non-2xx provider response (absent for - * protocol/usage errors that have no HTTP status). + * Typed error for LLM-related failures. Extends {@link HarnessError}, so the + * `code` string (e.g. `AUTH`, `RATE_LIMIT`, `NO_ADAPTER`) is shared taxonomy; + * `status` carries the HTTP status when the error originated from a non-2xx + * provider response (absent for protocol/usage errors that have no HTTP status). */ -export class LlmError extends Error { - constructor(message: string, public code: string, public status?: number) { - super(message) +export class LlmError extends HarnessError { + constructor(message: string, code: string, public status?: number, options?: ErrorOptions) { + super(message, code, options) this.name = 'LlmError' } } diff --git a/packages/llm/tests/service.spec.ts b/packages/llm/tests/service.spec.ts index dfbe2d50d7..24bb7b359c 100644 --- a/packages/llm/tests/service.spec.ts +++ b/packages/llm/tests/service.spec.ts @@ -94,6 +94,28 @@ describe('LlmService', () => { expect(err.code).toBe('CUSTOM_CODE') }) + it('LlmError extends the shared HarnessError base', async () => { + const { HarnessError, isHarnessError } = await import('@deepseek-ai/dsh-llm') + const err = new LlmError('boom', 'AUTH', 401) + expect(err).toBeInstanceOf(HarnessError) + expect(isHarnessError(err)).toBe(true) + expect(err.code).toBe('AUTH') + expect(err.status).toBe(401) + }) + + it('HarnessError carries a code, names itself by subclass, and chains cause', async () => { + const { HarnessError, isHarnessError } = await import('@deepseek-ai/dsh-llm') + const root = new Error('root cause') + const err = new HarnessError('wrapper', 'UNKNOWN', { cause: root }) + expect(err).toBeInstanceOf(Error) + expect(err.name).toBe('HarnessError') + expect(err.code).toBe('UNKNOWN') + expect(err.cause).toBe(root) + expect(isHarnessError(err)).toBe(true) + expect(isHarnessError(root)).toBe(false) + expect(isHarnessError('nope')).toBe(false) + }) + it('disposes adapter registration on adapter-change event emission', async () => { const ctx = new Context() await ctx.plugin(LlmService) diff --git a/packages/session/src/types.ts b/packages/session/src/types.ts index 25628bc3a6..c187fd5cd4 100644 --- a/packages/session/src/types.ts +++ b/packages/session/src/types.ts @@ -62,7 +62,7 @@ export interface SessionEventMap { /** Assembled assistant message for one step (derived history uses this). */ 'assistant/message': { turn: number; step: number; content: ContentBlock[] } 'tool/call': { turn: number; step: number; callId: CallId; name: string; arguments: string } - 'tool/result': { turn: number; step: number; callId: CallId; content: ContentBlock[]; isError: boolean } + 'tool/result': { turn: number; step: number; callId: CallId; content: ContentBlock[]; isError: boolean; error?: { name: string; code: string } } /** Steering content injected between steps of a running turn. */ 'steering/message': { turn: number; content: ContentBlock[]; source: MessageSource } 'usage': { turn: number; step: number; usage: TokenUsage } diff --git a/packages/tools/README.md b/packages/tools/README.md index 5d1678512e..560195387d 100644 --- a/packages/tools/README.md +++ b/packages/tools/README.md @@ -26,7 +26,7 @@ Tool registry and execution waterfall. Tool plugins register their schemas and e - `ToolDefinition` — `ToolSchema` + `execute(args, exec): Promise`. - `ToolExecution` — one pending tool call: `{ callId, name, arguments, agent?, signal? }`. -- `ToolExecutionResult` — outcome: `{ callId, content, isError }`. +- `ToolExecutionResult` — outcome: `{ callId, content, isError, error? }`. On failure with a `HarnessError`, `error: { name, code }` carries the structured failure class alongside the model-facing text (the loop forwards it onto the `tool/result` session event for retry/sandbox plugins and replay). ### Extension points diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index 9ac5c51bad..45c40cdadc 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -9,6 +9,7 @@ import { Context, Service } from 'cordis' import type { CallId, ContentBlock, ToolSchema } from '@deepseek-ai/dsh-llm' +import { HarnessError } from '@deepseek-ai/dsh-llm' import type { Agent } from '@deepseek-ai/dsh-agent' import type {} from '@deepseek-ai/dsh-system-prompt' @@ -65,11 +66,36 @@ export interface ToolExecution { signal?: AbortSignal } +/** Structured error metadata for a failed tool call (alongside the model-facing text). */ +export interface ToolErrorInfo { + name: string + 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 content: ContentBlock[] isError: boolean + /** + * Set when the call failed with a {@link HarnessError}: machine-routable + * `{ name, code }` for retry/sandbox plugins and replay. The model-facing + * text in `content` is always present; this is extra structure for code. + */ + error?: ToolErrorInfo } /** @@ -87,6 +113,11 @@ function errorMessage(error: unknown): string { return String(error) } +/** Structured `{ name, code }` for a thrown HarnessError, else undefined. */ +function errorInfo(error: unknown): ToolErrorInfo | undefined { + return error instanceof HarnessError ? { name: error.name, code: error.code } : undefined +} + /** * Tool registry (`ctx.tools`): tool plugins register definitions; the agent * loop executes calls through the `tools/execute` waterfall. The registry @@ -142,28 +173,27 @@ 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 { return this.ctx.waterfall(this, 'tools/execute', exec, async (): Promise => { - 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) { + const info = errorInfo(error) return { callId: exec.callId, content: [{ type: 'text', text: `Error: ${errorMessage(error)}` }], isError: true, + ...info ? { error: info } : {}, } } }) diff --git a/packages/tools/src/schema.ts b/packages/tools/src/schema.ts index 577a200504..5572e43ff3 100644 --- a/packages/tools/src/schema.ts +++ b/packages/tools/src/schema.ts @@ -20,7 +20,7 @@ */ import type { ContentBlock } from '@deepseek-ai/dsh-llm' -import { assertNever } from '@deepseek-ai/dsh-llm' +import { assertNever, HarnessError } from '@deepseek-ai/dsh-llm' import type { ToolDefinition, ToolExecution } from './index.ts' // --------------------------------------------------------------------------- @@ -174,20 +174,17 @@ export function schemaSpecToJsonSchema(spec: SchemaSpec): JsonSchemaObject { /** * Thrown by a {@link defineTool} tool when the model-generated arguments don't - * match the declared {@link SchemaSpec}. The registry's execute waterfall - * catches it and returns an `isError` result so the model can self-correct. - * - * Plain `Error` for now (carries a `code` field); a later change promotes the - * harness error taxonomy and this extends a common base. + * match the declared {@link SchemaSpec}. Extends {@link HarnessError} + * (`code: 'INVALID_ARGS'`); the registry's execute waterfall catches it and + * returns an `isError` ToolExecutionResult carrying the structured error, so + * the model can self-correct and downstream plugins can route on the code. */ -export class ToolArgsError extends Error { - /** Machine-routable code; stable across the message wording. */ - readonly code = 'INVALID_ARGS' +export class ToolArgsError extends HarnessError { /** The individual violation messages, in declaration order. */ readonly violations: string[] constructor(violations: string[]) { - super(`invalid arguments: ${violations.join('; ')}`) + super(`invalid arguments: ${violations.join('; ')}`, 'INVALID_ARGS') this.name = 'ToolArgsError' this.violations = violations } diff --git a/packages/tools/tests/tools.spec.ts b/packages/tools/tests/tools.spec.ts index 4eaf222687..86e3d6c531 100644 --- a/packages/tools/tests/tools.spec.ts +++ b/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) @@ -717,6 +730,52 @@ describe('defineTool validation (RFC 005 part 1)', () => { expect(err.message).toBe('invalid arguments: missing required property "a"; "b" must be a number') }) + it('a schema-invalid call surfaces the structured error on the result', async () => { + const ctx = await setup() + ctx.tools.register(defineTool({ + name: 'reader', + description: 'reads a path', + parameters: { path: { type: 'string', required: true } }, + async execute(args) { + return [{ type: 'text', text: args.path }] + }, + })) + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'reader', arguments: {} }) + expect(result.isError).toBe(true) + expect(result.error).toEqual({ name: 'ToolArgsError', code: 'INVALID_ARGS' }) + }) + + it('a tool throwing a HarnessError surfaces its name and code', async () => { + const { HarnessError } = await import('@deepseek-ai/dsh-llm') + const ctx = await setup() + ctx.tools.register({ + ...echoTool, + name: 'coded', + async execute() { + throw new HarnessError('disk full', 'ENOSPC') + }, + }) + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'coded', arguments: {} }) + expect(result.isError).toBe(true) + expect(result.error).toEqual({ name: 'HarnessError', code: 'ENOSPC' }) + expect(result.content[0]).toMatchObject({ text: 'Error: disk full' }) + }) + + it('a non-HarnessError throw has no structured error (only the text)', async () => { + const ctx = await setup() + ctx.tools.register({ + ...echoTool, + name: 'plain', + async execute() { + throw new Error('just a message') + }, + }) + const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'plain', arguments: {} }) + expect(result.isError).toBe(true) + expect(result.error).toBeUndefined() + expect(result.content[0]).toMatchObject({ text: 'Error: just a message' }) + }) + it('raw-registered tools are NOT validated by defineTool (MCP keeps its own)', async () => { const ctx = await setup() // A raw ToolDefinition: no defineTool wrapping, so no validateArgs guard. diff --git a/yarn.lock b/yarn.lock index ee8d65477f..34aa3f9a3b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -621,6 +621,7 @@ __metadata: cordis: "npm:^4.0.0-rc.6" peerDependencies: "@deepseek-ai/dsh-agent": ^0.0.1 + "@deepseek-ai/dsh-llm": ^0.0.1 "@deepseek-ai/dsh-session": ^0.0.1 cordis: ^4.0.0-rc.6 languageName: unknown