refactor(spec,client)!: retire the notification-list cursor on both halves and drop the fictional limit default (#6361) - #6866
Merged
Conversation
… halves and drop the fictional `limit` default (#6361) `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This was referenced Aug 9, 2026
…ifications-cursor-removal # Conflicts: # packages/spec/src/migrations/registry.ts
…ifications-cursor-removal # Conflicts: # packages/spec/src/migrations/registry.ts
github-actions
Bot
added
size/l
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 116 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This was referenced Aug 9, 2026
…ifications-cursor-removal
`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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M
os-zhuang
marked this pull request as ready for review
August 9, 2026 05:21
This was referenced Aug 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6361
GET /api/v1/notificationsdeclaredcursoron both halves of its contract and honoured it on neither. The dispatcher domain readsread/type/limitand 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. 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
cursorto the query string, so the dead parameter was reachable from ordinary typed code. This isdata.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; this one removes a declaration there was no implementation to make true about. Opposite repairs, one rule.
Premise re-verified before implementing
Measured on
origin/main@6de592c(not taken from the issue body):cursoratprotocol.zod.ts~:1158cursorat ~:1164cursorat ~:3809read/limit/typeDEFAULT_NOTIFICATION_ROUTESdeclaresListNotificationsRequestSchema"The one correction: the route catalog entry for
GET ''carries norequestSchemaat all (onlyresponseSchema). #3899 stripped the notification table's request schemas. The declaration that misled callers lives inprotocol.zod.tsitself — the exported schema plus thelistNotifications(request: ListNotificationsRequest)protocol method — not in the catalog. The defect is real; that one sentence of its explanation was not.What changed
cursoris tombstoned, not deleted, on both schemas. This is the one substantive departure from the dispatch card's wording, and the gate is what taught me:check:authorable-surfacerefuses a bare deletion because neither schema is.strict(), so deleting the key would make Zod silently strip whatever callers keep sending — a clean parse and a parameter that never takes effect, i.e. this very defect re-created one layer down (ComputedFieldCacheSchema 是 2026-06 字段剪除留下的第二个孤儿(#3726 表格误记为「已清理」) #3733, ADR-0104).retiredKey()types the keyneverand raises the prescription at parse time. Both keys registered inRETIRED_KEYS_BY_MAJOR[17].cursorremoved fromclient.notifications.list(). Passing it is now TS2353.limitloses its.default(20)(the ruling's second arm). Rationale below.notification-list-cursor-retired. No D2 conversion: a conversion rewrites an authored source or a storedsys_metadatarow, and both shapes are HTTP-only — nobody authors aListNotificationsRequestand nothing persists one. Same dispositionBatchOptions.validateOnly(BatchOptions.validateOnly 声明了 dry-run 但从不实现 —— "预演"会真实落库(PD #10) #4052) already takes in this major.DEFAULT_CHANGES_BY_MAJOR[17]— this table's first entry ever; it shipped empty at 17 and a second gate (spec 门禁盲区:可作者化 key 的「默认值 / 约束」变更不被任何 gate、tombstone 或 conversion 记录(#4650 / #4659 同族) #4666) demanded the declaration.The
limittrade-off, as askedThe ruling allowed either declaring the real server default (50) or dropping it as server-decided. Dropped, because the fiction was the mechanism, not the number: nothing parses this query string through the schema (#3899 wired
requestSchemato the real entry for bodies only), so.default(20)never stamped anything onto anything. Re-spelling it50keeps a declaration that does not execute and merely makes it coincide with the implementation until someone moves the clamp — a fiction with a truer number is still a fiction.Cost of the choice, stated plainly:
ListNotificationsRequestParsed.limitbecomesnumber | undefinedinstead ofnumber, so a consumer reading the parsed shape loses a guaranteed number. That is the honest type — the window is the server's answer, not the schema's.No
.int(),.positive()or.max(200)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 defect mirrored.Wire behaviour is UNCHANGED — measured, as required
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. Asserted over a real socket in the integration suite, and at the domain level in the unit suite. An omittedlimitstill yields the server's 50-row window.unreadCountis #6363's landed business and is untouched.Reverse verification (direction predicted first)
list({ cursor: 'n_42' })→error TS2353: Object literal may only specify known properties, and 'cursor' does not exist in type '{ read?: boolean | undefined; type?: string | undefined; limit?: number | undefined; }'. Probe deleted.cursor: z.string().optional()on the request half turned the new spec pin red (1 failed | 34 passed) and the@ts-expect-errorpin red (src/api/protocol.test.ts: 1 type error(s)). Restored. Both pins are load-bearing rather than decorative.Verification (re-run after merging
origin/main)mainmoved twice under this branch; both merges conflicted inpackages/spec/src/migrations/registry.tsbecause #6862 and a sibling retirement appended to the same two tables. Resolved by keeping both sides in both places, then regenerating — never hand-merging — the driver-deferred artifacts. Asserted present after resolution:action-descriptor-is-async-retiredandnotification-list-cursor-retired;automation/ActionDescriptor:isAsyncand bothapi/ListNotifications*:cursorkeys.pnpm --filter @objectstack/spec check:generated→ All 10 generated artifacts are up to date.typecheckgreen on all four packages.lint.yml, run locally one by one: 52 passed, 0 failed.check:type-check-debt,check:i18n,check:i18n-coverageandcheck:app-nav-i18neach answeredPREREQUISITE NOT MET("Nothing was checked") because they refuse to run without a fully built workspace, and the container's shared build lock was seven agents deep. CI has since run all four on a clean checkout:ESLintandTypeScript Type Checkboth concludedsuccess, along with every other job.success, 1skipped(Console Pin Gate), 0 failures.Consumer sweep
Prefix direction (
...@objectstack/spec,...@objectstack/client). The onlycursorreader on this route was the SDK's query append. objectui'suseClientNotifications.tscallslist({})— unaffected.cloudhas no reference. Every pin of the old shape was flipped to assert the new substance, not merely to drop the old assertion.Effect on #3877 Stage D (mandatory answer)
Unchanged — and the card's own evidence gets sharper. The Stage D double-assertion ratchet still cannot see either fact:
cursorwasoptional, so "no producer emits it" was always a legal parse and the KEY assertion (subset, not equality) could not see it; removing an optional key changes no parse verdict either. Both the defect and the fix had to be written by hand, which is precisely the Stage D input recorded on #6363, now demonstrated in both directions — #6363 repaired by making a declaration true, #6361 by removing one. No other notification-family card is made easier or harder; the retired key is registered where Stage D can read it if the ratchet ever grows a declaration-level input.Generated by Claude Code