chore: standardize pnpm commands by gringokiwi · Pull Request #591 · arkade-os/wallet

gringokiwi · 2026-05-01T01:12:00Z

Summary by CodeRabbit

Documentation
- Simplified command syntax examples in README documentation.
Chores
- Refined Docker build configuration for improved consistency.

coderabbitai · 2026-05-01T01:12:06Z

Walkthrough

Dockerfile build process updated to invoke Vite through pnpm exec instead of npx within the builder stage. README documentation examples simplified to use direct PNPM script invocation syntax (e.g., pnpm start vs pnpm run start).

Changes

Cohort / File(s)	Summary
Build Configuration `Dockerfile`	Modified builder stage to use `pnpm exec` for Vite invocation instead of `npx` when executing the build after worker compilation.
Documentation `README.md`	Updated command examples across development, production build, regtest, and e2e sections to use simplified PNPM script syntax (`pnpm <script>` instead of `pnpm run <script>`).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'chore: standardize pnpm commands' accurately reflects the main change: updating pnpm command invocation syntax across Dockerfile and README for consistency.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch standardize-pnpm

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

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

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

arkanaai

Review: `chore: standardize pnpm commands`

README.md changes ✅

All pnpm run <script> → pnpm <script> changes are correct. pnpm run and pnpm are equivalent for running scripts — the shorthand is idiomatic pnpm.

Dockerfile — `npx vite build` → `pnpm dlx vite build` ❌

Dockerfile:34 — This change introduces a subtle but real bug.

npx and pnpm dlx are not equivalent when the package is already installed locally:

npx vite build — checks node_modules/.bin first, finds the locally installed vite@^7.1.3 (pinned by pnpm-lock.yaml via pnpm install --frozen-lockfile), and runs it. Deterministic.
pnpm dlx vite build — downloads the latest vite from the registry into a temp directory and runs it, ignoring the locally installed version. Non-deterministic. Could pull a breaking major version in the future.

The correct pnpm equivalent of npx <local-package> is pnpm exec, not pnpm dlx:

pnpm build:worker && pnpm exec vite build

Or even simpler — just use the build script which already does vite build:

pnpm build

(Though note pnpm build also runs git-info and build:worker, so you'd drop the separate pnpm build:worker call. The Dockerfile already writes _gitCommit.ts manually, so using pnpm build directly would run git-info redundantly but harmlessly.)

TL;DR: Change pnpm dlx vite build → pnpm exec vite build to preserve the current behavior of using the lockfile-pinned vite version.

🤖 Reviewed by Arkana

cloudflare-workers-and-pages · 2026-05-01T13:55:26Z

Deploying tmp-boltz-upstream-mainnet-arkade-wallet with Cloudflare Pages

Latest commit:	`8b03450`
Status:	✅ Deploy successful!
Preview URL:	https://f6dc5248.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev
Branch Preview URL:	https://standardize-pnpm.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev

View logs

arkanaai

Previous feedback addressed: pnpm dlx vite build → pnpm exec vite build ✅

All changes look correct:

Dockerfile:34: pnpm exec vite build correctly uses the lockfile-pinned vite. Good.
README.md: All pnpm run <script> → pnpm <script> shorthand changes are idiomatic and equivalent.

LGTM.

🤖 Reviewed by Arkana

cloudflare-workers-and-pages · 2026-05-01T13:56:11Z

Deploying wallet-mutinynet with Cloudflare Pages

Latest commit:	`8b03450`
Status:	✅ Deploy successful!
Preview URL:	https://4f096975.arkade-wallet.pages.dev
Branch Preview URL:	https://standardize-pnpm.arkade-wallet.pages.dev

View logs

cloudflare-workers-and-pages · 2026-05-01T13:56:14Z

Deploying wallet-bitcoin with Cloudflare Pages

Latest commit:	`8b03450`
Status:	✅ Deploy successful!
Preview URL:	https://6d204043.wallet-bitcoin.pages.dev
Branch Preview URL:	https://standardize-pnpm.wallet-bitcoin.pages.dev

View logs

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@README.md`:
- Around line 114-116: The README mentions a non-existent "pnpm regtest" script;
update the docs to reference the actual npm scripts (regtest:start,
regtest:setup, regtest:stop, regtest:clean) or add a top-level "regtest" script
in package.json that chains the appropriate commands (e.g., run regtest:setup
then regtest:start) so the examples work; also remove the stray blockquote
marker causing the MD028 warning (remove the leading ">" on the affected line)
and apply the same fixes to the other occurrence of the same section.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9314d54e-499f-4882-906f-23bb36e0cc6b

📥 Commits

Reviewing files that changed from the base of the PR and between 0474193 and 8b03450.

📒 Files selected for processing (2)

Dockerfile
README.md

coderabbitai · 2026-05-01T13:58:20Z

+### `pnpm regtest`

 Starts the regtest environment and sets up the arkd instance.\


⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

pnpm regtest is not a defined script in this repo.

Line 114 and Line 131 document pnpm regtest, but package.json defines only regtest:start, regtest:setup, regtest:stop, and regtest:clean. This will fail for readers. This section also has the MD028 blockquote warning (Line 132).

Suggested README fix

-### `pnpm regtest` +### `pnpm regtest:start && pnpm regtest:setup` Starts the regtest environment and sets up the arkd instance.\ Requires Docker to be installed and [Nigiri](https://nigiri.vulpem.com/) to be running with `--ln` flag. @@ > note: e2e tests require a regtest environment to be running. -> `pnpm regtest` to start and setup the regtest environment. - +> `pnpm regtest:start && pnpm regtest:setup` to start and setup the regtest environment. +> > note: e2e tests use playwright for ui testing, you may need to run > `pnpm exec playwright install` once to download new browsers.

Also applies to: 130-134

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 114 - 116, The README mentions a non-existent "pnpm regtest" script; update the docs to reference the actual npm scripts (regtest:start, regtest:setup, regtest:stop, regtest:clean) or add a top-level "regtest" script in package.json that chains the appropriate commands (e.g., run regtest:setup then regtest:start) so the examples work; also remove the stray blockquote marker causing the MD028 warning (remove the leading ">" on the affected line) and apply the same fixes to the other occurrence of the same section.

Dunsin-cyber · 2026-05-01T21:31:44Z

-### `pnpm run regtest`
+### `pnpm regtest`

 Starts the regtest environment and sets up the arkd instance.\


hi @gringokiwi, since you're already updating the docs here, would it be worth adding git submodule update --init --recursive right before the pnpm regtest setups? I noticed on a fresh clone that pnpm run regtest:start fails silently without it. just a thought to save new users some debugging.

cc/ @bordalix

arkanaai Bot requested changes May 1, 2026

View reviewed changes

chore: standardize pnpm commands

8b03450

gringokiwi force-pushed the standardize-pnpm branch from c96728a to 8b03450 Compare May 1, 2026 13:55

arkanaai Bot approved these changes May 1, 2026

View reviewed changes

coderabbitai Bot reviewed May 1, 2026

View reviewed changes

Dunsin-cyber reviewed May 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: standardize pnpm commands#591

chore: standardize pnpm commands#591
gringokiwi wants to merge 1 commit into
masterfrom
standardize-pnpm

gringokiwi commented May 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 1, 2026 •

edited

Loading

Uh oh!

arkanaai Bot left a comment

Uh oh!

cloudflare-workers-and-pages Bot commented May 1, 2026 •

edited

Loading

Uh oh!

arkanaai Bot left a comment

Uh oh!

cloudflare-workers-and-pages Bot commented May 1, 2026

Uh oh!

cloudflare-workers-and-pages Bot commented May 1, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot May 1, 2026

Uh oh!

Dunsin-cyber May 1, 2026

Uh oh!

tiero May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		### `pnpm regtest`

		Starts the regtest environment and sets up the arkd instance.\

Conversation

gringokiwi commented May 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

arkanaai Bot left a comment

Choose a reason for hiding this comment

Review: chore: standardize pnpm commands

README.md changes ✅

Dockerfile — npx vite build → pnpm dlx vite build ❌

Uh oh!

cloudflare-workers-and-pages Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying tmp-boltz-upstream-mainnet-arkade-wallet with Cloudflare Pages

Uh oh!

arkanaai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cloudflare-workers-and-pages Bot commented May 1, 2026

Deploying wallet-mutinynet with Cloudflare Pages

Uh oh!

cloudflare-workers-and-pages Bot commented May 1, 2026

Deploying wallet-bitcoin with Cloudflare Pages

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Dunsin-cyber May 1, 2026

Choose a reason for hiding this comment

Uh oh!

tiero May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gringokiwi commented May 1, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 1, 2026 •

edited

Loading

Review: `chore: standardize pnpm commands`

Dockerfile — `npx vite build` → `pnpm dlx vite build` ❌

cloudflare-workers-and-pages Bot commented May 1, 2026 •

edited

Loading