Skip to content

Fix Cloud reconcile for membership-less previews - #16109

Merged
teamleaderleo merged 3 commits into
mainfrom
fix/cloud-membershipless-reconcile
Sep 30, 2026
Merged

teamleaderleo merged 3 commits into
mainfrom
fix/cloud-membershipless-reconcile

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

A second local workspace bound to the same Cloud workspace could fail every projection reconcile when that workspace contained a Desktop or forwarded-port preview without a daemon membership row. The failed placement lookup aborted the pass before obsolete panes were closed or the native layout was applied.

Membership-less previews now resolve as local placements when they have no explicit daemon tab or Cloud membership token. Explicit terminal tabs and display membership tokens keep their strict stale-placement errors. A parameterized regression covers both Desktop and forwarded-port previews across two local bindings to the same Cloud workspace.

Changelog

Fixed: Cloud workspace reconciliation now completes when a second local workspace shows a preview without remote membership metadata.

Validation

  • python3 scripts/verify-local.py --swift-changed upstream/main --timeout 600 (syntax, app-source wiring, test wiring, feature flags passed)
  • git diff --check
  • Focused native test added: CloudWorkspaceLiveProjectionTests/membershiplessPreviewDoesNotBlockSecondWorkspace(isPort:)

Native app/test compilation was not run on the M5 Air; CI can compile and execute the focused suite.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes Cloud workspace reconciliation so a second local workspace bound to the same Cloud workspace no longer fails every projection pass when it contains a Desktop or forwarded-port preview without a daemon membership row. Previously, that failed placement lookup aborted the pass before obsolete panes were closed or the native layout was applied.

  • Membership-less previews now resolve as local placements when they carry no daemon tab or Cloud membership token.
  • Terminals with no view metadata still fail closed, and displays that are real daemon tabs keep their tab identity.
  • Adds regression tests for both Desktop and forwarded-port previews across two local bindings to the same Cloud workspace.

Written for commit fd5006b. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Display and forwarded-port previews without a remote tab are now handled as local previews, rather than incorrectly attempting to resolve a remote view.
    • Display previews show the matching cloud membership view when one is available; otherwise, no remote view is shown.
    • A preview without a remote tab in one workspace no longer prevents a second workspace from displaying its preview during reconciliation.

@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 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 5b612563-21b7-4e60-be02-e3572ed5889f

📥 Commits

Reviewing files that changed from the base of the PR and between da9fba1 and fd5006b.

📒 Files selected for processing (1)
  • cmuxTests/SurfaceCatalogTests.swift
 _______________________________________________
< Finding more bugs than your brain at 4:00 pm. >
 -----------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ

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: f302fedd-e9e0-4be9-af60-1e9ce13fd79c

📥 Commits

Reviewing files that changed from the base of the PR and between a302b3a and da9fba1.

📒 Files selected for processing (2)
  • Sources/Surfaces/SurfaceCatalog+CloudDisplayMembership.swift
  • cmuxTests/CloudWorkspaceLiveProjectionTests.swift

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


📝 Walkthrough

Walkthrough

When a display or forwarded-port placement has no remote tab ID or remote views, resolution now returns a matching display membership view when available. Otherwise, it returns nil. Tests cover reconciliation of previews across two bound workspaces.

Changes

Cloud preview membership

Layer / File(s) Summary
Preview resolution and reconciliation
Sources/Surfaces/SurfaceCatalog+CloudDisplayMembership.swift, cmuxTests/CloudWorkspaceLiveProjectionTests.swift
Display and forwarded-port previews without remote tab IDs return nil when no display membership view resolves. Tests verify that reconciliation projects the preview to a second workspace without recording a failure.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to da9fb

Membershipless Desktop and forwarded-port previews can reconcile locally across bound workspaces without weakening explicit placement checks. No merge-blocking issue was identified; native compilation and test execution remain for normal CI validation.

Security Architecture Review

Security architecture risk: 🔵 Low · up to da9fb

The fallback is narrowly limited to Desktop and port previews without remote placement metadata. Explicit remote placement identifiers still fail when stale, and no new access-control bypass was established. Repeated reconciliation and failure recovery remain incompletely validated.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is additional local preview panes for an existing resource on a bound Cloud machine. The inspected path does not establish newly granted cross-machine, tenant, credential, or remote-tab authority; it should not be interpreted as a complete assessment of downstream browser security.

Trust Boundaries and Controls

  • observed — The preview route still passes through destination-ownership checks. Browser configuration checks the resource's machine, expected service port, private-address URL policy, and trusted internal URL policy before configuring access. Missing membership metadata is therefore not, by itself, permission to target another machine or arbitrary origin.

Resilience and Maintainability Implications

  • observed — Workspace-scoped materialization keys prevent reconciliation opens in different local workspaces from coalescing into one pane. Completion handling discards abandoned, deleted-resource, and duplicate materializations. Ordinary reconciliation failures do not themselves enqueue an automatic retry in the inspected catch path; later requests can initiate another pass.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: fixing Cloud reconciliation for previews without membership metadata.
Description check ✅ Passed The description explains the problem, resulting behavior, regression coverage, validation commands, and changelog entry. It uses a Validation heading instead of the template's Testing heading and omit…
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 pull request changes only Cloud placement resolution and adds a reconciliation regression test. It does not create or change cmux-tui clients, transports, snapshots, manual renderers, input …
Cmux Swift Actor Isolation ✅ Passed PASS: The only production change extends SurfaceCatalog, which is explicitly @MainActor in Sources/Surfaces/SurfaceCatalog.swift. The new placement logic therefore remains MainActor-isolated and…
Cmux Swift Blocking Runtime ✅ Passed The production diff only changes placement-resolution logic in SurfaceCatalog+CloudDisplayMembership.swift. It adds no semaphores, blocking waits, sleeps, delayed dispatch, polling, main-queue sync,…
Cmux Browser Automation Off-Main ✅ Passed PASS: The PR changes only Cloud placement reconciliation and its regression test. The authoritative diff contains no changes to browser socket automation, processV2Command, socketWorkerMethods, We…
Cmux Expensive Synchronous Load ✅ Passed PASS: The production diff only changes in-memory Cloud placement resolution in SurfaceCatalog+CloudDisplayMembership.swift and adds a reconciliation test. It adds no `RestorableAgentSessionIndex.loa…
Cmux Cache Substitution Correctness ✅ Passed PASS: The production diff changes live Cloud workspace placement reconciliation only. It adds a nil local-preview result for display or forwarded-port placements with no remote tab and no remote views…
Cmux No Hacky Sleeps ✅ Passed PASS. The review-scoped diff changes only Swift files: SurfaceCatalog+CloudDisplayMembership.swift and CloudWorkspaceLiveProjectionTests.swift. The custom check applies to production non-Swift Typ…
Cmux Algorithmic Complexity ✅ Passed The production diff only changes placement branching in Sources/Surfaces/SurfaceCatalog+CloudDisplayMembership.swift. It adds no loop, nested scan, sort, filter, or per-target rescan. The display pa…
Cmux Swift Concurrency ✅ Passed The diff does not introduce or materially expand the checked legacy async patterns. The production change remains a synchronous throws placement resolver and adds no DispatchQueue, Combine state, …
Cmux Swift @Concurrent ✅ Passed The diff adds no @concurrent or nonisolated async declarations. The production change is in synchronous SurfaceCatalog.remoteView(for:placement:). The added test is intentionally @MainActor, a…
Cmux Swift Package Boundaries ✅ Passed PASS. The production diff changes only SurfaceCatalog resolution in Sources/Surfaces/SurfaceCatalog+CloudDisplayMembership.swift. The new branch reads the app-owned resource registry, accepted clo…
Cmux Swiftpm Lockfiles ✅ Passed PASS — The PR changes only Sources/Surfaces/SurfaceCatalog+CloudDisplayMembership.swift and cmuxTests/CloudWorkspaceLiveProjectionTests.swift. The diff contains no Package.swift, `Package.resolv…
Cmux Swift Logging ✅ Passed The pull request adds no logging. The production diff adds placement-resolution logic and comments only, with no print, debugPrint, dump, NSLog, ad hoc file/stdout logging, Logger, or sensit…
Cmux User-Facing Error Privacy ✅ Passed PASS. The production diff changes placement-resolution control flow and adds internal comments only. It adds no user-facing error, alert, CLI output, API body, or recovery copy. For membership-less di…
Cmux Full Internationalization ✅ Passed PASS — The PR changes reconciliation logic and adds a regression test. The only new Swift text is a developer comment and a test name, both allowed. The production error strings in `SurfaceCatalog+Clo…
Cmux Swiftui State Layout ✅ Passed The diff does not introduce SwiftUI state or layout code. The production change is in SurfaceCatalog+CloudDisplayMembership.swift, which imports CmuxSurfaceCatalogModel and Foundation and adds p…
Cmux Architecture Rethink ✅ Passed PASS. The PR makes a small local correctness change in SurfaceCatalog.remoteView(for:fallbackWorkspaceID:). It preserves strict resolution for explicit remote tab IDs and membership tokens, and trea…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS. The PR changes only Cloud surface placement resolution and a test-only projection fixture. The added lines contain no NSWindow, NSPanel, NSWindowController, SwiftUI Window/WindowGroup, close-sho…
Cmux Source Artifacts ✅ Passed The diff changes only two intentional source-control paths: a hand-written Swift source file and a hand-written Swift regression test. The changes add Cloud preview handling and test coverage. No logs…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The only changed production file is Sources/Surfaces/SurfaceCatalog+CloudDisplayMembership.swift. The PR modifies existing remoteView(for:fallbackWorkspaceID:) reconciliation logic to accept membe…
  • 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
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

The membership-less preview branch only fires for displays and forwarded
ports whose resource carries no view metadata. Nothing pinned either
restriction: deleting the kind check turned a terminal with no views from
fail-closed into a silent projection with no remote provenance, and
deleting the `remoteViews == nil` check made a display that really is a
daemon tab lose its tab identity. Both mutations survived the suite.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Sep 30, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review: land.

Executed A/B, not just reading: the changed function and its two helpers were extracted verbatim into a standalone Swift program and run old vs new over 15 inputs. Under the pre-fix branch, both of the new test's vectors (display and forwarded port, remoteViews == nil, remoteTabID == nil, ws a) throw .unavailable(_, "remote placement data is unavailable"), which flips both assertions through CloudWorkspaceProjectionCoordinator's catch into failures[second]. The test is not vacuous.

Executed coverage on da9fba16f6: macos / app-host unit tests (changed suites) ran -only-testing:cmuxTests/CloudWorkspaceLiveProjectionTests and its log shows both parameterized cases (isPort → false, isPort → true) started and passed. SurfaceCatalogTests and CloudDisplayMembershipProjectionTests ran in the same shard.

What I checked against the three ways this kind of loosening usually goes wrong:

  • A membership-less display is not treated as a member anywhere. It returns nil, not a synthesized view. recordCloudProjection then fills the missing remoteWorkspaceID from the binding, so the projection still satisfies CloudWorkspaceProjectionPlan's isLocalWorkspaceView && remoteWorkspaceID != nil. I looked for a project/retire flap on the second reconcile and there is none.
  • Empty does not become "all". The guard is resource.remoteViews == nil, not isEmpty, which is the right distinction: nil means the provider does not model views, [] means a live resource with zero views. Behaviour for [] is byte-identical to before.
  • Nothing leaks into a non-preview path. (kind == .display || id.isForwardedPort) is the same predicate SurfaceProjection.isLocalWorkspaceView and localWorkspaceMembers use. Terminals and plain browsers still fail closed.

The inner membership lookup is not dead: cloudWorkspaceResources appends a membership-augmented presentation copy and never writes back to resources, so resources[display] keeps remoteViews == nil even when membership rows exist.

Fixed: three of the four clauses in the new if are load-bearing and nothing pinned them. The mutation run left these alive — dropping the kind check turns a terminal with remoteViews == nil and a remoteWorkspaceID from fail-closed into a silent projection with no remote provenance, and dropping remoteViews == nil makes a display that really is a daemon tab resolve to nil and lose its tab identity. SurfaceCatalogTests's existing "Explicit remote placement fails closed" test looks like it covers the first one but calls the other overload, so it does not constrain this function. Pushed fd5006bde1f with two direct assertions on remoteView(for:fallbackWorkspaceID:) for exactly those two mutants.

Left, worth a follow-up rather than a commit here:

  1. The new test asserts resource == preview.id && workspaceID == second but not remoteWorkspaceID == "a", and it does not run a second reconcile to show the pane is not then retired as obsolete. Both sibling tests do. I traced that it is "a" and there is no flap, but the test would not notice if that broke.
  2. Adding placement.remoteTabID == nil is a tightening the PR comment does not mention: a display placement naming an explicit daemon tab while the resource has no remoteViews now throws instead of silently substituting the membership view. That is more correct and matches the doc comment, and I could not find a live producer of such a placement, so it looks unreachable today. Worth one sentence.
  3. Two clients attaching the same display to one workspace used to throw and now degrades to a membership-less preview. Harmless in practice (the resource then carries both views, and project's reuse predicate collapses them onto one pane), but the comment justifies an absent membership, not an ambiguous one.

Minor: line 75's resource.id.isForwardedPort is the same value as placement.resource.isForwardedPort, and line 81's resource.kind == .display is redundant because cloudDisplayMembershipView already guards on kind.

Merging on green.

— Raindrop g2 🫧

@teamleaderleo
teamleaderleo merged commit 1ad9de2 into main Sep 30, 2026
14 of 15 checks passed
@teamleaderleo
teamleaderleo deleted the fix/cloud-membershipless-reconcile branch September 30, 2026 21:21
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for fd5006bde1, merged 2026-09-30 21:21:08 UTC

  • Not verified at merge: ci-status (not reported), CI fast guards (in progress), Fast static checks (in progress), Testbox broker trust boundary (in progress), Web complexity (in progress)
  • Verified: web-validation
  • Skipped by policy: web-build, web-database-tests, web-tests
  • Full suite: runs on main after merge.

Labeled merged-unverified: if main breaks near this merge, look here first.

@github-actions github-actions Bot added the merged-unverified A judging check was not green at merge; see the merge receipt comment label Sep 30, 2026
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 30, 2026
579a979 Align Cloud machine stale status punctuation (manaflow-ai#16228)
c7d37b6 Merge pull request manaflow-ai#15806 from manaflow-ai/feat-team-settings-minimal
a62dacc docs: tell contributors to start after claiming and not wait on app-host tests (manaflow-ai#15941)
7befdc7 Fix Cloud notification pane attribution (manaflow-ai#15972)
071e912 fix(ios): prevent computer picker status clipping (manaflow-ai#15799)
20cd45f Merge remote-tracking branch 'origin/main' into feat-team-settings-minimal
1ad9de2 Fix Cloud reconcile for membership-less previews (manaflow-ai#16109)
a4c7eda Make new windows honor configured sidebar width (manaflow-ai#11539)
d1c9a6f Merge pull request manaflow-ai#16219 from manaflow-ai/feat-team-invites-app
ad16812 Fix Cloud sidebar parity after individual notification reads (manaflow-ai#15933)
285dbf7 Add live physical iOS IRX log viewer (manaflow-ai#12008)
cce8214 Fix blank terminal surface after completion notification (manaflow-ai#11954)
a4e862d Keep Codex auto-naming credentials out of argv (manaflow-ai#16201)
f14e824 Merge remote-tracking branch 'origin/main' into feat-team-invites-app
4b832ea fix: honor OpenCode XDG and database path overrides (manaflow-ai#16229)
633852a Merge remote-tracking branch 'origin/main' into feat-team-invites-app
ff131f9 Merge remote-tracking branch 'origin/main' into feat-team-invites-app
0bf5223 Merge remote-tracking branch 'origin/main' into feat-team-invites-app
f221acb Merge remote-tracking branch 'origin/main' into feat-team-invites-app
7037871 Merge remote-tracking branch 'origin/main' into feat-team-invites-app
97395db Merge origin/main into feat-team-invites-app
4443dc1 cloud: header keeps only the team row and Invite after main's layout change
8e8839c Merge origin/main into feat-team-invites-app
77c068a ci: re-trigger pull request checks
ac5842e cloud: received-invitation reads wait for the teams client bootstrap
04ce3d1 vendor/bonsplit: follow main's pointer
6d33293 Merge origin/main into feat-team-invites-app
aea3cf4 Merge origin/feat-dashboard-tanstack into feat-team-invites-app
0c0f4dc web: invite email strips control characters from names and carries no token-derived header
e775fee settings: the Invitations card loads from a zero-size anchor
e1d3272 Merge origin/feat-dashboard-tanstack into feat-team-invites-app
282354c socket: register the received-invitation methods with the capability list and execution policy
9b95924 web tests: team detail seeds an email invitation row
7b2979e cloud: received invitations in Settings, the team menu, notifications, the socket and the CLI
548d240 web: cmux sends team invitation emails through Resend and serves received invitations
3f12b3e settings: Members card strings resolve from the package bundle
ed80608 Merge branch 'feat-team-invites-app' into feat-team-settings-minimal
76c7093 cloud: the seats-open line uses the localized formatter
722fabb Merge branch 'feat-team-invites-app' into feat-team-settings-minimal
c2b5afd cloud: nonisolated socket method table; plural seats line uses the localized formatter
5a7939c settings: format plural team strings with the localized formatter
3e3e486 Merge branch 'feat-team-invites-app' into feat-team-settings-minimal
532e318 tests: header height helper drops the removed refresh and agent menu arguments
a373523 settings: plain Members card with per-row progress
550a580 web tests: carry every real stack export in dashboard mocks; drop the no-op mock restore
8d8af37 web: carry memberLimit and seat_limit into the oRPC team contract; teams-routes restores its module mocks
94f4aba Merge remote-tracking branch 'origin/feat-dashboard-tanstack' into feat-team-invites-app
688eb31 cloud: team chip uses the tab bar grey
7549cd4 cloud: Invite label uses the tab bar grey and ends under the close glyph
c2d1d39 cloud: no seat-check loading line, quiet Invite button, header aligned with the tree
1020ab9 Merge origin/feat-dashboard-tanstack into feat-team-invites-app
7d9bbc2 cloud: header keeps only Invite; empty section offers New Machine
5c2fd03 cloud: Invite is a popover on the header button; Revoke is a real button
8b35736 cloud: invite sheet fits its content; link button never wraps
2df3a09 cloud: invite sheet is a plain form
0e25775 cloud: redesign the invite sheet
4660c0a cloud: invite sheet height fits its content
ea905ca cloud: invite sheet is a fixed-size hosting view, not a preferred-size controller
5bffd69 cloud: invite sheet uses a single-line email field so the sheet can open
d3b6e85 cloud: a dedicated invite sheet explains what a member gets
5f657ae tests: seat queue SQL contract behind CMUX_DB_TEST
06f4a7c teams: seats follow membership in the docs and UI copy
3a1790a billing: Team seat quantity follows the member count
7579097 cloud header: Invite and New Machine buttons, refresh and agent launchers in an overflow menu
93c06da l10n: keep main's text for untouched keys
1ef0878 settings: team members and invites live in Settings › Account
5f7699e cli: team roster verbs wait 60s for the network round trip
d4aef24 web: log the Stack error class behind a failed team request
fee9821 web: log why a team invitation send failed
6c53598 auth: team member error mapping is nonisolated for the socket path
149993d test: synthetic team origin requests use a test host
3428fa8 test: team detail billing summary includes memberLimit
6b58941 app: the team members window owns the standard close shortcut
808c9f9 l10n: keep main's catalog order; append the team members keys
0c4553a Merge origin/main into feat-team-invites-app
13f72e9 xcodeproj: add CloudTeamMembersWindow and HostAccountFlow+TeamMembers to the app target
adda7a2 test: new auth.team socket methods run on the socket worker
e78d62d socket: hoist async roster reads out of argument lists
c763126 swift: hoist path encoding out of interpolation; focus invite field only on request
0c344bf l10n: team members and invites strings in all nine locales
6eaa946 wip: swift team invites and members (pre-localization)
1c45713 web: cap Pro and Max personal teams at 3 members
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-unverified A judging check was not green at merge; see the merge receipt comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant