fix: cancel deferred configureTmux timer on session destroy (flaky pty-server test)#728
Merged
Merged
Conversation
The post-spawn configureTmux() runs on a 200ms setTimeout whose handle was never stored, so destroySession() could not cancel it. A torn-down session still sourced its tmux config 200ms later, and in the test suite that stale timer leaked a default-socket source-file spawn into a later test — making 'sources tmux config after timeout when socket is provided' flaky on slow CI runners (it matched the leaked dev3 socket instead of conf-socket). Store the timer on the session, clear it in destroySession, and make the affected tests match the source-file call for their own socket. Adds a deterministic regression test.
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.
Hi, this is Claude (the AI assistant working on this branch) 👋
Summary
Fixes the flaky
pty-servertestconfigureTmux via spawnPty › sources tmux config after timeout when socket is provided, which failed intermittently on slow CI runners withexpected [ 'tmux', '-L', 'dev3', ... ] to include 'conf-socket'.Root cause — not a fake-timer race, but a leaked real timer across tests.
createSession→spawnPtyschedulessetTimeout(() => configureTmux(...), 200), but the handle was never stored, sodestroySession()could not cancel it. Tests that use real timers tear the session down inafterEachbefore the 200ms fires; the stale timer then fires during a later test, callingspawn('source-file')with that session's socket (usually the defaultdev3).vi.clearAllMocks()resetsmock.callsper test but not pending OS timers, so the leaked spawn lands in the current test'smock.calls, and.find(c => includes('source-file'))returned the wrong call.Fix
configureTmuxtimer on the session (configureTimer) andclearTimeoutit indestroySession. A torn-down session no longer sources its tmux config 200ms later — a genuine correctness fix that also removes the cross-test leak at the source.source-filecall for their own socket instead of the first one found.source-filespawn.Verified the regression test fails without the production fix, then passes with it.
bun run lintclean; full suite green (mainview 1636, bun 1548).