From 13c52342ea336048df142501c15525b52c8ad3d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 14:50:26 +0000 Subject: [PATCH] feat(spec)!: retire the five keys the lint could never warn about, and connect doc.tags (#4509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the "顺带的三个小清理候选" section of #4509 — the part left over after #4558 landed the four structural disconnects. What groups the five retirements is not the type they sit on but WHY they had to go out in a major rather than after a deprecation cycle: four of the five carry schema DEFAULTS, and a default materialises at parse time, so the liveness advisory lint cannot tell a value the author wrote from one the schema supplied. Marking them would have warned on every mapping and every selector in existence — which is why the ledger recorded `_authorWarnSkipped` instead of `authorWarn`. For a key in that state, removal is not the escalation after a warning; it is the only channel that ever reaches the author. With spec at 17.0.0-rc.1 and pre-mode still open, that channel closes at `changeset pre exit` and reopens in v18. Removed (strict deletion + `guidance` prescriptions, ledger rows deleted): mapping.extractQuery promised an export path no exporter implements mapping.errorPolicy error handling belongs to the import REQUEST mapping.batchSize the write path sizes its own batches app.contextSelectors[].includeAll app.contextSelectors[].placement `includeAll` is the one worth reading twice: not unread but deliberately DISOBEYED, and for a security reason. Context selectors are mandatory-scope, so an "All" row would clear a scope that exists to be scoped — on Studio's package selector that means listing the platform's own system/cloud kernel packages to a developer who scoped to their own package. STUDIO_APP shipped authoring `includeAll: true` against a renderer that ignored it; that authoring site goes with the key here. `batchSize` deliberately offers no rename. bulkActionDef/connector/sync/offline /seed-loader/NoSQL-cursor `batchSize` are all live and enforced, but each is a different key on a different type sizing its own path. "Removed" plus a familiar name one line away is exactly how a dead setting gets laundered into a live-looking one — the same trap datasource.retryPolicy had to defuse against hook/job retryPolicy (which spell the delay `backoffMs`) in #4583. A pin test asserts the message names them as DIFFERENT keys. Retired ALIAS spellings (query, onError, errorHandling, errorMode, batch, chunkSize, skipErrors, showall, location) route to the same prescriptions rather than suggesting a rename onto a key that is also gone. Connected, not removed — doc.tags: `BookGroup.include` has always accepted `{ tag }`, and it could never match a single doc in any stack. Not because the matcher was missing: `matchesInclude` compares `doc.tags`, the book route already forwards `tags: d.tags`, and `ResolverDoc` already declared `tags?: string[]` annotated "(P3d; absent today)". The gap was one line at the AUTHORING end — DocSchema is strict and had no `tags` key, so writing one was a parse error and every doc reached the resolver with tags undefined. ADR-0049 says enforcement wins when the feature exists; removing the variant would also have discarded working matcher code and left authors a bare union error carrying no prescription. ADR-0087: new conversion `mapping-inert-keys-removed` (scoped to the `mappings` collection deliberately — a stack-wide strip would delete an enforced batchSize from connector/sync/bulk-action/offline) plus an extension of `app-dead-authoring-keys-removed` to drill the contextSelectors array; both wired into the protocol-17 D3 chain step. `allValue` was re-verified as its ledger note required: still live (the shell reads it for auto-selection and query-param defaulting), but its describe() no longer calls it "the value emitted when All is selected" — an event that cannot occur and never could. Incidental, from confirming the area gates while working the selector keys: filterAppForUser walks only the top-level `navigation` tree and never reads `item.areas`, so area-level visible/requiredPermissions are FAIL-OPEN, not merely unread. Recorded accurately in the ledger and filed as #4651 rather than fixed here — inventing an authorization mechanism inside a retirement PR is exactly what #4583 declined to do for managed read-only. mapping joins datasource at zero dead keys. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01E5CYr5SDwe85gH2Jr5KSgu --- .../doc-tags-connects-book-tag-include.md | 36 +++++ ...ontext-selector-unwarnable-keys-retired.md | 63 ++++++++ content/docs/references/data/mapping.mdx | 3 - content/docs/references/system/doc.mdx | 1 + content/docs/references/ui/app.mdx | 4 +- docs/protocol-upgrade-guide.md | 5 +- .../utils/lint-liveness-properties.test.ts | 38 +++-- .../platform-objects/src/apps/studio.app.ts | 7 +- packages/spec/authorable-surface.json | 8 +- packages/spec/liveness/README.md | 6 +- packages/spec/liveness/app.json | 18 +-- packages/spec/liveness/doc.json | 6 + packages/spec/liveness/mapping.json | 21 +-- packages/spec/spec-changes.json | 20 ++- packages/spec/src/conversions/registry.ts | 134 +++++++++++++++++- packages/spec/src/data/mapping.test.ts | 92 +++++++----- packages/spec/src/data/mapping.zod.ts | 81 +++++++++-- packages/spec/src/migrations/registry.ts | 23 ++- packages/spec/src/system/book.test.ts | 31 ++++ packages/spec/src/system/book.zod.ts | 2 +- packages/spec/src/system/doc.zod.ts | 19 +++ packages/spec/src/ui/app.test.ts | 50 +++++++ packages/spec/src/ui/app.zod.ts | 65 +++++++-- 23 files changed, 593 insertions(+), 140 deletions(-) create mode 100644 .changeset/doc-tags-connects-book-tag-include.md create mode 100644 .changeset/mapping-context-selector-unwarnable-keys-retired.md diff --git a/.changeset/doc-tags-connects-book-tag-include.md b/.changeset/doc-tags-connects-book-tag-include.md new file mode 100644 index 0000000000..8d32e14236 --- /dev/null +++ b/.changeset/doc-tags-connects-book-tag-include.md @@ -0,0 +1,36 @@ +--- +'@objectstack/spec': minor +--- + +feat(spec): declare `doc.tags`, so a book group's `include: { tag }` can finally match something (#4509) + +`BookGroup.include` has always accepted two shapes — a glob over doc names, or +`{ tag: '' }`. The tag variant could never match a single doc in any stack, +and not because the matcher was missing. Everything downstream already existed: + +- `matchesInclude` compares `doc.tags` against the rule (`book.zod.ts`) +- the book route already forwards `tags: d.tags` into the resolver (`rest-server.ts`) +- `ResolverDoc` already declares `tags?: string[]` — annotated `(P3d; absent today)` + +The gap was one line at the *authoring* end: `DocSchema` is `.strict()` and had +no `tags` key, so writing `tags:` on a doc was a parse error. Every doc therefore +reached the resolver with `tags === undefined`, and the variant matched nothing, +forever. + +This is the enforce half of ADR-0049 enforce-or-remove. Removal was the +alternative and was rejected on two grounds: a union member has no clean +tombstone (`retiredKey` covers object keys), so authors would have received a +bare union error carrying no prescription — and it would have discarded a +working matcher to fix a declaration. + +```ts +defineDoc({ name: 'crm_guide_lead', content: '# Leads', tags: ['tutorial'] }) +defineBook({ name: 'crm', groups: [{ key: 'tut', label: 'Tutorials', include: { tag: 'tutorial' } }] }) +``` + +Prefer a name convention (`include: 'crm_guide_*'`) where one exists — tags earn +their place when membership cuts *across* naming, e.g. a `tutorial` tag spanning +several feature prefixes, which no glob can collect. + +Additive: `DocSchema` previously rejected `tags`, so nothing that parsed before +parses differently now. diff --git a/.changeset/mapping-context-selector-unwarnable-keys-retired.md b/.changeset/mapping-context-selector-unwarnable-keys-retired.md new file mode 100644 index 0000000000..8068f350c7 --- /dev/null +++ b/.changeset/mapping-context-selector-unwarnable-keys-retired.md @@ -0,0 +1,63 @@ +--- +'@objectstack/spec': major +'@objectstack/platform-objects': patch +--- + +feat(spec)!: retire the five keys the advisory lint could never have warned about — mapping `extractQuery`/`errorPolicy`/`batchSize`, contextSelector `includeAll`/`placement` (#4509) + +Five authorable keys parsed, stored, and controlled nothing. What groups them is +not the type they sit on but **why they had to go out in a major rather than +after a deprecation cycle**: four of the five carry schema DEFAULTS, and a +default materialises at parse time — so the liveness advisory lint cannot tell a +value the author wrote from one the schema supplied. Marking them would have +warned on every mapping and every selector in existence, which is why the ledger +recorded them as `_authorWarnSkipped` instead. For a key in that state, removal +is not the escalation after a warning. It is the only channel that ever reaches +the author. + +**The retirement kit:** + +| FROM | TO | Fix | +|---|---|---| +| `mapping.extractQuery` | *(removed)* | Delete the key. Exports run through the ordinary query API (`POST /api/v1/data/:object/query`) — no exporter has ever read a mapping artifact. | +| `mapping.errorPolicy` | *(removed)* | Delete the key. Error handling on the import path belongs to the import REQUEST's own options, not the stored mapping. | +| `mapping.batchSize` | *(removed)* | Delete the key. The write path sizes its own batches. **Do not relocate the value** — see below. | +| `app.contextSelectors[].includeAll` | *(removed)* | Delete the key. Selectors are mandatory-scope; widen `optionsSource.filter` to widen the choices. | +| `app.contextSelectors[].placement` | *(removed)* | Delete the key. Selectors always render in the sidebar header; `'topbar'` placed nothing. | + +Run `os migrate meta --from 16` to rewrite existing sources automatically. + +**`includeAll` is the one worth reading twice.** It was not unread — it was +deliberately *disobeyed*, and for a security reason. A context selector is a +mandatory scope, so an "All" row would clear the scope on a surface that exists +to be scoped; on Studio's package selector that means listing the platform's own +system/cloud kernel packages to a developer who scoped to their own package. The +renderer never offered an All row regardless of the flag, so `includeAll: false` +hardened nothing and `includeAll: true` unlocked nothing. `STUDIO_APP` shipped +authoring `includeAll: true` against a renderer that ignored it — that authoring +site goes with the key in this change. + +**`batchSize` deliberately offers no rename.** `bulkActionDef.batchSize`, +`connector.batchSize`, `sync.batchSize`, `offline.batchSize`, the seed loader's +and the NoSQL driver cursor's are all LIVE and enforced — but each is a +different key on a different type sizing its own path, and none of them sizes a +mapping import. The rejection says so explicitly, because "removed" plus a +familiar name one line away is exactly how a dead setting gets laundered into a +live-looking one. Same trap `datasource.retryPolicy` had to defuse against +`hook`/`job` `retryPolicy` (which spell the delay `backoffMs`) one issue +earlier. + +Both schemas are `.strict()`, so the keys are deleted from the shape and +rejected with a `guidance` prescription rather than tombstoned; their liveness +rows are deleted rather than kept. The retired ALIAS spellings (`query`, +`onError`, `errorHandling`, `errorMode`, `batch`, `chunkSize`, `skipErrors`, +`showall`, `location`) route to the same prescriptions instead of suggesting a +rename onto a key that is also gone. + +Registered as the ADR-0087 D2 conversion `mapping-inert-keys-removed` and an +extension of `app-dead-authoring-keys-removed`, both wired into the protocol-17 +D3 chain step. The mapping conversion is scoped to the `mappings` collection +deliberately — a stack-wide strip would delete an enforced `batchSize` from +connector, sync, bulk-action and offline shapes. + +`datasource` reached zero dead keys in #4583; `mapping` reaches zero here. diff --git a/content/docs/references/data/mapping.mdx b/content/docs/references/data/mapping.mdx index 0db4f71486..0cd86d8712 100644 --- a/content/docs/references/data/mapping.mdx +++ b/content/docs/references/data/mapping.mdx @@ -60,9 +60,6 @@ const result = FieldMappingSchema.parse(data); | **fieldMapping** | `{ source: string \| string[]; target: string \| string[]; transform: Enum<'none' \| 'constant' \| 'lookup' \| 'split' \| 'join' \| 'javascript' \| 'map'>; params?: object }[]` | ✅ | | | **mode** | `Enum<'insert' \| 'update' \| 'upsert'>` | ✅ | | | **upsertKey** | `string[]` | optional | Fields to match for upsert (e.g. email) | -| **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). | diff --git a/content/docs/references/system/doc.mdx b/content/docs/references/system/doc.mdx index be3a3df033..d66c12f937 100644 --- a/content/docs/references/system/doc.mdx +++ b/content/docs/references/system/doc.mdx @@ -67,6 +67,7 @@ const result = DocSchema.parse(data); | **content** | `string` | ✅ | Raw Markdown content (CommonMark + GFM) | | **order** | `number` | optional | Sort key within a book group (ADR-0046 §6) | | **group** | `string` | optional | Explicit book-group key (ADR-0046 §6); rules usually suffice | +| **tags** | `string[]` | optional | Membership tags matched by a book group's `include: { tag }` rule (ADR-0046 §5) | | **translations** | `Record` | optional | Per-locale `{label?,description?,content}` variants; the base doc is the fallback | | **_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. | diff --git a/content/docs/references/ui/app.mdx b/content/docs/references/ui/app.mdx index 6c069797a0..3bd2579dd4 100644 --- a/content/docs/references/ui/app.mdx +++ b/content/docs/references/ui/app.mdx @@ -128,10 +128,8 @@ const result = ActionNavItemSchema.parse(data); | **label** | `string` | ✅ | Dropdown label | | **icon** | `string` | optional | Icon name | | **optionsSource** | `{ endpoint: string; valueKey: string; labelKey: string; filter?: { key: string; op: Enum<'eq' \| 'ne' \| 'in' \| 'nin'>; value: string \| string[] }[] }` | ✅ | Option data source | -| **includeAll** | `boolean` | ✅ | Prepend an "All" option that clears the scope | -| **allValue** | `string` | ✅ | Template value when "All" is selected (empty = no filter) | +| **allValue** | `string` | ✅ | Sentinel value meaning "no concrete selection yet" (empty string is almost always right) | | **persist** | `Enum<'query' \| 'session' \| 'none'>` | ✅ | Persist selection via URL query, sessionStorage, or not at all | -| **placement** | `Enum<'sidebar_header' \| 'topbar'>` | ✅ | Render location in the app chrome | --- diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 23216d5e6b..13d65b6e26 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -162,6 +162,8 @@ The datasource close-out also graduates the four legacy `datasource.config` spel The `script` flow node converges on its one real path (#4343). It had four ways to name what it ran and only one of them ran anything: `config.actionType: 'email' | 'slack'` were logger-backed stubs that wrote a line, reported success and delivered nothing under any configuration — with `config.template` / `.recipients` / `.variables` feeding a message no channel ever sent; inline `config.script` was recognized and never executed (the built-in runtime has no server-side JS sandbox), so the node warned and no-op'd; and every other `actionType` value was shorthand for a registered-function name, a second spelling of `config.function`. All five keys are retired and `function` becomes required, which is also what finally made the contract PARSEABLE: while the legal key set depended on `actionType`, a flat parse would either reject valid shapes or wave everything through, so `script` (with `subflow`) now runs through the same execute-time contract parse #4277 gave the flat builtins. A shorthand `actionType` CONVERTS into `function` — that is what it meant — unless `function` is already set, in which case it was dead metadata the executor never reached. The other four are dropped outright: nothing read them, so there is no value to preserve, and rebuilding the intent is an authoring decision the tombstones prescribe per branch (a `notify` node for mail — it delivers through the messaging service, the in-app inbox by default and real email once `@objectstack/plugin-email` is installed; a `connector_action` with the Slack connector, or an `http` node posting to a webhook, for Slack; a registered function for an inline body). Retired from the load path for the same reason as the rest: absorbing `actionType: 'email'` silently would let an author keep believing the flow sends mail. +Finally, five keys retire because the advisory lint could never have warned about them (#4509): mapping `extractQuery` / `errorPolicy` / `batchSize`, and app `contextSelectors[].includeAll` / `.placement`. Four of the five carry schema DEFAULTS, and a default materialises at parse time — so the liveness lint cannot tell a value the author wrote from one the schema supplied, and marking them would have warned on every mapping and every selector in existence. For a key in that state removal is not the escalation after a warning; it is the only channel that ever reaches the author, which is why they ship inside the 17.0.0 window rather than after a deprecation cycle. What they claimed: `extractQuery` promised an export path no exporter implements (exports go through the ordinary query API); `errorPolicy` offered skip/abort/retry where error handling belongs to the import REQUEST; `batchSize` sized batches the write path sizes itself; `placement` offered a topbar that places nothing. `includeAll` is the one worth reading twice — it was not unread but deliberately DISOBEYED, because context selectors are mandatory-scope and an "All" row would clear the scope: on Studio's package selector that means listing the platform's own system/cloud kernel packages to a developer who scoped to their package. `STUDIO_APP` authored `includeAll: true` against a renderer that ignored it. The mapping prescription for `batchSize` deliberately offers no rename: bulk-action, connector, sync, offline, seed-loader and NoSQL-cursor `batchSize` are all live, but each is a different key sizing its own path — the same trap `datasource.retryPolicy` vs `hook`/`job` `retryPolicy` had to defuse one issue earlier. + ### Mechanical (applied for you) | Conversion | Surface | Change | Load window | @@ -179,7 +181,7 @@ The `script` flow node converges on its one real path (#4343). It had four ways | `flow-node-script-config-aliases` | `flow.node.script.config` | script flow-node config keys 'functionName' → 'function', 'input' → 'inputs' (#3796) | live — protocol 17 loader accepts the old shape | | `permission-rls-priority-removed` | `permission.rowLevelSecurity.priority` | RLS-policy key 'priority' removed (#3896 audit — policies OR-combine, so the promised conflict-resolution semantics cannot exist; dropping it changes no outcome) | retired — `migrate meta` only | | `tool-inert-authoring-keys-removed` | `tool.category / tool.permissions / tool.active / tool.builtIn` | tool keys 'category'/'permissions'/'active'/'builtIn' removed (#3896 close-out — authorable and inert; permissions gated nothing, active:false withdrew nothing) | retired — `migrate meta` only | -| `app-dead-authoring-keys-removed` | `app.version / app.aria / app.objects / app.apis / app.sharing / app.embed / app.mobileNavigation` | app keys 'version'/'aria'/'objects'/'apis'/'sharing'/'embed'/'mobileNavigation' removed (2026-06 liveness audit — never read; sharing/embed declared a public surface no route enforced, mobileNavigation was fully unimplemented) | retired — `migrate meta` only | +| `app-dead-authoring-keys-removed` | `app.version / app.aria / app.objects / app.apis / app.sharing / app.embed / app.mobileNavigation / app.contextSelectors.includeAll / app.contextSelectors.placement` | app keys 'version'/'aria'/'objects'/'apis'/'sharing'/'embed'/'mobileNavigation' plus contextSelectors 'includeAll'/'placement' removed (liveness audits #4001, #4509 — never read; sharing/embed declared a public surface no route enforced, mobileNavigation was fully unimplemented, and includeAll was deliberately disobeyed because an 'All' row would clear a mandatory scope) | retired — `migrate meta` only | | `field-required-notnull-explicit` | `object.fields.*.required / object.fields.*.storage.notNull` | required fields gain explicit 'storage.notNull: true' (ADR-0113 — pre-17 'required' implied the column constraint; post-17 it is only the write contract) | retired — `migrate meta` only | | `action-inert-keys-removed` | `action.shortcut / action.bulkEnabled` | action keys 'shortcut'/'bulkEnabled' removed (#3896 close-out — no keydown path dispatches shortcuts; the multi-select toolbar reads the view's bulkActions) | retired — `migrate meta` only | | `flow-inert-keys-removed` | `flow.active / flow.template / flow.nodes[].outputSchema / flow.errorHandling.fallbackNodeId` | flow keys 'active'/'template', node 'outputSchema' and errorHandling 'fallbackNodeId' removed (#3896 close-out — active:false never stopped a flow; status is the enforced lifecycle) | retired — `migrate meta` only | @@ -192,6 +194,7 @@ The `script` flow node converges on its one real path (#4343). It had four ways | `datasource-read-replicas-removed` | `datasource.readReplicas` | datasource key 'readReplicas' removed (#4468 — no driver opened a replica connection and no query path splits reads from writes; front replicas behind one endpoint and point `config` at it) | retired — `migrate meta` only | | `datasource-capabilities-removed` | `datasource.capabilities` | datasource key 'capabilities' removed (#4583 — eleven flags no code read; pushdown comes from the driver's own supports.*, and `readOnly` never made anything read-only) | retired — `migrate meta` only | | `datasource-inert-blocks-removed` | `datasource.retryPolicy / datasource.healthCheck / datasource.external.label / datasource.external.requirePermission` | datasource keys 'retryPolicy'/'healthCheck' and external 'label'/'requirePermission' removed (#4583 — nothing retried, nothing probed on a schedule, and the federation label/permission were read by nobody) | retired — `migrate meta` only | +| `mapping-inert-keys-removed` | `mapping.extractQuery / mapping.errorPolicy / mapping.batchSize` | mapping keys 'extractQuery'/'errorPolicy'/'batchSize' removed (#4509 — no exporter reads a mapping, error handling belongs to the import request, and the write path sizes its own batches) | retired — `migrate meta` only | | `datasource-config-driver-key-aliases` | `datasource.config` | datasource config keys → canonical per driver: sqlite 'file'/'database' → 'filename', postgres/mysql 'connectionString' → 'url' and 'user' → 'username', mongo 'uri' → 'url' and 'user' → 'username' (#4456 — driver-factory `??` fallback graduation) | retired — `migrate meta` only | | `flow-node-script-branch-keys-removed` | `flow.node.script.config.actionType / flow.node.script.config.template / flow.node.script.config.recipients / flow.node.script.config.variables / flow.node.script.config.script` | script flow-node config keys 'actionType' (→ 'function' when it was shorthand for one; otherwise removed — 'email'/'slack' were logger-backed stubs that delivered nothing), plus 'template' / 'recipients' / 'variables' (fed those stubs) and 'script' (inline JS the runtime never executed) (#4343) | retired — `migrate meta` only | diff --git a/packages/cli/src/utils/lint-liveness-properties.test.ts b/packages/cli/src/utils/lint-liveness-properties.test.ts index 4ba6aaa946..4c83549e3e 100644 --- a/packages/cli/src/utils/lint-liveness-properties.test.ts +++ b/packages/cli/src/utils/lint-liveness-properties.test.ts @@ -323,9 +323,11 @@ describe('lintLivenessProperties', () => { }); // book: both inline translations maps are dead (the doc-level map two files - // over works, which is what makes these read alive); job.id and - // mapping.extractQuery are the other flat dead keys. - it('warns on book/job/mapping dead keys (#4488)', () => { + // over works, which is what makes these read alive); job.id is the other flat + // dead key. `mapping.extractQuery` used to be asserted here too — it was + // REMOVED in 17.0.0 (#4509), so the strict parse owns it now and this + // advisory lint correctly says nothing about it. + it('warns on book/job dead keys (#4488)', () => { const findings = lintLivenessProperties({ books: [{ name: 'crm_guide', @@ -339,25 +341,25 @@ describe('lintLivenessProperties', () => { schedule: { type: 'cron', expression: '0 0 * * *' }, handler: 'syncAll', }], - mappings: [{ - name: 'csv_import_contacts', - targetObject: 'contact', - fieldMapping: [], - extractQuery: { object: 'contact', fields: ['name'] }, - }], }); const msgs = paths(findings); expect(msgs.some((m) => m.includes('`translations`'))).toBe(true); expect(msgs.some((m) => m.includes('groups.translations'))).toBe(true); expect(msgs.some((m) => m.includes('`id`'))).toBe(true); - expect(msgs.some((m) => m.includes('extractQuery'))).toBe(true); }); - // The unwarnable-default rule, negative direction: errorPolicy/batchSize - // (mapping) and includeAll/placement (app selectors) materialize from schema - // defaults on every compiled artifact, so their dead entries carry - // _authorWarnSkipped instead of authorWarn — a compiled stack that only has - // defaults must stay silent. + // The unwarnable-default rule, and what became of it. + // + // mapping `errorPolicy`/`batchSize` and selector `includeAll`/`placement` + // were the four keys this lint structurally COULD NOT warn about: their + // schema defaults materialize on every compiled artifact, so presence never + // implied authorship, and their ledger entries carried `_authorWarnSkipped` + // rather than `authorWarn`. That is precisely why they were REMOVED in 17.0.0 + // (#4509) instead of being warned about first — the strict schemas reject + // them now, which is the only channel that ever reached those authors. + // + // The surviving assertion is the rule itself, not those four keys: a compiled + // artifact carrying only materialized defaults must stay silent. it('stays silent on schema-default values and live-only artifacts (#4488)', () => { const findings = lintLivenessProperties({ mappings: [{ @@ -366,10 +368,8 @@ describe('lintLivenessProperties', () => { fieldMapping: [{ source: 'Total', target: 'total' }], mode: 'upsert', upsertKey: ['external_ref'], - // materialized defaults — must NOT warn: + // materialized default — must NOT warn: sourceFormat: 'csv', - errorPolicy: 'skip', - batchSize: 1000, }], apps: [{ name: 'sales', @@ -379,10 +379,8 @@ describe('lintLivenessProperties', () => { label: 'Region', optionsSource: { endpoint: '/api/v1/regions', valueKey: 'id', labelKey: 'name' }, // materialized defaults — must NOT warn: - includeAll: true, allValue: '', persist: 'query', - placement: 'sidebar_header', }], }], seeds: [], diff --git a/packages/platform-objects/src/apps/studio.app.ts b/packages/platform-objects/src/apps/studio.app.ts index 079b7fc16e..5fbe640dec 100644 --- a/packages/platform-objects/src/apps/studio.app.ts +++ b/packages/platform-objects/src/apps/studio.app.ts @@ -71,10 +71,13 @@ export const STUDIO_APP: AppInput = { labelKey: 'manifest.name', filter: [{ key: 'manifest.scope', op: 'nin', value: ['system', 'cloud'] }], }, - includeAll: true, + // `includeAll` / `placement` retired in 17.0.0 (#4509). Both were + // authored here and neither was read. The shell never offered an "All" + // row — correct for THIS selector, since an All row would undo the + // `filter` above and list the platform's own kernel packages — and + // selectors always render in the sidebar header. allValue: '', persist: 'query', - placement: 'sidebar_header', }, ], navigation: [ diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index c9857043d7..60624d105e 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -1,5 +1,5 @@ { - "description": "Ratchet of every AUTHORABLE key in the spec \u2014 what a metadata author may write, which for this platform IS the third-party API. Auto-updated on additions (commit the change). A key that disappears without a tombstone fails gen:schema, because these schemas are not .strict() and Zod would silently strip it. \"[RETIRED]\" marks a tombstoned key that still rejects with an upgrade prescription. See #3855, ADR-0059 \u00a75.", + "description": "Ratchet of every AUTHORABLE key in the spec — what a metadata author may write, which for this platform IS the third-party API. Auto-updated on additions (commit the change). A key that disappears without a tombstone fails gen:schema, because these schemas are not .strict() and Zod would silently strip it. \"[RETIRED]\" marks a tombstoned key that still rejects with an upgrade prescription. See #3855, ADR-0059 §5.", "keys": [ "ai/AIModelConfig:maxTokens", "ai/AIModelConfig:model", @@ -3580,9 +3580,6 @@ "data/Mapping:_packageId", "data/Mapping:_packageVersion", "data/Mapping:_provenance", - "data/Mapping:batchSize", - "data/Mapping:errorPolicy", - "data/Mapping:extractQuery", "data/Mapping:fieldMapping", "data/Mapping:label", "data/Mapping:mode", @@ -5937,6 +5934,7 @@ "system/Doc:label", "system/Doc:name", "system/Doc:order", + "system/Doc:tags", "system/Doc:translations", "system/EmailAddressConfig:address", "system/EmailAddressConfig:name", @@ -7170,11 +7168,9 @@ "ui/AppContextSelector:allValue", "ui/AppContextSelector:icon", "ui/AppContextSelector:id", - "ui/AppContextSelector:includeAll", "ui/AppContextSelector:label", "ui/AppContextSelector:optionsSource", "ui/AppContextSelector:persist", - "ui/AppContextSelector:placement", "ui/AppearanceConfig:allowedVisualizations", "ui/AppearanceConfig:showDescription", "ui/AriaProps:ariaDescribedBy", diff --git a/packages/spec/liveness/README.md b/packages/spec/liveness/README.md index dddfa26a80..3056c82d51 100644 --- a/packages/spec/liveness/README.md +++ b/packages/spec/liveness/README.md @@ -508,12 +508,12 @@ for t, v in r['types'].items(): | query | 16 | 1 | 4 | – | **not a metadata type** — the REQUEST surface (`QuerySchema`: client SDK QueryBuilder output; the `POST /data/:object/query` body), governed via `SPEC_ONLY_SCHEMAS` (#4286). The gate's one-level walk resolves 1 experimental; the 7 marker-experimental search affordances sit one level deeper, below the walk — resolved from `[EXPERIMENTAL — not enforced]` describe markers, not ledger entries (search `fuzzy`/`operator`/`boost`/`minScore`/`language`/`highlight` + `aggregations[].filter` — declared engine affordances no executor receives). The #4286 sweep closed out same-release: `having` ENFORCED 2026-07-31 (engine-side post-aggregation filter, both paths; was finding 1); dead 4 = the tombstoned removals `joins`/`windowFunctions`/`cursor`/`distinct` — REMOVED 2026-07-31 (retiredKey keeps each in the walked shape so the rows stay; protocol-17 semantic migrations; the JoinNode + WindowFunctionNode clusters and the `QueryBuilder.cursor()`/`.distinct()` producers deleted with their keys; `distinct`'s mis-wired REST count suppression deleted too — finding 2) | | datasource | 30 | 0 | 0 | 0 | seeded 2026-08-01 (#4487) — the **highest dead ratio of any governed type** (20 of 43), and it was ungoverned until now, which is not a coincidence: #4410/#4465/#4481 found six inert keys here by hand, two security-shaped (`schemaMode` left an external DB constructible as `managed` with DDL ungated; `ssl` configured nothing while looking configured). Dead set = `capabilities.*` (all 11 — the engine gates pushdown on the runtime driver's `supports.*` object, a non-overlapping vocabulary), `healthCheck.*` (3 — nothing schedules a datasource probe; the 20 `healthCheck` hits in the repo all belong to the PLUGIN health monitor and other surfaces), `retryPolicy.*` (4 — `retryPolicy` IS enforced on `hook` and `job`, which is what makes this one read alive; the shapes differ), `external.label`, `external.requirePermission`. **`capabilities.readOnly` is the one to know**: it reads as a safety switch, gates nothing, and two shipped prescriptions pointed authors at it until #4487 — `external.allowWrites: false` is the enforced write gate. `config` is a `z.record`, so its per-driver keys sit outside the walk (recorded in the entry's note, not silently skipped) **批 A CLOSED 2026-08-02 (#4583)**: the `capabilities` block — 11 flags, every one dead and authorWarn'd — was REMOVED rather than bridged; pushdown comes from the runtime driver's own `supports.*`, so there was nothing to connect it to. Its rows are deleted (strict-removal route), which is why dead falls 20 → 9. `readOnly` was the reason the audit was worth doing: it read as a safety switch, gated nothing, and had already been MOVED twice toward somewhere it might be enforced (#4410, #4465) — the shipped CRM example called a datasource a read replica on the strength of it while the datasource took writes. Removing it does NOT hand the author a working alternative: `external.allowWrites` only gates FEDERATED datasources, so a managed one has no read-only gate at all (#4584). Remaining 9 = healthCheck ×3 + retryPolicy ×4 + external ×2, batches B/C/D of #4583 **BATCHES B/C/D CLOSED 2026-08-02 — datasource now has ZERO dead properties**, down from the 20 it was seeded with (the highest dead ratio of any governed type). `retryPolicy` ×4 and `healthCheck` ×3 went as whole blocks, `external.label` / `external.requirePermission` as keys. None was bridgeable: each already had a different LIVE mechanism doing the job — the boot policy, the driver handle's on-demand `ping()`/`checkHealth()`, the top-level `label`, and ordinary permission sets + RLS. The `retryPolicy` rejection deliberately refuses to offer a rename: `hook`/`job` retryPolicy ARE enforced but spell the delay `backoffMs`, and that inconsistency is itself the evidence nothing read the datasource one (#4488's sharpest trap) | | webhook | 11 | 0 | 0 | – | **not a registered metadata type** — governed via the gate's spec-only schema override (`SPEC_ONLY_SCHEMAS`), not `getMetadataTypeSchema`; folding it onto the registry is the #3490 reassessment. This row once read 0/1/16 ("the ENTIRE authoring surface is dead", #3461) and both halves of that were CLOSED same-quarter: #3489 built the materializer bridge (authored `webhooks:` entries now land as `sys_webhook` dispatcher rows) and #3494 pruned the aspirational props outright — so the surviving surface is fully live. Kept in the table as the worked example that a dead verdict is a worklist entry, not a tombstone: enforce-or-remove resolved this one by ENFORCING | -| app | 45 | – | 14 | – | seeded 2026-08-01 (#4488). Dead 14 = the seven #4142 `retiredKey` tombstones (version/aria/objects/apis/sharing/embed/mobileNavigation — rows stay while the tombstones hold the keys in the walked shape) + `homePageId` (the landing IS the first nav item; root landing follows `isDefault` routing) + the **fail-open area gates** `areas.visible` / `areas.requiredPermissions` (nothing evaluates them, while the per-ITEM siblings are enforced server- and client-side — the audit's most important app finding, both authorWarn'd) + `areas.order`/`description` + selector `includeAll` (deliberately ignored: selectors are mandatory-scope; an "All" would leak system metadata) and `placement`. Nav walk covers the union's `object` variant; other variants hand-verified live except the `actionDef` dispatch gap (renders, but no shipped shell passes `onAction`) — #4509 | +| app | 45 | – | 12 | – | seeded 2026-08-01 (#4488). Dead 12 = the seven #4142 `retiredKey` tombstones (version/aria/objects/apis/sharing/embed/mobileNavigation — rows stay while the tombstones hold the keys in the walked shape) + `homePageId` (the landing IS the first nav item; root landing follows `isDefault` routing) + the **fail-open area gates** `areas.visible` / `areas.requiredPermissions` — nothing evaluates them while the per-ITEM siblings are enforced server- and client-side; `filterAppForUser` never reads `item.areas` at all (rest-server.ts:1823), so a "hidden" area shows to everyone. Both authorWarn'd, the audit's most important app finding, now tracked for decision as #4651 + `areas.order`/`description`. RETIRED 17.0.0 (#4509, rows deleted — the selector schema is strict): selector `includeAll` (deliberately DISOBEYED, not merely unread — selectors are mandatory-scope and an "All" row would clear the scope, leaking system metadata through Studio's package filter; STUDIO_APP authored it against a renderer that ignored it) and `placement` (no renderer read it; "topbar" placed nothing). Nav walk covers the union's `object` variant; other variants hand-verified live, and the `actionDef` dispatch gap closed in #4509 | | book | 13 | – | 2 | – | seeded 2026-08-01 (#4488). ADR-0046 §6 spine; `audience` is ENFORCED and fail-closed (tree 401/403 + per-doc effective-audience union on both list and tree). Dead 2 = BOTH inline `translations` maps (book-level and per-group): no resolver reads them and the bundle translator doesn't cover `book` — the trap is that `doc.translations` two files over works on every read path. Also recorded: the `include: { tag }` rule variant can never match (DocSchema declares no `tags`) | -| doc | 7 | – | 0 | – | seeded 2026-08-01 (#4488). Fully live: the kernel stores `content` unparsed, but the REST read layer localizes (resolveDocLocale), audience-gates, list-strips `content`, and the book resolver consumes name/label/description/order/group — plus the objectui console portal renders it all. The schema's own "docs are inert data" header describes the kernel, not the type | +| doc | 15 | – | 0 | – | seeded 2026-08-01 (#4488). Fully live: the kernel stores `content` unparsed, but the REST read layer localizes (resolveDocLocale), audience-gates, list-strips `content`, and the book resolver consumes name/label/description/order/group — plus the objectui console portal renders it all. The schema's own "docs are inert data" header describes the kernel, not the type. **`tags` DECLARED in 17.0.0 (#4509)** — the enforce half of enforce-or-remove: the book resolver's `include: { tag }` matcher, the REST transport and `ResolverDoc.tags` all already existed, but DocSchema is strict and had no `tags` key, so authoring one was a parse error and the variant could never match. Live on arrival | | email_template | 21 | 0 | 0 | 0 | this row read 8/–/13/– for one day (seeded 2026-08-01, #4488: "every authorable property is dead", the webhook shape on AUTH mail) and #4509 CLOSED it by ENFORCING — the second worked example, after `webhook`, that a dead verdict is a worklist entry rather than a tombstone. `bootstrapDeclaredEmailTemplates` materializes declared items into the `sys_email_template` rows `sendTemplate` reads, sharing `mapTemplateToRow` with the built-in seeder so the two doors cannot drift, and re-materializes on live metadata writes (`email_template` is `allowRuntimeCreate: true`, so boot-only would have left Studio saves inert). Three breaks had to close, not one: the engine never registered `emailTemplates:` into the registry, built-in seeds masqueraded as `managed_by: admin` and outranked declared templates, and nothing materialized. ADR-0054 proof bound on `subject` (`email-template-materialization`) | | job | 13 | 0 | 3 | 0 | seeded 2026-08-01 (#4488). The file-authored path is fully enforced: all three schedule shapes honored by the adapters, `retryPolicy`/`timeout` enforced since #3494 (this is the retryPolicy the datasource ledger warns about confusing with its dead namesake), `enabled: false` skips scheduling. Dead 3 = `id` (authorWarn — `name` is the identity everywhere) + label/description (docs-kept). The type-level gap CLOSED 2026-08-02 (#4509) by closing the door rather than bridging it: `handler` names a function in the compiled bundle's function table, which a runtime writer cannot name, so `allowRuntimeCreate` **and** `allowOrgOverride` are now false and `*.job.ts` / `defineStack({ jobs })` are the supported doors. The kind stays registered — its file loader is genuinely consumed (ADR-0088 admission test) | -| mapping | 7 | – | 3 | – | seeded 2026-08-01 (#4488). The import half (#2611) is loudly enforced — unsupported transforms/formats are 400s, `mode`/`upsertKey` default the request, the wizard picker renders `label`. Dead 3 = `extractQuery` (authorWarn — "for export only" promises an export path that does not exist) + `errorPolicy`/`batchSize` (dead but UNWARNABLE: their schema defaults materialize at compile, so presence ≠ authored — `_authorWarnSkipped`, the non-boolean instance of the default(true) rule) | +| mapping | 14 | – | 0 | – | seeded 2026-08-01 (#4488) at 8/11 live; **0 dead since #4509** retired the three that were not. The import half (#2611) is loudly enforced — unsupported transforms/formats are 400s, `mode`/`upsertKey` default the request, the wizard picker renders `label`. RETIRED 17.0.0: `extractQuery` (authorWarn — "for export only" promised an export path no exporter implements) + `errorPolicy`/`batchSize`, which were dead AND **unwarnable** (schema defaults materialize at parse, so presence ≠ authored — `_authorWarnSkipped`, the non-boolean instance of the default(true) rule). That unwarnability is why they went out in the 17.0.0 window rather than after a deprecation cycle: removal was the only channel that could ever reach the author. Rows DELETED, not tombstoned — MappingSchema is strict, so the keys left the walked shape | | seed | 5 | – | 0 | – | seeded 2026-08-01 (#4488). Fully live via SeedLoaderService on both doors (boot/per-org replay + runtime-draft publish). `records` is the z.record walk boundary: the keys an author writes are the target object's fields, governed by that object's own definitions — recorded in the entry, not silently skipped | | translation | 10 | – | 1 | – | seeded 2026-08-01 (#4488) — after fixing the walker: the registered schema is a z.preprocess pipe (#3778 retired-dialect guard) whose transform side the unwrap always took, so the type was literally unwalkable. 10 of 11 groups live across spec resolvers, REST localization, objectui client resolvers and plugin-audit (whose composed-key `t()` calls make `messages` easy to mis-verify as dead). Dead 1 = `validationMessages` (authorWarn): nothing resolves it, and #3778's own legacy-key migration table steers `errors:` authors into it — a shipped false signpost, the capabilities.readOnly shape | | validation | 15 | 0 | 3 | 0 | seeded 2026-08-01 (#4488). The ADR-0020 carrier: the evaluator honors active/events/priority/severity/type/condition/message (the zod header's "only reads type/condition/…" prose is STALE — trust the ledger). Dead 3 = label/description/tags, declared governance metadata, kept unmarked. Union walk boundary recorded: only base + `script` keys walked; per-variant keys are governed by the evaluator's tests, not ledger rows. **No longer a registered metadata kind** — #4509 retired it under ADR-0088 (a standalone rule had no object-binding key and every variant is `.strict()`, so it bound to nothing and gated no write; a state machine authored that way saved cleanly and did nothing). The rule VOCABULARY is untouched and fully live via `object.validations[]`, so the ledger keeps governing it through the gate's spec-only override, alongside `webhook` and `query`. The contrast with the two bridges in the same batch is the point: enforce-or-remove picked ENFORCE where the feature existed and only the wiring was missing, and REMOVE where the shape itself could not carry the feature | diff --git a/packages/spec/liveness/app.json b/packages/spec/liveness/app.json index e7c3f985d3..c307c6d478 100644 --- a/packages/spec/liveness/app.json +++ b/packages/spec/liveness/app.json @@ -1,6 +1,6 @@ { "type": "app", - "_note": "AppSchema — the navigation shell, the densest hand-authored surface on the platform. Consumers: the REST read layer's filterAppForUser (packages/rest/src/rest-server.ts:1796-1847 — the SERVER-side authority for app/nav permission + capability gating and ADR-0045 hidden-app visibility), the spec i18n translateApp (i18n-resolver.ts:472), and objectui's shell (@940ba24: app-shell AppSidebar/ConsoleLayout/ContextSelectors, layout NavigationRenderer, console RootLandingRedirect). The #4001/#4142 app step already retired seven dead keys as retiredKey tombstones — they stay in the walked shape, so their rows stay here (tombstone rule, orphans.mts). WALK BOUNDARY (#3095 union rule): `navigation` drills into the union's FIRST member (the `object` variant + base keys); the other variants' payload keys sit outside the walk and were verified by hand — dashboardName (NavigationRenderer.tsx:433), pageName (:435-442), url/target (:462), reportName (:460), componentRef (:464,:644), group `expanded` (:856) all live. The one GAP found there is now CLOSED (#4509, objectui @e8bec83): an `action` item's click dispatches through a host-supplied `onAction` prop that no shipped shell passed, so `actionDef.actionName` reached no dispatcher and every such item dead-clicked. objectui's `useNavActionDispatch` (objectui: packages/app-shell/src/hooks/useNavActionDispatch.ts) resolves the name against `action` metadata and dispatches through the console action runtime, and UnifiedSidebar passes it (objectui: packages/app-shell/src/layout/UnifiedSidebar.tsx:473). A shell that still passes no handler now HIDES action items rather than rendering them dead (objectui: packages/layout/src/NavigationRenderer.tsx:971) — the renderer stops manufacturing the trap. Also note filterAppForUser strips only the TOP-LEVEL `navigation` tree; `areas` trees rely on the client-side per-item gates. Seeded 2026-08-01 (#4488).", + "_note": "AppSchema — the navigation shell, the densest hand-authored surface on the platform. Consumers: the REST read layer's filterAppForUser (packages/rest/src/rest-server.ts:1796-1847 — the SERVER-side authority for app/nav permission + capability gating and ADR-0045 hidden-app visibility), the spec i18n translateApp (i18n-resolver.ts:472), and objectui's shell (@940ba24: app-shell AppSidebar/ConsoleLayout/ContextSelectors, layout NavigationRenderer, console RootLandingRedirect). The #4001/#4142 app step already retired seven dead keys as retiredKey tombstones — they stay in the walked shape, so their rows stay here (tombstone rule, orphans.mts). WALK BOUNDARY (#3095 union rule): `navigation` drills into the union's FIRST member (the `object` variant + base keys); the other variants' payload keys sit outside the walk and were verified by hand — dashboardName (NavigationRenderer.tsx:433), pageName (:435-442), url/target (:462), reportName (:460), componentRef (:464,:644), group `expanded` (:856) all live. The one GAP found there is now CLOSED (#4509, objectui @e8bec83): an `action` item's click dispatches through a host-supplied `onAction` prop that no shipped shell passed, so `actionDef.actionName` reached no dispatcher and every such item dead-clicked. objectui's `useNavActionDispatch` (objectui: packages/app-shell/src/hooks/useNavActionDispatch.ts) resolves the name against `action` metadata and dispatches through the console action runtime, and UnifiedSidebar passes it (objectui: packages/app-shell/src/layout/UnifiedSidebar.tsx:473). A shell that still passes no handler now HIDES action items rather than rendering them dead (objectui: packages/layout/src/NavigationRenderer.tsx:971) — the renderer stops manufacturing the trap. Also note filterAppForUser walks ONLY the top-level `navigation` tree — it never reads `item.areas` at all (rest-server.ts:1823 returns early when `navigation` is absent), and the client area switcher renders every area. So area-level `visible` / `requiredPermissions` are FAIL-OPEN gates, not merely unread: a \"hidden\" or permission-gated area shows to everyone. Confirmed 2026-08-02 while removing the contextSelectors keys; filed as #4651 (enforce-or-remove decision, deliberately NOT taken in #4509 so a retirement PR does not invent an authorization mechanism). Seeded 2026-08-01 (#4488). CONTEXT SELECTORS, 17.0.0 (#4509): `includeAll` and `placement` rows DELETED — AppContextSelectorSchema is strict, so the keys left the walked shape and retained rows would report ORPHAN. Both were unwarnable (schema defaults materialize at parse, so the lint could not tell authored from supplied), which made removal the only channel that could reach an author. `includeAll` was the sharp one: not unread but deliberately DISOBEYED — selectors are mandatory-scope, and an All row would clear the scope, which on Studio's package selector means listing the platform's own system/cloud kernel packages. STUDIO_APP authored `includeAll: true` against a renderer that ignored it, and that authoring site went with the key.", "props": { "name": { "status": "live", @@ -231,29 +231,17 @@ "evidence": "objectui @940ba24: packages/app-shell/src/layout/ContextSelectors.tsx:90-125", "note": "endpoint fetched, valueKey/labelKey dotted-path mapped, `filter` predicates applied per row (rowPasses, :69-71)." }, - "includeAll": { - "status": "dead", - "verifiedAt": "2026-08-01", - "_authorWarnSkipped": "default(true) boolean — the lint cannot tell author-set from schema default, so marking it would warn on every selector.", - "note": "DELIBERATELY ignored by the renderer (ContextSelectors.tsx:242-246): selectors are mandatory-scope — an 'All' row would unscope the surface and, for Studio's package filter, leak system metadata. The renderer never shows an All option regardless of this flag. Candidate for retiredKey removal (the renderer comment is the prescription)." - }, "allValue": { "status": "live", - "verifiedAt": "2026-08-01", + "verifiedAt": "2026-08-02", "evidence": "objectui @940ba24: packages/app-shell/src/layout/ContextSelectors.tsx:177, :199, :246", - "note": "READ, but only as the 'nothing concrete selected' sentinel for auto-selection and query-param defaulting — its documented purpose ('value emitted when All is selected') can never occur because includeAll is ignored (see above). Live by the letter, vestigial by intent; re-verify if includeAll is ever removed." + "note": "RE-VERIFIED 2026-08-02 (#4509) — the re-check the previous note asked for, now that includeAll is gone. Still live, and the reading is unchanged: it is the 'nothing concrete selected' sentinel for auto-selection and query-param defaulting. What changed is the DECLARATION: the describe() no longer calls it 'the value emitted when All is selected', because that event cannot occur and never could — selectors are mandatory-scope and the renderer never offered an All row. Live by the letter AND by the letter now; the vestigial-by-intent caveat retired with includeAll." }, "persist": { "status": "live", "verifiedAt": "2026-08-01", "evidence": "objectui @940ba24: packages/app-shell/src/layout/ContextSelectors.tsx:164", "note": "'none' opts out of persistence; query/session honored." - }, - "placement": { - "status": "dead", - "verifiedAt": "2026-08-01", - "_authorWarnSkipped": "enum with default('sidebar_header') — the default materializes at compile, so presence ≠ authored (same rule as errorPolicy on mapping).", - "note": "no renderer reads it — selectors always render in the sidebar header block (AppSidebar:469-472); 'topbar' places nothing in the topbar." } }, "note": "Drilled because includeAll/placement diverge (dead) from the live core." diff --git a/packages/spec/liveness/doc.json b/packages/spec/liveness/doc.json index 7227e2120d..423b9cffa3 100644 --- a/packages/spec/liveness/doc.json +++ b/packages/spec/liveness/doc.json @@ -43,6 +43,12 @@ "verifiedAt": "2026-08-01", "evidence": "packages/spec/src/system/doc.zod.ts:110, packages/rest/src/rest-server.ts:2996, packages/rest/src/rest-server.ts:3388", "note": "per-locale {label,description,content} variants collapsed by resolveDocLocale on every read path (list, tree corpus, single item); the map itself is stripped from responses. This is the doc's OWN i18n mechanism — the generic bundle translator does not cover `doc`." + }, + "tags": { + "status": "live", + "verifiedAt": "2026-08-02", + "evidence": "packages/spec/src/system/book.zod.ts:213 (matchesInclude tag branch), reached from :255 and :281; transported by packages/rest/src/rest-server.ts:3218", + "note": "DECLARED 2026-08-02 (#4509, ADR-0049) — the enforce half of enforce-or-remove. The consumer, the transport and the resolver-side interface all predated this key: matchesInclude compared `doc.tags` against a group's `include: { tag }`, the book route already forwarded `tags: d.tags`, and ResolverDoc declared `tags?: string[]` marked '(P3d; absent today)'. What was missing was one line HERE — DocSchema is strict, so authoring `tags:` was a parse error, every doc reached the resolver with tags undefined, and the `{ tag }` include variant could never match. Removing the variant was the alternative and was rejected: a union member has no clean tombstone (retiredKey covers object keys), so authors would have gotten a bare union error, and it would have discarded working matcher code. Live on arrival — the branch it feeds is reachable the moment a doc carries a tag." } } } diff --git a/packages/spec/liveness/mapping.json b/packages/spec/liveness/mapping.json index ff7c1202ba..cc19fb51f9 100644 --- a/packages/spec/liveness/mapping.json +++ b/packages/spec/liveness/mapping.json @@ -1,6 +1,6 @@ { "type": "mapping", - "_note": "MappingSchema (#2611 reusable import mapping). Consumers: the REST import path — resolveNamedMapping fetches the artifact by name and validates it against the request (packages/rest/src/import-mapping.ts:60-107), applyMappingToRows runs the fieldMapping pipeline (:115-167), and import-prepare adopts the artifact's mode/upsertKey as request defaults (packages/rest/src/import-prepare.ts:321-326); objectui's ImportWizard offers registered mappings in a saved-mapping picker (@940ba24 packages/plugin-grid/src/ImportWizard.tsx:979). 8 of 11 live. The IMPORT half of the schema is real and loudly enforced (unsupported transforms/formats are 400s, not silent skips — Prime Directive #10); the EXPORT half (`extractQuery`) and the tuning knobs (`errorPolicy`, `batchSize`) have no consumer anywhere. Seeded 2026-08-01 (#4488).", + "_note": "MappingSchema (#2611 reusable import mapping). Consumers: the REST import path — resolveNamedMapping fetches the artifact by name and validates it against the request (packages/rest/src/import-mapping.ts:60-107), applyMappingToRows runs the fieldMapping pipeline (:115-167), and import-prepare adopts the artifact's mode/upsertKey as request defaults (packages/rest/src/import-prepare.ts:321-326); objectui's ImportWizard offers registered mappings in a saved-mapping picker (@940ba24 packages/plugin-grid/src/ImportWizard.tsx:979). Seeded 2026-08-01 (#4488) at 8 of 11 live; 7 of 7 live since #4509 removed the three that were not. The IMPORT half of the schema is real and loudly enforced (unsupported transforms/formats are 400s, not silent skips — Prime Directive #10). What left in 17.0.0: the EXPORT half (`extractQuery` — no exporter reads a mapping artifact at all) and the two tuning knobs (`errorPolicy`, `batchSize` — error handling belongs to the import REQUEST, and the write path sizes its own batches). Rows DELETED rather than kept: MappingSchema is strict, so the keys left the walked shape and a retained row would report ORPHAN. Both knobs were unwarnable (schema defaults materialize at parse, so the lint could not tell authored from supplied) — removal was the only channel that could reach an author, which is why they went out inside the 17.0.0 window rather than after a deprecation cycle.", "props": { "name": { "status": "live", @@ -43,25 +43,6 @@ "verifiedAt": "2026-08-01", "evidence": "packages/rest/src/import-prepare.ts:325", "note": "adopted as the upsert match fields when the request names none." - }, - "extractQuery": { - "status": "dead", - "verifiedAt": "2026-08-01", - "authorWarn": true, - "authorHint": "Delete it. 'Query to run for export only' promises an export path that does not exist — no exporter reads any mapping artifact. Exports run through the ordinary query API; when a mapping-driven export lands, this is where it plugs in, but authoring it today configures nothing.", - "note": "A whole QuerySchema subtree hangs off this optional key; the single flat verdict covers all of it (no consumer reaches any child)." - }, - "errorPolicy": { - "status": "dead", - "verifiedAt": "2026-08-01", - "_authorWarnSkipped": "schema default('skip') materializes at compile (defineMapping parses), so on the compiled stack the lint cannot tell an authored value from the default — a warn here would fire on every mapping artifact. Same reason default(true) booleans are never marked.", - "note": "No import code reads it: error handling on the import path is the request's own options, and 'retry'/'abort' configure nothing. Dead but unwarnable — see _authorWarnSkipped." - }, - "batchSize": { - "status": "dead", - "verifiedAt": "2026-08-01", - "_authorWarnSkipped": "schema default(1000) materializes at compile — same unwarnable shape as errorPolicy.", - "note": "No import code batches by it; the write path sizes its own batches." } } } diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index e2d76d5b65..b04a085a2c 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -147,8 +147,8 @@ "toMajor": 17 }, { - "surface": "app.version / app.aria / app.objects / app.apis / app.sharing / app.embed / app.mobileNavigation", - "to": "app keys 'version'/'aria'/'objects'/'apis'/'sharing'/'embed'/'mobileNavigation' removed (2026-06 liveness audit — never read; sharing/embed declared a public surface no route enforced, mobileNavigation was fully unimplemented)", + "surface": "app.version / app.aria / app.objects / app.apis / app.sharing / app.embed / app.mobileNavigation / app.contextSelectors.includeAll / app.contextSelectors.placement", + "to": "app keys 'version'/'aria'/'objects'/'apis'/'sharing'/'embed'/'mobileNavigation' plus contextSelectors 'includeAll'/'placement' removed (liveness audits #4001, #4509 — never read; sharing/embed declared a public surface no route enforced, mobileNavigation was fully unimplemented, and includeAll was deliberately disobeyed because an 'All' row would clear a mandatory scope)", "conversionId": "app-dead-authoring-keys-removed", "toMajor": 17 }, @@ -224,6 +224,12 @@ "conversionId": "datasource-inert-blocks-removed", "toMajor": 17 }, + { + "surface": "mapping.extractQuery / mapping.errorPolicy / mapping.batchSize", + "to": "mapping keys 'extractQuery'/'errorPolicy'/'batchSize' removed (#4509 — no exporter reads a mapping, error handling belongs to the import request, and the write path sizes its own batches)", + "conversionId": "mapping-inert-keys-removed", + "toMajor": 17 + }, { "surface": "datasource.config", "to": "datasource config keys → canonical per driver: sqlite 'file'/'database' → 'filename', postgres/mysql 'connectionString' → 'url' and 'user' → 'username', mongo 'uri' → 'url' and 'user' → 'username' (#4456 — driver-factory `??` fallback graduation)", @@ -660,8 +666,8 @@ "toMajor": 17 }, { - "surface": "app.version / app.aria / app.objects / app.apis / app.sharing / app.embed / app.mobileNavigation", - "to": "app keys 'version'/'aria'/'objects'/'apis'/'sharing'/'embed'/'mobileNavigation' removed (2026-06 liveness audit — never read; sharing/embed declared a public surface no route enforced, mobileNavigation was fully unimplemented)", + "surface": "app.version / app.aria / app.objects / app.apis / app.sharing / app.embed / app.mobileNavigation / app.contextSelectors.includeAll / app.contextSelectors.placement", + "to": "app keys 'version'/'aria'/'objects'/'apis'/'sharing'/'embed'/'mobileNavigation' plus contextSelectors 'includeAll'/'placement' removed (liveness audits #4001, #4509 — never read; sharing/embed declared a public surface no route enforced, mobileNavigation was fully unimplemented, and includeAll was deliberately disobeyed because an 'All' row would clear a mandatory scope)", "conversionId": "app-dead-authoring-keys-removed", "toMajor": 17 }, @@ -737,6 +743,12 @@ "conversionId": "datasource-inert-blocks-removed", "toMajor": 17 }, + { + "surface": "mapping.extractQuery / mapping.errorPolicy / mapping.batchSize", + "to": "mapping keys 'extractQuery'/'errorPolicy'/'batchSize' removed (#4509 — no exporter reads a mapping, error handling belongs to the import request, and the write path sizes its own batches)", + "conversionId": "mapping-inert-keys-removed", + "toMajor": 17 + }, { "surface": "datasource.config", "to": "datasource config keys → canonical per driver: sqlite 'file'/'database' → 'filename', postgres/mysql 'connectionString' → 'url' and 'user' → 'username', mongo 'uri' → 'url' and 'user' → 'username' (#4456 — driver-factory `??` fallback graduation)", diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index ca85bfcc00..164e99b027 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -1591,16 +1591,59 @@ const flowNodeScriptConfigAliases: MetadataConversion = { * * `retiredFromLoadPath`: the schema tombstones each key (`retiredKey`, tsc * `never` + a parse-time prescription), same posture as its step-17 siblings. + * + * ## Extended by #4509 — the two context-selector keys + * + * `contextSelectors[].includeAll` and `contextSelectors[].placement` join the + * same conversion rather than opening a second `app` entry: both target major + * 17, both are pure deletes on the same collection, and a separate entry would + * have to keep its fixture disjoint from this one's for no gain. + * + * They differ from the seven above in why they went out. Both carried schema + * defaults, which the liveness advisory lint cannot warn on — a default + * materialises at parse time, so an authored value is indistinguishable from a + * supplied one. Removal was the only channel that could reach an author. + * + * `includeAll` was the sharper of the two: not unread but deliberately + * DISOBEYED. Context selectors are mandatory-scope, so the shell never rendered + * an "All" row — on Studio's package selector an All row would undo the + * selector's own `filter` and list the platform's system/cloud kernel packages. + * `STUDIO_APP` authored `includeAll: true` against a renderer that ignored it, + * which is what a flag reading alive while doing nothing looks like from the + * inside. */ const appDeadAuthoringKeysRemoved: MetadataConversion = { id: 'app-dead-authoring-keys-removed', toMajor: 17, retiredFromLoadPath: true, - surface: 'app.version / app.aria / app.objects / app.apis / app.sharing / app.embed / app.mobileNavigation', - summary: "app keys 'version'/'aria'/'objects'/'apis'/'sharing'/'embed'/'mobileNavigation' removed (2026-06 liveness audit — never read; sharing/embed declared a public surface no route enforced, mobileNavigation was fully unimplemented)", + surface: + 'app.version / app.aria / app.objects / app.apis / app.sharing / app.embed / ' + + 'app.mobileNavigation / app.contextSelectors.includeAll / app.contextSelectors.placement', + summary: "app keys 'version'/'aria'/'objects'/'apis'/'sharing'/'embed'/'mobileNavigation' plus contextSelectors 'includeAll'/'placement' removed (liveness audits #4001, #4509 — never read; sharing/embed declared a public surface no route enforced, mobileNavigation was fully unimplemented, and includeAll was deliberately disobeyed because an 'All' row would clear a mandatory scope)", apply(stack, emit) { const RETIRED = ['version', 'aria', 'objects', 'apis', 'sharing', 'embed', 'mobileNavigation']; - return mapCollection(stack, 'apps', (app, path) => stripKeys(app, RETIRED, emit, path)); + const RETIRED_SELECTOR = ['includeAll', 'placement']; + return mapCollection(stack, 'apps', (app, path) => { + const next = stripKeys(app, RETIRED, emit, path); + // `contextSelectors` is an ARRAY one level down, so stripKeys (top-level + // only) cannot reach it. Drill in and copy-on-write at both levels, so an + // app with nothing to strip keeps its identity for change detection. + const selectors = next.contextSelectors; + if (!Array.isArray(selectors)) return next; + let touched = false; + const nextSelectors = selectors.map((sel, i) => { + if (!sel || typeof sel !== 'object' || Array.isArray(sel)) return sel; + const stripped = stripKeys( + sel as Record, + RETIRED_SELECTOR, + emit, + `${path}.contextSelectors[${i}]`, + ); + if (stripped !== sel) touched = true; + return stripped; + }); + return touched ? { ...next, contextSelectors: nextSelectors } : next; + }); }, fixture: { before: { @@ -1611,6 +1654,13 @@ const appDeadAuthoringKeysRemoved: MetadataConversion = { sharing: { enabled: true }, embed: { enabled: true }, mobileNavigation: { mode: 'bottom_nav' }, + contextSelectors: [{ + id: 'active_package', + label: 'Package', + optionsSource: { endpoint: '/api/v1/packages', valueKey: 'id', labelKey: 'name' }, + includeAll: true, + placement: 'sidebar_header', + }], navigation: [{ id: 'nav_home', label: 'Home', type: 'object', objectName: 'account' }], }], }, @@ -1618,10 +1668,17 @@ const appDeadAuthoringKeysRemoved: MetadataConversion = { apps: [{ name: 'portal', label: 'Portal', + contextSelectors: [{ + id: 'active_package', + label: 'Package', + optionsSource: { endpoint: '/api/v1/packages', valueKey: 'id', labelKey: 'name' }, + }], navigation: [{ id: 'nav_home', label: 'Home', type: 'object', objectName: 'account' }], }], }, - expectedNotices: 4, + // Six notices: four top-level keys (`version`, `sharing`, `embed`, + // `mobileNavigation`) plus the two on the single context selector. + expectedNotices: 6, }, }; @@ -2314,6 +2371,74 @@ const datasourceInertBlocksRemoved: MetadataConversion = { }, }; +/** + * `mapping.extractQuery` / `errorPolicy` / `batchSize` removed (protocol 17, + * #4509, ADR-0049). + * + * Three keys on the import/export mapping artifact that parsed, stored, and + * controlled nothing: + * + * - `extractQuery` — "Query to run for export only" against an export path that + * does not exist: no exporter reads a mapping artifact at all. A whole + * `QuerySchema` subtree hung off it, which is what made it read as a designed + * feature rather than an aspiration. + * - `errorPolicy` — `skip` / `abort` / `retry` selecting between three + * behaviours that were one behaviour. Error handling on the import path is + * the import REQUEST's own options; nothing consults the mapping. + * - `batchSize` — the write path sizes its own batches and never asked. + * + * Two of the three were **unwarnable**, which is why they went out now rather + * than after a deprecation window: `errorPolicy` and `batchSize` carried schema + * defaults, and a default materialises at parse time, so the liveness advisory + * lint could not distinguish an authored value from a supplied one + * (`_authorWarnSkipped` in `liveness/mapping.json`). For a key in that state, + * removal is not the escalation after a warning — it is the only channel that + * ever reaches the author. + * + * `batchSize` is also the name most likely to be "fixed" by relocation, so the + * schema prescription says outright that `bulkActionDef` / `connector` / `sync` + * / `offline` / seed-loader / NoSQL-cursor `batchSize` are live and are all + * different keys sizing different paths. Same shape as the `retryPolicy` / + * `backoffMs` trap #4583 had to defuse on `datasource`. + * + * `retiredFromLoadPath`: the schema is `.strict()`, so the keys are gone from + * the shape and rejected with a `guidance` prescription rather than tombstoned. + */ +const mappingInertKeysRemoved: MetadataConversion = { + id: 'mapping-inert-keys-removed', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'mapping.extractQuery / mapping.errorPolicy / mapping.batchSize', + summary: "mapping keys 'extractQuery'/'errorPolicy'/'batchSize' removed (#4509 — no exporter reads a mapping, error handling belongs to the import request, and the write path sizes its own batches)", + apply(stack, emit) { + const RETIRED = ['extractQuery', 'errorPolicy', 'batchSize']; + // Scoped to the `mappings` collection deliberately: `batchSize` is live on + // connector, sync, bulk-action and offline shapes, and a stack-wide strip + // would delete an enforced key from all of them. + return mapCollection(stack, 'mappings', (m, path) => stripKeys(m, RETIRED, emit, path)); + }, + fixture: { + before: { + mappings: [{ + name: 'csv_import_contacts', + targetObject: 'contact', + fieldMapping: [{ source: 'Email', target: 'email' }], + extractQuery: { object: 'contact', fields: ['email'] }, + errorPolicy: 'abort', + batchSize: 500, + }], + }, + after: { + mappings: [{ + name: 'csv_import_contacts', + targetObject: 'contact', + fieldMapping: [{ source: 'Email', target: 'email' }], + }], + }, + expectedNotices: 3, + }, +}; + /** * `datasource.capabilities` removed (protocol 17, #4583). * @@ -2742,6 +2867,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly { { source: 'name', target: 'full_name' } ], mode: 'upsert', - upsertKey: ['email'], - errorPolicy: 'skip', - batchSize: 500 + upsertKey: ['email'] }; expect(() => MappingSchema.parse(fullMapping)).not.toThrow(); @@ -221,8 +219,6 @@ describe('MappingSchema', () => { expect(mapping.sourceFormat).toBe('csv'); expect(mapping.mode).toBe('insert'); - expect(mapping.errorPolicy).toBe('skip'); - expect(mapping.batchSize).toBe(1000); }); it('should accept different source formats', () => { @@ -265,45 +261,69 @@ describe('MappingSchema', () => { expect(mapping.upsertKey).toEqual(['email', 'phone']); }); - it('should accept different error policies', () => { - const policies: Array = ['skip', 'abort', 'retry']; - - policies.forEach(policy => { - const mapping = MappingSchema.parse({ - name: 'test_mapping', - targetObject: 'object', - fieldMapping: [], - errorPolicy: policy - }); - expect(mapping.errorPolicy).toBe(policy); - }); + // ── Retired in 17.0.0 (#4509, ADR-0049) ─────────────────────────────────── + // + // `extractQuery` / `errorPolicy` / `batchSize` parsed and controlled nothing. + // These pin the REJECTION, not just the absence: the schema is strict, so a + // bare "unrecognized key" would already fail the parse — what has to survive + // refactors is that the author is handed the prescription. Two of the three + // were unwarnable (schema defaults materialise at parse, so the liveness lint + // could never distinguish authored from supplied), which made this rejection + // the ONLY channel that reaches them. + + const base = { name: 'test_mapping', targetObject: 'object', fieldMapping: [] }; + + it('rejects the retired `extractQuery` with the export-path prescription', () => { + expect(() => MappingSchema.parse({ + ...base, + extractQuery: { object: 'contact', fields: ['id', 'email'] }, + })).toThrow(/extractQuery.*removed.*17\.0\.0.*export path that does not exist/s); }); - it('should accept custom batch size', () => { - const mapping = MappingSchema.parse({ - name: 'test_mapping', - targetObject: 'object', - fieldMapping: [], - batchSize: 100 - }); + it('rejects the retired `errorPolicy` and points at the import request', () => { + expect(() => MappingSchema.parse({ ...base, errorPolicy: 'abort' })) + .toThrow(/errorPolicy.*removed.*17\.0\.0.*import REQUEST/s); + }); - expect(mapping.batchSize).toBe(100); + it('rejects the retired `batchSize` WITHOUT offering a rename', () => { + // The trap this pins: `batchSize` is a live, enforced key on bulk-action, + // connector, sync, offline, the seed loader and the NoSQL driver cursor. An + // author (or an agent) reading "removed" is one step from relocating the + // value onto one of those, so the message must name them as DIFFERENT keys + // rather than as a migration target. Same shape as the `datasource` + // `retryPolicy` → `hook`/`job` `backoffMs` trap defused in #4583. + const parse = () => MappingSchema.parse({ ...base, batchSize: 100 }); + expect(parse).toThrow(/batchSize.*removed.*17\.0\.0/s); + // NB: matched against the serialised ZodError, where inner quotes arrive + // escaped — so the assertion deliberately avoids the quoted word. + expect(parse).toThrow(/this by relocating the value to a neighbouring/s); + expect(parse).toThrow(/connector\.batchSize|sync\.batchSize/s); }); - it('should accept extractQuery for export', () => { + it('routes the retired ALIAS spellings to the same prescriptions', () => { + // `onError` / `batch` / `query` aliased the three removed keys. Leaving them + // in `aliases` would have answered "did you mean `errorPolicy`?" — a rename + // suggestion pointing at a key that is also gone, i.e. a second rejection. + expect(() => MappingSchema.parse({ ...base, onError: 'abort' })) + .toThrow(/errorPolicy.*removed/s); + expect(() => MappingSchema.parse({ ...base, chunkSize: 100 })) + .toThrow(/batchSize.*removed/s); + expect(() => MappingSchema.parse({ ...base, query: {} })) + .toThrow(/extractQuery.*removed/s); + }); + + it('leaves the surviving mapping surface intact', () => { const mapping = MappingSchema.parse({ - name: 'export_mapping', + ...base, targetObject: 'contact', - fieldMapping: [{ source: 'email', target: 'email' }], - extractQuery: { - object: 'contact', - fields: ['id', 'email', 'name'], - filters: ['status', '=', 'active'] - } + fieldMapping: [{ source: 'Email', target: 'email' }], + mode: 'upsert', + upsertKey: ['email'], }); - - expect(mapping.extractQuery).toBeDefined(); - expect(mapping.extractQuery?.object).toBe('contact'); + expect(mapping).not.toHaveProperty('errorPolicy'); + expect(mapping).not.toHaveProperty('batchSize'); + expect(mapping).not.toHaveProperty('extractQuery'); + expect(mapping.upsertKey).toEqual(['email']); }); it('should handle CSV import mapping', () => { diff --git a/packages/spec/src/data/mapping.zod.ts b/packages/spec/src/data/mapping.zod.ts index 4c08927c9f..3f37e612bc 100644 --- a/packages/spec/src/data/mapping.zod.ts +++ b/packages/spec/src/data/mapping.zod.ts @@ -4,7 +4,8 @@ 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'; +// `QuerySchema` left with `extractQuery` in 17.0.0 (#4509) — a mapping no +// longer carries a query of its own. /** * Shared history for this file (#4001). @@ -19,6 +20,62 @@ 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.'; +/** + * Keys retired from `MappingSchema` in 17.0.0 (#4509, ADR-0049). + * + * All three parsed, stored, and controlled nothing. They are grouped here + * rather than inlined because two of them were **unwarnable**: `errorPolicy` + * and `batchSize` carried schema defaults, and a default materialises at parse + * time, so the liveness lint could not tell an authored value from one the + * schema filled in (`_authorWarnSkipped` in `liveness/mapping.json`). A key the + * advisory lint structurally cannot warn about has exactly one way to become + * audible to its author, and this is it — which is why they went out in the + * 17.0.0 window rather than waiting to be "warned about first". + * + * The old alias spellings (`query`, `onError`, `errorHandling`, `errorMode`, + * `batch`, `chunkSize`, `skipErrors`) are listed too: an author who learned the + * alias should land on the prescription, not on a "did you mean" pointing at a + * key that is also gone. A `guidance` entry suppresses the rename suggestion, + * which is the behaviour we want here. + */ +const RETIRED_EXTRACT_QUERY = + '`mapping.extractQuery` was removed in @objectstack/spec 17.0.0 (#4509, ADR-0049) — no ' + + 'exporter ever read a mapping artifact, so "Query to run for export only" promised an ' + + 'export path that does not exist. Delete the key. Exports run through the ordinary ' + + 'query API (`POST /api/v1/data/:object/query`); if a mapping-driven export is ever ' + + 'designed, this is where it plugs back in. Run `os migrate meta --from 16` to rewrite ' + + 'existing sources automatically.'; + +const RETIRED_ERROR_POLICY = + '`mapping.errorPolicy` was removed in @objectstack/spec 17.0.0 (#4509, ADR-0049) — no ' + + 'import code ever read it, so `skip` / `abort` / `retry` selected between three ' + + 'behaviours that were all the same behaviour. Delete the key. Error handling on the ' + + 'import path belongs to the import REQUEST\'s own options, not to the stored mapping. ' + + 'Run `os migrate meta --from 16` to rewrite existing sources automatically.'; + +const RETIRED_BATCH_SIZE = + '`mapping.batchSize` was removed in @objectstack/spec 17.0.0 (#4509, ADR-0049) — no ' + + 'import code ever batched by it; the write path sizes its own batches. Delete the key. ' + + 'CAREFUL — do NOT "fix" this by relocating the value to a neighbouring `batchSize`: ' + + '`bulkActionDef.batchSize`, `connector.batchSize`, `sync.batchSize`, `offline.batchSize`, ' + + 'the seed loader\'s and the NoSQL driver cursor\'s are all LIVE and enforced — but each is ' + + 'a DIFFERENT key on a different type sizing its own path, and none of them sizes a ' + + 'mapping import. Run `os migrate meta --from 16` to rewrite existing sources ' + + 'automatically.'; + +const MAPPING_RETIRED_KEY_GUIDANCE: Readonly> = { + extractQuery: RETIRED_EXTRACT_QUERY, + query: RETIRED_EXTRACT_QUERY, + errorPolicy: RETIRED_ERROR_POLICY, + onError: RETIRED_ERROR_POLICY, + errorHandling: RETIRED_ERROR_POLICY, + errorMode: RETIRED_ERROR_POLICY, + skipErrors: RETIRED_ERROR_POLICY, + batchSize: RETIRED_BATCH_SIZE, + batch: RETIRED_BATCH_SIZE, + chunkSize: RETIRED_BATCH_SIZE, +}; + /** * Transformation Logic * Built-in helpers for converting data during import. @@ -115,15 +172,18 @@ export const MappingSchema = lazySchema(() => strictObject({ 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', + // NOTE: `query` / `onError` / `errorHandling` / `errorMode` / `batch` / + // `chunkSize` were aliases onto `extractQuery` / `errorPolicy` / + // `batchSize`, all three removed in 17.0.0 (#4509). An alias pointing at a + // key that no longer exists is worse than no alias — it routes the author + // into a second rejection. Their spellings now fall through to the + // `guidance` prescriptions below, which is where the real answer is. }, 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", + ...MAPPING_RETIRED_KEY_GUIDANCE, }, }, { /** Identity */ @@ -141,12 +201,11 @@ export const MappingSchema = lazySchema(() => strictObject({ 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), + // `extractQuery`, `errorPolicy` and `batchSize` were removed in 17.0.0 + // (#4509) — see MAPPING_RETIRED_KEY_GUIDANCE above for what each promised and + // what actually controls it. The live mechanisms: exports go through the + // ordinary query API, and both error handling and batch sizing belong to the + // import request / write path, neither of which consults the mapping. // ADR-0010 — runtime protection envelope (internal — set by the loader). // `mapping` is a registered metadata type, so `MetadataPlugin`'s loader diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index c438eda8bb..170589885e 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -577,7 +577,27 @@ const step17: MigrationStep = { + 'installed; a `connector_action` with the Slack connector, or an `http` node posting to a ' + 'webhook, for Slack; a registered function for an inline body). Retired from the load path ' + 'for the same reason as the rest: absorbing `actionType: \'email\'` silently would let an ' - + 'author keep believing the flow sends mail.', + + 'author keep believing the flow sends mail.\n\n' + + 'Finally, five keys retire because the advisory lint could never have warned about them ' + + '(#4509): mapping `extractQuery` / `errorPolicy` / `batchSize`, and app ' + + '`contextSelectors[].includeAll` / `.placement`. Four of the five carry schema DEFAULTS, ' + + 'and a default materialises at parse time — so the liveness lint cannot tell a value the ' + + 'author wrote from one the schema supplied, and marking them would have warned on every ' + + 'mapping and every selector in existence. For a key in that state removal is not the ' + + 'escalation after a warning; it is the only channel that ever reaches the author, which ' + + 'is why they ship inside the 17.0.0 window rather than after a deprecation cycle. What ' + + 'they claimed: `extractQuery` promised an export path no exporter implements (exports go ' + + 'through the ordinary query API); `errorPolicy` offered skip/abort/retry where error ' + + 'handling belongs to the import REQUEST; `batchSize` sized batches the write path sizes ' + + 'itself; `placement` offered a topbar that places nothing. `includeAll` is the one worth ' + + 'reading twice — it was not unread but deliberately DISOBEYED, because context selectors ' + + 'are mandatory-scope and an "All" row would clear the scope: on Studio\'s package selector ' + + 'that means listing the platform\'s own system/cloud kernel packages to a developer who ' + + 'scoped to their package. `STUDIO_APP` authored `includeAll: true` against a renderer that ' + + 'ignored it. The mapping prescription for `batchSize` deliberately offers no rename: ' + + 'bulk-action, connector, sync, offline, seed-loader and NoSQL-cursor `batchSize` are all ' + + 'live, but each is a different key sizing its own path — the same trap `datasource.' + + 'retryPolicy` vs `hook`/`job` `retryPolicy` had to defuse one issue earlier.', conversionIds: [ 'action-execute-to-target', 'field-conditionalRequired-to-requiredWhen', @@ -603,6 +623,7 @@ const step17: MigrationStep = { 'stack-api-require-auth-removed', 'datasource-capabilities-removed', 'datasource-inert-blocks-removed', + 'mapping-inert-keys-removed', 'flow-node-wait-timeout-keys-removed', 'datasource-read-replicas-removed', 'datasource-config-driver-key-aliases', diff --git a/packages/spec/src/system/book.test.ts b/packages/spec/src/system/book.test.ts index 1eb7a13b55..8ff07fe767 100644 --- a/packages/spec/src/system/book.test.ts +++ b/packages/spec/src/system/book.test.ts @@ -13,6 +13,7 @@ import { type Book, type ResolverDoc, } from './book.zod'; +import { DocSchema } from './doc.zod'; const docs = (names: string[]): ResolverDoc[] => names.map((name) => ({ name })); @@ -53,6 +54,36 @@ describe('resolveBookTree — derived membership (the AI-safety core)', () => { expect(tree.groups[1].entries.map((e) => e.doc)).toEqual(['crm_ref_api']); }); + it('derives membership by TAG include — the variant DocSchema.tags finally makes reachable', () => { + // #4509: the tag branch of `matchesInclude` shipped long before anything + // could feed it. `DocSchema` is strict and had no `tags` key, so authoring + // tags was a parse error, every doc arrived with `tags === undefined`, and + // this variant could not match a single doc in any stack. Declaring + // `DocSchema.tags` was the whole fix — the matcher, the REST transport and + // `ResolverDoc.tags` were all already in place. + const book: Book = { + name: 'crm', + groups: [{ key: 'tut', label: 'Tutorials', include: { tag: 'tutorial' } }], + }; + const tree = resolveBookTree(book, [ + { name: 'crm_guide_lead', tags: ['tutorial'] }, + { name: 'setup_sso', tags: ['tutorial', 'admin'] }, + { name: 'crm_ref_api', tags: ['reference'] }, + { name: 'crm_ref_bare' }, // no tags at all + ]); + // Tags cut ACROSS the naming convention — that is the point of the variant: + // a glob could not have collected these two. + expect(tree.groups[0].entries.map((e) => e.doc)).toEqual(['crm_guide_lead', 'setup_sso']); + }); + + it('accepts `tags` on an authored doc (the strict schema used to reject it)', () => { + expect(() => DocSchema.parse({ + name: 'crm_guide_lead', + content: '# Leads', + tags: ['tutorial', 'crm'], + })).not.toThrow(); + }); + it('a NEW doc matching a rule appears with zero edits to the book (create-and-forget)', () => { const book: Book = { name: 'crm', groups: [{ key: 'guides', label: 'Guides', include: 'crm_guide_*' }] }; const before = resolveBookTree(book, docs(['crm_guide_lead'])); diff --git a/packages/spec/src/system/book.zod.ts b/packages/spec/src/system/book.zod.ts index ee4eba246d..ac2244e52d 100644 --- a/packages/spec/src/system/book.zod.ts +++ b/packages/spec/src/system/book.zod.ts @@ -169,7 +169,7 @@ export interface ResolverDoc { order?: number; /** Explicit placement: the `key` of the group this doc belongs to. */ group?: string; - /** Tags for `include: { tag }` matching (P3d; absent today). */ + /** Tags for `include: { tag }` matching — `DocSchema.tags` (declared #4509). */ tags?: string[]; /** Owning package id (stamped as `_packageId`); used to scope `include`. */ packageId?: string; diff --git a/packages/spec/src/system/doc.zod.ts b/packages/spec/src/system/doc.zod.ts index d0ebc476e2..5db3cf172e 100644 --- a/packages/spec/src/system/doc.zod.ts +++ b/packages/spec/src/system/doc.zod.ts @@ -106,6 +106,25 @@ export const DocSchema = lazySchema(() => strictObject({ */ group: z.string().optional().describe('Explicit book-group key (ADR-0046 §6); rules usually suffice'), + /** + * Membership tags — the operand of a book group's `include: { tag: '' }` + * rule (ADR-0046 §5). + * + * The tag half of `include` has always been implemented on the resolver side + * (`matchesInclude` in `book.zod.ts` compares against these) and the REST book + * route has always forwarded the value, but this schema is `.strict()` and did + * not declare the key — so authoring `tags:` on a doc was a parse error, every + * doc reached the resolver with `tags === undefined`, and `include: { tag }` + * could never match anything. Declared here in 17.0.0 (#4509, ADR-0049): the + * consumer already existed, so this is the enforce half of enforce-or-remove. + * + * Prefer a name convention (`include: "crm_guide_*"`) when one exists — tags + * are for membership that cuts across naming, e.g. a `tutorial` tag spanning + * several feature prefixes. + */ + tags: z.array(z.string()).optional() + .describe('Membership tags matched by a book group\'s `include: { tag }` rule (ADR-0046 §5)'), + /** * Per-locale content variants (ADR-0046 i18n addendum). Compiled from * sibling `..md` files; the base `.md` is the default diff --git a/packages/spec/src/ui/app.test.ts b/packages/spec/src/ui/app.test.ts index 0251865e84..fd59e3bb8b 100644 --- a/packages/spec/src/ui/app.test.ts +++ b/packages/spec/src/ui/app.test.ts @@ -1241,4 +1241,54 @@ describe('unknown keys are rejected, not stripped (#4001 PR B)', () => { })!.message).toContain('`targetGroup` → `group`'); }); }); + + // ── Context-selector keys retired in 17.0.0 (#4509, ADR-0049) ───────────── + describe('retired context-selector keys', () => { + const withSelector = (extra: Record) => ({ + name: 'app_a', + label: 'A', + contextSelectors: [{ + id: 'pkg', label: 'Package', + optionsSource: { endpoint: '/api/v1/packages' }, + ...extra, + }], + }); + + it('rejects `includeAll` with the mandatory-scope reason, not a bare "unknown key"', () => { + // The reason matters more than the rejection here. `includeAll` was not + // merely unread — the shell deliberately disobeyed it, because an "All" + // row clears a scope that exists to be mandatory, and on Studio's package + // selector that means listing the platform's own kernel packages. An + // author who only learns "removed" may go looking for the replacement + // knob; there is none, and this message has to say why. + const msg = unknownKeyIssue(AppSchema, withSelector({ includeAll: true }))!.message; + expect(msg).toMatch(/includeAll.*removed.*17\.0\.0/s); + expect(msg).toMatch(/mandatory|clear the scope/is); + expect(msg).toMatch(/optionsSource\.filter/s); + }); + + it('rejects `placement` and says the topbar never existed', () => { + const msg = unknownKeyIssue(AppSchema, withSelector({ placement: 'topbar' }))!.message; + expect(msg).toMatch(/placement.*removed.*17\.0\.0/s); + expect(msg).toMatch(/sidebar header/s); + }); + + it('routes the retired alias spellings to the same prescriptions', () => { + expect(unknownKeyIssue(AppSchema, withSelector({ showall: true }))!.message) + .toMatch(/includeAll.*removed/s); + expect(unknownKeyIssue(AppSchema, withSelector({ location: 'topbar' }))!.message) + .toMatch(/placement.*removed/s); + }); + + it('keeps `allValue` — it is the no-selection sentinel, not an "All" value', () => { + // The one key whose verdict had to be RE-checked when includeAll went: + // its ledger note asked for exactly that. It survives because the shell + // really does read it (auto-selection + query-param defaulting) — only + // its documented rationale was wrong. + const parsed = AppSchema.parse(withSelector({ allValue: '' })); + expect(parsed.contextSelectors![0].allValue).toBe(''); + expect(parsed.contextSelectors![0]).not.toHaveProperty('includeAll'); + expect(parsed.contextSelectors![0]).not.toHaveProperty('placement'); + }); + }); }); diff --git a/packages/spec/src/ui/app.zod.ts b/packages/spec/src/ui/app.zod.ts index dfcf580436..6116abc153 100644 --- a/packages/spec/src/ui/app.zod.ts +++ b/packages/spec/src/ui/app.zod.ts @@ -696,6 +696,44 @@ export const NavigationAreaSchema = lazySchema(() => z.object({ * params: { type: 'object', package: '{active_package}' } } * ``` */ +/** + * Keys retired from {@link AppContextSelectorSchema} in 17.0.0 (#4509, ADR-0049). + * + * Both carried schema defaults, so the liveness advisory lint could never warn + * on them — a default materialises at parse time, making an authored value + * indistinguishable from one the schema supplied (`_authorWarnSkipped` in + * `liveness/app.json`). Removal was the only channel that could reach an + * author, which is why they went out inside the 17.0.0 window. + * + * `includeAll` is the more important of the two: it was not merely unread, it + * was deliberately DISOBEYED, and for a security reason. + */ +const CONTEXT_SELECTOR_RETIRED_KEY_GUIDANCE: Readonly> = { + includeAll: + '`contextSelectors[].includeAll` was removed in @objectstack/spec 17.0.0 (#4509, ' + + 'ADR-0049) — the shell deliberately ignored it. A context selector is a MANDATORY ' + + 'scope: an "All" row would clear the scope on a surface that exists to be scoped, and ' + + "on Studio's package selector that means listing the platform's own system/cloud " + + 'kernel metadata to a developer who scoped to their own package. The renderer never ' + + 'offered an All row regardless of this flag, so `includeAll: false` hardened nothing ' + + 'and `includeAll: true` unlocked nothing. Delete the key. To widen what a selector ' + + 'offers, widen `optionsSource.filter` instead. Run `os migrate meta --from 16` to ' + + 'rewrite existing sources automatically.', + showall: + '`contextSelectors[].includeAll` (which `showall` aliased) was removed in ' + + '@objectstack/spec 17.0.0 (#4509) — selectors are mandatory-scope and never render an ' + + '"All" row. Delete the key; widen `optionsSource.filter` to widen the choices.', + placement: + '`contextSelectors[].placement` was removed in @objectstack/spec 17.0.0 (#4509, ' + + 'ADR-0049) — no renderer ever read it. Selectors always render in the sidebar header ' + + "block, and `'topbar'` placed nothing in the topbar. Delete the key. Run " + + '`os migrate meta --from 16` to rewrite existing sources automatically.', + location: + '`contextSelectors[].placement` (which `location` aliased) was removed in ' + + '@objectstack/spec 17.0.0 (#4509) — selectors always render in the sidebar header. ' + + 'Delete the key.', +}; + export const AppContextSelectorSchema = lazySchema(() => z.object({ /** * Identifier — also the template-variable name the selected value is @@ -763,24 +801,31 @@ export const AppContextSelectorSchema = lazySchema(() => z.object({ }), }).strict().describe('Option data source'), - /** Whether to prepend an "All" option that clears the scope. */ - includeAll: z.boolean().default(true).describe('Prepend an "All" option that clears the scope'), + // `includeAll` and `placement` were removed in 17.0.0 (#4509) — see + // CONTEXT_SELECTOR_RETIRED_KEY_GUIDANCE above. - /** Value emitted when "All" is selected (empty string = no filter). */ - allValue: z.string().default('').describe('Template value when "All" is selected (empty = no filter)'), + /** + * The "nothing concrete is selected" sentinel. + * + * NOT an "All option" value — there is no All option (see the `includeAll` + * prescription above). This is the value the scope variable holds before the + * user picks a row: the shell auto-selects the first option when this is the + * current value, and omits the query parameter while the selection equals it. + * Empty string is almost always right; set it only if a real option value + * would collide with `''`. + */ + allValue: z.string().default('') + .describe('Sentinel value meaning "no concrete selection yet" (empty string is almost always right)'), /** How the selection is persisted across navigation. */ persist: z.enum(['query', 'session', 'none']).default('query') .describe('Persist selection via URL query, sessionStorage, or not at all'), - - /** Where the dropdown is rendered. */ - placement: z.enum(['sidebar_header', 'topbar']).default('sidebar_header') - .describe('Render location in the app chrome'), }, { error: strictUnknownKeyError({ surface: 'this app context selector', - knownKeys: ['id', 'label', 'icon', 'optionsSource', 'includeAll', 'allValue', 'persist', 'placement'], - aliases: { name: 'id', title: 'label', source: 'optionsSource', options: 'optionsSource', showall: 'includeAll', location: 'placement' }, + knownKeys: ['id', 'label', 'icon', 'optionsSource', 'allValue', 'persist'], + aliases: { name: 'id', title: 'label', source: 'optionsSource', options: 'optionsSource' }, + guidance: CONTEXT_SELECTOR_RETIRED_KEY_GUIDANCE, history: 'Until #4001 these were dropped silently — the selector still parsed, so its scope ' + 'variable behaved differently than declared.',