mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(tui): keep the way out on the bar, and make Alt+1..9 actually switch
Four faults, all reported at once, and three of them were mine from the last two commits. THE HINT VANISHED. Two independent causes. First, a leaked F1 binding: an attach whose TUI was killed leaves `F1 -> detach-client` in tmux's root table, and the claim treated "already bound" as someone else's key, so every later attach fell back to advertising the tmux chord — the bar stopped saying F1 while F1 still worked. A key already bound to `detach-client` now counts as ours. Second, width: tmux truncates a status line that overflows and drops the RIGHT-aligned segment, which is the hint. The strip now gets a budget measured from the terminal's width minus the hint, and it drops tabs from the far end until it fits. ⚠️ Measured on VISIBLE columns, not format bytes: `#[reverse]` costs zero columns, and counting it made a strip that "fitted" still truncate the hint at 80, 100, 120 and 176 columns on a real terminal. ALT+N DID NOT SWITCH. On the dashboard, a bare digit meant jump AND ATTACH, and a terminal sends Alt+N as ESC then N: when those land in separate reads — routine over SSH — the chord decodes as Escape plus a bare digit, so "switch to tab 2" threw the user into tab 2's pane. A digit now SELECTS, matching what Alt+N means in the web UI; Enter is how you go in. Inside a pane the keys never reached the TUI at all, since tmux owns the terminal, so the attach now binds Alt+1..9 in tmux's root table to `switch-client` — the strip is usable rather than decorative. ⚠️ The bar is applied to every session the strip can reach, each highlighting its own tab: with it on the attached session only, switching landed the user in a pane with no strip and no way out on screen. ⚠️ The leaked-state sweep was missing `status-position`, so it removed the marker and left the position behind — and with no marker the leftover no longer matched, making it permanently unsweepable. Found by diffing every session's options after a detach.
This commit is contained in:
+108
-29
@@ -57,6 +57,7 @@ import { formatAwayDigest } from './tui-digest.js';
|
||||
import {
|
||||
ATTACH_BANNER_MARKER,
|
||||
TuiClient,
|
||||
type TuiSessionOptions,
|
||||
type TuiApprovalAnswer,
|
||||
type TuiEventStream,
|
||||
type TuiLiveSessionMetrics,
|
||||
@@ -348,6 +349,8 @@ export function buildAttachBanner(options: {
|
||||
oneKey?: string;
|
||||
/** The other sessions, drawn as a strip so they stay visible from inside a pane. */
|
||||
tabs?: readonly TuiAttachTab[];
|
||||
/** The attaching terminal's width, so the strip can be kept clear of the hint. */
|
||||
cols?: number;
|
||||
}): Record<string, string> {
|
||||
const chord = escapeTmuxFormat(detachChord(options.prefix, options.detachKey));
|
||||
// Named on the bar because it is what people actually type: keeping Ctrl held
|
||||
@@ -359,9 +362,17 @@ export function buildAttachBanner(options: {
|
||||
// owns the whole line, which is what removes tmux's window list (`0:bash*`)
|
||||
// from the middle of it. The window-status options that would otherwise hide
|
||||
// it are WINDOW options, so `set-option -t <session>` cannot even reach them.
|
||||
// The hint is measured, not estimated, and the strip is given whatever is
|
||||
// left. tmux truncates a status line that overflows, and what it drops is the
|
||||
// RIGHT-aligned segment — which is the hint, the one thing on the bar a user
|
||||
// cannot do without. Every width tested lost it before this budget existed.
|
||||
const hint = options.oneKey
|
||||
? ` ${options.oneKey} ${ATTACH_BANNER_MARKER} `
|
||||
: ` ${chord}${alias} ${ATTACH_BANNER_MARKER} `;
|
||||
const budget = Math.max(0, (options.cols ?? Number.POSITIVE_INFINITY) - hint.length - 1);
|
||||
// The strip names the session it highlights, so the standalone label is only
|
||||
// a fallback for when there is no strip to draw (degraded mode has no list).
|
||||
const strip = buildAttachTabs(options.tabs ?? []);
|
||||
const strip = buildAttachTabs(options.tabs ?? [], 6, budget);
|
||||
const left = strip || (label ? ` ${label} ` : '');
|
||||
return {
|
||||
status: 'on',
|
||||
@@ -387,6 +398,8 @@ export interface TuiAttachTab {
|
||||
index: number;
|
||||
label: string;
|
||||
active: boolean;
|
||||
/** tmux session to switch to when its number is pressed inside a pane. */
|
||||
muxName?: string;
|
||||
}
|
||||
|
||||
/** Per-tab label cap. Eight of these plus separators still fit an 80-column terminal. */
|
||||
@@ -403,7 +416,7 @@ const ATTACH_TAB_LABEL_MAX = 12;
|
||||
* lost. Ellipses mark what is not shown, so a truncated strip reads as
|
||||
* truncated rather than as the whole list.
|
||||
*/
|
||||
export function buildAttachTabs(tabs: readonly TuiAttachTab[], maxTabs = 6): string {
|
||||
export function buildAttachTabs(tabs: readonly TuiAttachTab[], maxTabs = 6, budget = Number.POSITIVE_INFINITY): string {
|
||||
if (tabs.length === 0) return '';
|
||||
const active = Math.max(
|
||||
0,
|
||||
@@ -412,16 +425,28 @@ export function buildAttachTabs(tabs: readonly TuiAttachTab[], maxTabs = 6): str
|
||||
let start = Math.max(0, Math.min(active - Math.floor(maxTabs / 2), tabs.length - maxTabs));
|
||||
if (start < 0) start = 0;
|
||||
const shown = tabs.slice(start, start + maxTabs);
|
||||
const parts = shown.map((tab) => {
|
||||
const label = truncateLabel(tab.label, ATTACH_TAB_LABEL_MAX);
|
||||
const shownLabels = shown.map((tab) => truncateLabel(tab.label, ATTACH_TAB_LABEL_MAX));
|
||||
const parts = shown.map((tab, i) => {
|
||||
const label = shownLabels[i];
|
||||
const text = escapeTmuxFormat(`${tab.index} ${label}`);
|
||||
// The active tab is inverted rather than bracketed: brackets cost two
|
||||
// columns per tab and read as punctuation next to the session names.
|
||||
return tab.active ? `#[reverse] ${text} #[noreverse]` : ` ${text} `;
|
||||
});
|
||||
// Drop tabs from the far end until the strip fits the space the hint leaves.
|
||||
// ⚠️ Measured on the VISIBLE text, not the format string: `#[reverse]` and
|
||||
// friends cost zero columns, and counting them made a strip that "fitted"
|
||||
// truncate the hint on a real terminal at every width tested.
|
||||
const visible = (index: number): number => shownLabels[index].length + String(shown[index].index).length + 3;
|
||||
let width = shown.reduce((total, _tab, index) => total + visible(index), 0);
|
||||
let last = shown.length;
|
||||
while (last > 1 && width + 2 > budget) {
|
||||
last -= 1;
|
||||
width -= visible(last);
|
||||
}
|
||||
const head = start > 0 ? '…' : '';
|
||||
const tail = start + maxTabs < tabs.length ? '…' : '';
|
||||
return `${head}${parts.join('')}${tail}`;
|
||||
const tail = start + last < tabs.length ? '…' : '';
|
||||
return `${head}${parts.slice(0, last).join('')}${tail}`;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -504,7 +529,8 @@ export async function beginAttachHandoff(
|
||||
client: TuiClient,
|
||||
muxName: string,
|
||||
label: string,
|
||||
tabs: readonly TuiAttachTab[] = []
|
||||
tabs: readonly TuiAttachTab[] = [],
|
||||
cols?: number
|
||||
): Promise<TuiAttachHandoff> {
|
||||
const prefix = (await client.readPrefixKey(muxName)) ?? undefined;
|
||||
// Read, not assumed: see detachChord() for the `d` vs `D` mix-up this closes.
|
||||
@@ -521,30 +547,71 @@ export async function beginAttachHandoff(
|
||||
// binding the user put in their own config.
|
||||
const alias = heldCtrlAlias(detachKey ?? DEFAULT_DETACH_KEY);
|
||||
const claimed = alias && (await client.readPrefixBinding(alias)) === null ? await client.bindDetachKey(alias) : false;
|
||||
// The one-key way out, in the prefix-less table. Same rule: only if free.
|
||||
// The one-key way out, in the prefix-less table.
|
||||
//
|
||||
// ⚠️ "Already bound to detach-client" counts as CLAIMED, not as taken. An
|
||||
// attach whose TUI was killed leaks the binding, and treating that leak as
|
||||
// someone else's binding made every later attach fall back to advertising
|
||||
// the tmux chord — so the bar stopped saying F1 while F1 still worked, which
|
||||
// is the worst of both. Anything else there is genuinely the user's and is
|
||||
// left alone.
|
||||
const existing = await client.readPrefixBinding(ONE_KEY_DETACH, 'root');
|
||||
const oneKey =
|
||||
(await client.readPrefixBinding(ONE_KEY_DETACH, 'root')) === null
|
||||
? await client.bindDetachKey(ONE_KEY_DETACH, 'root')
|
||||
: false;
|
||||
const banner = buildAttachBanner({
|
||||
...(prefix ? { prefix } : {}),
|
||||
...(detachKey ? { detachKey } : {}),
|
||||
...(claimed && alias ? { heldAlias: alias } : {}),
|
||||
...(oneKey ? { oneKey: ONE_KEY_DETACH } : {}),
|
||||
label,
|
||||
tabs,
|
||||
});
|
||||
const options = await client.readSessionOptions(muxName, Object.keys(banner));
|
||||
await client.applySessionOptions(muxName, banner);
|
||||
existing === 'detach-client'
|
||||
? true
|
||||
: existing === null
|
||||
? await client.bindDetachKey(ONE_KEY_DETACH, 'root')
|
||||
: false;
|
||||
// Alt+1..9 switch sessions from inside the pane, so the strip on the bar is
|
||||
// usable rather than decorative. Only the first nine, only sessions that
|
||||
// really have a pane, and only keys tmux reports as free — the same rule the
|
||||
// way-out key follows, so a binding of the user's own is never shadowed.
|
||||
const switchKeys: string[] = [];
|
||||
for (const tab of tabs.slice(0, 9)) {
|
||||
if (!tab.muxName) continue;
|
||||
const key = `M-${tab.index}`;
|
||||
const bound = await client.readPrefixBinding(key, 'root');
|
||||
if (bound !== null && !bound.startsWith('switch-client')) continue;
|
||||
if (await client.bindSwitchKey(key, tab.muxName)) switchKeys.push(key);
|
||||
}
|
||||
const bannerFor = (activeMux: string, ownLabel: string): Record<string, string> =>
|
||||
buildAttachBanner({
|
||||
...(prefix ? { prefix } : {}),
|
||||
...(detachKey ? { detachKey } : {}),
|
||||
...(claimed && alias ? { heldAlias: alias } : {}),
|
||||
...(oneKey ? { oneKey: ONE_KEY_DETACH } : {}),
|
||||
...(cols ? { cols } : {}),
|
||||
label: ownLabel,
|
||||
tabs: tabs.map((tab) => ({ ...tab, active: tab.muxName === activeMux })),
|
||||
});
|
||||
|
||||
// ⚠️ The bar goes on EVERY session the strip can switch to, not just the one
|
||||
// being attached. `switch-client` moves this client to another session, and
|
||||
// that session draws its OWN status line: with the bar only on the first one,
|
||||
// pressing Alt+2 landed the user in a pane with no strip and, worse, no way
|
||||
// out on screen. Each copy highlights its own tab, so the strip tracks where
|
||||
// you actually are.
|
||||
const banner = bannerFor(muxName, label);
|
||||
const dressed: Array<{ muxName: string; options: TuiSessionOptions }> = [];
|
||||
const targets = new Map<string, string>([[muxName, label]]);
|
||||
for (const tab of tabs.slice(0, 9)) {
|
||||
if (tab.muxName && !targets.has(tab.muxName)) targets.set(tab.muxName, tab.label);
|
||||
}
|
||||
for (const [target, targetLabel] of targets) {
|
||||
const snapshot = await client.readSessionOptions(target, Object.keys(banner));
|
||||
if (snapshot) dressed.push({ muxName: target, options: snapshot });
|
||||
await client.applySessionOptions(target, bannerFor(target, targetLabel));
|
||||
}
|
||||
return {
|
||||
chord: oneKey ? ONE_KEY_DETACH : detachChord(prefix, detachKey),
|
||||
async restore(): Promise<void> {
|
||||
for (const key of switchKeys) await client.unbindSwitchKey(key);
|
||||
if (oneKey) await client.unbindDetachKey(ONE_KEY_DETACH, 'root');
|
||||
if (claimed && alias) await client.unbindDetachKey(alias);
|
||||
// Options first, then the size: dropping the status bar gives its row
|
||||
// back to the pane, and the resize is what re-pins the browser's
|
||||
// authority over the window.
|
||||
if (options) await client.restoreSessionOptions(muxName, options);
|
||||
for (const entry of dressed) await client.restoreSessionOptions(entry.muxName, entry.options);
|
||||
if (sizing) await client.restoreWindowSizing(muxName, sizing);
|
||||
},
|
||||
};
|
||||
@@ -644,11 +711,11 @@ export function footerKeysFor(mode: TuiUiMode, glyphs: TuiGlyphSet, context: Tui
|
||||
return ['j/k scroll', 'esc close'];
|
||||
case 'list': {
|
||||
if (!context.server) {
|
||||
return [`${glyphs.updown} select`, `${glyphs.enter} attach`, '1-9 jump', '? help', 'q quit'];
|
||||
return [`${glyphs.updown} select`, `${glyphs.enter} attach`, '1-9 switch', '? help', 'q quit'];
|
||||
}
|
||||
const keys = [`${glyphs.updown} select`, `${glyphs.enter} attach`];
|
||||
if (context.approval === 'menu') keys.push('y approve', 'n deny', '1-9 option');
|
||||
else keys.push('1-9 jump');
|
||||
else keys.push('1-9 switch');
|
||||
keys.push(context.approval === 'idle' ? 'p reply' : 'p prompt');
|
||||
if (context.approval !== 'menu') keys.push('n new');
|
||||
keys.push('x kill', '/ search', 'g digest', '? help', 'q quit');
|
||||
@@ -665,11 +732,10 @@ export function helpKeysFor(glyphs: TuiGlyphSet, context: TuiKeymapContext): Arr
|
||||
const keys: Array<[string, string]> = [
|
||||
[`${glyphs.updown} / j k`, 'select'],
|
||||
[glyphs.enter, 'attach — on a RECENT row, resume that conversation'],
|
||||
['1-9', 'jump and attach'],
|
||||
['1-9', 'switch to that session'],
|
||||
// The web UI's tab switching, as close as a terminal can carry it: Alt+N
|
||||
// matches exactly, while Alt+[ / Alt+] cannot be transmitted (ESC+[ IS the
|
||||
// CSI introducer) so the brackets do that job unmodified.
|
||||
['alt+1-9', 'switch to that session, without attaching'],
|
||||
['[ / ]', 'previous / next session'],
|
||||
['tab', 'next session'],
|
||||
// The one key that is not the TUI's: an attach hands the terminal to tmux,
|
||||
@@ -1533,7 +1599,13 @@ class TuiApp {
|
||||
for (const row of this.model.rows()) {
|
||||
if (row.group === 'recent') continue;
|
||||
index += 1;
|
||||
tabs.push({ index, label: rowLabel(row.session), active: row.session.sessionId === activeId });
|
||||
const muxName = (row.session.muxName ?? '').trim();
|
||||
tabs.push({
|
||||
index,
|
||||
label: rowLabel(row.session),
|
||||
active: row.session.sessionId === activeId,
|
||||
...(muxName ? { muxName } : {}),
|
||||
});
|
||||
}
|
||||
return tabs;
|
||||
}
|
||||
@@ -1629,7 +1701,13 @@ class TuiApp {
|
||||
}
|
||||
|
||||
if (value >= '1' && value <= '9') {
|
||||
if (this.model.cursorToIndex(Number.parseInt(value, 10))) void this.attachSelected();
|
||||
// SELECT, never attach. A digit used to jump AND hand the terminal to
|
||||
// that pane, which made Alt+N unusable: a terminal sends Alt+N as ESC
|
||||
// then N, and when those land in separate reads — routine over SSH — the
|
||||
// chord decodes as Escape plus a bare digit, so "switch to tab 2" threw
|
||||
// the user into tab 2's pane instead. Selecting matches what Alt+N means
|
||||
// in the web UI, and Enter is how you go in.
|
||||
this.model.cursorToIndex(Number.parseInt(value, 10));
|
||||
return;
|
||||
}
|
||||
switch (value) {
|
||||
@@ -2172,7 +2250,8 @@ class TuiApp {
|
||||
this.client,
|
||||
muxName,
|
||||
rowLabel(row.session),
|
||||
this.attachTabs(row.session.sessionId)
|
||||
this.attachTabs(row.session.sessionId),
|
||||
this.currentLayout().cols
|
||||
);
|
||||
this.detachChordLabel = handoff.chord;
|
||||
this.screen.leave();
|
||||
|
||||
@@ -951,6 +951,36 @@ export class TuiClient {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Bind a prefix-less key to switch this client to another session, so the tab
|
||||
* strip on the attach bar is not just a picture: Alt+1..9 moves between
|
||||
* sessions from INSIDE a pane, the way it does in the web UI.
|
||||
*
|
||||
* `switch-client` rather than detach-then-attach: it keeps the terminal, so
|
||||
* the move is instant and the TUI stays blocked in its `spawnSync` exactly as
|
||||
* before, still holding the restore it owes.
|
||||
*/
|
||||
async bindSwitchKey(key: string, target: string): Promise<boolean> {
|
||||
if (!MUX_NAME_PATTERN.test(target)) return false;
|
||||
try {
|
||||
await this.exec('tmux', ['-L', this.socket, 'bind-key', '-T', 'root', key, 'switch-client', '-t', target]);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Give back a switch key, but only while it still points at a session. */
|
||||
async unbindSwitchKey(key: string): Promise<void> {
|
||||
const bound = await this.readPrefixBinding(key, 'root');
|
||||
if (!bound || !bound.startsWith('switch-client')) return;
|
||||
try {
|
||||
await this.exec('tmux', ['-L', this.socket, 'unbind-key', '-T', 'root', key]);
|
||||
} catch {
|
||||
/* a stray switch binding is harmless next to failing an attach */
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Give a key back, but ONLY while it still means `detach-client`. Anything
|
||||
* else there is the user's, arrived after we bound ours, and must not be
|
||||
@@ -1103,6 +1133,12 @@ export class TuiClient {
|
||||
// unsetting one index leaves an empty array, which renders as a blank bar.
|
||||
await this.setOption(['-u', '-t', name, 'status-format']);
|
||||
await this.setOption(['-u', '-t', name, 'status-style']);
|
||||
// ⚠️ `status-position` MUST be swept with the rest. It was missing here,
|
||||
// so a sweep cleaned the marker and left the position behind — and with
|
||||
// the marker gone the leftover no longer matched, which made it
|
||||
// permanently unsweepable. Every option the banner writes has to be
|
||||
// undone by the same pass that recognises it.
|
||||
await this.setOption(['-u', '-t', name, 'status-position']);
|
||||
await this.setOption(['-t', name, 'status', 'off']);
|
||||
cleared += 1;
|
||||
}
|
||||
|
||||
@@ -161,7 +161,7 @@ describe('footerKeysFor', () => {
|
||||
it('advertises only the verbs this build implements', () => {
|
||||
const keys = footerKeysFor('list', GLYPHS, { server: true }).join(' ');
|
||||
expect(keys).toContain('attach');
|
||||
expect(keys).toContain('1-9 jump');
|
||||
expect(keys).toContain('1-9 switch');
|
||||
expect(keys).toContain('n new');
|
||||
expect(keys).toContain('p prompt');
|
||||
expect(keys).toContain('/ search');
|
||||
@@ -179,7 +179,7 @@ describe('footerKeysFor', () => {
|
||||
expect(keys).toContain('1-9 option');
|
||||
// `n` cannot mean two things at once, and denying is what it does here.
|
||||
expect(keys).not.toContain('n new');
|
||||
expect(keys).not.toContain('1-9 jump');
|
||||
expect(keys).not.toContain('1-9 switch');
|
||||
});
|
||||
|
||||
it('sends an idle prompt to the composer instead of offering approve/deny', () => {
|
||||
@@ -673,6 +673,38 @@ describe('the way out of an attach', () => {
|
||||
expect(crowded['status-format[0]']).toContain('#[bold]F1#[nobold] back to the codeman dashboard');
|
||||
});
|
||||
|
||||
it('fits the strip to the terminal, measuring VISIBLE columns not format bytes', () => {
|
||||
// The test above only checks the hint is in the format STRING, which it
|
||||
// always was. tmux truncates what it cannot fit and drops the right-aligned
|
||||
// segment, so on a real terminal the hint vanished at every width tested
|
||||
// while that assertion stayed green.
|
||||
const hint = ' F1 back to the codeman dashboard ';
|
||||
for (const cols of [80, 100, 120, 176]) {
|
||||
const bar = buildAttachBanner({
|
||||
oneKey: 'F1',
|
||||
cols,
|
||||
tabs: Array.from({ length: 12 }, (_, i) => ({
|
||||
index: i + 1,
|
||||
label: `w${i + 1}-session-name`,
|
||||
active: i === 1,
|
||||
})),
|
||||
})['status-format[0]'];
|
||||
const visible = bar.replace(/#\[[^\]]*\]/g, '');
|
||||
expect({ cols, fits: visible.length <= cols }).toEqual({ cols, fits: true });
|
||||
expect(visible.length).toBeGreaterThanOrEqual(hint.length);
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps at least one tab even when the hint eats almost the whole bar', () => {
|
||||
const bar = buildAttachBanner({
|
||||
oneKey: 'F1',
|
||||
cols: 40,
|
||||
tabs: [{ index: 1, label: 'w1-case', active: true }],
|
||||
})['status-format[0]'];
|
||||
expect(bar).toContain('1 w1-case');
|
||||
expect(bar).toContain('back to the codeman dashboard');
|
||||
});
|
||||
|
||||
it("tells the help overlay how to get back, in the socket's own prefix", () => {
|
||||
const keys = helpKeysFor(GLYPHS, { server: true, detach: 'Ctrl+A then d' });
|
||||
const detach = keys.find(([key]) => key === 'Ctrl+A then d');
|
||||
|
||||
@@ -659,6 +659,11 @@ describe('TuiClient.clearLeakedAttachBanners', () => {
|
||||
// leaves an EMPTY array, which renders as a blank bar.
|
||||
expect(sets.some((args) => args.includes('-u') && args.includes('status-format'))).toBe(true);
|
||||
expect(sets.some((args) => args.includes('-u') && args.includes('status-style'))).toBe(true);
|
||||
// ⚠️ Every option the banner writes must be undone by the pass that
|
||||
// recognises it. `status-position` was missing, so a sweep removed the
|
||||
// marker and left the position behind — and with no marker the leftover
|
||||
// stopped matching, making it permanently unsweepable.
|
||||
expect(sets.some((args) => args.includes('-u') && args.includes('status-position'))).toBe(true);
|
||||
expect(sets.some((args) => args.join(' ').endsWith('status off'))).toBe(true);
|
||||
expect(sets.every((args) => !args.includes('status-format[0]'))).toBe(true);
|
||||
});
|
||||
|
||||
@@ -158,7 +158,7 @@ describe('renderFrame structure', () => {
|
||||
' │',
|
||||
' │',
|
||||
' │',
|
||||
' ↑↓ select · ↵ attach · 1-9 jump · y/n answer · p prompt · n new · x kill · / search · g digest · ?',
|
||||
' ↑↓ select · ↵ attach · 1-9 switch · y/n answer · p prompt · n new · x kill · / search · g digest ·',
|
||||
]);
|
||||
});
|
||||
|
||||
@@ -183,7 +183,7 @@ describe('renderFrame structure', () => {
|
||||
'',
|
||||
'',
|
||||
'',
|
||||
' ↑↓ select · ↵ attach · 1-9 jump · y/n answe',
|
||||
' ↑↓ select · ↵ attach · 1-9 switch · y/n ans',
|
||||
]);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user