Skip to content

Move network layer off @MainActor isolation#21695

Merged
ashleeradka merged 2 commits into
mainfrom
devin/lum-492-1774540425
Mar 26, 2026
Merged

Move network layer off @MainActor isolation#21695
ashleeradka merged 2 commits into
mainfrom
devin/lum-492-1774540425

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Mar 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Removes @MainActor from the network client layer (~44 files in clients/shared/Network/). @MainActor should only isolate UI state — not networking, URL construction, or I/O. Previously, every HTTP request (including the 15-second health check) forced URLRequest construction, URL resolution, and synchronous lockfile reads onto the main thread, contributing to app hangs (LUM-457).

Why this is needed:

  • Eliminates main-thread contention from all network request construction and I/O
  • Aligns with Apple's concurrency model: the cooperative thread pool handles networking while @MainActor is reserved for UI mutations
  • Directly addresses the Sentry AppHang stacktrace (URLRequest.init_SwiftURL._makeCFURL)

What changed:

  • Removed @MainActor from GatewayHTTPClient, HealthCheckClient, HostToolExecutor static methods, and ~40 network client protocols + structs
  • Changed Task { @MainActor inTask { in BtwClient.sendMessage() so streaming runs on the cooperative thread pool
  • Made resolveConnection() async so it can safely await the @MainActor-isolated AuthService.shared.baseURL via actor hop — this cascades async to isConnectionManaged(), buildURL(), handleAuthenticationFailure(), and InteractionClient.approvalPath() (all callers were already async)

What stays @MainActor:

  • EventStreamClient — stateful SSE subscriber management
  • GatewayConnectionManagerObservableObject driving UI state
  • AuthService — singleton whose baseURL/configuredBaseURL is mutated from UI/settings code

Why this is safe:

  • All removed clients are stateless (no stored properties = no data races)
  • URLSession, UserDefaults, and Keychain APIs are thread-safe
  • All ViewModel/UI callers already use await, so actor-hopping works transparently
  • The only @MainActor-isolated read (AuthService.shared.baseURL) is now properly awaited behind an if/else to avoid Swift's ?? autoclosure limitation
  • No logic or behavior changes — only annotation removals and async propagation

Review & Testing Checklist for Human

  • Verify the Xcode build passes with strict concurrency checking — CI requires macOS runners (all checks skipped). A local Xcode build is the primary gate.
  • Verify handleAuthenticationFailure() becoming async is safe — Previously synchronous (returned immediately, spawned internal Task). Now awaited from performHealthCheck(). The managed-mode early-return path (broadcast error + disconnect) is synchronous, but confirm the health check flow still behaves correctly on 401s.
  • Spot-check no callers of isConnectionManaged() or buildURL() were missed — All external callers found via grep were updated, but verify no other call sites exist in platform-specific targets.
  • Test end-to-end — Send a message, verify health checks succeed, try BTW streaming, open apps. Pay special attention to managed-assistant flows (session token auth) since resolveConnection() now actor-hops for baseURL.

Notes

  • Addresses LUM-492 (sub-issue of LUM-457). Follow-up issues LUM-493 (lockfile I/O off main thread) and LUM-494 (image processing off DispatchQueue.main.sync) remain for separate PRs.

Link to Devin session: https://app.devin.ai/sessions/e19a9c46f61546368efaba6020397e17
Requested by: @ashleeradka


Open with Devin

Move all network client protocols, structs, and the GatewayHTTPClient enum
off @mainactor isolation. Per Apple best practices (WWDC25), @mainactor
should only be used for UI state — not for networking, URL construction,
or I/O operations.

This eliminates the root cause of app hangs (LUM-457) where URLRequest
construction, URL resolution, and lockfile reads were forced onto the
main thread by @mainactor annotations on the entire network layer.

Changes:
- Remove @mainactor from GatewayHTTPClient enum
- Remove @mainactor from HealthCheckClient enum
- Remove @mainactor from ~40 network client protocols and structs
- Remove @mainactor from HostToolExecutor static methods
- Remove unnecessary @mainactor Task in BtwClient.sendMessage()

Kept @mainactor on:
- EventStreamClient (stateful class managing SSE subscribers)
- GatewayConnectionManager (ObservableObject driving UI)

No logic or behavior changes — only annotation removals.
All callers already use await, which handles actor-hopping correctly.

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@linear

linear Bot commented Mar 26, 2026

Copy link
Copy Markdown
LUM-492 Move network layer off @MainActor isolation

Context

The entire network layer (clients/shared/Network/) has 46 files with MainActor isolation, totaling 99 @MainActor annotations. This means all HTTP request construction, URL building, and response handling runs on the main thread.

Problem

Apple's best practice is to keep networking code off the main thread. @MainActor should only be used for UI-state mutations, not for request building or I/O. The current architecture forces every network call — including the health check that runs every 15 seconds — to serialize through the main thread.

Scope

  • Remove @MainActor from GatewayHTTPClient (the central HTTP client)
  • Remove @MainActor from all 40+ network client structs that inherit this isolation solely because they call GatewayHTTPClient
  • Keep @MainActor only where genuinely needed (e.g., GatewayConnectionManager which is an ObservableObject driving UI)
  • Ensure await MainActor.run {} is used only for the final UI-state assignments

Acceptance Criteria

  • GatewayHTTPClient.buildRequest(), resolveConnection(), and all HTTP methods (get, post, put, delete, patch) execute off the main thread
  • Network client structs (MessageClient, ConversationClient, HealthCheckClient, etc.) no longer require @MainActor
  • All existing tests pass
  • No new warnings in strict concurrency mode

Summary: Remove inappropriate @MainActor isolation from the shared networking layer so request building and HTTP I/O do not execute/serialize on the main thread.

Key Context:

  • Current network layer has 46 files and 99 @MainActor annotations, pushing URL construction/response handling onto the main thread.
  • This affects all network calls, including a health check that runs every ~15 seconds, causing unnecessary main-thread contention.
  • Targeted changes: de-isolate GatewayHTTPClient and downstream client structs; keep @MainActor only where UI state is mutated and use MainActor.run {} only for final assignments.

Generated by Linear Issue Context Agent

devin-ai-integration[bot]

This comment was marked as resolved.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

…ctor-isolated AuthService.baseURL

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
@devin-ai-integration devin-ai-integration Bot changed the title Remove @MainActor from network layer (LUM-492) Move network layer off @MainActor isolation Mar 26, 2026
@ashleeradka ashleeradka merged commit dd23f7a into main Mar 26, 2026
4 checks passed
@ashleeradka ashleeradka deleted the devin/lum-492-1774540425 branch March 26, 2026 16:23
devin-ai-integration Bot added a commit that referenced this pull request Mar 26, 2026
ashleeradka pushed a commit that referenced this pull request Mar 26, 2026
This reverts commit dd23f7a.

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
devin-ai-integration Bot added a commit that referenced this pull request Mar 26, 2026
Remove @mainactor isolation from GatewayHTTPClient, HealthCheckClient,
and all ~40 stateless network client protocols/structs. This moves HTTP
request construction, URL resolution, and synchronous file I/O off the
main thread onto the cooperative thread pool.

Unlike the previous attempt (#21695), resolveConnection() stays
synchronous. The original PR made it async to safely read
AuthService.shared.baseURL via await, but the resulting actor-hopping
(MainActor → cooperative pool → MainActor → cooperative pool) during the
startup health-check path caused the app to hang immediately on launch.

The fix: use if/else instead of the ?? operator for the AuthService
fallback. The ?? operator wraps its right operand in an autoclosure,
which is an error for @MainActor-isolated property access. Direct access
in a nonisolated function body is a Swift 5 warning (safe at runtime
since String is a value type and the underlying reads are thread-safe).

What changes:
- @mainactor removed from 43 network client files
- resolveConnection() remains synchronous (no async, no actor-hopping)
- isConnectionManaged(), buildURL() remain synchronous
- handleAuthenticationFailure() remains synchronous
- BtwClient Task keeps @mainactor annotation
- EventStreamClient and GatewayConnectionManager retain @mainactor

Refs: LUM-492
Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
ashleeradka added a commit that referenced this pull request Mar 26, 2026
* Remove @mainactor from network layer, keep resolveConnection synchronous

Remove @mainactor isolation from GatewayHTTPClient, HealthCheckClient,
and all ~40 stateless network client protocols/structs. This moves HTTP
request construction, URL resolution, and synchronous file I/O off the
main thread onto the cooperative thread pool.

Unlike the previous attempt (#21695), resolveConnection() stays
synchronous. The original PR made it async to safely read
AuthService.shared.baseURL via await, but the resulting actor-hopping
(MainActor → cooperative pool → MainActor → cooperative pool) during the
startup health-check path caused the app to hang immediately on launch.

The fix: use if/else instead of the ?? operator for the AuthService
fallback. The ?? operator wraps its right operand in an autoclosure,
which is an error for @MainActor-isolated property access. Direct access
in a nonisolated function body is a Swift 5 warning (safe at runtime
since String is a value type and the underlying reads are thread-safe).

What changes:
- @mainactor removed from 43 network client files
- resolveConnection() remains synchronous (no async, no actor-hopping)
- isConnectionManaged(), buildURL() remain synchronous
- handleAuthenticationFailure() remains synchronous
- BtwClient Task keeps @mainactor annotation
- EventStreamClient and GatewayConnectionManager retain @mainactor

Refs: LUM-492
Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>

* fix: make AuthService.configuredBaseURL thread-safe for nonisolated access

Move configuredBaseURL storage to a module-level lock-protected variable
so GatewayHTTPClient.resolveConnection() can read it without crossing
into @mainactor isolation.

- Add NSLock-protected module-level storage for configuredBaseURL
- Add nonisolated AuthService.currentConfiguredBaseURL static accessor
- Mark resolveBaseURL/normalizedBaseURL as nonisolated (pure functions)
- Replace AuthService.shared.baseURL with direct call to nonisolated
  AuthService.resolveBaseURL in GatewayHTTPClient.resolveConnection()

Fixes Swift 6 compile error: main actor-isolated property 'baseURL'
cannot be referenced from a nonisolated context.

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: ashlee@vellum.ai <ashlee@vellum.ai>
devin-ai-integration Bot added a commit that referenced this pull request Apr 16, 2026
The 15-second health check loop previously ran as a Task { @mainactor ... }
so the while-loop, Task.sleep scheduling, and error handling all occupied
the main actor. Under memory pressure this periodic work contributes to
app hangs (LUM-914 recorded a ≥2000ms hang attributed to this loop on a
device with ~131MB free RAM).

Change the loop to a detached task at .utility priority and hop to
@mainactor only for the state reads (isUpdateInProgress,
healthCheckInterval, shouldReconnect) and the update-timeout cleanup
that mutates @published properties. performHealthCheck() itself stays
@mainactor so all @published writes and cached-assistant reads remain on
main — no new data races, no changes to resolveConnection() or the rest
of the network stack that triggered the earlier revert of #21695.

Refs: WWDC25 Embracing Swift concurrency
(https://developer.apple.com/videos/play/wwdc2025/268/),
Task.detached(priority:), MainActor.run.

Closes LUM-916

Co-Authored-By: tkheyfets <timur@vellum.ai>
devin-ai-integration Bot added a commit that referenced this pull request Apr 16, 2026
The 15-second health check loop previously ran as a Task { @mainactor ... }
so the while-loop, Task.sleep scheduling, and error handling all occupied
the main actor. Under memory pressure this periodic work contributes to
app hangs (LUM-914 recorded a ≥2000ms hang attributed to this loop on a
device with ~131MB free RAM).

Change the loop to a detached task at .utility priority and hop to
@mainactor only for the state reads (isUpdateInProgress,
healthCheckInterval, shouldReconnect) and the update-timeout cleanup
that mutates @published properties. performHealthCheck() itself stays
@mainactor so all @published writes and cached-assistant reads remain on
main — no new data races, no changes to resolveConnection() or the rest
of the network stack that triggered the earlier revert of #21695.

Refs: WWDC25 Embracing Swift concurrency
(https://developer.apple.com/videos/play/wwdc2025/268/),
Task.detached(priority:), MainActor.run.

Closes LUM-916

Co-Authored-By: tkheyfets <timur@vellum.ai>
tkheyfets added a commit that referenced this pull request Apr 17, 2026
…ctor (#26082)

* Run GatewayConnectionManager health check loop off @mainactor

The 15-second health check loop previously ran as a Task { @mainactor ... }
so the while-loop, Task.sleep scheduling, and error handling all occupied
the main actor. Under memory pressure this periodic work contributes to
app hangs (LUM-914 recorded a ≥2000ms hang attributed to this loop on a
device with ~131MB free RAM).

Change the loop to a detached task at .utility priority and hop to
@mainactor only for the state reads (isUpdateInProgress,
healthCheckInterval, shouldReconnect) and the update-timeout cleanup
that mutates @published properties. performHealthCheck() itself stays
@mainactor so all @published writes and cached-assistant reads remain on
main — no new data races, no changes to resolveConnection() or the rest
of the network stack that triggered the earlier revert of #21695.

Refs: WWDC25 Embracing Swift concurrency
(https://developer.apple.com/videos/play/wwdc2025/268/),
Task.detached(priority:), MainActor.run.

Closes LUM-916

Co-Authored-By: tkheyfets <timur@vellum.ai>

* Update comments to reflect @observable migration from #25496

Co-Authored-By: tkheyfets <timur@vellum.ai>

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: tkheyfets <timur@vellum.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant