Skip to content

fix(board): reliability fixes for SSE lifecycle, watchdog, transport, and rendering#3563

Merged
dgageot merged 2 commits into
docker:mainfrom
dgageot:worktree-board-bce352e83261497d
Jul 10, 2026
Merged

fix(board): reliability fixes for SSE lifecycle, watchdog, transport, and rendering#3563
dgageot merged 2 commits into
docker:mainfrom
dgageot:worktree-board-bce352e83261497d

Conversation

@dgageot

@dgageot dgageot commented Jul 9, 2026

Copy link
Copy Markdown
Member

The board's SSE event stream had several reliability gaps that caused missed events, stalled connections, and occasional rendering glitches under real-world network conditions.

This change tightens the full SSE lifecycle: the watchdog timer now resets on every received line rather than only on data frames, so comment and keep-alive lines properly defer the timeout. Keep-alive probes are disabled on the Unix domain socket transport, which doesn't need them and was generating spurious traffic. The heartbeat goroutine is now drained before the function returns, preventing a subtle goroutine leak on reconnect. The title-truncation helper now trims both halves of a split title consistently. A defer replaces the manual unlock in SetSamplingWithToolsHandler, fixing the mutex lint warning. Regression tests cover the watchdog reset behaviour and the title truncation, and timing margins in the heartbeat test were widened to reduce flakiness on slower CI runners.

All existing tests pass (task test), the linter is clean (task lint), and the binary builds (task build). The watchdog and heartbeat paths were also verified with targeted -race runs.

dgageot added 2 commits July 9, 2026 20:16
Reset watchdog on every SSE line, not just data frames; add keep-alives
disable on the Unix transport; drain heartbeat goroutine before return;
truncate both splitTitle lines; defer mutex unlock in SetSamplingWithToolsHandler.

Assisted-By: claude-opus-4-5
@dgageot dgageot requested a review from a team as a code owner July 9, 2026 18:47

@docker-agent docker-agent 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.

Assessment: 🟢 APPROVE

The changes are well-reasoned and correctly address the stated reliability gaps in the SSE lifecycle. Here is a brief summary of the review findings:

  • Watchdog timer reset (client.go): Moving resetWatchdog() before the heartbeat-comment branch so every received SSE line (including : ping comments) resets the watchdog is logically correct and matches the advertised fix.
  • DisableKeepAlives: true on Unix socket transport (client.go): Correct — keep-alive probes are meaningless on Unix domain sockets and could generate spurious traffic; disabling them is appropriate.
  • Heartbeat goroutine drain (server.go): The heartbeatDone channel approach correctly prevents the goroutine leak on reconnect; the deferred closure stops the heartbeat and waits for heartbeatDone, and the goroutine closes it on exit.
  • splitTitle truncation (tui/view.go): Truncating line1 symmetrically with the remainder is the correct fix for the rendering inconsistency.
  • defer c.mu.Unlock() (session_client.go): Replacing manual Unlock() with defer is safer (panic-proof) and fixes the mutex lint warning as intended.
  • Tests: Timing margin widening and new regression tests for watchdog reset and title truncation are appropriate.

No bugs were found in the newly introduced code (+ lines). All changes appear correct and improve reliability.

@aheritier aheritier added area/mcp MCP protocol, MCP tool servers, integration area/tui For features/issues/fixes related to the TUI kind/fix PR fixes a bug (maps to fix:). Use on PRs only. labels Jul 9, 2026
@dgageot dgageot merged commit 12dc9db into docker:main Jul 10, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/mcp MCP protocol, MCP tool servers, integration area/tui For features/issues/fixes related to the TUI kind/fix PR fixes a bug (maps to fix:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants