Skip to content

fix(#598,#519): use process.argv[1] + NVM env vars to resolve Pi CLI - #599

Merged
HenryLach merged 1 commit into
mainfrom
fix/598-resolvepicli-argv1
Jun 17, 2026
Merged

fix(#598,#519): use process.argv[1] + NVM env vars to resolve Pi CLI#599
HenryLach merged 1 commit into
mainfrom
fix/598-resolvepicli-argv1

Conversation

@HenryLach

Copy link
Copy Markdown
Owner

Closes #598. Closes #519.

When Taskplane runs as a Pi extension, process.argv[1] IS the absolute path to Pi's cli.js — set by Node from the parent process invocation. Trusting it directly is the single most reliable Pi-resolution strategy: it works for npm-global, mise, asdf, NVM (Windows + Unix), Nix, Bun-installed Pi, and any future install method we can't enumerate.

The bug

resolvePiCliPath previously ignored argv[1] entirely and relied on npm root -g plus a small static fallback list. That stranded:

The sibling resolver resolveTaskplanePackageFile already used the process.argv[1] trick (candidate 8, since #560 remediation). The same fix was somehow never applied to resolvePiCliPath, which is the function with the actual job of finding Pi.

The fix

Adds process.argv[1] as candidate 0, with two sanity guards:

  • endsWith('cli.js') — matches Pi's bin entrypoint per its package.json's "bin": { "pi": "dist/cli.js" }
  • existsSync() — handles stale state in test mocks or wrapper invocations

Both guards must pass or the resolver falls through to the existing base-directory search.

Defense in depth — two new env-var-driven bases

For environments where argv[1] is unavailable (test harnesses, indirect spawn through a wrapper, etc.):

Env var Set by Resolves to
NVM_SYMLINK NVM-for-Windows (points at active version) $NVM_SYMLINK\node_modules
NVM_BIN NVM-for-Unix ($NVM_BIN = active bin dir) dirname($NVM_BIN)/../lib/node_modules

Both env vars are set by NVM and inherited by child processes even when PATH is stripped, so they survive the exact failure mode that defeated npm root -g for galiling.

Tests

extensions/tests/path-resolver-pi-scope.test.ts — 6 new tests:

# Asserts
8.8 argv[1] → real cli.js resolves directly without any npm root
8.9 argv[1] takes precedence over npm-root-installed Pi (priority test)
8.10 Falls through when argv[1] doesn't end in cli.js (false-positive guard against test runners / wrappers)
8.11 Falls through when argv[1] ends in cli.js but file is missing (stale-state guard)
8.12 Resolves under $NVM_SYMLINK/node_modules (Windows NVM)
8.13 Resolves under dirname($NVM_BIN)/../lib/node_modules (Unix NVM)

The existing #560 regression tests (8.4–8.7) still pass unchanged, verifying back-compat for both @earendil-works and @mariozechner scopes and the both-scopes-named error message.

Validation

Gate Result
npm run typecheck ✅ pass
npm run lint ✅ 286 warnings / 671 infos — identical to main
npm run format:check ✅ pass (1 biome auto-format applied to new tests)
path-resolver-pi-scope.test.ts ✅ 10 / 10 pass (6 new for #598)
Full test suite ✅ 3,714 / 3,715 pass, 1 skipped — zero new failures
taskplane help / doctor ✅ pass

Apology to @chenxin-yan

PR #520 was closed on 2026-05-25 with the claim that the fix was already in main. I had misread path-resolver.ts — the process.argv[1] usage at line 235 was in the sibling resolveTaskplanePackageFile function, not in resolvePiCliPath. The fix the contributor actually proposed (which is also what this PR lands) was never applied to the function it was meant for. Reopening #519 with an apology comment after this merges.

Release plan

This is a blocker for fresh installs across NVM + non-canonical install layouts (which are common). Recommend shipping as a patch release v0.30.2 immediately after merge.

When Taskplane runs as a Pi extension, process.argv[1] IS the absolute
path to Pi's cli.js \u2014 set by Node from the parent process invocation.
Trusting it directly is the single most reliable Pi-resolution strategy:
it works for npm-global, mise, asdf, NVM (Windows + Unix), Nix,
Bun-installed Pi, and any future install method we can't enumerate.

resolvePiCliPath previously ignored argv[1] entirely and relied on
'npm root -g' plus a small static fallback list. That stranded:

  * #598 (itguy327 + galiling): NVM-on-Windows users whose child
    processes inherit a stripped PATH \u2014 npm root -g returns empty
    and the fallback list doesn't include NVM-versioned dirs.
  * #519 (chenxin-yan): Nix users whose Pi lives at /nix/store/...,
    not under any npm prefix the resolver knew about.

The fix adds process.argv[1] as candidate 0, with two sanity guards:
endsWith('cli.js') (matches Pi's bin entrypoint per its package.json's
"bin": { "pi": "dist/cli.js" }) AND existsSync() (handles stale state
in tests or wrappers). Both guards must pass or the resolver falls
through to the existing base-directory search.

Defense in depth: two new env-var-driven bases for NVM environments
where argv[1] is unavailable (e.g., test harnesses, indirect spawn):

  * NVM_SYMLINK\node_modules  (NVM-for-Windows, points at active version)
  * dirname(NVM_BIN)/../lib/node_modules  (NVM-for-Unix, same property)

Both env vars are set by NVM and inherited by child processes even when
PATH is stripped, so they survive the failure mode that defeated
npm root -g for galiling.

Tests (extensions/tests/path-resolver-pi-scope.test.ts):

  8.8  \u2014 argv[1] -> real cli.js resolves directly without any npm root
  8.9  \u2014 argv[1] takes precedence over npm-root-installed Pi (priority)
  8.10 \u2014 falls through when argv[1] doesn't end in cli.js (false-positive
         guard against wrappers/test runners)
  8.11 \u2014 falls through when argv[1] ends in cli.js but file is missing
  8.12 \u2014 resolves under $NVM_SYMLINK/node_modules (Windows NVM)
  8.13 \u2014 resolves under dirname($NVM_BIN)/../lib/node_modules (Unix NVM)

The existing #560 regression tests (8.4\u20138.7) still pass unchanged,
verifying back-compat for both @earendil-works and @mariozechner scopes
and the both-scopes-named error message.

Validation

  npm run typecheck            pass
  npm run lint                 286 warnings / 671 infos (identical to main)
  npm run format:check         pass (1 biome format applied to new tests)
  path-resolver tests          10/10 pass (6 new for #598)
  Full test suite              3714/3715 pass, 1 skipped, zero new failures
  taskplane help / doctor      pass

Closes #598
Closes #519

Apologies to @chenxin-yan: PR #520 was closed on 2026-05-25 with the
claim that the fix was already in main, but I had misread path-resolver.ts
\u2014 the process.argv[1] usage at line 235 was in the sibling
resolveTaskplanePackageFile function, NOT in resolvePiCliPath. The fix
the contributor actually proposed (which is also what this commit lands)
was never applied to the function it was meant for. Reopening #519.
@HenryLach
HenryLach enabled auto-merge June 17, 2026 19:41
@HenryLach
HenryLach merged commit ff4eefd into main Jun 17, 2026
1 check passed
@HenryLach
HenryLach deleted the fix/598-resolvepicli-argv1 branch June 17, 2026 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot find Pi CLI entrypoint Worker agent spawn fails when Pi is installed via Nix

1 participant