mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 10:39:32 +01:00
fix: let go of an adopted tab whose tmux session ended (release review)
When a foreign session ended, remain-on-exit kept Codeman's own tmux client on the wrapper's dead pane, so the tab kept its pid and busy, input went nowhere, both attach routes refused it, and a new session of that name only led back to the dead tab. paneExit stays out of it by design. The reconnect watcher's tick now detaches a client from a dead wrapper. That is an ordinary PTY exit with status 0, so the pid goes null and selecting the tab runs the attach again: it joins the target if it is back and otherwise prints the existing guard text. Checked on an isolated instance and in a headless browser: boot recovery, target gone, target back, view detached, tab closed, and no restart loop while the tab sits unselected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
82593a4dd6
commit
e7c68f1340
3 files changed
+125
-1
No files matched your search
@@ -111,7 +111,7 @@ Input arriving during a wake is buffered: a chunk over 4 KB is dropped whole, ne
|
||||
|
||||
**No workspace.** ⚠️ An adopted session keeps its connection on `adopt.remote`/`adopt.docker`, so `session.remote` and `session.docker` are undefined, and its `workingDir` is the neutral `/tmp`. Code that branches on those two reads an in-container or over-ssh adoption as a local session working in `/tmp`. So anything that treats `workingDir` as the session's workspace must also test `session.isAdopted` (`adopt` on a pure view): uploads, the upload GC, git status, git fetch and its scheduler, the image watcher, Read My Mind's git signals, the Discord folder, `last-response`, guarded input and Ralph's `@fix_plan.md` and `@ralph_prompt.md` routes all do. The file surfaces (Files panel, File Viewer and its edit mode) still treat `/tmp` as the workspace, which is harmless: it is world-writable anyway, and adoption is admin-only in multi-user mode. A wrapper reports `statusLineExporter: 'remote'`, which is also what keeps keep-warm off it.
|
||||
|
||||
**Lifecycle.** `paneExitApplies` is false for a wrapper, so an attach that ended is never `paneExit` and the sweep never closes the tab. Boot recovery re-attaches a dead wrapper, which prints that the session is gone and exits. Reboot restore refuses one, since all Codeman ever had was an attach command. ⚠️ A wrapper that cannot start fails the start, and selecting the tab tries it again (`_failAdoptedStart()`). It never takes the direct-PTY fallback, which would run a new local `claude --dangerously-skip-permissions` or a server-account shell in `/tmp` under the foreign session's name, and `runPrompt()` refuses it for the same reason.
|
||||
**Lifecycle.** `paneExitApplies` is false for a wrapper, so an attach that ended is never `paneExit` and the sweep never closes the tab. ⚠️ `remain-on-exit` still keeps Codeman's own tmux client on the dead pane, where a pid and `busy` hid it and both attach routes refused it, so `detachEndedAdoptedAttaches()` detaches that client on the reconnect watcher's tick: the tab closes nothing, its pid goes null, and selecting it (or "Go to tab" once a session of that name is back) runs the guard again. Boot recovery re-attaches a dead wrapper, which prints that the session is gone and exits. Reboot restore refuses one, since all Codeman ever had was an attach command. ⚠️ A wrapper that cannot start fails the start, and selecting the tab tries it again (`_failAdoptedStart()`). It never takes the direct-PTY fallback, which would run a new local `claude --dangerously-skip-permissions` or a server-account shell in `/tmp` under the foreign session's name, and `runPrompt()` refuses it for the same reason.
|
||||
|
||||
### Docker cases
|
||||
|
||||
|
||||
@@ -499,6 +499,19 @@ export function hasObservablePaneSession(sessions: Iterable<MuxSession>): boolea
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Which adopted wrappers have a dead pane with a tmux client still attached, from rows of
|
||||
* `#{pane_dead} #{session_attached} #{session_name}`. Exported for unit testing.
|
||||
*/
|
||||
export function endedAdoptedAttaches(rows: string, adopted: ReadonlySet<string>): string[] {
|
||||
const ended: string[] = [];
|
||||
for (const row of rows.split('\n')) {
|
||||
const [dead, attached, muxName] = row.trim().split(' ');
|
||||
if (dead === '1' && Number(attached) > 0 && adopted.has(muxName)) ended.push(muxName);
|
||||
}
|
||||
return ended;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a target pane id from `tmux list-panes -F '#{pane_id}:#{pane_active}'`.
|
||||
* Prefers the active pane and falls back to the first valid pane.
|
||||
@@ -3989,9 +4002,51 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
||||
} catch (err) {
|
||||
console.error('[TmuxManager] Remote reconnect watcher error:', err);
|
||||
}
|
||||
void this.detachEndedAdoptedAttaches().catch((err) => {
|
||||
console.error('[TmuxManager] Adopted attach watcher error:', err);
|
||||
});
|
||||
}, intervalMs);
|
||||
}
|
||||
|
||||
/**
|
||||
* Let go of a wrapper whose pane died (#510). That pane holds the attach to someone
|
||||
* else's tmux session, so it dies when that session ends, the transport drops or the
|
||||
* view gets detached, and `paneExit` rightly reports none of them. But `remain-on-exit`
|
||||
* keeps Codeman's own tmux client on the dead pane, so the tab kept its pid and `busy`,
|
||||
* input went nowhere and both attach routes refused it.
|
||||
*
|
||||
* Detaching that client is an ordinary PTY exit with status 0: the pid goes null, the tab
|
||||
* stays open, and selecting it starts the wrapper again, which joins the target if it is
|
||||
* back and otherwise prints that it is gone. Only Codeman's own socket is touched.
|
||||
*/
|
||||
async detachEndedAdoptedAttaches(): Promise<void> {
|
||||
const adopted = new Set<string>();
|
||||
for (const session of this.sessions.values()) {
|
||||
if (session.adopt) adopted.add(session.muxName);
|
||||
}
|
||||
if (adopted.size === 0) return;
|
||||
for (const muxName of endedAdoptedAttaches(await this.readAttachRows(), adopted)) {
|
||||
await this.detachClients(muxName);
|
||||
}
|
||||
}
|
||||
|
||||
/** Rows for {@link endedAdoptedAttaches}. Empty when tmux did not answer. */
|
||||
protected async readAttachRows(): Promise<string> {
|
||||
if (IS_TEST_MODE) return '';
|
||||
const format = '#{pane_dead} #{session_attached} #{session_name}';
|
||||
const { stdout } = await execAsync(`${this.tmux()} list-panes -a -F '${format}' 2>/dev/null || true`, {
|
||||
encoding: 'utf-8',
|
||||
timeout: EXEC_TIMEOUT_MS,
|
||||
});
|
||||
return stdout;
|
||||
}
|
||||
|
||||
/** Detach every tmux client from one of Codeman's own sessions. */
|
||||
protected async detachClients(muxName: string): Promise<void> {
|
||||
if (IS_TEST_MODE || !isValidMuxName(muxName)) return;
|
||||
await execAsync(`${this.tmux()} detach-client -s "${muxName}"`, { timeout: EXEC_TIMEOUT_MS });
|
||||
}
|
||||
|
||||
stopRemoteReconnectWatcher(): void {
|
||||
if (this.remoteReconnectInterval) {
|
||||
clearInterval(this.remoteReconnectInterval);
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
parsePaneRows,
|
||||
derivePaneExits,
|
||||
hasObservablePaneSession,
|
||||
endedAdoptedAttaches,
|
||||
type PaneRow,
|
||||
resolveActivePaneTarget,
|
||||
} from '../src/tmux-manager.js';
|
||||
@@ -1721,3 +1722,71 @@ describe('hasObservablePaneSession', () => {
|
||||
// the rule is enforced; that pairing is pinned in session-pane-exit.test.ts,
|
||||
// where a real Session can answer for itself.
|
||||
});
|
||||
|
||||
describe('an adopted wrapper whose pane died', () => {
|
||||
const adopted = new Set(['codeman-aaaa1111']);
|
||||
|
||||
class WatchedManager extends TmuxManager {
|
||||
rows = '';
|
||||
detached: string[] = [];
|
||||
protected override async readAttachRows(): Promise<string> {
|
||||
return this.rows;
|
||||
}
|
||||
protected override async detachClients(muxName: string): Promise<void> {
|
||||
this.detached.push(muxName);
|
||||
}
|
||||
adopt(sessionId: string, adopt?: MuxSession['adopt']): void {
|
||||
const session = {
|
||||
sessionId,
|
||||
muxName: `codeman-${sessionId}`,
|
||||
pid: 1,
|
||||
createdAt: 0,
|
||||
workingDir: '/tmp',
|
||||
mode: 'shell',
|
||||
attached: false,
|
||||
adopt,
|
||||
};
|
||||
(this as unknown as { sessions: Map<string, MuxSession> }).sessions.set(sessionId, session as MuxSession);
|
||||
}
|
||||
}
|
||||
const adoption = {
|
||||
location: 'local',
|
||||
socketPath: '/tmp/s',
|
||||
targetSession: 'work',
|
||||
viewSession: 'codeman-view-aaaa1111',
|
||||
} as MuxSession['adopt'];
|
||||
|
||||
it('reads as ended only with a dead pane and a client still on it', () => {
|
||||
expect(endedAdoptedAttaches('1 1 codeman-aaaa1111\n', adopted)).toEqual(['codeman-aaaa1111']);
|
||||
expect(endedAdoptedAttaches('1 2 codeman-aaaa1111', adopted)).toEqual(['codeman-aaaa1111']);
|
||||
// Nothing to detach: a second try would only print `no current client` every tick.
|
||||
expect(endedAdoptedAttaches('1 0 codeman-aaaa1111', adopted)).toEqual([]);
|
||||
expect(endedAdoptedAttaches('0 1 codeman-aaaa1111', adopted)).toEqual([]);
|
||||
expect(endedAdoptedAttaches('', adopted)).toEqual([]);
|
||||
});
|
||||
|
||||
it("leaves every other session's dead pane alone, since that one is an agent's exit", () => {
|
||||
expect(endedAdoptedAttaches('1 1 codeman-bbbb2222', adopted)).toEqual([]);
|
||||
});
|
||||
|
||||
it('detaches the client from a dead wrapper on the tick', async () => {
|
||||
const manager = new WatchedManager();
|
||||
manager.adopt('aaaa1111', adoption);
|
||||
manager.adopt('bbbb2222');
|
||||
manager.rows = '1 1 codeman-aaaa1111\n1 1 codeman-bbbb2222\n';
|
||||
|
||||
await manager.detachEndedAdoptedAttaches();
|
||||
|
||||
expect(manager.detached).toEqual(['codeman-aaaa1111']);
|
||||
});
|
||||
|
||||
it('does not read tmux while nothing is adopted', async () => {
|
||||
const manager = new WatchedManager();
|
||||
manager.adopt('bbbb2222');
|
||||
const read = vi.spyOn(manager as unknown as { readAttachRows: () => Promise<string> }, 'readAttachRows');
|
||||
|
||||
await manager.detachEndedAdoptedAttaches();
|
||||
|
||||
expect(read).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user