Repository navigation
Conversation
added 2 commits
July 13, 2026 12:22
…liveness Implements stablyai#6784, stablyai#8498, stablyai#4500 from stablyai#8591. - stablyai#6784: mirror the real socket error per host (getLastConnectionError + useLastConnectionError) and surface it via classifyConnection's hint instead of a canned Tailscale label. - stablyai#8498: refetch worktrees on transition into 'connected' (not every poll tick), add pull-to-refresh, and write through setCachedWorktrees from the host screen so a reconnect can't serve a stale cached snapshot. - stablyai#4500: reap a socket that is warm via app frames but silent on pong — split wsPonged from wsAlive so the liveness check releases the driver. Extracted new hooks (use-worktree-resync, use-host-connection-state, use-last-connection-error) to stay under the max-lines budget. stablyai#8129 (notification replay) scoped to a follow-up bead. Verified: pnpm run typecheck clean; vitest 23/23 (ws-transport) + 9/9 (connection-health) + 40 new lines (worktree-cache); max-lines ratchet OK.
…tablyai#8129) Implements stablyai#8129 (the 4th fix from stablyai#8591), isolated from the stablyai#6784/stablyai#8498/stablyai#4500 commit. - Desktop: MobileNotificationReplayBuffer records every dispatch with a monotonic seq (cap 256, oldest-first evict). dispatchMobileNotification surfaces the seq to live listeners; getMissedNotificationsSince(seq) returns exactly seq > watermark -> idempotent by construction. - Mobile: persist lastSeenSeq (AsyncStorage), send it as the catch-up watermark via notifications.getMissedSince on reconnect, and keep an in-memory seen-set keyed by id+seq so a replay re-including an already delivered event is dropped (defense-in-depth; a legit live re-delivery at a NEW seq is still allowed). - Principal fix: deliverLive now marks events seen on every path (live + replay); previously only fetchMissed did, so a live-then-replay duplicate double-pushed. Re-verified 11/11. Verified: pnpm run typecheck clean; vitest 7/7 (replay) + 11/11 (mobile-notifications, incl dedupe regression); max-lines ratchet OK. No push.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe mobile client now captures and surfaces socket errors in connection health verdicts, centralizes per-host connection state, and supports worktree resynchronization through reconnect-triggered fetches and pull-to-refresh. Worktree fetches update the cache. Desktop notification delivery now assigns sequence numbers and exposes replay data through RPC, while mobile subscriptions persist watermarks and deduplicate reconnect catch-up events. WebSocket heartbeat cleanup now requires protocol PONG responses in addition to inbound traffic. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0c14f157-85bd-4c31-90e8-0acf8ef6e4f6
📒 Files selected for processing (20)
mobile/app/h/[hostId]/index.tsxmobile/app/h/[hostId]/session/[worktreeId].tsxmobile/app/h/[hostId]/tasks.tsxmobile/app/index.tsxmobile/src/cache/worktree-cache.test.tsmobile/src/notifications/mobile-notifications.test.tsmobile/src/notifications/mobile-notifications.tsmobile/src/transport/client-context.tsxmobile/src/transport/connection-health.test.tsmobile/src/transport/connection-health.tsmobile/src/transport/rpc-client.tsmobile/src/transport/use-host-connection-state.tsmobile/src/transport/use-last-connection-error.tsmobile/src/transport/use-worktree-resync.tssrc/main/runtime/mobile-notification-replay.test.tssrc/main/runtime/mobile-notification-replay.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/methods/notifications.tssrc/main/runtime/rpc/ws-transport.test.tssrc/main/runtime/rpc/ws-transport.ts
Comment on lines
+16
to
+18
| export type ReplayableMobileNotification = MobileNotificationEvent & { | ||
| seq: number | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: check how the mobile client reads the watermark from live vs replayed events.
rg -n "notificationSeq|lastSeenSeq|getMissedSince|\.seq\b" -g '!**/node_modules/**' mobile/src/notifications/mobile-notifications.ts mobile/src/notifications/mobile-notifications.test.ts 2>/dev/nullRepository: stablyai/orca
Length of output: 3044
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the replay implementation and the live notification path that sets the watermark field.
ast-grep outline src/main/runtime/mobile-notification-replay.ts --view expanded || true
printf '\n---\n'
ast-grep outline src/main/runtime/orca-runtime.ts --view expanded || true
printf '\n---\n'
sed -n '1,220p' src/main/runtime/mobile-notification-replay.ts
printf '\n--- ORCA RUNTIME MATCHES ---\n'
rg -n "notificationSeq|seq\b|dispatchMobileNotification|getMissedSince|record\(" src/main/runtime/orca-runtime.ts src/main/runtime/mobile-notification-replay.ts
printf '\n--- CLIENT MATCHES ---\n'
sed -n '1,220p' mobile/src/notifications/mobile-notifications.ts
printf '\n--- CLIENT REPLAY/DELIVERY LOGIC ---\n'
sed -n '300,420p' mobile/src/notifications/mobile-notifications.tsRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' src/main/runtime/mobile-notification-replay.ts
printf '\n---\n'
sed -n '7350,7405p' src/main/runtime/orca-runtime.ts
printf '\n---\n'
sed -n '1,120p' mobile/src/notifications/mobile-notifications.ts
printf '\n---\n'
sed -n '330,390p' mobile/src/notifications/mobile-notifications.ts
printf '\n---\n'
sed -n '410,455p' mobile/src/notifications/mobile-notifications.test.ts
printf '\n---\n'
sed -n '470,540p' mobile/src/notifications/mobile-notifications.test.tsRepository: stablyai/orca
Length of output: 16981
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every place that mentions the replay watermark field or the getMissedSince RPC.
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \
"getMissedSince|notificationSeq\b|ReplayableMobileNotification|seq\b" \
src mobile shared packagesRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '2038,2072p' src/main/runtime/orca-runtime.ts
printf '\n---\n'
sed -n '118,140p' mobile/src/notifications/mobile-notifications.ts
printf '\n---\n'
sed -n '320,380p' mobile/src/notifications/mobile-notifications.ts
printf '\n---\n'
sed -n '1,130p' src/main/runtime/mobile-notification-replay.test.tsRepository: stablyai/orca
Length of output: 9796
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the replay handling in the mobile client and the RPC wiring for notifications.getMissedSince.
sed -n '380,460p' mobile/src/notifications/mobile-notifications.ts
printf '\n---\n'
rg -n "notifications.getMissedSince|getMissedNotificationsSince|mobileNotificationReplay|dispatchMobileNotification" src/main/runtime/orca-runtime.ts src/main/runtime/*.ts src/main/runtime/rpc/*.ts
printf '\n---\n'
sed -n '7370,7405p' src/main/runtime/orca-runtime.ts
printf '\n---\n'
sed -n '1,220p' src/main/runtime/runtime-rpc.tsRepository: stablyai/orca
Length of output: 14446
Replay events need notificationSeq
ReplayableMobileNotification and record() currently stamp replayed events as seq, but the live mobile path and reconnect logic read notificationSeq. notifications.getMissedSince() therefore returns payloads the client won’t watermark or dedupe correctly. Unify the field name at the replay source, or translate it at the RPC boundary.
- stablyai#6784 (CRITICAL): rpc-client.ts ws.onerror called an undefined captureConnectionError(); replace with the real lastConnectionError assignment so the connection-error capture actually works at runtime. - stablyai#8498: HostClientEntry now includes state (matches useAllHostClients shape) in use-host-connection-state.ts. - stablyai#8129: watermark load uses Math.max(lastDeliveredSeq, seq) so an async storage read can't move the replay watermark backward. C4 (replay seq vs notificationSeq) is a false positive: notificationSeq is assigned the buffer's seq at the dispatch boundary, so the client watermark matches the desktop cut. No change. Verified: pnpm run typecheck clean; vitest rpc-client 29/29, connection-health 9/9, mobile-notifications 11/11, replay 7/7; max-lines ratchet OK.
Contributor
Author
CodeRabbit review — triage & fixesThanks for the review. Triage of the 4 comments: Fixed (3 real):
Not a bug (1 false positive):
All three fixes are in commit |
nwparker
added a commit
that referenced
this pull request
Jul 14, 2026
… reconnect (#8498 #8129) Take-over of #8605 (issue #8591). Ships #8498 (worktree resync + pull-to-refresh + cache write-through) and #8129 (idempotent notification replay on reconnect). Fixes the original PR's field mismatch (seq vs notificationSeq) and adds the missing notifications.getMissedSince mobile RPC allowlist entry. #6784 and #4500 held back to avoid conflicting with the relay work (#8536). Co-authored-by: Brandon Bennett (@branben).
Contributor
|
Thank you for this, @branben — the root-cause investigation in #8591 and the architecture here were genuinely excellent, and the idempotent-replay design in particular was the right shape. I've taken the two fixes that are clear of the in-flight relay work (#8536) and shipped them, corrected, in #8690 (now merged to
A few things I found in review, for future reference:
I held #6784 and #4500 back on purpose: they live in the connection-health / ws-heartbeat surface that the relay PR #8536 is actively rewriting, so changing them now would conflict. (#6784's raw-error approach was also a bit different from what the issue asks for — a Tailscale recommendation.) Closing in favor of #8690 — but huge thanks, this was a high-quality contribution. |
enrick-dev
pushed a commit
to enrick-dev/orca
that referenced
this pull request
Oct 15, 2026
… reconnect (stablyai#8498 stablyai#8129) Take-over of stablyai#8605 (issue stablyai#8591). Ships stablyai#8498 (worktree resync + pull-to-refresh + cache write-through) and stablyai#8129 (idempotent notification replay on reconnect). Fixes the original PR's field mismatch (seq vs notificationSeq) and adds the missing notifications.getMissedSince mobile RPC allowlist entry. stablyai#6784 and stablyai#4500 held back to avoid conflicting with the relay work (stablyai#8536). Co-authored-by: Brandon Bennett (@branben).
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
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
Implements the four mobile-reconnect fixes documented in #8591 (and the
investigation spec). When the Orca mobile app returns from background/sleep,
it re-opens the WebSocket but previously performed no resync of
desktop-held state — this PR closes that gap across notifications, terminal
input ownership, worktree snapshots, and real connection-error visibility.
Shipped as two isolated commits for clean review/rollback:
fix(mobile): surface real conn errors, resync worktrees, harden pong-liveness(Mobile: surface a Tailscale recommendation when the remote runtime is unreachable #6784 [Bug]: Mobile Projects view frequently shows an incomplete worktree list and cannot be manually refreshed #8498 [Bug]: Terminal input silently locks after a mobile "driver" disconnects ungracefully (~30s, or permanently behind a reverse proxy) #4500)fix(mobile): replay missed notifications idempotently on reconnect([Feature]: Real push notifications for mobile — events are lost while the app is backgrounded/disconnected #8129)Fixes
getLastConnectionError+useLastConnectionError) viaclassifyConnection's hint instead of a canned Tailscale label.connected(not every poll tick), add pull-to-refresh, and write throughsetCachedWorktreesfrom the host screen so a reconnect can't serve a stale cached snapshot.wsPongedfromwsAliveso the liveness check releases the driver (the exact hardening the adversarial review called for).MobileNotificationReplayBuffer(monotonicseq, cap 256) +getMissedNotificationsSince(seq); mobile persistslastSeenSeqand replays missed notifications idempotently (no duplicate pushes — the key risk the review flagged).Verification
pnpm run typecheckcleanvitest:ws-transport23/23 ([Bug]: Terminal input silently locks after a mobile "driver" disconnects ungracefully (~30s, or permanently behind a reverse proxy) #4500 regression),connection-health9/9 (Mobile: surface a Tailscale recommendation when the remote runtime is unreachable #6784),worktree-cache([Bug]: Mobile Projects view frequently shows an incomplete worktree list and cannot be manually refreshed #8498 write-through),mobile-notification-replay7/7 +mobile-notifications11/11 ([Feature]: Real push notifications for mobile — events are lost while the app is backgrounded/disconnected #8129 dedupe regression)check-max-lines-ratchetOK (no new bypasses)use-worktree-resync,use-host-connection-state,use-last-connection-error) to stay under the max-lines budgetCloses #8591.