fix(test): end the CI unit-test hang by making Turbine buffer emissions - #21
Merged
Conversation
PR #17 could be merged while its checks were still running (and the unit-test job went on to fail) because main has no required status checks. Add a single always()-guarded job that fails unless every CI job succeeded, so a branch ruleset only needs to require the one stable check name "CI passed" instead of tracking all four job names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6
The Turbine helper's expectInitial recursed on unchanged state with no suspension point whenever the current value equaled the expected one: the task never yields, pinning a cooperative-pool thread and allocating async frames without bound. Locally the view model's next emission interrupts the spin within milliseconds, but on the small CI runners all turbine tests spin at once, occupy the whole pool, starve the very work that would emit the next value, and the test process dies ~90s in with no summary — the "test-process hang" that survived PR #17. Rebuild the helper on an AsyncStream: every emission is buffered and consumed oldest-first, waiting is a real suspension, and expectInitial consumes the initial value when it is still there or puts the first value back when the source progressed before subscription. Add TurbineTests pinning both semantics; the first test is the minimal reproduction of the spin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6
Constrain the protocol's Value to Sendable: requirement calls hop into the MainActor-isolated implementation, and the unconstrained associatedtype made that hop ill-formed through any Turbine. Advance the AsyncStream iterator via a local copy: a mutating async call on an isolated stored property is rejected because the exclusive access would span the suspension, and the iterator is only a handle to the stream's shared storage, so a copy is equivalent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6
The plain next() is nonisolated, so even on a local copy the call sends the non-Sendable, main-actor-region iterator across an isolation boundary and Swift 6 rejects it. next(isolation:) runs the advance isolated to the caller, keeping the whole step on the main actor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6
Changed Files
|
The branch ruleset will require the four job checks directly instead of a single aggregate name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6
Creating a ruleset needs repository admin permission, so it cannot be automated from CI or a scoped session. Keep the intended configuration versioned here; apply it via Settings -> Rules -> New ruleset -> Import a ruleset, or POST it to the rulesets API. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6
fardavide
enabled auto-merge (squash)
August 5, 2026 04:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the "test-process hang" that survived #17 and is currently keeping
mainred: the unit-test job dies ~90s into the run with exit code 1 and no test summary, on both PR and push runs.Root cause
Turbine.expectInitialrecursed on unchanged state whenever the current value equaled the expected one. That recursion has no suspension point, so the task never yields: it pins a cooperative-pool thread and allocates async frames without bound until a different value happens to arrive.load()work that would produce the interrupting emissions. The process stalls, memory grows, and it dies without reporting — taking the unrelated suites down with it.Fix
RealTurbineis rebuilt on anAsyncStream: every emission is buffered and consumed oldest-first, and waiting is a genuine suspension (iterator.next(isolation:)), so the pool is never touched.expectInitialkeeps its contract — consume the initial value if the subscription caught it, or put the first value back forvalue()if the view model progressed before we subscribed. Verified against every call site that the FIFO semantics return exactly what the old latest-only code returned (each observed field is emitted once per load/send, withremoveDuplicatesupstream).Two Swift 6 strict-concurrency requirements shaped the implementation: the protocol's
associatedtype Valuemust beSendable(values hop into the MainActor-isolated witness), and the stream iterator is advanced via a local copy withnext(isolation: #isolation)(SE-0421), since amutating asynccall on an isolated stored property — or a plain nonisolatednext()— is rejected.Also in this PR
TurbineTests(new, in the previously emptyTestTeststarget): 4 regression tests; the first is the minimal reproduction that spins forever under the old code..github/rulesets/protect-main.json: importable branch ruleset requiring all four CI checks onmainwith an empty bypass list. Creating rulesets needs repo-admin, so apply it via Settings → Rules → New ruleset → Import a ruleset, orgh api repos/fardavide/Swiftly/rulesets --method POST --input .github/rulesets/protect-main.json.ci.ymlis net-unchanged: an aggregate "CI passed" gate job was added and later removed in favor of requiring the four job checks directly.Verification
Full CI dispatched on this branch: run 30963134024 — all jobs green. The unit-test step completes in 82s with a full summary, where it previously died at ~90s.
Merging this turns
maingreen again.🤖 Generated with Claude Code
https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6