fix: exclude spaceless windows from narrowed Space scopes - #192
nikolaskaralis wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f475d994e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // | ||
| // Explicit expanded-tab rows remain eligible because the user asked | ||
| // to surface native tabs individually. | ||
| if spaces.confirmedSpaceless.contains(row.cgWindowID), !row.isTabSibling { |
There was a problem hiding this comment.
When Stage Manager is enabled and at least one catalog window resolves normally, WindowServer can report real off-stage titled windows as confirmedSpaceless; WindowEnumerator.resolveTabStacks intentionally leaves those windows as ordinary, non-isTabSibling rows, and the existing phantom filter preserves them via its title safeguard. This unconditional check removes them under Current Space or Visible Spaces, undoing the Stage Manager behavior from #122 and hiding real user windows from the switcher. The predicate needs to distinguish collapsed native-tab artifacts without discarding titled Stage Manager windows (and similarly protected real minimized windows).
Useful? React with 👍 / 👎.
| if spaces.confirmedSpaceless.contains(row.cgWindowID), !row.isTabSibling { | ||
| dropOffsets.insert(offset) | ||
| continue |
There was a problem hiding this comment.
When the Space API succeeds but every queried window is reported with zero memberships, allowedSpaces and confirmedSpaceless are populated while spaceByWindow is empty. The existing guard returns before this new branch executes, so a lone or all-spaceless ordinary row is still retained and activating it can cause exactly the unwanted Space materialization this change targets. Availability should be determined from allowedSpaces, not from whether any window happened to resolve to one Space.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
The current early-return guard in filterToAllowedSpaces prevents the new confirmed-spaceless filtering from running when spaceByWindow is empty, leaving the targeted spaceless-row leak unfixed in that scenario.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes a Space-scoping edge case where WindowServer-reported “confirmed spaceless” NSWindows (notably native AppKit background tabs) could leak into narrowed Space scopes (Current/Visible), and activating them could cause macOS to materialize them onto the current Space.
Changes:
- Drop confirmed-spaceless ordinary switcher rows when the Space scope is narrowed, while keeping explicit
isTabSiblingrows eligible. - Add a regression test covering ordinary spaceless rows vs tab-sibling rows under a narrowed Space scope.
File summaries
| File | Description |
|---|---|
| BetterCmdTab/Catalog/CatalogFilter.swift | Updates Space-scope filtering to exclude confirmed-spaceless non-tab-sibling rows when narrowing to allowed Spaces. |
| BetterCmdTabTests/SpacelessSpaceFilterTests.swift | Adds a new test validating that narrowed Space scopes drop ordinary spaceless rows but keep explicit tab-sibling rows. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| #expect(keptIDs == [10, 30]) | ||
| } | ||
| } |
| static func filterToAllowedSpaces(_ rows: [SwitcherRow], _ spaces: SpaceResolution) -> [SwitcherRow] { | ||
| guard !spaces.allowedSpaces.isEmpty, !spaces.spaceByWindow.isEmpty else { return rows } | ||
| var dropOffsets = Set<Int>() |
Summary
Prevent confirmed-spaceless windows from appearing as ordinary switcher rows when the Space scope is narrowed to Current Space or Visible Spaces.
Native AppKit background tabs (e.g. TextEdit / Preview) can remain real NSWindows while tabbed away, but WindowServer reports them as belonging to no Space. Previously,
filterToAllowedSpaceskept these rows because it only rejected windows positively resolved to another Space.If one of these background-tab windows is then selected by BetterCmdTab, activating it can cause macOS to materialize it on the currently visible Space.
The filter now drops confirmed-spaceless ordinary rows under a narrowed Space scope.
isTabSiblingrows are deliberately exempt so "Expand native tabs as windows" continues to work as requested.Reproduction
Testing
Manual A/B isolation:
Regression test:
[10, 20, 30]is retained, incorrectly keeping the ordinary spaceless row[10, 30], while the explicit native-tab sibling remains available