mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 14:09:42 +02:00
fix(tui): refuse to attach to a dead pane, and stop naming sessions after CLI noise
Two more from the same beta round, both reported as "basic things are broken".
Attaching to a DEAD pane trapped the user. Codeman sets `remain-on-exit on`, so
a session whose agent has exited does not disappear: the row looks ordinary,
the server still reports it idle, and Enter handed the terminal to a pane that
reads no input. With the detach chord also wrong at the time, that was a hard
freeze with no way out. Enter now probes `#{pane_dead}` first and refuses with
an Error card naming the session and what to do instead. The probe fails OPEN,
so it can never block an attach to a live pane. ⚠️ It also has to paint: the
keypress that reaches attachToSession() has already painted by the time an
awaited probe resolves, so message() alone left the refusal invisible and Enter
looked inert, which is the bug it was added to fix.
A session started from the TUI came out unnamed, because startSession() sent no
sessionName and rowLabel() then fell back to the transcript's first line. A
brand-new session has no prompt to be named after, so the list showed a
perfectly healthy session called "Login interrupted" — the CLI's startup
output, reading like a failure report. Sessions the TUI starts are now named
`w<n>-<case>` like the web UI's, and rowLabel() prefers the case directory over
a scraped prompt for any row with a mux name, since a LIVE pane is identified
by where it runs while a history row genuinely is its prompt.
This commit is contained in:
+48
-1
@@ -324,6 +324,24 @@ export function buildAttachBanner(options: {
|
||||
/** Long enough for a session name, short enough to survive a narrow terminal. */
|
||||
const ATTACH_BANNER_LABEL_MAX = 28;
|
||||
|
||||
/**
|
||||
* The name a newly started session gets: `w<n>-<case>`, the same convention the
|
||||
* web UI uses, with `n` one past the highest already in use.
|
||||
*
|
||||
* A session created with no name at all is not merely unlabelled: rowLabel()
|
||||
* falls back to the transcript's first line, and a session that has not been
|
||||
* prompted yet gets named after whatever its CLI printed while starting up.
|
||||
*/
|
||||
export function nextSessionName(caseName: string, existing: readonly string[]): string {
|
||||
let highest = 0;
|
||||
for (const name of existing) {
|
||||
const match = /^w(\d+)-/.exec((name ?? '').trim());
|
||||
const index = match ? Number.parseInt(match[1] ?? '', 10) : Number.NaN;
|
||||
if (Number.isSafeInteger(index) && index > highest) highest = index;
|
||||
}
|
||||
return `w${highest + 1}-${caseName}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* What pressing Enter on a RECENT row does, decided from the row alone.
|
||||
*
|
||||
@@ -1887,6 +1905,23 @@ class TuiApp {
|
||||
return;
|
||||
}
|
||||
|
||||
// A dead pane still LISTS, because Codeman keeps `remain-on-exit on`: the
|
||||
// row looks ordinary and the server still calls it idle. Attaching to one
|
||||
// hands the terminal to a pane that reads nothing, which a beta tester
|
||||
// experienced as the TUI freezing with no way out.
|
||||
if (await this.client.isPaneDead(muxName)) {
|
||||
this.message(
|
||||
'err',
|
||||
`${rowLabel(row.session)} has exited — its pane is dead, so there is nothing there to type into. ` +
|
||||
'Close the row with x, or start a fresh session with n.'
|
||||
);
|
||||
// ⚠️ message() only sets state. The keypress that got us here painted
|
||||
// BEFORE this await resolved, so without a paint of our own the refusal
|
||||
// is invisible and Enter looks like it did nothing at all.
|
||||
this.paint();
|
||||
return;
|
||||
}
|
||||
|
||||
// tmux is about to own this terminal. The dashboard is not on screen, and
|
||||
// the pane the preview would keep re-reading is the one the user is now
|
||||
// looking at directly, so the poll stops for the whole handoff (an attach
|
||||
@@ -2078,7 +2113,19 @@ class TuiApp {
|
||||
|
||||
private async startSession(caseName: string, mode: TuiRunMode): Promise<void> {
|
||||
try {
|
||||
const result = await this.client.quickStart({ caseName, mode });
|
||||
const result = await this.client.quickStart({
|
||||
caseName,
|
||||
mode,
|
||||
// Named here rather than left to the server: an unnamed session falls
|
||||
// back to whatever rowLabel() can find, and before the user has typed
|
||||
// anything that was the CLI's first line of output. `w<n>-<case>` is
|
||||
// the web UI's own convention, so a session started from either surface
|
||||
// reads the same in both.
|
||||
sessionName: nextSessionName(
|
||||
caseName,
|
||||
this.model.sessions().map((session) => session.name ?? '')
|
||||
),
|
||||
});
|
||||
// The row appears with the next resync; remember which one to select.
|
||||
this.pendingSelectId = result.sessionId;
|
||||
this.message('info', `started ${mode} in ${caseName}`);
|
||||
|
||||
@@ -897,6 +897,36 @@ export class TuiClient {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Is this session's active pane DEAD — the process it ran has exited and tmux
|
||||
* is holding the corpse on screen?
|
||||
*
|
||||
* Codeman sets `remain-on-exit on` for every pane it owns, so a session whose
|
||||
* agent exited does not disappear: it stays listed, the server still reports
|
||||
* it `idle`, and attaching hands the terminal to a pane that reads no input.
|
||||
* A beta tester hit exactly that and could not type or get out.
|
||||
*
|
||||
* Fails OPEN (`false`): a probe that cannot run must never block an attach to
|
||||
* a pane that is perfectly alive.
|
||||
*/
|
||||
async isPaneDead(muxName: string): Promise<boolean> {
|
||||
if (!MUX_NAME_PATTERN.test(muxName)) return false;
|
||||
try {
|
||||
const { stdout } = await this.exec('tmux', [
|
||||
'-L',
|
||||
this.socket,
|
||||
'display-message',
|
||||
'-p',
|
||||
'-t',
|
||||
muxName,
|
||||
'#{pane_dead}',
|
||||
]);
|
||||
return stdout.trim() === '1';
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Snapshot the session-level options an attach is about to overwrite. */
|
||||
async readSessionOptions(muxName: string, keys: readonly string[]): Promise<TuiSessionOptions | null> {
|
||||
if (!MUX_NAME_PATTERN.test(muxName)) return null;
|
||||
|
||||
@@ -254,9 +254,17 @@ export function formatPlanUsage(usage: StatusTelemetry | null | undefined, separ
|
||||
*/
|
||||
export function rowLabel(session: TuiSessionRow): string {
|
||||
if (session.name) return session.name;
|
||||
const base = (session.workingDir ?? '').split('/').filter(Boolean).pop();
|
||||
// ⚠️ A LIVE pane (it has a mux name) is identified by WHERE it runs, never by
|
||||
// a line scraped out of its transcript. A session created before the user has
|
||||
// typed anything has no prompt to be named after, so the fallback took
|
||||
// whatever the CLI happened to print first: a beta tester's new session
|
||||
// appeared in the list called "Login interrupted", which reads like a failure
|
||||
// report and was in fact a healthy session. A history row is the opposite
|
||||
// case, where the prompt IS the identity, so it keeps the old order.
|
||||
if (session.muxName && base) return base;
|
||||
const prompt = (session.firstPrompt ?? '').trim();
|
||||
if (prompt && prompt !== '(no content)') return prompt;
|
||||
const base = (session.workingDir ?? '').split('/').filter(Boolean).pop();
|
||||
return base || session.sessionId.slice(0, 8);
|
||||
}
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
confirmAccepts,
|
||||
confirmKillStep,
|
||||
detachChord,
|
||||
nextSessionName,
|
||||
footerKeysFor,
|
||||
formatPrefixKey,
|
||||
helpKeysFor,
|
||||
@@ -457,6 +458,25 @@ describe('buildListLines', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('naming a session the TUI starts', () => {
|
||||
it("follows the web UI's w<n>-<case> convention", () => {
|
||||
expect(nextSessionName('mirofish', [])).toBe('w1-mirofish');
|
||||
expect(nextSessionName('mirofish', ['w1-codeman', 'w2-codeman'])).toBe('w3-mirofish');
|
||||
});
|
||||
|
||||
it('counts past names that are not w<n>- at all', () => {
|
||||
// A session named by hand, or by another surface, must not reset the run.
|
||||
expect(nextSessionName('demo', ['tui-demo-agent', 'w4-codeman', ''])).toBe('w5-demo');
|
||||
});
|
||||
|
||||
it('never returns an empty name, which is what caused the bad label', () => {
|
||||
// An unnamed session falls through rowLabel() to the transcript's first
|
||||
// line, which put "Login interrupted" in the list as a session name.
|
||||
expect(nextSessionName('c', [])).not.toBe('');
|
||||
expect(nextSessionName('c', ['w9007199254740991-x'])).toMatch(/^w\d+-c$/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the way out of an attach', () => {
|
||||
it('spells the prefix the way a human reads it, and never assumes C-b', () => {
|
||||
expect(formatPrefixKey('C-b')).toBe('Ctrl+B');
|
||||
|
||||
@@ -410,6 +410,39 @@ describe('formatting helpers', () => {
|
||||
);
|
||||
expect(rowLabel({ sessionId: 'abcdef1234', sources: [] })).toBe('abcdef12');
|
||||
});
|
||||
|
||||
it('names a LIVE pane after its case, never after scraped output', () => {
|
||||
// Regression: a session started from the TUI before the user typed anything
|
||||
// had no name and no prompt, so the fallback took the CLI's first line of
|
||||
// output. A healthy new session showed up in the list called
|
||||
// "Login interrupted", which reads like a failure.
|
||||
expect(
|
||||
rowLabel({
|
||||
sessionId: 'abcdef12',
|
||||
firstPrompt: 'Login interrupted',
|
||||
workingDir: '/home/u/codeman-cases/mirofish',
|
||||
muxName: 'codeman-abcdef12',
|
||||
sources: [],
|
||||
})
|
||||
).toBe('mirofish');
|
||||
});
|
||||
|
||||
it('still names a HISTORY row by its prompt, where the prompt IS the identity', () => {
|
||||
expect(
|
||||
rowLabel({
|
||||
sessionId: 'abcdef12',
|
||||
firstPrompt: 'do the thing',
|
||||
workingDir: '/home/u/codeman-cases/mirofish',
|
||||
sources: [],
|
||||
})
|
||||
).toBe('do the thing');
|
||||
});
|
||||
|
||||
it('prefers a real name over both, on a live row and a history row alike', () => {
|
||||
const base = { sessionId: 'abcdef12', firstPrompt: 'Login interrupted', workingDir: '/a/b/case', sources: [] };
|
||||
expect(rowLabel({ ...base, name: 'w2-case' })).toBe('w2-case');
|
||||
expect(rowLabel({ ...base, name: 'w2-case', muxName: 'codeman-abcdef12' })).toBe('w2-case');
|
||||
});
|
||||
});
|
||||
|
||||
describe('the approval card', () => {
|
||||
|
||||
Reference in New Issue
Block a user