BitSquare/Codeman
2
0
Fork 0
mirror of https://github.com/Ark0N/Codeman.git synced 2026-10-05 06:59:42 +02:00
Code Issues Packages Projects Releases Wiki Activity

feat(tmux): report a dead pane's exit from the batched pane list

Browse Source 
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 Ark0N/Codeman#446.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Michael Grundberg
2026-09-21 10:00:54 +02:00
co-authored by Claude Opus 5
parent 9466acfc1a
commit 02dc46dcd7
4 changed files with 546 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/tmux-manager.test.ts
+170 -17
View File
@@ -14,7 +14,8 @@ import {
buildRemoteKillCommand,
buildRemoteLaunchCommand,
formatPaneSnapshot,
parsePaneList,
parsePaneRows,
derivePaneExits,
resolveActivePaneTarget,
} from '../src/tmux-manager.js';
import { execSync, exec } from 'node:child_process';
@@ -910,41 +911,44 @@ describe('TmuxManager (unit)', () => {
// exec without TTY). See PR #71.
// ============================================================================
describe('parsePaneList', () => {
describe('parsePaneRows', () => {
/** Pull the name → pid map reconciliation builds, so these cases read as they used to. */
const pids = (output: string) => new Map(parsePaneRows(output).map((row) => [row.sessionName, row.pid]));
it('parses well-formed output into name → pid', () => {
const out = 'codeman-aaaa|1234\ncodeman-bbbb|5678\nclaudeman-cccc|9999';
const result = parsePaneList(out);
const result = pids(out);
expect(result.size).toBe(3);
expect(result.get('codeman-aaaa')).toBe(1234);
expect(result.get('codeman-bbbb')).toBe(5678);
expect(result.get('claudeman-cccc')).toBe(9999);
});
it('returns an empty map for empty output', () => {
expect(parsePaneList('').size).toBe(0);
it('returns no rows for empty output', () => {
expect(parsePaneRows('')).toEqual([]);
});
it('skips blank lines', () => {
const result = parsePaneList('\ncodeman-aaaa|100\n\n\ncodeman-bbbb|200\n');
const result = pids('\ncodeman-aaaa|100\n\n\ncodeman-bbbb|200\n');
expect(result.size).toBe(2);
expect(result.get('codeman-aaaa')).toBe(100);
expect(result.get('codeman-bbbb')).toBe(200);
});
it('skips lines without the separator', () => {
const result = parsePaneList('codeman-aaaa 1234\ncodeman-bbbb|5678');
const result = pids('codeman-aaaa 1234\ncodeman-bbbb|5678');
expect(result.size).toBe(1);
expect(result.get('codeman-bbbb')).toBe(5678);
});
it('skips lines with a non-numeric pid', () => {
const result = parsePaneList('codeman-aaaa|notapid\ncodeman-bbbb|5678');
const result = pids('codeman-aaaa|notapid\ncodeman-bbbb|5678');
expect(result.size).toBe(1);
expect(result.get('codeman-bbbb')).toBe(5678);
});
it('skips lines with an empty session name', () => {
const result = parsePaneList('|1234\ncodeman-bbbb|5678');
const result = pids('|1234\ncodeman-bbbb|5678');
expect(result.size).toBe(1);
expect(result.get('codeman-bbbb')).toBe(5678);
});
@@ -955,15 +959,164 @@ describe('parsePaneList', () => {
// tab byte. With the '|' separator, such literals must not be silently
// treated as a delimiter — the line is discarded because there is no '|'.
const literalBackslashT = 'codeman-aaaa\\t1234';
const result = parsePaneList(literalBackslashT);
expect(result.size).toBe(0);
expect(parsePaneRows(literalBackslashT)).toEqual([]);
});
it('splits on the first separator only', () => {
// Numeric trailing junk after the pid is tolerated by parseInt — proves
// that splitting on the first '|' leaves the pid extractable even if a
// future tmux ever appended extra fields.
const result = parsePaneList('codeman-aaaa|1234|extra-field');
expect(result.get('codeman-aaaa')).toBe(1234);
it('keeps a row whose pane_dead fields are missing, and calls its deadness unknown', () => {
// A tmux old enough to have shipped the previous two-field format, or one
// that dropped the trailing fields, must still yield its pid.
const [row] = parsePaneRows('codeman-aaaa|1234');
expect(row.pid).toBe(1234);
expect(row.dead).toBeUndefined();
expect(row.exitStatus).toBeUndefined();
expect(row.exitSignal).toBeUndefined();
});
it('reads a live pane as not dead, with no status or signal', () => {
// Measured against tmux 3.2a: a live pane leaves both numeric fields blank.
const [row] = parsePaneRows('codeman-aaaa|1234|0|||1');
expect(row.dead).toBe(false);
expect(row.exitStatus).toBeUndefined();
expect(row.exitSignal).toBeUndefined();
});
it('reads a dead pane with its exit status', () => {
const [row] = parsePaneRows('codeman-aaaa|1234|1|7|');
expect(row.dead).toBe(true);
expect(row.exitStatus).toBe(7);
expect(row.exitSignal).toBeUndefined();
});
it('reads a dead pane with its killing signal', () => {
const [row] = parsePaneRows('codeman-aaaa|1234|1||9');
expect(row.dead).toBe(true);
expect(row.exitStatus).toBeUndefined();
expect(row.exitSignal).toBe(9);
});
it('leaves a status of 0 as 0 rather than dropping it', () => {
// The whole point of the field: a clean exit is the case part 2 acts on.
const [row] = parsePaneRows('codeman-aaaa|1234|1|0|');
expect(row.exitStatus).toBe(0);
});
it('calls a non-numeric dead flag unknown rather than false', () => {
const [row] = parsePaneRows('codeman-aaaa|1234|?||');
expect(row.dead).toBeUndefined();
});
it('returns one row per pane of a split session, in tmux order', () => {
const rows = parsePaneRows('codeman-aaaa|100|0|||\ncodeman-aaaa|200|1|0|');
expect(rows.map((row) => row.pid)).toEqual([100, 200]);
expect(rows.map((row) => row.sessionName)).toEqual(['codeman-aaaa', 'codeman-aaaa']);
});
});
describe('derivePaneExits', () => {
const NOW = 1_700_000_000_000;
it('reports a single dead pane with its exit status', () => {
const exits = derivePaneExits(parsePaneRows('codeman-aaaa|1234|1|0|'), NOW);
expect(exits.get('codeman-aaaa')).toEqual({ panePid: 1234, exit: { status: 0, at: NOW } });
});
it('reports a signalled death without inventing a status', () => {
// Folding an absent status into 0 would turn an unexplained death into the
// clean exit part 2 closes on sight.
const exits = derivePaneExits(parsePaneRows('codeman-aaaa|1234|1||9'), NOW);
expect(exits.get('codeman-aaaa')).toEqual({ panePid: 1234, exit: { signal: 9, at: NOW } });
});
it('reports a death tmux could not explain at all', () => {
// Measured on tmux 3.2a: a SIGKILLed pane reports pane_dead=1 and nothing else.
const exits = derivePaneExits(parsePaneRows('codeman-aaaa|1234|1||'), NOW);
expect(exits.get('codeman-aaaa')).toEqual({ panePid: 1234, exit: { at: NOW } });
});
it('says nothing about a live pane', () => {
const exits = derivePaneExits(parsePaneRows('codeman-aaaa|1234|0|||'), NOW);
expect(exits.has('codeman-aaaa')).toBe(false);
});
it('says nothing about a pane whose deadness tmux did not report', () => {
expect(derivePaneExits(parsePaneRows('codeman-aaaa|1234'), NOW).size).toBe(0);
});
it('says nothing about a session with more than one pane, even when all are dead', () => {
// A session the user split by hand has no single "the agent" to report on,
// and guessing which pane speaks for it could call a live session exited.
const exits = derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|\ncodeman-aaaa|200|1|0|'), NOW);
expect(exits.size).toBe(0);
});
it("answers per session, so one session's split does not silence another", () => {
const exits = derivePaneExits(
parsePaneRows('codeman-aaaa|100|1|0|\ncodeman-bbbb|200|1|0|\ncodeman-bbbb|201|0|||'),
NOW
);
expect([...exits.keys()]).toEqual(['codeman-aaaa']);
});
});
describe('TmuxManager pane-exit bookkeeping', () => {
const NOW = 1_700_000_000_000;
it('reports nothing before any tick has run', () => {
const manager = new TmuxManager();
expect(manager.getPaneExit('codeman-aaaa')).toBeUndefined();
});
it('keeps the timestamp of the FIRST tick that saw an unchanged exit', () => {
// The stamp says when the agent was found gone, so a pane that stays dead
// must not have its age reset every two seconds.
const manager = new TmuxManager();
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW));
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW + 2000));
expect(manager.getPaneExit('codeman-aaaa')).toEqual({ status: 0, at: NOW });
});
it('starts a new observation when the exit status changes', () => {
const manager = new TmuxManager();
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW));
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|137|'), NOW + 2000));
expect(manager.getPaneExit('codeman-aaaa')).toEqual({ status: 137, at: NOW + 2000 });
});
it('forgets the exit once the same session reports a live pane', () => {
const manager = new TmuxManager();
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW));
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|101|0|||'), NOW + 2000));
expect(manager.getPaneExit('codeman-aaaa')).toBeUndefined();
});
it('prunes an exit for a session an authoritative read did not mention', () => {
// `list-panes -a` lists every pane on the socket, so a session missing from
// a successful read has no pane at all and no exit to report. Keeping the
// entry would grow the map forever as tmux sessions come and go outside
// killSession(). A FAILED or empty read never reaches here — refreshPaneExits
// returns before calling this, which is the case the next test covers.
const manager = new TmuxManager();
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW));
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-bbbb|200|1|0|'), NOW));
expect(manager.getPaneExit('codeman-aaaa')).toBeUndefined();
expect(manager.getPaneExit('codeman-bbbb')).toEqual({ status: 0, at: NOW });
});
it('starts a new observation when the same status comes from a different pane pid', () => {
// A second command in the same pane that also exited 0 is a NEW death, and
// its `at` must say so. Only reachable when the respawn bypassed
// respawnPane() — a hand-run `tmux respawn-pane` — since every Codeman path
// clears the entry outright.
const manager = new TmuxManager();
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW));
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|101|1|0|'), NOW + 60_000));
expect(manager.getPaneExit('codeman-aaaa')).toEqual({ status: 0, at: NOW + 60_000 });
});
it('forgets an exit on request, which is what a respawned pane needs', () => {
const manager = new TmuxManager();
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW));
manager.clearPaneExit('codeman-aaaa');
expect(manager.getPaneExit('codeman-aaaa')).toBeUndefined();
});
});