Repository navigation
Conversation
Update omoPluginName from 'oh-my-opencode' to 'oh-my-openagent' and add backward-compatible check for legacy plugin name. Every cmux omo run was appending 'oh-my-opencode' to the shadow config plugin array (since it compared against the stale constant). The oh-my-openagent plugin then detected this as a legacy entry and auto-migrated it back, creating an infinite warning loop. Also updates install messages and config file references to use the current package name, with fallback support for legacy filenames.
|
Someone is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughCLI/cmux.swift introduces dual plugin name support by adding a ChangesDual Plugin Name Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 13 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (13 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@CLI/cmux.swift`:
- Around line 12067-12068: Replace the hardcoded filenames
("oh-my-openagent.json", "oh-my-openagent.jsonc", "oh-my-opencode.json",
"oh-my-opencode.jsonc") with uses of the new constants by constructing filenames
from Self.omoPluginName and Self.legacyOmoPluginName plus the extensions
(".json" and ".jsonc"); e.g. use "\(Self.omoPluginName).json",
"\(Self.omoPluginName).jsonc", "\(Self.legacyOmoPluginName).json",
"\(Self.legacyOmoPluginName).jsonc" wherever those four literals appear
(including the for-loop array and the other occurrences mentioned) so the code
consistently uses the defined constants.
- Around line 12139-12150: The mutable optional collection "found" flagged by
SwiftLint (discouraged_optional_collection) should be replaced by a single
immutable lookup: compute a let found = userOmoConfigURLs.lazy.compactMap { url
in if let data = try? Data(contentsOf: url), let existing = try?
JSONSerialization.jsonObject(with: data) as? [String: Any] { return existing }
else { return nil } }.first and then use if let found { omoConfig = found; try?
fm.removeItem(at: omoConfigURL) } so you remove the var/loop/break and keep the
same behavior for omoConfig and fm.removeItem(at: omoConfigURL).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 7d7ad8c0-9d78-4c87-8110-e18f9a174838
📒 Files selected for processing (1)
CLI/cmux.swift
| for filename in ["oh-my-openagent.json", "oh-my-openagent.jsonc", | ||
| "oh-my-opencode.json", "oh-my-opencode.jsonc"] { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Use Self.omoPluginName / Self.legacyOmoPluginName constants instead of hardcoded string literals
The new constants are defined in hunk 1 but are then bypassed by hardcoded literals scattered across four sites. If the plugin name changes again the constants would be updated but these literals would silently diverge.
♻️ Proposed fix — replace literals with constants
- for filename in ["oh-my-openagent.json", "oh-my-openagent.jsonc",
- "oh-my-opencode.json", "oh-my-opencode.jsonc"] {
+ for filename in ["\(Self.omoPluginName).json", "\(Self.omoPluginName).jsonc",
+ "\(Self.legacyOmoPluginName).json", "\(Self.legacyOmoPluginName).jsonc"] {- let omoConfigURL = shadowDir.appendingPathComponent("oh-my-openagent.json")
+ let omoConfigURL = shadowDir.appendingPathComponent("\(Self.omoPluginName).json")- let candidate = shadowDir.appendingPathComponent("oh-my-opencode.json")
+ let candidate = shadowDir.appendingPathComponent("\(Self.legacyOmoPluginName).json")- let userOmoConfigURLs = [
- userDir.appendingPathComponent("oh-my-openagent.json"),
- userDir.appendingPathComponent("oh-my-opencode.json")
- ]
+ let userOmoConfigURLs = [
+ userDir.appendingPathComponent("\(Self.omoPluginName).json"),
+ userDir.appendingPathComponent("\(Self.legacyOmoPluginName).json")
+ ]Also applies to: 12121-12121, 12127-12127, 12136-12138
🤖 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 `@CLI/cmux.swift` around lines 12067 - 12068, Replace the hardcoded filenames
("oh-my-openagent.json", "oh-my-openagent.jsonc", "oh-my-opencode.json",
"oh-my-opencode.jsonc") with uses of the new constants by constructing filenames
from Self.omoPluginName and Self.legacyOmoPluginName plus the extensions
(".json" and ".jsonc"); e.g. use "\(Self.omoPluginName).json",
"\(Self.omoPluginName).jsonc", "\(Self.legacyOmoPluginName).json",
"\(Self.legacyOmoPluginName).jsonc" wherever those four literals appear
(including the for-loop array and the other occurrences mentioned) so the code
consistently uses the defined constants.
| var found: [String: Any]? | ||
| for url in userOmoConfigURLs { | ||
| if let data = try? Data(contentsOf: url), | ||
| let existing = try? JSONSerialization.jsonObject(with: data) as? [String: Any] { | ||
| found = existing | ||
| break | ||
| } | ||
| } | ||
| if let found { | ||
| omoConfig = found | ||
| // Remove the symlink so we can write our own copy | ||
| try? fm.removeItem(at: omoConfigURL) |
There was a problem hiding this comment.
Fix SwiftLint discouraged_optional_collection warning on var found: [String: Any]?
SwiftLint 0.63.2 flags this as a discouraged_optional_collection violation. The mutable optional can be replaced with a single lazy.compactMap.first expression, which also eliminates the var and the break.
♻️ Proposed fix
- var found: [String: Any]?
- for url in userOmoConfigURLs {
- if let data = try? Data(contentsOf: url),
- let existing = try? JSONSerialization.jsonObject(with: data) as? [String: Any] {
- found = existing
- break
- }
- }
- if let found {
+ let found = userOmoConfigURLs.lazy.compactMap { url -> [String: Any]? in
+ guard let data = try? Data(contentsOf: url) else { return nil }
+ return try? JSONSerialization.jsonObject(with: data) as? [String: Any]
+ }.first
+ if let found {
omoConfig = found
// Remove the symlink so we can write our own copy
try? fm.removeItem(at: omoConfigURL)🧰 Tools
🪛 SwiftLint (0.63.2)
[Warning] 12139-12139: Prefer empty collection over optional collection
(discouraged_optional_collection)
🤖 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 `@CLI/cmux.swift` around lines 12139 - 12150, The mutable optional collection
"found" flagged by SwiftLint (discouraged_optional_collection) should be
replaced by a single immutable lookup: compute a let found =
userOmoConfigURLs.lazy.compactMap { url in if let data = try? Data(contentsOf:
url), let existing = try? JSONSerialization.jsonObject(with: data) as? [String:
Any] { return existing } else { return nil } }.first and then use if let found {
omoConfig = found; try? fm.removeItem(at: omoConfigURL) } so you remove the
var/loop/break and keep the same behavior for omoConfig and fm.removeItem(at:
omoConfigURL).
Greptile SummaryThis PR fixes an infinite warning loop in
Confidence Score: 4/5The change is narrowly scoped to renaming the plugin constant and adding backward-compatible fallback paths; no control-flow or data-persistence logic is restructurally altered. The root-cause fix is correct and well-contained. Two style-level issues remain: hardcoded legacy name strings that bypass the new constant in the file-copy loop and one IIFE, and ad-hoc stderr writes that predate this PR but are actively modified here without being moved to unified logging. CLI/cmux.swift — the file-copy loop (lines 12067-12068) and the IIFE fallback (line 12127) use literal Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["cmux omo launched — omoEnsurePlugin()"] --> B["Read user opencode.json plugin list"]
B --> C{Contains oh-my-openagent?}
C -- Yes --> E["Skip append — no loop"]
C -- No --> D{Contains oh-my-opencode legacy?}
D -- Yes --> E
D -- No --> F["Append oh-my-openagent to shadow plugin list"]
F --> G["Write shadow opencode.json"]
E --> G
G --> H{"node_modules/oh-my-openagent exists?"}
H -- Yes --> I["Skip install"]
H -- No --> J["bun/npm add oh-my-openagent"]
J --> K["Read/write oh-my-openagent.json tmux config"]
I --> K
K --> L["setenv OPENCODE_CONFIG_DIR to shadow dir"]
Reviews (1): Last reviewed commit: "fix: prevent omo legacy plugin warning l..." | Re-trigger Greptile |
| for filename in ["oh-my-openagent.json", "oh-my-openagent.jsonc", | ||
| "oh-my-opencode.json", "oh-my-opencode.jsonc"] { |
There was a problem hiding this comment.
Hardcoded legacy name diverges from
legacyOmoPluginName constant
The file-copy loop uses the literal "oh-my-opencode" string instead of Self.legacyOmoPluginName. The same pattern appears inside the IIFE on line 12127 (shadowDir.appendingPathComponent("oh-my-opencode.json")). If the legacy name ever changes again, these two sites would need to be updated manually and are easy to miss — the whole value of the new constant is to have a single source of truth.
| @@ -12093,39 +12096,56 @@ struct CMUXCLI { | |||
| currentDirectoryURL: installDir | |||
| ) | |||
| if retryStatus != 0 { | |||
| throw CLIError(message: "Failed to install oh-my-opencode. Try manually: npm install -g oh-my-opencode") | |||
| throw CLIError(message: "Failed to install \(Self.omoPluginName). Try manually: npm install -g \(Self.omoPluginName)") | |||
| } | |||
| } | |||
| } else if let npmPath = resolveExecutableInPath("npm") { | |||
| FileHandle.standardError.write("Installing oh-my-opencode plugin (this may take a minute on first run)...\n".data(using: .utf8)!) | |||
| FileHandle.standardError.write("Installing \(Self.omoPluginName) 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") | |||
| throw CLIError(message: "Failed to install \(Self.omoPluginName). Try manually: npm install -g \(Self.omoPluginName)") | |||
| } | |||
| } else { | |||
| throw CLIError(message: "Neither bun nor npm found in PATH. Install oh-my-opencode manually: bunx oh-my-opencode install") | |||
| throw CLIError(message: "Neither bun nor npm found in PATH. Install \(Self.omoPluginName) manually: bunx \(Self.omoPluginName) install") | |||
| } | |||
| FileHandle.standardError.write("oh-my-opencode plugin installed\n".data(using: .utf8)!) | |||
| FileHandle.standardError.write("\(Self.omoPluginName) plugin installed\n".data(using: .utf8)!) | |||
There was a problem hiding this comment.
Ad-hoc stderr writes bypass unified logging
Every FileHandle.standardError.write(...) call in this block is raw stderr output, sidestepping os_log/Logger. This violates the project's unified-logging rule (no print/ad-hoc file/stdout logging in production Swift). These lines were pre-existing but are being actively modified by this PR, making them in-scope. The install progress messages should route through a Logger subsystem and category so they appear in Console.app, are filterable, and don't mix with other process output.
Rule Used: Flag production Swift diagnostics that bypass unif... (source)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thank you for this, @liyue2008! You had the fix first: the omo legacy plugin warning loop got fixed on main a few days after you opened this, in #3960, which checks both the new and legacy plugin names. So this one's covered. Really appreciate it :) |
Fixes an infinite warning loop where every 'cmux omo' run prints the legacy plugin migration warning.
Root Cause
The omoPluginName constant in CLI/cmux.swift was 'oh-my-opencode' (old name). On every launch, omoEnsurePlugin() appends it to the shadow config plugin array since the user config uses 'oh-my-openagent' (new name). The plugin then auto-migrates it back, creating a loop.
Fix
Summary by cubic
Prevents the infinite legacy plugin warning loop in
cmux omoby using the current plugin name and recognizing the old one. Users no longer see migration warnings on every run.omoPluginNametooh-my-openagent; addedlegacyOmoPluginName(oh-my-opencode).oh-my-openagent.json(.c)andoh-my-opencode.json(.c), writing to the current name.Written for commit a246017. Summary will update on new commits.
Summary by CodeRabbit