fix(tmux): refuse a start whose pane cannot reach its working directory (#1713) - #1769
fix(tmux): refuse a start whose pane cannot reach its working directory (#1713)#1769asheshgoplani wants to merge 2 commits into
Conversation
…ry (#1713) `add -w` produced a session whose pane held only a broken shell — "shell-init: error retrieving current directory: getcwd: cannot access parent directories" — while agent-deck reported it created, started and alive. The agent never ran, and only `tmux capture-pane` revealed it. Root cause, reproduced with a bare tmux (3.7b), no agent-deck involved: mkdir -p /tmp/x/srv /tmp/x/good ( cd /tmp/x/srv && tmux -L probe new-session -d -s seed ) rm -rf /tmp/x/srv # unlink the SERVER's cwd tmux -L probe new-session -d -s t -c /tmp/x/good bash -c pwd # pane_current_path is /tmp/x/srv, NOT the requested -c, and the # pane prints the shell-init getcwd error Once a tmux server's own working directory is unlinked, tmux stops honouring the requested -c and every new pane inherits the server's dead cwd. That explains every observation in the report: the worktree really was fine, a restart hit the same server, three different worktree paths behaved identically, plain `add <good path>` without -w failed the same way, and host shells were unaffected. A server inherits its cwd from whatever process started it, and agent-deck is often run from a worktree that is later removed — which is why worktree users hit this first. The sibling defect on the same path: with a healthy server, `new-session -c <missing dir>` does not fail either. tmux silently starts the pane in $HOME, so a session whose project directory was deleted came up "successfully" with the agent pointed at the user's home directory. Three guards, in order of preference: - Prevention: every tmux spawn agent-deck issues now runs from "/" (the one directory that cannot be unlinked), so a server agent-deck starts can never inherit a deletable cwd. Verified: launching from a deleted cwd previously gave the server that dead cwd; it now gets "/". - Pre-flight: resolveStartWorkDir absolutises and validates the working directory before anything is spawned, and refuses the start with a message naming the directory instead of landing the agent in $HOME. - Post-flight: after creation, verify where the pane actually landed. A pane sitting in a path that provably does not exist (the already poisoned server, which prevention cannot retro-fix) fails loudly and the half-alive session is torn down rather than recorded as started. Only a provably missing path fails a start: an unreadable probe, or a pane that changed directory itself (the fork / multi-repo `cd <dir> &&` commands), is logged and tolerated. A deleted reading is re-confirmed first, since tmux creates the pane process before it has chdir()ed. SSH sessions opt out via WorkDirIsPlaceholder: their local path is only a placeholder, the pane runs an ssh client, and the project lives on the remote host — so a missing local directory must not make them unstartable. Committed by Ashesh Goplani
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👋 Thanks for opening this — a couple of intake fields would help the maintainer's validation pipeline (and any reviewer) understand it faster. Nothing here blocks you and nothing is closing: this is a heads-up, and editing the description re-runs this check automatically.
The full field-level contract is in .github/INTAKE.md. AI-authored PRs are welcome here with equal standing. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| dir = abs | ||
| } | ||
|
|
||
| info, err := os.Stat(dir) |
| if a == b { | ||
| return true | ||
| } | ||
| ai, err := os.Stat(a) |
| if workDirErr != nil { | ||
| if !s.WorkDirIsPlaceholder { | ||
| statusLog.Warn("tmux_start_refused_bad_workdir", | ||
| slog.String("session", s.Name), |
| if !s.WorkDirIsPlaceholder { | ||
| statusLog.Warn("tmux_start_refused_bad_workdir", | ||
| slog.String("session", s.Name), | ||
| slog.String("requested_workdir", s.WorkDir), |
| // ssh client, so a missing local directory must not block the start. | ||
| // Keep tmux's historical $HOME landing, but say so instead of hiding it. | ||
| statusLog.Warn("tmux_start_placeholder_workdir_fallback", | ||
| slog.String("session", s.Name), |
| reported, err := panePathProbe(s) | ||
| if err != nil { | ||
| statusLog.Debug("pane_cwd_probe_failed", | ||
| slog.String("session", s.Name), |
| switch verdict { | ||
| case paneCwdDeleted: | ||
| statusLog.Error("pane_born_in_deleted_cwd", | ||
| slog.String("session", s.Name), |
| case paneCwdDeleted: | ||
| statusLog.Error("pane_born_in_deleted_cwd", | ||
| slog.String("session", s.Name), | ||
| slog.String("requested_workdir", workDir), |
| // tmux's $HOME substitution is already prevented by | ||
| // resolveStartWorkDir. Logged so the mismatch is diagnosable. | ||
| statusLog.Warn("pane_cwd_differs_from_requested", | ||
| slog.String("session", s.Name), |
| // resolveStartWorkDir. Logged so the mismatch is diagnosable. | ||
| statusLog.Warn("pane_cwd_differs_from_requested", | ||
| slog.String("session", s.Name), | ||
| slog.String("requested_workdir", workDir), |
…t process Committed by Ashesh Goplani
|
Heads-up on the CodeQL check, with the context needed to triage it — I do not think this is fixable in this diff without giving up the fix, so it wants a policy call rather than a code change. 12 new alerts, all two rules:
Why the high ones cannot be sanitised away: the whole purpose of the change is to The rule is also already accepted elsewhere in this codebase for exactly this reason — On the medium ones: these are structured Options, in the order I would suggest:
Every other check on this PR is green, including |
2 participants
Fixes #1713.
What was happening
add -wproduced a session whose pane held only a broken shell:agent-deck reported it created, started and
health_class: alive. The agent never ran, and only a directtmux capture-panerevealed it. Thank you @kewtyboi — the report ruled out so much up front that it pointed straight at the real culprit, which turned out not to be the worktree at all.Root cause
Reproduced with a bare tmux (3.7b, macOS), no agent-deck involved:
Once a tmux server's own working directory has been unlinked, tmux stops honouring the requested
-cstart directory and every new pane inherits the server's dead cwd. The pane's first process cannotgetcwd(), so it prints that message and the agent never execs.That accounts for every observation in the report:
session restarthit the same error-w, pointing at an already-good pathzsh -i -c pwd/bash -c pwdfine on the hostA tmux server inherits its cwd from whatever process starts it. agent-deck is frequently run from a worktree, and a worktree is exactly the kind of directory that later gets removed (
agent-deck rm,worktree cleanup, a manual cleanup during resource pressure — which the report mentions). After that, every session created on that server is born broken. This also means the window stays broken until the server is restarted, matching "any dispatch attempt during this window".Sibling defect found on the same code path
With a healthy server,
new-session -c <missing dir>does not fail either — tmux silently starts the pane in$HOME. So a session whose project directory or worktree was deleted came up "successfully" with the agent pointed at the user's home directory.Start()even carried the comment// Ensure working directory existsabove code that never checked.The fix
Three guards, in order of preference (
internal/tmux/workdir_guard.go):/, the one directory that cannot be unlinked, so a server agent-deck starts can never inherit a deletable cwd.resolveStartWorkDirabsolutises and validates the working directory before anything is spawned, and refuses the start with a message naming the directory rather than silently landing the agent in$HOME.Deliberately conservative: only a provably missing path fails a start. An unreadable probe, or a pane that changed directory itself (the fork / multi-repo paths build
cd <dir> && …commands), is logged and tolerated. A "deleted" reading is re-confirmed before it is believed, because tmux creates the pane process before it haschdir()ed.SSH sessions opt out through
WorkDirIsPlaceholder: their local path is only a placeholder, the pane runs ansshclient, and the project lives on the remote host, so a missing local directory must not make them unstartable. They keep tmux's historical$HOMElanding, logged rather than hidden.Verification
Behaviour checked against real tmux via a scratch binary built from this branch and from
main(isolated-Lsocket + isolatedTMUX_TMPDIR, servers killed with the same socket resolution):The reported scenario — server started from a directory that is then deleted, then a session created with a perfectly good working directory:
with the error text:
Unrelated sessions on that server were untouched — the teardown is session-scoped, never
kill-server.Prevention, before vs after — agent-deck launched from a directory deleted out from under it, creating the first session on a fresh socket:
main/private/tmp/…/launchdir(deleted)/On
mainthat server is poisoned from birth for every later session; on this branch it cannot be.Also verified: a missing working directory is refused with no tmux spawn at all; a placeholder (SSH) session with a missing local directory still starts; healthy starts are unchanged.
Tests
internal/tmux/issue1713_workdir_guard_test.go— the verdict classifier (ok / unknown / deleted / elsewhere, symlinked-prefix tolerance),resolveStartWorkDir(missing, file, empty, relative-absolutised), the re-confirm-before-failing retry, probe-failure tolerance,Start()refusing a deleted directory without spawning,Start()tearing down a session born in a deleted directory, the placeholder opt-out, and the real-tmux probe against a pane whose directory is deleted after birth. Plus a lint pinning that every spawn inSession.Startgoes throughnewSpawnCommandand that both guards stay wired.internal/session/issue1713_placeholder_workdir_test.go— pins the SSH opt-out at the single chokepoint the threeStart()paths share.internal/session/instance_test.go— one pre-existing test started a session in/tmp/claude-zombie-reject, which does not exist; it now uses a real temp dir. It was passing only because tmux silently redirected it to$HOME.Every test in the repo that starts a session was audited for non-existent paths; that was the only one.
Notes for review
Session.Startnow returns before mutating session state when the directory is unusable, so a refused start leaves the session untouched. Errors already surface at the CLI (session startexits 1) and are persisted byrecordTmuxStartFailure, sosession showexplains it too.new-session(internal/session/instance.go) also spawns from/now: it is the only other call that could create a server. Its pane runsdocker exec, whose working directory is fixed at container create, so nothing else changes.internal/uirefresh paths are touched, so there is no overlap with the TUI performance work in flight.respawn-panedoes not pass-c, so a restart re-uses the pane's recorded directory and falls back to$HOMEif that directory is gone. Worth a separate look; re-anchoring restarts to the session's project directory is a behaviour change that deserves its own discussion.