feat(cli): onboarding fixes batch for launch (CIP-2982…CIP-3001)#349
feat(cli): onboarding fixes batch for launch (CIP-2982…CIP-3001)#349calvinbrewer merged 1 commit intomainfrom
Conversation
Closes or addresses 16 Linear tickets from the end-to-end onboarding test (CIP-2982 … CIP-3001). Scope is `packages/cli` and `packages/stack`. Highlights - `db install` absorbs the old `db setup` scaffolding and is now the single canonical command. `db setup` is removed. - `db install` auto-detects Supabase (from DATABASE_URL host) and Drizzle (from drizzle.config.* or package.json deps), so the `--supabase` and `--drizzle` flags default on when relevant. - Non-superuser roles now auto-fall back to the no-operator-family (OPE) install variant instead of aborting, which unblocks Supabase/Neon/RDS onboarding. - `--supabase` now threads through the `--drizzle` migration path (it was silently dropped before). - Drizzle's `encryptedType` emits `"public"."eql_v2_encrypted"` so generated migrations no longer contain `"undefined"."eql_v2_encrypted"`. - Drizzle migrations that use `ALTER COLUMN ... SET DATA TYPE` (which fails because there's no implicit cast) are automatically rewritten to ADD + UPDATE placeholder + DROP + RENAME — safe on empty tables, with a comment reminding users to backfill via encryptModel for non-empty tables. Rewrite runs in both the CLI drizzle path and the wizard's post-agent step. - Wizard now: prompts to install Claude skills into `./.claude/skills/`, writes a timestamped markdown log to `.cipherstash/wizard-log.md`, and scans src/app, src/lib, app/, lib/ for encrypt/decrypt call sites (report-only, no file mutations). - `init` streams npm/pnpm/yarn output during package install instead of hiding it behind a silent spinner. - CLI loads `.env.local` before `.env` (Next.js precedence). - `db install` prints a "what next" block pointing at the wizard and direct SDK usage. - Forge branding removed from user-facing CLI strings. - Experimental `env` command scaffold gated behind `STASH_EXPERIMENTAL_ENV_CMD=1`. The CTS mint endpoint is TBD; command returns a clear "not ready" message until then. Known-issue markers - CIP-2996 (auth login profile dir mismatch) is upstream in `@cipherstash/auth`. TODO marker added in login.ts; bump the catalog pin once a fixed release ships. Tests - 20 new tests: `detect.test.ts` (detectSupabase/detectDrizzle) and `rewrite-migrations.test.ts` (ALTER COLUMN rewrite cases). - Full CLI suite: 124 pass / 5 skipped. - Stack tests unchanged: `drizzle/index.ts` edit verified via existing drizzle-operators suite. Auth-gated integration tests fail the same way on `main` without a CTS token. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
📝 WalkthroughWalkthroughThe pull request refactors the CLI database setup workflow by removing the standalone Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes This diff involves substantial refactoring across multiple functional areas (CLI wiring, command restructuring, wizard flow), introduces several new modules with interdependencies, and modifies public APIs (installer options, Drizzle types). While many individual changes are straightforward, the heterogeneity of modifications across the setup/install flow, detection logic, migration rewriting, and wizard integration—combined with 10+ file modifications and 3 new modules—requires careful verification of control flow interactions, provider flag threading, and backward-compatibility implications. Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/cli/src/bin/stash.ts (1)
76-92:⚠️ Potential issue | 🟡 Minor
env --writeis wired but undocumented in HELP.Line 262 reads
flags.write, but the HELP text for theenvcommand (line 76) and the flags block (lines 82–92) don't mention--write. Even though the command is experimental and currently returns "not ready", users exploring--helpwon't discover the flag. Consider adding a one-liner under the env entry.Suggested help tweak
- env (experimental) Print production env vars for deployment + env (experimental) Print production env vars for deployment + Flags: --write (persist output to a file)Also applies to: 261-263
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/bin/stash.ts` around lines 76 - 92, The help text for the "env" command is missing documentation for the wired-but-undocumented --write flag referenced by flags.write; update the "env" entry in the printed help and the flags block to include a one-line description for --write (e.g., "--write (experimental) Write production env vars to .env or stdout"), making clear it's experimental and currently "not ready", and ensure the help string near the env command and the flags listing match the implementation that reads flags.write.packages/cli/src/commands/db/install.ts (1)
84-113:⚠️ Potential issue | 🟡 MinorAuto-fallback triggers on
!isSuperuserregardless of whether permissions are already sufficient.The new branch at Line 90-94 only inspects
permissions.isSuperuser— it doesn't also require!permissions.ok. That means any non-superuser role (including one that has been explicitly grantedCREATE SCHEMA/CREATE TYPE/CREATE EXTENSIONand would have succeeded under the full install prior to this PR) is now silently downgraded to the no-operator-family variant, producing a different on-disk schema than before.This is probably fine for the stated Supabase/Neon/RDS onboarding case, but it's a subtle behavior change for self-hosted Postgres setups with pre-granted roles, and they only get a single-line spinner message announcing the switch. Two options worth considering:
- Gate the fallback on
!permissions.isSuperuser && !permissions.okso users who already have the right grants keep the full install.- Keep the current behavior but log a clearer note (e.g. "installing OPE variant; pass
--no-exclude-operator-familyto force the full install") so the choice is discoverable.Either way, a follow-up on the downgrade decision in the output would help future debugging since the permissions diagnostic block at Line 101-108 is skipped entirely in this branch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/db/install.ts` around lines 84 - 113, The auto-fallback to the no-operator-family install currently triggers on !permissions.isSuperuser alone; change the condition so it only falls back when the role is not a superuser AND the permissions are insufficient (i.e., require !permissions.isSuperuser && !permissions.ok in the if that sets excludeOperatorFamily), and update the s.stop message to make the downgrade explicit and actionable (e.g., mention how to force the full install or pass --no-exclude-operator-family); reference the existing symbols excludeOperatorFamily, permissions.isSuperuser, permissions.ok, resolved.supabase, options.excludeOperatorFamily, s.stop, and p.log/p.note so the fix is made in the same block.
🧹 Nitpick comments (9)
packages/cli/src/commands/init/steps/install-forge.ts (1)
60-63: Minor:err.messageduplicates noise now that stderr streams to the terminal.With
stdio: 'inherit', the package manager's stderr is already shown to the user.execSync's thrownerr.messagefor a non-zero exit is typically a genericCommand failed: <cmd>string (stderr is not captured), so logging it viap.log.error(message)mostly restates the command the user just saw on line 54. Consider dropping the secondp.log.error(message)(or gating it to only print when the message contains information beyondCommand failed:), keeping the${packageName} installation failedline and the manual-install note.♻️ Proposed tweak
- } catch (err) { - const message = err instanceof Error ? err.message : String(err) - p.log.error(`${packageName} installation failed`) - p.log.error(message) + } catch (err) { + p.log.error(`${packageName} installation failed`) + if (err instanceof Error && !/^Command failed:/.test(err.message)) { + p.log.error(err.message) + } p.note(`You can install it manually:\n ${cmd}`, 'Manual Installation') return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/init/steps/install-forge.ts` around lines 60 - 63, In the catch block in install-forge.ts (the catch handling execSync with stdio: 'inherit'), remove the redundant p.log.error(message) that reprints execSync's generic "Command failed:" text; instead keep the existing p.log.error(`${packageName} installation failed`) and the manual-install guidance, or if you prefer to preserve extra info only log message when it contains more than the generic prefix (e.g., guard by checking that message does not startWith or include "Command failed:"). Ensure you update the catch that references packageName and p.log.error accordingly.packages/cli/src/commands/env/index.ts (1)
33-39:process.exit(1)insidetry-less flow is fine, but considerp.cancel+ return for consistency.Other cancellation paths in this file use
p.cancel(...) ; return(line 51–52). Usingprocess.exit(1)here skips any future cleanup (e.g.,p.outro, file handles) and makes the function harder to test. Not a blocker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/env/index.ts` around lines 33 - 39, The code exits the process directly when fetchProdCredentials() returns falsy; replace the process.exit(1) path with the same cancellation pattern used elsewhere by calling p.cancel(...) with a clear error message (e.g., same string passed to p.log.error) and then return to allow cleanup and keep behavior consistent with other cancellation paths around fetchProdCredentials/creds and p.log.error.packages/cli/src/commands/db/config-scaffold.ts (2)
52-65: RedundantdefaultValue/initialValuepairing.In
@clack/prompts,initialValueprefills the editor (so the user just presses Enter to accept), whiledefaultValueis only applied when the input is empty on submit. Setting both to the same string is redundant —initialValuealone covers the ergonomic path. Not a bug; just clutter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/db/config-scaffold.ts` around lines 52 - 65, The p.text prompt for clientPath redundantly passes both defaultValue and initialValue; remove the unnecessary defaultValue and keep initialValue (or vice versa) to avoid duplicate configuration. Edit the p.text call that creates clientPath in config-scaffold.ts (the clientPath constant / p.text({...}) block) and delete the defaultValue property, leaving initialValue: detected ?? './src/encryption/index.ts' (and retain message, placeholder, validate) so the prompt behavior remains identical but the config is not duplicated.
71-79: Generated config uses non-null assertion onDATABASE_URL.
process.env.DATABASE_URL!silences TypeScript but does nothing at runtime — if the user hasn't setDATABASE_URL, downstream code receivesundefinedand will fail with a less helpful error than an explicit throw. Consider emitting a runtime check in the generated config.♻️ Proposed fix
- return `import { defineConfig } from '@cipherstash/cli' + return `import { defineConfig } from '@cipherstash/cli' + +if (!process.env.DATABASE_URL) { + throw new Error('DATABASE_URL is not set. Add it to your .env file.') +} export default defineConfig({ - databaseUrl: process.env.DATABASE_URL!, + databaseUrl: process.env.DATABASE_URL, client: '${clientPath}', }) `🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/db/config-scaffold.ts` around lines 71 - 79, The generated config in generateConfig currently uses process.env.DATABASE_URL! which only silences TypeScript but doesn't guard at runtime; change the template to perform a runtime check (e.g., if (!process.env.DATABASE_URL) throw new Error(...)) before passing the value to defineConfig so the generated file throws a clear, descriptive error when DATABASE_URL is unset while preserving the client: '${clientPath}' and export default defineConfig(...) structure.packages/cli/src/commands/wizard/lib/wire-call-sites.ts (1)
60-72: Usepath.joininstead of manual string concatenation.
absPath = \${cwd.replace(//$/, '')}/${relPath}`hard-codes forward slashes and won't handle Windows paths cleanly. Sinceglobalready yields paths relative tocwd, you can just usepath.join(cwd, relPath)(or skip the absolute round-trip entirely and read viajoin(cwd, relPath)then reportrelPath` directly).♻️ Proposed fix
-import { readFile } from 'node:fs/promises' -import { glob } from 'node:fs/promises' -import { relative } from 'node:path' +import { glob, readFile } from 'node:fs/promises' +import { join } from 'node:path' @@ for (const relPath of files) { - const absPath = `${cwd.replace(/\/$/, '')}/${relPath}` + const absPath = join(cwd, relPath) let text: string try { text = await readFile(absPath, 'utf-8') } catch { continue } const matches = findMatches(text, tables, integration) for (const m of matches) { - results.push({ ...m, file: relative(cwd, absPath) }) + results.push({ ...m, file: relPath }) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/wizard/lib/wire-call-sites.ts` around lines 60 - 72, The code builds absPath with string concatenation which breaks on Windows; replace the manual path assembly for each relPath by using path.join(cwd, relPath) (or skip making an absolute path entirely and call readFile(path.join(cwd, relPath), 'utf-8') or readFile(relPath, 'utf-8') if glob already yields correct relative paths) in the loop where absPath is defined; update the usages in the try/catch that call readFile(absPath, ...) and the push to results (you can keep reporting file: relPath or continue to use relative(cwd, absPath) if you keep the absPath variable) and ensure path is imported where wire-call-sites.ts defines findMatches, readFile, results, cwd, files, and relative.packages/cli/src/commands/db/detect.ts (1)
22-26: Nit:.pooler.supabase.comis already covered by.supabase.com.Any hostname ending in
.pooler.supabase.comalso ends in.supabase.com, so the third branch is dead. Not a bug — just redundant. Keep it if you want the call site to read as explicit intent; otherwise drop it.Optional simplification
- return ( - host.endsWith('.supabase.co') || - host.endsWith('.supabase.com') || - host.endsWith('.pooler.supabase.com') - ) + return ( + host.endsWith('.supabase.co') || + host.endsWith('.supabase.com') + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/db/detect.ts` around lines 22 - 26, The conditional that returns whether a host is Supabase is redundant because host.endsWith('.pooler.supabase.com') is already matched by host.endsWith('.supabase.com'); remove the third branch (the endsWith('.pooler.supabase.com') check) from the return expression so it becomes just the two checks for '.supabase.co' and '.supabase.com', leaving the rest of the function and the host variable usage unchanged.packages/cli/src/bin/stash.ts (1)
3-10: Minor: "Next.js precedence" isn't exactly Next.js —.env.developmentis loaded unconditionally.dotenv's non-overwrite default makes the
.env.local→.envwinner ordering correct, so runtime behavior is fine. However, Next.js only loads.env.development[.local]whenNODE_ENV === 'development'(and.env.production[.local]in production). Here those files load regardless ofNODE_ENV, so a user withNODE_ENV=productionwill still pick up.env.developmentvalues. Either gate the dev-specific files onNODE_ENVor soften the comment to "Next.js-like precedence" to avoid surprising users running the CLI in prod contexts.Suggested NODE_ENV-aware load
-// Load env files in Next.js precedence order. dotenv's default behavior is to -// not overwrite vars that are already set, so loading .env.local first means -// its values win over .env for the same keys. Users can still set anything in -// the real environment to override both. -config({ path: '.env.local' }) -config({ path: '.env.development.local' }) -config({ path: '.env.development' }) -config({ path: '.env' }) +// Load env files in Next.js precedence order. dotenv's default behavior is to +// not overwrite vars that are already set, so loading .env.local first means +// its values win over .env for the same keys. Users can still set anything in +// the real environment to override both. +const nodeEnv = process.env.NODE_ENV ?? 'development' +config({ path: '.env.local' }) +config({ path: `.env.${nodeEnv}.local` }) +config({ path: `.env.${nodeEnv}` }) +config({ path: '.env' })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/bin/stash.ts` around lines 3 - 10, The comment and loading are misleading because .env.development(.local) are always loaded; either gate those two config calls on NODE_ENV === 'development' or change the comment to "Next.js-like precedence" to avoid implying exact Next.js behavior. Locate the four config(...) calls (config({ path: '.env.local' }), config({ path: '.env.development.local' }), config({ path: '.env.development' }), config({ path: '.env' })) and implement one of two fixes: wrap the .env.development.local and .env.development config(...) calls in a conditional check for process.env.NODE_ENV === 'development', or update the preceding comment text to "Next.js-like precedence" so it does not claim exact Next.js behavior.packages/stack/src/drizzle/index.ts (1)
176-189: Back-compat detection looks correct — no current inconsistency with the parallel impl, but note a potential future maintenance issue.This file now recognizes both the bare
eql_v2_encryptedand the new fully-qualified"public"."eql_v2_encrypted"forms for back-compat. The parallel implementation inpackages/drizzle/src/pg/index.tscurrently emits only the bare form in itsdataType()method and detects only the bare form ingetEncryptedColumnConfig().However, if
packages/drizzleis updated to emit the schema-qualified form to match this file's new behavior, its detection path will silently miss encrypted columns without the same back-compat branch applied here. Consider synchronizing both implementations or document this dependency for future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack/src/drizzle/index.ts` around lines 176 - 189, The parallel implementation's detection must match this file's handling of both bare and schema-qualified encrypted type names: update the other module's dataType() and getEncryptedColumnConfig() logic to also accept the fully-qualified form (e.g., "\"public\".\"eql_v2_encrypted\"") in addition to the bare "eql_v2_encrypted", or add a clear comment documenting the dependency; specifically, extend its isEncryptedTypeString-like check (or the functions that currently only match 'eql_v2_encrypted') to also return true when given the quoted schema-qualified string, ensuring the functions named dataType and getEncryptedColumnConfig (or their equivalents) will detect encrypted columns consistently with isEncryptedTypeString/isEncrypted here.packages/cli/src/commands/db/install.ts (1)
172-178:detectDrizzle(process.cwd())hard-codes the working directory.
resolveProviderOptionstakes nocwdparameter and callsprocess.cwd()directly. Elsewhere in the CLI (e.g. the wizard)cwdis threaded through as an explicit option so invocations and tests can override it. Consider passingcwdin alongsidedatabaseUrlfor consistency and easier unit testing of the resolver.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/db/install.ts` around lines 172 - 178, The code hard-codes process.cwd() into detectDrizzle; change the call site so it uses a passed-in cwd (e.g. options.cwd or the same cwd used for databaseUrl) and thread that cwd into resolveProviderOptions instead of letting those functions call process.cwd() directly; update detectDrizzle usage in the db install flow (the block using options.drizzle and detectDrizzle) to call detectDrizzle(cwd) and adjust resolveProviderOptions signature/usage to accept a cwd parameter so tests and alternative invocations can override the working directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/db/install.ts`:
- Around line 22-32: Update the JSDoc for the excludeOperatorFamily option to
stop referencing detectSupabase and instead state that undefined triggers an
auto-fallback based on the superuser check in installCommand
(permissions.isSuperuser); mention that an explicit true/false from the user is
preserved and skips the superuser-driven detection so readers look at
installCommand and permissions.isSuperuser for the fallback logic.
In `@packages/cli/src/commands/env/index.ts`:
- Around line 43-60: The file write currently saves production secrets with
default permissions via writeFileSync(target, block, 'utf-8'); change this so
the created .env.production.local is restricted (owner read/write only). Update
the write path around the writeFileSync call for target (or immediately after)
to set file mode to 0o600 (either by passing an options object with mode: 0o600
to writeFileSync or calling fs.chmodSync(target, 0o600)); ensure the success log
(p.log.success) still runs only after the permission change succeeds.
In `@packages/cli/src/commands/wizard/lib/changelog.ts`:
- Around line 35-57: The flush() method currently appends the entire
this.entries every call, causing duplicate log content; modify the
implementation to track which entries have already been written (e.g., add a
flushedIndex or clear/advance this.entries) and change render to accept an
optional fromIndex parameter (render(fromIndex = 0)) so flush() only renders and
writes entries[fromIndex..end]; ensure the first write still emits the "## Run"
header but subsequent writes skip that header, update flush() to call
render(flushedIndex) and advance flushedIndex after successful write so flush()
becomes idempotent.
In `@packages/cli/src/commands/wizard/lib/install-skills.ts`:
- Around line 42-62: Detect existing skill directories before copying by
checking each destination path (dest = join(destRoot, name)) with
existsSync/statSync to build two lists: existing (would be overwritten) and new
(would be added); update the confirmation prompt (the p.confirm call that
references available) to mention counts/names of existing vs new (e.g.,
"Overwrite N existing skill(s) and install M new one(s)?") and if the user
declines, return []; then during the copy loop (where cpSync is called) either
skip copying for destinations in the existing list unless the user opted into
overwriting, or honor an explicit overwrite choice and keep cpSync({ recursive:
true, force: true }) only when overwriting; ensure p.log.warn is kept for
per-skill failures.
In `@packages/cli/src/commands/wizard/lib/post-agent.ts`:
- Around line 108-133: rewriteEncryptedMigrations currently returns as soon as
it finds the first existing DRIZZLE_OUT_DIRS candidate, which can skip the real
migrations dir; change the control flow in rewriteEncryptedMigrations so it only
returns (exits) when rewriteEncryptedAlterColumns(abs) actually rewrote files
(rewritten.length > 0), and otherwise continues the loop to try the next
candidate; likewise, on catch do not return immediately—log the error and
continue scanning other DRIZZLE_OUT_DIRS; keep references to DRIZZLE_OUT_DIRS,
rewriteEncryptedAlterColumns, existsSync and the p.log calls when making the
change.
In `@packages/cli/src/commands/wizard/lib/wire-call-sites.ts`:
- Around line 106-122: The loop over lines can push multiple entries when a
single line matches multiple regexes (e.g., drizzleInsert and drizzleSelect);
update the logic in the lines.forEach block to pick a single kind per line
(prefer insert, then update, then select) or dedupe pushes by tracking seen
keys. Concretely, replace the multiple if checks that call out.push with a
single decision per line (e.g., compute let kind = drizzleInsert.test(line) ?
'insert' : drizzleUpdate.test(line) ? 'update' : drizzleSelect.test(line) ?
'select' : undefined and push only if kind) or maintain a Set of `${i}-${kind}`
to avoid duplicate out.push calls for the same (line, kind); apply the same
approach for the supabase branch using
supabaseFromInsert/supabaseFromUpdate/supabaseFromSelect.
---
Outside diff comments:
In `@packages/cli/src/bin/stash.ts`:
- Around line 76-92: The help text for the "env" command is missing
documentation for the wired-but-undocumented --write flag referenced by
flags.write; update the "env" entry in the printed help and the flags block to
include a one-line description for --write (e.g., "--write (experimental)
Write production env vars to .env or stdout"), making clear it's experimental
and currently "not ready", and ensure the help string near the env command and
the flags listing match the implementation that reads flags.write.
In `@packages/cli/src/commands/db/install.ts`:
- Around line 84-113: The auto-fallback to the no-operator-family install
currently triggers on !permissions.isSuperuser alone; change the condition so it
only falls back when the role is not a superuser AND the permissions are
insufficient (i.e., require !permissions.isSuperuser && !permissions.ok in the
if that sets excludeOperatorFamily), and update the s.stop message to make the
downgrade explicit and actionable (e.g., mention how to force the full install
or pass --no-exclude-operator-family); reference the existing symbols
excludeOperatorFamily, permissions.isSuperuser, permissions.ok,
resolved.supabase, options.excludeOperatorFamily, s.stop, and p.log/p.note so
the fix is made in the same block.
---
Nitpick comments:
In `@packages/cli/src/bin/stash.ts`:
- Around line 3-10: The comment and loading are misleading because
.env.development(.local) are always loaded; either gate those two config calls
on NODE_ENV === 'development' or change the comment to "Next.js-like precedence"
to avoid implying exact Next.js behavior. Locate the four config(...) calls
(config({ path: '.env.local' }), config({ path: '.env.development.local' }),
config({ path: '.env.development' }), config({ path: '.env' })) and implement
one of two fixes: wrap the .env.development.local and .env.development
config(...) calls in a conditional check for process.env.NODE_ENV ===
'development', or update the preceding comment text to "Next.js-like precedence"
so it does not claim exact Next.js behavior.
In `@packages/cli/src/commands/db/config-scaffold.ts`:
- Around line 52-65: The p.text prompt for clientPath redundantly passes both
defaultValue and initialValue; remove the unnecessary defaultValue and keep
initialValue (or vice versa) to avoid duplicate configuration. Edit the p.text
call that creates clientPath in config-scaffold.ts (the clientPath constant /
p.text({...}) block) and delete the defaultValue property, leaving initialValue:
detected ?? './src/encryption/index.ts' (and retain message, placeholder,
validate) so the prompt behavior remains identical but the config is not
duplicated.
- Around line 71-79: The generated config in generateConfig currently uses
process.env.DATABASE_URL! which only silences TypeScript but doesn't guard at
runtime; change the template to perform a runtime check (e.g., if
(!process.env.DATABASE_URL) throw new Error(...)) before passing the value to
defineConfig so the generated file throws a clear, descriptive error when
DATABASE_URL is unset while preserving the client: '${clientPath}' and export
default defineConfig(...) structure.
In `@packages/cli/src/commands/db/detect.ts`:
- Around line 22-26: The conditional that returns whether a host is Supabase is
redundant because host.endsWith('.pooler.supabase.com') is already matched by
host.endsWith('.supabase.com'); remove the third branch (the
endsWith('.pooler.supabase.com') check) from the return expression so it becomes
just the two checks for '.supabase.co' and '.supabase.com', leaving the rest of
the function and the host variable usage unchanged.
In `@packages/cli/src/commands/db/install.ts`:
- Around line 172-178: The code hard-codes process.cwd() into detectDrizzle;
change the call site so it uses a passed-in cwd (e.g. options.cwd or the same
cwd used for databaseUrl) and thread that cwd into resolveProviderOptions
instead of letting those functions call process.cwd() directly; update
detectDrizzle usage in the db install flow (the block using options.drizzle and
detectDrizzle) to call detectDrizzle(cwd) and adjust resolveProviderOptions
signature/usage to accept a cwd parameter so tests and alternative invocations
can override the working directory.
In `@packages/cli/src/commands/env/index.ts`:
- Around line 33-39: The code exits the process directly when
fetchProdCredentials() returns falsy; replace the process.exit(1) path with the
same cancellation pattern used elsewhere by calling p.cancel(...) with a clear
error message (e.g., same string passed to p.log.error) and then return to allow
cleanup and keep behavior consistent with other cancellation paths around
fetchProdCredentials/creds and p.log.error.
In `@packages/cli/src/commands/init/steps/install-forge.ts`:
- Around line 60-63: In the catch block in install-forge.ts (the catch handling
execSync with stdio: 'inherit'), remove the redundant p.log.error(message) that
reprints execSync's generic "Command failed:" text; instead keep the existing
p.log.error(`${packageName} installation failed`) and the manual-install
guidance, or if you prefer to preserve extra info only log message when it
contains more than the generic prefix (e.g., guard by checking that message does
not startWith or include "Command failed:"). Ensure you update the catch that
references packageName and p.log.error accordingly.
In `@packages/cli/src/commands/wizard/lib/wire-call-sites.ts`:
- Around line 60-72: The code builds absPath with string concatenation which
breaks on Windows; replace the manual path assembly for each relPath by using
path.join(cwd, relPath) (or skip making an absolute path entirely and call
readFile(path.join(cwd, relPath), 'utf-8') or readFile(relPath, 'utf-8') if glob
already yields correct relative paths) in the loop where absPath is defined;
update the usages in the try/catch that call readFile(absPath, ...) and the push
to results (you can keep reporting file: relPath or continue to use
relative(cwd, absPath) if you keep the absPath variable) and ensure path is
imported where wire-call-sites.ts defines findMatches, readFile, results, cwd,
files, and relative.
In `@packages/stack/src/drizzle/index.ts`:
- Around line 176-189: The parallel implementation's detection must match this
file's handling of both bare and schema-qualified encrypted type names: update
the other module's dataType() and getEncryptedColumnConfig() logic to also
accept the fully-qualified form (e.g., "\"public\".\"eql_v2_encrypted\"") in
addition to the bare "eql_v2_encrypted", or add a clear comment documenting the
dependency; specifically, extend its isEncryptedTypeString-like check (or the
functions that currently only match 'eql_v2_encrypted') to also return true when
given the quoted schema-qualified string, ensuring the functions named dataType
and getEncryptedColumnConfig (or their equivalents) will detect encrypted
columns consistently with isEncryptedTypeString/isEncrypted here.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b56241f4-ba82-4e59-a423-0c76e0004353
📒 Files selected for processing (25)
packages/cli/README.mdpackages/cli/src/__tests__/detect.test.tspackages/cli/src/__tests__/rewrite-migrations.test.tspackages/cli/src/bin/stash.tspackages/cli/src/commands/auth/login.tspackages/cli/src/commands/db/config-scaffold.tspackages/cli/src/commands/db/detect.tspackages/cli/src/commands/db/install.tspackages/cli/src/commands/db/rewrite-migrations.tspackages/cli/src/commands/db/setup.tspackages/cli/src/commands/env/index.tspackages/cli/src/commands/index.tspackages/cli/src/commands/init/providers/base.tspackages/cli/src/commands/init/providers/drizzle.tspackages/cli/src/commands/init/providers/supabase.tspackages/cli/src/commands/init/steps/install-forge.tspackages/cli/src/commands/wizard/lib/changelog.tspackages/cli/src/commands/wizard/lib/install-skills.tspackages/cli/src/commands/wizard/lib/post-agent.tspackages/cli/src/commands/wizard/lib/prerequisites.tspackages/cli/src/commands/wizard/lib/wire-call-sites.tspackages/cli/src/commands/wizard/run.tspackages/cli/src/installer/index.tspackages/cli/tsup.config.tspackages/stack/src/drizzle/index.ts
💤 Files with no reviewable changes (1)
- packages/cli/src/commands/db/setup.ts
| /** | ||
| * `undefined` means "auto-detect" (via {@link detectSupabase}). An explicit | ||
| * `true`/`false` from the user is preserved and skips detection. | ||
| */ | ||
| excludeOperatorFamily?: boolean | ||
| supabase?: boolean | ||
| drizzle?: boolean | ||
| latest?: boolean | ||
| name?: string | ||
| out?: string | ||
| }) { | ||
| } |
There was a problem hiding this comment.
JSdoc for excludeOperatorFamily is misleading.
The comment says undefined means "auto-detect (via {@link detectSupabase})", but detectSupabase only controls the supabase flag — excludeOperatorFamily's auto-fallback is driven by the permissions.isSuperuser check inside installCommand (Line 89-99), not by Supabase detection. Consider rewording to reference the superuser fallback so readers aren't sent looking for a Supabase→OPE link that doesn't exist.
✏️ Suggested tweak
- /**
- * `undefined` means "auto-detect" (via {`@link` detectSupabase}). An explicit
- * `true`/`false` from the user is preserved and skips detection.
- */
- excludeOperatorFamily?: boolean
+ /**
+ * `undefined` means "auto-fall back to OPE when the connected role is not a
+ * superuser" (CIP-2989). An explicit `true`/`false` from the user is
+ * preserved and skips the auto-fallback.
+ */
+ excludeOperatorFamily?: boolean📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * `undefined` means "auto-detect" (via {@link detectSupabase}). An explicit | |
| * `true`/`false` from the user is preserved and skips detection. | |
| */ | |
| excludeOperatorFamily?: boolean | |
| supabase?: boolean | |
| drizzle?: boolean | |
| latest?: boolean | |
| name?: string | |
| out?: string | |
| }) { | |
| } | |
| /** | |
| * `undefined` means "auto-fall back to OPE when the connected role is not a | |
| * superuser" (CIP-2989). An explicit `true`/`false` from the user is | |
| * preserved and skips the auto-fallback. | |
| */ | |
| excludeOperatorFamily?: boolean | |
| supabase?: boolean | |
| drizzle?: boolean | |
| latest?: boolean | |
| name?: string | |
| out?: string | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/db/install.ts` around lines 22 - 32, Update the
JSDoc for the excludeOperatorFamily option to stop referencing detectSupabase
and instead state that undefined triggers an auto-fallback based on the
superuser check in installCommand (permissions.isSuperuser); mention that an
explicit true/false from the user is preserved and skips the superuser-driven
detection so readers look at installCommand and permissions.isSuperuser for the
fallback logic.
| if (options.write) { | ||
| const target = resolve(process.cwd(), '.env.production.local') | ||
| if (existsSync(target)) { | ||
| const overwrite = await p.confirm({ | ||
| message: `${target} already exists. Overwrite?`, | ||
| initialValue: false, | ||
| }) | ||
| if (p.isCancel(overwrite) || !overwrite) { | ||
| p.cancel('Aborted.') | ||
| return | ||
| } | ||
| } | ||
|
|
||
| writeFileSync(target, block, 'utf-8') | ||
| p.log.success(`Wrote ${target}`) | ||
| p.outro('Done!') | ||
| return | ||
| } |
There was a problem hiding this comment.
Restrict permissions on the written .env.production.local file.
writeFileSync(target, block, 'utf-8') writes production credentials (CS_CLIENT_KEY) with the default umask (typically 0o644 — world-readable). Once the CTS mint endpoint is wired up, this file will contain a live secret, and leaving it group/world-readable on shared dev machines or CI runners is a secrets leak waiting to happen.
🔒 Proposed fix
- writeFileSync(target, block, 'utf-8')
+ writeFileSync(target, block, { encoding: 'utf-8', mode: 0o600 })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (options.write) { | |
| const target = resolve(process.cwd(), '.env.production.local') | |
| if (existsSync(target)) { | |
| const overwrite = await p.confirm({ | |
| message: `${target} already exists. Overwrite?`, | |
| initialValue: false, | |
| }) | |
| if (p.isCancel(overwrite) || !overwrite) { | |
| p.cancel('Aborted.') | |
| return | |
| } | |
| } | |
| writeFileSync(target, block, 'utf-8') | |
| p.log.success(`Wrote ${target}`) | |
| p.outro('Done!') | |
| return | |
| } | |
| if (options.write) { | |
| const target = resolve(process.cwd(), '.env.production.local') | |
| if (existsSync(target)) { | |
| const overwrite = await p.confirm({ | |
| message: `${target} already exists. Overwrite?`, | |
| initialValue: false, | |
| }) | |
| if (p.isCancel(overwrite) || !overwrite) { | |
| p.cancel('Aborted.') | |
| return | |
| } | |
| } | |
| writeFileSync(target, block, { encoding: 'utf-8', mode: 0o600 }) | |
| p.log.success(`Wrote ${target}`) | |
| p.outro('Done!') | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/env/index.ts` around lines 43 - 60, The file write
currently saves production secrets with default permissions via
writeFileSync(target, block, 'utf-8'); change this so the created
.env.production.local is restricted (owner read/write only). Update the write
path around the writeFileSync call for target (or immediately after) to set file
mode to 0o600 (either by passing an options object with mode: 0o600 to
writeFileSync or calling fs.chmodSync(target, 0o600)); ensure the success log
(p.log.success) still runs only after the permission change succeeds.
| /** | ||
| * Serialize the collected entries and append to `<cwd>/.cipherstash/wizard-log.md`. | ||
| * Safe to call multiple times — only appends new content. | ||
| */ | ||
| async flush(): Promise<string | undefined> { | ||
| if (this.entries.length === 0) return undefined | ||
|
|
||
| const dir = resolve(this.cwd, '.cipherstash') | ||
| if (!existsSync(dir)) mkdirSync(dir, { recursive: true }) | ||
| const path = join(dir, 'wizard-log.md') | ||
|
|
||
| const isNew = !existsSync(path) | ||
| const header = isNew ? '# CipherStash Wizard log\n\n' : '' | ||
| const body = this.render() | ||
|
|
||
| if (isNew) { | ||
| await writeFile(path, header + body, 'utf-8') | ||
| } else { | ||
| await appendFile(path, body, 'utf-8') | ||
| } | ||
|
|
||
| return path | ||
| } |
There was a problem hiding this comment.
flush() is not idempotent despite the JSDoc claim.
The docstring says "Safe to call multiple times — only appends new content", but render() iterates over the full this.entries array each time and entries are never cleared or marked as flushed. If flush() is invoked twice in a run (e.g., from both success and error paths, or once mid-run and again at end), the file will contain duplicate entries. Either clear/advance a cursor after writing, or document flush as "call exactly once".
♻️ Proposed fix
async flush(): Promise<string | undefined> {
- if (this.entries.length === 0) return undefined
+ if (this.flushedCount >= this.entries.length) return undefined
const dir = resolve(this.cwd, '.cipherstash')
if (!existsSync(dir)) mkdirSync(dir, { recursive: true })
const path = join(dir, 'wizard-log.md')
const isNew = !existsSync(path)
const header = isNew ? '# CipherStash Wizard log\n\n' : ''
- const body = this.render()
+ const body = this.render(this.flushedCount)
+ this.flushedCount = this.entries.length…and have render(fromIndex = 0) only emit entries from fromIndex onward (and skip the ## Run header on subsequent calls).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/wizard/lib/changelog.ts` around lines 35 - 57, The
flush() method currently appends the entire this.entries every call, causing
duplicate log content; modify the implementation to track which entries have
already been written (e.g., add a flushedIndex or clear/advance this.entries)
and change render to accept an optional fromIndex parameter (render(fromIndex =
0)) so flush() only renders and writes entries[fromIndex..end]; ensure the first
write still emits the "## Run" header but subsequent writes skip that header,
update flush() to call render(flushedIndex) and advance flushedIndex after
successful write so flush() becomes idempotent.
| const confirmed = await p.confirm({ | ||
| message: `Install ${available.length} Claude skill(s) into ./.claude/skills/ (${available.join(', ')})?`, | ||
| initialValue: true, | ||
| }) | ||
| if (p.isCancel(confirmed) || !confirmed) return [] | ||
|
|
||
| const destRoot = resolve(cwd, '.claude', 'skills') | ||
| mkdirSync(destRoot, { recursive: true }) | ||
|
|
||
| const copied: string[] = [] | ||
| for (const name of available) { | ||
| const src = join(bundledRoot, name) | ||
| const dest = join(destRoot, name) | ||
| try { | ||
| cpSync(src, dest, { recursive: true, force: true }) | ||
| copied.push(name) | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : String(err) | ||
| p.log.warn(`Failed to install skill ${name}: ${message}`) | ||
| } | ||
| } |
There was a problem hiding this comment.
force: true silently overwrites user customizations in .claude/skills/<name>/.
If a user has previously installed a skill and edited it locally (e.g. customized stash-drizzle/SKILL.md), re-running the wizard will overwrite those edits without warning. The confirm prompt at Line 42-45 doesn't mention that existing files will be clobbered, so this is a silent data-loss path for a small subset of users.
Consider one of:
- Detect pre-existing skill dirs and adjust the confirm copy (e.g. "Overwrite N existing skill(s) and install M new one(s)?").
- Skip copying for destinations that already exist unless the user opts in to overwrite.
Not a launch blocker since .claude/skills/ should generally be treated as CLI-managed, but worth a note in the prompt.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/wizard/lib/install-skills.ts` around lines 42 - 62,
Detect existing skill directories before copying by checking each destination
path (dest = join(destRoot, name)) with existsSync/statSync to build two lists:
existing (would be overwritten) and new (would be added); update the
confirmation prompt (the p.confirm call that references available) to mention
counts/names of existing vs new (e.g., "Overwrite N existing skill(s) and
install M new one(s)?") and if the user declines, return []; then during the
copy loop (where cpSync is called) either skip copying for destinations in the
existing list unless the user opted into overwriting, or honor an explicit
overwrite choice and keep cpSync({ recursive: true, force: true }) only when
overwriting; ensure p.log.warn is kept for per-skill failures.
| async function rewriteEncryptedMigrations(cwd: string): Promise<void> { | ||
| for (const dir of DRIZZLE_OUT_DIRS) { | ||
| const abs = resolve(cwd, dir) | ||
| if (!existsSync(abs)) continue | ||
|
|
||
| try { | ||
| const rewritten = await rewriteEncryptedAlterColumns(abs) | ||
| if (rewritten.length > 0) { | ||
| p.log.info( | ||
| `Rewrote ${rewritten.length} migration file(s) in ${dir}/ to use ADD+DROP+RENAME for encrypted columns.`, | ||
| ) | ||
| for (const file of rewritten) p.log.step(` - ${file}`) | ||
| p.log.warn( | ||
| 'If any of these tables already have rows, backfill the new column via @cipherstash/stack before running the migration in production. See the comments in the rewritten SQL.', | ||
| ) | ||
| } | ||
| // Only rewrite the first dir that matches — running again on a | ||
| // different candidate would double-transform already-rewritten SQL. | ||
| return | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : String(err) | ||
| p.log.warn(`Could not rewrite migrations in ${dir}: ${message}`) | ||
| return | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Early return on the first existing candidate dir may skip the real migrations directory.
rewriteEncryptedMigrations walks DRIZZLE_OUT_DIRS in order and returns on the first candidate that existsSync succeeds on — regardless of whether that directory actually contains any migrations drizzle-kit just emitted. If a project has a leftover or unrelated drizzle/ directory but drizzle.config.* points out to migrations/ (or src/db/migrations/), the rewrite silently no-ops on the empty drizzle/ and never touches the real output directory.
Consider reading drizzle.config.ts/js for the authoritative out value and falling back to the candidate list only if that fails, or at minimum continuing the loop when rewritten.length === 0 (the double-transform concern the comment raises only applies when something was actually rewritten).
♻️ Sketch
for (const dir of DRIZZLE_OUT_DIRS) {
const abs = resolve(cwd, dir)
if (!existsSync(abs)) continue
try {
const rewritten = await rewriteEncryptedAlterColumns(abs)
- if (rewritten.length > 0) {
- p.log.info(...)
- for (const file of rewritten) p.log.step(` - ${file}`)
- p.log.warn(...)
- }
- // Only rewrite the first dir that matches — running again on a
- // different candidate would double-transform already-rewritten SQL.
- return
+ if (rewritten.length > 0) {
+ p.log.info(...)
+ for (const file of rewritten) p.log.step(` - ${file}`)
+ p.log.warn(...)
+ // Only stop once we've actually rewritten something — otherwise keep
+ // looking; a stale empty candidate dir shouldn't mask the real one.
+ return
+ }
} catch (err) {
const message = err instanceof Error ? err.message : String(err)
p.log.warn(`Could not rewrite migrations in ${dir}: ${message}`)
return
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function rewriteEncryptedMigrations(cwd: string): Promise<void> { | |
| for (const dir of DRIZZLE_OUT_DIRS) { | |
| const abs = resolve(cwd, dir) | |
| if (!existsSync(abs)) continue | |
| try { | |
| const rewritten = await rewriteEncryptedAlterColumns(abs) | |
| if (rewritten.length > 0) { | |
| p.log.info( | |
| `Rewrote ${rewritten.length} migration file(s) in ${dir}/ to use ADD+DROP+RENAME for encrypted columns.`, | |
| ) | |
| for (const file of rewritten) p.log.step(` - ${file}`) | |
| p.log.warn( | |
| 'If any of these tables already have rows, backfill the new column via @cipherstash/stack before running the migration in production. See the comments in the rewritten SQL.', | |
| ) | |
| } | |
| // Only rewrite the first dir that matches — running again on a | |
| // different candidate would double-transform already-rewritten SQL. | |
| return | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : String(err) | |
| p.log.warn(`Could not rewrite migrations in ${dir}: ${message}`) | |
| return | |
| } | |
| } | |
| } | |
| async function rewriteEncryptedMigrations(cwd: string): Promise<void> { | |
| for (const dir of DRIZZLE_OUT_DIRS) { | |
| const abs = resolve(cwd, dir) | |
| if (!existsSync(abs)) continue | |
| try { | |
| const rewritten = await rewriteEncryptedAlterColumns(abs) | |
| if (rewritten.length > 0) { | |
| p.log.info( | |
| `Rewrote ${rewritten.length} migration file(s) in ${dir}/ to use ADD+DROP+RENAME for encrypted columns.`, | |
| ) | |
| for (const file of rewritten) p.log.step(` - ${file}`) | |
| p.log.warn( | |
| 'If any of these tables already have rows, backfill the new column via `@cipherstash/stack` before running the migration in production. See the comments in the rewritten SQL.', | |
| ) | |
| // Only stop once we've actually rewritten something — otherwise keep | |
| // looking; a stale empty candidate dir shouldn't mask the real one. | |
| return | |
| } | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : String(err) | |
| p.log.warn(`Could not rewrite migrations in ${dir}: ${message}`) | |
| return | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/wizard/lib/post-agent.ts` around lines 108 - 133,
rewriteEncryptedMigrations currently returns as soon as it finds the first
existing DRIZZLE_OUT_DIRS candidate, which can skip the real migrations dir;
change the control flow in rewriteEncryptedMigrations so it only returns (exits)
when rewriteEncryptedAlterColumns(abs) actually rewrote files (rewritten.length
> 0), and otherwise continues the loop to try the next candidate; likewise, on
catch do not return immediately—log the error and continue scanning other
DRIZZLE_OUT_DIRS; keep references to DRIZZLE_OUT_DIRS,
rewriteEncryptedAlterColumns, existsSync and the p.log calls when making the
change.
| lines.forEach((line, i) => { | ||
| if (integration === 'drizzle') { | ||
| if (drizzleInsert.test(line)) | ||
| out.push({ kind: 'insert', line: i + 1, snippet: line.trim() }) | ||
| if (drizzleUpdate.test(line)) | ||
| out.push({ kind: 'update', line: i + 1, snippet: line.trim() }) | ||
| if (drizzleSelect.test(line)) | ||
| out.push({ kind: 'select', line: i + 1, snippet: line.trim() }) | ||
| } else if (integration === 'supabase') { | ||
| if (supabaseFromInsert.test(line)) | ||
| out.push({ kind: 'insert', line: i + 1, snippet: line.trim() }) | ||
| if (supabaseFromUpdate.test(line)) | ||
| out.push({ kind: 'update', line: i + 1, snippet: line.trim() }) | ||
| if (supabaseFromSelect.test(line)) | ||
| out.push({ kind: 'select', line: i + 1, snippet: line.trim() }) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Duplicate matches when a single line triggers multiple kinds.
If one line happens to contain both (say) .from(users) and .update(users) — common in chained Drizzle query builders written on one line — you'll push two entries for the same source position. Consider picking the most specific match per line (insert/update before select) or deduping by (line, kind). Minor, only affects report noise.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/wizard/lib/wire-call-sites.ts` around lines 106 -
122, The loop over lines can push multiple entries when a single line matches
multiple regexes (e.g., drizzleInsert and drizzleSelect); update the logic in
the lines.forEach block to pick a single kind per line (prefer insert, then
update, then select) or dedupe pushes by tracking seen keys. Concretely, replace
the multiple if checks that call out.push with a single decision per line (e.g.,
compute let kind = drizzleInsert.test(line) ? 'insert' :
drizzleUpdate.test(line) ? 'update' : drizzleSelect.test(line) ? 'select' :
undefined and push only if kind) or maintain a Set of `${i}-${kind}` to avoid
duplicate out.push calls for the same (line, kind); apply the same approach for
the supabase branch using
supabaseFromInsert/supabaseFromUpdate/supabaseFromSelect.
1 participant
Summary
Addresses 16 Linear issues filed after the 2026-04-17 end-to-end onboarding test. Scope:
packages/cliandpackages/stackonly.Closes (in-scope)
Deferred (tracked, out of scope) — CIP-2998 (Supabase OAuth), CIP-2999 (analytics research), CIP-3000 (QA matrix).
Blocked upstream — CIP-2996.
@cipherstash/auth@0.35.0is the latest on npm;TODO(CIP-2996)marker added in login.ts. Bump the catalog pin once a fixed release ships.What changed
CLI command consolidation
db setupremoved entirely.db installnow scaffoldsstash.config.tson the fly when it's missing, so onboarding is one command.db installauto-detects Supabase (fromDATABASE_URLhost) and Drizzle (fromdrizzle.config.*ordrizzle-orm/drizzle-kitinpackage.json).--supabase/--drizzlestay as explicit overrides.--supabasenow threads through the--drizzlemigration path (previously silently dropped).db installprints a "what next" note pointing at the wizard and direct SDK usage.Drizzle migration correctness
encryptedTypeemits"public"."eql_v2_encrypted"so generated migrations no longer contain"undefined"."eql_v2_encrypted".ALTER COLUMN ... SET DATA TYPE eql_v2_encryptedstatements are automatically rewritten to an ADD + UPDATE-placeholder + DROP + RENAME sequence. Safe on empty tables; non-empty tables need backfill viaencryptModel— the comment in the rewritten SQL explains this. Rewrite lives at packages/cli/src/commands/db/rewrite-migrations.ts and runs in both the CLI drizzle path and the wizard's post-agent step.Wizard enhancements
./.claude/skills/(newinstall-skills.ts). Skills are bundled into the CLI at build time viatsup.config.ts..cipherstash/wizard-log.mdon each run (newchangelog.ts).src/app,src/lib,app/,lib/for Drizzle/Supabase insert/update/select call sites on encrypted tables and prints a report with recommendedencryptModel/decryptModelpatterns (newwire-call-sites.ts). Report-only — no file mutations.CLI runtime polish
initstreams npm/pnpm/yarn output instead of hiding it behind a silent spinner..env.local→.env.development.local→.env.development→.env(Next.js precedence, first-win).New experimental command
envcommand scaffold registered, gated behindSTASH_EXPERIMENTAL_ENV_CMD=1. CTS mint endpoint is TBD; command returns a clear "not ready" message until then.Files
Modified:
bin/stash.ts,commands/db/install.ts,commands/auth/login.ts,commands/index.ts,commands/init/providers/{base,drizzle,supabase}.ts,commands/init/steps/install-forge.ts,commands/wizard/{run.ts,lib/post-agent.ts,lib/prerequisites.ts},installer/index.ts,tsup.config.ts,packages/cli/README.md,packages/stack/src/drizzle/index.ts.New:
commands/db/{config-scaffold,detect,rewrite-migrations}.ts,commands/env/index.ts,commands/wizard/lib/{changelog,install-skills,wire-call-sites}.ts, plus tests__tests__/detect.test.tsand__tests__/rewrite-migrations.test.ts.Deleted:
commands/db/setup.ts.Test plan
pnpm -C packages/cli build— cleanpnpm -C packages/stack build— cleanpnpm -C packages/cli test— 124 pass / 5 skipped (20 new tests coveringdetect+rewrite-migrations)--helpoutput verified: nodb setup, nosecretssubcommand, no "Forge"DATABASE_URLin.env.localonly, runnpx @cipherstash/cli db installwithout flags, verify auto-detection, OPE fallback, next-steps note, and absence of"undefined"."eql_v2_encrypted"in the generated migrationwizardend-to-end — verify.cipherstash/wizard-log.mdis written, skills prompt fires, call-site report listssrc/app/...entriescipherstash/docs(drafted but not opened yet —forge/→cli/rename + behavior prose updates)Notes for the reviewer
main— they're not regressions from this PR.packages/cli/src/commands/auth/login.tshas aTODO(CIP-2996)marker explaining the upstream fix we're waiting on. Please don't patch around it in the CLI; the right fix is in@cipherstash/auth.encryptModelfor non-empty tables. On empty tables it behaves like DROP + ADD.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
envcommand for managing environment variables.Documentation
Tests