You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit b9d5422
Browse filesBrowse the repository at this point in the historyBrowse files
fix(spec): UserSchema.image and OrganizationSchema.logo accept null, the shape better-auth serves (#18718)
Fixes#18509
Clause-②: yes (widening)
> ⚠️ **The claim comment says `Clause-②: no`, and this line deliberately
does not copy it.** The claim was written for the outcome the
dispatching seat expected — "does not reproduce", therefore no diff. The
measurement below went the other way, so this PR widens a **published
accept set** (`@objectstack/spec`) and takes a `minor` changeset.
AGENTS.md's Post-Task Checklist §3 makes `yes` mechanical for that act,
and `check-changeset-no-major.mjs`'s level axis grades the declaration
against the levels; declaring `no` here would be green **and false**, on
exactly the gate the seat invoked when it asked for the line to be
carried ("`Check Changeset` reads the BODY, not the card"). The
divergence is named here and in the dev report rather than chosen
silently — **the claim comment is the seat's to correct**, and the dev
does not `PATCH` a body.
## 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 real
`ObjectQL` on a real `SqliteWasmDriver`, with the platform's own
`sys_user` / `sys_organization` definitions, **both keys are served
present-and-null**:
| route | key | served |
|---|---|---|
| `POST /auth/sign-up/email` | `user.image` | `null` |
| `GET /auth/get-session` | `user.image` | `null` |
| `POST /auth/organization/create` | `logo` | `null` |
| `GET /auth/organization/list` | `[0].logo` | `null` |
| `GET /auth/organization/get-full-organization` | `logo` | `null` |
| `GET /auth/organization/get-full-organization` |
`members[].user.image` | `null` |
The 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_info` reports `sys_user.image` as `varchar(255) notnull=0`
and `sys_organization.logo` as `varchar(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, and `SessionUserSchema.safeParse` passes 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 `Map` of plain objects). Under it
`image` read **ABSENT**, not present-and-null, and
`UserSchema.safeParse` **passed** — 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 as `null`. 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/src` returns **8** declarations, the
same 8 the card reported. This PR moves exactly **2** of them. The other
6 — `ai/agent.zod.ts` `avatar`, `ui/app.zod.ts` `logo`,
`api/auth.zod.ts` `SessionUser.image` (already `.nullish()`) and
`RegisterRequest.image` (a REQUEST surface, client-authored, not
better-auth-served), `kernel/plugin-registry.zod.ts` `logo`,
`kernel/plugin-security-advanced.zod.ts` `image` — appear nowhere in the
conclusion and are untouched.
## ⭐ How does `.url()` coexist with `null`? — the question this card
could not answer
Both keys carry `.url()`; `SessionUserSchema.image` did not. So this is
not a copy of #17235 and the answer had to be measured. It was, on all
three candidate forms:
| input | `.url().optional()` (today) | `.url().nullish()` (ruled
remedy) | `.url().nullable()` (the shape #17235 refused) |
|---|---|---|---|
| key ABSENT | pass | **pass** | **FAIL** `invalid_type` |
| `null` | FAIL `invalid_type` | **pass** | pass |
| `""` | FAIL `invalid_format` | FAIL `invalid_format` | FAIL
`invalid_format` |
| `"https://x/a.png"` | pass | pass | pass |
| `"not-a-url"` | FAIL `invalid_format` | FAIL `invalid_format` | FAIL
`invalid_format` |
| `42` | FAIL `invalid_type` | FAIL `invalid_type` | FAIL `invalid_type`
|
**The answer: `.url()` and `null` do not compete, because they never
meet.** `.nullish()` wraps the whole `z.string().url()`, so `null` and
`undefined` are 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.image` accepts `""` and warned it "becomes
live if step 2 adds `.url()` reasoning to this family". Measured: it
does not. `SessionUser.image` has that hole because it is bare
`z.string()`; these two keys carry `.url()`, so `""` is refused before
and after — the `invalid_format` row 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
```
UserSchema.safeParse(SERVED_GET_SESSION_USER)
before: FAIL [{ path: ["image"], code: "invalid_type",
message: "Invalid input: expected string, received null" }]
after: PASS
OrganizationSchema.safeParse(SERVED_ORG_CREATE_BODY)
before: FAIL logo [invalid_type] expected string, received null
updatedAt [invalid_type] expected string, received undefined
after: FAIL updatedAt [invalid_type] expected string, received undefined
```
`image` is the ONLY divergence `UserSchema` had against the served user,
so that body now parses clean. `logo` was one of three on
`OrganizationSchema` — 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:
- **`metadata` is served present-and-null.** `/auth/organization/list`
and `/auth/organization/get-full-organization` serve `"metadata": null`
against `z.record(z.string(), z.unknown()).optional()`. Same
present-and-null shape, different key, and a `z.record` rather than a
`z.string().url()` — so it deserves its own reasoning, not this one by
extension.
- **`/auth/organization/create` omits `updatedAt`**, 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.ts` so
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.ts` and
`organization.test.ts` assert 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 to `image`/`logo` and 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, plus `git hash-object` differing from
the `HEAD` blob on both files), and the restore was proved byte-exact
the same way (`git diff HEAD` empty; both disk hashes equal to their
`HEAD` blobs). These tests resolve `./identity.zod` **relatively**, i.e.
to `src`, not through the package `exports` to `dist`, so no rebuild is
interposed and the dist-preflight step does not apply.
```
ABLATED_TEST_EXIT=1 -> Test Files 2 failed | Tests 5 failed | 48 passed (53)
× accepts `null` — the value every /auth/* user body carries
× accepts `null` — the value every organization body carries
× lit control: the instrument reports a neighbour when a neighbour is wrong (x2)
× does NOT (yet) accept a served body whole — metadata/updatedAt are separate cards
```
The 48 that stayed green are the rows that must not move: absent, a
valid URL, `""`, `"not-a-url"`, a number.
## Verification
| leg | result |
|---|---|
| `pnpm --filter @objectstack/spec test` | **486 files / 13895 tests
passed** |
| `pnpm --filter @objectstack/spec typecheck` | **pass** — `tsc
--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:generated` | **15/15 up to
date** after regenerating the one it proved stale (`gen:docs`) |
| `pnpm --filter '@objectstack/spec^...' build` | **empty run** — "No
projects matched the filters"; `packages/spec` has no workspace
dependencies, so there is no upstream closure. Reported as empty, not as
a pass. |
| gates | `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 0 |
**Lint was narrowed, and the narrowing is measured rather than assumed**
(at `6f01ef3491`): the config-derived population is **6817** files (read
by walking `git ls-files` through ESLint's own `isPathIgnored`, 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** — every `parserOptions` in
`eslint.config.mjs` carries only `ecmaVersion` / `sourceType`, with no
`project` or `projectService`, 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:generated` proved exactly one artifact stale and it was
regenerated with `--fix` (never the whole set). The diff is two table
cells, both intended: `image` and `logo` render as `string | null` in
`content/docs/references/identity/`. `authorable-surface.base.json` did
not move and `check:authorable-surface` is green.
## Surface note
The dispatch named the two `.zod.ts` files, plus `.changeset/*.md` and
gate-required derivatives, and marked `plugin-auth` /
`plugin-hono-server` / `client` read-only. **Those three were not
written to** — the probe runs from the scratchpad against built `dist`,
so the measurement sites were only read. Two files were touched beyond
the literal list: the sibling `identity.test.ts` and
`organization.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 by `2c86fe3ea7` and `4b5702ab77`, both landed). The
regenerated `content/docs/references/identity/*.mdx` are the
gate-required derivative.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01JbZnqu8bt6YqfJsr9vaFb3)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
`UserSchema.image` and `OrganizationSchema.logo` are declared `z.string().url().nullish()` — a URL string, `null`, or the key absent are all accepted — so the user and organization bodies this platform serves parse against the schemas it publishes (#18509).
6
+
7
+
Both were `z.string().url().optional()`: a URL string or the key's absence, and `null` refused. Both columns are better-auth-owned and nullable — `sys_user.image` and `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.
8
+
9
+
Measured through a real `AuthManager` (better-auth 1.7.3) over a real `ObjectQL` on a real `SqliteWasmDriver`, with the platform's own `sys_user` / `sys_organization` object definitions:
10
+
11
+
```
12
+
/auth/sign-up/email -> user.image = null
13
+
/auth/get-session -> user.image = null
14
+
/auth/organization/create -> logo = null
15
+
/auth/organization/list -> [0].logo = null
16
+
/auth/organization/get-full-organization
17
+
-> logo = null
18
+
-> members[].user.image = null
19
+
20
+
UserSchema.safeParse(<the served session user>)
21
+
-> [{ path: ["image"], code: "invalid_type",
22
+
message: "Invalid input: expected string, received null" }]
23
+
OrganizationSchema.safeParse(<the served organization>)
24
+
-> [{ path: ["logo"], code: "invalid_type",
25
+
message: "Invalid input: expected string, received null" }, … ]
26
+
```
27
+
28
+
Those two paths now parse.
29
+
30
+
-**Measured, not inferred.**#18509 exists because PR #18501's contract review named these two siblings as *not measured* rather than folding them into the `SessionUserSchema.image` ruling it had. The verdict here comes from the probe above, run the way that ruling's own evidence was taken; the analogy was only ever a reason to look.
31
+
-**The declaration was the thing that was wrong.** Prime Directive #12's default — fix the producer, never widen the consumer — rests on the premise it states out loud, that we own both ends. We do not: the nullable columns belong to a third-party model, so PD #12's own exit clause is the operative sentence.
32
+
-**A pure widening.**`.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`. Every body legal before this change is still legal.
33
+
-**`.url()` is kept, and it does not fight `null`.** These two declarations carry `.url()`, which `SessionUserSchema.image` did not, so the question had to be answered rather than copied. `.nullish()` wraps the whole `z.string().url()`: `null` and `undefined` are separate branches the URL check never sees, while a present string is still required to be a well-formed URL. Of six inputs — absent, `null`, `''`, a URL, a non-URL, a number — exactly one row moves, and it is the ruled one. `''` and `'not-a-url'` are still refused.
34
+
-**No key is added or removed** — both keys were already authored and already published, so no authorable surface moves and nothing is retired.
35
+
-**`OrganizationSchema` is not made whole by this.** The same probe found `metadata` served present-and-null and `/auth/organization/create` omitting the required `updatedAt`. Those are separate defects with their own reasoning, filed separately rather than folded in; #18509 asked about `logo`.
0 commit comments