Skip to content

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

Open
latenighthackathon wants to merge 1 commit intoNVIDIA:mainfrom
latenighthackathon:fix/onboard-lsof-sudo
Open

fix(onboard): add sudo prefix to lsof port-conflict suggestions#1073
latenighthackathon wants to merge 1 commit 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 by ensuring diagnostic command suggestions now include the necessary privilege escalation prefix for proper execution.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 29, 2026

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: eb11f969-d512-4896-afdc-2c05757f5699

📥 Commits

Reviewing files that changed from the base of the PR and between 5d33433 and 2ab6426.

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

📝 Walkthrough

Walkthrough

A single-file change adds the sudo prefix to lsof command suggestions shown during onboarding when a port conflict is detected, updating guidance text without altering control flow or runtime behavior.

Changes

Cohort / File(s) Summary
Onboarding Port Troubleshooting
bin/lib/onboard.js
Added sudo prefix to lsof command instructions in the preflight() function's port-conflict guidance messages so the suggested diagnostic command runs with elevated privileges when needed.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 A troublesome port blocked the way,
With lsof advice that wouldn't play,
I hopped in and whispered, "add sudo, hooray!"
Now processes show up in bright array,
Happy hops for a smoother day.

🚥 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 sudo prefix to lsof port-conflict suggestions in onboarding.
Linked Issues check ✅ Passed The code changes successfully address issue #726 by adding sudo prefix to both lsof command suggestions in the port conflict error paths.
Out of Scope Changes check ✅ Passed All changes are directly related to the objective of adding sudo prefix to lsof commands; no unrelated modifications are present.
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
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.

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

1 participant