Skip to content

ci: drop the -- separator when forwarding args through pnpm run - #388

Merged
kraenhansen merged 1 commit into
mainfrom
kh/fix-pnpm-arg-forwarding
Aug 9, 2026
Merged

ci: drop the -- separator when forwarding args through pnpm run#388
kraenhansen merged 1 commit into
mainfrom
kh/fix-pnpm-arg-forwarding

Conversation

@kraenhansen

Copy link
Copy Markdown
Collaborator

The Android lane has been broken since the pnpm migration (#381). Surfaced by CI on #372, which is the first run to carry the Android 🤖 label since then.

Root cause

npm and pnpm disagree on run <script> -- <args>:

resulting command
npm run build -- --android ferric build --android
pnpm run build -- --android ferric build -- --android

npm swallows the separator and appends only the args; pnpm appends the separator verbatim. #381 rewrote these call sites mechanically (npm runpnpm run), so the -- started leaking into each script's own argv.

Impact

packages/ferric-example — hard failure. The script is ferric build, so it ran as ferric build -- --android. Commander reads -- as the options terminator, --android becomes a positional operand, and buildCommand declares only options and zero arguments:

error: too many arguments for 'build'. Expected 0 arguments but got 1.

--android is a perfectly valid option (androidTarget) — it just never gets parsed as one.

test:*:allTests — subtler. These scripts end in -- (required, so node --run forwards to the inner script), so the extra separator survives as a literal -- sitting ahead of --mode Release in the forwarded args, which then feeds concurrently's {@} placeholder.

Fix

Drop the separator at the three call sites in check.yml (two live, one commented-out iOS TODO). Verified against Node 24 / pnpm 10 that this reproduces exactly the argv these steps had under npm:

$ node --run inner -- --mode Release          → GOT: --mode Release     # `--` is required here
$ pnpm run outer_dash --mode Release          → GOT: --mode Release     # ✅ this PR
$ pnpm run outer_dash -- --mode Release       → GOT: -- --mode Release  # ❌ on main today

(where outer_dash is node --run inner -- , mirroring test:android:allTests)

And against the real CLI, reproducing the CI failure locally and confirming the fix parses:

$ ferric build -- --android --help
error: too many arguments for 'build'. Expected 0 arguments but got 2.

$ ferric build --android --help
Usage: ferric build [options]
  --android    Use all Android targets

Why this wasn't caught

test-android is gated to if: contains(github.event.pull_request.labels.*.name, 'Android 🤖') — added in c77f968 because the self-hosted runner was offline and the job would otherwise queue forever on main pushes (tracked in #379). So the Android lane hasn't run since #381 landed.

Test plan

  • This PR carries the Android 🤖 label, so Test app (Android) runs here: the "Build ferric-example for all architectures" step should get past the argument error, and the emulator step should then run in Release mode.

Note the iOS Release line is still commented out — that TODO is untouched beyond the separator.


Generated by Claude Code

npm and pnpm disagree on `npm/pnpm run <script> -- <args>`: npm swallows the
separator and appends only the args, pnpm appends the separator verbatim. The
migration in #381 rewrote these call sites mechanically, so the `--` started
leaking into the scripts' own argv.

For `packages/ferric-example` this is a hard failure — the script is
`ferric build`, so it ran as `ferric build -- --android`, commander read `--`
as the options terminator and `--android` became a positional operand of a
subcommand that takes none:

    error: too many arguments for 'build'. Expected 0 arguments but got 1.

For the `test:*:allTests` scripts (which end in `-- ` so `node --run` forwards
to the inner script) it is subtler: the extra separator survives as a literal
`--` in the forwarded args, ahead of `--mode Release`.

Dropping the separator restores exactly the argv these steps had under npm.

This only shows up on PRs labeled "Android 🤖": test-android is gated to those
while the self-hosted runner is offline (#379), so nothing had exercised the
Android lane since #381 landed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@kraenhansen kraenhansen added the Android 🤖 Anything related to the Android platform (Gradle, NDK, Android SDK) label Aug 9, 2026
@kraenhansen kraenhansen self-assigned this Aug 9, 2026
@kraenhansen
kraenhansen marked this pull request as ready for review August 9, 2026 19:43
@kraenhansen
kraenhansen merged commit 8a15546 into main Aug 9, 2026
15 checks passed
@kraenhansen
kraenhansen deleted the kh/fix-pnpm-arg-forwarding branch August 9, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Android 🤖 Anything related to the Android platform (Gradle, NDK, Android SDK)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant