fix(oneshot): shut down memory provider to prevent SIGABRT on exit

[koshaji]

Problem

In oneshot mode (hermes -z), the memory provider's shutdown() is never called — only the interactive CLI registers the atexit cleanup. Providers with daemon worker threads blocked in C-level I/O at interpreter shutdown (e.g. Honcho's httpx socket recv) are then forcibly killed by CPython at Py_FinalizeEx, producing SIGABRT (signal 6) with no Python traceback.

Built-in memory (SQLite) is unaffected — it has no daemon threads.

Reproduction

# With Honcho memory provider configured:
hermes -z "hello"
# Process crashes with SIGABRT (no traceback)

Fix

• hermes_cli/oneshot.py: register an atexit hook in _run_agent() calling agent.shutdown_memory_provider(), mirroring the interactive CLI's _run_cleanup. This is the general fix for any memory provider with background threads.
• plugins/memory/honcho/__init__.py: harden shutdown() to join all daemon threads with a timeout, close the httpx client, and mark the session as closed so subsequent calls fail fast instead of hanging.
• plugins/memory/honcho/session.py: add closed flag and _close() method.

Testing

Verified on production fleet (3 Hermes gateway instances running 24/7 with Honcho memory). The crash no longer occurs after this fix.

References

• CPython issue: gh-97940 — threads blocked in I/O at interpreter shutdown

[Copilot]

Pull request overview

This PR aims to prevent hermes -z (oneshot mode) from exiting with SIGABRT when using memory providers that spawn daemon threads (notably the Honcho provider), by ensuring memory-provider shutdown runs before CPython interpreter teardown and by making Honcho’s shutdown more robust.

Changes:

• Register an atexit cleanup in hermes_cli/oneshot.py to call shutdown_memory_provider() in oneshot runs.
• Track Honcho “context prefetch” background threads and attempt to join them during shutdown.
• Add Honcho-provider shutdown hardening intended to unblock in-flight httpx recv calls and join worker threads.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
hermes_cli/oneshot.py	Adds oneshot-specific atexit cleanup to shut down the memory provider.
plugins/memory/honcho/session.py	Tracks context-prefetch threads and joins them during shutdown.
plugins/memory/honcho/__init__.py	Attempts to close Honcho’s http client and join worker threads during provider shutdown.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #49498 — same author, same three files, and effectively the same mechanism (an atexit hook in hermes_cli/oneshot.py calling shutdown_memory_provider() plus Honcho-side thread-join/httpx-close hardening). #49498 (opened 2026-06-20) is the earlier open version; this re-submission reorganizes the hook inline. Suggest closing this in favor of #49498 (or folding any refinements back into it). Related distinct-mechanism siblings: #52186 (join prefetch threads), #50217 (skip prefetch enqueue), #31664 (try/finally).

[koshaji]

Bump — friendly check-in on review status for the oneshot SIGABRT fix. Happy to address any feedback or rebase if needed. (This is the rebased replacement for #49498.)

[koshaji]

Copilot review feedback addressed

Pushed b55db41 addressing all 5 review comments:

1. Shutdown ordering — _close_honcho_http_client() is now called before _manager.shutdown(), so threads blocked in socket recv are interrupted first. The duplicate _manager.shutdown() call (post-http-close) has been removed; the manager shutdown now runs once, after the client close.

2. Thread list pruning — prefetch_context() now prunes completed threads ([th for th in ... if th.is_alive()]) before appending new ones, preventing unbounded growth in long-lived sessions.

3. Thread ref preservation — shutdown() keeps references to threads that don't join within the timeout instead of calling .clear(). Only threads that are still alive after the join attempt are retained for retry.

4. Transcript gap — _shutdown_oneshot_and_exit() is now called explicitly after run_conversation() returns, so the memory provider's teardown receives the real session state. The atexit registration remains as a guarded fallback (_shutdown_done flag prevents double-call).

5. closed flag — Added self._closed to HonchoSessionManager. Both shutdown() and prefetch_context() check it to fail fast after teardown.

[teknium1]

Thanks for pursuing a real current-main oneshot cleanup gap: hermes_cli/oneshot.py:419 currently returns directly after run_conversation() without memory-provider teardown.

Problems

• hermes_cli/oneshot.py:437 invokes shutdown_memory_provider() without messages. run_agent.py:3313 therefore sends [] to on_session_end(), not the real transcript described in the PR. The established CLI pattern is cli.py:1132-1145, which forwards _session_messages when it is a list.
• plugins/memory/honcho/session.py:694-709 has an unsynchronized check-then-register sequence. A prefetch can pass _closed before shutdown() sets it and then append/start a daemon after shutdown() has taken its thread-list snapshot at lines 569-575.
• The PR changes lifecycle and concurrent teardown behavior but adds no regression test coverage.

Suggested changes

• Mirror the guarded transcript-forwarding behavior from cli.py:1132-1145 for both explicit and atexit cleanup.
• Protect the closed flag and prefetch-thread registration with a shared lock; have shutdown atomically mark closed and snapshot tracked threads before joining.
• Add focused oneshot transcript, fallback/idempotence, and shutdown-race tests.

This is an automated hermes-sweeper review.

[teknium1]

shutdown_memory_provider() receives no transcript here, so run_agent.py:3313 calls every provider's on_session_end([]). Please mirror cli.py:1132-1145: pass agent._session_messages when it is a list, with no-argument fallback only for partial/test agents.

[teknium1]

This check is not synchronized with shutdown(): a caller can observe _closed == False, then shutdown can set it and snapshot the list, after which this call appends and starts a new daemon. Guard the closed check plus thread registration and shutdown's closed/snapshot transition with the same lock.

[koshaji]

#49498 is now closed — this PR supersedes it (rebased + all Copilot feedback incorporated). The duplicate label can be removed; I don't have admin rights to do so on this repo.

[ottodevs]

Independently hit this on v0.18.2 (2026.7.7.2), NixOS, glibc 2.42, CPython 3.12: hermes -z printed its answer, then died with SIGABRT (exit 134) on every run. Core backtrace matched the mechanism this PR targets — daemon thread blocked in sock_recv wakes during finalization → take_gil → PyThread_exit_thread → pthread_exit → glibc abort while unwinding.

I wrote an equivalent fix before finding this PR and can confirm the approach: force-closing the SDK's httpx client is what actually unblocks the stuck threads — join-with-timeout alone (#37635 and friends) can't, because the read never returns. With the client closed + the manager shutdown sentinel, exit went from a consistent 134 to 0 across repeated runs (verified against the deployed build, not just the repo checkout).

Two small deltas from my version that might be worth folding in:

1. Public accessor instead of _manager._honcho — the manager sets _honcho lazily in the honcho property, so the traversal can come up empty when nothing touched the property in that run; the singleton slot always knows:

# plugins/memory/honcho/client.py
def peek_honcho_client() -> "Honcho | None":
    """Return the client singleton if one was built, without creating it."""
    return _honcho_client_slot.peek()

2. Register the atexit hook in the plugin itself — covers every command path that initializes the provider, not just oneshot (with an idempotency guard in shutdown() so explicit + atexit double calls are harmless):

# plugins/memory/honcho/__init__.py
def register(ctx) -> None:
    """Register Honcho as a memory provider plugin."""
    provider = HonchoMemoryProvider()
    ctx.register_memory_provider(provider)
    atexit.register(provider.shutdown)

Happy to test an updated revision on my setup if useful — repro here is 100% deterministic.

[alt-glitch]

This was generated by AI during triage.

Correction: #49498 is closed, and the author confirms this PR is its rebased replacement with additional review fixes. The stale duplicate relation has been removed. Related to open #31664 and the broader oneshot-Honcho SIGABRT fix cluster.

[koshaji]

Pushed 944fbbe27 — rebased onto current main and addressed all three review points:

1. Transcript forwarding — one note: current main already forwards the transcript on the explicit finally path in _run_agent (the review referenced an older main). What remained was the atexit fallback: there is now a shared, once-guarded _shutdown_memory_provider_with_transcript(agent) mirroring cli.py's guarded pattern (_session_messages forwarded when it is a list, bare call otherwise), used by both the explicit path and the atexit hook — whichever fires first wins, the other is a no-op.
2. Shutdown/prefetch race — shutdown() now atomically marks _closed and snapshots tracked prefetch threads inside _prefetch_threads_lock, and prefetch_context() re-checks _closed, registers, and starts the thread under the same lock. A prefetch thread now either lands in shutdown's join snapshot or never starts.
3. Regression tests — new tests/hermes_cli/test_oneshot_memory_cleanup.py (8 tests: real-transcript forwarding, once-only cleanup across explicit+atexit, non-list fallback, exception safety) and TestShutdownPrefetchRace in tests/honcho_plugin/test_honcho_shutdown.py (4 tests, incl. a deterministic TOCTOU interleaving that injects shutdown() between prefetch's closed-check and registration and asserts the thread never starts).

Test results: 19 targeted tests pass; full tests/honcho_plugin/ suite 437 passed / 20 skipped.

[koshaji]

Rebased onto current main (9da6d455, was 3481 commits behind) and verified locally.

Three conflict resolutions in plugins/memory/honcho/session.py — all in the shutdown() region, all caused by main adding an _async_thread_lock and a lazy honcho property that starts the async writer on first access (unrelated to this fix):

1. Init (line ~204): kept both main's _async_thread_lock = threading.Lock() and this PR's _context_prefetch_threads: list[threading.Thread] = []. Independent attributes, both wanted.
2. shutdown() async-writer guard: main introduced a two-level guard (if self._async_queue is not None: → flush_all() → if self._async_thread is not None and .is_alive(): → signal+join). This PR's prior version combined them into a single if queue is not None and thread is not None:. Kept main's two-level form — it's safer: if the writer thread was never started (the new lazy honcho property means _async_thread can be None even when _async_queue is set), main's version still flushes queued messages, the combined form would skip flush_all() and leak them. The prefetch-thread joining from this PR is preserved unchanged.
3. TOCTOU race fix (final commit): the atomic _prefetch_threads_lock-guarded closed-check + snapshot is preserved verbatim; only the async-writer guard line underneath was switched to main's two-level form per point 2.

Verification (Python 3.13, full dep install):

• tests/honcho_plugin/ + tests/hermes_cli/test_oneshot_memory_cleanup.py + oneshot tests: 316 passed, 0 failed (was 297 baseline; the +19 are this PR's new tests, all green on current main).
• Full tests/hermes_cli/ sweep shows 166 webhook_cli failures, but I confirmed these are pre-existing on plain main (same 166 fail without this PR's changes) — a test-isolation issue in test_webhook_cli.py unrelated to this fix.

Should be MERGEABLE now with no conflicts. The SIGABRT mechanism this targets is still live on main (daemon prefetch threads are still spawned and still not joined at teardown).