From e5d56a920c2b42f96ce4351ac7c329fc7d4481b8 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 21 Jun 2026 00:21:40 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B7=20fix:=20Address=20Codex=20re-revi?= =?UTF-8?q?ew=20on=20memory=20capability=20(round=207)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Recognize inline memory across both execution-context agent shapes: initializeAgent now sets a LibreChat-only memoryToolsRegistered flag on the InitializedAgent, and the opt-in/detection checks accept that flag OR the raw 'memory' marker. Fixes memory failing for processAddedConvo agents (which store the initialized config, marker already expanded) while staying MCP-name-collision-safe (api/app/clients/tools/util/handleTools.js, packages/api/src/agents/initialize.ts, api/server/controllers/agents/client.js). - Scope keyed memory context to memory-enabled agents only: useMemory now returns both keyed and unkeyed contexts, and buildMessages injects the keyed one (memory keys + token metadata) only to agents that can call delete_memory, while the primary/post-turn path keeps the unkeyed values — so a primary without memory tools no longer sees memory keys it doesn't need. --- api/app/clients/tools/util/handleTools.js | 14 ++-- api/server/controllers/agents/client.js | 85 ++++++++------------ api/server/controllers/agents/client.test.js | 26 ++++-- packages/api/src/agents/initialize.ts | 10 ++- 4 files changed, 71 insertions(+), 64 deletions(-) diff --git a/api/app/clients/tools/util/handleTools.js b/api/app/clients/tools/util/handleTools.js index 4383251ebf..62240adccd 100644 --- a/api/app/clients/tools/util/handleTools.js +++ b/api/app/clients/tools/util/handleTools.js @@ -86,20 +86,24 @@ async function isMemoryToolUsable(req, writePermissions = []) { } /** - * Whether the agent opted into the LibreChat inline memory capability, checked - * via the `memory` capability marker on the agent's tools. The raw agent kept - * in the tool-execution context retains that marker (not the expanded - * `set_memory`/`delete_memory` names), so this: + * Whether the agent opted into the LibreChat inline memory capability. The + * tool-execution context may hold the raw agent (still carrying the `memory` + * capability marker on `tools`) or the initialized config (marker already + * expanded into `set_memory`/`delete_memory`, with `memoryToolsRegistered` + * set). Both are LibreChat-only signals, so this: * - refuses a hallucinated/undeclared memory call on a non-memory agent, and * - does not let an MCP tool that merely shares the `set_memory`/`delete_memory` * name get replaced by the built-in memory mutator. - * @param {{ tools?: Array }} [agent] + * @param {{ tools?: Array, memoryToolsRegistered?: boolean }} [agent] * @returns {boolean} */ function agentOptedIntoMemory(agent) { if (!agent) { return false; } + if (agent.memoryToolsRegistered === true) { + return true; + } return (agent.tools ?? []).some( (entry) => (typeof entry === 'string' ? entry : entry?.name) === Tools.memory, ); diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 056bb969d8..f28cc12dda 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -17,8 +17,6 @@ const { omitTitleOptions, getProviderConfig, memoryInstructions, - SET_MEMORY_TOOL_NAME, - DELETE_MEMORY_TOOL_NAME, createTokenCounter, applyContextToAgent, isMemoryAgentEnabled, @@ -86,29 +84,24 @@ const loadAgent = (params) => loadAgentFn(params, { getAgent: db.getAgent, getMC const MEMORY_INPUT_CHARS_PER_TOKEN = 8; -/** Names that mark inline memory tooling on an initialized agent. After - * `initializeAgent` the `memory` capability marker is expanded into the - * `set_memory`/`delete_memory` definitions, so a string `includes('memory')` - * on `agent.tools` (now loaded instances/definitions) no longer matches. */ -const INLINE_MEMORY_TOOL_NAMES = new Set([ - AgentCapabilities.memory, - SET_MEMORY_TOOL_NAME, - DELETE_MEMORY_TOOL_NAME, -]); - /** - * Whether an initialized agent carries the inline memory tools, checked across - * both its loaded tool instances and its serializable tool definitions. - * @param {Agent & { toolDefinitions?: Array<{ name?: string }> }} [agent] + * Whether an agent carries the inline memory tools. Prefers the authoritative + * `memoryToolsRegistered` flag set by `initializeAgent` (LibreChat-only, so it + * never matches an MCP tool that merely shares the `set_memory`/`delete_memory` + * name), falling back to the raw `memory` capability marker on `tools`. + * @param {Agent & { memoryToolsRegistered?: boolean }} [agent] * @returns {boolean} */ function agentHasInlineMemoryTools(agent) { if (!agent) { return false; } - const matches = (entry) => - INLINE_MEMORY_TOOL_NAMES.has(typeof entry === 'string' ? entry : entry?.name); - return (agent.toolDefinitions ?? []).some(matches) || (agent.tools ?? []).some(matches); + if (agent.memoryToolsRegistered === true) { + return true; + } + return (agent.tools ?? []).some( + (entry) => (typeof entry === 'string' ? entry : entry?.name) === AgentCapabilities.memory, + ); } class AgentClient extends BaseClient { @@ -486,11 +479,14 @@ class AgentClient extends BaseClient { } } - /** Memory context (user preferences/memories) */ - const withoutKeys = await this.useMemory(); - const memoryContext = withoutKeys - ? `${memoryInstructions}\n\n# Existing memory about the user:\n${withoutKeys}` - : undefined; + /** Memory context (user preferences/memories). Keyed context (with memory + * keys + token metadata) is reserved for agents that can call + * `delete_memory`; everyone else gets the unkeyed values only. */ + const memories = await this.useMemory(); + const buildMemoryContext = (text) => + text ? `${memoryInstructions}\n\n# Existing memory about the user:\n${text}` : undefined; + const memoryContext = buildMemoryContext(memories?.withoutKeys); + const keyedMemoryContext = buildMemoryContext(memories?.withKeys); const sharedRunContext = sharedRunContextParts.join('\n\n'); const memoryAgentEnabled = isMemoryAgentEnabled(this.options.req.config?.memory); @@ -541,13 +537,13 @@ class AgentClient extends BaseClient { await Promise.all( allAgents.map(({ agent, agentId }) => { const agentRunContextParts = [sharedRunContext]; + const agentHasMemory = agentHasInlineMemoryTools(agent); + const agentMemoryContext = agentHasMemory ? keyedMemoryContext : memoryContext; if ( - memoryContext && - (agentId === this.options.agent.id || - memoryAgentEnabled || - agentHasInlineMemoryTools(agent)) + agentMemoryContext && + (agentId === this.options.agent.id || memoryAgentEnabled || agentHasMemory) ) { - agentRunContextParts.push(memoryContext); + agentRunContextParts.push(agentMemoryContext); } const scopedContext = agentScopedContext.get(agentId); if (scopedContext) { @@ -598,7 +594,7 @@ class AgentClient extends BaseClient { } /** - * @returns {Promise} + * @returns {Promise<{ withKeys?: string; withoutKeys?: string } | undefined>} */ async useMemory() { const user = this.options.req.user; @@ -627,19 +623,10 @@ class AgentClient extends BaseClient { const userId = this.options.req.user.id + ''; this.processMemory = undefined; - /** Inline memory tools (the `memory` capability) let an agent read/write - * memory itself, so the run needs the keyed memory context — otherwise - * `delete_memory` has no visible key to target. Checked across the primary - * and every sub-agent, since a handoff agent may carry memory alone. */ - const agentHasMemoryTools = [ - this.options.agent, - ...(this.agentConfigs ? Array.from(this.agentConfigs.values()) : []), - ].some(agentHasInlineMemoryTools); - if (!isMemoryAgentEnabled(memoryConfig)) { try { const { withKeys, withoutKeys } = await db.getFormattedMemories({ userId }); - return agentHasMemoryTools ? withKeys : withoutKeys; + return { withKeys, withoutKeys }; } catch (error) { logger.error( '[api/server/controllers/agents/client.js #useMemory] Error loading memories', @@ -756,18 +743,16 @@ class AgentClient extends BaseClient { }); this.processMemory = processMemory; - if (agentHasMemoryTools) { - try { - const { withKeys } = await db.getFormattedMemories({ userId }); - return withKeys; - } catch (error) { - logger.error( - '[api/server/controllers/agents/client.js #useMemory] Error loading keyed memories', - error, - ); - } + let withKeys = withoutKeys; + try { + ({ withKeys } = await db.getFormattedMemories({ userId })); + } catch (error) { + logger.error( + '[api/server/controllers/agents/client.js #useMemory] Error loading keyed memories', + error, + ); } - return withoutKeys; + return { withKeys, withoutKeys }; } /** diff --git a/api/server/controllers/agents/client.test.js b/api/server/controllers/agents/client.test.js index c71ede7b23..1320e362c1 100644 --- a/api/server/controllers/agents/client.test.js +++ b/api/server/controllers/agents/client.test.js @@ -2361,7 +2361,9 @@ describe('AgentClient - titleConvo', () => { it('should only pass memory context to the primary agent by default', async () => { const memoryContent = 'User prefers dark mode. User is a software developer.'; - client.useMemory = jest.fn().mockResolvedValue(memoryContent); + client.useMemory = jest + .fn() + .mockResolvedValue({ withKeys: memoryContent, withoutKeys: memoryContent }); const parallelAgent1 = { id: 'parallel-agent-1', @@ -2414,7 +2416,9 @@ describe('AgentClient - titleConvo', () => { it('should pass memory context to parallel agents when automatic memory updates are enabled', async () => { const memoryContent = 'User prefers dark mode. User is a software developer.'; - client.useMemory = jest.fn().mockResolvedValue(memoryContent); + client.useMemory = jest + .fn() + .mockResolvedValue({ withKeys: memoryContent, withoutKeys: memoryContent }); mockReq.config.memory.agent = { enabled: true, id: 'memory-agent', @@ -2485,7 +2489,9 @@ describe('AgentClient - titleConvo', () => { it('should handle parallel agents without existing instructions when memory stays primary-only', async () => { const memoryContent = 'User is a data scientist.'; - client.useMemory = jest.fn().mockResolvedValue(memoryContent); + client.useMemory = jest + .fn() + .mockResolvedValue({ withKeys: memoryContent, withoutKeys: memoryContent }); const parallelAgentNoInstructions = { id: 'parallel-agent-no-instructions', @@ -2521,7 +2527,9 @@ describe('AgentClient - titleConvo', () => { it('should not modify agentConfigs when none exist', async () => { const memoryContent = 'User prefers concise responses.'; - client.useMemory = jest.fn().mockResolvedValue(memoryContent); + client.useMemory = jest + .fn() + .mockResolvedValue({ withKeys: memoryContent, withoutKeys: memoryContent }); client.agentConfigs = null; @@ -2547,7 +2555,9 @@ describe('AgentClient - titleConvo', () => { it('should handle empty agentConfigs map', async () => { const memoryContent = 'User likes detailed explanations.'; - client.useMemory = jest.fn().mockResolvedValue(memoryContent); + client.useMemory = jest + .fn() + .mockResolvedValue({ withKeys: memoryContent, withoutKeys: memoryContent }); client.agentConfigs = new Map(); @@ -2720,7 +2730,7 @@ describe('AgentClient - titleConvo', () => { const result = await client.useMemory(); - expect(result).toBe('likes pasta'); + expect(result).toEqual({ withKeys: 'food: likes pasta', withoutKeys: 'likes pasta' }); expect(mockGetFormattedMemories).toHaveBeenCalledWith({ userId: 'user-123' }); expect(mockInitializeAgent).not.toHaveBeenCalled(); expect(mockCreateMemoryProcessor).not.toHaveBeenCalled(); @@ -2745,7 +2755,7 @@ describe('AgentClient - titleConvo', () => { const result = await client.useMemory(); - expect(result).toBe(''); + expect(result).toEqual({ withKeys: '', withoutKeys: '' }); expect(mockGetFormattedMemories).toHaveBeenCalledWith({ userId: 'user-123' }); expect(mockInitializeAgent).not.toHaveBeenCalled(); expect(mockCreateMemoryProcessor).not.toHaveBeenCalled(); @@ -2770,7 +2780,7 @@ describe('AgentClient - titleConvo', () => { const result = await client.useMemory(); - expect(result).toBe('prefers concise answers'); + expect(result).toEqual({ withKeys: 'tone: concise', withoutKeys: 'prefers concise answers' }); expect(mockLoadAgent).not.toHaveBeenCalled(); expect(mockInitializeAgent).not.toHaveBeenCalled(); expect(mockCreateMemoryProcessor).not.toHaveBeenCalled(); diff --git a/packages/api/src/agents/initialize.ts b/packages/api/src/agents/initialize.ts index cc7c44e26d..2d3f2606a4 100644 --- a/packages/api/src/agents/initialize.ts +++ b/packages/api/src/agents/initialize.ts @@ -256,6 +256,12 @@ export type InitializedAgent = Agent & { toolDefinitions?: LCTool[]; /** Precomputed flag indicating if any tools have defer_loading enabled (for efficient runtime checks) */ hasDeferredTools?: boolean; + /** Whether the inline memory tools (`set_memory`/`delete_memory`) were + * registered for this agent. Authoritative LibreChat-only signal of the + * inline memory opt-in for the execution path, since some contexts hold the + * initialized config (the `memory` marker already expanded out of `tools`) + * rather than the raw agent document. */ + memoryToolsRegistered?: boolean; /** Whether the actions capability is enabled (resolved during tool loading) */ actionsEnabled?: boolean; /** Maximum characters allowed in a single tool result before truncation. */ @@ -1058,7 +1064,8 @@ export async function initializeAgent( * per-agent opt-in. The runtime instances are created in the tool service. */ const agentRequestsMemory = (agent.tools ?? []).includes(Tools.memory); - if (params.memoryAvailable === true && agentRequestsMemory) { + const inlineMemoryRegistered = params.memoryAvailable === true && agentRequestsMemory; + if (inlineMemoryRegistered) { const memoryResult = registerMemoryTools({ toolRegistry, toolDefinitions, @@ -1248,6 +1255,7 @@ export async function initializeAgent( hasDeferredTools, actionsEnabled, baseContextTokens, + memoryToolsRegistered: inlineMemoryRegistered, codeEnvAvailable: effectiveCodeEnvAvailable, reasoningKey: customEndpointConfig?.customParams?.reasoningKey, skillAuthoringAvailable,