docs(drizzle): refresh the stash-drizzle skill; correct the eq/ne index contract#604
docs(drizzle): refresh the stash-drizzle skill; correct the eq/ne index contract#604coderdan wants to merge 1 commit into
Conversation
…ex contract
`skills/stash-drizzle/SKILL.md` ships inside the `stash` tarball and `stash init`
installs it for every Drizzle project. It was last substantively updated
2026-05-14; the Drizzle adapter moved 2026-07-08.
Four defects, two of which produce silently wrong behaviour.
**`eq`/`ne` require `equality`.** Both the skill and the operator's own TSDoc said
"either `equality` or `orderAndRange`". The runtime disagrees:
`createComparisonOperator` gates `orderAndRange` on `['gt','gte','lt','lte']` and
takes the encrypting path only when `config.equality` is set. On an
`orderAndRange`-only encrypted column, `eq` falls through to the plain Drizzle
operator, which binds the raw plaintext through the column's `toDriver` and
compares it against ciphertext. Verified:
eq on equality:true -> lazy operator (encrypts)
eq on orderAndRange -> plain SQL, bound param "x" (plaintext)
No throw, no match. Contrast the `jsonb*` operators, which throw
`EncryptionOperatorError` when `searchableJson` is absent. Corrected in the skill
and in the TSDoc; documented as a hazard. The only Drizzle test file covers
jsonb, which is why nothing caught it.
**`npx generate-eql-migration` cannot work as documented.** The skill says to
install `@cipherstash/stack` and then run that bin, but it belongs to the separate
`@cipherstash/drizzle` package, which stack does not depend on. npm then resolves
a registry package by that name and returns 404. Replaced with
`stash eql install --drizzle`, which runs `drizzle-kit generate --custom` with the
bundled EQL SQL (the older bin pins its own EQL version and downloads from
GitHub).
**An additive `db push` is promoted by `stash db activate`.** The skill told Proxy
users cutover would promote the rollout's pending config. It does not -- cutover
promotes only the rename pending -- and the cutover-time `db push` calls
`discardPendingConfig()` before writing its own, so the un-activated rollout
pending is discarded. Proxy keeps serving an active config that knows nothing
about `<col>_encrypted` for the whole dual-write window. `db activate` appeared
nowhere in the skill.
**Encrypted columns must be nullable.** The storage DDL declared an encrypted
`jsonb NOT NULL` column, which invariant 1 of the doctrine shipped alongside it
forbids. Also corrected the "EQL extension" wording: EQL is a schema plus a
composite type, installed by `stash eql install`.
Also: documents the Drizzle rename migration `encrypt cutover` scaffolds to keep
drizzle-kit's snapshot in sync; adds the missing `timestamp` and `text` dataType
values (CastAs has 8 members, the skill listed 6); repoints closed #447 at open
#585; states that Drizzle is EQL v2 only and that `@cipherstash/stack/drizzle` is
self-contained, not a re-export of `@cipherstash/drizzle`.
🦋 Changeset detectedLatest commit: d1ec144 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 |
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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 |
1 participant
Why
skills/stash-drizzle/SKILL.mdships inside thestashtarball andstash initinstalls it into every Drizzle project's.claude/skills/(and inlines it into theirAGENTS.md). It was last substantively updated 2026-05-14; the Drizzle adapter moved 2026-07-08.Four defects. Two produce silently wrong behaviour, one is a broken first step, one violates the doctrine shipped beside it.
1.
eq/nerequireequality— not "equalityororderAndRange"The skill said either index worked. So did the operator's own TSDoc. The runtime disagrees:
createComparisonOperatorgates theorderAndRangebranch on['gt','gte','lt','lte']and takes the encrypting path only whenconfig.equalityis set (packages/stack/src/drizzle/operators.ts:666,717). Otherwise it falls through toreturn operator === 'eq' ? eq(left, right) : ne(left, right)— the plain Drizzle operator, with your plaintext value.Verified against the built adapter:
Drizzle then pushes that plaintext through the column's
toDriver, producing a composite literal compared against ciphertext. It does not throw. The query simply cannot match.Contrast the
jsonb*operators, which do throwEncryptionOperatorErrorwhensearchableJsonis missing. That asymmetry is now documented.Fixed in the skill and in
operators.ts's TSDoc (which carried the same claim, twice). No behaviour change here — see "Follow-up" below.Nothing caught this because the only Drizzle test file is
drizzle-operators-jsonb.test.ts.2.
npx generate-eql-migrationcannot work as documentedThe skill says:
That bin belongs to
@cipherstash/drizzle— a separate published package that@cipherstash/stackdoes not depend on. With only@cipherstash/stackinstalled there's no local bin, so npx resolves a registry package namedgenerate-eql-migration:Replaced with
stash eql install --drizzle, which runsdrizzle-kit generate --customand writes the bundled EQL SQL. (The old bin pins its own EQL version and downloads the SQL from GitHub.)3. An additive
db pushis promoted bystash db activate, not by cutoverThe skill told Proxy users the rollout's pending config "will be promoted to active by
stash encrypt cutover". It won't. Perpackages/cli/src/commands/db/push.ts:110-145, a purely-additive pending is promoted bystash db activate; cutover promotes only the rename pending.And it's worse than a no-op: the cutover-time
db pushcallsdiscardPendingConfig()before writing its own pending (push.ts:119). So an un-activated rollout pending is thrown away, and Proxy keeps serving an active config that knows nothing about<col>_encryptedfor the entire dual-write window.db activateappeared nowhere in the skill. It does now, with the failure mode spelled out.4. Encrypted columns must be nullable
The storage DDL declared an encrypted column as
jsonb NOT NULL. Invariant 1 of the doctrine that ships alongside this skill forbids exactly that: "Never mark themNOT NULLat creation — the application writes ciphertext after the column exists, and aNOT NULLconstraint will break inserts."Also corrected the "EQL extension" wording — EQL is a schema (
eql_v2) plus a composite type (public.eql_v2_encrypted), installed bystash eql install.Also
stash encrypt cutoverscaffolds (cutover.ts:130-148) to keep drizzle-kit's snapshot in sync. Omitted entirely before, and it's exactly the sort of thing a Drizzle skill exists to say.timestampandtextdataTypevalues — theCastAsunion has 8 members; the skill listed 6.stash db pushwhen not using Proxy #447 at open EQL v3: make it the default, and stop the CLI recommendingdb push(a v2/Proxy-only step) #585.encryptedTypeemitseql_v2_encrypted;--eql-version 3skips the Drizzle path), and that@cipherstash/stack/drizzleis self-contained — not a re-export of@cipherstash/drizzle, which is an older parallel implementation with different symbol names.Verification
eqfallback against the built adapter (output above). My first probe was wrong — it calledgetEncryptedColumnConfigwith one argument instead of two, so no config attached and both columns looked like they fell back. Re-run correctly, the contrast is clean.jsonbPathExiststhrowsEncryptionOperatorErrorwithoutsearchableJson.npm view generate-eql-migration→ E404, and that@cipherstash/stackdeclares nobin.discardPendingConfigruns before the new pending insert (push.ts:119).vitest run __tests__/drizzle-operators-jsonb.test.ts— 11 passing. Biome clean onoperators.ts.steVecSelector,::eql_v2_encryptedcast),asc/desc, batchedand/or, and everyResultsnippet narrows.failurebefore.data.Follow-up (not in this PR)
eqon an encrypted column with the wrong index should probably throw, like thejsonb*operators do, rather than silently degrading to a plaintext comparison. The fallback comment says "Return regular Drizzle operator for non-encrypted columns" — but the branch also catches encrypted columns that merely lack the index, which is a different situation. That's a behaviour change to the flagship package, so I've documented the hazard here and left the code alone. Happy to do it separately.