Un-deadlock CmxConnectivityPeerSessionTests (red swift-package-tests gate on main) by lawrencecchen · Pull Request #10741 · manaflow-ai/cmux

lawrencecchen · 2026-08-25T13:36:29Z

Un-deadlocks the swift-package-tests CI job, red on main since at least Aug 19 (run 32212616081).

CmxConnectivityPeerSessionTests.onePeerTraceUsesOneAliasAndOneEstablishedSessionEvent awaits peer.connectedSession(for:) built on a GatedConnectivitySessionBuilder, and no line ever releases the gate, so the dial parks forever. The per-suite CI runner then hits its 300 s timeout, retries once, times out again, and fails the job on every run of every PR. The fix releases the gate the way the neighboring concurrentCallersShareOneDialAndOneAdmittedSession test does: start the dial in a task, wait until the builder records the call, release, then await the dial. The suite goes from a deterministic 600 s double-timeout to completing in milliseconds (17 tests).

Test-only change; no runtime code is touched. Split out of #10739 so a revert of that feature PR cannot un-fix this CI gate.

Revert: git revert <sha> restores the deadlock and the red gate; nothing else depends on this commit.

Dictionary: per-suite runner = scripts/ci/run-swift-testing-suites.sh, which runs each test suite in its own process with a 300 s timeout and one retry; gate = a test helper continuation that parks a fake dial until the test resumes it.

Summary by cubic

Fixes a deadlock in CmxConnectivityPeerSessionTests.onePeerTraceUsesOneAliasAndOneEstablishedSessionEvent that was timing out the swift-package-tests job on main. Previously the test awaited a gated dial without releasing the gate; it now runs the dial in a task, waits for the builder to record the call, releases the gate, then awaits the result, eliminating the hang.

Test-only change; no runtime code is modified. Reverting this will reintroduce the red CI gate.

^{Written for commit 1f05608. Summary will update on new commits.}

Summary by CodeRabbit

Tests
- Improved connectivity test synchronization to reliably validate asynchronous connection establishment.
- Ensured connection progress is observed before allowing setup to complete.

…rSessionTests onePeerTraceUsesOneAliasAndOneEstablishedSessionEvent awaited a GatedConnectivitySessionBuilder dial that no line ever released, so the suite hung until the 300s CI timeout, twice, failing swift-package-tests on main since at least Aug 19 (run 32212616081). Release the gate the way the neighboring concurrent-callers test does; the suite now completes in milliseconds.

coderabbitai · 2026-08-25T13:37:34Z

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: 0130197a-716a-43e6-8999-6780e70a4f50

📥 Commits

Reviewing files that changed from the base of the PR and between bd985bd and 1f05608.

📒 Files selected for processing (1)

Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Walkthrough

The test now starts the gated connection asynchronously, waits for dialing to begin, releases the builder gate, and awaits connection completion.

Changes

Peer session test synchronization

Layer / File(s)	Summary
Coordinate the gated dial `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`	The test launches the gated dial in a task, waits for the builder to receive the dial, releases the gate, and awaits the task.

Estimated code review effort: 2 (Simple) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 1f056

This test-only change removes a deterministic test hang without altering production behavior, so no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: azooz2003-bit

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the deadlock fix in CmxConnectivityPeerSessionTests and the affected swift-package-tests CI gate.
Description check	✅ Passed	The description clearly explains what changed, why the deadlock occurred, how the fix works, the test impact, and that runtime code is unchanged. It omits the template headings, checklist, and review-…
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	PASS. The commit changes one file, `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`. The diff only updates a test and uses the existing private ac…
Cmux Swift Blocking Runtime	✅ Passed	PASS. The diff changes only `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`. It starts a test dial task, waits for the test builder's recorded ca…
Cmux Browser Automation Off-Main	✅ Passed	PASS: The pull request changes only `CmxConnectivityPeerSessionTests.swift`. The diff adds asynchronous gating for `CmxConnectivityPeerSession`; it contains no `browser.*` command, WebKit/AppKit acces…
Cmux Expensive Synchronous Load	✅ Passed	PASS: The commit changes only `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`. It only changes test control flow by starting a gated dial in a ta…
Cmux Cache Substitution Correctness	✅ Passed	PASS. The commit changes only `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`. It updates test synchronization around `GatedConnectivitySessionBu…
Cmux No Hacky Sleeps	✅ Passed	PASS. The diff changes only the Swift test file `CmxConnectivityPeerSessionTests.swift`; it does not modify TypeScript, JavaScript, shell, or build/runtime scripts. The added coordination uses the tes…
Cmux Algorithmic Complexity	✅ Passed	PASS. The diff changes only `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`. It updates test coordination around a gated dial and adds no product…
Cmux Swift Concurrency	✅ Passed	PASS: The commit changes only the Swift test file. It adds a local `Task` for the gated dial, waits for `builder.callCount()`, releases the gate, and awaits `dial.value`. This is not fire-and-forget w…
Cmux Swift `@Concurrent`	✅ Passed	PASS. The diff changes only test coordination in `onePeerTraceUsesOneAliasAndOneEstablishedSessionEvent`: it creates a `Task`, waits for the gated builder, releases it, and awaits the task. The change…
Cmux Swift Package Boundaries	✅ Passed	PASS: The pull request changes only `Packages/Shared/CmuxIrohTransport/Tests/.../CmxConnectivityPeerSessionTests.swift`. The diff contains test synchronization changes only. It introduces no productio…
Cmux Swiftpm Lockfiles	✅ Passed	PASS. The pull request changes only `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift` (+7/-1). It does not change `Package.swift`, `Package.resolve…
Cmux Swift Logging	✅ Passed	PASS: The commit changes only `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`, which is part of the `CmuxIrohTransportTests` test target. The add…
Cmux User-Facing Error Privacy	✅ Passed	PASS: The diff changes only a Swift test file. It adds asynchronous test coordination and a developer-only comment. It does not add or modify production user-facing errors, alerts, command output, API…
Cmux Full Internationalization	✅ Passed	PASS: The pull request changes only `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`. The diff updates test control flow and adds developer-only c…
Cmux Swiftui State Layout	✅ Passed	PASS — The pull request changes only a Swift Testing file, `CmxConnectivityPeerSessionTests.swift`. The diff adds asynchronous test coordination for `GatedConnectivitySessionBuilder`; it does not add …
Cmux Architecture Rethink	✅ Passed	PASS. The diff changes only a Swift test. It replaces an await on a deliberately gated dial with a test Task, a bounded test helper wait, gate release, and Task value await. The architectural rule exp…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PASS: The pull request changes only `CmxConnectivityPeerSessionTests.swift`. The diff adds asynchronous test gating and does not add or materially change `NSWindow`, `NSPanel`, `NSWindowController`, S…
Cmux Source Artifacts	✅ Passed	PASS. The pull request changes one existing Swift test at `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`. The diff adds test synchronization log…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PASS: The only changed file is `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`. It is under `Tests/`, and the diff adds only test synchronization…
Cmux No Ambient Global State	✅ Passed	PASS: The pull request changes only `Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift`. The diff changes test orchestration only and introduces no p…

Full details: Description check

Explanation

The description clearly explains what changed, why the deadlock occurred, how the fix works, the test impact, and that runtime code is unchanged. It omits the template headings, checklist, and review-trigger block, but the core required information is present.

Full details: Cmux Swift Actor Isolation

Explanation

PASS. The commit changes one file, Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift. The diff only updates a test and uses the existing private actor test helper. No production Swift files changed, so the actor-isolation failure conditions do not apply.

Full details: Cmux Swift Blocking Runtime

Explanation

PASS. The diff changes only Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift. It starts a test dial task, waits for the test builder's recorded call, releases the test continuation, and awaits completion. The repository rule allows deterministic test-only scaffolding. No production Swift file or production blocking/timing primitive changed.

Full details: Cmux Browser Automation Off-Main

Explanation

PASS: The pull request changes only CmxConnectivityPeerSessionTests.swift. The diff adds asynchronous gating for CmxConnectivityPeerSession; it contains no browser.* command, WebKit/AppKit access, socket-worker routing, processV2Command, or browser policy-test change. The browser automation off-main failure conditions are therefore not applicable.

Full details: Cmux Expensive Synchronous Load

Explanation

PASS: The commit changes only Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift. It only changes test control flow by starting a gated dial in a task, waiting for the builder call, releasing the gate, and awaiting completion. No production Swift code or synchronous agent-history load path changed.

Full details: Cmux Cache Substitution Correctness

Explanation

PASS. The commit changes only Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift. It updates test synchronization around GatedConnectivitySessionBuilder; it does not change production Swift, TypeScript, or JavaScript code. Therefore, the cache-substitution failure condition does not apply.

Full details: Cmux No Hacky Sleeps

Explanation

PASS. The diff changes only the Swift test file CmxConnectivityPeerSessionTests.swift; it does not modify TypeScript, JavaScript, shell, or build/runtime scripts. The added coordination uses the test-only GatedConnectivitySessionBuilder, Self.waitUntil, and explicit builder.release(). The custom check excludes this Swift test scaffolding and reports only covered non-Swift production/runtime changes.

Full details: Cmux Algorithmic Complexity

Explanation

PASS. The diff changes only Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift. It updates test coordination around a gated dial and adds no production code. The repository rule explicitly passes test-only scaffolding and fixtures, so the algorithmic-complexity failure conditions do not apply.

Full details: Cmux Swift Concurrency

Explanation

PASS: The commit changes only the Swift test file. It adds a local Task for the gated dial, waits for builder.callCount(), releases the gate, and awaits dial.value. This is not fire-and-forget work. It is an awaited test synchronization task, which the modernization rules explicitly allow. The diff adds no Dispatch queues, Combine state, or completion-handler API.

Full details: Cmux Swift `@Concurrent`

Explanation

PASS. The diff changes only test coordination in onePeerTraceUsesOneAliasAndOneEstablishedSessionEvent: it creates a Task, waits for the gated builder, releases it, and awaits the task. The changed file has no @MainActor, @concurrent, or nonisolated isolation involved. The task calls CmxConnectivityPeerSession.connectedSession, which is an actor-isolated method, and the change adds no CPU-heavy, file-heavy, or network helper called from UI isolation. No explicit @concurrent violation is introduced.

Full details: Cmux Swift Package Boundaries

Explanation

PASS: The pull request changes only Packages/Shared/CmuxIrohTransport/Tests/.../CmxConnectivityPeerSessionTests.swift. The diff contains test synchronization changes only. It introduces no production Swift feature logic or app-target code, so the Swift package boundary failure condition does not apply.

Full details: Cmux Swiftpm Lockfiles

Explanation

PASS. The pull request changes only Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift (+7/-1). It does not change Package.swift, Package.resolved, .gitignore, workflow files, or Xcode package references. Therefore, it introduces no SwiftPM lockfile or ignore-policy violation.

Full details: Cmux Swift Logging

Explanation

PASS: The commit changes only Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift, which is part of the CmuxIrohTransportTests test target. The added lines create a task, wait for the gated builder, release it, and await the task. They add no print, debugPrint, dump, NSLog, file/stdout diagnostics, Logger, or data logging. The existing diagnosticLog wiring is unchanged. The Swift logging check therefore has no applicable failure condition.

Full details: Cmux User-Facing Error Privacy

Explanation

PASS: The diff changes only a Swift test file. It adds asynchronous test coordination and a developer-only comment. It does not add or modify production user-facing errors, alerts, command output, API bodies, or recovery copy. The repository rule explicitly allows tests and developer-only comments.

Full details: Cmux Full Internationalization

Explanation

PASS: The pull request changes only Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift. The diff updates test control flow and adds developer-only comments. It adds no user-facing Swift text, localization keys, string catalogs, web messages, or locale data. The rule explicitly allows tests and developer-only comments.

Full details: Cmux Swiftui State Layout

Explanation

PASS — The pull request changes only a Swift Testing file, CmxConnectivityPeerSessionTests.swift. The diff adds asynchronous test coordination for GatedConnectivitySessionBuilder; it does not add or modify SwiftUI views, ObservableObject, @Published, GeometryReader, lazy/list row stores, or render-time state mutation. The SwiftUI state-layout check is therefore inapplicable.

Full details: Cmux Architecture Rethink

Explanation

PASS. The diff changes only a Swift test. It replaces an await on a deliberately gated dial with a test Task, a bounded test helper wait, gate release, and Task value await. The architectural rule explicitly allows test-only synchronization. The change introduces no runtime timing path, mutable state owner, duplicate wiring, or UI lifecycle owner. The parent test contained the deadlock, and the neighboring test already uses the same gate-release pattern.

Full details: Cmux Swift Auxiliary Window Close Shortcuts

Explanation

PASS: The pull request changes only CmxConnectivityPeerSessionTests.swift. The diff adds asynchronous test gating and does not add or materially change NSWindow, NSPanel, NSWindowController, SwiftUI Window, or WindowGroup code. The changed file contains no auxiliary-window identifiers or close-shortcut routing. This is a test-only fixture change, which the rule allows.

Full details: Cmux Source Artifacts

Explanation

PASS. The pull request changes one existing Swift test at Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift. The diff adds test synchronization logic and comments only. It adds no local output, generated artifact, cache, temporary directory, dependency checkout, build output, or broad artifact directory.

Full details: Cmux No Test Or Debug Seam In Production Source

Explanation

PASS: The only changed file is Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift. It is under Tests/, and the diff adds only test synchronization code. No production Swift file under a Sources/ path changed, so the custom check does not apply.

Full details: Cmux No Ambient Global State

Explanation

PASS: The pull request changes only Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift. The diff changes test orchestration only and introduces no production Swift changes. Therefore this production-only ambient-global-state check is not applicable.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix-peer-session-test-deadlock

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

greptile-apps · 2026-08-25T13:38:56Z

Greptile Summary

This test-only change fixes a deterministic deadlock in the peer-session diagnostic test.

Starts the gated connection attempt in a task.
Waits until the builder records the dial before releasing its gate.
Awaits the completed dial before validating lifecycle diagnostics.

Confidence Score: 5/5

The PR appears safe to merge and fixes the test deadlock without affecting production code.

The test now observes that the gated builder has begun its dial before releasing the continuation, and actor serialization prevents release from racing ahead of continuation installation.

Important Files Changed

Filename	Overview
Packages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxConnectivityPeerSessionTests.swift	Correctly sequences the gated test dial using the established neighboring-test pattern, with no production behavior changes or actionable defects identified.

_{Reviews (1): Last reviewed commit: "test(iroh): release the gated dial that ..." | Re-trigger Greptile}

lawrencecchen mentioned this pull request Aug 25, 2026

iOS Iroh: bounded dials, hint-refresh fallback, fail-open credential refresh #10739

Closed

lawrencecchen merged commit d7b59ba into main Aug 25, 2026
7 checks passed

Navigation Menu

Repository navigation

Un-deadlock CmxConnectivityPeerSessionTests (red swift-package-tests gate on main) - #10741

Un-deadlock CmxConnectivityPeerSessionTests (red swift-package-tests gate on main)#10741
lawrencecchen merged 1 commit into
mainfrom
fix-peer-session-test-deadlock

lawrencecchen commented Aug 25, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 25, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot commented Aug 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lawrencecchen commented Aug 25, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot commented Aug 25, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lawrencecchen commented Aug 25, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 25, 2026 •

edited

Loading