diff --git a/.changeset/precise-supabase-init-and-backdated-push-guidance.md b/.changeset/precise-supabase-init-and-backdated-push-guidance.md new file mode 100644 index 000000000..a194da85e --- /dev/null +++ b/.changeset/precise-supabase-init-and-backdated-push-guidance.md @@ -0,0 +1,7 @@ +--- +'stash': patch +--- + +Correct two inaccuracies in the bundled `stash-cli` skill. The `stash init` overview said the **Supabase** flow always generates an EQL migration; it now says **local Supabase**, matching `resolveMigrationRoute` — only a project with local `supabase/` CLI scaffolding takes the migration-first route, while a hosted Supabase project with no `supabase/` directory falls through to a direct `stash eql install`. And the guidance for back-dating the Supabase install migration no longer recommends `supabase db push --include-all` unconditionally: on a remote where `stash eql install` has already run, pushing the file re-runs a bundle that opens with `DROP SCHEMA IF EXISTS eql_v3 CASCADE`, dropping every index, constraint, and RLS policy on those schemas. That case is now `supabase migration repair --status applied ` (ledger only, no SQL) — after confirming EQL is genuinely installed on that remote with `psql "$REMOTE_DATABASE_URL" -Atc "select eql_v3.version()"`; `--include-all` stays for a remote that still needs the SQL applied. + +The same correction lands in the CLI itself, and in the two other places that repeated the old advice — the `stash-supabase` skill and the CLI README. `stash eql migration --supabase` warns when the project already has EQL-referencing migrations that sort before the install it is about to write, and that warning carried the identical blanket `--include-all` advice. Since this warning only fires on projects that ran `stash eql install` directly — so the remote usually already has the bundle and is missing only the ledger row — it now names `supabase migration repair --status applied ` as the remedy, spells out the `DROP SCHEMA IF EXISTS eql_v3 CASCADE` hazard of pushing the file instead, and keeps `--include-all` for the remote that has not had the SQL applied. diff --git a/.changeset/supabase-eql-migration-file.md b/.changeset/supabase-eql-migration-file.md new file mode 100644 index 000000000..75af4108c --- /dev/null +++ b/.changeset/supabase-eql-migration-file.md @@ -0,0 +1,33 @@ +--- +'stash': minor +--- + +Add `stash eql migration --supabase`, so an EQL v3 install survives `supabase db reset` (#613). + +Supabase projects previously had only `stash eql install --supabase`, which applies the SQL directly to a running database. `supabase db reset` — the ordinary local development loop — drops that database and replays `supabase/migrations/`, so the install was wiped and the next query failed with `type "eql_v3_encrypted" does not exist`. There was no supported way to get EQL into the migrations directory. + +`stash eql migration --supabase` now writes `supabase/migrations/_cipherstash_eql.sql`, carrying the EQL v3 bundle, the `anon` / `authenticated` / `service_role` grants, and the `cipherstash.cs_migrations` tracking schema — so one `supabase db reset` provisions everything `stash encrypt` needs. The file is timestamped at generation time, so it sorts after everything already applied and pushes without `--include-all`. A second run exits rather than adding a duplicate install; `--force` regenerates the existing one in place. + +The command now warns when the migrations directory already holds EQL-referencing migrations that sort *before* the install it is about to write. A project that ran `stash eql install` directly and then added `public.eql_v3_*` columns against the live database gets an install stamped today — after those migrations — and `supabase db reset`, which replays in version order with no dependency awareness, then fails with `type "eql_v3_text_search" does not exist`. The warning names the specific files and the remedy (rename the install below the earliest of them, then reconcile each remote — see below). It fires on `--dry-run` too, and nothing is renamed automatically — the ordering of someone else's deployed history is not ours to change silently. + +That warning's remote guidance now requires you to verify the remote before writing to its ledger. It splits by whether the remote already has EQL: one where it does needs only the ledger row (`supabase migration repair --status applied `, which runs no SQL — pushing the file instead re-runs a bundle opening with `DROP SCHEMA IF EXISTS eql_v3 CASCADE`), and one where it does not needs the SQL genuinely applied (`supabase db push --include-all`, the back-dated version being a gap in the middle of that history). Previously the first branch was recommended on an assumption the user was never asked to check, and it is the one operation here with no self-correcting failure: mark a version applied on a remote that never ran the SQL and EQL is permanently absent *and* permanently marked applied, so no future push installs it and the first migration referencing `eql_v3` fails with nothing pointing at the cause. The warning now prints the check first — `psql "$REMOTE_DATABASE_URL" -Atc "select eql_v3.version()"` — and says never to mark applied when it errors. It asks for `eql_v3.version()` rather than the `eql_v3` schema deliberately: that function is created by the bundle's closing statements, so it cannot resolve on an install that aborted partway, while the schema is created by its opening ones and survives one. The same correction lands in the `stash-cli` and `stash-supabase` skills and the CLI README, and a guard test now fails the build if a shipped skill recommends the ledger-only repair without that check above it. + +`--force`'s follow-up guidance was wrong and is now correct. It said to re-apply with `supabase db reset` (local) **or `supabase db push` (remote)**, but a push never re-applies a rewritten migration: the Supabase CLI decides what is pending by comparing versions, never file content, so an in-place rewrite keeping its version is skipped and push reports `Remote database is up to date.` The remote recipe is now `supabase migration repair --status reverted ` (tracking table only — it applies no SQL) followed by `supabase db push`, with `--include-all` called out as a conditional: it is needed only when migrations sort *after* the install, which leaves the reverted version as a gap in the middle of remote history. Reverting the newest version leaves it at the tail, where a plain push applies it — and the flag applies every out-of-order migration you have, so recommending it unconditionally was itself a hazard. The warning also names the hazard it never mentioned: the EQL bundle opens with `DROP SCHEMA IF EXISTS eql_v3 CASCADE`, so re-applying drops every index, constraint, and RLS policy that references `eql_v3` / `eql_v3_internal` — free on a fresh `db reset`, destructive on a populated remote. + +`--out` on a bare `--supabase` now warns. The Supabase CLI's migrations directory is not configurable — `supabase db reset` and `supabase db push` read `/supabase/migrations` and nothing else, `config.toml` has no key for it, and `--workdir` relocates the whole `supabase/` directory rather than this subdirectory — so an install written elsewhere is never applied, which is the original bug relocated. The flag still writes the file (a project may apply that directory through its own tooling) but names the consequence, on `--dry-run` too. `--out` alongside `--drizzle --supabase` is unaffected: there it is drizzle-kit's output directory. + +`--supabase` keeps its existing meaning alongside `--drizzle` (append the role grants to the Drizzle migration); only a bare `--supabase` selects the new emitter. + +`stash init --supabase` now generates that migration instead of installing directly, when the project has local `supabase/` scaffolding — a hosted project without it still installs directly. Re-running init over a project that already has an install migration reports it and moves on, rather than treating the duplicate refusal as a failed setup. Its next steps no longer tell you to run `eql install --supabase` and then `supabase db reset`, which was the exact sequence that destroyed the install. + +`stash init`'s EQL summary line now distinguishes the migration it wrote from one it merely found. A re-run over an existing install migration says "EQL migration **already present**" instead of "EQL migration generated" — same apply guidance, same successful exit, but no claim about work the run did not do. + +`stash init`'s EQL prompt now names the action for the route it is actually on. On the migration-first routes it asks whether to generate a migration (naming `supabase/migrations/` or your Drizzle migrations folder) rather than whether to install into your database, which described the wrong action on both. Declining is fixed the same way: the retry hint is now `stash eql migration --supabase` / `--drizzle` on those routes instead of `stash eql install`, which on Supabase would reinstate the very bug above. + +`stash init` now routes on the integration flags themselves rather than on the provider's display name, so combining them works. `stash init --drizzle --supabase` is accepted — and is the natural invocation for a Drizzle project on Supabase — but init joined the matched flags into a single provider name (`drizzle-supabase`) for referrer tracking and then compared that name against `'drizzle'` and `'supabase'` everywhere it had a decision to make. Every comparison went false. A local Supabase stack answers on `127.0.0.1:54322`, so host detection reports plain Postgres and the flags are the only signal left: the run installed EQL directly instead of writing a migration — nothing in `supabase/migrations/`, no `anon` / `authenticated` / `service_role` grants — which is the #613 failure this release exists to fix, reached through a flag combination the CLI accepts. The same fall-through dropped the `supabase status` hint when resolving `DATABASE_URL` (the one lookup that finds a local stack's URL), lost the Prisma Next branch for `--prisma --supabase` — scaffolding a client Prisma Next never uses and running a duplicate EQL install that races `prisma-next migrate`'s journal — and installed no integration adapter at all, where a combined run needs both `@cipherstash/stack-drizzle` and `@cipherstash/stack-supabase`. The provider now carries the matched flags alongside its name and every step reads those; the combined name is still exactly what gets recorded as the referrer, it is simply no longer what the CLI branches on. Drizzle still wins the migration route when both flags fire — it owns the migration history, and `--supabase` is the grants modifier there. Single-flag runs behave exactly as before. + +Also corrects the remote apply command across the Supabase guidance: a bare `supabase migration up` targets the local database, so the instructions now say `supabase db push`. + +Also corrects the `eql install --migration` removal message, which pointed every Supabase user at `--drizzle`. + +The Supabase CLI behaviour all of the above depends on is now pinned by a live test rather than by reading the CLI's source. `supabase-push.live.test.ts` drives the real binary against a real Postgres — `db push --db-url` needs neither Docker nor a linked project — and covers: the generated install applying with no `--include-all`; `anon` reaching `eql_v3` via `SET ROLE` through the grants carried in the emitted file (not just the ones `eql install --direct` applies); an out-of-order version aborting the whole push rather than being skipped; a `--force`-replaced file never re-applying; `--include-all` being needed only for the gap case; and a leaked `.tmp` file being ignored. Gated on `STASH_TEST_SUPABASE_DB_URL` + `STASH_TEST_SUPABASE_CLI`, so the default suite is unchanged. diff --git a/e2e/tests/package-managers.e2e.test.ts b/e2e/tests/package-managers.e2e.test.ts index 498f929cf..830fcf753 100644 --- a/e2e/tests/package-managers.e2e.test.ts +++ b/e2e/tests/package-managers.e2e.test.ts @@ -61,7 +61,7 @@ describe('CLI init providers — package-manager-aware Next Steps', () => { label: 'supabase', create: createSupabaseProvider, firstStep: (r) => - `Install EQL: ${r} stash eql install --supabase (prompts for migration vs direct)`, + `Install EQL: ${r} stash eql migration --supabase (writes it into supabase/migrations/)`, }, ] diff --git a/packages/cli/README.md b/packages/cli/README.md index a34d07111..2d7be7455 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -283,9 +283,11 @@ Reads `databaseUrl` from `stash.config.ts`. --- -## Drizzle migration mode +## Migration mode -Use `eql migration --drizzle` to add EQL v3 installation to Drizzle migration history instead of applying it directly. +Use `eql migration` to add the EQL v3 installation to your migration history instead of applying it directly. The install then ships to every environment through the same migrate step as the rest of your schema. + +### Drizzle ```bash npx stash eql migration --drizzle @@ -306,6 +308,49 @@ npx drizzle-kit migrate `drizzle-kit` must be installed in your project (`npm install -D drizzle-kit`). The `--out` directory must match your `drizzle.config.ts`. +Add `--supabase` on a Supabase-hosted Drizzle project to append the `anon` / `authenticated` / `service_role` grants. + +### Supabase + +```bash +npx stash eql migration --supabase +supabase db reset # local +supabase db push # remote/linked project +``` + +This writes `supabase/migrations/_cipherstash_eql.sql` containing the EQL v3 bundle, the Supabase role grants, and the `cipherstash.cs_migrations` tracking schema — so one reset provisions everything `stash encrypt` needs. + +**Use this rather than `eql install --supabase` whenever the project has a local `supabase/` directory.** A direct install does not survive `supabase db reset`, which drops the database and replays the migrations directory. + +The file is timestamped at generation time, so it sorts after everything already applied and pushes with no extra flag. An out-of-order version is not merely skipped — `supabase db push` aborts the whole push with `Found local migration files to be inserted before the last migration on remote database.` and applies nothing until you re-run with `--include-all`. + +If the project already has migrations that reference EQL (an `eql_v3_*` column added back when `eql install` was applied directly), those now sort *before* the install. `supabase db reset` replays in version order with no dependency awareness, so they run first and the reset fails with `type "eql_v3_text_search" does not exist`. The command warns and names them; rename the install migration to a version below the earliest of them so it replays first. + +How that back-dated version reaches a remote depends on what that remote actually has, so check before touching the ledger: + +```bash +psql "$REMOTE_DATABASE_URL" -Atc "select eql_v3.version()" +``` + +`eql_v3.version()` is created by the bundle's last statements, so it answers "is the whole install there" — a probe for the `eql_v3` schema does not, since that schema is created by the bundle's first statements and survives an install that aborted partway. + +If it prints a version, EQL is present and only the ledger row is missing — mark it applied with `supabase migration repair --status applied `, which writes the row and runs no SQL. Do not push the file there instead: that re-runs a bundle opening with `DROP SCHEMA IF EXISTS eql_v3 CASCADE`, dropping every index, constraint, and RLS policy that references those schemas. + +If it errors, that remote genuinely still needs the SQL applied: `supabase db push --include-all`. Never mark it applied there — the ledger row would claim SQL that never ran, so no later push installs EQL, and the first migration referencing `eql_v3` fails with nothing pointing at the cause. + +Pass `--force` to regenerate an existing install migration in place. It keeps its version, so `supabase db push` will **not** re-apply it — pending migrations are decided by version, never by file content, and push reports `Remote database is up to date.` Use `supabase db reset` locally, or on a remote: + +```bash +supabase migration repair --status reverted # clear the ledger row (applies no SQL) +supabase db push # re-apply +``` + +Add `--include-all` to that push only if it aborts with `Found local migration files to be inserted before the last migration on remote database.` — that happens when migrations sort after the install, leaving the reverted version as a gap in the middle of history. Reverting the newest version leaves it at the tail, which a plain push applies. The flag applies every out-of-order migration you have, so don't pass it pre-emptively. + +Weigh that before doing it to a populated database: the EQL bundle opens with `DROP SCHEMA IF EXISTS eql_v3 CASCADE` (and `eql_v3_internal`), so re-applying also drops every index, constraint, and RLS policy that references those schemas. + +Don't pass `--out` here. The Supabase CLI reads `/supabase/migrations` and nothing else — the path is not configurable in `config.toml`, and `--workdir` moves the whole `supabase/` directory, not this one. An install written elsewhere is never applied by `supabase db reset` / `db push`, which is the failure this command exists to avoid. The flag still works (and warns) for projects that apply another directory through their own tooling. + --- ### `npx stash eql repair --drizzle` diff --git a/packages/cli/src/__tests__/skill-supabase-apply.test.ts b/packages/cli/src/__tests__/skill-supabase-apply.test.ts new file mode 100644 index 000000000..68f79fac9 --- /dev/null +++ b/packages/cli/src/__tests__/skill-supabase-apply.test.ts @@ -0,0 +1,103 @@ +import { readdirSync, readFileSync } from 'node:fs' +import { dirname, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +const CLI_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '../..') +const REPO_ROOT = resolve(CLI_ROOT, '../..') +const SKILLS_ROOT = resolve(REPO_ROOT, 'skills') + +/** + * `supabase migration up` applies to the **local** database. The remote forms + * are `supabase db push` and `supabase migration up --linked`. + * + * Skills ship inside the `stash` tarball and are copied into customer repos, so + * naming the local command as the remote one is not a typo — it means a user + * follows the instructions, believes production has EQL, and every encrypted + * query there fails at runtime. Nothing else checks these files, which is why + * this guard exists (same reasoning as the version-pin guard in + * `release-train.test.ts`). + * + * The rule: any `supabase migration up` in a shipped skill must either carry + * `--linked` or be immediately qualified as the local command ("… applies to + * the local database"). A nearby "locally" is not enough — the wording this + * guard exists to catch, "apply with `supabase migration up` (or `supabase db + * reset` locally)", has one, attached to the other command. + */ +const SKILL_FILES = readdirSync(SKILLS_ROOT, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => ({ + skill: entry.name, + body: readFileSync(resolve(SKILLS_ROOT, entry.name, 'SKILL.md'), 'utf8'), + })) + +describe('skills — Supabase apply commands', () => { + it('finds skills to check (a moved directory must not silently pass)', () => { + expect(SKILL_FILES.length).toBeGreaterThan(0) + }) + + it.each( + SKILL_FILES, + )('$skill never presents a bare `supabase migration up` as the remote apply', ({ + body, + }) => { + // Collapse wrapping and drop markdown emphasis first: the qualifier + // routinely lands on the next source line or arrives as `**local**`, and + // either would fail the match on formatting rather than content. + const prose = body.replace(/\s+/g, ' ').replace(/[*`_]/g, '') + + for (const match of prose.matchAll(/supabase migration up/g)) { + // Only what immediately follows the command counts. A window wide + // enough to find a "local database" elsewhere in the sentence accepts + // the very wording this guard rejects — "apply with supabase migration + // up (or supabase db reset locally, once the local database exists)" + // qualifies the other command, not this one. + const following = prose.slice(match.index + match[0].length) + + expect( + following.slice(0, 60), + '`supabase migration up` applies to the LOCAL database — add `--linked`, say "applies to the local database", or use `supabase db push` for remote', + ).toMatch(/^(?: --linked\b| applies to the local database\b)/i) + } + }) + + /** + * `supabase migration repair --status applied ` writes a ledger row + * and runs no SQL. That is the right move for a back-dated install on a + * remote that already HAS EQL — pushing the file there re-runs a bundle + * opening with `DROP SCHEMA IF EXISTS eql_v3 CASCADE`. It is unrecoverable on + * a remote that does not: the row asserts SQL ran that never did, so no later + * push ever installs EQL, and the first `eql_v3` reference fails with nothing + * pointing at the cause. Every other remedy in this area fails loudly and can + * be retried; this one fails silently and cannot. + * + * So the rule: wherever a shipped skill recommends the ledger-only repair, a + * command that establishes the remote's actual EQL state must appear shortly + * BEFORE it. Before, because a check printed after the repair verifies + * nothing — the row is already written. `eql_v3.version()` specifically, + * because it is created by the bundle's closing statements and so cannot + * resolve on a half-applied install, unlike the `eql_v3` schema itself. + */ + it.each( + SKILL_FILES, + )('$skill never recommends the ledger-only repair without a check above it', ({ + body, + }) => { + // Same wrap-collapsing as above, minus the `_` strip: the marker here is + // `eql_v3.version()`, which that strip would turn into `eqlv3.version()`. + const prose = body.replace(/\s+/g, ' ').replace(/[*`]/g, '') + + for (const match of prose.matchAll(/migration repair --status applied/g)) { + // A paragraph's worth of lead-in. Wide enough for the sentence that + // introduces the check plus the one that explains what its output means, + // narrow enough that an `eql_v3.version()` mention elsewhere in the + // document cannot stand in for one attached to this recommendation. + const preceding = prose.slice(Math.max(0, match.index - 700), match.index) + + expect( + preceding, + '`migration repair --status applied` writes a ledger row for SQL that may never have run — an unrecoverable state on a remote without EQL. Print the `select eql_v3.version()` check above this recommendation, not after it', + ).toContain('eql_v3.version()') + } + }) +}) diff --git a/packages/cli/src/bin/main.ts b/packages/cli/src/bin/main.ts index 16f26f639..465f7d620 100644 --- a/packages/cli/src/bin/main.ts +++ b/packages/cli/src/bin/main.ts @@ -109,7 +109,7 @@ Commands: telemetry Manage anonymous usage analytics (status, enable, disable) eql install Scaffold stash.config.ts (if missing) and install EQL extensions - eql migration Generate an EQL v3 install migration for your ORM (Drizzle) + eql migration Generate an EQL v3 install migration (Drizzle, or supabase/migrations/) eql repair Repair migrations with an un-runnable ALTER COLUMN to an encrypted type eql upgrade Upgrade EQL extensions to the latest version eql status Show EQL installation status @@ -261,6 +261,7 @@ async function runEqlCommand( supabase: flags.supabase, name: values.name, out: values.out, + force: flags.force, dryRun: flags['dry-run'], }) break diff --git a/packages/cli/src/cli/registry.ts b/packages/cli/src/cli/registry.ts index 4496cda3e..3233b76e3 100644 --- a/packages/cli/src/cli/registry.ts +++ b/packages/cli/src/cli/registry.ts @@ -325,10 +325,24 @@ export const registry: CommandGroup[] = [ { name: 'eql migration', summary: - 'Generate an EQL v3 install migration for your ORM (Drizzle; Prisma Next installs EQL through its own migrations)', + 'Generate an EQL v3 install migration (Drizzle, or supabase/migrations/; Prisma Next installs EQL through its own migrations)', + long: [ + 'Migration-first is the preferred way to install EQL: it lands in your', + 'migration history and ships to every environment through the same', + 'migrate step as the rest of your schema. On Supabase it is the only', + 'durable way — `supabase db reset` replays the migrations directory, so', + 'a direct `eql install` is wiped by the next reset.', + ].join('\n'), examples: [ 'eql migration --drizzle', 'eql migration --drizzle --supabase', + 'eql migration --supabase', + // Deliberately no `--out` here. The Supabase CLI reads + // `supabase/migrations` and nothing else, so an example pointing the + // install elsewhere would teach the exact "EQL isn't in the replayed + // directory" failure this command exists to fix. `--force` is what + // needed demonstrating; it works fine on its own. + 'eql migration --supabase --force', ], flags: [ { @@ -344,7 +358,7 @@ export const registry: CommandGroup[] = [ { name: '--supabase', description: - 'Append the Supabase role grants (eql_v3 + eql_v3_internal for anon/authenticated/service_role).', + 'On its own, write the install into supabase/migrations/ so it survives `supabase db reset`. With --drizzle, instead append the Supabase role grants (eql_v3 + eql_v3_internal for anon/authenticated/service_role) to the Drizzle migration.', }, { name: '--name', @@ -356,7 +370,12 @@ export const registry: CommandGroup[] = [ name: '--out', value: '', description: - 'Directory drizzle-kit writes the migration into (passed to `drizzle-kit generate --out`). Defaults to `drizzle`; set it to match your drizzle.config.ts.', + 'Where the migration is written. Drizzle: passed to `drizzle-kit generate --out`, defaults to `drizzle` — set it to match your drizzle.config.ts. Supabase: leave it alone. The Supabase CLI replays `supabase/migrations` and has no setting to move it, so pointing elsewhere means `supabase db reset` / `db push` never apply the install; the command warns when you do.', + }, + { + name: '--force', + description: + 'Write a Supabase install migration even though one already exists. Not needed for --drizzle (drizzle-kit numbers each generated migration).', }, DRY_RUN_FLAG, ], diff --git a/packages/cli/src/commands/db/detect.ts b/packages/cli/src/commands/db/detect.ts index 3684c5ca5..c8d014aea 100644 --- a/packages/cli/src/commands/db/detect.ts +++ b/packages/cli/src/commands/db/detect.ts @@ -33,19 +33,24 @@ export function detectSupabase(databaseUrl: string | undefined): boolean { */ export interface SupabaseProjectInfo { /** - * Whether the migrations directory exists AND is a directory. Used to pick - * the migration-vs-direct default in the `eql install --supabase` prompt. + * Whether the migrations directory exists AND is a directory. Together with + * {@link hasConfigToml} this is what `stash init --supabase` reads to decide + * whether the project has somewhere local to write an install migration — + * a hosted Supabase project with no CLI scaffolding does not, and falls back + * to a direct `eql install`. */ hasMigrationsDir: boolean /** - * Whether `supabase/config.toml` exists. Informational only — it doesn't - * influence the prompt default but is useful for diagnostics. + * Whether `supabase/config.toml` exists. The stronger of the two signals: a + * project that has run `supabase init` but never written a migration has the + * config and no migrations directory. Also gates the `supabase status` + * fallback in the database-URL resolver. */ hasConfigToml: boolean /** - * Absolute path to the migrations directory we'd write into. Defaults to + * Absolute path to the migrations directory to write into. Defaults to * `/supabase/migrations`, or `override` (resolved against `cwd` when - * relative) when supplied via `--migrations-dir`. + * relative) when supplied via `eql migration --supabase --out`. */ migrationsDir: string } @@ -53,13 +58,16 @@ export interface SupabaseProjectInfo { /** * Inspect the working directory for Supabase CLI scaffolding. * - * IMPORTANT: this is a hint for choosing the install-mode prompt default — - * it does NOT enable `--supabase`. The user must pass `--supabase` explicitly - * for any of the migration-file flow to activate. + * IMPORTANT: this is a hint — it does NOT enable `--supabase`. The user must + * pass `--supabase` explicitly for the migration-file flow to activate + * (`stash init --supabase` counts as that explicit choice). + * + * `migrationsDir` is returned whether or not it exists, because it is also the + * path `eql migration --supabase` creates. * * @param cwd - Project root to inspect. - * @param override - Optional `--migrations-dir` override. Absolute paths are - * used as-is; relative paths are resolved against `cwd`. + * @param override - Optional `--out` override. Absolute paths are used as-is; + * relative paths are resolved against `cwd`. */ export function detectSupabaseProject( cwd: string, diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index edfd1f196..f79509eaa 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -41,7 +41,7 @@ export function validateInstallFlags( return '`eql install --drizzle` has been removed. Generate an EQL v3 Drizzle migration with `stash eql migration --drizzle` (and pass --name/--out there).' } if (options.migration === true || options.migrationsDir !== undefined) { - return '`eql install --migration` has been removed. Use `stash eql migration --drizzle` to keep the EQL v3 install in migration history, adding `--supabase` when needed.' + return '`eql install --migration` has been removed. Use `stash eql migration` to keep the EQL v3 install in migration history: `--supabase` writes into supabase/migrations/, `--drizzle` emits a Drizzle migration (add `--supabase` there for the role grants). Pass the target directory as `--out`.' } if (options.direct === true) { return '`--direct` has been removed because `stash eql install` is now always a direct EQL v3 install.' diff --git a/packages/cli/src/commands/eql/__tests__/migration.test.ts b/packages/cli/src/commands/eql/__tests__/migration.test.ts index 6d1328f7a..faf76b318 100644 --- a/packages/cli/src/commands/eql/__tests__/migration.test.ts +++ b/packages/cli/src/commands/eql/__tests__/migration.test.ts @@ -2,12 +2,13 @@ import { existsSync, mkdirSync, mkdtempSync, + readdirSync, readFileSync, rmSync, writeFileSync, } from 'node:fs' import { tmpdir } from 'node:os' -import { join, resolve } from 'node:path' +import { join, resolve, sep } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { CliExit } from '../../../cli/exit.js' import { messages } from '../../../messages.js' @@ -166,17 +167,609 @@ describe('eqlMigrationCommand — target selection', () => { () => messages.eql.migrationOneTarget, ], ['--prisma', { prisma: true }, () => messages.eql.migrationPrismaNotNeeded], - // `--supabase` is a modifier, not a target. - [ - '--supabase alone', - { supabase: true }, - () => messages.eql.migrationNeedsTarget, - ], ])('exits 1 with an actionable message for %s', async (_label, opts, msg) => { await expect(eqlMigrationCommand(opts)).rejects.toBeInstanceOf(CliExit) expect(clack.log.error).toHaveBeenCalledWith(msg()) expect(spawnMock).not.toHaveBeenCalled() }) + + /** + * `--prisma --supabase` is not caught by the `drizzle && prisma` + * mutual-exclusion check — the only thing standing between it and the + * Supabase emitter is BRANCH ORDERING: the `--prisma` rejection sits above + * the `--supabase` dispatch, so the command exits before + * `generateSupabaseEqlMigration` runs. That guard is invisible in the source + * and a future reorder would silently route this invocation into the + * emitter, so pin it here. + * + * `expect(spawnMock).not.toHaveBeenCalled()` (the assertion the sibling cases + * use) cannot detect that regression: the Supabase emitter never spawns + * anything, it writes files directly. So stub `process.cwd` at a fresh + * tmpdir the way the `--out` suite below does and assert the directory is + * untouched — the emitter would create `supabase/migrations/` under it. + */ + it('rejects `--prisma --supabase` before the Supabase emitter writes anything', async () => { + const tmp = mkdtempSync(join(tmpdir(), 'stash-eql-prisma-supabase-')) + const cwd = vi.spyOn(process, 'cwd').mockReturnValue(tmp) + try { + await expect( + eqlMigrationCommand({ prisma: true, supabase: true }), + ).rejects.toBeInstanceOf(CliExit) + + expect(clack.log.error).toHaveBeenCalledWith( + messages.eql.migrationPrismaNotNeeded, + ) + // Nothing written, nothing created, no emitter side effects. + expect(readdirSync(tmp)).toHaveLength(0) + expect(existsSync(join(tmp, 'supabase', 'migrations'))).toBe(false) + expect(clack.log.success).not.toHaveBeenCalled() + expect(spawnMock).not.toHaveBeenCalled() + } finally { + cwd.mockRestore() + rmSync(tmp, { recursive: true, force: true }) + } + }) + + it('treats `--drizzle --supabase` as one target, not two', async () => { + // `--supabase` is the grants modifier here, not a second target. Counting + // it as one would reject the documented Supabase-hosted-Drizzle invocation. + const tmp = mkdtempSync(join(tmpdir(), 'stash-eql-targets-')) + try { + await eqlMigrationCommand({ + drizzle: true, + supabase: true, + out: tmp, + dryRun: true, + }) + } finally { + rmSync(tmp, { recursive: true, force: true }) + } + expect(clack.log.error).not.toHaveBeenCalled() + }) +}) + +/** + * The Supabase emitter. No drizzle-kit, no journal, no ALTER COLUMN sweep — + * just the install SQL written where `supabase db reset` will replay it. + */ +describe('eqlMigrationCommand — Supabase', () => { + let tmp: string + beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), 'stash-eql-supabase-')) + }) + afterEach(() => { + rmSync(tmp, { recursive: true, force: true }) + }) + + it('writes the install into --out and never spawns drizzle-kit', async () => { + await eqlMigrationCommand({ supabase: true, out: tmp }) + + const written = readdirSync(tmp) + expect(written).toHaveLength(1) + expect(written[0]).toMatch(/^\d{14}_cipherstash_eql\.sql$/) + expect(spawnMock).not.toHaveBeenCalled() + }) + + it('always includes the role grants — a Supabase file is applied by Supabase', async () => { + await eqlMigrationCommand({ supabase: true, out: tmp }) + + const body = readFileSync(join(tmp, readdirSync(tmp)[0]), 'utf-8') + expect(body).toContain( + 'GRANT USAGE ON SCHEMA eql_v3 TO anon, authenticated, service_role', + ) + expect(body).toContain( + 'GRANT USAGE ON SCHEMA eql_v3_internal TO anon, authenticated, service_role', + ) + // One reset provisions everything `stash encrypt` needs. + expect(body).toContain('cs_migrations') + }) + + it('dry run previews the directory and writes nothing', async () => { + await eqlMigrationCommand({ supabase: true, out: tmp, dryRun: true }) + + expect(readdirSync(tmp)).toHaveLength(0) + expect(clack.note).toHaveBeenCalledWith( + expect.stringContaining(tmp), + 'Dry Run', + ) + }) + + it('dry run predicts the refusal when an install migration already exists', async () => { + // Regression: the preview always claimed it "would write" a new file, even + // in a directory where the real run exits 1. A dry run that predicts the + // wrong outcome is worse than no dry run. + writeFileSync(join(tmp, '20260101000000_cipherstash_eql.sql'), '') + + await eqlMigrationCommand({ supabase: true, out: tmp, dryRun: true }) + + const [note] = vi.mocked(clack.note).mock.calls.at(-1) ?? [] + expect(note).toContain('20260101000000_cipherstash_eql.sql') + expect(note).toMatch(/--force/) + expect(note).not.toMatch(/Would write/i) + }) + + it('dry run predicts the in-place overwrite under --force', async () => { + writeFileSync(join(tmp, '20260101000000_cipherstash_eql.sql'), '') + + await eqlMigrationCommand({ + supabase: true, + out: tmp, + dryRun: true, + force: true, + }) + + const [note] = vi.mocked(clack.note).mock.calls.at(-1) ?? [] + expect(note).toContain('20260101000000_cipherstash_eql.sql') + expect(note).toMatch(/replace/i) + }) + + it('exits 1 rather than adding a second install migration', async () => { + await eqlMigrationCommand({ supabase: true, out: tmp }) + await expect( + eqlMigrationCommand({ supabase: true, out: tmp }), + ).rejects.toBeInstanceOf(CliExit) + + expect(readdirSync(tmp)).toHaveLength(1) + expect(clack.log.error).toHaveBeenCalledWith( + expect.stringContaining('already exists'), + ) + }) + + it('replaces in place under --force and warns about applied databases', async () => { + await eqlMigrationCommand({ supabase: true, out: tmp }) + const original = readdirSync(tmp)[0] + + await eqlMigrationCommand({ supabase: true, out: tmp, force: true }) + + expect(readdirSync(tmp)).toEqual([original]) + // The warning must name the hazard, not just note the replacement: a + // database that already ran the old file is the whole point of saying + // anything. + const [warning] = vi.mocked(clack.log.warn).mock.calls.at(-1) ?? [] + expect(warning).toMatch(/already applied/) + }) + + /** + * The re-apply guidance after an in-place overwrite. `supabase db push` does + * NOT re-apply a rewritten file: `FindPendingMigrations` + * (`pkg/migration/apply.go`) computes the pending set positionally — + * `pending := localMigrations[len(remoteMigrations):]` — with no content hash + * and no statement diff, unlike seed files, which carry a `Hash`/`Dirty` pair + * and do re-run on change. Equal counts mean an empty pending set, so + * `push.Run` prints "Remote database is up to date." and applies nothing. A + * user following "re-apply with db push" would believe the remote was updated + * when it was not. + */ + describe('--force re-apply guidance', () => { + const replaceInPlace = async () => { + await eqlMigrationCommand({ supabase: true, out: tmp }) + const version = readdirSync(tmp)[0].slice(0, 14) + vi.clearAllMocks() + await eqlMigrationCommand({ supabase: true, out: tmp, force: true }) + return version + } + const lastWarning = () => + String(vi.mocked(clack.log.warn).mock.calls.at(-1)?.[0] ?? '') + const lastNote = () => + String(vi.mocked(clack.note).mock.calls.at(-1)?.[0] ?? '') + + it('says db push will not re-apply the replaced file', async () => { + await replaceInPlace() + + expect(lastWarning()).toMatch(/`supabase db push` will not re-apply/) + // The reason, not just the verdict — otherwise it reads as a bug report. + expect(lastWarning()).toMatch(/by version, not by content/) + }) + + it('names the cascade hazard of re-applying to a live database', async () => { + // The bundle opens with `DROP SCHEMA IF EXISTS eql_v3 CASCADE;` / + // `DROP SCHEMA IF EXISTS eql_v3_internal CASCADE;`, so a re-apply takes + // every dependent index, constraint, and RLS policy with it. Free on a + // fresh `db reset`; not on a populated remote. + await replaceInPlace() + + expect(lastWarning()).toContain('DROP SCHEMA IF EXISTS eql_v3 CASCADE') + expect(lastWarning()).toMatch(/RLS polic/) + }) + + it('gives the repair-then-push remote recipe, with --include-all as a conditional', async () => { + const version = await replaceInPlace() + + // `migration repair --status reverted` clears the ledger row (tracking + // table only — it applies no SQL), which puts the version back in the + // pending set. + expect(lastNote()).toContain( + `supabase migration repair --status reverted ${version}`, + ) + expect(lastNote()).toContain('supabase db reset') + + // The plain push is the instruction; --include-all is the fallback for + // when it aborts. Reverting the NEWEST version leaves it at the tail of + // remote history, where a plain push applies it — only a version with + // migrations above it is the gap that trips ErrMissingRemote. Pinned live + // in supabase-push.live.test.ts against supabase/cli 2.111.0; an earlier + // revision of this message demanded the flag unconditionally, which + // applies every out-of-order migration the user has. + const note = lastNote() + expect(note).toContain('supabase db push\n') + expect(note).toMatch(/re-run it as `supabase db push --include-all`/) + expect(note).toMatch(/only when the push tells you to/i) + }) + + it('keeps the plain apply note when nothing was replaced', async () => { + await eqlMigrationCommand({ supabase: true, out: tmp }) + + expect(lastNote()).toContain('supabase db push') + expect(lastNote()).not.toContain('migration repair') + }) + }) + + /** + * Brownfield ordering (#613's second act). A user who ran `stash eql install` + * directly, then added migrations creating encrypted columns, then found this + * command, gets an install stamped with today's date — which sorts AFTER those + * migrations. `supabase db reset` replays in version order with no dependency + * awareness, so they run first, reference a domain that does not exist yet, + * and the reset fails. + * + * Detection and a warning, deliberately not a fix: back-dating the file or + * renaming theirs are both the user's call. + */ + describe('EQL-dependent migrations that sort before the install', () => { + const warnings = () => + clack.log.warn.mock.calls.map((c) => String(c[0])).join('\n') + /** Distinctive enough to assert absence on. */ + const FRAGMENT = 'replays the directory in version order' + const EARLIER = '20260101000000_add_email_encrypted.sql' + const ENCRYPTED_COLUMN_SQL = + 'ALTER TABLE users ADD COLUMN email_encrypted public.eql_v3_text_search;\n' + + it('warns, naming the file and the consequence', async () => { + writeFileSync(join(tmp, EARLIER), ENCRYPTED_COLUMN_SQL) + + await eqlMigrationCommand({ supabase: true, out: tmp }) + + expect(clack.log.warn).toHaveBeenCalledWith( + messages.eql.migrationSupabaseEqlBeforeInstall(tmp, [EARLIER]), + ) + expect(warnings()).toContain(EARLIER) + expect(warnings()).toContain('supabase db reset') + // The remedy, including the flag a back-dated push needs. + expect(warnings()).toContain('--include-all') + }) + + it('splits the remote remedy by whether EQL is already installed there', async () => { + // The brownfield case this warning fires on is, by definition, a project + // that ran `stash eql install` directly — so the remote usually HAS EQL + // and is missing only the ledger row. Sending that user to `db push + // --include-all` re-runs a bundle opening with `DROP SCHEMA IF EXISTS + // eql_v3 CASCADE`, taking every dependent index, constraint, and RLS + // policy with it. The ledger-only repair must be the named default, with + // --include-all kept for a remote that genuinely lacks the SQL. + writeFileSync(join(tmp, EARLIER), ENCRYPTED_COLUMN_SQL) + + await eqlMigrationCommand({ supabase: true, out: tmp }) + + expect(warnings()).toContain('supabase migration repair --status applied') + expect(warnings()).toContain('DROP SCHEMA IF EXISTS eql_v3 CASCADE') + expect(warnings()).toMatch(/RLS polic/) + // Both halves present, and the destructive one is the conditional. + expect(warnings()).toContain('--include-all') + }) + + it('makes the remote state a check to run, ahead of the ledger repair', async () => { + // Which half of that split applies turns on a fact the user is otherwise + // never asked to establish. Marking a version applied is the one remedy + // here with no self-correcting failure: get it wrong and the ledger + // claims SQL ran that never did, so no later push installs EQL and the + // first `eql_v3` reference fails with nothing pointing at the cause. The + // check therefore has to be a printed command, above the repair. + writeFileSync(join(tmp, EARLIER), ENCRYPTED_COLUMN_SQL) + + await eqlMigrationCommand({ supabase: true, out: tmp }) + + const warning = warnings() + expect(warning).toContain( + 'psql "$REMOTE_DATABASE_URL" -Atc "select eql_v3.version()"', + ) + expect(warning.indexOf('select eql_v3.version()')).toBeLessThan( + warning.indexOf('supabase migration repair --status applied'), + ) + }) + + it('checks a bundle-final object, which a half-applied install lacks', async () => { + // `eql_v3.version()` is created by the bundle's closing statements, so it + // is present only if the whole install ran. A probe for the `eql_v3` + // schema would pass on an install that aborted halfway — the schema is + // created by the bundle's opening statements — and "partially installed" + // read as "installed" is exactly the state the ledger row must not be + // written for. + writeFileSync(join(tmp, EARLIER), ENCRYPTED_COLUMN_SQL) + + await eqlMigrationCommand({ supabase: true, out: tmp }) + + expect(warnings()).toMatch(/last statements of the bundle/) + // The unrecoverable direction is named, not left as an inference. + expect(warnings()).toMatch(/[Nn]ever mark it applied/) + }) + + it('stays quiet when the EQL-referencing migration sorts after the install', async () => { + writeFileSync( + join(tmp, '20990101000000_add_email_encrypted.sql'), + ENCRYPTED_COLUMN_SQL, + ) + + await eqlMigrationCommand({ supabase: true, out: tmp }) + + expect(warnings()).not.toContain(FRAGMENT) + }) + + it('never warns for an earlier migration that does not reference EQL', async () => { + writeFileSync( + join(tmp, '20260101000000_users.sql'), + 'CREATE TABLE users (id uuid PRIMARY KEY, email text);\n', + ) + + await eqlMigrationCommand({ supabase: true, out: tmp }) + + expect(warnings()).not.toContain(FRAGMENT) + }) + + it('warns on a dry run, which is where the prediction is still free', async () => { + writeFileSync(join(tmp, EARLIER), ENCRYPTED_COLUMN_SQL) + + await eqlMigrationCommand({ supabase: true, out: tmp, dryRun: true }) + + expect(warnings()).toContain(FRAGMENT) + // Nothing written — the user's own migration is all that is there. + expect(readdirSync(tmp)).toEqual([EARLIER]) + }) + + it('stays quiet for an empty migrations directory', async () => { + await eqlMigrationCommand({ supabase: true, out: tmp }) + + expect(warnings()).not.toContain(FRAGMENT) + }) + + it('stays quiet for a migrations directory that does not exist yet', async () => { + await eqlMigrationCommand({ supabase: true, out: join(tmp, 'nope') }) + + expect(warnings()).not.toContain(FRAGMENT) + }) + + it('warns above the dry-run branch, inside the command frame', async () => { + // Same placement discipline as the --out warning: after the intro (so + // clack renders it inside the frame) and before the dry-run return. + writeFileSync(join(tmp, EARLIER), ENCRYPTED_COLUMN_SQL) + + await eqlMigrationCommand({ supabase: true, out: tmp, dryRun: true }) + + const introAt = vi.mocked(clack.intro).mock.invocationCallOrder[0] + const warnAt = clack.log.warn.mock.calls.findIndex((c) => + String(c[0]).includes(FRAGMENT), + ) + expect(warnAt).toBeGreaterThanOrEqual(0) + expect(clack.log.warn.mock.invocationCallOrder[warnAt]).toBeGreaterThan( + introAt, + ) + }) + }) + + it('warns that --name is ignored rather than silently dropping it', async () => { + // The filename is load-bearing: duplicate detection matches the + // `_cipherstash_eql.sql` suffix, so --name cannot be honoured here. Saying + // nothing would leave the user believing they had renamed it. + await eqlMigrationCommand({ supabase: true, out: tmp, name: 'my-install' }) + + expect(clack.log.warn).toHaveBeenCalledWith( + messages.eql.migrationNameDrizzleOnly, + ) + expect(readdirSync(tmp)[0]).toMatch(/^\d{14}_cipherstash_eql\.sql$/) + }) + + it('warns about --name inside the command frame, not above it', async () => { + // clack renders log lines into the frame the intro opens. Warning first + // put the line above the banner, detached from the command it belongs to. + await eqlMigrationCommand({ supabase: true, out: tmp, name: 'my-install' }) + + const introAt = vi.mocked(clack.intro).mock.invocationCallOrder[0] + const warnAt = vi.mocked(clack.log.warn).mock.invocationCallOrder[0] + expect(warnAt).toBeGreaterThan(introAt) + }) + + it('still warns about --name on a dry run, which also ignores it', async () => { + await eqlMigrationCommand({ + supabase: true, + out: tmp, + name: 'my-install', + dryRun: true, + }) + + expect(clack.log.warn).toHaveBeenCalledWith( + messages.eql.migrationNameDrizzleOnly, + ) + }) + + it('stays quiet about --name when it was not passed', async () => { + await eqlMigrationCommand({ supabase: true, out: tmp }) + + expect(clack.log.warn).not.toHaveBeenCalledWith( + messages.eql.migrationNameDrizzleOnly, + ) + }) + + it('emits the standalone banners by default', async () => { + await eqlMigrationCommand({ supabase: true, out: tmp }) + + expect(clack.intro).toHaveBeenCalledWith('CipherStash EQL migration') + expect(clack.outro).toHaveBeenCalledWith('Done!') + expect(printNextSteps).toHaveBeenCalled() + }) + + it('suppresses intro/outro/next-steps when embedded, but still writes', async () => { + // `stash init` renders its own summary and agent handoff; two competing + // "what next" blocks is the bug this flag exists to prevent. + await eqlMigrationCommand({ supabase: true, out: tmp, embedded: true }) + + expect(readdirSync(tmp)).toHaveLength(1) + expect(clack.intro).not.toHaveBeenCalled() + expect(clack.outro).not.toHaveBeenCalled() + expect(printNextSteps).not.toHaveBeenCalled() + }) + + it('suppresses the abort outro when embedded but still exits 1', async () => { + await eqlMigrationCommand({ supabase: true, out: tmp }) + vi.clearAllMocks() + + await expect( + eqlMigrationCommand({ supabase: true, out: tmp, embedded: true }), + ).rejects.toBeInstanceOf(CliExit) + expect(clack.outro).not.toHaveBeenCalled() + }) + + /** + * `--out` on a bare `--supabase` can silently reintroduce #613, the very bug + * this emitter exists to fix. The Supabase CLI's migrations directory is not + * configurable — `db reset` and `db push` read `/supabase/migrations` + * and nothing else — so a file written anywhere else is EQL missing from the + * replayed directory all over again, just relocated. The flag stays (a user + * may have their own apply step) but must not be silent. + * + * `process.cwd` is stubbed rather than `process.chdir`-ing, because the + * default arm has to write a real file and doing that relative to the repo + * root would litter it. + */ + describe('--out outside supabase/migrations', () => { + const warnings = () => + clack.log.warn.mock.calls.map((c) => String(c[0])).join('\n') + + // Restored here rather than by a global `restoreAllMocks`, which would also + // tear down the module-level `vi.fn()` mocks this file depends on. + let restoreCwd: (() => void) | undefined + const stubCwd = (dir: string) => { + const spy = vi.spyOn(process, 'cwd').mockReturnValue(dir) + restoreCwd = () => spy.mockRestore() + } + afterEach(() => { + restoreCwd?.() + restoreCwd = undefined + }) + + it('warns for a relative --out that is not the default', async () => { + stubCwd(tmp) + + await eqlMigrationCommand({ supabase: true, out: 'db/migrations' }) + + expect(clack.log.warn).toHaveBeenCalledWith( + messages.eql.migrationSupabaseOutNotReplayed( + join(tmp, 'db', 'migrations'), + ), + ) + // The consequence, not just the deviation: a user who reads "non-standard + // directory" and shrugs is exactly the user this warning is for. + expect(warnings()).toContain('supabase db reset') + }) + + it('warns for an absolute --out that is not the default', async () => { + stubCwd(tmp) + const out = join(tmp, 'elsewhere') + + await eqlMigrationCommand({ supabase: true, out }) + + expect(clack.log.warn).toHaveBeenCalledWith( + messages.eql.migrationSupabaseOutNotReplayed(out), + ) + }) + + it('stays quiet when --out is omitted', async () => { + stubCwd(tmp) + + await eqlMigrationCommand({ supabase: true }) + + expect(readdirSync(join(tmp, 'supabase', 'migrations'))).toHaveLength(1) + expect(warnings()).not.toContain('--out points at') + }) + + it('stays quiet when --out resolves to exactly the default', async () => { + // Same directory, spelled the long way. Comparing the raw string would + // fire here — the check has to compare resolved paths. + stubCwd(tmp) + + await eqlMigrationCommand({ supabase: true, out: 'supabase/migrations' }) + + expect(readdirSync(join(tmp, 'supabase', 'migrations'))).toHaveLength(1) + expect(warnings()).not.toContain('--out points at') + }) + + it('stays quiet for an absolute --out that normalises to the default', async () => { + // An absolute --out is taken verbatim by `detectSupabaseProject`, so the + // trailing separator a user typed (or a shell completed) survives into the + // comparison. Raw string equality would warn about the directory it is + // already writing to. + stubCwd(tmp) + + await eqlMigrationCommand({ + supabase: true, + out: `${join(tmp, 'supabase', 'migrations')}${sep}`, + }) + + expect(warnings()).not.toContain('--out points at') + }) + + it('warns on a dry run, which is where the prediction matters most', async () => { + // A dry run exists to tell you what the real run will do. Withholding the + // one thing that makes the real run pointless would defeat it. + stubCwd(tmp) + + await eqlMigrationCommand({ + supabase: true, + out: 'db/migrations', + dryRun: true, + }) + + expect(clack.log.warn).toHaveBeenCalledWith( + messages.eql.migrationSupabaseOutNotReplayed( + join(tmp, 'db', 'migrations'), + ), + ) + }) + + it('leaves the replacement warning last so --force still reads correctly', async () => { + // Both fire on a forced re-run into a custom directory. The out warning is + // an up-front flag advisory; the replacement warning is the outcome, and + // has to stay adjacent to the success line it qualifies. + stubCwd(tmp) + await eqlMigrationCommand({ supabase: true, out: 'db/migrations' }) + // `clearAllMocks` only clears recorded calls; the cwd stub's return value + // survives, and re-stubbing would nest a second spy the restore can't undo. + vi.clearAllMocks() + + await eqlMigrationCommand({ + supabase: true, + out: 'db/migrations', + force: true, + }) + + const [last] = vi.mocked(clack.log.warn).mock.calls.at(-1) ?? [] + expect(last).toMatch(/already applied/) + }) + + it('never warns on the --drizzle --supabase grants path', async () => { + // There `--supabase` is the grants modifier and `--out` is a drizzle-kit + // output directory, which has nothing to do with supabase/migrations. + // Warning would be nonsense advice on the documented invocation. + const out = join(tmp, 'drizzle') + + await eqlMigrationCommand({ + drizzle: true, + supabase: true, + out, + dryRun: true, + }) + + expect(warnings()).not.toContain('--out points at') + }) + }) }) describe('eqlMigrationCommand — Drizzle', () => { diff --git a/packages/cli/src/commands/eql/__tests__/supabase-migration.test.ts b/packages/cli/src/commands/eql/__tests__/supabase-migration.test.ts new file mode 100644 index 000000000..48d0e7fbc --- /dev/null +++ b/packages/cli/src/commands/eql/__tests__/supabase-migration.test.ts @@ -0,0 +1,408 @@ +import { + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { buildEqlV3MigrationSql } from '../migration.js' +import { + findEqlDependentMigrationsBefore, + findExistingEqlMigration, + SUPABASE_EQL_MIGRATION_SUFFIX, + writeSupabaseEqlMigration, +} from '../supabase-migration.js' + +// Real filesystem throughout: the whole point of this module is what lands on +// disk, and a mocked `node:fs` would assert our own mock's behaviour. +// +// The one exception is `writeFile`, a spy that delegates to the real impl so +// the atomicity test can make just that call fail. `vi.spyOn` cannot do this — +// an ESM namespace is not configurable — so the module is mocked and the +// delegating default restored in `beforeEach` after `clearAllMocks`. +const fsWrite = vi.hoisted(() => ({ + real: (() => { + throw new Error( + 'fsWrite.real not initialised: node:fs/promises mock factory did not run', + ) + }) as typeof import('node:fs/promises').writeFile, + spy: vi.fn(), +})) +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal() + fsWrite.real = actual.writeFile + return { ...actual, default: actual, writeFile: fsWrite.spy } +}) + +let tmp: string + +beforeEach(() => { + fsWrite.spy.mockImplementation(fsWrite.real) + tmp = mkdtempSync(join(tmpdir(), 'stash-supabase-migration-')) +}) + +afterEach(() => { + rmSync(tmp, { recursive: true, force: true }) +}) + +const FIXED_NOW = new Date('2026-08-04T02:19:25.000Z') +const FIXED_FILENAME = `20260804021925${SUPABASE_EQL_MIGRATION_SUFFIX}` + +describe('findExistingEqlMigration', () => { + it('returns null for a directory that does not exist', () => { + expect(findExistingEqlMigration(join(tmp, 'nope'))).toBeNull() + }) + + it('returns null when no install migration is present', () => { + mkdirSync(join(tmp, 'migrations')) + writeFileSync(join(tmp, 'migrations', '20260101000000_users.sql'), '') + expect(findExistingEqlMigration(join(tmp, 'migrations'))).toBeNull() + }) + + it('matches on the suffix, not an exact filename', () => { + // The timestamp differs on every run, so an exact-name check would miss a + // file this command itself wrote — and we would install EQL twice. + mkdirSync(join(tmp, 'migrations')) + const path = join(tmp, 'migrations', `20991231235959_cipherstash_eql.sql`) + writeFileSync(path, '') + expect(findExistingEqlMigration(join(tmp, 'migrations'))).toBe(path) + }) + + it('ignores a directory that happens to carry the suffix', () => { + // readdirSync returns directories too. Treating one as the target made it + // targetPath, and the write then failed with a raw EISDIR through the + // generic error path. + mkdirSync(join(tmp, 'migrations', '20260101000000_cipherstash_eql.sql'), { + recursive: true, + }) + expect(findExistingEqlMigration(join(tmp, 'migrations'))).toBeNull() + }) + + it('returns the lexically last when several exist', () => { + mkdirSync(join(tmp, 'migrations')) + writeFileSync( + join(tmp, 'migrations', '20260101000000_cipherstash_eql.sql'), + '', + ) + const newer = join(tmp, 'migrations', '20270101000000_cipherstash_eql.sql') + writeFileSync(newer, '') + expect(findExistingEqlMigration(join(tmp, 'migrations'))).toBe(newer) + }) +}) + +describe('writeSupabaseEqlMigration', () => { + it('creates the migrations directory when it is absent', async () => { + const dir = join(tmp, 'supabase', 'migrations') + expect(existsSync(dir)).toBe(false) + + const result = await writeSupabaseEqlMigration({ + migrationsDir: dir, + sql: 'SELECT 1;', + now: FIXED_NOW, + }) + + expect(result.overwritten).toBe(false) + expect(result.path).toBe(join(dir, FIXED_FILENAME)) + expect(existsSync(result.path)).toBe(true) + }) + + it('names the file _cipherstash_eql.sql', async () => { + const result = await writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: 'SELECT 1;', + now: FIXED_NOW, + }) + expect(result.path.endsWith(FIXED_FILENAME)).toBe(true) + }) + + it('reports the version the file carries', async () => { + const result = await writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: 'SELECT 1;', + now: FIXED_NOW, + }) + expect(result.version).toBe('20260804021925') + }) + + it('sorts after an already-applied migration rather than before it', async () => { + // A version BELOW the highest applied one is "out of order" to the Supabase + // CLI, and it is not merely skipped: `supabase db push` aborts the whole + // push with `Found local migration files to be inserted before the last + // migration on remote database.` and applies nothing, until the user knows + // to re-run with --include-all. The retired v2 writer used an all-zero + // prefix and had exactly that problem. + writeFileSync(join(tmp, '20260101000000_users.sql'), '') + const result = await writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: 'SELECT 1;', + now: FIXED_NOW, + }) + const [first] = readdirSync(tmp).sort() + expect(first).toBe('20260101000000_users.sql') + expect(result.path.endsWith(FIXED_FILENAME)).toBe(true) + }) + + it('writes a header above the SQL body', async () => { + const result = await writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: 'SELECT 1;', + now: FIXED_NOW, + }) + const body = readFileSync(result.path, 'utf-8') + expect(body).toMatch(/^-- CipherStash EQL v3/) + expect(body).toContain('supabase db reset') + expect(body.trimEnd().endsWith('SELECT 1;')).toBe(true) + }) + + it('carries the v3 bundle, the Supabase grants, and the tracking schema', async () => { + // The real SQL, not a stub — this is the contract that one `supabase db + // reset` provisions everything `stash encrypt` needs. + const result = await writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: buildEqlV3MigrationSql({ supabase: true }), + now: FIXED_NOW, + }) + const body = readFileSync(result.path, 'utf-8') + + expect(body).toContain('eql_v3') + expect(body).toContain('eql_v3_internal') + for (const role of ['anon', 'authenticated', 'service_role']) { + expect(body).toContain(role) + } + expect(body).toContain('cs_migrations') + }) + + it('refuses a second install migration without force', async () => { + await writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: 'SELECT 1;', + now: FIXED_NOW, + }) + + await expect( + writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: 'SELECT 2;', + now: new Date('2026-09-04T02:19:25.000Z'), + }), + ).rejects.toThrow(/already exists/) + + expect(readdirSync(tmp)).toHaveLength(1) + }) + + it('overwrites in place under force, keeping the original version', async () => { + // Not a second, newer-versioned file: the first one may already be applied + // and cannot be deleted without desyncing schema_migrations, which would + // leave two EQL installs in the history. + const first = await writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: 'SELECT 1;', + now: FIXED_NOW, + }) + + const second = await writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: 'SELECT 2;', + force: true, + now: new Date('2026-09-04T02:19:25.000Z'), + }) + + expect(second.path).toBe(first.path) + expect(second.overwritten).toBe(true) + // The version is what the remote ledger keys on, so it is the thing the + // re-apply guidance has to name — reported, not re-derived by the caller. + expect(second.version).toBe(first.version) + expect(readdirSync(tmp)).toHaveLength(1) + expect(readFileSync(second.path, 'utf-8')).toContain('SELECT 2;') + }) + + it('leaves no partial .sql behind when the write fails', async () => { + // The migrations directory is executed wholesale by `supabase db reset`, so + // a truncated file from an interrupted write is not inert — it runs. The + // write goes to a temp sibling and is renamed, so a failure leaves the + // directory exactly as it was. + fsWrite.spy.mockRejectedValueOnce( + new Error('ENOSPC: no space left on device'), + ) + + await expect( + writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: 'SELECT 1;', + now: FIXED_NOW, + }), + ).rejects.toThrow(/ENOSPC/) + + expect(readdirSync(tmp)).toHaveLength(0) + }) + + it('does not leave a temp file behind on a successful write', async () => { + await writeSupabaseEqlMigration({ + migrationsDir: tmp, + sql: 'SELECT 1;', + now: FIXED_NOW, + }) + + expect(readdirSync(tmp)).toEqual([FIXED_FILENAME]) + }) +}) + +/** + * Brownfield detection. A current-timestamp install sorts LAST, which is right + * for a greenfield project and wrong for one that already has encrypted-column + * migrations on disk: `supabase db reset` replays in version order with no + * dependency awareness, so those run before EQL exists and the reset dies on + * `type "eql_v3_text_search" does not exist`. + */ +describe('findEqlDependentMigrationsBefore', () => { + // What the stash-supabase skill tells people to write by hand for an + // encrypted twin: a `public.eql_v3_*` domain the bundle creates. + const ENCRYPTED_COLUMN_SQL = + 'ALTER TABLE users ADD COLUMN email_encrypted public.eql_v3_text_search;\n' + + it('returns nothing for a directory that does not exist', () => { + expect(findEqlDependentMigrationsBefore(join(tmp, 'nope'))).toEqual([]) + }) + + it('returns nothing for an empty directory', () => { + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual( + [], + ) + }) + + it('finds an earlier migration that names an EQL domain', () => { + writeFileSync( + join(tmp, '20260101000000_encrypt_email.sql'), + ENCRYPTED_COLUMN_SQL, + ) + + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual([ + '20260101000000_encrypt_email.sql', + ]) + }) + + it('finds an earlier migration that calls into the eql_v3 schema', () => { + // The other reference form: a function/operator call rather than a column + // domain (`eql_v3.query_text(...)`, `eql_v3.ste_vec(...)`). + writeFileSync( + join(tmp, '20260101000000_index_email.sql'), + 'CREATE INDEX ON users (eql_v3.hmac_256(email_encrypted));\n', + ) + + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual([ + '20260101000000_index_email.sql', + ]) + }) + + it('ignores a migration that sorts after the install', () => { + writeFileSync( + join(tmp, '20990101000000_encrypt_email.sql'), + ENCRYPTED_COLUMN_SQL, + ) + + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual( + [], + ) + }) + + it('ignores an earlier migration that never mentions EQL', () => { + writeFileSync( + join(tmp, '20260101000000_users.sql'), + 'CREATE TABLE users (id uuid PRIMARY KEY, email text);\n', + ) + + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual( + [], + ) + }) + + it('does not trip on a longer identifier that merely contains the token', () => { + writeFileSync( + join(tmp, '20260101000000_notes.sql'), + 'CREATE TABLE not_eql_v3_notes (id integer);\n', + ) + + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual( + [], + ) + }) + + it('ignores our own install migrations', () => { + // An older `_cipherstash_eql.sql` is another copy of the install, not + // something depending on it — reporting it would tell the user to reorder + // the install below itself. + writeFileSync( + join(tmp, '20260101000000_cipherstash_eql.sql'), + 'CREATE SCHEMA eql_v3;\n', + ) + writeFileSync(join(tmp, '20260301000000_cipherstash_eql.sql'), '') + + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual( + [], + ) + }) + + it('ignores files the Supabase CLI itself skips', () => { + // `^([0-9]+)_(.*)\.sql$` in pkg/migration/file.go. A name that fails it is + // never applied (the CLI prints `Skipping migration ...` and moves on), so + // it cannot break a reset however it sorts. + writeFileSync(join(tmp, 'encrypt_email.sql'), ENCRYPTED_COLUMN_SQL) + writeFileSync( + join(tmp, '20260101000000_encrypt_email.sql.bak'), + ENCRYPTED_COLUMN_SQL, + ) + writeFileSync( + join(tmp, '.20260101000000_encrypt_email.sql'), + ENCRYPTED_COLUMN_SQL, + ) + + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual( + [], + ) + }) + + it('ignores a directory that carries a migration-shaped name', () => { + mkdirSync(join(tmp, '20260101000000_encrypt_email.sql')) + + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual( + [], + ) + }) + + it('sorts the hits so the earliest is first', () => { + writeFileSync( + join(tmp, '20260201000000_encrypt_name.sql'), + ENCRYPTED_COLUMN_SQL, + ) + writeFileSync( + join(tmp, '20260101000000_encrypt_email.sql'), + ENCRYPTED_COLUMN_SQL, + ) + + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual([ + '20260101000000_encrypt_email.sql', + '20260201000000_encrypt_name.sql', + ]) + }) + + it('compares against the version --force keeps, not the clock', () => { + // With an install migration already on disk, that file is overwritten in + // place and keeps ITS version — so the ordering question is about that + // version, not today's. Comparing against the clock would report a + // dependant that in fact replays after the install. + writeFileSync(join(tmp, '20260101000000_cipherstash_eql.sql'), '') + writeFileSync( + join(tmp, '20260102000000_encrypt_email.sql'), + ENCRYPTED_COLUMN_SQL, + ) + + expect(findEqlDependentMigrationsBefore(tmp, { now: FIXED_NOW })).toEqual( + [], + ) + }) +}) diff --git a/packages/cli/src/commands/eql/__tests__/supabase-push.live.test.ts b/packages/cli/src/commands/eql/__tests__/supabase-push.live.test.ts new file mode 100644 index 000000000..514921e88 --- /dev/null +++ b/packages/cli/src/commands/eql/__tests__/supabase-push.live.test.ts @@ -0,0 +1,418 @@ +/** + * Live-Supabase-CLI coverage for the generated EQL install migration. + * + * Everything else about `eql migration --supabase` is tested against the + * filesystem, which is right for the writer's control flow and useless for the + * only question that actually matters: what the *Supabase CLI* does with the + * file we wrote. The unit suite's ordering test sorts two filenames in a + * tmpdir — it pins that our name sorts second and says nothing about whether + * `db push` applies it, refuses it, or ignores it. + * + * That gap mattered. The `--force` re-apply guidance this command prints (and + * ships to customers in `skills/stash-cli` and `skills/stash-supabase`) was + * derived by reading supabase/cli's Go source, and reading got one detail + * wrong — see `needs --include-all only when the install is not the newest + * migration` below. + * + * `db push --db-url` needs no Docker and no linked project (the flag is + * mutually exclusive with `--linked`/`--local`), so a bare Postgres cluster is + * enough to exercise the real binary. What that still leaves unproven is + * `db reset` — it removes the container and volume, so it needs the full local + * stack — and the PostgREST HTTP round-trip. The grants are covered here one + * layer down, via `SET ROLE`, which is exactly what PostgREST does after + * connecting as `authenticator`. + * + * Gated on both env vars so the default `pnpm test` stays green. Locally: + * + * initdb -D /tmp/sbpg -U postgres + * pg_ctl -D /tmp/sbpg -o "-p 55444 -k /tmp" -l /tmp/sbpg.log start + * createdb -h 127.0.0.1 -p 55444 -U postgres sbtest + * psql -h 127.0.0.1 -p 55444 -U postgres -d sbtest \ + * -c "CREATE ROLE anon NOLOGIN; CREATE ROLE authenticated NOLOGIN; CREATE ROLE service_role NOLOGIN;" + * export STASH_TEST_SUPABASE_DB_URL='postgresql://postgres@127.0.0.1:55444/sbtest?sslmode=disable' + * export STASH_TEST_SUPABASE_CLI='npx --yes supabase@2.111.0' + * + * The three roles must exist before anything here runs: the emitted SQL grants + * to `anon`/`authenticated`/`service_role` and sets default privileges FOR ROLE + * `postgres`, none of which a bare cluster has. On a real Supabase project they + * are all present — this is a fixture requirement, not a product gap. + * + * `sslmode=disable` is likewise a bare-cluster detail: the CLI negotiates TLS + * by default and fails with `The server does not support SSL connections` + * against a stock `initdb`. + */ + +import { spawnSync } from 'node:child_process' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterAll, afterEach, beforeEach, describe, expect, it } from 'vitest' +import { buildEqlV3MigrationSql } from '../migration.js' +import { writeSupabaseEqlMigration } from '../supabase-migration.js' + +const DATABASE_URL = process.env.STASH_TEST_SUPABASE_DB_URL +const CLI = process.env.STASH_TEST_SUPABASE_CLI +const describeLive = DATABASE_URL && CLI ? describe : describe.skip + +/** + * Pushing the real ~2.6 MB bundle takes a few seconds per call, and several + * tests push more than once. + */ +const LIVE_TIMEOUT = 120_000 + +/** A migration the install must land *after*, so the ledger has prior state. */ +const EARLIER_VERSION = '20260101000000' + +describeLive('eql migration --supabase — live Supabase CLI', () => { + let projectDir: string + let migrationsDir: string + + // The CLI is given as a command line ("npx --yes supabase@2.111.0" or a bare + // binary path), so it splits into argv rather than going through a shell — + // the connection string carries credentials and must never be word-split or + // interpreted. + function supabase(...args: string[]): { + status: number | null + stdout: string + stderr: string + } { + const parts = (CLI as string).split(/\s+/).filter(Boolean) + const result = spawnSync( + parts[0], + [...parts.slice(1), ...args, '--db-url', DATABASE_URL as string], + { cwd: projectDir, encoding: 'utf-8', stdio: 'pipe' }, + ) + return { + status: result.status, + stdout: result.stdout ?? '', + stderr: result.stderr ?? '', + } + } + + async function withClient( + fn: (q: (sql: string) => Promise<{ rows: unknown[][] }>) => Promise, + ): Promise { + const { default: pg } = await import('pg') + const client = new pg.Client({ connectionString: DATABASE_URL }) + await client.connect() + try { + return await fn(async (sql: string) => { + const result = await client.query({ text: sql, rowMode: 'array' }) + return { rows: result.rows as unknown[][] } + }) + } finally { + await client.end().catch(() => undefined) + } + } + + /** + * Back to a database that has never seen EQL, or these tests. + * + * Drops the ledger, because a leftover `schema_migrations` row makes the next + * push a no-op — which looks exactly like the bug the `--force` test checks + * for. And drops `public` wholesale rather than naming the tables and marker + * schemas each test creates: the EQL bundle puts its column domains in + * `public` too, and an enumerated list silently rots the first time someone + * adds a test. Getting this wrong does not fail loudly — the suite passes + * against a fresh cluster and fails on the second run, which is the worst + * shape a test can have. + */ + async function resetDatabase(): Promise { + await withClient(async (q) => { + await q('DROP SCHEMA IF EXISTS supabase_migrations CASCADE') + await q('DROP SCHEMA IF EXISTS eql_v3 CASCADE') + await q('DROP SCHEMA IF EXISTS eql_v3_internal CASCADE') + await q('DROP SCHEMA IF EXISTS cipherstash CASCADE') + // Marker schemas the re-apply and temp-file tests use as canaries. + await q('DROP SCHEMA IF EXISTS force_marker CASCADE') + await q('DROP SCHEMA IF EXISTS leaked_temp_file CASCADE') + await q('DROP SCHEMA IF EXISTS public CASCADE') + await q('CREATE SCHEMA public') + // Re-granted explicitly: a hand-created `public` does not inherit the + // grants initdb gives the one it ships, and the emitted migration's + // `ALTER DEFAULT PRIVILEGES FOR ROLE postgres IN SCHEMA public` needs the + // roles to be able to reach it. + await q('GRANT USAGE, CREATE ON SCHEMA public TO PUBLIC') + }) + } + + /** Write the real install SQL, through the real writer, into the project. */ + async function writeInstall(options: { force?: boolean; now?: Date } = {}) { + return await writeSupabaseEqlMigration({ + migrationsDir, + sql: buildEqlV3MigrationSql({ supabase: true }), + ...options, + }) + } + + beforeEach(async () => { + projectDir = mkdtempSync(join(tmpdir(), 'stash-supabase-push-')) + migrationsDir = join(projectDir, 'supabase', 'migrations') + // `db push` resolves `/supabase/migrations` and wants the project + // marker beside it; both have to exist before the CLI is invoked. + mkdirSync(migrationsDir, { recursive: true }) + writeFileSync( + join(projectDir, 'supabase', 'config.toml'), + 'project_id = "stash-live-test"\n', + ) + await resetDatabase() + }) + + afterEach(() => { + rmSync(projectDir, { recursive: true, force: true }) + }) + + afterAll(async () => { + await resetDatabase() + }) + + /** + * Seed one applied migration so the install is never the only thing in the + * ledger — "sorts after what is already applied" is only meaningful when + * something already is. + */ + function seedApplied(): void { + writeFileSync( + join(migrationsDir, `${EARLIER_VERSION}_users.sql`), + 'CREATE TABLE users (id serial primary key);\n', + ) + } + + it( + 'applies the generated install through the real CLI, with no --include-all', + async () => { + seedApplied() + const written = await writeInstall() + + const push = supabase('db', 'push') + expect(push.status, push.stdout + push.stderr).toBe(0) + expect(push.stdout).toContain(`${written.version}_cipherstash_eql.sql`) + + await withClient(async (q) => { + const ledger = await q( + 'SELECT version FROM supabase_migrations.schema_migrations ORDER BY version', + ) + expect(ledger.rows.map((r) => r[0])).toEqual([ + EARLIER_VERSION, + written.version, + ]) + + // The install really ran, rather than the ledger row being written for + // a body Postgres rejected. Also the only proof that the CLI's + // statement splitter survives the bundle's dollar-quoted function + // bodies — ~2.6 MB of them. + const schemas = await q( + "SELECT nspname FROM pg_namespace WHERE nspname IN ('eql_v3','eql_v3_internal','cipherstash') ORDER BY 1", + ) + expect(schemas.rows.map((r) => r[0])).toEqual([ + 'cipherstash', + 'eql_v3', + 'eql_v3_internal', + ]) + }) + }, + LIVE_TIMEOUT, + ) + + it( + 'grants the Supabase roles through the emitted file, not just through `eql install`', + async () => { + await writeInstall() + const push = supabase('db', 'push') + expect(push.status, push.stdout + push.stderr).toBe(0) + + // `SET ROLE anon` is what PostgREST does after connecting as + // `authenticator`, so this is the same privilege check a request makes — + // one layer below the HTTP round-trip the Docker suite covers, and + // against the grants carried INSIDE the generated migration rather than + // the ones `eql install --direct` applies. + await withClient(async (q) => { + const privileges = await q( + "SELECT has_schema_privilege('anon','eql_v3','USAGE'), has_schema_privilege('anon','eql_v3_internal','USAGE')", + ) + expect(privileges.rows[0]).toEqual([true, true]) + + await q('SET ROLE anon') + const call = await q( + `SELECT eql_v3.ciphertext('{"c":"x","i":{"t":"t","c":"c"},"v":2}'::jsonb)`, + ) + expect(call.rows[0][0]).toBe('x') + }) + }, + LIVE_TIMEOUT, + ) + + it( + 'refuses an out-of-order install rather than silently skipping it', + async () => { + seedApplied() + // Apply the seed on its own first, so the back-dated install lands below + // something the remote has already recorded. + expect(supabase('db', 'push').status).toBe(0) + + await writeInstall({ now: new Date('2025-01-01T00:00:00.000Z') }) + + const push = supabase('db', 'push') + expect(push.status).not.toBe(0) + const output = push.stdout + push.stderr + expect(output).toContain( + 'Found local migration files to be inserted before the last migration on remote database.', + ) + expect(output).toContain('--include-all') + + // Aborted, not partially applied: the whole push is rejected before any + // file runs. This is the behaviour the timestamped version exists to + // avoid, and it is NOT the "silently skipped" one an earlier comment in + // this codebase claimed. + await withClient(async (q) => { + const schemas = await q( + "SELECT count(*) FROM pg_namespace WHERE nspname = 'eql_v3'", + ) + expect(Number(schemas.rows[0][0])).toBe(0) + }) + }, + LIVE_TIMEOUT, + ) + + it( + 'does not re-apply a --force-replaced file: the push is a silent no-op', + async () => { + const written = await writeInstall() + expect(supabase('db', 'push').status).toBe(0) + + // Replace the body with something whose effect is trivially detectable. + // A real `--force` run rewrites the bundle; the question here is only + // whether the CLI notices a content change at an applied version. + await writeSupabaseEqlMigration({ + migrationsDir, + sql: 'CREATE SCHEMA force_marker;', + force: true, + }) + + const push = supabase('db', 'push') + expect(push.status, push.stdout + push.stderr).toBe(0) + expect(push.stdout).toMatch(/up to date/i) + + // The heart of it: pending is computed by version, never by content, so + // the replaced body never runs. Any guidance that tells a user to + // re-apply a `--force`d install with a plain `db push` is wrong, and this + // is the assertion that says so. + await withClient(async (q) => { + const marker = await q( + "SELECT count(*) FROM pg_namespace WHERE nspname = 'force_marker'", + ) + expect(Number(marker.rows[0][0])).toBe(0) + }) + + // And the recipe we print instead does work: clear the ledger row, then + // push. `migration repair` touches the tracking table only. + const repair = supabase( + 'migration', + 'repair', + '--status', + 'reverted', + written.version as string, + ) + expect(repair.status, repair.stdout + repair.stderr).toBe(0) + + const rePush = supabase('db', 'push') + expect(rePush.status, rePush.stdout + rePush.stderr).toBe(0) + await withClient(async (q) => { + const marker = await q( + "SELECT count(*) FROM pg_namespace WHERE nspname = 'force_marker'", + ) + expect(Number(marker.rows[0][0])).toBe(1) + }) + }, + LIVE_TIMEOUT, + ) + + it( + 'needs --include-all only when the install is not the newest migration', + async () => { + // The correction to the shipped recipe. After `migration repair --status + // reverted`, whether the follow-up push needs `--include-all` depends + // entirely on where the reverted version sits: at the tail it is just + // pending and a plain push takes it, and only a version with applied + // migrations ABOVE it is the "gap in the middle" that trips + // ErrMissingRemote. The greenfield flow puts encrypted-column migrations + // after the install, which is exactly the gap case — so the flag belongs + // in the guidance, but as a condition rather than a rule. + seedApplied() + const written = await writeInstall() + expect(supabase('db', 'push').status).toBe(0) + + // Install is newest: reverting it leaves a tail, not a gap. + expect( + supabase( + 'migration', + 'repair', + '--status', + 'reverted', + written.version as string, + ).status, + ).toBe(0) + const tailPush = supabase('db', 'push') + expect(tailPush.status, tailPush.stdout + tailPush.stderr).toBe(0) + expect(tailPush.stdout).toContain('cipherstash_eql.sql') + + // Now give it a successor, so reverting the install leaves a hole. + writeFileSync( + join(migrationsDir, '20270101000000_later.sql'), + 'CREATE TABLE later_table (id int);\n', + ) + expect(supabase('db', 'push').status).toBe(0) + expect( + supabase( + 'migration', + 'repair', + '--status', + 'reverted', + written.version as string, + ).status, + ).toBe(0) + + const gapPush = supabase('db', 'push') + expect(gapPush.status).not.toBe(0) + expect(gapPush.stdout + gapPush.stderr).toContain( + 'Found local migration files to be inserted before the last migration on remote database.', + ) + + const includeAll = supabase('db', 'push', '--include-all') + expect(includeAll.status, includeAll.stdout + includeAll.stderr).toBe(0) + }, + LIVE_TIMEOUT, + ) + + it( + 'ignores a leaked temp file instead of applying a half-written one', + async () => { + const written = await writeInstall() + expect(supabase('db', 'push').status).toBe(0) + + // The exact name the atomic write uses between `writeFile` and `rename`, + // holding SQL that must never run. + writeFileSync( + join(migrationsDir, `.${written.version}_cipherstash_eql.sql.tmp`), + 'CREATE SCHEMA leaked_temp_file;', + ) + + const push = supabase('db', 'push') + expect(push.status, push.stdout + push.stderr).toBe(0) + // Inert, but not invisible: the CLI reads the whole directory and reports + // every name that fails `^([0-9]+)_(.*)\.sql$`, on every push and reset + // until someone deletes it. + expect(push.stdout + push.stderr).toContain( + 'file name must match pattern', + ) + + await withClient(async (q) => { + const leaked = await q( + "SELECT count(*) FROM pg_namespace WHERE nspname = 'leaked_temp_file'", + ) + expect(Number(leaked.rows[0][0])).toBe(0) + }) + }, + LIVE_TIMEOUT, + ) +}) diff --git a/packages/cli/src/commands/eql/migration.ts b/packages/cli/src/commands/eql/migration.ts index 1a003c4b6..4fa1dbb3c 100644 --- a/packages/cli/src/commands/eql/migration.ts +++ b/packages/cli/src/commands/eql/migration.ts @@ -5,8 +5,14 @@ import { join, resolve } from 'node:path' import { MIGRATIONS_SCHEMA_SQL } from '@cipherstash/migrate' import * as p from '@clack/prompts' import { CliExit } from '@/cli/exit.js' +import { detectSupabaseProject } from '@/commands/db/detect.js' import { printNextSteps, SAFE_MIGRATION_NAME } from '@/commands/db/install.js' import { rewriteEncryptedAlterColumns } from '@/commands/db/rewrite-migrations.js' +import { + findEqlDependentMigrationsBefore, + findExistingEqlMigration, + writeSupabaseEqlMigration, +} from '@/commands/eql/supabase-migration.js' import { reportSweepFailure, reportSweepResult, @@ -64,12 +70,35 @@ export interface EqlMigrationOptions { * migration framework, so there is nothing for this command to emit. */ prisma?: boolean - /** Append the Supabase role grants (`eql_v3` + `eql_v3_internal`). */ + /** + * Two roles, decided by whether another target is present: + * + * - alone, it IS the target — write the install into `supabase/migrations/`; + * - with `--drizzle`, it's a modifier that appends the Supabase role grants + * (`eql_v3` + `eql_v3_internal`) to the Drizzle migration. + * + * The grants are in the emitted SQL either way; only the destination differs. + */ supabase?: boolean /** Migration name (Drizzle). Defaults to `install-eql`. */ name?: string - /** Output directory (Drizzle). Defaults to `drizzle`. */ + /** + * Output directory: where drizzle-kit writes under `--drizzle` (default + * `drizzle`), or the migrations directory under `--supabase` (default + * `supabase/migrations`). + * + * Under a bare `--supabase` anything other than the default earns a warning — + * the Supabase CLI replays `supabase/migrations` and nothing else, so a file + * written elsewhere is never applied. See + * `messages.eql.migrationSupabaseOutNotReplayed`. + */ out?: string + /** + * Write a Supabase install migration even though one is already there. + * Drizzle doesn't need this — drizzle-kit numbers each generated migration, + * so a re-run never collides. + */ + force?: boolean /** Describe what would happen without writing anything. */ dryRun?: boolean /** @@ -109,10 +138,21 @@ export function buildEqlV3MigrationSql(opts: { supabase: boolean }): string { /** * `stash eql migration` — generate an EQL v3 install migration for the target - * ORM, rather than running SQL directly against the database (that's `stash eql - * install`). Migration-first is the preferred path: the install lands in the - * project's migration history and ships to every environment through the ORM's - * own migrate step. + * ORM or platform, rather than running SQL directly against the database + * (that's `stash eql install`). Migration-first is the preferred path: the + * install lands in the project's migration history and ships to every + * environment through that project's own migrate step. + * + * Two emitters, one SQL body ({@link buildEqlV3MigrationSql}): + * + * - `--drizzle` scaffolds through drizzle-kit so the migration is journaled; + * - `--supabase` (alone) writes into `supabase/migrations/`, which is what + * makes the install survive `supabase db reset` — a reset drops the database + * and replays that directory, so a direct install is wiped by it. + * + * `--supabase` alongside `--drizzle` is NOT a second target: it's the grants + * modifier for a Supabase-hosted Drizzle project. Only a bare `--supabase` + * selects the Supabase emitter. * * v3 only — there is no `--eql-version` here. prisma-next never shipped v2, and * the Drizzle v3 surface is the documented one. @@ -124,15 +164,7 @@ export function buildEqlV3MigrationSql(opts: { supabase: boolean }): string { export async function eqlMigrationCommand( options: EqlMigrationOptions, ): Promise { - const targets = [ - options.drizzle && 'drizzle', - options.prisma && 'prisma', - ].filter(Boolean) - if (targets.length === 0) { - p.log.error(messages.eql.migrationNeedsTarget) - throw new CliExit(1) - } - if (targets.length > 1) { + if (options.drizzle && options.prisma) { p.log.error(messages.eql.migrationOneTarget) throw new CliExit(1) } @@ -146,7 +178,172 @@ export async function eqlMigrationCommand( throw new CliExit(1) } - await generateDrizzleEqlMigration(options) + if (options.drizzle) { + await generateDrizzleEqlMigration(options) + return + } + + if (options.supabase) { + await generateSupabaseEqlMigration(options) + return + } + + p.log.error(messages.eql.migrationNeedsTarget) + throw new CliExit(1) +} + +/** + * Write the EQL v3 install into `supabase/migrations/`. + * + * Deliberately unlike the Drizzle path in two ways. There is no drizzle-kit to + * scaffold through — Supabase migrations are plain timestamped `.sql` files + * with no journal to keep in step, so we write the file ourselves. And there is + * no ALTER COLUMN sweep: that exists because `drizzle-kit generate` emits + * in-place type changes that cannot run, and nothing here generates SQL from a + * schema diff. + */ +async function generateSupabaseEqlMigration( + options: EqlMigrationOptions, +): Promise { + // Reuses the resolver that already knows the `supabase/migrations` default + // and how to resolve a relative --out against the cwd. + const { migrationsDir } = detectSupabaseProject(process.cwd(), options.out) + + // Load the SQL up front so a corrupt/missing bundle fails BEFORE we create + // any directory, with the same spinner-free error the Drizzle path uses. + let sql: string + try { + // Always with grants: a file written for Supabase is applied by Supabase, + // where PostgREST reaches these tables as anon/authenticated/service_role. + sql = buildEqlV3MigrationSql({ supabase: true }) + } catch (error) { + p.log.error(error instanceof Error ? error.message : String(error)) + throw new CliExit(1) + } + + const embedded = options.embedded ?? false + if (!embedded) p.intro('CipherStash EQL migration') + + // After the intro, so the line lands inside the frame clack opens rather than + // above the banner. Before the dry-run branch, which ignores `--name` too. + // + // The filename is fixed (`_cipherstash_eql.sql`) because + // `findExistingEqlMigration` matches on that suffix to refuse duplicates. + // Silently dropping --name would leave the user believing they renamed it. + if (options.name !== undefined) { + p.log.warn(messages.eql.migrationNameDrizzleOnly) + } + + // `--out` is the one flag here that can quietly undo the whole command. The + // Supabase CLI's migrations directory is not configurable — `db reset` and + // `db push` read `/supabase/migrations` and nothing else — so an + // install written anywhere else is EQL missing from the replayed directory, + // which is #613 verbatim, just relocated. Compare RESOLVED paths: an absolute + // `--out` is taken verbatim by `detectSupabaseProject`, so `…/migrations/` + // and `…/migrations` are the same directory and only one of them is a string + // match. + // + // A warning, not a refusal. Some projects genuinely do apply another + // directory through their own tooling, and this command has no way to know — + // the same latitude the Drizzle path gives a `drizzle.config.ts` that writes + // somewhere unexpected. What the user must not be left assuming is that + // `supabase db reset` will pick the file up. + // + // Above the dry-run branch on purpose: predicting the real run is the dry + // run's entire job, and "this file will never be applied" is the most + // consequential thing there is to predict. + if ( + resolve(migrationsDir) !== resolve(process.cwd(), 'supabase', 'migrations') + ) { + p.log.warn(messages.eql.migrationSupabaseOutNotReplayed(migrationsDir)) + } + + // The install is stamped with the current time, which sorts it LAST. That is + // right for a greenfield project — nothing that needs EQL exists yet — and + // wrong for one that ran `stash eql install` first and wrote encrypted-column + // migrations against the live database. Those already carry `eql_v3_*` + // references and now sort BEFORE the install, so the next `supabase db reset` + // replays them first and fails on a domain nothing has created. + // + // Warn, don't fix. Back-dating the install would push a migration below the + // remote's last applied version, which is a `--include-all` push and a + // decision about someone else's deployed history — not ours to make silently. + // + // Above the dry-run branch for the same reason as the --out warning: a dry run + // that stays quiet about the reset it is about to break is not a prediction. + const eqlDependentsBefore = findEqlDependentMigrationsBefore(migrationsDir) + if (eqlDependentsBefore.length > 0) { + p.log.warn( + messages.eql.migrationSupabaseEqlBeforeInstall( + migrationsDir, + eqlDependentsBefore, + ), + ) + } + + if (options.dryRun) { + // Predict the real run's outcome, including its refusals — a dry run that + // always claims "would write" is worse than no dry run in the one directory + // where the answer is actually interesting. + const existing = findExistingEqlMigration(migrationsDir) + let preview: string + if (!existing) { + preview = `Would write the EQL v3 install SQL (with Supabase grants) into a new _cipherstash_eql.sql in ${migrationsDir}` + } else if (options.force) { + preview = `Would replace the EQL v3 install SQL in ${existing}, keeping its version.` + } else { + preview = `Would refuse: an EQL install migration already exists at ${existing}. Re-run with --force to replace it, or delete that file first.` + } + p.note(preview, 'Dry Run') + if (!embedded) p.outro('Dry run complete.') + return + } + + const s = p.spinner() + s.start('Writing EQL v3 install migration...') + let written: Awaited> + try { + written = await writeSupabaseEqlMigration({ + migrationsDir, + sql, + force: options.force ?? false, + }) + // Status only — the path is reported once, by the success line below, the + // same shape the Drizzle path uses. + s.stop('EQL v3 install SQL written.') + } catch (error) { + s.stop('Failed to write the migration.') + p.log.error(error instanceof Error ? error.message : String(error)) + if (!embedded) p.outro('Migration aborted.') + throw new CliExit(1) + } + + if (written.overwritten) { + // Rewriting a migration that some database has already applied leaves the + // file describing a shape that database never got from it — the same hazard + // `eql repair` guards against. We can't check that from here (no + // connection), so say it plainly, including the two things the success line + // cannot show: `db push` will not notice the rewrite (it diffs versions, not + // content), and re-applying cascade-drops whatever depends on eql_v3. + p.log.warn(messages.eql.migrationSupabaseForceReplaced) + } + + p.log.success( + `Migration ${written.overwritten ? 'replaced' : 'created'}: ${written.path}`, + ) + p.note( + // The plain apply note is only correct for a version no database has seen. + // Once the file has been replaced in place, `db push` is a no-op and the + // remote needs the ledger row cleared first. + written.overwritten + ? messages.eql.migrationSupabaseReapply(written.version) + : `Apply it:\n\n supabase db reset # local — replays every migration\n supabase db push # remote/linked project`, + 'Next Steps', + ) + if (!embedded) { + printNextSteps() + p.outro('Done!') + } } async function generateDrizzleEqlMigration( diff --git a/packages/cli/src/commands/eql/supabase-migration.ts b/packages/cli/src/commands/eql/supabase-migration.ts new file mode 100644 index 000000000..f4efa3ff6 --- /dev/null +++ b/packages/cli/src/commands/eql/supabase-migration.ts @@ -0,0 +1,289 @@ +import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs' +import { mkdir, rename, rm, writeFile } from 'node:fs/promises' +import { basename, join } from 'node:path' + +/** + * Suffix every generated Supabase EQL install migration carries. + * + * The filename is `_cipherstash_eql.sql`, so the stem varies + * per run and only the suffix is stable. {@link findExistingEqlMigration} + * matches on it — an exact-filename check would miss a file this command + * itself wrote at a different second, and re-running would silently install + * EQL a second time in the migration ledger. + */ +export const SUPABASE_EQL_MIGRATION_SUFFIX = '_cipherstash_eql.sql' + +/** + * A Supabase migration version is the leading `YYYYMMDDHHMMSS`. Generating it + * from the current time (rather than the all-zero prefix the retired EQL v2 + * writer used) keeps the file sorting *after* everything already applied. + * + * A lower-sorting version is "out of order" to the Supabase CLI, and it is not + * merely skipped: `FindPendingMigrations` (`pkg/migration/apply.go`) collects + * every local version below the last remote one into `unapplied` and returns + * `ErrMissingRemote` — "Found local migration files to be inserted before the + * last migration on remote database." — which `push.Run` propagates *before* + * applying anything. The whole push aborts until the user re-runs with + * `--include-all` (the only escape, in `internal/migration/up/up.go`). A + * current timestamp lands in the pending tail and pushes with no flag at all. + * + * That is the right default, not a universal one: an install stamped today also + * sorts after any encrypted-column migration a project already has, and those + * replay first on `supabase db reset`. See + * {@link findEqlDependentMigrationsBefore}, which detects that case so the + * command can say so rather than write a file that breaks the next reset. + */ +function migrationVersion(now: Date): string { + return now + .toISOString() + .replace(/[-:.TZ]/g, '') + .slice(0, 14) +} + +/** + * The Supabase CLI's own migration filename filter, `^([0-9]+)_(.*)\.sql$` + * (`migrateFilePattern`, `pkg/migration/file.go`). A file that fails it is + * never applied: `ListLocalMigrations` prints `Skipping migration ... + * (file name must match pattern "_name.sql")` to stderr and moves + * on. Mirrored here so this module's ordering analysis considers exactly the + * files the CLI will run. + */ +const SUPABASE_MIGRATION_FILENAME = /^(\d+)_.*\.sql$/ + +function migrationVersionOf(filename: string): string | null { + return SUPABASE_MIGRATION_FILENAME.exec(filename)?.[1] ?? null +} + +/** + * Anything that needs the EQL install to have run first: a `public.eql_v3_*` + * column domain, an `eql_v3.*` function or operator, an `eql_v3_internal.*` + * index-term type. One token covers all three, and the leading `\b` keeps it + * off lookalikes (`not_eql_v3_notes` does not match — the `_` before it is a + * word character, so there is no boundary). + * + * Deliberately unaware of SQL comments. A migration whose only mention of EQL + * is in a comment gets a warning it does not need; a migration whose dependency + * we miss gets a `supabase db reset` that fails with no explanation. The first + * costs a line of terminal output, the second costs a debugging session. + */ +const EQL_REFERENCE = /\beql_v3/ + +function referencesEql(path: string): boolean { + try { + return EQL_REFERENCE.test(readFileSync(path, 'utf-8')) + } catch { + // Unreadable, or a directory (EISDIR) — either way not a migration the + // Supabase CLI will run against EQL. + return false + } +} + +/** + * Migration filenames in `migrationsDir` that reference EQL and sort BEFORE the + * version this command's install file will carry. + * + * The failure this exists to catch: a project that ran `stash eql install` + * directly, added encrypted-column migrations against the EQL it put on the + * live database, and only then discovered #613 and ran `stash eql migration + * --supabase`. The install is stamped with today's date, so it sorts after + * those migrations. `supabase db reset` replays the directory in version order + * (`fs.ReadDir` order, filtered by `LoadPartialMigrations`) with no dependency + * awareness at all, so they run first, reference a domain nothing has created, + * and the reset fails. + * + * Version comparison is a plain string compare on the numeric prefix, matching + * both consumers in the Go CLI: `db reset` replays in `fs.ReadDir`'s + * filename order, and `FindPendingMigrations` compares version strings with + * `<`. Supabase versions are fixed-width timestamps, so the two agree. + * + * Returns names rather than paths: every hit is in `migrationsDir`, and the + * name is both what the user sees and what the ordering is computed from. + * Sorted, so `[0]` is the one to sort the install below. + */ +export function findEqlDependentMigrationsBefore( + migrationsDir: string, + options: { now?: Date } = {}, +): string[] { + const { now = new Date() } = options + + // The version the install will actually carry. A `--force` run overwrites the + // existing file IN PLACE and keeps its version, so comparing against the + // clock there would report dependants that in fact replay after it. + const existing = findExistingEqlMigration(migrationsDir) + const installVersion = + (existing && migrationVersionOf(basename(existing))) ?? + migrationVersion(now) + + let entries: string[] + try { + entries = readdirSync(migrationsDir) + } catch { + return [] + } + + return entries + .filter((entry) => { + // Our own install migrations are the thing being ordered, not something + // ordered against it — an older duplicate would otherwise be reported as + // a dependant of the file replacing it. + if (entry.endsWith(SUPABASE_EQL_MIGRATION_SUFFIX)) return false + const version = migrationVersionOf(entry) + return version !== null && version < installVersion + }) + .filter((entry) => referencesEql(join(migrationsDir, entry))) + .sort() +} + +/** + * Header prepended to the generated migration, for whoever opens + * `supabase/migrations/` in six months and finds 4,000 lines of EQL. + */ +function migrationHeader(): string { + return `-- CipherStash EQL v3 — generated by \`stash eql migration --supabase\`. +-- +-- Installs the CipherStash Encrypt Query Language (EQL) types, functions, and +-- operators into the \`eql_v3\` and \`eql_v3_internal\` schemas, grants Supabase's +-- \`anon\`, \`authenticated\`, and \`service_role\` roles access to them, and adds +-- the \`cipherstash.cs_migrations\` tracking schema that \`stash encrypt\` writes +-- its per-column progress into. +-- +-- Keeping the install here rather than applying it directly is what makes it +-- survive \`supabase db reset\` — a reset replays this directory, so EQL comes +-- back with everything else. +-- +-- Generated file: edit the bundle version by re-running the command with +-- --force rather than hand-editing this SQL. +-- +-- Docs: https://cipherstash.com/docs/stack/cipherstash/supabase +` +} + +/** + * Return the path of an EQL install migration already present in + * `migrationsDir`, or `null`. Lexically last wins, so the reported path is the + * newest when several exist. + */ +export function findExistingEqlMigration(migrationsDir: string): string | null { + if (!existsSync(migrationsDir)) return null + let entries: string[] + try { + entries = readdirSync(migrationsDir) + } catch { + return null + } + const matches = entries + .filter( + (entry) => + entry.endsWith(SUPABASE_EQL_MIGRATION_SUFFIX) && + // readdirSync returns directories too, and one named `…_cipherstash_eql.sql` + // would otherwise become the write target and fail with a raw EISDIR. + isFile(join(migrationsDir, entry)), + ) + .sort() + return matches.length > 0 + ? join(migrationsDir, matches[matches.length - 1]) + : null +} + +function isFile(path: string): boolean { + try { + return statSync(path).isFile() + } catch { + return false + } +} + +export interface WriteSupabaseEqlMigrationOptions { + /** + * Absolute path to the directory the migration should be written into. + * Created recursively when absent — `supabase init` makes it, but a project + * that has never run a migration may not have it yet. + */ + migrationsDir: string + /** The install SQL body (from `buildEqlV3MigrationSql({ supabase: true })`). */ + sql: string + /** + * Write even though an EQL install migration is already present. Without + * this the function throws rather than adding a second one. + */ + force?: boolean + /** Injectable clock, so tests can pin the generated filename. */ + now?: Date +} + +export interface WriteSupabaseEqlMigrationResult { + /** Absolute path to the migration written. */ + path: string + /** Whether this replaced an install migration that was already there. */ + overwritten: boolean + /** + * The `YYYYMMDDHHMMSS` the file carries — a `force` run keeps the original, + * so this is not derivable from the clock. It is the key the remote ledger + * (`supabase_migrations.schema_migrations`) records, hence what the re-apply + * guidance has to name. + * + * `null` only for an existing install file whose name has no numeric prefix, + * which the Supabase CLI skips entirely rather than applying. + */ + version: string | null +} + +/** + * Write `/_cipherstash_eql.sql`. + * + * A `force` run overwrites the existing install migration **in place**, keeping + * its original version. Writing a second, newer-versioned file instead would + * leave the first one applied and unremovable (deleting an applied migration + * desyncs `supabase_migrations.schema_migrations`), so the user would end up + * with two EQL installs in their history. Overwriting keeps it to one. + * + * @throws when an EQL install migration already exists and `force` is unset. + */ +export async function writeSupabaseEqlMigration( + options: WriteSupabaseEqlMigrationOptions, +): Promise { + const { migrationsDir, sql, force = false, now = new Date() } = options + + const existing = findExistingEqlMigration(migrationsDir) + if (existing && !force) { + throw new Error( + `An EQL install migration already exists at ${existing}. Re-run with --force to replace it, or delete that file first.`, + ) + } + + const targetPath = + existing ?? + join( + migrationsDir, + `${migrationVersion(now)}${SUPABASE_EQL_MIGRATION_SUFFIX}`, + ) + const body = `${migrationHeader()}\n${sql.trimEnd()}\n` + + await mkdir(migrationsDir, { recursive: true }) + + // Write to a sibling and rename, rather than straight to targetPath. The + // migrations directory is executed wholesale by `supabase db reset`, so a + // truncated file from an interrupted or failed write is not inert — it runs. + // The rename is atomic within the filesystem, and the temp name fails the + // Supabase CLI's `^([0-9]+)_(.*)\.sql$` filter twice over (leading dot, + // trailing `.tmp`), so a crash between the two leaves nothing that will ever + // be applied. Not nothing that will be SEEN, though: the CLI reads the whole + // directory and reports each rejected name — `Skipping migration + // ._cipherstash_eql.sql.tmp... (file name must match pattern + // "_name.sql")` on stderr, on every `db reset` and `db push` until + // someone deletes it. Inert and noisy, which is the trade we want. + const tempPath = join(migrationsDir, `.${basename(targetPath)}.tmp`) + try { + await writeFile(tempPath, body, 'utf-8') + await rename(tempPath, targetPath) + } catch (error) { + await rm(tempPath, { force: true }).catch(() => {}) + throw error + } + + return { + path: targetPath, + overwritten: existing !== null, + version: migrationVersionOf(basename(targetPath)), + } +} diff --git a/packages/cli/src/commands/init/__tests__/init-command.test.ts b/packages/cli/src/commands/init/__tests__/init-command.test.ts index cc56815a3..5a7ce68d6 100644 --- a/packages/cli/src/commands/init/__tests__/init-command.test.ts +++ b/packages/cli/src/commands/init/__tests__/init-command.test.ts @@ -2,7 +2,7 @@ import * as p from '@clack/prompts' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { CliExit } from '../../../cli/exit.js' import { messages } from '../../../messages.js' -import type { InitState } from '../types.js' +import type { InitProvider, InitState } from '../types.js' // `--region` is the non-interactive escape hatch for `stash init`; it must land // on `state.regionFlag` before the authenticate step runs (that step calls @@ -10,7 +10,12 @@ import type { InitState } from '../types.js' // the pipeline is inert and observable — `authenticateStep.run` is the spy we // assert on; the rest just pass state through. Also keeps native-loading steps // (`authenticate`, `install-deps`) out of the fast suite. -const authRun = vi.hoisted(() => vi.fn(async (state: InitState) => state)) +// Typed with the provider argument the pipeline actually passes, so the +// provider assertions below read it off the call directly instead of asserting +// their way past a one-element tuple type. +const authRun = vi.hoisted(() => + vi.fn(async (state: InitState, _provider: InitProvider) => state), +) const passthrough = { run: async (s: InitState) => s } // Controllable so the honest-summary tests can vary whether EQL installed. const eqlRun = vi.hoisted(() => @@ -85,10 +90,42 @@ describe('initCommand — integration flags', () => { expect(authRun).toHaveBeenCalledTimes(1) // Steps receive the resolved provider as their second argument; `--prisma` // must resolve to the Prisma Next provider whose referrer name is `prisma`. - const providerArg = authRun.mock.calls[0][1] as { name?: string } + const providerArg = authRun.mock.calls[0][1] expect(providerArg.name).toBe('prisma') }) + it('keeps the combined name for referrer tracking, and carries both flags on `selected`', async () => { + // Two contracts in one run. `name` is the referrer: `authenticateStep` + // passes it straight to `login()`, and `stash auth login --drizzle + // --supabase` records the same alphabetical 'drizzle-supabase' — a refactor + // that changes it silently changes attribution. `selected` is what every + // routing decision reads instead, precisely so nothing has to parse `name`. + await initCommand({ drizzle: true, supabase: true }, {}) + + const providerArg = authRun.mock.calls[0][1] + expect(providerArg.name).toBe('drizzle-supabase') + expect(providerArg.selected).toEqual(['supabase', 'drizzle']) + }) + + it('leaves a single-flag run with its plain provider name', async () => { + // The other side of the combined case: one flag must still produce the + // bare name (the referrer `stash auth login --supabase` records) and a + // one-element `selected`. + await initCommand({ supabase: true }, {}) + + const providerArg = authRun.mock.calls[0][1] + expect(providerArg.name).toBe('supabase') + expect(providerArg.selected).toEqual(['supabase']) + }) + + it('leaves a flagless run on the base provider with nothing selected', async () => { + await initCommand({}, {}) + + const providerArg = authRun.mock.calls[0][1] + expect(providerArg.name).toBe('base') + expect(providerArg.selected).toEqual([]) + }) + it('errors on the renamed `--prisma-next` flag before running any step', async () => { // `--prisma-next` was renamed to `--prisma`; init must fail loudly with // guidance rather than silently ignore a previously-documented flag. @@ -158,6 +195,152 @@ describe('initCommand — honest summary', () => { expect(body).not.toContain('✓ EQL extension installed') }) + it('points a Supabase migration run at supabase, not drizzle-kit', async () => { + // Regression: the apply-command branch read `state.integration`, which + // `detectIntegration` sets from the DATABASE_URL host — and a LOCAL + // Supabase stack is `127.0.0.1:54322`, so integration lands on + // 'postgresql' while the provider is 'supabase'. `installEqlStep` routes on + // either signal, so it generated a Supabase migration and the summary then + // told the user to run `drizzle-kit migrate`, contradicting the provider's + // own next-steps block a few lines later. That is exactly the local-dev + // user this feature targets. + eqlRun.mockImplementationOnce(async (s: InitState) => ({ + ...s, + integration: 'postgresql', + eqlInstalled: false, + eqlMigrationPending: true, + })) + + await expect(initCommand({ supabase: true }, {})).resolves.toBeUndefined() + + const summary = vi + .mocked(p.note) + .mock.calls.find(([, title]) => title === 'Setup complete') + const body = summary?.[0] as string + expect(body).toContain('EQL migration generated') + expect(body).toContain('supabase db reset') + expect(body).not.toContain('drizzle-kit migrate') + // This run really did write the file, so the verb must stay "generated". + expect(body).not.toContain('already present') + }) + + it('says "already present" — not "generated" — when init found the migration on disk', async () => { + // `installEqlStep` returns `eqlMigrationPending` for BOTH the migration it + // just wrote and one a previous run (or a standalone `stash eql migration + // --supabase`) left on disk. The apply guidance is identical either way — + // the file still has to be applied — but "EQL migration generated" over a + // run that generated nothing is a claim the user can disprove from their + // own diff. `eqlMigrationAlreadyPresent` carries the distinction. + eqlRun.mockImplementationOnce(async (s: InitState) => ({ + ...s, + // The local-Supabase shape: `detectIntegration` reads the host from the + // DATABASE_URL, and `127.0.0.1:54322` lands on 'postgresql' — only the + // provider says Supabase. + integration: 'postgresql', + eqlInstalled: false, + eqlMigrationPending: true, + eqlMigrationAlreadyPresent: true, + })) + + await expect(initCommand({ supabase: true }, {})).resolves.toBeUndefined() + + const summary = vi + .mocked(p.note) + .mock.calls.find(([, title]) => title === 'Setup complete') + expect(summary).toBeDefined() + const body = summary?.[0] as string + expect(body).toContain('EQL migration already present') + expect(body).not.toContain('generated') + // Unchanged from the freshly-generated case: the same apply guidance, the + // same successful exit. An already-present migration is not an incomplete + // setup, so no ✗ line and no non-zero exit. + expect(body).toContain('supabase db reset') + expect(body).not.toContain('✗ EQL extension NOT installed') + expect(vi.mocked(p.note)).not.toHaveBeenCalledWith( + expect.any(String), + messages.init.setupIncomplete, + ) + }) + + it('still points a Drizzle-on-Supabase run at drizzle-kit', async () => { + // The mirror image: `--supabase` is only the grants modifier there, and + // drizzle-kit owns the migration history, so the apply command is its own. + eqlRun.mockImplementationOnce(async (s: InitState) => ({ + ...s, + integration: 'drizzle', + eqlInstalled: false, + eqlMigrationPending: true, + })) + + await expect( + initCommand({ drizzle: true, supabase: true }, {}), + ).resolves.toBeUndefined() + + const summary = vi + .mocked(p.note) + .mock.calls.find(([, title]) => title === 'Setup complete') + const body = summary?.[0] as string + expect(body).toContain('drizzle-kit migrate') + expect(body).not.toContain('supabase db reset') + }) + + it('names drizzle-kit for a combined `--drizzle --supabase` run on a local Supabase stack', async () => { + // The same host-detection blind spot as the Supabase case above, but with + // both flags passed: `integration` lands on 'postgresql', so the apply-step + // routing has only the flags to go on. Reading them off the combined + // provider name ('drizzle-supabase') matched neither branch, so the run + // fell through to the drizzle-kit default for the wrong reason — right + // string, no reasoning behind it, and it would have printed `supabase db + // reset` the moment the default flipped. Drizzle wins here on purpose: it + // owns the migration history and `--supabase` is only the grants modifier. + eqlRun.mockImplementationOnce(async (s: InitState) => ({ + ...s, + integration: 'postgresql', + eqlInstalled: false, + eqlMigrationPending: true, + })) + + await expect( + initCommand({ drizzle: true, supabase: true }, {}), + ).resolves.toBeUndefined() + + const summary = vi + .mocked(p.note) + .mock.calls.find(([, title]) => title === 'Setup complete') + const body = summary?.[0] as string + expect(body).toContain('EQL migration generated') + expect(body).toContain('drizzle-kit migrate') + expect(body).not.toContain('supabase db reset') + }) + + it('names drizzle-kit for a combined run whose host says supabase', async () => { + // A Drizzle project on a HOSTED Supabase database: `detectIntegration` + // reads the supabase host and sets integration 'supabase', while the user + // passed both flags. `installEqlStep` writes the migration into the + // DRIZZLE folder (drizzle owns the history), so the summary has to say + // `drizzle-kit migrate`. Matching on the combined provider name made + // `isDrizzle` false, leaving only the integration signal — which says + // supabase — so the summary told the user to run `supabase db reset` over + // a migration that was never written into supabase/migrations/. + eqlRun.mockImplementationOnce(async (s: InitState) => ({ + ...s, + integration: 'supabase', + eqlInstalled: false, + eqlMigrationPending: true, + })) + + await expect( + initCommand({ drizzle: true, supabase: true }, {}), + ).resolves.toBeUndefined() + + const summary = vi + .mocked(p.note) + .mock.calls.find(([, title]) => title === 'Setup complete') + const body = summary?.[0] as string + expect(body).toContain('drizzle-kit migrate') + expect(body).not.toContain('supabase db reset') + }) + it('summary says "kept (existing file)" when an existing client is kept', async () => { // The three-way encryption-client checkmark fork was untested — the keep // path (`build-schema` sets clientFilePath + schemaGenerated: false) now diff --git a/packages/cli/src/commands/init/index.ts b/packages/cli/src/commands/init/index.ts index d44670920..ea45285f1 100644 --- a/packages/cli/src/commands/init/index.ts +++ b/packages/cli/src/commands/init/index.ts @@ -14,16 +14,27 @@ import { gatherContextStep } from './steps/gather-context.js' import { installDepsStep } from './steps/install-deps.js' import { installEqlStep } from './steps/install-eql.js' import { resolveDatabaseStep } from './steps/resolve-database.js' -import type { InitProvider, InitState } from './types.js' +import type { InitProvider, InitState, ProviderKey } from './types.js' import { CancelledError } from './types.js' import { detectPackageManager, runnerCommand } from './utils.js' -const PROVIDER_MAP: Record InitProvider> = { +/** + * The integration flags and the provider each selects. Declaration order is the + * tie-break for a multi-flag run: the first match supplies the UX (intro copy), + * and `provider.selected` lists the matches in this order — so anything + * iterating the selection is deterministic regardless of argv order. + */ +const PROVIDER_MAP: Record InitProvider> = { supabase: createSupabaseProvider, drizzle: createDrizzleProvider, prisma: createPrismaProvider, } +/** Derived from the map rather than written out again: a hand-maintained copy + * would let a new provider be added in one place only, and the flag would then + * silently do nothing. */ +const PROVIDER_KEYS = Object.keys(PROVIDER_MAP) as ProviderKey[] + /** * `stash init` does scaffold-once work only: auth, database connection, * schema introspection, dep install, EQL install, context gathering. It @@ -44,24 +55,43 @@ const STEPS = [ gatherContextStep, ] +/** + * Turn the integration flags into the provider the pipeline threads through + * every step. + * + * The flags are NOT mutually exclusive — `stash init --drizzle --supabase` is a + * real invocation (a Drizzle project on Supabase), and nothing upstream rejects + * it. Two separate things fall out of that, and conflating them was the bug: + * + * - `name` is the REFERRER. A multi-flag run joins every matched flag + * alphabetically, matching what `stash auth login --drizzle --supabase` + * records, and `authenticateStep` passes it to `login()`. + * - `selected` is the CAPABILITY SIGNAL. Because the combined name equals no + * single flag, every `provider.name === 'supabase'` test in the pipeline went + * false on a combined run: init installed EQL directly instead of writing a + * migration, skipped the Supabase grants, skipped the Prisma branch, and + * installed no adapter package. Steps read this list instead, so the two + * concerns can't drift back together. + */ function resolveProvider(flags: Record): InitProvider { - // When multiple flags are set, use the first matching provider but - // combine all flag names into the provider name for referrer tracking. - const matchedKeys = Object.keys(PROVIDER_MAP).filter((key) => flags[key]) + const matchedKeys = PROVIDER_KEYS.filter((key) => flags[key]) if (matchedKeys.length === 0) { return createBaseProvider() } - // Use the first matched provider for UX (intro message, connection options, etc.) + // The first matched provider supplies the UX (intro message). // matchedKeys[0] is guaranteed by the length check above; the optional chain // is just to satisfy biome's no-non-null-assertion rule. const factory = PROVIDER_MAP[matchedKeys[0]] const provider = factory ? factory() : createBaseProvider() - // Combine all matched flag names for the referrer + provider.selected = matchedKeys + // Combine all matched flag names for the referrer. Sorted on a COPY: sorting + // `matchedKeys` in place would reorder `selected` too, now that it is the + // same array. if (matchedKeys.length > 1) { - provider.name = matchedKeys.sort().join('-') + provider.name = [...matchedKeys].sort().join('-') } return provider @@ -135,24 +165,51 @@ export async function initCommand( if (state.eqlInstalled) { checkmarks.push('✓ EQL extension installed') } else if (state.eqlMigrationPending) { - // The Drizzle flow (and Supabase `--migration` mode) GENERATES an EQL - // migration rather than applying it — EQL isn't in the database until - // the user runs the migration. That's the intended, honest end state - // for these flows (applying is the ORM/migration tool's job), so it's - // NOT an incomplete setup — but we must not claim "installed" either. - const applyCmd = - state.integration === 'supabase' - ? 'supabase db push' - : 'drizzle-kit migrate' - checkmarks.push( - `○ EQL migration generated — apply it with \`${applyCmd}\``, - ) + // The Drizzle and Supabase flows GENERATE an EQL migration rather than + // applying it — EQL isn't in the database until the user runs the + // migration. That's the intended, honest end state for these flows + // (applying is the migration tool's job), so it's NOT an incomplete + // setup — but we must not claim "installed" either. + // + // Match on BOTH signals, exactly as `installEqlStep` routes. `integration` + // alone is wrong: `detectIntegration` reads it from the DATABASE_URL host, + // and a local Supabase stack is `127.0.0.1:54322` — so integration lands on + // 'postgresql' while the provider is 'supabase', and this printed + // `drizzle-kit migrate` at the very user the Supabase route targets. + // Drizzle wins when both fire: it owns the migration history there, and + // `--supabase` is only the grants modifier. + // + // The flag half reads `provider.selected`, never `provider.name` — a + // combined `--drizzle --supabase` run names itself 'drizzle-supabase', + // which is neither, so both halves went false and the apply step fell + // through to the drizzle-kit default with no reasoning behind it. + const isDrizzle = + state.integration === 'drizzle' || provider.selected.includes('drizzle') + const isSupabase = + state.integration === 'supabase' || + provider.selected.includes('supabase') + const applyStep = + isSupabase && !isDrizzle + ? 'apply it with `supabase db reset` (local) or `supabase db push` (remote)' + : 'apply it with `drizzle-kit migrate`' + // `eqlMigrationPending` covers two different runs that need the same + // apply guidance: the migration this run wrote, and one an earlier run + // (or a standalone `stash eql migration --supabase`) already left on + // disk. Only the verb differs — saying "generated" over the second is a + // claim about work this run did not do, and the user can disprove it + // from their own diff. `eqlMigrationAlreadyPresent` is deliberately not + // consulted by the `eqlPending` check below: either way a migration + // exists and the setup is complete. + const verb = state.eqlMigrationAlreadyPresent + ? 'already present' + : 'generated' + checkmarks.push(`○ EQL migration ${verb} — ${applyStep}`) } // EQL is required for encryption. Some integrations install it out-of-band // and legitimately leave `eqlInstalled` false here: Prisma Next installs it - // via `prisma-next migrate`, and the Drizzle flow generates a migration the - // user applies with `drizzle-kit migrate` (`eqlMigrationPending`). Only a + // via `prisma-next migrate`, and the Drizzle and Supabase flows generate a + // migration the user applies themselves (`eqlMigrationPending`). Only a // run that neither installed EQL nor generated a migration to install it is // genuinely incomplete — say so and exit non-zero so automation can't read // a false success from a run where encryption would fail at query time. diff --git a/packages/cli/src/commands/init/lib/setup-prompt.ts b/packages/cli/src/commands/init/lib/setup-prompt.ts index a74cac5a0..ec89a26e1 100644 --- a/packages/cli/src/commands/init/lib/setup-prompt.ts +++ b/packages/cli/src/commands/init/lib/setup-prompt.ts @@ -62,7 +62,9 @@ function migrationCommands( return { tool: 'Supabase CLI', generate: 'supabase migration new ', - apply: 'supabase migration up (remote) or supabase db reset (local)', + // A bare `supabase migration up` targets the LOCAL database; the remote + // forms are `db push` and `migration up --linked`. + apply: 'supabase db push (remote) or supabase db reset (local)', } } return undefined @@ -507,7 +509,10 @@ function planSharedNotDoBlock(ctx: SetupPromptContext): string[] { `Run \`${cli} encrypt backfill\`, \`${cli} encrypt drop\`, or any other state-mutating command.`, ), bullet( - 'Run schema migrations (`drizzle-kit migrate`, `supabase migration up`, `prisma migrate`, etc.).', + // `supabase db reset` rather than `migration up`: it is the command an + // agent on a local Supabase project would actually reach for, and it + // drops the database — the most important one in a do-not-run list. + 'Run schema migrations (`drizzle-kit migrate`, `supabase db reset`, `prisma migrate`, etc.).', ), bullet( 'Modify the placeholder encryption client beyond what is required to read it.', diff --git a/packages/cli/src/commands/init/providers/__tests__/supabase.test.ts b/packages/cli/src/commands/init/providers/__tests__/supabase.test.ts index 69304e524..c552c52c7 100644 --- a/packages/cli/src/commands/init/providers/__tests__/supabase.test.ts +++ b/packages/cli/src/commands/init/providers/__tests__/supabase.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest' +import type { InitState } from '../../types.js' import { createSupabaseProvider } from '../supabase.js' describe('createSupabaseProvider getNextSteps', () => { @@ -7,14 +8,14 @@ describe('createSupabaseProvider getNextSteps', () => { it('uses npx when package manager is npm', () => { const steps = provider.getNextSteps({}, 'npm') expect(steps[0]).toBe( - 'Install EQL: npx stash eql install --supabase (prompts for migration vs direct)', + 'Install EQL: npx stash eql migration --supabase (writes it into supabase/migrations/)', ) }) it('uses bunx when package manager is bun', () => { const steps = provider.getNextSteps({}, 'bun') expect(steps[0]).toBe( - 'Install EQL: bunx stash eql install --supabase (prompts for migration vs direct)', + 'Install EQL: bunx stash eql migration --supabase (writes it into supabase/migrations/)', ) expect(steps[2]).toContain('bunx stash wizard') // wizard step is third for (const s of steps) expect(s).not.toMatch(/\bnpx\b/) @@ -22,23 +23,58 @@ describe('createSupabaseProvider getNextSteps', () => { it('uses pnpm dlx when package manager is pnpm', () => { const steps = provider.getNextSteps({}, 'pnpm') - expect(steps[0]).toContain('pnpm dlx stash eql install --supabase') + expect(steps[0]).toContain('pnpm dlx stash eql migration --supabase') }) it('uses yarn dlx when package manager is yarn', () => { const steps = provider.getNextSteps({}, 'yarn') expect(steps[0]).toBe( - 'Install EQL: yarn dlx stash eql install --supabase (prompts for migration vs direct)', + 'Install EQL: yarn dlx stash eql migration --supabase (writes it into supabase/migrations/)', ) expect(steps[2]).toContain('yarn dlx stash wizard') // Sanity: the supabase CLI commands stay untouched. expect(steps.join('\n')).toContain('supabase db reset') - expect(steps.join('\n')).toContain('supabase migration up') + expect(steps.join('\n')).toContain('supabase db push') }) it('leaves the supabase CLI commands alone (those are not npm packages)', () => { const steps = provider.getNextSteps({}, 'bun') expect(steps.join('\n')).toContain('supabase db reset') - expect(steps.join('\n')).toContain('supabase migration up') + expect(steps.join('\n')).toContain('supabase db push') + }) + + it('never pairs a direct `eql install` with `supabase db reset` (#613)', () => { + // That pairing was the defect: these steps told the user to install EQL + // directly and then run the one command that drops it. Whatever the + // wording, a direct install must not appear alongside a reset. + for (const state of [{}, { eqlMigrationPending: true } as InitState]) { + const joined = provider.getNextSteps(state, 'npm').join('\n') + expect(joined).not.toContain('eql install') + } + }) + + it('says only "apply it" once init has already generated the migration', () => { + // init writes the migration itself on this path, so repeating the generate + // step would have the user run a command that then refuses (one install + // migration already exists). + const steps = provider.getNextSteps( + { eqlMigrationPending: true } as InitState, + 'npm', + ) + + expect(steps[0]).toBe( + 'Apply the generated EQL migration: supabase db reset (local) or supabase db push (remote/linked)', + ) + expect(steps.join('\n')).not.toContain('eql migration --supabase') + }) + + it('never sends a remote apply to a bare `supabase migration up`', async () => { + // That form targets the LOCAL database — the remote ones are `db push` and + // `migration up --linked`. Telling a user it is the remote command means + // their production database silently never gets EQL. + for (const state of [{}, { eqlMigrationPending: true } as InitState]) { + const joined = provider.getNextSteps(state, 'npm').join('\n') + expect(joined).not.toMatch(/supabase migration up(?! --linked)/) + } }) }) diff --git a/packages/cli/src/commands/init/providers/base.ts b/packages/cli/src/commands/init/providers/base.ts index 47064d530..7cf38d2ae 100644 --- a/packages/cli/src/commands/init/providers/base.ts +++ b/packages/cli/src/commands/init/providers/base.ts @@ -4,6 +4,7 @@ import { type PackageManager, runnerCommand } from '../utils.js' export function createBaseProvider(): InitProvider { return { name: 'base', + selected: [], introMessage: 'Setting up CipherStash for your project...', getNextSteps(state: InitState, pm: PackageManager): string[] { const cli = runnerCommand(pm, 'stash') diff --git a/packages/cli/src/commands/init/providers/drizzle.ts b/packages/cli/src/commands/init/providers/drizzle.ts index 3516491cc..b81b27b03 100644 --- a/packages/cli/src/commands/init/providers/drizzle.ts +++ b/packages/cli/src/commands/init/providers/drizzle.ts @@ -4,6 +4,7 @@ import { type PackageManager, runnerCommand } from '../utils.js' export function createDrizzleProvider(): InitProvider { return { name: 'drizzle', + selected: ['drizzle'], introMessage: 'Setting up CipherStash for your Drizzle project...', getNextSteps(state: InitState, pm: PackageManager): string[] { const cli = runnerCommand(pm, 'stash') diff --git a/packages/cli/src/commands/init/providers/prisma.ts b/packages/cli/src/commands/init/providers/prisma.ts index aa6233e27..62f0568ac 100644 --- a/packages/cli/src/commands/init/providers/prisma.ts +++ b/packages/cli/src/commands/init/providers/prisma.ts @@ -12,6 +12,7 @@ import { type PackageManager, runnerCommand } from '../utils.js' export function createPrismaProvider(): InitProvider { return { name: 'prisma', + selected: ['prisma'], introMessage: 'Setting up CipherStash for your Prisma Next project...', // Note: Prisma Next absorbs the EQL bundle install and schema // scaffold steps via its migration framework. The next-steps list diff --git a/packages/cli/src/commands/init/providers/supabase.ts b/packages/cli/src/commands/init/providers/supabase.ts index d8b824d9b..391004979 100644 --- a/packages/cli/src/commands/init/providers/supabase.ts +++ b/packages/cli/src/commands/init/providers/supabase.ts @@ -4,13 +4,24 @@ import { type PackageManager, runnerCommand } from '../utils.js' export function createSupabaseProvider(): InitProvider { return { name: 'supabase', + selected: ['supabase'], introMessage: 'Setting up CipherStash for your Supabase project...', getNextSteps(state: InitState, pm: PackageManager): string[] { const cli = runnerCommand(pm, 'stash') - const steps = [ - `Install EQL: ${cli} eql install --supabase (prompts for migration vs direct)`, - 'Apply it: supabase db reset (local) or supabase migration up (remote)', - ] + // Migration-first, always. A direct `eql install` does not survive + // `supabase db reset` — the reset drops the database and replays + // supabase/migrations/, so an install that isn't in there is gone. + // `supabase db push` for remote, not a bare `supabase migration up` — + // that applies to the LOCAL database (the remote forms are `db push` and + // `migration up --linked`). + const apply = + 'supabase db reset (local) or supabase db push (remote/linked)' + const steps = state.eqlMigrationPending + ? [`Apply the generated EQL migration: ${apply}`] + : [ + `Install EQL: ${cli} eql migration --supabase (writes it into supabase/migrations/)`, + `Apply it: ${apply}`, + ] const manualEdit = state.clientFilePath ? `edit ${state.clientFilePath} directly` diff --git a/packages/cli/src/commands/init/steps/__tests__/build-schema.test.ts b/packages/cli/src/commands/init/steps/__tests__/build-schema.test.ts index 4e1116bfe..2268a52a6 100644 --- a/packages/cli/src/commands/init/steps/__tests__/build-schema.test.ts +++ b/packages/cli/src/commands/init/steps/__tests__/build-schema.test.ts @@ -40,7 +40,7 @@ import { buildSchemaStep } from '../build-schema.js' const baseState = { databaseUrl: 'postgresql://localhost:5432/app', } as unknown as InitState -const provider = { name: 'postgresql' } as unknown as InitProvider +const provider = { name: 'base', selected: [] } as unknown as InitProvider describe('buildSchemaStep', () => { beforeEach(() => { @@ -84,7 +84,10 @@ describe('buildSchemaStep', () => { // value stays 'prisma-next' so skill/dep/prompt wiring is unchanged. // Prisma Next derives its schema from contract.json, so there is no // placeholder client to write. - const prismaProvider = { name: 'prisma' } as unknown as InitProvider + const prismaProvider = { + name: 'prisma', + selected: ['prisma'], + } as unknown as InitProvider const result = await buildSchemaStep.run(baseState, prismaProvider) @@ -92,4 +95,23 @@ describe('buildSchemaStep', () => { expect(result.schemaGenerated).toBe(false) expect(writeFileSyncMock).not.toHaveBeenCalled() }) + + it('still forces prisma-next when `--prisma` is combined with another flag', async () => { + // `stash init --prisma --supabase` joins the flags into a single provider + // name for referrer tracking — 'prisma-supabase', which is not 'prisma'. + // Reading that name here dropped the run onto `detectIntegration`, and a + // fresh project with no prisma-next config detects as raw postgres: init + // then wrote a placeholder client Prisma Next never uses and reported an + // integration the rest of the pipeline routes on. + const prismaSupabase = { + name: 'prisma-supabase', + selected: ['supabase', 'prisma'], + } as unknown as InitProvider + + const result = await buildSchemaStep.run(baseState, prismaSupabase) + + expect(result.integration).toBe('prisma-next') + expect(result.schemaGenerated).toBe(false) + expect(writeFileSyncMock).not.toHaveBeenCalled() + }) }) diff --git a/packages/cli/src/commands/init/steps/__tests__/install-deps.test.ts b/packages/cli/src/commands/init/steps/__tests__/install-deps.test.ts index 0f069c5a0..b4a4340e8 100644 --- a/packages/cli/src/commands/init/steps/__tests__/install-deps.test.ts +++ b/packages/cli/src/commands/init/steps/__tests__/install-deps.test.ts @@ -29,6 +29,7 @@ const FIXTURE_VERSIONS: Record = vi.hoisted(() => ({ stash: '9.9.9-test.1', '@cipherstash/stack': '9.9.9-test.1', '@cipherstash/stack-supabase': '9.9.9-test.1', + '@cipherstash/stack-drizzle': '9.9.9-test.1', })) vi.mock('../../../../runtime-versions.js', async (importOriginal) => ({ // Keep the real pure helpers (compareVersions, parseEmbeddedVersions); @@ -71,8 +72,17 @@ import { import { installDepsStep, versionSkew } from '../install-deps.js' const baseState = {} as unknown as InitState -const provider = { name: 'postgresql' } as unknown as InitProvider -const supabaseProvider = { name: 'supabase' } as unknown as InitProvider +const provider = { name: 'base', selected: [] } as unknown as InitProvider +const supabaseProvider = { + name: 'supabase', + selected: ['supabase'], +} as unknown as InitProvider +/** What `resolveProvider` builds for `stash init --drizzle --supabase`: a + * combined `name` for referrer tracking, both flags on `selected`. */ +const drizzleSupabaseProvider = { + name: 'drizzle-supabase', + selected: ['supabase', 'drizzle'], +} as unknown as InitProvider /** Presence by package name — clearer and more robust than call counters. */ function present(...pkgs: string[]) { @@ -157,6 +167,89 @@ describe('installDepsStep', () => { expect(dev).toEqual(['stash@9.9.9-test.1']) }) + it('installs no adapter package for a flagless run', async () => { + // The baseline the combined-flag cases below are measured against: a plain + // Postgres project has no adapter to install. + await installDepsStep.run(baseState, provider) + + const [, prod, dev] = installCall() + expect(prod).toEqual(['@cipherstash/stack@9.9.9-test.1']) + expect(dev).toEqual(['stash@9.9.9-test.1']) + }) + + it('installs BOTH adapters for a combined `--drizzle --supabase` run', async () => { + // The adapter was looked up by provider NAME, and a combined run's name is + // 'drizzle-supabase' — not a key of the adapter map, so the run installed + // neither adapter and the scaffolded client's imports could not resolve. + // The user asked for both integrations; both packages are real and both + // are needed. + await installDepsStep.run(baseState, drizzleSupabaseProvider) + + const [, prod, dev] = installCall() + expect(prod).toEqual([ + '@cipherstash/stack@9.9.9-test.1', + '@cipherstash/stack-supabase@9.9.9-test.1', + '@cipherstash/stack-drizzle@9.9.9-test.1', + ]) + expect(dev).toEqual(['stash@9.9.9-test.1']) + }) + + it('lists an adapter once when the detected integration and the flag agree', async () => { + // `state.integration` and `provider.selected` both say Supabase on a + // hosted Supabase project. The package list is deduped, so the install + // command names the adapter once. + await installDepsStep.run( + { integration: 'supabase' } as unknown as InitState, + supabaseProvider, + ) + + const [, prod] = installCall() + expect(prod).toEqual([ + '@cipherstash/stack@9.9.9-test.1', + '@cipherstash/stack-supabase@9.9.9-test.1', + ]) + }) + + it('resolves the Prisma adapter from the flag alone', async () => { + // `--prisma` names the flag; the integration it selects is `prisma-next`. + // Resolving the adapter straight from the provider name missed that, and + // the package only got installed because `build-schema` happens to run + // first and put 'prisma-next' on state. Step ordering is not the contract. + const prismaProvider = { + name: 'prisma', + selected: ['prisma'], + } as unknown as InitProvider + + await installDepsStep.run(baseState, prismaProvider) + + const [, prod] = installCall() + expect(prod).toContain('@cipherstash/stack-prisma') + }) + + it('names every adapter in the already-installed line', async () => { + present( + '@cipherstash/stack', + '@cipherstash/stack-supabase', + '@cipherstash/stack-drizzle', + 'stash', + ) + resolvedVersions({ + '@cipherstash/stack': FIXTURE_VERSIONS['@cipherstash/stack'], + '@cipherstash/stack-supabase': + FIXTURE_VERSIONS['@cipherstash/stack-supabase'], + '@cipherstash/stack-drizzle': + FIXTURE_VERSIONS['@cipherstash/stack-drizzle'], + stash: FIXTURE_VERSIONS.stash, + }) + + await installDepsStep.run(baseState, drizzleSupabaseProvider) + + expect(p.log.success).toHaveBeenCalledWith( + '@cipherstash/stack, @cipherstash/stack-supabase, @cipherstash/stack-drizzle and stash are already installed.', + ) + expect(execSyncMock).not.toHaveBeenCalled() + }) + it('warns on version skew and aligns with the dev/prod split intact (#661)', async () => { // The dist-tag failure mode: node_modules holds stale versions of both // the runtime package (prod) and the CLI (dev). diff --git a/packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts b/packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts index 2c89b7b99..ed7cad6bc 100644 --- a/packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts +++ b/packages/cli/src/commands/init/steps/__tests__/install-eql.test.ts @@ -4,10 +4,30 @@ import type { InitProvider, InitState } from '../../types.js' // installCommand is the unit under test's collaborator — mock it so we assert // what init asks for without touching a database. vi.mock('../../../db/install.js', () => ({ installCommand: vi.fn() })) -// The Drizzle branch generates a v3 migration instead of calling installCommand. +// The Drizzle and Supabase branches generate a v3 migration instead of calling +// installCommand. vi.mock('../../../eql/migration.js', () => ({ eqlMigrationCommand: vi.fn(async () => undefined), })) +// Whether a Supabase project has local `supabase/` scaffolding decides between +// the migration and direct-install routes. Real detection walks the cwd (this +// package), which has neither — so toggle it per test. +vi.mock('../../../db/detect.js', async (importOriginal) => ({ + // Spread the original: replacing the whole module would leave + // detectSupabase / detectDrizzle / detectPrismaNext undefined for anything + // else that imports it. Nothing needs them today; this keeps it that way. + ...(await importOriginal()), + detectSupabaseProject: vi.fn(() => ({ + hasConfigToml: false, + hasMigrationsDir: false, + migrationsDir: '/project/supabase/migrations', + })), +})) +// Whether an install migration is already on disk decides between generating +// one and reporting the existing one as pending. +vi.mock('../../../eql/supabase-migration.js', () => ({ + findExistingEqlMigration: vi.fn(() => null), +})) // `eql install` normally scaffolds these; the Drizzle branch does it itself. vi.mock('../../../db/config-scaffold.js', () => ({ offerStashConfig: vi.fn(async () => 'src/encryption/index.ts'), @@ -34,26 +54,77 @@ import { CliExit } from '../../../../cli/exit.js' import { isInteractive } from '../../../../config/tty.js' import { ensureEncryptionClient } from '../../../db/client-scaffold.js' import { offerStashConfig } from '../../../db/config-scaffold.js' +import { detectSupabaseProject } from '../../../db/detect.js' import { installCommand } from '../../../db/install.js' import { eqlMigrationCommand } from '../../../eql/migration.js' +import { findExistingEqlMigration } from '../../../eql/supabase-migration.js' import { installEqlStep } from '../install-eql.js' +/** Pretend the cwd has (or lacks) `supabase init` scaffolding. */ +function withSupabaseScaffolding(present: boolean): void { + vi.mocked(detectSupabaseProject).mockReturnValue({ + hasConfigToml: present, + hasMigrationsDir: present, + migrationsDir: '/project/supabase/migrations', + }) +} + +/** The message the step put in front of the user before acting. */ +function confirmMessage(): string { + return vi.mocked(p.confirm).mock.calls[0][0].message +} + +/** Every `p.note` body, joined — the step emits at most one per run. */ +function noteBody(): string { + return vi + .mocked(p.note) + .mock.calls.map(([body]) => body) + .join('\n') +} + +const supabaseState = { + integration: 'supabase', + databaseUrl: 'postgresql://localhost:54322/postgres', +} as unknown as InitState +const supabaseProvider = { + name: 'supabase', + selected: ['supabase'], +} as unknown as InitProvider + const drizzleState = { integration: 'drizzle', databaseUrl: 'postgresql://localhost:5432/app', } as unknown as InitState -const drizzleProvider = { name: 'drizzle' } as unknown as InitProvider +const drizzleProvider = { + name: 'drizzle', + selected: ['drizzle'], +} as unknown as InitProvider const baseState = { integration: 'postgresql', databaseUrl: 'postgresql://localhost:5432/app', } as unknown as InitState -const provider = { name: 'postgresql' } as unknown as InitProvider +const provider = { name: 'base', selected: [] } as unknown as InitProvider + +/** + * What `resolveProvider` hands the steps for `stash init --drizzle --supabase`: + * the first matched provider supplies the UX, `name` combines every matched + * flag for referrer tracking, and `selected` carries the flags themselves. + * `name` is deliberately neither 'drizzle' nor 'supabase' — that is exactly the + * shape the equality checks used to fall through on. + */ +const drizzleSupabaseProvider = { + name: 'drizzle-supabase', + selected: ['supabase', 'drizzle'], +} as unknown as InitProvider describe('installEqlStep', () => { beforeEach(() => { vi.clearAllMocks() vi.mocked(isInteractive).mockReturnValue(true) + // clearAllMocks clears calls but keeps implementations, so a + // mockReturnValue set in one test would leak into every later one. + vi.mocked(findExistingEqlMigration).mockReturnValue(null) }) it("requests scaffoldConfig: 'ensure' so init still creates a stash.config.ts (#581 regression)", async () => { @@ -155,6 +226,9 @@ describe('installEqlStep', () => { expect(result.eqlInstalled).toBe(false) expect(result.eqlMigrationPending).toBe(true) + // This run really did generate it, so the summary's verb must stay + // "generated" — only the already-on-disk branch sets this flag. + expect(result.eqlMigrationAlreadyPresent).toBeFalsy() }) it('scaffolds stash.config.ts + the client, which `eql install` would have done (#581)', async () => { @@ -171,7 +245,7 @@ describe('installEqlStep', () => { it('forwards --supabase so a Drizzle-on-Supabase project gets the role grants', async () => { await installEqlStep.run( { ...drizzleState, integration: 'drizzle' } as InitState, - { name: 'supabase' } as unknown as InitProvider, + supabaseProvider, ) expect(vi.mocked(eqlMigrationCommand).mock.calls[0][0].supabase).toBe( @@ -241,6 +315,402 @@ describe('installEqlStep', () => { }) }) + describe('Supabase', () => { + it('writes the install into supabase/migrations/ so it survives `db reset` (#613)', async () => { + // The defect: init ran a direct `eql install`, and `supabase db reset` — + // the ordinary local development loop — drops the database and replays + // supabase/migrations/, taking EQL with it. The install has to be IN that + // directory to come back. + withSupabaseScaffolding(true) + + await installEqlStep.run(supabaseState, supabaseProvider) + + expect(installCommand).not.toHaveBeenCalled() + expect(eqlMigrationCommand).toHaveBeenCalledTimes(1) + expect(vi.mocked(eqlMigrationCommand).mock.calls[0][0]).toMatchObject({ + supabase: true, + embedded: true, + }) + // Not a Drizzle run — `--drizzle` would shell out to drizzle-kit, which + // a plain Supabase project does not have. + expect( + vi.mocked(eqlMigrationCommand).mock.calls[0][0].drizzle, + ).toBeFalsy() + }) + + it('maps the generated migration to eqlMigrationPending, NOT eqlInstalled', async () => { + withSupabaseScaffolding(true) + + const result = await installEqlStep.run(supabaseState, supabaseProvider) + + expect(result.eqlInstalled).toBe(false) + expect(result.eqlMigrationPending).toBe(true) + }) + + it('scaffolds stash.config.ts + the client, which `eql install` would have done', async () => { + withSupabaseScaffolding(true) + + await installEqlStep.run(supabaseState, supabaseProvider) + + expect(offerStashConfig).toHaveBeenCalledWith({ ensure: true }) + expect(ensureEncryptionClient).toHaveBeenCalledTimes(1) + }) + + it('installs directly when the project has no local supabase/ scaffolding', async () => { + // A project pointed at a hosted Supabase database with no `supabase init` + // has nowhere to write a migration and no `supabase` binary to apply one. + // Routing it to the migration path would leave EQL uninstalled. + withSupabaseScaffolding(false) + + const result = await installEqlStep.run(supabaseState, supabaseProvider) + + expect(eqlMigrationCommand).not.toHaveBeenCalled() + expect(installCommand).toHaveBeenCalledTimes(1) + expect(vi.mocked(installCommand).mock.calls[0][0].supabase).toBe(true) + expect(result.eqlInstalled).toBe(true) + }) + + it('degrades to "not installed" (never crashes init) when the write fails', async () => { + withSupabaseScaffolding(true) + vi.mocked(eqlMigrationCommand).mockRejectedValueOnce(new Error('boom')) + + const result = await installEqlStep.run(supabaseState, supabaseProvider) + + expect(result.eqlInstalled).toBe(false) + expect(result.eqlMigrationPending).toBeFalsy() + }) + + it('does not leak the database URL when the write fails', async () => { + withSupabaseScaffolding(true) + vi.mocked(eqlMigrationCommand).mockRejectedValueOnce( + new Error('connect postgresql://user:hunter2@localhost:54322/postgres'), + ) + + await installEqlStep.run(supabaseState, supabaseProvider) + + const logged = [ + ...vi.mocked(p.log.error).mock.calls, + ...vi.mocked(p.note).mock.calls, + ] + .flat() + .join('\n') + expect(logged).not.toContain('hunter2') + }) + + it('re-running init over an existing install migration is a no-op, not a failure', async () => { + // Regression: `eql migration --supabase` refuses to write a second + // install migration, so a second `stash init --supabase` used to take the + // catch branch, return no `eqlMigrationPending`, and make initCommand + // report "✗ EQL extension NOT installed" and exit 1 — pointing the user + // at the direct `stash eql install` this route exists to avoid. Nothing + // is wrong with the project: the migration is right there. + withSupabaseScaffolding(true) + vi.mocked(findExistingEqlMigration).mockReturnValue( + '/project/supabase/migrations/20260804021925_cipherstash_eql.sql', + ) + + const result = await installEqlStep.run(supabaseState, supabaseProvider) + + expect(eqlMigrationCommand).not.toHaveBeenCalled() + expect(installCommand).not.toHaveBeenCalled() + expect(result.eqlMigrationPending).toBe(true) + expect(result.eqlInstalled).toBe(false) + }) + + it('marks the pending migration as already present, not freshly generated', async () => { + // `eqlMigrationPending` alone cannot tell "written this run" from "found + // on disk": both routes set it, and both want the same apply guidance. + // Without a second signal `initCommand` prints "EQL migration generated" + // over a run that generated nothing — a claim the user can disprove from + // their own diff. + withSupabaseScaffolding(true) + vi.mocked(findExistingEqlMigration).mockReturnValue( + '/project/supabase/migrations/20260804021925_cipherstash_eql.sql', + ) + + const result = await installEqlStep.run(supabaseState, supabaseProvider) + + expect(result.eqlMigrationAlreadyPresent).toBe(true) + }) + + it('does not mark a migration it wrote this run as already present', async () => { + // The symmetric negative: `findExistingEqlMigration` returns null, the + // step writes the file, and the summary must still say "generated". + withSupabaseScaffolding(true) + + const result = await installEqlStep.run(supabaseState, supabaseProvider) + + expect(result.eqlMigrationPending).toBe(true) + expect(result.eqlMigrationAlreadyPresent).toBeFalsy() + }) + + it('names the existing migration so the user knows what to apply', async () => { + withSupabaseScaffolding(true) + vi.mocked(findExistingEqlMigration).mockReturnValue( + '/project/supabase/migrations/20260804021925_cipherstash_eql.sql', + ) + + await installEqlStep.run(supabaseState, supabaseProvider) + + const logged = vi + .mocked(p.log.info) + .mock.calls.flat() + .concat(vi.mocked(p.log.success).mock.calls.flat()) + .join('\n') + expect(logged).toContain('20260804021925_cipherstash_eql.sql') + }) + + it('still scaffolds stash.config.ts when the migration is already there', async () => { + // The scaffolding is not the migration's job — `eql migration` writes SQL + // and nothing else, so init supplies the config and client every other + // route gets (the #581 contract). Skipping the generate call must not + // skip that too: a project whose migration came from a standalone `stash + // eql migration --supabase` has never had a stash.config.ts written, and + // init would report "Setup complete" over a project that cannot load one. + withSupabaseScaffolding(true) + vi.mocked(findExistingEqlMigration).mockReturnValue( + '/project/supabase/migrations/20260804021925_cipherstash_eql.sql', + ) + + await installEqlStep.run(supabaseState, supabaseProvider) + + expect(offerStashConfig).toHaveBeenCalledWith({ ensure: true }) + expect(ensureEncryptionClient).toHaveBeenCalledTimes(1) + }) + + it('keeps a Supabase-hosted Drizzle project on the Drizzle route', async () => { + // Both signals are true here. Drizzle owns the migration history, so it + // must win — `--supabase` degrades to the grants modifier it has always + // been on that path. + withSupabaseScaffolding(true) + + await installEqlStep.run( + { ...drizzleState, integration: 'drizzle' } as InitState, + supabaseProvider, + ) + + expect(vi.mocked(eqlMigrationCommand).mock.calls[0][0]).toMatchObject({ + drizzle: true, + supabase: true, + }) + }) + }) + + describe('combined integration flags (`--drizzle --supabase`)', () => { + // The CLI accepts both flags (nothing rejects the combination), and + // `resolveProvider` then joins them into a single `name` for referrer + // tracking. Every routing decision here reads the FLAGS, never that name — + // a name of 'drizzle-supabase' equals neither 'drizzle' nor 'supabase', so + // matching on it sent a combined run down the direct-install path. + + it('routes a LOCAL Supabase + Drizzle project to the Drizzle migration, not a direct install (#613)', async () => { + // The reported failure. `detectIntegration` reads the DATABASE_URL host + // and a local Supabase stack is 127.0.0.1:54322, so `state.integration` + // lands on 'postgresql' — the flags are the only signal left. With both + // going false, `resolveMigrationRoute` returned null and init installed + // EQL directly: no migration file, no Supabase role grants, and the next + // `supabase db reset` wipes the install. + // + // No local `supabase/` scaffolding here on purpose: the Drizzle route + // does not need it, so this also pins that the route is Drizzle's. + withSupabaseScaffolding(false) + + const result = await installEqlStep.run( + { + integration: 'postgresql', + databaseUrl: 'postgresql://127.0.0.1:54322/postgres', + } as unknown as InitState, + drizzleSupabaseProvider, + ) + + expect(installCommand).not.toHaveBeenCalled() + expect(eqlMigrationCommand).toHaveBeenCalledTimes(1) + expect(vi.mocked(eqlMigrationCommand).mock.calls[0][0]).toMatchObject({ + drizzle: true, + // `--supabase` is the grants modifier on the Drizzle route: without it + // the migration omits the anon/authenticated/service_role grants. + supabase: true, + embedded: true, + }) + expect(result.eqlMigrationPending).toBe(true) + expect(result.eqlInstalled).toBe(false) + }) + + it('takes the same route when the integration was detected as supabase', async () => { + withSupabaseScaffolding(true) + + await installEqlStep.run(supabaseState, drizzleSupabaseProvider) + + expect(installCommand).not.toHaveBeenCalled() + expect(vi.mocked(eqlMigrationCommand).mock.calls[0][0]).toMatchObject({ + drizzle: true, + supabase: true, + }) + }) + + it('takes the same route when the integration was detected as drizzle', async () => { + await installEqlStep.run(drizzleState, drizzleSupabaseProvider) + + expect(installCommand).not.toHaveBeenCalled() + expect(vi.mocked(eqlMigrationCommand).mock.calls[0][0]).toMatchObject({ + drizzle: true, + supabase: true, + }) + }) + + it('offers the migration prompt, not the database-install prompt', async () => { + // The prompt must describe the route the run is actually on — the + // combined run used to be asked about installing into the database and + // then... installed into the database, which at least agreed with itself + // but was the wrong route. + withSupabaseScaffolding(false) + + await installEqlStep.run( + { integration: 'postgresql' } as unknown as InitState, + drizzleSupabaseProvider, + ) + + expect(confirmMessage()).toMatch(/migration/i) + expect(confirmMessage()).not.toContain( + 'Install the EQL extension into your database', + ) + }) + + it('keeps the Prisma skip when `--prisma` is combined with another flag', async () => { + // `--prisma --supabase` joins to 'prisma-supabase', which is not + // 'prisma', so the skip fell through and init ran a duplicate install + // that races `prisma-next migrate`'s journal. + const prismaSupabase = { + name: 'prisma-supabase', + selected: ['supabase', 'prisma'], + } as unknown as InitProvider + + const result = await installEqlStep.run( + { integration: 'postgresql' } as unknown as InitState, + prismaSupabase, + ) + + expect(p.confirm).not.toHaveBeenCalled() + expect(installCommand).not.toHaveBeenCalled() + expect(eqlMigrationCommand).not.toHaveBeenCalled() + expect(result.eqlInstalled).toBe(false) + }) + }) + + describe('the confirm prompt names the action the route will take', () => { + // The prompt is the user's only description of what pressing `y` does, and + // two of the three routes never touch the database — they write a file. + // "Install the EQL extension into your database now?" followed by a + // generated migration described the wrong action on both of them. + + it('offers a database install on the direct route', async () => { + await installEqlStep.run(baseState, provider) + + expect(confirmMessage()).toContain( + 'Install the EQL extension into your database', + ) + expect(confirmMessage()).toContain('(required for encryption)') + }) + + it('offers a generated migration on the Drizzle route', async () => { + await installEqlStep.run(drizzleState, drizzleProvider) + + expect(confirmMessage()).toMatch(/migration/i) + expect(confirmMessage()).not.toContain( + 'Install the EQL extension into your database', + ) + expect(confirmMessage()).toContain('(required for encryption)') + }) + + it('names supabase/migrations/ on the Supabase migration route', async () => { + withSupabaseScaffolding(true) + + await installEqlStep.run(supabaseState, supabaseProvider) + + expect(confirmMessage()).toContain('supabase/migrations/') + expect(confirmMessage()).not.toContain( + 'Install the EQL extension into your database', + ) + expect(confirmMessage()).toContain('(required for encryption)') + }) + + it('keeps the database-install wording for a hosted Supabase project', async () => { + // No local scaffolding means the direct route, so the prompt must follow + // the ROUTING, not the `--supabase` flag. + withSupabaseScaffolding(false) + + await installEqlStep.run(supabaseState, supabaseProvider) + + expect(confirmMessage()).toContain( + 'Install the EQL extension into your database', + ) + }) + + it('still defaults to yes on a migration-first route', async () => { + withSupabaseScaffolding(true) + + await installEqlStep.run(supabaseState, supabaseProvider) + + expect(vi.mocked(p.confirm).mock.calls[0][0].initialValue).toBe(true) + }) + }) + + describe('declining the prompt', () => { + // The retry hint has to name the command for the route the step WOULD have + // taken. `stash eql install` is right for exactly one of the three. + + it('points the direct route at `stash eql install`', async () => { + vi.mocked(p.confirm).mockResolvedValueOnce(false) + + const result = await installEqlStep.run(baseState, provider) + + expect(result.eqlInstalled).toBe(false) + expect(installCommand).not.toHaveBeenCalled() + expect(noteBody()).toContain('stash eql install') + }) + + it('points the Drizzle route at `stash eql migration --drizzle`', async () => { + // `stash eql install --drizzle` is v2-only — under the v3 default it + // rejects the flag outright — and a bare direct install never lands in + // the migration history the project ships from. Sending a declining + // Drizzle user there is sending them at the one command this route + // exists to avoid. + vi.mocked(p.confirm).mockResolvedValueOnce(false) + + const result = await installEqlStep.run(drizzleState, drizzleProvider) + + expect(result.eqlInstalled).toBe(false) + expect(eqlMigrationCommand).not.toHaveBeenCalled() + expect(noteBody()).toContain('stash eql migration --drizzle') + }) + + it('points the Supabase migration route at `stash eql migration --supabase`', async () => { + // Retrying with `stash eql install --supabase` here reinstates the #613 + // defect outright: the install is wiped by the next `supabase db reset`. + withSupabaseScaffolding(true) + vi.mocked(p.confirm).mockResolvedValueOnce(false) + + const result = await installEqlStep.run(supabaseState, supabaseProvider) + + expect(result.eqlInstalled).toBe(false) + expect(eqlMigrationCommand).not.toHaveBeenCalled() + expect(noteBody()).toContain('stash eql migration --supabase') + }) + + it('points a hosted Supabase project at `stash eql install`', async () => { + // Same flag, other side of the routing fork: with no `supabase/` + // directory there is nowhere to write a migration, so the direct install + // really is the retry command. + withSupabaseScaffolding(false) + vi.mocked(p.confirm).mockResolvedValueOnce(false) + + await installEqlStep.run(supabaseState, supabaseProvider) + + expect(noteBody()).toContain('stash eql install') + expect(noteBody()).not.toContain('stash eql migration') + }) + }) + it('re-throws CliExit instead of reframing it as a connection failure', async () => { // `installCommand` throws CliExit for hard stops it has ALREADY reported on // with its own actionable error (e.g. an unsafe `--name`). The broad catch diff --git a/packages/cli/src/commands/init/steps/__tests__/resolve-database.test.ts b/packages/cli/src/commands/init/steps/__tests__/resolve-database.test.ts new file mode 100644 index 000000000..20d9c8030 --- /dev/null +++ b/packages/cli/src/commands/init/steps/__tests__/resolve-database.test.ts @@ -0,0 +1,61 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { InitProvider, InitState } from '../../types.js' + +// The resolver is the step's only collaborator: it walks --database-url → env → +// `supabase status` → prompt → hard fail. Mock it so the step's one decision — +// whether to hint that this is a Supabase project — is observable without a +// database, a Supabase CLI, or a TTY. +vi.mock('../../../../config/database-url.js', () => ({ + resolveDatabaseUrl: vi.fn(async () => 'postgresql://localhost:5432/app'), +})) + +import { resolveDatabaseUrl } from '../../../../config/database-url.js' +import { resolveDatabaseStep } from '../resolve-database.js' + +const state = {} as unknown as InitState + +/** The `{ supabase }` hint the step passed to the resolver. */ +function supabaseHint(): boolean | undefined { + return vi.mocked(resolveDatabaseUrl).mock.calls[0][0]?.supabase +} + +describe('resolveDatabaseStep', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('hints Supabase for `--supabase`, so the resolver may try `supabase status`', async () => { + const result = await resolveDatabaseStep.run(state, { + name: 'supabase', + selected: ['supabase'], + } as unknown as InitProvider) + + expect(supabaseHint()).toBe(true) + expect(result.databaseUrl).toBe('postgresql://localhost:5432/app') + }) + + it('still hints Supabase when `--supabase` is combined with another flag', async () => { + // `stash init --drizzle --supabase` joins the flags into a single provider + // name for referrer tracking. 'drizzle-supabase' is not 'supabase', so the + // hint was dropped and a local Supabase project — whose URL is only + // discoverable via `supabase status` — fell through to the interactive + // prompt, or to a hard failure in a non-interactive run. + await resolveDatabaseStep.run(state, { + name: 'drizzle-supabase', + selected: ['supabase', 'drizzle'], + } as unknown as InitProvider) + + expect(supabaseHint()).toBe(true) + }) + + it('does not hint Supabase for a run that never asked for it', async () => { + // The symmetric negative: hinting unconditionally would shell out to + // `supabase status` on every plain Postgres project. + await resolveDatabaseStep.run(state, { + name: 'drizzle', + selected: ['drizzle'], + } as unknown as InitProvider) + + expect(supabaseHint()).toBe(false) + }) +}) diff --git a/packages/cli/src/commands/init/steps/build-schema.ts b/packages/cli/src/commands/init/steps/build-schema.ts index 5cc43331d..b43dbb926 100644 --- a/packages/cli/src/commands/init/steps/build-schema.ts +++ b/packages/cli/src/commands/init/steps/build-schema.ts @@ -60,10 +60,14 @@ export const buildSchemaStep: InitStep = { name: 'Generate encryption client', async run(state: InitState, provider: InitProvider): Promise { const cwd = process.cwd() - const integration = - provider.name === 'prisma' - ? 'prisma-next' - : detectIntegration(cwd, state.databaseUrl) + // `provider.selected`, not `provider.name`: a combined run + // (`--prisma --supabase`) names itself 'prisma-supabase', so an equality + // test here dropped it onto `detectIntegration` — which, on a project whose + // prisma-next config isn't in place yet, answers 'postgresql' and sends the + // rest of the pipeline down the wrong route. + const integration = provider.selected.includes('prisma') + ? 'prisma-next' + : detectIntegration(cwd, state.databaseUrl) const clientFilePath = DEFAULT_CLIENT_PATH const resolvedPath = resolve(cwd, clientFilePath) diff --git a/packages/cli/src/commands/init/steps/install-deps.ts b/packages/cli/src/commands/init/steps/install-deps.ts index 70e382d0a..6032590b0 100644 --- a/packages/cli/src/commands/init/steps/install-deps.ts +++ b/packages/cli/src/commands/init/steps/install-deps.ts @@ -10,7 +10,7 @@ import { RUNTIME_PACKAGE_VERSIONS, } from '../../../runtime-versions.js' import type { InitProvider, InitState, InitStep } from '../types.js' -import { CancelledError } from '../types.js' +import { CancelledError, PROVIDER_KEY_INTEGRATION } from '../types.js' import { combinedInstallCommands, detectPackageManager, @@ -44,6 +44,35 @@ function integrationPackageFor(integration?: string): string | null { return INTEGRATION_ADAPTER_PACKAGES[integration] ?? null } +/** + * Every adapter package this run needs: the one for the DETECTED integration, + * plus one for each integration flag the user passed. Deduped, detected first. + * + * A list rather than a single package because the flags are not mutually + * exclusive. `stash init --drizzle --supabase` used to look the adapter up by + * provider NAME, and a combined run's name is 'drizzle-supabase' — not a key of + * {@link INTEGRATION_ADAPTER_PACKAGES}, so the run installed NEITHER adapter + * and whatever the user imported next failed to resolve. Both integrations were + * asked for; both packages are real. + * + * Flags are mapped through {@link PROVIDER_KEY_INTEGRATION} so `--prisma` + * resolves `@cipherstash/stack-prisma` on its own name — it used to arrive here + * only because `build-schema` runs first and leaves 'prisma-next' on state, and + * step ordering is not a contract this should depend on. + */ +function adapterPackagesFor( + state: InitState, + provider: InitProvider, +): string[] { + const packages = [ + integrationPackageFor(state.integration), + ...provider.selected.map((key) => + integrationPackageFor(PROVIDER_KEY_INTEGRATION[key]), + ), + ].filter((pkg): pkg is string => pkg !== null) + return [...new Set(packages)] +} + /** Sentinel shown when a package directory exists but its manifest can't be * read — a broken state worth surfacing, not skipping (aborted installs leave * exactly this behind). */ @@ -162,20 +191,11 @@ export const installDepsStep: InitStep = { id: 'install-deps', name: 'Install dependencies', async run(state: InitState, provider: InitProvider): Promise { - const integrationPkg = - integrationPackageFor(state.integration) ?? - integrationPackageFor(provider.name) + const integrationPkgs = adapterPackagesFor(state, provider) const stackPresent = isPackageInstalled(STACK_PACKAGE) const cliPresent = isPackageInstalled(CLI_PACKAGE) - const integrationPresent = integrationPkg - ? isPackageInstalled(integrationPkg) - : true - const allPackages = [ - STACK_PACKAGE, - ...(integrationPkg ? [integrationPkg] : []), - CLI_PACKAGE, - ] + const allPackages = [STACK_PACKAGE, ...integrationPkgs, CLI_PACKAGE] // Surface skew FIRST and unconditionally — before any prompt, decline, // failure, or early return can skip it (#661). Every path below inherits @@ -217,7 +237,9 @@ export const installDepsStep: InitStep = { // What's missing outright (pinned, prod/dev split). const missing: string[] = [] if (!stackPresent) missing.push(STACK_PACKAGE) - if (integrationPkg && !integrationPresent) missing.push(integrationPkg) + for (const pkg of integrationPkgs) { + if (!isPackageInstalled(pkg)) missing.push(pkg) + } if (!cliPresent) missing.push(CLI_PACKAGE) const missingSplit = splitProdDev(missing) @@ -250,12 +272,12 @@ export const installDepsStep: InitStep = { const offerAlign = skewed.length > 0 && isInteractive() // Nothing missing and no interactive alignment to offer: `missing` empty - // implies all three packages are present, so both flags are true. + // implies every package in `allPackages` is present, so both flags are true. if (missing.length === 0 && !offerAlign) { if (skewed.length === 0) { - const installed = integrationPkg - ? `${STACK_PACKAGE}, ${integrationPkg} and ${CLI_PACKAGE}` - : `${STACK_PACKAGE} and ${CLI_PACKAGE}` + // "a and b" / "a, b and c" / "a, b, c and d" — every adapter named, + // however many the flags selected. + const installed = `${allPackages.slice(0, -1).join(', ')} and ${allPackages[allPackages.length - 1]}` p.log.success(`${installed} are already installed.`) } else { // Non-interactive with skew: warned above; never mutate, print the fix. @@ -339,18 +361,16 @@ export const installDepsStep: InitStep = { // per-package tracking, not a composite flag. const stackInstalled = isPackageInstalled(STACK_PACKAGE) const cliInstalled = isPackageInstalled(CLI_PACKAGE) - const integrationInstalled = integrationPkg - ? isPackageInstalled(integrationPkg) - : true + const missingAdapters = integrationPkgs.filter( + (pkg) => !isPackageInstalled(pkg), + ) - if (stackInstalled && cliInstalled && integrationInstalled) { + if (stackInstalled && cliInstalled && missingAdapters.length === 0) { p.log.success('Stack dependencies installed.') } else { const stillMissing = [ ...(stackInstalled ? [] : [`${pinnedSpec(STACK_PACKAGE)} (prod)`]), - ...(integrationPkg && !integrationInstalled - ? [`${pinnedSpec(integrationPkg)} (prod)`] - : []), + ...missingAdapters.map((pkg) => `${pinnedSpec(pkg)} (prod)`), ...(cliInstalled ? [] : [`${pinnedSpec(CLI_PACKAGE)} (dev)`]), ] p.log.warn(`Still missing: ${stillMissing.join(', ')}.`) diff --git a/packages/cli/src/commands/init/steps/install-eql.ts b/packages/cli/src/commands/init/steps/install-eql.ts index 0bf8bd796..e92dbde59 100644 --- a/packages/cli/src/commands/init/steps/install-eql.ts +++ b/packages/cli/src/commands/init/steps/install-eql.ts @@ -4,18 +4,180 @@ import { isInteractive } from '../../../config/tty.js' import { pinnedSpec } from '../../../runtime-versions.js' import { ensureEncryptionClient } from '../../db/client-scaffold.js' import { offerStashConfig } from '../../db/config-scaffold.js' +import { detectSupabaseProject } from '../../db/detect.js' import { installCommand } from '../../db/install.js' -import { eqlMigrationCommand } from '../../eql/migration.js' +import { + type EqlMigrationOptions, + eqlMigrationCommand, +} from '../../eql/migration.js' +import { findExistingEqlMigration } from '../../eql/supabase-migration.js' import type { InitProvider, InitState, InitStep } from '../types.js' import { CancelledError } from '../types.js' import { isPackageInstalled } from '../utils.js' +/** + * Whether this project has a local `supabase/` directory to write a migration + * into. `config.toml` is the stronger signal — `supabase init` writes it before + * any migration exists — but an imported project may carry only the migrations + * directory, so either counts. + */ +function hasLocalSupabaseScaffolding(): boolean { + const project = detectSupabaseProject(process.cwd()) + return project.hasConfigToml || project.hasMigrationsDir +} + +/** + * Re-running `stash init --supabase` over a project that already has an install + * migration is a no-op, not a failure. + * + * `eql migration --supabase` refuses to write a second one, so without this the + * generate call throws, the catch below reports a write failure, and `initCommand` + * sees no `eqlMigrationPending` — printing "✗ EQL extension NOT installed", + * telling the user to run the direct `stash eql install` this route exists to + * avoid, and exiting 1. Nothing is wrong: the migration is right there. + * + * Passing `force: true` from init would also unblock it, but that silently + * rewrites a file some environment may already have applied. + */ +function existingSupabaseMigration(): string | null { + const { migrationsDir } = detectSupabaseProject(process.cwd()) + return findExistingEqlMigration(migrationsDir) +} + +/** + * `eql migration` deliberately does no config/client scaffolding of its own + * (unlike `eql install`), so init does it here — otherwise the migration-first + * routes would silently skip half the init contract every other integration + * gets (#581). + * + * Every migration-first exit runs this, including the one that finds the + * migration already written: a project whose migration came from a standalone + * `stash eql migration --supabase` has never had a `stash.config.ts` written, + * and skipping it here would report "Setup complete" over a project that + * cannot load one. + */ +async function scaffoldConfigAndClient(state: InitState): Promise { + const clientPath = await offerStashConfig({ ensure: true }) + if (clientPath) { + ensureEncryptionClient(clientPath, process.cwd(), state.databaseUrl) + } +} + +/** + * A migration-first route: one of the two branches that WRITE an EQL migration + * file rather than touching the database. + * + * Resolved as a value before the confirm prompt so the prompt, the + * non-interactive notice, and the decline hint can all name what will actually + * happen. They used to be written for the direct-install route and reused + * verbatim on every route — the prompt asked about installing into the + * database and then wrote a file, and declining pointed a Drizzle or Supabase + * user at `stash eql install`, the one command each route exists to avoid. + */ +interface MigrationRoute { + /** Which branch this is. Only `supabase` can find a migration already on + * disk (`eql migration --drizzle` shells out to drizzle-kit, which owns + * duplicate detection itself). */ + kind: 'drizzle' | 'supabase' + /** Confirm-prompt copy. Keeps the "(required for encryption)" force of the + * direct-install prompt — declining is not a neutral choice on any route. */ + prompt: string + options: EqlMigrationOptions + retryCommand: string + failureHint: string +} + +/** + * Which migration-first route, if any, this project takes — EQL v3 either way. + * + * **Drizzle.** `eql install --drizzle` is v2-only: under the v3 default it + * rejects the flag outright, so routing Drizzle through it would provision a + * v2 database while every other integration (and a bare `stash eql install`) + * gets v3. That also contradicts the stash-drizzle skill installed into the + * very same project, which documents the v3 surface (`types.*` domains, + * `Encryption`) and would have the user's agent author v3 code against a v2 + * database. `stash eql migration --drizzle` (added in #691) closes that gap: + * v3 SQL, still migration-first, and it bundles the `cs_migrations` tracking + * schema so one `drizzle-kit migrate` covers everything `stash encrypt` needs. + * + * **Supabase.** Same migration-first shape, different motivation. A direct + * install works, and then `supabase db reset` — the ordinary local development + * loop — drops the database and replays supabase/migrations/, taking EQL with + * it. Writing the install into that directory is the only way it survives + * (#613). It also means one `db reset` provisions everything `stash encrypt` + * needs, since the emitted SQL carries `cs_migrations` too. Gated on local CLI + * scaffolding: a project pointed at a hosted Supabase database with no + * `supabase/` directory has nowhere to write and no `supabase` binary to apply + * it with, so it must keep installing directly. + * + * Drizzle wins when both signals fire — it owns the migration history there, + * and `--supabase` degrades to the grants modifier it has always been on that + * path. `initCommand`'s apply-step routing makes the same call, for the same + * reason. The `&&` short-circuit keeps the filesystem probe off every + * non-Supabase project. + */ +function resolveMigrationRoute( + supabase: boolean, + drizzle: boolean, +): MigrationRoute | null { + if (drizzle) { + return { + kind: 'drizzle', + prompt: + 'Generate an EQL migration in your Drizzle migrations folder now? (required for encryption)', + options: { drizzle: true, supabase: supabase || undefined }, + retryCommand: 'stash eql migration --drizzle', + failureHint: + 'Could not generate the EQL migration — check that drizzle-kit is installed and configured.', + } + } + if (supabase && hasLocalSupabaseScaffolding()) { + return { + kind: 'supabase', + prompt: + 'Generate an EQL migration in supabase/migrations/ now? (required for encryption)', + options: { supabase: true }, + retryCommand: 'stash eql migration --supabase', + failureHint: + 'Could not write the EQL migration into supabase/migrations/.', + } + } + return null +} + +/** + * Shared body of the two migration-first routes. + * + * The failure path never echoes the underlying error: `eqlMigrationCommand` + * has already logged its own actionable diagnostics, and errors on this path + * can carry a connection string. + */ +async function generateEqlMigration( + state: InitState, + route: MigrationRoute, +): Promise { + await scaffoldConfigAndClient(state) + + try { + await eqlMigrationCommand({ ...route.options, embedded: true }) + } catch { + p.log.error(route.failureHint) + p.note(`Re-run with: ${route.retryCommand}`, 'You can retry manually') + return { ...state, eqlInstalled: false } + } + + // A migration file was WRITTEN, not applied — EQL lands in the database when + // the user runs their migrate step. + return { ...state, eqlInstalled: false, eqlMigrationPending: true } +} + /** * Install EQL programmatically after a y/N confirm. * - * Two routes, both EQL v3: Drizzle projects generate a v3 install migration - * (`stash eql migration --drizzle`) so the install lands in the project's - * migration history; everything else runs `stash eql install` directly. + * Two routes, both EQL v3. Migration-first wherever the project has a + * migration history to land in — Drizzle projects (`stash eql migration + * --drizzle`), and Supabase projects with local CLI scaffolding (`stash eql + * migration --supabase`). Everything else runs `stash eql install` directly. * * EQL is the Postgres extension every CipherStash query relies on. Without * it, the encryption client can't read or write to encrypted columns. @@ -23,9 +185,7 @@ import { isPackageInstalled } from '../utils.js' * it as the first thing to run before any migration. * * We pass the URL we already resolved at the start of init (state.databaseUrl) - * through to `installCommand` so the user is never re-prompted. The installer - * picks the Supabase migration / direct mode itself based on `--supabase` and - * project layout — we don't pre-decide it here. + * through to `installCommand` so the user is never re-prompted. * * `installCommand` ends init on a hard failure (mutually-exclusive flag clash, * scaffold cancellation, an unsafe `--name`) — either by calling @@ -46,25 +206,47 @@ export const installEqlStep: InitStep = { // migrations — running `stash eql install` here would be a // duplicate install and would race with the framework's // migration journal. Skip with guidance instead. - if (integration === 'prisma-next' || provider.name === 'prisma') { + if (integration === 'prisma-next' || provider.selected.includes('prisma')) { p.log.success( 'Skipping `stash eql install` — Prisma Next installs the EQL bundle via `prisma-next migrate` (runs alongside your app migrations).', ) return { ...state, eqlInstalled: false } } - const supabase = integration === 'supabase' || provider.name === 'supabase' - const drizzle = integration === 'drizzle' || provider.name === 'drizzle' + // Two signals per integration: what the project looks like, and what the + // user asked for. The flag half reads `provider.selected` rather than + // `provider.name` because the flags combine — `stash init --drizzle + // --supabase` names itself 'drizzle-supabase', which equals neither, so + // both went false, `resolveMigrationRoute` returned null, and a local + // Supabase + Drizzle project (integration 'postgresql', because the host is + // 127.0.0.1:54322) got a direct install with no migration file and no role + // grants — the #613 failure, reached through a flag combination the CLI + // accepts. + const supabase = + integration === 'supabase' || provider.selected.includes('supabase') + const drizzle = + integration === 'drizzle' || provider.selected.includes('drizzle') + + // Resolved BEFORE the prompt, not at the branch below, because everything + // the user reads next has to describe the route they are actually on. + // Both inputs are already available here: the two flags above, and a pair + // of `existsSync` calls behind `hasLocalSupabaseScaffolding()`. + const migrationRoute = resolveMigrationRoute(supabase, drizzle) // Non-interactive (CI, agents, pipes): there's no TTY to answer the prompt, - // so take the default (install) and continue rather than hang or abort. This + // so take the default (proceed) and continue rather than hang or abort. This // is what makes `stash init` honour its documented non-interactive contract. if (!isInteractive()) { - p.log.info('Installing the EQL extension (non-interactive).') + p.log.info( + migrationRoute + ? 'Generating the EQL migration (non-interactive).' + : 'Installing the EQL extension (non-interactive).', + ) } const proceed = isInteractive() ? await p.confirm({ message: + migrationRoute?.prompt ?? 'Install the EQL extension into your database now? (required for encryption)', initialValue: true, }) @@ -75,7 +257,9 @@ export const installEqlStep: InitStep = { if (!proceed) { p.log.info('Skipping EQL installation.') p.note( - 'Run `stash eql install` before applying any migration that references encrypted columns.', + migrationRoute + ? `Run \`${migrationRoute.retryCommand}\`, then apply it before any migration that references encrypted columns.` + : 'Run `stash eql install` before applying any migration that references encrypted columns.', 'EQL not installed', ) return { ...state, eqlInstalled: false } @@ -100,51 +284,31 @@ export const installEqlStep: InitStep = { return { ...state, eqlInstalled: false } } - // Drizzle: generate an EQL **v3** migration (`stash eql migration - // --drizzle`) rather than routing through `eql install`. - // - // `eql install --drizzle` is v2-only — under the v3 default it rejects the - // flag outright, so routing Drizzle through it would provision a v2 - // database while every other integration (and a bare `stash eql install`) - // gets v3. That also contradicts the stash-drizzle skill installed into the - // very same project, which documents the v3 surface (`types.*` domains, - // `Encryption`) and would have the user's agent author v3 code against a v2 - // database. - // - // `stash eql migration --drizzle` (added in #691) closes that gap: v3 SQL, - // still migration-first, and it bundles the `cs_migrations` tracking schema - // so one `drizzle-kit migrate` covers everything `stash encrypt` needs. - // `eql install`'s config/client scaffolding isn't part of that command, so - // we do it here to keep the rest of the init contract identical. - if (drizzle) { - const clientPath = await offerStashConfig({ ensure: true }) - if (clientPath) { - ensureEncryptionClient(clientPath, process.cwd(), state.databaseUrl) + // The migration-first routes (see `resolveMigrationRoute` for why each one + // exists). `eql migration` does none of `eql install`'s config/client + // scaffolding, so `generateEqlMigration` does it here to keep the rest of + // the init contract identical. + if (migrationRoute) { + if (migrationRoute.kind === 'supabase') { + const existing = existingSupabaseMigration() + if (existing) { + // Still scaffold: the migration may have come from a standalone `stash + // eql migration --supabase`, which writes SQL and nothing else. + await scaffoldConfigAndClient(state) + p.log.success(`EQL install migration already present: ${existing}`) + // `eqlMigrationAlreadyPresent` is what stops the summary claiming + // this run "generated" a file it only found. The apply guidance is + // unchanged — an unapplied migration is an unapplied migration — so + // `eqlMigrationPending` still carries the completeness signal. + return { + ...state, + eqlInstalled: false, + eqlMigrationPending: true, + eqlMigrationAlreadyPresent: true, + } + } } - - try { - await eqlMigrationCommand({ - drizzle: true, - supabase: supabase || undefined, - embedded: true, - }) - } catch { - // Most likely drizzle-kit missing or misconfigured. Don't echo the - // error (same reasoning as below re: connection strings) — the command - // has already logged its own actionable diagnostics. - p.log.error( - 'Could not generate the EQL migration — check that drizzle-kit is installed and configured.', - ) - p.note( - 'Re-run with: stash eql migration --drizzle', - 'You can retry manually', - ) - return { ...state, eqlInstalled: false } - } - - // A migration file was WRITTEN, not applied — EQL lands in the database - // when the user runs `drizzle-kit migrate`. - return { ...state, eqlInstalled: false, eqlMigrationPending: true } + return await generateEqlMigration(state, migrationRoute) } try { diff --git a/packages/cli/src/commands/init/steps/resolve-database.ts b/packages/cli/src/commands/init/steps/resolve-database.ts index 2c53cbb4f..fb1fd3050 100644 --- a/packages/cli/src/commands/init/steps/resolve-database.ts +++ b/packages/cli/src/commands/init/steps/resolve-database.ts @@ -20,10 +20,13 @@ export const resolveDatabaseStep: InitStep = { id: 'resolve-database', name: 'Resolve database URL', async run(state: InitState, provider: InitProvider): Promise { - // The provider name carries the integration flag the user passed at the - // CLI (`--supabase` → 'supabase'), which lets the resolver try - // `supabase status` even before we've inspected the project layout. - const supabaseHint = provider.name === 'supabase' + // `provider.selected` carries the integration flags the user passed at the + // CLI, which lets the resolver try `supabase status` even before we've + // inspected the project layout. Membership, not equality on + // `provider.name`: the flags combine, and `--drizzle --supabase` names + // itself 'drizzle-supabase' — which dropped the hint on exactly the local + // Supabase projects whose URL only `supabase status` knows. + const supabaseHint = provider.selected.includes('supabase') const databaseUrl = await resolveDatabaseUrl({ supabase: supabaseHint }) return { ...state, databaseUrl } }, diff --git a/packages/cli/src/commands/init/types.ts b/packages/cli/src/commands/init/types.ts index 738955b26..9abf65282 100644 --- a/packages/cli/src/commands/init/types.ts +++ b/packages/cli/src/commands/init/types.ts @@ -4,6 +4,26 @@ import type { PackageManager } from './utils.js' export type Integration = 'drizzle' | 'supabase' | 'prisma-next' | 'postgresql' +/** + * The integration flags `stash init` accepts (`--supabase`, `--drizzle`, + * `--prisma`). They are not mutually exclusive — `stash init --drizzle + * --supabase` is a real, accepted invocation for a Drizzle project on Supabase. + */ +export type ProviderKey = 'supabase' | 'drizzle' | 'prisma' + +/** + * The {@link Integration} each flag selects. `--prisma` is the odd one out: the + * flag is short for consistency with `--supabase` / `--drizzle`, but the + * integration it selects is Prisma Next (see providers/prisma.ts). + */ +export const PROVIDER_KEY_INTEGRATION: Readonly< + Record +> = { + supabase: 'supabase', + drizzle: 'drizzle', + prisma: 'prisma-next', +} + export type DataType = 'string' | 'number' | 'boolean' | 'date' | 'json' /** @@ -75,6 +95,15 @@ export interface InitState { * summary reports "migration generated, apply it" instead of a false * "installed" or a spurious "setup incomplete". */ eqlMigrationPending?: boolean + /** True when the pending migration was ALREADY on disk — a re-run of `stash + * init --supabase`, or a project whose migration came from a standalone + * `stash eql migration --supabase`. Refines `eqlMigrationPending`, never + * replaces it: the state of the world is the same either way (a migration + * exists, it has not been applied) and so is the apply guidance, so the + * incompleteness check must keep reading `eqlMigrationPending` alone. All + * this changes is the summary's verb — "already present" rather than + * "generated", which was a claim about work this run did not do. */ + eqlMigrationAlreadyPresent?: boolean /** Detected ORM / framework integration. Set by build-schema. */ integration?: Integration /** Schema definitions written to the encryption client. Carries every @@ -139,7 +168,24 @@ export interface HandoffStep { } export interface InitProvider { + /** + * Referrer / display identity, NOT a routing signal. A multi-flag run joins + * every matched flag alphabetically (`stash init --drizzle --supabase` → + * `'drizzle-supabase'`), matching the referrer `stash auth login --drizzle + * --supabase` records, and `authenticateStep` passes it straight to + * `login()`. Because that combined string equals no single flag name, code + * that branched on `provider.name === 'supabase'` fell through on every + * combined run — read {@link InitProvider.selected} instead. + */ name: string + /** + * The integration flags the user actually passed, in `PROVIDER_KEYS` order + * (`resolveProvider`, init/index.ts). This is the capability signal: every + * step that asks "is this a Supabase run?" tests membership here rather than + * parsing `name`, so combined flags keep working and `name` stays free to + * carry whatever the referrer needs. + */ + selected: readonly ProviderKey[] introMessage: string getNextSteps(state: InitState, pm: PackageManager): string[] } diff --git a/packages/cli/src/messages.ts b/packages/cli/src/messages.ts index 78e1d4a3c..aff38c792 100644 --- a/packages/cli/src/messages.ts +++ b/packages/cli/src/messages.ts @@ -66,12 +66,15 @@ export const messages = { * actionable command + `--force` note are appended at the call site. */ prismaNextDetected: 'This looks like a Prisma Next project', - /** `stash eql migration` with no `--drizzle`/`--prisma` target. */ + /** `stash eql migration` with no `--drizzle`/`--supabase`/`--prisma` target. */ migrationNeedsTarget: - 'Specify a target: `stash eql migration --drizzle` (or `--prisma`).', - /** More than one target passed to `stash eql migration`. */ + 'Specify a target: `stash eql migration --drizzle` for a Drizzle project, or `stash eql migration --supabase` to write into supabase/migrations/ (or `--prisma`).', + /** + * `--drizzle --prisma`. Note that `--drizzle --supabase` is NOT this error: + * there, `--supabase` is the role-grants modifier, not a second target. + */ migrationOneTarget: - 'Pass exactly one target: `--drizzle` or `--prisma`, not both.', + 'Pass exactly one target: `--drizzle` or `--prisma`, not both. (`--supabase` is a target on its own, and the role-grants modifier when combined with `--drizzle`.)', /** * `--prisma` is registered only to route people to the right mechanism: * Prisma Next installs the EQL bundle through its own migration framework @@ -84,6 +87,128 @@ export const messages = { /** `--name` carried characters outside `[A-Za-z0-9_-]`. */ migrationBadName: 'Migration name must contain only letters, numbers, dashes, and underscores.', + /** + * `--name` with `--supabase`. The Supabase filename is fixed because + * duplicate detection matches on the `_cipherstash_eql.sql` suffix, so the + * flag cannot be honoured — warn rather than rename nothing silently. + */ + migrationNameDrizzleOnly: + '`--name` applies to `--drizzle` only and is ignored here — the Supabase migration is always named `_cipherstash_eql.sql`, which is how a duplicate install is detected.', + /** + * `--out` with a bare `--supabase`, pointing anywhere other than + * `/supabase/migrations`. + * + * The Supabase CLI's migrations directory is NOT configurable. In the Go + * implementation it is `filepath.Join(SupabaseDirPath, "migrations")` with + * `SupabaseDirPath = "supabase"`, and the path builder that derives it from + * a `--config` path still carries a literal `// TODO: make base path + * configurable from toml`; the TypeScript CLI hard-codes + * `path.join(workdir, "supabase", "migrations")` in both the `db reset` and + * `db push` handlers. `--workdir` / `SUPABASE_WORKDIR` moves the whole + * project root, not this subdirectory, and `config.toml` has no key for it + * (supabase/supabase#33257 is the open request to add one). + * + * So a file written elsewhere is exactly the failure this command exists to + * fix — EQL missing from the directory a reset replays — just relocated. + * A warning rather than a hard error, because the user may well have their + * own apply step for that directory; what they cannot be allowed to assume + * is that `supabase db reset` will pick it up. + */ + migrationSupabaseOutNotReplayed: (migrationsDir: string) => + `--out points at ${migrationsDir}, but the Supabase CLI only ever replays /supabase/migrations — that path is hard-coded, with no config.toml key to move it (--workdir relocates the whole supabase/ directory, not this one). \`supabase db reset\` and \`supabase db push\` will not apply this file, so EQL will still be missing after the next reset. Drop --out to write into supabase/migrations/, unless you have your own step that applies this directory.`, + /** + * `--supabase --force` replaced an install migration in place. + * + * Two things the user cannot see from the success line. First, `supabase db + * push` will NOT pick the new bundle up: `FindPendingMigrations` + * (`pkg/migration/apply.go`) computes the pending set positionally — + * `pending := localMigrations[len(remoteMigrations):]` — with no content + * hash and no statement diff. (Seed files DO carry a `Hash`/`Dirty` pair and + * re-run on change; migrations do not.) Equal counts mean an empty pending + * set, so push prints "Remote database is up to date." and applies nothing. + * Telling people to `db push` here leaves them believing a remote was + * updated when it was not. + * + * Second, re-applying is not free. The EQL bundle opens with `DROP SCHEMA IF + * EXISTS eql_v3 CASCADE;` / `DROP SCHEMA IF EXISTS eql_v3_internal + * CASCADE;`, so it takes every dependent index, constraint, and RLS policy + * with it. On a fresh `supabase db reset` that is a no-op on an empty + * database; on a populated remote it is destructive. + */ + migrationSupabaseForceReplaced: + 'Replaced the EQL install migration in place, keeping its version. A database that already applied that version still has the OLD bundle, and `supabase db push` will not re-apply it — the Supabase CLI decides what is pending by version, not by content, so a version already in the ledger is never re-run (push just reports "Remote database is up to date."). Re-applying is not free either: the EQL bundle opens with `DROP SCHEMA IF EXISTS eql_v3 CASCADE` (and `eql_v3_internal`), which also drops every index, constraint, and RLS policy that references those schemas. Harmless against a fresh `supabase db reset`; destructive against a populated remote.', + /** + * The re-apply recipe for a replaced install migration, in place of the + * plain "Apply it" note. `migration repair --status reverted` deletes the + * ledger row and nothing else — Supabase's docs are explicit that it updates + * the tracking table without applying or reverting any SQL — which puts the + * version back in the pending set. + * + * Whether the follow-up push then needs `--include-all` depends on where + * the reverted version sits, which is why this does not just print the flag + * and be done with it. Reverting the NEWEST version leaves it at the tail of + * remote history: `FindPendingMigrations` returns it as ordinary pending + * work and a plain `db push` applies it. Only a version with applied + * migrations above it is the "gap in the middle" that `ErrMissingRemote` + * rejects. Verified live against supabase/cli 2.111.0 — the tail case + * pushes clean, the gap case aborts — in `supabase-push.live.test.ts` + * ("needs --include-all only when the install is not the newest migration"). + * + * Printing `--include-all` unconditionally would not just be verbose: it + * applies EVERY out-of-order local migration, including any the user has + * deliberately left unapplied. + */ + migrationSupabaseReapply: (version: string | null) => + `Re-apply the replaced migration.\n\nLocal:\n\n supabase db reset\n\nRemote — clear the ledger row first, or the push is a no-op:\n\n supabase migration repair --status reverted ${version ?? ''}\n supabase db push\n\n\`migration repair\` updates the tracking table only; it applies no SQL. If migrations sort AFTER the install (the usual shape — encrypted-column migrations written once EQL was in place), the reverted version is a gap in the middle of remote history and that push aborts with \`Found local migration files to be inserted before the last migration on remote database.\`; re-run it as \`supabase db push --include-all\`. Reach for that flag only when the push tells you to — it applies every out-of-order migration you have, not just this one. Read the CASCADE warning above before doing any of this to a populated database.`, + /** + * Migrations already in the directory that reference EQL and sort BEFORE the + * install this command writes. + * + * The brownfield case: `stash eql install` applied EQL straight to the + * database, encrypted-column migrations were written against it, and only + * then did the project move to the migration-first install. A current + * timestamp sorts LAST, so those migrations replay before EQL exists and + * `supabase db reset` dies on the first `eql_v3_*` reference. + * + * Detection and a warning, not a fix: back-dating the install, renaming the + * user's migrations, or squashing the lot are all their call, and a + * back-dated file has its own remote consequence that they have to be the + * ones to accept. + * + * That consequence is split by the remote's state, and getting it wrong is + * destructive. This warning only ever fires on a project that already ran + * `stash eql install` — that is what put EQL in the database ahead of the + * migration history — so the remote typically HAS the bundle and is missing + * only the ledger row. `migration repair --status applied` is the answer + * there: it writes the row and runs no SQL. Pushing the file instead re-runs + * a bundle that opens with `DROP SCHEMA IF EXISTS eql_v3 CASCADE`, dropping + * every index, constraint, and RLS policy that references those schemas. + * `--include-all` stays for the other case — a remote that genuinely has not + * had the SQL applied — where the back-dated version is a gap in the middle + * of history that `db push` otherwise refuses to step over. + * + * "Typically" is doing dangerous work in that paragraph, which is why the + * message prints a check rather than a premise. Every other remedy here + * fails loudly and can be retried; marking a version applied fails silently + * and cannot. On a remote that does NOT have EQL — misremembered, a + * different environment, reset since — the row asserts SQL that never ran, + * so the version is permanently pending-free: no future `db push` will ever + * install EQL, and the first migration touching `eql_v3` fails with nothing + * in the history pointing at the cause. + * + * The check is `eql_v3.version()` rather than the more obvious probe for the + * `eql_v3` schema because of where each object sits in the bundle. The + * schema is created by its opening statements and survives an install that + * aborted partway; `version()` is created by its closing ones, so it + * resolves only if the whole bundle ran. Marking applied on a half-installed + * remote is the same dead end as marking applied on a bare one, so the check + * must not pass there. + */ + migrationSupabaseEqlBeforeInstall: ( + migrationsDir: string, + files: string[], + ) => + `Migrations in ${migrationsDir} reference EQL and sort BEFORE the EQL install migration:\n\n ${files.join('\n ')}\n\n\`supabase db reset\` replays the directory in version order, with no dependency awareness, so each of those runs before EQL is installed and the reset fails (\`type "eql_v3_text_search" does not exist\`). Rename the install migration to a version below ${files[0]} so it replays first.\n\nHow that back-dated version reaches a remote depends on what that remote actually has. Check it — do not go by memory:\n\n psql "$REMOTE_DATABASE_URL" -Atc "select eql_v3.version()"\n\n\`eql_v3.version()\` is created by the last statements of the bundle, so it answers "is the whole install there". The \`eql_v3\` schema alone does not: it is created by the bundle's first statements and survives an install that aborted partway.\n\nIf that prints a version, EQL is present and only the ledger row is missing — mark it applied, which writes the ledger row and runs no SQL:\n\n supabase migration repair --status applied \n\nDo NOT push the file to that remote instead: the bundle opens with \`DROP SCHEMA IF EXISTS eql_v3 CASCADE\` (and \`eql_v3_internal\`), so re-applying it drops every index, constraint, and RLS policy that references those schemas.\n\nIf it errors instead (\`schema "eql_v3" does not exist\`, or \`function eql_v3.version() does not exist\` on a half-applied install), that remote genuinely still needs the SQL: \`supabase db push --include-all\`, because the back-dated version lands as a gap in the middle of that history. Never mark it applied there — the ledger row would claim SQL that never ran, so no later push installs EQL and the first migration referencing \`eql_v3\` fails with nothing pointing at the cause.`, /** `stash eql repair` with no `--drizzle` target. */ repairNeedsTarget: 'Specify a target: `stash eql repair --drizzle`.', /** `--out` (or its `drizzle` default) points at a directory that isn't there. */ diff --git a/packages/cli/tests/e2e/command-help.e2e.test.ts b/packages/cli/tests/e2e/command-help.e2e.test.ts index 1dc33110a..f71634919 100644 --- a/packages/cli/tests/e2e/command-help.e2e.test.ts +++ b/packages/cli/tests/e2e/command-help.e2e.test.ts @@ -33,6 +33,7 @@ describe('per-command --help', () => { expect(r.output).toContain('Usage: npx stash eql migration [options]') expect(r.output).toContain('--drizzle') expect(r.output).toContain('--supabase') + expect(r.output).toContain('--force') }) it('renders full command help for `eql install --help`', async () => { diff --git a/packages/cli/tests/e2e/smoke.e2e.test.ts b/packages/cli/tests/e2e/smoke.e2e.test.ts index 72a007d4d..20f5be783 100644 --- a/packages/cli/tests/e2e/smoke.e2e.test.ts +++ b/packages/cli/tests/e2e/smoke.e2e.test.ts @@ -118,6 +118,20 @@ describe('stash CLI — non-interactive smoke', () => { expect(r.output).toContain('bogus-sub') }) + /** + * clack hard-wraps to the pty width (100 cols), so any assertion phrase long + * enough to straddle a wrap fails on formatting rather than content. Collapse + * the wrapping before matching — line breaks here are presentation. + * + * The `│` gutter is part of that wrapping: clack inserts one at every wrap + * point, so collapsing whitespace alone still leaves it embedded mid-phrase. + * Today's phrases happen not to straddle a wrap, but the next edit to any of + * these messages shifts the wrap points — which is the exact failure this + * helper exists to stop. + */ + const unwrapped = (output: string): string => + output.replace(/[\s│]+/g, ' ').trim() + // The retired `--migration` flag fails before any I/O or prompt, so these // cases can observe the install entry path deterministically without a DB. it('db install still works as a deprecated alias and warns', async () => { @@ -128,8 +142,10 @@ describe('stash CLI — non-interactive smoke', () => { expect(r.output).toContain('stash db install" is deprecated') expect(r.output).toContain('eql install" instead') // The alias reaches the real install command (its flag validation ran). - expect(r.output).toContain('eql install --migration` has been removed') - expect(r.output).toContain('eql migration --drizzle') + expect(unwrapped(r.output)).toContain( + 'eql install --migration` has been removed', + ) + expect(unwrapped(r.output)).toContain('stash eql migration') }) it('eql install routes to the install command without a deprecation warning', async () => { @@ -137,8 +153,13 @@ describe('stash CLI — non-interactive smoke', () => { const { exitCode } = await r.exit expect(exitCode).toBe(1) expect(r.output).not.toContain('is deprecated') - expect(r.output).toContain('eql install --migration` has been removed') - expect(r.output).toContain('eql migration --drizzle') + expect(unwrapped(r.output)).toContain( + 'eql install --migration` has been removed', + ) + // Both replacements, so a Supabase project is no longer sent to drizzle-kit + // (#613) — that misdirection was half the reported bug. + expect(unwrapped(r.output)).toContain('`--supabase` writes into') + expect(unwrapped(r.output)).toContain('`--drizzle` emits a Drizzle') }) it('db migrate is a stub that exits 0 with a "not yet implemented" warning', async () => { diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index cc87a2e9d..8065c1b9d 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -35,9 +35,10 @@ The entry point, for humans and agents alike: npx stash init # PostgreSQL / Drizzle / Prisma (auto-detected) npx stash init --supabase # Supabase npx stash init --prisma # Prisma Next +npx stash init --drizzle --supabase # Drizzle on Supabase (the flags combine) ``` -`stash init` installs the CLI as a project dev dependency, so subsequent commands can drop the `npx`. The CLI is package-manager aware — before init, use whichever one-shot runner your project uses (`npx`, `pnpm dlx`, `bunx`, `yarn dlx`). Installs are **pinned to the exact `@cipherstash/*` versions this CLI release shipped with** (never bare dist-tags, which can lag behind a release), and init flags any already-installed `@cipherstash/*` package whose resolved version differs from the release's. The fix depends on direction, and init says which applies: an **older** install should be aligned to the release (init offers the exact command); a **newer** install must NOT be downgraded — update the `stash` CLI to the matching release instead (init prints that command too). **Non-interactively, an older ("behind") skew is fatal** — init refuses with a non-zero exit and the align command rather than scaffolding against mismatched packages and reporting a false success. Interactively it offers to align. Likewise, if the EQL extension isn't installed at the end, init reports **"Setup incomplete"** and exits non-zero — it never claims a setup is complete when encryption would fail at query time. Integrations that install EQL through a migration are the exception and exit 0: **Prisma Next** installs it via the top-level `prisma-next migrate`, and the **Drizzle** flow *generates* an EQL migration, which init reports honestly as "EQL migration generated — apply it with `drizzle-kit migrate`" rather than claiming the extension is already installed. +`stash init` installs the CLI as a project dev dependency, so subsequent commands can drop the `npx`. The CLI is package-manager aware — before init, use whichever one-shot runner your project uses (`npx`, `pnpm dlx`, `bunx`, `yarn dlx`). Installs are **pinned to the exact `@cipherstash/*` versions this CLI release shipped with** (never bare dist-tags, which can lag behind a release), and init flags any already-installed `@cipherstash/*` package whose resolved version differs from the release's. The fix depends on direction, and init says which applies: an **older** install should be aligned to the release (init offers the exact command); a **newer** install must NOT be downgraded — update the `stash` CLI to the matching release instead (init prints that command too). **Non-interactively, an older ("behind") skew is fatal** — init refuses with a non-zero exit and the align command rather than scaffolding against mismatched packages and reporting a false success. Interactively it offers to align. Likewise, if the EQL extension isn't installed at the end, init reports **"Setup incomplete"** and exits non-zero — it never claims a setup is complete when encryption would fail at query time. Integrations that install EQL through a migration are the exception and exit 0: **Prisma Next** installs it via the top-level `prisma-next migrate`, and the **Drizzle** and **local Supabase** flows (a Supabase project with a local `supabase/` directory — a hosted one with no CLI scaffolding installs directly) *generate* an EQL migration, which init reports honestly as "EQL migration generated — apply it with `drizzle-kit migrate`" (Supabase: `supabase db reset` locally, `supabase db push` remotely) rather than claiming the extension is already installed. Re-running init over a project whose install migration is already on disk reports "EQL migration **already present**" — same apply step, same zero exit, no claim that this run generated anything. **If you are an agent, do this first:** @@ -224,11 +225,13 @@ Six mechanical steps, no agent handoff. It prompts only when it can't pick a sen 2. **Resolve database** — per the resolution order above; verifies the connection. 3. **Build schema** — auto-detects Drizzle, Supabase, and Prisma Next and writes the placeholder encryption client. **Prisma Next is the exception:** it derives schemas from `contract.json`, so no encryption-client file is written and none is needed. 4. **Install dependencies** — one combined prompt for `@cipherstash/stack` and `stash`. -5. **Install EQL** — always EQL v3. Drizzle generates `eql migration --drizzle`; Prisma Next installs through `prisma-next migrate`; other integrations install directly. +5. **Install EQL** — always EQL v3, migration-first wherever there is a migration history to land in. Drizzle generates `eql migration --drizzle`; a Supabase project with a local `supabase/` directory generates `eql migration --supabase`; Prisma Next installs through `prisma-next migrate`; everything else (including a hosted Supabase project with no CLI scaffolding) installs directly. The migration routes leave EQL **generated, not applied** — the summary says so, and you run the migrate step yourself. 6. **Gather context** — detects available coding agents and writes `.cipherstash/context.json`. Flags: `--supabase`, `--drizzle`, `--prisma`, `--region `. +**The integration flags combine.** `stash init --drizzle --supabase` is a Drizzle project on Supabase: the EQL migration goes into your Drizzle migrations folder (drizzle-kit owns the history there) with the Supabase role grants appended, both adapter packages are installed, and the database-URL resolver may use `supabase status` to find a local stack. `--prisma` combined with another flag still takes the Prisma Next route. Combined flags are recorded together as the referrer (`drizzle-supabase`), exactly as `stash auth login --drizzle --supabase` does. This is `init` only — `eql migration` takes exactly one target (see below). + | Generated file | Purpose | |---|---| | `./src/encryption/index.ts` | Placeholder encryption client — declare encrypted columns here, or let `plan`/`impl` do it. **Not written for Prisma Next** (`--prisma`), which derives schemas from `contract.json` | @@ -338,6 +341,7 @@ Flags below are the decision-relevant ones. Run `stash --help` for the ```bash stash eql install stash eql migration --drizzle +stash eql migration --supabase stash eql repair --drizzle stash eql upgrade stash eql status @@ -347,7 +351,9 @@ stash eql status #### `eql install` -Gets a project from zero to a direct EQL v3 install. It loads an existing `stash.config.ts` (or offers to scaffold one), scaffolds the encryption client if missing, and applies the pinned `@cipherstash/eql` bundle. To put installation in Drizzle migration history, use `eql migration --drizzle` instead. +Gets a project from zero to a direct EQL v3 install. It loads an existing `stash.config.ts` (or offers to scaffold one), scaffolds the encryption client if missing, and applies the pinned `@cipherstash/eql` bundle. To put the installation in migration history instead, use `eql migration` — `--drizzle` for Drizzle, `--supabase` for a Supabase project. + +**On Supabase with a local `supabase/` directory, use `eql migration --supabase`, not this.** A direct install does not survive `supabase db reset`, which drops the database and replays `supabase/migrations/`. Reserve `eql install --supabase` for a hosted project administered without the Supabase CLI. | Flag | Description | |---|---| @@ -362,23 +368,44 @@ The removed `--eql-version`, `--latest`, `--drizzle`, `--migration`, `--direct`, #### `eql migration` -Generates an **EQL v3 install migration** for your ORM, instead of running SQL directly against the database (`eql install`). Migration-first is the preferred path: the install lands in your migration history and ships to every environment through the ORM's own migrate step. v3 only — there is no `--eql-version` here. +Generates an **EQL v3 install migration**, instead of running SQL directly against the database (`eql install`). Migration-first is the preferred path: the install lands in your migration history and ships to every environment through the same migrate step as the rest of your schema. On Supabase it is the *only* durable path — `supabase db reset` replays the migrations directory, so a direct install is wiped by the next reset. v3 only — there is no `--eql-version` here. ```bash stash eql migration --drizzle # Drizzle custom migration in drizzle/ stash eql migration --drizzle --supabase # also grant eql_v3 to anon/authenticated/service_role +stash eql migration --supabase # supabase/migrations/_cipherstash_eql.sql ``` +**`--supabase` plays two roles.** On its own it is the target: write the install into `supabase/migrations/`. Combined with `--drizzle` it is a modifier on the Drizzle migration, adding the role grants. Only a bare `--supabase` selects the Supabase emitter. + | Flag | Description | |---|---| | `--drizzle` | Emit a Drizzle custom migration (via `drizzle-kit generate --custom`, then inject the SQL). Requires `drizzle-kit`. | | `--prisma` | **Not needed** — Prisma Next installs the EQL bundle through its own migration framework (the extension pack's `migrations/cipherstash/` contract space; run `prisma-next migrate`). The flag exists only to say so and point you there. | -| `--supabase` | Append the Supabase role grants (`eql_v3` + `eql_v3_internal` → `anon`, `authenticated`, `service_role`). Harmless when you connect directly as `postgres`; needed when the same tables are reached via PostgREST/RLS. | +| `--supabase` | Alone: write the install into `supabase/migrations/`, so it survives `supabase db reset`. With `--drizzle`: append the Supabase role grants (`eql_v3` + `eql_v3_internal` → `anon`, `authenticated`, `service_role`) instead. Harmless when you connect directly as `postgres`; needed when the same tables are reached via PostgREST/RLS. | | `--name ` | Migration name (Drizzle). Default `install-eql`. Letters, numbers, `-`, and `_` only — anything else is rejected. | -| `--out ` | Output directory (Drizzle). Default `drizzle`. Passed straight to `drizzle-kit --out`, so set it to match your `drizzle.config.ts` if that writes elsewhere. | +| `--out ` | Where the migration is written. Drizzle: default `drizzle`, passed straight to `drizzle-kit --out`, so set it to match your `drizzle.config.ts` if that writes elsewhere. Supabase: leave it alone — see below. | +| `--force` | Regenerate the Supabase install migration in place when one already exists (keeping its version, so an applied ledger stays consistent). Without it, a second run exits 1. Re-applying the replaced file takes a specific recipe — see "Re-applying after `--force`" below. Not needed for `--drizzle` — drizzle-kit numbers each generated migration. | | `--dry-run` | Show what would happen without writing anything. | -Pass exactly one of `--drizzle` / `--prisma`. The generated migration also installs the `cs_migrations` tracking schema, so one `drizzle-kit migrate` covers everything `stash encrypt …` needs. +The Supabase file is timestamped at generation time, so it sorts **after** everything already applied and pushes with no extra flag. That is worth having, because an out-of-order version is not merely skipped: `supabase db push` aborts the *entire* push with `Found local migration files to be inserted before the last migration on remote database.` and applies nothing, until you re-run it with `--include-all`. The file carries the EQL bundle, the role grants, and the `cipherstash.cs_migrations` tracking schema, so one `supabase db reset` provisions everything `stash encrypt` needs. + +**Sorting last is wrong if the project already has encrypted-column migrations.** A project that ran `stash eql install` first, then wrote migrations adding `public.eql_v3_*` columns against the live database, ends up with an install stamped today — i.e. *after* those migrations. `supabase db reset` replays the directory in version order with no dependency awareness, so they run first and the reset fails with `type "eql_v3_text_search" does not exist`. The command detects this before writing (on `--dry-run` too) and warns, naming the offending files. It does not fix it: rename the install migration to a version below the earliest of them so it replays first. How that back-dated version reaches a remote depends on what that remote actually has — check it, don't go by memory: `psql "$REMOTE_DATABASE_URL" -Atc "select eql_v3.version()"`. That function is created by the bundle's last statements, so it answers "is the whole install there"; a probe for the `eql_v3` schema does not, because the schema is created by the bundle's first statements and survives an install that aborted partway. If it prints a version, EQL is present and only the ledger row is missing — mark the version applied without executing any SQL: `supabase migration repair --status applied `. ⚠️ Do not push the file there instead: the bundle opens with `DROP SCHEMA IF EXISTS eql_v3 CASCADE` (and `eql_v3_internal`), so re-applying it drops every index, constraint, and RLS policy that references those schemas — see "Re-applying after `--force`" below. If the check errors, that remote genuinely still needs the SQL applied: `supabase db push --include-all`, because the back-dated version lands as a gap in the middle of that history. ⚠️ Never mark that remote applied. Every other remedy here fails loudly and can be retried; this one fails silently — the ledger row claims SQL that never ran, so no later push installs EQL, and the first migration referencing `eql_v3` fails with nothing pointing at the cause. + +**Re-applying after `--force`.** `--force` rewrites the install in place and keeps its version, so a database that already applied that version still has the old bundle — and `supabase db push` will *not* re-apply it. The Supabase CLI decides what is pending by comparing versions, never file content (seed files are hashed; migrations are not), so a version already in the ledger is simply never re-run and push reports `Remote database is up to date.` The working recipe: + +```bash +supabase db reset # local — replays every migration + +supabase migration repair --status reverted # remote — clear the ledger row first +supabase db push # ...then re-apply +``` + +`migration repair` updates the tracking table only; it applies no SQL. Add `--include-all` to that push only if it aborts with `Found local migration files to be inserted before the last migration on remote database.` — which happens when migrations sort *after* the install, the usual shape once encrypted-column migrations have been written against it. Reverting the newest version instead leaves it at the tail of remote history, where a plain push applies it. Don't reach for the flag pre-emptively: it applies every out-of-order migration you have, not just this one. ⚠️ Before doing this to a populated database: the EQL bundle opens with `DROP SCHEMA IF EXISTS eql_v3 CASCADE` (and `eql_v3_internal`), so re-applying also drops every index, constraint, and RLS policy that references those schemas. That is free on a fresh `supabase db reset` and destructive on a live remote. + +**Do not pass `--out` with a bare `--supabase`.** The Supabase CLI's migrations directory is not configurable: `supabase db reset` and `supabase db push` read `/supabase/migrations` and nothing else, there is no `config.toml` key to move it, and `--workdir` / `SUPABASE_WORKDIR` relocates the whole `supabase/` directory rather than this one. An install written anywhere else is simply never applied — the same "EQL is missing after a reset" failure that makes `eql install --supabase` the wrong tool on a CLI-scaffolded project. The command warns rather than refusing, because a project may have its own step that applies that directory; if you do not, drop the flag. (`--out` with `--drizzle --supabase` is unaffected — there it is drizzle-kit's output directory.) + +Pass exactly one target: `--drizzle`, `--supabase`, or `--prisma`. (`--drizzle --supabase` is not two targets — see above.) Either generated migration also installs the `cs_migrations` tracking schema, so one migrate step covers everything `stash encrypt …` needs. After writing the migration, `--drizzle` sweeps the output directory for sibling migrations containing an in-place `ALTER COLUMN … SET DATA TYPE ` — drizzle-kit emits these when you change a plaintext column to an encrypted one, and Postgres rejects them (there is no cast from `text`/`numeric` to an EQL type). Each is rewritten into a staged `ADD COLUMN` for the encrypted twin, while preserving the source column, and the rewritten files are listed. The rewrite never emits `DROP COLUMN` or `RENAME COLUMN`. If the sweep cannot prove a column's source type, finds that the encrypted twin already exists, or encounters another unsafe form, it leaves that statement untouched and the command exits non-zero so you review the migration directory before running `drizzle-kit migrate`. Populated plaintext tables then take the staged EQL v3 rollout from there: dual-write, backfill, switch the application to the encrypted column by name, and drop plaintext only after verification. diff --git a/skills/stash-supabase/SKILL.md b/skills/stash-supabase/SKILL.md index eac3e9789..71e5ece46 100644 --- a/skills/stash-supabase/SKILL.md +++ b/skills/stash-supabase/SKILL.md @@ -70,20 +70,100 @@ this is also how **Supabase Edge Functions** get credentials in local dev — ### 1. Install EQL v3 on the database +Install it as a **migration**, not directly: + ```bash -stash eql install --supabase -``` +stash eql migration --supabase # writes supabase/migrations/_cipherstash_eql.sql +supabase db reset # local — replays every migration +supabase db push # remote/linked project +``` + +> A bare `supabase migration up` applies to the **local** database. The remote +> forms are `supabase db push` and `supabase migration up --linked`. + +> ⚠️ **Do not use `stash eql install --supabase` on a project with a local +> `supabase/` directory.** It applies the SQL straight to the running database, +> and `supabase db reset` — the ordinary local development loop — drops that +> database and replays `supabase/migrations/`. EQL is not in there, so it is +> gone, and the next query fails with `type "eql_v3_encrypted" does not exist`. +> `stash eql install --supabase` is for a **hosted** project you administer +> without the Supabase CLI, where there is no migrations directory to write to. + +The generated file carries three things, in order: the EQL v3 bundle, the role +grants, and the `cipherstash.cs_migrations` tracking schema that `stash +encrypt` records per-column progress in. One `supabase db reset` therefore +provisions everything — no out-of-band `stash eql install` afterwards. + +It refuses to write a second install migration; pass `--force` to regenerate +the existing one in place (same version, so an applied ledger stays consistent). +Because the version is unchanged, `supabase db push` will **not** re-apply it — +the Supabase CLI decides what is pending by version, never by file content, so a +version already in the ledger is never re-run and push reports `Remote database +is up to date.` Re-apply with `supabase db reset` locally; on a remote, clear +the ledger row first with `supabase migration repair --status reverted +` (tracking table only — it applies no SQL) and then `supabase db +push`. Add `--include-all` to that push only if it aborts with `Found local +migration files to be inserted before the last migration on remote database.`, +which happens when migrations sort after the install; reverting the newest +version leaves it at the tail, where a plain push applies it. The flag applies +every out-of-order migration you have, so don't reach for it pre-emptively. +⚠️ On a populated database, weigh it first: the EQL +bundle opens with `DROP SCHEMA IF EXISTS eql_v3 CASCADE` (and +`eql_v3_internal`), so re-applying also drops every index, constraint, and RLS +policy that references those schemas. + +**If you already have encrypted-column migrations**, note that the generated +install is stamped with the current time and therefore sorts *after* them. A +reset replays in version order with no dependency awareness, so those migrations +run before EQL exists and `supabase db reset` fails with `type +"eql_v3_text_search" does not exist`. The command detects this and warns, naming +the files; the fix is to rename the install migration to a version below the +earliest of them. How that back-dated version reaches a remote depends on what +that remote actually has. This case only arises on a project that ran `stash eql +install` directly, so the remote usually has EQL already — but "usually" is not +what you want to bet a ledger row on, so check it: + +```bash +psql "$REMOTE_DATABASE_URL" -Atc "select eql_v3.version()" +``` + +`eql_v3.version()` is created by the bundle's last statements, so it answers "is +the whole install there". A probe for the `eql_v3` schema does not: that schema +is created by the bundle's first statements and survives an install that aborted +partway. + +If it prints a version, only the ledger row is missing — mark it applied with +`supabase migration repair --status applied `, which writes the row and +runs no SQL. ⚠️ Do not push the file there instead: that re-runs the bundle's +opening `DROP SCHEMA IF EXISTS eql_v3 CASCADE` (and `eql_v3_internal`), dropping +every index, constraint, and RLS policy that references those schemas. + +If it errors, that remote genuinely still needs the SQL applied: `supabase db +push --include-all`, the flag being required because the back-dated version is a +gap in the middle of that history. ⚠️ Never mark it applied there. Every other +remedy on this page fails loudly and can be retried; this one fails silently — +the ledger row claims SQL that never ran, so no later push installs EQL, and the +first query against an encrypted column fails with nothing pointing at the +cause. + +There is no `--out` to reach for here: the Supabase CLI's migrations directory +is not configurable. `supabase db reset` and `supabase db push` read +`/supabase/migrations` and nothing else, `config.toml` has no key for +it, and `--workdir` / `SUPABASE_WORKDIR` moves the whole `supabase/` directory +rather than this subdirectory. `stash eql migration --supabase --out ` +still writes the file, and warns, because a project may have its own step that +applies that directory — but the Supabase CLI will not, so EQL is gone again +after the next reset. Since eql-3.0.0 there is **one** v3 SQL artifact for every target — there is no separate Supabase variant. The bundle's only superuser-requiring statements (the ORE operator class/family) skip themselves when the install role lacks the privilege, and the bundle then disables the ORE-opclass-backed -domains it cannot support. `--supabase` changes one thing: it additionally -applies the role grants for `anon` / `authenticated` / `service_role` to the -two schemas the bundle creates — `eql_v3` (the operator-backing functions) -and `eql_v3_internal` (SEM internals). Without the grants, encrypted queries -fail loudly with a permission error (e.g. `permission denied for schema -eql_v3_internal`). +domains it cannot support. `--supabase` adds the role grants for `anon` / +`authenticated` / `service_role` on the two schemas the bundle creates — +`eql_v3` (the operator-backing functions) and `eql_v3_internal` (SEM +internals). Without the grants, encrypted queries fail loudly with a +permission error (e.g. `permission denied for schema eql_v3_internal`). No **Exposed schemas** change is needed: the column domains and their operators live in `public`, so bare `col = term` filters resolve under @@ -665,7 +745,10 @@ ALTER TABLE users ADD COLUMN email_encrypted public.eql_v3_text_search; -- nullable ``` -Apply with `supabase db reset` locally or `supabase migration up` against the remote project. +Apply with `supabase db reset` locally or `supabase db push` against the +remote project. The reset is safe here because the EQL install is itself a +migration (step 1) — it is replayed before this one, so the `eql_v3_text_search` +domain exists by the time this `ALTER TABLE` runs. No client-side schema change is required — `encryptedSupabase` introspects the new column's domain at the next client startup. If you use declared @@ -753,7 +836,7 @@ email_encrypted IS NULL` at apply time, raises if any remain, and only then drops the column. It requires the `backfilled` phase plus a live coverage check at generation time. Legacy v2 state is rejected. -Review and apply with `supabase migration up` (or `supabase db reset` locally). Then remove the dual-write code from app paths — the plaintext column is gone; only the encrypted column is written now, through the wrapper. +Review and apply with `supabase db reset` locally, or `supabase db push` against the remote project. Then remove the dual-write code from app paths — the plaintext column is gone; only the encrypted column is written now, through the wrapper. ### Inspecting progress at any time