Skip to content

fix: pr-bug-scan validated finding from #8690 - #8759

Closed
buf0-bot[bot] wants to merge 1 commit into
mainfrom
bot/pr-bug-scan-8690-1784057474
Closed

buf0-bot[bot] wants to merge 1 commit into
mainfrom
bot/pr-bug-scan-8690-1784057474

Conversation

@buf0-bot

@buf0-bot buf0-bot Bot commented Jul 14, 2026 •

Copy link
Copy Markdown
Contributor

Automated fix-PR from pr-bug-scan for parent #8690.

Fixer status: FIXED_WITH_CODE_PROOF
Summary: In index.tsx wireUp, stop tearing down the notification subscription on non-'connected' states so its stream survives reconnect and reconnectReadyCount can exceed 1, firing fetchMissed.
Blocked proof link: mobile/app/index.tsx:534-536 wireUp's else branch no longer calls unsubNotif()/removeStreamListener; the notifications.subscribe stream now persists across the setState('reconnecting') at rpc-client.ts:732, so replay re-emits 'ready' on the same subscription and reconnectReadyCount reaches 2, satisfying the >1 gate at mobile-notifications.ts:354.
Typecheck: skipped

Proof (from validator)

  • C2 — proof_type: code_analysis
    • trigger: Phone backgrounds/sleeps, its WebSocket is reaped, then it foregrounds and the RPC client reconnects — the ordinary background→reconnect cycle on the home screen.
    • observable: notifications.getMissedSince is never called on reconnect; notifications dispatched while the phone's socket was reaped are never fetched or shown. The catch-up feature ([Feature]: Real push notifications for mobile — events are lost while the app is backgrounded/disconnected #8129) is dead code in production.
    • path:
      • mobile/app/index.tsx:514 subscribeToDesktopNotifications is called inside wireUp only when state === 'connected'; this is the only production caller (grep-confirmed).
      • mobile/app/index.tsx:534-536 wireUp's else branch runs on ANY non-'connected' state and calls unsubNotif() (the subscription's returned cleanup), setting unsubNotif=null.
      • mobile/src/transport/rpc-client.ts:1187 that cleanup ends in removeStreamListener(id), deleting the notifications.subscribe stream so the reconnect replay loop cannot re-emit 'ready' on it.
      • mobile/src/transport/rpc-client.ts:732 (and :768) every socket close calls setState('reconnecting') before any reconnect, guaranteeing wireUp's teardown fires first — no reconnect keeps state 'connected'.
      • mobile/app/index.tsx:511-514 on the subsequent 'connected' transition wireUp constructs a brand-new subscription.
      • mobile/src/notifications/mobile-notifications.ts:335 the new subscription initializes reconnectReadyCount = 0, so its first (reconnect) 'ready' at :344 only reaches count 1.

Generated by pr-bug-scan (proof-machine architecture). Human approval required before merge.

ELI5

Mobile notification subscriptions were torn down on reconnect, so missed notifications never fetched. The stream stays up across reconnect so catch-up can run more than once.

In index.tsx wireUp, stop tearing down the notification subscription on non-'connected' states so its stream survives reconnect and reconnectReadyCount can exceed 1, firing fetchMissed.
@nwparker nwparker added the bug Something isn't working label Jul 27, 2026
@brennanb2025

Copy link
Copy Markdown
Contributor

Thanks for identifying the reconnect teardown issue. Closing as superseded by #10816, which preserves subscription teardown while keeping the watermark and deduplication state alive per host, and adds direct reconnect-teardown regression coverage plus the broader counter-restart fix.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants