fix(mac): connection-stack resilience — lenient decode, honest heartbeat, RPC deadlines, watchdog repair - #443
Merged
Conversation
…t wedge the app Both subscription streams replay from a client-held resume cursor, so a decode that throws is not a dropped row — it is a permanent outage. Shell stream: `OrchestrationShellStreamEvent.init(from:)` threw on any unrecognized `kind` (and on any nested decode failure). The throw propagated out of `consumeShell` into `runSocketSession`'s catch, which reconnected and re-issued `subscribeShell(afterSequence: lastShellSequence)` — a cursor that was never advanced past the failing event, because the decode died before `handleShellItem` ran. The server replayed the same event, which failed identically: a Mac one release behind a server that emits a new shell event kind flaps Connecting/Reconnecting forever with no escape. Thread stream: `OrchestrationThreadStreamItem.init(from:)` threw the same way, killing the per-thread request id and leaving `lastThreadSequence[threadID]` parked in front of the poison item, so every reopen of that chat failed identically and the timeline was permanently un-loadable. Both unions now decode unknown/undecodable items as an `.unrecognized` case carrying the sequence, mirroring `OrchestrationEvent`'s existing `.other` path, and `LiveBackend` advances the cursor past them. A `snapshot` that fails to decode still throws on purpose: there is no timeline without it, and the throw is what fails the parked `timeline()` waiters instead of leaving the chat on a spinner. The collections inside a snapshot now decode per row (`LenientDecoding.swift`), so one undecodable thread, message, or activity costs that row instead of the only frame that carries every other one. Skips are reported to stderr rather than vanishing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tions `awaitingPong` was set true on every 5s tick and cleared only by an inbound `Pong`, so the connection was declared dead after ~10s without one. But the receive loop is parked inside `socket.receive()` until a WHOLE WebSocket message has arrived, and the incoming cap is 64 MiB: a multi-MiB thread snapshot on a slow relay or LAN link legitimately holds the loop past two ticks with bytes flowing the entire time. The watchdog then tore the socket down with `T3Error.pingTimeout`, `runSocketSession` reconnected, `runSubscriptions` re-issued the identical `subscribeThread`/`subscribeShell`, and the same oversized frame was killed again ~10s later — an unbreakable Connecting → Reconnecting(N) flap, with every thread status frozen, on exactly the threads whose history made them large. This is the most likely root cause of "can't connect over remote". Liveness is now evidence-based rather than Pong-based: any inbound message stamps `lastInboundAt` and clears `awaitingPong` (a frame this build cannot even parse still proves the peer is alive, so the stamp happens before decoding). A Ping is only sent to a socket that has been quiet for a full interval, and death is only declared after `inboundSilenceTimeout` (30s) of total silence with a Ping already outstanding. The verdict itself is a pure `heartbeatAction` function so the policy is tested without waiting on real 5s ticks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…in the UI `RpcConnection.request` registered a pending continuation and enqueued the frame with no deadline, and nothing above it added one either. The only liveness signal was the transport heartbeat — which a server whose RPC handlers are wedged (a held SQLite lock, a stuck migration, an exhausted fiber pool) still answers, so the watchdog never fires. The sidecar boots, passes its readiness probe, accepts the socket, then blocks in `server.getConfig`: the UI sits on `.connecting` forever with no error, no retry and no way to escalate — "can't connect" with a running server. Every unary RPC had the same exposure (send, approve, checkpoint restore). Unary requests now carry a deadline (180s by default — a wedge-breaker, not a latency budget, since some RPCs do real git/checkpoint work). Expiry fails that one continuation with `T3Error.requestTimeout(tag:)` and sends the wire `Interrupt` so the server stops working on it; the connection itself survives, which is the honest reading of "the transport is fine and one handler is stuck". Session setup gets a much shorter one: `getConfig` is raced against a 20s deadline via the new `withDeadline` primitive, so a wedged server turns into a failed attempt that the existing backoff/reconnect loop can retry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t session `timeline(threadID:)` parks a continuation in `snapshotWaiters` when no cached timeline exists, and `failAllSnapshotWaiters` ran only from `stop()`. Every early return out of `runSocketSession` — the remote `.unauthorized` branch, the missing-auth-client guard, the three stale-generation returns — left those continuations parked forever, and `failThreadWaitersIfSocketConnectionOwned` is already a no-op by then because the connection generation was invalidated on the way out. Concretely: on a remote device whose pairing token expired, opening a thread that has never been loaded leaves `isLoadingTimeline` true and the chat on a spinner that survives re-pairing, because nothing ever resolves the waiter. A `defer` at the top of `runSocketSession` covers all of those exits, and `teardownSocketSession` fails them too — it cancels the session task first, so the defer correctly declines (a successor session may be starting) and nothing else would. A failed waiter is recoverable: `performTimelineLoad` leaves `hasLoadedTimeline` false, so the next selection retries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…zing the chat A `subscribeThread` stream can end on its own while the socket stays healthy — a server-side stream completion, or an error that fails only that request id. The old code deregistered the slot on the stated assumption that "the next timeline() re-subscribes", but nothing calls `timeline()` again for an already-open thread: `AppModel.loadTimelineIfNeeded` returns early on `guard !state.hasLoadedTimeline`, and `hasLoadedTimeline` is only cleared by LRU eviction. Failure: the agent is mid-turn, one thread stream ends, and the chat stops receiving messages and deltas entirely — no spinner, no error, sidebar still updating from the separate shell stream — until the user navigates far enough away to evict the thread and comes back. LiveBackend now owns the recovery: any id still in `activeThreadIDs` re-subscribes from inside the backend with `ServerProcess.backoffDelay` between attempts, and the resume cursor makes it a cheap resume rather than a fresh snapshot. The attempt counter resets as soon as a stream delivers anything, so a single blip does not leave the thread on the 10s cap. Ids nobody is watching still just free their slot. The cursor is read after the backoff sleep, so an eviction during the delay correctly downgrades the resume to the full snapshot `timeline()`'s waiters need. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…and retract itself Three defects in one mechanism, all of which left a thread visibly wrong with no way back. 1. Zombie after a socket reconnect. `cancelAllRunningLivenessChecks()` ran only from `stop()` and `teardownSocketSession()` — never from the in-session reconnect paths in `runSocketSession`. The monitor task kept polling through the OLD, disconnected `T3Client`; every `getThreadLiveness` threw `notConnected`, `monitorRunningLiveness` swallowed it and slept, and the turn key never changed — so `scheduleRunningLivenessCheck` hit `guard existingCheck.turnKey != turnKey` and refused to arm a replacement on the fresh client. A thread whose terminal event was lost during the blip spun "Thinking" forever. Checks now carry their socket connection generation: a stale one self-cancels on its next `isCurrent` and no longer blocks a re-arm. 2. The stale mark was permanent. The monitor returned right after `onStale`, and re-arming the same turn key was barred, so `markRunningThreadLive` was unreachable — a merely slow relay (two probes missing a delayed terminal event) marked the thread for the rest of its life. The watch now continues past a stale verdict so a later positive probe retracts it. 3. It claimed a failure that never happened. Marking wrote a client-side `.error`, re-applied on every subsequent shell upsert. That fired a red "X failed" notification for a healthy turn, and because `.error` is not "actively working", the real completion arriving later as `.error -> .done` delivered no finished notification either — a failure alert and no completion alert, for a turn that succeeded. The watchdog now records a *stall*, which is what it actually observed: the status stays whatever the server projects, the verdict folds into `ThreadHealth.stalled` alongside the server's own `session.health` signal (same meaning, one field, one UI vocabulary), `AgentNotificationPolicy` reports `.stalled`, and the genuine completion still reads as running -> done. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ervers The child pid lived only in `ServerProcess.process`, in memory, with no pidfile, no startup sweep, and no single-instance guard anywhere in SidecarKit or the server. Foundation does not kill a child when its `Process` reference is dropped, so an app crash, SIGKILL, or force-quit left the node sidecar running — holding provider sessions and writing `~/Library/Application Support/SergeCode`. On relaunch `SidecarConfig` picks a fresh ephemeral port, so the orphan is invisible: the new server spawns happily and both processes drive agents and mutate the same SQLite state. Force-quit during a running turn and relaunch, and the old agent keeps running and writing thread state the new UI's server also writes — duplicated turns, statuses that flip back, "backend acting weird". `launch()` now records the child in `<baseDir>/sidecar.pid` and `start()` terminates a previous run still holding it (SIGTERM, then SIGKILL after the existing 2s grace) before spawning. A clean `stop()` removes the file, so the normal path sweeps nothing. The record carries the kernel-reported start time, not just the pid, and `orphanVerdict` refuses to signal anything whose start time no longer matches: pids are recycled, and SIGKILLing an unrelated process is much worse than leaving an orphan alive. That rule is a pure function so the identity cases (no record, dead pid, recycled pid, pid <= 1, genuine match) are tested without spawning anything. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… consumers, lost crash logs Four smaller reconnect-path defects, all with the same shape: state that outlives the connection it belonged to. Duplicated assistant text after a reconnect. `pendingAssistantDeltas` was cleared only by `stop()` and the per-thread timeline reset, so deltas buffered in the 33ms flush window before a socket drop flushed into the NEXT session — after the resubscribe had already emitted a `timelineReset` carrying the server's authoritative text. `AppModel.applyDelta` appended them on top, so the tail of the assistant reply appeared twice. Every connection-generation change now discards the buffer. Stranded event-stream consumers. `events()` overwrote `eventContinuation` and `stop()` nilled it, neither finishing the old one, so a previously vended stream never ended and its consumer parked for the process lifetime holding its captures. Only `AppModel.prepareForTermination` cancelling `eventTask` first hid this, and nothing in the API enforced that ordering. A `Chunk` for a non-streaming request failed the RPC locally but never told the server, which kept producing the whole stream while this side failed every chunk through the same miss path — wasted bandwidth on exactly the relay links that can least afford it. It now sends `Interrupt` alongside the mandatory `Ack`. Sidecar log rotation destroyed the crash output you need. `makeLogHandles` ran for every backoff restart and kept exactly one generation, so two restarts into a crash loop both `stderr.log` and `stderr.log.1` held only the last two identical uninformative runs. Rotation is now tied to a user-initiated start; crash-restarts append, keeping the first explanatory failure at the top of one file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
13 tasks
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
Connection-stack batch from the 2026-08-02 bug hunt — the "backend connection issues" cluster. All three P0 wedges plus the watchdog/reconnect defects:
.error— status stays server-projected, the false "failed" banner is gone, and the real completion still lands.Skipped deliberately (product decisions): local-reconnect SIGTERM confirmation, idle-sleep assertion scoping, bounded-retry escalation UX. Noted in session inventory.
Note: touches
AgentNotificationPolicy/LiveBackendShellResumeTestsalso touched by #442 — whichever merges second needs a trivial conflict pass.Area
apps/mac— native macOS appapps/windowsapps/mobileapps/serverRelease size
size:XSsize:Ssize:Msize:Lsize:XLVerification
pnpm run verify --allafter merging current main: all 5 steps pass, exit 0🤖 Generated with Claude Code