Prevent terminated e2e runs from leaking tmux

[mariusvniekerk]

Terminated e2e runners can leave daemonized tmux servers behind because tmux outlives its parent, while ordinary defers, exit callbacks, and late graceful cleanup are skipped or cut short by signals.\n\n- Stop each Go e2e server's private tmux server before bounded handler draining, including reset and termination paths.\n- Track Playwright-owned children from spawn through confirmed exit, preserve the shared socket root while a child can still create tmux state, and keep repeated SIGINT/SIGTERM handling active during cleanup.\n- Build one run-owned e2e-server binary before Playwright starts, share it with every worker, and let only the builder remove it. This keeps the real server process in the runner's signal path instead of a go run wrapper.\n- Sweep sockets before and after child exit, removing stale sockets only when tmux verifies their server is gone and retaining ambiguous failures for recovery.\n- Synchronize Go test tmux startup with signal cleanup so termination cannot land between daemonization and ownership registration.\n- Recover dead-owner directories only when their resources belong to the current local user. Developer tmux servers remain outside the cleanup boundary.\n\n^{generated by a clanker}

[roborev-ci]

roborev: Combined Review (5438ee2)

Code review found one High-severity credential-exposure risk and four Medium-severity cleanup gaps.

High

• Unauthenticated stale tmux socket connection — frontend/tests/e2e-full/support/e2eServer.ts:137
reapStaleE2ETmuxDirs trusts matching /tmp/kf-e2e-tmux-* directories and their owner.json without verifying ownership or permissions. A local attacker could provide a malicious Unix socket and capture environment data, potentially including CI credentials, when the runner invokes tmux -S <socket> kill-server. Require the directory, owner file, and socket to be owned by the current UID with private permissions—preferably under a private per-user runtime directory—and launch cleanup clients with a minimal, scrubbed environment.

Medium

• Signal handling ends before bounded cleanup — internal/server/api_test.go:101, internal/server/kataapi/testmain_test.go:30, internal/server/workspacetest/testmain_test.go:39
  A SIGTERM during the subsequent 5–10-second cleanup window can immediately terminate the test binary and leak tmux resources. Call runCleanup() before stopSignalCleanup(); sync.Once already serializes concurrent cleanup safely.

• Fresh standalone servers lack termination cleanup — frontend/tests/e2e-full/support/e2eServer.ts:903
  The freshProcess: true branch registers servers in standaloneServers but never calls installCleanup, so workers using only this path can leak server and tmux resources on termination. Install cleanup when the first standalone server is registered and add a termination test for this path.

• Child liveness and PID escalation are unsafe — frontend/tests/e2e-full/support/e2eServer.ts:506
  Checking only child.exitCode misses signal-terminated children, while child may represent the go run wrapper rather than serverPID. Cleanup can wait on an exited wrapper and later signal a stale, reused PID. Track both exitCode and signalCode, and escalate through a verified process handle or owned process group.

• Real-tmux tests rely only on t.Cleanup — cmd/e2e-server/main_test.go:556
  Terminating the Go test binary can bypass cleanup and leak test-created tmux daemons. Add package-level signal cleanup that tracks and kills all private sockets created by these tests before exit.

---

Reviewers: 2 done | Synthesis: codex, 19s | Total: 13m17s

[roborev-ci]

roborev: Combined Review (e8fd2b2)

The shutdown cleanup is improved, but three medium-severity races can still leak tmux daemons or server processes.

Medium

• frontend/tests/e2e-full/support/e2eServer.ts:649 — The initial tmux cleanup removes the directory while server children are still shutting down. A child can create a socket between the scan and removal, unlinking a live server’s socket and preventing final cleanup. During pre-wait cleanup, kill discovered sockets without removing the directory; remove it only after all children exit and a final rescan handles late sockets.

• cmd/e2e-server/main.go:963 — tmuxStopOnce suppresses cleanup after the initial pre-drain attempt. An in-flight request can start or restart tmux afterward, allowing termination or reset to leak the daemon. Retain the early kill, but perform an additional, ungated kill after HTTP handlers drain and immediately before exit.

• frontend/tests/e2e-full/support/e2eServer.ts:506 — Before readiness, cleanup tracks only the go run wrapper PID. Signaling that wrapper may leave the actual e2e-server descendant orphaned, and the new test bypasses this path by spawning a prebuilt binary directly. Launch a directly managed binary or run the wrapper and descendant in a process group that cleanup terminates and waits for together.

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 6m59s

[roborev-ci]

roborev: Combined Review (c667efd)

One-line verdict: Two medium-severity teardown races remain in the Playwright e2e server lifecycle.

Medium

• frontend/tests/e2e-full/support/e2eServer.ts:652 — Before readiness, default servers launched via go run are tracked only by the Go wrapper PID. Shutdown does not signal the process group, so the compiled child may survive, publish readiness later, and leak tmux state after the socket root is removed. Launch a prebuilt binary or use an owned process group and terminate the entire group during pre-readiness cleanup.

• frontend/tests/e2e-full/support/e2eServer.ts:658 — shutdownOwnedServers clears ownership registries before child processes exit. Concurrent shutdown or standalone stop() can then remove the shared tmux root while children remain alive, reopening the late-socket race. Serialize shutdown with a shared in-flight promise and retain ownership, or track shutdown-in-progress state, until all captured children exit and the final socket sweep completes.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 9m43s

[roborev-ci]

roborev: Combined Review (f215e3e)

High-severity cleanup flaw remains; two medium-severity shutdown races may also leak resources or break active handlers.

High

• frontend/tests/e2e-full/support/e2eServer.ts:595 — Before server info is available, termination targets child.pid. With the default go run command, this is the Go wrapper’s PID rather than the e2e binary’s, so killing the wrapper can orphan a running server after its ownership and tmux root are released. Launch the server in an owned process group and terminate/wait for that group, or execute a built binary directly. Add coverage using the default go run invocation.

Medium

• cmd/e2e-server/main.go:963 — tmuxStopOnce permanently suppresses cleanup retries. A handler may create a tmux server after the early pre-drain kill, or that kill may time out, while the later close() cannot retry. Keep the early best-effort kill, then retry after handlers and workspace cleanup have drained using synchronization that prevents concurrent—not subsequent—attempts.

• cmd/e2e-server/main.go:984 — If handler draining times out, close() immediately cleans workspaces, closes SQLite, and removes the temporary directory while old handlers may still be running. During an in-process reset, those handlers can access closed or deleted resources. Defer destructive cleanup until handlerWG completes; only the bounded shutdown path should return early.

---

Reviewers: 2 done | Synthesis: codex, 14s | Total: 9m22s

[roborev-ci]

roborev: Combined Review (f786b39)

The changes introduce three medium-severity shutdown and cleanup race conditions; no security vulnerabilities were identified.

Medium

• frontend/tests/e2e-full/support/e2eServer.ts:840 — Playwright workers track spawned servers only in process-local sets. During normal worker exit, cleanupImmediately signals children without awaiting them, while the root runner may remove the shared tmux directory before those children finish exiting. A late-created socket could then be unlinked without terminating its tmux daemon. Persist child ownership in the shared directory or add asynchronous worker teardown that lets the root owner wait for every worker-owned server.

• frontend/tests/e2e-full/support/e2eServer.ts:875 — If asynchronous cleanup rejects, process.exit(exitCode) is never reached. Since signalCleanupStarted remains true, later SIGINT/SIGTERM signals are ignored, potentially leaving the runner alive or causing an unhandled-rejection exit before cleanup completes. Catch and report cleanup failures, and invoke process.exit(exitCode) from a finally block.

• cmd/e2e-server/main.go:984 — After handler draining times out, shutdown still deletes workspaces, closes SQLite, and removes the temporary directory while handlers remain active. During an in-process reset, those handlers can access destroyed state, causing request failures or races. Keep the early tmux stop bounded, but defer destructive cleanup until handlerWG finishes.

---

Reviewers: 2 done | Synthesis: codex, 26s | Total: 9m56s

[roborev-ci]

roborev: Combined Review (f3e19b7)

The test cleanup is improved overall, but one medium-severity orphan-process leak remains.

Medium

• frontend/tests/e2e-full/support/e2eServer.ts:878 — The root runner waits for worker-published server PIDs but never terminates them. If a worker crashes or receives SIGKILL before its exit handler runs, its e2e server can remain alive indefinitely, causing cleanup to time out and preserve the tmux root. Publish sufficient process identity for safe root-runner termination, and add a cross-process test that abruptly kills a worker and verifies both the server process and tmux root disappear.

---

Reviewers: 2 done | Synthesis: codex, 13s | Total: 12m31s

[roborev-ci]

roborev: Combined Review (481ab95)

Review found two medium-severity reliability issues in tmux lifecycle handling.

Medium

• frontend/tests/e2e-full/support/e2eServer.ts:226 — Stale-root recovery deletes the tmux directory when its root owner is dead without checking live server-owner-*.json children. An orphaned worker may still be using that socket root. Preserve the root while any published server PID is live, and test the dead-root/live-server case.

• cmd/e2e-server/main.go:1187 — The executable gate can mask an unavailable underlying tmux binary, suppressing PTY-owner fallback and advertising targets that cannot launch. Install the gate only when the underlying tmux command is executable; otherwise retain the unavailable command.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 8m15s

[roborev-ci]

roborev: Combined Review (d46e9ec)

Medium-severity shutdown races and an external-server build regression remain.

Medium

• cmd/e2e-server/main.go:981 — Shutdown may kill tmux without acquiring the creation lock. An admitted new-session can daemonize afterward, while tmuxStopOnce prevents further cleanup. Terminate or await the admitted creator before killing tmux; never proceed without owning the gate.

• internal/testutil/testsignal/testsignal.go:35 — Signal cleanup runs concurrently with m.Run, allowing active tests to create tmux resources after one-shot cleanup and before exit. Block new resource creation at shutdown, await admitted creators, then clean up and exit.

• frontend/playwright-e2e.config.ts:4 — The runner always builds a local server binary before checking for a reusable external server, unnecessarily requiring frontend and Go builds. Build lazily only when a local server must be spawned.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 7m31s

[roborev-ci]

roborev: Combined Review (43f16c9)

Shutdown cleanup still has three medium-severity race conditions.

Medium

• frontend/tests/e2e-full/support/e2eServer.ts:281 — Shared owner files may disappear between readdir, lstat, and readFile. This routine concurrent cleanup is treated as an ownership failure, permanently retaining the tmux root. Treat ENOENT as progress and retry the scan; publish owner files atomically.

• cmd/e2e-server/main.go:1061 — If handler draining times out, close destroys the database and temporary files while handlers remain active, causing use-after-close failures during resets. Defer destruction until handlers finish, or return on timeout and clean up asynchronously afterward.

• cmd/e2e-server/main.go:992 — After five seconds, tmux shutdown proceeds without the creation lock. An admitted new-session command can create a server after kill-server, while tmuxStopOnce prevents further cleanup. Track and terminate the admitted creator, or wait for the barrier and perform a final server sweep.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 7m44s

[roborev-ci]

roborev: Combined Review (f3971fa)

The changes have three cleanup race/ownership issues, including one high-severity risk to concurrently running specs.

High

• frontend/tests/e2e-full/support/e2eServer.ts:910 — Exit cleanup calls cleanupManagedServerProcess(managedChild) even when managedChild is null. The function then falls back to the inherited PLAYWRIGHT_E2E_SERVER_INFO_FILE, allowing workers and signal-test helpers to terminate the runner-owned shared server and disrupt concurrent specs.
  • Fix: Run cleanup only when the module owns a non-null managedChild, pass its local info file explicitly, and discard inherited shared-server ownership variables in subprocess helpers.

Medium

• frontend/tests/e2e-full/support/e2eServer.ts:818 — Individual shutdown decides whether to remove ownedTmuxDir using only in-process server collections, ignoring live server-owner-*.json records from other processes. Stopping the creator’s last local server can therefore kill another worker’s tmux server and delete its shared socket root.

  • Fix: Check shared owner records before removing the root, or reserve root removal for coordinated global shutdown.

• cmd/e2e-server/main.go:994 — After five seconds, tmuxCreationGate.stop proceeds without acquiring creation.lock. An already admitted new-session can create a tmux server after kill-server completes, while tmuxStopOnce prevents further cleanup.

  • Fix: Do not finalize shutdown without owning the gate; terminate and await the admitted creator on timeout, or retain retryable cleanup state and perform a final socket sweep.

---

Reviewers: 2 done | Synthesis: codex, 13s | Total: 10m21s