From b5bbc0e5b95a6051bc4a5e00691ab445bde82934 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 09:38:44 +0000 Subject: [PATCH] fix(spec): the manifest `permissions` block names its surface and offers the rename MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016N6xmWt5hYm94ffVEwGH8x --- .changeset/permissions-block-named-refusal.md | 18 +++ .../src/kernel/manifest-unknown-keys.test.ts | 109 ++++++++++++++++++ packages/spec/src/kernel/manifest.zod.ts | 40 ++++--- 3 files changed, 154 insertions(+), 13 deletions(-) create mode 100644 .changeset/permissions-block-named-refusal.md diff --git a/.changeset/permissions-block-named-refusal.md b/.changeset/permissions-block-named-refusal.md new file mode 100644 index 0000000000..9f2cd2f1bf --- /dev/null +++ b/.changeset/permissions-block-named-refusal.md @@ -0,0 +1,18 @@ +--- +"@objectstack/spec": patch +--- + +The manifest `permissions` block's unknown-key refusal now names the surface and offers the rename, like every other block on the manifest. + +`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. + +It now uses the same `strictObject` helper as its neighbours, so the refusal reads: + +``` +Unrecognized key(s) on the `permissions` block of this package manifest: `hoooks`. +Did you mean `hoooks` → `hooks`? … +``` + +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`. + +**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. diff --git a/packages/spec/src/kernel/manifest-unknown-keys.test.ts b/packages/spec/src/kernel/manifest-unknown-keys.test.ts index 9b1582d0b8..215bf0fe69 100644 --- a/packages/spec/src/kernel/manifest-unknown-keys.test.ts +++ b/packages/spec/src/kernel/manifest-unknown-keys.test.ts @@ -356,3 +356,112 @@ describe('#14192 — the accept side does not move, and `main` is declared', () expect(issue.message).not.toContain('`capabilities`'); }); }); + +describe('#16328 — the `permissions` union door names the surface and the rename, like every other door', () => { + // ## What #16328 reported, and what was actually wrong + // + // The card measured `{ services: ['object'], hoooks: ['x'] }` refused with a + // keyless `invalid_union` at `['permissions']` and attributed it to + // `formatZodError` flattening the union's nested refusal away — the same + // premise #14722 was filed on. + // + // Re-measured on `origin/main` `f89812e4d`: that premise is FALSE, and the + // strictness ledger's `state-machine.zod.ts` row already says so in as many + // words — the flattening was lifted at #4971 and consolidated into + // `selectUnionBranches` (`shared/union-branch-policy.ts`) at #8318. + // `formatZodIssue` descends `invalid_union`, drops the `z.array(z.string())` + // arm as kind-mismatch-only, and renders the object arm verbatim. The RAW + // issue list is keyless; what the author reads is not. + // + // The real defect was one level in: `PluginPermissionsSchema` was the one + // closed object of the three known union doors that never adopted + // `strictObject`. Born `.strict()` at #1487, it never passed through the + // #4001 campaign, so its nested line was zod's own `Unrecognized key: + // "hoooks"` — the key echoed, but no surface and no rename, while its two + // sibling doors (`ManifestSchema` through `devPlugins[]`, and + // `ActionRef` / `GuardRef`) carry all three. + // + // So this pins the CONTENT of the nested line, not the union's shape. The + // union is deliberately untouched: reshaping it costs either the accept set + // or the published JSON Schema, which is the standing finding recorded on + // the `devPlugins[]` guard above. + const near = () => ({ ...legal(), permissions: { services: ['object'], hoooks: ['x'] } }); + + it('the author reads the key, the surface and the rename — through `formatZodError`', () => { + const result = ManifestSchema.safeParse(near()); + expect(result.success).toBe(false); + if (result.success) return; + const rendered = formatZodError(result.error); + expect(rendered).toContain('permissions'); + expect(rendered, 'the offending key is named').toContain('hoooks'); + expect(rendered, 'the surface is named').toContain('the `permissions` block of this package manifest'); + expect(rendered, 'the rename is offered').toContain('Did you mean `hoooks` → `hooks`?'); + }); + + it('the named refusal is the object arm\'s own issue, carried inside the union issue', () => { + // The raw shape, stated because it is the half the card measured: the + // top-level issue IS a keyless `invalid_union` and that is not the defect. + const result = ManifestSchema.safeParse(near()); + expect(result.success).toBe(false); + if (result.success) return; + const union = result.error.issues.find((i) => i.code === 'invalid_union') as + | { path: (string | number)[]; errors: Array> } + | undefined; + expect(union).toBeDefined(); + expect(union!.path).toEqual(['permissions']); + const nested = union!.errors.flat().find((i) => i.code === 'unrecognized_keys'); + expect(nested, 'the named refusal is carried inside the union issue').toBeDefined(); + expect(nested!.keys).toEqual(['hoooks']); + expect(nested!.message).toContain('Did you mean `hoooks` → `hooks`?'); + }); + + it('the accept set does not move — both arms of the union still parse', () => { + // #16328's negative control, and the reason a "fix" here could be worse + // than the defect. `strictObject` is `z.object(shape, { error }).strict()`: + // the shape and the strictness are unchanged, and an error map is consulted + // only once an issue is already being raised. + expect(ManifestSchema.safeParse({ ...legal(), permissions: { services: ['object'] } }).success).toBe(true); + expect(ManifestSchema.safeParse({ ...legal(), permissions: ['read', 'write'] }).success).toBe(true); + expect(ManifestSchema.safeParse({ ...legal(), permissions: [] }).success).toBe(true); + expect(ManifestSchema.safeParse({ + ...legal(), + permissions: { services: ['object'], hooks: ['record.beforeInsert'], network: ['api.acme.com'], fs: [] }, + }).success).toBe(true); + }); + + it('every declared key is accepted alone — the candidate list cannot have drifted from the shape', () => { + for (const key of ['services', 'hooks', 'network', 'fs']) { + expect( + ManifestSchema.safeParse({ ...legal(), permissions: { [key]: ['x'] } }).success, + `${key} is declared and must parse`, + ).toBe(true); + } + }); + + it('a spelled-out abbreviation reaches `fs`, which edit distance never could', () => { + const result = ManifestSchema.safeParse({ ...legal(), permissions: { filesystem: ['/tmp'] } }); + expect(result.success).toBe(false); + if (result.success) return; + expect(formatZodError(result.error)).toContain('Did you mean `filesystem` → `fs`?'); + }); + + it('the five doors that already named their key are untouched by this change', () => { + // The hard acceptance limb: this change adds an error map to ONE block, so + // no other door's message may move. Each of these is asserted in full + // elsewhere in this file; here they are re-read together as the regression + // baseline #16328 asked for. + const doors: Array<[string, Record, string]> = [ + ['manifest root', (() => { const { namespace: _n, ...r } = legal(); return { ...r, namesapce: 'probe' }; })(), 'Did you mean `namesapce` → `namespace`?'], + ['contributes', { ...legal(), contributes: { kind: [{ id: 'sys.bi.report' }] } }, 'Did you mean `kind` → `kinds`?'], + ['contributes.kinds entry', { ...legal(), contributes: { kinds: [{ id: 'sys.bi.report', descriptio: 'x' }] } }, 'Did you mean `descriptio` → `description`?'], + ['engines', { ...legal(), engines: { protocl: '^17' } }, 'Did you mean `protocl` → `protocol`?'], + ['engine (legacy)', { ...legal(), engine: { bogusKey: 'x' } }, 'Unrecognized key(s) on the legacy `engine` block'], + ]; + for (const [name, input, expected] of doors) { + const result = ManifestSchema.safeParse(input); + expect(result.success, `${name} must refuse`).toBe(false); + if (result.success) continue; + expect(formatZodError(result.error), `${name} keeps its message`).toContain(expected); + } + }); +}); diff --git a/packages/spec/src/kernel/manifest.zod.ts b/packages/spec/src/kernel/manifest.zod.ts index 70da3f2328..786ec80424 100644 --- a/packages/spec/src/kernel/manifest.zod.ts +++ b/packages/spec/src/kernel/manifest.zod.ts @@ -38,19 +38,33 @@ import { NavigationContributionSchema } from '../ui/app.zod'; * "network": ["api.acme.com"], "fs": [] } * ``` */ -export const PluginPermissionsSchema = z - .object({ - services: z.array(z.string()).optional() - .describe('Platform services the plugin may resolve (e.g. "object", "http")'), - hooks: z.array(z.string()).optional() - .describe('Lifecycle hooks the plugin may register (e.g. "record.beforeInsert")'), - network: z.array(z.string()).optional() - .describe('Network hosts the plugin may reach (e.g. "api.acme.com")'), - fs: z.array(z.string()).optional() - .describe('Filesystem paths the plugin may access'), - }) - .strict() - .describe('Structured plugin permission grants (ADR-0025 §3.2)'); +export const PluginPermissionsSchema = strictObject({ + surface: 'the `permissions` block of this package manifest', + history: + 'This block has refused unknown keys since it was introduced, but through zod\'s own ' + + 'bare message: a transposed `hoooks` was echoed back and nothing else — no surface, no ' + + 'rename — while every neighbouring block on this manifest named all three. Reaching ' + + 'the author one level down inside the `permissions` union made that the whole message, ' + + 'and this block decides which services, hooks, network hosts and filesystem paths the ' + + 'plugin may touch. The declared keys are `services`, `hooks`, `network` and `fs`.', + aliases: { + // Edit distance cannot reach a two-letter abbreviation from the word it + // abbreviates, and `fs` is the one key here an author is most likely to + // spell out in full. + filesystem: 'fs', + paths: 'fs', + hosts: 'network', + }, +}, { + services: z.array(z.string()).optional() + .describe('Platform services the plugin may resolve (e.g. "object", "http")'), + hooks: z.array(z.string()).optional() + .describe('Lifecycle hooks the plugin may register (e.g. "record.beforeInsert")'), + network: z.array(z.string()).optional() + .describe('Network hosts the plugin may reach (e.g. "api.acme.com")'), + fs: z.array(z.string()).optional() + .describe('Filesystem paths the plugin may access'), +}).describe('Structured plugin permission grants (ADR-0025 §3.2)'); export type PluginPermissions = z.input;