Stop a blocking wait helper from shadowing the pumping ones

[ejc3]

Three tests fail on main because adding a timeout: argument to a wait silently changes which helper
runs. Two are in the sidebar git probe suites and one is in TabManagerWorkspaceOwnershipTests, and
the product is correct in all three cases.

Why the wait cannot succeed

TabManagerUnitTests.swift and WorkspacePullRequestSidebarTests.swift each define a file-private
waitForCondition that polls by hopping through DispatchQueue.main under XCTWaiter, so the main
queue keeps draining while a test waits. The test target also has a module-scope
waitForCondition(timeout:pollInterval:_:) in CLICodexHookTimeoutRegressionTestSupport.swift that
polls with Thread.sleep and runs no run loop at all.

Swift prefers the overload that fills in fewer defaulted parameters. waitForCondition(timeout: 12.0) { … } leaves only pollInterval defaulted on the module-scope one, but would leave pollInterval,
file and line defaulted on the file-private one, so it binds to the blocking version. A call with
no timeout: argument cannot bind to the module-scope one at all and gets the pumping version. So the
waits that pass a timeout block the main thread for their entire budget, and every neighbouring wait in
the same file does not.

That is fatal for these tests, because they wait on main-actor work. The two probe tests fail on their
very first assertion, before reaching what they mean to check. An initial sidebar git probe is
registered synchronously at schedule time — the state map is written when the ladder task is installed,
and the accessor unions both maps, so a probe reports as active before it has run a single attempt.
Clearing it needs the ladder task's first hop, the detached snapshot, and its await MainActor.run
apply. Twelve seconds of Thread.sleep on the main thread denies all three, so the set cannot drain
and the condition can never become true. The logs show a workspace.gitProbe.schedule … reason=initial
line, then 16.5 seconds of silence with no apply, and the crash reporter logging an ANR partway
through.

None of the three failures carries the "Timed out waiting for condition" message that both
file-private helpers emit on timeout, which is independent confirmation that the blocking overload ran
in each case.

The product was right in all three cases

For the remote-split test, the panel id in the failure is the workspace's original local panel, not the
split. The split panel has no gitProbe.schedule line at all, so the skip guard did fire, and the two
assertions that check the panel is remote both passed. ProbeSchedulingTests.swift in CmuxSidebarGit
pins the same behavior deterministically with a fake host and a manual clock, and it passes.

For the defaults test, a non-repository directory does not leak a probe: the snapshot resolves to "not
found", the refresh stops on the first attempt, and both map entries are removed.
testTrackedWorkspaceGitMetadataPollCandidatesExcludeDirectoriesWithoutResolvedGitMetadata waits on
the same accessor for a plain temp directory and passes in 0.077 seconds, using the pumping helper.

The title test differs from the other two in one way worth noting: its earlier assertions pass and it
fails at the behavior it is named after, rather than before reaching it. Delivery there is genuinely
asynchronous: the .ghosttyDidSetTitle observer is registered
with queue: .main, so it runs as a queued main-queue operation rather than inline with the post, and
the apply is deferred again by the panel title coalescer's default 1/30s delay. A second of
Thread.sleep on the main thread starves both hops.

The change

Rename the blocking helper to waitForConditionBlocking and document what it is for. The three calls
then resolve to their file-private helpers again with the same budgets and poll intervals — no
assertion, timeout, or interval is touched. Its three existing callers wait on a socket accumulator
filled off the main thread, which is what the blocking form is appropriate for, so they keep it.

Renaming rather than shadow-proofing the call sites is what makes this stay fixed: a future
waitForCondition(timeout:) written in a file without a pumping helper now fails to compile instead of
quietly blocking the main thread.

Test plan

Measured on 4253cc2884, one GUI test host at a time, same checkout and DerivedData for both arms:

xcodebuild test -scheme cmux-unit -configuration Debug -destination 'platform=macOS' \
  -only-testing:cmuxTests/WorkspacePullRequestSidebarTests \
  -only-testing:cmuxTests/TabManagerPullRequestProbeTests \
  -only-testing:cmuxTests/TabManagerWorkspaceOwnershipTests \
  -only-testing:cmuxTests/CLICodexHookTimeoutRegressionTests

arm	XCTest failures across a 36-test run	distinct tests red
main unchanged	15 of 36	9
this branch	10 of 36	6

The three tests that flip are testRemoteSplitSkipsInitialGitMetadataProbe,
testUnrelatedDefaultsChangeDoesNotRestartGitMetadataRefreshes and
testFocusedPanelTitleRefreshesAutoWorkspaceTitleInSplitWorkspace. CLICodexHookTimeoutRegressionTests,
which owns the renamed helper, reports Test run with 8 tests in 1 suite passed in both arms, so its
three callers are unaffected. No host restarts in either arm.

Pull-request CI on this repo runs review bots and security scanners, not the test suite, so the arms
above are the only test evidence this carries.

The six tests still red on this branch fail for reasons unrelated to waits: two git-index fixtures that
describe a state git cannot produce, a scoped socket report sent to a manager AppDelegate cannot
resolve, an async test using the pumping helper from a main-actor job, a stub that watches a subprocess
the product no longer spawns, and a product bug in TabManager.closeWorkspace.

Summary by CodeRabbit

• Tests
  • Improved automated test synchronization by using a blocking condition helper with clearer timeout/polling behavior.
  • Updated regression test steps to wait for expected asynchronous command snapshots at each stage.
  • Expanded inline documentation for the blocking wait helper to improve readability and reduce flaky timing issues.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d10a1c1e-f791-4e1f-a0db-6e54d9a231a2

📥 Commits

Reviewing files that changed from the base of the PR and between 29e33cd and c5bd939.

📒 Files selected for processing (2)

• cmuxTests/CLICodexHookTimeoutRegressionTestSupport.swift
• cmuxTests/CLICodexHookTimeoutRegressionTests.swift

---

📝 Walkthrough

Walkthrough

The blocking condition polling helper is renamed to clarify its behavior, with expanded documentation. Three synchronization points in the Codex timeout regression test now call the renamed helper.

Changes

Codex timeout regression test synchronization

Layer / File(s)	Summary
Rename and update blocking wait usage cmuxTests/CLICodexHookTimeoutRegressionTestSupport.swift, cmuxTests/CLICodexHookTimeoutRegressionTests.swift	The polling helper is renamed and documented, and the regression test updates three condition waits to use it.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 25

✅ Passed checks (25 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly reflects the main change: renaming the blocking helper to avoid shadowing the pumping wait helpers.
Description check	✅ Passed	The description covers the summary, rationale, and testing in detail; only non-critical template sections like demo video, review trigger, and checklist are missing.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation	✅ Passed	Diff only renames a test helper and updates test calls; no production Swift isolation changes or new MainActor/Sendable issues were introduced.
Cmux Swift Blocking Runtime	✅ Passed	Only cmuxTests changed; the renamed helper remains test-only scaffolding, and no production Swift blocking/timing sync was introduced or expanded.
Cmux Browser Automation Off-Main	✅ Passed	Diff only renames a test wait helper and updates test calls; no browser.* routing or main-thread automation code changed.
Cmux Expensive Synchronous Load	✅ Passed	Diff only renames a test polling helper and updates three test call sites; no agent-history load or main-actor synchronous parsing was added or moved.
Cmux Cache Substitution Correctness	✅ Passed	Diff only renames a blocking wait helper in tests; it does not replace any authoritative read with cached/opportunistic data in a persistence, history, undo, or snapshot path.
Cmux No Hacky Sleeps	✅ Passed	PASS: The PR only renames a Swift test helper and updates call sites; no new fixed sleeps or delays were introduced, and the rule is out of scope for Swift/test-only code.
Cmux Algorithmic Complexity	✅ Passed	Only test-support/test files changed; the helper rename doesn’t add or worsen scalable production-path scans.
Cmux Swift Concurrency	✅ Passed	Diff only renames a test helper and updates three test call sites; no new DispatchQueue/Combine/Task pattern is introduced in cmux-owned production code.
Cmux Swift @Concurrent	✅ Passed	No async/@Concurrent changes were introduced; the PR only renames a synchronous blocking helper and updates sync test call sites.
Cmux Swift Package Boundaries	✅ Passed	Diff only touches cmuxTests files; the changed helper and calls are test fixtures, which the boundary rules explicitly allow.
Cmux Swiftpm Lockfiles	✅ Passed	Diff only touches two Swift test files; no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project lockfile changes are present.
Cmux Swift Logging	✅ Passed	Diff only renames a test helper and updates calls; no print/debugPrint/dump/NSLog/Logger changes or sensitive logging were added.
Cmux User-Facing Error Privacy	✅ Passed	Commit only renames a test helper and updates test call sites/docs; no production user-facing errors or sensitive upstream details are added.
Cmux Full Internationalization	✅ Passed	Only test/support code changed; no production user-facing text, locale assets, or web/i18n content was added or modified.
Cmux Swiftui State Layout	✅ Passed	Diff only renames a test helper and updates test calls; no SwiftUI state/layout patterns are introduced.
Cmux Architecture Rethink	✅ Passed	PASS: The diff is test-only: it renames a blocking wait helper in cmuxTests and updates three socket-accumulator waits; no product lifecycle or ownership path changes.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	Diff only renames a test helper and updates test calls; no user-visible window, controller, or cmuxAuxiliaryWindowIdentifiers changes.
Cmux Source Artifacts	✅ Passed	Both changed paths are intentional Swift source/test files; no logs, temp dirs, DerivedData, caches, or other artifact paths appear in the diff.
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PR diff against origin/main only changes cmuxTests files; no production Sources/ file adds a debug/test seam.
Cmux No Ambient Global State	✅ Passed	PASS: This is test-only support code; the patch only renames an existing file-scope helper and adds docs, with no new global var, singleton, or static namespace.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[greptile-apps]

Greptile Summary

This PR fixes three flaky tests caused by Swift overload resolution silently binding waitForCondition(timeout:) to the module-scope blocking helper instead of the per-file XCTWaiter-pumping helpers. The fix renames the blocking helper to waitForConditionBlocking and adds a doc comment explaining the restriction, so any future call in a file without a pumping helper fails to compile rather than silently starving the main thread.

• CLICodexHookTimeoutRegressionTestSupport.swift: waitForCondition renamed to waitForConditionBlocking with a doc comment that documents the Thread.sleep behavior and explicitly prohibits main-thread use.
• CLICodexHookTimeoutRegressionTests.swift: three call sites updated to use the new name; all three wait on commands.snapshot(), a socket accumulator filled off the main thread, which is the correct use case for the blocking form.

Confidence Score: 5/5

Safe to merge — all changes are confined to test files with no production code touched.

The change is a targeted rename in two test-support files. The three updated call sites all wait on a socket accumulator filled by background threads, which is the explicitly documented correct use of the blocking form. The rename itself becomes a compile-time guard against future misuse: any new waitForCondition(timeout:) in a file without a pumping helper will fail to compile rather than silently stalling the main thread. No production logic is modified.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
cmuxTests/CLICodexHookTimeoutRegressionTestSupport.swift	Blocking wait helper renamed from waitForCondition to waitForConditionBlocking; doc comment added to explain Thread.sleep behavior and prohibit main-thread use. No logic changes.
cmuxTests/CLICodexHookTimeoutRegressionTests.swift	Three call sites updated from waitForCondition to waitForConditionBlocking; all three correctly wait on a socket accumulator filled by background threads, so the blocking form is appropriate.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Before
        A[waitForCondition timeout 2.0] --> B{Swift overload resolution}
        B -->|fewer defaults needed| C[module-scope waitForCondition Thread.sleep blocks main]
        B -.->|more defaults needed| D[file-private waitForCondition XCTWaiter pumps main queue]
        C --> E[Main thread starved - condition never true]
    end
    subgraph After
        F[waitForConditionBlocking timeout 2.0] --> G[module-scope waitForConditionBlocking Thread.sleep explicit]
        H[waitForCondition timeout 2.0] --> I[file-private waitForCondition XCTWaiter pumps main queue]
        J[waitForCondition timeout 2.0 in file without pumping helper] --> K[Compile error - no matching overload]
        G --> L[Socket/file conditions filled off main thread]
        I --> M[Main-actor work can complete]
    end

Loading

_{Reviews (3): Last reviewed commit: "cmuxTests: stop a blocking wait helper f..." | Re-trigger Greptile}