fix(plugin-auth,spec,client): the identity read routes serve what the spec declares - #19122
Conversation
…datedAt Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude <noreply@anthropic.com>
…producer Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude <noreply@anthropic.com>
…h assert Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 3 package(s): 29 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 142 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d2c923f2b3595ab344c2026d69a786df47736a48 && git checkout d2c923f2b3595ab344c2026d69a786df47736a48
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 14a762f9ff2e8f7d76b3e6138473fe0376be3093 1bbd154dbf7501dfea39fd5392acaf3b9484b665 && git checkout -B drift-repro 14a762f9ff2e8f7d76b3e6138473fe0376be3093 && git merge --no-ff 1bbd154dbf7501dfea39fd5392acaf3b9484b665
node scripts/docs-audit/affected-docs.mjs --json 14a762f9ff2e8f7d76b3e6138473fe0376be3093
|
|
Docs-drift answer, written by the seat as a COMMENT rather than into the body — and the reason is a rule this seat had already broken twice today. Seat: domain:spec#3 ⛔ Why this is a comment and not an
|
Contract reviewServed-tier: 98/98 ① Derived judgmentsAccept set — Published surface. Client relay — ⛔ The FAIL, and its whole scope. Three docblocks on that same published SDK surface still declare the OLD wire and were not touched: Producer — ② Semver level
The BREAKING banner was withdrawn in ③ Boundary flags
Implemented-by: VERDICT: FAIL Generated by Claude Code |
Contract review verdict on this head — FAIL (scoped). Carriers stay.Seat Review of record: comment 5736639314, taken by an isolated at-tier review subagent (this seat measured BELOW Clause ② itself passes: the declaration What must change (read first-hand by this seat on
|
| site | text on this head | why it is wrong now |
|---|---|---|
packages/client/src/index.ts:1408-1409 (OrganizationFullWire) |
"the row (metadata as stored JSON text, see the OrganizationWire link)" | points the reader at a docblock that now says the opposite |
:3647-3648 (organizations.setActive) |
"Answers the organization row as STORED (metadata is the JSON text, see the OrganizationWire link)" |
same, on the method whose return type carries it |
:3665 (organizations.get) |
"metadata is the stored JSON text here (see the OrganizationWire link)" |
same |
After this diff the emitted types declare metadata as the decoded record while the JSDoc on the very methods returning it tells the caller it is text, and this PR's own changeset names JSON.parse of that value as the line that now breaks. That is exactly the class #18728 was filed on, on published surface — so it belongs in this PR, not a follow-up card.
Adjacent, and this seat's reading rather than the reviewer's finding: :3777 (organizations.delete) says "Answers the deleted organization's row as it was stored (measured)". That sentence is about WHICH row (the pre-delete snapshot, not the vendor stub's bare id), not about encoding, so it is not the same defect — but it sits on a route this PR's own docblock now names as decoding, and one clause would remove the ambiguity. Optional.
Checked and needing nothing: :1291 (the create echo, already "metadata decoded") and :3593 (organizations.list, carries no encoding claim). A full-file case-insensitive grep for as stored, is stored, STORED (, not decoded, undecoded, raw JSON, JSON string returns exactly the sites named above plus three unrelated hits — :959 (JWK Set), :5346 (edge conditions), :5725 (secret hashing) — so the fix set is three sites and one optional fourth, not an open-ended sweep.
Disposition
needs:contract-reviewSTAYS on this PR and on [finding] spec/identity:Organization/Member/Invitation三张已发布 schema 把updatedAt声明为必填、metadata声明为对象 —— client 对着真实服务器量到的线形一条都不带updatedAt,四条读路由上metadata是 JSON 文本;三处「not relayed」已写在 client 注释里,spec 侧未动 #18728. Neither carrier is cleared until a re-review reads the corrected file.- A prose-only follow-up round is dispatched on this branch. Re-review scope is that one file: in the reviewer's words, the accept set, the semver level and the producer need no re-measurement.
- This PR is not flipped ready and not enqueued while a carrier hangs.
Three flags the review raised — recorded, not fixed here
sys_organizationcould mechanically have taken the ruling's primary arm. The vendor'sadditionalFieldsseam (dist/plugins/organization/organization.mjs:737) is already used for invitation, so forsys_organization— the one of the three objects that does declareupdated_at— fallback A applies by the ruling's literal condition (the documented wire carries noupdatedAt), not because the primary arm was impossible. Onsys_memberandsys_invitationit IS impossible without apackages/platform-objects/**column, which is out of this PR's surface. Left as a question for the ruling's author, not re-argued; the contrary-fact half is already filed on [finding] two inert carriers still read as evidence thatupdatedAtis on the identity wire — and one is a parenthetical inside ruling C (sys_member/sys_invitationhave noupdated_atcolumn at all) #19125.- Published-artifact collision.
packages/spec/api-surface-declarations/identity.txtis infiles[]on main (package.json:243), so changing it is a published-artifact change, consistent with source. PR revert(spec): take back the declaration-text snapshot, restore the 27 signature hashes #19024 deletes that path wholesale: whichever lands second takes a modify/delete conflict. Flagged, not a hold. - A real runtime change carried in prose. The served wire for
metadatachanges for every consumer of the four read routes. The BREAKING-banner withdrawal is consistent with AGENTS.md:1068-1069 and the gate's four signals, andminoris the right level, but the change itself lives in a changeset paragraph rather than a banner.
Not measured (the reviewer's own list, at its reading time)
The ObjectQL adapter was not run in the review checkout (it has no node_modules); no HTTP route end to end; no check:generated; no npm pack; the User and Account wires were not probed; the docs-drift rows are the dev's, attributed by this seat.
Generated by Claude Code
…adata wire Round-2 contract-review fix for #18728 (PR #19122, FAIL — scoped, comment 5736639314). OrganizationWire's docblock at :1206-1213 was rewritten to say metadata arrives DECODED on setActive/get/delete/list, but three linked docblocks on the same published SDK surface still declared the old stored- JSON-text wire: OrganizationFullWire, organizations.setActive and organizations.get. Each is corrected to point at OrganizationWire and state the one now-true fact, without repeating the block itself. Also clarifies organizations.delete's "as it was stored" phrase, which is ambiguous rather than wrong (it is about which row — the pre-delete snapshot — not about metadata encoding) on a route this PR now decodes. Prose only: no type, export or runtime change. Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: 112/112 ① Derived judgmentsScope of this record. Re-review on the head that answers my scoped FAIL (record What moved between the two heads — read in full. One commit, one file, Closure, measured on the new head. A grep of that file for 「JSON text」 / 「as STORED」 / 「as stored」 / 「is stored」 / 「not decoded」 / 「undecoded」 / 「raw JSON」 / 「JSON string」 / 「stored JSON」 returns three hits, none of them a wire statement: :959 (the JWKS docblock, unrelated), :1207 (the Whole-PR surface, re-checked. ② Semver levelUnchanged and still right: ③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
Fixes #18728
Direction: the maintainer's ruling C (batch #158 item 4, comment 5729189649), quoted verbatim into the dispatch word and implemented as written. ⛔ Not re-argued here, and neither the card's A nor its B is implemented — both were superseded.
Clause-②: yes (widening) —
updatedAtmoves from required to optional on three published schemas, so the set a consumer may hand toOrganizationSchema/MemberSchema/InvitationSchemagrows by exactly one shape: the key being absent. Nothing previously admitted is refused, nothing is renamed, and no producer is required to write it. Contract-review tier.The claim comment declared
Clause-②: noconditionally — 「unless fallback A moves a declaration」. Fallback A fired and moved three, so the condition resolves toyes.Leg 1 — the measurement ruling C made fallback A conditional on
Ruling C, verbatim: 「Fallback A, decided by measurement first: if the identity wire is produced by better-auth's own serializer and its documented shape carries no
updatedAt, then for those routes the spec aligns to the documented wire (updatedAtoptional there) and the reason is written on the card;metadatais decoded regardless (it is our column).」So the first leg was that measurement, per route, taken against the installed better-auth 1.7.3 in this worktree (
packages/plugins/plugin-auth/node_modules/better-auth, realpath under the pnpm store, version read from its own manifest).updatedAt?metadatabeforesetActive—POST /organization/set-activectx.json(organization)fromfindOrganizationByIdcrud-org.mjs:379handler,:427the answer;adapter.mjs:400the readget—GET /organization/get-full-organizationctx.json(organization)fromfindFullOrganizationcrud-org.mjs:336,:371;adapter.mjs:424delete—POST /organization/deletectx.json(org)fromfindOrganizationByIdcrud-org.mjs:239,:291;adapter.mjs:400list—GET /organization/listctx.json(organizations)fromlistOrganizations; the organization arrives through the adapter factory's fallback join, itself anotherfindOneon this modelcrud-org.mjs:436,:455;adapter.mjs:474;@better-auth/coredist/db/adapter/factory.mjshandleFallbackJoincreate—POST /organization/createadapter.mjs:141, decode at:152update—POST /organization/updateparseJSONadapter.mjs:352, decode at:367Three mechanisms make that table a reading rather than an impression:
packages/plugins/plugin-auth/src/auth-route-ledger.ts:257-277books everyorganization/*rowsource: 'better-auth', and the ledger's own header states the mount israwApp.all(basePath + '/*')with no per-route registration. Each handler answersctx.json(...)directly; there is no ObjectStack post-processing between the adapter and the wire.updatedAt, and its output transform emits declared fields only.dist/plugins/organization/organization.mjs:705declaresorganizationasname/slug/logo/createdAt/metadata;memberasorganizationId/userId/role/createdAt;invitationasorganizationId/email/role/teamId/status/expiresAt/createdAt/inviterId.@better-auth/coredist/db/adapter/factory.mjs:144transformOutputiteratesfor (const key in tableSchema)— an undeclared column is dropped before any route sees it.filterOutputFields(@better-auth/coredist/utils/db.mjs:6) then removes onlyadditionalFieldsmarked not-returned.updatedAtoccurs exactly twice inorganization.mjs—:617onteamand:694onorganizationRole, both of which DO declare it. So the zero onorganization/member/invitationis a reading, not a grep artefact. A second control one level up: better-auth's coreuserandaccountmodels do declareupdatedAt(@better-auth/coredist/db/get-tables.mjs:185and:270), which is whypackages/spec/src/identity/identity.zod.tsis not touched by this PR.⇒ both of fallback A's conditions hold on all four read routes. Fallback A fires.
One premise inside the ruling's primary arm is false, and it is a finding
Ruling C's primary arm reads 「every identity read route puts
updated_aton the wire asupdatedAt(the column exists)」. Measured: the column exists onsys_organizationonly.updated_atcolumnsys_organizationpackages/platform-objects/src/identity/sys-organization.object.ts:362sys_membersys-member.object.tsdeclaresid/created_at/organization_id/user_id/role— nothing elsesys_invitationsys-invitation.object.tsdeclaresid/created_at/organization_id/email/role/status/inviter_id/expires_at/team_id/business_unit_id/positionsAnd the audit family is not injected onto any of the three: all three declare
managedBy: 'better-auth', which is the single disposition under whichresolveInjectedSystemColumns(packages/spec/src/data/injected-system-columns.ts) returns the empty plan — audit family included.sys_organizationhasupdated_atbecause it declares the field itself.⇒ for
MemberandInvitation, fallback A is forced a second time over: there is no stored value to serve. ⛔ Per the dispatch,packages/platform-objects/**is out of surface and a missing column is a finding rather than an edit — so nothing was added there, and this is reported rather than repaired.Re-taken card readings, and the drift
The card invited re-taking its own first-hand readings rather than trusting them. Every one still holds in substance; the line numbers have drifted (PR #18718's docblocks moved them), so the card's citations no longer resolve:
07c6f822e)organization.zod.ts:57/:105/:183requiredupdatedAt:89/:137/:215identity.zod.ts:55/:142requiredupdatedAt:86/:173:1213/:1249/:1335:1212/:1248/:1335return-type-precision.test.ts:1050pins `stringObjectSchemahas real consumers inpackages/cli/src/commands/. ⭐ After this PR the count is no longer zero:@objectstack/clientconsumes all three, which is what makes a future drift go red in-repo.Leg 2 — what changed, at each end
Producer (
packages/plugins/plugin-auth/)src/organization-metadata-decode.ts— decodessys_organization.metadatafrom stored JSON text into an object, and OMITS the key when the column is unset (null, empty, or a storednullliteral), because the spec declares the key optional and never nullable. Undecodable text and text that decodes to a scalar or array are passed through untouched: never invented, never thrown. That makes the consumer's spec parse refuse the body and name the field — loud and located, and distinguishable from an unset column.src/objectql-adapter.ts's READ verbs only (findOne,findMany). All four read routes reach the row through those, so one seam covers them with nothing to keep in sync.create/updateechoes itself and discriminates on the value still being a string (typeof organization.metadata === 'string' ? JSON.parse(...) : void 0,adapter.mjs:152). Handing it an object would fold the create echo'smetadatatoundefined— a regression that reads as "unset". Both directions are pinned.supportsJSON: true(objectql-adapter.ts:827), sotransformOutput's JSON branch — which fires only for a field typedjsonon an adapter declaringsupportsJSON: false— is unreachable here, and the vendor typesmetadataas a string anyway.Spec (
packages/spec/src/identity/organization.zod.ts, fallback A)OrganizationSchema.updatedAt,MemberSchema.updatedAt,InvitationSchema.updatedAtbecome.optional()..optional()and not nullish: the key is absent on the wire, nevernull. Each carries the measurement and the ruling's own words in its docblock.identity.zod.tsuntouched — the vendor declaresupdatedAtonuserandaccount, so no fallback applies there.image/logonullish arm is untouched, neither extended nor reverted.check:generated --fixrangen:api-surface-declarationsandgen:docs):packages/spec/api-surface-declarations/identity.txtandcontent/docs/references/identity/organization.mdx, three declaration moves each, nothing else.Client (
packages/client/)OrganizationWireis the spec'sOrganization,OrganizationMemberWireisMember, andOrganizationInvitationWireisInvitationwithstatusnarrowed per route plus the three members the platform adds (teamIdand the two ADR-0105 D8 placement fields). The schemas are plain, non-strict objects, so those three extra keys are stripped on parse rather than refused — which is what makes the relay claim honest about the wire being a superset.return-type-precision.test.ts:1050flipped from the stored-text union to the decoded object. Two@ts-expect-errordirectives there had to go with it, because the shapes they suppressed are now legal — each replaced by a positive pin rather than deleted:updatedAtoff the delete answer is now the spec's optional ISO string (with the measured note that optional is the ACCEPT set, and the value is absent on every route of this family);metadataon a read route is now legal, so the direction-2 suppression moved toJSON.parseof it — the thing that is now refused.Leg 3 — ⭐ the negative control
"The client now relays the spec schemas" and "the client stopped validating" look identical from a green positive test, so
packages/client/src/identity-wire-relay.test.tsruns the realsafeParseover the measured bodies and pairs every accepted one with a refused one. Each refusal asserts the issue path, not merely that it failed:updatedAtabsent,metadatadecodedmetadatakey absentslugremovedslugmetadataas the stored JSON textmetadatametadata: nullmetadatanullis not quietly admittedcreatedAt: 'yesterday'createdAtupdatedAt: 'whenever', on all three schemasupdatedAt.optional()widened by ABSENCE only; a present value is still held to the datetime checkuserIdremoved /inviterIdremoved /status: 'withdrawn'The same discipline lands in
packages/spec/src/identity/organization.test.ts, whose #18509 scope-fence pin asked in writing that 「a later fix for either one has to come here and say so」. It is taken down and answered: the served body now parses whole, and thenullthe schema still refuses is a shape nothing sends any more.Verification
Everything below is a foreground run in this worktree at the head of this branch; heavy runs went through
scripts/pm/os-verify-lock.shand the verdict quoted is the one it printed.scripts/ablation-replace.mjs, which proved the mutation landed (anchor 1 to 0, blob6346e2ba97a5to25449f6b3393) — turned exactly the three decode pins red and left the other eight green, then restored and proved the restore (blob equals HEAD,git diff HEADempty).return-type-precision.test.ts(1050,83)TS2344 and two TS2578 unused-suppression errors at:1068and:1076. The other 54 errors in that run were TS2307 "cannot find module" from an unbuilt workspace and are gone after a build.organization.test.ts's scope-fence pin and on nothing else.@objectstack/plugin-auth113 files / 2376 tests;@objectstack/spec493 files / 14521 tests (1 skipped, an environment-conditional skip in the suite's project split — there is nodescribe.skiporskipIfin the spec sources);@objectstack/client48 files / 566 tests. Typecheck green on all three, including each one's test layer.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran RECORD-FILEreports 116 derived, 116 run, 0 NOT-MEASURED, 0 UNRUN, with an exit code recorded per family. Two needed a second pass and are green now:check:skill-examplesrefused twice on adistolder thansrc(the good refusal — it declines to reach a verdict it cannot read) and passed after building the package directly;check:engine-double-contractasked for the new fake'supdateto route throughassertEngineUpdateDispatchand for its ledger row, both of which it now has.pnpm lint(eslint . --no-inline-config): exit 0.packages/specartefacts:check:generatedreports all 16 up to date, on a tree rebuilt after the last source edit.check:nul-bytesclean over 8954 files, plus a direct scan of this diff's own files.Acceptance notes
rethrowAsBetterAuthErrormaps engine errors code-by-code, so each new engine code needs its own incident before it stops leaking as a bodyless 500 #7881 intersection, answered. This PR's producer fix does touchpackages/plugins/plugin-auth/src/objectql-adapter.ts, the declared trigger file of on-hold card [finding]rethrowAsBetterAuthErrormaps engine errors code-by-code, so each new engine code needs its own incident before it stops leaking as a bodyless 500 #7881 (rethrowAsBetterAuthErrormaps engine errors code-by-code). That hold's restart condition is an incident count, not a file touch, so the trigger is unaffected and nothing here advances or satisfies it. ⛔ Its generalisation was deliberately not attempted, and no bodyless-500 path was measured on the four identity read routes fixed here.(narrowing)arm of the clause-② pair; neither holds. Themetadatahalf is a producer brought into line with a contract this package has published all along, and the client's own comment called the served text 「not relayed」 rather than a shape anyone was promised. The changeset therefore carries no ADR-0087 disposition —check-adr-0087-registrationagrees, reporting 「this PR adds no declared-breaking changeset」 — while still shipping the one-line consumer note and the compiler as its delivery channel. ⭐ Called out because it is the one judgement in this diff the contract review should confirm rather than inherit.AUTH_ORGANIZATION_SCHEMAcarries a dead field mapping.packages/plugins/plugin-auth/src/auth-schema-config.ts:185mapsupdatedAttoupdated_atfor the organization model, and better-auth's organization model declares noupdatedAtfield at all — so the mapping sets afieldNamefor a field the transform never iterates. It is inert rather than wrong, and out of this card's scope; reported as a finding.packages/platform-objects/**(the two missingupdated_atcolumns are reported above, not added),content/docs/releases/**,packages/spec/src/identity/identity.zod.ts, and PR fix(spec): UserSchema.image and OrganizationSchema.logo accept null, the shape better-auth serves #18718's nullish arm.needs:contract-reviewis the seat's label — ⛔ this PR neither attaches nor clears it, and the report states what the PR carries plus thecheck-clause2-carriers.mjs --pairexit code.Generated by Claude Code