feat: migrate @cipherstash/cli to stash#387
Merged
calvinbrewer merged 2 commits intomainfrom May 1, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: db50f07 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 |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 ignored due to path filters (1)
📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThis PR renames the CLI package from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 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. Review rate limit: 0/1 reviews remaining, refill in 40 minutes and 42 seconds.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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/basic/package.json (1)
1-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd explicit Node/pnpm constraints for the example package.
This touched example manifest still doesn’t declare the required runtime/tooling versions, which can cause inconsistent local behavior.
Proposed fix
{ "name": "@cipherstash/basic-example", "private": true, "version": "1.2.8", "type": "module", + "engines": { + "node": ">=22" + }, + "packageManager": "pnpm@9", "scripts": { "start": "tsx index.ts" },As per coding guidelines
examples/**/package.json: Use Node.js >= 22 and pnpm 9.x for example apps.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/basic/package.json` around lines 1 - 23, Add explicit Node and pnpm constraints to the package.json by adding an "engines" entry and a "packageManager" field: set "engines.node" to ">=22" and "engines.pnpm" (or "packageManager") to require pnpm 9.x (e.g., "pnpm": ">=9 <10" and "packageManager": "pnpm@9"). Update the top-level manifest keys in this package.json so tooling and runtime enforce Node.js >=22 and pnpm 9.x.packages/stack/README.md (1)
497-497:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stale auth command in Local Development section.
Line 497 still points to
npx@cipherstash/stackauth login, which is not the CLI invocation used elsewhere in this rename and can mislead users.📝 Proposed doc fix
-No environment variables or credentials are needed for local development. Run `npx `@cipherstash/stack` auth login` to authenticate via the device code flow, and the SDK and CLI will use the token saved to `~/.cipherstash/auth.json`. +No environment variables or credentials are needed for local development. Run `npx stash auth login` to authenticate via the device code flow, and the SDK and CLI will use the token saved to `~/.cipherstash/auth.json`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack/README.md` at line 497, Update the stale CLI example: replace the old invocation string "npx `@cipherstash/stack` auth login" in the Local Development paragraph with the current CLI command used across the repository (use the renamed invocation without the deprecated "auth" subcommand, e.g., "npx `@cipherstash/stack` login"), ensuring the README's text and the saved token path reference (~/.cipherstash/auth.json) remain accurate.
🧹 Nitpick comments (2)
packages/cli/src/installer/index.ts (1)
318-319: ⚡ Quick winDeduplicate the bundled-script error copy
Both catch blocks now carry the same message literal. Extracting one shared formatter/constant will prevent future divergence.
♻️ Proposed cleanup
+const bundledSqlLoadError = (filename: string) => + `Failed to load bundled EQL install script (${filename}). The package may be corrupted — try reinstalling stash.` ... - throw new Error( - `Failed to load bundled EQL install script (${filename}). The package may be corrupted — try reinstalling stash.`, - { cause: error }, - ) + throw new Error(bundledSqlLoadError(filename), { cause: error }) ... - throw new Error( - `Failed to load bundled EQL install script (${filename}). The package may be corrupted — try reinstalling stash.`, - { cause: error }, - ) + throw new Error(bundledSqlLoadError(filename), { cause: error })Based on learnings: "Update E2E tests when changing user-facing exit messages and strings; store assertion-stable strings in
src/messages.tsas a typedconstobject grouped by area (cli,auth,db)."Also applies to: 389-390
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/installer/index.ts` around lines 318 - 319, The same error message literal used in two catch blocks in installer/index.ts should be extracted to a shared constant or formatter (e.g. export const MESSAGES.cli.bundledEqlScriptLoadFailure = (filename) => `Failed to load bundled EQL install script (${filename}). The package may be corrupted — try reinstalling stash.`) and both error-handling sites should call that constant/formatter and pass the same { cause: error } into processLogger.error / exit so the message is not duplicated; update any tests that assert the literal to reference the new constant if needed.AGENTS.md (1)
144-147: ⚡ Quick winMake the “requires a build” note more actionable (explicit build step or dependency).
Right now Line 145-147 says the E2E suite “requires a build”, but doesn’t spell out the recommended command sequence. Given the E2E suite typically asserts against the built CLI artifact, it would help to add one concrete line like:
pnpm --filter stash build(or: “pnpm --filter stash test:e2edepends on build”)This reduces contributor confusion and flakiness when
dist/bin/stash.jsis missing/outdated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 144 - 147, Update the note in AGENTS.md about the pty-driven E2E tests under packages/cli/tests/e2e/** to explicitly state the required build step and give the concrete command to run before tests; e.g., say that pnpm --filter stash test:e2e depends on the built CLI artifact and instruct contributors to run pnpm --filter stash build (or run pnpm --filter stash build && pnpm --filter stash test:e2e) so dist/bin/stash.js is present and up to date when running the E2E suite.
🤖 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/protect/src/bin/stash.ts`:
- Around line 169-171: Replace all example CLI invocations that use the unscoped
"npx stash" with the scoped package invocation so users run
`@cipherstash/protect`; specifically update the example lines that show commands
like "npx stash secrets set ..." to use either "npx -p `@cipherstash/protect`
stash ..." or "npx `@cipherstash/protect` ..." instead. Locate the invocation
examples in this file (instances of the string "npx stash" around the examples
shown and other occurrences reported) and perform a global replace of those
example prefixes to the scoped form while preserving the rest of each example
command.
In `@packages/wizard/src/agent/interface.ts`:
- Around line 69-70: The allowlist currently uses startsWith so strings like
"npx stash db push && rm -rf /" bypass it; change the validation around the
entries 'npx stash db' and 'stash db' so you don't allow arbitrary shell
operators: instead of startsWith, split the user input into tokens (whitespace),
verify the first N tokens exactly match one of the allowed token sequences
(e.g., ['npx','stash','db'] or ['stash','db']), and then either require there
are no further tokens or validate remaining tokens only match known safe
subcommands; additionally reject any input containing shell metacharacters (&&,
||, ;, |, ``, $(), >, <) before accepting. Ensure you update the
startsWith-based check to use this tokenized exact-match plus metacharacter
rejection logic.
---
Outside diff comments:
In `@examples/basic/package.json`:
- Around line 1-23: Add explicit Node and pnpm constraints to the package.json
by adding an "engines" entry and a "packageManager" field: set "engines.node" to
">=22" and "engines.pnpm" (or "packageManager") to require pnpm 9.x (e.g.,
"pnpm": ">=9 <10" and "packageManager": "pnpm@9"). Update the top-level manifest
keys in this package.json so tooling and runtime enforce Node.js >=22 and pnpm
9.x.
In `@packages/stack/README.md`:
- Line 497: Update the stale CLI example: replace the old invocation string "npx
`@cipherstash/stack` auth login" in the Local Development paragraph with the
current CLI command used across the repository (use the renamed invocation
without the deprecated "auth" subcommand, e.g., "npx `@cipherstash/stack` login"),
ensuring the README's text and the saved token path reference
(~/.cipherstash/auth.json) remain accurate.
---
Nitpick comments:
In `@AGENTS.md`:
- Around line 144-147: Update the note in AGENTS.md about the pty-driven E2E
tests under packages/cli/tests/e2e/** to explicitly state the required build
step and give the concrete command to run before tests; e.g., say that pnpm
--filter stash test:e2e depends on the built CLI artifact and instruct
contributors to run pnpm --filter stash build (or run pnpm --filter stash build
&& pnpm --filter stash test:e2e) so dist/bin/stash.js is present and up to date
when running the E2E suite.
In `@packages/cli/src/installer/index.ts`:
- Around line 318-319: The same error message literal used in two catch blocks
in installer/index.ts should be extracted to a shared constant or formatter
(e.g. export const MESSAGES.cli.bundledEqlScriptLoadFailure = (filename) =>
`Failed to load bundled EQL install script (${filename}). The package may be
corrupted — try reinstalling stash.`) and both error-handling sites should call
that constant/formatter and pass the same { cause: error } into
processLogger.error / exit so the message is not duplicated; update any tests
that assert the literal to reference the new constant if needed.
🪄 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: 317cd4e5-dbe7-410e-9d7d-53c304fae660
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (54)
.changeset/breezy-cloths-wave.md.github/workflows/tests.ymlAGENTS.mddocs/plans/cli-pty-integration-tests.mde2e/package.jsone2e/tests/package-managers.e2e.test.tsexamples/basic/package.jsonexamples/basic/stash.config.tspackages/cli/AGENTS.mdpackages/cli/README.mdpackages/cli/package.jsonpackages/cli/src/bin/stash.tspackages/cli/src/commands/auth/index.tspackages/cli/src/commands/db/config-scaffold.tspackages/cli/src/commands/db/install.tspackages/cli/src/commands/db/push.tspackages/cli/src/commands/db/rewrite-migrations.tspackages/cli/src/commands/db/status.tspackages/cli/src/commands/db/supabase-migration.tspackages/cli/src/commands/db/test-connection.tspackages/cli/src/commands/db/upgrade.tspackages/cli/src/commands/db/validate.tspackages/cli/src/commands/env/index.tspackages/cli/src/commands/init/__tests__/utils.test.tspackages/cli/src/commands/init/providers/__tests__/base.test.tspackages/cli/src/commands/init/providers/__tests__/drizzle.test.tspackages/cli/src/commands/init/providers/__tests__/supabase.test.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/init/utils.tspackages/cli/src/config/index.tspackages/cli/src/index.tspackages/cli/src/installer/index.tspackages/cli/src/messages.tspackages/cli/tests/helpers/pty.tspackages/protect/src/bin/stash.tspackages/stack/README.mdpackages/wizard/README.mdpackages/wizard/src/__tests__/errors-runner.test.tspackages/wizard/src/__tests__/format.test.tspackages/wizard/src/__tests__/gateway-messages.test.tspackages/wizard/src/__tests__/interface.test.tspackages/wizard/src/__tests__/post-agent.test.tspackages/wizard/src/__tests__/prerequisites.test.tspackages/wizard/src/agent/commandments.tspackages/wizard/src/agent/errors.tspackages/wizard/src/agent/interface.tspackages/wizard/src/lib/post-agent.tspackages/wizard/src/lib/prerequisites.tspackages/wizard/src/lib/rewrite-migrations.tsskills/stash-cli/SKILL.mdskills/stash-secrets/SKILL.md
auxesis
approved these changes
May 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
@cipherstash/clitostash.npx @cipherstash/clitonpx stash.stashinstead of@cipherstash/cli.