fix(cli): runner-aware help banners + wizard reference#384
fix(cli): runner-aware help banners + wizard reference#384
Conversation
🦋 Changeset detectedLatest commit: 03d845e The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 selected for processing (6)
✨ 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 30 minutes and 10 seconds.Comment |
Follow-up to PR #377 addressing Lindsay's feedback that the HELP banners and the wizard reference in `db install`'s "Next steps" panel still hard-coded `npx`. #379 already added `runnerCommand(pm, ref)` + `detectPackageManager()`; this sweep applies it to the remaining places. User-visible: a user who runs `bunx @cipherstash/cli --help` now sees `bunx @cipherstash/cli` in the Usage line and Examples, and the wizard suggestion at the end of `db install` matches the user's runner. Same for `pnpm dlx`, `yarn dlx`, default `npx`. Touches: - `bin/stash.ts` — `HELP` template now interpolates a `STASH` constant (`runnerCommand(PM, '@cipherstash/cli')`) for the Usage line, the six Examples, and the `requireStack` hint. - `commands/auth/index.ts` — same pattern for the auth HELP, with a `STASH_AUTH` constant for the Usage line + two examples. - `commands/db/install.ts` — the wizard line in `printNextSteps` uses `runnerCommand(pm, '@cipherstash/wizard')`. Also brings the `p.intro('npx @cipherstash/cli db install')` line in line with the runner-aware pattern (was already pinned for the same reason in PR #377; this branch is off main, so it lands here too). Messages: `messages.cli.usagePrefix` and `messages.auth.usagePrefix` become stable leaders (`'Usage: '`) — the runner+package suffix is appended at render time. E2E assertions stay runner-agnostic because they `toContain('Usage: ')` rather than the full prefix. Out of scope (still): the migrate stub message in `messages.db.migrateNotImplemented` keeps its hard-coded `npx` form. The stub is a placeholder and the inconsistency only shows when someone runs a not-yet-implemented command — not worth plumbing for.
Adds 8 new E2E tests in `tests/e2e/runner-aware-help.e2e.test.ts` asserting that `--help` and `auth --help` surface the right runner (`npx` / `bunx` / `pnpm dlx` / `yarn dlx`) based on `npm_config_user_agent`. Covers the gap between the existing unit tests for `detectPackageManager` / `runnerCommand` (which prove the helpers work in isolation) and the user-visible HELP rendering (which the smoke suite checks for existence but not runner-correctness). The Next-steps panel in `db install` and the `requireStack` hint are still uncovered by automation — they require either a real DB or a fixture without `@cipherstash/stack`. Documented as known gaps in the PR description.
3a708f9 to
03d845e
Compare
Closes #385.
Follow-up to #377 addressing Lindsay's feedback that the
--helpbanners and the wizard suggestion indb install's "Next steps" panel still hard-codednpx @cipherstash/cliregardless of how the user invoked the CLI.#379 already shipped
runnerCommand(pm, ref)+detectPackageManager(). This applies them to the remaining places.Before / after
Same for
pnpm dlx,yarn dlx, defaultnpx.Touches
bin/stash.ts—HELPinterpolates aSTASHconstant (runnerCommand(PM, '@cipherstash/cli')) for the usage line, six examples, and therequireStackhint shown when@cipherstash/stackisn't installed.commands/auth/index.ts— same pattern for the auth HELP (usage line + two examples).commands/db/install.ts— the wizard line inprintNextStepsusesrunnerCommand(pm, '@cipherstash/wizard').Messages
messages.cli.usagePrefixandmessages.auth.usagePrefixbecome stable leaders ('Usage: ') — the runner+package portion is appended at render time. The smoke E2E tests already usetoContainagainst these constants, so they stay runner-agnostic without test changes.Out of scope
messages.db.migrateNotImplementedkeeps its hard-codednpxform.db migrateis a not-yet-implemented stub; the inconsistency only surfaces when someone runs a command that doesn't exist yet. Not worth plumbing.Manual test plan
I ran each of the below locally; would appreciate reviewers running them too.
git checkout dan/runner-aware-help-banners pnpm install pnpm --filter @cipherstash/cli build STASH=$PWD/packages/cli/dist/bin/stash.jsDetection prefers
npm_config_user_agentover lockfile; overriding it directly is the easiest way to test each runner without installing them.1. Top-level
--help— Usage line and the six Examples should all use the runner:```sh
node $STASH --help # → npx (default)
npm_config_user_agent='bun/1.x' node $STASH --help # → bunx
npm_config_user_agent='pnpm/9.x' node $STASH --help # → pnpm dlx
npm_config_user_agent='yarn/4.x' node $STASH --help # → yarn dlx
```
2.
auth --help(or justauthwith no subcommand):```sh
npm_config_user_agent='bun/1.x' node $STASH auth
Expected: "Usage: bunx @cipherstash/cli auth [options]"
Examples: "bunx @cipherstash/cli auth login" / "...auth login --supabase"
```
3.
db install"Next steps" panel — the wizard line should match the runner:```sh
mkdir /tmp/runner-test && cd /tmp/runner-test
DATABASE_URL=postgresql://test/x npm_config_user_agent='bun/1.x' \
node $STASH db install --dry-run
When the panel prints, the wizard line should read:
bunx @cipherstash/wizard
(You'll also see "bunx @cipherstash/cli db install" as the intro — covered by #377.)
```
4.
requireStackhint — surfaces when runningdb push/validate/schema buildwithout@cipherstash/stackinstalled:```sh
cd /tmp/runner-test
DATABASE_URL=postgresql://test/x npm_config_user_agent='pnpm/9.x' \
node $STASH db push
Expected error:
@cipherstash/stack is required for this command.
Install it with: pnpm add @cipherstash/stack
Or run: pnpm dlx @cipherstash/cli init
```
5. Lockfile fallback — without
npm_config_user_agent, detection falls back to lockfile:```sh
cd /tmp/runner-test
touch bun.lock
node $STASH --help # without npm_config_user_agent override
Expected: bunx-flavoured output
rm bun.lock && touch pnpm-lock.yaml
node $STASH --help
Expected: pnpm dlx
rm pnpm-lock.yaml
```
Automated coverage
runnerCommand/detectPackageManagerthemselvessrc/commands/init/__tests__/utils.test.ts(from #379)--helpUsage line + Examplestests/e2e/runner-aware-help.e2e.test.ts— 4 cases (npx/bunx/pnpm dlx/yarn dlx) asserting the right runner appearsauth --helpUsage line + Examplesdb install"Next steps" panel (wizard line)requireStackhint@cipherstash/stackinstalled so the path doesn't fire in CI. Manual only.messages.cli.usagePrefixno longer hardcoded withnpxThe two gaps (Next-steps panel and requireStack hint) are awkward to E2E without standing up a real DB or surgically removing
@cipherstash/stackfrom the test environment. If a future PR adds either of those harnesses, both would be cheap to cover then.Test plan
pnpm --filter @cipherstash/cli buildcleanpnpm --filter @cipherstash/cli test— unit tests passpnpm --filter @cipherstash/cli test:e2e— 18 E2E tests pass (was 10; +8 from newrunner-aware-help.e2e.test.ts)biome checkclean on changed filesnode dist/bin/stash.js --helpwithnpm_config_user_agentcleared / set topnpm/9/ set tobun/1and confirmed the rendered Usage line + Examples match the runner.