test(stack): EQL v3 integration harness + shared test kit, and two Supabase adapter fixes (PR1 of 4)#616
Draft
coderdan wants to merge 18 commits into
Draft
Conversation
Plan for a comprehensive real-crypto integration suite across the EQL v3 adapters, and the three refactors it unblocks. Reviewing #535 surfaced four structural gaps: - The Supabase v3 adapter has zero real-crypto coverage. Its only "live" suite stubs ZeroKMS with `hm-${plaintext}`, so equality assertions are true by construction. - The canonical per-domain types from `@cipherstash/eql` are imported nowhere; both adapters erase payloads to `unknown` and drop the Result wrapper. - v3 JSON is unimplemented, not merely untested — the bundle ships eql_v3_json but the SDK admits no 'json' cast kind. - The Supabase and Drizzle adapters ship from inside stack, unlike prisma-next. Sequenced tests first, so each later refactor lands against a suite that proves query correctness: PR1 harness + matrices, PR2 type robustness, PR3 adapter package split, PR4 v3 JSON. `docs/` does not ship in the stash tarball, so no changeset.
Two defects found reviewing #535, both verified against a real PostgREST. **Raw `.filter(col, 'in', […])` encrypted the whole list as one ciphertext.** The raw-filter path reached `in` with none of the element-splitting the `in()`, `not(…, 'in', …)` and `.or()` paths perform. The two wire formats then failed differently, which is why it went unnoticed: v2's `("json")` composite literal is already parenthesized, so PostgREST parsed it as a one-element list and answered `200 []` — a filter that silently matched nothing. v3's bare `{…}` envelope is not, so PostgREST rejected the request with PGRST100. Each element is now encrypted separately and the operand rendered as a quoted list literal, reusing `collectInListTerms` and `formatInListOperand`. As on the `not` path, a PostgREST list literal now throws — pass an array. Plaintext columns are untouched, including postgrest-js's own quirk of rendering `.filter(col,'in',[array])` as an unparenthesized `in.a,b`. **Aliased date columns lost their `Date` reconstruction.** `selectKeyToDb` was dropped from `buildSelectString`, so `.select('ts:createdAt')` returned an ISO string where the typed surface promises a `Date`. Restored by extracting `resolveSelectToken`, shared by `addJsonbCastsV3` and the new `selectKeyToDbV3` — the keys reported are now, by construction, the keys the cast helper causes PostgREST to emit, so the two cannot drift again. Six new tests, four of which fail against the unfixed source.
…ackage The integration suites need one catalog, one oracle, and one operation table shared across packages: after the adapter split, Supabase and Drizzle tests live in different packages, and neither may reach into stack's __tests__ tree. Duplicating the catalog would let the two copies disagree about which domains exist — exactly the drift the `satisfies Record<EqlV3TypeName, DomainSpec>` was written to prevent. `packages/test-kit` is private, unpublished, and has no build step; it is consumed as TypeScript source. That keeps `pnpm test` independent of `pnpm build`, which matters because the dts build is currently fragile. Resolution is aliased in three places that must stay in step: `vitest.shared.ts` (runtime), `packages/test-kit/tsconfig.json` and `packages/stack/tsconfig.json` (compile time). `tsc` does not read Vitest's aliases — without the tsconfig half, the catalog's builders lose their precise return types and the `.test-d.ts` matrix silently collapses to `never`. New `DomainSpec.deferred` records why a domain is skipped rather than dropping its row, so removing coverage stays a visible diff. The nine block-ORE domains are deferred: their opclass is superuser-only, so they are absent on managed Postgres. `test-kit-families.test.ts` asserts the families partition the catalog. The `satisfies Record<…>` only forces a row to EXIST; a domain whose slug matched no family prefix would belong to no test file and go untested while the suites stayed green. That test immediately caught such a bug: `eqlTypeSlug` strips only the `public.` schema, so slugs are `eql_v3_integer_ord`, and the family prefixes matched nothing. Verified: catalog coverage still fails the build when a domain row is removed (named explicitly); 1554 unit tests pass; stack and test-kit both typecheck.
Adds the two database variants the integration suites run against, installs
EQL v3 through the real `stash eql install`, and replaces the credential
`describe.skipIf` gates with a failure that names what is missing.
Installing via the CLI (not a hand-rolled SQL bundle apply) means an installer
regression fails the integration jobs instead of hiding behind a test-only code
path that reimplements half of it. `--supabase --direct`: the first applies the
eql_v3 + eql_v3_internal grants, the second suppresses an interactive prompt
that would hang CI.
Measured, not assumed, against `supabase/postgres:17.4.1.048`:
- The CLI installs fine as the NON-superuser `postgres` role. No superuser
connection, no SUPABASE_ADMIN_URL — the plan had budgeted for both.
- `_ord_ore` domains are NOT "absent" on managed Postgres, as the plan claimed.
All 51 domains and all 33 ORE comparison functions install; only the btree
opclass is skipped (`pg_opclass` count 0 vs 1 on plain Postgres). So ORE
columns are creatable and `eql_v3.gt`/`lt` still compare correctly, but
`ORDER BY ord_term_ore` silently falls back to bytea order. Corrected in the
catalog's deferral reason and in the plan doc; pinned by a harness test that
asserts the opclass count against `rolsuper`.
Three fixture bugs found by running it rather than reasoning about it:
- `authenticator` is a RESERVED role; even `postgres` cannot set its password
("only superusers can modify it"). It must come from an initdb script.
- That script must sort AFTER `migrate.sh`, which is where the image creates
the roles. A `99-` prefix sorts BEFORE it (`'9' < 'm'`) and aborts the boot.
- `pg_isready` is not a readiness check for either image: both boot a temporary
server for init that listens on the unix socket only. `up --wait` returned
while the real server was still restarting and the EQL install died with
"Connection terminated unexpectedly". The postgres variant now probes TCP;
the supabase variant probes the last thing init does.
Ports are 55432/55433/55430, never the defaults: a developer running this very
likely also has `local/docker-compose.yml`, a `supabase start` stack, or their
own Postgres on 5432, and shadowing the database they think they are talking to
is worse than failing to bind.
Verified end to end: unconfigured runs fail loudly with an actionable message;
a `~/.cipherstash` profile stands in for the four CS_* vars; both variants boot
clean and install with no sleeps; `pnpm test` still runs 1554 unit tests and
zero integration tests.
Follow-up to the harness commit, which said `ORDER BY ord_term_ore` "falls back to raw bytea ordering" without explaining the mechanism or bounding the damage. Measured on the two variants: - `eql_v3_internal.ore_block_256` is a COMPOSITE type, not a domain over bytea. Ordering it resolves the btree opclass `ore_block_256_operator_class`, which self-skips on `insufficient_privilege`. With no opclass, Postgres does not error — it falls back to built-in RECORD comparison, which walks down to the raw `bytes` field and compares bytewise. - Bytewise order over ORE ciphertext is deterministic and stable but uncorrelated with plaintext order: over 200 random well-formed terms it disagreed with the ORE comparator on 87. A stable, plausible-looking, silently wrong ordering. - RANGE FILTERS ARE UNAFFECTED. The `<`/`>`/`<=`/`>=` operators are backed by `ore_block_256_lt` → the ORE comparator, and operators need no opclass. Only `ORDER BY`, which resolves an opclass rather than an operator, takes the fallback. A btree index on the column cannot be created at all. Proof the two variants take different code paths: `ORDER BY` over a malformed 1-byte term returns rows on supabase/postgres, and raises `Malformed ORE term` from `ore_block_256_lt` on plain Postgres. Comment-only; no behaviour change.
🦋 Changeset detectedLatest commit: f67f448 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a shared EQL v3 integration testing foundation for @cipherstash/stack, centered around a new internal @cipherstash/test-kit workspace package and a dedicated packages/stack/integration/ Vitest configuration, while also landing two Supabase v3 adapter correctness fixes (raw filter(..., 'in', ...) element-wise encryption + date reconstruction under select aliases).
Changes:
- Add
packages/test-kit(domain catalog + plaintext oracle + row planning + adapter interface + env/CLI install helpers) and wire source-level resolution viavitest.shared.ts+ TSpaths. - Add Stack integration test harness (
packages/stack/integration/*) and local Docker variants (local/docker-compose.*) to run real EQL v3 installs via the builtstashCLI. - Fix Supabase v3 raw
infilter encryption semantics and ensure date-like columns reconstruct toDateeven when selected under caller-chosen PostgREST aliases.
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vitest.shared.ts | Centralizes Vitest resolve.alias mappings to ensure @cipherstash/test-kit and Stack subpath imports resolve to source (not dist). |
| turbo.json | Adds a test:integration Turbo task to support integration suite execution as a separate pipeline. |
| pnpm-lock.yaml | Records the new packages/test-kit workspace importer and its dev dependencies. |
| packages/test-kit/package.json | Defines the private @cipherstash/test-kit package (source exports, no build) and test:types script. |
| packages/test-kit/tsconfig.json | Establishes compile-time paths to Stack source and Stack’s internal @/* alias so the kit typechecks against source. |
| packages/test-kit/src/index.ts | Public entrypoint exporting catalog/oracle/driver utilities used by unit + integration suites. |
| packages/test-kit/src/catalog.ts | Moves the EQL v3 domain catalog into the shared kit, preserving compile-enforced coverage via satisfies Record<...>. |
| packages/test-kit/src/families.ts | Introduces plaintext “family” grouping to drive per-family integration suites and prevent unowned domains. |
| packages/test-kit/src/oracle.ts | Provides plaintext oracle helpers (plainValue, comparisons, expected key sets, ordering model). |
| packages/test-kit/src/rows.ts | Plans per-run tables and seed rows (including single-vs-bulk insert splits) for integration suites. |
| packages/test-kit/src/ops.ts | Derives positive/negative operation sets from capability flags + adapter support. |
| packages/test-kit/src/adapter.ts | Defines the IntegrationAdapter interface used to keep adapter suites aligned. |
| packages/test-kit/src/env.ts | Implements integration env gating with actionable error messages (and supports ~/.cipherstash profile). |
| packages/test-kit/src/install.ts | Installs EQL v3 by shelling out to the built stash eql install --eql-version 3 CLI (real installer surface). |
| packages/stack/vitest.config.ts | Adds sharedAlias and excludes integration/** from regular pnpm test runs. |
| packages/stack/integration/vitest.config.ts | Adds dedicated integration Vitest config (timeouts, global setup, aliasing, no file parallelism). |
| packages/stack/integration/global-setup.ts | Fails fast on missing env and runs one-time EQL v3 install via installEqlV3. |
| packages/stack/integration/harness.integration.test.ts | Verifies the harness itself (EQL version, domains, opclass behavior, Supabase grants, PostgREST reachability). |
| packages/stack/tsconfig.json | Adds compile-time paths aliases for @cipherstash/test-kit and Stack public subpaths to keep type-level matrix precise. |
| packages/stack/package.json | Adds test:integration script to run integration suites via the dedicated config. |
| packages/stack/src/supabase/query-builder.ts | Fixes encrypted raw filter(..., 'in', [...]) to encrypt elements individually and reassemble a PostgREST list literal; rejects list-literal strings on encrypted columns. |
| packages/stack/src/supabase/query-builder-v3.ts | Restores/strengthens date reconstruction under select aliases via a select-key→db-name map shared with cast emission. |
| packages/stack/src/supabase/helpers.ts | Extracts resolveSelectToken, reuses it for both addJsonbCastsV3 and new selectKeyToDbV3 mapping. |
| packages/stack/tests/v3-matrix/catalog.ts | Replaces the in-test-tree catalog with a shim re-export from @cipherstash/test-kit/catalog. |
| packages/stack/tests/test-kit-families.test.ts | Adds unit test to ensure family partitioning covers all catalog domains (covered + deferred). |
| packages/stack/tests/supabase-v3-pgrest-live.test.ts | Adds real-server assertions for .in() and raw .filter('in', ...) encrypted behavior plus alias-driven Date reconstruction. |
| packages/stack/tests/supabase-v3-builder.test.ts | Pins builder-level behavior for raw in element splitting and alias-driven Date reconstruction (mocked). |
| local/docker-compose.postgres.yml | Adds dedicated plain Postgres compose variant for Drizzle integration suites (non-default port, reliable healthcheck). |
| local/docker-compose.supabase.yml | Adds Supabase-flavored compose variant (pinned digests), authenticator password init, and healthcheck tuned for init behavior. |
| local/supabase-init.sql | Sets authenticator password for PostgREST connectivity and documents ordering/superuser constraints. |
| docs/superpowers/plans/2026-07-10-eql-v3-integration-tests.md | Adds the overall multi-PR plan document for the integration harness and follow-on work. |
| .github/workflows/tests.yml | Adds a CI step to typecheck @cipherstash/test-kit so catalog coverage enforcement runs in CI. |
| .github/actions/integration-setup/action.yml | Adds a composite action to standardize integration job setup (deps + build stash). |
| .changeset/supabase-in-list-operands.md | Updates changeset notes to document the raw filter(..., 'in', [...]) behavior fix and its impact. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…al crypto
First real-crypto coverage of the Supabase v3 adapter. 62 tests against real
ZeroKMS ciphertext, a real PostgREST, and supabase/postgres.
The driver (`@cipherstash/test-kit/suite`) derives everything from the domain's
catalog row: which operations must work, which must be rejected, and what rows
each should return, computed from a plaintext oracle. A family file is three
lines. The adapter is built through the public `encryptedSupabaseV3` factory so
the shipped construction path runs — introspection over `pg`, declared-schema
verification, a real `Encryption({eqlVersion: 3})` client.
Both the `in()` method and the raw `filter(col, 'in', […])` path are driven
against the same oracle, because they are different code paths. Rows are split
into disjoint interleaved halves — single-encrypt {a,c}, bulk-encrypt {b,d} — so
any predicate matching more than one row necessarily spans both, and an explicit
crossover assertion pins it.
Verified the suite is worth having, rather than assuming it:
- Reverting `src/supabase/` to the base commit fails exactly four tests, all of
them `filter(in)` — the raw-filter bug this PR fixes, caught end to end.
- Removing `equality` from `integer_eq`'s catalog row flips its four positive
tests into rejection tests, which then fail because the column really does
support equality. The rejection matrix is derived, not decorative.
Two integration hazards found by running it:
- `@cipherstash/test-kit`'s barrel must not import `vitest`, even transitively.
`globalSetup` runs in a different context from the test workers and imports
that barrel, so a `vitest` reached from there dies with "Vitest failed to
access its internal state". The driver therefore lives behind its own subpath.
- The kit resolves to source in another package, outside this config's root, so
Vitest externalizes it and loads it through Node rather than the transform
pipeline. It must be listed in `server.deps.inline`.
…uction
609 tests across the nine plaintext families, on real ZeroKMS ciphertext against
supabase/postgres + PostgREST. Every domain the SDK models except the nine
deferred block-ORE ones, every operation its capabilities allow, every operation
they forbid, both encrypt paths, against a plaintext oracle.
`select-alias.integration.test.ts` covers what the family driver cannot express:
`Date` reconstruction across all three ways PostgREST can key a row — the JS
property, the raw DB column name, and a caller-chosen alias — plus `select('*')`.
It is a projection concern, not a query operation.
Proof the suite earns its keep. Against the base commit's `src/supabase/`:
41 failed | 568 passed
- 38 × `filter(in)`, on every eq-capable domain in every family: the raw
filter path encrypted the whole in-list as one ciphertext.
- 3 × aliased `Date`: `selectKeyToDb` was dropped from `buildSelectString`.
With the fixes restored: 609 passed, 0 failed.
A silent skip found and removed. `contains` derived its needle from the
domain's MINIMUM sample, which for text is the empty string — shorter than
`token_length`, so `it.runIf` skipped it. `text_match` is the only match-only
domain, and free-text search is its only capability: the suite was silently not
testing the one thing that domain exists to do. The needle is now taken from the
first sample long enough to build one, and a domain that cannot produce one
throws instead of skipping.
That is the same failure this whole PR exists to eliminate — a test that appears
to cover something and does not — reproduced inside the harness meant to prevent
it. Worth stating plainly rather than quietly fixing.
A single 3-character needle was insufficient coverage for free-text search, and the reason is subtle: its whole-value token IS a trigram, so it matches whether or not `include_original` is honoured. That is the degenerate case, and it was the only case the suite exercised. The match index blooms downcased 3-grams; `include_original: true` additionally blooms the whole value as one token. Storage wants that. A QUERY operand must not have it, or the needle's bloom carries a whole-needle token the haystack's bloom cannot contain, and every strict-substring search returns zero rows. Both v3 adapters build match operands with `encrypt` — the full storage envelope — rather than `encryptQuery`, because PostgREST cannot cast a filter value to `eql_v3.query_*`. Both are therefore structurally exposed. It is masked today only because protect-ffi ignores `include_original` outright, so neither side carries the token: two bugs cancel. When protect-ffi is fixed, substring search breaks silently. Two layers of coverage: - The text family now searches four needles per match-capable domain: one trigram (degenerate), a strict interior substring longer than a trigram (the discriminating case), the whole stored value, and a needle absent from every value. - `match-bloom.integration.test.ts` asserts the precondition directly on the wire — bloom(needle) must be a subset of bloom(haystack) for every substring — so a failure names its cause instead of surfacing as "a query returned no rows". It also proves its own sensitivity: the honest operand is contained, and unioning in the bits of one value the haystack never saw is enough to break the subset. An `include_original` token is exactly such a foreign token. These go red the day protect-ffi honours the flag. That is the intent: the fix is to stop the operand carrying the original token, not to relax the tests.
Completes the Supabase half of PR1. 655 tests now run in one job against real ZeroKMS ciphertext, a real PostgREST, and supabase/postgres. - `supabase-v3-pgrest-live.test.ts` moves to `integration/supabase/wire. integration.test.ts`, de-gated. It is NOT superseded by the real-crypto matrix: it uniquely proves the 23514 rejection of a narrowed encryptQuery term (a shape the adapter cannot produce, so it must be hand-built), dense PostgREST parse edges, plaintext-passthrough containment, and the grants as `anon`. Its stubbed ZeroKMS is the point — it needs no credentials. Its `installEqlV3IfNeeded` + `SUPABASE_PERMISSIONS_SQL_V3` calls are gone: `globalSetup` installs both by running the real `stash eql install --eql-version 3 --supabase --direct`. Re-applying them here would only have tested a hand-rolled approximation of the installer. - `LIVE_SUPABASE_PGREST_ENABLED` / `describeLiveSupabasePgrest` are deleted along with the `live-coverage-guard` assertion that policed them. The guard existed because a false gate meant a silent whole-suite skip on a green job; a suite that throws needs no such guard. The remaining `LIVE_*` gates and their assertions stay — their suites have not moved. - `tests.yml` loses the PostgREST step and `PGRST_URL`. The moved suite was its only consumer, so the unit job no longer starts a container it does not need. - `integration-supabase.yml` runs the suites on `supabase/postgres` + PostgREST, fork-PR gated, secrets passed as job env rather than written to a `.env`. `CS_IT_SUITE` scopes the run to the suites this database serves, so the Drizzle suites (plain Postgres, no PostgREST) can have their own job without either provisioning the other's dependencies. Verified: 655 pass / 8 skipped under the exact CI invocation; `pnpm test` still runs 1554 unit tests and zero integration tests; both typechecks clean.
…base The v3 Supabase adapter rejected `order()` on every encrypted column, on the grounds that PostgREST cannot emit `ORDER BY eql_v3.ord_term(col)`. True, but it can emit a jsonb path — and the path selects exactly that term. Measured against a live PostgREST over 10 rows: order=amount->>op.asc -> plaintext order (integer_ord) order=amount->>op.desc -> plaintext order order=name->>op.asc -> plaintext order (text_search) order=amount.asc -> r00,r04,r08,r01,… (sorts the envelope) So the bare-ORDER-BY objection was right and the conclusion was wrong. The builder now rewrites an encrypted ordering column to `col->>op` through a new `orderColumnName` seam — deliberately separate from `filterColumnName`, since filters compare whole envelopes through the `eql_v3.*` operators and must not see the path. Rejection is now keyed on the ordering flavour rather than on encryption: `ope` is supported, `ore` is refused (its `ob` term needs the superuser-only opclass, and such a column cannot hold data on managed Postgres at all), and columns with no ordering term are refused as before. `V3OrderableKeys` widens accordingly. `is(col, true)` had been borrowing the orderable key set — the comment there said the two axes were "threaded separately so they can diverge", and now they have — so it gets its own `V3PlaintextKeys` and stays plaintext-only. The integration matrix asserts ordering against the plaintext oracle for all ten OPE-backed domains, in both directions. Reverting `orderColumnName` to a bare column name turns them red, so they discriminate rather than merely pass. Ties are broken on `row_key` in both the oracle and the query — date and timestamp have two samples across three rows, so without that the test would flake instead of proving order. Also collapses the nine three-line family files into one that iterates `FAMILY_NAMES`, so a family added to the kit is covered without touching it.
…tly wrong Measured on supabase/postgres as the non-superuser `postgres` role: the ORE domains are created, but their CHECK calls `ore_domain_unavailable()` and the first INSERT raises, with a hint naming the alternatives. The same insert succeeds on plain Postgres as a superuser. So ORE is loudly unusable on managed Postgres, not silently wrong. The bundle guards correctly. The earlier reason — 'ORDER BY silently mis-sorts' — described a type-level property (`ore_block_256` is a composite type, so with no opclass Postgres falls back to record comparison) that no value can reach, because the write is refused first. The deferral still stands, for a better reason: a matrix that must pass on both a superuser and a managed database cannot seed an ORE column at all. Corrects the claim in the plan doc, and in cipherstash/encrypt-query-language#395.
`->` (jsonb) rather than `->>` (text), matching the canonical EQL form `ORDER BY eql_v3.ord_term(col)` more closely — it keeps the comparison on the typed value. It does NOT avoid the database collation, which is worth saying plainly because it is the obvious reason to prefer it. Postgres compares jsonb strings with `varstr_cmp` under the default collation, exactly as it does text. Measured on `en_US.UTF-8`: `ORDER BY j->'op'` and `ORDER BY j->>'op'` both put `'a'` before `'B'`, while `COLLATE "C"` does the reverse. The two operators are equivalent for ordering. What actually makes the jsonb path agree with `ord_term()` is the term's encoding, so `ope-term.integration.test.ts` asserts it directly: - lowercase hex (`[0-9a-f]`), so every collation orders it identically — digits before letters, hex letters among themselves; - FIXED width on numeric and date domains (130 chars for `integer_ord`), where positional comparison equals numeric comparison only if the block count is constant; - per-character on text (16 hex chars each plus a 2-char header: `ada` -> 50, `adam` -> 66, `zebra` -> 82), so lexicographic order compares the plaintexts character by character and a prefix sorts first — the semantics of comparing the strings themselves. Change that encoding to variable-width numerics, uppercase, or base64, and ordering through PostgREST silently stops matching `ord_term()`. That file is the tripwire. The Drizzle adapter emits `ord_term()` directly and is unaffected.
Both adapters are now driven by one catalog, one plaintext oracle and one driver, so they cannot claim different coverage. Drizzle: 617 tests. Supabase: 665. Where they legitimately differ they differ in `supportedOps`, not in what a shared test asserts. Ordering is where they finally agree. Drizzle emits the canonical `ORDER BY eql_v3.ord_term(col)`; Supabase orders the same `op` term through the jsonb path `col->op`. Both satisfy the identical oracle, on every OPE-backed domain, in both directions. `operators-live-pg.test.ts` becomes `drizzle-v3/relational.integration.test.ts`, de-gated. Its per-domain `it.each` blocks — eq/ne/inArray/notInArray, ranges, between, asc/desc, contains, and the capability rejections — are deleted: the driver derives all of them from the catalog. What remains is what the driver cannot express, because it is about SQL shape rather than about a domain: `and`/`or`/`not` over disjoint predicates, `exists`/`notExists`, joins, pagination, the free-text needle guards, and a typed bigint round-trip. Two silent holes found while porting, both the failure this PR exists to kill: - The raw `in`-list variant was running against Drizzle, which has one `inArray` and no raw-filter path — 38 duplicate tests under a name that means nothing there. `IntegrationAdapter.hasRawInListPath` now declares it. - `'%s contains rejects a non-ASCII needle in the ORE term'` drove `matchDomains.filter(spec.indexes.ore)`. Since the OPE re-pin no match domain carries an `ore` index — `text_search` is `unique + ope + match` — so the `it.each` had ZERO cases and reported nothing. A vacuous test reads exactly like a passing one. Replaced with an assertion that the set is empty, so if an ORE-flavoured match domain ever returns, whoever adds it must restore the needle test with it. `integration-drizzle.yml` runs the suites on plain Postgres, fork-PR gated, `PGRST_URL` deliberately unset so a Supabase suite scoped here would throw. GitHub Actions does not support YAML anchors, so the path filters are repeated.
coderdan
commented
Jul 10, 2026
| node-version: | ||
| description: Node major to run the suites on. | ||
| required: false | ||
| default: '22' |
Contributor
Author
There was a problem hiding this comment.
This is old! Should be 24 or even 26 now.
Contributor
Author
There was a problem hiding this comment.
Bumped to 24 in c78b60f4. Not 26: nothing in the repo uses it yet (engines is >=22, tests.yml runs a 22/24 matrix, local dev is 24), and a default is the wrong place for a new major to appear first.
coderdan
commented
Jul 10, 2026
| # A one-element matrix, not a cross-product: the Drizzle adapter only runs | ||
| # against plain Postgres. Kept as a matrix so adding a variant is one line. | ||
| matrix: | ||
| db: [postgres] |
Contributor
Author
There was a problem hiding this comment.
We should add supabase to this now
Contributor
Author
There was a problem hiding this comment.
Done in c78b60f4 — the job is now a two-cell matrix over db: [postgres, supabase].
Worth doing: it found two real defects, neither visible on plain Postgres.
-
relational.integration.test.tsbuilt its table from all 39 catalog rows, including the ninedeferred_ord_oredomains. On managed Postgres the seed INSERT raisedore_domain_unavailable. It now filters onisCovered, which is what the field is for. -
Worse, it created that table with
CREATE TABLE IF NOT EXISTS. A table left by an earlier run keeps its old columns, so even after the filter the leftover table still carried its nine ORE columns and every INSERT still raised — even leaving them NULL, because the domain CHECK calls a function that RAISEs rather than returning false. Silent schema drift across runs. Now DROP-then-CREATE.
Both suites now pass identically on each database: 617 passed / 10 skipped / 0 failed.
Copilot on #616: `hasCipherStashCredentials()` returned true on the presence of `CS_WORKSPACE_CRN` alone, so a partial environment passed a gate whose entire purpose is to fail loudly. Reproduced against a live suite. With `CS_WORKSPACE_CRN` set and the three client variables missing, `globalSetup` waved the run through and every test then died with `[encryption]: Not authenticated` — four identical failures, none naming a variable, none naming a fix. Exactly the class of failure the gate exists to prevent, and the shape a rotated or cleared CI secret takes. Credentials now resolve from EITHER all four `CS_*` variables OR a `~/.cipherstash` profile. A partial environment is neither, and is refused: CipherStash credentials are PARTIALLY configured — missing CS_CLIENT_ID, CS_CLIENT_KEY, CS_CLIENT_ACCESS_KEY. A partial environment is rejected rather than half-used: it would otherwise fail later with "[encryption]: Not authenticated", which names nothing. Either set all four (...), or unset them all and run `stash auth login`. It is refused rather than half-used because it is ambiguous, not merely incomplete: with a profile on disk there is no way to tell whether the caller meant to override one field or to use the environment wholesale. Refusing to guess is the honest answer. `test-kit-env.test.ts` covers the gate, which had no test at all — the one thing standing between a rotated secret and a confusing failure. Three of its seven tests fail against the old check: the partial-environment regression, the "names exactly the missing variables" contract, and empty-string handling (a cleared GitHub secret expands to `''`, not to unset, so `!process.env[x]` is the correct predicate and `x in process.env` would not be).
A green Supabase integration run printed 213 ERROR blocks with stack traces. All 213 came from PASSING tests, and the run reported 665 passed / 0 failed. The job was honestly green and looked broken. The capability-rejection tests are the bulk of the suite: for each domain, the driver derives the operations its capabilities forbid and asserts the adapter refuses them. `boolean` is storage-only, so it gets twelve — including `order`, which is why the log shows an adapter refusing to order a bool. That is the test working. `EncryptedQueryBuilderImpl.execute` logs `logger.error(...)` before returning its `Result` error, so every passing rejection emits an ERROR block. `silent: 'passed-only'` suppresses console output from passing tests and keeps it for failing ones — which is when it is worth reading. Measured on the Supabase family suite: all passing: 10 output lines, 0 ERROR blocks 212 failures: 3510 output lines, 212 ERROR blocks (all shown) Confined to the integration vitest config. The stack logger bottoms out at `error` (`STASH_STACK_LOG` has no silent level), so suppressing at the runner is the only way to do this without changing what the product logs.
…create Two review comments from @coderdan on #616. **Run the Drizzle suites against Supabase too.** The adapter talks straight to Postgres, but it still has to work on managed Postgres, where `postgres` is not a superuser, the EQL install takes its self-skipping path, and the ORE domains cannot hold data. A suite that passes on a superuser database can fail there. The job is now a two-cell matrix over `db: [postgres, supabase]`. Wiring it up found two real defects, neither visible on plain Postgres: - `relational.integration.test.ts` built its table from all 39 catalog rows, including the nine `deferred` `_ord_ore` domains. On managed Postgres the seed INSERT raised `ore_domain_unavailable`. It now filters on `isCovered`, which is what the field is for. - Worse, it created that table with `CREATE TABLE IF NOT EXISTS`. A table left by an earlier run keeps its old columns, so after the filter the leftover table still carried its nine ORE columns and every INSERT still raised — even leaving them NULL, because the domain CHECK calls a function that RAISEs rather than returning false. Silent schema drift across runs. Now DROP-then-CREATE. **Node 24 in the integration setup action**, was 22. It is the newest major `tests.yml` exercises and what the repo develops on; `engines` is `>=22` and the unit job still covers 22 in its matrix. Not 26 — nothing in the repo uses it yet, and a default should not be the first place a new major appears. Verified: the Drizzle suites now pass identically on both databases (617 passed, 10 skipped, 0 failed on each). The Supabase job is unchanged at 665 passed.
coderdan
changed the title
A skipped test reads exactly like a passing one. Every silent hole this suite has found took that shape, and the skips it was itself carrying hid one more. **The bug the skips were hiding.** `dbVariant()` inferred the database from the presence of `PGRST_URL`. The new Drizzle `db=supabase` cell needs no PostgREST and left it unset, so the variant reported `postgres`: EQL installed WITHOUT `--supabase`, the role grants were never applied, and the grants test quietly did not run. The suite passed because Drizzle connects as `postgres`, never as `anon`. The variant is now explicit (`CS_IT_DB_VARIANT`) in both workflows, and the inference survives only as a documented fallback for a hand-run suite. **The skips are gone**, all ten: - The eight per-family "defers <domain>" notices were `it.skip`. They asserted nothing. `domainsForFamily` already excludes deferred domains, and `test-kit-families.test.ts` asserts — as a PASSING test — that the excluded set is exactly the nine block-ORE domains, each with a reason. - The two harness gates were `it.runIf`. They are now single tests that assert both branches: on Supabase the three roles exist and hold the `eql_v3` and `eql_v3_internal` grants; on plain Postgres those roles do not exist at all, which is a fact worth stating and makes the variant claim falsifiable. PostgREST is asserted on its own axis (`PGRST_URL` set) rather than on the variant — conflating "is this Supabase" with "is PostgREST up" is what made `dbVariant()` lie. **And a guard so they cannot come back.** `no-skips-reporter.ts` fails the integration run if any test is skipped, naming each one. Verified: adding a single `it.skip` exits 1. Scoped to the integration config. The unit suites still carry the `LIVE_*` gates and their 16 skips; working those out is a separate change. Verified on all three CI cells, each 0 skipped, 0 failed: drizzle / postgres 619 passed drizzle / supabase 619 passed supabase / supabase 665 passed
Both are consequences of PR1 and both are about the same thing: a skipped test reads exactly like a passing one. The integration suites now carry zero skips and a reporter that fails the run if any appear; the unit suites still carry 16, behind the LIVE_* gates that live-coverage-guard.test.ts exists to police.
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
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.
Full plan, covering all four PRs:
docs/superpowers/plans/2026-07-10-eql-v3-integration-tests.md.Why
Reviewing #535 turned up four structural gaps. This PR addresses the first; the plan sequences the rest (type robustness → adapter packaging → v3 JSON).
Query correctness was largely unproven. The Supabase v3 adapter had zero real-crypto coverage. Its only "live" suite ran against a real PostgREST but stubbed ZeroKMS, and its equality term was literally
`hm-${plaintext}`(__tests__/helpers/v3-envelope.ts:22). So.in('nickname', ['ada','nobody']) → ['ada']was true by construction — it comparedhm-adaagainsthm-ada.drizzle-v3/operators-live-pg.test.tswas a genuine end-to-end suite, but it was credential-gated into silent skips and never exercised the single-encryptModelinsert path.Tests first, so the three refactors that follow land against a suite that actually proves query correctness.
What's here
packages/test-kit— private, unpublished, no build step. The v3 domain catalog moves here so both adapter suites share one source of truth and cannot disagree about which domains exist. Thesatisfies Record<EqlV3TypeName, DomainSpec>coverage mechanism survives: deleting a domain row still fails the build, naming the domain.The harness — two docker variants (plain Postgres;
supabase/postgres+ PostgREST), EQL v3 installed by the realstash eql install, and a credential gate that throws instead of skipping.One driver, both adapters. Everything a test asserts is derived from the domain's catalog row: which operations must work, which must be rejected, what rows each returns — computed from a plaintext oracle. Drizzle 617 tests, Supabase 665. Where they legitimately differ they differ in
supportedOps, not in what a shared test asserts.Two bug fixes and one new capability, all found by building the suite.
Behaviour changes (
@cipherstash/stack, bothminor)Raw
.filter(col, 'in', […])encrypted the whole list as one ciphertext. The raw-filter path reachedinwith none of the element-splitting thein(),not(…)and.or()paths perform. The two wire formats then failed differently, which is why nobody noticed: v2's("json")composite literal is already parenthesized, so PostgREST parsed it as a one-element list and answered200 []— a filter that silently matched nothing. v3's bare{…}envelope is not, so PostgREST rejected it withPGRST100. The v2 form has shipped.order()now works on encrypted v3 ordering columns. It was rejected outright on every encrypted column, on the grounds that PostgREST cannot emitORDER BY eql_v3.ord_term(col). True — but it can emit a jsonb path, and the path selects exactly that term:The guard is now on the ordering flavour, not on encryption:
opesupported,orerefused, no-ordering-term refused.V3OrderableKeyswidens to match;is(col, true)had been borrowing that key set and now has its ownV3PlaintextKeys.Also: aliased date columns lost their
Datereconstruction (selectKeyToDbwas dropped frombuildSelectString), so.select('ts:createdAt')returned an ISO string where the typed surface promises aDate.Things measured, not assumed
Several claims in my own plan turned out to be wrong. Each was corrected against a live database:
_ord_orecolumns cannot hold data on managed Postgres — and that is correct. The domains install, but their CHECK callsore_domain_unavailable()and the first INSERT raises, with a hint naming the alternatives. Even leaving the column NULL raises. There is no silent wrongness; the bundle guards properly. I filed this as a bug upstream and then retracted and closed it when I checked the write path.SUPABASE_ADMIN_URL. Neither is needed.col->opdoes not avoid the database collation. Postgres compares jsonb strings withvarstr_cmpunder the default collation, exactly as it does text. What makes the ordering correct is the term's encoding — lowercase hex, fixed-width for numeric and date domains, per-character for text — soope-term.integration.test.tspins that shape rather than trusting the operator.bloomTokens()implementedinclude_originalitself, so a test assertinginclude_originalbehaviour was checking the stub, not the product.Three fixture bugs surfaced only by running it:
authenticatoris a reserved role that evenpostgrescannot modify; the initdb script must sort aftermigrate.sh; andpg_isreadyis not a readiness check for either image, because both boot a temporary server on the unix socket only.Silent holes found — and closed
The suite exists to kill tests that look like coverage and aren't. It kept finding them, including in itself:
containsnever ran ontext_match. The needle was derived from each domain's minimum sample, which for text is''— shorter thantoken_length, soit.runIfskipped it.text_matchis the only match-only domain and free-text search is its only capability.include_originalbug (fix(stack): retract the include_original substring-search fiction (CIP-3483) #615): its whole-value token is a trigram, so it matches either way. The text family now searches four needles per domain, andmatch-bloom.integration.test.tsassertsbloom(needle) ⊆ bloom(haystack)directly — and proves its own sensitivity by showing one foreign token breaks the subset.'%s contains rejects a non-ASCII needle in the ORE term'had zero cases. Since the OPE re-pin no match domain carries anoreindex, so theit.eachran nothing and reported nothing. Replaced with an assertion that the set is empty.in-list variant ran against Drizzle, which has oneinArray— 38 duplicate tests under a meaningless name. Now a declared capability.relational.integration.test.tsusedCREATE TABLE IF NOT EXISTS, so a table from an earlier run kept its old columns and silently drifted from the catalog.Review feedback addressed
hasCipherStashCredentials()passed onCS_WORKSPACE_CRNalone. Reproduced: a partial environment sailed through the gate and every test then died with[encryption]: Not authenticated. Credentials now resolve from all fourCS_*vars or a~/.cipherstashprofile; a partial environment is refused because it is ambiguous, not merely incomplete.test-kit-env.test.tscovers the gate, which had no test; three of its seven cases fail against the old check.relationaldefects above. Node default in the setup action bumped 22 → 24.execute()logs before returning itsResulterror.silent: 'passed-only'now suppresses logs from passing tests and keeps them for failing ones. Green run: 10 lines. A run with 212 failures: 3510 lines, all 212 ERROR blocks shown.Verification
src/supabase/to the base commit fails 41 tests: 38 ×filter(in)on every eq-capable domain, 3 × aliasedDate. With the fixes: 0.equalityfrominteger_eq's catalog row flips its four positive tests into rejection tests, which then fail because the column really does support equality.orderColumnNameto a bare column name turns them red.supabase/postgres. Supabase: 665 passed.pnpm testruns zero integration tests; both typechecks clean.~/.cipherstashprofile stands in for theCS_*vars.