Skip to content

opencode plugin: send surface_id on feed events - #14781

Merged
teamleaderleo merged 1 commit into
mainfrom
fix/opencode-feed-surface-id
Sep 26, 2026
Merged

teamleaderleo merged 1 commit into
mainfrom
fix/opencode-feed-surface-id

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

opencode feed events only carry workspace_id, so attention targeting falls back to the hook-session store or the focused pane. The Swift side already prefers an explicit surface (WorkstreamEvent.surfaceId, #6798). This reads CMUX_SURFACE_ID the same way as CMUX_WORKSPACE_ID and sets surface_id when present.

This is the plugin half of #6504 by @e-jung, which main still needed. Co-authored with them.

🤖 Generated with Claude Code


Summary by cubic

Adds surface_id to opencode feed events so attention targeting can use the explicit surface instead of falling back to the hook-session store or focused pane.

  • Reads CMUX_SURFACE_ID the same way as CMUX_WORKSPACE_ID and sets surface_id on the event when present.
  • Mirrors the Swift side's existing preference for an explicit WorkstreamEvent.surfaceId.

Written for commit 4d5b61b. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Events now include the current surface ID when a non-empty value is available.

Feed events from opencode only carried workspace_id, so attention
targeting fell back to the hook-session store or the focused pane even
though the Swift side (WorkstreamEvent.surfaceId, #6798) already prefers
an explicit surface. Read CMUX_SURFACE_ID like CMUX_WORKSPACE_ID and set
surface_id when present.

From #6504 by @e-jung.

Co-authored-by: e-jung <e-jung@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4377296e-666f-47d8-af96-91f6c2762652

📥 Commits

Reviewing files that changed from the base of the PR and between 11bcc80 and 4d5b61b.

📒 Files selected for processing (1)
  • Resources/opencode-plugin.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The OpenCode event base function reads and trims CMUX_SURFACE_ID. It adds the surface_id field only when the trimmed value is non-empty.

Changes

Event surface ID

Layer / File(s) Summary
Read and add the surface ID
Resources/opencode-plugin.js
The base function trims CMUX_SURFACE_ID and adds surface_id to the event only when the value is non-empty.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 4d5b6

This change enables explicit surface targeting while retaining existing fallback behavior when no surface ID is available. No merge-blocking risk is apparent.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 4d5b6

A plugin-provided surface identifier may influence which pane receives an attention event or permission prompt. The available code does not establish that OpenCode events are bound to the pane that launched them, although permission replies still target their original OpenCode session.

Retained concerns

  • Medium · security · inferred: An environment-supplied surface claim is added to OpenCode attention and blocking events, while the inspected Feed delivery-target ownership check applies only to Pi events. A mismatched claim could retarget a prompt or attention event to another pane.
Security review details

Security Blast Radius

  • inferred — The plausible new exposure is misdelivery among surfaces reachable by the local Feed, including blocking prompts. The available evidence does not establish cross-user, remote, or cross-tenant reachability.

Security Findings and Attack Paths

  • inferred — If a plugin process can supply a surface ID unrelated to its session, it can emit a blocking frame claiming that surface. The inspected resolver does not reconcile OpenCode claims; whether another control rejects the frame or prevents an unintended user from resolving it remains unknown.

Trust Boundaries and Controls

  • observed — The plugin trims the environment value but does not bind it to workspace_id or session_id when constructing the event. On resolution, the plugin retains the originating OpenCode session and request identifiers.

Resilience and Maintainability Implications

  • observed — Pending blocking requests are correlated by request ID rather than by surface or workspace. That correlation behavior was not changed by the surface_id addition, but it cannot itself verify the responding surface.

Hardening Proposals

  • proposed — Bind an OpenCode surface claim to the live process, session, or workspace before using it to route attention or blocking prompts, and preserve that binding when accepting a response.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description clearly explains the problem and resulting behavior, but it omits the required Testing section and checklist. It does not state which tests ran, what passed, or why tests were not adde… Add a Testing section that names executed tests or explains why no tests were added. State what remains unverified. Add the applicable checklist items and mark them accurately, including the review status.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: sending surface_id in opencode feed events.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS. The PR changes only Resources/opencode-plugin.js and adds a trimmed, non-empty CMUX_SURFACE_ID value as event.surface_id in the existing feed-event builder. It does not create or change Cl…
Cmux Swift Actor Isolation ✅ Passed PASS: The pull request changes only Resources/opencode-plugin.js. The diff introduces no Swift production changes, so it cannot introduce or worsen Swift 6 actor isolation mistakes.
Cmux Swift Blocking Runtime ✅ Passed The pull request changes only Resources/opencode-plugin.js. The diff adds environment-variable handling and a surface_id event field. It contains no Swift changes, so it does not introduce or expa…
Cmux Browser Automation Off-Main ✅ Passed PASS. The pull request changes only Resources/opencode-plugin.js. It adds trimmed CMUX_SURFACE_ID handling to feed event construction. It does not change browser socket commands, WebKit/AppKit acc…
Cmux Expensive Synchronous Load ✅ Passed PASS: The review-scoped diff changes only Resources/opencode-plugin.js. It adds trimmed environment-variable reads and an optional surface_id event field. It adds no Swift code and no synchronous …
Cmux Cache Substitution Correctness ✅ Passed The PR adds a direct, trimmed read of process.env.CMUX_SURFACE_ID in base() and conditionally adds event.surface_id. The diff does not replace an authoritative read with a cache, and the changed…
Cmux No Hacky Sleeps ✅ Passed The pull request changes only Resources/opencode-plugin.js and adds CMUX_SURFACE_ID parsing plus conditional surface_id assignment. The diff introduces no sleep, timer, polling, delay, retry, or…
Cmux Algorithmic Complexity ✅ Passed The PR adds only two CMUX_SURFACE_ID string checks, trim() calls, and one conditional event assignment in Resources/opencode-plugin.js. These are constant work over a scalar environment value. T…
Cmux Swift Concurrency ✅ Passed The pull request changes only Resources/opencode-plugin.js. The diff adds JavaScript environment-variable handling for CMUX_SURFACE_ID; it does not modify cmux-owned Swift code or introduce any Sw…
Cmux Swift @Concurrent ✅ Passed PASS: The pull request changes only Resources/opencode-plugin.js. It adds surface_id from CMUX_SURFACE_ID to feed events. No Swift file or Swift concurrency annotation is changed, so the `@concu…
Cmux Swift Package Boundaries ✅ Passed PASS: The pull request changes only Resources/opencode-plugin.js. It introduces no production Swift changes and cannot violate the Swift package boundary rule.
Cmux Swiftpm Lockfiles ✅ Passed PASS: The PR changes only Resources/opencode-plugin.js. It does not modify SwiftPM packages, Xcode project package references, .gitignore, workflows, or dependencies. The SwiftPM lockfile policy t…
Cmux Swift Logging ✅ Passed PASS. The authoritative PR diff changes only Resources/opencode-plugin.js; it adds environment-value handling and an event field. It contains no Swift changes and no logging statements covered by `.…
Cmux User-Facing Error Privacy ✅ Passed The diff only reads and trims CMUX_SURFACE_ID, then adds the value as surface_id metadata to internal feed.push events. It adds no user-facing error, alert text, command output, API error body, …
Cmux Full Internationalization ✅ Passed PASS: The PR changes only Resources/opencode-plugin.js. It reads CMUX_SURFACE_ID, trims it, and adds the literal surface_id protocol field to feed events when present. It adds no user-facing tex…
Cmux Swiftui State Layout ✅ Passed PASS: The pull request changes only Resources/opencode-plugin.js. The diff adds environment handling for CMUX_SURFACE_ID and surface_id; it contains no Swift or SwiftUI changes. Therefore, the S…
Cmux Architecture Rethink ✅ Passed PASS: The authoritative diff changes only Resources/opencode-plugin.js. It adds trimmed CMUX_SURFACE_ID handling to the existing event builder and conditionally sets event.surface_id. It introdu…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The pull request changes only Resources/opencode-plugin.js. It adds surface_id to feed events from CMUX_SURFACE_ID; it adds no Swift window, panel, controller, SwiftUI Window, or WindowGroup…
Cmux Source Artifacts ✅ Passed The PR changes only Resources/opencode-plugin.js, an existing tracked JavaScript source file. The diff adds CMUX_SURFACE_ID handling and the surface_id event field; it does not add logs, screens…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The pull request changes only Resources/opencode-plugin.js. The authoritative diff contains no Swift file and no path under production Sources/, so the specified production Swift test/debug …
Full details: Description check

Explanation

The description clearly explains the problem and resulting behavior, but it omits the required Testing section and checklist. It does not state which tests ran, what passed, or why tests were not added. A demo is not clearly required because this is not a UI change.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@teamleaderleo
teamleaderleo merged commit 1508a9b into main Sep 26, 2026
61 checks passed
@teamleaderleo
teamleaderleo deleted the fix/opencode-feed-surface-id branch September 26, 2026 02:47
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 4d5b61bc8e: every check was green at merge (15 verified; 15 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 26, 2026
f39a1e5 Localize the Cancel button in close-confirmation dialogs (manaflow-ai#14780)
1508a9b opencode plugin: send surface_id on feed events (manaflow-ai#14781)
766c2c2 deps: bump iroh-ffi to 1.2.0-cmux.1.ios17 (iroh 1.2.0 + noq 1.3.0) (manaflow-ai#14714)
79a6ff6 Keep active pane border aligned when split zoom changes pane bounds (manaflow-ai#14646)
a3a8726 mobile: give each terminal's render-grid output its own QUIC stream (manaflow-ai#14699)
b7c3d23 fix: echo requested PID from delivery target resolution (manaflow-ai#11166)
11bcc80 ci: fill idle and briefly busy owned minis before Blacksmith (manaflow-ai#14774)

# Conflicts:
#	.github/workflows/test-e2e.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant