Skip to content

fix(tui): support /quit --delete to remove session on exit - #74925

Closed
zjin1126 wants to merge 1 commit into
NousResearch:mainfrom
zjin1126:fix/tui-quit-delete
Closed

zjin1126 wants to merge 1 commit into
NousResearch:mainfrom
zjin1126:fix/tui-quit-delete

Conversation

@zjin1126

Copy link
Copy Markdown

What does this PR do?

The TUI's /quit command silently ignores the --delete flag that the CLI has supported since #27101. In the classic CLI, /exit --delete parses the flag, arms _delete_session_on_exit, and the shutdown path calls SessionDB.delete_session(). In the TUI, the Ink frontend handler (core.ts) receives the argument as _arg (underscore-prefixed = intentionally unused) and calls ctx.session.die() unconditionally — the flag never reaches any deletion logic.

This PR ports the CLI's /exit --delete behavior to the TUI:

  1. New gateway RPC session.exit (tui_gateway/methods_session.py) — deletes the session's SQLite rows and on-disk transcripts, mirroring the CLI shutdown path (cli.py:17317). Unlike session.delete (used by the resume picker), it intentionally bypasses the active-session guard because the caller is about to terminate the process.

  2. Frontend --delete parsing (ui-tui/src/app/slash/commands/core.ts) — the /quit handler now reads its argument, accepts --delete / -d, rejects unknown flags with a usage hint, and calls session.exit before ctx.session.die(). Best-effort: .finally() ensures the TUI exits regardless of the RPC outcome, matching the CLI's try/except around delete_session.

  3. Docs — updates slash-commands.md to reflect that /quit --delete works in both surfaces, and removes /quit from the CLI-only list (it was already handled by the TUI frontend; the listing was inaccurate).

Related Issue

Ports #27101 (feat(cli): add /exit --delete flag) to the TUI surface. Related: #56280 (in-TUI session management).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tui_gateway/methods_session.py (+36): new session.exit RPC method
  • ui-tui/src/app/slash/commands/core.ts (+20, −2): parse --delete/-d in /quit handler, call session.exit before exit
  • tests/test_tui_gateway_server.py (+141): 7 tests for session.exit (mirrors the existing session.delete test structure)
  • website/docs/reference/slash-commands.md (+3, −3): /quit description, destructive commands table, CLI-only notes

How to Test

  1. Start the TUI: hermes
  2. Send a message so the session has content
  3. Run /quit --delete
  4. Verify the session is gone: hermes sessions list should not show it, and ~/.hermes/sessions/<id>* files should be deleted
  5. Verify plain /quit still preserves the session
  6. Verify /quit --foo prints usage: /quit [--delete] and does not exit

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.6

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — N/A (gateway RPC + Ink frontend, no platform-specific code)
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

$ pytest tests/test_tui_gateway_server.py -q -k "session_exit"
....... 7 passed, 496 deselected in 0.52s

$ pytest tests/test_tui_gateway_server.py -q
503 passed in 10.63s

$ npx tsc --noEmit -p tsconfig.json
(clean, exit 0)

The TUI's /quit handler ignored its argument entirely — the --delete
flag added for the CLI in NousResearch#27101 never reached any deletion logic.

Add a session.exit gateway RPC that mirrors the CLI shutdown path
(cli.py:17317): deletes the session's SQLite rows and on-disk
transcripts. Unlike session.delete (resume picker), it bypasses the
active-session guard because the caller is about to terminate.

The frontend now parses --delete / -d, rejects unknown flags with a
usage hint, and calls session.exit before ctx.session.die(). Deletion
is best-effort (.finally) — the TUI exits regardless, matching the
CLI's try/except behavior.
@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) area/sessions Session lifecycle, resume, persistence, history P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation labels Jul 30, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to #61353: both restore TUI /quit --delete, but this PR adds session.exit while #61353 extends session.close({delete:true}). The session-lifecycle contract needs a maintainer choice.

@zjin1126

Copy link
Copy Markdown
Author

Apologies for the duplicate — I missed #61353 when searching. Their approach is strictly better: extending the existing session.close RPC instead of adding a new one, resolving the durable SessionDB id (avoiding the runtime-sid false-success no-op), checking gateway ownership before deletion (preventing the #60609 routing loop), and proper _teardown_session lifecycle handling. Closing in favor of #61353.

@zjin1126 zjin1126 closed this Jul 30, 2026
@zjin1126
zjin1126 deleted the fix/tui-quit-delete branch July 30, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/tui Terminal UI (ui-tui/ + tui_gateway/) needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants