Remove residual @MainActor overhead from nonisolated network layer#21806
Merged
Conversation
BtwClient is no longer @mainactor (removed in #21729), and streamBtw() only calls nonisolated GatewayHTTPClient methods. The Task { @mainactor in } was adding a needless main-thread hop. Changed to Task { } so the streaming work runs on the cooperative thread pool. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
⚙️ Control Options:
|
…mpatibility The nonisolated resolveBaseURL() and normalizedBaseURL() functions reference platformURLOverrideEnvironmentKey, authServiceBaseURLDefaultsName, and defaultBaseURL — all static let properties on the @mainactor class. In Swift 6 language mode, this is an error (cross-isolation access). Move the constant values to module-level private lets (nonisolated by default) and alias them back into the class for backward compatibility. The nonisolated functions now reference the module-level constants directly. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
The class-level private static lets (platformURLOverrideEnvironmentKey, authServiceBaseURLDefaultsName, defaultBaseURL) are unused — all consumers in resolveBaseURL() now reference the module-level constants directly. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
Merged
3 tasks
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.
Summary
After #21729 moved the network layer off
@MainActor, two residual isolation issues remained:1. Unnecessary main-thread hop in
BtwClient.sendMessage()The
Task { @MainActor in ... }wrapper forces every BTW streaming request onto the main thread, even thoughBtwClientand all downstream methods (streamBtw(),GatewayHTTPClient.streamPostWithRetry()) are now nonisolated. Changed toTask { ... }so the work runs on the cooperative thread pool.Why this matters: Every BTW side-chain message unnecessarily blocked the main thread for the duration of SSE stream setup. With the network layer nonisolated, this hop has zero purpose and adds latency + main-thread contention.
2. Swift 6 cross-isolation errors in
AuthService.resolveBaseURL()The
nonisolatedfunctionsresolveBaseURL()andnormalizedBaseURL()referenceplatformURLOverrideEnvironmentKey,authServiceBaseURLDefaultsName, anddefaultBaseURL— allprivate static leton the@MainActorclass. Swift 6 language mode treats this as a cross-isolation error because static properties on a@MainActorclass inherit actor isolation, even if they are immutable.Fix: Move the three constants to module-level
private letdeclarations (nonisolated by default). The values are unchanged — only their declaration site moved.Why this is safe:
BtwClient:streamBtw()only callsGatewayHTTPClient.streamPostWithRetry()(nonisolated),JSONSerialization(thread-safe), andAsyncThrowingStream.Continuation(Sendable). No MainActor-dependent code.AuthService: All three constants are immutableletvalues initialized once. Moving them to module scope changes when they initialize (module load vs. class static init) but this is equivalent — the values are compile-time#if DEBUGbranches with no runtime dependencies.References:
@MainActortypes inherit isolation@MainActorfor UI state onlyReview & Testing Checklist for Human
platformURLOverrideEnvironmentKey,authServiceBaseURLDefaultsName,defaultBaseURL) are gone and no new warnings appear.Taskchange.VELLUM_PLATFORM_URLenvironment override and debugUserDefaultsfallback still resolve correctly.Notes
Taskannotation is used.Link to Devin session: https://app.devin.ai/sessions/e19a9c46f61546368efaba6020397e17
Requested by: @ashleeradka