From 77c1f735d7e5aaaf8de23d727a7f0a477f0f7f4b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 18:16:14 +0000 Subject: [PATCH] feat(spec)!: mapping/agent/page close, and strictObject stops suggesting removed keys (#4001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The helper bug first, because this campaign put it there. `skill` closed last batch while still carrying `retiredKey` tombstones, and `strictObject` built its "did you mean" candidates from the whole shape — tombstones included. So a `triggerPhrase` typo was answered with "Did you mean `triggerPhrases`?", a key that had been REMOVED, and the author who complied got a second rejection telling them to delete what they had just been told to write. Third occurrence of a shape the ledger already records twice, and the first one in a SHARED helper, where it would have reached every conversion after it. Both helpers were correct alone; only the combination was wrong. Fixed structurally rather than by special-casing tombstones: never suggest a key the schema cannot accept. They stay complementary — `retiredKey` is stronger than `guidance`, since typing the key as `never` also fails `tsc` through a variable, where excess-property checking would not fire. `agent` had two security-shaped removals with no tombstone. `visibility` and `tenantId` were deleted as unenforced security properties — correctly — but without a prescription, because the shape was `.strip` and there was no rejection to hang one on. An author who wrote `visibility: 'private'` believed the agent was hidden; it was listed to everyone, and always had been. The `skill.permissions` class again. Both now name what actually gates an agent. `route` on a page was a fiction the platform's own suite carried. `stack.test.ts` authored `route: '/landing'` for years; `PageSchema` never declared it, and a page is routed by its `name` — which in the map format under test IS the map key, which the test asserted six lines below the key contradicting it. Fifth test found codifying a strip-era fiction, and the likeliest to be reinvented. `mapping` and `page` gain their ADR-0010 envelope. The undeclared-envelope debt list is down to two, from eight. The headline number is now derived instead of tallied — it had already drifted by one, in a campaign whose recurring finding is that hand-maintained coverage measurements go stale. `STILL_STRIP` carries a reverse pin like the envelope list; verified it goes red in both directions before being trusted. Registered types closed: 21 of 25. Still open: action, dashboard, field, view. The warning layer's covered roots drop 6 → 3, verified as a hand-off rather than a hole: `agent.zzz`, `page.zzz` and nested `page.regions[0].zzz` are each now rejected by the parse. A broken walk and a graduation shrink that count identically, so the check is pinned next to the number. Verified: 284 files / 7239 tests, tsc clean, 8 generated artifacts current, all spec gates green, and the CRM/Todo/showcase/platform-objects builds pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WnqGjQFQMqd5k81LYV8SCY --- .changeset/mapping-agent-page-strict.md | 25 ++++ content/docs/references/data/mapping.mdx | 19 ++- content/docs/references/ui/page.mdx | 7 + .../2026-07-unknown-key-strictness-ledger.md | 47 +++++++ packages/spec/authorable-surface.json | 14 ++ packages/spec/src/ai/agent.zod.ts | 132 +++++++++++++++++- packages/spec/src/ai/skill.test.ts | 18 +++ packages/spec/src/data/mapping.zod.ts | 95 +++++++++++-- .../kernel/metadata-authoring-lint.test.ts | 58 ++++++-- .../src/kernel/metadata-type-schemas.test.ts | 87 +++++++++++- .../spec/src/shared/strict-object.test.ts | 56 ++++++++ packages/spec/src/shared/strict-object.ts | 54 ++++++- packages/spec/src/stack.test.ts | 6 +- packages/spec/src/ui/page.zod.ts | 129 ++++++++++++++++- 14 files changed, 702 insertions(+), 45 deletions(-) create mode 100644 .changeset/mapping-agent-page-strict.md diff --git a/.changeset/mapping-agent-page-strict.md b/.changeset/mapping-agent-page-strict.md new file mode 100644 index 0000000000..47f180f779 --- /dev/null +++ b/.changeset/mapping-agent-page-strict.md @@ -0,0 +1,25 @@ +--- +'@objectstack/spec': minor +--- + +`mapping`, `agent` and `page` reject unknown keys — and `strictObject` stops suggesting keys that were removed. + +**A bug this campaign introduced into its own helper, fixed first.** `skill` closed in the last batch while still carrying `retiredKey` tombstones, and `strictObject` built its "did you mean" candidates from the whole shape — tombstones included. So a `triggerPhrase` typo was answered with *"Did you mean `triggerPhrases`?"*, a key that had been **removed**. An author who complied landed on the tombstone and got a second rejection telling them to delete what they had just been told to write. + +Third occurrence of a shape the ledger already records twice — this campaign's fix pointing the way into the failure it exists to kill — and the first one in a *shared* helper, where it would have reached every conversion after it. Fixed structurally: **never suggest a key the schema cannot accept.** Candidates that accept `never` are dropped, so the rule holds without knowing why a key is unwritable. The two helpers stay complementary; `retiredKey` is *stronger* than a `guidance` entry, because typing the key as `never` also fails `tsc` when the config arrives through a variable, where excess-property checking would not fire. + +**`agent` had two security-shaped removals with no tombstone.** `visibility` and `tenantId` were deleted as unenforced security properties — correctly, since neither did anything — but deleted without a prescription, because the shape was `.strip` and there was no rejection to attach one to. An author who wrote `visibility: 'private'` believed the agent was hidden. It was listed to everyone, and always had been. This is the `skill.permissions` class: a key that reads as a security control, is not one, and says nothing when you write it. Closing the shape created the channel, so both now name what actually gates an agent (`access` / `permissions`, enforced at the chat route since #1884). + +**`route` on a page was a fiction the platform's own test suite carried.** `stack.test.ts` authored `route: '/landing'` for years. `PageSchema` has never declared it — a page is routed by its `name`, which in the map format under test IS the map key, which the test asserted six lines below the key contradicting it. Fifth test found codifying a strip-era fiction as intent, and the most likely to be reinvented, since `route` is the first key anyone reaches for on a page. Tombstoned with `path` and `url`. + +Also tombstoned from each file's own comments, now that there is somewhere to put them: `agent.memory.shortTerm` (declared a working-memory window nothing consumed — ADR-0013 D3), `page.recordReview` and `page.blankLayout` (page types with no renderer, removed in framework#2265), and wrong-layer pointers for the page keys that read like real controls — `interfaceConfig.visualization` (the display mode is chosen from `appearance.allowedVisualizations`, and is not a page type), `guardrails.allowedTopics` (there is no allow-list, only `blockedTopics`). + +`mapping` and `page` also gain their ADR-0010 protection envelope, which their loaders stamp and their schemas could not hold. **The undeclared-envelope debt list is down to two** (`action`, `field`), from eight. + +Registered types closed at the top level: **21 of 25**. Still open: `action`, `dashboard`, `field`, `view`. + +That count is now derived and pinned rather than tallied by hand — it had already drifted by one, in a campaign whose recurring lesson is that hand-maintained measurements of coverage go stale. `metadata-type-schemas.test.ts` walks each registered schema for its top-level catchall and carries the open list as a reverse pin, so closing a type fails the test until the list shrinks, exactly like the envelope debt list next to it. + +The unknown-key warning layer's covered roots drop from 6 to 3 — verified as a hand-off rather than a hole: `agent.zzz`, `page.zzz` and the nested `page.regions[0].zzz` are each now rejected by the parse. A broken walk and a successful graduation shrink that count identically, so the check is pinned in the test alongside the number. + +Authoring impact: a key none of these shapes declares is now rejected instead of silently discarded — it was already being ignored, so no working metadata changes. diff --git a/content/docs/references/data/mapping.mdx b/content/docs/references/data/mapping.mdx index 3d91ef5025..54641f0ca8 100644 --- a/content/docs/references/data/mapping.mdx +++ b/content/docs/references/data/mapping.mdx @@ -5,9 +5,17 @@ description: Mapping protocol schemas {/* ⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate. Hand-written docs live in the module folders under content/docs/. */} -Transformation Logic +Shared history for this file (#4001). -Built-in helpers for converting data during import. +An import mapping is instructions for moving somebody's data. A dropped key + +does not fail the import — it runs, to completion, with a "success" the + +author reads as "the data arrived the way I described it". The rows land + +untransformed, or unmatched, or duplicated, and the diagnosis starts from a + +green run. **Source:** `packages/spec/src/data/mapping.zod.ts` @@ -55,6 +63,13 @@ const result = FieldMapping.parse(data); | **extractQuery** | `{ object: string; fields?: string[]; where?: any; search?: string \| { query: string; fields?: string[]; fuzzy: boolean; operator: Enum<'and' \| 'or'>; … }; … }` | optional | Query to run for export only | | **errorPolicy** | `Enum<'skip' \| 'abort' \| 'retry'>` | ✅ | | | **batchSize** | `number` | ✅ | | +| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | +| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | +| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). | +| **_provenance** | `Enum<'package' \| 'org' \| 'env-forced'>` | optional | Origin of the item (package \| org \| env-forced). | +| **_packageId** | `string` | optional | Owning package machine id. | +| **_packageVersion** | `string` | optional | Owning package version. | +| **_lockDocsUrl** | `string` | optional | Optional documentation link surfaced next to _lockReason. | --- diff --git a/content/docs/references/ui/page.mdx b/content/docs/references/ui/page.mdx index 7691003f41..c2fe4f57e4 100644 --- a/content/docs/references/ui/page.mdx +++ b/content/docs/references/ui/page.mdx @@ -89,6 +89,13 @@ Interface-level page configuration (Airtable parity) | **slots** | `{ header?: { type: Enum<'page:header' \| 'page:footer' \| 'page:sidebar' \| 'page:tabs' \| 'page:accordion' \| 'page:card' \| 'page:section' \| 'record:details' \| 'record:highlights' \| 'record:related_list' \| 'record:activity' \| 'record:chatter' \| 'record:path' \| 'record:alert' \| 'record:quick_actions' \| 'record:reference_rail' \| 'record:history' \| 'app:launcher' \| 'nav:menu' \| 'nav:breadcrumb' \| 'global:search' \| 'global:notifications' \| 'user:profile' \| 'ai:chat_window' \| 'ai:suggestion' \| 'element:text' \| 'element:number' \| 'element:image' \| 'element:divider' \| 'element:button' \| 'element:filter' \| 'element:form' \| 'element:record_picker' \| 'element:text_input'> \| string; id?: string; label?: string; properties?: Record; … } \| { type: Enum<'page:header' \| 'page:footer' \| 'page:sidebar' \| 'page:tabs' \| 'page:accordion' \| 'page:card' \| 'page:section' \| 'record:details' \| 'record:highlights' \| 'record:related_list' \| 'record:activity' \| 'record:chatter' \| 'record:path' \| 'record:alert' \| 'record:quick_actions' \| 'record:reference_rail' \| 'record:history' \| 'app:launcher' \| 'nav:menu' \| 'nav:breadcrumb' \| 'global:search' \| 'global:notifications' \| 'user:profile' \| 'ai:chat_window' \| 'ai:suggestion' \| 'element:text' \| 'element:number' \| 'element:image' \| 'element:divider' \| 'element:button' \| 'element:filter' \| 'element:form' \| 'element:record_picker' \| 'element:text_input'> \| string; id?: string; label?: string; properties?: Record; … }[]; actions?: { type: Enum<'page:header' \| 'page:footer' \| 'page:sidebar' \| 'page:tabs' \| 'page:accordion' \| 'page:card' \| 'page:section' \| 'record:details' \| 'record:highlights' \| 'record:related_list' \| 'record:activity' \| 'record:chatter' \| 'record:path' \| 'record:alert' \| 'record:quick_actions' \| 'record:reference_rail' \| 'record:history' \| 'app:launcher' \| 'nav:menu' \| 'nav:breadcrumb' \| 'global:search' \| 'global:notifications' \| 'user:profile' \| 'ai:chat_window' \| 'ai:suggestion' \| 'element:text' \| 'element:number' \| 'element:image' \| 'element:divider' \| 'element:button' \| 'element:filter' \| 'element:form' \| 'element:record_picker' \| 'element:text_input'> \| string; id?: string; label?: string; properties?: Record; … } \| { type: Enum<'page:header' \| 'page:footer' \| 'page:sidebar' \| 'page:tabs' \| 'page:accordion' \| 'page:card' \| 'page:section' \| 'record:details' \| 'record:highlights' \| 'record:related_list' \| 'record:activity' \| 'record:chatter' \| 'record:path' \| 'record:alert' \| 'record:quick_actions' \| 'record:reference_rail' \| 'record:history' \| 'app:launcher' \| 'nav:menu' \| 'nav:breadcrumb' \| 'global:search' \| 'global:notifications' \| 'user:profile' \| 'ai:chat_window' \| 'ai:suggestion' \| 'element:text' \| 'element:number' \| 'element:image' \| 'element:divider' \| 'element:button' \| 'element:filter' \| 'element:form' \| 'element:record_picker' \| 'element:text_input'> \| string; id?: string; label?: string; properties?: Record; … }[]; alerts?: { type: Enum<'page:header' \| 'page:footer' \| 'page:sidebar' \| 'page:tabs' \| 'page:accordion' \| 'page:card' \| 'page:section' \| 'record:details' \| 'record:highlights' \| 'record:related_list' \| 'record:activity' \| 'record:chatter' \| 'record:path' \| 'record:alert' \| 'record:quick_actions' \| 'record:reference_rail' \| 'record:history' \| 'app:launcher' \| 'nav:menu' \| 'nav:breadcrumb' \| 'global:search' \| 'global:notifications' \| 'user:profile' \| 'ai:chat_window' \| 'ai:suggestion' \| 'element:text' \| 'element:number' \| 'element:image' \| 'element:divider' \| 'element:button' \| 'element:filter' \| 'element:form' \| 'element:record_picker' \| 'element:text_input'> \| string; id?: string; label?: string; properties?: Record; … } \| { type: Enum<'page:header' \| 'page:footer' \| 'page:sidebar' \| 'page:tabs' \| 'page:accordion' \| 'page:card' \| 'page:section' \| 'record:details' \| 'record:highlights' \| 'record:related_list' \| 'record:activity' \| 'record:chatter' \| 'record:path' \| 'record:alert' \| 'record:quick_actions' \| 'record:reference_rail' \| 'record:history' \| 'app:launcher' \| 'nav:menu' \| 'nav:breadcrumb' \| 'global:search' \| 'global:notifications' \| 'user:profile' \| 'ai:chat_window' \| 'ai:suggestion' \| 'element:text' \| 'element:number' \| 'element:image' \| 'element:divider' \| 'element:button' \| 'element:filter' \| 'element:form' \| 'element:record_picker' \| 'element:text_input'> \| string; id?: string; label?: string; properties?: Record; … }[]; highlights?: { type: Enum<'page:header' \| 'page:footer' \| 'page:sidebar' \| 'page:tabs' \| 'page:accordion' \| 'page:card' \| 'page:section' \| 'record:details' \| 'record:highlights' \| 'record:related_list' \| 'record:activity' \| 'record:chatter' \| 'record:path' \| 'record:alert' \| 'record:quick_actions' \| 'record:reference_rail' \| 'record:history' \| 'app:launcher' \| 'nav:menu' \| 'nav:breadcrumb' \| 'global:search' \| 'global:notifications' \| 'user:profile' \| 'ai:chat_window' \| 'ai:suggestion' \| 'element:text' \| 'element:number' \| 'element:image' \| 'element:divider' \| 'element:button' \| 'element:filter' \| 'element:form' \| 'element:record_picker' \| 'element:text_input'> \| string; id?: string; label?: string; properties?: Record; … } \| { type: Enum<'page:header' \| 'page:footer' \| 'page:sidebar' \| 'page:tabs' \| 'page:accordion' \| 'page:card' \| 'page:section' \| 'record:details' \| 'record:highlights' \| 'record:related_list' \| 'record:activity' \| 'record:chatter' \| 'record:path' \| 'record:alert' \| 'record:quick_actions' \| 'record:reference_rail' \| 'record:history' \| 'app:launcher' \| 'nav:menu' \| 'nav:breadcrumb' \| 'global:search' \| 'global:notifications' \| 'user:profile' \| 'ai:chat_window' \| 'ai:suggestion' \| 'element:text' \| 'element:number' \| 'element:image' \| 'element:divider' \| 'element:button' \| 'element:filter' \| 'element:form' \| 'element:record_picker' \| 'element:text_input'> \| string; id?: string; label?: string; properties?: Record; … }[]; … }` | optional | Slot override map for slotted pages | | **source** | `string` | optional | Page source text. For kind==='html' (alias 'jsx') it is constrained JSX/HTML+Tailwind compiled to the tree by @objectstack/sdui-parser at save time (parse, never execute). For kind==='react' it is real React/JSX executed at render by @object-ui/react-runtime (trusted tier). Authoritative over `regions` in both. | | **requires** | `string[]` | optional | Plugin namespaces the JSX source references (validated at save and load) | +| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). | +| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. | +| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). | +| **_provenance** | `Enum<'package' \| 'org' \| 'env-forced'>` | optional | Origin of the item (package \| org \| env-forced). | +| **_packageId** | `string` | optional | Owning package machine id. | +| **_packageVersion** | `string` | optional | Owning package version. | +| **_lockDocsUrl** | `string` | optional | Optional documentation link surfaced next to _lockReason. | --- diff --git a/docs/audits/2026-07-unknown-key-strictness-ledger.md b/docs/audits/2026-07-unknown-key-strictness-ledger.md index abf3a136ac..2eeee262df 100644 --- a/docs/audits/2026-07-unknown-key-strictness-ledger.md +++ b/docs/audits/2026-07-unknown-key-strictness-ledger.md @@ -239,6 +239,53 @@ dropped at parse, and nothing failed. column being the live one. Fifth instance of finding 9's pattern, and the first where the blinded instrument was a gate rather than a measurement. +12. **The campaign's own helper started signposting removed keys.** `skill` + (batch 4) closed with `strictObject` while still carrying `retiredKey` + tombstones — and `strictObject` built its candidate list from + `Object.keys(shape)`, which includes them. So: + + Unrecognized key(s) on this skill: `triggerPhrase`. … + Did you mean `triggerPhrase` → `triggerPhrases`? + + `triggerPhrases` was REMOVED. An author who complied landed on the tombstone + and got a second rejection telling them to delete what they had just been + told to write. + + **Third occurrence of finding 7's shape** — this campaign's fix pointing the + way into the failure it exists to kill — and the first one the campaign put + in its own *shared* helper, where it would have reached every conversion + after it. Both helpers were correct alone; only the combination was wrong, + which is the kind of defect no per-schema test looks for. + + Fixed structurally rather than by special-casing tombstones: **never suggest + a key the schema cannot accept.** `strictObject` drops candidates that + accept `never` (however wrapped), so the rule holds without knowing why a + key is unwritable. Note the two helpers stay complementary — `retiredKey` is + *stronger* than a `guidance` entry, not redundant with it, because it types + the key as `never` and so fails `tsc` even when the config arrives through a + variable, where excess-property checking would not fire. + +13. **`route` on a page — a fiction the platform's own test suite carried.** + `stack.test.ts` authored `route: '/landing'` on a page for years. + `PageSchema` has never declared `route`; a page is routed by its `name`, + which in the map format under test IS the map key. The test asserted exactly + that, six lines below the key contradicting it. + + Fifth instance of a test codifying a strip-era fiction as intent + (`position.parent`, `object.namespace`, `compactLayout`, + `skill.permissions`, now `page.route`) — and the most likely of them to be + reinvented, because `route` is the first key anyone reaches for on a page. + Now tombstoned along with `path` and `url`. + + Two more from the same file's own comments, and they are the + `skill.permissions` class again: `agent.visibility` and `agent.tenantId` + were **removed as unenforced security properties** and left without a + tombstone, because at the time the shape was `.strip` and there was no + rejection to attach a prescription to. An author who wrote + `visibility: 'private'` believed the agent was hidden; it was listed to + everyone, and always had been. Closing the shape created the channel, so + both got their sentence. + This is the empirical argument for the ratchet: the inference "no metadata in the repo carries unknown keys" was **false three times over**, and only the strict gate could prove it. Note the asymmetry in the two schema gaps — both diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 5067ec62c4..170f384210 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -3583,6 +3583,13 @@ "data/LocationValue:altitude", "data/LocationValue:lat", "data/LocationValue:lng", + "data/Mapping:_lock", + "data/Mapping:_lockDocsUrl", + "data/Mapping:_lockReason", + "data/Mapping:_lockSource", + "data/Mapping:_packageId", + "data/Mapping:_packageVersion", + "data/Mapping:_provenance", "data/Mapping:batchSize", "data/Mapping:errorPolicy", "data/Mapping:extractQuery", @@ -7937,6 +7944,13 @@ "ui/OfflineConfig:queueMaxSize", "ui/OfflineConfig:strategy", "ui/OfflineConfig:sync", + "ui/Page:_lock", + "ui/Page:_lockDocsUrl", + "ui/Page:_lockReason", + "ui/Page:_lockSource", + "ui/Page:_packageId", + "ui/Page:_packageVersion", + "ui/Page:_provenance", "ui/Page:aria", "ui/Page:assignedProfiles", "ui/Page:description", diff --git a/packages/spec/src/ai/agent.zod.ts b/packages/spec/src/ai/agent.zod.ts index e17e1df690..6db24b2d51 100644 --- a/packages/spec/src/ai/agent.zod.ts +++ b/packages/spec/src/ai/agent.zod.ts @@ -10,7 +10,30 @@ import { StateMachineSchema } from '../automation/state-machine.zod'; * AI Model Configuration */ import { lazySchema } from '../shared/lazy-schema'; -export const AIModelConfigSchema = lazySchema(() => z.object({ +import { strictObject } from '../shared/strict-object'; + +/** + * Shared history for this file (#4001). + * + * An agent is configuration for something that then behaves autonomously. A + * dropped key does not stop it — the agent registers, answers, and calls tools, + * just without the constraint its author wrote. The output looks like a working + * agent, which is why this surface hides its mistakes better than most. + */ +const AGENT_HISTORY = + 'Until #4001 closed this shape these were dropped silently — the agent still registered ' + + 'and still answered, minus whatever the key was meant to configure or constrain.'; + +export const AIModelConfigSchema = lazySchema(() => strictObject({ + surface: 'this model configuration', + history: AGENT_HISTORY, + aliases: { + modelName: 'model', llm: 'model', deployment: 'model', engine: 'model', + temp: 'temperature', + maxOutputTokens: 'maxTokens', max_tokens: 'maxTokens', tokenLimit: 'maxTokens', + top_p: 'topP', nucleus: 'topP', + }, +}, { provider: z.enum(['openai', 'azure_openai', 'anthropic', 'local']).default('openai'), model: z.string().describe('Model name (e.g. gpt-4, claude-3-opus)'), temperature: z.number().min(0).max(2).default(0.7), @@ -53,7 +76,18 @@ export const TransformPipelineStepSchema = lazySchema(() => z.enum([ * Structured Output Configuration * Controls how the agent formats and validates its output */ -export const StructuredOutputConfigSchema = lazySchema(() => z.object({ +export const StructuredOutputConfigSchema = lazySchema(() => strictObject({ + surface: 'this structured-output configuration', + history: AGENT_HISTORY, + aliases: { + type: 'format', outputFormat: 'format', + jsonSchema: 'schema', responseSchema: 'schema', + retries: 'maxRetries', maxAttempts: 'maxRetries', + retryOnFailure: 'retryOnValidationFailure', retry: 'retryOnValidationFailure', + fallback: 'fallbackFormat', + pipeline: 'transformPipeline', transforms: 'transformPipeline', postProcess: 'transformPipeline', + }, +}, { /** Output format type */ format: StructuredOutputFormatSchema.describe('Expected output format'), @@ -104,7 +138,53 @@ export type StructuredOutputConfig = z.infer z.object({ +export const AgentSchema = lazySchema(() => strictObject({ + surface: 'this agent', + history: AGENT_HISTORY, + aliases: { + displayName: 'label', title: 'label', + persona: 'role', systemRole: 'role', + prompt: 'instructions', systemPrompt: 'instructions', primeDirectives: 'instructions', + enabled: 'active', isActive: 'active', + allowedUsers: 'access', users: 'access', audience: 'access', + capabilities: 'skills', abilities: 'skills', + icon: 'avatar', image: 'avatar', + output: 'structuredOutput', responseFormat: 'structuredOutput', + limits: 'guardrails', safety: 'guardrails', + }, + guidance: { + // ── The two security-shaped removals ────────────────────────────────── + // Both were dropped rather than tombstoned (see the block below `access`) + // because at the time there was no rejection to hang a prescription on — + // the shape was `.strip`, so any tombstone would have been prose in a + // comment. Closing the shape creates that channel, so they get one now. + // These are `guidance` rather than `retiredKey` deliberately: both removals + // are a major behind, and re-declaring a key on the published type a major + // after deleting it is a worse trade than carrying the sentence here. + // + // This is the `skill.permissions` class, which is the class this campaign + // cares most about: a key that READS as a security control, is not one, and + // says nothing when you write it. An author who set `visibility: 'private'` + // believed the agent was hidden. It was listed to everyone, and had been + // all along. + visibility: + '`visibility` was removed in #1901 — it never hid anything. No runtime read it: ' + + 'the chat-access evaluator ignored it and the agent list route did not filter on it, ' + + "so `private` listed the agent to everyone. Use `access` (who may chat) and/or " + + '`permissions` (required permission-set capabilities) — both ENFORCED at the chat route since #1884.', + tenantId: + '`tenantId` was removed in #2377 — it never scoped anything. Tenancy comes from the ' + + 'request context (`resolveAuthzContext`), never from a field on the artifact, so this ' + + 'key did not restrict which tenant could reach the agent. Delete it; scope reachability ' + + 'with `access` / `permissions`.', + // ── Wrong-layer pointers ────────────────────────────────────────────── + temperature: 'model settings live under `model` — write `model: { temperature: … }`', + provider: 'model settings live under `model` — write `model: { provider: … }`', + maxTokens: + 'ambiguous at this level — per-request model output is `model.maxTokens`; a budget ' + + 'ceiling for the whole invocation is `guardrails.maxTokensPerInvocation`', + }, +}, { /** Identity */ name: z.string().regex(/^[a-z_][a-z0-9_]*$/).describe('Agent unique identifier'), label: z.string().describe('Agent display name'), @@ -193,13 +273,29 @@ export const AgentSchema = lazySchema(() => z.object({ // real owner/org semantics. /** Autonomous Reasoning */ - planning: z.object({ + planning: strictObject({ + surface: 'this planning configuration', + history: AGENT_HISTORY, + aliases: { maxSteps: 'maxIterations', maxLoops: 'maxIterations', iterations: 'maxIterations', maxTurns: 'maxIterations' }, + }, { /** Maximum reasoning iterations before stopping */ maxIterations: z.number().int().min(1).max(100).default(10).describe('Maximum planning loop iterations'), }).optional().describe('Autonomous reasoning and planning configuration'), /** Memory Management */ - memory: z.object({ + memory: strictObject({ + surface: 'this memory configuration', + history: AGENT_HISTORY, + aliases: { persistent: 'longTerm', persistence: 'longTerm', reflection: 'reflectionInterval', reflectEvery: 'reflectionInterval' }, + guidance: { + // The removal recorded in the note below, given the rejection it never had. + shortTerm: + '`shortTerm` was removed (ADR-0013 D3, cloud#339) — it declared a working-memory ' + + 'window nothing in the runtime consumed. Cross-turn grounding comes from tools reading ' + + 'live state, and the context budget is governed by the per-request token guardrail, ' + + 'not by this block. Delete it.', + }, + }, { // NOTE: `shortTerm` ({maxMessages,maxTokens}) was removed (ADR-0013 D3, // cloud#339). It declared a working-memory window that NOTHING in the // runtime consumed — a config that lies. Cross-turn grounding is done by @@ -208,7 +304,11 @@ export const AgentSchema = lazySchema(() => z.object({ // `reflectionInterval` are kept as forward-looking, off-by-default config. /** Long-term (persistent) memory configuration */ - longTerm: z.object({ + longTerm: strictObject({ + surface: 'this long-term memory configuration', + history: AGENT_HISTORY, + aliases: { backend: 'store', storage: 'store', provider: 'store', limit: 'maxEntries', maxItems: 'maxEntries', active: 'enabled' }, + }, { /** Whether long-term memory is enabled */ enabled: z.boolean().default(false).describe('Enable long-term memory persistence'), @@ -224,7 +324,25 @@ export const AgentSchema = lazySchema(() => z.object({ }).optional().describe('[EXPERIMENTAL — not enforced] Agent memory management. Parsed but no runtime consumer yet (liveness #1878/#1893).'), /** Guardrails */ - guardrails: z.object({ + guardrails: strictObject({ + surface: 'these guardrails', + history: AGENT_HISTORY, + aliases: { + maxTokens: 'maxTokensPerInvocation', tokenBudget: 'maxTokensPerInvocation', tokenLimit: 'maxTokensPerInvocation', + timeout: 'maxExecutionTimeSec', maxExecutionTime: 'maxExecutionTimeSec', timeoutSec: 'maxExecutionTimeSec', + blocked: 'blockedTopics', forbiddenTopics: 'blockedTopics', denyTopics: 'blockedTopics', + }, + guidance: { + // Guardrails are the block an author reaches for when they want a limit + // enforced, so a near-miss here is more likely than elsewhere to be + // someone asking for a control that does not exist. Say which do. + allowedTopics: + 'there is no allow-list — only `blockedTopics` (a deny-list). Steer permitted scope ' + + 'in `instructions`; restrict WHO can invoke the agent with `access` / `permissions`', + maxCostUsd: 'there is no cost ceiling here — budget by tokens with `maxTokensPerInvocation`', + rateLimit: 'per-caller rate limiting is not an agent field — it is enforced by the quota service', + }, + }, { /** Maximum tokens the agent may consume per invocation */ maxTokensPerInvocation: z.number().int().min(1).optional().describe('Token budget per single invocation'), diff --git a/packages/spec/src/ai/skill.test.ts b/packages/spec/src/ai/skill.test.ts index 203da23a3e..681f92c9f6 100644 --- a/packages/spec/src/ai/skill.test.ts +++ b/packages/spec/src/ai/skill.test.ts @@ -196,4 +196,22 @@ describe('#3896 close-out — retired `triggerPhrases`', () => { expect(message).toMatch(/triggerConditions/); expect(message).toMatch(/#3896/); }); + + it('does not offer the retired key as the fix for a near-miss of it', () => { + // When `skill` went strict (#4001 batch 4) the tombstone landed in the + // suggester's candidate list, so a `triggerPhrase` typo was answered with + // "Did you mean `triggerPhrases`?" — routing the author onto a REMOVED key, + // and into a second rejection telling them to delete what they had just + // been told to write. `strictObject` now excludes keys the schema cannot + // accept; see `shared/strict-object.test.ts` for the general rule. + const result = SkillSchema.safeParse({ + name: 'case_mgmt', + label: 'Cases', + triggerPhrase: ['open a ticket'], + }); + expect(result.success).toBe(false); + const message = result.error?.issues.map((i) => i.message).join(' | ') ?? ''; + expect(message).toContain('`triggerPhrase`'); + expect(message).not.toContain('triggerPhrases'); + }); }); diff --git a/packages/spec/src/data/mapping.zod.ts b/packages/spec/src/data/mapping.zod.ts index 5610083af8..4c08927c9f 100644 --- a/packages/spec/src/data/mapping.zod.ts +++ b/packages/spec/src/data/mapping.zod.ts @@ -2,8 +2,23 @@ import { z } from 'zod'; import { SnakeCaseIdentifierSchema } from '../shared/identifiers.zod'; +import { strictObject } from '../shared/strict-object'; +import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { QuerySchema } from './query.zod'; +/** + * Shared history for this file (#4001). + * + * An import mapping is instructions for moving somebody's data. A dropped key + * does not fail the import — it runs, to completion, with a "success" the + * author reads as "the data arrived the way I described it". The rows land + * untransformed, or unmatched, or duplicated, and the diagnosis starts from a + * green run. + */ +const MAPPING_HISTORY = + 'Until #4001 closed this shape these were dropped silently — the mapping still ran to ' + + 'completion and reported success, minus whatever the key was meant to control.'; + /** * Transformation Logic * Built-in helpers for converting data during import. @@ -22,30 +37,55 @@ export const TransformType = z.enum([ /** * Field Mapping Item */ -export const FieldMappingSchema = lazySchema(() => z.object({ +export const FieldMappingSchema = lazySchema(() => strictObject({ + surface: 'this field mapping', + history: MAPPING_HISTORY, + aliases: { + from: 'source', sourceField: 'source', column: 'source', header: 'source', + to: 'target', targetField: 'target', field: 'target', + type: 'transform', operation: 'transform', fn: 'transform', + config: 'params', options: 'params', args: 'params', + }, +}, { /** Source Column */ source: z.union([z.string(), z.array(z.string())]).describe('Source column header(s)'), - + /** Target Field */ target: z.union([z.string(), z.array(z.string())]).describe('Target object field(s)'), - + /** Transformation */ transform: TransformType.default('none'), - + /** Configuration for transform */ - params: z.object({ + // One flat bag rather than a per-`transform` discriminated union, so which + // keys are meaningful depends on `transform`. Closing it catches the spelling; + // it does NOT catch `separator` on a `lookup` (a key that is real but inert + // here). Narrowing per transform is a refinement, not a strictness question. + params: strictObject({ + surface: 'this transform’s params', + history: MAPPING_HISTORY, + aliases: { + default: 'value', defaultValue: 'value', constant: 'value', + lookupObject: 'object', targetObject: 'object', + match: 'fromField', matchOn: 'fromField', matchField: 'fromField', keyField: 'fromField', + returnField: 'toField', valueField: 'toField', + create: 'autoCreate', createIfMissing: 'autoCreate', upsert: 'autoCreate', + map: 'valueMap', mapping: 'valueMap', values: 'valueMap', valueMapping: 'valueMap', + delimiter: 'separator', splitOn: 'separator', joinWith: 'separator', + }, + }, { // Constant value: z.unknown().optional(), - + // Lookup object: z.string().optional(), // Lookup Object fromField: z.string().optional(), // Match on (e.g. "name") toField: z.string().optional(), // Value to take (e.g. "id") autoCreate: z.boolean().optional(), // Create if missing - + // Map valueMap: z.record(z.string(), z.unknown()).optional(), // { "Open": "draft" } - + // Split/Join separator: z.string().optional() }).optional() @@ -67,28 +107,53 @@ export const FieldMappingSchema = lazySchema(() => z.object({ * - 'SalesforceToCRM' (PascalCase) * - 'CSV Import' (spaces) */ -export const MappingSchema = lazySchema(() => z.object({ +export const MappingSchema = lazySchema(() => strictObject({ + surface: 'this mapping', + history: MAPPING_HISTORY, + aliases: { + object: 'targetObject', target: 'targetObject', to: 'targetObject', + format: 'sourceFormat', source: 'sourceFormat', sourceType: 'sourceFormat', + mappings: 'fieldMapping', fields: 'fieldMapping', columns: 'fieldMapping', fieldMappings: 'fieldMapping', + key: 'upsertKey', matchOn: 'upsertKey', externalId: 'upsertKey', externalIdField: 'upsertKey', + query: 'extractQuery', + onError: 'errorPolicy', errorHandling: 'errorPolicy', errorMode: 'errorPolicy', + batch: 'batchSize', chunkSize: 'batchSize', + }, + guidance: { + // `mode: 'upsert'` needs `upsertKey`; an author reaching for a + // dedup/matching knob under another name is describing that pair. + dedupe: 'deduplication is `mode: \'upsert\'` plus `upsertKey: []` — there is no separate dedupe switch', + skipErrors: "`errorPolicy: 'skip'` is the default and already does this — remove the key", + }, +}, { /** Identity */ name: SnakeCaseIdentifierSchema.describe('Mapping unique name (lowercase snake_case)'), label: z.string().optional(), - + /** Scope */ sourceFormat: z.enum(['csv', 'json', 'xml', 'sql']).default('csv'), targetObject: z.string().describe('Target Object Name'), - + /** Column Mappings */ fieldMapping: z.array(FieldMappingSchema), - + /** Upsert Logic */ mode: z.enum(['insert', 'update', 'upsert']).default('insert'), upsertKey: z.array(z.string()).optional().describe('Fields to match for upsert (e.g. email)'), - + /** Extract Logic (For Export) */ extractQuery: QuerySchema.optional().describe('Query to run for export only'), - + /** Error Handling */ errorPolicy: z.enum(['skip', 'abort', 'retry']).default('skip'), - batchSize: z.number().default(1000) + batchSize: z.number().default(1000), + + // ADR-0010 — runtime protection envelope (internal — set by the loader). + // `mapping` is a registered metadata type, so `MetadataPlugin`'s loader + // stamps `_packageId` / `_provenance` on it like every sibling. Undeclared, + // they were dropped on every parse: protection metadata lost on round-trip, + // and a hard 422 the day this shape closed — which is today. + ...MetadataProtectionFields, })); export type Mapping = z.infer; diff --git a/packages/spec/src/kernel/metadata-authoring-lint.test.ts b/packages/spec/src/kernel/metadata-authoring-lint.test.ts index 33edded9c3..82eb2c1a7f 100644 --- a/packages/spec/src/kernel/metadata-authoring-lint.test.ts +++ b/packages/spec/src/kernel/metadata-authoring-lint.test.ts @@ -21,6 +21,8 @@ import { import { STACK_KEY_GUIDANCE, STACK_RUNTIME_MEMBERS } from '../data/authoring-key-lint'; import { PLURAL_TO_SINGULAR } from '../shared/metadata-collection.zod'; import { ObjectSchema } from '../data/object.zod'; +import { PageSchema } from '../ui/page.zod'; +import { AgentSchema } from '../ai/agent.zod'; import { ObjectStackDefinitionSchema } from '../stack.zod'; import { getMetadataTypeSchema } from './metadata-type-schemas'; @@ -39,15 +41,23 @@ describe('coverage derivation (#3786 — no third hand-written list)', () => { // list below; that confirmation is the whole point of pinning a number here. // 15 → 13 when `seed` + `doc` graduated (#4001 registered-types batch); // 13 → 12 when `object` closed on the parse path; 12 → 6 when the seven - // small registered types closed in one batch. Note what did NOT shrink with - // `object`: its 71 NESTED strip sites still report, because the walk no - // longer gates a whole collection on its root's posture — so this number - // tracks ROOTS that graduated, not coverage lost. - expect(lintables.length).toBeGreaterThanOrEqual(6); + // small registered types closed in one batch; 6 → 3 when `mapping`, `agent` + // and `page` closed. Note what did NOT shrink with `object`: its 71 NESTED + // strip sites still report, because the walk no longer gates a whole + // collection on its root's posture — so this number tracks ROOTS that + // graduated, not coverage lost. + // + // Lowering this number is only honest if each departed root is now REJECTED + // by the parse rather than merely unwatched. That was checked directly for + // this shrink — `agent.zzz`, `page.zzz`, and the nested + // `page.regions[0].zzz` all `safeParse` to failure — and the check is worth + // repeating on the next one, because a broken walk and a successful + // graduation shrink this count identically. + expect(lintables.length).toBeGreaterThanOrEqual(3); // `view` matters doubly: it is a UNION (container | ViewItem | overlay), so // its presence pins the union half of the posture logic — a regression that // silently dropped unions would shrink coverage without failing the count. - for (const expected of ['page', 'agent', 'dashboard', 'action', 'view']) { + for (const expected of ['dashboard', 'action', 'view']) { expect(lintableTypes, `expected '${expected}' to be lint-covered`).toContain(expected); } }); @@ -146,18 +156,35 @@ describe('the #4148 behaviours survive the generalization', () => { }); it('reports across collections in one walk, with per-type surfaces', () => { + // `page` and `agent` used to appear here too. They closed (#4001 batch 6a), + // so the parse rejects those keys and the lint correctly stays quiet — the + // hand-off this whole layer was built to make. `dashboard` keeps a + // second collection in the walk so this still proves per-type surfacing + // rather than degenerating into a single-collection test. const findings = lintUnknownAuthoringKeys({ objects: [{ name: 'a', label: 'A', fields: { x: { type: 'text', indexed: true } } }], - pages: [{ name: 'p', label: 'P', zzz: 1 }], - agents: [{ name: 'ag', zzz: 1 }], + dashboards: [{ name: 'd', label: 'D', zzz: 1 }], }); expect(findings.map((f) => `${f.surface}:${f.path}`).sort()).toEqual([ - 'agent:agents.ag.zzz', + 'dashboard:dashboards.d.zzz', 'field:objects.a.fields.x.indexed', - 'page:pages.p.zzz', ]); }); + it('hands off to the parse for the roots that closed', () => { + // The other half of the assertion above, and the one that makes lowering + // the coverage floor honest: these keys are not unreported, they are + // REJECTED. A broken walk would produce the same empty lint result. + expect(lintUnknownAuthoringKeys({ + pages: [{ name: 'p', label: 'P', zzz: 1 }], + agents: [{ name: 'ag', zzz: 1 }], + })).toEqual([]); + expect(PageSchema.safeParse({ name: 'p', label: 'P', zzz: 1 }).success).toBe(false); + expect(AgentSchema.safeParse({ + name: 'ag', label: 'A', role: 'r', instructions: 'i', zzz: 1, + }).success).toBe(false); + }); + it('survives malformed input rather than throwing', () => { for (const junk of [undefined, null, 42, 'x', {}, { objects: 'nope' }, { pages: [null, 7] }]) { expect(() => lintUnknownAuthoringKeys(junk)).not.toThrow(); @@ -193,10 +220,17 @@ describe('nested descent (#4001 evidence phase)', () => { }); it('reports inside an array element, indexed by position', () => { + // Was `pages[].regions[]` until `page` closed (#4001 batch 6a) — the parse + // rejects that key now. `object.actions[]` is the same structural case and + // carries a second property worth pinning: `object` itself is CLOSED, and + // its nested strip sites still report. That is the #4522 fix — the walk + // descends per node instead of gating a whole collection on its root's + // posture — so this test now covers the array index and that regression at + // once. const [finding] = lintUnknownAuthoringKeys({ - pages: [{ name: 'p1', regions: [{ name: 'r', zzz_nested: 1 }] }], + objects: [{ name: 'o1', actions: [{ name: 'a', zzz_nested: 1 }] }], }); - expect(finding).toMatchObject({ path: 'pages.p1.regions.0.zzz_nested', surface: 'page' }); + expect(finding).toMatchObject({ path: 'objects.o1.actions.0.zzz_nested', surface: 'object' }); }); it('carries the field surface through a record INTO its nested array', () => { diff --git a/packages/spec/src/kernel/metadata-type-schemas.test.ts b/packages/spec/src/kernel/metadata-type-schemas.test.ts index d905914337..f2ae03416d 100644 --- a/packages/spec/src/kernel/metadata-type-schemas.test.ts +++ b/packages/spec/src/kernel/metadata-type-schemas.test.ts @@ -76,7 +76,7 @@ const PROBE: Record = { * when its groups were closed, leaving 4. */ const UNDECLARED_ENVELOPE = new Set([ - 'action', 'field', 'mapping', 'page', + 'action', 'field', ]); /** @@ -198,3 +198,88 @@ describe('registered metadata types', () => { }, ); }); + +/** + * The #4001 headline number, derived instead of tallied. + * + * "N of 25 registered types closed" was carried by hand across batches and had + * drifted by one before anyone measured it — in a campaign whose recurring + * finding is that hand-maintained measurements of coverage go stale (the ledger + * gate's non-recursive walk, the hollow envelope probe, the lint's root-gated + * posture). A number nobody can check is the same class of instrument. + * + * `STILL_STRIP` carries the same reverse pin as `UNDECLARED_ENVELOPE`: closing a + * type fails this suite until the list shrinks, so the list cannot outlive the + * debt and start exempting types that no longer need exempting. + */ +const STILL_STRIP = new Set(['action', 'dashboard', 'field', 'view']); + +/** The registered schema's own top-level posture: `.strict()` sets a `never` catchall. */ +function topLevelPosture(schema: unknown, depth = 0): 'strict' | 'strip' | null { + if (!schema || depth > 12) return null; + const s = schema as { _zod?: { def?: Def }; def?: Def }; + const def = s._zod?.def ?? s.def; + switch (def?.type) { + case 'object': { + const catchall = (def as { catchall?: { _zod?: { def?: Def } } }).catchall; + return catchall?._zod?.def?.type === 'never' ? 'strict' : 'strip'; + } + case 'lazy': + try { + return topLevelPosture(def.getter?.(), depth + 1); + } catch { + return null; + } + case 'pipe': + return topLevelPosture(def.out, depth + 1) ?? topLevelPosture(def.in, depth + 1); + case 'union': { + // A union is closed only when EVERY variant is — one open variant is an + // open door, and `validation` is exactly this shape. + const seen = (def.options ?? []).map((o) => topLevelPosture(o, depth + 1)); + if (seen.length === 0) return null; + if (seen.every((r) => r === 'strict')) return 'strict'; + return seen.some((r) => r !== null) ? 'strip' : null; + } + case 'optional': + case 'nullable': + case 'default': + case 'prefault': + case 'readonly': + case 'nonoptional': + case 'catch': + return topLevelPosture(def.innerType, depth + 1); + default: + return null; + } +} + +describe('#4001 — registered-type closure is derived, not tallied', () => { + const types = listMetadataTypeSchemaTypes(); + + it.each(types.filter((t) => !STILL_STRIP.has(t)))('%s REJECTS unknown keys', (type) => { + const posture = topLevelPosture(getMetadataTypeSchema(type)); + // A type the walker cannot resolve is a hard failure, never a pass — the + // walker going quiet is precisely when this would stop covering something. + expect(posture, `could not resolve a top-level posture for '${type}'`).not.toBeNull(); + expect( + posture, + `'${type}' still strips unknown keys. Close it with \`strictObject\`, or add it to ` + + 'STILL_STRIP with a reason.', + ).toBe('strict'); + }); + + it.each([...STILL_STRIP])('%s is still on the strip list (remove it once closed)', (type) => { + expect(types).toContain(type); + expect( + topLevelPosture(getMetadataTypeSchema(type)), + `'${type}' is now strict — remove it from STILL_STRIP and update the count.`, + ).toBe('strip'); + }); + + it('reports the campaign number so a reader never has to count', () => { + const closed = types.filter((t) => !STILL_STRIP.has(t)); + expect(closed.length + STILL_STRIP.size).toBe(types.length); + expect(closed.length).toBe(21); + expect(types.length).toBe(25); + }); +}); diff --git a/packages/spec/src/shared/strict-object.test.ts b/packages/spec/src/shared/strict-object.test.ts index a7ab7922bb..6b57b09440 100644 --- a/packages/spec/src/shared/strict-object.test.ts +++ b/packages/spec/src/shared/strict-object.test.ts @@ -153,3 +153,59 @@ describe('strictObject', () => { expect(stripJson.additionalProperties).toBe(false); }); }); + +/** + * Never suggest a key the schema cannot accept. + * + * `retiredKey` declares a removed key as `z.never().optional()` so the removal + * is audible in both channels an upgrading author hits — `tsc` and the parse. + * That is deliberate and stronger than a `guidance` entry. But it also leaves + * the dead key in `Object.keys(shape)`, and the suggester offered it: on + * `skill`, a `triggerPhrase` typo was answered with + * "Did you mean `triggerPhrases`?" — a key that had been REMOVED. An author who + * complied got a second rejection telling them to delete what they had just + * been told to write. + * + * Ledger finding 7, third occurrence: this campaign's own fix signposting the + * way into the failure mode it exists to kill. Pinned here because the two + * helpers are each correct alone and only wrong in combination — which is the + * kind of defect no per-schema test goes looking for. + */ +describe('strictObject × retiredKey — suggestions never point at a dead key', () => { + const Tombstoned = lazySchema(() => + strictObject( + { + surface: 'this thing', + history: 'Until #4001 these were dropped silently.', + }, + { + label: z.string(), + // Shaped exactly as `retiredKey()` builds it. + triggerPhrases: z + .never({ error: () => '`triggerPhrases` was removed in vX. Delete it.' }) + .optional(), + }, + ), + ); + + const messageFor = (body: Record) => { + const r = Tombstoned.safeParse({ label: 'x', ...body }); + expect(r.success).toBe(false); + return r.error!.issues.map((i) => i.message).join(' | '); + }; + + it('does not offer a removed key as the fix for a near-miss of it', () => { + expect(messageFor({ triggerPhrase: ['hi'] })).not.toContain('triggerPhrases'); + }); + + it('still offers a LIVE key for a near-miss of it', () => { + // The narrowing must not cost the suggester its actual job. + expect(messageFor({ lable: 'x' })).toContain('`lable` → `label`'); + }); + + it('still raises the tombstone prescription when the dead key is written', () => { + // Excluded from the CANDIDATE list, not from the shape — writing it must + // still produce the upgrade instruction, not a generic unknown-key error. + expect(messageFor({ triggerPhrases: ['hi'] })).toContain('was removed in vX'); + }); +}); diff --git a/packages/spec/src/shared/strict-object.ts b/packages/spec/src/shared/strict-object.ts index 350948e893..1019080f6a 100644 --- a/packages/spec/src/shared/strict-object.ts +++ b/packages/spec/src/shared/strict-object.ts @@ -62,6 +62,51 @@ import { z } from 'zod'; import { strictUnknownKeyError } from './suggestions.zod'; +/** + * True when `schema` accepts no value at all — a `z.never()`, however wrapped. + * + * The case this exists for is {@link retiredKey}, which declares a removed key + * as `z.never().optional()` so the removal is audible in both channels an + * upgrading author hits: `tsc` (the input type is `never`) and the parse (the + * value raises the upgrade prescription). That declaration is deliberate and + * strictly stronger than a `guidance` entry — but it also puts the dead key in + * `Object.keys(shape)`, and the suggester happily offered it. + * + * Which produced this, on `skill`, from the campaign's own helper: + * + * Unrecognized key(s) on this skill: `triggerPhrase`. … + * Did you mean `triggerPhrase` → `triggerPhrases`? + * + * `triggerPhrases` was REMOVED. An author who took the advice landed on the + * tombstone and got a second rejection telling them to delete what they had + * just been told to write. Not silent — but it is the shape the ledger's + * finding 7 already records twice: **this campaign's own fix signposting the + * way into the failure mode it exists to kill.** + * + * The rule is narrower than "skip tombstones" and holds without knowing why a + * key is unwritable: **never suggest a key the schema cannot accept.** A + * structural check gets that for free, and keeps working if the tombstone + * helper is ever reshaped. + */ +function acceptsNothing(schema: unknown, depth = 0): boolean { + if (depth > 6) return false; + const def = (schema as { _zod?: { def?: { type?: string; innerType?: unknown } } })._zod?.def; + if (!def?.type) return false; + if (def.type === 'never') return true; + switch (def.type) { + case 'optional': + case 'nullable': + case 'default': + case 'prefault': + case 'readonly': + case 'nonoptional': + case 'catch': + return acceptsNothing(def.innerType, depth + 1); + default: + return false; + } +} + /** Authoring-surface metadata for {@link strictObject}. */ export interface StrictObjectOptions { /** Prose name of the surface the key was written on (e.g. `'this widget'`). */ @@ -97,7 +142,14 @@ export function strictObject(options: StrictObjectOptio .object(shape, { error: strictUnknownKeyError({ surface, - knownKeys: [...Object.keys(shape), ...extraKeys], + // Declared-but-unwritable keys (tombstones) are excluded — see + // `acceptsNothing`. They stay in the SHAPE, so writing one still raises + // its own prescription; they are only kept out of the candidate list a + // typo gets pointed at. + knownKeys: [ + ...Object.keys(shape).filter((k) => !acceptsNothing(shape[k])), + ...extraKeys, + ], history, aliases, guidance, diff --git a/packages/spec/src/stack.test.ts b/packages/spec/src/stack.test.ts index c9ee95321b..0397d7f0bd 100644 --- a/packages/spec/src/stack.test.ts +++ b/packages/spec/src/stack.test.ts @@ -409,7 +409,11 @@ describe('defineStack - Map Format Support', () => { landing: { label: 'Landing Page', type: 'app', - route: '/landing', + // `route: '/landing'` used to sit here. `PageSchema` has never + // declared a `route` key — a page is routed by its `name`, which in + // map format IS the map key (`landing`), which is what this test + // asserts six lines down. `.strip` ate it, so the fiction rode along + // in the platform's own suite until the shape closed (#4001). regions: [ { name: 'main', components: [{ type: 'page:section', properties: {} }] }, ], diff --git a/packages/spec/src/ui/page.zod.ts b/packages/spec/src/ui/page.zod.ts index d4e9689339..5e145b5be8 100644 --- a/packages/spec/src/ui/page.zod.ts +++ b/packages/spec/src/ui/page.zod.ts @@ -22,7 +22,26 @@ import { * A named region in the template where components are dropped. */ import { lazySchema } from '../shared/lazy-schema'; -export const PageRegionSchema = lazySchema(() => z.object({ +import { strictObject } from '../shared/strict-object'; +import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; + +/** + * Shared history for this file (#4001). + * + * A page's failure mode is visual, and therefore easy to misread. A dropped key + * renders a page — just not the one that was authored. The author sees output, + * assumes the schema was understood, and goes looking for the mistake in their + * layout rather than in their spelling. + */ +const PAGE_HISTORY = + 'Until #4001 closed this shape these were dropped silently — the page still rendered, ' + + 'without whatever the key was meant to configure.'; + +export const PageRegionSchema = lazySchema(() => strictObject({ + surface: 'this page region', + history: PAGE_HISTORY, + aliases: { id: 'name', region: 'name', children: 'components', items: 'components', content: 'components', size: 'width', span: 'width' }, +}, { name: z.string().describe('Region name (e.g. "sidebar", "main", "header")'), width: z.enum(['small', 'medium', 'large', 'full']).optional(), components: z.array(z.lazy(() => PageComponentSchema)).describe('Components in this region') @@ -53,7 +72,17 @@ export const PageComponentType = z.enum([ * Per-element data binding for multi-object pages. * Overrides page-level object context so each element can query a different object. */ -export const ElementDataSourceSchema = lazySchema(() => z.object({ +export const ElementDataSourceSchema = lazySchema(() => strictObject({ + surface: 'this element data source', + history: PAGE_HISTORY, + aliases: { + objectName: 'object', from: 'object', source: 'object', entity: 'object', + viewName: 'view', listView: 'view', + filters: 'filter', where: 'filter', criteria: 'filter', + orderBy: 'sort', sortBy: 'sort', + top: 'limit', pageSize: 'limit', maxRecords: 'limit', count: 'limit', + }, +}, { object: z.string().describe('Object to query'), view: z.string().optional().describe('Named view to apply'), filter: FilterConditionSchema.optional().describe('Additional filter criteria'), @@ -139,7 +168,17 @@ export const PageComponentSchema = lazySchema(() => z.object({ * writes the user's selection into `selectedProjectId`; predicates then read it * as `page.selectedProjectId`. */ -export const PageVariableSchema = lazySchema(() => z.object({ +export const PageVariableSchema = lazySchema(() => strictObject({ + surface: 'this page variable', + history: PAGE_HISTORY, + aliases: { default: 'defaultValue', initial: 'defaultValue', initialValue: 'defaultValue', value: 'defaultValue', boundTo: 'source', writer: 'source', from: 'source', component: 'source' }, + guidance: { + // The binding direction is the one thing authors reverse here, and the + // JSDoc above spells it out — so say it at the point of the mistake too. + target: 'the binding names the WRITER, not a target — `source` is the id of the component that writes this variable; readers reference it as `page.`', + bindTo: 'the binding names the WRITER, not a target — `source` is the id of the component that writes this variable; readers reference it as `page.`', + }, +}, { name: z.string().describe('Variable name. Exposed to expressions as `page.`.'), type: z.enum(['string', 'number', 'boolean', 'object', 'array', 'record_id']).default('string'), defaultValue: z.unknown().optional() @@ -228,7 +267,31 @@ export const PAGE_TYPE_ROADMAP = [ * * @see Airtable Interface → right panel (Page / Data / Appearance / User filters / User actions / Advanced) */ -export const InterfacePageConfigSchema = lazySchema(() => z.object({ +export const InterfacePageConfigSchema = lazySchema(() => strictObject({ + surface: 'this interface page configuration', + history: PAGE_HISTORY, + aliases: { + object: 'source', objectName: 'source', sourceObject: 'source', + fields: 'columns', columnList: 'columns', + orderBy: 'sort', sortBy: 'sort', + filter: 'filterBy', filters: 'filterBy', where: 'filterBy', baseFilter: 'filterBy', + view: 'sourceView', + actions: 'userActions', toolbar: 'buttons', toolbarButtons: 'buttons', + quickFilters: 'userFilters', filterBar: 'userFilters', + onRecordClick: 'recordAction', rowAction: 'recordAction', openIn: 'recordAction', + createRecord: 'addRecord', newRecord: 'addRecord', + showCount: 'showRecordCount', recordCount: 'showRecordCount', + printable: 'allowPrinting', printing: 'allowPrinting', + }, + guidance: { + // The doc block on PageTypeSchema makes this point at length: the display + // mode is a VISUALIZATION, not a page type, and it is configured here. + visualization: "the display mode is chosen at runtime from `appearance.allowedVisualizations` (grid | kanban | calendar | gallery | timeline) — it is not a page-level key, and it is not a page `type`", + visualizations: "use `appearance.allowedVisualizations` — the whitelist lives under `appearance`", + kanban: "kanban is a VISUALIZATION of a `list` page, not a setting — allow it via `appearance.allowedVisualizations`", + groupBy: 'grouping belongs to the visualization — configure it under `appearance`', + }, +}, { /** Data binding (ADR-0047: pages REFERENCE views, never restate them) */ source: z.string().optional().describe('Source object name for the page'), @@ -298,7 +361,47 @@ export const InterfacePageConfigSchema = lazySchema(() => z.object({ * - 'PageDashboard' (PascalCase) * - 'Settings Page' (spaces) */ -export const PageSchema = lazySchema(() => z.object({ +export const PageSchema = lazySchema(() => strictObject({ + surface: 'this page', + history: PAGE_HISTORY, + aliases: { + title: 'label', displayName: 'label', + objectName: 'object', entity: 'object', + pageType: 'type', kindType: 'type', + layout: 'template', layoutTemplate: 'template', + sections: 'regions', areas: 'regions', zones: 'regions', + components: 'regions', children: 'regions', + state: 'variables', params: 'variables', vars: 'variables', + config: 'interfaceConfig', interface: 'interfaceConfig', + profiles: 'assignedProfiles', assignedTo: 'assignedProfiles', + default: 'isDefault', + jsx: 'source', html: 'source', code: 'source', content: 'source', + dependencies: 'requires', plugins: 'requires', + }, + guidance: { + // The removals this file's own comments record. Each was a page type or a + // block with no renderer; deleting them left an author writing something + // that had never worked and, until now, was not told so. + recordReview: + '`recordReview` was removed with the `record_review` page type (framework#2265) — it had ' + + 'no renderer, so the page validated and then failed at runtime. Use `type: \'record\'` with ' + + '`regions`, or `kind: \'slotted\'` to override individual slots.', + blankLayout: + '`blankLayout` was removed with the `blank` page type (framework#2265) — it had no renderer. ' + + 'A free-form page is `kind: \'html\'` with `source` (ADR-0080).', + // `route` is the key an author reaches for first, and it has never existed. + // The page's `name` IS its routing identity — this file's own naming + // convention says so ("Page names are used in routing") — so a page that + // declared `route: '/landing'` was routed by its name regardless, and the + // author's chosen URL silently did nothing. The platform's own test suite + // authored one for years; see `stack.test.ts`. + route: '`route` is not a page key — a page is routed by its `name` (lowercase snake_case). Rename the page rather than declaring a path.', + path: '`path` is not a page key — a page is routed by its `name` (lowercase snake_case).', + url: '`url` is not a page key — a page is routed by its `name`. To link OUT to an address, use a navigation node on the app.', + visibleWhen: 'page-level conditional rendering does not exist — put `visibleWhen` on the COMPONENT inside a region, or gate the page with `assignedProfiles`', + permissions: 'a page is not permission-gated by a field — reach it through `assignedProfiles`, and gate the DATA it shows with the object\'s permission sets (which is what actually protects the records)', + }, +}, { name: SnakeCaseIdentifierSchema.describe('Page unique name (lowercase snake_case)'), label: I18nLabelSchema, description: I18nLabelSchema.optional(), @@ -389,7 +492,14 @@ export const PageSchema = lazySchema(() => z.object({ * * Only honored when `kind === 'slotted'`. */ - slots: z.object({ + slots: strictObject({ + surface: 'this slot map', + history: PAGE_HISTORY, + // The slot menu is a CLOSED v1 set (each slot has a `buildDefault*` + // sub-builder behind it), so an unknown slot name is not a slot that will + // start working later — it is content that never renders. + aliases: { activity: 'discussion', chatter: 'discussion', comments: 'discussion', related: 'tabs', relatedLists: 'tabs', fields: 'details', body: 'details', banner: 'alerts', toolbar: 'actions', buttons: 'actions', summary: 'highlights' }, + }, { header: z.union([PageComponentSchema, z.array(PageComponentSchema)]).optional(), actions: z.union([PageComponentSchema, z.array(PageComponentSchema)]).optional(), alerts: z.union([PageComponentSchema, z.array(PageComponentSchema)]).optional(), @@ -411,6 +521,13 @@ export const PageSchema = lazySchema(() => z.object({ /** Plugin namespaces the JSX source references — inferred at compile, checked at save AND load (ADR-0048 provenance). */ requires: z.array(z.string()).optional() .describe('Plugin namespaces the JSX source references (validated at save and load)'), + + // ADR-0010 — runtime protection envelope (internal — set by the loader). + // `page` is a registered metadata type, so `MetadataPlugin`'s loader stamps + // `_packageId` / `_provenance` on it. Undeclared, they were dropped on every + // parse — protection metadata lost on round-trip, and a hard 422 the day this + // shape closed. + ...MetadataProtectionFields, }).superRefine((page, ctx) => { // ADR-0080/0081 + ADR-0078 (completeness): an html/react/jsx page with no // `source` is silently inert — fail loudly at author time, never render empty.