Make remote connections recover reliably#867
Merged
Merged
Conversation
Average users on the iOS connection surfaces should see only concrete states and machine names, never the transport route. Two primary machine-row sites still leaked the route kind, and a pairing help line recommended one route as fastest. - Account machine rows (Account + Settings > Machines) rendered machine.routeLabel (Tailscale / Local network / ADE relay) as their subtitle. Use the presence-only machineStatusHint instead, matching the saved rows in the same list. Route kind stays in the Connection details diagnostics section. - Pairing help no longer recommends Tailscale as fastest; it keeps the plain reachable-from-any-network message. - Add a regression test locking the account row hint route-neutral even when the machine advertises tailnet/relay endpoints. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThe pull request updates account authentication, Relay transport negotiation and authorization, sync recovery, terminal transcript streaming, desktop and iOS connection flows, tunnel-relay deployment/testing, and machine reachability UI across the CLI, desktop, iOS, and worker applications. ChangesRelay worker and deployment
Account and sync services
Terminal and client recovery
Machine connection presentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
apps/ios/ADETests/PairingAndDpopTests.swift (1)
297-310: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winVerify the relay proof cryptographically, not only by inequality.
These assertions would pass if the nonce and signature merely changed between calls, even when the signature was not generated over the canonical relay reauthorization challenge. Extract the proof fields and verify each signature against
buildRelayReauthorizationChallenge(...); also assert that changing the nonce causes verification to fail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ios/ADETests/PairingAndDpopTests.swift` around lines 297 - 310, Strengthen testRelayReauthorizationProofUsesDistinctNoncePerAttempt by extracting each proof’s nonce and signature, then cryptographically verifying both signatures against buildRelayReauthorizationChallenge(...) using the corresponding nonce and relay parameters. Also verify that substituting one proof’s nonce into the other signature fails, while retaining the distinct-nonce assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/webclient/sync/client.ts`:
- Around line 1207-1231: Guard payload.error before accessing retryable, code,
or message in the terminal input acknowledgment handling block. Update the
!payload.ok path in the terminal input queue flow to treat a missing or
malformed error object as a definitive failure via failTerminalInput, ensuring
the queued operation cannot remain stuck.
In `@apps/ios/ADE/Services/SyncTerminalInputQueue.swift`:
- Around line 147-153: Update SyncTerminalInputQueue.prepareForReconnect to
reset each queued item’s retry accounting when restoring delivery to .unsent:
clear attemptCount, firstSentUptime, and lastSentUptime along with delivery.
Preserve all other item data so the resumed connection receives a fresh retry
budget.
In `@apps/ios/ADE/Views/Settings/SettingsSupportTypes.swift`:
- Around line 84-97: Update statusLabel(for:canReconnectToSavedHost:) so the
.disconnected case always returns "Not connected"; reserve "Can't reach this
Mac" for the .unreachable case, preserving the existing neutral disconnected
styling and reconnect behavior elsewhere.
In `@apps/ios/ADE/Views/Work/SwiftTermSessionView.swift`:
- Line 113: Reset inputStatusMessage to nil when input is successfully accepted
or the stream re-hydrates, updating the relevant success and rehydration paths
in SwiftTermSessionView. Preserve the existing failure assignments for rejected
input while ensuring later blocked-input events fall back to the “Reconnecting”
caption.
- Around line 260-262: Update handleConnectionChange(isConnected:) to gate the
PTY resize re-assert on the reconnect-persistent desiredTerminalSessionIds
signal instead of subscribedTerminalSessionIds. Preserve the resize behavior
immediately after resetTerminalTransportStateForReconnect(), before
restoreTerminalSubscriptions() completes asynchronously.
In `@apps/tunnel-relay/src/tunnelDo.ts`:
- Around line 24-26: Update the “Close codes” table in the relay README to
document CLOSE_STALE_PIPE (4508), CLOSE_FORWARD_FAILED (4509), and
CLOSE_NOT_READY (4510), using the existing descriptions from the corresponding
constants in tunnelDo.ts so operators can interpret these codes during
debugging.
---
Nitpick comments:
In `@apps/ios/ADETests/PairingAndDpopTests.swift`:
- Around line 297-310: Strengthen
testRelayReauthorizationProofUsesDistinctNoncePerAttempt by extracting each
proof’s nonce and signature, then cryptographically verifying both signatures
against buildRelayReauthorizationChallenge(...) using the corresponding nonce
and relay parameters. Also verify that substituting one proof’s nonce into the
other signature fails, while retaining the distinct-nonce assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0555554e-514c-41fb-81b1-32d3a8dc7f4b
⛔ Files ignored due to path filters (14)
CHANGELOG.mdis excluded by!*.mdapps/ios/ADE.xcodeproj/project.pbxprojis excluded by!**/*.xcodeproj/project.pbxprojapps/tunnel-relay/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.jsonchangelog/index.mdxis excluded by!changelog/**changelog/v1.2.33.mdxis excluded by!changelog/**docs.jsonis excluded by!docs.jsondocs/ARCHITECTURE.mdis excluded by!docs/**docs/features/sync-and-multi-device/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/crdt-model.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**docs/features/terminals-and-sessions/README.mdis excluded by!docs/**docs/features/terminals-and-sessions/pty-and-processes.mdis excluded by!docs/**docs/features/terminals-and-sessions/ui-surfaces.mdis excluded by!docs/**docs/features/web-client/README.mdis excluded by!docs/**
📒 Files selected for processing (89)
.github/workflows/ci.yml.github/workflows/deploy-web.ymlapps/ade-cli/README.mdapps/ade-cli/src/cli.tsapps/ade-cli/src/services/account/accountAttestationVerifier.test.tsapps/ade-cli/src/services/account/accountAttestationVerifier.tsapps/ade-cli/src/services/account/accountAuthService.test.tsapps/ade-cli/src/services/account/accountAuthService.tsapps/ade-cli/src/services/account/accountMachineDirectoryService.test.tsapps/ade-cli/src/services/account/accountMachineDirectoryService.tsapps/ade-cli/src/services/account/accountMachinePublisherService.test.tsapps/ade-cli/src/services/account/accountMachinePublisherService.tsapps/ade-cli/src/services/sync/brainProjectActionsSyncHandler.tsapps/ade-cli/src/services/sync/relayAuthorization.test.tsapps/ade-cli/src/services/sync/relayAuthorization.tsapps/ade-cli/src/services/sync/sharedSyncListener.tsapps/ade-cli/src/services/sync/syncCloudRelayStore.tsapps/ade-cli/src/services/sync/syncHostService.test.tsapps/ade-cli/src/services/sync/syncHostService.tsapps/ade-cli/src/services/sync/syncPeerService.test.tsapps/ade-cli/src/services/sync/syncPeerService.tsapps/ade-cli/src/services/sync/syncTunnelClientService.test.tsapps/ade-cli/src/services/sync/syncTunnelClientService.tsapps/ade-cli/src/tuiClient/__tests__/remoteLauncher.test.tsapps/ade-cli/src/tuiClient/remoteLauncher.tsapps/desktop/src/main/services/pty/ptyService.test.tsapps/desktop/src/main/services/pty/ptyService.tsapps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.test.tsapps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.tsapps/desktop/src/main/services/sync/syncHostService.test.tsapps/desktop/src/main/services/usage/usageTrackingService.test.tsapps/desktop/src/renderer/components/terminals/TerminalView.test.tsxapps/desktop/src/renderer/components/terminals/TerminalView.tsxapps/desktop/src/renderer/webclient/adapter/__tests__/adapter.test.tsapps/desktop/src/renderer/webclient/adapter/analytics.tsapps/desktop/src/renderer/webclient/adapter/app.tsapps/desktop/src/renderer/webclient/adapter/infra/commandCaller.tsapps/desktop/src/renderer/webclient/adapter/sessionsPty.tsapps/desktop/src/renderer/webclient/shell/MachinePicker.tsxapps/desktop/src/renderer/webclient/shell/WebClientRoot.tsxapps/desktop/src/renderer/webclient/shell/WebShell.tsxapps/desktop/src/renderer/webclient/shell/__tests__/MachinePicker.test.tsxapps/desktop/src/renderer/webclient/shell/__tests__/WebClientRoot.test.tsxapps/desktop/src/renderer/webclient/shell/shellTokens.tsapps/desktop/src/renderer/webclient/sync/__tests__/sync.test.tsapps/desktop/src/renderer/webclient/sync/client.tsapps/desktop/src/renderer/webclient/sync/connection.tsapps/desktop/src/renderer/webclient/sync/dpop.tsapps/desktop/src/shared/accountDirectory.tsapps/desktop/src/shared/types/sessions.tsapps/desktop/src/shared/types/sync.tsapps/ios/ADE/Services/AccountDirectory.swiftapps/ios/ADE/Services/AccountService.swiftapps/ios/ADE/Services/Database.swiftapps/ios/ADE/Services/DpopKeyService.swiftapps/ios/ADE/Services/SyncConnectionRace.swiftapps/ios/ADE/Services/SyncRecoveryPolicy.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Services/SyncTerminalInputQueue.swiftapps/ios/ADE/Views/Account/AccountConnectionsSection.swiftapps/ios/ADE/Views/Account/MobileAccessGateView.swiftapps/ios/ADE/Views/Components/MachineRowView.swiftapps/ios/ADE/Views/Hub/HubQuickConnect.swiftapps/ios/ADE/Views/Settings/ConnectionSettingsView.swiftapps/ios/ADE/Views/Settings/SettingsConnectionHeader.swiftapps/ios/ADE/Views/Settings/SettingsDiagnosticsSection.swiftapps/ios/ADE/Views/Settings/SettingsPairingSection.swiftapps/ios/ADE/Views/Settings/SettingsSupportTypes.swiftapps/ios/ADE/Views/Work/SwiftTermSessionView.swiftapps/ios/ADE/Views/Work/TerminalSessionScreen.swiftapps/ios/ADE/Views/Work/WorkArtifactTerminalViews.swiftapps/ios/ADETests/ADETests.swiftapps/ios/ADETests/PairingAndDpopTests.swiftapps/ios/ADETests/SyncRecoveryPolicyTests.swiftapps/tunnel-relay/README.mdapps/tunnel-relay/package.jsonapps/tunnel-relay/src/relay.tsapps/tunnel-relay/src/tunnelDo.tsapps/tunnel-relay/test/env.d.tsapps/tunnel-relay/test/health.workerd.test.tsapps/tunnel-relay/test/relay.test.tsapps/tunnel-relay/test/relay.workerd.test.tsapps/tunnel-relay/test/smoke-url.mjsapps/tunnel-relay/test/smoke-url.test.mjsapps/tunnel-relay/test/smoke.mjsapps/tunnel-relay/tsconfig.jsonapps/tunnel-relay/vitest.unit.config.tsapps/tunnel-relay/vitest.workerd.config.tsapps/tunnel-relay/wrangler.jsonc
💤 Files with no reviewable changes (1)
- apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts
This was referenced Jul 22, 2026
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
Validation
Release plan
After merge: deploy hosted web client and tunnel Worker, tag v1.2.33 and verify/publish per-arch desktop artifacts, then archive/upload/distribute iOS 1.1.10 build 36 with widgets and App Clip.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Greptile Summary
This PR improves recovery for remote connections and terminal sessions. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
The updated PTY pipeline canonicalizes split and malformed surrogate sequences before transcript accounting and broadcast. Pending high surrogates are flushed during exit and disposal.
No blocking issue related to the previous terminal offset failure remains.
What T-Rex did
Important Files Changed
Reviews (2): Last reviewed commit: "fix(sync): close remaining recovery gaps" | Re-trigger Greptile