Resync the session when the app returns to the foreground - #17
PanAndDuck wants to merge 4 commits into
Conversation
Backgrounding the app suspends (and often kills) the live WebSocket, but nothing observed the scene returning to active, so a session left open during that time just sat frozen — the only way to see new content was to back all the way out to the list and re-enter, which tears down and rebuilds the whole screen via RootView's `.id(...)`. `SessionDetailView` now watches `scenePhase` and calls the view model's new `refreshOnForeground()` on a `.background` -> `.active` transition specifically (not just any transition to `.active`, which also fires for the transient `.inactive` blips a sheet/alert/control-center pull causes). `load()`'s `.existing` branch is refactored into a shared `syncExisting` so both paths reuse the exact same fetch + reattach-if-live logic instead of a parallel implementation; `refreshOnForeground` skips the `.loading` phase flip and the forced re-pin-to-bottom, so resuming the app doesn't visibly reload the transcript out from under someone reading history, and a background failure leaves the existing transcript up rather than replacing it with an error screen. Verified: builds and runs on Simulator and a physical device.
Returning from the background almost always lands on .active via an intermediate .inactive frame first (.background -> .inactive -> .active), so the original check (comparing only the immediately-prior phase) matched the direct .background -> .active case but missed the far more common two-step one. A device only fired the refresh occasionally, and inconsistently, as a result. wasBackgrounded now remembers that a .background frame happened at all since the last refresh, regardless of how many .inactive frames follow it, and is consumed on the next .active. A sheet/alert/control-center pull's own .active -> .inactive -> .active blip never sets the flag, so it still doesn't trigger a spurious refresh. Verified: builds and runs on Simulator and a physical device.
Backgrounding the app while a turn was streaming leaves liveTurn (and its stream) in place — iOS suspends the socket rather than closing it, so nothing locally notices it die. reattachIfLive no-ops whenever liveTurn != nil (its "we're already streaming" fast path, correct at a fresh load() but not when called from refreshOnForeground), so the freshly- fetched final reply rendered ALONGSIDE a permanently-stuck "still in progress" placeholder and Stop button — the compose bar never returned to its send state. refreshOnForeground now discards that stale local tracking first (WITHOUT sending the server a cancel — the turn may genuinely still be running), so reattachIfLive's subsequent discovery reflects the true current state: a fresh stream if it's still going, or nothing further if the server-fetched turns already carry its finished reply. Verified: builds and runs on Simulator and a physical device.
Credit: this reconnect-in-place approach (resumeStreamAfterForeground, the .snapshot mid-turn-adoption fix in consume) is adapted from Adam-Dalloul's xintaofei#10 (fix/live-stream-recovery), which independently diagnosed the same underlying problem this branch's earlier commits worked around with a blunter full re-fetch: iOS suspends the socket without closing it, the silent-reconnect backoff can't make progress while suspended, and a mid-turn reconnect's snapshot wasn't being adopted into the live turn it belongs to. refreshOnForeground now branches on whether a turn is actually live: - If so, resumeStreamAfterForeground() reconnects the SAME liveTurn / connection in place (resetting the reconnect budget iOS silently burned while suspended) instead of discarding and re-fetching — so whatever the agent produced while we were away arrives as a continuation of the turn already on screen, not a separate fetch racing a stuck "in progress" placeholder (the bug fixed two commits ago in this branch). - Otherwise (nothing was streaming), it falls back to this branch's existing full re-sync, which xintaofei#10 does not cover — an idle session renamed, or messaged from another client, while backgrounded. Verified: builds and runs on Simulator and a physical device.
|
Keep it here, one PR beats two. I'll close #10 once this lands. Could you put a trailer on that commit so the attribution sticks? One thing worth a look. Now that a live turn takes the resume path, |
Squashed from upstream xintaofei/codeg-ios PR xintaofei#17 (PanAndDuck), applied on top of PR xintaofei#10: - cb09583 Resync the session when the app returns to the foreground - 2e0b9eb Fix foreground refresh missing most real resumes - 0eda348 Fix duplicate stuck "in progress" state after backgrounding mid-turn - 091977d Reconnect a live turn in place on foreground, adapted from xintaofei#10 PR xintaofei#17 already carries xintaofei#10's live-turn recovery (the `.snapshot` adoption in `consume` and `resumeStreamAfterForeground`) verbatim, so the conflict is resolved by taking xintaofei#17's version of both files: the view's scenePhase handler now routes through `refreshOnForeground()`, which reconnects a live turn in place via xintaofei#10's path and otherwise re-fetches the session.
The README now opens by saying this is a fork of xintaofei/codeg-ios and why. NOTICE keeps the upstream attribution and states that Codeg Plus is a modified version. docs/FORK.md records: - the app identity and where to change it; - the push entitlements and the Apple-side setup they need; - each upstream PR carried, with its author, and how xintaofei#10 and xintaofei#17 were reconciled; - how to turn on the TestFlight job, including running it on a self-hosted Mac runner.
Summary
RootView's.id(...).SessionDetailViewnow watchesscenePhaseand calls a newrefreshOnForeground()on a real background round-trip. AwasBackgroundedlatch is used rather than comparing adjacent phases, since returning to the foreground reports.background→.inactive→.active, not a direct.background→.active.refreshOnForeground()branches on whether a turn was actually live:liveTurn/connection in place (see "Related to fix(session): recover the live stream after a drop or backgrounding #10" below).syncExisting, shared with the initialload()), so e.g. a rename or a message sent from another client while backgrounded is picked up too.Related to #10
@Adam-Dalloul's #10 independently diagnosed the same root cause for the "turn was actively streaming" case — iOS suspends the socket without closing it, and the silent-reconnect backoff can't make progress while suspended — and fixed it more surgically (reconnect the same live turn in place, plus a real bug in
consume's.snapshothandling that was dropping content produced during the outage). I've adapted that approach into this branch'sresumeStreamAfterForeground()/consume()changes rather than duplicating it, with credit in the commit message. This PR's earlier commits also fix a case #10 doesn't cover (nothing was streaming, but the session changed elsewhere while backgrounded) and a duplicate-stuck-"in progress"-state bug that showed up while building the full-refetch fallback. Happy to drop the overlapping part of this PR if you'd rather land #10 as the canonical fix for the live-turn case — just say which shape you'd prefer.Test plan