Skip to content

fix(webchat): reattach a reloaded browser to its in-flight turn - #1539

Merged
frozenluo merged 4 commits into
mainfrom
fix/webchat-reload-reattach
Aug 27, 2026
Merged

fix(webchat): reattach a reloaded browser to its in-flight turn#1539
frozenluo merged 4 commits into
mainfrom
fix/webchat-reload-reattach

Conversation

@frozenluo

Copy link
Copy Markdown
Contributor

What

A page refresh during a streaming webchat turn lost both the typing indicator and the streamed reply until the turn completed and persisted. This adds cold-load stream discovery — a read-only attach probe — on top of the existing daemon replay/resume machinery, so a reloaded console reattaches to the in-flight turn and keeps streaming.

Why

All live-turn state (busy flag, per-lane ordered cursors, streamed steps) lives only in browser memory, and the webchat reply is written to the transcript only at turn end. The existing resume op requires the exact turnId + cursor the reload just wiped, and the relay ready frame carries no active-turn info — so a cold-loaded client had no way back into the stream.

How

  • protocol: new RelayWebchatOp member attach { agentId? }; RdAck gains optional generation; new daemon capability webchat-attach-v1.
  • daemon: WebchatTransport.probeWebchatStream(agentId, conversationId) names the live (un-completed) stream — turnId + current resume generation. Trimmed replay heads refuse with stream_gap (a from-scratch replay can't rebuild them; the transcript covers it at turn end); idle/completed answer stream_not_found. Capability advertised on rd/hello.
  • relay: parses the browser {type:'attach'} envelope, forwards per participant like resume, answers {type:'attached'} with the verdict (generation passed through). Fails closed on daemons without the capability, and every miss — unsupported, offline, delivery failure — is a quiet per-agent refusal, never an error frame.
  • web: opening a webchat session detail connects the conversation socket and probes each verified roster member on ready. An accepted attached recreates the stream lane (cursor generation seeded from the daemon's, so the follow-up resume outruns pre-reload generations), restores the busy indicator, and replays the reply from scratch through the ordinary resume path. Reconnects, done handling, and dedup against the persisted transcript all reuse the existing machinery. Bare error frames now only surface a warning step while a turn is actually in flight — an older relay answers the probe with unrecognized frame, which must stay silent on an idle transcript.

Design note added to docs/designs/webchat-multi-agents.md §6.1.

Rolling compatibility

  • old daemon + new relay: probe refused locally via the capability gate.
  • old relay + new web: probe answered with an error frame, which the idle guard swallows.
  • Either way the behavior degrades to exactly the pre-change one (reply appears after turn end via the transcript tail refresh).

Testing

  • daemon-webchat.test.ts: probe verdicts (live hit incl. generation, idle miss, trimmed-replay gap, completed turn) + full attach→resume replay; file green (46 tests).
  • relay-browser-connection.test.ts: envelope parsing, forwarding + attached verdict with generation, capability-gate refusal, offline quiet refusal (548 package tests green).
  • protocol schema round-trips for the new op and ack field (451 green).
  • web suite fully green (2044 tests); typecheck across all four packages; repo lint clean.

🤖 Generated with Claude Code

A page refresh mid-turn lost the typing indicator and the streamed
reply: all live-turn state (busy flag, stream-lane cursors, streamed
steps) lived only in browser memory, the persisted transcript gets the
reply only at turn end, and nothing let a cold-loaded client rediscover
the stream — resume requires the exact turnId the reload just wiped.

Add cold-load stream discovery on the existing replay machinery:

- protocol: new RelayWebchatOp `attach { agentId? }` (read-only probe),
  RdAck gains optional `generation`, capability `webchat-attach-v1`
- daemon: `probeWebchatStream` names the live stream for a
  (conversation, agent) — turnId plus current resume generation;
  refuses trimmed-replay (`stream_gap`) and completed/idle streams
- relay: parses `{type:'attach'}`, forwards it per participant gated on
  the daemon capability, answers `{type:'attached'}`; every miss is a
  quiet per-agent refusal, never an error frame
- web: opening a webchat session detail probes the roster over the
  conversation socket; a hit recreates the lane (generation seeded from
  the daemon), restores the busy indicator, and replays the reply from
  scratch through the ordinary resume path. Idle `error` frames no
  longer push a warning step (an older relay answers the probe that way).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-scoped addition. Confirmed the actual PR diff via three-dot merge-base comparison (the branch predates #1537, so a naive two-dot diff falsely shows acknowledgeTrigger being reverted — it isn't part of this PR).

Traced the new attach probe end-to-end:

  • Daemon probeWebchatStream correctly reuses the existing webchatStreamKey(turnId, agentId) scheme, refuses stream_gap/stream_not_found consistently with resumeWebchatStream.
  • Generation seeding is monotonic: probe returns the daemon's current resumeGeneration, web seeds the cursor to that value, and sendLaneResume increments before sending — so the resume always lands strictly ahead of the stored generation, matching resumeWebchatStream's generation <= stream.resumeGeneration rejection.
  • Relay fails closed on capability/offline/delivery-failure, always via quiet attached{accepted:false}, never the legacy error frame — verified against the new tests.
  • Web's pgAttach/connect reuse: an existing live connection skips the probe entirely (by design — nothing was lost), so no risk of double-attaching on a non-reload mount.
  • afterIndex: -1 on the follow-up resume matches "replay from scratch," and the schema's min(-1) bound.

No blocking issues found.

sent by review-bot-fast (Claude Agent · sonnet) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attach/resume plumbing is well-fenced across protocol, daemon, and relay, but there is one blocking UI reconciliation issue: a cold-attached replay has neither the local prompt step nor a canonical post id that the existing live-step reconciler requires. Once the turn finishes and the transcript tail loads the persisted reply, the replayed live reply remains and the session renders the answer twice until another page reload. Please give cold-attached turns a durable reconciliation/retirement anchor.

I could not independently rerun the focused package tests in this isolated checkout because dependencies are absent and Corepack's pinned-pnpm download is blocked by the runner; git diff --check passed.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

if (a?.accepted === true && typeof a.turnId === 'string' && typeof a.agentId === 'string') {
const key = laneKey(id, a.agentId)
if (!streamCursors.current.has(key) && !finishedFor(id, a.turnId)?.has(a.agentId)) {
const cursor = createWebchatCursor<WebchatOutput, WebchatDone>(a.turnId)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve a reconciliation anchor for the cold-attached replay. This cursor replays only agent output into wcSteps; unlike sendTurn, it never pushes the local kind: 'msg' prompt, and streamed reply steps have no postId. After done clears busy and the transcript tail refreshes, reconcilePersistedLiveSteps finds neither of its anchors (postId or a prompt-led turn), so it retains these steps while the persisted agent row is rendered too. Every successful cold attach therefore shows the completed answer twice until the page is reloaded again. Please carry an identity that lets the persisted reply retire the reattached live steps (or otherwise remove them only after the canonical row is present).

A cold `attach` replays the live turn's agent output with no local `kind:'msg'`
prompt step and no `postId`, so `reconcilePersistedLiveSteps` finds neither of
its anchors: the replayed reply survived the transcript-tail refresh and the
session rendered the completed answer twice until another reload.

Take the canonical reply `postId` off the turn's own `rd/webchat-post` frame
(routed by conversation, so a reloaded socket receives it) and stamp it onto the
replayed steps — the same anchor #753 gave agent-initiated posts. Whichever of
that frame and the lane's `done` lands second does the stamping: the success
path posts before `done`, the failure path sends `done` first. An
agent-initiated post is a different turn's reply and never anchors the attached
one. The existing exact-`postId` arm then retires the replay when the persisted
row arrives.

Also add the new `webchat-attach-v1` capability to the `rd/hello` expectation in
relay-client.test.ts, which asserts the advertised list exactly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified checkout is exactly the trusted head 0611253. This revision's sole change (commit 0611253) directly fixes the duplicate-render bug flagged on the prior revision: a cold-attached replay has no local prompt step, so reconcilePersistedLiveSteps's turn-shaped arm couldn't retire it once the transcript tail caught up.

The fix stamps the canonical reply postId (from the turn's own rd/webchat-post frame, which is fanned out per-conversation to every relay the daemon holds — confirmed this is pre-existing infra, so a reloaded socket does receive it) onto the replayed cold-attach steps via coldAttached/anchorColdTurn, then lets the existing exact-postId reconciliation arm retire them. Checked both orderings:

  • success path: post arrives before doneanchorColdTurn no-ops until done sets cold.done, then stamps.
  • failure path: done arrives first → stamped once the post frame lands.
    Both are covered by new tests in PlaygroundSend.test.tsx. Confirmed the m.initiator === 'agent' guard correctly excludes agent-initiated posts (a different turn's reply) from anchoring, and that a normal (non-cold-attached) turn's own reply post is unaffected since coldAttached only ever gets an entry via the attached handler.

One narrow, non-blocking edge case: rejectLane (used for a rejected fresh turn op) doesn't clear a same-key coldAttached entry, unlike failStream. In practice this can't produce a wrong stamp — anchorColdTurn matches on the stale entry's original turnId, which a subsequent unrelated turn won't share — and the busy flag set at cold-attach time should prevent a new turn send on the same lane while the replay is still in flight. Not worth blocking on.

No blocking issues. Approving this revision.

sent by review-bot-fast (Claude Agent · sonnet) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The synchronized commit fixes the original same-relay reconciliation problem: replayed steps now receive the canonical post ID regardless of whether done or rd/webchat-post arrives first. One blocking multi-relay gap remains, though. A browser-initiated turn's canonical post sink stays bound to the relay connection that admitted the turn, while resume only rebinds the stream transport. If a reload connects through another relay instance, the new browser receives replay and done but not the canonical post ID, so the replayed live steps still cannot reconcile with the persisted reply and the duplicate remains. Please rebind the post sink on resume or carry the durable reply identity on the replay/done path. Review performed against the exact synchronized commit through GitHub data because this generation did not provide a trusted local checkout; local tests were therefore skipped.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

// postId as that turn's retirement anchor. An agent-initiated post is
// a DIFFERENT turn's reply and must not anchor the attached one.
const coldKey = laneKey(id, agentId)
const cold = m.initiator === 'agent' ? undefined : coldAttached.current.get(coldKey)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Rebind the canonical post path when resuming through another relay. The daemon's original postSink is bound to the RelayClient that admitted the turn (handleMsg builds it from this.sendWebchatPost), while resumeWebchatStream only replaces stream.transport. If a reload's token lands on relay B, output and done replay through B but the human-turn rd/webchat-post still goes to relay A, whose browser sink is gone. This handler on the reloaded socket never sees postId, so coldAttached cannot stamp the replay and the completed reply still duplicates after transcript refresh. Please carry the reply identity on the replay/done path or rebind postSink along with the stream transport.

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified checkout is exactly the trusted head c02fc88. This revision's sole commit (c02fc88) directly fixes the cross-relay reconciliation gap flagged on the prior revision: a browser turn's reply post previously went back only down the originating relay socket (RelayClient.sendWebchatPost called directly via the threaded post callback), so a reload landing on a different relay instance never received the canonical postId needed to retire the cold-attached replay.

The fix removes that per-call postSink/post plumbing entirely (RelayClientDeps.onRelayMsg, handleRelayMsg, dispatchRelayOp, dispatchWebchatTurn all drop the parameter) and instead unconditionally sets stream.postSink = (p) => this.host.sendWebchatPost(p) — the exact same daemon-wide RelayManager.sendWebchatPost fan-out already used (and already audited) for agent-initiated wakes (#753). Traced the chain:

  • RelayManager.sendWebchatPost broadcasts to every ready relay client this daemon holds.
  • Each relay's WebchatRouter.deliverPost no-ops for a conversationId it doesn't hold a browser sink for, so this is safe fan-out, not a leak.
  • Peer-daemon context fan-out (relay's onWebchatPost) is unchanged — same mechanism as before, just reached uniformly now regardless of turn origin.

Verified all call-site signatures line up after the parameter removal (dispatchWebchatTurn's trailing args shifted from (remoteMcp, mentions, post, postSink, worktree) to (remoteMcp, mentions, post, worktree), and the daemon.ts call site drops the extra post arg accordingly — no off-by-one). New test (daemon-webchat.test.ts) asserts the reply post now goes through this.relays.sendWebchatPost rather than the old per-socket path; existing continuation/turn-refresh tests updated consistently to the new two-arg handleRelayMsg signature.

No blocking issues. Approving this revision.

sent by review-bot-fast (Claude Agent · sonnet) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. This revision resolves the cross-relay reconciliation gap from the previous review by routing browser-turn canonical posts through RelayManager.sendWebchatPost, so a reloaded browser receives the reply postId even when it reconnects through another relay. Non-owning relays safely no-op at browser delivery, while repeated peer-context deliveries remain deduplicated by the canonical post ID and activation rendezvous. The updated tests cover daemon-wide post routing, and the existing relay-manager tests cover broadcasting only to READY relay connections. I found no blocking issues. I could not execute the focused daemon tests in this restricted checkout because pnpm failed before test startup while opening its cache database; static inspection and git diff --check completed successfully.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified checkout is exactly the trusted merge of base 20f574c7 and head 3f695d79.

This revision is a merge-with-main sync (3f695d79, "Merge origin/main into fix/webchat-reload-reattach") with no new commits of its own on the PR side — its first parent is c02fc885 (the previously reviewed and approved tip) and its second parent is the current origin/main. The raw two-dot stat against the old tip looks huge (~220 files) purely because ~30 unrelated PRs landed on main in the interim; that's not this PR's diff.

Confirmed via the three-dot (merge-base) diff against the new base — which equals git merge-base exactly, so it isolates only this branch's own changes — that every webchat-attach file (protocol/relay-daemon.ts, daemon/webchat/transport.ts, daemon/daemon.ts, daemon/cp/relay-client.ts, relay/relay-browser-connection.ts, web/PlaygroundProvider.tsx, web/SessionDetailView.tsx, docs/designs/webchat-multi-agents.md) is byte-for-byte identical to the content already reviewed and approved across the prior two revisions (0611253, c02fc88). The merge picked up unrelated main-side changes to the same files (e.g. dropWaitNotices/retireWaitNotice notice handling, acknowledgeTrigger reaction chrome) but resolved cleanly alongside the PR's pgAttach/coldAttached/anchorColdTurn logic with no conflict-resolution regressions — e.g. the applyStreamResult useCallback dependency array correctly lists both retireWaitNotice (main) and anchorColdTurn (this PR).

No new findings. Approving this revision.

sent by review-bot-fast (Claude Agent · sonnet) · open in session

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The new head is a merge of the previously approved c02fc885 feature revision with base 20f574c7; the three PR commits are unchanged. I inspected the exact 3f695d79 tree through GitHub and checked the overlapping merge areas: cold attach still restores the lane and seeds the resume generation, replayed steps still receive the canonical reply postId after done/post ordering settles, browser-turn posts still fan daemon-wide for cross-relay reloads, and the updated base's notice-lane handling remains intact. I found no blocking regression from the merge. No local execution was performed because this review generation supplied no trusted checkout; at review time several GitHub checks were still running, while the completed checks were successful.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

@frozenluo
frozenluo merged commit b3666e2 into main Aug 27, 2026
14 checks passed
@frozenluo
frozenluo deleted the fix/webchat-reload-reattach branch August 27, 2026 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant