Skip to content

Commit b4705e6

Browse files
committed
Merge origin/main into claude/issue-4661-retry-policy-dual-source
#4664 (spec key retirements + doc.tags) landed on main and touched the same five files. Resolution: - authorable-surface.json / spec-changes.json / docs/protocol-upgrade-guide.md REGENERATED from source (gen:schema / gen:spec-changes / gen:upgrade-guide), never hand-merged — hand-editing the authorable surface is forbidden (#4650). The regenerated surface differs from main by exactly this branch's delta: `automation/RetryPolicy:backoffMs` added, `:retryDelayMs` relabelled [RETIRED], and system/RetryPolicy gaining jitter / maxRetryDelayMs / the tombstone. - conversions/registry.ts auto-merged; verified 40 entries, zero duplicate ids, every declared conversion grouped, both `mappingInertKeysRemoved` (#4664) and `retryPolicyConverged` (#4661) present in the major-17 block. - migrations/registry.ts hand-resolved: both sides appended a paragraph to step17's `rationale` and an entry to `conversionIds`. Kept both. #4664's paragraph ended the string literal, so the concatenation was repaired and this branch's opener reworded ("Finally" -> "The same window") to avoid two "Finally"s in one rationale. Re-verified after the merge that exactly ONE conversion clause still ends in `.retryDelayMs` and it is `retry-policy-converged` — #4664 added five retired leaves (extractQuery / errorPolicy / batchSize / includeAll / placement), none of which collide with this cluster under the #4659 leaf-name match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M9uWvoEp9CoLzYjNExj9sL
2 parents 645e27c + 5966c2a commit b4705e6

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: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,9 @@ 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 it converges the retry policy (#4661). `@objectstack/spec/automation` and `@objectstack/spec/system` each exported a `RetryPolicy`/`RetryPolicySchema` resolving to a DIFFERENT declaration, so which shape a consumer got depended only on the import path (#4411) — yet both computed `delay = base * multiplier^(retry-1)` and both executors implemented that same formula. One declaration now serves both entries with the union of their capabilities, so `job.retryPolicy` gains the `maxRetryDelayMs` ceiling and `jitter` (both enforced in `runWithPolicy`, not merely declared — jitter is what stops a fleet of jobs that failed on one outage from retrying in lockstep). The single authorable casualty is the automation spelling of the base delay: `retryDelayMs``backoffMs`, a pure rename that replays losslessly and is what the already-enforced retry policies (`job.retryPolicy`, `hook.retryPolicy`) call it.
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+
169+
The same window converges the retry policy (#4661). `@objectstack/spec/automation` and `@objectstack/spec/system` each exported a `RetryPolicy`/`RetryPolicySchema` resolving to a DIFFERENT declaration, so which shape a consumer got depended only on the import path (#4411) — yet both computed `delay = base * multiplier^(retry-1)` and both executors implemented that same formula. One declaration now serves both entries with the union of their capabilities, so `job.retryPolicy` gains the `maxRetryDelayMs` ceiling and `jitter` (both enforced in `runWithPolicy`, not merely declared — jitter is what stops a fleet of jobs that failed on one outage from retrying in lockstep). The single authorable casualty is the automation spelling of the base delay: `retryDelayMs``backoffMs`, a pure rename that replays losslessly and is what the already-enforced retry policies (`job.retryPolicy`, `hook.retryPolicy`) call it.
168170

169171
The subtle half is the defaults, and it is worth stating because no gate can see it: `job.retryPolicy` defaulted `maxRetries: 3` / `backoffMultiplier: 2` while the automation shape defaulted 0 / 1, and the authorable-surface gate compares KEY SETS — a changed default is invisible to it, to the tombstone mechanism and to `spec_changes` alike. The merged declaration takes 0 / 1 (retry replays side effects, so it is opt-in — the same reading already recorded in `flow-retry-max-retries-required`), and the conversion writes the pre-17 numbers into every existing `job.retryPolicy` that omitted them. Deployed stacks therefore keep their exact behaviour; what changes is only what a NEWLY authored omission means.
170172

@@ -185,7 +187,7 @@ The subtle half is the defaults, and it is worth stating because no gate can see
185187
| `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 |
186188
| `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 |
187189
| `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 |
188-
| `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 |
190+
| `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 |
189191
| `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 |
190192
| `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 |
191193
| `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 |
@@ -198,6 +200,7 @@ The subtle half is the defaults, and it is worth stating because no gate can see
198200
| `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 |
199201
| `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 |
200202
| `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 |
203+
| `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 |
201204
| `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 |
202205
| `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 |
203206
| `retry-policy-converged` | `flow.node.config.retry.retryDelayMs / job.retryPolicy.maxRetries / job.retryPolicy.backoffMultiplier` | retry policy unified across job.retryPolicy and try_catch retry: base delay 'retryDelayMs' → 'backoffMs', and the pre-17 job defaults (maxRetries 3, backoffMultiplier 2) written out explicitly now that the merged default is 0 / 1 (#4661) | live — protocol 17 loader accepts the old shape |

0 commit comments

Comments
 (0)