Skip to content

fix(swift-ios): retain Stop feedback until the session is inactive - #10765

Closed
saphid wants to merge 25 commits into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-stop-feedback-20260908
Closed

saphid wants to merge 25 commits into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-stop-feedback-20260908

Conversation

@saphid

@saphid saphid commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

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: +4060 / −25 lines across 11 files; app code +238 / −16, the rest is tests and docs.

An accepted Stop request is not a terminal session outcome. Retain Stopping feedback across thread navigation, expose an unconfirmed state when the outcome cannot be verified, and check current status before Retry stop dispatches again. Keep repeated taps from duplicating an outstanding stop. Disable pending question controls while Stop is active, and resume other queued submissions even when Stop fails.

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. The Stop regression run passed 119 tests; native before/after captures verify pending question controls are disabled until Stop completes. Skipped checks are not claimed as executed proof.

Delivery: stacked. This branch contains #10758, #10759, #10761, #10762, #10763, #10764; 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.

Visual verification: Baseline a361b2212 and candidate 9bcc08bea use actual ThreadDetailView. Both captures passed and assert one Stop dispatch. The candidate retains unconfirmed feedback after acknowledgement and clears it after completed status. The fixture supplies no scoped connection, so it correctly does not claim a confirmed remote outcome. Native model tests separately cover connected acknowledgement, close/reopen retention, duplicate taps and status reconciliation before retry.

Actual before/after stills, held for 3.5 seconds each; not a motion or latency measurement.

Stop acknowledgement: no retained feedback before; unconfirmed until inactive after — still-state comparison

Before screenshot · After screenshot

Before (base), full recorded sequence at 12 fps and real-time playback:

Before: Stop acknowledgement: no retained feedback before; unconfirmed until inactive after

After (candidate), full recorded sequence at 12 fps and real-time playback:

After: Stop acknowledgement: no retained feedback before; unconfirmed until inactive after

Before video · Clean candidate video · Annotated candidate video · Evidence receipt

Additional Stop regression proof: baseline faa5574e6 leaves Submit actionable during an outstanding Stop. Candidate 58ad6fe56 disables the answer panel until completion. Both native captures passed; semantic snapshots confirm controls become available again after completion, and no answer was sent. The fixture invokes the actual model Stop method after selecting Continue; it does not depict a Stop-button tap. A separate regression verifies failed Stop resumes other queued submissions.

Actual before/after stills, held for 3.5 seconds each; not a motion or latency measurement.

Pending question: Submit remains enabled before; disabled during Stop after — still-state comparison

Before screenshot · After screenshot

Before (base), full recorded sequence at 12 fps and real-time playback:

Before: Pending question: Submit remains enabled before; disabled during Stop after

After (candidate), full recorded sequence at 12 fps and real-time playback:

After: Pending question: Submit remains enabled before; disabled during Stop after

Before video · Clean candidate video · Annotated video · Evidence receipt

Capture currency: checked against 5e0055bdc. The original Stop scenario contains no pending question or queued creation, so later changes to those paths do not alter that demonstration. The separate pending-question capture at 58ad6fe56 includes the final control fix; only environment lookup error propagation and its test changed afterward.

Visual review: inspected the before/after images and recorded interaction states at 390 px width against current head 5e0055bdc. The demonstrated UI is unchanged from the labeled capture revisions; retained content, recovery controls, spacing, and text are legible. This is the author’s visual assessment, not maintainer approval.

Independent cross-provider review was unavailable: claude auth status returned exit 1 with loggedIn: 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). Head 33c35dd32f.

  • Focused tests at this layer, measured before the review fixes below (NativeMultiEnvironmentTests, NativeThreadCatchUpTests, NativeRetryIdentityTests, FeatureRootModelTests, iOS Simulator, per-test time limits): 117 passed.
  • 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 fixes (round 7: Macroscope and Astra)

  • Stop could not be confirmed while a turn was streaming. Stop status refreshes the shell cache and reads it back, but a refresh superseded by a newer stream update threw a cancellation, so the 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 now answers instead; after a disconnect or lost authority the check still fails. This works for peer environments as well as the active one. Test: stopStatusDuringLiveUpdates(environmentID:authorityLost:) (cases active superseded, active authority lost, peer superseded). The original code fails the superseded cases.
  • GPT-6 Astra (codex exec -m gpt-6-astra -c model_reasoning_effort="xhigh" --sandbox read-only) reviewed this fix across rounds 8–12. It rejected two broader versions: one that trusted the cache after a reconnect, and one that answered from the HTTP read alone, which could overwrite newer visible state and leave cancelTurn validating a stale cache. The final version returned ready with no medium or higher findings.

Follow-ups (minor, not blocking)

  • After a failed hydration, a later HTTP-only hydration can leave the root connection state at disconnected until a socket snapshot arrives (Macroscope).

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

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). labels Sep 8, 2026
@saphid
saphid marked this pull request as ready for review September 8, 2026 23:05
Comment thread apps/swift-ios/Features/Root/FeatureRootModel.swift
Comment thread apps/swift-ios/Features/Chat/FeatureComposerView.swift
@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR introduces a substantial cross-cutting rewrite of Swift iOS synchronization, lifecycle, thread reconciliation, and Stop behavior across production paths, rather than a narrowly isolated fix. Unresolved correctness risks remain in Stop completion handling and shell hydration/connection-state recovery.

Not approved because:

  • 3 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

Comment thread apps/swift-ios/App/NativeFeatureClient.swift Outdated
@cursor

cursor Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot 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.

@saphid

saphid commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@t3dotgg Alex has approved this PR for your review. Stop feedback persists until the session settles, including pending-question controls. The current description contains the verification evidence.

@saphid
saphid force-pushed the pr/swiftui-stop-feedback-20260908 branch from 5e0055b to b0a4aa4 Compare September 26, 2026 23:47
@@ -1340,6 +1529,7 @@ public final class FeatureRootModel {
pendingRewindRecoveryIDs.remove(thread.id)
rewindErrors[thread.id] = nil
}
for thread in value.threads { reconcileStop(thread) }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High Root/FeatureRootModel.swift:1532

An idle snapshot leaves a successful Stop request in .awaitingOutcome, so the thread remains non-actionable and its Stop-related controls stay disabled indefinitely. install now calls reconcileStop for every snapshot, but that reconciliation rejects the valid inactive idle status even though the native mapper represents it as a completed latest turn; update reconcileStop to finalize the request for idle snapshots as well.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/Features/Root/FeatureRootModel.swift around line 1532:

An `idle` snapshot leaves a successful Stop request in `.awaitingOutcome`, so the thread remains non-actionable and its Stop-related controls stay disabled indefinitely. `install` now calls `reconcileStop` for every snapshot, but that reconciliation rejects the valid inactive `idle` status even though the native mapper represents it as a completed latest turn; update `reconcileStop` to finalize the request for `idle` snapshots as well.

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>
@saphid
saphid force-pushed the pr/swiftui-stop-feedback-20260908 branch from b0a4aa4 to cfb8289 Compare September 27, 2026 00:56
return true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High App/NativeFeatureClient.swift:1202

The same-client fast path preserves the previous shell epoch, so after a stream restart acceptActiveShell rejects the lower-sequence HTTP hydration and the UI remains on stale projects and threads. Update activeShellConnectionID and activeShellEpochHasSnapshot before returning from this path, as in the replacement-client path.

-            return true
+            activeShellConnectionID = adoptedConnectionID
+            activeShellEpochHasSnapshot = adoptedConnectionID != nil
+                && shellConnectionIDsByEnvironmentID[environment.id] == adoptedConnectionID
+            return true
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 1202:

The same-client fast path preserves the previous shell epoch, so after a stream restart `acceptActiveShell` rejects the lower-sequence HTTP hydration and the UI remains on stale projects and threads. Update `activeShellConnectionID` and `activeShellEpochHasSnapshot` before returning from this path, as in the replacement-client path.

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>
@saphid
saphid force-pushed the pr/swiftui-stop-feedback-20260908 branch from cfb8289 to 22a2817 Compare September 27, 2026 20:29
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>
@saphid
saphid force-pushed the pr/swiftui-stop-feedback-20260908 branch from 22a2817 to edb3c12 Compare September 27, 2026 21:14
Comment thread apps/swift-ios/App/NativeFeatureClient.swift Outdated
github-actions Bot and others added 5 commits September 28, 2026 07:26
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>
@saphid
saphid force-pushed the pr/swiftui-stop-feedback-20260908 branch from edb3c12 to 3595ca0 Compare September 27, 2026 22:06
github-actions Bot and others added 14 commits September 28, 2026 08:23
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>
@saphid
saphid force-pushed the pr/swiftui-stop-feedback-20260908 branch from 3595ca0 to 33c35dd Compare September 27, 2026 22:27
guard let activeEnvironment else { return }
publish(makeSnapshot(
environments: environments, activeEnvironment: activeEnvironment,
connectionState: connectHeader ? .connected : (latestSnapshot?.connection.state ?? .reconnecting),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium App/NativeFeatureClient.swift:454

A successful fallback HTTP hydration applies the shell but leaves the published root snapshot in .disconnected when the previous hydration failed and no WebSocket snapshot has arrived. This preserves latestSnapshot.connection.state because connectHeader is false, so consumers continue to treat the reachable session as offline; publish the successful shell with a connected state instead.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 454:

A successful fallback HTTP hydration applies the shell but leaves the published root snapshot in `.disconnected` when the previous hydration failed and no WebSocket snapshot has arrived. This preserves `latestSnapshot.connection.state` because `connectHeader` is false, so consumers continue to treat the reachable session as offline; publish the successful shell with a connected state instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged: after a failed hydration, a later HTTP-only hydration applies the shell but can leave the root connection state at disconnected until the socket snapshot arrives. Listed as a follow-up.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

@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 33c35dd32f:

  • 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.
  • 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 the Stop-status fix in this layer across rounds 8–12. It rejected two broader versions; the final version returned ready with no medium or higher findings. The rest of the layer was covered by the SWE-2 Max review of the whole chain.

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.

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

The new Stop unconfirmed and Retry stop workflow changes cancellation behavior across navigation and reconnects. That scope requires prior approval. The comment mentioning Alex’s approval does not link maintainer direction. Please link that approval or agree on the Stop workflow with maintainers, then request reconsideration.

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

Labels

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.

2 participants