diff --git a/packages/kernel-agents/CHANGELOG.md b/packages/kernel-agents/CHANGELOG.md index 276ea3dde3..236742b58b 100644 --- a/packages/kernel-agents/CHANGELOG.md +++ b/packages/kernel-agents/CHANGELOG.md @@ -14,5 +14,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 +- 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 + +### 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 [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..b68bfb1063 100644 --- a/packages/kernel-agents/src/capabilities/capability.ts +++ b/packages/kernel-agents/src/capabilities/capability.ts @@ -1,23 +1,9 @@ -import type { ExtractRecordKeys } from '../types/capability.ts'; import type { CapabilityRecord, CapabilitySpec, CapabilitySchema, - Capability, } from '../types.ts'; -/** - * 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 }); - type SchemaEntry = [string, { schema: CapabilitySchema }]; /** * 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/test/make-capability.ts b/packages/kernel-agents/test/make-capability.ts new file mode 100644 index 0000000000..ccb04b1d93 --- /dev/null +++ b/packages/kernel-agents/test/make-capability.ts @@ -0,0 +1,40 @@ +import { S, makeDiscoverableExo } from '@metamask/kernel-utils'; +import type { DescribedMethod } from '@metamask/kernel-utils'; + +import { discoverLocal } 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 { interfaceGuard, schemas } = S.interface(name, { + [method]: described, + }); + const exo = makeDiscoverableExo( + name, + { [method]: impl } as Record unknown>, + schemas, + interfaceGuard, + ); + const record = discoverLocal(exo) as Record< + string, + CapabilitySpec + >; + return record[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"