test: fix environment sensitivity in resolveNpmCommandInvocation test by medns · Pull Request #83405 · openclaw/openclaw

medns · 2026-05-18T03:13:40Z

Summary

Problem: The resolveNpmCommandInvocation test "uses the platform npm command when npm_execpath is missing" was failing locally because it implicitly read the npm_execpath environment variable injected by the local npm test runner.
Why it matters: It causes local test failures for contributors running npm test, making the test suite environment-sensitive.
What changed: Explicitly passed npmExecPath: "" to the function call in the test to bypass the environment variable fallback.
What did NOT change (scope boundary): The actual implementation of resolveNpmCommandInvocation is unchanged.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

This PR fixes a bug or regression

Real behavior proof (required for external PRs)

Behavior or issue addressed: Local test failure in test/openclaw-npm-release-check.test.ts due to environment sensitivity.
Real environment tested: Windows 10, Node.js, running locally via PowerShell.
Exact steps or command run after this patch: npm test test/openclaw-npm-release-check.test.ts
Evidence after fix:
Terminal output:

> [email protected] test
> node scripts/test-projects.mjs test/openclaw-npm-release-check.test.ts

[test] starting test/vitest/vitest.tooling.config.ts

 RUN  v4.1.6 D:/openclaw

 Test Files  1 passed (1)
      Tests  57 passed (57)
   Start at  11:08:50
   Duration  1.02s

Observed result after fix: The test suite passes successfully without being affected by the local npm_execpath environment variable.
What was not tested: Other platforms (Linux/macOS) were not manually tested, but the fix is platform-agnostic.

Root Cause (if applicable)

Root cause: The test did not explicitly provide an npmExecPath argument, causing the function to fall back to process.env.npm_execpath, which is populated by the npm test runner.
Missing detection / guardrail: The test was likely written and verified in an environment where npm_execpath was not set, or run via a different runner.
Contributing context (if known): N/A

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: test/openclaw-npm-release-check.test.ts
Scenario the test should lock in: The function should correctly fall back to the platform npm command when npmExecPath is explicitly empty.
Why this is the smallest reliable guardrail: It directly tests the fallback logic in isolation.
Existing test that already covers this (if any): The modified test itself.
If no new test is added, why not: N/A

User-visible / Behavior Changes

None.

Diagram (if applicable)

N/A

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: Windows 10
Runtime/container: Node.js
Model/provider: N/A
Integration/channel (if any): N/A
Relevant config (redacted): N/A

Steps

Run npm test test/openclaw-npm-release-check.test.ts locally.

Expected

Test passes.

Actual

Test passes.

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

Verified scenarios: Running the specific test file locally via npm test.
Edge cases checked: N/A
What you did not verify: Full CI suite, as this is a localized test fix.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No

Risks and Mitigations

None.

clawsweeper · 2026-05-18T03:14:43Z

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
The PR changes one release-check test to pass an explicit empty npmExecPath when asserting the Windows platform npm fallback.

Reproducibility: yes. by source inspection: current main omits npmExecPath in the fallback test while the helper reads process.env.npm_execpath when that parameter is omitted. I did not run the test because this review was required to keep the checkout read-only.

PR rating
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Summary: This is a small, well-scoped test fix with sufficient terminal proof and no actionable findings.

Rank-up moves:

none

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Real behavior proof
Sufficient (terminal): The PR body includes after-fix Windows PowerShell terminal output for the focused test file showing 57 tests passing, and the inspected current diff still matches that one-line test isolation change.

Next step before merge
No repair lane is needed because the open PR already contains the focused fix and has no actionable review finding.

Security
Cleared: The diff changes only one test assertion and does not touch runtime command execution, dependencies, workflows, secrets, or package resolution.

Review details

Best possible solution:

Land the one-line test isolation so the fallback assertion is deterministic across npm- and pnpm-run test environments.

Do we have a high-confidence way to reproduce the issue?

Yes, by source inspection: current main omits npmExecPath in the fallback test while the helper reads process.env.npm_execpath when that parameter is omitted. I did not run the test because this review was required to keep the checkout read-only.

Is this the best way to solve the issue?

Yes. The PR uses the helper's existing npmExecPath injection seam to isolate the test from ambient environment without changing production release-check command resolution.

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix Windows PowerShell terminal output for the focused test file showing 57 tests passing, and the inspected current diff still matches that one-line test isolation change.

Label justifications:

P3: This is a low-risk test-infrastructure fix for local environment sensitivity with no runtime product impact.
rating: 🐚 platinum hermit: Current PR rating is 🐚 platinum hermit because proof is 🐚 platinum hermit, patch quality is 🐚 platinum hermit, and This is a small, well-scoped test fix with sufficient terminal proof and no actionable findings.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix Windows PowerShell terminal output for the focused test file showing 57 tests passing, and the inspected current diff still matches that one-line test isolation change.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix Windows PowerShell terminal output for the focused test file showing 57 tests passing, and the inspected current diff still matches that one-line test isolation change.

What I checked:

Current main test is environment-sensitive: On current main, the fallback assertion calls resolveNpmCommandInvocation({ platform: "win32" }), so it can inherit ambient process.env.npm_execpath. (test/openclaw-npm-release-check.test.ts:309, 4399eee6e0e4)
Helper already has the injection seam: The helper uses params.npmExecPath ?? process.env.npm_execpath and falls back to the platform npm command when the effective value is empty or not an npm executable path. (scripts/openclaw-npm-release-check.ts:447, 4399eee6e0e4)
PR diff is a one-line test isolation change: The current PR head changes only the affected assertion to pass npmExecPath: "", leaving production command resolution unchanged. (test/openclaw-npm-release-check.test.ts:309, b2c2e9a6948f)
Feature history provenance: The release-check npm invocation helper and the affected fallback tests appear to have been introduced in the npm preflight fix commit. (scripts/openclaw-npm-release-check.ts:440, 36d6ba55e387)
Code ownership signal: The release-check script is owned by the release-manager CODEOWNERS team, which is the right review route for this release-path test surface. (.github/CODEOWNERS:57, 4399eee6e0e4)
Formatting sanity: The PR diff has no whitespace errors under git diff --check. (test/openclaw-npm-release-check.test.ts:309, b2c2e9a6948f)

Likely related people:

TheRipper: Commit 36d6ba5 added resolveNpmCommandInvocation and the affected fallback test coverage. (role: introduced behavior; confidence: high; commits: 36d6ba55e387; files: scripts/openclaw-npm-release-check.ts, test/openclaw-npm-release-check.test.ts)
Peter Steinberger: History for the release-check files shows repeated recent work by this author, including the latest release timeout expectation alignment in this surface. (role: recent area contributor; confidence: medium; commits: a185ca283a74; files: scripts/openclaw-npm-release-check.ts, test/openclaw-npm-release-check.test.ts)
openclaw/openclaw-release-managers: .github/CODEOWNERS assigns scripts/openclaw-npm-release-check.ts to this release-path owner team. (role: CODEOWNERS routing owner; confidence: high; files: .github/CODEOWNERS, scripts/openclaw-npm-release-check.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 4399eee6e0e4.

Copilot

Pull request overview

This PR makes a release-check unit test deterministic by preventing resolveNpmCommandInvocation from falling back to the caller’s process.env.npm_execpath during the Windows platform fallback assertion.

Changes:

Passes npmExecPath: "" explicitly in the affected test case.
Keeps production implementation unchanged.

clawsweeper · 2026-05-21T19:59:51Z

ClawSweeper PR egg

✨ Hatched: 🌱 uncommon Clockwork Lint Imp

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🌱 uncommon.
Trait: keeps receipts.
Image traits: location workflow harbor; accessory rollback rope; palette coral, mint, and warm cream; mood watchful; pose sitting proudly on a smooth stone; shell translucent glimmer shell; lighting warm desk-lamp glow; background miniature CI buoys.
Share on X: post this hatch
Copy: My PR egg hatched a 🌱 uncommon Clockwork Lint Imp in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

odysseus0 · 2026-05-21T20:15:54Z

Merged via squash.

Prepared head SHA: b2c2e9a6948fdc17bc925a0147f57087cbfe4190
Merge commit: faf96ff99bd46cae5af9098b2d209903a7bb9d01

Thanks @medns!

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

@odysseus0

…openclaw#83405) Merged via squash. Prepared head SHA: b2c2e9a Co-authored-by: medns <[email protected]> Co-authored-by: odysseus0 <[email protected]> Reviewed-by: @odysseus0

Copilot AI review requested due to automatic review settings May 18, 2026 03:13

openclaw-barnacle Bot added size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels May 18, 2026

Copilot started reviewing on behalf of medns May 18, 2026 03:14 View session

Copilot AI reviewed May 18, 2026

View reviewed changes

clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels May 18, 2026

odysseus0 force-pushed the fix-npm-release-check-test branch from e82a69e to b941562 Compare May 21, 2026 19:52

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 21, 2026

clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 21, 2026

test: fix environment sensitivity in resolveNpmCommandInvocation test

b2c2e9a

odysseus0 force-pushed the fix-npm-release-check-test branch from b941562 to b2c2e9a Compare May 21, 2026 20:09

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 21, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 21, 2026

odysseus0 merged commit faf96ff into openclaw:main May 21, 2026
85 checks passed

github-actions Bot mentioned this pull request May 21, 2026

📡 Upstream Digest — 2026-05-21 21:22 UTC curtismercier/openclaw-mods#911

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

test: fix environment sensitivity in resolveNpmCommandInvocation test#83405

test: fix environment sensitivity in resolveNpmCommandInvocation test#83405
odysseus0 merged 1 commit into
openclaw:mainfrom
medns:fix-npm-release-check-test

medns commented May 18, 2026

Uh oh!

clawsweeper Bot commented May 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

clawsweeper Bot commented May 21, 2026

Uh oh!

Uh oh!

odysseus0 commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

medns commented May 18, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Real behavior proof (required for external PRs)

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

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

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

clawsweeper Bot commented May 21, 2026

Hatch command

Uh oh!

Uh oh!

odysseus0 commented May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

clawsweeper Bot commented May 18, 2026 •

edited

Loading