From dfc181b6a15a6c14ebfa47fd8b1a04edb8506cb3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 01:50:30 +0000 Subject: [PATCH 1/2] refactor(spec,client)!: retire the notification-list `cursor` on both halves and drop the fictional `limit` default (#6361) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GET /api/v1/notifications` declared `cursor` on the request AND the response schema and honoured it on neither: the dispatcher domain reads `read`/`type`/ `limit` and nothing else, and no emit site ever wrote the response key. The SDK appended it to the query string, so a caller paginating by the published contract re-read the first window forever — no error, no 400, both pages parsing green against the response schema. Maintainer ruling 2026-08-07 (Option A), ruled jointly with #6363: one capability's two halves are never half-deleted. - `cursor` is tombstoned via `retiredKey()` on BOTH schemas — not deleted. Neither is `.strict()`, so a bare deletion would have made Zod silently strip what callers keep sending, re-creating this very defect one layer down (#3733, ADR-0104). Registered in RETIRED_KEYS_BY_MAJOR[17]. - The `cursor` parameter is removed from `client.notifications.list()`; passing it is now TS2353. - `limit` loses its `.default(20)` rather than being re-spelled 50: nothing parses this query through the schema (#3899 wired requestSchema for bodies only), so the default never executed at any value. Declared in DEFAULT_CHANGES_BY_MAJOR[17] — this table's first entry. - ADR-0087 D3 entry `notification-list-cursor-retired`; no D2 conversion (both shapes are HTTP-only — nothing authors or stores one). Wire behaviour is deliberately UNCHANGED: `?cursor=` is still ignored, not refused, and an omitted `limit` still yields the server's 50-row window. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M --- .../notification-list-cursor-retired.md | 74 +++++++++++++++ content/docs/references/api/protocol.mdx | 8 +- docs/protocol-upgrade-guide.md | 3 + packages/client/src/client.test.ts | 23 +++++ packages/client/src/index.ts | 12 ++- ...ion-schema-conformance.integration.test.ts | 70 +++++++++++---- .../notification-schema-conformance.test.ts | 65 +++++++++++--- packages/spec/authorable-defaults/api.json | 1 - packages/spec/authorable-surface/api.json | 4 +- packages/spec/scripts/lib/default-changes.ts | 40 +++++++-- packages/spec/spec-changes.json | 14 +++ packages/spec/src/api/plugin-rest-api.zod.ts | 7 +- packages/spec/src/api/protocol.test.ts | 64 +++++++++++++ packages/spec/src/api/protocol.zod.ts | 81 ++++++++++++++++- .../src/contracts/notification-service.ts | 16 +++- packages/spec/src/migrations/registry.ts | 90 +++++++++++++++++++ 16 files changed, 521 insertions(+), 51 deletions(-) create mode 100644 .changeset/notification-list-cursor-retired.md diff --git a/.changeset/notification-list-cursor-retired.md b/.changeset/notification-list-cursor-retired.md new file mode 100644 index 0000000000..bae2e5265a --- /dev/null +++ b/.changeset/notification-list-cursor-retired.md @@ -0,0 +1,74 @@ +--- +"@objectstack/spec": major +"@objectstack/client": major +--- + +refactor(spec,client)!: retire the `cursor` half of `GET /api/v1/notifications` and stop declaring a `limit` default nothing applied (#6361, ADR-0049) + +`GET /api/v1/notifications` declared `cursor` on **both** halves of its contract +and honoured it on neither. The dispatcher domain reads `read` / `type` / `limit` +and nothing else, and no emit site has ever written the response key — so a +caller paginating by the published contract re-read the first window forever, +with no error and no 400. Measured over a real boot with 60 unread before the +removal: `page2 === page1`, and **both pages parsed green** against the response +schema, which is why no conformance gate could see it. + +It was worse than inert, because it had a shipped **producer**: the SDK appended +`cursor` to the query string, so the dead parameter was reachable from ordinary +typed code. That 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. + +Ruled jointly with #6363 (maintainer ruling 2026-08-07, Option A): one +capability's two halves are never half-deleted. #6363 made its declaration +**true** (`unreadCount` really is the total); this one removes a declaration +there was no implementation to make true **about**. Opposite repairs, one rule. + +### Migration: FROM → TO + +| FROM | TO | +| :--- | :--- | +| `client.notifications.list({ cursor })` | `client.notifications.list({ limit })` — ask for a bigger window | +| reading `response.cursor` | nothing; it was never emitted, so it always read `undefined` | +| relying on the declared `limit` default of `20` | send `limit: 20` explicitly, or omit `limit` and take the server's window | + +**One-line fix:** delete the `cursor` argument. This route is **not paginated** — +it answers the newest `limit` notifications and stops, so a larger `limit` is the +only way to see further back. There is no continuation token to carry over, and +nothing ever minted one, so no caller holds a value that needs migrating. + +`cursor` is **tombstoned rather than deleted** on both schemas: neither is +`.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 very defect re-created one layer down (#3733, ADR-0104). Writing +it is now a `tsc` error (TS2353) and a parse-time rejection carrying the fix. + +### `limit`: the default is dropped, not re-spelled + +The ruling allowed either declaring the real server default (50) or dropping it +and describing the window as server-decided. The **second** is taken, because the +fiction was the *mechanism* and not the number: nothing parses this query string +through the schema (#3899 wired the route catalog's `requestSchema` to the real +entry for **bodies** only), so `.default(20)` never stamped anything onto +anything. Re-spelling it `50` would have kept a declaration that does not execute +and merely made it coincide with the implementation until someone moved the +clamp. `limit` is now plainly `.optional()`, with the server's behaviour +described as the server's: the platform inbox answers **50** and clamps any +requested value into **1..200**. + +No `.int()`, `.positive()` or `.max(200)` constraint is declared either — the +service *clamps* an out-of-range limit rather than refusing it, and declaring a +rejection the wire does not perform is the same declared-not-enforced defect +mirrored. + +### Behaviour on the wire is UNCHANGED + +Deliberately, and worth stating because a removal invites the opposite +assumption: a request still carrying `?cursor=` is **ignored, not refused**. The +route reads three named query keys and validates no query against a schema, so an +unknown key never produced a 400 and does not start now. A caller that omitted +`limit` receives the same 50 rows it always received. What changed is the +contract, which stopped promising what the wire never delivered. `unreadCount` is +#6363's landed business and is untouched. + + diff --git a/content/docs/references/api/protocol.mdx b/content/docs/references/api/protocol.mdx index a3817db766..e29c7a1b60 100644 --- a/content/docs/references/api/protocol.mdx +++ b/content/docs/references/api/protocol.mdx @@ -1073,8 +1073,8 @@ Install package response | :--- | :--- | :--- | :--- | | **read** | `boolean` | optional | Filter by read status | | **type** | `string` | optional | Filter by notification type | -| **limit** | `number` | ✅ | Maximum number of notifications to return | -| **cursor** | `string` | optional | Pagination cursor | +| **limit** | `number` | optional | Maximum number of notifications to return — the newest N. Omitted leaves the window to the server, which is not a fixed part of this contract: the platform inbox answers 50 and clamps any requested value into 1..200 rather than refusing it. This endpoint is not paginated — there is no continuation token, so a larger window is the only way to see more. | +| **cursor** | `never` | optional | [REMOVED] `cursor` was removed from GET /api/v1/notifications in @objectstack/spec 17 (#6361, ADR-0049) — it was declared on the request AND the response and honoured on neither: the server reads only `read`/`type`/`limit`, and no emit site ever wrote the response key, so a caller paginating by it re-read the first window forever with no error and no 400. Delete the key; the `cursor` argument of `client.notifications.list()` was removed with it. This route is NOT paginated — it answers the newest `limit` notifications and stops, so ask for a bigger window (`limit`, clamped by the server into 1..200) instead of a next page. A first-class inbox cursor, if ever built, will be a response-minted opaque token, not this key. | --- @@ -1085,9 +1085,9 @@ Install package response | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **notifications** | `{ id: string; type: string; title: string; body: string; … }[]` | ✅ | List of notifications | +| **notifications** | `{ id: string; type: string; title: string; body: string; … }[]` | ✅ | List of notifications — the newest window, not a page | | **unreadCount** | `number` | ✅ | Total number of unread notifications | -| **cursor** | `string` | optional | Next page cursor | +| **cursor** | `never` | optional | [REMOVED] `cursor` was removed from GET /api/v1/notifications in @objectstack/spec 17 (#6361, ADR-0049) — it was declared on the request AND the response and honoured on neither: the server reads only `read`/`type`/`limit`, and no emit site ever wrote the response key, so a caller paginating by it re-read the first window forever with no error and no 400. Delete the key; the `cursor` argument of `client.notifications.list()` was removed with it. This route is NOT paginated — it answers the newest `limit` notifications and stops, so ask for a bigger window (`limit`, clamped by the server into 1..200) instead of a next page. A first-class inbox cursor, if ever built, will be a response-minted opaque token, not this key. | --- diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 5c996e65b8..9548e4a656 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -392,6 +392,9 @@ One entry in this step is not a removal at all but a SECURE-DEFAULT FLIP, the sh - **`action-descriptor-resume-authority-default-flip`** — `automation.ActionDescriptor.resumeAuthority — an OMITTED value on a pausing node descriptor (supportsPause: true, or any executor whose execute() returns suspend: true)` → an explicit resumeAuthority: 'any' on the descriptor, for a pausing node whose pauses really are meant to be continued through the generic resume route (POST /automation/:name/runs/:runId/resume) — a screen-style collected-input pause, or a signal wait an external producer resumes. Declare 'service' instead if continuing is the tail of a decision your own service must authorize and record first. Either value is a one-line addition; only the silence changed meaning - Why not automatic: A SECURE-DEFAULT FLIP with no metadata shape to rewrite — the same category as protocol 12's `rest-requireauth-default-flip`, and it is registered here for the same reason: whether a given pause is genuinely open to the generic route is a trust judgment no transform can make. The #3801 resume gate keys on the SUSPENDED NODE, and `ActionDescriptor.resumeAuthority` used to default to `'any'`, so a pausing node type shipped raw-resumable unless its author remembered the field. It now resolves to `'service'` when absent: an unclaimed pause is refused on the generic route with `PERMISSION_DENIED` / 403 until its descriptor states who may continue it. #3823 is the incident that decided the direction — ADR-0044 pointed an approval's revise edge at a generic `wait`, `wait` is legitimately `'any'`, and the pause standing in a service-owned position inherited a fail-open value nobody chose; the demonstrated cost was an unaudited resubmit plus a destroyed remote run. The two possible mistakes are asymmetric, which is the whole argument: guessing `'any'` walks past a decision nothing recorded and is silent, while guessing `'service'` returns a refusal naming the missing field. ⚠️ The surface is a DESCRIPTOR FIELD set in plugin CODE, never stack metadata, so there is no source for a D2 conversion to rewrite and deliberately no schema tombstone — the disposition `data-driver-find-stream-retired` (#4484), `storage-service-list-retired` (#5540) and `actor-user-roles-to-positions` (#6011) already carry. It differs from those in one way a reader should not have to infer: nothing is REMOVED, so tsc reports nothing at all — the field was already optional after step one and an omission still compiles. The enforced channels are all run-time: a registration warning naming the node type (once per type per engine), the refusal message on the resume itself, and `check:resume-authority-declared` for executors living in this repo. For a third-party plugin the generated upgrade guide is the only channel that arrives BEFORE a user hits a run that will not continue. In-tree the flip moves nothing: all six shipped pausing types (screen, wait, subflow, map, approval, approval_revise) declare their authority explicitly. ADR-0044 amendment (2026-07-28) and its 2026-08-08 landing section, ADR-0019 #3801 addendum, #5561. - Done when: Every action descriptor your plugin registers for a node type that can suspend declares `resumeAuthority`. Booting the stack logs no `declares supportsPause but never declares resumeAuthority` warning naming one of your types, and a run parked on each of your pausing nodes can still be continued the way you intend: a resume through the generic route succeeds for the ones you declared `'any'`, and answers 403 (`PERMISSION_DENIED`) for the ones you declared `'service'`, which continue through your own service API instead. ⚠️ `supportsPause` is a declaration nothing enforces (#5703), so an executor whose `execute()` returns `suspend: true` while leaving `supportsPause` false is warned about by NEITHER channel — check those by hand against the same rule. +- **`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 + - 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. + - 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. --- diff --git a/packages/client/src/client.test.ts b/packages/client/src/client.test.ts index 57a51d3bb3..57face3100 100644 --- a/packages/client/src/client.test.ts +++ b/packages/client/src/client.test.ts @@ -707,6 +707,29 @@ describe('Notifications namespace', () => { expect(url).toContain('limit=10'); }); + it('[#6361] never puts a `cursor` on the query string — the SDK producer is gone', async () => { + // The retired half of #6361 asserted where it was PRODUCED. `cursor` was + // never a server-read filter; what made it harmful rather than inert is + // that this method appended it, so a caller paginating by the published + // contract re-read the first window forever with no error. + // + // The type surface is the enforced channel — `list({ cursor })` is a + // TS2353 excess-property error, verified by reverse-verification and + // unavailable to a runtime assertion. This pins the RUNTIME half, which + // tsc cannot reach: an untyped caller (plain JS, a `Record` spread, a + // hand-built options object) must not smuggle the parameter through. + const { client, fetchMock } = createMockClient({ + success: true, + data: { notifications: [], unreadCount: 0 } + }); + const untypedOptions = { read: false, limit: 10, cursor: 'n_42' } as unknown as { read?: boolean; limit?: number }; + await client.notifications.list(untypedOptions); + const url = fetchMock.mock.calls[0][0] as string; + expect(url).toContain('limit=10'); + expect(url).not.toContain('cursor'); + expect(url).not.toContain('n_42'); + }); + it('should mark notifications as read', async () => { const { client, fetchMock } = createMockClient({ success: true, diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index dc05d13ad2..9eeb819c7c 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -3798,15 +3798,21 @@ export class ObjectStackClient { */ notifications = { /** - * List notifications for the current user + * List notifications for the current user. + * + * Returns the newest `limit` notifications — a WINDOW, not a page. The + * `cursor` parameter was removed in protocol 17 (#6361): it was appended to + * the query string here and read by nothing on the server, so a caller + * paginating by it re-read the first window forever. Omit `limit` to take + * the server's window (the platform inbox answers 50, clamped to 1..200); + * raise it to see further back. There is no continuation token. */ - list: async (options?: { read?: boolean; type?: string; limit?: number; cursor?: string }): Promise => { + list: async (options?: { read?: boolean; type?: string; limit?: number }): Promise => { const route = this.getRoute('notifications'); const params = new URLSearchParams(); if (options?.read !== undefined) params.set('read', String(options.read)); if (options?.type) params.set('type', options.type); if (options?.limit) params.set('limit', String(options.limit)); - if (options?.cursor) params.set('cursor', options.cursor); const qs = params.toString(); const res = await this.fetch(`${this.baseUrl}${route}${qs ? `?${qs}` : ''}`); return this.unwrapResponse(res); diff --git a/packages/runtime/src/notification-schema-conformance.integration.test.ts b/packages/runtime/src/notification-schema-conformance.integration.test.ts index 49a47c7009..4c0f48b95f 100644 --- a/packages/runtime/src/notification-schema-conformance.integration.test.ts +++ b/packages/runtime/src/notification-schema-conformance.integration.test.ts @@ -38,6 +38,7 @@ import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; import { MessagingServicePlugin, MessagingService } from '@objectstack/service-messaging'; import { envelopeViolations, + ListNotificationsRequestSchema, ListNotificationsResponseSchema, MarkNotificationsReadResponseSchema, MarkAllNotificationsReadResponseSchema, @@ -230,15 +231,28 @@ describe('[#5792] the notification wire bodies conform to the schemas the catalo // // Both were pinned here as the measured behaviour of `origin/main`, on the // note that whichever way #6361 / #6363 were ruled, these assertions are the - // ones that must flip. #6363 has been ruled (2026-08-07, Option A: make the - // declaration true) and its assertion has flipped — it now pins the fix, over - // the wire, which is the only place the whole stack is in play. The `cursor` - // half is unchanged: it is one capability's two halves and is being retired - // with #6361, so it stays pinned as measured until that lands. + // ones that must flip. BOTH have now been ruled (2026-08-07, Option A, and + // ruled JOINTLY — one capability's two halves are never half-deleted), and + // both assertions have flipped: + // + // * #6363 made the declaration true — `unreadCount` really is the total; + // * #6361 removed the declaration instead — `cursor` is gone from the + // request half, the response half and the SDK producer, because there was + // no implementation to make it true ABOUT. Opposite repairs, same rule: + // declared must equal enforced. + // + // The two directions are why the pair is worth keeping side by side. Note the + // #6361 assertion below now pins something subtler than the #6363 one: the + // WIRE did not change (an unknown `?cursor=` was ignored before and is + // ignored now), so what it proves is that the CONTRACT stopped promising the + // thing the wire never delivered. A test that only checked "page2 === page1" + // would be just as green before and after, which is exactly the vacuity this + // family keeps paying for. // // The Stage D input stands either way, and is if anything sharper now: the // ratchet still cannot see EITHER fact. Both had to be written by hand, and - // the fix below would have been just as invisible to it as the defect was. + // the fixes below would have been just as invisible to it as the defects were + // — a removed optional key changes no parse verdict at all. describe('[#6361 / #6363] the gaps the double assertion cannot see', () => { it('[#6363] `unreadCount` is the TOTAL the schema describes, and survives a smaller window', async () => { const all = await getJson(GAP_USER, '/api/v1/notifications'); @@ -261,22 +275,46 @@ describe('[#5792] the notification wire bodies conform to the schemas the catalo expect(ListNotificationsResponseSchema.safeParse(windowed).success).toBe(true); }); - it('[#6361 / #6363] `cursor` is declared on both sides and honoured on neither', async () => { + it('[#6361] `cursor` is declared on NEITHER side now, and the wire is unchanged', async () => { const page1 = await getJson(GAP_USER, '/api/v1/notifications?limit=2'); const ids1 = page1.notifications.map((n: any) => n.id); - // Response half (#6363): the declared `cursor` key is never emitted. + // Both halves are TOMBSTONED — retired, not silently dropped. Asserted as + // a refusal on each schema, because a bare deletion on a non-strict object + // would have re-created this very issue's defect (silent strip, ADR-0104). expect(Object.prototype.hasOwnProperty.call(page1, 'cursor')).toBe(false); - expect(declaredListKeys().has('cursor')).toBe(true); - - // Request half (#6361): sending the declared `cursor` returns the SAME - // page. An SDK caller paginating by the published contract loops forever. - const page2 = await getJson(GAP_USER, `/api/v1/notifications?limit=2&cursor=${encodeURIComponent(ids1[ids1.length - 1])}`); - expect(page2.notifications.map((n: any) => n.id)).toEqual(ids1); + for (const schema of [ListNotificationsRequestSchema, ListNotificationsResponseSchema]) { + const refused = schema.safeParse({ notifications: [], unreadCount: 0, cursor: 'n_42' }); + expect(refused.success).toBe(false); + expect(refused.error!.issues.some((i) => i.path.join('.') === 'cursor')).toBe(true); + } - // Both pages conform — the whole reason this needed measuring by hand. + // ⚠️ WIRE BEHAVIOUR DELIBERATELY UNCHANGED, measured over a real socket: + // a request still carrying `?cursor=` is IGNORED, not refused. Nothing + // validates this query against a schema, so the unknown key is simply not + // read — it returned the same window before the removal and it returns the + // same window after. Removing a declaration must not silently start + // rejecting traffic, and this is the assertion that would catch it. + const stillSent = await getJson( + GAP_USER, + `/api/v1/notifications?limit=2&cursor=${encodeURIComponent(ids1[ids1.length - 1])}`, + ); + expect(stillSent.notifications.map((n: any) => n.id)).toEqual(ids1); expect(ListNotificationsResponseSchema.safeParse(page1).success).toBe(true); - expect(ListNotificationsResponseSchema.safeParse(page2).success).toBe(true); + expect(ListNotificationsResponseSchema.safeParse(stillSent).success).toBe(true); + }); + + it('[#6361] the removed `limit` default was never in effect — the server window still answers', async () => { + // The other half of the ruling, over the wire. The declaration used to say + // `default(20)`; the server has always answered its own window. With the + // fiction removed, the two agree by SAYING LESS rather than by changing + // behaviour — so the fixture's whole inbox must still come back on a + // request that names no limit. + const all = await getJson(GAP_USER, '/api/v1/notifications'); + expect(all.notifications.length).toBeGreaterThan(1); + // Never truncated at the retired declared default. + expect(all.notifications.length).toBeLessThanOrEqual(50); + expect(ListNotificationsRequestSchema.parse({})).not.toHaveProperty('limit'); }); }); }); diff --git a/packages/runtime/src/notification-schema-conformance.test.ts b/packages/runtime/src/notification-schema-conformance.test.ts index 7ded5e4b4f..fdfdcb5044 100644 --- a/packages/runtime/src/notification-schema-conformance.test.ts +++ b/packages/runtime/src/notification-schema-conformance.test.ts @@ -197,17 +197,16 @@ describe('[#5792] /notifications conforms to the schemas the catalog declares', expect(seen[0]).toEqual({ read: false, type: 'deal.won', limit: 7 }); }); - it('[#6361] recorded, not endorsed: the declared `limit` DEFAULT never reaches the provider', async () => { - // `ListNotificationsRequestSchema.limit` is `z.number().default(20)`, - // but this route never parses the query through that schema — with no - // `limit` the domain forwards `undefined` and the provider applies its - // own window (50). So the declared default has never been in effect. + it('[#6361] the declaration no longer states a `limit` default the route never applies', async () => { + // FLIPPED by #6361 (maintainer ruling 2026-08-07, Option A). This pin + // used to record the DEFECT: `limit` was `z.number().default(20)` while + // the route forwarded `undefined` and the provider applied its own 50, + // so the declared default had never once been in effect. // - // Pinned as the measured fact it is, NOT as the desired behaviour: - // #6361 is the judgement call (align the declaration to 50, or wire - // the query through the schema so 20 takes effect). Whichever way it - // is ruled, this assertion is the one that must change — which is the - // point of pinning it rather than leaving it for the next reader. + // The runtime half of that measurement is UNCHANGED and is re-asserted + // below, because the ruled direction was the schema catching up to the + // implementation — not the reverse. What changed is the declaration: + // there is no default to be wrong about any more. const seen: unknown[] = []; const dispatcher = makeDispatcher({ listInbox: async (_userId: string, options: unknown) => { seen.push(options); return LIST_BODY; }, @@ -217,8 +216,52 @@ describe('[#5792] /notifications conforms to the schemas the catalog declares', await dispatcher.handleNotification('', 'GET', undefined, {}, CTX); + // Behaviour: untouched. An omitted `limit` still reaches the provider + // as `undefined`, which is what lets the provider window it. expect(seen[0]).toEqual({ read: undefined, type: undefined, limit: undefined }); - expect((ListNotificationsRequestSchema as any).shape.limit._zod.def.defaultValue).toBe(20); + + // Declaration: `limit` is now plainly optional — no `default`, so + // parsing an empty query stamps no number onto it. Asserted through + // the PARSE rather than through `_zod.def`, so it cannot go quietly + // vacuous if the internal shape of a Zod default ever moves. + const parsedEmpty = ListNotificationsRequestSchema.parse({}); + expect(Object.prototype.hasOwnProperty.call(parsedEmpty, 'limit')).toBe(false); + expect((parsedEmpty as { limit?: number }).limit).toBeUndefined(); + // An explicit limit still parses — only the invented default is gone. + expect(ListNotificationsRequestSchema.parse({ limit: 7 }).limit).toBe(7); + }); + + it('[#6361] `cursor` is retired on BOTH halves, and the ROUTE still ignores it', async () => { + // The declaration half: tombstoned, not deleted, on request AND + // response (one capability, two halves — the ruled clause). + for (const schema of [ListNotificationsRequestSchema, ListNotificationsResponseSchema]) { + const refused = schema.safeParse({ notifications: [], unreadCount: 0, cursor: 'n2' }); + expect(refused.success).toBe(false); + expect(refused.error!.issues.some((i) => i.path.join('.') === 'cursor')).toBe(true); + } + + // ⚠️ THE ROUTE'S BEHAVIOUR IS UNCHANGED, deliberately, and this is + // the half worth pinning here. The tombstone is loud only where + // something PARSES — and this route parses no query at all (#3899 + // wired the catalog's requestSchema for bodies only). So a request + // still carrying `cursor` is IGNORED, exactly as before: 200, three + // named keys read, no 400. The ruled direction was to stop declaring + // a filter nobody reads, NOT to start refusing traffic, and this + // assertion is what would catch the difference. + const seen: unknown[] = []; + const dispatcher = makeDispatcher({ + listInbox: async (_userId: string, options: unknown) => { seen.push(options); return LIST_BODY; }, + markRead: async () => MARK_READ_BODY, + markAllRead: async () => MARK_ALL_BODY, + }); + + const result = await dispatcher.handleNotification( + '', 'GET', undefined, { cursor: 'n2', limit: '2' }, CTX, + ); + + expect(result.response?.status).toBe(200); + expect(seen[0]).toEqual({ read: undefined, type: undefined, limit: 2 }); + expect(seen[0]).not.toHaveProperty('cursor'); }); }); diff --git a/packages/spec/authorable-defaults/api.json b/packages/spec/authorable-defaults/api.json index 025d84ad71..7cbf53264d 100644 --- a/packages/spec/authorable-defaults/api.json +++ b/packages/spec/authorable-defaults/api.json @@ -95,7 +95,6 @@ "api/ListImportJobsRequest:limit = 50", "api/ListImportJobsRequest:offset = 0", "api/ListInstalledPackagesRequest:limit = 50", - "api/ListNotificationsRequest:limit = 20", "api/ListRunsRequest:limit = 20", "api/LoginRequest:type = \"email\"", "api/MetadataBulkRegisterRequest:continueOnError = false", diff --git a/packages/spec/authorable-surface/api.json b/packages/spec/authorable-surface/api.json index 49d16b75f7..feac532db1 100644 --- a/packages/spec/authorable-surface/api.json +++ b/packages/spec/authorable-surface/api.json @@ -953,11 +953,11 @@ "api/ListInstalledPackagesResponse:error", "api/ListInstalledPackagesResponse:meta", "api/ListInstalledPackagesResponse:success", - "api/ListNotificationsRequest:cursor", + "api/ListNotificationsRequest:cursor [RETIRED]", "api/ListNotificationsRequest:limit", "api/ListNotificationsRequest:read", "api/ListNotificationsRequest:type", - "api/ListNotificationsResponse:cursor", + "api/ListNotificationsResponse:cursor [RETIRED]", "api/ListNotificationsResponse:notifications", "api/ListNotificationsResponse:unreadCount", "api/ListPackagesRequest:enabled", diff --git a/packages/spec/scripts/lib/default-changes.ts b/packages/spec/scripts/lib/default-changes.ts index 8417152ff3..7801751f47 100644 --- a/packages/spec/scripts/lib/default-changes.ts +++ b/packages/spec/scripts/lib/default-changes.ts @@ -60,10 +60,38 @@ import type { DeclaredDefaultChange } from './authorable-defaults.js'; /** * Declared default changes, keyed by the protocol major that shipped them. * - * Empty at major 17: this ratchet lands with no default change to declare, and - * that emptiness is the gate's own proof — `check:authorable-surface` is green - * on origin/main with the table holding nothing, which means every default in - * the tree matches its recorded fingerprint. The first entry will be written by - * whoever first needs to move one. + * The table landed EMPTY at major 17 — that emptiness was the ratchet's own + * proof that every default in the tree matched its recorded fingerprint. The + * entry below is the first one written (#6361), and it is worth noting what + * kind of change opened the account: not a behaviour flip, but the removal of a + * default that had never once been applied. */ -export const DEFAULT_CHANGES_BY_MAJOR: Readonly> = {}; +export const DEFAULT_CHANGES_BY_MAJOR: Readonly> = { + 17: [ + { + key: 'api/ListNotificationsRequest:limit', + from: '20', + to: '(none)', + reason: + 'GET /api/v1/notifications declared `limit: z.number().default(20)` while the server ' + + 'has always answered its own window of 50 (MessagingService.listInbox clamps into ' + + '1..200). The declared default was never in effect on ANY request path — nothing ' + + 'parses this query string through this schema, because #3899 wired the route ' + + "catalog's requestSchema to the real entry for BODIES only — so this removal moves " + + 'no deployed behaviour whatsoever: a caller that omitted `limit` received 50 before ' + + 'and receives 50 after. What changes is the DECLARATION, which stops promising a ' + + 'number nobody applied. The maintainer ruling (2026-08-07, #6361 Option A) allowed ' + + 'either re-declaring the real 50 or dropping the default as server-decided; the ' + + 'second was taken because the fiction was the MECHANISM, not the number — a ' + + '`.default()` on a schema no request path parses cannot take effect at whatever ' + + 'value it is spelled, and re-spelling it 50 would merely make it coincide with the ' + + 'implementation until someone moved the clamp. ' + + 'A consumer who genuinely relied on 20 was relying on client-side code of their ' + + 'own, since the wire never delivered it: to keep a 20-row window, send it — ' + + '`client.notifications.list({ limit: 20 })`. To keep what the server actually ' + + 'gave you, change nothing. Reading `ListNotificationsRequestParsed.limit` now ' + + 'yields `number | undefined` instead of `number`; the honest answer to "how big is ' + + 'the window" is the server\'s, and it is documented on the key.', + }, + ], +}; diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index b3a2ca4d7a..ffd930df47 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -699,6 +699,13 @@ "migrationId": "action-descriptor-resume-authority-default-flip", "toMajor": 17, "rationale": "A SECURE-DEFAULT FLIP with no metadata shape to rewrite — the same category as protocol 12's `rest-requireauth-default-flip`, and it is registered here for the same reason: whether a given pause is genuinely open to the generic route is a trust judgment no transform can make. The #3801 resume gate keys on the SUSPENDED NODE, and `ActionDescriptor.resumeAuthority` used to default to `'any'`, so a pausing node type shipped raw-resumable unless its author remembered the field. It now resolves to `'service'` when absent: an unclaimed pause is refused on the generic route with `PERMISSION_DENIED` / 403 until its descriptor states who may continue it. #3823 is the incident that decided the direction — ADR-0044 pointed an approval's revise edge at a generic `wait`, `wait` is legitimately `'any'`, and the pause standing in a service-owned position inherited a fail-open value nobody chose; the demonstrated cost was an unaudited resubmit plus a destroyed remote run. The two possible mistakes are asymmetric, which is the whole argument: guessing `'any'` walks past a decision nothing recorded and is silent, while guessing `'service'` returns a refusal naming the missing field. ⚠️ The surface is a DESCRIPTOR FIELD set in plugin CODE, never stack metadata, so there is no source for a D2 conversion to rewrite and deliberately no schema tombstone — the disposition `data-driver-find-stream-retired` (#4484), `storage-service-list-retired` (#5540) and `actor-user-roles-to-positions` (#6011) already carry. It differs from those in one way a reader should not have to infer: nothing is REMOVED, so tsc reports nothing at all — the field was already optional after step one and an omission still compiles. The enforced channels are all run-time: a registration warning naming the node type (once per type per engine), the refusal message on the resume itself, and `check:resume-authority-declared` for executors living in this repo. For a third-party plugin the generated upgrade guide is the only channel that arrives BEFORE a user hits a run that will not continue. In-tree the flip moves nothing: all six shipped pausing types (screen, wait, subflow, map, approval, approval_revise) declare their authority explicitly. ADR-0044 amendment (2026-07-28) and its 2026-08-08 landing section, ADR-0019 #3801 addendum, #5561." + }, + { + "surface": "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)", + "replacement": "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", + "migrationId": "notification-list-cursor-retired", + "toMajor": 17, + "rationale": "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." } ], "removed": [] @@ -1457,6 +1464,13 @@ "migrationId": "action-descriptor-resume-authority-default-flip", "toMajor": 17, "rationale": "A SECURE-DEFAULT FLIP with no metadata shape to rewrite — the same category as protocol 12's `rest-requireauth-default-flip`, and it is registered here for the same reason: whether a given pause is genuinely open to the generic route is a trust judgment no transform can make. The #3801 resume gate keys on the SUSPENDED NODE, and `ActionDescriptor.resumeAuthority` used to default to `'any'`, so a pausing node type shipped raw-resumable unless its author remembered the field. It now resolves to `'service'` when absent: an unclaimed pause is refused on the generic route with `PERMISSION_DENIED` / 403 until its descriptor states who may continue it. #3823 is the incident that decided the direction — ADR-0044 pointed an approval's revise edge at a generic `wait`, `wait` is legitimately `'any'`, and the pause standing in a service-owned position inherited a fail-open value nobody chose; the demonstrated cost was an unaudited resubmit plus a destroyed remote run. The two possible mistakes are asymmetric, which is the whole argument: guessing `'any'` walks past a decision nothing recorded and is silent, while guessing `'service'` returns a refusal naming the missing field. ⚠️ The surface is a DESCRIPTOR FIELD set in plugin CODE, never stack metadata, so there is no source for a D2 conversion to rewrite and deliberately no schema tombstone — the disposition `data-driver-find-stream-retired` (#4484), `storage-service-list-retired` (#5540) and `actor-user-roles-to-positions` (#6011) already carry. It differs from those in one way a reader should not have to infer: nothing is REMOVED, so tsc reports nothing at all — the field was already optional after step one and an omission still compiles. The enforced channels are all run-time: a registration warning naming the node type (once per type per engine), the refusal message on the resume itself, and `check:resume-authority-declared` for executors living in this repo. For a third-party plugin the generated upgrade guide is the only channel that arrives BEFORE a user hits a run that will not continue. In-tree the flip moves nothing: all six shipped pausing types (screen, wait, subflow, map, approval, approval_revise) declare their authority explicitly. ADR-0044 amendment (2026-07-28) and its 2026-08-08 landing section, ADR-0019 #3801 addendum, #5561." + }, + { + "surface": "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)", + "replacement": "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", + "migrationId": "notification-list-cursor-retired", + "toMajor": 17, + "rationale": "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." } ], "removed": [] diff --git a/packages/spec/src/api/plugin-rest-api.zod.ts b/packages/spec/src/api/plugin-rest-api.zod.ts index 1aef52b556..91aa3a54f5 100644 --- a/packages/spec/src/api/plugin-rest-api.zod.ts +++ b/packages/spec/src/api/plugin-rest-api.zod.ts @@ -1083,7 +1083,12 @@ export const DEFAULT_NOTIFICATION_ROUTES: RestApiRouteRegistration = { category: 'notification', public: false, summary: 'List notifications', - description: 'Returns paginated list of notifications for the current user', + // NOT "paginated" (#6361). The route answers the newest `limit` rows and + // stops; there is no continuation token on either half of the contract + // since `cursor` was removed in protocol 17. The catalog is a + // machine-readable surface (Route & surface ownership rule 4), so a + // pagination claim here is read by SDKs and codegen as a capability. + description: 'Returns the newest window of notifications for the current user (not paginated)', tags: ['Notifications'], responseSchema: 'ListNotificationsResponseSchema', cacheable: false, diff --git a/packages/spec/src/api/protocol.test.ts b/packages/spec/src/api/protocol.test.ts index 921dd1d5ce..8ab8551048 100644 --- a/packages/spec/src/api/protocol.test.ts +++ b/packages/spec/src/api/protocol.test.ts @@ -58,6 +58,7 @@ import { GetFieldLabelsRequestSchema, GetFieldLabelsResponseSchema, } from './protocol.zod'; +import type { ListNotificationsRequest } from './protocol.zod'; describe('ObjectStack Protocol', () => { @@ -272,6 +273,69 @@ describe('ObjectStack Protocol', () => { expect(MarkNotificationsReadRequestSchema.safeParse({ ids: ['n1', 'n2'] }).success).toBe(true); }); + /** + * [#6361] `GET /api/v1/notifications` declares no pagination — on either half. + * + * Maintainer ruling 2026-08-07 (Option A), ruled jointly with #6363: one + * capability's two halves are never half-deleted. `cursor` was declared on the + * request AND the response and honoured on neither, and `limit` declared a + * `.default(20)` no request path has ever applied (the server windows at 50). + * + * Asserted on the SHAPES rather than only through `safeParse`, and that choice + * is the whole point: both retired keys were `optional`, so every parse was + * green before the removal and every parse is green after it. A value-level + * test cannot see this class of defect at all — which is exactly why the + * family's double-assertion ratchet (#3877 Stage D) was blind to it. + */ + it('[#6361] tombstones `cursor` on BOTH halves, with the prescription', () => { + // BOTH halves or neither — the ruling's "one capability, two halves" clause, + // asserted rather than trusted. A half-deletion is the specific outcome the + // maintainer ruled out, so it gets a test rather than a comment. + for (const [half, schema] of [ + ['request', ListNotificationsRequestSchema], + ['response', ListNotificationsResponseSchema], + ] as const) { + const probe = half === 'request' + ? { read: false, limit: 10, cursor: 'n_42' } + : { notifications: [], unreadCount: 0, cursor: 'n_42' }; + const result = schema.safeParse(probe); + + // Refused, not stripped. Neither schema is `.strict()`, so a BARE DELETION + // would have parsed this cleanly and dropped the key — the silent-strip + // class (#3733, ADR-0104), i.e. the very defect this issue reports, + // re-created one layer down. That is why the assertion is on the refusal. + expect(result.success, `${half} half must refuse a retired \`cursor\``).toBe(false); + const issue = result.error!.issues.find((i) => i.path.join('.') === 'cursor'); + expect(issue, `${half} half must fault on the \`cursor\` path`).toBeDefined(); + + // The message IS the migration doc (retiredKey's contract), so its + // substance is asserted, not merely its existence: what was removed, that + // the route is not paginated, and the replacement to reach for. + expect(issue!.message).toMatch(/`cursor` was removed from GET \/api\/v1\/notifications/); + expect(issue!.message).toMatch(/not paginated/i); + expect(issue!.message).toMatch(/limit/); + expect(issue!.message).toMatch(/#6361/); + } + + // `tsc` is the first channel retiredKey buys — the input type is `never`. + // Pinned as a compile-time fact so deleting the tombstone cannot pass as a + // refactor. @ts-expect-error fails the build if the key becomes writable. + // @ts-expect-error `cursor` is retired: its input type is `never`. + const rejectedByTsc: ListNotificationsRequest = { cursor: 'n_42' }; + void rejectedByTsc; + + // The surviving keys, named. `limit` is plainly optional now — parsing an + // empty query stamps NO number onto it, where it used to stamp 20 that no + // request path had ever applied. Stated through the parse result so it holds + // if Zod's internal representation of a default ever moves. + expect(Object.keys((ListNotificationsRequestSchema as any).shape)).toEqual(['read', 'type', 'limit', 'cursor']); + const parsedEmpty = ListNotificationsRequestSchema.parse({}); + expect(Object.prototype.hasOwnProperty.call(parsedEmpty, 'limit')).toBe(false); + expect(ListNotificationsRequestSchema.parse({ limit: 7 }).limit).toBe(7); + // The response half keeps exactly #6363's landed business, and gains nothing. + expect(ListNotificationsResponseSchema.safeParse({ notifications: [], unreadCount: 0 }).success).toBe(true); + }); + /** * These replace the `AiNlq*` / `AiSuggest*` / `AiInsights*` cases (#3718). * Those parsed cleanly for years against endpoints no repo has ever mounted diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index c456b5dfac..6a16e7220f 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -1151,17 +1151,90 @@ export const NotificationSchema = lazySchema(() => z.object({ createdAt: z.string().datetime().describe('When notification was created'), })); +// ========================================== +// Notification inbox listing — NOT paginated +// ========================================== + +// `cursor` was declared on BOTH halves of `GET /api/v1/notifications` and +// honoured on neither, and `limit` declared a default the server has never +// applied. Both are removed per the maintainer ruling of 2026-08-07 (#6361, +// Option A), ruled together with #6363 as one capability's two halves — a +// pagination capability is never half-deleted. +// +// ## What the route actually does +// +// It answers a WINDOW, never a page: `MessagingService.listInbox` reads the +// newest `limit` rows and stops. There is no continuation token, no `hasMore`, +// and no ordering key a caller could resume from — so `cursor` had nothing to +// carry even if something had read it. The request half was ignored by the +// domain (which reads `read` / `type` / `limit` and nothing else) and the +// response half was never emitted, so an SDK caller looping "until the cursor +// runs out" re-read page 1 forever, with no error and no 400. +// +// This is `data.query.cursor` (#4286, `query-cursor-retired`) one layer up, +// with the same verdict for the same reason: a caller-visible pagination +// parameter no engine implements is worse than inert, because it has a shipped +// producer. That one deleted `QueryBuilder.cursor()` with the key; this one +// deletes the `cursor` argument of `client.notifications.list()`. +// +// ## Why `limit` loses its default rather than gaining a truer number +// +// The ruling allowed either "declare the real default (50)" or "drop the +// default and describe it as server-decided". The second is taken because the +// FICTION IS THE MECHANISM, not just the number: nothing parses a query string +// through this schema (#3899 wired the catalog's `requestSchema` to the real +// entry for BODIES only), so `.default(...)` has never stamped anything onto +// anything. Re-spelling `20` as `50` would keep a declaration that does not +// execute and merely make it coincide with the server for as long as nobody +// moves the clamp. `.optional()` plus prose is true on both axes: the schema +// claims no behaviour it does not perform, and the server's window is +// described as the server's. +// +// Deliberately NOT declared: `.int()`, `.positive()` or `.max(200)`. The +// service CLAMPS an out-of-range limit (`Math.min(Math.max(limit ?? 50, 1), +// 200)`); it does not refuse one. A constraint here would declare a rejection +// the wire does not perform — the same declared-not-enforced defect in the +// opposite direction. ADR-0049, #6361. + +/** + * One prescription, two rejection sites — the `cursor` key was declared on both + * halves of this route, so both tombstone it with the same string. + * + * Tombstoned rather than deleted for the ADR-0104 reason these schemas keep + * paying for: neither is `.strict()`, so a bare deletion makes Zod SILENTLY + * STRIP whatever the caller keeps sending — a clean parse and a parameter that + * never takes effect, which is the very failure this issue is about, moved one + * layer down. `retiredKey()` types the key as `never` (so `tsc` refuses it at + * the authoring site) and raises this text at parse time. + */ +const NOTIFICATIONS_CURSOR_REMOVED = + '`cursor` was removed from GET /api/v1/notifications in @objectstack/spec 17 ' + + '(#6361, ADR-0049) — it was declared on the request AND the response and honoured on ' + + 'neither: the server reads only `read`/`type`/`limit`, and no emit site ever wrote the ' + + 'response key, so a caller paginating by it re-read the first window forever with no ' + + 'error and no 400. Delete the key; the `cursor` argument of ' + + '`client.notifications.list()` was removed with it. This route is NOT paginated — it ' + + 'answers the newest `limit` notifications and stops, so ask for a bigger window ' + + '(`limit`, clamped by the server into 1..200) instead of a next page. A first-class ' + + 'inbox cursor, if ever built, will be a response-minted opaque token, not this key.'; + export const ListNotificationsRequestSchema = lazySchema(() => z.object({ read: z.boolean().optional().describe('Filter by read status'), type: z.string().optional().describe('Filter by notification type'), - limit: z.number().default(20).describe('Maximum number of notifications to return'), - cursor: z.string().optional().describe('Pagination cursor'), + limit: z.number().optional().describe( + 'Maximum number of notifications to return — the newest N. Omitted leaves the window ' + + 'to the server, which is not a fixed part of this contract: the platform inbox ' + + 'answers 50 and clamps any requested value into 1..200 rather than refusing it. ' + + 'This endpoint is not paginated — there is no continuation token, so a larger ' + + 'window is the only way to see more.', + ), + cursor: retiredKey(NOTIFICATIONS_CURSOR_REMOVED), })); export const ListNotificationsResponseSchema = lazySchema(() => z.object({ - notifications: z.array(NotificationSchema).describe('List of notifications'), + notifications: z.array(NotificationSchema).describe('List of notifications — the newest window, not a page'), unreadCount: z.number().describe('Total number of unread notifications'), - cursor: z.string().optional().describe('Next page cursor'), + cursor: retiredKey(NOTIFICATIONS_CURSOR_REMOVED), })); export const MarkNotificationsReadRequestSchema = lazySchema(() => z.object({ diff --git a/packages/spec/src/contracts/notification-service.ts b/packages/spec/src/contracts/notification-service.ts index 719b10b1c1..9c8ec92cee 100644 --- a/packages/spec/src/contracts/notification-service.ts +++ b/packages/spec/src/contracts/notification-service.ts @@ -62,9 +62,19 @@ export interface NotificationResult { /** * Filters for {@link INotificationService.listInbox}. Mirrors - * `ListNotificationsRequestSchema` minus `cursor` — no implementation paginates - * by cursor yet, and declaring a parameter nothing honours is the - * `declared ≠ enforced` gap this file exists to close (#4127). + * `ListNotificationsRequestSchema` — now EXACTLY, key for key. + * + * It used to mirror it "minus `cursor`": #4127 dropped the key from this + * internal contract because no implementation paginates by cursor, while the + * wire schema kept declaring it to callers for another nine majors. That split + * is what #6361 closed — the wire half was removed in protocol 17 (maintainer + * ruling 2026-08-07, Option A), so the two faces of one query finally agree and + * this interface no longer has to explain a subtraction. The `declared ≠ + * enforced` gap this file exists to close (#4127) is closed on both faces. + * + * `limit` stays advisory on purpose: implementations CLAMP rather than refuse + * (the platform inbox windows at 50 and bounds requests into 1..200), which is + * why neither this interface nor the wire schema declares a maximum. */ export interface InboxQuery { /** Filter by read state; omitted returns both. */ diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 106e3ff569..6be023183f 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -2747,6 +2747,79 @@ const step17: MigrationStep = { + 'leaving `supportsPause` false is warned about by NEITHER channel — check those by ' + 'hand against the same rule.', }, + { + id: 'notification-list-cursor-retired', + // No backticks in `surface` — build-upgrade-guide.ts renders it inside a + // code span AND a table cell (see the note on `spec-type-alias-input-suffix-retired`). + surface: + '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)', + replacement: + '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', + reason: + '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.', + acceptanceCriteria: + '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.', + }, ], }; @@ -2875,6 +2948,23 @@ export const RETIRED_KEYS_BY_MAJOR: Readonly> 'ui/ElementRecordPickerProps:multiple', 'ui/ElementRecordPickerProps:searchFields', 'ui/PageCardProps:body', + // #6361 — the notification-inbox pagination key, tombstoned on BOTH halves + // of `GET /api/v1/notifications` because one capability is never half- + // deleted (maintainer ruling 2026-08-07, ruled jointly with #6363). Two + // keys, one prescription: `NOTIFICATIONS_CURSOR_REMOVED` in + // `api/protocol.zod.ts` is the single string both rejection sites raise. + // + // Registered here but NOT in `src/conversions/registry.ts`, and that + // asymmetry is the point rather than an omission: a D2 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. The prescription reaches consumers as the D3 semantic entry + // `notification-list-cursor-retired` plus this tombstone, which is the + // disposition `BatchOptions.validateOnly` and the `AnalyticsQueryRequest` + // envelope keys already take in this major ("a semantic TODO for API + // callers rather than a stack conversion"). + 'api/ListNotificationsRequest:cursor', + 'api/ListNotificationsResponse:cursor', ], }; From 6ca8d57618c2f818d1245a3028bf46807f7c7752 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 04:31:31 +0000 Subject: [PATCH 2/2] chore(spec): regenerate ADR-0087 artifacts on the merged tree (#6361) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `main` gained #6868 (`page-tabs-type-to-tab-style` + `ui/PageTabsProps:type`) after this branch's last CI head. The merge itself was clean — git placed the three siblings' entries at different offsets — but the generated artifacts are driver-deferred and were regenerated from the merged tree rather than text-merged. All three protocol-17 siblings verified present after regeneration: `notification-list-cursor-retired` (this PR), `action-descriptor-is-async-retired` (#6862) and `page-tabs-type-to-tab-style` (#6868, a D2 conversion). `authorable-surface/api.json` and `authorable-defaults/api.json` pick up `api/ApiRoutes:email` and `api/MetadataEndpointsConfig:maskObjectFields` from other PRs merged in the same window — this branch's copies were simply behind. check:generated 10/10. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M --- docs/protocol-upgrade-guide.md | 7 +++++-- packages/spec/authorable-defaults/api.json | 1 + packages/spec/authorable-surface/api.json | 2 ++ packages/spec/spec-changes.json | 16 ++++++++++++++-- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 11c9e898d1..e8801744eb 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -218,6 +218,8 @@ The last of the #4001 enforce-or-remove batch lands on two more `ui/` files (#50 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. +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). + 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. 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. @@ -278,6 +280,7 @@ The same descriptor loses a key in this step, and the pairing is the point (#674 | `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 | | `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 | | `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 | +| `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 | ### Semantic (delegated to you, with acceptance criteria) @@ -368,9 +371,9 @@ The same descriptor loses a key in this step, and the pairing is the point (#674 - **`actor-user-roles-to-positions`** — `action body / AI route: ctx.user.roles (req.user.roles)` → ctx.user.positions (an AI route handler reads `req.user.positions`) — the same array, under the one spelling ADR-0090 D3 sanctions - Why not automatic: The THIRD face of the ADR-0090 `roles` → `positions` rename, and the only one whose surface the spec never declared. `ActorUser` (`packages/runtime/src/security/actor-user.ts`) is the ONE producer of the `user` envelope handed to an action body as `ctx.user` and to an AI route handler as `req.user`; it declared `positions` and `roles` side by side and filled them from a SINGLE assignment (`roles: core.positions`), so the two keys were verbatim identical on every dispatch — a second spelling of the vocabulary ADR-0090 D3 reserves and bans, published straight into author-written code. The maintainer ruled it closed IMMEDIATELY (2026-08-06 14:49Z, #6011): no deprecation window, no dual-emit, the alias simply gone in 17 (PR #6048). ⚠️ Do not read this entry across to its neighbour above: `action-session-roles-to-positions` governs `ctx.session`, a DIFFERENT object reached through the same `ctx`, and that one KEEPS its one-window dual-emit (#5613). Same word, same dispatch, two faces, two schedules — `ctx.user.roles` is absent in 17 while `ctx.session.roles` still answers for the length of its window. What makes this entry different in KIND from both session-side siblings: `ctx.user` has no spec schema and never had one. It is a runtime TS interface, so unlike `HookContext.session.roles` (tombstoned on a deliberately non-strict `HookContextSchema`, #5050) and unlike `ActionSessionSchema` (declared contract-first at #5697 precisely so its key could be renamed), there is no schema key here to tombstone and no `retiredKey()` prescription that could reach anybody — nothing ever ran an `ActorUser` through a `.parse()`, so a prescription there would have no one to reach. The enforced channel is tsc, and it reports at the READ site inside the author's own body; for an untyped or sandboxed body there is no enforced channel at all, which is exactly why this ledger entry has to exist — `spec-changes.json` and the generated upgrade guide are the ONLY way such a reader learns of the rename. It is the `findStream` (#4484) / `IStorageService.list` (#5540) disposition — a TS/API contract, no stored source, no tombstone, tsc at the call site — applied to a surface that lives one layer further out than either: those two are at least DECLARED in `packages/spec/src/contracts`, this one only in `packages/runtime`. Why it is a D3 semantic TODO and not a D2 conversion, on the same two independent grounds as its session sibling: FIRST, there is no source to convert — an `ActorUser` is constructed per dispatch and never persisted, so no `sys_metadata` row, example or template can carry the key (the `openApi31` (#4579) / `activationEvents` (#4657) / `hook-context-session-roles-retired` (#5050) shape). SECOND, the only place the key is ever SPELLED is inside an action body or an AI route handler: author-written JS/TS, or a sandboxed script. A declarative transform cannot safely rewrite an identifier inside free-form code — the same reason the ADR-0090 wave delegated `current_user.roles` to the author at step 13 (`cel-current-user-roles-to-positions`) instead of substituting text. The removal's hard precondition was met before it landed, and the result is recorded here because the ledger is where an upgrading consumer meets it: the declaration's own comment claimed the alias was "kept for the REST/AI shapes", and that claim was DISPROVEN face by face against `origin/main` — repo-wide `user.roles` was 4 hits, all of them in the pins PR #6048 flipped; the four `ActorUser` construction sites build server-side envelopes that never enter a response body; objectui's `.roles` reads belong to two unrelated producers (the better-auth session, and the `/auth/me/permissions` payload). The `cloud` repo was NOT reachable in that session and is the one consumer face left unverified — this entry, and the changeset's FROM/TO prescription, are its disposition. ADR-0090 D3 / ADR-0049 / ADR-0087, #6011 (PR #6048). - Done when: No action body reads `ctx.user.roles` and no AI route handler reads `req.user.roles`; every such read is `.positions` and observes the SAME array — the value was `ExecutionContext.positions` on both sides, so this is a pure key rename and no value has to be re-derived. Privilege is NOT re-derived from either spelling: a read that was `roles.includes('admin')` as an access check is rewritten to ask the security service (capability grants / placements / derived posture, ADR-0095), never renamed to `positions.includes('admin')` — renaming that read migrates the defect rather than the code. Unlike `ctx.session` there is NO window to migrate inside: in 17 the key is already absent, so a typed body fails `tsc` at the read while an untyped or sandboxed one silently sees `undefined` — move the read AS you upgrade, not after it. Verify against a real dispatch rather than a fixture: invoke an action (and an AI route) as a caller holding positions, assert the body observed them under the canonical key, and assert the old key is ABSENT by key existence (`'roles' in ctx.user === false`) rather than by `undefined`, which cannot tell a removed key from one left behind holding nothing — the runtime pin `action-ctx-user-shape.test.ts` asserts both halves that way. -- **`storage-service-list-retired`** — `contracts.IStorageService.list` → no replacement — track the keys you wrote (sys_file / file-reference records, queryable through ObjectQL with real pagination) instead of enumerating the bucket +- **`storage-service-list-retired`** — `contracts.IStorageService.list` → track the keys you wrote (sys_file / file-reference records, queryable through ObjectQL with real pagination) instead of enumerating the bucket — and where no such record exists, the cursor-shaped `list(prefix, { cursor, limit })` this entry reserved, restored in #6781 - Why not automatic: `list(prefix)` was an OPTIONAL contract method documented as "List files in a directory/prefix", and the two shipped adapters answered the same call with two different semantics — both of them silently incomplete. `LocalStorageAdapter.list` was a single-level `readdir`, so a nested key `a/b/c` was invisible under `list('a')` (only `a/b` came back), and a subdirectory that `stat` succeeded on was pushed into the result as a file, yielding a `StorageFileInfo` whose `size` is a directory inode and which cannot be downloaded at all. `S3StorageAdapter.list` was RECURSIVE (`ListObjectsV2` matches the whole key) and read neither `IsTruncated` nor `ContinuationToken`, so past 1000 objects the "all files" a caller received was the first page, with no signal. One contract method, two dialects, both quietly incomplete — and the first feature that genuinely needed to enumerate a prefix (backup, orphan sweep, migration audit) would have got two different answers on two deployments without an error on either. #5172 was nearly that feature: it planned to drive attachment reclamation off `list(EMAIL_ATTACHMENT_KEY_PREFIX)`, found the local adapter could not see one level down, and switched to queue-driven deferred work instead. Nothing consumed it afterwards: the only in-repo call site was the `SwappableStorageService` pass-through (which itself rejects when the active adapter has no `list`), and REST, CLI and the storage routes never called it. Remove was chosen over align-and-tighten (maintainer ruling, 2026-08-05, #5266): aligning would grow a conformance surface nobody walks, while a prefix listing that cannot paginate is the wrong signature to inherit — when a real caller needs enumeration it returns cursor-shaped, `list(prefix, { cursor, limit })`, with adapter-conformance cases (nested keys, directory entries, >1000 objects) proving both backends agree. This is a TS/API contract surface — a storage adapter is CODE, never stack metadata — so there is no source for the chain to rewrite, and deliberately no schema tombstone: nothing ever ran an adapter through a `.parse()`, so a prescription there would reach no one. The enforced channel is tsc, and it reports at the call site. Same disposition, and the same reason, as `data-driver-find-stream-retired` (#4484). ADR-0049 / ADR-0087, #5540 (analysis #5266). - - Done when: No code calls `storage.list(...)` on the `file-storage` service or on any `IStorageService` value. Code that needed "which files are under this prefix" reads the records it wrote — `sys_file` / file-reference rows carry the storage key and page deterministically through ObjectQL — rather than asking the bucket, which is also the only form that stays correct past 1000 objects and across both adapters. An adapter that still IMPLEMENTS `list` keeps compiling (an extra method is not an error on a class) and is simply unreachable through the contract, so deleting it is cleanup that can follow. The break is on the CALLER side: `storage.list(...)` no longer type-checks, and a PROXY typed against `IStorageService` that forwards to `inner.list` is exactly such a caller — the one in `@objectstack/service-storage` goes with the adapters (#5541). + - Done when: No code calls `storage.list(...)` on the `file-storage` service or on any `IStorageService` value. Code that needed "which files are under this prefix" reads the records it wrote — `sys_file` / file-reference rows carry the storage key and page deterministically through ObjectQL — rather than asking the bucket, which is also the only form that stays correct past 1000 objects and across both adapters. An adapter that still IMPLEMENTS `list` keeps compiling (an extra method is not an error on a class) and is simply unreachable through the contract, so deleting it is cleanup that can follow. The break is on the CALLER side: `storage.list(...)` no longer type-checks, and a PROXY typed against `IStorageService` that forwards to `inner.list` is exactly such a caller — the one in `@objectstack/service-storage` goes with the adapters (#5541). ⚠️ AMENDED 2026-08-09 (#6781, maintainer ruling on cloud#1203, option B): the RESERVED route in the paragraph above was taken. `list` exists again on the contract, cursor-shaped — `list(prefix, { cursor, limit })` returning `{ items, nextCursor }` — because cloud had two first-party callers this repo could not see when the measurement said "nothing calls it" (tenant attachment reclamation, marketplace snapshot GC). This does NOT un-retire anything and the acceptance criterion above is unchanged for what it actually governs: the single-argument `list(prefix): StorageFileInfo[]` is gone for good, a call written against it still fails to compile, and the two dialects it had are now pinned against each other in `storage-adapter-list.conformance.test.ts` rather than left to diverge. What changed for an upgrader is only the destination: prefer the records you wrote, and reach for the restored member when there are none. - **`driver-aggregate-undeclared-key-aliases-removed`** — `driver aggregate() call argument — query.aggregate and aggregations[].func` → query.aggregations and aggregations[].function — the spellings QueryASTSchema and AggregationNodeSchema have always declared - Why not automatic: `SqlDriver.aggregate` and `RemoteTransport.aggregate` each read two aliases the Query Protocol has never declared: `query.aggregations || query.aggregate` and `agg.function || agg.func`. "Never declared" is measured, not assumed — `git log -S` over `data/query.zod.ts` finds no commit that ever introduced either name, there is no `retiredKey()` tombstone and no alias-table entry for them (the file's only alias table is `SortNode`'s `direction` → `order`), and neither appears in any upgrade guide or release note. So this entry does not record a declared surface being withdrawn; it records a LENIENCY being withdrawn, which is why it is here rather than behind a tombstone. The only writers in this repository were the two driver packages' own fixtures — #4984's family, where a fixture spelling the alias keeps the tolerant limb green forever and no test in existence can go red on its deletion — so ADR-0049 enforce-or-remove applies once those are re-spelt. ⚠️ Do NOT read this across to `dashboard`/`page` measures: `aggregate` IS the canonical key there and `func` IS a declared, loudly-suggesting alias (`DatasetMeasureSchema`, ui/dataset.zod.ts). That neighbouring vocabulary is untouched, and it is the most likely reason an off-repo caller ever wrote these keys on a QUERY — one habit, two surfaces, only one of which declared it. This is a driver CALL ARGUMENT — code, never stack metadata — so there is no source for the D2 chain to rewrite and deliberately no schema tombstone: nothing ever ran a query through `QueryASTSchema.parse()` on this path. The enforced channel is tsc at the call site, once the parameter is `DriverQuery` — and for an untyped JS caller there is no enforced channel at all, which is exactly why this ledger entry has to exist: the generated upgrade guide is the only way such a reader learns of the rename. Same disposition, and the same reason, as `data-driver-find-stream-retired` (#4484), `storage-service-list-retired` (#5540) and `actor-user-roles-to-positions` (#6011). ADR-0049 / ADR-0087, #6321 (PR #6404). - Done when: No caller passes `aggregate:` to a driver's `aggregate()`, and no aggregation entry spells its function `func:`; both are written `aggregations:` / `function:`. An inline literal still using either old spelling no longer type-checks (TS2353 at the call site). An untyped JS caller that keeps writing `aggregate:` silently receives no aggregate column — the grouping still happens, the measure is simply absent — and one that keeps writing `func:` receives INVALID_QUERY / 400 naming the undeclared function, identically on the local driver and the Turso remote transport. diff --git a/packages/spec/authorable-defaults/api.json b/packages/spec/authorable-defaults/api.json index 7cbf53264d..06762c6b86 100644 --- a/packages/spec/authorable-defaults/api.json +++ b/packages/spec/authorable-defaults/api.json @@ -102,6 +102,7 @@ "api/MetadataCacheResponse:notModified = false", "api/MetadataEndpointsConfig:cacheTtl = 3600", "api/MetadataEndpointsConfig:enableCache = true", + "api/MetadataEndpointsConfig:maskObjectFields = true", "api/MetadataEndpointsConfig:prefix = \"/meta\"", "api/MetadataExportRequest:format = \"json\"", "api/MetadataImportRequest:conflictResolution = \"skip\"", diff --git a/packages/spec/authorable-surface/api.json b/packages/spec/authorable-surface/api.json index feac532db1..599d0d85af 100644 --- a/packages/spec/authorable-surface/api.json +++ b/packages/spec/authorable-surface/api.json @@ -146,6 +146,7 @@ "api/ApiRoutes:data", "api/ApiRoutes:datasources", "api/ApiRoutes:discovery", + "api/ApiRoutes:email", "api/ApiRoutes:i18n", "api/ApiRoutes:mcp", "api/ApiRoutes:metadata", @@ -1025,6 +1026,7 @@ "api/MetadataEndpointsConfig:cacheTtl", "api/MetadataEndpointsConfig:enableCache", "api/MetadataEndpointsConfig:endpoints", + "api/MetadataEndpointsConfig:maskObjectFields", "api/MetadataEndpointsConfig:prefix", "api/MetadataEvent:definition", "api/MetadataEvent:id", diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 8fc5253349..5db3efca85 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -367,6 +367,12 @@ "to": "inline type:'api' action prop 'params' (object form) → 'bodyExtra' (#5777 — the payload gets its own key; `params` stays the ActionParam[] definition array)", "conversionId": "inline-action-api-params-to-body-extra", "toMajor": 17 + }, + { + "surface": "page.component.page:tabs.type", + "to": "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)", + "conversionId": "page-tabs-type-to-tab-style", + "toMajor": 17 } ], "migrated": [ @@ -645,7 +651,7 @@ }, { "surface": "contracts.IStorageService.list", - "replacement": "no replacement — track the keys you wrote (sys_file / file-reference records, queryable through ObjectQL with real pagination) instead of enumerating the bucket", + "replacement": "track the keys you wrote (sys_file / file-reference records, queryable through ObjectQL with real pagination) instead of enumerating the bucket — and where no such record exists, the cursor-shaped `list(prefix, { cursor, limit })` this entry reserved, restored in #6781", "migrationId": "storage-service-list-retired", "toMajor": 17, "rationale": "`list(prefix)` was an OPTIONAL contract method documented as \"List files in a directory/prefix\", and the two shipped adapters answered the same call with two different semantics — both of them silently incomplete. `LocalStorageAdapter.list` was a single-level `readdir`, so a nested key `a/b/c` was invisible under `list('a')` (only `a/b` came back), and a subdirectory that `stat` succeeded on was pushed into the result as a file, yielding a `StorageFileInfo` whose `size` is a directory inode and which cannot be downloaded at all. `S3StorageAdapter.list` was RECURSIVE (`ListObjectsV2` matches the whole key) and read neither `IsTruncated` nor `ContinuationToken`, so past 1000 objects the \"all files\" a caller received was the first page, with no signal. One contract method, two dialects, both quietly incomplete — and the first feature that genuinely needed to enumerate a prefix (backup, orphan sweep, migration audit) would have got two different answers on two deployments without an error on either. #5172 was nearly that feature: it planned to drive attachment reclamation off `list(EMAIL_ATTACHMENT_KEY_PREFIX)`, found the local adapter could not see one level down, and switched to queue-driven deferred work instead. Nothing consumed it afterwards: the only in-repo call site was the `SwappableStorageService` pass-through (which itself rejects when the active adapter has no `list`), and REST, CLI and the storage routes never called it. Remove was chosen over align-and-tighten (maintainer ruling, 2026-08-05, #5266): aligning would grow a conformance surface nobody walks, while a prefix listing that cannot paginate is the wrong signature to inherit — when a real caller needs enumeration it returns cursor-shaped, `list(prefix, { cursor, limit })`, with adapter-conformance cases (nested keys, directory entries, >1000 objects) proving both backends agree. This is a TS/API contract surface — a storage adapter is CODE, never stack metadata — so there is no source for the chain to rewrite, and deliberately no schema tombstone: nothing ever ran an adapter through a `.parse()`, so a prescription there would reach no one. The enforced channel is tsc, and it reports at the call site. Same disposition, and the same reason, as `data-driver-find-stream-retired` (#4484). ADR-0049 / ADR-0087, #5540 (analysis #5266)." @@ -1222,6 +1228,12 @@ "to": "inline type:'api' action prop 'params' (object form) → 'bodyExtra' (#5777 — the payload gets its own key; `params` stays the ActionParam[] definition array)", "conversionId": "inline-action-api-params-to-body-extra", "toMajor": 17 + }, + { + "surface": "page.component.page:tabs.type", + "to": "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)", + "conversionId": "page-tabs-type-to-tab-style", + "toMajor": 17 } ], "migrated": [ @@ -1430,7 +1442,7 @@ }, { "surface": "contracts.IStorageService.list", - "replacement": "no replacement — track the keys you wrote (sys_file / file-reference records, queryable through ObjectQL with real pagination) instead of enumerating the bucket", + "replacement": "track the keys you wrote (sys_file / file-reference records, queryable through ObjectQL with real pagination) instead of enumerating the bucket — and where no such record exists, the cursor-shaped `list(prefix, { cursor, limit })` this entry reserved, restored in #6781", "migrationId": "storage-service-list-retired", "toMajor": 17, "rationale": "`list(prefix)` was an OPTIONAL contract method documented as \"List files in a directory/prefix\", and the two shipped adapters answered the same call with two different semantics — both of them silently incomplete. `LocalStorageAdapter.list` was a single-level `readdir`, so a nested key `a/b/c` was invisible under `list('a')` (only `a/b` came back), and a subdirectory that `stat` succeeded on was pushed into the result as a file, yielding a `StorageFileInfo` whose `size` is a directory inode and which cannot be downloaded at all. `S3StorageAdapter.list` was RECURSIVE (`ListObjectsV2` matches the whole key) and read neither `IsTruncated` nor `ContinuationToken`, so past 1000 objects the \"all files\" a caller received was the first page, with no signal. One contract method, two dialects, both quietly incomplete — and the first feature that genuinely needed to enumerate a prefix (backup, orphan sweep, migration audit) would have got two different answers on two deployments without an error on either. #5172 was nearly that feature: it planned to drive attachment reclamation off `list(EMAIL_ATTACHMENT_KEY_PREFIX)`, found the local adapter could not see one level down, and switched to queue-driven deferred work instead. Nothing consumed it afterwards: the only in-repo call site was the `SwappableStorageService` pass-through (which itself rejects when the active adapter has no `list`), and REST, CLI and the storage routes never called it. Remove was chosen over align-and-tighten (maintainer ruling, 2026-08-05, #5266): aligning would grow a conformance surface nobody walks, while a prefix listing that cannot paginate is the wrong signature to inherit — when a real caller needs enumeration it returns cursor-shaped, `list(prefix, { cursor, limit })`, with adapter-conformance cases (nested keys, directory entries, >1000 objects) proving both backends agree. This is a TS/API contract surface — a storage adapter is CODE, never stack metadata — so there is no source for the chain to rewrite, and deliberately no schema tombstone: nothing ever ran an adapter through a `.parse()`, so a prescription there would reach no one. The enforced channel is tsc, and it reports at the call site. Same disposition, and the same reason, as `data-driver-find-stream-retired` (#4484). ADR-0049 / ADR-0087, #5540 (analysis #5266)."