fix: tmux and term shell out with no context or timeout #454

New issue

Open

opened 2026-07-25 23:33:33 +02:00 by dries · 0 comments

dries commented

2026-07-25 23:33:33 +02:00

Owner

Code-quality audit (P2). internal/tmux and internal/term make 28 exec.Command calls and 0 exec.CommandContext calls.

Examples: tmux/sessions.go:129,155,189,222; term/term.go:142,232,385,454.

cmd := exec.Command("tmux", "list-sessions", "-F", format)   // no ctx, no timeout

Why it matters

handleTmuxSessions, handleTmuxClients, and handleSessionEvents call into these from the HTTP request path. A wedged or unresponsive tmux server hangs the handler indefinitely — no timeout, and a client disconnect cannot cancel it. Each hung request holds a goroutine and, for the SSE paths, a connection. internal/git already gets this right via internal/gitexec, so this is an inconsistency, not an unsolved problem.

Suggested fix

Mirror internal/gitexec:

Add internal/tmuxexec with Command(ctx, args...) applying a default timeout (git uses gitCommandTimeout; pick a comparable bound).
Convert the 28 call sites in internal/tmux / internal/term to take a ctx.
Thread r.Context() from handleTmuxSessions / handleTmuxClients / handleSessionEvents; background callers pass their own ctx.

Long-lived PTY processes in internal/term are the exception — they should get the ctx for cancellation but not the short default timeout.

Acceptance criteria

exec.Command count in internal/tmux + internal/term is 0 (excluding the PTY spawn, which uses CommandContext without a deadline).
A hung tmux binary fails the handler within the timeout rather than hanging (test with a stub runner that blocks).
Cancelling the request context aborts the child process.

Effort: M.

Code-quality audit (P2). `internal/tmux` and `internal/term` make **28** `exec.Command` calls and **0** `exec.CommandContext` calls. Examples: `tmux/sessions.go:129,155,189,222`; `term/term.go:142,232,385,454`. ```go cmd := exec.Command("tmux", "list-sessions", "-F", format) // no ctx, no timeout ``` ## Why it matters `handleTmuxSessions`, `handleTmuxClients`, and `handleSessionEvents` call into these from the HTTP request path. A wedged or unresponsive tmux server hangs the handler **indefinitely** — no timeout, and a client disconnect cannot cancel it. Each hung request holds a goroutine and, for the SSE paths, a connection. `internal/git` already gets this right via `internal/gitexec`, so this is an inconsistency, not an unsolved problem. ## Suggested fix Mirror `internal/gitexec`: - Add `internal/tmuxexec` with `Command(ctx, args...)` applying a default timeout (git uses `gitCommandTimeout`; pick a comparable bound). - Convert the 28 call sites in `internal/tmux` / `internal/term` to take a `ctx`. - Thread `r.Context()` from `handleTmuxSessions` / `handleTmuxClients` / `handleSessionEvents`; background callers pass their own ctx. Long-lived PTY processes in `internal/term` are the exception — they should get the ctx for cancellation but not the short default timeout. ## Acceptance criteria - [ ] `exec.Command` count in `internal/tmux` + `internal/term` is 0 (excluding the PTY spawn, which uses `CommandContext` without a deadline). - [ ] A hung `tmux` binary fails the handler within the timeout rather than hanging (test with a stub runner that blocks). - [ ] Cancelling the request context aborts the child process. Effort: M.