Conversation
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The production refresh subsystem is substantially reworked from one aggregate loop into per-environment workers with new cancellation, topology reconciliation, snapshot publication, and rejected-credential behavior. This changes existing network and state-management behavior across passive environments and warrants human review. You can add or adjust custom eligibility rules. Learn more. |
|
@t3dotgg Could you review this SwiftUI reliability/performance fix? It will refresh passive environments independently so one stalled peer does not block healthy peers. It does not change the visible interface. This can be reviewed independently against your SwiftUI branch. |
Per-environment row reuse could republish stale active rows during the shell publish debounce; the projection cache already avoids remapping. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eers Restore the specific failure text for unreachable peers. While a peer's pairing is rejected, the catalogue loop waits instead of probing: each probe mints a WebSocket ticket, and the rejection only surfaces through the shell refresh, not the RPC call.
aea364a to
2691d2f
Compare
|
Note This comment is posted by Julius' dot Closing under the prior approval rule. The slow-peer stall is documented and covered by focused tests. This fix replaces the aggregate refresh lifecycle with separate shell and catalogue workers, topology reconciliation, and cancellation. A substantial bug fix needs a maintainer-triaged issue establishing the failure and intended behavior; this PR, #5178, and #10761 provide no such decision. Please get that maintainer decision, agree on the scope, link it here, and request reconsideration. |
A slow or unreachable passive environment (a paired computer that isn't the active one) held up refreshes for every other passive environment, because the aggregate loop fetched all passive shells in one batch and waited for the slowest.
Each enabled passive environment now gets its own bounded refresh worker (shell poll plus a one-shot catalogue probe). A topology loop reconciles workers against the saved environments. Workers are cancelled when their environment is removed, disabled, edited, or becomes active, or when the owning session changes, and they check membership again after every suspension so a late response can't bring back a removed row. Cached rows stay visible while a peer is unreachable, keeping its failure detail, and failures back off to the configured failure interval without slowing healthy peers. A peer whose pairing was rejected stops probing its catalogue (each probe mints a WebSocket ticket) until it is paired again. The managed connection is published before passive refresh starts.
This PR is stacked on
t3code/rebuild-mobile-app-swift(#5178): the SwiftUI client only exists on that branch, so this targets it rather thanmain. Rebased onto the base tip157476f1fb.No UI changes, so there's no before/after media.
Verification
2691d2fd5a(isolated DerivedData):NativeMultiEnvironmentTests33 passed andNativePassiveThreadRefreshTests7 passed, including new cases for failure-backoff isolation, healthy publishes while a peer shell and catalogue are held, stale-generation rejection, removal cancelling a held peer, and a rejected peer minting no catalogue tickets. That last test times out against the previous code.devin -p --model swe-2-max, exit 0. No correctness defects. Its low-severity notes (the catalogue's own failure retry usesTask.sleeprather than the injected per-peer sleep; one snapshot build per peer wake, deduplicated on publish) were left as they are.Coordination trace: T3 thread 31463569-aace-46fd-a6e8-1a56ad2a35a7
Model and harness: GPT-6 / Codex (original), Claude Opus 5 / Claude Code (refresh and simplification), Claude Opus 5.5 / Claude Code (rebase onto
157476f1fband rejected-peer fix), SWE-2 High / Cursor via T3 Code (base verification, independent review, and PR upkeep).🤖 Generated with Claude Code