Skip to content

Fix lsof path resolution and support server_port fallback#134

Open
athal7 wants to merge 2 commits into
mainfrom
lsof-path-and-port-fallback-fixes-3601355634390134876
Open

Fix lsof path resolution and support server_port fallback#134
athal7 wants to merge 2 commits into
mainfrom
lsof-path-and-port-fallback-fixes-3601355634390134876

Conversation

@athal7

@athal7 athal7 commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Dynamically resolves the lsof path on linux and supports fallback to preferredPort when lsof checks cannot find the running opencode server.

Fixes #117


PR created automatically by Jules for task 3601355634390134876 started by @athal7

Summary by CodeRabbit

  • Bug Fixes

    • Improved server discovery when port information is unavailable.
    • Added support for detecting running servers across common system configurations.
    • Configured preferred ports are now checked reliably during server discovery.
  • Tests

    • Added coverage for discovering a server through its configured preferred port.

…iscovery

- Scans standard paths (/usr/sbin/lsof, /usr/bin/lsof, /bin/lsof, /sbin/lsof) first and falls back to plain 'lsof' to resolve Ubuntu-specific path issues.
- Appends the configured server_port (preferredPort) to the scanned ports array in discoverOpencodeServer, ensuring the port is checked even if lsof fails to detect it due to user permission or context restrictions.

Co-authored-by: athal7 <467872+athal7@users.noreply.github.com>
@google-labs-jules

Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Port discovery now resolves lsof from multiple absolute locations. Server discovery also checks the configured preferred port when port detection returns no matching port, with a corresponding unit test.

Changes

Port discovery resilience

Layer / File(s) Summary
Resolve lsof executable
service/actions.js
getOpencodePorts probes multiple absolute lsof locations and uses the first existing executable.
Check preferred port fallback
service/actions.js, test/unit/actions.test.js
discoverOpencodeServer appends a configured preferred port when absent from detected ports, and tests discovery when no ports are detected.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 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 clearly matches the main change: lsof path resolution and server port fallback.
Linked Issues check ✅ Passed The changes address #117 by resolving lsof from common locations and falling back to the configured preferred server port.
Out of Scope Changes check ✅ Passed The diff stays focused on lsof discovery logic and its test coverage, with no obvious unrelated additions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch lsof-path-and-port-fallback-fixes-3601355634390134876

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@service/actions.js`:
- Around line 196-202: Update the lsof candidate selection loop to validate each
path with executable permissions, using accessSync and X_OK alongside existsSync
or an equivalent executable check. Only assign and break on a runnable
candidate; otherwise continue through lsofPaths so the existing default lsof
PATH fallback remains available.
🪄 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 Plus

Run ID: c8b5f5ec-fb38-429b-8477-5bf721b6020f

📥 Commits

Reviewing files that changed from the base of the PR and between 091b6e1 and d3d4eda.

📒 Files selected for processing (2)
  • service/actions.js
  • test/unit/actions.test.js

Comment thread service/actions.js
Comment on lines +196 to +202
let lsofBin = 'lsof';
for (const p of lsofPaths) {
if (existsSync(p)) {
lsofBin = p;
break;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Check that the selected lsof path is executable.
existsSync() can pick a directory or non-executable file, and execSync() will then fail without trying the remaining candidates or the PATH fallback. Use accessSync(path, X_OK) or keep iterating until one candidate can actually run.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@service/actions.js` around lines 196 - 202, Update the lsof candidate
selection loop to validate each path with executable permissions, using
accessSync and X_OK alongside existsSync or an equivalent executable check. Only
assign and break on a runnable candidate; otherwise continue through lsofPaths
so the existing default lsof PATH fallback remains available.

@athal7

athal7 commented Jul 16, 2026

Copy link
Copy Markdown
Owner Author

@jules can you address the outstanding review comment from CodeRabbit above — the executable-check issue at service/actions.js:202 (use accessSync with X_OK instead of existsSync)?

…iscovery

- Scans standard paths (/usr/sbin/lsof, /usr/bin/lsof, /bin/lsof, /sbin/lsof) first and falls back to plain 'lsof' to resolve Ubuntu-specific path issues.
- Appends the configured server_port (preferredPort) to the scanned ports array in discoverOpencodeServer, ensuring the port is checked even if lsof fails to detect it due to user permission or context restrictions.

Co-authored-by: athal7 <467872+athal7@users.noreply.github.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.

lsof is not at /usr/sbin on linux

1 participant