docs(skills): stop teaching deprecated LockContext.identify() (#591)#642
Merged
Conversation
The stash-encryption + stash-supabase skills and the @cipherstash/stack README
led with LockContext.identify() as the primary identity-aware-encryption path.
identify() is deprecated — per-operation CTS tokens were removed in protect-ffi
0.25. Lead with the current pattern instead (OidcFederationStrategy +
.withLockContext({ identityClaim })), and demote identify() to a clearly-marked
deprecated note.
Carries forward the identity-docs correction from the now-superseded skill-refresh
PRs (#598/#604/#606), which predated the adapter split + contains→matches rename
and would otherwise regress the package paths and matches() surface. The current
candidate skills are the source of truth for structure/paths/matches; only the
identity-docs fix needed salvaging.
Skills ship in the stash tarball → stash patch; README → @cipherstash/stack patch.
Refs #591
Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
🦋 Changeset detectedLatest commit: 7117032 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 |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughDocumentation updates make EQL v3 the primary documented surface across the stack README, encryption, Drizzle, Supabase, CLI, and agent guidance. Authentication now uses OIDC federation with per-operation lock contexts, while EQL v2 remains documented only for legacy and specified exceptions. ChangesEQL v3 core API
Integration documentation
Authentication and release 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 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 |
This was referenced Jul 13, 2026
calvinbrewer
approved these changes
Jul 13, 2026
…xts) Authentication is its own concern: the client authenticates to ZeroKMS via config.authStrategy (auto by default; AccessKeyStrategy for services; OidcFederationStrategy to authenticate AS the end user), and an OIDC-authenticated client works fine with no lock context. Lock contexts are the optional extension in the other direction — they REQUIRE an OidcFederationStrategy-authenticated client, since the claim's value resolves from the federated JWT. The README and both skills now carry an Authentication section (strategy selection + the OIDC federation example + the #602 typecheck callout, which is an auth concern) above a lock-context-focused Identity-Aware Encryption section that states the dependency explicitly in both directions. Also fixes the supabase example's invalid comma-after-comment sketch, ages the callout's "protect-ffi 0.28" to "0.28+", and repairs the README TOC/anchor links for the renamed heading. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@packages/stack/README.md`:
- Around line 451-454: Pin the stash CLI version in every documented invocation:
update the authentication examples in packages/stack/README.md (451-454),
skills/stash-encryption/SKILL.md (421-421), and skills/stash-supabase/SKILL.md
(294-299) so each npx stash command uses the approved explicit version.
- Around line 493-500: Guard the Result values before accessing their data
fields: in packages/stack/README.md lines 493-500, handle encrypted.failure
before passing encrypted.data to decrypt; in skills/stash-encryption/SKILL.md
lines 488-492, check identified.failure before using identified.data from
LockContext.identify().
- Around line 467-475: Align both OIDC examples with the existing AuthStrategy
type workaround: update packages/stack/README.md lines 467-475 and
skills/stash-supabase/SKILL.md lines 304-313 to cast strategy.data or apply the
documented `@ts-expect-error` pattern used in skills/stash-encryption/SKILL.md,
while preserving the existing OidcFederationStrategy.create and Encryption flow.
In `@skills/stash-encryption/SKILL.md`:
- Around line 488-492: Update the LockContext identify flow to check
identified.failure before accessing identified.data. Handle the failure using
the established Result pattern, and only pass successful identification data to
client.encrypt(...).withLockContext.
In `@skills/stash-supabase/SKILL.md`:
- Around line 323-339: Update the withLockContext example in SKILL.md to pass a
supported LockContext instance instead of the plain { identityClaim: ["sub"] }
object. Use the existing LockContext construction/API from
packages/stack-supabase, while preserving the federated JWT claim-binding
behavior and the same claim for encryption and decryption.
🪄 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
Run ID: aaa7b1f5-4df3-48f8-a8d2-d24f02d34be1
📒 Files selected for processing (4)
.changeset/skills-identity-docs-refresh.mdpackages/stack/README.mdskills/stash-encryption/SKILL.mdskills/stash-supabase/SKILL.md
…v vars for CI-prod Env vars were presented as the first/main configuration path. The preferred order is: `npx stash auth login` for local development (no credentials in the environment; `npx stash init` is the agent-assisted end-to-end flow), and the CS_* environment variables for CI and production. Restructures the stash-encryption skill's Configuration section around that split, adds the credentials-first pointer at the top of the supabase skill's Setup, reorders the Authentication section openers in all three files, and fixes the README's stale `npx @cipherstash/stack auth login` spelling. Auto-strategy precedence (env vars over profile when both present) is stated as a fact, separate from the recommendation. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Rewrites stash-encryption, stash-drizzle, and stash-supabase skills and the stack README so the EQL v3 typed surface (EncryptionV3, types.* concrete domains, stack-drizzle/v3, encryptedSupabaseV3) is the only approach taught. EQL v2 shrinks to one short Legacy section per document (the API remains for existing deployments; docs-site link), with two explicit carve-outs: - DynamoDB still requires the v2 schema surface — noted, tracked in #657. - The encrypt rollout lifecycle (stash encrypt backfill/cutover, @cipherstash/migrate) currently targets v2 columns — sections kept (launch-critical) under a #648 callout that scopes their eql_v2_* names. Also fixes the legacy packages/drizzle README still recommending the removed @cipherstash/stack/drizzle subpath (now names @cipherstash/stack-drizzle), the README's stale sorting-not-supported claim (superseded by ord_term / OPE-backed order()), the stale ops.contains free-text guidance (matches), and Node >= 18 → >= 22 in Requirements. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
…oach # Conflicts: # skills/stash-drizzle/SKILL.md # skills/stash-encryption/SKILL.md # skills/stash-supabase/SKILL.md
This was referenced Jul 15, 2026
Fixes from the #658 review (10 verified findings + below-the-fold): Correctness: - Quick Start + mid-doc examples now narrow the Result union before reading .data (failure branches throw) — they previously failed strict TS (TS2339). - Supabase EQL install: docs now show `--supabase` (grants) and stop implying it is obsolete; without it encrypted queries hit permission-denied. - Removed the false "Supabase cannot ORDER BY encrypted columns" claim from stash-encryption (it contradicted stash-supabase, the README, and the shipped OPE-backed order()). - #648 rollout dead-end fixed: the Supabase skill now EMBEDS the interim v2-encrypted-twin recipe (schema + encryptedSupabase + dual-write) that backfill/cutover need today, instead of pointing at a deleted section. Reference rot & stale claims: - Removed the #602 "known type error" callout (fixed by protect-ffi 0.29; #602 closed) from stash-encryption; corrected the stale #447 citation in all three skills; fixed the "EQL v3 Typed Schema" dead cross-reference and the generate-eql-migration misattribution in stash-drizzle. - stash-cli: --eql-version default 2→3; corrected the Supabase ORDER-BY and read-path (encryptedSupabaseV3) claims. - AGENTS.md Key Concepts now teaches the v3 typed surface (rule 7). Scoping corrections: like/ilike (matches, with the untyped-only shim noted), empty-needle rejection (adapter-level, not core encryptQuery), SQL-NULL ne inclusion (both adapters), Supabase selector leaf types (JSON scalars only), null-operand handling (forwarded, not rejected), per-column (not per-query) ZeroKMS batching, #137-vs-#654 exposure attribution, drizzle operator opts (async encrypting ops + selector methods only). README polish: init flag table (--region etc.) + removed the redundant post-init eql install step; wasm-inline in Requirements; returnType values note restored; markdown `--`→`---` rules. Added the missing @cipherstash/drizzle changeset for its README fix. The doc claims about the Supabase short-needle guard, withLockContext({identityClaim}), and the StackError discriminated union are made true by the companion code PR (short-needle guard, LockContextInput widening, EncryptionErrorTypes as const) — both are RC-gated to ship together. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Contributor
There was a problem hiding this comment.
Pull request overview
Updates customer-shipped documentation (agent skills + READMEs) to stop teaching the deprecated LockContext.identify() flow and align guidance with the current identity-aware encryption pattern: authenticate via OidcFederationStrategy, then bind identity per operation with .withLockContext(...). It also standardizes the skills/README narrative around EQL v3 as the primary documented surface (with explicit “Legacy: EQL v2” sections).
Changes:
- Refreshes
stash-encryption,stash-supabase, andstash-drizzleskills to lead with EQL v3 types/domains and the current identity-aware encryption flow, demotingLockContext.identify()to a deprecated note. - Updates
@cipherstash/stackREADME (and legacy@cipherstash/drizzleREADME pointer) to match the post-split package layout and v3-first guidance. - Adds Changesets for the documentation changes shipped via
stashand@cipherstash/stack(plus@cipherstash/drizzle).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| skills/stash-supabase/SKILL.md | Reworks Supabase skill around encryptedSupabaseV3, EQL v3 domains, updated query semantics, and identity-aware encryption docs. |
| skills/stash-encryption/SKILL.md | Reframes the canonical encryption skill as v3-first, adds auth/lock-context guidance, and keeps a concise v2 legacy section. |
| skills/stash-drizzle/SKILL.md | Updates Drizzle skill to v3 /v3 subpath, types.* domains, operators (matches, JSON selectors), and revised lifecycle notes. |
| skills/stash-cli/SKILL.md | Updates documented defaults/wording around EQL install behavior and post-cutover read-path guidance for v3/v2 wrappers. |
| packages/stack/README.md | Updates the package README to v3 typed client + OidcFederationStrategy-based identity docs; adds explicit legacy v2 section. |
| packages/drizzle/README.md | Fixes the “new projects” pointer to the split @cipherstash/stack-drizzle package (v3 on /v3). |
| AGENTS.md | Updates repo “Key Concepts and APIs” bullets to describe the v3 typed surface as the documented approach. |
| .changeset/skills-identity-docs-refresh.md | Changeset for stash + @cipherstash/stack doc updates focused on identity docs correction. |
| .changeset/eql-v3-sole-docs.md | Changeset for stash + @cipherstash/stack doc updates establishing v3 as the sole documented approach. |
| .changeset/drizzle-legacy-readme-pointer.md | Changeset for @cipherstash/drizzle README pointer correction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…and-corrections-refresh
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
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.
Targets the 1.0 candidate. Closes the identity-docs half of #591 and supersedes the three stale skill-refresh PRs.
What
The
stash-encryption+stash-supabaseskills and the@cipherstash/stackREADME taught the deprecatedLockContext.identify()as the primary identity-aware-encryption path.identify()fetches a per-operation CTS token that was removed in protect-ffi 0.25 and is no longer used by encryption. These ship to customers (skills via thestashtarball; README with the package), so it's wrong guidance at launch.Now they lead with the current pattern — authenticate with
OidcFederationStrategy, then.withLockContext({ identityClaim })per operation — and demoteidentify()to a clearly-marked deprecated note.Why not just merge #598 / #604 / #606
Those skill-refresh PRs predate both the adapter split (they still reference
@cipherstash/stack/{supabase,drizzle}, 0 references to the new@cipherstash/stack-*packages) and thecontains()→matches()rename (0matches(). Each is a ~400-line near-total rewrite, so merging/rebasing would regress the package paths and thematches()surface. The current candidate skills are the source of truth for structure/paths/matches; the only RC-critical content those PRs held was the identity-docs correction (which #598 nailed), carried forward here.I'm closing #598 / #604 / #606 as superseded.
Scope notes
packages/protect/README.mdstill referencesidentify()but is the legacy package — left as a follow-up, not RC-critical.encryption/index.tsauthStrategychange is already on the candidate; the "vacuously-passing" unitlock-context.test.tsis now superseded by test(stack): federate a Clerk M2M JWT for the identity suites; wire into CI #620's realmatrix-identityintegration coverage — the leftover redundant unit test is a hygiene item for Extend no-skip enforcement to the unit vitest config #628 (unit no-skip gate).Changesets:
stashpatch (skills) +@cipherstash/stackpatch (README).Refs #591
https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Summary by CodeRabbit
withLockContext) with JWT identity claim names.LockContext.identify()and documented the removal of per-operation CTS tokens.