Repository navigation
Present terminal renderers in on-screen windows that never report an occlusion .visible bit (fixes the display-liveness CI regression from #10815) - #10922
Conversation
…occlusion .visible bit #10815 gates renderer presentation on NSWindow.occlusionState.contains(.visible). On the CI display-churn harness the app runs on a CGVirtualDisplay where AppKit never raises that bit for a window that is ordered in and drawing, so the renderer was never presented and DisplayResolutionRegressionUITests counted 0 terminal presents (the step last passed before #10815 landed). One rule now decides window visibility (TerminalRendererWindowVisibility): the occlusion bit or key window wins; until a window has reported .visible at least once its ordinary on-screen state (visible, not miniaturized, on the active Space) is trusted. Once the bit has been seen the occlusion verdict is honored, so miniaturized, covered, and inactive-Space windows still release GPU as #10815 intended. Key/main transitions and screen changes re-evaluate the rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe change adds a public renderer-window visibility predicate, integrates it with ChangesRenderer window visibility
QUIC keepalive test timing
Mobile and surface asynchronous access
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The PR updates terminal rendering for headless display windows and includes focused tests; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant AppKitWindow
participant GhosttyNSView
participant TerminalRendererWindowVisibility
participant Renderer
AppKitWindow->>GhosttyNSView: Send occlusion or key-window notification
GhosttyNSView->>TerminalRendererWindowVisibility: Evaluate window state
TerminalRendererWindowVisibility-->>GhosttyNSView: Return visibility
GhosttyNSView->>Renderer: Update renderer window visibility
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (22 passed)
Full details: Description checkExplanation The description explains what changed, why the change is required, the root cause, the visibility rule, affected paths, and test coverage. It does not include the template's Demo Video, Review Trigger, or Checklist sections, but the core description and testing information are complete. Full details: Cmux Swift Actor IsolationExplanation PASS. The production changes do not introduce a stated actor-isolation failure. The new pure visibility helper is in the Swift 6 Full details: Cmux Swift Blocking RuntimeExplanation PASS: The only newly added timing primitive is Full details: Cmux Browser Automation Off-MainExplanation PASS — The PR diff from Full details: Cmux Expensive Synchronous LoadExplanation PASS. The pull-request diff adds no Full details: Cmux Cache Substitution CorrectnessExplanation PASS. The PR does not replace a fresh authoritative read with a cache in a persistence, history, undo, or snapshot path. The visibility state is a weak, in-memory UI presentation hint, which the policy allows. The Full details: Cmux No Hacky SleepsExplanation PASS. The PR changes six Swift files only; it changes no TypeScript, JavaScript, shell, or non-Swift build/runtime script. The only added sleep/polling is in the Swift test Full details: Cmux Algorithmic ComplexityExplanation PASS. The production diff adds only constant-time visibility branches, membership checks on an Full details: Cmux Swift ConcurrencyExplanation PASS — The PR adds no new background Dispatch queues, DispatchGroup, Combine state, completion-handler API, or fire-and-forget Full details: Cmux Swift `@Concurrent`Explanation PASS. The diff adds only a synchronous pure visibility helper, test polling with Full details: Cmux Swift Package BoundariesExplanation PASS. The new visibility rule is in the SwiftPM Full details: Cmux Swiftpm LockfilesExplanation PASS. The PR diff from 39a1488 to 7e47d56 changes only Swift source and test files. It does not change Package.swift, Package.resolved, .gitignore, workflows, or Xcode project/workspace files. Packages/macOS/CmuxTerminal/Package.swift is unchanged, and that package has no local Package.resolved or .gitignore. Therefore, no dependency-resolution or package-reference change introduced by this PR requires a lockfile diff. Full details: Cmux Swift LoggingExplanation PASS — The PR diff from 39a1488 to HEAD adds no Full details: Cmux User-Facing Error PrivacyExplanation PASS — The PR adds no user-facing error, alert, command-output, API-error, or recovery text. The new Full details: Cmux Full InternationalizationExplanation PASS: The PR introduces no new or changed user-facing copy. The production diff adds only renderer-visibility logic, observer wiring, actor-isolation fixes, and awaited catalog access. Added prose is developer documentation/comments, and the added tests and keepalive comments are exempt. The changed paths contain no Full details: Cmux Swiftui State LayoutExplanation PASS. The diff introduces no SwiftUI state or layout pattern covered by the rule. The visibility changes are inside Full details: Cmux Architecture RethinkExplanation The diff adds a process-wide mutable side channel: Resolution Remove the static window table from Full details: Cmux Swift Auxiliary Window Close ShortcutsExplanation PASS: The PR does not add or materially change a standalone cmux-owned window. Its only production NSWindow change is visibility observation in Full details: Cmux Source ArtifactsExplanation PASS. The base-to-HEAD diff contains only six tracked Swift source and test paths. The two added files are under the expected CmuxTerminal Sources and Tests directories. The four modified files are existing hand-written source or test files. No local logs, screenshots, recordings, temp or scratch directories, dependency checkouts, caches, build output, or package-manager downloads enter the diff. Full details: Cmux No Test Or Debug Seam In Production SourceExplanation The PR adds no test or debug seam in production Swift source. The new Full details: Cmux No Ambient Global StateExplanation The PR adds a caseless public enum used only as a static-helper namespace. In Resolution Move the visibility behavior onto a constructable, injectable owning type. For example, make
✨ 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 |
…duced by #10889; the determinism gate blocks main) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
…atics/captured self, surfaces socket shared access) Same fixes as #10905, carried here so the warning-budget gate lets the display step run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
525352e ios: match launch screen logo to the App Store icon glyph (manaflow-ai#10913) a1f0cf9 Present terminal renderers in on-screen windows that never report an occlusion .visible bit (fixes the display-liveness CI regression from manaflow-ai#10815) (manaflow-ai#10922) 86061c8 iOS: show the unread count on workspace indicators, in parity with macOS (manaflow-ai#10791) 5cacd70 fix: clear three Swift warning buckets over the CI budget on main (manaflow-ai#10905)
Regression. Since #10815 (581d900) the
tests-build-and-lagstep Run display UI regressions fails deterministically onmain:DisplayResolutionRegressionUITests.testRapidDisplayResolutionChangesKeepTerminalResponsive — XCTAssertGreaterThanOrEqual failed: ("0") is less than ("6")on both attempts (runs 33028942968 rerun, 33034886551 on a pre-#10887 baseline that includes #10815; the step last passed on 2026-08-19, run 32212616081, before #10815). Harness diagnostics showrenderWindowVisible=0/renderAppIsActive=0throughout.Root cause. #10815 presents the renderer only while
NSWindow.occlusionState.contains(.visible). The harness runs the app on aCGVirtualDisplay(scripts/create-virtual-display.m); AppKit never raises.visiblethere for a window that is ordered in and drawing, so the renderer stays released and presents never advance.Rule (
TerminalRendererWindowVisibility, one helper used by the occlusion observer, the initial attach, key/main transitions and screen changes): visible if the occlusion bit is set or the window is key; otherwise, until the window has reported.visibleat least once, trust its on-screen state (isVisible && !isMiniaturized && isOnActiveSpace); once the bit has been seen, honor the occlusion verdict — so miniaturized, covered and inactive-Space windows still release GPU exactly as #10815 intended.Tests:
TerminalRendererWindowVisibilityTests(4 cases). No change to typing hot paths.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes the display-liveness CI regression from #10815: terminal renderers now present in on-screen windows that never report an occlusion
.visiblebit, and the IRX keepalive test waits for the first pong instead of sleeping a fixed span. Carries the warning-budget fixes from #10905 so the warning gate lets the display step run.TerminalRendererWindowVisibility: trusts a window's ordinary on-screen state (visible, not miniaturized, on the active Space) until.visiblehas been observed at least once, then honors the occlusion verdict; key windows always present.nonisolatedIRX defaults keys, hoisted weak captures beforeMainActor.runhops, and awaitedSurfaceCatalog.sharedaccess.Written for commit 7e47d56. Summary will update on new commits.
Summary by CodeRabbit