From abc62105e0315cb89cb0ef13f0d72d1e74c4f5df Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 23:35:26 +0000 Subject: [PATCH 1/6] ci: add a "CI passed" aggregate gate job for branch protection 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 Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6 --- .github/workflows/ci.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4b957dd..d8422fd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -156,3 +156,32 @@ jobs: path: snapshot-report retention-days: 7 if-no-files-found: warn + + # Aggregate gate over every job above, meant to be the ONE required status check in the branch + # protection ruleset for main. Requiring the single stable name "CI passed" instead of the four + # job names means adding, renaming or re-matrixing jobs can never desync the ruleset — a stale + # required name would otherwise sit "Expected" forever and block every merge, while a new job + # missing from the list simply wouldn't gate. Runs on the cheap Linux runner: it only inspects + # results. + ci-passed: + name: CI passed + # always() keeps the gate reporting when an upstream job fails or is cancelled. Without it the + # gate would be SKIPPED in those runs, and GitHub counts a skipped required check as satisfied + # — the merge would sail through on red CI, which is the exact hole this job closes. + if: always() + needs: [unit-tests, build, snapshot-tests] + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Require every job to have succeeded + env: + RESULTS: ${{ join(needs.*.result, ' ') }} + run: | + set -euo pipefail + echo "Job results: $RESULTS" + for result in $RESULTS; do + if [ "$result" != "success" ]; then + echo "::error::A required job finished with result '$result'." + exit 1 + fi + done From ad38c9d67c275924a1e5574346f9a44271d0ae14 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 23:47:45 +0000 Subject: [PATCH 2/6] fix(test): make Turbine buffer emissions instead of spinning on state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6 --- SwiftlyCore/Sources/Common/Test/Turbine.swift | 100 +++++++++--------- .../Tests/Common/TestTests/TurbineTests.swift | 80 ++++++++++++++ 2 files changed, 128 insertions(+), 52 deletions(-) create mode 100644 SwiftlyCore/Tests/Common/TestTests/TurbineTests.swift diff --git a/SwiftlyCore/Sources/Common/Test/Turbine.swift b/SwiftlyCore/Sources/Common/Test/Turbine.swift index 005425e..d0fb7e4 100644 --- a/SwiftlyCore/Sources/Common/Test/Turbine.swift +++ b/SwiftlyCore/Sources/Common/Test/Turbine.swift @@ -15,9 +15,9 @@ public func test( .removeDuplicates() .eraseToAnyPublisher() ) - + await block(turbine) - + turbine.complete() } @@ -26,80 +26,76 @@ public func test( public protocol Turbine: Sendable { associatedtype Value + /// The oldest unconsumed emission, suspending until one arrives if none is buffered. func value() async -> Value + + /// Consumes the first emission when it equals `value`; otherwise puts it back for the next `value()` call. + /// + /// A `@Published` publisher replays the current state on subscribe, so what arrives first depends on + /// timing: subscribe before the view model's startup work lands and it is the initial state; subscribe + /// after and the initial state was never seen, only the progressed one. Both orders are legitimate, which + /// is why a mismatch is put back rather than reported as a failure. func expectInitial(value: Value) async } -/// `@unchecked` because the compiler cannot see that the only mutable state is a `CurrentValueSubject` -/// (safe to send and read concurrently) and a `cancellables` array written once, during `init`. -final class RealTurbine: Turbine, @unchecked Sendable { +/// Buffers every emission into an `AsyncStream` and hands them out oldest-first, so waiting for the next +/// value is a genuine suspension. +/// +/// The previous implementation kept only the latest emission and, when `expectInitial` saw the expected +/// value, recursed on unchanged state until something new arrived. That recursion has no suspension point, +/// so it never yields its cooperative-pool thread and allocates async frames without bound. On a many-core +/// dev machine the view model's next emission lands in milliseconds and the spin goes unnoticed; on a small +/// CI runner a handful of tests spinning together occupy the whole pool, the work that would produce those +/// emissions can never run, and the process starves until it dies — taking every suite's results with it. +/// +/// `@MainActor` rather than `@unchecked Sendable`: every touchpoint already lives there — the publisher is +/// subscribed and emits on the main actor, and the test body consuming the turbine runs on it too. Single +/// consumer by contract, matching the sequential test bodies: two concurrent `value()` calls would race on +/// the iterator. +@MainActor +final class RealTurbine: Turbine { - private let subject = CurrentValueSubject, Never>(.notReady) + private var iterator: AsyncStream.Iterator + private let continuation: AsyncStream.Continuation + private var pushedBack: Value? private var cancellables: [AnyCancellable] = [] init(publisher: AnyPublisher) { - publisher.removeDuplicates() - .sink { value in self.subject.value = .ready(value) } + let (stream, continuation) = AsyncStream.makeStream(of: Value.self) + iterator = stream.makeAsyncIterator() + self.continuation = continuation + publisher + .removeDuplicates() + .sink { continuation.yield($0) } .store(in: &cancellables) } func value() async -> Value { - let value = switch subject.value { - case .notReady: await awaitFirst() - case let .ready(ready): ready - } - subject.value = .notReady - return value + await next() } func expectInitial(value: Value) async { - switch subject.value { - case .notReady: - let v = await awaitFirst() - if v == value { - await expectInitial(value: v) - } - subject.value = .notReady - case let .ready(v): - if v == value { - await expectInitial(value: v) - } + let first = await next() + if first != value { + pushedBack = first } } func complete() { - subject.send(completion: .finished) + continuation.finish() for cancellable in cancellables { cancellable.cancel() } } - private func awaitFirst() async -> Value { - await withUnsafeContinuation { continuation in - var cancellable: AnyCancellable? - - // Unwrap to `.ready` values *before* `first()`. `subject` replays its current value on subscribe, and - // `awaitFirst` is only reached when that value is `.notReady` — so a bare `first()` delivers `.notReady`, - // takes the `break`, and completes without ever resuming the continuation. The awaiting task then stays - // suspended forever: the assertions still pass, but the process can never exit. - cancellable = subject - .compactMap { value -> Value? in - switch value { - case .notReady: nil - case let .ready(value): value - } - } - .first() - .sink { _ in - cancellable?.cancel() - } receiveValue: { value in - continuation.resume(returning: value) - } + private func next() async -> Value { + if let pushedBack { + self.pushedBack = nil + return pushedBack + } + guard let value = await iterator.next() else { + fatalError("Turbine completed while a test was still awaiting a value") } + return value } } - -enum TurbineValue { - case notReady - case ready(_ value: V) -} diff --git a/SwiftlyCore/Tests/Common/TestTests/TurbineTests.swift b/SwiftlyCore/Tests/Common/TestTests/TurbineTests.swift new file mode 100644 index 0000000..d359a69 --- /dev/null +++ b/SwiftlyCore/Tests/Common/TestTests/TurbineTests.swift @@ -0,0 +1,80 @@ +import Combine +import SwiftlyTest +import Testing + +@MainActor +struct TurbineTests { + + /// The minimal reproduction of the CI hang: `expectInitial` matching the current value used to spin + /// instead of consuming it, so this test never reached `send(1)`. + @Test + func whenSubscribedBeforeAnyChange_initialIsConsumed() async { + // given + let subject = CurrentValueSubject(0) + + await test(subject) { turbine in + await turbine.expectInitial(value: 0) + + // when + subject.send(1) + + // then + let result = await turbine.value() + #expect(result == 1) + } + } + + @Test + func whenInitialWasMissed_firstValueIsPutBack() async { + // given: the observed source already progressed past its initial value before we subscribed + let subject = CurrentValueSubject(5) + + await test(subject) { turbine in + // when + await turbine.expectInitial(value: 0) + + // then + let result = await turbine.value() + #expect(result == 5) + } + } + + @Test + func valuesEmittedWhileNotAwaiting_areBufferedInOrder() async { + // given + let subject = CurrentValueSubject(0) + + await test(subject) { turbine in + await turbine.expectInitial(value: 0) + + // when + subject.send(1) + subject.send(2) + + // then + let first = await turbine.value() + let second = await turbine.value() + #expect(first == 1) + #expect(second == 2) + } + } + + @Test + func whenValueArrivesWhileAwaiting_awaitResumesWithIt() async { + // given + let subject = CurrentValueSubject(0) + + await test(subject) { turbine in + await turbine.expectInitial(value: 0) + + // when: emitted only after value() below has suspended and freed the main actor + Task { + subject.send(7) + } + + // then + let result = await turbine.value() + #expect(result == 7) + } + } +} From d373787138aca9b5ce8cf46a61ea5a672d1e091b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 00:14:07 +0000 Subject: [PATCH 3/6] fix(test): satisfy Swift 6 isolation rules in the Turbine rewrite 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 Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6 --- SwiftlyCore/Sources/Common/Test/Turbine.swift | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/SwiftlyCore/Sources/Common/Test/Turbine.swift b/SwiftlyCore/Sources/Common/Test/Turbine.swift index d0fb7e4..50de776 100644 --- a/SwiftlyCore/Sources/Common/Test/Turbine.swift +++ b/SwiftlyCore/Sources/Common/Test/Turbine.swift @@ -24,7 +24,10 @@ public func test( /// `Sendable` so a turbine can be awaited from the `@MainActor` test body without the compiler treating /// each `await` as sending it across an isolation boundary. public protocol Turbine: Sendable { - associatedtype Value + /// `Sendable` because values cross from the caller of these requirements into the main-actor-isolated + /// implementation; without the constraint the compiler rejects that hop for `any Turbine`. `test()` + /// already demands it of every value type. + associatedtype Value: Sendable /// The oldest unconsumed emission, suspending until one arrives if none is buffered. func value() async -> Value @@ -93,9 +96,14 @@ final class RealTurbine: Turbine { self.pushedBack = nil return pushedBack } + // A `mutating async` call on an isolated stored property is rejected — the exclusive access would + // span the suspension. The iterator struct is only a handle to the stream's shared storage, so + // advancing a local copy is equivalent; the write-back keeps the stored handle current. + var iterator = self.iterator guard let value = await iterator.next() else { fatalError("Turbine completed while a test was still awaiting a value") } + self.iterator = iterator return value } } From 832f637b2332f138ed3e4c69f9b519268d6b383d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 00:23:34 +0000 Subject: [PATCH 4/6] fix(test): advance the Turbine iterator via next(isolation:) 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 Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6 --- SwiftlyCore/Sources/Common/Test/Turbine.swift | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/SwiftlyCore/Sources/Common/Test/Turbine.swift b/SwiftlyCore/Sources/Common/Test/Turbine.swift index 50de776..55e8fed 100644 --- a/SwiftlyCore/Sources/Common/Test/Turbine.swift +++ b/SwiftlyCore/Sources/Common/Test/Turbine.swift @@ -97,10 +97,13 @@ final class RealTurbine: Turbine { return pushedBack } // A `mutating async` call on an isolated stored property is rejected — the exclusive access would - // span the suspension. The iterator struct is only a handle to the stream's shared storage, so - // advancing a local copy is equivalent; the write-back keeps the stored handle current. + // span the suspension — so advance a local copy; the iterator struct is only a handle to the + // stream's shared storage, and the write-back keeps the stored handle current. The advance goes + // through `next(isolation:)` because the plain `next()` is nonisolated and calling it would send + // this non-Sendable, main-actor-region iterator across an isolation boundary; passing the current + // isolation keeps the whole step on the main actor. var iterator = self.iterator - guard let value = await iterator.next() else { + guard let value = await iterator.next(isolation: #isolation) else { fatalError("Turbine completed while a test was still awaiting a value") } self.iterator = iterator From 72e5ed7730d77d54b149e9cd7024cd6e1fe6f972 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 04:46:12 +0000 Subject: [PATCH 5/6] ci: remove the "CI passed" aggregate gate job The branch ruleset will require the four job checks directly instead of a single aggregate name. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6 --- .github/workflows/ci.yml | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d8422fd..4b957dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -156,32 +156,3 @@ jobs: path: snapshot-report retention-days: 7 if-no-files-found: warn - - # Aggregate gate over every job above, meant to be the ONE required status check in the branch - # protection ruleset for main. Requiring the single stable name "CI passed" instead of the four - # job names means adding, renaming or re-matrixing jobs can never desync the ruleset — a stale - # required name would otherwise sit "Expected" forever and block every merge, while a new job - # missing from the list simply wouldn't gate. Runs on the cheap Linux runner: it only inspects - # results. - ci-passed: - name: CI passed - # always() keeps the gate reporting when an upstream job fails or is cancelled. Without it the - # gate would be SKIPPED in those runs, and GitHub counts a skipped required check as satisfied - # — the merge would sail through on red CI, which is the exact hole this job closes. - if: always() - needs: [unit-tests, build, snapshot-tests] - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: - - name: Require every job to have succeeded - env: - RESULTS: ${{ join(needs.*.result, ' ') }} - run: | - set -euo pipefail - echo "Job results: $RESULTS" - for result in $RESULTS; do - if [ "$result" != "success" ]; then - echo "::error::A required job finished with result '$result'." - exit 1 - fi - done From 84dc8e76fba9bfe394f59855f9a26c2f43f6fdc4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 04:48:59 +0000 Subject: [PATCH 6/6] ci: add importable branch ruleset requiring the CI checks on main 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 Claude-Session: https://claude.ai/code/session_01Myrncy7JAwkAjnfT8jm2h6 --- .github/rulesets/protect-main.json | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 .github/rulesets/protect-main.json diff --git a/.github/rulesets/protect-main.json b/.github/rulesets/protect-main.json new file mode 100644 index 0000000..4194b7d --- /dev/null +++ b/.github/rulesets/protect-main.json @@ -0,0 +1,26 @@ +{ + "name": "Protect main", + "target": "branch", + "enforcement": "active", + "conditions": { + "ref_name": { + "include": ["~DEFAULT_BRANCH"], + "exclude": [] + } + }, + "bypass_actors": [], + "rules": [ + { + "type": "required_status_checks", + "parameters": { + "strict_required_status_checks_policy": false, + "required_status_checks": [ + { "context": "Unit tests (SwiftlyCore)" }, + { "context": "Build app (iOS)" }, + { "context": "Build app (macOS)" }, + { "context": "Snapshot tests (iOS)" } + ] + } + } + ] +}