From a7beb890d093e2cf66ed1ac8debff690b75e1f6d Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Mon, 4 May 2026 12:07:13 -0700 Subject: [PATCH] feat(memory): add Auto Memory inbox flow with canonical-patch contract (#26338) --- docs/cli/settings.md | 2 +- docs/reference/configuration.md | 6 +- evals/auto_memory_contract.eval.ts | 489 +++++++++++ evals/auto_memory_modes.eval.ts | 447 ++++++++++ packages/cli/src/acp/commands/memory.ts | 25 +- packages/cli/src/config/settingsSchema.ts | 2 +- packages/cli/src/ui/commands/memoryCommand.ts | 7 +- ...oxDialog.test.tsx => InboxDialog.test.tsx} | 207 ++++- .../{SkillInboxDialog.tsx => InboxDialog.tsx} | 327 ++++++- .../core/src/agents/local-executor.test.ts | 66 ++ packages/core/src/agents/local-executor.ts | 43 +- .../src/agents/skill-extraction-agent.test.ts | 107 ++- .../core/src/agents/skill-extraction-agent.ts | 192 ++++- packages/core/src/agents/types.ts | 15 + packages/core/src/commands/memory.test.ts | 617 ++++++++++++++ packages/core/src/commands/memory.ts | 799 ++++++++++++++++++ packages/core/src/config/config.test.ts | 166 ++++ packages/core/src/config/config.ts | 101 ++- packages/core/src/config/scoped-config.ts | 39 + packages/core/src/config/storage.ts | 4 - .../core/src/services/memoryPatchUtils.ts | 68 +- .../core/src/services/memoryService.test.ts | 104 +++ packages/core/src/services/memoryService.ts | 271 +++++- schemas/settings.schema.json | 4 +- scripts/check-inbox.js | 60 ++ scripts/seed-test-inbox.js | 226 +++++ 26 files changed, 4279 insertions(+), 115 deletions(-) create mode 100644 evals/auto_memory_contract.eval.ts create mode 100644 evals/auto_memory_modes.eval.ts rename packages/cli/src/ui/components/{SkillInboxDialog.test.tsx => InboxDialog.test.tsx} (76%) rename packages/cli/src/ui/components/{SkillInboxDialog.tsx => InboxDialog.tsx} (70%) create mode 100644 scripts/check-inbox.js create mode 100644 scripts/seed-test-inbox.js diff --git a/docs/cli/settings.md b/docs/cli/settings.md index d39a0e18f7..b908356ab6 100644 --- a/docs/cli/settings.md +++ b/docs/cli/settings.md @@ -177,7 +177,7 @@ they appear in the UI. | Enable Gemma Model Router | `experimental.gemmaModelRouter.enabled` | Enable the Gemma Model Router (experimental). Requires a local endpoint serving Gemma via the Gemini API using LiteRT-LM shim. | `false` | | Auto-start LiteRT Server | `experimental.gemmaModelRouter.autoStartServer` | Automatically start the LiteRT-LM server when Gemini CLI starts and the Gemma router is enabled. | `false` | | Memory v2 | `experimental.memoryV2` | Disable the built-in save_memory tool and let the main agent persist project context by editing markdown files directly with edit/write_file. Route facts across four tiers: team-shared conventions go to project GEMINI.md files, project-specific personal notes go to the per-project private memory folder (MEMORY.md as index + sibling .md files for detail), and cross-project personal preferences go to the global ~/.gemini/GEMINI.md (the only file under ~/.gemini/ that the agent can edit — settings, credentials, etc. remain off-limits). Set to false to fall back to the legacy save_memory tool. | `true` | -| Auto Memory | `experimental.autoMemory` | Automatically extract reusable skills from past sessions in the background. Review results with /memory inbox. | `false` | +| Auto Memory | `experimental.autoMemory` | Automatically extract memory patches and skills from past sessions in the background. Every change is written as a unified diff `.patch` file under `/.inbox//` and held for review in /memory inbox; nothing is applied until you approve it. | `false` | | Use the generalist profile to manage agent contexts. | `experimental.generalistProfile` | Suitable for general coding and software development tasks. | `false` | | Enable Context Management | `experimental.contextManagement` | Enable logic for context management. | `false` | diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index 3498634dd1..c75db12364 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -1927,8 +1927,10 @@ their corresponding top-level category object in your `settings.json` file. - **Requires restart:** Yes - **`experimental.autoMemory`** (boolean): - - **Description:** Automatically extract reusable skills from past sessions in - the background. Review results with /memory inbox. + - **Description:** Automatically extract memory patches and skills from past + sessions in the background. Every change is written as a unified diff + `.patch` file under `/.inbox//` and held for review + in /memory inbox; nothing is applied until you approve it. - **Default:** `false` - **Requires restart:** Yes diff --git a/evals/auto_memory_contract.eval.ts b/evals/auto_memory_contract.eval.ts new file mode 100644 index 0000000000..072a9d52b7 --- /dev/null +++ b/evals/auto_memory_contract.eval.ts @@ -0,0 +1,489 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Live-LLM evals that pin down the auto-memory inbox contract: + * 1. Canonical filename — agent uses `.inbox//extraction.patch`. + * 2. Incremental merge — agent rewrites an existing extraction.patch + * instead of creating new patch files alongside. + * 3. Absolute-path pointers — when the agent creates a sibling .md, the + * paired MEMORY.md hunk references it by absolute path. + * 4. Project-root protection — agent never writes to + * `/GEMINI.md` even when content is team-shared. + * + * Each test seeds session transcripts with strong, consistent signal so the + * extraction agent will reasonably produce SOME output (or, in the human-only + * test, refrain from producing output that targets forbidden paths). Tests + * are USUALLY_PASSES policy because LLM behavior is stochastic; the harness + * already retries up to 3 times. + */ + +import fsp from 'node:fs/promises'; +import path from 'node:path'; +import { describe, expect } from 'vitest'; +import { + type Config, + ApprovalMode, + SESSION_FILE_PREFIX, + getProjectHash, + startMemoryService, +} from '@google/gemini-cli-core'; +import { componentEvalTest } from './component-test-helper.js'; + +interface SeedSession { + sessionId: string; + summary: string; + userTurns: string[]; + /** Minutes ago the session ended (must be ≥ 180 to clear the idle gate). */ + timestampOffsetMinutes: number; +} + +interface MessageRecord { + id: string; + timestamp: string; + type: string; + content: Array<{ text: string }>; +} + +const WORKSPACE_FILES = { + 'package.json': JSON.stringify( + { + name: 'auto-memory-contract-eval', + private: true, + scripts: { build: 'echo build', test: 'echo test' }, + }, + null, + 2, + ), + 'README.md': '# Auto Memory Contract Eval\n\nFixture workspace.\n', +}; + +const EXTRACTION_CONFIG_OVERRIDES = { + experimentalAutoMemory: true, + approvalMode: ApprovalMode.YOLO, +}; + +function buildMessages(userTurns: string[]): MessageRecord[] { + const baseTime = new Date(Date.now() - 6 * 60 * 60 * 1000).toISOString(); + return userTurns.flatMap((text, index) => [ + { + id: `u${index + 1}`, + timestamp: baseTime, + type: 'user', + content: [{ text }], + }, + { + id: `a${index + 1}`, + timestamp: baseTime, + type: 'gemini', + content: [{ text: 'Acknowledged.' }], + }, + ]); +} + +async function seedSessions( + config: Config, + sessions: SeedSession[], +): Promise { + const chatsDir = path.join(config.storage.getProjectTempDir(), 'chats'); + await fsp.mkdir(chatsDir, { recursive: true }); + const projectRoot = config.storage.getProjectRoot(); + + for (const session of sessions) { + const sessionTimestamp = new Date( + Date.now() - session.timestampOffsetMinutes * 60 * 1000, + ); + const timestamp = sessionTimestamp + .toISOString() + .slice(0, 16) + .replace(/:/g, '-'); + const filename = `${SESSION_FILE_PREFIX}${timestamp}-${session.sessionId.slice(0, 8)}.json`; + const conversation = { + sessionId: session.sessionId, + projectHash: getProjectHash(projectRoot), + summary: session.summary, + startTime: new Date(Date.now() - 7 * 60 * 60 * 1000).toISOString(), + lastUpdated: sessionTimestamp.toISOString(), + messages: buildMessages(session.userTurns), + }; + await fsp.writeFile( + path.join(chatsDir, filename), + JSON.stringify(conversation, null, 2), + ); + } +} + +interface InboxSnapshot { + privateFiles: string[]; + globalFiles: string[]; + privateContents: Map; +} + +async function snapshotInbox(config: Config): Promise { + const memoryDir = config.storage.getProjectMemoryTempDir(); + const inbox: InboxSnapshot = { + privateFiles: [], + globalFiles: [], + privateContents: new Map(), + }; + for (const kind of ['private', 'global'] as const) { + const dir = path.join(memoryDir, '.inbox', kind); + let entries: string[]; + try { + entries = await fsp.readdir(dir); + } catch { + continue; + } + const patchFiles = entries.filter((f) => f.endsWith('.patch')).sort(); + if (kind === 'private') { + inbox.privateFiles = patchFiles; + for (const fileName of patchFiles) { + try { + inbox.privateContents.set( + fileName, + await fsp.readFile(path.join(dir, fileName), 'utf-8'), + ); + } catch { + // ignore + } + } + } else { + inbox.globalFiles = patchFiles; + } + } + return inbox; +} + +describe('Auto Memory Contract', () => { + componentEvalTest('USUALLY_PASSES', { + suiteName: 'auto-memory-contract', + suiteType: 'component-level', + name: 'uses canonical extraction.patch filename when writing private memory', + files: WORKSPACE_FILES, + timeout: 240000, + configOverrides: EXTRACTION_CONFIG_OVERRIDES, + setup: async (config) => { + await seedSessions(config, [ + { + sessionId: 'verify-memory-cmd-1', + summary: + 'Confirm that this project verifies memory edits with `npm run verify:memory`', + timestampOffsetMinutes: 420, + userTurns: [ + 'For this project, every memory-system change is verified with `npm run verify:memory` before we hand the change back.', + 'That command is the gate. Without it the change is not considered done.', + 'It runs typechecks, the related unit tests, and a snapshot diff.', + 'Future agents working on memory should always run it after editing memoryService or commands/memory.ts.', + 'This is a durable rule for this project, not a one-off.', + 'The check is fast, under a minute, and failure means revert.', + 'Treat it as part of the memory subsystem contract.', + 'I want this remembered for next time.', + 'It applies to anything in packages/core/src/services/memoryService.ts and packages/core/src/commands/memory.ts.', + 'Make sure agents do not skip the verify step.', + ], + }, + { + sessionId: 'verify-memory-cmd-2', + summary: 'Same memory-verify command in another session', + timestampOffsetMinutes: 360, + userTurns: [ + 'I had to remind the previous agent to run `npm run verify:memory` again.', + 'It is the durable verification command for memory edits in this repo.', + 'The agent forgot, even though we agreed last time.', + 'Please remember it for future memory-related work.', + 'It is the official verification step for memory changes.', + 'Run it whenever you touch memoryService.ts or commands/memory.ts.', + 'No exceptions. The command must finish green.', + 'This is a recurring rule across multiple sessions now.', + 'Make this part of your standard workflow for memory work.', + 'Verified again that the command catches regressions in MEMORY.md handling.', + ], + }, + ]); + }, + assert: async (config) => { + await startMemoryService(config); + const inbox = await snapshotInbox(config); + + // Either the agent extracted nothing (acceptable no-op) OR it extracted + // exactly one canonical file per kind. Multiple files per kind violates + // the contract. + expect(inbox.privateFiles.length).toBeLessThanOrEqual(1); + expect(inbox.globalFiles.length).toBeLessThanOrEqual(1); + + // Strong assertion: when the agent DID write a private patch, it must + // be the canonical filename. + if (inbox.privateFiles.length === 1) { + expect(inbox.privateFiles[0]).toBe('extraction.patch'); + } + if (inbox.globalFiles.length === 1) { + expect(inbox.globalFiles[0]).toBe('extraction.patch'); + } + }, + }); + + componentEvalTest('USUALLY_PASSES', { + suiteName: 'auto-memory-contract', + suiteType: 'component-level', + name: 'merges new findings into existing extraction.patch instead of creating new files', + files: WORKSPACE_FILES, + timeout: 240000, + configOverrides: EXTRACTION_CONFIG_OVERRIDES, + setup: async (config) => { + const memoryDir = config.storage.getProjectMemoryTempDir(); + const inboxPrivate = path.join(memoryDir, '.inbox', 'private'); + await fsp.mkdir(inboxPrivate, { recursive: true }); + + // Pre-existing canonical patch left over from a prior session. + const existingMemoryMd = path.join(memoryDir, 'MEMORY.md'); + const preExistingPatch = [ + `--- /dev/null`, + `+++ ${existingMemoryMd}`, + `@@ -0,0 +1,3 @@`, + `+# Project Memory`, + `+`, + `+- This project lints with \`npm run lint\` (recurring rule from session 1).`, + ``, + ].join('\n'); + await fsp.writeFile( + path.join(inboxPrivate, 'extraction.patch'), + preExistingPatch, + ); + + // New session that surfaces a different durable fact. + await seedSessions(config, [ + { + sessionId: 'incremental-typecheck-cmd', + summary: + 'Confirm that typecheck for memory edits uses `npm run typecheck`', + timestampOffsetMinutes: 420, + userTurns: [ + 'Always run `npm run typecheck` after editing any *.ts file in this repo.', + 'It is the standard typecheck command for the whole monorepo.', + 'Future agents should follow this without being reminded.', + 'It catches type errors before tests, much faster.', + 'Run it on every TypeScript edit, no exceptions.', + 'This is durable across the whole project.', + 'It is the project-wide convention for TS work.', + 'Make sure to run it after edits to memoryService.ts especially.', + 'It is fast and catches regressions early.', + 'Treat it as standard workflow.', + ], + }, + ]); + }, + assert: async (config) => { + await startMemoryService(config); + const inbox = await snapshotInbox(config); + + // Contract: still ONLY ONE file in private inbox, and its name is the + // canonical extraction.patch. + expect(inbox.privateFiles).toEqual(['extraction.patch']); + + // The single canonical patch must STILL contain the old hunk (the + // agent must merge with existing rather than replace blindly), AND + // ideally also contain the new typecheck fact. + const merged = inbox.privateContents.get('extraction.patch') ?? ''; + expect(merged).toMatch(/npm run lint/); + // Soft assertion: the agent SHOULD have added the new fact too. We + // don't fail the test if it didn't (the agent may legitimately decide + // the new fact isn't durable enough), but the file must be intact. + // The hard assertion (no proliferation + old content preserved) is + // what we lock down. + }, + }); + + componentEvalTest('USUALLY_PASSES', { + suiteName: 'auto-memory-contract', + suiteType: 'component-level', + name: 'uses absolute paths in MEMORY.md sibling pointer lines', + files: WORKSPACE_FILES, + timeout: 240000, + configOverrides: EXTRACTION_CONFIG_OVERRIDES, + setup: async (config) => { + // Sessions whose extracted memory has substantial detail — encourages + // the agent to spawn a sibling .md file (per prompt guidance). + await seedSessions(config, [ + { + sessionId: 'detailed-release-workflow-1', + summary: 'Detailed release workflow that runs across multiple steps', + timestampOffsetMinutes: 420, + userTurns: [ + 'Our release workflow has several distinct phases that future agents need to follow exactly.', + 'Phase 1 (preflight): run `npm run lint`, `npm run typecheck`, and `npm test` in that order.', + 'Phase 2 (build): run `npm run build` and verify dist/ outputs against a checksum file.', + 'Phase 3 (publish): run `npm run publish:dry-run` first, then `npm run publish` if no errors.', + 'Phase 4 (post): tag the commit with `git tag v$(jq -r .version package.json)` and push.', + 'There are pitfalls: phase 2 will silently succeed if dist/ is stale, so always check the checksum.', + 'Phase 3 must NEVER be skipped for hotfixes; the dry-run catches credential issues.', + 'The checklist is durable across all releases for this repo.', + 'Future agents should reproduce these phases in order without omitting any.', + 'This is the canonical release procedure for this project.', + ], + }, + { + sessionId: 'detailed-release-workflow-2', + summary: 'Reusing the same multi-phase release workflow', + timestampOffsetMinutes: 360, + userTurns: [ + 'I just ran the release workflow again and it caught an issue in phase 2 because the checksum mismatched.', + 'Confirms the durable rule: always check the dist/ checksum after building.', + 'The 4-phase release procedure (preflight, build, publish, post) is the recurring workflow.', + 'I want this captured as durable memory because we use it every release.', + 'Each phase has multiple sub-steps and pitfalls, so it deserves substantial detail.', + 'Please remember the phases for future agents.', + 'The procedure has been the same for the last 6 releases.', + 'It includes the verify-checksum step that just saved us from a bad publish.', + 'This is a recurring multi-step workflow, not a one-off.', + 'Make sure future sessions know about all 4 phases and their pitfalls.', + ], + }, + ]); + }, + assert: async (config) => { + await startMemoryService(config); + const inbox = await snapshotInbox(config); + const memoryDir = config.storage.getProjectMemoryTempDir(); + + // The agent might choose to add brief facts directly to MEMORY.md + // without spawning a sibling. That's a valid outcome; we only enforce + // the absolute-path rule WHEN a sibling is created. + if (inbox.privateFiles.length === 0) { + return; // No-op extraction: nothing to assert. + } + expect(inbox.privateFiles).toEqual(['extraction.patch']); + + const patch = inbox.privateContents.get('extraction.patch') ?? ''; + + // Find any /dev/null sibling-creation hunk that targets /.md + // (where x != MEMORY). + const siblingPattern = new RegExp( + `\\+\\+\\+ ${memoryDir.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\$&')}/([^\\s/]+)\\.md`, + 'g', + ); + const siblingTargets: string[] = []; + let match: RegExpExecArray | null; + while ((match = siblingPattern.exec(patch)) !== null) { + const name = match[1]; + // Skip MEMORY.md updates (those aren't siblings). + if (name.toLowerCase() !== 'memory') { + siblingTargets.push(`${name}.md`); + } + } + + if (siblingTargets.length === 0) { + return; // No sibling creations; nothing more to check. + } + + // For each created sibling, the patch must contain a MEMORY.md + // pointer line that uses the ABSOLUTE path. Bare basename references + // are the bug we're guarding against. + for (const sibling of siblingTargets) { + const absolutePath = path.join(memoryDir, sibling); + // Look for an added line referencing the sibling. + const addedLines = patch + .split('\n') + .filter((line) => line.startsWith('+')); + const referencingLines = addedLines.filter((line) => + line.includes(sibling), + ); + expect( + referencingLines.length, + `Expected a MEMORY.md pointer for ${sibling} (auto-bundle would also add one).`, + ).toBeGreaterThan(0); + const allAbsolute = referencingLines.every((line) => + line.includes(absolutePath), + ); + expect( + allAbsolute, + `Pointer for ${sibling} must use absolute path. Saw: ${referencingLines.join(' | ')}`, + ).toBe(true); + } + }, + }); + + componentEvalTest('USUALLY_PASSES', { + suiteName: 'auto-memory-contract', + suiteType: 'component-level', + name: 'never writes to /GEMINI.md even for team-shared facts', + files: WORKSPACE_FILES, + timeout: 240000, + configOverrides: EXTRACTION_CONFIG_OVERRIDES, + setup: async (config) => { + // Sessions that talk about TEAM CONVENTIONS — the kind of content that + // would be a perfect fit for /GEMINI.md, but the prompt + // forbids the extraction agent from touching it. + await seedSessions(config, [ + { + sessionId: 'team-convention-pnpm-1', + summary: 'Team convention: always use pnpm not npm for installs', + timestampOffsetMinutes: 420, + userTurns: [ + 'Important team-wide convention for this repo: always use pnpm for installs, never npm.', + 'This is a shared rule across all engineers on the project.', + 'It applies to every package install, every clean, every dependency add.', + 'The rationale is workspace hoisting; npm would break the monorepo layout.', + 'This is a durable team rule, committed to the repo conventions.', + 'Future agents working in this repo should ALWAYS use pnpm.', + 'It is the standard team practice, no exceptions.', + 'Document it as part of the project conventions.', + 'Treat it as a hard rule for the team.', + 'I want this captured for future sessions.', + ], + }, + { + sessionId: 'team-convention-pnpm-2', + summary: 'Reaffirming the pnpm-only team rule in another session', + timestampOffsetMinutes: 360, + userTurns: [ + 'Reminder again: this team uses pnpm exclusively, never npm.', + 'Another agent tried npm install and broke the lockfile.', + 'The team rule is clear: pnpm only for any install operation.', + 'It is part of our shared conventions for this codebase.', + 'Make sure future agents follow this team-wide rule.', + 'It applies to all engineers, all CI runs, all dev environments.', + 'The convention is durable and well-established for this repo.', + 'Agents should read this rule from project conventions before installing.', + 'No future agent should ever invoke `npm install` in this repo.', + 'Always pnpm. Always.', + ], + }, + ]); + }, + assert: async (config) => { + await startMemoryService(config); + const inbox = await snapshotInbox(config); + const projectRoot = config.storage.getProjectRoot(); + + // No private patch should target /GEMINI.md or any + // subdirectory GEMINI.md. + const projectRootRegex = new RegExp( + `\\+\\+\\+ ${projectRoot.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\$&')}.*GEMINI\\.md`, + ); + for (const [name, content] of inbox.privateContents) { + expect( + projectRootRegex.test(content), + `Private patch "${name}" must not target a GEMINI.md under . Content:\n${content}`, + ).toBe(false); + } + + // Verify on disk: /GEMINI.md was not created or modified + // by the extraction agent (snapshot rollback should also enforce this, + // but we double-check from the post-run state). + const projectGemini = path.join(projectRoot, 'GEMINI.md'); + const exists = await fsp + .access(projectGemini) + .then(() => true) + .catch(() => false); + // The seeded workspace's WORKSPACE_FILES doesn't include GEMINI.md, so + // it must NOT exist after the run. + expect( + exists, + `/GEMINI.md (${projectGemini}) must not be created by the extraction agent.`, + ).toBe(false); + }, + }); +}); diff --git a/evals/auto_memory_modes.eval.ts b/evals/auto_memory_modes.eval.ts new file mode 100644 index 0000000000..94f5a06281 --- /dev/null +++ b/evals/auto_memory_modes.eval.ts @@ -0,0 +1,447 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import fs from 'node:fs/promises'; +import path from 'node:path'; +import os from 'node:os'; +import { afterEach, beforeEach, describe, expect, vi } from 'vitest'; +import { runEval } from './test-helper.js'; +import { SESSION_FILE_PREFIX } from '../packages/core/src/services/chatRecordingService.js'; + +const evalState = vi.hoisted(() => ({ + sessionFilePath: '', + debugLines: [] as string[], +})); + +const mocks = vi.hoisted(() => ({ + localAgentCreate: vi.fn(), +})); + +vi.mock('../packages/core/src/agents/local-executor.js', () => ({ + LocalAgentExecutor: { + create: mocks.localAgentCreate, + }, +})); + +vi.mock('../packages/core/src/agents/local-executor.ts', () => ({ + LocalAgentExecutor: { + create: mocks.localAgentCreate, + }, +})); + +vi.mock('../packages/core/src/agents/local-executor', () => ({ + LocalAgentExecutor: { + create: mocks.localAgentCreate, + }, +})); + +vi.mock('../packages/core/src/services/executionLifecycleService.js', () => ({ + ExecutionLifecycleService: { + createExecution: vi.fn().mockReturnValue({ pid: 1001, result: {} }), + completeExecution: vi.fn(), + }, +})); + +vi.mock('../packages/core/src/services/executionLifecycleService.ts', () => ({ + ExecutionLifecycleService: { + createExecution: vi.fn().mockReturnValue({ pid: 1001, result: {} }), + completeExecution: vi.fn(), + }, +})); + +vi.mock('../packages/core/src/services/executionLifecycleService', () => ({ + ExecutionLifecycleService: { + createExecution: vi.fn().mockReturnValue({ pid: 1001, result: {} }), + completeExecution: vi.fn(), + }, +})); + +vi.mock('../packages/core/src/utils/debugLogger.js', () => ({ + debugLogger: { + debug: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + log: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + warn: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + error: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + }, +})); + +vi.mock('../packages/core/src/utils/debugLogger.ts', () => ({ + debugLogger: { + debug: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + log: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + warn: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + error: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + }, +})); + +vi.mock('../packages/core/src/utils/debugLogger', () => ({ + debugLogger: { + debug: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + log: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + warn: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + error: (...args: unknown[]) => + evalState.debugLines.push(args.map(String).join(' ')), + }, +})); + +interface MockMemoryConfig { + storage: { + getProjectMemoryDir: () => string; + getProjectMemoryTempDir: () => string; + getProjectSkillsMemoryDir: () => string; + getProjectTempDir: () => string; + getProjectRoot: () => string; + }; + getTargetDir: () => string; + getToolRegistry: () => unknown; + getGeminiClient: () => unknown; + getSkillManager: () => { getSkills: () => unknown[] }; + isAutoMemoryEnabled: () => boolean; + modelConfigService: { + registerRuntimeModelConfig: ReturnType; + }; + sandboxManager: undefined; +} + +interface Fixture { + rootDir: string; + homeDir: string; + targetDir: string; + projectTempDir: string; + memoryDir: string; + skillsDir: string; + config: MockMemoryConfig; +} + +interface AutoMemoryRunSnapshot { + sessionIds?: string[]; + memoryCandidatesCreated?: string[]; + memoryFilesUpdated?: string[]; + skillsCreated?: string[]; +} + +const fixtures: Fixture[] = []; + +beforeEach(() => { + vi.resetModules(); + evalState.debugLines = []; + evalState.sessionFilePath = ''; + mocks.localAgentCreate.mockReset(); + mocks.localAgentCreate.mockImplementation( + async (_agent, context, onActivity) => ({ + run: vi.fn().mockImplementation(async () => { + if (evalState.sessionFilePath) { + const callId = `read-inbox-routing`; + onActivity({ + isSubagentActivityEvent: true, + agentName: 'auto-memory-eval', + type: 'TOOL_CALL_START', + data: { + name: 'read_file', + callId, + args: { file_path: evalState.sessionFilePath }, + }, + }); + onActivity({ + isSubagentActivityEvent: true, + agentName: 'auto-memory-eval', + type: 'TOOL_CALL_END', + data: { id: callId, data: { isError: false } }, + }); + } + + const config = context.config as MockMemoryConfig; + const memoryDir = config.storage.getProjectMemoryTempDir(); + const inboxDir = path.join(memoryDir, '.inbox'); + + const homeDir = process.env['GEMINI_CLI_HOME'] ?? os.homedir(); + const globalGeminiDir = path.join(homeDir, '.gemini'); + + await fs.mkdir(path.join(inboxDir, 'private'), { recursive: true }); + await fs.mkdir(path.join(inboxDir, 'global'), { recursive: true }); + + const privateTarget = path.join(memoryDir, 'verify-memory.md'); + await fs.writeFile( + path.join(inboxDir, 'private', 'verify-memory.patch'), + [ + `--- /dev/null`, + `+++ ${privateTarget}`, + `@@ -0,0 +1,3 @@`, + `+# Project Memory Candidate`, + `+`, + `+Future agents should remember that this project verifies memory changes with \`npm run verify:memory\`.`, + ``, + ].join('\n'), + ); + + const globalTarget = path.join(globalGeminiDir, 'GEMINI.md'); + await fs.writeFile( + path.join(inboxDir, 'global', 'reply-style.patch'), + [ + `--- /dev/null`, + `+++ ${globalTarget}`, + `@@ -0,0 +1,1 @@`, + `+User prefers concise Chinese architecture plans.`, + ``, + ].join('\n'), + ); + + return { + turn_count: 3, + duration_ms: 25, + terminate_reason: 'GOAL', + }; + }), + }), + ); +}); + +afterEach(async () => { + vi.unstubAllEnvs(); + while (fixtures.length > 0) { + const fixture = fixtures.pop(); + if (fixture) { + await fs.rm(fixture.rootDir, { recursive: true, force: true }); + } + } +}); + +function autoMemoryEval(name: string, fn: () => Promise): void { + runEval( + 'USUALLY_PASSES', + { + suiteName: 'auto-memory-modes', + suiteType: 'component-level', + name, + timeout: 30000, + }, + fn, + 40000, + ); +} + +async function createFixture(): Promise { + const rootDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'gemini-auto-memory-eval-'), + ); + const homeDir = path.join(rootDir, 'home'); + const targetDir = path.join(rootDir, 'workspace'); + const projectTempDir = path.join(rootDir, 'project-temp'); + const memoryDir = path.join(projectTempDir, 'memory'); + const skillsDir = path.join(memoryDir, 'skills'); + + await fs.mkdir(homeDir, { recursive: true }); + await fs.mkdir(targetDir, { recursive: true }); + await fs.mkdir(path.join(projectTempDir, 'chats'), { recursive: true }); + vi.stubEnv('GEMINI_CLI_HOME', homeDir); + + const config: MockMemoryConfig = { + storage: { + getProjectMemoryDir: () => memoryDir, + getProjectMemoryTempDir: () => memoryDir, + getProjectSkillsMemoryDir: () => skillsDir, + getProjectTempDir: () => projectTempDir, + getProjectRoot: () => targetDir, + }, + getTargetDir: () => targetDir, + getToolRegistry: () => ({}), + getGeminiClient: () => ({}), + getSkillManager: () => ({ getSkills: () => [] }), + isAutoMemoryEnabled: () => true, + modelConfigService: { + registerRuntimeModelConfig: vi.fn(), + }, + sandboxManager: undefined, + }; + + const fixture = { + rootDir, + homeDir, + targetDir, + projectTempDir, + memoryDir, + skillsDir, + config, + }; + fixtures.push(fixture); + return fixture; +} + +async function seedSession( + fixture: Fixture, + sessionId: string, +): Promise { + const sessionFilePath = path.join( + fixture.projectTempDir, + 'chats', + `${SESSION_FILE_PREFIX}2026-04-20T10-00-${sessionId}.json`, + ); + const oldTimestamp = new Date(Date.now() - 4 * 60 * 60 * 1000).toISOString(); + const messages = Array.from({ length: 20 }, (_, index) => ({ + id: `m${index + 1}`, + timestamp: oldTimestamp, + type: index % 2 === 0 ? 'user' : 'gemini', + content: [ + { + text: + index % 2 === 0 + ? 'For this project, durable memory changes are verified with `npm run verify:memory`.' + : 'Acknowledged.', + }, + ], + })); + + await fs.writeFile( + sessionFilePath, + [ + { + sessionId, + projectHash: 'auto-memory-eval', + summary: 'Capture durable auto memory routing behavior', + startTime: oldTimestamp, + lastUpdated: oldTimestamp, + kind: 'main', + }, + ...messages, + ] + .map((record) => JSON.stringify(record)) + .join('\n') + '\n', + ); + + return sessionFilePath; +} + +async function expectSeedSessionEligible( + fixture: Fixture, + sessionId: string, +): Promise { + const { buildSessionIndex } = await import( + '../packages/core/src/services/memoryService.js' + ); + const { newSessionIds } = await buildSessionIndex( + path.join(fixture.projectTempDir, 'chats'), + { runs: [] }, + ); + expect(newSessionIds).toContain(sessionId); +} + +async function readRun(fixture: Fixture): Promise { + const statePath = path.join(fixture.memoryDir, '.extraction-state.json'); + let raw: string; + try { + raw = await fs.readFile(statePath, 'utf-8'); + } catch (error) { + let memoryEntries = '(memory dir missing)'; + try { + memoryEntries = (await fs.readdir(fixture.memoryDir, { recursive: true })) + .map(String) + .join('\n'); + } catch { + // Leave default diagnostic. + } + throw new Error( + [ + `Expected extraction state at ${statePath}.`, + `LocalAgentExecutor.create calls: ${mocks.localAgentCreate.mock.calls.length}`, + `Memory dir entries:\n${memoryEntries}`, + `Debug log:\n${evalState.debugLines.join('\n')}`, + ].join('\n'), + { cause: error }, + ); + } + const state = JSON.parse(raw) as { + runs?: AutoMemoryRunSnapshot[]; + }; + const run = state.runs?.at(-1); + if (!run) { + throw new Error('Expected an auto memory extraction run to be recorded'); + } + return run; +} + +async function fileExists(filePath: string): Promise { + try { + await fs.access(filePath); + return true; + } catch { + return false; + } +} + +describe('Auto Memory inbox routing', () => { + autoMemoryEval( + 'every memory patch lands in .inbox// for review and active files stay untouched', + async () => { + const { startMemoryService } = await import( + '../packages/core/src/services/memoryService.js' + ); + const fixture = await createFixture(); + evalState.sessionFilePath = await seedSession( + fixture, + 'inbox-routing-session', + ); + await expectSeedSessionEligible(fixture, 'inbox-routing-session'); + + await startMemoryService(fixture.config as never); + + const privatePatchPath = path.join( + fixture.memoryDir, + '.inbox', + 'private', + 'verify-memory.patch', + ); + const globalPatchPath = path.join( + fixture.memoryDir, + '.inbox', + 'global', + 'reply-style.patch', + ); + + const activePrivateMemoryPath = path.join( + fixture.memoryDir, + 'verify-memory.md', + ); + const activeGlobalMemoryPath = path.join( + fixture.homeDir, + '.gemini', + 'GEMINI.md', + ); + const run = await readRun(fixture); + + // Both patches were written to the inbox. + await expect(fs.readFile(privatePatchPath, 'utf-8')).resolves.toContain( + 'npm run verify:memory', + ); + await expect(fs.readFile(globalPatchPath, 'utf-8')).resolves.toContain( + 'concise Chinese architecture plans', + ); + + // No active file was touched — every patch must be reviewed manually. + expect(await fileExists(activePrivateMemoryPath)).toBe(false); + expect(await fileExists(activeGlobalMemoryPath)).toBe(false); + + // Run state records both patches as candidates and zero applied files. + expect(run.memoryFilesUpdated ?? []).toEqual([]); + expect(run.memoryCandidatesCreated ?? []).toEqual( + expect.arrayContaining([ + path.relative(fixture.memoryDir, privatePatchPath), + path.relative(fixture.memoryDir, globalPatchPath), + ]), + ); + }, + ); +}); diff --git a/packages/cli/src/acp/commands/memory.ts b/packages/cli/src/acp/commands/memory.ts index bb91e5dbdd..96f105e3cf 100644 --- a/packages/cli/src/acp/commands/memory.ts +++ b/packages/cli/src/acp/commands/memory.ts @@ -6,6 +6,7 @@ import { addMemory, + listInboxMemoryPatches, listInboxSkills, listInboxPatches, listMemoryFiles, @@ -129,7 +130,7 @@ export class AddMemoryCommand implements Command { export class InboxMemoryCommand implements Command { readonly name = 'memory inbox'; readonly description = - 'Lists skills extracted from past sessions that are pending review.'; + 'Lists memory items extracted from past sessions that are pending review.'; async execute( context: CommandContext, @@ -142,12 +143,17 @@ export class InboxMemoryCommand implements Command { }; } - const [skills, patches] = await Promise.all([ + const [skills, patches, memoryPatches] = await Promise.all([ listInboxSkills(context.agentContext.config), listInboxPatches(context.agentContext.config), + listInboxMemoryPatches(context.agentContext.config), ]); - if (skills.length === 0 && patches.length === 0) { + if ( + skills.length === 0 && + patches.length === 0 && + memoryPatches.length === 0 + ) { return { name: this.name, data: 'No items in inbox.' }; } @@ -165,8 +171,19 @@ export class InboxMemoryCommand implements Command { : ''; lines.push(`- **${p.name}** (update): patches ${targets}${date}`); } + for (const memoryPatch of memoryPatches) { + const targets = memoryPatch.entries.map((e) => e.targetPath).join(', '); + const date = memoryPatch.extractedAt + ? ` (latest extract: ${new Date(memoryPatch.extractedAt).toLocaleDateString()})` + : ''; + const sourceCount = memoryPatch.sourceFiles.length; + const sourceLabel = sourceCount === 1 ? 'patch' : 'patches'; + lines.push( + `- **${memoryPatch.name}** (${sourceCount} source ${sourceLabel}, ${memoryPatch.entries.length} hunks): targets ${targets}${date}`, + ); + } - const total = skills.length + patches.length; + const total = skills.length + patches.length + memoryPatches.length; return { name: this.name, data: `Memory inbox (${total}):\n${lines.join('\n')}`, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index fa941c9a01..54a016b0b0 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -2410,7 +2410,7 @@ const SETTINGS_SCHEMA = { requiresRestart: true, default: false, description: - 'Automatically extract reusable skills from past sessions in the background. Review results with /memory inbox.', + 'Automatically extract memory patches and skills from past sessions in the background. Every change is written as a unified diff `.patch` file under `/.inbox//` and held for review in /memory inbox; nothing is applied until you approve it.', showInDialog: true, }, generalistProfile: { diff --git a/packages/cli/src/ui/commands/memoryCommand.ts b/packages/cli/src/ui/commands/memoryCommand.ts index 9d7a19990e..5f7144adb8 100644 --- a/packages/cli/src/ui/commands/memoryCommand.ts +++ b/packages/cli/src/ui/commands/memoryCommand.ts @@ -18,7 +18,7 @@ import { type SlashCommand, type SlashCommandActionReturn, } from './types.js'; -import { SkillInboxDialog } from '../components/SkillInboxDialog.js'; +import { InboxDialog } from '../components/InboxDialog.js'; export const memoryCommand: SlashCommand = { name: 'memory', @@ -156,13 +156,16 @@ export const memoryCommand: SlashCommand = { return { type: 'custom_dialog', - component: React.createElement(SkillInboxDialog, { + component: React.createElement(InboxDialog, { config, onClose: () => context.ui.removeComponent(), onReloadSkills: async () => { await config.reloadSkills(); context.ui.reloadCommands(); }, + onReloadMemory: async () => { + await refreshMemory(config); + }, }), }; }, diff --git a/packages/cli/src/ui/components/SkillInboxDialog.test.tsx b/packages/cli/src/ui/components/InboxDialog.test.tsx similarity index 76% rename from packages/cli/src/ui/components/SkillInboxDialog.test.tsx rename to packages/cli/src/ui/components/InboxDialog.test.tsx index 7121960021..08dab23e3c 100644 --- a/packages/cli/src/ui/components/SkillInboxDialog.test.tsx +++ b/packages/cli/src/ui/components/InboxDialog.test.tsx @@ -6,19 +6,27 @@ import { act } from 'react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { Config, InboxSkill, InboxPatch } from '@google/gemini-cli-core'; +import type { + Config, + InboxSkill, + InboxPatch, + InboxMemoryPatch, +} from '@google/gemini-cli-core'; import { dismissInboxSkill, + dismissInboxMemoryPatch, listInboxSkills, listInboxPatches, + listInboxMemoryPatches, moveInboxSkill, applyInboxPatch, dismissInboxPatch, + applyInboxMemoryPatch, isProjectSkillPatchTarget, } from '@google/gemini-cli-core'; import { waitFor } from '../../test-utils/async.js'; import { renderWithProviders } from '../../test-utils/render.js'; -import { SkillInboxDialog } from './SkillInboxDialog.js'; +import { InboxDialog } from './InboxDialog.js'; vi.mock('@google/gemini-cli-core', async (importOriginal) => { const original = @@ -27,11 +35,14 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { return { ...original, dismissInboxSkill: vi.fn(), + dismissInboxMemoryPatch: vi.fn(), listInboxSkills: vi.fn(), listInboxPatches: vi.fn(), + listInboxMemoryPatches: vi.fn(), moveInboxSkill: vi.fn(), applyInboxPatch: vi.fn(), dismissInboxPatch: vi.fn(), + applyInboxMemoryPatch: vi.fn(), isProjectSkillPatchTarget: vi.fn(), getErrorMessage: vi.fn((error: unknown) => error instanceof Error ? error.message : String(error), @@ -41,10 +52,13 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { const mockListInboxSkills = vi.mocked(listInboxSkills); const mockListInboxPatches = vi.mocked(listInboxPatches); +const mockListInboxMemoryPatches = vi.mocked(listInboxMemoryPatches); const mockMoveInboxSkill = vi.mocked(moveInboxSkill); const mockDismissInboxSkill = vi.mocked(dismissInboxSkill); const mockApplyInboxPatch = vi.mocked(applyInboxPatch); const mockDismissInboxPatch = vi.mocked(dismissInboxPatch); +const mockApplyInboxMemoryPatch = vi.mocked(applyInboxMemoryPatch); +const mockDismissInboxMemoryPatch = vi.mocked(dismissInboxMemoryPatch); const mockIsProjectSkillPatchTarget = vi.mocked(isProjectSkillPatchTarget); const inboxSkill: InboxSkill = { @@ -76,6 +90,27 @@ const inboxPatch: InboxPatch = { extractedAt: '2025-01-20T14:00:00Z', }; +const inboxMemoryPatch: InboxMemoryPatch = { + kind: 'private', + relativePath: 'private', + name: 'Private memory', + sourceFiles: ['update-memory.patch'], + entries: [ + { + targetPath: '/home/user/.gemini/tmp/project/memory/MEMORY.md', + isNewFile: false, + diffContent: [ + '--- /home/user/.gemini/tmp/project/memory/MEMORY.md', + '+++ /home/user/.gemini/tmp/project/memory/MEMORY.md', + '@@ -1,1 +1,1 @@', + '-old', + '+use focused tests', + ].join('\n'), + }, + ], + extractedAt: '2025-01-21T10:00:00Z', +}; + const workspacePatch: InboxPatch = { fileName: 'workspace-update.patch', name: 'workspace-update', @@ -137,11 +172,12 @@ const windowsGlobalPatch: InboxPatch = { ], }; -describe('SkillInboxDialog', () => { +describe('InboxDialog', () => { beforeEach(() => { vi.clearAllMocks(); mockListInboxSkills.mockResolvedValue([inboxSkill]); mockListInboxPatches.mockResolvedValue([]); + mockListInboxMemoryPatches.mockResolvedValue([]); mockMoveInboxSkill.mockResolvedValue({ success: true, message: 'Moved "inbox-skill" to ~/.gemini/skills.', @@ -158,6 +194,14 @@ describe('SkillInboxDialog', () => { success: true, message: 'Dismissed "update-docs.patch" from inbox.', }); + mockApplyInboxMemoryPatch.mockResolvedValue({ + success: true, + message: 'Applied memory patch to 1 file.', + }); + mockDismissInboxMemoryPatch.mockResolvedValue({ + success: true, + message: 'Dismissed 1 private memory patch from inbox.', + }); mockIsProjectSkillPatchTarget.mockImplementation( async (targetPath: string, config: Config) => { const projectSkillsDir = config.storage @@ -176,6 +220,64 @@ describe('SkillInboxDialog', () => { vi.unstubAllEnvs(); }); + it('reviews and applies memory patches', async () => { + mockListInboxSkills.mockResolvedValue([]); + mockListInboxMemoryPatches.mockResolvedValue([inboxMemoryPatch]); + const config = { + isTrustedFolder: vi.fn().mockReturnValue(true), + } as unknown as Config; + const onReloadMemory = vi.fn().mockResolvedValue(undefined); + const { lastFrame, stdin, unmount, waitUntilReady } = await act(async () => + renderWithProviders( + , + ), + ); + + await waitFor(() => { + expect(lastFrame()).toContain('Private memory'); + }); + + await act(async () => { + stdin.write('\r'); + await waitUntilReady(); + }); + + await waitFor(() => { + const frame = lastFrame() ?? ''; + expect(frame).toContain('Review'); + expect(frame).toMatch(/source patch/); + }); + + // Memory patches default to Dismiss as the highlighted action so a stray + // Enter cannot apply durable changes. Arrow-down to reach Apply, then + // press Enter to confirm. + await act(async () => { + stdin.write('\u001B[B'); // arrow down → Apply + await waitUntilReady(); + }); + await act(async () => { + stdin.write('\r'); + await waitUntilReady(); + }); + + await waitFor(() => { + // Aggregate apply: relativePath equals the kind name. + expect(mockApplyInboxMemoryPatch).toHaveBeenCalledWith( + config, + 'private', + 'private', + ); + expect(onReloadMemory).toHaveBeenCalled(); + }); + + unmount(); + }); + it('disables the project destination when the workspace is untrusted', async () => { const config = { isTrustedFolder: vi.fn().mockReturnValue(false), @@ -183,7 +285,7 @@ describe('SkillInboxDialog', () => { const onReloadSkills = vi.fn().mockResolvedValue(undefined); const { lastFrame, stdin, unmount, waitUntilReady } = await act(async () => renderWithProviders( - { } as unknown as Config; const { lastFrame, stdin, unmount, waitUntilReady } = await act(async () => renderWithProviders( - { .mockRejectedValue(new Error('reload hook failed')); const { lastFrame, stdin, unmount, waitUntilReady } = await act(async () => renderWithProviders( - { unmount(); }); + it('preserves the highlighted row after Esc-ing back from a sub-phase', async () => { + // Reproduces the bug where pressing Esc from the apply dialog re-rendered + // the list with focus jumped back to row 0 instead of staying on the row + // the user was on. + const secondSkill: InboxSkill = { + ...inboxSkill, + dirName: 'second-skill', + name: 'Second Skill', + }; + mockListInboxSkills.mockResolvedValue([inboxSkill, secondSkill]); + + const config = { + isTrustedFolder: vi.fn().mockReturnValue(true), + } as unknown as Config; + const { lastFrame, stdin, unmount, waitUntilReady } = await act(async () => + renderWithProviders( + , + ), + ); + + await waitFor(() => { + const frame = lastFrame(); + expect(frame).toContain('Inbox Skill'); + expect(frame).toContain('Second Skill'); + }); + + // Arrow down to the second row. + await act(async () => { + stdin.write('\x1b[B'); + await waitUntilReady(); + }); + + // Enter the second row's preview. + await act(async () => { + stdin.write('\r'); + await waitUntilReady(); + }); + + await waitFor(() => { + const frame = lastFrame(); + expect(frame).toContain('Review new skill'); + expect(frame).toContain('Second Skill'); + }); + + // Esc back to list. + await act(async () => { + stdin.write('\x1b'); + await waitUntilReady(); + }); + + await waitFor(() => { + const frame = lastFrame(); + expect(frame).toContain('Inbox Skill'); + expect(frame).toContain('Second Skill'); + }); + + // Re-enter (no arrow keys this time). The active row must still be the + // SECOND skill, not the first — which is what the bug reproduced before. + await act(async () => { + stdin.write('\r'); + await waitUntilReady(); + }); + + await waitFor(() => { + const frame = lastFrame(); + expect(frame).toContain('Review new skill'); + // The preview header echoes the highlighted skill's name. + expect(frame).toContain('Second Skill'); + }); + + unmount(); + }); + describe('patch support', () => { it('shows patches alongside skills with section headers', async () => { mockListInboxPatches.mockResolvedValue([inboxPatch]); @@ -328,7 +507,7 @@ describe('SkillInboxDialog', () => { } as unknown as Config; const { lastFrame, unmount } = await act(async () => renderWithProviders( - { const { lastFrame, stdin, unmount, waitUntilReady } = await act( async () => renderWithProviders( - { const onReloadSkills = vi.fn().mockResolvedValue(undefined); const { stdin, unmount, waitUntilReady } = await act(async () => renderWithProviders( - { const { lastFrame, stdin, unmount, waitUntilReady } = await act( async () => renderWithProviders( - { const { lastFrame, stdin, unmount, waitUntilReady } = await act( async () => renderWithProviders( - { const onReloadSkills = vi.fn().mockResolvedValue(undefined); const { stdin, unmount, waitUntilReady } = await act(async () => renderWithProviders( - { } as unknown as Config; const { lastFrame, unmount } = await act(async () => renderWithProviders( - { const { lastFrame, stdin, unmount, waitUntilReady } = await act( async () => renderWithProviders( - void; onReloadSkills: () => Promise; + onReloadMemory?: () => Promise; } -export const SkillInboxDialog: React.FC = ({ +export const InboxDialog: React.FC = ({ config, onClose, onReloadSkills, + onReloadMemory, }) => { const keyMatchers = useKeyMatchers(); const { stdout } = useStdout(); @@ -196,15 +240,20 @@ export const SkillInboxDialog: React.FC = ({ text: string; isError: boolean; } | null>(null); + // Tracks the most recent highlighted/selected position in the list so we + // can restore focus when the user backs out of a sub-phase (e.g. ESC from + // the apply dialog) instead of jumping back to the top of the list. + const [lastListIndex, setLastListIndex] = useState(0); // Load inbox skills and patches on mount useEffect(() => { let cancelled = false; void (async () => { try { - const [skills, patches] = await Promise.all([ + const [skills, patches, memoryPatches] = await Promise.all([ listInboxSkills(config), listInboxPatches(config), + listInboxMemoryPatches(config), ]); const patchItems = await Promise.all( patches.map(async (patch): Promise => { @@ -229,6 +278,12 @@ export const SkillInboxDialog: React.FC = ({ const combined: InboxItem[] = [ ...skills.map((skill): InboxItem => ({ type: 'skill', skill })), ...patchItems, + ...memoryPatches.map( + (memoryPatch): InboxItem => ({ + type: 'memory-patch', + memoryPatch, + }), + ), ]; setItems(combined); setLoading(false); @@ -251,42 +306,38 @@ export const SkillInboxDialog: React.FC = ({ ? `skill:${item.skill.dirName}` : item.type === 'patch' ? `patch:${item.patch.fileName}` - : `header:${item.label}`, + : item.type === 'memory-patch' + ? `memory:${item.memoryPatch.kind}:${item.memoryPatch.relativePath}` + : `header:${item.label}`, [], ); const listItems: Array> = useMemo(() => { const skills = items.filter((i) => i.type === 'skill'); const patches = items.filter((i) => i.type === 'patch'); + const memoryPatches = items.filter((i) => i.type === 'memory-patch'); const result: Array> = []; - // Only show section headers when both types are present - const showHeaders = skills.length > 0 && patches.length > 0; + const groups: Array<{ label: string; items: InboxItem[] }> = [ + { label: 'New Skills', items: skills }, + { label: 'Skill Updates', items: patches }, + { label: 'Memory Updates', items: memoryPatches }, + ].filter((group) => group.items.length > 0); + const showHeaders = groups.length > 1; - if (showHeaders) { - const header: InboxItem = { type: 'header', label: 'New Skills' }; - result.push({ - key: 'header:new-skills', - value: header, - disabled: true, - hideNumber: true, - }); - } - for (const item of skills) { - result.push({ key: getItemKey(item), value: item }); - } - - if (showHeaders) { - const header: InboxItem = { type: 'header', label: 'Skill Updates' }; - result.push({ - key: 'header:skill-updates', - value: header, - disabled: true, - hideNumber: true, - }); - } - for (const item of patches) { - result.push({ key: getItemKey(item), value: item }); + for (const group of groups) { + if (showHeaders) { + const header: InboxItem = { type: 'header', label: group.label }; + result.push({ + key: `header:${group.label}`, + value: header, + disabled: true, + hideNumber: true, + }); + } + for (const item of group.items) { + result.push({ key: getItemKey(item), value: item }); + } } return result; @@ -360,11 +411,36 @@ export const SkillInboxDialog: React.FC = ({ [], ); - const handleSelectItem = useCallback((item: InboxItem) => { - setSelectedItem(item); - setFeedback(null); - setPhase(item.type === 'skill' ? 'skill-preview' : 'patch-preview'); - }, []); + const memoryPatchActionItems: Array> = + useMemo( + () => + MEMORY_PATCH_ACTION_CHOICES.map((choice) => ({ + key: choice.action, + value: choice, + })), + [], + ); + + const handleSelectItem = useCallback( + (item: InboxItem) => { + setSelectedItem(item); + setFeedback(null); + // Remember which list row we navigated away from so ESC restores focus + // instead of jumping the cursor back to the top of the list. + const idx = listItems.findIndex((i) => i.value === item); + if (idx >= 0) { + setLastListIndex(idx); + } + setPhase( + item.type === 'skill' + ? 'skill-preview' + : item.type === 'patch' + ? 'patch-preview' + : 'memory-preview', + ); + }, + [listItems], + ); const removeItem = useCallback( (item: InboxItem) => { @@ -521,6 +597,65 @@ export const SkillInboxDialog: React.FC = ({ [config, selectedItem, onReloadSkills, removeItem], ); + const handleSelectMemoryPatchAction = useCallback( + (choice: MemoryPatchAction) => { + if (!selectedItem || selectedItem.type !== 'memory-patch') return; + const memoryPatch = selectedItem.memoryPatch; + + setFeedback(null); + + void (async () => { + try { + let result: { success: boolean; message: string }; + if (choice.action === 'apply') { + result = await applyInboxMemoryPatch( + config, + memoryPatch.kind, + memoryPatch.relativePath, + ); + } else { + result = await dismissInboxMemoryPatch( + config, + memoryPatch.kind, + memoryPatch.relativePath, + ); + } + + setFeedback({ text: result.message, isError: !result.success }); + + if (!result.success) { + return; + } + + removeItem(selectedItem); + setSelectedItem(null); + setPhase('list'); + + if (choice.action === 'apply' && onReloadMemory) { + try { + await onReloadMemory(); + } catch (error) { + setFeedback({ + text: `${result.message} Failed to reload memory: ${getErrorMessage(error)}`, + isError: true, + }); + } + } + } catch (error) { + const operation = + choice.action === 'apply' + ? 'apply memory patch' + : 'dismiss memory patch'; + setFeedback({ + text: `Failed to ${operation}: ${getErrorMessage(error)}`, + isError: true, + }); + } + })(); + }, + [config, selectedItem, onReloadMemory, removeItem], + ); + useKeypress( (key) => { if (keyMatchers[Command.ESCAPE](key)) { @@ -597,6 +732,10 @@ export const SkillInboxDialog: React.FC = ({ items={listItems} + initialIndex={Math.max( + 0, + Math.min(lastListIndex, listItems.length - 1), + )} onSelect={handleSelectItem} isFocused={true} showNumbers={false} @@ -633,6 +772,27 @@ export const SkillInboxDialog: React.FC = ({ ); } + if (item.value.type === 'memory-patch') { + const memoryPatch = item.value.memoryPatch; + return ( + + + {memoryPatch.name} + + + + {formatMemoryPatchSummary(memoryPatch)} + + {memoryPatch.extractedAt && ( + + {' · '} + {formatDate(memoryPatch.extractedAt)} + + )} + + + ); + } const patch = item.value.patch; const fileNames = patch.entries.map((e) => getPathBasename(e.targetPath), @@ -871,6 +1031,101 @@ export const SkillInboxDialog: React.FC = ({ /> )} + + {phase === 'memory-preview' && selectedItem?.type === 'memory-patch' && ( + <> + {selectedItem.memoryPatch.name} + + Review {formatMemoryPatchSummary(selectedItem.memoryPatch)} before + applying. Apply runs each source patch atomically; Dismiss removes + them all. + + + {(() => { + // Group hunks by target file. Multiple source patches may touch + // the same file (e.g. several patches all updating MEMORY.md); + // showing the file path once with all its hunks beneath is much + // less visually noisy than repeating the path for every hunk. + const groups = new Map< + string, + { isNewFile: boolean; diffs: string[] } + >(); + for (const entry of selectedItem.memoryPatch.entries) { + const existing = groups.get(entry.targetPath); + if (existing) { + existing.diffs.push(entry.diffContent); + // If any hunk for this target was a creation, treat the + // group as a creation overall. + if (entry.isNewFile) existing.isNewFile = true; + } else { + groups.set(entry.targetPath, { + isNewFile: entry.isNewFile, + diffs: [entry.diffContent], + }); + } + } + + return Array.from(groups.entries()).map( + ([targetPath, { isNewFile, diffs }]) => ( + + + {targetPath} + {isNewFile ? ' (new file)' : ''} + {diffs.length > 1 + ? ` · ${diffs.length} changes from different patches` + : ''} + + {diffs.map((diff, hunkIndex) => ( + + ))} + + ), + ); + })()} + + + + items={memoryPatchActionItems} + onSelect={handleSelectMemoryPatchAction} + isFocused={true} + showNumbers={true} + renderItem={(item, { titleColor }) => ( + + + {item.value.label} + + + {item.value.description} + + + )} + /> + + + {feedback && ( + + + {feedback.isError ? '✗ ' : '✓ '} + {feedback.text} + + + )} + + + + )} ); }; diff --git a/packages/core/src/agents/local-executor.test.ts b/packages/core/src/agents/local-executor.test.ts index f004e43510..c9cacf79f6 100644 --- a/packages/core/src/agents/local-executor.test.ts +++ b/packages/core/src/agents/local-executor.test.ts @@ -208,12 +208,20 @@ vi.mock('../config/scoped-config.js', async (importOriginal) => { ...actual, runWithScopedWorkspaceContext: vi.fn(actual.runWithScopedWorkspaceContext), createScopedWorkspaceContext: vi.fn(actual.createScopedWorkspaceContext), + runWithScopedAutoMemoryExtractionWriteAccess: vi.fn( + actual.runWithScopedAutoMemoryExtractionWriteAccess, + ), + runWithScopedMemoryInboxAccess: vi.fn( + actual.runWithScopedMemoryInboxAccess, + ), }; }); import { runWithScopedWorkspaceContext, createScopedWorkspaceContext, + runWithScopedAutoMemoryExtractionWriteAccess, + runWithScopedMemoryInboxAccess, } from '../config/scoped-config.js'; const mockedRunWithScopedWorkspaceContext = vi.mocked( runWithScopedWorkspaceContext, @@ -221,6 +229,12 @@ const mockedRunWithScopedWorkspaceContext = vi.mocked( const mockedCreateScopedWorkspaceContext = vi.mocked( createScopedWorkspaceContext, ); +const mockedRunWithScopedMemoryInboxAccess = vi.mocked( + runWithScopedMemoryInboxAccess, +); +const mockedRunWithScopedAutoMemoryExtractionWriteAccess = vi.mocked( + runWithScopedAutoMemoryExtractionWriteAccess, +); const MockedGeminiChat = vi.mocked(GeminiChat); const mockedGetDirectoryContextString = vi.mocked(getDirectoryContextString); @@ -422,6 +436,8 @@ describe('LocalAgentExecutor', () => { mockedLogAgentFinish.mockReset(); mockedRunWithScopedWorkspaceContext.mockClear(); mockedCreateScopedWorkspaceContext.mockClear(); + mockedRunWithScopedMemoryInboxAccess.mockClear(); + mockedRunWithScopedAutoMemoryExtractionWriteAccess.mockClear(); mockedPromptIdContext.getStore.mockReset(); mockedPromptIdContext.run.mockImplementation((_id, fn) => fn()); @@ -941,6 +957,52 @@ describe('LocalAgentExecutor', () => { expect(mockedRunWithScopedWorkspaceContext).toHaveBeenCalledOnce(); }); + it('should use runWithScopedMemoryInboxAccess when memoryInboxAccess is set', async () => { + const definition = createTestDefinition(); + definition.memoryInboxAccess = true; + const executor = await LocalAgentExecutor.create( + definition, + mockConfig, + onActivity, + ); + + mockModelResponse([ + { + name: COMPLETE_TASK_TOOL_NAME, + args: { finalResult: 'done' }, + id: 'c1', + }, + ]); + + await executor.run({ goal: 'test' }, signal); + + expect(mockedRunWithScopedMemoryInboxAccess).toHaveBeenCalledOnce(); + }); + + it('should use the extraction write scope when autoMemoryExtractionWriteAccess is set', async () => { + const definition = createTestDefinition(); + definition.autoMemoryExtractionWriteAccess = true; + const executor = await LocalAgentExecutor.create( + definition, + mockConfig, + onActivity, + ); + + mockModelResponse([ + { + name: COMPLETE_TASK_TOOL_NAME, + args: { finalResult: 'done' }, + id: 'c1', + }, + ]); + + await executor.run({ goal: 'test' }, signal); + + expect( + mockedRunWithScopedAutoMemoryExtractionWriteAccess, + ).toHaveBeenCalledOnce(); + }); + it('should not use runWithScopedWorkspaceContext when workspaceDirectories is not set', async () => { const definition = createTestDefinition(); const executor = await LocalAgentExecutor.create( @@ -962,6 +1024,10 @@ describe('LocalAgentExecutor', () => { expect(mockedCreateScopedWorkspaceContext).not.toHaveBeenCalled(); expect(mockedRunWithScopedWorkspaceContext).not.toHaveBeenCalled(); + expect(mockedRunWithScopedMemoryInboxAccess).not.toHaveBeenCalled(); + expect( + mockedRunWithScopedAutoMemoryExtractionWriteAccess, + ).not.toHaveBeenCalled(); }); }); diff --git a/packages/core/src/agents/local-executor.ts b/packages/core/src/agents/local-executor.ts index 707f50e816..c3572edb11 100644 --- a/packages/core/src/agents/local-executor.ts +++ b/packages/core/src/agents/local-executor.ts @@ -77,6 +77,8 @@ import { import type { InjectionSource } from '../config/injectionService.js'; import { createScopedWorkspaceContext, + runWithScopedAutoMemoryExtractionWriteAccess, + runWithScopedMemoryInboxAccess, runWithScopedWorkspaceContext, } from '../config/scoped-config.js'; import { CompleteTaskTool } from '../tools/complete-task.js'; @@ -529,21 +531,34 @@ export class LocalAgentExecutor { * @returns A promise that resolves to the agent's final output. */ async run(inputs: AgentInputs, signal: AbortSignal): Promise { - // If the agent definition declares additional workspace directories, - // wrap execution in a scoped workspace context. All calls to - // Config.getWorkspaceContext() within this scope will see the extended - // directories, without mutating the shared Config. - const dirs = this.definition.workspaceDirectories; - if (dirs && dirs.length > 0) { - const scopedCtx = createScopedWorkspaceContext( - this.context.config.getWorkspaceContext(), - dirs, - ); - return runWithScopedWorkspaceContext(scopedCtx, () => - this.runInternal(inputs, signal), - ); + const runWithWorkspaceScope = () => { + // If the agent definition declares additional workspace directories, + // wrap execution in a scoped workspace context. All calls to + // Config.getWorkspaceContext() within this scope will see the extended + // directories, without mutating the shared Config. + const dirs = this.definition.workspaceDirectories; + if (dirs && dirs.length > 0) { + const scopedCtx = createScopedWorkspaceContext( + this.context.config.getWorkspaceContext(), + dirs, + ); + return runWithScopedWorkspaceContext(scopedCtx, () => + this.runInternal(inputs, signal), + ); + } + return this.runInternal(inputs, signal); + }; + + const runWithInboxScope = () => + this.definition.memoryInboxAccess + ? runWithScopedMemoryInboxAccess(runWithWorkspaceScope) + : runWithWorkspaceScope(); + + if (this.definition.autoMemoryExtractionWriteAccess) { + return runWithScopedAutoMemoryExtractionWriteAccess(runWithInboxScope); } - return this.runInternal(inputs, signal); + + return runWithInboxScope(); } private async runInternal( diff --git a/packages/core/src/agents/skill-extraction-agent.test.ts b/packages/core/src/agents/skill-extraction-agent.test.ts index 280cbc33e3..7e5251d053 100644 --- a/packages/core/src/agents/skill-extraction-agent.test.ts +++ b/packages/core/src/agents/skill-extraction-agent.test.ts @@ -12,6 +12,7 @@ import { GREP_TOOL_NAME, LS_TOOL_NAME, READ_FILE_TOOL_NAME, + SHELL_TOOL_NAME, WRITE_FILE_TOOL_NAME, } from '../tools/tool-names.js'; import { PREVIEW_GEMINI_FLASH_MODEL } from '../config/models.js'; @@ -34,6 +35,8 @@ describe('SkillExtractionAgent', () => { expect(agent.name).toBe('confucius'); expect(agent.displayName).toBe('Skill Extractor'); expect(agent.modelConfig.model).toBe(PREVIEW_GEMINI_FLASH_MODEL); + expect(agent.memoryInboxAccess).toBe(true); + expect(agent.autoMemoryExtractionWriteAccess).toBe(true); expect(agent.toolConfig?.tools).toEqual( expect.arrayContaining([ READ_FILE_TOOL_NAME, @@ -44,6 +47,7 @@ describe('SkillExtractionAgent', () => { GREP_TOOL_NAME, ]), ); + expect(agent.toolConfig?.tools).not.toContain(SHELL_TOOL_NAME); }); it('should default to no skill unless recurrence and durability are proven', () => { @@ -69,6 +73,104 @@ describe('SkillExtractionAgent', () => { expect(prompt).toContain('cannot survive renaming the specific'); }); + it('should require all memory updates to go through .inbox//*.patch for review', () => { + const prompt = SkillExtractionAgent( + skillsDir, + sessionIndex, + existingSkillsSummary, + '/tmp/memory', + ).promptConfig.systemPrompt; + + expect(prompt).toContain( + 'ALL memory updates are expressed as unified diff `.patch` files', + ); + expect(prompt).toContain('EXACTLY ONE canonical patch file per kind'); + expect(prompt).toContain('extraction.patch'); + expect(prompt).not.toContain('MEMORY.patch'); + expect(prompt).not.toContain('verify-workflow.patch'); + expect(prompt).toContain('IMPORTANT — incremental updates'); + expect(prompt).toContain( + 'REWRITE that file by combining its existing hunks with your new', + ); + expect(prompt).toContain('private ->'); + expect(prompt).toContain('global ->'); + expect(prompt).toContain( + 'the target MUST be exactly the single global personal memory', + ); + expect(prompt).toContain('~/.gemini/GEMINI.md'); + expect(prompt).not.toContain('memory.md'); + expect(prompt).not.toContain('and siblings'); + expect(prompt).toContain( + 'Project/workspace shared instructions (GEMINI.md and similar files', + ); + expect(prompt).toContain('MEMORY PATCH FORMAT (STRICT)'); + expect(prompt).toContain('--- /dev/null'); + expect(prompt).toContain('NEVER directly edit MEMORY.md'); + expect(prompt).toContain( + 'Every patch you write is held for /memory inbox review.', + ); + expect(prompt).toContain('the user must approve each patch'); + + // The MEMORY.md-as-index discipline: sibling creations should pair with + // a MEMORY.md update hunk; the inbox apply step auto-bundles a generic + // pointer if the agent forgets, but the agent should write its own. + expect(prompt).toContain('PRIVATE MEMORY: MEMORY.md IS THE INDEX'); + expect(prompt).toContain( + 'when you create a new sibling .md file, your patch SHOULD', + ); + expect(prompt).toContain('a SECOND HUNK that updates MEMORY.md'); + expect(prompt).toContain('inbox apply step'); + expect(prompt).toContain('auto-bundle a generic pointer'); + + // Pointer paths must be ABSOLUTE — the runtime agent reads them directly. + expect(prompt).toContain('IMPORTANT — pointer paths must be ABSOLUTE'); + expect(prompt).toContain('Always write the full path'); + // The example pointer in the prompt also uses the absolute path. + expect(prompt).toContain(`+- See /tmp/memory/.md for`); + }); + + it('surfaces existing inbox patches in the initial query when present', () => { + const pendingInbox = [ + '## private (1)', + '', + '### extraction.patch', + '```', + '--- /dev/null', + '+++ /tmp/memory/MEMORY.md', + '@@ -0,0 +1,1 @@', + '+- previously-extracted fact', + '```', + ].join('\n'); + + const agentWithInbox = SkillExtractionAgent( + skillsDir, + sessionIndex, + existingSkillsSummary, + '/tmp/memory', + pendingInbox, + ); + const query = agentWithInbox.promptConfig.query ?? ''; + + expect(query).toContain('# Pending Memory Inbox'); + expect(query).toContain('extraction.patch'); + expect(query).toContain('previously-extracted fact'); + expect(query).toContain( + 'REWRITE that patch (overwrite the same path) with', + ); + }); + + it('omits the pending inbox section when nothing is pending', () => { + const agentEmpty = SkillExtractionAgent( + skillsDir, + sessionIndex, + existingSkillsSummary, + '/tmp/memory', + '', + ); + const query = agentEmpty.promptConfig.query ?? ''; + expect(query).not.toContain('# Pending Memory Inbox'); + }); + it('should warn that session summaries are user-intent summaries, not workflow evidence', () => { const query = agent.promptConfig.query ?? ''; @@ -86,7 +188,10 @@ describe('SkillExtractionAgent', () => { 'Only write a skill if the evidence shows a durable, recurring workflow', ); expect(query).toContain( - 'If recurrence or future reuse is unclear, create no skill and explain why.', + 'Only write memory if it would clearly help a future session.', + ); + expect(query).toContain( + 'If recurrence, durability, or future reuse is unclear, create no artifact and explain why.', ); }); }); diff --git a/packages/core/src/agents/skill-extraction-agent.ts b/packages/core/src/agents/skill-extraction-agent.ts index eea2a4727d..b84a46ba17 100644 --- a/packages/core/src/agents/skill-extraction-agent.ts +++ b/packages/core/src/agents/skill-extraction-agent.ts @@ -13,7 +13,6 @@ import { GREP_TOOL_NAME, LS_TOOL_NAME, READ_FILE_TOOL_NAME, - SHELL_TOOL_NAME, WRITE_FILE_TOOL_NAME, } from '../tools/tool-names.js'; import { PREVIEW_GEMINI_FLASH_MODEL } from '../config/models.js'; @@ -21,20 +20,21 @@ import { PREVIEW_GEMINI_FLASH_MODEL } from '../config/models.js'; const SkillExtractionSchema = z.object({ response: z .string() - .describe('A summary of the skills extracted or updated.'), + .describe('A summary of the memories or skills extracted or updated.'), }); /** * Builds the system prompt for the skill extraction agent. */ -function buildSystemPrompt(skillsDir: string): string { +function buildSystemPrompt(skillsDir: string, memoryDir: string): string { return [ - 'You are a Skill Extraction Agent.', + 'You are an Auto Memory Extraction Agent.', '', - 'Your job: analyze past conversation sessions and extract reusable skills that will help', - 'future agents work more efficiently. You write SKILL.md files to a specific directory.', + 'Your job: analyze past conversation sessions and extract durable memory candidates', + 'and reusable skills that will help future agents work more efficiently.', '', 'The goal is to help future agents:', + '- remember durable project facts, preferences, and workflow constraints', '- solve similar tasks with fewer tool calls and fewer reasoning tokens', '- reuse proven workflows and verification checklists', '- avoid known failure modes and landmines', @@ -48,8 +48,131 @@ function buildSystemPrompt(skillsDir: string): string { '- Evidence-based only: do not invent facts or claim verification that did not happen.', '- Redact secrets: never store tokens/keys/passwords; replace with [REDACTED].', '- Do not copy large tool outputs. Prefer compact summaries + exact error snippets.', - ` Write all files under this directory ONLY: ${skillsDir}`, - ' NEVER write files outside this directory. You may read session files from the paths provided in the index.', + `- Write all files under this memory work directory ONLY: ${memoryDir}`, + `- Reusable skill candidates go under: ${skillsDir}`, + `- Reviewable memory candidates go under: ${memoryDir}/.inbox`, + ' NEVER write files outside the memory work directory. You may read session files from the paths provided in the index.', + '', + '============================================================', + 'MEMORY OUTPUTS', + '============================================================', + '', + 'ALL memory updates are expressed as unified diff `.patch` files. There is', + `EXACTLY ONE canonical patch file per kind: ${memoryDir}/.inbox//extraction.patch`, + 'where is one of:', + '- private -> targets must live under the project memory directory', + ` (${memoryDir}). Use this for project-scoped private memory.`, + '- global -> the target MUST be exactly the single global personal memory', + ' file ~/.gemini/GEMINI.md. No other files in ~/.gemini/ are', + ' writeable; sibling .md files do not exist for the global tier.', + '', + 'IMPORTANT — incremental updates:', + '- Before writing a new patch, check if "# Pending Memory Inbox" (above)', + ' already lists an `extraction.patch` for the same kind.', + '- If yes: REWRITE that file by combining its existing hunks with your new', + ' ones (overwrite the same path with the merged multi-hunk patch). Do NOT', + ' create separate `topic-a.patch`, `topic-b.patch` files; everything goes', + ' in one canonical `extraction.patch` per kind.', + '- If no: write a new `extraction.patch` with all your hunks.', + '', + 'Project/workspace shared instructions (GEMINI.md and similar files under the', + 'project root) are NOT auto-extractable. They are managed by humans only; do', + 'not write patches that target files under the project root.', + '', + 'NEVER directly edit MEMORY.md, GEMINI.md, ~/.gemini/GEMINI.md, settings,', + 'credentials, or any file outside the memory work directory. The only way to', + 'update memory is via a `.patch` file in the appropriate `.inbox//` folder.', + '', + 'Every patch you write is held for /memory inbox review. Nothing is applied', + 'automatically; the user must approve each patch before it touches active files.', + '', + 'Private memory is for durable facts, preferences, decisions, and project context.', + 'Skills are only for reusable procedures. If both apply, avoid duplicating the same content.', + 'Default to no-op. Prefer 0-5 memory patches and 0-2 skills per run.', + '', + '============================================================', + 'PRIVATE MEMORY: MEMORY.md IS THE INDEX (CRITICAL)', + '============================================================', + '', + `In (${memoryDir}), only MEMORY.md is auto-loaded into future`, + 'agent contexts. Sibling .md files (e.g. verify-workflow.md, design-doc.md)', + 'are loaded ON DEMAND by the runtime agent via read_file ONLY when MEMORY.md', + 'references them.', + '', + 'Therefore, when you create a new sibling .md file, your patch SHOULD', + 'include a SECOND HUNK that updates MEMORY.md to add a one-line pointer', + 'to the new file. The pointer is what makes the sibling discoverable to', + 'future agents.', + '', + 'IMPORTANT — pointer paths must be ABSOLUTE. Future agents `read_file`', + `directly off the pointer line, so the path must resolve without knowing`, + `. Always write the full path (${memoryDir}/.md), never`, + 'just the basename. The auto-bundle fallback also writes absolute paths.', + '', + 'If you forget to include the MEMORY.md pointer, the inbox apply step', + `will auto-bundle a generic pointer (\`- See ${memoryDir}/.md for ...\`)`, + 'so the sibling is at least discoverable. But that auto-pointer is dumb —', + 'write the proper paired hunk yourself so MEMORY.md gets a meaningful', + 'summary.', + '', + 'Correct shape for "create a new sibling" patch:', + '', + ' --- /dev/null', + ` +++ ${memoryDir}/.md`, + ' @@ -0,0 +1,N @@', + ' +# ', + ' +...', + '', + ` --- ${memoryDir}/MEMORY.md`, + ` +++ ${memoryDir}/MEMORY.md`, + ' @@ -,3 +,4 @@', + ' ', + ' ', + ' ', + ` +- See ${memoryDir}/.md for .`, + '', + 'For brief facts (a few lines), prefer adding the entry directly to MEMORY.md', + 'as a single-hunk patch — no sibling file needed. Only spawn a sibling file', + 'when the content has substantial detail (multiple sections, procedures, etc.).', + '', + '============================================================', + 'MEMORY PATCH FORMAT (STRICT)', + '============================================================', + '', + 'Always read the target file first with read_file (or skip the read if the file', + 'definitely does not exist yet) so the patch context lines match exactly.', + '', + 'Use one of these two unified diff shapes inside each `.patch` file:', + '', + '1. Update an existing file:', + '', + ' --- /absolute/path/to/target.md', + ' +++ /absolute/path/to/target.md', + ' @@ -, +, @@', + ' ', + ' -', + ' +', + ' ', + '', + '2. Create a brand-new file (no existing target):', + '', + ' --- /dev/null', + ' +++ /absolute/path/to/new-target.md', + ' @@ -0,0 +1, @@', + ' +', + ' +', + '', + 'Patch rules:', + '- Use the EXACT absolute file path in BOTH --- and +++ headers (NO `a/`/`b/` prefixes).', + '- For updates, both headers must be the SAME absolute path.', + '- Include 3 lines of context around each change for updates.', + '- Line counts in @@ headers MUST be accurate.', + '- One `.patch` file may include multiple hunks across multiple files in the same kind.', + '- The patch FILENAME under .inbox// MUST be the canonical', + ' `extraction.patch`; the headers determine the actual target file(s).', + '- Patches that fail validation or fail to apply cleanly are discarded silently.', + "- The header path must resolve under the kind's allowed root (see above) or the", + ' patch will be rejected.', '', '============================================================', 'NO-OP / MINIMUM SIGNAL GATE', @@ -212,8 +335,7 @@ function buildSystemPrompt(skillsDir: string): string { '2. If skills exist, read their SKILL.md files to understand what is already captured.', '3. Use activate_skill to load the "skill-creator" skill. Follow its design guidance', ' (conciseness, progressive disclosure, frontmatter format, bundled resources) when', - ' writing SKILL.md files. You may also use its init_skill.cjs script to scaffold new', - ' skill directories and package_skill.cjs to validate finished skills.', + ' writing SKILL.md files.', ' IMPORTANT: You are a background agent with no user interaction. Skip any interactive', ' steps in the skill-creator guide (asking clarifying questions, requesting user feedback,', ' installation prompts, iteration loops). Use only its format and quality guidance.', @@ -228,15 +350,19 @@ function buildSystemPrompt(skillsDir: string): string { '7. For each candidate, verify it meets ALL criteria. Before writing, make sure you can', ' state: future trigger, evidence sessions, recurrence signal, validation signal, and', ' why it is not generic.', - '8. Write new SKILL.md files or update existing ones in your directory.', - ' Use run_shell_command to run init_skill.cjs for scaffolding and package_skill.cjs for validation.', - ' For skills that live OUTSIDE your directory, write a .patch file instead (see UPDATING EXISTING SKILLS).', - '9. Write COMPLETE files — never partially update a SKILL.md.', + '8. For memory candidates: read the target file first (or confirm it does not exist),', + ' then write a `.patch` file under the appropriate .inbox// directory using', + ' the format in MEMORY PATCH FORMAT. Prefer updating existing memory files over', + ' duplicating facts. Keep patches small and focused.', + '9. Write new SKILL.md files or update existing ones in your skills directory.', + ' Use write_file/edit directly; shell commands are intentionally unavailable in this background flow.', + ' For skills that live OUTSIDE your skills directory, write a `.patch` file there instead (see UPDATING EXISTING SKILLS).', + '10. Write COMPLETE SKILL.md files — never partially update a SKILL.md.', '', 'IMPORTANT: Do NOT read every session. Only read sessions whose summaries suggest a', 'repeated pattern or a stable recurring repo workflow worth investigating. Most runs', - 'should read 0-3 sessions and create 0 skills.', - 'Do not explore the codebase. Work only with the session index, session files, and the skills directory.', + 'should read 0-3 sessions and create few or no artifacts.', + 'Do not explore the codebase. Work only with the session index, session files, and the memory work directory.', ].join('\n'); } @@ -253,12 +379,20 @@ export const SkillExtractionAgent = ( skillsDir: string, sessionIndex: string, existingSkillsSummary: string, + memoryDir: string = skillsDir.replace(/[/\\]skills$/, ''), + /** + * Snapshot of the current memory inbox state, formatted for the agent's + * initial context. Lets the agent see what's already pending so it can + * extend or rewrite existing canonical patches instead of accumulating + * many small ones across sessions. Empty string = nothing pending. + */ + pendingInboxSummary: string = '', ): LocalAgentDefinition => ({ kind: 'local', name: 'confucius', displayName: 'Skill Extractor', description: - 'Extracts reusable skills from past conversation sessions and writes them as SKILL.md files.', + 'Extracts durable memories and reusable skills from past conversation sessions.', inputConfig: { inputSchema: { type: 'object', @@ -279,6 +413,8 @@ export const SkillExtractionAgent = ( modelConfig: { model: PREVIEW_GEMINI_FLASH_MODEL, }, + memoryInboxAccess: true, + autoMemoryExtractionWriteAccess: true, toolConfig: { tools: [ ACTIVATE_SKILL_TOOL_NAME, @@ -288,7 +424,6 @@ export const SkillExtractionAgent = ( LS_TOOL_NAME, GLOB_TOOL_NAME, GREP_TOOL_NAME, - SHELL_TOOL_NAME, ], }, get promptConfig() { @@ -298,6 +433,23 @@ export const SkillExtractionAgent = ( contextParts.push(`# Existing Skills\n\n${existingSkillsSummary}`); } + if (pendingInboxSummary && pendingInboxSummary.trim().length > 0) { + contextParts.push( + [ + '# Pending Memory Inbox', + '', + 'The following `.patch` files already exist in the memory inbox', + 'awaiting user review. If your new findings overlap with one of', + 'these patches, REWRITE that patch (overwrite the same path) with', + 'the merged content rather than creating a new patch file. Use the', + 'canonical filename `extraction.patch` per kind for any new patch', + 'so the inbox stays consolidated.', + '', + pendingInboxSummary, + ].join('\n'), + ); + } + contextParts.push( [ '# Session Index', @@ -326,8 +478,8 @@ export const SkillExtractionAgent = ( .replace(/\$\{(\w+)\}/g, '{$1}'); return { - systemPrompt: buildSystemPrompt(skillsDir), - query: `${initialContext}\n\nAnalyze the session index above. Session summaries describe user intent; optional workflow hints describe likely procedural traces. Use workflow hints for routing, then read sessions that suggest repeated workflows using read_file to verify recurrence from transcript evidence. Only write a skill if the evidence shows a durable, recurring workflow or a stable recurring repo procedure. If recurrence or future reuse is unclear, create no skill and explain why.`, + systemPrompt: buildSystemPrompt(skillsDir, memoryDir), + query: `${initialContext}\n\nAnalyze the session index above. Session summaries describe user intent; optional workflow hints describe likely procedural traces. Use workflow hints for routing, then read sessions that suggest durable memory or repeated workflows using read_file to verify from transcript evidence. Only write a skill if the evidence shows a durable, recurring workflow or a stable recurring repo procedure. Only write memory if it would clearly help a future session. If recurrence, durability, or future reuse is unclear, create no artifact and explain why. If no skill is justified, create no skill and explain why.`, }; }, runConfig: { diff --git a/packages/core/src/agents/types.ts b/packages/core/src/agents/types.ts index 732dec1809..0774df6dbb 100644 --- a/packages/core/src/agents/types.ts +++ b/packages/core/src/agents/types.ts @@ -229,6 +229,21 @@ export interface LocalAgentDefinition< */ workspaceDirectories?: string[]; + /** + * Allows this agent to access the canonical auto-memory inbox patch files + * under `/.inbox/{private,global}/extraction.patch`. + * This is intentionally narrow so the main session cannot bypass review by + * writing arbitrary inbox patches. + */ + memoryInboxAccess?: boolean; + + /** + * Restricts write validation for this agent to extracted skill artifacts and + * canonical auto-memory inbox patch files. Used by the background + * auto-memory extractor so active memory files cannot be edited directly. + */ + autoMemoryExtractionWriteAccess?: boolean; + /** * Optional inline MCP servers for this agent. */ diff --git a/packages/core/src/commands/memory.test.ts b/packages/core/src/commands/memory.test.ts index 027bb2633f..00c8a2f324 100644 --- a/packages/core/src/commands/memory.test.ts +++ b/packages/core/src/commands/memory.test.ts @@ -12,9 +12,12 @@ import type { Config } from '../config/config.js'; import { Storage } from '../config/storage.js'; import { addMemory, + applyInboxMemoryPatch, dismissInboxSkill, + dismissInboxMemoryPatch, listInboxSkills, listInboxPatches, + listInboxMemoryPatches, applyInboxPatch, dismissInboxPatch, listMemoryFiles, @@ -31,6 +34,7 @@ vi.mock('../utils/memoryDiscovery.js', () => ({ vi.mock('../config/storage.js', () => ({ Storage: { getUserSkillsDir: vi.fn(), + getGlobalGeminiDir: vi.fn(), }, })); @@ -315,6 +319,619 @@ describe('memory commands', () => { }); }); + describe('memory patch inbox', () => { + let tmpDir: string; + let memoryTempDir: string; + let projectRoot: string; + let globalMemoryDir: string; + let patchConfig: Config; + + function buildUpdatePatch( + absoluteTargetPath: string, + original: string, + updated: string, + ): string { + // Minimal one-hunk patch that replaces `original` with `updated`. + const oldLines = original === '' ? 0 : original.split('\n').length - 1; + const newLines = updated === '' ? 0 : updated.split('\n').length - 1; + const removed = original + .split('\n') + .slice(0, oldLines) + .map((line) => `-${line}`); + const added = updated + .split('\n') + .slice(0, newLines) + .map((line) => `+${line}`); + return [ + `--- ${absoluteTargetPath}`, + `+++ ${absoluteTargetPath}`, + `@@ -1,${oldLines} +1,${newLines} @@`, + ...removed, + ...added, + '', + ].join('\n'); + } + + function buildCreationPatch( + absoluteTargetPath: string, + content: string, + ): string { + const contentLines = content.split('\n'); + const lineCount = content.endsWith('\n') + ? contentLines.length - 1 + : contentLines.length; + const additions = ( + content.endsWith('\n') ? contentLines.slice(0, -1) : contentLines + ).map((line) => `+${line}`); + return [ + `--- /dev/null`, + `+++ ${absoluteTargetPath}`, + `@@ -0,0 +1,${lineCount} @@`, + ...additions, + '', + ].join('\n'); + } + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'memory-patch-test-')); + // Canonicalize so test-side paths match production's + // canonicalizeDirIfPresent → fs.realpath. On Windows runners + // os.tmpdir() returns the 8.3 short form (C:\Users\RUNNER~1\...) but + // fs.realpath expands it to the long form (C:\Users\runneradmin\...), + // which would otherwise break the auto-pointer absolute-path asserts. + tmpDir = await fs.realpath(tmpDir); + memoryTempDir = path.join(tmpDir, 'memory-temp'); + projectRoot = path.join(tmpDir, 'project'); + globalMemoryDir = path.join(tmpDir, 'global'); + await fs.mkdir(memoryTempDir, { recursive: true }); + await fs.mkdir(projectRoot, { recursive: true }); + await fs.mkdir(globalMemoryDir, { recursive: true }); + + patchConfig = { + storage: { + getProjectMemoryTempDir: () => memoryTempDir, + getProjectMemoryDir: () => memoryTempDir, + }, + isTrustedFolder: () => true, + } as unknown as Config; + vi.mocked(Storage.getGlobalGeminiDir).mockReturnValue(globalMemoryDir); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + it('aggregates all .patch files of a kind into a single inbox entry', async () => { + // Multiple physical .patch files in the kind dir → ONE consolidated + // inbox entry per kind, with all hunks merged into entries[]. + const target = path.join(memoryTempDir, 'MEMORY.md'); + await fs.writeFile(target, '- old\n'); + + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'a-update.patch'), + buildUpdatePatch(target, '- old\n', '- new\n'), + ); + // Second source patch — same kind, different hunk. + const sibling = path.join(memoryTempDir, 'topic.md'); + await fs.writeFile(sibling, 'topic A\n'); + await fs.writeFile( + path.join(patchDir, 'b-topic.patch'), + buildUpdatePatch(sibling, 'topic A\n', 'topic B\n'), + ); + + const patches = await listInboxMemoryPatches(patchConfig); + + expect(patches).toHaveLength(1); + const memoryPatch = patches[0]; + expect(memoryPatch).toMatchObject({ + kind: 'private', + relativePath: 'private', + name: 'Private memory', + }); + // Both source files contributed their hunks. + expect(memoryPatch.entries).toHaveLength(2); + expect(memoryPatch.sourceFiles).toEqual([ + 'a-update.patch', + 'b-topic.patch', + ]); + expect(memoryPatch.entries[0].targetPath).toBe(target); + expect(memoryPatch.entries[0].isNewFile).toBe(false); + expect(memoryPatch.entries[1].targetPath).toBe(sibling); + expect(memoryPatch.extractedAt).toBeDefined(); + }); + + it('omits patches whose headers leave the allowed root from the listing', async () => { + // Bad patches must NOT show up in the inbox at all — listing filters + // them out so the user only ever sees actionable items. (They'd also + // be rejected at Apply time, but we don't want to surface them.) + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'escape.patch'), + buildCreationPatch(path.join(projectRoot, 'GEMINI.md'), 'Hi.\n'), + ); + + const patches = await listInboxMemoryPatches(patchConfig); + expect(patches).toHaveLength(0); + + // Direct apply still rejects it (defense-in-depth). + const result = await applyInboxMemoryPatch( + patchConfig, + 'private', + 'escape.patch', + ); + expect(result.success).toBe(false); + expect(result.message).toMatch(/outside the private memory root/i); + }); + + it('omits global patches with disallowed targets from the listing', async () => { + // Same defense for the global tier: only ~/.gemini/GEMINI.md is allowed. + // memory.md (legacy lowercase), sibling .md files, and settings.json all + // get filtered out of the listing instead of confusing the user. + const patchDir = path.join(memoryTempDir, '.inbox', 'global'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'wrong-name.patch'), + buildCreationPatch( + path.join(globalMemoryDir, 'memory.md'), + 'rejected\n', + ), + ); + await fs.writeFile( + path.join(patchDir, 'sibling.patch'), + buildCreationPatch( + path.join(globalMemoryDir, 'notes.md'), + 'rejected\n', + ), + ); + await fs.writeFile( + path.join(patchDir, 'settings.patch'), + buildCreationPatch(path.join(globalMemoryDir, 'settings.json'), '{}\n'), + ); + + const patches = await listInboxMemoryPatches(patchConfig); + expect(patches).toHaveLength(0); + }); + + it('applies a private update patch and removes it from the inbox', async () => { + const target = path.join(memoryTempDir, 'MEMORY.md'); + await fs.writeFile(target, '- old\n'); + + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'MEMORY.patch'), + buildUpdatePatch(target, '- old\n', '- accepted\n'), + ); + + const result = await applyInboxMemoryPatch( + patchConfig, + 'private', + 'MEMORY.patch', + ); + + expect(result.success).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe('- accepted\n'); + await expect( + fs.access(path.join(patchDir, 'MEMORY.patch')), + ).rejects.toThrow(); + }); + + it('applies a private creation patch with a paired MEMORY.md pointer', async () => { + // The auto-memory contract: creating a sibling .md file requires a + // hunk that adds a pointer to MEMORY.md (so the sibling becomes + // discoverable to future sessions). + const memoryMd = path.join(memoryTempDir, 'MEMORY.md'); + await fs.writeFile(memoryMd, '# Project Memory\n'); + + const target = path.join(memoryTempDir, 'topic.md'); + await expect(fs.access(target)).rejects.toThrow(); + + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + const multiHunkPatch = + buildCreationPatch(target, '# Topic\n- new fact\n') + + buildUpdatePatch( + memoryMd, + '# Project Memory\n', + '# Project Memory\n- See topic.md for the new fact.\n', + ); + await fs.writeFile(path.join(patchDir, 'topic.patch'), multiHunkPatch); + + const result = await applyInboxMemoryPatch( + patchConfig, + 'private', + 'topic.patch', + ); + + expect(result.success).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe( + '# Topic\n- new fact\n', + ); + await expect(fs.readFile(memoryMd, 'utf-8')).resolves.toContain( + 'See topic.md', + ); + await expect( + fs.access(path.join(patchDir, 'topic.patch')), + ).rejects.toThrow(); + }); + + it('auto-bundles a MEMORY.md pointer when the patch creates an orphan sibling', async () => { + // Sibling .md files in are loaded by future sessions ONLY + // when MEMORY.md references them. To avoid orphans, applying a sibling + // creation patch with no MEMORY.md update auto-bundles a pointer line. + const memoryMd = path.join(memoryTempDir, 'MEMORY.md'); + await fs.writeFile(memoryMd, '# Project Memory\n'); + + const target = path.join(memoryTempDir, 'orphan-topic.md'); + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'orphan-topic.patch'), + buildCreationPatch(target, '# Orphan Topic\n'), + ); + + const result = await applyInboxMemoryPatch( + patchConfig, + 'private', + 'orphan-topic.patch', + ); + + expect(result.success).toBe(true); + expect(result.message).toMatch(/auto-added MEMORY\.md pointer/i); + expect(result.message).toContain('"orphan-topic.md"'); + // The sibling exists. + await expect(fs.readFile(target, 'utf-8')).resolves.toBe( + '# Orphan Topic\n', + ); + // MEMORY.md now references the sibling — using ABSOLUTE PATH so a + // future agent can `read_file` it without resolving relatives. We + // assert the line shape is `- See /orphan-topic.md ...` and + // verify the path is absolute via path.isAbsolute (cross-platform — + // the previous /^- See \/.+\/.../ regex was Unix-only and broke on + // Windows where the absolute path is e.g. `C:\Users\...\orphan-topic.md`). + const memoryAfter = await fs.readFile(memoryMd, 'utf-8'); + expect(memoryAfter).toContain(target); + const pointerLineMatch = memoryAfter.match( + /^- See (.+orphan-topic\.md) /m, + ); + expect(pointerLineMatch).not.toBeNull(); + expect(path.isAbsolute(pointerLineMatch![1])).toBe(true); + // The patch was committed and removed from inbox. + await expect( + fs.access(path.join(patchDir, 'orphan-topic.patch')), + ).rejects.toThrow(); + }); + + it('auto-creates MEMORY.md if it does not exist when bundling pointers', async () => { + // No MEMORY.md on disk + a creation patch for a sibling → + // auto-bundle should create MEMORY.md from scratch with the pointer. + const memoryMd = path.join(memoryTempDir, 'MEMORY.md'); + await expect(fs.access(memoryMd)).rejects.toThrow(); + + const target = path.join(memoryTempDir, 'fresh-topic.md'); + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'fresh-topic.patch'), + buildCreationPatch(target, '# Fresh Topic\n'), + ); + + const result = await applyInboxMemoryPatch( + patchConfig, + 'private', + 'fresh-topic.patch', + ); + + expect(result.success).toBe(true); + expect(result.message).toMatch(/auto-added MEMORY\.md pointer/i); + const memoryAfter = await fs.readFile(memoryMd, 'utf-8'); + expect(memoryAfter).toContain('Project Memory'); + // Pointer must be absolute so the future agent can read_file directly. + expect(memoryAfter).toContain(target); + }); + + it('accepts a private creation patch when MEMORY.md already references the new file', async () => { + // If MEMORY.md was previously prepared with a pointer (e.g. by a + // separately-applied patch), the follow-up creation patch is fine. + const memoryMd = path.join(memoryTempDir, 'MEMORY.md'); + await fs.writeFile( + memoryMd, + '# Project Memory\n- See later-topic.md for details.\n', + ); + + const target = path.join(memoryTempDir, 'later-topic.md'); + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'later-topic.patch'), + buildCreationPatch(target, '# Later Topic\n'), + ); + + const result = await applyInboxMemoryPatch( + patchConfig, + 'private', + 'later-topic.patch', + ); + + expect(result.success).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe( + '# Later Topic\n', + ); + }); + + it('applies a global creation patch to ~/.gemini/GEMINI.md', async () => { + const target = path.join(globalMemoryDir, 'GEMINI.md'); + // Sanity check: target does not exist before apply. + await expect(fs.access(target)).rejects.toThrow(); + + const patchDir = path.join(memoryTempDir, '.inbox', 'global'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'GEMINI.patch'), + buildCreationPatch(target, '# Personal preferences\n- prefer X\n'), + ); + + const result = await applyInboxMemoryPatch( + patchConfig, + 'global', + 'GEMINI.patch', + ); + + expect(result.success).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe( + '# Personal preferences\n- prefer X\n', + ); + await expect( + fs.access(path.join(patchDir, 'GEMINI.patch')), + ).rejects.toThrow(); + }); + + it('applies a global update patch to ~/.gemini/GEMINI.md', async () => { + const target = path.join(globalMemoryDir, 'GEMINI.md'); + await fs.writeFile(target, '- prefer X\n'); + + const patchDir = path.join(memoryTempDir, '.inbox', 'global'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'GEMINI.patch'), + buildUpdatePatch(target, '- prefer X\n', '- prefer Y\n'), + ); + + const result = await applyInboxMemoryPatch( + patchConfig, + 'global', + 'GEMINI.patch', + ); + + expect(result.success).toBe(true); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe('- prefer Y\n'); + await expect( + fs.access(path.join(patchDir, 'GEMINI.patch')), + ).rejects.toThrow(); + }); + + it('dismisses a single memory patch from the inbox (legacy single-file mode)', async () => { + const patchDir = path.join(memoryTempDir, '.inbox', 'global'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'GEMINI.patch'), + buildCreationPatch( + path.join(globalMemoryDir, 'GEMINI.md'), + 'Prefer concise.\n', + ), + ); + + const result = await dismissInboxMemoryPatch( + patchConfig, + 'global', + 'GEMINI.patch', + ); + + expect(result.success).toBe(true); + await expect( + fs.access(path.join(patchDir, 'GEMINI.patch')), + ).rejects.toThrow(); + }); + + it('apply with relativePath = kind runs every source patch in sequence', async () => { + // Aggregate apply: pass `relativePath = kind`. Each .patch file under + // the kind dir is applied atomically in lexical order; the result + // message summarizes successes/failures. + const memoryMd = path.join(memoryTempDir, 'MEMORY.md'); + await fs.writeFile(memoryMd, '- old\n'); + const sibling = path.join(memoryTempDir, 'topic.md'); + await fs.writeFile(sibling, 'topic A\n'); + + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'a-update.patch'), + buildUpdatePatch(memoryMd, '- old\n', '- new\n'), + ); + await fs.writeFile( + path.join(patchDir, 'b-topic.patch'), + buildUpdatePatch(sibling, 'topic A\n', 'topic B\n'), + ); + + const result = await applyInboxMemoryPatch( + patchConfig, + 'private', + 'private', // ← aggregate mode + ); + + expect(result.success).toBe(true); + expect(result.message).toMatch(/applied all 2 private memory patches/i); + + // Both targets were updated, both source patches removed. + await expect(fs.readFile(memoryMd, 'utf-8')).resolves.toBe('- new\n'); + await expect(fs.readFile(sibling, 'utf-8')).resolves.toBe('topic B\n'); + await expect( + fs.access(path.join(patchDir, 'a-update.patch')), + ).rejects.toThrow(); + await expect( + fs.access(path.join(patchDir, 'b-topic.patch')), + ).rejects.toThrow(); + }); + + it('aggregate apply reports successes and failures when one source patch is stale', async () => { + const memoryMd = path.join(memoryTempDir, 'MEMORY.md'); + await fs.writeFile(memoryMd, '- old\n'); + + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + // Good patch: updates the existing line. + await fs.writeFile( + path.join(patchDir, 'a-good.patch'), + buildUpdatePatch(memoryMd, '- old\n', '- new\n'), + ); + // Stale patch: context expects something that doesn't exist. + await fs.writeFile( + path.join(patchDir, 'b-stale.patch'), + buildUpdatePatch(memoryMd, '- never existed\n', '- attempted\n'), + ); + + const result = await applyInboxMemoryPatch( + patchConfig, + 'private', + 'private', + ); + + // Any failure → success=false so the dialog keeps the inbox entry + // visible. (The successful sub-patches were already removed from disk; + // the next listing will surface only the failures for retry.) + expect(result.success).toBe(false); + expect(result.message).toMatch(/applied 1 of 2/i); + expect(result.message).toMatch(/b-stale\.patch/); + + // Good patch committed and removed; stale patch stays in inbox. + await expect(fs.readFile(memoryMd, 'utf-8')).resolves.toBe('- new\n'); + await expect( + fs.access(path.join(patchDir, 'a-good.patch')), + ).rejects.toThrow(); + await expect( + fs.access(path.join(patchDir, 'b-stale.patch')), + ).resolves.toBeUndefined(); + }); + + it('dismiss with relativePath = kind removes all source patches', async () => { + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'a.patch'), + buildCreationPatch(path.join(memoryTempDir, 'a.md'), 'a\n'), + ); + await fs.writeFile( + path.join(patchDir, 'b.patch'), + buildCreationPatch(path.join(memoryTempDir, 'b.md'), 'b\n'), + ); + + const result = await dismissInboxMemoryPatch( + patchConfig, + 'private', + 'private', + ); + + expect(result.success).toBe(true); + expect(result.message).toMatch(/dismissed 2/i); + await expect(fs.access(path.join(patchDir, 'a.patch'))).rejects.toThrow(); + await expect(fs.access(path.join(patchDir, 'b.patch'))).rejects.toThrow(); + }); + + it('rejects global patches that target anything other than ~/.gemini/GEMINI.md', async () => { + const patchDir = path.join(memoryTempDir, '.inbox', 'global'); + await fs.mkdir(patchDir, { recursive: true }); + + // memory.md (lowercase) is NOT a valid global memory file. + await fs.writeFile( + path.join(patchDir, 'wrong-name.patch'), + buildCreationPatch( + path.join(globalMemoryDir, 'memory.md'), + 'Should be rejected.\n', + ), + ); + + // Sibling .md files in ~/.gemini/ are also not allowed. + await fs.writeFile( + path.join(patchDir, 'sibling.patch'), + buildCreationPatch( + path.join(globalMemoryDir, 'notes.md'), + 'Should be rejected.\n', + ), + ); + + // Non-memory files (settings, credentials) must stay off-limits. + await fs.writeFile( + path.join(patchDir, 'settings.patch'), + buildCreationPatch( + path.join(globalMemoryDir, 'settings.json'), + '{"foo": 1}\n', + ), + ); + + for (const fileName of [ + 'wrong-name.patch', + 'sibling.patch', + 'settings.patch', + ]) { + const result = await applyInboxMemoryPatch( + patchConfig, + 'global', + fileName, + ); + expect(result.success).toBe(false); + expect(result.message).toMatch(/outside the global memory root/i); + } + + // None of the bogus targets were created. + for (const orphan of ['memory.md', 'notes.md', 'settings.json']) { + await expect( + fs.access(path.join(globalMemoryDir, orphan)), + ).rejects.toThrow(); + } + }); + + it('rejects invalid memory patch paths', async () => { + const result = await applyInboxMemoryPatch( + patchConfig, + 'private', + '../MEMORY.patch', + ); + + expect(result.success).toBe(false); + expect(result.message).toBe('Invalid memory patch path.'); + }); + + it('rejects a creation patch whose target already exists', async () => { + const target = path.join(memoryTempDir, 'MEMORY.md'); + await fs.writeFile(target, 'pre-existing\n'); + + const patchDir = path.join(memoryTempDir, '.inbox', 'private'); + await fs.mkdir(patchDir, { recursive: true }); + await fs.writeFile( + path.join(patchDir, 'MEMORY.patch'), + buildCreationPatch(target, 'replacement\n'), + ); + + const result = await applyInboxMemoryPatch( + patchConfig, + 'private', + 'MEMORY.patch', + ); + + expect(result.success).toBe(false); + expect(result.message).toMatch(/declares a new file/); + await expect(fs.readFile(target, 'utf-8')).resolves.toBe( + 'pre-existing\n', + ); + await expect( + fs.access(path.join(patchDir, 'MEMORY.patch')), + ).resolves.toBeUndefined(); + }); + }); + describe('moveInboxSkill', () => { let tmpDir: string; let skillsDir: string; diff --git a/packages/core/src/commands/memory.ts b/packages/core/src/commands/memory.ts index 286cbe0e3e..53f9564871 100644 --- a/packages/core/src/commands/memory.ts +++ b/packages/core/src/commands/memory.ts @@ -13,11 +13,15 @@ import type { Config } from '../config/config.js'; import { Storage } from '../config/storage.js'; import { flattenMemory } from '../config/memory.js'; import { loadSkillFromFile, loadSkillsFromDir } from '../skills/skillLoader.js'; +import { getGlobalMemoryFilePath } from '../tools/memoryTool.js'; import { type AppliedSkillPatchTarget, + applyParsedPatchesWithAllowedRoots, applyParsedSkillPatches, + canonicalizeAllowedPatchRoots, hasParsedPatchHunks, isProjectSkillPatchTarget, + resolveTargetWithinAllowedRoots, validateParsedSkillPatchHeaders, } from '../services/memoryPatchUtils.js'; import { readExtractionState } from '../services/memoryService.js'; @@ -338,6 +342,46 @@ export interface InboxPatch { extractedAt?: string; } +export type InboxMemoryPatchKind = 'private' | 'global'; + +/** + * One target file inside a memory patch (most patches will have a single entry). + */ +export interface InboxMemoryPatchEntry { + /** Absolute path of the markdown file the patch will modify. */ + targetPath: string; + /** Unified diff for this single file (used for UI preview). */ + diffContent: string; + /** True when this entry creates a new file (`/dev/null` source). */ + isNewFile: boolean; +} + +/** + * Represents the AGGREGATED inbox state for one memory kind. Even when the + * extraction agent has produced multiple `.patch` files under + * `/.inbox//` (e.g. across several sessions), the inbox + * surfaces them as ONE entry per kind. Apply runs each underlying patch in + * sequence; Dismiss removes them all. + */ +export interface InboxMemoryPatch { + /** Memory tier — one entry per kind in the inbox. */ + kind: InboxMemoryPatchKind; + /** + * Stable identifier for this consolidated entry. Set to the kind itself + * (`"private"` or `"global"`); kept in the type for backwards-compat with + * the per-file API the dialog passes through. + */ + relativePath: string; + /** Display name shown in the inbox row (e.g. `"Private memory"`). */ + name: string; + /** All hunks from all underlying source patches, concatenated in order. */ + entries: InboxMemoryPatchEntry[]; + /** Basenames of the underlying `.patch` files being aggregated. */ + sourceFiles: string[]; + /** Most recent mtime across the source files (ISO string), if known. */ + extractedAt?: string; +} + interface StagedInboxPatchTarget { targetPath: string; tempPath: string; @@ -372,6 +416,97 @@ function getErrorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } +function getMemoryPatchRoot( + memoryDir: string, + kind: InboxMemoryPatchKind, +): string { + return path.join(memoryDir, '.inbox', kind); +} + +function isSubpathOrSame(childPath: string, parentPath: string): boolean { + const relativePath = path.relative(parentPath, childPath); + return ( + relativePath === '' || + (!relativePath.startsWith('..') && !path.isAbsolute(relativePath)) + ); +} + +function normalizeInboxMemoryPatchPath( + relativePath: string, +): string | undefined { + if ( + relativePath.length === 0 || + path.isAbsolute(relativePath) || + relativePath.includes('\\') + ) { + return undefined; + } + + const normalizedPath = path.posix.normalize(relativePath); + if ( + normalizedPath === '.' || + normalizedPath.startsWith('../') || + normalizedPath === '..' || + !normalizedPath.endsWith('.patch') + ) { + return undefined; + } + return normalizedPath; +} + +/** + * Returns the directory roots (or single-file allowlists) that a memory patch + * of the given kind is allowed to modify. Memory patch headers must reference + * paths inside / equal to one of these entries after canonical resolution. + * + * - `private` allows any markdown file inside the project memory directory. + * - `global` is intentionally a single-file allowlist: the only writeable + * global file is the personal `~/.gemini/GEMINI.md`. Other files under + * `~/.gemini/` (settings, credentials, oauth, keybindings, etc.) are off-limits. + */ +export function getAllowedMemoryPatchRoots( + config: Config, + kind: InboxMemoryPatchKind, +): string[] { + switch (kind) { + case 'private': + return [path.resolve(config.storage.getProjectMemoryTempDir())]; + case 'global': + return [path.resolve(getGlobalMemoryFilePath())]; + default: + throw new Error(`Unknown memory patch kind: ${kind as string}`); + } +} + +async function getFileMtimeIso(filePath: string): Promise { + try { + const stats = await fs.stat(filePath); + return stats.mtime.toISOString(); + } catch { + return undefined; + } +} + +async function getInboxMemoryPatchSourcePath( + config: Config, + kind: InboxMemoryPatchKind, + relativePath: string, +): Promise { + const normalizedPath = normalizeInboxMemoryPatchPath(relativePath); + if (!normalizedPath) { + return undefined; + } + + const patchRoot = path.resolve( + getMemoryPatchRoot(config.storage.getProjectMemoryTempDir(), kind), + ); + const sourcePath = path.resolve(patchRoot, ...normalizedPath.split('/')); + if (!isSubpathOrSame(sourcePath, patchRoot)) { + return undefined; + } + return sourcePath; +} + async function patchTargetsProjectSkills( targetPaths: string[], config: Config, @@ -395,6 +530,670 @@ async function getPatchExtractedAt( } } +function formatMemoryKindLabel(kind: InboxMemoryPatchKind): string { + switch (kind) { + case 'private': + return 'Private memory'; + case 'global': + return 'Global memory'; + default: + return kind; + } +} + +/** + * Returns the absolute paths of every `.patch` file currently in the kind's + * inbox directory (sorted by basename for stable ordering at apply time). + * + * NOTE: this is a raw filesystem listing — it does NOT validate patch shape + * or that targets fall inside the kind's allowed root. Callers that need + * "what the user actually sees in the inbox" should use `listValidInboxPatchFiles`. + */ +async function listInboxPatchFiles( + config: Config, + kind: InboxMemoryPatchKind, +): Promise { + const patchRoot = getMemoryPatchRoot( + config.storage.getProjectMemoryTempDir(), + kind, + ); + const found: string[] = []; + + async function walk(currentDir: string): Promise { + let dirEntries: Array; + try { + dirEntries = await fs.readdir(currentDir, { withFileTypes: true }); + } catch { + return; + } + + for (const entry of dirEntries) { + const entryPath = path.join(currentDir, entry.name); + if (entry.isDirectory()) { + await walk(entryPath); + continue; + } + if (entry.isFile() && entry.name.endsWith('.patch')) { + found.push(entryPath); + } + } + } + + await walk(patchRoot); + return found.sort(); +} + +/** + * Returns only the inbox patch files that pass the same validation as the + * inbox listing (parseable, has hunks, valid headers, targets in the + * kind's allowed root). Used by aggregate apply so the user only ever sees + * results for patches the inbox actually surfaced. + */ +async function listValidInboxPatchFiles( + config: Config, + kind: InboxMemoryPatchKind, +): Promise { + const patchFiles = await listInboxPatchFiles(config, kind); + if (patchFiles.length === 0) { + return []; + } + + const allowedRoots = await canonicalizeAllowedPatchRoots( + getAllowedMemoryPatchRoots(config, kind), + ); + + const valid: string[] = []; + for (const sourcePath of patchFiles) { + let content: string; + try { + content = await fs.readFile(sourcePath, 'utf-8'); + } catch { + continue; + } + + let parsed: Diff.StructuredPatch[]; + try { + parsed = Diff.parsePatch(content); + } catch { + continue; + } + if (!hasParsedPatchHunks(parsed)) { + continue; + } + + const validated = validateParsedSkillPatchHeaders(parsed); + if (!validated.success) { + continue; + } + + const targetsAllAllowed = await Promise.all( + validated.patches.map( + async (header) => + (await resolveTargetWithinAllowedRoots( + header.targetPath, + allowedRoots, + )) !== undefined, + ), + ); + if (!targetsAllAllowed.every(Boolean)) { + continue; + } + + valid.push(sourcePath); + } + return valid; +} + +/** + * Scans `/.inbox/{private,global}/` and returns ONE consolidated + * inbox entry per kind. Each entry aggregates all hunks from every valid + * underlying `.patch` file. Patches that fail validation (unparseable, no + * hunks, target outside allowed root) are silently skipped so they don't + * pollute the inbox UI. + */ +export async function listInboxMemoryPatches( + config: Config, +): Promise { + const kinds: InboxMemoryPatchKind[] = ['private', 'global']; + const aggregated: InboxMemoryPatch[] = []; + + for (const kind of kinds) { + const allowedRoots = await canonicalizeAllowedPatchRoots( + getAllowedMemoryPatchRoots(config, kind), + ); + const patchFiles = await listInboxPatchFiles(config, kind); + + const aggregatedEntries: InboxMemoryPatchEntry[] = []; + const sourceFiles: string[] = []; + let latestMtime: string | undefined; + + for (const sourcePath of patchFiles) { + let content: string; + try { + content = await fs.readFile(sourcePath, 'utf-8'); + } catch { + continue; + } + + let parsed: Diff.StructuredPatch[]; + try { + parsed = Diff.parsePatch(content); + } catch { + continue; + } + if (!hasParsedPatchHunks(parsed)) { + continue; + } + + const validated = validateParsedSkillPatchHeaders(parsed); + if (!validated.success) { + continue; + } + + // Skip the entire source file if ANY of its targets escapes the kind's + // allowed root. + const targetsAllAllowed = await Promise.all( + validated.patches.map( + async (header) => + (await resolveTargetWithinAllowedRoots( + header.targetPath, + allowedRoots, + )) !== undefined, + ), + ); + if (!targetsAllAllowed.every(Boolean)) { + continue; + } + + for (const [index, header] of validated.patches.entries()) { + aggregatedEntries.push({ + targetPath: header.targetPath, + isNewFile: header.isNewFile, + diffContent: formatParsedDiff(parsed[index]), + }); + } + + sourceFiles.push(path.basename(sourcePath)); + + const mtime = await getFileMtimeIso(sourcePath); + if (mtime && (!latestMtime || mtime > latestMtime)) { + latestMtime = mtime; + } + } + + if (aggregatedEntries.length === 0) { + continue; + } + + aggregated.push({ + kind, + relativePath: kind, + name: formatMemoryKindLabel(kind), + entries: aggregatedEntries, + sourceFiles, + extractedAt: latestMtime, + }); + } + + return aggregated; +} + +/** + * Applies an inbox memory patch atomically and removes the patch on success. + * + * Process: + * 1. Parse + validate the patch headers (absolute paths only, no `a/`/`b/`). + * 2. Dry-run the patch against the current target content (or empty for + * `/dev/null` creation patches). + * 3. Stage the patched content to a temp file, then rename into place. + * 4. On any failure, restore previous content from the staged snapshot and + * leave the inbox patch intact for retry. + */ +/** + * Applies one inbox memory entry. Two modes: + * - Aggregate mode (`relativePath === kind`): walk every `.patch` file in + * the kind's inbox directory and apply each one in lexical order. Each + * file is its own atomic transaction; failures don't block subsequent + * successes. Returns an aggregated summary (e.g. "Applied 3 of 4 sub- + * patches; 1 failed: …"). + * - Single-file mode (legacy): `relativePath` points at a specific + * `.patch` filename. Used by tests and direct callers. + */ +export async function applyInboxMemoryPatch( + config: Config, + kind: InboxMemoryPatchKind, + relativePath: string, +): Promise<{ success: boolean; message: string }> { + if (relativePath === kind) { + return applyAllInboxPatchesForKind(config, kind); + } + + const normalizedPath = normalizeInboxMemoryPatchPath(relativePath); + if (!normalizedPath) { + return { success: false, message: 'Invalid memory patch path.' }; + } + + const sourcePath = await getInboxMemoryPatchSourcePath( + config, + kind, + normalizedPath, + ); + if (!sourcePath) { + return { success: false, message: 'Invalid memory patch path.' }; + } + + return applyMemoryPatchFile(config, kind, sourcePath, normalizedPath); +} + +async function applyAllInboxPatchesForKind( + config: Config, + kind: InboxMemoryPatchKind, +): Promise<{ success: boolean; message: string }> { + // Only attempt patches the user actually saw in the inbox listing. + // Files that were filtered (bad headers, escape allowed root, etc.) stay + // on disk untouched. + const patchFiles = await listValidInboxPatchFiles(config, kind); + if (patchFiles.length === 0) { + return { + success: false, + message: `No ${kind} memory patches in inbox.`, + }; + } + + const successes: string[] = []; + const failures: Array<{ name: string; reason: string }> = []; + let pointersAddedAcrossPatches: string[] = []; + + for (const sourcePath of patchFiles) { + const basename = path.basename(sourcePath); + const result = await applyMemoryPatchFile( + config, + kind, + sourcePath, + basename, + ); + if (result.success) { + successes.push(basename); + // Surface auto-added MEMORY.md pointer info if present. + const pointerMatch = result.message.match( + /Auto-added MEMORY\.md pointer for ([^.]+)\./, + ); + if (pointerMatch) { + pointersAddedAcrossPatches.push(pointerMatch[1]); + } + } else { + failures.push({ name: basename, reason: result.message }); + } + } + + // De-dup pointer notes (same sibling could have been mentioned twice). + pointersAddedAcrossPatches = Array.from(new Set(pointersAddedAcrossPatches)); + + const total = successes.length + failures.length; + if (failures.length === 0) { + const pointerNote = + pointersAddedAcrossPatches.length > 0 + ? ` Auto-added MEMORY.md pointer(s) for ${pointersAddedAcrossPatches.join('; ')}.` + : ''; + return { + success: true, + message: `Applied all ${successes.length} ${kind} memory patch${ + successes.length === 1 ? '' : 'es' + }.${pointerNote}`, + }; + } + + const failureSummary = failures + .map((f) => `"${f.name}" — ${f.reason}`) + .join('; '); + // Any failure → success=false so the dialog keeps the inbox entry visible + // (the user needs to see and retry/dismiss the remaining sub-patches). + // The successful sub-patches have already been removed from disk by + // applyMemoryPatchFile, so the next listing will show only the failures. + return { + success: false, + message: + `Applied ${successes.length} of ${total} ${kind} memory patches. ` + + `${failures.length} failed: ${failureSummary}`, + }; +} + +async function canonicalizeDirIfPresent(dirPath: string): Promise { + try { + return await fs.realpath(dirPath); + } catch { + return path.resolve(dirPath); + } +} + +/** + * Returns the basenames of any sibling .md files (not MEMORY.md itself) that + * are being CREATED by this patch under `/` directly. + */ +function findSiblingCreations( + appliedResults: readonly AppliedSkillPatchTarget[], + memoryDir: string, +): AppliedSkillPatchTarget[] { + return appliedResults.filter((entry) => { + if (!entry.isNewFile) return false; + const targetDir = path.dirname(path.resolve(entry.targetPath)); + if (targetDir !== memoryDir) return false; + const basename = path.basename(entry.targetPath); + if (basename.toLowerCase() === 'memory.md') return false; + return basename.toLowerCase().endsWith('.md'); + }); +} + +interface AutoPointerAugmentation { + /** Patch results, possibly with a synthesized/extended MEMORY.md entry. */ + results: AppliedSkillPatchTarget[]; + /** Sibling basenames a pointer was auto-added for (empty if none). */ + pointersAdded: string[]; +} + +/** + * MEMORY.md is the index that gets injected into future agent contexts. + * Sibling .md files in `/` are loaded ON DEMAND by the runtime + * agent via `read_file` — but only IF MEMORY.md references them by name + * (see `getUserProjectMemoryPaths`). + * + * If a private patch creates a sibling without also referencing it from + * MEMORY.md, the new file would never be discoverable. Rather than rejecting + * the patch (bad UX), we auto-bundle a MEMORY.md update that adds a + * one-line pointer per orphan sibling. The augmented entry is then committed + * atomically alongside the rest of the patch. + * + * If the patch already updates/creates MEMORY.md and the new content already + * references the sibling, no augmentation is needed. + */ +async function augmentWithAutoPointers( + config: Config, + appliedResults: readonly AppliedSkillPatchTarget[], +): Promise { + const memoryDir = await canonicalizeDirIfPresent( + config.storage.getProjectMemoryTempDir(), + ); + const memoryMdPath = path.join(memoryDir, 'MEMORY.md'); + + const siblingCreations = findSiblingCreations(appliedResults, memoryDir); + if (siblingCreations.length === 0) { + return { results: [...appliedResults], pointersAdded: [] }; + } + + // Locate (or initialize) the MEMORY.md entry we'll mutate. + const existingIdx = appliedResults.findIndex( + (entry) => path.resolve(entry.targetPath) === memoryMdPath, + ); + let memoryEntry: AppliedSkillPatchTarget; + if (existingIdx >= 0) { + memoryEntry = { ...appliedResults[existingIdx] }; + } else { + let originalContent = ''; + let isNewFile = true; + try { + originalContent = await fs.readFile(memoryMdPath, 'utf-8'); + isNewFile = false; + } catch { + // MEMORY.md doesn't exist yet — we'll create it with a default heading. + } + memoryEntry = { + targetPath: memoryMdPath, + original: originalContent, + patched: isNewFile ? '# Project Memory\n' : originalContent, + isNewFile, + }; + } + + const pointersAdded: string[] = []; + for (const sibling of siblingCreations) { + const basename = path.basename(sibling.targetPath); + // Resolve to absolute path so the runtime agent can `read_file` the + // sibling directly without needing to know . + const absoluteTarget = path.resolve(sibling.targetPath); + // Existing reference can be by either basename or absolute path; both count. + if ( + memoryEntry.patched.includes(basename) || + memoryEntry.patched.includes(absoluteTarget) + ) { + continue; // Already referenced. + } + const stem = basename.replace(/\.md$/i, '').replace(/[-_]/g, ' ').trim(); + const pointer = `- See ${absoluteTarget} for ${stem || basename} notes.`; + memoryEntry.patched = memoryEntry.patched.endsWith('\n') + ? `${memoryEntry.patched}${pointer}\n` + : `${memoryEntry.patched}\n${pointer}\n`; + pointersAdded.push(basename); + } + + if (pointersAdded.length === 0) { + return { results: [...appliedResults], pointersAdded: [] }; + } + + const results = [...appliedResults]; + if (existingIdx >= 0) { + results[existingIdx] = memoryEntry; + } else { + results.push(memoryEntry); + } + return { results, pointersAdded }; +} + +/** + * Internal helper: parses, validates, and atomically commits a memory patch + * file at a known absolute path. Separated from `applyInboxMemoryPatch` so the + * path-resolution and patch-apply concerns stay testable independently. + */ +async function applyMemoryPatchFile( + config: Config, + kind: InboxMemoryPatchKind, + patchPath: string, + displayName: string, +): Promise<{ success: boolean; message: string }> { + let content: string; + try { + content = await fs.readFile(patchPath, 'utf-8'); + } catch { + return { + success: false, + message: `Memory patch "${displayName}" not found in inbox.`, + }; + } + + let parsed: Diff.StructuredPatch[]; + try { + parsed = Diff.parsePatch(content); + } catch (error) { + return { + success: false, + message: `Failed to parse memory patch "${displayName}": ${getErrorMessage(error)}`, + }; + } + if (!hasParsedPatchHunks(parsed)) { + return { + success: false, + message: `Memory patch "${displayName}" contains no valid hunks.`, + }; + } + + const allowedRoots = await canonicalizeAllowedPatchRoots( + getAllowedMemoryPatchRoots(config, kind), + ); + const applied = await applyParsedPatchesWithAllowedRoots( + parsed, + allowedRoots, + ); + if (!applied.success) { + switch (applied.reason) { + case 'missingTargetPath': + return { + success: false, + message: `Memory patch "${displayName}" is missing a target file path.`, + }; + case 'invalidPatchHeaders': + return { + success: false, + message: `Memory patch "${displayName}" has invalid diff headers.`, + }; + case 'outsideAllowedRoots': + return { + success: false, + message: `Memory patch "${displayName}" targets a file outside the ${kind} memory root: ${applied.targetPath}`, + }; + case 'newFileAlreadyExists': + return { + success: false, + message: `Memory patch "${displayName}" declares a new file, but the target already exists: ${applied.targetPath}`, + }; + case 'targetNotFound': + return { + success: false, + message: `Target file not found: ${applied.targetPath}`, + }; + case 'doesNotApply': + return { + success: false, + message: applied.isNewFile + ? `Memory patch "${displayName}" failed to apply for new file ${applied.targetPath}.` + : `Memory patch does not apply cleanly to ${applied.targetPath}.`, + }; + default: + return { + success: false, + message: `Memory patch "${displayName}" could not be applied.`, + }; + } + } + + // Auto-bundle a MEMORY.md pointer for any sibling .md the patch creates + // without referencing it from MEMORY.md. Without that pointer the new file + // would never be loaded into a future session (see augmentWithAutoPointers). + let pointersAdded: string[] = []; + let resultsToCommit: AppliedSkillPatchTarget[] = [...applied.results]; + if (kind === 'private') { + const augmented = await augmentWithAutoPointers(config, applied.results); + resultsToCommit = augmented.results; + pointersAdded = augmented.pointersAdded; + } + + let stagedTargets: StagedInboxPatchTarget[]; + try { + stagedTargets = await stageInboxPatchTargets(resultsToCommit); + } catch (error) { + return { + success: false, + message: `Memory patch "${displayName}" could not be staged: ${getErrorMessage(error)}.`, + }; + } + + const committedTargets: StagedInboxPatchTarget[] = []; + try { + for (const stagedTarget of stagedTargets) { + await fs.rename(stagedTarget.tempPath, stagedTarget.targetPath); + committedTargets.push(stagedTarget); + } + } catch (error) { + for (const committedTarget of committedTargets.reverse()) { + try { + await restoreCommittedInboxPatchTarget(committedTarget); + } catch { + // Best-effort rollback. We still report the commit failure below. + } + } + await cleanupStagedInboxPatchTargets( + stagedTargets.filter((target) => !committedTargets.includes(target)), + ); + return { + success: false, + message: `Memory patch "${displayName}" could not be applied atomically: ${getErrorMessage(error)}.`, + }; + } + + await fs.unlink(patchPath); + + const fileCount = resultsToCommit.length; + const baseMessage = `Applied memory patch to ${fileCount} file${fileCount !== 1 ? 's' : ''}.`; + const pointerNote = + pointersAdded.length > 0 + ? ` Auto-added MEMORY.md pointer for ${pointersAdded + .map((name) => `"${name}"`) + .join(', ')} so the new sibling file is discoverable.` + : ''; + return { + success: true, + message: `${baseMessage}${pointerNote}`, + }; +} + +/** + * Removes inbox memory patch(es) without applying. Two modes: + * - Aggregate (`relativePath === kind`): unlink every `.patch` file in the + * kind's inbox directory. Used by the consolidated inbox UI's Dismiss. + * - Single-file (legacy): unlink one specific `.patch` file. + */ +export async function dismissInboxMemoryPatch( + config: Config, + kind: InboxMemoryPatchKind, + relativePath: string, +): Promise<{ success: boolean; message: string }> { + if (relativePath === kind) { + // Dismiss the same set of files the listing surfaced — leave the + // already-filtered (bad-target, malformed) files alone for forensic + // inspection. + const patchFiles = await listValidInboxPatchFiles(config, kind); + if (patchFiles.length === 0) { + return { + success: false, + message: `No ${kind} memory patches in inbox.`, + }; + } + let removed = 0; + for (const sourcePath of patchFiles) { + try { + await fs.unlink(sourcePath); + removed += 1; + } catch { + // Best-effort: keep going if one delete fails. + } + } + return { + success: removed > 0, + message: `Dismissed ${removed} ${kind} memory patch${ + removed === 1 ? '' : 'es' + } from inbox.`, + }; + } + + const normalizedPath = normalizeInboxMemoryPatchPath(relativePath); + if (!normalizedPath) { + return { success: false, message: 'Invalid memory patch path.' }; + } + + const sourcePath = await getInboxMemoryPatchSourcePath( + config, + kind, + normalizedPath, + ); + if (!sourcePath) { + return { success: false, message: 'Invalid memory patch path.' }; + } + + try { + await fs.access(sourcePath); + } catch { + return { + success: false, + message: `Memory patch "${normalizedPath}" not found in inbox.`, + }; + } + + await fs.unlink(sourcePath); + + return { + success: true, + message: `Dismissed "${normalizedPath}" from inbox.`, + }; +} + async function findNearestExistingDirectory( startPath: string, ): Promise { diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 843acda12f..efff35eda7 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -72,6 +72,10 @@ import { } from './models.js'; import { Storage } from './storage.js'; import type { AgentLoopContext } from './agent-loop-context.js'; +import { + runWithScopedAutoMemoryExtractionWriteAccess, + runWithScopedMemoryInboxAccess, +} from './scoped-config.js'; vi.mock('fs', async (importOriginal) => { const actual = await importOriginal(); @@ -3656,6 +3660,168 @@ describe('Config JIT Initialization', () => { config.isPathAllowed(path.join(globalDir, 'oauth_creds.json')), ).toBe(false); }); + + it('should NOT allow isPathAllowed to write into the auto-memory inbox', () => { + // /.inbox/ is owned by the extraction agent and the + // /memory inbox review flow. The main agent must not be able to drop + // patches in there directly, even though it falls inside . + // We bypass Config.initialize() (the GitService init path is independently + // flaky in this suite) by spying on the storage methods isPathAllowed + // actually consults. + const params: ConfigParameters = { + sessionId: 'test-session', + targetDir: '/tmp/test', + debugMode: false, + model: 'test-model', + cwd: '/tmp/test', + }; + + config = new Config(params); + + const fakeMemoryTempDir = '/tmp/test-fake-temp/memory'; + const fakeProjectTempDir = '/tmp/test-fake-temp'; + vi.spyOn(config.storage, 'getProjectMemoryTempDir').mockReturnValue( + fakeMemoryTempDir, + ); + vi.spyOn(config.storage, 'getProjectTempDir').mockReturnValue( + fakeProjectTempDir, + ); + + const inboxRoot = path.join(fakeMemoryTempDir, '.inbox'); + + // The inbox directory itself and any path under it are denied. + expect(config.isPathAllowed(inboxRoot)).toBe(false); + expect( + config.isPathAllowed(path.join(inboxRoot, 'private', 'foo.patch')), + ).toBe(false); + expect( + config.isPathAllowed(path.join(inboxRoot, 'global', 'bar.patch')), + ).toBe(false); + + // Sibling files under stay reachable so the main + // agent can edit MEMORY.md and topic notes directly. + expect( + config.isPathAllowed(path.join(fakeMemoryTempDir, 'MEMORY.md')), + ).toBe(true); + expect( + config.isPathAllowed(path.join(fakeMemoryTempDir, 'some-topic.md')), + ).toBe(true); + }); + + it('should allow scoped extraction access only to canonical inbox patches', () => { + const params: ConfigParameters = { + sessionId: 'test-session', + targetDir: '/tmp/test', + debugMode: false, + model: 'test-model', + cwd: '/tmp/test', + }; + + config = new Config(params); + + const fakeMemoryTempDir = '/tmp/test-fake-temp/memory'; + const fakeProjectTempDir = '/tmp/test-fake-temp'; + vi.spyOn(config.storage, 'getProjectMemoryTempDir').mockReturnValue( + fakeMemoryTempDir, + ); + vi.spyOn(config.storage, 'getProjectTempDir').mockReturnValue( + fakeProjectTempDir, + ); + + const inboxRoot = path.join(fakeMemoryTempDir, '.inbox'); + const privateExtractionPatch = path.join( + inboxRoot, + 'private', + 'extraction.patch', + ); + const globalExtractionPatch = path.join( + inboxRoot, + 'global', + 'extraction.patch', + ); + + expect(config.isPathAllowed(privateExtractionPatch)).toBe(false); + + runWithScopedMemoryInboxAccess(() => { + expect(config.isPathAllowed(privateExtractionPatch)).toBe(true); + expect(config.validatePathAccess(privateExtractionPatch)).toBeNull(); + expect(config.isPathAllowed(globalExtractionPatch)).toBe(true); + expect( + config.isPathAllowed(path.join(inboxRoot, 'private', 'other.patch')), + ).toBe(false); + expect( + config.isPathAllowed( + path.join(inboxRoot, 'private', 'nested', 'extraction.patch'), + ), + ).toBe(false); + }); + + expect(config.isPathAllowed(privateExtractionPatch)).toBe(false); + }); + + it('should restrict scoped auto-memory extraction writes to generated artifacts', () => { + const params: ConfigParameters = { + sessionId: 'test-session', + targetDir: '/tmp/test', + debugMode: false, + model: 'test-model', + cwd: '/tmp/test', + }; + + config = new Config(params); + + const fakeMemoryTempDir = '/tmp/test-fake-temp/memory'; + const fakeProjectTempDir = '/tmp/test-fake-temp'; + const fakeSkillsMemoryDir = path.join(fakeMemoryTempDir, 'skills'); + vi.spyOn(config.storage, 'getProjectMemoryTempDir').mockReturnValue( + fakeMemoryTempDir, + ); + vi.spyOn(config.storage, 'getProjectTempDir').mockReturnValue( + fakeProjectTempDir, + ); + vi.spyOn(config.storage, 'getProjectSkillsMemoryDir').mockReturnValue( + fakeSkillsMemoryDir, + ); + + const inboxRoot = path.join(fakeMemoryTempDir, '.inbox'); + const privateExtractionPatch = path.join( + inboxRoot, + 'private', + 'extraction.patch', + ); + const skillArtifact = path.join( + fakeSkillsMemoryDir, + 'my-skill', + 'SKILL.md', + ); + const activeMemoryPath = path.join(fakeMemoryTempDir, 'MEMORY.md'); + const projectTempPath = path.join(fakeProjectTempDir, 'logs', 'run.log'); + const workspaceMemoryPath = path.join('/tmp/test', 'GEMINI.md'); + + expect(config.validatePathAccess(activeMemoryPath)).toBeNull(); + + runWithScopedAutoMemoryExtractionWriteAccess(() => { + expect(config.validatePathAccess(skillArtifact)).toBeNull(); + expect(config.validatePathAccess(activeMemoryPath)).toContain( + 'Auto-memory extraction write denied', + ); + expect(config.validatePathAccess(projectTempPath)).toContain( + 'Auto-memory extraction write denied', + ); + expect(config.validatePathAccess(workspaceMemoryPath)).toContain( + 'Auto-memory extraction write denied', + ); + + // Reads still use the normal workspace/temp allowlists. + expect(config.validatePathAccess(activeMemoryPath, 'read')).toBeNull(); + }); + + runWithScopedMemoryInboxAccess(() => { + runWithScopedAutoMemoryExtractionWriteAccess(() => { + expect(config.validatePathAccess(privateExtractionPatch)).toBeNull(); + }); + }); + }); }); describe('isAutoMemoryEnabled', () => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 9d52450d03..985915e6ff 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -140,7 +140,11 @@ import type { GenerateContentParameters } from '@google/genai'; export type { MCPOAuthConfig, AnyToolInvocation, AnyDeclarativeTool }; import type { AnyToolInvocation, AnyDeclarativeTool } from '../tools/tools.js'; import { WorkspaceContext } from '../utils/workspaceContext.js'; -import { getWorkspaceContextOverride } from './scoped-config.js'; +import { + getWorkspaceContextOverride, + hasScopedAutoMemoryExtractionWriteAccess, + hasScopedMemoryInboxAccess, +} from './scoped-config.js'; import { Storage } from './storage.js'; import type { ShellExecutionConfig } from '../services/shellExecutionService.js'; import { FileExclusions } from '../utils/ignorePatterns.js'; @@ -3063,6 +3067,52 @@ export class Config implements McpContext, AgentLoopContext { this.ideMode = value; } + private isScopedMemoryInboxPatchPathAllowed( + absolutePath: string, + resolvedPath: string, + inboxRoot: string, + ): boolean { + if (!hasScopedMemoryInboxAccess()) { + return false; + } + + const normalizedPath = path.resolve(absolutePath); + const isCanonicalPatchPath = (['private', 'global'] as const).some( + (kind) => + normalizedPath === path.resolve(inboxRoot, kind, 'extraction.patch'), + ); + if (!isCanonicalPatchPath) { + return false; + } + + const resolvedMemoryRoot = resolveToRealPath( + this.storage.getProjectMemoryTempDir(), + ); + return isSubpath(resolvedMemoryRoot, resolvedPath); + } + + private isScopedAutoMemoryExtractionWritePathAllowed( + absolutePath: string, + resolvedPath: string, + ): boolean { + if (!hasScopedAutoMemoryExtractionWriteAccess()) { + return false; + } + + const resolvedSkillsMemoryDir = resolveToRealPath( + this.storage.getProjectSkillsMemoryDir(), + ); + if (isSubpath(resolvedSkillsMemoryDir, resolvedPath)) { + return true; + } + + return this.isScopedMemoryInboxPatchPathAllowed( + absolutePath, + resolvedPath, + path.join(this.storage.getProjectMemoryTempDir(), '.inbox'), + ); + } + /** * Get the current FileSystemService */ @@ -3077,12 +3127,48 @@ export class Config implements McpContext, AgentLoopContext { * file (the latter is the only file under `~/.gemini/` that is reachable — * settings, credentials, keybindings, etc. remain disallowed). * + * One subtree is *carved back out*: `/.inbox/` is owned by + * the auto-memory extraction agent and the `/memory inbox` review flow. The + * main agent is denied access to it even though it falls inside the project + * temp dir; the extraction agent receives a narrow execution-scoped exception + * for `.inbox/{private,global}/extraction.patch`. + * * @param absolutePath The absolute path to check. * @returns true if the path is allowed, false otherwise. */ isPathAllowed(absolutePath: string): boolean { const resolvedPath = resolveToRealPath(absolutePath); + // The auto-memory inbox (`/.inbox/`) is owned by the + // background extraction agent and the `/memory inbox` review flow. The + // main agent must NOT drop files into it directly (that would let the + // model bypass review). Deny first, even if the path also satisfies the + // workspace or project-temp allowlists below. + const inboxRoot = path.join( + this.storage.getProjectMemoryTempDir(), + '.inbox', + ); + const resolvedInboxRoot = resolveToRealPath(inboxRoot); + const normalizedPath = path.resolve(absolutePath); + const normalizedInboxRoot = path.resolve(inboxRoot); + if ( + resolvedPath === resolvedInboxRoot || + isSubpath(resolvedInboxRoot, resolvedPath) || + normalizedPath === normalizedInboxRoot || + isSubpath(normalizedInboxRoot, normalizedPath) + ) { + if ( + this.isScopedMemoryInboxPatchPathAllowed( + absolutePath, + resolvedPath, + inboxRoot, + ) + ) { + return true; + } + return false; + } + const workspaceContext = this.getWorkspaceContext(); if (workspaceContext.isPathWithinWorkspace(resolvedPath)) { return true; @@ -3122,6 +3208,19 @@ export class Config implements McpContext, AgentLoopContext { absolutePath: string, checkType: 'read' | 'write' = 'write', ): string | null { + if (checkType === 'write' && hasScopedAutoMemoryExtractionWriteAccess()) { + const resolvedPath = resolveToRealPath(absolutePath); + if ( + this.isScopedAutoMemoryExtractionWritePathAllowed( + absolutePath, + resolvedPath, + ) + ) { + return null; + } + return `Auto-memory extraction write denied: Attempted path "${absolutePath}" is outside the extraction write allowlist. Extraction may only write extracted skills under ${this.storage.getProjectSkillsMemoryDir()} and canonical inbox patches under ${path.join(this.storage.getProjectMemoryTempDir(), '.inbox', '{private,global}', 'extraction.patch')}.`; + } + // For read operations, check read-only paths first if (checkType === 'read') { if (this.getWorkspaceContext().isPathReadable(absolutePath)) { diff --git a/packages/core/src/config/scoped-config.ts b/packages/core/src/config/scoped-config.ts index 90cdea2da6..e44a73d4a2 100644 --- a/packages/core/src/config/scoped-config.ts +++ b/packages/core/src/config/scoped-config.ts @@ -19,6 +19,9 @@ import { WorkspaceContext } from '../utils/workspaceContext.js'; * This follows the same pattern as `toolCallContext` and `promptIdContext`. */ const workspaceContextOverride = new AsyncLocalStorage(); +const memoryInboxAccessOverride = new AsyncLocalStorage(); +const autoMemoryExtractionWriteAccessOverride = + new AsyncLocalStorage(); /** * Returns the current workspace context override, if any. @@ -44,6 +47,42 @@ export function runWithScopedWorkspaceContext( return workspaceContextOverride.run(scopedContext, fn); } +/** + * Returns true when the current async execution is allowed to access the + * canonical auto-memory inbox patch files. + */ +export function hasScopedMemoryInboxAccess(): boolean { + return memoryInboxAccessOverride.getStore() === true; +} + +/** + * Runs a function with access to the canonical auto-memory inbox patch files. + * This is intended for the background extraction agent only; the main agent + * continues to have the inbox carved out of its normal temp-dir access. + */ +export function runWithScopedMemoryInboxAccess(fn: () => T): T { + return memoryInboxAccessOverride.run(true, fn); +} + +/** + * Returns true when the current async execution is using the narrow + * auto-memory extraction write allowlist. + */ +export function hasScopedAutoMemoryExtractionWriteAccess(): boolean { + return autoMemoryExtractionWriteAccessOverride.getStore() === true; +} + +/** + * Runs a function with the auto-memory extraction write allowlist active. + * This prevents the background extractor from writing active memory files + * directly; it may only write extracted skills and canonical inbox patches. + */ +export function runWithScopedAutoMemoryExtractionWriteAccess( + fn: () => T, +): T { + return autoMemoryExtractionWriteAccessOverride.run(true, fn); +} + /** * Creates a {@link WorkspaceContext} that extends a parent's directories * with additional ones. diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index 5a40648a4a..fcc3cddc84 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -106,10 +106,6 @@ export class Storage { return path.join(Storage.getGlobalAgentsDir(), 'skills'); } - static getGlobalMemoryFilePath(): string { - return path.join(Storage.getGlobalGeminiDir(), 'memory.md'); - } - static getUserPoliciesDir(): string { return path.join(Storage.getGlobalGeminiDir(), 'policies'); } diff --git a/packages/core/src/services/memoryPatchUtils.ts b/packages/core/src/services/memoryPatchUtils.ts index 44b87353fe..66cb0c6092 100644 --- a/packages/core/src/services/memoryPatchUtils.ts +++ b/packages/core/src/services/memoryPatchUtils.ts @@ -247,6 +247,27 @@ export type ApplyParsedSkillPatchesResult = export async function applyParsedSkillPatches( parsedPatches: StructuredPatch[], config: Config, +): Promise { + const allowedRoots = await getCanonicalAllowedSkillPatchRoots(config); + return applyParsedPatchesWithAllowedRoots(parsedPatches, allowedRoots); +} + +/** + * Applies parsed unified diff patches against any caller-supplied set of + * allowed root directories. This is the kind-agnostic core used by both the + * skill patch flow and the memory patch flow. + * + * The patch headers must reference absolute paths inside one of the allowed + * roots (after canonical resolution). Update patches must reference an + * existing target; creation patches (`/dev/null` source) must reference a path + * that does not yet exist. + * + * Returns the per-target before/after content so callers can stage commits + * and roll back on failure. + */ +export async function applyParsedPatchesWithAllowedRoots( + parsedPatches: StructuredPatch[], + allowedRoots: string[], ): Promise { const results = new Map(); const patchedContentByTarget = new Map(); @@ -260,9 +281,9 @@ export async function applyParsedSkillPatches( for (const [index, patch] of parsedPatches.entries()) { const { targetPath, isNewFile } = validatedHeaders.patches[index]; - const resolvedTargetPath = await resolveAllowedSkillPatchTarget( + const resolvedTargetPath = await resolveTargetWithinAllowedRoots( targetPath, - config, + allowedRoots, ); if (!resolvedTargetPath) { return { @@ -337,3 +358,46 @@ export async function applyParsedSkillPatches( results: Array.from(results.values()), }; } + +/** + * Canonicalizes a caller-supplied allowed root list once so callers can pass + * raw `Storage` paths without each call doing realpath traversal. + */ +export async function canonicalizeAllowedPatchRoots( + roots: string[], +): Promise { + const canonicalRoots = await Promise.all( + roots.map((root) => resolvePathWithExistingAncestors(root)), + ); + return Array.from( + new Set( + canonicalRoots.filter((root): root is string => typeof root === 'string'), + ), + ); +} + +/** + * Returns the canonical target path if it falls inside (or exactly equals) + * one of the supplied allowed roots, otherwise `undefined`. Allowed roots may + * be either directories (subtree allowlist) or single file paths + * (single-file allowlist) — `isSubpath(file, file)` returns true for the + * same-path case. + * + * Exported so that `listInboxMemoryPatches` can pre-filter patches whose + * headers escape the kind's allowed root, instead of surfacing them in the + * UI just to fail at Apply time. + */ +export async function resolveTargetWithinAllowedRoots( + targetPath: string, + allowedRoots: string[], +): Promise { + const canonicalTargetPath = + await resolvePathWithExistingAncestors(targetPath); + if (!canonicalTargetPath) { + return undefined; + } + if (allowedRoots.some((root) => isSubpath(root, canonicalTargetPath))) { + return canonicalTargetPath; + } + return undefined; +} diff --git a/packages/core/src/services/memoryService.test.ts b/packages/core/src/services/memoryService.test.ts index 86a7885295..e0fcaf9803 100644 --- a/packages/core/src/services/memoryService.test.ts +++ b/packages/core/src/services/memoryService.test.ts @@ -74,6 +74,7 @@ vi.mock('../agents/registry.js', () => ({ vi.mock('../config/storage.js', () => ({ Storage: { getUserSkillsDir: vi.fn().mockReturnValue('/tmp/fake-user-skills'), + getGlobalGeminiDir: vi.fn().mockReturnValue('/tmp/fake-global-gemini'), }, })); @@ -566,6 +567,109 @@ describe('memoryService', () => { ); }); + it('records inbox patches as memoryCandidatesCreated without applying them', async () => { + const { startMemoryService, readExtractionState } = 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-inbox-only'); + const skillsDir = path.join(tmpDir, 'skills-inbox-only'); + const projectTempDir = path.join(tmpDir, 'temp-inbox-only'); + const chatsDir = path.join(projectTempDir, 'chats'); + await fs.mkdir(memoryDir, { recursive: true }); + await fs.mkdir(skillsDir, { recursive: true }); + await fs.mkdir(chatsDir, { recursive: true }); + + const conversation = createConversation({ + sessionId: 'inbox-only-session', + messageCount: 20, + }); + await fs.writeFile( + path.join(chatsDir, 'session-2025-01-01T00-00-inbox001.json'), + JSON.stringify(conversation), + ); + + vi.mocked(LocalAgentExecutor.create).mockResolvedValueOnce({ + run: vi.fn().mockImplementation(async () => { + const inboxDir = path.join(memoryDir, '.inbox'); + await fs.mkdir(path.join(inboxDir, 'private'), { recursive: true }); + await fs.mkdir(path.join(inboxDir, 'global'), { recursive: true }); + await fs.writeFile( + path.join(inboxDir, 'private', 'MEMORY.patch'), + [ + `--- /dev/null`, + `+++ ${path.join(memoryDir, 'MEMORY.md')}`, + `@@ -0,0 +1,1 @@`, + `+- new project fact`, + ``, + ].join('\n'), + ); + await fs.writeFile( + path.join(inboxDir, 'global', 'reply-style.patch'), + [ + `--- /dev/null`, + `+++ /workspace/global/GEMINI.md`, + `@@ -0,0 +1,1 @@`, + `+Prefer concise architecture summaries.`, + ``, + ].join('\n'), + ); + 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); + + // No patch was applied — active files do not exist. + await expect( + fs.access(path.join(memoryDir, 'MEMORY.md')), + ).rejects.toThrow(); + + // Both patches remain in inbox awaiting review. + for (const relativePath of [ + path.join('.inbox', 'private', 'MEMORY.patch'), + path.join('.inbox', 'global', 'reply-style.patch'), + ]) { + await expect( + fs.access(path.join(memoryDir, relativePath)), + ).resolves.toBeUndefined(); + } + + const state = await readExtractionState( + path.join(memoryDir, '.extraction-state.json'), + ); + expect(state.runs.at(-1)?.memoryFilesUpdated ?? []).toEqual([]); + expect(state.runs.at(-1)?.memoryCandidatesCreated ?? []).toEqual( + expect.arrayContaining([ + path.join('.inbox', 'private', 'MEMORY.patch'), + path.join('.inbox', 'global', 'reply-style.patch'), + ]), + ); + }); + it('records only sessions whose read_file completed successfully as processed', async () => { const { startMemoryService, readExtractionState } = await import( './memoryService.js' diff --git a/packages/core/src/services/memoryService.ts b/packages/core/src/services/memoryService.ts index 5ea27ac38e..edc4539412 100644 --- a/packages/core/src/services/memoryService.ts +++ b/packages/core/src/services/memoryService.ts @@ -6,7 +6,7 @@ import * as fs from 'node:fs/promises'; import * as path from 'node:path'; -import { constants as fsConstants } from 'node:fs'; +import { constants as fsConstants, type Dirent } from 'node:fs'; import { randomUUID } from 'node:crypto'; import * as Diff from 'diff'; import type { Config } from '../config/config.js'; @@ -45,6 +45,11 @@ import { sanitizeWorkflowSummaryForScratchpad } from './sessionScratchpadUtils.j 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) +// Throttle: skip background extraction if the most recent run finished less +// than this long ago. Pairs with the advisory lock — the lock prevents +// concurrent runs; this throttle prevents back-to-back runs across short +// CLI sessions on workspaces with a lot of session history. +const MIN_EXTRACTION_INTERVAL_MS = 30 * 60 * 1000; // 30 minutes const MIN_USER_MESSAGES = 10; const MIN_IDLE_MS = 3 * 60 * 60 * 1000; // 3 hours const MAX_SESSION_INDEX_SIZE = 50; @@ -78,6 +83,8 @@ export interface ExtractionRun { sessionIds: string[]; candidateSessions?: SessionVersion[]; processedSessions?: SessionVersion[]; + memoryCandidatesCreated?: string[]; + memoryFilesUpdated?: string[]; skillsCreated: string[]; turnCount?: number; durationMs?: number; @@ -163,6 +170,8 @@ function isExtractionRunLike(value: unknown): value is { sessionIds?: unknown; candidateSessions?: unknown; processedSessions?: unknown; + memoryCandidatesCreated?: unknown; + memoryFilesUpdated?: unknown; skillsCreated: unknown; turnCount?: unknown; durationMs?: unknown; @@ -194,22 +203,44 @@ function buildExtractionRun(value: unknown): ExtractionRun | null { const candidateSessions = normalizeSessionVersions(value.candidateSessions); const processedSessions = normalizeSessionVersions(value.processedSessions); const sessionIds = normalizeStringArray(value.sessionIds); - - return { + const run: ExtractionRun = { runAt: value.runAt, sessionIds: sessionIds.length > 0 ? sessionIds : processedSessions.map((session) => session.sessionId), - candidateSessions: - candidateSessions.length > 0 ? candidateSessions : undefined, - processedSessions: - processedSessions.length > 0 ? processedSessions : undefined, skillsCreated: normalizeStringArray(value.skillsCreated), - turnCount: normalizeOptionalNumber(value.turnCount), - durationMs: normalizeOptionalNumber(value.durationMs), - terminateReason: normalizeOptionalString(value.terminateReason), }; + + if (candidateSessions.length > 0) { + run.candidateSessions = candidateSessions; + } + if (processedSessions.length > 0) { + run.processedSessions = processedSessions; + } + if ('memoryCandidatesCreated' in value) { + run.memoryCandidatesCreated = normalizeStringArray( + value.memoryCandidatesCreated, + ); + } + if ('memoryFilesUpdated' in value) { + run.memoryFilesUpdated = normalizeStringArray(value.memoryFilesUpdated); + } + + const turnCount = normalizeOptionalNumber(value.turnCount); + if (turnCount !== undefined) { + run.turnCount = turnCount; + } + const durationMs = normalizeOptionalNumber(value.durationMs); + if (durationMs !== undefined) { + run.durationMs = durationMs; + } + const terminateReason = normalizeOptionalString(value.terminateReason); + if (terminateReason !== undefined) { + run.terminateReason = terminateReason; + } + + return run; } function getTimestampMs(timestamp: string): number { @@ -897,6 +928,164 @@ export async function validatePatches( return validPatches; } +type FileSnapshot = Map; + +async function snapshotFiles( + rootDir: string, + shouldIncludeFile: (relativePath: string) => boolean = () => true, + shouldDescendDirectory: (relativePath: string) => boolean = () => true, +): Promise { + const snapshot: FileSnapshot = new Map(); + + async function walk(currentDir: string): Promise { + let entries: Array>; + try { + entries = await fs.readdir(currentDir, { withFileTypes: true }); + } catch { + return; + } + + for (const entry of entries) { + const absolutePath = path.join(currentDir, entry.name); + const relativePath = path.relative(rootDir, absolutePath); + if (!relativePath) { + continue; + } + + if (entry.isDirectory()) { + if (shouldDescendDirectory(relativePath)) { + await walk(absolutePath); + } + continue; + } + + if (!entry.isFile() || !shouldIncludeFile(relativePath)) { + continue; + } + + try { + snapshot.set(relativePath, await fs.readFile(absolutePath, 'utf-8')); + } catch { + // Best-effort snapshot: ignore files that disappear or are unreadable. + } + } + } + + await walk(rootDir); + return snapshot; +} + +async function snapshotInboxCandidates( + memoryDir: string, +): Promise { + return snapshotFiles(path.join(memoryDir, '.inbox')); +} + +/** + * Builds a human-readable summary of the current memory inbox state, grouped + * by kind and showing the contents of each `.patch` file. Used as part of the + * extraction agent's initial context so the agent can extend existing + * canonical patches in-place rather than creating new files each session. + * + * Returns an empty string if the inbox is empty. + */ +async function buildPendingInboxSummary(memoryDir: string): Promise { + const sections: string[] = []; + for (const kind of ['private', 'global'] as const) { + const kindRoot = path.join(memoryDir, '.inbox', kind); + let entries: Array>; + try { + entries = await fs.readdir(kindRoot, { withFileTypes: true }); + } catch { + continue; + } + + const patchFiles = entries + .filter((e) => e.isFile() && e.name.endsWith('.patch')) + .map((e) => e.name) + .sort(); + + if (patchFiles.length === 0) { + continue; + } + + const filesSection: string[] = [`## ${kind} (${patchFiles.length})`]; + for (const fileName of patchFiles) { + const fullPath = path.join(kindRoot, fileName); + let content = ''; + try { + content = await fs.readFile(fullPath, 'utf-8'); + } catch { + continue; + } + // Guard against indirect prompt injection: patch contents originate + // from past sessions (which may include user-pasted text), so a + // crafted payload could include a closing ``` fence to break out of + // the surrounding markdown block. Pick a fence longer than the + // longest backtick-run actually present in the content so the close + // is guaranteed to terminate the block. + const longestBacktickRun = (content.match(/`+/g) ?? []).reduce( + (max, run) => Math.max(max, run.length), + 2, // never go below the standard 3-backtick fence + ); + const fence = '`'.repeat(longestBacktickRun + 1); + filesSection.push(''); + filesSection.push(`### ${fileName}`); + filesSection.push(fence); + filesSection.push(content.trimEnd()); + filesSection.push(fence); + } + sections.push(filesSection.join('\n')); + } + return sections.join('\n\n'); +} + +interface FileSnapshotDiff { + added: string[]; + updated: string[]; + deleted: string[]; +} + +function diffFileSnapshots( + before: FileSnapshot, + after: FileSnapshot, +): FileSnapshotDiff { + const added: string[] = []; + const updated: string[] = []; + const deleted: string[] = []; + + for (const [relativePath, content] of after) { + if (!before.has(relativePath)) { + added.push(relativePath); + } else if (before.get(relativePath) !== content) { + updated.push(relativePath); + } + } + + for (const relativePath of before.keys()) { + if (!after.has(relativePath)) { + deleted.push(relativePath); + } + } + + return { + added: added.sort(), + updated: updated.sort(), + deleted: deleted.sort(), + }; +} + +function getChangedSnapshotPaths(diff: FileSnapshotDiff): string[] { + return [...diff.added, ...diff.updated].sort(); +} + +function prefixRelativePaths( + prefix: string, + relativePaths: string[], +): string[] { + return relativePaths.map((relativePath) => path.join(prefix, relativePath)); +} + /** * Main entry point for the skill extraction background task. * Designed to be called fire-and-forget on session startup. @@ -947,6 +1136,24 @@ export async function startMemoryService(config: Config): Promise { `[MemoryService] State loaded: ${previousRuns} previous run(s), ${previouslyProcessed} session(s) already processed`, ); + // Throttle: short-circuit if the most recent run finished less than + // MIN_EXTRACTION_INTERVAL_MS ago. Avoids re-scanning session history on + // every CLI start when the user opens several short sessions in a row. + const lastRun = state.runs.at(-1); + if (lastRun?.runAt) { + const lastRunMs = Date.parse(lastRun.runAt); + if ( + Number.isFinite(lastRunMs) && + Date.now() - lastRunMs < MIN_EXTRACTION_INTERVAL_MS + ) { + const minutesAgo = Math.round((Date.now() - lastRunMs) / 60000); + debugLogger.log( + `[MemoryService] Skipped: last run was ${minutesAgo} minute(s) ago (min interval ${MIN_EXTRACTION_INTERVAL_MS / 60000}m)`, + ); + return; + } + } + // Build session index: all eligible sessions with summaries + file paths. // The agent decides which to read in full via read_file. const { sessionIndex, newSessionIds, candidateSessions } = @@ -988,6 +1195,8 @@ export async function startMemoryService(config: Config): Promise { `[MemoryService] ${skillsBefore.size} existing skill(s) in memory`, ); + const inboxCandidatesBefore = await snapshotInboxCandidates(memoryDir); + // Read existing skills for context (memory-extracted + global/workspace) const existingSkillsSummary = await buildExistingSkillsSummary( skillsDir, @@ -999,11 +1208,23 @@ export async function startMemoryService(config: Config): Promise { ); } + // Surface the current inbox state to the agent so it can rewrite + // existing canonical patches in place instead of accumulating new ones + // across sessions. + const pendingInboxSummary = await buildPendingInboxSummary(memoryDir); + if (pendingInboxSummary) { + debugLogger.log( + `[MemoryService] Pending inbox surfaced to agent:\n${pendingInboxSummary}`, + ); + } + // Build agent definition and context const agentDefinition = SkillExtractionAgent( skillsDir, sessionIndex, existingSkillsSummary, + memoryDir, + pendingInboxSummary, ); const context = buildAgentLoopContext(config); @@ -1109,6 +1330,18 @@ export async function startMemoryService(config: Config): Promise { ); } + // Anything still in .inbox/ is reviewable; nothing is auto-applied. + const memoryFilesUpdated: string[] = []; + const memoryCandidatesCreated = prefixRelativePaths( + '.inbox', + getChangedSnapshotPaths( + diffFileSnapshots( + inboxCandidatesBefore, + await snapshotInboxCandidates(memoryDir), + ), + ), + ); + const processedSessions = candidateSessions .filter((session) => processedSessionKeys.has(getSessionVersionKey(session)), @@ -1127,6 +1360,8 @@ export async function startMemoryService(config: Config): Promise { lastUpdated: session.lastUpdated, })), processedSessions, + memoryCandidatesCreated, + memoryFilesUpdated, skillsCreated, turnCount: normalizeOptionalNumber(executorResult?.turn_count), durationMs: normalizeOptionalNumber(executorResult?.duration_ms), @@ -1139,8 +1374,17 @@ export async function startMemoryService(config: Config): Promise { }; await writeExtractionState(statePath, updatedState); - if (skillsCreated.length > 0 || patchesCreatedThisRun.length > 0) { + if ( + skillsCreated.length > 0 || + patchesCreatedThisRun.length > 0 || + memoryCandidatesCreated.length > 0 + ) { const completionParts: string[] = []; + if (memoryCandidatesCreated.length > 0) { + completionParts.push( + `prepared ${memoryCandidatesCreated.length} memory candidate(s): ${memoryCandidatesCreated.join(', ')}`, + ); + } if (skillsCreated.length > 0) { completionParts.push( `created ${skillsCreated.length} skill(s): ${skillsCreated.join(', ')}`, @@ -1155,6 +1399,11 @@ export async function startMemoryService(config: Config): Promise { `[MemoryService] Completed in ${elapsed}s. ${completionParts.join('; ')} (read ${processedSessions.length}/${candidateSessions.length} surfaced session(s))`, ); const feedbackParts: string[] = []; + if (memoryCandidatesCreated.length > 0) { + feedbackParts.push( + `${memoryCandidatesCreated.length} memory candidate${memoryCandidatesCreated.length > 1 ? 's' : ''} extracted from past sessions`, + ); + } if (skillsCreated.length > 0) { feedbackParts.push( `${skillsCreated.length} new skill${skillsCreated.length > 1 ? 's' : ''} extracted from past sessions: ${skillsCreated.join(', ')}`, diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index 1ec77c7697..c4d33a7414 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -3300,8 +3300,8 @@ }, "autoMemory": { "title": "Auto Memory", - "description": "Automatically extract reusable skills from past sessions in the background. Review results with /memory inbox.", - "markdownDescription": "Automatically extract reusable skills from past sessions in the background. Review results with /memory inbox.\n\n- Category: `Experimental`\n- Requires restart: `yes`\n- Default: `false`", + "description": "Automatically extract memory patches and skills from past sessions in the background. Every change is written as a unified diff `.patch` file under `/.inbox//` and held for review in /memory inbox; nothing is applied until you approve it.", + "markdownDescription": "Automatically extract memory patches and skills from past sessions in the background. Every change is written as a unified diff `.patch` file under `/.inbox//` and held for review in /memory inbox; nothing is applied until you approve it.\n\n- Category: `Experimental`\n- Requires restart: `yes`\n- Default: `false`", "default": false, "type": "boolean" }, diff --git a/scripts/check-inbox.js b/scripts/check-inbox.js new file mode 100644 index 0000000000..ef2cdd0455 --- /dev/null +++ b/scripts/check-inbox.js @@ -0,0 +1,60 @@ +#!/usr/bin/env node + +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Diagnostic: instantiate the real Config and call the same listing functions + * the inbox UI uses. Should print out all skills + skill patches + memory + * patches the user would see in `/memory inbox`. + */ +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = path.resolve(SCRIPT_DIR, '..'); +const corePath = path.join(REPO_ROOT, 'packages/core/dist/src/index.js'); + +const { Storage, listInboxSkills, listInboxPatches, listInboxMemoryPatches } = + await import(corePath); + +const cwd = process.cwd(); +const storage = new Storage(cwd); +await storage.initialize(); + +const config = { + storage, + isTrustedFolder: () => true, + getProjectRoot: () => cwd, +}; + +const [skills, skillPatches, memoryPatches] = await Promise.all([ + listInboxSkills(config), + listInboxPatches(config), + listInboxMemoryPatches(config), +]); + +console.log(`\nInbox content for ${cwd}\n`); + +console.log(`Skills (${skills.length}):`); +for (const s of skills) { + console.log(` - ${s.name} (${s.dirName})`); +} + +console.log(`\nSkill update patches (${skillPatches.length}):`); +for (const p of skillPatches) { + console.log(` - ${p.name} → ${p.entries.length} entry/entries`); +} + +console.log(`\nMemory patches (${memoryPatches.length}):`); +for (const m of memoryPatches) { + console.log( + ` - [${m.kind}] ${m.relativePath} → ${m.entries.length} entry/entries`, + ); + for (const e of m.entries) { + console.log(` ${e.isNewFile ? 'CREATE' : 'UPDATE'} ${e.targetPath}`); + } +} diff --git a/scripts/seed-test-inbox.js b/scripts/seed-test-inbox.js new file mode 100644 index 0000000000..f3c735e1b9 --- /dev/null +++ b/scripts/seed-test-inbox.js @@ -0,0 +1,226 @@ +#!/usr/bin/env node + +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Seeds the auto-memory inbox with REALISTIC patches for manual end-to-end + * testing of `/memory inbox`. Mirrors what one extraction-agent run would + * produce in practice: a single canonical `extraction.patch` per kind, + * containing multiple hunks (MEMORY.md update + sibling creation, etc.). + * + * Run AFTER `npm run build` from the project root: + * node scripts/seed-test-inbox.js + * + * The script will: + * 1. Initialize Storage for the current working directory. + * 2. Compute = ~/.gemini/tmp//memory/. + * 3. Seed `MEMORY.md` and TWO canonical inbox patches: + * - .inbox/private/extraction.patch (multi-hunk: update MEMORY.md + * + create verify-workflow.md + add MEMORY.md pointer to it) + * - .inbox/global/extraction.patch (creates ~/.gemini/GEMINI.md) + * 4. Print a verification checklist + the launch command. + * + * To clean up later, delete `/.inbox/` and the seeded + * MEMORY.md / GEMINI.md files. + */ + +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { fileURLToPath } from 'node:url'; + +const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = path.resolve(SCRIPT_DIR, '..'); + +const corePath = path.join(REPO_ROOT, 'packages/core/dist/src/index.js'); +try { + await fs.access(corePath); +} catch { + console.error( + `Cannot find built core at ${corePath}. Run \`npm run build\` first.`, + ); + process.exit(1); +} + +const { Storage } = await import(corePath); + +const cwd = process.cwd(); +const storage = new Storage(cwd); +await storage.initialize(); + +const memoryDir = storage.getProjectMemoryTempDir(); +const inboxPrivate = path.join(memoryDir, '.inbox', 'private'); +const inboxGlobal = path.join(memoryDir, '.inbox', 'global'); +const homeDir = os.homedir(); +const globalGeminiMd = path.join(homeDir, '.gemini', 'GEMINI.md'); + +console.log(`\n🔧 Seeding inbox for cwd: ${cwd}`); +console.log(` memoryDir = ${memoryDir}\n`); + +await fs.mkdir(inboxPrivate, { recursive: true }); +await fs.mkdir(inboxGlobal, { recursive: true }); + +const seeded = []; +async function seed(filePath, content, label) { + await fs.mkdir(path.dirname(filePath), { recursive: true }); + await fs.writeFile(filePath, content, 'utf-8'); + seeded.push({ filePath, label }); +} + +// --- 1. Pre-existing private MEMORY.md so the update hunk has something to modify --- +const memoryMd = path.join(memoryDir, 'MEMORY.md'); +await seed( + memoryMd, + '# Project Memory\n\n- old fact about this project\n', + 'pre-existing active MEMORY.md', +); + +// --- 2. Canonical PRIVATE extraction.patch --- +// One file, multi-hunk: update MEMORY.md AND create verify-workflow.md +// AND add a pointer line for the sibling. This is what one extraction +// agent run typically produces. +const verifyWorkflowMd = path.join(memoryDir, 'verify-workflow.md'); +await fs.rm(verifyWorkflowMd, { force: true }); +await seed( + path.join(inboxPrivate, 'extraction.patch'), + [ + // Hunk 1: replace the existing fact and append a sibling pointer. + `--- ${memoryMd}`, + `+++ ${memoryMd}`, + `@@ -1,3 +1,4 @@`, + ` # Project Memory`, + ` `, + `-- old fact about this project`, + `+- new fact extracted from session analysis`, + `+- See ${verifyWorkflowMd} for the project's verification commands.`, + // Hunk 2: create the verify-workflow.md sibling. + `--- /dev/null`, + `+++ ${verifyWorkflowMd}`, + `@@ -0,0 +1,5 @@`, + `+# Verify Workflow`, + `+`, + `+- Run \`npm run typecheck\` after editing any *.ts file.`, + `+- Run \`npm run build --workspace @google/gemini-cli-core\` before testing CLI changes.`, + `+- Inbox patches are guarded by /memory inbox.`, + ``, + ].join('\n'), + 'canonical PRIVATE extraction.patch (2 hunks: MEMORY.md update + sibling create)', +); + +// --- 3. Canonical GLOBAL extraction.patch --- +// Creates ~/.gemini/GEMINI.md. Backs up any existing one first. +let existingGlobalGemini = null; +try { + existingGlobalGemini = await fs.readFile(globalGeminiMd, 'utf-8'); +} catch { + // Doesn't exist yet — fine. +} +if (existingGlobalGemini !== null) { + const backupPath = `${globalGeminiMd}.seed-test-backup-${Date.now()}`; + await fs.copyFile(globalGeminiMd, backupPath); + console.log( + ` ℹ️ Backed up existing ${globalGeminiMd} → ${backupPath}\n` + + ` (restore manually after testing if you wish.)\n`, + ); + await fs.rm(globalGeminiMd, { force: true }); +} +await seed( + path.join(inboxGlobal, 'extraction.patch'), + [ + `--- /dev/null`, + `+++ ${globalGeminiMd}`, + `@@ -0,0 +1,3 @@`, + `+# Global Personal Preferences`, + `+`, + `+- Prefer concise architecture summaries.`, + ``, + ].join('\n'), + 'canonical GLOBAL extraction.patch (creates ~/.gemini/GEMINI.md)', +); + +// --- Summary --- +console.log('Seeded files:'); +for (const { filePath, label } of seeded) { + console.log(` ✓ ${path.relative(cwd, filePath)}`); + console.log(` ${label}\n`); +} + +console.log('━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━'); +console.log('NEXT STEPS'); +console.log('━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━'); +console.log(` +1. Enable autoMemory in your settings (the inbox command requires it): + + ~/.gemini/settings.json should contain: + { + "experimental": { "autoMemory": true } + } + + Or run this to set it: + node -e "const fs=require('fs'),p=require('os').homedir()+'/.gemini/settings.json';let s={};try{s=JSON.parse(fs.readFileSync(p,'utf-8'))}catch{}s.experimental=s.experimental||{};s.experimental.autoMemory=true;fs.mkdirSync(require('path').dirname(p),{recursive:true});fs.writeFileSync(p,JSON.stringify(s,null,2))" + +2. Launch the just-built CLI from THIS REPO ONLY. Do NOT use any globally + installed "gemini" binary — it will be a stale build that doesn't know + about memory patches and will silently show only skills. + + npm run start + + (or, equivalently: node ${path.relative(cwd, REPO_ROOT)}/bundle/gemini.js) + + Sanity check before launching: + node ${path.relative(cwd, path.join(REPO_ROOT, 'scripts/check-inbox.js'))} + should report 2 memory patches (Private memory + Global memory). + +3. In the CLI, run: + + /memory inbox + + You should see exactly 2 entries in the "Memory Updates" group: + - Private memory 2 hunks from 1 source patch + - Global memory 1 hunk from 1 source patch + +4. Test focus preservation: arrow-down to "Global memory" → Enter → Esc → + cursor MUST still be on "Global memory" (not row 0). + +5. Open "Private memory" preview. You'll see TWO target sections (no + duplicates), since both hunks come from one source patch: + + ${memoryMd} + - new fact extracted from session analysis + - See ${verifyWorkflowMd} for the project's verification commands. + + ${verifyWorkflowMd} (new file) + # Verify Workflow + ... + +6. Apply each entry: + + ┌──────────────────┬──────────┬───────────────────────────────────────┐ + │ Item │ Action │ Expected outcome │ + ├──────────────────┼──────────┼───────────────────────────────────────┤ + │ Private memory │ Apply │ "Applied all 1 private memory patch." │ + │ │ │ MEMORY.md updated; verify-workflow.md │ + │ │ │ created. │ + │ Global memory │ Apply │ "Applied all 1 global memory patch." │ + │ │ │ ~/.gemini/GEMINI.md created. │ + └──────────────────┴──────────┴───────────────────────────────────────┘ + +7. Verify final state on disk: + + cat ${path.relative(cwd, memoryMd)} # should show new fact + pointer line + cat ${path.relative(cwd, verifyWorkflowMd)} # should exist + cat ${globalGeminiMd} # should show "Prefer concise..." + ls ${path.relative(cwd, inboxPrivate)} # should be empty + ls ${path.relative(cwd, inboxGlobal)} # should be empty + +8. Cleanup: + + rm -rf ${path.relative(cwd, path.join(memoryDir, '.inbox'))} + rm -f ${path.relative(cwd, memoryMd)} + rm -f ${path.relative(cwd, verifyWorkflowMd)} + rm -f ${globalGeminiMd} +`);