cloudflare / cloudflare/cloudflare-os

Chat subscription can silently stop during an agent turn until the page is refreshed

Open
#305 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
9.9k
Forks
1.2k
Avg merge
1d 20h
Merged PRs (30d)
107

Description

🤖

### Summary

After updating to commit [`1ef6020a42fbabb6d27dd1063db3a075ba95c974`](https://github.com/cloudflare/cloudflare-os/commit/1ef6020a42fbabb6d27dd1063db3a075ba95c974) (PR #275), the chat UI intermittently stops showing progress during an active agent turn. The agent continues running and its messages and code changes are persisted, but the browser receives no visible updates. Refreshing the page immediately reveals all of the activity that appeared to be missing.

The behavior appeared after the git/OT synchronization change in PR #275. It is intermittent, but longer turns with several tool calls and code edits made it easier to encounter.

### Steps to reproduce

1. Run Cloudflare OS at commit `1ef6020a42fbabb6d27dd1063db3a075ba95c974`.
2. Open a chat and ask the agent to perform a task involving several tool calls and code edits.
3. Keep the chat open while the agent works.
4. After some callbacks, observe that the UI may stop receiving new agent activity even though the agent continues running.
5. Refresh the page.

The messages and changes produced after the apparent stall are then present.

### Expected behavior

The active chat should continue receiving agent messages and code-change updates. If its subscription fails, the client should reconnect or show an error rather than remain silently stale.

### Actual behavior

The chat can stop updating without an error or reconnect. The underlying work is not lost; reloading establishes a new subscription and catches the UI up.

### Investigation

The new OT delivery path appears capable of silently removing the chat subscriber after one rejected callback:

```ts
// packages/workshop-backend/src/overseer.ts
subscriber.changeApplied(
row.chatId,
row.generation,
row.revision,
row.author,
row.change,
row.submission,
).catch(() => {
subscriber[Symbol.dispose]();
this.#chatSubscribers.delete(subscriber);
});
```

The retained-row replay path similarly uses `subscriber.changeApplied(...).catch(unsubscribe)`.

The frontend reconnect path, however, is attached to `overseer.onRpcBroken()`. Rejecting one subscriber callback does not necessarily break the surrounding Overseer RPC connection, so disposing/removing that subscriber can leave the page connected but permanently unsubscribed. The rejection is also swallowed, which makes the failure look like an agent stall.

This would explain the observed recovery after refresh: the work remains durable and a fresh `subscribeToChat()` call replays the current state.

### Mitigation tested

As a diagnostic, I changed only the live and replay `changeApplied` rejection handlers to log the error and keep the subscription. Dead clients were still removed through the existing `subscriber.onRpcBroken(() => unsubscribe())` handler.

With that change:

- a focused regression test confirmed that a subscriber which rejects one `changeApplied` callback is still called for the next row;
- the existing backend chat-change suite passed (35 tests);
- the issue could no longer be reproduced manually during an equivalent agent workflow.

Because the original issue is intermittent, this does not prove why the callback rejected. It does strongly suggest that treating any individual callback rejection as a permanent, silent unsubscription turns a recoverable or transient callback failure into the visible stall.

### Suggested fix

Please consider making an individual `changeApplied` rejection recoverable. Possible approaches include:

- retaining the subscriber and logging/reporting the callback failure, relying on `onRpcBroken` to remove genuinely disconnected clients;
- explicitly closing the parent RPC connection so the frontend's existing reconnect path runs; or
- adding a subscription-level reconnect/error signal and having the client resubscribe.

A regression test should verify that one rejected `changeApplied` delivery either permits a later delivery or causes an observable resubscription, rather than silently disabling all subsequent chat updates.

### Environment

- Cloudflare OS commit: `1ef6020a42fbabb6d27dd1063db3a075ba95c974`
- Commit title: `Convert backing storage to git, change sync to OT, editor widget to CodeMirror (#275)`
- Node.js: `24.19.0`
- pnpm: `11.17.0`

Contributor guide

Open the contributing guide

Research direction

Start in packages/workshop-backend/src/overseer.ts, reading both live and retained-row replay changeApplied rejection handlers alongside the existing onRpcBroken path. Run the focused regression test and the backend chat-change suite. Done means one rejected callback no longer silently prevents later updates, with either continued delivery or an observable resubscription/error.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
backend, frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.