Skip to content

Avoid menu resync on window activation - #1646

Closed
lawrencecchen wants to merge 2 commits into
mainfrom
task-1641-window-focus-lag
Closed

lawrencecchen wants to merge 2 commits into
mainfrom
task-1641-window-focus-lag

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Mar 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • stop menu state reads from calling synchronizeActiveMainWindowContext on every window activation
  • add a non-mutating preferredTabManager lookup for menu/UI reads so focus changes reuse the already-updated active window context

Testing

  • ./scripts/reload.sh --tag task-1641-window-focus-lag ✅
  • switched Finder -> cmux DEV task-1641-window-focus-lag via AppleScript and confirmed /tmp/cmux-debug-task-1641-window-focus-lag.log contains 0 shortcut.sync entries on the activation path

Issues


Summary by cubic

Stop resyncing menu state on window activation by using a non‑mutating tab manager lookup and centralizing focus restore. This removes focus lag, stops redundant shortcut syncs, and reliably restores terminal focus (closes #1641).

  • Refactors
    • Added AppDelegate.preferredTabManager(...) and switched activeTabManager to use it with key/main window fallback, replacing synchronizeActiveMainWindowContext for menu/UI reads.
    • Centralized window activation focus restore in AppDelegate (invoked on didBecomeKey), removed the per-view observer in GhosttyTerminalView, and updated setActiveMainWindow to return context, avoid pointer churn when unchanged, and repair missing sidebar state only when nil.

Written for commit 17c5ea6. Summary will update on new commits.

Summary by CodeRabbit

Release Notes

  • Refactor
    • Enhanced window and tab context selection logic for improved reliability.
    • Optimized active context synchronization with better state management.

@vercel

vercel Bot commented Mar 18, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Mar 18, 2026 0:58am

@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.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@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 2 files

@coderabbitai

coderabbitai Bot commented Mar 18, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

AppDelegate.swift adds a new preferredTabManager() helper method for read-only TabManager selection and updates synchronizeActiveMainWindowContext() to return a TabManager. cmuxApp.swift replaces its context synchronization call with the new helper method, using a fallback to the existing tabManager if unavailable. These changes provide an explicit access pattern for obtaining the best-fit TabManager without mutating global state.

Changes

Cohort / File(s) Summary
AppDelegate methods
Sources/AppDelegate.swift
Added preferredTabManager() helper to select the optimal TabManager based on window context hierarchy (provided window → keyWindow → mainWindow → active → first known). Updated synchronizeActiveMainWindowContext() to return a TabManager with the resolved context, enabling context-aware access patterns.
cmuxApp call update
Sources/cmuxApp.swift
Replaced synchronization call with new preferredTabManager() helper, adding null-coalescing fallback to existing tabManager to ensure a TabManager is always available.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • #766: Modifies AppDelegate's active-window/context selection logic with new helpers for choosing and restoring the preferred main window and tab manager.

Poem

🐰✨ A helper hops in, preferences held high,
No mutations dance—just read-only sighs,
Select the best context, no state left to sway,
The rabbit approves this cleaner way! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: avoiding menu resynchronization on window activation.
Linked Issues check ✅ Passed The changes directly address issue #1641 by eliminating redundant menu resync calls on window activation to reduce focus lag.
Out of Scope Changes check ✅ Passed All changes are scoped to addressing the window activation lag issue; modifications are limited to AppDelegate and cmuxApp for the stated purpose.
Description check ✅ Passed The PR description clearly explains what changed and why, includes specific testing details with script and log verification, references the closed issue, and covers all key sections.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task-1641-window-focus-lag
📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

@lawrencecchen lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 23, 2026

This branch was successfully deployed

1 active deployment
Preview — 17c5ea62 Deployed Mar 18, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Occasional lag when focusing cmux windows

2 participants