Skip to content

fix(client): slow networks no longer drop working connections - #19426

Open
t3dotgg wants to merge 8 commits into
mainfrom
t3/slow-links-keep-connections
Open

t3dotgg wants to merge 8 commits into
mainfrom
t3/slow-links-keep-connections

Conversation

@t3dotgg

@t3dotgg t3dotgg commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Healthy slow connections can be dropped when a health check or cached T3 Connect ticket takes more than three seconds. The shared connection supervisor now gives quick probes six seconds and extends them when raw WebSocket messages still arrive, with an absolute fifteen-second cap. Cached ticket requests also get six seconds, including learned LAN and tailnet routes.

Frame counting runs before the RPC read loop, so a full subscription buffer cannot hide incoming traffic. The message listener belongs to the socket scope and is removed on release. Mobile returns from an inactive state without a background visit reset retries and check for a better route without probing or resubscribing shell/thread streams. Genuine background resumes retain their existing probe/reconnect behavior. Effect's five-second ping cadence, direct endpoint identity checks, and current main's token-preservation rules are unchanged.

Maintainer takeover of #19426, building on @t3dotgg's work and @nick-potts's original contribution in #18704. All contributor commits are preserved; current main was merged and follow-ups simplify the tests, socket ownership, and comments.

Verification on headless Linux, Node 24, at aa7bb7087e:

  • Focused tests: 421 tests / 19 files → 419 tests / 19 files, passing. Eight mutations demonstrate failures for the old quick timeout, missing frame extension, wrong retry window, excessive cap, missing raw listener, old ticket timeout, inactive refocus probe, and refocus resubscription. Client-runtime, mobile, web, and desktop typechecks pass; all changed files pass lint/format and whitespace checks.
  • Actual loopback WebSocket timing evidence: production RpcSession and EnvironmentSupervisor with Node's WebSocket client and a controlled server. Substituting the base supervisor replaces the healthy socket at 3,014 ms; the rebuilt head keeps the same socket through an 8,006 ms probe reply while messages arrive. A silent socket is replaced at 6,018 ms; continuous incoming messages cannot extend a stalled probe beyond 15,022 ms. All owned loopback servers closed. This is protocol-level evidence with synthetic configuration, not a browser recording or a real WAN reproduction.
  • Both independent full-diff adversarial reviewers found no substantive issues in round 1. One independently reran 224 tests across seven relevant files. Human review of authorization-path and runtime default changes remains required; this audit does not supply approval.
  • Trade-off: detecting silence through a quick probe now takes six seconds. Incoming messages can defer replacement to fifteen seconds. A stalled cached ticket uses six seconds of the existing fifteen-second setup budget, leaving roughly nine seconds for renewal and the next ticket request on that route.
  • Native device access is disabled in this environment. Real slow-network T3 Connect/Tailscale connections, iOS Control Center/Face ID and background lifecycle, Android, macOS, Windows, hosted browser, and Electron desktop were not exercised. Device/network verification and human approval remain outstanding. Current-head CI passes every executed job, including Check, Test, Test Web, Build and Typecheck, Lint, and eight server shards. Correctness and CodeRabbit pass; Approvability remains neutral. A duplicate fingerprint job was cancelled before a successful replacement. This PR remains unmerged.

Tests: GPT-6-Astra (high); implementation: GPT-6.1-Sol (high); adversarial reviews: Claude Opus 5.5 and GPT-6-Sol through T3 Code delegated tasks. Coordinated with GPT-6.1-Sol through the Codex harness in T3 Code.

Codex and others added 4 commits October 14, 2026 05:18
Skip the resume check when iOS only went inactive, and give the quick
liveness probe and the cached T3 Connect ticket 6s instead of 3s so a
high-latency, lossy link is not mistaken for a dead one.
A health check on a slow link timed out after the quick 6s window even
while pongs and stream data were still arriving, dropping a working
connection. Count socket reads in the RPC session, and when the probe
window expires with new data received, wait another window, capped at
the full 15s probe timeout.
Pings now run every 3 seconds, so a working idle link always delivers a
frame inside the 6 second quick probe window. A probe that a retry
shortened now extends by the retry's window, not the original one. A
mobile refocus without a background visit sends application-refocused,
which restarts the retry ladder and checks for a better route but does
not probe the live session. Adds a test of the read counter against
real pongs.
Review follow-ups for the slow-link probe changes:

- Count each WebSocket frame as it arrives, with a message listener on the
  session's socket, instead of counting read-loop pulls. A read loop held up
  by a full stream buffer no longer hides a link that still delivers. The
  session field is now receivedFrames.
- Keep Effect's 5 second ping interval. Delivery is in order, so a pong for
  a ping sent after the probe arrives after the probe's answer and cannot
  extend it. The 3 second interval only added ping traffic on every client.
- Comments no longer promise a frame in every probe window. The local
  window variable is now probeWindow, so it does not shadow the DOM global.
- Shell and thread tests check that a mobile refocus does not resubscribe.
- The connection runtime doc describes the probe extension and the refocus.

Co-authored-by: Nick <1109914+nick-potts@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Oct 14, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 14, 2026
@github-actions github-actions Bot added the size:M 30-99 changed lines (additions + deletions). label Oct 14, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This PR changes default connection and cached-ticket timeouts and modifies central WebSocket, mobile lifecycle, and authorization behavior. Those production defaults and authorization-path changes require human review despite the focused tests and otherwise targeted bug-fix scope.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB −7 B (−0.1%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB −7 B (−0.6%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB +24 B (+0.5%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB +24 B (+2.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB +41 B (+0.2%) 29.3 KiB ✅
Claude Live turn messages 1 2 +1 (+100.0%) 8 ✅

Baseline: dd5c7bd · PR result: b57afe0 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: b2e1ff20-b7fc-4b55-b6da-50daaf8c4e85
📥 Commits

Reviewing files that changed from the base of the PR and between b57afe0 and aa7bb70.

📒 Files selected for processing: 16
  • apps/mobile/src/connection/app-state-wakeups.test.ts
  • apps/mobile/src/connection/app-state-wakeups.ts
  • docs/internals/connection-runtime.md
  • packages/client-runtime/src/authorization/layer.test.ts
  • packages/client-runtime/src/authorization/service.ts
  • packages/client-runtime/src/connection/registry.test.ts
  • packages/client-runtime/src/connection/supervisor.test.ts
  • packages/client-runtime/src/connection/supervisor.ts
  • packages/client-runtime/src/connection/wakeups.ts
  • packages/client-runtime/src/operations/commands.performance.test.ts
  • packages/client-runtime/src/rpc/session.test.ts
  • packages/client-runtime/src/rpc/session.ts
  • packages/client-runtime/src/state/server.test.ts
  • packages/client-runtime/src/state/serverUsage.test.ts
  • packages/client-runtime/src/state/shell-sync.test.ts
  • packages/client-runtime/src/state/threads-sync.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

📝 Walkthrough

The connection runtime now counts incoming WebSocket frames and uses frame activity when timing session probes. Mobile wakeups distinguish brief refocus from background visits. The cached-endpoint socket timeout increases from three to six seconds.

Changes: Connection runtime

Layer / File(s) Summary
RPC received-frame counting
packages/client-runtime/src/rpc/session.ts, packages/client-runtime/src/rpc/session.test.ts, packages/client-runtime/src/connection/registry.test.ts, packages/client-runtime/src/operations/*.test.ts, packages/client-runtime/src/rpc/client.test.ts, packages/client-runtime/src/state/*.test.ts, apps/web/src/cloud/linkEnvironment.test.ts
RpcSession counts WebSocket message events and exposes the count through receivedFrames. Session mocks and fixtures provide the new field. A test checks that a Pong is counted while the config-stream consumer is blocked.
Probe timing and frame-driven extensions
packages/client-runtime/src/connection/supervisor.ts, packages/client-runtime/src/connection/supervisor.test.ts, docs/internals/connection-runtime.md
Quick probes use a six-second window. Incoming frames can extend a probe deadline, up to 15 seconds from probe start; retry signals can shorten the deadline and set its extension window. Tests cover probe timing, and the connection-runtime documentation describes the behavior.
Cached-endpoint authorization timeout
packages/client-runtime/src/authorization/service.ts, packages/client-runtime/src/authorization/layer.test.ts
The cached-endpoint socket timeout increases to six seconds. Tests cover delayed ticket responses and a pending cached-endpoint request that proceeds through relay bootstrap after six seconds.
Mobile refocus wakeups
apps/mobile/src/connection/app-state-wakeups.ts, apps/mobile/src/connection/app-state-wakeups.test.ts, apps/mobile/src/connection/platform.ts, packages/client-runtime/src/connection/wakeups.ts, packages/client-runtime/src/connection/supervisor.test.ts, packages/client-runtime/src/state/shell-sync.test.ts, packages/client-runtime/src/state/threads-sync.test.ts, docs/internals/connection-runtime.md
Mobile returns without a background timestamp now emit application-refocused. The runtime classifies it as an application-active wakeup. Tests cover retry behavior, connected-session behavior, and state wakeup handling.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant WebSocket
  participant RpcSession
  participant ConnectionSupervisor
  WebSocket->>RpcSession: message event increments received-frame count
  ConnectionSupervisor->>RpcSession: read receivedFrames at probe deadline
  alt New frames arrived and the 15-second cap remains
    ConnectionSupervisor->>ConnectionSupervisor: extend deadline by current probe window
  else No new frames or the cap is reached
    ConnectionSupervisor->>ConnectionSupervisor: handle probe timeout
  end
Loading

Possibly related PRs

  • pingdotgg/t3code#18704: Both pull requests change probe timing and mobile refocus behavior. This PR includes socket-lifetime frame counting, a 15-second probe cap, and related refocus handling.

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to aa7bb

No concrete merge-blocking issue remains; proceed with normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preventing healthy slow client connections from being dropped.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description is detailed and covers the problem, implementation, scope, verification results, limitations, trade-offs, and contributor or maintainer context. It does not use the template section he…
Full details: Description check

Explanation

The description is detailed and covers the problem, implementation, scope, verification results, limitations, trade-offs, and contributor or maintainer context. It does not use the template section headings, but the required information is present and the description is relevant and substantially complete.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

An inactive app keeps running, so a refocus never suspended the socket and
its streams are still live. The wakeup comment no longer calls the inactive
state brief, since a call can keep the app inactive for minutes.

Co-authored-by: Nick <1109914+nick-potts@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one as maintainer for batch ssh-relay-remote-10. I will preserve the contributor commits, merge current main, simplify the implementation and tests, and push follow-ups here. Slow-link and native verification limits will stay explicit; this remains unmerged pending human approval.

…nnections

# Conflicts:
#	packages/client-runtime/src/connection/supervisor.test.ts
#	packages/client-runtime/src/connection/supervisor.ts

This branch has not been deployed

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

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants