Repository navigation
forward-port the 3.3.1 hotfix into develop - #1585
Merged
Merged
Conversation
…in actor (#1580) * deliver the audiobook token refresh outcome on the main actor (PP-5299) AudiobookLoader is @mainactor, so resolveSource checks isolation on entry. Since 3.3.0 TPPNetworkExecutor.refreshTokenAndResume delivers its callback from its own Task, and refreshTokenIfNeeded continues from there into resolveSource, which traps (Crashlytics 90c78a67e737857b30512e157c75a045). All four exits now reach the main actor: the two direct callbacks hop through a carrier holding the completion and outcome, neither being Sendable, and TokenReadyCompletionBox.fire is @mainactor so no awaitTokenReady path can deliver off-main. The matrix gains an E3-Token-Expiry row for the token-auth and lapsed-token intersection no audiobook row covered. Scope: the awaitTokenReady exits carry a test; the direct callbacks need a seam. Not done: TPPNetworkExecutor's own delivery executor is unchanged. * chore(release): bump to build 512 for the PP-5299 audiobook fix 3.3.1 was cut on 2026-09-25 for PP-5240 and never submitted, so this rebuilds the same marketing version rather than burning a new one. Build 512 is the next number in the single global sequence; 511 is the highest on main. The waiver carries forward the nine fixes held for 3.4.0 on 2026-09-25, plus three test-only commits that reached develop afterwards. Not done: 9a4b48a (#1571, PP-5293/PP-5294) is left unwaived on purpose — it touches production and is patron-facing, so holding it is a release decision. * pin the audiobook token refresh delivery with real tests (PP-5299) Correct the mechanism this branch recorded. The executor has delivered the refresh callback from inside its own Task since at least 3.2.4; the delivery did not move. What changed in 3.3.0 is the language mode (876f763: SWIFT_VERSION 5.0 to 6.0, SWIFT_STRICT_CONCURRENCY complete), which promotes the dynamic isolation check from a warning to an assert, so a silent race became a trap. RefreshOutcomeDelivery is internal so a test can construct it off-main and assert both the thread and the outcome. awaitTokenReady's two branches are pinned by keychain state rather than inherited from whatever a prior suite left, and each asserts the delivered Bool. TokenRefreshAndRetryQueueTests characterizes the off-main delivery the hop exists for, so moving it later goes red deliberately. Scope: four of six hand-derived mutants now fail a named test, up from one; palace_mutate derives no points on these lines, so those are illustration. Not done: reverting either call site to a direct completion still survives, which needs an injection seam on the loader. * drop the token refresh tests that mutate the shared account (PP-5299) Three of the four tests added here could only choose a branch by writing credentials into the process-wide account, which left an expired token behind for later suites and made a single-flight guard elsewhere count two transitions instead of one. The carrier test remains. It needs no singleton and still fails when deliverOnMain's body is emptied or its hop removed. Also waives 9a4b48a for 3.4.0: only the targeted fix ships on this line. Not done: the fire(true)/fire(false) swap and the executor's off-main delivery are unpinned, pending an injection seam on the loader. * pin the poll carrier's hop, restore the delivery test (PP-5299) Nothing pinned @mainactor on TokenReadyCompletionBox.fire once the earlier tests were removed, and dropping that attribute leaves await as a warning, so it compiles silently. The box is now internal with a test that constructs it off-main and asserts both the thread and the value. Restores the delivery characterization removed in 28c7290. That reason was wrong: the suite builds its executor through the full-DI initializer against account mocks whose token storage is in-memory, so it never touched the process-wide account. Not done: the two call sites and the fire(true)/fire(false) values stay unpinned, pending an injection seam on the loader. * join the delivery hop in tests instead of racing a deadline (PP-5299) STARVE-001 flagged three new fixed-deadline waits. Two are now gone rather than annotated: deliverOnMain returns its hop so a test can await it, and the poll carrier's fire is already awaited, so neither test needs an expectation. A small reference-type record proves the completion ran. The third, on the executor refresh, is annotated: it waits on a stubbed HTTP round trip that answers synchronously, which is the bounded case the rule's own hatch is for, and it matches the sibling refresh tests in that file. Scope: deliverOnMain gains @discardableResult and returns Task<Void, Never>; production call sites are unchanged. * name the carrier mutation precisely, and why the wait stays (PP-5299) The doc comments pointed at emptying deliverOnMain's body, which no longer compiles now that it returns the hop. The mutation they mean is emptying the hop's closure. The STARVE-001-OK reason claimed a synchronous stub bounds the wait. The rule guards against starvation under parallel clones, so the real reason is that refreshTokenAndResume exposes no join seam. Comment and annotation text only.
3.3.1 (build 512) carried the audiobook token-refresh isolation fix, which existed only on the release line; develop had none of it, so 3.4.0 would have reshipped the crash. Conflicts: the build number takes 512, since these are one sequence and develop's 510 would let the next bump reuse a shipped number; the token-ready carrier takes the release line's version, which is the fix; the release waiver follows develop's rename of its directory. Marketing version stays 3.4.0. Also corrects the token-expiry row, which told a tester to advance the Simulator clock — there is no such control there. Scope: the forward-port and that one doc row.
mauricecarrier7
added
hold merge
🏗️ Architecture and AccessibilityNothing to act on. No major accessibility issues and no new architecture findings. All figures below are whole-repo, measured at this PR's head commit — not a diff against the base. A number that looks alarming is usually the repo's standing baseline, not something this branch did. ♿ Accessibility🟡 20 issues, none blocking. Worth fixing, safe to merge.
Top Issues:
📋 View All Accessibility Issues
🏛️ ArchitectureLargest dependency tangle: 14 components — 1 distinct cycle. This is the number to watch during the modularisation: it should fall as packages come out. Extracting a leaf without cutting a cycle leaves it unchanged. All architecture metrics
Two of these read the opposite way to how they sound:
🔄 Dependency Cycles (1)1 distinct dependency cycle (largest spans 14 components).
Cycle 1 — 14 components, 39 edges between them
edgesDiscounted 📋 Architecture Findings
🔍 Reachability Analysis
ℹ️ Reachability not evaluated. Its analyzer reads a top-level Scanned: whole repo at Powered by CodeAtlas Ledger |
🧪 Unit Test Results📊 View Full Interactive Report ✅ ALL TESTS PASSED9543 tests | 9525 passed | 0 failed | 16 skipped | ⏱️ 15m 33s | 📊 99.8% | 📈 54.0% coverage All 1048 classes (full matrix — click to expand)
📊 Full interactive matrix: report 📊 Testing Coverage BreakdownUnit Test Line Coverage (testable surfaces): 54.0% Total coverage incl. UI/lifecycle: 53.4% (17 files excluded from testable denominator — see
🔗 Interactive HTML Report | CI Run Details Counts above were produced by this CI run's xcresult parse — reproduce via the run link. 📦 Downloadable Artifacts
|
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
1 participant
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.
Merge this with a merge commit — never squash. The two parents are the point.
What
Forward-ports
origin/main(3.3.1, build 512) into develop. Develop had zerooccurrences of the PP-5299 audiobook token-refresh isolation fix, so 3.4.0 would
have reshipped the crash.
Why not squash
Squashing collapses the merge and discards main's original SHAs, so when the next
release branch merges into main git treats those commits as unrelated history —
the failure that produced 296 conflicts on
release/3.1.0 → main. See "Release &hotfix merge policy" in CLAUDE.md. Squash stays fine for ordinary feature PRs
into develop; this is the exception.
Conflict resolutions
project.pbxproj: build number takes 512, not develop's 510. These are oneglobal sequence, so keeping 510 would let the next bump reuse a shipped number.
MARKETING_VERSIONstays 3.4.0.AudiobookLoader.swift: took main's hunk — the non-privateTokenReadyCompletionBoxis part of the fix, since a test constructs it.config/ci/release-waivers/,matching the sibling 3.3.1 waiver already there.
Also corrects the
E3-Token-Expiryrow, which told testers to advance theSimulator clock — there is no such control.
How verified
AudiobookLoaderDispatchTests,TokenRefreshAndRetryQueueTests,TokenRefreshTests: 45 executed, 0 failures, zero restarts. A scopedspot-check, not the full suite — CI covers that.