Repository navigation
test: find the onboarding window the test presented, not a leftover - #15015
Merged
Merged
Conversation
ComputerUseOnboardingWindowTests looked the main onboarding window up by identifier alone. An earlier test's onboarding window can still be in NSApp.windows, so offscreenCompanionReturnsWithoutMovingTheOverview and permissionCompanionRemainsWhenAnotherApplicationActivates both read the same stale, invisible window (0xc10ac7200) and failed on .isVisible in run 36316398822 shard 3 on cmux13s. The lookup now takes only a window that present() opened. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughWindow tests now select onboarding and permission-companion windows created by the current test action. A helper compares window identities before and after the action to avoid selecting retained windows with matching identifiers. ChangesWindow test selection
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to This test-only change selects windows created by the current action rather than stale retained windows, with cleanup registered before required lookups. Static inspection finds no concrete merge-blocking risk; normal app-host CI remains the final validation. 🚥 Pre-merge checks | ✅ 23 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (23 passed)
Full details: Description checkExplanation The description explains the failure, implementation, and intended CI verification. However, it does not use the required Summary, Testing, Changelog, Demo Video, and Checklist sections, and it does not provide the required changelog or checklist information. Resolution Rewrite the description using the repository template. Add Summary, Testing, Changelog, Demo Video, and Checklist sections. State the exact CI job or command used for verification, use
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Review: the permission companion is also kept when closed, so the five bare companion lookups could read an earlier test's panel the same way. One helper, newWindow(identified:_:), now serves both identifiers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Merge receipt for |
rustybret
pushed a commit
to rustybret/bmux
that referenced
this pull request
Sep 27, 2026
648d5c1 Add a Paste Last Screenshot action with an unbound shortcut (manaflow-ai#14955) ff61677 ci: avoid partial blobs in catch-up merges (manaflow-ai#15023) 4d0d112 ci: retry transient catch-up GraphQL failures (manaflow-ai#15021) 212e808 ci: attribution scores a lone suspect and reports app-host crashes apart (manaflow-ai#14952) 4cabdf4 test: settle the window before measuring the unread sidebar-row invalidation (manaflow-ai#14568) 12ec99b Add a release-media capture tool for changelog screenshots and clips (manaflow-ai#15010) ee2cda0 Backfill Unreleased changelog and draft next release cards (manaflow-ai#14999) be4adf8 Show a brief notice when Cmd+V fails on an oversized image or a timeout (manaflow-ai#14953) 23d22d7 ci: an owned pool the run starts on now beats an earlier one it queues on (manaflow-ai#14993) 05d0190 ci: catch-up posts once per head, says less, and merges inserted declarations (manaflow-ai#15018) 4ee4b21 ci: fail stalled Swift package tests instead of waiting out the job timeout (manaflow-ai#14997) 9ce512a merge-main: run local guards only when asked (manaflow-ai#15016) d60108a ci: clear test-e2e's fixed DerivedData with clear-dirs.sh (manaflow-ai#14994) 1d7895e ci: run the shell and CLI no-socket lanes in parallel (manaflow-ai#14990) 6e7d25f Honor macOS Differentiate Without Color, Increase Contrast and Reduce Transparency (manaflow-ai#14991) 966b355 Stop interrupting focused work: sidebar jumps, Computer Use focus steal, quit dialog on logout (manaflow-ai#14961) e1f1cb2 Strip control characters from feedback attachment filenames (manaflow-ai#14783) 0758c9f test: find the onboarding window the test presented, not a leftover (manaflow-ai#15015) b35c540 fix(spm): resolve GhosttyKit/GhosttyRuntimeTestStubs target name collisions (manaflow-ai#10569) ef33bed Map .purs artifacts to the Haskell highlight.js grammar (manaflow-ai#14202) e2a167a Highlight Elixir and Erlang files in the file editor (manaflow-ai#13732) 972c449 fix: wrap Linux browser download card label (manaflow-ai#11157) f563884 Add Aside to browser data import detection (manaflow-ai#13379) 091d0ea Add cmux send --paste and hint at it for large multi-line sends (manaflow-ai#14937) 3ffcdbb test(ios): keep folder-tap stat tests off the real 2 s deadline (manaflow-ai#15017) 68d3936 test: keep CmuxTerminal pasteboard tests off the cooperative pool (manaflow-ai#15006)
teamleaderleo
added a commit
that referenced
this pull request
Sep 27, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Run 36316398822 (PR #14974), app-host shard 3 on cmux13s, failed two
ComputerUseOnboardingWindowTests:offscreenCompanionReturnsWithoutMovingTheOverviewpermissionCompanionRemainsWhenAnotherApplicationActivatesBoth failed on
.isVisible → false, and both read the same window,0xc10ac7200. Each test presents its own controller, so a window shared by the two can only be a leftover. The tests looked up the main onboarding window by identifier alone. An earlier test's onboarding window can still be inNSApp.windows, closed but retained or left open, and the lookup found that one. This is a test isolation bug, not the machine: the hook's console gate admitted the job on an unlocked session, and the other GUI tests in the shard passed.What
newWindow(identified:_:)snapshotsNSApp.windows, runs the action and returns only a window with that identifier the action opened. It serves the two main-window lookups afterpresent()and the five permission-companion lookups afterconfigureForPermissionCompanion, since the companion is also kept when closed (subagent review).defer { controller.dismiss() }now comes before the#require, so a failed lookup still dismisses.Verification
Per the no-local-tests rule on Air Blue, this runs in the app-host shards on CI.
🤖 Generated with Claude Code
Summary by CodeRabbit