fix(memory): respect toolsets and reconnect openviking by flyingdoubleG · Pull Request #18166 · NousResearch/hermes-agent

flyingdoubleG · 2026-05-01T02:42:49Z

Summary

gate external memory provider tool injection on enabled_toolsets / disabled_toolsets so platform toolset filters are respected
lazily reconnect OpenViking on tool calls after startup health check failures instead of keeping the provider permanently disconnected
add regressions for both issue-derived failure modes

Related issues

Addresses bug + feature request: Memory provider tools auto-injected regardless of platform_toolsets config — 10x latency penalty on local models #5544
Addresses OpenViking memory provider fails permanently if server is unreachable at agent startup; never recovers #5721

Validation

.venv-win\Scripts\python.exe -m pytest tests\agent\test_memory_provider.py -q
.venv-win\Scripts\python.exe -m pytest tests\plugins\memory\test_openviking_provider.py -q

flyingdoubleG · 2026-05-01T12:55:14Z

Real-agent validation (2026-05-01): PASS

Isolated HERMES_HOME: D:\工作\hermes-agent-pr-real-agent\pr18166
Command shape: python -m hermes_cli.main --yolo --provider openai-codex -m gpt-5.5 -t terminal,file -z ...
Scenario: configured memory.provider: openviking with memory disabled and an unreachable endpoint, then ran the real OpenAI Codex-backed Hermes agent with only terminal,file toolsets.
Agent result: PR18166_AGENT_OK；本轮只启用了 terminal,file toolsets，且未调用 memory 工具。

This validates that the memory provider does not leak into a turn where memory is not enabled.

…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

The OpenViking reconnect change for #5721 looks reasonable to me. I left one inline note on the #5544 toolset gate.

ehz0ah · 2026-05-04T09:33:09Z

+        return False
+    if enabled_toolsets is None:
+        return True
+    return "memory" in enabled_toolsets


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.

teknium1 · 2026-07-12T23:02:35Z

Thanks for covering two real failure modes. The enabled-toolsets half has since landed on current main in a more complete form, but the disabled-toolsets and recovery concerns still need a current-main salvage.

Problems

run_agent.py:872 only recognizes a literal "memory" entry. As the existing inline review notes, composite selections such as hermes-acp and aliases such as all also include memory tools. Current main resolves those through agent/memory_manager.py:82-97.
The target injection site moved: agent/agent_init.py:1419-1420 now calls the shared helper, while MCP refresh has a separate reinjection path at tools/mcp_tool.py:5376-5388. The submitted patch would not apply cleanly or cover all current paths.

Suggested changes

Move the disabled-toolsets check into the shared memory-provider injection gate and use it from initial and refresh paths.
Adapt reconnect behavior to the current OpenViking runtime-health/autostart flow in plugins/memory/openviking/__init__.py:2027-2177; add a same-session recovery regression.

Automated hermes-sweeper review.

kshitijk4poor · 2026-07-22T08:51:35Z

Superseded by #51952, which includes your commit cherry-picked with authorship preserved. Current main already resolves enabled composite toolsets correctly; your disabled-toolset gating was adapted to the shared memory-provider injection path and the MCP tool-snapshot refresh, with the reconnect behavior folded into the new runtime-health handling. Thanks @flyingdoubleG — credit retained in git history and scripts/release.py.

fix(memory): respect toolsets and reconnect openviking

4b53897

alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels May 1, 2026

SelfParody mentioned this pull request May 2, 2026

feat(cli): filter MCP server spawning by -t/--toolsets flag + fix orphan subprocess leak #19000

Open

6 tasks

ehz0ah reviewed May 4, 2026

View reviewed changes

This was referenced May 4, 2026

OpenViking PR Report - 2026-05-04 ehz0ah/hermes-agent#1

Open

OpenViking PR Report - 2026-05-05 ehz0ah/hermes-agent#2

Open

ehz0ah mentioned this pull request Jun 24, 2026

fix(openviking): harden tool gating, config auth, and runtime shutdown #51952

Closed

19 tasks

teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026

teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label Jul 19, 2026

kshitijk4poor closed this Jul 22, 2026

kshitijk4poor mentioned this pull request Jul 22, 2026

fix(openviking): harden tool gating, config auth, and runtime shutdown (salvage #51952) #69221

Merged

Navigation Menu

Uh oh!

fix(memory): respect toolsets and reconnect openviking - #18166

fix(memory): respect toolsets and reconnect openviking#18166
flyingdoubleG wants to merge 1 commit into
NousResearch:mainfrom
flyingdoubleG:codex/fix-memory-provider-issue-regressions

flyingdoubleG commented May 1, 2026

Uh oh!

flyingdoubleG commented May 1, 2026

Uh oh!

ehz0ah left a comment •

edited

Loading

Uh oh!

ehz0ah May 4, 2026

Uh oh!

teknium1 commented Jul 12, 2026

Uh oh!

kshitijk4poor commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

flyingdoubleG commented May 1, 2026

Summary

Related issues

Validation

Uh oh!

flyingdoubleG commented May 1, 2026

Uh oh!

ehz0ah left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ehz0ah May 4, 2026

Choose a reason for hiding this comment

Uh oh!

teknium1 commented Jul 12, 2026

Problems

Suggested changes

Uh oh!

kshitijk4poor commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ehz0ah left a comment •

edited

Loading