fix(skills,wizard,cli): clear the residue from the v2 removal and #823

.changeset/shipped-skills-v2-removal-residue.md

@@ -0,0 +1,13 @@
+---
+'stash': patch
+---
+
+Correct the EQL v2 callout in the shipped `stash-encryption` skill.
+
+The skill opened by pointing at an older EQL v2 schema surface "with chainable
+capability builders" that "still exists for existing deployments". The v2 schema
+builders and the `@cipherstash/stack/client` subpath were removed; v2 is a
+read-compatibility path for stored payloads only, which is what the same file
+already said two sections later. The opening callout now says so — it is the
+first thing an agent reads in a customer's repo, and `SKILL_MAP.drizzle` installs
+this skill into every Drizzle project.

.changeset/wizard-db-push-residue-and-sweep-report.md

@@ -0,0 +1,31 @@
+---
+'@cipherstash/wizard': patch
+---
+
+Drop the last `stash db push` references from the wizard's output, and name the
+migration files a failed sweep rewrote before it stopped.
+
+- The "Post-agent steps complete" changelog line claimed `db push` had run.
+  `stash db push` was retired with the CipherStash Proxy lifecycle and
+  `runPostAgentSteps` never invoked it; the line now reports what the step
+  actually does (package install, `eql install`, migrations). The `--plan` help
+  text no longer promises "no db pushes" either, and the package README — which
+  ships in the tarball — no longer lists `db push` as a prerequisite or a
+  post-agent step.
+- When a candidate directory's ALTER COLUMN sweep threw, the wizard reported the
+  failure but skipped the per-directory report, so files it had already rewritten
+  on disk — and statements it had flagged — went unnamed. It now lists them
+  ("Rewrote N migration file(s) in drizzle/ before the sweep stopped", followed by
+  the flagged statements and their reasons), matching
+  `stash eql migration --drizzle`.
+- The cross-directory summary ("Rewrote N migration file(s) in the drizzle output
+  to add staged encrypted columns while preserving the source columns") is now
+  suppressed when any directory failed to sweep. It is built from a total that
+  counts clean and partially-swept directories alike, so on that path it restated
+  the reassuring framing the per-directory report deliberately drops. A *flagged*
+  statement still prints the summary — there the sweep finished and the count is
+  accurate.
+
+Both are reporting-only: the rewritten SQL is additive and the wizard still
+throws before the migrate prompt, so this changes what the user is told, not what
+runs.

packages/wizard/README.md

@@ -19,8 +19,8 @@ bunx @cipherstash/wizard      # bun
 
 Before running the wizard, your project should have:
 
-- `stash` available (the wizard shells out to `stash eql install` /
-  `db push` after the agent finishes editing)
+- `stash` available (the wizard shells out to `stash eql install` after the
+  agent finishes editing)
 - A `stash.config.ts` (or the wizard will run `stash eql install` to scaffold one)
 - A reachable database via `DATABASE_URL`
 - An authenticated CipherStash session (`stash auth login`)
@@ -33,7 +33,7 @@ Before running the wizard, your project should have:
 4. Hands a surgical prompt to the Claude Agent SDK, which edits your schema
    and call sites to use `@cipherstash/stack`'s encryption APIs.
 5. Runs deterministic post-agent steps: package install, `eql install`,
-   `db push`, framework-specific migrations.
+   framework-specific migrations.
 6. Reports remaining call sites that need `encryptModel` / `decryptModel`
    wiring.
 

packages/wizard/src/__tests__/format.test.ts

@@ -71,7 +71,7 @@ describe('formatAgentOutput', () => {
       '## Next Steps',
       '',
       '1. Run `npx drizzle-kit generate`',
-      '2. Run `npx stash db push`',
+      '2. Run `npx drizzle-kit migrate`',
     ].join('\n')
 
     const result = formatAgentOutput(input)
@@ -81,5 +81,12 @@ describe('formatAgentOutput', () => {
     expect(result).toContain(pc.cyan('stash.config.ts'))
     expect(result).toContain(pc.bold(pc.cyan('Next Steps')))
     expect(result).toContain(pc.dim('1.'))
+    // Pin the second item's content, not just its marker: `stash db push` was
+    // retired with the Proxy lifecycle (#814 / #825), and an unasserted fixture
+    // is how it survived here in the first place (#837).
+    expect(result).toContain(
+      `${pc.dim('2.')} Run ${pc.cyan('npx drizzle-kit migrate')}`,
+    )
+    expect(result).not.toContain('stash db push')
   })
 })

packages/wizard/src/__tests__/interface.test.ts

@@ -145,9 +145,9 @@ describe('wizardCanUseTool', () => {
       expect(wizardCanUseTool('Bash', { command: 'npx tsc --noEmit' })).toBe(
         true,
       )
-      expect(wizardCanUseTool('Bash', { command: 'npx stash db push' })).toBe(
-        true,
-      )
+      expect(
+        wizardCanUseTool('Bash', { command: 'npx stash db validate' }),
+      ).toBe(true)
     })
 
     it('blocks commands not in allowlist', () => {

packages/wizard/src/__tests__/post-agent.test.ts

@@ -15,9 +15,12 @@ vi.mock('@clack/prompts', async (importOriginal) => {
   return { ...actual, confirm: vi.fn(async () => false) }
 })
 
-// Wraps the REAL sweep, so every test below still exercises it for free. Only
-// the empty-message case overrides it, because no real filesystem error is
-// reachable with a blank `message`.
+// Wraps the REAL sweep, so every test below still exercises it for free. Two
+// cases override it, both because the state they need is not reachable through
+// the real filesystem from here: the empty-error-`message` case, and the
+// partial-set case, which needs the sweep to throw mid-rewrite-loop rather than
+// during its read pass. Each override says so at the call site; prefer a real
+// fixture for anything else.
 vi.mock('../lib/rewrite-migrations.js', async (importOriginal) => {
   const actual =
     await importOriginal<typeof import('../lib/rewrite-migrations.js')>()
@@ -207,6 +210,7 @@ describe('drizzle migrate prompt after a staged rewrite', () => {
   })
 
   it('defaults to Yes and explains the staged addition when a file was rewritten', async () => {
+    const info = vi.spyOn(p.log, 'info').mockImplementation(() => {})
     makeDrizzleOut('drizzle')
     // The sweep is fail-closed: it rewrites a column only when the corpus
     // positively declares it (and it isn't already encrypted). A real drizzle
@@ -238,6 +242,20 @@ describe('drizzle migrate prompt after a staged rewrite', () => {
     expect(swept).toContain('ADD COLUMN "email_encrypted"')
     expect(swept).not.toMatch(/\b(?:DROP|RENAME)\s+COLUMN\b/i)
     expect(swept).not.toContain('SET DATA TYPE')
+
+    // The per-directory report takes the clean arm of its message: a sweep that
+    // finished must not borrow the "before the sweep stopped" wording the
+    // partial path uses. Anchored on `in drizzle/` so the cross-directory
+    // summary ("in the drizzle output") cannot satisfy it instead.
+    expect(info).toHaveBeenCalledWith(
+      expect.stringContaining(
+        'in drizzle/ to add staged encrypted columns while preserving the source columns',
+      ),
+    )
+    expect(info).not.toHaveBeenCalledWith(
+      expect.stringContaining('before the sweep stopped'),
+    )
+    info.mockRestore()
   })
 
   it('fails before prompting when a statement was flagged rather than rewritten', async () => {
@@ -280,6 +298,122 @@ describe('drizzle migrate prompt after a staged rewrite', () => {
     warn.mockRestore()
   })
 
+  // The cross-directory summary is printed off `totals.rewritten`, which counts
+  // a clean directory and a failed one alike. Printing "…while preserving the
+  // source columns" after a directory failed re-asserts the reassuring framing
+  // the per-directory report deliberately drops, for a sweep that did not
+  // finish. The per-directory lines already name every file, so the summary has
+  // nothing to add on that path.
+  it('does not summarise the sweep as clean when another directory failed', async () => {
+    const warn = vi.spyOn(p.log, 'warn').mockImplementation(() => {})
+    const info = vi.spyOn(p.log, 'info').mockImplementation(() => {})
+    // drizzle/ declares its column, so its ALTER is rewritten cleanly...
+    makeDrizzleOut('drizzle')
+    fs.writeFileSync(
+      path.join(cwd, 'drizzle', '0000_declare.sql'),
+      'CREATE TABLE "users" ("email" text);\n',
+    )
+    fs.writeFileSync(
+      path.join(cwd, 'drizzle', '0001_encrypt.sql'),
+      'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search;\n',
+    )
+    // ...while migrations/ cannot be swept at all: a directory named `*.sql`
+    // makes readFile throw EISDIR mid-sweep.
+    makeDrizzleOut('migrations')
+    fs.mkdirSync(path.join(cwd, 'migrations', '0001_alter.sql'))
+
+    await expect(runDrizzle()).rejects.toThrow('unsafe or unverified SQL')
+
+    expect(info).not.toHaveBeenCalledWith(
+      expect.stringContaining('in the drizzle output'),
+    )
+    // The per-directory report still stands on its own for the clean directory.
+    expect(info).toHaveBeenCalledWith(
+      expect.stringContaining(
+        'in drizzle/ to add staged encrypted columns while preserving the source columns',
+      ),
+    )
+    expect(warn).toHaveBeenCalledWith(
+      expect.stringContaining('Could not rewrite migrations in migrations'),
+    )
+    info.mockRestore()
+    warn.mockRestore()
+  })
+
+  // A sweep that throws part-way through has already written the files it got
+  // to, and may have flagged statements before it stopped —
+  // `sweepMigrationDirs` propagates both on the failure path. The reporting used
+  // to sit after a `continue`, so the wizard surfaced neither: the user was told
+  // a directory failed without being told which of its files had changed or
+  // what it had flagged. The CLI twin has always reported the partial set
+  // (`packages/cli/src/commands/eql/migration.ts`). #837.
+  //
+  // Mocked, unlike the tests above: the throw has to land *inside* the rewrite
+  // loop to leave a partial set behind, which needs a mid-loop `writeFile`
+  // failure (see `rewrite-migrations.test.ts`, "reports files rewritten before a
+  // later write failure"). The real filesystem errors reachable from here throw
+  // during the read pass, before anything is rewritten or flagged.
+  it('names the files rewritten and statements flagged before a failed sweep stopped', async () => {
+    const warn = vi.spyOn(p.log, 'warn').mockImplementation(() => {})
+    const info = vi.spyOn(p.log, 'info').mockImplementation(() => {})
+    const step = vi.spyOn(p.log, 'step').mockImplementation(() => {})
+    vi.mocked(sweepMigrationDirs).mockResolvedValueOnce([
+      {
+        dir: 'drizzle',
+        rewritten: ['drizzle/0001_email.sql'],
+        // The twin the rewritten file added. Carried on the failure path
+        // deliberately: it is already on disk, and already divergent from
+        // `schema.ts` and the snapshot.
+        staged: [
+          {
+            file: 'drizzle/0001_email.sql',
+            table: 'users',
+            column: 'email',
+            encryptedColumn: 'email_encrypted',
+            domain: 'eql_v3_text_search',
+          },
+        ],
+        skipped: [
+          {
+            file: 'drizzle/0002_total.sql',
+            statement:
+              'ALTER TABLE "orders" ALTER COLUMN "total" SET DATA TYPE eql_v3_text_search;',
+            reason: 'source-unknown',
+          },
+        ],
+        error: 'EACCES: permission denied, open drizzle/0003_name.sql',
+      },
+    ])
+
+    await expect(runDrizzle()).rejects.toThrow('unsafe or unverified SQL')
+
+    expect(p.confirm).not.toHaveBeenCalled()
+    // The failure itself is still reported...
+    expect(warn).toHaveBeenCalledWith(
+      expect.stringContaining('Could not rewrite migrations in drizzle'),
+    )
+    // ...and so is what it changed on the way there, named file by file.
+    expect(info).toHaveBeenCalledWith(
+      expect.stringContaining('before the sweep stopped'),
+    )
+    expect(step).toHaveBeenCalledWith(
+      expect.stringContaining('drizzle/0001_email.sql'),
+    )
+    // ...as is what it flagged, with the reason the user has to act on.
+    expect(warn).toHaveBeenCalledWith(
+      expect.stringContaining('rewrite left alone'),
+    )
+    expect(step).toHaveBeenCalledWith(
+      expect.stringContaining('drizzle/0002_total.sql'),
+    )
+    expect(step).toHaveBeenCalledWith(
+      expect.stringContaining('could not find where this column was declared'),
+    )
+    step.mockRestore()
+    info.mockRestore()
+    warn.mockRestore()
+  })
+
   // `error` is built as `err instanceof Error ? err.message : String(err)`, and
   // `new Error()` has an empty message — so a thrown error can arrive as `''`.
   // Testing it for truthiness rather than presence would drop that directory

packages/wizard/src/agent/__tests__/interface.test.ts

@@ -88,8 +88,10 @@ describe('wizardCanUseTool — DLX command allowlist', () => {
       )
     })
 
-    it('allows stash db push', () => {
-      expect(wizardCanUseTool('Bash', { command: 'stash db push' })).toBe(true)
+    it('allows stash db validate', () => {
+      expect(wizardCanUseTool('Bash', { command: 'stash db validate' })).toBe(
+        true,
+      )
     })
   })
 

packages/wizard/src/bin/wizard.ts

@@ -40,7 +40,7 @@ Options:
   --version, -v        Show version
   --debug              Print extra diagnostics from the agent
   --plan               Drafts \`.cipherstash/plan.md\` for review.
-                       No code or schema changes, no db pushes.
+                       No code or schema changes, no migrations run.
   --implement          Full setup flow (the default).
   --mode <plan|implement>
                        Long form of \`--plan\` / \`--implement\`. Last mode

packages/wizard/src/lib/post-agent.ts

@@ -78,7 +78,14 @@ export async function runPostAgentSteps(opts: PostAgentOptions): Promise<void> {
     const skipped = sweep.skipped > 0
     const unverified = sweep.failedDirs.length > 0
 
-    if (didStage) {
+    // Suppressed when a directory failed: this line is a cross-directory
+    // summary built from `totals.rewritten`, which counts a clean directory and
+    // a partially-swept one alike, so on that path it re-asserts the reassuring
+    // framing the per-directory report deliberately drops. Those per-directory
+    // lines already name every file, so nothing is lost by staying quiet here.
+    // A *flagged* statement is different — the sweep finished, and the summary
+    // is accurate — so this stays gated on the failure, not on `skipped`.
+    if (didStage && !unverified) {
       p.log.info(
         `Rewrote ${sweep.rewritten} migration file(s) in the drizzle output to add staged encrypted columns while preserving the source columns.`,
       )
@@ -196,12 +203,19 @@ async function rewriteEncryptedMigrations(cwd: string): Promise<{
       p.log.warn(
         `Could not rewrite migrations in ${dir}: ${error || 'unknown error'}`,
       )
-      continue
+      // Deliberately NOT `continue`: a directory whose sweep threw may already
+      // have rewritten files on disk, and `sweepMigrationDirs` propagates that
+      // partial set on the failure path. Skipping the reporting below would
+      // leave the user with a failure and no list of what it changed before it
+      // stopped. The CLI twin reports the partial set for the same reason —
+      // `packages/cli/src/commands/eql/migration.ts` (#786, #837).
     }
 
     if (rewritten.length > 0) {
       p.log.info(
-        `Rewrote ${rewritten.length} migration file(s) in ${dir}/ to add staged encrypted columns while preserving the source columns.`,
+        error === undefined
+          ? `Rewrote ${rewritten.length} migration file(s) in ${dir}/ to add staged encrypted columns while preserving the source columns.`
+          : `Rewrote ${rewritten.length} migration file(s) in ${dir}/ before the sweep stopped:`,
       )
       for (const file of rewritten) p.log.step(`  - ${file}`)
     }

packages/wizard/src/run.ts

@@ -14,7 +14,6 @@ import {
   trackWizardStarted,
 } from './lib/analytics.js'
 import { WizardChangelog } from './lib/changelog.js'
-import { INTEGRATIONS } from './lib/constants.js'
 import {
   detectIntegration,
   detectPackageManager,
@@ -240,7 +239,7 @@ export async function run(options: RunOptions) {
         })
         changelog.phase(
           'Post-agent steps complete',
-          'Package install, `eql install`, `db push`, and migrations finished.',
+          'Package install, `eql install`, and migrations finished.',
         )
 
         const scanResult = await scanPromise

skills/stash-drizzle/SKILL.md

@@ -61,7 +61,7 @@ stash eql migration --drizzle --supabase   # also grants eql_v3 to anon/authenti
 
 The generated migration also installs the `cs_migrations` tracking schema, so a single `drizzle-kit migrate` covers everything `stash encrypt …` needs — no out-of-band `stash eql install`. EQL v3 ships one SQL bundle for every target including Supabase; `--supabase` only adds the PostgREST/RLS role grants (harmless when you connect directly as `postgres`). Requires `drizzle-kit` installed and configured.
 
-**Changing an existing plaintext column to an encrypted one.** `drizzle-kit generate` emits an in-place `ALTER TABLE … ALTER COLUMN … SET DATA TYPE eql_v3_<name>`, which Postgres rejects — there is no cast from `text`/`numeric` to an EQL domain. (On drizzle-kit 0.31.0 and later the emitted type is also mangled to `"undefined"."eql_v3_<name>"`, since a `customType` has no `typeSchema`.) The `stash eql migration --drizzle` sweep repairs the invalid statement — the `stash-cli` skill covers what it rewrites. The repair is **add-only**: it adds a staged `<column>_encrypted` column and leaves the source column in place, so it never emits `DROP COLUMN` or `RENAME COLUMN` and is safe to apply on a populated table. It repairs only what it can place: the swept directory must also contain the migration that declared the column. If the sweep cannot prove the source column's type, or the encrypted twin already exists, it leaves that statement untouched and the command exits non-zero so you review the directory before running `drizzle-kit migrate`. Applying the swept migration only *adds* the column — encrypting the data is the staged flow in **Migrating an Existing Column to Encrypted** below.
+**Changing an existing plaintext column to an encrypted one.** `drizzle-kit generate` emits an in-place `ALTER TABLE … ALTER COLUMN … SET DATA TYPE eql_v3_<name>`, which Postgres rejects — there is no cast from `text`/`numeric` to an EQL domain. (On drizzle-kit 0.31.0 and later the emitted type is also mangled to `"undefined"."eql_v3_<name>"`, since a `customType` has no `typeSchema`.) The `stash eql migration --drizzle` sweep repairs the invalid statement — the `stash-cli` skill covers what it rewrites. The repair is **add-only**: it adds a staged `<column>_encrypted` column and leaves the source column in place, so it never emits `DROP COLUMN` or `RENAME COLUMN` and is safe to apply on a populated table. It repairs only what it can prove — the swept directory must also contain the migration that declared the column, so the sweep can see the source type. A statement it cannot place, one whose column is already encrypted, one whose encrypted twin already exists, or one outside the strict matcher (a hand-authored `SET DATA TYPE … USING …`) is left untouched, and the command exits non-zero so you review the directory before running `drizzle-kit migrate`. Applying the swept migration only *adds* the column — encrypting the data is the staged flow in **Migrating an Existing Column to Encrypted** below.
 
 **Reconcile your schema after a sweep — `drizzle-kit generate` will not tell you to.** The sweep repairs SQL and nothing else, so once you apply the swept migration three artefacts disagree: