[js] remove npm dependency by using bazel for everything

[titusfortner]

There was one place in node.rake that still used system npm, this removes it so that bazel is used for everything.
Also we're hard coding a pnpm version that I don't think even gets read since it uses the version from aspect_rules_js

💥 What does this PR do?

• remove hard coded npm and pnpm references

🤖 AI assistance

• No substantial AI assistance used

💡 Additional Considerations

Updating dependencies when working on JS code requires:

bazel run -- @pnpm//:pnpm install --dir "$PWD" --lockfile-only

or

./go node:pin

[qodo-code-review]

Review Summary by Qodo

Replace npm with Bazel-managed pnpm for Node.js tooling
✨ Enhancement

Walkthroughs

Description

• Replace npm with pnpm for package linking via Bazel
• Remove hardcoded pnpm version from package.json
• Standardize on Bazel for all Node.js tooling

Diagram

flowchart LR
  A["npm link command"] -->|replaced with| B["Bazel pnpm execution"]
  C["Hardcoded pnpm version"] -->|removed| D["Bazel-managed version"]
  B --> E["Unified Bazel tooling"]
  D --> E

Loading

File Changes

1. rake_tasks/node.rake ✨ Enhancement +3/-4

Replace npm link with Bazel pnpm execution

• Updated :install task to use pnpm link instead of npm link
• Changed from direct shell command to Bazel execution of pnpm
• Modified to use File.expand_path for proper path handling
• Updated task description to reference pnpm instead of npm

rake_tasks/node.rake

---

2. package.json ⚙️ Configuration changes +0/-1

Remove hardcoded pnpm version specification

• Removed hardcoded packageManager field specifying pnpm@9.15.4
• Relies on Bazel-managed pnpm version instead

package.json

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. cmd_line built via shell 📘 Rule violation ⛨ Security

Description

On Windows, Bazel.execute builds a single shell command string (cmd_line) and runs it via
backticks, but its manual quoting only accounts for whitespace and " and does not robustly escape
cmd.exe metacharacters (e.g., &, |, <, >, (, ), ^), risking argument-splitting,
broken tasks, or unintended command execution when inputs contain special characters.

Code

rake_tasks/bazel.rb[R28-29]

+      quoted = cmd.map { |a| a.match?(/[\s"]/) ? %("#{a.gsub('"', '\\"')}") : a }
+      cmd_line = "#{quoted.join(' ')} 2>&1"

Evidence

Rule 15 requires safe argument handling in scripts, yet the Windows-specific branch constructs a
command string from cmd, appends 2>&1, and executes it via backticks, which means the shell
performs parsing. Because the new quoting predicate only checks for whitespace or ", arguments
containing cmd.exe metacharacters (for example Dir.pwd containing &) can be emitted unquoted
and then interpreted by cmd.exe as command separators/redirections, changing the intended command
structure and potentially executing additional commands.

rake_tasks/bazel.rb[28-30]
rake_tasks/bazel.rb[26-32]
rake_tasks/node.rake[42-45]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Bazel.execute` on Windows constructs a single command-line string (`cmd_line`) and executes it via backticks (shell parsing). The current manual quoting only covers whitespace and `"`, leaving `cmd.exe` metacharacters (e.g., `&`, `|`, `<`, `>`, `(`, `)`, `^`) unescaped/unquoted, which can alter parsing, break invocations, or allow unintended extra commands to run.

## Issue Context
This Windows code path still executes through the shell (backticks) and appends `2>&1`, so any unhandled `cmd.exe` metacharacters inside arguments can change how the command is interpreted. This method is used by multiple rake tasks, including ones that pass `Dir.pwd` as an argument (e.g., `node:pin`), so a working directory containing such characters can trigger failures or unexpected behavior.

## Fix Focus Areas
- rake_tasks/bazel.rb[28-32]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. pkg_dir unquoted on Windows ✓ Resolved 📘 Rule violation ☼ Reliability

Description

The new pnpm invocation passes an absolute --dir path that may contain spaces, but
Bazel.execute builds a Windows command string via cmd.join(' ') without quoting. This can break
the node:install task on Windows and violates the safe-argument-handling requirement for scripts.

Code

rake_tasks/node.rake[R117-118]

+  pkg_dir = File.expand_path('bazel-bin/javascript/selenium-webdriver/selenium-webdriver')
+  Bazel.execute('run', ['--', '--dir', pkg_dir, 'link', '--global'], '@pnpm//:pnpm')

Evidence

Rule 14 requires safe argument handling and quoting of paths in scripts. The change introduces
pkg_dir = File.expand_path(...) and passes it as a --dir argument, while the Windows execution
path in Bazel.execute concatenates arguments with spaces (cmd.join(' ')) without quoting, making
the new code fragile/incorrect when paths contain spaces.

rake_tasks/node.rake[117-118]
rake_tasks/bazel.rb[26-32]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`node:install` now computes `pkg_dir` via `File.expand_path(...)` and passes it to `Bazel.execute` as `--dir <path>`. On Windows, `Bazel.execute` assembles a single command line with `cmd.join(' ')` (no quoting), so any space-containing path will break argument parsing.

## Issue Context
This change introduces a new space-sensitive argument (`--dir`) that was not present previously.

## Fix Focus Areas
- rake_tasks/node.rake[117-118]
- rake_tasks/bazel.rb[26-32]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Unpinned pnpm version 🐞 Bug ☼ Reliability

Description

Removing the root packageManager field drops the repo’s explicit pnpm version pin, so contributors
using locally installed pnpm (allowed by CONTRIBUTING.md) can produce lockfile churn or incompatible
pnpm-lock.yaml formats. This undermines reproducibility of dependency updates across developer
environments.

Code

package.json[R1-5]

{
  "name": "selenium",
-  "packageManager": "pnpm@9.15.4",
  "workspaces": [
    "javascript/grid-ui",
    "javascript/selenium-webdriver"

Evidence

The repo’s lockfile format is explicitly versioned, and contributor docs permit using a locally
installed pnpm; without a packageManager pin, local pnpm versions can diverge and cause
lockfile-format or dependency-resolution churn.

pnpm-lock.yaml[1-7]
CONTRIBUTING.md[126-149]
package.json[1-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The PR removes the root `packageManager` pin (previously `pnpm@…`). Since the repo uses a pnpm lockfile with a specific lockfile format version, allowing unpinned local pnpm usage can create inconsistent lockfile updates and noisy diffs.

### Issue Context
- `pnpm-lock.yaml` declares `lockfileVersion: '9.0'`, indicating the lockfile format is tied to pnpm major behavior.
- `CONTRIBUTING.md` explicitly says contributors may use pnpm installed locally.

### Fix Focus Areas
- package.json[1-6]
- CONTRIBUTING.md[126-149]
- pnpm-lock.yaml[1-7]

### Suggested fixes (choose one)
1) Restore a `"packageManager": "pnpm@<desired>"` entry in the root `package.json` to keep local and CI/dev aligned.
2) If the intent is to forbid local pnpm usage, update `CONTRIBUTING.md` to remove/clarify the “pnpm installed on your local machine” path and direct contributors to Bazel-managed pnpm only.
3) (Optional) Add a short note near the JS dependency instructions about the expected pnpm major that matches the lockfile version to prevent accidental lockfile upgrades.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Persistent review updated to latest commit 9630c52