From 9d4fcdb53648c4ec16ba4549336447ad74ac0d26 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 11 Jun 2026 18:34:53 +0000 Subject: [PATCH] fix(mnemopi): keep consolidate() reachable from aliased subagent enqueue The previous refactor dropped `if (this.aliasOf) return` into consolidate(), which made `/memory enqueue` invoked from inside a subagent a no-op. The old enqueue() body short-circuited only forceRetainCurrentSession (via its own guard) but still flushed extractions and slept on state.memory, which for aliased states points at the parent's shared retain bank. Moved the alias guard back out of consolidate(): forceRetainCurrentSession keeps its own early-return (the subagent's transcript is the parent's concern), so consolidate() runs the SQL-level flush+sleep on every owned bank for both primary and aliased states. The lifecycle guard stays in dispose() so disposing a subagent still does not flush, sleep, or close the parent's memory. Added a regression test: a /memory enqueue routed through the aliased child state calls flushExtractions + sleepAllSessions(false) on the parent's owned memory exactly once, and forceRetainCurrentSession runs on the child (where its own guard returns early) but not on the parent. Addresses #2327 review. --- packages/coding-agent/CHANGELOG.md | 2 +- packages/coding-agent/src/mnemopi/state.ts | 10 ++++- .../coding-agent/test/memory-tools.test.ts | 42 +++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index dbeab22d6..478d897f6 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed Mnemopi memory consolidation never running on session shutdown: `MnemopiSessionState.dispose()` now drains pending fact extractions and runs `sleepAllSessions` on every owned bank before closing handles (and `AgentSession.dispose()` awaits the result), matching the `/memory enqueue` slash command. `mnemopiBackend.clear` opts out via `dispose({ consolidate: false })` so the destructive `/memory clear` path does not spend tokens consolidating memories that are wiped on the next line. Without this, `episodic_memory`, `gists`, `consolidation_log`, `graph_edges`, and `triples` stayed empty for every deployment because the SHMR/beam pipeline only ran when a user typed `/memory enqueue|rebuild` ([#2320](https://github.com/can1357/oh-my-pi/issues/2320)). +- Fixed Mnemopi memory consolidation never running on session shutdown: `MnemopiSessionState.dispose()` now drains pending fact extractions and runs `sleepAllSessions` on every owned bank before closing handles (and `AgentSession.dispose()` awaits the result), matching the `/memory enqueue` slash command. `mnemopiBackend.clear` opts out via `dispose({ consolidate: false })` so the destructive `/memory clear` path does not spend tokens consolidating memories that are wiped on the next line. `consolidate()` deliberately keeps no `aliasOf` short-circuit so `/memory enqueue` from a subagent still flushes and sleeps the parent's shared banks (the alias guard lives in `dispose` for lifecycle, not in `consolidate` for content). Without this, `episodic_memory`, `gists`, `consolidation_log`, `graph_edges`, and `triples` stayed empty for every deployment because the SHMR/beam pipeline only ran when a user typed `/memory enqueue|rebuild` ([#2320](https://github.com/can1357/oh-my-pi/issues/2320)). ## [15.11.2] - 2026-06-11 diff --git a/packages/coding-agent/src/mnemopi/state.ts b/packages/coding-agent/src/mnemopi/state.ts index 1dba44e2e..b43c3b274 100644 --- a/packages/coding-agent/src/mnemopi/state.ts +++ b/packages/coding-agent/src/mnemopi/state.ts @@ -377,9 +377,17 @@ export class MnemopiSessionState { * callers can keep using the state. {@link dispose} composes this with the * close step so normal session shutdown promotes working memory to * episodic/gists/graph automatically (see issue #2320). + * + * Aliased subagent states share `scoped` (and therefore the actual SQLite + * banks) with their parent. `consolidate()` deliberately does NOT + * short-circuit on `aliasOf`: `forceRetainCurrentSession` already guards + * itself, and an explicit `/memory enqueue` invoked from within a subagent + * still needs to flush extractions and sleep the parent's shared banks — + * otherwise enqueue would report success while leaving the subagent's + * retained memories unconsolidated until the parent eventually shuts down + * (PR #2327 review). */ async consolidate(): Promise { - if (this.aliasOf) return; await this.forceRetainCurrentSession(); for (const memory of this.scoped.owned) { await memory.flushExtractions(); diff --git a/packages/coding-agent/test/memory-tools.test.ts b/packages/coding-agent/test/memory-tools.test.ts index 3edb38b1a..a3598948d 100644 --- a/packages/coding-agent/test/memory-tools.test.ts +++ b/packages/coding-agent/test/memory-tools.test.ts @@ -582,6 +582,48 @@ describe("Mnemopi backend lifecycle", () => { expect(parentRetainSpy).not.toHaveBeenCalled(); }); + it("aliased subagent enqueue still flushes and sleeps the parent's shared banks (#2327 review)", async () => { + const settings = Settings.isolated({ "memory.backend": "mnemopi" }); + const parentState = registerMnemopiState(); + const parentMemory = parentState.getScopedRetainTarget().memory; + const childSession = { + sessionId: "child-session-id", + settings, + sessionManager: { getEntries: () => [], getCwd: () => "/tmp" }, + emitNotice: () => {}, + modelRegistry: {} as never, + getMnemopiSessionState: () => getMnemopiSessionState(childSession), + } as never; + await mnemopiBackend.start({ + session: childSession, + settings, + modelRegistry: {} as never, + agentDir: path.dirname(tempDbPath!), + taskDepth: 1, + parentMnemopiSessionState: parentState, + }); + const childState = getMnemopiSessionState(childSession); + expect(childState?.aliasOf).toBe(parentState); + + const flushSpy = vi.spyOn(parentMemory, "flushExtractions"); + const sleepSpy = vi.spyOn(parentMemory, "sleepAllSessions"); + const parentRetainSpy = vi.spyOn(parentState, "forceRetainCurrentSession"); + const childRetainSpy = vi.spyOn(childState!, "forceRetainCurrentSession"); + + await mnemopiBackend.enqueue(path.dirname(tempDbPath!), "/tmp", childSession); + + // /memory enqueue from a subagent must still consolidate the shared + // banks; `forceRetainCurrentSession` is the one piece that the alias + // guard short-circuits (the subagent's transcript is the parent's + // concern), but the SQL-level flush and sleep must reach every owned + // bank or the user's enqueue silently no-ops. + expect(flushSpy).toHaveBeenCalledTimes(1); + expect(sleepSpy).toHaveBeenCalledTimes(1); + expect(sleepSpy).toHaveBeenCalledWith(false); + expect(childRetainSpy).toHaveBeenCalledTimes(1); + expect(parentRetainSpy).not.toHaveBeenCalled(); + }); + it("clears every scoped Mnemopi database for per-project-tagged mode", async () => { const config = makeMnemopiConfig({ scoping: "per-project-tagged",