fix(tmux): decouple pane-exit detection from reconnect backoff, close test-race - #378
Merged
tstapler merged 1 commit intoAug 7, 2026
Merged
Conversation
… test-race TestTmuxServerRegistry_PaneExitChannel intermittently failed with "SubscribePaneExit channel not closed within 3s after kill-session". Root cause #1 (server_registry.go): while reconnectLoop is sleeping out exponential backoff (100ms..30s), no syncSessions() runs at all until the next reconnect completes, so pane-exit detection latency was effectively bound by backoff instead of by anything caller-facing. Fixed with a syncMu-guarded fast-recheck path (waitBackoffWithFastRecheck + syncSessionsFastRecheck) that makes a small, bounded number of independent resync attempts during a long backoff wait without blocking on or interfering with the normal blocking syncSessions() callers. Ceiling is documented inline: fastRecheckAttempts * (fastRecheckSyncTimeout + fastRecheckInterval) = 700ms, gated behind fastRecheckMinBackoff=1600ms (below that, the plain wait alone already leaves ample margin, and unconditional fast-rechecking measurably worsened flakiness under load by adding avoidable list-sessions forks with zero benefit). TestTmuxServerRegistry_PaneExitDetectedDespiteElevatedBackoff exercises this structurally, by elevating backoff to 3200ms via a clean control-mode outage and asserting detection within 1.5s. Root cause #2 (server_registry_integration_test.go), found after the above fix still left a residual ~10-15% failure rate: two independent gaps in the test scaffolding, not the tmux server itself dying, root-caused with tmux's own -v/-vv server-side protocol log. - Isolated test servers were spawned without -f, so they silently loaded this developer's real ~/.tmux.conf (including a `run '~/.tmux/plugins/tpm/tpm'` that forks extra tmux subcommands against the fresh server as part of config load). Fixed with -f /dev/null on the command that starts each isolated server. - startIsolatedRegistry returned before the control-mode client had actually finished attaching, letting a test's own session-create race ahead of the registry's own attach-session. When it won, the session was created before the control client subscribed, so tmux never emitted %session-created/%sessions-changed for it (no event replay), and -- since the connection then stayed healthy with no further drops -- nothing ever triggered a resync before the test's poll timeout. Confirmed directly via a captured failure with zero reconnect/backoff log lines in between. Fixed by blocking on registry.IsHealthy() (set only after a live post-connect sync, which requires the server to have already processed the earlier-submitted attach-session) before startIsolatedRegistry returns. Verified: go test -race -tags integration ./session/tmux -run TestTmuxServerRegistry_PaneExitChannel at -count=40, three consecutive -count=20 runs, and -count=100 -- 260/260, zero failures, across two independent worktrees. Full session/tmux suite (including TestEnsureServerRunning_NoOp, TestKillOrphanedControlModeClients, and the new regression test) and make ci both pass cleanly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y3suSzoDYXnvbg2KQyD2qG
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes a flaky tmux integration test by making pane-exit detection independent of reconnect backoff, and hardening the test harness to avoid config pollution and control-mode attach races.
Changes:
- Add a non-blocking “fast recheck” resync path during reconnect backoff to bound pane-exit detection latency.
- Fix integration test harness races by isolating tmux config (
-f /dev/null) and waiting for registry health before proceeding. - Add planning/validation artifacts documenting root cause, design decisions, and verification.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| session/tmux/server_registry.go | Serializes session syncs and adds bounded fast-recheck during backoff to decouple pane-exit detection from reconnect timing. |
| session/tmux/server_registry_integration_test.go | Improves isolated socket naming, isolates tmux config, waits for registry readiness, and adds an elevated-backoff regression test. |
| project_plans/tmux-paneexit-reconnect-flake/requirements.md | Captures the problem statement, constraints, and acceptance criteria for the flake fix. |
| project_plans/tmux-paneexit-reconnect-flake/research/architecture.md | Documents the intended integration point and timing/locking approach for fast recheck. |
| project_plans/tmux-paneexit-reconnect-flake/research/build-vs-buy.md | Justifies hand-rolling vs. adopting a retry/backoff dependency. |
| project_plans/tmux-paneexit-reconnect-flake/research/features.md | Enumerates edge cases and failure modes for a fast-recheck mechanism. |
| project_plans/tmux-paneexit-reconnect-flake/research/pitfalls.md | Analyzes concurrency pitfalls (lost updates, goroutine leaks) and mitigation strategy. |
| project_plans/tmux-paneexit-reconnect-flake/research/stack.md | Notes assumed Go/dependency environment for the implementation. |
| project_plans/tmux-paneexit-reconnect-flake/implementation/plan.md | Full implementation plan including patterns, risks, and verification steps. |
| project_plans/tmux-paneexit-reconnect-flake/implementation/pre-mortem.md | Pre-mortem describing plausible failure modes and mitigations. |
| project_plans/tmux-paneexit-reconnect-flake/implementation/validation.md | Maps requirements to tests and describes validation/coverage expectations. |
| project_plans/tmux-paneexit-reconnect-flake/implementation/architecture-review.md | Review notes on the plan (blockers/concerns). |
| project_plans/tmux-paneexit-reconnect-flake/implementation/adversarial-review.md | Adversarial review notes on margins/coverage gaps and recommended tweaks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
✅ Registry ValidationTest Coverage: 34/186 features have
|
Contributor
E2E RPC Latency |
Contributor
Go Benchmarks (Tier 1) |
Contributor
Frontend Terminal Throughput |
Contributor
📊 Feature E2E CoverageFeature coverage report unavailable
|
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
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
TestTmuxServerRegistry_PaneExitChannelintermittently failed withSubscribePaneExit channel not closed within 3s after kill-session, reproducing on unmodifiedmain(per.claude/rules/fix-flaky-tests-dont-defer.md— filed and fixed rather than re-excused as a known flake).Two independent root causes, found and fixed in sequence:
Detection latency bound by reconnect backoff (
server_registry.go): whilereconnectLoopsleeps out exponential backoff (100ms..30s), nosyncSessions()runs at all until the next reconnect completes, so pane-exit detection latency was effectively bound by backoff instead of anything caller-facing. Fixed with asyncMu-guarded fast-recheck path that makes a small, bounded number of independent resync attempts during a long backoff wait, without blocking or interfering with the normal blockingsyncSessions()callers. The latency ceiling (fastRecheckAttempts × (fastRecheckSyncTimeout + fastRecheckInterval) = 700ms) is documented inline with aponytail:-style comment.Test-scaffolding race (
server_registry_integration_test.go), found after fix ci: fix forbidigo pattern field for golangci-lint 1.60.1 #1 still left a residual ~10-15% failure rate. Root-caused with tmux's own-v/-vvserver-side protocol log — not the tmux server dying, but two gaps in the test harness:-f, silently inheriting the developer's real~/.tmux.conf(a TPMrunhook forks extratmuxsubcommands against the fresh server). Fixed with-f /dev/null.startIsolatedRegistryreturned before the control-mode client had finished attaching, letting a test's own session-create race ahead of the registry'sattach-session. When it won, the session predated the control client's subscription, so tmux never emitted%session-created/%sessions-changedfor it (no event replay), and since the connection then stayed healthy with no further drops, nothing ever re-synced before the test's timeout. Fixed by blocking onregistry.IsHealthy()beforestartIsolatedRegistryreturns.Also fixed a margin-calculation bug in
TestTmuxServerRegistry_PaneExitDetectedDespiteElevatedBackoffcaught bysdd:6-verify's idiom review:minElevatedBackoffCycles=5only guaranteed a 1600ms backoff wait at assertion time, not the documented 3200ms — bumped to 6 and corrected the comment.Full root-cause narrative:
project_plans/tmux-paneexit-reconnect-flake/requirements.md.What Changed
session/tmux/server_registry.go—syncMu+syncSessionsFastRecheck(non-blockingTryLock) +waitBackoffWithFastRecheck, decoupling pane-exit detection latency from reconnect backoff.session/tmux/server_registry_integration_test.go—-f /dev/nullisolation for test tmux servers, a health-wait instartIsolatedRegistrybefore returning, and a corrected backoff-margin constant in the elevated-backoff regression test.project_plans/tmux-paneexit-reconnect-flake/**— SDD planning artifacts (requirements, research, plan, pre-mortem, validation, reviews).Test plan
go test -race -tags integration ./session/tmux -run TestTmuxServerRegistry_PaneExitChannel -count=40— 40/40go test -race -tags integration ./session/tmux -run TestTmuxServerRegistry_PaneExitChannel -count=20× 3 consecutive runs — 20/20, 20/20, 20/20 (60/60)go test -race -tags integration ./session/tmux -run TestTmuxServerRegistry_PaneExitChannel -count=100— 100/100go test -race -tags integration ./session/tmux/...(full package, includingTestEnsureServerRunning_NoOp,TestKillOrphanedControlModeClients, and the newTestTmuxServerRegistry_PaneExitDetectedDespiteElevatedBackoffregression test) — passes cleanly across repeated runsmake ci— run to completion twice (original worktree + the clean-branch worktree matching what's pushed here), both exit 0 with zero FAIL/Error linessdd:6-verifyLayer 1 (idioms) + Layer 2 (architecture) review agents — no BLOCKERs, no MUST FIX findings; one SUGGEST finding fixed and reverifiedgit diff --name-only origin/main HEAD— exactly 13 files (2 source files + 11 of this item's own planning docs)🤖 Generated with Claude Code
https://claude.ai/code/session_01Y3suSzoDYXnvbg2KQyD2qG