feat(stack-supabase)!: remove EQL v2 authoring surface, de-suffix v3 API#769
Conversation
Re-parameterize EncryptedQueryBuilderImpl over AnyV3Table and inline every EncryptedQueryBuilderV3Impl dialect override, then delete query-builder-v3.ts. Pure refactor: no runtime behaviour or wire encoding change. The decrypt path stays generation-agnostic (decryptModel/bulkDecryptModels), so stored EQL v2 payloads still decrypt (Decision 6).
encryptedSupabase is now the introspecting EQL v3 factory (was encryptedSupabaseV3); encryptedSupabaseV3 kept as a @deprecated type-identical alias. The legacy v2 encryptedSupabase({ encryptionClient, supabaseClient }).from(table, schema) wrapper and EncryptedSupabaseConfig are removed. All *V3 type exports de-suffixed to canonical names with @deprecated *V3 aliases retained. v2 decrypt is unaffected.
Point tests at the folded EncryptedQueryBuilderImpl, drop the removed v2 authoring tests (v2 live suite, v2 wire-encoding block, v2 builder type test), convert the shared execute() error-threading tests to a v3 table, and add canonical-name type assertions.
🦋 Changeset detectedLatest commit: 3d69a59 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 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 |
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
v2 read via this adapter is intentionally removed; v2 ciphertext still decrypts through the core @cipherstash/stack client. Mixed-generation handling is customer-side (install both), per #707 out-of-scope stance.
…ap 91 -> 71 The EQL v2 removal folded `query-builder-v3.ts` into `query-builder.ts` (2b4e2e9). FTA penalises size superlinearly, so two files that each passed the complexity gate became one that did not: 90.88 + 73.95 -> 105.77, against a cap of 91. That failed the "Analyze v3 complexity" check on #769. `fta-v3.yml` is explicit that the cap is "a ratchet, not an aspiration", so decompose rather than raise it. The 2331-line class becomes a pipeline: column-map.ts ColumnMap - name + capability resolution, the concern every stage needed (was six correlated fields) query-encrypt.ts filter-operand terms: collect, validate, batch-encrypt query-mutation.ts row-data encryption for insert/update/upsert query-dbspace.ts property-space -> DB-space, once query-filters.ts operand substitution onto the PostgREST query query-results.ts decryption + Date reconstruction query-builder.ts the class: recorded state, fluent surface, execute() Worst file 105.77 -> 70.12 (query-encrypt.ts); cap lowered 91 -> 71. Also removed, all no-ops from the pre-fold v2/v3 inheritance: - `notFilterOperator` - an identity function with an unused parameter - `applyPatternFilter`'s dead `_wasEncrypted` parameter - `protected` throughout, now `private`; nothing extends the class and collapsed the six-times-repeated withLockContext/audit/await dance into one `withOpContext` helper - a skipped lock context would encrypt under the wrong data key, so the three steps must stay identical. `EncryptedQueryBuilderImpl` keeps its name, export site and constructor; nothing here is part of the package's public surface (index.ts does not re-export it). 466 tests pass unchanged. One test reached the unsupported- queryType backstop by subclassing to override `queryTypeForRawOp`, which is now a module function; `assertTermQueryable` is exported instead and the test calls it directly, dropping a subclass its own comment called "breaking the internal contract". Docs: correct comments asserting adapter-side EQL v2 reads. The adapter is v3 only and does not auto-read `eql_v2_encrypted` columns - introspection matches `public.eql_v3_*` domains exclusively, so such a column never enters the encrypt config. No ciphertext is stranded: core decryption stays generation-agnostic. Also retires the two-dialect scaffolding in types.ts, whose header claimed v3 does free-text via `contains` - contradicting the typed surface forty lines above it and re-introducing the exact confusion #617 removed. `EncryptedQueryBuilderCore`'s OK/BK defaults are kept: they are live for the untyped surface, only their v2-era rationale was wrong.
…array CodeRabbit review of the query-builder split. Six findings, all pre-dating the refactor; these are the two worth acting on. `single()`/`maybeSingle()` have always returned ONE object at runtime, but returned `Self`, so the builder kept advertising the array shape it was created with — `data` was typed `T[] | null` while holding a single row. Callers had to launder it, and the test suite documented the lie in a comment while casting through `unknown` to reach the row's fields. Both now return `EncryptedSingleQueryBuilder<T>`, awaiting `EncryptedSupabaseResponse<T>` (`data: T | null`) — which already covers the zero-row case for `maybeSingle()` and the error case for both, so no separate null modelling was needed. The impl class carries the awaited shape as a `TData` parameter so the promise cannot keep advertising `T[]` after the runtime has been switched to single-row mode; `returns<U>()` preserves that shape. Filters and transforms are deliberately absent from the single-row builder, matching supabase-js: applying one after `single()` would change the query the single-row promise was made about. Also drops two unnecessary `as unknown as T[]` bridges in query-results.ts (`[] as T[]` and the bulk-decrypt map both compile directly). Not acted on, with reasons: - The missing `assertPostgrestCanQueryEncryptedOperator` in the not-filter branch is a false positive. The guard fires upstream in `assertTermQueryable` (`contains`/`matches` map to `freeTextSearch`), before encryption, and `supabase-v3-json.test.ts` already covers `.not(col,'contains')` and `.not(col,'matches')` under EQL 3.0.2. The suggested guard keys on `wasEncrypted`, which that path never reaches. - Routing plaintext `in` arrays through `formatInListOperand` would change behaviour: `.filter()` is the raw escape hatch and forwards verbatim, as supabase-js does. The encrypted path only intervenes because it must encrypt element-wise. - The `as never` on `bulkEncrypt` args and the `term.column` double assertion need `ScalarQueryTerm['column']` widened in `@cipherstash/stack` — a different package's public type.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/stack-supabase/__tests__/supabase-v3-builder.test.ts (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftRetain coverage through the builder execution path.
This replacement calls the internal
assertTermQueryabledirectly with a forged term, bypassingEncryptedQueryBuilderImplexecution and its error propagation. It can pass even if the builder stops invoking the guard or mishandles the resulting error. Keep this focused unit assertion if useful, but retain/add a regression that exercises the builder path.Also applies to: 1479-1505
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stack-supabase/__tests__/supabase-v3-builder.test.ts` at line 5, Retain the direct assertTermQueryable assertion if useful, but add a regression test that invokes the forged term through EncryptedQueryBuilderImpl’s normal execution path and verifies the guard error is propagated. Ensure the test would fail if the builder stops calling the guard or mishandles its error.Source: Coding guidelines
packages/stack-supabase/src/query-encrypt.ts (1)
598-605: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace type-erasing assertions with narrowed types or a reasoned Biome ignore. These casts erase all type information at the FFI/column boundary; the guideline requires narrowing or a specific assertion, suppressing deliberate cases with a reasoned Biome ignore. Neither site has one.
packages/stack-supabase/src/query-encrypt.ts#L598-L605:as neveron bothbulkEncrypt(...)arguments fully erases the operand/context types — introduce a precise parameter type forbulkEncrypt(or a single, documentedbiome-ignorewith rationale) instead.packages/stack-supabase/src/query-encrypt.ts#L512-L512:term.column as unknown as V3ColumnLike— narrowScalarQueryTerm['column']toV3ColumnLikeat the source, or add a reasonedbiome-ignore.As per coding guidelines: "Avoid type-erasing assertions such as
as any,as never, andas unknownin source; narrow types or use a specific assertion, suppressing deliberate cases with a reasoned Biome ignore."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stack-supabase/src/query-encrypt.ts` around lines 598 - 605, Replace the type-erasing assertions in packages/stack-supabase/src/query-encrypt.ts#L598-L605 by giving bulkEncrypt precise parameter types and passing narrowed values, or add one documented Biome ignore with a specific rationale; also narrow ScalarQueryTerm['column'] before the term.column use at packages/stack-supabase/src/query-encrypt.ts#L512 or apply a reasoned Biome ignore. Preserve the existing encryption behavior and avoid as never, as unknown, or equivalent erasing casts.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/stack-supabase/__tests__/supabase-v3-builder.test.ts`:
- Line 5: Retain the direct assertTermQueryable assertion if useful, but add a
regression test that invokes the forged term through EncryptedQueryBuilderImpl’s
normal execution path and verifies the guard error is propagated. Ensure the
test would fail if the builder stops calling the guard or mishandles its error.
In `@packages/stack-supabase/src/query-encrypt.ts`:
- Around line 598-605: Replace the type-erasing assertions in
packages/stack-supabase/src/query-encrypt.ts#L598-L605 by giving bulkEncrypt
precise parameter types and passing narrowed values, or add one documented Biome
ignore with a specific rationale; also narrow ScalarQueryTerm['column'] before
the term.column use at packages/stack-supabase/src/query-encrypt.ts#L512 or
apply a reasoned Biome ignore. Preserve the existing encryption behavior and
avoid as never, as unknown, or equivalent erasing casts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a8ea556-a9a6-4e90-a0ad-96b54fc85245
📒 Files selected for processing (29)
.changeset/remove-eql-v2-supabase-authoring.md.changeset/remove-eql-v2-supabase-skill.md.changeset/supabase-single-row-typing.md.github/workflows/fta-v3.ymlpackages/stack-supabase/README.mdpackages/stack-supabase/__tests__/supabase-encryption-error.test.tspackages/stack-supabase/__tests__/supabase-v3-builder.test.tspackages/stack-supabase/__tests__/supabase-v3-factory.test.tspackages/stack-supabase/__tests__/supabase-v3-json.test.tspackages/stack-supabase/__tests__/supabase-v3-matrix.test.tspackages/stack-supabase/__tests__/supabase-v3-select-star.test.tspackages/stack-supabase/__tests__/supabase-v3-wire.test.tspackages/stack-supabase/__tests__/supabase-v3.test-d.tspackages/stack-supabase/__tests__/supabase.test.tspackages/stack-supabase/integration/wire.integration.test.tspackages/stack-supabase/package.jsonpackages/stack-supabase/src/column-map.tspackages/stack-supabase/src/helpers.tspackages/stack-supabase/src/index.tspackages/stack-supabase/src/introspect.tspackages/stack-supabase/src/query-builder-v3.tspackages/stack-supabase/src/query-builder.tspackages/stack-supabase/src/query-dbspace.tspackages/stack-supabase/src/query-encrypt.tspackages/stack-supabase/src/query-filters.tspackages/stack-supabase/src/query-mutation.tspackages/stack-supabase/src/query-results.tspackages/stack-supabase/src/types.tsskills/stash-supabase/SKILL.md
💤 Files with no reviewable changes (2)
- packages/stack-supabase/tests/supabase.test.ts
- packages/stack-supabase/src/query-builder-v3.ts
freshtonic
left a comment
freshtonic
left a comment
There was a problem hiding this comment.
Reviewed by Claude Code on behalf of James Sadler.
Verdict: Approve (non-blocking suggestions below).
This is a large but disciplined breaking-change refactor. I read the full diff and cross-checked the PR-head tree (the single() typing change, the query-builder fold, the de-suffixing, and the module split). No blocking correctness/security/API defects.
What I verified
De-suffixing is complete and non-breaking.
encryptedSupabaseV3→encryptedSupabasewith a runtime@deprecatedalias (export const encryptedSupabaseV3 = encryptedSupabase).- Every renamed type (
EncryptedSupabaseV3Options,*V3Instance,EncryptedQueryBuilderV3[/Untyped],V3FilterableKeys,V3OrderableKeys,V3PlaintextKeys,NonQueryableV3Keys,NonOrderableV3Keys,V3SearchableJsonKeys,V3EncryptedFreeTextKeys,V3FreeTextSearchableKeys) keeps a type-identical@deprecatedalias, and both the canonical and alias names are re-exported fromindex.ts.EncryptedSupabaseConfig(v2-only) is fully removed on the branch — confirmed no residual references insrc/. - No
srcfile on the branch imports the deletedquery-builder-v3.ts; all test/integration imports were repointed toquery-builder.
No security regression in the v3 fold. The 989-line EncryptedQueryBuilderV3Impl was folded into EncryptedQueryBuilderImpl (now natively v3) and split across column-map/query-encrypt/query-dbspace/query-filters/query-mutation/query-results. The fail-safe guards all survive 1:1:
validateTransforms()is still invoked (buildAndExecuteQuery, line 665) — the only protection against silently sorting a ciphertext envelope on the untyped surface.- The free-text needle floor (
matchNeedleError), the null/length-mismatch envelope contract checks inbulkEncryptGroup, the empty-needle JSON containment rejection, and the EQL 3.0.2assertPostgrestCanQueryEncryptedOperatorfail-before-encrypt gate are all preserved. withOpContextcentralizes lock-context/audit application, so no crossing can silently drop the lock context (which would encrypt under the wrong key).
v2 removal is intentional and correctly scoped. Only the v2 authoring/emission surface is removed; no ciphertext is stranded because core @cipherstash/stack decrypt is generation-agnostic. Changesets (stack-supabase: major x2, stash: patch), README, and the stash-supabase skill are all updated consistently.
Non-blocking suggestions
-
Changeset overclaims
.single().returns<U>()..changeset/supabase-single-row-typing.mdsaysreturns<U>()preserves the awaited shape "so.single().returns<U>()still awaits one row." But the public return typeEncryptedSingleQueryBuilder<T>(types.ts) exposes onlyabortSignal/throwOnError— noreturns. Through the public typed surface.single().returns<U>()is a compile error (only the impl class, which no consumer sees, carries theTData-preservingreturns). Either addreturnstoEncryptedSingleQueryBuilder, or drop that sentence from the changeset. -
EncryptedSingleQueryBuilderis not re-exported fromindex.ts. It's the return type of the publicsingle()/maybeSingle()(viaEncryptedQueryBuilderCore), so consumers can use it structurally but cannot name it. Minor DX gap — worth adding to theexport type { ... }block for parity with the other public surface types. -
fta-v3.ymlsupabase cap = 71 with ~0.9 margin. The PR's own comment flags this as fragile (top three files cluster at 70.12/70.05/69.13). Acceptable as a ratchet; just noting the next reflow may trip it — the comment already tells the next person to re-measure before assuming a real regression.
Nice work on the guard-preservation discipline and the exhaustive @deprecated alias coverage.
2 participants
Part of the EQL v2 removal effort (#707). PR 4 of the linear stack — stacked on #768 (PR 3, stack core).
What this does
Makes
@cipherstash/stack-supabaseEQL v3 only for authoring, per the design (docs/plans/2026-07-22-eql-v2-final-removal-design.md§"PR 4").src/query-builder-v3.ts(989 lines); re-parameterizedEncryptedQueryBuilderImploverAnyV3Tablewith the v3 dialect overrides inlined and the generation-agnostic machinery preserved verbatim. No wire-encoding or Result-shape change.encryptedSupabaseV3→encryptedSupabase(+@deprecatedalias); ~13*V3public types de-suffixed, each keeping a@deprecatedalias (Decision 5). Removed the v2encryptedSupabase(...).from(table, schema)wrapper and v2 config/instance types.stash-supabase), README, changesets (stack-supabase: major,stash: patch) updated.Green gate
build ✓ · test:types 35 pass · test 466 pass (0 skipped) ·
code:check0 errors.Decision 6 (v2 decrypt) — RESOLVED
This adapter is now EQL v3 only for both read and write: it will not auto-read an
eql_v2_encryptedcolumn. That is intentional, not a gap:@cipherstash/stackclient (decrypt/decryptModel), which is generation-agnostic.@cipherstash/stackdirectly, or create aneql_v3_*twin and run the v3 rollout (see thestash-supabaseskill), or pin the last release that shipped the v2 wrapper.(The wizard PR #771 keeps both-domain awareness, but that is a detection clobber-guard, not a decrypt path — a different operation, not a conflicting choice.)
Notes
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
encryptedSupabasenow provides an asynchronous, connect-time introspecting EQL v3 integration.Breaking Changes
from()schema form.Bug Fixes
.single()and.maybeSingle()result types to represent a single row.Documentation
*V3names remain deprecated compatibility aliases.