fix(coding-agents): stop unsafe gitlog cleanup (#3879)

The git-log deepen path enumerated bank-wide `source:git-log` documents and deleted every returned non-canonical one as stale. The document listing endpoint's inclusive `all` tag mode can return untagged documents, so that sweep deleted unrelated documents — and because deletion cascades to facts, a routine coding-agents sync could remove unrelated memories from a shared bank.

Remove the cleanup entirely: git-log sync now owns only its canonical document and never deletes other document IDs. Internal multi-tag strategy probes use `all_strict`, while `listDocumentIds()` keeps its existing public `all` default for external callers. A git-log snapshot counts as current only when the current-HEAD query contains this repository's canonical document; otherwise the sync performs an idempotent upsert.

Canonical IDs remain `gitlog:<repoName>`, so same-named repositories and forks can still share one ID. This change prevents cross-document deletion; renamespacing safely needs a separate ownership/migration design.

Fixes #3877.

hindsight-integrations/coding-agents/src/core/git.gitlog.test.ts

@@ -4,7 +4,14 @@ import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import type { HindsightClient } from "./hindsight";
-import { gitLogNewestAuthorDate, gitLogText, ingestGitLog, repoNameOf, retainCommit } from "./git";
+import {
+  gitLogNewestAuthorDate,
+  gitLogText,
+  ingestGitLog,
+  repoNameOf,
+  retainCommit,
+  syncGitLog,
+} from "./git";
 
 let dir: string;
 
@@ -132,3 +139,52 @@ describe("ingestGitLog", () => {
     });
   });
 });
+
+describe("syncGitLog", () => {
+  beforeEach(() => {
+    dir = mkdtempSync(join(tmpdir(), "hs-gitlog-sync-"));
+    initRepo(dir);
+  });
+
+  it("never enumerates or deletes foreign git-log documents from a shared bank", async () => {
+    execFileSync("git", ["-C", dir, "commit", "--allow-empty", "-m", "feat: current repo"]);
+    const listDocumentIds = vi.fn(
+      async (_tag: string, _tagsMatch?: "all" | "all_strict") => new Set(["gitlog:foreign-repo"])
+    );
+    const retain = vi.fn().mockResolvedValue(undefined);
+    const deleteDocument = vi.fn().mockResolvedValue(undefined);
+    const client = {
+      listDocumentIds,
+      retain,
+      deleteDocument,
+      opIds: [],
+    } as unknown as HindsightClient;
+
+    const failures = await syncGitLog(client, dir, { limit: 10 });
+
+    expect(failures).toBe(0);
+    expect(retain).toHaveBeenCalledTimes(1);
+    expect(listDocumentIds).toHaveBeenCalledTimes(1);
+    expect(listDocumentIds.mock.calls[0][0]).toMatch(/^gitlog-head:/);
+    expect(listDocumentIds.mock.calls[0][1]).toBe("all_strict");
+    expect(listDocumentIds).not.toHaveBeenCalledWith("source:git-log");
+    expect(deleteDocument).not.toHaveBeenCalled();
+  });
+
+  it("skips the upsert only when this repository's canonical document has the current HEAD tag", async () => {
+    execFileSync("git", ["-C", dir, "commit", "--allow-empty", "-m", "feat: current repo"]);
+    const listDocumentIds = vi.fn(
+      async (_tag: string, _tagsMatch?: "all" | "all_strict") =>
+        new Set([`gitlog:${repoNameOf(dir)}`])
+    );
+    const retain = vi.fn().mockResolvedValue(undefined);
+    const client = { listDocumentIds, retain, opIds: [] } as unknown as HindsightClient;
+
+    const failures = await syncGitLog(client, dir, { limit: 10 });
+
+    expect(failures).toBe(0);
+    expect(listDocumentIds.mock.calls[0][0]).toMatch(/^gitlog-head:/);
+    expect(listDocumentIds.mock.calls[0][1]).toBe("all_strict");
+    expect(retain).not.toHaveBeenCalled();
+  });
+});

hindsight-integrations/coding-agents/src/core/git.ts

@@ -254,3 +254,33 @@ export async function ingestGitLog(
     return 1;
   }
 }
+
+/**
+ * Ensure this repository's canonical aggregated git-log document is current.
+ *
+ * Older deepen versions enumerated every `source:git-log` document in the bank and deleted every
+ * non-canonical id. A bank can be shared by unrelated repositories, so that name comparison was
+ * not an ownership check and could cascade-delete foreign memories (#3877). The canonical retain is
+ * already an idempotent upsert; ambiguous legacy documents are deliberately left untouched.
+ */
+export async function syncGitLog(
+  client: HindsightClient,
+  repo: string,
+  opts: { limit: number; log?: (m: string) => void; stampFor?: () => RetainStamp }
+): Promise<number> {
+  const log = opts.log ?? (() => {});
+  const head = gitHeadSha(repo);
+  const canonical = `gitlog:${repoNameOf(repo)}`;
+  const current =
+    head !== null &&
+    (
+      await client
+        .listDocumentIds(`gitlog-head:${head}`, "all_strict")
+        .catch(() => new Set<string>())
+    ).has(canonical);
+  if (current) {
+    log("[gitlog] current with HEAD — skipping");
+    return 0;
+  }
+  return ingestGitLog(client, repo, opts);
+}

hindsight-integrations/coding-agents/src/core/hindsight.test.ts

@@ -33,6 +33,46 @@ describe("HindsightClient.maxParallelRetains", () => {
   });
 });
 
+describe("HindsightClient document-list safety", () => {
+  it("uses strict strategy-tag matching on every page", async () => {
+    const client = new HindsightClient({ apiUrl: "http://x", bank: "shared-bank" });
+    const firstPage = Array.from({ length: 500 }, (_, i) => ({ id: `git:${i}` }));
+    const fetchMock = vi.fn(async (_url: string | URL | Request) => {
+      const offset = String(_url).includes("offset=500") ? 500 : 0;
+      return jsonResponse(200, {
+        items: offset === 0 ? firstPage : [{ id: "git:500" }],
+        total: 501,
+      });
+    });
+    vi.stubGlobal("fetch", fetchMock);
+
+    const ids = await client.listDocumentIds("source:git", "all_strict");
+
+    expect(ids.size).toBe(501);
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+    expect(String(fetchMock.mock.calls[0][0])).toBe(
+      "http://x/v1/default/banks/shared-bank/documents?tags=source%3Agit&tags_match=all_strict&limit=500&offset=0"
+    );
+    expect(String(fetchMock.mock.calls[1][0])).toBe(
+      "http://x/v1/default/banks/shared-bank/documents?tags=source%3Agit&tags_match=all_strict&limit=500&offset=500"
+    );
+  });
+
+  it("preserves the inclusive all mode for existing callers that do not opt into strict matching", async () => {
+    const client = new HindsightClient({ apiUrl: "http://x", bank: "shared-bank" });
+    const fetchMock = vi.fn(async (_url: string | URL | Request) =>
+      jsonResponse(200, { items: [], total: 0 })
+    );
+    vi.stubGlobal("fetch", fetchMock);
+
+    await client.listDocumentIds("custom:scope");
+
+    expect(String(fetchMock.mock.calls[0][0])).toContain(
+      "tags=custom%3Ascope&tags_match=all&limit=500&offset=0"
+    );
+  });
+});
+
 describe("HindsightClient.drain", () => {
   it("polls at most maxParallelRetains ops concurrently", async () => {
     const cap = 2;

hindsight-integrations/coding-agents/src/core/hindsight.ts

@@ -319,11 +319,14 @@ export class HindsightClient {
    * Set. Powers the incremental git-sync's "what's already ingested?" check — since git commits are stored
    * with document_id `git:<sha>`, the returned Set lets a caller diff a ref's commits against memory.
    */
-  async listDocumentIds(tag: string): Promise<Set<string>> {
+  async listDocumentIds(
+    tag: string,
+    tagsMatch: "all" | "all_strict" = "all"
+  ): Promise<Set<string>> {
     const ids = new Set<string>();
     const limit = 500;
     for (let offset = 0; ; offset += limit) {
-      const q = `?tags=${encodeURIComponent(tag)}&tags_match=all&limit=${limit}&offset=${offset}`;
+      const q = `?tags=${encodeURIComponent(tag)}&tags_match=${tagsMatch}&limit=${limit}&offset=${offset}`;
       const r = await this.req("GET", this.bankUrl(`/documents${q}`));
       let items: { id?: string }[] = [];
       let total = 0;
@@ -389,7 +392,8 @@ export class HindsightClient {
     }
   }
 
-  /** Delete one document (cascades its memory units/links). Used by deepen's self-cleanup. */
+  /** Explicitly delete one document (and its cascaded memory units/links). Background sync must
+   *  never use this as a cleanup primitive: a document id alone does not prove repository ownership. */
   async deleteDocument(documentId: string): Promise<void> {
     await this.req("DELETE", this.bankUrl(`/documents/${encodeURIComponent(documentId)}`));
   }

hindsight-integrations/coding-agents/src/core/session-start.test.ts

@@ -1,4 +1,8 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
+import { execFileSync } from "node:child_process";
+import { mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 import { buildSessionStartContext, runSessionStartHook } from "./session-start";
 import { resolveConfig } from "./config";
 import { HOOK_HARNESSES } from "../harness/hook-lifecycle";
@@ -168,6 +172,35 @@ describe("buildSessionStartContext", () => {
     expect(out.systemMessage).toContain("is tracking the decisions");
   });
 
+  it("does not report git in sync from another repository's same-HEAD document", async () => {
+    const repo = mkdtempSync(join(tmpdir(), "hs-session-start-shared-bank-"));
+    try {
+      execFileSync("git", ["-C", repo, "init", "-q"]);
+      execFileSync("git", ["-C", repo, "config", "user.email", "test@example.com"]);
+      execFileSync("git", ["-C", repo, "config", "user.name", "Test User"]);
+      execFileSync("git", ["-C", repo, "commit", "--allow-empty", "-m", "initial"]);
+      const listDocumentIds = vi.fn(async (tag: string, _match?: "all" | "all_strict") =>
+        tag === "source:git" ? new Set(["git:existing"]) : new Set(["gitlog:foreign-repo"])
+      );
+
+      const out = await buildSessionStartContext({
+        cwd: repo,
+        bankId: "shared-bank",
+        cfg: resolveConfig({ codebaseSurvey: false }),
+        client: { listDocumentIds, listPages: listPagesOk },
+        hasGit: () => true,
+        startSeed: vi.fn(),
+      });
+
+      expect(out.systemMessage).toContain("catching up on new commits");
+      expect(out.systemMessage).not.toContain("git in sync");
+      expect(listDocumentIds.mock.calls[1][0]).toMatch(/^gitlog-head:/);
+      expect(listDocumentIds.mock.calls[1][1]).toBe("all_strict");
+    } finally {
+      rmSync(repo, { recursive: true, force: true });
+    }
+  });
+
   it("listDocumentIds throws (server unreachable) -> no seed, roster preamble only", async () => {
     const startSeed = vi.fn();
     const client = {

hindsight-integrations/coding-agents/src/core/session-start.ts

@@ -19,7 +19,7 @@
  *   - empty set (cold)                 -> start the background seed, seededAt written, note added
  */
 import { readFileSync } from "node:fs";
-import { gitHeadSha, hasGitHistory, commitsSince } from "./git";
+import { gitHeadSha, hasGitHistory, commitsSince, repoNameOf } from "./git";
 import { DEEPEN_DIFF_TARGET } from "./status";
 import { startBackgroundSeed } from "./seed";
 import { syncCompanionSkill } from "./skill-sync";
@@ -39,7 +39,7 @@ import { sessionCacheFile, sessionRootDir, writeSessionCache } from "./session-c
 
 /** Minimal client shape `buildSessionStartContext` needs. */
 interface SeedContextClient {
-  listDocumentIds(tag: string): Promise<Set<string>>;
+  listDocumentIds(tag: string, tagsMatch?: "all" | "all_strict"): Promise<Set<string>>;
   listPages(): Promise<unknown>;
   knowledgePagesSupported?: boolean;
   // Optional: used to write the survey-baseline marker (Option A). HindsightClient has it; the
@@ -87,8 +87,8 @@ async function gitSyncNote(args: {
   const head = gitHeadSha(cwd);
   if (!head) return undefined;
   const gitlogCurrent = await client
-    .listDocumentIds(`gitlog-head:${head}`)
-    .then((s) => s.size > 0)
+    .listDocumentIds(`gitlog-head:${head}`, "all_strict")
+    .then((s) => s.has(`gitlog:${repoNameOf(cwd)}`))
     .catch(() => undefined);
   if (gitlogCurrent === undefined) return undefined; // server hiccup: say nothing rather than guess
   if (mode === "message") return gitlogCurrent ? "git in sync" : "catching up on new commits";
@@ -210,7 +210,7 @@ export async function buildSessionStartContext(args: {
       {
         let docIds: Set<string> | undefined;
         try {
-          docIds = await client.listDocumentIds("source:git");
+          docIds = await client.listDocumentIds("source:git", "all_strict");
         } catch {
           docIds = undefined; // server unreachable: transient — do nothing, try again next session
         }
@@ -250,7 +250,7 @@ export async function buildSessionStartContext(args: {
             const sha = resolveHeadSha(cwd);
             if (sha) {
               const markers = await client
-                .listDocumentIds(SURVEY_BASELINE_TAG)
+                .listDocumentIds(SURVEY_BASELINE_TAG, "all_strict")
                 .catch(() => new Set<string>());
               const counts: number[] = [];
               for (const id of markers) {
@@ -262,7 +262,7 @@ export async function buildSessionStartContext(args: {
               // A baseline without FINDINGS means the surveyed agent died before ingesting (no
               // CLI on PATH, budget kill) — the marker alone must not suppress retries forever.
               const uploads = await client
-                .listDocumentIds("source:upload")
+                .listDocumentIds("source:upload", "all_strict")
                 .catch(() => new Set<string>());
               const findingsAbsent =
                 counts.length > 0 && !SURVEY_DOC_IDS.some((id) => uploads.has(id));

hindsight-integrations/coding-agents/src/core/status.ts

@@ -21,7 +21,7 @@ import { SURVEY_DOC_IDS } from "./survey";
 
 /** Minimal client shape (HindsightClient satisfies it structurally). */
 export interface StatusClient {
-  listDocumentIds(tag: string): Promise<Set<string>>;
+  listDocumentIds(tag: string, tagsMatch?: "all" | "all_strict"): Promise<Set<string>>;
   listPages(): Promise<unknown>;
   knowledgePagesSupported?: boolean;
   activeOperations(): Promise<number>;
@@ -61,11 +61,15 @@ export async function syncStatus(
   bank: string,
   repoDir?: string
 ): Promise<SyncStatus> {
-  const gitIds = await client.listDocumentIds("source:git");
-  const chatIds = await client.listDocumentIds("source:chat").catch(() => new Set<string>());
+  const gitIds = await client.listDocumentIds("source:git", "all_strict");
+  const chatIds = await client
+    .listDocumentIds("source:chat", "all_strict")
+    .catch(() => new Set<string>());
   const pages = parsePageList(await client.listPages().catch(() => null));
   const activeOps = await client.activeOperations().catch(() => null);
-  const uploads = await client.listDocumentIds("source:upload").catch(() => new Set<string>());
+  const uploads = await client
+    .listDocumentIds("source:upload", "all_strict")
+    .catch(() => new Set<string>());
   const surveyDocs = SURVEY_DOC_IDS.filter((id) => uploads.has(id)).length;
 
   // Survey observability: the survey-baseline:<sha> markers Chris's re-survey mechanism writes.
@@ -74,7 +78,7 @@ export async function syncStatus(
   let surveyCommitsBehind: number | null = null;
   if (repoDir) {
     try {
-      const markers = await client.listDocumentIds("source:survey-baseline");
+      const markers = await client.listDocumentIds("source:survey-baseline", "all_strict");
       let best: { sha: string; behind: number } | undefined;
       for (const id of markers) {
         const sha = id.replace(/^survey-baseline:/, "");

hindsight-integrations/coding-agents/src/core/sync.ts

@@ -73,7 +73,7 @@ export async function syncGit(
   const shas = (revs ? revs.split("\n") : []).filter(Boolean);
   if (!shas.length) return { ref, total: 0, ingested: 0, failures: 0, inSync: true };
 
-  const ingestedIds = await client.listDocumentIds("source:git"); // Set of `git:<sha>` already in the bank
+  const ingestedIds = await client.listDocumentIds("source:git", "all_strict"); // Set of `git:<sha>` already in the bank
   const missing = shas.filter((sha) => !ingestedIds.has(`git:${sha}`));
   if (!missing.length) {
     log(`[sync] in sync — all ${shas.length} commits on ${ref} already ingested`);

hindsight-integrations/coding-agents/src/deepen.ts

@@ -26,7 +26,7 @@ import { join } from "node:path";
 import { deriveBankId } from "./core/bank";
 import { ingestChats } from "./core/chat";
 import { applyBankConfig, loadConfig } from "./core/config";
-import { commitsSince, gitHeadSha, ingestGitLog, repoNameOf, retainCommit } from "./core/git";
+import { commitsSince, repoNameOf, retainCommit, syncGitLog } from "./core/git";
 import { SURVEY_DOC_IDS } from "./core/survey";
 import { buildPageTrigger } from "./core/missions";
 import { HindsightClient } from "./core/hindsight";
@@ -159,7 +159,7 @@ async function main() {
       });
     }
 
-    const gitIds = await client.listDocumentIds("source:git");
+    const gitIds = await client.listDocumentIds("source:git", "all_strict");
 
     // chats FIRST: few, and they carry the decisions that make memory necessary — never starved
     // behind the git flood. Dedup against what's already in the bank (chat:<id>).
@@ -172,7 +172,9 @@ async function main() {
     if (!cfg.retainSessions) {
       log("[chat] retainSessions: false — skipping conversation import");
     } else {
-      const chatIds = await client.listDocumentIds("source:chat").catch(() => new Set<string>());
+      const chatIds = await client
+        .listDocumentIds("source:chat", "all_strict")
+        .catch(() => new Set<string>());
       const all = await harness.chatReader.read({ conversations: CONV, repo: REPO });
       sessions = all.filter((s, i) => !chatIds.has(`chat:${s.id || `s${i}`}`));
       if (all.length !== sessions.length)
@@ -197,30 +199,7 @@ async function main() {
     if (GIT_INGEST === "none") {
       log("[git] gitIngest=none — git ingestion disabled");
     } else {
-      const head = gitHeadSha(REPO!);
-      const gitlogCurrent =
-        head !== null &&
-        (await client.listDocumentIds(`gitlog-head:${head}`).catch(() => new Set())).size > 0;
-      if (gitlogCurrent) {
-        log("[gitlog] current with HEAD — skipping");
-      } else {
-        gitFails += await ingestGitLog(client, REPO!, { limit: GITLOG_LIMIT, log, stampFor });
-      }
-      // Self-cleanup: earlier versions named the gitlog doc per WORKTREE (gitlog:my-repo-wt2 …),
-      // duplicating the history in the shared bank. Delete any gitlog doc that isn't the
-      // canonical (worktree-aware) id.
-      try {
-        const canonical = `gitlog:${repoNameOf(REPO!)}`;
-        const logDocs = await client.listDocumentIds("source:git-log");
-        for (const id of logDocs) {
-          if (id !== canonical) {
-            await client.deleteDocument(id);
-            log(`[gitlog] removed stale duplicate ${id} (canonical: ${canonical})`);
-          }
-        }
-      } catch {
-        /* cleanup is best-effort */
-      }
+      gitFails += await syncGitLog(client, REPO!, { limit: GITLOG_LIMIT, log, stampFor });
 
       if (GIT_INGEST === "full") {
         // progressive depth: next batch of un-ingested commits, newest first, full message + diff.
@@ -259,11 +238,13 @@ async function main() {
     // baseline marker from "researching…" to "completed" (lazy — the detached survey agent can't
     // reliably do it itself). The `survey-state:done` tag makes this a one-time upsert.
     try {
-      const uploads = await client.listDocumentIds("source:upload").catch(() => new Set<string>());
+      const uploads = await client
+        .listDocumentIds("source:upload", "all_strict")
+        .catch(() => new Set<string>());
       if (SURVEY_DOC_IDS.some((id) => uploads.has(id))) {
-        const markers = await client.listDocumentIds("source:survey-baseline");
+        const markers = await client.listDocumentIds("source:survey-baseline", "all_strict");
         const done = await client
-          .listDocumentIds("survey-state:done")
+          .listDocumentIds("survey-state:done", "all_strict")
           .catch(() => new Set<string>());
         let best: { id: string; sha: string; behind: number } | undefined;
         for (const id of markers) {