mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 01:53:33 +00:00
🧷 fix: Address Codex re-review on memory capability (round 7)
- 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.
This commit is contained in:
parent
24311a11d4
commit
e5d56a920c
4 changed files with 71 additions and 64 deletions
|
|
@ -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<unknown> }} [agent]
|
||||
* @param {{ tools?: Array<unknown>, 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,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -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<string | undefined>}
|
||||
* @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 };
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue