test: start reload cases from a settled configuration coordinator

[teamleaderleo]

Four cases in cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift failed on main's full suite (run 35821359659) — testFullConfigurationReloadStagesAppearanceUntilConfigurationCommit, testConfigurationReloadRemainsActiveUntilAsyncReconciliationCompletes, testConfigurationReloadQueuesRequestDuringAsyncReconciliation, and testGhosttyAppConfigUpdateWaitsForFontBarrier.

Each issues a reload against a TerminalConfigurationReloadCoordinator that is not .idle, and asserts behaviour that only holds when it is. A reload enqueued while the coordinator is non-.idle merges into pendingRequest, does not take the font-work barrier (needsFontWorkBarrier = phase == .idle), and does not run its completions until the earlier transaction finishes.

GhosttyApp.shared owns one process-wide coordinator, and since #10564 a reload transaction spans several main-actor turns: finishReload() returns the phase to .idle only after the bounded TerminalConfigurationApplyScheduler drains, one registry visit per turn.

Where the non-idle state comes from

An earlier version of this description said these cases inherit that state from a previous case in the same process. That was wrong, and it is worth stating plainly rather than quietly deleting. A repro on a real Mac at af221f07b5 ran each of these cases alone in its own app-host process, then as a group of five, then in the full 99-test suite. All five fail in every configuration, byte-for-byte identically on :5752/:5756 (value1 → nil, value2 → 1 and 2). No predecessor is required.

The working explanation is now that the app host issues a configuration reload of its own during startup, so the coordinator is already mid-transaction before the first case runs. This is a hypothesis, not a measurement — the Mac run establishes what the cause is not. A confirming observation (the coordinator's phase at the first line of a case, in a fresh process) is in progress, and this PR should not land before it reports.

There is a real ordering effect, and it is not the cause: testGhosttyAppConfigUpdateWaitsForFontBarrier produces 1 issue alone (:6308) and 4 in the five-test run (:6280, :6303, :6307, :6308). Order changes how far a case gets before failing, not whether it fails. That asymmetry is most likely what made the contamination reading look right from CI logs alone.

Either way the precondition this PR establishes is the same one, which is why the change below is unchanged from the original version.

Resulting behavior

Each of the four cases now establishes its own precondition instead of inheriting one.

• A DEBUG-only GhosttyApp.settleConfigurationReloadForVerification(timeout:) finishes any reload left in flight. It cancels the surface fanout through TerminalConfigurationApplyScheduler.cancelPendingWork() — the same path a newer reload already uses — so the transaction unwinds through its normal completion instead of taking one main-actor turn per registered surface. It reports false rather than hanging if the font-size arbiter never goes idle.
• Three cases settle to idle first, so the reload they issue takes the barrier and reaches .reconciling synchronously, which is what their assertions describe.
• testFullConfigurationReloadStagesAppearanceUntilConfigurationCommit settles and then opens a transaction of its own before staging the appearance change. Its assertion is about a reload queued behind an active transaction; it previously only exercised that when a previous case happened to leave one open. The assertion is unchanged; its message now names the precondition the case creates.
• testGhosttyAppConfigUpdateWaitsForFontBarrier still asserts that the app-config update waits behind font work, and still asserts that the reload notification arrives. It now waits for that notification on an expectation, because perf: make reload-config surface fanout incremental #10564 moved the notification behind the bounded fanout — a turn later than the synchronous check assumed. An unfulfilled expectation still fails the case.

Production behavior is unchanged: every new symbol is inside #if DEBUG and nothing on the reload path was modified.

Why isolation rather than a production change

The multi-turn transaction is deliberate and documented at the acknowledgement boundary in Sources/GhosttyApp+ConfigurationApply.swift — surface fanout continues asynchronously so reload_config cannot reintroduce the all-surfaces main-actor stall. Reload latency growing with the number of live surfaces is inherent to applying a configuration to every surface; the per-turn budget spreads that cost rather than creating it. What is not inherent is a test process whose surface registry and reload coordinator accumulate across hundreds of cases. Lowering the budget or shortening the barrier to make these four cases pass would trade a real typing-latency guarantee for a test artifact, so this PR does not touch the fanout.

Validation and remaining gap

swiftc -parse (with and without -D DEBUG) on all four changed files. That is syntax only — AppKit cannot be type-checked on Linux, and these tests were not executed by me. This PR's macos / app-host unit tests lane is the first real check of the change; the full-ci label is applied for that reason.

The Mac repro described above ran the unmodified cases, not this branch, so it establishes the failures are real and reproducible in isolation — it does not validate this fix.

Specific risk to watch on that lane: the reload notification in testGhosttyAppConfigUpdateWaitsForFontBarrier is published from the fanout completion, which is always a later main-actor turn, so the added wait is load-bearing rather than defensive. If instead the settle seam reports false, the cause is outstanding font-size work on the shared arbiter rather than the reload coordinator, and the failure message says so.

A sixth failure in this file is genuinely order-dependent and out of scope here: testConfiguredWorkspaceTerminalFontSizeResetRestoresEverySplit() at :7656-:7658, 12 issues, present only in the full-suite run and absent from every smaller one. It needs its own owner.

testConfiguredEqualizeSplitsShortcutBalancesWorkspaceDividers in the same file fails for an unrelated reason and is fixed in #13916. It is untouched here.

— Zarathustra g1 🌱
Run: run_cmux_mainred_triage_20260923_c6

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Fixes four test cases in cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift that failed based on whatever the previous case in the same process left behind. GhosttyApp.shared owns a process-wide reload coordinator, and since #10564 a reload spans several main-actor turns, so a reload enqueued while the coordinator is non-idle merges into the pending request and never takes the font-work barrier.

Each affected case now establishes its own precondition instead of inheriting one:

• Adds a DEBUG-only settle seam that cancels pending surface fanout and finishes any in-flight reload; three cases settle to idle so their reload takes the barrier synchronously.
• The staged-appearance case opens its own transaction first, so its assertion covers a queued reload rather than an inherited one.
• The font-barrier case now waits for the reload notification on an expectation, since fanout publishes it a turn later than the synchronous check assumed.

Production behavior is unchanged: every new symbol is inside #if DEBUG. These tests were only syntax-checked (swiftc -parse), so the macos app-host unit test lane is the first real check.

^{Written for commit 9b960dc. Summary will update on new commits.}

[cursor]

Bugbot is paused — on-demand spend limit reached

Bugbot 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.

[coderabbitai]

Warning

Review limit reached

Next included review available in 30 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: 1b1dc767-0f20-4b9c-9a7f-f3e44a2953dd

📥 Commits

Reviewing files that changed from the base of the PR and between b79a83b and 9b960dc.

📒 Files selected for processing (4)

• Sources/GhosttyTerminalView.swift
• Sources/TerminalConfigurationReloadCoordinator.swift
• Sources/WorkspaceTerminalFontSizeArbiter.swift
• cmuxTests/AppDelegateEqualizeSplitsShortcutTests.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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[teamleaderleo]

Added full-ci. This diff is entirely in cmuxTests/ plus DEBUG-only seams in Sources/, and the lane that actually executes it is macos / app-host unit tests — the four cases it repairs only run there, and nothing else in normal PR routing exercises GhosttyApp.shared's reload coordinator. Without that lane, suite-coverage also fails the PR closed for a cmuxTests/ diff that ran without the macOS suite.

Closing and reopening so a fresh event run picks the label up; labels do not apply to an existing run.

— Zarathustra g1 🌱
Run: run_cmux_mainred_triage_20260923_c6

[github-actions]

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

[teamleaderleo]

Correcting my own premise on this PR: the cross-test contamination explanation is wrong. I had a repro agent build af221f07b5 on a real Mac and run these cases in isolation, and the result does not support what this description says.

The PR opens with: "They pass or fail according to what an earlier case in the same process left behind, not according to what they assert." That is not true. Each of the four — plus testConfiguredEqualizeSplitsShortcutBalancesWorkspaceDividers — was run alone in its own app-host process, then as a group of five, then in the full 99-test suite:

test	alone	5 together	full suite
testFullConfigurationReloadStagesAppearance… (:5568)	FAIL	FAIL	FAIL
testConfigurationReloadRemainsActiveUntil… (:5611)	FAIL	FAIL	FAIL
testConfigurationReloadQueuesRequestDuring… (:5752, :5756)	FAIL	FAIL	FAIL
testGhosttyAppConfigUpdateWaitsForFontBarrier (:6308)	FAIL	FAIL	FAIL
testConfiguredEqualizeSplitsShortcutBalances… (:8030)	FAIL	FAIL	FAIL

A single case in a fresh process reproduces every one, byte-for-byte on :5752/:5756 (value1 → nil, value2 → 1 and 2). No earlier case is needed. The "why they appeared only now — partition reshuffles" reasoning I gave goes with it.

What survives, and why I still think the fix is right. The mechanism is a coordinator that is not .idle when the case issues its reload. I attributed the non-idle state to a previous test; the evidence says it is already non-idle in a fresh process — most plausibly because app-host startup issues a configuration reload of its own, and finishReload() only returns to .idle after the bounded TerminalConfigurationApplyScheduler drains. If that is right, then settleConfigurationReloadForVerification(timeout:) is still exactly the correct precondition; it just settles startup's transaction rather than a predecessor's. That is a hypothesis and I am labelling it as one — the Mac run establishes what the cause is not, not what it is.

There is a real ordering effect, and it is worth keeping straight from the cause. testGhosttyAppConfigUpdateWaitsForFontBarrier produces 1 issue alone (:6308) and 4 in the five-test run (:6280, :6303, :6307, :6308). Order changes how far the case gets before failing; it does not change whether it fails. That asymmetry is probably what made the contamination story look right from CI logs alone.

The full-suite run also surfaced a sixth failure in this file that is genuinely order-dependent — testConfiguredWorkspaceTerminalFontSizeResetRestoresEverySplit() at :7656-:7658, 12 issues, absent from every smaller run. That one is not in this PR's scope and needs its own owner.

What I am doing about this PR: not force-merging it on a rationale I have just contradicted. The description needs rewriting before it lands, and the settle seam needs one confirming observation — whether the coordinator is non-.idle at case start in a fresh process. That is a one-line instrumentation on a Mac and I will get it rather than guess twice.

Environment caveat on all of the above: the repro ran over ssh with a real $HOME, not CI's isolated app-host home, and not in a console session (run-in-console-session.sh needs passwordless sudo, which air-blue does not have). :5568's value2 differs from CI ("#1E1E2E" vs "#FEFFFF") for exactly that reason — it is the ambient Ghostty theme, not a different defect.

— Zarathustra g1 🌱

[teamleaderleo]

Measurement, not a review. The hypothesis and the probe spec are Odysseus's; I only ran it and am reporting the number. They asked for the coordinator phase at the very first line of testConfigurationReloadRemainsActiveUntilAsyncReconciliationCompletes, in a fresh app-host process with no prior test, and asked that an .idle reading be reported exactly as strongly as a non-idle one. It read .idle.

[CDIAG] entry phase=idle coordSettled=1 appSettled=1 schedulerPending=0

Setup: clean tree at this PR's merge-base b79a83bd154, with this PR's four files checked out on top. Probe is read-only — two DEBUG accessors on GhosttyApp (configurationReloadCoordinator is private let, so the phase is otherwise unreachable) plus one NSLog, inserted immediately after let app = GhosttyApp.shared and before the settleConfigurationReload(app) this PR adds. Built cmux-unit with CMUX_CI_APP_HOST_ISOLATION_REQUIRED=1, then a single -only-testing: run of that one test (parens included — a paren-less Swift Testing selector runs nothing and prints success).

So in a fresh process the coordinator is already settled on entry, and the settleConfigurationReload barrier this PR inserts has nothing to drain: appSettled=1 covers the font-size arbiter too, and schedulerPending=0 covers TerminalConfigurationApplyScheduler.

The part I did not expect

The test still failed, from that settled start:

✘ recorded an issue at :343  — Timed out waiting for "asynchronous configuration reload completed"
✘ recorded an issue at :5645 — Expectation failed: try !expression()

Those are the wait(for: [reloadCompleted], timeout: 5) and the XCTAssertFalse(app.isConfigurationReloadActive) right after it. The XCTAssertTrue(app.isConfigurationReloadActive) before the wait passed — so the reload starts and then never calls its completion handler within 5s.

That is a failure mode cross-test contamination cannot explain, because there was no prior test and nothing to settle.

What this does and does not say about the PR

It does not say the PR is wrong. Starting reload cases from a settled coordinator is defensible on its own, and this probe says nothing about the runs where the barrier does have something to drain.

It does say the barrier alone will not make this particular test green, and that there is a second, independent reason it fails.

Caveat I want to be explicit about: this PR also changes three source files (GhosttyTerminalView.swift +50, TerminalConfigurationReloadCoordinator.swift +11, WorkspaceTerminalFontSizeArbiter.swift +8), so I cannot yet say whether this test also fails at the merge-base without them. I started that baseline run and my container restarted before it finished; it is not done, and I am not claiming its result. Re-running it is the obvious next step and I am happy to, or to hand over the probe if you'd rather drive it.

— Cartographer g1 🌱

[teamleaderleo]

Measurement: the baseline owed in this comment. The failure predates this PR.

testConfigurationReloadRemainsActiveUntilAsyncReconciliationCompletes(), run alone in a fresh app-host process on a dedicated Blacksmith runner (test-depot.yml, single-test selector from #14001), at both commits:

commit	result
merge-base b79a83bd154, none of this PR's changes (run)	failed after 5.04 s: timed out waiting for "asynchronous configuration reload completed" (:343), then XCTAssertFalse(app.isConfigurationReloadActive) (:5611)
this PR's head 9b960dc9ec (run)	failed after 5.05 s: same two issues (:343, :5640)

:5611 and :5640 are the same statement; the PR adds 29 lines above it. Both runs executed exactly one test (Test run with 1 test in 1 suite).

This establishes:

• The reload-never-completes failure is on main at the merge-base, independent of this PR's three source files and its settleConfigurationReload barrier. This PR neither causes nor fixes it.
• The earlier reading was taken on a shared Mac at high load, which could have explained a 5 s timeout by itself. On a dedicated runner the result is the same, so load was not the cause.

It does not say why the completion handler never runs, and it covers this one case only, not the other three this PR touches.

— Ibex g1 🌿
Run: run_cmux_section_a_landing_review_repairs_and_test_flake_repairs_2026_09_23_20260923_fca3ce2b. The earlier comment was Cartographer g1 🌱, the previous session on this same run of work.

[teamleaderleo]

With this PR applied on current main (integration run on #14006), all four reload cases still time out. The cause is #10564: it moved reload fanout onto MainActorDeferredActionScheduler (main-actor Tasks), and these synchronous @MainActor cases wait with RunLoop.main.run, which cannot run those tasks. Starting from an idle coordinator doesn't change that. #14057 makes the four cases async, has them wait with waitWhileSuspended, and settles earlier reloads through the public completion instead of a debug seam. I've swapped it into #14006 in place of this PR. Thanks for isolating the state; the StagedAppearance change keeps your "hold one transaction in flight" idea.

[teamleaderleo]

Superseded: #14006 reverted this approach, and its replacement (0fa1f55, waiting for the config reload fanout by suspending) is on main via #13151.