Skip to content

Commit 24644c5

Browse files
grypezclaude
andcommitted
fix(kernel-utils): make described combinators' schema faithful to their guards
Resolve two review findings on the described() combinators: - S.object is documented as a closed/shaped object but M.splitRecord defaults its rest pattern to M.any(), so neither the pattern nor the schema rejected extra keys. Close both: pass an empty-record rest pattern and emit additionalProperties: false (which routes jsonSchemaToStruct through its strict branch). - S.method emits optional trailing args via M.callWhen(...).optional(...), but MethodSchema could not express optionality, so methodArgsToStruct and method-schema-convert treated every arg as required. Add an optional `required` field to MethodSchema (mirroring object JsonSchema's `required`), populate it from S.method, honor it in methodArgsToStruct via a `{ required }` option, and map it to ValueSpec.optional in method-schema-convert. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 6bc1fee commit 24644c5

8 files changed

Lines changed: 90 additions & 11 deletions

File tree

packages/kernel-utils/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111

1212
- Add a `./described` export with a combinator namespace `S` (`S.string`/`S.number`/`S.boolean`/`S.arrayOf`/`S.record`/`S.object`/`S.nothing` leaves, plus `S.arg`/`S.method`/`S.interface`) that authors an `@endo/patterns` interface guard and a matching `MethodSchema` from a single source, so a discoverable exo's enforced shape and its `__getDescription__` hint cannot drift ([#958](https://github.com/MetaMask/ocap-kernel/pull/958))
13+
- Add an optional `required` field to `MethodSchema` (mirroring `required` on object `JsonSchema`) naming which arguments are required, and a `{ required }` option on `methodArgsToStruct` that validates unlisted arguments as optional, so a method's argument schema can faithfully represent the optional trailing arguments its guard already allows ([#958](https://github.com/MetaMask/ocap-kernel/pull/958))
1314
- Add `getLibp2pRelayHome()` to the `./nodejs` exports, returning the libp2p relay's bookkeeping directory (default `~/.libp2p-relay`, overridable via `$LIBP2P_RELAY_HOME`) — kept separate from `$OCAP_HOME` so one relay can serve daemons with different OCAP_HOMEs ([#952](https://github.com/MetaMask/ocap-kernel/pull/952))
1415
- `startRelay()` accepts an optional `publicIp` that is fed to libp2p's `appendAnnounce`, so a relay running on a NAT-backed host can announce its publicly-reachable IPv4 alongside its bound NIC addresses ([#952](https://github.com/MetaMask/ocap-kernel/pull/952))
1516

packages/kernel-utils/src/described.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,19 @@ describe('leaves', () => {
8282
type: 'object',
8383
properties: { id: { type: 'string' }, label: { type: 'string' } },
8484
required: ['id'],
85+
additionalProperties: false,
8586
});
8687
expect(matches({ id: 'x' }, described.pattern)).toBe(true);
8788
expect(matches({ id: 'x', label: 'y' }, described.pattern)).toBe(true);
8889
expect(matches({ label: 'y' }, described.pattern)).toBe(false);
8990
});
9091

92+
it('rejects extra keys on a closed object leaf', () => {
93+
const described = S.object({ id: S.string() });
94+
expect(matches({ id: 'x' }, described.pattern)).toBe(true);
95+
expect(matches({ id: 'x', extra: 1 }, described.pattern)).toBe(false);
96+
});
97+
9198
it('builds a void return leaf with no schema', () => {
9299
const described = S.nothing();
93100
expect(described.schema).toBeUndefined();
@@ -106,6 +113,7 @@ describe('S.method', () => {
106113
expect(method.schema).toStrictEqual({
107114
description: 'Add a list of numbers.',
108115
args: { summands: { type: 'array', items: { type: 'number' } } },
116+
required: ['summands'],
109117
returns: { type: 'number', description: 'The sum of the numbers.' },
110118
});
111119
const payload = payloadOf(method.guard);
@@ -143,6 +151,9 @@ describe('S.method', () => {
143151
additionalProperties: true,
144152
},
145153
});
154+
// The optional arg is omitted from `required`, so the schema's enforced
155+
// shape matches the guard's optional trailing arg.
156+
expect(method.schema.required).toStrictEqual(['final']);
146157
});
147158

148159
it('handles a no-arg method', () => {

packages/kernel-utils/src/described.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,10 @@ const record = (description?: string): Described =>
135135
});
136136

137137
/**
138-
* A closed/shaped object leaf: matches a record with the given properties,
139-
* where keys not listed in `optional` are required. Describes
140-
* `{ type: 'object', properties, required }`.
138+
* A closed/shaped object leaf: matches a record with exactly the given
139+
* properties (extra keys are rejected), where keys not listed in `optional` are
140+
* required. Describes `{ type: 'object', properties, required,
141+
* additionalProperties: false }`.
141142
*
142143
* @param properties - The described properties, keyed by name.
143144
* @param options - Options bag.
@@ -165,9 +166,16 @@ const object = (
165166
}
166167
}
167168
return harden({
168-
pattern: M.splitRecord(requiredPatterns, optionalPatterns),
169+
// The empty-record rest pattern closes the record: keys beyond those listed
170+
// are rejected, matching the schema's `additionalProperties: false`.
171+
pattern: M.splitRecord(requiredPatterns, optionalPatterns, {}),
169172
schema: withDescription(
170-
{ type: 'object', properties: schemaProperties, required },
173+
{
174+
type: 'object',
175+
properties: schemaProperties,
176+
required,
177+
additionalProperties: false,
178+
},
171179
description,
172180
),
173181
});
@@ -242,6 +250,7 @@ const describedMethod = (
242250
const schema: MethodSchema = {
243251
description,
244252
args: schemaArgs,
253+
required: required.map((each) => each.name),
245254
...(returns.schema === undefined ? {} : { returns: returns.schema }),
246255
};
247256

packages/kernel-utils/src/json-schema-to-struct.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,19 @@ describe('methodArgsToStruct', () => {
6363
expect(() => assert({ a: 1 }, struct)).toThrow(/path: b/u);
6464
});
6565

66+
it('treats args absent from `required` as optional', () => {
67+
const struct = methodArgsToStruct(
68+
{
69+
final: { type: 'string' },
70+
attachments: { type: 'object', properties: {} },
71+
},
72+
{ required: ['final'] },
73+
);
74+
assert({ final: 'done' }, struct);
75+
assert({ final: 'done', attachments: { note: 1 } }, struct);
76+
expect(() => assert({ attachments: {} }, struct)).toThrow(/final/u);
77+
});
78+
6679
it('accepts an empty args map', () => {
6780
assert({}, methodArgsToStruct({}));
6881
});

packages/kernel-utils/src/json-schema-to-struct.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,19 @@ export function jsonSchemaToStruct(schema: JsonSchema): Struct<unknown> {
9696

9797
/**
9898
* Build a Superstruct object struct for a method/capability `args` map
99-
* (name → per-argument {@link JsonSchema}). All listed arguments are required.
99+
* (name → per-argument {@link JsonSchema}). Arguments not named in
100+
* `options.required` are validated as optional; an absent `options.required`
101+
* treats every argument as required.
100102
*
101103
* @param args - Same shape as {@link MethodSchema.args}.
104+
* @param options - Options bag.
105+
* @param options.required - Names of the required arguments. Mirrors
106+
* {@link MethodSchema.required}; absent means all arguments are required.
102107
* @returns A struct that validates a plain object with one field per declared argument.
103108
*/
104109
export function methodArgsToStruct(
105110
args: Record<string, JsonSchema>,
111+
options: { required?: string[] } = {},
106112
): Struct<Record<string, unknown>> {
107113
const entries = Object.entries(args);
108114
if (entries.length === 0) {
@@ -113,8 +119,12 @@ export function methodArgsToStruct(
113119
return true;
114120
}) as Struct<Record<string, unknown>>;
115121
}
122+
const required = new Set(options.required ?? Object.keys(args));
116123
const shape = Object.fromEntries(
117-
entries.map(([name, jsonSchema]) => [name, jsonSchemaToStruct(jsonSchema)]),
124+
entries.map(([name, jsonSchema]) => {
125+
const fieldStruct = jsonSchemaToStruct(jsonSchema);
126+
return [name, required.has(name) ? fieldStruct : optional(fieldStruct)];
127+
}),
118128
);
119129
return object(shape) as Struct<Record<string, unknown>>;
120130
}

packages/kernel-utils/src/schema.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ export type MethodSchema = {
5050
* Each argument includes its type and description.
5151
*/
5252
args: Record<string, JsonSchema>;
53+
/**
54+
* Names of the required arguments. Mirrors {@link ObjectJsonSchema.required}:
55+
* an argument not listed here may be omitted by the caller, and an absent
56+
* `required` means every argument in `args` is required.
57+
*/
58+
required?: string[];
5359
/**
5460
* Return value schema, including type and description.
5561
*/

packages/service-discovery-types/src/index.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,30 @@ describe('methodSchemaToMethodSpec', () => {
228228
});
229229
});
230230

231+
it('marks args absent from `required` as optional parameters', () => {
232+
const spec: MethodSpec = methodSchemaToMethodSpec({
233+
description: 'send a message',
234+
args: {
235+
text: { type: 'string' },
236+
attachments: { type: 'object', properties: {} },
237+
},
238+
required: ['text'],
239+
returns: { type: 'string' },
240+
});
241+
expect(spec).toStrictEqual({
242+
description: 'send a message',
243+
parameters: [
244+
{ description: 'text', type: { kind: 'string' } },
245+
{
246+
description: 'attachments',
247+
type: { kind: 'object', spec: { properties: {} } },
248+
optional: true,
249+
},
250+
],
251+
returnType: { kind: 'string' },
252+
});
253+
});
254+
231255
it('defaults returnType to void when unspecified', () => {
232256
expect(
233257
methodSchemaToMethodSpec({ description: 'do a thing', args: {} }),

packages/service-discovery-types/src/method-schema-convert.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111
* use the iteration order of the args record, and we drop the names. The
1212
* names are preserved as `ValueSpec.description` if no description was
1313
* otherwise present, so they remain human-readable.
14-
* - `MethodSchema` does not mark individual args as optional; the converter
15-
* treats all parameters as required.
1614
* - `JsonSchema` has no notion of `remotable`, `null`, `void`, `bigint`,
1715
* `unknown`, or `union`. The converter never emits those kinds.
1816
*/
@@ -94,17 +92,24 @@ export function jsonSchemaToObjectSpec(
9492
* Because `MethodSchema.args` is a named record while `MethodSpec.parameters`
9593
* is a positional array, the parameters are emitted in the iteration order of
9694
* the args record, and each parameter's name is preserved in its
97-
* `description` field when the source did not supply one.
95+
* `description` field when the source did not supply one. An argument absent
96+
* from `schema.required` becomes an optional parameter; an absent `required`
97+
* treats every parameter as required.
9898
*
9999
* @param schema - The source MethodSchema.
100100
* @returns The equivalent MethodSpec.
101101
*/
102102
export function methodSchemaToMethodSpec(schema: MethodSchema): MethodSpec {
103+
const required = new Set(schema.required ?? Object.keys(schema.args));
103104
const parameters: ValueSpec[] = [];
104105
for (const [name, argSchema] of Object.entries(schema.args)) {
105106
const type = jsonSchemaToTypeSpec(argSchema);
106107
const description = argSchema.description ?? name;
107-
parameters.push({ description, type });
108+
const parameter: ValueSpec = { description, type };
109+
if (!required.has(name)) {
110+
parameter.optional = true;
111+
}
112+
parameters.push(parameter);
108113
}
109114
const returnType: TypeSpec = schema.returns
110115
? jsonSchemaToTypeSpec(schema.returns)

0 commit comments

Comments
 (0)