fix(spec): UserSchema.image and OrganizationSchema.logo accept null, the shape better-auth serves - #18718
Conversation
…the shape better-auth serves
Both were `z.string().url().optional()` — a URL string or the key absent,
`null` refused. Both columns are better-auth-owned and nullable
(`sys_user.image` / `sys_organization.logo` are each
`Field.url({ required: false })`, reaching SQLite as `varchar(255)` with
`notnull=0`), and better-auth SELECTs them and serialises them
present-and-null for a user who never set an avatar and an organization
created without a logo.
Measured through a real `AuthManager` (better-auth 1.7.3) over a real
`ObjectQL` on a real `SqliteWasmDriver`, with the platform's own object
definitions — not inferred from the sibling ruling:
/auth/sign-up/email -> user.image = null
/auth/get-session -> user.image = null
/auth/organization/create -> logo = null
/auth/organization/list -> [0].logo = null
/auth/organization/get-full-organization -> logo = null
-> members[].user.image = null
`.nullish()`, not `.nullable()`: the key's absence is a legal shape today,
so `.nullable()` would retire a live shape as the price of admitting null.
`.url()` is kept and does not fight `null` — `.nullish()` wraps the whole
`z.string().url()`, so null and undefined are branches the URL check never
sees while a present string must still be a well-formed URL. Of six inputs
(absent / null / '' / a URL / a non-URL / a number) exactly one row moves.
Co-authored-by: Claude <[email protected]>
Claude-Session: https://claude.ai/code/session_01JbZnqu8bt6YqfJsr9vaFb3
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 136 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 4dc4c0486bd12a504cf30267d771b3884ed1e601 && git checkout 4dc4c0486bd12a504cf30267d771b3884ed1e601
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin df1b275c71fb7939f085c013779f3daefa5114c7 2d3dea2a59f0c254b8ede6b14b1e77b1710bc670 && git checkout -B drift-repro df1b275c71fb7939f085c013779f3daefa5114c7 && git merge --no-ff 2d3dea2a59f0c254b8ede6b14b1e77b1710bc670
node scripts/docs-audit/affected-docs.mjs --json df1b275c71fb7939f085c013779f3daefa5114c7
|
The docblocks were written before the PR existed and cited a guessed number. No assertion changes. Co-authored-by: Claude <[email protected]> Claude-Session: https://claude.ai/code/session_01JbZnqu8bt6YqfJsr9vaFb3
Contract reviewServed-tier: 119/119 Tier evidence: every ① Derived judgmentsWhat moves, row by row — accept set and public surface:
The head column moves exactly one row from base —
Parses on the served bodies:
② Semver levelChangeset ③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
Fixes #18509
Clause-②: yes (widening)
Verdict: it reproduces. Both of them.
#18509 asked for a measurement, not a widening, and warned that "does not reproduce" would be the good outcome. It is not the outcome. Measured through a real
AuthManager(better-auth 1.7.3) over a realObjectQLon a realSqliteWasmDriver, with the platform's ownsys_user/sys_organizationdefinitions, both keys are served present-and-null:POST /auth/sign-up/emailuser.imagenullGET /auth/get-sessionuser.imagenullPOST /auth/organization/createlogonullGET /auth/organization/list[0].logonullGET /auth/organization/get-full-organizationlogonullGET /auth/organization/get-full-organizationmembers[].user.imagenullThe mechanism is the #17235 one, confirmed at the DDL layer rather than assumed. Both columns are
Field.url({ required: false })in the platform's own object definitions and reach SQLite as nullable columns —PRAGMA table_inforeportssys_user.imageasvarchar(255) notnull=0andsys_organization.logoasvarchar(255) notnull=0. better-auth SELECTs them and serialises the null.So the remedy is the ruled one, per the card's item 2:
.nullish()— not.nullable(), which would retire the legal "key absent" shape.The two controls
LIT — the probe fires.
SessionUserSchema.image, the declaration #17235 fixed, is a field known to be served present-and-null. It shows up under this probe:*** PRESENT-AND-NULL ***on both the sign-up and get-session bodies, andSessionUserSchema.safeParsepasses on them (because #18501 already widened it). A probe that could not see that value could not be trusted to report its absence elsewhere.⭐ The lit control earned its keep — it caught a dead first instrument. The first version of this probe used the in-memory engine shape the plugin-auth suites use (a
Mapof plain objects). Under itimageread ABSENT, not present-and-null, andUserSchema.safeParsepassed — a clean, wrong "does not reproduce". The reason is the whole distinction this card is about: a schemaless store has no columns, so "never set" is key-absent there, while a real nullable column reads back asnull. The LIT control was the only thing that said so. The instrument was replaced with a real store and the result inverted.DARK — what must read 0. The population was re-derived rather than trusted:
grep -rn '^\s*\(image\|avatar\|avatarUrl\|logo\)\s*:\s*z\.' --include=*.zod.ts packages/spec/srcreturns 8 declarations, the same 8 the card reported. This PR moves exactly 2 of them. The other 6 —ai/agent.zod.tsavatar,ui/app.zod.tslogo,api/auth.zod.tsSessionUser.image(already.nullish()) andRegisterRequest.image(a REQUEST surface, client-authored, not better-auth-served),kernel/plugin-registry.zod.tslogo,kernel/plugin-security-advanced.zod.tsimage— appear nowhere in the conclusion and are untouched.⭐ How does
.url()coexist withnull? — the question this card could not answerBoth keys carry
.url();SessionUserSchema.imagedid not. So this is not a copy of #17235 and the answer had to be measured. It was, on all three candidate forms:.url().optional()(today).url().nullish()(ruled remedy).url().nullable()(the shape #17235 refused)invalid_typenullinvalid_type""invalid_formatinvalid_formatinvalid_format"https://x/a.png""not-a-url"invalid_formatinvalid_formatinvalid_format42invalid_typeinvalid_typeinvalid_typeThe answer:
.url()andnulldo not compete, because they never meet..nullish()wraps the wholez.string().url(), sonullandundefinedare separate branches that the URL check never evaluates, while a present string is still required to be a well-formed URL. The middle column moves exactly one row from the left column, and it is the ruled one. The right column moves two rows in opposite directions — that second, upward move is the narrowing #17235 refused, and the table is why the same refusal holds here.⭐ The card's carried boundary note does not come live. #18509 recorded that
SessionUser.imageaccepts""and warned it "becomes live if step 2 adds.url()reasoning to this family". Measured: it does not.SessionUser.imagehas that hole because it is barez.string(); these two keys carry.url(), so""is refused before and after — theinvalid_formatrow is unchanged in every column. Nothing in this PR widens toward the empty string, and the note stays where the card put it.Before / after, on the real bodies
imageis the ONLY divergenceUserSchemahad against the served user, so that body now parses clean.logowas one of three onOrganizationSchema— see the scope fence below.⛔ Scope fence: two further findings, named and NOT fixed here
The same probe found two more divergences on
OrganizationSchema, both out of this card's scope and both reported for separate filing rather than folded in:metadatais served present-and-null./auth/organization/listand/auth/organization/get-full-organizationserve"metadata": nullagainstz.record(z.string(), z.unknown()).optional(). Same present-and-null shape, different key, and az.recordrather than az.string().url()— so it deserves its own reasoning, not this one by extension./auth/organization/createomitsupdatedAt, which the schema declares required. That is the opposite shape — a missing key, not a null one — and the remedy is a different question.Folding either in would be exactly the step #18509 exists to prevent. They are pinned as current behaviour in
organization.test.tsso the fence is visible and a later fix has to come here and say so.Tests
New pin blocks in
packages/spec/src/identity/identity.test.tsandorganization.test.tsassert the whole accept set, not just the row that moved — so a later flip to.nullable()(retiring the absent-key shape) or a drop of.url()(admitting"") goes red here instead of passing as "still accepts null". They assert issue paths, so a refusal is attributed toimage/logoand not to a neighbour, and each block carries a lit control that removes a neighbouring required key and checks the instrument names it.Ablation — the pins can fail. Both declarations were reverted to
.optional()from the committed state; the mutation was proved on disk before any result was read (anchor counts 1 → 0 for the injected text and 0 → 1 for the removed text, plusgit hash-objectdiffering from theHEADblob on both files), and the restore was proved byte-exact the same way (git diff HEADempty; both disk hashes equal to theirHEADblobs). These tests resolve./identity.zodrelatively, i.e. tosrc, not through the packageexportstodist, so no rebuild is interposed and the dist-preflight step does not apply.The 48 that stayed green are the rows that must not move: absent, a valid URL,
"","not-a-url", a number.Verification
pnpm --filter @objectstack/spec testpnpm --filter @objectstack/spec typechecktsc --noEmit+check:scripts-typecheck+check:test-typecheck(the first excludes**/*.test.ts; the third is what covers the new pins)pnpm --filter @objectstack/spec check:generatedgen:docs)pnpm --filter '@objectstack/spec^...' buildpackages/spechas no workspace dependencies, so there is no upstream closure. Reported as empty, not as a pass.check:nul-bytes,check:spec-docblock-symbol-anchors,check:comment-mask-adoption,check:comment-mask-corpus,check:doc-frontmatter,check:docs-section-name,check:keyed-text-bounds,check:pm-widening-tells,check:spec-parsed-alias,check:docs-spec-enumerations,check:doc-anchors,check:empty-changeset— all exit 0Lint was narrowed, and the narrowing is measured rather than assumed (at
6f01ef3491): the config-derived population is 6817 files (read by walkinggit ls-filesthrough ESLint's ownisPathIgnored, not guessed); 4 files were linted, counted from--format json, 0 errors / 0 warnings; and the narrowing excludes nothing because type-aware linting is not enabled — everyparserOptionsineslint.config.mjscarries onlyecmaVersion/sourceType, with noprojectorprojectService, so each file is judged from its own source text and this diff cannot move the verdict of a file it did not touch. The repo-wide run is CI's.Generated artifacts
check:generatedproved exactly one artifact stale and it was regenerated with--fix(never the whole set). The diff is two table cells, both intended:imageandlogorender asstring | nullincontent/docs/references/identity/.authorable-surface.base.jsondid not move andcheck:authorable-surfaceis green.Surface note
The dispatch named the two
.zod.tsfiles, plus.changeset/*.mdand gate-required derivatives, and markedplugin-auth/plugin-hono-server/clientread-only. Those three were not written to — the probe runs from the scratchpad against builtdist, so the measurement sites were only read. Two files were touched beyond the literal list: the siblingidentity.test.tsandorganization.test.ts, because shipping a spec widening with no pin is the always-green hazard this repo refuses, and the Definition of Done requires the coverage. Both were checked for in-flight holders first (last touched by2c86fe3ea7and4b5702ab77, both landed). The regeneratedcontent/docs/references/identity/*.mdxare the gate-required derivative.Generated by Claude Code