Skip to content

fix(phase4): use bundled node for Windows version probe - #83

Merged
gitcommit90 merged 1 commit into
mainfrom
fix/windows-version-probe-python
Aug 5, 2026
Merged

fix(phase4): use bundled node for Windows version probe#83
gitcommit90 merged 1 commit into
mainfrom
fix/windows-version-probe-python

Conversation

@gitcommit90

@gitcommit90 gitcommit90 commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Result

Fixes the sole remaining Windows candidate-dress-rehearsal failure. The exact candidate installed successfully and reached health, but Assert-DistroVersion called bare node; the host installer intentionally exposes Node only through /opt/1helm/node-current/bin/node.

Change

  • invoke the stable installed bundled-Node path for all four Windows version assertions
  • add focused regression coverage requiring that path

Evidence

  • failed run 30978297630 logged 1Helm v0.0.41 is running before failing only with node: command not found
  • the same installed host later invoked its bundled Node successfully during teardown
  • npm run typecheck passed
  • npm run build passed
  • npm test passed: 177 tests, 175 passed, 2 skipped, 0 failed
  • focused Phase 4 suite passed: 8/8
  • git diff --check passed

Summary by CodeRabbit

  • Bug Fixes
    • Improved Windows validation by using the application’s bundled Node.js runtime, ensuring version checks work even when Node.js is not installed globally.
  • Tests
    • Added acceptance coverage to verify that Windows validation uses the bundled runtime.

The Windows acceptance lane installs and runs 1Helm successfully, but its version assertion called bare node even though the Linux host contract keeps Node at /opt/1helm/node-current/bin/node. Use that stable bundled path and lock it into the Phase 4 regression contract.

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The Windows platform acceptance script now uses 1Helm’s bundled Node binary for WSL version checks. The Phase 4 acceptance test verifies that the bundled path appears in the validation script.

Changes

Windows validation

Layer / File(s) Summary
Use bundled Node in Windows validation
ops/platform-acceptance/windows.ps1, test/phase4-platform-acceptance.mjs
The WSL version assertion invokes /opt/1helm/node-current/bin/node. The acceptance test verifies this path.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the fix and verification, but it omits the required template sections for change type, release notes, and post-merge checks. Use the repository template headings and add the change type, release-notes status, and applicable post-merge checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: using the bundled Node executable for the Windows version probe.
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 fix/windows-version-probe-python

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: 1

🤖 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 `@test/phase4-platform-acceptance.mjs`:
- Line 215: Update the assertion in the phase 4 platform acceptance test to
match the actual version-probe command, including execution of
/opt/1helm/node-current/bin/node with the -p argument, rather than only matching
the path text. Ensure the test cannot pass when the probe uses bare node, either
by asserting the complete command or explicitly rejecting the bare-node form.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 44d0a45c-60f1-4bf1-80bd-e3d6e9f9e021

📥 Commits

Reviewing files that changed from the base of the PR and between f6f58a4 and 17f275e.

📒 Files selected for processing (2)
  • ops/platform-acceptance/windows.ps1
  • test/phase4-platform-acceptance.mjs

assert.match(windows, /apply-linux-release\.sh/);
assert.match(windows, /function Assert-DistroVersion/);
assert.equal((windows.match(/Assert-DistroVersion \$(?:Version|PreviousVersion)/g) || []).length, 4);
assert.match(windows, /\/opt\/1helm\/node-current\/bin\/node/);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Match the version-probe command, not only the path text.

Line 215 can match the path in ops/platform-acceptance/windows.ps1 Lines 55-57, even if the executable on Line 60 reverts to bare node. Match test "$(/opt/1helm/node-current/bin/node -p or also assert that the version probe does not use bare node.

🤖 Prompt for 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.

In `@test/phase4-platform-acceptance.mjs` at line 215, Update the assertion in the
phase 4 platform acceptance test to match the actual version-probe command,
including execution of /opt/1helm/node-current/bin/node with the -p argument,
rather than only matching the path text. Ensure the test cannot pass when the
probe uses bare node, either by asserting the complete command or explicitly
rejecting the bare-node form.

@gitcommit90
gitcommit90 merged commit 833a680 into main Aug 5, 2026
6 checks passed
@gitcommit90
gitcommit90 deleted the fix/windows-version-probe-python branch August 5, 2026 07:43
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