Swift 6.4 tools version, async/await download stack, and WASI guards#19
Conversation
Contribute declared `// swift-tools-version: 5.8` — the lowest in the BrightDigit stack — but CI only ever builds and tests on Swift 6.4 nightly, so that 5.8 floor was never verified. Bump the manifest to 6.4 so the package matches the rest of the stack and builds in Swift 6 language mode with complete strict concurrency checking. Strict-concurrency fixes the language-mode change required: - `MarkdownContentBuilderOptions` is now `Sendable`; its static option constants were global mutable state without it. - `ImportError`'s nested field enums (`ResponseComponent`, `VideoField`, `EpisodeField`, `NewsletterField`) are `Sendable`, and the `Any` payloads on the (already deprecated) cases became `any Sendable`. In Swift 6 `Error` refines `Sendable`, so `Any` payloads warned. - `FileManagerProtocol`, `URLSessionable` and `URLDownloader` now refine `Sendable`, and `URLDownloader`/`FileURLDownloader` completion handlers are `@Sendable`. `FileURLDownloader.downloadFromNetwork` hands `self` and the completion to `URLSessionable.download`'s already-`@Sendable` handler, so both had to be `Sendable` to compile. `FileManager` and `URLSession` — the standard conformers — are already `Sendable`. - `FileManagerSpy` follows: its call-recording flags moved behind an `NSLock`. Platform floors are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Document the 2026-07-24 hygiene pass across the seven Wave 1 package branches (un-deprecation of ContributeYouTube/ContributeMailchimp + tests, Publish header normalization, TailwindKit dead-workflow removal), the Contribute→6.4 migration (brightdigit/Contribute#19) and the coordinated ContributeWordPress #18 data-race fix that must land with it, the stale CI-comment cleanup PRs, and the two open items needing a human (Plot/Files/Ink missing CLAUDE_CODE_OAUTH_TOKEN; SyndiKit has no macOS-platforms job). Adds .claude/memory/wave1-hygiene-pass.md (+ MEMORY.md index line) and threads the coordinated-landing constraint and resolved ContributeWordPress .swift-version drift through MERGE-AND-TAG.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the completion-handler download API with async/await throughout, on top of the Swift 6.4 tools-version raise: - `URLSessionable.download(fromURL:)` returns `(URL, URLResponse)` and is backed by `URLSession.download(from:)`, Foundation's native async overload, instead of wrapping `downloadTask(with:completionHandler:)`. - `URLDownloader.download(from:to:allowOverwrite:)` is `async throws`; errors now propagate instead of being handed to an `@escaping @sendable (Error?) -> Void`. - `FileURLDownloader` splits into `downloadFromNetwork` (async) and `copyToDestination` (sync file I/O), removing the nested-callback path. This supersedes the `@escaping @Sendable` hardening: async signatures carry the same Sendable guarantees without the callback plumbing, and it lets consumers fan out with structured concurrency rather than a DispatchGroup. A behavior fix falls out of it: the old `downloadFromNetwork` dropped the session error whenever a destination URL was also returned. Failures are no longer silently swallowed. Tests move to async: NetworkManagerSpy returns/throws instead of calling back, and the XCTestExpectation plumbing in the download helpers is gone. Verified on macOS and in the nightly-6.4 Linux container (29 XCTest + 19 swift-testing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`URLSession` — and `URLResponse` with it — does not exist on WASI, but the download stack imported and used them unconditionally. That predates the async conversion; nothing caught it because Contribute sets ENABLE_WASM=false (Yams cannot build for wasm: `cannot find 'DBL_DECIMAL_DIG' in scope`), so the wasm legs never run here. Follows the pattern ButtondownKit already uses (ButtondownClient.swift): keep the type available everywhere, gate only the URLSession-backed members. - `URLSessionable` is gated in full — its requirement returns `URLResponse`, which is itself unavailable on WASI. - `FileURLDownloader` stays available: copying from a `file:` URL works on every platform. Its `networkManager`, the `URLSession.shared` default initializer, and `downloadFromNetwork` are gated; a non-file URL on WASI throws `URLDownloaderError.networkUnavailable` instead of failing to compile. - Adds `URLDownloaderError` rather than extending `ImportError`, which is deprecated and would warn at every new use site. Verified by cross-compiling the five download-stack files as a standalone wasm target (the full package stops on Yams before reaching them). That probe caught a real defect in the first attempt: `downloadFromNetwork` survived on WASI while the `networkManager` it referenced did not. macOS: 29 XCTest + 19 swift-testing green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI failed on every platform with:
AssetDownloader.swift:40:15: error: stored property 'urlDownloader' of
'Sendable'-conforming struct 'AssetDownloader' has non-Sendable type
'any URLDownloader'
The manifest pins Contribute to `branch: "main"`, but Package.resolved still
recorded 5dc9049 — main as it was *before* brightdigit/Contribute#19 merged. SwiftPM
honours the lockfile revision, so CI compiled the task-group AssetDownloader
against the pre-async Contribute, where URLDownloader is neither Sendable nor
async.
`swift package update Contribute` repins to 2d346bd (current main). Local builds
missed this because Contribute was in `swift package edit` mode, which drops the
dependency from Package.resolved and substitutes a working copy — so passing tests
said nothing about what CI would resolve.
Verified against the real dependency with edit mode off: 45/45 tests pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contribute's download stack and ContributeWordPress's asset fan-out are async, so `MarkdownProcessor.begin` is `async throws`. The enclosing `execute()` was already `async throws`, so this is the only call-site change in the root. Regenerates Package.resolved for Contribute (brightdigit/Contribute#19 merged to main: 6.4 tools version, async/await download stack, WASI guards) and ContributeWordPress (DispatchGroup fan-out replaced with withThrowingTaskGroup, removing an unsynchronized [URL: Error] data race). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Why
Package.swiftdeclared// swift-tools-version: 5.8— the lowest of any BrightDigit package (every other one is on 6.4; SyndiKit is on 5.10). But CI here only ever builds and tests against Swift 6.4 nightly, so the advertised 5.8 support has never actually been verified by anything. Rather than add a second CI matrix to back a claim nobody relies on, raise the floor to 6.4 so Contribute matches the rest of the stack and keeps the single 6.4 matrix.At tools-version 5.8 the package built in Swift 5 language mode. At 6.4 it builds in Swift 6 language mode with complete strict concurrency checking, which surfaced a handful of real issues. All of them are fixed properly — no
swiftLanguageMode(.v5), no relaxed-strict-concurrency, no suppressed diagnostics, no@unchecked Sendable.Strict-concurrency fixes
MarkdownContentBuilderOptions→SendableIts
internal static let shouldOverwriteExisting/includeMissingPreviouswere hard errors (static property … is not concurrency-safe because non-'Sendable' type … may have shared mutable state). The type is anOptionSetover a singlelet rawValue: Int, so it just needed the conformance spelled out — public types don't get it inferred.ImportErrorpayloadsIn Swift 6
ErrorrefinesSendable, so every associated value of anError-conforming enum is checked. The four nested field enums (ResponseComponent,VideoField,EpisodeField,NewsletterField) now declare: Sendable, and theAnypayloads oninvalidPodcastEpisodeFromRSSItem,missingFieldForVideo,missingVideoForEpisode,missingFieldFromPodcastEpisodeandduplicateTitle(_:forVideos:)becameany Sendable.Note this is a signature change on a public, already
@available(*, deprecated)enum. Nothing outside this package uses those cases (brightdigit.com only throwsImportError.newsletterMissingField, which is unchanged), andany Sendableaccepts everything a caller would realistically pass, so the practical break is nil — but flagging it since this is a released v1.0.0 package.Sendable propagation through the download stack — the substantive one.
URLSessionable.download(fromURL:completion:)already took an@escaping @Sendablecompletion (it has to:URLSession.downloadTask(with:completionHandler:)demands one).FileURLDownloader.downloadFromNetworkpasses a closure to it that captures bothselfand its owncompletion, which in Swift 6 mode is two hard errors:There's no way to restructure that away — the closure genuinely needs the file manager to finish the local copy, and it genuinely needs the caller's completion handler. So the dependency abstractions became
Sendable:FileManagerProtocol: SendableURLSessionable: SendableURLDownloader: Sendable, with_ completion: @escaping @Sendable (Error?) -> Void@SendableonFileURLDownloader's threedownload*methodsThis is a correctness improvement, not a workaround: both standard conformers (
FileManager,URLSession) are alreadySendable, andFileURLDownloaderis a struct over twolets, so it satisfiesURLDownloader: Sendableas-is.FileManagerSpy(tests) follows from the above — aSendableprotocol needs aSendableconformer, and the spy's fourIsCalledflags are mutable. They moved into a privateCallRecordstruct guarded by anNSLock, exposed through computed properties.Synchronization.Mutexwould encode that in the type system, but it requires macOS 15 / iOS 18 and this package supports macOS 12 / iOS 13, so the lock is external and the storage isnonisolated(unsafe)(targeted, and every access goes through the two locked accessors — not the blanket@unchecked Sendablethe SwiftLintno_unchecked_sendablerule bans).XCTestCase+Extension— the tworunFileURLDownloader*helpers forward their completion into the now-@SendableAPI, so their parameters are@escaping @Sendabletoo.Platform floors
Unchanged:
.macOS(.v12),.iOS(.v13),.tvOS(.v13),.watchOS(.v6). Nothing in the 6.4 toolchain forced a bump, and this is a released v1.0.0 package. README's Requirements section is updated to Swift 6.4+ with those same floors.One cosmetic side effect: at tools-version 6.4 the manifest now emits
'v13' is deprecated: iOS 15.0 is the oldest supported version(and the tvOS/watchOS equivalents) — warnings fromPackageDescription, not errors. Left as-is deliberately; narrowing platform support is a separate decision.Verification (macOS, Swift 6.4 / swiftlang-6.4.0.27.1)
swift build— cleanswift build -c release— clean (the SwiftSoup fork's inline-crash workaround still holds)swift test— 48 tests pass, 0 failures (29 XCTest + 19 swift-testing across 2 suites)LINT_MODE=STRICT ./Scripts/lint.sh—Found 0 violations, 0 serious in 55 files, periphery reports no unused code, "Linting completed successfully"The only remaining build warning is the pre-existing
found 1 file(s) which are unhandled … Documentation.docc, which is present onmaintoo (verified by reverting just the tools-version line).Downstream impact — read before merging
Making
URLDownloader's completion@Sendableis not optional (see above), and it does break ContributeWordPress, which is the only package in the stack that touches these APIs. I verified this by building ContributeWordPress against this branch (swift package edit Contribute --path …). Three sites need follow-up there:Sources/ContributeWordPress/Images/AssetDownloader.swift:97—error: capture of 'asset' with non-Sendable type 'AssetImport' in a '@Sendable' closureSources/ContributeWordPress/Images/AssetDownloader.swift:97—error: mutation of captured var 'errors' in concurrently-executing codeTests/ContributeWordPressTests/Helpers/Spy/FileDownloaderSpy.swift:6—error: stored property 'downloadIsCalled' of 'Sendable'-conforming class 'FileDownloaderSpy' is mutable(2) is worth calling out:
AssetDownloader.downloadUsingGroupDispatchfans downloads out over aDispatchGroupand every callback writes into the samevar errors = [URL: Error]()with no synchronization. With the realFileURLDownloaderthose callbacks arrive onURLSession's delegate queue, so that is an actual pre-existing data race that Swift 5 mode was hiding — this change surfaces it rather than causing it.Nothing else is affected: ContributeMailchimp / ContributeYouTube / ContributeRSS only use
MarkdownContentBuilderOptions(unchanged apart from gainingSendable), and brightdigit.com itself only throwsImportError.newsletterMissingField.So this should land together with a matching ContributeWordPress PR.
🤖 Generated with Claude Code