Skip to content

fix(install): fail fast on missing binutils at preflight (#4415)#4491

Open
latenighthackathon wants to merge 1 commit into
NVIDIA:mainfrom
latenighthackathon:fix/4415-binutils-preflight
Open

fix(install): fail fast on missing binutils at preflight (#4415)#4491
latenighthackathon wants to merge 1 commit into
NVIDIA:mainfrom
latenighthackathon:fix/4415-binutils-preflight

Conversation

@latenighthackathon
Copy link
Copy Markdown
Contributor

@latenighthackathon latenighthackathon commented May 29, 2026

Summary

Adds an early preflight check so the installer fails fast with an actionable message when binutils (the strings binary) is missing, instead of running for several minutes and then aborting at the final OpenShell verification step.

Problem

scripts/install-openshell.sh uses strings (from binutils) in openshell_has_required_messaging_features to confirm the OpenShell CLI binary carries the credential-rewrite endpoints. That check only runs during OpenShell install/verification, which happens late in the installer. On a clean host without binutils the installer therefore ran for around five minutes (Node.js install, repo clone, npm install, tsc build, OpenShell download and checksum) before aborting with 'strings' is required to verify OpenShell messaging credential rewrite support. All of that work was discarded, and the installer could only complete after binutils was installed and the run repeated. Reported on a clean Ubuntu 24.04 host in #4415.

Changes

  • Add ensure_openshell_build_deps and call it in main() immediately after ensure_docker, before any clone, build, or download work.
  • When strings is absent the check exits with a single actionable line pointing at sudo apt-get install -y binutils.
  • Skip the check when NEMOCLAW_DEFER_OPENSHELL_INSTALL=1, because that flag postpones all OpenShell work to a later phase where install-openshell.sh runs the same strings check itself, so the pre-upgrade backup flow is unaffected.
  • Mirror the existing ensure_docker and ensure_supported_runtime detect-and-fail-fast preflight pattern rather than auto-installing a system package.

Related Issue

Closes #4415

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Ran: full test/install-preflight.test.ts suite passes (101/101), including two new cases - the installer fails fast with the binutils guidance and never reaches the OpenShell install or clone when strings is missing, and the preflight stays silent under NEMOCLAW_DEFER_OPENSHELL_INSTALL=1; bash -n scripts/install.sh clean; shellcheck passes via the pre-push hook.


Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com

Summary by CodeRabbit

  • New Features

    • Installer now validates required build tools upfront and aborts early with clear remediation guidance when a dependency is missing. This check can be deferred via an environment variable for specialized workflows.
  • Tests

    • Added preflight tests that verify failure when a build tool is absent and confirm the deferred-install behavior when the environment variable is set.

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 29, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1daa2b8c-0d01-4800-b221-2c9930e1c936

📥 Commits

Reviewing files that changed from the base of the PR and between dc62b02 and 8524bd9.

📒 Files selected for processing (2)
  • scripts/install.sh
  • test/install-preflight.test.ts
💤 Files with no reviewable changes (1)
  • test/install-preflight.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/install.sh

📝 Walkthrough

Walkthrough

Adds an early preflight check that fails fast if the strings binary (from binutils) is missing before any OpenShell download/build work; the check is skipped when NEMOCLAW_DEFER_OPENSHELL_INSTALL=1. Tests simulate a PATH without strings and verify the fail-fast behavior and the deferred-install bypass.

Changes

Build dependency preflight enforcement

Layer / File(s) Summary
Preflight check implementation
scripts/install.sh
Adds ensure_openshell_build_deps() which verifies strings availability and is invoked from main() immediately after the third-party notice preflight; respects NEMOCLAW_DEFER_OPENSHELL_INSTALL to skip the check.
Preflight runtime tests
test/install-preflight.test.ts
Adds helpers to build an isolated PATH excluding specified binaries and to stub docker/systemctl. Tests assert installer exits early with a strings/binutils remediation message when strings is absent, and that the check is bypassed when NEMOCLAW_DEFER_OPENSHELL_INSTALL=1.

Sequence Diagram

sequenceDiagram
  participant Installer as Installer main()
  participant Preflight as ensure_openshell_build_deps()
  participant Shell as host shell (which strings)
  Installer->>Preflight: invoke after third-party preflight
  Preflight->>Shell: run "which strings"
  alt strings found
    Shell-->>Preflight: path returned
    Preflight-->>Installer: continue
  else strings not found
    Shell-->>Preflight: no result
    Preflight-->>Installer: exit 1 with apt-get install -y binutils hint
  end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#4060: Modifies scripts/install.sh around the OpenShell onboarding flow and is related to conditional OpenShell bootstrap behavior.

Suggested Labels

OpenShell

Suggested Reviewers

  • ericksoa
  • cv

Poem

🐰 I checked the PATH with twitchy paws tonight,
Found a missing "strings" and halted the long flight—
Five minutes saved, no fruitless build brigade,
A tiny apt hint, and the rabbit hops away. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding an early preflight check to fail fast when the binutils package is missing.
Linked Issues check ✅ Passed The pull request fully addresses the requirements from issue #4415: it implements an early preflight check for the strings binary from binutils, fails fast with an actionable error message, and skips the check when deferred.
Out of Scope Changes check ✅ Passed All changes are directly scoped to addressing the linked issue: adding the ensure_openshell_build_deps() function, integrating it into the install flow, and adding corresponding test coverage.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✏️ 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.

@wscurran wscurran added fix Platform: Ubuntu Support for Linux Ubuntu labels May 29, 2026
@wscurran
Copy link
Copy Markdown
Contributor

✨ Thanks for submitting this detailed PR about adding an early preflight check for missing binutils. This proposes a code change to fail fast with an actionable message when binutils is missing, instead of running for several minutes and then aborting at the final OpenShell verification step.


Related open issues:

scripts/install-openshell.sh uses `strings` (from binutils) to verify the
OpenShell CLI binary carries the credential-rewrite endpoints. That check only
ran during OpenShell install/verification, late in the installer, so on a host
without binutils the installer ran for ~5 minutes (Node.js install, repo clone,
npm install, tsc build, OpenShell download + checksum) before aborting at the
final verification step. Reported on a clean Ubuntu 24.04 host in NVIDIA#4415.

Add an ensure_openshell_build_deps preflight in main(), right after
ensure_docker and before any clone/build/download work, that fails fast with an
actionable message (Debian/Ubuntu: sudo apt-get install -y binutils) when
`strings` is absent. The check is skipped when NEMOCLAW_DEFER_OPENSHELL_INSTALL=1,
since that flag postpones all OpenShell work to a later phase where
install-openshell.sh runs the same check itself, leaving the pre-upgrade backup
flow unaffected.

Tests: two cases in test/install-preflight.test.ts assert the installer fails
fast with the binutils guidance and never reaches the OpenShell install/clone
when `strings` is missing, and that the preflight stays silent under
NEMOCLAW_DEFER_OPENSHELL_INSTALL=1. Full install-preflight suite passes (101/101)
in the nemoclaw-test container.

Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
@latenighthackathon latenighthackathon force-pushed the fix/4415-binutils-preflight branch from dc62b02 to 8524bd9 Compare May 31, 2026 16:01
@wscurran wscurran added the v0.0.58 Release target label Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Platform: Ubuntu Support for Linux Ubuntu v0.0.58 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Ubuntu 24.04][Install] missing binutils not detected at preflight; OpenShell verification fails ~5 min into install

2 participants