Repository navigation
Commit d7b9817
fix(rest): an import row for a NOT NULL refusal or a unique conflict answers what the create door answers (#20956)
Part of #20701
Clause-②: no
REST item 5 of #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 #20922.
## What changed
- **`packages/rest/src/import-runner.ts`.** `toFailedResult` widens item
4's adoption gate (PR #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 #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 #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 (`#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>1 parent de8cd58 commit d7b9817
4 files changed
Lines changed: 208 additions & 57 deletions
Lines changed: 27 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
20 | | - | |
| 19 | + | |
21 | 20 | | |
22 | 21 | | |
Lines changed: 117 additions & 39 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
4 | | - | |
5 | | - | |
6 | | - | |
7 | | - | |
8 | | - | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
9 | 11 | | |
10 | | - | |
11 | | - | |
12 | | - | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
13 | 19 | | |
14 | | - | |
| 20 | + | |
15 | 21 | | |
16 | | - | |
17 | | - | |
18 | | - | |
19 | | - | |
20 | | - | |
21 | | - | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
22 | 27 | | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
29 | 35 | | |
30 | 36 | | |
31 | | - | |
32 | | - | |
33 | | - | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
34 | 41 | | |
35 | 42 | | |
36 | 43 | | |
| |||
175 | 182 | | |
176 | 183 | | |
177 | 184 | | |
178 | | - | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
179 | 229 | | |
180 | 230 | | |
181 | 231 | | |
182 | | - | |
| 232 | + | |
183 | 233 | | |
184 | | - | |
185 | | - | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | | - | |
190 | | - | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
191 | 241 | | |
192 | 242 | | |
193 | | - | |
194 | | - | |
195 | | - | |
196 | | - | |
197 | | - | |
198 | | - | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
199 | 277 | | |
200 | 278 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
371 | 371 | | |
372 | 372 | | |
373 | 373 | | |
374 | | - | |
375 | | - | |
376 | | - | |
377 | | - | |
378 | | - | |
379 | | - | |
380 | | - | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
381 | 400 | | |
382 | 401 | | |
383 | 402 | | |
384 | | - | |
385 | | - | |
| 403 | + | |
386 | 404 | | |
387 | | - | |
388 | | - | |
389 | | - | |
390 | | - | |
391 | | - | |
392 | | - | |
393 | | - | |
| 405 | + | |
| 406 | + | |
394 | 407 | | |
395 | 408 | | |
396 | 409 | | |
| |||
402 | 415 | | |
403 | 416 | | |
404 | 417 | | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
405 | 452 | | |
406 | 453 | | |
407 | 454 | | |
| |||
0 commit comments