Skip to content

Commit 7f745c3

Browse files
claude[bot]claude
andauthored
fix(spec): the manifest permissions block names its surface and offers the rename (#16846)
`PluginPermissionsSchema` was the one closed object among the three known "strict object inside a union" doors that never adopted `strictObject`. Born `.strict()` with the ADR-0025 plugin-distribution work, it never passed through the unknown-key campaign, so its refusal was zod's own bare `Unrecognized key: "hoooks"` — the key echoed, but no surface and no rename, while every neighbouring block on the same manifest carried all three. The cause reported on the card — `formatZodError` flattening the union's nested refusal away — is false on this base. `formatZodIssue` descends `invalid_union` and ranks the arms through `selectUnionBranches` (`shared/union-branch-policy.ts`), dropping the `z.array(z.string())` arm as kind-mismatch-only and rendering the object arm verbatim. That flattening was lifted at #4971 and consolidated at #8318, and the strictness ledger's `state-machine.zod.ts` row already records it as spent. So the union is deliberately untouched: reshaping it costs either the accept set or the published JSON Schema and buys a message the author already has. The accept set does not move. `strictObject` is `z.object(shape, { error }).strict()`: same shape, same strictness, plus an error map consulted only once an issue is already being raised. Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0145680 commit 7f745c3

3 files changed

Lines changed: 154 additions & 13 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
The manifest `permissions` block's unknown-key refusal now names the surface and offers the rename, like every other block on the manifest.
6+
7+
`PluginPermissionsSchema` decides which services, hooks, network hosts and filesystem paths a plugin may touch. It has refused unknown keys since it was introduced, but through zod's own bare message: an author who transposed `hooks` as `hoooks` read `Unrecognized key: "hoooks"` — the key echoed back, with no surface name and no suggested spelling — while every neighbouring block on the same manifest (`contributes`, `contributes.kinds[]`, `engines`, the legacy `engine`, and the manifest root itself) named all three. Born closed at the ADR-0025 plugin-distribution work, it never passed through the unknown-key campaign that gave the others their error maps.
8+
9+
It now uses the same `strictObject` helper as its neighbours, so the refusal reads:
10+
11+
```
12+
Unrecognized key(s) on the `permissions` block of this package manifest: `hoooks`.
13+
Did you mean `hoooks` → `hooks`? …
14+
```
15+
16+
Three spelled-out near-misses that edit distance cannot reach are curated as aliases: `filesystem` and `paths` point at `fs`, and `hosts` points at `network`.
17+
18+
**The accept set does not move.** `strictObject` is `z.object(shape, { error }).strict()` — the declared keys and the strictness are unchanged, and an error map is consulted only once an issue is already being raised. The `permissions` union keeps both arms (the legacy flat string list and the structured block), and the union itself is untouched. Only the text of a refusal that already happened is different.

packages/spec/src/kernel/manifest-unknown-keys.test.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,3 +356,112 @@ describe('#14192 — the accept side does not move, and `main` is declared', ()
356356
expect(issue.message).not.toContain('`capabilities`');
357357
});
358358
});
359+
360+
describe('#16328 — the `permissions` union door names the surface and the rename, like every other door', () => {
361+
// ## What #16328 reported, and what was actually wrong
362+
//
363+
// The card measured `{ services: ['object'], hoooks: ['x'] }` refused with a
364+
// keyless `invalid_union` at `['permissions']` and attributed it to
365+
// `formatZodError` flattening the union's nested refusal away — the same
366+
// premise #14722 was filed on.
367+
//
368+
// Re-measured on `origin/main` `f89812e4d`: that premise is FALSE, and the
369+
// strictness ledger's `state-machine.zod.ts` row already says so in as many
370+
// words — the flattening was lifted at #4971 and consolidated into
371+
// `selectUnionBranches` (`shared/union-branch-policy.ts`) at #8318.
372+
// `formatZodIssue` descends `invalid_union`, drops the `z.array(z.string())`
373+
// arm as kind-mismatch-only, and renders the object arm verbatim. The RAW
374+
// issue list is keyless; what the author reads is not.
375+
//
376+
// The real defect was one level in: `PluginPermissionsSchema` was the one
377+
// closed object of the three known union doors that never adopted
378+
// `strictObject`. Born `.strict()` at #1487, it never passed through the
379+
// #4001 campaign, so its nested line was zod's own `Unrecognized key:
380+
// "hoooks"` — the key echoed, but no surface and no rename, while its two
381+
// sibling doors (`ManifestSchema` through `devPlugins[]`, and
382+
// `ActionRef` / `GuardRef`) carry all three.
383+
//
384+
// So this pins the CONTENT of the nested line, not the union's shape. The
385+
// union is deliberately untouched: reshaping it costs either the accept set
386+
// or the published JSON Schema, which is the standing finding recorded on
387+
// the `devPlugins[]` guard above.
388+
const near = () => ({ ...legal(), permissions: { services: ['object'], hoooks: ['x'] } });
389+
390+
it('the author reads the key, the surface and the rename — through `formatZodError`', () => {
391+
const result = ManifestSchema.safeParse(near());
392+
expect(result.success).toBe(false);
393+
if (result.success) return;
394+
const rendered = formatZodError(result.error);
395+
expect(rendered).toContain('permissions');
396+
expect(rendered, 'the offending key is named').toContain('hoooks');
397+
expect(rendered, 'the surface is named').toContain('the `permissions` block of this package manifest');
398+
expect(rendered, 'the rename is offered').toContain('Did you mean `hoooks` → `hooks`?');
399+
});
400+
401+
it('the named refusal is the object arm\'s own issue, carried inside the union issue', () => {
402+
// The raw shape, stated because it is the half the card measured: the
403+
// top-level issue IS a keyless `invalid_union` and that is not the defect.
404+
const result = ManifestSchema.safeParse(near());
405+
expect(result.success).toBe(false);
406+
if (result.success) return;
407+
const union = result.error.issues.find((i) => i.code === 'invalid_union') as
408+
| { path: (string | number)[]; errors: Array<Array<{ code: string; keys?: string[]; message?: string }>> }
409+
| undefined;
410+
expect(union).toBeDefined();
411+
expect(union!.path).toEqual(['permissions']);
412+
const nested = union!.errors.flat().find((i) => i.code === 'unrecognized_keys');
413+
expect(nested, 'the named refusal is carried inside the union issue').toBeDefined();
414+
expect(nested!.keys).toEqual(['hoooks']);
415+
expect(nested!.message).toContain('Did you mean `hoooks` → `hooks`?');
416+
});
417+
418+
it('the accept set does not move — both arms of the union still parse', () => {
419+
// #16328's negative control, and the reason a "fix" here could be worse
420+
// than the defect. `strictObject` is `z.object(shape, { error }).strict()`:
421+
// the shape and the strictness are unchanged, and an error map is consulted
422+
// only once an issue is already being raised.
423+
expect(ManifestSchema.safeParse({ ...legal(), permissions: { services: ['object'] } }).success).toBe(true);
424+
expect(ManifestSchema.safeParse({ ...legal(), permissions: ['read', 'write'] }).success).toBe(true);
425+
expect(ManifestSchema.safeParse({ ...legal(), permissions: [] }).success).toBe(true);
426+
expect(ManifestSchema.safeParse({
427+
...legal(),
428+
permissions: { services: ['object'], hooks: ['record.beforeInsert'], network: ['api.acme.com'], fs: [] },
429+
}).success).toBe(true);
430+
});
431+
432+
it('every declared key is accepted alone — the candidate list cannot have drifted from the shape', () => {
433+
for (const key of ['services', 'hooks', 'network', 'fs']) {
434+
expect(
435+
ManifestSchema.safeParse({ ...legal(), permissions: { [key]: ['x'] } }).success,
436+
`${key} is declared and must parse`,
437+
).toBe(true);
438+
}
439+
});
440+
441+
it('a spelled-out abbreviation reaches `fs`, which edit distance never could', () => {
442+
const result = ManifestSchema.safeParse({ ...legal(), permissions: { filesystem: ['/tmp'] } });
443+
expect(result.success).toBe(false);
444+
if (result.success) return;
445+
expect(formatZodError(result.error)).toContain('Did you mean `filesystem` → `fs`?');
446+
});
447+
448+
it('the five doors that already named their key are untouched by this change', () => {
449+
// The hard acceptance limb: this change adds an error map to ONE block, so
450+
// no other door's message may move. Each of these is asserted in full
451+
// elsewhere in this file; here they are re-read together as the regression
452+
// baseline #16328 asked for.
453+
const doors: Array<[string, Record<string, unknown>, string]> = [
454+
['manifest root', (() => { const { namespace: _n, ...r } = legal(); return { ...r, namesapce: 'probe' }; })(), 'Did you mean `namesapce` → `namespace`?'],
455+
['contributes', { ...legal(), contributes: { kind: [{ id: 'sys.bi.report' }] } }, 'Did you mean `kind` → `kinds`?'],
456+
['contributes.kinds entry', { ...legal(), contributes: { kinds: [{ id: 'sys.bi.report', descriptio: 'x' }] } }, 'Did you mean `descriptio` → `description`?'],
457+
['engines', { ...legal(), engines: { protocl: '^17' } }, 'Did you mean `protocl` → `protocol`?'],
458+
['engine (legacy)', { ...legal(), engine: { bogusKey: 'x' } }, 'Unrecognized key(s) on the legacy `engine` block'],
459+
];
460+
for (const [name, input, expected] of doors) {
461+
const result = ManifestSchema.safeParse(input);
462+
expect(result.success, `${name} must refuse`).toBe(false);
463+
if (result.success) continue;
464+
expect(formatZodError(result.error), `${name} keeps its message`).toContain(expected);
465+
}
466+
});
467+
});

packages/spec/src/kernel/manifest.zod.ts

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,33 @@ import { NavigationContributionSchema } from '../ui/app.zod';
3838
* "network": ["api.acme.com"], "fs": [] }
3939
* ```
4040
*/
41-
export const PluginPermissionsSchema = z
42-
.object({
43-
services: z.array(z.string()).optional()
44-
.describe('Platform services the plugin may resolve (e.g. "object", "http")'),
45-
hooks: z.array(z.string()).optional()
46-
.describe('Lifecycle hooks the plugin may register (e.g. "record.beforeInsert")'),
47-
network: z.array(z.string()).optional()
48-
.describe('Network hosts the plugin may reach (e.g. "api.acme.com")'),
49-
fs: z.array(z.string()).optional()
50-
.describe('Filesystem paths the plugin may access'),
51-
})
52-
.strict()
53-
.describe('Structured plugin permission grants (ADR-0025 §3.2)');
41+
export const PluginPermissionsSchema = strictObject({
42+
surface: 'the `permissions` block of this package manifest',
43+
history:
44+
'This block has refused unknown keys since it was introduced, but through zod\'s own '
45+
+ 'bare message: a transposed `hoooks` was echoed back and nothing else — no surface, no '
46+
+ 'rename — while every neighbouring block on this manifest named all three. Reaching '
47+
+ 'the author one level down inside the `permissions` union made that the whole message, '
48+
+ 'and this block decides which services, hooks, network hosts and filesystem paths the '
49+
+ 'plugin may touch. The declared keys are `services`, `hooks`, `network` and `fs`.',
50+
aliases: {
51+
// Edit distance cannot reach a two-letter abbreviation from the word it
52+
// abbreviates, and `fs` is the one key here an author is most likely to
53+
// spell out in full.
54+
filesystem: 'fs',
55+
paths: 'fs',
56+
hosts: 'network',
57+
},
58+
}, {
59+
services: z.array(z.string()).optional()
60+
.describe('Platform services the plugin may resolve (e.g. "object", "http")'),
61+
hooks: z.array(z.string()).optional()
62+
.describe('Lifecycle hooks the plugin may register (e.g. "record.beforeInsert")'),
63+
network: z.array(z.string()).optional()
64+
.describe('Network hosts the plugin may reach (e.g. "api.acme.com")'),
65+
fs: z.array(z.string()).optional()
66+
.describe('Filesystem paths the plugin may access'),
67+
}).describe('Structured plugin permission grants (ADR-0025 §3.2)');
5468

5569
export type PluginPermissions = z.input<typeof PluginPermissionsSchema>;
5670

0 commit comments

Comments
 (0)