diff --git a/packages/coding-agent/src/mnemopi/state.ts b/packages/coding-agent/src/mnemopi/state.ts index e4c403380..4034592f0 100644 --- a/packages/coding-agent/src/mnemopi/state.ts +++ b/packages/coding-agent/src/mnemopi/state.ts @@ -108,11 +108,16 @@ export interface MnemopiMemoryEditOptions { } export interface MnemopiMemoryEditResult { - status: "updated" | "deleted" | "invalidated" | "not_found"; + status: "updated" | "deleted" | "invalidated" | "not_found" | "not_editable"; bank?: string; - store?: "working" | "episodic"; + store?: MnemopiMemoryStore; } +/** Which mnemopi table a resolved memory id lives in. `fact` rows are + * read-only projections of fact extraction (issue #4725): resolvable for + * reads, never editable. */ +export type MnemopiMemoryStore = "working" | "episodic" | "fact"; + interface MnemopiStoredMemoryRow { id?: unknown; content?: unknown; @@ -136,7 +141,7 @@ interface MnemopiStoredMemoryRow { */ export interface MnemopiScopedMemoryHit { bank: string; - store: "working" | "episodic"; + store: MnemopiMemoryStore; row: { id: string; content: string; @@ -256,7 +261,8 @@ export class MnemopiSessionState { for (const target of targets) { const raw = target.memory.get(id) as MnemopiStoredMemoryRow | null; if (!raw) continue; - const store: MnemopiScopedMemoryHit["store"] = raw.memory_store === "episodic" ? "episodic" : "working"; + const store: MnemopiMemoryStore = + raw.memory_store === "episodic" || raw.memory_store === "fact" ? raw.memory_store : "working"; return { bank: target.bank, store, @@ -291,8 +297,16 @@ export class MnemopiSessionState { for (const target of targets) { const row = target.memory.get(id) as MnemopiStoredMemoryRow | null; if (!row) continue; - const store: MnemopiMemoryEditResult["store"] = row.memory_store === "episodic" ? "episodic" : "working"; + const store: MnemopiMemoryStore = + row.memory_store === "episodic" || row.memory_store === "fact" ? row.memory_store : "working"; const resultContext: Pick = { bank: target.bank, store }; + if (store === "fact") { + // Facts are read-only: no memory_edit op mutates the facts + // table, so report that precisely instead of `not_found` + // (the id DID resolve — issue #4725). + ineligible ??= { status: "not_editable", ...resultContext }; + continue; + } if ((op === "update" || op === "forget") && store !== "working") { ineligible ??= { status: "not_found", ...resultContext }; continue; diff --git a/packages/coding-agent/src/prompts/tools/memory-edit.md b/packages/coding-agent/src/prompts/tools/memory-edit.md index 641c482c2..0cb3314ff 100644 --- a/packages/coding-agent/src/prompts/tools/memory-edit.md +++ b/packages/coding-agent/src/prompts/tools/memory-edit.md @@ -5,6 +5,8 @@ Use only with ids returned by the `recall` tool. Operations: - `forget`: permanently delete a working memory. - `invalidate`: softly supersede a working or episodic memory, optionally pointing at `replacement_id`. +Fact ids (recall results marked `[facts]`) are read-only: inspect them with `read memory://`; every edit op on a fact id returns `not_editable`. + Prefer `invalidate` when a memory became stale but its history may still be useful. Use `forget` only for content that should be hard-deleted. **Always read the full memory before `update`.** Recall results are clipped previews (the trailing `…` marks a truncation and `full_length` reports the original size); `update` replaces content wholesale, so overwriting the preview would delete the unseen tail. Fetch the row first with `read memory://`, then pass the merged content in `content`. diff --git a/packages/coding-agent/src/tools/memory-edit.ts b/packages/coding-agent/src/tools/memory-edit.ts index 45b3ab020..1ac505355 100644 --- a/packages/coding-agent/src/tools/memory-edit.ts +++ b/packages/coding-agent/src/tools/memory-edit.ts @@ -50,7 +50,9 @@ export class MemoryEditTool implements AgentTool { const text = result.status === "not_found" ? `Memory ${params.id} was not found${location}.` - : `Memory ${params.id} ${result.status}${location}.`; + : result.status === "not_editable" + ? `Memory ${params.id} is a read-only fact${location}; it cannot be edited. Read it with memory://${params.id}.` + : `Memory ${params.id} ${result.status}${location}.`; return { content: [{ type: "text", text }], details: result, diff --git a/packages/coding-agent/test/internal-urls/memory-protocol.test.ts b/packages/coding-agent/test/internal-urls/memory-protocol.test.ts index b88eee52d..331ad66ce 100644 --- a/packages/coding-agent/test/internal-urls/memory-protocol.test.ts +++ b/packages/coding-agent/test/internal-urls/memory-protocol.test.ts @@ -238,6 +238,51 @@ describe("MemoryProtocolHandler — mnemopi bridge (issue #4443)", () => { }); }); + it("resolves memory:// to a read-only fact row (issue #4725)", async () => { + await withMnemopiSession(async ({ state }) => { + const beam = state.memory.beam; + beam.db + .prepare( + "INSERT INTO facts (fact_id, session_id, subject, predicate, object, timestamp, confidence) VALUES (?, ?, ?, ?, ?, ?, ?)", + ) + .run("0473bbdb8da6df92", beam.sessionId, "Glab", "works-without", "mise prefix", "2026-07-01T00:00:00.000Z", 0.9); + + const router = InternalUrlRouter.instance(); + const resource = await router.resolve("memory://0473bbdb8da6df92"); + + expect(resource.content).toContain("id: 0473bbdb8da6df92"); + expect(resource.content).toContain("store: fact"); + expect(resource.content).toContain("Glab works-without mise prefix"); + }); + }); + + it("reports not_editable (not not_found) for memory_edit ops on a fact id (issue #4725)", async () => { + await withMnemopiSession(async ({ state }) => { + const beam = state.memory.beam; + beam.db + .prepare( + "INSERT INTO facts (fact_id, session_id, subject, predicate, object, timestamp, confidence) VALUES (?, ?, ?, ?, ?, ?, ?)", + ) + .run("fact-readonly", beam.sessionId, "service", "uses", "postgres", "2026-07-01T00:00:00.000Z", 0.9); + + expect(state.editScopedMemory("update", "fact-readonly", { content: "x" })).toMatchObject({ + status: "not_editable", + store: "fact", + }); + expect(state.editScopedMemory("forget", "fact-readonly")).toMatchObject({ + status: "not_editable", + store: "fact", + }); + expect(state.editScopedMemory("invalidate", "fact-readonly")).toMatchObject({ + status: "not_editable", + store: "fact", + }); + + // The fact row itself is untouched by the rejected edits. + expect(beam.db.prepare("SELECT fact_id FROM facts WHERE fact_id = ?").get("fact-readonly")).not.toBeNull(); + }); + }); + it("routes memory://root to the file-backed summary even when mnemopi is active", async () => { await withMnemopiSession(async () => { const router = InternalUrlRouter.instance(); diff --git a/packages/mnemopi/src/core/beam/store.ts b/packages/mnemopi/src/core/beam/store.ts index 5d0d2f9dd..225452fc5 100644 --- a/packages/mnemopi/src/core/beam/store.ts +++ b/packages/mnemopi/src/core/beam/store.ts @@ -665,7 +665,46 @@ export function get(beam: BeamMemoryState, memoryId: string): Row | null { WHERE id = ? AND (session_id = ? OR scope = 'global') `) .get(memoryId, beam.sessionId) as Row | null | undefined; - return episodic == null ? null : { ...episodic, metadata: episodic.metadata_json, memory_store: "episodic" }; + if (episodic != null) return { ...episodic, metadata: episodic.metadata_json, memory_store: "episodic" }; + + return getFact(beam, memoryId); +} + +/** + * Read-only resolution for ids minted from the `facts` table. `recall` + * surfaces `facts.fact_id` as a result id (`factRecall`), so `get` must + * resolve those ids too — otherwise every surfaced fact id is a dead end + * for the read path (issue #4725). Visibility mirrors `factRecall`: + * same-session facts plus explicitly global ones (`scope` is an optional + * column on `facts`; `SELECT *` tolerates banks without it, in which case + * only same-session facts resolve). The row is shaped like the + * working/episodic hits with the full triple as content; + * `memory_store: "fact"` marks it read-only — no update/forget/invalidate + * path mutates `facts`. + */ +function getFact(beam: BeamMemoryState, memoryId: string): Row | null { + const fact = beam.db.prepare("SELECT * FROM facts WHERE fact_id = ?").get(memoryId) as Row | null | undefined; + if (fact == null) return null; + if (fact.session_id !== beam.sessionId && fact.scope !== "global") return null; + const subject = typeof fact.subject === "string" ? fact.subject : ""; + const predicate = typeof fact.predicate === "string" ? fact.predicate : ""; + const object = typeof fact.object === "string" ? fact.object : ""; + return { + id: fact.fact_id, + content: [subject, predicate, object].filter(part => part.length > 0).join(" "), + source: "facts", + timestamp: fact.timestamp ?? null, + session_id: fact.session_id ?? null, + importance: fact.confidence ?? null, + metadata: JSON.stringify({ + subject, + predicate, + object, + source_msg_id: fact.source_msg_id ?? null, + }), + created_at: fact.created_at ?? null, + memory_store: "fact", + }; } export function forgetWorking(beam: BeamMemoryState, memoryId: string): boolean { diff --git a/packages/mnemopi/test/beam-store.test.ts b/packages/mnemopi/test/beam-store.test.ts index f9af3a8b5..fb09c7a62 100644 --- a/packages/mnemopi/test/beam-store.test.ts +++ b/packages/mnemopi/test/beam-store.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it } from "bun:test"; +import { recallEnhanced } from "@oh-my-pi/pi-mnemopi/core/beam/recall"; import { initBeam } from "@oh-my-pi/pi-mnemopi/core/beam/schema"; import { exportToDict, @@ -217,3 +218,70 @@ describe("beam store free functions", () => { expect(scratchpadRead(dest).map(row => row.content)).toEqual([]); }); }); + +describe("fact-id read path (issue #4725)", () => { + function insertFact( + beam: BeamMemoryState, + factId: string, + sessionId: string, + subject: string, + predicate: string, + object: string, + confidence = 0.9, + ): void { + beam.db + .prepare( + "INSERT INTO facts (fact_id, session_id, subject, predicate, object, timestamp, confidence) VALUES (?, ?, ?, ?, ?, ?, ?)", + ) + .run(factId, sessionId, subject, predicate, object, "2026-05-30T00:00:00.000Z", confidence); + } + + it("resolves an id surfaced by fact recall to a read-only fact row", async () => { + const beam = makeState(); + insertFact(beam, "fact-postgres", beam.sessionId, "service", "uses", "postgres database", 0.91); + + const results = await recallEnhanced(beam, "postgres", 5, { includeFacts: true }); + const surfaced = results.find(result => result.source === "facts"); + expect(surfaced?.id).toBe("fact-postgres"); + + // memory:// reads and memory_edit both resolve ids via get(); a + // surfaced fact id must not be a dead end. + const row = get(beam, "fact-postgres"); + expect(row).toMatchObject({ + id: "fact-postgres", + content: "service uses postgres database", + source: "facts", + importance: 0.91, + session_id: beam.sessionId, + memory_store: "fact", + }); + expect(JSON.parse(String(row?.metadata))).toMatchObject({ + subject: "service", + predicate: "uses", + object: "postgres database", + }); + }); + + it("keeps fact reads session-scoped like fact recall, honoring explicit global scope", () => { + const beam = makeState(); + insertFact(beam, "fact-other", "session-other", "service", "uses", "postgres database"); + expect(get(beam, "fact-other")).toBeNull(); + + beam.db.run("ALTER TABLE facts ADD COLUMN scope TEXT DEFAULT 'session'"); + beam.db.run("UPDATE facts SET scope = 'global' WHERE fact_id = 'fact-other'"); + expect(get(beam, "fact-other")?.memory_store).toBe("fact"); + }); + + it("keeps working rows first on id collision and never deletes facts via forgetWorking", () => { + const beam = makeState(); + insertFact(beam, "shared-id", beam.sessionId, "service", "uses", "postgres database"); + const workingId = remember(beam, "working row shadowing a fact id"); + beam.db.prepare("UPDATE working_memory SET id = ? WHERE id = ?").run("shared-id", workingId); + + expect(get(beam, "shared-id")?.memory_store).toBe("working"); + + expect(forgetWorking(beam, "fact-missing")).toBe(false); + expect(forgetWorking(beam, "shared-id")).toBe(true); + expect(get(beam, "shared-id")?.memory_store).toBe("fact"); + }); +});