fix(mac): auto-start runner with the env it actually needs#783
Conversation
PR #779 introduced no-auth mode but left three rough edges in the Mac app side of the spawn: - HOST=127.0.0.1 wasn't set in the runner env. start-local.ts defaults HOST=0.0.0.0, and the no-auth bind guard refuses non-loopback, so Start would silently crash the runner on boot. - ENCRYPTION_KEY isn't set on personal installs and the embedded gateway refuses to boot without it (or LOBU_ALLOW_EPHEMERAL_ENCRYPTION_KEY=1). For a personal install the ephemeral key is the only sensible default — opt in here so the user doesn't manage a secret manually. - The bundled lobu CLI requires Node 22.x–24.x (isolated-vm constraint) but a user on Node 25+ via Homebrew's `node` formula would crash the runner. Prefer well-known Node 22 install paths (Homebrew keg-only `node@22`, mise, fnm) by prepending them to PATH for the spawn. Also cleans up two related UX issues: - AppState.connect() now auto-fires on launch when credentials are nil and the URL targets the managed runner — no Start button click needed for the 99% case. The connection card only surfaces on actual failure (CLI missing, port conflict, etc). - startLocalLobu()'s failure handler no longer echoes the runner error into state.status. The connection card's connectStatusLine already shows it in orange under the button; echoing into state.status duplicates the message at the top of the popover.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe PR refines the local runner startup flow on macOS: auto-connect at launch is now conditional on the server URL matching the embedded managed runner, the spawned process is configured with ephemeral encryption support and preferred Node discovery paths, and error handling is adjusted to avoid redundant status messages. ChangesLocal runner startup improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/mac/Lobu/LocalLobuRunner.swift (1)
166-187: 💤 Low valueConsider adding nvm and asdf paths for broader coverage.
The current candidates cover Homebrew keg-only, mise, and fnm. Two other popular version managers (nvm and asdf) have different path structures that aren't covered:
- nvm:
~/.nvm/versions/node/v22.*/bin- asdf:
~/.asdf/installs/nodejs/22.*/binThese require glob expansion, which adds complexity. The current approach is pragmatic since Homebrew and mise/fnm cover the most common macOS setups, and users of nvm/asdf typically have their shell already configured to put the right Node on PATH. This is a nice-to-have for future consideration.
🤖 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 `@apps/mac/Lobu/LocalLobuRunner.swift` around lines 166 - 187, preferredPath currently checks a fixed list of candidate prefixes (variable candidates) and only covers Homebrew, mise and fnm; add expansion for nvm and asdf installs by globbing their versioned directories and inserting any matching bin paths ahead of currentPath. Modify preferredPath to: (1) construct the existing candidates, (2) expand globs for "~/.nvm/versions/node/v22*/bin" and "~/.asdf/installs/nodejs/22*/bin" (resolving ~ with NSHomeDirectory()), (3) filter the resulting prefixes using FileManager.default.isExecutableFile(atPath: "\(prefix)/node") as done now, and (4) return (prefixes + [currentPath]).joined(separator: ":") so nvm/asdf matches are considered first if present.
🤖 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.
Nitpick comments:
In `@apps/mac/Lobu/LocalLobuRunner.swift`:
- Around line 166-187: preferredPath currently checks a fixed list of candidate
prefixes (variable candidates) and only covers Homebrew, mise and fnm; add
expansion for nvm and asdf installs by globbing their versioned directories and
inserting any matching bin paths ahead of currentPath. Modify preferredPath to:
(1) construct the existing candidates, (2) expand globs for
"~/.nvm/versions/node/v22*/bin" and "~/.asdf/installs/nodejs/22*/bin" (resolving
~ with NSHomeDirectory()), (3) filter the resulting prefixes using
FileManager.default.isExecutableFile(atPath: "\(prefix)/node") as done now, and
(4) return (prefixes + [currentPath]).joined(separator: ":") so nvm/asdf matches
are considered first if present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8dc1f78a-ef2a-47cd-989b-f8be07231a6e
📒 Files selected for processing (2)
apps/mac/Lobu/AppState.swiftapps/mac/Lobu/LocalLobuRunner.swift
2 participants
Summary
PR #779 introduced no-auth mode but left three rough edges in the Mac app side of the spawn — each of which made the "Start" path silently fail end-to-end:
Plus two UX cleanups:
Test plan
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements