feat: add built-in OpenCode status integration#1426
feat: add built-in OpenCode status integration#1426coygeek wants to merge 16 commits intomanaflow-ai:mainfrom
Conversation
Ship a bundled opencode wrapper that injects a local cmux plugin when OpenCode runs inside a cmux terminal. Translate OpenCode session, permission, question, retry, idle, and error events into sidebar status pills and notifications so OpenCode sessions expose the same working-state cues as Claude Code.
|
@coygeek is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a built-in OpenCode cmux integration: a wrapper injects a bundled OpenCode plugin when a live cmux socket is detected; the plugin maps OpenCode events to Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Wrapper as OpenCode Wrapper
participant OC as OpenCode App
participant Plugin as Cmux Plugin
participant Cmux as Cmux Socket
participant Sidebar as Sidebar UI
User->>Wrapper: run `opencode`
Wrapper->>Wrapper: probe cmux socket
alt socket live
Wrapper->>Wrapper: create temp OPENCODE_CONFIG_DIR (symlink existing config), copy plugin
else socket missing/stale
Wrapper->>Wrapper: skip injection
end
Wrapper->>OC: exec with OPENCODE_CONFIG_DIR
OC->>Plugin: emit session.*, permission.*, question.*, error events
Plugin->>Cmux: cmux set-status / cmux notify / cmux clear-notifications
Cmux->>Sidebar: update sidebar status & notifications
OC->>Plugin: emit session.deleted
Plugin->>Cmux: cmux clear-status
Wrapper->>Wrapper: cleanup temp config
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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}
There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/notifications.md">
<violation number="1" location="docs/notifications.md:124">
P2: The manual plugin example can throw when `cmux` is unavailable because it lacks a fallback path. Add the documented macOS fallback so the event hook remains resilient outside cmux.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
CLI/cmux.swift (1)
1736-1751: Validate--pidas a positive integer before sending.Right now
--pidis accepted as any string and forwarded. Invalid values will silently bypass PID tracking semantics.Suggested patch
- let (pid, r3) = parseOption(r2, name: "--pid") + let (pidRaw, r3) = parseOption(r2, name: "--pid") + let pid: Int? + if let pidRaw { + guard let parsedPid = Int(pidRaw), parsedPid > 0 else { + throw CLIError(message: "--pid must be a positive integer") + } + pid = parsedPid + } else { + pid = nil + } let (wsFlag, r4) = parseOption(r3, name: "--workspace") @@ - if let pid { socketCmd += " --pid=\(socketQuote(pid))" } + if let pid { socketCmd += " --pid=\(pid)" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 1736 - 1751, The --pid option is currently forwarded as any string; update the parsing/validation after let (pid, r3) = parseOption(r2, name: "--pid") to ensure, when pid is non-nil, it represents a positive integer (e.g., attempt to convert to Int and check > 0) and throw a CLIError with a clear message if it’s invalid; after validation, continue using the validated pid value when appending to socketCmd (the set_status command) so only a valid PID is included, keeping existing flows for workspace resolution via resolveWorkspaceId and socketQuote usage for other fields.
🤖 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 5257-5258: The top-level usage() output and the set-status
subcommand help are inconsistent because usage() omits the newly added --pid
flag; update the usage() function to include the --pid <pid> description
(matching the subcommand help) so both help surfaces present the same options,
locating the usage() method and the set-status subcommand definition in
CLI/cmux.swift to mirror the exact text "--pid <pid> Track a process
for stale-pill cleanup".
In `@docs/notifications.md`:
- Around line 117-127: The provided manual fallback only emits session.idle and
misquotes the shell body; update the example so it matches the full integration
behavior (emit Running/Retrying/Needs input/Error mappings) by either replacing
the snippet with the full CmuxNotificationPlugin implementation from
Resources/bin/opencode-cmux-plugin.js or inlining the complete plugin logic
(keep the exported symbol CmuxNotificationPlugin and its event handler), and
ensure shell invocations quote the body (e.g., wrap the --body argument in
quotes) so multi-word messages aren’t split.
In `@Resources/bin/opencode`:
- Around line 73-98: Check that CMUX_OPENCODE_CONFIG_DIR is non-empty and a
valid directory immediately after the mktemp -d call (and before any
overlays/links are created); if mktemp failed, exec the real opencode binary
(use ${OPENCODE_BIN:-opencode}) to bail out rather than continuing. Likewise,
verify mkdir -p "$CMUX_OPENCODE_CONFIG_DIR/plugins" and cp "$PLUGIN_SOURCE" ...
succeed; on any failure restore/cleanup and exec the real binary instead of
proceeding. Keep the existing cleanup() and trap behavior so any created temp
dir is removed on exit, and make these checks around the
CMUX_OPENCODE_CONFIG_DIR, mkdir, and cp steps (reference symbols:
CMUX_OPENCODE_CONFIG_DIR, cleanup(), mkdir, cp, PLUGIN_SOURCE, OPENCODE_BIN).
In `@Resources/bin/opencode-cmux-plugin.js`:
- Around line 90-118: The cleanup helpers currently call global clears and can
erase other sessions; change clearNotifications() and clearStatus() to only
clear items scoped to this process by using the pid used in setStatus()
(parse/store the pid from the applied value or keep a currentPid variable) and
call cmux with the same --pid option (e.g. `cmux clear-notifications --pid
<pid>` and `cmux clear-status opencode --pid <pid>`), and also guard clears so
they run only when the stored pid matches process.pid and attention/applied
correspond to this process; remove any global clear paths in wrapper cleanup
that don't supply or check the pid.
---
Nitpick comments:
In `@CLI/cmux.swift`:
- Around line 1736-1751: The --pid option is currently forwarded as any string;
update the parsing/validation after let (pid, r3) = parseOption(r2, name:
"--pid") to ensure, when pid is non-nil, it represents a positive integer (e.g.,
attempt to convert to Int and check > 0) and throw a CLIError with a clear
message if it’s invalid; after validation, continue using the validated pid
value when appending to socketCmd (the set_status command) so only a valid PID
is included, keeping existing flows for workspace resolution via
resolveWorkspaceId and socketQuote usage for other fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 39e7f30b-4f83-411a-9772-43d11ae144a4
📒 Files selected for processing (8)
CLI/cmux.swiftGhosttyTabs.xcodeproj/project.pbxprojResources/bin/opencodeResources/bin/opencode-cmux-plugin.jsTODO.mddocs/notifications.mdtests/test_opencode_plugin_runtime.jstests/test_opencode_wrapper_hooks.py
| async function clearNotifications() { | ||
| if (!attention) return | ||
| try { | ||
| await $`cmux clear-notifications` | ||
| attention = false | ||
| } catch {} | ||
| } | ||
|
|
||
| async function setStatus(value, icon, color) { | ||
| const pid = typeof process.pid === "number" && process.pid > 0 ? process.pid : 0 | ||
| const next = `${value}\u0000${icon}\u0000${color}\u0000${pid}` | ||
| if (applied === next) return | ||
| try { | ||
| if (pid > 0) { | ||
| await $`cmux set-status opencode ${value} --icon ${icon} --color ${color} --pid ${pid}` | ||
| } else { | ||
| await $`cmux set-status opencode ${value} --icon ${icon} --color ${color}` | ||
| } | ||
| applied = next | ||
| } catch {} | ||
| } | ||
|
|
||
| async function clearStatus() { | ||
| if (!applied) return | ||
| try { | ||
| await $`cmux clear-status opencode` | ||
| applied = "" | ||
| } catch {} | ||
| } |
There was a problem hiding this comment.
These cleanup helpers can erase another session’s UI state.
clearNotifications() wipes the whole workspace, and clearStatus() is not tied to the --pid used in setStatus(). That means one OpenCode process resuming or exiting can clear another live OpenCode session’s pill or another agent’s notification in the same workspace. The wrapper cleanup path has the same side effect. Please switch to source/pid-scoped cleanup, or rely on stale-PID cleanup instead of issuing global clears.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Resources/bin/opencode-cmux-plugin.js` around lines 90 - 118, The cleanup
helpers currently call global clears and can erase other sessions; change
clearNotifications() and clearStatus() to only clear items scoped to this
process by using the pid used in setStatus() (parse/store the pid from the
applied value or keep a currentPid variable) and call cmux with the same --pid
option (e.g. `cmux clear-notifications --pid <pid>` and `cmux clear-status
opencode --pid <pid>`), and also guard clears so they run only when the stored
pid matches process.pid and attention/applied correspond to this process; remove
any global clear paths in wrapper cleanup that don't supply or check the pid.
- Validate --pid as a positive integer in set-status CLI command - Add --pid to top-level usage() help to match subcommand help - Guard mktemp/mkdir/cp failures in opencode wrapper with fallback - Fix shell quoting and add osascript fallback in docs manual example - Add reference to bundled plugin for full integration in docs - Document workspace-global clear limitation in plugin comments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review feedback addressed in ece7e89Investigated all bot review comments. Here's what was fixed vs. what was intentionally left as-is. Fixed
Not fixed (by design)
Test resultsAll three test suites pass after the changes:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Resources/bin/opencode-cmux-plugin.js (1)
90-98:⚠️ Potential issue | 🟠 MajorLine 95 and Line 119 still clear workspace-global UI state.
setStatus()is pid-scoped, butclear-notificationsandclear-status opencodeare not. When this process leaves an attention state or drops its last session, it can erase another live OpenCode process's notification or status in the same workspace. Please keep cleanup pid/source-scoped, or skip the global clears and rely on stale-PID cleanup instead.Also applies to: 114-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/bin/opencode-cmux-plugin.js` around lines 90 - 98, The current cleanup commands (in clearNotifications() and the similar cleanup block around the clear-status / clear-notifications calls) invoke workspace-global cmux commands and can wipe other processes' UI state; change these to use pid/source-scoped clears if cmux supports that (e.g., call cmux clear-notifications and clear-status with the current PID/source flag) by updating clearNotifications() and the corresponding cleanup section (search for clearNotifications and any clear-status or clear-notifications invocations), or if cmux has no PID-scoping option, remove/skip emitting those global clear commands and rely on the existing stale-PID cleanup path instead so this process does not clear another live OpenCode instance's status/notifications.
🧹 Nitpick comments (2)
.gitignore (2)
89-91: Redundant.DS_Storepatterns.Lines 90-91 duplicate the existing
.DS_Storepattern on line 9. Gitignore patterns are recursive by default, so.DS_Storealready matches these files in all directories. The**/.DS_Storeand.DS_Store?patterns add no additional coverage.♻️ Suggested cleanup
Remove the redundant patterns:
# Os -**/.DS_Store -.DS_Store? ._*The existing line 9
.DS_Storepattern is sufficient.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 89 - 91, Remove the redundant .gitignore patterns: delete the duplicate entries "**/.DS_Store" and ".DS_Store?" and keep the single ".DS_Store" pattern (the existing ".DS_Store" already matches files recursively), so update the file by removing those two extra patterns referenced in the diff.
101-104: Consolidate duplicate Python bytecode patterns.Lines 102-104 overlap with existing patterns on lines 29-30. The
*.py[cod]pattern on line 102 already matches*.pycand*.pyo(covered on lines 29-30), and line 103's*.pydis also covered by the bracket expression[cod].♻️ Suggested consolidation
Option 1: Keep the original section and remove redundant new patterns:
# Python Bytecode -*.py[cod] -*.pyd *$py.classOption 2: Consolidate into the new section and remove from the original:
Remove lines 29-30 from the original Python section and keep lines 102-104, but drop line 103:
# Python Bytecode *.py[cod] -*.pyd *$py.class🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 101 - 104, Consolidate the duplicate Python bytecode patterns by keeping the bracket pattern '*.py[cod]' as the canonical entry and removing redundant explicit patterns like '*.pyc' and '*.pyo' and the duplicate '*.pyd' elsewhere; also ensure '*$py.class' appears only once (remove its duplicate). Update the gitignore so only the unique patterns ('*.py[cod]', '*.pyd' if you choose to explicitly keep it, and '*$py.class') remain and remove any overlapping entries to avoid duplication.
🤖 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 1739-1743: The CLI currently parses --pid into a Swift Int (using
pidRaw -> parsedPid) but the server expects an Int32, so change the parsing to
parse and validate as Int32 (e.g., attempt Int32(pidRaw)), ensure the parsed
value is > 0 and within Int32.min..Int32.max, and on failure throw the same
CLIError message; assign the Int32 value to the existing pid variable (or
convert explicitly where needed) so the client only sends PIDs the server can
accept (refer to symbols pidRaw, parsedPid/pid and CLIError in cmux.swift).
---
Duplicate comments:
In `@Resources/bin/opencode-cmux-plugin.js`:
- Around line 90-98: The current cleanup commands (in clearNotifications() and
the similar cleanup block around the clear-status / clear-notifications calls)
invoke workspace-global cmux commands and can wipe other processes' UI state;
change these to use pid/source-scoped clears if cmux supports that (e.g., call
cmux clear-notifications and clear-status with the current PID/source flag) by
updating clearNotifications() and the corresponding cleanup section (search for
clearNotifications and any clear-status or clear-notifications invocations), or
if cmux has no PID-scoping option, remove/skip emitting those global clear
commands and rely on the existing stale-PID cleanup path instead so this process
does not clear another live OpenCode instance's status/notifications.
---
Nitpick comments:
In @.gitignore:
- Around line 89-91: Remove the redundant .gitignore patterns: delete the
duplicate entries "**/.DS_Store" and ".DS_Store?" and keep the single
".DS_Store" pattern (the existing ".DS_Store" already matches files
recursively), so update the file by removing those two extra patterns referenced
in the diff.
- Around line 101-104: Consolidate the duplicate Python bytecode patterns by
keeping the bracket pattern '*.py[cod]' as the canonical entry and removing
redundant explicit patterns like '*.pyc' and '*.pyo' and the duplicate '*.pyd'
elsewhere; also ensure '*$py.class' appears only once (remove its duplicate).
Update the gitignore so only the unique patterns ('*.py[cod]', '*.pyd' if you
choose to explicitly keep it, and '*$py.class') remain and remove any
overlapping entries to avoid duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bbd21ff2-7c04-4827-9204-3276b37b4be0
📒 Files selected for processing (5)
.gitignoreCLI/cmux.swiftResources/bin/opencodeResources/bin/opencode-cmux-plugin.jsdocs/notifications.md
🚧 Files skipped from review as they are similar to previous changes (1)
- Resources/bin/opencode
Server-side (TerminalController.swift:13072) parses --pid as Int32. CLI was accepting any positive Int, meaning values exceeding Int32.max would be silently rejected by the server without error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review feedback addressed in c80abe6Fixed
Test resultsAll three test suites pass after the change:
|
|
Woohoo! I cannot wait to see this one merged, if it works! |
Detects busy->idle transition in session.status events and sends a "Done: Session completed" notification with attention flag. This creates an unread notification badge similar to Claude Code behavior.
Add a 5-second cooldown to prevent completion notifications from flashing and disappearing immediately when OpenCode rapidly transitions between states.
Track completed sessions with a 'completed' flag. When a session transitions from busy to idle, mark it as completed and show a persistent 'Done' status in the sidebar that stays until the session is cleared (on new session or deletion). This mimics the Claude Code behavior where completion notifications remain until you switch to that workspace/terminal.
Fix indentation bug that was causing notify() and sync() to always execute regardless of the busy->idle transition condition, potentially causing phantom notifications.
Prevent phantom 'Done' notifications by resetting the completed flag when a session transitions from idle to busy (starting new work). This ensures completion notifications only appear for actual work completion, not for every idle state transition.
Remove accidentally duplicated code block that was causing the plugin to stop processing events after session.status handling, which broke all other event types (permissions, questions, etc.).
Clear waiting text when transitioning from idle to busy state (starting work or resuming from permission/question) to prevent phantom notifications from stale waiting text.
cmux CLI commands (set-status, notify, clear-notifications, clear-status) print 'OK' to stdout on success. When executed via Bun Shell's $ tagged template, this output leaks into the terminal causing phantom 'OK' notifications and TUI flickering. Use Bun Shell's .quiet() method to suppress stdout/stderr while still executing commands normally.
…anaflow-ai#1973) * test: cover queued restore-time terminal portal shift * fix: resync terminal portal after restore-time bind
fix: persistent completion notifications + suppress phantom OK text
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Resources/bin/opencode-cmux-plugin.js (1)
93-101:⚠️ Potential issue | 🟠 MajorThe cleanup helpers are still workspace-global.
clear-notificationsandclear-status opencodecan still erase another live OpenCode instance’s prompt or pill in the same workspace. SincesetStatus()is already pid-scoped, the safer fallback is to avoid proactive clears until cmux grows scoped clear APIs.Based on learnings, notifications are automatically cleared by cmux when the user navigates to the relevant tab. There is no need to explicitly call
cmux clear-notificationsin agent hook scripts.Also applies to: 117-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/bin/opencode-cmux-plugin.js` around lines 93 - 101, The cleanup helpers are using workspace-global cmux clears which can erase another OpenCode instance; remove any proactive calls to `cmux clear-notifications` and `cmux clear-status opencode` and stop invoking the `clearNotifications()` helper (or turn it into a no-op), relying on cmux's automatic tab-aware clearing and the pid-scoped `setStatus()` behavior instead; update references to the `clearNotifications` function and any explicit `clear-status opencode` invocations so they no longer perform global clears.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Resources/bin/opencode-cmux-plugin.js`:
- Around line 73-76: The code currently returns a new pill state "Done" in the
completed-sessions check (the items.some((item) => item.completed) branch);
change that to reuse the existing "Idle" state so we don't add a sixth
state—return { value: "Idle", icon: "checkmark.circle.fill", color: BLUE,
attention: true } (keep attention:true and the same icon/color) so completion
still triggers attention without introducing a new pill value.
---
Duplicate comments:
In `@Resources/bin/opencode-cmux-plugin.js`:
- Around line 93-101: The cleanup helpers are using workspace-global cmux clears
which can erase another OpenCode instance; remove any proactive calls to `cmux
clear-notifications` and `cmux clear-status opencode` and stop invoking the
`clearNotifications()` helper (or turn it into a no-op), relying on cmux's
automatic tab-aware clearing and the pid-scoped `setStatus()` behavior instead;
update references to the `clearNotifications` function and any explicit
`clear-status opencode` invocations so they no longer perform global clears.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 34de6488-b95e-457f-8dcf-5067cd23e157
📒 Files selected for processing (2)
Resources/bin/opencode-cmux-plugin.jstests/test_opencode_plugin_runtime.js
✅ Files skipped from review due to trivial changes (1)
- tests/test_opencode_plugin_runtime.js
Also addresses CodeRabbit review feedback on Eric's completion changes: - Replace "Done" sixth state with "Idle" + attention:true (blue ring) - Merge duplicate idle->busy condition blocks - Fix inconsistent indentation (normalize to 2-space) - Fix misaligned else block in setStatus()
|
I need this! |
|
I'd love to see this go live too! |
|
I have built the PR branch locally, fyi, and the effect is this: 
Confirming it works and looks great! |
|
@greptile-apps review |
@coygeek I have started the AI code review. It will take a few minutes to complete. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
1 issue found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".gitignore">
<violation number="1" location=".gitignore:130">
P2: Broad `.env.*` ignore rule also ignores common template files like `.env.example`, risking accidental exclusion of onboarding env templates.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| vendor/bundle/ | ||
|
|
||
| # Security | ||
| .env.* |
There was a problem hiding this comment.
P2: Broad .env.* ignore rule also ignores common template files like .env.example, risking accidental exclusion of onboarding env templates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .gitignore, line 130:
<comment>Broad `.env.*` ignore rule also ignores common template files like `.env.example`, risking accidental exclusion of onboarding env templates.</comment>
<file context>
@@ -47,3 +47,92 @@ tests/visual_report.html
+vendor/bundle/
+
+# Security
+.env.*
+*.pem
+*.key
</file context>
| .env.* | |
| .env.* | |
| !**/.env.example | |
| !**/.env.sample |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
cmuxTests/TerminalAndGhosttyTests.swift (1)
2987-2994: Avoid a time-based queue flush in this regression test.The bind path under test queues multiple main-queue turns, so a fixed
0.05second run-loop wait can still race on a busy CI runner. This test case already hasdrainMainQueue(), and calling it twice makes the sequencing explicit.♻️ Proposed tweak
DispatchQueue.main.async { shiftedContainer.frame.origin.x += 72 contentView.layoutSubtreeIfNeeded() window.displayIfNeeded() } - RunLoop.current.run(until: Date().addingTimeInterval(0.05)) + drainMainQueue() + drainMainQueue()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/TerminalAndGhosttyTests.swift` around lines 2987 - 2994, Replace the ad-hoc time-based RunLoop delay with explicit main-queue drainage: instead of calling RunLoop.current.run(until: Date().addingTimeInterval(0.05)) after the DispatchQueue.main.async block that mutates shiftedContainer.frame and calls contentView.layoutSubtreeIfNeeded() and window.displayIfNeeded(), call drainMainQueue() twice (or call drainMainQueue() immediately after the async block and again to ensure any queued follow-up work runs) so the test deterministically flushes the main queue without relying on a fixed timeout.
🤖 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 2248-2360: There are duplicate switch cases for
set-status/clear-status/list-status/set-progress/clear-progress/log/clear-log/list-log/sidebar-state
causing a compile error and the new --pid handling in the second block never
reaches the delegated path; remove the duplicate block and consolidate behavior
by either (A) updating forwardSidebarMetadataCommand(...) to parse "--pid" (use
parseOption to capture pidRaw, validate with Int32(pidRaw) > 0, include "
--pid=\(pid)" in the socket command built before calling sendV1Command) or (B)
delete the forwardSidebarMetadataCommand calls for these cases and keep the
single direct implementations (using parseOption, resolveWorkspaceId,
socketQuote and sendV1Command) that include the --pid validation; ensure only
one implementation per case label and that sendV1Command/socket command
construction includes the validated pid when present.
In `@Sources/AppDelegate.swift`:
- Around line 2420-2439: stabilizeUITestLaunchWindowAndForeground currently
considers any NSApp window as satisfying readiness, causing false positives when
non-main windows (settings/import/debug) appear; change the fallback creation
and all readiness checks to only consider main terminal windows by using
isMainTerminalWindow(_:). Specifically, when creating a fallback in the initial
if-block call openNewMainWindow only if no main terminal window exists (filter
NSApp.windows with isMainTerminalWindow), and replace
hasWindow/hasVisibleWindow/hasKeyWindow checks with checks that operate on
NSApp.windows.filter(isMainTerminalWindow) (e.g., any main window exists, any
main window isVisible, NSApp.keyWindow is a main terminal window). Keep calls to
moveUITestWindowToTargetDisplayIfNeeded, activateUITestAppIfNeeded, and
writeUITestDiagnosticsIfNeeded but ensure they operate or log with respect to
main terminal windows only.
- Around line 2447-2450: The macOS 14+ branch drops .activateIgnoringOtherApps
causing weaker activation; to fix, before calling
NSRunningApplication.current.activate in the macOS 14+ branch call
NSApp.yieldActivation(to: NSRunningApplication.current) and then invoke
NSRunningApplication.current.activate(options: [.activateAllWindows,
.activateIgnoringOtherApps]) so behavior matches the other activation sites
(compare with other activate(...) calls) while also performing Apple's
recommended cooperative yield first.
---
Nitpick comments:
In `@cmuxTests/TerminalAndGhosttyTests.swift`:
- Around line 2987-2994: Replace the ad-hoc time-based RunLoop delay with
explicit main-queue drainage: instead of calling RunLoop.current.run(until:
Date().addingTimeInterval(0.05)) after the DispatchQueue.main.async block that
mutates shiftedContainer.frame and calls contentView.layoutSubtreeIfNeeded() and
window.displayIfNeeded(), call drainMainQueue() twice (or call drainMainQueue()
immediately after the async block and again to ensure any queued follow-up work
runs) so the test deterministically flushes the main queue without relying on a
fixed timeout.
🪄 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: 014ac9cf-d899-4efb-9362-7bd5019f9d9a
📒 Files selected for processing (8)
.github/workflows/ci.ymlCLI/cmux.swiftGhosttyTabs.xcodeproj/project.pbxprojResources/bin/opencode-cmux-plugin.jsSources/AppDelegate.swiftSources/TerminalWindowPortal.swiftTODO.mdcmuxTests/TerminalAndGhosttyTests.swift
✅ Files skipped from review due to trivial changes (3)
- TODO.md
- GhosttyTabs.xcodeproj/project.pbxproj
- Resources/bin/opencode-cmux-plugin.js
| case "set-status": | ||
| let (icon, r1) = parseOption(commandArgs, name: "--icon") | ||
| let (color, r2) = parseOption(r1, name: "--color") | ||
| let (pidRaw, r3) = parseOption(r2, name: "--pid") | ||
| let pid: Int32? | ||
| if let pidRaw { | ||
| guard let parsedPid = Int32(pidRaw), parsedPid > 0 else { | ||
| throw CLIError(message: "--pid must be a positive integer") | ||
| } | ||
| pid = parsedPid | ||
| } else { | ||
| pid = nil | ||
| } | ||
| let (wsFlag, r4) = parseOption(r3, name: "--workspace") | ||
| guard r4.count >= 2 else { | ||
| throw CLIError(message: "set-status requires <key> and <value>") | ||
| } | ||
| let key = r4[0] | ||
| let value = r4.dropFirst().joined(separator: " ") | ||
| guard !value.isEmpty else { | ||
| throw CLIError(message: "set-status requires a non-empty value") | ||
| } | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| var socketCmd = "set_status \(key) \(socketQuote(value))" | ||
| if let icon { socketCmd += " --icon=\(socketQuote(icon))" } | ||
| if let color { socketCmd += " --color=\(socketQuote(color))" } | ||
| if let pid { socketCmd += " --pid=\(pid)" } | ||
| socketCmd += " --tab=\(wsId)" | ||
| let response = try sendV1Command(socketCmd, client: client) | ||
| print(response) | ||
|
|
||
| case "clear-status": | ||
| let (wsFlag, csRemaining) = parseOption(commandArgs, name: "--workspace") | ||
| guard let key = csRemaining.first else { | ||
| throw CLIError(message: "clear-status requires a <key>") | ||
| } | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| let response = try sendV1Command("clear_status \(key) --tab=\(wsId)", client: client) | ||
| print(response) | ||
|
|
||
| case "list-status": | ||
| let (wsFlag, _) = parseOption(commandArgs, name: "--workspace") | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| let response = try sendV1Command("list_status --tab=\(wsId)", client: client) | ||
| print(response) | ||
|
|
||
| case "set-progress": | ||
| let (label, spR1) = parseOption(commandArgs, name: "--label") | ||
| let (wsFlag, spR2) = parseOption(spR1, name: "--workspace") | ||
| guard let valueStr = spR2.first else { | ||
| throw CLIError(message: "set-progress requires a progress value (0.0-1.0)") | ||
| } | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| var socketCmd = "set_progress \(valueStr)" | ||
| if let label { socketCmd += " --label=\(socketQuote(label))" } | ||
| socketCmd += " --tab=\(wsId)" | ||
| let response = try sendV1Command(socketCmd, client: client) | ||
| print(response) | ||
|
|
||
| case "clear-progress": | ||
| let (wsFlag, _) = parseOption(commandArgs, name: "--workspace") | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| let response = try sendV1Command("clear_progress --tab=\(wsId)", client: client) | ||
| print(response) | ||
|
|
||
| case "log": | ||
| let (level, r1) = parseOption(commandArgs, name: "--level") | ||
| let (source, r2) = parseOption(r1, name: "--source") | ||
| let (wsFlag, r3) = parseOption(r2, name: "--workspace") | ||
| // Strip leading "--" separator if present | ||
| let positional = r3.first == "--" ? Array(r3.dropFirst()) : r3 | ||
| let message = positional.joined(separator: " ") | ||
| guard !message.isEmpty else { | ||
| throw CLIError(message: "log requires a message") | ||
| } | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| var socketCmd = "log" | ||
| if let level { socketCmd += " --level=\(level)" } | ||
| if let source { socketCmd += " --source=\(socketQuote(source))" } | ||
| socketCmd += " --tab=\(wsId) -- \(socketQuote(message))" | ||
| let response = try sendV1Command(socketCmd, client: client) | ||
| print(response) | ||
|
|
||
| case "clear-log": | ||
| let (wsFlag, _) = parseOption(commandArgs, name: "--workspace") | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| let response = try sendV1Command("clear_log --tab=\(wsId)", client: client) | ||
| print(response) | ||
|
|
||
| case "list-log": | ||
| let (limitStr, r1) = parseOption(commandArgs, name: "--limit") | ||
| let (wsFlag, _) = parseOption(r1, name: "--workspace") | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| var socketCmd = "list_log" | ||
| if let limitStr { socketCmd += " --limit=\(limitStr)" } | ||
| socketCmd += " --tab=\(wsId)" | ||
| let response = try sendV1Command(socketCmd, client: client) | ||
| print(response) | ||
|
|
||
| case "sidebar-state": | ||
| let (wsFlag, _) = parseOption(commandArgs, name: "--workspace") | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| let response = try sendV1Command("sidebar_state --tab=\(wsId)", client: client) | ||
| print(response) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect each sidebar metadata command below to be reported twice.
python3 - <<'PY'
from pathlib import Path
import re
path = Path("CLI/cmux.swift")
targets = {
"set-status", "clear-status", "list-status",
"set-progress", "clear-progress",
"log", "clear-log", "list-log", "sidebar-state",
}
hits = {}
for lineno, line in enumerate(path.read_text().splitlines(), 1):
m = re.match(r'\s*case\s+"([^"]+)":', line)
if m and m.group(1) in targets:
hits.setdefault(m.group(1), []).append(lineno)
for name in sorted(targets):
lines = hits.get(name, [])
if len(lines) > 1:
print(f"{name}: duplicate case labels at lines {lines}")
PYRepository: manaflow-ai/cmux
Length of output: 619
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show context around each duplicate set to understand the switch structure
echo "=== CONTEXT AROUND FIRST SET (2156-2165 for set-status) ==="
sed -n '2145,2175p' CLI/cmux.swift
echo ""
echo "=== CONTEXT AROUND SECOND SET (2248-2260 for set-status) ==="
sed -n '2240,2270p' CLI/cmux.swift
echo ""
echo "=== CONTEXT AROUND THIRD SET (7001-7010 for set-status) ==="
sed -n '6990,7020p' CLI/cmux.swiftRepository: manaflow-ai/cmux
Length of output: 4119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the forwardSidebarMetadataCommand function definition
echo "=== forwardSidebarMetadataCommand function ==="
rg -A 30 'func forwardSidebarMetadataCommand' CLI/cmux.swift | head -50
echo ""
echo "=== Check if forwardSidebarMetadataCommand references --pid ==="
rg 'forwardSidebarMetadataCommand' CLI/cmux.swift -B 2 -A 15Repository: manaflow-ai/cmux
Length of output: 4994
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the complete forwardSidebarMetadataCommand function
echo "=== Complete forwardSidebarMetadataCommand function ==="
rg -A 100 'private func forwardSidebarMetadataCommand' CLI/cmux.swift | head -120Repository: manaflow-ai/cmux
Length of output: 3859
Remove or consolidate the duplicate sidebar-metadata case block.
Lines 2156-2235 already handle set-status, clear-status, list-status, set-progress, clear-progress, log, clear-log, list-log, and sidebar-state via forwardSidebarMetadataCommand(). The block at Lines 2248-2360 re-declares the same cases in the same switch command, making CLI/cmux.swift invalid Swift—the CLI will not compile and the new --pid support never becomes active.
The second block adds --pid validation (Int32(pidRaw), pidRaw > 0) for set-status, which the delegated helper does not perform. Consolidate by either updating forwardSidebarMetadataCommand to parse and pass --pid to the socket command, or replace the delegated calls with direct implementations that include --pid handling. Keep only one implementation path per case label.
🧰 Tools
🪛 SwiftLint (0.63.2)
[Error] 2248-2248: Duplicate sets of conditions in the same branch instruction should be avoided
(duplicate_conditions)
[Error] 2280-2280: Duplicate sets of conditions in the same branch instruction should be avoided
(duplicate_conditions)
[Error] 2290-2290: Duplicate sets of conditions in the same branch instruction should be avoided
(duplicate_conditions)
[Error] 2297-2297: Duplicate sets of conditions in the same branch instruction should be avoided
(duplicate_conditions)
[Error] 2311-2311: Duplicate sets of conditions in the same branch instruction should be avoided
(duplicate_conditions)
[Error] 2318-2318: Duplicate sets of conditions in the same branch instruction should be avoided
(duplicate_conditions)
[Error] 2337-2337: Duplicate sets of conditions in the same branch instruction should be avoided
(duplicate_conditions)
[Error] 2344-2344: Duplicate sets of conditions in the same branch instruction should be avoided
(duplicate_conditions)
[Error] 2355-2355: Duplicate sets of conditions in the same branch instruction should be avoided
(duplicate_conditions)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 2248 - 2360, There are duplicate switch cases
for
set-status/clear-status/list-status/set-progress/clear-progress/log/clear-log/list-log/sidebar-state
causing a compile error and the new --pid handling in the second block never
reaches the delegated path; remove the duplicate block and consolidate behavior
by either (A) updating forwardSidebarMetadataCommand(...) to parse "--pid" (use
parseOption to capture pidRaw, validate with Int32(pidRaw) > 0, include "
--pid=\(pid)" in the socket command built before calling sendV1Command) or (B)
delete the forwardSidebarMetadataCommand calls for these cases and keep the
single direct implementations (using parseOption, resolveWorkspaceId,
socketQuote and sendV1Command) that include the --pid validation; ensure only
one implementation per case label and that sendV1Command/socket command
construction includes the validated pid when present.
| if NSApp.windows.isEmpty, !didRequestFallbackUITestWindow { | ||
| didRequestFallbackUITestWindow = true | ||
| openNewMainWindow(nil) | ||
| } | ||
|
|
||
| moveUITestWindowToTargetDisplayIfNeeded() | ||
| activateUITestAppIfNeeded() | ||
|
|
||
| let hasWindow = !NSApp.windows.isEmpty | ||
| let hasVisibleWindow = NSApp.windows.contains { $0.isVisible } | ||
| let hasKeyWindow = NSApp.keyWindow != nil | ||
| let stage = attempt == 0 ? "afterForceWindow" : "afterForceWindow.retry\(attempt)" | ||
| writeUITestDiagnosticsIfNeeded(stage: stage) | ||
|
|
||
| guard attempt < 20 else { return } | ||
| if !hasWindow || !hasVisibleWindow || !hasKeyWindow || !NSRunningApplication.current.isActive { | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { [weak self] in | ||
| self?.stabilizeUITestLaunchWindowAndForeground(attempt: attempt + 1) | ||
| } | ||
| } |
There was a problem hiding this comment.
Gate UI-test stabilization on a main terminal window.
This loop succeeds as soon as any AppKit window is visible/key. If a settings/import/debug window appears first, retries stop even though no cmux.main window is ready, and the helper can keep moving/activating the wrong window. Filter both the fallback creation and the ready checks to isMainTerminalWindow(_:).
Suggested fix
- if NSApp.windows.isEmpty, !didRequestFallbackUITestWindow {
+ let mainTerminalWindows = NSApp.windows.filter { self.isMainTerminalWindow($0) }
+ if mainTerminalWindows.isEmpty, !didRequestFallbackUITestWindow {
didRequestFallbackUITestWindow = true
openNewMainWindow(nil)
}
moveUITestWindowToTargetDisplayIfNeeded()
activateUITestAppIfNeeded()
- let hasWindow = !NSApp.windows.isEmpty
- let hasVisibleWindow = NSApp.windows.contains { $0.isVisible }
- let hasKeyWindow = NSApp.keyWindow != nil
+ let hasWindow = !mainTerminalWindows.isEmpty
+ let hasVisibleWindow = mainTerminalWindows.contains { $0.isVisible }
+ let hasKeyWindow = NSApp.keyWindow.map(isMainTerminalWindow) ?? false
let stage = attempt == 0 ? "afterForceWindow" : "afterForceWindow.retry\(attempt)"
writeUITestDiagnosticsIfNeeded(stage: stage)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if NSApp.windows.isEmpty, !didRequestFallbackUITestWindow { | |
| didRequestFallbackUITestWindow = true | |
| openNewMainWindow(nil) | |
| } | |
| moveUITestWindowToTargetDisplayIfNeeded() | |
| activateUITestAppIfNeeded() | |
| let hasWindow = !NSApp.windows.isEmpty | |
| let hasVisibleWindow = NSApp.windows.contains { $0.isVisible } | |
| let hasKeyWindow = NSApp.keyWindow != nil | |
| let stage = attempt == 0 ? "afterForceWindow" : "afterForceWindow.retry\(attempt)" | |
| writeUITestDiagnosticsIfNeeded(stage: stage) | |
| guard attempt < 20 else { return } | |
| if !hasWindow || !hasVisibleWindow || !hasKeyWindow || !NSRunningApplication.current.isActive { | |
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { [weak self] in | |
| self?.stabilizeUITestLaunchWindowAndForeground(attempt: attempt + 1) | |
| } | |
| } | |
| let mainTerminalWindows = NSApp.windows.filter { self.isMainTerminalWindow($0) } | |
| if mainTerminalWindows.isEmpty, !didRequestFallbackUITestWindow { | |
| didRequestFallbackUITestWindow = true | |
| openNewMainWindow(nil) | |
| } | |
| moveUITestWindowToTargetDisplayIfNeeded() | |
| activateUITestAppIfNeeded() | |
| let hasWindow = !mainTerminalWindows.isEmpty | |
| let hasVisibleWindow = mainTerminalWindows.contains { $0.isVisible } | |
| let hasKeyWindow = NSApp.keyWindow.map(isMainTerminalWindow) ?? false | |
| let stage = attempt == 0 ? "afterForceWindow" : "afterForceWindow.retry\(attempt)" | |
| writeUITestDiagnosticsIfNeeded(stage: stage) | |
| guard attempt < 20 else { return } | |
| if !hasWindow || !hasVisibleWindow || !hasKeyWindow || !NSRunningApplication.current.isActive { | |
| DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { [weak self] in | |
| self?.stabilizeUITestLaunchWindowAndForeground(attempt: attempt + 1) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 2420 - 2439,
stabilizeUITestLaunchWindowAndForeground currently considers any NSApp window as
satisfying readiness, causing false positives when non-main windows
(settings/import/debug) appear; change the fallback creation and all readiness
checks to only consider main terminal windows by using isMainTerminalWindow(_:).
Specifically, when creating a fallback in the initial if-block call
openNewMainWindow only if no main terminal window exists (filter NSApp.windows
with isMainTerminalWindow), and replace hasWindow/hasVisibleWindow/hasKeyWindow
checks with checks that operate on NSApp.windows.filter(isMainTerminalWindow)
(e.g., any main window exists, any main window isVisible, NSApp.keyWindow is a
main terminal window). Keep calls to moveUITestWindowToTargetDisplayIfNeeded,
activateUITestAppIfNeeded, and writeUITestDiagnosticsIfNeeded but ensure they
operate or log with respect to main terminal windows only.
| if #available(macOS 14.0, *) { | ||
| NSRunningApplication.current.activate(options: [.activateAllWindows]) | ||
| } else { | ||
| NSRunningApplication.current.activate(options: [.activateAllWindows, .activateIgnoringOtherApps]) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
On macOS 14 and later, is NSRunningApplication.ActivationOptions.activateIgnoringOtherApps still supported for NSRunningApplication.current.activate(options:), and how does its behavior differ from using only .activateAllWindows?
💡 Result:
Yes, NSRunningApplication.ActivationOptions.activateIgnoringOtherApps (also known as .activateIgnoringOtherApps) is still supported on macOS 14 (Sonoma) and later for NSRunningApplication.current.activate(options:). Apple's official documentation lists it as an available option in NSApplication.ActivationOptions, used by the activate(options:) method, with copyright dated 2026 indicating current support. A GitHub issue from MacVim confirms it is deprecated in macOS 14 but "still works," with no breakage in macOS 15 planned yet. Rust bindings explicitly note it as deprecated in macOS 14 with no effect, but Apple docs do not mark it deprecated and it remains documented. It differs from .activateAllWindows as follows: - .activateIgnoringOtherApps: Activates the target app regardless of the currently active app, potentially stealing focus (discouraged for poor UX). - .activateAllWindows: Activates the app (cooperatively) and brings all its windows forward (default brings only main/key windows forward). These can be combined. Default activation deactivates the calling app first then activates the target only if no other app is active meanwhile. On macOS 14+, prefer cooperative activation with NSApp.yieldActivation(to:) before calling activate.
Citations:
- 1: https://developer.apple.com/documentation/appkit/nsrunningapplication/activate(options:)
- 2: https://developer.apple.com/documentation/appkit/nsrunningapplication?changes=_4
- 3: https://developer.apple.com/documentation/appkit/nsapplication/activationoptions/activateignoringotherapps
- 4: Use macOS cooperative app activation to activate app to foreground macvim-dev/macvim#1456
- 5: https://developer.apple.com/documentation/appkit/nsrunningapplication/activate(from:options:)
- 6: https://learn.microsoft.com/en-us/dotnet/api/appkit.nsapplicationactivationoptions?view=net-macos-26.0-10.0
- 7: https://learn.microsoft.com/en-us/dotnet/api/appkit.nsapplicationactivationoptions
- 8: https://docs.rs/objc2-app-kit/latest/objc2_app_kit/struct.NSApplicationActivationOptions.html
🏁 Script executed:
# Find and examine the AppDelegate.swift file around lines 2447-2450
fd -type f -name "AppDelegate.swift" | head -20Repository: manaflow-ai/cmux
Length of output: 230
🏁 Script executed:
# Search for context around the target lines and other uses of activateIgnoringOtherApps
rg -n "activateIgnoringOtherApps" --max-count=20 -B3 -A3Repository: manaflow-ai/cmux
Length of output: 2760
🏁 Script executed:
# Get the full context of AppDelegate.swift to understand what this code block is doing
wc -l Sources/AppDelegate.swiftRepository: manaflow-ai/cmux
Length of output: 91
🏁 Script executed:
# Get more context around lines 2447-2450 to understand the function and purpose
sed -n '2430,2460p' Sources/AppDelegate.swift | cat -nRepository: manaflow-ai/cmux
Length of output: 1602
Address the weaker activation on macOS 14+, but consider Apple's deprecation guidance.
The macOS 14+ branch inconsistently drops .activateIgnoringOtherApps, making this path weaker than all other activation calls in the file (AppDelegate lines 6065, 10905, 11392, and TerminalController 6484, 6486). This can leave NSRunningApplication.current.isActive false through retries.
However, note that Apple deprecated .activateIgnoringOtherApps on macOS 14 (though it still functions). Instead of unifying around the deprecated option, consider using Apple's recommended cooperative activation: NSApp.yieldActivation(to:) before activate(options:) for macOS 14+, or restore .activateIgnoringOtherApps for consistency if immediate activation is required for test reliability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/AppDelegate.swift` around lines 2447 - 2450, The macOS 14+ branch
drops .activateIgnoringOtherApps causing weaker activation; to fix, before
calling NSRunningApplication.current.activate in the macOS 14+ branch call
NSApp.yieldActivation(to: NSRunningApplication.current) and then invoke
NSRunningApplication.current.activate(options: [.activateAllWindows,
.activateIgnoringOtherApps]) so behavior matches the other activation sites
(compare with other activate(...) calls) while also performing Apple's
recommended cooperative yield first.
Greptile SummaryThis PR ships a bundled Key issues found:
The bash wrapper, JS plugin logic, tests, and the CI/UITest stabilization changes are all well-structured and correct. Confidence Score: 3/5The core new CLI feature (--pid support in set-status) is dead code due to unreachable duplicate switch cases; needs a fix before the intended behavior is active. One P1 finding: all nine new sidebar command case blocks in CLI/cmux.swift are unreachable because identical case labels already exist earlier in the same switch statement. The primary advertised feature of this PR (--pid stale-process cleanup for set-status) is never executed. The wrapper, plugin, and tests are clean, but the Swift CLI change is fundamentally broken for its stated purpose. CLI/cmux.swift — duplicate switch cases make the new set-status/--pid implementation unreachable; the pre-existing forwardSidebarMetadataCommand cases (lines 2156–2235) must be removed or updated. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Wrapper as Resources/bin/opencode (bash)
participant Plugin as opencode-cmux-plugin.js
participant OpenCode as Real opencode binary
participant CMux as cmux CLI
User->>Wrapper: opencode [args]
Wrapper->>Wrapper: Check CMUX_SURFACE_ID + socket ping
alt Integration active
Wrapper->>Wrapper: mktemp overlay config dir
Wrapper->>Wrapper: Symlink existing user config
Wrapper->>Wrapper: Copy cmux-integration.js to plugins/
Wrapper->>CMux: clear-status opencode
Wrapper->>OpenCode: exec with OPENCODE_CONFIG_DIR=overlay
OpenCode->>Plugin: session.created / session.status events
Plugin->>CMux: set-status opencode Running/Idle/Error
Plugin->>CMux: notify (permission / question / error)
OpenCode->>Plugin: busy to idle (completion)
Plugin->>CMux: notify Done
Plugin->>CMux: set-status opencode Idle (blue attention)
OpenCode-->>Wrapper: exit
Wrapper->>CMux: clear-status opencode
Wrapper->>CMux: clear-notifications
Wrapper->>Wrapper: cleanup temp config dir (EXIT trap)
else Socket unavailable / disabled
Wrapper->>OpenCode: exec directly (passthrough)
end
Wrapper-->>User: exit $STATUS
Reviews (1): Last reviewed commit: "merge: resolve CLI/cmux.swift conflict w..." | Re-trigger Greptile |
| case "set-status": | ||
| let (icon, r1) = parseOption(commandArgs, name: "--icon") | ||
| let (color, r2) = parseOption(r1, name: "--color") | ||
| let (pidRaw, r3) = parseOption(r2, name: "--pid") | ||
| let pid: Int32? | ||
| if let pidRaw { | ||
| guard let parsedPid = Int32(pidRaw), parsedPid > 0 else { | ||
| throw CLIError(message: "--pid must be a positive integer") | ||
| } | ||
| pid = parsedPid | ||
| } else { | ||
| pid = nil | ||
| } | ||
| let (wsFlag, r4) = parseOption(r3, name: "--workspace") | ||
| guard r4.count >= 2 else { | ||
| throw CLIError(message: "set-status requires <key> and <value>") | ||
| } | ||
| let key = r4[0] | ||
| let value = r4.dropFirst().joined(separator: " ") | ||
| guard !value.isEmpty else { | ||
| throw CLIError(message: "set-status requires a non-empty value") | ||
| } | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| var socketCmd = "set_status \(key) \(socketQuote(value))" | ||
| if let icon { socketCmd += " --icon=\(socketQuote(icon))" } | ||
| if let color { socketCmd += " --color=\(socketQuote(color))" } | ||
| if let pid { socketCmd += " --pid=\(pid)" } | ||
| socketCmd += " --tab=\(wsId)" | ||
| let response = try sendV1Command(socketCmd, client: client) | ||
| print(response) | ||
|
|
||
| case "clear-status": | ||
| let (wsFlag, csRemaining) = parseOption(commandArgs, name: "--workspace") | ||
| guard let key = csRemaining.first else { | ||
| throw CLIError(message: "clear-status requires a <key>") | ||
| } | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| let response = try sendV1Command("clear_status \(key) --tab=\(wsId)", client: client) | ||
| print(response) | ||
|
|
||
| case "list-status": | ||
| let (wsFlag, _) = parseOption(commandArgs, name: "--workspace") | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| let response = try sendV1Command("list_status --tab=\(wsId)", client: client) | ||
| print(response) | ||
|
|
||
| case "set-progress": | ||
| let (label, spR1) = parseOption(commandArgs, name: "--label") | ||
| let (wsFlag, spR2) = parseOption(spR1, name: "--workspace") | ||
| guard let valueStr = spR2.first else { | ||
| throw CLIError(message: "set-progress requires a progress value (0.0-1.0)") | ||
| } | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| var socketCmd = "set_progress \(valueStr)" | ||
| if let label { socketCmd += " --label=\(socketQuote(label))" } | ||
| socketCmd += " --tab=\(wsId)" | ||
| let response = try sendV1Command(socketCmd, client: client) | ||
| print(response) | ||
|
|
||
| case "clear-progress": | ||
| let (wsFlag, _) = parseOption(commandArgs, name: "--workspace") | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| let response = try sendV1Command("clear_progress --tab=\(wsId)", client: client) | ||
| print(response) | ||
|
|
||
| case "log": | ||
| let (level, r1) = parseOption(commandArgs, name: "--level") | ||
| let (source, r2) = parseOption(r1, name: "--source") | ||
| let (wsFlag, r3) = parseOption(r2, name: "--workspace") | ||
| // Strip leading "--" separator if present | ||
| let positional = r3.first == "--" ? Array(r3.dropFirst()) : r3 | ||
| let message = positional.joined(separator: " ") | ||
| guard !message.isEmpty else { | ||
| throw CLIError(message: "log requires a message") | ||
| } | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| var socketCmd = "log" | ||
| if let level { socketCmd += " --level=\(level)" } | ||
| if let source { socketCmd += " --source=\(socketQuote(source))" } | ||
| socketCmd += " --tab=\(wsId) -- \(socketQuote(message))" | ||
| let response = try sendV1Command(socketCmd, client: client) | ||
| print(response) | ||
|
|
||
| case "clear-log": | ||
| let (wsFlag, _) = parseOption(commandArgs, name: "--workspace") | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| let response = try sendV1Command("clear_log --tab=\(wsId)", client: client) | ||
| print(response) | ||
|
|
||
| case "list-log": | ||
| let (limitStr, r1) = parseOption(commandArgs, name: "--limit") | ||
| let (wsFlag, _) = parseOption(r1, name: "--workspace") | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| var socketCmd = "list_log" | ||
| if let limitStr { socketCmd += " --limit=\(limitStr)" } | ||
| socketCmd += " --tab=\(wsId)" | ||
| let response = try sendV1Command(socketCmd, client: client) | ||
| print(response) | ||
|
|
||
| case "sidebar-state": | ||
| let (wsFlag, _) = parseOption(commandArgs, name: "--workspace") | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| let response = try sendV1Command("sidebar_state --tab=\(wsId)", client: client) | ||
| print(response) |
There was a problem hiding this comment.
CLI/cmux.swift already contains matching case labels for every command added in this PR — set-status (line 2156), clear-status (line 2165), list-status (line 2174), set-progress (line 2183), clear-progress (line 2192), log (line 2201), clear-log (line 2210), list-log (line 2219), and sidebar-state (line 2228) — all implemented via forwardSidebarMetadataCommand. Because Swift evaluates switch cases top-to-bottom and stops at the first match, the new implementations added here (lines 2248–2360) are never reached.
The critical consequence for this PR is that the --pid validation and the explicit set_status <key> <value> argument shape added at lines 2248–2278 are dead code. A user running cmux set-status opencode Running --pid -99 will silently forward --pid -99 to the socket (via forwardSidebarMetadataCommand) instead of receiving the --pid must be a positive integer error.
To fix, either:
- Remove the pre-existing
forwardSidebarMetadataCommandcases for these commands (lines 2156–2235) and keep the new implementations, or - Update the existing cases in-place to add
--pidparsing/validation rather than inserting new cases afterclaude-hook.
| } | ||
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| var socketCmd = "set_progress \(valueStr)" |
There was a problem hiding this comment.
set-progress value is not validated
valueStr is interpolated directly into the socket command without checking that it is a valid float in [0.0, 1.0]. This is inconsistent with the explicit positive-integer guard added for --pid in set-status, and the help text promises 0.0-1.0. An invalid string (e.g. "banana") would reach the server with no useful client-side error.
| var socketCmd = "set_progress \(valueStr)" | |
| guard let valueStr = spR2.first else { | |
| throw CLIError(message: "set-progress requires a progress value (0.0-1.0)") | |
| } | |
| guard let progressVal = Double(valueStr), (0.0...1.0).contains(progressVal) else { | |
| throw CLIError(message: "set-progress value must be a number between 0.0 and 1.0") | |
| } |
| let workspaceArg = wsFlag ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | ||
| let wsId = try resolveWorkspaceId(workspaceArg, client: client) | ||
| var socketCmd = "log" | ||
| if let level { socketCmd += " --level=\(level)" } |
There was a problem hiding this comment.
--level is not quoted, inconsistent with --source
--source is wrapped in socketQuote() on the very next line, but --level is interpolated bare. If a level value somehow contains spaces or special characters the socket parser would misinterpret it. For consistency:
| if let level { socketCmd += " --level=\(level)" } | |
| if let level { socketCmd += " --level=\(socketQuote(level))" } |
| if (event.type === "session.idle") { | ||
| const sessionID = event.properties?.sessionID | ||
| if (!sessionID) return | ||
| const state = ensure(sessions, sessionID) | ||
| state.state = "idle" | ||
| state.error = "" | ||
| await sync() | ||
| return | ||
| } |
There was a problem hiding this comment.
session.idle does not trigger the completion path
The completion detection — setting state.completed = true and firing the "Done" notification — lives exclusively in the session.status handler on the busy → idle transition (line 169). The session.idle event handler at line 177 only sets state.state = "idle" and calls sync(), so if OpenCode emits session.idle as its primary completion signal, the blue-attention "Idle" badge and "Done" notification will never fire.
If session.idle is a distinct "work finished" event you expect OpenCode to emit, consider mirroring the completion logic here:
if (event.type === "session.idle") {
const sessionID = event.properties?.sessionID
if (!sessionID) return
const state = ensure(sessions, sessionID)
const prevState = state.state
state.state = "idle"
state.error = ""
if (prevState === "busy") {
state.completed = true
await notify("Done", "Session completed")
}
await sync()
return
}If session.idle and session.status { type: "idle" } are always fired together, this is safe to leave as-is, but a comment explaining the assumption would help future readers.
4 participants
Summary
opencodewrapper that injects a local cmux integration plugin when OpenCode is launched inside cmuxcmux set-statuswith--pidso OpenCode statuses participate in stale-process cleanupTesting
python3 tests/test_opencode_wrapper_hooks.pynode tests/test_opencode_plugin_runtime.jspython3 tests/test_claude_wrapper_hooks.pybash -n Resources/bin/opencode && python3 tests/test_opencode_wrapper_hooks.py && node tests/test_opencode_plugin_runtime.js && python3 tests/test_claude_wrapper_hooks.pyDemo Video
Checklist
Related
Summary by cubic
Adds built-in OpenCode status integration with an auto-injected plugin that maps session events to sidebar status and notifications. When work finishes, it shows “Idle” with an attention badge (blue ring) until new work starts. Works automatically when
opencoderuns insidecmux; setCMUX_OPENCODE_INTEGRATION_DISABLED=1to opt out.New Features
opencodewrapper injectsopencode-cmux-plugin.jswhen a livecmuxsocket is detected; falls back to plainopencodeon any setup failure.cmux set-statuswith--pid(validated positive Int32) and adds sidebar commands:clear-status,list-status,set-progress/clear-progress,log/clear-log/list-log,sidebar-state; CLI help updated. Xcode project shipsopencodeand the plugin; docs add a manual example withosascriptfallback and a reference to the bundled plugin.Bug Fixes
cmuxcommand output via.quiet()to prevent “OK” text from leaking into terminals.Written for commit f007166. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores