Migrate to zod v4 #163
No reviewers
Labels
No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
lz/agent-nexus!163
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "chore/zod-v4"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #148.
zod
^3.24.1→^4.2.0, resolved to 4.6.5. No@modelcontextprotocol/*dependency is added — that is #154.The issue's measurement was wrong, and so was mine
The issue recorded zero occurrences of all nine classic v3→v4 breaking patterns. Three of the nine do occur:
.default(.url(.email(.default()is the one that matters, because v4 changed it from "parse the default through the schema" to "return it directly". Nine of the 22 are inconfig.ts, includingPORT: z.coerce.number().int().positive().default(3001)— a coerce stacked on a default, exactly the combination the change is about. All 22 turned out to be safe (every default is already an output-type value), but that had to be checked rather than counted.I then replayed the real config schema plus replicas of every other body through both versions — 64 cases — and concluded "every accept/reject decision and every parsed value is identical; only message wording changed." An independent review falsified that. The differential was real, but its inputs had blind spots. Four genuine behaviour changes:
1.
z.url()trims." http://172.30.0.1:3001\n"now parses to"http://172.30.0.1:3001"; zod 3 returned the padding verbatim. This is the bump, not thez.string().url()→z.url()edit — v4's deprecated method form trims identically.2.
z.url()accepts unicode whitespace that zod 3 rejected outright — NBSP, U+1680, U+2000, U+2028, U+3000, U+FEFF — then strips it. A URL pasted with an NBSP used to 400; now it is repaired.Both land on
WORKER_CALLBACK_URL(injected into workers asNEXUS_URL, curl'd bynotify-preview) and onconnections.base_url(sealed under the DEK, used for every Forgejo call). Both are improvements in direction, and both now have a pinning test.3.
.int()requires a safe integer.9007199254740992and1e21are rejected where v3 accepted them. Reachable attokens'expires_at, which now 400s at the schema instead of reachingcreateTokenand failing its century guard. Fact #24's microseconds-as-milliseconds case is below 2^53 and still behaves exactly as documented.4.
z.number()rejects+Infinity(v3 accepted). Only reachable at the quota route, whose comment promised to "accept anything non-negative and clamp" — butJSON.parsecannot produceInfinity, so the code is right and only the comment was wrong. Corrected.Everything else did hold: message wording changed everywhere, and nothing reads it. No test anywhere asserts on a zod-generated string, and no client code consumes the error payload —
lib/api/client.tsreads onlypayload.error..flatten()is deliberately untouched18 call sites do
json({ error: 'invalid body', details: X.error.flatten() }, { status: 400 })..flatten()is deprecated in v4 but present, and returns the same{formErrors, fieldErrors}shape as thez.flattenError()that supersedes it — verified byte-identical.A trap for whoever finishes this: zod's own deprecation text points at
z.treeifyError(), which produces a different shape ({errors, properties}). Following it would silently change the 400 body of 17 routes. Exactly one thing would notice —tokens/server.test.ts:112, which readsdetails?.fieldErrors?.scopes. The right fix is an extractedinvalidBody(err)helper inlib/server/lib/http.ts, not 18 mechanical swaps, and it is not a dependency-bump change.Source changes: four lines, all idiom
z.string().url()→z.url()inconfig.tsandconnections/+server.ts(the method form is deprecated). Accept/reject identical across a 51-case URL corpus.{ message }→ a bare string in one.refine()and onez.custom(). The issue asked for{ error }; the bare string drops a wrapper object that only exists for multi-key params, and it is the formworkers/+server.tsandmounts/test/+server.tsalready used — so this leaves the repo with one shape for refinement messages instead of two.New: a test for
config.tsIt is the only zod schema validated against the real
process.env, and the one place a validation change fails at container boot rather than at typecheck. It had no test at all.parseConfig(env)is now exported as the seam —loadConfig()stays theprocess.exit(1)wrapper its two callers use — so the test needs noprocess.envswap and noprocess.exitstub.reserved-volume-default.test.tsmoves onto it too; it was the lastloadConfig()in a test, and a bad ambient env there would have killed the vitest worker mid-run with no assertion output.Nine cases: every default under an empty env, the two
z.coerce.number()vars and their rejection matrix, the exact truthy set forFORCE_SECURE_COOKIE, unknown-key stripping, theNODE_ENVenum, and the three URL behaviours above. One of the nine cross-checks the defaults the settings registry declares a second time — drift there shows the operator a default in the Settings panel that the process never boots with.Every assertion was proven to fail. Nine mutations of
config.ts, one of the registry'sPORTdefault, and one negative control that makes an unrelated field required. That last one caught a real defect the first draft shipped: the originalrejects()helper asked only "did the whole env fail", so all ten rejection assertions stayed green while testing nothing.rejectsOn(env, field)now checks every issue points at the named var, and under that same mutation all nine cases fail.Gates
pnpm typecheckpnpm testpnpm lintAll three re-run on the final commit, not on a pre-review snapshot.
Beyond the three, because the issue's "done when" names a boot:
pnpm buildsucceeds — zod ispackages: 'external'inbuild-server.mjs, so the bump costs 0 bytes in every shipped artifact (client included).node build/server.jsreachesagent-nexus ready; migrations ran,loadConfigaccepted a real env./api/auth/setup400s; after unlock,/api/connectionswith{"kind":"nope","label":"","base_url":"not a url"}returns theflatten()shape intact, and/api/tokenswith["bogus"]returns{"scopes":["unknown scope"]}— thez.custommessage surviving its rewrite.A full
docker compose up -dwas not run (no docker daemon in this worktree). The booted production bundle is the closest substitute and covers the failure mode the issue was worried about.Cost
zod's module graph grew 5.7× — 10 modules / 147 KiB → 95 modules / 832 KiB — which measures as +64 ms of blocking startup on a warm cache, once per container boot.
zod/miniwas measured and rejected: it saves 4 ms because both entry points unconditionally re-export 64 locale modules (341 KB, 40% of the graph), and it would cost a rewrite of every schema to.check()form.Deliberately not done
z.url({ protocol: /^https?$/ })onWORKER_CALLBACK_URL. v4 can express it, and it would reject the scheme-dropped typo this PR pins as accepted:z.url()needs only a scheme, sohost.docker.internal:3001parses the waymailto:does, boots green, and yields aNEXUS_URLthatnotify-previewcan never reach. (The IP-shaped form is rejected already — a scheme cannot start with a digit.) It is a real gap, it is true on zod 3 as well, and it belongs in its own change: it alters what the container accepts at boot, and folding a deliberate decision change into a bump means a boot that breaks after this lands cannot be attributed to either. Pinned by a test meanwhile, so it stays a known gap.Also noted but out of scope: a bare
z.string().default(...)accepts''and does not fall back to the default, soCOOKIE_NAME=in an env file yields an empty cookie name andDATA_DIR=puts SQLite at''. Identical on zod 3 — pre-existing, and a.min(1)decision rather than a migration one.Merge note
No conflict with #160 (
fix/unlock-rate-limit). That branch editsroutes/api/auth/unlock/+server.ts, which this branch does not touch — itsPassphraseSchemaisz.string().min(8).max(1024), carrying none of the four idioms migrated here, so it needs no change under zod 4. The only shared file would bepackage.json, and only if that branch also moves a dependency.@modelcontextprotocol/server v2.0.0 needs zod ^4.2.0. Two majors of a validation library in one process is a trap for whoever next imports the wrong `z`, so Nexus moves ahead of the MCP work rather than alongside it. Resolved to 4.6.5; no MCP dependency is added here. The issue's measurement was that every classic v3->v4 breaking pattern occurs zero times. Three of the nine do occur: `.default(` 22 times, `.url(` twice, `.email(` never. `.default()` is the one whose semantics actually changed — v4 returns the default directly instead of parsing it through the schema — so it was the one worth checking rather than counting. Checked by replaying the real config schema (extracted from config.ts, not retyped) plus replicas of the mount, token, quota, session, connection, comment and preference bodies through both versions: 64 cases covering absent/empty/garbage/realistic env, every URL shape the operator could plausibly set, and the failure path of each schema. Every accept/reject decision and every parsed value is identical. The only differences are the wording of rejection messages, which reach API consumers through `error.flatten()` but no UI — nothing in src/lib or src/routes reads `fieldErrors`. `error.flatten()` still exists in v4 and returns the same `{formErrors, fieldErrors}` shape as the `z.flattenError()` that supersedes it, so the 18 call sites are left alone. The four source edits are v4 idiom, not fixes. `z.string().url()` is deprecated in favour of the top-level `z.url()`; both were verified to accept and reject identically across 13 URL shapes, including the `host.docker.internal` and bridge-IP forms WORKER_CALLBACK_URL is documented to take. `{ message }` in a `.refine()` and a `z.custom()` becomes `{ error }`, which v4 prefers; the deprecated alias still works and produces the same string.Quality pass over the zod bump. Four findings, three applied. The config test bent around `loadConfig`'s shape instead of fixing it: it swapped `process.env` wholesale, stubbed `process.exit` to throw and silenced `console.error`, all to reach a `safeParse` six lines in. `parseConfig(env)` is that call, exported; `loadConfig` stays the `process.exit(1)` wrapper its two callers already use. The test keeps none of the scaffolding, and the failure mode it carried goes with it — a spy that stopped throwing would have run the real `process.exit(1)` and killed the vitest worker mid-run. The "every default" case wrote the default table a third time, after the schema and the settings registry, in a form that could never catch drift between the first two. The literals stay — they are what a library upgrade can move — and a second case now walks the nine `SETTINGS` rows whose `envVar` names a config key and asserts each declares the same default. Registry drift shows the operator a default in the Settings panel that the process never boots with; the count is asserted too, so a row losing its `envVar` cannot quietly empty the loop. `z.custom` and `.refine` take a bare message string, so the `{ error: … }` wrapper the bump introduced is gone from both sites. That is also the form `workers/+server.ts` and `mounts/test/+server.ts` already use — the bump had left four refinement messages in two different shapes. Both mutation suites were re-run against the rewritten test: eight mutations (seven of config.ts, one of the registry's PORT default) each break at least one case, and dropping the PORT default now breaks two. Not applied: constraining `WORKER_CALLBACK_URL` to `z.url({ protocol: /^https?$/ })`. It is a real gap — v4 can express it, and it would reject the scheme-dropped typo the new test pins as accepted — but it changes what the container accepts at boot. The whole claim of this branch is that the bump changed no decision anywhere; smuggling a deliberate decision change into it means a boot that breaks after this lands cannot be attributed. It belongs in its own change.