mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
feat(session): close sessions whose agent exited cleanly (#486)
* fix(cleanup): keep .claude-images while a sibling session uses the same dir
cleanupSession() recursively removes {workingDir}/.claude-images. That
directory belongs to the working directory rather than to the session, and
several sessions routinely share one case directory, so closing one session
deleted the pasted images a live sibling still referred to.
The removal now runs only when no other live session has the same working
directory. A session that is itself being cleaned up does not count as live,
so two sessions of one case closed together still remove the dir.
Split out ahead of the exited-agent sweep for Ark0N/Codeman#446, which closes
sessions unattended and would otherwise make the loss routine.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* feat(session): close sessions whose agent exited cleanly (#446)
Part 2 of Ark0N/Codeman#446. Part 1 records an exited agent as
SessionState.paneExit. A session whose agent the user ended with /exit is
now closed through cleanupSession(), the same path the X button takes, so
finished sessions stop piling up on the board. The lifecycle log records
the reason as "agent exited cleanly (status 0)", and the conversation stays
resumable from the Resume list.
shouldCloseCleanlyExitedSession() in the new pure module pane-exit-sweep.ts
holds the rule. It closes a session only when all of these hold:
- The exit status is an explicit numeric 0 with no signal. An absent status
is how a SIGKILL presents on tmux 3.2a, so it counts as unknown and the
row stays. A non-zero status or any signal also keeps the row, with the
exit code on the tab.
- Two authoritative pane reads agreed on that exit.
TmuxManager.getPaneExitReadCount() counts them, and a failed, empty or
skipped read neither confirms nor resets the count.
- No start, attach or relaunch is running for the pane.
Session.paneLifecycleInFlight covers _setupOrAttachMuxSession(), whose
dead-pane branch revives an exited pane on purpose, and restartCli().
setPaneExit() already scopes paneExit to local mux-backed sessions, so
remote, docker and direct-PTY sessions are never closed.
planRebootRestore() now refuses a record whose persisted paneExit is a
clean exit. That covers an agent that exited just before a reboot, before
the sweep reached it. A crashed agent's record stays eligible, like its row.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* fix(web): show "exited" on the phone overview and desktop home rail (#446)
Part 1 of Ark0N/Codeman#446 taught the tab strip and the rich rail rows
to say that a session's agent has exited. The phone overview and the
desktop home rail still said "idle", beside a green or pulsing dot.
_mobileOverviewExit() in mobile-overview.js is now the one rule for all
three surfaces, and _sidebarRichRow() uses it as well. It changes what a
row shows and leaves the row's state alone, because the state still picks
the section and the sort order. An exited row gets an "exited" pill, a
neutral dot and row accent, and a duration measured from when the server
first saw the pane dead. A pending permission prompt or question still
wins, as it does on the tab.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* fix(cleanup): close the gaps review found in the #446 sweep and image guard
Four fixes from a dual review of Ark0N/Codeman#446 part 2.
- The .claude-images guard compares canonical paths, so a sibling that
reaches the same directory through a symlink keeps it. Its comment used to
say that case only missed a deletion; it caused one.
- A detached session counts as a live sibling. DELETE ?killMux=false removes
it from the server's map while its pane keeps running, so the guard now
reads persisted records too, and exempts only sessions being killed rather
than every session in cleaningUp.
- A session being closed refuses startInteractive() and startShell(). The
/interactive route awaits listener setup before the start, and a start
that raced the close could launch a CLI in a tmux session whose record was
then deleted. A failed close clears the mark again.
- The clean-exit sweep tries each exit once, keyed by session id and the
exit's at stamp, so a close that fails is not retried and logged every
two seconds.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* fix(session): keep a clean exit that lands within 10 s of a pane start (#446)
A CLI that prints a startup error ("not logged in", a bad profile, a config
error) and exits 0 used to lose its tab, and the error with it, about 4 s
after launch. The sweep now keeps any clean exit that lands within
CLEAN_EXIT_MIN_PANE_LIFETIME_MS (10 s) of the last start, attach or relaunch
finishing (Session.paneStartedAt, stamped when _withPaneLifecycle ends). The
row stays as "exited (0)" for the user to read and close.
Verified on an isolated instance: a shell that ran `exit 0` 2 s after start
kept its row, one that exited after 13 s was closed.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Michael Grundberg
co-authored by
Claude Opus 5.5
Claude Opus 5.5
parent
d67da5c9d0
commit
b80d47aff8
@@ -0,0 +1,194 @@
|
||||
// Port: none (pure model + fake-DOM row builders — no browser, no server).
|
||||
//
|
||||
// The phone overview and the desktop home rail showing an exited agent as
|
||||
// "exited" rather than "idle" (Ark0N/Codeman#446).
|
||||
//
|
||||
// The server publishes `session.paneExit` when the agent inside a local tmux
|
||||
// pane has exited, while `status` stays `idle` or `busy` by design. Part 1 of
|
||||
// #446 taught the tab strip and the rich rail rows to say so; both home screens
|
||||
// still said "idle" beside nothing running. `_mobileOverviewExit()`
|
||||
// (mobile-overview.js) is now the one rule all three surfaces read. It changes
|
||||
// what a row SHOWS and leaves its `state` alone, because `state` picks the
|
||||
// section and the sort order.
|
||||
//
|
||||
// `paneExitLabel()` is lifted out of the shipped app.js rather than restated
|
||||
// here, so a change to what counts as "exited" there reaches these tests.
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
const PUBLIC = resolve(import.meta.dirname, '../src/web/public');
|
||||
|
||||
/** The shipped `paneExitLabel()` from app.js, as source text. */
|
||||
function paneExitLabelSource(): string {
|
||||
const appJs = readFileSync(resolve(PUBLIC, 'app.js'), 'utf8');
|
||||
const match = appJs.match(/function paneExitLabel\(paneExit\) \{[\s\S]*?\n\}\n/);
|
||||
if (!match) throw new Error('paneExitLabel() not found in app.js');
|
||||
return match[0];
|
||||
}
|
||||
|
||||
function fakeElement(): any {
|
||||
const el: any = {
|
||||
className: '',
|
||||
type: '',
|
||||
title: '',
|
||||
textContent: '',
|
||||
dataset: {},
|
||||
style: {},
|
||||
children: [] as any[],
|
||||
setAttribute() {},
|
||||
appendChild(child: any) {
|
||||
el.children.push(child);
|
||||
return child;
|
||||
},
|
||||
};
|
||||
return el;
|
||||
}
|
||||
|
||||
/** Every className in a fake-DOM subtree, depth first. */
|
||||
function classNames(el: any): string[] {
|
||||
return [el.className, ...(el.children || []).flatMap(classNames)].filter(Boolean);
|
||||
}
|
||||
|
||||
function loadApp({ withPaneExitLabel = true } = {}) {
|
||||
const CodemanApp = function CodemanApp(this: any) {};
|
||||
const context = vm.createContext({
|
||||
CodemanApp,
|
||||
console,
|
||||
window: { innerWidth: 1512 },
|
||||
document: {
|
||||
documentElement: { getAttribute: () => null },
|
||||
getElementById: () => null,
|
||||
createElement: () => fakeElement(),
|
||||
createElementNS: () => fakeElement(),
|
||||
},
|
||||
MobileDetection: { getDeviceType: () => 'desktop' },
|
||||
});
|
||||
if (withPaneExitLabel) vm.runInContext(paneExitLabelSource(), context, { filename: 'app.js' });
|
||||
for (const file of ['constants.js', 'mobile-overview.js', 'home-sessions.js']) {
|
||||
vm.runInContext(readFileSync(resolve(PUBLIC, file), 'utf8'), context, { filename: file });
|
||||
}
|
||||
const app = new (CodemanApp as any)();
|
||||
app.getSessionName = (session: any) => session.name || session.id.slice(0, 8);
|
||||
app._shortenHomePath = (p: string) => p || '';
|
||||
app.loadAppSettingsFromStorage = () => ({});
|
||||
app.cases = [];
|
||||
app.pendingHooks = new Map();
|
||||
return app;
|
||||
}
|
||||
|
||||
const EXIT_AT = 1_700_000_000_000;
|
||||
|
||||
function sessions(list: Array<Record<string, any>>) {
|
||||
return new Map(list.map((s) => [s.id, { status: 'idle', mode: 'claude', workingDir: '/w', ...s }]));
|
||||
}
|
||||
|
||||
describe('the desktop home rail', () => {
|
||||
it('says "exited" for an idle session whose agent exited, and measures from the exit', () => {
|
||||
const app = loadApp();
|
||||
app.sessions = sessions([{ id: 'gone', paneExit: { status: 3, at: EXIT_AT }, lastActivityAt: 5 }]);
|
||||
app.sessionOrder = ['gone'];
|
||||
|
||||
const [row] = app.buildHomeSessionRows();
|
||||
expect(row.state).toBe('idle');
|
||||
expect(row.display).toBe('exited');
|
||||
expect(row.pill).toBe('exited');
|
||||
expect(row.since).toEqual({ key: 'exited', at: EXIT_AT });
|
||||
});
|
||||
|
||||
it('draws a neutral row: no idle or working class on the row, the dot or the pill', () => {
|
||||
// A pane whose agent died mid-turn still has `status: 'busy'`, so without
|
||||
// the display class its dot would pulse green beside "exited".
|
||||
const app = loadApp();
|
||||
app.sessions = sessions([{ id: 'gone', status: 'busy', paneExit: { at: EXIT_AT } }]);
|
||||
app.sessionOrder = ['gone'];
|
||||
|
||||
const [row] = app.buildHomeSessionRows();
|
||||
expect(row.state).toBe('working');
|
||||
const classes = classNames(app._buildHomeSessionRow(row));
|
||||
expect(classes).toEqual(
|
||||
expect.arrayContaining([
|
||||
'home-sessions-row home-sessions-row--exited',
|
||||
'home-sessions-dot home-sessions-dot--exited',
|
||||
'home-sessions-pill home-sessions-pill--exited',
|
||||
])
|
||||
);
|
||||
expect(classes.join(' ')).not.toMatch(/--(working|idle)\b/);
|
||||
});
|
||||
|
||||
it('lets a pending permission prompt win over the exit', () => {
|
||||
const app = loadApp();
|
||||
app.sessions = sessions([{ id: 'blocked', paneExit: { status: 0, at: EXIT_AT } }]);
|
||||
app.sessionOrder = ['blocked'];
|
||||
app.pendingHooks = new Map([['blocked', new Set(['permission_prompt'])]]);
|
||||
|
||||
const [row] = app.buildHomeSessionRows();
|
||||
expect(row.display).toBe('needs');
|
||||
expect(row.pill).toBe('needs you');
|
||||
});
|
||||
|
||||
it('leaves a session without an exit exactly as it was', () => {
|
||||
const app = loadApp();
|
||||
app.sessions = sessions([{ id: 'live', lastActivityAt: 5 }]);
|
||||
app.sessionOrder = ['live'];
|
||||
|
||||
const [row] = app.buildHomeSessionRows();
|
||||
expect(row.display).toBe('idle');
|
||||
expect(row.pill).toBe('idle');
|
||||
expect(row.since).toEqual({ key: 'idle', at: 5 });
|
||||
});
|
||||
|
||||
it('degrades to no override when a stale cached app.js lacks paneExitLabel', () => {
|
||||
const app = loadApp({ withPaneExitLabel: false });
|
||||
app.sessions = sessions([{ id: 'gone', paneExit: { status: 3, at: EXIT_AT } }]);
|
||||
app.sessionOrder = ['gone'];
|
||||
|
||||
expect(app.buildHomeSessionRows()[0].pill).toBe('idle');
|
||||
});
|
||||
});
|
||||
|
||||
describe('the phone overview', () => {
|
||||
it('says "exited" and keeps the row in its section', () => {
|
||||
const app = loadApp();
|
||||
const model = app.buildMobileOverviewModel({
|
||||
sessions: sessions([
|
||||
{ id: 'gone', paneExit: { status: 137, at: EXIT_AT } },
|
||||
{ id: 'live', lastActivityAt: 5 },
|
||||
]),
|
||||
cases: [],
|
||||
sessionOrder: ['gone', 'live'],
|
||||
});
|
||||
|
||||
const byId = Object.fromEntries(model.current.map((r: any) => [r.id, r]));
|
||||
expect(byId.gone).toMatchObject({ state: 'idle', display: 'exited', pill: 'exited' });
|
||||
expect(byId.gone.since).toEqual({ key: 'exited', at: EXIT_AT });
|
||||
expect(byId.live).toMatchObject({ display: 'idle', pill: 'idle' });
|
||||
});
|
||||
|
||||
it('draws a neutral row', () => {
|
||||
const app = loadApp();
|
||||
const model = app.buildMobileOverviewModel({
|
||||
sessions: sessions([{ id: 'gone', status: 'busy', paneExit: { at: EXIT_AT } }]),
|
||||
cases: [],
|
||||
});
|
||||
app._pendingApprovalForSession = () => null;
|
||||
|
||||
const classes = classNames(app._buildMobileOverviewRow(model.current[0]));
|
||||
expect(classes).toEqual(
|
||||
expect.arrayContaining([
|
||||
'mobile-overview-row mobile-overview-row--exited',
|
||||
'mobile-overview-dot mobile-overview-dot--exited',
|
||||
'mobile-overview-pill mobile-overview-pill--exited',
|
||||
])
|
||||
);
|
||||
expect(classes.join(' ')).not.toMatch(/--(working|idle)\b/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the exited pill styles', () => {
|
||||
it('gives both home screens a neutral exited pill', () => {
|
||||
expect(readFileSync(resolve(PUBLIC, 'styles.css'), 'utf8')).toMatch(/\.home-sessions-pill--exited \{/);
|
||||
expect(readFileSync(resolve(PUBLIC, 'mobile.css'), 'utf8')).toMatch(/\.mobile-overview-pill--exited \{/);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,330 @@
|
||||
/**
|
||||
* @fileoverview The clean-exit sweep (Ark0N/Codeman#446, part 2).
|
||||
*
|
||||
* A session whose agent the user ended with `/exit` is closed the way the X
|
||||
* button closes it, so finished sessions stop piling up on the board. A crashed
|
||||
* agent keeps its row, marked with the exit code.
|
||||
*
|
||||
* Each rule pinned here guards against a specific wrong close:
|
||||
*
|
||||
* 1. **Only an explicit numeric 0 is clean.** On tmux 3.2a a SIGKILLed pane
|
||||
* reports neither a status nor a signal, so an absent status is the
|
||||
* crashed-agent case, not the clean one.
|
||||
* 2. **Two agreeing reads, not one.** A single reading can describe a pane
|
||||
* that is about to be revived.
|
||||
* 3. **Never while a start or attach is in flight.** The dead-pane respawn in
|
||||
* `_setupOrAttachMuxSession()` revives an exited pane on purpose, and the
|
||||
* pane reads as dead until it finishes.
|
||||
* 4. **Only local mux-backed sessions.** A remote session's local pane is its
|
||||
* ssh client, whose death may be a transport drop.
|
||||
*
|
||||
* Port: 3189
|
||||
*/
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import { Session } from '../src/session.js';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
import type { PaneExit, SessionRemote } from '../src/types.js';
|
||||
import type { MuxSession, TerminalMultiplexer } from '../src/mux-interface.js';
|
||||
import {
|
||||
CLEAN_EXIT_CLOSE_REASON,
|
||||
CLEAN_EXIT_CONFIRMING_READS,
|
||||
CLEAN_EXIT_MIN_PANE_LIFETIME_MS,
|
||||
isCleanPaneExit,
|
||||
shouldCloseCleanlyExitedSession,
|
||||
} from '../src/pane-exit-sweep.js';
|
||||
|
||||
const PORT = 3189;
|
||||
const AT = 1_700_000_000_000;
|
||||
|
||||
describe('isCleanPaneExit', () => {
|
||||
it('accepts an explicit status of 0 with no signal', () => {
|
||||
expect(isCleanPaneExit({ status: 0, at: AT })).toBe(true);
|
||||
});
|
||||
|
||||
it('refuses an absent status, which is how a SIGKILL presents on tmux 3.2a', () => {
|
||||
expect(isCleanPaneExit({ at: AT })).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses a non-zero status', () => {
|
||||
expect(isCleanPaneExit({ status: 1, at: AT })).toBe(false);
|
||||
expect(isCleanPaneExit({ status: 137, at: AT })).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses any reported signal, even beside a 0', () => {
|
||||
expect(isCleanPaneExit({ signal: 9, at: AT })).toBe(false);
|
||||
expect(isCleanPaneExit({ status: 0, signal: 15, at: AT })).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses an unknown exit', () => {
|
||||
expect(isCleanPaneExit(undefined)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldCloseCleanlyExitedSession', () => {
|
||||
const clean = {
|
||||
paneExit: { status: 0, at: AT } as PaneExit,
|
||||
confirmingReads: CLEAN_EXIT_CONFIRMING_READS,
|
||||
paneLifecycleInFlight: false,
|
||||
closing: false,
|
||||
paneStartedAt: 0,
|
||||
};
|
||||
|
||||
it('keeps a clean exit that lands within the startup window, as a startup failure', () => {
|
||||
const justStarted = AT - CLEAN_EXIT_MIN_PANE_LIFETIME_MS + 1;
|
||||
expect(shouldCloseCleanlyExitedSession({ ...clean, paneStartedAt: justStarted, confirmingReads: 9 })).toBe(false);
|
||||
// An exit read during the start itself is earlier than the stamp.
|
||||
expect(shouldCloseCleanlyExitedSession({ ...clean, paneStartedAt: AT + 500, confirmingReads: 9 })).toBe(false);
|
||||
});
|
||||
|
||||
it('closes a clean exit once the pane has outlived the startup window', () => {
|
||||
const settled = AT - CLEAN_EXIT_MIN_PANE_LIFETIME_MS;
|
||||
expect(shouldCloseCleanlyExitedSession({ ...clean, paneStartedAt: settled })).toBe(true);
|
||||
});
|
||||
|
||||
it('closes a clean exit that enough reads agreed on', () => {
|
||||
expect(CLEAN_EXIT_CONFIRMING_READS).toBe(2);
|
||||
expect(shouldCloseCleanlyExitedSession(clean)).toBe(true);
|
||||
expect(shouldCloseCleanlyExitedSession({ ...clean, confirmingReads: 5 })).toBe(true);
|
||||
});
|
||||
|
||||
it('waits for the second read', () => {
|
||||
expect(shouldCloseCleanlyExitedSession({ ...clean, confirmingReads: 1 })).toBe(false);
|
||||
expect(shouldCloseCleanlyExitedSession({ ...clean, confirmingReads: 0 })).toBe(false);
|
||||
});
|
||||
|
||||
it('leaves a session alone while its pane is being started or revived', () => {
|
||||
expect(shouldCloseCleanlyExitedSession({ ...clean, paneLifecycleInFlight: true })).toBe(false);
|
||||
});
|
||||
|
||||
it('does not queue a second close for a session already closing', () => {
|
||||
expect(shouldCloseCleanlyExitedSession({ ...clean, closing: true })).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps a crashed agent however many reads saw it', () => {
|
||||
expect(shouldCloseCleanlyExitedSession({ ...clean, paneExit: { status: 137, at: AT }, confirmingReads: 9 })).toBe(
|
||||
false
|
||||
);
|
||||
expect(shouldCloseCleanlyExitedSession({ ...clean, paneExit: { at: AT }, confirmingReads: 9 })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the sweep on a real server', () => {
|
||||
// Drives the real `paneExitsUpdated` wiring: the mux reports a reading, the
|
||||
// event fires, and the test checks whether the server closed the session.
|
||||
let server: WebServer | null = null;
|
||||
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
await server?.stop?.();
|
||||
server = null;
|
||||
});
|
||||
|
||||
const remote: SessionRemote = { hostId: 'h1', label: 'box', host: 'box', user: 'dev' } as SessionRemote;
|
||||
|
||||
/** The mux members `Session` needs to be a local mux-backed session that can be stopped. */
|
||||
const sessionMux = () =>
|
||||
({
|
||||
isAvailable: () => true,
|
||||
killSession: async () => true,
|
||||
}) as unknown as TerminalMultiplexer;
|
||||
|
||||
const addSession = (web: WebServer, extra: Record<string, unknown> = {}) => {
|
||||
const muxName = `codeman-${Math.random().toString(16).slice(2, 10)}`;
|
||||
const session = new Session({
|
||||
workingDir: '/tmp',
|
||||
mode: 'claude',
|
||||
useMux: true,
|
||||
mux: sessionMux(),
|
||||
muxSession: { muxName, sessionId: 'x' } as unknown as MuxSession,
|
||||
...extra,
|
||||
});
|
||||
(web as unknown as { sessions: Map<string, Session> }).sessions.set(session.id, session);
|
||||
return { session, muxName };
|
||||
};
|
||||
|
||||
/** A server whose mux reports `exit` for every pane, seen by `reads` reads. */
|
||||
const build = (exit: PaneExit | undefined, reads: number) => {
|
||||
const web = new WebServer(PORT, false, true);
|
||||
server = web;
|
||||
const mux = (web as unknown as { mux: Record<string, unknown> }).mux;
|
||||
const state = { exit, reads };
|
||||
mux.getPaneExit = () => state.exit;
|
||||
mux.getPaneExitReadCount = () => (state.exit ? state.reads : 0);
|
||||
const tick = () => (mux as unknown as { emit: (e: string) => void }).emit('paneExitsUpdated');
|
||||
const cleanup = vi
|
||||
.spyOn(web as unknown as { cleanupSession: (...a: unknown[]) => Promise<void> }, 'cleanupSession')
|
||||
.mockResolvedValue(undefined);
|
||||
return { web, state, tick, cleanup };
|
||||
};
|
||||
|
||||
it('closes a cleanly exited session on the second read, with a reason in the lifecycle log', () => {
|
||||
const { web, state, tick, cleanup } = build({ status: 0, at: AT }, 1);
|
||||
const { session } = addSession(web);
|
||||
|
||||
tick();
|
||||
expect(cleanup).not.toHaveBeenCalled();
|
||||
// The exit is on the record already, so the tab says "exited (0)" meanwhile.
|
||||
expect(session.paneExit).toEqual({ status: 0, at: AT });
|
||||
|
||||
state.reads = 2;
|
||||
tick();
|
||||
expect(cleanup).toHaveBeenCalledTimes(1);
|
||||
expect(cleanup).toHaveBeenCalledWith(session.id, true, CLEAN_EXIT_CLOSE_REASON);
|
||||
});
|
||||
|
||||
it('keeps a session whose pane started moments before the clean exit', () => {
|
||||
const { web, tick, cleanup } = build({ status: 0, at: AT }, 2);
|
||||
const { session } = addSession(web);
|
||||
(session as unknown as { _paneStartedAt: number })._paneStartedAt = AT - 2_000;
|
||||
|
||||
tick();
|
||||
expect(cleanup).not.toHaveBeenCalled();
|
||||
expect(session.paneExit).toEqual({ status: 0, at: AT });
|
||||
});
|
||||
|
||||
it('tries each exit once, so a failed close is not retried every tick', () => {
|
||||
const { web, tick, cleanup } = build({ status: 0, at: AT }, 2);
|
||||
const { session } = addSession(web);
|
||||
|
||||
tick();
|
||||
tick();
|
||||
tick();
|
||||
expect(cleanup).toHaveBeenCalledTimes(1);
|
||||
// The spy resolved without removing the session, which is what a failed
|
||||
// close looks like from here: the row stays, with its exit badge.
|
||||
expect(session.paneExit).toEqual({ status: 0, at: AT });
|
||||
});
|
||||
|
||||
it('gives a new exit in the same pane its own attempt', () => {
|
||||
const { web, state, tick, cleanup } = build({ status: 0, at: AT }, 2);
|
||||
addSession(web);
|
||||
|
||||
tick();
|
||||
state.exit = { status: 0, at: AT + 60_000 };
|
||||
tick();
|
||||
expect(cleanup).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('keeps a crashed agent on the board', () => {
|
||||
const { web, tick, cleanup } = build({ status: 137, at: AT }, 5);
|
||||
addSession(web);
|
||||
tick();
|
||||
expect(cleanup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps an agent whose exit status tmux did not report', () => {
|
||||
const { web, tick, cleanup } = build({ at: AT }, 5);
|
||||
addSession(web);
|
||||
tick();
|
||||
expect(cleanup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('never closes a remote session, whose local pane is only the ssh client', () => {
|
||||
const { web, tick, cleanup } = build({ status: 0, at: AT }, 5);
|
||||
addSession(web, { remote });
|
||||
tick();
|
||||
expect(cleanup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('leaves a session alone while its pane is being revived', () => {
|
||||
const { web, tick, cleanup } = build({ status: 0, at: AT }, 5);
|
||||
const { session } = addSession(web);
|
||||
vi.spyOn(session, 'paneLifecycleInFlight', 'get').mockReturnValue(true);
|
||||
tick();
|
||||
expect(cleanup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('closes nothing when the mux cannot count its reads', () => {
|
||||
const { web, tick, cleanup } = build({ status: 0, at: AT }, 5);
|
||||
delete (web as unknown as { mux: Record<string, unknown> }).mux.getPaneExitReadCount;
|
||||
addSession(web);
|
||||
tick();
|
||||
expect(cleanup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('removes the session for real through cleanupSession', async () => {
|
||||
// No spy on cleanupSession here: the close runs the same path as the X
|
||||
// button, and the session leaves the server's map.
|
||||
const web = new WebServer(PORT, false, true);
|
||||
server = web;
|
||||
const mux = (web as unknown as { mux: Record<string, unknown> }).mux;
|
||||
mux.getPaneExit = () => ({ status: 0, at: AT });
|
||||
mux.getPaneExitReadCount = () => 2;
|
||||
const { session } = addSession(web);
|
||||
const sessions = (web as unknown as { sessions: Map<string, Session> }).sessions;
|
||||
|
||||
(mux as unknown as { emit: (e: string) => void }).emit('paneExitsUpdated');
|
||||
await vi.waitFor(() => expect(sessions.has(session.id)).toBe(false));
|
||||
});
|
||||
});
|
||||
|
||||
describe('a session the server is closing', () => {
|
||||
const session = () =>
|
||||
new Session({
|
||||
workingDir: '/tmp',
|
||||
mode: 'shell',
|
||||
useMux: true,
|
||||
mux: { isAvailable: () => true } as unknown as TerminalMultiplexer,
|
||||
muxSession: { muxName: 'codeman-aaaa', sessionId: 'aaaa' } as unknown as MuxSession,
|
||||
});
|
||||
|
||||
it('refuses to start, so a racing start cannot orphan a tmux session', async () => {
|
||||
const s = session();
|
||||
s.markClosing(true);
|
||||
await expect(s.startInteractive()).rejects.toThrow('Session is being closed');
|
||||
await expect(s.startShell()).rejects.toThrow('Session is being closed');
|
||||
});
|
||||
|
||||
it('is cleared again when the server gives the close up', async () => {
|
||||
const web = new WebServer(PORT, false, true);
|
||||
try {
|
||||
const s = session();
|
||||
const sessions = (web as unknown as { sessions: Map<string, Session> }).sessions;
|
||||
sessions.set(s.id, s);
|
||||
const internals = web as unknown as {
|
||||
_doCleanupSession: (...a: unknown[]) => Promise<void>;
|
||||
cleanupSession: (id: string, kill: boolean, reason: string) => Promise<void>;
|
||||
};
|
||||
let closingDuring = false;
|
||||
vi.spyOn(internals, '_doCleanupSession').mockImplementation(async () => {
|
||||
closingDuring = (s as unknown as { _closing: boolean })._closing;
|
||||
throw new Error('layout prune unavailable');
|
||||
});
|
||||
|
||||
await expect(internals.cleanupSession(s.id, true, 'test')).rejects.toThrow('layout prune unavailable');
|
||||
expect(closingDuring).toBe(true);
|
||||
expect((s as unknown as { _closing: boolean })._closing).toBe(false);
|
||||
} finally {
|
||||
vi.restoreAllMocks();
|
||||
await web.stop();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('the pane lifecycle mark on Session', () => {
|
||||
it('is raised for the whole of restartCli and lowered afterwards, even on failure', async () => {
|
||||
let seenDuring: boolean | null = null;
|
||||
let session: Session;
|
||||
const mux = {
|
||||
isAvailable: () => true,
|
||||
muxSessionExists: () => true,
|
||||
respawnPane: async () => {
|
||||
seenDuring = session.paneLifecycleInFlight;
|
||||
throw new Error('respawn failed');
|
||||
},
|
||||
clearPaneExit: () => {},
|
||||
} as unknown as TerminalMultiplexer;
|
||||
session = new Session({
|
||||
workingDir: '/tmp',
|
||||
mode: 'claude',
|
||||
useMux: true,
|
||||
mux,
|
||||
muxSession: { muxName: 'codeman-aaaa', sessionId: 'aaaa' } as unknown as MuxSession,
|
||||
});
|
||||
|
||||
expect(session.paneLifecycleInFlight).toBe(false);
|
||||
await expect(session.restartCli()).rejects.toThrow('respawn failed');
|
||||
expect(seenDuring).toBe(true);
|
||||
expect(session.paneLifecycleInFlight).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,165 @@
|
||||
/**
|
||||
* @fileoverview Deleting a session keeps `.claude-images` while a sibling in the
|
||||
* same working directory is still live (Ark0N/Codeman#446).
|
||||
*
|
||||
* `cleanupSession()` removes `{workingDir}/.claude-images` recursively. That
|
||||
* dir belongs to the working directory, not to the session, and several
|
||||
* sessions routinely share one case directory, so closing one used to delete
|
||||
* the pasted images a live sibling still referred to. The exited-agent sweep
|
||||
* closes sessions unattended, which turns that from an occasional loss into a
|
||||
* routine one.
|
||||
*
|
||||
* Port: 3188
|
||||
*/
|
||||
import { existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
import { pasteImageDirInUseByOtherSession } from '../src/web/paste-image-gc.js';
|
||||
|
||||
const PORT = 3188;
|
||||
|
||||
describe('pasteImageDirInUseByOtherSession', () => {
|
||||
const none = new Set<string>();
|
||||
const check = (
|
||||
live: Array<{ id: string; workingDir: string }>,
|
||||
opts: { persisted?: Array<{ id: string; workingDir: string; status?: string }>; killing?: Set<string> } = {}
|
||||
) =>
|
||||
pasteImageDirInUseByOtherSession({
|
||||
live,
|
||||
persisted: opts.persisted ?? [],
|
||||
closingId: 'a',
|
||||
workingDir: '/work/case',
|
||||
killing: opts.killing ?? none,
|
||||
});
|
||||
|
||||
it('finds a live sibling in the same working directory', () => {
|
||||
expect(
|
||||
check([
|
||||
{ id: 'a', workingDir: '/work/case' },
|
||||
{ id: 'b', workingDir: '/work/case' },
|
||||
])
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('ignores the session being closed', () => {
|
||||
expect(check([{ id: 'a', workingDir: '/work/case' }])).toBe(false);
|
||||
});
|
||||
|
||||
it('ignores sessions in other directories, including a subdirectory', () => {
|
||||
expect(
|
||||
check([
|
||||
{ id: 'a', workingDir: '/work/case' },
|
||||
{ id: 'b', workingDir: '/work/other' },
|
||||
{ id: 'c', workingDir: '/work/case/sub' },
|
||||
])
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('normalises a trailing slash and dot segments', () => {
|
||||
expect(check([{ id: 'b', workingDir: '/work/x/../case/' }])).toBe(true);
|
||||
});
|
||||
|
||||
it('does not count a sibling that is being killed too', () => {
|
||||
// Two sessions of one case killed together must not each defer to the
|
||||
// other, or neither removes the dir.
|
||||
expect(check([{ id: 'b', workingDir: '/work/case' }], { killing: new Set(['a', 'b']) })).toBe(false);
|
||||
});
|
||||
|
||||
it('counts a detached session, which left the map but still runs in tmux', () => {
|
||||
// `DELETE ?killMux=false` removes the session from the server's map and
|
||||
// keeps its persisted record and its pane.
|
||||
expect(check([], { persisted: [{ id: 'b', workingDir: '/work/case', status: 'idle' }] })).toBe(true);
|
||||
});
|
||||
|
||||
it('does not count a persisted record demoted to stopped, or the closing session', () => {
|
||||
expect(
|
||||
check([], {
|
||||
persisted: [
|
||||
{ id: 'a', workingDir: '/work/case', status: 'idle' },
|
||||
{ id: 'b', workingDir: '/work/case', status: 'stopped' },
|
||||
],
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('matches a sibling that reaches the same directory through a symlink', () => {
|
||||
const root = mkdtempSync(join(tmpdir(), 'codeman-paste-link-'));
|
||||
try {
|
||||
const real = join(root, 'case');
|
||||
mkdirSync(real);
|
||||
symlinkSync(real, join(root, 'current'));
|
||||
expect(
|
||||
pasteImageDirInUseByOtherSession({
|
||||
live: [{ id: 'b', workingDir: join(root, 'current') }],
|
||||
persisted: [],
|
||||
closingId: 'a',
|
||||
workingDir: real,
|
||||
killing: none,
|
||||
})
|
||||
).toBe(true);
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('deleting a session that shares its working directory', () => {
|
||||
let server: WebServer;
|
||||
let workingDir: string;
|
||||
const base = `http://localhost:${PORT}`;
|
||||
|
||||
beforeAll(async () => {
|
||||
workingDir = mkdtempSync(join(tmpdir(), 'codeman-paste-shared-'));
|
||||
server = new WebServer(PORT, false, true);
|
||||
await server.start();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await server.stop();
|
||||
rmSync(workingDir, { recursive: true, force: true });
|
||||
}, 60000);
|
||||
|
||||
const create = async (): Promise<string> => {
|
||||
const res = await fetch(`${base}/api/sessions`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ workingDir }),
|
||||
});
|
||||
const body = await res.json();
|
||||
return body.data.session.id as string;
|
||||
};
|
||||
|
||||
const remove = (id: string) => fetch(`${base}/api/sessions/${id}`, { method: 'DELETE' });
|
||||
|
||||
it('keeps the images while a sibling is live, and removes them with the last session', async () => {
|
||||
const first = await create();
|
||||
const second = await create();
|
||||
const imageDir = join(workingDir, '.claude-images');
|
||||
mkdirSync(imageDir, { recursive: true });
|
||||
writeFileSync(join(imageDir, 'paste-1.png'), 'x');
|
||||
|
||||
expect((await remove(first)).status).toBe(200);
|
||||
expect(existsSync(join(imageDir, 'paste-1.png'))).toBe(true);
|
||||
|
||||
expect((await remove(second)).status).toBe(200);
|
||||
expect(existsSync(imageDir)).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps the images while a sibling is only detached, since it still runs in tmux', async () => {
|
||||
const detached = await create();
|
||||
const deleted = await create();
|
||||
// Persisting a new record is debounced, and a detach cancels the pending
|
||||
// write, so wait for the record a long-running session would already have.
|
||||
const store = (server as unknown as { store: { getSession: (id: string) => unknown } }).store;
|
||||
await vi.waitFor(() => expect(store.getSession(detached)).toBeTruthy(), { timeout: 10_000 });
|
||||
const imageDir = join(workingDir, '.claude-images');
|
||||
mkdirSync(imageDir, { recursive: true });
|
||||
writeFileSync(join(imageDir, 'paste-2.png'), 'x');
|
||||
|
||||
expect((await fetch(`${base}/api/sessions/${detached}?killMux=false`, { method: 'DELETE' })).status).toBe(200);
|
||||
expect((await remove(deleted)).status).toBe(200);
|
||||
expect(existsSync(join(imageDir, 'paste-2.png'))).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -108,6 +108,25 @@ describe('which dead sessions may be rebuilt', () => {
|
||||
expect(plan.skipped).toEqual([{ sessionId: 'gone', reason: 'no-persisted-record' }]);
|
||||
});
|
||||
|
||||
it('never revives a session whose agent the user ended with a clean exit (#446)', () => {
|
||||
// The clean-exit sweep would have closed it, had the power not gone first.
|
||||
const persisted = { exited: persistedSession({ id: 'exited', paneExit: { status: 0, at: NOW - HOUR } }) };
|
||||
const plan = planRebootRestore(['exited'], persisted, () => true);
|
||||
expect(plan.restore).toEqual([]);
|
||||
expect(plan.skipped).toEqual([{ sessionId: 'exited', reason: 'agent-exited' }]);
|
||||
});
|
||||
|
||||
it('still offers a crashed agent, and one whose exit status tmux never reported', () => {
|
||||
// Same explicit-0 rule as the sweep: an absent status is unknown, not clean,
|
||||
// and a crash keeps its row on the board, so it stays eligible here too.
|
||||
const persisted = {
|
||||
crashed: persistedSession({ id: 'crashed', paneExit: { status: 137, at: NOW - HOUR } }),
|
||||
killed: persistedSession({ id: 'killed', paneExit: { at: NOW - HOUR } }),
|
||||
};
|
||||
const plan = planRebootRestore(['crashed', 'killed'], persisted, () => true);
|
||||
expect(plan.restore.map((s) => s.sessionId)).toEqual(['crashed', 'killed']);
|
||||
});
|
||||
|
||||
it('never revives a pane whose PTY-exit breaker had tripped', () => {
|
||||
const persisted = { crashy: persistedSession({ id: 'crashy', respawnBlocked: true }) };
|
||||
expect(planRebootRestore(['crashy'], persisted, () => true).skipped[0].reason).toBe('respawn-blocked');
|
||||
|
||||
@@ -163,9 +163,12 @@ describe('the rich row pill of an exited session', () => {
|
||||
// which reads `status` and knows nothing about the exit, so without an override
|
||||
// the muted dot sat beside a pill saying "idle".
|
||||
const appJs = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8');
|
||||
const fn = (re: RegExp, name: string) => {
|
||||
const m = appJs.match(re)?.[0];
|
||||
if (!m) throw new Error(`${name} not found in app.js`);
|
||||
// The exit rule itself is shared with both home screens and lives in
|
||||
// mobile-overview.js, so it is lifted from there rather than stubbed.
|
||||
const overviewJs = readFileSync(resolve(import.meta.dirname, '../src/web/public/mobile-overview.js'), 'utf8');
|
||||
const fn = (re: RegExp, name: string, source = appJs) => {
|
||||
const m = source.match(re)?.[0];
|
||||
if (!m) throw new Error(`${name} not found`);
|
||||
return m;
|
||||
};
|
||||
type Row = { state: string; exited: boolean; pill: string; since: { key: string; at: number } | null };
|
||||
@@ -174,6 +177,7 @@ describe('the rich row pill of an exited session', () => {
|
||||
return {
|
||||
${fn(/ {2}_sidebarRichPillLabel\(state\) \{[\s\S]*?\n {2}\}/, '_sidebarRichPillLabel')},
|
||||
${fn(/ {2}_sidebarRichRow\(id, session\) \{[\s\S]*?\n {2}\}/, '_sidebarRichRow')},
|
||||
${fn(/ {2}_mobileOverviewExit\(state, session\) \{[\s\S]*?\n {2}\}/, '_mobileOverviewExit', overviewJs)},
|
||||
_mobileOverviewState(session, hooks) {
|
||||
if (hooks && hooks.has('permission_prompt')) return 'needs';
|
||||
if (hooks && hooks.has('idle_prompt')) return 'waiting';
|
||||
|
||||
@@ -1126,6 +1126,37 @@ describe('TmuxManager pane-exit bookkeeping', () => {
|
||||
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW));
|
||||
manager.clearPaneExit('codeman-aaaa');
|
||||
expect(manager.getPaneExit('codeman-aaaa')).toBeUndefined();
|
||||
expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(0);
|
||||
});
|
||||
|
||||
// The clean-exit sweep closes a session only once two reads agreed on its
|
||||
// exit (Ark0N/Codeman#446), so the count must rise only on an exact repeat.
|
||||
it('counts the reads that agreed on one exit', () => {
|
||||
const manager = new TmuxManager();
|
||||
expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(0);
|
||||
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW));
|
||||
expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(1);
|
||||
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|0|'), NOW + 2000));
|
||||
expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(2);
|
||||
});
|
||||
|
||||
it('starts the count again when the status or the pane pid changes', () => {
|
||||
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));
|
||||
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|100|1|1|'), NOW + 4000));
|
||||
expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(1);
|
||||
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|101|1|1|'), NOW + 6000));
|
||||
expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(1);
|
||||
});
|
||||
|
||||
it('drops the count once a read sees the pane alive again', () => {
|
||||
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.getPaneExitReadCount('codeman-aaaa')).toBe(0);
|
||||
manager.applyPaneExits(derivePaneExits(parsePaneRows('codeman-aaaa|101|1|0|'), NOW + 4000));
|
||||
expect(manager.getPaneExitReadCount('codeman-aaaa')).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1214,6 +1245,20 @@ describe('the pane-exit watcher tick', () => {
|
||||
expect(manager.getPaneExit('codeman-s1')).toEqual({ status: 137, at: NOW });
|
||||
});
|
||||
|
||||
it('does not count an empty read as confirming an exit', async () => {
|
||||
// The clean-exit sweep closes on the second agreeing read. A read tmux did
|
||||
// not answer agrees with nothing, so it must not supply that second read.
|
||||
const manager = withLocalSession();
|
||||
manager.rows = parsePaneRows('codeman-s1|100|1|0|');
|
||||
await manager.refreshPaneExits(NOW);
|
||||
manager.rows = [];
|
||||
await manager.refreshPaneExits(NOW + 2000);
|
||||
expect(manager.getPaneExitReadCount('codeman-s1')).toBe(1);
|
||||
manager.rows = parsePaneRows('codeman-s1|100|1|0|');
|
||||
await manager.refreshPaneExits(NOW + 4000);
|
||||
expect(manager.getPaneExitReadCount('codeman-s1')).toBe(2);
|
||||
});
|
||||
|
||||
it('discards a read that started before the pane was cleared', async () => {
|
||||
// The guard that stops an in-flight read from republishing a death over
|
||||
// the pane that has just replaced it.
|
||||
|
||||
Reference in New Issue
Block a user