From 88f08412a090ae0b21ae82fbe1f41ab74d4d7f67 Mon Sep 17 00:00:00 2001 From: os-project-manager Date: Sun, 9 Aug 2026 04:39:34 +0000 Subject: [PATCH] refactor(spec): fold the two residual unrecognized_keys maps and close the class (#6805) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #6416's blind spot was declared closed by #6619/PR #6804, but its inventory was two short. `strictToolError` (ai/tool.zod.ts) and `strictCapabilitiesError` (data/object.zod.ts) were the same shape — `unrecognized_keys` prescription tables attached to a `.strict()` object via `{ error: … }`, seen by no registry — and `TOOL_RETIRED_KEY_GUIDANCE` is a hand-maintained per-key retirement table, the most rot-prone content the audit exists for. - Both maps fold into `strictObject`'s `guidance` channel. #6619's stated blocker (the template appends `history` unconditionally; these surfaces emitted no trailing sentence) was a gap in the TEXT, not a limit of the template — the slot encodes position, and both surfaces had a real history nobody had written down. - Registry visibility 291 -> 293 surfaces (129 -> 131 carrying guidance); added exactly `the tool definition` and `` `enable` ``, removed none. - alias-integrity gains a CLASS pin: no module may hand a zod shape a hand-written map that decides `unrecognized_keys`. Two conjuncts (attached AND deciding that code), no allowlist — `uniqueScopeError` (invalid_union) and `objectStackErrorMap` (per-parse, never attached) are out of class by measurement, each pinned as a live control. - Acceptance byte-for-byte unchanged: 46-case probe matrix identical before and after (parse output + issue code/path). Message assembly moves per the #6804 precedent; 8 keys gain the rename channel. - strictness-ledger fixture moves to PerOperationRequiredPermissionsSchema, per that test's own instruction, with a control on the two vacated sites. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018ffcE95NaMJcL9XJ9VDYgk --- .../fold-residual-unrecognized-key-maps.md | 18 ++ .../spec/scripts/strictness-ledger.test.ts | 39 ++- packages/spec/src/ai/tool.test.ts | 75 ++++++ packages/spec/src/ai/tool.zod.ts | 52 ++-- packages/spec/src/data/field.zod.ts | 24 +- packages/spec/src/data/object.test.ts | 50 ++++ packages/spec/src/data/object.zod.ts | 90 +++++-- .../spec/src/shared/alias-integrity.test.ts | 248 ++++++++++++++++++ 8 files changed, 533 insertions(+), 63 deletions(-) create mode 100644 .changeset/fold-residual-unrecognized-key-maps.md diff --git a/.changeset/fold-residual-unrecognized-key-maps.md b/.changeset/fold-residual-unrecognized-key-maps.md new file mode 100644 index 0000000000..8dc76987ec --- /dev/null +++ b/.changeset/fold-residual-unrecognized-key-maps.md @@ -0,0 +1,18 @@ +--- +"@objectstack/spec": patch +--- + +refactor(spec): 折叠 #6619 漏掉的两个手写 `unrecognized_keys` 错误映射，并把闭合钉从实例拓宽为类（#6805） + +#6416 命名的盲区由 #6619/PR #6804 折掉三个手写 `$ZodErrorMap` 后宣告闭合，但那份清单**少了两个**——同一形态（`unrecognized_keys` 处方表经 `{ error: … }` 挂在 `.strict()` 对象上），同样不在任何注册表视野内： + +| 手写映射 | 位置（在 `origin/main` @ `2672f855f` 实测） | 折叠后 | +|---|---|---| +| `strictToolError` | `ai/tool.zod.ts:83`（消费于 `:180`） | `strictObject` + `guidance: TOOL_RETIRED_KEY_GUIDANCE` | +| `strictCapabilitiesError` | `data/object.zod.ts:169`（消费于 `:274`） | `strictObject` + `guidance: CAPABILITIES_RETIRED_KEY_GUIDANCE` | + +- **两张表首次进入 `alias-integrity.test.ts` 的审计视野**（注册表可见面 291 → 293，带 guidance 的面 129 → 131，新增恰为 `` `enable` `` 与 `the tool definition`，无一移除）。`TOOL_RETIRED_KEY_GUIDANCE` 是一张手工维护的**按键退役处方表**——正是这道门为之存在的最易腐烂的内容。 +- **#6619 记录的「折不了的理由」被证伪，而不是被绕过**：模板无条件追加 `history`，而这两张表不发解释句。那是**文案的缺口，不是模板的极限**——`history` 槽位编码的是位置（两条修复通道之后），两个面都有真实历史，只是从没写下来。写下来即可折。 +- **闭合钉从实例拓宽为类**：`alias-integrity.test.ts` 新增一条按 AST 判定的钉子——包内任何模块把自己写的、分支在 `unrecognized_keys` 上的错误映射交给 `z.(…, { error })`，即红。判据是「已挂载」∧「决断 `unrecognized_keys`」两个合取项，**无豁免名单**：`data/field.zod.ts` 的 `uniqueScopeError`（`invalid_union`，值级）与 `shared/error-map.zod.ts` 的 `objectStackErrorMap`（按次解析的全局兜底，从不挂到 shape 上）都由仪器本身判为不在类，各自作为实测对照钉住。 +- **接受面逐字节不变**：46 例探针矩阵在折叠前后逐例比对，parse 输出与 issue `code`+`path` 完全一致。消息装配按 #6804 的既有三类变化移动——处方文本逐字节保留，无处方的键改由模板的编辑距离通道作答（`labl`→`label`、`searchible`→`searchable` 等 8 处，此前只被告知「不是本 schema 的字段」），解释句在最后。 +- `scripts/strictness-ledger.test.ts` 的 `z.object(…).strict()` 夹具按其自身注释搬到 `PerOperationRequiredPermissionsSchema`（同文件），并新增「被腾空的两处现读为 helper」的对照。 diff --git a/packages/spec/scripts/strictness-ledger.test.ts b/packages/spec/scripts/strictness-ledger.test.ts index b1e1f0ca31..f3fe222a18 100644 --- a/packages/spec/scripts/strictness-ledger.test.ts +++ b/packages/spec/scripts/strictness-ledger.test.ts @@ -169,19 +169,34 @@ describe('posture reading, with a red control for each', () => { // wiring; #5593 migrated all four to `strictObject`, so the file no longer // exercised the branch under test, and the fixture moved to // `TenancyConfigSchema` — until #6619 folded ITS hand-written map into the - // shared template (the set-keyed `guidance` form gave the template the - // vocabulary #6416 had recorded as out of reach) and the site became - // `strictObject` like the rest. `ObjectCapabilities`, same file, is the - // spelling's remaining deliberate carrier: its map - // (`strictCapabilitiesError`) emits NO trailing history sentence, which - // the shared template still cannot express. If it is ever converted, move - // this fixture rather than deleting the assertion — the AST reader still - // has to make the reading, and `packages/spec` is not the only tree it - // reads. + // shared template. It then moved to `ObjectCapabilities`, same file, on the + // reading that its map (`strictCapabilitiesError`) emitted NO trailing + // history sentence and so could not fold. **#6805 disproved that reading**: + // the missing sentence was a gap in the TEXT, not a limit of the template + // (`history` encodes position, and `enable` had a real history nobody had + // written down), so that site is `strictObject` too now. + // + // `PerOperationRequiredPermissionsSchema`, still the same file, is the + // spelling's carrier today — and a more durable one, because it carries no + // guidance table at all and therefore nothing pulls it toward the helper. + // If it is ever converted, move this fixture AGAIN rather than deleting the + // assertion: the AST reader still has to make the reading, and + // `packages/spec` is not the only tree it reads. const objectSites = analyzeSites(at('data/object.zod.ts')); - const capabilities = objectSites.find((s) => s.name === 'ObjectCapabilities'); - expect(capabilities?.posture, 'a plain `.strict()` chain is still strict').toBe('strict'); - expect(capabilities?.idiom).toBe('z.object'); + const perOperation = objectSites.find((s) => s.name === 'PerOperationRequiredPermissionsSchema'); + expect(perOperation?.posture, 'a plain `.strict()` chain is still strict').toBe('strict'); + expect(perOperation?.idiom).toBe('z.object'); + + // …and the two sites the fixture vacated read as the helper now, which is + // the control that keeps the line above a statement about the READER + // rather than about one lucky survivor. Without it, a reader that simply + // stopped distinguishing idioms would still satisfy the assertion. + for (const name of ['ObjectCapabilities', 'TenancyConfigSchema']) { + const folded = objectSites.find((s) => s.name === name); + expect(folded, `${name} is not a site any more — re-point this test, do not delete it`).toBeDefined(); + expect(folded?.idiom, `${name} folded into the helper at #6619/#6805`).toBe('strictObject'); + expect(folded?.posture).toBe('strict'); + } // The permission file's four are now the helper, and still strict — the // control that keeps this test a statement about the READER rather than diff --git a/packages/spec/src/ai/tool.test.ts b/packages/spec/src/ai/tool.test.ts index bc50ab3e0a..f56c186486 100644 --- a/packages/spec/src/ai/tool.test.ts +++ b/packages/spec/src/ai/tool.test.ts @@ -207,4 +207,79 @@ describe('defineTool', () => { } expect(message).toMatch(/skills?\/agents|skill\/agent/i); }); + + // ── #6805 — the map folded into the shared `strictObject` template ──────── + // `strictToolError` was a hand-written `$ZodErrorMap`, so no registry saw + // `TOOL_RETIRED_KEY_GUIDANCE` and nothing judged it (#6416's blind spot, + // which #6619's inventory recorded as closed while this one survived). The + // acceptance surface did not move — every case above is unchanged and still + // passes — so what these pin is the assembly: the two channels the template + // brings that the hand-rolled map did not have. + + const rejectionMessage = (extra: Record): string => { + const result = ToolSchema.safeParse({ + name: 't', label: 'T', description: 'd', parameters: {}, ...extra, + }); + expect(result.success).toBe(false); + return result.success ? '' : result.error.issues.map((i) => i.message).join('\n'); + }; + + it.each([ + ['labl', 'label'], + ['paramaters', 'parameters'], + ['objectname', 'objectName'], + ['outputSchem', 'outputSchema'], + ])('a near-miss `%s` now gets the rename channel, not just "not a ToolSchema field"', (written, canonical) => { + // The hand-written map's fallback line named the problem and never the + // fix: every unprescribed key got "`x` is not a ToolSchema field." and + // stopped there. The template's edit-distance channel answers instead. + // This is the one reader-visible gain of the fold, so it is pinned rather + // than left as a claim in the PR body. + const message = rejectionMessage({ [written]: 'x' }); + expect(message).toContain(`\`${written}\` → \`${canonical}\``); + expect(message).not.toContain('is not a ToolSchema field'); + }); + + it('a key beyond edit distance is still named, with no misleading suggestion', () => { + // The other half of the same contract: the rename channel must stay quiet + // when it has nothing true to say. Suggesting SOMETHING for `notAToolField` + // would be ledger finding 7 — steering an author at a key they did not + // want — which is worse than the removed fallback line, not better. + const message = rejectionMessage({ notAToolField: 1 }); + expect(message).toContain('`notAToolField`'); + expect(message).not.toContain('Did you mean'); + }); + + it('emission order: which key is wrong → the fix → the history, last (#5955)', () => { + // The template's ordering contract, asserted on this surface because the + // fold is what brings this surface under it. `history` sat in the middle + // until #5955 and pushed the fix past ~character 220 on the single-line + // renderers several consumers use. + const message = rejectionMessage({ requiresConfirmation: true, labl: 'x' }); + const preamble = 'Unrecognized key(s) on the tool definition:'; + const fix = 'action.ai.requiresConfirmation'; + const history = 'the #1535 silent-strip class'; + + expect(message.startsWith(preamble)).toBe(true); + expect(message.indexOf(fix)).toBeGreaterThan(message.indexOf(preamble)); + expect(message.indexOf(history)).toBeGreaterThan(message.indexOf(fix)); + expect(message.trimEnd().endsWith(`${history}).`)).toBe(true); + // One history sentence per message, however many keys were written. + expect(message.split(history)).toHaveLength(2); + }); + + it('the prescriptions survive the fold byte-for-byte — the fold moved assembly, not text', () => { + // The whole value of `TOOL_RETIRED_KEY_GUIDANCE` is the text. A fold that + // quietly reworded a retirement prescription would be the defect this + // table exists to prevent, wearing the fold as cover. + expect(rejectionMessage({ permissions: ['x'] })).toContain( + '`tool.permissions` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — it ' + + 'promised a capability gate on tool invocation that nothing ever enforced', + ); + expect(rejectionMessage({ builtIn: true })).toContain( + '`tool.builtIn` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — no ' + + 'runtime branches on it; it never affected registration, selection or execution. Delete ' + + 'the key.', + ); + }); }); diff --git a/packages/spec/src/ai/tool.zod.ts b/packages/spec/src/ai/tool.zod.ts index 0888ef1d73..d3bca491f8 100644 --- a/packages/spec/src/ai/tool.zod.ts +++ b/packages/spec/src/ai/tool.zod.ts @@ -9,6 +9,7 @@ import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; // ========================================== import { lazySchema } from '../shared/lazy-schema'; +import { strictObject } from '../shared/strict-object'; /* * REMOVED — `ToolCategorySchema` / `ToolCategory` (#3896 audit close-out). @@ -71,26 +72,33 @@ const TOOL_RETIRED_KEY_GUIDANCE: Record = { }; /** - * Custom zod `error` for the `.strict()` ToolSchema. + * The standing history sentence for the tool surface, emitted LAST on every + * rejection — the shared template's `history` slot. * - * `.strict()` matters more than usual here. Removing a key from a NON-strict - * schema replaces one silent no-op with another: the author keeps writing - * `requiresConfirmation: true`, zod strips it without a word, and the safety - * flag goes on meaning nothing — the exact "silent strip" ADR-0032 / #1535 - * closed for objects. Rejecting loudly, with the prescription attached, is what - * turns the removal into a fix instead of a rename of the problem. + * `.strict()` matters more than usual here, and this sentence is why. Removing + * a key from a NON-strict schema replaces one silent no-op with another: the + * author keeps writing `requiresConfirmation: true`, zod strips it without a + * word, and the safety flag goes on meaning nothing — the exact "silent strip" + * ADR-0032 / #1535 closed for objects. Rejecting loudly, with the prescription + * attached, is what turns the removal into a fix instead of a rename of the + * problem. + * + * ## Why the slot, and why it was empty before (#6805) + * + * Until #6805 this file carried a hand-written `$ZodErrorMap` instead of the + * shared template, and #6416/#6619 recorded the reason it could not fold: the + * template appends `history` unconditionally (`${message} ${history}`) and this + * surface emitted no trailing sentence at all. That is a statement about the + * TEXT, not about the template — the surface has a real history, it simply had + * never been written down. Writing it is what makes the fold possible, and the + * fold is what puts `TOOL_RETIRED_KEY_GUIDANCE` — a hand-maintained per-key + * retirement table, the most rot-prone content the audit exists for — under + * `alias-integrity.test.ts` for the first time. */ -const strictToolError: z.core.$ZodErrorMap = (issue) => { - if (issue.code !== 'unrecognized_keys') return undefined; - const keys = (issue as { keys?: readonly string[] }).keys ?? []; - const lines = keys.map((key) => - TOOL_RETIRED_KEY_GUIDANCE[key] ?? `\`${key}\` is not a ToolSchema field.`, - ); - return ( - `Unrecognized key(s) on the tool definition: ${keys.map((k) => `\`${k}\``).join(', ')}.\n` + - lines.map((l) => ` • ${l}`).join('\n') - ); -}; +const TOOL_STRICT_HISTORY = + 'Until this shape was closed an undeclared key was dropped without a word — the tool ' + + 'still registered and still reached the LLM tool set, minus whatever the key was meant ' + + 'to do (the #1535 silent-strip class).'; /** * Tool Schema @@ -121,7 +129,11 @@ const strictToolError: z.core.$ZodErrorMap = (issue) => { * }); * ``` */ -export const ToolSchema = lazySchema(() => z.object({ +export const ToolSchema = lazySchema(() => strictObject({ + surface: 'the tool definition', + history: TOOL_STRICT_HISTORY, + guidance: TOOL_RETIRED_KEY_GUIDANCE, +}, { /** Machine name (snake_case, globally unique) */ name: z.string().regex(/^[a-z_][a-z0-9_]*$/).describe('Tool unique identifier (snake_case)'), @@ -177,7 +189,7 @@ export const ToolSchema = lazySchema(() => z.object({ // ADR-0010 — runtime protection envelope (internal — set by loader). ...MetadataProtectionFields, -}, { error: strictToolError }).strict().describe('AI tool definition. [READ-ONLY PROJECTION — not an execution entry point] Authoring a tool as metadata does NOT make it runnable: this schema has no `implementation`/`handler` field and no framework executor loads a metadata-authored tool. The runtime executes a separately-registered `AIToolDefinition` (cloud `@objectstack/service-ai`); tool metadata is a one-way projection for Studio/discovery. Do not expect a hand-authored tool to run in the open edition (liveness audit #1878/#1892).')); +}).describe('AI tool definition. [READ-ONLY PROJECTION — not an execution entry point] Authoring a tool as metadata does NOT make it runnable: this schema has no `implementation`/`handler` field and no framework executor loads a metadata-authored tool. The runtime executes a separately-registered `AIToolDefinition` (cloud `@objectstack/service-ai`); tool metadata is a one-way projection for Studio/discovery. Do not expect a hand-authored tool to run in the open edition (liveness audit #1878/#1892).')); export type Tool = z.input; diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index ebc6d609c7..03836f4491 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -262,11 +262,25 @@ export const AddressSchema = lazySchema(() => z.object({ */ /** * Prescriptive rejection for a mis-spelled `unique` scope (ADR-0120 - * §Terminology; pattern of `strictCapabilitiesError`): the error must carry the - * vocabulary and, for the two predictable near-misses (`'tenant'`, `'org'`), - * name `'organization'` explicitly — a typo must be a loud, fixable parse - * error, never a silent scope change. Declared before `UniqueScopeSchema` - * because `OS_EAGER_SCHEMAS=1` evaluates the factory at module load (TDZ). + * §Terminology): the error must carry the vocabulary and, for the two + * predictable near-misses (`'tenant'`, `'org'`), name `'organization'` + * explicitly — a typo must be a loud, fixable parse error, never a silent + * scope change. Declared before `UniqueScopeSchema` because + * `OS_EAGER_SCHEMAS=1` evaluates the factory at module load (TDZ). + * + * ⚠️ **The last hand-written `$ZodErrorMap` in `packages/spec`, and it stays + * one.** This docblock used to say "pattern of `strictCapabilitiesError`"; + * #6805 folded that sibling into the shared `strictObject` template and the + * pointer would have gone stale, so it is replaced by the reason this map is + * NOT following it. The fold's channel is `unrecognized_keys` — an unknown + * KEY, answered from a per-key `guidance` table. This map answers + * `invalid_union`, a VALUE-level verdict on a key the schema declares, which + * `strictObject` does not address at any level. Folding it would be a category + * error, and `alias-integrity.test.ts`'s class pin + * (`NO module outside the shared helpers writes its own unrecognized_keys + * map`) is scoped by `issue.code` precisely so this site is out of class by + * measurement rather than by an exemption — that pin reads this file as a live + * control. */ const uniqueScopeError: z.core.$ZodErrorMap = (issue) => { if (issue.code !== 'invalid_union') return undefined; diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index 1ea16a57f6..b434555884 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -66,6 +66,56 @@ describe('ObjectCapabilities', () => { const result = ObjectCapabilities.safeParse({ feedEnabled: true }); expect(result.success).toBe(false); }); + + // ── #6805 — the map folded into the shared `strictObject` template ──────── + // `strictCapabilitiesError` was a hand-written `$ZodErrorMap`, so + // `CAPABILITIES_RETIRED_KEY_GUIDANCE` registered in no registry and nothing + // judged it. Acceptance did not move (every case above is unchanged); these + // pin the assembly the template brings. + + const capabilityRejection = (body: Record): string => { + const result = ObjectCapabilities.safeParse(body); + expect(result.success).toBe(false); + return result.success ? '' : result.error.issues.map((i) => i.message).join('\n'); + }; + + it.each([ + ['searchible', 'searchable'], + ['trackHistroy', 'trackHistory'], + ['clon', 'clone'], + ['feed', 'feeds'], + ])('a near-miss `%s` now gets the rename channel, not just "not an `enable` capability flag"', (written, canonical) => { + const message = capabilityRejection({ [written]: true }); + expect(message).toContain(`\`${written}\` → \`${canonical}\``); + expect(message).not.toContain('is not an `enable` capability flag'); + }); + + it('a key beyond edit distance is still named, with no misleading suggestion', () => { + const message = capabilityRejection({ feedEnabled: true }); + expect(message).toContain('`feedEnabled`'); + expect(message).not.toContain('Did you mean'); + }); + + it('emission order: which key is wrong → the fix → the history, last (#5955)', () => { + const message = capabilityRejection({ trash: false, searchible: true }); + const preamble = 'Unrecognized key(s) on `enable`:'; + const fix = 'os migrate meta --from 16'; + const history = 'every flag carries an enforcement contract (#2707)'; + + expect(message.startsWith(preamble)).toBe(true); + expect(message.indexOf(fix)).toBeGreaterThan(message.indexOf(preamble)); + expect(message.indexOf(history)).toBeGreaterThan(message.indexOf(fix)); + expect(message.trimEnd().endsWith(`${history}.`)).toBe(true); + expect(message.split(history)).toHaveLength(2); + }); + + it('the tombstone text survives the fold byte-for-byte', () => { + expect(capabilityRejection({ mru: true })).toContain( + '`enable.mru` was removed from @objectstack/spec in the 16.x line (#2377/#3207, ' + + 'ADR-0049) — Most-Recently-Used tracking was never implemented; no reader ' + + 'existed, so the flag changed nothing.', + ); + }); }); describe('LifecycleSchema (ADR-0057)', () => { diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index d80405c3d5..abe39c9bc9 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -151,32 +151,44 @@ const CAPABILITIES_RETIRED_KEY_GUIDANCE: Record = { }; /** - * Custom zod `error` for the `.strict()` capabilities block: an unknown key — - * a retired `trash`/`mru` or a typo like `feedEnabled` — is a loud, *fixable* - * parse error instead of a silent strip (#1535), and a retired key's error - * carries its upgrade prescription. Every other issue code defers to zod's - * default. + * The standing history sentence for the `enable` block, emitted LAST on every + * rejection — the shared template's `history` slot. * - * The LAST hand-written `unrecognized_keys` map in this file — #6619 folded - * its sibling `strictTenancyError` into the shared `strictObject` template, - * and this one stayed out for a reason the template can measure: it emits NO - * trailing history sentence, and `strictUnknownKeyError` appends its `history` - * unconditionally. Fold it only when the template can express a - * history-less surface; `scripts/strictness-ledger.test.ts` uses the - * `ObjectCapabilities` site below as its `z.object(…).strict()` fixture, so - * move that fixture in the same change. + * ## Why the slot, and why the fold waited for it (#6805) + * + * This block was the LAST hand-written `unrecognized_keys` map in this file. + * #6619 folded its sibling `strictTenancyError` into the shared `strictObject` + * template and left this one out for a reason it stated precisely: the map + * emitted NO trailing sentence while `strictUnknownKeyError` appends its + * `history` unconditionally (`${message} ${history}`). Measured again at #6805, + * that is a statement about the TEXT and not about the template — `enable` has + * a real history, it had simply never been written down. Writing it is the + * whole of what the fold needed. + * + * The reading of the slot is #6619's, unchanged: what it encodes is *position* + * — the one standing sentence that follows both fix channels (#5955 / #6416) — + * so the surface decides what belongs there, and background is as legitimate as + * literal history. + * + * The rejection itself is unchanged in kind: an unknown key — a retired + * `trash`/`mru` or a typo like `feedEnabled` — is a loud, *fixable* parse error + * instead of a silent strip (#1535), a retired key's error carries its upgrade + * prescription, and every other issue code defers to zod's default. What the + * fold changes is the *other* key: `searchible` now resolves to `searchable` + * through the template's rename channel instead of being told only that it "is + * not an `enable` capability flag", which named the problem and never the fix. + * And it is what puts this table under `alias-integrity.test.ts`, which no + * hand-rolled map has ever been judged by. + * + * ⚠️ `scripts/strictness-ledger.test.ts` used the `ObjectCapabilities` site + * below as its `z.object(…).strict()` fixture. #6805 moved it to + * `PerOperationRequiredPermissionsSchema` in this same file rather than + * deleting the assertion, exactly as that test's own note instructs. */ -const strictCapabilitiesError: z.core.$ZodErrorMap = (issue) => { - if (issue.code !== 'unrecognized_keys') return undefined; - const keys = (issue as { keys?: readonly string[] }).keys ?? []; - const lines = keys.map((key) => - CAPABILITIES_RETIRED_KEY_GUIDANCE[key] ?? `\`${key}\` is not an \`enable\` capability flag.`, - ); - return ( - `Unrecognized key(s) on \`enable\`: ${keys.map((k) => `\`${k}\``).join(', ')}.\n` + - lines.map((l) => ` • ${l}`).join('\n') - ); -}; +const CAPABILITIES_HISTORY = + 'Until this shape was closed an unknown flag was dropped without a word — the object ' + + 'shipped as if the author had never written it (#1535); `enable` is a closed vocabulary ' + + 'in which every flag carries an enforcement contract (#2707).'; /** * Capability Flags @@ -195,6 +207,20 @@ const strictCapabilitiesError: z.core.$ZodErrorMap = (issue) => { * `.strict()`: unknown keys (incl. the retired `trash` / `mru`, #2377) are * rejected with guidance, not stripped (#1535). * + * Closed with the shared `strictObject` template since #6805 — see + * {@link CAPABILITIES_HISTORY} for why the fold waited on one sentence, and + * what changes (and does not) about the message. + * + * ⚠️ ORDER IS LOAD-BEARING here for the same reason it is at + * `ObjectSchemaBase` (#5593, ~1000 lines below): `strictObject` evaluates its + * options object at CONSTRUCTION — that is what lets `alias-integrity.test.ts` + * judge the table against the real `.shape` — so both + * `CAPABILITIES_RETIRED_KEY_GUIDANCE` and `CAPABILITIES_HISTORY` must be + * declared ABOVE this site. Moving either below it reintroduces the temporal + * dead zone as a module-init crash under `OS_EAGER_SCHEMAS=1` (how + * `build-schemas.ts` runs), which the test suite does not reach because tests + * import lazily. + * * @example * { * trackHistory: true, @@ -203,7 +229,11 @@ const strictCapabilitiesError: z.core.$ZodErrorMap = (issue) => { * activities: false * } */ -export const ObjectCapabilities = z.object({ +export const ObjectCapabilities = strictObject({ + surface: '`enable`', + history: CAPABILITIES_HISTORY, + guidance: CAPABILITIES_RETIRED_KEY_GUIDANCE, +}, { /** * History tracking (Audit Trail) master switch — opt-in. * @@ -271,7 +301,7 @@ export const ObjectCapabilities = z.object({ /** Allow cloning records */ clone: z.boolean().default(true).describe('Allow record deep cloning'), -}, { error: strictCapabilitiesError }).strict(); +}); /** * Schema for database indexes. @@ -573,6 +603,14 @@ export const ObjectAccessConfigSchema = lazySchema(() => strictObject({ * Operation→class mapping mirrors the CRUD permission bits: `transfer`/`restore` * fold into `update`, `purge` into `delete`. `.strict()` so a mistyped key * (e.g. `reads`) is rejected at author time rather than silently ignored. + * + * ⚠️ This site is `scripts/strictness-ledger.test.ts`'s fixture for the OLDER + * `z.object(…).strict()` spelling — the reading the ledger's AST walker has to + * keep making, and `packages/spec` is not the only tree it reads. The fixture + * has moved twice as the campaign converted its predecessors + * (`security/permission.zod.ts` → `TenancyConfigSchema` at #5593 → + * `ObjectCapabilities` at #6619 → here at #6805). If THIS one is ever + * converted, move the fixture again rather than deleting the assertion. */ export const PerOperationRequiredPermissionsSchema = z.object({ read: z.array(z.string()).optional().describe('Capabilities required to read (find/findOne/count/aggregate).'), diff --git a/packages/spec/src/shared/alias-integrity.test.ts b/packages/spec/src/shared/alias-integrity.test.ts index 3e6b5827bb..20065700d5 100644 --- a/packages/spec/src/shared/alias-integrity.test.ts +++ b/packages/spec/src/shared/alias-integrity.test.ts @@ -371,6 +371,110 @@ function callSites(file: string, callee: keyof typeof CALLEES): CallSite[] { const CALL_SITES = MODULES.flatMap((f) => callSites(f, 'strictObject')); +/** + * Sites where a module hands a zod shape an `{ error: … }` of its own that + * decides **`unrecognized_keys`** — the structural signature of a hand-written + * unknown-key error map (#6805). + * + * The criterion is the class's own definition, in two conjuncts, and BOTH are + * load-bearing: + * + * 1. **Attached.** The map is passed as `error` in the params object of a + * `z.(…)` call — the construction-time wiring that makes it *this + * shape's* unknown-key voice. A map that is never attached to a shape has no + * alias/guidance table for a registry to judge, so it is not what #6416 + * named. Two live specimens prove the conjunct is doing work rather than + * decorating the sentence: `shared/error-map.zod.ts`'s `objectStackErrorMap` + * *does* decide `unrecognized_keys`, but it is a per-parse map a CALLER + * passes to `safeParse` (a generic "check for typos" fallback carrying no + * per-key content), and `carriesUnknownKey` in the same file only *reads* + * `issue.code` to rank union branches. Neither is a per-schema table, and + * the first draft of this scan flagged both. + * 2. **Deciding `unrecognized_keys`.** Judged by the code the map branches on, + * never by its name. `data/field.zod.ts`'s `uniqueScopeError` is attached + * exactly this way — `z.union([…], { error: uniqueScopeError })` — and is + * NOT in the class, because it answers `invalid_union`, a value-level + * verdict `strictObject`'s guidance channel does not address. + * + * AST rather than text throughout, which is the other load-bearing choice: the + * literal `'unrecognized_keys'` appears in PROSE all over this package + * (including in the pins that call this function), and a comment is not a node + * this walk visits. + */ +interface HandwrittenMapSite { readonly line: number; readonly name: string } + +function scanSource(file: string, text: string): HandwrittenMapSite[] { + const source = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true); + + /** True when `node`'s subtree compares something to `'unrecognized_keys'`. */ + const decidesUnknownKeys = (node: ts.Node): boolean => { + let found = false; + const walk = (n: ts.Node): void => { + if (found) return; + if ( + (ts.isStringLiteral(n) || ts.isNoSubstitutionTemplateLiteral(n)) + && n.text === 'unrecognized_keys' + ) { found = true; return; } + ts.forEachChild(n, walk); + }; + walk(node); + return found; + }; + + // Module-scope `const x = …` / `function x() {}`, so an `{ error: x }` can be + // resolved back to the body it names. Same-module only: a map imported from + // elsewhere is judged where it is DECLARED, by this same scan over that file. + const declarations = new Map(); + const collect = (node: ts.Node): void => { + if (ts.isVariableDeclaration(node) && ts.isIdentifier(node.name) && node.initializer) { + declarations.set(node.name.text, node.initializer); + } else if (ts.isFunctionDeclaration(node) && node.name) { + declarations.set(node.name.text, node); + } + ts.forEachChild(node, collect); + }; + collect(source); + + const sites: HandwrittenMapSite[] = []; + const visit = (node: ts.Node): void => { + // `z.object(…)`, `z.union(…)`, `z.never(…)`, … — a zod FACTORY call, which + // is where a params object binds a map to a shape. Deliberately not any + // `{ error: … }` anywhere: `schema.safeParse(data, { error: map })` is a + // caller's choice for one parse, not a property of the shape. + if ( + ts.isCallExpression(node) + && ts.isPropertyAccessExpression(node.expression) + && ts.isIdentifier(node.expression.expression) + && node.expression.expression.text === 'z' + ) { + for (const arg of node.arguments) { + if (!ts.isObjectLiteralExpression(arg)) continue; + for (const p of arg.properties) { + if (!ts.isPropertyAssignment(p)) continue; + if (!ts.isIdentifier(p.name) || p.name.text !== 'error') continue; + const init = p.initializer; + // An identifier is resolved to its declaration; an inline function is + // its own body. A CALL (`strictObjectError(options, shape)`) is the + // shared template being invoked — the opposite of hand-written — and + // its body lives in a helper module this scan does not read. + const body = ts.isIdentifier(init) ? declarations.get(init.text) : init; + if (!body || !decidesUnknownKeys(body)) continue; + sites.push({ + line: source.getLineAndCharacterOfPosition(p.getStart()).line + 1, + name: ts.isIdentifier(init) ? init.text : '(inline)', + }); + } + } + } + ts.forEachChild(node, visit); + }; + visit(source); + return sites; +} + +const handwrittenMapSites = (file: string): HandwrittenMapSite[] => + scanSource(file, fs.readFileSync(file, 'utf8')); + // --------------------------------------------------------------------------- // 1. Coverage — the walk reached every table the source declares // --------------------------------------------------------------------------- @@ -755,6 +859,150 @@ describe('alias integrity — every table is a true claim about its schema', () expect(Object.keys(tenancy!.options.guidance ?? {}).sort()).toEqual(['crossTenantAccess', 'strategy']); }); + it('the two maps #6619 MISSED are folded and judged here too (#6805)', () => { + // #6619's inventory was two short, and both survivors were the same shape + // as the three above — `unrecognized_keys` prescription tables attached to + // a `.strict()` object through `{ error: … }`, seen by no registry. The + // one that mattered is `strictToolError`: it carried + // `TOOL_RETIRED_KEY_GUIDANCE`, a hand-maintained PER-KEY retirement table, + // which is the most rot-prone content this audit exists for (the #6756 / + // #6758 sweep found live prescriptions pointing at keys that no longer + // exist, in tables nothing was judging either). + // + // Recorded as its own case rather than folded into the one above, so the + // two closures stay separately readable: #6619 closed the instances #6416 + // NAMED, #6805 closed the two it missed, and the class pin below closes the + // shape so there is no third round. + const bySurface = new Map(SURFACES.map((s) => [s.options.surface, s])); + + const tool = bySurface.get('the tool definition'); + expect(tool, 'ToolSchema no longer declares through strictObject').toBeDefined(); + expect(Object.keys(tool!.options.guidance ?? {}).sort()).toEqual([ + 'active', 'builtIn', 'category', 'permissions', 'requiresConfirmation', + ]); + + const capabilities = bySurface.get('`enable`'); + expect(capabilities, 'ObjectCapabilities no longer declares through strictObject').toBeDefined(); + expect(Object.keys(capabilities!.options.guidance ?? {}).sort()).toEqual(['mru', 'trash']); + }); + + it('NO module outside the shared helpers writes its own `unrecognized_keys` map (#6805)', () => { + // The class, not the instances. Both closure pins above name surfaces, so + // each only holds the line it was written for — #6416 named three, and the + // inventory that produced the number was two short. A pin over the SHAPE + // cannot be two short: any new hand-rolled unknown-key map fails here on + // arrival, whatever it is called and whatever surface it closes. + // + // ⚠️ Scoped to `unrecognized_keys` DELIBERATELY, and this is the whole + // discrimination. `data/field.zod.ts`'s `uniqueScopeError` is a + // `$ZodErrorMap` too and is NOT in this class: it branches on + // `invalid_union`, a VALUE-level verdict, which `strictObject`'s guidance + // channel does not address and could not absorb. A blanket "no error maps + // outside the helpers" rule would have to carry an exemption for it, and an + // exemption is exactly the seam an inventory drifts through. Judged by the + // `issue.code` the map decides on, never by its name. + // + // Adjacent to #6635 (a general "a retirement updated some mentions and not + // others" gate) and deliberately not it: that one compares PROSE mentions + // of a retired symbol against each other and catches a table whose + // prescriptions have gone stale; this one is structural and catches a + // guidance channel that never entered a registry. Neither subsumes the + // other — after this fold, `TOOL_RETIRED_KEY_GUIDANCE` is visible to the + // audit and could still name a key that no longer exists, which is #6635's + // to find and this pin is silent about. + // ⚠️ Scanned over EVERY module, `HELPER_MODULES` included — deliberately no + // exemption. The direct-call ratchet above needs one because + // `strictUnknownKeyError`'s own definition is a call site of itself; this + // criterion needs none, because the helper does not hand a shape a + // hand-written map either: `strict-object.ts`'s one + // `z.object(shape, { error: … })` passes a CALL to the shared factory, and + // a call is exactly what "not hand-written" looks like. Measured, not + // assumed — an exemption that excuses nothing reads as coverage for a case + // nobody checked, which is the failure mode of the inventory this pin + // replaces. + const offenders = MODULES + .flatMap((f) => handwrittenMapSites(f) + .map((s) => `${path.relative(SPEC_SRC, f)}:${s.line} — \`${s.name}\``)); + + expect( + offenders.sort(), + 'build the shape with `strictObject(options, shape)` and put the prescriptions in ' + + '`guidance` / `guidanceSets` — a hand-rolled map registers in no registry, so its ' + + 'aliases and prescriptions are unmeasured rather than clean (#6416/#6619/#6805)', + ).toEqual([]); + }); + + it('…and that scan is alive: the pre-fold shape is found, prose and the two out-of-class maps are not (#6805)', () => { + // Anti-vacuity for the verdict above, which asserts that a search came back + // EMPTY — the shape that passes just as well when the instrument is dead. + // Four controls, each closing a different way it could be. + // + // Note what is NOT a control here: "the helper modules light up". Under an + // earlier, coarser criterion (the bare `'unrecognized_keys'` literal) they + // did, and that reading was the reason the first draft also flagged + // `objectStackErrorMap`. The tightened criterion asks whether a shape was + // handed a hand-written map, which the helper never does — so the honest + // liveness evidence is (a) below, a fresh file the scan has never seen. + // + // (a) it recognises the class in a file it has never seen. The specimen is + // `strictToolError` and its wiring as they stood on `main` immediately + // before this PR folded them, so what is pinned is the real thing + // rather than a stylised stand-in — this is the exact source the + // verdict must never let back in. + const specimen = ` + const strictToolError: z.core.$ZodErrorMap = (issue) => { + if (issue.code !== 'unrecognized_keys') return undefined; + const keys = (issue as { keys?: readonly string[] }).keys ?? []; + return \`Unrecognized key(s): \${keys.join(', ')}.\`; + }; + export const ToolSchema = z.object({ + name: z.string(), + }, { error: strictToolError }).strict(); + `; + expect(scanSource('specimen.ts', specimen)).toEqual([{ line: 9, name: 'strictToolError' }]); + + // (b) it is not a grep. This package's docblocks discuss + // `unrecognized_keys` by name constantly — including the pin above — + // and prose is not a map. A text scan would flag every one of them, and + // the verdict would have needed an allowlist, which is the maintenance + // shape this pin exists to avoid. + const prose = ` + /** The alias table is consulted from the unrecognized_keys path only. */ + // A guidance entry answers an 'unrecognized_keys' issue. + export const NOT_A_MAP = 1; + `; + expect(scanSource('prose.ts', prose)).toEqual([]); + + // (c) the ATTACHMENT conjunct earns its place. `objectStackErrorMap` + // decides `unrecognized_keys` and is deliberately out of class: it is a + // per-parse map a caller passes to `safeParse`, with one generic "check + // for typos" sentence and no per-key content, so there is no table for + // a registry to judge. The first draft of this scan — literal presence + // alone — flagged it and its neighbour `carriesUnknownKey` (a reader, + // not a map). Recorded as a MEASURED negative rather than an exemption: + // if that file ever does attach a per-schema map, this control does not + // protect it. + const errorMap = path.join(SPEC_SRC, 'shared/error-map.zod.ts'); + const errorMapSource = fs.readFileSync(errorMap, 'utf8'); + expect(errorMapSource, 'objectStackErrorMap no longer decides unknown keys — re-point this control') + .toContain("issue.code === 'unrecognized_keys'"); + expect(handwrittenMapSites(errorMap)).toEqual([]); + + // (d) the CODE conjunct earns its place, on the live specimen #6805's card + // named as out of class. `uniqueScopeError` is attached exactly the way + // the class is — `z.union([…], { error: uniqueScopeError })` — so the + // scan reaches it and declines it on `issue.code` alone. That is the + // discrimination made by the instrument rather than by an exemption, + // which is what keeps "do not sweep it in" from decaying into a name. + const field = path.join(SPEC_SRC, 'data/field.zod.ts'); + const fieldSource = fs.readFileSync(field, 'utf8'); + expect(fieldSource, 'uniqueScopeError is no longer attached — re-point this control') + .toContain('error: uniqueScopeError,'); + expect(fieldSource, 'uniqueScopeError no longer branches on invalid_union — re-read the class') + .toContain("issue.code !== 'invalid_union'"); + expect(handwrittenMapSites(field)).toEqual([]); + }); + it('no guidance key is itself a declared key (the same dead entry, other channel)', () => { // `guidance` is consulted from the same `unrecognized_keys` path, so a // prescription filed under a key the shape DECLARES is unreachable in