Skip to content

test: pin key-window focus, rebind retired terminals, diagnose tmux divider parity - #13672

Merged
teamleaderleo merged 3 commits into
manaflow-ai:fix/app-host-greenfrom
teamleaderleo:fix/app-host-focus-isolation
Sep 22, 2026
Merged

teamleaderleo merged 3 commits into
manaflow-ai:fix/app-host-greenfrom
teamleaderleo:fix/app-host-focus-isolation

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Test fixes for the focus, geometry and order-dependent app-host failures on #13643 (items A–E). Reading from the #13643 run at f7132b3 (shards 3, 4, 5).

A. Focus trio: host dependency, order-dependent. The app-host process runs headless and usually isn't the active app, so makeKeyAndOrderFront never makes the test window key. Terminal focus paths check isKeyWindow: the automatic first-responder apply, focus redraws, the deferred focus reapply, and the window activation in ensureFocus. So these tests passed only when an earlier test in the shard had activated the app. Evidence: in shard 4, the three WorkspaceTerminalFocusRecoverySwiftTests cases that call makeFirstResponder directly passed. The two that rely on the automatic apply failed with identical setup. Fix: move the existing KeyStatusTestWindow (already used privately by two suites) into shared test support. Use it in WorkspaceTerminalFocusRecoveryTests, WorkspaceTerminalFocusRecoverySwiftTests and TerminalNotificationDirectInteractionTests. No assertion changes.

B. offPlanGeometryWithUnchangedSizingInputsReconverges: not fixed; diagnostic added. The CI message shows all 3 parity re-arms ran (rearms=3), so the recovery passes do reach imposeDividerPlan. A standalone bonsplit (faf8418) replay of the same impose → clear → 0.8 → re-impose sequence converges to 320. A sibling test without a bound, portal-visible workspace heals the same displacement. That points at the TerminalPortalTestWorkspace + bind(to:) fixture added on 09-16. It does not point at bonsplit or the mirror. The failure message now includes the live split view's arranged widths and the split model's imposed extent. The next run will show which of three layers is at fault: bonsplit refused the apply, the imposition was cleared, or the portal didn't follow.

C. workspaceRevealKeepsTerminalSizeUntilAnUnchangedGeometryPass: stale test, fails every run. The earlier "pass" in 35666501009 also failed at the same lines. Since #12607, hideEntry removes the hosted view from the window, and only bind reinstalls it. The test only flipped portal visibility, so the view stayed detached and no size commit could land. Fix: rebind the way TerminalPortalReconciliation does, assert the retire/reattach contract, and drop the 5 s wait loop.

D. visibilityToggleKeepsAppKitTableContainerMounted (10 vs 5): not fixed. The failure started when 0929dc2 put the test window on screen, in both this test and testMinimalModeToggleDoesNotReevaluateChromeHeavyBodies. After that the sidebar body evaluates twice on reveal, and each evaluation re-projects every row. This is probably a real extra render pass. The write that triggers it isn't identified yet. Suspects are the reveal-side window chrome updates in ContentView, and snapshot rebuilds that observe cloudBindingState from body. It needs a local _printChanges run.

E. Order dependence. The shared leak fixed here is app activation and key-window state (A). The 09-20 "passed 06:39–10:49, then failed" flip is not evidence of state leakage. Before #13178 (merged 09-20 ~11:10), a run whose test host restarted after a crash reported only the post-restart subset, which masked failures.

Checks: tests/test_ci_pbxproj_test_wiring.sh and scripts/check-test-determinism.py pass, and edited files pass swiftc -parse. No local app-host run.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes two order- and setup-dependent app-host test failures and adds a diagnostic for a third.

Bug Fixes

  • Terminal focus suites now share KeyStatusTestWindow, which pins key status; the headless app-host run is usually not the active app, so isKeyWindow-gated tests previously passed only by test order.
  • workspaceRevealKeepsTerminalSizeUntilAnUnchangedGeometryPass rebinds the retired hosted view the way TerminalPortalReconciliation does, since hiding only retires it from the window.
  • The 5 s wait loop in that test is dropped.

The offPlanGeometryWithUnchangedSizingInputsReconverges failure message now reports the live split view's arranged widths and the split model's imposed extent, so the next run shows whether bonsplit refused the apply, the imposition was cleared, or the portal did not follow. The render-count failure (10 vs 5) is left alone; it's suspected to be a real extra render pass.

Written for commit 41d8f87. Summary will update on new commits.

Review in cubic

teamleaderleo and others added 3 commits September 22, 2026 10:11
… does

Since manaflow-ai#12607 hiding a terminal removes its hosted view from the window, and
only a bind reinstalls it. The test flipped portal visibility on the detached
view, so no size commit could ever land and the final shrink check failed
every run. Rebind like TerminalPortalReconciliation and drop the wait loop.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The app-host test process runs headless and is usually not the active app,
so makeKeyAndOrderFront never makes a programmatic window key. Terminal focus
paths gate on isKeyWindow (automatic first-responder apply, focus redraws,
deferred focus reapply, ensureFocus window activation), so these suites
passed only when an earlier test in the shard had activated the app.

Share the existing KeyStatusTestWindow and use it in
WorkspaceTerminalFocusRecoveryTests, WorkspaceTerminalFocusRecoverySwiftTests
and TerminalNotificationDirectInteractionTests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
offPlanGeometryWithUnchangedSizingInputsReconverges fails every run with all
three output-parity re-arms spent and the hosted view still at the perturbed
0.8 divider. A standalone bonsplit replay of the same sequence converges, and
a sibling test without a bound (portal-visible) workspace heals the same
displacement. Add the live split view's arranged widths and the split model's
imposed extent to the failure message so the next run shows whether bonsplit
refused the apply, the imposition was cleared, or the portal did not follow.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: cfc1988d-2544-4694-9956-47e5d953081d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@teamleaderleo
teamleaderleo merged commit 41d8f87 into manaflow-ai:fix/app-host-green Sep 22, 2026
26 checks passed
@teamleaderleo
teamleaderleo deleted the fix/app-host-focus-isolation branch September 23, 2026 11:35
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