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 84f86cd8bc..e8801744eb 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -404,6 +404,9 @@ The same descriptor loses a key in this step, and the pairing is the point (#674 - **`action-descriptor-is-async-retired`** — `ActionDescriptor.isAsync (the descriptor an executor publishes via `registerNodeExecutor` / `defineActionDescriptor`)` → nothing to re-declare — delete the key. Suspension is `execute()` RETURNING `suspend: true`, and permission to suspend is `supportsPause: true` on the same descriptor (with the `resumeAuthority` its pauses need) - Why not automatic: ADR-0049 enforce-or-remove. `isAsync` declared "this action suspends the flow awaiting an external reply" and NOTHING read it: a fresh three-repo measurement (#6748, re-run at pickup) found zero property reads across objectstack, objectui and cloud — every hit was the declaration itself, a generated baseline, one of five shipped descriptors WRITING it, a test fixture pinning the shape, or prose. So declaring it never made a node suspend and omitting it never stopped one, which is the silently-inert declaration ADR-0049 exists to end. It was always a second, weaker spelling of the capability `supportsPause` states, and the two diverged in exactly the way a duplicated declaration does: `screen` declared both, `map` and `wait` declared `isAsync` alongside `supportsPause`, and nothing anywhere reconciled them. The sibling took the ENFORCE leg of the same ruling in #6667 — `AutomationEngine` now refuses a suspension whose type does not declare `supportsPause: true` — so the capability this key gestured at is now a real, enforced fact under one name. This one had no consumer to grow into and takes the remove leg. Why D3 semantic and not a D2 conversion: an ActionDescriptor is published from an executor's TypeScript, never stored in stack metadata — no stack, example or template carries the key — so there is no source for the chain to rewrite and `os migrate meta` cannot reach it. The schema tombstones it via `retiredKey()` and descriptor authors delete the key themselves; that rejection (a `tsc` error at the authoring site, and a parse error inside `defineActionDescriptor`) is the channel a third-party plugin author actually meets. The `EnhancedApiError.fieldErrors` disposition, one layer down. - Done when: No descriptor declares `isAsync` — not the five that shipped it (`screen`, `map`, `wait`, `approval`, `approval_revise`), not a plugin's. Every node type that returns `suspend: true` from `execute()` declares `supportsPause: true` on its descriptor together with a `resumeAuthority`, and its runs still pause and resume as before: the behaviour never depended on `isAsync`, so deleting the key changes no run. Authoring `isAsync` fails `tsc` at the descriptor literal and fails `defineActionDescriptor()` at runtime with the prescription, instead of parsing clean and being stripped. +- **`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 ac198978b3..bf8cd244c1 100644 --- a/packages/client/src/client.test.ts +++ b/packages/client/src/client.test.ts @@ -737,6 +737,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 f49623cc23..d96668c4fb 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -3859,15 +3859,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 d59472e167..06762c6b86 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 7e8b37457d..599d0d85af 100644 --- a/packages/spec/authorable-surface/api.json +++ b/packages/spec/authorable-surface/api.json @@ -954,11 +954,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 e1f7bb59ae..5db3efca85 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -725,6 +725,13 @@ "migrationId": "action-descriptor-is-async-retired", "toMajor": 17, "rationale": "ADR-0049 enforce-or-remove. `isAsync` declared \"this action suspends the flow awaiting an external reply\" and NOTHING read it: a fresh three-repo measurement (#6748, re-run at pickup) found zero property reads across objectstack, objectui and cloud — every hit was the declaration itself, a generated baseline, one of five shipped descriptors WRITING it, a test fixture pinning the shape, or prose. So declaring it never made a node suspend and omitting it never stopped one, which is the silently-inert declaration ADR-0049 exists to end. It was always a second, weaker spelling of the capability `supportsPause` states, and the two diverged in exactly the way a duplicated declaration does: `screen` declared both, `map` and `wait` declared `isAsync` alongside `supportsPause`, and nothing anywhere reconciled them. The sibling took the ENFORCE leg of the same ruling in #6667 — `AutomationEngine` now refuses a suspension whose type does not declare `supportsPause: true` — so the capability this key gestured at is now a real, enforced fact under one name. This one had no consumer to grow into and takes the remove leg. Why D3 semantic and not a D2 conversion: an ActionDescriptor is published from an executor's TypeScript, never stored in stack metadata — no stack, example or template carries the key — so there is no source for the chain to rewrite and `os migrate meta` cannot reach it. The schema tombstones it via `retiredKey()` and descriptor authors delete the key themselves; that rejection (a `tsc` error at the authoring site, and a parse error inside `defineActionDescriptor`) is the channel a third-party plugin author actually meets. The `EnhancedApiError.fieldErrors` disposition, one layer down." + }, + { + "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": [] @@ -1509,6 +1516,13 @@ "migrationId": "action-descriptor-is-async-retired", "toMajor": 17, "rationale": "ADR-0049 enforce-or-remove. `isAsync` declared \"this action suspends the flow awaiting an external reply\" and NOTHING read it: a fresh three-repo measurement (#6748, re-run at pickup) found zero property reads across objectstack, objectui and cloud — every hit was the declaration itself, a generated baseline, one of five shipped descriptors WRITING it, a test fixture pinning the shape, or prose. So declaring it never made a node suspend and omitting it never stopped one, which is the silently-inert declaration ADR-0049 exists to end. It was always a second, weaker spelling of the capability `supportsPause` states, and the two diverged in exactly the way a duplicated declaration does: `screen` declared both, `map` and `wait` declared `isAsync` alongside `supportsPause`, and nothing anywhere reconciled them. The sibling took the ENFORCE leg of the same ruling in #6667 — `AutomationEngine` now refuses a suspension whose type does not declare `supportsPause: true` — so the capability this key gestured at is now a real, enforced fact under one name. This one had no consumer to grow into and takes the remove leg. Why D3 semantic and not a D2 conversion: an ActionDescriptor is published from an executor's TypeScript, never stored in stack metadata — no stack, example or template carries the key — so there is no source for the chain to rewrite and `os migrate meta` cannot reach it. The schema tombstones it via `retiredKey()` and descriptor authors delete the key themselves; that rejection (a `tsc` error at the authoring site, and a parse error inside `defineActionDescriptor`) is the channel a third-party plugin author actually meets. The `EnhancedApiError.fieldErrors` disposition, one layer down." + }, + { + "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 045e8b951f..c7ee59c170 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -2906,6 +2906,79 @@ const step17: MigrationStep = { + '`defineActionDescriptor()` at runtime with the prescription, instead of parsing ' + 'clean and being stripped.', }, + { + 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.', + }, ], }; @@ -3049,6 +3122,23 @@ export const RETIRED_KEYS_BY_MAJOR: Readonly> // MetadataConversion — there is no stored source for `os migrate meta` to // rewrite. The `EnhancedApiError.fieldErrors` precedent. 'automation/ActionDescriptor:isAsync', + // #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', ], };