Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
267 changes: 210 additions & 57 deletions CLI/cmux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9688,6 +9688,163 @@ struct CMUXCLI {
.appendingPathComponent("omo-config", isDirectory: true)
}

private func omoFileType(at url: URL) -> FileAttributeType? {
let attrs = try? FileManager.default.attributesOfItem(atPath: url.path)
return attrs?[.type] as? FileAttributeType
}

private func omoEnsureShadowPackageManifest(at shadowPackageURL: URL) throws {
let fm = FileManager.default
if omoFileType(at: shadowPackageURL) == .typeSymbolicLink {
try? fm.removeItem(at: shadowPackageURL)
}

// 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.
Comment on lines +9702 to +9704

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Misleading inline 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.

let packageManifest: [String: Any] = [
"dependencies": [
Self.omoPluginName: "latest"
],
"name": "cmux-omo-shadow",
"private": true
]
let output = try JSONSerialization.data(withJSONObject: packageManifest, options: [.prettyPrinted, .sortedKeys])
let existing = try? Data(contentsOf: shadowPackageURL)
if existing != output {
try output.write(to: shadowPackageURL, options: .atomic)
}
}

private func omoEnsureShadowNodeModulesSymlink(
shadowNodeModules: URL,
userNodeModules: URL
) throws {
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)
}
Comment on lines +9723 to +9741

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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 omoRunPackageInstall(
executablePath: String,
arguments: [String],
currentDirectoryURL: URL
) throws -> Int32 {
let process = Process()
process.currentDirectoryURL = currentDirectoryURL
process.executableURL = URL(fileURLWithPath: executablePath)
process.arguments = arguments
process.standardOutput = FileHandle.standardError
process.standardError = FileHandle.standardError
try process.run()
process.waitUntilExit()
return process.terminationStatus
}

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
}
Comment on lines +9760 to +9776

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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().


private func omoBindableLoopbackPort(_ port: UInt16) -> UInt16? {
let socketDescriptor = socket(AF_INET, SOCK_STREAM, 0)
guard socketDescriptor >= 0 else { return nil }
defer { close(socketDescriptor) }

var reuseAddress: Int32 = 1
_ = setsockopt(
socketDescriptor,
SOL_SOCKET,
SO_REUSEADDR,
&reuseAddress,
socklen_t(MemoryLayout<Int32>.size)
)

var address = sockaddr_in()
address.sin_len = UInt8(MemoryLayout<sockaddr_in>.stride)
address.sin_family = sa_family_t(AF_INET)
address.sin_port = port.bigEndian
address.sin_addr = in_addr(s_addr: inet_addr("127.0.0.1"))

let bindResult = withUnsafePointer(to: &address) {
$0.withMemoryRebound(to: sockaddr.self, capacity: 1) {
Darwin.bind(socketDescriptor, $0, socklen_t(MemoryLayout<sockaddr_in>.stride))
}
}
guard bindResult == 0 else { return nil }

if port != 0 {
return port
}

var boundAddress = address
var boundAddressLength = socklen_t(MemoryLayout<sockaddr_in>.stride)
let nameResult = withUnsafeMutablePointer(to: &boundAddress) {
$0.withMemoryRebound(to: sockaddr.self, capacity: 1) {
getsockname(socketDescriptor, $0, &boundAddressLength)
}
}
guard nameResult == 0 else { return nil }

return UInt16(bigEndian: boundAddress.sin_port)
}

private func omoResolvedPort(
commandArgs: [String],
processEnvironment: [String: String]
) -> String {
if let requestedPort = omoRequestedPort(from: commandArgs) {
return requestedPort
}

if let environmentPort = processEnvironment["OPENCODE_PORT"]?
.trimmingCharacters(in: .whitespacesAndNewlines),
let parsedEnvironmentPort = UInt16(environmentPort),
parsedEnvironmentPort != 0,
omoBindableLoopbackPort(parsedEnvironmentPort) != nil {
return environmentPort
}

if let preferredPort = omoBindableLoopbackPort(4096) {
return String(preferredPort)
}

if let fallbackPort = omoBindableLoopbackPort(0) {
return String(fallbackPort)
}

return "4096"
Comment on lines +9838 to +9845

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Ephemeral port TOCTOU window

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)
}

}

/// Creates a shadow config directory that layers oh-my-opencode on top of the user's
/// existing opencode config without modifying the original. Sets OPENCODE_CONFIG_DIR
/// to point at the shadow directory.
Expand Down Expand Up @@ -9727,27 +9884,15 @@ struct CMUXCLI {
// Symlink node_modules from the user's config dir so installed packages resolve
let shadowNodeModules = shadowDir.appendingPathComponent("node_modules")
let userNodeModules = userDir.appendingPathComponent("node_modules")
if fm.fileExists(atPath: userNodeModules.path) {
// Remove stale symlink or directory if it exists
if let attrs = try? fm.attributesOfItem(atPath: shadowNodeModules.path),
attrs[.type] as? FileAttributeType == .typeSymbolicLink {
let target = try? fm.destinationOfSymbolicLink(atPath: shadowNodeModules.path)
if target != userNodeModules.path {
try? fm.removeItem(at: shadowNodeModules)
}
}
if !fm.fileExists(atPath: shadowNodeModules.path) {
try fm.createSymbolicLink(at: shadowNodeModules, withDestinationURL: userNodeModules)
}
}
try omoEnsureShadowNodeModulesSymlink(shadowNodeModules: shadowNodeModules, userNodeModules: userNodeModules)

// Symlink package.json and bun.lock so bun/npm can resolve in the shadow dir
for filename in ["package.json", "bun.lock"] {
let userFile = userDir.appendingPathComponent(filename)
let shadowFile = shadowDir.appendingPathComponent(filename)
if fm.fileExists(atPath: userFile.path) && !fm.fileExists(atPath: shadowFile.path) {
try fm.createSymbolicLink(at: shadowFile, withDestinationURL: userFile)
}
// The shadow config owns its own package metadata so yanked/stale pins in the
// user's opencode package.json/bun.lock cannot poison plugin installation.
let shadowPackageURL = shadowDir.appendingPathComponent("package.json")
let shadowBunLockURL = shadowDir.appendingPathComponent("bun.lock")
try omoEnsureShadowPackageManifest(at: shadowPackageURL)
if omoFileType(at: shadowBunLockURL) == .typeSymbolicLink {
try? fm.removeItem(at: shadowBunLockURL)
}

// Copy oh-my-opencode plugin config (jsonc) if the user has one
Expand All @@ -9762,37 +9907,43 @@ struct CMUXCLI {
// Install the package if not available via the symlinked node_modules
let pluginPackageDir = shadowNodeModules.appendingPathComponent(Self.omoPluginName)
if !fm.fileExists(atPath: pluginPackageDir.path) {
// Need to install into the real user config dir so the symlink picks it up
let installDir = fm.fileExists(atPath: userNodeModules.path) ? userDir : shadowDir
// If installing into shadow dir, remove the symlink first
if installDir == shadowDir {
try? fm.removeItem(at: shadowNodeModules)
}
let process = Process()
process.currentDirectoryURL = installDir
let installDir = shadowDir
if let bunPath = resolveExecutableInPath("bun") {
process.executableURL = URL(fileURLWithPath: bunPath)
process.arguments = ["add", Self.omoPluginName]
FileHandle.standardError.write("Installing oh-my-opencode plugin (this may take a minute on first run)...\n".data(using: .utf8)!)
let installArguments = ["add", Self.omoPluginName]
let firstAttemptStatus = try omoRunPackageInstall(
executablePath: bunPath,
arguments: installArguments,
currentDirectoryURL: installDir
)
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")
}
}
Comment on lines +9919 to +9932

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Retry may not clear partial 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)!)
    }
}

} 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")
}
Comment on lines +9929 to +9942

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

} else {
throw CLIError(message: "Neither bun nor npm found in PATH. Install oh-my-opencode manually: bunx oh-my-opencode install")
}
// Show install output directly so the user sees progress (npm can take 30s+)
process.standardOutput = FileHandle.standardError
process.standardError = FileHandle.standardError
FileHandle.standardError.write("Installing oh-my-opencode plugin (this may take a minute on first run)...\n".data(using: .utf8)!)
try process.run()
process.waitUntilExit()
if process.terminationStatus != 0 {
throw CLIError(message: "Failed to install oh-my-opencode. Try manually: npm install -g oh-my-opencode")
}
FileHandle.standardError.write("oh-my-opencode plugin installed\n".data(using: .utf8)!)
// Re-create symlink if we installed into user dir
if installDir == userDir && !fm.fileExists(atPath: shadowNodeModules.path) {
try fm.createSymbolicLink(at: shadowNodeModules, withDestinationURL: userNodeModules)
}
}

// Ensure tmux mode is enabled in oh-my-opencode config.
Expand Down Expand Up @@ -9857,13 +10008,9 @@ struct CMUXCLI {
executablePath: String,
socketPath: String,
explicitPassword: String?,
focusedContext: TmuxCompatFocusedContext?
focusedContext: TmuxCompatFocusedContext?,
openCodePort: String
) {
// Tell oh-my-opencode the API server port so subagent attach works
var extraEnvVars: [(key: String, value: String)] = []
if processEnvironment["OPENCODE_PORT"] == nil {
extraEnvVars.append((key: "OPENCODE_PORT", value: "4096"))
}
configureTmuxCompatEnvironment(
processEnvironment: processEnvironment,
shimDirectory: shimDirectory,
Expand All @@ -9874,7 +10021,7 @@ struct CMUXCLI {
tmuxPathPrefix: "cmux-omo",
cmuxBinEnvVar: "CMUX_OMO_CMUX_BIN",
termOverrideEnvVar: "CMUX_OMO_TERM",
extraEnvVars: extraEnvVars
extraEnvVars: [(key: "OPENCODE_PORT", value: openCodePort)]
)
}

Expand Down Expand Up @@ -9916,23 +10063,29 @@ struct CMUXCLI {
processEnvironment: launcherEnvironment,
explicitPassword: explicitPassword
)
let openCodePort = omoResolvedPort(
commandArgs: commandArgs,
processEnvironment: launcherEnvironment
)
launcherEnvironment["OPENCODE_PORT"] = openCodePort
configureOMOEnvironment(
processEnvironment: launcherEnvironment,
shimDirectory: shimDirectory,
executablePath: executablePath,
socketPath: socketPath,
explicitPassword: explicitPassword,
focusedContext: focusedContext
focusedContext: focusedContext,
openCodePort: openCodePort
)

let launchPath = openCodeExecutablePath ?? "opencode"
// oh-my-openagent needs the OpenCode API server running to attach
// subagent sessions to tmux panes. Inject --port 4096 unless the
// user already specified a port.
// subagent sessions to tmux panes. Prefer the historic default port
// when it is available, otherwise fall back to a free loopback port.
var effectiveArgs = commandArgs
if !commandArgs.contains("--port") {
if omoRequestedPort(from: commandArgs) == nil {
effectiveArgs.append("--port")
effectiveArgs.append("4096")
effectiveArgs.append(openCodePort)
}
var argv = ([launchPath] + effectiveArgs).map { strdup($0) }
defer {
Expand Down
34 changes: 33 additions & 1 deletion scripts/reload.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,27 @@ CMUX_DEBUG_LOG=""
CLI_PATH=""
LAST_SOCKET_PATH_DIR="$HOME/Library/Application Support/cmux"
LAST_SOCKET_PATH_FILE="${LAST_SOCKET_PATH_DIR}/last-socket-path"
AUTO_SKIP_ZIG_BUILD_REASON=""

should_skip_ghostty_cli_helper_zig_build() {
if [[ "${CMUX_SKIP_ZIG_BUILD:-}" == "1" ]]; then
AUTO_SKIP_ZIG_BUILD_REASON="CMUX_SKIP_ZIG_BUILD=1"
return 0
fi

local product_version zig_version major_version
product_version="$(sw_vers -productVersion 2>/dev/null || true)"
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Exact-version match will silently stop applying on next zig patch

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

AUTO_SKIP_ZIG_BUILD_REASON="macOS ${product_version} + zig ${zig_version}"
return 0
fi

AUTO_SKIP_ZIG_BUILD_REASON=""
return 1
}

write_dev_cli_shim() {
local target="$1"
Expand Down Expand Up @@ -258,6 +279,13 @@ if [[ -z "$TAG" ]]; then
exit 1
fi

if should_skip_ghostty_cli_helper_zig_build; then
if [[ "${CMUX_SKIP_ZIG_BUILD:-}" != "1" ]]; then
echo "Auto-enabling CMUX_SKIP_ZIG_BUILD=1 for Ghostty CLI helper (${AUTO_SKIP_ZIG_BUILD_REASON})"
fi
export CMUX_SKIP_ZIG_BUILD=1
fi

if [[ -n "$TAG" ]]; then
TAG_ID="$(sanitize_bundle "$TAG")"
TAG_SLUG="$(sanitize_path "$TAG")"
Expand Down Expand Up @@ -427,7 +455,11 @@ if [[ -d "$PWD/cmuxd" ]]; then
(cd "$PWD/cmuxd" && zig build -Doptimize=ReleaseFast)
fi
if [[ -d "$PWD/ghostty" ]]; then
(cd "$PWD/ghostty" && zig build cli-helper -Dapp-runtime=none -Demit-macos-app=false -Demit-xcframework=false -Doptimize=ReleaseFast)
if [[ "${CMUX_SKIP_ZIG_BUILD:-}" == "1" ]]; then
echo "Skipping direct ghostty CLI helper zig build (CMUX_SKIP_ZIG_BUILD=1)"
else
(cd "$PWD/ghostty" && zig build cli-helper -Dapp-runtime=none -Demit-macos-app=false -Demit-xcframework=false -Doptimize=ReleaseFast)
fi
fi
if [[ -x "$CMUXD_SRC" ]]; then
BIN_DIR="$APP_PATH/Contents/Resources/bin"
Expand Down
Loading