refactor(stack): EQL v3 types namespace on @cipherstash/stack/eql/v3#541
refactor(stack): EQL v3 types namespace on @cipherstash/stack/eql/v3#541tobyhede wants to merge 10 commits into
types namespace on @cipherstash/stack/eql/v3#541Conversation
|
| Name | Type |
|---|---|
| @cipherstash/stack | Minor |
| @cipherstash/bench | Patch |
| @cipherstash/prisma-next | Patch |
| @cipherstash/basic-example | Patch |
| @cipherstash/prisma-next-example | Patch |
| @cipherstash/e2e | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 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 |
Replace the 35 verbose `encrypted<Domain>Column` factories with a single `types` namespace whose members mirror the underlying `eql_v3.<name>` domains 1:1 (`types.TextEq`, `types.Int4Ord`, `types.Timestamptz`, …), and split the 992-line `src/schema/v3/index.ts` into a cohesive module under `src/eql/v3/` (`columns.ts`, `types.ts`, `table.ts`, curated `index.ts`). The authoring subpath is renamed `@cipherstash/stack/schema/v3` -> `@cipherstash/stack/eql/v3`; the `./v3` typed-client surface now re-exports the `types` namespace instead of the standalone factories. Behaviour is preserved: same classes, same nominal-typing mechanism, same `build()` output. - Rewire tsup entry, package.json exports/typesVersions/analyze:complexity, the `@/eql/v3` re-export, `[eql/v3]` error prefix, and fta-v3.yml paths. - Migrate all v3 tests + CJS smoke test to `types.*` and the new subpath. - Reconcile the three unreleased changesets and refresh tracked v3 design docs (supersede banners on completed-work records; correct the not-yet-built Stryker gate spec's single-file premise for the 4-file split). Verified: build emits dist/eql/v3; schema/v3 subpath and factories are gone (ERR_PACKAGE_PATH_NOT_EXPORTED, undefined on both subpaths); 101 v3 runtime + 50 type tests pass; FTA scores all 4 files (max 68.68 < 72); e2e authoring config byte-matches expected.
Two JS properties whose builders resolve to the same DB name (getName()) silently overwrote in the built config — the later column won and the first's config was dropped. Throw instead, matching the existing duplicate-tableName guard in buildEncryptConfig and the reserved-key guard in encryptedTable. Regression tests: `EncryptedTable.build()` and `buildEncryptConfig` both throw on a duplicate DB name (schema-v3.test.ts, eql_v3 encryptedTable block).
The structural builder contracts (BuildableColumn, BuildableQueryColumn, BuildableV3QueryableColumn, BuildableTable, BuildableTableColumns) and the encryptModel/bulkEncryptModels return-type mapper (EncryptedFromBuildableTable) appear in public return positions but were not re-exported from `@cipherstash/stack/types`, so consumers could not name them — an inconsistency with the already-exposed `EncryptedFromSchema`. No build breakage (the mapped types were emitted inline); this closes the nameability gap. Regression guard: types-public-surface.test-d.ts imports each contract from the public `@/types-public` entrypoint (a missing re-export fails typecheck). Note: these types are inherited from the base branch (feat/eql-v3-text-search-schema, PR #535); the export is added here in response to review feedback on the stacked PR.
The v3-matrix domain suite (catalog.ts + matrix tests) landed on the base branch via PR #540 after this branch was cut, and used the pre-refactor `@/schema/v3` path and `encrypted<Domain>Column` factories. Retarget it to `@/eql/v3` and the `types.*` namespace so the base's matrix coverage keeps working on top of the refactor. `EqlTypeForColumn` (which #540's catalog.ts consumes) is preserved — ported into eql/v3/columns.ts and re-exported from the barrel during the rebase. Post-rebase reconciliation only; no behavior change.
b98ad89 to
2a078b8
Compare
Close two coverage gaps on the eql/v3 branch that only live/e2e tests
touched:
- encrypt-lock-context-guards: assert NaN/+Inf/-Inf are rejected on the
`encrypt(...).withLockContext(...)` path and short-circuit before the
FFI call. The non-lock guards run only under the live number-protect
suite; the lock-context arm (encrypt.ts:163-168) had no coverage.
- wasm-inline-new-client: assert the protect-ffi 0.25 single-object
`newClient({ strategy, encryptConfig, clientId, clientKey })` shape,
incl. cast_as normalisation. Previously exercised only by the
secret-gated Deno e2e, so a regression to the 0.24 two-arg form would
pass normal CI.
Both run offline (mocked FFI).
The all-35-domain live Postgres suite was force-skipped (describe.skip, not credential-gated) after `beforeAll`'s dynamic INSERT crashed with `invalid input syntax for type json` (PR #540). That crash was a postgres.js serialization gap — a bare ciphertext object stringified to "[object Object]" — and was fixed 32 minutes later by wrapping every INSERT param in `sql.json(...)` (commit 53cf854). The force-skip was simply left stale; it is not an FFI limitation. Restore the credential-gated form (`LIVE_EQL_V3_PG_ENABLED ? describe : describe.skip`) as the file's own comment instructed, so the 35-domain SQL round-trip runs in CI (which supplies DATABASE_URL + CS_* creds) and self-skips locally. The genuine FFI-level skip — timestamptz `cast_as:'date'` time-of-day truncation in schema-v3-client.test.ts — stays skipped (needs a native 'timestamp' cast_as variant). timestamptz matrix cases are unaffected (midnight samples, no truncation).
freshtonic
left a comment
There was a problem hiding this comment.
I approve but I notice that timestamptz is back - I thought we finally reached a conclusion (that Claude's advice was incorrect)?
…equirement)
The `eql_v3.text_ord` and `eql_v3.text_ord_ore` Postgres domains require BOTH
`hm` (HMAC) and `ob` (ORE) in the stored ciphertext — text equality is
HMAC-based (their `eql_v3.eq_term` extracts `hm`), unlike numeric/date order
domains which answer equality via `ob` and need only ORE. The SDK's
`indexesForCapabilities` treated every order/range domain identically, emitting
`ore` only, so text-order ciphertexts lacked `hm` and a real INSERT failed with
`value for domain eql_v3.text_ord_ore violates check constraint`. (Surfaced by
re-enabling matrix-live-pg; masked before by the suite skip.)
Make index derivation castAs-aware: emit `unique` (hm) when equality is
answered via HMAC — equality-only domains of any type, AND text order domains
(`string` + order/range). Numeric/date order domains are unchanged (`ore` only).
Query path follows automatically: `resolvesEqualityViaOre` only fires when
`unique` is absent, so text-order equality now resolves to the `hm` index
(eq_term) while numeric/date order equality still resolves to `ore`.
TDD: text_ord/text_ord_ore build() now emits { unique, ore }; numeric order
stays { ore }; text-order equality resolves to unique. Catalog + matrix build()
assertions updated (TEXT_ORD_IDX). Verified against the eql_v3 domain checks in
the fixture; live SQL runs in CI.
| BOOL, | ||
| DATE, | ||
| DATE_EQ, | ||
| DATE_ORD, | ||
| DATE_ORD_ORE, | ||
| EncryptedBoolColumn, | ||
| EncryptedDateColumn, | ||
| EncryptedDateEqColumn, |
There was a problem hiding this comment.
What's the difference between BOOL and EncryptedBoolColumn?
| import * as ffi from '@cipherstash/protect-ffi' | ||
|
|
||
| const users = encryptedTable('users', { | ||
| score: encryptedColumn('score').dataType('number').equality().orderAndRange(), |
There was a problem hiding this comment.
AFAIU, the domain types will basically get mapped into the same thing this fluent-builder does. But the test might be less brittle if we used a domain type here directly?
|
|
||
| beforeEach(async () => { | ||
| vi.clearAllMocks() | ||
| process.env.CS_WORKSPACE_CRN = 'crn:ap-southeast-2.aws:test-workspace' |
There was a problem hiding this comment.
Oh - because its not a real workspace ID. Carry on!
| @@ -0,0 +1,91 @@ | |||
| /** | |||
| * Offline guard tests for the lock-context encrypt path. | |||
| .TextSearch('email') | ||
| .freeTextSearch({ |
There was a problem hiding this comment.
This is pretty great tbh! We should make sure its documented in the Typedoc.
There was a problem hiding this comment.
The merge semantics are documented in the JSDoc on freeTextSearch() (columns.ts:458-462): each provided key replaces its default, omitted keys keep the default, mirroring v2's opts?.x ?? default. There's currently no Typedoc pipeline in the repo — happy to set one up as a follow-up if that's what you're after.
|
From Verdict: coverage is strong; one small lopsided gap worth closing. The bulk of this PR is a rename/refactor ( Inline gap (1)
Additional gaps (body only)
No security issues noticed incidentally. |
… guards
Test-only additions (separated from the in-flight EQL v3 bundle upgrade so they
land on this branch, not the bundle branch):
- encrypt-lock-context-guards.test.ts: run every non-finite-number guard case
against BOTH a v2 fluent-builder column and a v3 domain column, since the
guard lives on the shared EncryptOperationWithLockContext.
- schema-v3.test.ts: `.freeTextSearch()` no-arg is a no-op (pins the
opts===undefined branch); a text_match mutable-state aliasing guard (base-class
match-clone path, which the text_search-only test can't cover); and
buildEncryptConfig() with zero tables yields { v: 1, tables: {} }.
- wasm-inline-strategy.test.ts: Biome line-wrap formatting only.
Refactors the EQL v3 authoring surface: the per-domain
encrypted<Domain>Columnfactories become a singletypesnamespace whose members mirror the underlyingeql_v3.<name>domains, on the renamed@cipherstash/stack/eql/v3subpath.Before → after — the name maps 1:1 to the EQL v3 domain:
Per-domain plaintext inference and compile-time queryability are unchanged:
The
@cipherstash/stack/v3typed client re-exportstypes(in place of the standalone builders):typesmembersOne member per generated EQL v3 domain (PascalCase of the
eql_v3.<name>):Int4Int4EqInt4OrdOreInt4Ord·Int2*·Date*·Timestamptz*·Numeric*·TextTextEqTextMatchTextOrdOreTextOrdTextSearch·Bool·Float4*Float8*(int8/bigint still omitted pending lossless FFI I/O). Each returns its concrete branded class, so per-column inference stays precise.Scope
schema/v3/index.tsintosrc/eql/v3/{columns,types,table,index}.ts.schema/v3→eql/v3(exports, tsup entry, FTA gate,./v3re-export); the old subpath and the standalone factories are removed.build()output (text_searchstays byte-identical).Verified:
schema/v3no longer resolves (ERR_PACKAGE_PATH_NOT_EXPORTED) and factories areundefinedon both subpaths; 101 v3 runtime + 50 type tests pass; FTA scores all 4 files (max 68.68 < 72). Stacked on #535.