From ea58e82ff32a82e0f423470afbac5c084ca8e2dd Mon Sep 17 00:00:00 2001 From: hjqcan Date: Sat, 22 Aug 2026 12:42:50 -0400 Subject: [PATCH] Keep MCP recall tools storage-read-only --- src/api/goodMemoryAssembly.ts | 10 +-- src/api/internalSupport.ts | 2 + src/install/hostExecutionContext.ts | 21 +++++- src/install/hostMcpServer.ts | 8 ++- .../host-mcp-server.standalone.direct.test.ts | 66 ++++++++++++++++++- 5 files changed, 98 insertions(+), 9 deletions(-) diff --git a/src/api/goodMemoryAssembly.ts b/src/api/goodMemoryAssembly.ts index 3aeef1f5..c459205c 100644 --- a/src/api/goodMemoryAssembly.ts +++ b/src/api/goodMemoryAssembly.ts @@ -146,10 +146,12 @@ export function createGoodMemoryAssembly(input: { : resolvedRuntime; const storagePlan = runtimeResolution.storagePlan; const explicitStorage = storagePlan.mode === "explicit" ? storagePlan.storage : null; - const recallObservationsEnabled = !( - internal?.sqliteReadOnly && - explicitStorage?.provider === "sqlite" - ); + const recallObservationsEnabled = + internal?.postRecallMutations !== false && + !( + internal?.sqliteReadOnly && + explicitStorage?.provider === "sqlite" + ); const sqliteStoreOptions = internal?.sqliteReadOnly ? { readOnly: true } : undefined; diff --git a/src/api/internalSupport.ts b/src/api/internalSupport.ts index 87caf044..5ecb8657 100644 --- a/src/api/internalSupport.ts +++ b/src/api/internalSupport.ts @@ -36,6 +36,8 @@ export interface InternalGoodMemoryOptions { runtimeCompactionExtraction?: boolean; /** Repo-only immutable SQLite view; also disables post-recall mutations. */ sqliteReadOnly?: boolean; + /** Internal host surface switch for recall diagnostics that must not persist observations. */ + postRecallMutations?: boolean; } export function attachInternalGoodMemorySupport(input: { diff --git a/src/install/hostExecutionContext.ts b/src/install/hostExecutionContext.ts index ebbd297c..975f6482 100644 --- a/src/install/hostExecutionContext.ts +++ b/src/install/hostExecutionContext.ts @@ -1,5 +1,8 @@ import { resolve } from "node:path"; -import { createGoodMemory } from "../api/createGoodMemory"; +import { + createGoodMemory, + createInternalGoodMemory, +} from "../api/createGoodMemory"; import type { GoodMemory, GoodMemoryConfig, @@ -50,6 +53,10 @@ export interface InstalledHostContextInput { sessionId?: string; } +interface InstalledHostMemoryOptions { + postRecallMutations?: boolean; +} + export interface InstalledHostResolvedContext { activationMode: InstalledHostActivationMode; contextMode: InstalledHostContextMode; @@ -220,6 +227,7 @@ function createGlobalWorkspaceConfig(workspaceRoot: string): WorkspaceHostOptInC export function createInstalledHostMemory( context: HostMemoryRuntimeContext, dependencies: InstalledHostContextDependencies = {}, + options: InstalledHostMemoryOptions = {}, ): GoodMemory { const providerAdapters = buildInstalledHostProviderAdapters(context.providers); const sharedDocumentStore = buildSharedAgentDocumentStore(context); @@ -230,7 +238,7 @@ export function createInstalledHostMemory( ...(sharedDocumentStore ? { documentStore: sharedDocumentStore } : {}), } : undefined; - return (dependencies.createMemory ?? createGoodMemory)({ + const config: GoodMemoryConfig = { ...(adapters ? { adapters } : {}), ...(context.language ? { language: context.language } : {}), // 1:1 with GoodMemoryRetrievalConfig; absence keeps rules-only parity. @@ -253,6 +261,15 @@ export function createInstalledHostMemory( ], }, storage: context.storage, + }; + if (dependencies.createMemory) { + return dependencies.createMemory(config); + } + if (options.postRecallMutations === undefined) { + return createGoodMemory(config); + } + return createInternalGoodMemory(config, { + postRecallMutations: options.postRecallMutations, }); } diff --git a/src/install/hostMcpServer.ts b/src/install/hostMcpServer.ts index 9b29bbaa..e9b379d6 100644 --- a/src/install/hostMcpServer.ts +++ b/src/install/hostMcpServer.ts @@ -129,7 +129,9 @@ export function createGoodMemoryMcpServer( const context = resolveStandaloneMcpContext(input.standalone, args); return { ...context, - memory: createInstalledHostMemory(context, dependencies), + memory: createInstalledHostMemory(context, dependencies, { + postRecallMutations: false, + }), }; } return loadInstalledHostExecutionContext( @@ -769,7 +771,9 @@ async function loadInstalledHostExecutionContext( } return { ...resolved.context, - memory: createInstalledHostMemory(resolved.context, dependencies), + memory: createInstalledHostMemory(resolved.context, dependencies, { + postRecallMutations: false, + }), }; } diff --git a/tests/unit/host-mcp-server.standalone.direct.test.ts b/tests/unit/host-mcp-server.standalone.direct.test.ts index 70f8c2dc..859573dc 100644 --- a/tests/unit/host-mcp-server.standalone.direct.test.ts +++ b/tests/unit/host-mcp-server.standalone.direct.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from "bun:test"; -import { basename, resolve } from "node:path"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { basename, join, resolve } from "node:path"; +import { createGoodMemory } from "../../src/api/createGoodMemory"; import type { BuildContextInput, ExportMemoryInput, @@ -611,6 +614,67 @@ describe("goodmemory mcp server standalone direct handlers", () => { ).toBe(1); }); + it("keeps context recall storage-read-only when the write tool is enabled", async () => { + const root = await mkdtemp(join(tmpdir(), "goodmemory-mcp-read-only-")); + const storageUrl = join(root, "memory.sqlite"); + const scope = { + userId: "standalone-read-only-user", + workspaceId: "standalone-read-only-workspace", + }; + const server = inspectServer( + createGoodMemoryMcpServer({ + allowWrite: true, + standalone: { + storage: { provider: "sqlite", url: storageUrl }, + ...scope, + }, + }), + ); + + try { + const memory = createGoodMemory({ + storage: { provider: "sqlite", url: storageUrl }, + }); + const written = await memory.remember({ + annotations: [ + { + confirmed: true, + messageIndex: 0, + remember: "always", + }, + ], + extractionStrategy: "rules-only", + messages: [ + { + content: "Remember this project decision: use SQLite in WAL mode.", + observedAt: "2020-01-01T00:00:00.000Z", + role: "user", + }, + ], + scope, + }); + expect(written.accepted).toBeGreaterThan(0); + + const before = await memory.exportMemory({ scope }); + expect(before.durable.experiences).toHaveLength(0); + expect(before.durable.promotions).toHaveLength(0); + expect(before.durable.proposals).toHaveLength(0); + + const recalled = await server._registeredTools.goodmemory_get_context!.handler({ + query: "Which database and journal mode does this project use?", + }); + expect(recalled.isError).toBeUndefined(); + + const after = await memory.exportMemory({ scope }); + expect(after.durable.facts.length).toBeGreaterThan(0); + expect(after.durable.experiences).toHaveLength(0); + expect(after.durable.promotions).toHaveLength(0); + expect(after.durable.proposals).toHaveLength(0); + } finally { + await rm(root, { force: true, recursive: true }); + } + }); + it("routes installed-mode writes through the installed context", async () => { const { memory } = createFakeMemory(); // All-ENOENT readFile: installed mode without config must surface the