Skip to content

fix: close finalized LCM engine clones - #470

Closed
bennybuoy wants to merge 2 commits into
stephenschoettler:mainfrom
bennybuoy:fix/finalize-cloned-engine-resources
Closed

bennybuoy wants to merge 2 commits into
stephenschoettler:mainfrom
bennybuoy:fix/finalize-cloned-engine-resources

Conversation

@bennybuoy

@bennybuoy bennybuoy commented Aug 2, 2026 •

Copy link
Copy Markdown

Summary

  • register a supported on_session_finalize plugin hook
  • resolve the exact per-agent LCM clone by durable session ID
  • shut down only that finalized clone, leaving other active clones and the plugin prototype untouched
  • preserve interactive CLI /new, which reuses its agent/context engine

Why

Hermes deep-copies the registered context-engine prototype for each AIAgent. In long-lived Desktop and gateway processes, retiring an agent does not currently guarantee that the clone's SQLite-backed LCM helpers are closed. Finalized sessions can therefore leave Store, DAG, and lifecycle descriptors open.

The plugin already owns an exact weak session-to-engine registry and an idempotent shutdown() boundary. This change connects those existing pieces through Hermes' supported hard session-boundary hook; it does not patch Hermes core or sweep unrelated engines.

Verification

  • focused plugin lifecycle, hook registration, and packaging matrix: 33 passed
  • 12-cycle Linux FD soak while retaining references to every finalized clone: post-warm-up LCM FD count returned from 13 to the steady baseline of 10 after every cycle
  • Ruff: passed
  • Python compilation: passed
  • git diff --check: passed

Scope

This closes plugin-owned LCM SQLite resources. It does not claim to close Hermes-owned state.db connections.

Automated review

GitHub Copilot reviewed the identical diff in bennybuoy/hermes-lcm#24, because fork contributors cannot request reviewers on the upstream repository. Its initial test-helper suggestion was addressed in 085d6ee; a second review of that commit covered both changed files and generated no new comments.

@bennybuoy

Copy link
Copy Markdown
Author

Superseded by #528, refreshed onto current main with updated lifecycle/packaging coverage and explicit compatibility validation against #501.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant