fix(cli): prisma-next init/plan/impl crash + stash-prisma-next skill (rc.2 M1)#683
fix(cli): prisma-next init/plan/impl crash + stash-prisma-next skill (rc.2 M1)#683coderdan wants to merge 3 commits into
Conversation
…sma-next skill
SKILL_MAP was typed Record<Integration, ...> but omitted the 'prisma-next'
key, so installSkills and the AGENTS.md builder hit SKILL_MAP['prisma-next']
= undefined and threw 'not iterable' for any repo the CLI detected as Prisma
Next. tsc catches the missing key, but the build (tsup) transpiles without
type-checking, so the error shipped in rc.2 — reachable by any Prisma user
running stash init (Prisma Next is auto-detected).
- Add the prisma-next SKILL_MAP entry.
- Route both consumers through a new skillsFor() helper that degrades an
unmapped integration to the base skill set (stash-encryption + stash-cli)
instead of crashing — belt-and-braces for the next Integration variant,
since tsup won't type-check the map.
- New skills/stash-prisma-next/SKILL.md documenting the EQL v3 Prisma Next
surface: domain-named column types (EncryptedTextSearch, EncryptedDoubleOrd,
...), cipherstashFromStackV3 wiring, runtime envelopes, the eql* operators,
and EQL install via prisma-next migration apply (not stash eql install).
- Tests: SKILL_MAP has a non-empty entry for every integration; skillsFor
falls back for an unmapped one; buildAgentsMdBody('prisma-next', ...) inlines
the skill (the exact call that used to crash); installSkills copies it.
- Meta: AGENTS.md skill-map table + skills list updated.
Companion to #655 (the @cipherstash/prisma-next EQL v3 package) — that PR
releases the package but touches no CLI code, so this crash survives it.
Stacked on feat/prisma-next-eql-v3; retarget to main when #655 lands.
Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
🦋 Changeset detectedLatest commit: 29d0ea5 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 |
|
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 |
There was a problem hiding this comment.
Pull request overview
Fixes a CLI crash when stash init/plan/impl detects a Prisma Next project by adding the missing prisma-next skills mapping, and introduces a new shipped agent skill documenting the Prisma Next EQL v3 surface.
Changes:
- Add
prisma-nextto the CLI initSKILL_MAPand route skill resolution via a new resilientskillsFor()helper. - Inline/install the new
stash-prisma-nextskill duringAGENTS.mdbuilding and skills installation, with regression tests for the prior “not iterable” crash. - Update repo meta docs and add a changeset to ship the new skill + fix.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/stash-prisma-next/SKILL.md | New Prisma Next EQL v3 integration skill content (schema types, runtime wiring, operators, migrations). |
| packages/cli/src/commands/init/lib/install-skills.ts | Adds prisma-next mapping and skillsFor() fallback used by both installers and AGENTS.md builder. |
| packages/cli/src/commands/init/lib/build-agents-md.ts | Switches skill inlining to use skillsFor() instead of direct SKILL_MAP indexing. |
| packages/cli/src/commands/init/lib/tests/install-skills.test.ts | Adds tests ensuring all integrations have non-empty skills; tests skillsFor() fallback + prisma-next mapping. |
| packages/cli/src/commands/init/lib/tests/build-agents-md.test.ts | Adds regression test ensuring prisma-next inlines the new skill (and doesn’t crash). |
| AGENTS.md | Updates the published skill list and mapping table to include stash-prisma-next. |
| .changeset/stash-prisma-next-skill.md | Changeset describing the CLI crash fix and new shipped skill. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ```prisma | ||
| model User { | ||
| id String @id | ||
| email cipherstash.EncryptedTextSearch() // eq + range + free-text + ORDER BY | ||
| salary cipherstash.EncryptedDoubleOrd() // eq + range + ORDER BY | ||
| accountId cipherstash.EncryptedBigIntOrd() // eq + range + ORDER BY | ||
| birthday cipherstash.EncryptedDateOrd() // eq + range + ORDER BY | ||
| emailVerified cipherstash.EncryptedBoolean() // storage-only (no operators) | ||
| preferences cipherstash.EncryptedJson() // containment (@>) | ||
| } | ||
| ``` | ||
|
|
||
| | Column type | Domain | Query capability | | ||
| |---|---|---| | ||
| | `EncryptedTextSearch()` | `eql_v3_text_search` | equality, range, free-text, ORDER BY | | ||
| | `EncryptedDoubleOrd()` | `eql_v3_double_ord` | equality, range, ORDER BY | | ||
| | `EncryptedBigIntOrd()` | `eql_v3_bigint_ord` | equality, range, ORDER BY | | ||
| | `EncryptedDateOrd()` | `eql_v3_date_ord` | equality, range, ORDER BY | | ||
| | `EncryptedBoolean()` | `eql_v3_boolean` | storage-only (no operators) | | ||
| | `EncryptedJson()` | `eql_v3_json` | containment (`@>`) | |
There was a problem hiding this comment.
I just noticed that these type names don't actually match the EQLv3 names. The cipherstash namespace is sufficient to disambiguate.
// Do this
cipherstash.TextSearch()
// instead of
// cipherstash.EncryptedTextSearch()That lines up with the other adapters.
There was a problem hiding this comment.
Agreed, and it's inconsistent with the other adapters — I checked: Drizzle/stack use types.TextSearch(...) / types.DoubleOrd(...) (namespace types, no Encrypted prefix). So the type-name alignment is exactly your suggestion: drop Encrypted → cipherstash.TextSearch(), cipherstash.DoubleOrd(), etc.
One nuance on the namespace: in Prisma the cipherstash. prefix comes from how the extension registers into the PSL, not a free-choice TS namespace like Drizzle's types. So renaming the types (drop Encrypted) is straightforward; whether the namespace can become eql. (vs cipherstash.) is a prisma-next PSL-registration question worth confirming with CJ.
This is a @cipherstash/prisma-next package change (CJ's), not a skill fix — the skill documents today's shipped name. When the rename lands I'll update the skill in lockstep.
There was a problem hiding this comment.
Done in #685 (stacked on this PR): cipherstash.EncryptedTextSearch() → cipherstash.TextSearch() etc., at the single v3PascalName seam. Namespace stays cipherstash (it's the extension vendor identity — eql would rewrite every codec id; and Drizzle uses types., not eql.). Package builds clean, authoring/psl suites green.
|
|
||
| ```typescript | ||
| import 'dotenv/config' | ||
| import { cipherstashFromStackV3 } from '@cipherstash/prisma-next/v3' |
There was a problem hiding this comment.
IMHO the V3 is redundant and should be dropped from the name. V3 is the default/standard. If we want to keep supporting V2 then that can get the special name.
Similarly, it can probably be important from the root barrel.
import { cipherstashFromStack } from '@cipherstash/prisma-next'
There was a problem hiding this comment.
Makes sense — if v3 is the standard, cipherstashFromStackV3 → cipherstashFromStack from the root barrel (@cipherstash/prisma-next), with the v2 path taking the special name. Note it's a breaking change for existing v2 consumers of cipherstashFromStack (from /stack), so it wants a coordinated rename + changeset in the package. Package change, not a skill fix; skill tracks current naming and updates with it.
There was a problem hiding this comment.
Done in #685: cipherstashFromStackV3 → cipherstashFromStack; the v2 fn becomes cipherstashFromStackV2 (your pick). Root-barrel export follows from /v3 as before.
| The envelope for a `double` column is `EncryptedNumber` (JS `number`); the schema | ||
| column type is `EncryptedDoubleOrd`. Envelope ↔ column pairing: `EncryptedString` | ||
| ↔ `EncryptedTextSearch`, `EncryptedNumber` ↔ `EncryptedDoubleOrd`, | ||
| `EncryptedBigInt` ↔ `EncryptedBigIntOrd`, `EncryptedDate` ↔ `EncryptedDateOrd`, | ||
| `EncryptedBoolean` ↔ `EncryptedBoolean`, `EncryptedJson` ↔ `EncryptedJson`. | ||
|
|
There was a problem hiding this comment.
tbh I find it confusing that the envelope types don't match the EQL type names.
For example, you could have:
- Database column type:
eql_v3_text_search - Prisma schema type (current):
EncryptedTextSearch - Prisma runtime type (current):
EncryptedString
Can we just use the same names all the way through? All the Runtime types for the EQL text variants could be aliases of each other but at least they retain semantic meaning. In other words, I can reason about a type's capabilities based on the name (core EQL V3 principle).
There was a problem hiding this comment.
This is a real ergonomics call, and nothing here is unsafe — the design deliberately separates two axes:
- Runtime envelope (
EncryptedString.from(x)) = the plaintext shape. It's a write-side value wrapper; its job is to carry a value of the right JS type (string/number/…) and redact on serialization. It doesn't need capability info because writing doesn't involve operators. - Column type (
EncryptedTextSearch) = the capability. This is where search capability lives, and it's enforced at query time.
They don't line up 1:1 on purpose: multiple domains share a plaintext shape (text, text_eq, text_ord, text_search all take a string), so today they share one envelope (EncryptedString). Giving each domain its own envelope would be ~40 classes that mostly alias, to make the write path carry a distinction it never uses.
So 'same names all the way through' is a legitimate preference for readability, but the safety you'd get from it is already present on the column/operator layer (see my reply on line 190/198). Worth a design chat with CJ; not a correctness issue.
| | Operator | Meaning | Requires | | ||
| |---|---|---| | ||
| | `eqlEq(v)` / `eqlNeq(v)` | equality / inequality | any searchable domain | | ||
| | `eqlIn(vs)` / `eqlNotIn(vs)` | membership | any searchable domain | | ||
| | `eqlMatch(term)` | free-text token match (`eql_v3.contains`) | `EncryptedTextSearch` | | ||
| | `eqlGt/eqlGte/eqlLt/eqlLte(v)` | range comparison | an `*Ord` domain | | ||
| | `eqlBetween(lo,hi)` / `eqlNotBetween(lo,hi)` | range window | an `*Ord` domain | | ||
| | `eqlAsc()` / `eqlDesc()` | ORDER BY | an `*Ord` or `TextSearch` domain | | ||
| | `eqlJsonContains(obj)` | encrypted JSON containment (`@>`) | `EncryptedJson` | | ||
|
|
||
| ```typescript | ||
| // range | ||
| await db.orm.public.User.where((u) => u.salary.eqlGt(100_000)).all() | ||
| // free-text | ||
| await db.orm.public.User.where((u) => u.email.eqlMatch('example.com')).all() | ||
| // between | ||
| await db.orm.public.User.where((u) => u.birthday.eqlBetween(lo, hi)).all() | ||
| // bigint membership | ||
| await db.orm.public.User.where((u) => u.accountId.eqlIn([100_000_000_001n])).all() | ||
| // encrypted JSON containment | ||
| await db.orm.public.User.where((u) => u.preferences.eqlJsonContains({ theme: 'dark' })).all() |
There was a problem hiding this comment.
This is why I think using runtime types for each EQL variant would be better. Because you'd get type-checked capabilities. You should never be able to call eqlEq on an EncryptedString if it doesn't map to the eql_v3_text_search (and instead maps to eql_v3_text which is the non-searchable type).
IMHO we should name the runtime types the same as the schema types and only implement the operators for the types that support them.
There was a problem hiding this comment.
The capability type-checking you're describing already exists — but on the column expression, not the runtime envelope. You never call operators on EncryptedString; you call them on u.email (the column accessor from the Contract type), which is domain-specific and capability-typed.
Concretely: eqlEq is typed self: { traits: V3EqualityMarker }, eqlGt → V3OrderAndRangeMarker, eqlMatch → V3FreeTextSearchMarker (operation-types.ts), and each column's codec carries only the traits its capabilities grant (v3TraitsForCapabilities). A storage-only column carries no operator traits, so those methods don't exist on it → calling eqlEq on it is a compile error. That's exactly your 'never be able to call eqlEq on a non-searchable column' — it's already the case, just keyed off the column type rather than the envelope.
| Applying an operator its domain doesn't support (e.g. `eqlGt` on a | ||
| storage-only `EncryptedBoolean`, or `eqlMatch` on a non-text domain) is a typed | ||
| error at build time, not a runtime surprise. |
There was a problem hiding this comment.
My read of the code is that this wouldn't actually be possible at the moment. Maybe I'm missing something, but at the very least, we should have tests to cover it.
There was a problem hiding this comment.
It is possible today, and the tests you're asking for already exist — packages/prisma-next/test/operation-types.types.test-d.ts has @ts-expect-error assertions for exactly this: eqlEq rejected on storage-only eql_v3_boolean/eql_v3_text, eqlMatch rejected on eql_v3_text_eq, eqlGt rejected on eql_v3_double_eq. I ran tsc against them — every @ts-expect-error is satisfied (none 'unused'), so each really is a compile error. There's also a runtime guard suite (operator-gating-v3.test.ts) for defense in depth.
However, you're right to want it guaranteed: the package typecheck is currently red (~38 unrelated implicit-any errors) and isn't wired into any CI workflow — so those .test-d.ts assertions aren't enforced on every change. Filed #684 to green the typecheck and gate it in CI, which turns 'true today' into 'can't regress'.
…#683 - init does NOT scaffold wiring for Prisma Next (it skips the encryption-client scaffold — schema is derived from contract.json); say so instead of 'scaffolds the wiring'. (Copilot) - eqlAsc/eqlDesc are free functions taking the column expression (eqlAsc(u.salary)), not zero-arg methods; fix the ORDER BY table row to match the code example. (Copilot) - install-skills.ts: the skillsFor regression guard is a test asserting SKILL_MAP has an entry for every value in a maintained ALL_INTEGRATIONS list, not one that reads the provider registry; correct the comment. (Copilot) Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Prisma Next installs the EQL bundle through its own migration ledger
(migrations/cipherstash/ applied by `prisma-next migration apply`), so the
standalone installer is the wrong tool — running it applies EQL out-of-band
from the framework's ledger. `stash init --prisma-next` already skips the
installer; this closes the manual-invocation hole.
- New pure `prismaNextInstallGuard(cwd, { force })` (mirrors
validateInstallFlags): returns actionable guidance when a Prisma Next
project is detected and --force isn't set, else null. Called early in
installCommand, before any DB I/O — fails fast with a pointer to
`prisma-next migration apply`.
- --force overrides (deliberate standalone install escape hatch).
- messages.eql.prismaNextDetected as the stable leader.
- Tests: 4 unit (config-file detection, dep detection, --force override,
non-prisma-next passthrough) + 1 pty-less e2e proving the wiring (refuses,
exit 1, no DB). Skill gotcha + changeset updated.
Addresses CJ/Dan's #683 review ask to enforce this in the CLI.
Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
What
Companion to #655. #655 releases the
@cipherstash/prisma-nextEQL v3 package but touches no CLI code — so the CLI-side Prisma Next experience is still broken. This PR fixes it:SKILL_MAPis typedRecord<Integration, …>but omitted the'prisma-next'key.installSkillsand the AGENTS.md builder doSKILL_MAP[integration]→undefined→not iterablefor any repo the CLI detects as Prisma Next.tsccatches the missing key, but the build (tsup) transpiles without type-checking, so it shipped in rc.2 — reachable by any Prisma user runningstash init(Prisma Next is auto-detected viadetectPrismaNext).stash-prisma-nextskill documenting the EQL v3 surface: domain-named column types (EncryptedTextSearch,EncryptedDoubleOrd, …),cipherstashFromStackV3wiring, the runtime value envelopes, theeql*query operators, and EQL install viaprisma-next migration apply(notstash eql install). Authored from Feat/prisma next eql v3 #655's converted example app and package README, so it reflects the real v3 API.How
prisma-nextSKILL_MAPentry (stash-encryption,stash-prisma-next,stash-cli).skillsFor()helper that degrades an unmapped integration to the base skill set instead of crashing — belt-and-braces for the nextIntegrationvariant, sincetsupwon't type-check the map. (The real gap it papers over: the CLI package has notsc --noEmitgate in CI.)Tests
SKILL_MAPhas a non-empty entry for every integration (the runtime guardtsccan't be in this build).skillsFor()falls back for an unmapped integration.buildAgentsMdBody('prisma-next', 'doctrine-plus-skills')inlines the skill — the exact call that used to thrownot iterable.installSkills(…, 'prisma-next')copiesstash-prisma-next/SKILL.md(verified end-to-end).code:checkclean.Ordering
Stacked on
feat/prisma-next-eql-v3so the skill reflects the released v3 API; retarget tomainwhen #655 lands (GitHub does this automatically). The skill defersstash envflag detail to thestash-cliskill so it's correct regardless of #682's merge order.Closes the rc.2 M1 finding.
https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w