feat(memory): allow disabling built-in memory toolfeat(memory): allow disabling built-in memory tool - #39531
feat(memory): allow disabling built-in memory toolfeat(memory): allow disabling built-in memory tool#39531MyQiongbao wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the built-in schema from external provider tools. The request remains relevant on current main, but the current patch needs a scope decision before salvage.
Problems
- The schema filter does not stop the global built-in store from reaching the model. Current main loads it when
memory_enabledoruser_profile_enabledis true (agent/agent_init.py:1363-1375) and injects itsMEMORY.md/USER.mdsnapshots independently of tool availability (agent/system_prompt.py:460-469). That leaves the multi-user isolation rationale incomplete. agent._builtin_memory_tool_enabledis assigned in the proposedagent/agent_init.py:1065, but the proposed filter independently reloads config inmodel_tools.py; the stored value has no reader.- The new provider test omits the built-in-memory enable flags and does not test prompt injection, so it misses the deployment state described in the PR.
Suggested changes
- Decide whether this flag is schema-only or disables the built-in memory subsystem; if it is for isolation, gate the global-store prompt path too while preserving provider tools.
- Add a temp-
HERMES_HOMEintegration test covering provider tools, built-in schema visibility, and built-in prompt blocks.
Automated hermes-sweeper review.
|
|
||
| # Ask the registry for schemas (only returns tools whose check_fn passes) | ||
| filtered_tools = registry.get_definitions(tools_to_include, quiet=quiet_mode) | ||
| filtered_tools = _filter_builtin_memory_tool( |
There was a problem hiding this comment.
This removes only the API schema. With memory_enabled or user_profile_enabled still true, current main loads the global store in agent/agent_init.py:1363-1375 and injects its snapshots in agent/system_prompt.py:460-469; that does not fully address the multi-user isolation rationale. Please define whether this flag is schema-only or must also gate built-in prompt/store behavior.
| @@ -1064,6 +1065,7 @@ def init_agent( | |||
| agent._memory_store = None | |||
There was a problem hiding this comment.
This attribute is not read by this PR: the schema path independently reloads config in model_tools.py. Please remove it or use one shared decision path so the new configuration cannot silently diverge between agent state and tool construction.
|
Fixed on main via #90559 (salvage of #90413). You were among the earliest (Jun 5) to propose hiding the built-in memory tool while keeping external providers — thank you, credited here. The landed approach gates via the existing |
Closes #39492
What does this PR do?
Adds
memory.builtin_tool.enabledto allow deployments to hide the built-in globalmemorytool while keeping external memory providers enabled.