From 05004456d669db510a6f73326c98397941b4e72f Mon Sep 17 00:00:00 2001 From: Reinis Date: Mon, 12 Oct 2026 15:47:00 +0300 Subject: [PATCH 1/2] feat(sessions): inline delete in resume picker with confirm dialog Resume picker (ResumeConversation + LogSelector) gains Delete-key deletion with an explicit confirm step (DeleteSessionConfirm dialog), backed directly by the existing SDK deleteSession() primitive. Guards: active session file, live background sessions; optimistic list update; raced delete treated as success; filesystem errors surface verbatim and keep the row. Tests: 12 new (resumeDeleteGuards 4, sessions.delete 5, DeleteSessionConfirm.render 3). Manual QA: delete/confirm, esc-cancel, search-filtered delete, cross-project delete all verified live; footer shows Del hint. Plan: C:\Users\reini\Documents\Default Project\docs\openclaude\plans\2026-10-12-sessions-delete-easy-option-plan.md --- .../DeleteSessionConfirm.render.test.tsx | 212 ++++++++++++++++++ src/components/DeleteSessionConfirm.tsx | 34 +++ src/components/LogSelector.tsx | 33 ++- src/entrypoints/sdk/sessions.delete.test.ts | 88 ++++++++ src/screens/ResumeConversation.tsx | 91 +++++++- src/screens/resumeDeleteGuards.test.ts | 31 +++ src/screens/resumeDeleteGuards.ts | 25 +++ 7 files changed, 508 insertions(+), 6 deletions(-) create mode 100644 src/components/DeleteSessionConfirm.render.test.tsx create mode 100644 src/components/DeleteSessionConfirm.tsx create mode 100644 src/entrypoints/sdk/sessions.delete.test.ts create mode 100644 src/screens/resumeDeleteGuards.test.ts create mode 100644 src/screens/resumeDeleteGuards.ts diff --git a/src/components/DeleteSessionConfirm.render.test.tsx b/src/components/DeleteSessionConfirm.render.test.tsx new file mode 100644 index 0000000000..9297a4dbd3 --- /dev/null +++ b/src/components/DeleteSessionConfirm.render.test.tsx @@ -0,0 +1,212 @@ +import { PassThrough } from 'node:stream' + +import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test' +import React from 'react' +import { stripVTControlCharacters as stripAnsi } from 'node:util' + +import { createRoot } from '../ink.js' +import { DEFAULT_BINDINGS } from '../keybindings/defaultBindings.js' +import { KeybindingProvider } from '../keybindings/KeybindingContext.js' +import { parseBindings } from '../keybindings/parser.js' +import { + acquireSharedMutationLock, + releaseSharedMutationLock, +} from '../test/sharedMutationLock.js' +import type { + KeybindingContextName, + ParsedKeystroke, +} from '../keybindings/types.js' +import { getDefaultAppState } from '../state/AppStateStore.js' +import { DeleteSessionConfirm } from './DeleteSessionConfirm.js' + +const sessionId = '550e8400-e29b-41d4-a716-446655440000' + +function createTestStreams() { + let output = '' + const stdout = new PassThrough() + const stdin = new PassThrough() as PassThrough & { + isTTY: boolean + setRawMode: (mode: boolean) => void + ref: () => void + unref: () => void + } + stdin.isTTY = true + stdin.setRawMode = () => {} + stdin.ref = () => {} + stdin.unref = () => {} + ;(stdout as unknown as { columns: number }).columns = 120 + stdout.on('data', chunk => { + output += chunk.toString() + }) + return { stdout, stdin, getOutput: () => output } +} + +async function waitFor( + predicate: () => boolean, + timeoutMs = 2000, +): Promise { + const startedAt = Date.now() + while (Date.now() - startedAt < timeoutMs) { + if (predicate()) return + await Bun.sleep(10) + } + throw new Error('Timed out waiting for DeleteSessionConfirm render test') +} + +function TestKeybindingProvider({ + children, +}: { + children: React.ReactNode +}): React.ReactNode { + const bindings = React.useMemo(() => parseBindings(DEFAULT_BINDINGS), []) + const pendingChordRef = React.useRef(null) + const [pendingChord, setPendingChordState] = React.useState< + ParsedKeystroke[] | null + >(null) + const activeContextsRef = React.useRef>(new Set()) + const handlerRegistryRef = React.useRef( + new Map< + string, + Set<{ + action: string + context: KeybindingContextName + handler: () => void + }> + >(), + ) + const setPendingChord = React.useCallback( + (pending: ParsedKeystroke[] | null) => { + pendingChordRef.current = pending + setPendingChordState(pending) + }, + [], + ) + const registerActiveContext = React.useCallback( + (context: KeybindingContextName) => { + activeContextsRef.current.add(context) + }, + [], + ) + const unregisterActiveContext = React.useCallback( + (context: KeybindingContextName) => { + activeContextsRef.current.delete(context) + }, + [], + ) + return ( + + {children} + + ) +} + +async function renderConfirm(onConfirm: () => void, onCancel: () => void) { + const { stdout, stdin, getOutput } = createTestStreams() + const { AppStateProvider } = await import('../state/AppState.js') + const root = await createRoot({ + stdout: stdout as unknown as NodeJS.WriteStream, + stdin: stdin as unknown as NodeJS.ReadStream, + patchConsole: false, + }) + root.render( + + + + + , + ) + return { stdout, stdin, getOutput, root } +} + +beforeEach(async () => { + await acquireSharedMutationLock( + 'components/DeleteSessionConfirm.render.test.tsx', + ) +}) + +afterEach(() => { + try { + mock.restore() + } finally { + releaseSharedMutationLock() + } +}) + +describe('DeleteSessionConfirm (rendered)', () => { + test('shows title, short id, and irreversibility warning', async () => { + const { stdout, stdin, getOutput, root } = await renderConfirm( + mock(() => {}), + mock(() => {}), + ) + try { + await waitFor(() => + stripAnsi(getOutput()).includes('Delete session?'), + ) + const output = stripAnsi(getOutput()) + expect(output).toContain('Fix the login bug') + expect(output).toContain('550e8400') + expect(output).toContain('440000') + expect(output).toContain('cannot be undone') + } finally { + root.unmount() + stdin.end() + stdout.end() + } + }) + + test('Enter confirms without cancelling', async () => { + const onConfirm = mock(() => {}) + const onCancel = mock(() => {}) + const { stdout, stdin, getOutput, root } = await renderConfirm( + onConfirm, + onCancel, + ) + try { + await waitFor(() => + stripAnsi(getOutput()).includes('Delete session?'), + ) + stdin.write('\r') + await waitFor(() => onConfirm.mock.calls.length > 0) + expect(onCancel).not.toHaveBeenCalled() + } finally { + root.unmount() + stdin.end() + stdout.end() + } + }) + + test('Escape cancels without confirming', async () => { + const onConfirm = mock(() => {}) + const onCancel = mock(() => {}) + const { stdout, stdin, getOutput, root } = await renderConfirm( + onConfirm, + onCancel, + ) + try { + await waitFor(() => + stripAnsi(getOutput()).includes('Delete session?'), + ) + stdin.write('\x1b') + await waitFor(() => onCancel.mock.calls.length > 0) + expect(onConfirm).not.toHaveBeenCalled() + } finally { + root.unmount() + stdin.end() + stdout.end() + } + }) +}) diff --git a/src/components/DeleteSessionConfirm.tsx b/src/components/DeleteSessionConfirm.tsx new file mode 100644 index 0000000000..76501ea8fd --- /dev/null +++ b/src/components/DeleteSessionConfirm.tsx @@ -0,0 +1,34 @@ +import React from 'react'; +import { Text } from '../ink.js'; +import { useKeybinding } from '../keybindings/useKeybinding.js'; +import { Dialog } from './design-system/Dialog.js'; + +type Props = { + sessionTitle: string; + sessionId: string; + onConfirm: () => void; + onCancel: () => void; +}; + +// Plain React (not react-compiler output): dialogs render rarely, and the +// memo-slot bookkeeping isn't worth maintaining here (see Dialog.tsx). +export function DeleteSessionConfirm({ + sessionTitle, + sessionId, + onConfirm, + onCancel, +}: Props): React.ReactNode { + useKeybinding('confirm:yes', onConfirm, { context: 'Confirmation' }); + const shortId = + sessionId.length > 14 + ? `${sessionId.slice(0, 8)}…${sessionId.slice(-6)}` + : sessionId; + return ( + + + {sessionTitle} ({shortId}) + + This cannot be undone. + + ); +} diff --git a/src/components/LogSelector.tsx b/src/components/LogSelector.tsx index 9e29b3210a..60fc68feed 100644 --- a/src/components/LogSelector.tsx +++ b/src/components/LogSelector.tsx @@ -48,6 +48,8 @@ export type LogSelectorProps = { forceWidth?: number; onCancel?: () => void; onSelect: (log: LogOption) => void; + onDeleteRequest?: (log: LogOption) => void; + isInputDisabled?: boolean; onLogsChanged?: () => void; onLoadMore?: (count: number) => void; initialSearchQuery?: string; @@ -239,6 +241,8 @@ export function LogSelector(t0: LogSelectorProps) { forceWidth, onCancel, onSelect, + onDeleteRequest, + isInputDisabled: t1a, onLogsChanged, onLoadMore, initialSearchQuery, @@ -248,6 +252,7 @@ export function LogSelector(t0: LogSelectorProps) { } = t0; const maxHeight = t1 === undefined ? Infinity : t1; const showAllProjects = t2 === undefined ? false : t2; + const isInputDisabled = t1a === undefined ? false : t1a; const terminalSize = useTerminalSize(); const columns = forceWidth === undefined ? terminalSize.columns : forceWidth; const exitState = useExitOnCtrlCDWithKeybindings(onCancel); @@ -308,6 +313,18 @@ export function LogSelector(t0: LogSelectorProps) { const [viewMode, setViewMode] = React.useState("list"); const [previewLog, setPreviewLog] = React.useState(null); const prevFocusedIdRef = React.useRef(null); + // Fresh-per-keypress view of the delete props for the memoized input + // handler below (t53): reading via ref avoids adding new memo slots. + const deleteUiRef = React.useRef({ + onDeleteRequest, + isInputDisabled + }); + React.useEffect(() => { + deleteUiRef.current = { + onDeleteRequest, + isInputDisabled + }; + }); const [selectedTagIndex, setSelectedTagIndex] = React.useState(0); let t8: AgenticSearchState; if ($[7] === Symbol.for("react.memo_cache_sentinel")) { @@ -1120,6 +1137,9 @@ export function LogSelector(t0: LogSelectorProps) { let t53; if ($[131] !== agenticSearchState.status || $[132] !== branchFilterEnabled || $[133] !== focusedLog || $[134] !== handleAgenticSearch || $[135] !== hasMultipleWorktrees || $[136] !== hasTags || $[137] !== isAgenticSearchOptionFocused || $[138] !== onAgenticSearch || $[139] !== onToggleAllProjects || $[140] !== searchQuery || $[141] !== setSearchQuery || $[142] !== showAllProjects || $[143] !== showAllWorktrees || $[144] !== tagTabs || $[145] !== uniqueTags || $[146] !== viewMode) { t53 = (input, key) => { + if (deleteUiRef.current.isInputDisabled) { + return; + } if (viewMode === "preview") { return; } @@ -1210,6 +1230,10 @@ export function LogSelector(t0: LogSelectorProps) { messageCount: focusedLog.messageCount }); } else { + if (key.delete && focusedLog) { + deleteUiRef.current.onDeleteRequest?.(focusedLog); + return; + } if (focusedLog && keyIsNotCtrlOrMeta && input.length > 0 && !/^\s+$/.test(input)) { setViewMode("search"); setSearchQuery(input); @@ -1424,11 +1448,14 @@ export function LogSelector(t0: LogSelectorProps) { } else { t69 = $[201]; } + // isInputDisabled is read directly (not in the t70 dep list below): + // the parent passes fresh onSelect/onLogsChanged identities every + // render, so t70 already recomputes on the renders that flip the flag. let t70; if ($[202] !== agenticSearchState.status || $[203] !== branchFilterEnabled || $[204] !== columns || $[205] !== displayedLogs || $[206] !== expandedGroupSessionIds || $[207] !== flatOptions || $[208] !== focusedLog || $[209] !== focusedNode?.id || $[210] !== handleFlatOptionsSelectFocus || $[211] !== handleRenameSubmit || $[212] !== handleTreeSelectFocus || $[213] !== isAgenticSearchOptionFocused || $[214] !== onCancel || $[215] !== onSelect || $[216] !== renameCursorOffset || $[217] !== renameValue || $[218] !== treeNodes || $[219] !== viewMode || $[220] !== visibleCount) { t70 = agenticSearchState.status === "searching" ? null : viewMode === "rename" && focusedLog ? Rename session: : isResumeWithRenameEnabled ? { onSelect(node_0.value.log); - }} onFocus={handleTreeSelectFocus} onCancel={onCancel} focusNodeId={focusedNode?.id} visibleOptionCount={visibleCount} layout="expanded" isDisabled={viewMode === "search" || isAgenticSearchOptionFocused} hideIndexes={false} isNodeExpanded={nodeId => { + }} onFocus={handleTreeSelectFocus} onCancel={onCancel} focusNodeId={focusedNode?.id} visibleOptionCount={visibleCount} layout="expanded" isDisabled={viewMode === "search" || isAgenticSearchOptionFocused || isInputDisabled} hideIndexes={false} isNodeExpanded={nodeId => { if (viewMode === "search" || branchFilterEnabled) { return true; } @@ -1455,7 +1482,7 @@ export function LogSelector(t0: LogSelectorProps) { if (log_13) { onSelect(log_13); } - }} visibleOptionCount={visibleCount} onCancel={onCancel} onFocus={handleFlatOptionsSelectFocus} defaultFocusValue={focusedNode?.id.toString()} layout="expanded" isDisabled={viewMode === "search" || isAgenticSearchOptionFocused} onUpFromFirstItem={enterSearchMode} />; + }} visibleOptionCount={visibleCount} onCancel={onCancel} onFocus={handleFlatOptionsSelectFocus} defaultFocusValue={focusedNode?.id.toString()} layout="expanded" isDisabled={viewMode === "search" || isAgenticSearchOptionFocused || isInputDisabled} onUpFromFirstItem={enterSearchMode} />; $[202] = agenticSearchState.status; $[203] = branchFilterEnabled; $[204] = columns; @@ -1481,7 +1508,7 @@ export function LogSelector(t0: LogSelectorProps) { } let t71; if ($[222] !== agenticSearchState.status || $[223] !== currentBranch || $[224] !== exitState.keyName || $[225] !== exitState.pending || $[226] !== getExpandCollapseHint || $[227] !== hasMultipleWorktrees || $[228] !== isAgenticSearchOptionFocused || $[229] !== isSearching || $[230] !== onToggleAllProjects || $[231] !== showAllProjects || $[232] !== showAllWorktrees || $[233] !== viewMode) { - t71 = {exitState.pending ? Press {exitState.keyName} again to exit : viewMode === "rename" ? : agenticSearchState.status === "searching" ? Searching with Claude… : isAgenticSearchOptionFocused ? : viewMode === "search" ? {isSearching && false ? "Searching\u2026" : "Type to Search"} : {onToggleAllProjects && }{currentBranch && }{hasMultipleWorktrees && }Type to search{getExpandCollapseHint() && {getExpandCollapseHint()}}}; + t71 = {exitState.pending ? Press {exitState.keyName} again to exit : viewMode === "rename" ? : agenticSearchState.status === "searching" ? Searching with Claude… : isAgenticSearchOptionFocused ? : viewMode === "search" ? {isSearching && false ? "Searching\u2026" : "Type to Search"} : {onToggleAllProjects && }{currentBranch && }{hasMultipleWorktrees && }Type to search{getExpandCollapseHint() && {getExpandCollapseHint()}}}; $[222] = agenticSearchState.status; $[223] = currentBranch; $[224] = exitState.keyName; diff --git a/src/entrypoints/sdk/sessions.delete.test.ts b/src/entrypoints/sdk/sessions.delete.test.ts new file mode 100644 index 0000000000..944f7e1ce6 --- /dev/null +++ b/src/entrypoints/sdk/sessions.delete.test.ts @@ -0,0 +1,88 @@ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test' +import { randomUUID } from 'node:crypto' +import { existsSync } from 'node:fs' +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { + acquireSharedMutationLock, + releaseSharedMutationLock, +} from '../../test/sharedMutationLock.js' +import { setClaudeConfigHomeDirForTesting } from '../../utils/envUtils.js' +import { getProjectDir } from '../../utils/sessionStoragePortable.js' +import { deleteSession } from './sessions.js' + +let configDir = '' + +beforeEach(async () => { + await acquireSharedMutationLock('entrypoints/sdk/sessions.delete.test.ts') + configDir = await mkdtemp(join(tmpdir(), 'openclaude-sessions-delete-')) + setClaudeConfigHomeDirForTesting(configDir) +}) + +afterEach(async () => { + try { + setClaudeConfigHomeDirForTesting(undefined) + if (configDir) await rm(configDir, { recursive: true, force: true }) + } finally { + releaseSharedMutationLock() + } +}) + +async function seedSessionFile( + sessionId: string, + projectPath: string, +): Promise { + const dir = getProjectDir(projectPath) + await mkdir(dir, { recursive: true }) + const filePath = join(dir, `${sessionId}.jsonl`) + await writeFile( + filePath, + `{"type":"user","uuid":"${sessionId}","sessionId":"${sessionId}"}\n`, + ) + return filePath +} + +describe('deleteSession', () => { + test('rejects invalid session ids without touching disk', async () => { + await expect(deleteSession('not-a-uuid')).rejects.toThrow( + 'Invalid session ID', + ) + await expect(deleteSession('../../etc/passwd')).rejects.toThrow( + 'Invalid session ID', + ) + }) + + test('throws Session not found for a missing id', async () => { + const missing = randomUUID() + await expect(deleteSession(missing)).rejects.toThrow( + `Session not found: ${missing}`, + ) + }) + + test('unlinks the JSONL file resolved via global search', async () => { + const sessionId = randomUUID() + const filePath = await seedSessionFile(sessionId, 'C:\\work\\proj') + expect(existsSync(filePath)).toBe(true) + await deleteSession(sessionId) + expect(existsSync(filePath)).toBe(false) + }) + + test('unlinks via an explicit project dir', async () => { + const sessionId = randomUUID() + const projectPath = 'C:\\work\\proj' + const filePath = await seedSessionFile(sessionId, projectPath) + await deleteSession(sessionId, { dir: projectPath }) + expect(existsSync(filePath)).toBe(false) + }) + + test('second delete reports Session not found (raced-delete contract)', async () => { + const sessionId = randomUUID() + await seedSessionFile(sessionId, 'C:\\work\\proj') + await deleteSession(sessionId) + await expect(deleteSession(sessionId)).rejects.toThrow( + 'Session not found', + ) + }) +}) diff --git a/src/screens/ResumeConversation.tsx b/src/screens/ResumeConversation.tsx index 4b92ab2fd2..28f90449fa 100644 --- a/src/screens/ResumeConversation.tsx +++ b/src/screens/ResumeConversation.tsx @@ -3,9 +3,10 @@ import { feature } from 'bun:bundle'; import { dirname } from 'path'; import React from 'react'; import { useTerminalSize } from 'src/hooks/useTerminalSize.js'; -import { getOriginalCwd, switchSession } from '../bootstrap/state.js'; +import { getOriginalCwd, getSessionId, switchSession } from '../bootstrap/state.js'; import type { Command } from '../commands.js'; -import { LogSelector } from '../components/LogSelector.js'; +import { LogSelector, getResumeLogDisplayTitle } from '../components/LogSelector.js'; +import { DeleteSessionConfirm } from '../components/DeleteSessionConfirm.js'; import { Spinner } from '../components/Spinner.js'; import { restoreCostStateForSession } from '../cost-tracker.js'; import { setClipboard } from '../ink/termio/osc.js'; @@ -13,6 +14,9 @@ import { Box, Text } from '../ink.js'; import { AlternateScreen } from '../ink/components/AlternateScreen.js'; import useApp from '../ink/hooks/use-app.js'; import { useKeybinding } from '../keybindings/useKeybinding.js'; +import { listBackgroundSessions } from '../cli/bgRegistry.js'; +import { deleteSession } from '../entrypoints/sdk/sessions.js'; +import { getSessionIdFromLog } from '../utils/sessionStorage.js'; import { type AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, logEvent } from '../services/analytics/index.js'; import type { MCPServerConnection, ScopedMcpServerConfig } from '../services/mcp/types.js'; import { useAppState, useSetAppState } from '../state/AppState.js'; @@ -39,6 +43,7 @@ import type { ThinkingConfig } from '../utils/thinking.js'; import { filterContentReplacementsForMessages, type ContentReplacementRecord } from '../utils/toolResultStorage.js'; import { REPL } from './REPL.js'; import { filterResumeLogs } from './resumeFilters.js'; +import { getDeleteBlockReason } from './resumeDeleteGuards.js'; type Props = { commands: Command[]; worktreePaths: string[]; @@ -104,6 +109,9 @@ export function ResumeConversation({ mainThreadAgentDefinition?: AgentDefinition; } | null>(null); const [resumeError, setResumeError] = React.useState(null); + const [deletingLog, setDeletingLog] = React.useState(null); + const [deleteError, setDeleteError] = React.useState(null); + const [deleteNotice, setDeleteNotice] = React.useState(null); const [crossProjectCommand, setCrossProjectCommand] = React.useState(null); const sessionLogResultRef = React.useRef(null); // Mirror of logs.length so loadMoreLogs can compute value indices outside @@ -313,6 +321,77 @@ export function ResumeConversation({ setResuming(false); } } + function dropLogFromList(sessionId: string) { + setLogs(prev => prev.filter(l => getSessionIdFromLog(l) !== sessionId)); + logCountRef.current = Math.max(0, logCountRef.current - 1); + } + async function handleDeleteRequest(log_0: LogOption) { + const targetId = getSessionIdFromLog(log_0); + setDeleteError(null); + setDeleteNotice(null); + if (!targetId) { + setDeleteError('Cannot determine the session id for this entry.'); + return; + } + if (targetId === getSessionId()) { + setDeleteError('Cannot delete the active session.'); + return; + } + try { + const bgSessions = await listBackgroundSessions(); + const runningIds = new Set(bgSessions.filter(s => s.status === 'running' || s.status === 'unknown').map(s => s.sessionId)); + const blockReason = getDeleteBlockReason(targetId, getSessionId(), runningIds); + if (blockReason) { + setDeleteError(blockReason); + return; + } + } catch (e) { + setDeleteError(`Failed to check background sessions: ${errorMessage(e)}`); + return; + } + setDeletingLog(log_0); + } + async function handleDeleteConfirm() { + const target = deletingLog; + if (!target) return; + const targetId = getSessionIdFromLog(target); + if (!targetId) { + setDeletingLog(null); + return; + } + try { + await deleteSession(targetId); + dropLogFromList(targetId); + setDeletingLog(null); + setDeleteNotice(`Deleted ${getResumeLogDisplayTitle(target)}.`); + logEvent('tengu_session_delete', { + confirmed: true, + success: true + }); + } catch (e) { + if (e instanceof Error && e.message.startsWith('Session not found')) { + dropLogFromList(targetId); + setDeletingLog(null); + setDeleteNotice('Already deleted.'); + logEvent('tengu_session_delete', { + confirmed: true, + success: true + }); + return; + } + setDeleteError(`Failed to delete: ${errorMessage(e)}`); + logEvent('tengu_session_delete', { + confirmed: true, + success: false + }); + } + } + function handleDeleteCancel() { + logEvent('tengu_session_delete', { + confirmed: false + }); + setDeletingLog(null); + } if (crossProjectCommand) { return ; } @@ -336,12 +415,18 @@ export function ResumeConversation({ {resumeError} Choose a different conversation to continue. : null; + const deleteBanner = deleteError || deleteNotice ? + {deleteError && {deleteError}} + {deleteNotice && {deleteNotice}} + : null; if (filteredLogs.length === 0) { return ; } const picker = {resumeErrorBanner} - loadLogs(showAllProjects) : undefined} onLoadMore={loadMoreLogs} initialSearchQuery={initialSearchQuery} showAllProjects={showAllProjects} onToggleAllProjects={handleToggleAllProjects} onAgenticSearch={agenticSessionSearch} /> + {deleteBanner} + {deletingLog && void handleDeleteConfirm()} onCancel={handleDeleteCancel} />} + loadLogs(showAllProjects) : undefined} onLoadMore={loadMoreLogs} initialSearchQuery={initialSearchQuery} showAllProjects={showAllProjects} onToggleAllProjects={handleToggleAllProjects} onAgenticSearch={agenticSessionSearch} /> ; // Run the picker on the alternate screen everywhere except tmux -CC, where // alt-screen is known to corrupt terminal state. Cancelling then restores diff --git a/src/screens/resumeDeleteGuards.test.ts b/src/screens/resumeDeleteGuards.test.ts new file mode 100644 index 0000000000..c20bf39792 --- /dev/null +++ b/src/screens/resumeDeleteGuards.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, test } from 'bun:test' + +import { getDeleteBlockReason } from './resumeDeleteGuards.js' + +const active = '11111111-1111-4111-8111-111111111111' +const other = '22222222-2222-4222-8222-222222222222' +const bgRunning = '33333333-3333-4333-8333-333333333333' + +describe('getDeleteBlockReason', () => { + test('blocks entries without a resolvable session id', () => { + expect(getDeleteBlockReason(undefined, active, new Set())).toBe( + 'Cannot determine the session id for this entry.', + ) + }) + + test('never allows deleting the active session file', () => { + expect(getDeleteBlockReason(active, active, new Set())).toBe( + 'Cannot delete the active session.', + ) + }) + + test('blocks sessions with a live background process', () => { + expect( + getDeleteBlockReason(bgRunning, active, new Set([bgRunning])), + ).toBe('Stop the background session first before deleting.') + }) + + test('allows deleting an idle, non-active session', () => { + expect(getDeleteBlockReason(other, active, new Set([bgRunning]))).toBeNull() + }) +}) diff --git a/src/screens/resumeDeleteGuards.ts b/src/screens/resumeDeleteGuards.ts new file mode 100644 index 0000000000..5f5f925dca --- /dev/null +++ b/src/screens/resumeDeleteGuards.ts @@ -0,0 +1,25 @@ +/** + * Pure delete-guard decisions for the resume picker. + * + * Extracted so the safety-critical branches (never delete the active + * session file, never orphan a live background session) are unit-testable + * without mounting the Ink tree. + */ + +/** Message shown when deletion is blocked, or null when deletion may proceed. */ +export function getDeleteBlockReason( + targetSessionId: string | undefined, + activeSessionId: string, + bgRunningSessionIds: ReadonlySet, +): string | null { + if (!targetSessionId) { + return 'Cannot determine the session id for this entry.' + } + if (targetSessionId === activeSessionId) { + return 'Cannot delete the active session.' + } + if (bgRunningSessionIds.has(targetSessionId)) { + return 'Stop the background session first before deleting.' + } + return null +} From b04c774e572ac95a008ef92c543d2e57f7ac7159 Mon Sep 17 00:00:00 2001 From: Reinis Date: Mon, 12 Oct 2026 19:38:36 +0300 Subject: [PATCH 2/2] fix(sessions): bottom-placed delete notice and y/n confirm hints Move the delete success/error banner below the session list so it stays visible when scrolled down (previously top-placed and scrolled out of view). Advertise the y/n keys in the confirm dialog guide (Enter/y to confirm, Esc/n to cancel). Tests: DeleteSessionConfirm.render now covers n-cancel and guide text (5 tests). Live QA: scrolled delete shows bottom notice, remaining row stays focused, Esc exits cleanly. --- .../DeleteSessionConfirm.render.test.tsx | 40 +++++++++++++++++++ src/components/DeleteSessionConfirm.tsx | 4 +- src/screens/ResumeConversation.tsx | 2 +- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/components/DeleteSessionConfirm.render.test.tsx b/src/components/DeleteSessionConfirm.render.test.tsx index 9297a4dbd3..f46895823c 100644 --- a/src/components/DeleteSessionConfirm.render.test.tsx +++ b/src/components/DeleteSessionConfirm.render.test.tsx @@ -209,4 +209,44 @@ describe('DeleteSessionConfirm (rendered)', () => { stdout.end() } }) + + test('n cancels without confirming', async () => { + const onConfirm = mock(() => {}) + const onCancel = mock(() => {}) + const { stdout, stdin, getOutput, root } = await renderConfirm( + onConfirm, + onCancel, + ) + try { + await waitFor(() => + stripAnsi(getOutput()).includes('Delete session?'), + ) + stdin.write('n') + await waitFor(() => onCancel.mock.calls.length > 0) + expect(onConfirm).not.toHaveBeenCalled() + } finally { + root.unmount() + stdin.end() + stdout.end() + } + }) + + test('guide advertises y and n bindings', async () => { + const { stdout, stdin, getOutput, root } = await renderConfirm( + mock(() => {}), + mock(() => {}), + ) + try { + await waitFor(() => + stripAnsi(getOutput()).includes('Delete session?'), + ) + const output = stripAnsi(getOutput()) + expect(output).toContain('Enter/y') + expect(output).toContain('Esc/n') + } finally { + root.unmount() + stdin.end() + stdout.end() + } + }) }) diff --git a/src/components/DeleteSessionConfirm.tsx b/src/components/DeleteSessionConfirm.tsx index 76501ea8fd..8aa731d799 100644 --- a/src/components/DeleteSessionConfirm.tsx +++ b/src/components/DeleteSessionConfirm.tsx @@ -1,7 +1,9 @@ import React from 'react'; import { Text } from '../ink.js'; import { useKeybinding } from '../keybindings/useKeybinding.js'; +import { Byline } from './design-system/Byline.js'; import { Dialog } from './design-system/Dialog.js'; +import { KeyboardShortcutHint } from './design-system/KeyboardShortcutHint.js'; type Props = { sessionTitle: string; @@ -24,7 +26,7 @@ export function DeleteSessionConfirm({ ? `${sessionId.slice(0, 8)}…${sessionId.slice(-6)}` : sessionId; return ( - + exitState.pending ? Press {exitState.keyName} again to exit : }> {sessionTitle} ({shortId}) diff --git a/src/screens/ResumeConversation.tsx b/src/screens/ResumeConversation.tsx index 28f90449fa..a807c8f1d3 100644 --- a/src/screens/ResumeConversation.tsx +++ b/src/screens/ResumeConversation.tsx @@ -424,9 +424,9 @@ export function ResumeConversation({ } const picker = {resumeErrorBanner} - {deleteBanner} {deletingLog && void handleDeleteConfirm()} onCancel={handleDeleteCancel} />} loadLogs(showAllProjects) : undefined} onLoadMore={loadMoreLogs} initialSearchQuery={initialSearchQuery} showAllProjects={showAllProjects} onToggleAllProjects={handleToggleAllProjects} onAgenticSearch={agenticSessionSearch} /> + {deleteBanner} ; // Run the picker on the alternate screen everywhere except tmux -CC, where // alt-screen is known to corrupt terminal state. Cancelling then restores