Repository navigation
fix(rest): an import row for a missing database column answers what the create door answers - #20941
Conversation
… door answers toFailedResult classifies a write error through mapDataError, the mapper POST /data/:object uses, and takes the door's INVALID_FIELD verdict (code, field, sentence) when it gives one. A declared field whose column is missing now fails the commit row and the async job's rows with INVALID_FIELD naming the field, not the driver's SQLITE_ERROR and raw text. Unique-conflict and NOT NULL rows keep their answers. Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 23 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 15 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 7ba63ebbcd79daf71ffbf91e44fb7c18209e941d && git checkout 7ba63ebbcd79daf71ffbf91e44fb7c18209e941d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin bee75cebe63889f93fa1e6dc358cd8c5f7b9a928 132e93c05bd736c80a166cff38955b910ec8cdf9 && git checkout -B drift-repro bee75cebe63889f93fa1e6dc358cd8c5f7b9a928 && git merge --no-ff 132e93c05bd736c80a166cff38955b910ec8cdf9
node scripts/docs-audit/affected-docs.mjs --json bee75cebe63889f93fa1e6dc358cd8c5f7b9a928
|
Contract reviewServed-tier: Inputs read: card #20701 body and all sixteen comments (governing: ① Derived judgments(a)
(b) Both call sites pass the right object, and the dry run is untouched. (c) Row shape: existing keys only, and the sentence is safe. The adopted literal is (d) Changeset ( ② Semver level
③ Boundary flagsDev
Dev
File list: three paths, none a governed surface ( Check-runs on Implemented-by: VERDICT: PASS Generated by Claude Code |
…answers what the create door answers (objectstack-ai#20956) Part of objectstack-ai#20701 Clause-②: no REST item 5 of objectstack-ai#20701, as triage's answer `5920419144` scopes it and the claim `5920542737` holds it: the NOT NULL row and the unique-conflict row's sentence. Item 1 (the drop signal) is not in this PR and remains open on the card; it waits on objectstack-ai#20922. ## What changed - **`packages/rest/src/import-runner.ts`.** `toFailedResult` widens item 4's adoption gate (PR objectstack-ai#20941, `f80e2a6dad`) on `mapDataError`'s verdict, the mapper `POST /api/v1/data/:object` uses, from `INVALID_FIELD` to two more verdicts. The gate lives in `adoptDoorVerdict`, with the verdict set `ADOPTED_DOOR_VERDICTS`. - **A driver's NOT NULL refusal.** The door answers `400 VALIDATION_FAILED` with `fields: [{ field, code: 'required' }]`. The row now reads `code: 'required'`, `field` and the door's sentence. It used to relay the dialect code (`SQLITE_CONSTRAINT_NOTNULL`) with no `field`. No driver dialect's code reaches the wire `code` (ADR-0112). - **A unique conflict.** `code: 'UNIQUE_VIOLATION'` and `field` are the same as before. The sentence is now the door's ("A record with this code already exists"), where it was the engine's (which the door ships as `developerMessage`). - The gate is on the door's VERDICT, not on the error. Which arm fired, the `field` and the sentence are all the mapper's, and the import derives none of them. A `VALIDATION_FAILED` that carries no finding is not taken, and a finding on the thrown error still wins over the door, as before. - No key is added. The door's `hint`, `object` and `developerMessage` are not keys of `ImportRowResultSchema`, and they stay off the row. - **Pins, `packages/rest/src/import-row-schema-drift-20701.test.ts`.** PR objectstack-ai#20941's two "unchanged by this card" control pins flip on purpose. The pins cover the commit (insert and upsert) and the async job's results route. Each asserts the row equals the door's answer, the row's key set is exactly `row, ok, action, code, field, error`, and the `code` is no dialect code. The item-4 drift cases and a writable row beside each failed row stay as controls. - **`packages/rest/src/error-response.ts`** is not edited. The mapper could be adopted as it stands. - **Changesets.** The new `.changeset/20701-import-row-not-null-and-unique-door-answer.md` is `@objectstack/rest` `patch`. See the next section for the second changeset file. ## A pending release note corrected: please confirm This PR also edits `.changeset/20701-import-row-schema-drift-door-answer.md`, item 4's changeset from PR objectstack-ai#20941. That changeset is still pending (it has not been released). It said: "Rows for a unique conflict or a NOT NULL failure are unchanged." This PR changes exactly those rows, and both notes ship in the same release, so the sentence would be false in the CHANGELOG. This PR removes that one sentence and changes nothing else in that file. This is the DELIBERATE CORRECTION class that `scripts/check-empty-changeset.mjs` names. Its scan exits 1 here on purpose, and `Check Changeset` stays red until a person confirms the correction on this PR. `Check Changeset` is not a required context. ⛔ `skip-changeset` does not apply. ## Measured: before and after Readings are from real ObjectQL, `ObjectStackProtocolImplementation` and `better-sqlite3` `:memory:`. The object has `code` (`unique: true`) and `must` (`storage: { notNull: true }`, not `required`). The probe was throwaway and was never committed. "Before" is `75519e1c0a` and "after" is `1eada1f134`. | door or row | before | after | |:--|:--|:--| | `POST /data/:object` without `must` | `400 VALIDATION_FAILED`, `fields [{ must, required }]`, "must is required", `hint` | unchanged | | `PATCH /data/:object/:id` with `must: null` | the same | unchanged | | import commit row, insert, without `must` | `code SQLITE_CONSTRAINT_NOTNULL`, "must is required.", no `field` | `code required`, `field must`, "must is required" | | import commit row, upsert with no match, without `must` | the same as insert | the same as insert | | async job results row, without `must` | the same as insert | the same as insert | | `POST /data/:object`, repeated `code` | `409 UNIQUE_VIOLATION`, `field code`, "A record with this code already exists", `developerMessage` = the engine's sentence | unchanged | | `PATCH /data/:object/:id`, repeated `code` | the same 409 | unchanged | | import commit row, insert or upsert-update, repeated `code` | `UNIQUE_VIOLATION`, `field code`, the engine's sentence "Duplicate record refused on 'proj_probe': …" | `UNIQUE_VIOLATION`, `field code`, "A record with this code already exists" | | async job results row, repeated `code` | the same as the commit | the same as the commit | | dry run, both rows | `ok`, `created` | unchanged (not pinned; see the notes) | **Why the row's `code` is `required`, not `VALIDATION_FAILED`.** The row applies the rule that a finding wins over the top-level code (`objectstack-ai#4633`), and it applies it to the door's finding as it does to the engine's. A metadata-`required` field the engine refuses already reads `{ field, code: 'required' }` on the row (`import-integration.test.ts`, "required-field dry-run fidelity"), while the door answers `VALIDATION_FAILED` with that same finding. So this is the row's existing rendering of the door's verdict, not a new one. ## Verification (at `1eada1f134`) - **Pins and the neighbouring import suites** (drift pins, `import-row-report-field-20701`, `import-runner-unique-violation-row`, `import-runner-error-sanitize`, `import-job-integration`, `import-integration`, `import-dryrun-parity`, `import-runner-bulk`, and the probe): `Test Files 9 passed (9)`, `Tests 163 passed (163)`. - **`pnpm --filter @objectstack/rest exec vitest run --project local --maxWorkers=2`:** `Test Files 244 passed (244)`, `Tests 4873 passed | 114 skipped (4987)`, VERDICT command-exit 0. - **`pnpm --filter @objectstack/rest typecheck`:** exit 0, "check:test-typecheck: OK". `tsc -p tsconfig.test.json --listFilesOnly` lists the pin file (count 1). - **Ablation.** The fix was committed first. Each mutation went through `scripts/ablation-replace.mjs` in wrap mode, with the anchor at x1 → x0 and the blob changed. Each was restored to blob == HEAD with an empty `git diff HEAD`. - A. `UNIQUE_VIOLATION` removed from the verdict set: `Tests 3 failed | 20 passed (23)`. The three failures are the unique insert, the unique upsert and the async job case. - B. `VALIDATION_FAILED` removed: 3 failed. They are the NOT NULL insert, the NOT NULL upsert and the async job case. - C. The door's finding no longer wins over its top-level code: 3 failed, on the same NOT NULL cases. - All three moved in the predicted direction. The drift pins and `import-runner-unique-violation-row` stayed green in each run. `import-runner.ts` is imported relatively from `packages/rest/src`, so no build or dist preflight applies. - **Lint, narrowed with proof:** - (1) Population from eslint's own config: `calculateConfigForFile` lints both `.ts` files, and `isPathIgnored` is true for both changesets. - (2) `eslint --no-inline-config --format json`: files=2, errors=0, warnings=0. - (3) Invariance: `parserOptions.project` and `projectService` are null on both files, and the active rules are single-file syntactic rules. So type-aware linting is off, and this diff cannot move eslint's verdict on any untouched file. - **Control-byte self-scan** over the 4 changed files: clean. - **`dispatch-gates --commands`** at `1eada1f134` (merge base `75519e1c0`) derived 61 commands, and each was run with its exit code recorded before any pipe. `--ran`: "61 derived famil(ies) accounted for — 59 run, 2 NOT-MEASURED". - 58 exited 0. - `check-empty-changeset.mjs --base origin/main` exited 1, by design (see the section above). - NOT MEASURED: `check:dual-build-cjs-loads` and `check:type-check-debt`. Reason: PREREQUISITE NOT MET (exit 3). Both need every package's `dist/`. The one full-build attempt was cut off by a container restart, and it was not repeated on this shared box. `Build Core` and `Lint & Repo Gates` cover them on this PR. - `origin/main` moved to `013f97df93` after the base. None of its three commits touches `packages/rest`, `objectql`, `types`, `metadata-protocol`, `driver-sql` or the `20701` changesets. ## Acceptance notes - **The dry run is unchanged.** Both rows preview as `ok` / `created`. `engine.validate` reads metadata and does not judge `storage.notNull` or uniqueness. Triage settled the drift case (`5920419144`, Ask 2), and this PR adds no preview-side check. - **The other dialects are read from source only, NOT MEASURED.** The mapper's not-null branch matches SQLite `NOT NULL constraint failed: t.c`, Postgres `null value in column "c"` (SQLSTATE `23502`) and MySQL `Column 'c' cannot be null` (`ER_BAD_NULL_ERROR`). None of the three is caught earlier by the missing-column limbs. The mapper is not widened. - **A driver's unique refusal that reaches the row without the engine's envelope** now reads `UNIQUE_VIOLATION` with the door's `field` and sentence, where it relayed the dialect code. On the real engine this path is unreachable: insert, update and the bulk path all envelope the refusal (measured on the bulk path). - **A sandboxed producer that declares `UNIQUE_VIOLATION` with no finding** now carries its business sentence (`innerMessage`) instead of the QuickJS debug wrapper. That is the door's answer, and it is the same residue class item 4's record named for `INVALID_FIELD`. - **A producer that throws `code: 'UNIQUE_VIOLATION'` with a declared status and its own `field`** leaves through the door's passthrough, which carries no `field`. The row now matches the door and carries no `field` either. No producer of that shape reaches the import through the engine: driver-memory's refusal is enveloped. Noted. - **`isEngineDuplicateRecordEnvelope` in `toFailedResult`** is now reached only when the thrown error carries a finding without a code. The engine's envelope carries none, so for that envelope the door's arm answers first. It is left in place and is unreachable in practice. - **The door's NOT NULL `hint` names drift** ("the physical schema has drifted from metadata. Run 'os migrate'"). For a field that declares `storage.notNull` on purpose, that advice is wrong. The hint is the door's text in `error-response.ts` and does not reach the row. Noted, not changed. --- _Generated by [Claude Code](https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB)_ Co-authored-by: Claude <noreply@anthropic.com>
Part of #20701
Clause-②: no
Item 4 of the REST half of #20701 (the schema-drift row), as triage's answer
5917724406scopes it. Item 1, the drop signal on the row report, is not in this PR: it waits on #20922, which waits on #20921. #20701 remains open.What changed
toFailedResult(packages/rest/src/import-runner.ts) now classifies a write error throughmapDataError, the mapperPOST /api/v1/data/:objectuses, for the same object. When the mapper's verdict isINVALID_FIELD, the row carries the door'scode,fieldand sentence. Every other verdict leaves the row as it was.So a declared field whose database column is missing (metadata and the physical schema have drifted) now fails the commit row, and the async job's rows, the way the create door fails it:
INVALID_FIELD,fieldset, and the sentence "The database table of object 'X' has no column for field 'f'. If the object declares 'f', its database schema has drifted from the metadata: run 'os migrate' to reconcile." Before, the row carried the driver's code (SQLITE_ERROR), the driver's text and nofield. The sync route and the async job worker both run throughrunImport, so one function covers both.A field-level finding still wins over the door, as it does for
code. An undeclared key reads exactly as before, because the door relays the engine's ownINVALID_FIELDenvelope in the engine's own words.packages/rest/src/error-response.tsis not edited:mapDataErroris exported, side-effect free and callable as it stands.Repro (SQLite
:memory:, real ObjectQL +ObjectStackProtocolImplementation+ RestServer routes)Drift is made the way it happens: the object declares
lateafter the DDL ran, with no re-sync.67c1b11a20)132e93c05b)POST /api/v1/data/:objectwithlate400 INVALID_FIELD,field: 'late', the drift sentenceSQLITE_ERROR, "table proj_probe has no column named late", nofieldINVALID_FIELD,field: 'late', the door's sentenceSQLITE_ERROR, "no such column: late", nofieldINVALID_FIELD,field: 'late', the door's sentenceGET .../import/jobs/:jobId/resultsSQLITE_ERROR, the driver's text, nofieldINVALID_FIELD,field: 'late', the door's sentenceUNIQUE_VIOLATION,field: 'code', the engine's sentenceSQLITE_CONSTRAINT_NOTNULL, "must is required.", nofieldWhy only the
INVALID_FIELDverdictThe claim's controls say a unique-conflict row and a NOT NULL row keep today's answers. The door answers both in other words, so routing the whole row through the mapper would move them:
UNIQUE_VIOLATION,field, and the engine's sentence ("Duplicate record refused on 'X': a unique constraint on 'code' already holds this value. No record was written."). Door: the samecodeandfield, but the curated sentence "A record with this code already exists" (no period), with the engine's sentence moved todeveloperMessage.SQLITE_CONSTRAINT_NOTNULLon SQLite), "must is required.", nofield. Door:400 VALIDATION_FAILEDwith arequiredfinding formust, "must is required" and a drifthint.These differences are named in the report on #20701. This PR does not change them.
Other dialects (read from source, NOT MEASURED)
The mapper's driver-string branch recognises SQLite
has no column named candno such column: c, MySQLUnknown column 'c'(the pattern is not anchored toin 'field list', so an update or where-clause spelling matches too), and Postgres throughmatchMissingColumnOfRelation, which matches only the write-path templatecolumn "c" of relation "t" does not exist. The Postgres read-path spellingcolumn "c" does not existis not matched, so an upsert match lookup on a drifted column in Postgres would keep the row's old answer. The mapper is not widened here.Pins
packages/rest/src/import-row-schema-drift-20701.test.ts, 5 cases through the public doors: the insert commit row, the upsert (update path) commit row and the async job's results row each equal the create door's{ code, field, error }and carry none of the driver's words; a writable row beside the drifted one is created (control); the unique-conflict and NOT NULL rows keep their answers (controls). The control pins say "unchanged by this PR", not "right": a card that converges them updates them on purpose.Verification (head
132e93c05b)Heavy steps ran under
scripts/pm/os-verify-lock.sh.turbo run build --filter='@objectstack/rest^...': 24 tasks successful.import-row-report-field-20701,import-runner-unique-violation-row,import-runner-error-sanitize,import-job-integration,import-integration,import-dryrun-parity,import-runner-bulk, and the throwaway probe):Test Files 9 passed (9),Tests 160 passed (160).pnpm --filter @objectstack/rest test:Test Files 243 passed (243),Tests 4858 passed | 114 skipped (4972). The--maxWorkers=2I passed after a bare--was dropped by vitest, so this ran with the default worker count; it is still the wholelocalproject. The package's onerepo-project test reads none of the changed files.pnpm --filter @objectstack/rest typecheck: exit 0,check:test-typecheck: OK;tsc -p tsconfig.test.json --listFilesOnlylists the new pin file.scripts/ablation-replace.mjsWRAP mode. The subject is read frompackages/rest/srcby relative import, so nodist/is involved. Anchorconst door = mapDataError(err, objectName).body;replaced by an empty door, 1 to 0; blobe8012ff2402atobf0b6d366a5c. Result over the new file and the fix(rest): an import row names its column from the error envelope, and the driver-string branch stops calling a declared field unknown #20905 pin file: 3 failed, 11 passed. The three are the insert, upsert and async-job drift cells, each back atSQLITE_ERRORwith the driver's text; the controls and the fix(rest): an import row names its column from the error envelope, and the driver-string branch stops calling a declared field unknown #20905 pins stayed green, as predicted. Restored:blob == HEAD (e8012ff2402a),git diff HEADempty.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsat132e93c05bderives 61 commands. Each ran with its exit code recorded before any pipe, and--ranreads "61 derived famil(ies) accounted for — 60 run, 1 NOT-MEASURED". NOT MEASURED:check:dual-build-cjs-loads,PREREQUISITE NOT MET(exit 3), because it needs every package'sdist/, which is the full build CI runs.check:dts-closure,check:sourcemap-no-sources-contentandcheck:type-check-debtwere re-run after building@objectstack/restand the six packages the debt re-measure needs, and read 0.132e93c05b. (1) eslint's own config lints the two.tsfiles in this diff, andisPathIgnoredis true for the changeset.md, so those two files are the diff's whole lint population. (2)eslint --no-inline-config --format jsonover them:files=2 errors=0 warnings=0. (3)calculateConfigForFileshowsparserOptions.projectandprojectServicenull on both, so type-aware linting is off and this diff cannot change eslint's verdict on any file it does not touch.Acceptance notes
engine.validatereads the metadata, never the table, so the preview promises a row the commit then fails. That is the card's own path ("an import preview promises only rows the commit will write"), and the report names it for the seat. The new pin file does not vouch for the dry-run reading.INVALID_FIELDwith no field-level finding now gets the door's answer on its row too, which is the business sentence the sandbox carried rather than the QuickJS debug wrapper. Read from source (classifyDataError's sandbox unwrap), NOT MEASURED.INVALID_FIELDkeepsanitizeRowError. That covers the two controls above, and any driver textsanitizeRowErrorpasses through because it does not read as SQL. The door's answer for those was not measured here.Generated by Claude Code