From cf4502f162d2cca7f88374776169aa89117d05fb Mon Sep 17 00:00:00 2001 From: grypez <143971198+grypez@users.noreply.github.com> Date: Tue, 16 Jun 2026 17:09:53 -0400 Subject: [PATCH] refactor(kernel-agents): make the exo membrane the sole capability arg enforcer Now that every capability is a pattern-guarded discoverable exo, retire the membraneless authoring and validation paths: - Remove the `capability()` authoring helper and the `validateCapabilityArgs` validator (and its now-dead module). A capability's arguments are enforced only by its exo's interface guard; the chat strategy no longer re-validates before invoking, relying on the guard rejection it already catches. - Add a `test/make-capability.ts` helper that builds a guarded capability from the `described*()` combinators via `makeInternalCapabilities`, and migrate the chat and JSON evaluator tests (and the capability test, repurposed to cover the surviving `extract*` helpers) onto it. - Collapse the redundant `CapabilitySchema` type into kernel-utils' `MethodSchema`: a capability's `schema` is exactly the `MethodSchema` its exo describes, so the parallel type (and its `ExtractRecordKeys` helper) is gone. - Drop the unused `@metamask/superstruct` dependency. BREAKING: `capability` and `validateCapabilityArgs` are no longer exported. Co-Authored-By: Claude Opus 4.8 --- packages/kernel-agents/CHANGELOG.md | 5 ++ packages/kernel-agents/package.json | 1 - .../src/capabilities/capability.test.ts | 34 +++++++---- .../src/capabilities/capability.ts | 22 +------ .../validate-capability-args.test.ts | 59 ------------------- .../capabilities/validate-capability-args.ts | 17 ------ .../src/strategies/chat-agent.test.ts | 57 ++++++++++-------- .../src/strategies/chat-agent.ts | 5 +- .../src/strategies/json/evaluator.test.ts | 13 ++-- packages/kernel-agents/src/types.ts | 1 - .../kernel-agents/src/types/capability.ts | 13 +--- .../kernel-agents/test/make-capability.ts | 35 +++++++++++ yarn.lock | 1 - 13 files changed, 112 insertions(+), 151 deletions(-) delete mode 100644 packages/kernel-agents/src/capabilities/validate-capability-args.test.ts delete mode 100644 packages/kernel-agents/src/capabilities/validate-capability-args.ts create mode 100644 packages/kernel-agents/test/make-capability.ts diff --git a/packages/kernel-agents/CHANGELOG.md b/packages/kernel-agents/CHANGELOG.md index b91659d842..109868d48c 100644 --- a/packages/kernel-agents/CHANGELOG.md +++ b/packages/kernel-agents/CHANGELOG.md @@ -10,5 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - The built-in capabilities (`math`, `end`, `examples`) are now pattern-guarded discoverable exos authored with the `described*()` combinators, so their argument shapes are enforced by the exo's interface guard at invocation rather than only described in the prompt ([#959](https://github.com/MetaMask/ocap-kernel/pull/959)) +- A capability's arguments are now validated solely by its exo's interface guard (the membrane); the chat strategy no longer re-validates arguments before invoking a capability ([#960](https://github.com/MetaMask/ocap-kernel/pull/960)) + +### Removed + +- **BREAKING:** Remove the `capability()` authoring helper and the `validateCapabilityArgs` validator. Capabilities are authored as pattern-guarded discoverable exos (via the `described*()` combinators in `@metamask/kernel-utils`) and discovered into capability records, so there is no membraneless authoring path and the membrane is the sole argument enforcer ([#960](https://github.com/MetaMask/ocap-kernel/pull/960)) [Unreleased]: https://github.com/MetaMask/ocap-kernel/ diff --git a/packages/kernel-agents/package.json b/packages/kernel-agents/package.json index bc1358fae4..a6b65725b3 100644 --- a/packages/kernel-agents/package.json +++ b/packages/kernel-agents/package.json @@ -196,7 +196,6 @@ "@metamask/kernel-errors": "workspace:^", "@metamask/kernel-utils": "workspace:^", "@metamask/logger": "workspace:^", - "@metamask/superstruct": "^3.2.1", "@ocap/kernel-language-model-service": "workspace:^", "partial-json": "^0.1.7", "ses": "^1.14.0" diff --git a/packages/kernel-agents/src/capabilities/capability.test.ts b/packages/kernel-agents/src/capabilities/capability.test.ts index d364c48b9e..b494bb3df4 100644 --- a/packages/kernel-agents/src/capabilities/capability.test.ts +++ b/packages/kernel-agents/src/capabilities/capability.test.ts @@ -1,17 +1,31 @@ +import { S } from '@metamask/kernel-utils'; import { describe, it, expect } from 'vitest'; -import { capability } from './capability.ts'; +import { extractCapabilities, extractCapabilitySchemas } from './capability.ts'; +import { makeCapability } from '../../test/make-capability.ts'; -describe('capability', () => { - it('creates a capability with func and schema', () => { - const testCapability = capability(async () => Promise.resolve('test'), { - description: 'a test capability', - args: {}, - }); - expect(testCapability.func).toBeInstanceOf(Function); - expect(testCapability.schema).toStrictEqual({ - description: 'a test capability', +describe('capability extraction', () => { + const makeRecord = () => ({ + ping: makeCapability( + 'Server', + 'ping', + async () => 'pong', + S.method('Ping', [], S.string()), + ), + }); + + it('extractCapabilities returns the functions keyed by name', async () => { + const funcs = extractCapabilities(makeRecord()); + expect(Object.keys(funcs)).toStrictEqual(['ping']); + expect(await funcs.ping(undefined as never)).toBe('pong'); + }); + + it('extractCapabilitySchemas returns the schemas keyed by name', () => { + const schemas = extractCapabilitySchemas(makeRecord()); + expect(schemas.ping).toStrictEqual({ + description: 'Ping', args: {}, + returns: { type: 'string' }, }); }); }); diff --git a/packages/kernel-agents/src/capabilities/capability.ts b/packages/kernel-agents/src/capabilities/capability.ts index 6bce8cb45f..ca74cd4cfb 100644 --- a/packages/kernel-agents/src/capabilities/capability.ts +++ b/packages/kernel-agents/src/capabilities/capability.ts @@ -1,24 +1,8 @@ -import type { ExtractRecordKeys } from '../types/capability.ts'; -import type { - CapabilityRecord, - CapabilitySpec, - CapabilitySchema, - Capability, -} from '../types.ts'; +import type { MethodSchema } from '@metamask/kernel-utils'; -/** - * Create a capability specification. - * - * @param func - The function to create a capability specification for - * @param schema - The schema for the capability - * @returns A capability specification - */ -export const capability = , Return = null>( - func: Capability, - schema: CapabilitySchema>, -): CapabilitySpec => ({ func, schema }); +import type { CapabilityRecord, CapabilitySpec } from '../types.ts'; -type SchemaEntry = [string, { schema: CapabilitySchema }]; +type SchemaEntry = [string, { schema: MethodSchema }]; /** * Extract only the serializable schemas from the capabilities * diff --git a/packages/kernel-agents/src/capabilities/validate-capability-args.test.ts b/packages/kernel-agents/src/capabilities/validate-capability-args.test.ts deleted file mode 100644 index 21bdd4b0ca..0000000000 --- a/packages/kernel-agents/src/capabilities/validate-capability-args.test.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { describe, expect, it } from 'vitest'; - -import { validateCapabilityArgs } from './validate-capability-args.ts'; - -describe('validateCapabilityArgs', () => { - it('accepts values matching primitive arg schemas', () => { - expect(() => - validateCapabilityArgs( - { a: 1, b: 2 }, - { - description: 'add', - args: { - a: { type: 'number' }, - b: { type: 'number' }, - }, - }, - ), - ).not.toThrow(); - }); - - it('throws when a required argument is missing', () => { - expect(() => - validateCapabilityArgs( - { a: 1 }, - { - description: 'add', - args: { - a: { type: 'number' }, - b: { type: 'number' }, - }, - }, - ), - ).toThrow(/At path: b -- Expected a number/u); - }); - - it('throws when a value does not match the schema', () => { - expect(() => - validateCapabilityArgs( - { a: 'not-a-number' }, - { - description: 'x', - args: { a: { type: 'number' } }, - }, - ), - ).toThrow(/Expected a number/u); - }); - - it('does nothing when there are no declared arguments', () => { - expect(() => - validateCapabilityArgs( - { extra: 1 }, - { - description: 'ping', - args: {}, - }, - ), - ).not.toThrow(); - }); -}); diff --git a/packages/kernel-agents/src/capabilities/validate-capability-args.ts b/packages/kernel-agents/src/capabilities/validate-capability-args.ts deleted file mode 100644 index 7b4668f26e..0000000000 --- a/packages/kernel-agents/src/capabilities/validate-capability-args.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { methodArgsToStruct } from '@metamask/kernel-utils/json-schema-to-struct'; -import { assert } from '@metamask/superstruct'; - -import type { CapabilitySchema } from '../types.ts'; - -/** - * Assert `values` match the capability's declared argument schemas using Superstruct. - * - * @param values - Parsed tool arguments (a plain object). - * @param capabilitySchema - {@link CapabilitySchema} for this capability. - */ -export function validateCapabilityArgs( - values: Record, - capabilitySchema: CapabilitySchema, -): void { - assert(values, methodArgsToStruct(capabilitySchema.args)); -} diff --git a/packages/kernel-agents/src/strategies/chat-agent.test.ts b/packages/kernel-agents/src/strategies/chat-agent.test.ts index f9f068efdc..de909e26f5 100644 --- a/packages/kernel-agents/src/strategies/chat-agent.test.ts +++ b/packages/kernel-agents/src/strategies/chat-agent.test.ts @@ -1,5 +1,6 @@ import '@ocap/repo-tools/test-utils/mock-endoify'; +import { S } from '@metamask/kernel-utils'; import type { ChatMessage, ChatResult, @@ -9,7 +10,7 @@ import { describe, expect, it, vi } from 'vitest'; import { makeChatAgent } from './chat-agent.ts'; import type { BoundChat } from './chat-agent.ts'; -import { capability } from '../capabilities/capability.ts'; +import { makeCapability } from '../../test/make-capability.ts'; const makeToolCall = ( id: string, @@ -62,15 +63,17 @@ describe('makeChatAgent', () => { }); it('dispatches a tool call and returns final text answer', async () => { - const add = vi.fn(async ({ a, b }: { a: number; b: number }) => a + b); - const addCap = capability(add, { - description: 'Add two numbers', - args: { - a: { type: 'number' }, - b: { type: 'number' }, - }, - returns: { type: 'number' }, - }); + const add = vi.fn(async (a: number, b: number) => a + b); + const addCap = makeCapability( + 'Math', + 'add', + add, + S.method( + 'Add two numbers', + [S.arg('a', S.number()), S.arg('b', S.number())], + S.number(), + ), + ); let call = 0; const chat: BoundChat = async () => { @@ -86,17 +89,18 @@ describe('makeChatAgent', () => { const agent = makeChatAgent({ chat, capabilities: { add: addCap } }); const result = await agent.task('add 3 and 4'); - expect(add).toHaveBeenCalledWith({ a: 3, b: 4 }); + expect(add).toHaveBeenCalledWith(3, 4); expect(result).toBe('7'); }); it('injects tool result message before next turn', async () => { const recorded: ChatMessage[][] = []; - const ping = capability(async () => 'pong', { - description: 'Ping', - args: {}, - returns: { type: 'string' }, - }); + const ping = makeCapability( + 'Server', + 'ping', + async () => 'pong', + S.method('Ping', [], S.string()), + ); let call = 0; const chat: BoundChat = async ({ messages }) => { @@ -152,10 +156,12 @@ describe('makeChatAgent', () => { }); it('throws when invocation budget is exceeded', async () => { - const ping = capability(async () => 'pong', { - description: 'Ping', - args: {}, - }); + const ping = makeCapability( + 'Server', + 'ping', + async () => 'pong', + S.method('Ping', [], S.string()), + ); const chat: BoundChat = async () => makeToolCallResponse('0', [makeToolCall('c1', 'ping', {})]); @@ -177,11 +183,12 @@ describe('makeChatAgent', () => { it('passes tools to the chat function', async () => { const recordedTools: unknown[] = []; - const ping = capability(async () => 'pong', { - description: 'Ping the server', - args: {}, - returns: { type: 'string' }, - }); + const ping = makeCapability( + 'Server', + 'ping', + async () => 'pong', + S.method('Ping the server', [], S.string()), + ); const chat: BoundChat = async ({ tools }) => { recordedTools.push(tools); diff --git a/packages/kernel-agents/src/strategies/chat-agent.ts b/packages/kernel-agents/src/strategies/chat-agent.ts index 99c6a187c7..66c21eaebc 100644 --- a/packages/kernel-agents/src/strategies/chat-agent.ts +++ b/packages/kernel-agents/src/strategies/chat-agent.ts @@ -7,7 +7,6 @@ import type { import { parseToolArguments } from '@ocap/kernel-language-model-service/utils/parse-tool-arguments'; import { extractCapabilitySchemas } from '../capabilities/capability.ts'; -import { validateCapabilityArgs } from '../capabilities/validate-capability-args.ts'; import type { Agent } from '../types/agent.ts'; import { Message } from '../types/messages.ts'; import type { CapabilityRecord, Experience } from '../types.ts'; @@ -178,7 +177,9 @@ export const makeChatAgent = ({ let toolResult: unknown; try { const args = parseToolArguments(argsJson); - validateCapabilityArgs(args, spec.schema); + // The capability is backed by a pattern-guarded exo, so its + // interface guard enforces the argument shape; a mismatch rejects + // here and is reported as the tool error below. toolResult = await spec.func(args as never); } catch (error) { const errorContent = `Error calling ${name}: ${(error as Error).message}`; diff --git a/packages/kernel-agents/src/strategies/json/evaluator.test.ts b/packages/kernel-agents/src/strategies/json/evaluator.test.ts index fa1cf1290d..a6883d1d47 100644 --- a/packages/kernel-agents/src/strategies/json/evaluator.test.ts +++ b/packages/kernel-agents/src/strategies/json/evaluator.test.ts @@ -1,15 +1,18 @@ +import { S } from '@metamask/kernel-utils'; import { describe, it, expect } from 'vitest'; import { makeEvaluator } from './evaluator.ts'; import { AssistantMessage, CapabilityResultMessage } from './messages.ts'; -import { capability } from '../../capabilities/capability.ts'; +import { makeCapability } from '../../../test/make-capability.ts'; describe('invokeCapabilities', () => { it("invokes the assistant's chosen capability", async () => { - const testCapability = capability(async () => Promise.resolve('test'), { - description: 'a test capability', - args: {}, - }); + const testCapability = makeCapability( + 'Test', + 'testCapability', + async () => Promise.resolve('test'), + S.method('a test capability', [], S.string()), + ); const evaluator = makeEvaluator({ capabilities: { testCapability } }); const result = await evaluator( [], diff --git a/packages/kernel-agents/src/types.ts b/packages/kernel-agents/src/types.ts index 82d53a5d3e..f3d6c6c52f 100644 --- a/packages/kernel-agents/src/types.ts +++ b/packages/kernel-agents/src/types.ts @@ -1,7 +1,6 @@ export type { Capability, CapabilityRecord, - CapabilitySchema, CapabilitySpec, } from './types/capability.ts'; export type { TaskArgs } from './types/task.ts'; diff --git a/packages/kernel-agents/src/types/capability.ts b/packages/kernel-agents/src/types/capability.ts index 9d6dbc9280..acd29e2f3e 100644 --- a/packages/kernel-agents/src/types/capability.ts +++ b/packages/kernel-agents/src/types/capability.ts @@ -1,24 +1,15 @@ -import type { JsonSchema } from '@metamask/kernel-utils'; +import type { MethodSchema } from '@metamask/kernel-utils'; export type Capability, Return = null> = ( args: Args, ) => Promise; -export type CapabilitySchema = { - description: string; - args: Record; - returns?: JsonSchema; -}; - -export type ExtractRecordKeys = - Rec extends Record ? Key : never; - export type CapabilitySpec< Args extends Record = Record, Return = void, > = { func: Capability; - schema: CapabilitySchema>; + schema: MethodSchema; }; export type CapabilityRecord = Record< diff --git a/packages/kernel-agents/test/make-capability.ts b/packages/kernel-agents/test/make-capability.ts new file mode 100644 index 0000000000..6cb7c647ba --- /dev/null +++ b/packages/kernel-agents/test/make-capability.ts @@ -0,0 +1,35 @@ +import { S } from '@metamask/kernel-utils'; +import type { DescribedMethod } from '@metamask/kernel-utils'; + +import { makeInternalCapabilities } from '../src/capabilities/discover.ts'; +import type { CapabilitySpec } from '../src/types.ts'; + +/** + * Build a single capability backed by a pattern-guarded discoverable exo, for + * tests that need an ad-hoc capability. Mirrors how the built-in capabilities + * are authored, so the exo's interface guard enforces the method's argument + * shape — there is no membraneless authoring path. + * + * @param name - The exo/interface name. + * @param method - The method (and capability) name. + * @param impl - The method implementation (positional arguments). + * @param described - The method's guard and schema (use the `described*()` + * combinators from `@metamask/kernel-utils`). + * @returns The capability spec. + */ +export const makeCapability = ( + name: string, + method: string, + impl: (...args: never[]) => unknown, + described: DescribedMethod, +): CapabilitySpec => { + const capabilities = makeInternalCapabilities( + name, + { [method]: impl } as Record< + string, + (...args: never[]) => Promise + >, + S.interface(name, { [method]: described }), + ); + return capabilities[method] as CapabilitySpec; +}; diff --git a/yarn.lock b/yarn.lock index f5c8836960..cdce88f2c3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3989,7 +3989,6 @@ __metadata: "@metamask/kernel-errors": "workspace:^" "@metamask/kernel-utils": "workspace:^" "@metamask/logger": "workspace:^" - "@metamask/superstruct": "npm:^3.2.1" "@ocap/kernel-language-model-service": "workspace:^" "@ocap/repo-tools": "workspace:^" "@ts-bridge/cli": "npm:^0.6.3"