Fix reconnect UI after a long background (skeleton instead of stale data + auth toast) - #245
Merged
Merged
Conversation
…ata + auth toast After the app sits backgrounded for hours and is reopened, the torrents list kept showing stale cards and the torrent detail screen popped an "Authentication failed" toast (then "Torrent not found") for a few seconds while the app silently re-logged in. Root cause: TorrentContext's "clear recovery" effect compared dataUpdatedAt against 0, but that timestamp is the last successful sync from potentially hours ago, so it cleared isRecoveringFromBackground on the very next render instead of waiting for a genuinely new fetch. That's why the existing skeleton guard on the torrents list never engaged, and why nothing signaled a reconnect was in flight at all — checkAndReconnect() never had a visible "in progress" state anywhere. - context/TorrentContext.tsx: baseline the recovery-clear effect against the pre-recovery dataUpdatedAt instead of comparing to 0; add a 15s safety cap so a wedged recovery can't leave the skeleton up forever. - context/ServerContext.tsx: add isReconnecting, set only by the checkAndReconnect() run that owns the in-flight promise, kept separate from isLoading/isConnecting so their existing consumers are unaffected. - app/(tabs)/(torrents)/index.tsx: skeleton guard also covers isReconnecting (a session dying while already foregrounded, not just on background return). - app/(tabs)/(torrents)/torrent/[hash].tsx: classify a load failure as a dead session (same check useReactiveReconnect uses) and show a new SkeletonTorrentDetail instead of toasting the raw auth error and falling through to "Torrent not found"; retries immediately once the reconnect resolves instead of waiting on the next 2s poll. - components/SkeletonLoader.tsx: new SkeletonTorrentDetail placeholder. No preference keys, colors, or stored ServerConfig fields were touched — this is purely in-memory UI/state, so no upgrade path is needed for existing users. No qBittorrent API surface changed either. 3.8.43, changelog entry added.
… RN test traps
AGENTS.md had drifted from the repo in ways that actively misled agents:
- **`develop` is gone.** The doc mandated it in ten places ("Never commit to
develop", "Always cut from develop — there is no hotfix exception",
`gh pr create --base develop`), but no such branch exists locally or on
origin, and no PR has ever targeted it. Everything since #219 goes straight
to main (#218 and earlier used preview). An agent following the doc would
have `git switch develop` fail outright. Rewritten around main, with an
explicit note that develop/preview are retired so the next reader doesn't
re-derive it, plus the release/vX.Y.ZZ branch that does get used.
- **docs/RELEASING.md's changelog tree was half-dead** for the same reason —
the "On develop → .TESTFLIGHT placeholder" branch can never fire
(`grep -c TESTFLIGHT constants/changelog.ts` is 0). Removed; the main-branch
rules it kept are unchanged.
The run-cost model was off by ~25x, which was buying caution nobody needed:
one suite ~18s claimed → 1.2s actual; tsc 25s warm / 85s cold → 0.8s / 3.2s
full npm test 4.9s (1095 tests); lint 6.4s; whole batch ~12s
The three-tier "exactly three moments to run anything" ceremony was therefore
discouraging verification to save ~12 seconds. Replaced with the constraint
that actually binds — context, not wall time, since npm test dumps hundreds of
lines of unrelated act()/Animated noise. New rule: run the narrow check freely,
pipe it through tail. Commit-time batch is unchanged.
Also added two rn-project test traps that each cost about an hour to rediscover:
a synchronous act(() => ...) around a promise-returning call silently swallows
the state update (looks exactly like a product bug), and render() returns a
promise so an outside-provider throw needs .rejects.toThrow.
Smaller corrections: the Alert deviations list was missing ConfirmModal itself
(nine calls across eight files, not eight); File Index gains
SkeletonTorrentDetail and notes ServerContext.isReconnecting and how
TorrentContext's recovery flag is cleared.
…te into bugfix/reconnect-after-background
…has section Independent review of the AGENTS.md rewrite from the previous commit: - Fixed a dead anchor (#dont-burn-runs) left over from renaming that section, and collapsed the top "How to work a task" workflow from 7 steps to 6 so it actually matches the run-checks-freely rewrite instead of still describing the old run-nothing-mid-task / verify-separately two-step split. - Wrote a script to slugify every heading and check every (#anchor) link resolves — all 19 links against 31 headings check out now. - Corrected a stale File Index claim: useTorrentActions.ts was documented as "used by both list and detail," but only the list screen imports it — the detail screen hand-rolls its own parallel handlePauseResume/handleDelete/etc. This fed the "Add a torrent action" recipe, which told an agent to edit one place when two need it; fixed both the Hooks entry and the recipe. Also, per request: framed the file as a living document (a short note near the top pointing at where to fix drift), and added a new §10 Gotchas section for surprises that don't have a natural home elsewhere — tooling quirks or misleading errors that look like product bugs. Seeded with the RN act() testing trap as the first entry, with instructions for future sessions to append rather than silently work around and move on.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After the app sits backgrounded for hours and is reopened, the torrents list kept showing stale cards, and tapping a torrent on the detail screen popped an "Authentication failed" toast (then "Torrent not found") for a few seconds while the app silently re-logged in.
Root cause:
TorrentContext's "clear recovery" effect compareddataUpdatedAtagainst0, but that timestamp is the last successful sync from potentially hours ago — so it clearedisRecoveringFromBackgroundon the very next render instead of waiting for a genuinely new fetch. That's why the existing skeleton guard on the torrents list never actually engaged, and why nothing in the UI could tell a reconnect was in flight at all (checkAndReconnect()never had a visible "in progress" state anywhere).Changes
context/TorrentContext.tsx— baseline the recovery-clear effect against the pre-recoverydataUpdatedAtinstead of comparing to0; added a 15s safety cap so a wedged recovery can't leave the skeleton up forever.context/ServerContext.tsx— addedisReconnecting, set only by thecheckAndReconnect()run that owns the in-flight promise. Kept deliberately separate fromisLoading/isConnectingso their existing consumers are unaffected.app/(tabs)/(torrents)/index.tsx— skeleton guard also coversisReconnecting(a session dying while the app is already foregrounded, not just on background return).app/(tabs)/(torrents)/torrent/[hash].tsx— classifies a load failure as a dead session (same checkuseReactiveReconnectuses) and shows a newSkeletonTorrentDetailinstead of toasting the raw auth error and falling through to "Torrent not found". Retries immediately once the reconnect resolves instead of waiting on the next 2s poll.components/SkeletonLoader.tsx— newSkeletonTorrentDetailplaceholder, loosely shaped after the real hero card.package.json/constants/changelog.ts— version bump to 3.8.43 with a changelog entry (done at the user's explicit request).Compatibility
No preference keys,
colorsentries, or storedServerConfigfields were touched — this is purely in-memory UI/state, so there's no upgrade path needed for existing users' saved data. No qBittorrent API surface changed either (no new endpoints, no version-gated parameters), so this is not a 4.x/5.0 concern.Test plan
npx tsc --noEmit— exit 0npm test— 1095/1095 passing across both projects (added 2 new tests totests/rn/context/ServerContext.test.tsxforisReconnecting's lifecycle, and a newtests/rn/context/TorrentContext.test.tsxreproducing the original bug — foreground recovery flag surviving a failed re-sync — plus a smoke test forSkeletonTorrentDetail)npm run lint— 0 errors (37 pre-existing warnings, unrelated to this change)npm run format— clean🤖 Generated with Claude Code