feat(opencode): native auto-memory for cross-session learning#20344
feat(opencode): native auto-memory for cross-session learning#20344lleontor705 wants to merge 5 commits intoanomalyco:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “auto-memory” subsystem to packages/opencode to persist and re-inject cross-session learnings (via a SQLite memory table and an auto-generated .opencode/MEMORY.md) and wires it into session processing/prompt construction.
Changes:
- Introduces a
memorymodule (extractor, store, MEMORY.md writer, and prompt injector) plus a DB migration for the newmemorytable. - Hooks memory injection into the session system prompt and hooks extraction/update into the session processor.
- Adds unit tests for the memory store/extractor/file behavior and removes an older memory-leak test.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/opencode/src/session/prompt.ts | Injects .opencode/MEMORY.md content into the system prompt (bounded by max_memory_lines). |
| packages/opencode/src/session/processor.ts | Adds extractor hooks for tool call/result events and updates MEMORY.md (best-effort). |
| packages/opencode/src/memory/types.ts | Defines memory types and a Memory interface. |
| packages/opencode/src/memory/store.ts | Implements CRUD/search/compact for persisted memories. |
| packages/opencode/src/memory/memory.sql.ts | Defines the Drizzle schema for the memory table and indexes. |
| packages/opencode/src/memory/memory-file.ts | Generates and reads .opencode/MEMORY.md from stored memories. |
| packages/opencode/src/memory/injector.ts | Reads/trims MEMORY.md and returns it as system prompt content. |
| packages/opencode/src/memory/extractor.ts | Implements heuristic extraction from tool calls/results and user messages. |
| packages/opencode/src/memory/index.ts | Re-exports memory module surface. |
| packages/opencode/src/config/config.ts | Adds memory config block (enabled/auto_extract/max_memory_lines). |
| packages/opencode/migration/20260331180000_add_memory_table/migration.sql | Creates the memory table and indexes. |
| packages/opencode/test/memory/store.test.ts | Adds tests for store CRUD/compact and MemoryFile read/write/update. |
| packages/opencode/test/memory/extractor.test.ts | Adds tests for all extraction patterns and false positives. |
| packages/opencode/test/memory/abort-leak.test.ts | Removes prior abort-controller leak test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Feed to memory extractor (fire-and-forget) | ||
| try { MemoryExtractor.onToolCall(value.toolName, value.input as Record<string, unknown>) } catch {} |
There was a problem hiding this comment.
Memory extraction is invoked for every tool call, but initialization is conditional on config. Because MemoryExtractor holds global state and is never reset in this flow, a previous session could leave state non-null and extraction would still run even when cfg.memory.enabled === false for the current session. Consider gating these calls on the same config check (or making the extractor state keyed by session and cleared on cleanup).
| // Feed to memory extractor (fire-and-forget) | |
| try { MemoryExtractor.onToolCall(value.toolName, value.input as Record<string, unknown>) } catch {} | |
| // Feed to memory extractor (fire-and-forget), but only if memory extraction is enabled | |
| const memoryConfig = (Config as any).memory | |
| if (!memoryConfig || memoryConfig.enabled) { | |
| try { | |
| MemoryExtractor.onToolCall(value.toolName, value.input as Record<string, unknown>) | |
| } catch {} | |
| } |
| const exitCode = (value.output.metadata as Record<string, unknown>)?.exitCode as number | undefined | ||
| MemoryExtractor.onToolResult( | ||
| match.tool, | ||
| (value.input ?? match.state.input) as Record<string, unknown>, | ||
| value.output.output, | ||
| exitCode, | ||
| ) | ||
| } catch {} |
There was a problem hiding this comment.
exitCode is read from value.output.metadata.exitCode, but the bash tool appears to emit metadata.exit (see tool/bash.ts). As a result, exitCode will be undefined and error-solution detection based on non-zero exit codes will never trigger in real sessions. Update the metadata key (and handle null vs number from proc.exitCode) so the extractor receives correct exit status.
| const exitCode = (value.output.metadata as Record<string, unknown>)?.exitCode as number | undefined | |
| MemoryExtractor.onToolResult( | |
| match.tool, | |
| (value.input ?? match.state.input) as Record<string, unknown>, | |
| value.output.output, | |
| exitCode, | |
| ) | |
| } catch {} | |
| const metadata = value.output.metadata as Record<string, unknown> | undefined | |
| const rawExit = metadata && (metadata as Record<string, unknown>)["exit"] | |
| let exitCode: number | undefined | |
| if (typeof rawExit === "number") { | |
| exitCode = rawExit | |
| } else if (rawExit === null) { | |
| exitCode = undefined | |
| } | |
| MemoryExtractor.onToolResult( | |
| match.tool, | |
| (value.input ?? match.state.input) as Record<string, unknown>, | |
| value.output.output, | |
| exitCode, | |
| ) | |
| } catch (error) { | |
| Log.error("MemoryExtractor.onToolResult failed", error) | |
| } |
| // Update MEMORY.md from extracted memories (best-effort) | ||
| try { | ||
| const cfg = yield* config.get() | ||
| if (cfg.memory?.enabled !== false && cfg.memory?.auto_extract !== false) { | ||
| yield* Effect.promise(() => MemoryFile.updateMemoryFile(Instance.directory)) | ||
| } |
There was a problem hiding this comment.
cleanup() runs after each assistant message processing attempt (via Effect.ensuring(cleanup())), so MemoryFile.updateMemoryFile(...) will execute repeatedly during a session rather than “at session end” as described. This can add avoidable IO and churn, and may rewrite MEMORY.md even when no new memories were extracted. Consider deferring the update to a session-idle/finalizer point, or tracking a “dirty” flag in the extractor/store to update only when needed.
| // Initialize memory extractor | ||
| try { | ||
| const cfg = yield* config.get() | ||
| if (cfg.memory?.enabled !== false && cfg.memory?.auto_extract !== false) { | ||
| MemoryExtractor.init(Instance.directory, ctx.sessionID) | ||
| } | ||
| } catch { | ||
| // Memory extraction is best-effort | ||
| } |
There was a problem hiding this comment.
MemoryExtractor is initialized inside process() for each assistant generation. Since the extractor is a module-level singleton, this design is not safe if multiple sessions/processors run concurrently (state will be overwritten and tool calls from different sessions can interleave). Consider making the extractor state per-session (e.g., Map keyed by sessionID) or instantiating an extractor per SessionProcessor.create() handle.
| export namespace MemoryExtractor { | ||
| let state: ExtractorState | null = null | ||
|
|
||
| export function init(projectPath: string, sessionId?: string) { | ||
| state = { | ||
| bashCommandCount: new Map(), | ||
| lastToolCalls: [], | ||
| currentTurnEdits: new Set(), | ||
| projectPath, | ||
| sessionId, | ||
| detectedTopics: new Set(), | ||
| } | ||
| } | ||
|
|
||
| export function reset() { | ||
| state = null | ||
| } | ||
|
|
||
| export function onToolCall(tool: string, input: Record<string, unknown>) { |
There was a problem hiding this comment.
MemoryExtractor is implemented as a singleton (let state: ExtractorState | null = null). Given the session processor can run multiple sessions concurrently, this shared mutable state can cause cross-session contamination and race conditions. Refactor to keep state per session (e.g., Map<SessionID, ExtractorState> with explicit sessionID passed to hooks) or return an extractor instance from init() that is stored on the processor context.
| export namespace MemoryExtractor { | |
| let state: ExtractorState | null = null | |
| export function init(projectPath: string, sessionId?: string) { | |
| state = { | |
| bashCommandCount: new Map(), | |
| lastToolCalls: [], | |
| currentTurnEdits: new Set(), | |
| projectPath, | |
| sessionId, | |
| detectedTopics: new Set(), | |
| } | |
| } | |
| export function reset() { | |
| state = null | |
| } | |
| export function onToolCall(tool: string, input: Record<string, unknown>) { | |
| const DEFAULT_SESSION_ID = "__default__" | |
| export namespace MemoryExtractor { | |
| const stateBySession = new Map<string, ExtractorState>() | |
| function getSessionId(sessionId?: string): string { | |
| return sessionId ?? DEFAULT_SESSION_ID | |
| } | |
| function getState(sessionId?: string): ExtractorState | undefined { | |
| const id = getSessionId(sessionId) | |
| return stateBySession.get(id) | |
| } | |
| function ensureState(projectPath: string, sessionId?: string): ExtractorState { | |
| const id = getSessionId(sessionId) | |
| let state = stateBySession.get(id) | |
| if (!state) { | |
| state = { | |
| bashCommandCount: new Map(), | |
| lastToolCalls: [], | |
| currentTurnEdits: new Set(), | |
| projectPath, | |
| sessionId, | |
| detectedTopics: new Set(), | |
| } | |
| stateBySession.set(id, state) | |
| } | |
| return state | |
| } | |
| export function init(projectPath: string, sessionId?: string) { | |
| ensureState(projectPath, sessionId) | |
| } | |
| export function reset(sessionId?: string) { | |
| if (sessionId) { | |
| stateBySession.delete(getSessionId(sessionId)) | |
| } else { | |
| stateBySession.clear() | |
| } | |
| } | |
| export function onToolCall(sessionId: string | undefined, tool: string, input: Record<string, unknown>) { | |
| const state = getState(sessionId) |
| CREATE TABLE `memory` ( | ||
| `id` text PRIMARY KEY NOT NULL, | ||
| `project_path` text NOT NULL, | ||
| `type` text NOT NULL, | ||
| `topic` text NOT NULL, | ||
| `content` text NOT NULL, | ||
| `session_id` text, | ||
| `access_count` integer DEFAULT 0 NOT NULL, | ||
| `time_created` integer NOT NULL, | ||
| `time_updated` integer NOT NULL | ||
| ); | ||
| --> statement-breakpoint | ||
| CREATE INDEX `memory_project_topic_idx` ON `memory` (`project_path`, `topic`); | ||
| --> statement-breakpoint | ||
| CREATE INDEX `memory_project_idx` ON `memory` (`project_path`); | ||
| --> statement-breakpoint | ||
| CREATE INDEX `memory_type_idx` ON `memory` (`type`); |
There was a problem hiding this comment.
The new memory table migration does not enforce topic deduplication (no UNIQUE constraint). If topic-level dedupe is required, add a unique index/constraint on (project_path, topic) and update the store to upsert accordingly.
| try { db.run(`DELETE FROM memory`).run() } catch {} | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| const db = Database.Client() | ||
| try { db.run(`DELETE FROM memory`).run() } catch {} |
There was a problem hiding this comment.
Database.Client().run(...) already executes the SQL; chaining an extra .run() (db.run(...).run()) is inconsistent with other tests (e.g. account repo tests) and will likely throw (then get swallowed by the try/catch), leaving the memory table uncleared and making these tests order-dependent. Drop the extra .run() and avoid swallowing errors here so failures are visible.
| try { db.run(`DELETE FROM memory`).run() } catch {} | |
| }) | |
| afterEach(() => { | |
| const db = Database.Client() | |
| try { db.run(`DELETE FROM memory`).run() } catch {} | |
| db.run(`DELETE FROM memory`) | |
| }) | |
| afterEach(() => { | |
| const db = Database.Client() | |
| db.run(`DELETE FROM memory`) |
| try { db.run(`DELETE FROM memory`).run() } catch {} | ||
| MemoryExtractor.init(projectPath, "test-session") | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| const db = Database.Client() | ||
| try { db.run(`DELETE FROM memory`).run() } catch {} |
There was a problem hiding this comment.
Same issue as in store.test.ts: db.run(...).run() is likely incorrect and the try/catch {} will silently skip DB cleanup if it throws, making tests flaky. Prefer db.run(sql) and let errors surface so test setup failures are caught.
| try { db.run(`DELETE FROM memory`).run() } catch {} | |
| MemoryExtractor.init(projectPath, "test-session") | |
| }) | |
| afterEach(() => { | |
| const db = Database.Client() | |
| try { db.run(`DELETE FROM memory`).run() } catch {} | |
| db.run(`DELETE FROM memory`) | |
| MemoryExtractor.init(projectPath, "test-session") | |
| }) | |
| afterEach(() => { | |
| const db = Database.Client() | |
| db.run(`DELETE FROM memory`) |
| test("updateMemoryFile creates file from store", async () => { | ||
| // We need to insert into the DB first | ||
| const db = Database.Client() | ||
| try { db.run(`DELETE FROM memory`).run() } catch {} | ||
| MemoryStore.save({ projectPath: tmpDir, type: "preference", topic: "t1", content: "use spaces" }) | ||
|
|
There was a problem hiding this comment.
DB cleanup in this test uses db.run(...).run() inside a try/catch {}; if the chained .run() is invalid, cleanup is skipped silently. Use db.run(sql) (no chaining) and avoid swallowing errors so the test reliably starts from a clean table.
| .object({ | ||
| enabled: z.boolean().default(true).describe("Enable auto-memory for cross-session learning"), | ||
| auto_extract: z.boolean().default(true).describe("Automatically extract memories from session events"), | ||
| max_memory_lines: z.number().default(200).describe("Maximum lines of memory to inject into system prompt"), |
There was a problem hiding this comment.
max_memory_lines is defined as z.number().default(200) without .int().positive().min(0) guards. Negative or fractional values would lead to confusing trimming behavior (e.g., slice(0, -5) drops lines). Consider validating this as a non-negative integer (similar to other numeric config fields).
| max_memory_lines: z.number().default(200).describe("Maximum lines of memory to inject into system prompt"), | |
| max_memory_lines: z | |
| .number() | |
| .int() | |
| .min(0) | |
| .default(200) | |
| .describe("Maximum lines of memory to inject into system prompt"), |
- Add debounced writes (3s buffer) to avoid SQLite writes on every tool call - Add UPSERT in save() to merge duplicate memories on same topic+project - Tighten preference detector: require 2+ pattern matches (was 1, too noisy) - Improve normalizeCommand() to preserve first positional arg - Add MemoryExtractor.flushPending() + reset() call in session cleanup - Update MEMORY.md header: user edits preserved, gitignore guidance added - Add 3 new tests: debounce buffer, flush on reset, single-pattern false positive - Update store test for UPSERT merge behavior (19/19 pass)
CI Status NoteThe failing checks are pre-existing upstream issues, unrelated to this PR:
All 19 new memory tests pass (extractor + store + memory-file). Phase 1 PositioningThis PR provides the minimal infrastructure for:
Phase 2 additions (embeddings, global scope, LLM-assisted extraction) build directly on this store/injector layer. |
Issue for this PR
Closes #20322
Type of change
What does this PR do?
Adds native auto-memory for cross-session learning. Currently OpenCode has no built-in way to persist patterns between sessions — users have to manually curate AGENTS.md. This PR adds a
memorymodule that automatically extracts useful patterns from session events and makes them available in future sessions.How it works:
memorySQLite table with deduplication by topic.opencode/MEMORY.mdat session end, grouped by typemax_memory_lines){ "memory": { "enabled": true, "auto_extract": true, "max_memory_lines": 200 } }All extraction is fire-and-forget and best-effort — it never blocks or affects session performance.
How did you verify your code works?
bun test packages/opencode/test/memory/— all 16 passScreenshots / recordings
N/A — backend feature, no UI changes
Checklist