From 366036a197983ec1a797abc15ee2607b1150d385 Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Thu, 16 Apr 2026 10:19:49 -0700 Subject: [PATCH] feat(core): introduce MemoryProvider seam and MemoryService orchestrator Refactors the existing background skill extractor into a layered memory subsystem that GeminiClient can drive uniformly across interactive and non-interactive sessions. - Adds an internal `MemoryProvider` interface (not exported, not surfaced via extensions) as a typing seam between the orchestrator and its concrete implementation - Refactors the existing skill-extraction logic into `DefaultMemoryProvider`, the sole implementation of the seam - Introduces `MemoryService` as a thin orchestrator that owns a `DefaultMemoryProvider` and wraps every lifecycle call in try/catch so a buggy provider can't crash a turn - Moves MemoryService ownership into `GeminiClient` (private field exposed via optional methods) so non-interactive entry points share the same lifecycle as the interactive UI - Keeps builtin memory extraction asynchronous on startup to avoid blocking the first turn - Adds unit tests for MemoryService, DefaultMemoryProvider, and the GeminiClient integration The whole subsystem remains gated behind the existing `isMemoryManagerEnabled()` experimental flag. --- packages/cli/src/nonInteractiveCli.ts | 1 + .../cli/src/nonInteractiveCliAgentSession.ts | 1 + packages/cli/src/ui/AppContainer.tsx | 9 +- packages/core/src/commands/memory.ts | 2 +- packages/core/src/core/client.test.ts | 144 ++ packages/core/src/core/client.ts | 216 ++- packages/core/src/index.ts | 5 +- .../services/defaultMemoryProvider.test.ts | 1462 +++++++++++++++++ .../src/services/defaultMemoryProvider.ts | 756 +++++++++ packages/core/src/services/memoryProvider.ts | 50 + .../core/src/services/memoryService.test.ts | 1386 ++-------------- packages/core/src/services/memoryService.ts | 837 +--------- 12 files changed, 2801 insertions(+), 2068 deletions(-) create mode 100644 packages/core/src/services/defaultMemoryProvider.test.ts create mode 100644 packages/core/src/services/defaultMemoryProvider.ts create mode 100644 packages/core/src/services/memoryProvider.ts diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index dc5255edee..aa1eda6144 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -529,6 +529,7 @@ export async function runNonInteractive( // Cleanup stdin cancellation before other cleanup cleanupStdinCancellation(); + await config.getGeminiClient()?.shutdownSessionServices?.(); scheduler?.dispose(); consolePatcher.cleanup(); coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback); diff --git a/packages/cli/src/nonInteractiveCliAgentSession.ts b/packages/cli/src/nonInteractiveCliAgentSession.ts index 4fee7eb610..f6e164bff5 100644 --- a/packages/cli/src/nonInteractiveCliAgentSession.ts +++ b/packages/cli/src/nonInteractiveCliAgentSession.ts @@ -616,6 +616,7 @@ export async function runNonInteractive({ cleanupStdinCancellation(); abortController.signal.removeEventListener('abort', abortSession); + await config.getGeminiClient()?.shutdownSessionServices?.(); scheduler?.dispose(); consolePatcher.cleanup(); coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback); diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index f17ac0d756..8a23ccfd15 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -92,7 +92,6 @@ import { ApiKeyUpdatedEvent, LegacyAgentProtocol, type InjectionSource, - startMemoryService, } from '@google/gemini-cli-core'; import { validateAuthMethod } from '../config/auth.js'; import process from 'node:process'; @@ -482,13 +481,6 @@ export const AppContainer = (props: AppContainerProps) => { setConfigInitialized(true); startupProfiler.flush(config); - // Fire-and-forget memory service (skill extraction from past sessions) - if (config.isMemoryManagerEnabled()) { - startMemoryService(config).catch((e) => { - debugLogger.error('Failed to start memory service:', e); - }); - } - const sessionStartSource = resumedSessionData ? SessionStartSource.Resume : SessionStartSource.Startup; @@ -540,6 +532,7 @@ export const AppContainer = (props: AppContainerProps) => { // Fire SessionEnd hook on cleanup (only if hooks are enabled) await config?.getHookSystem()?.fireSessionEndEvent(SessionEndReason.Exit); + await config?.getGeminiClient()?.shutdownSessionServices?.(); }; registerCleanup(cleanupFn); diff --git a/packages/core/src/commands/memory.ts b/packages/core/src/commands/memory.ts index 286cbe0e3e..74d20df757 100644 --- a/packages/core/src/commands/memory.ts +++ b/packages/core/src/commands/memory.ts @@ -20,7 +20,7 @@ import { isProjectSkillPatchTarget, validateParsedSkillPatchHeaders, } from '../services/memoryPatchUtils.js'; -import { readExtractionState } from '../services/memoryService.js'; +import { readExtractionState } from '../services/defaultMemoryProvider.js'; import { refreshServerHierarchicalMemory } from '../utils/memoryDiscovery.js'; import type { MessageActionReturn, ToolActionReturn } from './types.js'; diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index e28ea9cfa4..67ece91111 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -34,6 +34,7 @@ import { import { getCoreSystemPrompt } from './prompts.js'; import { DEFAULT_GEMINI_MODEL_AUTO } from '../config/models.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; +import { MemoryService } from '../services/memoryService.js'; import { setSimulate429 } from '../utils/testUtils.js'; import { tokenLimit } from './tokenLimits.js'; import { ideContextStore } from '../ide/ideContext.js'; @@ -94,6 +95,29 @@ interface MockTurnContext { } const mockTurnRunFn = vi.fn(); +const memoryServiceMocks = vi.hoisted(() => { + const onSessionStart = vi.fn(); + const getSystemInstructions = vi.fn().mockResolvedValue(''); + const getTurnContext = vi.fn().mockResolvedValue(''); + const onTurnComplete = vi.fn(); + const onSessionEnd = vi.fn(); + const factory = vi.fn(() => ({ + onSessionStart, + getSystemInstructions, + getTurnContext, + onTurnComplete, + onSessionEnd, + })); + + return { + onSessionStart, + getSystemInstructions, + getTurnContext, + onTurnComplete, + onSessionEnd, + factory, + }; +}); vi.mock('./turn', async (importOriginal) => { const actual = await importOriginal(); @@ -117,6 +141,9 @@ vi.mock('./turn', async (importOriginal) => { }); vi.mock('../config/config.js'); +vi.mock('../services/memoryService.js', () => ({ + MemoryService: vi.fn().mockImplementation(() => memoryServiceMocks.factory()), +})); vi.mock('./prompts'); vi.mock('../utils/getFolderStructure', () => ({ getFolderStructure: vi.fn().mockResolvedValue('Mock Folder Structure'), @@ -172,6 +199,15 @@ describe('Gemini Client (client.ts)', () => { vi.resetAllMocks(); ClearcutLogger.clearInstance(); vi.mocked(uiTelemetryService.setLastPromptTokenCount).mockClear(); + vi.mocked(MemoryService).mockImplementation( + () => memoryServiceMocks.factory() as unknown as MemoryService, + ); + memoryServiceMocks.factory.mockClear(); + memoryServiceMocks.onSessionStart.mockReset(); + memoryServiceMocks.getSystemInstructions.mockReset().mockResolvedValue(''); + memoryServiceMocks.getTurnContext.mockReset().mockResolvedValue(''); + memoryServiceMocks.onTurnComplete.mockReset(); + memoryServiceMocks.onSessionEnd.mockReset(); mockGenerateContentFn = vi.fn().mockResolvedValue({ candidates: [{ content: { parts: [{ text: '{"key": "value"}' }] } }], @@ -231,6 +267,8 @@ describe('Gemini Client (client.ts)', () => { minPrunableThresholdTokens: 30000, protectLatestTurn: true, }), + getExtensions: vi.fn().mockReturnValue([]), + isMemoryManagerEnabled: vi.fn().mockReturnValue(false), getSessionId: vi.fn().mockReturnValue('test-session-id'), getProxy: vi.fn().mockReturnValue(undefined), @@ -431,6 +469,112 @@ describe('Gemini Client (client.ts)', () => { }); }); + describe('memory service integration', () => { + it('does not construct the memory service when the experimental flag is off', async () => { + vi.mocked(mockConfig.isMemoryManagerEnabled).mockReturnValue(false); + + client.dispose(); + client = new GeminiClient(mockConfig as unknown as AgentLoopContext); + await client.initialize(); + + expect(memoryServiceMocks.factory).not.toHaveBeenCalled(); + expect(memoryServiceMocks.onSessionStart).not.toHaveBeenCalled(); + }); + + it('initializes the memory service and injects system instructions when the experimental flag is on', async () => { + vi.mocked(mockConfig.isMemoryManagerEnabled).mockReturnValue(true); + memoryServiceMocks.getSystemInstructions.mockResolvedValue( + 'Memory instructions', + ); + vi.mocked(getCoreSystemPrompt).mockReturnValue('Base prompt'); + + client.dispose(); + client = new GeminiClient(mockConfig as unknown as AgentLoopContext); + await client.initialize(); + + const mockChat = { + setSystemInstruction: vi.fn(), + } as unknown as GeminiChat; + client['chat'] = mockChat; + client.updateSystemInstruction(); + + expect(memoryServiceMocks.factory).toHaveBeenCalledTimes(1); + expect(memoryServiceMocks.onSessionStart).toHaveBeenCalledWith( + 'test-session-id', + ); + expect(mockChat.setSystemInstruction).toHaveBeenCalledWith( + 'Base prompt\n\n\nMemory instructions\n', + ); + }); + + it('injects turn context and completes the prompt lifecycle after the final turn', async () => { + vi.mocked(mockConfig.isMemoryManagerEnabled).mockReturnValue(true); + memoryServiceMocks.getTurnContext.mockResolvedValue('recalled context'); + mockTurnRunFn.mockReturnValue( + (async function* () { + yield { + type: GeminiEventType.Finished, + value: { reason: 'STOP', usageMetadata: undefined }, + }; + })(), + ); + + const stream = client.sendMessageStream( + [{ text: 'How do I deploy this?' }], + new AbortController().signal, + 'prompt-id-memory', + ); + + await fromAsync(stream); + + expect(memoryServiceMocks.getTurnContext).toHaveBeenCalledWith( + 'How do I deploy this?', + ); + expect(mockTurnRunFn).toHaveBeenCalledWith( + { model: 'default-routed-model', isChatModel: true }, + [ + { text: 'How do I deploy this?' }, + { text: 'recalled context' }, + ], + expect.any(AbortSignal), + undefined, + ); + expect(memoryServiceMocks.onTurnComplete).toHaveBeenCalledWith( + 'How do I deploy this?', + 'Mock Response', + ); + }); + + it('invokes onSessionEnd when shutdownSessionServices is called', async () => { + vi.mocked(mockConfig.isMemoryManagerEnabled).mockReturnValue(true); + + client.dispose(); + client = new GeminiClient(mockConfig as unknown as AgentLoopContext); + await client.initialize(); + + await client.shutdownSessionServices(); + + expect(memoryServiceMocks.onSessionEnd).toHaveBeenCalledTimes(1); + }); + + it('skips the memory service for subagent-scoped clients', async () => { + vi.mocked(mockConfig.isMemoryManagerEnabled).mockReturnValue(true); + + client.dispose(); + const subagentContext = Object.assign(Object.create(mockConfig), { + config: mockConfig, + promptId: 'subagent-prompt-id', + parentSessionId: 'main-prompt-id', + }) as AgentLoopContext; + const subagentClient = new GeminiClient(subagentContext); + + await subagentClient.initialize(); + + expect(memoryServiceMocks.factory).not.toHaveBeenCalled(); + subagentClient.dispose(); + }); + }); + describe('tryCompressChat', () => { const mockGetHistory = vi.fn(); diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 25509862fb..506268d04f 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -43,6 +43,7 @@ import type { } from '../services/chatRecordingService.js'; import type { ContentGenerator } from './contentGenerator.js'; import { LoopDetectionService } from '../services/loopDetectionService.js'; +import { MemoryService } from '../services/memoryService.js'; import { ChatCompressionService } from '../context/chatCompressionService.js'; import { AgentHistoryProvider } from '../context/agentHistoryProvider.js'; import { ideContextStore } from '../ide/ideContext.js'; @@ -89,6 +90,14 @@ type BeforeAgentHookReturn = | { additionalContext: string | undefined } | undefined; +interface PromptLifecycleState { + originalUserMessage: string; +} + +interface SendMessageStreamOptions { + internalContinuation?: boolean; +} + export class GeminiClient { private chat?: GeminiChat; private sessionTurnCount = 0; @@ -107,6 +116,10 @@ export class GeminiClient { * being forced and did it fail? */ private hasFailedCompressionAttempt = false; + private memoryService?: MemoryService; + private memorySystemInstructions = ''; + private memorySessionStarted = false; + private promptLifecycleStateMap = new Map(); constructor(private readonly context: AgentLoopContext) { this.loopDetector = new LoopDetectionService(this.config); @@ -248,6 +261,150 @@ export class GeminiClient { } } + private shouldUseMemoryService(): boolean { + if (this.context.parentSessionId) { + return false; + } + // The entire memory subsystem (built-in extractor and any + // extension-contributed providers) is gated behind a single + // experimental flag while the architecture stabilizes. + return this.config.isMemoryManagerEnabled(); + } + + private getOrCreateMemoryService(): MemoryService | undefined { + if (!this.shouldUseMemoryService()) { + return undefined; + } + + if (!this.memoryService) { + this.memoryService = new MemoryService(this.config); + } + + return this.memoryService; + } + + private async refreshMemorySystemInstructions(): Promise { + if (!this.memoryService) { + this.memorySystemInstructions = ''; + return; + } + + this.memorySystemInstructions = + await this.memoryService.getSystemInstructions(); + } + + private async ensureMemoryServiceInitialized(): Promise { + const memoryService = this.getOrCreateMemoryService(); + if (!memoryService) { + return; + } + + if (!this.memorySessionStarted) { + this.memorySessionStarted = true; + await memoryService.onSessionStart(this.config.getSessionId()); + } + + await this.refreshMemorySystemInstructions(); + } + + private buildSystemInstruction(): string { + const systemMemory = this.config.getSystemInstructionMemory(); + const systemInstruction = getCoreSystemPrompt(this.config, systemMemory); + const memoryInstructions = this.memorySystemInstructions.trim(); + + if (!memoryInstructions) { + return systemInstruction; + } + + return `${systemInstruction}\n\n\n${memoryInstructions}\n`; + } + + private getOrCreatePromptLifecycleState( + prompt_id: string, + request: PartListUnion, + ): PromptLifecycleState { + let state = this.promptLifecycleStateMap.get(prompt_id); + if (!state) { + state = { + originalUserMessage: partListUnionToString(request), + }; + this.promptLifecycleStateMap.set(prompt_id, state); + } + + return state; + } + + private clearPromptLifecycleState(prompt_id: string): void { + this.promptLifecycleStateMap.delete(prompt_id); + } + + private requestHasFunctionResponse(request: PartListUnion): boolean { + const parts = Array.isArray(request) ? request : [request]; + return parts.some( + (part) => + typeof part === 'object' && part !== null && 'functionResponse' in part, + ); + } + + private appendMemoryContextToRequest( + request: PartListUnion, + memoryContext: string, + ): PartListUnion { + const trimmedContext = memoryContext.trim(); + if (!trimmedContext) { + return request; + } + + const requestParts = Array.isArray(request) ? request : [request]; + return [ + ...requestParts, + { text: `${trimmedContext}` }, + ]; + } + + private async withMemoryTurnContext( + request: PartListUnion, + originalUserMessage: string, + isInternalContinuation: boolean, + ): Promise { + if (isInternalContinuation || this.requestHasFunctionResponse(request)) { + return request; + } + + await this.ensureMemoryServiceInitialized(); + if (!this.memoryService || !originalUserMessage.trim()) { + return request; + } + + const memoryContext = + await this.memoryService.getTurnContext(originalUserMessage); + + return this.appendMemoryContextToRequest(request, memoryContext); + } + + private async finalizePromptLifecycle( + prompt_id: string, + turn: Turn, + ): Promise { + try { + const assistantMessage = turn.getResponseText() || ''; + const lifecycleState = this.promptLifecycleStateMap.get(prompt_id); + const userMessage = lifecycleState?.originalUserMessage ?? ''; + + if ( + this.memoryService && + (assistantMessage.trim().length > 0 || turn.finishReason !== undefined) + ) { + // Fire-and-forget by contract — provider does its own async work. + this.memoryService.onTurnComplete(userMessage, assistantMessage); + await this.refreshMemorySystemInstructions(); + this.updateSystemInstruction(); + } + } finally { + this.clearPromptLifecycleState(prompt_id); + } + } + async initialize() { this.chat = await this.startChat(); this.updateTelemetryTokenCount(); @@ -356,9 +513,7 @@ export class GeminiClient { return; } - const systemMemory = this.config.getSystemInstructionMemory(); - const systemInstruction = getCoreSystemPrompt(this.config, systemMemory); - this.getChat().setSystemInstruction(systemInstruction); + this.getChat().setSystemInstruction(this.buildSystemInstruction()); } async startChat( @@ -376,8 +531,8 @@ export class GeminiClient { const history = await getInitialChatHistory(this.config, extraHistory); try { - const systemMemory = this.config.getSystemInstructionMemory(); - const systemInstruction = getCoreSystemPrompt(this.config, systemMemory); + await this.ensureMemoryServiceInitialized(); + const systemInstruction = this.buildSystemInstruction(); const chat = new GeminiChat( this.config, systemInstruction, @@ -839,6 +994,8 @@ export class GeminiClient { boundedTurns - 1, true, displayContent, + false, + { internalContinuation: true }, ); return turn; } @@ -872,6 +1029,8 @@ export class GeminiClient { boundedTurns - 1, false, // isInvalidStreamRetry is false displayContent, + false, + { internalContinuation: true }, ); return turn; } @@ -888,7 +1047,14 @@ export class GeminiClient { isInvalidStreamRetry: boolean = false, displayContent?: PartListUnion, stopHookActive: boolean = false, + options?: SendMessageStreamOptions, ): AsyncGenerator { + const isInternalContinuation = options?.internalContinuation ?? false; + const promptLifecycleState = this.getOrCreatePromptLifecycleState( + prompt_id, + displayContent ?? request, + ); + if (!isInvalidStreamRetry) { this.config.resetTurn(); } @@ -910,6 +1076,9 @@ export class GeminiClient { 'type' in hookResult && hookResult.type === GeminiEventType.AgentExecutionStopped ) { + if (!isInternalContinuation) { + this.clearPromptLifecycleState(prompt_id); + } // Add user message to history before returning so it's kept in the transcript this.getChat().addHistory(createUserContent(request)); yield hookResult; @@ -918,6 +1087,9 @@ export class GeminiClient { 'type' in hookResult && hookResult.type === GeminiEventType.AgentExecutionBlocked ) { + if (!isInternalContinuation) { + this.clearPromptLifecycleState(prompt_id); + } yield hookResult; return new Turn(this.getChat(), prompt_id); } else if ('additionalContext' in hookResult) { @@ -933,6 +1105,12 @@ export class GeminiClient { } } + request = await this.withMemoryTurnContext( + request, + promptLifecycleState.originalUserMessage, + isInternalContinuation, + ); + const boundedTurns = Math.min(turns, MAX_TURNS); let turn = new Turn(this.getChat(), prompt_id); let continuationHandled = false; @@ -1008,11 +1186,15 @@ export class GeminiClient { false, displayContent, true, // stopHookActive: signal retry to AfterAgent hooks + { internalContinuation: true }, ); } } } catch (error) { if (signal?.aborted || isAbortError(error)) { + if (!isInternalContinuation) { + this.clearPromptLifecycleState(prompt_id); + } yield { type: GeminiEventType.UserCancelled }; return turn; } @@ -1035,9 +1217,31 @@ export class GeminiClient { } } + if ( + !isInternalContinuation && + !turn.pendingToolCalls.length && + !signal.aborted + ) { + await this.finalizePromptLifecycle(prompt_id, turn); + } + return turn; } + async shutdownSessionServices(): Promise { + if (!this.memoryService) { + this.promptLifecycleStateMap.clear(); + return; + } + + try { + await this.memoryService.onSessionEnd(); + await this.refreshMemorySystemInstructions(); + } finally { + this.promptLifecycleStateMap.clear(); + } + } + async generateContent( modelConfigKey: ModelConfigKey, contents: Content[], @@ -1277,6 +1481,8 @@ export class GeminiClient { boundedTurns - 1, isInvalidStreamRetry, displayContent, + false, + { internalContinuation: true }, ); } } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 62a0b127bd..60269ec303 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -141,9 +141,10 @@ export * from './services/modelConfigService.js'; export * from './sandbox/windows/WindowsSandboxManager.js'; export * from './services/sessionSummaryUtils.js'; export { - startMemoryService, + DefaultMemoryProvider, validatePatches, -} from './services/memoryService.js'; +} from './services/defaultMemoryProvider.js'; +export { MemoryService } from './services/memoryService.js'; export { isProjectSkillPatchTarget } from './services/memoryPatchUtils.js'; export * from './context/memoryContextManager.js'; export * from './services/trackerService.js'; diff --git a/packages/core/src/services/defaultMemoryProvider.test.ts b/packages/core/src/services/defaultMemoryProvider.test.ts new file mode 100644 index 0000000000..da2ffd0358 --- /dev/null +++ b/packages/core/src/services/defaultMemoryProvider.test.ts @@ -0,0 +1,1462 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import type { Config } from '../config/config.js'; +import { + SESSION_FILE_PREFIX, + type ConversationRecord, +} from './chatRecordingService.js'; +import type { + ExtractionState, + ExtractionRun, +} from './defaultMemoryProvider.js'; +import { coreEvents } from '../utils/events.js'; +import { Storage } from '../config/storage.js'; +import { debugLogger } from '../utils/debugLogger.js'; + +// Mock external modules used by DefaultMemoryProvider +vi.mock('../agents/local-executor.js', () => ({ + LocalAgentExecutor: { + create: vi.fn().mockResolvedValue({ + run: vi.fn().mockResolvedValue(undefined), + }), + }, +})); + +vi.mock('../agents/skill-extraction-agent.js', () => ({ + SkillExtractionAgent: vi.fn().mockReturnValue({ + name: 'skill-extraction', + promptConfig: { systemPrompt: 'test' }, + tools: [], + outputSchema: {}, + modelConfig: { model: 'test-model' }, + }), +})); + +vi.mock('./executionLifecycleService.js', () => ({ + ExecutionLifecycleService: { + createExecution: vi.fn().mockReturnValue({ pid: 42, result: {} }), + completeExecution: vi.fn(), + }, +})); + +vi.mock('../tools/tool-registry.js', () => ({ + ToolRegistry: vi.fn(), +})); + +vi.mock('../prompts/prompt-registry.js', () => ({ + PromptRegistry: vi.fn(), +})); + +vi.mock('../resources/resource-registry.js', () => ({ + ResourceRegistry: vi.fn(), +})); + +vi.mock('../policy/policy-engine.js', () => ({ + PolicyEngine: vi.fn(), +})); + +vi.mock('../policy/types.js', () => ({ + PolicyDecision: { ALLOW: 'ALLOW' }, +})); + +vi.mock('../confirmation-bus/message-bus.js', () => ({ + MessageBus: vi.fn(), +})); + +vi.mock('../agents/registry.js', () => ({ + getModelConfigAlias: vi.fn().mockReturnValue('skill-extraction-config'), +})); + +vi.mock('../config/storage.js', () => ({ + Storage: { + getUserSkillsDir: vi.fn().mockReturnValue('/tmp/fake-user-skills'), + }, +})); + +vi.mock('../skills/skillLoader.js', () => ({ + FRONTMATTER_REGEX: /^---\n([\s\S]*?)\n---/, + parseFrontmatter: vi.fn().mockReturnValue(null), +})); + +vi.mock('../utils/debugLogger.js', () => ({ + debugLogger: { + debug: vi.fn(), + log: vi.fn(), + warn: vi.fn(), + }, +})); + +vi.mock('../utils/events.js', () => ({ + coreEvents: { + emitFeedback: vi.fn(), + }, +})); + +// Helper to create a minimal ConversationRecord +function createConversation( + overrides: Partial & { messageCount?: number } = {}, +): ConversationRecord { + const { messageCount = 4, ...rest } = overrides; + const messages = Array.from({ length: messageCount }, (_, i) => ({ + id: String(i + 1), + timestamp: new Date().toISOString(), + content: [{ text: `Message ${i + 1}` }], + type: i % 2 === 0 ? ('user' as const) : ('gemini' as const), + })); + return { + sessionId: rest.sessionId ?? `session-${Date.now()}`, + projectHash: 'abc123', + startTime: '2025-01-01T00:00:00Z', + lastUpdated: '2025-01-01T01:00:00Z', + messages, + ...rest, + }; +} + +describe('memoryService', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'skill-extract-test-')); + }); + + afterEach(async () => { + vi.clearAllMocks(); + vi.unstubAllEnvs(); + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + describe('tryAcquireLock', () => { + it('successfully acquires lock when none exists', async () => { + const { tryAcquireLock } = await import('./defaultMemoryProvider.js'); + + const lockPath = path.join(tmpDir, '.extraction.lock'); + const result = await tryAcquireLock(lockPath); + + expect(result).toBe(true); + + const content = JSON.parse(await fs.readFile(lockPath, 'utf-8')); + expect(content.pid).toBe(process.pid); + expect(content.startedAt).toBeDefined(); + }); + + it('returns false when lock is held by a live process', async () => { + const { tryAcquireLock } = await import('./defaultMemoryProvider.js'); + + const lockPath = path.join(tmpDir, '.extraction.lock'); + // Write a lock with the current PID (which is alive) + const lockInfo = { + pid: process.pid, + startedAt: new Date().toISOString(), + }; + await fs.writeFile(lockPath, JSON.stringify(lockInfo)); + + const result = await tryAcquireLock(lockPath); + + expect(result).toBe(false); + }); + + it('cleans up and re-acquires stale lock (dead PID)', async () => { + const { tryAcquireLock } = await import('./defaultMemoryProvider.js'); + + const lockPath = path.join(tmpDir, '.extraction.lock'); + // Use a PID that almost certainly doesn't exist + const lockInfo = { + pid: 2147483646, + startedAt: new Date().toISOString(), + }; + await fs.writeFile(lockPath, JSON.stringify(lockInfo)); + + const result = await tryAcquireLock(lockPath); + + expect(result).toBe(true); + const content = JSON.parse(await fs.readFile(lockPath, 'utf-8')); + expect(content.pid).toBe(process.pid); + }); + + it('cleans up and re-acquires stale lock (too old)', async () => { + const { tryAcquireLock } = await import('./defaultMemoryProvider.js'); + + const lockPath = path.join(tmpDir, '.extraction.lock'); + // Lock from 40 minutes ago with current PID — old enough to be stale (>35min) + const oldDate = new Date(Date.now() - 40 * 60 * 1000).toISOString(); + const lockInfo = { + pid: process.pid, + startedAt: oldDate, + }; + await fs.writeFile(lockPath, JSON.stringify(lockInfo)); + + const result = await tryAcquireLock(lockPath); + + expect(result).toBe(true); + const content = JSON.parse(await fs.readFile(lockPath, 'utf-8')); + expect(content.pid).toBe(process.pid); + // The new lock should have a recent timestamp + const newLockAge = Date.now() - new Date(content.startedAt).getTime(); + expect(newLockAge).toBeLessThan(5000); + }); + }); + + describe('isLockStale', () => { + it('returns true when PID is dead', async () => { + const { isLockStale } = await import('./defaultMemoryProvider.js'); + + const lockPath = path.join(tmpDir, '.extraction.lock'); + const lockInfo = { + pid: 2147483646, + startedAt: new Date().toISOString(), + }; + await fs.writeFile(lockPath, JSON.stringify(lockInfo)); + + expect(await isLockStale(lockPath)).toBe(true); + }); + + it('returns true when lock is too old (>35 min)', async () => { + const { isLockStale } = await import('./defaultMemoryProvider.js'); + + const lockPath = path.join(tmpDir, '.extraction.lock'); + const oldDate = new Date(Date.now() - 40 * 60 * 1000).toISOString(); + const lockInfo = { + pid: process.pid, + startedAt: oldDate, + }; + await fs.writeFile(lockPath, JSON.stringify(lockInfo)); + + expect(await isLockStale(lockPath)).toBe(true); + }); + + it('returns false when PID is alive and lock is fresh', async () => { + const { isLockStale } = await import('./defaultMemoryProvider.js'); + + const lockPath = path.join(tmpDir, '.extraction.lock'); + const lockInfo = { + pid: process.pid, + startedAt: new Date().toISOString(), + }; + await fs.writeFile(lockPath, JSON.stringify(lockInfo)); + + expect(await isLockStale(lockPath)).toBe(false); + }); + + it('returns true when file cannot be read', async () => { + const { isLockStale } = await import('./defaultMemoryProvider.js'); + + const lockPath = path.join(tmpDir, 'nonexistent.lock'); + + expect(await isLockStale(lockPath)).toBe(true); + }); + }); + + describe('releaseLock', () => { + it('deletes the lock file', async () => { + const { releaseLock } = await import('./defaultMemoryProvider.js'); + + const lockPath = path.join(tmpDir, '.extraction.lock'); + await fs.writeFile(lockPath, '{}'); + + await releaseLock(lockPath); + + await expect(fs.access(lockPath)).rejects.toThrow(); + }); + + it('does not throw when file is already gone', async () => { + const { releaseLock } = await import('./defaultMemoryProvider.js'); + + const lockPath = path.join(tmpDir, 'nonexistent.lock'); + + await expect(releaseLock(lockPath)).resolves.not.toThrow(); + }); + }); + + describe('readExtractionState / writeExtractionState', () => { + it('returns default state when file does not exist', async () => { + const { readExtractionState } = await import( + './defaultMemoryProvider.js' + ); + + const statePath = path.join(tmpDir, 'nonexistent-state.json'); + const state = await readExtractionState(statePath); + + expect(state).toEqual({ runs: [] }); + }); + + it('reads existing state file', async () => { + const { readExtractionState } = await import( + './defaultMemoryProvider.js' + ); + + const statePath = path.join(tmpDir, '.extraction-state.json'); + const existingState: ExtractionState = { + runs: [ + { + runAt: '2025-01-01T00:00:00Z', + sessionIds: ['session-1', 'session-2'], + skillsCreated: [], + }, + ], + }; + await fs.writeFile(statePath, JSON.stringify(existingState)); + + const state = await readExtractionState(statePath); + + expect(state).toEqual(existingState); + }); + + it('writes state atomically via temp file + rename', async () => { + const { writeExtractionState, readExtractionState } = await import( + './defaultMemoryProvider.js' + ); + + const statePath = path.join(tmpDir, '.extraction-state.json'); + const state: ExtractionState = { + runs: [ + { + runAt: '2025-01-01T00:00:00Z', + sessionIds: ['session-abc'], + skillsCreated: [], + }, + ], + }; + + await writeExtractionState(statePath, state); + + // Verify the temp file does not linger + const files = await fs.readdir(tmpDir); + expect(files).not.toContain('.extraction-state.json.tmp'); + + // Verify the final file is readable + const readBack = await readExtractionState(statePath); + expect(readBack).toEqual(state); + }); + }); + + describe('DefaultMemoryProvider', () => { + it('skips when lock is held by another instance', async () => { + const { DefaultMemoryProvider } = await import( + './defaultMemoryProvider.js' + ); + const { LocalAgentExecutor } = await import( + '../agents/local-executor.js' + ); + + const memoryDir = path.join(tmpDir, 'memory'); + const skillsDir = path.join(tmpDir, 'skills'); + const projectTempDir = path.join(tmpDir, 'temp'); + await fs.mkdir(memoryDir, { recursive: true }); + + // Pre-acquire the lock with current PID + const lockPath = path.join(memoryDir, '.extraction.lock'); + await fs.writeFile( + lockPath, + JSON.stringify({ + pid: process.pid, + startedAt: new Date().toISOString(), + }), + ); + + const mockConfig = { + storage: { + getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), + getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), + getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), + getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), + }, + getToolRegistry: vi.fn(), + getMessageBus: vi.fn(), + getGeminiClient: vi.fn(), + sandboxManager: undefined, + } as unknown as Config; + + const provider = new DefaultMemoryProvider(); + provider.onSessionStart(mockConfig, 'session-id'); + + await vi.waitFor(() => { + expect(debugLogger.log).toHaveBeenCalledWith( + '[MemoryService] Skipped: lock held by another instance', + ); + }); + + // Agent should never have been created + expect(LocalAgentExecutor.create).not.toHaveBeenCalled(); + }); + + it('starts skill extraction in the background on session start', async () => { + const { DefaultMemoryProvider } = await import( + './defaultMemoryProvider.js' + ); + const { LocalAgentExecutor } = await import( + '../agents/local-executor.js' + ); + const { ExecutionLifecycleService } = await import( + './executionLifecycleService.js' + ); + + const memoryDir = path.join(tmpDir, 'memory-background'); + const skillsDir = path.join(tmpDir, 'skills-background'); + const projectTempDir = path.join(tmpDir, 'temp-background'); + const chatsDir = path.join(projectTempDir, 'chats'); + await fs.mkdir(memoryDir, { recursive: true }); + await fs.mkdir(chatsDir, { recursive: true }); + + const conversation = createConversation({ + sessionId: 'background-session', + messageCount: 20, + }); + await fs.writeFile( + path.join(chatsDir, 'session-2025-01-01T00-00-background.json'), + JSON.stringify(conversation), + ); + + let resolveRun: (() => void) | undefined; + const runFinished = new Promise((resolve) => { + resolveRun = resolve; + }); + const runStarted = vi.fn(); + + vi.mocked(LocalAgentExecutor.create).mockResolvedValueOnce({ + run: vi.fn().mockImplementation(async () => { + runStarted(); + await runFinished; + return undefined; + }), + } as never); + + const mockConfig = { + storage: { + getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), + getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), + getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), + getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), + }, + getToolRegistry: vi.fn(), + getMessageBus: vi.fn(), + getGeminiClient: vi.fn(), + getSkillManager: vi.fn().mockReturnValue({ getSkills: () => [] }), + modelConfigService: { + registerRuntimeModelConfig: vi.fn(), + }, + sandboxManager: undefined, + } as unknown as Config; + + const provider = new DefaultMemoryProvider(); + provider.onSessionStart(mockConfig, 'session-id'); + + await vi.waitFor(() => { + expect(runStarted).toHaveBeenCalledTimes(1); + }); + expect( + ExecutionLifecycleService.completeExecution, + ).not.toHaveBeenCalled(); + + resolveRun?.(); + + await vi.waitFor(() => { + expect(ExecutionLifecycleService.completeExecution).toHaveBeenCalled(); + }); + }); + + it('skips when no unprocessed sessions exist', async () => { + const { DefaultMemoryProvider } = await import( + './defaultMemoryProvider.js' + ); + const { LocalAgentExecutor } = await import( + '../agents/local-executor.js' + ); + + const memoryDir = path.join(tmpDir, 'memory2'); + const skillsDir = path.join(tmpDir, 'skills2'); + const projectTempDir = path.join(tmpDir, 'temp2'); + await fs.mkdir(memoryDir, { recursive: true }); + // Create an empty chats directory + await fs.mkdir(path.join(projectTempDir, 'chats'), { recursive: true }); + + const mockConfig = { + storage: { + getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), + getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), + getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), + getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), + }, + getToolRegistry: vi.fn(), + getMessageBus: vi.fn(), + getGeminiClient: vi.fn(), + sandboxManager: undefined, + } as unknown as Config; + + const provider = new DefaultMemoryProvider(); + provider.onSessionStart(mockConfig, 'session-id'); + + // Lock should be released + const lockPath = path.join(memoryDir, '.extraction.lock'); + await vi.waitFor(async () => { + await expect(fs.access(lockPath)).rejects.toThrow(); + }); + + expect(LocalAgentExecutor.create).not.toHaveBeenCalled(); + }); + + it('releases lock on error', async () => { + const { DefaultMemoryProvider } = await import( + './defaultMemoryProvider.js' + ); + const { LocalAgentExecutor } = await import( + '../agents/local-executor.js' + ); + const { ExecutionLifecycleService } = await import( + './executionLifecycleService.js' + ); + + const memoryDir = path.join(tmpDir, 'memory3'); + const skillsDir = path.join(tmpDir, 'skills3'); + const projectTempDir = path.join(tmpDir, 'temp3'); + const chatsDir = path.join(projectTempDir, 'chats'); + await fs.mkdir(memoryDir, { recursive: true }); + await fs.mkdir(chatsDir, { recursive: true }); + + // Write a valid session that will pass all filters + const conversation = createConversation({ + sessionId: 'error-session', + messageCount: 20, + }); + await fs.writeFile( + path.join(chatsDir, 'session-2025-01-01T00-00-err00001.json'), + JSON.stringify(conversation), + ); + + // Make LocalAgentExecutor.create throw + vi.mocked(LocalAgentExecutor.create).mockRejectedValueOnce( + new Error('Agent creation failed'), + ); + + const mockConfig = { + storage: { + getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), + getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), + getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), + getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), + }, + getToolRegistry: vi.fn(), + getMessageBus: vi.fn(), + getGeminiClient: vi.fn(), + sandboxManager: undefined, + } as unknown as Config; + + const provider = new DefaultMemoryProvider(); + provider.onSessionStart(mockConfig, 'session-id'); + + // Lock should be released despite the error + const lockPath = path.join(memoryDir, '.extraction.lock'); + await vi.waitFor(async () => { + await expect(fs.access(lockPath)).rejects.toThrow(); + }); + + // ExecutionLifecycleService.completeExecution should have been called with error + await vi.waitFor(() => { + expect( + ExecutionLifecycleService.completeExecution, + ).toHaveBeenCalledWith( + 42, + expect.objectContaining({ + error: expect.any(Error), + }), + ); + }); + }); + + it('emits feedback when new skills are created during extraction', async () => { + const { DefaultMemoryProvider } = await import( + './defaultMemoryProvider.js' + ); + const { LocalAgentExecutor } = await import( + '../agents/local-executor.js' + ); + + // Reset mocks that may carry state from prior tests + vi.mocked(coreEvents.emitFeedback).mockClear(); + vi.mocked(LocalAgentExecutor.create).mockReset(); + + const memoryDir = path.join(tmpDir, 'memory4'); + const skillsDir = path.join(tmpDir, 'skills4'); + const projectTempDir = path.join(tmpDir, 'temp4'); + const chatsDir = path.join(projectTempDir, 'chats'); + await fs.mkdir(memoryDir, { recursive: true }); + await fs.mkdir(skillsDir, { recursive: true }); + await fs.mkdir(chatsDir, { recursive: true }); + + // Write a valid session with enough messages to pass the filter + const conversation = createConversation({ + sessionId: 'skill-session', + messageCount: 20, + }); + await fs.writeFile( + path.join(chatsDir, 'session-2025-01-01T00-00-skill001.json'), + JSON.stringify(conversation), + ); + + // Override LocalAgentExecutor.create to return an executor whose run + // creates a new skill directory with a SKILL.md in the skillsDir + vi.mocked(LocalAgentExecutor.create).mockResolvedValueOnce({ + run: vi.fn().mockImplementation(async () => { + const newSkillDir = path.join(skillsDir, 'my-new-skill'); + await fs.mkdir(newSkillDir, { recursive: true }); + await fs.writeFile( + path.join(newSkillDir, 'SKILL.md'), + '# My New Skill', + ); + return undefined; + }), + } as never); + + const mockConfig = { + storage: { + getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), + getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), + getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), + getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), + }, + getToolRegistry: vi.fn(), + getMessageBus: vi.fn(), + getGeminiClient: vi.fn(), + getSkillManager: vi.fn().mockReturnValue({ getSkills: () => [] }), + modelConfigService: { + registerRuntimeModelConfig: vi.fn(), + }, + sandboxManager: undefined, + } as unknown as Config; + + const provider = new DefaultMemoryProvider(); + provider.onSessionStart(mockConfig, 'session-id'); + + await vi.waitFor(() => { + expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + 'info', + expect.stringContaining('my-new-skill'), + ); + expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + 'info', + expect.stringContaining('/memory inbox'), + ); + }); + }); + }); + + describe('getProcessedSessionIds', () => { + it('returns empty set for empty state', async () => { + const { getProcessedSessionIds } = await import( + './defaultMemoryProvider.js' + ); + + const result = getProcessedSessionIds({ runs: [] }); + + expect(result).toBeInstanceOf(Set); + expect(result.size).toBe(0); + }); + + it('collects session IDs across multiple runs', async () => { + const { getProcessedSessionIds } = await import( + './defaultMemoryProvider.js' + ); + + const state: ExtractionState = { + runs: [ + { + runAt: '2025-01-01T00:00:00Z', + sessionIds: ['s1', 's2'], + skillsCreated: [], + }, + { + runAt: '2025-01-02T00:00:00Z', + sessionIds: ['s3'], + skillsCreated: [], + }, + ], + }; + + const result = getProcessedSessionIds(state); + + expect(result).toEqual(new Set(['s1', 's2', 's3'])); + }); + + it('deduplicates IDs that appear in multiple runs', async () => { + const { getProcessedSessionIds } = await import( + './defaultMemoryProvider.js' + ); + + const state: ExtractionState = { + runs: [ + { + runAt: '2025-01-01T00:00:00Z', + sessionIds: ['s1', 's2'], + skillsCreated: [], + }, + { + runAt: '2025-01-02T00:00:00Z', + sessionIds: ['s2', 's3'], + skillsCreated: [], + }, + ], + }; + + const result = getProcessedSessionIds(state); + + expect(result.size).toBe(3); + expect(result).toEqual(new Set(['s1', 's2', 's3'])); + }); + }); + + describe('buildSessionIndex', () => { + let chatsDir: string; + + beforeEach(async () => { + chatsDir = path.join(tmpDir, 'chats'); + await fs.mkdir(chatsDir, { recursive: true }); + }); + + it('returns empty index and no new IDs when chats dir is empty', async () => { + const { buildSessionIndex } = await import('./defaultMemoryProvider.js'); + + const result = await buildSessionIndex(chatsDir, { runs: [] }); + + expect(result.sessionIndex).toBe(''); + expect(result.newSessionIds).toEqual([]); + }); + + it('returns empty index when chats dir does not exist', async () => { + const { buildSessionIndex } = await import('./defaultMemoryProvider.js'); + + const nonexistentDir = path.join(tmpDir, 'no-such-dir'); + const result = await buildSessionIndex(nonexistentDir, { runs: [] }); + + expect(result.sessionIndex).toBe(''); + expect(result.newSessionIds).toEqual([]); + }); + + it('marks sessions as [NEW] when not in any previous run', async () => { + const { buildSessionIndex } = await import('./defaultMemoryProvider.js'); + + const conversation = createConversation({ + sessionId: 'brand-new', + summary: 'A brand new session', + messageCount: 20, + }); + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2025-01-01T00-00-brandnew.json`, + ), + JSON.stringify(conversation), + ); + + const result = await buildSessionIndex(chatsDir, { runs: [] }); + + expect(result.sessionIndex).toContain('[NEW]'); + expect(result.sessionIndex).not.toContain('[old]'); + }); + + it('marks sessions as [old] when already in a previous run', async () => { + const { buildSessionIndex } = await import('./defaultMemoryProvider.js'); + + const conversation = createConversation({ + sessionId: 'old-session', + summary: 'An old session', + messageCount: 20, + }); + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2025-01-01T00-00-oldsess1.json`, + ), + JSON.stringify(conversation), + ); + + const state: ExtractionState = { + runs: [ + { + runAt: '2025-01-01T00:00:00Z', + sessionIds: ['old-session'], + skillsCreated: [], + }, + ], + }; + + const result = await buildSessionIndex(chatsDir, state); + + expect(result.sessionIndex).toContain('[old]'); + expect(result.sessionIndex).not.toContain('[NEW]'); + }); + + it('includes file path and summary in each line', async () => { + const { buildSessionIndex } = await import('./defaultMemoryProvider.js'); + + const conversation = createConversation({ + sessionId: 'detailed-session', + summary: 'Debugging the login flow', + messageCount: 20, + }); + const fileName = `${SESSION_FILE_PREFIX}2025-01-01T00-00-detail01.json`; + await fs.writeFile( + path.join(chatsDir, fileName), + JSON.stringify(conversation), + ); + + const result = await buildSessionIndex(chatsDir, { runs: [] }); + + expect(result.sessionIndex).toContain('Debugging the login flow'); + expect(result.sessionIndex).toContain(path.join(chatsDir, fileName)); + }); + + it('filters out subagent sessions', async () => { + const { buildSessionIndex } = await import('./defaultMemoryProvider.js'); + + const conversation = createConversation({ + sessionId: 'sub-session', + kind: 'subagent', + messageCount: 20, + }); + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2025-01-01T00-00-sub00001.json`, + ), + JSON.stringify(conversation), + ); + + const result = await buildSessionIndex(chatsDir, { runs: [] }); + + expect(result.sessionIndex).toBe(''); + expect(result.newSessionIds).toEqual([]); + }); + + it('filters out sessions with fewer than 10 user messages', async () => { + const { buildSessionIndex } = await import('./defaultMemoryProvider.js'); + + // 2 messages total: 1 user (index 0) + 1 gemini (index 1) + const conversation = createConversation({ + sessionId: 'short-session', + messageCount: 2, + }); + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2025-01-01T00-00-short001.json`, + ), + JSON.stringify(conversation), + ); + + const result = await buildSessionIndex(chatsDir, { runs: [] }); + + expect(result.sessionIndex).toBe(''); + expect(result.newSessionIds).toEqual([]); + }); + + it('caps at MAX_SESSION_INDEX_SIZE (50)', async () => { + const { buildSessionIndex } = await import('./defaultMemoryProvider.js'); + + // Create 3 eligible sessions, verify all 3 appear (well under cap) + for (let i = 0; i < 3; i++) { + const conversation = createConversation({ + sessionId: `capped-session-${i}`, + summary: `Summary ${i}`, + messageCount: 20, + }); + const paddedIndex = String(i).padStart(4, '0'); + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2025-01-0${i + 1}T00-00-cap${paddedIndex}.json`, + ), + JSON.stringify(conversation), + ); + } + + const result = await buildSessionIndex(chatsDir, { runs: [] }); + + const lines = result.sessionIndex.split('\n').filter((l) => l.length > 0); + expect(lines).toHaveLength(3); + expect(result.newSessionIds).toHaveLength(3); + }); + + it('returns newSessionIds only for unprocessed sessions', async () => { + const { buildSessionIndex } = await import('./defaultMemoryProvider.js'); + + // Write two sessions: one already processed, one new + const oldConv = createConversation({ + sessionId: 'processed-one', + summary: 'Old', + messageCount: 20, + }); + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2025-01-01T00-00-proc0001.json`, + ), + JSON.stringify(oldConv), + ); + + const newConv = createConversation({ + sessionId: 'fresh-one', + summary: 'New', + messageCount: 20, + }); + await fs.writeFile( + path.join( + chatsDir, + `${SESSION_FILE_PREFIX}2025-01-02T00-00-fres0001.json`, + ), + JSON.stringify(newConv), + ); + + const state: ExtractionState = { + runs: [ + { + runAt: '2025-01-01T00:00:00Z', + sessionIds: ['processed-one'], + skillsCreated: [], + }, + ], + }; + + const result = await buildSessionIndex(chatsDir, state); + + expect(result.newSessionIds).toEqual(['fresh-one']); + expect(result.newSessionIds).not.toContain('processed-one'); + // Both sessions should still appear in the index + expect(result.sessionIndex).toContain('[NEW]'); + expect(result.sessionIndex).toContain('[old]'); + }); + }); + + describe('ExtractionState runs tracking', () => { + it('readExtractionState parses runs array with skillsCreated', async () => { + const { readExtractionState } = await import( + './defaultMemoryProvider.js' + ); + + const statePath = path.join(tmpDir, 'state-with-skills.json'); + const state: ExtractionState = { + runs: [ + { + runAt: '2025-06-01T00:00:00Z', + sessionIds: ['s1'], + skillsCreated: ['debug-helper', 'test-gen'], + }, + ], + }; + await fs.writeFile(statePath, JSON.stringify(state)); + + const result = await readExtractionState(statePath); + + expect(result.runs).toHaveLength(1); + expect(result.runs[0].skillsCreated).toEqual([ + 'debug-helper', + 'test-gen', + ]); + expect(result.runs[0].sessionIds).toEqual(['s1']); + expect(result.runs[0].runAt).toBe('2025-06-01T00:00:00Z'); + }); + + it('writeExtractionState + readExtractionState roundtrips runs correctly', async () => { + const { writeExtractionState, readExtractionState } = await import( + './defaultMemoryProvider.js' + ); + + const statePath = path.join(tmpDir, 'roundtrip-state.json'); + const runs: ExtractionRun[] = [ + { + runAt: '2025-01-01T00:00:00Z', + sessionIds: ['a', 'b'], + skillsCreated: ['skill-x'], + }, + { + runAt: '2025-01-02T00:00:00Z', + sessionIds: ['c'], + skillsCreated: [], + }, + ]; + const state: ExtractionState = { runs }; + + await writeExtractionState(statePath, state); + const result = await readExtractionState(statePath); + + expect(result).toEqual(state); + }); + + it('readExtractionState handles old format without runs', async () => { + const { readExtractionState } = await import( + './defaultMemoryProvider.js' + ); + + const statePath = path.join(tmpDir, 'old-format-state.json'); + // Old format: an object without a runs array + await fs.writeFile( + statePath, + JSON.stringify({ lastProcessed: '2025-01-01' }), + ); + + const result = await readExtractionState(statePath); + + expect(result).toEqual({ runs: [] }); + }); + }); + + describe('validatePatches', () => { + let skillsDir: string; + let globalSkillsDir: string; + let projectSkillsDir: string; + let validateConfig: Config; + + beforeEach(() => { + skillsDir = path.join(tmpDir, 'skills'); + globalSkillsDir = path.join(tmpDir, 'global-skills'); + projectSkillsDir = path.join(tmpDir, 'project-skills'); + + vi.mocked(Storage.getUserSkillsDir).mockReturnValue(globalSkillsDir); + validateConfig = { + storage: { + getProjectSkillsDir: () => projectSkillsDir, + }, + } as unknown as Config; + }); + + it('returns empty array when no patch files exist', async () => { + const { validatePatches } = await import('./defaultMemoryProvider.js'); + + await fs.mkdir(skillsDir, { recursive: true }); + // Add a non-patch file to ensure it's ignored + await fs.writeFile(path.join(skillsDir, 'some-file.txt'), 'hello'); + + const result = await validatePatches(skillsDir, validateConfig); + + expect(result).toEqual([]); + }); + + it('returns empty array when directory does not exist', async () => { + const { validatePatches } = await import('./defaultMemoryProvider.js'); + + const result = await validatePatches( + path.join(tmpDir, 'nonexistent-dir'), + validateConfig, + ); + + expect(result).toEqual([]); + }); + + it('removes invalid patch files', async () => { + const { validatePatches } = await import('./defaultMemoryProvider.js'); + + await fs.mkdir(skillsDir, { recursive: true }); + + // Write a malformed patch + const patchPath = path.join(skillsDir, 'bad-skill.patch'); + await fs.writeFile(patchPath, 'this is not a valid patch'); + + const result = await validatePatches(skillsDir, validateConfig); + + expect(result).toEqual([]); + // Verify the invalid patch was deleted + await expect(fs.access(patchPath)).rejects.toThrow(); + }); + + it('keeps valid patch files', async () => { + const { validatePatches } = await import('./defaultMemoryProvider.js'); + + await fs.mkdir(skillsDir, { recursive: true }); + await fs.mkdir(projectSkillsDir, { recursive: true }); + + // Create a real target file to patch + const targetFile = path.join(projectSkillsDir, 'target.md'); + await fs.writeFile(targetFile, 'line1\nline2\nline3\n'); + + // Write a valid unified diff patch with absolute paths + const patchContent = [ + `--- ${targetFile}`, + `+++ ${targetFile}`, + '@@ -1,3 +1,4 @@', + ' line1', + ' line2', + '+line2.5', + ' line3', + '', + ].join('\n'); + const patchPath = path.join(skillsDir, 'good-skill.patch'); + await fs.writeFile(patchPath, patchContent); + + const result = await validatePatches(skillsDir, validateConfig); + + expect(result).toEqual(['good-skill.patch']); + // Verify the valid patch still exists + await expect(fs.access(patchPath)).resolves.toBeUndefined(); + }); + + it('keeps patches with repeated sections for the same file when hunks apply cumulatively', async () => { + const { validatePatches } = await import('./defaultMemoryProvider.js'); + + await fs.mkdir(skillsDir, { recursive: true }); + await fs.mkdir(projectSkillsDir, { recursive: true }); + + const targetFile = path.join(projectSkillsDir, 'target.md'); + await fs.writeFile(targetFile, 'alpha\nbeta\ngamma\ndelta\n'); + + const patchPath = path.join(skillsDir, 'multi-section.patch'); + await fs.writeFile( + patchPath, + [ + `--- ${targetFile}`, + `+++ ${targetFile}`, + '@@ -1,4 +1,5 @@', + ' alpha', + ' beta', + '+beta2', + ' gamma', + ' delta', + `--- ${targetFile}`, + `+++ ${targetFile}`, + '@@ -2,4 +2,5 @@', + ' beta', + ' beta2', + ' gamma', + '+gamma2', + ' delta', + '', + ].join('\n'), + ); + + const result = await validatePatches(skillsDir, validateConfig); + + expect(result).toEqual(['multi-section.patch']); + await expect(fs.access(patchPath)).resolves.toBeUndefined(); + }); + + it('removes /dev/null patches that target an existing skill file', async () => { + const { validatePatches } = await import('./defaultMemoryProvider.js'); + + await fs.mkdir(skillsDir, { recursive: true }); + await fs.mkdir(projectSkillsDir, { recursive: true }); + + const targetFile = path.join(projectSkillsDir, 'existing-skill.md'); + await fs.writeFile(targetFile, 'original content\n'); + + const patchPath = path.join(skillsDir, 'bad-new-file.patch'); + await fs.writeFile( + patchPath, + [ + '--- /dev/null', + `+++ ${targetFile}`, + '@@ -0,0 +1 @@', + '+replacement content', + '', + ].join('\n'), + ); + + const result = await validatePatches(skillsDir, validateConfig); + + expect(result).toEqual([]); + await expect(fs.access(patchPath)).rejects.toThrow(); + expect(await fs.readFile(targetFile, 'utf-8')).toBe('original content\n'); + }); + + it('removes patches with malformed diff headers', async () => { + const { validatePatches } = await import('./defaultMemoryProvider.js'); + + await fs.mkdir(skillsDir, { recursive: true }); + await fs.mkdir(projectSkillsDir, { recursive: true }); + + const targetFile = path.join(projectSkillsDir, 'target.md'); + await fs.writeFile(targetFile, 'line1\nline2\nline3\n'); + + const patchPath = path.join(skillsDir, 'bad-headers.patch'); + await fs.writeFile( + patchPath, + [ + `--- ${targetFile}`, + '+++ .gemini/skills/foo/SKILL.md', + '@@ -1,3 +1,4 @@', + ' line1', + ' line2', + '+line2.5', + ' line3', + '', + ].join('\n'), + ); + + const result = await validatePatches(skillsDir, validateConfig); + + expect(result).toEqual([]); + await expect(fs.access(patchPath)).rejects.toThrow(); + expect(await fs.readFile(targetFile, 'utf-8')).toBe( + 'line1\nline2\nline3\n', + ); + }); + + it('removes patches that contain no hunks', async () => { + const { validatePatches } = await import('./defaultMemoryProvider.js'); + + await fs.mkdir(skillsDir, { recursive: true }); + const patchPath = path.join(skillsDir, 'empty.patch'); + await fs.writeFile( + patchPath, + [ + `--- ${path.join(projectSkillsDir, 'target.md')}`, + `+++ ${path.join(projectSkillsDir, 'target.md')}`, + '', + ].join('\n'), + ); + + const result = await validatePatches(skillsDir, validateConfig); + + expect(result).toEqual([]); + await expect(fs.access(patchPath)).rejects.toThrow(); + }); + + it('removes patches that target files outside the allowed skill roots', async () => { + const { validatePatches } = await import('./defaultMemoryProvider.js'); + + await fs.mkdir(skillsDir, { recursive: true }); + const outsideFile = path.join(tmpDir, 'outside.md'); + await fs.writeFile(outsideFile, 'line1\nline2\nline3\n'); + + const patchPath = path.join(skillsDir, 'outside.patch'); + await fs.writeFile( + patchPath, + [ + `--- ${outsideFile}`, + `+++ ${outsideFile}`, + '@@ -1,3 +1,4 @@', + ' line1', + ' line2', + '+line2.5', + ' line3', + '', + ].join('\n'), + ); + + const result = await validatePatches(skillsDir, validateConfig); + + expect(result).toEqual([]); + await expect(fs.access(patchPath)).rejects.toThrow(); + }); + + it('removes patches that escape the allowed roots through a symlinked parent', async () => { + const { validatePatches } = await import('./defaultMemoryProvider.js'); + + await fs.mkdir(skillsDir, { recursive: true }); + await fs.mkdir(projectSkillsDir, { recursive: true }); + + const outsideDir = path.join(tmpDir, 'outside-dir'); + const linkedDir = path.join(projectSkillsDir, 'linked'); + await fs.mkdir(outsideDir, { recursive: true }); + await fs.symlink( + outsideDir, + linkedDir, + process.platform === 'win32' ? 'junction' : 'dir', + ); + + const outsideFile = path.join(outsideDir, 'escaped.md'); + await fs.writeFile(outsideFile, 'line1\nline2\nline3\n'); + + const patchPath = path.join(skillsDir, 'symlink.patch'); + await fs.writeFile( + patchPath, + [ + `--- ${path.join(linkedDir, 'escaped.md')}`, + `+++ ${path.join(linkedDir, 'escaped.md')}`, + '@@ -1,3 +1,4 @@', + ' line1', + ' line2', + '+line2.5', + ' line3', + '', + ].join('\n'), + ); + + const result = await validatePatches(skillsDir, validateConfig); + + expect(result).toEqual([]); + await expect(fs.access(patchPath)).rejects.toThrow(); + expect(await fs.readFile(outsideFile, 'utf-8')).not.toContain('line2.5'); + }); + }); + + describe('DefaultMemoryProvider feedback for patch-only runs', () => { + it('emits feedback when extraction produces only patch suggestions', async () => { + const { DefaultMemoryProvider } = await import( + './defaultMemoryProvider.js' + ); + const { LocalAgentExecutor } = await import( + '../agents/local-executor.js' + ); + + vi.mocked(coreEvents.emitFeedback).mockClear(); + vi.mocked(LocalAgentExecutor.create).mockReset(); + + const memoryDir = path.join(tmpDir, 'memory-patch-only'); + const skillsDir = path.join(tmpDir, 'skills-patch-only'); + const projectTempDir = path.join(tmpDir, 'temp-patch-only'); + const chatsDir = path.join(projectTempDir, 'chats'); + const projectSkillsDir = path.join(tmpDir, 'workspace-skills'); + await fs.mkdir(memoryDir, { recursive: true }); + await fs.mkdir(skillsDir, { recursive: true }); + await fs.mkdir(chatsDir, { recursive: true }); + await fs.mkdir(projectSkillsDir, { recursive: true }); + + const existingSkill = path.join(projectSkillsDir, 'existing-skill.md'); + await fs.writeFile(existingSkill, 'line1\nline2\nline3\n'); + + const conversation = createConversation({ + sessionId: 'patch-only-session', + messageCount: 20, + }); + await fs.writeFile( + path.join(chatsDir, 'session-2025-01-01T00-00-patchonly.json'), + JSON.stringify(conversation), + ); + + vi.mocked(Storage.getUserSkillsDir).mockReturnValue( + path.join(tmpDir, 'global-skills'), + ); + vi.mocked(LocalAgentExecutor.create).mockResolvedValueOnce({ + run: vi.fn().mockImplementation(async () => { + const patchPath = path.join(skillsDir, 'existing-skill.patch'); + await fs.writeFile( + patchPath, + [ + `--- ${existingSkill}`, + `+++ ${existingSkill}`, + '@@ -1,3 +1,4 @@', + ' line1', + ' line2', + '+line2.5', + ' line3', + '', + ].join('\n'), + ); + return undefined; + }), + } as never); + + const mockConfig = { + storage: { + getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), + getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), + getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), + getProjectSkillsDir: vi.fn().mockReturnValue(projectSkillsDir), + getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), + }, + getToolRegistry: vi.fn(), + getMessageBus: vi.fn(), + getGeminiClient: vi.fn(), + getSkillManager: vi.fn().mockReturnValue({ getSkills: () => [] }), + modelConfigService: { + registerRuntimeModelConfig: vi.fn(), + }, + sandboxManager: undefined, + } as unknown as Config; + + const provider = new DefaultMemoryProvider(); + provider.onSessionStart(mockConfig, 'session-id'); + + await vi.waitFor(() => { + expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + 'info', + expect.stringContaining('skill update'), + ); + expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + 'info', + expect.stringContaining('/memory inbox'), + ); + }); + }); + + it('does not emit feedback for old inbox patches when this run creates none', async () => { + const { DefaultMemoryProvider } = await import( + './defaultMemoryProvider.js' + ); + const { LocalAgentExecutor } = await import( + '../agents/local-executor.js' + ); + const { ExecutionLifecycleService } = await import( + './executionLifecycleService.js' + ); + + vi.mocked(coreEvents.emitFeedback).mockClear(); + vi.mocked(LocalAgentExecutor.create).mockReset(); + + const memoryDir = path.join(tmpDir, 'memory-old-patch'); + const skillsDir = path.join(tmpDir, 'skills-old-patch'); + const projectTempDir = path.join(tmpDir, 'temp-old-patch'); + const chatsDir = path.join(projectTempDir, 'chats'); + const projectSkillsDir = path.join(tmpDir, 'workspace-old-patch'); + await fs.mkdir(memoryDir, { recursive: true }); + await fs.mkdir(skillsDir, { recursive: true }); + await fs.mkdir(chatsDir, { recursive: true }); + await fs.mkdir(projectSkillsDir, { recursive: true }); + + const existingSkill = path.join(projectSkillsDir, 'existing-skill.md'); + await fs.writeFile(existingSkill, 'line1\nline2\nline3\n'); + await fs.writeFile( + path.join(skillsDir, 'existing-skill.patch'), + [ + `--- ${existingSkill}`, + `+++ ${existingSkill}`, + '@@ -1,3 +1,4 @@', + ' line1', + ' line2', + '+line2.5', + ' line3', + '', + ].join('\n'), + ); + + const conversation = createConversation({ + sessionId: 'old-patch-session', + messageCount: 20, + }); + await fs.writeFile( + path.join(chatsDir, 'session-2025-01-01T00-00-oldpatch.json'), + JSON.stringify(conversation), + ); + + vi.mocked(Storage.getUserSkillsDir).mockReturnValue( + path.join(tmpDir, 'global-skills'), + ); + vi.mocked(LocalAgentExecutor.create).mockResolvedValueOnce({ + run: vi.fn().mockResolvedValue(undefined), + } as never); + + const mockConfig = { + storage: { + getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), + getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), + getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), + getProjectSkillsDir: vi.fn().mockReturnValue(projectSkillsDir), + getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), + }, + getToolRegistry: vi.fn(), + getMessageBus: vi.fn(), + getGeminiClient: vi.fn(), + getSkillManager: vi.fn().mockReturnValue({ getSkills: () => [] }), + modelConfigService: { + registerRuntimeModelConfig: vi.fn(), + }, + sandboxManager: undefined, + } as unknown as Config; + + const provider = new DefaultMemoryProvider(); + provider.onSessionStart(mockConfig, 'session-id'); + + await vi.waitFor(() => { + expect(ExecutionLifecycleService.completeExecution).toHaveBeenCalled(); + }); + + expect(coreEvents.emitFeedback).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/core/src/services/defaultMemoryProvider.ts b/packages/core/src/services/defaultMemoryProvider.ts new file mode 100644 index 0000000000..d51921c30a --- /dev/null +++ b/packages/core/src/services/defaultMemoryProvider.ts @@ -0,0 +1,756 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { Config } from '../config/config.js'; +import type { MemoryProvider } from './memoryProvider.js'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import { constants as fsConstants } from 'node:fs'; +import { randomUUID } from 'node:crypto'; +import * as Diff from 'diff'; +import { + SESSION_FILE_PREFIX, + type ConversationRecord, +} from './chatRecordingService.js'; +import { debugLogger } from '../utils/debugLogger.js'; +import { coreEvents } from '../utils/events.js'; +import { isNodeError } from '../utils/errors.js'; +import { FRONTMATTER_REGEX, parseFrontmatter } from '../skills/skillLoader.js'; +import { LocalAgentExecutor } from '../agents/local-executor.js'; +import { SkillExtractionAgent } from '../agents/skill-extraction-agent.js'; +import { getModelConfigAlias } from '../agents/registry.js'; +import { ExecutionLifecycleService } from './executionLifecycleService.js'; +import { PromptRegistry } from '../prompts/prompt-registry.js'; +import { ResourceRegistry } from '../resources/resource-registry.js'; +import { PolicyEngine } from '../policy/policy-engine.js'; +import { PolicyDecision } from '../policy/types.js'; +import { MessageBus } from '../confirmation-bus/message-bus.js'; +import { Storage } from '../config/storage.js'; +import type { AgentLoopContext } from '../config/agent-loop-context.js'; +import { + applyParsedSkillPatches, + hasParsedPatchHunks, +} from './memoryPatchUtils.js'; + +const LOCK_FILENAME = '.extraction.lock'; +const STATE_FILENAME = '.extraction-state.json'; +const LOCK_STALE_MS = 35 * 60 * 1000; // 35 minutes (exceeds agent's 30-min time limit) +const MIN_USER_MESSAGES = 10; +const MIN_IDLE_MS = 3 * 60 * 60 * 1000; // 3 hours +const MAX_SESSION_INDEX_SIZE = 50; + +// ... keeping all existing helper types and functions above startMemoryService + +interface LockInfo { + pid: number; + startedAt: string; +} + +export interface ExtractionRun { + runAt: string; + sessionIds: string[]; + skillsCreated: string[]; +} + +export interface ExtractionState { + runs: ExtractionRun[]; +} + +export function getProcessedSessionIds(state: ExtractionState): Set { + const ids = new Set(); + for (const run of state.runs) { + for (const id of run.sessionIds) { + ids.add(id); + } + } + return ids; +} + +function isLockInfo(value: unknown): value is LockInfo { + return ( + typeof value === 'object' && + value !== null && + 'pid' in value && + typeof value.pid === 'number' && + 'startedAt' in value && + typeof value.startedAt === 'string' + ); +} + +function isConversationRecord(value: unknown): value is ConversationRecord { + return ( + typeof value === 'object' && + value !== null && + 'sessionId' in value && + typeof value.sessionId === 'string' && + 'messages' in value && + Array.isArray(value.messages) && + 'projectHash' in value && + 'startTime' in value && + 'lastUpdated' in value + ); +} + +function isExtractionRun(value: unknown): value is ExtractionRun { + return ( + typeof value === 'object' && + value !== null && + 'runAt' in value && + typeof value.runAt === 'string' && + 'sessionIds' in value && + Array.isArray(value.sessionIds) && + 'skillsCreated' in value && + Array.isArray(value.skillsCreated) + ); +} + +function isExtractionState(value: unknown): value is { runs: unknown[] } { + return ( + typeof value === 'object' && + value !== null && + 'runs' in value && + Array.isArray(value.runs) + ); +} + +export async function tryAcquireLock( + lockPath: string, + retries = 1, +): Promise { + const lockInfo: LockInfo = { + pid: process.pid, + startedAt: new Date().toISOString(), + }; + + try { + const fd = await fs.open( + lockPath, + fsConstants.O_CREAT | fsConstants.O_EXCL | fsConstants.O_WRONLY, + ); + try { + await fd.writeFile(JSON.stringify(lockInfo)); + } finally { + await fd.close(); + } + return true; + } catch (error: unknown) { + if (isNodeError(error) && error.code === 'EEXIST') { + if (retries > 0 && (await isLockStale(lockPath))) { + debugLogger.debug('[MemoryService] Cleaning up stale lock file'); + await releaseLock(lockPath); + return tryAcquireLock(lockPath, retries - 1); + } + debugLogger.debug( + '[MemoryService] Lock held by another instance, skipping', + ); + return false; + } + throw error; + } +} + +export async function isLockStale(lockPath: string): Promise { + try { + const content = await fs.readFile(lockPath, 'utf-8'); + const parsed: unknown = JSON.parse(content); + if (!isLockInfo(parsed)) { + return true; + } + const lockInfo = parsed; + + try { + process.kill(lockInfo.pid, 0); + } catch { + return true; + } + + const lockAge = Date.now() - new Date(lockInfo.startedAt).getTime(); + if (lockAge > LOCK_STALE_MS) { + return true; + } + + return false; + } catch { + return true; + } +} + +export async function releaseLock(lockPath: string): Promise { + try { + await fs.unlink(lockPath); + } catch (error: unknown) { + if (isNodeError(error) && error.code === 'ENOENT') { + return; + } + debugLogger.warn( + `[MemoryService] Failed to release lock: ${error instanceof Error ? error.message : String(error)}`, + ); + } +} + +export async function readExtractionState( + statePath: string, +): Promise { + try { + const content = await fs.readFile(statePath, 'utf-8'); + const parsed: unknown = JSON.parse(content); + if (!isExtractionState(parsed)) { + return { runs: [] }; + } + + const runs: ExtractionRun[] = []; + for (const run of parsed.runs) { + if (!isExtractionRun(run)) continue; + runs.push({ + runAt: run.runAt, + sessionIds: run.sessionIds.filter( + (sid): sid is string => typeof sid === 'string', + ), + skillsCreated: run.skillsCreated.filter( + (sk): sk is string => typeof sk === 'string', + ), + }); + } + + return { runs }; + } catch (error) { + debugLogger.debug( + '[MemoryService] Failed to read extraction state:', + error, + ); + return { runs: [] }; + } +} + +export async function writeExtractionState( + statePath: string, + state: ExtractionState, +): Promise { + const tmpPath = `${statePath}.tmp`; + await fs.writeFile(tmpPath, JSON.stringify(state, null, 2)); + await fs.rename(tmpPath, statePath); +} + +function shouldProcessConversation(parsed: ConversationRecord): boolean { + if (parsed.kind === 'subagent') return false; + + const lastUpdated = new Date(parsed.lastUpdated).getTime(); + if (Date.now() - lastUpdated < MIN_IDLE_MS) return false; + + const userMessageCount = parsed.messages.filter( + (m) => m.type === 'user', + ).length; + if (userMessageCount < MIN_USER_MESSAGES) return false; + + return true; +} + +async function scanEligibleSessions( + chatsDir: string, +): Promise> { + let allFiles: string[]; + try { + allFiles = await fs.readdir(chatsDir); + } catch { + return []; + } + + const sessionFiles = allFiles.filter( + (f) => f.startsWith(SESSION_FILE_PREFIX) && f.endsWith('.json'), + ); + + sessionFiles.sort((a, b) => b.localeCompare(a)); + + const results: Array<{ conversation: ConversationRecord; filePath: string }> = + []; + + for (const file of sessionFiles) { + if (results.length >= MAX_SESSION_INDEX_SIZE) break; + + const filePath = path.join(chatsDir, file); + try { + const content = await fs.readFile(filePath, 'utf-8'); + const parsed: unknown = JSON.parse(content); + if (!isConversationRecord(parsed)) continue; + if (!shouldProcessConversation(parsed)) continue; + + results.push({ conversation: parsed, filePath }); + } catch { + // Skip unreadable files + } + } + + return results; +} + +export async function buildSessionIndex( + chatsDir: string, + state: ExtractionState, +): Promise<{ sessionIndex: string; newSessionIds: string[] }> { + const processedSet = getProcessedSessionIds(state); + const eligible = await scanEligibleSessions(chatsDir); + + if (eligible.length === 0) { + return { sessionIndex: '', newSessionIds: [] }; + } + + const lines: string[] = []; + const newSessionIds: string[] = []; + + for (const { conversation, filePath } of eligible) { + const userMessageCount = conversation.messages.filter( + (m) => m.type === 'user', + ).length; + const isNew = !processedSet.has(conversation.sessionId); + if (isNew) { + newSessionIds.push(conversation.sessionId); + } + + const status = isNew ? '[NEW]' : '[old]'; + const summary = conversation.summary ?? '(no summary)'; + lines.push( + `${status} ${summary} (${userMessageCount} user msgs) — ${filePath}`, + ); + } + + return { sessionIndex: lines.join('\n'), newSessionIds }; +} + +async function buildExistingSkillsSummary( + skillsDir: string, + config: Config, +): Promise { + const sections: string[] = []; + + const memorySkills: string[] = []; + try { + const entries = await fs.readdir(skillsDir, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isDirectory()) continue; + + const skillPath = path.join(skillsDir, entry.name, 'SKILL.md'); + try { + const content = await fs.readFile(skillPath, 'utf-8'); + const match = content.match(FRONTMATTER_REGEX); + if (match) { + const parsed = parseFrontmatter(match[1]); + const name = parsed?.name ?? entry.name; + const desc = parsed?.description ?? ''; + memorySkills.push(`- **${name}**: ${desc}`); + } else { + memorySkills.push(`- **${entry.name}**`); + } + } catch { + // Skip unreadable files + } + } + } catch { + // Skip unreadable files + } + + if (memorySkills.length > 0) { + sections.push( + `## Previously Extracted Skills (in ${skillsDir})\n${memorySkills.join('\n')}`, + ); + } + + try { + const discoveredSkills = config.getSkillManager().getSkills(); + if (discoveredSkills.length > 0) { + const userSkillsDir = Storage.getUserSkillsDir(); + const globalSkills: string[] = []; + const workspaceSkills: string[] = []; + const extensionSkills: string[] = []; + const builtinSkills: string[] = []; + + for (const s of discoveredSkills) { + const loc = s.location; + if (loc.includes('/bundle/') || loc.includes('\\bundle\\')) { + builtinSkills.push(`- **${s.name}**: ${s.description}`); + } else if (loc.startsWith(userSkillsDir)) { + globalSkills.push(`- **${s.name}**: ${s.description} (${loc})`); + } else if ( + loc.includes('/extensions/') || + loc.includes('\\extensions\\') + ) { + extensionSkills.push(`- **${s.name}**: ${s.description}`); + } else { + workspaceSkills.push(`- **${s.name}**: ${s.description} (${loc})`); + } + } + + if (globalSkills.length > 0) { + sections.push( + `## Global Skills (~/.gemini/skills — do NOT duplicate)\n${globalSkills.join('\n')}`, + ); + } + if (workspaceSkills.length > 0) { + sections.push( + `## Workspace Skills (.gemini/skills — do NOT duplicate)\n${workspaceSkills.join('\n')}`, + ); + } + if (extensionSkills.length > 0) { + sections.push( + `## Extension Skills (from installed extensions — do NOT duplicate)\n${extensionSkills.join('\n')}`, + ); + } + if (builtinSkills.length > 0) { + sections.push( + `## Builtin Skills (bundled with CLI — do NOT duplicate)\n${builtinSkills.join('\n')}`, + ); + } + } + } catch { + // Skip unreadable files + } + + return sections.join('\n\n'); +} + +function buildAgentLoopContext(config: Config): AgentLoopContext { + const autoApprovePolicy = new PolicyEngine({ + rules: [ + { + toolName: '*', + decision: PolicyDecision.ALLOW, + priority: 100, + }, + ], + }); + const autoApproveBus = new MessageBus(autoApprovePolicy); + + return { + config, + promptId: `skill-extraction-${randomUUID().slice(0, 8)}`, + toolRegistry: config.getToolRegistry(), + promptRegistry: new PromptRegistry(), + resourceRegistry: new ResourceRegistry(), + messageBus: autoApproveBus, + geminiClient: config.getGeminiClient(), + sandboxManager: config.sandboxManager, + }; +} + +export async function validatePatches( + skillsDir: string, + config: Config, +): Promise { + let entries: string[]; + try { + entries = await fs.readdir(skillsDir); + } catch { + return []; + } + + const patchFiles = entries.filter((e) => e.endsWith('.patch')); + const validPatches: string[] = []; + + for (const patchFile of patchFiles) { + const patchPath = path.join(skillsDir, patchFile); + let valid = true; + let reason = ''; + + try { + const patchContent = await fs.readFile(patchPath, 'utf-8'); + const parsedPatches = Diff.parsePatch(patchContent); + + if (!hasParsedPatchHunks(parsedPatches)) { + valid = false; + reason = 'no hunks found in patch'; + } else { + const applied = await applyParsedSkillPatches(parsedPatches, config); + if (!applied.success) { + valid = false; + switch (applied.reason) { + case 'missingTargetPath': + reason = 'missing target file path in patch header'; + break; + case 'invalidPatchHeaders': + reason = 'invalid diff headers'; + break; + case 'outsideAllowedRoots': + reason = `target file is outside skill roots: ${applied.targetPath}`; + break; + case 'newFileAlreadyExists': + reason = `new file target already exists: ${applied.targetPath}`; + break; + case 'targetNotFound': + reason = `target file not found: ${applied.targetPath}`; + break; + case 'doesNotApply': + reason = `patch does not apply cleanly to ${applied.targetPath}`; + break; + default: + reason = 'unknown patch validation failure'; + break; + } + } + } + } catch (err) { + valid = false; + reason = `failed to read or parse patch: ${err}`; + } + + if (valid) { + validPatches.push(patchFile); + debugLogger.log(`[MemoryService] Patch validated: ${patchFile}`); + } else { + debugLogger.warn( + `[MemoryService] Removing invalid patch ${patchFile}: ${reason}`, + ); + try { + await fs.unlink(patchPath); + } catch { + // Best-effort cleanup + } + } + } + + return validPatches; +} + +export class DefaultMemoryProvider implements MemoryProvider { + readonly id = 'gemini-cli-builtin-memory'; + + onSessionStart(config: Config, _sessionId: string): void { + void this.startSkillExtractionTask(config).catch((error) => { + debugLogger.warn( + '[MemoryService] Failed to start background skill extraction:', + error, + ); + }); + } + + getSystemInstructions(): string { + return ''; + } + + getTurnContext(_query: string): string { + return ''; + } + + onTurnComplete(_userMessage: string, _assistantMessage: string): void {} + + onSessionEnd(): void {} + + private async startSkillExtractionTask(config: Config): Promise { + const memoryDir = config.storage.getProjectMemoryTempDir(); + const skillsDir = config.storage.getProjectSkillsMemoryDir(); + const lockPath = path.join(memoryDir, LOCK_FILENAME); + const statePath = path.join(memoryDir, STATE_FILENAME); + const chatsDir = path.join(config.storage.getProjectTempDir(), 'chats'); + + await fs.mkdir(skillsDir, { recursive: true }); + + debugLogger.log(`[MemoryService] Starting. Skills dir: ${skillsDir}`); + + if (!(await tryAcquireLock(lockPath))) { + debugLogger.log('[MemoryService] Skipped: lock held by another instance'); + return; + } + debugLogger.log('[MemoryService] Lock acquired'); + + const abortController = new AbortController(); + const handle = ExecutionLifecycleService.createExecution( + '', + () => abortController.abort(), + 'none', + undefined, + 'Skill extraction', + 'silent', + ); + const executionId = handle.pid; + + const startTime = Date.now(); + let completionResult: { error: Error } | undefined; + try { + const state = await readExtractionState(statePath); + const previousRuns = state.runs.length; + const previouslyProcessed = getProcessedSessionIds(state).size; + debugLogger.log( + `[MemoryService] State loaded: ${previousRuns} previous run(s), ${previouslyProcessed} session(s) already processed`, + ); + + const { sessionIndex, newSessionIds } = await buildSessionIndex( + chatsDir, + state, + ); + + const totalInIndex = sessionIndex ? sessionIndex.split('\n').length : 0; + debugLogger.log( + `[MemoryService] Session scan: ${totalInIndex} eligible session(s) found, ${newSessionIds.length} new`, + ); + + if (newSessionIds.length === 0) { + debugLogger.log('[MemoryService] Skipped: no new sessions to process'); + return; + } + + const skillsBefore = new Set(); + const patchContentsBefore = new Map(); + try { + const entries = await fs.readdir(skillsDir); + for (const e of entries) { + if (e.endsWith('.patch')) { + try { + patchContentsBefore.set( + e, + await fs.readFile(path.join(skillsDir, e), 'utf-8'), + ); + } catch { + // Skip unreadable files + } + continue; + } + skillsBefore.add(e); + } + } catch { + // Skip unreadable files + } + debugLogger.log( + `[MemoryService] ${skillsBefore.size} existing skill(s) in memory`, + ); + + const existingSkillsSummary = await buildExistingSkillsSummary( + skillsDir, + config, + ); + if (existingSkillsSummary) { + debugLogger.log( + `[MemoryService] Existing skills context:\n${existingSkillsSummary}`, + ); + } + + const agentDefinition = SkillExtractionAgent( + skillsDir, + sessionIndex, + existingSkillsSummary, + ); + + const context = buildAgentLoopContext(config); + + const modelAlias = getModelConfigAlias(agentDefinition); + config.modelConfigService.registerRuntimeModelConfig(modelAlias, { + modelConfig: agentDefinition.modelConfig, + }); + debugLogger.log( + `[MemoryService] Starting extraction agent (model: ${agentDefinition.modelConfig.model}, maxTurns: 30, maxTime: 30min)`, + ); + + const executor = await LocalAgentExecutor.create( + agentDefinition, + context, + ); + + await executor.run( + { request: 'Extract skills from the provided sessions.' }, + abortController.signal, + ); + + const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); + + const skillsCreated: string[] = []; + try { + const entriesAfter = await fs.readdir(skillsDir); + for (const e of entriesAfter) { + if (!skillsBefore.has(e) && !e.endsWith('.patch')) { + skillsCreated.push(e); + } + } + } catch { + // Skip unreadable files + } + + const validPatches = await validatePatches(skillsDir, config); + const patchesCreatedThisRun: string[] = []; + for (const patchFile of validPatches) { + const patchPath = path.join(skillsDir, patchFile); + let currentContent: string; + try { + currentContent = await fs.readFile(patchPath, 'utf-8'); + } catch { + continue; + } + if (patchContentsBefore.get(patchFile) !== currentContent) { + patchesCreatedThisRun.push(patchFile); + } + } + if (validPatches.length > 0) { + debugLogger.log( + `[MemoryService] ${validPatches.length} valid patch(es) currently in inbox; ${patchesCreatedThisRun.length} created or updated this run`, + ); + } + + const run: ExtractionRun = { + runAt: new Date().toISOString(), + sessionIds: newSessionIds, + skillsCreated, + }; + const updatedState: ExtractionState = { + runs: [...state.runs, run], + }; + await writeExtractionState(statePath, updatedState); + + if (skillsCreated.length > 0 || patchesCreatedThisRun.length > 0) { + const completionParts: string[] = []; + if (skillsCreated.length > 0) { + completionParts.push( + `created ${skillsCreated.length} skill(s): ${skillsCreated.join(', ')}`, + ); + } + if (patchesCreatedThisRun.length > 0) { + completionParts.push( + `prepared ${patchesCreatedThisRun.length} patch(es): ${patchesCreatedThisRun.join(', ')}`, + ); + } + debugLogger.log( + `[MemoryService] Completed in ${elapsed}s. ${completionParts.join('; ')} (processed ${newSessionIds.length} session(s))`, + ); + const feedbackParts: string[] = []; + if (skillsCreated.length > 0) { + feedbackParts.push( + `${skillsCreated.length} new skill${skillsCreated.length > 1 ? 's' : ''} extracted from past sessions: ${skillsCreated.join(', ')}`, + ); + } + if (patchesCreatedThisRun.length > 0) { + feedbackParts.push( + `${patchesCreatedThisRun.length} skill update${patchesCreatedThisRun.length > 1 ? 's' : ''} extracted from past sessions`, + ); + } + coreEvents.emitFeedback( + 'info', + `${feedbackParts.join('. ')}. Use /memory inbox to review.`, + ); + } else { + debugLogger.log( + `[MemoryService] Completed in ${elapsed}s. No new skills or patches created (processed ${newSessionIds.length} session(s))`, + ); + } + } catch (error) { + const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); + if (abortController.signal.aborted) { + debugLogger.log(`[MemoryService] Cancelled after ${elapsed}s`); + } else { + debugLogger.log( + `[MemoryService] Failed after ${elapsed}s: ${error instanceof Error ? error.message : String(error)}`, + ); + } + completionResult = { + error: error instanceof Error ? error : new Error(String(error)), + }; + return; + } finally { + await releaseLock(lockPath); + debugLogger.log('[MemoryService] Lock released'); + if (executionId !== undefined) { + ExecutionLifecycleService.completeExecution( + executionId, + completionResult, + ); + } + } + } +} diff --git a/packages/core/src/services/memoryProvider.ts b/packages/core/src/services/memoryProvider.ts new file mode 100644 index 0000000000..49e7d38655 --- /dev/null +++ b/packages/core/src/services/memoryProvider.ts @@ -0,0 +1,50 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { Config } from '../config/config.js'; + +/** + * Internal contract implemented by the memory backend that {@link MemoryService} + * delegates to. This interface is intentionally *not* exported from the + * package's public surface (`src/index.ts`) and is *not* surfaced via + * `GeminiCLIExtension` — it exists purely as a typing seam between + * `MemoryService` and its concrete implementation so that the two can evolve + * independently and be substituted in tests. + */ +export interface MemoryProvider { + /** Stable identifier used in diagnostic logs. */ + readonly id: string; + + /** + * Invoked once per session, when the owning `GeminiClient` initializes the + * memory subsystem. Implementations may kick off background work but must + * return synchronously. + */ + onSessionStart(config: Config, sessionId: string): void; + + /** + * Returns static instructions to inject into the LLM's system prompt. + * Called once per session and again after `onTurnComplete`. + */ + getSystemInstructions(): string; + + /** + * Returns dynamic, recalled context for the current turn based on the + * user's query. Called once per non-internal turn. + */ + getTurnContext(query: string): string; + + /** + * Invoked after the LLM completes a turn. Must return synchronously; any + * persistence work should be fired fire-and-forget by the implementation. + */ + onTurnComplete(userMessage: string, assistantMessage: string): void; + + /** + * Invoked when the owning session is shutting down gracefully. + */ + onSessionEnd(): void; +} diff --git a/packages/core/src/services/memoryService.test.ts b/packages/core/src/services/memoryService.test.ts index 69d7183ece..fc421818c7 100644 --- a/packages/core/src/services/memoryService.test.ts +++ b/packages/core/src/services/memoryService.test.ts @@ -4,1323 +4,175 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import * as fs from 'node:fs/promises'; -import * as path from 'node:path'; -import * as os from 'node:os'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { MemoryService } from './memoryService.js'; +import { DefaultMemoryProvider } from './defaultMemoryProvider.js'; import type { Config } from '../config/config.js'; -import { - SESSION_FILE_PREFIX, - type ConversationRecord, -} from './chatRecordingService.js'; -import type { ExtractionState, ExtractionRun } from './memoryService.js'; -import { coreEvents } from '../utils/events.js'; -import { Storage } from '../config/storage.js'; - -// Mock external modules used by startMemoryService -vi.mock('../agents/local-executor.js', () => ({ - LocalAgentExecutor: { - create: vi.fn().mockResolvedValue({ - run: vi.fn().mockResolvedValue(undefined), - }), - }, -})); - -vi.mock('../agents/skill-extraction-agent.js', () => ({ - SkillExtractionAgent: vi.fn().mockReturnValue({ - name: 'skill-extraction', - promptConfig: { systemPrompt: 'test' }, - tools: [], - outputSchema: {}, - modelConfig: { model: 'test-model' }, - }), -})); - -vi.mock('./executionLifecycleService.js', () => ({ - ExecutionLifecycleService: { - createExecution: vi.fn().mockReturnValue({ pid: 42, result: {} }), - completeExecution: vi.fn(), - }, -})); - -vi.mock('../tools/tool-registry.js', () => ({ - ToolRegistry: vi.fn(), -})); - -vi.mock('../prompts/prompt-registry.js', () => ({ - PromptRegistry: vi.fn(), -})); - -vi.mock('../resources/resource-registry.js', () => ({ - ResourceRegistry: vi.fn(), -})); - -vi.mock('../policy/policy-engine.js', () => ({ - PolicyEngine: vi.fn(), -})); - -vi.mock('../policy/types.js', () => ({ - PolicyDecision: { ALLOW: 'ALLOW' }, -})); - -vi.mock('../confirmation-bus/message-bus.js', () => ({ - MessageBus: vi.fn(), -})); - -vi.mock('../agents/registry.js', () => ({ - getModelConfigAlias: vi.fn().mockReturnValue('skill-extraction-config'), -})); - -vi.mock('../config/storage.js', () => ({ - Storage: { - getUserSkillsDir: vi.fn().mockReturnValue('/tmp/fake-user-skills'), - }, -})); - -vi.mock('../skills/skillLoader.js', () => ({ - FRONTMATTER_REGEX: /^---\n([\s\S]*?)\n---/, - parseFrontmatter: vi.fn().mockReturnValue(null), -})); +import { debugLogger } from '../utils/debugLogger.js'; vi.mock('../utils/debugLogger.js', () => ({ debugLogger: { + warn: vi.fn(), debug: vi.fn(), log: vi.fn(), - warn: vi.fn(), + error: vi.fn(), }, })); -vi.mock('../utils/events.js', () => ({ - coreEvents: { - emitFeedback: vi.fn(), - }, -})); +vi.mock('./defaultMemoryProvider.js', () => { + // Default behaviors are no-ops; individual tests override via spies. + class MockDefaultMemoryProvider { + readonly id = 'gemini-cli-builtin-memory'; + onSessionStart = vi.fn(); + getSystemInstructions = vi.fn().mockReturnValue(''); + getTurnContext = vi.fn().mockReturnValue(''); + onTurnComplete = vi.fn(); + onSessionEnd = vi.fn(); + } + return { DefaultMemoryProvider: MockDefaultMemoryProvider }; +}); -// Helper to create a minimal ConversationRecord -function createConversation( - overrides: Partial & { messageCount?: number } = {}, -): ConversationRecord { - const { messageCount = 4, ...rest } = overrides; - const messages = Array.from({ length: messageCount }, (_, i) => ({ - id: String(i + 1), - timestamp: new Date().toISOString(), - content: [{ text: `Message ${i + 1}` }], - type: i % 2 === 0 ? ('user' as const) : ('gemini' as const), - })); - return { - sessionId: rest.sessionId ?? `session-${Date.now()}`, - projectHash: 'abc123', - startTime: '2025-01-01T00:00:00Z', - lastUpdated: '2025-01-01T01:00:00Z', - messages, - ...rest, - }; +function createConfig(): Config { + return {} as unknown as Config; } -describe('memoryService', () => { - let tmpDir: string; +function getProvider(service: MemoryService): { + onSessionStart: ReturnType; + getSystemInstructions: ReturnType; + getTurnContext: ReturnType; + onTurnComplete: ReturnType; + onSessionEnd: ReturnType; +} { + // Reach into the private field to manipulate the underlying mocked provider. + return (service as unknown as { provider: ReturnType }) + .provider as never; +} - beforeEach(async () => { - tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'skill-extract-test-')); - }); - - afterEach(async () => { +describe('MemoryService', () => { + beforeEach(() => { vi.clearAllMocks(); - vi.unstubAllEnvs(); - await fs.rm(tmpDir, { recursive: true, force: true }); }); - describe('tryAcquireLock', () => { - it('successfully acquires lock when none exists', async () => { - const { tryAcquireLock } = await import('./memoryService.js'); + it('constructs a DefaultMemoryProvider on instantiation', () => { + const service = new MemoryService(createConfig()); + expect(getProvider(service)).toBeInstanceOf(DefaultMemoryProvider); + }); - const lockPath = path.join(tmpDir, '.extraction.lock'); - const result = await tryAcquireLock(lockPath); + describe('lifecycle delegation', () => { + it('delegates onSessionStart to the provider', async () => { + const config = createConfig(); + const service = new MemoryService(config); - expect(result).toBe(true); - - const content = JSON.parse(await fs.readFile(lockPath, 'utf-8')); - expect(content.pid).toBe(process.pid); - expect(content.startedAt).toBeDefined(); + await service.onSessionStart('session-123'); + expect(getProvider(service).onSessionStart).toHaveBeenCalledWith( + config, + 'session-123', + ); }); - it('returns false when lock is held by a live process', async () => { - const { tryAcquireLock } = await import('./memoryService.js'); + it('delegates getSystemInstructions to the provider', async () => { + const service = new MemoryService(createConfig()); + getProvider(service).getSystemInstructions.mockReturnValue( + 'static instructions', + ); - const lockPath = path.join(tmpDir, '.extraction.lock'); - // Write a lock with the current PID (which is alive) - const lockInfo = { - pid: process.pid, - startedAt: new Date().toISOString(), - }; - await fs.writeFile(lockPath, JSON.stringify(lockInfo)); - - const result = await tryAcquireLock(lockPath); - - expect(result).toBe(false); + const result = await service.getSystemInstructions(); + expect(result).toBe('static instructions'); }); - it('cleans up and re-acquires stale lock (dead PID)', async () => { - const { tryAcquireLock } = await import('./memoryService.js'); + it('delegates getTurnContext to the provider', async () => { + const service = new MemoryService(createConfig()); + getProvider(service).getTurnContext.mockReturnValue('relevant context'); - const lockPath = path.join(tmpDir, '.extraction.lock'); - // Use a PID that almost certainly doesn't exist - const lockInfo = { - pid: 2147483646, - startedAt: new Date().toISOString(), - }; - await fs.writeFile(lockPath, JSON.stringify(lockInfo)); - - const result = await tryAcquireLock(lockPath); - - expect(result).toBe(true); - const content = JSON.parse(await fs.readFile(lockPath, 'utf-8')); - expect(content.pid).toBe(process.pid); + const result = await service.getTurnContext('how do I deploy?'); + expect(result).toBe('relevant context'); + expect(getProvider(service).getTurnContext).toHaveBeenCalledWith( + 'how do I deploy?', + ); }); - it('cleans up and re-acquires stale lock (too old)', async () => { - const { tryAcquireLock } = await import('./memoryService.js'); + it('delegates onTurnComplete to the provider', () => { + const service = new MemoryService(createConfig()); + service.onTurnComplete('user msg', 'assistant msg'); + expect(getProvider(service).onTurnComplete).toHaveBeenCalledWith( + 'user msg', + 'assistant msg', + ); + }); - const lockPath = path.join(tmpDir, '.extraction.lock'); - // Lock from 40 minutes ago with current PID — old enough to be stale (>35min) - const oldDate = new Date(Date.now() - 40 * 60 * 1000).toISOString(); - const lockInfo = { - pid: process.pid, - startedAt: oldDate, - }; - await fs.writeFile(lockPath, JSON.stringify(lockInfo)); - - const result = await tryAcquireLock(lockPath); - - expect(result).toBe(true); - const content = JSON.parse(await fs.readFile(lockPath, 'utf-8')); - expect(content.pid).toBe(process.pid); - // The new lock should have a recent timestamp - const newLockAge = Date.now() - new Date(content.startedAt).getTime(); - expect(newLockAge).toBeLessThan(5000); + it('delegates onSessionEnd to the provider', async () => { + const service = new MemoryService(createConfig()); + await service.onSessionEnd(); + expect(getProvider(service).onSessionEnd).toHaveBeenCalled(); }); }); - describe('isLockStale', () => { - it('returns true when PID is dead', async () => { - const { isLockStale } = await import('./memoryService.js'); - - const lockPath = path.join(tmpDir, '.extraction.lock'); - const lockInfo = { - pid: 2147483646, - startedAt: new Date().toISOString(), - }; - await fs.writeFile(lockPath, JSON.stringify(lockInfo)); - - expect(await isLockStale(lockPath)).toBe(true); - }); - - it('returns true when lock is too old (>35 min)', async () => { - const { isLockStale } = await import('./memoryService.js'); - - const lockPath = path.join(tmpDir, '.extraction.lock'); - const oldDate = new Date(Date.now() - 40 * 60 * 1000).toISOString(); - const lockInfo = { - pid: process.pid, - startedAt: oldDate, - }; - await fs.writeFile(lockPath, JSON.stringify(lockInfo)); - - expect(await isLockStale(lockPath)).toBe(true); - }); - - it('returns false when PID is alive and lock is fresh', async () => { - const { isLockStale } = await import('./memoryService.js'); - - const lockPath = path.join(tmpDir, '.extraction.lock'); - const lockInfo = { - pid: process.pid, - startedAt: new Date().toISOString(), - }; - await fs.writeFile(lockPath, JSON.stringify(lockInfo)); - - expect(await isLockStale(lockPath)).toBe(false); - }); - - it('returns true when file cannot be read', async () => { - const { isLockStale } = await import('./memoryService.js'); - - const lockPath = path.join(tmpDir, 'nonexistent.lock'); - - expect(await isLockStale(lockPath)).toBe(true); - }); - }); - - describe('releaseLock', () => { - it('deletes the lock file', async () => { - const { releaseLock } = await import('./memoryService.js'); - - const lockPath = path.join(tmpDir, '.extraction.lock'); - await fs.writeFile(lockPath, '{}'); - - await releaseLock(lockPath); - - await expect(fs.access(lockPath)).rejects.toThrow(); - }); - - it('does not throw when file is already gone', async () => { - const { releaseLock } = await import('./memoryService.js'); - - const lockPath = path.join(tmpDir, 'nonexistent.lock'); - - await expect(releaseLock(lockPath)).resolves.not.toThrow(); - }); - }); - - describe('readExtractionState / writeExtractionState', () => { - it('returns default state when file does not exist', async () => { - const { readExtractionState } = await import('./memoryService.js'); - - const statePath = path.join(tmpDir, 'nonexistent-state.json'); - const state = await readExtractionState(statePath); - - expect(state).toEqual({ runs: [] }); - }); - - it('reads existing state file', async () => { - const { readExtractionState } = await import('./memoryService.js'); - - const statePath = path.join(tmpDir, '.extraction-state.json'); - const existingState: ExtractionState = { - runs: [ - { - runAt: '2025-01-01T00:00:00Z', - sessionIds: ['session-1', 'session-2'], - skillsCreated: [], - }, - ], - }; - await fs.writeFile(statePath, JSON.stringify(existingState)); - - const state = await readExtractionState(statePath); - - expect(state).toEqual(existingState); - }); - - it('writes state atomically via temp file + rename', async () => { - const { writeExtractionState, readExtractionState } = await import( - './memoryService.js' - ); - - const statePath = path.join(tmpDir, '.extraction-state.json'); - const state: ExtractionState = { - runs: [ - { - runAt: '2025-01-01T00:00:00Z', - sessionIds: ['session-abc'], - skillsCreated: [], - }, - ], - }; - - await writeExtractionState(statePath, state); - - // Verify the temp file does not linger - const files = await fs.readdir(tmpDir); - expect(files).not.toContain('.extraction-state.json.tmp'); - - // Verify the final file is readable - const readBack = await readExtractionState(statePath); - expect(readBack).toEqual(state); - }); - }); - - describe('startMemoryService', () => { - it('skips when lock is held by another instance', async () => { - const { startMemoryService } = await import('./memoryService.js'); - const { LocalAgentExecutor } = await import( - '../agents/local-executor.js' - ); - - const memoryDir = path.join(tmpDir, 'memory'); - const skillsDir = path.join(tmpDir, 'skills'); - const projectTempDir = path.join(tmpDir, 'temp'); - await fs.mkdir(memoryDir, { recursive: true }); - - // Pre-acquire the lock with current PID - const lockPath = path.join(memoryDir, '.extraction.lock'); - await fs.writeFile( - lockPath, - JSON.stringify({ - pid: process.pid, - startedAt: new Date().toISOString(), - }), - ); - - const mockConfig = { - storage: { - getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), - getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), - getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), - getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), - }, - getToolRegistry: vi.fn(), - getMessageBus: vi.fn(), - getGeminiClient: vi.fn(), - sandboxManager: undefined, - } as unknown as Parameters[0]; - - await startMemoryService(mockConfig); - - // Agent should never have been created - expect(LocalAgentExecutor.create).not.toHaveBeenCalled(); - }); - - it('skips when no unprocessed sessions exist', async () => { - const { startMemoryService } = await import('./memoryService.js'); - const { LocalAgentExecutor } = await import( - '../agents/local-executor.js' - ); - - const memoryDir = path.join(tmpDir, 'memory2'); - const skillsDir = path.join(tmpDir, 'skills2'); - const projectTempDir = path.join(tmpDir, 'temp2'); - await fs.mkdir(memoryDir, { recursive: true }); - // Create an empty chats directory - await fs.mkdir(path.join(projectTempDir, 'chats'), { recursive: true }); - - const mockConfig = { - storage: { - getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), - getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), - getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), - getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), - }, - getToolRegistry: vi.fn(), - getMessageBus: vi.fn(), - getGeminiClient: vi.fn(), - sandboxManager: undefined, - } as unknown as Parameters[0]; - - await startMemoryService(mockConfig); - - expect(LocalAgentExecutor.create).not.toHaveBeenCalled(); - - // Lock should be released - const lockPath = path.join(memoryDir, '.extraction.lock'); - await expect(fs.access(lockPath)).rejects.toThrow(); - }); - - it('releases lock on error', async () => { - const { startMemoryService } = await import('./memoryService.js'); - const { LocalAgentExecutor } = await import( - '../agents/local-executor.js' - ); - const { ExecutionLifecycleService } = await import( - './executionLifecycleService.js' - ); - - const memoryDir = path.join(tmpDir, 'memory3'); - const skillsDir = path.join(tmpDir, 'skills3'); - const projectTempDir = path.join(tmpDir, 'temp3'); - const chatsDir = path.join(projectTempDir, 'chats'); - await fs.mkdir(memoryDir, { recursive: true }); - await fs.mkdir(chatsDir, { recursive: true }); - - // Write a valid session that will pass all filters - const conversation = createConversation({ - sessionId: 'error-session', - messageCount: 20, + describe('error isolation', () => { + it('catches and logs onSessionStart errors', async () => { + const service = new MemoryService(createConfig()); + getProvider(service).onSessionStart.mockImplementation(() => { + throw new Error('startup boom'); }); - await fs.writeFile( - path.join(chatsDir, 'session-2025-01-01T00-00-err00001.json'), - JSON.stringify(conversation), - ); - // Make LocalAgentExecutor.create throw - vi.mocked(LocalAgentExecutor.create).mockRejectedValueOnce( - new Error('Agent creation failed'), - ); - - const mockConfig = { - storage: { - getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), - getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), - getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), - getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), - }, - getToolRegistry: vi.fn(), - getMessageBus: vi.fn(), - getGeminiClient: vi.fn(), - sandboxManager: undefined, - } as unknown as Parameters[0]; - - await startMemoryService(mockConfig); - - // Lock should be released despite the error - const lockPath = path.join(memoryDir, '.extraction.lock'); - await expect(fs.access(lockPath)).rejects.toThrow(); - - // ExecutionLifecycleService.completeExecution should have been called with error - expect(ExecutionLifecycleService.completeExecution).toHaveBeenCalledWith( - 42, - expect.objectContaining({ - error: expect.any(Error), - }), + await expect(service.onSessionStart('s1')).resolves.toBeUndefined(); + expect(debugLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('threw during onSessionStart'), + expect.any(Error), ); }); - it('emits feedback when new skills are created during extraction', async () => { - const { startMemoryService } = await import('./memoryService.js'); - const { LocalAgentExecutor } = await import( - '../agents/local-executor.js' - ); - - // Reset mocks that may carry state from prior tests - vi.mocked(coreEvents.emitFeedback).mockClear(); - vi.mocked(LocalAgentExecutor.create).mockReset(); - - const memoryDir = path.join(tmpDir, 'memory4'); - const skillsDir = path.join(tmpDir, 'skills4'); - const projectTempDir = path.join(tmpDir, 'temp4'); - const chatsDir = path.join(projectTempDir, 'chats'); - await fs.mkdir(memoryDir, { recursive: true }); - await fs.mkdir(skillsDir, { recursive: true }); - await fs.mkdir(chatsDir, { recursive: true }); - - // Write a valid session with enough messages to pass the filter - const conversation = createConversation({ - sessionId: 'skill-session', - messageCount: 20, + it('catches and logs getSystemInstructions errors, returning empty string', async () => { + const service = new MemoryService(createConfig()); + getProvider(service).getSystemInstructions.mockImplementation(() => { + throw new Error('instructions boom'); }); - await fs.writeFile( - path.join(chatsDir, 'session-2025-01-01T00-00-skill001.json'), - JSON.stringify(conversation), - ); - // Override LocalAgentExecutor.create to return an executor whose run - // creates a new skill directory with a SKILL.md in the skillsDir - vi.mocked(LocalAgentExecutor.create).mockResolvedValueOnce({ - run: vi.fn().mockImplementation(async () => { - const newSkillDir = path.join(skillsDir, 'my-new-skill'); - await fs.mkdir(newSkillDir, { recursive: true }); - await fs.writeFile( - path.join(newSkillDir, 'SKILL.md'), - '# My New Skill', - ); - return undefined; - }), - } as never); - - const mockConfig = { - storage: { - getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), - getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), - getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), - getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), - }, - getToolRegistry: vi.fn(), - getMessageBus: vi.fn(), - getGeminiClient: vi.fn(), - getSkillManager: vi.fn().mockReturnValue({ getSkills: () => [] }), - modelConfigService: { - registerRuntimeModelConfig: vi.fn(), - }, - sandboxManager: undefined, - } as unknown as Parameters[0]; - - await startMemoryService(mockConfig); - - expect(coreEvents.emitFeedback).toHaveBeenCalledWith( - 'info', - expect.stringContaining('my-new-skill'), - ); - expect(coreEvents.emitFeedback).toHaveBeenCalledWith( - 'info', - expect.stringContaining('/memory inbox'), + const result = await service.getSystemInstructions(); + expect(result).toBe(''); + expect(debugLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('threw during getSystemInstructions'), + expect.any(Error), ); }); - }); - describe('getProcessedSessionIds', () => { - it('returns empty set for empty state', async () => { - const { getProcessedSessionIds } = await import('./memoryService.js'); - - const result = getProcessedSessionIds({ runs: [] }); - - expect(result).toBeInstanceOf(Set); - expect(result.size).toBe(0); - }); - - it('collects session IDs across multiple runs', async () => { - const { getProcessedSessionIds } = await import('./memoryService.js'); - - const state: ExtractionState = { - runs: [ - { - runAt: '2025-01-01T00:00:00Z', - sessionIds: ['s1', 's2'], - skillsCreated: [], - }, - { - runAt: '2025-01-02T00:00:00Z', - sessionIds: ['s3'], - skillsCreated: [], - }, - ], - }; - - const result = getProcessedSessionIds(state); - - expect(result).toEqual(new Set(['s1', 's2', 's3'])); - }); - - it('deduplicates IDs that appear in multiple runs', async () => { - const { getProcessedSessionIds } = await import('./memoryService.js'); - - const state: ExtractionState = { - runs: [ - { - runAt: '2025-01-01T00:00:00Z', - sessionIds: ['s1', 's2'], - skillsCreated: [], - }, - { - runAt: '2025-01-02T00:00:00Z', - sessionIds: ['s2', 's3'], - skillsCreated: [], - }, - ], - }; - - const result = getProcessedSessionIds(state); - - expect(result.size).toBe(3); - expect(result).toEqual(new Set(['s1', 's2', 's3'])); - }); - }); - - describe('buildSessionIndex', () => { - let chatsDir: string; - - beforeEach(async () => { - chatsDir = path.join(tmpDir, 'chats'); - await fs.mkdir(chatsDir, { recursive: true }); - }); - - it('returns empty index and no new IDs when chats dir is empty', async () => { - const { buildSessionIndex } = await import('./memoryService.js'); - - const result = await buildSessionIndex(chatsDir, { runs: [] }); - - expect(result.sessionIndex).toBe(''); - expect(result.newSessionIds).toEqual([]); - }); - - it('returns empty index when chats dir does not exist', async () => { - const { buildSessionIndex } = await import('./memoryService.js'); - - const nonexistentDir = path.join(tmpDir, 'no-such-dir'); - const result = await buildSessionIndex(nonexistentDir, { runs: [] }); - - expect(result.sessionIndex).toBe(''); - expect(result.newSessionIds).toEqual([]); - }); - - it('marks sessions as [NEW] when not in any previous run', async () => { - const { buildSessionIndex } = await import('./memoryService.js'); - - const conversation = createConversation({ - sessionId: 'brand-new', - summary: 'A brand new session', - messageCount: 20, + it('catches and logs getTurnContext errors, returning empty string', async () => { + const service = new MemoryService(createConfig()); + getProvider(service).getTurnContext.mockImplementation(() => { + throw new Error('context boom'); }); - await fs.writeFile( - path.join( - chatsDir, - `${SESSION_FILE_PREFIX}2025-01-01T00-00-brandnew.json`, - ), - JSON.stringify(conversation), + + const result = await service.getTurnContext('query'); + expect(result).toBe(''); + expect(debugLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('threw during getTurnContext'), + expect.any(Error), ); - - const result = await buildSessionIndex(chatsDir, { runs: [] }); - - expect(result.sessionIndex).toContain('[NEW]'); - expect(result.sessionIndex).not.toContain('[old]'); }); - it('marks sessions as [old] when already in a previous run', async () => { - const { buildSessionIndex } = await import('./memoryService.js'); - - const conversation = createConversation({ - sessionId: 'old-session', - summary: 'An old session', - messageCount: 20, + it('catches and logs synchronous onTurnComplete errors', () => { + const service = new MemoryService(createConfig()); + getProvider(service).onTurnComplete.mockImplementation(() => { + throw new Error('turn boom'); }); - await fs.writeFile( - path.join( - chatsDir, - `${SESSION_FILE_PREFIX}2025-01-01T00-00-oldsess1.json`, - ), - JSON.stringify(conversation), + + expect(() => service.onTurnComplete('u', 'a')).not.toThrow(); + expect(debugLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('threw during onTurnComplete'), + expect.any(Error), ); - - const state: ExtractionState = { - runs: [ - { - runAt: '2025-01-01T00:00:00Z', - sessionIds: ['old-session'], - skillsCreated: [], - }, - ], - }; - - const result = await buildSessionIndex(chatsDir, state); - - expect(result.sessionIndex).toContain('[old]'); - expect(result.sessionIndex).not.toContain('[NEW]'); }); - it('includes file path and summary in each line', async () => { - const { buildSessionIndex } = await import('./memoryService.js'); - - const conversation = createConversation({ - sessionId: 'detailed-session', - summary: 'Debugging the login flow', - messageCount: 20, + it('catches and logs onSessionEnd errors', async () => { + const service = new MemoryService(createConfig()); + getProvider(service).onSessionEnd.mockImplementation(() => { + throw new Error('end boom'); }); - const fileName = `${SESSION_FILE_PREFIX}2025-01-01T00-00-detail01.json`; - await fs.writeFile( - path.join(chatsDir, fileName), - JSON.stringify(conversation), + + await expect(service.onSessionEnd()).resolves.toBeUndefined(); + expect(debugLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('threw during onSessionEnd'), + expect.any(Error), ); - - const result = await buildSessionIndex(chatsDir, { runs: [] }); - - expect(result.sessionIndex).toContain('Debugging the login flow'); - expect(result.sessionIndex).toContain(path.join(chatsDir, fileName)); - }); - - it('filters out subagent sessions', async () => { - const { buildSessionIndex } = await import('./memoryService.js'); - - const conversation = createConversation({ - sessionId: 'sub-session', - kind: 'subagent', - messageCount: 20, - }); - await fs.writeFile( - path.join( - chatsDir, - `${SESSION_FILE_PREFIX}2025-01-01T00-00-sub00001.json`, - ), - JSON.stringify(conversation), - ); - - const result = await buildSessionIndex(chatsDir, { runs: [] }); - - expect(result.sessionIndex).toBe(''); - expect(result.newSessionIds).toEqual([]); - }); - - it('filters out sessions with fewer than 10 user messages', async () => { - const { buildSessionIndex } = await import('./memoryService.js'); - - // 2 messages total: 1 user (index 0) + 1 gemini (index 1) - const conversation = createConversation({ - sessionId: 'short-session', - messageCount: 2, - }); - await fs.writeFile( - path.join( - chatsDir, - `${SESSION_FILE_PREFIX}2025-01-01T00-00-short001.json`, - ), - JSON.stringify(conversation), - ); - - const result = await buildSessionIndex(chatsDir, { runs: [] }); - - expect(result.sessionIndex).toBe(''); - expect(result.newSessionIds).toEqual([]); - }); - - it('caps at MAX_SESSION_INDEX_SIZE (50)', async () => { - const { buildSessionIndex } = await import('./memoryService.js'); - - // Create 3 eligible sessions, verify all 3 appear (well under cap) - for (let i = 0; i < 3; i++) { - const conversation = createConversation({ - sessionId: `capped-session-${i}`, - summary: `Summary ${i}`, - messageCount: 20, - }); - const paddedIndex = String(i).padStart(4, '0'); - await fs.writeFile( - path.join( - chatsDir, - `${SESSION_FILE_PREFIX}2025-01-0${i + 1}T00-00-cap${paddedIndex}.json`, - ), - JSON.stringify(conversation), - ); - } - - const result = await buildSessionIndex(chatsDir, { runs: [] }); - - const lines = result.sessionIndex.split('\n').filter((l) => l.length > 0); - expect(lines).toHaveLength(3); - expect(result.newSessionIds).toHaveLength(3); - }); - - it('returns newSessionIds only for unprocessed sessions', async () => { - const { buildSessionIndex } = await import('./memoryService.js'); - - // Write two sessions: one already processed, one new - const oldConv = createConversation({ - sessionId: 'processed-one', - summary: 'Old', - messageCount: 20, - }); - await fs.writeFile( - path.join( - chatsDir, - `${SESSION_FILE_PREFIX}2025-01-01T00-00-proc0001.json`, - ), - JSON.stringify(oldConv), - ); - - const newConv = createConversation({ - sessionId: 'fresh-one', - summary: 'New', - messageCount: 20, - }); - await fs.writeFile( - path.join( - chatsDir, - `${SESSION_FILE_PREFIX}2025-01-02T00-00-fres0001.json`, - ), - JSON.stringify(newConv), - ); - - const state: ExtractionState = { - runs: [ - { - runAt: '2025-01-01T00:00:00Z', - sessionIds: ['processed-one'], - skillsCreated: [], - }, - ], - }; - - const result = await buildSessionIndex(chatsDir, state); - - expect(result.newSessionIds).toEqual(['fresh-one']); - expect(result.newSessionIds).not.toContain('processed-one'); - // Both sessions should still appear in the index - expect(result.sessionIndex).toContain('[NEW]'); - expect(result.sessionIndex).toContain('[old]'); - }); - }); - - describe('ExtractionState runs tracking', () => { - it('readExtractionState parses runs array with skillsCreated', async () => { - const { readExtractionState } = await import('./memoryService.js'); - - const statePath = path.join(tmpDir, 'state-with-skills.json'); - const state: ExtractionState = { - runs: [ - { - runAt: '2025-06-01T00:00:00Z', - sessionIds: ['s1'], - skillsCreated: ['debug-helper', 'test-gen'], - }, - ], - }; - await fs.writeFile(statePath, JSON.stringify(state)); - - const result = await readExtractionState(statePath); - - expect(result.runs).toHaveLength(1); - expect(result.runs[0].skillsCreated).toEqual([ - 'debug-helper', - 'test-gen', - ]); - expect(result.runs[0].sessionIds).toEqual(['s1']); - expect(result.runs[0].runAt).toBe('2025-06-01T00:00:00Z'); - }); - - it('writeExtractionState + readExtractionState roundtrips runs correctly', async () => { - const { writeExtractionState, readExtractionState } = await import( - './memoryService.js' - ); - - const statePath = path.join(tmpDir, 'roundtrip-state.json'); - const runs: ExtractionRun[] = [ - { - runAt: '2025-01-01T00:00:00Z', - sessionIds: ['a', 'b'], - skillsCreated: ['skill-x'], - }, - { - runAt: '2025-01-02T00:00:00Z', - sessionIds: ['c'], - skillsCreated: [], - }, - ]; - const state: ExtractionState = { runs }; - - await writeExtractionState(statePath, state); - const result = await readExtractionState(statePath); - - expect(result).toEqual(state); - }); - - it('readExtractionState handles old format without runs', async () => { - const { readExtractionState } = await import('./memoryService.js'); - - const statePath = path.join(tmpDir, 'old-format-state.json'); - // Old format: an object without a runs array - await fs.writeFile( - statePath, - JSON.stringify({ lastProcessed: '2025-01-01' }), - ); - - const result = await readExtractionState(statePath); - - expect(result).toEqual({ runs: [] }); - }); - }); - - describe('validatePatches', () => { - let skillsDir: string; - let globalSkillsDir: string; - let projectSkillsDir: string; - let validateConfig: Config; - - beforeEach(() => { - skillsDir = path.join(tmpDir, 'skills'); - globalSkillsDir = path.join(tmpDir, 'global-skills'); - projectSkillsDir = path.join(tmpDir, 'project-skills'); - - vi.mocked(Storage.getUserSkillsDir).mockReturnValue(globalSkillsDir); - validateConfig = { - storage: { - getProjectSkillsDir: () => projectSkillsDir, - }, - } as unknown as Config; - }); - - it('returns empty array when no patch files exist', async () => { - const { validatePatches } = await import('./memoryService.js'); - - await fs.mkdir(skillsDir, { recursive: true }); - // Add a non-patch file to ensure it's ignored - await fs.writeFile(path.join(skillsDir, 'some-file.txt'), 'hello'); - - const result = await validatePatches(skillsDir, validateConfig); - - expect(result).toEqual([]); - }); - - it('returns empty array when directory does not exist', async () => { - const { validatePatches } = await import('./memoryService.js'); - - const result = await validatePatches( - path.join(tmpDir, 'nonexistent-dir'), - validateConfig, - ); - - expect(result).toEqual([]); - }); - - it('removes invalid patch files', async () => { - const { validatePatches } = await import('./memoryService.js'); - - await fs.mkdir(skillsDir, { recursive: true }); - - // Write a malformed patch - const patchPath = path.join(skillsDir, 'bad-skill.patch'); - await fs.writeFile(patchPath, 'this is not a valid patch'); - - const result = await validatePatches(skillsDir, validateConfig); - - expect(result).toEqual([]); - // Verify the invalid patch was deleted - await expect(fs.access(patchPath)).rejects.toThrow(); - }); - - it('keeps valid patch files', async () => { - const { validatePatches } = await import('./memoryService.js'); - - await fs.mkdir(skillsDir, { recursive: true }); - await fs.mkdir(projectSkillsDir, { recursive: true }); - - // Create a real target file to patch - const targetFile = path.join(projectSkillsDir, 'target.md'); - await fs.writeFile(targetFile, 'line1\nline2\nline3\n'); - - // Write a valid unified diff patch with absolute paths - const patchContent = [ - `--- ${targetFile}`, - `+++ ${targetFile}`, - '@@ -1,3 +1,4 @@', - ' line1', - ' line2', - '+line2.5', - ' line3', - '', - ].join('\n'); - const patchPath = path.join(skillsDir, 'good-skill.patch'); - await fs.writeFile(patchPath, patchContent); - - const result = await validatePatches(skillsDir, validateConfig); - - expect(result).toEqual(['good-skill.patch']); - // Verify the valid patch still exists - await expect(fs.access(patchPath)).resolves.toBeUndefined(); - }); - - it('keeps patches with repeated sections for the same file when hunks apply cumulatively', async () => { - const { validatePatches } = await import('./memoryService.js'); - - await fs.mkdir(skillsDir, { recursive: true }); - await fs.mkdir(projectSkillsDir, { recursive: true }); - - const targetFile = path.join(projectSkillsDir, 'target.md'); - await fs.writeFile(targetFile, 'alpha\nbeta\ngamma\ndelta\n'); - - const patchPath = path.join(skillsDir, 'multi-section.patch'); - await fs.writeFile( - patchPath, - [ - `--- ${targetFile}`, - `+++ ${targetFile}`, - '@@ -1,4 +1,5 @@', - ' alpha', - ' beta', - '+beta2', - ' gamma', - ' delta', - `--- ${targetFile}`, - `+++ ${targetFile}`, - '@@ -2,4 +2,5 @@', - ' beta', - ' beta2', - ' gamma', - '+gamma2', - ' delta', - '', - ].join('\n'), - ); - - const result = await validatePatches(skillsDir, validateConfig); - - expect(result).toEqual(['multi-section.patch']); - await expect(fs.access(patchPath)).resolves.toBeUndefined(); - }); - - it('removes /dev/null patches that target an existing skill file', async () => { - const { validatePatches } = await import('./memoryService.js'); - - await fs.mkdir(skillsDir, { recursive: true }); - await fs.mkdir(projectSkillsDir, { recursive: true }); - - const targetFile = path.join(projectSkillsDir, 'existing-skill.md'); - await fs.writeFile(targetFile, 'original content\n'); - - const patchPath = path.join(skillsDir, 'bad-new-file.patch'); - await fs.writeFile( - patchPath, - [ - '--- /dev/null', - `+++ ${targetFile}`, - '@@ -0,0 +1 @@', - '+replacement content', - '', - ].join('\n'), - ); - - const result = await validatePatches(skillsDir, validateConfig); - - expect(result).toEqual([]); - await expect(fs.access(patchPath)).rejects.toThrow(); - expect(await fs.readFile(targetFile, 'utf-8')).toBe('original content\n'); - }); - - it('removes patches with malformed diff headers', async () => { - const { validatePatches } = await import('./memoryService.js'); - - await fs.mkdir(skillsDir, { recursive: true }); - await fs.mkdir(projectSkillsDir, { recursive: true }); - - const targetFile = path.join(projectSkillsDir, 'target.md'); - await fs.writeFile(targetFile, 'line1\nline2\nline3\n'); - - const patchPath = path.join(skillsDir, 'bad-headers.patch'); - await fs.writeFile( - patchPath, - [ - `--- ${targetFile}`, - '+++ .gemini/skills/foo/SKILL.md', - '@@ -1,3 +1,4 @@', - ' line1', - ' line2', - '+line2.5', - ' line3', - '', - ].join('\n'), - ); - - const result = await validatePatches(skillsDir, validateConfig); - - expect(result).toEqual([]); - await expect(fs.access(patchPath)).rejects.toThrow(); - expect(await fs.readFile(targetFile, 'utf-8')).toBe( - 'line1\nline2\nline3\n', - ); - }); - - it('removes patches that contain no hunks', async () => { - const { validatePatches } = await import('./memoryService.js'); - - await fs.mkdir(skillsDir, { recursive: true }); - const patchPath = path.join(skillsDir, 'empty.patch'); - await fs.writeFile( - patchPath, - [ - `--- ${path.join(projectSkillsDir, 'target.md')}`, - `+++ ${path.join(projectSkillsDir, 'target.md')}`, - '', - ].join('\n'), - ); - - const result = await validatePatches(skillsDir, validateConfig); - - expect(result).toEqual([]); - await expect(fs.access(patchPath)).rejects.toThrow(); - }); - - it('removes patches that target files outside the allowed skill roots', async () => { - const { validatePatches } = await import('./memoryService.js'); - - await fs.mkdir(skillsDir, { recursive: true }); - const outsideFile = path.join(tmpDir, 'outside.md'); - await fs.writeFile(outsideFile, 'line1\nline2\nline3\n'); - - const patchPath = path.join(skillsDir, 'outside.patch'); - await fs.writeFile( - patchPath, - [ - `--- ${outsideFile}`, - `+++ ${outsideFile}`, - '@@ -1,3 +1,4 @@', - ' line1', - ' line2', - '+line2.5', - ' line3', - '', - ].join('\n'), - ); - - const result = await validatePatches(skillsDir, validateConfig); - - expect(result).toEqual([]); - await expect(fs.access(patchPath)).rejects.toThrow(); - }); - - it('removes patches that escape the allowed roots through a symlinked parent', async () => { - const { validatePatches } = await import('./memoryService.js'); - - await fs.mkdir(skillsDir, { recursive: true }); - await fs.mkdir(projectSkillsDir, { recursive: true }); - - const outsideDir = path.join(tmpDir, 'outside-dir'); - const linkedDir = path.join(projectSkillsDir, 'linked'); - await fs.mkdir(outsideDir, { recursive: true }); - await fs.symlink( - outsideDir, - linkedDir, - process.platform === 'win32' ? 'junction' : 'dir', - ); - - const outsideFile = path.join(outsideDir, 'escaped.md'); - await fs.writeFile(outsideFile, 'line1\nline2\nline3\n'); - - const patchPath = path.join(skillsDir, 'symlink.patch'); - await fs.writeFile( - patchPath, - [ - `--- ${path.join(linkedDir, 'escaped.md')}`, - `+++ ${path.join(linkedDir, 'escaped.md')}`, - '@@ -1,3 +1,4 @@', - ' line1', - ' line2', - '+line2.5', - ' line3', - '', - ].join('\n'), - ); - - const result = await validatePatches(skillsDir, validateConfig); - - expect(result).toEqual([]); - await expect(fs.access(patchPath)).rejects.toThrow(); - expect(await fs.readFile(outsideFile, 'utf-8')).not.toContain('line2.5'); - }); - }); - - describe('startMemoryService feedback for patch-only runs', () => { - it('emits feedback when extraction produces only patch suggestions', async () => { - const { startMemoryService } = await import('./memoryService.js'); - const { LocalAgentExecutor } = await import( - '../agents/local-executor.js' - ); - - vi.mocked(coreEvents.emitFeedback).mockClear(); - vi.mocked(LocalAgentExecutor.create).mockReset(); - - const memoryDir = path.join(tmpDir, 'memory-patch-only'); - const skillsDir = path.join(tmpDir, 'skills-patch-only'); - const projectTempDir = path.join(tmpDir, 'temp-patch-only'); - const chatsDir = path.join(projectTempDir, 'chats'); - const projectSkillsDir = path.join(tmpDir, 'workspace-skills'); - await fs.mkdir(memoryDir, { recursive: true }); - await fs.mkdir(skillsDir, { recursive: true }); - await fs.mkdir(chatsDir, { recursive: true }); - await fs.mkdir(projectSkillsDir, { recursive: true }); - - const existingSkill = path.join(projectSkillsDir, 'existing-skill.md'); - await fs.writeFile(existingSkill, 'line1\nline2\nline3\n'); - - const conversation = createConversation({ - sessionId: 'patch-only-session', - messageCount: 20, - }); - await fs.writeFile( - path.join(chatsDir, 'session-2025-01-01T00-00-patchonly.json'), - JSON.stringify(conversation), - ); - - vi.mocked(Storage.getUserSkillsDir).mockReturnValue( - path.join(tmpDir, 'global-skills'), - ); - vi.mocked(LocalAgentExecutor.create).mockResolvedValueOnce({ - run: vi.fn().mockImplementation(async () => { - const patchPath = path.join(skillsDir, 'existing-skill.patch'); - await fs.writeFile( - patchPath, - [ - `--- ${existingSkill}`, - `+++ ${existingSkill}`, - '@@ -1,3 +1,4 @@', - ' line1', - ' line2', - '+line2.5', - ' line3', - '', - ].join('\n'), - ); - return undefined; - }), - } as never); - - const mockConfig = { - storage: { - getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), - getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), - getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), - getProjectSkillsDir: vi.fn().mockReturnValue(projectSkillsDir), - getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), - }, - getToolRegistry: vi.fn(), - getMessageBus: vi.fn(), - getGeminiClient: vi.fn(), - getSkillManager: vi.fn().mockReturnValue({ getSkills: () => [] }), - modelConfigService: { - registerRuntimeModelConfig: vi.fn(), - }, - sandboxManager: undefined, - } as unknown as Parameters[0]; - - await startMemoryService(mockConfig); - - expect(coreEvents.emitFeedback).toHaveBeenCalledWith( - 'info', - expect.stringContaining('skill update'), - ); - expect(coreEvents.emitFeedback).toHaveBeenCalledWith( - 'info', - expect.stringContaining('/memory inbox'), - ); - }); - - it('does not emit feedback for old inbox patches when this run creates none', async () => { - const { startMemoryService } = await import('./memoryService.js'); - const { LocalAgentExecutor } = await import( - '../agents/local-executor.js' - ); - - vi.mocked(coreEvents.emitFeedback).mockClear(); - vi.mocked(LocalAgentExecutor.create).mockReset(); - - const memoryDir = path.join(tmpDir, 'memory-old-patch'); - const skillsDir = path.join(tmpDir, 'skills-old-patch'); - const projectTempDir = path.join(tmpDir, 'temp-old-patch'); - const chatsDir = path.join(projectTempDir, 'chats'); - const projectSkillsDir = path.join(tmpDir, 'workspace-old-patch'); - await fs.mkdir(memoryDir, { recursive: true }); - await fs.mkdir(skillsDir, { recursive: true }); - await fs.mkdir(chatsDir, { recursive: true }); - await fs.mkdir(projectSkillsDir, { recursive: true }); - - const existingSkill = path.join(projectSkillsDir, 'existing-skill.md'); - await fs.writeFile(existingSkill, 'line1\nline2\nline3\n'); - await fs.writeFile( - path.join(skillsDir, 'existing-skill.patch'), - [ - `--- ${existingSkill}`, - `+++ ${existingSkill}`, - '@@ -1,3 +1,4 @@', - ' line1', - ' line2', - '+line2.5', - ' line3', - '', - ].join('\n'), - ); - - const conversation = createConversation({ - sessionId: 'old-patch-session', - messageCount: 20, - }); - await fs.writeFile( - path.join(chatsDir, 'session-2025-01-01T00-00-oldpatch.json'), - JSON.stringify(conversation), - ); - - vi.mocked(Storage.getUserSkillsDir).mockReturnValue( - path.join(tmpDir, 'global-skills'), - ); - vi.mocked(LocalAgentExecutor.create).mockResolvedValueOnce({ - run: vi.fn().mockResolvedValue(undefined), - } as never); - - const mockConfig = { - storage: { - getProjectMemoryDir: vi.fn().mockReturnValue(memoryDir), - getProjectMemoryTempDir: vi.fn().mockReturnValue(memoryDir), - getProjectSkillsMemoryDir: vi.fn().mockReturnValue(skillsDir), - getProjectSkillsDir: vi.fn().mockReturnValue(projectSkillsDir), - getProjectTempDir: vi.fn().mockReturnValue(projectTempDir), - }, - getToolRegistry: vi.fn(), - getMessageBus: vi.fn(), - getGeminiClient: vi.fn(), - getSkillManager: vi.fn().mockReturnValue({ getSkills: () => [] }), - modelConfigService: { - registerRuntimeModelConfig: vi.fn(), - }, - sandboxManager: undefined, - } as unknown as Parameters[0]; - - await startMemoryService(mockConfig); - - expect(coreEvents.emitFeedback).not.toHaveBeenCalled(); }); }); }); diff --git a/packages/core/src/services/memoryService.ts b/packages/core/src/services/memoryService.ts index 29b2b18701..090ae40375 100644 --- a/packages/core/src/services/memoryService.ts +++ b/packages/core/src/services/memoryService.ts @@ -4,814 +4,81 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as fs from 'node:fs/promises'; -import * as path from 'node:path'; -import { constants as fsConstants } from 'node:fs'; -import { randomUUID } from 'node:crypto'; -import * as Diff from 'diff'; import type { Config } from '../config/config.js'; -import { - SESSION_FILE_PREFIX, - type ConversationRecord, -} from './chatRecordingService.js'; +import { DefaultMemoryProvider } from './defaultMemoryProvider.js'; +import type { MemoryProvider } from './memoryProvider.js'; import { debugLogger } from '../utils/debugLogger.js'; -import { coreEvents } from '../utils/events.js'; -import { isNodeError } from '../utils/errors.js'; -import { FRONTMATTER_REGEX, parseFrontmatter } from '../skills/skillLoader.js'; -import { LocalAgentExecutor } from '../agents/local-executor.js'; -import { SkillExtractionAgent } from '../agents/skill-extraction-agent.js'; -import { getModelConfigAlias } from '../agents/registry.js'; -import { ExecutionLifecycleService } from './executionLifecycleService.js'; -import { PromptRegistry } from '../prompts/prompt-registry.js'; -import { ResourceRegistry } from '../resources/resource-registry.js'; -import { PolicyEngine } from '../policy/policy-engine.js'; -import { PolicyDecision } from '../policy/types.js'; -import { MessageBus } from '../confirmation-bus/message-bus.js'; -import { Storage } from '../config/storage.js'; -import type { AgentLoopContext } from '../config/agent-loop-context.js'; -import { - applyParsedSkillPatches, - hasParsedPatchHunks, -} from './memoryPatchUtils.js'; - -const LOCK_FILENAME = '.extraction.lock'; -const STATE_FILENAME = '.extraction-state.json'; -const LOCK_STALE_MS = 35 * 60 * 1000; // 35 minutes (exceeds agent's 30-min time limit) -const MIN_USER_MESSAGES = 10; -const MIN_IDLE_MS = 3 * 60 * 60 * 1000; // 3 hours -const MAX_SESSION_INDEX_SIZE = 50; /** - * Lock file content for coordinating across CLI instances. - */ -interface LockInfo { - pid: number; - startedAt: string; -} - -/** - * Metadata for a single extraction run. - */ -export interface ExtractionRun { - runAt: string; - sessionIds: string[]; - skillsCreated: string[]; -} - -/** - * Tracks extraction history with per-run metadata. - */ -export interface ExtractionState { - runs: ExtractionRun[]; -} - -/** - * Returns all session IDs that have been processed across all runs. - */ -export function getProcessedSessionIds(state: ExtractionState): Set { - const ids = new Set(); - for (const run of state.runs) { - for (const id of run.sessionIds) { - ids.add(id); - } - } - return ids; -} - -function isLockInfo(value: unknown): value is LockInfo { - return ( - typeof value === 'object' && - value !== null && - 'pid' in value && - typeof value.pid === 'number' && - 'startedAt' in value && - typeof value.startedAt === 'string' - ); -} - -function isConversationRecord(value: unknown): value is ConversationRecord { - return ( - typeof value === 'object' && - value !== null && - 'sessionId' in value && - typeof value.sessionId === 'string' && - 'messages' in value && - Array.isArray(value.messages) && - 'projectHash' in value && - 'startTime' in value && - 'lastUpdated' in value - ); -} - -function isExtractionRun(value: unknown): value is ExtractionRun { - return ( - typeof value === 'object' && - value !== null && - 'runAt' in value && - typeof value.runAt === 'string' && - 'sessionIds' in value && - Array.isArray(value.sessionIds) && - 'skillsCreated' in value && - Array.isArray(value.skillsCreated) - ); -} - -function isExtractionState(value: unknown): value is { runs: unknown[] } { - return ( - typeof value === 'object' && - value !== null && - 'runs' in value && - Array.isArray(value.runs) - ); -} - -/** - * Attempts to acquire an exclusive lock file using O_CREAT | O_EXCL. - * Returns true if the lock was acquired, false if another instance owns it. - */ -export async function tryAcquireLock( - lockPath: string, - retries = 1, -): Promise { - const lockInfo: LockInfo = { - pid: process.pid, - startedAt: new Date().toISOString(), - }; - - try { - // Atomic create-if-not-exists - const fd = await fs.open( - lockPath, - fsConstants.O_CREAT | fsConstants.O_EXCL | fsConstants.O_WRONLY, - ); - try { - await fd.writeFile(JSON.stringify(lockInfo)); - } finally { - await fd.close(); - } - return true; - } catch (error: unknown) { - if (isNodeError(error) && error.code === 'EEXIST') { - // Lock exists — check if it's stale - if (retries > 0 && (await isLockStale(lockPath))) { - debugLogger.debug('[MemoryService] Cleaning up stale lock file'); - await releaseLock(lockPath); - return tryAcquireLock(lockPath, retries - 1); - } - debugLogger.debug( - '[MemoryService] Lock held by another instance, skipping', - ); - return false; - } - throw error; - } -} - -/** - * Checks if a lock file is stale (owner PID is dead or lock is too old). - */ -export async function isLockStale(lockPath: string): Promise { - try { - const content = await fs.readFile(lockPath, 'utf-8'); - const parsed: unknown = JSON.parse(content); - if (!isLockInfo(parsed)) { - return true; // Invalid lock data — treat as stale - } - const lockInfo = parsed; - - // Check if PID is still alive - try { - process.kill(lockInfo.pid, 0); - } catch { - // PID is dead — lock is stale - return true; - } - - // Check if lock is too old - const lockAge = Date.now() - new Date(lockInfo.startedAt).getTime(); - if (lockAge > LOCK_STALE_MS) { - return true; - } - - return false; - } catch { - // Can't read lock — treat as stale - return true; - } -} - -/** - * Releases the lock file. - */ -export async function releaseLock(lockPath: string): Promise { - try { - await fs.unlink(lockPath); - } catch (error: unknown) { - if (isNodeError(error) && error.code === 'ENOENT') { - return; // Already removed - } - debugLogger.warn( - `[MemoryService] Failed to release lock: ${error instanceof Error ? error.message : String(error)}`, - ); - } -} - -/** - * Reads the extraction state file, or returns a default state. - */ -export async function readExtractionState( - statePath: string, -): Promise { - try { - const content = await fs.readFile(statePath, 'utf-8'); - const parsed: unknown = JSON.parse(content); - if (!isExtractionState(parsed)) { - return { runs: [] }; - } - - const runs: ExtractionRun[] = []; - for (const run of parsed.runs) { - if (!isExtractionRun(run)) continue; - runs.push({ - runAt: run.runAt, - sessionIds: run.sessionIds.filter( - (sid): sid is string => typeof sid === 'string', - ), - skillsCreated: run.skillsCreated.filter( - (sk): sk is string => typeof sk === 'string', - ), - }); - } - - return { runs }; - } catch (error) { - debugLogger.debug( - '[MemoryService] Failed to read extraction state:', - error, - ); - return { runs: [] }; - } -} - -/** - * Writes the extraction state atomically (temp file + rename). - */ -export async function writeExtractionState( - statePath: string, - state: ExtractionState, -): Promise { - const tmpPath = `${statePath}.tmp`; - await fs.writeFile(tmpPath, JSON.stringify(state, null, 2)); - await fs.rename(tmpPath, statePath); -} - -/** - * Determines if a conversation record should be considered for processing. - * Filters out subagent sessions, sessions that haven't been idle long enough, - * and sessions with too few user messages. - */ -function shouldProcessConversation(parsed: ConversationRecord): boolean { - // Skip subagent sessions - if (parsed.kind === 'subagent') return false; - - // Skip sessions that are still active (not idle for 3+ hours) - const lastUpdated = new Date(parsed.lastUpdated).getTime(); - if (Date.now() - lastUpdated < MIN_IDLE_MS) return false; - - // Skip sessions with too few user messages - const userMessageCount = parsed.messages.filter( - (m) => m.type === 'user', - ).length; - if (userMessageCount < MIN_USER_MESSAGES) return false; - - return true; -} - -/** - * Scans the chats directory for eligible session files (sorted most-recent-first, - * capped at MAX_SESSION_INDEX_SIZE). Shared by buildSessionIndex. - */ -async function scanEligibleSessions( - chatsDir: string, -): Promise> { - let allFiles: string[]; - try { - allFiles = await fs.readdir(chatsDir); - } catch { - return []; - } - - const sessionFiles = allFiles.filter( - (f) => f.startsWith(SESSION_FILE_PREFIX) && f.endsWith('.json'), - ); - - // Sort by filename descending (most recent first) - sessionFiles.sort((a, b) => b.localeCompare(a)); - - const results: Array<{ conversation: ConversationRecord; filePath: string }> = - []; - - for (const file of sessionFiles) { - if (results.length >= MAX_SESSION_INDEX_SIZE) break; - - const filePath = path.join(chatsDir, file); - try { - const content = await fs.readFile(filePath, 'utf-8'); - const parsed: unknown = JSON.parse(content); - if (!isConversationRecord(parsed)) continue; - if (!shouldProcessConversation(parsed)) continue; - - results.push({ conversation: parsed, filePath }); - } catch { - // Skip unreadable files - } - } - - return results; -} - -/** - * Builds a session index for the extraction agent: a compact listing of all - * eligible sessions with their summary, file path, and new/previously-processed status. - * The agent can use read_file on paths to inspect sessions that look promising. + * Orchestrates the memory subsystem for a single GeminiClient. The service + * owns a `DefaultMemoryProvider` and exposes a stable lifecycle surface + * (`onSessionStart`, `getSystemInstructions`, `getTurnContext`, + * `onTurnComplete`, `onSessionEnd`) so that GeminiClient can stay agnostic + * of the underlying implementation. Each lifecycle call is wrapped in a + * try/catch so a buggy provider can never crash a turn. * - * Returns the index text and the list of new (unprocessed) session IDs. + * The methods return `Promise`s for callsite consistency even though the + * current provider is synchronous; this keeps the boundary stable should + * the provider ever need to do asynchronous work. */ -export async function buildSessionIndex( - chatsDir: string, - state: ExtractionState, -): Promise<{ sessionIndex: string; newSessionIds: string[] }> { - const processedSet = getProcessedSessionIds(state); - const eligible = await scanEligibleSessions(chatsDir); +export class MemoryService { + private readonly provider: MemoryProvider = new DefaultMemoryProvider(); - if (eligible.length === 0) { - return { sessionIndex: '', newSessionIds: [] }; - } - - const lines: string[] = []; - const newSessionIds: string[] = []; - - for (const { conversation, filePath } of eligible) { - const userMessageCount = conversation.messages.filter( - (m) => m.type === 'user', - ).length; - const isNew = !processedSet.has(conversation.sessionId); - if (isNew) { - newSessionIds.push(conversation.sessionId); - } - - const status = isNew ? '[NEW]' : '[old]'; - const summary = conversation.summary ?? '(no summary)'; - lines.push( - `${status} ${summary} (${userMessageCount} user msgs) — ${filePath}`, - ); - } - - return { sessionIndex: lines.join('\n'), newSessionIds }; -} - -/** - * Builds a summary of all existing skills — both memory-extracted skills - * in the skillsDir and globally/workspace-discovered skills from the SkillManager. - * This prevents the extraction agent from duplicating already-available skills. - */ -async function buildExistingSkillsSummary( - skillsDir: string, - config: Config, -): Promise { - const sections: string[] = []; - - // 1. Memory-extracted skills (from previous runs) - const memorySkills: string[] = []; - try { - const entries = await fs.readdir(skillsDir, { withFileTypes: true }); - for (const entry of entries) { - if (!entry.isDirectory()) continue; - - const skillPath = path.join(skillsDir, entry.name, 'SKILL.md'); - try { - const content = await fs.readFile(skillPath, 'utf-8'); - const match = content.match(FRONTMATTER_REGEX); - if (match) { - const parsed = parseFrontmatter(match[1]); - const name = parsed?.name ?? entry.name; - const desc = parsed?.description ?? ''; - memorySkills.push(`- **${name}**: ${desc}`); - } else { - memorySkills.push(`- **${entry.name}**`); - } - } catch { - // Skill directory without SKILL.md, skip - } - } - } catch { - // Skills directory doesn't exist yet - } - - if (memorySkills.length > 0) { - sections.push( - `## Previously Extracted Skills (in ${skillsDir})\n${memorySkills.join('\n')}`, - ); - } - - // 2. Discovered skills — categorize by source location - try { - const discoveredSkills = config.getSkillManager().getSkills(); - if (discoveredSkills.length > 0) { - const userSkillsDir = Storage.getUserSkillsDir(); - const globalSkills: string[] = []; - const workspaceSkills: string[] = []; - const extensionSkills: string[] = []; - const builtinSkills: string[] = []; - - for (const s of discoveredSkills) { - const loc = s.location; - if (loc.includes('/bundle/') || loc.includes('\\bundle\\')) { - builtinSkills.push(`- **${s.name}**: ${s.description}`); - } else if (loc.startsWith(userSkillsDir)) { - globalSkills.push(`- **${s.name}**: ${s.description} (${loc})`); - } else if ( - loc.includes('/extensions/') || - loc.includes('\\extensions\\') - ) { - extensionSkills.push(`- **${s.name}**: ${s.description}`); - } else { - workspaceSkills.push(`- **${s.name}**: ${s.description} (${loc})`); - } - } - - if (globalSkills.length > 0) { - sections.push( - `## Global Skills (~/.gemini/skills — do NOT duplicate)\n${globalSkills.join('\n')}`, - ); - } - if (workspaceSkills.length > 0) { - sections.push( - `## Workspace Skills (.gemini/skills — do NOT duplicate)\n${workspaceSkills.join('\n')}`, - ); - } - if (extensionSkills.length > 0) { - sections.push( - `## Extension Skills (from installed extensions — do NOT duplicate)\n${extensionSkills.join('\n')}`, - ); - } - if (builtinSkills.length > 0) { - sections.push( - `## Builtin Skills (bundled with CLI — do NOT duplicate)\n${builtinSkills.join('\n')}`, - ); - } - } - } catch { - // SkillManager not available - } - - return sections.join('\n\n'); -} - -/** - * Builds an AgentLoopContext from a Config for background agent execution. - */ -function buildAgentLoopContext(config: Config): AgentLoopContext { - // Create a PolicyEngine that auto-approves all tool calls so the - // background sub-agent never prompts the user for confirmation. - const autoApprovePolicy = new PolicyEngine({ - rules: [ - { - toolName: '*', - decision: PolicyDecision.ALLOW, - priority: 100, - }, - ], - }); - const autoApproveBus = new MessageBus(autoApprovePolicy); - - return { - config, - promptId: `skill-extraction-${randomUUID().slice(0, 8)}`, - toolRegistry: config.getToolRegistry(), - promptRegistry: new PromptRegistry(), - resourceRegistry: new ResourceRegistry(), - messageBus: autoApproveBus, - geminiClient: config.getGeminiClient(), - sandboxManager: config.sandboxManager, - }; -} - -/** - * Validates all .patch files in the skills directory using the `diff` library. - * Parses each patch, reads the target file(s), and attempts a dry-run apply. - * Removes patches that fail validation. Returns the filenames of valid patches. - */ -export async function validatePatches( - skillsDir: string, - config: Config, -): Promise { - let entries: string[]; - try { - entries = await fs.readdir(skillsDir); - } catch { - return []; - } - - const patchFiles = entries.filter((e) => e.endsWith('.patch')); - const validPatches: string[] = []; - - for (const patchFile of patchFiles) { - const patchPath = path.join(skillsDir, patchFile); - let valid = true; - let reason = ''; + constructor(private readonly config: Config) {} + async onSessionStart(sessionId: string): Promise { try { - const patchContent = await fs.readFile(patchPath, 'utf-8'); - const parsedPatches = Diff.parsePatch(patchContent); - - if (!hasParsedPatchHunks(parsedPatches)) { - valid = false; - reason = 'no hunks found in patch'; - } else { - const applied = await applyParsedSkillPatches(parsedPatches, config); - if (!applied.success) { - valid = false; - switch (applied.reason) { - case 'missingTargetPath': - reason = 'missing target file path in patch header'; - break; - case 'invalidPatchHeaders': - reason = 'invalid diff headers'; - break; - case 'outsideAllowedRoots': - reason = `target file is outside skill roots: ${applied.targetPath}`; - break; - case 'newFileAlreadyExists': - reason = `new file target already exists: ${applied.targetPath}`; - break; - case 'targetNotFound': - reason = `target file not found: ${applied.targetPath}`; - break; - case 'doesNotApply': - reason = `patch does not apply cleanly to ${applied.targetPath}`; - break; - default: - reason = 'unknown patch validation failure'; - break; - } - } - } - } catch (err) { - valid = false; - reason = `failed to read or parse patch: ${err}`; - } - - if (valid) { - validPatches.push(patchFile); - debugLogger.log(`[MemoryService] Patch validated: ${patchFile}`); - } else { + this.provider.onSessionStart(this.config, sessionId); + } catch (error) { debugLogger.warn( - `[MemoryService] Removing invalid patch ${patchFile}: ${reason}`, + `[MemoryService] Provider "${this.provider.id}" threw during onSessionStart:`, + error, ); - try { - await fs.unlink(patchPath); - } catch { - // Best-effort cleanup - } } } - return validPatches; -} - -/** - * Main entry point for the skill extraction background task. - * Designed to be called fire-and-forget on session startup. - * - * Coordinates across multiple CLI instances via a lock file, - * scans past sessions for reusable patterns, and runs a sub-agent - * to extract and write SKILL.md files. - */ -export async function startMemoryService(config: Config): Promise { - const memoryDir = config.storage.getProjectMemoryTempDir(); - const skillsDir = config.storage.getProjectSkillsMemoryDir(); - const lockPath = path.join(memoryDir, LOCK_FILENAME); - const statePath = path.join(memoryDir, STATE_FILENAME); - const chatsDir = path.join(config.storage.getProjectTempDir(), 'chats'); - - // Ensure directories exist - await fs.mkdir(skillsDir, { recursive: true }); - - debugLogger.log(`[MemoryService] Starting. Skills dir: ${skillsDir}`); - - // Try to acquire exclusive lock - if (!(await tryAcquireLock(lockPath))) { - debugLogger.log('[MemoryService] Skipped: lock held by another instance'); - return; + async getSystemInstructions(): Promise { + try { + return this.provider.getSystemInstructions(); + } catch (error) { + debugLogger.warn( + `[MemoryService] Provider "${this.provider.id}" threw during getSystemInstructions:`, + error, + ); + return ''; + } } - debugLogger.log('[MemoryService] Lock acquired'); - // Register with ExecutionLifecycleService for background tracking - const abortController = new AbortController(); - const handle = ExecutionLifecycleService.createExecution( - '', // no initial output - () => abortController.abort(), // onKill - 'none', - undefined, // no format injection - 'Skill extraction', - 'silent', - ); - const executionId = handle.pid; - - const startTime = Date.now(); - let completionResult: { error: Error } | undefined; - try { - // Read extraction state - const state = await readExtractionState(statePath); - const previousRuns = state.runs.length; - const previouslyProcessed = getProcessedSessionIds(state).size; - debugLogger.log( - `[MemoryService] State loaded: ${previousRuns} previous run(s), ${previouslyProcessed} session(s) already processed`, - ); - - // Build session index: all eligible sessions with summaries + file paths. - // The agent decides which to read in full via read_file. - const { sessionIndex, newSessionIds } = await buildSessionIndex( - chatsDir, - state, - ); - - const totalInIndex = sessionIndex ? sessionIndex.split('\n').length : 0; - debugLogger.log( - `[MemoryService] Session scan: ${totalInIndex} eligible session(s) found, ${newSessionIds.length} new`, - ); - - if (newSessionIds.length === 0) { - debugLogger.log('[MemoryService] Skipped: no new sessions to process'); - return; - } - - // Snapshot existing skill directories before extraction - const skillsBefore = new Set(); - const patchContentsBefore = new Map(); + async getTurnContext(query: string): Promise { try { - const entries = await fs.readdir(skillsDir); - for (const e of entries) { - if (e.endsWith('.patch')) { - try { - patchContentsBefore.set( - e, - await fs.readFile(path.join(skillsDir, e), 'utf-8'), - ); - } catch { - // Ignore unreadable existing patches. - } - continue; - } - skillsBefore.add(e); - } - } catch { - // Empty skills dir - } - debugLogger.log( - `[MemoryService] ${skillsBefore.size} existing skill(s) in memory`, - ); - - // Read existing skills for context (memory-extracted + global/workspace) - const existingSkillsSummary = await buildExistingSkillsSummary( - skillsDir, - config, - ); - if (existingSkillsSummary) { - debugLogger.log( - `[MemoryService] Existing skills context:\n${existingSkillsSummary}`, + return this.provider.getTurnContext(query); + } catch (error) { + debugLogger.warn( + `[MemoryService] Provider "${this.provider.id}" threw during getTurnContext:`, + error, ); + return ''; } + } - // Build agent definition and context - const agentDefinition = SkillExtractionAgent( - skillsDir, - sessionIndex, - existingSkillsSummary, - ); - - const context = buildAgentLoopContext(config); - - // Register the agent's model config since it's not going through AgentRegistry. - const modelAlias = getModelConfigAlias(agentDefinition); - config.modelConfigService.registerRuntimeModelConfig(modelAlias, { - modelConfig: agentDefinition.modelConfig, - }); - debugLogger.log( - `[MemoryService] Starting extraction agent (model: ${agentDefinition.modelConfig.model}, maxTurns: 30, maxTime: 30min)`, - ); - - // Create and run the extraction agent - const executor = await LocalAgentExecutor.create(agentDefinition, context); - - await executor.run( - { request: 'Extract skills from the provided sessions.' }, - abortController.signal, - ); - - const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); - - // Diff skills directory to find newly created skills - const skillsCreated: string[] = []; + onTurnComplete(userMessage: string, assistantMessage: string): void { try { - const entriesAfter = await fs.readdir(skillsDir); - for (const e of entriesAfter) { - if (!skillsBefore.has(e) && !e.endsWith('.patch')) { - skillsCreated.push(e); - } - } - } catch { - // Skills dir read failed + this.provider.onTurnComplete(userMessage, assistantMessage); + } catch (error) { + debugLogger.warn( + `[MemoryService] Provider "${this.provider.id}" threw during onTurnComplete:`, + error, + ); } + } - // Validate any .patch files the agent generated - const validPatches = await validatePatches(skillsDir, config); - const patchesCreatedThisRun: string[] = []; - for (const patchFile of validPatches) { - const patchPath = path.join(skillsDir, patchFile); - let currentContent: string; - try { - currentContent = await fs.readFile(patchPath, 'utf-8'); - } catch { - continue; - } - if (patchContentsBefore.get(patchFile) !== currentContent) { - patchesCreatedThisRun.push(patchFile); - } - } - if (validPatches.length > 0) { - debugLogger.log( - `[MemoryService] ${validPatches.length} valid patch(es) currently in inbox; ${patchesCreatedThisRun.length} created or updated this run`, - ); - } - - // Record the run with full metadata - const run: ExtractionRun = { - runAt: new Date().toISOString(), - sessionIds: newSessionIds, - skillsCreated, - }; - const updatedState: ExtractionState = { - runs: [...state.runs, run], - }; - await writeExtractionState(statePath, updatedState); - - if (skillsCreated.length > 0 || patchesCreatedThisRun.length > 0) { - const completionParts: string[] = []; - if (skillsCreated.length > 0) { - completionParts.push( - `created ${skillsCreated.length} skill(s): ${skillsCreated.join(', ')}`, - ); - } - if (patchesCreatedThisRun.length > 0) { - completionParts.push( - `prepared ${patchesCreatedThisRun.length} patch(es): ${patchesCreatedThisRun.join(', ')}`, - ); - } - debugLogger.log( - `[MemoryService] Completed in ${elapsed}s. ${completionParts.join('; ')} (processed ${newSessionIds.length} session(s))`, - ); - const feedbackParts: string[] = []; - if (skillsCreated.length > 0) { - feedbackParts.push( - `${skillsCreated.length} new skill${skillsCreated.length > 1 ? 's' : ''} extracted from past sessions: ${skillsCreated.join(', ')}`, - ); - } - if (patchesCreatedThisRun.length > 0) { - feedbackParts.push( - `${patchesCreatedThisRun.length} skill update${patchesCreatedThisRun.length > 1 ? 's' : ''} extracted from past sessions`, - ); - } - coreEvents.emitFeedback( - 'info', - `${feedbackParts.join('. ')}. Use /memory inbox to review.`, - ); - } else { - debugLogger.log( - `[MemoryService] Completed in ${elapsed}s. No new skills or patches created (processed ${newSessionIds.length} session(s))`, - ); - } - } catch (error) { - const elapsed = ((Date.now() - startTime) / 1000).toFixed(1); - if (abortController.signal.aborted) { - debugLogger.log(`[MemoryService] Cancelled after ${elapsed}s`); - } else { - debugLogger.log( - `[MemoryService] Failed after ${elapsed}s: ${error instanceof Error ? error.message : String(error)}`, - ); - } - completionResult = { - error: error instanceof Error ? error : new Error(String(error)), - }; - return; - } finally { - await releaseLock(lockPath); - debugLogger.log('[MemoryService] Lock released'); - if (executionId !== undefined) { - ExecutionLifecycleService.completeExecution( - executionId, - completionResult, + async onSessionEnd(): Promise { + try { + this.provider.onSessionEnd(); + } catch (error) { + debugLogger.warn( + `[MemoryService] Provider "${this.provider.id}" threw during onSessionEnd:`, + error, ); } }