fix(cli, wizard): make stash db push opt-in for Proxy users only#448
Conversation
🦋 Changeset detectedLatest commit: f322aae The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
3d9cc22 to
13edf57
Compare
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces a ChangesCipherStash Proxy mode detection and conditional behavior
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
stash db push opt-in for Proxy users only
`stash db push` is only required when querying encrypted data via CipherStash Proxy. SDK users (Drizzle, Supabase, plain pg) have the encryption config in app code, so the database doesn't need a copy. Agents reading the rollout/cutover prompts and skills were insisting on running it anyway. This change: - Adds `--proxy` / `--no-proxy` flags and an interactive prompt to `stash init`. The choice persists to `.cipherstash/context.json` as `usesProxy`. Default is SDK-only (false). - Threads `usesProxy` through `stash plan` and `stash impl` so the setting survives re-runs, and into the wizard's `GatheredContext`. - Gates all `stash db push` (and `stash db activate`) steps in `setup-prompt.ts` renderers on `usesProxy`. SDK renderings drop the steps and renumber; Proxy renderings preserve the current text. - Reframes the four skills (`stash-cli`, `stash-encryption`, `stash-drizzle`, `stash-supabase`) so default walkthroughs are SDK-only, with `db push` moved into `> **Using CipherStash Proxy?**` callouts. - Gates the wizard's post-agent push step on `gathered.usesProxy`, with a visible skip log when off. - Documents a known gap: `stash encrypt cutover` currently requires a pending EQL config (set by `db push`), so SDK-only users running the migrate-existing-column flow hit "No pending EQL configuration" from cutover. Workaround: run `db push` once before cutover. Decoupling cutover from EQL config for SDK users is tracked as a follow-up. Fixes #447. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
13edf57 to
bb9764d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/commands/init/lib/read-context.ts`:
- Around line 40-45: Normalize the usesProxy field to a strict boolean when
returning the parsed context in read-context.ts: instead of forwarding any
non-nullish value from parsed, set usesProxy to true only if parsed.usesProxy is
exactly the boolean true (e.g., parsed.usesProxy === true), otherwise false;
update the return object in the block that currently spreads ...parsed and
references usesProxy and ensure this change is made where isContextFile(parsed)
is checked.
In `@packages/cli/src/commands/init/lib/setup-prompt.ts`:
- Around line 300-303: The cutover step text "4. **Switch the schema, then
cutover.**" currently omits the SDK-only workaround for missing pending EQL
config; update that string (and any duplicate cutover instruction blocks with
the same heading) to append a short workaround note instructing SDK users to run
the DB push command to register a pending EQL configuration before cutover
(e.g., run the equivalent of `stash db push` to create the pending config) or
follow the manual pending-config registration if they see "No pending EQL
configuration", so cutover won't fail when not using the Proxy.
In `@skills/stash-encryption/SKILL.md`:
- Around line 662-695: Update the CLI sequence so SDK-only users run the
required database schema push before the cutover: insert a step invoking "stash
db push" (or note to run it in CI) immediately before "stash encrypt cutover
--table users --column email" in the example block, and add a short
parenthetical reminder near the "stash encrypt cutover" and "stash encrypt
backfill" commands that SDK-only deployments must run "stash db push" once to
ensure the schema/column rename is present; reference the "stash encrypt
backfill", "stash encrypt cutover", and "stash db push" commands when making the
change.
In `@skills/stash-supabase/SKILL.md`:
- Around line 555-558: Fix the MD028 markdownlint error by removing the blank
line inside the blockquote that begins with "**Known gap (SDK-only users):**"
and the subsequent "**Using CipherStash Proxy?**" lines in SKILL.md; ensure each
blockquote line is contiguous (no empty lines between lines starting with ">")
so the two quoted paragraphs are either merged into one continuous blockquote or
split correctly with an explicit end to the blockquote before any non-quoted
content.
🪄 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: 1d64123e-3f95-4dbc-9652-828c231483c9
📒 Files selected for processing (18)
.changeset/proxy-only-db-push.mdpackages/cli/src/bin/stash.tspackages/cli/src/commands/impl/index.tspackages/cli/src/commands/init/index.tspackages/cli/src/commands/init/lib/__tests__/setup-prompt.test.tspackages/cli/src/commands/init/lib/read-context.tspackages/cli/src/commands/init/lib/setup-prompt.tspackages/cli/src/commands/init/lib/write-context.tspackages/cli/src/commands/init/steps/resolve-proxy-choice.tspackages/cli/src/commands/init/types.tspackages/cli/src/commands/plan/index.tspackages/wizard/src/__tests__/post-agent.test.tspackages/wizard/src/lib/gather.tspackages/wizard/src/lib/post-agent.tsskills/stash-cli/SKILL.mdskills/stash-drizzle/SKILL.mdskills/stash-encryption/SKILL.mdskills/stash-supabase/SKILL.md
## What Coerce `usesProxy` to `true` only when the parsed value is exactly the boolean `true`, instead of forwarding any non-nullish value. ## Why - `isContextFile()` never validated `usesProxy`, so a hand-edited context.json could put a string or number into the field - `?? false` only guarded null/undefined, letting truthy non-booleans like the string `"false"` flow through and flip rollout rendering ## How - **Strict equality.** Set `usesProxy` from `parsed.usesProxy === true`, which collapses both missing values and bad types to `false`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## What The SDK-only cutover step carried a Proxy-framed aside about `db push`. Replace it with a note covering the known gap where `stash encrypt cutover` fails for SDK-only setups. ## Why - `stash encrypt cutover` aborts with "No pending EQL configuration" unless a pending EQL config exists - SDK-only users never run `stash db push`, so the SDK rollout prompt walked them into a cutover that fails ## How - **Workaround in place.** State the gap on the cutover step and tell the user to run `db push` once if cutover reports the error. - **Test follows intent.** Update the setup-prompt test to assert the workaround note instead of the removed Proxy aside. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## What The SDK-only CLI sequence ran `stash encrypt cutover` with no preceding `stash db push`. Add the push step ahead of cutover. ## Why - `stash encrypt cutover` requires a pending EQL config, so the example as written fails at the cutover line for SDK-only deployments - The Known limitation callout above the block named the workaround but the runnable sequence still omitted it ## How - **Runnable example.** Insert `stash db push` before `stash encrypt cutover` with a comment pointing back at the Known limitation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## What A blank line separated the "Known gap" and "Using CipherStash Proxy?" blockquotes. Replace it with a `>` marker. ## Why - markdownlint flags the blank line between two blockquotes as MD028 - A bare `>` keeps the two callouts as distinct paragraphs within one contiguous blockquote, so the rendering is unchanged ## How - **Contiguous blockquote.** Swap the empty line for a `>` line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Don't run
stash db pushunless the user explicitly states they're using CipherStash Proxy:stash plan/stash implprompts and skills were insisting on running it anyway.--proxy/--no-proxyflags and an interactive prompt onstash initcapture whether the user queries via CipherStash Proxy..cipherstash/context.jsonasusesProxy, and is honoured bystash plan,stash impl, and the wizard's post-agent step.stash-cli,stash-encryption,stash-drizzle,stash-supabase) updated to be SDK focuseddb pushmoves into> **Using CipherStash Proxy?**callouts. A "Known gap" callout warns SDK users about the cutover precondition before they hit it.Why
Background in #447.
The README already had the right framing — "Only required when using CipherStash Proxy" — but the agent-facing prompts and skills didn't.
Limitations
stash encrypt cutovercurrently requires a pending EQL config (registered viastash db push), so SDK-only users running the migrate-existing-column flow will hit aNo pending EQL configurationerror from cutover.The workaround is to run
stash db pushonce beforestash encrypt cutover.The longer term fix is to either:
encrypt cutoverfor SDK-only users at cutover timeSummary by CodeRabbit
Release Notes
New Features
--proxyand--no-proxyflags and interactive prompt during initialization to select between CipherStash Proxy or SDK querying.stash db pushonly included for Proxy users.Documentation