Conversation
|
Macroscope has since reviewed this pull request. An earlier review was skipped by a cost limit; a review has now completed, so that notice no longer applies. |
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. |
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This is a large Swift iOS production change spanning transport synchronization, transcript rendering, navigation, and new receipt/stop behavior. Unresolved findings identify stale tool activity, lost attachment previews, and incorrect historical work-log rows, so the runtime impact warrants 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. |
9eab047 to
672cd13
Compare
| ordered[ordered.count - 1] = mutation | ||
| } else { ordered.append(mutation) } | ||
| case .activity: | ||
| ordered.append(mutation) |
There was a problem hiding this comment.
🟡 Medium App/NativeFeatureClient.swift:8157
A replayed .activity with the same ID is appended instead of replacing the earlier mutation, so NativeTranscriptTimeline accepts the first event and permanently displays stale tool activity until a full rebuild. Deduplicate .activity mutations by activity ID and replace the existing entry with the latest contents, as is done for .message.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 8157:
A replayed `.activity` with the same ID is appended instead of replacing the earlier mutation, so `NativeTranscriptTimeline` accepts the first event and permanently displays stale tool activity until a full rebuild. Deduplicate `.activity` mutations by activity ID and replace the existing entry with the latest contents, as is done for `.message`.
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>
672cd13 to
2ea99ef
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>
2ea99ef to
0556009
Compare
0556009 to
de53f2d
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>
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>
e841e28 to
dd1720b
Compare
| // Older pages contain earlier turn windows, but carry current session | ||
| // metadata. That metadata must not revive their historical tool rows. | ||
| return NativeTranscriptTimeline( | ||
| messages: messages, activities: thread.activities, sessionIsLive: false |
There was a problem hiding this comment.
🟡 Medium App/NativeFeatureClient.swift:7061
Loading an older page adds stale work-log rows for in-progress historical tool activity. sessionIsLive: false causes NativeTranscriptTimeline to call finishActiveWork(), which materializes unfinished entries instead of discarding them; use a history-specific path that drops unfinished tool entries without reviving them.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 7061:
Loading an older page adds stale work-log rows for in-progress historical tool activity. `sessionIsLive: false` causes `NativeTranscriptTimeline` to call `finishActiveWork()`, which materializes unfinished entries instead of discarding them; use a history-specific path that drops unfinished tool entries without reviving them.
There was a problem hiding this comment.
Acknowledged: older pages can materialise work-log rows for tool activity that was still running in that history. Listed as a follow-up in the PR body rather than changed in this stack.
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 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>
…ismissed early Reopening a thread in the compact split view can report the new thread view as disappeared right after it appears while it stays on screen. That cancelled the draft restore, so the composer showed a busy send button until relaunch. Restore outside the view task's lifetime. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…cted Pairing rejection cancelled the polling tasks but left the reconciliation loop issuing rejected shell reads every 30 seconds. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A passive peer whose socket stayed open kept its 30-second quiet shell reads after a 401, and showed as connected. Handle the rejected credential before the live-stream shortcut and switch to the long back-off. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The open socket's updates marked a rejected peer connected again, and stream repairs woke its HTTP loop before the back-off. Keep the rejection in the peer's shared state. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The live-peer fixture this test needs now lands below the Stop layer, so the regression moved out of the command-acknowledgement layer returns here.
Stop status refreshes the shell cache and reads it back. A live turn advancing the shell during that read made the refresh discard itself as superseded, so the status check failed and Stop could not be confirmed or retried. While the environment's stream is authoritative on the current socket, the cache it keeps current answers instead; after a disconnect or lost authority the check still fails. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
dd1720b to
419e4eb
Compare
|
@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. |
419e4eb to
e54dd91
Compare
|
@t3dotgg heads-up: #10766 had two concerns, so it's now split.
New merge order: #10758 → #10759 → #10761 → #10762 → #10763 → #10764 → #10765 → #10766 → #14021 → #10767. #12655 stays independent. Each PR's GitHub diff includes the whole chain below it, so every description now starts with a link to that layer's own change:
|
|
Note This comment is posted by Julius' dot Preserving tool chronology and adding a thread receipt timestamp solve separate problems. The chronology repair does not need the new header receipt display. Under the one problem rule, please split the receipt feature from the ordering fix and establish its scope with maintainers before requesting reconsideration. |
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: +807 / −330 lines across 15 files; app code +474 / −321, the rest is tests and docs.
Keep tool observations in chronological order across intervening assistant text, and record when this device last received an update for the open thread. The thread header shows that receipt time with its own connection label, so Connected is not mistaken for a new message.
Delivery: stacked. This branch contains #10758, #10759, #10761, #10762, #10763, #10764, #10765; 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).
Split (2026-09-28): this PR originally also replaced the header receipt line with the age of the last message. That change now lives in #14021, stacked directly on this PR. This PR adds the receipt line; #14021 replaces it with the message age.
Chronological tool ordering: newly captured against prerequisite #10765 at
5e0055bdcand4434db280(the current header/activity changes do not affect this idle, receipt-free transcript fixture). The same synthetic wire snapshot contains Read 0, assistant text, then Read 2 for the same tool call. Actual NativeFeatureClient mapping and ThreadDetailView show the baseline replacing the earlier work row with Read 2 before the text; the candidate retains all three rows in order. Both capture tests passed (1 test each, exit 0). Fixed 1 January 2026 timestamps are fixture values. This is a static snapshot comparison, not an incremental-animation or latency measurement.Full before screenshot · Full after screenshot · Capture and edit receipt
Independent Claude review was unavailable:
claude auth statusreturned exit 1 withloggedIn: false. Visual inspection is the author’s assessment, not maintainer approval.Refresh onto
157476f1fb(2026-09-27)Rebased with the thread-sync chain onto the current
t3code/rebuild-mobile-app-swift(157476f1fb). Heade54dd91a82.Focused tests at this layer, measured before the review fixes and the split below (
NativeMultiEnvironmentTests,NativeThreadCatchUpTests,NativeRetryIdentityTests,FeatureRootModelTests, iOS Simulator, per-test time limits): 119 passed.Full
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.Independent review:
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.Split check: this layer now compiles on its own; the header receipt view needed a case for the upstream Needs pairing connection state, which the removed age commits had masked by deleting that view. Full
T3CodeTestsat this head (e54dd91a82): 343 XCTest cases passed; Swift Testing failures only the 2UsageModelsTestscurrency tests.Follow-ups (minor, not blocking)
Model and harness: GPT-6 / Codex. Rebase onto
157476f1fb: Claude Opus 5.5 / Claude Code.