diff --git a/src/tui/tui-app.ts b/src/tui/tui-app.ts index 1222199a..299852ec 100644 --- a/src/tui/tui-app.ts +++ b/src/tui/tui-app.ts @@ -508,33 +508,30 @@ export function isSelfSession(sessionId: string, env: { CODEMAN_SESSION_ID?: str // Kill confirmation (pure) // ───────────────────────────────────────────────────────────────────────────── -export type TuiConfirmStep = - | { kind: 'typing'; typed: string } - | { kind: 'confirm' } - | { kind: 'reject' } - | { kind: 'cancel' } - | { kind: 'ignore' }; - -/** Does the typed text authorize the kill? The shown name, or the id prefix a mux name carries. */ -export function confirmAccepts(state: TuiConfirmState, typed = state.typed): boolean { - const value = typed.trim(); - if (value === '') return false; - return value === state.name || value === state.sessionId.slice(0, 8); -} +export type TuiConfirmStep = { kind: 'confirm' } | { kind: 'cancel' } | { kind: 'ignore' }; /** - * One keystroke of the typed confirmation. Enter on text that does not match is - * a `reject`, never a silent no-op: a confirmation that appears to do nothing - * reads as a broken key. + * One keystroke of the kill confirmation: `y` kills, anything else does not. + * + * ⚠️ It used to demand the session's NAME typed out in full. That is the right + * ceremony for deleting a production database and the wrong one for closing a + * pane you are looking at: the beta tester's verdict was "thats stupid, just + * make me type Y to confirm". Kill is already two deliberate keystrokes (`x` + * then `y`) on a row the user selected, and the session's work lives in its + * transcript, which a kill does not touch. + * + * Everything that is NOT `y` cancels rather than being ignored, so a stray key + * closes the dialog instead of leaving a live kill prompt waiting for whatever + * the user types next. */ -export function confirmKillStep(state: TuiConfirmState, event: TuiInputEvent): TuiConfirmStep { +export function confirmKillStep(_state: TuiConfirmState, event: TuiInputEvent): TuiConfirmStep { switch (event.type) { case 'char': - return { kind: 'typing', typed: state.typed + event.value }; - case 'backspace': - return { kind: 'typing', typed: [...state.typed].slice(0, -1).join('') }; + return event.value === 'y' || event.value === 'Y' ? { kind: 'confirm' } : { kind: 'cancel' }; case 'enter': - return confirmAccepts(state) ? { kind: 'confirm' } : { kind: 'reject' }; + // Enter alone is NOT a confirmation: it is the key most likely to be + // pressed by reflex, and this is the one dialog that destroys something. + return { kind: 'cancel' }; case 'escape': return { kind: 'cancel' }; case 'ctrl': @@ -576,7 +573,7 @@ export function footerKeysFor(mode: TuiUiMode, glyphs: TuiGlyphSet, context: Tui case 'help': return ['esc close']; case 'confirm-kill': - return ['type the name', `${glyphs.enter} confirm`, 'esc cancel']; + return ['y kill', 'any other key cancels']; case 'message': return context.resumeOffer ? ['r resume', 'esc dismiss'] : ['esc dismiss']; case 'new-session': @@ -629,7 +626,7 @@ export function helpKeysFor(glyphs: TuiGlyphSet, context: TuiKeymapContext): Arr ['/', 'search sessions, events and files'], ['g', 'away digest'], ['n', 'new session'], - ['x', 'kill (typed confirmation)'] + ['x', 'kill (y to confirm)'] ); } keys.push(['?', 'this help'], ['esc', 'close an overlay'], ['q', 'quit']); @@ -1606,15 +1603,9 @@ class TuiApp { } const step = confirmKillStep(state, event); switch (step.kind) { - case 'typing': - this.model.setConfirmInput(step.typed); - return; case 'cancel': this.model.closeOverlay(); return; - case 'reject': - this.message('warn', `type "${state.name}" exactly, or esc to cancel`); - return; case 'confirm': void this.killSession(state.sessionId, state.name); return; @@ -2134,7 +2125,7 @@ class TuiApp { this.message('warn', 'that is the session this TUI is running in'); return; } - this.model.beginConfirmKill(row); + this.model.beginConfirmKill(row, rowLabel(row.session)); } private async killSession(sessionId: string, name: string): Promise { diff --git a/src/tui/tui-model.ts b/src/tui/tui-model.ts index 30f15fbf..d8db248a 100644 --- a/src/tui/tui-model.ts +++ b/src/tui/tui-model.ts @@ -442,22 +442,22 @@ export class TuiModelStore implements TuiRenderModel { * for every caller: a second copy here answered the same question differently * (it refused the id prefix a mux name carries) and nothing consulted it. */ - beginConfirmKill(row: TuiRow): void { + beginConfirmKill(row: TuiRow, label: string): void { this.confirm = { sessionId: row.session.sessionId, - name: row.session.name ?? row.session.sessionId.slice(0, 8), - typed: '', + // ⚠️ Passed in, not derived here. `row.session.name ?? id.slice(0,8)` + // used to compute it, and `??` falls back only on null/undefined: a + // session whose name is the EMPTY STRING (every session the server did + // not name) sailed through it and the dialog read "Kill ?". A destructive + // prompt that cannot say what it is about to destroy is worse than no + // prompt, and it is now one keystroke. The caller passes the same label + // the LIST shows, so the dialog names the row the user is looking at. + name: label, }; this.mode = 'confirm-kill'; this.touch(); } - setConfirmInput(typed: string): void { - if (!this.confirm) return; - this.confirm = { ...this.confirm, typed }; - this.touch(); - } - /** Drop whatever overlay owns the keyboard and go back to the list. */ closeOverlay(): void { this.confirm = null; diff --git a/src/tui/tui-render.ts b/src/tui/tui-render.ts index c54f27ca..c58c7138 100644 --- a/src/tui/tui-render.ts +++ b/src/tui/tui-render.ts @@ -614,7 +614,7 @@ const FOOTER_KEYS: Record string> = { 'q quit', ].join(` ${g.separator} `), help: (g) => `esc ${g.separator} ? close`, - 'confirm-kill': (g) => `type the name ${g.separator} ${g.enter} confirm ${g.separator} esc cancel`, + 'confirm-kill': (g) => `y kill ${g.separator} any other key cancels`, message: () => 'esc dismiss', prompt: (g) => `${g.enter} send ${g.separator} esc cancel`, search: (g) => `${g.updown} results ${g.separator} ${g.enter} open ${g.separator} esc close`, @@ -805,10 +805,9 @@ function overlayContent( } case 'confirm-kill': { if (!model.confirm) return null; - const { name, typed } = model.confirm; return { title: 'Kill session', - lines: [`Kill ${name}?`, '', 'Type the name to confirm:', ` ${typed}_`], + lines: [`Kill ${model.confirm.name}?`, '', 'press y to kill, any other key cancels'], }; } case 'message': diff --git a/src/tui/tui-types.ts b/src/tui/tui-types.ts index c30232be..50b51ebe 100644 --- a/src/tui/tui-types.ts +++ b/src/tui/tui-types.ts @@ -120,7 +120,6 @@ export interface TuiMessage { export interface TuiConfirmState { sessionId: string; name: string; - typed: string; } export interface TuiPickerItem { diff --git a/test/tui/tui-app.test.ts b/test/tui/tui-app.test.ts index c220d308..b1a35a05 100644 --- a/test/tui/tui-app.test.ts +++ b/test/tui/tui-app.test.ts @@ -15,7 +15,6 @@ import { applyMuxNames, buildAttachBanner, buildListLines, - confirmAccepts, confirmKillStep, detachChord, heldCtrlAlias, @@ -135,37 +134,25 @@ describe('isSelfSession', () => { describe('the kill confirmation', () => { const state: TuiConfirmState = { sessionId: 'abcdef01-2345', name: 'w4-api', typed: '' }; - it('accepts the shown name or the id prefix a mux name carries, and nothing else', () => { - expect(confirmAccepts(state, 'w4-api')).toBe(true); - expect(confirmAccepts(state, ' w4-api ')).toBe(true); - expect(confirmAccepts(state, 'abcdef01')).toBe(true); - expect(confirmAccepts(state, 'w4')).toBe(false); - expect(confirmAccepts(state, 'W4-API')).toBe(false); - expect(confirmAccepts(state, '')).toBe(false); - expect(confirmAccepts(state, ' ')).toBe(false); + it('kills on y, upper or lower', () => { + // Was: type the session's full name. The tester's verdict on that was + // "thats stupid, just make me type Y to confirm", and they were right — + // `x` then `y` is already two deliberate keystrokes on a selected row. + expect(confirmKillStep(state, { type: 'char', value: 'y' })).toEqual({ kind: 'confirm' }); + expect(confirmKillStep(state, { type: 'char', value: 'Y' })).toEqual({ kind: 'confirm' }); }); - it('types, backspaces and cancels', () => { - expect(confirmKillStep({ ...state, typed: 'w4' }, { type: 'char', value: '-' })).toEqual({ - kind: 'typing', - typed: 'w4-', - }); - expect(confirmKillStep({ ...state, typed: 'w4-' }, { type: 'backspace' })).toEqual({ kind: 'typing', typed: 'w4' }); - expect(confirmKillStep({ ...state, typed: '' }, { type: 'backspace' })).toEqual({ kind: 'typing', typed: '' }); + it('cancels on every other key, rather than leaving the prompt armed', () => { + // A dialog that ignores unknown keys sits there consuming whatever the + // user types next, which for a destructive prompt is the wrong default. + expect(confirmKillStep(state, { type: 'char', value: 'n' })).toEqual({ kind: 'cancel' }); + expect(confirmKillStep(state, { type: 'char', value: 'x' })).toEqual({ kind: 'cancel' }); expect(confirmKillStep(state, { type: 'escape' })).toEqual({ kind: 'cancel' }); expect(confirmKillStep(state, { type: 'ctrl', key: 'c' })).toEqual({ kind: 'cancel' }); - expect(confirmKillStep(state, { type: 'ctrl', key: 'a' })).toEqual({ kind: 'ignore' }); - expect(confirmKillStep(state, { type: 'tab' })).toEqual({ kind: 'ignore' }); }); - it('confirms only on a match, and says so rather than doing nothing otherwise', () => { - expect(confirmKillStep({ ...state, typed: 'w4-api' }, { type: 'enter' })).toEqual({ kind: 'confirm' }); - expect(confirmKillStep({ ...state, typed: 'w4' }, { type: 'enter' })).toEqual({ kind: 'reject' }); - expect(confirmKillStep({ ...state, typed: '' }, { type: 'enter' })).toEqual({ kind: 'reject' }); - }); - - it('backspaces one whole character, not one code unit', () => { - expect(confirmKillStep({ ...state, typed: 'a🙂' }, { type: 'backspace' })).toEqual({ kind: 'typing', typed: 'a' }); + it('does NOT kill on Enter, the key most likely to be hit by reflex', () => { + expect(confirmKillStep(state, { type: 'enter' })).toEqual({ kind: 'cancel' }); }); }); @@ -215,7 +202,7 @@ describe('footerKeysFor', () => { expect.arrayContaining([ 'attach — on a RECENT row, resume that conversation', 'new session', - 'kill (typed confirmation)', + 'kill (y to confirm)', 'quit', ]) ); @@ -227,7 +214,7 @@ describe('footerKeysFor', () => { it('follows the overlay that owns the keyboard', () => { expect(footerKeysFor('help', GLYPHS, { server: true })).toEqual(['esc close']); - expect(footerKeysFor('confirm-kill', GLYPHS, { server: true }).join(' ')).toContain('type the name'); + expect(footerKeysFor('confirm-kill', GLYPHS, { server: true }).join(' ')).toContain('y kill'); expect(footerKeysFor('message', GLYPHS, { server: true })).toEqual(['esc dismiss']); expect(footerKeysFor('new-session', GLYPHS, { server: true }).join(' ')).toContain('type to filter'); expect(footerKeysFor('prompt', GLYPHS, { server: true }).join(' ')).toContain('send'); diff --git a/test/tui/tui-e2e.test.ts b/test/tui/tui-e2e.test.ts index 0191f599..9163e366 100644 --- a/test/tui/tui-e2e.test.ts +++ b/test/tui/tui-e2e.test.ts @@ -556,23 +556,32 @@ describe('codeman tui (under a pty)', () => { it('opens and closes the help overlay', async () => { term.write('?'); await waitFor(() => frameLines(output).some((line) => line.includes('Keys')), 'the help overlay'); - expect(frameLines(output).join('\n')).toContain('kill (typed confirmation)'); + expect(frameLines(output).join('\n')).toContain('kill (y to confirm)'); term.write('\u001b'); await waitFor(() => !frameLines(output).some((line) => line.includes('Keys')), 'escape to close the overlay'); }); - it('asks for the session name before killing anything', async () => { + it('asks for a y before killing anything, and names what it would kill', async () => { term.write('x'); await waitFor(() => frameLines(output).some((line) => line.includes('Kill session')), 'the kill confirmation'); const overlay = frameLines(output).join('\n'); - expect(overlay).toContain('Type the name to confirm'); + expect(overlay).toContain('press y to kill'); expect(overlay).toContain('w2-beta'); term.write('\u001b'); await waitFor(() => !frameLines(output).some((line) => line.includes('Kill session')), 'escape to cancel the kill'); }); + it('cancels the kill on any key that is not y, and kills nothing', async () => { + term.write('x'); + await waitFor(() => frameLines(output).some((line) => line.includes('Kill session')), 'the kill confirmation'); + term.write('n'); + await waitFor(() => !frameLines(output).some((line) => line.includes('Kill session')), 'the dialog to close'); + // The session is still listed: `n` cancelled rather than killed. + await waitFor(() => frameLines(output).some((line) => line.includes('w2-beta')), 'w2-beta to still be listed'); + }); + it('sends a one-line prompt with p', async () => { term.write('p'); await waitFor(() => frameLines(output)[ROWS - 1].startsWith(' >'), 'the composer to open'); diff --git a/test/tui/tui-model.test.ts b/test/tui/tui-model.test.ts index 3a490831..2c3b34ef 100644 --- a/test/tui/tui-model.test.ts +++ b/test/tui/tui-model.test.ts @@ -267,11 +267,17 @@ describe('the store', () => { it('tracks the confirm-kill overlay, keyed to the name it showed', () => { const model = createTuiModel(); model.replaceSessions([session({ sessionId: 'a', name: 'w4-api' })]); - model.beginConfirmKill(model.rows()[0]); + model.beginConfirmKill(model.rows()[0], 'w4-api'); expect(model.mode).toBe('confirm-kill'); - expect(model.confirm).toEqual({ sessionId: 'a', name: 'w4-api', typed: '' }); - model.setConfirmInput('w4-ap'); - expect(model.confirm?.typed).toBe('w4-ap'); + // The name is the whole payload: it is what the dialog shows so the user + // knows WHICH session a `y` is about to destroy. + expect(model.confirm).toEqual({ sessionId: 'a', name: 'w4-api' }); + + // Regression: the label is supplied by the caller. Deriving it here with + // `name ?? id` let an EMPTY name through, and the dialog read "Kill ?". + model.replaceSessions([session({ sessionId: 'b', name: '' })]); + model.beginConfirmKill(model.rows()[0], 'mirofish'); + expect(model.confirm?.name).toBe('mirofish'); model.closeOverlay(); expect(model.mode).toBe('list'); expect(model.confirm).toBeNull(); diff --git a/test/tui/tui-render.test.ts b/test/tui/tui-render.test.ts index 2fa113a7..fc64e5e8 100644 --- a/test/tui/tui-render.test.ts +++ b/test/tui/tui-render.test.ts @@ -300,15 +300,15 @@ describe('overlays', () => { expect(lines[lines.length - 1]).toContain('close'); }); - it('draws the typed confirmation for a kill', () => { + it('names what a kill would destroy, and asks for one key', () => { const model = fixture(); - model.beginConfirmKill(model.rows()[0]); - model.setConfirmInput('w6-d'); + model.beginConfirmKill(model.rows()[0], 'w6-docs'); const text = frameLines(render(model, 100, 30)).join('\n'); expect(text).toContain('Kill w6-docs?'); - expect(text).toContain('Type the name to confirm:'); - expect(text).toContain('w6-d_'); - expect(text).toContain('esc cancel'); + expect(text).toContain('press y to kill, any other key cancels'); + // The name is the point of the dialog: it is what tells the user WHICH + // session a keystroke is about to destroy. + expect(text).not.toContain('Type the name to confirm'); }); it('draws a message box', () => {