feat(memory): implement persistent session logging and background memory consolidation#24892
feat(memory): implement persistent session logging and background memory consolidation#24892hackerxj2010 wants to merge 4 commits intogoogle-gemini:mainfrom
Conversation
This commit implements the foundation for a persistent memory system similar to Claude Code's memory feature: - Add memory type taxonomy (user, feedback, project, reference) - Implement MEMORY.md file discovery and loading - Build session logging system with daily log files - Create memory consolidation background process - Integrate memory context into system prompt Memory files are stored in ~/.gemini/projects/<project-slug>/memory/ and include: - MEMORY.md: Index file pointing to memory files - Individual memory files with frontmatter (name, description, type) The consolidation worker runs automatically after sessions to keep memories accurate and up-to-date. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a persistent, hierarchical memory system for the Gemini CLI. By logging session activity and automatically consolidating insights into project-specific documentation, the system enables the agent to maintain long-term context across sessions. Additionally, a new background daemon service has been added to support proactive project monitoring and automated tasks, significantly enhancing the agent's ability to stay informed about project state and user preferences. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a persistent memory system and a background daemon service for proactive project management. The memory system features daily session logging and an LLM-driven consolidation process that merges insights into GEMINI.md files using background worker threads. The daemon service monitors project files to detect TODOs, failing tests, and outdated dependencies, providing desktop notifications. Review feedback identifies critical command injection vulnerabilities in the notification service across macOS, Linux, and Windows. Additionally, improvements are suggested for daemon status tracking and the robust resolution of worker script paths and model versions.
Note: Security Review did not run due to the size of the PR.
| private async notifyMacOS(options: NotificationOptions): Promise<boolean> { | ||
| const { title, body, sound = true } = options; | ||
|
|
||
| // Escape quotes in the title and body | ||
| const escapedTitle = title.replace(/"/g, '\\"'); | ||
| const escapedBody = body.replace(/"/g, '\\"'); | ||
|
|
||
| // Use osascript for macOS notifications | ||
| const script = sound | ||
| ? `display notification "${escapedBody}" with title "${escapedTitle}" sound name "default"` | ||
| : `display notification "${escapedBody}" with title "${escapedTitle}"`; | ||
|
|
||
| try { | ||
| const { exec } = await import('node:child_process'); | ||
| return await new Promise((resolve) => { | ||
| exec(`osascript -e '${script}'`, (error) => { | ||
| if (error) { | ||
| debugLogger.error('[Notifier] macOS notification failed:', error); | ||
| resolve(false); | ||
| } else { | ||
| resolve(true); | ||
| } | ||
| }); | ||
| }); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
The notifyMacOS method is vulnerable to command injection. The title and body strings are interpolated into a shell command string after only escaping double quotes. Since the entire script is wrapped in single quotes for the exec call, an attacker can break out of the single-quoted string by providing a title containing a single quote (e.g., '; touch /tmp/pwned; ').
Additionally, using child_process.exec is generally discouraged for executing commands with user-provided input as it spawns a shell. It is much safer to use child_process.execFile which accepts arguments as an array and does not invoke a shell.
| return await new Promise((resolve) => { | ||
| exec(`notify-send ${args.map(a => `"${a}"`).join(' ')}`, (error) => { | ||
| if (error) { | ||
| debugLogger.error('[Notifier] Linux notification failed:', error); | ||
| resolve(false); | ||
| } else { | ||
| resolve(true); | ||
| } | ||
| }); | ||
| }); | ||
| } catch { |
There was a problem hiding this comment.
The notifyLinux method is vulnerable to command injection because it joins arguments into a single string for child_process.exec. Shell metacharacters in the title or body (e.g., backticks or $(...)) will be evaluated by the system shell.
Refactor this to use execFile for safe argument passing.
const { execFile } = await import('node:child_process');
return await new Promise((resolve) => {
execFile('notify-send', args, (error) => {
if (error) {
debugLogger.error('[Notifier] Linux notification failed:', error);
resolve(false);
} else {
resolve(true);
}
});
});| private async notifyWindows(options: NotificationOptions): Promise<boolean> { | ||
| const { title, body } = options; | ||
|
|
||
| // Escape for PowerShell | ||
| const escapedTitle = title.replace(/'/g, "''"); | ||
| const escapedBody = body.replace(/'/g, "''"); | ||
|
|
||
| // Use PowerShell with Windows native toast notification | ||
| const psScript = ` | ||
| [Windows.UI.Notifications.ToastNotificationManager, Windows.UI.Notifications, ContentType = WindowsRuntime] | Out-Null | ||
| [Windows.Data.Xml.Dom.XmlDocument, Windows.Data.Xml.Dom.XmlDocument, ContentType = WindowsRuntime] | Out-Null | ||
|
|
||
| $template = @" | ||
| <toast> | ||
| <visual> | ||
| <binding template="ToastText02"> | ||
| <text id="1">${escapedTitle}</text> | ||
| <text id="2">${escapedBody}</text> | ||
| </binding> | ||
| </visual> | ||
| </toast> | ||
| "@ | ||
|
|
||
| $xml = New-Object Windows.Data.Xml.Dom.XmlDocument | ||
| $xml.LoadXml($template) | ||
| $toast = [Windows.UI.Notifications.ToastNotification]::new($xml) | ||
| [Windows.UI.Notifications.ToastNotificationManager]::CreateToastNotifier("Gemini CLI").Show($toast) | ||
| `; | ||
|
|
||
| try { | ||
| const { exec } = await import('node:child_process'); | ||
| return await new Promise((resolve) => { | ||
| exec(`powershell -Command "${psScript.replace(/\n/g, ' ')}"`, (error) => { | ||
| if (error) { | ||
| // Fallback to a simpler method | ||
| this.notifyWindowsFallback(title, body).then(resolve); | ||
| } else { | ||
| resolve(true); | ||
| } | ||
| }); | ||
| }); | ||
| } catch { | ||
| return this.notifyWindowsFallback(title, body); | ||
| } | ||
| } |
There was a problem hiding this comment.
The notifyWindows method is vulnerable to command injection. The psScript is interpolated into a powershell -Command "..." string. If the title or body contains a double quote, it can break out of the PowerShell command string and execute arbitrary shell commands.
Use execFile to invoke powershell.exe and pass the script via the -EncodedCommand flag or as a direct argument without wrapping it in a shell-interpreted string.
| export async function getDaemonStatus(config: Config): Promise<DaemonStatus> { | ||
| // If we have a running instance, return its status | ||
| if (daemonInstance) { | ||
| return daemonInstance.getStatus(); | ||
| } | ||
|
|
||
| // Otherwise, check if there's a daemon PID file | ||
| const tempInstance = new DaemonService({ | ||
| projectRoot: config.getProjectRoot(), | ||
| configDir: config.storage.getProjectTempDir(), | ||
| }); | ||
|
|
||
| // The status will reflect whether a daemon is running | ||
| return { | ||
| state: 'stopped', | ||
| pid: null, | ||
| uptime: null, | ||
| lastTick: null, | ||
| actionCount: 0, | ||
| watchCount: 0, | ||
| }; | ||
| } |
There was a problem hiding this comment.
The getDaemonStatus function incorrectly reports the daemon as 'stopped' if the local daemonInstance singleton is null. Instead of relying on the existence of the instance object, use an explicit state variable to track the daemon's lifecycle. When checking if the background process is alive, ensure the implementation follows repository guidelines for Windows by avoiding high-overhead checks and potentially assuming the process is alive to prevent performance degradation.
References
- When managing the state of asynchronous operations, rely on an explicit state variable (e.g., a state enum) rather than checking for the existence of a promise object.
- When checking if a process is alive on Windows, avoid solutions that introduce significant performance overhead, even if it means making assumptions (e.g., always returning true) to prevent performance degradation.
| logDir: config.storage.getSessionLogDir(), | ||
| geminiMdPath: path.join(config.getProjectRoot(), 'GEMINI.md'), | ||
| apiKey, | ||
| model: 'gemini-2.5-flash', |
There was a problem hiding this comment.
The model name gemini-2.5-flash appears to be a typo or refers to a non-existent version (the current latest is gemini-2.0-flash). Hardcoding this specific version string will cause the consolidation worker to fail if the model is not available in the Gemini API. It is better to use config.getActiveModel() or a constant like DEFAULT_GEMINI_FLASH_MODEL from the core configuration.
| model: 'gemini-2.5-flash', | |
| model: config.getActiveModel(), |
| try { | ||
| // Use import.meta.url for ESM compatibility | ||
| const { fileURLToPath } = await import('node:url'); | ||
| const currentDir = path.dirname(fileURLToPath(import.meta.url)); |
There was a problem hiding this comment.
Hardcoding the .js extension for the worker script is fragile. If the CLI is running in a development environment, the compiled .js file may not exist. Use a robust path resolution function like resolveToRealPath to handle both source and compiled files. Also, be aware that relative paths from a dist directory might require navigating up three levels (../../../) due to the project's build structure.
References
- Ensure consistent path resolution by using a single, robust function (e.g.,
resolveToRealPath) for all related path validations. - Relative paths from a
distdirectory might require navigating up three levels (../../../) due to the project's build structure.
gemini-cli
bot
added
the
status/need-issue
1 participant
Summary
Implements a persistent, hierarchical memory system for the Gemini CLI that automatically logs session activity and consolidates insights into project-level
GEMINI.mdfiles.Changes
.gemini/GEMINI.mdin trusted workspaces.GEMINI.md.MemoryContextManager,SessionSummaryUtils, andlocal-executoragent loop.Files Changed
packages/core/src/templates/geminiMdTemplate.ts(NEW)packages/core/src/services/sessionLogTypes.ts(NEW)packages/core/src/services/sessionLogger.ts(NEW)packages/core/src/services/sessionLogger.test.ts(NEW)packages/core/src/services/memoryConsolidator.ts(NEW)packages/core/src/services/memoryConsolidator.test.ts(NEW)packages/core/src/services/memoryConsolidationWorker.ts(NEW)packages/core/src/services/sessionSummaryUtils.ts(MODIFIED)packages/core/src/context/memoryContextManager.ts(MODIFIED)packages/core/src/agents/local-executor.ts(MODIFIED)packages/core/src/config/config.ts(MODIFIED)Testing
SessionLogger(all passing).MemoryConsolidator(all passing).eslintwith zero errors.