Skip to content

Commit 7c3845b

Browse files
grypezclaude
andauthored
refactor(kernel-agents): make the exo membrane the sole capability arg enforcer (#960)
## Explanation The base PRs ([#958](#958), [#959](#959)) made every built-in capability a pattern-guarded discoverable exo. Now that the exo's interface guard already enforces each capability's argument shape, this PR retires the parallel membraneless authoring and validation paths so the guard is the single argument enforcer: - Removes the `capability()` authoring helper and the internal `validateCapabilityArgs` validator (and its now-dead module). The chat strategy no longer re-validates arguments before invoking — it relies on the guard rejection it catches and reports as an `Error calling …` tool message. That catch is hardened to handle a non-`Error` rejection, so an invalid-argument tool call surfaces as a tool error instead of crashing the task (covered by a new regression test). - Collapses the redundant `CapabilitySchema` type into kernel-utils' `MethodSchema` (a capability's `schema` is exactly the `MethodSchema` its exo describes), removing the parallel type and its `ExtractRecordKeys` helper. - Adds a `test/make-method-capability.ts` helper that builds a guarded, discovered single-method capability from an `S.method`, and migrates the chat and JSON evaluator tests (and the capability test, repurposed to cover the surviving `extract*` helpers) onto it. - Drops the now-unused `@metamask/superstruct` dependency. ### Breaking changes - The `capability()` authoring helper is no longer exported from `@ocap/kernel-agents/capabilities/capability`. Author capabilities as pattern-guarded discoverable exos (via the `described*()` combinators in `@metamask/kernel-utils`) and convert them with `discover`. (`validateCapabilityArgs` was internal and never exported.) ## Test plan - [x] \`yarn workspace @ocap/kernel-agents test:dev:quiet\` (56 pass), incl. a chat-strategy regression test that an invalid-argument tool call comes back as an \`Error calling …\` tool message instead of crashing the task - [x] \`yarn workspace @ocap/kernel-agents-repl test:dev:quiet\` (178 pass) - [x] \`build\` + \`lint\` for both packages; changelog validates <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Breaking public API (`capability()` removal) affects downstream authors, but runtime behavior stays aligned with prior exo-backed builtins; main risk is consumers still using the old helper or assuming pre-invoke Superstruct errors. > > **Overview** > **Breaking:** Removes the exported `capability()` helper and the internal Superstruct-based `validateCapabilityArgs` path. Capabilities are expected to be authored as pattern-guarded discoverable exos (`described*()` + `discover` / `makeInternalCapabilities`); `CapabilitySpec.schema` is now kernel-utils `MethodSchema` instead of a parallel `CapabilitySchema` type. > > Invocation errors from the exo interface guard are normalized in `capabilitiesFrom` to `Error calling <name>(<params>): …` so chat and other callers can surface actionable tool messages without a second validation layer. The chat agent parses tool JSON locally when needed, invokes capabilities directly, and pushes guard/implementation failures as tool errors (including a regression test for bad args) instead of crashing the loop. > > Tests migrate to `test/make-method-capability.ts`; `@metamask/superstruct` is dropped from dependencies. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 6dc44a8. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c631ac4 commit 7c3845b

15 files changed

Lines changed: 236 additions & 179 deletions

packages/kernel-agents/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- 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))
1313

14+
### Removed
15+
16+
- **BREAKING:** Remove the `capability()` authoring helper from `@ocap/kernel-agents/capabilities/capability`. Author capabilities as pattern-guarded discoverable exos (via the `described*()` combinators in `@metamask/kernel-utils`) and convert them with `discover`, so the exo's interface guard is the sole argument enforcer ([#960](https://github.com/MetaMask/ocap-kernel/pull/960))
17+
1418
[Unreleased]: https://github.com/MetaMask/ocap-kernel/

packages/kernel-agents/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,6 @@
196196
"@metamask/kernel-errors": "workspace:^",
197197
"@metamask/kernel-utils": "workspace:^",
198198
"@metamask/logger": "workspace:^",
199-
"@metamask/superstruct": "^3.2.1",
200199
"@ocap/kernel-language-model-service": "workspace:^",
201200
"partial-json": "^0.1.7",
202201
"ses": "^1.14.0"
Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,32 @@
1+
import { S } from '@metamask/kernel-utils';
12
import { describe, it, expect } from 'vitest';
23

3-
import { capability } from './capability.ts';
4+
import { extractCapabilities, extractCapabilitySchemas } from './capability.ts';
5+
import { makeMethodCapability } from '../../test/make-method-capability.ts';
46

5-
describe('capability', () => {
6-
it('creates a capability with func and schema', () => {
7-
const testCapability = capability(async () => Promise.resolve('test'), {
8-
description: 'a test capability',
9-
args: {},
10-
});
11-
expect(testCapability.func).toBeInstanceOf(Function);
12-
expect(testCapability.schema).toStrictEqual({
13-
description: 'a test capability',
7+
describe('capability extraction', () => {
8+
const makeRecord = () => ({
9+
ping: makeMethodCapability(
10+
'Server',
11+
'ping',
12+
async () => 'pong',
13+
S.method('Ping', [], S.string()),
14+
),
15+
});
16+
17+
it('extractCapabilities returns the functions keyed by name', async () => {
18+
const funcs = extractCapabilities(makeRecord());
19+
expect(Object.keys(funcs)).toStrictEqual(['ping']);
20+
expect(await funcs.ping(undefined as never)).toBe('pong');
21+
});
22+
23+
it('extractCapabilitySchemas returns the schemas keyed by name', () => {
24+
const schemas = extractCapabilitySchemas(makeRecord());
25+
expect(schemas.ping).toStrictEqual({
26+
description: 'Ping',
1427
args: {},
28+
required: [],
29+
returns: { type: 'string' },
1530
});
1631
});
1732
});

packages/kernel-agents/src/capabilities/capability.ts

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,8 @@
1-
import type { ExtractRecordKeys } from '../types/capability.ts';
2-
import type {
3-
CapabilityRecord,
4-
CapabilitySpec,
5-
CapabilitySchema,
6-
Capability,
7-
} from '../types.ts';
1+
import type { MethodSchema } from '@metamask/kernel-utils';
82

9-
/**
10-
* Create a capability specification.
11-
*
12-
* @param func - The function to create a capability specification for
13-
* @param schema - The schema for the capability
14-
* @returns A capability specification
15-
*/
16-
export const capability = <Args extends Record<string, unknown>, Return = null>(
17-
func: Capability<Args, Return>,
18-
schema: CapabilitySchema<ExtractRecordKeys<Args>>,
19-
): CapabilitySpec<Args, Return> => ({ func, schema });
3+
import type { CapabilityRecord, CapabilitySpec } from '../types.ts';
204

21-
type SchemaEntry = [string, { schema: CapabilitySchema<string> }];
5+
type SchemaEntry = [string, { schema: MethodSchema }];
226
/**
237
* Extract only the serializable schemas from the capabilities
248
*

packages/kernel-agents/src/capabilities/discover.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,24 @@ describe('makeInternalCapabilities', () => {
6969
expect(await rejects(capabilities.add.func({} as never))).toBe(true);
7070
});
7171

72+
it('normalizes the rejection into a real error naming the expected signature', async () => {
73+
// Whatever the guard rejects with (an opaque value under the test shim), the
74+
// membrane rethrows a real `Error` carrying the method signature so callers
75+
// can surface an actionable message to the model.
76+
let caught: unknown;
77+
try {
78+
await capabilities.count.func({ word: 12345 } as never);
79+
} catch (error) {
80+
caught = error;
81+
}
82+
expect(caught).toBeInstanceOf(Error);
83+
expect(
84+
(caught as Error).message.startsWith(
85+
'Error calling count(word: string): ',
86+
),
87+
).toBe(true);
88+
});
89+
7290
it('throws at construction when an implementation has no matching schema', () => {
7391
expect(() =>
7492
makeInternalCapabilities(

packages/kernel-agents/src/capabilities/discover.ts

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,21 @@ import type { CapabilityRecord, CapabilitySpec } from '../types.ts';
1616
*/
1717
type Invoke = (method: string, positionalArgs: unknown[]) => unknown;
1818

19+
/**
20+
* Render a method's expected call signature from its schema — e.g.
21+
* `add(a: number, b: number)` — for use in invocation-error messages.
22+
*
23+
* @param name - The method name.
24+
* @param schema - The method schema whose `args` describe the parameters.
25+
* @returns The formatted signature.
26+
*/
27+
const formatSignature = (name: string, schema: MethodSchema): string => {
28+
const params = Object.entries(schema.args)
29+
.map(([arg, argSchema]) => `${arg}: ${argSchema.type}`)
30+
.join(', ');
31+
return `${name}(${params})`;
32+
};
33+
1934
/**
2035
* Build a {@link CapabilityRecord} from a method-schema description, mapping each
2136
* capability's object arguments to positional arguments for the exo method.
@@ -36,12 +51,26 @@ const capabilitiesFrom = (
3651
Object.fromEntries(
3752
Object.entries(description).map(([name, schema]) => {
3853
const argNames = Object.keys(schema.args);
39-
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
40-
const func = async (args: Record<string, unknown>) =>
41-
invoke(
42-
name,
43-
argNames.map((argName) => args[argName]),
44-
);
54+
const func = async (args: Record<string, unknown>): Promise<unknown> => {
55+
try {
56+
return await invoke(
57+
name,
58+
argNames.map((argName) => args[argName]),
59+
);
60+
} catch (error) {
61+
// The exo's interface guard is the sole argument enforcer, so a shape
62+
// mismatch rejects here before the implementation runs — but that is
63+
// indistinguishable from an error thrown by the implementation, so
64+
// the signature is reported as context, not a diagnosed cause.
65+
// Wrapping also guarantees a real `Error` even when the guard rejects
66+
// with an opaque value (e.g. under the test shim), so every caller
67+
// gets the method signature to surface to the model.
68+
const detail = error instanceof Error ? error.message : String(error);
69+
throw new Error(
70+
`Error calling ${formatSignature(name, schema)}: ${detail}`,
71+
);
72+
}
73+
};
4574
return [name, { func, schema }] as [
4675
string,
4776
CapabilitySpec<never, unknown>,

packages/kernel-agents/src/capabilities/validate-capability-args.test.ts

Lines changed: 0 additions & 59 deletions
This file was deleted.

packages/kernel-agents/src/capabilities/validate-capability-args.ts

Lines changed: 0 additions & 17 deletions
This file was deleted.

packages/kernel-agents/src/strategies/chat-agent.test.ts

Lines changed: 79 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import '@ocap/repo-tools/test-utils/mock-endoify';
22

3+
import { S } from '@metamask/kernel-utils';
34
import type {
45
ChatMessage,
56
ChatResult,
@@ -9,7 +10,7 @@ import { describe, expect, it, vi } from 'vitest';
910

1011
import { makeChatAgent } from './chat-agent.ts';
1112
import type { BoundChat } from './chat-agent.ts';
12-
import { capability } from '../capabilities/capability.ts';
13+
import { makeMethodCapability } from '../../test/make-method-capability.ts';
1314

1415
const makeToolCall = (
1516
id: string,
@@ -62,15 +63,17 @@ describe('makeChatAgent', () => {
6263
});
6364

6465
it('dispatches a tool call and returns final text answer', async () => {
65-
const add = vi.fn(async ({ a, b }: { a: number; b: number }) => a + b);
66-
const addCap = capability(add, {
67-
description: 'Add two numbers',
68-
args: {
69-
a: { type: 'number' },
70-
b: { type: 'number' },
71-
},
72-
returns: { type: 'number' },
73-
});
66+
const add = vi.fn(async (a: number, b: number) => a + b);
67+
const addCap = makeMethodCapability(
68+
'Math',
69+
'add',
70+
add,
71+
S.method(
72+
'Add two numbers',
73+
[S.arg('a', S.number()), S.arg('b', S.number())],
74+
S.number(),
75+
),
76+
);
7477

7578
let call = 0;
7679
const chat: BoundChat = async () => {
@@ -86,17 +89,18 @@ describe('makeChatAgent', () => {
8689
const agent = makeChatAgent({ chat, capabilities: { add: addCap } });
8790

8891
const result = await agent.task('add 3 and 4');
89-
expect(add).toHaveBeenCalledWith({ a: 3, b: 4 });
92+
expect(add).toHaveBeenCalledWith(3, 4);
9093
expect(result).toBe('7');
9194
});
9295

9396
it('injects tool result message before next turn', async () => {
9497
const recorded: ChatMessage[][] = [];
95-
const ping = capability(async () => 'pong', {
96-
description: 'Ping',
97-
args: {},
98-
returns: { type: 'string' },
99-
});
98+
const ping = makeMethodCapability(
99+
'Server',
100+
'ping',
101+
async () => 'pong',
102+
S.method('Send a ping', [], S.string()),
103+
);
100104

101105
let call = 0;
102106
const chat: BoundChat = async ({ messages }) => {
@@ -151,11 +155,60 @@ describe('makeChatAgent', () => {
151155
).toBe(true);
152156
});
153157

158+
it('injects a tool error for an invalid-argument tool call and continues', async () => {
159+
const add = vi.fn(async (a: number, b: number) => a + b);
160+
const addCap = makeMethodCapability(
161+
'Math',
162+
'add',
163+
add,
164+
S.method(
165+
'Add two numbers',
166+
[S.arg('a', S.number()), S.arg('b', S.number())],
167+
S.number(),
168+
),
169+
);
170+
171+
const recorded: ChatMessage[][] = [];
172+
let call = 0;
173+
const chat: BoundChat = async ({ messages }) => {
174+
recorded.push([...messages]);
175+
call += 1;
176+
if (call === 1) {
177+
// `b` is missing, so the exo's interface guard rejects the call.
178+
return makeToolCallResponse('0', [makeToolCall('c1', 'add', { a: 3 })]);
179+
}
180+
return makeTextResponse('recovered');
181+
};
182+
183+
const agent = makeChatAgent({ chat, capabilities: { add: addCap } });
184+
const result = await agent.task('add 3 and ?');
185+
186+
// The guard rejection surfaces as a tool error rather than crashing the
187+
// task, and the implementation never runs with the bad arguments.
188+
expect(result).toBe('recovered');
189+
expect(add).not.toHaveBeenCalled();
190+
const secondTurn = recorded[1] ?? [];
191+
// The membrane normalizes the rejection into a real error carrying the
192+
// expected signature, so the message is actionable even when the guard
193+
// itself rejects with an opaque value (as it does under the test shim).
194+
expect(
195+
secondTurn.some(
196+
(message) =>
197+
message.role === 'tool' &&
198+
message.content.startsWith(
199+
'Error calling add(a: number, b: number):',
200+
),
201+
),
202+
).toBe(true);
203+
});
204+
154205
it('throws when invocation budget is exceeded', async () => {
155-
const ping = capability(async () => 'pong', {
156-
description: 'Ping',
157-
args: {},
158-
});
206+
const ping = makeMethodCapability(
207+
'Server',
208+
'ping',
209+
async () => 'pong',
210+
S.method('Send a ping', [], S.string()),
211+
);
159212
const chat: BoundChat = async () =>
160213
makeToolCallResponse('0', [makeToolCall('c1', 'ping', {})]);
161214

@@ -177,11 +230,12 @@ describe('makeChatAgent', () => {
177230

178231
it('passes tools to the chat function', async () => {
179232
const recordedTools: unknown[] = [];
180-
const ping = capability(async () => 'pong', {
181-
description: 'Ping the server',
182-
args: {},
183-
returns: { type: 'string' },
184-
});
233+
const ping = makeMethodCapability(
234+
'Server',
235+
'ping',
236+
async () => 'pong',
237+
S.method('Ping the server', [], S.string()),
238+
);
185239

186240
const chat: BoundChat = async ({ tools }) => {
187241
recordedTools.push(tools);

0 commit comments

Comments
 (0)