diff --git a/README.md b/README.md index a7f4dc30..5c139ee7 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ Details, errors and join rules: [docs/connected-windows.md](docs/connected-windo | Command | Action | | --- | --- | | `/optchat` | Status and actions menu. | -| `/optchat profile` | Select or create a profile. Switching starts a fresh Pi session. | +| `/optchat profile` | Select, create, or delete a profile. Ctrl+D deletes the highlighted one. | | `/optchat settings` | This profile's [settings](#settings). | | `/optchat model` | Compactor model and effort for this profile. Type to filter the models you are logged in to; the current one is marked. | | `/optchat agents` | Live agent tree and saved run history. | @@ -209,7 +209,7 @@ Profile data lives in `~/.optchat/profiles//` (override the root with `OPT | `memory.html` | Snapshot from `/optchat browse` | - Each profile folder is a local Git repository, committed after each turn. It has no remote, so it is not a backup: copy the folder while Pi is closed. -- To delete a profile, delete its folder. Your original Pi sessions are kept in Pi's normal session directory. +- To delete a profile, press Ctrl+D on it in `/optchat profile` and confirm: the folder goes away, and the last-profile pointer with it when it named the deleted profile. Pi refuses a profile another Pi is using, so that Pi never writes into a folder that is gone. Your original Pi sessions are kept in Pi's normal session directory. Git checkpoints and the pluggable `Store` seam (for memory on another machine): [docs/storage.md](docs/storage.md). diff --git a/src/index.ts b/src/index.ts index 11b7b9ab..ba13f79b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,7 +9,7 @@ import { Type } from 'typebox'; import { Memory } from './memory.ts'; import { atomicWrite, storeFor } from './store.ts'; import { createCompressor, type ModelChoice } from './compactor.ts'; -import { createProfile, holdProfile, instructions, isWindows, lastProfile, listProfiles, loadConfig, profilePath, rememberProfile, saveConfig, ProfileBusyError, type ProfileConfig } from './profiles.ts'; +import { createProfile, deleteProfile, holdProfile, instructions, isWindows, lastProfile, listProfiles, loadConfig, profilePath, rememberProfile, saveConfig, ProfileBusyError, type ProfileConfig } from './profiles.ts'; import { allowSearch, CONTINUITY, IMPORT_GUIDANCE, SEARCH_DOC, TURN_REFRESHED, VIEW_DOC } from './core/prompts.ts'; import { MASTER, modelsHint } from './prompts.ts'; import { cachePayload, record } from './core/cache.ts'; @@ -35,7 +35,7 @@ import { openConnectedWindow, registerConnectedRenderer } from './connected-wind import { createHandoffSummarizer } from './handoff.ts'; import { mainTitle, TabTitle } from './title.ts'; import { showModelPicker, showSettings } from './settings-page.ts'; -import { showMenu } from './menu.ts'; +import { CREATE_PROFILE, profilePicker, showMenu, type ProfilePick } from './menu.ts'; const binding = 'optchat.profile'; /** The profile a session is bound to; the latest binding wins. */ @@ -224,13 +224,25 @@ export default function optchat(pi: ExtensionAPI) { const chooseProfile = async (ctx: ExtensionContext): Promise => { // Outside the TUI (print, json, RPC hosts) never pick or create a profile: a dialog would hang the host, and a guess could write into the wrong memory. if (ctx.mode !== 'tui') return undefined; - const names = listProfiles(), last = lastProfile(); - if (last) names.sort((a, b) => Number(b === last) - Number(a === last)); - const selected = await ctx.ui.select('OptChat profile', [...names, '+ Create profile']); - if (selected !== '+ Create profile') return selected; - const name = (await ctx.ui.input('New profile name', 'work or personal'))?.trim(); - if (!name) return undefined; - createProfile(name); return name; + for (;;) { + const names = listProfiles(), last = lastProfile(); + if (last) names.sort((a, b) => Number(b === last) - Number(a === last)); + const picked = await ctx.ui.custom((tui, theme, keys, done) => profilePicker(theme, keys, names, done, () => tui.requestRender())); + if (!picked) return undefined; + if (typeof picked === 'string') { + if (picked !== CREATE_PROFILE) return picked; + const name = (await ctx.ui.input('New profile name', 'work or personal'))?.trim(); + if (!name) return undefined; + createProfile(name); return name; + } + const name = picked.delete; + // The profile this session serves is open in this process: deleting the folder under a live store writes into nothing. + if (name === active?.name) { ctx.ui.notify(`Cannot delete ${name}: this session is using it. Switch profiles first.`, 'error'); continue; } + const confirmed = await ctx.ui.confirm(`Delete profile ${name}?`, 'Its memory, runs, usage ledger and settings are removed from disk. Pi sessions are kept.'); + if (!confirmed) continue; + try { await deleteProfile(name); ctx.ui.notify(`Deleted ${name}.`, 'info'); } + catch (error) { ctx.ui.notify(errorText(error), 'error'); } + } }; // An unbound TUI session skips the picker when the choice is clear (#238): /new keeps the window's profile, then OPTCHAT_PROFILE, then a lone profile. const defaultProfile = (ctx: ExtensionContext, event: SessionStartEvent): string | undefined => { diff --git a/src/menu.ts b/src/menu.ts index b48539b0..c6ba7c20 100644 --- a/src/menu.ts +++ b/src/menu.ts @@ -1,4 +1,4 @@ -import { Box, SelectList, Spacer, Text } from '@earendil-works/pi-tui'; +import { Box, KeybindingsManager, SelectList, Spacer, Text, type SelectItem } from '@earendil-works/pi-tui'; import type { ExtensionContext, Theme } from '@earendil-works/pi-coding-agent'; import type { ModelChoice } from './compactor.ts'; import { count, oneLine } from './inspector.ts'; @@ -8,7 +8,7 @@ import { frame, showModel } from './settings-page.ts'; export const ACTIONS = ['profile', 'settings', 'model', 'agents', 'usage', 'activity', 'instructions', 'browse', 'import'] as const; export type MenuAction = typeof ACTIONS[number]; const DESCRIPTIONS: Record = { - profile: 'Switch or create a profile', + profile: 'Switch, create, or delete a profile', settings: 'Models, subagent limits, memory options', model: 'Compactor model and effort', agents: 'Live agent tree and saved runs', @@ -30,27 +30,57 @@ export const statusRows = (s: MenuStatus) => [ const failing = (error: string) => `Summaries failing: ${oneLine(error)}`; /** Framed like the settings page: title, the profile's status as aligned rows, then the actions with what each opens. */ -export function optchatMenu(theme: Theme, title: string, status: MenuStatus | undefined, close: (action?: MenuAction) => void, redraw = () => {}) { - const list = new SelectList(ACTIONS.map(value => ({ value, label: value, description: DESCRIPTIONS[value] })), ACTIONS.length, { +/** The select list and, with it, the j/k movement both menus share: rows styled once, a delete shortcut can pre-empt a key. */ +function sharedList(items: SelectItem[], theme: Theme, close: (value?: string) => void, redraw = () => {}, + shortcut?: (data: string, current: string | undefined) => boolean, maxVisible = 10) { + const list = new SelectList(items, maxVisible, { selectedPrefix: t => theme.fg('accent', t), selectedText: t => theme.fg('accent', t), description: t => theme.fg('muted', t), scrollInfo: t => theme.fg('muted', t), noMatch: t => theme.fg('muted', t) }, - { minPrimaryColumnWidth: 1, maxPrimaryColumnWidth: Math.max(...ACTIONS.map(a => a.length)) + 4 }); - list.onSelect = item => close(ACTIONS.find(a => a === item.value)); + { minPrimaryColumnWidth: 1, maxPrimaryColumnWidth: Math.max(...items.map(i => i.label.length)) + 4 }); + list.onSelect = item => close(item.value); list.onCancel = () => close(); + const block = new Box(1, 0); block.addChild(list); + return { block, handleInput: (data: string) => { + if (!shortcut?.(data, list.getSelectedItem()?.value)) { + // j and k move too, as in Pi's own select dialog that the pages replace. + const step = data === 'j' ? 1 : data === 'k' ? -1 : 0, at = items.findIndex(item => item.value === list.getSelectedItem()?.value); + if (step) list.setSelectedIndex(Math.max(0, Math.min(items.length - 1, at + step))); + else list.handleInput(data); + } + redraw(); + } }; +} + +/** Framed like the settings page: title, the profile's status as aligned rows, then the actions with what each opens. */ +export function optchatMenu(theme: Theme, title: string, status: MenuStatus | undefined, close: (action?: MenuAction) => void, redraw = () => {}) { + const actions = sharedList(ACTIONS.map(value => ({ value, label: value, description: DESCRIPTIONS[value] })), theme, + value => close(ACTIONS.find(a => a === value)), redraw); const rows = status ? statusRows(status) : []; const labels = Math.max(0, ...rows.map(([label]) => label.length)); const facts = status ? rows.map(([label, value]) => new Text(`${theme.fg('muted', label.padEnd(labels))} ${value}`, 1, 0)) : [new Text(theme.fg('muted', 'No active profile'), 1, 0)]; if (status?.error) facts.push(new Text(theme.fg('warning', failing(status.error)), 1, 0)); - const actions = new Box(1, 0); actions.addChild(list); - const page = frame(theme, title, status?.profile, [new Spacer(1), ...facts, new Spacer(1), actions, new Spacer(1), + const page = frame(theme, title, status?.profile, [new Spacer(1), ...facts, new Spacer(1), actions.block, new Spacer(1), new Text(theme.fg('dim', '↑↓ select · Enter open · Esc close'), 1, 0)]); - return Object.assign(page, { handleInput: (data: string) => { - // j and k move too, as in Pi's own select dialog that this menu replaces. - const step = data === 'j' ? 1 : data === 'k' ? -1 : 0, at = ACTIONS.findIndex(a => a === list.getSelectedItem()?.value); - if (step) list.setSelectedIndex(Math.max(0, Math.min(ACTIONS.length - 1, at + step))); - else list.handleInput(data); - redraw(); - } }); + return Object.assign(page, { handleInput: actions.handleInput }); +} + +/** The profile list's own entry for creating one; never a valid profile name (the pattern allows lowercase only). */ +export const CREATE_PROFILE = '+ Create profile'; +/** What the picker returns: a profile to open, a profile to delete, or nothing for a cancel. */ +export type ProfilePick = string | { delete: string } | undefined; + +/** The profile list: Pi's session list for profiles, so Ctrl+D deletes the highlighted one the way it deletes a session there. */ +export function profilePicker(theme: Theme, keys: KeybindingsManager, names: string[], close: (pick?: ProfilePick) => void, redraw = () => {}) { + const items = [...names.map(name => ({ value: name, label: name })), { value: CREATE_PROFILE, label: CREATE_PROFILE }]; + const picker = sharedList(items, theme, value => close(value as ProfilePick), redraw, (data, current) => { + // Delete goes through the user's keybindings, so it fires in every terminal encoding Pi reads; the create row is not a profile to delete. + if (!keys.matches(data, 'app.session.delete')) return false; + if (current && current !== CREATE_PROFILE) close({ delete: current }); + return true; + }); + const page = frame(theme, 'OptChat profile', undefined, [new Spacer(1), picker.block, new Spacer(1), + new Text(theme.fg('dim', names.length ? '↑↓ select · Enter open · Ctrl+D delete · Esc cancel' : 'Enter create · Esc cancel'), 1, 0)]); + return Object.assign(page, { handleInput: picker.handleInput }); } /** The menu in interactive Pi; elsewhere (RPC hosts) Pi's plain select with the same facts and actions. */ diff --git a/src/profiles.ts b/src/profiles.ts index 752067f5..ddd9fffc 100644 --- a/src/profiles.ts +++ b/src/profiles.ts @@ -1,8 +1,8 @@ import { createHash } from 'node:crypto'; -import { chmodSync, existsSync, lstatSync, mkdirSync, readFileSync, readdirSync, realpathSync, rmSync, statSync, unlinkSync, writeFileSync, type Stats } from 'node:fs'; +import { chmodSync, existsSync, lstatSync, mkdirSync, readFileSync, readdirSync, realpathSync, renameSync, rmSync, statSync, unlinkSync, writeFileSync, type Stats } from 'node:fs'; +import { basename, dirname, join, resolve } from 'node:path'; import { createConnection, createServer } from 'node:net'; import { homedir } from 'node:os'; -import { join, resolve } from 'node:path'; import type { ModelChoice } from './compactor.ts'; import { record } from './core/cache.ts'; import { atomicWrite } from './store.ts'; @@ -29,6 +29,25 @@ export function listProfiles() { const root = join(dataHome(), 'profiles'); return existsSync(root) ? readdirSync(root, { withFileTypes: true }).filter(f => f.isDirectory() && /^[a-z0-9][a-z0-9_-]{0,63}$/.test(f.name)).map(f => f.name).sort() : []; } +/** Removes a profile's folder, and the saved last-profile pointer when it names it. The real lock decides: a profile another Pi is using is open, so deleting it would leave that Pi writing into a folder that no longer exists. */ +export async function deleteProfile(name: string) { + const dir = profilePath(name); + if (!existsSync(dir)) throw new Error(`Profile ${name} does not exist.`); + const unlock = await lockProfile(dir, 'deleting'); + try { + // Renamed to a hidden sibling before the rmSync: a delete that fails on Windows then cannot leave half a profile in the picker (listProfiles() hides dot names). + const reserved = join(dataHome(), 'profiles', `.deleting-${name}-${Date.now()}`); + renameSync(dir, reserved); + // listProfiles() already hides a name with no folder, but without this a profile created again under the same name would inherit the stale pointer. + // Clear it immediately after the rename, before the recursive removal, so an rmSync failure cannot leave a stale pointer behind. + const last = join(dataHome(), 'last-profile'); + try { if (readFileSync(last, 'utf8').trim() === name) unlinkSync(last); } + catch (error) { if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; } + rmSync(reserved, { recursive: true, force: true, maxRetries: 3 }); + return dir; + } finally { await unlock(); } +} + export function createProfile(name: string) { const dir = profilePath(name); if (existsSync(dir)) throw new Error(`Profile already exists: ${name}`); @@ -67,9 +86,17 @@ export function rememberProfile(name: string) { atomicWrite(join(dataHome(), 'la export class ProfileBusyError extends Error { constructor(readonly owner: string) { super(`Profile already running: ${owner}`); } } -/** POSIX keeps a socket file in the profile itself, so every Pi on the profile finds the same one whatever its TMPDIR, and Git skips it. Windows has no Unix sockets, so the profile path is hashed into a named pipe in the machine-wide pipe namespace. */ +/** + * The lock lives outside the profile directory so a deletion can rename the profile away without moving the socket. + * An old server's close therefore cannot unlink a replacement owner's socket at the same path. + * POSIX keeps the lock file under a hidden per-profile directory, so every Pi on the profile finds the same one whatever its TMPDIR, and Git skips it. + * Windows has no Unix sockets, so the profile path is hashed into a named pipe in the machine-wide pipe namespace. + */ export function profileSocket(dir: string, purpose: 'lock' | 'windows' = 'lock') { - if (!isWindows) return join(dir, `${purpose}.sock`); + if (!isWindows) { + if (purpose === 'lock') return join(dataHome(), '.locks', basename(dir), 'lock.sock'); + return join(dir, `${purpose}.sock`); + } // Hash the directory's real identity, so another casing, a junction or a symlink to the same profile finds the same pipe. let identity = resolve(dir); try { identity = realpathSync.native(identity); } catch {} return `${PIPE_PREFIX}optchat-${createHash('sha256').update(identity.toLowerCase()).digest('hex').slice(0, 16)}-${purpose}`; @@ -106,6 +133,7 @@ function readOwner(path: string) { /** OS-owned socket lifetime, no timeout-based stealing of a busy profile. */ export async function lockProfile(dir: string, description: string) { const socketPath = profileSocket(dir); checkSocketPath(socketPath); + if (!isWindows) mkdirSync(dirname(socketPath), { recursive: true }); const server = createServer(socket => { socket.on('error', () => socket.destroy()); socket.end(description); }); const listen = () => new Promise((resolve, reject) => { const failed = (error: Error) => { server.off('listening', ready); reject(error); }; diff --git a/test/menu.test.ts b/test/menu.test.ts index 0de7a2af..e0fc3c8f 100644 --- a/test/menu.test.ts +++ b/test/menu.test.ts @@ -1,11 +1,14 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import type { Theme } from '@earendil-works/pi-coding-agent'; -import { optchatMenu, type MenuAction } from '../src/menu.ts'; +import { KeybindingsManager } from '@earendil-works/pi-tui'; +import { CREATE_PROFILE, optchatMenu, profilePicker, type MenuAction, type ProfilePick } from '../src/menu.ts'; import { defaults } from '../src/profiles.ts'; const plain = { fg: (_color: string, text: string) => text, bold: (text: string) => text } as unknown as Theme; const status = { profile: 'personal', messages: 5251, pending: 1, compactor: defaults.compactor, agents: defaults.subagent, error: 'Rate limited' }; +/** The real matcher, pointing the delete keybinding at Pi's default for the session list. */ +const keys = new KeybindingsManager({ 'app.session.delete': { defaultKeys: 'ctrl+d' } } as never); test('/optchat shows the profile status as aligned rows and returns the chosen action', () => { const chosen: (MenuAction | undefined)[] = []; @@ -25,3 +28,26 @@ test('/optchat without a profile says so and keeps every action', () => { assert.ok(lines.includes(' OptChat') && lines.includes(' No active profile')); assert.ok(lines.some(line => line.includes('import') && line.includes('Import history'))); }); + +test('/optchat profile lists profiles, and its keys decide delete, create and cancel', () => { + const picks: ProfilePick[] = []; + const picker = profilePicker(plain, keys, ['only'], pick => picks.push(pick)); + const lines = picker.render(80).map(line => line.trimEnd()); + assert.ok(lines.some(line => line.includes('OptChat profile'))); + assert.ok(lines.some(line => line.includes('Ctrl+D delete'))); + assert.ok(lines.some(line => line.includes(CREATE_PROFILE))); + picker.handleInput('j'); // now on the create row + picker.handleInput('\x04'); + assert.deepEqual(picks, [], 'the create row is not a profile to delete'); + picker.handleInput('k'); // back on the profile row + picker.handleInput('\x04'); + assert.deepEqual(picks, [{ delete: 'only' }]); + // A list with no profiles still offers create; Enter opens the row, Esc cancels. + const fresh: ProfilePick[] = []; + const empty = profilePicker(plain, keys, [], pick => fresh.push(pick)); + assert.ok(empty.render(80).some(line => line.includes('Enter create'))); + empty.handleInput('j'); empty.handleInput('\r'); + assert.deepEqual(fresh, [CREATE_PROFILE]); + empty.handleInput('\x1b'); + assert.deepEqual(fresh, [CREATE_PROFILE, undefined]); +}); diff --git a/test/profile-busy.test.ts b/test/profile-busy.test.ts index b31f00e4..14435661 100644 --- a/test/profile-busy.test.ts +++ b/test/profile-busy.test.ts @@ -1,14 +1,14 @@ import { after, test } from 'node:test'; import assert from 'node:assert/strict'; -import { mkdirSync, mkdtempSync, readdirSync, readFileSync, symlinkSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, symlinkSync, writeFileSync } from 'node:fs'; import { createConnection } from 'node:net'; import { basename, join } from 'node:path'; import { tmpdir } from 'node:os'; import { createAgentSession, DefaultResourceLoader, ModelRuntime, SessionManager, SettingsManager, type ExtensionUIContext, type SessionStartEvent } from '@earendil-works/pi-coding-agent'; import optchat from '../src/index.ts'; import { Memory } from '../src/memory.ts'; -import { checkSocketPath, createProfile, isWindows, loadConfig, lockProfile, profilePath, profileSocket, saveConfig, SOCKET_PATH_LIMIT } from '../src/profiles.ts'; -import { removeDir } from './support.ts'; +import { checkSocketPath, createProfile, isWindows, lastProfile, listProfiles, loadConfig, lockProfile, profilePath, profileSocket, rememberProfile, saveConfig, SOCKET_PATH_LIMIT } from '../src/profiles.ts'; +import { profilePickerAnswers, removeDir } from './support.ts'; const agentDir = process.env.PI_CODING_AGENT_DIR = mkdtempSync(join(tmpdir(), 'optchat-agent-')); after(() => removeDir(agentDir)); @@ -55,21 +55,23 @@ test('a busy profile offers to connect or pick another profile, and picking anot unlock = await lockProfile(profilePath('busy'), 'busy · PID 1 · elsewhere'); // New session: pick `busy`, it's taken, choose "Back", then pick `other`. - const asked: { title: string, options: string[] }[] = []; - const picks = ['busy', 'Back', 'other']; - const fresh = await start(dir, { select: async (title, options) => { asked.push({ title, options }); return picks.shift(); } }); + const asked: { title: string, options: string[] }[] = [], shown: string[][] = []; + const fresh = await start(dir, { + select: async (title, options) => { asked.push({ title, options }); return 'Back'; }, + custom: profilePickerAnswers(['busy', 'other'], ['busy', 'other', 'third'], shown), + }); sessions.push(fresh.session); - assert.deepEqual(asked.map(a => a.title.split('\n')[0]), ['OptChat profile', 'busy is open in another window', 'OptChat profile']); - assert.match(asked[1].title, /busy · PID 1 · elsewhere/); - assert.deepEqual(asked[1].options, ['Start a connected subagent conversation here', 'Back']); + assert.deepEqual(shown.map(lines => lines.find(line => line.includes('OptChat profile'))?.trim()), ['OptChat profile', 'OptChat profile']); + assert.deepEqual(asked.map(a => a.title.split('\n')[0]), ['busy is open in another window']); + assert.match(asked[0].title, /busy · PID 1 · elsewhere/); + assert.deepEqual(asked[0].options, ['Start a connected subagent conversation here', 'Back']); assert.equal(fresh.titles[0], 'π other'); assert.deepEqual(fresh.errors, []); const bound = fresh.manager.getEntries().filter(e => e.type === 'custom' && e.customType === 'optchat.profile'); assert.deepEqual(bound.map(e => e.type === 'custom' && e.data), [{ name: 'other' }], 'the session is bound to the profile actually opened'); // `/optchat profile` makes a fresh session already bound to its pick; a busy pick can still go Back and rebind. - const switchedPicks = ['Back', 'third']; // `other` is held by the session above - const switched = await start(dir, { select: async () => switchedPicks.shift() }, 'busy'); + const switched = await start(dir, { select: async () => 'Back', custom: profilePickerAnswers(['third'], ['busy', 'other', 'third']) }, 'busy'); sessions.push(switched.session); assert.equal(switched.titles[0], 'π third'); assert.deepEqual(switched.errors, []); @@ -98,9 +100,10 @@ test('a connected window whose original window closed can leave: /new, /resume, try { createProfile('busy'); owner = (await start(dir, {}, 'busy')).session; - const notes: string[] = [], asked: string[] = []; - const picks = ['Start a connected subagent conversation here', undefined]; - window = (await start(dir, { notify: text => { notes.push(text); }, select: async title => { asked.push(title.split('\n')[0]); return picks.shift(); } }, 'busy')).session; + const notes: string[] = [], asked: string[] = [], shown: string[][] = []; + window = (await start(dir, { notify: text => { notes.push(text); }, + select: async title => { asked.push(title.split('\n')[0]); return 'Start a connected subagent conversation here'; }, + custom: profilePickerAnswers([undefined], ['busy'], shown) }, 'busy')).session; const switchBlocked = async () => (await window!.extensionRunner.emit({ type: 'session_before_switch', reason: 'new' }))?.cancel === true; const forkBlocked = async () => (await window!.extensionRunner.emit({ type: 'session_before_fork', entryId: 'x', position: 'at' }))?.cancel === true; assert.equal(await switchBlocked(), true, 'a live connected conversation keeps its window'); @@ -111,7 +114,8 @@ test('a connected window whose original window closed can leave: /new, /resume, assert.equal(await switchBlocked(), false, '/new and /resume are no longer cancelled'); assert.equal(await forkBlocked(), false, '/fork and /clone are no longer cancelled'); await window.prompt('/optchat profile'); - assert.deepEqual(asked, ['busy is open in another window', 'OptChat profile'], '/optchat profile offers the profile picker'); + assert.deepEqual(asked, ['busy is open in another window']); + assert.deepEqual(shown.map(lines => lines.find(line => line.includes('OptChat profile'))?.trim()), ['OptChat profile'], '/optchat profile offers the profile picker'); assert.ok(!notes.some(n => n.includes('original window')), 'no pointer to a window that is gone'); } finally { for (const session of [owner, window]) if (session) { await session.extensionRunner.emit({ type: 'session_shutdown', reason: 'quit' }); session.dispose(); } @@ -212,7 +216,7 @@ test('a second writer that got past the lock is refused on its next turn, with t try { createProfile('shared'); saveConfig(profilePath('shared'), { ...loadConfig(profilePath('shared')), compactor: { provider: 'fixture', model: 'fixture', thinking: 'off' }, subagent: { provider: 'fixture', model: 'fixture', thinking: 'off' } }); - const opened = await start(dir, { select: async () => 'shared' }); + const opened = await start(dir, { custom: profilePickerAnswers(['shared'], ['shared']) }); session = opened.session; const other = new Memory(profilePath('shared'), async () => 'summary', () => {}); other.append('user', 'written by the other process'); await other.close(); @@ -269,12 +273,12 @@ test('OPTCHAT_PROFILE picks among several profiles, and a missing one warns and assert.equal(chosen.titles[0], 'π personal'); process.env.OPTCHAT_PROFILE = 'nope'; - const warnings: string[] = [], asked: string[] = []; + const warnings: string[] = [], shown: string[][] = []; const fallback = await start(dir, { notify: (text, type) => { if (type === 'warning') warnings.push(text); }, - select: async title => { asked.push(title); return 'work'; } }); + custom: profilePickerAnswers(['work'], ['personal', 'work'], shown) }); sessions.push(fallback.session); assert.deepEqual(warnings, ['OPTCHAT_PROFILE: no profile named nope.']); - assert.deepEqual(asked, ['OptChat profile']); + assert.ok(shown.flat().some(line => line.includes('OptChat profile')), 'the fallback shows the picker'); assert.equal(fallback.titles[0], 'π work'); })); @@ -282,9 +286,68 @@ test('Back from a lone busy profile shows the picker instead of retrying it', () createProfile('busy'); const unlock = await lockProfile(profilePath('busy'), 'busy · PID 1 · elsewhere'); try { - const asked: string[] = [], picks = ['Back', undefined]; - const { session } = await start(dir, { select: async title => { asked.push(title.split('\n')[0]); return picks.shift(); } }); + const asked: string[] = [], shown: string[][] = []; + const { session } = await start(dir, { + select: async title => { + if (asked.length) assert.fail(`the busy dialog came back: ${title}`); + asked.push(title.split('\n')[0]); return 'Back'; + }, + custom: profilePickerAnswers([undefined], ['busy'], shown) }); sessions.push(session); - assert.deepEqual(asked, ['busy is open in another window', 'OptChat profile']); + assert.deepEqual(asked, ['busy is open in another window'], 'busy is offered to connect or decline'); + assert.ok(shown.flat().some(line => line.includes('OptChat profile')), 'the picker is shown, not a busy retry'); } finally { await unlock(); } })); + +test('Ctrl+D in the profile picker deletes the profile once it is confirmed', async () => { + const dir = mkdtempSync(join(tmpdir(), 'optchat-delete-')), oldHome = process.env.OPTCHAT_HOME; + process.env.OPTCHAT_HOME = join(dir, 'home'); + let session: Awaited>['session'] | undefined; + try { + createProfile('keep'); createProfile('gone'); + rememberProfile('gone'); + const asked: string[][] = [], shown: string[][] = []; + const opened = await start(dir, { + confirm: async (title, message) => { asked.push([title, message]); return true; }, + custom: profilePickerAnswers([{ delete: 'gone' }, undefined], ['gone', 'keep'], shown), + }); + session = opened.session; + const shownLines = shown.flat(); + assert.ok(shownLines.some(line => line.includes('gone')), 'the doomed profile was on screen'); + assert.deepEqual(asked.map(([title]) => title), ['Delete profile gone?']); + assert.match(asked[0][1], /removed from disk\. Pi sessions are kept/); + assert.deepEqual(listProfiles(), ['keep']); + assert.equal(existsSync(profilePath('gone')), false, 'the folder is gone'); + assert.equal(lastProfile(), undefined, 'and so is the pointer that named it'); + assert.equal(opened.errors.length, 0, opened.errors.join('\n')); + } finally { + if (session) { await session.extensionRunner.emit({ type: 'session_shutdown', reason: 'quit' }); session.dispose(); } + if (oldHome === undefined) delete process.env.OPTCHAT_HOME; else process.env.OPTCHAT_HOME = oldHome; + removeDir(dir); + } +}); + +test('a declined delete keeps the profile and the picker comes back', async () => { + const dir = mkdtempSync(join(tmpdir(), 'optchat-delete-')), oldHome = process.env.OPTCHAT_HOME; + process.env.OPTCHAT_HOME = join(dir, 'home'); + let session: Awaited>['session'] | undefined; + try { + createProfile('keep'); createProfile('safe'); + const asked: string[] = [], shown: string[][] = []; + const opened = await start(dir, { + confirm: async title => { asked.push(title); return false; }, + // Declined once, then the second opening takes the profile instead. + custom: profilePickerAnswers([{ delete: 'safe' }, 'safe'], ['keep', 'safe'], shown), + }); + session = opened.session; + assert.deepEqual(asked, ['Delete profile safe?']); + assert.equal(existsSync(profilePath('safe')), true); + assert.equal(shown.length, 2, 'the picker reopened after the refusal'); + assert.deepEqual(opened.errors, []); + } finally { + if (session) { await session.extensionRunner.emit({ type: 'session_shutdown', reason: 'quit' }); session.dispose(); } + if (oldHome === undefined) delete process.env.OPTCHAT_HOME; else process.env.OPTCHAT_HOME = oldHome; + removeDir(dir); + } +}); + diff --git a/test/profile-delete.test.ts b/test/profile-delete.test.ts new file mode 100644 index 00000000..c82a06be --- /dev/null +++ b/test/profile-delete.test.ts @@ -0,0 +1,75 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import { mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createProfile, deleteProfile, lastProfile, listProfiles, lockProfile, profilePath, ProfileBusyError, rememberProfile } from '../src/profiles.ts'; +import { removeDir } from './support.ts'; + +function sandbox() { + const dir = mkdtempSync(join(tmpdir(), 'oc-delete-')), previous = process.env.OPTCHAT_HOME; + process.env.OPTCHAT_HOME = dir; + return () => { + if (previous === undefined) delete process.env.OPTCHAT_HOME; else process.env.OPTCHAT_HOME = previous; + removeDir(dir); + }; +} + +test('deleting a profile removes its folder and the last-profile pointer that named it', async () => { + const cleanup = sandbox(); + try { + createProfile('work'); createProfile('home'); + rememberProfile('work'); + assert.equal(lastProfile(), 'work'); + assert.equal(await deleteProfile('work'), profilePath('work')); + assert.equal(fs.existsSync(profilePath('work')), false); + assert.deepEqual(listProfiles(), ['home']); + assert.equal(lastProfile(), undefined); + // Recreated under the same name, it is not the last profile on the strength of the old pointer. + createProfile('work'); + assert.equal(lastProfile(), undefined, 'the stale pointer is gone even if the profile name is reused'); + } finally { cleanup(); } +}); + +test('deleting one profile keeps the last-profile pointer that names another', async () => { + const cleanup = sandbox(); + try { + createProfile('work'); createProfile('home'); + rememberProfile('home'); + await deleteProfile('work'); + assert.equal(lastProfile(), 'home'); + } finally { cleanup(); } +}); + +test('a profile another Pi is holding is refused and keeps every file', async () => { + const cleanup = sandbox(); + try { + createProfile('busy'); + fs.writeFileSync(join(profilePath('busy'), 'main.jsonl'), 'my memory'); + const unlock = await lockProfile(profilePath('busy'), 'busy · PID 1234 · this machine'); + try { + await assert.rejects(deleteProfile('busy'), error => error instanceof ProfileBusyError); + } finally { await unlock(); } + assert.equal(fs.readFileSync(join(profilePath('busy'), 'main.jsonl'), 'utf8'), 'my memory'); + // The refusal is about the holder, not the profile: once it is released the delete goes through. + await deleteProfile('busy'); + assert.equal(fs.existsSync(profilePath('busy')), false); + } finally { cleanup(); } +}); + +test('a profile that is not there is refused by name', async () => { + const cleanup = sandbox(); + try { + await assert.rejects(deleteProfile('missing'), /Profile missing does not exist/); + } finally { cleanup(); } +}); + +test('a name that is not a profile name is refused before anything is removed', async () => { + const cleanup = sandbox(); + try { + createProfile('work'); + await assert.rejects(deleteProfile('../work'), /Profile names/); + assert.deepEqual(listProfiles(), ['work']); + } finally { cleanup(); } +}); diff --git a/test/support.ts b/test/support.ts index d0527402..58af5297 100644 --- a/test/support.ts +++ b/test/support.ts @@ -1,6 +1,8 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { basename, join } from 'node:path'; +import { KeybindingsManager } from '@earendil-works/pi-tui'; +import type { ExtensionUIContext, Theme } from '@earendil-works/pi-coding-agent'; // Every temp dir made through os.tmpdir() lands in this root, removed at exit. It also takes what a test's own cleanup misses: Pi rewrites auth.json after a test removed its dir, and a worker thread loads this file again but is terminated before 'exit'. // macOS's default tmpdir is ~49 chars, and the extra level would push profile lock-socket paths past the 103-byte limit, so the outermost root goes under /tmp there. A child process or worker thread nests inside its parent's root, so it goes away with it even if it never reaches 'exit'. @@ -22,4 +24,31 @@ export function removeDir(path: string) { } } +/** A theme that keeps text as written, so a rendered line reads as the user sees it without colors. */ +export const plainTheme = { fg: (_color: string, text: string) => text, bold: (text: string) => text } as unknown as Theme; + +/** What the profile picker shows and what it is answered with, one answer per opening. */ +export type PickerAnswer = string | { delete: string } | undefined; + +/** + * Answers OptChat's profile picker the way a person would: renders it once to record what was shown, walks to the + * named profile with `j`, then presses Enter, Ctrl+D or Escape. `names` is the order the picker was given, so a + * delete answer is walked to before Ctrl+D. Delete goes through Pi's real matcher, so whatever a terminal sends for + * Ctrl+D reaches the picker the same way it does in play. + */ +export function profilePickerAnswers(answers: PickerAnswer[], names: string[], shown: string[][] = []): ExtensionUIContext['custom'] { + const keys = new KeybindingsManager({ 'app.session.delete': { defaultKeys: 'ctrl+d' } }); + return (async (factory: (tui: { requestRender: () => void }, theme: Theme, keys: unknown, done: (value: T) => void) => { render: (width: number) => string[], handleInput: (data: string) => void }) => + new Promise((resolve, reject) => { + const page = factory({ requestRender: () => {} }, plainTheme, keys, value => resolve(value)); + shown.push(page.render(80).map(line => line.trimEnd())); + const queued = answers.shift() as PickerAnswer | undefined; + if (queued === undefined) { page.handleInput('\x1b'); return; } + const wanted = typeof queued === 'string' ? queued : queued.delete, at = names.indexOf(wanted); + if (at === -1) { reject(new Error(`${wanted} is not one of the profiles the picker was given`)); return; } + for (let step = 0; step < at; step++) page.handleInput('j'); + page.handleInput(typeof queued === 'string' ? '\r' : '\x04'); + })) as unknown as ExtensionUIContext['custom']; +} + process.on('exit', () => { try { rmSync(root, { recursive: true, force: true }); } catch { /* EBUSY on Windows: leave it */ } });