refactor(site/src/pages/AgentsPage): use createReconnectingWebSocket in git and workspace watchers#23736
Open
DanielleMaywood wants to merge 3 commits intomainfrom
Open
refactor(site/src/pages/AgentsPage): use createReconnectingWebSocket in git and workspace watchers#23736DanielleMaywood wants to merge 3 commits intomainfrom
DanielleMaywood wants to merge 3 commits intomainfrom
Conversation
…it and workspace watchers Replace hand-rolled reconnection logic in useGitWatcher with the shared createReconnectingWebSocket utility, eliminating ~60 lines of duplicated backoff/timer/disposal code. Add reconnection to the workspace watcher in AgentDetail which previously had none — a dropped socket silently killed the watcher.
github-actions
bot
added
the
community
DanielleMaywood
changed the title
The setQueryData updater compared prev.latest_build.resources with === (referential equality). Since each WebSocket message deserializes fresh JSON, resources is always a new object, so the check never short- circuited. Replace with comparisons of the specific agent fields the UI actually reads: id, status, name, and expanded_directory.
… onOpen The Storybook WebSocket mock never fires an 'open' event (stories only declare message events). The refactored code set socketRef.current in onOpen, so the message handler's stale socket guard rejected every message. Move the assignment into the connect factory where the old code had it, so messages work even when open never fires.
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.
Replace hand-rolled reconnection logic in
useGitWatcherwith the sharedcreateReconnectingWebSocketutility, and add reconnection support to theworkspace watcher in
AgentDetailwhich previously had none.Changes
useGitWatcher.ts— RemovedreconnectAttemptRef,reconnectTimerRef,disposedRef,MAX_BACKOFF_MS, and the manualconnect()function with itsopen/close/errorevent handlers and backoff timer logic. Replaced with asingle
createReconnectingWebSocketcall. ThesocketRefis retained forbidirectional
sendMessage/refreshsupport, populated viaonOpen. Backofftiming is identical (
maxMs: 30_000). All 16 existing tests pass unchanged.AgentDetail.tsx— Wrapped the workspace watcheruseEffectincreateReconnectingWebSocket. Previously, a dropped socket silently killed thewatcher with no reconnection. Now it reconnects with exponential backoff and
invalidates the workspace query on reconnect to fetch fresh data (matching the
pattern the chat list watcher already uses).
Decision log
useDesktopConnectionleft alone: RFB takes socket ownership, customreconnection features (max retries, stable timer, connect timeout) do not
fit the utility API.
createReconnectingWebSocket— no changes needed.MessageEventcast in the git watcher message handler is necessarybecause the
Closableinterface types event handler args asunknown.socketRef.current !== socket) kept in the messagehandler as defense-in-depth even though
createReconnectingWebSocketcloses old sockets before creating new ones.