refactor: prune core tool and prompt surface

This commit is contained in:
Tianyi Cui
2026-07-14 02:54:30 +08:00
parent a0359bc4a9
commit 0236a12324
21 changed files with 50 additions and 139 deletions

View File

@@ -994,7 +994,7 @@ export interface Config {
export type ToolPresentationMode = 'native' | 'code' | 'both' export type ToolPresentationMode = 'native' | 'code' | 'both'
``` ```
Source: [`packages/core/tools/src/index.ts:401`](../packages/core/tools/src/index.ts) Source: [`packages/core/tools/src/index.ts:400`](../packages/core/tools/src/index.ts)
## `@deepseek-ai/dsh-user-approval` ## `@deepseek-ai/dsh-user-approval`

View File

@@ -276,7 +276,7 @@ async execute(exec: ToolExecutionInput): Promise<ToolExecutionResult>
Types: [ToolDefinition](../core-data-structures/tools.md) · [ToolExecutionInput](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md) Types: [ToolDefinition](../core-data-structures/tools.md) · [ToolExecutionInput](../core-data-structures/tools.md) · [ToolExecutionResult](../core-data-structures/tools.md)
Source: [`packages/core/tools/src/index.ts:493`](../../packages/core/tools/src/index.ts) Source: [`packages/core/tools/src/index.ts:492`](../../packages/core/tools/src/index.ts)
## `ctx.userInteraction` — `UserInteractionService` ## `ctx.userInteraction` — `UserInteractionService`

View File

@@ -137,7 +137,6 @@ type ToolGuard = (execution: Readonly<ToolExecution>) => string | undefined
```ts type-equiv ```ts type-equiv
interface ToolExecutionResult { interface ToolExecutionResult {
callId: CallId
content: ContentBlock[] content: ContentBlock[]
isError: boolean isError: boolean
/** /**
@@ -167,6 +166,8 @@ interface ToolExecutionResult {
} }
``` ```
The result carries only the outcome. Call identity remains on the immutable `ToolExecution` that accompanies it through every hook and on the durable `tool/call` / `tool/result` session events, so wrappers cannot create a second, disagreeing identity.
The registry materializes and freezes the final accepted result immediately before `tools/result`. Its content, structured error, additional context, and presentation metadata must round-trip losslessly through JSON; an invalid outcome becomes a JSON-safe `isError` result, so the observed live outcome is safe for the later durable `tool/result` append. The registry materializes and freezes the final accepted result immediately before `tools/result`. Its content, structured error, additional context, and presentation metadata must round-trip losslessly through JSON; an invalid outcome becomes a JSON-safe `isError` result, so the observed live outcome is safe for the later durable `tool/result` append.
Each interception waterfall returns a typed **Decision** (the idiom shared with the `agent/*` seams). `tools/pre-execute` listeners receive `(exec, next)` and return a `PreToolDecision`; `tools/execute` wrappers return a `ToolExecutionResult`; `tools/post-execute` listeners receive `(exec, result, next)` and return a `PostToolDecision`: Each interception waterfall returns a typed **Decision** (the idiom shared with the `agent/*` seams). `tools/pre-execute` listeners receive `(exec, next)` and return a `PreToolDecision`; `tools/execute` wrappers return a `ToolExecutionResult`; `tools/post-execute` listeners receive `(exec, result, next)` and return a `PostToolDecision`:

View File

@@ -31,7 +31,7 @@ export type SurfaceOp =
### SurfaceManager: delta-based, not full rebuild ### SurfaceManager: delta-based, not full rebuild
A `SurfaceManager` class (private to `Session`) maintains the cached linked list. It tracks `_lastProcessedSeq` and processes only the **delta** (new events since the last access) rather than rescanning the entire log. Because the log is append-only, prior events never change — full rebuild is only needed after a wholesale log replacement (e.g., seeding). A `SurfaceManager` owned by `Session` maintains the cached linked list. It tracks `_lastProcessedSeq` and processes only the **delta** (new events since the last access) rather than rescanning the entire log. The seed is fixed before the manager is created and the log is append-only afterward, so prior events never change and no invalidation path is needed.
Delta processing is O(1) when no new events and O(new events) when new events arrive. Delta processing is O(1) when no new events and O(new events) when new events arrive.

View File

@@ -57,9 +57,8 @@ Signal replacement is by **in-place mutation of `exec.signal`**, not by passing
`timeout-policy` owns both uses of the `TOOL_TIMEOUT` code: the internal deadline code passed to `deadline()`/`timeoutOf()` (scoped so a nested outer deadline reads as an ordinary cancel) and the structured tool-result error code. Its replacement result is: `timeout-policy` owns both uses of the `TOOL_TIMEOUT` code: the internal deadline code passed to `deadline()`/`timeoutOf()` (scoped so a nested outer deadline reads as an ordinary cancel) and the structured tool-result error code. Its replacement result is:
```ts ignore-check ```ts ignore-check
function toolTimeoutResult(callId: CallId, timeoutMs: number): ToolExecutionResult { function toolTimeoutResult(timeoutMs: number): ToolExecutionResult {
return { return {
callId,
content: [{ type: 'text', text: `Error: tool call timed out after ${timeoutMs}ms` }], content: [{ type: 'text', text: `Error: tool call timed out after ${timeoutMs}ms` }],
isError: true, isError: true,
error: { name: 'ToolTimeoutError', code: 'TOOL_TIMEOUT' }, error: { name: 'ToolTimeoutError', code: 'TOOL_TIMEOUT' },

View File

@@ -26,7 +26,7 @@ Amend the session-surface and reconstructable-request RFCs where they describe t
## Acceptance criteria ## Acceptance criteria
- `SurfaceManager.nodes` is one ordered seq array with no `SurfaceNode`, link fields, or seq-to-node map; incremental append processing and the internal replace-generation signal remain, while the separate public `invalidate()` deletion stays owned by the dead-surface RFC. - `SurfaceManager.nodes` is one ordered seq array with no `SurfaceNode`, link fields, or seq-to-node map; incremental append processing and the internal replace-generation signal remain.
- Replaying full changed-header snapshots reconstructs exactly the same requests; no header-delta event/type/codec remains. - Replaying full changed-header snapshots reconstructs exactly the same requests; no header-delta event/type/codec remains.
- A v0 seed or persisted log containing legacy `request/header-delta` is rejected before replay, with coverage for JSONL and SQLite load paths. - A v0 seed or persisted log containing legacy `request/header-delta` is rejected before replay, with coverage for JSONL and SQLite load paths.
- New-shape v0 JSONL/SQLite replay, provenance, crash repair, compaction, snapshots, invariants, typecheck, coverage, doc-sync, build, and hygiene pass. - New-shape v0 JSONL/SQLite replay, provenance, crash repair, compaction, snapshots, invariants, typecheck, coverage, doc-sync, build, and hygiene pass.

View File

@@ -940,7 +940,7 @@ export const TYPE_API: readonly TypeApiEntry[] = [
}, },
{ {
name: 'ToolExecutionResult', name: 'ToolExecutionResult',
declaration: 'export interface ToolExecutionResult {\n callId: CallId;\n content: ContentBlock[];\n isError: boolean;\n error?: ToolErrorInfo;\n additionalContext?: HookContext;\n meta?: unknown;\n}', declaration: 'export interface ToolExecutionResult {\n content: ContentBlock[];\n isError: boolean;\n error?: ToolErrorInfo;\n additionalContext?: HookContext;\n meta?: unknown;\n}',
}, },
{ {
name: 'ToolExecutionToken', name: 'ToolExecutionToken',

View File

@@ -899,12 +899,8 @@ async function runStep(
}) })
session.append('tool/result', { session.append('tool/result', {
turn, step, turn, step,
// The correlation id MUST be the loop's authoritative call.id (the // Correlation comes from the immutable execution input; the result does
// model-transcript id that deriveMessages turns into toolCallId), NOT // not duplicate this authoritative transcript identity.
// result.callId — a post-execute waterfall listener returning a
// mismatched id would otherwise orphan the call↔result pairing in the
// next model request. A listener-internal id, if ever needed, belongs in
// a separate diagnostic field, never overloaded onto callId.
callId: call.id, callId: call.id,
content: result.content, content: result.content,
isError: result.isError, isError: result.isError,

View File

@@ -1078,8 +1078,7 @@ describe('tool result call identity', () => {
// A post-execute listener transforms the result (accept-with-replacement). // A post-execute listener transforms the result (accept-with-replacement).
// The loop must still record the tool/result under the model's authoritative // The loop must still record the tool/result under the model's authoritative
// call.id (the loop ignores result.callId — which the registry always sets to // call.id, which is the immutable identity carried by the execution input.
// exec.callId anyway — and uses call.id, the model-transcript id).
ctx.on('tools/post-execute', (exec, _result) => { ctx.on('tools/post-execute', (exec, _result) => {
expect(exec.callId).toBe(CallId('c1')) // the loop passed the real id in expect(exec.callId).toBe(CallId('c1')) // the loop passed the real id in
return Promise.resolve({ kind: 'accept', content: [{ type: 'text', text: 'ok' }] }) return Promise.resolve({ kind: 'accept', content: [{ type: 'text', text: 'ok' }] })
@@ -1089,8 +1088,7 @@ describe('tool result call identity', () => {
send(agent, 'use tool') send(agent, 'use tool')
await waitForIdle(ctx, agent) await waitForIdle(ctx, agent)
// The logged tool/result.callId is the originating call.id, NOT the // The logged tool/result.callId is the originating call.id.
// listener's wrong id.
const resultEvent = [...agent.session.events].find(e => e.type === 'tool/result') const resultEvent = [...agent.session.events].find(e => e.type === 'tool/result')
expect(resultEvent?.type).toBe('tool/result') expect(resultEvent?.type).toBe('tool/result')
if (resultEvent?.type === 'tool/result') { if (resultEvent?.type === 'tool/result') {

View File

@@ -35,7 +35,7 @@ Plain class (not a Cordis Service). Create via `ctx.sessions.create()`.
- `session.append(type, data, opts?): SessionEvent` — synchronous, never blocks on I/O. At this durable boundary, data and surface metadata are lossless-JSON snapshotted and deep-frozen. For an attached session, a reentrant append during dispatch/observer publication rejects, and detach waits for that publication to unwind. Callbacks resolve before the log push; the push is the commit point, after which each observer failure is contained independently. Runtime surface validation covers widened unions and raw seed/load logs. - `session.append(type, data, opts?): SessionEvent` — synchronous, never blocks on I/O. At this durable boundary, data and surface metadata are lossless-JSON snapshotted and deep-frozen. For an attached session, a reentrant append during dispatch/observer publication rejects, and detach waits for that publication to unwind. Callbacks resolve before the log push; the push is the commit point, after which each observer failure is contained independently. Runtime surface validation covers widened unions and raw seed/load logs.
- `session.deriveMessages(): Message[]` — the LLM message history, CACHED: each surface node is projected exactly once, when first seen (O(new nodes) per call; a surface rewrite rebuilds via `surface.replaceGeneration`). Returns a fresh array per call over shared, deep-frozen `Message` objects. Each projection reuses the already deep-frozen content in its durable log event, so no second deep clone is needed and a consumer still cannot mutate logged data. The surface is the single source of derived history — there is no raw-log fallback. - `session.deriveMessages(): Message[]` — the LLM message history, CACHED: each surface node is projected exactly once, when first seen (O(new nodes) per call; a surface rewrite rebuilds via `surface.replaceGeneration`). Returns a fresh array per call over shared, deep-frozen `Message` objects. Each projection reuses the already deep-frozen content in its durable log event, so no second deep clone is needed and a consumer still cannot mutate logged data. The surface is the single source of derived history — there is no raw-log fallback.
- `session.deriveEventMessage(event): Message | null` — the per-event projection `deriveMessages()` folds: a fresh message wrapper that reuses the event's already frozen content, or `null` when the event produces none (a non-surface event, or an empty-content `assistant/message` hosting only usage). External reconstructors and the dev invariant fold the same function over a log prefix's surface, so no two paths can disagree about what a request's messages were (the reconstructability RFC). - `session.deriveEventMessage(event): Message | null` — the per-event projection `deriveMessages()` folds: a fresh message wrapper that reuses the event's already frozen content, or `null` when the event produces none (a non-surface event, or an empty-content `assistant/message` hosting only usage). External reconstructors and the dev invariant fold the same function over a log prefix's surface, so no two paths can disagree about what a request's messages were (the reconstructability RFC).
- `session.surface: SurfaceManager` — the derived surface, lazily rebuilt from `surfaceOp` markers in the log. Processes only new events (delta) on each access — the log is append-only, so prior events never change. `surface.replaceGeneration` is the rewrite signal: bumped by every folded `replace` and by `invalidate()`, never reset, so an incremental consumer comparing generations cannot be fooled. - `session.surface: SurfaceManager` — the derived surface, lazily built from `surfaceOp` markers in the log. Processes only new events (delta) on each access — the log is append-only, so prior events never change. `surface.replaceGeneration` is the rewrite signal, bumped by every folded `replace`, so an incremental consumer knows when to rebuild.
- `session.events` — a cached, frozen array snapshot over deep-frozen events. Repeated reads without an append return the same array; an append invalidates the cache and the next read returns a new snapshot, while earlier snapshots stay unchanged. Neither a cast nor a retained reference can push into the live log or rewrite an accepted event. - `session.events` — a cached, frozen array snapshot over deep-frozen events. Repeated reads without an append return the same array; an append invalidates the cache and the next read returns a new snapshot, while earlier snapshots stay unchanged. Neither a cast nor a retained reference can push into the live log or rewrite an accepted event.
- `session.seq`, `session.id` — current sequence and readonly typed identity. - `session.seq`, `session.id` — current sequence and readonly typed identity.
- `session.header: SessionHeader` — detached, deep-frozen creation metadata (`version`, `id`, `createdAt`, optional `cwd`/`parentSession`/`seedLength`). Construction validates the durable record and requires its id to match `session.id`. - `session.header: SessionHeader` — detached, deep-frozen creation metadata (`version`, `id`, `createdAt`, optional `cwd`/`parentSession`/`seedLength`). Construction validates the durable record and requires its id to match `session.id`.

View File

@@ -73,36 +73,21 @@ export class SurfaceManager {
private _nodes: SurfaceNode[] = [] private _nodes: SurfaceNode[] = []
/** Map from event seq → node. */ /** Map from event seq → node. */
private _nodeBySeq = new Map<number, SurfaceNode>() private _nodeBySeq = new Map<number, SurfaceNode>()
/** The last processed seq. -1 forces a full rebuild on first access. */ /** The last processed seq. -1 marks the initial lazy build. */
private _lastProcessedSeq = -1 private _lastProcessedSeq = -1
/** Rewrite generation — see {@link replaceGeneration}. */ /** Replacement generation — see {@link replaceGeneration}. */
private _replaceGeneration = 0 private _replaceGeneration = 0
constructor(private log: readonly SessionEvent[]) {} constructor(private log: readonly SessionEvent[]) {}
/** /**
* Reset to unprocessed state. Call after the log has been replaced * The surface's replacement generation, bumped by every folded `replace` op.
* wholesale (e.g. after Session seed). Not needed for normal appends — * A replace is the ONE operation that rewrites the surface non-monotonically,
* those are picked up incrementally. * so an incremental consumer of {@link nodes} (the session's derived-message
*/ * cache) compares this between visits — an unchanged generation guarantees
invalidate(): void { * every node it has not seen is a pure tail append; a changed one means its
this._lastProcessedSeq = -1 * view must rebuild.
this._nodes = []
this._nodeBySeq.clear()
// A wholesale rebuild is a rewrite: bump the generation so incremental
// consumers (the session's derived-message cache) discard their view.
this._replaceGeneration += 1
}
/**
* The surface's rewrite generation: bumped by every folded `replace` op and
* by {@link invalidate}. A replace is the ONE operation that rewrites the
* surface non-monotonically, so an incremental consumer of {@link nodes}
* (the session's derived-message cache) compares this between visits — an
* unchanged generation guarantees every node it has not seen is a pure tail
* append; a changed one means its view must rebuild. Monotonic: it never
* moves backwards, so comparisons cannot be fooled by a re-fold.
*/ */
get replaceGeneration(): number { get replaceGeneration(): number {
if (this._lastProcessedSeq < this.log.length - 1) this._processDelta() if (this._lastProcessedSeq < this.log.length - 1) this._processDelta()

View File

@@ -1,7 +1,7 @@
/** /**
* Derived-message cache tests: the session projects each surface node exactly * Derived-message cache tests: the session projects each surface node exactly
* once (O(new nodes) per call), rebuilds on a surface rewrite (replace / * once (O(new nodes) per call), rebuilds on a surface replacement (the
* invalidate — the replaceGeneration signal), returns a fresh array snapshot * replaceGeneration signal), returns a fresh array snapshot
* per call over shared frozen messages, and stays deep-equal to a from-scratch * per call over shared frozen messages, and stays deep-equal to a from-scratch
* replay derivation at every step — the incremental==scratch property the * replay derivation at every step — the incremental==scratch property the
* reconstructability RFC's invariant enforces in dev at request time. * reconstructability RFC's invariant enforces in dev at request time.
@@ -66,17 +66,6 @@ describe('derived-message cache', () => {
expect(Object.isFrozen(first[0])).toBe(true) expect(Object.isFrozen(first[0])).toBe(true)
}) })
it('rebuilds after surface.invalidate() (the generation covers wholesale rebuilds too)', () => {
const session = new Session(SessionId('cache-invalidate'))
session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } })
userText(session, 'one')
const before = session.deriveMessages()
session.surface.invalidate()
const after = session.deriveMessages()
expect(after).toEqual(before)
// A rebuild re-projects: fresh objects, same values.
expect(after[0]).not.toBe(before[0])
})
}) })
describe('Session.deriveEventMessage — the per-event projection', () => { describe('Session.deriveEventMessage — the per-event projection', () => {

View File

@@ -28,14 +28,6 @@ describe('SurfaceManager', () => {
expect(nodes[1]!.next).toBeNull() expect(nodes[1]!.next).toBeNull()
}) })
it('invalidate resets to full rebuild', () => {
const s = surfaceSession()
expect(s.surface.nodes.length).toBe(2)
// After invalidate, the surface should rebuild from scratch on next access.
;(s.surface).invalidate()
expect(s.surface.nodes.length).toBe(2) // same result, but rebuilt
})
it('empty surface yields empty nodes', () => { it('empty surface yields empty nodes', () => {
const s = new Session(SessionId('empty')) const s = new Session(SessionId('empty'))
// Only turn boundaries, no surface nodes. // Only turn boundaries, no surface nodes.
@@ -336,7 +328,7 @@ describe('surface type guards', () => {
}) })
describe('SurfaceManager.replaceGeneration', () => { describe('SurfaceManager.replaceGeneration', () => {
it('folds the pending log delta on access and counts replaces and invalidations', () => { it('folds the pending log delta on access and counts replacements', () => {
const s = new Session(SessionId('gen')) const s = new Session(SessionId('gen'))
s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) s.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } })
s.append('user/message', { content: [{ type: 'text', text: 'one' }], source: { kind: 'user' } }, { surfaceOp: 'append' }) s.append('user/message', { content: [{ type: 'text', text: 'one' }], source: { kind: 'user' } }, { surfaceOp: 'append' })
@@ -350,10 +342,5 @@ describe('SurfaceManager.replaceGeneration', () => {
content: [{ type: 'text', text: 'summary' }], source: { kind: 'plugin', plugin: 'compact' }, content: [{ type: 'text', text: 'summary' }], source: { kind: 'plugin', plugin: 'compact' },
}, { surfaceOp: { op: 'replace', start: nodes[0]!.seq, end: nodes[1]!.seq }, sourceEventSeqs: [nodes[0]!.seq, nodes[1]!.seq] }) }, { surfaceOp: { op: 'replace', start: nodes[0]!.seq, end: nodes[1]!.seq }, sourceEventSeqs: [nodes[0]!.seq, nodes[1]!.seq] })
expect(s.surface.replaceGeneration).toBe(1) expect(s.surface.replaceGeneration).toBe(1)
// invalidate() is a rewrite too: the generation moves forward (and the
// refold re-counts the replace), never backwards.
s.surface.invalidate()
expect(s.surface.replaceGeneration).toBeGreaterThan(1)
}) })
}) })

View File

@@ -360,7 +360,7 @@ export class SystemPrompt extends Service {
private scopedVariableProviders = new Map<ScopeKey, Map<string, (context: AssembleContext) => string | undefined>>() private scopedVariableProviders = new Map<ScopeKey, Map<string, (context: AssembleContext) => string | undefined>>()
private readonly toolOrder: string[] | undefined private readonly toolOrder: string[] | undefined
constructor(ctx: Context, public config: Config) { constructor(ctx: Context, config: Config) {
super(ctx, 'systemPrompt') super(ctx, 'systemPrompt')
this.toolOrder = validateToolOrder(config.toolOrder) this.toolOrder = validateToolOrder(config.toolOrder)
// The harness-owned openers. They live HERE (not on the loop plugin) so a // The harness-owned openers. They live HERE (not on the loop plugin) so a

View File

@@ -36,7 +36,7 @@ The live registry pipeline has three transformable waterfalls followed by the ob
- `ToolExecutionInput` — the caller-supplied call description: `{ callId, name, arguments, agent?, parent?, signal? }`; callers may pass an enclosing execution's opaque token as `parent` but never choose the new execution's own token. - `ToolExecutionInput` — the caller-supplied call description: `{ callId, name, arguments, agent?, parent?, signal? }`; callers may pass an enclosing execution's opaque token as `parent` but never choose the new execution's own token.
- `ToolExecutionToken` — a fresh branded `Symbol` assigned by the registry. It supports equality correlation only and never crosses a model, log, or worker boundary. - `ToolExecutionToken` — a fresh branded `Symbol` assigned by the registry. It supports equality correlation only and never crosses a model, log, or worker boundary.
- `ToolExecution` — the pipeline-owned call: immutable `{ token, callId, name, arguments, agent?, parent? }` identity plus optional operational `signal`, which an around wrapper may add, replace, remove, and restore. A nested call's `parent` is a `ToolExecutionToken`, not an execution object. - `ToolExecution` — the pipeline-owned call: immutable `{ token, callId, name, arguments, agent?, parent? }` identity plus optional operational `signal`, which an around wrapper may add, replace, remove, and restore. A nested call's `parent` is a `ToolExecutionToken`, not an execution object.
- `ToolExecutionResult` — losslessly JSON-serializable outcome: `{ callId, content, isError, error?, additionalContext?, meta? }`. The registry materializes and freezes the complete post-policy value before final observation. On failure with a `HarnessError`, `error: { name, code }` carries the structured failure class alongside the model-facing text. - `ToolExecutionResult` — losslessly JSON-serializable outcome: `{ content, isError, error?, additionalContext?, meta? }`. Call identity stays on the immutable `ToolExecution` supplied alongside the result instead of being duplicated on the outcome. The registry materializes and freezes the complete post-policy value before final observation. On failure with a `HarnessError`, `error: { name, code }` carries the structured failure class alongside the model-facing text.
- `PreToolDecision` — `{kind:'allow'}` | `{kind:'deny', reason}` | `{kind:'ask', reason?}`. Input rewrite is deliberately not offered; `ask` is serviced by [`ctx.approval`](../../ui/user-approval/README.md) when mounted and otherwise degrades to deny. - `PreToolDecision` — `{kind:'allow'}` | `{kind:'deny', reason}` | `{kind:'ask', reason?}`. Input rewrite is deliberately not offered; `ask` is serviced by [`ctx.approval`](../../ui/user-approval/README.md) when mounted and otherwise degrades to deny.
- `PostToolDecision` — `{kind:'accept', content?, additionalContext?}` (keep the call successful, optionally replacing the model-facing content) | `{kind:'block', feedback, additionalContext?}` (turn it into an `isError` whose content is the corrective feedback). Output replacement is clean because `tool/result` is logged AFTER `execute()` returns. - `PostToolDecision` — `{kind:'accept', content?, additionalContext?}` (keep the call successful, optionally replacing the model-facing content) | `{kind:'block', feedback, additionalContext?}` (turn it into an `isError` whose content is the corrective feedback). Output replacement is clean because `tool/result` is logged AFTER `execute()` returns.
- `ToolGuard` — `(execution) => string | undefined`; the returned string is a final monotonic denial reason evaluated after the reorderable pre-execute waterfall and before dispatch. - `ToolGuard` — `(execution) => string | undefined`; the returned string is a final monotonic denial reason evaluated after the reorderable pre-execute waterfall and before dispatch.

View File

@@ -290,7 +290,7 @@ export interface ToolErrorInfo {
* distinguish it from a tool body's own error. * distinguish it from a tool body's own error.
*/ */
export class ToolNotFoundError extends HarnessError { export class ToolNotFoundError extends HarnessError {
constructor(public readonly toolName: string) { constructor(toolName: string) {
super(`unknown tool "${toolName}"`, 'UNKNOWN_TOOL') super(`unknown tool "${toolName}"`, 'UNKNOWN_TOOL')
this.name = 'ToolNotFoundError' this.name = 'ToolNotFoundError'
} }
@@ -298,7 +298,6 @@ export class ToolNotFoundError extends HarnessError {
/** The outcome of one tool call. */ /** The outcome of one tool call. */
export interface ToolExecutionResult { export interface ToolExecutionResult {
callId: CallId
content: ContentBlock[] content: ContentBlock[]
isError: boolean isError: boolean
/** /**
@@ -918,7 +917,7 @@ export class ToolRegistry extends Service {
} }
} catch (error: unknown) { } catch (error: unknown) {
execution = { ...base, arguments: undefined } execution = { ...base, arguments: undefined }
const result = this.materializeFinalResult(toolErrorResult(callId, error)) const result = this.materializeFinalResult(toolErrorResult(error))
this.notifyResult(execution, result) this.notifyResult(execution, result)
return result return result
} }
@@ -928,7 +927,7 @@ export class ToolRegistry extends Service {
} catch (error: unknown) { } catch (error: unknown) {
// Outer backstop: a throwing pre/post-execute listener, guard, or the // Outer backstop: a throwing pre/post-execute listener, guard, or the
// waterfall machinery becomes an isError result, never a turn failure. // waterfall machinery becomes an isError result, never a turn failure.
result = this.materializeFinalResult(toolErrorResult(execution.callId, error)) result = this.materializeFinalResult(toolErrorResult(error))
} }
this.notifyResult(execution, result) this.notifyResult(execution, result)
return result return result
@@ -953,7 +952,6 @@ export class ToolRegistry extends Service {
// Every non-grant, including a failed/unavailable approval request, takes // Every non-grant, including a failed/unavailable approval request, takes
// the same deny path and still reaches post-policy plus result observers. // the same deny path and still reaches post-policy plus result observers.
const denied: ToolExecutionResult = { const denied: ToolExecutionResult = {
callId: exec.callId,
content: [{ type: 'text', text: `Error: ${denialReason}` }], content: [{ type: 'text', text: `Error: ${denialReason}` }],
isError: true, isError: true,
} }
@@ -984,16 +982,12 @@ export class ToolRegistry extends Service {
const returned = await tool.execute(exec.arguments, exec) const returned = await tool.execute(exec.arguments, exec)
const content = Array.isArray(returned) ? returned : returned.content const content = Array.isArray(returned) ? returned : returned.content
const meta = Array.isArray(returned) ? undefined : returned.meta const meta = Array.isArray(returned) ? undefined : returned.meta
return { callId: exec.callId, content, isError: false, ...meta !== undefined ? { meta } : {} } return { content, isError: false, ...meta !== undefined ? { meta } : {} }
} catch (error: unknown) { } catch (error: unknown) {
return toolErrorResult(exec.callId, error) return toolErrorResult(error)
} }
}, },
) )
if (result.callId !== exec.callId) {
throw new TypeError(`tools/execute returned callId "${String(result.callId)}" for authoritative call "${exec.callId}"`)
}
return await this.postExecute(exec, result) return await this.postExecute(exec, result)
} }
@@ -1068,7 +1062,6 @@ export class ToolRegistry extends Service {
const additionalContext = decision.additionalContext const additionalContext = decision.additionalContext
if (decision.kind === 'block') { if (decision.kind === 'block') {
return { return {
callId: result.callId,
content: decision.feedback, content: decision.feedback,
isError: true, isError: true,
...additionalContext ? { additionalContext } : {}, ...additionalContext ? { additionalContext } : {},
@@ -1097,10 +1090,9 @@ function createExecutionToken(): ToolExecutionToken {
return Symbol('dsh.tool.execution') as ToolExecutionToken return Symbol('dsh.tool.execution') as ToolExecutionToken
} }
function toolErrorResult(callId: ToolExecution['callId'], error: unknown): ToolExecutionResult { function toolErrorResult(error: unknown): ToolExecutionResult {
const info = errorInfo(error) const info = errorInfo(error)
return { return {
callId,
content: [{ type: 'text', text: `Error: ${errorMessage(error)}` }], content: [{ type: 'text', text: `Error: ${errorMessage(error)}` }],
isError: true, isError: true,
...info ? { error: info } : {}, ...info ? { error: info } : {},

View File

@@ -548,7 +548,6 @@ describe('scoped execution dispatch', () => {
expect(reads).toBe(1) expect(reads).toBe(1)
expect(result).toEqual({ expect(result).toEqual({
callId: CallId('unstable-arguments'),
content: [{ type: 'text', text: 'ran:t' }], content: [{ type: 'text', text: 'ran:t' }],
isError: false, isError: false,
}) })
@@ -564,10 +563,9 @@ describe('scoped execution dispatch', () => {
ctx.on('internal/dispatch', (mode, name) => { ctx.on('internal/dispatch', (mode, name) => {
if (name === 'tools/result') dispatchModes.push(mode) if (name === 'tools/result') dispatchModes.push(mode)
}) })
ctx.on('tools/execute', async (exec, next) => { ctx.on('tools/execute', async (_exec, next) => {
await next() await next()
return { return {
callId: exec.callId,
content: [{ type: 'text', text: 'outer failure' }], content: [{ type: 'text', text: 'outer failure' }],
isError: true, isError: true,
} }

View File

@@ -80,7 +80,7 @@ describe('ToolRegistry', () => {
const ctx = await setup() const ctx = await setup()
ctx.tools.register(echoTool) ctx.tools.register(echoTool)
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } })
expect(result).toEqual({ callId: CallId('c1'), content: [{ type: 'text', text: 'hi' }], isError: false }) expect(result).toEqual({ content: [{ type: 'text', text: 'hi' }], isError: false })
}) })
it('threads a tool-attached meta (object return form) onto the result', async () => { it('threads a tool-attached meta (object return form) onto the result', async () => {
@@ -94,7 +94,6 @@ describe('ToolRegistry', () => {
}) })
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'meta-tool', arguments: {} }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'meta-tool', arguments: {} })
expect(result).toEqual({ expect(result).toEqual({
callId: CallId('c1'),
content: [{ type: 'text', text: 'ok' }], content: [{ type: 'text', text: 'ok' }],
isError: false, isError: false,
meta: { diffs: [{ path: 'a', oldText: null, newText: 'x' }] }, meta: { diffs: [{ path: 'a', oldText: null, newText: 'x' }] },
@@ -111,7 +110,7 @@ describe('ToolRegistry', () => {
}, },
}) })
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'no-meta-tool', arguments: {} }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'no-meta-tool', arguments: {} })
expect(result).toEqual({ callId: CallId('c1'), content: [{ type: 'text', text: 'ok' }], isError: false }) expect(result).toEqual({ content: [{ type: 'text', text: 'ok' }], isError: false })
expect('meta' in result).toBe(false) expect('meta' in result).toBe(false)
}) })
@@ -178,13 +177,12 @@ describe('ToolRegistry', () => {
}) })
}) })
it('ToolNotFoundError carries the tool name and a stable code', async () => { it('ToolNotFoundError carries a stable message and code', async () => {
const { HarnessError } = await import('@deepseek-ai/dsh-llm') const { HarnessError } = await import('@deepseek-ai/dsh-llm')
const err = new ToolNotFoundError('ghost') const err = new ToolNotFoundError('ghost')
expect(err).toBeInstanceOf(HarnessError) expect(err).toBeInstanceOf(HarnessError)
expect(err.name).toBe('ToolNotFoundError') expect(err.name).toBe('ToolNotFoundError')
expect(err.code).toBe('UNKNOWN_TOOL') expect(err.code).toBe('UNKNOWN_TOOL')
expect(err.toolName).toBe('ghost')
expect(err.message).toBe('unknown tool "ghost"') expect(err.message).toBe('unknown tool "ghost"')
}) })
@@ -425,7 +423,7 @@ describe('ToolRegistry', () => {
ctx.on('tools/post-execute', async (_exec, _result, next) => { order.push('post'); return next() }) ctx.on('tools/post-execute', async (_exec, _result, next) => { order.push('post'); return next() })
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'traced', arguments: { text: 'hi' } }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'traced', arguments: { text: 'hi' } })
expect(result).toEqual({ callId: CallId('c1'), content: [{ type: 'text', text: 'hi' }], isError: false }) expect(result).toEqual({ content: [{ type: 'text', text: 'hi' }], isError: false })
// The around seam wraps dispatch; pre gates before it, post runs over its result. // The around seam wraps dispatch; pre gates before it, post runs over its result.
expect(order).toEqual(['pre', 'execute:before', 'dispatch', 'execute:after', 'post']) expect(order).toEqual(['pre', 'execute:before', 'dispatch', 'execute:after', 'post'])
}) })
@@ -526,8 +524,8 @@ describe('ToolRegistry', () => {
async execute() { dispatched = true; return [] }, async execute() { dispatched = true; return [] },
}) })
ctx.on('tools/execute', async (exec: ToolExecution, _next: () => Promise<ToolExecutionResult>): Promise<ToolExecutionResult> => ctx.on('tools/execute', async (_exec: ToolExecution, _next: () => Promise<ToolExecutionResult>): Promise<ToolExecutionResult> =>
({ callId: exec.callId, content: [{ type: 'text', text: 'short-circuited' }], isError: false })) ({ content: [{ type: 'text', text: 'short-circuited' }], isError: false }))
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'never-runs', arguments: {} }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'never-runs', arguments: {} })
expect(dispatched).toBe(false) // returning without next() skips core dispatch expect(dispatched).toBe(false) // returning without next() skips core dispatch
@@ -537,8 +535,7 @@ describe('ToolRegistry', () => {
it('preserves additionalContext supplied by an around-dispatch result', async () => { it('preserves additionalContext supplied by an around-dispatch result', async () => {
const ctx = await setup() const ctx = await setup()
ctx.tools.register(echoTool) ctx.tools.register(echoTool)
ctx.on('tools/execute', async exec => ({ ctx.on('tools/execute', async () => ({
callId: exec.callId,
content: [{ type: 'text', text: 'short-circuited with context' }], content: [{ type: 'text', text: 'short-circuited with context' }],
isError: false, isError: false,
additionalContext: { additionalContext: {
@@ -556,20 +553,6 @@ describe('ToolRegistry', () => {
}) })
}) })
it('normalizes a tools/execute result with the wrong call id', async () => {
const ctx = await setup()
ctx.tools.register(echoTool)
ctx.on('tools/execute', async () => ({ callId: CallId('other'), content: [], isError: false }))
const result = await ctx.tools.execute({
callId: CallId('malformed-shape'), name: 'echo', arguments: {},
})
expect(result.isError).toBe(true)
expect(result.content[0]).toMatchObject({
text: 'Error: tools/execute returned callId "other" for authoritative call "malformed-shape"',
})
})
it('returns an isError result when a tools/execute listener throws', async () => { it('returns an isError result when a tools/execute listener throws', async () => {
const ctx = await setup() const ctx = await setup()
ctx.tools.register(echoTool) ctx.tools.register(echoTool)
@@ -577,7 +560,6 @@ describe('ToolRegistry', () => {
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } })
expect(result).toEqual({ expect(result).toEqual({
callId: CallId('c1'),
content: [{ type: 'text', text: 'Error: wrapper broke' }], content: [{ type: 'text', text: 'Error: wrapper broke' }],
isError: true, isError: true,
}) })
@@ -593,7 +575,6 @@ describe('ToolRegistry', () => {
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } })
expect(result).toEqual({ expect(result).toEqual({
callId: CallId('c1'),
content: [{ type: 'text', text: 'Error: permission hook broke' }], content: [{ type: 'text', text: 'Error: permission hook broke' }],
isError: true, isError: true,
}) })
@@ -609,7 +590,6 @@ describe('ToolRegistry', () => {
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } })
expect(result).toEqual({ expect(result).toEqual({
callId: CallId('c1'),
content: [{ type: 'text', text: 'Error: post hook broke' }], content: [{ type: 'text', text: 'Error: post hook broke' }],
isError: true, isError: true,
}) })
@@ -625,7 +605,6 @@ describe('ToolRegistry', () => {
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } })
expect(result).toMatchObject({ expect(result).toMatchObject({
callId: CallId('c1'),
isError: true, isError: true,
error: { name: 'HarnessError', code: 'DENIED' }, error: { name: 'HarnessError', code: 'DENIED' },
}) })
@@ -1263,7 +1242,7 @@ describe('defineTool validation (the runtime-validation RFC, part 1)', () => {
}, },
})) }))
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'reader', arguments: { path: '/x' } }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'reader', arguments: { path: '/x' } })
expect(result).toEqual({ callId: CallId('c1'), content: [{ type: 'text', text: 'read /x' }], isError: false }) expect(result).toEqual({ content: [{ type: 'text', text: 'read /x' }], isError: false })
}) })
it('ToolArgsError carries a stable code and the violation list', () => { it('ToolArgsError carries a stable code and the violation list', () => {

View File

@@ -869,9 +869,9 @@ describe('scoped-dispatch invariants', () => {
['agent/error', [agent, 1, 0, new Error('x')]], ['agent/error', [agent, 1, 0, new Error('x')]],
['approval/request', [{ agent, toolName: 'echo' }, () => Promise.resolve('unavailable')]], ['approval/request', [{ agent, toolName: 'echo' }, () => Promise.resolve('unavailable')]],
['tools/pre-execute', [{ callId: 'c', name: 't', arguments: {}, agent }, () => Promise.resolve({ kind: 'allow' })]], ['tools/pre-execute', [{ callId: 'c', name: 't', arguments: {}, agent }, () => Promise.resolve({ kind: 'allow' })]],
['tools/execute', [{ callId: 'c', name: 't', arguments: {}, agent }, () => Promise.resolve({ callId: 'c', content: [], isError: false })]], ['tools/execute', [{ callId: 'c', name: 't', arguments: {}, agent }, () => Promise.resolve({ content: [], isError: false })]],
['tools/post-execute', [{ callId: 'c', name: 't', arguments: {}, agent }, { callId: 'c', content: [], isError: false }, () => Promise.resolve({ kind: 'accept' })]], ['tools/post-execute', [{ callId: 'c', name: 't', arguments: {}, agent }, { content: [], isError: false }, () => Promise.resolve({ kind: 'accept' })]],
['tools/result', [{ callId: 'c', name: 't', arguments: {}, agent }, { callId: 'c', content: [], isError: false }]], ['tools/result', [{ callId: 'c', name: 't', arguments: {}, agent }, { content: [], isError: false }]],
] ]
for (const [event, args] of rows) { for (const [event, args] of rows) {
const subject = agent const subject = agent

View File

@@ -33,7 +33,6 @@
*/ */
import type { Context } from 'cordis' import type { Context } from 'cordis'
import type { CallId } from '@deepseek-ai/dsh-llm'
import { deadline, timeoutOf } from '@deepseek-ai/dsh-timeout' import { deadline, timeoutOf } from '@deepseek-ai/dsh-timeout'
import type { ToolExecutionResult } from '@deepseek-ai/dsh-tools' import type { ToolExecutionResult } from '@deepseek-ai/dsh-tools'
@@ -57,13 +56,11 @@ export const inject = ['tools']
* is the model-facing message; `error.code` is the same {@link TOOL_TIMEOUT} * is the model-facing message; `error.code` is the same {@link TOOL_TIMEOUT}
* this plugin owns, so a retry/sandbox plugin (and replay) can route on it. * this plugin owns, so a retry/sandbox plugin (and replay) can route on it.
* *
* @param callId - the timed-out call's id, carried onto the replacement result.
* @param timeoutMs - the elapsed budget, rendered into the model-facing message. * @param timeoutMs - the elapsed budget, rendered into the model-facing message.
* @returns the `isError` {@link ToolExecutionResult} with a `TOOL_TIMEOUT` error. * @returns the `isError` {@link ToolExecutionResult} with a `TOOL_TIMEOUT` error.
*/ */
export function toolTimeoutResult(callId: CallId, timeoutMs: number): ToolExecutionResult { function toolTimeoutResult(timeoutMs: number): ToolExecutionResult {
return { return {
callId,
content: [{ type: 'text', text: `Error: tool call timed out after ${timeoutMs}ms` }], content: [{ type: 'text', text: `Error: tool call timed out after ${timeoutMs}ms` }],
isError: true, isError: true,
error: { name: 'ToolTimeoutError', code: TOOL_TIMEOUT }, error: { name: 'ToolTimeoutError', code: TOOL_TIMEOUT },
@@ -108,7 +105,7 @@ export function apply(ctx: Context): void {
// quiescence; replace whatever it returned (its own abort result) with the // quiescence; replace whatever it returned (its own abort result) with the
// structured TOOL_TIMEOUT the model sees. // structured TOOL_TIMEOUT the model sees.
if (timeoutOf(d.signal, TOOL_TIMEOUT) !== undefined) { if (timeoutOf(d.signal, TOOL_TIMEOUT) !== undefined) {
return toolTimeoutResult(exec.callId, timeoutMs) return toolTimeoutResult(timeoutMs)
} }
return result return result
} finally { } finally {

View File

@@ -11,9 +11,9 @@ import { Context } from 'cordis'
import Loader from '@cordisjs/plugin-loader' import Loader from '@cordisjs/plugin-loader'
import { CallId, HarnessError } from '@deepseek-ai/dsh-llm' import { CallId, HarnessError } from '@deepseek-ai/dsh-llm'
import SystemPrompt from '@deepseek-ai/dsh-system-prompt' import SystemPrompt from '@deepseek-ai/dsh-system-prompt'
import ToolRegistry, { defineTool, type ToolExecutionInput, type ToolExecutionResult, type PostToolDecision } from '@deepseek-ai/dsh-tools' import ToolRegistry, { defineTool, type ToolExecutionInput, type PostToolDecision } from '@deepseek-ai/dsh-tools'
import * as timeoutPolicy from '@deepseek-ai/dsh-timeout-policy' import * as timeoutPolicy from '@deepseek-ai/dsh-timeout-policy'
import { TOOL_TIMEOUT, toolTimeoutResult } from '@deepseek-ai/dsh-timeout-policy' import { TOOL_TIMEOUT } from '@deepseek-ai/dsh-timeout-policy'
/** Mount the registry + the zero-config timeout-policy enforcer. */ /** Mount the registry + the zero-config timeout-policy enforcer. */
async function setup() { async function setup() {
@@ -60,7 +60,7 @@ describe('timeout-policy delegation (unconfigured / fast)', () => {
ctx.tools.register(defineTool({ name: 'fast', description: 'd', parameters: {}, timeoutMs: 10_000, ctx.tools.register(defineTool({ name: 'fast', description: 'd', parameters: {}, timeoutMs: 10_000,
async execute() { return [{ type: 'text' as const, text: 'ok' }] } })) async execute() { return [{ type: 'text' as const, text: 'ok' }] } }))
const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'fast', arguments: {} }) const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'fast', arguments: {} })
expect(result).toEqual({ callId: CallId('c1'), content: [{ type: 'text', text: 'ok' }], isError: false }) expect(result).toEqual({ content: [{ type: 'text', text: 'ok' }], isError: false })
}) })
it('a budgeted tool receives the DERIVED deadline signal (not the caller signal) during dispatch', async () => { it('a budgeted tool receives the DERIVED deadline signal (not the caller signal) during dispatch', async () => {
@@ -109,7 +109,6 @@ describe('timeout-policy TOOL_TIMEOUT replacement (deadline wins)', () => {
await vi.advanceTimersByTimeAsync(150) await vi.advanceTimersByTimeAsync(150)
const result = await pending const result = await pending
expect(result).toEqual({ expect(result).toEqual({
callId: CallId('c1'),
content: [{ type: 'text', text: 'Error: tool call timed out after 100ms' }], content: [{ type: 'text', text: 'Error: tool call timed out after 100ms' }],
isError: true, isError: true,
error: { name: 'ToolTimeoutError', code: 'TOOL_TIMEOUT' }, error: { name: 'ToolTimeoutError', code: 'TOOL_TIMEOUT' },
@@ -140,16 +139,7 @@ describe('timeout-policy TOOL_TIMEOUT replacement (deadline wins)', () => {
}) })
}) })
describe('toolTimeoutResult', () => { describe('timeout-policy contract', () => {
it('builds the structured TOOL_TIMEOUT result', () => {
expect(toolTimeoutResult(CallId('c9'), 250)).toEqual({
callId: CallId('c9'),
content: [{ type: 'text', text: 'Error: tool call timed out after 250ms' }],
isError: true,
error: { name: 'ToolTimeoutError', code: 'TOOL_TIMEOUT' },
} satisfies ToolExecutionResult)
})
it('exposes the owned code constant', () => { it('exposes the owned code constant', () => {
expect(TOOL_TIMEOUT).toBe('TOOL_TIMEOUT') expect(TOOL_TIMEOUT).toBe('TOOL_TIMEOUT')
}) })