Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions .changeset/notification-list-cursor-retired.md
Original file line number Diff line number Diff line change
@@ -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.

<!-- adr-0087: registered notification-list-cursor-retired -->
8 changes: 4 additions & 4 deletions content/docs/references/api/protocol.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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. |


---
Expand All @@ -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. |


---
Expand Down
3 changes: 3 additions & 0 deletions docs/protocol-upgrade-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

---

Expand Down
23 changes: 23 additions & 0 deletions packages/client/src/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 9 additions & 3 deletions packages/client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ListNotificationsResponse> => {
list: async (options?: { read?: boolean; type?: string; limit?: number }): Promise<ListNotificationsResponse> => {
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<ListNotificationsResponse>(res);
Expand Down
Loading
Loading