Skip to content

Commit 5d3fbb7

Browse files
committed
fix(memory): terminal-state precedence in m[1] mutation-log fold (#12); accept archive non-revival (#13)
#12 — getMemoryMutationsForRender coalesced newest-id-per-target with no terminal precedence, so archive-then-update on a rendered memory emitted <updated> in the m[1] <memory-updates> delta even though the memory left the active set (an archived row won't reappear in the next m[0] baseline → the agent is told a gone memory is present/changed). Fix: a terminal mutation (archive/delete/superseded) now outranks a later non-terminal update for the same target, regardless of id order; same-terminality still resolves newest-id-wins. Safe because un-archive (restore) happens via an epoch-bump full re-materialize that advances the cursor past the archive row — there is no un-archive signal inside the log. Shared core fn, so OpenCode + Pi both get it (Pi imports getMemoryMutationsForRender from core). +3 fold tests (archive>update, update-then-delete, multi-update). #13 — ACCEPTED as designed (not a fix): archiving is a deliberate dreamer/user suppression, so re-observing a fact must NOT silently revive it. getMemoryByHash matches the archived row, bumps seen_count (recurrence recorded), does not re-insert/un-archive; revival is restore-only (epoch bump). Characterization test relabeled from 'CURRENT BEHAVIOR' to a locked contract. plugin 1706/0, Pi 403/0, tsc + biome clean, both dists rebuilt.
1 parent ce80a94 commit 5d3fbb7

3 files changed

Lines changed: 93 additions & 25 deletions

File tree

packages/plugin/src/features/magic-context/memory/promotion.test.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -365,13 +365,14 @@ describe("promotion", () => {
365365
});
366366
});
367367

368-
// Audit finding #13: getMemoryByHash does NOT filter by status, so a
369-
// re-observed fact whose prior instance was archived matches the archived
370-
// row, bumps its seen_count, and is NOT revived (stays archived/invisible).
371-
// This characterization test documents the CURRENT behavior so we can decide
372-
// whether revival-on-re-observation is wanted before changing it.
368+
// ACCEPTED BEHAVIOR (audit decision): archiving a memory is a deliberate
369+
// dreamer/user suppression, so re-observing the same fact must NOT silently
370+
// revive it. getMemoryByHash matches the archived row by (project,category,
371+
// hash), bumps its seen_count (recurrence is still recorded), and does not
372+
// re-insert or un-archive. Revival happens only through an explicit restore
373+
// (which bumps the project epoch). This test locks that contract.
373374
describe("#given a previously-archived fact is re-observed", () => {
374-
it("CURRENT BEHAVIOR: dedupe matches the archived row and does not revive it", () => {
375+
it("dedupe matches the archived row and does NOT revive it (archive is deliberate)", () => {
375376
db = makeMemoryDatabase();
376377
const content = "Use SQLite for cross-session memory";
377378
const hash = computeNormalizedHash(content);

packages/plugin/src/features/magic-context/storage-memory-mutation-log.test.ts

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -113,23 +113,20 @@ describe("storage-memory-mutation-log", () => {
113113
expect(getMemoryMutationsForRender(database, "/repo/a", 0, [])).toEqual([]);
114114
});
115115

116-
// Audit finding #12: render coalescing is "newest mutation-log id per target",
117-
// with no terminal-state precedence. If a memory rendered in m[0] is archived
118-
// and then later updated (update does NOT gate on status), the update row has
119-
// the higher id and WINS — the m[1] <memory-updates> delta shows the memory as
120-
// present/updated even though the canonical table state is archived. This
121-
// characterization test documents the CURRENT behavior so we can decide
122-
// whether archive/delete should take precedence over a later update before
123-
// changing it.
124-
test("CURRENT BEHAVIOR: a later update outranks an earlier archive (no terminal precedence)", () => {
116+
// A terminal mutation (archive/delete/superseded) takes precedence over a
117+
// later non-terminal `update` for the same target, regardless of id order.
118+
// Without this, archive-then-update would render the memory as
119+
// present/updated in the m[1] <memory-updates> delta even though it left the
120+
// active set (an archived row won't reappear in the next m[0] baseline).
121+
test("a terminal archive outranks a later update for the same target", () => {
125122
const database = makeDb();
126-
queueMemoryMutation(database, {
123+
const archive = queueMemoryMutation(database, {
127124
projectPath: "/repo/a",
128125
mutationType: "archive",
129126
targetMemoryId: 10,
130127
queuedAt: 100,
131128
});
132-
const update = queueMemoryMutation(database, {
129+
queueMemoryMutation(database, {
133130
projectPath: "/repo/a",
134131
mutationType: "update",
135132
targetMemoryId: 10,
@@ -138,11 +135,50 @@ describe("storage-memory-mutation-log", () => {
138135
queuedAt: 200,
139136
});
140137

141-
// Memory 10 was rendered in m[0] (active at materialization), then archived,
142-
// then updated. Newest-by-id wins → the render shows the UPDATE, not the archive.
143138
const rows = getMemoryMutationsForRender(database, "/repo/a", 0, [10]);
144139
expect(rows).toHaveLength(1);
145-
expect(rows[0]).toEqual(update);
146-
expect(rows[0]?.mutationType).toBe("update"); // archive is masked
140+
expect(rows[0]).toEqual(archive); // terminal wins; update is masked
141+
expect(rows[0]?.mutationType).toBe("archive");
142+
});
143+
144+
test("update-then-archive still resolves to the archive (newest terminal)", () => {
145+
const database = makeDb();
146+
queueMemoryMutation(database, {
147+
projectPath: "/repo/a",
148+
mutationType: "update",
149+
targetMemoryId: 10,
150+
newContent: "edited",
151+
queuedAt: 100,
152+
});
153+
const archive = queueMemoryMutation(database, {
154+
projectPath: "/repo/a",
155+
mutationType: "delete",
156+
targetMemoryId: 10,
157+
queuedAt: 200,
158+
});
159+
160+
const rows = getMemoryMutationsForRender(database, "/repo/a", 0, [10]);
161+
expect(rows).toEqual([archive]);
162+
});
163+
164+
test("multiple updates with no terminal → newest update wins", () => {
165+
const database = makeDb();
166+
queueMemoryMutation(database, {
167+
projectPath: "/repo/a",
168+
mutationType: "update",
169+
targetMemoryId: 10,
170+
newContent: "v1",
171+
queuedAt: 100,
172+
});
173+
const newest = queueMemoryMutation(database, {
174+
projectPath: "/repo/a",
175+
mutationType: "update",
176+
targetMemoryId: 10,
177+
newContent: "v2",
178+
queuedAt: 200,
179+
});
180+
181+
const rows = getMemoryMutationsForRender(database, "/repo/a", 0, [10]);
182+
expect(rows).toEqual([newest]);
147183
});
148184
});

packages/plugin/src/features/magic-context/storage-memory-mutation-log.ts

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,18 @@ export type MemoryMutationType = "archive" | "delete" | "update" | "superseded";
44

55
const MEMORY_MUTATION_TYPES = new Set<string>(["archive", "delete", "update", "superseded"]);
66

7+
// Terminal mutations mean the memory LEFT the active set (renders as
8+
// <removed>/<superseded>). `update` is non-terminal — the memory is still
9+
// present with new content (renders as <updated>). The render fold gives
10+
// terminal states precedence over a later `update` so an update queued after an
11+
// archive/delete cannot resurrect a memory that is gone (archive is deliberate;
12+
// un-archive happens via an epoch-bump full re-materialize, never via the log).
13+
const TERMINAL_MUTATION_TYPES = new Set<MemoryMutationType>(["archive", "delete", "superseded"]);
14+
15+
function isTerminalMutation(row: MemoryMutationLogRow): boolean {
16+
return TERMINAL_MUTATION_TYPES.has(row.mutationType);
17+
}
18+
719
export interface MemoryMutationLogRow {
820
id: number;
921
projectPath: string;
@@ -118,11 +130,30 @@ export function getMemoryMutationsForRender(
118130
)
119131
.all(projectPath, afterId ?? 0, ...uniqueIds) as MemoryMutationLogDbRow[];
120132

121-
const newestByTarget = new Map<number, MemoryMutationLogRow>();
122-
for (const row of rows) {
123-
newestByTarget.set(row.target_memory_id, toMemoryMutation(row));
133+
// Coalesce to one row per target. Newest-id wins among same-terminality, BUT
134+
// a terminal mutation (archive/delete/superseded) always outranks a
135+
// non-terminal `update` regardless of id order — otherwise an update queued
136+
// after an archive would render the memory as still-present/updated even
137+
// though it left the active set (it won't appear in the next m[0] baseline).
138+
const chosenByTarget = new Map<number, MemoryMutationLogRow>();
139+
for (const dbRow of rows) {
140+
const candidate = toMemoryMutation(dbRow);
141+
const current = chosenByTarget.get(candidate.targetMemoryId);
142+
if (!current) {
143+
chosenByTarget.set(candidate.targetMemoryId, candidate);
144+
continue;
145+
}
146+
const currentTerminal = isTerminalMutation(current);
147+
const candidateTerminal = isTerminalMutation(candidate);
148+
if (currentTerminal && !candidateTerminal) continue; // keep terminal over later update
149+
if (!currentTerminal && candidateTerminal) {
150+
chosenByTarget.set(candidate.targetMemoryId, candidate); // terminal supersedes update
151+
continue;
152+
}
153+
// Same terminality → newest id wins (rows are ASC, so candidate is newer).
154+
chosenByTarget.set(candidate.targetMemoryId, candidate);
124155
}
125-
return [...newestByTarget.values()].sort((left, right) => left.id - right.id);
156+
return [...chosenByTarget.values()].sort((left, right) => left.id - right.id);
126157
}
127158

128159
export function getMaxMemoryMutationId(db: Database, projectPath: string): number | null {

0 commit comments

Comments
 (0)