Skip to content

fix(swift-ios): release accepted commands before optional refreshes - #10758

Open
saphid wants to merge 313 commits into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-send-ack-20260908
Open

saphid wants to merge 313 commits into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-send-ack-20260908

Conversation

@saphid

@saphid saphid commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Review this layer

This is the first PR in the chain, so the diff GitHub shows here is this layer alone.

This layer's diff: +992 / −29 lines across 4 files; app code +150 / −24, the rest is tests and docs.

Accepted Send and Stop commands keep their UI actions waiting for optional HTTP reads. Release the accepted action immediately and let a generation-owned refresh reconcile the thread separately. Coalesce overlapping Send/Stop refresh requirements without replaying the command.

Pre-restack CI verification: GitHub checks passed on the original head, including contract fixtures and native tests. Skipped checks are not claimed as executed proof.

Before this restack, all 12 standalone command-acknowledgement/retry XCTest cases passed (exit 0). The old-environment late-ack regression relies on the later live-bootstrap connection lifetime and remains covered in the dependent chain.

Delivery: first of the stacked thread-sync chain (#10758 → #10759 → #10761 → #10762 → #10763 → #10764 → #10765 → #10766 → #14021 → #10767); merges on its own.

Base: Theo’s t3code/rebuild-mobile-app-swift, 157476f1fb.

Historical verification scope (before this restack): Native tests passed with optional HTTP reads held while accepted Send/Stop completes. They verify outbox completion, refresh coalescing, cancellation on close/disconnect, and absence of command replay. The direct PR excludes a test requiring the later passive-live lifetime; that test remains in the dependent chain. The native CI job linked above contains the passing receipts. The tests exercise the protocol, state, or accessibility-value boundary changed here.

Refresh onto 157476f1fb (2026-09-27)

Rebased with the thread-sync chain onto the current t3code/rebuild-mobile-app-swift (157476f1fb). Head 93fe314002.

  • Focused tests at this head (iOS Simulator, per-test time limits): NativeRetryIdentityTests 14 passed; before the delete fix, NativeMultiEnvironmentTests, NativeThreadCatchUpTests, NativeRetryIdentityTests and FeatureRootModelTests passed 78 tests.
  • Full T3CodeTests at the chain top (68d92c2752): 343 XCTest cases passed (1 skipped). Swift Testing ran 892 tests; the only 2 failures are UsageModelsTests currency formatting, which fail identically on unmodified 157476f1fb.
  • Independent review: devin -p --sandbox --permission-mode smart --model swe-2-max over 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, because stopSurvivesCloseReopenAndMissingReconnectSeed requires that behaviour.
  • Review fix: Macroscope noted that deleting a thread did not cancel its detached refresh, so a late read could publish the deleted thread again. deleteThread now cancels it once the server accepts the delete (testDeletingThreadCancelsItsAcceptedSendRefresh; without the fix the test hangs). NativeRetryIdentityTests: 14 passed.

Review fixes (rounds 2–4)

  • A detail read that started before a newer send was accepted could publish after it and hide the newer message. A superseded read is no longer published (its refresh loop reads again), and a cancelled shell refresh can no longer write its result. Test: testSupersededAcceptedSendRefreshDoesNotPublishItsOlderDetail (fails without the fix).
  • A Stop accepted during a send's detail read discarded that read without requesting another, and an older detail could replace the transcript before it included the accepted message. The discarded read now triggers a re-read, and delivered messages stay visible until a server transcript includes them. Test: deliveredMessageSurvivesAnOlderDetailUntilTheServerIncludesIt.
  • Retained messages are released once a server transcript confirms them, so they no longer reappear after a rewind, and are dropped when thread details are removed or cleared. Tests: confirmedDeliveredMessageDoesNotReturnAfterARewind, clearedDetailsDropRetainedDeliveredMessages.

Review fixes (round 6: Macroscope and Astra)

  • Evicting a cached thread detail forgot its accepted messages, so reopening it with a stale transcript hid them. Eviction now keeps them, and clears only the transcript ID cache so memory stays bounded. Test: evictedDetailKeepsItsDeliveredMessageOnReopen (fails without the fix).
  • A restored outbox or a new task's local detail was recorded as the server transcript, so an accepted new-task prompt was treated as confirmed and disappeared on the next older read. Only server transcripts confirm delivery now. Test: startedTaskKeepsItsPromptUntilTheServerIncludesIt (fails without the fix).
  • Astra xhigh re-checked these fixes: it found one medium (eviction kept the transcript ID cache, so it could grow without bound), which is fixed above; the re-check of that fix returned ready.

Review fixes (round 7: Macroscope and Astra)

  • A newer send can be delivered while an older one waits to retry. Delivered copies were placed before every queued copy, which reversed them; both are now merged by send time. Test: localCopiesKeepSendOrderWhenANewerSendIsDeliveredFirst (fails without the fix). Astra found no issues with this change.

Independent review

GPT-6 Astra (codex exec -m gpt-6-astra -c model_reasoning_effort="xhigh" --sandbox read-only) reviewed this layer four times. Rounds 1–3 found the defects fixed above. Round 4 returned ready to merge.

Follow-ups (minor, not blocking)

  • An accepted send's in-flight detail refresh can publish after another client deleted the thread and briefly re-add it. Upstream's awaited refresh has the same race; cancelling accepted-command refreshes on shell removal would close it (Macroscope).

Reproduction

iOS 27 Simulator, Debug builds, paired to a disposable local server through a fault-injecting proxy that never answers GET /api/orchestration/threads/* or /api/orchestration/shell, like a stalled mobile connection. Commands still go through. Both builds include #12655 so the composer can be focused. Send one message, then a second.

  • Before (157476f1fb): the first send holds the composer until its refresh reads time out (8 s thread read, then 61 s shell read). The second message's send button stays disabled for about 70 s.
  • After (this PR): the second message sends immediately while the reads are still stalled, and its reply arrives.

Before: second message blocked while refresh reads stall (4x speed) After: second message sends while reads stall (4x speed)

GIFs are 4× speed. Real-time recordings: before.mp4 · after.mp4.

Model and harness: GPT-6 / Codex. Rebase onto 157476f1fb: Claude Opus 5.5 / Claude Code.

t3dotgg and others added 30 commits August 18, 2026 14:32
- Cache EnvironmentStore's decoded document; publishes read it several
  times per second and previously hit disk + JSON decode every time
- Memoize parsed ISO8601 dates in NativeFeatureClient; every publish
  re-parsed ~10 timestamps per thread through two formatters
- Cache the mapped provider catalog per environment, invalidated on
  server config writes, instead of rebuilding hundreds of structs per
  publish
- Sort activity logs by raw ISO string instead of parsing dates inside
  the comparator (O(n log n) formatter calls per detail rebuild)
- Skip attachment hydration entirely for text-only threads
- Dedupe emitConnection so per-event yields only publish transitions
- Early-out acknowledgeDeliveredMessages when the outbox is empty
- Cap terminal buffers at 256KB; unbounded append could OOM on verbose
  commands and re-layout megabyte strings per chunk
- Key the markdown document cache by content fingerprint instead of the
  whole source string, stop inserting streaming intermediates (which
  churned completed messages out), and promote the final streamed render
  on completion instead of reparsing on the main thread
- Throttle streaming markdown renders (150ms leading-edge) instead of a
  200ms trailing debounce that never fired at an 80ms publish cadence,
  which left streaming messages as plain text for whole turns
- Share JSONEncoder/Decoder.t3 instances (were rebuilt per call) and
  skip sortedKeys on throwaway JSONValue bridging encodes
- Weak-capture the RPC connection loop and keepalive so released
  clients can actually deinit; snapshot dictionary keys before
  reentrant iteration in connected(); scope subscription Interrupts to
  the connection that assigned the request ID; add reconnect jitter
- Cache DPoP JWK + thumbprint (SHA-256 per managed request before) and
  the proof encoder; reuse ISO8601 formatter in command builders
- Replace crash-prone Dictionary(uniqueKeysWithValues:) on
  server-supplied IDs with first-wins reduces (model picker, agent
  awareness); retain per-thread states instead of full FeatureThread
  copies for transition signals; idle the home list timer to 60s when
  nothing is working

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Move table column-width estimation into the background render pass
  and store it on MarkdownRenderedTable; the view measured every cell's
  AttributedString per body evaluation on the main thread
- Make rendered markdown blocks Equatable (inline runs compare by
  reference thanks to the streaming run cache) and wrap block views in
  .equatable(), so only the changed tail of a streaming message
  re-renders instead of every block
- Gate the thread header's per-second TimelineView on working status;
  idle threads render a static status
- Decode local attachment previews off-main through the shared
  thumbnail cache; UIImage(data:) ran in body per reconfigure
- Memoize composer trigger parsing (was parsed 4x per keystroke)
- Drop markdown caches on memory warnings
- Stop the QR capture session on viewDidDisappear/backgrounding and
  resume on reappear; dismantle was the only stop path
- Retry pending workspace navigation requests when snapshot data lands
  so cold-start deep links are not silently dropped
- Scope approval/user-input resolution to details that contain the
  request instead of deep-comparing every cached transcript
- Binary-search diff hydration anchors (was O(n^2) on large files)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Move streaming markdown rendering out of .task(id:) into a renderer
  that survives revision changes; the per-revision task cancelled any
  in-flight parse, so messages that parse slower than the publish
  cadence never rendered. One render runs at a time, latest revision
  wins, 150ms throttle. Keep showing the previous streamed document
  between renders instead of flashing back to plain text.
- Cap the terminal buffer in UTF-8 bytes (the unit the limit is defined
  in) instead of Character count, snapping to a character boundary.
- Start the home list timer after items are populated so an initially
  working thread gets the 1Hz interval; re-evaluate on every update.
- Sort activities by memoized parsed dates (stable, wire-order ties);
  raw ISO strings sort wrong across mixed fractional representations.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A cancelled drain unwinding after a replacement started could clear the
replacement's task slot, allowing two concurrent drains to deliver out
of order. Generation-stamp each drain; only the current generation may
clear the shared slot, loop, or deliver.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The stale-document fallback kept any revision-mismatched render while
streaming, so a reused transcript cell whose @State survived recycling
could briefly show another message's markdown. Only accept a stale
document whose source is a prefix of the current message.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@saphid
saphid force-pushed the pr/swiftui-send-ack-20260908 branch from 9543d11 to 220aa67 Compare September 26, 2026 23:46
Comment thread apps/swift-ios/App/NativeFeatureClient.swift Outdated
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>
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 27, 2026
github-actions Bot and others added 3 commits September 27, 2026 18:36
…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>
Comment thread apps/swift-ios/Features/Root/FeatureRootModel.swift Outdated
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>
@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). and removed size:XL 500-999 changed lines (additions + deletions). labels Sep 27, 2026
Comment thread apps/swift-ios/Features/Root/FeatureRootModel.swift
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>
@saphid

saphid commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

@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 93fe314002:

  • CI: all checks pass on this head.
  • Full T3CodeTests at the chain top (70258b6a6b, iOS Simulator): 343 XCTest cases passed (1 skipped). Swift Testing ran 892 tests; the only 2 failures are UsageModelsTests currency formatting, which fail identically on unmodified 157476f1fb.
  • Each review fix has a focused regression test that fails without the fix, except where the description says why not.
  • Simulator reproduction with stalled reads: on 157476f1fb the second message's send button stayed disabled for about 70 s; with this PR it sends immediately (before/after video in the description).
  • An earlier build of this chain (before the review rounds) fixed the stuck-send behaviour on a physical iPhone, as confirmed by the user. The current heads have not been re-verified on a device.

Independent review: GPT-6 Astra xhigh (read-only) reviewed this layer in four rounds and returned ready to merge in round 4. It then re-checked the later fixes: the eviction/local-detail fix (one medium found and fixed, re-check ready) and the send-order fix (no issues).

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.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 1, 2026 — with ChatGPT Codex Connector
@t3dotgg
t3dotgg force-pushed the t3code/rebuild-mobile-app-swift branch 2 times, most recently from 1637f70 to cf79028 Compare October 1, 2026 21:36

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:XXL 1,000+ changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants