Skip to content

Commit 41f9f46

Browse files
huangyiireneclaude
andauthored
test(runtime): pin the multi-value cascade-delete path on PostgreSQL, not only SQLite (#18732)
Fixes #18617 Clause-②: no ## What moved `packages/runtime/src/cascade-delete-multivalue-lookup-real-driver.integration.test.ts` was the only real-stack pin of the multi-value cascade-delete path — ObjectQL over a real `SqlDriver` through the protocol data-plane delete — and it hard-coded `client: 'better-sqlite3'`. That literal is the named mechanism by which #18172 reached two published releases: every DELETE of an object targeted by a `multiple: true` reference answered 500 on PostgreSQL, always, and this pin stayed green throughout. The file now declares a driver axis: the embedded SQLite cell plus a live PostgreSQL cell provisioned by `OS_TEST_POSTGRES_URL`. Everything below the one `newDriver()` seam is written once and measured on both. `packages/runtime/src/expected-read-refusal-noise.ts` had to move with it, and that is declared here rather than buried — see **In-place repair** below. ## The measurement — this pin was shown to go RED before it was allowed to be green The acceptance bar was not coverage. Ablation: `SqlDriver.applyJsonMembership` made to answer `false` unconditionally behind a runtime-evaluated marker, so the cascade probe's `$contains` falls through to the pre-#17590 substring emitter. driver-sql rebuilt each leg (runtime's tests resolve `@objectstack/driver-sql` through `exports`, i.e. `dist/`), and `scripts/ablation-dist-preflight.mjs` used to prove the mutation reached the artifact before any colour was read. ``` mutate -> on-disk anchor 1, marker 1; blob d25e03f4 vs HEAD 2a17fc1 -> rebuild -> preflight: marker present in 2 built files -> vitest: 6 failed | 9 passed (15) ALL SIX failures in "(live postgres)" — ZERO in "(better-sqlite3)" restore -> git checkout HEAD -- THE_PATH; git diff HEAD empty; marker 0 on disk -> rebuild -> preflight --absent: marker absent from all 6 built files, working tree clean against HEAD -> vitest: 15 passed (15) -> final blob 2a17fc1 == HEAD blob ``` The fault reproduced byte for byte, on a live PostgreSQL 16.13 with `timezone=Asia/Shanghai`: ``` [sql-driver] DATABASE_ERROR - the backend refused a read on 'zz_field_zoo' (42883). ... select * from "zz_field_zoo" where "f_lookups" LIKE $1 ESCAPE $2 - operator does not exist: json ~~ text Serialized Error: { code: '42883', file: 'parse_oper.c', routine: 'op_error' } ``` That asymmetry is the whole card. A `multiple: true` lookup is a TEXT column on SQLite and a real `json` column on PostgreSQL — measured through `information_schema` on the live server, and now asserted as this cell's non-vacuity guard. On TEXT the membership lowering and the substring lowering are indistinguishable, so no assertion written against SQLite can separate them. Only the live cell can, and now it does. ## Without a server The unprovisioned cell is a NAMED skip that says which variable would run it, never a silent omission — the suite is emitted either way, because a cell that emits nothing at all is not a skip and vitest's summary counts would read as coverage. Under `OS_EXPECT_LIVE_DIALECT_MATRIX=1` the missing URL is a failure instead, so a runner that declared it provisioned a server cannot quietly degrade to SQLite-only. Measured: the full `@objectstack/runtime` suite without a URL reports `3661 passed | 1 skipped`, and that one skip is this cell naming itself. ⚠️ Read the limit honestly: a named skip is a report, not coverage — see **Not done here**. ## Isolation The live cell owns a PostgreSQL schema named from this file's repo-relative path (`os_lv_cascade_delete_multivalue_lookup_r_b7005a0a8835`), recreated per test and dropped in `afterAll`. Object names here are short and generic (`zz_account`, `zz_guard`) and CI provisions ONE server for every live leg, so a shared schema would not be contention but destruction. Same derivation as driver-sql's `live-dialect-matrix.testkit.ts` and metadata-protocol's `live-mysql-database.testkit.ts` — same `os_lv_` prefix, same 34-character slug cap, same 12 hex of sha256 over the workspace-relative path — so the copies stay jointly injective on that one server. `pnpm check:live-db-isolation` green. ## In-place repair, declared: the noise capture recognised only SQLite `expected-read-refusal-noise.ts` matched its reason half against the literal `no such table: TABLE_NAME`, which is SQLite's phrasing and only SQLite's. On PostgreSQL the driver writes `relation "sys_organization" does not exist`, so the fail-soft tenancy probe's refusal was never recognised: the capture printed the noise it exists to withhold AND `silentChannels()` reported the channel silent. Measured — all eight cases of the live cell red on that assertion, with the driver's `(42P01)` line sitting in the log immediately above it. Relaxing the pin is explicitly forbidden by its own docblock, and the pin was not wrong; the recogniser was SQLite-shaped. So the reason half became the caller's: `MissingTableReason`, defaulting to SQLite's sentence so all eighteen existing call sites are byte-unchanged, with the PostgreSQL spelling exported beside it and this matrix picking per cell through a `Record` keyed on the cell id — a new cell cannot be added without answering the question. Neither half is widened: the envelope still pins the table positively and the reason must still be that dialect's missing-table sentence about that same table, so a permission denial, a dropped connection or a syntax fault still fails it on every dialect. ⚠️ The first cut reached for `isMissingTableError` in `@objectstack/types` instead, and that was measured and withdrawn. This module is imported by RELATIVE path out of `trigger-record-change` and `plugin-approvals`, so a bare workspace specifier added to it lands in THEIR resolution domain: `check:test-source-alias` went red naming `@objectstack/trigger-record-change: NEW unaliased artifact import(s) ... @objectstack/types`, and green again on the reverted tree (controlled revert, both directions run). The gate's prescribed repair is an alias in those packages' vitest configs — outside this card's file surface and a different defect class — so the design changed instead of the surface. MySQL is deliberately absent from that vocabulary: its sentence is database-qualified (`Table 'db.t' doesn't exist`), the plain fragment form cannot carry it, and no fixture here rigs MySQL. Declaring an entry nothing exercises is the declared-not-enforced trap. It arrives with the fixture that needs it. ## Not done here: the CI leg (outside this card's file surface) ⚠️ The `Temporal Conformance (live PG + MySQL)` job runs driver-sql, the non-SQL temporal backends, and metadata-protocol. It does NOT run `@objectstack/runtime`, so nothing in CI hands this cell a URL and the cell will report itself un-run on every job until that changes. The honest wiring is two steps in `.github/workflows/ci.yml`, in the job that already provisions the postgres service: ```yaml - name: Build runtime and its dependencies run: pnpm exec turbo run build --filter=@objectstack/runtime... --concurrency=4 - name: Run the runtime cascade-delete matrix against live PostgreSQL env: OS_TEST_POSTGRES_URL: postgres://postgres:postgres@127.0.0.1:5432/postgres OS_EXPECT_LIVE_DIALECT_MATRIX: '1' run: | pnpm --filter @objectstack/runtime exec vitest run --project local \ cascade-delete-multivalue-lookup-real-driver ``` The positional is a SUBSTRING, not a glob — the same reading the metadata-protocol step records for itself, where the glob form matched zero files. `.github/workflows/**` is outside the declared surface for this card, so no workflow is edited here; this is reported for the seat to place. ## Does this fold into #18200? Measured, and the reading is no. #18200 is about WHICH FILES a local green covers — a `driver-sql` run blind to 11 of its 188 files. This card is about WHICH DIALECT one file runs on. Running all 188 driver-sql files would not have caught #18172, because the only real-stack pin of the cascade path does not live in driver-sql at all; it lives in `packages/runtime` and it ran, green, on the one dialect that cannot fail. Two different mechanisms with the same symptom. #18200 is not addressed here and remains open. ## Scope and grading - **Clause-②: no** — measured, not assumed. Both changed files live in `packages/runtime/src/` and neither is reachable from `src/index.ts`, which is tsup's single entry. Grepping the built artifact across the package's whole `files[]` (`dist`, `README.md`, `CHANGELOG.md`): `captureExpectedReadRefusals`, `POSTGRES_MISSING_TABLE_REASON`, `MissingTableReason`, `zz_field_zoo`, `os_lv_`, `OS_TEST_POSTGRES_URL` — 0 files each. Positive control on the same built tree: `AppPlugin` 6 files, `DriverPlugin` 4. Nothing published moves. - **skip-changeset** — same measurement. No released package ships anything this diff touches. - `needs:contract-review` is the seat's label; this PR neither hangs nor removes it. ## Verification Run on `bb71ba4526` (this branch merged with `origin/main` through `scripts/pm/os-regen-merge.sh`), every heavy run through `scripts/pm/os-verify-lock.sh`. | what | reading | |---|---| | `@objectstack/runtime` suite (no PG URL) | 265 files, `3661 passed \| 1 skipped` | | the matrix file, live PG + `OS_EXPECT_LIVE_DIALECT_MATRIX=1` | `15 passed (15)` — 7 SQLite, 8 live postgres | | ablation, live PG | `6 failed \| 9 passed` — all six in the live cell, none in SQLite | | `@objectstack/runtime typecheck` | green (test layer 27 files / 191 errors / 69 pinned signatures, unchanged) | | `@objectstack/trigger-record-change test` | `101 passed (101)` — cross-package consumer of the changed helper | | `@objectstack/plugin-approvals test` | `764 passed (764)` — same | | `@objectstack/spec check:generated` | all 15 generated artifacts up to date (post-merge) | | derived gate families (`scripts/pm/dispatch-gates.mjs`) | 50 derived, 49 run green, 1 NOT MEASURED | | `pnpm check:live-db-isolation` | green (outside the derivation; run anyway) | | `eslint . --no-inline-config` | exit 0 over the whole population — 6822 files, 0 findings, on `bb71ba4526` | `pnpm check:dual-build-cjs-loads` is the one NOT MEASURED: it exits 3 with `PREREQUISITE NOT MET` because it reads built output for ~30 packages this worktree never built. That is not a pass and not a finding; CI builds the repo and measures it there. ## Acceptance notes Observations found on the way, filed nowhere and fixed nowhere, per Prime Directive #10: - 29 other test files under `packages/runtime/src/` also hard-code `client: 'better-sqlite3'`. **noted, not filed** — none of them carries #18617's receipt (a dialect-specific defect that shipped), most pin behaviour with no dialect axis at all, and converting them wholesale would be exactly the scope expansion the directive forbids. The carrier for the next one is whichever card names a dialect-specific defect on that path. - `expected-read-refusal-noise.ts` is imported by RELATIVE path from two packages outside `packages/runtime` (`trigger-record-change/src/record-change-integration.test.ts`, `plugin-approvals/src/status-mirror-cascade.integration.test.ts`), which is what makes any bare workspace import added to it a change to their resolution domains. **noted, not filed** — it is a coupling, not a defect, and it is now recorded in the module's own header where the next author will meet it. --- _Generated by [Claude Code](https://claude.ai/code/session_01CqmCgU5RGDoJYhHUMVp2af)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 30bac28 commit 41f9f46

2 files changed

Lines changed: 409 additions & 14 deletions

File tree

0 commit comments

Comments
 (0)