Skip to content

Keep Cloud focus lookup indexed - #16348

Open
teamleaderleo wants to merge 2 commits into
mainfrom
fix/cloud-focus-indexed-lookup
Open

teamleaderleo wants to merge 2 commits into
mainfrom
fix/cloud-focus-indexed-lookup

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Cloud pointer and input focus convergence repeatedly resolves the owning workspace on the terminal event path. The resolver then scans the manager's full workspace array, adding avoidable latency as a window accumulates workspaces and making the visible focus ring more likely to lag behind the pane receiving input.

Fix

Use TabManager.workspacesById for the final ownership lookup. The index is maintained whenever the workspace collection changes, so Cloud focus convergence remains constant time while retaining the existing ownership and retired-workspace checks.

Follow-up to #16271.

Validation

  • git diff --check
  • python3 scripts/verify-local.py --swift-changed (16/16 selected checks passed; native checks skipped on the MacBook Air)

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

Uses the maintained workspacesById index for cloud pointer and input focus ownership lookup instead of scanning the full workspace array, keeping focus convergence constant time as a window accumulates workspaces. Also moves the vmReadyPollInterval override test into the CLI target, where the CLI type lives, so cmuxTests compiles.

Written for commit 0990d11. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Performance

    • Workspace retrieval now uses a direct lookup, which can make accessing a live workspace more efficient. The behavior of workspace access remains unchanged.
  • Tests

    • Added coverage for how the CLI handles valid, missing, and invalid virtual-machine readiness polling interval settings.

@cursor

cursor Bot commented Oct 1, 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.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

🧰 Additional context used
📚 Code guidelines (3)
.github/review-bot-rules/swift-architectural-rethink.md — configured
.github/review-bot-rules/source-control-artifacts.md — configured
.github/review-bot-rules/swiftpm-package-resolved.md — configured

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: c0d9dc15-f580-44a0-85b3-0add5863e7b6

📥 Commits

Reviewing files that changed from the base of the PR and between e6a71ad and 0990d11.

📒 Files selected for processing (3)
  • cmux.xcodeproj/project.pbxproj
  • cmuxCLITests/CLIVMReadyPollIntervalTests.swift
  • cmuxTests/CLIVMTransferTests.swift
💤 Files with no reviewable changes (1)
  • cmuxTests/CLIVMTransferTests.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

Workspace.liveWorkspace(id:) now uses the tab manager’s workspacesById index instead of scanning tabs. New CLI tests check poll interval environment overrides, and an oversized-override assertion is removed from an existing test.

Changes

Workspace lookup

Layer / File(s) Summary
Indexed workspace lookup
Sources/Surfaces/Workspace+CloudManualMirror.swift
liveWorkspace(id:) uses workspacesById instead of scanning tabs. Its documentation describes the lookup as constant time.

CLI poll interval tests

Layer / File(s) Summary
Poll interval test coverage
cmuxCLITests/CLIVMReadyPollIntervalTests.swift, cmuxTests/CLIVMTransferTests.swift, cmux.xcodeproj/project.pbxproj
Parameterized tests check valid overrides ("0.05" and "3") and expect the default interval for missing, malformed, zero, negative, nan, and "3600" values. The Xcode project registers the test file in the CLI test target. An existing transfer test no longer asserts the fallback for an oversized override.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to 0990d

No material behavior issue is established in the workspace lookup or poll-interval test changes; the PR appears ready to merge.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1… 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 main change: keeping Cloud focus lookup indexed.
Description check ✅ Passed The description explains the problem, fix, and validation, and gives useful context about the test changes. It omits the template’s Changelog and Checklist sections, and provides no demo despite descr…
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 The diff changes only Workspace.liveWorkspace(id:) from a tabs.first search to tabManagerFor(tabId:)?.workspacesById[id]; it leaves Cloud pane creation, transport, input, and admission behavior …
Cmux Swift Actor Isolation ✅ Passed The only production Swift change replaces tabs.first with workspacesById[id] in Workspace.liveWorkspace(id:). The surrounding Workspace extension is explicitly @MainActor, and the change add…
Cmux Swift Blocking Runtime ✅ Passed The only production Swift change replaces tabs.first { $0.id == id } with workspacesById[id] in Workspace.liveWorkspace(id:). It adds no blocking wait, sleep, delayed dispatch, polling, main-que…
Cmux Browser Automation Off-Main ✅ Passed The check does not apply to this diff. The only production change replaces the workspace-array scan in Workspace.liveWorkspace(id:) with workspacesById[id]. The other changes update VM poll-interv…
Cmux Expensive Synchronous Load ✅ Passed The production Swift diff changes only Workspace.liveWorkspace(id:): it replaces a linear tabs.first lookup with workspacesById[id]. The changed path adds no agent-history loader, file read, dir…
Cmux Cache Substitution Correctness ✅ Passed The only production change replaces the workspace-array scan with TabManager.workspacesById. This lookup serves live workspace ownership and Cloud focus routing, not persistence, history, undo, or a d…
Cmux No Hacky Sleeps ✅ Passed The PR changes only Swift files and the Xcode project file. The project-file edits register a Swift test, and the new interval checks are deterministic test code. The production lookup change is Swift…
Cmux Algorithmic Complexity ✅ Passed The production change replaces tabs.first(where:) in Workspace.liveWorkspace(id:) with workspacesById[id]. TabManager maintains this dictionary when the workspace list changes, so the per-work…
Cmux Swift Concurrency ✅ Passed The diff does not introduce or materially expand a legacy async pattern. The workspace change replaces a synchronous array lookup with a synchronous workspacesById lookup. The new CLI test uses a sy…
Cmux Swift @Concurrent ✅ Passed The changed lookup is a synchronous function. The PR changes its array scan to a workspacesById dictionary lookup and adds no async, nonisolated, or @concurrent declarations. The new tests are…
Cmux Swift Package Boundaries ✅ Passed The production diff changes only Workspace.liveWorkspace(id:) to read TabManager.workspacesById. The resolver uses AppDelegate.shared and tabManagerFor, so it remains app-lifecycle workspace l…
Cmux Swiftpm Lockfiles ✅ Passed The diff changes cmux.xcodeproj/project.pbxproj only to register a test source; it adds no SwiftPM package references or dependency pins. No Package.swift, .gitignore, workflow, or `Package.reso…
Cmux Swift Logging ✅ Passed The production Swift diff only updates documentation and replaces the workspace-array scan with an indexed lookup. It adds or materially changes no logging. The other Swift changes are tests, which th…
Cmux User-Facing Error Privacy ✅ Passed The production diff changes only the workspace lookup from a tabs scan to workspacesById and adds a developer-only performance comment. It adds no user-facing error, alert, command output, API err…
Cmux Full Internationalization ✅ Passed The production change replaces a workspace-array scan with an indexed lookup. Its added text is a developer-only code comment. The remaining changes are test code and Xcode test-target wiring. The PR …
Cmux Swiftui State Layout ✅ Passed The diff does not introduce a SwiftUI view or state-layout pattern covered by the check. Its application-code change replaces a workspace-array scan with an indexed lookup; the other Swift changes add…
Cmux Architecture Rethink ✅ Passed The Swift change is a small lookup correction. liveWorkspace(id:) still resolves the owning TabManager through tabManagerFor(tabId:), then reads that manager’s workspacesById index instead of …
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR does not add or materially change a standalone cmux-owned window. The production Swift change replaces a workspace-array scan with an indexed lookup. The other Swift changes move interval asser…
Cmux Source Artifacts ✅ Passed The changed paths contain Swift product source, Xcode project configuration, and an intentional test source file. The remaining test change removes an assertion. No generated output, caches, scratch d…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR changes only Workspace.liveWorkspace(id:) in production Swift source, replacing a scan of tabs with a lookup in workspacesById. The diff adds no test/debug-guarded extension, test/debug-n…
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch fix/cloud-focus-indexed-lookup
🧪 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.

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 1 file

Re-trigger cubic

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Dogfood tours of e6a71add

sidebar-and-chrome-tour at e6a71add: not run

skipped: CI left no app build for this head (its compile failed or was cancelled)

Tours are picked by the paths globs in dogfood/scenarios/*.json; a Dogfood-tours: a, b line in the description picks them instead (none turns this off). Look at every frame before merging: a green tour only means no step failed.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

CI failure attribution

CI failed on e6a71addd8 (run 36812138762 attempt 3): 1 code.

Job Verdict Why
macos / macOS compile admission code a compile error
Matched log lines
macos / macOS compile admission: /tmp/cmux-ci/src/cmuxTests/CLIVMTransferTests.swift:730:21: error: type 'CMUXCLI' (aka 'CmuxTuiRemoteRouting') has no member 'vmReadyPollInterval'

Not re-run automatically: macos / macOS compile admission is not a machine failure.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

…ompiles

Co-authored-by: Austin Wang <austinwang115@gmail.com>
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Follow-up 0990d11dac9 restores the app-host test build that was failing on this PR's compile-admission lane (CLIVMTransferTests was calling a CLI-only vmReadyPollInterval helper through the app target's routing alias).

The pure interval-policy assertions now live in cmuxCLITests, the process-level test keeps only the bundled CLI exercise, and the project wiring is restored. Test-wiring, project normalization, and diff checks pass locally.

@cursor

cursor Bot commented Oct 6, 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.

This branch has not been deployed

No deployments
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.

2 participants