Skip to content

Commit 8640f39

Browse files
committed
fix(cli): keep the #6860 allowlist pin's oracle honest — an unrecognized spelling is not a driver kind
#6345's CLI-side refusal of an explicitly-named unknown driver reused UnsupportedDriverError with the operator's RAW TOKEN in driverType. The #6860 pin uses resolveStorageDefinition as its oracle and reads driverType out of that error, so its deliberately over-broad candidate scan started reporting every lowercase literal in storage-driver.ts ('safe', 'on-disconnect', 'factory', 'string', ...) as a driver kind. The allowlist itself was already correct: #6860 landed the canonical seven (sqlite, sqlite-wasm, turso, postgres, mysql, mongodb, memory), mongodb included. start.ts and dev.ts are therefore untouched. UnsupportedDriverError now carries 'recognized', defaulting true so the pre-#6345 turso-with-no-URL call sites keep their meaning, and the pin returns null for the unrecognized case. The assertion is unchanged: both sides still derived, still required to be equal. Also regenerates spec-changes.json / protocol-upgrade-guide.md, which the merge brought stale (os-regen guard).
1 parent 6a71bd8 commit 8640f39

4 files changed

Lines changed: 75 additions & 2 deletions

File tree

‎docs/protocol-upgrade-guide.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,8 @@ The last of the #4001 enforce-or-remove batch lands on two more `ui/` files (#50
220220

221221
Last, it reconciles the SDUI component-props surface with the renderers that serve it (#5775). #5068 wired the first parse `ComponentPropsMap` ever had, and the corpus it landed on diverged in BOTH directions: keys objectui honours that the schema never declared, and keys the schema declared — one of them REQUIRED — that no renderer reads. The maintainer ruled direction A (2026-08-06), the #5611 rule again: the delivered and authorized shape is the contract. So the honoured keys are declared (`element:record_picker` `labelField`/`valueField`/`label`/`emptyText`, `record:path` `stages[].terminal`, `page:tabs` `items[].value`/`items[].count`, `page:card` `children`, and `children` on `page:section`/`page:footer`/`page:sidebar`, which were declared `EmptyProps` while their renderers rendered a child list), and four keys retire. Two are synonym renames: `element:record_picker.displayField` → `labelField` (the required key no renderer read, while `labelField ?? 'name'` is what actually renders the row — so an author who followed the schema got a picker listing `name` with no diagnostic, the ADR-0078 shape), and `page:card.body` → `children` (one composition key across every container; the card renderer already reads both, and the showcase authors `children`). Two are enforce-or-remove deletions: `element:record_picker.searchFields` and `.multiple` — the control is a shadcn single-select with no search input, binding ONE record id into a page variable, so `searchFields` narrowed nothing and `multiple: true` selected nothing extra while reporting success. Either returns the day the capability is implemented (#5021 / #4988). Not in scope, and deliberately: `page:card.visible` is a component-level visibility predicate written into `properties` and hoisted by the renderer — a page to rewrite onto the ADR-0089 `visibleWhen`, not a key to declare.
222222

223+
That count turned out to be incomplete, and #6776 finishes it: five more keys the renderers read were still undeclared. Four are plain additions with no behaviour change (`page:header` `recordChrome`/`showStar`/`showCopyId`, which select between the record-chip header and the bare heading a dashboard wants, and `page:accordion.variant`, which decides whether the accordion draws its own dividers or leaves the border to each panel). The fifth is a rename, and the only one in the family whose defect is structural rather than an oversight: the tab strip's visual style was declared as `page:tabs.type`, which collides with the page component's OWN dispatch key. objectui's `SchemaRenderer` refuses to hoist `properties.type` for exactly that reason, `sdui-parser`'s `BASE_PROPS` contains `type` and skips it before any validation runs, and in a flat or JSX carrier the node reads `{ type: 'page:tabs', … }` so the name is already taken. The key was therefore unauthorable in every carrier but the nested `properties` object, and unvalidated even there. It becomes `tabStyle` — the spelling objectui publishes and the renderer already reads first in the flat carriers — which is `displayField` → `labelField` again: converge on the spelling that works, not the one that declares well, and keep one spelling rather than two (Prime Directive #12).
224+
223225
Finally it narrows the aggregation vocabulary: `array_agg` and `string_agg` leave `AggregationFunction` (#6188, ADR-0049). The enum declared eight functions and the SQL family compiles five — `SqlDriver.mapAggregateFunc` and the Turso `RemoteTransport.aggregate` each lower `count`/`sum`/`avg`/`min`/`max` and route the rest to one refusal — so three were declared-but-unenforced against the backends this platform targets. What makes these two worse than an ordinary inert declaration is that another package had to carry a denylist for them: `service-analytics` subtracted `array_agg` and `string_agg` by name in `UNSUPPORTED_AGGREGATES`, because without that subtraction they reached the Cube strategy's `default` and returned `COUNT(*)` — a row count in place of the requested value, with no error and no log. The maintainer SPLIT the three rather than retiring them as a block (2026-08-07), and the split is the point: `count_distinct` STAYS and takes the enforce leg — one portable lowering (`COUNT(DISTINCT x)`), a dashboard staple, already lowered by `service-analytics` — with its SQL implementation following on its own card, so that declaration leads its implementation by decision rather than by drift. These two take the remove leg: display conveniences with no measured pull, and `string_agg` never had one shape to lower to (the delimiter is a second argument in PostgreSQL, a `SEPARATOR` clause in MySQL, a differently named function in SQL Server). This is an enum VALUE, not a key, so — as with `crypto.hash` above — there is no `retiredKey()` tombstone: the enum error map carries the prescription, keyed on the received value so only the two spellings that used to be legal are told they "were removed". Of the two authoring surfaces only one is stored metadata: the conversion rewrites `dataset.measures[].aggregate`, dropping the measure outright (a measure with neither `aggregate` nor `derived` fails the dataset's own refinement, so stripping just the key would emit an item that cannot parse) plus any derived measure the drop strands, with a notice each. Nothing is lost: `compileDataset` refused both by name already, so such a measure never produced a number. `QueryAST.aggregations[].function` is a request surface with no stored source — one semantic TODO below. The mongodb and in-memory backends that implemented these two are inside the #5499 freeze and are untouched; their code is simply no longer reachable through a spec-valid request.
224226

225227
One entry in this step is not a removal at all but a SECURE-DEFAULT FLIP, the shape protocol 12 last used for `api.requireAuth`: an omitted `ActionDescriptor.resumeAuthority` resolves to `'service'` instead of `'any'`, so a pausing node type that never states who may continue its pauses is refused on the generic resume route rather than open to it (#5561, ADR-0044's 2026-07-28 amendment). Nothing is removed and no metadata shape changes — the field has been optional since step one of the same issue — so tsc reports nothing and only the MEANING of silence moved. That is exactly why it needs a ledger entry: a third-party plugin author has no compile error to discover it with, and the one-line prescription (declare `resumeAuthority` on the descriptor) has to arrive before a user meets a run that will not continue.
@@ -281,6 +283,7 @@ The same descriptor loses a key in this step, and the pairing is the point (#674
281283
| `record-picker-inert-keys-removed` | `page.component.element:record_picker.searchFields / page.component.element:record_picker.multiple` | record-picker component props 'searchFields'/'multiple' removed (#5775 — the control is a plain single-select with no search box; neither key had a reader) | retired — `migrate meta` only |
282284
| `page-card-body-to-children` | `page.component.page:card.body` | page:card component prop 'body' → 'children' (#5775 — one composition key across every container; the card renderer already reads both) | retired — `migrate meta` only |
283285
| `inline-action-api-params-to-body-extra` | `page.component.element:button.action.params` | inline type:'api' action prop 'params' (object form) → 'bodyExtra' (#5777 — the payload gets its own key; `params` stays the ActionParam[] definition array) | live — protocol 17 loader accepts the old shape |
286+
| `page-tabs-type-to-tab-style` | `page.component.page:tabs.type` | page:tabs component prop 'type' → 'tabStyle' (#6776 — a props key named `type` collides with the node's dispatch key and is unauthorable in flat/JSX carriers; `tabStyle` is the spelling the renderer reads in all of them) | retired — `migrate meta` only |
284287

285288
### Semantic (delegated to you, with acceptance criteria)
286289

@@ -404,6 +407,9 @@ The same descriptor loses a key in this step, and the pairing is the point (#674
404407
- **`action-descriptor-is-async-retired`** — `ActionDescriptor.isAsync (the descriptor an executor publishes via `registerNodeExecutor` / `defineActionDescriptor`)` → nothing to re-declare — delete the key. Suspension is `execute()` RETURNING `suspend: true`, and permission to suspend is `supportsPause: true` on the same descriptor (with the `resumeAuthority` its pauses need)
405408
- Why not automatic: ADR-0049 enforce-or-remove. `isAsync` declared "this action suspends the flow awaiting an external reply" and NOTHING read it: a fresh three-repo measurement (#6748, re-run at pickup) found zero property reads across objectstack, objectui and cloud — every hit was the declaration itself, a generated baseline, one of five shipped descriptors WRITING it, a test fixture pinning the shape, or prose. So declaring it never made a node suspend and omitting it never stopped one, which is the silently-inert declaration ADR-0049 exists to end. It was always a second, weaker spelling of the capability `supportsPause` states, and the two diverged in exactly the way a duplicated declaration does: `screen` declared both, `map` and `wait` declared `isAsync` alongside `supportsPause`, and nothing anywhere reconciled them. The sibling took the ENFORCE leg of the same ruling in #6667 — `AutomationEngine` now refuses a suspension whose type does not declare `supportsPause: true` — so the capability this key gestured at is now a real, enforced fact under one name. This one had no consumer to grow into and takes the remove leg. Why D3 semantic and not a D2 conversion: an ActionDescriptor is published from an executor's TypeScript, never stored in stack metadata — no stack, example or template carries the key — so there is no source for the chain to rewrite and `os migrate meta` cannot reach it. The schema tombstones it via `retiredKey()` and descriptor authors delete the key themselves; that rejection (a `tsc` error at the authoring site, and a parse error inside `defineActionDescriptor`) is the channel a third-party plugin author actually meets. The `EnhancedApiError.fieldErrors` disposition, one layer down.
406409
- Done when: No descriptor declares `isAsync` — not the five that shipped it (`screen`, `map`, `wait`, `approval`, `approval_revise`), not a plugin's. Every node type that returns `suspend: true` from `execute()` declares `supportsPause: true` on its descriptor together with a `resumeAuthority`, and its runs still pause and resume as before: the behaviour never depended on `isAsync`, so deleting the key changes no run. Authoring `isAsync` fails `tsc` at the descriptor literal and fails `defineActionDescriptor()` at runtime with the prescription, instead of parsing clean and being stripped.
410+
- **`notification-list-cursor-retired`** — `api.listNotifications cursor — the key on BOTH halves of GET /api/v1/notifications (ListNotificationsRequestSchema and ListNotificationsResponseSchema) and the cursor argument of the client SDK call client.notifications.list(). The same entry covers the limit default: the request schema no longer declares default(20)` → a larger `limit` — the route answers the newest N notifications and has no page 2. There is no replacement for `cursor`, deliberately: nothing ever minted one, so no caller holds a value to carry over. Callers that looped on it were re-reading the first window and should read one window sized to what they display (the Console bell polls exactly this way). For the removed `limit` default, send the number you want explicitly if you were relying on 20 — omitting it takes the server window, which is 50 on the platform inbox and clamped into 1..200, and has been since before the declaration existed
411+
- Why not automatic: One capability, both halves, never half-deleted (maintainer ruling 2026-08-07, Option A, ruled jointly with #6363). `cursor` was declared on the request and on the response and honoured on neither: the dispatcher domain reads `read` / `type` / `limit` and nothing else, and no emit site has ever written the response key. It was worse than inert because it had a shipped PRODUCER — the SDK appended it to the query string — so a caller paginating by the published contract looped on page 1 forever, with no error and no 400. Measured over a real boot with 60 unread before the removal: page2 === page1, both parsing green against the response schema, which is why no conformance gate could see it. This is `data.query.cursor` (#4286, `query-cursor-retired`) one layer up, with the same verdict for the same reason, down to deleting the SDK producer alongside the key. A first-class inbox cursor, if one is ever designed, will be a response-minted opaque token — a different API — so keeping this one preserved a wrong design rather than a roadmap. The `limit` default goes with it because the FICTION WAS THE MECHANISM, not the number: no request path parses a query string through this schema (#3899 wired the catalog's requestSchema to the real entry for BODIES only), so `.default(20)` never stamped anything onto anything, and the server has always applied its own 50. Re-spelling 20 as 50 — the other arm the ruling allowed — would have kept a declaration that does not execute and merely made it coincide with the implementation until someone moved the clamp; `.optional()` plus prose is true about both the schema and the server. No constraint (`.int()` / `.max(200)`) is declared either, because the service CLAMPS an out-of-range limit rather than refusing it, and declaring a rejection the wire does not perform is the same defect mirrored. Route 2, and the split is worth stating exactly because the two halves of the bookkeeping go different ways. There IS a tombstone: both schemas are non-strict, so a bare deletion would have made Zod SILENTLY STRIP whatever a caller kept sending — a clean parse and a parameter that never takes effect, which is this issue's own defect re-created one layer down (#3733, ADR-0104). So `cursor` is `retiredKey()` on both halves, typed `never` for tsc and raising the prescription at any parse, and both keys are registered in RETIRED_KEYS_BY_MAJOR[17]. There is NO D2 conversion: a conversion rewrites an authored source or a stored `sys_metadata` row, and these two shapes are HTTP-only — nobody authors a `ListNotificationsRequest` and nothing persists one. Request AND response shapes: two semantic TODOs for API callers, no stack conversion — the same disposition `BatchOptions.validateOnly` (#4052) and the `AnalyticsQueryRequest` envelope keys already take in this major. The `limit` default is declared separately and mechanically, in DEFAULT_CHANGES_BY_MAJOR[17] (#4666), whose `from`/`to` fingerprints are re-derived on every build. ADR-0049 / ADR-0078, #6361.
412+
- Done when: No caller sends `cursor` to `GET /api/v1/notifications` and no SDK call site passes it: `client.notifications.list({ cursor })` is a `tsc` error (TS2353, excess property), which is the enforced channel — the removal is loud at compile time for every TypeScript consumer. Reading `response.cursor` no longer type-checks either, and always answered `undefined` before. ⚠️ Behaviour on the wire is deliberately UNCHANGED and must be verified as such: a request still carrying `?cursor=…` is IGNORED, not refused — the domain reads three named query keys and no route validates this query against a schema, so an unknown key has never produced a 400 and does not start doing so here. The declaration stopped promising what the wire never did; the wire did not change. `unreadCount` is untouched (#6363) and still reports the total across the whole matching inbox rather than the window. A caller that omitted `limit` receives the same 50 rows it always received.
407413

408414
---
409415

‎packages/cli/src/commands/database-driver-allowlist.pin.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,17 @@ function candidateTokens(): string[] {
100100
* today that is `turso` with no URL, which throws `UnsupportedDriverError`. A URL
101101
* is supplied so it resolves normally; the catch is kept so the derivation
102102
* survives another kind growing the same "recognized but unusable" shape.
103+
*
104+
* `err.recognized` is what keeps that catch honest (#6345). The resolver now
105+
* ALSO throws `UnsupportedDriverError` for a spelling nothing claims — the CLI
106+
* half of "both hosts refuse the same input", which replaced a silent fall-through
107+
* to the dev SQLite default. Reading `driverType` off that error would report the
108+
* operator's raw token as a driver kind, and since the candidate net below is a
109+
* deliberately over-broad scan of every lowercase literal in `storage-driver.ts`,
110+
* the derived set would have grown `safe`, `on-disconnect`, `factory`, `string`
111+
* and the rest — a set that no allowlist could ever equal. The distinction is
112+
* carried on the error rather than re-derived here, so this file keeps asking the
113+
* RESOLVER what a token means instead of growing its own opinion.
103114
*/
104115
function canonicalDriverIdOf(token: string): string | null {
105116
try {
@@ -109,7 +120,7 @@ function canonicalDriverIdOf(token: string): string | null {
109120
});
110121
return resolution?.driverId ?? null;
111122
} catch (err) {
112-
if (err instanceof UnsupportedDriverError) return err.driverType;
123+
if (err instanceof UnsupportedDriverError) return err.recognized ? err.driverType : null;
113124
throw err;
114125
}
115126
}

‎packages/cli/src/utils/storage-driver.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,38 @@ function missingUrlMessage(kind: BuiltinDriverId): string {
139139
* silently became SQLite-in-memory).
140140
*/
141141
export class UnsupportedDriverError extends Error {
142+
/**
143+
* The selection that was refused.
144+
*
145+
* Read {@link recognized} before treating this as a driver id: for the
146+
* unknown-spelling case it is the operator's raw token (`sqlite3`), NOT a
147+
* canonical kind.
148+
*/
142149
readonly driverType: string;
143-
constructor(driverType: string, message: string) {
150+
/**
151+
* Was the refused selection a driver this CLI KNOWS (`turso` with no URL), or
152+
* a spelling nothing claims (`--database-driver sqlite3`)?
153+
*
154+
* Both are fatal and both are this class — `serve.ts` re-throws on the type,
155+
* and an operator needs the same "stop, do not fall back to SQLite" outcome
156+
* either way. But they are not the same fact, and a consumer asking "which
157+
* driver kinds exist" must not read an unrecognized token as one.
158+
*
159+
* That consumer is real: `commands/database-driver-allowlist.pin.test.ts`
160+
* (#6860) derives the canonical kinds by using {@link resolveStorageDefinition}
161+
* as its oracle and reading `driverType` out of this error. When #6345 taught
162+
* the resolver to refuse unknown spellings too, that oracle started reporting
163+
* every stray string literal in this file (`safe`, `on-disconnect`, `factory`)
164+
* as a driver kind. This flag is what keeps the two answers apart.
165+
*/
166+
readonly recognized: boolean;
167+
constructor(driverType: string, message: string, opts: { recognized?: boolean } = {}) {
144168
super(message);
145169
this.name = 'UnsupportedDriverError';
146170
this.driverType = driverType;
171+
// Defaults to `true` so the pre-#6345 call sites (turso with no URL) keep
172+
// their meaning without restating it.
173+
this.recognized = opts.recognized ?? true;
147174
}
148175
}
149176

@@ -310,6 +337,9 @@ export function resolveStorageDefinition(
310337
+ 'Booting on the SQLite default instead would silently ignore the driver you asked for '
311338
+ 'and write into a local database (#3276). Fix the value, or leave the driver unset to '
312339
+ 'let the database URL scheme select it.',
340+
// NOT a driver kind — `driverType` here is the operator's raw token, and a
341+
// caller enumerating kinds must not count it as one.
342+
{ recognized: false },
313343
);
314344
}
315345

0 commit comments

Comments
 (0)