fix(memory): respect toolsets and reconnect openviking#18166
Open
flyingdoubleG wants to merge 1 commit into
Open
fix(memory): respect toolsets and reconnect openviking#18166flyingdoubleG wants to merge 1 commit into
flyingdoubleG wants to merge 1 commit into
Conversation
Author
|
Real-agent validation (2026-05-01): PASS Isolated This validates that the memory provider does not leak into a turn where memory is not enabled. |
Open
6 tasks
SelfParody
added a commit
to SelfParody/hermes-agent
that referenced
this pull request
May 2, 2026
…han subprocess leak v5 — addresses GPT-5.5 fusion-judge SHIP_WITH_FIXES on v4. One-line cosmetic fix: SIGTERM handler now uses cleanup-scope local alias `_os_local.kill(_os_local.getpid(), signum)` instead of bare `os.kill(os.getpid(), signum)`, completing the self-containment intent. Behavior unchanged (verified: SIGTERM still exits 143, SIGINT still 130). ═══ Code review history ═══ - v1: qwen3-max single review → SHIP_WITH_FIXES - v2: Fusion (Codex+Sonnet+Gemini, GPT-5.5 judge) → NEEDS_REWORK (7 fixes) - v3: Fusion → SHIP_WITH_FIXES (3 required + 4 hardening fixes) - v4: Fusion → SHIP_WITH_FIXES (1 cosmetic — use _os_local in SIGTERM) - v5 (this commit): cosmetic fix applied; behavior verified unchanged ═══ Final fix tracking ═══ | v2 required fix | Final | |---------------------------------------|-------| | 1. Fail-open import behavior | FIXED | | 2. SIGTERM exit code 143 | FIXED | | 3. Unmatched -t warning logic | FIXED | | 4. _stdio_pids robustness | FIXED | | 5. time/os imports verified | FIXED | | 6. Poll-loop logger.debug | FIXED | | 7. Idempotency guard | FIXED | | 8. SIGTERM uses self-contained _os | FIXED (v5) | ═══ Verified locally (final) ═══ hermes -z "ACK" -t web : 4.8s wall (was 65s — 93% reduction) hermes -z "ACK" -t slack : 5.5s wall (was 67s — 92% reduction) hermes -z "ACK" (no -t) : 75s (unchanged — backwards-compat) SIGINT mid-flight : exit 130 ✓ SIGTERM mid-flight : exit 143 ✓ Orphan accumulation : 0 new PPID=1 across 10+ runs Related issues: - Fixes the MCP subprocess component of NousResearch#18438 (gateway memory leak) - Supersedes the startup-drag portion of NousResearch#18523 (closed unmerged) - Extends toolset-gating pattern from NousResearch#18166 and NousResearch#5788 (memory) to MCPs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
ehz0ah
reviewed
May 4, 2026
| return False | ||
| if enabled_toolsets is None: | ||
| return True | ||
| return "memory" in enabled_toolsets |
Contributor
There was a problem hiding this comment.
I think this should resolve toolsets before checking for memory. As written, enabled_toolsets=["hermes-acp"], ["hermes-cli"], or ["all"] returns false even though those toolsets resolve to include "memory", so external provider tools can disappear when memory is actually enabled.
This was referenced May 4, 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
3 participants
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.
Summary
Related issues
Validation