diff --git a/extensions/sessions/index.ts b/extensions/sessions/index.ts index f29299aa..5effe53f 100644 --- a/extensions/sessions/index.ts +++ b/extensions/sessions/index.ts @@ -1,4 +1,5 @@ // Adapted from pi-agent-extensions (MIT); see THIRD_PARTY_NOTICES.md. +import path from "node:path"; import type { ExtensionAPI, ExtensionCommandContext, @@ -41,6 +42,7 @@ import { buildSessionLabel, buildSessionPreview, buildSessionSearchEntries, + deleteSessionFile, filterSessionEntries, formatRelativeTime, getSessionPaneLayout, @@ -599,6 +601,31 @@ async function showSessionPicker( let focus: "list" | "preview" = "list"; let showAllWorkspaces = false; let isLoading = false; + let confirmingDeletePath: string | null = null; + + const handleDelete = async (sessionPath: string) => { + const result = await deleteSessionFile(sessionPath); + if (result.ok) { + sorted = sorted.filter((s) => s.path !== sessionPath); + sessionByPath.delete(sessionPath); + statsUniverseVersion++; + entries = buildSessionSearchEntries(sorted); + rebuild(); + schedulePreviewLoad(); + ctx.ui.notify( + result.method === "trash" + ? "Session moved to trash" + : "Session deleted", + "info", + ); + } else { + ctx.ui.notify( + `Failed to delete session: ${result.error ?? "unknown error"}`, + "error", + ); + } + tui.requestRender(); + }; const cancelPreviewLoad = () => { previewSeq++; @@ -818,21 +845,20 @@ async function showSessionPicker( selectList.onSelectionChange = (item) => setSelectedPath(item.value); container.addChild(selectList); } - container.addChild( - new Text( - hintLine( - theme, - [ - ["↑↓", "navigate"], - ["enter", "open"], - ["esc", "cancel"], - ], - Math.max(1, width - 2), - ), - 1, - 0, - ), - ); + const singlePaneHints = + confirmingDeletePath !== null + ? theme.fg("error", "Delete session? Enter confirm · Esc cancel") + : hintLine( + theme, + [ + ["↑↓", "navigate"], + ["enter", "open"], + ["ctrl+d", "delete"], + ["esc", "cancel"], + ], + Math.max(1, width - 2), + ); + container.addChild(new Text(singlePaneHints, 1, 0)); container.addChild( new DynamicBorder((text: string) => theme.fg("accent", text)), ); @@ -925,31 +951,30 @@ async function showSessionPicker( previewScrollOffset, renderedPreview.maxScroll, ); - // Hints live on their own line under the frame instead of being packed - // into the bottom border, where they had to compete with the border for - // the same row and lost the keys in a wall of dim text. - const hints = hintLine( - theme, + const hintItems = [ + renderedPreview.maxScroll > 0 + ? ([ + "", + `${previewScrollOffset + 1}-${Math.min(previewScrollOffset + contentHeight, renderedPreview.totalLines)}/${renderedPreview.totalLines}`, + ] as const) + : undefined, + renderedPreview.maxScroll > 0 + ? (["pgup/pgdn", "scroll"] as const) + : undefined, + ["t", toolsExpanded ? "compact" : "tools"] as const, + ["h", thinkingVisible ? "hide thinking" : "thinking"] as const, [ - renderedPreview.maxScroll > 0 - ? ([ - "", - `${previewScrollOffset + 1}-${Math.min(previewScrollOffset + contentHeight, renderedPreview.totalLines)}/${renderedPreview.totalLines}`, - ] as const) - : undefined, - renderedPreview.maxScroll > 0 - ? (["pgup/pgdn", "scroll"] as const) - : undefined, - ["t", toolsExpanded ? "compact" : "tools"], - ["h", thinkingVisible ? "hide thinking" : "thinking"], - [ - "opt+w/ctrl+t", - showAllWorkspaces ? "current workspace" : "all workspaces", - ], - ["esc", "close"], - ], - width, - ); + "opt+w/ctrl+t", + showAllWorkspaces ? "current workspace" : "all workspaces", + ] as const, + ["ctrl+d", "delete"] as const, + ["esc", "close"] as const, + ]; + + const hints = + confirmingDeletePath !== null + ? theme.fg("error", "Delete session? Enter confirm · Esc cancel") + : hintLine(theme, hintItems, width); const lines = [buildTopBorder(layout.listWidth, layout.previewWidth)]; for (let i = 0; i < contentHeight; i++) { @@ -981,6 +1006,29 @@ async function showSessionPicker( }, dispose: disposePicker, handleInput: (data) => { + if (confirmingDeletePath !== null) { + if ( + matchesKey(data, Key.enter) || + kb.matches(data, "tui.select.confirm") + ) { + const target = confirmingDeletePath; + confirmingDeletePath = null; + void handleDelete(target); + return; + } + if ( + matchesKey(data, Key.escape) || + kb.matches(data, "tui.select.cancel") + ) { + confirmingDeletePath = null; + tui.requestRender(); + return; + } + confirmingDeletePath = null; + tui.requestRender(); + return; + } + if (data === "\u0014" || data === "\u001bw") { showAllWorkspaces = !showAllWorkspaces; void loadWorkspaceSessions(showAllWorkspaces); @@ -1074,6 +1122,31 @@ async function showSessionPicker( } if (focus === "list") { + if ( + matchesKey(data, Key.ctrl("d")) || + kb.matches(data, "app.session.delete") || + data === "\u0004" + ) { + const selectedSession = sessionByPath.get(selectedPath); + if (selectedSession) { + const activeSessionPath = ctx.sessionManager.getSessionFile(); + if ( + activeSessionPath && + path.resolve(selectedSession.path) === + path.resolve(activeSessionPath) + ) { + ctx.ui.notify( + "Cannot delete the currently active session", + "error", + ); + return; + } + confirmingDeletePath = selectedSession.path; + tui.requestRender(); + return; + } + } + if (isPrintable(data)) { filter += data; rebuild(); diff --git a/extensions/sessions/sessions.ts b/extensions/sessions/sessions.ts index a6f29589..35d96133 100644 --- a/extensions/sessions/sessions.ts +++ b/extensions/sessions/sessions.ts @@ -1,3 +1,6 @@ +import { spawn } from "node:child_process"; +import { existsSync } from "node:fs"; +import { unlink } from "node:fs/promises"; import { sanitizeTerminalText } from "../shared/terminal-text.ts"; export interface SessionInfoLike { @@ -447,3 +450,106 @@ export function buildPreviewError( error: message, }; } + +const TRASH_TIMEOUT_MS = 2_000; +const TRASH_KILL_GRACE_MS = 500; + +type TrashOutcome = "closed" | "aborted" | "uncertain"; + +/** Do not fall back to unlink until the helper is confirmed closed. */ +function runTrashAsync( + sessionPath: string, + timeoutMs = TRASH_TIMEOUT_MS, + signal?: AbortSignal, +): Promise { + if (signal?.aborted) return Promise.resolve("aborted"); + const trashArgs = sessionPath.startsWith("-") + ? ["--", sessionPath] + : [sessionPath]; + return new Promise((resolve) => { + let child: ReturnType; + try { + child = spawn("trash", trashArgs, { stdio: "ignore" }); + } catch { + // Spawn failed before a helper process could be started. + resolve("closed"); + return; + } + + let finished = false; + let stopped = false; + let aborted = false; + let killTimer: ReturnType | undefined; + const finish = (result: TrashOutcome) => { + if (finished) return; + finished = true; + clearTimeout(deadline); + if (killTimer) clearTimeout(killTimer); + signal?.removeEventListener("abort", onAbort); + resolve(result); + }; + const stop = (wasAborted: boolean) => { + if (stopped || finished) return; + stopped = true; + aborted = wasAborted; + // SIGKILL is required: a helper may ignore SIGTERM, including the + // SIGTERM sent by execFile's built-in timeout. + try { + child.kill("SIGKILL"); + } catch { + // A failed kill is not proof the helper has exited. + } + killTimer = setTimeout(() => { + child.unref(); + finish("uncertain"); + }, TRASH_KILL_GRACE_MS); + killTimer.unref?.(); + }; + const onAbort = () => stop(true); + const deadline = setTimeout(() => stop(false), timeoutMs); + deadline.unref?.(); + child.on("error", () => { + // A failed spawn is followed by close; wait for it rather than racing + // an error against a helper that may have already started. + }); + child.on("close", () => finish(aborted ? "aborted" : "closed")); + signal?.addEventListener("abort", onAbort, { once: true }); + if (signal?.aborted) stop(true); + }); +} + +export async function deleteSessionFile( + sessionPath: string, + options?: { timeoutMs?: number; signal?: AbortSignal }, +): Promise<{ ok: boolean; method: "trash" | "unlink"; error?: string }> { + if (!existsSync(sessionPath)) { + return { ok: false, method: "unlink", error: "File not found" }; + } + + const timeoutMs = + options?.timeoutMs !== undefined && + Number.isFinite(options.timeoutMs) && + options.timeoutMs > 0 + ? Math.min(options.timeoutMs, TRASH_TIMEOUT_MS) + : TRASH_TIMEOUT_MS; + const outcome = await runTrashAsync(sessionPath, timeoutMs, options?.signal); + if (outcome === "uncertain") { + return { + ok: false, + method: "trash", + error: "Trash helper termination unconfirmed; fallback deletion skipped", + }; + } + if (outcome === "aborted") { + return { ok: false, method: "trash", error: "Session deletion cancelled" }; + } + if (!existsSync(sessionPath)) return { ok: true, method: "trash" }; + + try { + await unlink(sessionPath); + return { ok: true, method: "unlink" }; + } catch (err) { + const error = err instanceof Error ? err.message : String(err); + return { ok: false, method: "unlink", error }; + } +} diff --git a/tests/extensions/sessions/sessions.test.ts b/tests/extensions/sessions/sessions.test.ts index 04838c6b..d5cc1e92 100644 --- a/tests/extensions/sessions/sessions.test.ts +++ b/tests/extensions/sessions/sessions.test.ts @@ -1,10 +1,15 @@ import assert from "node:assert/strict"; +import { existsSync } from "node:fs"; +import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; import test from "node:test"; import { buildSessionDescription, buildSessionLabel, buildSessionPreview, buildSessionSearchEntries, + deleteSessionFile, filterSessionEntries, formatRelativeTime, parseLimit, @@ -147,6 +152,114 @@ test("bounded preview reports omitted messages and content bytes", () => { assert.match(preview.subtitle, /100 messages/); }); +test("deleteSessionFile removes an existing file", async () => { + const file = path.join(tmpdir(), `openpi-del-test-${Date.now()}.jsonl`); + await writeFile(file, "{}"); + assert.equal(existsSync(file), true); + + const result = await deleteSessionFile(file); + assert.equal(result.ok, true); + assert.equal(existsSync(file), false); +}); + +test("deleteSessionFile returns error on non-existent file", async () => { + const file = path.join(tmpdir(), `nonexistent-${Date.now()}.jsonl`); + const result = await deleteSessionFile(file); + assert.equal(result.ok, false); +}); + +test("deleteSessionFile waits for a successful trash helper and reports trash", { + skip: process.platform === "win32" && "POSIX executable fixture", +}, async (t) => { + const dir = await mkdtemp(path.join(tmpdir(), "openpi-trash-success-")); + const oldPath = process.env.PATH; + t.after(async () => { + process.env.PATH = oldPath; + await rm(dir, { recursive: true, force: true }); + }); + const file = path.join(dir, "session.jsonl"); + await writeFile(file, "{}"); + const helper = path.join(dir, "trash"); + await writeFile( + helper, + `#!/usr/bin/env node +const fs = require("node:fs"); +fs.renameSync(process.argv[2], process.argv[2] + ".trashed"); +`, + ); + await chmod(helper, 0o755); + process.env.PATH = `${dir}${path.delimiter}${path.dirname(process.execPath)}${path.delimiter}${oldPath ?? ""}`; + assert.deepEqual(await deleteSessionFile(file), { + ok: true, + method: "trash", + }); + assert.equal(existsSync(file), false); + assert.equal(existsSync(`${file}.trashed`), true); +}); + +test("deleteSessionFile does not delete when cancelled before starting", async (t) => { + const dir = await mkdtemp(path.join(tmpdir(), "openpi-trash-abort-")); + t.after(() => rm(dir, { recursive: true, force: true })); + const file = path.join(dir, "session.jsonl"); + await writeFile(file, "{}"); + const controller = new AbortController(); + controller.abort(); + const result = await deleteSessionFile(file, { signal: controller.signal }); + assert.equal(result.ok, false); + assert.equal(existsSync(file), true); +}); + +test("deleteSessionFile waits for a SIGTERM-resistant trash helper to close before unlink", { + skip: process.platform === "win32" && "POSIX executable fixture", +}, async (t) => { + const dir = await mkdtemp(path.join(tmpdir(), "openpi-trash-test-")); + const oldPath = process.env.PATH; + const oldReady = process.env.OPENPI_TEST_TRASH_READY; + const file = path.join(dir, "session.jsonl"); + const ready = path.join(dir, "ready"); + let pid: number | undefined; + t.after(async () => { + process.env.PATH = oldPath; + if (oldReady === undefined) delete process.env.OPENPI_TEST_TRASH_READY; + else process.env.OPENPI_TEST_TRASH_READY = oldReady; + if (pid !== undefined) { + try { + process.kill(pid, "SIGKILL"); + } catch {} + } + await rm(dir, { recursive: true, force: true }); + }); + + await writeFile(file, "{}"); + const helper = path.join(dir, "trash"); + await writeFile( + helper, + `#!/usr/bin/env node +const fs = require("node:fs"); +process.on("SIGTERM", () => {}); +fs.writeFileSync(process.env.OPENPI_TEST_TRASH_READY, String(process.pid)); +setInterval(() => {}, 1000); +`, + ); + await chmod(helper, 0o755); + process.env.PATH = `${dir}${path.delimiter}${path.dirname(process.execPath)}${path.delimiter}${oldPath ?? ""}`; + process.env.OPENPI_TEST_TRASH_READY = ready; + + let pending = true; + let eventLoopResponsive = false; + setTimeout(() => { + if (pending) eventLoopResponsive = true; + }, 10); + const result = await deleteSessionFile(file, { timeoutMs: 1_500 }); + pending = false; + pid = Number(await readFile(ready, "utf8")); + assert.equal(eventLoopResponsive, true); + assert.deepEqual(result, { ok: true, method: "unlink" }); + assert.equal(existsSync(file), false); + // Once fallback has run, the trash helper cannot later touch the path. + assert.throws(() => process.kill(pid!, 0), { code: "ESRCH" }); +}); + test("formatRelativeTime deterministically formats relative hours and calendar dates with frozen clocks", () => { const baseNow = new Date("2026-09-21T12:00:00"); const tenMins = new Date("2026-09-21T11:50:00");