Skip to content

fix: keep the installed runtime authoritative against context-aware shims - #25

Merged
zkochan merged 2 commits into
mainfrom
disable-runtime-shim
Aug 9, 2026
Merged

fix: keep the installed runtime authoritative against context-aware shims#25
zkochan merged 2 commits into
mainfrom
disable-runtime-shim

Conversation

@zkochan

@zkochan zkochan commented Aug 9, 2026

Copy link
Copy Markdown
Member

Summary

pnpm 12 links global runtime bins as context-aware shims: running node from $PNPM_HOME/bin inside a project switches to the version that project pins in devEngines.runtime, fetching it on demand. That defeats the runtime this action was asked to install — a matrix job asking for node@22 runs the repository's pinned version instead — and even when the two versions agree, pnpm materializes a second copy outside $PNPM_HOME.

This is exactly the shadowing the action already guards against for the install step with pnpm install --no-runtime; the shims reintroduced it for every step after the action.

Whenever the action installs a runtime, it now exports PNPM_CONFIG_GLOBAL_SHIMS with that runtime disabled:

PNPM_CONFIG_GLOBAL_SHIMS={"node":false}

The setting merges key-wise over pnpm's defaults, so runtimes the action did not install keep theirs. A value the workflow set itself always wins — checked under both names pnpm reads (PNPM_CONFIG_GLOBAL_SHIMS and pnpm_config_global_shims) — so opting back into the switching behaviour stays possible:

- uses: pnpm/setup@v2
  env:
    PNPM_CONFIG_GLOBAL_SHIMS: '{"node":"auto"}'
  with:
    runtime: node@22

Motivation

pnpm's own TS CI hit this the moment it moved to pnpm 12.0.0-rc.2: the Verify Node version step asserts pn node -v matches the matrix Node that this action provisioned, and it reported v26.7.0 (the repo's devEngines.runtime pin) instead of the requested v22.13.0https://github.com/pnpm/pnpm/actions/runs/31318748252/job/93258395237. Without this change the whole 22/24/26 matrix silently collapses onto one version, and that applies to every repository that pins a runtime in devEngines.runtime and sets a different one in CI.

Verification

Ran the built action end-to-end locally (dist/index.js with the runner's INPUT_* env encoding) against a workspace pinning devEngines.runtime node 20.19.0, with runtime: node@22:

  • GITHUB_ENV receives PNPM_CONFIG_GLOBAL_SHIMS={"node":false}.
  • With that value set, node --version in the project → v22.23.2 (the installed runtime).
  • Without it (control), node --version in the same project → v20.19.0 — the bug.
  • With pnpm_config_global_shims pre-set, the action logs that it is leaving the setting alone and writes nothing to GITHUB_ENV.

Two workflow jobs cover both directions: runtime-survives-context-shims asserts the installed runtime wins and the variable is exported, and context-shims-opt-out asserts a workflow-set value is untouched and pnpm still switches to the project's pin — the canary that keeps the first job from going vacuous if pnpm's defaults ever change.


Written by an agent (Claude Code, claude-opus-5).

Summary by CodeRabbit

  • New Features

    • Installed runtimes now remain active even when the project specifies a different runtime version.
    • Workflows can preserve existing pnpm shim settings to enable automatic project-based runtime switching.
  • Documentation

    • Added guidance explaining context-aware pnpm shims and how to opt into automatic switching.
    • Updated runtime input documentation to describe the new behavior and override options.
  • Tests

    • Added workflow coverage for runtime persistence and configurable shim behavior.

…hims

pnpm 12 links global runtime bins as context-aware shims: running `node`
from `$PNPM_HOME/bin` inside a project switches to the version that project
pins in `devEngines.runtime`, fetching it on demand. That defeats the
runtime this action was asked to install — a matrix job asking for
`node@22` runs the repository's pinned version instead — and even when the
two versions agree pnpm materializes a second copy outside `$PNPM_HOME`.

Whenever the action installs a runtime, export `PNPM_CONFIG_GLOBAL_SHIMS`
with that runtime disabled (`{"node":false}`). The setting merges key-wise
over pnpm's defaults, so the runtimes the action did not install keep
theirs. A value the workflow set itself always wins, under either of the
two names pnpm reads it from, so opting back into the switching behaviour
stays possible.

`pnpm install --no-runtime` already expresses this for the install step;
the shims reintroduced the shadowing for every step after it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@zkochan, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 56 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 84edf272-6220-42c2-acbf-844e4def2061

📥 Commits

Reviewing files that changed from the base of the PR and between a1f8bb6 and 5a7e6e9.

📒 Files selected for processing (2)
  • .github/workflows/test.yaml
  • src/install-runtime/index.ts
📝 Walkthrough

Walkthrough

The runtime setup action now controls pnpm context-aware global shims. It disables shimming for installed runtimes by default, preserves workflow-provided settings, documents the behavior, and adds Ubuntu workflow validation for both paths.

Changes

Runtime shim control

Layer / File(s) Summary
Configure runtime shims
src/install-runtime/index.ts
The action detects existing pnpm shim settings. If none exist, it exports PNPM_CONFIG_GLOBAL_SHIMS with the installed runtime disabled.
Document shim behavior
action.yml, README.md
The action metadata and README document the default behavior and the workflow override.
Validate runtime selection
.github/workflows/test.yaml
Ubuntu jobs verify that Node 22 remains active by default and that a workflow-provided shim setting is preserved.င်

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • pnpm/setup#1: Extends the same runtime installation module with environment handling.
  • pnpm/setup#2: Addresses conflicts between selected runtimes and project-defined settings through a different mechanism.
  • pnpm/setup#11: Modifies the same runtime and pnpm shim execution behavior.

Poem

A rabbit watched Node hop in line,
While pnpm shims were told, “Not mine.”
The chosen runtime stayed in place,
Unless workflows changed the race.
Tests and docs now guard the way. 🐇

🚥 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 clearly and concisely describes the main change: preventing context-aware shims from overriding the installed runtime.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch disable-runtime-shim

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/test.yaml:
- Around line 470-513: Add a counterpart to the context-shims-opt-out coverage
that sets lowercase pnpm_config_global_shims in the action environment, then
verify the uppercase PNPM_CONFIG_GLOBAL_SHIMS remains unexported and node
resolves to the package.json project runtime v20.19.0. Keep the existing
uppercase-variable test intact and mirror its assertions for the lowercase
workflow override contract.

In `@action.yml`:
- Around line 36-39: Update the workflow documentation in action.yml lines 36-39
and README.md line 115 to state that the action’s default
PNPM_CONFIG_GLOBAL_SHIMS export occurs only when neither supported shim variable
has been set by the workflow; make README step 3 explicitly conditional on both
supported variables being unset.

In `@src/install-runtime/index.ts`:
- Around line 72-79: Update keepInstalledRuntimeAuthoritative to detect
configured environment variables by checking whether process.env[envName] is
undefined, rather than relying on truthiness, so explicitly empty values are
preserved and no override is exported.
🪄 Autofix

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 Plus

Run ID: bb4f1a29-c4c4-4a87-bf3c-1f350d2e34c2

📥 Commits

Reviewing files that changed from the base of the PR and between ed0c46d and a1f8bb6.

⛔ Files ignored due to path filters (1)
  • dist/index.js is excluded by !**/dist/**
📒 Files selected for processing (4)
  • .github/workflows/test.yaml
  • README.md
  • action.yml
  • src/install-runtime/index.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Analyze (actions)
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-11T16:19:49.450Z
Learnt from: zkochan
Repo: pnpm/setup PR: 1
File: src/cache-restore/run.ts:35-35
Timestamp: 2026-05-11T16:19:49.450Z
Learning: When using `actions/exec` (`getExecOutput` / `exec`), it is valid for the `commandLine` option to include both the command and its arguments in a single string (e.g., `getExecOutput('pnpm store path --silent')`). The library tokenizes `commandLine` internally (via `argStringToArray()`), so this behaves like passing an equivalent command + args array (e.g., `getExecOutput('pnpm', ['store','path','--silent'])`). In code reviews, do not flag this as incorrect—this matches documented behavior and a production-tested pattern.

Applied to files:

  • src/install-runtime/index.ts
🪛 ast-grep (0.45.0)
src/install-runtime/index.ts

[warning] 1-1: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from 'child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🪛 zizmor (1.29.0)
.github/workflows/test.yaml

[warning] 435-435: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[warning] 478-478: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🔇 Additional comments (2)
src/install-runtime/index.ts (1)

1-15: LGTM!

.github/workflows/test.yaml (1)

427-469: LGTM!

Comment thread .github/workflows/test.yaml
Comment thread action.yml
Comment thread src/install-runtime/index.ts
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix: keep installed runtime authoritative over pnpm 12 context shims

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Disable pnpm 12 context-aware global shims for the runtime this action installs.
• Respect workflow-provided global shim settings, avoiding any overrides.
• Add CI coverage and docs explaining the shim behavior and opt-out mechanism.
Diagram

graph TD
A["Workflow job"] --> B["pnpm/setup action"] --> C["pnpm runtime set -g"] --> D["Export globalShims off"] --> E["Later steps run node"] --> F["Installed runtime"]
B --> G["Project devEngines.runtime"] --> E
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Write pnpm global config instead of exporting env
  • ➕ Would apply even if a later step clears environment variables
  • ➕ Matches a persistent “configure once” model
  • ➖ Persists beyond the action step/job scope and can leak into other tooling
  • ➖ Harder to respect workflow-level overrides cleanly
  • ➖ More risk across pnpm versions and runner images
2. Disable shims only when pnpm>=12 is detected
  • ➕ Minimizes behavior change for older pnpm versions
  • ➕ Reduces reliance on undocumented behavior for pnpm<12
  • ➖ Requires reliably detecting pnpm major version and handling prereleases
  • ➖ Still leaves a footgun when version detection fails
  • ➖ Current approach is safe even if pnpm ignores the setting
3. Avoid global runtime bins; expose runtime via explicit path outputs
  • ➕ Eliminates PATH/shim ambiguity entirely
  • ➕ Lets workflows choose exactly which binary to run
  • ➖ Breaks existing expectations that node/bun/deno are on PATH
  • ➖ Requires pervasive workflow changes and reduces ergonomics

Recommendation: Keep the current env-export approach. It’s job-scoped, disables only the shim for the runtime the action installed, and preserves workflow-provided configuration by checking both env var spellings pnpm honors.

Files changed (4) +133 / -2

Bug fix (1) +26 / -1
index.tsExport PNPM_CONFIG_GLOBAL_SHIMS to prevent runtime switching via pnpm shims +26/-1

Export PNPM_CONFIG_GLOBAL_SHIMS to prevent runtime switching via pnpm shims

• After successfully running 'pnpm runtime set', exports PNPM_CONFIG_GLOBAL_SHIMS={"<runtime>":false} to keep the installed runtime authoritative in subsequent steps. Detects both PNPM_CONFIG_GLOBAL_SHIMS and pnpm_config_global_shims and avoids overwriting either if already set by the workflow.

src/install-runtime/index.ts

Tests (1) +87 / -0
test.yamlAdd CI jobs covering pnpm 12 context-aware shim behavior +87/-0

Add CI jobs covering pnpm 12 context-aware shim behavior

• Adds two new workflow jobs: one asserting the action’s installed Node remains active inside a project with a conflicting devEngines.runtime pin, and one asserting that a workflow-provided PNPM_CONFIG_GLOBAL_SHIMS opt-out is not overwritten. These jobs act as a regression test and a canary if pnpm defaults change.

.github/workflows/test.yaml

Documentation (1) +15 / -1
README.mdDocument context-aware global shims and the opt-out escape hatch +15/-1

Document context-aware global shims and the opt-out escape hatch

• Updates the “How it works” section to note that the action exports PNPM_CONFIG_GLOBAL_SHIMS after installing a runtime. Adds a dedicated section explaining pnpm 12 context-aware shims, why they can break CI matrices, and how workflows can opt back into shim switching.

README.md

Other (1) +5 / -0
action.ymlClarify runtime input behavior regarding PNPM_CONFIG_GLOBAL_SHIMS +5/-0

Clarify runtime input behavior regarding PNPM_CONFIG_GLOBAL_SHIMS

• Extends the runtime input description to explain that the action disables pnpm’s context-aware shim for the installed runtime by exporting PNPM_CONFIG_GLOBAL_SHIMS, and that workflows can override by setting the variable themselves.

action.yml

pnpm reads the setting from PNPM_CONFIG_GLOBAL_SHIMS or
pnpm_config_global_shims, whichever comes first, so honouring only one of
them would let the action override a workflow that used the other. Drive
the opt-out job from a matrix over both names.

Also record why an empty value counts as unset: pnpm ignores an empty
value, so stepping aside for one would leave the shim enabled while
looking like the workflow had opted in.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zkochan
zkochan merged commit 84cb39b into main Aug 9, 2026
30 checks passed
zkochan added a commit to pnpm/pnpm that referenced this pull request Aug 9, 2026
pnpm 12 links global runtime bins as context-aware shims, so `node` inside
this repository resolved to the version pinned in `devEngines.runtime`
rather than the one the workflow asked for. The `Verify Node version` step
caught it as soon as CI moved to pnpm 12.0.0-rc.2: it reported v26.7.0
where the Node 22 matrix entry expects v22.13.0, and the whole 22/24/26
matrix would otherwise have collapsed onto a single version.

pnpm/setup#25 makes the action export `PNPM_CONFIG_GLOBAL_SHIMS` with the
runtime it installed disabled, restoring what `pnpm install --no-runtime`
already expressed for the install step. Pin every usage to the commit that
carries it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
zkochan added a commit to pnpm/pnpm that referenced this pull request Aug 9, 2026
pnpm 12 links global runtime bins as context-aware shims, so `node` inside
this repository resolved to the version pinned in `devEngines.runtime`
rather than the one the workflow asked for. The `Verify Node version` step
caught it as soon as CI moved to pnpm 12.0.0-rc.2: it reported v26.7.0
where the Node 22 matrix entry expects v22.13.0, and the whole 22/24/26
matrix would otherwise have collapsed onto a single version.

pnpm/setup#25 makes the action export `PNPM_CONFIG_GLOBAL_SHIMS` with the
runtime it installed disabled, restoring what `pnpm install --no-runtime`
already expressed for the install step. Pin every usage to the commit that
carries it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant