[briantcl1992]

Change

Make the node_link test fixture independent of the machine's installed Node.

The fixture's pc(false) means there is no user Node, but its original_path
also included /usr/bin:/bin. On a Linux machine with a recent /usr/bin/node,
the production installer correctly retained that existing Node and did not make
the link the test expected. The purported no-user-Node case therefore depended
on the host rather than the fixture.

Restrict only the fixture's original_path to its temporary userPath.
Deliberately supplied fixture Nodes still work. The shell keeps its normal
utility PATH; production install.sh and all assertions are unchanged.
This is a test-only correction, with no installed-app behavior change.

Validation

• Before the correction, the absent-user-Node case failed on this host even
  with test concurrency limited to two. Other cases in that file passed.
• bun test --max-concurrency=2 scripts/install-node-fallback.test.ts:
  29 passed, 0 failed, 80 expectations on the exact corrected file.
• Standard bun run check fast: workflow syntax, generated types/themes,
  typecheck and build passed. Unit tests: 9,165 passed, 36 skipped, 1 failed
  (9,202 tests across 591 files; 416,617 expectations). The source fingerprint
  remained unchanged. This is not a passing full local check.
• The single full-run failure was unrelated server/request-log.test.ts:
  neither follows nor writes a symlinked log path expected its temporary target
  to retain mode 0640. This validation service used umask 0077, so the
  fixture's writeFileSync(..., {mode: 0o640}) created it as 0600 before the
  logger ran. The log writer refused the symlink with ELOOP as intended.
  The unchanged file then passed 14/14 tests, 32,084 expectations, under
  umask 0027. No production permission or assertion was changed; no second
  full-suite pass is claimed. This existing fixture-mode assumption is disclosed,
  not included as an unrelated change in this PR.
• Tooling: pinned Bun 1.4.2; local Node 24.21.0 differs from CI's Node 22.
  Local test settings: UNIT_TEST_WORKERS=1 BUN_OPTIONS=--max-concurrency=2.
  The ordinary check script is unmodified; no tests are skipped by those settings.
• Integration/browser and native Windows installation were not run locally;
  there is no production, HTTP/WebSocket, UI or installer behavior change.
• The one-file correction received a separate independent source review.

No live Herdr session, application installation or user credentials are used
by the fixture validation. A local pass is not a claim that upstream CI passed.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: main
• File: .coderabbit.yaml
• Commit: 96da06f
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: cda58012-21b7-486d-8525-3d2a3df1d079

Commits

Reviewing files that changed from the base of the PR and between 96da06f and b3a7f4e.

Files selected for processing: 1

• scripts/install-node-fallback.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Summary

Summary by CodeRabbit

• Tests
  • Updated a test fixture so system-installed Node versions do not affect its lookup results.

The node_link test fixture now uses only its temporary user-path directory in original_path. A comment explains that this prevents a host Node executable from satisfying the fixture’s lookup.

Changes: Node fallback test

Layer / File(s)	Summary
Fixture PATH isolation scripts/install-node-fallback.test.ts	The fixture removes /usr/bin and /bin from original_path. A comment notes that this prevents a host Node executable from filling an absent fixture.

Priority: Low

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

Change: Other

Possibly related PRs

• devswha/ovis#2100: Changes the installer's node_link behavior and adds related tests.
• devswha/ovis#1908: Introduces the installer’s Node fallback and related test coverage.

Suggested reviewers: devswha

Merge Risk: Minimal · up to b3a7f

No actionable merge-blocking risk was identified in this fixture-only change.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title "test(install): isolate absent Node fixture from host PATH" matches the single change in the PR: the node_link test fixture's original_path is restricted to its temporary userPath so a host …
Description check		The description follows the repository template. It has a Change section that explains the problem (the fixture's original_path included /usr/bin:/bin, so a host Node could invalidate the absent-Node …
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "test(install): isolate absent Node fixture from host PATH" matches the single change in the PR: the node_link test fixture's original_path is restricted to its temporary userPath so a host Node cannot satisfy the absent-user-Node case. It is specific, concise, and follows conventional commit style.

Full details: Description check

Explanation

The description follows the repository template. It has a Change section that explains the problem (the fixture's original_path included /usr/bin:/bin, so a host Node could invalidate the absent-Node case), the resulting behavior, and the scope (test-only). It has a Validation section that lists the commands run, their results, and the checks not run. The template's reference to CONTRIBUTING.md and changelog.d is not addressed, and the description does not mention a changelog entry or whether one is needed for this test-only change. Those are minor gaps.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[devswha]

Merging b3a7f4e, thank you @briantcl1992! Test-only: the node_link fixture for 'no user Node' put /usr/bin:/bin on original_path, so on a host with a recent /usr/bin/node the installer (correctly) kept that Node and the test failed. The fixture's original_path is now only its own temporary user path; install.sh and every assertion are unchanged. Reviewed by the merge pass: one line in scripts/install-node-fallback.test.ts, safe for fork CI; node_link reads Node only from original_path, the shell keeps its own PATH for utilities.