Skip to content

fix(onboard): add sudo prefix to lsof port-conflict suggestions#1073

Open
latenighthackathon wants to merge 4 commits intoNVIDIA:mainfrom
latenighthackathon:fix/onboard-lsof-sudo
Open

fix(onboard): add sudo prefix to lsof port-conflict suggestions#1073
latenighthackathon wants to merge 4 commits intoNVIDIA:mainfrom
latenighthackathon:fix/onboard-lsof-sudo

Conversation

@latenighthackathon
Copy link
Copy Markdown
Contributor

@latenighthackathon latenighthackathon commented Mar 29, 2026

Summary

On Linux, lsof requires root privileges to see processes owned by other users. The onboarding port-conflict message suggested lsof without sudo, which returns empty output.

Related Issue

Closes #726

Changes

  • Added sudo prefix to lsof command in the 'process found' error path
  • Added sudo prefix to lsof command in the 'could not identify' fallback path

Type of Change

  • Code change for a new feature, bug fix, or refactor.
  • Code change with doc updates.
  • Doc only. Prose changes without code sample modifications.
  • Doc only. Includes code sample changes.

Testing

  • npx prek run --all-files passes (or equivalently make check).
  • npm test passes.
  • make docs builds without warnings. (for doc-only changes)

Checklist

General

Code Changes

  • Formatters applied.
  • No secrets, API keys, or credentials committed.

Summary by CodeRabbit

  • Bug Fixes
    • Improved troubleshooting guidance for port conflicts during setup: diagnostic command suggestions for inspecting or terminating a conflicting process now include the necessary privilege escalation (so commands work as expected in common environments).

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 29, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e351444e-7dfd-40c8-9043-00bc7e857896

📥 Commits

Reviewing files that changed from the base of the PR and between b82c1fb and 2a57b13.

📒 Files selected for processing (1)
  • bin/lib/onboard.js
✅ Files skipped from review due to trivial changes (1)
  • bin/lib/onboard.js

📝 Walkthrough

Walkthrough

Prefixed lsof troubleshooting commands with sudo in onboarding messages and troubleshooting docs to ensure processes holding ports can be identified; no control flow or runtime behavior changed.

Changes

Cohort / File(s) Summary
Onboarding port troubleshooting
bin/lib/onboard.js
Prepended sudo to suggested lsof commands in preflight() port-conflict messages; no other logic or branching changed.
Documentation: troubleshooting guidance
docs/reference/troubleshooting.md, .agents/skills/nemoclaw-reference/references/troubleshooting.md
Changed example lsof -i :18789 to sudo lsof -i :18789; subsequent kill <PID> steps unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 A port was hidden, quiet and snug,
I nudged the command with a helpful hug,
"Add sudo," I whispered, tiptoeing light,
Now PIDs appear, brought back to sight,
Hops of joy — small paws, big delight.

🚥 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 summarizes the main change: adding sudo prefix to lsof commands in port-conflict troubleshooting.
Linked Issues check ✅ Passed All code objectives from issue #726 are met: sudo prefix added to lsof in both error paths and both documentation files updated.
Out of Scope Changes check ✅ Passed All changes directly address issue #726 requirements; no unrelated modifications detected in the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.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.

On Linux, lsof requires root privileges to see processes owned by
other users. Without sudo, the suggested command returns empty output,
leaving users unable to identify the conflicting process.

Closes NVIDIA#726
@brandonpelfrey brandonpelfrey self-assigned this Mar 30, 2026
@brandonpelfrey
Copy link
Copy Markdown
Collaborator

The change itself does appear to miss two user-facing doc copies of the same guidance:

  • docs/reference/troubleshooting.md:98 still says lsof -i :18789 without sudo.
  • .agents/skills/nemoclaw-reference/references/troubleshooting.md:72 has the same stale command.

Please add these updates. Code-wise looks good to me.

Update the two remaining doc copies that still showed lsof without
sudo: docs/reference/troubleshooting.md and the agent skill reference.

Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
@latenighthackathon
Copy link
Copy Markdown
Contributor Author

Good catch — added sudo to both remaining copies:

  • docs/reference/troubleshooting.md:98
  • .agents/skills/nemoclaw-reference/references/troubleshooting.md:72

Ready for re-review.

@wscurran wscurran added the fix label Mar 30, 2026
@wscurran
Copy link
Copy Markdown
Contributor

✨ Thanks for submitting this PR with a detailed summary, it addresses a bug with lsof port-conflict suggestions and proposes a fix to improve the user experience of NemoClaw.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lsof -i:8080 command on onboarding needs sudo prefix to work properly

3 participants