Skip to content

fix(cli): shut down oneshot memory providers - #63931

Draft
madeat3am wants to merge 1 commit into
NousResearch:mainfrom
madeat3am:fix/oneshot-memory-provider-shutdown
Draft

madeat3am wants to merge 1 commit into
NousResearch:mainfrom
madeat3am:fix/oneshot-memory-provider-shutdown

Conversation

@madeat3am

Copy link
Copy Markdown

Summary

  • mirror the interactive CLI memory-provider lifecycle in hermes -z
  • close the oneshot-owned SessionDB after agent shutdown
  • extend the existing oneshot integration test to assert both resources close

Reproduction

With Honcho active, a successful hermes -z tool call printed its final response and then exited 134 with SIGABRT. A native backtrace showed Python finalizing while a daemon Honcho HTTP worker was blocked in socket receive. hermes_cli/oneshot.py bypasses cli.py and returned without calling AIAgent.shutdown_memory_provider().

After this patch, real Honcho write and read oneshots both returned the expected marker, empty stderr, and exit 0.

Test

scripts/run_tests.sh tests/hermes_cli/test_tui_resume_flow.py

Result: 48 passed.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard tool/memory Memory tool and memory providers P3 Low — cosmetic, nice to have labels Jul 13, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Related: #31664 (earliest-open oneshot memory-provider shutdown), #52186 (join Honcho prefetch threads), #61875 (atexit hook), #50217 (skip oneshot prefetch). Same oneshot-SIGABRT class but this PR is a superset (also closes the oneshot-owned SessionDB); not a duplicate. A maintainer should pick one canonical approach for the cluster.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused lifecycle fix. Current main still creates the oneshot SessionDB and AIAgent in hermes_cli/oneshot.py:388-417, then returns immediately after run_conversation() at hermes_cli/oneshot.py:425-426; it does not perform the equivalent of interactive cleanup. The proposed finally mirrors the existing interactive CLI behavior at cli.py:1113-1147, and AIAgent.shutdown_memory_provider() performs the required provider end-session and shutdown calls at run_agent.py:3388-3404.

The proposed test also verifies both resources are released from the oneshot owner path. The GitHub comparison reports the PR as 14 commits behind its base, but none of those base changes touch either PR-modified path, so this should be mechanically salvageable.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 16, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Summary

Two PRs address the oneshot SIGABRT/lifecycle failure: #63931 adds owner-path cleanup for the memory provider and SessionDB, while #31172 attempted a broader shutdown strategy that also skipped oneshot prefetch and stopped Honcho’s async writer.

Related pull requests

  • #31172 [closed] related — (+101/-12) — closed reference implementation, not a strict duplicate: it overlaps #63931 by shutting down the oneshot memory provider and closing the SessionDB, but additionally disables next-turn prefetch, changes Honcho manager shutdown, and includes unrelated gateway enablement changes; it remains relevant as evidence for the broader worker-teardown approach.
  • #63931 related — (+67/-34) — keep open with a salvage path: its focused finally mirrors the interactive lifecycle by calling shutdown_memory_provider() and closing the oneshot-owned SessionDB, with a test covering both releases. This agrees with the maintainer-bot keep_open review, while the contributor’s cluster note should be addressed by treating #63931 as the focused owner-lifecycle candidate rather than conflating it with the extra prefetch and Honcho changes in #31172.

Duplicates

#31172 and #63931 overlap on oneshot memory-provider and SessionDB cleanup, but they are not strict duplicates: #31172 also changes prefetch behavior, Honcho shutdown, and gateway configuration.

Suggested consolidation

Keep #63931 open with the concrete salvage path of rebasing its focused oneshot finally cleanup and its dual-resource lifecycle test onto current main. Retain closed #31172 only as a reference for separately evaluating the additional prefetch and Honcho worker-shutdown changes; no PR in this pair should be closed as a duplicate of the other.

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 17 kB of PR diffs, 2 kB of issue/PR text, <1 kB of discussion (1 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

This branch has not been deployed

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

Labels

area/memory Memory subsystem: store, providers, sync, background reviews comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants