Repository navigation
fix(test): isolate daemon session routes from the default tmux server - #844
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Ready for lead: #844 at
Both required checks and all five planned Linux shards passed in CI run 43592448391. Local verification with real tmux: 39 passed / 0 failed, across the session route, CLI private-socket, shell timeout and tmux backend tests; typecheck, check/privacy and changed-file lint pass. The same persistent Opus reviewer cleared the final source tree No extra macOS shard dispatch was requested. Hosted macOS evidence is deferred; please run #813 remains unchanged at |
Tests can invoke tmux directly or through helper children and inherit the user's default server. Test startup now owns a private socket root, removes inherited `TMUX`, and stops its server after teardown, including assertion failures. The E2E runner also owns a private server scope closed in `finally`. Bun's spawn APIs and Node's child-process APIs receive an explicit environment with a registered private `TMUX_TMPDIR`. This covers scripts and helper children with omitted or minimal environments. Direct tmux commands additionally validate socket arguments and directory ownership/mode. Cleanup runs after the checkout guard's Git children; the global-state guard retains its fixed read-only native-environment probe. The daemon-session fixture merged in #844 keeps its stricter explicit socket, empty config, finite pane lifetime, per-case cleanup and real session-presence assertions. This PR adds only four socket-registration lines to that file. Other real-tmux fixtures register their roots, and project-view unit tests inject session data. Seven compiler environment-contract checks use an async-local scope for their owned, pure compiler fixture only. Those fixtures keep the original explicit compiler environment and unchanged PATH-only assertions. Sibling commands and commands outside the scope remain isolated; these pure fixtures never invoke tmux. Regressions cover indirect real tmux calls reaching the owned server PID, Bun/Node API forms, null/undefined argv preserving options, fixture-only environment scopes, unsafe-socket refusals using a harmless stand-in, and cleanup after a deliberately failing child test. Validation after a normal merge of frozen `next` at `8e7242db94`: - Linux, Bun 1.4.4, real tmux, synthetic inherited tmux selection: 760 passed, 1 skipped, 0 failed across 36 suites (4,617 assertions), including all compiler transport suites and the incoming Rust test-runner tests. The existing compiled re-exec case skips without its compiled artifact. - Default `tmux list-sessions -F '#{session_id}:#{session_name}'`, with `TMUX` and `TMUX_TMPDIR` unset: byte-identical stdout and stderr before/after; 3 sessions before and after. No user session names are included here. - Root typecheck/check/privacy and docs checks passed. The other 21 owned files are unchanged from reviewed head `1a09a7037`; the contributor guide retains all incoming guidance. Scripts that deliberately erase their environment or name an explicit socket must maintain isolation themselves; explicit `Bun.$.env(...)` is unwrapped. SIGKILL skips teardown and may leave a private server/root; stale-root recovery remains follow-up work. Lead full-suite/Mac acceptance remains separate from local evidence. Fixes HACK-2242. --------- Co-authored-by: Kit <344842+roodboi@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The daemon session route tests created
myproject123,my-projectandmy_projecton the developer's default tmux server and never stopped them. The first name matches the orphan reported after a parallel test batch.Run these route tests against a fresh private socket instead. A fixture PATH shim invokes the real tmux executable with that socket and an empty config, clears inherited
TMUX, and starts a fixed sleeping command so no user shell startup files run. Each case stops only its private server, verifies it is absent, and restores PATH even when the case fails. Final cleanup removes the owned fixture. Successful name validation now requires an actual 201 response; the alphanumeric case verifies its session exists on the private socket.Validation: 39 tests across
daemon-sessions,session-list-stop-tmux,shell-timeoutandtmux-backendpass locally with real tmux. Typecheck, check/privacy and changed-file lint pass. The same persistent independent Opus reviewer is clean after two rounds, including all three session-presence assertions and guarded PATH restoration. No macOS result is claimed yet; no extra macOS CI dispatch was requested.This fixes a confirmed test leak found during HACK-2219 investigation. It does not establish that the leak causes the post-parallel authored-start failures. The diagnostic #813 head remains separate and stable for the requested five-round Mac reproduction. Existing sessions on the default server are untouched.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.