From 46f4b34f6f1408920d731c938baeb60cbddb1fce Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 30 Jul 2026 12:54:03 +1000 Subject: [PATCH 1/5] fix(skills,wizard): clear the residue from the v2 removal and #823 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every item here has the same root cause: a change updated one copy of something and left the other. Closes #837. **`stash-encryption` contradicted itself in the customer's repo.** `skills/stash-encryption/SKILL.md:12` pointed at an EQL v2 schema surface "with chainable capability builders" that "still exists", while `:201` and `:1015` of the same file said the v2 builders are gone and v2 is a decrypt-only read path — which is what the code does after #829. The opening callout now says that. It is the first thing an agent reads, and `SKILL_MAP.drizzle` installs this skill into every Drizzle project. **`db push` residue.** `stash db push` was retired with the Proxy lifecycle (#814 / #825) and `post-agent.ts` says it "must never run", but the "Post-agent steps complete" changelog line still claimed it had, and three tests used it as their example of an allowed `stash db` command. The tests now use `db validate`, which is in the manifest. Also dropped "no db pushes" from the `--plan` help text. The `'stash db'` allowlist prefix is unchanged — it still serves `db validate`, `db migrate` and `db test-connection`. **Wizard sweep reporting gap.** When a directory's sweep threw, the `continue` in `rewriteEncryptedMigrations` skipped the per-directory report, so files it had already rewritten on disk went unnamed — `sweepMigrationDirs` propagates that partial set precisely so the caller can report it, and the CLI twin does. Now reported as "Rewrote N migration file(s) in / before the sweep stopped", with the file list. Behaviour-neutral otherwise: the SQL is additive and the wizard still throws before the migrate prompt. Two tests pin it, including one that the clean path does not borrow the partial wording; the first fails without the fix. Dropped in the rebase: the `stash-drizzle` add-only correction and the CLI rewriter test assertion this commit originally carried both landed on main independently (91452c14), so the tree already has them. The `stash-drizzle` bullet is gone from the changeset for the same reason. The `err as Partial` cast at `rewrite-migrations.ts:946` is deliberately untouched — it is a fail-closed correctness defect tracked as item 3 of #836, not cleanup. --- .../shipped-skills-v2-removal-residue.md | 13 +++++ ...wizard-db-push-residue-and-sweep-report.md | 18 ++++++ packages/wizard/src/__tests__/format.test.ts | 2 +- .../wizard/src/__tests__/interface.test.ts | 6 +- .../wizard/src/__tests__/post-agent.test.ts | 58 +++++++++++++++++++ .../src/agent/__tests__/interface.test.ts | 6 +- packages/wizard/src/bin/wizard.ts | 2 +- packages/wizard/src/lib/post-agent.ts | 11 +++- packages/wizard/src/run.ts | 2 +- skills/stash-encryption/SKILL.md | 2 +- 10 files changed, 109 insertions(+), 11 deletions(-) create mode 100644 .changeset/shipped-skills-v2-removal-residue.md create mode 100644 .changeset/wizard-db-push-residue-and-sweep-report.md diff --git a/.changeset/shipped-skills-v2-removal-residue.md b/.changeset/shipped-skills-v2-removal-residue.md new file mode 100644 index 000000000..efa45721f --- /dev/null +++ b/.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. diff --git a/.changeset/wizard-db-push-residue-and-sweep-report.md b/.changeset/wizard-db-push-residue-and-sweep-report.md new file mode 100644 index 000000000..f0c3da579 --- /dev/null +++ b/.changeset/wizard-db-push-residue-and-sweep-report.md @@ -0,0 +1,18 @@ +--- +'@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. +- 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 went unnamed. It now lists them ("Rewrote N migration file(s) in + drizzle/ before the sweep stopped"), matching `stash eql migration --drizzle`. + The rewritten SQL is additive and the wizard still fails before the migrate + prompt, so this changes what the user is told, not what runs. diff --git a/packages/wizard/src/__tests__/format.test.ts b/packages/wizard/src/__tests__/format.test.ts index acc5d5f7d..0e8126f5b 100644 --- a/packages/wizard/src/__tests__/format.test.ts +++ b/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) diff --git a/packages/wizard/src/__tests__/interface.test.ts b/packages/wizard/src/__tests__/interface.test.ts index b81a25911..9368eccf1 100644 --- a/packages/wizard/src/__tests__/interface.test.ts +++ b/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', () => { diff --git a/packages/wizard/src/__tests__/post-agent.test.ts b/packages/wizard/src/__tests__/post-agent.test.ts index ebd4d4f51..dd6b22a9a 100644 --- a/packages/wizard/src/__tests__/post-agent.test.ts +++ b/packages/wizard/src/__tests__/post-agent.test.ts @@ -280,6 +280,64 @@ describe('drizzle migrate prompt after a staged rewrite', () => { warn.mockRestore() }) + // A sweep that throws part-way through has already written the files it got + // to — `sweepMigrationDirs` propagates that partial set on the failure path. + // The reporting used to sit after a `continue`, so the wizard named none of + // them and the user was told a directory failed without being told which of + // its files had changed. The CLI twin has always reported the partial set + // (`packages/cli/src/commands/eql/migration.ts`). #837. + it('names the files rewritten 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'], + skipped: [], + error: 'EISDIR: illegal operation on a directory, read', + }, + ]) + + 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'), + ) + step.mockRestore() + info.mockRestore() + warn.mockRestore() + }) + + // The partial-report path above must not bleed into the clean one: a + // successful sweep still says it preserved the source columns, not that it + // stopped. + it('does not claim the sweep stopped when it completed', async () => { + const info = vi.spyOn(p.log, 'info').mockImplementation(() => {}) + vi.mocked(sweepMigrationDirs).mockResolvedValueOnce([ + { dir: 'drizzle', rewritten: ['drizzle/0001_email.sql'], skipped: [] }, + ]) + + await runDrizzle() + + expect(info).toHaveBeenCalledWith( + expect.stringContaining('preserving the source columns'), + ) + expect(info).not.toHaveBeenCalledWith( + expect.stringContaining('before the sweep stopped'), + ) + info.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 diff --git a/packages/wizard/src/agent/__tests__/interface.test.ts b/packages/wizard/src/agent/__tests__/interface.test.ts index a14be3d6a..137bb00db 100644 --- a/packages/wizard/src/agent/__tests__/interface.test.ts +++ b/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, + ) }) }) diff --git a/packages/wizard/src/bin/wizard.ts b/packages/wizard/src/bin/wizard.ts index 9452261e4..b33cd3019 100644 --- a/packages/wizard/src/bin/wizard.ts +++ b/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 Long form of \`--plan\` / \`--implement\`. Last mode diff --git a/packages/wizard/src/lib/post-agent.ts b/packages/wizard/src/lib/post-agent.ts index 30201b77d..51f39e110 100644 --- a/packages/wizard/src/lib/post-agent.ts +++ b/packages/wizard/src/lib/post-agent.ts @@ -196,12 +196,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}`) } diff --git a/packages/wizard/src/run.ts b/packages/wizard/src/run.ts index 134732138..b1326d854 100644 --- a/packages/wizard/src/run.ts +++ b/packages/wizard/src/run.ts @@ -240,7 +240,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 diff --git a/skills/stash-encryption/SKILL.md b/skills/stash-encryption/SKILL.md index 8e682d01c..48a123266 100644 --- a/skills/stash-encryption/SKILL.md +++ b/skills/stash-encryption/SKILL.md @@ -9,7 +9,7 @@ Comprehensive guide for implementing field-level encryption with `@cipherstash/s Encrypted columns are **EQL v3 concrete Postgres domains** (`public.eql_v3_text_search`, `public.eql_v3_integer_ord`, ...): each column's query capabilities are fixed by the domain type you pick in the schema, and the `Encryption` client (typed for an all-v3 schema set) derives precise TypeScript types from that schema — wrong-typed plaintext is a compile error, not a runtime surprise. -> An older schema surface (EQL v2, with chainable capability builders) still exists for existing deployments — see "Legacy: EQL v2" at the end. Everything else in this skill is the v3 surface. New projects should use v3. +> EQL v2 is a **read-compatibility path only**. The v2 schema builders and the `@cipherstash/stack/client` subpath have been removed; `decrypt` / `decryptModel` still read stored v2 payloads so existing deployments keep working — see "Legacy: EQL v2" at the end. Author every schema and every new write with the v3 surface this skill describes. ## When to Use This Skill From 6a616234667001b7bc50ace0f2d181ef2ca6178a Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 30 Jul 2026 10:14:01 +1000 Subject: [PATCH 2/5] test(wizard): assert the format fixture's second step, not just its marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `stash db push` → `drizzle-kit migrate` fixture swap in the previous commit was unasserted: the test pinned `pc.dim('1.')` and nothing about item 2, so the retired command could drift back in and the suite would stay green — which is how it survived the Proxy-lifecycle removal (#814 / #825) here in the first place. Pins the rendered second item (`2. Run npx drizzle-kit migrate`) and the absence of `stash db push`, leaving the existing heading/checkmark/inline-code assertions untouched. Verified the new assertions fail when the fixture is reverted to `stash db push`. --- packages/wizard/src/__tests__/format.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/wizard/src/__tests__/format.test.ts b/packages/wizard/src/__tests__/format.test.ts index 0e8126f5b..ce855d12f 100644 --- a/packages/wizard/src/__tests__/format.test.ts +++ b/packages/wizard/src/__tests__/format.test.ts @@ -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') }) }) From 459cf55a3bb2c39717f5a0e89bed279609cf2a0e Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 30 Jul 2026 10:27:23 +1000 Subject: [PATCH 3/5] fix(wizard,skills): address review findings on the sweep report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Suppress the cross-directory summary when a sweep failed** (review finding 3). `runPostAgentSteps` prints "Rewrote N migration file(s) in the drizzle output to add staged encrypted columns while preserving the source columns" off `totals.rewritten`, which counts a clean directory and a partially-swept one alike. On the failure path that restated the reassuring framing the new per-directory report deliberately drops, immediately after it. Gated on the failure, not on `skipped` — a flagged statement means the sweep finished and the count is accurate. Test written first and watched fail on the second `info` call. **Cover the flagged statements a failed sweep leaves behind** (review finding 6). Removing the `continue` also let a failed directory report its `skipped` statements, which nothing asserted. Folded into the partial-set test, and verified load-bearing by gating the skipped block on `error === undefined` and watching the assertion fail. **Stop mocking the sweep for the clean path** (review findings 1 and 2). The clean-arm test overrode `sweepMigrationDirs` where a real fixture works, against the convention stated at the top of the file — and that override silently made the file's header comment false, which is exactly the defect class this branch exists to close. Deleted it; its two assertions now live on the existing real-fixture test, anchored on `in drizzle/` so the cross-directory summary cannot satisfy them instead. The header comment now names the two remaining overrides and why each is unreachable through the real filesystem, and each override repeats the reason at its call site. Verified by flipping the ternary: three tests fail, including the folded one. **Name all four skip reasons in the Drizzle skill** (review finding 4). The paragraph listed `source-unknown` and `target-exists` but not `already-encrypted` or `unrecognised-form`, so a reader could infer a hand-authored `SET DATA TYPE … USING …` gets repaired. Also cut the duplicated statement of the declaring-migration requirement. Verified: wizard 344 passed / 5 skipped, cli 895 passed, `tsc --noEmit` clean, biome clean on the changed files. Review finding 5 (skills also ship in the `@cipherstash/wizard` tarball, but the skills changeset is scoped to `stash` alone) is left as-is: it follows repo precedent (`adapter-split-skills.md`), and both tarballs are bumped here anyway. It is a question about the convention, not this branch. --- ...wizard-db-push-residue-and-sweep-report.md | 19 ++- .../wizard/src/__tests__/post-agent.test.ts | 124 +++++++++++++----- packages/wizard/src/lib/post-agent.ts | 9 +- skills/stash-drizzle/SKILL.md | 2 +- 4 files changed, 118 insertions(+), 36 deletions(-) diff --git a/.changeset/wizard-db-push-residue-and-sweep-report.md b/.changeset/wizard-db-push-residue-and-sweep-report.md index f0c3da579..3c6214be5 100644 --- a/.changeset/wizard-db-push-residue-and-sweep-report.md +++ b/.changeset/wizard-db-push-residue-and-sweep-report.md @@ -12,7 +12,18 @@ migration files a failed sweep rewrote before it stopped. text no longer promises "no db pushes" either. - 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 went unnamed. It now lists them ("Rewrote N migration file(s) in - drizzle/ before the sweep stopped"), matching `stash eql migration --drizzle`. - The rewritten SQL is additive and the wizard still fails before the migrate - prompt, so this changes what the user is told, not what runs. + 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. diff --git a/packages/wizard/src/__tests__/post-agent.test.ts b/packages/wizard/src/__tests__/post-agent.test.ts index dd6b22a9a..7858a151e 100644 --- a/packages/wizard/src/__tests__/post-agent.test.ts +++ b/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() @@ -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,13 +298,62 @@ 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 — `sweepMigrationDirs` propagates that partial set on the failure path. - // The reporting used to sit after a `continue`, so the wizard named none of - // them and the user was told a directory failed without being told which of - // its files had changed. The CLI twin has always reported the partial set + // 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. - it('names the files rewritten before a failed sweep stopped', async () => { + // + // 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(() => {}) @@ -294,8 +361,15 @@ describe('drizzle migrate prompt after a staged rewrite', () => { { dir: 'drizzle', rewritten: ['drizzle/0001_email.sql'], - skipped: [], - error: 'EISDIR: illegal operation on a directory, read', + 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', }, ]) @@ -313,29 +387,19 @@ describe('drizzle migrate prompt after a staged rewrite', () => { expect(step).toHaveBeenCalledWith( expect.stringContaining('drizzle/0001_email.sql'), ) - step.mockRestore() - info.mockRestore() - warn.mockRestore() - }) - - // The partial-report path above must not bleed into the clean one: a - // successful sweep still says it preserved the source columns, not that it - // stopped. - it('does not claim the sweep stopped when it completed', async () => { - const info = vi.spyOn(p.log, 'info').mockImplementation(() => {}) - vi.mocked(sweepMigrationDirs).mockResolvedValueOnce([ - { dir: 'drizzle', rewritten: ['drizzle/0001_email.sql'], skipped: [] }, - ]) - - await runDrizzle() - - expect(info).toHaveBeenCalledWith( - expect.stringContaining('preserving the source columns'), + // ...as is what it flagged, with the reason the user has to act on. + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('rewrite left alone'), ) - expect(info).not.toHaveBeenCalledWith( - expect.stringContaining('before the sweep stopped'), + 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 diff --git a/packages/wizard/src/lib/post-agent.ts b/packages/wizard/src/lib/post-agent.ts index 51f39e110..3b0df666a 100644 --- a/packages/wizard/src/lib/post-agent.ts +++ b/packages/wizard/src/lib/post-agent.ts @@ -78,7 +78,14 @@ export async function runPostAgentSteps(opts: PostAgentOptions): Promise { 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.`, ) diff --git a/skills/stash-drizzle/SKILL.md b/skills/stash-drizzle/SKILL.md index bf0740424..779aa8f0a 100644 --- a/skills/stash-drizzle/SKILL.md +++ b/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_`, 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_"`, 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 `_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_`, 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_"`, 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 `_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: From 0fc041ac5f48bd333a7c8847b2abacdec07f639e Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 30 Jul 2026 12:43:02 +1000 Subject: [PATCH 4/5] fix(wizard): drop the db push residue from the shipped README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The README still named `db push` in two places — as a `stash` prerequisite ("shells out to `stash eql install` / `db push`") and as a post-agent step. `runPostAgentSteps` never invoked it, so both lines were the same false claim this branch removes from `run.ts` and `wizard.ts`. README.md is in the wizard package's `files` array, so it ships in the tarball: leaving it recreates the one-copy-updated defect #837 is about. Also removes the unused `INTEGRATIONS` import from `run.ts`, which was the file's only biome warning. --- .changeset/wizard-db-push-residue-and-sweep-report.md | 4 +++- packages/wizard/README.md | 6 +++--- packages/wizard/src/run.ts | 1 - 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.changeset/wizard-db-push-residue-and-sweep-report.md b/.changeset/wizard-db-push-residue-and-sweep-report.md index 3c6214be5..ffaa963be 100644 --- a/.changeset/wizard-db-push-residue-and-sweep-report.md +++ b/.changeset/wizard-db-push-residue-and-sweep-report.md @@ -9,7 +9,9 @@ migration files a failed sweep rewrote before it stopped. `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. + 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 diff --git a/packages/wizard/README.md b/packages/wizard/README.md index 89965ce48..3d8a559fd 100644 --- a/packages/wizard/README.md +++ b/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. diff --git a/packages/wizard/src/run.ts b/packages/wizard/src/run.ts index b1326d854..3e5d579f8 100644 --- a/packages/wizard/src/run.ts +++ b/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, From e2382d292f7dead29e6db52baf4a5f5ffbefc39c Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 30 Jul 2026 12:57:23 +1000 Subject: [PATCH 5/5] test(wizard): carry the staged twin through the failed-sweep fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rebase onto main lands `sweep.staged` (#836, item 2) under the mocked `sweepMigrationDirs` result in the partial-set test, which predates it: the fixture had no `staged`, so `totals.staged.push(...staged)` threw "staged is not iterable" and the test failed on the wrong error. The twin is spelled out rather than stubbed to `[]` — the fixture's rewritten `0001_email.sql` is exactly a file that added one, and `rewriteEncryptedMigrations` accumulates twins from a directory whose sweep later threw precisely because they are already on disk and already divergent from `schema.ts` and the snapshot. A stub would have made the fixture contradict the code path it stands in for. --- packages/wizard/src/__tests__/post-agent.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/wizard/src/__tests__/post-agent.test.ts b/packages/wizard/src/__tests__/post-agent.test.ts index 7858a151e..96286900c 100644 --- a/packages/wizard/src/__tests__/post-agent.test.ts +++ b/packages/wizard/src/__tests__/post-agent.test.ts @@ -361,6 +361,18 @@ describe('drizzle migrate prompt after a staged rewrite', () => { { 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',