feat(cli): add the stash-sql and stash-edge skills — raw-SQL predicates and the WASM entry#777
Open
coderdan wants to merge 1 commit into
Open
feat(cli): add the stash-sql and stash-edge skills — raw-SQL predicates and the WASM entry#777coderdan wants to merge 1 commit into
coderdan wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 3895d94 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 |
📝 WalkthroughWalkthroughAdds ChangesSQL and edge integration guidance
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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.
Inline comments:
In @.changeset/stash-sql-edge-skills.md:
- Around line 48-51: Update the change description to include repository
examples covering the revised stash init/impl handoffs, wizard skill
installation, and raw-SQL/WASM guidance, and explicitly note that the separately
maintained documentation site requires corresponding updates. Preserve the
existing integration-specific guidance and cross-links.
In `@skills/stash-drizzle/SKILL.md`:
- Around line 420-421: Update the raw SQL example in the surrounding
documentation to remove the nested backticks from the inline Markdown code span.
Use a safe placeholder such as db.execute(sqlTemplate), or convert the example
to a fenced TypeScript block while preserving the guidance about operand casts
and driver-specific parameter binding.
In `@skills/stash-edge/SKILL.md`:
- Around line 291-294: Update the fenced code block containing the
EncryptedTextSearchColumn error in SKILL.md to specify the text language tag,
such as ```text, while leaving the documented error content unchanged.
In `@skills/stash-sql/SKILL.md`:
- Around line 68-73: Update the fenced code blocks containing the eql_v3
mappings in SKILL.md, including the additionally referenced block, by adding the
text language identifier to each opening fence while preserving their contents.
- Around line 182-188: Update every recipe using encryptQuery, including the
snippets around term and their corresponding examples, to guard the Result
before dereferencing .data: check .failure and handle or return the failure,
then execute the SQL only on success. Apply the same pattern consistently to all
referenced recipes, or explicitly state that success/error handling is omitted
for brevity.
- Line 3: Update the description metadata in SKILL.md to include the missing <=
and > operators alongside the existing operator list, keeping it consistent with
the predicate matrix and preserving all other description content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 17c1183c-c043-47b6-878b-9830d357e011
📒 Files selected for processing (15)
.changeset/stash-sql-edge-skills.mdAGENTS.mdpackages/cli/src/commands/init/lib/__tests__/install-skills.test.tspackages/cli/src/commands/init/lib/install-skills.tspackages/cli/src/commands/init/lib/setup-prompt.tspackages/stack/src/wasm-inline.tspackages/wizard/src/lib/install-skills.tsskills/stash-cli/SKILL.mdskills/stash-drizzle/SKILL.mdskills/stash-edge/SKILL.mdskills/stash-encryption/SKILL.mdskills/stash-indexing/SKILL.mdskills/stash-prisma-next/SKILL.mdskills/stash-sql/SKILL.mdskills/stash-supabase/SKILL.md
Comment on lines
+48
to
+51
| `stash init` / `stash impl` handoffs and the `@cipherstash/wizard` skills | ||
| prompt install both skills for the `postgresql` and `supabase` integrations. | ||
| Drizzle and Prisma Next get cross-links from their own skills instead, since | ||
| those integrations emit correctly-typed operands themselves. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Add example coverage and a documentation-site follow-up.
These changes alter the public stash init / wizard handoff workflow and introduce raw-SQL/WASM usage guidance. Add or update repository examples and explicitly track corresponding updates for the separately maintained documentation site.
As per coding guidelines, “When public APIs or user-facing workflows change, update repository examples and flag that the separately maintained documentation site needs corresponding updates.”
🤖 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 @.changeset/stash-sql-edge-skills.md around lines 48 - 51, Update the change
description to include repository examples covering the revised stash init/impl
handoffs, wizard skill installation, and raw-SQL/WASM guidance, and explicitly
note that the separately maintained documentation site requires corresponding
updates. Preserve the existing integration-specific guidance and cross-links.
Source: Coding guidelines
Comment on lines
+420
to
+421
| > **Dropping to raw SQL?** `db.execute(sql\`…\`)` bypasses the operators this integration emits, so you own the operand casts yourself — an encrypted predicate needs its needle cast to the column's `eql_v3.query_*` domain, and the driver's parameter-binding rules differ between `pg` and `postgres-js`. The `stash-sql` skill is the reference for both. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid nested backticks inside this inline code span.
Line 420 embeds a template literal inside an inline Markdown code span, triggering MD038 and risking incorrect rendering. Use a fenced TypeScript example or a placeholder such as db.execute(sqlTemplate).
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 420-420: Spaces inside code span elements
(MD038, no-space-in-code)
🪛 SkillSpector (2.3.11)
[warning] 27: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
🤖 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 `@skills/stash-drizzle/SKILL.md` around lines 420 - 421, Update the raw SQL
example in the surrounding documentation to remove the nested backticks from the
inline Markdown code span. Use a safe placeholder such as
db.execute(sqlTemplate), or convert the example to a fenced TypeScript block
while preserving the guidance about operand casts and driver-specific parameter
binding.
Source: Linters/SAST tools
Comment on lines
+291
to
+294
| ``` | ||
| Type 'EncryptedTextSearchColumn' is not assignable to type 'AnyEncryptedV3Column'. | ||
| Types have separate declarations of a private property 'columnName'. | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language tag to this fenced block.
Line 291 should use ```text (or another appropriate language) so the skill passes MD040 and renders the TypeScript error example consistently.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 291-291: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@skills/stash-edge/SKILL.md` around lines 291 - 294, Update the fenced code
block containing the EncryptedTextSearchColumn error in SKILL.md to specify the
text language tag, such as ```text, while leaving the documented error content
unchanged.
Source: Linters/SAST tools
| @@ -0,0 +1,365 @@ | |||
| --- | |||
| name: stash-sql | |||
| description: Query EQL v3 encrypted columns from hand-written SQL over `pg` (node-postgres) or `postgres` (postgres-js) — no ORM. Covers the column-domain-to-query-domain operator matrix (which of `=`, `<>`, `<`, `>=`, `@@`, `@>` each encrypted domain accepts), minting search needles with `encryptQuery`, the per-driver parameter-binding rules for encrypted payloads, and the double-encoding failure that trips the domain CHECK with a message naming neither JSON nor encoding. Use when writing INSERT/SELECT against an encrypted column without an ORM, when a predicate returns zero rows or raises "operator does not exist", or when a domain CHECK constraint rejects an encrypted value on write. | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the operator list consistent with the predicate matrix.
The description omits <= and > even though the matrix documents both. Include every operator covered by the skill so users do not incorrectly conclude those predicates are unsupported.
🤖 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 `@skills/stash-sql/SKILL.md` at line 3, Update the description metadata in
SKILL.md to include the missing <= and > operators alongside the existing
operator list, keeping it consistent with the predicate matrix and preserving
all other description content.
Comment on lines
+68
to
+73
| ``` | ||
| public.eql_v3_text_eq → eql_v3.query_text_eq | ||
| public.eql_v3_text_search → eql_v3.query_text_search | ||
| public.eql_v3_bigint_ord → eql_v3.query_bigint_ord | ||
| public.eql_v3_timestamp_ord → eql_v3.query_timestamp_ord | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to fenced code blocks.
The bare fences trigger Markdownlint MD040. Mark these blocks as text (or another appropriate language) so the published skill passes documentation checks.
Also applies to: 159-161
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@skills/stash-sql/SKILL.md` around lines 68 - 73, Update the fenced code
blocks containing the eql_v3 mappings in SKILL.md, including the additionally
referenced block, by adding the text language identifier to each opening fence
while preserving their contents.
Source: Linters/SAST tools
Comment on lines
+182
to
+188
| ```ts | ||
| const term = await client.encryptQuery(email, { | ||
| table: users, column: users.email, queryType: 'equality', | ||
| }) | ||
| await sql`SELECT * FROM users | ||
| WHERE email = ${sql.json(term.data)}::jsonb::eql_v3.query_text_eq` | ||
| ``` |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle encryptQuery failures in every recipe.
These snippets dereference .data without checking .failure, unlike the earlier examples. Add the same Result guard to each recipe, or explicitly state that success handling is omitted for brevity.
Also applies to: 195-203, 212-219, 229-235, 247-251, 257-260
🤖 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 `@skills/stash-sql/SKILL.md` around lines 182 - 188, Update every recipe using
encryptQuery, including the snippets around term and their corresponding
examples, to guard the Result before dereferencing .data: check .failure and
handle or return the failure, then execute the SQL only on success. Apply the
same pattern consistently to all referenced recipes, or explicitly state that
success/error handling is omitted for brevity.
…es and the WASM entry Closes #754. No shipped skill covered the integrations that don't use an ORM: hand-written SQL over `pg` / `postgres-js`, and the WASM entry on Deno / Supabase Edge Functions / Workers. Grepping the skills `stash init` installs for `postgres-js|::jsonb::eql|sql.json|query_text_search` returned one hit, in an unrelated code comment — so an integration on that path had to recover the binding surface from `dist/*.d.ts`, the Postgres catalog, and experiment. Two skills rather than one, per the scope question in the issue: the raw-SQL predicate cookbook serves a supported plain-Node path (`hono-pg`) with no edge or WASM involvement, so scoping it under an edge-named skill would leave that surface uncovered. skills/stash-sql — the predicate matrix (which of `=`, `<>`, `<`, `>=`, `@@`, `@>` each column domain accepts, against which `eql_v3.query_*` operand), storage-vs-query payload shapes, per-driver parameter binding, and recipes for equality / free-text / range / ORDER BY / JSON containment / field selectors. skills/stash-edge — import specifier per runtime, the four mandatory `CS_*` variables and minting them with `stash env`, how the WASM client surface differs from the native typed client, and why a schema module can't be shared across the two entries. Both carry the credential-identity rule, also folded into stash-cli (under `env` and `encrypt backfill`, where it bites) and stash-supabase: EQL index terms derive from the ZeroKMS client key, so rows written under one credential and queried under another decrypt correctly and never match a query. Three claims were corrected against a live EQL v3 3.0.2 install rather than carried over from the issue: - The binding rule is driver-specific. On postgres-js a bare object fails to INSERT and `JSON.stringify(...)::jsonb` fails the domain CHECK; only `sql.json()` works in both positions. On `pg` all three forms work. - A bare-`jsonb` operand does not fall through to native jsonb semantics — EQL defines `jsonb` overloads that coerce to the *storage* domain, which requires the ciphertext key `c`, so a query term without the domain cast raises a CHECK violation rather than silently matching nothing. - The schema type incompatibility reproduces in both directions, not one. Also fixes the wasm-inline module JSDoc, which passed `OidcFederationStrategy.create(...)`'s Result straight to `config.authStrategy` without unwrapping — the same JSDoc the raw-SQL surface was being reverse-engineered from. SKILL_MAP (CLI + wizard) installs both for `postgresql` and `supabase`; Drizzle and Prisma Next get cross-links from their own skills instead.
coderdan
force-pushed
the
feat/stash-sql-edge-skills
branch
from
July 24, 2026 07:34
e6498d4 to
3895d94
Compare
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
1 participant
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.
Closes #754.
Problem
There is a
stash-drizzle, astash-supabase, and astash-dynamodb. Nothing covered the integrations that don't use an ORM — hand-written SQL overpg/postgres-js, and the WASM entry on Deno / Supabase Edge Functions / Workers.Grepping the skills
stash initinstalls forpostgres-js|::jsonb::eql|sql.json|query_text_search|query_numeric_ord|eql_v3.matches(returns one hit across all of them, in an unrelated code comment. An integration on that path had to recover the binding surface fromdist/*.d.tsJSDoc, the Postgres catalog, and experiment.Two skills, not one
Per the scope question in the issue: the raw-SQL predicate cookbook serves a supported plain-Node path (
hono-pg— no Deno, no WASM, same binding problem), so scoping it under an edge-named skill would leave that surface uncovered. Same reasoning that splitstash-indexingout rather than folding it intostash-encryption.skills/stash-sql/SKILL.md(367 lines) — the column-domain × query-domain operator matrix, storage-vs-query payload shapes, per-driver parameter binding, and recipes for equality / free-text / range /ORDER BY/ JSON containment / JSON field selectors, plus theinformation_schemadrift check.skills/stash-edge/SKILL.md(398 lines) — import specifier per runtime, the four mandatoryCS_*variables and minting them withstash env, how the WASM client surface differs from the native one, and why a schema module can't be shared across entries.Both are single self-sufficient files, required for the Codex/editor-agent inline path.
Verified against a live install, not transcribed
I had a Supabase stack with EQL v3 3.0.2 running, so the predicate matrix is extracted from
pg_operatorrather than inferred, and the binding rules were tested. Three claims came out differently than the issue recorded:postgres-jsa bare object fails to INSERT (invalid input syntax for type json) andJSON.stringify(...)::jsonbfails the domain CHECK; onlysql.json()works in both positions. Onpg, all three forms work. "Bind as an object" is postgres-js guidance, so the skill splits it per driver.jsonboperand does not fall through to native jsonb semantics. EQL definesjsonboverloads that coerce to the storage domain —eql_v3.eq_term(b::public.eql_v3_text_search)— which requires the ciphertext keyc. A ciphertext-free query term without the domain cast therefore raises a CHECK violation rather than silently matching nothing. Corrected in three places where I'd first written the issue's version.Types have separate declarations of a private property 'columnName'. That rules out "author against wasm-inline and share" as a fix — the guidance is to author against exactly one entry. Re-verified on this branch after feat(stack)!: EQL v3 audit-on-decrypt + collapse EncryptionV3 into Encryption (EQL v2 removal PR 3) #768 collapsed the client factories.The double-encoding failure also gets a one-line diagnosis it didn't have:
SELECT jsonb_typeof($1::jsonb)returns'string', and every EQL domain CHECK opens withjsonb_typeof(VALUE) = 'object'— which is why the error names neither JSON nor encoding.Two bugs found along the way
packages/stack/src/wasm-inline.ts— the module JSDoc passedOidcFederationStrategy.create(...)'sResultstraight toconfig.authStrategywithout unwrapping. That is the same JSDoc the issue names as the reverse-engineering source, so it was actively teaching the mistake. Fixed there and written correctly in the skill.npm:@cipherstash/stack@^1.0.0/wasm-inlineform I first wrote would not resolve — semver ranges exclude prereleases, and the published version is1.0.0-rc.4. The skill pins exactly and explains why.Alignment with the v2 removal
Rebased from
mainontoremove-v2so the new skills describe only the post-removal surface. What that changed:EncryptionV3is now a deprecated alias ofEncryption(feat(stack)!: EQL v3 audit-on-decrypt + collapse EncryptionV3 into Encryption (EQL v2 removal PR 3) #768), sostash-edge's "useEncryption, notEncryptionV3" contrast was stale. Replaced with the distinction that actually survives: both entries name the factoryEncryption, so the import path is the only thing separating them — which is a better warning anyway, since the two take different config and different bulk shapes.@cipherstash/stack/v3, matchingstash-encryptionon this branch.stash-encryption's subpath table resolved in favour ofremove-v2's rows (@cipherstash/stack-drizzleat root,encryptedSupabase, the v3-only DynamoDB note), keeping only mywasm-inlineaddition.eql_v2,EncryptionV3,encryptedColumn,encryptedSupabaseV3, orstack-drizzle/v3reference is introduced.Folded into existing skills
stash-cli— the credential-identity rule underenvand underencrypt backfill, where it actually bites (backfill from a laptop, query from an Edge Function → every row decrypts, no search ever matches). Backfill is version-aware, not v2-only, so this applies to the v3 columns these skills are about.stash-supabase— same rule, plus pointers to both new skills.stash-encryption— the entry table said the WASM entry has "the v3 authoring surface", which reads as though a schema can be shared. Corrected, with an[!IMPORTANT]explaining the nominal-typing mechanism and whyas anyis the wrong fix.stash-drizzle,stash-prisma-next— cross-links for the raw-SQL escape hatch (db.execute(sql\…`)` bypasses the operators those integrations emit).stash-indexing— its "Raw SQL / plain PostgreSQL" bullet now points somewhere.Wiring
SKILL_MAP(CLI + wizard) installs both forpostgresqlandsupabase— Supabase because Edge Functions are the flagship WASM-entry use and its migrations/RPC are hand-written SQL. Drizzle and Prisma Next get cross-links instead, since they emit correctly-typed operands themselves.SKILL_PURPOSESentries added (covered by the existing "purpose line for every installable skill" test), plus newSKILL_MAPassertions.Verification
On the rebased base:
pnpm run code:check— 0 errors, 196 warnings, matching theremove-v2baseline in feat!: remove EQL v2 repo-wide (umbrella, #707) #772.pnpm --filter stash test— 841 passed;pnpm --filter @cipherstash/wizard test— 265 passed.packages/cli/dist/skills/after build.stash manifest --json.Not done
The schema non-interchange is documented here rather than fixed, per the issue's scoping ("worth stating explicitly either way").
Correction to an earlier version of this section: it suggested dropping
privateoncolumnName/definitionto restore structural compatibility across the two.d.tsbundles. That is the wrong fix, and the code says so directly (packages/stack/src/eql/v3/columns.ts:426-433):Those privates are load-bearing. Removing them would make storage-only domains mutually assignable and degrade the very inference the v3 surface exists to provide — trading a papercut for a real regression.
It is also not an isolated papercut. #780's Not in this PR section defers a second symptom of the same root cause:
encryptQueryin the published.d.tscollapses its plaintext parameter toneverunder private-field type erasure. Reproduced against a freshdist/build:Both symptoms share a cause — the v3 column classes' type identity does not survive
.d.tsemission intact — and both are invisible to CI for the same reason: the type tests import source, notdist/. Whatever the fix turns out to be, adist/-consuming type test is the thing that would have caught either one. Worth scoping the two together rather than separately.