fix(test): unlink throwaway tmux socket in tmux-integration teardown (stop leaking tgctl-test-* sockets)#37
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de044e33d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…teardown (no leaked tgctl-test-* sockets)
The ctl-tmux integration suite ran every test against a private
-L tgctl-test-<pid> server and tore it down in afterAll with
`tmux kill-server`. On macOS, kill-server ends the server PROCESS but
does NOT unlink the socket file under /tmp/tmux-<uid>/. The socket inode
therefore leaked every run; alongside rig-cli's identical bug, ~185
tgctl-test-*/rigtest-* sockets piled up in /tmp/tmux-501/ and once
starved/killed the developer's live tmux server.
Fix: afterAll now kills the server AND rmSync's its socket file. The path
is computed by socketPathFor(), which mirrors tmux's server_create_socket
(TMUX_TMPDIR honored only when absolute, else /tmp) and is reused by the
new regression test.
Adds a regression test ("teardown leaves no leaked tmux socket file"):
boots a real server, asserts the socket file exists, runs the same
kill+unlink, asserts the file is gone. Verified the suite leaves zero
tgctl-test-* sockets behind in both default and TMUX_TMPDIR-set envs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
de044e3 to
b20af56
Compare
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.
Problem
tests/ctl-tmux-integration.test.tsruns every test against a private-L tgctl-test-<pid>server and tears it down inafterAllwithtmux kill-server. On macOS,kill-serverends the server process but does NOT unlink the socket file under/tmp/tmux-<uid>/. The socket inode leaked every run.This is the same bug as rig-cli's e2e fixture (alex-mextner/rig-cli#31). Together they leaked ~185
tgctl-test-*/rigtest-*sockets into/private/tmp/tmux-501/on the dev machine, which once starved/killed the live tmux server.Fix
afterAllnow kills the server ANDrmSyncs its socket file.socketPathFor()helper that mirrors tmux'sserver_create_socket(TMUX_TMPDIRhonored only when absolute, else/tmp) and is reused by the new regression test.Test
Adds
teardown leaves no leaked tmux socket file: boots a real server, asserts the socket file exists, runs the same kill+unlink, asserts the file is gone.Verified: full
bun testsuite green (1015 pass), and the tmux integration suite leaves zerotgctl-test-*sockets behind in both the default andTMUX_TMPDIR-set environments.🤖 Generated with Claude Code