Skip to content

Fix copied surface link live identity - #9786

Merged
lawrencecchen merged 2 commits into
mainfrom
feat-surface-link-identity
Aug 7, 2026
Merged

lawrencecchen merged 2 commits into
mainfrom
feat-surface-link-identity

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Aug 7, 2026 •

Copy link
Copy Markdown
Contributor

Fix Copy Surface Link so copied surface links use the same live identity as Copy IDs and the socket tree.

Root cause:

  • Copy IDs and the control socket tree use Workspace.id and live surface IDs.
  • Copy Surface Link used Workspace.stableId and Panel.stableSurfaceId.
  • ContentView.copyFocusedSurfaceLink() built the link inline, so the command palette path bypassed the shared helper.

Fix:

  • WorkspaceSurfaceIdentifierClipboardText.makeSurfaceLink(workspace:panelId:) now resolves the ownership target and emits workspace.id plus the live target surface ID.
  • ContentView.copyFocusedSurfaceLink() now calls the shared helper.
  • Tests cover the command-palette copy path and the remote-tmux projected-pane copy path.

Verification:

  • Red test commit 163faca9b1c9f94f14eb5e295858bf0a9d39a2bd: cmuxTests/CmuxDurableDeepLinkRestoreTests failed in https://github.com/manaflow-ai/cmux/actions/runs/31151528078 because copiedSurfaceLinkMatchesCopyIdsLiveIdentity() saw cmux-dev://workspace/1FF69241-F971-4941-B1C4-FF6FFADAE43A/surface/2654739B-E6B9-4D11-9406-0292D9E84346 instead of live workspace.id and panel.id values C64FB2DD-AF16-4C33-AA3A-09F88A6BED93 and 2C8401A5-C051-4B52-8FF9-02855C79F21A.
  • Fixed branch d6f313aa4a8194c4fd46f2184231fbe0cead2a81: cmuxTests/CmuxDurableDeepLinkRestoreTests passed in https://github.com/manaflow-ai/cmux/actions/runs/31150382314.
  • Fixed branch d6f313aa4a8194c4fd46f2184231fbe0cead2a81: cmuxTests/RemoteTmuxNotificationLifecycleTests passed in https://github.com/manaflow-ai/cmux/actions/runs/31151227252.
  • Cloud reload built cmux DEV slink2.app with ./scripts/reload-cloud.sh --tag slink2 --builder fleet --no-remote-derived-cache.
  • Isolated preflight used bundle com.cmuxterm.app.debug.slink2 and socket /tmp/cmux-debug-slink2.sock, not the default socket. Copy IDs returned workspace_id=8C25B979-6F70-4A44-9F62-18DC127EC319 and surface_id=E8712CAF-3307-432F-875D-4C8A5F947F3A. Copy Surface Link returned cmux-dev-slink2://workspace/8C25B979-6F70-4A44-9F62-18DC127EC319/surface/E8712CAF-3307-432F-875D-4C8A5F947F3A. tree --all on /tmp/cmux-debug-slink2.sock contained the workspace ID at line 80 and the surface ID at line 103.

Note

Medium Risk
Changes navigation URL parsing, link generation, and resolution paths used for focus and deep links; behavior is well covered by tests but touches session-restore edge cases.

Overview
Copy Surface Link now matches Copy IDs: links use live workspace.id and the ownership target’s live surface ID, with optional stable_workspace_id / stable_surface_id query parameters for durability. The command palette path calls the shared makeSurfaceLink(workspace:panelId:) helper instead of building stable-only URLs inline.

Deep link handling parses those stable query params into CmuxNavigationURLRequest.stableFallback, and AppDelegate resolves full requests (not just path targets). When live path IDs are stale—e.g. after session restore remaps panel IDs—CmuxNavigationTargetResolver falls back to stable IDs. Surface descriptors also register runtime surface aliases (including remote tmux projected pane IDs) so links to owned/projected surfaces resolve to the container panel.

Tests cover live-vs-stable parity, restore after ID remap, owned runtime IDs, and remote tmux copy-link resolution.

Reviewed by Cursor Bugbot for commit 049f961. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Fixes “Copy Surface Link” to use live workspace.id and live surface IDs, with stable-ID fallbacks so links keep working after restores, remaps, and remote tmux projections. Deep links now parse these fallbacks and resolve via the command palette and projected panes.

  • Bug Fixes
    • WorkspaceSurfaceIdentifierClipboardText.makeSurfaceLink(workspace:panelId:) now emits live IDs and appends stable_workspace_id / stable_surface_id query params; added a helper to build links with fallbacks.
    • CmuxNavigationURLRequest.parse accepts and validates those fallback params and stores them as stableFallbackWorkspaceId / stableFallbackSurfaceId; surfaceLink(...) can include them when generating URLs.
    • AppDelegate passes the full request to CmuxNavigationTargetResolver, which adds resolve(_ request:) to use fallbacks when live IDs are stale and maps runtimeSurfaceIds (e.g., projected panes) back to the container panel.
    • Workspace.cmuxNavigationDescriptor now includes runtimeSurfaceIds for surfaces; ContentView.copyFocusedSurfaceLink() calls the shared helper.
    • Tests updated for live-ID parity with Copy IDs, fallback resolution after panel ID remaps across restores, and remote tmux projected-pane links.

Written for commit 957397c. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Surface links now preserve live workspace and panel identities, with stable fallback identifiers for reliable navigation.
    • Deep links continue to resolve after panel IDs change during session restoration.
    • Copied links are generated only when valid; failures provide immediate feedback.
  • Bug Fixes

    • Improved navigation and restoration for remote tmux surfaces and legacy links.
    • Added validation to reject malformed or unsupported link parameters.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Surface links now carry live workspace and panel identities with stable workspace and surface fallbacks. URL parsing and target resolution support these identifiers across panel remapping and session restoration.

Changes

Live surface-link identity

Layer / File(s) Summary
Live identity link construction
Sources/CmuxNavigationSurfaceDescriptor.swift, Sources/Workspace+CmuxNavigationDescriptor.swift, Sources/WorkspaceSurfaceIdentifierClipboardText.swift, Sources/CmuxSSHURLRequest.swift, Sources/ContentViewIdentifierCopyCommands.swift
Surface descriptors collect runtime aliases. Link construction includes live identifiers and optional stable fallbacks. Copy commands copy only successfully generated links.
Fallback-aware target resolution
Sources/CmuxSSHURLRequest.swift, Sources/CmuxNavigationTargetResolver.swift, Sources/AppDelegate+CmuxNavigationDeepLinks.swift
Navigation URLs validate fallback parameters. Resolution checks runtime aliases before stable workspace and surface identifiers.
Live identity restoration validation
cmuxTests/CmuxDurableDeepLinkRestoreTests.swift, cmuxTests/CmuxNavigationTargetResolverTests.swift, cmuxTests/RemoteTmuxNotificationLifecycleTests.swift
Tests cover generated links, parsed requests, runtime alias resolution, stale IDs, panel remapping, and repeated session restoration.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • manaflow-ai/cmux#8695: Adds workspace-ID preservation that this change extends with runtime and stable surface-link fallbacks.
  • manaflow-ai/cmux#9766: Uses related runtime-first and restart-stable identifier resolution for control-socket commands.
  • manaflow-ai/cmux#9110: Modifies remote tmux identity handling used by surface and workspace routing.

Suggested reviewers: austinywang

Sequence Diagram(s)

sequenceDiagram
  participant CopyCommand
  participant LinkBuilder
  participant URLParser
  participant TargetResolver
  participant RestoredWorkspace
  CopyCommand->>LinkBuilder: build link with live workspace and panel IDs
  LinkBuilder-->>CopyCommand: URL with stable fallback IDs
  CopyCommand->>URLParser: parse surface URL
  URLParser-->>TargetResolver: navigation request
  TargetResolver->>RestoredWorkspace: resolve runtime alias or stable fallback
  RestoredWorkspace-->>TargetResolver: containing panel
  TargetResolver-->>CopyCommand: resolved navigation target
Loading

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (3 errors, 1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Cmux Algorithmic Complexity ❌ Error Sources/Workspace+CmuxNavigationDescriptor.swift:22 sorts runtime surface aliases for every panel; this is repeated O(k log k) work on scalable remote-tmux pane data with no bound or benchmark. Preserve control-pane order while deduplicating with a Set, or cache the sorted aliases; add a benchmark and documented bound if sorting is required.
Cmux Swift Package Boundaries ❌ Error The diff materially expands pure Foundation deep-link parsing and target resolution in app Sources; the resolver is explicitly value-based and unit-testable without cmux UI. Create Packages/macOS/CmuxNavigation and move the pure URL request, descriptors, resolution, and resolver there; expose CmuxNavigationURLRequest first. Keep Workspace/AppDelegate/AppKit glue in the app.
Cmux No Ambient Global State ❌ Error Sources/WorkspaceSurfaceIdentifierClipboardText.swift:76 adds a new static API to the caseless, static-only namespace enum, extending ambient behavior instead of using an injectable owner. Move surface-link construction to an owning constructable type, such as Workspace or an injected link builder; keep only private/fileprivate formatting helpers at file scope.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Cmux User-Facing Error Privacy ❓ Inconclusive Investigation is still in progress. Inspect changed production paths and all parse-error consumers before deciding.
✅ Passed checks (20 passed)
Check name Status Explanation
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 Swift Actor Isolation ✅ Passed Production changes add only value types and pure resolver/parser logic; no new Sendable reference types or service protocols. The Workspace-dependent helper remains explicitly @MainActor, and app t...
Cmux Swift Blocking Runtime ✅ Passed The production diff adds no semaphores, waits, sleeps, delayed dispatch, timers, polling, main-queue sync, or manual locks; added loops only index and validate IDs.
Cmux Browser Automation Off-Main ✅ Passed The PR does not change TerminalController.swift or ControlCommandExecutionPolicy.swift, and its diff contains no browser.* automation, WebKit waits, worker routing, or policy-test changes.
Cmux Expensive Synchronous Load ✅ Passed The production diff adds no agent-history/file/JSON loader. Changed copy and deep-link paths use in-memory workspace, panel, and remote-tmux topology data only.
Cmux Cache Substitution Correctness ✅ Passed The diff reads live Workspace.id, target.surfaceID, panels, and freshly built descriptors; it does not substitute a cache in a persistence, history, undo, or snapshot path.
Cmux No Hacky Sleeps ✅ Passed The PR changes only Swift source and Swift test files; no TypeScript, JavaScript, shell, or build/runtime script diff is in scope for this check.
Cmux Swift Concurrency ✅ Passed The diff adds no DispatchQueue, Task, Combine, completion-handler, or fire-and-forget patterns; production changes remain synchronous identity and URL logic.
Cmux Swift @Concurrent ✅ Passed The PR adds no async, await, nonisolated, @concurrent, Task, or DispatchQueue code. Changed link and descriptor paths are synchronous; Workspace and AppDelegate access remains actor-isolated.
Cmux Swiftpm Lockfiles ✅ Passed The diff changes only Swift source and test files. It contains no Package.swift, Package.resolved, Xcode project, .gitignore, workflow, or dependency changes covered by the rule.
Cmux Swift Logging ✅ Passed The production Swift diff adds no print, debugPrint, dump, NSLog, ad hoc I/O logging, Logger declarations, or diagnostic data logging.
Cmux Full Internationalization ✅ Passed The production diff adds no user-facing natural-language text or catalog/web changes; added URL/query identifiers are literal protocol tokens, and existing Copy Surface Link text remains localized.
Cmux Swiftui State Layout ✅ Passed The diff adds no new SwiftUI state, GeometryReader, lazy/list row store references, or render-time state writes; the ContentView change is an event-handler link-copy update.
Cmux Architecture Rethink ✅ Passed The diff adds no timing, blocking, observer, lock, or side-channel repair. It centralizes link creation in the shared helper and uses immutable descriptor-based resolution with explicit runtime/sta...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The diff adds no standalone window or close-shortcut code, and scripts/lint_auxiliary_window_close_shortcuts.py passes for all 35 existing identifiers.
Cmux Source Artifacts ✅ Passed All 10 changed paths are Swift source or test files under Sources/ and cmuxTests/. No artifact-like directories, binary files, generated outputs, or scratch paths appear in the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The production diff adds no DEBUG/test-build guard, seam-named member, or visibility widening with a wrapper; the existing authURLDebugSummary seam is unchanged.
Title check ✅ Passed The title clearly identifies the primary change: copied surface links now use live identity.
Description check ✅ Passed The description explains the change, root cause, testing, and verification, but omits some template sections.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-surface-link-identity

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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d6f313a. Configure here.

Comment thread Sources/WorkspaceSurfaceIdentifierClipboardText.swift
@lawrencecchen
lawrencecchen force-pushed the feat-surface-link-identity branch from d6f313a to 469ef7d Compare August 7, 2026 06:29
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@lawrencecchen
lawrencecchen force-pushed the feat-surface-link-identity branch from 469ef7d to 049f961 Compare August 7, 2026 06:51
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@lawrencecchen
lawrencecchen force-pushed the feat-surface-link-identity branch from 049f961 to 957397c Compare August 7, 2026 06:56
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@lawrencecchen
lawrencecchen merged commit 7daa9c6 into main Aug 7, 2026
7 checks passed
@lawrencecchen
lawrencecchen deleted the feat-surface-link-identity branch August 7, 2026 07:25
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