diff --git a/src/commands/memory.js b/src/commands/memory.js index e3d1b5a2c..87f0f8326 100644 --- a/src/commands/memory.js +++ b/src/commands/memory.js @@ -25,10 +25,10 @@ import { } from 'discord.js'; import { info, warn } from '../logger.js'; import { + checkAndRecoverMemory, deleteAllMemories, deleteMemory, getMemories, - isMemoryAvailable, searchMemories, } from '../modules/memory.js'; import { isOptedOut, toggleOptOut } from '../modules/optout.js'; @@ -113,7 +113,7 @@ export async function execute(interaction) { return; } - if (!isMemoryAvailable()) { + if (!checkAndRecoverMemory()) { await interaction.reply({ content: '๐Ÿง  Memory system is currently unavailable. The bot still works, just without long-term memory.', @@ -261,26 +261,45 @@ async function handleForgetAll(interaction, userId, username) { async function handleForgetTopic(interaction, userId, username, topic) { await interaction.deferReply({ ephemeral: true }); - // Search for memories matching the topic (results include IDs) - const { memories: matches } = await searchMemories(userId, topic, 10); + const BATCH_SIZE = 100; + const MAX_ITERATIONS = 10; + let totalDeleted = 0; + let totalFound = 0; + let iterations = 0; - if (matches.length === 0) { - await interaction.editReply({ - content: `๐Ÿ” No memories found matching "${topic}".`, - }); - return; - } + // Loop to delete all matching memories (not just the first batch) + while (iterations < MAX_ITERATIONS) { + iterations++; + const { memories: matches } = await searchMemories(userId, topic, BATCH_SIZE); + + if (matches.length === 0) break; + totalFound += matches.length; + + const matchesWithIds = matches.filter( + (m) => m.id !== undefined && m.id !== null && m.id !== '', + ); - // Use memory IDs directly from search results and delete in parallel - const matchesWithIds = matches.filter((m) => m.id); - const results = await Promise.allSettled(matchesWithIds.map((m) => deleteMemory(m.id))); - const deletedCount = results.filter((r) => r.status === 'fulfilled' && r.value === true).length; + if (matchesWithIds.length === 0) break; - if (deletedCount > 0) { + const results = await Promise.allSettled(matchesWithIds.map((m) => deleteMemory(m.id))); + const batchDeleted = results.filter((r) => r.status === 'fulfilled' && r.value === true).length; + totalDeleted += batchDeleted; + + // If we got fewer results than the batch size, we've reached the end + if (matches.length < BATCH_SIZE) break; + // If nothing was deleted this round, stop to avoid infinite loop + if (batchDeleted === 0) break; + } + + if (totalDeleted > 0) { await interaction.editReply({ - content: `๐Ÿงน Forgot ${deletedCount} memor${deletedCount === 1 ? 'y' : 'ies'} related to "${topic}".`, + content: `๐Ÿงน Forgot ${totalDeleted} memor${totalDeleted === 1 ? 'y' : 'ies'} related to "${topic}".`, + }); + info('Topic memories cleared', { userId, username, topic, count: totalDeleted }); + } else if (totalFound === 0) { + await interaction.editReply({ + content: `๐Ÿ” No memories found matching "${topic}".`, }); - info('Topic memories cleared', { userId, username, topic, count: deletedCount }); } else { await interaction.editReply({ content: `โŒ Found memories about "${topic}" but couldn't delete them. Please try again.`, @@ -309,7 +328,7 @@ async function handleAdmin(interaction, subcommand) { return; } - if (!isMemoryAvailable()) { + if (!checkAndRecoverMemory()) { await interaction.reply({ content: '๐Ÿง  Memory system is currently unavailable. The bot still works, just without long-term memory.', diff --git a/src/index.js b/src/index.js index d5a4cc778..8e77036b4 100644 --- a/src/index.js +++ b/src/index.js @@ -28,7 +28,7 @@ import { } from './modules/ai.js'; import { loadConfig } from './modules/config.js'; import { registerEventHandlers } from './modules/events.js'; -import { checkMem0Health } from './modules/memory.js'; +import { checkMem0Health, markUnavailable } from './modules/memory.js'; import { startTempbanScheduler, stopTempbanScheduler } from './modules/moderation.js'; import { loadOptOuts } from './modules/optout.js'; import { HealthMonitor } from './utils/health.js'; @@ -293,8 +293,26 @@ async function startup() { // Load opt-out preferences from DB before enabling memory features await loadOptOuts(); - // Check mem0 availability for user memory features - await checkMem0Health(); + // Check mem0 availability for user memory features (with timeout to avoid blocking startup). + // AbortController prevents a late-resolving health check from calling markAvailable() + // after the timeout has already called markUnavailable(). + const healthAbort = new AbortController(); + try { + await Promise.race([ + checkMem0Health({ signal: healthAbort.signal }), + new Promise((_, reject) => + setTimeout(() => { + healthAbort.abort(); + reject(new Error('mem0 health check timed out')); + }, 10_000), + ), + ]); + } catch (err) { + markUnavailable(); + warn('mem0 health check timed out or failed โ€” continuing without memory features', { + error: err.message, + }); + } // Register event handlers with live config reference registerEventHandlers(client, config, healthMonitor); diff --git a/src/modules/ai.js b/src/modules/ai.js index 1d7a285d9..61e8b8387 100644 --- a/src/modules/ai.js +++ b/src/modules/ai.js @@ -400,15 +400,20 @@ You're witty, knowledgeable about programming and tech, and always eager to help Keep responses concise and Discord-friendly (under 2000 chars). You can use Discord markdown formatting.`; - // Pre-response: inject user memory context into system prompt + // Pre-response: inject user memory context into system prompt (with timeout) if (userId) { try { - const memoryContext = await buildMemoryContext(userId, username, userMessage); + const memoryContext = await Promise.race([ + buildMemoryContext(userId, username, userMessage), + new Promise((_, reject) => + setTimeout(() => reject(new Error('Memory context timeout')), 5000), + ), + ]); if (memoryContext) { systemPrompt += memoryContext; } } catch (err) { - // Memory lookup failed โ€” continue without it + // Memory lookup failed or timed out โ€” continue without it logWarn('Memory context lookup failed', { userId, error: err.message }); } } diff --git a/src/modules/memory.js b/src/modules/memory.js index 3e5f49bf2..f9ad16276 100644 --- a/src/modules/memory.js +++ b/src/modules/memory.js @@ -99,7 +99,7 @@ let client = null; * After RECOVERY_COOLDOWN_MS, the next request will be allowed through * to check if the service has recovered. */ -function markUnavailable() { +export function markUnavailable() { mem0Available = false; mem0UnavailableSince = Date.now(); } @@ -154,15 +154,30 @@ export function getMemoryConfig() { } /** - * Check if memory feature is enabled and mem0 is available. - * Supports auto-recovery: if mem0 was marked unavailable due to a transient - * error and the cooldown period has elapsed, it will be tentatively re-enabled - * so the next request can check if the service has recovered. + * Pure availability check โ€” no side effects. + * Returns true only if memory is both enabled in config and currently marked available. + * Does NOT trigger auto-recovery. Use {@link checkAndRecoverMemory} when you want + * the cooldown-based recovery logic. * @returns {boolean} */ export function isMemoryAvailable() { const memConfig = getMemoryConfig(); if (!memConfig.enabled) return false; + return mem0Available; +} + +/** + * Check if memory feature is enabled and mem0 is available, with auto-recovery. + * If mem0 was marked unavailable due to a transient error and the cooldown period + * has elapsed, this will tentatively re-enable it so the next request can check + * if the service has recovered. + * + * Use this instead of {@link isMemoryAvailable} when you want the recovery side effect. + * @returns {boolean} + */ +export function checkAndRecoverMemory() { + const memConfig = getMemoryConfig(); + if (!memConfig.enabled) return false; if (mem0Available) return true; @@ -231,9 +246,12 @@ export function _setClient(newClient) { * Run a health check against the mem0 platform on startup. * Verifies the API key is configured and the SDK client can actually * communicate with the hosted platform by performing a lightweight search. + * @param {object} [options] + * @param {AbortSignal} [options.signal] - When aborted, prevents a late-resolving + * check from calling {@link markAvailable} (guards against race with startup timeout). * @returns {Promise} true if mem0 is ready */ -export async function checkMem0Health() { +export async function checkMem0Health({ signal } = {}) { const memConfig = getMemoryConfig(); if (!memConfig.enabled) { info('Memory module disabled via config'); @@ -262,6 +280,11 @@ export async function checkMem0Health() { limit: 1, }); + // Guard against late resolution after a startup timeout has already + // called markUnavailable(). If the caller's AbortSignal has fired, + // the timeout won the race and we must not flip availability back on. + if (signal?.aborted) return false; + markAvailable(); info('mem0 health check passed (SDK connectivity verified)'); return true; @@ -286,7 +309,7 @@ export async function checkMem0Health() { * @returns {Promise} true if stored successfully */ export async function addMemory(userId, text, metadata = {}) { - if (!isMemoryAvailable()) return false; + if (!checkAndRecoverMemory()) return false; try { const c = getClient(); @@ -318,7 +341,7 @@ export async function addMemory(userId, text, metadata = {}) { * @returns {Promise<{memories: Array<{memory: string, score?: number}>, relations: Array}>} */ export async function searchMemories(userId, query, limit) { - if (!isMemoryAvailable()) return { memories: [], relations: [] }; + if (!checkAndRecoverMemory()) return { memories: [], relations: [] }; const memConfig = getMemoryConfig(); const maxResults = limit ?? memConfig.maxContextMemories; @@ -339,7 +362,7 @@ export async function searchMemories(userId, query, limit) { const relations = result?.relations || []; const memories = rawMemories.map((m) => ({ - id: m.id || '', + id: m.id ?? '', memory: m.memory || m.text || m.content || '', score: m.score ?? null, })); @@ -358,7 +381,7 @@ export async function searchMemories(userId, query, limit) { * @returns {Promise>} All user memories */ export async function getMemories(userId) { - if (!isMemoryAvailable()) return []; + if (!checkAndRecoverMemory()) return []; try { const c = getClient(); @@ -373,7 +396,7 @@ export async function getMemories(userId) { const memories = Array.isArray(result) ? result : result?.results || []; return memories.map((m) => ({ - id: m.id || '', + id: m.id ?? '', memory: m.memory || m.text || m.content || '', })); } catch (err) { @@ -389,7 +412,7 @@ export async function getMemories(userId) { * @returns {Promise} true if deleted successfully */ export async function deleteAllMemories(userId) { - if (!isMemoryAvailable()) return false; + if (!checkAndRecoverMemory()) return false; try { const c = getClient(); @@ -411,7 +434,7 @@ export async function deleteAllMemories(userId) { * @returns {Promise} true if deleted successfully */ export async function deleteMemory(memoryId) { - if (!isMemoryAvailable()) return false; + if (!checkAndRecoverMemory()) return false; try { const c = getClient(); @@ -435,21 +458,29 @@ export async function deleteMemory(memoryId) { export function formatRelations(relations) { if (!relations || relations.length === 0) return ''; - const lines = relations.map((r) => `- ${r.source} โ†’ ${r.relationship} โ†’ ${r.target}`); + const lines = relations + .filter((r) => r.source && r.relationship && r.target) + .map((r) => `- ${r.source} โ†’ ${r.relationship} โ†’ ${r.target}`); + + if (lines.length === 0) return ''; return `\nRelationships:\n${lines.join('\n')}`; } +/** Maximum characters for memory context injected into system prompt */ +const MAX_MEMORY_CONTEXT_CHARS = 2000; + /** * Build a context string from user memories to inject into the system prompt. * Includes both regular memories and graph relations for richer context. + * Enforces a character budget to prevent oversized system prompts. * @param {string} userId - Discord user ID * @param {string} username - Display name * @param {string} query - The user's current message (for relevance search) * @returns {Promise} Context string or empty string */ export async function buildMemoryContext(userId, username, query) { - if (!isMemoryAvailable()) return ''; + if (!checkAndRecoverMemory()) return ''; if (isOptedOut(userId)) return ''; const { memories, relations } = await searchMemories(userId, query); @@ -468,6 +499,11 @@ export async function buildMemoryContext(userId, username, query) { context += relationsContext; } + // Enforce character budget to prevent oversized system prompts + if (context.length > MAX_MEMORY_CONTEXT_CHARS) { + context = `${context.substring(0, MAX_MEMORY_CONTEXT_CHARS)}...`; + } + return context; } @@ -482,7 +518,7 @@ export async function buildMemoryContext(userId, username, query) { * @returns {Promise} true if any memories were stored */ export async function extractAndStoreMemories(userId, username, userMessage, assistantReply) { - if (!isMemoryAvailable()) return false; + if (!checkAndRecoverMemory()) return false; if (isOptedOut(userId)) return false; const memConfig = getMemoryConfig(); @@ -493,13 +529,14 @@ export async function extractAndStoreMemories(userId, username, userMessage, ass if (!c) return false; const messages = [ - { role: 'user', content: `${username}: ${userMessage}` }, + { role: 'user', content: userMessage }, { role: 'assistant', content: assistantReply }, ]; await c.add(messages, { user_id: userId, app_id: APP_ID, + metadata: { username }, enable_graph: true, }); @@ -510,8 +547,10 @@ export async function extractAndStoreMemories(userId, username, userMessage, ass }); return true; } catch (err) { + // Only log โ€” do NOT call markUnavailable() here. + // This runs fire-and-forget in the background; a failure for one user's + // extraction should not disable the memory system for all other users. logWarn('Memory extraction failed', { userId, error: err.message }); - if (!isTransientError(err)) markUnavailable(); return false; } } diff --git a/tests/commands/memory.test.js b/tests/commands/memory.test.js index b4cf5e15a..2fa8cb1d1 100644 --- a/tests/commands/memory.test.js +++ b/tests/commands/memory.test.js @@ -177,7 +177,7 @@ vi.mock('../../src/utils/splitMessage.js', () => ({ // Mock memory module vi.mock('../../src/modules/memory.js', () => ({ - isMemoryAvailable: vi.fn(() => true), + checkAndRecoverMemory: vi.fn(() => true), getMemories: vi.fn(() => Promise.resolve([])), deleteAllMemories: vi.fn(() => Promise.resolve(true)), searchMemories: vi.fn(() => Promise.resolve({ memories: [], relations: [] })), @@ -201,10 +201,10 @@ vi.mock('../../src/logger.js', () => ({ import { PermissionFlagsBits } from 'discord.js'; import { data, execute } from '../../src/commands/memory.js'; import { + checkAndRecoverMemory, deleteAllMemories, deleteMemory, getMemories, - isMemoryAvailable, searchMemories, } from '../../src/modules/memory.js'; import { isOptedOut, toggleOptOut } from '../../src/modules/optout.js'; @@ -253,7 +253,7 @@ function createMockInteraction({ describe('memory command', () => { beforeEach(() => { vi.clearAllMocks(); - isMemoryAvailable.mockReturnValue(true); + checkAndRecoverMemory.mockReturnValue(true); getMemories.mockResolvedValue([]); deleteAllMemories.mockResolvedValue(true); searchMemories.mockResolvedValue({ memories: [], relations: [] }); @@ -271,7 +271,7 @@ describe('memory command', () => { describe('unavailable state', () => { it('should reply with unavailable message when memory is not available', async () => { - isMemoryAvailable.mockReturnValue(false); + checkAndRecoverMemory.mockReturnValue(false); const interaction = createMockInteraction(); await execute(interaction); @@ -369,7 +369,7 @@ describe('memory command', () => { }); it('should work even when memory system is unavailable', async () => { - isMemoryAvailable.mockReturnValue(false); + checkAndRecoverMemory.mockReturnValue(false); toggleOptOut.mockReturnValue({ optedOut: true }); const interaction = createMockInteraction({ subcommand: 'optout' }); @@ -513,7 +513,7 @@ describe('memory command', () => { await execute(interaction); expect(interaction.deferReply).toHaveBeenCalledWith({ ephemeral: true }); - expect(searchMemories).toHaveBeenCalledWith('123456', 'Rust', 10); + expect(searchMemories).toHaveBeenCalledWith('123456', 'Rust', 100); expect(deleteMemory).toHaveBeenCalledWith('mem-1'); expect(interaction.editReply).toHaveBeenCalledWith( expect.objectContaining({ @@ -561,6 +561,86 @@ describe('memory command', () => { ); }); + it('should not drop memories with falsy but valid IDs (e.g. numeric 0)', async () => { + searchMemories.mockResolvedValue({ + memories: [ + { id: 0, memory: 'Memory with numeric zero ID', score: 0.9 }, + { id: 'mem-2', memory: 'Normal ID memory', score: 0.8 }, + ], + relations: [], + }); + deleteMemory.mockResolvedValue(true); + + const interaction = createMockInteraction({ + subcommand: 'forget', + topic: 'test', + }); + + await execute(interaction); + + expect(deleteMemory).toHaveBeenCalledTimes(2); + expect(deleteMemory).toHaveBeenCalledWith(0); + expect(deleteMemory).toHaveBeenCalledWith('mem-2'); + }); + + it('should skip memories with empty string, null, or undefined IDs', async () => { + searchMemories.mockResolvedValue({ + memories: [ + { id: '', memory: 'Empty string ID', score: 0.9 }, + { id: null, memory: 'Null ID', score: 0.8 }, + { memory: 'No ID field', score: 0.7 }, + { id: 'valid-id', memory: 'Valid ID', score: 0.6 }, + ], + relations: [], + }); + deleteMemory.mockResolvedValue(true); + + const interaction = createMockInteraction({ + subcommand: 'forget', + topic: 'test', + }); + + await execute(interaction); + + expect(deleteMemory).toHaveBeenCalledTimes(1); + expect(deleteMemory).toHaveBeenCalledWith('valid-id'); + }); + + it('should loop to delete all matching memories across multiple batches', async () => { + // First call returns a full batch (simulating more results exist) + searchMemories + .mockResolvedValueOnce({ + memories: Array.from({ length: 100 }, (_, i) => ({ + id: `mem-batch1-${i}`, + memory: `Batch 1 memory ${i}`, + score: 0.9, + })), + relations: [], + }) + .mockResolvedValueOnce({ + memories: [{ id: 'mem-batch2-0', memory: 'Batch 2 last one', score: 0.8 }], + relations: [], + }); + deleteMemory.mockResolvedValue(true); + + const interaction = createMockInteraction({ + subcommand: 'forget', + topic: 'test', + }); + + await execute(interaction); + + // Should have called search twice (second batch < 100 = done) + expect(searchMemories).toHaveBeenCalledTimes(2); + // 100 from batch 1 + 1 from batch 2 + expect(deleteMemory).toHaveBeenCalledTimes(101); + expect(interaction.editReply).toHaveBeenCalledWith( + expect.objectContaining({ + content: expect.stringContaining('101 memories'), + }), + ); + }); + it('should report correct count for multiple parallel deletions', async () => { searchMemories.mockResolvedValue({ memories: [ @@ -689,7 +769,7 @@ describe('memory command', () => { }); it('should reply unavailable when memory system is down', async () => { - isMemoryAvailable.mockReturnValue(false); + checkAndRecoverMemory.mockReturnValue(false); const interaction = createMockInteraction({ subcommand: 'view', subcommandGroup: 'admin', diff --git a/tests/index.test.js b/tests/index.test.js index 6d958658e..a2221a2a0 100644 --- a/tests/index.test.js +++ b/tests/index.test.js @@ -57,6 +57,11 @@ const mocks = vi.hoisted(() => ({ getPermissionError: vi.fn(), }, + memory: { + checkMem0Health: vi.fn().mockResolvedValue(false), + markUnavailable: vi.fn(), + }, + registerCommands: vi.fn(), dotenvConfig: vi.fn(), })); @@ -143,7 +148,8 @@ vi.mock('../src/modules/events.js', () => ({ })); vi.mock('../src/modules/memory.js', () => ({ - checkMem0Health: vi.fn().mockResolvedValue(false), + checkMem0Health: mocks.memory.checkMem0Health, + markUnavailable: mocks.memory.markUnavailable, })); vi.mock('../src/modules/moderation.js', () => ({ @@ -184,6 +190,7 @@ async function importIndex({ readdirFiles = [], loadConfigReject = null, throwOnExit = true, + checkMem0HealthImpl = null, } = {}) { vi.resetModules(); @@ -238,6 +245,13 @@ async function importIndex({ mocks.health.getInstance.mockReset().mockReturnValue({}); mocks.permissions.hasPermission.mockReset().mockReturnValue(true); mocks.permissions.getPermissionError.mockReset().mockReturnValue('nope'); + mocks.memory.checkMem0Health.mockReset(); + if (checkMem0HealthImpl) { + mocks.memory.checkMem0Health.mockImplementation(checkMem0HealthImpl); + } else { + mocks.memory.checkMem0Health.mockResolvedValue(false); + } + mocks.memory.markUnavailable.mockReset(); mocks.registerCommands.mockReset().mockResolvedValue(undefined); mocks.dotenvConfig.mockReset(); @@ -308,6 +322,33 @@ describe('index.js', () => { expect(mocks.client.login).toHaveBeenCalledWith('abc'); }); + it('should call markUnavailable when checkMem0Health rejects', async () => { + await importIndex({ + token: 'abc', + databaseUrl: null, + checkMem0HealthImpl: () => Promise.reject(new Error('mem0 health check timed out')), + }); + + expect(mocks.memory.markUnavailable).toHaveBeenCalled(); + expect(mocks.logger.warn).toHaveBeenCalledWith( + 'mem0 health check timed out or failed โ€” continuing without memory features', + { error: 'mem0 health check timed out' }, + ); + // Startup should still complete despite the failure + expect(mocks.client.login).toHaveBeenCalledWith('abc'); + }); + + it('should not call markUnavailable when checkMem0Health succeeds', async () => { + await importIndex({ + token: 'abc', + databaseUrl: null, + checkMem0HealthImpl: () => Promise.resolve(true), + }); + + expect(mocks.memory.markUnavailable).not.toHaveBeenCalled(); + expect(mocks.client.login).toHaveBeenCalledWith('abc'); + }); + it('should load state from disk when state file exists', async () => { await importIndex({ token: 'abc', databaseUrl: null, stateFile: true }); expect(mocks.ai.setConversationHistory).toHaveBeenCalled(); diff --git a/tests/modules/ai.test.js b/tests/modules/ai.test.js index a30ecbb3c..ad1b2b56f 100644 --- a/tests/modules/ai.test.js +++ b/tests/modules/ai.test.js @@ -305,6 +305,37 @@ describe('ai module', () => { }); }); + it('should timeout memory context lookup after 5 seconds', async () => { + vi.useFakeTimers(); + + // buildMemoryContext never resolves + buildMemoryContext.mockImplementation(() => new Promise(() => {})); + + const mockResponse = { + ok: true, + json: vi.fn().mockResolvedValue({ + choices: [{ message: { content: 'Still working without memory!' } }], + }), + }; + vi.spyOn(globalThis, 'fetch').mockResolvedValue(mockResponse); + + const config = { ai: { systemPrompt: 'You are a bot.' } }; + const replyPromise = generateResponse('ch1', 'Hi', 'user', config, null, 'user-123'); + + // Advance past the 5s timeout + await vi.advanceTimersByTimeAsync(5000); + + const reply = await replyPromise; + expect(reply).toBe('Still working without memory!'); + + // System prompt should NOT contain memory context + const fetchCall = globalThis.fetch.mock.calls[0]; + const body = JSON.parse(fetchCall[1].body); + expect(body.messages[0].content).toBe('You are a bot.'); + + vi.useRealTimers(); + }); + it('should continue working when memory context lookup fails', async () => { buildMemoryContext.mockRejectedValue(new Error('mem0 down')); diff --git a/tests/modules/memory.test.js b/tests/modules/memory.test.js index 28b4b9c90..e4e05be98 100644 --- a/tests/modules/memory.test.js +++ b/tests/modules/memory.test.js @@ -38,6 +38,7 @@ import { _setMem0Available, addMemory, buildMemoryContext, + checkAndRecoverMemory, checkMem0Health, deleteAllMemories, deleteMemory, @@ -46,6 +47,7 @@ import { getMemories, getMemoryConfig, isMemoryAvailable, + markUnavailable, searchMemories, } from '../../src/modules/memory.js'; import { isOptedOut } from '../../src/modules/optout.js'; @@ -131,26 +133,64 @@ describe('memory module', () => { }); }); - describe('isMemoryAvailable', () => { + describe('isMemoryAvailable (pure โ€” no side effects)', () => { + it('should return false when mem0 is not available', () => { + _setMem0Available(false); + expect(isMemoryAvailable()).toBe(false); + }); + + it('should return true when enabled and available', () => { + _setMem0Available(true); + expect(isMemoryAvailable()).toBe(true); + }); + + it('should return false when disabled in config', () => { + _setMem0Available(true); + getConfig.mockReturnValue({ memory: { enabled: false } }); + expect(isMemoryAvailable()).toBe(false); + }); + + it('should NOT auto-recover (no side effects)', async () => { + vi.useFakeTimers(); + _setMem0Available(true); + + const failingClient = createMockClient({ + add: vi.fn().mockRejectedValue(new Error('API error')), + }); + _setClient(failingClient); + + // This will fail and call markUnavailable() + await addMemory('user123', 'test'); + expect(isMemoryAvailable()).toBe(false); + + // Advance time past the cooldown โ€” pure check should still return false + vi.advanceTimersByTime(_getRecoveryCooldownMs()); + expect(isMemoryAvailable()).toBe(false); + + vi.useRealTimers(); + }); + }); + + describe('checkAndRecoverMemory (with auto-recovery side effect)', () => { afterEach(() => { // Ensure fake timers never leak into other tests, even if a test fails mid-way vi.useRealTimers(); }); - it('should return false when mem0 is not available', () => { + it('should return false when mem0 is not available and cooldown not expired', () => { _setMem0Available(false); - expect(isMemoryAvailable()).toBe(false); + expect(checkAndRecoverMemory()).toBe(false); }); it('should return true when enabled and available', () => { _setMem0Available(true); - expect(isMemoryAvailable()).toBe(true); + expect(checkAndRecoverMemory()).toBe(true); }); it('should return false when disabled in config', () => { _setMem0Available(true); getConfig.mockReturnValue({ memory: { enabled: false } }); - expect(isMemoryAvailable()).toBe(false); + expect(checkAndRecoverMemory()).toBe(false); }); it('should auto-recover after cooldown period expires', async () => { @@ -164,13 +204,13 @@ describe('memory module', () => { // This will fail and call markUnavailable() await addMemory('user123', 'test'); - expect(isMemoryAvailable()).toBe(false); + expect(checkAndRecoverMemory()).toBe(false); // Advance time past the cooldown vi.advanceTimersByTime(_getRecoveryCooldownMs()); // Should now auto-recover - expect(isMemoryAvailable()).toBe(true); + expect(checkAndRecoverMemory()).toBe(true); }); it('should not auto-recover before cooldown expires', async () => { @@ -183,15 +223,15 @@ describe('memory module', () => { // Trigger a failure to markUnavailable await addMemory('user123', 'test'); - expect(isMemoryAvailable()).toBe(false); + expect(checkAndRecoverMemory()).toBe(false); // Advance time but not enough vi.advanceTimersByTime(_getRecoveryCooldownMs() - 1000); - expect(isMemoryAvailable()).toBe(false); + expect(checkAndRecoverMemory()).toBe(false); // Now advance past the cooldown vi.advanceTimersByTime(1000); - expect(isMemoryAvailable()).toBe(true); + expect(checkAndRecoverMemory()).toBe(true); }); it('should re-disable if recovery attempt also fails', async () => { @@ -205,15 +245,58 @@ describe('memory module', () => { // Trigger initial failure await addMemory('user123', 'test'); - expect(isMemoryAvailable()).toBe(false); + expect(checkAndRecoverMemory()).toBe(false); // Advance past cooldown - auto-recovery kicks in vi.advanceTimersByTime(_getRecoveryCooldownMs()); - expect(isMemoryAvailable()).toBe(true); + expect(checkAndRecoverMemory()).toBe(true); // But the next operation also fails await searchMemories('user123', 'test'); + expect(checkAndRecoverMemory()).toBe(false); + }); + }); + + describe('markUnavailable', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it('should enable auto-recovery by setting unavailable timestamp', () => { + vi.useFakeTimers(); + _setMem0Available(true); + expect(isMemoryAvailable()).toBe(true); + + // Calling markUnavailable should disable memory and set the timestamp + markUnavailable(); expect(isMemoryAvailable()).toBe(false); + expect(checkAndRecoverMemory()).toBe(false); + + // After cooldown expires, auto-recovery should kick in + vi.advanceTimersByTime(_getRecoveryCooldownMs()); + expect(checkAndRecoverMemory()).toBe(true); + }); + + it('should allow auto-recovery even when called from initial state', () => { + // Simulates the health check timeout scenario: mem0Available starts false, + // mem0UnavailableSince starts at 0. Without markUnavailable(), auto-recovery + // can never trigger because checkAndRecoverMemory requires + // mem0UnavailableSince > 0. + vi.useFakeTimers(); + _setMem0Available(false); + // At this point, _setMem0Available(false) hard-disables (sets timestamp to 0) + expect(checkAndRecoverMemory()).toBe(false); + + // Now call markUnavailable to set the recovery timestamp + markUnavailable(); + expect(isMemoryAvailable()).toBe(false); + + // Still can't recover before cooldown + expect(checkAndRecoverMemory()).toBe(false); + + // After cooldown, should auto-recover + vi.advanceTimersByTime(_getRecoveryCooldownMs()); + expect(checkAndRecoverMemory()).toBe(true); }); }); @@ -372,6 +455,22 @@ describe('memory module', () => { expect(result).toBe(false); expect(isMemoryAvailable()).toBe(false); }); + + it('should not call markAvailable when signal is already aborted', async () => { + process.env.MEM0_API_KEY = 'test-api-key'; + const mockClient = createMockClient({ + search: vi.fn().mockResolvedValue({ results: [], relations: [] }), + }); + _setClient(mockClient); + + // Simulate: startup timeout fired before health check resolved + const controller = new AbortController(); + controller.abort(); + + const result = await checkMem0Health({ signal: controller.signal }); + expect(result).toBe(false); + expect(isMemoryAvailable()).toBe(false); + }); }); describe('addMemory', () => { @@ -536,6 +635,28 @@ describe('memory module', () => { const result = await searchMemories('user123', 'test'); expect(result).toEqual({ memories: [], relations: [] }); }); + + it('should preserve falsy-but-valid IDs like 0', async () => { + _setMem0Available(true); + const mockClient = createMockClient({ + search: vi.fn().mockResolvedValue({ + results: [ + { id: 0, memory: 'zero id memory', score: 0.9 }, + { id: '', memory: 'empty string id memory', score: 0.8 }, + { id: null, memory: 'null id memory', score: 0.7 }, + ], + relations: [], + }), + }); + _setClient(mockClient); + + const result = await searchMemories('user123', 'test'); + expect(result.memories).toEqual([ + { id: 0, memory: 'zero id memory', score: 0.9 }, + { id: '', memory: 'empty string id memory', score: 0.8 }, + { id: '', memory: 'null id memory', score: 0.7 }, + ]); + }); }); describe('getMemories', () => { @@ -599,6 +720,27 @@ describe('memory module', () => { const result = await getMemories('user123'); expect(result).toEqual([]); }); + + it('should preserve falsy-but-valid IDs like 0', async () => { + _setMem0Available(true); + const mockClient = createMockClient({ + getAll: vi.fn().mockResolvedValue({ + results: [ + { id: 0, memory: 'zero id memory' }, + { id: '', memory: 'empty string id memory' }, + { id: null, memory: 'null id memory' }, + ], + }), + }); + _setClient(mockClient); + + const result = await getMemories('user123'); + expect(result).toEqual([ + { id: 0, memory: 'zero id memory' }, + { id: '', memory: 'empty string id memory' }, + { id: '', memory: 'null id memory' }, + ]); + }); }); describe('deleteAllMemories', () => { @@ -687,6 +829,33 @@ describe('memory module', () => { expect(formatRelations([])).toBe(''); }); + it('should skip relations with missing source, relationship, or target', () => { + const relations = [ + { source: 'Joe', relationship: 'likes', target: 'pizza' }, + { source: undefined, relationship: 'works at', target: 'Google' }, + { source: 'Joe', relationship: undefined, target: 'cats' }, + { source: 'Joe', relationship: 'owns', target: undefined }, + { source: null, relationship: null, target: null }, + {}, + ]; + + const result = formatRelations(relations); + expect(result).toContain('Joe โ†’ likes โ†’ pizza'); + expect(result).not.toContain('undefined'); + expect(result).not.toContain('null'); + // Only one valid line + expect(result.match(/^- /gm)).toHaveLength(1); + }); + + it('should return empty string when all relations have missing properties', () => { + const relations = [ + { source: undefined, relationship: 'likes', target: 'pizza' }, + { source: 'Joe', relationship: undefined, target: 'cats' }, + ]; + + expect(formatRelations(relations)).toBe(''); + }); + it('should format relations as readable lines', () => { const relations = [ { @@ -794,6 +963,40 @@ describe('memory module', () => { expect(result).not.toContain('What you know about'); }); + it('should truncate memory context when it exceeds 2000 characters', async () => { + _setMem0Available(true); + // Create memories that will exceed 2000 chars + const longMemories = Array.from({ length: 50 }, (_, i) => ({ + memory: `This is a very long memory entry number ${i} with lots of detail about the user preferences and interests that takes up significant space in the context`, + score: 0.9, + })); + const mockClient = createMockClient({ + search: vi.fn().mockResolvedValue({ + results: longMemories, + relations: [], + }), + }); + _setClient(mockClient); + + const result = await buildMemoryContext('user123', 'testuser', 'test'); + expect(result.length).toBeLessThanOrEqual(2003); // 2000 + "..." + expect(result).toMatch(/\.\.\.$/); + }); + + it('should not truncate memory context within budget', async () => { + _setMem0Available(true); + const mockClient = createMockClient({ + search: vi.fn().mockResolvedValue({ + results: [{ memory: 'Short memory', score: 0.9 }], + relations: [], + }), + }); + _setClient(mockClient); + + const result = await buildMemoryContext('user123', 'testuser', 'test'); + expect(result).not.toMatch(/\.\.\.$/); + }); + it('should return context with only memories when no relations found', async () => { _setMem0Available(true); const mockClient = createMockClient({ @@ -857,18 +1060,19 @@ describe('memory module', () => { expect(mockClient.add).toHaveBeenCalledWith( [ - { role: 'user', content: "testuser: I'm learning Rust" }, + { role: 'user', content: "I'm learning Rust" }, { role: 'assistant', content: 'Rust is awesome! What project are you working on?' }, ], { user_id: 'user123', app_id: 'bills-bot', + metadata: { username: 'testuser' }, enable_graph: true, }, ); }); - it('should return false on SDK failure and mark unavailable', async () => { + it('should return false on SDK failure but NOT mark unavailable (fire-and-forget safety)', async () => { _setMem0Available(true); const mockClient = createMockClient({ add: vi.fn().mockRejectedValue(new Error('API error')), @@ -877,7 +1081,8 @@ describe('memory module', () => { const result = await extractAndStoreMemories('user123', 'testuser', 'hi', 'hello'); expect(result).toBe(false); - expect(isMemoryAvailable()).toBe(false); + // Should still be available โ€” background failures must not disable the system + expect(isMemoryAvailable()).toBe(true); }); it('should return false when client is null', async () => {