Repository navigation
fix(schema,entity,query)!: nullable produced a JSON Schema that rejects null, and a scaled Money round-tripped as unscaled - #106
Merged
Conversation
…ects null, and a scaled Money round-tripped as unscaled
Slices 01, 04 and 09 of the deep-dive audit: the projection contract — what a value
becomes when it leaves the process as JSON Schema, as a Postgres row, as a cursor, or as
a log line — and the tier-0/1 logic bugs underneath it.
**`t.nullable(x)` emitted a schema that rejects `null`.** `json-schema.ts` copied the
inner conversion and set `nullable: true`, a keyword no JSON Schema draft after OpenAPI
3.0 defines. Every consumer that validates — the generated OpenAPI, an MCP client, a
contract test — saw `{ type: 'string' }` and rejected the one value the declaration exists
to permit. Now `{ anyOf: [<converted>, { type: 'null' }] }` with the annotations hoisted,
which every draft understands and OpenAPI 3.1 accepts unchanged.
**A shared default bled between requests.** `.default(value)` stored one object and handed
the same reference to every parse that omitted the field, so a handler's mutation became
the next request's starting value. Defaults are now cloned per parse via `structuredClone`,
and a default that cannot be cloned is refused where it is written — at the first import of
the authoring file — as `X_SCHEMA_DEFAULT_UNSHAREABLE`, rather than shared and bled.
**Money lost its scale on the way to the database and back.** A `Money` with an explicit
`scale` was written to a `numeric` column that recorded only minor units, so a value
declared at six decimal places read back at two — a 10,000× error on the read path, the
mirror of the conversion bug #105 fixed on the compute path. `entity` now persists scale
as a third physical column (`<p>_scale smallint null`), and `pg-row.ts`, `describe.ts`,
`realtime`'s logical-decoding row decoder and both scaffold templates fold it back the
same way. `pg-entity-row-parity.test.ts` pins the two decoders together so the replication
path and the query path cannot drift again — they had.
**A cursor silently truncated `Date` and `bigint`, and ordered wrong.** A cursor is JSON;
a `Date` became a string and a `bigint` threw, so a read ordered by `createdAt` resumed at
a position no comparison could reproduce and pages repeated or skipped. Values are now
tagged (`{ $x: 'date' | 'bigint', v }`) and revived on the way back. A sort value that is
neither scalar nor one of those two is refused where the cursor is minted, as
`X_CURSOR_VALUE_UNSUPPORTED` — the mistake is the read's own `orderBy`, so no retry
repairs it.
**A query input that cannot survive a query string is now refused at declaration.** A read
is served as `GET /_x/query/<name>`, so its input is characters: the typed client encoded a
nested object as JSON text and `coerceQuery` had no inverse, and a required
`t.nullable(...)` member was skipped entirely. `X_QUERY_INPUT_UNENCODABLE` raises at
`query()`, in the file that declared it, instead of at the first request that hits it.
Also in this slice, each with a failing test written first:
- `logger.ts` serialised a value that `JSON.stringify` refuses (a `bigint`, a circular
reference, a throwing getter) by throwing inside the writer — losing the line it was
trying to write and, on a `console` writer, the ones after it. Serialisation is now
total: every field is reduced to something stringifiable before the final call.
- `cache/graph.ts` walked every registered dependency on each invalidation; a `byEntity`
index makes it proportional to the entity's own edges.
- `mcp/cross-surface.test.ts` pins `action`'s `mcpSchemaOf` against `mcp`'s `toWireSchema`,
which had already diverged once on `pattern`. The two live either side of a tier line
and cannot share a home below tier 4 today — the guard is the honest fix until one
exists.
`examples/dummy/packages/db/migrations/0002_money_scale.sql` ships without a `.hash`
sidecar on purpose: the sidecar is written by `x db migrate` against a live database, and
inventing one here would assert a checksum nothing computed.
New codes: `X_SCHEMA_DEFAULT_UNSHAREABLE`, `X_CURSOR_VALUE_UNSUPPORTED`,
`X_QUERY_INPUT_UNENCODABLE` — all three in `wiki/Error-Codes.md` and the manifest.
Breaking: `nullable` disappears from generated JSON Schema in favour of `anyOf`; a
`.default()` holding an uncloneable value now fails at import; a `query()` whose input is
not query-string-encodable now fails at declaration.
bun run verify: 14 of 17 green, 3 skipped (drift, contract-diff, budgets).
bun run scripts/reference-app-gate.ts: every pin holds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 32 minutes Limit details: You’ve used all 1 included review currently available under your plan. You completed 80 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (122)
Comment |
sebyx07
added a commit
that referenced
this pull request
Aug 17, 2026
auth and entity needed nothing — every finding naming them was already closed by #104 and #106, verified file-by-file before #112 was scoped. That is what took the PR from an estimated ~70 files to 43. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D
sebyx07
added a commit
that referenced
this pull request
Aug 17, 2026
…her, and ten codes paged the on-call for a caller's mistake (#112) * fix(action,http,core)!: one caller's idempotent response went to another, and ten codes paged the on-call for a caller's mistake The http and action remainder of audit slices 02 and 06, plus the gate step slice 02 asks for by name. auth and entity are absent because they are already closed — every finding naming them landed in #104 and #106, verified before this wave was scoped. `idempotencyKeyFor(actionName, key)` namespaced by action name only, with no actor anywhere, so alice's stored `charge` response was returned to bob whenever bob sent the same key; a differing payload gave bob X_IDEMPOTENCY_CONFLICT instead, which is a cross-actor denial of service against any key. And `Headers.get()` answers '' rather than null for `Idempotency-Key:`, so a blank header was a live key every blank sender shared. The status table is closed, so a code with no row falls to 500 — and stages.ts reports every status >= 500 to the error monitor. Ten caller-caused codes were in that state: a reused key, an expired cursor, a weak password, a duplicate signup. The sharpest was the framework contradicting its own published contract, action/http.ts:151 declaring '409' for X_IDEMPOTENCY_CONFLICT while the runtime answered 500. Nothing would have caught the eleventh, so the errors step gained a fourth host rule. The specified predicate — every code owned by a tier <= 4 package needs a row — flags 237 of 394, which is a step an agent disables in week one; and whether a code can reach a request is not derivable, since X_MIGRATION_DESTRUCTIVE and X_TENANCY_CROSS_DENIED are the same tier and the same shape and blanket re-exports collapse import reachability to the whole package. So it is a ratchet on the expectedRed idiom: 226 undecided codes pinned with a reason, the list may only shrink, and a pin says "nobody has decided yet" rather than "this can never reach a request". It caught two codes on its first run, both added by its own teammates in this PR. Also: a second server after a drain bound a port it could never serve from and kept accepting connections after its own stop() returned; ?__proto__= replaced the parsed query object's prototype, which also let a schema coerce an inherited function through `key in record`; an empty issues array read as validation success and reached a handler as an impossible undefined; the third copy of FNV-1a/32 over client-chosen input, here backing both the idempotency requestHash and the job dedupe key; and an unfenced settle overwriting a record already replaced. BREAKING CHANGE: `idempotencyKeyFor` takes a required third Actor argument; the stored key's shape changed, so on the shared Postgres store a retry crossing the deploy boundary re-runs the handler inside the 24h window (`truncate x_idempotency` makes that state honest); `Idempotency-Key` is now enforced at the 255 characters OpenAPI already published; action's `fingerprint` is SHA-256/16, changing job-handle.ts's dedupe key; `markReady()` throws X_LIFECYCLE_DRAINED on a drained lifecycle instead of declining silently. New codes: X_IDEMPOTENCY_KEY_INVALID, X_LIFECYCLE_DRAINED, X_ERROR_STATUS_MISSING, X_ERROR_STATUS_BACKLOG_STALE, X_ERROR_STATUS_UNKNOWN_CODE. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D * docs(plan): slices 02 and 06 are done across #107, #110 and #112 auth and entity needed nothing — every finding naming them was already closed by #104 and #106, verified file-by-file before #112 was scoped. That is what took the PR from an estimated ~70 files to 43. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slices 01, 04 and 09 of the deep-dive audit: the projection contract — what a value becomes when it leaves the process as JSON Schema, as a Postgres row, as a cursor, or as a log line — plus the tier-0/1 logic bugs underneath it.
123 files. Fifth PR of the sweep, after #101, #102, #103, #104, #105.
The five that matter
t.nullable(x)emitted a schema that rejectsnullpackages/schema/src/json-schema.ts:107nullable: true, a keyword no JSON Schema draft after OpenAPI 3.0 defines. Every validating consumer — generated OpenAPI, an MCP client, a contract test — saw{ type: 'string' }and rejected the one value the declaration exists to permit.default()bled between requestspackages/schema/src/builder.ts:120packages/entity/src/pg-row.ts,columns.tsMoneywith explicitscalewrote to anumericrecording only minor units, so six decimal places read back at two — a 10,000× error on the read path, mirroring the conversion bug #105 fixed on the compute pathDateand threw onbigintpackages/query/src/cursor.tscreatedAtresumed at a position no comparison could reproduce; pages repeated or skippedpackages/query/src/query.tsGET /_x/query/<name>, so its input is characters. The typed client encoded a nested object as JSON text with no inverse incoerceQuery, and a requiredt.nullable(…)member was skipped outrightThe fixes
anyOf, notnullable.{ anyOf: [<converted>, { type: 'null' }] }with annotations hoisted — understood by every draft, accepted unchanged by OpenAPI 3.1.structuredCloneper parse, and a default that cannot be cloned is refused where it is written, at the first import of the authoring file, asX_SCHEMA_DEFAULT_UNSHAREABLE— rather than shared and bled.<p>_scale smallint null), per the decision taken on this sweep.pg-row.ts,describe.ts,realtime's logical-decoding row decoder and both scaffold templates fold it back the same way, andpg-entity-row-parity.test.tspins the two decoders together so the replication path and the query path cannot drift again — they had.{ $x: 'date' | 'bigint', v }) and revived on the way back. A sort value that is neither scalar nor one of those two is refused where the cursor is minted, asX_CURSOR_VALUE_UNSUPPORTED: the mistake is the read's ownorderBy, so no retry and no fresh first page repairs it.X_QUERY_INPUT_UNENCODABLEraises atquery(), in the file that declared it, instead of at the first request that hits it.Also in the slice
logger.tslost the line it was writing when a field was somethingJSON.stringifyrefuses — abigint, a circular reference, a throwing getter — and on aconsolewriter, the lines after it too. Serialisation is now total: every field is reduced to something stringifiable before the final call.cache/graph.tswalked every registered dependency on each invalidation; abyEntityindex makes it proportional to the entity's own edges.mcp/cross-surface.test.tspinsaction'smcpSchemaOfagainstmcp'stoWireSchema, which had already diverged once onpattern. They live either side of a tier line and cannot share a home below tier 4 today — the guard is the honest fix until one exists, and the structural half is filed for the architecture PR.Deliberate
examples/dummy/packages/db/migrations/0002_money_scale.sqlships without a.hashsidecar. The sidecar is written byx db migrateagainst a live database; inventing one here would assert a checksum nothing computed.Breaking
nullabledisappears from generated JSON Schema in favour ofanyOf.default()holding an uncloneable value now fails at importquery()whose input is not query-string-encodable now fails at declarationThird breaking PR of the sweep, stacking onto #87's major-version tally with #102's
job({ tenant })and #105'sEPOCH→epoch().New codes
X_SCHEMA_DEFAULT_UNSHAREABLE·X_CURSOR_VALUE_UNSUPPORTED·X_QUERY_INPUT_UNENCODABLE— all three inwiki/Error-Codes.mdandframework.manifest.json(387 codes).Gate
🤖 Generated with Claude Code
https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.