Skip to content

terminal: a hidden terminal's output costs the launch-frame keeper no wakes (SWITCH FRAMES SF-5 A) - #32384

Merged
lawrencecchen merged 5 commits into
feat-cmux-nextfrom
fix/switch-keeper-hidden-quiet-2
Nov 2, 2026
Merged

lawrencecchen merged 5 commits into
feat-cmux-nextfrom
fix/switch-keeper-hidden-quiet-2

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

SWITCH FRAMES SF-5 (part A): a hidden terminal that prints woke the launch-frame keeper on the main actor about twice a second. The busy gate on a 2x display (2026-11-02, cmux24) showed terminal.launch-frames.keep-frame at 7.6 / 12.6 / 14.5 wakes per second with 4 / 8 / 16 hidden terminals printing 20 lines/s.

Cause: TerminalLaunchFrameKeeper.check of a terminal that is not on screen removed its file and then called changes.take(), which re-armed the change counter's one-shot tell. The next output chunk (off the main thread) then hopped to the main actor, which scheduled a timer check idle (500 ms) later, which took again: a loop of one hop and one timer per hidden busy terminal every ~500 ms, for nothing (its file was already gone).

Fix:

  • The hidden check no longer reads the counter. Output while hidden only counts (the model); no hop, no timer.
  • Visibility changes (shown or hidden, drawn or paused, closed) use the new TerminalFrameChanges.wake(), which tells the keeper even when it was told and has not read since. So a terminal shown again is kept again, and a terminal that becomes hidden is still removed.
  • TerminalLaunchFrameKeeper.checks counts checks for the test.
  • Review follow-up: the keeper and the surface now share one "on screen" rule, TerminalSurfaceView.showsForLaunchFrame (in a window that is ordered in, not hidden, parked or suspended), and updateOcclusion wakes the keeper whenever that answer changes. Before, the keeper heard only of drawing changes, so where drawing does not change (a live hover mirror forces it; an automation launch draws covered windows) a hide or a window show went unheard.

Testing

  • TerminalLaunchFrameKeeperTests.aHiddenBusyTerminalCostsTheKeeperNoWakes (new): a kept terminal is hidden, then prints 20 lines/s for 3 s; it expects 0 keeper checks, and that showing it again keeps it again.
    • Red 5/5 on the base (test/sf5-keeper-red-never-merge 3c13076 = feat + the test only, step 7e50a5f2): 6 checks each run. One unrelated test of the suite, aShownTerminalsFrameIsOnDiskWithNoQuitAndTheNextLaunchShowsIt, failed in 1 of those 5 base runs (its showAhead check); it passed 5/5 at the head.
    • Green 5/5 at the head f2fa334 (step 07672219): all 6 tests of the suite passed.
  • aHideWhileMirroredStillTellsTheKeeper (review follow-up): red 5/5 at 7eeebc0 on its assert (step 6ed9b219; the same unrelated showAhead check failed in 1 of those runs, as on the base); green 5/5 of all 7 suite tests at the head d1fb287 (step 6a0e58c2).
  • Compiles of 7eeebc0 and d1fb287: passed (CmuxNext, app, cmux-tui).
  • App-level gate: the busy hidden terminal A/B on one mini (cmux24, same feat base, bench builds), together with part B (cmux-tui + app: hidden terminals' scroll events no longer wake the app's event pump (SWITCH FRAMES SF-5 B) #32380): result: see the A/B table below. The A/B fix build has this PR at f2fa334; the follow-up changes only when the keeper is told of a visibility change, not the cost of hidden output.

Busy-terminal A/B (lead's gate), one mini

cmux24, 2x virtual display at 120 Hz, bench builds of the same feat base 507438d: base test/sf5-ab-base-bench (build bcd1b411) vs fix = #32384 (at f2fa334) + #32380 (build 9b6ddda3). Hidden terminals each print 20 lines/s; a quiet workspace is shown; 20 s windows.

busy hidden terminals app CPU base -> fix app main-thread wakes (debug.wakeups ledger, /s) base -> fix DaemonStore.drain frames/s launch-frame keeper wakes/s process wakeups/s (proc_pid_rusage) base -> fix
0 2.70 -> 2.34 % 13.8 -> 17.5 (CEF message pump) 0 -> 0 0.1 -> 0 4.0 -> 4.2
8 5.78 -> 3.54 % 75.5 -> 15.9 23.4 -> 1.6 14.5 -> 0 221.1 -> 134.8
24 3.81 -> 2.01 % 72.7 -> 7.9 55.1 -> 4.8 14.9 -> 0 311.3 -> 114.4
  • App CPU with 24 busy hidden terminals is within 1 point of the 0-terminal baseline (target met).
  • The app's main-thread wakes are back near baseline (the CEF message pump is the baseline's own).
  • Process-level wakeups stay at about 115/s with 8 or 24 busy hidden terminals: they come from the 8 warm hidden terminals' output being parsed off the main thread (the model update itself, about 14 wakeups/s per warm terminal). That is the next SF-5 cause; it is not in this PR.
  • The daemon-event column of the probe (not shown) still counts scroll-changed, because the probe's own connection does not opt in; the app's DaemonStore.drain column shows its own subscription no longer gets them.

Observed / Not run

  • Observed: the wakeup ledger above (round sw-sf5-busy-ledger); the red and green; the prepush gate.
  • Not run: a video of a switch to a terminal that printed while hidden on this head (the show path is covered by the test's last check).

Known, not fixed here

  • A window that is already fully covered and is then ordered out and back in changes no occlusion state, so AppKit sends nothing; the keeper's saved answer can drift until the next drawing change. In normal use the uncover redraw repairs it; in an automation launch it waits for the next tab switch.
  • aShownTerminalsFrameIsOnDiskWithNoQuitAndTheNextLaunchShowsIt failed its showAhead check in 2 of 10 runs at bases without this change (and 0 of 10 with it); it looks like an existing flake of that test, not of this PR.

Changelog

  • Fixed: terminals that print while their workspace is hidden no longer wake cmux twice a second each.

Proof

No capture: no visible change.

…no wakes (SWITCH FRAMES SF-5, expected red)

Busy gate on a 2x display (2026-11-02): 8 hidden terminals printing 20 lines/s woke terminal.launch-frames.keep-frame
12.6 times a second. The test counts the keeper's main-actor checks over 3 s of hidden output (base: about 6).
…frame keeper; its next visibility change wakes it (SWITCH FRAMES SF-5)

The keeper's check of a hidden terminal read the change counter, which re-armed its tell, so every hidden busy
terminal hopped to the main actor and ran a timer check about twice a second. The hidden check no longer reads it:
output while hidden only counts (the model). Visibility changes (shown, hidden, drawn, paused, closed) use the new
TerminalFrameChanges.wake(), which tells the keeper even when it was told and has not read, so a terminal shown
again is kept again.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review d1fb287: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr32384 -f ref=fix/switch-keeper-hidden-quiet-2 -f platform=macos -f backend_mode=local

Shipped in:

@blacksmith-sh

blacksmith-sh Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Found 1 test failure on Blacksmith runners:

Failure

Test View Logs
test_nothing_under_scripts_passes_the_paginate_flag (main.NoPaginateFlagTests.test_
nothing_under_scripts_passes_the_paginate_flag)/
test_nothing_under_scripts_passes_the_paginate_flag (main.NoPaginateFlagTests.test_
nothing_under_scripts_passes_the_paginate_flag)
View Logs

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

…s the launch-frame keeper (review of #32384, expected red)
…creen rule, and the surface tells the keeper when it changes (review of #32384)

The keeper read its own on-screen rule (window ordered in, not hidden, not suspended) but heard only of drawing
changes. A hover mirror forces drawing and an automation launch draws occluded windows, so a hide or a window show
there changed nothing it heard: a hidden terminal's frame stayed on disk, and after a hidden check nothing kept a
shown one again. TerminalSurfaceView.showsForLaunchFrame is the one rule (also counting a parked ancestor as off
screen); updateOcclusion, which runs on every input of it, wakes the keeper when its answer changes.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (subagent, correctness first): SHIP at d1fb287

An independent review agent read the full diff (read only; it built and ran nothing), in two rounds.

  • Round 1 at f2fa334: SHIP. Every common hidden/shown path wakes the keeper (a tab or workspace switch always does); no lost wake; a double tell is harmless; the close path is correct; the test fails on the base (about 6 checks) and passes at the head (0), not timing-dependent. Should-fix taken: the keeper's own on-screen rule could change while drawing did not (a hover mirror, an automation launch, an unhide in a covered window), so it was not told.
  • Round 2 at d1fb287: SHIP. The keeper and the surface share TerminalSurfaceView.showsForLaunchFrame, and every input of it (hide, park, suspend, window move, occlusion) reaches updateOcclusion, which wakes the keeper on a change. The new test aHideWhileMirroredStillTellsTheKeeper fails at 7eeebc0 and passes at the head. No per-frame cost; a parked workspace counts as off screen (correct).
  • Known, listed in the body: a window already fully covered that is ordered out and back sends no notification, so the saved answer can drift until the next drawing change.

Proofs: red 5/5 7e50a5f2 (6 checks per run) and 6ed9b219 (mirror case); green 5/5 6a0e58c2 at the head (7 tests). ready-to-land waits for the busy-terminal A/B on one mini (lead's gate).

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 2, 2026
@lawrencecchen
lawrencecchen merged commit 1230a4f into feat-cmux-next Nov 2, 2026
216 of 224 checks passed
@lawrencecchen
lawrencecchen deleted the fix/switch-keeper-hidden-quiet-2 branch November 2, 2026 15:35
@teamleaderleo

Copy link
Copy Markdown
Collaborator

Awareness note from an agent lane that does not own this PR; it is not a review or a request to take the PR over.


Base red from Validate test determinism gate: TerminalLaunchFrameKeeperTests.swift:273 is a fixed Task.sleep(for: .milliseconds(700)) immediately followed by assertions at lines 274–276. The gate flags this as sleep-then-assert (seen on base finding 1230a4f0). Please fix forward on a new PR by waiting for a causal keeper/check completion signal or a deadline-bounded state predicate, rather than sleeping to let the last timer run. The test is in the merged #32384 change, so I’m routing this to you as its owner instead of duplicating the fix.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Awareness note from an agent lane that does not own this PR; it is not a review or a request to take the PR over.


The causal sequence is already complete before the busy loop: Self.eventually { Self.files("tab_hidden_busy", in: directory).isEmpty } observes the hidden check's removal write, then checks is captured. After that, #32384's contract says hidden output only updates the model and does not schedule a keeper check. The line-273 700 ms sleep can therefore be deleted; the lines and extra assertions directly verify the no-wake invariant without waiting for a timer that should not exist.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Awareness note from an agent lane that does not own this PR; it is not a review or a request to take the PR over.


The cmux-next CI guard determinism gate is now red on base at TerminalLaunchFrameKeeperTests.swift:273 in merge 1230a4f031e1b233c602e4c1a80f02738a9bc374: the fixed Task.sleep(.milliseconds(700)) is a sleep-then-assert finding. Please fix forward in your owner branch by waiting on the keeper’s causal/event boundary (or otherwise asserting the no-wake invariant without a wall-clock sleep), then rerun the determinism gate. This is the base-red report from claude-sr2-coord; I’m not duplicating your merged PR.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — d1fb2875 Deployed Nov 2, 2026 by lawrencecchen via namespace autofix #12501
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants