fix(cli): mixed-table encrypt lifecycle, and a stash init scaffold that compiles (#772 review findings 6, 7) by tobyhede · Pull Request #787 · cipherstash/stack

tobyhede · 2026-07-25T04:34:33Z

Review remediation for #772 — findings 6 and 7, both verified by executing the real commands.

Finding 7 — `encrypt cutover`/`drop` acted on a guessed column

On a table holding a legacy EQL v2 pair (ssn / ssn_encrypted) plus one unrelated EQL v3 column, the v2 ciphertext column is not classified as an EQL column at all, so resolution fell through to the sole-EQL-column rule and claimed the unrelated v3 column. Three consequences:

cutover reported success for work it never did. No via gate, and its v3 branch returns without setting exitCode, so finally never fires process.exit — it printed "point your application at email_enc" and exited 0 while the v2 rename never ran. drop.ts:106 already gated on via === 'sole' for exactly this reason.
The recorded pairing was discarded. backfill writes the true encryptedColumn to the manifest, so the answer was on disk, but a hint that failed to resolve was dropped and the re-pick reached the guess. Now split by cause: a hint naming a column that is gone is stale and still falls back to convention; a hint naming a column that exists but is not EQL v3 is reported by name.
drop's remedy prescribed the guess. Following --encrypted-column <guess> recorded it as fact, so the next run resolved via: 'hint', walked past the gate, and passed the coverage check vacuously — an unrelated but legitimately backfilled column is non-NULL on every row — then generated a live DROP COLUMN on the plaintext at exit 0.

Tested at the layer that had none: resolve-eql.test.ts covered only explainUnresolved, and encrypt-v3.test.ts stubs resolveColumnLifecycle outright, so neither could see the hint discard. The new tests keep pickEncryptedColumn real and replace only the two I/O boundaries.

Finding 6 — `stash init` scaffolded a file that does not compile

Both templates emitted await Encryption({ schemas: [] }), a hard TS2769 against both overloads. Every stash init left a project failing its first tsc.

Relaxing the constraint is not an option — it is S-6 from #778, and catches a real mistake. stash init has no table names in scope by design. So the scaffold declares a sentinel __stash_placeholder__ table, and loadEncryptConfig exits 1 while that is the only table, naming the file.

The gap that let it ship matters more than the fix. Nothing compiled, parsed or executed the generated output: packages/cli has no typecheck step, the codegen tests only toContain-match fragments, and build-schema.test.ts mocks the generator to '// placeholder'. Both templates are now committed as .generated.ts fixtures compiled by a scoped tsconfig in CI, pinned byte-for-byte to the generator by a unit test. Verified the gate reproduces the original TS2769 when reverted.

Verification

pnpm --filter stash test → 884 passed. typecheck:scaffold clean. Biome: 0 errors.

Changesets: encrypt-lifecycle-mixed-table (stash patch), init-scaffold-compiles (stash patch). skills/stash-cli updated for the new scaffold behaviour.

changeset-bot · 2026-07-25T04:34:37Z

🦋 Changeset detected

Latest commit: 937c6f3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages

Name	Type
stash	Patch
@cipherstash/migrate	Minor
@cipherstash/basic-example	Patch
@cipherstash/e2e	Patch
@cipherstash/stack	Patch
@cipherstash/stack-drizzle	Patch
@cipherstash/stack-supabase	Patch
@cipherstash/prisma-next	Patch
@cipherstash/wizard	Patch
@cipherstash/bench	Patch
@cipherstash/test-kit	Patch
@cipherstash/prisma-next-example	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

coderabbitai · 2026-07-25T04:34:41Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c7b23300-9f9f-4874-b50b-fc44d21c83e8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/cli-lifecycle-and-scaffold

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

freshtonic

Request changes — one blocking regression; the rest of the PR is strong.

The init-scaffold half (finding 6) is excellent: committing the generated output as byte-for-byte-pinned .generated.ts fixtures with tsconfig.scaffold.json and a typecheck:scaffold CI gate is exactly the right systemic fix for "nothing ever compiled the generated output." The cutover via:'sole' guard (finding 7) correctly mirrors drop.

Blocking — pure single-column EQL v2 tables backfilled with this release are now refused by cutover/drop with misleading guidance.

The unresolvedHint branch in resolveColumnLifecycle (resolve-eql.ts) has no candidates.length > 0 guard, and explainUnresolved checks unresolvedHint before the empty-candidates return null. Traced end-to-end against the branch:

backfill.ts records encryptedColumn in .cipherstash/migrations.json unconditionally, v2 included (lines 138-139, 205-213).
resolveColumnLifecycle reads that as hint. For a pure-v2 table, listEncryptedColumns returns [] (classifyEqlDomain recognizes eql_v3_* only), so candidates is empty and hinted is null.
hint && columnExists(hint) is true (the eql_v2_encrypted column exists) → returns { info: null, candidates: [], unresolvedHint }.
explainUnresolved returns the "…legacy eql_v2 column, which this command no longer manages … use a stash release that still supports it" message → cutover/drop exit 1.

Pre-PR this fell through to the v2 rename/config machinery — which is still fully present in cutover.ts/drop.ts in this same release. So the message tells users to downgrade for a lifecycle this build still implements, and the changeset's "Pure-v2 … unaffected" is inaccurate for the backfill-with-this-release path.

The finding-7 protection is aimed at mixed tables (a v2 pair + an unrelated v3 column), which always have candidates. Gating the fail-closed on candidates.length > 0 preserves that protection exactly while restoring the safe pure-v2 fall-through. Please pick one:

Preferred: gate the columnExists/explainUnresolved branch on candidates.length > 0; empty candidates → fall through to the v2 ladder as before.
Or, if the intent is genuinely to drop v2 lifecycle from stash encrypt (this is on the remove-v2 train, so that's plausible): do it coherently — remove the now-dead v2 code in cutover.ts/drop.ts/backfill.ts, fix the message (don't say "use an older release" while shipping the code), and correct the changeset.
Either way, add a test for the untested shape: candidates: [] + a recorded hint. The existing resolveColumnLifecycle tests only cover candidates: [V3_OTHER], which is why this slipped through.

Non-blocking:

The placeholder-client guard lives in loadEncryptConfig, hit only by stash db push/validate. stash encrypt backfill (via loadEncryptionContext/requireTable), cutover, and drop aren't gated, so they still emit the "table not found" error the changeset/skill say is eliminated. Either extend the guard or soften the wording.
resolve-eql.test.ts spreads await importOriginal<@cipherstash/migrate>(), so it fails on a clean checkout until @cipherstash/migrate is built (CI is green only because turbo runs ^build first). packages/cli/AGENTS.md wants the unit config self-contained — prefer a full explicit mock.
packages/cli/package.json re-encodes em-dashes to — in description / the //optionalDependencies comment — unrelated editor noise; revert to keep the diff focused.

The `unresolvedHint` fail-closed added for #772 finding 7 had no `candidates.length > 0` guard, so it fired on pure EQL v2 tables too. `encrypt backfill` records `encryptedColumn` in migrations.json unconditionally, v2 included. On a pure-v2 table `listEncryptedColumns` returns [] (the classifier recognises `eql_v3_*` only), so the hint failed to resolve, `columnExists` found the real `eql_v2_encrypted` column, and `cutover` / `drop` exited 1 — telling users to downgrade for a lifecycle this same build still fully implements in cutover.ts / drop.ts. Gate the fail-closed on a non-empty candidate list. Finding 7's protection is unchanged: the mixed table it targets always has candidates. Order `explainUnresolved`'s empty-candidates fall-through ahead of the hint branch so both agree for direct callers. Also drop the "this release no longer manages that lifecycle" claim from the cutover/drop `via: 'sole'` messages — the build does still implement it; the command simply resolves EQL v3 counterparts only. Tests cover the previously untested shape: candidates [] + a recorded hint. Review follow-ups: - Extend the placeholder-table guard to `encrypt backfill` via loadEncryptionContext; correct the changeset/skill wording to name the commands that actually refuse (cutover/drop never read the client file). - Alias `@cipherstash/migrate` to source in the CLI unit vitest config, so `pnpm --filter stash test` no longer needs a prior workspace build (verified: 888 tests pass with packages/migrate/dist removed). - Revert the unrelated em-dash re-encoding in packages/cli/package.json.

freshtonic

Reviewed in an isolated worktree at 1d144128, running all gates myself and confirming the CI failure against the live CI logs.

Requesting changes — and to be clear, this is for one newly-introduced, unrelated CI blocker, not for the target work. Both findings are fixed excellently, and the prior CHANGES_REQUESTED review is fully resolved.

Blocker

packages/cli/package.json:57 — posthog-node was bumped to ^5.41.0 in commit 1d144128 without regenerating pnpm-lock.yaml (lockfile still pins ^5.40.0). This makes pnpm install --frozen-lockfile fail with ERR_PNPM_OUTDATED_LOCKFILE, so every CI job (Lint, Tests Node 22/24, Bun, E2E, WASM E2E) fails at the install step before any test runs. Confirmed locally and in the live logs (run 30226138197, job 89856521458: posthog-node (lockfile: ^5.40.0, manifest: ^5.41.0)). It also trips the AGENTS.md supply-chain rules (--frozen-lockfile is load-bearing; dependency changes are an audit decision). Fix: revert the bump (it's unrelated to this PR) or run pnpm install and commit the lockfile with a supply-chain justification.
Related nit: even with the lockfile fixed, the posthog-node bump is scope creep for a CLI-lifecycle/scaffold PR and doesn't belong here.

Minor

packages/cli/vitest.config.ts (the @cipherstash/migrate → ../migrate/src/index.ts alias): the commit message claims the unit suite now needs no prior workspace build, but aliasing to migrate's source pulls in migrate's import ... from '@cipherstash/stack', so the suite transitively still requires @cipherstash/stack built. Verified: with @cipherstash/stack unbuilt, resolve-eql.test.ts fails to load and typecheck:scaffold fails (Cannot find module '@cipherstash/stack/v3'); after building it, both pass. It swaps one prebuilt-workspace dependency for a deeper one. Non-blocking (matches CI's ^build ordering), but the "self-contained" claim isn't fully met.

What's done right

Finding 7 (guessed-column lifecycle) is systemically fixed: cutover and drop are gated on via:'sole', the mixed-table unresolvedHint fail-closed is correctly scoped by candidates.length > 0, and finally/exitCode semantics are right. Exceptional "why" documentation on resolve-eql.ts/cutover.ts/drop.ts.
Finding 6 (non-compiling stash init scaffold) is fixed the right way: a __stash_placeholder__ sentinel table, generated output committed as .generated.ts fixtures pinned byte-for-byte by a unit test and compiled by a scoped typecheck:scaffold CI gate — closing the "nothing ever compiled the scaffold" gap.
Tests cover exactly the previously-untested shapes (candidates: [] + recorded hint; sentinel-only client rejected by loadEncryptionContext) and assert behaviour/wording, not implementation.
Verified in the worktree: CLI unit suite 888/888, resolve-eql.test.ts 10/10, scaffold typecheck clean, Biome clean. The prior blocker (pure single-column v2 tables wrongly refused) and all three non-blocking items are resolved.

Once the lockfile blocker is cleared I'd be happy to flip this to approve.

Follow-up to 1d14412, from an adversarial cross-check of that commit. Blocking, and self-inflicted: `git checkout origin/main -- packages/cli/package.json` (the em-dash revert) also imported main's Dependabot bump of posthog-node to ^5.41.0, which this branch's lockfile predates. Every CI job starts with `pnpm install --frozen-lockfile`, so the PR died at step 1 before a test ran. Back to ^5.40.0, matching the lockfile and packages/wizard. resolve-eql's private `columnExists` used a bare `to_regclass($1)`. That form parses and case-folds unquoted identifiers, so on a Prisma-style "User" table the probe reported the column missing, the recorded pairing was treated as stale, and the #772 fail-closed silently did not fire — falling through to the sole/convention rules and resolving the guess it exists to prevent. migrate already documents this exact anti-pattern (REGCLASS_SQL) and its test asserts `not.toMatch(/to_regclass$\$1$/)`. Moved to a shared, case-exact `columnExists` export and deleted the CLI copy; the test double is now case-exact too, so it cannot hide a regression. The placeholder guard read the harvested export map while the `db push` / `db validate` guard it mirrors reads `getEncryptConfig().tables`. Those disagree in both directions on one file: `schemas: [placeholderTable]` minus the `export` keyword fell through to "table not found … Available: (none)" — the error the guard replaces — and a stale placeholder export beside real tables wrongly fired it. Now reads the same source. cutover's `via:'sole'` refusal was nested inside `version === 3`; drop's is top-level. Equivalent today, but cutover's v2 ladder does an irreversible rename plus config promotion, so a restored v2 classification or a v4 family would let cutover rename on a guess drop refuses. Hoisted to match. Text that was false: - the scaffold `stash init` writes into every customer project (and both fixtures) claimed `stash encrypt` commands refuse to run; only backfill does - skills/stash-cli listed the client as loaded by `schema build` and `encrypt *`, and omitted `db push` - skills/stash-cli and skills/stash-encryption still said cutover on a backfilled v3 column exits 0. Since `.cipherstash/` is gitignored, a clone or CI runner hits the new `sole` refusal on a pure-v3 table with an unconventional column name — the changeset's "pure-v3 unaffected" was wrong too - both `sole` messages said "the table's only EQL column"; pickEncryptedColumn excludes the plaintext column first, so it fires with two - the remedy said to drive the v2 lifecycle "directly"; there is no CLI route, so it now says to run the eql_v2 SQL yourself - vitest.config.ts and packages/cli/AGENTS.md claimed the unit suite is self-contained. It is not: @cipherstash/stack is still reached via migrate/src/backfill.ts. The alias removed one of two couplings typecheck:scaffold ran `tsc` directly, bypassing turbo, so it needed a prior build and passed only by accident of step ordering behind steps that read as independently droppable. Now a turbo task with dependsOn ^build. Added scripts/__tests__/cli-vitest-alias.test.mjs — the CLI's alias map sits outside the vitest.shared.ts guard and must, since stackSourceAlias's '@/' would clobber the CLI's own. Runs in test:scripts, ahead of the build-dependent suite. Verified: frozen-lockfile install clean; 891 CLI unit, 41 migrate, 93 scripts, 76 pty e2e; code:check 0 errors; scaffold gate green through turbo with packages/stack/dist absent.

…exposed The reviewer's two findings were both already fixed at bbade2c — this adds the guards that would have caught them, plus two accuracy fixes. `typecheck:scaffold` declares `dependsOn: ["^build"]`, so turbo has to be the thing invoking it. #787 first added it as a bare `pnpm --filter stash run typecheck:scaffold` and CI went green anyway: an earlier step in the same job happened to build the workspace via its own `^build`. Those earlier steps read as independent guards for other packages, so they look freely removable — delete one and the scaffold step fails `TS2307`, which reads as "the scaffold is broken" rather than "you skipped the build". Nothing caught that. scripts/__tests__/workflow-turbo-build-deps.test.mjs derives the protected set from turbo.json (every task declaring `^build`) rather than hardcoding names, and asserts tests.yml routes them through turbo. Mutation-tested against three regressions: reverting the step to the bare form, deleting the step outright, and adding a new bare build-dependent step — each fails with a message naming the step and the fix. Two pre-existing bare `typecheck` invocations (prisma-next, wizard) carry the same latent trap; they are recorded in KNOWN_BARE rather than silently skipped, and a fourth test fails if an entry goes stale so the list gets worked down instead of accumulating. Two accuracy fixes: - vitest.config.ts claimed the suite's residual `@cipherstash/stack` coupling runs through `migrate/src/backfill.ts`. That is only one of two routes: `init/lib/__tests__/introspect.test.ts:1` imports `@cipherstash/stack/eql/v3` directly, never touching migrate. As written, someone could decouple backfill.ts and expect the suite to go standalone. It would not. - cli-vitest-alias.test.mjs called `.endsWith()` on every alias value, so Vite's array form (`[{find, replacement}]`) would throw an opaque TypeError from a helper — reading as "the guard is broken" rather than "the config changed shape". It now fails cleanly, naming the offender, and says to update the guard rather than delete it. No changeset: repo tooling and a comment, no published surface touched.

The `unresolvedHint` fail-closed added for #772 finding 7 had no `candidates.length > 0` guard, so it fired on pure EQL v2 tables too. `encrypt backfill` records `encryptedColumn` in migrations.json unconditionally, v2 included. On a pure-v2 table `listEncryptedColumns` returns [] (the classifier recognises `eql_v3_*` only), so the hint failed to resolve, `columnExists` found the real `eql_v2_encrypted` column, and `cutover` / `drop` exited 1 — telling users to downgrade for a lifecycle this same build still fully implements in cutover.ts / drop.ts. Gate the fail-closed on a non-empty candidate list. Finding 7's protection is unchanged: the mixed table it targets always has candidates. Order `explainUnresolved`'s empty-candidates fall-through ahead of the hint branch so both agree for direct callers. Also drop the "this release no longer manages that lifecycle" claim from the cutover/drop `via: 'sole'` messages — the build does still implement it; the command simply resolves EQL v3 counterparts only. Tests cover the previously untested shape: candidates [] + a recorded hint. Review follow-ups: - Extend the placeholder-table guard to `encrypt backfill` via loadEncryptionContext; correct the changeset/skill wording to name the commands that actually refuse (cutover/drop never read the client file). - Alias `@cipherstash/migrate` to source in the CLI unit vitest config, so `pnpm --filter stash test` no longer needs a prior workspace build (verified: 888 tests pass with packages/migrate/dist removed). - Revert the unrelated em-dash re-encoding in packages/cli/package.json.

Follow-up to 1d14412, from an adversarial cross-check of that commit. Blocking, and self-inflicted: `git checkout origin/main -- packages/cli/package.json` (the em-dash revert) also imported main's Dependabot bump of posthog-node to ^5.41.0, which this branch's lockfile predates. Every CI job starts with `pnpm install --frozen-lockfile`, so the PR died at step 1 before a test ran. Back to ^5.40.0, matching the lockfile and packages/wizard. resolve-eql's private `columnExists` used a bare `to_regclass($1)`. That form parses and case-folds unquoted identifiers, so on a Prisma-style "User" table the probe reported the column missing, the recorded pairing was treated as stale, and the #772 fail-closed silently did not fire — falling through to the sole/convention rules and resolving the guess it exists to prevent. migrate already documents this exact anti-pattern (REGCLASS_SQL) and its test asserts `not.toMatch(/to_regclass$\$1$/)`. Moved to a shared, case-exact `columnExists` export and deleted the CLI copy; the test double is now case-exact too, so it cannot hide a regression. The placeholder guard read the harvested export map while the `db push` / `db validate` guard it mirrors reads `getEncryptConfig().tables`. Those disagree in both directions on one file: `schemas: [placeholderTable]` minus the `export` keyword fell through to "table not found … Available: (none)" — the error the guard replaces — and a stale placeholder export beside real tables wrongly fired it. Now reads the same source. cutover's `via:'sole'` refusal was nested inside `version === 3`; drop's is top-level. Equivalent today, but cutover's v2 ladder does an irreversible rename plus config promotion, so a restored v2 classification or a v4 family would let cutover rename on a guess drop refuses. Hoisted to match. Text that was false: - the scaffold `stash init` writes into every customer project (and both fixtures) claimed `stash encrypt` commands refuse to run; only backfill does - skills/stash-cli listed the client as loaded by `schema build` and `encrypt *`, and omitted `db push` - skills/stash-cli and skills/stash-encryption still said cutover on a backfilled v3 column exits 0. Since `.cipherstash/` is gitignored, a clone or CI runner hits the new `sole` refusal on a pure-v3 table with an unconventional column name — the changeset's "pure-v3 unaffected" was wrong too - both `sole` messages said "the table's only EQL column"; pickEncryptedColumn excludes the plaintext column first, so it fires with two - the remedy said to drive the v2 lifecycle "directly"; there is no CLI route, so it now says to run the eql_v2 SQL yourself - vitest.config.ts and packages/cli/AGENTS.md claimed the unit suite is self-contained. It is not: @cipherstash/stack is still reached via migrate/src/backfill.ts. The alias removed one of two couplings typecheck:scaffold ran `tsc` directly, bypassing turbo, so it needed a prior build and passed only by accident of step ordering behind steps that read as independently droppable. Now a turbo task with dependsOn ^build. Added scripts/__tests__/cli-vitest-alias.test.mjs — the CLI's alias map sits outside the vitest.shared.ts guard and must, since stackSourceAlias's '@/' would clobber the CLI's own. Runs in test:scripts, ahead of the build-dependent suite. Verified: frozen-lockfile install clean; 891 CLI unit, 41 migrate, 93 scripts, 76 pty e2e; code:check 0 errors; scaffold gate green through turbo with packages/stack/dist absent.

…exposed The reviewer's two findings were both already fixed at bbade2c — this adds the guards that would have caught them, plus two accuracy fixes. `typecheck:scaffold` declares `dependsOn: ["^build"]`, so turbo has to be the thing invoking it. #787 first added it as a bare `pnpm --filter stash run typecheck:scaffold` and CI went green anyway: an earlier step in the same job happened to build the workspace via its own `^build`. Those earlier steps read as independent guards for other packages, so they look freely removable — delete one and the scaffold step fails `TS2307`, which reads as "the scaffold is broken" rather than "you skipped the build". Nothing caught that. scripts/__tests__/workflow-turbo-build-deps.test.mjs derives the protected set from turbo.json (every task declaring `^build`) rather than hardcoding names, and asserts tests.yml routes them through turbo. Mutation-tested against three regressions: reverting the step to the bare form, deleting the step outright, and adding a new bare build-dependent step — each fails with a message naming the step and the fix. Two pre-existing bare `typecheck` invocations (prisma-next, wizard) carry the same latent trap; they are recorded in KNOWN_BARE rather than silently skipped, and a fourth test fails if an entry goes stale so the list gets worked down instead of accumulating. turbo.json is JSONC, so reading it needs the string-aware comment stripper #782 wrote for `turbo-skills-inputs.test.mjs` — a naive regex eats the `//` in `"$schema": "https://turbo.build/schema.json"`. Rather than hand-copy 30 lines of parser into an adjacent file, that function moves to `scripts/__tests__/lib/read-jsonc.mjs` and both guards import it. Two accuracy fixes: - vitest.config.ts claimed the suite's residual `@cipherstash/stack` coupling runs through `migrate/src/backfill.ts`. That is only one of two routes: `init/lib/__tests__/introspect.test.ts:1` imports `@cipherstash/stack/eql/v3` directly, never touching migrate. As written, someone could decouple backfill.ts and expect the suite to go standalone. It would not. - cli-vitest-alias.test.mjs called `.endsWith()` on every alias value, so Vite's array form (`[{find, replacement}]`) would throw an opaque TypeError from a helper — reading as "the guard is broken" rather than "the config changed shape". It now fails cleanly, naming the offender, and says to update the guard rather than delete it. No changeset: repo tooling and a comment, no published surface touched.

On a mixed table — a legacy v2 pair the classifier no longer sees, plus one unrelated EQL v3 column — pickEncryptedColumn's sole-EQL-column rule claims the v3 column for the v2 plaintext. Three separate defects turned that guess into wrong outcomes. cutover had no `via` gate at all. Its v3 branch returns from inside try with exitCode untouched, so the finally never fires process.exit: it printed "point your application at email_enc" and exited 0 while the v2 rename never ran. drop.ts:106 already gated on `via === 'sole'` for exactly this reason. The manifest hint was discarded. backfill records the true pairing, so the answer was on disk, but resolveColumnLifecycle dropped a hint that failed to resolve and re-picked without it — reaching the sole rule. Recording the pairing changed nothing. Split the two reasons a hint fails: a column that is GONE is stale and still falls through to convention; a column that EXISTS but is not EQL v3 (the legacy eql_v2_encrypted case) is reported by name. drop's remedy prescribed the guess — `--encrypted-column <guess>`. Recording it makes the next resolution `via: 'hint'`, which walks past the gate, and the coverage check passes vacuously because an unrelated backfilled column is non-NULL on every row. The message was the instruction manual for generating a live DROP COLUMN on the plaintext at exit 0. Tested at the layer that had none: resolve-eql.test.ts covered only explainUnresolved, and encrypt-v3.test.ts stubs resolveColumnLifecycle outright, so neither could see the hint discard. The new tests keep pickEncryptedColumn real and replace only the two I/O boundaries.

Both placeholder templates emitted `await Encryption({ schemas: [] })`. An empty schema set is a hard TS2769 against both overloads — deliberately, per S-6 — so every `stash init` left a project failing its first tsc, in the one file the CLI tells the user not to hand-edit. The old scaffold called EncryptionV3, whose `readonly AnyV3Table[]` bound accepted `[]`; collapsing it into an alias of Encryption tightened that away. Relaxing the constraint is not an option (it exists to catch a real mistake), and `stash init` has no table names in scope by design — it stopped introspecting, and `build-schema.ts` sets `schemas: []` on its own state. So the scaffold declares a sentinel table instead, which keeps the file compiling and keeps the "you haven't declared anything yet" signal: loadEncryptConfig exits 1 when `__stash_placeholder__` is the only table left, naming the file. The gap that let this ship is the more important half. packages/cli has no typecheck step (21 pre-existing errors), utils-codegen*.test.ts only `toContain`-matches fragments, and build-schema.test.ts mocks generatePlaceholderClient to '// placeholder' — so nothing anywhere compiled, parsed or executed the generated output. Both templates are now committed as `.generated.ts` fixtures compiled by a scoped tsconfig in CI, and pinned byte-for-byte to the generator by a unit test. Verified the gate reproduces the original TS2769 when the fixture is reverted. The `.generated.ts` suffix is load-bearing: biome.json already excludes it, so formatting cannot rewrite template output and break the byte comparison.

The `unresolvedHint` fail-closed added for #772 finding 7 had no `candidates.length > 0` guard, so it fired on pure EQL v2 tables too. `encrypt backfill` records `encryptedColumn` in migrations.json unconditionally, v2 included. On a pure-v2 table `listEncryptedColumns` returns [] (the classifier recognises `eql_v3_*` only), so the hint failed to resolve, `columnExists` found the real `eql_v2_encrypted` column, and `cutover` / `drop` exited 1 — telling users to downgrade for a lifecycle this same build still fully implements in cutover.ts / drop.ts. Gate the fail-closed on a non-empty candidate list. Finding 7's protection is unchanged: the mixed table it targets always has candidates. Order `explainUnresolved`'s empty-candidates fall-through ahead of the hint branch so both agree for direct callers. Also drop the "this release no longer manages that lifecycle" claim from the cutover/drop `via: 'sole'` messages — the build does still implement it; the command simply resolves EQL v3 counterparts only. Tests cover the previously untested shape: candidates [] + a recorded hint. Review follow-ups: - Extend the placeholder-table guard to `encrypt backfill` via loadEncryptionContext; correct the changeset/skill wording to name the commands that actually refuse (cutover/drop never read the client file). - Alias `@cipherstash/migrate` to source in the CLI unit vitest config, so `pnpm --filter stash test` no longer needs a prior workspace build (verified: 888 tests pass with packages/migrate/dist removed). - Revert the unrelated em-dash re-encoding in packages/cli/package.json.

Follow-up to 1d14412, from an adversarial cross-check of that commit. resolve-eql's private `columnExists` used a bare `to_regclass($1)`. That form parses and case-folds unquoted identifiers, so on a Prisma-style "User" table the probe reported the column missing, the recorded pairing was treated as stale, and the #772 fail-closed silently did not fire — falling through to the sole/convention rules and resolving the guess it exists to prevent. migrate already documents this exact anti-pattern (REGCLASS_SQL) and its test asserts `not.toMatch(/to_regclass$\$1$/)`. Moved to a shared, case-exact `columnExists` export and deleted the CLI copy; the test double is now case-exact too, so it cannot hide a regression. The placeholder guard read the harvested export map while the `db push` / `db validate` guard it mirrors reads `getEncryptConfig().tables`. Those disagree in both directions on one file: `schemas: [placeholderTable]` minus the `export` keyword fell through to "table not found … Available: (none)" — the error the guard replaces — and a stale placeholder export beside real tables wrongly fired it. Now reads the same source. cutover's `via:'sole'` refusal was nested inside `version === 3`; drop's is top-level. Equivalent today, but cutover's v2 ladder does an irreversible rename plus config promotion, so a restored v2 classification or a v4 family would let cutover rename on a guess drop refuses. Hoisted to match. Text that was false: - the scaffold `stash init` writes into every customer project (and both fixtures) claimed `stash encrypt` commands refuse to run; only backfill does - skills/stash-cli listed the client as loaded by `schema build` and `encrypt *`, and omitted `db push` - skills/stash-cli and skills/stash-encryption still said cutover on a backfilled v3 column exits 0. Since `.cipherstash/` is gitignored, a clone or CI runner hits the new `sole` refusal on a pure-v3 table with an unconventional column name — the changeset's "pure-v3 unaffected" was wrong too - both `sole` messages said "the table's only EQL column"; pickEncryptedColumn excludes the plaintext column first, so it fires with two - the remedy said to drive the v2 lifecycle "directly"; there is no CLI route, so it now says to run the eql_v2 SQL yourself - vitest.config.ts and packages/cli/AGENTS.md claimed the unit suite is self-contained. It is not: @cipherstash/stack is still reached via migrate/src/backfill.ts. The alias removed one of two couplings typecheck:scaffold ran `tsc` directly, bypassing turbo, so it needed a prior build and passed only by accident of step ordering behind steps that read as independently droppable. Now a turbo task with dependsOn ^build. Added scripts/__tests__/cli-vitest-alias.test.mjs — the CLI's alias map sits outside the vitest.shared.ts guard and must, since stackSourceAlias's '@/' would clobber the CLI's own. Runs in test:scripts, ahead of the build-dependent suite. Verified: frozen-lockfile install clean; 891 CLI unit, 41 migrate, 93 scripts, 76 pty e2e; code:check 0 errors; scaffold gate green through turbo with packages/stack/dist absent.

…exposed The reviewer's two findings were both already fixed at bbade2c — this adds the guards that would have caught them, plus two accuracy fixes. `typecheck:scaffold` declares `dependsOn: ["^build"]`, so turbo has to be the thing invoking it. #787 first added it as a bare `pnpm --filter stash run typecheck:scaffold` and CI went green anyway: an earlier step in the same job happened to build the workspace via its own `^build`. Those earlier steps read as independent guards for other packages, so they look freely removable — delete one and the scaffold step fails `TS2307`, which reads as "the scaffold is broken" rather than "you skipped the build". Nothing caught that. scripts/__tests__/workflow-turbo-build-deps.test.mjs derives the protected set from turbo.json (every task declaring `^build`) rather than hardcoding names, and asserts tests.yml routes them through turbo. Mutation-tested against three regressions: reverting the step to the bare form, deleting the step outright, and adding a new bare build-dependent step — each fails with a message naming the step and the fix. Two pre-existing bare `typecheck` invocations (prisma-next, wizard) carry the same latent trap; they are recorded in KNOWN_BARE rather than silently skipped, and a fourth test fails if an entry goes stale so the list gets worked down instead of accumulating. turbo.json is JSONC, so reading it needs the string-aware comment stripper #782 wrote for `turbo-skills-inputs.test.mjs` — a naive regex eats the `//` in `"$schema": "https://turbo.build/schema.json"`. Rather than hand-copy 30 lines of parser into an adjacent file, that function moves to `scripts/__tests__/lib/read-jsonc.mjs` and both guards import it. Two accuracy fixes: - vitest.config.ts claimed the suite's residual `@cipherstash/stack` coupling runs through `migrate/src/backfill.ts`. That is only one of two routes: `init/lib/__tests__/introspect.test.ts:1` imports `@cipherstash/stack/eql/v3` directly, never touching migrate. As written, someone could decouple backfill.ts and expect the suite to go standalone. It would not. - cli-vitest-alias.test.mjs called `.endsWith()` on every alias value, so Vite's array form (`[{find, replacement}]`) would throw an opaque TypeError from a helper — reading as "the guard is broken" rather than "the config changed shape". It now fails cleanly, naming the offender, and says to update the guard rather than delete it. No changeset: repo tooling and a comment, no published surface touched.

Follow-up to the #787 review. Five gaps, none raised by the reviewer, all found by auditing what the PR's own tests actually pin. A guard clause that no test held. `context-placeholder.test.ts` named a case it did not construct: its fixture returned `getEncryptConfig: () => ({})`, so `configuredTables` was empty and the guard short-circuited on the arity check before the sentinel-name comparison ever ran. Deleting `configuredTables.length === 1` left all 891 tests green — measured, not argued. The fixture now declares the sentinel FIRST alongside a real table, because the guard indexes `[0]`, and both tests spy `process.exit` so a regression fails as an assertion instead of killing the worker. One guard, not two. The same refusal was hand-copied into `loadEncryptConfig` and `loadEncryptionContext`, sharing only the constant — and it had already drifted: on a client whose `getEncryptConfig()` returns nothing, `db push` named the cause while `encrypt backfill` fell through to `Table "..." was not found`, the symptom-not-cause message the guard exists to replace. Both now call `requireUsableEncryptConfig`, pinned by a parity test asserting the two seams emit byte-identical text. The resolver/command seam, tested. `encrypt-v3.test.ts` stubs `resolveColumnLifecycle` and hand-writes its return value; `resolve-eql.test.ts` proves the pure-v2 shape produces that value. Nothing ran the real producer into the real consumer. The new composition test mocks no resolution at all — only pg, fs, prompts and the effectful migrate helpers — so the v2 verdict is derived from a real manifest hint and a real catalog read. Mutation-checked: the two #787 defences are independent, so removing either alone is masked by the other; removing both fails this test and nothing else. Its sibling types are now imported rather than re-declared, so the shape cannot drift silently. The workflow guard, applied to all workflows. It scanned only `tests.yml` while enforcing a rule its own docblock generalises. Widened to all 13, which surfaced six real bare invocations of `^build`-dependent tasks — five `test:integration`, plus a `test:e2e` that `rootScriptDelegatesToTurbo` was wrongly exempting (a `--filter` invocation runs the PACKAGE's script, so the root script's delegation says nothing about it). All six routed through turbo with `--env-mode=loose`: turbo defaults to strict and would otherwise withhold the step env these suites need, which `passThroughEnv` could only fix by enumerating ten variables across workflows this change cannot execute. Also corrects `AGENTS.md`'s path to `introspect.test.ts` and states the `stackSourceAlias` collision symmetrically — spreading it either way breaks one package's `'@/'`. Its "fails 10 files" figure was re-measured by deleting `packages/stack/dist`: exactly 10. `stash` unit suite 896/896, scripts 112/112, typecheck:scaffold clean, Biome 0 errors.

freshtonic

Approve — all prior blockers and minors resolved

Re-reviewed at HEAD 937c6f36 (rebased on remove-v2). Both of my earlier CHANGES_REQUESTED reviews are fully addressed, the two target findings (6, 7) remain excellently fixed, and all 13 CI checks are green.

Blockers — both cleared

Review 1 (pure single-column v2 tables refused with downgrade-guidance). Fixed correctly: resolve-eql.ts now gates the unresolvedHint/columnExists branch on candidates.length > 0 (line ~106), and explainUnresolved return nulls on empty candidates before touching the hint. Traced it: a pure-v2 table (listEncryptedColumns → []) no longer trips the fail-closed and falls back to the v2 ladder as before, while the mixed-table protection (which always has candidates) is preserved intact. The previously-untested candidates: [] + recorded hint shape now has coverage.
Review 2 (posthog-node ^5.41.0 vs lockfile ^5.40.0 → ERR_PNPM_OUTDATED_LOCKFILE). Fixed: bump reverted to ^5.40.0; both lockfile importer entries read specifier: ^5.40.0 / version: 5.40.0. pnpm install --frozen-lockfile is satisfied — Lint, Tests (Node 22/24, Bun), E2E, WASM E2E, and all four integration jobs pass.

Minors — resolved

The placeholder-client guard is now shared as requireUsableEncryptConfig and invoked from loadEncryptionContext, so stash encrypt backfill/cutover/drop get the same refusal that db push/validate do — naming the cause (__stash_placeholder__) instead of the requireTable "table not found" symptom. That closes my first review's NB1.
vitest.config.ts no longer over-claims "self-contained": the comment now documents precisely which coupling the migrate-source alias removes and which two routes (migrate's @cipherstash/stack import; introspect.test.ts's direct @cipherstash/stack/eql/v3 import) still require a packages/stack build, and why vitest.shared.ts's stackSourceAlias can't be spread here. Honest and accurate.
The stray em-dash package.json noise is gone.

Core fixes verified intact

Finding 7: cutover.ts and drop.ts both gate destructive work on info?.via === 'sole'; cutover's exitCode/finally semantics are correct (no more success-at-exit-0 for work it never did). The new packages/migrate/src/version.ts extracts classifyEqlDomain as the single domain-based (never name-based) classification source — addressing the root cause, not just the symptom. Exceptional "why" comments throughout.
Finding 6: the __stash_placeholder__ sentinel + .generated.ts fixtures pinned byte-for-byte to the generator and compiled by the typecheck:scaffold CI gate still stand.

Nice work turning both reviews around cleanly. LGTM.