Skip to content

Commit 5966c2a

Browse files
os-zhuangclaude
andauthored
feat(spec)!: retire the five keys the lint could never warn about, and connect doc.tags (#4509) (#4664)
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. Claude-Session: https://claude.ai/code/session_01E5CYr5SDwe85gH2Jr5KSgu Co-authored-by: Claude <noreply@anthropic.com>
1 parent ce5242c commit 5966c2a

23 files changed

Lines changed: 593 additions & 140 deletions
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
feat(spec): declare `doc.tags`, so a book group's `include: { tag }` can finally match something (#4509)
6+
7+
`BookGroup.include` has always accepted two shapes — a glob over doc names, or
8+
`{ tag: '<t>' }`. The tag variant could never match a single doc in any stack,
9+
and not because the matcher was missing. Everything downstream already existed:
10+
11+
- `matchesInclude` compares `doc.tags` against the rule (`book.zod.ts`)
12+
- the book route already forwards `tags: d.tags` into the resolver (`rest-server.ts`)
13+
- `ResolverDoc` already declares `tags?: string[]` — annotated `(P3d; absent today)`
14+
15+
The gap was one line at the *authoring* end: `DocSchema` is `.strict()` and had
16+
no `tags` key, so writing `tags:` on a doc was a parse error. Every doc therefore
17+
reached the resolver with `tags === undefined`, and the variant matched nothing,
18+
forever.
19+
20+
This is the enforce half of ADR-0049 enforce-or-remove. Removal was the
21+
alternative and was rejected on two grounds: a union member has no clean
22+
tombstone (`retiredKey` covers object keys), so authors would have received a
23+
bare union error carrying no prescription — and it would have discarded a
24+
working matcher to fix a declaration.
25+
26+
```ts
27+
defineDoc({ name: 'crm_guide_lead', content: '# Leads', tags: ['tutorial'] })
28+
defineBook({ name: 'crm', groups: [{ key: 'tut', label: 'Tutorials', include: { tag: 'tutorial' } }] })
29+
```
30+
31+
Prefer a name convention (`include: 'crm_guide_*'`) where one exists — tags earn
32+
their place when membership cuts *across* naming, e.g. a `tutorial` tag spanning
33+
several feature prefixes, which no glob can collect.
34+
35+
Additive: `DocSchema` previously rejected `tags`, so nothing that parsed before
36+
parses differently now.
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
---
2+
'@objectstack/spec': major
3+
'@objectstack/platform-objects': patch
4+
---
5+
6+
feat(spec)!: retire the five keys the advisory lint could never have warned about — mapping `extractQuery`/`errorPolicy`/`batchSize`, contextSelector `includeAll`/`placement` (#4509)
7+
8+
Five authorable keys parsed, stored, and controlled nothing. What groups them is
9+
not the type they sit on but **why they had to go out in a major rather than
10+
after a deprecation cycle**: four of the five carry schema DEFAULTS, and a
11+
default materialises at parse time — so the liveness advisory lint cannot tell a
12+
value the author wrote from one the schema supplied. Marking them would have
13+
warned on every mapping and every selector in existence, which is why the ledger
14+
recorded them as `_authorWarnSkipped` instead. For a key in that state, removal
15+
is not the escalation after a warning. It is the only channel that ever reaches
16+
the author.
17+
18+
**The retirement kit:**
19+
20+
| FROM | TO | Fix |
21+
|---|---|---|
22+
| `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. |
23+
| `mapping.errorPolicy` | *(removed)* | Delete the key. Error handling on the import path belongs to the import REQUEST's own options, not the stored mapping. |
24+
| `mapping.batchSize` | *(removed)* | Delete the key. The write path sizes its own batches. **Do not relocate the value** — see below. |
25+
| `app.contextSelectors[].includeAll` | *(removed)* | Delete the key. Selectors are mandatory-scope; widen `optionsSource.filter` to widen the choices. |
26+
| `app.contextSelectors[].placement` | *(removed)* | Delete the key. Selectors always render in the sidebar header; `'topbar'` placed nothing. |
27+
28+
Run `os migrate meta --from 16` to rewrite existing sources automatically.
29+
30+
**`includeAll` is the one worth reading twice.** It was not unread — it was
31+
deliberately *disobeyed*, and for a security reason. A context selector is a
32+
mandatory scope, so an "All" row would clear the scope on a surface that exists
33+
to be scoped; on Studio's package selector that means listing the platform's own
34+
system/cloud kernel packages to a developer who scoped to their own package. The
35+
renderer never offered an All row regardless of the flag, so `includeAll: false`
36+
hardened nothing and `includeAll: true` unlocked nothing. `STUDIO_APP` shipped
37+
authoring `includeAll: true` against a renderer that ignored it — that authoring
38+
site goes with the key in this change.
39+
40+
**`batchSize` deliberately offers no rename.** `bulkActionDef.batchSize`,
41+
`connector.batchSize`, `sync.batchSize`, `offline.batchSize`, the seed loader's
42+
and the NoSQL driver cursor's are all LIVE and enforced — but each is a
43+
different key on a different type sizing its own path, and none of them sizes a
44+
mapping import. The rejection says so explicitly, because "removed" plus a
45+
familiar name one line away is exactly how a dead setting gets laundered into a
46+
live-looking one. Same trap `datasource.retryPolicy` had to defuse against
47+
`hook`/`job` `retryPolicy` (which spell the delay `backoffMs`) one issue
48+
earlier.
49+
50+
Both schemas are `.strict()`, so the keys are deleted from the shape and
51+
rejected with a `guidance` prescription rather than tombstoned; their liveness
52+
rows are deleted rather than kept. The retired ALIAS spellings (`query`,
53+
`onError`, `errorHandling`, `errorMode`, `batch`, `chunkSize`, `skipErrors`,
54+
`showall`, `location`) route to the same prescriptions instead of suggesting a
55+
rename onto a key that is also gone.
56+
57+
Registered as the ADR-0087 D2 conversion `mapping-inert-keys-removed` and an
58+
extension of `app-dead-authoring-keys-removed`, both wired into the protocol-17
59+
D3 chain step. The mapping conversion is scoped to the `mappings` collection
60+
deliberately — a stack-wide strip would delete an enforced `batchSize` from
61+
connector, sync, bulk-action and offline shapes.
62+
63+
`datasource` reached zero dead keys in #4583; `mapping` reaches zero here.

content/docs/references/data/mapping.mdx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,6 @@ const result = FieldMappingSchema.parse(data);
6060
| **fieldMapping** | `{ source: string \| string[]; target: string \| string[]; transform: Enum<'none' \| 'constant' \| 'lookup' \| 'split' \| 'join' \| 'javascript' \| 'map'>; params?: object }[]` || |
6161
| **mode** | `Enum<'insert' \| 'update' \| 'upsert'>` || |
6262
| **upsertKey** | `string[]` | optional | Fields to match for upsert (e.g. email) |
63-
| **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 |
64-
| **errorPolicy** | `Enum<'skip' \| 'abort' \| 'retry'>` || |
65-
| **batchSize** | `number` || |
6663
| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). |
6764
| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. |
6865
| **_lockSource** | `Enum<'artifact' \| 'package' \| 'env-forced'>` | optional | Layer that set _lock (artifact \| package \| env-forced). |

content/docs/references/system/doc.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ const result = DocSchema.parse(data);
6767
| **content** | `string` || Raw Markdown content (CommonMark + GFM) |
6868
| **order** | `number` | optional | Sort key within a book group (ADR-0046 §6) |
6969
| **group** | `string` | optional | Explicit book-group key (ADR-0046 §6); rules usually suffice |
70+
| **tags** | `string[]` | optional | Membership tags matched by a book group's `include: { tag }` rule (ADR-0046 §5) |
7071
| **translations** | `Record<string, { label?: string; description?: string; content: string }>` | optional | Per-locale `{label?,description?,content}` variants; the base doc is the fallback |
7172
| **_lock** | `Enum<'none' \| 'no-overlay' \| 'no-delete' \| 'full'>` | optional | Item-level lock — controls overlay & delete (ADR-0010). |
7273
| **_lockReason** | `string` | optional | Human-readable reason shown when a write is refused by _lock. |

content/docs/references/ui/app.mdx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,8 @@ const result = ActionNavItemSchema.parse(data);
128128
| **label** | `string` || Dropdown label |
129129
| **icon** | `string` | optional | Icon name |
130130
| **optionsSource** | `{ endpoint: string; valueKey: string; labelKey: string; filter?: { key: string; op: Enum<'eq' \| 'ne' \| 'in' \| 'nin'>; value: string \| string[] }[] }` || Option data source |
131-
| **includeAll** | `boolean` || Prepend an "All" option that clears the scope |
132-
| **allValue** | `string` || Template value when "All" is selected (empty = no filter) |
131+
| **allValue** | `string` || Sentinel value meaning "no concrete selection yet" (empty string is almost always right) |
133132
| **persist** | `Enum<'query' \| 'session' \| 'none'>` || Persist selection via URL query, sessionStorage, or not at all |
134-
| **placement** | `Enum<'sidebar_header' \| 'topbar'>` || Render location in the app chrome |
135133

136134

137135
---

docs/protocol-upgrade-guide.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@ The `script` flow node converges on its one real path (#4343). It had four ways
164164

165165
The same audit reaches the driver contract itself: `IDataDriver.findStream` is removed (#4484). It was REQUIRED — every driver and every test double had to implement it — and documented as the read "optimized for large datasets to avoid memory overflow", while two of its three implementations awaited `find()` for the whole result set and then yielded it row by row, reaching exactly the peak it promised to avoid; the third streamed for real but was the one read in that driver that skipped `buildFindOptions`, so it dropped `query.fields`. Nothing anywhere called it, which is why a contract method could carry an inverted guarantee for this long and why ~20 test doubles could satisfy it by throwing `not implemented`. Paged `find()` is the read that exists and is enforced (its total-order guarantee is checked by the shared pagination-conformance cases); a cursor-based read is worth building when a caller asks for one, which is the honest order. A TS/API surface, never stored — one semantic TODO for driver authors, no source rewrite, and no tombstone: `DriverInterfaceSchema` describes a contract that code IMPLEMENTS and nothing ever `.parse()`d a driver, so tsc is the only channel that could carry the prescription, and it carries it where it matters — at a call site.
166166

167+
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.
168+
167169
### Mechanical (applied for you)
168170

169171
| Conversion | Surface | Change | Load window |
@@ -181,7 +183,7 @@ The same audit reaches the driver contract itself: `IDataDriver.findStream` is r
181183
| `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 |
182184
| `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 |
183185
| `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 |
184-
| `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 |
186+
| `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 |
185187
| `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 |
186188
| `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 |
187189
| `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 |
@@ -194,6 +196,7 @@ The same audit reaches the driver contract itself: `IDataDriver.findStream` is r
194196
| `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 |
195197
| `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 |
196198
| `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 |
199+
| `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 |
197200
| `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 |
198201
| `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 |
199202

0 commit comments

Comments
 (0)