Skip to content

fix(plugin-auth,spec,client): the identity read routes serve what the spec declares - #19122

Merged
os-elon-musk merged 8 commits into
mainfrom
claude/issue-18728-identity-wire-relays-spec
Sep 18, 2026
Merged

os-elon-musk merged 8 commits into
mainfrom
claude/issue-18728-identity-wire-relays-spec

Conversation

@os-elon-musk

Copy link
Copy Markdown
Collaborator

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) — updatedAt moves from required to optional on three published schemas, so the set a consumer may hand to OrganizationSchema / MemberSchema / InvitationSchema grows 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-②: no conditionally — 「unless fallback A moves a declaration」. Fallback A fired and moved three, so the condition resolves to yes.


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 (updatedAt optional there) and the reason is written on the card; metadata is 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).

route who serialises the response evidence documented updatedAt? metadata before
setActivePOST /organization/set-active better-auth's own handler, ctx.json(organization) from findOrganizationById crud-org.mjs:379 handler, :427 the answer; adapter.mjs:400 the read no stored JSON text
getGET /organization/get-full-organization better-auth's own handler, ctx.json(organization) from findFullOrganization crud-org.mjs:336, :371; adapter.mjs:424 no stored JSON text
deletePOST /organization/delete better-auth's own handler, ctx.json(org) from findOrganizationById crud-org.mjs:239, :291; adapter.mjs:400 no stored JSON text
listGET /organization/list better-auth's own handler, ctx.json(organizations) from listOrganizations; the organization arrives through the adapter factory's fallback join, itself another findOne on this model crud-org.mjs:436, :455; adapter.mjs:474; @better-auth/core dist/db/adapter/factory.mjs handleFallbackJoin no stored JSON text
createPOST /organization/create better-auth's handler; its organization adapter decodes the echo itself adapter.mjs:141, decode at :152 no decoded already
updatePOST /organization/update better-auth's handler; same, via parseJSON adapter.mjs:352, decode at :367 no decoded already

Three mechanisms make that table a reading rather than an impression:

  1. The routes are the vendor's, mounted through one catch-all. packages/plugins/plugin-auth/src/auth-route-ledger.ts:257-277 books every organization/* row source: 'better-auth', and the ledger's own header states the mount is rawApp.all(basePath + '/*') with no per-route registration. Each handler answers ctx.json(...) directly; there is no ObjectStack post-processing between the adapter and the wire.
  2. The vendor's declared model has no updatedAt, and its output transform emits declared fields only. dist/plugins/organization/organization.mjs:705 declares organization as name / slug / logo / createdAt / metadata; member as organizationId / userId / role / createdAt; invitation as organizationId / email / role / teamId / status / expiresAt / createdAt / inviterId. @better-auth/core dist/db/adapter/factory.mjs:144 transformOutput iterates for (const key in tableSchema) — an undeclared column is dropped before any route sees it. filterOutputFields (@better-auth/core dist/utils/db.mjs:6) then removes only additionalFields marked not-returned.
  3. Lit control, same file and same grep. updatedAt occurs exactly twice in organization.mjs:617 on team and :694 on organizationRole, both of which DO declare it. So the zero on organization / member / invitation is a reading, not a grep artefact. A second control one level up: better-auth's core user and account models do declare updatedAt (@better-auth/core dist/db/get-tables.mjs:185 and :270), which is why packages/spec/src/identity/identity.zod.ts is 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_at on the wire as updatedAt (the column exists)」. Measured: the column exists on sys_organization only.

object updated_at column evidence
sys_organization yes packages/platform-objects/src/identity/sys-organization.object.ts:362
sys_member no sys-member.object.ts declares id / created_at / organization_id / user_id / role — nothing else
sys_invitation no sys-invitation.object.ts declares id / created_at / organization_id / email / role / status / inviter_id / expires_at / team_id / business_unit_id / positions

And the audit family is not injected onto any of the three: all three declare managedBy: 'better-auth', which is the single disposition under which resolveInjectedSystemColumns (packages/spec/src/data/injected-system-columns.ts) returns the empty plan — audit family included. sys_organization has updated_at because it declares the field itself.

⇒ for Member and Invitation, 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:

card's citation on this base (07c6f822e) still true?
organization.zod.ts:57 / :105 / :183 required updatedAt :89 / :137 / :215 yes — all three required before this PR
identity.zod.ts:55 / :142 required updatedAt :86 / :173 yes, and deliberately left alone
three 「not relayed」 notes at client :1213 / :1249 / :1335 :1212 / :1248 / :1335 yes
return-type-precision.test.ts:1050 pins `string null undefined`
zero in-repo consumers of the three schemas zero yes — every hit is a generated artefact, a CHANGELOG or docs prose. Lit control: ObjectSchema has real consumers in packages/cli/src/commands/. ⭐ After this PR the count is no longer zero: @objectstack/client consumes 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/)

  • New src/organization-metadata-decode.ts — decodes sys_organization.metadata from stored JSON text into an object, and OMITS the key when the column is unset (null, empty, or a stored null literal), 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.
  • Wired into 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.
  • Deliberately NOT the write verbs. better-auth's own organization adapter decodes the create / update echoes 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's metadata to undefined — a regression that reads as "unset". Both directions are pinned.
  • Measured reason this could not be done by declaration instead: the adapter declares supportsJSON: true (objectql-adapter.ts:827), so transformOutput's JSON branch — which fires only for a field typed json on an adapter declaring supportsJSON: false — is unreachable here, and the vendor types metadata as a string anyway.

Spec (packages/spec/src/identity/organization.zod.ts, fallback A)

  • OrganizationSchema.updatedAt, MemberSchema.updatedAt, InvitationSchema.updatedAt become .optional(). .optional() and not nullish: the key is absent on the wire, never null. Each carries the measurement and the ruling's own words in its docblock.
  • identity.zod.ts untouched — the vendor declares updatedAt on user and account, so no fallback applies there.
  • ⛔ PR fix(spec): UserSchema.image and OrganizationSchema.logo accept null, the shape better-auth serves #18718's image / logo nullish arm is untouched, neither extended nor reverted.
  • Regenerated by the sanctioned producers only (check:generated --fix ran gen:api-surface-declarations and gen:docs): packages/spec/api-surface-declarations/identity.txt and content/docs/references/identity/organization.mdx, three declaration moves each, nothing else.

Client (packages/client/)

  • The three 「not relayed」 notes are gone, and the wires are relayed rather than transcribed: OrganizationWire is the spec's Organization, OrganizationMemberWire is Member, and OrganizationInvitationWire is Invitation with status narrowed per route plus the three members the platform adds (teamId and 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:1050 flipped from the stored-text union to the decoded object. Two @ts-expect-error directives there had to go with it, because the shapes they suppressed are now legal — each replaced by a positive pin rather than deleted:
    • reading updatedAt off 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);
    • reading into metadata on a read route is now legal, so the direction-2 suppression moved to JSON.parse of 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.ts runs the real safeParse over the measured bodies and pairs every accepted one with a refused one. Each refusal asserts the issue path, not merely that it failed:

body verdict why it is the control
the served read-route body, updatedAt absent, metadata decoded accepted the relay itself
same, metadata key absent accepted an unset column
slug removed refused, path slug a genuinely required field is still required
metadata as the stored JSON text refused, path metadata the exact dimension the producer fix moves — this one distinguishes "the producer decodes" from "the schema stopped caring"
metadata: null refused, path metadata the producer omits; null is not quietly admitted
createdAt: 'yesterday' refused, path createdAt the datetime check is live
updatedAt: 'whenever', on all three schemas refused, path updatedAt .optional() widened by ABSENCE only; a present value is still held to the datetime check
userId removed / inviterId removed / status: 'withdrawn' refused, each path named the other two schemas, same discipline

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 the null the 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.sh and the verdict quoted is the one it printed.

  • Red before the change, on the producer. An ablation of the decode assignment — written to disk by scripts/ablation-replace.mjs, which proved the mutation landed (anchor 1 to 0, blob 6346e2ba97a5 to 25449f6b3393) — turned exactly the three decode pins red and left the other eight green, then restored and proved the restore (blob equals HEAD, git diff HEAD empty).
  • Red before the change, on the client. Before the flip, the client's test project reported exactly three errors of mine: return-type-precision.test.ts(1050,83) TS2344 and two TS2578 unused-suppression errors at :1068 and :1076. The other 54 errors in that run were TS2307 "cannot find module" from an unbuilt workspace and are gone after a build.
  • Red before the change, on the spec. The full spec suite failed on organization.test.ts's scope-fence pin and on nothing else.
  • Green after: @objectstack/plugin-auth 113 files / 2376 tests; @objectstack/spec 493 files / 14521 tests (1 skipped, an environment-conditional skip in the suite's project split — there is no describe.skip or skipIf in the spec sources); @objectstack/client 48 files / 566 tests. Typecheck green on all three, including each one's test layer.
  • The gate families this diff derives were run and accounted: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran RECORD-FILE reports 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-examples refused twice on a dist older than src (the good refusal — it declines to reach a verdict it cannot read) and passed after building the package directly; check:engine-double-contract asked for the new fake's update to route through assertEngineUpdateDispatch and for its ledger row, both of which it now has.
  • Repo-wide pnpm lint (eslint . --no-inline-config): exit 0.
  • packages/spec artefacts: check:generated reports all 16 up to date, on a tree rebuilt after the last source edit.
  • Control bytes: check:nul-bytes clean over 8954 files, plus a direct scan of this diff's own files.

Acceptance notes

  • Hold [finding] rethrowAsBetterAuthError maps 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 touch packages/plugins/plugin-auth/src/objectql-adapter.ts, the declared trigger file of on-hold card [finding] rethrowAsBetterAuthError maps engine errors code-by-code, so each new engine code needs its own incident before it stops leaking as a bodyless 500 #7881 (rethrowAsBetterAuthError maps 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.
  • Not declared breaking, and the reason is the repo's own criterion rather than the level being convenient. AGENTS.md binds the breaking class to removing or renaming something an author can write, and to the (narrowing) arm of the clause-② pair; neither holds. The metadata half 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-registration agrees, 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_SCHEMA carries a dead field mapping. packages/plugins/plugin-auth/src/auth-schema-config.ts:185 maps updatedAt to updated_at for the organization model, and better-auth's organization model declares no updatedAt field at all — so the mapping sets a fieldName for a field the transform never iterates. It is inert rather than wrong, and out of this card's scope; reported as a finding.
  • Left deliberately untouched: packages/platform-objects/** (the two missing updated_at columns 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-review is the seat's label — ⛔ this PR neither attaches nor clears it, and the report states what the PR carries plus the check-clause2-carriers.mjs --pair exit code.
  • The PR opens as a draft and stays draft.

Generated by Claude Code

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 18, 2026
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/client, @objectstack/plugin-auth, @objectstack/spec, touching 24 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/spec/api-surface-declarations/identity.txt), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

29 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 14a762f9ff2e8f7d76b3e6138473fe0376be3093.

6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/spec/api-surface-declarations/identity.txt) — pages documenting those are invisible to this run
  • 1 cross-cutting symbol(s) contributed no route anchor: organizationId (7 routes)
  • 8 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 142 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 14a762f9ff2e8f7d76b3e6138473fe0376be3093packageMentionDocs.

Which tree this was computed on

This run read content/docs from d2c923f2b3595ab344c2026d69a786df47736a48 — the merge of head 1bbd154dbf7501dfea39fd5392acaf3b9484b665 into base 14a762f9ff2e8f7d76b3e6138473fe0376be3093, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 14a762f9ff2e8f7d76b3e6138473fe0376be3093 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator Author

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 · 2026-09-18T21:37Z

Seat: domain:spec#3

⛔ Why this is a comment and not an ## Acceptance notes edit

The dev declined to PATCH this PR's body and declared the conflict instead of choosing silently — correct on two counts, and the second one is the seat's error:

  1. .claude/agents/os-dev.md:56 — 「PR 正文 dev 只写一次,在开 PR 那一笔,⛔ 不 PATCH」, and :184 makes that clause govern over any dispatch word. ⇒ this seat's 21:18Z message asking it to 「state in ## Acceptance notes」 was the defective instruction.
  2. AGENTS.md:432, verbatim: 「Send the footer you want stored, read the body back after every write, and ⛔ never re-send a body that already carries an appended footer. Durable attribution lives in body prose or a comment.」 This body already carries its session-URL footer.

⚠️ This seat violated that rule twice today before reading it — it PATCHed the bodies of PR #19112 and PR #19115, each of which already carried a footer, and each ended with two. ⛔ Not repaired: a third write would add a third footer, and the rule's own remedy is 「a comment」, which is what this is. Recorded rather than quietly left.

The answer, as the dev measured it (its text, published by the seat)

Docs Drift Check (bot comment 5736281398) answered on the bot own tree: refs/pull/19122/merge resolves to d8cdd0a, the exact commit the bot names, and the re-derivation there returns the same 35 rows (29 hand-written + 6 release-owned). All 29 hand-written pages are STILL ACCURATE and none was corrected. Uniform reason, measured: every anchor that listed a page is one of seven FIELD anchors on the three wire declarations this diff converted from interface to a relayed spec type (userId in 20 of 29 rows, plus organizationId, createdAt, expiresAt, inviterId, teamId, businessUnitId), or a route/SDK anchor bridged from one of them, or a new package-internal symbol no page documents. The conversion moved every field declaration SITE; it moved no field TYPE — before, from the bot base: userId/organizationId/createdAt/expiresAt/inviterId string, teamId string-or-null, businessUnitId optional string-or-null; after, from the rebuilt spec declarations: identical, with teamId and businessUnitId carried verbatim in the intersection. So no page can be falsified through them. The bridged approvals and import-job anchors belong to surfaces this diff does not touch. Hand re-read, owed because the bot could not cover it and done over the whole hand-written tree rather than over the 29 rows: the two members that actually moved (metadata, updatedAt) produced ZERO rows, metadata being in the run own weakAnchorsDropped list as too generic. Zero hand-written pages document updatedAt at all; zero document the organization wire shape; the three hits for the org routes are about addMember being server-only and about the organization/create tenancy gate, neither touched here. Declared blind spot closed: zero hand-written pages mention api-surface-declarations, so the uncovered set is empty. The 6 release-owned pages are read-only and were not edited; none names any identity wire shape, not relayed, or updatedAt, and this change is unreleased, so none is falsified.

⛔ The seat did not re-verify the 29 rows itself: what it verified is that the dev answered them on the bot's own tree (refs/pull/19122/merged8cdd0a0a, the commit the bot names) rather than on a stale main, and that the two members which actually moved (metadata, updatedAt) produced zero anchors, which is why the hand re-read was owed at all. ⇒ the reading above is the dev's, attributed, ⛔ not adopted as the seat's own measurement.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Contract review

Served-tier: 98/98 CONTRACT_REVIEW_TIER
Head-sha: 485138e97

① Derived judgments

Accept set — packages/spec/src/identity/organization.zod.ts. Exactly three moves, each z.string().datetime() to z.string().datetime().optional(): OrganizationSchema.updatedAt (:116), MemberSchema.updatedAt (:180), InvitationSchema.updatedAt (:267). Nothing else in the accept set moves: metadata stays z.record(z.string(), z.unknown()).optional() (:79), still refusing null and the stored JSON text; identity.zod.ts (User, Account) is untouched. The widening is by ABSENCE only — a present updatedAt is still held to the datetime check — and both halves are pinned with issue paths in organization.test.ts and in the client's identity-wire-relay.test.ts. Nothing previously accepted is now refused: every refusal the diff adds was already a refusal before it. Declaration Clause-②: yes (widening) matches what the diff does.

Published surface. api-surface/identity.json (names and kinds) is unchanged. api-surface-declarations/identity.txt IS a published artifact: packages/spec/package.json lists api-surface-declarations in files[] on main (:243, beside api-surface at :242), so the +3/−3 (three ZodString to ZodOptional of ZodString) ships in the npm tarball. It is consistent with the source and was regenerated by the sanctioned producer. dist and src/**/*.zod.ts (both in files[]) carry the same three moves. content/docs/references/identity/organization.mdx is the docs site, not the tarball.

Client relay — packages/client/src/index.ts, published SDK declarations. OrganizationWire = Organization (:1231), OrganizationMemberWire = Member (:1270), OrganizationInvitationWire = Invitation minus status, intersected with status narrowed per route plus teamId / businessUnitId / positions (:1363). The three 「not relayed」 notes are gone; every export name is kept (interface to type alias). Consumer-visible type deltas: OrganizationWire.metadata from string | null | undefined to the string-keyed record or absent; updatedAt?: string added on all three wires. Pinned at return-type-precision.test.ts :1055 / :1063 / :1090 and at runtime by identity-wire-relay.test.ts (InvitationSchema strips the three platform keys rather than refusing them — the relay claim is honest about the wire being a superset).

⛔ The FAIL, and its whole scope. Three docblocks on that same published SDK surface still declare the OLD wire and were not touched: index.ts :1408–1409 (OrganizationFullWire: 「metadata as stored JSON text, see OrganizationWire」), :3647–3648 (setActive: 「Answers the organization row as STORED (metadata is the JSON text …)」), :3665 (get: 「metadata is the stored JSON text here (see OrganizationWire)」). All three pre-exist on main (:1396 / :3634 / :3652). After this diff the .d.ts types metadata as the decoded record while the JSDoc on the very methods that return it tells the same caller it is text — and the changeset names JSON.parse of that value as the line that now breaks. That is a published face contradicting the contract this PR exists to make whole, and it is the class the card was filed on: SDK comments recording a wire that diverges from the spec. Scope of the FAIL is those three docblocks and nothing else — the accept set, the level and the producer below need no re-measurement; a prose-only follow-up on that file is enough to re-review (:3777 「as it was stored」 is ambiguous rather than false; the fixer's call).

Producer — packages/plugins/plugin-auth, not packages/spec. decodeOrganizationMetadataOnRead is wired into the adapter's findOne (objectql-adapter.ts :922) and findMany (:965) for SystemObjectName.ORGANIZATION only; the new module is not re-exported, so no published declaration of plugin-auth changes. What changes is the served wire of a published server package on the four read routes: text to object, null / empty to key ABSENT. It touches no schema; the contract it touches is the spec's already-published metadata declaration, now honoured. The write verbs are correctly left alone (the vendor's own createOrganization / updateOrganization decode their echoes and discriminate on the value still being a string — adapter.mjs :152 / :367).

② Semver level

minor on all three packages is the right level. Spec: yes (widening) takes at least minor (AGENTS.md:1068); the (narrowing) arm is absent; nothing an author can write is removed or renamed (:1069) — these three schemas are not metadata types. Client: no export removed or renamed; one published member changes type and one is added. plugin-auth: a behaviour change on the served wire, package-internal module. major is refused pre-GA regardless.

The BREAKING banner was withdrawn in 118525bb9 before the gate ran, and the withdrawal is consistent with AGENTS.md:1068–1069's letter and with breakingDeclaration()'s four signals in check-adr-0087-registration.mjs (major, **BREAKING, a !: summary, the (narrowing) arm) — none present; CI's Check Changeset is green on this head. Judged for myself: the level is right and the banner is not owed under the written criterion. ⚠️ But the runtime change is real for an external SDK caller who followed the former docblock — JSON.parse on an object throws — and it is carried in the changeset's prose and before/after snippet rather than a banner. Under the repo's rules that is the correct carrier; a future reader must not read 「not declared breaking」 as 「no consumer-visible behaviour change」.

③ Boundary flags

  • Tier, at its granularity. The stamp control above counts assistant-message lines in this reviewer's own transcript (agent-abdef46d7a1af93e8.jsonl, confirmed to open with this review's prompt), every one stamped claude-fable-5-1, over the distinct request ids named in the report — ⛔ it is a count of lines, not of requests. Compared by exact string against CONTRACT_REVIEW_TIER at scripts/pm/dispatch-gates.mjs:11899.
  • The ruling's parenthetical is false on two of three — verified. sys_member (sys-member.object.ts :231–291: id / created_at / organization_id / user_id / role) and sys_invitation (sys-invitation.object.ts :187–271: id / created_at / organization_id / email / role / status / inviter_id / expires_at / team_id / business_unit_id / positions) declare no updated_at; sys_organization does (:362). All three are managedBy: 'better-auth' (:19 / :20 / :24), and resolveInjectedSystemColumns returns the empty plan for that disposition (injected-system-columns.ts :156). ⇒ on Member and Invitation the ruling's main arm is impossible without a platform-objects column, which was out of surface and is filed as a finding. This does not change the clause-② verdict: fallback A's own condition is met on all three independently.
  • Fallback-A measurement — verified against the installed better-auth 1.7.3. dist/plugins/organization/organization.mjs declares organization as name / slug / logo / createdAt / metadata, member as organizationId / userId / role / createdAt, invitation without updatedAt; updatedAt occurs exactly twice in that file, :617 (team) and :694 (organizationRole) — the lit control holds. @better-auth/core dist/db/get-tables.mjs :185 (user) and :270 (account) declare it — the second control holds, and it is why identity.zod.ts is untouched. transformOutput iterates for (const key in tableSchema) (dist/db/adapter/factory.mjs :152). The four read routes are vendor endpoints (every organization/* row but add-member is source: 'better-auth' in auth-route-ledger.ts; mount auth-plugin.ts :2984) answering ctx.json (crud-org.mjs :291 / :372 / :433 / :452).
  • ⚠️ Not licensed: reading this PR as 「updatedAt cannot be served on the organization wire」. For sys_organization the column exists AND the vendor's organization model takes additionalFields (organization.mjs :737), a seam this platform already uses on invitation (auth-schema-config.ts :253). So the ruling's primary arm was mechanically available for OrganizationSchema; fallback A applies there by the ruling's literal condition (the vendor's documented shape), not by impossibility. Whether the maintainer wants the primary arm on organization is a question for the seat, not decided here. Related: auth-schema-config.ts :188 maps updatedAt to updated_at on a model that declares no such field — inert, and it reads as if the key were on the wire (the dev's own finding).
  • The list leg — source-read was sufficient, and I executed the vendor half. With the installed 1.7.3 dist (createAdapterFactory + organization() + getOrgAdapter) and a stub custom adapter recording verbs, listOrganizations issued findMany(sys_member) with NO join passed to the adapter, then findOne(sys_organization) through handleFallbackJoin (plugin-auth enables no advanced.database.joinsauth-manager.ts :2495 sets cookies and CSRF only); transformSingleOutput kept metadata exactly as the adapter returned it (object with the seam, string in the no-decode control) and emitted neither updatedAt nor updated_at. The ObjectQL half — findOne running the decode — is the dev's pin through the real factory, green in CI on this head (Test Core 1–6). ⛔ Not run here: the ObjectQL adapter itself (this checkout has no node_modules), the HTTP route end to end, check:generated.
  • identity.txt sits on a path open PR revert(spec): take back the declaration-text snapshot, restore the 27 signature hashes #19024 deletes wholesale — all 17 api-surface-declarations/*.txt, the files[] entry, the gen/check scripts, replaced by api-surface-signatures.json. Whichever lands second takes a modify/delete conflict; if revert(spec): take back the declaration-text snapshot, restore the 27 signature hashes #19024 lands first, this PR owes a regenerate under the new producer instead of the .txt hunk. A reason to rebase deliberately, not to hold.
  • Not measured. No live server or npm pack; the docs-drift rows are the dev's, attributed by the seat, not re-verified here; the one pending commit status on the head is Vercel's deploy, not a code check; User/Account wires not probed.
  • Written 2026-09-18T21:53Z. Ruling C read at its own stamp 2026-09-18T11:14Z; the head's last CI check completed 2026-09-18T21:41:49Z.

Implemented-by: claude/issue-18728-identity-wire-relays-spec
Reviewed-by: session_019srGWGCBBCBHqcDoRZpQRh

VERDICT: FAIL


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Contract review verdict on this head — FAIL (scoped). Carriers stay.

Seat domain:spec#3, recorded 2026-09-18T21:57Z.

Review of record: comment 5736639314, taken by an isolated at-tier review subagent (this seat measured BELOW CONTRACT_REVIEW_TIER, so it may not self-review a clause-② verdict). Served-tier: reads 98/98 against CONTRACT_REVIEW_TIER = 'claude-fable-5-1' (scripts/pm/dispatch-gates.mjs:11899). Granularity, because it has bitten this seat before: 98 assistant-message lines in the reviewer's own transcript over 22 distinct request ids — not 98 requests. check-clause2-carriers.mjs --pair 19122 exits 0 and rows the record as C6-RECORD on 485138e97.

Clause ② itself passes: the declaration yes (widening) is what the diff does, minor x3 is the right level, fallback A's measurement was re-taken independently and stands, and the producer fix is correct. The FAIL is confined to three docblocks on published SDK surface.

What must change (read first-hand by this seat on 485138e97 at 2026-09-18T21:54Z)

This PR rewrote OrganizationWire's docblock (packages/client/src/index.ts:1206-1213) to say metadata is DECODED on setActive, get, delete and list. Three docblocks that link to it still declare the old wire:

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

Three flags the review raised — recorded, not fixed here

  1. sys_organization could mechanically have taken the ruling's primary arm. The vendor's additionalFields seam (dist/plugins/organization/organization.mjs:737) is already used for invitation, so for sys_organization — the one of the three objects that does declare updated_at — fallback A applies by the ruling's literal condition (the documented wire carries no updatedAt), not because the primary arm was impossible. On sys_member and sys_invitation it IS impossible without a packages/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 that updatedAt is on the identity wire — and one is a parenthetical inside ruling C (sys_member / sys_invitation have no updated_at column at all) #19125.
  2. Published-artifact collision. packages/spec/api-surface-declarations/identity.txt is in files[] 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.
  3. A real runtime change carried in prose. The served wire for metadata changes 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, and minor is 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>

Copy link
Copy Markdown
Collaborator Author

Contract review

Served-tier: 112/112 CONTRACT_REVIEW_TIER
Head-sha: 1bbd154db

① Derived judgments

Scope of this record. Re-review on the head that answers my scoped FAIL (record 5736639314 on 485138e97; the seat's disposition 5736681568). That record confined the FAIL to three docblocks in packages/client/src/index.ts and said a prose-only follow-up on that file was sufficient to re-review; this is that follow-up, judged on the real diff 485138e97..1bbd154db, ⛔ not on anyone's narration.

What moved between the two heads — read in full. One commit, one file, packages/client/src/index.ts, +9/−8, comment text only: no type, no export, no runtime line, no test, no changeset, no other file. OrganizationFullWire (:1408–1409) now reads 「the row (metadata decoded, see OrganizationWire)」; organizations.setActive (:3647–3648) 「Answers the organization row with metadata DECODED (see OrganizationWire)」; organizations.get (:3665) 「metadata is decoded here (see OrganizationWire)」; and the fourth site my record left to the fixer's call, organizations.delete (:3777–3778), now separates which row from what encoding — 「as it stood immediately before deletion (measured; metadata decoded, see OrganizationWire)」. Each statement matches the behaviour the producer half delivers (the adapter's read verbs decode sys_organization.metadata and omit it when unset) and the type the relay declares (OrganizationWire = Organization, still at :1231; OrganizationMemberWire = Member :1270; OrganizationInvitationWire :1363; OrganizationFullWire extends OrganizationWire :1414 — all intact).

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 OrganizationWire docblock describing the decode — 「from its stored JSON text into an object」, which is the correct sentence), :5726 (secret hashing). The published SDK prose now says one thing about metadata, and it is what the wire carries.

Whole-PR surface, re-checked. git diff --stat origin/main...1bbd154db is still the same 12 files; only index.ts's line delta changed. The accept set (three updatedAt moves to .optional()), the published artifacts (api-surface-declarations/identity.txt +3/−3, dist, src/**/*.zod.ts), the relay and the producer are byte-identical to what 5736639314 judged and need no re-measurement; that record's ① stands as written for everything but the FAIL paragraph, which this head closes.

② Semver level

Unchanged and still right: minor on @objectstack/spec, @objectstack/client, @objectstack/plugin-auth. The follow-up adds no changeset, and none is owed: a docblock edit publishes no new behaviour and changes no declaration. The banner reading in 5736639314 ② stands (not owed under AGENTS.md:1068–1069; the runtime wire change is carried in the changeset's prose, and a reader must not equate 「not declared breaking」 with 「no consumer-visible behaviour change」).

③ Boundary flags

  • Tier, at its granularity. The stamp control above was re-taken on THIS turn from this reviewer's own transcript (agent-abdef46d7a1af93e8.jsonl), counting assistant-message lines each stamped claude-fable-5-1, over the distinct request ids named beside it in the hand-back report — ⛔ lines, not requests; ⛔ not the earlier 98/98. Exact-string match against CONTRACT_REVIEW_TIER at scripts/pm/dispatch-gates.mjs:11899.
  • CI on this head was in flight when read: Check Changeset and Type Check · source gates green (2026-09-18T22:19:10Z for the latter); Test Core 1–6, Lint & Repo Gates and the remaining Type Check jobs in progress. A comment-only diff cannot move them, and the previous head's 39 runs were all green or skipped, but ⛔ this record does not claim green on jobs it did not see finish.
  • Every flag in 5736639314 ③ carries over unchanged: the ruling's parenthetical false on sys_member / sys_invitation (column absent, managedBy: 'better-auth', empty injection plan); the primary arm mechanically available on sys_organization via the vendor's additionalFields seam — fallback A licensed there by the ruling's literal condition, not by impossibility; the inert updatedAt mapping at auth-schema-config.ts:188; the list leg executed on the vendor half; identity.txt on a path PR revert(spec): take back the declaration-text snapshot, restore the 27 signature hashes #19024 deletes wholesale (rebase deliberately if revert(spec): take back the declaration-text snapshot, restore the 27 signature hashes #19024 lands first).
  • Not measured here: no tests or typecheck run in this checkout (no node_modules); the dev's 「566 tests / typecheck exit 0」 is a claim this record neither adopts nor needs — nothing in the inter-head diff is executable.
  • Written 2026-09-18T22:20Z.

Implemented-by: claude/issue-18728-identity-wire-relays-spec
Reviewed-by: session_019srGWGCBBCBHqcDoRZpQRh

VERDICT: PASS


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants