feat(plugin): tombstone forgotten rules and record their scope

packages/opencode/src/plugin/observer.ts

@@ -292,6 +292,7 @@ class SessionObserverManager {
     let stored = entry
     await this.mutate(async () => {
       for (const file of files) {
+        const scope: "global" | "project" = file === paths.project ? "project" : "global"
         try {
           await fs.mkdir(path.dirname(file), { recursive: true })
           let existing: SessionLearning[] = []
@@ -302,13 +303,18 @@ class SessionObserverManager {
           const found = existing.find((e) => e.lesson.trim().toLowerCase() === normalizedText)
           if (found) {
             // Re-recording refreshes recency AND moves the rule to the front, so it
-            // re-enters the injection window; it never duplicates.
+            // re-enters the injection window; it never duplicates. It also clears any
+            // tombstone, which is how a forgotten rule is restored.
             found.timestamp = new Date().toISOString()
+            found.scope = scope
+            found.deletedAt = undefined
+            found.deletedBy = undefined
             stored = found
-            await writeJsonAtomic(file, [found, ...existing.filter((e) => e !== found)].slice(0, 100))
+            await writeJsonAtomic(file, trimRules([found, ...existing.filter((e) => e !== found)]))
             continue
           }
-          await writeJsonAtomic(file, [entry, ...existing].slice(0, 100))
+          await writeJsonAtomic(file, trimRules([{ ...entry, scope }, ...existing]))
+          stored = { ...entry, scope }
         } catch {}
       }
     })
@@ -337,7 +343,7 @@ class SessionObserverManager {
       // global note (AIMETH-008 - shared repo facts win). One entry per rule text.
       const seen = new Set<string>()
       const merged: SessionLearning[] = []
-      for (const rule of [...(await read(paths.project)), ...(await read(paths.global))]) {
+      for (const rule of [...(await read(paths.project)), ...(await read(paths.global))].filter(isLive)) {
         const key = learningKey(rule)
         if (seen.has(key)) continue
         seen.add(key)
@@ -379,8 +385,8 @@ class SessionObserverManager {
       }
     }
     const paths = learningStorePaths(workspaceDir)
-    const global = await read(paths.global)
-    const project = await read(paths.project)
+    const global = (await read(paths.global)).filter(isLive)
+    const project = (await read(paths.project)).filter(isLive)
     const globalKeys = new Set(global.map(learningKey))
     const merged = [
       ...global.map((l) => ({ ...l, origin: "global" as const })),
@@ -393,49 +399,66 @@ class SessionObserverManager {
   }
 
   /**
-   * Remove rules whose text matches (case-insensitive substring) from memory and
-   * from both persisted stores. Returns the number of distinct rules removed; a
-   * rule present in both stores is counted once.
+   * Forget rules whose text matches (case-insensitive substring): mark them deleted
+   * (tombstone) in both persisted stores. Returns the number of distinct rules newly
+   * forgotten; a rule present in both stores is counted once.
    */
   public async forgetLearning(lessonText: string, workspaceDir?: string): Promise<number> {
     const needle = lessonText.trim().toLowerCase()
     if (!needle) return 0
     const matches = (l: SessionLearning) => String(l.lesson ?? "").toLowerCase().includes(needle)
     const paths = learningStorePaths(workspaceDir)
     const files = [paths.global, paths.project].filter((file): file is string => Boolean(file))
-    const removed = new Set<string>()
-    for (const file of files) {
-      for (const key of await this.dropRules(file, matches)) removed.add(key)
-    }
-    return removed.size
+    // One mutation for both stores, so a record cannot interleave between them.
+    return this.mutate(async () => {
+      const results: Array<{ keys: Set<string>; ok: boolean }> = []
+      for (const file of files) results.push(await this.applyTombstones(file, matches))
+      // If a store could not be read or written, we cannot claim its copy is gone, so
+      // report nothing rather than a count that may be wrong.
+      if (results.some((r) => !r.ok)) return 0
+      const forgotten = new Set<string>()
+      for (const result of results) for (const key of result.keys) forgotten.add(key)
+      return forgotten.size
+    })
   }
 
   /**
-   * Remove matching rules from one store atomically and count them. A removal is
-   * only reported when the write landed; a failed write returns nothing and the
-   * file is left untouched.
+   * Mark matching live rules as deleted in one store. The rows are kept as
+   * tombstones so a later merge or sync cannot resurrect them. Non-queuing: the
+   * caller must already hold the mutation lock. Returns the keys acted on and
+   * whether the read and write both succeeded; an absent store is not a failure.
    */
-  private dropRules(file: string, matches: (l: SessionLearning) => boolean): Promise<Set<string>> {
-    return this.mutate(async () => {
-      const found = new Set<string>()
-      try {
-        const parsed = JSON.parse(await fs.readFile(file, "utf8"))
-        if (!Array.isArray(parsed)) return found
-        const kept: SessionLearning[] = []
-        for (const row of parsed as SessionLearning[]) {
-          if (matches(row)) {
-            found.add(String(row.lesson ?? "").trim().toLowerCase())
-            continue
-          }
-          kept.push(row)
-        }
-        if (found.size === 0) return found
-        await writeJsonAtomic(file, kept.slice(0, 100))
-      } catch {
-        return new Set<string>()
+  private async applyTombstones(
+    file: string,
+    matches: (l: SessionLearning) => boolean,
+  ): Promise<{ keys: Set<string>; ok: boolean }> {
+    const keys = new Set<string>()
+    let raw: string
+    try {
+      raw = await fs.readFile(file, "utf8")
+    } catch (error) {
+      // Only a missing store is a successful absence; any other read error means we
+      // cannot know what the store holds.
+      const code = error && typeof error === "object" && "code" in error ? (error as { code?: unknown }).code : undefined
+      return { keys, ok: code === "ENOENT" }
+    }
+    try {
+      const parsed = JSON.parse(raw)
+      if (!Array.isArray(parsed)) return { keys, ok: false }
+      const rows = parsed as SessionLearning[]
+      let changed = false
+      for (const row of rows) {
+        if (!isLive(row) || !matches(row)) continue
+        row.deletedAt = new Date().toISOString()
+        row.deletedBy = process.env.USERNAME || process.env.USER || "local"
+        keys.add(learningKey(row))
+        changed = true
       }
-      return found
-    })
+      if (changed) await writeJsonAtomic(file, trimRules(rows))
+      return { keys, ok: true }
+    } catch {
+      return { keys, ok: false }
+    }
   }
 
   public getLatestRetrospectives(): SessionRetrospective[] {
@@ -461,6 +484,23 @@ export function learningStorePaths(workspaceDir?: string): { global?: string; pr
 
 const learningKey = (l: SessionLearning) => String(l.lesson ?? "").trim().toLowerCase()
 
+/** A rule is live unless it carries a tombstone. */
+const isLive = (l: SessionLearning) => !l.deletedAt
+
+const MAX_LIVE_RULES = 100
+const MAX_TOMBSTONES = 100
+
+/**
+ * Bound file size without evicting tombstones: a dropped tombstone would let a
+ * later merge resurrect the rule it records as deleted.
+ */
+function trimRules(rows: SessionLearning[]): SessionLearning[] {
+  return [
+    ...rows.filter(isLive).slice(0, MAX_LIVE_RULES),
+    ...rows.filter((r) => !isLive(r)).slice(0, MAX_TOMBSTONES),
+  ]
+}
+
 /** Write JSON via a temp file + rename so a reader never sees a half-written file. */
 async function writeJsonAtomic(file: string, rows: SessionLearning[]): Promise<void> {
   const tmp = `${file}.${process.pid}.${Math.random().toString(36).slice(2, 7)}.tmp`
@@ -474,6 +514,11 @@ export interface SessionLearning {
   lesson: string
   context?: string
   source: "user_feedback" | "buddy_review" | "auto_correction" | "session_retro"
+  /** Which store this row belongs to. */
+  scope?: "global" | "project"
+  /** Tombstone: set when the rule is forgotten, so a later merge cannot resurrect it. */
+  deletedAt?: string
+  deletedBy?: string
 }
 
 export const sessionObserver = new SessionObserverManager()

packages/opencode/test/plugin/observer.test.ts

@@ -168,4 +168,144 @@ describe("rule visibility", () => {
       restoreUserProfile(prev)
     }
   })
+
+  test("forgetting keeps the row as a tombstone instead of deleting it", async () => {
+    await using home = await tmpdir()
+    const prev = process.env.USERPROFILE
+    process.env.USERPROFILE = home.path
+    try {
+      const file = path.join(home.path, ".local", "share", "opencode", "learnings.json")
+      await sessionObserver.recordLearning({ lesson: "tombstone me", source: "user_feedback" })
+      expect(await sessionObserver.forgetLearning("tombstone me")).toBe(1)
+
+      expect((await sessionObserver.listLearnings()).find((r) => r.lesson === "tombstone me")).toBeUndefined()
+
+      const rows = JSON.parse(await fs.readFile(file, "utf8"))
+      const row = rows.find((r: { lesson: string }) => r.lesson === "tombstone me")
+      expect(row).toBeDefined()
+      expect(typeof row.deletedAt).toBe("string")
+      expect(row.scope).toBe("global")
+    } finally {
+      restoreUserProfile(prev)
+    }
+  })
+
+  test("re-recording a forgotten rule revives it without duplicating", async () => {
+    await using home = await tmpdir()
+    const prev = process.env.USERPROFILE
+    process.env.USERPROFILE = home.path
+    try {
+      const file = path.join(home.path, ".local", "share", "opencode", "learnings.json")
+      await sessionObserver.recordLearning({ lesson: "revive me", source: "user_feedback" })
+      expect(await sessionObserver.forgetLearning("revive me")).toBe(1)
+      expect((await sessionObserver.listLearnings()).find((r) => r.lesson === "revive me")).toBeUndefined()
+
+      await sessionObserver.recordLearning({ lesson: "revive me", source: "user_feedback" })
+
+      expect((await sessionObserver.listLearnings()).find((r) => r.lesson === "revive me")!.injected).toBe(true)
+      const rows = JSON.parse(await fs.readFile(file, "utf8"))
+      const rowsForLesson = rows.filter((r: { lesson: string }) => r.lesson === "revive me")
+      expect(rowsForLesson.length).toBe(1)
+      expect(rowsForLesson[0].deletedAt).toBeUndefined()
+    } finally {
+      restoreUserProfile(prev)
+    }
+  })
+
+  test("records the scope of each rule", async () => {
+    await using home = await tmpdir()
+    await using ws = await tmpdir()
+    const prev = process.env.USERPROFILE
+    process.env.USERPROFILE = home.path
+    try {
+      await sessionObserver.recordLearning({ lesson: "global rule", source: "user_feedback" })
+      await sessionObserver.recordLearning({ lesson: "project rule", source: "user_feedback" }, ws.path)
+
+      const global = JSON.parse(
+        await fs.readFile(path.join(home.path, ".local", "share", "opencode", "learnings.json"), "utf8"),
+      )
+      const project = JSON.parse(
+        await fs.readFile(path.join(ws.path, ".system_generated", "logs", "learnings.json"), "utf8"),
+      )
+      expect(global.find((r: { lesson: string }) => r.lesson === "global rule").scope).toBe("global")
+      expect(project.find((r: { lesson: string }) => r.lesson === "project rule").scope).toBe("project")
+    } finally {
+      restoreUserProfile(prev)
+    }
+  })
+
+  test("a tombstone survives the record cap", async () => {
+    await using home = await tmpdir()
+    const prev = process.env.USERPROFILE
+    process.env.USERPROFILE = home.path
+    try {
+      const file = path.join(home.path, ".local", "share", "opencode", "learnings.json")
+      const rows: Array<{
+        id: string
+        timestamp: string
+        lesson: string
+        source: string
+        deletedAt?: string
+        deletedBy?: string
+      }> = Array.from({ length: 100 }, (_, i) => ({
+        id: "r" + i,
+        timestamp: "2026-09-01T00:00:00.000Z",
+        lesson: "rule " + i,
+        source: "user_feedback",
+      }))
+      rows.push({
+        id: "dead",
+        timestamp: "2026-09-01T00:00:00.000Z",
+        lesson: "tombstoned rule",
+        source: "user_feedback",
+        deletedAt: "2026-09-02T00:00:00.000Z",
+        deletedBy: "test",
+      })
+      await fs.mkdir(path.dirname(file), { recursive: true })
+      await fs.writeFile(file, JSON.stringify(rows), "utf8")
+
+      await sessionObserver.recordLearning({ lesson: "one more", source: "user_feedback" })
+
+      const after = JSON.parse(await fs.readFile(file, "utf8"))
+      const tomb = after.find((r: { lesson: string }) => r.lesson === "tombstoned rule")
+      expect(tomb).toBeDefined()
+      expect(tomb.deletedAt).toBe("2026-09-02T00:00:00.000Z")
+    } finally {
+      restoreUserProfile(prev)
+    }
+  })
+
+  test("forgetting still counts when the other store does not exist", async () => {
+    await using home = await tmpdir()
+    await using ws = await tmpdir()
+    const prev = process.env.USERPROFILE
+    process.env.USERPROFILE = home.path
+    try {
+      await sessionObserver.recordLearning({ lesson: "only global", source: "user_feedback" }, ws.path)
+      // Remove the project store so only the global copy remains.
+      await fs.rm(path.join(ws.path, ".system_generated", "logs", "learnings.json"), { force: true })
+
+      expect(await sessionObserver.forgetLearning("only global", ws.path)).toBe(1)
+    } finally {
+      restoreUserProfile(prev)
+    }
+  })
+
+  test("forgetting reports nothing when a store is unreadable data", async () => {
+    await using home = await tmpdir()
+    await using ws = await tmpdir()
+    const prev = process.env.USERPROFILE
+    process.env.USERPROFILE = home.path
+    try {
+      await sessionObserver.recordLearning({ lesson: "fragile", source: "user_feedback" })
+      // The project store exists but holds valid JSON that is not the expected array.
+      const wsFile = path.join(ws.path, ".system_generated", "logs", "learnings.json")
+      await fs.mkdir(path.dirname(wsFile), { recursive: true })
+      await fs.writeFile(wsFile, JSON.stringify("not a rules array"), "utf8")
+
+      expect(await sessionObserver.forgetLearning("fragile", ws.path)).toBe(0)
+    } finally {
+      restoreUserProfile(prev)
+    }
+  })
 })