🤖 fix: surface slow server responses in the connection indicator - #4059
Conversation
The liveness check skipped its ping whenever any inbound WebSocket frame arrived in the last 3s and treated that as healthy, never measured round-trip time, and its pong-frame exclusion never matched the real oRPC reply, so "degraded" was unreachable while the server answered at all. Probe round-trip time instead: two consecutive slow observations (RTT or pending age over 2s) mark the connection degraded and carry the latency into the toast; a fast probe restores connected. Inbound frames only suppress the forced reconnect, which now requires total silence with a probe outstanding for 15s. Electron MessagePort clients get the same RTT-based degraded detection without a reconnect path.
Remote UAT found the total-silence reconnect went blank: connect() set "connecting", which the toast hides (it is meant for the initial load), and the "reconnecting" state was only set by the socket-close path. Any connect after a prior successful connection now reports "reconnecting" until the replacement socket authenticates.
This comment has been minimized.
This comment has been minimized.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed93ee7b1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…raded consumers - Reconnect after three consecutive rejected liveness probes so a lost session still reaches auth_required instead of pinning degraded. - Publish the live latency through a narrow ConnectionLatencyContext so per-tick updates re-render only the indicator, not every useAPI() consumer; degraded no longer carries latencyMs. - Keep the toast's aria-live text stable by hiding the changing figure from the accessibility tree. - ExperimentsContext and RightSidebar key on api availability, so a degraded (slow but usable) connection is not treated as offline.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ea532cdbe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A throttled or suspended tab can fire its next tick past MAX_PROBE_AGE_MS; the stalled probe was then replaced without the total-silence check, so a half-open socket missed its reconnect. Check silence first.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7624cb65d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Date.now() can step backwards on a clock correction, which made a stalled probe look young (no degrade, no reconnect) or a slow probe look fast. Use performance.now() for probe send times, RTT, and the silence window, measured from connect time until the first frame arrives.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28b5aa7e9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A prompt rejection is an answer, not slowness; recording it as a slow observation showed "slow to respond (0ms)" for one interval before the rejected-probe reconnect. Rejections now only feed the reconnect counter.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This comment has been minimized.
This comment has been minimized.
## Summary Version bump for the v0.28.4 patch release. The headline change since v0.28.3 is Gemini 3.8 Flash becoming the default Gemini Flash model (coder#4060). The release also carries browser Login with Coder on remote Xum servers (coder#4047), the opt-in project bundle for settings backup (coder#4043), the connection-indicator slow-response surfacing (coder#4059), send-queue and terminal-wake fixes (coder#4053, coder#4052), and the Effect Phase 11 runtime refactors. ## Implementation Bumped with `node ./scripts/set-package-version.js 0.28.4` so the root `package.json` and the legacy `packages/mux-compat` forwarding package stay version-locked (the v0.28.3 bump missed the compat package and broke `Test / Unit` on main, fixed in coder#4048). `src/common/compat/productIdentity.test.ts` passes locally. After this PR merges, the `v0.28.4` tag will be applied to the squash commit and the GitHub Release published to trigger the desktop/npm/docker pipelines. --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `medium` • Cost: `$0.00`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=medium costs=0.00 -->
Summary
The browser connection indicator never reflected a slow backend: with agent streams flowing, the liveness probe was skipped entirely, round-trip time was never measured, and a dead pong-frame heuristic reset the failure counter, so
degradedwas unreachable while the server answered at all. The liveness check now measures probe RTT and shows "Server is slow to respond (N.Ns); messages may be delayed" while the backend is slow, and a forced reconnect stays visible as "Reconnecting to server".Background
On a memory-starved host (cgroup at its 32 GB limit, heavy reclaim, one OOM kill) message sends took many seconds and pre-stream preparation ran 2 s to over 400 s, yet the UI stayed on "connected". Three defects in
src/browser/contexts/API.tsxcombined:hasRecentInboundTrafficskipped the ping and marked the connection healthy whenever any frame arrived in the last 3 s, so an active stream hid the stall.isLikelyOrpcPongResponseFramecheckedpayload.p.b === "pong", but the server returnsPong: <input>wrapped in oRPC's{json, meta}, so late pongs counted as inbound traffic and reset the failure counter on the next tick.Implementation
degraded, and one fast probe restoresconnected.ConnectionLatencyContext(useConnectionLatencyMs) rather than the API context, so per-tick updates re-render only the toast and everyuseAPI()consumer keeps a referentially stable value.degradeditself carries no latency field.auth_required; rejections never feed the latency indicator. The silence check runs before an overdue probe is aged out, so a throttled tab's late tick still reconnects a half-open socket. All probe timing uses the monotonicperformance.now(), so a wall-clock correction cannot make a stalled probe look young. The pong-frame parser and in-flight counter are deleted.connect()after a prior successful connection reportsreconnectinginstead ofconnecting, so the banner stays up through the replacement socket's handshake instead of going blank (found by remote UAT).ConnectionStatusToastrenders the latency withformatDuration(…, "precise")insidecounter-nums; the changing figure isaria-hiddenso the polite live region announces the warning once.ExperimentsContextandRightSidebarkey onapiavailability instead ofstatus === "connected", so a degraded (slow but usable) connection is not treated as offline.Validation
API.test.tsxreplaces the three tests that encoded the old "skip the probe while traffic flows" rationale with behavioral ones: slow pongs under stream traffic degrade without reconnecting, a fast probe recovers, a stalled probe stays single and grows the reported latency without republishing the API context value, total silence reconnects throughreconnecting, repeated rejected probes reconnect intoauth_requiredwithout ever showing the slow indicator, a late tick past the probe age limit reconnects instead of re-probing, a backward wall-clock step still degrades a stalled probe, and fast pongs under traffic keep probing. Each guard was checked red with its fix toggled off.Risks
Medium, scoped to the browser connection state machine. A slow-but-alive server no longer triggers a reconnect at all (previously three 3 s timeouts did when no frames arrived), so a server that answers every ping in 3 s to 15 s shows the slow toast instead of reconnecting; this is intentional to avoid reconnect storms against an overloaded backend. Any connect after a prior successful connection now shows "Reconnecting to server" rather than nothing, including the Retry button path. Detection latency is 7 s to 13 s after slowness starts (5 s interval, two observations).
Generated with
xum• Model:anthropic:claude-fable-5-1• Thinking:xhigh• Cost:$51.20