docs(supabase): correct the stash-supabase skill#606
Conversation
🦋 Changeset detectedLatest commit: 7af5a92 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 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:
📝 WalkthroughWalkthroughDocumentation-only update to skills/stash-supabase/SKILL.md revising database schema guidance, EQL install commands, query filter warnings, identity-aware encryption examples, error-handling notes, exported types, EQL v3 install caveats, and migration/cutover operational guidance. ChangesSKILL.md documentation revisions
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/stash-supabase/SKILL.md (1)
741-749: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClarify the SDK-only cutover workaround
stash db pushonly createspendingwhen an active config already exists; on a fresh SDK-only project it writesactive, sostash encrypt cutoverstill fails. Qualify this note with that prerequisite or spell out the missing bootstrap step.🤖 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-supabase/SKILL.md` around lines 741 - 749, The SDK-only cutover note is incomplete because stash db push only creates a pending EQL config after an active config already exists. Update the guidance in the stash encrypt cutover section to call out that a fresh SDK-only project must first bootstrap the initial config before the pending-row workaround applies, and reference the stash db push / stash encrypt cutover flow so readers know when the workaround is valid.
🤖 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.
Outside diff comments:
In `@skills/stash-supabase/SKILL.md`:
- Around line 741-749: The SDK-only cutover note is incomplete because stash db
push only creates a pending EQL config after an active config already exists.
Update the guidance in the stash encrypt cutover section to call out that a
fresh SDK-only project must first bootstrap the initial config before the
pending-row workaround applies, and reference the stash db push / stash encrypt
cutover flow so readers know when the workaround is valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f3c2082c-a172-4058-9c18-95f4f0e219c4
📒 Files selected for processing (1)
skills/stash-supabase/SKILL.md
There was a problem hiding this comment.
Pull request overview
This PR corrects and hardens the public stash-supabase skill (skills/stash-supabase/SKILL.md) so that the guidance installed into Supabase user projects matches the actual CLI + SDK behavior (EQL install paths, schema invariants, query semantics, identity-aware encryption, and error discrimination).
Changes:
- Fixes the database schema and EQL installation guidance (nullable
jsonbencrypted columns; documentstash eql install --supabase --migration; clarify EQL is not a PG extension). - Documents known adapter behaviors and footguns (notably
.filter()always encrypting operands asequality, the declared-vs-undeclared column failure modes, and how to distinguish encryption failures givenerror.encryptionErroris alwaysundefined). - Updates identity-aware encryption docs to the current strategy-based flow (no
LockContext.identify()), adds v3 install caveats, and refreshes the documented export surface.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rebased onto the EQL v3 integration branch (#616). Reconciled against CJ's eql-3.0.0 GA re-baseline of the skill, which #606 (branched from main) had not seen, and corrected the order() guidance now that #616 makes order() work on Supabase v3 ordering columns. - order(): the skill said ORDER BY on encrypted columns is unsupported on Supabase. That is still true for EQL v2 (operator families need superuser), but v3 order() now works on OPE-backed ordering columns (*_ord, text_ord, text_search) via the col->op jsonb path. Scoped the v2 statement, documented the v3 support matrix (ORE-only ordering columns and no-ordering-term columns are rejected), and fixed the typed-narrowing note (order() accepts plaintext AND OPE ordering columns, not plaintext only). - Exports: corrected the v3 type list to the real surface (EncryptedSupabaseV3Options not V3Config; added TypedEncryptedSupabaseV3Instance, EncryptedQueryBuilderV3Untyped, V3FreeTextSearchableKeys; kept EncryptedSupabaseResponse/Error and V3FilterableKeys). - Kept the newer re-baseline content (single v3 SQL artifact, ORE self-skip, --supabase grants, honest contains() exact-match limitation) and preserved the additive `supabase db reset` warning. - Dropped the include_original:false substring workaround: protect-ffi ignores the flag (proven by this branch's match-bloom tests), so it does not enable substring search. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
coderdan
force-pushed
the
docs/stash-supabase-skill-refresh
branch
from
015e8da to
ee9667e
Compare
…into docs/stash-supabase-skill-refresh
Restructures the skill so encryptedSupabaseV3 leads: connect-time introspection
(no schema arg), native public.eql_v3_* domains, select('*'), contains() (not
like/ilike), order() on OPE-backed columns, and a v3 migration guide. Relegates
v2 (encryptedSupabase, still shipped) to a slim "Legacy: EQL v2" section.
Addresses the review comments on #606: EQL 3.0.0 type names in the schema
section, schema declaration presented as optional (introspection), automatic
schema loading as the default, OPE ordering (v3), and the V3-first structure.
Verified the v3 API surface (encryptedSupabaseV3 options.config.authStrategy,
withLockContext/audit/contains/order on the builder, ./eql/v3 exports) and the
referenced CLI commands against `stash manifest --json`.
Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Link the encryptionError caveat to #626 and the interim full-envelope operand note to #622. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
|
Superseded by #642. Same reason as #604: predates the adapter split (references |
Update since first review (rebase onto #616)
order()on Supabase is no longer "impossible." The skill's blanket "ORDER BY on encrypted columns is not supported on Supabase" was true for EQL v2 (operator families need superuser) but is now wrong for v3:order()works on OPE-backed ordering columns (*_ord,text_ord,text_search) via thecol->opjsonb path. Scoped the v2 statement, added the v3 support matrix (ORE-only and no-ordering-term columns rejected with a clear error), and fixed the typed-narrowing note (order()accepts plaintext and OPE ordering columns).EncryptedSupabaseV3Options(notV3Config), plusTypedEncryptedSupabaseV3Instance,EncryptedQueryBuilderV3Untyped,V3FreeTextSearchableKeys.include_original: falsesubstring workaround —protect-ffiignores the flag (proven by test(stack): EQL v3 integration harness + shared test kit, and two Supabase adapter fixes (PR1 of 4) #616'smatch-bloomsuite), so it does not enable substring search. The skill now states the honest exact-match limitation, matching the re-baseline.stashpatch changeset (skills ship in the tarball).Why
skills/stash-supabase/SKILL.mdships in thestashtarball andstash initinstalls it into every Supabase project. Same defect classes as #598 (encryption) and #604 (drizzle), plus two of its own.This is the correctness pass. The restructure you asked for on #598 (local-dev section, v3 focus, migration split, cross-references) is a separate follow-up, built on the
feat/eql-v3-text-search-schemabranch.Defects
Two Linear issue IDs (
CIP-3402) were shipping to customers in this skill. It's a public artifact copied into user repos. Removed.CREATE EXTENSION IF NOT EXISTS eql_v2;— no such extension.eql_v2is a schema pluspublic.eql_v2_encrypted, installed by the CLI. The skill omitted the v2 install path entirely; it now documentsstash eql install --supabase --migration, and says why--migrationbeats--direct(a direct install does not survivesupabase db reset).Encrypted columns declared
jsonb NOT NULLin the headline schema — forbidden by invariant 1 of the doctrine that ships alongside this skill. The skill even states the rule correctly 550 lines later, and then contradicts it in its own opening example.An additive
db pushis promoted bystash db activate, not by cutover. The skill said "Cutover (later) will promote it." It won't — cutover promotes only the rename pending — and the cutover-timedb pushcallsdiscardPendingConfig()before writing its own (push.ts:119), so the un-activated additive pending is discarded and Proxy serves a stale config for the whole dual-write window.db activateappeared nowhere in the skill. Same bug as #604.Identity-aware encryption taught the deprecated
LockContext.identify()ceremony. Per-operation CTS tokens were removed inprotect-ffi0.25. Replaced withOidcFederationStrategyonconfig.authStrategyplus a directly-constructedLockContext. Documented a real asymmetry: the Supabase builder's.withLockContext()is typedLockContextonly, so unlike the core operations it will not accept a bare{ identityClaim }— verified withtsc.error.encryptionErroris alwaysundefined. The skill told users to branch on it.query-builder.ts:371-374hardcodesencryptionError: undefinedwhen constructing the error, discarding the populated value even for a genuineEncryptionFailedError. Documented the working discriminator (status === 500 && statusText === 'Encryption Error'), and flagged the code bug..filter(col, op, value)always encrypts anequalityoperand regardless ofop, so.filter('age','gt',21)builds an equality term for a range operator and cannot match. Documented, along with the two failure modes that differ from Drizzle: a wrong index on a declared column errors (good), while an undeclared column silently compares plaintext against JSONB (bad, and previously undocumented).Also
stash db pushwhen not using Proxy #447 → open EQL v3: make it the default, and stop the CLI recommendingdb push(a v2/Proxy-only step) #585.authStrategytype error (config.authStrategy doesn't typecheck: protect-ffi's AuthStrategy type is narrower than its runtime (auth 0.41 Result envelope) #602) so the identity snippet is honest about not compiling until protectjs-ffi#131 releases.--migrationis rejected under--eql-version 3, sosupabase db resetdrops EQL. Not previously stated, and this skill's own lifecycle usesdb reset.V3FilterableKeys,EncryptedSupabaseResponse,EncryptedSupabaseErrorexports.Verification
tsconfig.json(withpathsandcustomConditions: ["node"]).new LockContext()andnew LockContext({ context: { identityClaim: [...] } })compile;{ identityClaim: ['sub'] }does not — which is why the skill now says so. Only the known config.authStrategy doesn't typecheck: protect-ffi's AuthStrategy type is narrower than its runtime (auth 0.41 Result envelope) #602authStrategyerror remains.--supabase --eql-version 3selectscipherstash-encrypt-v3-supabase.sqland creates schemaeql_v3(ranloadBundledEqlSql).eql-3.0.0-alpha.2, and that botheql_v3andeql_v3_internalschemas are created.discardPendingConfig()runs before the new pending insert.select('*')rejection,::jsonbcasts, and everyResultsnippet narrowing.failurebefore.data.supabase-v3.test.tsfails onmaintoo (needs a live Postgres with ananonrole) — pre-existing, unrelated.Summary by CodeRabbit