Skip to content

feat(plugins): on_session_archived hook for deliberate session archives - #132639

Closed
pieralukasz wants to merge 1 commit into
NousResearch:mainfrom
pieralukasz:feat/on-session-archived-hook
Closed

pieralukasz wants to merge 1 commit into
NousResearch:mainfrom
pieralukasz:feat/on-session-archived-hook

Conversation

@pieralukasz

@pieralukasz pieralukasz commented Oct 4, 2026 •

Copy link
Copy Markdown

What does this PR do?

Adds an on_session_archived observer hook. It fires after a deliberate session archive commits, once per real unarchived → archived transition.

Archiving is a common "this is done" signal. Concrete consumer: an automation that opens a Hermes session per to-do task (Todoist) and should complete the task when the user archives that session. Today such a plugin can only find out by polling state.db (every few minutes in my setup). With the hook, the plugin can react immediately and needs no polling worker.

Related Issue

No existing issue or PR found (searched PRs/issues for on_session_archived, session archived hook, archive hook, session_archived).

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

  • hermes_cli/plugins.py: register on_session_archived in VALID_HOOKS (observer).
  • hermes_cli/lifecycle.py: archive_session(db, session_id, archived, *, surface, profile) wraps SessionDB.set_session_archived. It reads the prior flag, writes, then fires the hook only when the row went from unarchived to archived. notify_session_archived never raises.
  • Deliberate archive surfaces route through the helper:
    • Desktop/dashboard PATCH /api/sessions/{id} (hermes_cli/web_routers/sessions.py, surface="dashboard", passes body.profile)
    • TUI session.archive RPC (tui_gateway/methods_session.py, surface="tui") and the deferred pending_archived path in _ensure_session_db_row (tui_gateway/session_workdir.py)
    • hermes sessions archive (hermes_cli/sessions_cmd.py, surface="cli", through a new optional archive= callback on SessionDB.archive_sessions; default behaviour unchanged)
  • Intentionally not fired: the idle auto-archive sweep (archive_stale_sessions / _auto_archive_lineage), profile adoption retiring donor segments, repeated archives, and un-archives. Those are housekeeping, not a user's decision.
  • hermes_cli/hooks.py: synthetic payload for hermes hooks test on_session_archived.
  • Docs: catalog row + section in website/docs/user-guide/features/hooks.md, and the observer list in plugins.md.

Payload: session_id, surface ("dashboard" | "tui" | "cli"), profile (None for the serving profile).

How to Test

tests/hermes_cli/test_session_archived_hook.py runs against a real SessionDB under the isolated test HERMES_HOME, with the real plugin hook registry:

  1. Dashboard PATCH fires exactly once. The flag is already committed when the observer runs. Repeat archive, unrelated flags and un-archive stay silent. Re-archiving after an un-archive fires again.
  2. TUI session.archive fires once (with a repeat).
  3. hermes sessions archive fires once per archived session.
  4. The idle sweep archives without firing.
  5. A raising callback does not fail the archive.
scripts/run_tests.sh tests/hermes_cli/test_session_archived_hook.py tests/tui_gateway/test_session_archive_rpc.py tests/hermes_cli/test_prune_spares_pinned.py tests/hermes_cli/test_web_server.py tests/hermes_cli/test_kanban_lifecycle_hooks.py
=== Summary: 5 files, 214 tests passed, 0 failed ===

Without the source change, the new file gives 1 passed / 5 failed. Only the "sweep does not fire" case passes, as expected.

Broader run (tests/tui_gateway, tests/agent/test_shell_hooks.py, tests/hermes_cli/test_{hooks,sessions,plugins,lifecycle}*, tests/test_hermes_state*): 2767 passed, 28 failed. Exactly the same 28 tests fail on unmodified main in my local env (PM-free uv sync venv on macOS: subprocess/profile-listing tests). Compared with diff of the FAILED sets, they are identical.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature
  • I've run pytest tests/ -q and all tests pass (targeted + broader suites above; full suite not run locally)
  • I've added tests for my changes

Archiving a session is a common "this is done" signal, but plugins could only
learn about it by polling state.db. Fire an observer hook after a deliberate
archive commits, once per real unarchived -> archived transition:
Desktop/dashboard PATCH, TUI session.archive (including deferred drafts) and
hermes sessions archive. The idle auto-archive sweep, profile adoption,
repeats and un-archives stay silent, and a failing callback never fails the
archive.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Oct 4, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

on_session_archived is silent on the gateway's PATCH /api/sessions/{id}

Nice hook, and the "only on a real unarchived→archived transition, never from the idle sweep" contract reads clearly. One gap: a third HTTP archive entry point doesn't fire it.

This PR rewires the dashboard flag table (web_routers/sessions.py:848, ("archived", _archive_via_rest)). But a second handler for the same method, path and field keeps its own table and was left alone:

gateway/platforms/api_server.py:3250
for flag, setter in (("archived", db.set_session_archived), ("hidden", db.set_session_hidden),
                     ("pinned", db.set_session_pinned)):
    if flag in body: await asyncio.to_thread(setter, session_id, body[flag])

It's live, not dead code: registered in _http_route_table() (api_server.py:1766), behind a normal Platform.API_SERVER + API_SERVER_KEY gate, and tests/gateway/test_session_api.py:1195 already PATCHes {"archived": True} through it — that test exists on base too, so it still passes unchanged. It proves the flag persists, not that anything observes it.

Why a bug, not a deliberate exclusion. The comment above that loop ties the tables together:

# wins over an explicit hidden (same order as the dashboard's _RENAME_FLAG_SETTERS).

Ordering is still shared, so the comment still reads true — but the event source no longer is. And the new hooks.md enumerates surfaces by URL (PATCH /api/sessions/{id} with archived: true); the missed handler serves that exact URL from the gateway listener. Anyone building "archive means close the ticket" against the documented surface silently never fires on gateway archives — no error, nothing logged.

The new tests don't catch it: grep -c api_server tests/hermes_cli/test_session_archived_hook.py → 0. Four entry points exist; the other three are covered, this one is neither covered nor fixed.

Fix — route it through the same helper, which takes any SessionDB:

if flag == "archived":
    await asyncio.to_thread(archive_session, db, session_id, body[flag], surface="api")
else:
    await asyncio.to_thread(setter, session_id, body[flag])

If the gateway is deliberately out of scope, document the exception and drop that comment.

teknium1 added a commit that referenced this pull request Oct 18, 2026
…e archives

Finn's state-layer hook missed the paths Desktop actually uses: single
delete and archive go through the canonical mutation receipt
(mutate_runtime_session), not SessionDB.delete_session, so a plugin never
heard about either. Archive (the "this is done" signal #132639 asked for)
fired nowhere.

- mutate_runtime_session fires after its write commits for operation
  'delete' and for a real unarchived -> archived sidebar flip; an exact
  retry replays the receipt and stays silent.
- set_session_archived fires reason="archive" on a real flip, so TUI
  session.archive, `hermes sessions archive` and deferred TUI archives are
  covered from one place. Repeats, un-archives, the idle auto-archive
  sweep and profile adoption (notify=False) stay silent.
- Payload is now session_id + reason + profile on every path (profile
  from the store's home), deleted_ids when more than one row went;
  primary id is the one the caller asked for, not the alphabetically
  first.
- One fail-open seam (emit_session_deleted) calls lifecycle.invoke_hook
  directly; the extra notify_session_deleted wrapper is gone.
- Tests trimmed to two invariants against the real hook registry.

Co-authored-by: Lucas Piera <pieralukasz@gmail.com>
teknium1 added a commit that referenced this pull request Oct 18, 2026
…e archives

Finn's state-layer hook missed the paths Desktop actually uses: single
delete and archive go through the canonical mutation receipt
(mutate_runtime_session), not SessionDB.delete_session, so a plugin never
heard about either. Archive (the "this is done" signal #132639 asked for)
fired nowhere.

- mutate_runtime_session fires after its write commits for operation
  'delete' and for a real unarchived -> archived sidebar flip; an exact
  retry replays the receipt and stays silent.
- set_session_archived fires reason="archive" on a real flip, so TUI
  session.archive, `hermes sessions archive` and deferred TUI archives are
  covered from one place. Repeats, un-archives, the idle auto-archive
  sweep and profile adoption (notify=False) stay silent.
- Payload is now session_id + reason + profile on every path (profile
  from the store's home), deleted_ids when more than one row went;
  primary id is the one the caller asked for, not the alphabetically
  first.
- One fail-open seam (emit_session_deleted) calls lifecycle.invoke_hook
  directly; the extra notify_session_deleted wrapper is gone.
- Tests trimmed to two invariants against the real hook registry.

Co-authored-by: Lucas Piera <pieralukasz@gmail.com>
teknium1 added a commit that referenced this pull request Oct 18, 2026
…e archives

Finn's state-layer hook missed the paths Desktop actually uses: single
delete and archive go through the canonical mutation receipt
(mutate_runtime_session), not SessionDB.delete_session, so a plugin never
heard about either. Archive (the "this is done" signal #132639 asked for)
fired nowhere.

- mutate_runtime_session fires after its write commits for operation
  'delete' and for a real unarchived -> archived sidebar flip; an exact
  retry replays the receipt and stays silent.
- set_session_archived fires reason="archive" on a real flip, so TUI
  session.archive, `hermes sessions archive` and deferred TUI archives are
  covered from one place. Repeats, un-archives, the idle auto-archive
  sweep and profile adoption (notify=False) stay silent.
- Payload is now session_id + reason + profile on every path (profile
  from the store's home), deleted_ids when more than one row went;
  primary id is the one the caller asked for, not the alphabetically
  first.
- One fail-open seam (emit_session_deleted) calls lifecycle.invoke_hook
  directly; the extra notify_session_deleted wrapper is gone.
- Tests trimmed to two invariants against the real hook registry.

Co-authored-by: Lucas Piera <pieralukasz@gmail.com>
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @pieralukasz. This landed in #140995 (merged at 627ce03), folded into a single on_session_delete hook. You're credited with Co-authored-by on aab0dc7.

What carried over from your design:

  • A deliberate archive fires on_session_delete(session_id, reason="archive", profile=...) after the flag commits, and only on a real unarchived → archived flip.
  • Repeat archives, un-archives, the idle auto-archive sweep and profile adoption stay silent, so plugins can still read an archive as the user's "done" signal.
  • Every archive surface is covered from one place (SessionDB.set_session_archived plus the Desktop/dashboard runtime receipt path): Desktop PATCH, TUI session.archive (deferred included) and hermes sessions archive.
  • deleted_ids lists the whole compression chain that was hidden.

The difference: there's one hook with a reason field instead of a separate on_session_archived hook, so plugins that clean up per-session state get deletes and archives through the same callback. A Todoist-style consumer filters on reason == "archive".

Closing as superseded.

@teknium1 teknium1 closed this Oct 18, 2026
c0wb0yp0tat0 pushed a commit to c0wb0yp0tat0/hermes-agent that referenced this pull request Oct 18, 2026
…e archives

Finn's state-layer hook missed the paths Desktop actually uses: single
delete and archive go through the canonical mutation receipt
(mutate_runtime_session), not SessionDB.delete_session, so a plugin never
heard about either. Archive (the "this is done" signal NousResearch#132639 asked for)
fired nowhere.

- mutate_runtime_session fires after its write commits for operation
  'delete' and for a real unarchived -> archived sidebar flip; an exact
  retry replays the receipt and stays silent.
- set_session_archived fires reason="archive" on a real flip, so TUI
  session.archive, `hermes sessions archive` and deferred TUI archives are
  covered from one place. Repeats, un-archives, the idle auto-archive
  sweep and profile adoption (notify=False) stay silent.
- Payload is now session_id + reason + profile on every path (profile
  from the store's home), deleted_ids when more than one row went;
  primary id is the one the caller asked for, not the alphabetically
  first.
- One fail-open seam (emit_session_deleted) calls lifecycle.invoke_hook
  directly; the extra notify_session_deleted wrapper is gone.
- Tests trimmed to two invariants against the real hook registry.

Co-authored-by: Lucas Piera <pieralukasz@gmail.com>
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/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants