Repository navigation
Plugins hear about every session delete and archive: on_session_delete hook (#124511, salvage #124988, #132639) - #140995
Merged
Merged
Conversation
૮ >ﻌ< ა ci reviewran on 3d79bd7 — fix(plugins): on_session_delete review findings debug infoCI timingsCI timings · View report · View jobWall time 8m59s vs 13m1s (-31.0%). 2 job(s) slower, 15 faster, 3 unchanged.
|
[Medium impact] The PR appears safe to merge; all five previous findings are fixed, and no new blocking issue was found.
|
Collaborator
Author
|
All five Greptile findings were valid and are fixed in e4ff3c7:
Tests: 669 passed across 17 neighbor files, including @greptileai review |
teknium1
force-pushed
the
feat/hook-session-delete
branch
from
October 18, 2026 08:15
e4ff3c7 to
c028bc0
Compare
…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>
hermes_state_sessions.py crossed the 2000-line target with the delete/archive hook calls; the auto-archive wrapper belongs with the sweep it drives in hermes_state_maintenance.py. Also drops the api_server rollback comment so that facade does not grow.
- Dispatch under the store's own profile home: `sessions repair-profiles` deletes from another profile's store, and the event now reaches that profile's plugins instead of the launch profile's. - Archive events list every hidden chain row (deleted_ids), on both the SessionDB and the runtime-receipt path; the requested id stays primary. - Fire right after the commit, before media/transcript cleanup that can raise, so a cleanup failure no longer swallows the event. - mutate_runtime_session's write returns (result, SessionGoneEvent | None) instead of filling a shared list. - Drop the leftover duplicate hook reference block in hooks.md.
teknium1
force-pushed
the
feat/hook-session-delete
branch
from
October 18, 2026 09:00
c028bc0 to
3d79bd7
Compare
This was referenced Oct 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plugins now get one
on_session_delete(session_id, reason, profile, ...)call, after the commit, whenever a session is deleted or deliberately archived, from any surface.Closes #124511. Salvages #124988 by @Finn763 (state-layer delete hook, commits kept with authorship) and folds in #132639 by @pieralukasz (archive as a "done" signal;
Co-authored-by).What plugins get
session_idreasondelete,archive,empty_sweep,prune,ghost_prune,profile_repairprofiledefaultfor the root home); dispatch runs under that profile's home, so its plugins get the event even when another profile's process did the deletedeleted_idssessions_dirsessions/dir when the caller had oneObserver only: return ignored, a raising callback is logged and skipped, the delete still commits. Process-local delivery, no exactly-once guarantee (documented).
Changes
hermes_state_common.emit_session_deleted: one late-bound, fail-open seam intolifecycle.invoke_hook, called right after_execute_writereturns and before media/transcript cleanup (which can raise).delete_session,delete_sessions,delete_session_if_empty,delete_empty_sessions,prune_sessions,prune_empty_ghost_sessions,delete_moved_session(Finn's commits).mutate_runtime_sessionfires fordeleteand for a real archive flip. Desktop/dashboard single delete and archive go through this canonical receipt, notSessionDB.delete_session, so before this commit Desktop never fired. An exact retry replays the receipt and stays silent.set_session_archivedfiresreason="archive"only on a real unarchived → archived flip. That covers TUIsession.archive, deferred TUI archives andhermes sessions archive. Repeat archives, un-archives, the idle auto-archive sweep and profile adoption (notify=False) stay silent. One hook with areasonfield replaces the separateon_session_archivedhook.Callers covered (grep of every
DELETE FROM sessions/ archive writer): CLIsessions delete|prune|archive,/exit --delete, quit-time empty discard, curses browser, auto-maintenance ghost prune and auto-prune, dashboard bulk delete / "Delete empty" / prune, DesktopDELETE+PATCH archived, TUIsession.delete/session.archive, onboarding reset, gateway API server delete, gateway never-active prune, profile repair. Not fired: the api_server title-conflict rollback of a row inserted in the same transaction (commented), the seed-copydiscard_unadmitted_sessioncompensation path.Validation
tests/hermes_state/test_session_delete_hook.py(2 invariant tests, realSessionDB+ real hook registry)tests/hermes_state/test_hermes_state.pyalone, head vs origin/mainscripts/check(incl. code health)maybe_auto_archivemoved tohermes_state_maintenance.pyto keephermes_state_sessions.pyunder 2000 linesCross-profile check: a default-profile process ran
delete_moved_sessiononprofiles/work/state.db. Only the work profile's plugin logged{"reason": "profile_repair", "profile": "work"}.Live repro (real
hermesprocesses, scratchHERMES_HOME, a scratch user plugin registeringon_session_delete, three real chat sessions on Hermes-4-70B):On origin/main none of these produced a callback (hook not in
VALID_HOOKS, no dispatch).Infographic