Skip to content

fix(agent): #5544 strictly enforce toolset filters for memory provide… - #5552

Open
MustafaKara7 wants to merge 1 commit into
NousResearch:mainfrom
MustafaKara7:patch-4
Open

fix(agent): #5544 strictly enforce toolset filters for memory provide…#5552
MustafaKara7 wants to merge 1 commit into
NousResearch:mainfrom
MustafaKara7:patch-4

Conversation

@MustafaKara7

Copy link
Copy Markdown
Contributor

…r injection

What does this PR do?

Fixes #5544

The MemoryManager dynamically injects fact_store tools after the primary tool registry initialization. Previously, this bypassed enabled_toolsets and disabled_toolsets filters, causing massive token overhead and tool-loops for local models when platforms (e.g. Telegram) explicitly requested zero tools (telegram: []).

Added explicit validation against both self.enabled_toolsets and self.disabled_toolsets before appending memory schemas to self.tools, ensuring full compliance with user configurations.

Fixes #5544

@thundercat49

Copy link
Copy Markdown

Tested this approach on my local setup (RTX 3090, Qwen3-30B-A3B via llama.cpp) and the logic is correct — response time dropped from 42s to ~2s with telegram: [].
Minor: line 3 (if self._memory_manager and self.tools is not None:) looks like a leftover from the original code — it has no body and falls through silently. Should be removed to keep the diff clean.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/memory Memory tool and memory providers labels May 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Likely duplicate of #5788 — both fix memory tool injection bypassing platform_toolsets/enabled_toolsets config.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for identifying the late memory-tool injection path. The disabled-toolset part of the report remains valid on current main, but this patch needs a focused port.

Problems

  • The modified inline block no longer exists: current main injects via agent/memory_manager.py:100-145, invoked from agent/agent_init.py:1419-1420 after the base list has already been filtered at agent/agent_init.py:1159-1163.
  • The current helper checks only enabled toolsets at agent/memory_manager.py:112-116; it does not read disabled_toolsets, so the denylist bypass remains.
  • Do not port the direct membership check verbatim: agent/memory_manager.py:91-94 resolves composite toolsets, and tests/agent/test_memory_provider.py:1363-1368 requires hermes-acp to retain provider memory tools.

Suggested changes

  • Add a denylist-first memory gate in inject_memory_provider_tools(), retain the existing composite enabled-toolset resolver, and add tests for disabled-memory precedence in tests/agent/test_memory_provider.py.

Automated hermes-sweeper review.

Comment thread run_agent.py

# Inject memory provider tool schemas into the tool surface
if self._memory_manager and self.tools is not None:
# Respect both enabled and disabled toolsets configurations

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do not carry this direct membership check into the current helper unchanged: current main supports composite toolsets through memory_provider_tools_enabled() (agent/memory_manager.py:91-94), and tests/agent/test_memory_provider.py:1363-1368 verifies that hermes-acp exposes provider memory tools. Preserve that resolver and add the disabled-toolset check separately.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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 teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label Jul 19, 2026
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/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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.

bug + feature request: Memory provider tools auto-injected regardless of platform_toolsets config — 10x latency penalty on local models

4 participants