Skip to content

fix(cli): fail fast on unsupported onboarding hosts#1461

Closed
ChunkyMonkey11 wants to merge 20 commits intoNVIDIA:mainfrom
ChunkyMonkey11:feat/next-contribution-2026-04-02
Closed

fix(cli): fail fast on unsupported onboarding hosts#1461
ChunkyMonkey11 wants to merge 20 commits intoNVIDIA:mainfrom
ChunkyMonkey11:feat/next-contribution-2026-04-02

Conversation

@ChunkyMonkey11
Copy link
Copy Markdown
Contributor

@ChunkyMonkey11 ChunkyMonkey11 commented Apr 3, 2026

Summary

  • Add explicit host OS support classification for onboarding preflight.
  • Fail fast when host support status is error.
  • Keep onboarding non-blocking for warning hosts while surfacing clear guidance.

Root Cause

Onboarding did not gate early on unsupported host OS/platform combinations, which could lead users into later-stage failures after expensive setup steps.

Changes

  • Added src/lib/host-support.ts with Linux/macOS/platform classification helpers.
  • Added CJS shim bin/lib/host-support.js.
  • Wired checkHostSupport() into bin/lib/onboard.js preflight.
  • Added unit coverage in src/lib/host-support.test.ts.
  • Added onboarding regression test in test/onboard.test.js to verify fail-fast behavior before Docker checks.
  • Simplified the pre-20 EOL condition in src/lib/host-support.ts from a redundant clause to major < 20.

Validation

  • npm test -- src/lib/host-support.test.ts test/onboard.test.js
  • Pre-commit and pre-push hooks passed during commit/push in this environment.

Signed-off-by: Revant Patel revant.h.patel@gmail.com

Summary by CodeRabbit

  • New Features

    • Onboarding now evaluates host OS support up front, prints clear success/warning/error messages, and exits early on unsupported hosts to avoid running later checks.
    • Added a lightweight compatibility shim so the host-support logic is consumable in more environments.
    • Improved Linux distro lifecycle and macOS version detection with clearer, actionable warnings.
  • Tests

    • Added tests for OS-release parsing, Linux/macOS classification, host-support checks, and onboarding fail-fast behavior.

@ChunkyMonkey11 ChunkyMonkey11 marked this pull request as ready for review April 3, 2026 22:09
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 3, 2026

📝 Walkthrough

Walkthrough

Adds host-support detection (Linux/macOS) with parsing and classification, a CommonJS shim, unit and integration tests, and integrates host capability checks into the onboarding preflight to log status and fail fast on unsupported hosts.

Changes

Cohort / File(s) Summary
Host Support Source
src/lib/host-support.ts
New TypeScript module exporting types and checkHostSupport(); platform branching for linux/darwin/others, /etc/os-release parsing, macOS sw_vers handling, classification helpers, and injectable I/O for tests.
Host Support Tests
src/lib/host-support.test.ts
New Vitest suite validating parseOsRelease, classifyLinuxHost, classifyMacosHost, and checkHostSupport() behavior via injected read/get implementations and error paths.
Host Support Shim
bin/lib/host-support.js
New CommonJS shim that re-exports the compiled runtime at ../../dist/lib/host-support via module.exports = require(...).
Onboard Integration
bin/lib/onboard.js
preflight() now calls checkHostSupport() early; logs ok/warning to stdout, logs errors to stderr and invokes process.exit(1) on error, preventing later checks from running on unsupported hosts.
Integration Test
test/onboard.test.js
New test spawns bin/lib/onboard.js, stubs bin/lib/host-support.js to force an unsupported-host error, asserts the child exits with code 1 and that downstream actions (e.g., Docker invocation) do not run.

Sequence Diagram(s)

sequenceDiagram
  participant User as User
  participant Onboard as Onboard (preflight)
  participant HostSupport as HostSupport
  participant OS as OS/FileSystem
  participant Docker as DockerChecks

  User->>Onboard: run preflight
  Onboard->>HostSupport: checkHostSupport(opts)
  HostSupport->>OS: read /etc/os-release or run sw_vers
  OS-->>HostSupport: return version info or error
  HostSupport-->>Onboard: HostSupportResult (ok|warning|error)
  alt status == "error"
    Onboard->>Onboard: log to stderr
    Onboard-->>User: exit(1)
  else
    Onboard->>Onboard: log to stdout
    Onboard->>Docker: run docker & other checks
    Docker-->>Onboard: continue startup
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I sniff the OS with twitchy nose,
I parse the lines where version info grows.
A shim I call, a test I bead,
Preflight hops — decide with speed.
If support fails, I bound and flee.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately describes the main change: adding host OS support checks that fail fast during onboarding when unsupported hosts are detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@ChunkyMonkey11 ChunkyMonkey11 changed the title [codex] fix(cli): fail fast on unsupported onboarding hosts fix(cli): fail fast on unsupported onboarding hosts Apr 3, 2026
@ChunkyMonkey11
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 3, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ChunkyMonkey11
Copy link
Copy Markdown
Contributor Author

@cv when you have a moment, could you please approve workflows for PR #1461 so checks can run? Thanks!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/lib/host-support.ts (1)

143-151: Consider simplifying the EOL condition.

The condition major < 20 || (major === 18 && minor <= 4) is logically redundant — major < 20 already covers all 18.x versions. The second clause (major === 18 && minor <= 4) is a subset of major < 20 and never evaluated independently.

If the intent is to mark all pre-20.x as EOL, major < 20 alone suffices. If only specific 18.x versions should be EOL, the condition needs restructuring.

Simplified condition (if all pre-20.x is intended as EOL)
-  if (major < 20 || (major === 18 && minor <= 4)) {
+  if (major < 20) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/host-support.ts` around lines 143 - 151, The EOL check uses a
redundant clause: simplify the if-condition to remove the never-reached subset
and make intent explicit; replace the current `if (major < 20 || (major === 18
&& minor <= 4))` logic with either a single `major < 20` when all pre-20.x
releases are EOL or, if only specific 18.x minors should be EOL, change it to a
focused check like `major === 18 && minor <= 4`; update the branch that returns
the object containing os, version (normalized), status, code, and message
accordingly so the condition accurately reflects the intended EOL policy for
variables major, minor, and normalized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/host-support.ts`:
- Around line 143-151: The EOL check uses a redundant clause: simplify the
if-condition to remove the never-reached subset and make intent explicit;
replace the current `if (major < 20 || (major === 18 && minor <= 4))` logic with
either a single `major < 20` when all pre-20.x releases are EOL or, if only
specific 18.x minors should be EOL, change it to a focused check like `major ===
18 && minor <= 4`; update the branch that returns the object containing os,
version (normalized), status, code, and message accordingly so the condition
accurately reflects the intended EOL policy for variables major, minor, and
normalized.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 98c1c43d-3aa0-418e-bfd0-40775b730070

📥 Commits

Reviewing files that changed from the base of the PR and between b1bb01c and 9c10484.

📒 Files selected for processing (5)
  • bin/lib/host-support.js
  • bin/lib/onboard.js
  • src/lib/host-support.test.ts
  • src/lib/host-support.ts
  • test/onboard.test.js

@ChunkyMonkey11
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 3, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@wscurran wscurran added Getting Started Use this label to identify setup, installation, or onboarding issues. priority: medium Issue that should be addressed in upcoming releases NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). fix labels Apr 4, 2026
@wscurran
Copy link
Copy Markdown
Contributor

wscurran commented Apr 4, 2026

✨ Thanks for submitting this pull request, which proposes a way to fail fast during onboarding when the host OS is unsupported, improving the setup experience.

@ChunkyMonkey11
Copy link
Copy Markdown
Contributor Author

Hi @wscurran, quick update: I merged the latest upstream/main into this branch and resolved the onboarding preflight conflict. The branch is now up to date and ready for commit/review on your side.

@ChunkyMonkey11
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 5, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ChunkyMonkey11
Copy link
Copy Markdown
Contributor Author

@cv could you please review this PR when you have a moment? I’ve addressed the latest feedback and pushed updates. Thank you.

@ChunkyMonkey11
Copy link
Copy Markdown
Contributor Author

@coderabbitai resume

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 7, 2026

✅ Actions performed

Reviews resumed.

ChunkyMonkey11 and others added 2 commits April 7, 2026 12:57
macOS (Apple Silicon and Intel) is a documented supported platform with
Colima, Docker Desktop, and Podman runtimes. The original implementation
classified all macOS versions as warning/UNSUPPORTED_OS, contradicting
the project documentation.

Treat macOS 13 (Ventura) and later as supported, and older versions as
NEAR_EOL warnings. Add dedicated classifyMacosHost test coverage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/lib/host-support.ts (1)

123-159: Centralize Ubuntu support policy to reduce drift.

The support matrix is currently spread across conditionals. A small table/set-based policy would be easier to update when new LTS releases are added and lowers maintenance risk.

Refactor sketch
+const SUPPORTED_UBUNTU_LTS = new Set(["22.04", "24.04"]);
+const NEAR_EOL_UBUNTU = new Set(["20.04"]);
+
 export function classifyLinuxHost(id: string, versionId: string): HostSupportResult {
   ...
   const { major, minor } = parsed;
   const normalized = `${major}.${minor.toString().padStart(2, "0")}`;
 
-  if ((major === 24 && minor === 4) || (major === 22 && minor === 4)) {
+  if (SUPPORTED_UBUNTU_LTS.has(normalized)) {
     return { ...status: "ok", code: "SUPPORTED", ... };
   }
 
-  if (major === 20 && minor === 4) {
+  if (NEAR_EOL_UBUNTU.has(normalized)) {
     return { ...status: "warning", code: "NEAR_EOL", ... };
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/host-support.ts` around lines 123 - 159, Replace the scattered
if-blocks that check major/minor with a centralized policy lookup: create a
UBUNTU_SUPPORT_POLICY map or sets keyed by normalized version strings (e.g.,
"24.04", "22.04") and a NEAR_EOL set for "20.04", and implement a helper (e.g.,
getUbuntuSupportFor(major, minor, normalized)) that returns the same shape {os,
version, status, code, message} based on the map/sets and a single rule for EOL
(major < 20 => code "EOL") and default unsupported case; update the existing
branch that currently uses major, minor, normalized to call that helper so the
version-to-status mapping is defined in one place and easier to extend.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/host-support.ts`:
- Around line 196-202: The returned object for older macOS should block rather
than warn: change status from "warning" to "error" for the branch that returns {
os: "macos", version, status: "warning", code: "NEAR_EOL", ... } (use the same
variables `major` and `version` and keep `code: "NEAR_EOL"`), and update the
user-facing message to indicate unsupported macOS version and that Docker
Desktop and Colima require macOS 14 (adjust the comment that previously claimed
compatibility to reflect macOS 14 as the effective minimum for Docker Desktop
and Colima, while Podman CLI requires macOS 13). Ensure the logic enforces error
for macOS <13 if you intend to block those hosts, or change the threshold to 14
if the requirement is to block <14; update the message text accordingly to
reference the new minimum.

---

Nitpick comments:
In `@src/lib/host-support.ts`:
- Around line 123-159: Replace the scattered if-blocks that check major/minor
with a centralized policy lookup: create a UBUNTU_SUPPORT_POLICY map or sets
keyed by normalized version strings (e.g., "24.04", "22.04") and a NEAR_EOL set
for "20.04", and implement a helper (e.g., getUbuntuSupportFor(major, minor,
normalized)) that returns the same shape {os, version, status, code, message}
based on the map/sets and a single rule for EOL (major < 20 => code "EOL") and
default unsupported case; update the existing branch that currently uses major,
minor, normalized to call that helper so the version-to-status mapping is
defined in one place and easier to extend.
🪄 Autofix (Beta)

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: fbe06f3a-5e31-49ce-865b-d9ef6adc9ff3

📥 Commits

Reviewing files that changed from the base of the PR and between c4180a4 and c0a7f4b.

📒 Files selected for processing (2)
  • src/lib/host-support.test.ts
  • src/lib/host-support.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/host-support.test.ts

Comment on lines +196 to +202
return {
os: "macos",
version,
status: "warning",
code: "NEAR_EOL",
message: `macOS ${major} detected: older macOS version; upgrade is recommended.`,
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

As of April 2026, what are the minimum supported macOS versions for Docker Desktop, Colima, and Podman (machine/client tooling)?

💡 Result:

Minimum supported macOS versions (as of April 2026)

Tool Minimum supported macOS Source
Docker Desktop macOS 14 (Sonoma) (minimum to install or update current Docker Desktop) Docker Desktop release notes state the minimum OS to install/update is macOS Sonoma (14) or later. [1]
Colima No explicit project-wide minimum published; Homebrew bottles are provided for macOS Sonoma (14) and newer (Sonoma/Sequoia/Tahoe on Apple Silicon; Sonoma on Intel) Homebrew’s Colima formula shows bottle support only for Sonoma+ (and newer on Apple Silicon). [2]
Podman (CLI + podman machine) macOS 13 (Ventura) (Homebrew formula requirement) Homebrew’s Podman formula lists “Requires: macOS >= 13”. [3]
Podman Desktop (client UI tooling) macOS 12 (Monterey) (Homebrew cask requirement) Homebrew’s Podman Desktop cask lists “Requirements: macOS >= 12”. [4]

Notes

  • Docker also documents a rolling support policy of “current and two previous major macOS releases.” [5] (In April 2026, that still results in a minimum of macOS 14 if the current release is newer than 15.)

Sources: [1] (docs.docker.com) [2] (formulae.brew.sh) [3] (formulae.brew.sh) [4] (formulae.brew.sh) [5] (docs.docker.com)

Citations:


🏁 Script executed:

# Read the relevant sections of the file to verify the reviewer's claims
head -202 src/lib/host-support.ts | tail -40 | cat -n

Repository: NVIDIA/NemoClaw

Length of output: 1347


Unsupported macOS versions must fail fast with error status.

Line 196 returns status: "warning" for macOS <13, allowing preflight to continue. However, the code comment (lines 22–23) claims compatibility with Docker Desktop and Colima. According to current tool requirements (April 2026):

  • Docker Desktop requires macOS 14 minimum
  • Colima requires macOS 14 minimum
  • Podman CLI requires macOS 13 minimum

This means the code comment is inaccurate, and macOS <13 should return error (not warning) to block execution on unsupported hosts. Additionally, the threshold and comment should be updated to reflect that Docker Desktop and Colima, both listed as supported runtimes, require macOS 14 as the effective minimum for full compatibility.

Proposed fix
-  // macOS 13 (Ventura) and later are supported — matches documented
-  // compatibility for Colima, Docker Desktop, and Podman runtimes.
-  if (major >= 13) {
+  // macOS 14 (Sonoma) and later are supported — matches documented
+  // compatibility for Docker Desktop and Colima. Podman also works on macOS 13.
+  if (major >= 14) {
     return {
       os: "macos",
       version,
       status: "ok",
       code: "SUPPORTED",
       message: `macOS ${major} detected: supported.`,
     };
   }
 
   return {
     os: "macos",
     version,
     status: "error",
-    code: "NEAR_EOL",
-    message: `macOS ${major} detected: older macOS version; upgrade is recommended.`,
+    code: "UNSUPPORTED_OS",
+    message: `macOS ${major} detected: unsupported host operating system version for NemoClaw onboarding.`,
   };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/host-support.ts` around lines 196 - 202, The returned object for
older macOS should block rather than warn: change status from "warning" to
"error" for the branch that returns { os: "macos", version, status: "warning",
code: "NEAR_EOL", ... } (use the same variables `major` and `version` and keep
`code: "NEAR_EOL"`), and update the user-facing message to indicate unsupported
macOS version and that Docker Desktop and Colima require macOS 14 (adjust the
comment that previously claimed compatibility to reflect macOS 14 as the
effective minimum for Docker Desktop and Colima, while Podman CLI requires macOS
13). Ensure the logic enforces error for macOS <13 if you intend to block those
hosts, or change the threshold to 14 if the requirement is to block <14; update
the message text accordingly to reference the new minimum.

@cv
Copy link
Copy Markdown
Contributor

cv commented Apr 8, 2026

Holding off on this while we get our supported/unsupported-warn/unsupported-err decisions from above. I'll close this for now, and we'll implement when things are a bit more stable.

@cv cv closed this Apr 8, 2026
@ChunkyMonkey11
Copy link
Copy Markdown
Contributor Author

Holding off on this while we get our supported/unsupported-warn/unsupported-err decisions from above. I'll close this for now, and we'll implement when things are a bit more stable.

Please let me know when it is ready to re pull again thank you @cv.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Getting Started Use this label to identify setup, installation, or onboarding issues. NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). priority: medium Issue that should be addressed in upcoming releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants