From 02ca71db5755e1a55fffbdd37586eda4749ece2a Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 12 Jul 2026 22:39:01 +0800 Subject: [PATCH] refactor(core): simplify tools prompts and trusted services --- packages/core/system-prompt/README.md | 18 +- packages/core/system-prompt/package.json | 2 - packages/core/system-prompt/src/index.ts | 352 +++-------- .../core/system-prompt/tests/scoped.spec.ts | 24 +- .../system-prompt/tests/system-prompt.spec.ts | 200 +----- .../system-prompt/tests/tool-order.spec.ts | 94 --- packages/core/system-prompt/tsconfig.json | 3 - packages/core/tools/README.md | 26 +- packages/core/tools/src/code-mode.ts | 1 + packages/core/tools/src/index.ts | 595 ++++++------------ packages/core/tools/src/schema.ts | 53 +- packages/core/tools/tests/code-mode.spec.ts | 26 +- packages/core/tools/tests/scoped.spec.ts | 86 +-- packages/core/tools/tests/tools.spec.ts | 591 +---------------- packages/skill/skill/README.md | 18 +- packages/skill/skill/src/index.ts | 319 ++++------ packages/skill/skill/tests/skill.spec.ts | 299 ++------- packages/support/invariants/package.json | 4 - packages/support/invariants/src/index.ts | 61 +- .../invariants/tests/invariants.spec.ts | 86 --- packages/support/invariants/tsconfig.json | 6 - packages/ui/user-approval/README.md | 2 +- packages/ui/user-approval/src/index.ts | 186 ++---- .../ui/user-approval/tests/approval.spec.ts | 279 ++------ 24 files changed, 636 insertions(+), 2695 deletions(-) diff --git a/packages/core/system-prompt/README.md b/packages/core/system-prompt/README.md index 06d735e271..507ba86c5e 100644 --- a/packages/core/system-prompt/README.md +++ b/packages/core/system-prompt/README.md @@ -1,6 +1,6 @@ # dsh-system-prompt -System prompt assembly registry. Plugins contribute ordered text sections, tool-schema providers, named prompt variables, and owner-final named protections; the agent loop calls `assemble(context)` once per step, and `renderPrompt(assembly)` is the full system prompt the model sees. The plugin registers the harness-owned openers itself — the static `harness:identity` section and the deployment's `deployment:persona` section — so they exist for every agent regardless of which loop plugin drives it. +System prompt assembly registry. Plugins contribute ordered text sections, tool-schema providers, and named prompt variables; contributions that implement required protocol may declare themselves owner-final. The agent loop calls `assemble(context)` once per step, and `renderPrompt(assembly)` is the full system prompt the model sees. The plugin registers the harness-owned openers itself — the static `harness:identity` section and the deployment's `deployment:persona` section — so they exist for every agent regardless of which loop plugin drives it. ## Config @@ -13,22 +13,20 @@ System prompt assembly registry. Plugins contribute ordered text sections, tool- ### Public API -- `ctx.systemPrompt.section(section: PromptSection): () => Promise | void` Contribute a section. The registry reads `name`, `order`, and the text value/callback once, validates their fixed string/finite-number/string-or-function types, and stores only that accepted record; later caller-object mutation cannot rename or reshape it. The layer is the CALLING context's scope: `agent.ctx` contributes to that agent alone, SHADOWING a same-named global section there (the per-agent persona mechanism — a scoped `deployment:persona`). Duplicate names within one layer throw, and a globally protected section name cannot be shadowed. Disposed with the calling fiber. -- `ctx.systemPrompt.tools(provider: (context: AssembleContext) => ToolProviderResult): () => Promise | void` Contribute tool schemas, evaluated at each assembly with that assembly's context; a non-function provider rejects before effect storage. `ToolProviderResult` = `{ schemas, knownNames? }`: `schemas` is the post-restriction visible set for `context.scope`; `knownNames` (defaulting to the same captured schemas' names) is the pre-restriction universe `toolOrder` validates against. Assembly reads the result, each schema field, and the optional known-name list once before detaching them, rejects non-string schema names/descriptions or known names, and uses those same accepted strings for validation and the model-visible collection. A provider must not return a schema named `TOOL_ORDER_REST`. Scoped providers are consulted only for their scope's assemblies. Disposed with the calling fiber. -- `ctx.systemPrompt.variable(name: string, provider: (context) => string | undefined): () => Promise | void` Contribute a prompt variable, referenced from section text as `{{name}}`. The fixed string name and function provider types reject before effect storage. Scoped variables (via `agent.ctx`) shadow a same-named global for that agent. Duplicate-in-layer or unreferenceable names throw; `undefined` means "no value for this assembly". Disposed with the calling fiber. -- `ctx.systemPrompt.protect(protection: PromptProtection): () => Promise | void` Make named section/tool contributions owner-final after the assembly waterfall. Protection restores canonical registry/provider presence and definition; restored entries keep canonical order with one another and anchor before their first surviving later unprotected canonical neighbor (or at the end), without undoing listener reordering of unprotected entries. Canonical absence is owner-final too, so a mode-hidden tool cannot be fabricated by a listener. Calling through `agent.ctx` protects only that agent's assemblies. A global section protection additionally reserves its name against scoped shadows; registering either side of that conflict fails loudly instead of treating the shadow as canonical. Each optional field and array slot is read once, non-array fields or non-string names reject before effect storage, and the accepted arrays are deduplicated and frozen. Finalization materializes each waterfall-produced entry name once, so a stateful getter cannot evade canonical replacement. Empty protections throw, and disposal removes the protection. -- `ctx.systemPrompt.assemble(context?: AssembleContext): Promise` Assemble the prompt for one caller: the global layer merged with `context.scope`'s layer (scoped shadows global). Provider output becomes one coherent detached snapshot before `toolOrder` validation. Runs through the scope-filtered `system-prompt/assemble` waterfall, then restores protected contributions from the pre-waterfall canonical assembly. Rejects when a configured `toolOrder` names a tool outside the providers' `knownNames` universe (a restricted-away KNOWN tool is a normal absence), or when a provider returns the reserved rest-entry name. +- `ctx.systemPrompt.section(section: PromptSection): () => Promise | void` Contribute a section. The layer is the calling context's scope: `agent.ctx` contributes to that agent alone, shadowing a same-named global section there. Duplicate names within one layer and non-finite orders throw. `ownerFinal: true` restores this section's canonical definition after the complete waterfall and reserves a global section against scoped shadows. Disposed with the calling fiber. +- `ctx.systemPrompt.tools(provider: (context: AssembleContext) => ToolProviderResult): () => Promise | void` Contribute tool schemas, evaluated at each assembly with that assembly's context. `ToolProviderResult` = `{ schemas, knownNames?, ownerFinalNames? }`: `schemas` is the post-restriction visible set; `knownNames` is the pre-restriction universe used by `toolOrder`; `ownerFinalNames` makes those tools' canonical presence or absence survive the waterfall. A provider must not return a schema named `TOOL_ORDER_REST`. Scoped providers are consulted only for their scope's assemblies. Disposed with the calling fiber. +- `ctx.systemPrompt.variable(name: string, provider: (context) => string | undefined): () => Promise | void` Contribute a prompt variable, referenced from section text as `{{name}}`. Scoped variables shadow a same-named global for that agent. Duplicate-in-layer or unreferenceable names throw; `undefined` means "no value for this assembly". Disposed with the calling fiber. +- `ctx.systemPrompt.assemble(context?: AssembleContext): Promise` Assemble the prompt for one caller: the global layer merged with `context.scope`'s layer, with tool schemas detached before the transform seam. Runs through the scope-filtered `system-prompt/assemble` waterfall, then restores owner-final contributions from a private pre-waterfall snapshot. Restored entries keep canonical relative order without undoing listener reordering of ordinary entries. Rejects when a configured `toolOrder` names a tool outside the providers' `knownNames` universe, or when a provider returns the reserved rest-entry name. ### Live events -Prompt assembly is the scope-filtered transformable seam; registry change is the deliberately unfiltered notification that an assembly input changed, possibly for one scope. Exact signatures, dispatch modes, and filtering contracts live in the generated [Cordis event catalog](../../../docs/cordis-catalog/events.md). Named protections apply only after a successful assembly waterfall returns and are owned by the service rather than represented as another event listener. +Prompt assembly is the scope-filtered transformable seam; registry change is the deliberately unfiltered notification that an assembly input changed, possibly for one scope. Exact signatures, dispatch modes, and filtering contracts live in the generated [Cordis event catalog](../../../docs/cordis-catalog/events.md). Owner-final restoration applies only after a successful assembly waterfall returns. ### Key types - `AssembleContext` — what one `assemble()` call is FOR. Merge-extensible; declares `scope?: ScopeKey` (the layer selector) here, and `dsh-agent` declares `agent?: Agent` (the typed DX field — never set without `scope`; use `assembleContextFor(agent)`). Providers must tolerate absent fields (a bare `assemble()` carries an empty, scope-less context). -- `PromptSection` — `{ name, order, text: string | ((context) => string) }`. Sections are concatenated in ascending `order`. Order bands: `-100` is the harness identity, `0` the deployment persona (both registered by this plugin), tool guidance uses `100–199`; other negative orders also render before the persona. +- `PromptSection` — `{ name, order, text, ownerFinal? }`. Sections are concatenated in ascending `order`; `ownerFinal` is reserved for required protocol instructions. Order bands: `-100` is the harness identity, `0` the deployment persona, tool guidance uses `100–199`. - `PromptAssembly` — `{ sections: AssembledSection[], tools: ToolSchema[], variables: Record }`. Section texts arrive resolved but not yet interpolated; `variables` holds every registered variable resolved against the context. Tool schemas are part of the assembly by design: "what the model is told it can do" is one coherent thing, even though adapters transmit schemas as a separate wire field. -- `PromptProtection` — `{ sections?: readonly string[], tools?: readonly string[] }`. Named contributions whose canonical pre-waterfall state is restored after all listeners; protections compose by set union rather than callback order, and global section names are reserved against scoped shadows. - `renderPrompt(assembly)` — interpolates `{{variable}}` references in each section, drops empty sections, joins with blank lines. STRICT: an unknown reference (`Object.hasOwn` lookup — prototype names like `{{constructor}}` are unknown), a registered-but-valueless reference, a malformed complete `{{…}}` group, or a `{{` that opens no complete group while a `}}` still follows (`{{{model}}}`) throws — fail loud beats shipping a malformed prompt. A lone `{{` with no `}}` anywhere after it passes through verbatim; substituted values are never re-scanned. Merge-extensible: plugins can declare extra fields on `PromptAssembly` and `AssembleContext` via declaration merging. @@ -39,7 +37,7 @@ Merge-extensible: plugins can declare extra fields on `PromptAssembly` and `Asse - Variable providers: the agent loop registers `model` and `cwd`; any plugin can register the facts it owns (a future `date`, git state, …). - Tool schema providers: `ToolRegistry` registers itself as a tool provider automatically. - The `system-prompt/assemble` waterfall: mutate or replace the assembly per caller (dynamic tool filtering, extra variables). -- `systemPrompt.protect()`: reserve canonical section/tool contributions for invariants that ordinary waterfall listeners must not be able to remove or replace. +- Owner-final contributions: protocol owners declare finality on the section or tool contribution itself; there is no independent protection registry. ### What is NOT here diff --git a/packages/core/system-prompt/package.json b/packages/core/system-prompt/package.json index 69af28f3b5..120e10ef11 100644 --- a/packages/core/system-prompt/package.json +++ b/packages/core/system-prompt/package.json @@ -24,7 +24,6 @@ "peerDependencies": { "@deepseek-ai/dsh-llm": "^0.0.1", "@deepseek-ai/dsh-scope": "^0.0.1", - "@deepseek-ai/dsh-session": "^0.0.1", "cordis": "^4.0.0-rc.6" }, "dependencies": { @@ -33,7 +32,6 @@ "devDependencies": { "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-scope": "workspace:^", - "@deepseek-ai/dsh-session": "workspace:^", "cordis": "^4.0.0-rc.6" } } diff --git a/packages/core/system-prompt/src/index.ts b/packages/core/system-prompt/src/index.ts index 40eddc7ba1..b1946c59e1 100644 --- a/packages/core/system-prompt/src/index.ts +++ b/packages/core/system-prompt/src/index.ts @@ -1,8 +1,8 @@ /** * System prompt assembly registry. Plugins contribute ordered text sections, - * tool schema providers, named prompt variables, and owner-final named - * protections; `assemble(context)` collates them through a waterfall that - * runs once per step, restores protected contributions, and `renderPrompt` + * tool schema providers, and named prompt variables; protocol contributions + * may declare themselves owner-final. `assemble(context)` collates them through a waterfall that + * runs once per step, restores owner-final contributions, and `renderPrompt` * interpolates `{{variable}}` references into the final text. * * The harness-owned prompt openers live here too: this plugin registers the @@ -18,7 +18,6 @@ import z from 'schemastery' import { scopeOf, scopeTarget } from '@deepseek-ai/dsh-scope' import type { ScopeKey, Scoped } from '@deepseek-ai/dsh-scope' import type { ToolSchema } from '@deepseek-ai/dsh-llm' -import { snapshotJsonValue } from '@deepseek-ai/dsh-session' declare module 'cordis' { interface Context { @@ -45,7 +44,7 @@ declare module 'cordis' { */ 'system-prompt/assemble'(this: Scoped, assembly: PromptAssembly, context: AssembleContext, next: () => Promise): Promise /** - * A section, tool provider, variable provider, or protection was registered + * A section, tool provider, or variable provider was registered * or unregistered (the assembly inputs changed — possibly for one scope * only). An UNFILTERED registry-subject notification, deliberately not * scope-filtered dispatch: a global change concerns every agent's next @@ -81,19 +80,25 @@ export interface AssembleContext { /** One contributed section of the system prompt (registry input). */ export interface PromptSection { /** Unique name — a duplicate registration throws (see {@link SystemPrompt.section}). */ - name: string + readonly name: string /** * Sections are concatenated in ascending order. Convention: `-100` is the * harness identity, `0` the deployment persona, tool guidance uses 100–199; * other negative orders also render before the persona. */ - order: number + readonly order: number /** * Static text or a provider evaluated at each assembly with that assembly's * {@link AssembleContext}. The text may reference `{{variable}}`s — they are * interpolated later, by {@link renderPrompt}. */ - text: string | ((context: AssembleContext) => string) + readonly text: string | ((context: AssembleContext) => string) + /** + * Whether this section's canonical presence and definition survive the + * complete assembly waterfall. Use this only for owner-required protocol + * instructions; ordinary sections remain transformable. + */ + readonly ownerFinal?: boolean } /** One section of an assembly: {@link PromptSection} with its text resolved. */ @@ -118,30 +123,15 @@ export interface AssembledSection { */ export interface ToolProviderResult { /** The schemas this provider contributes to THIS assembly. */ - schemas: ToolSchema[] + readonly schemas: readonly ToolSchema[] /** The pre-restriction name universe for config validation (defaults to `schemas`' names). */ - knownNames?: readonly string[] -} - -/** - * Canonical prompt contributions that survive the assembly waterfall. - * - * Protection is declarative by contribution name rather than an ordered - * callback: after every `system-prompt/assemble` listener has finished, the - * service restores each protected name to the exact presence and definition - * produced by its registries before the waterfall. Restored entries keep - * canonical order with one another and anchor before their first surviving - * later unprotected canonical neighbor (or at the end); the service does not - * undo a listener's reordering of unprotected entries. A name absent from that - * canonical assembly is removed from the result. This makes mode-dependent - * absence protectable too (for example, a native tool that intentionally stays - * off the wire in Code Mode). - */ -export interface PromptProtection { - /** Section names whose canonical presence and definition are restored after the waterfall. */ - sections?: readonly string[] - /** Tool names whose canonical presence and definition are restored after the waterfall. */ - tools?: readonly string[] + readonly knownNames?: readonly string[] + /** + * Tool names this provider owns finally. The names need not be present in + * `schemas`: naming a mode-hidden tool makes its canonical absence final, so + * an assembly listener cannot fabricate it onto the wire. + */ + readonly ownerFinalNames?: readonly string[] } /** @@ -234,76 +224,28 @@ function orderTools(tools: ToolSchema[], toolOrder: string[] | undefined, knownN name === TOOL_ORDER_REST ? rest : tools.filter(tool => tool.name === name)) } -/** Snapshot one waterfall-produced named entry with a stable, own data `name`. */ -function snapshotNamedEntry(entry: T): { entry: T; name: string } { - // Read the name exactly once before protection matching. The waterfall owns - // its output and may return accessor-backed records; retaining such an entry - // would let a getter answer "unprotected" during filtering and the protected - // name later when a consumer reads the final assembly. - const name = entry.name - const snapshot: Record = {} - Object.defineProperty(snapshot, 'name', { - value: name, - enumerable: true, - configurable: true, - writable: true, - }) - // Copy every other enumerable field once while deliberately skipping name. - // defineProperty keeps a literal "__proto__" extension field ordinary data. - for (const key of Object.keys(entry)) { - if (key === 'name') continue - Object.defineProperty(snapshot, key, { - value: (entry as unknown as Record)[key], - enumerable: true, - configurable: true, - writable: true, - }) - } - return { entry: snapshot as T, name } -} - -/** Restore protected named entries from `canonical`, anchored before their next unprotected canonical neighbor. */ -function restoreProtected( - canonical: readonly T[], result: readonly T[], protectedNames: ReadonlySet, +/** Restore owner-final entries from `canonical`, anchored before their next ordinary canonical neighbor. */ +function restoreOwnerFinal( + canonical: readonly T[], result: readonly T[], ownerFinalNames: ReadonlySet, ): T[] { - const restored = result - .map(snapshotNamedEntry) - .filter(record => !protectedNames.has(record.name)) + const restored = result.filter(entry => !ownerFinalNames.has(entry.name)) for (const [index, entry] of canonical.entries()) { - if (!protectedNames.has(entry.name)) continue + if (!ownerFinalNames.has(entry.name)) continue // Protected entries are inserted in canonical order. Anchor each one // before the first later UNPROTECTED canonical neighbor that survived the // waterfall; if none survived, it belongs at the end. Looking only at - // unprotected neighbors avoids reversing adjacent protected entries. + // ordinary neighbors avoids reversing adjacent owner-final entries. const following = new Set( canonical.slice(index + 1) - .filter(candidate => !protectedNames.has(candidate.name)) + .filter(candidate => !ownerFinalNames.has(candidate.name)) .map(candidate => candidate.name), ) const next = restored.findIndex(candidate => following.has(candidate.name)) - restored.splice(next < 0 ? restored.length : next, 0, { - entry: structuredClone(entry), - name: entry.name, - }) + // `canonical` is an owned snapshot made before the waterfall; no second + // clone is needed when moving its entries into the finalized assembly. + restored.splice(next < 0 ? restored.length : next, 0, entry) } - return restored.map(record => record.entry) -} - -/** Validate and detach one protection-name array without rereading an element. */ -function snapshotProtectionNames(value: unknown, field: 'sections' | 'tools'): readonly string[] { - if (!Array.isArray(value)) { - throw new TypeError(`systemPrompt.protect() ${field} must be an array of strings`) - } - const names: string[] = [] - const length = value.length - for (let index = 0; index < length; index += 1) { - const name: unknown = value[index] - if (typeof name !== 'string') { - throw new TypeError(`systemPrompt.protect() ${field} must be an array of strings`) - } - names.push(name) - } - return Object.freeze([...new Set(names)]) + return restored } /** Lexicographic (code-unit) name comparison — locale-independent, so the order is identical on every machine. */ @@ -423,7 +365,7 @@ function interpolate(section: AssembledSection, variables: Record ToolProviderResult)[] = [] private variableProviders = new Map string | undefined>() - private protections: PromptProtection[] = [] /** Per-scope layers (`@deepseek-ai/dsh-scope`); entries drop when a layer empties, so a disposed scope leaves no residue. */ private scopedSections = new Map() private scopedToolProviders = new Map ToolProviderResult)[]>() private scopedVariableProviders = new Map string | undefined>>() - private scopedProtections = new Map() private readonly toolOrder: string[] | undefined constructor(ctx: Context, public config: Config) { @@ -480,13 +420,11 @@ export class SystemPrompt extends Service { * scoped context (`agent.ctx`) contributes to that scope alone — and a * scoped section SHADOWS a same-named global section for that scope's * assemblies (most-specific-wins; this is how a per-agent persona overrides - * `deployment:persona`) unless that global name is protected: global - * protection reserves its section name against scoped shadows so the + * `deployment:persona`) unless that global contribution is owner-final: it + * reserves its section name against scoped shadows so the * registration owner—not a later scope—defines the canonical value. The - * registry reads `name`, `order`, and `text` once, validates their fixed - * string/finite-number/string-or-function types, and stores only that - * accepted record, so later caller-object mutation cannot rename or reshape - * a contribution. Throws if the SAME layer already has the name (a + * readonly typed contribution is borrowed until disposal; only the semantic + * finite-order rule is checked at runtime. Throws if the SAME layer already has the name (a * duplicate would silently double prompt text — e.g. a double-loaded tool * plugin; the global-duplicate message names `agent.ctx` as the per-agent * alternative). Removed when the calling fiber is disposed. Emits @@ -497,25 +435,20 @@ export class SystemPrompt extends Service { * yield it directly — exact identity nests the teardown in order. */ section(section: PromptSection): () => Promise | void { - const input: unknown = section - if (typeof input !== 'object' || input === null) { - throw new TypeError('systemPrompt.section() requires a section object') - } - const accepted = input as PromptSection - const name = accepted.name - const order = accepted.order - const text = accepted.text - if (typeof name !== 'string') throw new TypeError('prompt section name must be a string') - if (typeof order !== 'number' || !Number.isFinite(order)) { - throw new TypeError(`prompt section "${name}" order must be a finite number`) - } - if (typeof text !== 'string' && typeof text !== 'function') { - throw new TypeError(`prompt section "${name}" text must be a string or function`) + if (!Number.isFinite(section.order)) { + throw new TypeError(`prompt section "${section.name}" order must be a finite number`) } const scope = scopeOf(this.ctx) - const snapshot: PromptSection = { name, order, text } - if (scope !== undefined && this.protections.some(record => record.sections?.includes(snapshot.name))) { - throw new Error(`prompt section "${snapshot.name}" is globally protected and cannot be shadowed in an agent scope`) + if (scope !== undefined + && this.sections.some(global => global.name === section.name && global.ownerFinal === true)) { + throw new Error(`prompt section "${section.name}" is globally owner-final and cannot be shadowed in an agent scope`) + } + if (scope === undefined && section.ownerFinal === true) { + const hasScopedShadow = [...this.scopedSections.values()] + .some(layer => layer.some(scoped => scoped.name === section.name)) + if (hasScopedShadow) { + throw new Error(`owner-final prompt section "${section.name}" cannot be registered while a scoped shadow exists`) + } } const dispose = this.ctx.effect(function* (this: SystemPrompt) { const layer = scope === undefined @@ -525,18 +458,18 @@ export class SystemPrompt extends Service { this.scopedSections.set(scope, created) return created })() - if (layer.some(existing => existing.name === snapshot.name)) { + if (layer.some(existing => existing.name === section.name)) { throw new Error(scope === undefined - ? `prompt section "${snapshot.name}" is already registered (for a per-agent override, register through that agent's \`agent.ctx\` instead)` - : `prompt section "${snapshot.name}" is already registered in this scope`) + ? `prompt section "${section.name}" is already registered (for a per-agent override, register through that agent's \`agent.ctx\` instead)` + : `prompt section "${section.name}" is already registered in this scope`) } - layer.push(snapshot) + layer.push(section) // Yield the rollback BEFORE emitting `system-prompt/change`: a generator // effect collects each yielded disposer before the next step runs, so a // throwing change listener removes the section instead of leaking it into // every future assembly. yield () => { - const index = layer.indexOf(snapshot) + const index = layer.indexOf(section) /* v8 ignore next 3 -- defensive: section was registered, so indexOf is guaranteed >= 0 */ if (index >= 0) layer.splice(index, 1) if (scope !== undefined && layer.length === 0) this.scopedSections.delete(scope) @@ -560,8 +493,7 @@ export class SystemPrompt extends Service { * `schemas`/`knownNames` split). The layer is decided by the calling * context: a scoped provider (registered through `agent.ctx`) is consulted * only for that scope's assemblies. Removed when the calling fiber is - * disposed. A non-function provider is rejected before any effect is stored. - * A provider must not return a schema named + * disposed. A provider must not return a schema named * {@link TOOL_ORDER_REST}; that name is reserved for * {@link Config.toolOrder}'s rest entry and rejects the assembly. Emits * `system-prompt/change`. @@ -571,9 +503,6 @@ export class SystemPrompt extends Service { * yield it directly — exact identity nests the teardown in order. */ tools(provider: (context: AssembleContext) => ToolProviderResult): () => Promise | void { - if (typeof provider !== 'function') { - throw new TypeError('system prompt tool provider must be a function') - } const scope = scopeOf(this.ctx) const dispose = this.ctx.effect(function* (this: SystemPrompt) { const layer = scope === undefined @@ -611,8 +540,7 @@ export class SystemPrompt extends Service { * deployment must not claim facts it does not have). The layer is decided * by the calling context: a scoped variable (registered through * `agent.ctx`) resolves only for that scope's assemblies and SHADOWS a - * same-named global variable there. The fixed name and callback types are - * validated before effect storage. Throws on a name that does not match + * same-named global variable there. Throws on a name that does not match * `[a-z][a-z0-9_]*` (it could never be referenced) or one already registered * in the SAME layer. Removed when the calling fiber is disposed; emits * `system-prompt/change` on register/unregister. @@ -623,13 +551,8 @@ export class SystemPrompt extends Service { * yield it directly — exact identity nests the teardown in order. */ variable(name: string, provider: (context: AssembleContext) => string | undefined): () => Promise | void { - const inputName: unknown = name - if (typeof inputName !== 'string') throw new TypeError('prompt variable name must be a string') - if (!VARIABLE_NAME.test(inputName)) { - throw new Error(`invalid prompt variable name "${inputName}" (must match ${String(VARIABLE_NAME)})`) - } - if (typeof provider !== 'function') { - throw new TypeError(`prompt variable "${inputName}" provider must be a function`) + if (!VARIABLE_NAME.test(name)) { + throw new Error(`invalid prompt variable name "${name}" (must match ${String(VARIABLE_NAME)})`) } const scope = scopeOf(this.ctx) const dispose = this.ctx.effect(function* (this: SystemPrompt) { @@ -663,91 +586,6 @@ export class SystemPrompt extends Service { return dispose } - /** - * Protect named section/tool contributions from the assembly waterfall. - * The layer is decided by the calling context: a global protection applies - * to every assembly, while one registered through `agent.ctx` applies only - * to that agent's scope. The name's canonical registry/provider output is - * restored AFTER the whole waterfall, so listener registration order cannot - * strip, replace, duplicate, or fabricate it. Canonical absence is restored - * too: if the protected name is intentionally absent for an assembly, a - * listener-injected entry with that name is removed. Each optional field and - * array slot is read once; non-array fields or non-string names reject before - * effect storage, and the accepted deduplicated arrays are frozen. During - * finalization each waterfall-produced entry name is likewise read once into - * an owned data record, so a stateful getter cannot look unprotected during - * filtering and later impersonate a protected name. An empty protection - * throws because it cannot affect output. - * Removed with the calling fiber and emits `system-prompt/change` on - * registration/unregistration. A global section protection also reserves the - * name against scoped section shadows; registering protection when such a - * shadow already exists fails loudly instead of protecting the wrong owner. - * @param protection - section and/or tool names whose canonical presence and definitions are restored after the waterfall. - * @returns the exact Cordis effect disposer that removes the protection. - */ - protect(protection: PromptProtection): () => Promise | void { - const input: unknown = protection - if (typeof input !== 'object' || input === null) { - throw new TypeError('systemPrompt.protect() requires a protection object') - } - const accepted = input as PromptProtection - const inputSections = accepted.sections - const inputTools = accepted.tools - const sections = inputSections === undefined - ? undefined - : snapshotProtectionNames(inputSections, 'sections') - const tools = inputTools === undefined - ? undefined - : snapshotProtectionNames(inputTools, 'tools') - const scope = scopeOf(this.ctx) - const snapshot: PromptProtection = Object.freeze({ - ...sections !== undefined ? { sections } : {}, - ...tools !== undefined ? { tools } : {}, - }) - if ((snapshot.sections?.length ?? 0) === 0 && (snapshot.tools?.length ?? 0) === 0) { - throw new Error('systemPrompt.protect() requires at least one section or tool name') - } - if (scope === undefined && snapshot.sections !== undefined) { - const protectedSections = new Set(snapshot.sections) - const conflicts = [...this.scopedSections.values()] - .flatMap(layer => layer.filter(section => protectedSections.has(section.name)).map(section => section.name)) - if (conflicts.length > 0) { - throw new Error(`systemPrompt.protect() cannot globally protect section${conflicts.length > 1 ? 's' : ''} ${[...new Set(conflicts)].map(name => `"${name}"`).join(', ')} while scoped shadows are registered`) - } - } - const dispose = this.ctx.effect(function* (this: SystemPrompt) { - const layer = scope === undefined - ? this.protections - : this.scopedProtections.get(scope) ?? (() => { - const created: PromptProtection[] = [] - this.scopedProtections.set(scope, created) - return created - })() - layer.push(snapshot) - yield () => { - const index = layer.indexOf(snapshot) - /* v8 ignore next 3 -- defensive: protection was registered, so indexOf is guaranteed >= 0 */ - if (index >= 0) layer.splice(index, 1) - if (scope !== undefined && layer.length === 0) this.scopedProtections.delete(scope) - this.ctx.emit('system-prompt/change') - } - this.ctx.emit('system-prompt/change') - }.bind(this), 'systemPrompt.protect()') - return dispose - } - - /** Resolve the owner-final names registered for one assembly scope. */ - private protectedNames(scope: ScopeKey | undefined): { sections: Set; tools: Set } { - const records = [ - ...this.protections, - ...(scope === undefined ? [] : this.scopedProtections.get(scope)) ?? [], - ] - return { - sections: new Set(records.flatMap(record => record.sections ?? [])), - tools: new Set(records.flatMap(record => record.tools ?? [])), - } - } - /** * Assemble the current prompt for one caller: the global layer merged with * {@link AssembleContext.scope}'s layer (scoped sections/variables SHADOW @@ -760,14 +598,12 @@ export class SystemPrompt extends Service { * the providers' `knownNames` universe rejects the assembly, while a known * name restricted away for this scope is a normal absence), and every * visible variable resolved against `context` into `assembly.variables`. - * Each provider result and schema field is read once; those same captured - * names drive both `toolOrder` validation and the model-visible collection. - * Tool schemas are deep-cloned because adapters and request waterfalls may - * mutate schema objects. Runs through the `system-prompt/assemble` + * Tool schemas are detached because assembly waterfalls may mutate them. + * Runs through the `system-prompt/assemble` * waterfall, giving listeners the opportunity to mutate or replace the - * assembly, then restores every visible {@link PromptProtection} from the - * pre-waterfall canonical assembly. Like the sections' `order` sort, tool - * canonicalization happens on the initial assembly; unprotected listener + * assembly, then restores every contribution whose owner declared it final + * from the pre-waterfall canonical assembly. Like the sections' `order` sort, tool + * canonicalization happens on the initial assembly; ordinary listener * output owns its own determinism. Await the result before reading the * assembly values — waterfall listeners may be async. * Interpolation happens later, in {@link renderPrompt}. @@ -780,10 +616,6 @@ export class SystemPrompt extends Service { // (`assemble().catch(...)` would miss it). async assemble(context: AssembleContext = {}): Promise { const scope = context.scope - // Protection is a registry input too: snapshot which names are protected - // at assembly start. Registrations that land while an async waterfall is - // in flight affect the NEXT assembly, matching the other registries. - const protectedNames = this.protectedNames(scope) // Variables: global layer first, then the scope's layer OVERWRITES // same-named entries (shadowing — a per-agent value wins for that agent). const variables: Record = {} @@ -803,6 +635,11 @@ export class SystemPrompt extends Service { for (const section of (scope === undefined ? [] : this.scopedSections.get(scope)) ?? []) { sectionByName.set(section.name, section) } + const ownerFinalSections = new Set( + [...sectionByName.values()] + .filter(section => section.ownerFinal === true) + .map(section => section.name), + ) // Tools: consult the global providers plus the scope's, each with this // assembly's context. `schemas` are what the model may see (already // post-restriction, per provider); `knownNames` (defaulting to the @@ -815,45 +652,18 @@ export class SystemPrompt extends Service { ] const collected: ToolSchema[] = [] const knownNames = new Set() + const ownerFinalTools = new Set() for (const provider of providers) { const result = provider(context) - // One provider result snapshot: `schemas`, `knownNames`, and each schema - // field may be accessor-backed. The same captured names must drive both - // toolOrder validation and the model-visible collection. - const inputSchemas = result.schemas - const inputKnownNames = result.knownNames - const schemas = inputSchemas.map((tool, index): ToolSchema => { - const name = tool.name - const description = tool.description - const inputParameters = tool.parameters - if (typeof name !== 'string') { - throw new TypeError(`system prompt tool schema at index ${index} name must be a string`) - } - if (typeof description !== 'string') { - throw new TypeError(`system prompt tool "${name}" description must be a string`) - } - const parameters = snapshotJsonValue(inputParameters) - if (parameters === undefined) { - throw new TypeError(`system prompt tool "${name}" parameters must be losslessly JSON-serializable`) - } - return { name, description, parameters } - }) - let acceptedKnownNames: string[] - if (inputKnownNames === undefined) { - acceptedKnownNames = schemas.map(tool => tool.name) - } else { - if (!Array.isArray(inputKnownNames)) { - throw new TypeError('system prompt tool provider knownNames must be an array of strings') - } - acceptedKnownNames = Array.from(inputKnownNames, (name) => { - if (typeof name !== 'string') { - throw new TypeError('system prompt tool provider knownNames must be an array of strings') - } - return name - }) - } + const schemas = result.schemas.map(({ name, description, parameters }): ToolSchema => ({ + name, + description, + parameters: structuredClone(parameters), + })) + const acceptedKnownNames = result.knownNames ?? schemas.map(tool => tool.name) collected.push(...schemas) for (const name of acceptedKnownNames) knownNames.add(name) + for (const name of result.ownerFinalNames ?? []) ownerFinalTools.add(name) } const assembly: PromptAssembly = { sections: [...sectionByName.values()] @@ -866,11 +676,11 @@ export class SystemPrompt extends Service { tools: orderTools(collected, this.toolOrder, knownNames), variables, } - // Snapshot only the fields protection can restore. The waterfall receives + // Snapshot only the owner-final fields. The waterfall receives // `assembly` by reference and may mutate it or return a replacement; these // independent snapshots remain the authoritative registry product. - const canonicalSections = protectedNames.sections.size > 0 ? structuredClone(assembly.sections) : undefined - const canonicalTools = protectedNames.tools.size > 0 ? structuredClone(assembly.tools) : undefined + const canonicalSections = ownerFinalSections.size > 0 ? structuredClone(assembly.sections) : undefined + const canonicalTools = ownerFinalTools.size > 0 ? structuredClone(assembly.tools) : undefined const result = await this.ctx.waterfall( scopeTarget(this, scope), 'system-prompt/assemble', assembly, context, () => Promise.resolve(assembly), @@ -881,10 +691,10 @@ export class SystemPrompt extends Service { return { ...result, ...canonicalSections !== undefined - ? { sections: restoreProtected(canonicalSections, result.sections, protectedNames.sections) } + ? { sections: restoreOwnerFinal(canonicalSections, result.sections, ownerFinalSections) } : {}, ...canonicalTools !== undefined - ? { tools: restoreProtected(canonicalTools, result.tools, protectedNames.tools) } + ? { tools: restoreOwnerFinal(canonicalTools, result.tools, ownerFinalTools) } : {}, } } diff --git a/packages/core/system-prompt/tests/scoped.spec.ts b/packages/core/system-prompt/tests/scoped.spec.ts index d59e3a234a..329a66813f 100644 --- a/packages/core/system-prompt/tests/scoped.spec.ts +++ b/packages/core/system-prompt/tests/scoped.spec.ts @@ -63,19 +63,16 @@ describe('scoped sections', () => { expect(() => scope.ctx.systemPrompt.section({ name: 'y', order: 1, text: 'b' })).toThrow(/already registered in this scope/) }) - it.each([ - [['reserved'], 'section "reserved"'], - [['first', 'second'], 'sections "first", "second"'], - ])('rejects global protection added after scoped shadows (%j)', async (names, message) => { + it('rejects a global owner-final section added after a scoped shadow', async () => { const ctx = await mount() const scope = await mintScope(ctx, 'child') - for (const name of names) { - scope.ctx.systemPrompt.section({ name, order: 1, text: `scoped ${name}` }) - } + scope.ctx.systemPrompt.section({ name: 'reserved', order: 1, text: 'scoped reserved' }) - expect(() => ctx.systemPrompt.protect({ sections: names })).toThrow(message) + expect(() => ctx.systemPrompt.section({ + name: 'reserved', order: 1, text: 'global reserved', ownerFinal: true, + })).toThrow('owner-final prompt section "reserved"') expect(renderPrompt(await ctx.systemPrompt.assemble({ scope: scopeKeyOf(scope) }))) - .toContain(`scoped ${names[0]}`) + .toContain('scoped reserved') }) }) @@ -164,13 +161,18 @@ describe('scoped assemble dispatch', () => { expect(shaped).toHaveLength(1) }) - it('a scoped protection finalizes only its own assemblies and disappears with the scope', async () => { + it('scoped owner-final contributions finalize only their assemblies and disappear with the scope', async () => { const ctx = await mount() const scope = await mintScope(ctx, 'child') const key = scopeKeyOf(scope) ctx.systemPrompt.section({ name: 'required', order: 10, text: 'required' }) ctx.systemPrompt.tools(() => ({ schemas: [schema('required')] })) - scope.ctx.systemPrompt.protect({ sections: ['required'], tools: ['required'] }) + scope.ctx.systemPrompt.section({ + name: 'required', order: 10, text: 'scoped required', ownerFinal: true, + }) + scope.ctx.systemPrompt.tools(() => ({ + schemas: [schema('required')], ownerFinalNames: ['required'], + })) ctx.on('system-prompt/assemble', async (_assembly, _context, next) => { const result = await next() result.sections = result.sections.filter(section => section.name !== 'required') diff --git a/packages/core/system-prompt/tests/system-prompt.spec.ts b/packages/core/system-prompt/tests/system-prompt.spec.ts index c37ddd9c6c..3417364613 100644 --- a/packages/core/system-prompt/tests/system-prompt.spec.ts +++ b/packages/core/system-prompt/tests/system-prompt.spec.ts @@ -111,86 +111,14 @@ describe('SystemPrompt', () => { expect(contributed(assembly).map(s => s.text)).toEqual(['first']) }) - it('rejects malformed fixed registration fields before storing an effect', async () => { + it('rejects a non-finite section order', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) - const badName = { value: 'name' } - const badText = { value: 'text' } - - expect(() => ctx.systemPrompt.section(null as unknown as Parameters[0])) - .toThrow('requires a section object') - expect(() => ctx.systemPrompt.section(1 as unknown as Parameters[0])) - .toThrow('requires a section object') - expect(() => ctx.systemPrompt.section({ name: badName as unknown as string, order: 1, text: 'x' })) - .toThrow('prompt section name must be a string') - expect(() => ctx.systemPrompt.section({ name: 'bad-order', order: '1' as unknown as number, text: 'x' })) - .toThrow('order must be a finite number') expect(() => ctx.systemPrompt.section({ name: 'bad-order', order: Number.NaN, text: 'x' })) .toThrow('order must be a finite number') - expect(() => ctx.systemPrompt.section({ name: 'bad-text', order: 1, text: badText as unknown as string })) - .toThrow('text must be a string or function') - expect(() => ctx.systemPrompt.tools(1 as unknown as Parameters[0])) - .toThrow('tool provider must be a function') - expect(() => ctx.systemPrompt.variable({} as unknown as string, () => 'x')) - .toThrow('prompt variable name must be a string') - expect(() => ctx.systemPrompt.variable('valid', 1 as unknown as Parameters[1])) - .toThrow('provider must be a function') - expect(() => ctx.systemPrompt.protect(null as unknown as Parameters[0])) - .toThrow('requires a protection object') - expect(() => ctx.systemPrompt.protect(1 as unknown as Parameters[0])) - .toThrow('requires a protection object') - expect(() => ctx.systemPrompt.protect({ sections: 'x' as unknown as string[] })) - .toThrow('sections must be an array of strings') - expect(() => ctx.systemPrompt.protect({ tools: 'x' as unknown as string[] })) - .toThrow('tools must be an array of strings') - expect(() => ctx.systemPrompt.protect({ sections: ['ok', {} as unknown as string] })) - .toThrow('sections must be an array of strings') - expect(() => ctx.systemPrompt.protect({ tools: [{} as unknown as string] })) - .toThrow('tools must be an array of strings') - - expect(Object.isFrozen(badName)).toBe(false) - expect(Object.isFrozen(badText)).toBe(false) expect(contributed(await ctx.systemPrompt.assemble())).toEqual([]) }) - it('reads each section field and protection-name slot once at registration', async () => { - const ctx = new Context() - await ctx.plugin(SystemPrompt) - const reads = { name: 0, order: 0, text: 0, sections: 0, item: 0 } - const section = Object.defineProperties({}, { - name: { - enumerable: true, - get: () => (++reads.name === 1 ? 'stable' : 42), - }, - order: { - enumerable: true, - get: () => (++reads.order === 1 ? 10 : Number.NaN), - }, - text: { - enumerable: true, - get: () => (++reads.text === 1 ? 'stable text' : null), - }, - }) as unknown as Parameters[0] - const names = new Array(1) - Object.defineProperty(names, 0, { - enumerable: true, - get: () => (++reads.item === 1 ? 'stable' : 'drifted'), - }) - const protection = { - get sections(): string[] { - reads.sections += 1 - return reads.sections === 1 ? names : ['drifted'] - }, - } - - ctx.systemPrompt.section(section) - ctx.systemPrompt.protect(protection) - const assembly = await ctx.systemPrompt.assemble() - - expect(reads).toEqual({ name: 1, order: 1, text: 1, sections: 1, item: 1 }) - expect(assembly.sections).toContainEqual({ name: 'stable', order: 10, text: 'stable text' }) - }) - it('rolls back a section when a system-prompt/change listener throws (P1-1)', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) @@ -285,28 +213,21 @@ describe('SystemPrompt', () => { expect(assembly.sections).toHaveLength(0) }) - describe('canonical contribution protection', () => { - it('restores exact protected definitions after every listener, in canonical relative order', async () => { + describe('owner-final contributions', () => { + it('restores exact owner-final definitions after every listener, in canonical relative order', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) ctx.systemPrompt.section({ name: 'before', order: 10, text: 'before' }) - ctx.systemPrompt.section({ name: 'protected', order: 20, text: 'canonical section' }) + ctx.systemPrompt.section({ name: 'protected', order: 20, text: 'canonical section', ownerFinal: true }) ctx.systemPrompt.section({ name: 'after', order: 30, text: 'after' }) ctx.systemPrompt.tools(() => ({ schemas: [ { name: 'alpha', description: 'alpha', parameters: {} }, { name: 'protected', description: 'canonical tool', parameters: { type: 'object', properties: { answer: { type: 'number' } } } }, { name: 'zulu', description: 'zulu', parameters: {} }, - ] })) - const protection = { sections: ['protected'], tools: ['protected'] } - ctx.systemPrompt.protect(protection) - // Registration snapshots its arrays; caller mutation cannot change what - // the service makes authoritative. - protection.sections[0] = 'after' - protection.tools[0] = 'zulu' + ], ownerFinalNames: ['protected'] })) - // Registered AFTER the protection and prepended: it is outside every - // ordinary listener that existed when protect() ran, but service-level - // finalization still restores the canonical entries after it returns. + // Service-level finalization restores the canonical entries after the + // complete listener chain returns. ctx.on('system-prompt/assemble', async (_assembly, _context, next) => { const result = await next() return Object.freeze({ @@ -338,121 +259,18 @@ describe('SystemPrompt', () => { expect(assembly.tools.map(tool => tool.name)).toEqual(['alpha', 'protected', 'zulu']) }) - it('reads protection accessors once so the checked names are the protected names', async () => { + it('makes an owner-final tool\'s canonical absence survive the waterfall', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt) - ctx.systemPrompt.section({ name: 'protected', order: 10, text: 'canonical' }) - let reads = 0 - const protection = { - get sections(): string[] { - reads += 1 - return reads === 1 ? ['protected'] : undefined as unknown as string[] - }, - } - ctx.systemPrompt.protect(protection) + ctx.systemPrompt.tools(() => ({ schemas: [], ownerFinalNames: ['mode-hidden'] })) ctx.on('system-prompt/assemble', async (_assembly, _context, next) => { const result = await next() - result.sections = result.sections.filter(section => section.name !== 'protected') - return result - }) - - const assembly = await ctx.systemPrompt.assemble() - - expect(reads).toBe(1) - expect(assembly.sections).toContainEqual({ name: 'protected', order: 10, text: 'canonical' }) - }) - - it('materializes waterfall entry names once before restoring protected definitions', async () => { - const ctx = new Context() - await ctx.plugin(SystemPrompt) - ctx.systemPrompt.section({ name: 'protected', order: 10, text: 'canonical section' }) - ctx.systemPrompt.tools(() => ({ schemas: [{ name: 'protected', description: 'canonical tool', parameters: {} }] })) - ctx.systemPrompt.protect({ sections: ['protected'], tools: ['protected'] }) - let sectionNameReads = 0 - let toolNameReads = 0 - const hostileSection = { - get name(): string { - sectionNameReads += 1 - return sectionNameReads === 1 ? 'impostor-section' : 'protected' - }, - order: 999, - text: 'listener section', - } - const hostileTool = { - get name(): string { - toolNameReads += 1 - return toolNameReads === 1 ? 'impostor-tool' : 'protected' - }, - description: 'listener tool', - parameters: {}, - } - ctx.on('system-prompt/assemble', async (_assembly, _context, next) => { - const result = await next() - result.sections = [ - ...result.sections.filter(section => section.name !== 'protected'), - hostileSection, - ] - result.tools = [ - ...result.tools.filter(tool => tool.name !== 'protected'), - hostileTool, - ] - return result - }) - - const assembly = await ctx.systemPrompt.assemble() - - expect(sectionNameReads).toBe(1) - expect(toolNameReads).toBe(1) - expect(assembly.sections.map(section => section.name)).toEqual([ - 'harness:identity', - 'deployment:persona', - 'impostor-section', - 'protected', - ]) - expect(assembly.tools.map(tool => tool.name)).toEqual(['impostor-tool', 'protected']) - }) - - it('protects canonical absence and rejects an empty protection', async () => { - const ctx = new Context() - await ctx.plugin(SystemPrompt) - // Separate registrations exercise the set-union contract: protections - // may name only sections or only tools and still compose. - ctx.systemPrompt.protect({ sections: ['mode-hidden'] }) - ctx.systemPrompt.protect({ tools: ['mode-hidden'] }) - ctx.on('system-prompt/assemble', async (_assembly, _context, next) => { - const result = await next() - result.sections.push({ name: 'mode-hidden', order: 100, text: 'fabricated' }) result.tools.push({ name: 'mode-hidden', description: 'fabricated', parameters: {} }) return result }) const assembly = await ctx.systemPrompt.assemble() - expect(assembly.sections.some(section => section.name === 'mode-hidden')).toBe(false) expect(assembly.tools.some(tool => tool.name === 'mode-hidden')).toBe(false) - expect(() => ctx.systemPrompt.protect({})).toThrow(/at least one section or tool name/) - expect(() => ctx.systemPrompt.protect({ sections: [], tools: [] })).toThrow(/at least one section or tool name/) - }) - - it('removes a protection with its contributing fiber (HMR safety)', async () => { - const ctx = new Context() - await ctx.plugin(SystemPrompt) - ctx.systemPrompt.section({ name: 'protected', order: 10, text: 'canonical' }) - ctx.on('system-prompt/assemble', async (_assembly, _context, next) => { - const result = await next() - result.sections = result.sections.filter(section => section.name !== 'protected') - return result - }) - let changes = 0 - ctx.on('system-prompt/change', () => { changes++ }) - const fiber = await ctx.plugin(Object.assign((inner: Context) => { - inner.systemPrompt.protect({ sections: ['protected'] }) - }, { inject: ['systemPrompt'] })) - - expect((await ctx.systemPrompt.assemble()).sections.some(section => section.name === 'protected')).toBe(true) - expect(changes).toBe(1) - await fiber.dispose() - expect((await ctx.systemPrompt.assemble()).sections.some(section => section.name === 'protected')).toBe(false) - expect(changes).toBe(2) }) }) diff --git a/packages/core/system-prompt/tests/tool-order.spec.ts b/packages/core/system-prompt/tests/tool-order.spec.ts index 088c6b70ed..16eff6e354 100644 --- a/packages/core/system-prompt/tests/tool-order.spec.ts +++ b/packages/core/system-prompt/tests/tool-order.spec.ts @@ -48,100 +48,6 @@ describe('SystemPrompt tool order', () => { expect(names(await ctx.systemPrompt.assemble())).toEqual(['todo_write', 'echo_a', 'echo_b', 'bash']) }) - it('reads provider schemas once so toolOrder validates the model-visible collection', async () => { - const ctx = await mount({ toolOrder: ['actual', TOOL_ORDER_REST] }) - let reads = 0 - ctx.systemPrompt.tools(() => ({ - get schemas(): ToolSchema[] { - reads += 1 - return reads === 1 ? [tool('actual')] : [tool('phantom')] - }, - })) - - const assembly = await ctx.systemPrompt.assemble() - - expect(reads).toBe(1) - expect(names(assembly)).toEqual(['actual']) - }) - - it('reads each provider schema field once before detaching it', async () => { - const ctx = await mount() - const accepted = { type: 'object', properties: { accepted: { type: 'string' } } } - let reads = 0 - const schema = { - name: 'stable', - description: 'stable', - get parameters(): object { - reads += 1 - return reads === 1 ? accepted : { type: 'object', properties: { drifted: { type: 'number' } } } - }, - } as ToolSchema - ctx.systemPrompt.tools(() => ({ schemas: [schema] })) - - const assembly = await ctx.systemPrompt.assemble() - - expect(reads).toBe(1) - expect(assembly.tools[0]?.parameters).toEqual(accepted) - }) - - it('rejects exotic provider parameters before model-visible assembly', async () => { - const ctx = await mount() - class ExoticParameters { - readonly type = 'object' - readonly properties = { value: { type: 'string' } } - } - ctx.systemPrompt.tools(() => ({ - schemas: [{ - name: 'exotic', - description: 'must not be sanitized', - parameters: new ExoticParameters() as unknown as ToolSchema['parameters'], - }], - })) - - await expect(ctx.systemPrompt.assemble()) - .rejects.toThrow(/parameters must be losslessly JSON-serializable/) - }) - - it('rejects malformed fixed provider fields without freezing caller objects', async () => { - const ctx = await mount() - const badName = { value: 'object-name' } - const badDescription = { value: 'object-description' } - ctx.systemPrompt.tools(() => ({ - schemas: [{ - name: badName as unknown as string, - description: 'bad name', - parameters: {}, - }], - })) - await expect(ctx.systemPrompt.assemble()).rejects.toThrow('name must be a string') - expect(Object.isFrozen(badName)).toBe(false) - - const descriptions = await mount() - descriptions.systemPrompt.tools(() => ({ - schemas: [{ - name: 'bad-description', - description: badDescription as unknown as string, - parameters: {}, - }], - })) - await expect(descriptions.systemPrompt.assemble()).rejects.toThrow('description must be a string') - expect(Object.isFrozen(badDescription)).toBe(false) - - const knownNames = await mount() - knownNames.systemPrompt.tools(() => ({ - schemas: [tool('valid')], - knownNames: [{} as unknown as string], - })) - await expect(knownNames.systemPrompt.assemble()).rejects.toThrow('knownNames must be an array of strings') - - const nonArrayKnownNames = await mount() - nonArrayKnownNames.systemPrompt.tools(() => ({ - schemas: [tool('valid')], - knownNames: 'valid' as unknown as string[], - })) - await expect(nonArrayKnownNames.systemPrompt.assemble()).rejects.toThrow('knownNames must be an array of strings') - }) - it('rejects the assembly when toolOrder names a tool that is not registered (misconfiguration blocks work)', async () => { const ctx = await mount({ toolOrder: ['todo_write', 'ghost', TOOL_ORDER_REST, 'wraith'] }) ctx.systemPrompt.tools(() => ({ schemas: [tool('bash'), tool('todo_write')] })) diff --git a/packages/core/system-prompt/tsconfig.json b/packages/core/system-prompt/tsconfig.json index a66ece4854..91e7bf1ba4 100644 --- a/packages/core/system-prompt/tsconfig.json +++ b/packages/core/system-prompt/tsconfig.json @@ -22,9 +22,6 @@ }, { "path": "../../core/scope" - }, - { - "path": "../../core/session" } ] } diff --git a/packages/core/tools/README.md b/packages/core/tools/README.md index 39874a41bd..a50e35a4b4 100644 --- a/packages/core/tools/README.md +++ b/packages/core/tools/README.md @@ -11,18 +11,16 @@ tools: mode: native # native (default) | code | both ``` -`native` contributes the calling agent's visible end capabilities as wire function definitions. Under `code`, this registry's canonical contribution is the reserved `run_code` transport plus the generated `tools:sdk` prompt section (see [Code Mode](#code-mode)); an assembly listener may still deliberately add unrelated schemas. `both` contributes the visible native definitions, `run_code`, and the SDK section. In non-native modes both infrastructure pieces are protected rather than filterable capabilities: restrictions and assembly listeners cannot remove them, a scoped section cannot shadow the globally protected `tools:sdk`, and registering, shadowing, or explicitly filtering `run_code` fails loudly. Non-native modes require a loaded `ctx.codeRuntime` with `language: 'typescript'`; a missing or mismatched runtime rejects every prompt assembly with an actionable error, and a `systemPrompt.toolOrder` naming tools the mode no longer contributes rejects the assembly the same way. +`native` contributes the calling agent's visible end capabilities as wire function definitions. Under `code`, this registry's canonical contribution is the reserved `run_code` transport plus the generated `tools:sdk` prompt section (see [Code Mode](#code-mode)); an assembly listener may still deliberately add unrelated schemas. `both` contributes the visible native definitions, `run_code`, and the SDK section. In non-native modes both infrastructure pieces are owner-final rather than filterable capabilities: restrictions and assembly listeners cannot remove them, a scoped section cannot shadow the globally owner-final `tools:sdk`, and registering, shadowing, or explicitly filtering `run_code` fails loudly. Non-native modes require a loaded `ctx.codeRuntime` with `language: 'typescript'`; a missing or mismatched runtime rejects every prompt assembly with an actionable error, and a `systemPrompt.toolOrder` naming tools the mode no longer contributes rejects the assembly the same way. ### Public API -- `ctx.tools.register(definition: ToolDefinition): () => Promise | void` Register a tool as a frozen snapshot. Every top-level caller field is read once into one coherent acceptance record; `name`/`description` must be strings and `timeoutMs`, when present, must be positive and finite before the snapshot can own them. Parameters are validated and detached by one recursive lossless-JSON traversal, so a stateful getter cannot show one value to a check and another to a prototype-erasing clone. Execute/presentation callbacks are bound once to the original definition as their method receiver, so later caller mutation cannot change the executable definition. The layer is the CALLING context's scope (`dsh-scope`): a plain plugin context registers globally; an agent's `agent.ctx` registers for that agent alone, SHADOWING a same-named global tool there (per-agent tool variants). Duplicate names within one layer throw; non-native modes also reject the reserved `run_code` transport name. Disposed with the calling fiber (= the agent, for scoped registrations). -- `ctx.tools.restrict(filter: ToolRestriction): () => Promise | void` Scoped-only (throws on a plain context): mask the GLOBAL end-capability surface for the calling agent — `allow` keeps only the listed tools, `deny` removes them; multiple restrictions intersect; scope-local registrations are merged after the global filter. The registry reads `allow`/`deny` once, so the values checked for an empty filter and unknown names are exactly the values enforced. A deny-list admits a later global tool unless it names that tool; an allow-list excludes later names; neither filters a later scope-local registration. The reserved `run_code` transport remains available automatically and cannot be named explicitly. Filter-value snapshot at registration, loud unknown-name validation, `restrict({})` rejects (the materialized-empty-config trap). This is live registration composition, not a parent-derived authority ceiling; see the [agent-scope security non-goal](../../../docs/rfc/implemented/architecture/2026-07-08-agent-scope-contexts.md#security-and-authority-are-explicit-non-goals). -- `ctx.tools.get(name: string, scope?: ScopeKey): ToolDefinition | undefined` Resolution as one scope sees it (shadowing applied; a restricted-away global reads as absent) — presenters pass the calling agent so the card matches what executed. Returned definitions are the registry's frozen snapshots. -- `ctx.tools.visible(scope?: ScopeKey): ToolDefinition[]` The canonical executable view — restricted global layer ∪ the scope's own layer, plus the reserved transport in non-native modes — feeding prompt assembly, `get`, and `execute`, so presentation and dispatch resolve the same frozen definitions. -- `ctx.tools.knownNames(scope?: ScopeKey): string[]` The PRE-restriction end-capability name universe `restrict` validates against: a typo fails loud while a restricted-away tool stays a normal absence. Presentation providers add reserved transport names separately when validating `toolOrder`. +- `ctx.tools.register(definition: ToolDefinition): () => Promise | void` Register a trusted typed same-process definition. The layer is the calling context's scope: a plain plugin context registers globally; an agent's `agent.ctx` registers for that agent alone, shadowing a same-named global tool there. Duplicate names within one layer throw; non-native modes also reject the reserved `run_code` transport name. `timeoutMs`, when present, must be positive and finite. `ownerFinal: true` makes the tool's canonical wire presence or absence survive prompt assembly listeners. Disposed with the calling fiber. +- `ctx.tools.restrict(filter: ToolRestriction): () => Promise | void` Scoped-only (throws on a plain context): mask the global end-capability surface for the calling agent — `allow` keeps only the listed global tools, `deny` removes them; multiple restrictions intersect; scope-local registrations are merged afterward. The readonly arrays compile once into private sets. Every listed name must exist in the current pre-restriction global registry; scope-local, unknown, and reserved `run_code` names fail loudly. A deny-list admits a later global tool unless it names that tool; an allow-list excludes later names; neither filters a later scope-local registration. `restrict({})` rejects. This is live registration composition, not a parent-derived authority ceiling; see the [agent-scope security non-goal](../../../docs/rfc/implemented/architecture/2026-07-08-agent-scope-contexts.md#security-and-authority-are-explicit-non-goals). +- `ctx.tools.get(name: string, scope?: ScopeKey): ToolDefinition | undefined` Resolution as one scope sees it (shadowing applied; a restricted-away global reads as absent) — presenters pass the calling agent so the card matches what executed. - `ctx.tools.schemas(scope?: ScopeKey): ToolSchema[]` Schemas of everything the scope can see (without the `execute` functions). The shipped tools' schemas are catalogued in [docs/tool-catalog.md](../../../docs/tool-catalog.md), generated by booting each tool plugin and harvesting this method (see [the tool-schema-catalog RFC](../../../docs/rfc/implemented/process/2026-07-02-tool-schema-catalog.md)). - `ctx.tools.guard(guard: ToolGuard): () => Promise | void` Register a monotonic synchronous execution guard after `tools/pre-execute`: returning a reason denies the call, while `undefined` leaves it unchanged. A plain-context guard applies globally; an `agent.ctx` guard applies only to that agent. Later waterfall listeners cannot turn a guard denial back into permission. Disposed with the calling fiber. -- `ctx.tools.execute(exec: ToolExecutionInput): Promise` Read each caller-owned top-level field once, require `callId` and `name` to yield strings, snapshot the single-use call into a pipeline-owned execution, assign its opaque correlation token, materialize `arguments` through one lossless-JSON traversal, deep-freeze them, and protect identity before running `tools/pre-execute` → guards → `tools/execute` → `tools/post-execute`; optional `signal` is the only operational field an around-dispatch wrapper may add, replace, or remove. After the required string correlation identity is captured, the same captured optional fields build the normalized error shell if a later accessor or validation fails, so policy, dispatch, routing, and `tools/result` cannot observe different caller values. Every top-level result field is likewise captured once and the complete result or post-decision is losslessly materialized before final observation. Invalid later input—including cloneable mutable exotics—and malformed or non-JSON listener/tool results normalize to `isError` outcomes rather than bypassing policy or failing later at the session log. A throwing accessor or non-string value in `callId` or `name` rejects before `tools/result` because no trustworthy result identity exists yet. +- `ctx.tools.execute(exec: ToolExecutionInput): Promise` Assign a fresh opaque correlation token, losslessly materialize and deep-freeze arguments once at the model/tool boundary, then run the call through `tools/pre-execute` → guards → `tools/execute` → `tools/post-execute`. Invalid arguments normalize through the same authoritative result path without reaching policy or the body. The final outcome is independently materialized and deep-frozen once before `tools/result`. Optional `signal` remains the operational field an around-dispatch wrapper may replace. ### Injected services @@ -34,12 +32,12 @@ The live registry pipeline has three transformable waterfalls followed by the ow ### Key types -- `ToolDefinition` — `ToolSchema` + `execute(args, exec): Promise` (the bare array is the model-facing content; the object form additionally attaches an opaque, JSON-serializable `meta` presentation payload persisted on the `tool/result` event and handed back to `presentResult`), plus optional `presentCall(args)` / `presentResult(args, result)` for tool-owned UI presentation (see below). It also carries an optional cooperative timeout budget `timeoutMs?: number` (ms) enforced by `@deepseek-ai/dsh-timeout-policy`, never sent to the model. Registration stores a frozen snapshot with detached JSON parameters and once-bound callback identities. -- `ToolExecutionInput` — the caller-supplied call description: `{ callId, name, arguments, agent?, parent?, signal? }`; `callId` and `name` must be strings, `arguments` must be losslessly JSON-serializable, and callers may pass an enclosing execution's opaque token as `parent` but never choose the new execution's own token. -- `ToolExecutionToken` — a frozen, property-free identity value assigned by the registry. It supports equality correlation only and exposes no live outer execution state. +- `ToolDefinition` — `ToolSchema` + `execute(args, exec)`, optional presentation callbacks, cooperative `timeoutMs`, and optional `ownerFinal`. `ownerFinal` is reserved for protocol tools such as `run_code` and structured-output capture whose canonical wire state must survive assembly listeners. +- `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. - `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 validates 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 (the loop forwards it onto the `tool/result` session event for retry/sandbox plugins and replay). `additionalContext` (a `HookContext`) ferries any `tools/post-execute` context up to the loop, which buffers it and appends it as a `context/message` after all `tool/result`s in the step. `meta` is the tool's opaque presentation payload from a successful `execute` (the object return form); the loop forwards it onto the `tool/result` session event for result-card rendering. -- `PreToolDecision` — `{kind:'allow'}` | `{kind:'deny', reason}` | `{kind:'ask', reason?}`. The registry validates this exact union at runtime: a JavaScript/casted value with an unknown kind, a malformed reason, or extra fields fails closed as an `isError`; the tool body does not run and final observers still receive one result. Input rewrite (changing `arguments`) is deliberately NOT offered (it would desync the pre-execution audit/history/UI from what ran — its own proposed RFC); `ask` is serviced by [`ctx.approval`](../../ui/user-approval/README.md) when a deployment mounts it (`allowed-once` proceeds to dispatch; `rejected`/`cancelled`/`unavailable` deny with distinct reasons) and degrades to `deny` when none is mounted or the execution carries no agent. +- `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. +- `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. - `ToolGuard` — `(execution) => string | undefined`; the returned string is a final monotonic denial reason evaluated after the reorderable pre-execute waterfall and before dispatch. - `ToolCallView` / `ToolResultView` — provider-neutral `card`-tagged render intents a tool returns from `presentCall` / `presentResult` to own how a UI renders ITS calls (see "Tool-owned UI presentation"). @@ -77,7 +75,7 @@ ctx.tools.register(defineTool({ })) ``` -The helper converts the author-facing `SchemaSpec` (with `required: true` as a per-property boolean) to standard JSON Schema for the wire format. Definition is a snapshot boundary: `defineTool` reads every top-level option once, detaches the schema, and derives both an independent wire schema and every later execute/presentation validation from that accepted snapshot. Stateful accessors or later caller mutation therefore cannot make the schema shown to the model disagree with the schema enforced at runtime. Raw JSON-Schema tool definitions (from MCP servers) are still accepted by the registry directly. +The helper converts the author-facing `SchemaSpec` (with `required: true` as a per-property boolean) to standard JSON Schema for the wire format and uses the same typed spec for execute/presentation validation. Raw JSON-Schema tool definitions (from MCP servers) are still accepted by the registry directly. A `defineTool` tool also **validates the model-generated arguments against its `SchemaSpec` before `execute` runs** (`validateArgs`). The model's JSON is untrusted — `InferArgs` is a compile-time claim, not a runtime guarantee — so on a mismatch (missing required key, wrong primitive, bad enum member, nested violation) the tool throws a `ToolArgsError` (`code: 'INVALID_ARGS'`); the registry turns it into an `isError` result whose text lists the violations, which the model sees and self-corrects from. Validation mirrors the JSON Schema conversion exactly: extra keys are allowed, `default` is not applied, and an `object`/`array` prop without `properties`/`items` only type-checks. Raw-registered tools (MCP) are **not** validated by the harness — they validate their own input. @@ -138,7 +136,7 @@ Under `mode: code` (or `both`) the registry turns the tool surface into a progra - **The dispatch bridge** (`run_code`'s execute): every binding call is JSON-normalized before dispatch (a value that does not survive — `BigInt`, circulars — rejects that one call, so the dispatched form and logged form are the same JSON value by construction), serialized through a per-run queue (even `Promise.all` executes underlying calls one at a time in submission order), given the outer execution's opaque token as `parent`, and run through the complete pre-execute → guards → execute → post-execute → result pipeline. A denial reaches the program as a binding rejection, and each sub-call is logged as a `tool/code-dispatch` session event with deterministic id `:code:`; `deriveMessages()` does not surface that event. Token correlation lets commit-style observers defer an inner success until the final `run_code` result without exposing the live outer execution; ordinary tool side effects are not rolled back. A sub-call's `additionalContext` is deliberately dropped because inserting it inside a running parent call would break tool-call/result adjacency. - **Settlement discipline**: the bridge owns a run-scoped abort that follows the outer signal in and fires when the run settles for any reason, so a budget expiry aborts an in-flight sub-tool instead of orphaning it; the bridge then drains its queue BEFORE returning, so every `tool/code-dispatch` lands inside the open turn. A failed run throws `CodeRunFailedError` (`code: 'CODE_RUN_FAILED'`, message = the failure kind + captured logs), which the pipeline converts to a structured `isError` the model self-corrects from. -The wire collapse is the registry's own contribution (`systemPrompt.tools()` is mode-aware), so the logged `request/header` records it for free. With no deliberate schema-adding assembly listener, `code` assembles exactly `[run_code]`, pinned by tests and the snapshot goldens; protection guarantees that `run_code` and `tools:sdk` remain present, not that unrelated listener additions are erased. Try it: `pnpm run demo:code-mode` ([the coding-agent example's Code Mode overlay](../../../examples/coding-agent/README.md#code-mode)); `pnpm run demo:code-mode acp` serves the same mode over ACP instead of the REPL. +The wire collapse is the registry's own contribution (`systemPrompt.tools()` is mode-aware), so the logged `request/header` records it for free. With no deliberate schema-adding assembly listener, `code` assembles exactly `[run_code]`, pinned by tests and the snapshot goldens; contribution-owned finality guarantees that `run_code` and `tools:sdk` remain present, not that unrelated listener additions are erased. Try it: `pnpm run demo:code-mode` ([the coding-agent example's Code Mode overlay](../../../examples/coding-agent/README.md#code-mode)); `pnpm run demo:code-mode acp` serves the same mode over ACP instead of the REPL. ### What is NOT here (TODO) diff --git a/packages/core/tools/src/code-mode.ts b/packages/core/tools/src/code-mode.ts index 323fbeed2b..702a403b03 100644 --- a/packages/core/tools/src/code-mode.ts +++ b/packages/core/tools/src/code-mode.ts @@ -151,6 +151,7 @@ function asRunCodeMeta(meta: unknown): RunCodeMeta | undefined { export function createRunCodeTool(registry: ToolRegistry, requireRuntime: () => CodeRuntime): ToolDefinition { return defineTool({ name: RUN_CODE_NAME, + ownerFinal: true, description: 'Execute a TypeScript program against the available tools. Write the BODY of an ' + 'async function (erasable syntax only; top-level `await` and `return` work) and ' diff --git a/packages/core/tools/src/index.ts b/packages/core/tools/src/index.ts index 4dd59efb8c..3365662458 100644 --- a/packages/core/tools/src/index.ts +++ b/packages/core/tools/src/index.ts @@ -91,9 +91,6 @@ declare module 'cordis' { * tool body never runs. Input rewrite is deliberately NOT offered here (see * {@link PreToolDecision}); `ask` is serviced by the `ctx.approval` seam * when one is mounted, and degrades to deny otherwise. - * The returned union is validated as an exact runtime shape before approval - * or guards run; a malformed JavaScript/casted decision fails closed as an - * `isError` result and the tool body never runs. * Scope-filtered dispatch (`@deepseek-ai/dsh-scope`) keys the carrier by `exec.agent`: a * listener registered through `agent.ctx` fires only for that agent's * calls, while a plain plugin listener fires for every call (including @@ -148,7 +145,7 @@ declare module 'cordis' { * @param result - the dispatch outcome a listener may accept, replace, or block. * @mode waterfall */ - 'tools/post-execute'(this: Scoped, exec: ToolExecution, result: ToolExecutionResult, next: () => Promise): Promise + 'tools/post-execute'(this: Scoped, exec: ToolExecution, result: Readonly, next: () => Promise): Promise /** * Awaited notification of the authoritative FINAL tool outcome, after the * complete pre/execute/post pipeline, final lossless-JSON validation, and @@ -202,6 +199,12 @@ export interface ToolDefinition extends ToolSchema { * cooperative implementation that can reach quiescence when the signal aborts. */ timeoutMs?: number + /** + * Whether this tool name's canonical wire presence or absence survives the + * complete system-prompt assembly waterfall. Reserved for protocol tools + * whose owner must retain the final definition. + */ + readonly ownerFinal?: boolean /** * Optional: how to present the PENDING state of one call in a UI, derived from * the call's `args` (parsed arguments, `unknown` — the tool validates/narrows @@ -239,23 +242,18 @@ export interface ToolResult { declare const toolExecutionTokenBrand: unique symbol -/** Tokens minted in this module; a cast or JavaScript object cannot forge membership. */ -const executionTokens = new WeakSet() - /** - * Opaque, immutable identity for one trip through the tool pipeline. Nested + * Opaque identity for one trip through the tool pipeline. Nested * transports carry the enclosing execution's token instead of its live object, * so observe-only result listeners can correlate calls without gaining a * mutation path into an outer around-dispatch wrapper. */ -export interface ToolExecutionToken { - readonly [toolExecutionTokenBrand]: true -} +export type ToolExecutionToken = symbol & { readonly [toolExecutionTokenBrand]: true } /** * Caller-supplied description of one tool call. {@link ToolRegistry.execute} - * snapshots this input into a pipeline-owned {@link ToolExecution}; callers do - * not choose the execution token. + * adds the registry-owned token to form a pipeline {@link ToolExecution}; + * callers do not choose that token. */ export interface ToolExecutionInput { readonly callId: CallId @@ -274,11 +272,11 @@ export interface ToolExecutionInput { } /** - * One pending tool call inside the registry pipeline. Call identity, the - * registry-assigned {@link token}, and a deep-frozen lossless-JSON snapshot of - * the parsed arguments are immutable from the first policy listener onward, - * while an around-dispatch wrapper may set, replace, or remove only `signal`. - * The registry freezes the complete object before `tools/result` observers run. + * One pending tool call inside the registry pipeline. Parsed arguments cross + * one lossless-JSON materialization boundary before policy and are deep-frozen; + * call identity and the registry-assigned {@link token} are readonly. An + * around-dispatch wrapper may set, replace, or remove `signal`. The registry + * freezes the complete object before `tools/result` observers run. */ export interface ToolExecution extends ToolExecutionInput { /** Registry-assigned identity shared with nested calls only as their opaque `parent` token. */ @@ -430,8 +428,8 @@ export interface Config { * `deny` removes the listed ones; both present = allow first, then deny. * Restrictions never touch scoped registrations — a tool registered through * the same scope is merged after the global filter (which is what keeps e.g. a - * structured-output capture tool alive under an allow-list). The filter values - * are snapshotted at registration, but resolution uses the live global registry: + * structured-output capture tool alive under an allow-list). The readonly + * filter values compile to private sets at registration, but resolution uses the live global registry: * a later global name passes a deny-only filter unless explicitly denied and * fails an allow-list unless explicitly allowed. The * reserved `run_code` presentation transport is likewise outside capability @@ -440,9 +438,27 @@ export interface Config { */ export interface ToolRestriction { /** Global tool names that stay visible; everything else is removed. */ - allow?: string[] + readonly allow?: readonly string[] /** Global tool names removed from visibility. */ - deny?: string[] + readonly deny?: readonly string[] +} + +/** One restriction compiled at registration for repeated live-global lookup. */ +interface CompiledToolRestriction { + readonly allow?: ReadonlySet + readonly deny?: ReadonlySet +} + +/** One scope's complete registry view, derived in a single layer traversal. */ +interface ToolView { + /** Visible definitions after restrictions, scoped shadowing, and transport insertion. */ + readonly visible: ReadonlyMap + /** Pre-restriction capability names used by prompt-order validation. */ + readonly knownNames: ReadonlySet + /** Current global names that a scoped restriction may name. */ + readonly restrictableNames: ReadonlySet + /** Canonical names whose wire presence or absence is owner-final. */ + readonly ownerFinalNames: ReadonlySet } /** @@ -475,7 +491,7 @@ interface ToolGuardRegistration { * that agent alone, disposed with the scope, and SHADOWING a global tool of * the same name for that agent (most-specific-wins; within one layer a * duplicate name still throws). {@link restrict} masks the global layer per - * scope. One visibility function ({@link visible}) feeds prompt assembly, + * scope. One private visibility resolver feeds prompt assembly, * {@link get}, and {@link execute} — and, under a non-native mode, the SDK * section and `run_code`'s bindings — so what the model is shown, what a * presenter renders, what a program can call, and what dispatches can never @@ -490,8 +506,8 @@ export class ToolRegistry extends Service { private global = new Map() private scoped = new Map>() - /** Snapshot-at-registration restriction filters, per scope (see {@link restrict}). */ - private restrictions = new Map() + /** Compiled restriction filters, per scope (see {@link restrict}). */ + private restrictions = new Map() /** Monotonic post-policy guards, split into global and per-agent layers. */ private globalGuards = new Set() private scopedGuards = new Map>() @@ -511,12 +527,13 @@ export class ToolRegistry extends Service { // the filterable global/scoped capability layers. this.codeTransport = this.mode === 'native' ? undefined - : deepFreeze(createRunCodeTool(this, () => this.requireCodeRuntime())) + : createRunCodeTool(this, () => this.requireCodeRuntime()) ctx.systemPrompt.tools(context => this.wireSchemas(context.scope)) if (this.mode !== 'native') { ctx.systemPrompt.section({ name: 'tools:sdk', order: SDK_SECTION_ORDER, + ownerFinal: true, // A lazy thunk over the live registry, per assembly CONTEXT: // regenerated at each assembly over the CALLING SCOPE's visible set // (scoped tools join, restricted globals vanish — the SDK declares @@ -529,11 +546,6 @@ export class ToolRegistry extends Service { return renderToolsSdk(this.schemas(context.scope).filter(schema => schema.name !== RUN_CODE_NAME)) }, }) - // These are presentation infrastructure, not optional end capabilities. - // Protect them at their owner: assembly listeners may still transform - // ordinary tools and prose, but cannot silently leave Code Mode without - // its only wire transport or the SDK that tells the model how to use it. - ctx.systemPrompt.protect({ sections: ['tools:sdk'], tools: [RUN_CODE_NAME] }) } } @@ -553,16 +565,24 @@ export class ToolRegistry extends Service { * `mode: 'code'` the universe is `[run_code]` and a `toolOrder` naming a * native tool is dead configuration that fails every assembly loud. Under * `mode: 'both'`, the provider adds the reserved transport to the - * capability-only {@link knownNames} universe for `toolOrder` validation. + * capability-only known-name universe for `toolOrder` validation. */ private wireSchemas(scope?: ScopeKey): ToolProviderResult { - if (this.mode === 'native') return { schemas: this.schemas(scope), knownNames: this.knownNames(scope) } - this.requireCodeRuntime() - const all = this.schemas(scope) - if (this.mode === 'code') { - return { schemas: all.filter(schema => schema.name === RUN_CODE_NAME), knownNames: [RUN_CODE_NAME] } + const view = this.view(scope) + const schemas = [...view.visible.values()].map(definition => this.schemaOf(definition, false)) + const ownerFinalNames = [...view.ownerFinalNames] + if (this.mode === 'native') { + return { schemas, knownNames: [...view.knownNames], ownerFinalNames } } - return { schemas: all, knownNames: [...this.knownNames(scope), RUN_CODE_NAME] } + this.requireCodeRuntime() + if (this.mode === 'code') { + return { + schemas: schemas.filter(schema => schema.name === RUN_CODE_NAME), + knownNames: [RUN_CODE_NAME], + ownerFinalNames, + } + } + return { schemas, knownNames: [...view.knownNames, RUN_CODE_NAME], ownerFinalNames } } /** @@ -593,13 +613,10 @@ export class ToolRegistry extends Service { * the shadowing feature, not an error; the global-duplicate message names * `agent.ctx` as the per-agent alternative), or if a non-native mode reserves * the `run_code` name for its presentation transport. The visible schema set - * flows into prompt assembly automatically. Registration materializes the JSON - * parameters in one pass, copies scalar fields, binds each callback once to the - * caller's definition as its method receiver, and freezes the stored snapshot; - * later mutation or callback replacement on the input object does not rewrite - * the registry. Every top-level field is read once into one coherent acceptance - * snapshot, so stateful accessors cannot make validation and storage use - * different values. Emits `tools/change` on register/unregister. + * flows into prompt assembly automatically. Definitions are trusted typed + * same-process contributions; JSON materialization happens when the schema or + * result reaches its model/log boundary. Emits `tools/change` on + * register/unregister. * @param definition - the tool's schema plus its execute (and optional * presentation) functions. * @returns the disposer that unregisters the tool. The exact @@ -608,78 +625,39 @@ export class ToolRegistry extends Service { */ register(definition: ToolDefinition): () => Promise | void { const scope = scopeOf(this.ctx) - // One coherent acceptance snapshot: a caller may expose fields through - // accessors, so every top-level value is read exactly once before any - // validation or binding. Checked parameters and stored parameters must be - // the same reference, and a callback cannot change between lookup/bind. const name = definition.name - const description = definition.description - const inputParameters = definition.parameters const timeoutMs = definition.timeoutMs - // eslint-disable-next-line @typescript-eslint/unbound-method - const inputExecute = definition.execute - // eslint-disable-next-line @typescript-eslint/unbound-method - const inputPresentCall = definition.presentCall - // eslint-disable-next-line @typescript-eslint/unbound-method - const inputPresentResult = definition.presentResult - // Reject malformed fixed fields before any caller-owned value can enter the - // frozen snapshot. In particular, a boxed string/object must not become a - // Map key or get recursively frozen as though it were a scalar. - if (typeof name !== 'string') throw new TypeError('tool name must be a string') - if (typeof description !== 'string') throw new TypeError(`tool "${name}" description must be a string`) if (timeoutMs !== undefined - && (typeof timeoutMs !== 'number' || !Number.isFinite(timeoutMs) || timeoutMs <= 0)) { + && (!Number.isFinite(timeoutMs) || timeoutMs <= 0)) { throw new TypeError(`tool "${name}" timeoutMs must be a positive finite number`) } - if (typeof inputExecute !== 'function') throw new TypeError(`tool "${name}" execute must be a function`) - if (inputPresentCall !== undefined && typeof inputPresentCall !== 'function') { - throw new TypeError(`tool "${name}" presentCall must be a function when provided`) - } - if (inputPresentResult !== undefined && typeof inputPresentResult !== 'function') { - throw new TypeError(`tool "${name}" presentResult must be a function when provided`) - } - const execute = inputExecute.bind(definition) - const presentCall = inputPresentCall?.bind(definition) - const presentResult = inputPresentResult?.bind(definition) - // A schema crosses the same model/log boundary as execution arguments. - // Validate and detach it in one traversal: validate-then-structuredClone - // would reread getters and could erase an exotic prototype returned only to - // the clone. A frozen Map is still mutable, so deepFreeze alone is not a - // sufficient registration boundary. - const parameters = snapshotJsonValue(inputParameters) - if (parameters === undefined) { - throw new TypeError('tool parameters must be losslessly JSON-serializable') - } - // Bind once so replacing a callback on the caller-owned definition after - // registration cannot change dispatch, while preserving the historical - // method receiver (`this === definition`) for callbacks that use it. - const snapshot: ToolDefinition = deepFreeze({ - name, - description, - parameters, - execute, - ...timeoutMs !== undefined ? { timeoutMs } : {}, - ...presentCall !== undefined ? { presentCall } : {}, - ...presentResult !== undefined ? { presentResult } : {}, - }) - if (this.codeTransport !== undefined && snapshot.name === RUN_CODE_NAME) { + if (this.codeTransport !== undefined && name === RUN_CODE_NAME) { throw new Error(`tool name "${RUN_CODE_NAME}" is reserved for the Code Mode presentation transport and cannot be registered or shadowed`) } + if (scope !== undefined && this.global.get(name)?.ownerFinal === true) { + throw new Error(`tool "${name}" is globally owner-final and cannot be shadowed in an agent scope`) + } + if (scope === undefined && definition.ownerFinal === true) { + const hasScopedShadow = [...this.scoped.values()].some(layer => layer.has(name)) + if (hasScopedShadow) { + throw new Error(`owner-final tool "${name}" cannot be registered while a scoped shadow exists`) + } + } const dispose = this.ctx.effect(function* (this: ToolRegistry) { const layer = scope === undefined ? this.global : this.layerFor(scope) - if (layer.has(snapshot.name)) { + if (layer.has(name)) { throw new Error(scope === undefined - ? `tool "${snapshot.name}" is already registered (for a per-agent variant, register through that agent's \`agent.ctx\` instead)` - : `tool "${snapshot.name}" is already registered in this scope`) + ? `tool "${name}" is already registered (for a per-agent variant, register through that agent's \`agent.ctx\` instead)` + : `tool "${name}" is already registered in this scope`) } - layer.set(snapshot.name, snapshot) + layer.set(name, definition) // Yield the rollback BEFORE emitting `tools/change`: a generator effect // collects each yielded disposer before the next step runs, so a throwing // `tools/change` listener removes the tool instead of leaking it (a leak // would wedge the duplicate-name check until restart). The duplicate // throw above fires before any mutation — it leaks nothing. yield () => { - layer.delete(snapshot.name) + layer.delete(name) // An emptied scope layer is dropped so a disposed scope leaves no // residue keyed by its (dead) key. if (scope !== undefined && layer.size === 0) this.scoped.delete(scope) @@ -701,16 +679,14 @@ export class ToolRegistry extends Service { * through a scoped context (`agent.ctx`) — restricting "everyone" is not a * thing (throw), and an empty filter (neither `allow` nor `deny`) is a no-op * that can only be a bug (throw — the materialized-empty-config trap). - * Validates every listed name against the scope's CURRENT pre-restriction - * name universe ({@link knownNames}) and throws on an unknown one (fail loud + * Validates every listed name against the CURRENT global end-capability + * universe and throws on an unknown or scope-local name (fail loud * beats a typo silently filtering nothing) — register restrictions after the * global tools they mask exist (the agent-creation `setup` window satisfies * this). A non-native mode's reserved `run_code` presentation transport is * not a filterable capability; naming it explicitly throws, while omitting - * it from an allow-list cannot remove it. `allow` and `deny` are each read - * once, then the filter VALUES are snapshotted at registration: the values - * checked are the values enforced, and later caller mutation of the arrays - * changes nothing. Resolution still uses the live global registry, so a later + * it from an allow-list cannot remove it. The readonly arrays are compiled to + * private sets at registration. Resolution still uses the live global registry, so a later * global name passes a deny-only filter unless named and fails an allow-list * unless named. Multiple restrictions compose by intersection. Scoped * registrations are merged after restrictions and therefore remain visible. @@ -726,36 +702,31 @@ export class ToolRegistry extends Service { if (scope === undefined) { throw new Error('tools.restrict() requires a scoped context (agent.ctx): a context-global restriction would mask every agent — deny the tool for the intended agent instead') } - // Read each caller-owned accessor once. The same values must decide - // whether the filter is meaningful AND become the enforced snapshot: a - // stateful getter must not pass the no-op check as `allow: []` and then - // disappear when the snapshot is built. const allow = filter.allow const deny = filter.deny if (allow === undefined && deny === undefined) { throw new Error('tools.restrict({}) is a no-op: pass `allow` and/or `deny` (an empty filter is almost always a materialized-empty-config bug)') } - // Snapshot BEFORE validation so what was checked is what is enforced. - const snapshot: ToolRestriction = { - ...allow !== undefined ? { allow: [...allow] } : {}, - ...deny !== undefined ? { deny: [...deny] } : {}, + const compiled: CompiledToolRestriction = { + ...allow !== undefined ? { allow: new Set(allow) } : {}, + ...deny !== undefined ? { deny: new Set(deny) } : {}, } if (this.codeTransport !== undefined - && [...snapshot.allow ?? [], ...snapshot.deny ?? []].includes(RUN_CODE_NAME)) { + && [...allow ?? [], ...deny ?? []].includes(RUN_CODE_NAME)) { throw new Error(`tools.restrict() cannot name reserved Code Mode presentation transport "${RUN_CODE_NAME}"; restrict end-capability tools instead`) } - const known = new Set(this.knownNames(scope)) - const unknown = [...snapshot.allow ?? [], ...snapshot.deny ?? []].filter(name => !known.has(name)) + const known = this.view(scope).restrictableNames + const unknown = [...allow ?? [], ...deny ?? []].filter(name => !known.has(name)) if (unknown.length > 0) { - throw new Error(`tools.restrict() names unknown tool${unknown.length > 1 ? 's' : ''} ${unknown.map(n => `"${n}"`).join(', ')}; known tools for this scope: ${[...known].sort().join(', ') || '(none)'}`) + throw new Error(`tools.restrict() names unknown global tool${unknown.length > 1 ? 's' : ''} ${unknown.map(n => `"${n}"`).join(', ')}; known global tools: ${[...known].sort().join(', ') || '(none)'}`) } const dispose = this.ctx.effect(function* (this: ToolRegistry) { const list = this.restrictions.get(scope) ?? [] this.restrictions.set(scope, list) - list.push(snapshot) + list.push(compiled) yield () => { - const index = list.indexOf(snapshot) - /* v8 ignore next 3 -- defensive: the snapshot was pushed, so indexOf is guaranteed >= 0 */ + const index = list.indexOf(compiled) + /* v8 ignore next 3 -- defensive: the compiled restriction was pushed, so indexOf is guaranteed >= 0 */ if (index >= 0) list.splice(index, 1) if (list.length === 0) this.restrictions.delete(scope) this.ctx.emit('tools/change') @@ -817,69 +788,68 @@ export class ToolRegistry extends Service { /** First monotonic denial from the global then matching scoped guard layers. */ private guardReason(exec: ToolExecution): string | undefined { - // Guards are policy, not another transform seam. The pipeline execution's - // identity and arguments are already protected; freeze a detached view so - // an untyped guard cannot replace the wrapper-mutable signal either. - const view: Readonly = Object.freeze({ ...exec }) for (const { guard } of this.globalGuards) { - const reason = guard(view) - if (reason !== undefined) return this.assertGuardReason(reason) + const reason = guard(exec) + if (reason !== undefined) return reason } if (exec.agent !== undefined) { for (const { guard } of this.scopedGuards.get(exec.agent) ?? []) { - const reason = guard(view) - if (reason !== undefined) return this.assertGuardReason(reason) + const reason = guard(exec) + if (reason !== undefined) return reason } } return undefined } - /** Runtime boundary for JavaScript/casted guards: only strings can deny. */ - private assertGuardReason(reason: unknown): string { - if (typeof reason !== 'string') { - throw new TypeError(`tools.guard() must return a denial string or undefined, got ${typeof reason}`) - } - return reason - } - /** Whether every restriction registered for `scope` admits the global tool `name` (intersection semantics). */ private admits(scope: ScopeKey | undefined, name: string): boolean { if (scope === undefined) return true const filters = this.restrictions.get(scope) if (!filters) return true return filters.every(filter => - (filter.allow === undefined || filter.allow.includes(name)) - && (filter.deny === undefined || !filter.deny.includes(name))) + (filter.allow === undefined || filter.allow.has(name)) + && (filter.deny === undefined || !filter.deny.has(name))) } /** - * THE visibility function — one resolution feeding prompt assembly, - * {@link get}, and {@link execute}: the global layer masked by the scope's - * restrictions, unioned with the scope's own layer, scoped shadowing global - * on a name conflict, then the non-native mode's reserved `run_code` - * presentation transport. No scope = the unrestricted global view. + * Resolve every registry fact one scope needs in one layer traversal. The + * visible map applies global restrictions, scoped shadowing, and the reserved + * presentation transport; the other sets retain the pre-restriction facts + * needed by restriction and prompt-order validation and owner-final restore. * @param scope - the viewing scope (the agent), or undefined for the global view. - * @returns the visible definitions (scoped shadows applied), in per-layer - * registration order, global layer first. + * @returns the complete derived view for that scope. */ - visible(scope?: ScopeKey): ToolDefinition[] { + private view(scope?: ScopeKey): ToolView { const layer = scope === undefined ? undefined : this.scoped.get(scope) - const result = new Map() + const visible = new Map() + const knownNames = new Set() + const restrictableNames = new Set() + const ownerFinalNames = new Set() for (const [name, definition] of this.global) { - if (this.admits(scope, name)) result.set(name, definition) + knownNames.add(name) + restrictableNames.add(name) + if (definition.ownerFinal === true) ownerFinalNames.add(name) + if (this.admits(scope, name)) visible.set(name, definition) } // Scoped layer second: same-name entries REPLACE (shadow) the global ones, // and scope-local registrations are never part of the global filter above. - for (const [name, definition] of layer ?? []) result.set(name, definition) + for (const [name, definition] of layer ?? []) { + knownNames.add(name) + if (definition.ownerFinal === true) ownerFinalNames.add(name) + visible.set(name, definition) + } // Presentation infrastructure is resolved last and outside capability // filtering. Registration rejects this reserved name, so this set is an // invariant assertion as well as protection against future layer changes. - if (this.codeTransport !== undefined) result.set(RUN_CODE_NAME, this.codeTransport) - return [...result.values()] + if (this.codeTransport !== undefined) { + visible.set(RUN_CODE_NAME, this.codeTransport) + if (this.codeTransport.ownerFinal === true) ownerFinalNames.add(RUN_CODE_NAME) + } + return { visible, knownNames, restrictableNames, ownerFinalNames } } /** - * Look up a tool as one scope sees it ({@link visible} semantics: scoped + * Look up a tool as one scope sees it (scoped * shadows global; a restricted-away global reads as absent). Presenters pass * the calling agent so the rendered card matches the definition that * actually executed. @@ -888,11 +858,7 @@ export class ToolRegistry extends Service { * @returns the definition the scope resolves, or undefined when none is visible. */ get(name: string, scope?: ScopeKey): ToolDefinition | undefined { - if (name === RUN_CODE_NAME && this.codeTransport !== undefined) return this.codeTransport - const shadowed = scope === undefined ? undefined : this.scoped.get(scope)?.get(name) - if (shadowed) return shadowed - if (!this.admits(scope, name)) return undefined - return this.global.get(name) + return this.view(scope).visible.get(name) } /** @@ -908,30 +874,17 @@ export class ToolRegistry extends Service { * @returns one deep-cloned schema per visible tool. */ schemas(scope?: ScopeKey): ToolSchema[] { - return this.visible(scope).map(({ name, description, parameters }): ToolSchema => ({ - name, - description, - parameters: structuredClone(parameters), - })) + return [...this.view(scope).visible.values()].map(definition => this.schemaOf(definition, true)) } - /** - * The PRE-restriction END-CAPABILITY name universe for `scope`: every global - * name plus the scope's own layer, ignoring restrictions. This is the set - * `restrict()` validates against, so a typo fails loud while a - * restricted-away tool remains a normal, non-erroneous absence. Reserved - * presentation transports are deliberately absent: `restrict()` rejects - * naming one, while {@link wireSchemas} adds it to the separate `toolOrder` - * validation universe when its presentation mode contributes it. - * @param scope - the viewing scope (the agent); omitted = global names only. - * @returns the known names, deduplicated. - */ - knownNames(scope?: ScopeKey): string[] { - const names = new Set(this.global.keys()) - if (scope !== undefined) { - for (const name of this.scoped.get(scope)?.keys() ?? []) names.add(name) + /** Project one definition onto the model-facing schema fields. */ + private schemaOf(definition: ToolDefinition, detachParameters: boolean): ToolSchema { + const { name, description, parameters } = definition + return { + name, + description, + parameters: detachParameters ? structuredClone(parameters) : parameters, } - return [...names] } /** @@ -952,129 +905,54 @@ export class ToolRegistry extends Service { * the final observe-only notification, the authoritative outcome is * materialized as a detached lossless-JSON snapshot; an invalid outcome is * normalized to an error. - * A malformed runtime/casted `tools/pre-execute` decision likewise normalizes - * to an error before approval, guards, or the tool body. - * Caller-owned arguments are validated and detached in one recursive - * lossless-JSON traversal; a violation normalizes to an error before policy - * or dispatch. - * @param exec - the single-use call input; every top-level field is read once. - * `callId` and `name` must each yield a string before that identity snapshot - * is protected and policy begins. - * @returns the final result after every waterfall. Once the required string - * `callId` and `name` correlation identity has been captured, later accessor, - * validation, listener, and tool failures resolve as `isError` results rather - * than rejections. A throwing accessor or non-string value in either identity - * field rejects because no trustworthy result correlation exists yet. + * @param exec - the typed same-process call input. The registry assigns its + * correlation token before policy begins. + * @returns the materialized final result after every waterfall; listener and + * tool failures resolve as `isError` results rather than rejections. */ async execute(exec: ToolExecutionInput): Promise { - // callId/name are the minimum correlation identity needed to construct a - // result at all. Capture each once, then validate the captured scalar before - // anything can treat it as a trustworthy identity. A JavaScript/casted - // caller that supplies another type rejects at this outer boundary: an error - // result carrying the same malformed value would not satisfy the correlation - // contract and might itself fail lossless-JSON materialization. Every other - // caller-controlled accessor is read once INSIDE the normalization boundary; - // if one throws, the error shell uses the fields captured before it and never - // rereads the hostile record. + const token = createExecutionToken() const callId = exec.callId const name = exec.name - if (typeof callId !== 'string') { - throw new TypeError('tool execution callId must be a string') + const agent = exec.agent + const parent = exec.parent + const signal = exec.signal + const base = { + token, + callId, + name, + ...agent !== undefined ? { agent } : {}, + ...parent !== undefined ? { parent } : {}, + ...signal !== undefined ? { signal } : {}, } - if (typeof name !== 'string') { - throw new TypeError('tool execution name must be a string') - } - let agent: Agent | undefined - let parent: ToolExecutionToken | undefined - let signal: AbortSignal | undefined let execution: ToolExecution try { - agent = exec.agent - parent = exec.parent - signal = exec.signal - const args = exec.arguments - const input: Readonly = Object.freeze({ - callId, - name, - arguments: args, - ...agent !== undefined ? { agent } : {}, - ...parent !== undefined ? { parent } : {}, - ...signal !== undefined ? { signal } : {}, - }) - execution = this.prepareExecution(input) + const detached = snapshotJsonValue(exec.arguments) + if (detached === undefined) { + throw new TypeError('tool execution arguments must be losslessly JSON-serializable') + } + execution = { + ...base, + arguments: deepFreeze(detached), + } } catch (error: unknown) { - // Contract-violating arguments outside the lossless-JSON vocabulary cannot - // enter a pipeline whose logged and executed forms must agree. Still - // publish one scoped final outcome, using an immutable identity shell, so - // result observers retain their every-call guarantee without seeing the - // invalid value. - execution = Object.freeze({ - token: createExecutionToken(), - callId, - name, - arguments: undefined, - ...agent !== undefined ? { agent } : {}, - ...isExecutionToken(parent) ? { parent } : {}, - ...signal !== undefined ? { signal } : {}, - }) - const result = toolErrorResult(execution.callId, error) + execution = { ...base, arguments: undefined } + const result = this.materializeFinalResult(toolErrorResult(callId, error)) await this.notifyResult(execution, result) return result } let result: ToolExecutionResult try { - // Materialize the authoritative FINAL result, not merely the tool body's - // intermediate return. Post-policy may replace content or attach context, - // and every one of these fields is session-bound. Reject anything outside - // the lossless-JSON vocabulary before the observe-only `tools/result` - // commit point sees success. - result = this.snapshotExecutionResult(execution, await this.executePipeline(execution)) + result = this.materializeFinalResult(await this.executePipeline(execution)) } catch (error: unknown) { // Outer backstop: a throwing pre/post-execute listener, guard, or the // waterfall machinery becomes an isError result, never a turn failure. - result = toolErrorResult(execution.callId, error) + result = this.materializeFinalResult(toolErrorResult(execution.callId, error)) } await this.notifyResult(execution, result) return result } - /** Snapshot one call into a shared pipeline object with immutable identity and mutable cancellation. */ - private prepareExecution(input: Readonly): ToolExecution { - if (input.parent !== undefined && !isExecutionToken(input.parent)) { - throw new TypeError('tool execution parent must be a registry-minted opaque token') - } - const args = snapshotJsonValue(input.arguments) - if (args === undefined) { - throw new TypeError('tool execution arguments must be losslessly JSON-serializable') - } - const execution: ToolExecution = { - token: createExecutionToken(), - callId: input.callId, - name: input.name, - arguments: deepFreeze(args), - ...input.agent !== undefined ? { agent: input.agent } : {}, - ...input.parent !== undefined ? { parent: input.parent } : {}, - ...input.signal !== undefined ? { signal: input.signal } : {}, - } - Object.defineProperties(execution, { - token: { value: execution.token, enumerable: true, writable: false, configurable: false }, - callId: { value: execution.callId, enumerable: true, writable: false, configurable: false }, - name: { value: execution.name, enumerable: true, writable: false, configurable: false }, - arguments: { value: execution.arguments, enumerable: true, writable: false, configurable: false }, - agent: { value: input.agent, enumerable: true, writable: false, configurable: false }, - parent: { value: input.parent, enumerable: true, writable: false, configurable: false }, - }) - if (input.signal !== undefined) { - Object.defineProperty(execution, 'signal', { - value: input.signal, - enumerable: true, - writable: true, - configurable: true, - }) - } - return execution - } - /** Run the transformable pipeline; {@link execute} owns final normalization and notification. */ private async executePipeline(exec: ToolExecution): Promise { // --- Gate: tools/pre-execute. An `ask` resolves through the optional @@ -1082,10 +960,10 @@ export class ToolRegistry extends Service { // carrier keys dispatch by exec.agent, so an `agent.ctx` listener gates only // its own agent's calls (agent-less calls are subject-less). const carrier = scopeTarget(this, exec.agent) - const gate = this.snapshotPreDecision(await this.ctx.waterfall( + const gate = await this.ctx.waterfall( carrier, 'tools/pre-execute', exec, () => Promise.resolve({ kind: 'allow' }), - )) + ) const decision = gate.kind === 'ask' ? await this.serviceAsk(exec, gate) : gate const denialReason = decision.kind === 'allow' ? this.guardReason(exec) @@ -1109,7 +987,7 @@ export class ToolRegistry extends Service { // before delegating and inspect the normalized result after. Dispatched with the // same carrier as the gate, so an `agent.ctx` wrapper wraps only its own // agent's calls. --- - const result = this.snapshotExecutionResult(exec, await this.ctx.waterfall( + const result = await this.ctx.waterfall( carrier, 'tools/execute', exec, async (): Promise => { try { @@ -1130,64 +1008,25 @@ export class ToolRegistry extends Service { return toolErrorResult(exec.callId, 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) } - /** Validate and detach the extensible gate's decision before any grant can dispatch. */ - private snapshotPreDecision(value: unknown): PreToolDecision { - if (typeof value !== 'object' || value === null || Array.isArray(value)) { - throw new TypeError('tools/pre-execute must return a PreToolDecision object') - } - const decision = value as { kind?: unknown; reason?: unknown } - const keys = Reflect.ownKeys(decision) - const hasExactKeys = (...expected: string[]): boolean => - keys.length === expected.length && expected.every(key => Object.hasOwn(decision, key)) - switch (decision.kind) { - case 'allow': - if (!hasExactKeys('kind')) { - throw new TypeError('tools/pre-execute allow decision must contain only kind') - } - return { kind: 'allow' } - case 'deny': { - const reason = decision.reason - if (!hasExactKeys('kind', 'reason') || typeof reason !== 'string') { - throw new TypeError('tools/pre-execute deny decision must contain only kind and a string reason') - } - return { kind: 'deny', reason } - } - case 'ask': { - const reason = decision.reason - if (!(hasExactKeys('kind') || hasExactKeys('kind', 'reason')) - || (reason !== undefined && typeof reason !== 'string')) { - throw new TypeError('tools/pre-execute ask decision must contain only kind and an optional string reason') - } - return { kind: 'ask', ...reason !== undefined ? { reason } : {} } - } - default: - throw new TypeError('tools/pre-execute must return an allow, deny, or ask decision') - } - } - /** Notify final-result observers without giving them a mutation/error channel into the outcome. */ private async notifyResult(exec: ToolExecution, result: ToolExecutionResult): Promise { // The pipeline is over: freeze the remaining mutable signal slot so every // observer sees the SAME WeakMap-keyable execution without a mutation race. Object.freeze(exec) - // Materialize once more at the observer boundary so every listener receives - // the same detached result even when an internal error path constructed it. - const detached = snapshotJsonValue(result) - if (detached === undefined) { - throw new TypeError('tool result notification must be losslessly JSON-serializable') - } - const snapshot = deepFreeze(detached) const callbacks = this.ctx.events.dispatch('parallel', [ - scopeTarget(this, exec.agent), 'tools/result', exec, snapshot, + scopeTarget(this, exec.agent), 'tools/result', exec, result, ]) await Promise.all(callbacks.map(async (callback) => { try { - await callback(exec, snapshot) + await callback(exec, result) } catch (error: unknown) { this.ctx.logger.warn(`tool "${exec.name}" (${exec.callId}): tools/result observer failed: ${errorMessage(error)}`) } @@ -1241,112 +1080,40 @@ export class ToolRegistry extends Service { * Runs inside `execute`'s outer try/catch (a throwing listener → isError). */ private async postExecute(exec: ToolExecution, result: ToolExecutionResult): Promise { - // Snapshot the protected outcome BEFORE the waterfall. A listener receives - // the same `result` reference, so a post-waterfall read of `result.callId`/ - // `.isError`/`.error` could carry a listener's mutation — violating the - // authoritative-call-id requirement and the "preserve the dispatched - // isError/error" contract. The decision is the ONLY sanctioned channel for a - // listener to change the outcome (block, or accept-with-replacement); the - // call id is always the authoritative `exec.callId`. The one-pass snapshot - // protects nested content, error, and meta from in-place listener mutation. - const dispatched = this.snapshotExecutionResult(exec, result) - const decision = snapshotJsonValue(await this.ctx.waterfall( + const decision = await this.ctx.waterfall( scopeTarget(this, exec.agent), 'tools/post-execute', exec, result, () => Promise.resolve({ kind: 'accept' }), - )) - if (decision === undefined) { - throw new TypeError('tools/post-execute must return a losslessly JSON-serializable decision') - } - this.assertPostDecision(decision) + ) const additionalContext = decision.additionalContext if (decision.kind === 'block') { return { - callId: dispatched.callId, + callId: result.callId, content: decision.feedback, isError: true, ...additionalContext ? { additionalContext } : {}, } } - // accept: replace content if supplied, preserve the dispatched isError/error. + // Accept: replace content if supplied and preserve the dispatched outcome. return { - ...dispatched, + ...result, ...decision.content ? { content: decision.content } : {}, ...additionalContext ? { additionalContext } : {}, } } - /** Validate and detach an around-dispatch result before policy can observe or mutate it. */ - private snapshotExecutionResult(exec: ToolExecution, value: unknown): ToolExecutionResult { - if (typeof value !== 'object' || value === null) { - throw new TypeError('tools/execute must return a ToolExecutionResult object') - } - const result = value as Partial - // Capture the provider/listener-owned result exactly once. The same values - // must pass shape/correlation checks and become the detached final outcome; - // a stateful accessor cannot validate one result and publish another. - const callId = result.callId - const content = result.content - const isError = result.isError - const error = result.error - const additionalContext = result.additionalContext - const meta = result.meta - if (!Array.isArray(content) || typeof isError !== 'boolean') { - throw new TypeError('tools/execute must return a ToolExecutionResult with content[] and boolean isError') - } - if (callId !== exec.callId) { - throw new TypeError(`tools/execute returned callId "${String(callId)}" for authoritative call "${exec.callId}"`) - } - const candidate = { - callId: exec.callId, - content, - isError, - ...error !== undefined ? { error } : {}, - ...additionalContext !== undefined ? { additionalContext } : {}, - ...meta !== undefined ? { meta } : {}, - } - // One traversal both validates and detaches the accepted result. A separate - // check followed by structuredClone would reread getters and could sanitize - // a class instance into an apparently valid plain record. - const snapshot = snapshotJsonValue(candidate) - if (snapshot === undefined) { - throw new TypeError('tools/execute must return a losslessly JSON-serializable ToolExecutionResult') - } - return snapshot - } - - /** Reject malformed JavaScript/casted post decisions at the public event boundary. */ - private assertPostDecision(value: unknown): asserts value is PostToolDecision { - if (typeof value !== 'object' || value === null) { - throw new TypeError('tools/post-execute must return a PostToolDecision object') - } - const decision = value as Partial - switch (decision.kind) { - case 'accept': - if (decision.content !== undefined && !Array.isArray(decision.content)) { - throw new TypeError('tools/post-execute accept content must be an array') - } - return - case 'block': - if (!Array.isArray(decision.feedback)) { - throw new TypeError('tools/post-execute block feedback must be an array') - } - return - default: - throw new TypeError('tools/post-execute must return an accept or block decision') + /** Materialize the authoritative commit outcome once, immediately before `tools/result`. */ + private materializeFinalResult(result: ToolExecutionResult): ToolExecutionResult { + const detached = snapshotJsonValue(result) + if (detached === undefined) { + throw new TypeError('tool result must be losslessly JSON-serializable') } + return deepFreeze(detached) } } -/** Mint a frozen, property-free correlation token whose identity is its value. */ +/** Mint a same-process correlation token whose identity is its value. */ function createExecutionToken(): ToolExecutionToken { - const token = Object.freeze(Object.create(null)) as ToolExecutionToken - executionTokens.add(token) - return token -} - -/** Runtime counterpart of the opaque token type, including `undefined` input. */ -function isExecutionToken(value: unknown): value is ToolExecutionToken { - return typeof value === 'object' && value !== null && executionTokens.has(value) + return Symbol('dsh.tool.execution') as ToolExecutionToken } function toolErrorResult(callId: ToolExecution['callId'], error: unknown): ToolExecutionResult { diff --git a/packages/core/tools/src/schema.ts b/packages/core/tools/src/schema.ts index add9c29e61..dc33a97299 100644 --- a/packages/core/tools/src/schema.ts +++ b/packages/core/tools/src/schema.ts @@ -20,7 +20,6 @@ */ import { assertNever, HarnessError } from '@deepseek-ai/dsh-llm' -import { snapshotJsonValue } from '@deepseek-ai/dsh-session' import type { ToolDefinition, ToolExecuteReturn, ToolExecution, ToolResult } from './index.ts' import type { ToolCallView, ToolResultView } from './presentation.ts' @@ -288,21 +287,23 @@ export function validateArgs(spec: SchemaSpec, args: unknown): string[] { /** Options for {@link defineTool}. */ export interface DefineToolOptions { /** Tool name (must be unique). */ - name: string + readonly name: string /** Human-readable description sent to the model. */ - description: string + readonly description: string /** * Parameter schema using the per-property-required DSL. Converted to * standard JSON Schema at runtime. */ - parameters: S + readonly parameters: S /** * Optional cooperative tool-call timeout budget in milliseconds. When given it * must be a positive finite number; it is attached to the produced * {@link ToolDefinition} for `@deepseek-ai/dsh-timeout-policy` to enforce and * is never sent to the model. */ - timeoutMs?: number + readonly timeoutMs?: number + /** Make this protocol tool's canonical wire presence or absence owner-final. */ + readonly ownerFinal?: boolean /** * Tool execution function. `args` is typed as {@link InferArgs} — zero * casts needed. Returns either a bare {@link ContentBlock}`[]` (model-facing @@ -355,11 +356,6 @@ export interface DefineToolOptions { * by `ToolRegistry.register()` directly — `defineTool` is sugar for * first-party plugin authors. * - * Definition is an acceptance boundary: every top-level option is read once, - * and the parameter spec is detached before either the wire schema or the - * runtime validators are built. Later mutation of the caller's options or - * schema therefore cannot make the model-visible schema disagree with execute - * or presentation validation. * @param options - the tool's name, description, typed parameter schema, * execute body, and optional presenters. * @returns a registry-ready {@link ToolDefinition}: its `execute` validates the @@ -369,13 +365,6 @@ export interface DefineToolOptions { * args). */ export function defineTool(options: DefineToolOptions): ToolDefinition { - // Capture every caller-owned top-level field before inspecting any nested - // schema value. Accessors may be stateful, so validation, presentation, and - // the returned definition must all derive from this one accepted record. - const name = options.name - const description = options.description - const inputParameters = options.parameters - const timeoutMs = options.timeoutMs // Object-literal execute methods don't use `this`; the reference is safe. // eslint-disable-next-line @typescript-eslint/unbound-method const userExecute = options.execute @@ -383,31 +372,21 @@ export function defineTool(options: DefineToolOptions): const userPresentCall = options.presentCall // eslint-disable-next-line @typescript-eslint/unbound-method const userPresentResult = options.presentResult - if (timeoutMs !== undefined && (!Number.isFinite(timeoutMs) || timeoutMs <= 0)) { - throw new Error(`defineTool(${name}): timeoutMs must be a positive finite number`) - } - // The internal SchemaSpec and public wire schema must not share mutable - // subobjects. Each is materialized through the lossless one-pass boundary; - // structuredClone alone could sanitize an exotic default or nested getter. - const parameterSpec = snapshotJsonValue(inputParameters) - if (parameterSpec === undefined) { - throw new Error(`defineTool(${name}): parameters must be losslessly JSON-serializable`) - } - const wireParameters = snapshotJsonValue(schemaSpecToJsonSchema(parameterSpec)) - if (wireParameters === undefined) { - throw new Error(`defineTool(${name}): generated parameters must be losslessly JSON-serializable`) + if (options.timeoutMs !== undefined && (!Number.isFinite(options.timeoutMs) || options.timeoutMs <= 0)) { + throw new Error(`defineTool(${options.name}): timeoutMs must be a positive finite number`) } const tool: ToolDefinition = { - name, - description, - parameters: wireParameters as unknown as Record, - ...(timeoutMs !== undefined ? { timeoutMs } : {}), + name: options.name, + description: options.description, + parameters: schemaSpecToJsonSchema(options.parameters) as unknown as Record, + ...(options.timeoutMs !== undefined ? { timeoutMs: options.timeoutMs } : {}), + ...(options.ownerFinal === true ? { ownerFinal: true } : {}), async execute(args: unknown, exec: ToolExecution): Promise { // Validate the model-generated args before the typed body runs. On // mismatch we throw ToolArgsError; the registry turns it into an // isError result so the model can self-correct. After this guard, the // cast to InferArgs reflects the validated shape. - const violations = validateArgs(parameterSpec, args) + const violations = validateArgs(options.parameters, args) if (violations.length > 0) throw new ToolArgsError(violations) return userExecute(args as InferArgs, exec) }, @@ -418,13 +397,13 @@ export function defineTool(options: DefineToolOptions): // than the hard `ToolArgsError` the execute path raises. if (userPresentCall) { tool.presentCall = (args: unknown): ToolCallView | undefined => { - if (validateArgs(parameterSpec, args).length > 0) return undefined + if (validateArgs(options.parameters, args).length > 0) return undefined return userPresentCall(args as InferArgs) } } if (userPresentResult) { tool.presentResult = (args: unknown, result: ToolResult): ToolResultView | undefined => { - if (validateArgs(parameterSpec, args).length > 0) return undefined + if (validateArgs(options.parameters, args).length > 0) return undefined return userPresentResult(args as InferArgs, result) } } diff --git a/packages/core/tools/tests/code-mode.spec.ts b/packages/core/tools/tests/code-mode.spec.ts index da5725f10e..83dedbdb00 100644 --- a/packages/core/tools/tests/code-mode.spec.ts +++ b/packages/core/tools/tests/code-mode.spec.ts @@ -215,41 +215,24 @@ describe('mode-aware wire contribution', () => { expect(() => scope.ctx.tools.register(impostor)).toThrow(/reserved for the Code Mode presentation transport/) expect(() => ctx.tools.register(impostor)).toThrow(/reserved for the Code Mode presentation transport/) expect(() => scope.ctx.systemPrompt.section({ name: 'tools:sdk', order: -999, text: 'malicious SDK' })) - .toThrow(/globally protected and cannot be shadowed/) + .toThrow(/globally owner-final and cannot be shadowed/) expect(() => scope.ctx.tools.restrict({ allow: [RUN_CODE_NAME] })).toThrow(/cannot name reserved Code Mode presentation transport/) expect(() => scope.ctx.tools.restrict({ deny: [RUN_CODE_NAME] })).toThrow(/cannot name reserved Code Mode presentation transport/) - const transport = ctx.tools.get(RUN_CODE_NAME)! - expect(Object.isFrozen(transport)).toBe(true) - expect(Object.isFrozen(transport.parameters)).toBe(true) - expect(() => { transport.name = 'mutated_transport' }).toThrow(TypeError) - - const mutableSection = { name: 'scoped-note', order: 149, text: 'safe note' } - scope.ctx.systemPrompt.section(mutableSection) - mutableSection.name = 'tools:sdk' - mutableSection.text = 'mutated SDK' - const mutableTool = defineTool({ + scope.ctx.systemPrompt.section({ name: 'scoped-note', order: 149, text: 'safe note' }) + scope.ctx.tools.register(defineTool({ name: 'scoped_safe', description: 'Safe scoped tool.', parameters: {}, execute: () => Promise.resolve([{ type: 'text' as const, text: 'safe' }]), - }) - scope.ctx.tools.register(mutableTool) - mutableTool.name = RUN_CODE_NAME - mutableTool.description = 'Mutated transport impostor.' - const stored = ctx.tools.get('scoped_safe', agent)! - expect(Object.isFrozen(stored)).toBe(true) - expect(Object.isFrozen(stored.parameters)).toBe(true) - expect(() => { stored.name = RUN_CODE_NAME }).toThrow(TypeError) + })) const assembly = await systemPrompt.assemble({ scope: agent }) const transports = assembly.tools.filter(tool => tool.name === RUN_CODE_NAME) expect(transports).toHaveLength(1) expect(transports[0]?.description).toContain('Execute a TypeScript program') - expect(assembly.sections.find(section => section.name === 'tools:sdk')?.text).not.toContain('mutated SDK') expect(assembly.sections.find(section => section.name === 'scoped-note')?.text).toBe('safe note') expect(assembly.sections.find(section => section.name === 'tools:sdk')?.text).toContain('scoped_safe(args:') expect(ctx.tools.get(RUN_CODE_NAME, agent)).toBe(ctx.tools.get(RUN_CODE_NAME)) - expect(ctx.tools.knownNames(agent)).not.toContain(RUN_CODE_NAME) const result = await runCode(ctx, 'return 1', { agent }) expect(result.content).toEqual([{ type: 'text', text: '(run_code completed with no output)' }]) }) @@ -262,7 +245,6 @@ describe('mode-aware wire contribution', () => { registerEcho(ctx) const { agent } = await mintAgentScope(ctx) - expect(ctx.tools.knownNames(agent)).toEqual(['echo']) const assembly = await systemPrompt.assemble({ scope: agent }) expect(assembly.tools.map(tool => tool.name)).toEqual(mode === 'code' ? [RUN_CODE_NAME] diff --git a/packages/core/tools/tests/scoped.spec.ts b/packages/core/tools/tests/scoped.spec.ts index 40671ae6fc..caf3ccb7be 100644 --- a/packages/core/tools/tests/scoped.spec.ts +++ b/packages/core/tools/tests/scoped.spec.ts @@ -4,7 +4,7 @@ import { createScope } from '@deepseek-ai/dsh-scope' import type { Scope } from '@deepseek-ai/dsh-scope' import SystemPrompt from '@deepseek-ai/dsh-system-prompt' import ToolRegistry from '@deepseek-ai/dsh-tools' -import type { PreToolDecision, ToolDefinition, ToolExecution, ToolExecutionInput, ToolExecutionToken, ToolRestriction } from '@deepseek-ai/dsh-tools' +import type { PreToolDecision, ToolDefinition, ToolExecution, ToolExecutionInput, ToolExecutionToken } from '@deepseek-ai/dsh-tools' import type { Agent, AgentId } from '@deepseek-ai/dsh-agent' import { CallId } from '@deepseek-ai/dsh-llm' import type { ContentBlock } from '@deepseek-ai/dsh-llm' @@ -89,6 +89,20 @@ describe('scoped tool registration', () => { expect(() => scope.ctx.tools.register(tool('y'))).toThrow(/already registered in this scope/) }) + it('rejects either registration order between a global owner-final tool and a scoped shadow', async () => { + const first = await mount() + const { scope: firstScope } = await mintAgentScope(first, 'first') + first.tools.register({ ...tool('reserved'), ownerFinal: true }) + expect(() => firstScope.ctx.tools.register(tool('reserved'))) + .toThrow(/globally owner-final and cannot be shadowed/) + + const second = await mount() + const { scope: secondScope } = await mintAgentScope(second, 'second') + secondScope.ctx.tools.register(tool('reserved')) + expect(() => second.tools.register({ ...tool('reserved'), ownerFinal: true })) + .toThrow(/owner-final tool "reserved" cannot be registered while a scoped shadow exists/) + }) + it('disposing the scope unwinds its registrations and leaves no residue', async () => { const ctx = await mount() const { scope, key } = await mintAgentScope(ctx, 'a') @@ -96,7 +110,7 @@ describe('scoped tool registration', () => { expect(ctx.tools.get('mine', key)).toBeDefined() await scope.dispose() expect(ctx.tools.get('mine', key)).toBeUndefined() - expect(ctx.tools.knownNames(key)).toEqual([]) + expect(ctx.tools.schemas(key)).toEqual([]) }) }) @@ -153,7 +167,7 @@ describe('restrict()', () => { expect(ctx.tools.schemas(key).map(t => t.name).sort()).toEqual(['a', 'c']) }) - it('snapshots the filter at registration (caller mutation changes nothing)', async () => { + it('compiles the readonly filter values at registration', async () => { const ctx = await mount() const { scope, key } = await mintAgentScope(ctx, 'a') ctx.tools.register(tool('a')) @@ -164,38 +178,21 @@ describe('restrict()', () => { expect(ctx.tools.schemas(key).map(t => t.name)).toEqual(['b']) }) - it('reads restriction accessors once so the checked filter is the enforced filter', async () => { - const ctx = await mount() - const { scope, key } = await mintAgentScope(ctx, 'a') - ctx.tools.register(tool('global')) - let allowReads = 0 - const filter = { - get allow(): string[] | undefined { - allowReads += 1 - return allowReads === 1 ? [] : undefined - }, - } as ToolRestriction - - scope.ctx.tools.restrict(filter) - - expect(allowReads).toBe(1) - expect(ctx.tools.schemas(key)).toEqual([]) - expect(await run(ctx, 'global', key)).toBe('Error: unknown tool "global"') - }) - - it('fails loud on an unscoped call, an empty filter, and unknown names', async () => { + it('fails loud on an unscoped call, an empty filter, and non-global names', async () => { const ctx = await mount() const { scope } = await mintAgentScope(ctx, 'a') ctx.tools.register(tool('real')) + scope.ctx.tools.register(tool('local')) expect(() => ctx.tools.restrict({ deny: ['real'] })).toThrow(/requires a scoped context/) expect(() => scope.ctx.tools.restrict({})).toThrow(/no-op/) - expect(() => scope.ctx.tools.restrict({ allow: ['reall'] })).toThrow(/unknown tool "reall"; known tools for this scope: real/) - expect(() => scope.ctx.tools.restrict({ deny: ['ghost', 'wraith'] })).toThrow(/unknown tools "ghost", "wraith"/) + expect(() => scope.ctx.tools.restrict({ allow: ['local'] })).toThrow(/unknown global tool "local"/) + expect(() => scope.ctx.tools.restrict({ allow: ['reall'] })).toThrow(/unknown global tool "reall"; known global tools: real/) + expect(() => scope.ctx.tools.restrict({ deny: ['ghost', 'wraith'] })).toThrow(/unknown global tools "ghost", "wraith"/) const emptyCtx = await mount() const { scope: emptyScope } = await mintAgentScope(emptyCtx, 'empty') expect(() => emptyScope.ctx.tools.restrict({ deny: ['ghost'] })) - .toThrow(/known tools for this scope: \(none\)/) + .toThrow(/known global tools: \(none\)/) }) }) @@ -230,9 +227,8 @@ describe('scoped execution dispatch', () => { return Promise.resolve([{ type: 'text', text: 'ran:t' }]) }, }) - let guardViewFrozen = false const guard = (execution: Readonly): string => { - guardViewFrozen = Object.isFrozen(execution) && Object.isFrozen(execution.arguments) + expect(Object.isFrozen(execution.arguments)).toBe(true) return 'terminal policy' } const liftFirst = scope.ctx.tools.guard(guard) @@ -243,7 +239,6 @@ describe('scoped execution dispatch', () => { scope.ctx.on('tools/pre-execute', () => Promise.resolve({ kind: 'allow' }), { prepend: true }) expect(await run(ctx, 't', key)).toBe('Error: terminal policy') - expect(guardViewFrozen).toBe(true) expect(await run(ctx, 't', other)).toBe('ran:t') expect(bodyCalls).toBe(1) @@ -271,13 +266,14 @@ describe('scoped execution dispatch', () => { expect(bodyCalls).toBe(0) }) - it('protects call identity before policy and dispatch while leaving only signal mutable', async () => { + it('shares one token and materialized argument value across the pipeline', async () => { const ctx = await mount() const { scope, key } = await mintAgentScope(ctx, 'a') let safeCalls = 0 let dangerCalls = 0 let scopedResults = 0 let safeArguments: unknown + const tokens = new Set() ctx.tools.register({ ...tool('safe'), execute: (args) => { @@ -295,17 +291,16 @@ describe('scoped execution dispatch', () => { }) scope.ctx.tools.guard(exec => exec.name === 'danger' ? 'danger denied' : undefined) ctx.on('tools/pre-execute', (exec, next) => { - expect(Reflect.set(exec, 'agent', undefined)).toBe(false) - expect(Reflect.set(exec, 'name', 'safe')).toBe(false) - expect(Reflect.set(exec.arguments as object, 'injected', true)).toBe(false) + tokens.add(exec.token) + expect(Object.isFrozen(exec.arguments)).toBe(true) return next() }) ctx.on('tools/execute', (exec, next) => { - expect(Reflect.set(exec, 'name', 'danger')).toBe(false) + tokens.add(exec.token) return next() }) ctx.on('tools/post-execute', (exec, _result, next) => { - expect(Reflect.set(exec, 'agent', undefined)).toBe(false) + tokens.add(exec.token) return next() }) scope.ctx.on('tools/result', () => { scopedResults += 1 }) @@ -323,6 +318,8 @@ describe('scoped execution dispatch', () => { expect(safeArguments).not.toBe(callerArguments) expect(Object.isFrozen(safeArguments)).toBe(true) expect(callerArguments).toEqual({ source: true }) + // One token for danger and one shared by every phase of safe. + expect(tokens.size).toBe(2) expect({ safeCalls, dangerCalls, scopedResults }).toEqual({ safeCalls: 1, dangerCalls: 0, @@ -395,25 +392,6 @@ describe('scoped execution dispatch', () => { expect(callerArguments.invalid).toBeTypeOf('function') }) - it('rejects a forged mutable parent token without exposing it to final observers', async () => { - const ctx = await mount() - ctx.tools.register(tool('t')) - const forged = { mutable: true } as unknown as ToolExecutionToken - let observedParent: ToolExecutionToken | undefined = forged - ctx.on('tools/result', (exec) => { observedParent = exec.parent }) - - const result = await ctx.tools.execute({ - callId: CallId('forged-parent'), name: 't', arguments: {}, parent: forged, - }) - - expect(result.isError).toBe(true) - expect(result.content).toEqual([{ - type: 'text', text: 'Error: tool execution parent must be a registry-minted opaque token', - }]) - expect(observedParent).toBeUndefined() - expect(Object.isFrozen(forged)).toBe(false) - }) - it('reads a stateful parent accessor once before policy, dispatch, and result observation', async () => { const ctx = await mount() const observed: (ToolExecutionToken | undefined)[] = [] diff --git a/packages/core/tools/tests/tools.spec.ts b/packages/core/tools/tests/tools.spec.ts index 1b83a8f633..4c52ba7910 100644 --- a/packages/core/tools/tests/tools.spec.ts +++ b/packages/core/tools/tests/tools.spec.ts @@ -6,8 +6,8 @@ import type { Agent } from '@deepseek-ai/dsh-agent' import ApprovalService, { type ApprovalOutcome, type ApprovalRequest } from '@deepseek-ai/dsh-user-approval' import ToolRegistry, { defineTool, schemaSpecToJsonSchema, validateArgs, ToolArgsError, ToolNotFoundError, - type DefineToolOptions, type InferArgs, type SchemaSpec, type PreToolDecision, type PostToolDecision, - type ToolDefinition, type ToolExecution, type ToolExecutionInput, type ToolExecutionResult, type ToolGuard, + type InferArgs, type SchemaSpec, type PreToolDecision, type PostToolDecision, + type ToolExecution, type ToolExecutionResult, } from '@deepseek-ai/dsh-tools' async function setup() { @@ -83,95 +83,6 @@ describe('ToolRegistry', () => { expect(result).toEqual({ callId: CallId('c1'), content: [{ type: 'text', text: 'hi' }], isError: false }) }) - it.each([ - { field: 'callId', value: 1n }, - { field: 'callId', value: 123 }, - { field: 'name', value: 1n }, - { field: 'name', value: 123 }, - ] as const)('rejects a non-string $field before final observation', async ({ field, value }) => { - const ctx = await setup() - let observed = 0 - ctx.on('tools/result', () => { observed += 1 }) - const input: Record = { - callId: CallId('valid-call'), - name: 'missing', - arguments: {}, - } - input[field] = value - - await expect(ctx.tools.execute(input as unknown as ToolExecutionInput)) - .rejects.toThrow(`tool execution ${field} must be a string`) - expect(observed).toBe(0) - }) - - it('reads correlation accessors once and normalizes a later hostile accessor', async () => { - const ctx = await setup() - const reads = { callId: 0, name: 0, arguments: 0 } - let observed: { callId: unknown; name: unknown; isError: boolean } | undefined - ctx.on('tools/result', (exec, result) => { - observed = { callId: exec.callId, name: exec.name, isError: result.isError } - }) - const input = Object.defineProperties({}, { - callId: { - enumerable: true, - get: () => { - reads.callId += 1 - if (reads.callId > 1) throw new Error('callId reread') - return CallId('one-read-call') - }, - }, - name: { - enumerable: true, - get: () => { - reads.name += 1 - if (reads.name > 1) throw new Error('name reread') - return 'missing' - }, - }, - arguments: { - enumerable: true, - get: () => { - reads.arguments += 1 - throw new Error('arguments accessor broke') - }, - }, - }) as unknown as ToolExecutionInput - - const result = await ctx.tools.execute(input) - - expect(reads).toEqual({ callId: 1, name: 1, arguments: 1 }) - expect(result).toMatchObject({ callId: CallId('one-read-call'), isError: true }) - expect(result.content[0]).toMatchObject({ text: 'Error: arguments accessor broke' }) - expect(observed).toEqual({ callId: CallId('one-read-call'), name: 'missing', isError: true }) - }) - - it('reads callId once before a hostile name accessor rejects correlation', async () => { - const ctx = await setup() - const reads = { callId: 0, name: 0 } - let observed = 0 - ctx.on('tools/result', () => { observed += 1 }) - const input = Object.defineProperties({ arguments: {} }, { - callId: { - enumerable: true, - get: () => { - reads.callId += 1 - return CallId('hostile-name') - }, - }, - name: { - enumerable: true, - get: () => { - reads.name += 1 - throw new Error('name accessor broke') - }, - }, - }) as unknown as ToolExecutionInput - - await expect(ctx.tools.execute(input)).rejects.toThrow('name accessor broke') - expect(reads).toEqual({ callId: 1, name: 1 }) - expect(observed).toBe(0) - }) - it('threads a tool-attached meta (object return form) onto the result', async () => { const ctx = await setup() ctx.tools.register({ @@ -224,77 +135,6 @@ describe('ToolRegistry', () => { expect(observedError).toBe(true) }) - it('reads each result value once so later getter drift cannot change the snapshot', async () => { - const ctx = await setup() - ctx.tools.register(echoTool) - let reads = 0 - const hostileBlock = Object.defineProperty({ type: 'text' }, 'text', { - enumerable: true, - get: () => ++reads === 1 ? 'safe' : new Map([['mutable', true]]), - }) - ctx.on('tools/execute', async exec => ({ - callId: exec.callId, - content: [hostileBlock], - isError: false, - }) as unknown as ToolExecutionResult) - - const result = await ctx.tools.execute({ - callId: CallId('unstable-result'), name: 'echo', arguments: {}, - }) - - expect(reads).toBe(1) - expect(result).toEqual({ - callId: CallId('unstable-result'), - content: [{ type: 'text', text: 'safe' }], - isError: false, - }) - }) - - it('reads every top-level execution result field once before validation', async () => { - const ctx = await setup() - ctx.tools.register(echoTool) - const reads = { callId: 0, content: 0, isError: 0, error: 0, additionalContext: 0, meta: 0 } - ctx.on('tools/execute', async exec => Object.defineProperties({}, { - callId: { enumerable: true, get: () => { reads.callId += 1; return reads.callId === 1 ? exec.callId : CallId('drifted') } }, - content: { enumerable: true, get: () => { reads.content += 1; return reads.content === 1 ? [{ type: 'text', text: 'accepted' }] : [] } }, - isError: { enumerable: true, get: () => { reads.isError += 1; return reads.isError !== 1 } }, - error: { enumerable: true, get: () => { reads.error += 1; return undefined } }, - additionalContext: { enumerable: true, get: () => { reads.additionalContext += 1; return undefined } }, - meta: { enumerable: true, get: () => { reads.meta += 1; return undefined } }, - }) as ToolExecutionResult) - - const result = await ctx.tools.execute({ - callId: CallId('one-read-result'), name: 'echo', arguments: {}, - }) - - expect(reads).toEqual({ callId: 1, content: 1, isError: 1, error: 1, additionalContext: 1, meta: 1 }) - expect(result).toEqual({ - callId: CallId('one-read-result'), - content: [{ type: 'text', text: 'accepted' }], - isError: false, - }) - }) - - it('rejects an exotic nested result before its prototype can be sanitized', async () => { - const ctx = await setup() - ctx.tools.register(echoTool) - class ExoticText { readonly value = 'not text' } - ctx.on('tools/execute', exec => Promise.resolve({ - callId: exec.callId, - content: [{ type: 'text', text: new ExoticText() }], - isError: false, - } as unknown as ToolExecutionResult)) - - const result = await ctx.tools.execute({ - callId: CallId('exotic-result'), name: 'echo', arguments: {}, - }) - - expect(result.isError).toBe(true) - expect(result.content).toEqual([{ - type: 'text', text: 'Error: tools/execute must return a losslessly JSON-serializable ToolExecutionResult', - }]) - }) - it('returns isError results for unknown tools and throwing tools', async () => { const ctx = await setup() ctx.tools.register({ @@ -362,85 +202,6 @@ describe('ToolRegistry', () => { expect(result.content[0]).toMatchObject({ text: 'Error: denied by policy' }) }) - it.each([ - { - name: 'non-object decision', - replacement: null, - message: 'tools/pre-execute must return a PreToolDecision object', - }, - { - name: 'unknown decision kind', - replacement: { kind: 'permit' }, - message: 'tools/pre-execute must return an allow, deny, or ask decision', - }, - { - name: 'allow decision carrying extra fields', - replacement: { kind: 'allow', reason: 'smuggled' }, - message: 'tools/pre-execute allow decision must contain only kind', - }, - { - name: 'deny decision without a reason', - replacement: { kind: 'deny' }, - message: 'tools/pre-execute deny decision must contain only kind and a string reason', - }, - { - name: 'deny decision with a non-string reason', - replacement: { kind: 'deny', reason: 42 }, - message: 'tools/pre-execute deny decision must contain only kind and a string reason', - }, - { - name: 'ask decision with a non-string reason', - replacement: { kind: 'ask', reason: true }, - message: 'tools/pre-execute ask decision must contain only kind and an optional string reason', - }, - { - name: 'ask decision carrying extra fields', - replacement: { kind: 'ask', cache: true }, - message: 'tools/pre-execute ask decision must contain only kind and an optional string reason', - }, - ])('fails closed on a malformed tools/pre-execute $name', async ({ replacement, message }) => { - const ctx = await setup() - let bodyCalls = 0 - const observed: ToolExecutionResult[] = [] - ctx.tools.register({ - ...echoTool, - async execute() { - bodyCalls += 1 - return [] - }, - }) - ctx.on('tools/pre-execute', async () => replacement as unknown as PreToolDecision) - ctx.on('tools/result', (_exec, result) => { observed.push(result) }) - - const result = await ctx.tools.execute({ - callId: CallId('malformed-pre'), name: 'echo', arguments: {}, - }) - - expect(result.isError).toBe(true) - expect(result.content[0]).toMatchObject({ text: `Error: ${message}` }) - expect(bodyCalls).toBe(0) - expect(observed).toEqual([result]) - }) - - it('rejects a JavaScript guard that returns an async/non-string decision', async () => { - const ctx = await setup() - let bodyCalls = 0 - ctx.tools.register({ - ...echoTool, - async execute() { - bodyCalls += 1 - return [] - }, - }) - ctx.tools.guard((() => Promise.resolve('late denial')) as unknown as ToolGuard) - - const result = await ctx.tools.execute({ callId: CallId('bad-guard'), name: 'echo', arguments: {} }) - expect(result.isError).toBe(true) - expect(result.content[0]?.type === 'text' && result.content[0].text) - .toContain('tools.guard() must return') - expect(bodyCalls).toBe(0) - }) - it('an ask decision degrades to deny when no approval seam is mounted', async () => { const ctx = await setup() ctx.tools.register(echoTool) @@ -617,55 +378,6 @@ describe('ToolRegistry', () => { expect(result.additionalContext).toMatchObject({ content: [{ text: 'fyi' }], source: { kind: 'plugin', plugin: 'test' } }) }) - it('a post-execute listener cannot mutate any nested part of the dispatched result', async () => { - // The decision is the ONLY sanctioned channel to change the outcome. A - // listener that reaches in and mutates the passed result reference (flipping - // isError, rewriting callId, attaching a bogus error) must NOT affect what - // execute() returns — the registry snapshots the authoritative fields before - // the waterfall and rebuilds from the snapshot + decision. - const ctx = await setup() - ctx.tools.register(echoTool) - ctx.on('tools/execute', async (_exec, next) => { - await next() - return { - callId: CallId('c1'), - content: [{ type: 'text', text: 'original' }], - isError: true, - error: { name: 'OriginalError', code: 'ORIGINAL' }, - meta: { nested: { label: 'original' } }, - } - }) - - ctx.on('tools/post-execute', async (_exec, result, next) => { - const mutable = result as { - callId: string - isError: boolean - error?: { name: string; code: string } - content: { type: 'text'; text: string }[] - meta?: { nested: { label: string } } - } - mutable.callId = 'hijacked' - mutable.isError = false - if (mutable.error) { - mutable.error.name = 'Evil' - mutable.error.code = 'EVIL' - } - mutable.content[0]!.text = 'MUTATED' - mutable.content.push({ type: 'text', text: 'INJECTED' }) // in-place array mutation - if (mutable.meta) mutable.meta.nested.label = 'MUTATED' - return next() // delegate to the default accept — no decision-level override - }) - - const result = await ctx.tools.execute({ callId: CallId('c1'), name: 'echo', arguments: { text: 'hi' } }) - expect(result.callId).toBe(CallId('c1')) // authoritative exec.callId, not 'hijacked' - expect(result.isError).toBe(true) - expect(result.error).toEqual({ name: 'OriginalError', code: 'ORIGINAL' }) - expect(result.content).toHaveLength(1) // the in-place push did not leak in - expect(result.content[0]).toMatchObject({ text: 'original' }) - expect(result.content.some(b => (b as { text?: string }).text === 'INJECTED')).toBe(false) - expect(result.meta).toEqual({ nested: { label: 'original' } }) - }) - it('composes pre + post waterfalls around dispatch (sandbox-wrap pattern)', async () => { const ctx = await setup() ctx.tools.register(echoTool) @@ -844,115 +556,20 @@ describe('ToolRegistry', () => { }) }) - it('normalizes malformed tools/execute results instead of treating them as success', async () => { + it('normalizes a tools/execute result with the wrong call id', async () => { const ctx = await setup() ctx.tools.register(echoTool) - let observedError: boolean | undefined - ctx.on('tools/execute', async (_exec, next) => { - await next() - return {} as ToolExecutionResult - }) - ctx.on('tools/result', (_exec, result) => { observedError = result.isError }) - - const result = await ctx.tools.execute({ callId: CallId('malformed'), name: 'echo', arguments: {} }) - expect(result.isError).toBe(true) - expect(result.content[0]).toMatchObject({ - text: 'Error: tools/execute must return a ToolExecutionResult with content[] and boolean isError', - }) - expect(observedError).toBe(true) - }) - - it('rejects non-JSON data at the defensive final-result notification boundary', async () => { - const ctx = await setup() - ctx.tools.register(echoTool) - let execution: ToolExecution | undefined - ctx.on('tools/execute', async (exec, next) => { - execution = exec - return next() - }) - await ctx.tools.execute({ callId: CallId('capture-execution'), name: 'echo', arguments: {} }) - if (execution === undefined) throw new Error('test fixture did not capture the execution') - - const internal = ctx.tools as unknown as { - notifyResult(exec: ToolExecution, result: ToolExecutionResult): Promise - } - const invalid = { - callId: CallId('capture-execution'), - content: new Map() as unknown as ToolExecutionResult['content'], - isError: false, - } - await expect(internal.notifyResult(execution, invalid)) - .rejects.toThrow('tool result notification must be losslessly JSON-serializable') - }) - - it.each([ - { - name: 'non-object result', - replacement: null, - message: 'tools/execute must return a ToolExecutionResult object', - }, - { - name: 'wrong call id', - replacement: { callId: CallId('other'), content: [], isError: false }, - message: 'tools/execute returned callId "other" for authoritative call "malformed-shape"', - }, - ])('normalizes a tools/execute $name', async ({ replacement, message }) => { - const ctx = await setup() - ctx.tools.register(echoTool) - ctx.on('tools/execute', async () => replacement as ToolExecutionResult) + 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: ${message}` }) - }) - - it('normalizes malformed tools/post-execute decisions', async () => { - const ctx = await setup() - ctx.tools.register(echoTool) - ctx.on('tools/post-execute', async () => ({ kind: 'accept', content: 'not blocks' }) as unknown as PostToolDecision) - - const result = await ctx.tools.execute({ callId: CallId('malformed-post'), name: 'echo', arguments: {} }) - expect(result.isError).toBe(true) expect(result.content[0]).toMatchObject({ - text: 'Error: tools/post-execute accept content must be an array', + text: 'Error: tools/execute returned callId "other" for authoritative call "malformed-shape"', }) }) - it.each([ - { - name: 'non-object decision', - replacement: null, - message: 'tools/post-execute must return a PostToolDecision object', - }, - { - name: 'block without feedback blocks', - replacement: { kind: 'block', feedback: 'not blocks' }, - message: 'tools/post-execute block feedback must be an array', - }, - { - name: 'unknown decision kind', - replacement: { kind: 'defer' }, - message: 'tools/post-execute must return an accept or block decision', - }, - { - name: 'non-JSON decision', - replacement: { kind: 'accept', content: new Map() }, - message: 'tools/post-execute must return a losslessly JSON-serializable decision', - }, - ])('normalizes a tools/post-execute $name', async ({ replacement, message }) => { - const ctx = await setup() - ctx.tools.register(echoTool) - ctx.on('tools/post-execute', async () => replacement as unknown as PostToolDecision) - - const result = await ctx.tools.execute({ - callId: CallId('malformed-post-shape'), name: 'echo', arguments: {}, - }) - expect(result.isError).toBe(true) - expect(result.content[0]).toMatchObject({ text: `Error: ${message}` }) - }) - it('returns an isError result when a tools/execute listener throws', async () => { const ctx = await setup() ctx.tools.register(echoTool) @@ -1030,109 +647,12 @@ describe('ToolRegistry', () => { }]) }) - it.each([ - ['Map', new Map([['mutable', true]])], - ['class instance', new (class Parameters { value = 1 })()], - ])('rejects cloneable non-JSON tool parameters (%s) without registry residue', async (_kind, parameters) => { + it('rejects a non-positive or non-finite registration timeout', async () => { const ctx = await setup() - const definition = { - ...echoTool, - name: 'invalid-parameters', - parameters, - } as unknown as typeof echoTool - - expect(() => ctx.tools.register(definition)).toThrow( - 'tool parameters must be losslessly JSON-serializable', - ) - expect(ctx.tools.get('invalid-parameters')).toBeUndefined() - }) - - it('reads nested tool parameters once into the accepted snapshot', async () => { - const ctx = await setup() - let reads = 0 - const parameters = Object.defineProperty({}, 'properties', { - enumerable: true, - get: () => ++reads === 1 ? {} : new Map([['mutable', true]]), - }) - - expect(() => ctx.tools.register({ - ...echoTool, - name: 'unstable-parameters', - parameters, - })).not.toThrow() - expect(reads).toBe(1) - expect(ctx.tools.get('unstable-parameters')?.parameters).toEqual({ properties: {} }) - }) - - it('reads a top-level parameters accessor once so validation and storage use one value', async () => { - const ctx = await setup() - const accepted = { type: 'object', properties: { accepted: { type: 'string' } } } - class DriftedParameters { - readonly type = 'object' - readonly properties = { drifted: { type: 'number' } } - } - let reads = 0 - const definition = { ...echoTool, name: 'top-level-parameters' } - Object.defineProperty(definition, 'parameters', { - enumerable: true, - get: () => { - reads += 1 - return reads === 1 ? accepted : new DriftedParameters() - }, - }) - - ctx.tools.register(definition) - - expect(reads).toBe(1) - expect(ctx.tools.get('top-level-parameters')?.parameters).toEqual(accepted) - }) - - it('rejects malformed fixed definition fields without freezing caller objects', async () => { - const ctx = await setup() - const badName = { value: 'object-name' } - const badDescription = { value: 'object-description' } - const badTimeout = { value: 100 } - - expect(() => ctx.tools.register({ ...echoTool, name: badName as unknown as string })) - .toThrow('tool name must be a string') - expect(() => ctx.tools.register({ ...echoTool, name: 'bad-description', description: badDescription as unknown as string })) - .toThrow('description must be a string') - expect(() => ctx.tools.register({ ...echoTool, name: 'bad-timeout', timeoutMs: badTimeout as unknown as number })) - .toThrow('timeoutMs must be a positive finite number') expect(() => ctx.tools.register({ ...echoTool, name: 'zero-timeout', timeoutMs: 0 })) .toThrow('timeoutMs must be a positive finite number') - expect(() => ctx.tools.register({ ...echoTool, name: 'bad-execute', execute: { bind() {} } as unknown as typeof echoTool.execute })) - .toThrow('execute must be a function') - expect(() => ctx.tools.register({ ...echoTool, name: 'bad-present-call', presentCall: 1 as unknown as NonNullable })) - .toThrow('presentCall must be a function') - expect(() => ctx.tools.register({ ...echoTool, name: 'bad-present-result', presentResult: 1 as unknown as NonNullable })) - .toThrow('presentResult must be a function') - expect(Object.isFrozen(badName)).toBe(false) - expect(Object.isFrozen(badDescription)).toBe(false) - expect(Object.isFrozen(badTimeout)).toBe(false) - expect(ctx.tools.schemas()).toEqual([]) - }) - - it('snapshots callbacks while preserving their registration-time method receiver', async () => { - const ctx = await setup() - const receivers: object[] = [] - const definition = { - ...echoTool, - name: 'callback-snapshot', - async execute() { - receivers.push(this) - return [{ type: 'text' as const, text: 'original' }] - }, - } - ctx.tools.register(definition) - definition.execute = async () => [{ type: 'text' as const, text: 'replacement' }] - - const result = await ctx.tools.execute({ - callId: CallId('callback-snapshot'), name: definition.name, arguments: {}, - }) - - expect(receivers).toEqual([definition]) - expect(result.content).toEqual([{ type: 'text', text: 'original' }]) + expect(() => ctx.tools.register({ ...echoTool, name: 'infinite-timeout', timeoutMs: Number.POSITIVE_INFINITY })) + .toThrow('timeoutMs must be a positive finite number') }) it('rejects duplicate names and unregisters on fiber dispose (HMR safety)', async () => { @@ -1305,101 +825,6 @@ describe('defineTool / schema DSL', () => { expect(result.content).toEqual([{ type: 'text', text: 'HELLO' }]) }) - it('reads defineTool options once and keeps wire and runtime schemas on one detached snapshot', async () => { - const accepted: SchemaSpec = { value: { type: 'string', required: true, enum: ['accepted'] } } - const drifted: SchemaSpec = { count: { type: 'number', required: true } } - const reads = { - name: 0, - description: 0, - parameters: 0, - timeoutMs: 0, - execute: 0, - presentCall: 0, - presentResult: 0, - } - const options = {} as DefineToolOptions - Object.defineProperties(options, { - name: { enumerable: true, get: () => { reads.name += 1; return reads.name === 1 ? 'accepted' : 'drifted' } }, - description: { enumerable: true, get: () => { reads.description += 1; return reads.description === 1 ? 'accepted description' : 'drifted description' } }, - parameters: { enumerable: true, get: () => { reads.parameters += 1; return reads.parameters === 1 ? accepted : drifted } }, - timeoutMs: { enumerable: true, get: () => { reads.timeoutMs += 1; return reads.timeoutMs === 1 ? 250 : 0 } }, - execute: { - enumerable: true, - get: () => { - reads.execute += 1 - return (args: Record) => Promise.resolve([{ type: 'text' as const, text: String(args['value']) }]) - }, - }, - presentCall: { - enumerable: true, - get: () => { - reads.presentCall += 1 - return (args: Record) => ({ card: 'generic' as const, title: String(args['value']) }) - }, - }, - presentResult: { - enumerable: true, - get: () => { - reads.presentResult += 1 - return (args: Record) => ({ card: 'generic' as const, title: String(args['value']) }) - }, - }, - }) - - const tool = defineTool(options) - accepted.value!.type = 'number' - accepted.value!.enum!.push('mutated') - - expect(tool).toMatchObject({ - name: 'accepted', - description: 'accepted description', - timeoutMs: 250, - parameters: { - type: 'object', - properties: { value: { type: 'string', enum: ['accepted'] } }, - required: ['value'], - }, - }) - await expect(tool.execute({ value: 'accepted' }, {} as ToolExecution)) - .resolves.toEqual([{ type: 'text', text: 'accepted' }]) - expect(tool.presentCall?.({ value: 'accepted' })).toEqual({ card: 'generic', title: 'accepted' }) - expect(tool.presentResult?.( - { value: 'accepted' }, - { content: [], isError: false }, - )).toEqual({ card: 'generic', title: 'accepted' }) - expect(reads).toEqual({ - name: 1, - description: 1, - parameters: 1, - timeoutMs: 1, - execute: 1, - presentCall: 1, - presentResult: 1, - }) - }) - - it('rejects an exotic defineTool schema before it can be normalized for the wire', () => { - class ExoticDefault { readonly value = 'not JSON' } - - expect(() => defineTool({ - name: 'exotic-schema', - description: 'must reject exotic defaults', - parameters: { - value: { type: 'string', default: new ExoticDefault() }, - }, - execute: () => Promise.resolve([]), - })).toThrow(/parameters must be losslessly JSON-serializable/) - }) - - it('rejects a malformed defineTool spec whose generated wire schema is not JSON', () => { - expect(() => defineTool({ - name: 'malformed-schema', - description: 'missing property type', - parameters: { value: {} } as unknown as SchemaSpec, - execute: () => Promise.resolve([]), - })).toThrow(/generated parameters must be losslessly JSON-serializable/) - }) - it('type-level: InferArgs maps required properties to non-optional', () => { // Compile-time check: if this compiles, InferArgs is correct. // args.a is string (required), args.b is number|undefined (optional). diff --git a/packages/skill/skill/README.md b/packages/skill/skill/README.md index b035acf767..ebfac17e2e 100644 --- a/packages/skill/skill/README.md +++ b/packages/skill/skill/README.md @@ -8,28 +8,28 @@ This package owns the `ctx.skills` interface. It does not know whether skills co ### Public API -- `ctx.skills.registerProvider(provider): () => Promise | void` Registers a provider by unique `provider.name`. Duplicate provider names throw, and `runtime` is reserved for `ctx.skills.register(...)`. The registry snapshots the name and callback identities at registration, so replacing those fields later cannot change lookup or HMR cleanup; callbacks remain bound to the original provider object and can still read its mutable state. The registration is effect-scoped and HMR-safe, and the exact Cordis disposer supports ordered composite teardown. -- `ctx.skills.list({ cwd?, signal? })` Snapshots the lookup options, then returns detached model-invocable summaries for the current workspace, merged across providers and sorted by name. -- `ctx.skills.get(name, { cwd?, signal? })` Uses one lookup-options snapshot to select and load the winner, rechecks cancellation after discovery or a cache hit, races provider loading against the same signal, then returns a detached full definition, including disabled-for-model skills. -- `ctx.skills.register(skill): () => Promise | void` Registers a detached runtime embedded skill. Same-name runtime registrations are first-wins: a duplicate logs a warning and gets a no-op disposer. Successful registrations return the exact Cordis disposer for ordered composite teardown. +- `ctx.skills.registerProvider(provider): () => Promise | void` Registers a readonly provider by unique `provider.name`. Duplicate provider names throw, and `runtime` is reserved for `ctx.skills.register(...)`. The registry borrows the provider object and invokes its methods directly. The registration is effect-scoped and HMR-safe, and the exact Cordis disposer supports ordered composite teardown. +- `ctx.skills.list({ cwd?, signal? })` Borrows the readonly lookup options, then returns model-invocable summaries for the current workspace, merged across providers and sorted by name. +- `ctx.skills.get(name, { cwd?, signal? })` Uses the same readonly options and winning candidate for discovery and loading, rechecks cancellation after discovery or a cache hit, races provider loading against the signal, validates the loaded definition, then returns it, including disabled-for-model skills. +- `ctx.skills.register(skill): () => Promise | void` Registers a readonly runtime embedded skill, adding `provider: "runtime"` when omitted. Same-name runtime registrations are first-wins: a duplicate logs a warning and gets a no-op disposer. Successful registrations return the exact Cordis disposer for ordered composite teardown. ### Config | Field | Default | Meaning | |---|---|---| -| `collectCacheMaxEntries` | `128` | Maximum completed cwd/provider catalog snapshots kept in memory. | +| `collectCacheMaxEntries` | `128` | Maximum completed cwd/provider catalogs kept in memory. | ## Provider Contract -A provider registers synchronously from its `apply()` and returns `SkillCandidate[]` from `list(options)` when discovery is requested. Registration copies `name` and binds the current `list` and `get` methods once; replacing those fields on the caller-owned object later does not rewrite the live registry entry, and disposal always removes the original name. Remote setup, authentication, and discovery belong in the awaited `list()` call rather than plugin registration. Providers should stop promptly when `options.signal` aborts; the registry also stops awaiting uncooperative discovery and loading work so agent cancellation cannot hang prefix composition or skill loading. +A provider registers synchronously from its `apply()` and returns `readonly SkillCandidate[]` from `list(options)` when discovery is requested. The provider, lookup options, candidates, and loaded definitions are readonly same-process contracts: the registry borrows them rather than cloning, freezing, or rebinding callbacks. Remote setup, authentication, and discovery belong in the awaited `list()` call rather than plugin registration. Providers should stop promptly when `options.signal` aborts; the registry also stops awaiting uncooperative discovery and loading work so agent cancellation cannot hang prefix composition or skill loading. -Each public lookup captures `cwd` and the abort-signal identity once before cache or provider work, and providers receive that frozen lookup record. The registry reads each returned candidate once, validates that snapshot, and detaches its resource metadata before caching it. The winning provider receives another detached candidate in `get(candidate, options)`, while `candidate.locator` preserves the exact provider-owned identity originally returned by `list()`; a local provider can therefore use a file-path handle while a remote provider can use a URL, id, or version token. A loaded definition is detached again before it reaches the caller. +The registry validates parsed provider candidates before caching them and validates loaded definitions before returning them. The winning provider receives the exact candidate and opaque `locator` identity it returned from `list()`; a local provider can therefore use a file-path handle while a remote provider can use a URL, id, or version token. Callers and providers must honor the readonly contract after handing values to the registry. -The registry validates fixed provider, candidate, runtime-registration, and loaded-definition fields before detachment: names/descriptions/content use their declared string types, ranks are finite numbers, and `disableModelInvocation` is boolean when present. Caller-owned objects masquerading as scalars are rejected without being frozen. Candidate contract violations fail fast because the provider plugin is malformed; a provider `list()` rejection is treated as a transient source failure, logged, skipped for that request, and not cached. Only completed, registry-owned catalogs are cached, and a provider/runtime revision change during discovery discards the stale result and retries. Duplicate skill names are resolved first-wins by `rank`, provider registration order, then the provider's own local order. The final summary list is sorted by skill `name` for deterministic consumers. +Parsed candidate and loaded-definition fields are validated at the provider boundary: names/descriptions/content use their declared string types, ranks are finite numbers, and `disableModelInvocation` is boolean when present. Candidate contract violations fail fast because the provider or its parser is malformed; a provider `list()` rejection is treated as a transient source failure, logged, skipped for that request, and not cached. Only completed catalogs are cached, and a provider/runtime revision change during discovery discards the stale result and retries. Duplicate skill names are resolved first-wins by `rank`, provider registration order, then the provider's own local order. The final summary list is sorted by skill `name` for deterministic consumers. ## Runtime Skills -`ctx.skills.register(...)` is a convenience for embedded runtime skills. Runtime skills use rank `250`: project providers can override them, while they override the shipped local provider's custom and user roots. Runtime registration detaches the accepted definition and nested resource metadata; later mutation of the registration object or a returned list/get value cannot rewrite the live skill. Registration is also first-wins within runtime contributions, so a duplicate contribution cannot remove the active one through its disposer. +`ctx.skills.register(...)` is a convenience for embedded runtime skills. Runtime skills use rank `250`: project providers can override them, while they override the shipped local provider's custom and user roots. Runtime definitions and nested resource metadata are borrowed readonly; the service only materializes the top-level definition needed to supply the default `provider`. Registration is first-wins within runtime contributions, so a duplicate contribution cannot remove the active one through its disposer. ## Consumer boundary diff --git a/packages/skill/skill/src/index.ts b/packages/skill/skill/src/index.ts index f06788cde7..af4af6f7a2 100644 --- a/packages/skill/skill/src/index.ts +++ b/packages/skill/skill/src/index.ts @@ -32,56 +32,56 @@ export type SkillSource = 'project-dsh' | 'project-agents' | 'runtime' | 'user-d /** Optional provider-specific base used by loaded skill bodies to resolve relative resources. */ export type SkillResourceBase = - | { kind: 'directory'; path: string } - | { kind: 'url'; url: string } - | { kind: 'opaque'; description: string } + | { readonly kind: 'directory'; readonly path: string } + | { readonly kind: 'url'; readonly url: string } + | { readonly kind: 'opaque'; readonly description: string } /** Model-visible skill metadata returned by `ctx.skills.list()` and rendered into request guidance. */ export interface SkillSummary { /** Kebab-case identifier used with the `skill` tool. */ - name: string + readonly name: string /** Short routing description shown to the model. */ - description: string + readonly description: string /** Optional extra routing guidance shown to the model. */ - whenToUse?: string + readonly whenToUse?: string /** Whether the skill is hidden from model listings while remaining loadable by trusted callers. */ - disableModelInvocation?: boolean + readonly disableModelInvocation?: boolean /** Discovery source that produced this winning skill. */ - source: SkillSource + readonly source: SkillSource /** Provider that owns this skill body. */ - provider: string + readonly provider: string /** Provider-specific base for relative resources. */ - resourceBase?: SkillResourceBase + readonly resourceBase?: SkillResourceBase } /** Provider catalog entry used by the registry to merge and later load skills. */ export interface SkillCandidate extends SkillSummary { /** Lower ranks win duplicate skill names before provider registration order is considered. */ - rank: number + readonly rank: number /** Opaque provider-owned handle passed back to `provider.get()`. */ - locator: unknown + readonly locator: unknown /** Absolute file path when the provider has one. */ - path?: string + readonly path?: string /** Parsed optional metadata object from provider-specific skill frontmatter. */ - metadata?: Record + readonly metadata?: Readonly> } /** Complete parsed skill definition, including the body loaded by `ctx.skills.get()`. */ export interface SkillDefinition extends SkillSummary { /** Markdown instruction body after any provider-specific metadata removal. */ - content: string + readonly content: string /** Absolute file path when the skill came from disk. */ - path?: string + readonly path?: string /** Parsed optional metadata object from frontmatter. */ - metadata?: Record + readonly metadata?: Readonly> } /** Runtime skill contribution accepted by `ctx.skills.register()`. */ -export type SkillRegistration = Omit & { provider?: string } +export type SkillRegistration = Omit & { readonly provider?: string } /** Caller context used for cwd-sensitive and abortable provider work. */ export interface SkillLookupOptions { - /** Workspace selector captured at lookup entry; providers receive a read-only snapshot. */ + /** Workspace selector for the current lookup. */ readonly cwd?: string | undefined /** Abort discovery or loading work for the current caller. */ readonly signal?: AbortSignal | undefined @@ -90,7 +90,7 @@ export interface SkillLookupOptions { /** Provider interface for one source of skills, such as local directories or a remote registry. */ export interface SkillProvider { /** Unique provider name in the `ctx.skills` registry. */ - name: string + readonly name: string /** * List available skill candidates for the current lookup context. Provider * plugins register synchronously during `apply()`; remote initialization, @@ -99,21 +99,20 @@ export interface SkillProvider { * @param options - lookup options; `cwd` selects workspace-sensitive skills and `signal` cancels work. * @returns provider candidates with precedence ranks and opaque locators. */ - list(options: SkillLookupOptions): Promise + readonly list: (options: SkillLookupOptions) => Promise /** * Load a complete skill body for a previously listed candidate. - * @param candidate - a detached snapshot of the winning candidate; its opaque - * `locator` retains the exact identity originally returned by this provider. + * @param candidate - the winning candidate originally returned by this provider. * @param options - lookup options; `cwd` selects workspace-sensitive skills and `signal` cancels work. * @returns the full skill body, or `undefined` if it is no longer loadable. */ - get(candidate: SkillCandidate, options: SkillLookupOptions): Promise + readonly get: (candidate: SkillCandidate, options: SkillLookupOptions) => Promise } /** Skill registry configuration. */ export interface Config { - /** Maximum number of completed cwd/provider catalog snapshots kept in memory. */ - collectCacheMaxEntries?: number + /** Maximum number of completed cwd/provider catalogs kept in memory. */ + readonly collectCacheMaxEntries?: number } declare module 'cordis' { @@ -163,7 +162,7 @@ export class SkillService extends Service { private readonly collectCacheMaxEntries: number private readonly providers = new Map() - private readonly runtime = new Map() + private readonly runtime = new Map() private readonly collectCache = new Map() private providerRevision = 0 private nextProviderOrder = 0 @@ -179,53 +178,38 @@ export class SkillService extends Service { * Register a skill provider synchronously during the provider plugin's * `apply()`. Throws if another provider already owns the same provider name, * including the reserved runtime provider name. Providers that need remote - * initialization do that work inside `list()` after registration. The name - * and callback identities are snapshotted at registration, so later - * replacement of those fields cannot change the registry key, dispatch - * callbacks, or HMR cleanup identity. Bound callbacks retain the original - * provider object as their receiver, so provider-owned mutable state remains - * live. Effect-scoped and HMR-safe: disposing the caller's fiber unregisters - * the provider and invalidates cached catalogs. + * initialization do that work inside `list()` after registration. Providers + * are readonly same-process registrations: the registry borrows the provider + * object and invokes its methods directly. Effect-scoped and HMR-safe: + * disposing the caller's fiber unregisters the provider and invalidates + * cached catalogs. * @param provider - the provider to register by `provider.name`. * @returns the exact Cordis effect disposer that unregisters this provider; * composite effects may yield it directly to preserve teardown ordering. */ registerProvider(provider: SkillProvider): () => Promise | void { - // Snapshot the registration contract before entering the effect. The - // callback binding preserves the historical method receiver while making - // replacement of `provider.list`/`provider.get` after registration inert. - // In particular, cleanup must never re-read caller-owned `provider.name`: - // an HMR host may mutate or reuse that object before its old fiber unloads. const name = provider.name - // eslint-disable-next-line @typescript-eslint/unbound-method - const inputList = provider.list - // eslint-disable-next-line @typescript-eslint/unbound-method - const inputGet = provider.get - if (typeof name !== 'string') throw new TypeError('skill provider name must be a string') - if (typeof inputList !== 'function') throw new TypeError(`skill provider "${name}" list must be a function`) - if (typeof inputGet !== 'function') throw new TypeError(`skill provider "${name}" get must be a function`) - const snapshot: SkillProvider = Object.freeze({ - name, - list: inputList.bind(provider), - get: inputGet.bind(provider), - }) - const dispose = this.ctx.effect(function* (this: SkillService) { - if (snapshot.name === RUNTIME_PROVIDER) { - throw new Error(`"${RUNTIME_PROVIDER}" is reserved for runtime skill registrations`) - } - if (this.providers.has(snapshot.name)) { - throw new Error(`a skill provider named "${snapshot.name}" is already registered`) - } - this.providers.set(snapshot.name, { provider: snapshot, order: this.nextProviderOrder }) - this.nextProviderOrder += 1 - this.invalidateCache() + if (name === RUNTIME_PROVIDER) { + throw new Error(`"${RUNTIME_PROVIDER}" is reserved for runtime skill registrations`) + } + if (this.providers.has(name)) { + throw new Error(`a skill provider named "${name}" is already registered`) + } + const providers = this.providers + const ctx = this.ctx + const order = this.nextProviderOrder + const invalidateCache = (): void => { this.invalidateCache() } + this.nextProviderOrder += 1 + const dispose = ctx.effect(function* () { + providers.set(name, { provider, order }) + invalidateCache() yield () => { - this.providers.delete(snapshot.name) - this.invalidateCache() - this.ctx.emit('skill/provider-removed', snapshot.name) + providers.delete(name) + invalidateCache() + ctx.emit('skill/provider-removed', name) } - this.ctx.emit('skill/provider-added', snapshot) - }.bind(this), 'skills.registerProvider()') + ctx.emit('skill/provider-added', provider) + }, 'skills.registerProvider()') return dispose } @@ -233,44 +217,46 @@ export class SkillService extends Service { * Register a runtime skill contribution. Runtime registrations are treated as * embedded provider entries with project-over-user priority. Same-name runtime * registrations are first-wins: a duplicate logs a warning and gets a no-op - * disposer so it cannot remove the active contribution. The registry detaches - * the accepted definition, including nested resource metadata, so later caller - * mutation cannot rewrite the live contribution. + * disposer so it cannot remove the active contribution. Runtime definitions + * are readonly same-process registrations; the registry borrows their nested + * resource metadata. * @param skill - the complete skill definition to expose for discovery. * @returns the exact Cordis effect disposer that removes this runtime * contribution and invalidates caches; composite effects may yield it * directly to preserve teardown ordering. */ register(skill: SkillRegistration): () => Promise | void { - const normalized = normalizeRuntimeSkill(skill) - const existing = this.runtime.get(normalized.name) + validateRuntimeSkill(skill) + const existing = this.runtime.get(skill.name) if (existing !== undefined) { - this.ctx.logger.warn(`runtime skill "${normalized.name}" ignored because it is already registered`) + this.ctx.logger.warn(`runtime skill "${skill.name}" ignored because it is already registered`) return () => {} } - const dispose = this.ctx.effect(function* (this: SkillService) { - this.runtime.set(normalized.name, normalized) - this.runtimeRevision += 1 - this.invalidateCache() + const runtime = this.runtime + const updateRevision = (): void => { this.runtimeRevision += 1 } + const invalidateCache = (): void => { this.invalidateCache() } + const dispose = this.ctx.effect(function* () { + runtime.set(skill.name, skill) + updateRevision() + invalidateCache() yield () => { - this.runtime.delete(normalized.name) - this.runtimeRevision += 1 - this.invalidateCache() + runtime.delete(skill.name) + updateRevision() + invalidateCache() } - }.bind(this), 'skills.register()') + }, 'skills.register()') return dispose } /** - * List model-invocable skill summaries for a workspace. The lookup options are - * snapshotted before discovery, and every returned summary is detached from the - * cached provider catalog. + * List model-invocable skill summaries for a workspace. Lookup options and + * provider candidates are readonly same-process values borrowed throughout + * discovery. * @param options - lookup options; `cwd` selects project roots and `signal` cancels discovery. * @returns sorted summaries, excluding skills disabled for model invocation. */ async list(options: SkillLookupOptions = {}): Promise { - const accepted = snapshotLookupOptions(options) - return (await this.collect(accepted)) + return (await this.collect(options)) .map(entry => entry.candidate) .filter(skill => skill.disableModelInvocation !== true) .map(toSummary) @@ -278,10 +264,10 @@ export class SkillService extends Service { } /** - * Load one full skill definition by name. One lookup-options snapshot selects - * and loads the winner; the provider receives detached candidate metadata with - * its opaque locator identity preserved, and the returned definition is also - * detached from provider-owned data. Cancellation is rechecked after catalog + * Load one full skill definition by name. The provider receives the winning + * candidate it returned during discovery, including its opaque locator, and + * the registry returns the provider's definition after validating it. + * Cancellation is rechecked after catalog * selection (including a cache hit), and provider loading is raced against the * same signal so an uncooperative provider cannot hang the caller. * @param name - kebab-case skill name. @@ -290,16 +276,17 @@ export class SkillService extends Service { */ async get(name: string, options: SkillLookupOptions = {}): Promise { if (!isSkillName(name)) return undefined - const accepted = snapshotLookupOptions(options) - const collected = await this.collect(accepted) - throwIfAborted(accepted.signal) + const collected = await this.collect(options) + throwIfAborted(options.signal) const match = collected.find(entry => entry.candidate.name === name) if (match === undefined) return undefined const definition = await waitWithAbort( - match.provider.get(copyCandidate(match.candidate), accepted), - accepted.signal, + match.provider.get(match.candidate, options), + options.signal, ) - return definition === undefined ? undefined : snapshotDefinition(definition) + if (definition === undefined) return undefined + validateDefinition(definition) + return definition } private async collect(options: SkillLookupOptions): Promise { @@ -358,21 +345,22 @@ export class SkillService extends Service { } for (const { provider, order } of [...this.providers.values()]) { let localOrder = 0 - let listed: SkillCandidate[] | undefined + let output: unknown try { - listed = await waitWithAbort(provider.list(options), options.signal) + output = await waitWithAbort(provider.list(options), options.signal) } catch (error) { if (options.signal?.aborted === true) throw toError(options.signal.reason) cacheable = false this.ctx.logger.warn(`skill provider "${provider.name}" skipped: ${errorMessage(error)}`) } - if (listed === undefined) continue - if (!Array.isArray(listed)) { + if (output === undefined) continue + if (!Array.isArray(output)) { throw new TypeError(`skill provider "${provider.name}" list() must return an array`) } + const listed = output as readonly SkillCandidate[] for (const candidate of listed) { - const snapshot = snapshotCandidate(candidate, provider.name) - candidates.push({ candidate: snapshot, provider, providerOrder: order, localOrder }) + validateCandidate(candidate, provider.name) + candidates.push({ candidate, provider, providerOrder: order, localOrder }) localOrder += 1 } } @@ -392,14 +380,20 @@ const RUNTIME_SKILL_PROVIDER: SkillProvider = { return Promise.resolve([]) }, get(candidate) { - const skill = candidate.locator as SkillDefinition - return Promise.resolve({ ...skill }) + const skill = candidate.locator as SkillRegistration + return Promise.resolve({ ...skill, provider: skill.provider ?? RUNTIME_PROVIDER }) }, } -function runtimeCandidate(skill: SkillDefinition): SkillCandidate { +function runtimeCandidate(skill: SkillRegistration): SkillCandidate { return { - ...toSummary(skill), + name: skill.name, + description: skill.description, + ...skill.whenToUse !== undefined ? { whenToUse: skill.whenToUse } : {}, + ...skill.disableModelInvocation !== undefined ? { disableModelInvocation: skill.disableModelInvocation } : {}, + source: skill.source, + provider: skill.provider ?? RUNTIME_PROVIDER, + ...skill.resourceBase !== undefined ? { resourceBase: skill.resourceBase } : {}, rank: RUNTIME_RANK, locator: skill, ...skill.path !== undefined ? { path: skill.path } : {}, @@ -407,50 +401,6 @@ function runtimeCandidate(skill: SkillDefinition): SkillCandidate { } } -/** Read provider candidate data once and detach it while preserving its opaque locator identity. */ -function copyCandidate(candidate: SkillCandidate, providerName?: string): SkillCandidate { - const name = candidate.name - const description = candidate.description - const whenToUse = candidate.whenToUse - const disableModelInvocation = candidate.disableModelInvocation - const source = candidate.source - const provider = candidate.provider - const resourceBase = candidate.resourceBase - const rank = candidate.rank - const locator = candidate.locator - const path = candidate.path - const metadata = candidate.metadata - const accepted: SkillCandidate = { - name, - description, - ...whenToUse !== undefined ? { whenToUse } : {}, - ...disableModelInvocation !== undefined ? { disableModelInvocation } : {}, - source, - provider, - ...resourceBase !== undefined ? { resourceBase } : {}, - rank, - // `locator` is the one deliberately provider-owned capability in a - // candidate. Its exact identity must round-trip back to provider.get(). - locator, - ...path !== undefined ? { path } : {}, - ...metadata !== undefined ? { metadata } : {}, - } - // Validate the exact scalar snapshot before cloning nested data. This keeps a - // malformed candidate's provider-contract error from being masked by an - // unrelated DataCloneError in its metadata. - if (providerName !== undefined) validateCandidate(accepted, providerName) - return { - ...accepted, - ...resourceBase !== undefined ? { resourceBase: structuredClone(resourceBase) } : {}, - ...metadata !== undefined ? { metadata: structuredClone(metadata) } : {}, - } -} - -/** Normalize one provider result into the registry-owned catalog snapshot. */ -function snapshotCandidate(candidate: SkillCandidate, providerName: string): SkillCandidate { - return copyCandidate(candidate, providerName) -} - function validateCandidate(candidate: SkillCandidate, providerName: string): void { if (typeof candidate.name !== 'string') { throw new TypeError(`skill provider "${providerName}" returned a non-string skill name`) @@ -487,58 +437,21 @@ function validateCandidate(candidate: SkillCandidate, providerName: string): voi } } -function normalizeRuntimeSkill(skill: SkillRegistration): SkillDefinition { - // Read every caller-owned top-level field once so validation and storage use - // one coherent definition even when JavaScript accessors are involved. - const name = skill.name - const description = skill.description - const whenToUse = skill.whenToUse - const disableModelInvocation = skill.disableModelInvocation - const source = skill.source - const inputProvider = skill.provider - const provider = inputProvider === undefined ? RUNTIME_PROVIDER : inputProvider - const resourceBase = skill.resourceBase - const content = skill.content - const path = skill.path - const metadata = skill.metadata - if (typeof name !== 'string') throw new TypeError('runtime skill name must be a string') - if (!SKILL_NAME.test(name)) throw new Error(`invalid skill name "${name}"`) - if (typeof description !== 'string') throw new TypeError(`skill "${name}" description must be a string`) - if (description.length === 0) throw new Error(`skill "${name}" requires a description`) - if (disableModelInvocation !== undefined && typeof disableModelInvocation !== 'boolean') { - throw new TypeError(`skill "${name}" disableModelInvocation must be a boolean`) - } - if (whenToUse !== undefined && typeof whenToUse !== 'string') throw new TypeError(`skill "${name}" whenToUse must be a string`) - if (typeof source !== 'string') throw new TypeError(`skill "${name}" source must be a string`) - if (typeof provider !== 'string') throw new TypeError(`skill "${name}" provider must be a string`) - if (typeof content !== 'string') throw new TypeError(`skill "${name}" content must be a string`) - if (path !== undefined && typeof path !== 'string') throw new TypeError(`skill "${name}" path must be a string`) - return { - name, - description, - ...whenToUse !== undefined ? { whenToUse } : {}, - ...disableModelInvocation !== undefined ? { disableModelInvocation } : {}, - source, - provider, - ...resourceBase !== undefined ? { resourceBase: structuredClone(resourceBase) } : {}, - content, - ...path !== undefined ? { path } : {}, - ...metadata !== undefined ? { metadata: structuredClone(metadata) } : {}, - } +function validateRuntimeSkill(skill: SkillRegistration): void { + if (!SKILL_NAME.test(skill.name)) throw new Error(`invalid skill name "${skill.name}"`) + if (skill.description.length === 0) throw new Error(`skill "${skill.name}" requires a description`) } -/** Detach a provider-loaded definition before it crosses back to the caller. */ -function snapshotDefinition(skill: SkillDefinition): SkillDefinition { +/** Validate a definition loaded from a provider-controlled parser or remote source. */ +function validateDefinition(skill: SkillDefinition): void { const name = skill.name const description = skill.description const whenToUse = skill.whenToUse const disableModelInvocation = skill.disableModelInvocation const source = skill.source const provider = skill.provider - const resourceBase = skill.resourceBase const content = skill.content const path = skill.path - const metadata = skill.metadata if (typeof name !== 'string') throw new TypeError('loaded skill name must be a string') if (!SKILL_NAME.test(name)) throw new Error(`loaded skill has invalid name "${name}"`) if (typeof description !== 'string') throw new TypeError(`loaded skill "${name}" description must be a string`) @@ -551,18 +464,6 @@ function snapshotDefinition(skill: SkillDefinition): SkillDefinition { if (typeof provider !== 'string') throw new TypeError(`loaded skill "${name}" provider must be a string`) if (typeof content !== 'string') throw new TypeError(`loaded skill "${name}" content must be a string`) if (path !== undefined && typeof path !== 'string') throw new TypeError(`loaded skill "${name}" path must be a string`) - return { - name, - description, - ...whenToUse !== undefined ? { whenToUse } : {}, - ...disableModelInvocation !== undefined ? { disableModelInvocation } : {}, - source, - provider, - ...resourceBase !== undefined ? { resourceBase: structuredClone(resourceBase) } : {}, - content, - ...path !== undefined ? { path } : {}, - ...metadata !== undefined ? { metadata: structuredClone(metadata) } : {}, - } } function toSummary(skill: SkillDefinition | SkillCandidate): SkillSummary { @@ -574,7 +475,7 @@ function toSummary(skill: SkillDefinition | SkillCandidate): SkillSummary { ...disableModelInvocation !== undefined ? { disableModelInvocation } : {}, source, provider, - ...resourceBase !== undefined ? { resourceBase: structuredClone(resourceBase) } : {}, + ...resourceBase !== undefined ? { resourceBase } : {}, } } @@ -604,16 +505,6 @@ function collectCacheKey(options: SkillLookupOptions, providerRevision: number, return JSON.stringify({ cwd: options.cwd, providerRevision, runtimeRevision }) } -/** Capture one lookup identity before any provider or cache async boundary. */ -function snapshotLookupOptions(options: SkillLookupOptions): Readonly { - const cwd = options.cwd - const signal = options.signal - return Object.freeze({ - ...cwd !== undefined ? { cwd } : {}, - ...signal !== undefined ? { signal } : {}, - }) -} - function waitWithAbort(promise: Promise, signal: AbortSignal | undefined): Promise { if (signal === undefined) return promise throwIfAborted(signal) diff --git a/packages/skill/skill/tests/skill.spec.ts b/packages/skill/skill/tests/skill.spec.ts index b47bdb6431..90bf1ea983 100644 --- a/packages/skill/skill/tests/skill.spec.ts +++ b/packages/skill/skill/tests/skill.spec.ts @@ -107,85 +107,9 @@ describe('SkillService registry', () => { expect((await ctx.skills.list()).map(skill => skill.name)).toEqual(['same-rank-skill', 'shadowed']) }) - it('snapshots a provider registration so caller mutation cannot corrupt HMR cleanup', async () => { + it('validates parsed candidate fields', async () => { const ctx = new Context() await ctx.plugin(SkillService) - const candidate: SkillCandidate = { - name: 'stable-skill', - description: 'Stable skill', - provider: 'stable-provider', - source: 'test', - rank: 1, - locator: 'original', - } - const originalList = vi.fn(() => Promise.resolve([candidate])) - const originalGet = vi.fn((listed: SkillCandidate) => Promise.resolve({ - ...listed, - content: 'Original body.', - })) - const provider: SkillProvider = { - name: 'stable-provider', - list: originalList, - get: originalGet, - } - const added: SkillProvider[] = [] - const removed: string[] = [] - ctx.on('skill/provider-added', (registered) => { added.push(registered) }) - ctx.on('skill/provider-removed', (name) => { removed.push(name) }) - const owner = await ctx.plugin({ - name: 'mutable-provider-owner', - inject: ['skills'], - apply(pluginCtx: Context) { - pluginCtx.skills.registerProvider(provider) - }, - }) - - provider.name = 'mutated-provider' - const replacementList = vi.fn(() => Promise.resolve([])) - const replacementGet = vi.fn(() => Promise.resolve(undefined)) - provider.list = replacementList - provider.get = replacementGet - - expect(added).toHaveLength(1) - expect(added[0]).not.toBe(provider) - expect(added[0]?.name).toBe('stable-provider') - expect(Object.isFrozen(added[0])).toBe(true) - expect((await ctx.skills.list()).map(skill => skill.name)).toEqual(['stable-skill']) - expect((await ctx.skills.get('stable-skill'))?.content).toBe('Original body.') - expect(originalList).toHaveBeenCalledOnce() - expect(originalGet).toHaveBeenCalledOnce() - expect(replacementList).not.toHaveBeenCalled() - expect(replacementGet).not.toHaveBeenCalled() - - await owner.dispose() - expect(removed).toEqual(['stable-provider']) - expect(await ctx.skills.list()).toEqual([]) - const replacement = new MemoryProvider([]) - Object.defineProperty(replacement, 'name', { value: 'stable-provider' }) - expect(() => ctx.skills.registerProvider(replacement)).not.toThrow() - }) - - it('rejects malformed provider and candidate scalar fields without freezing caller objects', async () => { - const ctx = new Context() - await ctx.plugin(SkillService) - const badProviderName = { value: 'object-provider' } - expect(() => ctx.skills.registerProvider({ - name: badProviderName as unknown as string, - list: () => Promise.resolve([]), - get: () => Promise.resolve(undefined), - })).toThrow('skill provider name must be a string') - expect(Object.isFrozen(badProviderName)).toBe(false) - expect(() => ctx.skills.registerProvider({ - name: 'bad-list', - list: { bind() {} } as unknown as SkillProvider['list'], - get: () => Promise.resolve(undefined), - })).toThrow('list must be a function') - expect(() => ctx.skills.registerProvider({ - name: 'bad-get', - list: () => Promise.resolve([]), - get: { bind() {} } as unknown as SkillProvider['get'], - })).toThrow('get must be a function') - const badDescription = { value: 'object-description' } ctx.skills.registerProvider({ name: 'bad-candidate', @@ -198,7 +122,6 @@ describe('SkillService registry', () => { get: () => Promise.resolve(undefined), }) await expect(ctx.skills.list()).rejects.toThrow('non-string description') - expect(Object.isFrozen(badDescription)).toBe(false) const badBoolean = new Context() await badBoolean.plugin(SkillService) @@ -258,46 +181,37 @@ describe('SkillService registry', () => { } }) - it('snapshots lookup options before asynchronous discovery and loading', async () => { + it('borrows the exact lookup options through discovery and loading', async () => { const ctx = new Context() await ctx.plugin(SkillService) - let release: (() => void) | undefined - const gate = new Promise((resolve) => { release = resolve }) - const listCwds: (string | undefined)[] = [] - const getCwds: (string | undefined)[] = [] + const options: SkillLookupOptions = { cwd: '/workspace/a' } + let listedWith: SkillLookupOptions | undefined + let loadedWith: SkillLookupOptions | undefined + const candidate: SkillCandidate = { + name: 'skill-a', + description: 'Skill A', + provider: 'contextual', + source: 'test', + rank: 1, + locator: 'skill-a', + } ctx.skills.registerProvider({ name: 'contextual', - async list(options) { - listCwds.push(options.cwd) - await gate - const name = options.cwd === '/workspace/a' ? 'skill-a' : 'skill-b' - return [ - { name, description: name, provider: 'contextual', source: 'test', rank: 1, locator: name }, - { name: 'vanished', description: 'Vanished', provider: 'contextual', source: 'test', rank: 2, locator: 'vanished' }, - ] + async list(received) { + listedWith = received + return [candidate] }, - async get(candidate, options) { - getCwds.push(options.cwd) - if (candidate.name === 'vanished') return undefined - return { ...candidate, content: `${options.cwd}:${candidate.name}` } + async get(received, lookup) { + expect(received).toBe(candidate) + loadedWith = lookup + return { ...received, content: 'Skill A body.' } }, }) - const listOptions: { cwd: string | undefined } = { cwd: '/workspace/a' } - const pending = ctx.skills.list(listOptions) - listOptions.cwd = '/workspace/b' - release?.() - - expect((await pending).map(skill => skill.name)).toEqual(['skill-a', 'vanished']) - expect((await ctx.skills.list({ cwd: '/workspace/a' })).map(skill => skill.name)).toEqual(['skill-a', 'vanished']) - expect(listCwds).toEqual(['/workspace/a']) - - const getOptions: { cwd: string | undefined } = { cwd: '/workspace/a' } - const loading = ctx.skills.get('skill-a', getOptions) - getOptions.cwd = '/workspace/b' - expect((await loading)?.content).toBe('/workspace/a:skill-a') - expect(await ctx.skills.get('vanished', { cwd: '/workspace/a' })).toBeUndefined() - expect(getCwds).toEqual(['/workspace/a', '/workspace/a']) + expect((await ctx.skills.list(options)).map(skill => skill.name)).toEqual(['skill-a']) + expect(await ctx.skills.get('skill-a', options)).toMatchObject({ content: 'Skill A body.' }) + expect(listedWith).toBe(options) + expect(loadedWith).toBe(options) }) it('rechecks cancellation after cached discovery before provider loading', async () => { @@ -402,7 +316,7 @@ describe('SkillService registry', () => { expect(settled).toBe('aborted') }) - it('detaches cached candidates and loaded definitions while preserving locator identity', async () => { + it('borrows cached candidates and loaded definitions from the provider', async () => { const ctx = new Context() await ctx.plugin(SkillService) const locator = { id: 'provider-owned' } @@ -445,50 +359,27 @@ describe('SkillService registry', () => { }, }) - const first = await ctx.skills.list() - candidate.name = 'Bad_Name' - candidate.description = '' - if (candidate.resourceBase?.kind === 'opaque') candidate.resourceBase.description = 'mutated candidate' - if (candidate.metadata) candidate.metadata.owner = 'mutated candidate' - if (first[0]?.resourceBase?.kind === 'opaque') first[0].resourceBase.description = 'mutated summary' - - const second = await ctx.skills.list() - expect(second).toEqual([expect.objectContaining({ + const listed = await ctx.skills.list() + expect(listed).toEqual([expect.objectContaining({ name: 'stable-skill', description: 'Stable description', resourceBase: { kind: 'opaque', description: 'candidate resources' }, })]) + expect(listed[0]?.resourceBase).toBe(candidate.resourceBase) expect(listCalls).toBe(1) const loaded = await ctx.skills.get('stable-skill') - expect(received).not.toBe(candidate) + expect(received).toBe(candidate) expect(received?.locator).toBe(locator) - expect(received).toMatchObject({ - name: 'stable-skill', - description: 'Stable description', - resourceBase: { kind: 'opaque', description: 'candidate resources' }, - metadata: { owner: 'candidate' }, - }) - expect(loaded).not.toBe(definition) - if (loaded?.resourceBase?.kind === 'opaque') loaded.resourceBase.description = 'mutated definition output' - if (loaded?.metadata) loaded.metadata.owner = 'mutated definition output' - - expect(await ctx.skills.get('stable-skill')).toMatchObject({ - resourceBase: { kind: 'opaque', description: 'definition resources' }, - metadata: { owner: 'definition' }, - }) - expect(definition).toMatchObject({ - resourceBase: { kind: 'opaque', description: 'definition resources' }, - metadata: { owner: 'definition' }, - }) + expect(loaded).toBe(definition) }) - it('detaches runtime registrations and every public resource view', async () => { + it('preserves readonly runtime resource identities while adding the default provider', async () => { const ctx = new Context() await ctx.plugin(SkillService) const resourceBase = { kind: 'opaque' as const, description: 'runtime resources' } const metadata = { owner: 'runtime' } - ctx.skills.register({ + const registration = { name: 'runtime-skill', description: 'Runtime', whenToUse: 'When runtime data is needed.', @@ -497,101 +388,20 @@ describe('SkillService registry', () => { resourceBase, metadata, content: 'Runtime body.', - }) + } + ctx.skills.register(registration) ctx.skills.register({ name: 'z-runtime', description: 'Second runtime skill', source: 'runtime', content: 'Second runtime body.', }) - resourceBase.description = 'mutated registration' - metadata.owner = 'mutated registration' - const listed = await ctx.skills.list() const loaded = await ctx.skills.get('runtime-skill') - expect(listed[0]?.resourceBase).toEqual({ kind: 'opaque', description: 'runtime resources' }) - expect(loaded?.metadata).toEqual({ owner: 'runtime' }) - if (listed[0]?.resourceBase?.kind === 'opaque') listed[0].resourceBase.description = 'mutated list output' - if (loaded?.resourceBase?.kind === 'opaque') loaded.resourceBase.description = 'mutated get output' - if (loaded?.metadata) loaded.metadata.owner = 'mutated get output' - - expect((await ctx.skills.list())[0]?.resourceBase).toEqual({ kind: 'opaque', description: 'runtime resources' }) - expect(await ctx.skills.get('runtime-skill')).toMatchObject({ - resourceBase: { kind: 'opaque', description: 'runtime resources' }, - metadata: { owner: 'runtime' }, - }) - }) - - it('rejects malformed runtime and loaded-definition scalar fields without freezing them', async () => { - const ctx = new Context() - await ctx.plugin(SkillService) - const runtimeDescription = { value: 'runtime-description' } - expect(() => ctx.skills.register({ - name: 'bad-runtime', - description: runtimeDescription as unknown as string, - source: 'runtime', - content: 'body', - })).toThrow('description must be a string') - expect(Object.isFrozen(runtimeDescription)).toBe(false) - expect(() => ctx.skills.register({ - name: 'bad-runtime-boolean', - description: 'Runtime', - disableModelInvocation: 'false' as unknown as boolean, - source: 'runtime', - content: 'body', - })).toThrow('disableModelInvocation must be a boolean') - expect(() => ctx.skills.register({ - name: 'bad-runtime-provider', - description: 'Runtime', - source: 'runtime', - provider: null as unknown as string, - content: 'body', - })).toThrow('provider must be a string') - - const loadedContent = { value: 'loaded-content' } - ctx.skills.registerProvider({ - name: 'bad-definition', - list: () => Promise.resolve([{ - name: 'bad-definition', - description: 'Candidate', - provider: 'bad-definition', - source: 'test', - rank: 1, - locator: 'bad-definition', - }]), - get: candidate => Promise.resolve({ - ...candidate, - content: loadedContent as unknown as string, - }), - }) - await expect(ctx.skills.get('bad-definition')).rejects.toThrow('content must be a string') - expect(Object.isFrozen(loadedContent)).toBe(false) - }) - - it('rejects every other malformed runtime scalar', async () => { - const ctx = new Context() - await ctx.plugin(SkillService) - type Registration = Parameters[0] - const valid: Registration = { - name: 'runtime-validation', - description: 'Runtime validation', - whenToUse: 'Use this runtime skill.', - disableModelInvocation: false, - source: 'runtime', - provider: 'runtime-validation', - content: 'Runtime body.', - path: '/skills/runtime-validation/SKILL.md', - } - const cases: { patch: Partial; expected: string }[] = [ - { patch: { name: { value: 'runtime' } as unknown as string }, expected: 'runtime skill name must be a string' }, - { patch: { whenToUse: 1 as unknown as string }, expected: 'whenToUse must be a string' }, - { patch: { source: { value: 'source' } as unknown as string }, expected: 'source must be a string' }, - { patch: { content: { value: 'content' } as unknown as string }, expected: 'content must be a string' }, - { patch: { path: 1 as unknown as string }, expected: 'path must be a string' }, - ] - for (const { patch, expected } of cases) { - expect(() => ctx.skills.register({ ...valid, ...patch })).toThrow(expected) - } + expect(listed[0]?.resourceBase).toBe(resourceBase) + expect(loaded?.resourceBase).toBe(resourceBase) + expect(loaded?.metadata).toBe(metadata) + expect(loaded?.provider).toBe('runtime') }) it('rejects every malformed scalar in provider-loaded definitions', async () => { @@ -853,39 +663,6 @@ describe('SkillService registry', () => { expect(settled).toBe('aborted') }) - it('does not miss an abort racing listener installation', async () => { - const ctx = new Context() - await ctx.plugin(SkillService) - const reason = new Error('racing abort') - let aborted = false - const signal = { - get aborted() { - return aborted - }, - reason, - throwIfAborted() { - if (aborted) throw reason - }, - addEventListener(_type: string, listener: () => void) { - aborted = true - listener() - }, - removeEventListener() {}, - } as unknown as AbortSignal - ctx.skills.registerProvider({ - name: 'racing-abort', - list() { - return Promise.reject(new Error('late provider failure')) - }, - async get() { - return undefined - }, - }) - - await expect(ctx.skills.list({ signal })).rejects.toBe(reason) - await Promise.resolve() - }) - it('rejects invalid runtime skill registrations and ignores duplicates', async () => { const ctx = new Context() await ctx.plugin(SkillService) diff --git a/packages/support/invariants/package.json b/packages/support/invariants/package.json index ba95eea315..588c658857 100644 --- a/packages/support/invariants/package.json +++ b/packages/support/invariants/package.json @@ -26,8 +26,6 @@ "@deepseek-ai/dsh-llm": "^0.0.1", "@deepseek-ai/dsh-scope": "^0.0.1", "@deepseek-ai/dsh-session": "^0.0.1", - "@deepseek-ai/dsh-system-prompt": "^0.0.1", - "@deepseek-ai/dsh-tools": "^0.0.1", "cordis": "^4.0.0-rc.6" }, "devDependencies": { @@ -35,8 +33,6 @@ "@deepseek-ai/dsh-llm": "workspace:^", "@deepseek-ai/dsh-scope": "workspace:^", "@deepseek-ai/dsh-session": "workspace:^", - "@deepseek-ai/dsh-system-prompt": "workspace:^", - "@deepseek-ai/dsh-tools": "workspace:^", "cordis": "^4.0.0-rc.6" } } diff --git a/packages/support/invariants/src/index.ts b/packages/support/invariants/src/index.ts index 20396c4f5d..f5ef6d2b4a 100644 --- a/packages/support/invariants/src/index.ts +++ b/packages/support/invariants/src/index.ts @@ -19,8 +19,6 @@ import type { Context } from 'cordis' import { carrierKeyOf, isScopeCarrier } from '@deepseek-ai/dsh-scope' -import type { AssembleContext } from '@deepseek-ai/dsh-system-prompt' -import type { ToolExecution } from '@deepseek-ai/dsh-tools' import { assertNever, HarnessError } from '@deepseek-ai/dsh-llm' import type { CallId, GenerateOptions } from '@deepseek-ai/dsh-llm' import type { Agent, AgentStatus } from '@deepseek-ai/dsh-agent' @@ -92,6 +90,12 @@ interface AgentSubject { agent: Agent } +/** Structural subject fields used without coupling this dev plugin to owning services. */ +interface ScopedSubjectFields { + agent?: Agent + scope?: object +} + /** Assert that a step-scoped event names the currently open turn and step. */ function requireOpenStep(trace: SessionTrace, kind: string, turn: number, step: number): void { if (trace.openTurn !== turn || trace.openStep !== step) { @@ -412,32 +416,6 @@ export function apply(ctx: Context): void { lastStatus.set(agent, status) }, { global: true }) - // --- Setup-drives invariant --------------------------------------------- - // - // CreateAgentOptions.setup COMPOSES the agent's scoped world; it must not - // DRIVE the agent. ReactLoopAgent rejects every driving verb structurally - // until rollback-covered publication reaches the session-start boundary; - // this event-level invariant remains the cross-implementation backstop for - // alternate Agent implementations and raw session writes. A turn/start - // candidate before agent/session-start is rejected by internal/dispatch, - // before Session commits it. Sessions of agents that exist BEFORE this - // plugin applies are marked started (their ordering is unknowable after the - // fact — never a false positive on HMR). `agents` is read via ctx.get (a - // strict, optional store lookup) rather than injected: the invariants plugin - // must load in harnesses that carry no agent registry at all (bare session - // tests), where this check simply never trips. - const sessionStarted = new WeakSet() - for (const agent of ctx.get('agents')?.list() ?? []) sessionStarted.add(agent.session) - const assertSessionStartedBeforeTurn = (session: Session, event: SessionEvent): void => { - if (event.type !== 'turn/start' || sessionStarted.has(session)) return - const owner = ctx.get('agents')?.list().find(agent => agent.session === session) - if (owner === undefined) return - throw new InvariantError( - `agent "${owner.id}": a turn opened before agent/session-start fired — ` - + 'CreateAgentOptions.setup composes the scoped world, it must not drive the agent ' - + '(send/steer/inject belong after creation returns)') - } - // --- Scoped-dispatch invariants (the agent-scoping seam) --------------- // // Every scope-filtered event family must dispatch with a scope carrier @@ -466,11 +444,11 @@ export function apply(ctx: Context): void { 'agent/turn-stop': args => args[0], 'agent/error': args => args[0], 'approval/request': args => (args[0] as AgentSubject).agent, - 'tools/pre-execute': args => (args[0] as ToolExecution).agent, - 'tools/execute': args => (args[0] as ToolExecution).agent, - 'tools/post-execute': args => (args[0] as ToolExecution).agent, - 'tools/result': args => (args[0] as ToolExecution).agent, - 'system-prompt/assemble': args => (args[1] as AssembleContext).scope, + 'tools/pre-execute': args => (args[0] as ScopedSubjectFields).agent, + 'tools/execute': args => (args[0] as ScopedSubjectFields).agent, + 'tools/post-execute': args => (args[0] as ScopedSubjectFields).agent, + 'tools/result': args => (args[0] as ScopedSubjectFields).agent, + 'system-prompt/assemble': args => (args[1] as ScopedSubjectFields).scope, 'session/created': null, 'session/disposed': null, 'session/event': null, @@ -491,33 +469,16 @@ export function apply(ctx: Context): void { `"${name}" was dispatched with a scope carrier keyed to a DIFFERENT subject than its arguments name — ` + 'the carrier key and the event\'s subject must be the same object (use agentEvents(ctx, agent))') } - if (name === 'agent/session-start') { - // Mark before product listeners run: a prepended session-start listener is - // explicitly allowed to inject the first turn's context synchronously. - sessionStarted.add((args[0] as Agent).session) - } if (name === 'session/event') { const [session, event] = args as [Session, SessionEvent] const trace = traceFor(session) const transition = validateEvent(trace, event) - assertSessionStartedBeforeTurn(session, event) // The exact event identity reaches the contained post-commit listener. // A later internal/dispatch listener may still veto; because validation // is pure, abandoning this weakly keyed transition does not advance the // committed trace or retain the session. stagedTransitions.set(event, { session, trace, transition }) } - // The assembly context must never carry the agent DX field without the - // scope layer selector: the assembly would silently miss the agent's - // scoped sections/tools (use assembleContextFor(agent)). - if (name === 'system-prompt/assemble') { - const context = args[1] as AssembleContext - if (context.agent !== undefined && context.scope !== context.agent) { - throw new InvariantError( - 'an assembly context carries `agent` without `scope` (or with a mismatched scope) — ' - + 'use assembleContextFor(agent) so the assembly resolves the agent\'s scoped layer') - } - } }, { global: true }) // Request-reconstruction cross-check (the reconstructability RFC): a diff --git a/packages/support/invariants/tests/invariants.spec.ts b/packages/support/invariants/tests/invariants.spec.ts index efbf1dd978..5c26b1f4d7 100644 --- a/packages/support/invariants/tests/invariants.spec.ts +++ b/packages/support/invariants/tests/invariants.spec.ts @@ -79,34 +79,6 @@ describe('session-log invariants', () => { expect(session.events.map(event => event.type)).toEqual(['turn/start', 'turn/end']) }) - it('does not stage a substituted candidate from prepended internal instrumentation', async () => { - const { ctx } = await setup() - const session = ctx.sessions.create(SessionId('dispatch-substitution-rollback')) - let substitute = true - const replacement = { - type: 'turn/start', - seq: 0, - time: 1, - data: { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }, - } as const - ctx.on('internal/dispatch', (_mode, name, args) => { - if (name !== 'session/event' || !substitute) return - substitute = false - args[1] = replacement - }, { prepend: true }) - - expect(() => session.append('turn/start', { - turn: 1, - trigger: { kind: 'message', source: { kind: 'user' } }, - })).toThrow('session/event internal dispatch replaced the accepted callback tuple') - expect(session.events).toEqual([]) - - expect(() => { - session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) - session.append('turn/end', { turn: 1, reason: { kind: 'completed' } }) - }).not.toThrow() - }) - it('applies the committed transition after a prepended observer throws', async () => { const { ctx } = await setup() const warnings: string[] = [] @@ -921,62 +893,4 @@ describe('scoped-dispatch invariants', () => { .not.toThrow() }) - it('rejects an assembly context carrying agent without scope', async () => { - const ctx = await scopedCtx() - const agent = { id: 'a1' } as unknown as Agent - const base = { name: 'systemPrompt' } - const assembly = { sections: [], tools: [], variables: {} } - const bad = { agent } - expect(() => { - // The carrier base stands in for the SystemPrompt service (the declared `this`); the invariant only reads the carrier marks. - void ctx.waterfall(scopeTarget(base, undefined) as never, 'system-prompt/assemble', assembly as never, bad as never, () => Promise.resolve(assembly as never)) - }).toThrow(/agent.*without.*scope|assembleContextFor/) - const good = { agent, scope: agent } - expect(() => { - void ctx.waterfall(scopeTarget(base, agent) as never, 'system-prompt/assemble', assembly as never, good as never, () => Promise.resolve(assembly as never)) - }).not.toThrow() - }) - - it('backstops alternate agents that open a turn before agent/session-start', async () => { - const ctx = await scopedCtx() - // A live agent whose session is in the store but whose session-start has - // not fired: appending turn/start must throw the teaching error. - const session = ctx.sessions.create(SessionId('drive-s')) - const agent = { id: 'driver', session } as unknown as Agent - // Provide a minimal agents lookup: the invariant reads ctx.get('agents'). - const registryStub = { list: () => [agent] } - ctx.root.provide('agents', registryStub as never) - expect(() => { - session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) - }).toThrow(/turn opened before agent\/session-start/) - expect(session.events).toEqual([]) - // The internal boundary marks the session before even a prepended product - // listener runs, so the supported session-start injection pattern can open - // and close its one-shot context turn synchronously. - ctx.on('agent/session-start', () => { - session.append('turn/start', { turn: 1, trigger: { kind: 'injection', source: { kind: 'plugin', plugin: 'test' } } }) - session.append('turn/end', { turn: 1, reason: { kind: 'completed' } }) - }, { prepend: true }) - expect(() => { - ctx.emit(scopeTarget(agent, agent), 'agent/session-start', agent, 'startup') - }).not.toThrow() - expect(session.events.map(event => event.type)).toEqual(['turn/start', 'turn/end']) - expect(() => { - session.append('turn/start', { turn: 2, trigger: { kind: 'message', source: { kind: 'user' } } }) - }).not.toThrow() - }) - - it('marks sessions of agents that predate the plugin as started (HMR re-apply safety)', async () => { - const ctx = new Context() - await ctx.plugin(SessionStore) - const session = ctx.sessions.create(SessionId('pre-s')) - const agent = { id: 'pre', session } as unknown as Agent - ctx.root.provide('agents', { list: () => [agent] } as never) - // Invariants apply AFTER the agent exists: its ordering is unknowable, so - // a turn opening without an observed session-start must NOT false-positive. - await ctx.plugin(Invariants) - expect(() => { - session.append('turn/start', { turn: 1, trigger: { kind: 'message', source: { kind: 'user' } } }) - }).not.toThrow() - }) }) diff --git a/packages/support/invariants/tsconfig.json b/packages/support/invariants/tsconfig.json index 88944f779e..cd17f67d7c 100644 --- a/packages/support/invariants/tsconfig.json +++ b/packages/support/invariants/tsconfig.json @@ -25,12 +25,6 @@ }, { "path": "../../core/scope" - }, - { - "path": "../../core/system-prompt" - }, - { - "path": "../../core/tools" } ] } diff --git a/packages/ui/user-approval/README.md b/packages/ui/user-approval/README.md index b47f112aae..a50bffad0b 100644 --- a/packages/ui/user-approval/README.md +++ b/packages/ui/user-approval/README.md @@ -2,7 +2,7 @@ User-approval seam. Owns the `ctx.approval` service ([`ApprovalService`](src/index.ts)) and the one-shot permission vocabulary the harness shares: `ApprovalRequest` (agent + tool identity + reason + abort signal), the closed `ApprovalOutcome` union (`allowed-once` / `rejected` / `cancelled` / `unavailable`), the `ApprovalRequestId` brand pairing the two log-only audit events (`approval/asked` / `approval/decided`), and the `approval/request` waterfall the answerers listen on. It lives in the UI group because its purpose is human permission, while remaining channel-neutral: it depends only on Cordis and core vocabulary packages, never on a concrete UI. -The contract in one line: `ctx.approval.request(req)` puts exactly one question — "may this specific action proceed?" — to whatever answerers the deployment composed, and its answerer phase always produces an outcome: an aborted signal yields `cancelled`, a throwing or missing answerer yields `unavailable`, and `allowed-once` is a grant for the single asked-about action, never a class of future ones. Acceptance is synchronous: the service reads the request fields and `agent.session` binding once, requires object agent/session identities, a string `toolName`, optional string `callId`/`reason`, and an AbortSignal-shaped live capability, then shallow-freezes a detached request record while preserving the exact `agent` and signal identities. A malformed request rejects before any audit append; later caller mutation cannot redirect scope, payload, cancellation, policy lookup, or either audit event. The other precondition is an open turn on the captured session — the audit pair is turn-enclosed by contract (the turn is the durable log's commit/replay boundary; a bare event between turns is crash-tail garbage on reload), so an idle ask also rejects before appending. Either audit append may reject before commit because returning an unlogged decision would violate the pair. Session contains post-commit observer failures, so an authoritative audit append cannot reject the request or suppress its matching event. +The contract in one line: `ctx.approval.request(req)` puts exactly one question — "may this specific action proceed?" — to whatever answerers the deployment composed, and its answerer phase always produces an outcome: an aborted signal yields `cancelled`, a throwing or missing answerer yields `unavailable`, and `allowed-once` is a grant for the single asked-about action, never a class of future ones. `ApprovalRequest` is a readonly same-process contract: the service borrows the exact request, agent, session, and abort signal rather than cloning or freezing them. The request requires an open turn because the audit pair is turn-enclosed by contract (the turn is the durable log's commit/replay boundary; a bare event between turns is crash-tail garbage on reload), so an idle ask rejects before appending. Either audit append may reject before commit because returning an unlogged decision would violate the pair. Session contains post-commit observer failures, so an authoritative audit append cannot reject the request or suppress its matching event. The service is the mechanism, answerers are the policy. Answerers are `approval/request` waterfall listeners occupying a single decision slot: answer for an agent you own by returning an outcome without calling `next()`, or delegate an agent you don't recognize by calling `next()` — the chain's built-in default is `unavailable`, so a deployment with no answerer (headless, CI) fails closed with zero configuration. Dispatch is keyed by `req.agent`: a listener registered through `agent.ctx` receives only that agent's questions, while a plain-context listener receives every agent's. Registration order across sibling plugins is not load-order deterministic; compose one terminal answerer per deployment and use `prepend` listeners only for decide-or-delegate gates. diff --git a/packages/ui/user-approval/src/index.ts b/packages/ui/user-approval/src/index.ts index 9b0347bb74..8496e40e62 100644 --- a/packages/ui/user-approval/src/index.ts +++ b/packages/ui/user-approval/src/index.ts @@ -63,10 +63,8 @@ declare module 'cordis' { * Scope-filtered dispatch (`@deepseek-ai/dsh-scope`) keys the carrier by `req.agent`: a * listener registered through `agent.ctx` receives only that agent's * questions, while a plain-context listener receives every agent's. - * `req` is the service's shallow-frozen acceptance snapshot: later caller - * mutation cannot redirect the question, while the `agent` and `signal` - * identity capabilities remain exact. - * @param req - the accepted decision (agent, tool identity, reason, signal). + * `req` is a readonly same-process value borrowed from the caller. + * @param req - the pending decision (agent, tool identity, reason, signal). * @mode waterfall */ 'approval/request'(this: Scoped, req: ApprovalRequest, next: () => Promise): Promise @@ -241,11 +239,9 @@ export function setApprovalPolicy(session: Session, policy: ApprovalPolicy): voi * for an answerer to present it and for the audit events to reconstruct what * was asked — it deliberately does NOT carry tool arguments: a UI answerer * attaches the prompt to the already-streamed tool call via `callId` instead - * of re-rendering the call. `request()` synchronously copies and shallow-freezes - * this record before crossing an asynchronous boundary. It reads each field - * and the agent's session binding once, validates the public fixed-field - * contract before audit, and detaches the scalar values; the `agent` and live - * `signal` identity capabilities are preserved rather than cloned or frozen. + * of re-rendering the call. This is a readonly same-process contract: + * `request()` borrows the request and its `agent` and `signal` capabilities + * directly rather than treating them as serialized input. */ export interface ApprovalRequest { /** @@ -253,28 +249,21 @@ export interface ApprovalRequest { * UI answerer only answers for agents it owns) and receives the audit * events on its session log. */ - agent: Agent + readonly agent: Agent /** The tool the question is about (presentation and audit). */ - toolName: string + readonly toolName: string /** * The exact tool call being decided, when the asker has one — lets a UI * attach the prompt to the tool call it already streamed. */ - callId?: CallId + readonly callId?: CallId /** The asker's human-readable explanation of WHY it is asking. */ - reason?: string + readonly reason?: string /** * Aborting withdraws the question: the request settles `'cancelled'` * immediately and a late answer from a still-pending answerer is discarded. */ - signal?: AbortSignal -} - -/** Live signal capability accepted at the synchronous request boundary. */ -interface AcceptedSignal { - signal: AbortSignal - addEventListener: AbortSignal['addEventListener'] - removeEventListener: AbortSignal['removeEventListener'] + readonly signal?: AbortSignal } /** Plugin config. All optional — `static Config` supplies the defaults. */ @@ -285,7 +274,7 @@ export interface Config { * (fail-closed with none); `'never'` auto-rejects every ask without * prompting (the deterministic CI/unattended stance). */ - policy?: ApprovalPolicy + readonly policy?: ApprovalPolicy } /** @@ -374,104 +363,26 @@ export class ApprovalService extends Service { } /** - * Ask the composed answerers to decide one request. Synchronously reads each - * request field and the agent's session binding once, validates the fixed - * agent/session, string, and live-signal contracts, and rejects before any - * audit append when malformed. The signal remains the caller's exact live - * identity capability; it is neither cloned nor frozen. Requires an open - * turn on the accepted session — the audit pair below is turn-enclosed by - * contract (the turn is the log's commit/replay boundary; an idle append - * would be dropped as crash tail on reload) — and likewise throws before - * appending anything when called idle; asking outside a turn is a deferred - * design. The answerer phase always produces an outcome: an aborted signal - * yields `'cancelled'`, a missing or throwing answerer yields `'unavailable'` - * (fail closed), and a rogue non-vocabulary return value is normalized to - * `'unavailable'`. A failure that prevents either audit append from committing - * still rejects; returning an unlogged decision would violate the audit pair. - * The caller-owned request is synchronously - * snapshotted, so later mutation cannot split routing, dispatch payload, - * cancellation, policy lookup, or the audit pair across agents/sessions. - * Appends the - * `approval/asked`/`approval/decided` audit pair (log-only) around the - * decision regardless of outcome. Session contains each post-commit observer - * failure, so an already authoritative audit event cannot make this request - * reject or suppress its matching event. + * Ask the composed answerers to decide one readonly same-process request. + * The service borrows the request, agent, session, and live signal directly. + * The request requires an open turn because the audit pair must be enclosed + * by the durable log's commit/replay boundary; an idle ask rejects before + * appending anything. The answerer phase always produces an outcome: an + * aborted signal yields `'cancelled'`, a missing or throwing answerer yields + * `'unavailable'` (fail closed), and a rogue non-vocabulary return value is + * normalized to `'unavailable'`. A failure that prevents either audit append + * from committing still rejects because returning an unlogged decision would + * violate the pair. Session contains post-commit observer failures, so an + * authoritative append cannot reject the request or suppress its matching + * audit event. * @param req - the pending decision (agent, tool identity, reason, signal). * @returns the closed outcome; `'allowed-once'` is the only grant. - * @throws when request acceptance fails, no turn is open, or either audit - * event fails before the session append commit point. + * @throws when no turn is open or either audit event fails before the session + * append commit point. */ async request(req: ApprovalRequest): Promise { - // Accept one immutable request shape before the first async boundary. The - // caller retains its record and may mutate it as soon as this async method - // returns; identity capabilities stay live, but the record is never reread. - const input: unknown = req - if (typeof input !== 'object' || input === null) { - throw new TypeError('approval.request() requires a request object') - } - const source = input as Record - const agentInput = source['agent'] - const toolName = source['toolName'] - const callId = source['callId'] - const reason = source['reason'] - const signalInput = source['signal'] - if (typeof agentInput !== 'object' || agentInput === null) { - throw new TypeError('approval request agent must be an object') - } - if (typeof toolName !== 'string') { - throw new TypeError('approval request toolName must be a string') - } - if (callId !== undefined && typeof callId !== 'string') { - throw new TypeError('approval request callId must be a string when provided') - } - if (reason !== undefined && typeof reason !== 'string') { - throw new TypeError('approval request reason must be a string when provided') - } - let acceptedSignal: AcceptedSignal | undefined - if (signalInput !== undefined) { - if (typeof signalInput !== 'object' || signalInput === null) { - throw new TypeError('approval request signal must be an AbortSignal when provided') - } - const signalRecord = signalInput as unknown as Record - const aborted = signalRecord['aborted'] - const addEventListener = signalRecord['addEventListener'] - const removeEventListener = signalRecord['removeEventListener'] - if (typeof aborted !== 'boolean' - || typeof addEventListener !== 'function' - || typeof removeEventListener !== 'function') { - throw new TypeError('approval request signal must be an AbortSignal when provided') - } - acceptedSignal = { - signal: signalInput as AbortSignal, - addEventListener: addEventListener as AbortSignal['addEventListener'], - removeEventListener: removeEventListener as AbortSignal['removeEventListener'], - } - } - const sessionInput = (agentInput as unknown as Record)['session'] - if (typeof sessionInput !== 'object' || sessionInput === null) { - throw new TypeError('approval request agent session must be an object') - } - const sessionRecord = sessionInput as unknown as Record - const events = sessionRecord['events'] - const append = sessionRecord['append'] - if (!Array.isArray(events)) { - throw new TypeError('approval request session events must be an array') - } - if (typeof append !== 'function') { - throw new TypeError('approval request session append must be a function') - } - const agent = agentInput as Agent - const session = sessionInput as Session - const acceptedCallId = callId as CallId | undefined - const signal = signalInput as AbortSignal | undefined - const accepted: Readonly = Object.freeze({ - agent, - toolName, - ...acceptedCallId !== undefined ? { callId: acceptedCallId } : {}, - ...reason !== undefined ? { reason } : {}, - ...signal !== undefined ? { signal } : {}, - }) - if (!hasOpenTurn(events)) { + const session = req.agent.session + if (!hasOpenTurn(session.events)) { throw new Error( 'approval.request() outside an open turn: the approval/asked + approval/decided audit pair ' + 'must be turn-enclosed (a bare event between turns is crash-tail garbage on reload). ' @@ -479,14 +390,14 @@ export class ApprovalService extends Service { ) } const id = ApprovalRequestId(randomUUID()) - Reflect.apply(append, session, ['approval/asked', { + session.append('approval/asked', { id, - toolName: accepted.toolName, - ...accepted.callId !== undefined ? { callId: accepted.callId } : {}, - ...accepted.reason !== undefined ? { reason: accepted.reason } : {}, - }]) - const outcome = await this.decide(accepted, session, acceptedSignal) - Reflect.apply(append, session, ['approval/decided', { id, outcome }]) + toolName: req.toolName, + ...req.callId !== undefined ? { callId: req.callId } : {}, + ...req.reason !== undefined ? { reason: req.reason } : {}, + }) + const outcome = await this.decide(req, session) + session.append('approval/decided', { id, outcome }) return outcome } @@ -502,16 +413,14 @@ export class ApprovalService extends Service { } /** - * Dispatch the waterfall, contained and raced against the accepted signal. - * @param req - the detached public request snapshot. - * @param session - the captured session used for policy lookup. - * @param acceptedSignal - the validated live signal capability, if supplied. + * Dispatch the waterfall, contained and raced against the request signal. + * @param req - the borrowed public request. + * @param session - the request agent's session used for policy lookup. * @returns the normalized closed outcome. */ - private async decide( - req: Readonly, session: Session, acceptedSignal: AcceptedSignal | undefined, - ): Promise { - if (acceptedSignal?.signal.aborted) return 'cancelled' + private async decide(req: ApprovalRequest, session: Session): Promise { + const signal = req.signal + if (signal?.aborted) return 'cancelled' // The 'never' policy is decided HERE, before any dispatch: a listener // registered with `prepend: true` after this service mounts would sit // ahead of any gate LISTENER, so a listener-shaped gate cannot keep the @@ -535,13 +444,18 @@ export class ApprovalService extends Service { // tool call open — the seam contains its callbacks. () => 'unavailable', ) - if (acceptedSignal === undefined) return answer - const { signal, addEventListener, removeEventListener } = acceptedSignal + if (signal === undefined) return answer return await new Promise((resolve) => { - const onAbort = () => { resolve('cancelled') } - addEventListener.call(signal, 'abort', onAbort, { once: true }) + const onAbort = () => { + signal.removeEventListener('abort', onAbort) + resolve('cancelled') + } + signal.addEventListener('abort', onAbort, { once: true }) + // Abort can win after the initial check but before listener installation. + // Recheck at the settlement boundary so that edge still cancels. + if (signal.aborted) onAbort() void answer.then((outcome) => { - removeEventListener.call(signal, 'abort', onAbort) + signal.removeEventListener('abort', onAbort) // After an abort won the race this resolve is a settled-promise no-op: // the late answer is discarded by construction. resolve(outcome) diff --git a/packages/ui/user-approval/tests/approval.spec.ts b/packages/ui/user-approval/tests/approval.spec.ts index 4d4b7aa67f..be9e723732 100644 --- a/packages/ui/user-approval/tests/approval.spec.ts +++ b/packages/ui/user-approval/tests/approval.spec.ts @@ -2,7 +2,8 @@ import { describe, expect, it, vi } from 'vitest' import { Context } from 'cordis' import type { Agent } from '@deepseek-ai/dsh-agent' import { CallId } from '@deepseek-ai/dsh-llm' -import { carrierKeyOf, scopeHost } from '@deepseek-ai/dsh-scope' +import { carrierKeyOf, createScope } from '@deepseek-ai/dsh-scope' +import type { Scope } from '@deepseek-ai/dsh-scope' import SessionStore, { Session, SessionId } from '@deepseek-ai/dsh-session' import type { SessionEvent } from '@deepseek-ai/dsh-session' import SystemPrompt from '@deepseek-ai/dsh-system-prompt' @@ -39,158 +40,6 @@ function requestOf(agent: Agent, overrides: Partial = {}): Appr } describe('ApprovalService.request', () => { - it('rejects malformed fixed fields and identities before appending or dispatching', async () => { - const ctx = await mounted() - const consulted = vi.fn() - ctx.on('approval/request', () => { - consulted() - return Promise.resolve('allowed-once') - }) - const { agent, appended } = fakeAgent() - const badSessionAppends: Array> = [] - const badSession = (events: unknown, append: unknown): Agent => ({ - session: { events, append }, - }) as unknown as Agent - const appendSpy = (): ReturnType => { - const append = vi.fn() - badSessionAppends.push(append) - return append - } - const validSignalShape = { - aborted: false, - addEventListener: () => {}, - removeEventListener: () => {}, - } - const cases: Array<{ request: unknown; message: string }> = [ - { request: null, message: 'requires a request object' }, - { request: 1, message: 'requires a request object' }, - { request: { agent: null, toolName: 'echo' }, message: 'agent must be an object' }, - { request: { agent: 1, toolName: 'echo' }, message: 'agent must be an object' }, - { request: { agent, toolName: 1 }, message: 'toolName must be a string' }, - { request: { agent, toolName: 'echo', callId: 1 }, message: 'callId must be a string' }, - { request: { agent, toolName: 'echo', reason: 1 }, message: 'reason must be a string' }, - { request: { agent, toolName: 'echo', signal: null }, message: 'signal must be an AbortSignal' }, - { request: { agent, toolName: 'echo', signal: 1 }, message: 'signal must be an AbortSignal' }, - { - request: { agent, toolName: 'echo', signal: { ...validSignalShape, aborted: 'no' } }, - message: 'signal must be an AbortSignal', - }, - { - request: { agent, toolName: 'echo', signal: { ...validSignalShape, addEventListener: 1 } }, - message: 'signal must be an AbortSignal', - }, - { - request: { agent, toolName: 'echo', signal: { ...validSignalShape, removeEventListener: 1 } }, - message: 'signal must be an AbortSignal', - }, - { - request: { agent: { session: null }, toolName: 'echo' }, - message: 'agent session must be an object', - }, - { - request: { agent: { session: 1 }, toolName: 'echo' }, - message: 'agent session must be an object', - }, - { - request: { agent: badSession(null, appendSpy()), toolName: 'echo' }, - message: 'session events must be an array', - }, - { - request: { agent: badSession([{ type: 'turn/start' }], 1), toolName: 'echo' }, - message: 'session append must be a function', - }, - ] - - for (const { request, message } of cases) { - await expect(ctx.approval.request(request as ApprovalRequest)).rejects.toThrow(message) - } - - expect(appended).toEqual([]) - for (const append of badSessionAppends) expect(append).not.toHaveBeenCalled() - expect(consulted).not.toHaveBeenCalled() - }) - - it('reads request fields, the agent session, and the session append method once', async () => { - const ctx = await mounted() - const { agent: acceptedSessionOwner, appended: acceptedAudit } = fakeAgent() - const { agent: replacementAgent, appended: replacementAudit } = fakeAgent() - const acceptedSession = acceptedSessionOwner.session - const acceptedAppend = acceptedSession.append.bind(acceptedSession) - const signal = new AbortController().signal - const reads = { - agent: 0, - toolName: 0, - callId: 0, - reason: 0, - signal: 0, - session: 0, - append: 0, - } - const session = { - events: acceptedSession.events, - get append(): Session['append'] { - reads.append += 1 - return reads.append === 1 ? acceptedAppend : undefined as unknown as Session['append'] - }, - } as Session - const agent = Object.defineProperty({}, 'session', { - enumerable: true, - get: () => { - reads.session += 1 - return reads.session === 1 ? session : replacementAgent.session - }, - }) as Agent - const request = Object.defineProperties({}, { - agent: { - enumerable: true, - get: () => (++reads.agent === 1 ? agent : null), - }, - toolName: { - enumerable: true, - get: () => (++reads.toolName === 1 ? 'stable-tool' : 1), - }, - callId: { - enumerable: true, - get: () => (++reads.callId === 1 ? CallId('stable-call') : {}), - }, - reason: { - enumerable: true, - get: () => (++reads.reason === 1 ? 'stable reason' : {}), - }, - signal: { - enumerable: true, - get: () => (++reads.signal === 1 ? signal : {}), - }, - }) as ApprovalRequest - let received: ApprovalRequest | undefined - ctx.on('approval/request', (accepted) => { - received = accepted - return Promise.resolve('allowed-once') - }) - - await expect(ctx.approval.request(request)).resolves.toBe('allowed-once') - - expect(reads).toEqual({ - agent: 1, - toolName: 1, - callId: 1, - reason: 1, - signal: 1, - session: 1, - append: 1, - }) - expect(received).toMatchObject({ - agent, - toolName: 'stable-tool', - callId: 'stable-call', - reason: 'stable reason', - signal, - }) - expect(Object.isFrozen(received)).toBe(true) - expect(acceptedAudit.map(event => event.type)).toEqual(['approval/asked', 'approval/decided']) - expect(replacementAudit).toEqual([]) - }) - it('throws before appending anything when no turn has ever opened (idle ask)', async () => { const ctx = await mounted() const { agent, appended } = fakeAgent([]) @@ -230,77 +79,38 @@ describe('ApprovalService.request', () => { expect(Object.keys(appended[0]?.data ?? {}).sort()).toEqual(['id', 'toolName']) }) - it('snapshots request identity, scope, payload, and audit before deferred dispatch', async () => { + it('borrows the exact readonly request for scoped dispatch and audit', async () => { const ctx = await mounted() - const { agent: acceptedAgent, appended: acceptedAudit } = fakeAgent() - const { agent: replacementAgent, appended: replacementAudit } = fakeAgent() - const host = await scopeHost(ctx, ['approval']) - const acceptedScope = host.mint(acceptedAgent) - const replacementScope = host.mint(replacementAgent) - const dispatchStarted = Promise.withResolvers<'started'>() - const answer = Promise.withResolvers() - const originalSignal = new AbortController().signal - const replacementSignal = new AbortController().signal - let heardBy: 'accepted' | 'replacement' | undefined + const { agent, appended } = fakeAgent() + let scope!: Scope + const scopeFiber = await ctx.plugin(Object.assign((inner: Context) => { + scope = createScope(inner, agent) + }, { inject: ['approval'] })) let received: ApprovalRequest | undefined let carrier: unknown - acceptedScope.ctx.on('approval/request', function (req) { - heardBy = 'accepted' + scope.ctx.on('approval/request', function (req) { received = req carrier = carrierKeyOf(this) - dispatchStarted.resolve('started') - return answer.promise + return Promise.resolve('allowed-once') }) - replacementScope.ctx.on('approval/request', function (req) { - heardBy = 'replacement' - received = req - carrier = carrierKeyOf(this) - dispatchStarted.resolve('started') - return answer.promise - }) - const request = requestOf(acceptedAgent, { - toolName: 'original-tool', - callId: CallId('original-call'), - reason: 'original reason', - signal: originalSignal, + const request = requestOf(agent, { + toolName: 'scoped-tool', + callId: CallId('scoped-call'), + reason: 'scoped reason', }) - const pending = ctx.approval.request(request) - // request() has returned, but the answerer dispatch is deliberately queued - // in a microtask. Mutating the caller-owned record must not redirect it. - request.agent = replacementAgent - request.toolName = 'mutated-before-dispatch' - request.callId = CallId('mutated-call') - request.reason = 'mutated reason' - request.signal = replacementSignal - await dispatchStarted.promise - // Mutation while the answer is pending must not redirect the final audit. - request.toolName = 'mutated-after-dispatch' - request.reason = 'mutated again' - answer.resolve('allowed-once') - - await expect(pending).resolves.toBe('allowed-once') - expect(heardBy).toBe('accepted') - expect(carrier).toBe(acceptedAgent) - expect(received).not.toBe(request) - expect(Object.isFrozen(received)).toBe(true) - expect(received).toMatchObject({ - agent: acceptedAgent, - toolName: 'original-tool', - callId: 'original-call', - reason: 'original reason', - signal: originalSignal, + await expect(ctx.approval.request(request)).resolves.toBe('allowed-once') + expect(carrier).toBe(agent) + expect(received).toBe(request) + expect(appended).toHaveLength(2) + expect(appended[0]?.data).toMatchObject({ + toolName: 'scoped-tool', + callId: 'scoped-call', + reason: 'scoped reason', }) - expect(acceptedAudit).toHaveLength(2) - expect(acceptedAudit[0]?.data).toMatchObject({ - toolName: 'original-tool', - callId: 'original-call', - reason: 'original reason', - }) - expect(acceptedAudit[1]?.data).toMatchObject({ outcome: 'allowed-once' }) - expect(acceptedAudit[1]?.data['id']).toBe(acceptedAudit[0]?.data['id']) - expect(replacementAudit).toEqual([]) - await host.dispose() + expect(appended[1]?.data).toMatchObject({ outcome: 'allowed-once' }) + expect(appended[1]?.data['id']).toBe(appended[0]?.data['id']) + await scopeFiber.dispose() }) it('contains an approval/asked observer throw after append and still completes the pair', async () => { @@ -388,9 +198,12 @@ describe('ApprovalService.request', () => { const ctx = await mounted() const { agent: agentA } = fakeAgent() const { agent: agentB } = fakeAgent() - const host = await scopeHost(ctx, ['approval']) - const scopeA = host.mint(agentA) - const scopeB = host.mint(agentB) + let scopeA!: Scope + let scopeB!: Scope + const scopesFiber = await ctx.plugin(Object.assign((inner: Context) => { + scopeA = createScope(inner, agentA) + scopeB = createScope(inner, agentB) + }, { inject: ['approval'] })) const heard: string[] = [] ctx.on('approval/request', (req, next) => { heard.push(req.agent === agentA ? 'global:A' : 'global:B') @@ -409,14 +222,16 @@ describe('ApprovalService.request', () => { await expect(ctx.approval.request(requestOf(agentB))).resolves.toBe('unavailable') expect(heard).toEqual(['global:A', 'scoped:A', 'global:B', 'scoped:B']) - await host.dispose() + await scopesFiber.dispose() }) it('keys the scoped dispatch carrier to the exact request agent', async () => { const ctx = await mounted() const { agent } = fakeAgent() - const host = await scopeHost(ctx, ['approval']) - const scope = host.mint(agent) + let scope!: Scope + const scopeFiber = await ctx.plugin(Object.assign((inner: Context) => { + scope = createScope(inner, agent) + }, { inject: ['approval'] })) let seenKey: object | undefined scope.ctx.on('approval/request', function (req, next) { seenKey = carrierKeyOf(this) @@ -427,7 +242,7 @@ describe('ApprovalService.request', () => { await expect(ctx.approval.request(requestOf(agent))).resolves.toBe('unavailable') expect(seenKey).toBe(agent) - await host.dispose() + await scopeFiber.dispose() }) it('contains a throwing answerer as unavailable', async () => { @@ -466,6 +281,26 @@ describe('ApprovalService.request', () => { expect(appended[1]?.data).toMatchObject({ outcome: 'cancelled' }) }) + it('does not miss an abort between the initial check and listener installation', async () => { + const ctx = await mounted() + const { agent, appended } = fakeAgent() + const answer = Promise.withResolvers() + ctx.on('approval/request', () => answer.promise) + const controller = new AbortController() + const addEventListener = controller.signal.addEventListener.bind(controller.signal) + const add = vi.spyOn(controller.signal, 'addEventListener').mockImplementation((type, listener, options) => { + controller.abort() + addEventListener(type, listener, options) + }) + + await expect(ctx.approval.request(requestOf(agent, { signal: controller.signal }))).resolves.toBe('cancelled') + + answer.resolve('allowed-once') + await Promise.resolve() + expect(add).toHaveBeenCalledOnce() + expect(appended[1]?.data).toMatchObject({ outcome: 'cancelled' }) + }) + it('resolves cancelled when the signal aborts mid-question and discards the late answer', async () => { const ctx = await mounted() const { agent, appended } = fakeAgent()