fix(tmux-manager): use '|' separator in reconcileSessions - #71
Merged
Merged
Conversation
Under non-tty execution contexts (launchd on macOS, systemd without TTY),
tmux emits '\t' in FORMAT strings as the literal two characters `\` + `t`
rather than as a tab. The parser's `line.indexOf('\t')` (a real tab char)
therefore never matches, `activeSessions` stays empty, `reconcileSessions`
returns `alive: []` / `discovered: []`, and `cleanupStaleSessions()` wipes
every entry in `state.json` — even though the underlying tmux sessions are
still alive. On the next startup the user sees an empty session list.
The bug reproduces reliably when codeman is launched via a user LaunchAgent
or a systemd unit without `TTYPath`. Interactive `npm run dev` hides it
because tmux's format parser does interpret `\t` when stdout is a TTY.
Fix: use `|` as the separator. tmux passes it through verbatim in every
environment, and `|` is not a valid tmux session-name character so it
cannot collide with the codeman-<uuid> / claudeman-<uuid> naming scheme.
Extract the inline pane-list parser from `reconcileSessions` into an exported `parsePaneList()` helper plus `PANE_LIST_SEP` / `PANE_LIST_FORMAT` constants, so the '|' separator contract can be unit-tested directly. The new tests lock in: - Well-formed parsing into name -> pid Map - Empty / blank-line / missing-separator handling - Non-numeric pid and empty-name rejection - A literal `\t` (backslash + t) in the input is NOT treated as a delimiter — guards against the launchd/systemd regression that motivated PR Ark0N#71. - Splitting on the first separator only. No behavior change in `reconcileSessions`; the body now delegates to the helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
|
Thanks for the great catch and clean writeup, @TeigenZhang! The launchd / systemd-without-TTY repro and the choice of I've pushed one follow-up commit (
This locks in the contract so nobody can quietly "clean up" the separator back to Hope that's OK with you — happy to drop the follow-up commit and merge as-is if you'd rather land the test in a separate PR. LGTM otherwise. 🚢 |
Ark0N
pushed a commit
that referenced
this pull request
Sep 23, 2026
Codeman creates every tmux pane with `remain-on-exit on`. When the agent exits,
tmux keeps the pane, the tmux session, and the `tmux attach-session` process
Codeman records as the session's pid, so no PTY exit handler fires and nothing
writes the exit down. tmux itself knows: it marks the pane dead and reports the
exit status. This reads that.
`PANE_LIST_FORMAT` gains `#{pane_dead}`, `#{pane_dead_status}` and
`#{pane_dead_signal}`, and `startPaneExitWatcher()` refreshes a
muxName-to-observation map from ONE batched `tmux list-panes -a` per tick. Boot
reconciliation already ran that same call, so it now fills the map too and
recovery starts with a reading.
The watcher owns its own interval rather than riding `startStatsCollection()`,
which the issue suggested. That collector is armed when a browser opens the
Monitor panel and DISARMED when it closes it, and boot skips it entirely unless
recovery found a live session, so a session created on a freshly booted server
would publish nothing and one browser could turn detection off for every other.
Measured on an isolated instance: a dead pane with status 0 reported nothing
until `POST /api/mux-sessions/stats/start` was called by hand. It is still one
batched read per tick; only the timer changed.
Three rules keep a positive answer trustworthy. A session answers only when
tmux listed exactly one pane for it, because Codeman never splits a pane and a
session the user split by hand has none that speaks for the agent. A pane
answers only when `#{pane_dead}` said 1 or 0, because an empty field is a tmux
that did not answer. An absent status stays absent rather than becoming 0:
measured on tmux 3.2a, a SIGKILLed pane reports neither a status nor a signal,
and calling that a clean exit would be wrong in the direction that matters.
Two guards stop a slow read undoing a fast one. `EXEC_TIMEOUT_MS` is 5000 ms
against a 2000 ms interval, so a read can outlive two ticks: one already in
flight suppresses the next, and a generation counter that every
`clearPaneExit()` bumps discards a read that started before a respawn or a
kill. An observation also carries its pane pid, so a second command in the same
pane that exits the same way starts a new timestamp rather than inheriting the
first death's.
A non-empty read of `list-panes -a` is authoritative for the whole socket, so
sessions missing from it are pruned, which also bounds the map as tmux sessions
come and go outside `killSession()`. A failed or empty read retracts nothing.
The manager reports the raw pane reading and applies no session-shape scoping,
because the remote-reconnect watcher beside it needs exactly that raw reading.
`parsePaneList` becomes `parsePaneRows`, returning one row per pane instead of
a name-to-pid map; reconciliation builds its map from the rows. The parser's
existing cases carry over unchanged, including the launchd/systemd literal-tab
regression from PR #71.
Refs #446.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Summary
reconcileSessions()invokestmux list-panes -a -F '#{session_name}\t#{pane_pid}'and parses the result withline.indexOf('\t').TTYPath, Dockerexecwithout TTY) tmux emits the\tin FORMAT strings as the literal two characters\+t, not as a tab (0x09). The parser therefore never matches,activeSessionsstays empty,reconcilereturns{alive: [], dead: [], discovered: []}, andcleanupStaleSessions()wipes every entry instate.jsoneven though the underlying tmux sessions are still alive.npm run devhides this because tmux's FORMAT parser resolves\twhen stdout is a TTY.Root cause
Introduced in
88c415f(perf: batch tmux reconciliation) — the regression only manifests in production deployments under launchd/systemd.Fix
Switch the separator to
|. tmux passes it through verbatim in every environment, and|is rejected by tmux's own session-name validation so it cannot collide with thecodeman-<uuid>/claudeman-<uuid>naming scheme.Test plan
[StateStore] Cleaned up N stale session(s) from statewhiletmux lsshows the sessions alive.[Server] Found N alive mux session(s) from previous run→ sessions restored with tokens and attached PTYs.npm run devstill works: create session → kill-restart server → session restored.npm run typecheck/npm run lint/npm run buildall pass.