diff --git a/packages/kernel-agents/CHANGELOG.md b/packages/kernel-agents/CHANGELOG.md index 0c82cb1ed6..b91659d842 100644 --- a/packages/kernel-agents/CHANGELOG.md +++ b/packages/kernel-agents/CHANGELOG.md @@ -7,4 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### 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)) + [Unreleased]: https://github.com/MetaMask/ocap-kernel/ diff --git a/packages/kernel-agents/src/capabilities/discover.test.ts b/packages/kernel-agents/src/capabilities/discover.test.ts new file mode 100644 index 0000000000..cc3497f368 --- /dev/null +++ b/packages/kernel-agents/src/capabilities/discover.test.ts @@ -0,0 +1,88 @@ +import { S } from '@metamask/kernel-utils'; +import { describe, expect, it } from 'vitest'; + +import { makeInternalCapabilities } from './discover.ts'; + +/** + * Whether a promise rejects. Used instead of `.rejects.toThrow()` because the + * interface guard rejects with an opaque (non-`Error`) value under the test + * shim; here we only care that the membrane blocked the call. + * + * @param promise - The promise to observe. + * @returns `true` if the promise rejects, `false` if it resolves. + */ +const rejects = async (promise: Promise): Promise => { + try { + await promise; + return false; + } catch { + return true; + } +}; + +// Build the capabilities once, mirroring how the built-in capability modules +// construct their exo at import. +const capabilities = makeInternalCapabilities( + 'Test', + { + async count(word: string) { + return word.length; + }, + async add(summands: number[]) { + return summands.reduce((acc, summand) => acc + summand, 0); + }, + }, + S.interface('Test', { + count: S.method( + 'Count characters.', + [S.arg('word', S.string('The string to measure.'))], + S.number('The number of characters.'), + ), + add: S.method( + 'Add numbers.', + [S.arg('summands', S.arrayOf(S.number()))], + S.number('The sum.'), + ), + }), +); + +describe('makeInternalCapabilities', () => { + it('projects a capability record keyed by the schema method names', () => { + expect(Object.keys(capabilities)).toStrictEqual(['count', 'add']); + expect(capabilities.count.schema.description).toBe('Count characters.'); + }); + + it('maps the named-args object to positional args and invokes the method', async () => { + expect(await capabilities.count.func({ word: 'abcdefg' })).toBe(7); + expect(await capabilities.add.func({ summands: [1, 2, 3, 4] })).toBe(10); + }); + + it('rejects a mistyped argument at the exo membrane', async () => { + // `count` expects a string; the interface guard rejects a number before the + // implementation runs. + expect( + await rejects(capabilities.count.func({ word: 12345 } as never)), + ).toBe(true); + }); + + it('rejects a missing required argument at the exo membrane', async () => { + expect(await rejects(capabilities.add.func({} as never))).toBe(true); + }); + + it('throws at construction when an implementation has no matching schema', () => { + expect(() => + makeInternalCapabilities( + 'Skewed', + { + // Typo: the schema declares `search`, not `serch`. + async serch(query: string) { + return query; + }, + }, + S.interface('Skewed', { + search: S.method('Search.', [S.arg('query', S.string())], S.string()), + }), + ), + ).toThrow(/implementation and schema method names must match/u); + }); +}); diff --git a/packages/kernel-agents/src/capabilities/discover.ts b/packages/kernel-agents/src/capabilities/discover.ts index 7d4f060238..3ad9bd4fb6 100644 --- a/packages/kernel-agents/src/capabilities/discover.ts +++ b/packages/kernel-agents/src/capabilities/discover.ts @@ -1,12 +1,58 @@ import { E } from '@endo/eventual-send'; -import { GET_DESCRIPTION } from '@metamask/kernel-utils'; -import type { DiscoverableExo, MethodSchema } from '@metamask/kernel-utils'; +import { GET_DESCRIPTION, makeDiscoverableExo } from '@metamask/kernel-utils'; +import type { + DescribedInterface, + DiscoverableExo, + MethodSchema, +} from '@metamask/kernel-utils'; import type { CapabilityRecord, CapabilitySpec } from '../types.ts'; /** - * Discover the capabilities of a discoverable exo. Intended for use from inside a vat. - * This function fetches the schema from the discoverable exo and creates capabilities that can be used by kernel agents. + * Invoke a discoverable exo's method with positional arguments. The async + * variant ({@link discover}) sends over an eventual-send boundary; the local + * variant ({@link makeInternalCapabilities}) calls the in-realm exo directly. + * Either way the exo's interface guard enforces the argument shape. + */ +type Invoke = (method: string, positionalArgs: unknown[]) => unknown; + +/** + * Build a {@link CapabilityRecord} from a method-schema description, mapping each + * capability's object arguments to positional arguments for the exo method. + * + * IMPORTANT: this relies on each `schema.args` having keys in the same order as + * the method's parameters. Schemas authored with the `described*()` combinators + * (`@metamask/kernel-utils`) satisfy this by construction, since their `args` + * record is built in declared positional order. + * + * @param description - The exo's method schemas, keyed by method name. + * @param invoke - How to invoke a method with positional arguments. + * @returns The capability record. + */ +const capabilitiesFrom = ( + description: Record, + invoke: Invoke, +): CapabilityRecord => + Object.fromEntries( + Object.entries(description).map(([name, schema]) => { + const argNames = Object.keys(schema.args); + // eslint-disable-next-line @typescript-eslint/explicit-function-return-type + const func = async (args: Record) => + invoke( + name, + argNames.map((argName) => args[argName]), + ); + return [name, { func, schema }] as [ + string, + CapabilitySpec, + ]; + }), + ); + +/** + * Discover the capabilities of a (possibly remote) discoverable exo. Fetches the + * schema over an eventual-send boundary and creates capabilities that invoke the + * exo's methods the same way. * * @param exo - The discoverable exo to convert to a capability record. * @returns A promise for a capability record. @@ -19,35 +65,68 @@ export const discover = async ( string, MethodSchema >; - - const capabilities: CapabilityRecord = Object.fromEntries( - Object.entries(description).map(([name, schema]) => { - // Get argument names in order from the schema. - // IMPORTANT: This relies on the schema's args object having keys in the same - // order as the method's parameters. The schema must be defined with argument - // names matching the method parameter order (e.g., for method `add(a, b)`, - // the schema must have `args: { a: ..., b: ... }` in that order). - // JavaScript objects preserve insertion order for string keys, so Object.keys() - // will return keys in the order they were defined in the schema. - const argNames = Object.keys(schema.args); - - // Create a capability function that accepts an args object - // and maps it to positional arguments for the exo method - // eslint-disable-next-line @typescript-eslint/explicit-function-return-type - const func = async (args: Record) => { - // Map object arguments to positional arguments in schema order. - // The order of argNames matches the method parameter order by convention. - const positionalArgs = argNames.map((argName) => args[argName]); - // @ts-expect-error - E type doesn't remember method names - return E(exo)[name](...positionalArgs); - }; - - return [name, { func, schema }] as [ - string, - CapabilitySpec, - ]; - }), + return capabilitiesFrom(description, async (method, positionalArgs) => + // @ts-expect-error - E type doesn't remember method names + E(exo)[method](...positionalArgs), ); +}; - return capabilities; +/** + * Construct an in-realm capability record from a guard+schema description and + * the method implementations, building (and then keeping private) the + * pattern-guarded exo that enforces the argument shape on every call. + * + * Unlike {@link discover}, this never crosses an eventual-send boundary and + * never reads `GET_DESCRIPTION`: the schemas are the ones just authored with the + * `described*()` combinators (`@metamask/kernel-utils`), so there is no + * round-trip through the exo to recover what the caller already holds. The exo + * is used purely as the in-realm enforcement membrane and is not surfaced — + * internal capabilities are guarded closures, not passable exos. To expose a + * capability across a boundary, publish a {@link DiscoverableExo} and + * {@link discover} it instead. + * + * @param name - The exo/interface name. + * @param methods - The method implementations, keyed by method name. + * @param described - The interface guard and per-method schemas, e.g. from + * `S.interface(...)`. + * @returns A capability record keyed by the method names. + */ +export const makeInternalCapabilities = ( + name: string, + methods: Record Promise>, + described: DescribedInterface, +): CapabilityRecord => { + const { interfaceGuard, schemas } = described; + // The implementation and schema method sets must match exactly. A missing + // implementation already throws inside `makeDiscoverableExo`, but an extra + // implementation absent from the schema is silently accepted by the guard's + // `defaultGuards: 'passable'` and would never be reachable as a capability. + // Catch both here so an authoring typo (e.g. `serch` vs `search`) fails loudly + // at construction instead of surfacing as a capability that resolves to + // `undefined`. + const missing = Object.keys(schemas).filter((method) => !(method in methods)); + const extra = Object.keys(methods).filter((method) => !(method in schemas)); + if (missing.length > 0 || extra.length > 0) { + throw new Error( + `makeInternalCapabilities("${name}"): implementation and schema method names must match. ` + + `Schema methods without an implementation: [${missing.join(', ')}]; ` + + `implementations without a schema: [${extra.join(', ')}].`, + ); + } + const exo = makeDiscoverableExo( + name, + methods as Record unknown>, + schemas, + interfaceGuard, + ); + const dispatch = exo as unknown as Record< + string, + (...args: unknown[]) => unknown + >; + // Dispatch as a member call so the exo method keeps its `this` binding. The + // construction check above guarantees `method` is present, so the optional + // chain never short-circuits in practice. + return capabilitiesFrom(schemas, (method, positionalArgs) => + dispatch[method]?.(...positionalArgs), + ) as CapabilityRecord; }; diff --git a/packages/kernel-agents/src/capabilities/end.ts b/packages/kernel-agents/src/capabilities/end.ts index 9dda4ab279..82288b30e0 100644 --- a/packages/kernel-agents/src/capabilities/end.ts +++ b/packages/kernel-agents/src/capabilities/end.ts @@ -1,5 +1,7 @@ +import { S } from '@metamask/kernel-utils'; + +import { makeInternalCapabilities } from './discover.ts'; import { ifDefined } from '../utils.ts'; -import { capability } from './capability.ts'; /** * A factory function to make a task's `end` capability, which stores the first @@ -10,36 +12,41 @@ import { capability } from './capability.ts'; */ // eslint-disable-next-line @typescript-eslint/explicit-function-return-type export const makeEnd = () => { + // Captured, mutable state for the first final result. Intentionally NOT + // hardened: the exo method below closes over and mutates it. const result: { final?: Result; attachments?: Record } = {}; - const end = capability( - async ({ - final, - attachments, - }: { - final: Result; - attachments?: Record; - }): Promise => { - if (!Object.hasOwn(result, 'final')) { - Object.assign(result, { final, ...ifDefined({ attachments }) }); - } - }, + + const { end } = makeInternalCapabilities( + 'End', { - description: 'Return a final response to the user.', - args: { - final: { - required: true, - type: 'string', - description: - 'A concise final response that restates the requested information.', - }, - attachments: { - required: false, - type: 'object', - description: 'Attachments to the final response.', - }, + async end( + final: Result, + attachments?: Record, + ): Promise { + if (!Object.hasOwn(result, 'final')) { + Object.assign(result, { final, ...ifDefined({ attachments }) }); + } }, }, + S.interface('End', { + end: S.method( + 'Return a final response to the user.', + [ + S.arg( + 'final', + S.string( + 'A concise final response that restates the requested information.', + ), + ), + S.arg('attachments', S.record('Attachments to the final response.'), { + optional: true, + }), + ], + S.nothing(), + ), + }), ); + return [end, () => 'final' in result, () => result.final as Result] as const; }; diff --git a/packages/kernel-agents/src/capabilities/examples.ts b/packages/kernel-agents/src/capabilities/examples.ts index 6e1a9c9383..8b6cb1bd03 100644 --- a/packages/kernel-agents/src/capabilities/examples.ts +++ b/packages/kernel-agents/src/capabilities/examples.ts @@ -1,43 +1,12 @@ -import { capability } from './capability.ts'; +import { S } from '@metamask/kernel-utils'; + +import { makeInternalCapabilities } from './discover.ts'; type SearchResult = { source: string; published: string; snippet: string; }; -export const search = capability( - async ({ query }: { query: string }): Promise => [ - { - source: 'https://www.google.com', - published: '2025-01-01', - snippet: `No information found for ${query}`, - }, - ], - { - description: 'Search the web for information.', - args: { query: { type: 'string', description: 'The query to search for' } }, - returns: { - type: 'array', - items: { - type: 'object', - properties: { - source: { - type: 'string', - description: 'The source of the information.', - }, - published: { - type: 'string', - description: 'The date the information was published.', - }, - snippet: { - type: 'string', - description: 'The snippet of information.', - }, - }, - }, - }, - }, -); const moonPhases = [ 'new moon', @@ -51,20 +20,45 @@ const moonPhases = [ ] as const; type MoonPhase = (typeof moonPhases)[number]; -export const getMoonPhase = capability( - async (): Promise => - moonPhases[Math.floor(Math.random() * moonPhases.length)] as MoonPhase, +const capabilities = makeInternalCapabilities( + 'Examples', { - description: 'Get the current phase of the moon.', - args: {}, - returns: { - type: 'string', - // TODO: Add enum support to the capability schema - // @ts-expect-error - enum is not supported by the capability schema - enum: moonPhases, - description: 'The current phase of the moon.', + async search(query: string): Promise { + return [ + { + source: 'https://www.google.com', + published: '2025-01-01', + snippet: `No information found for ${query}`, + }, + ]; + }, + async getMoonPhase(): Promise { + return moonPhases[ + Math.floor(Math.random() * moonPhases.length) + ] as MoonPhase; }, }, + S.interface('Examples', { + search: S.method( + 'Search the web for information.', + [S.arg('query', S.string('The query to search for'))], + S.arrayOf( + S.object({ + source: S.string('The source of the information.'), + published: S.string('The date the information was published.'), + snippet: S.string('The snippet of information.'), + }), + ), + ), + // TODO: Add enum support to the capability schema so the moon phases can be + // advertised as the allowed return values. + getMoonPhase: S.method( + 'Get the current phase of the moon.', + [], + S.string('The current phase of the moon.'), + ), + }), ); -export const exampleCapabilities = { search, getMoonPhase }; +export const { search, getMoonPhase } = capabilities; +export const exampleCapabilities = capabilities; diff --git a/packages/kernel-agents/src/capabilities/math.ts b/packages/kernel-agents/src/capabilities/math.ts index b328716276..97d30ee620 100644 --- a/packages/kernel-agents/src/capabilities/math.ts +++ b/packages/kernel-agents/src/capabilities/math.ts @@ -1,44 +1,43 @@ -import { capability } from './capability.ts'; +import { S } from '@metamask/kernel-utils'; -export const count = capability( - async ({ word }: { word: string }) => word.length, +import { makeInternalCapabilities } from './discover.ts'; + +const capabilities = makeInternalCapabilities( + 'Math', { - description: 'Count the number of characters in an arbitrary string', - args: { - word: { type: 'string', description: 'The string to get the length of.' }, + async count(word: string) { + return word.length; }, - returns: { - type: 'number', - description: 'The number of characters in the string.', + async add(summands: number[]) { + return summands.reduce((acc, summand) => acc + summand, 0); }, - }, -); - -export const add = capability( - async ({ summands }: { summands: number[] }) => - summands.reduce((acc, summand) => acc + summand, 0), - { - description: 'Add a list of numbers.', - args: { summands: { type: 'array', items: { type: 'number' } } }, - returns: { type: 'number', description: 'The sum of the numbers.' }, - }, -); - -export const multiply = capability( - async ({ factors }: { factors: number[] }) => - factors.reduce((acc, factor) => acc * factor, 1), - { - description: 'Multiply a list of numbers.', - args: { - factors: { - type: 'array', - description: 'The list of numbers to multiply.', - items: { type: 'number' }, - }, + async multiply(factors: number[]) { + return factors.reduce((acc, factor) => acc * factor, 1); }, - returns: { type: 'number', description: 'The product of the factors.' }, }, + S.interface('Math', { + count: S.method( + 'Count the number of characters in an arbitrary string', + [S.arg('word', S.string('The string to get the length of.'))], + S.number('The number of characters in the string.'), + ), + add: S.method( + 'Add a list of numbers.', + [S.arg('summands', S.arrayOf(S.number()))], + S.number('The sum of the numbers.'), + ), + multiply: S.method( + 'Multiply a list of numbers.', + [ + S.arg( + 'factors', + S.arrayOf(S.number(), 'The list of numbers to multiply.'), + ), + ], + S.number('The product of the factors.'), + ), + }), ); -const capabilities = { count, add, multiply }; +export const { count, add, multiply } = capabilities; export default capabilities; diff --git a/packages/kernel-agents/vitest.config.ts b/packages/kernel-agents/vitest.config.ts index a04eee63fa..cf701cf144 100644 --- a/packages/kernel-agents/vitest.config.ts +++ b/packages/kernel-agents/vitest.config.ts @@ -1,4 +1,5 @@ import { mergeConfig } from '@ocap/repo-tools/vitest-config'; +import { fileURLToPath } from 'node:url'; import { defineConfig, defineProject } from 'vitest/config'; import defaultConfig from '../../vitest.config.ts'; @@ -13,6 +14,14 @@ export default defineConfig((args) => { include: ['src/**/*.test.ts'], // Exclude E2E setup test from regular test runs exclude: ['test/e2e'], + // Capability modules build discoverable exos at import, which needs a + // `harden` global; install the endoify mock for every test in the + // package so it is present before any capability module loads. + setupFiles: [ + fileURLToPath( + import.meta.resolve('@ocap/repo-tools/test-utils/mock-endoify'), + ), + ], }, }), );