diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 838ff71d2b..7a597509fb 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -2772,6 +2772,129 @@ def test_session_list_returns_clean_error_when_state_db_is_unavailable(monkeypat assert "state.db unavailable: locking protocol" in resp["error"]["message"] +# -------------------------------------------------------------------------- +# session.delete — TUI resume picker `d` key +# -------------------------------------------------------------------------- + + +def test_session_delete_requires_session_id(monkeypatch): + """Empty / missing session_id is a 4006 client error (no DB call).""" + called: list[tuple] = [] + + class _DB: + def delete_session(self, *a, **kw): + called.append((a, kw)) + return True + + monkeypatch.setattr(server, "_get_db", lambda: _DB()) + + resp = server.handle_request({"id": "1", "method": "session.delete", "params": {}}) + assert "error" in resp + assert resp["error"]["code"] == 4006 + assert called == [] + + +def test_session_delete_returns_db_unavailable_when_no_db(monkeypatch): + monkeypatch.setattr(server, "_get_db", lambda: None) + monkeypatch.setattr(server, "_db_error", "locked") + + resp = server.handle_request( + {"id": "1", "method": "session.delete", "params": {"session_id": "abc"}} + ) + + assert "error" in resp + assert resp["error"]["code"] == 5036 + assert "state.db unavailable" in resp["error"]["message"] + + +def test_session_delete_refuses_active_session(monkeypatch): + """Cannot delete a session currently bound to a live TUI session.""" + called: list[str] = [] + + class _DB: + def delete_session(self, sid, sessions_dir=None): + called.append(sid) + return True + + monkeypatch.setattr(server, "_get_db", lambda: _DB()) + monkeypatch.setitem(server._sessions, "live", {"session_key": "key-live"}) + try: + resp = server.handle_request( + { + "id": "1", + "method": "session.delete", + "params": {"session_id": "key-live"}, + } + ) + finally: + server._sessions.pop("live", None) + + assert "error" in resp + assert resp["error"]["code"] == 4023 + assert "active session" in resp["error"]["message"] + assert called == [], "delete_session must not be called for active sessions" + + +def test_session_delete_returns_4007_when_missing(monkeypatch): + class _DB: + def delete_session(self, sid, sessions_dir=None): + return False + + monkeypatch.setattr(server, "_get_db", lambda: _DB()) + + resp = server.handle_request( + {"id": "1", "method": "session.delete", "params": {"session_id": "ghost"}} + ) + + assert "error" in resp + assert resp["error"]["code"] == 4007 + + +def test_session_delete_propagates_db_exception(monkeypatch): + class _DB: + def delete_session(self, sid, sessions_dir=None): + raise RuntimeError("disk full") + + monkeypatch.setattr(server, "_get_db", lambda: _DB()) + + resp = server.handle_request( + {"id": "1", "method": "session.delete", "params": {"session_id": "x"}} + ) + + assert "error" in resp + assert resp["error"]["code"] == 5036 + assert "disk full" in resp["error"]["message"] + + +def test_session_delete_success_returns_deleted_id(monkeypatch): + """Happy path — DB delete succeeds, response carries the deleted id + and the on-disk sessions dir is forwarded so transcript files get + cleaned up alongside the row.""" + captured: dict = {} + + class _DB: + def delete_session(self, sid, sessions_dir=None): + captured["sid"] = sid + captured["sessions_dir"] = sessions_dir + return True + + monkeypatch.setattr(server, "_get_db", lambda: _DB()) + + resp = server.handle_request( + {"id": "1", "method": "session.delete", "params": {"session_id": "old-1"}} + ) + + assert "result" in resp, resp + assert resp["result"] == {"deleted": "old-1"} + assert captured["sid"] == "old-1" + # sessions_dir must be forwarded so transcript files get cleaned up + # too — not just the SQLite row. The autouse _isolate_hermes_home + # fixture pins HERMES_HOME to a temp dir; the handler should append + # /sessions to it. + assert captured["sessions_dir"] is not None + assert str(captured["sessions_dir"]).endswith("sessions") + + # -------------------------------------------------------------------------- # model.options — curated-list parity with `hermes model` and classic /model # -------------------------------------------------------------------------- diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 667980d34b..7ffe834cba 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -2093,6 +2093,42 @@ def _(rid, params: dict) -> dict: ) +@method("session.delete") +def _(rid, params: dict) -> dict: + """Delete a stored session and its on-disk transcript files. + + Used by the TUI resume picker (``d`` key) so users can prune old + sessions without dropping to the CLI. Refuses to delete a session + that is currently active in this gateway process — those rows are + still being written to and removing them out from under the live + agent corrupts message ordering and trips FK constraints when the + next message append flushes. + """ + target = params.get("session_id", "") + if not target: + return _err(rid, 4006, "session_id required") + db = _get_db() + if db is None: + return _db_unavailable_error(rid, code=5036) + # Block deletion of any session currently bound to a live TUI session + # in this process. The picker hides the active session anyway, but a + # racing caller could still target it. + try: + active = {s.get("session_key") for s in _sessions.values() if s.get("session_key")} + except Exception: + active = set() + if target in active: + return _err(rid, 4023, "cannot delete an active session") + sessions_dir = get_hermes_home() / "sessions" + try: + deleted = db.delete_session(target, sessions_dir=sessions_dir) + except Exception as e: + return _err(rid, 5036, f"delete failed: {e}") + if not deleted: + return _err(rid, 4007, "session not found") + return _ok(rid, {"deleted": target}) + + @method("session.title") def _(rid, params: dict) -> dict: session, err = _sess_nowait(params, rid) diff --git a/ui-tui/src/components/sessionPicker.tsx b/ui-tui/src/components/sessionPicker.tsx index fd29d9e7ec..8399d39cd5 100644 --- a/ui-tui/src/components/sessionPicker.tsx +++ b/ui-tui/src/components/sessionPicker.tsx @@ -2,7 +2,7 @@ import { Box, Text, useInput, useStdout } from '@hermes/ink' import { useEffect, useState } from 'react' import type { GatewayClient } from '../gatewayClient.js' -import type { SessionListItem, SessionListResponse } from '../gatewayTypes.js' +import type { SessionDeleteResponse, SessionListItem, SessionListResponse } from '../gatewayTypes.js' import { asRpcResult, rpcErrorMessage } from '../lib/rpc.js' import type { Theme } from '../theme.js' @@ -31,6 +31,10 @@ export function SessionPicker({ gw, onCancel, onSelect, t }: SessionPickerProps) const [err, setErr] = useState('') const [sel, setSel] = useState(0) const [loading, setLoading] = useState(true) + // When non-null, the user pressed `d` on this index and we're waiting for + // `y`/`Y` to confirm deletion. Any other key cancels the prompt. + const [confirmDelete, setConfirmDelete] = useState(null) + const [deleting, setDeleting] = useState(false) const { stdout } = useStdout() const width = Math.max(MIN_WIDTH, Math.min(MAX_WIDTH, (stdout?.columns ?? 80) - 6)) @@ -59,7 +63,57 @@ export function SessionPicker({ gw, onCancel, onSelect, t }: SessionPickerProps) }) }, [gw]) + const performDelete = (index: number) => { + const target = items[index] + + if (!target || deleting) { + return + } + + setDeleting(true) + gw.request('session.delete', { session_id: target.id }) + .then(raw => { + const r = asRpcResult(raw) + + if (!r || r.deleted !== target.id) { + setErr('invalid response: session.delete') + setDeleting(false) + + return + } + + setItems(prev => { + const next = prev.filter((_, i) => i !== index) + setSel(s => Math.max(0, Math.min(s, next.length - 1))) + + return next + }) + setErr('') + setDeleting(false) + }) + .catch((e: unknown) => { + setErr(rpcErrorMessage(e)) + setDeleting(false) + }) + } + useInput((ch, key) => { + if (deleting) { + return + } + + if (confirmDelete !== null) { + if (ch === 'y' || ch === 'Y') { + const idx = confirmDelete + setConfirmDelete(null) + performDelete(idx) + } else { + setConfirmDelete(null) + } + + return + } + if (key.upArrow && sel > 0) { setSel(s => s - 1) } @@ -70,6 +124,14 @@ export function SessionPicker({ gw, onCancel, onSelect, t }: SessionPickerProps) if (key.return && items[sel]) { onSelect(items[sel]!.id) + + return + } + + if ((ch === 'd' || ch === 'D') && items[sel]) { + setConfirmDelete(sel) + + return } const n = parseInt(ch) @@ -83,7 +145,7 @@ export function SessionPicker({ gw, onCancel, onSelect, t }: SessionPickerProps) return loading sessions… } - if (err) { + if (err && !items.length) { return ( error: {err} @@ -109,11 +171,12 @@ export function SessionPicker({ gw, onCancel, onSelect, t }: SessionPickerProps) Resume Session - {offset > 0 && ↑ {offset} more} + {offset > 0 && ↑ {offset} more} {items.slice(offset, offset + VISIBLE).map((s, vi) => { const i = offset + vi const selected = sel === i + const pendingDelete = confirmDelete === i return ( @@ -135,18 +198,23 @@ export function SessionPicker({ gw, onCancel, onSelect, t }: SessionPickerProps) - {s.title || s.preview || '(untitled)'} + {pendingDelete ? 'delete? y/n' : s.title || s.preview || '(untitled)'} ) })} - {offset + VISIBLE < items.length && ↓ {items.length - offset - VISIBLE} more} - ↑/↓ select · Enter resume · 1-9 quick · Esc/q cancel + {offset + VISIBLE < items.length && ↓ {items.length - offset - VISIBLE} more} + {err && error: {err}} + {deleting ? ( + deleting… + ) : ( + ↑/↓ select · Enter resume · 1-9 quick · d delete · Esc/q cancel + )} ) } diff --git a/ui-tui/src/gatewayTypes.ts b/ui-tui/src/gatewayTypes.ts index 19f36afb2f..dfa84af9c7 100644 --- a/ui-tui/src/gatewayTypes.ts +++ b/ui-tui/src/gatewayTypes.ts @@ -129,6 +129,10 @@ export interface SessionListResponse { sessions?: SessionListItem[] } +export interface SessionDeleteResponse { + deleted: string +} + export interface SessionMostRecentResponse { session_id?: null | string source?: string