Repository navigation
fix(opencode-plugin): propagate surface_id on feed events so notifications target opencode pane - #6504
fix(opencode-plugin): propagate surface_id on feed events so notifications target opencode pane#6504e-jung wants to merge 2 commits into
Conversation
…he focused pane The bundled opencode plugin (Resources/opencode-plugin.js) bridges opencode's plugin event bus to cmux's `feed.push` socket verb. Blocking events (permission requests, plan approvals, questions) surface an in-app "Needs input" attention overlay plus a desktop notification banner. The attention overlay resolves its target surface and, when no surface can be resolved, falls back to the workspace's focused panel (FeedCoordinator surfaceBlockingDecisionAttention -> resolvePanelId ?? tab.focusedPanelId). In a split layout this routes the attention/bell to whichever pane happens to be focused instead of opencode's own pane. Root cause: the plugin's `base()` event builder propagated `workspace_id` from `CMUX_WORKSPACE_ID` (so the correct workspace/tab was targeted) but never propagated a surface id, so the surface could only come from the per-agent hook session store (~/.cmuxterm/opencode-hook-sessions.json, written by `cmux opencode-hook session-start`). When that store isn't populated for the opencode session, the surface is unresolvable and routing falls through to the focused pane -- the symptom described in manaflow-ai#2302. Fix: read `CMUX_SURFACE_ID` (which cmux injects into every agent shell alongside `CMUX_WORKSPACE_ID`, AppDelegate.swift environment setup) and propagate it as `surface_id` on every feed event, mirroring the existing `workspace_id` handling. This gives cmux a live, authoritative surface id for the running opencode pane so attention/notifications can target it directly instead of the focused pane. Refs manaflow-ai#2302
|
@e-jung is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds Changessurface_id field addition and consumption
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR completes the surface-routing fix for split-pane opencode workspaces. When opencode runs in one pane and another terminal occupies the other, blocking events (permission requests, plan approvals) previously fired their attention overlay on the focused pane instead of the opencode pane.
Confidence Score: 5/5Safe to merge — the three-file change is internally consistent and the new surface routing path is a direct, symmetric extension of the existing workspace routing path. All three layers of the fix (JS event emission, Swift model decoding, FeedCoordinator resolution) are aligned and consistent. The env-var override correctly wins over the ...extra spread. The UUID parse in resolveAttentionTarget silently falls back to the session store on invalid input, matching the established workspaceId behavior. No pre-existing invariants are broken and no new bad states are introduced. No files require special attention; the changes are minimal and each file's diff is self-contained. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant P as opencode-plugin.js
participant S as cmux socket
participant W as WorkstreamEvent (Swift)
participant F as FeedCoordinator
participant UI as Attention Overlay
P->>P: base() reads CMUX_WORKSPACE_ID + CMUX_SURFACE_ID
P->>S: "emit JSON {workspace_id, surface_id, hook_event_name, …}"
S->>W: decode JSON → WorkstreamEvent(.workspaceId, .surfaceId)
W->>F: resolveAttentionTarget(event)
F->>F: parse event.surfaceId as UUID (eventSurfaceId)
F->>F: "surfaceId = eventSurfaceId ?? sessionStore fallback"
F->>UI: target correct pane (opencode pane, not focused pane)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant P as opencode-plugin.js
participant S as cmux socket
participant W as WorkstreamEvent (Swift)
participant F as FeedCoordinator
participant UI as Attention Overlay
P->>P: base() reads CMUX_WORKSPACE_ID + CMUX_SURFACE_ID
P->>S: "emit JSON {workspace_id, surface_id, hook_event_name, …}"
S->>W: decode JSON → WorkstreamEvent(.workspaceId, .surfaceId)
W->>F: resolveAttentionTarget(event)
F->>F: parse event.surfaceId as UUID (eventSurfaceId)
F->>F: "surfaceId = eventSurfaceId ?? sessionStore fallback"
F->>UI: target correct pane (opencode pane, not focused pane)
Reviews (2): Last reviewed commit: "fix(opencode-plugin): add surfaceId to W..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -416,6 +420,7 @@ export const CMUXFeed = async (ctx) => { | |||
| ...extra, | |||
| }; | |||
| if (workspaceId) event.workspace_id = workspaceId; | |||
| if (surfaceId) event.surface_id = surfaceId; | |||
There was a problem hiding this comment.
surface_id is discarded before it can reach the resolver
WorkstreamEvent (defined in Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/Workstream/WorkstreamEvent.swift) has no surfaceId property and no "surface_id" CodingKey. When the Swift socket handler decodes the incoming JSON, surface_id falls through to the extraFieldsJSON catch-all and is never surfaced as a typed property. FeedCoordinator.resolveAttentionTarget reads event.workspaceId (a first-class property) but sources surfaceId exclusively from the hook-session store — it never inspects extraFieldsJSON. The attention overlay therefore still falls back to tab.focusedPanelId, leaving the bug fully intact.
The PR description mentions "a matching one-line change on the Swift side," but the actual minimum required is two changes: (1) add a public let surfaceId: String? field with CodingKey = "surface_id" to WorkstreamEvent, and (2) update resolveAttentionTarget to prefer event.surfaceId over sessionMatch?.surfaceId, exactly as it already prefers event.workspaceId.
| const surfaceId = | ||
| typeof process.env.CMUX_SURFACE_ID === "string" && process.env.CMUX_SURFACE_ID.trim() | ||
| ? process.env.CMUX_SURFACE_ID.trim() | ||
| : null; |
There was a problem hiding this comment.
surfaceId: nil
Even after the Swift model is updated, the TerminalNotificationPolicyPayload built in FeedCoordinator (line 1067 of FeedCoordinator.swift) hard-codes surfaceId: nil, so push notifications generated for blocking events will still not carry a surface target. This path is separate from the attention-overlay routing, but it means Bell/notification-centre alerts will remain workspace-level only until that site is also updated to read the event's surfaceId.
|
Good catch from the Greptile review — agreed this is half the fix. The JS side (propagating I have access to an M1 Mac for building/testing — happy to tackle the Swift side (add the |
…rdinator.resolveAttentionTarget
|
Update on the 'missing half' I flagged earlier: the Swift consumer side is now wired in 516f058 — We also independently verified the build on a fresh Xcode 26.5: it builds clean (0 Swift errors), and the relevant test suites pass — |
Reads CMUX_SURFACE_ID in the opencode feed plugin and sets surface_id on feed events, the plugin half of #6504. Co-authored-by: e-jung <e-jung@users.noreply.github.com>
2 participants
Refs #2302.
Problem
When opencode runs in a split workspace (opencode in one pane, another terminal in the other), blocking events from the bundled opencode plugin (
Resources/opencode-plugin.js) — permission requests, plan approvals, questions — surface their "Needs input" attention overlay and bell on whichever pane is currently focused, not the pane where opencode is actually running.Root cause
The plugin's
base()event builder propagatesworkspace_id(fromCMUX_WORKSPACE_ID, which cmux injects into every agent shell) so the correct workspace/tab is targeted — but it never propagated a surface id. With only a workspace id,FeedCoordinatorresolves the workspace correctly but can't resolve the surface; it then falls back totab.focusedPanelId(Sources/Feed/FeedCoordinator.swift,surfaceBlockingDecisionAttention→resolvePanelId ?? tab.focusedPanelId). In a split layout that is the wrong pane.The surface could otherwise only be recovered from the per-agent hook session store (
~/.cmuxterm/opencode-hook-sessions.json, written bycmux opencode-hook session-start); when that isn't populated for the opencode session, the surface is unresolvable and routing lands on the focused pane — the symptom in #2302.Fix
Read
CMUX_SURFACE_ID(cmux injects it alongsideCMUX_WORKSPACE_ID, see the environment setup inSources/AppDelegate.swift) and propagate it assurface_idon every feed event, mirroring the existingworkspace_idhandling inbase():This gives cmux a live, authoritative surface id for the running opencode pane so attention/notifications can target it directly instead of the focused pane. This is the direct analog of the fix recipe in #2302 (pass surface info from env), applied to the bundled socket-based plugin path.
Validation caveat (Linux-prepared, macOS validation needed)
cmux is a Swift/Xcode macOS app; this change was prepared and lint-checked on Linux only. The JS plugin passes a Node ESM syntax check. (
Resources/opencode-plugin.jsis outsidebiome.json'sfiles.includesscope, sobun run biome:checkdoes not cover it.)Note for the maintainer: this PR is the plugin-side half of the fix — it makes a live
surface_idavailable on every feed event.FeedCoordinator.resolveAttentionTargetcurrently sources the surface only from the per-agent hook session store, not from the event, so a matching one-line change on the Swift side (preferevent.surfaceIdthe same way it already prefersevent.workspaceId) is needed to complete pane-level routing for this path. I scoped this PR to the JS plugin per the task; the Swift follow-up needs macOS/Xcode validation, which I could not perform here.No Swift/Xcode build was attempted.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Route attention overlays and notifications to the opencode pane instead of the focused pane in split layouts. Propagates
surface_idfromCMUX_SURFACE_IDinResources/opencode-plugin.jsand wires it throughWorkstreamEventandFeedCoordinator.resolveAttentionTargetto prefer the event’s pane (with session-store fallback), addressing #2302.Written for commit 516f058. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Chores