Repository navigation
Commit f80e2a6
fix(rest): an import row for a missing database column answers what the create door answers (#20941)
Part of #20701
Clause-②: no
Item 4 of the REST half of #20701 (the schema-drift row), as triage's
answer `5917724406` scopes 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 through `mapDataError`, the mapper `POST
/api/v1/data/:object` uses, for the same object. When the mapper's
verdict is `INVALID_FIELD`, the row carries the door's `code`, `field`
and 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`, `field`
set, 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 no
`field`. The sync route and the async job worker both run through
`runImport`, 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 own `INVALID_FIELD` envelope in the engine's own words.
`packages/rest/src/error-response.ts` is not edited: `mapDataError` is
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 `late` after the
DDL ran, with no re-sync.
| surface | before (base `67c1b11a20`) | after (`132e93c05b`) |
|:--|:--|:--|
| `POST /api/v1/data/:object` with `late` | `400 INVALID_FIELD`, `field:
'late'`, the drift sentence | unchanged |
| import commit, insert | failed, `SQLITE_ERROR`, "table proj_probe has
no column named late", no `field` | failed, `INVALID_FIELD`, `field:
'late'`, the door's sentence |
| import commit, upsert onto an existing row | failed, `SQLITE_ERROR`,
"no such column: late", no `field` | failed, `INVALID_FIELD`, `field:
'late'`, the door's sentence |
| async job, `GET .../import/jobs/:jobId/results` | failed,
`SQLITE_ERROR`, the driver's text, no `field` | failed, `INVALID_FIELD`,
`field: 'late'`, the door's sentence |
| import dry run | ok, created | ok, created (unchanged, see Acceptance
notes) |
| unique conflict row (control) | `UNIQUE_VIOLATION`, `field: 'code'`,
the engine's sentence | byte-identical |
| NOT NULL row (control) | `SQLITE_CONSTRAINT_NOTNULL`, "must is
required.", no `field` | byte-identical |
| writable row beside the drifted one (control) | ok, created | ok,
created |
## Why only the `INVALID_FIELD` verdict
The 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 conflict.** Row: `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
same `code` and `field`, but the curated sentence "A record with this
code already exists" (no period), with the engine's sentence moved to
`developerMessage`.
- **NOT NULL.** Row: the driver's code (`SQLITE_CONSTRAINT_NOTNULL` on
SQLite), "must is required.", no `field`. Door: `400 VALIDATION_FAILED`
with a `required` finding for `must`, "must is required" and a drift
`hint`.
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
c` and `no such column: c`, MySQL `Unknown column 'c'` (the pattern is
not anchored to `in 'field list'`, so an update or where-clause spelling
matches too), and Postgres through `matchMissingColumnOfRelation`, which
matches only the write-path template `column "c" of relation "t" does
not exist`. The Postgres read-path spelling `column "c" does not exist`
is 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`.
- **Dependency closure** `turbo run build
--filter='@objectstack/rest^...'`: 24 tasks successful.
- **Pins plus the neighbouring import suites** (the new file,
`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)`.
- **Package suite** `pnpm --filter @objectstack/rest test`: `Test Files
243 passed (243)`, `Tests 4858 passed | 114 skipped (4972)`. The
`--maxWorkers=2` I passed after a bare `--` was dropped by vitest, so
this ran with the default worker count; it is still the whole `local`
project. The package's one `repo`-project test reads none of the changed
files.
- **Typecheck** `pnpm --filter @objectstack/rest typecheck`: exit 0,
`check:test-typecheck: OK`; `tsc -p tsconfig.test.json --listFilesOnly`
lists the new pin file.
- **Ablation**, with the change committed first, through
`scripts/ablation-replace.mjs` WRAP mode. The subject is read from
`packages/rest/src` by relative import, so no `dist/` is involved.
Anchor `const door = mapDataError(err, objectName).body;` replaced by an
empty door, 1 to 0; blob `e8012ff2402a` to `bf0b6d366a5c`. Result over
the new file and the #20905 pin file: 3 failed, 11 passed. The three are
the insert, upsert and async-job drift cells, each back at
`SQLITE_ERROR` with the driver's text; the controls and the #20905 pins
stayed green, as predicted. Restored: `blob == HEAD (e8012ff)`,
`git diff HEAD` empty.
- **Gates**: `node scripts/pm/dispatch-gates.mjs --repo
objectstack-ai/objectstack --commands` at `132e93c05b` derives 61
commands. Each ran with its exit code recorded before any pipe, and
`--ran` reads "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's `dist/`, which is
the full build CI runs. `check:dts-closure`,
`check:sourcemap-no-sources-content` and `check:type-check-debt` were
re-run after building `@objectstack/rest` and the six packages the debt
re-measure needs, and read 0.
- **Lint**, narrowed with proof at `132e93c05b`. (1) eslint's own config
lints the two `.ts` files in this diff, and `isPathIgnored` is true for
the changeset `.md`, so those two files are the diff's whole lint
population. (2) `eslint --no-inline-config --format json` over them:
`files=2 errors=0 warnings=0`. (3) `calculateConfigForFile` shows
`parserOptions.project` and `projectService` null on both, so type-aware
linting is off and this diff cannot change eslint's verdict on any file
it does not touch.
- Control-byte self-scan over the three changed files: clean.
## Acceptance notes
- **The dry run still previews a drifted column as ok.**
`engine.validate` reads 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.
- **A sandboxed producer that throws `INVALID_FIELD` with 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.
- **Rows whose door verdict is not `INVALID_FIELD` keep
`sanitizeRowError`.** That covers the two controls above, and any driver
text `sanitizeRowError` passes through because it does not read as SQL.
The door's answer for those was not measured here.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01VvcEokUG1tvVxkceYfR5XB)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 3fbf3ca commit f80e2a6
3 files changed
Lines changed: 260 additions & 4 deletions
File tree
- .changeset
- packages/rest/src
| 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 | + | |
| 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 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| |||
353 | 353 | | |
354 | 354 | | |
355 | 355 | | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
356 | 381 | | |
357 | | - | |
| 382 | + | |
358 | 383 | | |
359 | 384 | | |
360 | 385 | | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
361 | 395 | | |
362 | 396 | | |
363 | 397 | | |
| |||
776 | 810 | | |
777 | 811 | | |
778 | 812 | | |
779 | | - | |
| 813 | + | |
780 | 814 | | |
781 | 815 | | |
782 | 816 | | |
| |||
892 | 926 | | |
893 | 927 | | |
894 | 928 | | |
895 | | - | |
| 929 | + | |
896 | 930 | | |
897 | 931 | | |
898 | 932 | | |
| |||
0 commit comments