Repository navigation
Fix cmux omo bootstrap when user pins yanked plugin deps - #2280
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe changes refactor the OpenCode ( Changes
Sequence DiagramsequenceDiagram
actor User
participant CMUXCLI
participant ShadowDir as Shadow Dir<br/>(filesystem)
participant PackageManager as Package Manager<br/>(bun)
participant OpenCode
User->>CMUXCLI: cmux omo [args]
CMUXCLI->>ShadowDir: Inspect file types & symlinks
CMUXCLI->>ShadowDir: Write fresh package.json manifest<br/>(oh-my-opencode@latest only)
CMUXCLI->>ShadowDir: Remove stale bun.lock symlink
CMUXCLI->>ShadowDir: Ensure node_modules symlink<br/>→ user's node_modules
CMUXCLI->>PackageManager: Run: bun install in shadow dir
alt Install succeeds
PackageManager-->>CMUXCLI: Exit 0
else Install fails
PackageManager-->>CMUXCLI: Exit non-zero
CMUXCLI->>ShadowDir: Clear bun.lock
CMUXCLI->>ShadowDir: Recreate node_modules symlink
CMUXCLI->>PackageManager: Retry: bun install (once)
PackageManager-->>CMUXCLI: Exit status
end
CMUXCLI->>CMUXCLI: Resolve OpenCode port<br/>(--port arg / env / bindable)
CMUXCLI->>CMUXCLI: Configure environment<br/>with OPENCODE_PORT
CMUXCLI->>OpenCode: Launch with --port & args
OpenCode-->>User: Running
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes
Confidence Score: 5/5Safe to merge; all findings are P2 style/edge-case suggestions with no impact on the primary fix path. The core logic is sound: shadow package.json isolation correctly prevents yanked-pin poisoning, the retry on bun failure is well-structured, and the port-probing fallback is a clear improvement over hardcoding 4096. All review findings are P2 — a misleading comment, a silent-failure edge case in the retry cleanup that requires an unlikely combination of events, a TOCTOU window inherent to any probe-then-use port strategy, and a narrowly pinned zig version check in a dev script. None of these block the intended fix. No files require special attention; the retry-cleanup silent-failure path in Important Files Changed
Sequence DiagramsequenceDiagram
participant U as cmux omo (user)
participant S as omoSetupShadowConfig
participant PM as omoEnsureShadowPackageManifest
participant NM as omoEnsureShadowNodeModulesSymlink
participant PR as omoResolvedPort
participant BUN as bun add
U->>S: run omo bootstrap
S->>PM: write shadow package.json ("latest")
PM-->>S: symlink removed / file written
S->>NM: ensure node_modules symlink → userNodeModules
NM-->>S: symlink created or verified
S->>PR: resolve port (args → OPENCODE_PORT → 4096 → ephemeral)
PR-->>S: openCodePort string
S->>S: launcherEnvironment["OPENCODE_PORT"] = openCodePort
alt plugin NOT in node_modules
S->>BUN: bun add oh-my-opencode (attempt 1)
BUN-->>S: firstAttemptStatus
alt firstAttemptStatus != 0
S->>S: remove shadow bun.lock + node_modules (try?)
S->>NM: re-create node_modules symlink
S->>BUN: bun add oh-my-opencode (retry)
BUN-->>S: retryStatus
alt retryStatus != 0
S-->>U: throw CLIError
end
end
end
S->>U: execve opencode --port openCodePort
Reviews (1): Last reviewed commit: "Fix cmux omo bootstrap with yanked deps" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if firstAttemptStatus != 0 { | ||
| FileHandle.standardError.write("Retrying oh-my-opencode install with a clean shadow package state...\n".data(using: .utf8)!) | ||
| try? fm.removeItem(at: shadowBunLockURL) | ||
| try? fm.removeItem(at: shadowNodeModules) | ||
| try omoEnsureShadowNodeModulesSymlink(shadowNodeModules: shadowNodeModules, userNodeModules: userNodeModules) | ||
| let retryStatus = try omoRunPackageInstall( | ||
| executablePath: bunPath, | ||
| arguments: installArguments, | ||
| currentDirectoryURL: installDir | ||
| ) | ||
| if retryStatus != 0 { | ||
| throw CLIError(message: "Failed to install oh-my-opencode. Try manually: npm install -g oh-my-opencode") | ||
| } | ||
| } |
There was a problem hiding this comment.
node_modules if removal silently fails
On the first failed bun add, bun may have partially created shadowNodeModules as a real directory. If try? fm.removeItem(at: shadowNodeModules) then silently fails (e.g. a locked sub-file), omoEnsureShadowNodeModulesSymlink detects a non-symlink type and returns early:
} else {
return // real directory — leaves it alone
}The retry then runs against the same incomplete node_modules, likely producing the same failure. The try? on removeItem is appropriate for the common case, but if removal fails for a reason other than "file doesn't exist," the retry has no additional benefit over the first attempt.
A diagnostic log line on removal failure would at least make this visible to users who hit it:
if fm.fileExists(atPath: shadowNodeModules.path) {
do {
try fm.removeItem(at: shadowNodeModules)
} catch {
FileHandle.standardError.write(
"Warning: could not remove shadow node_modules (\(error)); retry may not help\n"
.data(using: .utf8)!)
}
}| return String(preferredPort) | ||
| } | ||
|
|
||
| if let fallbackPort = omoBindableLoopbackPort(0) { | ||
| return String(fallbackPort) | ||
| } | ||
|
|
||
| return "4096" |
There was a problem hiding this comment.
omoBindableLoopbackPort(0) binds to port 0, reads back the OS-assigned port via getsockname, then immediately closes the socket via defer { close(socketDescriptor) }. The port number is returned and used as openCodePort, but it is no longer held — another process (or the OS ephemeral-port recycler) can reassign it before opencode starts and calls bind().
This is a standard limitation of any "probe then use" port-selection strategy and is unlikely to matter in practice, but worth documenting in a comment so the next reader understands why a startup bind failure is possible even after this function returns a value:
// Note: the port is released immediately after discovery; there is a
// small TOCTOU window before opencode claims it.
if let fallbackPort = omoBindableLoopbackPort(0) {
return String(fallbackPort)
}| // Keep the shadow package isolated from stale/yanked pins in the user's | ||
| // opencode package.json. bun will update this manifest with the resolved | ||
| // oh-my-opencode version when installation succeeds. |
There was a problem hiding this comment.
The comment says "bun will update this manifest with the resolved oh-my-opencode version when installation succeeds," implying the pinned version persists. But omoEnsureShadowPackageManifest is called on every cmux omo invocation, and the existing != output check will detect that bun's pin (e.g. "oh-my-opencode": "1.2.3") differs from the template "latest" — so the manifest is reset back to "latest" on every run.
The actual design intent is the opposite: the shadow package.json deliberately stays pinned to "latest" so that if node_modules is ever absent, a fresh install always pulls the current published version rather than a potentially yanked pin. The comment should reflect this:
// Keep the shadow package.json fixed at "latest" so that if node_modules
// is absent and bun re-runs, it always installs the current published version
// rather than a potentially yanked pin from a stale lockfile.| zig_version="$(zig version 2>/dev/null || true)" | ||
| major_version="${product_version%%.*}" | ||
|
|
||
| if [[ "$zig_version" == "0.15.2" ]] && [[ "$major_version" =~ ^[0-9]+$ ]] && (( major_version >= 26 )); then |
There was a problem hiding this comment.
The auto-skip is gated on "$zig_version" == "0.15.2" (exact string match). If the host upgrades to 0.15.3 or any subsequent patch while the Ghostty CLI helper zig build is still broken on macOS 26, the guard won't fire and developers will see a build failure without an obvious reason.
Consider a prefix/range check, or at least document why the exact version is required:
# Skip if zig is 0.15.x on macOS >= 26 (cli-helper build broken for this zig minor)
if [[ "$zig_version" == 0.15.* ]] && [[ "$major_version" =~ ^[0-9]+$ ]] && (( major_version >= 26 )); then![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/reload.sh (1)
458-462: Consider warning if no prior ghostty binary exists when skipping the build.When skipping the build, if
GHOSTTY_HELPER_SRCdoesn't exist from a prior build, the app will silently lack the ghostty helper. For dev builds this is an acceptable tradeoff, but a warning could help developers understand why ghostty features might not work.💡 Optional: Add a warning when skipping with no prior binary
if [[ "${CMUX_SKIP_ZIG_BUILD:-}" == "1" ]]; then echo "Skipping direct ghostty CLI helper zig build (CMUX_SKIP_ZIG_BUILD=1)" + if [[ ! -x "$GHOSTTY_HELPER_SRC" ]]; then + echo " Warning: No prior ghostty binary at $GHOSTTY_HELPER_SRC; ghostty features may be unavailable" + fi else🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/reload.sh` around lines 458 - 462, When CMUX_SKIP_ZIG_BUILD is set and the script skips the zig build (the if branch checking CMUX_SKIP_ZIG_BUILD), add a check for the ghostty helper artifact referenced by GHOSTTY_HELPER_SRC (or the expected binary path used later) and emit a clear warning if that file does not exist; update the conditional block where CMUX_SKIP_ZIG_BUILD is evaluated so that before echoing the skip message you test [ -f "$GHOSTTY_HELPER_SRC" ] (or equivalent) and log a warning that the ghostty helper is missing and ghostty features may not work when the file is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 9760-9776: The parser currently returns nil for both "no --port
flag" and "flag present but malformed", so change omoRequestedPort(from:) to
distinguish these cases: if it sees "--port" with no next token or the next
token is empty or starts with "-" (another flag), or sees "--port=" with an
empty value, return an explicit sentinel (e.g. an empty String) instead of nil;
keep returning nil only when the flag is absent. Then update runOMO() to treat
that sentinel (empty String) as a CLI error and surface a helpful message rather
than appending a default "--port" again. Reference: omoRequestedPort(from:) and
runOMO().
- Around line 9723-9741: The current omoEnsureShadowNodeModulesSymlink leaves a
dangling or inappropriate shadow symlink when shadowNodeModules points at
userNodeModules that has since disappeared, allowing retries to reuse the user's
partial node_modules; change omoEnsureShadowNodeModulesSymlink so that if
shadowNodeModules is a symlink targeting userNodeModules but userNodeModules
does not exist (or is incomplete), remove the symlink and create a clean local
shadow node_modules (or recreate the correct symlink) before returning; apply
the same logic to the analogous npm retry path referenced around the other block
(lines handling shadowNodeModules / userNodeModules at 9919-9923) so retries
always operate on a freshly cleaned local shadow directory rather than a stale/
dangling symlink target.
- Around line 9929-9942: The recovery hint is misleading because the code
installs the plugin into the shadow config directory (pluginPackageDir /
installDir) but the CLIError messages suggest a global install; update the error
text in both places where we throw CLIError after omoRunPackageInstall (and any
other throw referencing npm install -g) to point users to the local shadow
install location (reference pluginPackageDir and installDir) and offer either a
local npm install command using those paths (or instructions to delete/recreate
the shadow dir and re-run bootstrap) so the hint matches how
omoRunPackageInstall and Self.omoPluginName actually install the package.
---
Nitpick comments:
In `@scripts/reload.sh`:
- Around line 458-462: When CMUX_SKIP_ZIG_BUILD is set and the script skips the
zig build (the if branch checking CMUX_SKIP_ZIG_BUILD), add a check for the
ghostty helper artifact referenced by GHOSTTY_HELPER_SRC (or the expected binary
path used later) and emit a clear warning if that file does not exist; update
the conditional block where CMUX_SKIP_ZIG_BUILD is evaluated so that before
echoing the skip message you test [ -f "$GHOSTTY_HELPER_SRC" ] (or equivalent)
and log a warning that the ghostty helper is missing and ghostty features may
not work when the file is absent.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19e0a7fd-54cd-451d-91ef-aa11a1ad5088
📒 Files selected for processing (2)
CLI/cmux.swiftscripts/reload.sh
| let fm = FileManager.default | ||
| guard fm.fileExists(atPath: userNodeModules.path) else { return } | ||
|
|
||
| if let type = omoFileType(at: shadowNodeModules) { | ||
| if type == .typeSymbolicLink { | ||
| let target = try? fm.destinationOfSymbolicLink(atPath: shadowNodeModules.path) | ||
| if target != userNodeModules.path { | ||
| try? fm.removeItem(at: shadowNodeModules) | ||
| } else { | ||
| return | ||
| } | ||
| } else { | ||
| return | ||
| } | ||
| } | ||
|
|
||
| if !fm.fileExists(atPath: shadowNodeModules.path) { | ||
| try fm.createSymbolicLink(at: shadowNodeModules, withDestinationURL: userNodeModules) | ||
| } |
There was a problem hiding this comment.
The retry is not isolated while shadow/node_modules points at the user's tree.
omoEnsureShadowNodeModulesSymlink() leaves a dangling shadow symlink untouched when ~/.config/opencode/node_modules disappears, and the bun retry later only removes the shadow link itself. So a failed first install can still reuse partial state from the user's node_modules, and the npm path has no retry at all. Bootstrap needs a real clean local shadow/node_modules for the retry, or it should explicitly clean the symlink target before rerunning.
Also applies to: 9919-9923
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 9723 - 9741, The current
omoEnsureShadowNodeModulesSymlink leaves a dangling or inappropriate shadow
symlink when shadowNodeModules points at userNodeModules that has since
disappeared, allowing retries to reuse the user's partial node_modules; change
omoEnsureShadowNodeModulesSymlink so that if shadowNodeModules is a symlink
targeting userNodeModules but userNodeModules does not exist (or is incomplete),
remove the symlink and create a clean local shadow node_modules (or recreate the
correct symlink) before returning; apply the same logic to the analogous npm
retry path referenced around the other block (lines handling shadowNodeModules /
userNodeModules at 9919-9923) so retries always operate on a freshly cleaned
local shadow directory rather than a stale/ dangling symlink target.
| private func omoRequestedPort(from commandArgs: [String]) -> String? { | ||
| for (index, arg) in commandArgs.enumerated() { | ||
| if arg == "--port" { | ||
| let nextIndex = commandArgs.index(after: index) | ||
| guard nextIndex < commandArgs.endIndex else { return nil } | ||
| let value = commandArgs[nextIndex].trimmingCharacters(in: .whitespacesAndNewlines) | ||
| return value.isEmpty ? nil : value | ||
| } | ||
|
|
||
| if arg.hasPrefix("--port=") { | ||
| let value = String(arg.dropFirst("--port=".count)).trimmingCharacters(in: .whitespacesAndNewlines) | ||
| return value.isEmpty ? nil : value | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
Malformed --port input turns into duplicated flags.
When the argv contains --port or --port= without a usable value, omoRequestedPort() returns nil, so runOMO() appends a second --port <default> instead of surfacing a CLI error. That makes cmux omo --port fail in a much more confusing way downstream.
Also applies to: 10085-10089
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 9760 - 9776, The parser currently returns nil
for both "no --port flag" and "flag present but malformed", so change
omoRequestedPort(from:) to distinguish these cases: if it sees "--port" with no
next token or the next token is empty or starts with "-" (another flag), or sees
"--port=" with an empty value, return an explicit sentinel (e.g. an empty
String) instead of nil; keep returning nil only when the flag is absent. Then
update runOMO() to treat that sentinel (empty String) as a CLI error and surface
a helpful message rather than appending a default "--port" again. Reference:
omoRequestedPort(from:) and runOMO().
| if retryStatus != 0 { | ||
| throw CLIError(message: "Failed to install oh-my-opencode. Try manually: npm install -g oh-my-opencode") | ||
| } | ||
| } | ||
| } else if let npmPath = resolveExecutableInPath("npm") { | ||
| process.executableURL = URL(fileURLWithPath: npmPath) | ||
| process.arguments = ["install", Self.omoPluginName] | ||
| FileHandle.standardError.write("Installing oh-my-opencode plugin (this may take a minute on first run)...\n".data(using: .utf8)!) | ||
| let status = try omoRunPackageInstall( | ||
| executablePath: npmPath, | ||
| arguments: ["install", Self.omoPluginName], | ||
| currentDirectoryURL: installDir | ||
| ) | ||
| if status != 0 { | ||
| throw CLIError(message: "Failed to install oh-my-opencode. Try manually: npm install -g oh-my-opencode") | ||
| } |
There was a problem hiding this comment.
The recovery hint points users at an install location this code never reads.
pluginPackageDir is always resolved under the shadow config directory, so npm install -g oh-my-opencode will not satisfy the next existence check. This should point users at a local install in the shadow dir, or at deleting/recreating that dir and rerunning bootstrap.
Suggested fix
- throw CLIError(message: "Failed to install oh-my-opencode. Try manually: npm install -g oh-my-opencode")
+ throw CLIError(
+ message: "Failed to install oh-my-opencode in \(shadowDir.path). Remove that directory and rerun, or install it locally there with `bun add \(Self.omoPluginName)`."
+ )
@@
- throw CLIError(message: "Failed to install oh-my-opencode. Try manually: npm install -g oh-my-opencode")
+ throw CLIError(
+ message: "Failed to install oh-my-opencode in \(shadowDir.path). Remove that directory and rerun, or install it locally there with `npm install \(Self.omoPluginName)`."
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 9929 - 9942, The recovery hint is misleading
because the code installs the plugin into the shadow config directory
(pluginPackageDir / installDir) but the CLIError messages suggest a global
install; update the error text in both places where we throw CLIError after
omoRunPackageInstall (and any other throw referencing npm install -g) to point
users to the local shadow install location (reference pluginPackageDir and
installDir) and offer either a local npm install command using those paths (or
instructions to delete/recreate the shadow dir and re-run bootstrap) so the hint
matches how omoRunPackageInstall and Self.omoPluginName actually install the
package.
This branch was successfully deployed
1 participant
Closes #2278
Summary
~/.cmuxterm/omo-configand write an isolated shadowpackage.jsoninsteadbun add oh-my-opencodeonce after clearing the shadowbun.lockandnode_modulescmux omofrom getting stuck behind a staleopencodeport by preferring4096only when it is free and otherwise falling back to an available loopback port while keepingOPENCODE_PORTalignedreload.shon the current macOS 26.3 + zig 0.15.2 host so tagged dev rebuilds complete locallyVerification
./scripts/reload.sh --tag omo-yanked-dep --launch./scripts/reload.sh --tag omo-yanked-depSummary by cubic
Fixes #2278 by isolating the
oh-my-opencodeinstall from user-pinned/yanked deps and choosing a free API port instead of always forcing 4096. Also auto-skips the Ghostty CLI helperzigbuild on macOS 26 +zig0.15.2 to keep dev reloads working.package.jsonin~/.cmuxterm/omo-config(no more symlinking userpackage.json/bun.lock) so yanked/stale pins can’t breakoh-my-opencodeinstall.bun add oh-my-opencode, then retry once after clearing the shadowbun.lockandnode_modules; fallback tonpm installifbunisn’t available.--portorOPENCODE_PORTwhen valid, otherwise prefer 4096 if free, else pick any free loopback port; setOPENCODE_PORTand inject--portto match so subagent attach works reliably.CMUX_SKIP_ZIG_BUILD=1and skip the Ghostty CLI helperzigbuild on macOS 26.x withzig0.15.2 (still respects a manual override).Written for commit b55bd15. Summary will update on new commits.
Summary by CodeRabbit