Skip to content

test: hit-test the browser portal tab strip with its own click - #15031

Merged
teamleaderleo merged 2 commits into
mainfrom
fix/browser-host-tabstrip-hittest-event
Sep 27, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
fix/browser-host-tabstrip-hittest-event

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

WindowBrowserHostViewTests.testHostViewPassesThroughUnderlyingTabStripInSecondWindowBelowTitlebarBand fails on main when other suites run first in the same app host (for example run 36326938624, app-host shard 5/7). When it runs alone it passes (run 36332446194).

The test calls hitTest(_:), which routes on the ambient NSApp.currentEvent. A probe run that logged it before the assertions (36334701691) found a real MouseEntered for another suite's window (winNum=19820). Hover events check only registered tab-bar regions (BonsplitTabBarPassThrough.usesRegisteredTabBarRegionsOnly), so the test's unregistered fake strip was not passed through. Real minimal-mode tab strips register their regions, and clicks still use the fallback, so the product behaves as designed. The test just never said which event it meant.

The first commit reproduces the failure deterministically: it leaves a stale mouseEntered for another window as NSApp.currentEvent, which is what the probe found. The second commit makes the test hit-test with its own left-mouse-down for each window and a private drag pasteboard, through performHitTest(at:currentEvent:dragPasteboard:), as the sibling Dock divider tests already do. The stale hover stays in the test as a guard.

Testing

Focused runs through scripts/run-e2e.sh on the owned mini lane (glaeda-std-xcode-26.6). I checked each log to confirm the test executed:

Commit Selection Result
d762999 (main c842f7d + logging) the test's suite plus 5 XCTest suites that precede it in shard 5 failed at both assertions, currentEvent = MouseEntered
9c41d18 (regression) the test alone failed at both assertions (1494, 1498)
a938445 (fix) the test alone passed
a938445 (fix) the same shard-5 composition, 105 XCTests passed

The composition is a subset of the shard, because run-e2e.sh caps its concurrency group at 400 characters. Its selectors are BrowserSessionHistoryRestoreTests, BrowserWindowPortalLifecycleTests, TerminalWindowPortalLifecycleTests, TabManagerReopenClosedBrowserFocusTests, TerminalNotificationCallerTests and WindowBrowserHostViewTests. Whether a stray hover event arrives depends on where the pointer sits, so this composition doesn't always fail on main: 36334723121 at badf9f6 passed. That's why the regression commit sets the event explicitly.

Changelog

none

🤖 Generated with Claude Code

teamleaderleo and others added 2 commits September 27, 2026 10:17
…hover event

testHostViewPassesThroughUnderlyingTabStripInSecondWindowBelowTitlebarBand
fails on CI when other suites share its app host: its hitTest(_:) calls
route on NSApp.currentEvent, and a probe run showed that was a real
mouseEntered for another window. Leave the same stale event before the
assertions so the failure no longer depends on where the pointer sits.

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

The test called hitTest(_:), which routes on the ambient NSApp.currentEvent.
After other suites ran in the same app host that was a mouseEntered for
one of their windows, and hover events use only registered tab-bar
regions, so the unregistered fake strip was not passed through. Pass a
left-mouse-down for each window and a private drag pasteboard, as the
sibling Dock divider tests already do.

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

coderabbitai Bot commented Sep 27, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: c000cd13-f188-4cc6-ba49-1601d15860cf

📥 Commits

Reviewing files that changed from the base of the PR and between 8efe28d and a938445.

📒 Files selected for processing (1)
  • cmuxTests/BrowserPanelTests.swift

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.

@github-actions

Copy link
Copy Markdown
Contributor

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

@teamleaderleo
teamleaderleo merged commit f412b05 into main Sep 27, 2026
52 of 53 checks passed
@teamleaderleo
teamleaderleo deleted the fix/browser-host-tabstrip-hittest-event branch September 27, 2026 18:12
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for a9384455c4: every check was green at merge (16 verified; 14 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 27, 2026
a64d59b tools: ui-lab renders view code in seconds; wire-app-sources.py (manaflow-ai#15049)
4e03ed2 fix(events): harden durable replay recovery (manaflow-ai#15054)
ac51546 Settings: native terminal theme gallery (manaflow-ai#14996)
867e7a0 Add native Ghostty option rows to Settings > Terminal (manaflow-ai#15005)
7f97b0d ui-tests: wait for static preflight when a reused compile skips the gate (manaflow-ai#15051)
c708e0c Add a chat view for the terminal's agent session (Claude Code, Codex) (manaflow-ai#14965)
b762a3d ci: the picker fetches kept bases' trees, not just checks their commits (manaflow-ai#15053)
2570eed docs: refresh and trim contributor build guidance (manaflow-ai#15050)
20019d3 ci: re-run by cause: host faults to Blacksmith, code failures back to the minis (manaflow-ai#15045)
36ee3e9 Add Warn Before Closing Workspace setting (manaflow-ai#14979)
4df2317 CI: run changed UI test classes in PRs, keep UI runs off Blacksmith, probe the GUI session (manaflow-ai#14964)
9efe05e Owned-pool sweeper: page the marker listing back to the runs it adopts (manaflow-ai#15033)
6361554 fix(events): restore durable replay across restarts (manaflow-ai#15030)
0bc5145 ci: place side lanes on the light minis one per idle side runner (manaflow-ai#15047)
9d4e92b ci: the E2E rule's queue-round reason names the owned pools the run may take (manaflow-ai#15044)
9e6e216 Dogfood the app from CI with JSON tours (manaflow-ai#14928)
fd3dcf6 ci: retry the picker's kept-base fetch and record how it went (manaflow-ai#15040)
3a64e0e Reload the Ghostty config when its files change, and show config errors (manaflow-ai#14859)
f412b05 test: hit-test the browser portal tab strip with its own click (manaflow-ai#15031)
64d5235 test: route the reopen-last-closed shortcut through the test's own window (manaflow-ai#15036)
7037079 ci: take the gui token in the E2E test job's step, not at job start (manaflow-ai#15037)

# Conflicts:
#	.github/workflows/ci-macos.yml
#	.github/workflows/ci.yml
#	.github/workflows/remote-daemon.yml
#	.github/workflows/test-e2e.yml
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