Skip to content

fix(cli): pick the first node >= 22 across all PATH entries - #189

Merged
0xKT merged 2 commits into
mainfrom
fix/find_node_min_version_all_path
Jul 23, 2026
Merged

0xKT merged 2 commits into
mainfrom
fix/find_node_min_version_all_path

Conversation

@arelchan

@arelchan arelchan commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

Change description

Fixes raven tui failing with a "Node version too low (found 20.20.1, need >= 22)" error on machines that do have a recent Node installed (e.g. Homebrew node 26), while raven agent works fine.

Two bugs in find_node() (raven/cli/tui_commands.py):

  1. No minimum-version filtering. It returned the first candidate whose --version merely parsed, regardless of _MIN_NODE_VERSION. The caller then rejected it if < 22 without ever asking for the next candidate -- so a too-old node in an earlier priority tier shadowed a valid newer one.
  2. PATH probed with shutil.which("node"), which returns only the first hit. A stale < 22 node earlier on PATH (an old /usr/local/bin/node, an nvm/asdf shim, etc.) hid a newer node later on PATH.

Together: a user with node 26 on PATH but a 20.x earlier on PATH got "node too low". raven agent was unaffected (no Node needed), which made it look like a TUI/streaming problem.

Fix

  • Enumerate every node on PATH (not just the first).
  • Return the first candidate that meets _MIN_NODE_VERSION, in priority order (RAVEN_NODE -> venv -> PATH -> private runtime).
  • Keep the highest below-minimum candidate so, when nothing qualifies, the caller still reports the real version (found 20.20.1, need >= 22) instead of a bare "not found".

Tests

Added to tests/test_cli_tui_bootstrap.py:

  • test_find_node_skips_below_min_and_scans_all_path -- a 20.20.1 earlier on PATH + a 26.5.0 later => find_node returns 26.5.0.
  • test_find_node_reports_below_min_when_nothing_qualifies -- only a 20.20.1 => returns it (so the caller can show the version).

Type of change

  • Bug fix
  • New feature
  • Document
  • Others

Checklists

Development

  • Lint rules pass locally (ruff check / ruff format)
  • Application changes have been tested thoroughly
  • Automated tests covering modified code pass

Code review

  • Pull request has a descriptive title and context useful to a reviewer

Related issues

Closes #198

Follow-up fix included (appended commit)

test_find_node_discovers_windows_private_runtime was failing on
non-Windows hosts (it mocked sys.platform="win32", but find_node keys its
Windows branches off os.name == "nt", so the mock never took effect). This
PR now also fixes it, replacing the earlier "flagged for a separate fix" note:

  • Add a _is_windows() seam that find_node consults for its venv, PATH, and
    private-runtime platform checks.
  • The test patches _is_windows instead of os.name -- patching os.name
    directly is not viable, since pathlib selects PosixPath/WindowsPath by
    os.name and would raise NotImplementedError on a POSIX host.

find_node returned the first node it could run without checking the
minimum version, and probed PATH via shutil.which which yields only the
first hit. So a stale < 22 node earlier on PATH (an old /usr/local/bin
node or a version-manager shim) shadowed a newer node later on PATH (e.g.
homebrew node 26): the caller received the old version and errored with
'node too low' even though a usable node was installed. raven agent
worked (no node needed) while raven tui failed, which made it look like a
tui/streaming issue.

enumerate every node on PATH (not just the first) and return the first
candidate that meets the >= 22 minimum, in priority order; keep the
highest below-minimum candidate so the caller still reports the real
version instead of a bare not-found.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@arelchan
arelchan force-pushed the fix/find_node_min_version_all_path branch from e1bf1b7 to 042dd9d Compare July 22, 2026 14:59
The test for find_node's Windows private-runtime layout was failing on
non-Windows hosts: it mocked sys.platform="win32", but find_node keys its
Windows branches off os.name == "nt", so the mock never took effect and
find_node fell through to the POSIX glob (node-*/bin/node), which cannot
match the Windows layout (node-*/node.exe). CI does not run this test in
its make test-python subset, so it stayed green.

Patching os.name directly is not an option: pathlib picks PosixPath vs
WindowsPath by os.name, so os.name="nt" makes every Path(...) in find_node
raise NotImplementedError on a POSIX host.

Add a _is_windows() seam that find_node consults for its venv, PATH, and
private-runtime platform checks, and have the test patch that seam. The
real os.name stays "posix" so pathlib keeps working, while the Windows
layout branch is still exercised.

Co-authored-by: Claude (claude-opus-4-8) <noreply@anthropic.com>
@0xKT

0xKT commented Jul 23, 2026

Copy link
Copy Markdown
Member

Appended a commit (as maintainer) fixing the pre-existing
test_find_node_discovers_windows_private_runtime failure this PR flagged --
the sys.platform vs os.name mock mismatch. The fix adds a _is_windows()
seam that find_node consults for its venv, PATH, and private-runtime checks,
and the test patches that seam instead of os.name (patching os.name
directly makes pathlib instantiate an unusable WindowsPath on POSIX hosts).

Also linked #198 (the node-selection bug this PR fixes) via Closes #198 in
the description, and folded your "flagged for a separate fix" note into a
"follow-up fix included" section. Thanks for the clear write-up -- it made
this easy to fold in.

@0xKT
0xKT self-requested a review July 23, 2026 08:49
@0xKT
0xKT merged commit 1745c14 into main Jul 23, 2026
8 checks passed
@0xKT
0xKT deleted the fix/find_node_min_version_all_path branch July 23, 2026 09:00
@0xKT 0xKT mentioned this pull request Jul 23, 2026
6 of 12 tasks
0xKT added a commit that referenced this pull request Jul 23, 2026
## Summary

Bump the package version from 0.1.8 to 0.1.9 (patch release). All five
PRs merged since v0.1.8 are fixes, no new features:

- #204 fix: add configurable llm_call_timeout to bound stalled LLM calls
- #199 fix(cli): make onboard readable on light-background terminals
- #189 fix(cli): pick the first node >= 22 across all PATH entries
- #196 fix(commitlint): unify commit/PR length to header-max-length
- #190 fix(tui): surface the real turn-failure detail, not just the code

## Type

- [ ] Fix
- [ ] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [x] Other

## Verification

- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [ ] User-facing docs or screenshots are updated when needed

Bump is limited to `pyproject.toml` and `uv.lock` (`uv lock` sync).
Commit message lint passes locally.

## Risk

- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes

Version-only change; no code or behavior change.

## Related Issues

N/A

Co-authored-by: Claude (claude-opus-4-8) <noreply@anthropic.com>
KevinHuSh pushed a commit to KevinHuSh/Raven that referenced this pull request Sep 14, 2026
…-AI#189)

## Change description

Fixes `raven tui` failing with a "Node version too low (found 20.20.1,
need >= 22)" error on machines that **do** have a recent Node installed
(e.g. Homebrew node 26), while `raven agent` works fine.

Two bugs in `find_node()` (`raven/cli/tui_commands.py`):

1. **No minimum-version filtering.** It returned the first candidate
whose `--version` merely parsed, regardless of `_MIN_NODE_VERSION`. The
caller then rejected it if `< 22` without ever asking for the next
candidate -- so a too-old node in an earlier priority tier shadowed a
valid newer one.
2. **PATH probed with `shutil.which("node")`,** which returns only the
*first* hit. A stale `< 22` node earlier on PATH (an old
`/usr/local/bin/node`, an nvm/asdf shim, etc.) hid a newer node later on
PATH.

Together: a user with node 26 on PATH but a 20.x earlier on PATH got
"node too low". `raven agent` was unaffected (no Node needed), which
made it look like a TUI/streaming problem.

### Fix

- Enumerate **every** `node` on PATH (not just the first).
- Return the first candidate that meets `_MIN_NODE_VERSION`, in priority
order (RAVEN_NODE -> venv -> PATH -> private runtime).
- Keep the highest below-minimum candidate so, when nothing qualifies,
the caller still reports the real version (`found 20.20.1, need >= 22`)
instead of a bare "not found".

### Tests

Added to `tests/test_cli_tui_bootstrap.py`:
- `test_find_node_skips_below_min_and_scans_all_path` -- a 20.20.1
earlier on PATH + a 26.5.0 later => `find_node` returns 26.5.0.
- `test_find_node_reports_below_min_when_nothing_qualifies` -- only a
20.20.1 => returns it (so the caller can show the version).

## Type of change
- [x] Bug fix
- [ ] New feature
- [ ] Document
- [ ] Others

## Checklists

### Development
- [x] Lint rules pass locally (`ruff check` / `ruff format`)
- [x] Application changes have been tested thoroughly
- [x] Automated tests covering modified code pass

### Code review
- [x] Pull request has a descriptive title and context useful to a
reviewer

## Related issues

Closes EverMind-AI#198

## Follow-up fix included (appended commit)

`test_find_node_discovers_windows_private_runtime` was failing on
non-Windows hosts (it mocked `sys.platform="win32"`, but `find_node`
keys its
Windows branches off `os.name == "nt"`, so the mock never took effect).
This
PR now also fixes it, replacing the earlier "flagged for a separate fix"
note:

- Add a `_is_windows()` seam that `find_node` consults for its venv,
PATH, and
  private-runtime platform checks.
- The test patches `_is_windows` instead of `os.name` -- patching
`os.name`
directly is not viable, since pathlib selects `PosixPath`/`WindowsPath`
by
  `os.name` and would raise `NotImplementedError` on a POSIX host.

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
KevinHuSh pushed a commit to KevinHuSh/Raven that referenced this pull request Sep 14, 2026
## Summary

Bump the package version from 0.1.8 to 0.1.9 (patch release). All five
PRs merged since v0.1.8 are fixes, no new features:

- EverMind-AI#204 fix: add configurable llm_call_timeout to bound stalled LLM calls
- EverMind-AI#199 fix(cli): make onboard readable on light-background terminals
- EverMind-AI#189 fix(cli): pick the first node >= 22 across all PATH entries
- EverMind-AI#196 fix(commitlint): unify commit/PR length to header-max-length
- EverMind-AI#190 fix(tui): surface the real turn-failure detail, not just the code

## Type

- [ ] Fix
- [ ] Feature
- [ ] Docs
- [ ] CI / tooling
- [ ] Refactor
- [x] Other

## Verification

- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [ ] User-facing docs or screenshots are updated when needed

Bump is limited to `pyproject.toml` and `uv.lock` (`uv lock` sync).
Commit message lint passes locally.

## Risk

- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes

Version-only change; no code or behavior change.

## Related Issues

N/A

Co-authored-by: Claude (claude-opus-4-8) <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.

bug(cli): raven tui reports node too low when a newer node exists later on PATH

2 participants