fix: confirm_scars must not green-light a failed retrieval (GIT-93 step 2)

src/services/session-state.ts

@@ -38,6 +38,18 @@ interface SessionContext {
    * conflict rather than on agreement.
    */
   recoveryConflict?: boolean;
+  /**
+   * GIT-93: set when a recall() attempt failed to reach the store, cleared when
+   * one succeeds.
+   *
+   * "No scars surfaced" has two causes that used to be indistinguishable:
+   * retrieval ran and found nothing relevant, or retrieval never ran. Only the
+   * first is safe to proceed on. Without this, confirm_scars answered both with
+   * "Proceed freely" — a green result for a broken store, which is how a
+   * retrieval path that had never worked survived indefinitely (the *_scar_search
+   * RPC 404'd on every call since it was written and nothing downstream said so).
+   */
+  recallFailure?: { message: string; at: string } | null;
 }
 
 // Global session state (single active session per MCP server instance)
@@ -165,6 +177,13 @@ function recoverSessionFromDisk(): SessionContext | null {
       currentSession.recallCalled = true;
     }
 
+    // GIT-93: restore an unresolved retrieval failure too. A restart must not
+    // launder a broken store into a clean slate — otherwise the very event this
+    // session state exists to survive becomes a way to lose the warning.
+    if (currentSession && data.recall_failure) {
+      currentSession.recallFailure = data.recall_failure;
+    }
+
     console.error(
       `[session-state] Recovered session ${data.session_id.slice(0, 8)} from disk after MCP restart ` +
       `(${currentSession?.surfacedScars.length ?? 0} surfaced scars)`
@@ -265,6 +284,69 @@ export function setRecallCalled(): void {
   }
 }
 
+/**
+ * GIT-93: record that a recall() attempt could not reach the store.
+ *
+ * Persisted alongside recall_called so it survives an MCP restart, for the same
+ * reason: the failure outlives the process that observed it, and a restart must
+ * not turn a broken store into an apparently clean one.
+ */
+export function setRecallFailure(message: string): void {
+  const session = resolveCurrentSession();
+  if (!session) return;
+
+  const failure = { message, at: new Date().toISOString() };
+  session.recallFailure = failure;
+  console.error(`[session-state] recall() failure recorded: ${message.slice(0, 120)}`);
+
+  try {
+    const sessionFilePath = getSessionPath(session.sessionId, "session.json");
+    if (!fs.existsSync(sessionFilePath)) return;
+    const data = JSON.parse(fs.readFileSync(sessionFilePath, "utf-8"));
+    data.recall_failure = failure;
+    fs.writeFileSync(sessionFilePath, JSON.stringify(data, null, 2));
+  } catch (error) {
+    // Non-fatal: the flag still holds for this process.
+    console.warn("[session-state] Failed to persist recall_failure:", error);
+  }
+}
+
+/**
+ * GIT-93: clear the failure marker after a recall() that reached the store.
+ *
+ * Cleared on success rather than on attempt, and regardless of how many scars
+ * came back: a successful search returning nothing is a real answer, while a
+ * failed one is not an answer at all. Only the former should let confirm_scars
+ * say "proceed".
+ */
+export function clearRecallFailure(): void {
+  const session = resolveCurrentSession();
+  if (!session || !session.recallFailure) return;
+
+  session.recallFailure = null;
+
+  try {
+    const sessionFilePath = getSessionPath(session.sessionId, "session.json");
+    if (!fs.existsSync(sessionFilePath)) return;
+    const data = JSON.parse(fs.readFileSync(sessionFilePath, "utf-8"));
+    if (!data.recall_failure) return; // nothing recorded — no write
+    delete data.recall_failure;
+    fs.writeFileSync(sessionFilePath, JSON.stringify(data, null, 2));
+  } catch (error) {
+    console.warn("[session-state] Failed to clear recall_failure:", error);
+  }
+}
+
+/**
+ * GIT-93: the unresolved retrieval failure for this session, if any.
+ *
+ * Returns null when retrieval last succeeded — which is the only state in which
+ * an empty scar set means "nothing relevant" rather than "nothing was asked".
+ */
+export function getRecallFailure(): { message: string; at: string } | null {
+  return resolveCurrentSession()?.recallFailure ?? null;
+}
+
 /**
  * Check if recall() was called this session.
  * Used by enforcement to avoid false positives when recall returns 0 scars.

src/tools/confirm-scars.ts

@@ -21,6 +21,7 @@ import {
   getSurfacedScars,
   addConfirmations,
   getConfirmations,
+  getRecallFailure,
 } from "../services/session-state.js";
 import { Timer, buildPerformanceData } from "../services/metrics.js";
 import { getSessionPath } from "../services/gitmem-dir.js";
@@ -185,6 +186,39 @@ export async function confirmScars(params: ConfirmScarsParams): Promise<ConfirmS
 
   if (recallScars.length === 0) {
     const performance = buildPerformanceData("confirm_scars", timer.elapsed(), 0);
+
+    // GIT-93: an empty scar set has two causes and they are not interchangeable.
+    // Retrieval ran and matched nothing — safe to proceed. Or retrieval never
+    // reached the store, in which case nothing has been checked and "proceed
+    // freely" is an assurance nobody earned. This used to answer both the same
+    // way, which is how a retrieval path that 404'd on every call since it was
+    // written went unnoticed: the system reported success for a failure.
+    const failure = getRecallFailure();
+    if (failure) {
+      const failedMsg = [
+        `${STATUS.rejected} Cannot confirm — institutional memory was not reached.`,
+        "",
+        `recall() failed at ${failure.at}: ${failure.message}`,
+        "",
+        "This is NOT the same as no relevant scars. Nothing was checked, so any",
+        "warning that applies to this work is still unseen. Treat it as memory",
+        "being unavailable, not as a clean bill of health.",
+        "",
+        "Retry recall(). If it keeps failing, say so rather than proceeding as",
+        "though the check had passed.",
+      ].join("\n");
+
+      return {
+        valid: false,
+        errors: [`recall() failed — institutional memory unavailable: ${failure.message}`],
+        confirmations: [],
+        missing_scars: [],
+        formatted_response: failedMsg,
+        display: wrapDisplay(failedMsg),
+        performance,
+      };
+    }
+
     const noScarsMsg = `${STATUS.ok} No recall-surfaced scars to confirm. Proceed freely.`;
     return {
       valid: true,

src/tools/recall.ts

@@ -32,7 +32,7 @@ import {
   formatVariantEnforcement,
   type ScarWithVariant,
 } from "../services/variant-assignment.js";
-import { addSurfacedScars, getCurrentSession, setRecallCalled } from "../services/session-state.js";
+import { addSurfacedScars, getCurrentSession, setRecallCalled, setRecallFailure, clearRecallFailure } from "../services/session-state.js";
 import { getAgentIdentity } from "../services/agent-detection.js";
 import { v4 as uuidv4 } from "uuid";
 import { wrapDisplay, productLine, SEV, boldText, dimText, ANSI, CITATION_LINE } from "../services/display-protocol.js";
@@ -443,6 +443,9 @@ export async function recall(params: RecallParams): Promise<RecallResult> {
       const rawScars = await getStorage().search(plan, matchCount);
       const searchLatencyMs = searchTimer.stop();
 
+      // GIT-93: free-tier local search also answered — same rule as the Pro path.
+      clearRecallFailure();
+
       const scars: FormattedScar[] = rawScars
         .map((scar) => ({
           id: scar.id,
@@ -563,6 +566,11 @@ export async function recall(params: RecallParams): Promise<RecallResult> {
 
     const searchLatencyMs = searchTimer.stop();
 
+    // GIT-93: the store answered. Clear any earlier failure regardless of how
+    // many scars came back — a search that ran and matched nothing is a real
+    // answer, and only that state may license confirm_scars to say "proceed".
+    clearRecallFailure();
+
     // Assign variants for A/B testing (dev tier only)
     // Agent identity is always available, so variants are always assigned
     const variantTimer = new Timer();
@@ -746,6 +754,12 @@ export async function recall(params: RecallParams): Promise<RecallResult> {
     const message = error instanceof Error ? error.message : String(error);
     console.error("[recall] Search failed:", message);
 
+    // GIT-93: record the failure on the session, not just in stderr. This
+    // returns scars: [] with a warning banner, and a later confirm_scars sees
+    // only "no scars surfaced" — which it used to answer with "Proceed freely".
+    // The marker is what lets it tell an empty answer from no answer.
+    setRecallFailure(message);
+
     const latencyMs = timer.stop();
     const perfData = buildPerformanceData("recall", latencyMs, 0);
     const mainErrMsg = `⚠️ Error querying institutional memory: ${message}`;

tests/unit/tools/confirm-scars-retrieval-failure.test.ts

@@ -0,0 +1,143 @@
+/**
+ * GIT-93 step 2: a failed retrieval must not read as a clean check.
+ *
+ * confirm_scars answered an empty scar set with "No recall-surfaced scars to
+ * confirm. Proceed freely." — regardless of why the set was empty. Two very
+ * different states produced that identical green result:
+ *
+ *   1. recall() ran, reached the store, matched nothing. Proceeding is correct.
+ *   2. recall() never reached the store. Nothing was checked, and any warning
+ *      that applies is still unseen. Proceeding is a guess.
+ *
+ * The second is not hypothetical: the *_scar_search RPC 404'd on every call from
+ * the day it was written, and the reason it survived that long is that the
+ * system reported success for the failure. Fixing the RPC name without fixing
+ * this signal would leave the next such break just as well hidden.
+ *
+ * These tests drive real session state on disk — the marker has to survive a
+ * process restart, so an in-memory-only assertion would not prove much.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import * as fs from "fs";
+import * as path from "path";
+import * as os from "os";
+import { setGitmemDir, clearGitmemDirCache } from "../../../src/services/gitmem-dir.js";
+import {
+  setCurrentSession,
+  clearCurrentSession,
+  setRecallFailure,
+  clearRecallFailure,
+  getRecallFailure,
+  addSurfacedScars,
+  resolveCurrentSession,
+} from "../../../src/services/session-state.js";
+import { confirmScars } from "../../../src/tools/confirm-scars.js";
+
+const SESSION_ID = "3c9f21ab-77de-4a10-9f31-2b8c4d5e6f70";
+let root: string;
+
+function startSession(): void {
+  fs.mkdirSync(path.join(root, "sessions", SESSION_ID), { recursive: true });
+  fs.writeFileSync(
+    path.join(root, "sessions", SESSION_ID, "session.json"),
+    JSON.stringify({
+      session_id: SESSION_ID,
+      agent: "cli",
+      project: "gitmem",
+      started_at: new Date().toISOString(),
+      hostname: os.hostname(),
+      pid: process.pid,
+      host_pid: process.pid,
+    })
+  );
+  fs.writeFileSync(path.join(root, "active-sessions.json"), JSON.stringify({ sessions: [] }));
+  setCurrentSession({
+    sessionId: SESSION_ID,
+    agent: "cli",
+    project: "gitmem",
+    startedAt: new Date(),
+  });
+}
+
+describe("GIT-93: confirm_scars distinguishes an empty answer from no answer", () => {
+  beforeEach(() => {
+    root = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git93-"));
+    clearGitmemDirCache();
+    setGitmemDir(root);
+    clearCurrentSession();
+    startSession();
+  });
+
+  afterEach(() => {
+    clearCurrentSession();
+    clearGitmemDirCache();
+    fs.rmSync(root, { recursive: true, force: true });
+  });
+
+  it("still says proceed when retrieval succeeded and matched nothing", async () => {
+    // No failure recorded — recall ran and simply found nothing relevant.
+    const result = await confirmScars({ confirmations: [] });
+
+    expect(result.valid).toBe(true);
+    expect(result.formatted_response).toContain("Proceed freely");
+  });
+
+  it("refuses to say proceed when retrieval never reached the store", async () => {
+    setRecallFailure("Supabase RPC error: 404 - PGRST202");
+
+    const result = await confirmScars({ confirmations: [] });
+
+    expect(result.valid).toBe(false);
+    expect(result.formatted_response).not.toContain("Proceed freely");
+    expect(result.errors.join(" ")).toMatch(/recall\(\) failed|unavailable/i);
+  });
+
+  it("names the underlying failure rather than reporting a generic error", async () => {
+    setRecallFailure("Supabase RPC error: 404 - PGRST202");
+
+    const result = await confirmScars({ confirmations: [] });
+
+    // A diagnosable message is the difference between this being noticed in a
+    // day and being noticed never.
+    expect(result.formatted_response).toContain("PGRST202");
+  });
+
+  it("says proceed again once a later recall reaches the store", async () => {
+    setRecallFailure("transient network error");
+    clearRecallFailure();
+
+    const result = await confirmScars({ confirmations: [] });
+
+    expect(result.valid).toBe(true);
+    expect(result.formatted_response).toContain("Proceed freely");
+  });
+
+  it("keeps the failure marker across an MCP restart", () => {
+    setRecallFailure("Supabase RPC error: 404 - PGRST202");
+
+    // Simulate the restart: in-memory state dies, disk survives.
+    clearCurrentSession();
+    const recovered = resolveCurrentSession();
+
+    expect(recovered?.sessionId).toBe(SESSION_ID);
+    // A restart must not launder a broken store into a clean slate.
+    expect(getRecallFailure()?.message).toContain("PGRST202");
+  });
+
+  it("does not suppress real surfaced scars when a stale failure is present", async () => {
+    // A failure followed by a successful recall that surfaced scars: the scars
+    // are what matter, and the normal confirmation path must still run.
+    setRecallFailure("earlier transient failure");
+    clearRecallFailure();
+    addSurfacedScars([
+      { scar_id: "abc12345", title: "some scar", severity: "high", source: "recall" },
+    ] as never);
+
+    const result = await confirmScars({ confirmations: [] });
+
+    // Missing confirmations for a surfaced scar is its own rejection — the
+    // point here is that it is NOT the retrieval-failure path.
+    expect(result.formatted_response).not.toContain("institutional memory was not reached");
+  });
+});

tests/unit/tools/provenance-citation.test.ts

@@ -156,6 +156,10 @@ vi.mock("../../../src/services/session-state.js", () => ({
   getCurrentSession: vi.fn(() => null),
   addSurfacedScars: vi.fn(),
   setRecallCalled: vi.fn(),
+  // GIT-93: recall records/clears a retrieval-failure marker so confirm_scars
+  // can tell "found nothing" from "never reached the store".
+  setRecallFailure: vi.fn(),
+  clearRecallFailure: vi.fn(),
 }));
 
 vi.mock("../../../src/services/agent-detection.js", () => ({