Conversation
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR is a large synchronization and lifecycle refactor that changes active/passive environment hydration, WebSocket ownership, optimistic message handling, and selected-thread recovery across existing iOS paths. Its production blast radius and unresolved medium-severity findings require human review. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
@t3dotgg Could you review this SwiftUI reliability/performance fix? It will reconcile selected-thread streams that silently stop progressing. It does not change the visible interface. This is a dependent PR; prerequisites: #10761, #10758, #10759. Its diff includes those prerequisites and needs rebasing after they land. |
5948002 to
3c7e703
Compare
A detached accepted-command refresh could finish after deleteThread and publish the deleted thread again. Cancel it once the server accepts the delete. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
3c7e703 to
0320c17
Compare
…ng older state A detail read that started before a newer send was accepted could publish after it and hide the newer message until the next read. Skip publishing a superseded read (its refresh loop reads again), and stop a cancelled shell refresh from writing a shell read before the change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ives A Stop accepted during a send's detail read discarded that read without requesting another, and an older detail could replace the transcript before the accepted message reached it. Re-read detail after a discarded read, and keep delivered messages until a server transcript includes them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Do not retain a delivered message that a server transcript already confirmed, keep only its display copy, and drop retained messages when their thread details are removed or cleared. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
0320c17 to
2c4dea1
Compare
Evicting a cached thread detail forgot its accepted messages, so a stale reopen hid them. A restored outbox or a new task's local detail was also recorded as the server transcript, so its accepted prompt was treated as confirmed and disappeared on the next older read. Only server transcripts now confirm delivery, and eviction keeps retained messages. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2c4dea1 to
504b913
Compare
A newer send can be delivered while an older one waits to retry. Delivered copies were placed before every queued copy, which reversed them. Merge both by send time. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A transient environment catalog read failure ended the active client's configuration subscription, so later provider and settings updates were ignored. Skip that publish instead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e probe A passive peer's catalogue probe started at bootstrap could finish after the user saved a preference there and replace it with the older value. Discard the probe when a config already arrived. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
504b913 to
94d7a16
Compare
A peer whose credential was rejected before its catalogue loaded kept opening a probe client every 20 seconds. The probe's own error cannot show the rejection (the socket ticket 401 surfaces as a timeout), so stop once the peer's shell read has marked it as needing pairing; re-pairing starts a new worker. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
94d7a16 to
8ef7e74
Compare
| func markStreamPaused(_ environment: Environment) { | ||
| guard let owner, owns(environment) else { return } | ||
| owner.shellConnectionIDsByEnvironmentID[environment.id] = nil | ||
| owner.environmentConnectionStates[environment.id] = .reconnecting |
There was a problem hiding this comment.
🟡 Medium App/NativeFeatureClient.swift:4506
markStreamPaused changes a passive environment from .needsPairing back to .reconnecting when the concurrent followShell subscription fails. After refreshShell marks rejected credentials with credentialRejected: true, this generic recovery path hides the pairing state in Settings and continues retrying the unusable credential. Preserve .needsPairing and stop the worker's recovery path instead of calling requestRepair for that environment.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 4506:
`markStreamPaused` changes a passive environment from `.needsPairing` back to `.reconnecting` when the concurrent `followShell` subscription fails. After `refreshShell` marks rejected credentials with `credentialRejected: true`, this generic recovery path hides the pairing state in Settings and continues retrying the unusable credential. Preserve `.needsPairing` and stop the worker's recovery path instead of calling `requestRepair` for that environment.
There was a problem hiding this comment.
Fixed further up the stack in #10764 (fix(swift-ios): share a peer's credential rejection with its live stream): markStreamPaused keeps Needs pairing for a rejected peer and a new stream snapshot no longer wakes the HTTP loop (test livePeerRejectedCredentialStopsReconciling). The chain merges in order.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
| ) | ||
| } | ||
| // A failed catalog read skips this publish; later events still apply. | ||
| guard let environments = try? await self.runtime.environments() else { continue } |
There was a problem hiding this comment.
🟡 Medium App/NativeFeatureClient.swift:4245
A transient failure from runtime.environments() causes this configuration event to skip publish, so the updated providers or settings remain cached but no FeatureSnapshot reaches the UI. Unlike the previous emitSnapshot path, the continue leaves the configuration stream idle until an unrelated later event publishes a snapshot; retain a fallback publication path when the environment lookup fails.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 4245:
A transient failure from `runtime.environments()` causes this configuration event to skip `publish`, so the updated providers or settings remain cached but no `FeatureSnapshot` reaches the UI. Unlike the previous `emitSnapshot` path, the `continue` leaves the configuration stream idle until an unrelated later event publishes a snapshot; retain a fallback publication path when the environment lookup fails.
There was a problem hiding this comment.
Intentional trade-off from the #10761 fix: previously a failed catalog read ended the whole configuration subscription. Now it skips one publish, and the cached config reaches the UI on the next snapshot publish. Listed as a follow-up (retry that publish).
| flushDetailPublish(route) | ||
| activeRawThread = nil | ||
| activeThreadSequence = sequence | ||
| scheduleDetailRefresh(threadID: threadID, client: route.client, force: true) |
There was a problem hiding this comment.
🟡 Medium App/NativeFeatureClient.swift:6109
When a shell completion exposes a missing detail event after the user loaded older pages, this recovery drops those pages and resets the pagination window. repairMissingCompletedTurn clears activeRawThread and calls the ordinary forced refreshThread path, which uses only initialThreadUserTurnLimit and replaces activeThreadPage; schedule this through the reconciliation path so the loaded history extent is retained.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 6109:
When a shell completion exposes a missing detail event after the user loaded older pages, this recovery drops those pages and resets the pagination window. `repairMissingCompletedTurn` clears `activeRawThread` and calls the ordinary forced `refreshThread` path, which uses only `initialThreadUserTurnLimit` and replaces `activeThreadPage`; schedule this through the reconciliation path so the loaded history extent is retained.
There was a problem hiding this comment.
Acknowledged: the missing-reply repair (#10759) reloads the initial page, so older pages loaded before it are dropped and can be loaded again. Listed as a follow-up.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
|
@t3dotgg this is ready for your review and merge. Merge order: stacked chain, merge in order: #10758 → #10759 → #10761 → #10762 → #10763 → #10764 → #10765 → #10766 → #10767. #12655 (composer focus freeze) is independent and can merge at any time. Evidence at head
Independent review: No layer-specific Astra review. Covered by the SWE-2 Max review of the whole chain (exit 0, no blocker/high/medium findings). Macroscope findings on the latest heads have been verified against the source: real ones are fixed, and the rest have a written reason and are listed under Follow-ups in the description. |
|
Note This comment is posted by Julius' dot The new 30-second HTTP reconciliation loop changes recovery ownership and how authoritative history replaces cached pages. This substantial recovery change has no linked maintainer-triaged issue or approved scope. Please establish that direction under the prior approval rule, then request reconsideration. |
2 participants
Review this layer
GitHub shows this PR against
t3code/rebuild-mobile-app-swift, so its diff includes every earlier PR in the chain. Review only this layer's change:This layer's diff: +1257 / −42 lines across 3 files; app code +192 / −27, the rest is tests and docs.
A healthy socket can stop delivering selected-thread events. Reconcile only the foreground selected transcript after 30 seconds without actual detail progress, defer to existing recovery owners, and validate selection and generation before applying results. Retain loaded messages, activities and checkpoints when a capped recovery page cannot cover them; use authoritative fallback rather than unioning stale cached history.
Verification: current-head CI passes, including contract fixtures and native tests. The final bootstrap/environment run passed 38 tests, covering environment switching, error visibility and archive hydration; separate focused checks cover passive-worker lifetime. Skipped checks are not claimed as executed proof.
Delivery: stacked. This branch contains #10758, #10759, #10761, #10762; merge the chain in order (#10758 → #10759 → #10761 → #10762 → #10763 → #10764 → #10765 → #10766 → #14021 → #10767). Each PR's own change is its top commit(s).
Base: Theo’s
t3code/rebuild-mobile-app-swift,157476f1fb.Verification scope: Native selected-reconciliation tests passed in CI before the repair and in the combined local verification, including the actual default deadline (30.080 seconds), progress postponement, cancellation in background, navigation rejection of held responses, and authoritative recovery of truncated history. The measured deadline is from the CI Simulator fixture, not a phone responsiveness benchmark. The native CI job linked above contains the passing receipts. The tests exercise the protocol, state, or accessibility-value boundary changed here.
Independent cross-provider review was unavailable:
claude auth statusreturned exit 1 withloggedIn: false. No Claude review or maintainer approval is claimed.Refresh onto
157476f1fb(2026-09-27)Rebased with the thread-sync chain onto the current
t3code/rebuild-mobile-app-swift(157476f1fb). Head8ef7e74c91.NativeMultiEnvironmentTests,NativeThreadCatchUpTests,NativeRetryIdentityTests,FeatureRootModelTests, iOS Simulator, per-test time limits): 110 passed.T3CodeTestsat the chain top (68d92c2752): 343 XCTest cases passed (1 skipped). Swift Testing ran 892 tests; the only 2 failures areUsageModelsTestscurrency formatting, which fail identically on unmodified157476f1fb.devin -p --sandbox --permission-mode smart --model swe-2-maxover the whole chain diff, exit 0. No blocker, high or medium findings. A dead-state finding was fixed. A report that Stop state outlives a vanished thread was not changed, becausestopSurvivesCloseReopenAndMissingReconnectSeedrequires that behaviour.session.status, which upstream uses as the only source of the working state.Follow-ups (minor, not blocking)
Model and harness: GPT-6 / Codex. Rebase onto
157476f1fb: Claude Opus 5.5 / Claude Code.