From 7988be71d46f189e9122c29efe27368b8d9f80f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:02:47 -0700 Subject: [PATCH 01/24] fix: never resume, displace or supersede a session of another project (GIT-86) One desktop process serves every chat, so hostname+pid does not name a session. session_start(Y) found the open X session by hostname+pid, replaced the caller's project with X's (stderr only) and carried on in X. - findSessionByHostPid and findResumableSessionOnDisk take the requested project and only return a session of that project. - params.project set and no match: a new session. The registry slot is now hostname+pid+project, so registerSession no longer displaces X, X is not marked superseded, and pruneOrphanedDirs no longer sees X as unregistered. - params.project absent: the most recent session is resumed, and the display names its project (project_from_resumed_session). - force:true carries startedAt, surfaced scars, confirmations, observations and children forward only from a session of the same project. Confirmations were never carried before, and the free tier dropped the carried startedAt. - session_close's registry fallback looks the session up by id. +9 unit tests (4 registry, 5 through session_start); 8 fail on origin/main. Co-Authored-By: Claude Opus 5 --- src/services/active-sessions.ts | 50 +++++-- src/services/session-state.ts | 4 +- src/tools/session-close.ts | 8 +- src/tools/session-start.ts | 64 +++++--- src/types/index.ts | 5 + tests/unit/services/active-sessions.test.ts | 35 +++++ .../tools/session-start-cross-project.test.ts | 141 ++++++++++++++++++ 7 files changed, 269 insertions(+), 38 deletions(-) create mode 100644 tests/unit/tools/session-start-cross-project.test.ts diff --git a/src/services/active-sessions.ts b/src/services/active-sessions.ts index 9be4ac0..6f3ad0b 100644 --- a/src/services/active-sessions.ts +++ b/src/services/active-sessions.ts @@ -126,25 +126,38 @@ function writeRegistry(registry: ActiveSessionsRegistry): void { // --- CRUD operations --- +/** + * GIT-86: the slot a session occupies in the registry is hostname+pid+project. + * + * One desktop process serves every chat, so hostname+pid alone does not name a + * session — two chats in two projects share it. Keying the slot on pid alone + * let session_start(Y) displace an open X session, mark it superseded, and + * leave its directory for pruneOrphanedDirs to delete. + */ +function sameSlot(a: ActiveSessionEntry, b: ActiveSessionEntry): boolean { + return a.hostname === b.hostname && a.pid === b.pid && a.project === b.project; +} + /** * Register a new session in the active-sessions registry. * Idempotent: re-registering the same session_id replaces the entry. - * Returns session IDs that were displaced (different session_id, same hostname+pid). + * Returns session IDs that were displaced (different session_id, same + * hostname+pid+project). Sessions of other projects on the same process are + * left open (GIT-86). */ export function registerSession(entry: ActiveSessionEntry): string[] { const displaced: string[] = []; withLockSync(getLockPath(), () => { const registry = readRegistry(); - // Collect displaced session IDs (same hostname+pid, different session_id) + // Collect displaced session IDs (same slot, different session_id) for (const s of registry.sessions) { - if (s.session_id !== entry.session_id && s.hostname === entry.hostname && s.pid === entry.pid) { + if (s.session_id !== entry.session_id && sameSlot(s, entry)) { displaced.push(s.session_id); } } - // Remove by session_id AND by hostname+pid to prevent duplicates + // Remove by session_id AND by slot to prevent duplicates registry.sessions = registry.sessions.filter((s) => - s.session_id !== entry.session_id && - !(s.hostname === entry.hostname && s.pid === entry.pid) + s.session_id !== entry.session_id && !sameSlot(s, entry) ); registry.sessions.push(entry); writeRegistry(registry); @@ -197,10 +210,16 @@ export function listActiveSessions(): ActiveSessionEntry[] { /** * Find a session by hostname and PID. * Used by session_start to detect if this process already has a registered session. + * + * GIT-86: with `project`, only a session of that project matches — a process + * can hold one open session per project. Without it, the most recently started + * session of the process is returned. */ -export function findSessionByHostPid(hostname: string, pid: number): ActiveSessionEntry | null { +export function findSessionByHostPid(hostname: string, pid: number, project?: string): ActiveSessionEntry | null { const registry = readRegistry(); - return registry.sessions.find((s) => s.hostname === hostname && s.pid === pid) || null; + return registry.sessions + .filter((s) => s.hostname === hostname && s.pid === pid && (project === undefined || s.project === project)) + .sort((a, b) => new Date(b.started_at).getTime() - new Date(a.started_at).getTime())[0] || null; } /** @@ -264,7 +283,7 @@ function toAgentIdentity(value: unknown): AgentIdentity { return AGENT_IDENTITIES.find((id) => id === normalized) ?? "Unknown"; } -export function findResumableSessionOnDisk(): ActiveSessionEntry | null { +export function findResumableSessionOnDisk(project?: string): ActiveSessionEntry | null { const currentHostname = os.hostname(); const currentPid = process.pid; const gitmemDir = getGitmemDir(); @@ -304,6 +323,11 @@ export function findResumableSessionOnDisk(): ActiveSessionEntry | null { const age = now - new Date(startedAt).getTime(); if (!Number.isFinite(age) || age > STALE_THRESHOLD_MS) continue; + // GIT-86: a session of another project is never the answer to a request + // for this one, whatever its pid says. + const sessionProject = typeof data.project === "string" ? data.project : "default"; + if (project !== undefined && sessionProject !== project) continue; + const pid = typeof data.pid === "number" ? data.pid : -1; candidates.push({ entry: { @@ -312,7 +336,7 @@ export function findResumableSessionOnDisk(): ActiveSessionEntry | null { started_at: startedAt, hostname: currentHostname, pid, - project: typeof data.project === "string" ? data.project : "default", + project: sessionProject, }, pidAlive: pid > 0 && pid !== currentPid && isPidAlive(pid), }); @@ -331,7 +355,7 @@ export function findResumableSessionOnDisk(): ActiveSessionEntry | null { } // 2. Orphaned by a restart. Adopt at most one — rebinding every dead-PID - // session would leave several rows sharing hostname+pid. + // session would leave several rows sharing hostname+pid+project. const orphaned = candidates.filter((c) => !c.pidAlive).sort(newestFirst)[0]; if (!orphaned) return null; @@ -382,9 +406,7 @@ function reconcileRegistryEntry(entry: ActiveSessionEntry): void { return; // already agrees — no write } registry.sessions = registry.sessions.filter( - (s) => - s.session_id !== entry.session_id && - !(s.hostname === entry.hostname && s.pid === entry.pid) + (s) => s.session_id !== entry.session_id && !sameSlot(s, entry) ); registry.sessions.push(entry); writeRegistry(registry); diff --git a/src/services/session-state.ts b/src/services/session-state.ts index 7d342de..fc021bf 100644 --- a/src/services/session-state.ts +++ b/src/services/session-state.ts @@ -78,12 +78,12 @@ let lastFailedRecoveryFingerprint: number | null | undefined = undefined; * Set the current active session * Called by session_start */ -export function setCurrentSession(context: Omit & { surfacedScars?: SurfacedScar[]; observations?: Observation[]; children?: SessionChild[]; threads?: ThreadObject[] }): void { +export function setCurrentSession(context: Omit & { surfacedScars?: SurfacedScar[]; confirmations?: ScarConfirmation[]; observations?: Observation[]; children?: SessionChild[]; threads?: ThreadObject[] }): void { currentSession = { ...context, recallCalled: false, surfacedScars: context.surfacedScars || [], - confirmations: [], + confirmations: context.confirmations || [], reflections: [], observations: context.observations || [], children: context.children || [], diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index 2388a4c..d19f229 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -43,7 +43,7 @@ import * as fs from "fs"; import * as path from "path"; import * as os from "os"; import { getGitmemPath, getGitmemDir, getSessionPath, getSessionDir } from "../services/gitmem-dir.js"; -import { unregisterSession, findSessionByHostPid } from "../services/active-sessions.js"; +import { unregisterSession, findSessionByHostPid, findSessionById } from "../services/active-sessions.js"; import { loadSuggestions, saveSuggestions, detectSuggestedThreads, loadRecentSessionEmbeddings } from "../services/thread-suggestions.js"; import { writeAgentBriefing } from "../services/agent-briefing.js"; import type { @@ -1285,8 +1285,10 @@ export async function sessionClose( // Fall back to active-sessions registry as last resort if (!existingSession) { try { - const mySession = findSessionByHostPid(os.hostname(), process.pid); - if (mySession && mySession.session_id === sessionId) { + // GIT-86: look up by id — a process can hold one session per project, + // so "this process's session" is not a single entry. + const mySession = findSessionById(sessionId); + if (mySession && mySession.hostname === os.hostname() && mySession.pid === process.pid) { existingSession = { id: sessionId, session_date: mySession.started_at?.split("T")[0] || new Date().toISOString().split("T")[0], diff --git a/src/tools/session-start.ts b/src/tools/session-start.ts index 28ffedd..62302d4 100644 --- a/src/tools/session-start.ts +++ b/src/tools/session-start.ts @@ -42,7 +42,7 @@ import * as os from "os"; import { formatDate } from "../services/timezone.js"; import { productLine, dimText, boldText } from "../services/display-protocol.js"; // Suggested threads removed from start display -import type { PerformanceBreakdown, ComponentPerformance, SurfacedScar, Observation, SessionChild } from "../types/index.js"; +import type { PerformanceBreakdown, ComponentPerformance, SurfacedScar, ScarConfirmation, Observation, SessionChild } from "../types/index.js"; import { querySessionsByDateRange, queryScarUsageByDateRange, @@ -515,7 +515,7 @@ async function sessionStartFree( metricsId: string, existingSessionId?: string, existingStartedAt?: Date, - forceCarryActivity?: { surfacedScars: SurfacedScar[]; observations: Observation[]; children: SessionChild[] }, + forceCarryActivity?: { surfacedScars: SurfacedScar[]; confirmations: ScarConfirmation[]; observations: Observation[]; children: SessionChild[] }, ): Promise { const storage = getStorage(); const isResuming = !!existingSessionId; @@ -613,7 +613,8 @@ async function sessionStartFree( // writeSessionFiles merges with existing file threads to preserve mid-session creations let freeMergedThreads = freeAggregatedThreads; try { - freeMergedThreads = writeSessionFiles(sessionId, agent, project, surfacedScars, freeAggregatedThreads, undefined, false, false, isResuming ? existingStartedAt : undefined); + // existingStartedAt is set only on resume or a same-project force carry-forward. + freeMergedThreads = writeSessionFiles(sessionId, agent, project, surfacedScars, freeAggregatedThreads, undefined, false, false, existingStartedAt); } catch (error) { console.warn("[session_start] Failed to persist session files:", error); } @@ -624,8 +625,12 @@ async function sessionStartFree( sessionId, linearIssue: params.linear_issue, agent, - startedAt: (isResuming && existingStartedAt) || new Date(), + project, + // GIT-86: the force carry-forward startedAt was passed in but only honoured + // on resume, so a forced free-tier session restarted its duration clock. + startedAt: existingStartedAt || new Date(), surfacedScars: freeMergedScars, + confirmations: forceCarryActivity?.confirmations, observations: forceCarryActivity?.observations, children: forceCarryActivity?.children, threads: freeMergedThreads, @@ -636,6 +641,7 @@ async function sessionStartFree( session_id: sessionId, agent, ...(isResuming && { resumed: true }), + ...(isResuming && !params.project && { project_from_resumed_session: true }), detected_environment: env, last_session: lastSession, ...(projectState && { project_state: projectState }), @@ -714,10 +720,16 @@ function restoreSessionState( * Uses the active-sessions registry (hostname+PID) to identify THIS process's * session, preventing cross-process session theft on shared filesystems. * Falls back to legacy active-session.json for backward compatibility. + * + * GIT-86: `project` is the project the caller asked for. When set, only a + * session of that project is resumed; one desktop process serves every chat, + * so this process's other sessions belong to other conversations. When + * absent, the most recent session is resumed, whatever its project. */ function checkExistingSession( agent: AgentIdentity, - force?: boolean + force?: boolean, + project?: Project ): { sessionId: string; agent: AgentIdentity; linearIssue?: string; startedAt?: Date; project?: Project } | null { if (force) { console.error("[session_start] force=true, skipping active session guard"); @@ -736,7 +748,7 @@ function checkExistingSession( // answer — a lost or diverged registry used to make an intact session on // disk invisible, so session_start would open a second session alongside it. const mySession = - findSessionByHostPid(os.hostname(), process.pid) ?? findResumableSessionOnDisk(); + findSessionByHostPid(os.hostname(), process.pid, project) ?? findResumableSessionOnDisk(project); if (mySession) { console.error(`[session_start] Found own session in registry: ${mySession.session_id} (host: ${mySession.hostname}, pid: ${mySession.pid})`); const data = readSessionFile(mySession.session_id); @@ -913,6 +925,12 @@ function formatStartDisplay(result: SessionStartResult, displayInfoMap?: Map | null = null; if (mySession) { diff --git a/src/types/index.ts b/src/types/index.ts index 61654e7..c2f51a4 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -198,6 +198,11 @@ export interface SessionStartResult { performance?: PerformanceData; /** Whether this session was resumed from an existing active session */ resumed?: boolean; + /** + * GIT-86: no project was passed, so the resumed session's project was used. + * The display names it, so the caller can see which namespace it is in. + */ + project_from_resumed_session?: boolean; /** Whether this result is from a mid-session refresh (no new session created) */ refreshed?: boolean; /** Message explaining session state */ diff --git a/tests/unit/services/active-sessions.test.ts b/tests/unit/services/active-sessions.test.ts index 8e163a2..2452783 100644 --- a/tests/unit/services/active-sessions.test.ts +++ b/tests/unit/services/active-sessions.test.ts @@ -123,6 +123,17 @@ describe("registerSession", () => { expect(listActiveSessions()[0].session_id).toBe("22222222-2222-2222-2222-222222222222"); }); + it("does not displace a session of another project on the same hostname+pid (GIT-86)", () => { + // One desktop process serves every chat: two projects share hostname+pid. + registerSession(makeEntry({ session_id: "11111111-1111-1111-1111-111111111111", hostname: "host-a", pid: 100, project: "proj-x" })); + const displaced = registerSession( + makeEntry({ session_id: "22222222-2222-2222-2222-222222222222", hostname: "host-a", pid: 100, project: "proj-y" }) + ); + + expect(displaced).toEqual([]); + expect(listActiveSessions().map((s) => s.project).sort()).toEqual(["proj-x", "proj-y"]); + }); + it("returns empty array when no sessions displaced", () => { const entry = makeEntry(); const displaced = registerSession(entry); @@ -224,6 +235,20 @@ describe("findSessionByHostPid", () => { expect(findSessionByHostPid("test-host", 1111)).toBeNull(); }); + it("with a project, matches only a session of that project (GIT-86)", () => { + registerSession(makeEntry({ session_id: "11111111-1111-1111-1111-111111111111", hostname: "h", pid: 7, project: "proj-x" })); + + expect(findSessionByHostPid("h", 7, "proj-y")).toBeNull(); + expect(findSessionByHostPid("h", 7, "proj-x")?.session_id).toBe("11111111-1111-1111-1111-111111111111"); + }); + + it("without a project, returns the most recently started session of the process (GIT-86)", () => { + registerSession(makeEntry({ session_id: "22222222-2222-2222-2222-222222222222", hostname: "h", pid: 7, project: "proj-y", started_at: new Date().toISOString() })); + registerSession(makeEntry({ session_id: "11111111-1111-1111-1111-111111111111", hostname: "h", pid: 7, project: "proj-x", started_at: new Date(Date.now() - 60_000).toISOString() })); + + expect(findSessionByHostPid("h", 7)?.session_id).toBe("22222222-2222-2222-2222-222222222222"); + }); + it("requires both hostname AND pid to match", () => { registerSession(makeEntry({ hostname: "host-A", pid: 100 })); registerSession(makeEntry({ @@ -632,6 +657,16 @@ describe("findResumableSessionOnDisk (GIT-89)", () => { ); }); + it("with a project, never resolves a session of another project (GIT-86)", () => { + seedDiskOnlySession("11111111-1111-1111-1111-111111111111", { project: "proj-x" }); + seedDiskOnlySession("22222222-2222-2222-2222-222222222222", { project: "proj-y", pid: DEAD_PID }); + + expect(findResumableSessionOnDisk("proj-z")).toBeNull(); + expect(findResumableSessionOnDisk("proj-x")?.session_id).toBe("11111111-1111-1111-1111-111111111111"); + // proj-y's orphan is adoptable for proj-y and left alone by the proj-x lookup above. + expect(findResumableSessionOnDisk("proj-y")?.session_id).toBe("22222222-2222-2222-2222-222222222222"); + }); + it("returns null when there is nothing on disk", () => { expect(findResumableSessionOnDisk()).toBeNull(); }); diff --git a/tests/unit/tools/session-start-cross-project.test.ts b/tests/unit/tools/session-start-cross-project.test.ts new file mode 100644 index 0000000..ce158ee --- /dev/null +++ b/tests/unit/tools/session-start-cross-project.test.ts @@ -0,0 +1,141 @@ +/** + * GIT-86: session_start must never resume a session of another project. + * + * One desktop process serves every chat, so this process's registry entries + * and session directories belong to several conversations at once. Before the + * fix, session_start(Y) found X by hostname+pid, overwrote the caller's + * project with X's (stderr only) and continued the X session in the Y chat. + * + * Runs the real session_start on the free tier against a temp .gitmem root. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +vi.mock("../../../src/services/agent-detection.js", () => ({ + detectAgent: () => ({ agent: "desktop", entrypoint: "claude-desktop", docker: false, hostname: os.hostname() }), +})); + +import { sessionStart } from "../../../src/tools/session-start.js"; +import { setGitmemDir, clearGitmemDirCache } from "../../../src/services/gitmem-dir.js"; +import { resetTier } from "../../../src/services/tier.js"; +import { listActiveSessions, findResumableSessionOnDisk, pruneStale } from "../../../src/services/active-sessions.js"; +import { clearCurrentSession, getCurrentSession, addSurfacedScars, addConfirmations } from "../../../src/services/session-state.js"; + +let tmpDir: string; + +const sessionDirExists = (id: string) => fs.existsSync(path.join(tmpDir, "sessions", id, "session.json")); + +beforeEach(() => { + vi.stubEnv("GITMEM_TIER", "free"); + resetTier(); + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git86-")); + setGitmemDir(tmpDir); + clearCurrentSession(); +}); + +afterEach(() => { + clearCurrentSession(); + clearGitmemDirCache(); + resetTier(); + vi.unstubAllEnvs(); + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +describe("session_start across projects (GIT-86)", () => { + it("X open; session_start(Y) starts a new Y session and X stays resumable", async () => { + const x = await sessionStart({ project: "proj-x" }); + expect(x.project).toBe("proj-x"); + + const y = await sessionStart({ project: "proj-y" }); + expect(y.session_id).not.toBe(x.session_id); + expect(y.project).toBe("proj-y"); + expect(y.resumed).toBeUndefined(); + + // X left open: registry entry, session directory, and a prune pass keep it. + pruneStale(); + expect(sessionDirExists(x.session_id)).toBe(true); + expect(listActiveSessions().map((s) => [s.session_id, s.project]).sort()).toEqual( + [[x.session_id, "proj-x"], [y.session_id, "proj-y"]].sort() + ); + + // ...and resumable by asking for X again. + const xAgain = await sessionStart({ project: "proj-x" }); + expect(xAgain.session_id).toBe(x.session_id); + expect(xAgain.resumed).toBe(true); + expect(xAgain.project).toBe("proj-x"); + + // Y, likewise, was not displaced by resuming X. + const yAgain = await sessionStart({ project: "proj-y" }); + expect(yAgain.session_id).toBe(y.session_id); + expect(yAgain.resumed).toBe(true); + }); + + it("resumes the same project's session as before", async () => { + const first = await sessionStart({ project: "proj-x" }); + const second = await sessionStart({ project: "proj-x" }); + expect(second.session_id).toBe(first.session_id); + expect(second.resumed).toBe(true); + expect(second.project_from_resumed_session).toBeUndefined(); + }); + + it("no project passed: resumes the most recent session and names its project in the display", async () => { + await sessionStart({ project: "proj-x" }); + await new Promise((r) => setTimeout(r, 5)); // distinct started_at + const y = await sessionStart({ project: "proj-y" }); + + const bare = await sessionStart({}); + expect(bare.session_id).toBe(y.session_id); + expect(bare.resumed).toBe(true); + expect(bare.project).toBe("proj-y"); + expect(bare.project_from_resumed_session).toBe(true); + expect(bare.display).toContain("Resumed project: proj-y"); + }); + + it("the disk scan honours the requested project when the registry is lost", async () => { + const x = await sessionStart({ project: "proj-x" }); + fs.writeFileSync(path.join(tmpDir, "active-sessions.json"), JSON.stringify({ sessions: [] })); + + expect(findResumableSessionOnDisk("proj-y")).toBeNull(); + expect(findResumableSessionOnDisk("proj-x")?.session_id).toBe(x.session_id); + + const y = await sessionStart({ project: "proj-y" }); + expect(y.session_id).not.toBe(x.session_id); + expect(y.resumed).toBeUndefined(); + }); + + it("force:true carries activity forward from a same-project session only", async () => { + const scar = { scar_id: "s-1", scar_title: "t", scar_severity: "medium", surfaced_at: new Date().toISOString(), source: "recall" as const }; + + // Different project in memory: nothing carried, the X session stays open. + const conf = (id: string) => ({ scar_id: id, scar_title: "t", decision: "APPLYING" as const, evidence: "e", confirmed_at: new Date().toISOString() }); + const x = await sessionStart({ project: "proj-x" }); + addSurfacedScars([scar]); + addConfirmations([conf("s-1")]); + const xStartedAt = getCurrentSession()!.startedAt; + await new Promise((r) => setTimeout(r, 5)); + const y = await sessionStart({ project: "proj-y", force: true }); + expect(y.session_id).not.toBe(x.session_id); + expect(getCurrentSession()!.surfacedScars).toHaveLength(0); + expect(getCurrentSession()!.confirmations).toHaveLength(0); + expect(getCurrentSession()!.startedAt.getTime()).toBeGreaterThan(xStartedAt.getTime()); + expect(sessionDirExists(x.session_id)).toBe(true); + expect(listActiveSessions().some((s) => s.session_id === x.session_id)).toBe(true); + + // Same project in memory: carried as before (t-f7c2fa01). + addSurfacedScars([{ ...scar, scar_id: "s-2" }]); + addConfirmations([conf("s-2")]); + const yStartedAt = getCurrentSession()!.startedAt; + const y2 = await sessionStart({ project: "proj-y", force: true }); + expect(y2.session_id).not.toBe(y.session_id); + expect(getCurrentSession()!.surfacedScars.map((s) => s.scar_id)).toEqual(["s-2"]); + expect(getCurrentSession()!.confirmations.map((c) => c.scar_id)).toEqual(["s-2"]); + expect(getCurrentSession()!.startedAt.getTime()).toBe(yStartedAt.getTime()); + // The same-project predecessor is displaced; X is still untouched. + const ids = listActiveSessions().map((s) => s.session_id); + expect(ids).toContain(x.session_id); + expect(ids).not.toContain(y.session_id); + }); +}); From cf5e7bd9382c6c65138ff15753939a30685f0666 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:02:47 -0700 Subject: [PATCH 02/24] =?UTF-8?q?test:=20venue=20driver=20projects=20mode?= =?UTF-8?q?=20=E2=80=94=20one=20process=20serving=20two=20projects=20(GIT-?= =?UTF-8?q?86)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit session_start(X), session_start(Y), session_start(X), session_start({}), session_start(X, force) through one server over stdio, as the desktop app does. Checks the stored rows, close_compliance, the registry and the session directories. Exits 1 on 1.10.0 (Y resumes X); exits 0 with the fix. Co-Authored-By: Claude Opus 5 --- tests/e2e/blank-supabase.mjs | 70 ++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 3 deletions(-) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..81443a4 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -14,6 +14,7 @@ * Usage (build first: npm run build): * VENUE_ENV=/path/outside/repo/.env node tests/e2e/blank-supabase.mjs flow --label main --out * VENUE_ENV=/path/outside/repo/.env node tests/e2e/blank-supabase.mjs egress --label pr31 --out [--rows 250] [--usage 5] + * VENUE_ENV=/path/outside/repo/.env node tests/e2e/blank-supabase.mjs projects --label git86 --out * * flow session_start -> list_threads -> recall -> confirm_scars -> create_thread * -> list_threads -> session_close -> health, with a local threads.json that @@ -21,6 +22,10 @@ * egress bytes received from the venue on a cold start, a warm start, and around * two session_starts (GIT-98 disk cache). --usage N seeds N scars with * >=3 usage rows so refresh_scar_behavioral_scores() has work to do. + * projects one server process serving two projects, as the desktop app does (GIT-86): + * session_start(X), session_start(Y), session_start(X), session_start({}), + * session_start(X, force). A session of one project is never resumed, + * displaced or superseded by a call for another. * * VENUE_ENV is a KEY=VALUE file OUTSIDE the repo defining SUPABASE_URL, * SUPABASE_SERVICE_ROLE_KEY and VENUE_REF. Credentials are never committed. @@ -96,8 +101,8 @@ function configError(message) { console.error(`[driver] ${message}`); process.exit(2); } -if (!["flow", "egress"].includes(mode)) { - configError("usage: blank-supabase.mjs flow|egress --label --out [--rows N] [--usage N]"); +if (!["flow", "egress", "projects"].includes(mode)) { + configError("usage: blank-supabase.mjs flow|egress|projects --label --out [--rows N] [--usage N]"); } const venueEnvPath = process.env.VENUE_ENV; @@ -514,7 +519,65 @@ async function egress() { return summary; } -const result = mode === "flow" ? await flow() : await egress(); +// ---------------------------------------------------------------- PROJECTS (GIT-86) +async function projects() { + await wipeVenue(); + const X = `${PROJECT}-x`, Y = `${PROJECT}-y`; + const { home, gitmemDir } = isolatedHome(`${label}-projects`); + const ollama = await startFakeOllama(); + const netlog = join(outDir, `netlog-${label}-projects.jsonl`); + writeFileSync(netlog, ""); + const stderr = []; + const srv = await startServer({ home, gitmemDir, ollamaUrl: ollama.url, netlog, stderrSink: stderr }); + await waitQuiet(netlog, { minMs: 2000 }); + + const steps = []; + const start = async (name, args) => { + const text = await srv.call("session_start", { agent_identity: "desktop", ...args }); + const id = (text.match(UUID) || [])[0] ?? null; + const step = { name, args, session_id: id, resumed: /\bresumed\b/.test(text.split("\n")[0]), + names_project: (text.match(/Resumed project: (\S+)/) || [])[1] ?? null }; + steps.push(step); + console.log(`[driver] ${name}: ${id?.slice(0, 8)} resumed=${step.resumed}${step.names_project ? ` names=${step.names_project}` : ""}`); + await waitQuiet(netlog, { minMs: 1500, quietMs: 2000 }); + return step; + }; + const x1 = await start("X", { project: X }); + const y1 = await start("Y while X open", { project: Y }); + const x2 = await start("X again", { project: X }); + const bare = await start("no project", {}); + const xf = await start("X force (Y in memory)", { project: X, force: true }); + await srv.close(); + ollama.close(); + allVenueRequests.push(...venueReqs(readNet(netlog))); + writeFileSync(join(outDir, `stderr-${label}-projects.log`), stderr.join("")); + + const rows = await rest("GET", `gitmem_sessions?select=id,project,close_compliance&id=in.(${[x1, y1, xf].map((s) => s.session_id).join(",")})`, undefined, { Prefer: "" }).then((r) => r.json()); + const row = (id) => rows.find((r) => r.id === id); + const superseded = (id) => row(id)?.close_compliance?.close_type === "superseded"; + const registry = JSON.parse(readFileSync(join(gitmemDir, "active-sessions.json"), "utf8")).sessions; + const dirExists = (id) => existsSync(join(gitmemDir, "sessions", id, "session.json")); + + const checks = { + "Y is a new session, not X resumed": y1.session_id !== x1.session_id && !y1.resumed, + "X row stored under X": row(x1.session_id)?.project === X, + "Y row stored under Y": row(y1.session_id)?.project === Y, + "X again resumes X": x2.session_id === x1.session_id && x2.resumed, + "no project resumes the most recent session (Y)": bare.session_id === y1.session_id && bare.resumed, + "no project names the resumed project": bare.names_project === Y, + "X force is a new session": xf.session_id !== x1.session_id && xf.session_id !== y1.session_id && !xf.resumed, + "Y never superseded by an X call": !superseded(y1.session_id), + "X superseded only by the same-project force": superseded(x1.session_id) && row(x1.session_id)?.close_compliance?.superseded_by === xf.session_id, + "Y still registered and on disk": registry.some((e) => e.session_id === y1.session_id && e.project === Y) && dirExists(y1.session_id), + "no cross-project override on stderr": !/Project override on resume/.test(stderr.join("")), + }; + const failed = Object.entries(checks).filter(([, ok]) => !ok).map(([k]) => k); + const summary = { label, mode, venue: SUPABASE_URL, steps, rows, registry, checks, failed }; + writeFileSync(join(outDir, `summary-${label}-projects.json`), JSON.stringify(summary, null, 2)); + return summary; +} + +const result = mode === "flow" ? await flow() : mode === "projects" ? await projects() : await egress(); const failureCheck = checkFailures(allVenueRequests); // Invariants the edge-function removal (GIT-97) guarantees, whatever the status code. const edgeCalls = allVenueRequests.filter((r) => r.path.startsWith("/functions/v1/")) @@ -531,6 +594,7 @@ if (mode === "flow" && !result.thread_counts_match) { `thread panel mismatch (GIT-97): session_start ${result.session_start_thread_count} != list_threads ${result.list_threads_open_right_after_session_start}` ); } +if (mode === "projects") failureCheck.unexpected.push(...result.failed.map((f) => `cross-project resume (GIT-86): ${f}`)); writeFileSync(join(outDir, `failure-check-${label}-${mode}.json`), JSON.stringify(failureCheck, null, 2)); console.log(JSON.stringify({ ...result, health_text: undefined, stderr_error_lines: undefined, failure_check: failureCheck }, null, 2)); if (failureCheck.unexpected.length > 0) { From e8268baf60a764f91455dab7af525112bdae53aa Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:11:54 -0700 Subject: [PATCH 03/24] fix: sync only the rows that changed when the learnings fingerprint misses (GIT-98) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a Pro store the fingerprint misses on nearly every session_start: refresh_scar_behavioral_scores() rewrites each scar with >=3 recent usage rows and bumps its updated_at. Each miss re-downloaded the whole index, every row with its 1536-dim embedding. - A miss with a cache from this store downloads the manifest (id, updated_at; no vectors) with the bulk load's filters, order and limit. - Rows whose id and updated_at match are reused from disk; changed and new ids are fetched in full, 100 ids per request; rows gone from the manifest are dropped. The result is in manifest order, what a full download returns. - Any failure, an empty manifest, or no cache from this store: the full download, as before. cache-flush still always takes the full download. - The bulk load, fingerprint, manifest and delta fetch share one filter set. Venue (v1.8.0 setup.sql, 250 learnings, --usage 5): the start after a session_start drops from 921,780 B to 42,823 B — 245 reused, 5 fetched. +8 unit tests. Co-Authored-By: Claude Opus 5 --- src/services/startup.ts | 55 +++++- src/services/supabase-client.ts | 56 +++++- src/services/vector-disk-cache.ts | 111 +++++++++-- tests/unit/services/vector-delta-sync.test.ts | 180 ++++++++++++++++++ 4 files changed, 377 insertions(+), 25 deletions(-) create mode 100644 tests/unit/services/vector-delta-sync.test.ts diff --git a/src/services/startup.ts b/src/services/startup.ts index 3cd2232..f9bee20 100644 --- a/src/services/startup.ts +++ b/src/services/startup.ts @@ -15,8 +15,9 @@ import * as fs from "fs"; import * as path from "path"; -import { isConfigured, loadScarsWithEmbeddings, getLearningsFingerprint, getSupabaseUrl } from "./supabase-client.js"; -import { computeStoreKey, readVectorCache, writeVectorCache, tryAcquireDownloadLock, waitForLeaderCache } from "./vector-disk-cache.js"; +import { isConfigured, loadScarsWithEmbeddings, loadLearningsManifest, loadLearningsByIds, getLearningsFingerprint, getSupabaseUrl } from "./supabase-client.js"; +import { computeStoreKey, readVectorCache, readVectorCacheBase, planDelta, assembleDelta, writeVectorCache, tryAcquireDownloadLock, waitForLeaderCache } from "./vector-disk-cache.js"; +import type { StoreFingerprint } from "./vector-disk-cache.js"; import { getTableName } from "./tier.js"; import { getGitmemDir } from "./gitmem-dir.js"; import { @@ -103,6 +104,38 @@ interface ScarWithEmbedding { [key: string]: unknown; } +/** Rows the bulk load and the delta manifest both cap at. */ +const INDEX_ROW_LIMIT = 500; + +/** + * GIT-98 per-row delta: bring a cache from this store up to date by fetching + * only the rows whose updated_at moved. Returns null — and the caller does the + * full download — when there is no base to reuse or anything goes wrong. + */ +async function deltaSyncFromCache(storeKey: string, fingerprint: StoreFingerprint): Promise { + const base = readVectorCacheBase(storeKey); + if (!base || base.length === 0) return null; + const startTime = Date.now(); + try { + const manifest = await loadLearningsManifest(INDEX_ROW_LIMIT); + // An empty manifest is more likely a transient failure than an emptied + // store; let the full download decide. + if (manifest.length === 0) return null; + const plan = planDelta(base, manifest); + const fetched = plan.fetchIds.length > 0 ? await loadLearningsByIds(plan.fetchIds) : []; + const rows = assembleDelta(manifest, plan, fetched); + console.error( + `[startup] Vector cache DELTA: ${rows.length} learnings in ${Date.now() - startTime}ms — ` + + `${plan.reuse.size} reused from disk, ${fetched.length} fetched, ${plan.dropped} dropped (bulk download skipped)` + ); + if (rows.length > 0) writeVectorCache(storeKey, fingerprint, rows); + return rows; + } catch (error) { + console.error("[startup] Delta sync failed, falling back to full download:", error instanceof Error ? error.message : error); + return null; + } +} + /** * Load all learnings with embeddings from Supabase * @@ -112,7 +145,8 @@ interface ScarWithEmbedding { * NOTE: Now loads all learning types (scars, patterns, wins, anti-patterns), * not just scars. This fixes the issue where ~64 patterns were being ignored. */ -async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean } = {}): Promise<{ +/** @internal exported for tests (GIT-98 delta sync). */ +export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean } = {}): Promise<{ scars: ScarWithEmbedding[]; latestUpdatedAt: string | null; source: "disk" | "network"; @@ -155,11 +189,20 @@ async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean } = {} } releaseLock = releaseLock ?? (() => {}); - console.error(`[startup] Loading ALL learnings with embeddings from Supabase (cross-project)`); - try { + // GIT-98: a miss is usually a handful of changed rows, not a new store. + // cache-flush (bypassDiskCache) always takes the full download. + if (!options.bypassDiskCache && fingerprint.count >= 0) { + const synced = await deltaSyncFromCache(storeKey, fingerprint); + if (synced) { + return { scars: synced, latestUpdatedAt: synced[0]?.updated_at || null, source: "network" }; + } + } + + console.error(`[startup] Loading ALL learnings with embeddings from Supabase (cross-project)`); + // Load ALL learnings across projects — semantic similarity handles relevance - const learnings = await loadScarsWithEmbeddings(undefined, 500); + const learnings = await loadScarsWithEmbeddings(undefined, INDEX_ROW_LIMIT); const elapsed = Date.now() - startTime; console.error(`[startup] Loaded ${learnings.length} learnings in ${elapsed}ms`); diff --git a/src/services/supabase-client.ts b/src/services/supabase-client.ts index a420f69..ca1855a 100644 --- a/src/services/supabase-client.ts +++ b/src/services/supabase-client.ts @@ -518,6 +518,18 @@ export async function directPatch( * * NOTE: Changed from "scars only" to "all learning types" */ +/** + * The learnings the local index is built from. The bulk load, the fingerprint, + * the delta manifest and the delta fetch MUST all use these, or they describe + * different sets (GIT-98). + */ +const INDEXED_LEARNING_FILTERS: Record = { + learning_type: "in.(scar,pattern,win,anti_pattern)", + is_active: "eq.true", +}; +const INDEXED_LEARNING_COLUMNS = + "id,title,description,severity,counter_arguments,applies_when,source_linear_issue,project,embedding,updated_at,learning_type,decay_multiplier"; + export async function loadScarsWithEmbeddings( project?: string, limit = 500 @@ -526,16 +538,13 @@ export async function loadScarsWithEmbeddings( const startTime = Date.now(); try { - const filters: Record = { - learning_type: "in.(scar,pattern,win,anti_pattern)", - is_active: "eq.true", - }; + const filters: Record = { ...INDEXED_LEARNING_FILTERS }; if (project) { filters.project = project; } const learnings = await directQuery(getTableName("learnings"), { - select: "id,title,description,severity,counter_arguments,applies_when,source_linear_issue,project,embedding,updated_at,learning_type,decay_multiplier", + select: INDEXED_LEARNING_COLUMNS, filters, order: "updated_at.desc", limit, @@ -551,6 +560,40 @@ export async function loadScarsWithEmbeddings( } } +/** + * Delta manifest (GIT-98): the id and updated_at of every row the bulk load + * would return — same filters, order and limit — and nothing else. About 70 + * bytes a row, versus ~20 KB a row with its 1536-dim embedding. + */ +export async function loadLearningsManifest(limit = 500): Promise> { + return directQuery<{ id: string; updated_at: string | null }>(getTableName("learnings"), { + select: "id,updated_at", + filters: { ...INDEXED_LEARNING_FILTERS }, + order: "updated_at.desc", + limit, + }); +} + +/** Ids per delta request. A UUID is 36 characters, so this keeps URLs near 4 KB. */ +const DELTA_FETCH_CHUNK = 100; + +/** + * Delta fetch (GIT-98): full rows, embedding included, for the given ids only. + * The indexed-learning filters still apply, so a row deactivated since the + * manifest was read is simply absent. + */ +export async function loadLearningsByIds(ids: string[]): Promise { + const rows: T[] = []; + for (let i = 0; i < ids.length; i += DELTA_FETCH_CHUNK) { + const chunk = ids.slice(i, i + DELTA_FETCH_CHUNK); + rows.push(...await directQuery(getTableName("learnings"), { + select: INDEXED_LEARNING_COLUMNS, + filters: { ...INDEXED_LEARNING_FILTERS, id: `in.(${chunk.join(",")})` }, + })); + } + return rows; +} + /** * Cheap fingerprint of the learnings the local index is built from (GIT-98). * @@ -569,8 +612,7 @@ export async function getLearningsFingerprint(): Promise<{ count: number; latest try { const url = new URL(`${SUPABASE_REST_URL}/${getTableName("learnings")}`); url.searchParams.set("select", "updated_at"); - url.searchParams.set("learning_type", "in.(scar,pattern,win,anti_pattern)"); - url.searchParams.set("is_active", "eq.true"); + for (const [key, value] of Object.entries(INDEXED_LEARNING_FILTERS)) url.searchParams.set(key, value); url.searchParams.set("order", "updated_at.desc"); url.searchParams.set("limit", "1"); diff --git a/src/services/vector-disk-cache.ts b/src/services/vector-disk-cache.ts index 91853b9..490a4ba 100644 --- a/src/services/vector-disk-cache.ts +++ b/src/services/vector-disk-cache.ts @@ -12,7 +12,21 @@ * 1. Ask the store for a FINGERPRINT — row count + newest updated_at. One * request, one row, a few hundred bytes (PostgREST `Prefer: count=exact`). * 2. If a cache file with the same fingerprint exists, load vectors from disk. - * 3. Otherwise download as before and rewrite the cache. + * 3. Otherwise sync the difference (below) and rewrite the cache. + * + * ## Per-row delta sync + * + * A fingerprint miss used to mean a full download, and on a Pro store it + * misses on nearly every session_start: refresh_scar_behavioral_scores() + * rewrites every scar with enough recent usage and bumps its updated_at. So + * a handful of changed rows cost the whole index. + * + * On a miss with a usable cache from this store, only the manifest (id, + * updated_at; no vectors) is downloaded. A row whose id and updated_at match + * the cache is reused from disk; only changed and new ids are fetched in full; + * rows no longer in the manifest are dropped. The result is in manifest order, + * exactly what a full download would have returned, given that updated_at + * moves on every edit (the same assumption the fingerprint already makes). * * ## What the fingerprint can and cannot see * @@ -105,31 +119,104 @@ export function readVectorCache(storeKey: string, expected: StoreFingerprint) // failures would otherwise "agree" and serve whatever is on disk. if (expected.count < 0) return { hit: false, reason: "store fingerprint unavailable" }; + const file = readCacheFile(storeKey); + if (!file.ok) return { hit: false, reason: file.reason }; + if (!file.parsed.fingerprint || !fingerprintsMatch(file.parsed.fingerprint, expected)) { + return { hit: false, reason: "store changed since cache was written" }; + } + return { hit: true, rows: file.parsed.rows, bytesOnDisk: file.bytesOnDisk }; +} + +/** + * The cached rows of this store, whatever fingerprint they were written under: + * the base a delta sync reuses unchanged rows from. Never served as-is — every + * row is checked against the store's manifest first. + */ +export function readVectorCacheBase(storeKey: string): T[] | null { + if (!isVectorDiskCacheEnabled()) return null; + const file = readCacheFile(storeKey); + return file.ok ? file.parsed.rows : null; +} + +export interface ManifestRow { + id: string; + updated_at: string | null; +} + +export interface DeltaPlan { + /** Cached rows still current in the store, by id. */ + reuse: Map; + /** Ids to fetch in full: changed since the cache was written, or new. */ + fetchIds: string[]; + /** Cached rows the store no longer returns. */ + dropped: number; +} + +/** + * Compare the store's manifest with the cached rows. A row is reused only when + * its id AND updated_at match; a cached row without updated_at is refetched. + */ +export function planDelta( + cached: T[], + manifest: ManifestRow[] +): DeltaPlan { + const cachedById = new Map(cached.map((r) => [r.id, r])); + const reuse = new Map(); + const fetchIds: string[] = []; + for (const m of manifest) { + const c = cachedById.get(m.id); + if (c && m.updated_at != null && c.updated_at === m.updated_at) reuse.set(m.id, c); + else fetchIds.push(m.id); + } + const inManifest = new Set(manifest.map((m) => m.id)); + const dropped = cached.filter((r) => !inManifest.has(r.id)).length; + return { reuse, fetchIds, dropped }; +} + +/** + * Rows in manifest order: reused where current, fetched otherwise. A manifest + * id that is in neither (deactivated between the manifest and the fetch) is + * left out, as a full download at that moment would have. + */ +export function assembleDelta( + manifest: ManifestRow[], + plan: DeltaPlan, + fetched: T[] +): T[] { + const fetchedById = new Map(fetched.map((r) => [r.id, r])); + const rows: T[] = []; + for (const m of manifest) { + const row = fetchedById.get(m.id) ?? plan.reuse.get(m.id); + if (row) rows.push(row); + } + return rows; +} + +function readCacheFile(storeKey: string): + | { ok: true; parsed: VectorCacheFile; bytesOnDisk: number } + | { ok: false; reason: string } { const file = getVectorCachePath(storeKey); let raw: string; try { - if (!fs.existsSync(file)) return { hit: false, reason: "no cache file" }; + if (!fs.existsSync(file)) return { ok: false, reason: "no cache file" }; raw = fs.readFileSync(file, "utf-8"); } catch (error) { - return { hit: false, reason: `unreadable: ${error instanceof Error ? error.message : String(error)}` }; + return { ok: false, reason: `unreadable: ${error instanceof Error ? error.message : String(error)}` }; } let parsed: VectorCacheFile; try { parsed = JSON.parse(raw) as VectorCacheFile; } catch { - return { hit: false, reason: "corrupt JSON" }; + return { ok: false, reason: "corrupt JSON" }; } - if (!parsed || typeof parsed !== "object") return { hit: false, reason: "corrupt shape" }; - if (parsed.format !== VECTOR_CACHE_FORMAT) return { hit: false, reason: `format ${parsed.format} != ${VECTOR_CACHE_FORMAT}` }; - if (parsed.storeKey !== storeKey) return { hit: false, reason: "written for a different store" }; - if (!Array.isArray(parsed.rows)) return { hit: false, reason: "corrupt rows" }; - if (!parsed.fingerprint || !fingerprintsMatch(parsed.fingerprint, expected)) { - return { hit: false, reason: "store changed since cache was written" }; - } + if (!parsed || typeof parsed !== "object") return { ok: false, reason: "corrupt shape" }; + if (parsed.format !== VECTOR_CACHE_FORMAT) return { ok: false, reason: `format ${parsed.format} != ${VECTOR_CACHE_FORMAT}` }; + if (parsed.storeKey !== storeKey) return { ok: false, reason: "written for a different store" }; + if (!Array.isArray(parsed.rows)) return { ok: false, reason: "corrupt rows" }; - return { hit: true, rows: parsed.rows, bytesOnDisk: Buffer.byteLength(raw) }; + return { ok: true, parsed, bytesOnDisk: Buffer.byteLength(raw) }; } /** diff --git a/tests/unit/services/vector-delta-sync.test.ts b/tests/unit/services/vector-delta-sync.test.ts new file mode 100644 index 0000000..332521b --- /dev/null +++ b/tests/unit/services/vector-delta-sync.test.ts @@ -0,0 +1,180 @@ +/** + * GIT-98: per-row delta sync. + * + * A fingerprint miss used to cost the whole index. On a Pro store it misses on + * nearly every session_start, because refresh_scar_behavioral_scores() bumps + * updated_at on every scar with enough recent usage. Now a miss with a cache + * from this store downloads the manifest (id, updated_at) and only the rows + * whose updated_at moved. + * + * Real filesystem cache, isolated GITMEM_DIR; the store is mocked at the + * supabase-client boundary so each call can be counted. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +const store = vi.hoisted(() => ({ + rows: [] as Array<{ id: string; title: string; updated_at: string; embedding: number[] }>, + manifestFails: false, + calls: { fingerprint: 0, manifest: 0, byIds: [] as string[][], bulk: 0 }, +})); + +const active = () => [...store.rows].sort((a, b) => b.updated_at.localeCompare(a.updated_at)); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + isConfigured: () => true, + getSupabaseUrl: () => "https://venue.example.supabase.co", + getLearningsFingerprint: vi.fn(async () => { + store.calls.fingerprint++; + const rows = active(); + return { count: rows.length, latestUpdatedAt: rows[0]?.updated_at ?? null }; + }), + loadLearningsManifest: vi.fn(async () => { + store.calls.manifest++; + if (store.manifestFails) throw new Error("manifest 500"); + return active().map(({ id, updated_at }) => ({ id, updated_at })); + }), + loadLearningsByIds: vi.fn(async (ids: string[]) => { + store.calls.byIds.push(ids); + return active().filter((r) => ids.includes(r.id)).map((r) => ({ ...r })); + }), + loadScarsWithEmbeddings: vi.fn(async () => { + store.calls.bulk++; + return active().map((r) => ({ ...r })); + }), +})); + +import { loadScarsFromSupabase } from "../../../src/services/startup.js"; +import { planDelta, assembleDelta } from "../../../src/services/vector-disk-cache.js"; + +let dir: string; +const ts = (n: number) => new Date(Date.UTC(2026, 8, 21, 0, 0, n)).toISOString(); +const vec = (i: number) => Array.from({ length: 8 }, (_, k) => i + k / 10); + +function seed(n: number) { + store.rows = Array.from({ length: n }, (_, i) => ({ id: `id-${String(i).padStart(3, "0")}`, title: `t${i}`, updated_at: ts(i), embedding: vec(i) })); +} +/** What refresh_scar_behavioral_scores() does to a scar: new score, new updated_at. */ +function rewrite(ids: string[], at: number) { + for (const r of store.rows) if (ids.includes(r.id)) { r.updated_at = ts(at); r.title = `${r.title}*`; r.embedding = vec(1000 + at); } +} +const resetCalls = () => { store.calls = { fingerprint: 0, manifest: 0, byIds: [], bulk: 0 }; }; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-vdelta-")); + vi.stubEnv("GITMEM_DIR", dir); + vi.stubEnv("GITMEM_VECTOR_DISK_CACHE", ""); + store.manifestFails = false; + seed(250); + resetCalls(); +}); + +afterEach(() => { + vi.unstubAllEnvs(); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +describe("planDelta / assembleDelta", () => { + const cached = [ + { id: "a", updated_at: "1", v: "a-old" }, + { id: "b", updated_at: "1", v: "b-old" }, + { id: "gone", updated_at: "1", v: "gone" }, + { id: "no-ts", v: "no-ts" }, + ]; + const manifest = [ + { id: "new", updated_at: "3" }, + { id: "b", updated_at: "2" }, + { id: "a", updated_at: "1" }, + { id: "no-ts", updated_at: "1" }, + ]; + + it("reuses only id+updated_at matches, fetches changed, new and timestamp-less rows, counts dropped ones", () => { + const plan = planDelta(cached as never[], manifest); + expect([...plan.reuse.keys()]).toEqual(["a"]); + expect(plan.fetchIds).toEqual(["new", "b", "no-ts"]); + expect(plan.dropped).toBe(1); + }); + + it("assembles in manifest order and omits ids that vanished before the fetch", () => { + const plan = planDelta(cached as never[], manifest); + const rows = assembleDelta(manifest, plan, [{ id: "b", v: "b-new" }, { id: "no-ts", v: "no-ts-new" }] as never[]); + expect(rows.map((r: { v: string }) => r.v)).toEqual(["b-new", "a-old", "no-ts-new"]); + }); +}); + +describe("loadScarsFromSupabase delta sync (GIT-98)", () => { + it("cold start: one full download, then a warm start costs only the fingerprint", async () => { + const cold = await loadScarsFromSupabase(); + expect(cold.scars).toHaveLength(250); + expect(store.calls.bulk).toBe(1); + + resetCalls(); + const warm = await loadScarsFromSupabase(); + expect(warm.source).toBe("disk"); + expect(store.calls).toEqual({ fingerprint: 1, manifest: 0, byIds: [], bulk: 0 }); + }); + + it("5 rows rewritten: fetches exactly those 5 vectors, not 250, and matches a full download", async () => { + await loadScarsFromSupabase(); + const changed = ["id-007", "id-042", "id-099", "id-150", "id-249"]; + rewrite(changed, 5000); + resetCalls(); + + const synced = await loadScarsFromSupabase(); + + expect(store.calls.bulk).toBe(0); + expect(store.calls.manifest).toBe(1); + expect(store.calls.byIds.flat().sort()).toEqual([...changed].sort()); + // Identical to what a full download would have returned, order included. + expect(synced.scars).toEqual(active()); + expect(synced.latestUpdatedAt).toBe(ts(5000)); + + // The rewritten cache serves the next start from disk. + resetCalls(); + expect((await loadScarsFromSupabase()).source).toBe("disk"); + expect(store.calls.byIds).toEqual([]); + }); + + it("inserts and deactivations: fetches the new row, drops the gone one", async () => { + await loadScarsFromSupabase(); + store.rows = store.rows.filter((r) => r.id !== "id-010"); + store.rows.push({ id: "id-new", title: "fresh", updated_at: ts(6000), embedding: vec(6000) }); + resetCalls(); + + const synced = await loadScarsFromSupabase(); + expect(store.calls.bulk).toBe(0); + expect(store.calls.byIds.flat()).toEqual(["id-new"]); + expect(synced.scars.map((r) => r.id)).toEqual(active().map((r) => r.id)); + expect(synced.scars.some((r) => r.id === "id-010")).toBe(false); + }); + + it("a failed manifest falls back to the full download", async () => { + await loadScarsFromSupabase(); + rewrite(["id-001"], 7000); + store.manifestFails = true; + resetCalls(); + + const loaded = await loadScarsFromSupabase(); + expect(store.calls.bulk).toBe(1); + expect(loaded.scars).toEqual(active()); + }); + + it("cache-flush (bypassDiskCache) always takes the full download", async () => { + await loadScarsFromSupabase(); + rewrite(["id-001"], 8000); + resetCalls(); + + await loadScarsFromSupabase({ bypassDiskCache: true }); + expect(store.calls.manifest).toBe(0); + expect(store.calls.bulk).toBe(1); + }); + + it("no cache from this store: full download, no manifest", async () => { + const loaded = await loadScarsFromSupabase(); + expect(store.calls.manifest).toBe(0); + expect(store.calls.bulk).toBe(1); + expect(loaded.scars).toHaveLength(250); + }); +}); From fb2893fb8354e86ac712deff77cb963e9b19a2ca Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:11:54 -0700 Subject: [PATCH 04/24] test: egress mode fails on any full index download after the cold start (GIT-98) Exits 1 on 1.10.0 (the start after each session_start re-downloads ~922 KB); exits 0 with per-row delta sync. Co-Authored-By: Claude Opus 5 --- tests/e2e/blank-supabase.mjs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..0787fd4 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -21,6 +21,8 @@ * egress bytes received from the venue on a cold start, a warm start, and around * two session_starts (GIT-98 disk cache). --usage N seeds N scars with * >=3 usage rows so refresh_scar_behavioral_scores() has work to do. + * Fails if any start after the cold one downloads the whole index again + * (GIT-98 per-row delta). * * VENUE_ENV is a KEY=VALUE file OUTSIDE the repo defining SUPABASE_URL, * SUPABASE_SERVICE_ROLE_KEY and VENUE_REF. Credentials are never committed. @@ -520,6 +522,15 @@ const failureCheck = checkFailures(allVenueRequests); const edgeCalls = allVenueRequests.filter((r) => r.path.startsWith("/functions/v1/")) .map((r) => `${r.method} ${r.path} -> ${r.status}`); if (edgeCalls.length) failureCheck.unexpected.push(...edgeCalls.map((c) => `edge function called (GIT-97): ${c}`)); +// GIT-98 per-row delta: after the cold start, a changed store costs the changed +// rows, never the whole index again. +if (mode === "egress") { + for (const st of result.starts.slice(1)) { + if (st.cache_log.some((l) => /Loading ALL learnings/.test(l))) { + failureCheck.unexpected.push(`full index download after the cold start (GIT-98): ${st.start}, ${st.total_bytes} B`); + } + } +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } From 936cc4fabcab6bf30fb298eb61a50f3e3770d02a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:20:03 -0700 Subject: [PATCH 05/24] fix: hooks resolve .gitmem like the server, skip dead-pid sessions, print the payload path (GIT-99) Since GIT-91 the server resolves its root independently of cwd, but the hooks still read ".gitmem/..." relative to theirs. A hook running in a repo read a different store from the one the server writes: no session, or someone else's, and a closing-payload.json the server never reads. - hooks/scripts/resolve-root.sh: GITMEM_DIR, then GITMEM_HOME/.gitmem, then ~/.gitmem, made absolute. Sourced by recall-check, post-tool-use and session-close-check. - Registry entries on this host whose pid is dead are skipped; entries without a pid or from another host are kept. recall-check takes the newest live session instead of sessions[0]. - The Stop hook prints the resolved absolute payload path, JSON-escaped. - Fixes a pre-existing break: without jq, session-close-check's node fallback was brace-expanded by bash into two broken node calls, so the Stop hook never blocked (3 of 32 hook tests red on main on a jq-less machine). test-hooks.sh: the existing tests relied on cwd resolution (cd $TMPDIR finding $TMPDIR/.gitmem); they now export GITMEM_DIR to the same store. +12 tests (44/44; main 31/44). Co-Authored-By: Claude Opus 5 --- hooks/scripts/post-tool-use.sh | 5 +- hooks/scripts/recall-check.sh | 17 ++--- hooks/scripts/resolve-root.sh | 81 +++++++++++++++++++++++ hooks/scripts/session-close-check.sh | 19 +++--- hooks/tests/test-hooks.sh | 96 ++++++++++++++++++++++++++++ 5 files changed, 195 insertions(+), 23 deletions(-) create mode 100755 hooks/scripts/resolve-root.sh diff --git a/hooks/scripts/post-tool-use.sh b/hooks/scripts/post-tool-use.sh index ae6c45a..86fb705 100755 --- a/hooks/scripts/post-tool-use.sh +++ b/hooks/scripts/post-tool-use.sh @@ -22,8 +22,9 @@ HOOK_INPUT=$(cat -) # ============================================================================ # Check active sessions registry -ACTIVE_SESSIONS=".gitmem/active-sessions.json" -if [ ! -f "$ACTIVE_SESSIONS" ]; then +# GIT-99: resolve .gitmem the way the server does, not from cwd. +. "$(dirname "${BASH_SOURCE[0]}")/resolve-root.sh" +if [ -z "$(gitmem_live_session_ids)" ]; then exit 0 fi diff --git a/hooks/scripts/recall-check.sh b/hooks/scripts/recall-check.sh index 1750ae1..71a9e82 100755 --- a/hooks/scripts/recall-check.sh +++ b/hooks/scripts/recall-check.sh @@ -31,20 +31,17 @@ HOOK_INPUT=$(cat -) # Resolve active session from registry # ============================================================================ -ACTIVE_SESSIONS=".gitmem/active-sessions.json" -if [ ! -f "$ACTIVE_SESSIONS" ]; then +# GIT-99: resolve .gitmem the way the server does, not from cwd. +. "$(dirname "${BASH_SOURCE[0]}")/resolve-root.sh" +if [ ! -f "$GITMEM_ACTIVE_SESSIONS" ]; then exit 0 fi -# Get first session ID from registry to find per-session data file +# Newest live session in the registry -> its per-session data file. +# Entries whose server pid is dead are skipped (GIT-99). SESSION_FILE="" -if command -v jq &>/dev/null; then - SID=$(jq -r '.sessions[0].session_id // empty' "$ACTIVE_SESSIONS" 2>/dev/null) - [ -n "$SID" ] && SESSION_FILE=".gitmem/sessions/${SID}/session.json" -elif command -v node &>/dev/null; then - SID=$(node -e "const fs=require('fs');try{const r=JSON.parse(fs.readFileSync('$ACTIVE_SESSIONS','utf8'));const s=(r.sessions||[])[0];process.stdout.write(s?.session_id||'')}catch(e){}" 2>/dev/null) - [ -n "$SID" ] && SESSION_FILE=".gitmem/sessions/${SID}/session.json" -fi +SID=$(gitmem_live_session_ids | head -n 1) +[ -n "$SID" ] && SESSION_FILE="$GITMEM_ROOT/sessions/${SID}/session.json" if [ -z "$SESSION_FILE" ] || [ ! -f "$SESSION_FILE" ]; then exit 0 diff --git a/hooks/scripts/resolve-root.sh b/hooks/scripts/resolve-root.sh new file mode 100755 index 0000000..f774148 --- /dev/null +++ b/hooks/scripts/resolve-root.sh @@ -0,0 +1,81 @@ +#!/bin/bash +# ============================================================================ +# gitmem-hooks — resolve the .gitmem root the way the MCP server does (GIT-99) +# +# Sourced, not executed: . "$(dirname "$0")/resolve-root.sh" +# +# The hooks used to read ".gitmem/..." relative to their cwd. Since GIT-91 the +# server resolves its root independently of cwd, so a hook running in a repo +# read a different store from the one the server writes: it saw no session, or +# someone else's, and pointed the agent at a closing-payload.json the server +# would never read. +# +# Precedence mirrors src/services/gitmem-dir.ts: +# 1. $GITMEM_DIR — names the .gitmem directory itself +# 2. $GITMEM_HOME/.gitmem — relocates the home the fallback uses +# 3. $HOME/.gitmem +# +# Sets (all absolute): +# GITMEM_ROOT the resolved .gitmem directory +# GITMEM_ACTIVE_SESSIONS $GITMEM_ROOT/active-sessions.json +# GITMEM_PAYLOAD_PATH $GITMEM_ROOT/closing-payload.json +# Provides: +# gitmem_live_session_ids registry session ids, newest first, skipping +# entries on this host whose pid is dead +# gitmem_json_escape escape a string for a JSON string literal +# ============================================================================ + +if [ -n "${GITMEM_DIR:-}" ]; then + _gitmem_root="$GITMEM_DIR" +elif [ -n "${GITMEM_HOME:-}" ]; then + _gitmem_root="$GITMEM_HOME/.gitmem" +else + _gitmem_root="$HOME/.gitmem" +fi +# Absolute, without requiring the directory to exist yet. +case "$_gitmem_root" in + /*) ;; + *) _gitmem_root="$PWD/$_gitmem_root" ;; +esac +GITMEM_ROOT="${_gitmem_root%/}" +unset _gitmem_root +GITMEM_ACTIVE_SESSIONS="$GITMEM_ROOT/active-sessions.json" +GITMEM_PAYLOAD_PATH="$GITMEM_ROOT/closing-payload.json" + +# A registry entry whose server died is not a session anyone can be in. Only +# entries on THIS host can be checked; entries without a pid, or from another +# host, are kept. `ps -p` rather than `kill -0`: kill fails with EPERM for a +# live process of another user, which is still alive. +gitmem_live_session_ids() { + [ -f "$GITMEM_ACTIVE_SESSIONS" ] || return 0 + local host rows + host=$(hostname 2>/dev/null || echo "") + if command -v jq &>/dev/null; then + rows=$(jq -r '(.sessions // []) | sort_by(.started_at // "") | reverse | .[] + | [(.session_id // ""), ((.pid // "") | tostring), (.hostname // "")] | join("|")' \ + "$GITMEM_ACTIVE_SESSIONS" 2>/dev/null) || return 0 + elif command -v node &>/dev/null; then + rows=$(GITMEM_REG="$GITMEM_ACTIVE_SESSIONS" node -e ' + try { + const r = JSON.parse(require("fs").readFileSync(process.env.GITMEM_REG, "utf8")); + const s = (r.sessions || []).slice().sort((a, b) => String(b.started_at || "").localeCompare(String(a.started_at || ""))); + process.stdout.write(s.map((e) => [e.session_id || "", e.pid ?? "", e.hostname || ""].join("|")).join("\n")); + } catch (e) {}' 2>/dev/null) || return 0 + else + return 0 + fi + local sid pid h + # "|" rather than a tab: read collapses runs of whitespace separators, so an + # empty pid would shift the hostname into its place. + while IFS='|' read -r sid pid h; do + [ -n "$sid" ] || continue + if [ -n "$pid" ] && [ "$h" = "$host" ] && ! ps -p "$pid" >/dev/null 2>&1; then + continue + fi + echo "$sid" + done <<< "$rows" +} + +gitmem_json_escape() { + printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' +} diff --git a/hooks/scripts/session-close-check.sh b/hooks/scripts/session-close-check.sh index ccbb57a..0b25075 100755 --- a/hooks/scripts/session-close-check.sh +++ b/hooks/scripts/session-close-check.sh @@ -35,16 +35,12 @@ fi IS_MEANINGFUL=false -ACTIVE_SESSIONS=".gitmem/active-sessions.json" +# GIT-99: resolve .gitmem the way the server does, not from cwd. +. "$(dirname "${BASH_SOURCE[0]}")/resolve-root.sh" +# A live registry entry means a session is open. Entries whose server pid is +# dead are skipped: nobody can close those from here (GIT-99). SESSION_STARTED=false -if [ -f "$ACTIVE_SESSIONS" ]; then - # Check if registry has active session entries - if command -v jq &>/dev/null; then - [ "$(jq '.sessions | length' "$ACTIVE_SESSIONS" 2>/dev/null || echo 0)" -gt 0 ] 2>/dev/null && SESSION_STARTED=true - elif command -v node &>/dev/null; then - [ "$(node -e "const fs=require('fs');try{const r=JSON.parse(fs.readFileSync('$ACTIVE_SESSIONS','utf8'));process.stdout.write(String((r.sessions||[]).length))}catch(e){process.stdout.write('0')}" 2>/dev/null)" -gt 0 ] 2>/dev/null && SESSION_STARTED=true - fi -fi +[ -n "$(gitmem_live_session_ids)" ] && SESSION_STARTED=true # Without state dir, we have no tracking data — can't determine meaningfulness. # This happens when the SessionStart hook didn't fire (plugin hooks issue). @@ -98,10 +94,11 @@ if [ "$SESSION_STARTED" = "true" ]; then mkdir -p "$STATE_DIR" touch "$STATE_DIR/stop_hook_active" - cat <<'HOOKJSON' + PAYLOAD_JSON=$(gitmem_json_escape "$GITMEM_PAYLOAD_PATH") + cat < "$R_CWD/.gitmem/active-sessions.json" +OUTPUT=$( cd "$R_CWD" && echo '{}' | bash "$SCRIPT_DIR/scripts/session-close-check.sh" 2>/dev/null ) +echo "$OUTPUT" | grep -q '"decision": "block"' && pass "Stop hook reads GITMEM_DIR from an unrelated cwd (decoy cwd/.gitmem ignored)" \ + || fail "Stop hook cwd independence" "block" "$OUTPUT" + +# The Stop hook names the absolute payload path it resolved. +PAYLOAD_EXPECTED="$GITMEM_DIR/closing-payload.json" +echo "$OUTPUT" | grep -qF "WRITE structured payload to $PAYLOAD_EXPECTED (this exact absolute path" \ + && pass "Stop hook text prints the resolved absolute payload path" \ + || fail "Stop hook payload path" "$PAYLOAD_EXPECTED" "$OUTPUT" +echo "$OUTPUT" | (command -v node >/dev/null && node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{JSON.parse(d)})' 2>/dev/null) \ + && pass "Stop hook output with the path is valid JSON" \ + || fail "Stop hook JSON" "parseable JSON" "$OUTPUT" + +# A path that needs escaping still yields valid JSON. +Q_DIR="$TMPDIR/q\"uote/.gitmem"; mkdir -p "$Q_DIR" +echo '{"sessions":[{"session_id":"q"}]}' > "$Q_DIR/active-sessions.json" +setup_state 10 60 +OUTPUT=$( GITMEM_DIR="$Q_DIR" bash -c 'echo "{}" | bash "$0/scripts/session-close-check.sh"' "$SCRIPT_DIR" 2>/dev/null ) +echo "$OUTPUT" | node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{const j=JSON.parse(d);if(!j.reason.includes(process.argv[1]))process.exit(1)})' "$Q_DIR/closing-payload.json" 2>/dev/null \ + && pass "payload path with a quote is JSON-escaped" \ + || fail "payload path escaping" "valid JSON containing $Q_DIR/closing-payload.json" "$OUTPUT" + +# Registry entries whose pid is dead on this host are skipped. +DEAD_PID=99999999 +while ps -p "$DEAD_PID" >/dev/null 2>&1; do DEAD_PID=$((DEAD_PID - 1)); done +HOST=$(hostname) +setup_state 10 60 +echo "{\"sessions\":[{\"session_id\":\"dead-one\",\"pid\":$DEAD_PID,\"hostname\":\"$HOST\",\"started_at\":\"2026-09-21T00:00:00Z\"}]}" > "$GITMEM_DIR/active-sessions.json" +OUTPUT=$(echo '{}' | bash "$SCRIPT_DIR/scripts/session-close-check.sh" 2>/dev/null) +! echo "$OUTPUT" | grep -q "block" && pass "Stop hook ignores a registry entry whose server pid is dead" \ + || fail "dead pid skipped (Stop)" "no block" "$OUTPUT" + +setup_state 10 60 +echo "{\"sessions\":[{\"session_id\":\"dead-one\",\"pid\":$DEAD_PID,\"hostname\":\"other-host-$HOST\",\"started_at\":\"2026-09-21T00:00:00Z\"}]}" > "$GITMEM_DIR/active-sessions.json" +OUTPUT=$(echo '{}' | bash "$SCRIPT_DIR/scripts/session-close-check.sh" 2>/dev/null) +echo "$OUTPUT" | grep -q "block" && pass "an entry from another host is kept (its pid cannot be checked here)" \ + || fail "other-host entry kept" "block" "$OUTPUT" + +# recall-check uses the newest LIVE session, not sessions[0]. +remove_session_registry +echo "{\"sessions\":[ + {\"session_id\":\"live-old\",\"pid\":$$,\"hostname\":\"$HOST\",\"started_at\":\"2026-09-21T00:00:00Z\"}, + {\"session_id\":\"dead-new\",\"pid\":$DEAD_PID,\"hostname\":\"$HOST\",\"started_at\":\"2026-09-21T09:00:00Z\"}]}" > "$GITMEM_DIR/active-sessions.json" +create_session_data "live-old" '[{"scar_id":"s1","scar_title":"Unconfirmed","source":"recall"}]' '[]' +create_session_data "dead-new" '[]' '[]' +GOT=$(bash -c '. "$0/scripts/resolve-root.sh"; gitmem_live_session_ids | tr "\n" " "' "$SCRIPT_DIR") +[ "$GOT" = "live-old " ] && pass "gitmem_live_session_ids: newest first, dead pid skipped" \ + || fail "live session ids" "live-old" "$GOT" +setup_state 0 0 +OUTPUT=$(echo '{"tool_name":"Bash","tool_input":{"command":"git push origin main"}}' | bash "$SCRIPT_DIR/scripts/recall-check.sh" 2>/dev/null) +echo "$OUTPUT" | grep -q "block" && pass "recall-check reads the live session's scars (dead newer entry skipped)" \ + || fail "recall-check live session" "block on unconfirmed scar of live-old" "$OUTPUT" + +remove_session_registry + # ============================================================================ # Summary # ============================================================================ From 709aa84f034c2551941b9cadca5267eadfd1ca10 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:20:03 -0700 Subject: [PATCH 06/24] fix: session_close names the payload path when there is nothing to close with (GIT-99) A standard close with no closing-payload.json and no inline reflection now returns "closing-payload.json not found at " instead of the "requires task_completion / closing_reflection with N answers" family, which sent agents to rewrite answers they had already written to the wrong root. An unparseable payload is reported as found but unreadable, at its path. session-close-supabase.test.ts: three title/decision tests closed "standard" with neither payload nor reflection, reaching persistence only because validation is mocked. That input is now rejected up front, so they carry a minimal reflection to keep testing what they test. +5 tests. Co-Authored-By: Claude Opus 5 --- src/tools/session-close.ts | 31 +++ .../tools/session-close-payload-path.test.ts | 203 ++++++++++++++++++ .../unit/tools/session-close-supabase.test.ts | 9 + 3 files changed, 243 insertions(+) create mode 100644 tests/unit/tools/session-close-payload-path.test.ts diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index 2388a4c..b21d73e 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -932,6 +932,7 @@ export async function sessionClose( // This keeps the visible MCP tool call small: just session_id + close_type. const payloadPath = getGitmemPath("closing-payload.json"); let payloadConsumed = false; + let payloadReadError: string | null = null; try { if (fs.existsSync(payloadPath)) { const filePayload = JSON.parse(fs.readFileSync(payloadPath, "utf-8")) as Partial; @@ -944,6 +945,36 @@ export async function sessionClose( } } catch (error) { console.warn("[session_close] Failed to read closing-payload.json:", error); + payloadReadError = + `closing-payload.json at ${path.resolve(payloadPath)} could not be read: ` + + `${error instanceof Error ? error.message : String(error)}. Fix or rewrite it, then call session_close again.`; + } + + // GIT-99: a standard close with no reflection at all. The agent almost always + // wrote the payload — to a root this server does not read (the hook and the + // server resolved .gitmem differently). Validation would then report + // "requires task_completion" / "requires closing_reflection with N answers", + // which sends the agent to rewrite answers it already wrote. Name the path. + if (params.close_type === "standard" && !payloadConsumed && !params.closing_reflection) { + return { + success: false, + session_id: params.session_id || "", + close_compliance: { + close_type: "standard", + agent: detectAgent().agent, + checklist_displayed: false, + questions_answered_by_agent: false, + human_asked_for_corrections: false, + learnings_stored: 0, + scars_applied: 0, + }, + validation_errors: [ + payloadReadError ?? + `closing-payload.json not found at ${path.resolve(payloadPath)}. ` + + `Write the closing payload to exactly that path (or pass closing_reflection inline), then call session_close again.`, + ], + performance: buildPerformanceData("session_close", timer.stop(), 0), + }; } // Sanitize scars_to_record: agents frequently write create_learning shape diff --git a/tests/unit/tools/session-close-payload-path.test.ts b/tests/unit/tools/session-close-payload-path.test.ts new file mode 100644 index 0000000..4fe12fd --- /dev/null +++ b/tests/unit/tools/session-close-payload-path.test.ts @@ -0,0 +1,203 @@ +/** + * GIT-99: a standard close with neither closing-payload.json nor an inline + * reflection names the path it looked at. + * + * The agent has usually written the payload — to a .gitmem root this server + * does not read. It used to get "requires task_completion" / "requires + * closing_reflection with N answers", which sends it to rewrite answers it + * already wrote instead of to the path mismatch. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +const root = vi.hoisted(() => ({ dir: "" })); + +// Mock dependencies before importing session-close +vi.mock("../../../src/services/agent-detection.js", () => ({ + detectAgent: () => ({ agent: "CLI", entrypoint: "cli", docker: true, hostname: "test" }), +})); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + listRecords: vi.fn().mockResolvedValue([]), + getRecord: vi.fn().mockResolvedValue(null), + directUpsert: vi.fn().mockResolvedValue(undefined), + directPatch: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../../src/services/embedding.js", () => ({ + embed: vi.fn().mockResolvedValue(null), + isEmbeddingAvailable: () => false, +})); + +vi.mock("../../../src/services/tier.js", () => ({ + hasSupabase: () => false, + hasBatchOperations: () => false, + hasTranscripts: () => false, + hasCacheManagement: () => false, + getTableName: (base: string) => `orchestra_${base}`, + hasProInsights: () => false, +})); + +vi.mock("../../../src/services/analytics.js", () => ({ + queryScarUsageByDateRange: vi.fn().mockResolvedValue([]), + enrichScarUsageTitles: vi.fn().mockResolvedValue([]), + formatBlindspotSnippet: vi.fn().mockReturnValue(null), + querySessionsByDateRange: vi.fn().mockResolvedValue([]), + computeLightweightSummary: vi.fn().mockReturnValue(null), +})); + +vi.mock("../../../src/services/storage.js", () => ({ + getStorage: () => ({ + get: vi.fn().mockResolvedValue(null), + upsert: vi.fn().mockResolvedValue(undefined), + }), +})); + +vi.mock("../../../src/services/session-state.js", () => ({ + clearCurrentSession: vi.fn(), + getSurfacedScars: () => [], + getObservations: () => [], + getChildren: () => [], + getThreads: () => [], + getSessionActivity: () => null, + isRecallCalled: () => true, +})); + +vi.mock("../../../src/services/thread-manager.js", () => ({ + normalizeThreads: vi.fn().mockReturnValue([]), + mergeThreadStates: vi.fn().mockReturnValue([]), + migrateStringThread: vi.fn().mockReturnValue({ id: "t-test", text: "test", status: "open", created_at: new Date().toISOString() }), + saveThreadsFile: vi.fn(), +})); + +vi.mock("../../../src/services/thread-dedup.js", () => ({ + deduplicateThreadList: vi.fn().mockImplementation((threads) => threads), +})); + +vi.mock("../../../src/services/thread-supabase.js", () => ({ + syncThreadsToSupabase: vi.fn().mockResolvedValue(undefined), + loadOpenThreadEmbeddings: vi.fn().mockResolvedValue([]), +})); + +vi.mock("../../../src/services/compliance-validator.js", () => ({ + validateSessionClose: () => ({ valid: true, errors: [], warnings: [] }), + buildCloseCompliance: vi.fn().mockReturnValue({ + close_type: "quick", + agent: "CLI", + checklist_displayed: true, + questions_answered_by_agent: false, + human_asked_for_corrections: false, + learnings_stored: 0, + scars_applied: 0, + }), +})); + +vi.mock("../../../src/services/metrics.js", () => ({ + Timer: class { stop() { return 100; } }, + recordMetrics: vi.fn().mockResolvedValue(undefined), + buildPerformanceData: (name: string, latency: number, count: number) => ({ + latency_ms: latency, + target_ms: 3000, + meets_target: latency < 3000, + result_count: count, + }), + updateRelevanceData: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../../src/tools/record-scar-usage-batch.js", () => ({ + recordScarUsageBatch: vi.fn().mockResolvedValue({ success: true }), +})); + +vi.mock("../../../src/services/effect-tracker.js", () => ({ + getEffectTracker: () => ({ + track: vi.fn(), + formatSummary: () => "No tracked effects this session.", + getHealthReport: () => ({ overall: { attempted: 0, succeeded: 0, failed: 0, successRate: "N/A", paths_with_failures: [] }, byPath: {}, recentFailures: [] }), + }), +})); + +vi.mock("../../../src/tools/save-transcript.js", () => ({ + saveTranscript: vi.fn().mockResolvedValue({ success: false }), +})); + +vi.mock("../../../src/services/transcript-chunker.js", () => ({ + processTranscript: vi.fn().mockResolvedValue({ success: false }), +})); + +vi.mock("../../../src/services/gitmem-dir.js", () => ({ + getGitmemPath: (filename: string) => `${root.dir}/${filename}`, + getGitmemDir: () => root.dir, + getSessionPath: (sid: string, filename: string) => `${root.dir}/sessions/${sid}/${filename}`, +})); + +vi.mock("../../../src/services/active-sessions.js", () => ({ + unregisterSession: vi.fn(), + findSessionByHostPid: vi.fn().mockReturnValue(null), +})); + +vi.mock("../../../src/services/thread-suggestions.js", () => ({ + loadSuggestions: vi.fn().mockReturnValue([]), + saveSuggestions: vi.fn(), + detectSuggestedThreads: vi.fn().mockReturnValue([]), + loadRecentSessionEmbeddings: vi.fn().mockResolvedValue(null), +})); + +import { sessionClose } from "../../../src/tools/session-close.js"; + +const SID = "393adb34-a80c-4c3a-b71a-bc0053b7a7ea"; +const payloadFile = () => path.join(root.dir, "closing-payload.json"); + +beforeEach(() => { + vi.clearAllMocks(); + root.dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git99-")); +}); + +afterEach(() => { + fs.rmSync(root.dir, { recursive: true, force: true }); +}); + +describe("session_close missing payload (GIT-99)", () => { + it("standard close, no payload, no inline reflection: names the absolute path", async () => { + const result = await sessionClose({ session_id: SID, close_type: "standard" }); + + expect(result.success).toBe(false); + expect(result.validation_errors).toHaveLength(1); + expect(result.validation_errors![0]).toBe( + `closing-payload.json not found at ${payloadFile()}. ` + + `Write the closing payload to exactly that path (or pass closing_reflection inline), then call session_close again.` + ); + expect(path.isAbsolute(result.validation_errors![0].split(" not found at ")[1].split(". Write")[0])).toBe(true); + expect(result.validation_errors!.join(" ")).not.toMatch(/answers|task_completion|recall\(\)/); + }); + + it("an unparseable payload is reported as found-but-unreadable, at its path", async () => { + fs.writeFileSync(payloadFile(), "{ not json"); + const result = await sessionClose({ session_id: SID, close_type: "standard" }); + + expect(result.success).toBe(false); + expect(result.validation_errors![0]).toContain(`closing-payload.json at ${payloadFile()} could not be read:`); + }); + + it("an inline reflection is enough — no payload file needed", async () => { + const result = await sessionClose({ + session_id: SID, + close_type: "standard", + closing_reflection: { what_broke: "x", what_worked: "y", scars_applied: [] }, + }); + expect((result.validation_errors || []).join(" ")).not.toContain("closing-payload.json not found"); + }); + + it("a payload at the path is consumed", async () => { + fs.writeFileSync(payloadFile(), JSON.stringify({ closing_reflection: { what_broke: "x", what_worked: "y", scars_applied: [] } })); + const result = await sessionClose({ session_id: SID, close_type: "standard" }); + expect((result.validation_errors || []).join(" ")).not.toContain("closing-payload.json"); + }); + + it("quick close never needs a payload", async () => { + const result = await sessionClose({ session_id: SID, close_type: "quick" }); + expect((result.validation_errors || []).join(" ")).not.toContain("closing-payload.json"); + }); +}); diff --git a/tests/unit/tools/session-close-supabase.test.ts b/tests/unit/tools/session-close-supabase.test.ts index d0d2872..70f0998 100644 --- a/tests/unit/tools/session-close-supabase.test.ts +++ b/tests/unit/tools/session-close-supabase.test.ts @@ -347,6 +347,12 @@ describe("session_close Supabase path", () => { }); }); + // GIT-99: a standard close with neither a payload nor an inline reflection is + // now rejected up front ("closing-payload.json not found at "). These + // tests are about titles and decisions, not validation (which is mocked), so + // they carry a reflection to reach the persistence path they check. + const REFLECTION = { what_broke: "n/a", what_worked: "n/a", scars_applied: [] }; + describe("session title update", () => { it("updates generic title with Linear issue + decision", async () => { vi.mocked(supabase.getRecord).mockResolvedValue( @@ -356,6 +362,7 @@ describe("session_close Supabase path", () => { await sessionClose({ session_id: VALID_UUID, close_type: "standard", + closing_reflection: REFLECTION, linear_issue: "PROJ-123", decisions: [{ title: "Chose X over Y", decision: "X", rationale: "because" }], }); @@ -372,6 +379,7 @@ describe("session_close Supabase path", () => { await sessionClose({ session_id: VALID_UUID, close_type: "standard", + closing_reflection: REFLECTION, linear_issue: "PROJ-123", decisions: [{ title: "Chose X over Y", decision: "X", rationale: "because" }], }); @@ -388,6 +396,7 @@ describe("session_close Supabase path", () => { await sessionClose({ session_id: VALID_UUID, close_type: "standard", + closing_reflection: REFLECTION, decisions: [ { title: "Decision A", decision: "Chose A", rationale: "faster" }, { title: "Decision B", decision: "Chose B", rationale: "simpler" }, From f1a18421528627a787f59ece4158dd02d4b79c66 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:20:03 -0700 Subject: [PATCH 07/24] test: venue flow checks a payload-less close names the absolute payload path (GIT-99) Exits 1 on 1.10.0 ("requires closing_reflection with 7 answers"); exits 0 with the fix. Co-Authored-By: Claude Opus 5 --- tests/e2e/blank-supabase.mjs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..cb53a70 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -366,6 +366,13 @@ async function flow() { const lt = await step("list_threads", { project: PROJECT }); const ltOpen = Number((lt.match(/(\d+) open/) || [])[1] ?? -1); + // GIT-99: a standard close with no payload file and no inline reflection + // names the absolute path this server reads, instead of "requires N answers". + const expectedPayloadPath = join(gitmemDir, "closing-payload.json"); + const scNoPayload = await step("session_close", { session_id: sessionId, close_type: "standard" }); + const missingPayloadNamed = scNoPayload.includes(`closing-payload.json not found at ${expectedPayloadPath}`) + && !/requires closing_reflection|requires task_completion/.test(scNoPayload); + const sc = await step("session_close", { session_id: sessionId, close_type: "standard", human_corrections: "none", closing_reflection: { @@ -408,6 +415,7 @@ async function flow() { remote_sessions_after: await count("gitmem_sessions"), remote_scar_usage_after: await count("gitmem_scar_usage"), session_close_persisted: closedRows.length === 1 && closedRows[0].closing_reflection != null, + missing_payload_named: missingPayloadNamed, relevance_readable: relevance.some((r) => scarIds.some((id) => (r.memories_applied || []).includes(id) && r.memory_relevance?.[id])), relevance, health_failed_total: failed, @@ -520,6 +528,9 @@ const failureCheck = checkFailures(allVenueRequests); const edgeCalls = allVenueRequests.filter((r) => r.path.startsWith("/functions/v1/")) .map((r) => `${r.method} ${r.path} -> ${r.status}`); if (edgeCalls.length) failureCheck.unexpected.push(...edgeCalls.map((c) => `edge function called (GIT-97): ${c}`)); +if (mode === "flow" && !result.missing_payload_named) { + failureCheck.unexpected.push("session_close without a payload did not name the absolute closing-payload.json path (GIT-99)"); +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } From 6fbe04593db0bd2bfdb7e5b13909df64cd8b278a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:30:01 -0700 Subject: [PATCH 08/24] fix: every write tool reports whether the write is durable; resolve_thread stops claiming success (GIT-101) resolve_thread computed supabaseSynced, logged it to metrics and returned success: true regardless, so a resolve the durable store never saw looked done while other sessions still saw the thread open. - WriteResult { success, durable, stored_in } (types + services/write-result.ts) is create_thread's contract, now shared: success = durable || !hasSupabase(), and false whenever nothing was stored. - Applied to create_learning, create_decision, create_thread, resolve_thread, archive_learning, record_scar_usage, record_scar_usage_batch, session_close, promote_suggestion (passes create_thread's outcome through) and save_transcript. Result types extend WriteResult, so every return states it. - resolve_thread on pro without a durable write: success false, stored_in local_only, and a display that says RESOLVED LOCALLY ONLY. - session_close reports whether the session row reached the store on every exit, including a failure after the row landed. One parameterized test, Supabase failing at the network, pro tier: each of the 10 tools reports success false, durable false, the right stored_in, and a display that says so. 10 of 12 fail on origin/main (resolve_thread: success true). +12 tests. Co-Authored-By: Claude Opus 5 --- src/services/write-result.ts | 47 ++++++ src/tools/archive-learning.ts | 15 +- src/tools/create-decision.ts | 5 +- src/tools/create-learning.ts | 7 +- src/tools/create-thread.ts | 14 +- src/tools/promote-suggestion.ts | 19 ++- src/tools/record-scar-usage-batch.ts | 11 +- src/tools/record-scar-usage.ts | 5 +- src/tools/resolve-thread.ts | 24 ++- src/tools/save-transcript.ts | 14 +- src/tools/session-close.ts | 28 ++-- src/types/index.ts | 30 ++-- .../unit/tools/write-result-contract.test.ts | 137 ++++++++++++++++++ 13 files changed, 292 insertions(+), 64 deletions(-) create mode 100644 src/services/write-result.ts create mode 100644 tests/unit/tools/write-result-contract.test.ts diff --git a/src/services/write-result.ts b/src/services/write-result.ts new file mode 100644 index 0000000..850fb8a --- /dev/null +++ b/src/services/write-result.ts @@ -0,0 +1,47 @@ +/** + * WriteResult — the one contract every write tool answers with (GIT-101). + * + * create_thread established it (GIT-63/GIT-67, R3/R4): say where the record + * landed and whether it survives this machine, and only call it a success + * when it reached the store of truth for this tier. Other write tools grew + * their own shapes — resolve_thread returned success: true whether or not the + * durable store accepted the write — so a caller could not tell a saved + * record from a lost one without reading prose. + * + * durable the record is in Supabase (pro/dev). Never true on free. + * stored_in "supabase" | "local" (free: the local store IS the truth) | + * "local_only" (pro: kept on this machine only, the durable + * write failed) | null (stored nowhere). + * success durable || !hasSupabase() — and false whenever nothing was + * stored at all. + */ + +import { hasSupabase } from "./tier.js"; +import type { WriteResult, StoredIn } from "../types/index.js"; + +export type { WriteResult, StoredIn }; + +/** + * The outcome of a write. + * + * @param durable the durable store (Supabase) accepted the write + * @param storedLocally a local copy exists (threads.json, the free-tier + * store, session state). Default: true on free, where + * reaching this call means the local write succeeded; + * false on pro, where a failed durable write keeps + * nothing unless the tool says it did. + */ +export function writeResult(durable: boolean, storedLocally: boolean = !hasSupabase()): WriteResult { + if (durable && hasSupabase()) return { success: true, durable: true, stored_in: "supabase" }; + if (!hasSupabase()) { + return storedLocally + ? { success: true, durable: false, stored_in: "local" } + : notStored(); + } + return { success: false, durable: false, stored_in: storedLocally ? "local_only" : null }; +} + +/** Nothing was stored anywhere. */ +export function notStored(): WriteResult { + return { success: false, durable: false, stored_in: null }; +} diff --git a/src/tools/archive-learning.ts b/src/tools/archive-learning.ts index 898413d..eeb020e 100644 --- a/src/tools/archive-learning.ts +++ b/src/tools/archive-learning.ts @@ -16,6 +16,8 @@ import { getStorage } from "../services/storage.js"; import { flushCache } from "../services/startup.js"; import { Timer } from "../services/metrics.js"; import { wrapDisplay } from "../services/display-protocol.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import type { WriteResult } from "../types/index.js"; export interface ArchiveLearningParams { /** UUID or short ID prefix of the learning to archive */ @@ -24,8 +26,7 @@ export interface ArchiveLearningParams { reason?: string; } -export interface ArchiveLearningResult { - success: boolean; +export interface ArchiveLearningResult extends WriteResult { id: string; archived_at?: string; reason?: string; @@ -95,7 +96,7 @@ export async function archiveLearning(params: ArchiveLearningParams): Promise {}); return { - success: true, + ...writeResult(hasSupabase()), decision_id: decisionId, performance: perfData, display: wrapDisplay(`Decision logged: "${params.title}"\nID: ${decisionId}`), @@ -151,7 +152,7 @@ export async function createDecision( const latencyMs = timer.stop(); const perfData = buildPerformanceData("create_decision", latencyMs, 0); return { - success: false, + ...notStored(), decision_id: "", performance: perfData, display: wrapDisplay(`Failed to log decision`), diff --git a/src/tools/create-learning.ts b/src/tools/create-learning.ts index b65a660..db9b929 100644 --- a/src/tools/create-learning.ts +++ b/src/tools/create-learning.ts @@ -32,6 +32,7 @@ import type { PerformanceBreakdown, ComponentPerformance, } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; /** * Validate scar-specific requirements @@ -85,7 +86,7 @@ export async function createLearning( const latencyMs = timer.stop(); const perfData = buildPerformanceData("create_learning", latencyMs, 0); return { - success: false, + ...notStored(), learning_id: "", embedding_generated: false, errors, @@ -314,7 +315,7 @@ export async function createLearning( const se = params.severity ? (SEV[params.severity] || "") + " " : ""; return { - success: true, + ...writeResult(hasSupabase()), learning_id: learningId, embedding_generated: embeddingGenerated, performance: perfData, @@ -327,7 +328,7 @@ export async function createLearning( const latencyMs = timer.stop(); const perfData = buildPerformanceData("create_learning", latencyMs, 0); return { - success: false, + ...notStored(), learning_id: "", embedding_generated: false, errors: [errorMsg], diff --git a/src/tools/create-thread.ts b/src/tools/create-thread.ts index 691de05..36e72d4 100644 --- a/src/tools/create-thread.ts +++ b/src/tools/create-thread.ts @@ -41,7 +41,7 @@ import { import { wrapDisplay, truncate } from "../services/display-protocol.js"; import { formatThreadForDisplay } from "../services/timezone.js"; import type { ThreadWithEmbedding } from "../services/thread-dedup.js"; -import type { ThreadObject, PerformanceData, Project } from "../types/index.js"; +import type { ThreadObject, PerformanceData, Project, WriteResult, StoredIn } from "../types/index.js"; // --- Types --- @@ -73,21 +73,17 @@ export type NotStoredReason = /** * Where a thread actually landed (R3: every response names its store). * `local_only` is the honest middle state from R4 — written somewhere, but - * not somewhere durable. + * not somewhere durable. GIT-101: this contract is now shared by every write + * tool (WriteResult). */ -export type StoredIn = "supabase" | "local" | "local_only" | null; +export type { StoredIn } from "../types/index.js"; -export interface CreateThreadResult { - success: boolean; +export interface CreateThreadResult extends WriteResult { /** * Whether a row exists. Distinct from `success` on purpose: a dedup refusal * is a well-formed answer to a well-formed request, not a crash. */ stored: boolean; - /** Whether what was stored survives this machine. */ - durable: boolean; - /** R3: names the store in-band, which also kills the f104e10d silent-local trap. */ - stored_in: StoredIn; reason?: NotStoredReason; thread?: ThreadObject; error?: string; diff --git a/src/tools/promote-suggestion.ts b/src/tools/promote-suggestion.ts index 1d00b5f..1b359d2 100644 --- a/src/tools/promote-suggestion.ts +++ b/src/tools/promote-suggestion.ts @@ -22,6 +22,8 @@ import { buildPerformanceData, } from "../services/metrics.js"; import type { ThreadObject, PerformanceData, Project, ThreadSuggestion } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import type { WriteResult } from "../types/index.js"; // --- Types --- @@ -30,8 +32,7 @@ export interface PromoteSuggestionParams { project?: Project; } -export interface PromoteSuggestionResult { - success: boolean; +export interface PromoteSuggestionResult extends WriteResult { thread?: ThreadObject; suggestion?: ThreadSuggestion; error?: string; @@ -49,7 +50,7 @@ export async function promoteSuggestion( if (!params.suggestion_id) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "suggestion_id is required", performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 0), display: wrapDisplay(`Failed: suggestion_id is required`), @@ -64,7 +65,7 @@ export async function promoteSuggestion( if (!target) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: `Pending suggestion not found: "${params.suggestion_id}"`, performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 0), display: wrapDisplay(`Suggestion not found: ${params.suggestion_id}`), @@ -79,8 +80,12 @@ export async function promoteSuggestion( if (!threadResult.success || !threadResult.thread) { const latencyMs = timer.stop(); + // GIT-101: pass create_thread's own outcome through (a local_only thread + // is not a promotion), rather than restating it. return { - success: false, + ...notStored(), + durable: threadResult.durable, + stored_in: threadResult.stored_in, error: `Thread creation failed: ${threadResult.error || "unknown"}`, performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 0), display: wrapDisplay(`Failed to promote suggestion`), @@ -93,7 +98,9 @@ export async function promoteSuggestion( const latencyMs = timer.stop(); return { - success: true, + success: threadResult.success, + durable: threadResult.durable, + stored_in: threadResult.stored_in, thread: threadResult.thread, suggestion: target, performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 1), diff --git a/src/tools/record-scar-usage-batch.ts b/src/tools/record-scar-usage-batch.ts index d364a08..5c634b7 100644 --- a/src/tools/record-scar-usage-batch.ts +++ b/src/tools/record-scar-usage-batch.ts @@ -15,6 +15,7 @@ import type { ScarUsageEntry, Project, } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; const TARGET_LATENCY_MS = 2000; // Target for batch operation @@ -98,9 +99,13 @@ export async function recordScarUsageBatch( const metricsId = uuidv4(); if (!hasSupabase()) { - // Free tier: scar usage tracked locally via record-scar-usage (single), not batch + // Free tier: scar usage tracked locally via record-scar-usage (single), not batch. + // GIT-101: nothing is stored here, and the result says so (stored_in: null); + // success stays true because there is no durable store to have missed. return { success: true, + durable: false, + stored_in: null, usage_ids: [], resolved_count: 0, failed_count: 0, @@ -191,7 +196,7 @@ export async function recordScarUsageBatch( }); return { - success: true, + ...writeResult(hasSupabase()), usage_ids: usageIds, resolved_count: resolvedCount, failed_count: failedIdentifiers.length, @@ -205,7 +210,7 @@ export async function recordScarUsageBatch( console.error("[record-scar-usage-batch] Error recording batch:", error); return { - success: false, + ...notStored(), error: error instanceof Error ? error.message : String(error), usage_ids: usageIds, resolved_count: resolvedCount, diff --git a/src/tools/record-scar-usage.ts b/src/tools/record-scar-usage.ts index 40ca688..9b5daa9 100644 --- a/src/tools/record-scar-usage.ts +++ b/src/tools/record-scar-usage.ts @@ -24,6 +24,7 @@ import type { RecordScarUsageResult, PerformanceBreakdown, } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; /** * Execute record_scar_usage tool @@ -101,7 +102,7 @@ export async function recordScarUsage( }).catch(() => {}); return { - success: true, + ...writeResult(hasSupabase()), usage_id: usageId, performance: perfData, display: wrapDisplay(`Scar usage recorded\nID: ${usageId}`), @@ -112,7 +113,7 @@ export async function recordScarUsage( const latencyMs = timer.stop(); const perfData = buildPerformanceData("record_scar_usage", latencyMs, 0); return { - success: false, + ...notStored(), usage_id: "", errors: [errorMsg], performance: perfData, diff --git a/src/tools/resolve-thread.ts b/src/tools/resolve-thread.ts index 6e0e0fb..3fbbc78 100644 --- a/src/tools/resolve-thread.ts +++ b/src/tools/resolve-thread.ts @@ -44,6 +44,8 @@ import { buildPerformanceData, } from "../services/metrics.js"; import { wrapDisplay } from "../services/display-protocol.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import { hasSupabase } from "../services/tier.js"; import { formatThreadForDisplay } from "../services/timezone.js"; import type { ResolveThreadParams, ResolveThreadResult, ThreadObject } from "../types/index.js"; @@ -57,7 +59,7 @@ export async function resolveThread( if (!params.thread_id && !params.text_match) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "Either thread_id or text_match is required", performance: buildPerformanceData("resolve_thread", latencyMs, 0), display: wrapDisplay(`Either thread_id or text_match is required`), @@ -87,7 +89,7 @@ export async function resolveThread( if (pos < 1 || pos > openThreads.length) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: `Thread #${pos} out of range (${openThreads.length} open threads)`, performance: buildPerformanceData("resolve_thread", latencyMs, 0), display: wrapDisplay(`Thread #${pos} out of range (${openThreads.length} open threads)`), @@ -162,7 +164,7 @@ export async function resolveThread( : `Thread not found: "${searchKey}"`; return { - success: false, + ...notStored(), error: message, performance: buildPerformanceData("resolve_thread", latencyMs, 0), display: wrapDisplay(message), @@ -193,7 +195,9 @@ export async function resolveThread( // Persist to local file (cache) saveThreadsFile(threads); - // Update Supabase (source of truth) — graceful fallback on failure + // Update Supabase (source of truth). GIT-101: the outcome is the result — + // it used to be computed, logged to metrics and then dropped, so a resolve + // the durable store never saw still answered success: true. let supabaseSynced = false; const supabaseSuccess = await resolveThreadInSupabase(resolved.id, { resolvedAt: resolved.resolved_at, @@ -275,8 +279,18 @@ export async function resolveThread( if (alsoResolved.length > 0) resolveMsg += `\nAlso resolved: ${alsoResolved.map(t => t.id).join(", ")}`; + // The local file was written above either way; on pro that alone is not a success. + const outcome = writeResult(supabaseSynced, true); + if (hasSupabase() && !outcome.durable) { + resolveMsg = + `RESOLVED LOCALLY ONLY — not durable (stored_in: local_only, durable: false).\n` + + `${resolveMsg}\n` + + `The durable store did not accept this resolve, so other sessions still see ${resolved.id} as open. ` + + `Retry when the store is reachable.`; + } + return { - success: true, + ...outcome, resolved_thread: formatThreadForDisplay(resolved), ...(alsoResolved.length > 0 && { also_resolved: alsoResolved.map(formatThreadForDisplay), diff --git a/src/tools/save-transcript.ts b/src/tools/save-transcript.ts index 88a487d..b48e6f5 100644 --- a/src/tools/save-transcript.ts +++ b/src/tools/save-transcript.ts @@ -12,6 +12,8 @@ import * as supabase from "../services/supabase-client.js"; import { Timer, buildPerformanceData } from "../services/metrics.js"; import { getProject } from "../services/session-state.js"; import type { Project, PerformanceData, PerformanceBreakdown } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import type { WriteResult } from "../types/index.js"; export interface SaveTranscriptParams { session_id: string; @@ -20,8 +22,7 @@ export interface SaveTranscriptParams { project?: Project; } -export interface SaveTranscriptResult { - success: boolean; +export interface SaveTranscriptResult extends WriteResult { transcript_path?: string; size_bytes?: number; size_kb?: number; @@ -43,7 +44,7 @@ export async function saveTranscript( if (!params.session_id) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "session_id is required", performance: buildPerformanceData("save_transcript", latencyMs, 0), }; @@ -52,7 +53,7 @@ export async function saveTranscript( if (!params.transcript) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "transcript content is required", performance: buildPerformanceData("save_transcript", latencyMs, 0), }; @@ -108,7 +109,8 @@ export async function saveTranscript( const estimatedTokens = Math.ceil(result.size_bytes / 4); return { - success: true, + // Transcripts only go to the durable store; reaching here means it took them. + ...writeResult(true), transcript_path: result.transcript_path, size_bytes: result.size_bytes, size_kb: Math.round(result.size_bytes / 1024 * 10) / 10, @@ -122,7 +124,7 @@ export async function saveTranscript( const latencyMs = timer.stop(); const errorMessage = error instanceof Error ? error.message : String(error); return { - success: false, + ...notStored(), error: `Failed to save transcript: ${errorMessage}`, performance: buildPerformanceData("save_transcript", latencyMs, 0), }; diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index 2388a4c..9cfe54a 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -36,6 +36,7 @@ import { import { wrapDisplay, truncate, productLine, boldText, dimText, STATUS, ANSI } from "../services/display-protocol.js"; import { queryScarUsageByDateRange, enrichScarUsageTitles, formatBlindspotSnippet } from "../services/analytics.js"; import { recordScarUsageBatch } from "./record-scar-usage-batch.js"; +import { writeResult, notStored } from "../services/write-result.js"; import { getEffectTracker } from "../services/effect-tracker.js"; import { saveTranscript } from "./save-transcript.js"; import { processTranscript } from "../services/transcript-chunker.js"; @@ -318,7 +319,7 @@ async function sessionCloseFree( const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, true); return { - success: true, + ...writeResult(false), session_id: sessionId, close_compliance: closeCompliance, performance: perfData, @@ -332,7 +333,7 @@ async function sessionCloseFree( const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`]); return { - success: false, + ...notStored(), session_id: sessionId, close_compliance: closeCompliance, validation_errors: [`Failed to persist session: ${errorMessage}`], @@ -862,7 +863,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: params.session_id, close_compliance: { close_type: params.close_type, @@ -908,7 +909,7 @@ export async function sessionClose( if (!params.session_id && params.close_type !== "retroactive") { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), session_id: "", close_compliance: { close_type: params.close_type, @@ -932,6 +933,9 @@ export async function sessionClose( // This keeps the visible MCP tool call small: just session_id + close_type. const payloadPath = getGitmemPath("closing-payload.json"); let payloadConsumed = false; + // GIT-101: whether the session row reached the durable store. Set only after + // the upsert resolves, so every exit below can say so. + let sessionRowStored = false; try { if (fs.existsSync(payloadPath)) { const filePayload = JSON.parse(fs.readFileSync(payloadPath, "utf-8")) as Partial; @@ -1066,7 +1070,7 @@ export async function sessionClose( // Hard gate: quick close requires session under 30 minutes if (params.close_type === "quick" && activity.duration_min >= 30) { return { - success: false, + ...notStored(), session_id: params.session_id || "", close_compliance: { close_type: "quick", @@ -1099,7 +1103,7 @@ export async function sessionClose( // Exemptions: quick (micro sessions), autonomous (CODA-1), hasReflection (agent already wrote full reflection) if (params.close_type === "standard" && !isRecallCalled() && !hasReflection) { return { - success: false, + ...notStored(), session_id: params.session_id || "", close_compliance: { close_type: "standard", @@ -1155,7 +1159,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: "", // Empty string when no session found close_compliance: { close_type: params.close_type, @@ -1179,7 +1183,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: "", // Empty string when search fails close_compliance: { close_type: params.close_type, @@ -1206,7 +1210,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: params.session_id, close_compliance: { close_type: params.close_type, @@ -1304,7 +1308,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: sessionId, close_compliance: closeCompliance, validation_errors: [`Session ${sessionId} not found in Supabase, local files, or registry. Was session_start called?`], @@ -1470,6 +1474,7 @@ export async function sessionClose( supabase.directUpsert(sessionsTable, sessionRow), blindspotPromise, ]); + sessionRowStored = true; // Tracked fire-and-forget embedding generation + session update + thread detection if (isEmbeddingAvailable()) { @@ -1645,6 +1650,8 @@ export async function sessionClose( const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, !partialPersist, allErrors.length > 0 ? allErrors : undefined, transcriptStatus, blindspotSnippet); return { + // The session row is durable; success also requires every thread to sync. + ...writeResult(true), success: !partialPersist, session_id: sessionId, close_compliance: closeCompliance, @@ -1663,6 +1670,7 @@ export async function sessionClose( const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], transcriptStatus, blindspotSnippet); return { + ...writeResult(sessionRowStored), success: false, session_id: sessionId, close_compliance: closeCompliance, diff --git a/src/types/index.ts b/src/types/index.ts index 61654e7..a7468bf 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -291,8 +291,21 @@ export interface CloseCompliance { retroactive?: boolean; // Optional: marks sessions created post-mortem } -export interface SessionCloseResult { +/** + * GIT-101: the contract every write tool answers with — where the record + * landed and whether it survives this machine. See services/write-result.ts. + */ +export type StoredIn = "supabase" | "local" | "local_only" | null; +export interface WriteResult { + /** durable || !hasSupabase(), and false when nothing was stored. */ success: boolean; + /** Whether what was stored survives this machine. */ + durable: boolean; + /** Names the store in-band. */ + stored_in: StoredIn; +} + +export interface SessionCloseResult extends WriteResult { session_id: string; close_compliance: CloseCompliance; validation_errors?: string[]; @@ -325,8 +338,7 @@ export interface CreateLearningParams { self_check_criteria?: string[]; } -export interface CreateLearningResult { - success: boolean; +export interface CreateLearningResult extends WriteResult { learning_id: string; embedding_generated: boolean; /** Error details when success=false */ @@ -348,8 +360,7 @@ export interface CreateDecisionParams { project?: Project; } -export interface CreateDecisionResult { - success: boolean; +export interface CreateDecisionResult extends WriteResult { decision_id: string; display?: string; performance: PerformanceData; @@ -445,8 +456,7 @@ export interface RecordScarUsageParams { variant_id?: string; // UUID of assigned variant for A/B testing } -export interface RecordScarUsageResult { - success: boolean; +export interface RecordScarUsageResult extends WriteResult { usage_id: string; /** Error details when success=false */ errors?: string[]; @@ -474,8 +484,7 @@ export interface RecordScarUsageBatchParams { project?: Project; } -export interface RecordScarUsageBatchResult { - success: boolean; +export interface RecordScarUsageBatchResult extends WriteResult { usage_ids: string[]; resolved_count: number; failed_count: number; @@ -598,8 +607,7 @@ export interface ResolveThreadParams { resolution_note?: string; } -export interface ResolveThreadResult { - success: boolean; +export interface ResolveThreadResult extends WriteResult { resolved_thread?: ThreadObject; /** Threads that were also resolved via duplicate cascade */ also_resolved?: ThreadObject[]; diff --git a/tests/unit/tools/write-result-contract.test.ts b/tests/unit/tools/write-result-contract.test.ts new file mode 100644 index 0000000..de7455e --- /dev/null +++ b/tests/unit/tools/write-result-contract.test.ts @@ -0,0 +1,137 @@ +/** + * GIT-101: every write tool answers with WriteResult, and none of them calls a + * write the durable store never saw a success. + * + * One parameterized test: Pro tier, Supabase configured, and every request to + * it fails at the network. Each tool runs its real write path and must report + * success: false, durable: false — and a stored_in that names what actually + * survived (local_only when a local copy was kept, null when nothing was). + * + * resolve_thread used to return success: true here unconditionally. + */ +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +const saved = vi.hoisted(() => { + const keys = ["GITMEM_TIER", "SUPABASE_URL", "SUPABASE_SERVICE_ROLE_KEY", "GITMEM_DIR", "OPENAI_API_KEY", "OPENROUTER_API_KEY", "GITMEM_EMBEDDING_PROVIDER"]; + const before = Object.fromEntries(keys.map((k) => [k, process.env[k]])); + const dir = require("fs").mkdtempSync(require("path").join(require("os").tmpdir(), "gitmem-git101-")); + Object.assign(process.env, { + GITMEM_TIER: "pro", + SUPABASE_URL: "https://stubbed-venue.supabase.co", + SUPABASE_SERVICE_ROLE_KEY: "stub-key", + GITMEM_DIR: dir, + }); + delete process.env.OPENAI_API_KEY; + delete process.env.OPENROUTER_API_KEY; + delete process.env.GITMEM_EMBEDDING_PROVIDER; + return { before, dir }; +}); + +import { resetTier, hasSupabase } from "../../../src/services/tier.js"; +import { setCurrentSession, clearCurrentSession } from "../../../src/services/session-state.js"; +import { saveThreadsFile } from "../../../src/services/thread-manager.js"; +import { saveSuggestions } from "../../../src/services/thread-suggestions.js"; +import { createLearning } from "../../../src/tools/create-learning.js"; +import { createDecision } from "../../../src/tools/create-decision.js"; +import { createThread } from "../../../src/tools/create-thread.js"; +import { resolveThread } from "../../../src/tools/resolve-thread.js"; +import { archiveLearning } from "../../../src/tools/archive-learning.js"; +import { recordScarUsage } from "../../../src/tools/record-scar-usage.js"; +import { recordScarUsageBatch } from "../../../src/tools/record-scar-usage-batch.js"; +import { sessionClose } from "../../../src/tools/session-close.js"; +import { promoteSuggestion } from "../../../src/tools/promote-suggestion.js"; +import { saveTranscript } from "../../../src/tools/save-transcript.js"; + +const SID = "393adb34-a80c-4c3a-b71a-bc0053b7a7ea"; +const SCAR = "11111111-2222-4333-8444-555555555555"; +const now = () => new Date().toISOString(); +let supabaseCalls = 0; + +type Case = { + tool: string; + run: () => Promise<{ success: boolean; durable: boolean; stored_in: unknown; display?: string }>; + /** What a failed durable write leaves behind. */ + storedIn: "local_only" | null; + setup?: () => void; +}; + +const CASES: Case[] = [ + { tool: "create_learning", storedIn: null, + run: () => createLearning({ learning_type: "win", title: "t", description: "d" } as never) }, + { tool: "create_decision", storedIn: null, + run: () => createDecision({ title: "t", decision: "d", rationale: "r" }) }, + { tool: "create_thread", storedIn: "local_only", + run: () => createThread({ text: "GIT-101 contract thread", allow_duplicate: true }) }, + { tool: "resolve_thread", storedIn: "local_only", + setup: () => saveThreadsFile([{ id: "t-git10101", text: "resolve me", status: "open", created_at: now() } as never]), + run: () => resolveThread({ thread_id: "t-git10101" }) }, + { tool: "archive_learning", storedIn: null, + run: () => archiveLearning({ id: SCAR }) }, + { tool: "record_scar_usage", storedIn: null, + run: () => recordScarUsage({ scar_id: SCAR, surfaced_at: now(), reference_type: "explicit", reference_context: "c" }) }, + { tool: "record_scar_usage_batch", storedIn: null, + run: () => recordScarUsageBatch({ scars: [{ scar_identifier: SCAR, surfaced_at: now(), reference_type: "explicit", reference_context: "c" }] } as never) }, + { tool: "session_close", storedIn: null, + run: () => sessionClose({ session_id: SID, close_type: "standard", human_corrections: "none", + closing_reflection: { what_broke: "x", what_took_longer: "x", do_differently: "x", what_worked: "x", + wrong_assumption: "x", scars_applied: [], institutional_memory_items: "x", collaborative_dynamic: "x", rapport_notes: "x" } } as never) }, + { tool: "promote_suggestion", storedIn: "local_only", + setup: () => saveSuggestions([{ id: "ts-git10101", text: "promote me", embedding: null, evidence_sessions: ["a", "b", "c"], + similarity_score: 0.9, status: "pending", created_at: now() } as never]), + run: () => promoteSuggestion({ suggestion_id: "ts-git10101" }) }, + { tool: "save_transcript", storedIn: null, + run: () => saveTranscript({ session_id: SID, transcript: "hello" }) }, +]; + +beforeAll(() => { + resetTier(); + vi.stubGlobal("fetch", vi.fn(async (input: unknown) => { + const url = String(input instanceof Request ? input.url : input); + if (url.includes("stubbed-venue.supabase.co")) supabaseCalls++; + throw new TypeError("fetch failed (GIT-101 stub: store unreachable)"); + })); +}); + +afterAll(() => { + vi.unstubAllGlobals(); + clearCurrentSession(); + for (const [k, v] of Object.entries(saved.before)) { + if (v === undefined) delete process.env[k]; else process.env[k] = v; + } + resetTier(); + fs.rmSync(saved.dir, { recursive: true, force: true }); +}); + +beforeEach(() => { + supabaseCalls = 0; + setCurrentSession({ sessionId: SID, agent: "cli", project: "default", startedAt: new Date() }); +}); + +describe("WriteResult contract with Supabase failing (GIT-101)", () => { + it("runs on the pro tier", () => { + expect(hasSupabase()).toBe(true); + }); + + it.each(CASES)("$tool: success false, durable false, stored_in $storedIn", async ({ run, setup, storedIn }) => { + setup?.(); + const result = await run(); + + expect(supabaseCalls).toBeGreaterThan(0); // the durable store was actually tried + expect(result.success).toBe(false); + expect(result.durable).toBe(false); + expect(result.stored_in).toBe(storedIn); + // Over MCP a client sees only `display` when a tool sets one (server.ts), + // so the display must carry the same verdict as the fields. + const display = (result as { display?: string }).display; + if (display !== undefined) expect(display).toMatch(/fail|not stored|not durable|locally only/i); + }); + + it("resolve_thread says so in its display, not just its fields", async () => { + saveThreadsFile([{ id: "t-git10102", text: "resolve me too", status: "open", created_at: now() } as never]); + const result = await resolveThread({ thread_id: "t-git10102" }); + expect(result.display).toContain("RESOLVED LOCALLY ONLY — not durable"); + }); +}); From e11d850cecaa30ffacf58aef0443761dffe9293d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:30:01 -0700 Subject: [PATCH 09/24] test: venue flow resolves a thread and checks it landed in gitmem_threads (GIT-101) Co-Authored-By: Claude Opus 5 --- tests/e2e/blank-supabase.mjs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..02a1eca 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -366,6 +366,17 @@ async function flow() { const lt = await step("list_threads", { project: PROJECT }); const ltOpen = Number((lt.match(/(\d+) open/) || [])[1] ?? -1); + // GIT-101: resolve a second driver thread; the durable store must record it + // and the response must not claim a local-only resolve as done. + const ct2 = await step("create_thread", { text: `Close out the quarterly vendor audit checklist (${label})`, allow_duplicate: true }); + const resolveId = (ct2.match(/\bt-[0-9a-f]{8}\b/) || [])[0] ?? null; + const rt = resolveId ? await step("resolve_thread", { thread_id: resolveId, resolution_note: "venue driver" }) : ""; + const resolvedRow = resolveId + ? await rest("GET", `gitmem_threads?select=status,resolved_by_session&thread_id=eq.${resolveId}`, undefined, { Prefer: "" }).then((r) => r.json()) + : []; + const resolveDurable = !!resolveId && /Thread resolved/.test(rt) && !/LOCALLY ONLY/.test(rt) + && resolvedRow[0]?.status === "resolved"; + const sc = await step("session_close", { session_id: sessionId, close_type: "standard", human_corrections: "none", closing_reflection: { @@ -408,6 +419,7 @@ async function flow() { remote_sessions_after: await count("gitmem_sessions"), remote_scar_usage_after: await count("gitmem_scar_usage"), session_close_persisted: closedRows.length === 1 && closedRows[0].closing_reflection != null, + resolve_thread_durable: resolveDurable, relevance_readable: relevance.some((r) => scarIds.some((id) => (r.memories_applied || []).includes(id) && r.memory_relevance?.[id])), relevance, health_failed_total: failed, @@ -520,6 +532,9 @@ const failureCheck = checkFailures(allVenueRequests); const edgeCalls = allVenueRequests.filter((r) => r.path.startsWith("/functions/v1/")) .map((r) => `${r.method} ${r.path} -> ${r.status}`); if (edgeCalls.length) failureCheck.unexpected.push(...edgeCalls.map((c) => `edge function called (GIT-97): ${c}`)); +if (mode === "flow" && !result.resolve_thread_durable) { + failureCheck.unexpected.push("resolve_thread did not land durably in gitmem_threads, or claimed it did not (GIT-101)"); +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } From 90690f19bf25ec4367d89ea36da49421b5f5981a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:36:47 -0700 Subject: [PATCH 10/24] fix: bounded write-path check with an UNVERIFIED verdict in health; self-describing close WARNs (GIT-102) - checkWritePathWithTimeout() races the startup probe against 10 s. A store that never answers gives "timed out after 10 s, durability UNVERIFIED" instead of silence; a late answer replaces it, marked late. server.ts uses it on both startup paths. - The last verdict is exposed through health (write_path, and a first line "Write path: ..."). It was visible only on stderr, and not at all on a hang. - A probe that errored with anything but a missing table was ignored, so a store refusing every connection logged "Write-path OK". It is now "unreachable", durability UNVERIFIED. - session_close's "WARN N write failures" becomes one line per failing subsystem: name, failure count, cause (its latest error), and a mandatory "session content stored OK / NOT stored" clause, from whether the session row actually landed. +14 tests; the unreachable and timeout cases fail on origin/main. Co-Authored-By: Claude Opus 5 --- src/server.ts | 12 +- src/services/write-health.ts | 125 +++++++++++++++++- src/tools/session-close.ts | 59 ++++++++- .../unit/services/write-health-probe.test.ts | 48 +++++++ .../services/write-health-timeout.test.ts | 111 ++++++++++++++++ 5 files changed, 343 insertions(+), 12 deletions(-) create mode 100644 tests/unit/services/write-health-probe.test.ts create mode 100644 tests/unit/services/write-health-timeout.test.ts diff --git a/src/server.ts b/src/server.ts index 956114d..9a5d3b6 100644 --- a/src/server.ts +++ b/src/server.ts @@ -64,7 +64,7 @@ import { flushCache, startBackgroundInit, } from "./services/startup.js"; -import { checkWritePath } from "./services/write-health.js"; +import { checkWritePathWithTimeout, getLastWritePathVerdict, formatWritePathLine } from "./services/write-health.js"; import { getEffectTracker } from "./services/effect-tracker.js"; import { RIPPLE, ANSI } from "./services/display-protocol.js"; import { getProject } from "./services/session-state.js"; @@ -371,9 +371,13 @@ export function createServer(): Server { const tracker = getEffectTracker(); const report = tracker.getHealthReport(failureLimit); const summary = tracker.formatSummary(); + // GIT-102: the startup write-path verdict, including "timed out, + // durability UNVERIFIED" — previously visible only on stderr, if at all. + const writePath = getLastWritePathVerdict(); result = { ...report, - text: summary || "No tracked effects this session.", + write_path: writePath, + text: `${formatWritePathLine(writePath)}\n\n${summary || "No tracked effects this session."}`, }; break; } @@ -501,14 +505,14 @@ export async function runServer(): Promise { } // Verify the write path with the FINAL tier (post-validation): catches an // invalid license silently downgrading writes to local files. - void checkWritePath(); + void checkWritePathWithTimeout(); // GIT-102: bounded; verdict kept for health }).catch((err) => { console.error(`[gitmem:license] Validation error: ${err}`); }); } else { // No license key to validate — verify the write path with the detected tier // (catches a GITMEM_TABLE_PREFIX / schema mismatch on the backward-compat path). - void checkWritePath(); + void checkWritePathWithTimeout(); // GIT-102: bounded; verdict kept for health } if (hasSupabase()) { diff --git a/src/services/write-health.ts b/src/services/write-health.ts index a424d76..40500d0 100644 --- a/src/services/write-health.ts +++ b/src/services/write-health.ts @@ -17,6 +17,12 @@ * Fire-and-forget: never throws (whole body is guarded), never blocks startup. * Logs a loud warning to stderr when misconfigured, a one-line confirmation * otherwise. + * + * GIT-102: the probe had no timeout. A store that accepts the connection and + * never answers left it pending forever — no warning, no confirmation, and + * nothing anywhere saying durability had not been checked. The server now runs + * it through checkWritePathWithTimeout(): raced against 10 s, with a + * "timed_out" verdict that says so, and the last verdict kept for `health`. */ import { getTier, hasSupabase, getTablePrefix, getTableName } from "./tier.js"; @@ -30,12 +36,115 @@ export type WritePathMode = | "free_with_credentials" // creds present but tier free — writes silently local (bug) | "missing_tables" // pro/dev but resolved tables absent (bug) | "supabase" // healthy: pro/dev with tables present - | "skipped"; // unexpected internal error — stayed silent, did not block startup + | "skipped" // unexpected internal error — stayed silent, did not block startup + | "timed_out" // GIT-102: no answer within the budget — durability UNVERIFIED + | "unreachable"; // GIT-102: probes errored (network/auth) — durability UNVERIFIED export interface WritePathResult { ok: boolean; mode: WritePathMode; missing?: string[]; + /** unreachable: the probe error, as the store reported it. */ + error?: string; +} + +/** The most recent write-path verdict, for `health` (GIT-102). */ +export interface WritePathVerdict extends WritePathResult { + /** One line a person can act on. */ + summary: string; + checked_at: string; + duration_ms: number; + /** A check that finished after its timeout had already been reported. */ + late?: boolean; +} + +export const WRITE_PATH_TIMEOUT_MS = 10_000; + +let lastVerdict: WritePathVerdict | null = null; + +/** The last write-path verdict, or null when no check has finished or timed out yet. */ +export function getLastWritePathVerdict(): WritePathVerdict | null { + return lastVerdict; +} + +/** For tests. */ +export function resetWritePathVerdict(): void { + lastVerdict = null; +} + +/** The `health` line for a verdict (GIT-102). */ +export function formatWritePathLine(v: WritePathVerdict | null): string { + if (!v) return "Write path: not checked yet (the startup check has not finished or timed out)"; + return `Write path: ${v.summary} (${v.ok ? "ok" : "NOT OK"}, checked ${v.checked_at}, ${v.duration_ms} ms` + + `${v.late ? ", finished after its timeout" : ""})`; +} + +function summarize(r: WritePathResult, timeoutMs: number): string { + switch (r.mode) { + case "supabase": return "Supabase — learnings/decisions tables present, writes are durable"; + case "local": return "local files (free tier, no Supabase configured) — intended"; + case "free_with_credentials": return "Supabase is configured but the tier resolved to FREE — writes are going to local files, NOT Supabase"; + case "missing_tables": return `tables missing on the Supabase backend (${(r.missing || []).join(", ")}) — create_learning / create_decision will fail`; + case "skipped": return "check skipped after an internal error — durability UNVERIFIED"; + case "timed_out": return `timed out after ${Math.round(timeoutMs / 1000)} s, durability UNVERIFIED`; + case "unreachable": return `store unreachable (${r.error ?? "probe failed"}), durability UNVERIFIED`; + } +} + +/** + * Run checkWritePath() against a deadline and record the verdict. + * + * Resolves by the deadline at the latest. If the check finishes afterwards, + * its real verdict replaces "timed_out" (marked late) — the store answered + * slowly, and the latest word on it is the one health should show. + */ +export async function checkWritePathWithTimeout( + timeoutMs: number = WRITE_PATH_TIMEOUT_MS, + check: () => Promise = checkWritePath +): Promise { + const started = Date.now(); + let timedOut = false; + let timer: ReturnType | undefined; + + const record = (r: WritePathResult, late = false): WritePathVerdict => { + lastVerdict = { + ...r, + summary: summarize(r, timeoutMs), + checked_at: new Date().toISOString(), + duration_ms: Date.now() - started, + ...(late && { late: true }), + }; + return lastVerdict; + }; + + const checked = check().then((r) => { + if (timedOut) { + const v = record(r, true); + console.error(`[gitmem] Write-path check finished late (${v.duration_ms} ms): ${v.summary}`); + return v; + } + return record(r); + }); + + const deadline = new Promise((resolve) => { + timer = setTimeout(() => { + timedOut = true; + const v = record({ ok: false, mode: "timed_out" }); + console.error( + `\n\u26a0\ufe0f [gitmem] WRITE PATH: ${v.summary}.\n` + + " The Supabase store did not answer the write-path probe. Writes may or may not be landing;\n" + + " check connectivity to SUPABASE_URL. `health` shows the latest verdict.\n" + ); + resolve(v); + }, timeoutMs); + timer.unref?.(); + }); + + try { + return await Promise.race([checked, deadline]); + } finally { + if (timer) clearTimeout(timer); + } } export async function checkWritePath(): Promise { @@ -65,6 +174,7 @@ export async function checkWritePath(): Promise { const prefix = getTablePrefix(); const prefixSource = process.env.GITMEM_TABLE_PREFIX ? "GITMEM_TABLE_PREFIX" : "default"; const missing: string[] = []; + const probeErrors: string[] = []; for (const base of ["learnings", "decisions"]) { const table = getTableName(base); @@ -73,7 +183,9 @@ export async function checkWritePath(): Promise { } catch (err) { const msg = err instanceof Error ? err.message : String(err); if (SCHEMA_MISS.test(msg)) missing.push(table); - // Transient network/auth errors are out of scope for this check. + // GIT-102: a network/auth error used to be ignored here, so a store + // that refused every connection was reported as "Write-path OK". + else probeErrors.push(`${table}: ${msg}`); } } @@ -89,6 +201,15 @@ export async function checkWritePath(): Promise { return { ok: false, mode: "missing_tables", missing }; } + if (probeErrors.length > 0) { + const error = probeErrors.join("; ").slice(0, 300); + console.error( + "\n\u26a0\ufe0f [gitmem] WRITE PATH: the Supabase store could not be probed — durability UNVERIFIED.\n" + + ` ${error}\n` + ); + return { ok: false, mode: "unreachable", error }; + } + console.error( `[gitmem] Write-path OK (tier ${getTier()}, prefix "${prefix}", learnings/decisions present).` ); diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index 2388a4c..b955b5f 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -329,7 +329,7 @@ async function sessionCloseFree( clearCurrentSession(); const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); - const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`]); + const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], undefined, undefined, false); return { success: false, @@ -363,6 +363,8 @@ function formatCloseDisplay( errors?: string[], transcriptStatus?: TranscriptStatus, blindspotSnippet?: string | null, + /** GIT-102: whether the session content itself landed; defaults to `success`. */ + sessionStored: boolean = success, ): string { const lines: string[] = []; @@ -434,15 +436,55 @@ function formatCloseDisplay( } // Write health — only on failure - const healthReport = getEffectTracker().getHealthReport(); - if (healthReport.overall.failed > 0) { + const warnings = formatWriteWarnings(getEffectTracker().getHealthReport(), sessionStored); + if (warnings.length > 0) { lines.push(""); - lines.push(`${STATUS.warn} ${healthReport.overall.failed} write failure${healthReport.overall.failed > 1 ? "s" : ""}`); + for (const w of warnings) lines.push(`${STATUS.warn} ${w}`); } return wrapDisplay(lines.join("\n")); } +/** GIT-102: what each tracked write path is, in words. */ +const SUBSYSTEM_NAMES: Record = { + triple_write: "knowledge graph (triples)", + embedding: "session embedding", + scar_usage: "scar usage records", + transcript: "transcript upload", + metrics: "query metrics", + relevance_update: "relevance feedback", + variant_generation: "scar variants", + cache_set: "local cache", +}; + +/** + * GIT-102: one self-describing line per failing write subsystem. + * + * The close used to print "WARN 3 write failures" and nothing else: not which + * writes, not why, and — the question every reader actually has — not whether + * the session itself was saved. Each line now names the subsystem, the cause + * (its most recent error), and states whether the session content was stored. + * That last clause is mandatory on every line, so no WARN can be read as + * "the close failed" when it didn't, or as harmless when it wasn't. + */ +export function formatWriteWarnings( + report: ReturnType["getHealthReport"]>, + sessionStored: boolean +): string[] { + const where = hasSupabase() ? "" : " (local)"; + const stored = sessionStored ? `session content stored OK${where}` : "session content NOT stored"; + const lines: string[] = []; + for (const [path, s] of Object.entries(report.byPath)) { + if (s.failed === 0) continue; + const name = SUBSYSTEM_NAMES[path] ?? path; + const cause = s.lastFailure?.error + ? truncate(s.lastFailure.error.replace(/\s+/g, " ").trim(), 160) + : "no error message recorded"; + lines.push(`${name}: ${s.failed} of ${s.attempted} write${s.attempted > 1 ? "s" : ""} failed — ${cause} · ${stored}`); + } + return lines; +} + /** * GIT-21: Clean up all session files for a closed session. * Unregisters from registry, deletes per-session directory, and removes legacy file. @@ -932,6 +974,9 @@ export async function sessionClose( // This keeps the visible MCP tool call small: just session_id + close_type. const payloadPath = getGitmemPath("closing-payload.json"); let payloadConsumed = false; + // Whether the session row reached the durable store (GIT-102: the close + // display says so on every WARN; set only after the upsert resolves). + let sessionRowStored = false; try { if (fs.existsSync(payloadPath)) { const filePayload = JSON.parse(fs.readFileSync(payloadPath, "utf-8")) as Partial; @@ -1470,6 +1515,7 @@ export async function sessionClose( supabase.directUpsert(sessionsTable, sessionRow), blindspotPromise, ]); + sessionRowStored = true; // Tracked fire-and-forget embedding generation + session update + thread detection if (isEmbeddingAvailable()) { @@ -1642,7 +1688,8 @@ export async function sessionClose( const allErrors = [...persistErrors, ...coverageWarnings, ...validation.warnings]; - const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, !partialPersist, allErrors.length > 0 ? allErrors : undefined, transcriptStatus, blindspotSnippet); + // The session row is upserted above; a partial persist is threads, not the session. + const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, !partialPersist, allErrors.length > 0 ? allErrors : undefined, transcriptStatus, blindspotSnippet, true); return { success: !partialPersist, @@ -1660,7 +1707,7 @@ export async function sessionClose( // Clear session state even on error (session is done either way) clearCurrentSession(); - const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], transcriptStatus, blindspotSnippet); + const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], transcriptStatus, blindspotSnippet, sessionRowStored); return { success: false, diff --git a/tests/unit/services/write-health-probe.test.ts b/tests/unit/services/write-health-probe.test.ts new file mode 100644 index 0000000..b3e7b8a --- /dev/null +++ b/tests/unit/services/write-health-probe.test.ts @@ -0,0 +1,48 @@ +/** + * GIT-102: checkWritePath's verdicts, store mocked at the client. + * + * A probe that errors for any reason other than a missing table used to be + * ignored, so a store refusing every connection was reported "Write-path OK". + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const probe = vi.hoisted(() => ({ impl: async (): Promise => [] })); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + isConfigured: () => true, + directQuery: vi.fn(() => probe.impl()), +})); +vi.mock("../../../src/services/tier.js", () => ({ + getTier: () => "pro", + hasSupabase: () => true, + getTablePrefix: () => "gitmem_", + getTableName: (b: string) => `gitmem_${b}`, +})); + +import { checkWritePath } from "../../../src/services/write-health.js"; + +beforeEach(() => { probe.impl = async () => []; }); + +describe("checkWritePath verdicts (GIT-102)", () => { + it("tables answer: supabase, ok", async () => { + expect(await checkWritePath()).toEqual({ ok: true, mode: "supabase" }); + }); + + it("a missing table: missing_tables", async () => { + probe.impl = async () => { throw new Error("PGRST205 Could not find the table"); }; + expect(await checkWritePath()).toMatchObject({ ok: false, mode: "missing_tables", missing: ["gitmem_learnings", "gitmem_decisions"] }); + }); + + it("the store refuses the connection: unreachable, NOT ok (was: Write-path OK)", async () => { + probe.impl = async () => { throw new TypeError("fetch failed: ECONNREFUSED"); }; + const r = await checkWritePath(); + expect(r.ok).toBe(false); + expect(r.mode).toBe("unreachable"); + expect(r.error).toContain("ECONNREFUSED"); + }); + + it("an auth failure is unreachable too, not OK", async () => { + probe.impl = async () => { throw new Error("Supabase query error: 401 - Invalid API key"); }; + expect(await checkWritePath()).toMatchObject({ ok: false, mode: "unreachable" }); + }); +}); diff --git a/tests/unit/services/write-health-timeout.test.ts b/tests/unit/services/write-health-timeout.test.ts new file mode 100644 index 0000000..b573ab7 --- /dev/null +++ b/tests/unit/services/write-health-timeout.test.ts @@ -0,0 +1,111 @@ +/** + * GIT-102: diagnostics that say what they know and what they don't. + * + * 1. The startup write-path probe is raced against a deadline. A store that + * never answers yields "timed out, durability UNVERIFIED" — not silence. + * The last verdict is kept for `health`, and a late answer replaces it. + * 2. session_close's WARN names the subsystem, the cause, and always whether + * the session content itself was stored. + */ +import { describe, it, expect, beforeEach } from "vitest"; +import { + checkWritePathWithTimeout, + getLastWritePathVerdict, + resetWritePathVerdict, + formatWritePathLine, + WRITE_PATH_TIMEOUT_MS, +} from "../../../src/services/write-health.js"; +import type { WritePathResult } from "../../../src/services/write-health.js"; +import { formatWriteWarnings } from "../../../src/tools/session-close.js"; + +const never = () => new Promise(() => {}); +const after = (ms: number, r: WritePathResult) => () => new Promise((res) => setTimeout(() => res(r), ms)); + +beforeEach(() => resetWritePathVerdict()); + +describe("write-path check timeout (GIT-102)", () => { + it("defaults to a 10 s budget", () => { + expect(WRITE_PATH_TIMEOUT_MS).toBe(10_000); + }); + + it("a probe that never answers resolves at the deadline as timed_out, durability UNVERIFIED", async () => { + const t0 = Date.now(); + const v = await checkWritePathWithTimeout(60, never); + expect(Date.now() - t0).toBeLessThan(1000); + expect(v.ok).toBe(false); + expect(v.mode).toBe("timed_out"); + expect(v.summary).toMatch(/timed out after .* durability UNVERIFIED/); + expect(getLastWritePathVerdict()?.mode).toBe("timed_out"); + }); + + it("names the configured budget in seconds", async () => { + const v = await checkWritePathWithTimeout(10_000, () => Promise.resolve({ ok: false, mode: "timed_out" })); + expect(v.summary).toBe("timed out after 10 s, durability UNVERIFIED"); + }); + + it("a fast probe's verdict is recorded as-is", async () => { + const v = await checkWritePathWithTimeout(1000, () => Promise.resolve({ ok: true, mode: "supabase" })); + expect(v.mode).toBe("supabase"); + expect(v.late).toBeUndefined(); + expect(getLastWritePathVerdict()).toEqual(v); + }); + + it("a probe that answers after the deadline replaces timed_out, marked late", async () => { + const v = await checkWritePathWithTimeout(30, after(90, { ok: false, mode: "missing_tables", missing: ["gitmem_learnings"] })); + expect(v.mode).toBe("timed_out"); + await new Promise((r) => setTimeout(r, 120)); + const latest = getLastWritePathVerdict()!; + expect(latest.mode).toBe("missing_tables"); + expect(latest.late).toBe(true); + expect(latest.summary).toContain("gitmem_learnings"); + }); + + it("health's line says not-checked, UNVERIFIED, or ok — never nothing", async () => { + expect(formatWritePathLine(null)).toMatch(/not checked yet/); + const timedOut = await checkWritePathWithTimeout(20, never); + expect(formatWritePathLine(timedOut)).toMatch(/^Write path: timed out after .* durability UNVERIFIED \(NOT OK, checked /); + const ok = await checkWritePathWithTimeout(1000, () => Promise.resolve({ ok: true, mode: "supabase" })); + expect(formatWritePathLine(ok)).toMatch(/^Write path: Supabase — .* \(ok, checked /); + }); +}); + +describe("session_close WARN lines (GIT-102)", () => { + const report = (byPath: Record) => ({ + byPath: Object.fromEntries(Object.entries(byPath).map(([k, v]) => [k, { + attempted: v.attempted, succeeded: v.attempted - v.failed, failed: v.failed, successRate: "", avgDurationMs: 0, + ...(v.error && { lastFailure: { error: v.error, timestamp: "" } }), + }])), + overall: { attempted: 0, succeeded: 0, failed: 0, successRate: "", paths_with_failures: [] }, + recentFailures: [], + uptimeMs: 0, + }); + + it("names subsystem and cause, and says the session content was stored", () => { + const lines = formatWriteWarnings(report({ + triple_write: { attempted: 3, failed: 3, error: 'POST knowledge_triples 400: invalid input syntax for type uuid: "t-abc"' }, + embedding: { attempted: 1, failed: 0 }, + }), true); + expect(lines).toHaveLength(1); + expect(lines[0]).toMatch(/^knowledge graph \(triples\): 3 of 3 writes failed — POST knowledge_triples 400: invalid input syntax/); + expect(lines[0]).toMatch(/· session content stored OK( \(local\))?$/); + }); + + it("says NOT stored when the session row did not land — on every line", () => { + const lines = formatWriteWarnings(report({ + triple_write: { attempted: 1, failed: 1, error: "boom" }, + transcript: { attempted: 1, failed: 1, error: "bucket missing" }, + }), false); + expect(lines).toHaveLength(2); + for (const l of lines) expect(l).toMatch(/· session content NOT stored$/); + expect(lines[1]).toMatch(/^transcript upload: 1 of 1 write failed — bucket missing/); + }); + + it("an unknown subsystem is named by its path, a missing cause is said to be missing", () => { + const [line] = formatWriteWarnings(report({ new_path: { attempted: 2, failed: 1 } }), true); + expect(line).toMatch(/^new_path: 1 of 2 writes failed — no error message recorded · session content stored OK/); + }); + + it("no failures, no WARN", () => { + expect(formatWriteWarnings(report({ triple_write: { attempted: 2, failed: 0 } }), true)).toEqual([]); + }); +}); From eef2f8e66781228fb2a5e4602d74e9adde55209f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:36:47 -0700 Subject: [PATCH 11/24] test: venue flow requires self-describing close WARNs and a write-path verdict in health (GIT-102) Exits 1 on 1.10.0 ("WARN 1 write failure"; health without a verdict). Co-Authored-By: Claude Opus 5 --- tests/e2e/blank-supabase.mjs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..98afedc 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -377,6 +377,12 @@ async function flow() { await waitQuiet(netlog, { minMs: 2000, quietMs: 3000 }); const health = await step("health", { failure_limit: 20 }); + // GIT-102: every WARN on the close names a subsystem and cause and says + // whether the session content was stored; health reports the write-path verdict. + const closeWarns = sc.replace(/\x1b\[[0-9;]*m/g, "").split("\n").filter((l) => /^WARN /.test(l)); + const closeWarnsSelfDescribing = closeWarns.every((l) => /^WARN .+: \d+ of \d+ writes? failed — .+ · session content (stored OK|NOT stored)/.test(l)); + const healthHasWritePath = /^Write path: /m.test(health); + // Did the close land? Is relevance readable from the store (GIT-109)? const closedRows = sessionId ? await rest("GET", `gitmem_sessions?select=id,closing_reflection&id=eq.${sessionId}`, undefined, { Prefer: "" }).then((r) => r.json()) @@ -408,6 +414,9 @@ async function flow() { remote_sessions_after: await count("gitmem_sessions"), remote_scar_usage_after: await count("gitmem_scar_usage"), session_close_persisted: closedRows.length === 1 && closedRows[0].closing_reflection != null, + close_warns: closeWarns, + close_warns_self_describing: closeWarnsSelfDescribing, + health_has_write_path: healthHasWritePath, relevance_readable: relevance.some((r) => scarIds.some((id) => (r.memories_applied || []).includes(id) && r.memory_relevance?.[id])), relevance, health_failed_total: failed, @@ -520,6 +529,12 @@ const failureCheck = checkFailures(allVenueRequests); const edgeCalls = allVenueRequests.filter((r) => r.path.startsWith("/functions/v1/")) .map((r) => `${r.method} ${r.path} -> ${r.status}`); if (edgeCalls.length) failureCheck.unexpected.push(...edgeCalls.map((c) => `edge function called (GIT-97): ${c}`)); +if (mode === "flow" && !result.close_warns_self_describing) { + failureCheck.unexpected.push(`session_close WARN is not self-describing (GIT-102): ${result.close_warns.join(" | ")}`); +} +if (mode === "flow" && !result.health_has_write_path) { + failureCheck.unexpected.push("health does not report the write-path verdict (GIT-102)"); +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } From a6f35ec584396c2a101d9a2f74697675581da299 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:41:50 -0700 Subject: [PATCH 12/24] fix: metrics wait for their session row instead of racing it to a 409 (GIT-73) session_start created its session row fire-and-forget and recorded its metrics at the same moment. gitmem_query_metrics.session_id references gitmem_sessions(id), so whichever request reached the store first decided whether the metrics insert failed with 409 (FK). recall right after session_start raced the same way. - session_start registers the pending session insert; recordMetrics waits for it (bounded at 15 s) before writing a row that references that session. - If the session row did not land, the metrics row is written without the FK and keeps the id in metadata.session_id, where session_close already reads it (GIT-109), instead of being lost. - Sessions this process did not create (resumed, another process's) are written immediately, as before. +5 tests. Co-Authored-By: Claude Opus 5 --- src/services/metrics.ts | 54 +++++++++++++- src/tools/session-start.ts | 15 +++- .../unit/services/metrics-session-fk.test.ts | 74 +++++++++++++++++++ 3 files changed, 137 insertions(+), 6 deletions(-) create mode 100644 tests/unit/services/metrics-session-fk.test.ts diff --git a/src/services/metrics.ts b/src/services/metrics.ts index 0211b88..16eac35 100644 --- a/src/services/metrics.ts +++ b/src/services/metrics.ts @@ -135,6 +135,48 @@ export class Timer { } } +/** + * GIT-73: session rows still being created, by session id. + * + * session_start creates its session row fire-and-forget and records its own + * metrics at the same moment, and gitmem_query_metrics.session_id references + * gitmem_sessions(id). Whichever request reached the store first decided + * whether the metrics insert 409'd (FK violation). recall right after + * session_start raced the same way. + * + * A metrics write for a session registered here waits for its row. If the row + * did not land (or takes longer than the wait budget), the metrics row is + * written without the FK and keeps the id in metadata.session_id — where + * session_close already looks for it (GIT-109) — instead of being lost to a 409. + * Entries stay after they settle so later writes know the answer too. + */ +const pendingSessionRows = new Map>(); +const SESSION_ROW_WAIT_MS = 15_000; + +export function registerPendingSessionRow(sessionId: string, created: Promise): void { + pendingSessionRows.set(sessionId, created.catch(() => false)); +} + +/** For tests. */ +export function clearPendingSessionRows(): void { + pendingSessionRows.clear(); +} + +async function sessionRowLanded(sessionId: string): Promise { + const pending = pendingSessionRows.get(sessionId); + if (!pending) return undefined; // not created by this process — nothing to wait for + let timer: ReturnType | undefined; + const budget = new Promise((resolve) => { + timer = setTimeout(() => resolve(false), SESSION_ROW_WAIT_MS); + timer.unref?.(); + }); + try { + return await Promise.race([pending, budget]); + } finally { + if (timer) clearTimeout(timer); + } +} + /** * Track a query and record metrics. * @@ -146,9 +188,17 @@ export async function recordMetrics(metrics: QueryMetrics): Promise { // Auto-detect agent if not provided by caller const agent = metrics.agent || getAgentIdentity() || null; + // GIT-73: never reference a session row that is not there yet. + let sessionId = metrics.session_id || null; + let metadata = metrics.metadata || {}; + if (sessionId && (await sessionRowLanded(sessionId)) === false) { + metadata = { ...metadata, session_id: sessionId }; + sessionId = null; + } + const record: Record = { id: metrics.id, - session_id: metrics.session_id || null, + session_id: sessionId, agent, tool_name: metrics.tool_name, query_text: metrics.query_text || null, @@ -160,7 +210,7 @@ export async function recordMetrics(metrics: QueryMetrics): Promise { phase_tag: metrics.phase_tag || null, linear_issue: metrics.linear_issue || null, memories_surfaced: metrics.memories_surfaced || null, - metadata: metrics.metadata || {}, + metadata, created_at: new Date().toISOString(), }; diff --git a/src/tools/session-start.ts b/src/tools/session-start.ts index 28ffedd..0e79189 100644 --- a/src/tools/session-start.ts +++ b/src/tools/session-start.ts @@ -24,6 +24,7 @@ import { getStorage } from "../services/storage.js"; import { Timer, recordMetrics, + registerPendingSessionRow, calculateContextBytes, PERFORMANCE_TARGETS, buildPerformanceData, @@ -426,14 +427,14 @@ async function createSessionRecord( project: Project, linearIssue?: string, preGeneratedId?: string // Accept pre-generated UUID for fire-and-forget pattern -): Promise<{ session_id: string; latency_ms: number; network_call: boolean }> { +): Promise<{ session_id: string; latency_ms: number; network_call: boolean; stored: boolean }> { const sessionId = preGeneratedId || uuidv4(); const today = new Date().toISOString().split("T")[0]; const timer = new Timer(); if (!hasSupabase()) { // Free tier: session tracked locally only - return { session_id: sessionId, latency_ms: timer.stop(), network_call: false }; + return { session_id: sessionId, latency_ms: timer.stop(), network_call: false, stored: false }; } try { @@ -458,6 +459,7 @@ async function createSessionRecord( session_id: sessionId, latency_ms: timer.stop(), network_call: true, // Always writes to Supabase + stored: true, }; } catch (error) { console.error("[session_start] Failed to create session record:", error); @@ -466,6 +468,7 @@ async function createSessionRecord( session_id: sessionId, latency_ms: timer.stop(), network_call: true, // Network was attempted + stored: false, }; } } @@ -1117,8 +1120,12 @@ export async function sessionStart( console.error(`[session_start] Resuming session ${sessionId} — skipping record creation`); } else { sessionId = uuidv4(); - // Fire-and-forget: don't await the Supabase write - createSessionRecord(agent, project, params.linear_issue, sessionId).catch(() => {}); + // Fire-and-forget: don't await the Supabase write. GIT-73: metrics writes + // for this session wait on it instead of racing it to a 409. + registerPendingSessionRow( + sessionId, + createSessionRecord(agent, project, params.linear_issue, sessionId).then((r) => r.stored) + ); // Mark prior in-memory session as superseded (force=true path) // Registry displacement in writeSessionFiles handles the registry case, // but priorSession may not be in the registry (e.g., after MCP restart) diff --git a/tests/unit/services/metrics-session-fk.test.ts b/tests/unit/services/metrics-session-fk.test.ts new file mode 100644 index 0000000..3ff787a --- /dev/null +++ b/tests/unit/services/metrics-session-fk.test.ts @@ -0,0 +1,74 @@ +/** + * GIT-73: a metrics row never references a session row that is not there yet. + * + * session_start created its session row fire-and-forget and wrote its metrics + * row at the same moment; gitmem_query_metrics.session_id is an FK to + * gitmem_sessions(id), so whichever arrived first decided whether the metrics + * insert 409'd. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const writes = vi.hoisted(() => ({ log: [] as Array<{ table: string; row: Record; at: number }> })); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + directUpsert: vi.fn(async (table: string, row: Record) => { + writes.log.push({ table, row, at: Date.now() }); + return row; + }), +})); +vi.mock("../../../src/services/tier.js", async (orig) => ({ + ...(await orig()), + hasSupabase: () => true, +})); + +import { recordMetrics, registerPendingSessionRow, clearPendingSessionRows } from "../../../src/services/metrics.js"; + +const SID = "393adb34-a80c-4c3a-b71a-bc0053b7a7ea"; +const metric = (extra: Record = {}) => ({ + id: "m-" + Math.random().toString(16).slice(2), session_id: SID, tool_name: "session_start", + latency_ms: 1, result_count: 0, metadata: { project: "p" }, ...extra, +}) as never; + +beforeEach(() => { writes.log = []; clearPendingSessionRows(); }); + +describe("metrics wait for their session row (GIT-73)", () => { + it("waits until the session row has landed, then keeps the FK", async () => { + let land!: (v: boolean) => void; + registerPendingSessionRow(SID, new Promise((r) => (land = r))); + + const pending = recordMetrics(metric()); + await new Promise((r) => setTimeout(r, 30)); + expect(writes.log).toHaveLength(0); // held back while the session row is in flight + + land(true); + await pending; + expect(writes.log).toHaveLength(1); + expect(writes.log[0].row.session_id).toBe(SID); + }); + + it("a session row that did not land: no FK, id kept in metadata.session_id", async () => { + registerPendingSessionRow(SID, Promise.resolve(false)); + await recordMetrics(metric()); + expect(writes.log[0].row.session_id).toBeNull(); + expect(writes.log[0].row.metadata).toEqual({ project: "p", session_id: SID }); + }); + + it("a session row whose creation threw is treated as not landed", async () => { + registerPendingSessionRow(SID, Promise.reject(new Error("store down"))); + await recordMetrics(metric()); + expect(writes.log[0].row.session_id).toBeNull(); + }); + + it("later writes for the same session reuse the settled answer", async () => { + registerPendingSessionRow(SID, Promise.resolve(true)); + await recordMetrics(metric()); + await recordMetrics(metric({ tool_name: "recall" })); + expect(writes.log.map((w) => w.row.session_id)).toEqual([SID, SID]); + }); + + it("a session this process did not create (resumed, or another process's) is written immediately", async () => { + await recordMetrics(metric()); + expect(writes.log).toHaveLength(1); + expect(writes.log[0].row.session_id).toBe(SID); + }); +}); From 203a2e2d9b9871c902ae52aaea5e3480e5b28fbe Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:41:57 -0700 Subject: [PATCH 13/24] test: remove GIT-73 from the venue driver's expected failures A 409 from gitmem_query_metrics now fails the run. 10 flow runs on the v1.8.0 schema with the fix: 0 409s. Co-Authored-By: Claude Opus 5 --- tests/e2e/blank-supabase.mjs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..00a8d41 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -48,7 +48,7 @@ const DENY_REFS = new Set(["cjptxyezuxdiinufgrrm"]); // production GitMem — ne * Known venue failures. Any other 4xx/5xx from /rest/v1/ or /functions/v1/ * fails the run. Each entry names the ticket that removes it — delete the entry * when that ticket merges, so this list only shrinks. An entry allows a failure; - * it does not require one (e.g. the GIT-73 race is intermittent). + * it does not require one (a failure may be intermittent). */ /** * Not defects: the store capability probe (src/services/store-columns.ts) is a @@ -72,7 +72,6 @@ function isCapabilityProbe(r) { } const EXPECTED_FAILURES = [ - { ticket: "GIT-73", what: "metrics/session_start FK race", method: "POST", path: /^\/rest\/v1\/gitmem_query_metrics$/, status: 409 }, { ticket: "GIT-105", what: "knowledge-triple thread id into a uuid column", method: "POST", path: /^\/rest\/v1\/knowledge_triples$/, status: 400 }, ]; const HERE = dirname(fileURLToPath(import.meta.url)); From e3c77fe7e94ac1919085081d0834a01301c5893f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:47:00 -0700 Subject: [PATCH 14/24] fix: migrate-root merges memory files by record id instead of skipping them (GIT-100) learnings.json, threads.json, decisions.json and sessions.json were skipped as "already exists", so a destination holding any local memory received none of the project store's, and the report listed the user's scars under "skipped". - Those four are merged by record id: new ids are added; where both sides hold a record the newer wins (updated_at, else resolved_at, last_touched_at, created_at), the destination on a tie or when neither is dated. - Every loser is written to /migrate-root-conflicts.json with which side was kept, deduplicated so a re-run logs nothing new. - The destination's memory files are backed up to /backups/ before the first write; nothing is backed up when nothing changes. - Idempotent: a re-run changes nothing. - A memory file that is not a readable list of records is reported as NOT merged (exit 1) and left untouched; never listed as skipped. - Other files keep copy-if-absent; backups/ and the conflicts log are never migrated out of a source. migrate-root.test.ts: "never overwrites a file that already exists" and "reports why each file was skipped" used learnings.json and asserted it was skipped, i.e. the old behaviour. They now use config.json to keep testing the non-memory rule. +11 tests. Co-Authored-By: Claude Opus 5 --- src/commands/migrate-root.ts | 266 +++++++++++++++++++---- tests/unit/commands/migrate-root.test.ts | 168 +++++++++++++- 2 files changed, 387 insertions(+), 47 deletions(-) diff --git a/src/commands/migrate-root.ts b/src/commands/migrate-root.ts index faa4241..48bc4c2 100644 --- a/src/commands/migrate-root.ts +++ b/src/commands/migrate-root.ts @@ -18,10 +18,21 @@ * is wrong about anything, the user still has their data where it was. Moving * would make a bad merge unrecoverable. * - * NEVER OVERWRITE. A file that already exists at the destination wins. The - * destination is the live store; the source is, by definition, the one that - * has not been read recently. Clobbering current memory with stale memory is - * worse than skipping. + * NEVER OVERWRITE a non-memory file. One that already exists at the + * destination wins; the destination is the live store. + * + * MERGE THE MEMORY FILES (GIT-100). learnings.json, threads.json, + * decisions.json and sessions.json used to be skipped as "already exists" — + * so a store with ANY local memory at the destination migrated none of the + * project store's memory, and the report listed the user's scars under + * "skipped". They are now merged by record id: new ids are added; where both + * sides hold a record, the newer (updated_at, else resolved_at / + * last_touched_at / created_at) wins, the destination on a tie or when + * neither is dated; every loser is written to migrate-root-conflicts.json. + * The destination's memory files are backed up before the first write. + * Re-running is idempotent: nothing changes, and a conflict is logged once. + * A memory file that cannot be read as a list of records is reported as NOT + * merged — never as skipped. * * REPORT EVERY SKIP. A silent partial migration would leave the user believing * they had merged when they had not — the failure class GIT-93 was about. @@ -31,11 +42,117 @@ import * as fs from "fs"; import * as path from "path"; import { findStrandedProjectRoots, getHomeGitmemDir } from "../services/gitmem-dir.js"; +/** Memory files merged by record id instead of skipped (GIT-100). */ +export const MERGED_FILES = ["learnings.json", "threads.json", "decisions.json", "sessions.json"] as const; +const CONFLICTS_FILE = "migrate-root-conflicts.json"; + +export interface MergeReport { + file: string; + added: number; + updated: number; + unchanged: number; + /** Both sides held the record and the destination's was kept. */ + kept_destination: number; + /** Conflicts written to the conflicts file by this run (already-logged ones are not repeated). */ + conflicts_logged: number; +} + interface MigrationPlan { source: string; destination: string; copied: string[]; skipped: Array<{ file: string; reason: string }>; + merged: MergeReport[]; + /** Memory files that could not be merged. Never silently skipped. */ + failed: Array<{ file: string; reason: string }>; + /** Where the destination's memory files were backed up, when anything was written. */ + backup?: string; + conflicts_file?: string; +} + +type MemoryRecord = Record & { id: string }; + +interface ConflictEntry { + file: string; + id: string; + kept: "destination" | "source"; + kept_timestamp: string | null; + lost_timestamp: string | null; + lost_record: unknown; + logged_at: string; +} + +const RECENCY_FIELDS = ["updated_at", "resolved_at", "last_touched_at", "created_at"] as const; + +function recency(r: MemoryRecord): { at: number | null; raw: string | null } { + for (const f of RECENCY_FIELDS) { + const v = r[f]; + if (typeof v === "string") { + const at = Date.parse(v); + if (Number.isFinite(at)) return { at, raw: v }; + } + } + return { at: null, raw: null }; +} + +/** Key-order-independent JSON, so re-serialized records compare equal. */ +function stable(v: unknown): string { + if (Array.isArray(v)) return `[${v.map(stable).join(",")}]`; + if (v && typeof v === "object") { + return `{${Object.keys(v as object).sort().map((k) => `${JSON.stringify(k)}:${stable((v as Record)[k])}`).join(",")}}`; + } + return JSON.stringify(v); +} + +function readRecords(file: string): { ok: true; records: MemoryRecord[] } | { ok: false; reason: string } { + let data: unknown; + try { + data = JSON.parse(fs.readFileSync(file, "utf-8")); + } catch (e) { + return { ok: false, reason: `unreadable JSON (${e instanceof Error ? e.message : String(e)})` }; + } + if (!Array.isArray(data)) return { ok: false, reason: "not a list of records" }; + const bad = data.findIndex((r) => !r || typeof r !== "object" || typeof (r as { id?: unknown }).id !== "string"); + if (bad !== -1) return { ok: false, reason: `record ${bad} has no string id` }; + return { ok: true, records: data as MemoryRecord[] }; +} + +interface PlannedMerge { + report: MergeReport; + merged: MemoryRecord[]; + conflicts: Omit[]; + changed: boolean; +} + +/** Merge source into destination by id. Pure: returns the result, writes nothing. */ +function planMerge(file: string, dest: MemoryRecord[], src: MemoryRecord[]): PlannedMerge { + const report: MergeReport = { file, added: 0, updated: 0, unchanged: 0, kept_destination: 0, conflicts_logged: 0 }; + const conflicts: Omit[] = []; + const merged = dest.slice(); + const index = new Map(merged.map((r, i) => [r.id, i])); + + for (const s of src) { + const i = index.get(s.id); + if (i === undefined) { + index.set(s.id, merged.length); + merged.push(s); + report.added++; + continue; + } + const d = merged[i]; + if (stable(d) === stable(s)) { report.unchanged++; continue; } + const rd = recency(d), rs = recency(s); + const sourceNewer = rs.at !== null && (rd.at === null || rs.at > rd.at); + if (sourceNewer) { + merged[i] = s; + report.updated++; + conflicts.push({ file, id: s.id, kept: "source", kept_timestamp: rs.raw, lost_timestamp: rd.raw, lost_record: d }); + } else { + report.kept_destination++; + conflicts.push({ file, id: s.id, kept: "destination", kept_timestamp: rd.raw, lost_timestamp: rs.raw, lost_record: s }); + } + } + return { report, merged, conflicts, changed: report.added > 0 || report.updated > 0 }; } /** @@ -47,19 +164,26 @@ export function findProjectRoots(): string[] { return findStrandedProjectRoots(); } +const isPerInstall = (relative: string, name: string) => + relative === "" && (name === "cache" || name === "license-cache.json" || name === "backups" || name === CONFLICTS_FILE); + +const isMergedFile = (relative: string, name: string) => + relative === "" && (MERGED_FILES as readonly string[]).includes(name); + /** - * Recursively copy `from` into `to`, never overwriting an existing file. - * - * Returns what was copied and what was left alone, so the caller can report both - * rather than claiming a clean merge. + * Walk `from`, planning (and unless dryRun, performing) the copy of every + * non-memory file that does not exist at `to`. Memory files present on both + * sides are collected for merging instead. */ -function copyTree( +function walk( from: string, to: string, plan: MigrationPlan, + dryRun: boolean, + toMerge: string[], relative = "" ): void { - fs.mkdirSync(to, { recursive: true }); + if (!dryRun) fs.mkdirSync(to, { recursive: true }); for (const entry of fs.readdirSync(from, { withFileTypes: true })) { const rel = relative ? path.join(relative, entry.name) : entry.name; const src = path.join(from, entry.name); @@ -67,50 +191,101 @@ function copyTree( // Caches and license state are per-install, not memory. Copying them would // move a license binding between roots, which is not this command's job. - if (relative === "" && (entry.name === "cache" || entry.name === "license-cache.json")) { + // Backups and the conflicts log belong to the root they were written in. + if (isPerInstall(relative, entry.name)) { plan.skipped.push({ file: rel, reason: "per-install state, not memory" }); continue; } if (entry.isDirectory()) { - copyTree(src, dst, plan, rel); + walk(src, dst, plan, dryRun, toMerge, rel); continue; } if (fs.existsSync(dst)) { - plan.skipped.push({ file: rel, reason: "already exists in destination" }); + if (isMergedFile(relative, entry.name)) { + toMerge.push(entry.name); + } else { + plan.skipped.push({ file: rel, reason: "already exists in destination" }); + } continue; } - fs.copyFileSync(src, dst); + if (!dryRun) fs.copyFileSync(src, dst); plan.copied.push(rel); } } +function loadConflicts(file: string): ConflictEntry[] { + try { + const data = JSON.parse(fs.readFileSync(file, "utf-8")); + return Array.isArray(data) ? data : []; + } catch { + return []; + } +} + +const conflictKey = (c: Omit) => `${c.file}\n${c.id}\n${stable(c.lost_record)}`; + export function migrateRoot(source: string, destination: string, dryRun: boolean): MigrationPlan { - const plan: MigrationPlan = { source, destination, copied: [], skipped: [] }; - - if (dryRun) { - // Walk the same tree without writing, so --dry-run reports the real plan - // rather than a guess at one. - const probe = (from: string, to: string, rel = ""): void => { - for (const entry of fs.readdirSync(from, { withFileTypes: true })) { - const r = rel ? path.join(rel, entry.name) : entry.name; - if (rel === "" && (entry.name === "cache" || entry.name === "license-cache.json")) { - plan.skipped.push({ file: r, reason: "per-install state, not memory" }); - continue; - } - if (entry.isDirectory()) { probe(path.join(from, entry.name), path.join(to, entry.name), r); continue; } - if (fs.existsSync(path.join(to, entry.name))) { - plan.skipped.push({ file: r, reason: "already exists in destination" }); - continue; - } - plan.copied.push(r); - } - }; - probe(source, destination); - return plan; + const plan: MigrationPlan = { source, destination, copied: [], skipped: [], merged: [], failed: [] }; + const toMerge: string[] = []; + + // Walk the same tree either way, so --dry-run reports the real plan rather + // than a guess at one. + walk(source, destination, plan, dryRun, toMerge); + + // Memory files on both sides: merge by record id (GIT-100). + const planned: Array = []; + for (const name of toMerge) { + const d = readRecords(path.join(destination, name)); + const s = readRecords(path.join(source, name)); + if (!d.ok || !s.ok) { + const reason = !d.ok ? `destination ${d.reason}` : `source ${(s as { reason: string }).reason}`; + plan.failed.push({ file: name, reason: `NOT merged: ${reason}` }); + continue; + } + planned.push({ name, ...planMerge(name, d.records, s.records) }); + } + + const conflictsPath = path.join(destination, CONFLICTS_FILE); + const logged = loadConflicts(conflictsPath); + const seen = new Set(logged.map(conflictKey)); + const newConflicts: ConflictEntry[] = []; + const now = new Date().toISOString(); + for (const p of planned) { + for (const c of p.conflicts) { + const k = conflictKey(c); + if (seen.has(k)) continue; + seen.add(k); + newConflicts.push({ ...c, logged_at: now }); + p.report.conflicts_logged++; + } + plan.merged.push(p.report); } + if (newConflicts.length > 0 || logged.length > 0) plan.conflicts_file = conflictsPath; - copyTree(source, destination, plan); + const writes = planned.filter((p) => p.changed); + if (dryRun || (writes.length === 0 && newConflicts.length === 0)) return plan; + + // Back up the destination's memory files before the first write. + if (writes.length > 0) { + const backup = path.join(destination, "backups", `migrate-root-${now.replace(/[:.]/g, "-")}`); + fs.mkdirSync(backup, { recursive: true }); + for (const name of MERGED_FILES) { + const f = path.join(destination, name); + if (fs.existsSync(f)) fs.copyFileSync(f, path.join(backup, name)); + } + plan.backup = backup; + } + + for (const p of writes) { + const target = path.join(destination, p.name); + const tmpFile = `${target}.migrate-${process.pid}.tmp`; + fs.writeFileSync(tmpFile, JSON.stringify(p.merged, null, 2)); + fs.renameSync(tmpFile, target); + } + if (newConflicts.length > 0) { + fs.writeFileSync(conflictsPath, JSON.stringify([...logged, ...newConflicts], null, 2)); + } return plan; } @@ -156,6 +331,23 @@ export function main(args: string[]): void { const plan = migrateRoot(source, home, dryRun); console.log(`${plan.copied.length} file(s) ${dryRun ? "would be " : ""}copied.`); + if (plan.merged.length > 0) { + console.log(`${plan.merged.length} memory file(s) ${dryRun ? "would be " : ""}merged by record id:`); + for (const m of plan.merged) { + console.log( + ` ${m.file} — ${m.added} added, ${m.updated} updated from the source, ${m.unchanged} unchanged, ` + + `${m.kept_destination} kept from the destination` + + (m.conflicts_logged > 0 ? `, ${m.conflicts_logged} conflict(s) ${dryRun ? "would be " : ""}logged` : "") + ); + } + } + if (plan.failed.length > 0) { + console.log(`${plan.failed.length} memory file(s) NOT merged — fix these and re-run:`); + for (const f of plan.failed) console.log(` ${f.file} — ${f.reason}`); + process.exitCode = 1; + } + if (plan.backup) console.log(`Destination memory files backed up to ${plan.backup}`); + if (plan.conflicts_file) console.log(`Conflicting records (the version not kept) are in ${plan.conflicts_file}`); if (plan.skipped.length > 0) { console.log(`${plan.skipped.length} skipped:`); for (const s of plan.skipped) console.log(` ${s.file} — ${s.reason}`); diff --git a/tests/unit/commands/migrate-root.test.ts b/tests/unit/commands/migrate-root.test.ts index e26adf7..01f6ade 100644 --- a/tests/unit/commands/migrate-root.test.ts +++ b/tests/unit/commands/migrate-root.test.ts @@ -10,7 +10,10 @@ * merely fail: * * copies, never moves — a wrong call leaves the original intact - * never overwrites — stale memory cannot clobber current memory + * never overwrites — a non-memory file at the destination wins + * merges memory by id — GIT-100: learnings/threads/decisions/sessions + * .json were skipped as "already exists", so a + * destination with any memory received none * reports every skip — a partial merge is never reported as complete * writes where gitmem reads — this one was a real bug: main() computed the * destination with os.homedir() instead of the @@ -67,24 +70,27 @@ describe("GIT-91: migrate-root", () => { expect(read(path.join(source, "learnings.json"))).toBe(before); }); - it("never overwrites a file that already exists in the destination", () => { - // The destination is the live store; the source is by definition the one - // that has not been read recently. Stale must not win. - fs.writeFileSync(path.join(destination, "learnings.json"), '{"learnings":["CURRENT"]}'); + it("never overwrites a non-memory file that already exists in the destination", () => { + // GIT-100: this used learnings.json and asserted it was skipped, i.e. the + // project store's memory was dropped whenever the destination had any. + // Memory files are merged now (below); other files keep this rule. + fs.writeFileSync(path.join(source, "config.json"), '{"project":"old"}'); + fs.writeFileSync(path.join(destination, "config.json"), '{"project":"CURRENT"}'); const plan = migrateRoot(source, destination, false); - expect(read(path.join(destination, "learnings.json"))).toBe('{"learnings":["CURRENT"]}'); - expect(plan.copied).not.toContain("learnings.json"); - expect(plan.skipped.map((s) => s.file)).toContain("learnings.json"); + expect(read(path.join(destination, "config.json"))).toBe('{"project":"CURRENT"}'); + expect(plan.copied).not.toContain("config.json"); + expect(plan.skipped.map((s) => s.file)).toContain("config.json"); }); it("reports why each file was skipped", () => { - fs.writeFileSync(path.join(destination, "learnings.json"), "{}"); + fs.writeFileSync(path.join(source, "config.json"), "{}"); + fs.writeFileSync(path.join(destination, "config.json"), "{}"); const plan = migrateRoot(source, destination, false); - const skip = plan.skipped.find((s) => s.file === "learnings.json"); + const skip = plan.skipped.find((s) => s.file === "config.json"); // A silent partial merge would read as a complete one. expect(skip?.reason).toMatch(/already exists/i); }); @@ -111,3 +117,145 @@ describe("GIT-91: migrate-root", () => { expect(dry.copied.sort()).toEqual(applied.copied.sort()); }); }); + +// =========================================================================== +// GIT-100: memory files are merged by record id, not skipped +// =========================================================================== + +const ts = (h: number) => `2026-09-${String(h).padStart(2, "0")}T00:00:00.000Z`; +const writeJson = (p: string, v: unknown) => fs.writeFileSync(p, JSON.stringify(v, null, 2)); +const readJson = (p: string) => JSON.parse(read(p)); +const ids = (p: string) => readJson(p).map((r: { id: string }) => r.id); + +describe("GIT-100: migrate-root merges memory files by record id", () => { + beforeEach(() => { + for (const f of ["learnings.json", "threads.json", "decisions.json", "sessions.json"]) { + fs.rmSync(path.join(source, f), { force: true }); + } + writeJson(path.join(source, "learnings.json"), [ + { id: "L-src-only", title: "only in project store", updated_at: ts(1) }, + { id: "L-both-src-newer", title: "project edit", updated_at: ts(10) }, + { id: "L-both-dst-newer", title: "stale project copy", updated_at: ts(2) }, + { id: "L-same", title: "identical", updated_at: ts(3) }, + ]); + writeJson(path.join(destination, "learnings.json"), [ + { id: "L-dst-only", title: "only in home store", updated_at: ts(4) }, + { id: "L-both-src-newer", title: "old home copy", updated_at: ts(5) }, + { id: "L-both-dst-newer", title: "home edit", updated_at: ts(9) }, + { updated_at: ts(3), title: "identical", id: "L-same" }, // same record, different key order + ]); + writeJson(path.join(source, "threads.json"), [{ id: "t-src", text: "project thread", created_at: ts(1) }]); + writeJson(path.join(destination, "threads.json"), [{ id: "t-dst", text: "home thread", created_at: ts(2) }]); + writeJson(path.join(source, "decisions.json"), [{ id: "D1", title: "undated A" }]); + writeJson(path.join(destination, "decisions.json"), [{ id: "D1", title: "undated B" }]); + writeJson(path.join(source, "sessions.json"), [{ id: "S-src", session_date: "2026-09-01" }]); + }); + + it("never lists a memory file under skipped", () => { + const plan = migrateRoot(source, destination, false); + const skipped = plan.skipped.map((s) => s.file); + for (const f of ["learnings.json", "threads.json", "decisions.json", "sessions.json"]) expect(skipped).not.toContain(f); + }); + + it("adds new ids, keeps destination-only ids, and the newer record wins", () => { + const plan = migrateRoot(source, destination, false); + const merged = readJson(path.join(destination, "learnings.json")); + const byId = Object.fromEntries(merged.map((r: { id: string; title: string }) => [r.id, r.title])); + + expect(byId).toEqual({ + "L-dst-only": "only in home store", + "L-both-src-newer": "project edit", // source newer: replaced + "L-both-dst-newer": "home edit", // destination newer: kept + "L-same": "identical", + "L-src-only": "only in project store", + }); + expect(plan.merged.find((m) => m.file === "learnings.json")).toMatchObject({ + added: 1, updated: 1, unchanged: 1, kept_destination: 1, conflicts_logged: 2, + }); + expect(ids(path.join(destination, "threads.json")).sort()).toEqual(["t-dst", "t-src"]); + }); + + it("a memory file only in the source is copied", () => { + const plan = migrateRoot(source, destination, false); + expect(plan.copied).toContain("sessions.json"); + expect(ids(path.join(destination, "sessions.json"))).toEqual(["S-src"]); + }); + + it("undated records that differ: the destination is kept and the loser logged", () => { + migrateRoot(source, destination, false); + expect(readJson(path.join(destination, "decisions.json"))).toEqual([{ id: "D1", title: "undated B" }]); + const conflicts = readJson(path.join(destination, "migrate-root-conflicts.json")); + expect(conflicts.find((c: { id: string }) => c.id === "D1")).toMatchObject({ kept: "destination", lost_record: { id: "D1", title: "undated A" } }); + }); + + it("logs every loser, with which side was kept", () => { + migrateRoot(source, destination, false); + const conflicts = readJson(path.join(destination, "migrate-root-conflicts.json")); + const learningConflicts = conflicts.filter((c: { file: string }) => c.file === "learnings.json"); + expect(learningConflicts).toHaveLength(2); + expect(learningConflicts.find((c: { id: string }) => c.id === "L-both-src-newer")).toMatchObject({ + kept: "source", lost_record: { title: "old home copy" }, kept_timestamp: ts(10), lost_timestamp: ts(5), + }); + expect(learningConflicts.find((c: { id: string }) => c.id === "L-both-dst-newer")).toMatchObject({ + kept: "destination", lost_record: { title: "stale project copy" }, + }); + }); + + it("backs up the destination's memory files before writing", () => { + const before = read(path.join(destination, "learnings.json")); + const plan = migrateRoot(source, destination, false); + expect(plan.backup).toBeDefined(); + expect(read(path.join(plan.backup!, "learnings.json"))).toBe(before); + expect(fs.existsSync(path.join(plan.backup!, "threads.json"))).toBe(true); + }); + + it("is idempotent: a re-run changes nothing, backs up nothing, logs nothing new", () => { + migrateRoot(source, destination, false); + const snapshot = ["learnings.json", "threads.json", "decisions.json", "sessions.json", "migrate-root-conflicts.json"] + .map((f) => read(path.join(destination, f))); + + const again = migrateRoot(source, destination, false); + + expect(["learnings.json", "threads.json", "decisions.json", "sessions.json", "migrate-root-conflicts.json"] + .map((f) => read(path.join(destination, f)))).toEqual(snapshot); + expect(again.backup).toBeUndefined(); + for (const m of again.merged) { + expect(m.added).toBe(0); + expect(m.updated).toBe(0); + expect(m.conflicts_logged).toBe(0); + } + }); + + it("leaves the source intact", () => { + const before = read(path.join(source, "learnings.json")); + migrateRoot(source, destination, false); + expect(read(path.join(source, "learnings.json"))).toBe(before); + }); + + it("dry-run writes nothing but reports the same merge", () => { + const before = read(path.join(destination, "learnings.json")); + const dry = migrateRoot(source, destination, true); + expect(read(path.join(destination, "learnings.json"))).toBe(before); + expect(fs.existsSync(path.join(destination, "migrate-root-conflicts.json"))).toBe(false); + expect(fs.existsSync(path.join(destination, "backups"))).toBe(false); + + const applied = migrateRoot(source, destination, false); + expect(dry.merged).toEqual(applied.merged); + }); + + it("an unreadable memory file is reported as NOT merged, never skipped, and left untouched", () => { + fs.writeFileSync(path.join(destination, "threads.json"), "{ not json"); + const plan = migrateRoot(source, destination, false); + expect(plan.failed).toEqual([{ file: "threads.json", reason: expect.stringMatching(/^NOT merged: destination unreadable JSON/) }]); + expect(plan.skipped.map((s) => s.file)).not.toContain("threads.json"); + expect(read(path.join(destination, "threads.json"))).toBe("{ not json"); + }); + + it("does not migrate backups or the conflicts log out of a source that has them", () => { + fs.mkdirSync(path.join(source, "backups", "old"), { recursive: true }); + fs.writeFileSync(path.join(source, "migrate-root-conflicts.json"), "[]"); + const plan = migrateRoot(source, destination, false); + expect(plan.copied.some((f) => f.startsWith("backups"))).toBe(false); + expect(plan.copied).not.toContain("migrate-root-conflicts.json"); + }); +}); From 71ba596c0733f3f1af94aef3c1485755b287386a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:48:30 -0700 Subject: [PATCH 15/24] fix: thread triples reference the thread's row id, not its "t-" id (GIT-105) knowledge_triples.source_id is a UUID column on every schema, including the v1.8.0 setup.sql. Thread triples wrote the thread id ("t-xxxxxxxx") into it, so every create_thread / resolve_thread triple failed with 400 (22P02) and the failure was swallowed. No schema change needed: - source_id is the thread's gitmem_threads.id, a UUID the store already has. create_thread passes the row id createThreadInSupabase returned; resolution triples look it up by thread_id. A failed or empty lookup writes the triple with source_id null rather than losing it. - writeTriples never sends a non-UUID source_id from any caller; it logs and sends null instead. thread-triples.test.ts: two assertions expected source_id "t-abc12345", the value the column rejects. They now expect the row id. +5 tests. Co-Authored-By: Claude Opus 5 --- src/services/triple-writer.ts | 57 +++++++++++++--- src/tools/create-thread.ts | 3 + .../services/thread-triples-write.test.ts | 68 +++++++++++++++++++ tests/unit/services/thread-triples.test.ts | 21 +++++- 4 files changed, 139 insertions(+), 10 deletions(-) create mode 100644 tests/unit/services/thread-triples-write.test.ts diff --git a/src/services/triple-writer.ts b/src/services/triple-writer.ts index 81d8410..009560c 100644 --- a/src/services/triple-writer.ts +++ b/src/services/triple-writer.ts @@ -12,7 +12,7 @@ import { v4 as uuidv4 } from "uuid"; import * as supabase from "./supabase-client.js"; -import { hasSupabase } from "./tier.js"; +import { hasSupabase, getTableName } from "./tier.js"; // --- Types --- @@ -32,7 +32,8 @@ interface TripleCandidate { predicate: Predicate; object: string; source_type: string; - source_id: string; + /** knowledge_triples.source_id is a UUID column: a row id, or null. */ + source_id: string | null; source_linear_issue?: string; domain?: string[]; project: string; @@ -282,11 +283,24 @@ export function extractDecisionTriples(params: DecisionTripleParams): TripleCand * failure (GIT-104) — callers run this under getEffectTracker().track(), which * never rethrows. */ +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + export async function writeTriples(candidates: TripleCandidate[]): Promise { if (candidates.length === 0 || !hasSupabase()) { return 0; } + // GIT-105: source_id is a UUID column in every schema. A non-UUID value + // (thread ids are "t-xxxxxxxx") failed the whole row with 22P02, so every + // thread triple 400'd and was lost. Never send one; the subject/object + // labels still carry the relation. + for (const c of candidates) { + if (c.source_id !== null && !UUID_RE.test(c.source_id)) { + console.error(`[triple-writer] source_id "${c.source_id}" is not a UUID — writing the triple with source_id null`); + c.source_id = null; + } + } + let written = 0; const errors: string[] = []; @@ -356,6 +370,12 @@ export function writeTriplesForDecision(params: DecisionTripleParams): Promise { - const triples = extractThreadCreationTriples(params); +export async function writeTriplesForThreadCreation(params: ThreadCreationTripleParams): Promise { + const triples = extractThreadCreationTriples(await withThreadRowId(params)); return writeTriples(triples); } +/** + * GIT-105: fill thread_row_id from the store when the caller did not have it. + * A failed or empty lookup leaves it null — the triple is still written. + */ +async function withThreadRowId(params: T): Promise { + if (params.thread_row_id !== undefined || !hasSupabase()) return params; + try { + const rows = await supabase.directQuery<{ id: string }>(getTableName("threads"), { + select: "id", + filters: { thread_id: params.thread_id }, + limit: 1, + }); + return { ...params, thread_row_id: rows[0]?.id ?? null }; + } catch (error) { + console.error(`[triple-writer] Could not look up the row id of ${params.thread_id}:`, error instanceof Error ? error.message : error); + return { ...params, thread_row_id: null }; + } +} + /** * Generate and write triples for a resolved thread. * Fire-and-forget — run under getEffectTracker().track(); rejects if any triple failed. */ -export function writeTriplesForThreadResolution(params: ThreadResolutionTripleParams): Promise { - const triples = extractThreadResolutionTriples(params); +export async function writeTriplesForThreadResolution(params: ThreadResolutionTripleParams): Promise { + const triples = extractThreadResolutionTriples(await withThreadRowId(params)); return writeTriples(triples); } diff --git a/src/tools/create-thread.ts b/src/tools/create-thread.ts index 691de05..7a74b64 100644 --- a/src/tools/create-thread.ts +++ b/src/tools/create-thread.ts @@ -296,12 +296,14 @@ export async function createThread( // Write to Supabase (source of truth) with embedding. let supabaseSynced = false; let supabaseError: string | undefined; + let threadRowId: string | null = null; // GIT-105: gitmem_threads.id, for triple source_id const embeddingJson = newEmbedding ? JSON.stringify(newEmbedding) : null; if (hasSupabase()) { try { const supabaseResult = await createThreadInSupabase(thread, project, embeddingJson); supabaseSynced = Boolean(supabaseResult); + threadRowId = supabaseResult?.id ?? null; if (!supabaseSynced) { supabaseError = "write returned no row"; } @@ -347,6 +349,7 @@ export async function createThread( getEffectTracker().track("triple_write", "thread_creation", () => writeTriplesForThreadCreation({ thread_id: thread.id, + thread_row_id: threadRowId, text: trimmedText, linear_issue: params.linear_issue, session_id: sessionId, diff --git a/tests/unit/services/thread-triples-write.test.ts b/tests/unit/services/thread-triples-write.test.ts new file mode 100644 index 0000000..cb7f84a --- /dev/null +++ b/tests/unit/services/thread-triples-write.test.ts @@ -0,0 +1,68 @@ +/** + * GIT-105: thread triples reach knowledge_triples on a v1.8.0 store. + * + * source_id is a UUID column; thread ids are "t-xxxxxxxx". Every + * create_thread / resolve_thread triple 400'd (22P02) and the failure was + * swallowed. The writer now stores the thread's row id (gitmem_threads.id), + * looking it up when the caller does not have it, and never sends a non-UUID. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const store = vi.hoisted(() => ({ + upserts: [] as Array>, + queries: [] as Array<{ table: string; filters: Record }>, + threadRows: {} as Record, + queryFails: false, +})); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + directUpsert: vi.fn(async (_t: string, row: Record) => { store.upserts.push(row); return row; }), + directQuery: vi.fn(async (table: string, opts: { filters: Record }) => { + store.queries.push({ table, filters: opts.filters }); + if (store.queryFails) throw new Error("store down"); + const id = store.threadRows[opts.filters.thread_id]; + return id ? [{ id }] : []; + }), +})); +vi.mock("../../../src/services/tier.js", async (orig) => ({ + ...(await orig()), + hasSupabase: () => true, +})); + +import { writeTriplesForThreadCreation, writeTriplesForThreadResolution, writeTriples } from "../../../src/services/triple-writer.js"; + +const ROW = "7d1c9a52-3f4e-4b8a-9c21-5e6f7a8b9c0d"; +const SESSION = "550e8400-e29b-41d4-a716-446655440000"; +const base = { thread_id: "t-abc12345", text: "Fix auth timeout", session_id: SESSION, project: "p", agent: "cli" }; + +beforeEach(() => { + store.upserts = []; store.queries = []; store.threadRows = {}; store.queryFails = false; +}); + +describe("thread triples on a UUID source_id column (GIT-105)", () => { + it("creation: uses the row id create_thread already has — no lookup", async () => { + await writeTriplesForThreadCreation({ ...base, thread_row_id: ROW }); + expect(store.queries).toHaveLength(0); + expect(store.upserts.map((r) => r.source_id)).toEqual([ROW]); + }); + + it("resolution: looks the row id up by thread_id", async () => { + store.threadRows["t-abc12345"] = ROW; + await writeTriplesForThreadResolution(base); + expect(store.queries).toEqual([{ table: "gitmem_threads", filters: { thread_id: "t-abc12345" } }]); + expect(store.upserts.map((r) => r.source_id)).toEqual([ROW]); + }); + + it("thread not in the store, or the lookup fails: source_id null, triple still written", async () => { + await writeTriplesForThreadResolution(base); + store.queryFails = true; + await writeTriplesForThreadResolution(base); + expect(store.upserts.map((r) => r.source_id)).toEqual([null, null]); + }); + + it("never sends a non-UUID source_id, whatever the caller passed", async () => { + await writeTriples([{ subject: "s", predicate: "created_thread", object: "o", source_type: "thread", + source_id: "t-abc12345", project: "p", half_life_days: 9999, created_by: "cli" }]); + expect(store.upserts[0].source_id).toBeNull(); + }); +}); diff --git a/tests/unit/services/thread-triples.test.ts b/tests/unit/services/thread-triples.test.ts index 8de0a03..fd2853c 100644 --- a/tests/unit/services/thread-triples.test.ts +++ b/tests/unit/services/thread-triples.test.ts @@ -17,8 +17,13 @@ import type { // ---------- Helpers ---------- +// GIT-105: source_id is the thread's row id (gitmem_threads.id, a UUID), not +// its "t-" thread_id — knowledge_triples.source_id is a UUID column. +const THREAD_ROW_ID = "7d1c9a52-3f4e-4b8a-9c21-5e6f7a8b9c0d"; + const BASE_CREATION_PARAMS: ThreadCreationTripleParams = { thread_id: "t-abc12345", + thread_row_id: THREAD_ROW_ID, text: "Fix auth timeout in production", session_id: "550e8400-e29b-41d4-a716-446655440000", linear_issue: "PROJ-123", @@ -28,6 +33,7 @@ const BASE_CREATION_PARAMS: ThreadCreationTripleParams = { const BASE_RESOLUTION_PARAMS: ThreadResolutionTripleParams = { thread_id: "t-abc12345", + thread_row_id: THREAD_ROW_ID, text: "Fix auth timeout in production", resolution_note: "Fixed by increasing timeout to 30s", session_id: "660e8400-e29b-41d4-a716-446655440000", @@ -88,7 +94,8 @@ describe("extractThreadCreationTriples", () => { for (const triple of triples) { expect(triple.source_type).toBe("thread"); - expect(triple.source_id).toBe("t-abc12345"); + // GIT-105: was "t-abc12345", which the UUID column rejects (22P02). + expect(triple.source_id).toBe(THREAD_ROW_ID); expect(triple.half_life_days).toBe(9999); expect(triple.project).toBe("test-project"); expect(triple.created_by).toBe("CLI"); @@ -129,7 +136,17 @@ describe("extractThreadResolutionTriples", () => { expect(triple.subject).toMatch(/^Session: /); expect(triple.object).toMatch(/^Thread: /); expect(triple.source_type).toBe("thread"); - expect(triple.source_id).toBe("t-abc12345"); + // GIT-105: was "t-abc12345", which the UUID column rejects (22P02). + expect(triple.source_id).toBe(THREAD_ROW_ID); + }); + + it("without a row id, source_id is null — never the thread_id (GIT-105)", () => { + for (const t of [ + ...extractThreadCreationTriples({ ...BASE_CREATION_PARAMS, thread_row_id: null }), + ...extractThreadResolutionTriples({ ...BASE_RESOLUTION_PARAMS, thread_row_id: undefined }), + ]) { + expect(t.source_id).toBeNull(); + } }); }); From b4abf5e72425dacae003723699125b1b4cd20447 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:48:30 -0700 Subject: [PATCH 16/24] test: venue flow requires linked thread triples; remove GIT-105 from expected failures (GIT-105) Exits 1 on 1.10.0 (0 thread triples stored); exits 0 with the fix. Co-Authored-By: Claude Opus 5 --- tests/e2e/blank-supabase.mjs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..3c52dc7 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -73,7 +73,6 @@ function isCapabilityProbe(r) { const EXPECTED_FAILURES = [ { ticket: "GIT-73", what: "metrics/session_start FK race", method: "POST", path: /^\/rest\/v1\/gitmem_query_metrics$/, status: 409 }, - { ticket: "GIT-105", what: "knowledge-triple thread id into a uuid column", method: "POST", path: /^\/rest\/v1\/knowledge_triples$/, status: 400 }, ]; const HERE = dirname(fileURLToPath(import.meta.url)); const REPO = resolve(HERE, "../.."); @@ -377,6 +376,11 @@ async function flow() { await waitQuiet(netlog, { minMs: 2000, quietMs: 3000 }); const health = await step("health", { failure_limit: 20 }); + // GIT-105: thread triples land, and their source_id is a real thread row id. + const threadTriples = await rest("GET", "knowledge_triples?select=predicate,source_id&source_type=eq.thread", undefined, { Prefer: "" }).then((r) => r.json()); + const threadRowIds = new Set((await rest("GET", "gitmem_threads?select=id", undefined, { Prefer: "" }).then((r) => r.json())).map((t) => t.id)); + const threadTriplesLinked = threadTriples.length > 0 && threadTriples.every((t) => threadRowIds.has(t.source_id)); + // Did the close land? Is relevance readable from the store (GIT-109)? const closedRows = sessionId ? await rest("GET", `gitmem_sessions?select=id,closing_reflection&id=eq.${sessionId}`, undefined, { Prefer: "" }).then((r) => r.json()) @@ -408,6 +412,8 @@ async function flow() { remote_sessions_after: await count("gitmem_sessions"), remote_scar_usage_after: await count("gitmem_scar_usage"), session_close_persisted: closedRows.length === 1 && closedRows[0].closing_reflection != null, + thread_triples: threadTriples.length, + thread_triples_linked: threadTriplesLinked, relevance_readable: relevance.some((r) => scarIds.some((id) => (r.memories_applied || []).includes(id) && r.memory_relevance?.[id])), relevance, health_failed_total: failed, @@ -520,6 +526,9 @@ const failureCheck = checkFailures(allVenueRequests); const edgeCalls = allVenueRequests.filter((r) => r.path.startsWith("/functions/v1/")) .map((r) => `${r.method} ${r.path} -> ${r.status}`); if (edgeCalls.length) failureCheck.unexpected.push(...edgeCalls.map((c) => `edge function called (GIT-97): ${c}`)); +if (mode === "flow" && !result.thread_triples_linked) { + failureCheck.unexpected.push(`thread triples missing or not linked to a gitmem_threads row (GIT-105): ${result.thread_triples} found`); +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } From 1b8f4a7eff0e6f2146eea9c579586761fa61ec64 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:52:07 -0700 Subject: [PATCH 17/24] fix: compare .gitmem roots by real path, not string (GIT-107) findStrandedProjectRoots compared candidate and home as strings. On macOS /var is a symlink to /private/var: process.cwd() reports /private/var/... and a home or GITMEM_DIR under $TMPDIR is spelled /var/..., so session_start showed "Memory store found but NOT being read" about the store it was reading. Any symlink in either path did the same. - canonicalPath() resolves with fs.realpathSync.native; a path that does not exist yet resolves through its nearest existing ancestor. - A candidate is not stranded if it is the same directory as the home root or an explicit GITMEM_DIR (the root actually being read). - migrate-root's "source and destination are the same" check uses it too. +5 tests (symlinked home, a different store still reported, GITMEM_DIR, sameDirectory spellings, and the macOS /var case); 4 fail on origin/main. Co-Authored-By: Claude Opus 5 --- src/commands/migrate-root.ts | 4 +- src/services/gitmem-dir.ts | 41 ++++++- .../unit/services/gitmem-root-symlink.test.ts | 103 ++++++++++++++++++ 3 files changed, 144 insertions(+), 4 deletions(-) create mode 100644 tests/unit/services/gitmem-root-symlink.test.ts diff --git a/src/commands/migrate-root.ts b/src/commands/migrate-root.ts index faa4241..919be0c 100644 --- a/src/commands/migrate-root.ts +++ b/src/commands/migrate-root.ts @@ -29,7 +29,7 @@ import * as fs from "fs"; import * as path from "path"; -import { findStrandedProjectRoots, getHomeGitmemDir } from "../services/gitmem-dir.js"; +import { findStrandedProjectRoots, getHomeGitmemDir, sameDirectory } from "../services/gitmem-dir.js"; interface MigrationPlan { source: string; @@ -144,7 +144,7 @@ export function main(args: string[]): void { } const source = sources[0]; - if (source === home) { + if (sameDirectory(source, home)) { // GIT-107: symlinks, /var vs /private/var console.log(`Source and destination are the same (${home}). Nothing to do.`); return; } diff --git a/src/services/gitmem-dir.ts b/src/services/gitmem-dir.ts index 4451b7e..feba098 100644 --- a/src/services/gitmem-dir.ts +++ b/src/services/gitmem-dir.ts @@ -179,15 +179,52 @@ export function describeGitmemRoot(root: string): GitmemRootContents { }; } +/** + * GIT-107: a path's canonical spelling, for comparing directories. + * + * Roots used to be compared as strings, but one directory has many spellings: + * on macOS /var is a symlink to /private/var, so a home under $TMPDIR + * (/var/folders/...) and a cwd reported as /private/var/folders/... named the + * same store and still read as different — and session_start announced + * "Memory store found but NOT being read" about the store it was reading. + * + * realpathSync.native resolves every symlink. A path that does not exist yet + * (a destination root before its first write) resolves through its nearest + * existing ancestor, so /var/x/.gitmem and /private/var/x/.gitmem still agree. + */ +export function canonicalPath(p: string): string { + const abs = path.resolve(p); + const rest: string[] = []; + let dir = abs; + for (;;) { + try { + return path.join(fs.realpathSync.native(dir), ...rest.reverse()); + } catch { + const parent = path.dirname(dir); + if (parent === dir) return abs; // nothing on the path exists + rest.push(path.basename(dir)); + dir = parent; + } + } +} + +/** GIT-107: the same directory, however it is spelled. */ +export function sameDirectory(a: string, b: string): boolean { + return canonicalPath(a) === canonicalPath(b); +} + export function findStrandedProjectRoots(): string[] { try { - const home = getHomeGitmemDir(); + // A root is only stranded if it is not the one being read: neither the + // developer-scoped home nor an explicit GITMEM_DIR. (process.env, not + // getGitmemDir(): that call is what triggers this scan.) + const readRoots = [getHomeGitmemDir(), ...(process.env.GITMEM_DIR ? [process.env.GITMEM_DIR] : [])]; const stranded: string[] = []; let dir = process.cwd(); const fsRoot = path.parse(dir).root; while (dir !== fsRoot) { const candidate = path.join(dir, ".gitmem"); - if (candidate !== home && isLiveGitmemRoot(candidate)) stranded.push(candidate); + if (!readRoots.some((r) => sameDirectory(candidate, r)) && isLiveGitmemRoot(candidate)) stranded.push(candidate); dir = path.dirname(dir); } return stranded; diff --git a/tests/unit/services/gitmem-root-symlink.test.ts b/tests/unit/services/gitmem-root-symlink.test.ts new file mode 100644 index 0000000..a7ec2cc --- /dev/null +++ b/tests/unit/services/gitmem-root-symlink.test.ts @@ -0,0 +1,103 @@ +/** + * GIT-107: roots are compared as directories, not strings. + * + * On macOS /var is a symlink to /private/var. process.cwd() reports the + * resolved /private/var/... while a home under $TMPDIR is spelled /var/..., so + * the store being read was announced as "Memory store found but NOT being + * read". Any symlink in either path does the same. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { findStrandedProjectRoots, sameDirectory, canonicalPath, clearGitmemDirCache } from "../../../src/services/gitmem-dir.js"; + +let tmp: string; +const saved = { home: process.env.GITMEM_HOME, dir: process.env.GITMEM_DIR }; + +/** A live project-scoped store: config.json marks a deliberate install. */ +function liveStore(dir: string): string { + const g = path.join(dir, ".gitmem"); + fs.mkdirSync(g, { recursive: true }); + fs.writeFileSync(path.join(g, "config.json"), JSON.stringify({ project: "p" })); + return g; +} + +beforeEach(() => { + tmp = fs.realpathSync.native(fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git107-"))); + delete process.env.GITMEM_DIR; + clearGitmemDirCache(); + vi.spyOn(console, "error").mockImplementation(() => {}); +}); + +afterEach(() => { + vi.restoreAllMocks(); + if (saved.home === undefined) delete process.env.GITMEM_HOME; else process.env.GITMEM_HOME = saved.home; + if (saved.dir === undefined) delete process.env.GITMEM_DIR; else process.env.GITMEM_DIR = saved.dir; + clearGitmemDirCache(); + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +describe("GIT-107: root comparison survives symlinks", () => { + it("home reached through a symlink is not reported as a stranded store", () => { + const real = path.join(tmp, "real-home"); + fs.mkdirSync(real); + liveStore(real); // the store gitmem reads + const link = path.join(tmp, "link-home"); + fs.symlinkSync(real, link, "dir"); + + process.env.GITMEM_HOME = link; // home spelled via the symlink + vi.spyOn(process, "cwd").mockReturnValue(path.join(real, "work")); // cwd spelled resolved + + expect(findStrandedProjectRoots()).toEqual([]); + }); + + it("a genuinely different live store is still reported", () => { + const home = path.join(tmp, "home"); + fs.mkdirSync(home); + const project = path.join(tmp, "project"); + const stranded = liveStore(project); + process.env.GITMEM_HOME = home; + vi.spyOn(process, "cwd").mockReturnValue(path.join(project, "src")); + + expect(findStrandedProjectRoots()).toEqual([stranded]); + }); + + it("an explicit GITMEM_DIR naming the project store is the store being read, not a stranded one", () => { + const project = path.join(tmp, "project"); + const store = liveStore(project); + process.env.GITMEM_HOME = path.join(tmp, "home"); + process.env.GITMEM_DIR = path.join(tmp, "project", ".", ".gitmem"); + vi.spyOn(process, "cwd").mockReturnValue(project); + + expect(findStrandedProjectRoots()).not.toContain(store); + }); + + it("sameDirectory: symlinks, trailing slashes, dot segments and not-yet-existing leaves", () => { + const real = path.join(tmp, "r"); + fs.mkdirSync(real); + const link = path.join(tmp, "l"); + fs.symlinkSync(real, link, "dir"); + + expect(sameDirectory(link, real)).toBe(true); + expect(sameDirectory(`${real}/`, path.join(real, ".", "."))).toBe(true); + // Destination roots may not exist yet; they resolve through their parent. + expect(sameDirectory(path.join(link, ".gitmem"), path.join(real, ".gitmem"))).toBe(true); + expect(canonicalPath(path.join(link, "a", "b"))).toBe(path.join(real, "a", "b")); + expect(sameDirectory(path.join(tmp, "x"), path.join(tmp, "y"))).toBe(false); + }); + + // The reported case: macOS /var -> /private/var. + const tmpdirReal = fs.realpathSync.native(os.tmpdir()); + it.skipIf(tmpdirReal === path.resolve(os.tmpdir()))("macOS: /var/... and /private/var/... are one store", () => { + const spelledTmp = path.join(os.tmpdir(), path.basename(tmp)); // /var/folders/... spelling + const real = path.join(tmp, "home"); + fs.mkdirSync(real); + liveStore(real); + process.env.GITMEM_HOME = path.join(spelledTmp, "home"); + vi.spyOn(process, "cwd").mockReturnValue(real); // what process.cwd() reports: /private/var/... + + expect(process.env.GITMEM_HOME).not.toBe(real); + expect(findStrandedProjectRoots()).toEqual([]); + }); +}); From b80d11bf63b3bf79ffb5482f95997070977e5867 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:52:07 -0700 Subject: [PATCH 18/24] test: venue flow fails on a false "NOT being read" notice (GIT-107) The driver's store lives under $TMPDIR, so 1.10.0 shows the notice on every macOS run (exit 1); exits 0 with the fix. Co-Authored-By: Claude Opus 5 --- tests/e2e/blank-supabase.mjs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..745b824 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -408,6 +408,9 @@ async function flow() { remote_sessions_after: await count("gitmem_sessions"), remote_scar_usage_after: await count("gitmem_scar_usage"), session_close_persisted: closedRows.length === 1 && closedRows[0].closing_reflection != null, + // GIT-107: the driver creates no stranded store; its GITMEM_DIR lives under + // $TMPDIR, i.e. /var/... vs /private/var/... on macOS. + false_stranded_notice: /NOT being read/.test(ss), relevance_readable: relevance.some((r) => scarIds.some((id) => (r.memories_applied || []).includes(id) && r.memory_relevance?.[id])), relevance, health_failed_total: failed, @@ -520,6 +523,9 @@ const failureCheck = checkFailures(allVenueRequests); const edgeCalls = allVenueRequests.filter((r) => r.path.startsWith("/functions/v1/")) .map((r) => `${r.method} ${r.path} -> ${r.status}`); if (edgeCalls.length) failureCheck.unexpected.push(...edgeCalls.map((c) => `edge function called (GIT-97): ${c}`)); +if (mode === "flow" && result.false_stranded_notice) { + failureCheck.unexpected.push("session_start reported the store it reads as 'NOT being read' (GIT-107)"); +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } From 00d8860957fc7db78d5c1f150f9275849b92e18f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:58:03 -0700 Subject: [PATCH 19/24] test: static schema-drift gate against the v1.8.0 setup.sql (GIT-109 step 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Customers never run SQL after initial setup, so every table and column gitmem touches must exist in the v1.8.0 schema or be deliberately guarded. - tests/helpers/schema-drift.ts parses setup.sql into table -> columns (CREATE TABLE, ALTER ... ADD COLUMN, views) and walks src/ with the TypeScript compiler API. It collects table + payload keys reaching directUpsert / directPatch / upsertRecord / storage.upsert, and select / filter / order keys reaching directQuery / directQueryAll / listRecords / getRecord. It follows getTableName(), const indirection, spreads, conditional spreads, later x.prop = assignments, local functions' returns and .map/.filter element shapes. Free-tier-only code is set aside; anything it cannot resolve is reported, never assumed fine. - tests/fixtures/schema/setup-v1.8.0.sql is the floor, byte-identical to `git show v1.8.0:schema/setup.sql`. - schema-drift.test.ts fails on a finding not allowlisted by ticket, a stale allowlist entry, an entry without a GIT-### ticket, a new unresolved call site, or a SESSION_COLUMNS entry v1.8.0 lacks that is not production-only. A synthetic source tree with planted drift proves the analyzer still catches it. First run: 74 call sites, 362 column accesses, 11 findings — 3 probe-guarded production-only columns (GIT-109) and 8 variant tables/columns reachable only on the dev tier (GIT-106). No other drift. +9 tests. Co-Authored-By: Claude Opus 5 --- tests/fixtures/schema/setup-v1.8.0.sql | 657 +++++++++++++++++++++++++ tests/helpers/schema-drift.ts | 529 ++++++++++++++++++++ tests/unit/schema-drift.test.ts | 179 +++++++ 3 files changed, 1365 insertions(+) create mode 100644 tests/fixtures/schema/setup-v1.8.0.sql create mode 100644 tests/helpers/schema-drift.ts create mode 100644 tests/unit/schema-drift.test.ts diff --git a/tests/fixtures/schema/setup-v1.8.0.sql b/tests/fixtures/schema/setup-v1.8.0.sql new file mode 100644 index 0000000..badd255 --- /dev/null +++ b/tests/fixtures/schema/setup-v1.8.0.sql @@ -0,0 +1,657 @@ +-- GitMem Schema Setup +-- Run this SQL in your Supabase SQL Editor to create the required tables. +-- Dashboard → SQL Editor → New query → Paste → Run + +-- Enable pgvector extension for semantic search +CREATE EXTENSION IF NOT EXISTS vector; + +-- ============================================================================ +-- Learnings table (scars, wins, patterns) +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_learnings ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + learning_type TEXT NOT NULL CHECK (learning_type IN ('scar', 'win', 'pattern', 'anti_pattern')), + title TEXT NOT NULL, + description TEXT NOT NULL, + severity TEXT CHECK (severity IN ('critical', 'high', 'medium', 'low')), + scar_type TEXT DEFAULT 'operational', + counter_arguments TEXT[] DEFAULT '{}', + problem_context TEXT DEFAULT '', + solution_approach TEXT DEFAULT '', + applies_when TEXT[] DEFAULT '{}', + keywords TEXT[] DEFAULT '{}', + domain TEXT[] DEFAULT '{}', + embedding vector(1536), + project TEXT DEFAULT 'default', + source_date DATE DEFAULT CURRENT_DATE, + source_linear_issue TEXT, + persona_name TEXT, + why_this_matters TEXT, + action_protocol TEXT, + self_check_criteria TEXT, + is_active BOOLEAN DEFAULT true, + decay_multiplier FLOAT DEFAULT 1.0, + repeat_mistake BOOLEAN DEFAULT false, + related_scar_id UUID, + repeat_mistake_details JSONB, + created_at TIMESTAMPTZ DEFAULT NOW(), + updated_at TIMESTAMPTZ DEFAULT NOW() +); + +-- Migration: ensure all columns exist (idempotent for upgrades) +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS embedding vector(1536); +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS source_linear_issue TEXT; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS persona_name TEXT; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS why_this_matters TEXT; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS action_protocol TEXT; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS self_check_criteria TEXT; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS is_active BOOLEAN DEFAULT true; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS decay_multiplier FLOAT DEFAULT 1.0; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS repeat_mistake BOOLEAN DEFAULT false; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS related_scar_id UUID; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS repeat_mistake_details JSONB; + +-- Indexes (created after migration ensures columns exist) +CREATE INDEX IF NOT EXISTS idx_gitmem_learnings_embedding + ON gitmem_learnings USING ivfflat (embedding vector_cosine_ops) + WITH (lists = 10); + +CREATE INDEX IF NOT EXISTS idx_gitmem_learnings_type + ON gitmem_learnings (learning_type); + +CREATE INDEX IF NOT EXISTS idx_gitmem_learnings_project + ON gitmem_learnings (project); + +-- ============================================================================ +-- Sessions table +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_sessions ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + session_title TEXT DEFAULT 'Interactive Session', + session_date DATE DEFAULT CURRENT_DATE, + agent TEXT DEFAULT 'Unknown', + project TEXT DEFAULT 'default', + linear_issue TEXT, + recording_path TEXT, + transcript_path TEXT, + decisions TEXT[] DEFAULT '{}', + open_threads TEXT[] DEFAULT '{}', + closing_reflection JSONB, + close_compliance JSONB, + rapport_summary TEXT, + embedding vector(1536), + created_at TIMESTAMPTZ DEFAULT NOW(), + updated_at TIMESTAMPTZ DEFAULT NOW() +); + +-- Migration: ensure all columns exist +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS embedding vector(1536); +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS linear_issue TEXT; +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS recording_path TEXT; +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS transcript_path TEXT; +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS close_compliance JSONB; +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS rapport_summary TEXT; + +CREATE INDEX IF NOT EXISTS idx_gitmem_sessions_agent + ON gitmem_sessions (agent); + +CREATE INDEX IF NOT EXISTS idx_gitmem_sessions_created + ON gitmem_sessions (created_at DESC); + +-- ============================================================================ +-- Decisions table +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_decisions ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + decision_date DATE DEFAULT CURRENT_DATE, + title TEXT NOT NULL, + decision TEXT NOT NULL, + rationale TEXT NOT NULL, + alternatives_considered TEXT[] DEFAULT '{}', + personas_involved TEXT[] DEFAULT '{}', + docs_affected TEXT[] DEFAULT '{}', + linear_issue TEXT, + session_id UUID REFERENCES gitmem_sessions(id), + project TEXT DEFAULT 'default', + embedding vector(1536), + created_at TIMESTAMPTZ DEFAULT NOW() +); + +-- Migration: ensure all columns exist +ALTER TABLE gitmem_decisions ADD COLUMN IF NOT EXISTS embedding vector(1536); +ALTER TABLE gitmem_decisions ADD COLUMN IF NOT EXISTS personas_involved TEXT[] DEFAULT '{}'; +ALTER TABLE gitmem_decisions ADD COLUMN IF NOT EXISTS docs_affected TEXT[] DEFAULT '{}'; +ALTER TABLE gitmem_decisions ADD COLUMN IF NOT EXISTS linear_issue TEXT; + +CREATE INDEX IF NOT EXISTS idx_gitmem_decisions_session + ON gitmem_decisions (session_id); + +-- ============================================================================ +-- Scar usage tracking +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_scar_usage ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + scar_id UUID REFERENCES gitmem_learnings(id), + session_id UUID REFERENCES gitmem_sessions(id), + agent TEXT DEFAULT 'Unknown', + issue_id TEXT, + issue_identifier TEXT, + reference_type TEXT CHECK (reference_type IN ('explicit', 'implicit', 'acknowledged', 'refuted', 'none')), + reference_context TEXT, + surfaced_at TIMESTAMPTZ, + acknowledged_at TIMESTAMPTZ, + referenced BOOLEAN, + execution_successful BOOLEAN, + variant_id UUID, + created_at TIMESTAMPTZ DEFAULT NOW() +); + +CREATE INDEX IF NOT EXISTS idx_gitmem_scar_usage_scar + ON gitmem_scar_usage (scar_id); + +CREATE INDEX IF NOT EXISTS idx_gitmem_scar_usage_session + ON gitmem_scar_usage (session_id); + +-- Migration: add columns for older installs +ALTER TABLE gitmem_scar_usage ADD COLUMN IF NOT EXISTS issue_id TEXT; +ALTER TABLE gitmem_scar_usage ADD COLUMN IF NOT EXISTS issue_identifier TEXT; +ALTER TABLE gitmem_scar_usage ADD COLUMN IF NOT EXISTS acknowledged_at TIMESTAMPTZ; +ALTER TABLE gitmem_scar_usage ADD COLUMN IF NOT EXISTS referenced BOOLEAN; +ALTER TABLE gitmem_scar_usage ADD COLUMN IF NOT EXISTS variant_id UUID; + +-- ============================================================================ +-- Threads table (cross-session work tracking) +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_threads ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + thread_id TEXT NOT NULL UNIQUE, + text TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'active' CHECK (status IN ('emerging', 'active', 'cooling', 'dormant', 'archived', 'resolved')), + thread_class TEXT DEFAULT 'operational' CHECK (thread_class IN ('operational', 'backlog')), + vitality_score FLOAT DEFAULT 1.0, + last_touched_at TIMESTAMPTZ DEFAULT NOW(), + touch_count INT DEFAULT 1, + resolved_at TIMESTAMPTZ, + resolution_note TEXT, + source_session UUID REFERENCES gitmem_sessions(id), + resolved_by_session UUID REFERENCES gitmem_sessions(id), + related_issues TEXT[] DEFAULT '{}', + domain TEXT[] DEFAULT '{}', + project TEXT DEFAULT 'default', + metadata JSONB DEFAULT '{}', + embedding vector(1536), + created_at TIMESTAMPTZ DEFAULT NOW(), + updated_at TIMESTAMPTZ DEFAULT NOW() +); + +CREATE INDEX IF NOT EXISTS idx_gitmem_threads_status + ON gitmem_threads (status); + +CREATE INDEX IF NOT EXISTS idx_gitmem_threads_project + ON gitmem_threads (project); + +-- ============================================================================ +-- Knowledge triples (knowledge graph) +-- ============================================================================ +CREATE TABLE IF NOT EXISTS knowledge_triples ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + subject TEXT NOT NULL, + predicate TEXT NOT NULL CHECK (predicate IN ('created_in', 'influenced_by', 'supersedes', 'demonstrates', 'affects_doc', 'created_thread', 'resolves_thread', 'relates_to_thread')), + object TEXT NOT NULL, + event_time TIMESTAMPTZ DEFAULT NOW(), + decay_weight FLOAT DEFAULT 1.0, + half_life_days INT DEFAULT 9999, + decay_floor FLOAT DEFAULT 0.1, + source_type TEXT, + source_id UUID, + source_linear_issue TEXT, + domain TEXT[] DEFAULT '{}', + project TEXT DEFAULT 'default', + created_by TEXT, + created_at TIMESTAMPTZ DEFAULT NOW(), + updated_at TIMESTAMPTZ DEFAULT NOW() +); + +CREATE INDEX IF NOT EXISTS idx_gitmem_triples_subject + ON knowledge_triples (subject); + +CREATE INDEX IF NOT EXISTS idx_gitmem_triples_project + ON knowledge_triples (project); + +-- ============================================================================ +-- Query metrics (performance tracking) +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_query_metrics ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + session_id UUID REFERENCES gitmem_sessions(id), + agent TEXT, + tool_name TEXT NOT NULL, + query_text TEXT, + tables_searched TEXT[], + latency_ms INT, + result_count INT, + similarity_scores FLOAT[], + context_bytes INT, + phase_tag TEXT, + linear_issue TEXT, + memories_surfaced UUID[], + metadata JSONB DEFAULT '{}', + created_at TIMESTAMPTZ DEFAULT NOW() +); + +-- ============================================================================ +-- Scar enforcement variants (A/B testing) +-- ============================================================================ +CREATE TABLE IF NOT EXISTS scar_enforcement_variants ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + scar_id UUID NOT NULL REFERENCES gitmem_learnings(id), + variant_type TEXT NOT NULL, + variant_config JSONB, + metadata JSONB DEFAULT '{}', + created_at TIMESTAMPTZ DEFAULT NOW(), + updated_at TIMESTAMPTZ DEFAULT NOW() +); + +-- ============================================================================ +-- Lite views (exclude embedding columns for reduced context) +-- ============================================================================ +CREATE OR REPLACE VIEW gitmem_learnings_lite AS + SELECT id, learning_type, title, description, severity, scar_type, + counter_arguments, problem_context, solution_approach, applies_when, + keywords, domain, project, source_date, source_linear_issue, + persona_name, is_active, decay_multiplier, created_at, updated_at + FROM gitmem_learnings; + +CREATE OR REPLACE VIEW gitmem_sessions_lite AS + SELECT id, session_title, session_date, agent, project, linear_issue, + decisions, open_threads, closing_reflection, close_compliance, + rapport_summary, created_at, updated_at + FROM gitmem_sessions; + +CREATE OR REPLACE VIEW gitmem_decisions_lite AS + SELECT id, decision_date, title, decision, rationale, + alternatives_considered, personas_involved, docs_affected, + linear_issue, session_id, project, created_at + FROM gitmem_decisions; + +CREATE OR REPLACE VIEW gitmem_threads_lite AS + SELECT id, thread_id, text, status, thread_class, vitality_score, + last_touched_at, touch_count, resolved_at, resolution_note, + source_session, resolved_by_session, related_issues, domain, + project, metadata, created_at, updated_at + FROM gitmem_threads; + +-- ============================================================================ +-- Semantic search RPC function +-- ============================================================================ +CREATE OR REPLACE FUNCTION gitmem_semantic_search( + query_embedding vector(1536), + match_count INT DEFAULT 5, + similarity_threshold FLOAT DEFAULT 0.0 +) +RETURNS TABLE ( + id UUID, + title TEXT, + description TEXT, + severity TEXT, + counter_arguments TEXT[], + similarity FLOAT +) +LANGUAGE plpgsql +AS $$ +BEGIN + RETURN QUERY + SELECT + l.id, + l.title, + l.description, + l.severity, + l.counter_arguments, + 1 - (l.embedding <=> query_embedding) AS similarity + FROM gitmem_learnings l + WHERE l.embedding IS NOT NULL + AND 1 - (l.embedding <=> query_embedding) > similarity_threshold + ORDER BY l.embedding <=> query_embedding + LIMIT match_count; +END; +$$; + +-- ============================================================================ +-- Scar search with temporal + behavioral decay weighting +-- ============================================================================ +CREATE OR REPLACE FUNCTION gitmem_scar_search( + query_embedding vector(1536), + match_count INT DEFAULT 5, + similarity_threshold FLOAT DEFAULT 0.0 +) +RETURNS TABLE ( + id UUID, + title TEXT, + description TEXT, + severity TEXT, + scar_type TEXT, + counter_arguments TEXT[], + decay_multiplier FLOAT, + similarity FLOAT, + weighted_similarity FLOAT +) +LANGUAGE plpgsql +AS $$ +DECLARE + decay_days INT; + temporal_weight FLOAT; +BEGIN + RETURN QUERY + SELECT + l.id, + l.title, + l.description, + l.severity, + l.scar_type, + l.counter_arguments, + COALESCE(l.decay_multiplier, 1.0) AS decay_multiplier, + (1 - (l.embedding <=> query_embedding))::FLOAT AS similarity, + -- Weighted similarity: raw * temporal_decay * behavioral_decay + ( + (1 - (l.embedding <=> query_embedding)) * + -- Temporal decay based on scar_type + CASE + WHEN l.scar_type = 'process' THEN 1.0 -- permanent + WHEN l.scar_type = 'incident' THEN + GREATEST(0.1, 1.0 - 0.9 * (EXTRACT(EPOCH FROM (NOW() - l.created_at)) / (180.0 * 86400))) + WHEN l.scar_type = 'context' THEN + GREATEST(0.1, 1.0 - 0.9 * (EXTRACT(EPOCH FROM (NOW() - l.created_at)) / (30.0 * 86400))) + ELSE 1.0 -- default: no decay + END * + -- Behavioral decay multiplier + COALESCE(l.decay_multiplier, 1.0) + )::FLOAT AS weighted_similarity + FROM gitmem_learnings l + WHERE l.embedding IS NOT NULL + AND COALESCE(l.is_active, true) = true + AND (1 - (l.embedding <=> query_embedding)) > similarity_threshold + ORDER BY weighted_similarity DESC + LIMIT match_count; +END; +$$; + +-- ============================================================================ +-- Refresh behavioral decay scores from scar_usage patterns +-- Aggregates last 90 days of usage, computes dismiss rate, +-- updates decay_multiplier on learnings (minimum 3 surfacings required) +-- ============================================================================ +CREATE OR REPLACE FUNCTION refresh_scar_behavioral_scores() +RETURNS TABLE (scars_updated INT, scars_scanned INT) +LANGUAGE plpgsql +AS $$ +DECLARE + v_scanned INT := 0; + v_updated INT := 0; +BEGIN + -- Aggregate scar_usage from last 90 days, compute dismiss rate + WITH usage_stats AS ( + SELECT + su.scar_id, + COUNT(*) AS times_surfaced, + COUNT(*) FILTER (WHERE su.reference_type IN ('none', 'refuted')) AS times_dismissed + FROM gitmem_scar_usage su + WHERE su.surfaced_at >= NOW() - INTERVAL '90 days' + GROUP BY su.scar_id + HAVING COUNT(*) >= 3 -- minimum surfacings for meaningful signal + ) + UPDATE gitmem_learnings l + SET decay_multiplier = GREATEST(0.1, 1.0 - (us.times_dismissed::FLOAT / us.times_surfaced::FLOAT) * 0.8), + updated_at = NOW() + FROM usage_stats us + WHERE l.id = us.scar_id + AND COALESCE(l.is_active, true) = true; + + GET DIAGNOSTICS v_updated = ROW_COUNT; + + SELECT COUNT(DISTINCT scar_id) INTO v_scanned + FROM gitmem_scar_usage + WHERE surfaced_at >= NOW() - INTERVAL '90 days'; + + RETURN QUERY SELECT v_updated, v_scanned; +END; +$$; + +-- ============================================================================ +-- Row Level Security +-- ============================================================================ + +-- Enable RLS on all tables +ALTER TABLE gitmem_learnings ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_sessions ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_decisions ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_scar_usage ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_threads ENABLE ROW LEVEL SECURITY; +ALTER TABLE knowledge_triples ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_query_metrics ENABLE ROW LEVEL SECURITY; +ALTER TABLE scar_enforcement_variants ENABLE ROW LEVEL SECURITY; + +-- Service role has full access (used by the MCP server) +-- Drop-then-create for idempotency (CREATE POLICY has no IF NOT EXISTS) +DO $$ BEGIN + -- gitmem_learnings + DROP POLICY IF EXISTS "Service role full access" ON gitmem_learnings; + CREATE POLICY "Service role full access" ON gitmem_learnings FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_learnings; + CREATE POLICY "Block anonymous access" ON gitmem_learnings FOR ALL USING (auth.role() != 'anon'); + -- gitmem_sessions + DROP POLICY IF EXISTS "Service role full access" ON gitmem_sessions; + CREATE POLICY "Service role full access" ON gitmem_sessions FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_sessions; + CREATE POLICY "Block anonymous access" ON gitmem_sessions FOR ALL USING (auth.role() != 'anon'); + -- gitmem_decisions + DROP POLICY IF EXISTS "Service role full access" ON gitmem_decisions; + CREATE POLICY "Service role full access" ON gitmem_decisions FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_decisions; + CREATE POLICY "Block anonymous access" ON gitmem_decisions FOR ALL USING (auth.role() != 'anon'); + -- gitmem_scar_usage + DROP POLICY IF EXISTS "Service role full access" ON gitmem_scar_usage; + CREATE POLICY "Service role full access" ON gitmem_scar_usage FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_scar_usage; + CREATE POLICY "Block anonymous access" ON gitmem_scar_usage FOR ALL USING (auth.role() != 'anon'); + -- gitmem_threads + DROP POLICY IF EXISTS "Service role full access" ON gitmem_threads; + CREATE POLICY "Service role full access" ON gitmem_threads FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_threads; + CREATE POLICY "Block anonymous access" ON gitmem_threads FOR ALL USING (auth.role() != 'anon'); + -- knowledge_triples + DROP POLICY IF EXISTS "Service role full access" ON knowledge_triples; + CREATE POLICY "Service role full access" ON knowledge_triples FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON knowledge_triples; + CREATE POLICY "Block anonymous access" ON knowledge_triples FOR ALL USING (auth.role() != 'anon'); + -- gitmem_query_metrics + DROP POLICY IF EXISTS "Service role full access" ON gitmem_query_metrics; + CREATE POLICY "Service role full access" ON gitmem_query_metrics FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_query_metrics; + CREATE POLICY "Block anonymous access" ON gitmem_query_metrics FOR ALL USING (auth.role() != 'anon'); + -- scar_enforcement_variants + DROP POLICY IF EXISTS "Service role full access" ON scar_enforcement_variants; + CREATE POLICY "Service role full access" ON scar_enforcement_variants FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON scar_enforcement_variants; + CREATE POLICY "Block anonymous access" ON scar_enforcement_variants FOR ALL USING (auth.role() != 'anon'); +END $$; + +-- ============================================================================ +-- Auto-update timestamps +-- ============================================================================ +CREATE OR REPLACE FUNCTION gitmem_update_timestamp() +RETURNS TRIGGER AS $$ +BEGIN + NEW.updated_at = NOW(); + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS gitmem_learnings_updated ON gitmem_learnings; +CREATE TRIGGER gitmem_learnings_updated + BEFORE UPDATE ON gitmem_learnings + FOR EACH ROW EXECUTE FUNCTION gitmem_update_timestamp(); + +DROP TRIGGER IF EXISTS gitmem_sessions_updated ON gitmem_sessions; +CREATE TRIGGER gitmem_sessions_updated + BEFORE UPDATE ON gitmem_sessions + FOR EACH ROW EXECUTE FUNCTION gitmem_update_timestamp(); + +-- ============================================================================ +-- License Management Tables (deployed on GitMem's infrastructure, not user's) +-- These tables are included here for reference and for our own deployment. +-- Users do NOT need to run these — they exist on gitmem-api.supabase.co +-- ============================================================================ + +CREATE TABLE IF NOT EXISTS gitmem_licenses ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + api_key TEXT UNIQUE NOT NULL, + tier TEXT NOT NULL DEFAULT 'pro' CHECK (tier IN ('pro', 'dev')), + owner_email TEXT, + is_active BOOLEAN NOT NULL DEFAULT true, + max_activations INTEGER NOT NULL DEFAULT 3, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + expires_at TIMESTAMPTZ, + last_validated_at TIMESTAMPTZ, + metadata JSONB DEFAULT '{}' +); + +CREATE TABLE IF NOT EXISTS gitmem_license_activations ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + license_id UUID NOT NULL REFERENCES gitmem_licenses(id) ON DELETE CASCADE, + install_id TEXT NOT NULL, + activated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + last_seen_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + device_info JSONB DEFAULT '{}', + UNIQUE(license_id, install_id) +); + +-- RLS: service role only (these are admin tables on our infrastructure) +ALTER TABLE gitmem_licenses ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_license_activations ENABLE ROW LEVEL SECURITY; + +CREATE POLICY "Service role full access" ON gitmem_licenses + FOR ALL USING (auth.role() = 'service_role'); + +CREATE POLICY "Service role full access" ON gitmem_license_activations + FOR ALL USING (auth.role() = 'service_role'); + +-- Index for fast key lookup +CREATE INDEX IF NOT EXISTS idx_gitmem_licenses_api_key + ON gitmem_licenses (api_key); + +CREATE INDEX IF NOT EXISTS idx_gitmem_license_activations_license + ON gitmem_license_activations (license_id); + +-- ============================================================================ +-- License Validation RPC +-- Checks key validity, device limit, registers/updates activation +-- ============================================================================ +CREATE OR REPLACE FUNCTION gitmem_validate_license(p_api_key TEXT, p_install_id TEXT) +RETURNS TABLE(tier TEXT, valid BOOLEAN, message TEXT) +LANGUAGE plpgsql +SECURITY DEFINER +AS $$ +DECLARE + v_license_id UUID; + v_tier TEXT; + v_is_active BOOLEAN; + v_max_activations INTEGER; + v_expires_at TIMESTAMPTZ; + v_current_activations INTEGER; +BEGIN + -- Look up the license + SELECT l.id, l.tier, l.is_active, l.max_activations, l.expires_at + INTO v_license_id, v_tier, v_is_active, v_max_activations, v_expires_at + FROM gitmem_licenses l + WHERE l.api_key = p_api_key; + + -- Key not found + IF v_license_id IS NULL THEN + RETURN QUERY SELECT NULL::TEXT, false, 'Invalid license key'::TEXT; + RETURN; + END IF; + + -- Key deactivated + IF NOT v_is_active THEN + RETURN QUERY SELECT NULL::TEXT, false, 'License has been deactivated'::TEXT; + RETURN; + END IF; + + -- Key expired + IF v_expires_at IS NOT NULL AND v_expires_at < NOW() THEN + RETURN QUERY SELECT NULL::TEXT, false, 'License has expired'::TEXT; + RETURN; + END IF; + + -- Count current activations (excluding this install_id if already registered) + SELECT COUNT(*) + INTO v_current_activations + FROM gitmem_license_activations a + WHERE a.license_id = v_license_id + AND a.install_id != p_install_id; + + -- Check device limit (if this is a new device) + IF v_current_activations >= v_max_activations THEN + -- Check if this install_id already exists (re-validation) + IF NOT EXISTS ( + SELECT 1 FROM gitmem_license_activations a + WHERE a.license_id = v_license_id AND a.install_id = p_install_id + ) THEN + RETURN QUERY SELECT NULL::TEXT, false, + format('Device limit reached (%s/%s). Deactivate another device or contact support.', v_current_activations, v_max_activations)::TEXT; + RETURN; + END IF; + END IF; + + -- Register or update activation + INSERT INTO gitmem_license_activations (license_id, install_id, last_seen_at) + VALUES (v_license_id, p_install_id, NOW()) + ON CONFLICT (license_id, install_id) + DO UPDATE SET last_seen_at = NOW(); + + -- Update last_validated_at on the license + UPDATE gitmem_licenses SET last_validated_at = NOW() WHERE id = v_license_id; + + -- Success + RETURN QUERY SELECT v_tier, true, 'Valid'::TEXT; +END; +$$; + +-- ============================================================================ +-- Device Deactivation RPC +-- Removes a device activation for a license key + install_id pair. +-- Called by `gitmem-mcp deactivate` to free up a device slot. +-- ============================================================================ +CREATE OR REPLACE FUNCTION gitmem_deactivate_device(p_api_key TEXT, p_install_id TEXT) +RETURNS TABLE(success BOOLEAN, message TEXT) +LANGUAGE plpgsql +SECURITY DEFINER +AS $$ +DECLARE + v_license_id UUID; + v_deleted INTEGER; +BEGIN + -- Look up the license + SELECT l.id INTO v_license_id + FROM gitmem_licenses l + WHERE l.api_key = p_api_key; + + IF v_license_id IS NULL THEN + RETURN QUERY SELECT false, 'Invalid license key'::TEXT; + RETURN; + END IF; + + -- Delete the activation for this install_id + DELETE FROM gitmem_license_activations a + WHERE a.license_id = v_license_id + AND a.install_id = p_install_id; + + GET DIAGNOSTICS v_deleted = ROW_COUNT; + + IF v_deleted > 0 THEN + RETURN QUERY SELECT true, 'Device deactivated'::TEXT; + ELSE + RETURN QUERY SELECT true, 'Device was not registered (already deactivated)'::TEXT; + END IF; +END; +$$; diff --git a/tests/helpers/schema-drift.ts b/tests/helpers/schema-drift.ts new file mode 100644 index 0000000..67b19dc --- /dev/null +++ b/tests/helpers/schema-drift.ts @@ -0,0 +1,529 @@ +/** + * GIT-109 step 2: static schema-drift analysis. + * + * Customers never run SQL after initial setup (standing rule, 2026-09-21), so + * every table and column gitmem touches must exist in the v1.8.0 setup.sql — + * or be deliberately guarded. Release B found closes lost to exactly this: the + * code wrote production-only columns a setup.sql store does not have. + * + * This walks src/ with the TypeScript compiler API and collects, per call site: + * writes — table + payload keys reaching directUpsert / directPatch / + * upsertRecord / storage.upsert + * reads — table + select / filter / order keys reaching directQuery, + * directQueryAll, listRecords, getRecord + * and compares them with a parsed setup.sql (table -> columns, views included). + * + * Resolution is deliberately conservative: a table or payload it cannot + * resolve statically is reported as UNRESOLVED, never assumed fine. + */ +import * as fs from "fs"; +import * as path from "path"; +import ts from "typescript"; + +// ---------------------------------------------------------------- schema + +export type Schema = Map>; + +const CONSTRAINT_WORDS = /^(PRIMARY|UNIQUE|CONSTRAINT|FOREIGN|CHECK|EXCLUDE)\b/i; + +/** setup.sql -> table/view name -> columns. */ +export function parseSchema(sql: string): Schema { + const schema: Schema = new Map(); + const clean = sql.replace(/--[^\n]*/g, ""); + const add = (t: string, c: string) => { + if (!schema.has(t)) schema.set(t, new Set()); + schema.get(t)!.add(c); + }; + + // CREATE TABLE ... ( body ); — body split on top-level commas + const tableRe = /CREATE TABLE(?: IF NOT EXISTS)?\s+(?:public\.)?(\w+)\s*\(/gi; + let m: RegExpExecArray | null; + while ((m = tableRe.exec(clean))) { + const table = m[1]; + let depth = 1, i = tableRe.lastIndex, start = i; + const parts: string[] = []; + for (; i < clean.length && depth > 0; i++) { + const ch = clean[i]; + if (ch === "(") depth++; + else if (ch === ")") { depth--; if (depth === 0) parts.push(clean.slice(start, i)); } + else if (ch === "," && depth === 1) { parts.push(clean.slice(start, i)); start = i + 1; } + } + if (!schema.has(table)) schema.set(table, new Set()); + for (const p of parts) { + const line = p.trim(); + if (!line || CONSTRAINT_WORDS.test(line)) continue; + const col = line.match(/^"?(\w+)"?/); + if (col) add(table, col[1]); + } + } + + const alterRe = /ALTER TABLE\s+(?:IF EXISTS\s+)?(?:public\.)?(\w+)\s+ADD COLUMN(?: IF NOT EXISTS)?\s+"?(\w+)"?/gi; + while ((m = alterRe.exec(clean))) add(m[1], m[2]); + + const viewRe = /CREATE (?:OR REPLACE )?VIEW\s+(?:public\.)?(\w+)\s+AS\s+SELECT\s+([\s\S]+?)\s+FROM\s+(?:public\.)?(\w+)/gi; + while ((m = viewRe.exec(clean))) { + const [, view, list, base] = m; + if (!schema.has(view)) schema.set(view, new Set()); + for (const item of list.split(",").map((s) => s.trim())) { + if (item === "*") { for (const c of schema.get(base) ?? []) add(view, c); continue; } + const alias = item.match(/\bAS\s+(\w+)$/i); + add(view, alias ? alias[1] : item.replace(/^\w+\./, "").match(/^(\w+)/)?.[1] ?? item); + } + } + return schema; +} + +// ---------------------------------------------------------------- code scan + +export type AccessKind = "write" | "select" | "filter" | "order"; + +export interface Access { + file: string; + line: number; + call: string; + table: string; + column: string; + kind: AccessKind; +} + +export interface Unresolved { + file: string; + line: number; + call: string; + what: string; +} + +export interface ScanResult { + accesses: Access[]; + /** Generic helpers whose table is a parameter (storage.ts, supabase-client.ts): checked at their callers. */ + passThrough: Unresolved[]; + /** Tables reached with no column information (e.g. a bare select=*). */ + tables: Array<{ file: string; line: number; call: string; table: string }>; + unresolved: Unresolved[]; + /** Call sites in free-tier-only code (local JSON files, not the store). */ + freeTierOnly: number; + callSites: number; +} + +const WRITE_CALLS = new Set(["directUpsert", "upsertRecord"]); +const PATCH_CALLS = new Set(["directPatch"]); +const QUERY_CALLS = new Set(["directQuery", "directQueryAll"]); +const DEFAULT_PREFIX = "gitmem_"; + +function calleeName(expr: ts.LeftHandSideExpression): { name: string; receiver?: string } | null { + if (ts.isIdentifier(expr)) return { name: expr.text }; + if (ts.isPropertyAccessExpression(expr)) { + const recv = expr.expression; + const receiver = ts.isIdentifier(recv) ? recv.text : ts.isCallExpression(recv) && ts.isIdentifier(recv.expression) ? `${recv.expression.text}()` : undefined; + return { name: expr.name.text, receiver }; + } + return null; +} + +/** Find the nearest declaration of `name` visible from `from` (same file). */ +function findDeclaration(name: string, from: ts.Node): ts.VariableDeclaration | ts.ParameterDeclaration | null { + let scope: ts.Node | undefined = from; + while (scope) { + let found: ts.VariableDeclaration | ts.ParameterDeclaration | null = null; + const visit = (n: ts.Node): void => { + if (found) return; + if (ts.isVariableDeclaration(n) && ts.isIdentifier(n.name) && n.name.text === name && n.pos < from.pos) found = n; + else if (ts.isParameter(n) && ts.isIdentifier(n.name) && n.name.text === name) found = n; + if (n !== scope && (ts.isFunctionLike(n) || ts.isClassLike(n))) { + // parameters of the enclosing function are visited via the scope itself + return; + } + ts.forEachChild(n, visit); + }; + if (ts.isFunctionLike(scope)) for (const p of scope.parameters) visit(p); + ts.forEachChild(scope, visit); + if (found) return found; + scope = scope.parent; + } + return null; +} + +function resolveTable(expr: ts.Expression | undefined): string | null { + if (!expr) return null; + if (ts.isStringLiteralLike(expr)) return expr.text; + if (ts.isParenthesizedExpression(expr) || ts.isAsExpression(expr) || ts.isNonNullExpression(expr)) return resolveTable(expr.expression); + if (ts.isCallExpression(expr) && ts.isIdentifier(expr.expression) && expr.expression.text === "getTableName") { + const arg = expr.arguments[0]; + return arg && ts.isStringLiteralLike(arg) ? DEFAULT_PREFIX + arg.text : null; + } + if (ts.isIdentifier(expr)) { + const decl = findDeclaration(expr.text, expr); + if (decl && ts.isVariableDeclaration(decl) && decl.initializer) return resolveTable(decl.initializer); + } + return null; +} + +interface KeySet { keys: Set; complete: boolean; why: string[] } + +function propName(p: ts.ObjectLiteralElementLike): string | null { + const n = p.name; + if (!n) return null; + if (ts.isIdentifier(n) || ts.isStringLiteralLike(n) || ts.isNumericLiteral(n)) return n.text; + return null; +} + +/** Keys of an object-valued expression, following spreads, conditionals and later assignments. */ +function resolveKeys(expr: ts.Expression | undefined, seen = new Set()): KeySet { + const out: KeySet = { keys: new Set(), complete: true, why: [] }; + const merge = (k: KeySet) => { k.keys.forEach((x) => out.keys.add(x)); if (!k.complete) { out.complete = false; out.why.push(...k.why); } }; + if (!expr) return { keys: new Set(), complete: false, why: ["no payload"] }; + if (seen.has(expr)) return out; + seen.add(expr); + + if (ts.isParenthesizedExpression(expr) || ts.isAsExpression(expr) || ts.isNonNullExpression(expr) || ts.isSatisfiesExpression?.(expr)) { + return resolveKeys((expr as ts.ParenthesizedExpression).expression, seen); + } + if (ts.isObjectLiteralExpression(expr)) { + for (const p of expr.properties) { + if (ts.isSpreadAssignment(p)) merge(resolveKeys(p.expression, seen)); + else if (ts.isShorthandPropertyAssignment(p)) out.keys.add(p.name.text); + else { + const n = propName(p); + if (n) out.keys.add(n); else { out.complete = false; out.why.push(`computed key ${p.getText().slice(0, 40)}`); } + } + } + return out; + } + // `cond && {…}` / `cond ? {…} : {…}` + if (ts.isBinaryExpression(expr) && (expr.operatorToken.kind === ts.SyntaxKind.AmpersandAmpersandToken || expr.operatorToken.kind === ts.SyntaxKind.BarBarToken || expr.operatorToken.kind === ts.SyntaxKind.QuestionQuestionToken)) { + if (ts.isObjectLiteralExpression(expr.right) || ts.isParenthesizedExpression(expr.right)) return resolveKeys(expr.right, seen); + } + if (ts.isConditionalExpression(expr)) { + merge(resolveKeys(expr.whenTrue, seen)); + merge(resolveKeys(expr.whenFalse, seen)); + return out; + } + if (ts.isIdentifier(expr)) { + const decl = findDeclaration(expr.text, expr); + // A callback parameter: `rows.map((r) => write(r))` — the elements of rows. + if (decl && ts.isParameter(decl) && ts.isIdentifier(decl.name)) { + const fn = decl.parent; + const call = fn.parent; + if ((ts.isArrowFunction(fn) || ts.isFunctionExpression(fn)) && ts.isCallExpression(call) && call.arguments[0] === fn && + ts.isPropertyAccessExpression(call.expression) && /^(map|forEach|flatMap)$/.test(call.expression.name.text)) { + return elementKeys(call.expression.expression, seen); + } + } + if (decl && ts.isVariableDeclaration(decl) && decl.initializer) { + merge(resolveKeys(decl.initializer, seen)); + // later `x.prop = …` / `x["prop"] = …` in the same function + const fn = enclosingFunction(decl) ?? decl.getSourceFile(); + const visit = (n: ts.Node): void => { + if (ts.isBinaryExpression(n) && n.operatorToken.kind === ts.SyntaxKind.EqualsToken) { + const l = n.left; + if (ts.isPropertyAccessExpression(l) && ts.isIdentifier(l.expression) && l.expression.text === expr.text) out.keys.add(l.name.text); + if (ts.isElementAccessExpression(l) && ts.isIdentifier(l.expression) && l.expression.text === expr.text && ts.isStringLiteralLike(l.argumentExpression)) out.keys.add(l.argumentExpression.text); + } + ts.forEachChild(n, visit); + }; + visit(fn); + return out; + } + return { keys: new Set(), complete: false, why: [`payload ${expr.text} not resolvable`] }; + } + if (ts.isCallExpression(expr)) { + // Column filters narrow a payload; the payload is their first argument. + const name = ts.isIdentifier(expr.expression) ? expr.expression.text : ts.isPropertyAccessExpression(expr.expression) ? expr.expression.name.text : ""; + if (/^filterTo|Columns$/.test(name) && expr.arguments[0]) { + const k = resolveKeys(expr.arguments[0], seen); + k.why.push(`filtered by ${name}`); + return k; + } + // A function in this file that returns an object literal. + const local = ts.isIdentifier(expr.expression) ? findLocalFunction(expr.expression.text, expr.getSourceFile()) : null; + if (local) return returnKeys(local, seen); + if (ts.isAwaitExpression(expr.parent)) {/* fallthrough */} + return { keys: new Set(), complete: false, why: [`payload from ${name || "call"}()`] }; + } + if (ts.isAwaitExpression(expr)) return resolveKeys(expr.expression, seen); + return { keys: new Set(), complete: false, why: [`payload ${ts.SyntaxKind[expr.kind]}`] }; +} + +function findLocalFunction(name: string, sf: ts.SourceFile): ts.FunctionLikeDeclaration | null { + let found: ts.FunctionLikeDeclaration | null = null; + const visit = (n: ts.Node): void => { + if (found) return; + if (ts.isFunctionDeclaration(n) && n.name?.text === name && n.body) found = n; + else if (ts.isVariableDeclaration(n) && ts.isIdentifier(n.name) && n.name.text === name && n.initializer && + (ts.isArrowFunction(n.initializer) || ts.isFunctionExpression(n.initializer))) found = n.initializer; + else ts.forEachChild(n, visit); + }; + visit(sf); + return found; +} + +/** Keys of every object a function returns (its own returns, not nested functions'). */ +function returnKeys(fn: ts.FunctionLikeDeclaration, seen: Set): KeySet { + const out: KeySet = { keys: new Set(), complete: true, why: [] }; + const merge = (k: KeySet) => { k.keys.forEach((x) => out.keys.add(x)); if (!k.complete) { out.complete = false; out.why.push(...k.why); } }; + if (fn.body && !ts.isBlock(fn.body)) { merge(resolveKeys(fn.body as ts.Expression, seen)); return out; } + const visit = (n: ts.Node): void => { + if (n !== fn && ts.isFunctionLike(n)) return; + if (ts.isReturnStatement(n) && n.expression) merge(resolveKeys(n.expression, seen)); + ts.forEachChild(n, visit); + }; + if (fn.body) visit(fn.body); + if (out.keys.size === 0 && out.complete) return { keys: out.keys, complete: false, why: ["function returns no object literal"] }; + return out; +} + +/** Keys of the elements of an array-valued expression. */ +function elementKeys(expr: ts.Expression, seen: Set): KeySet { + const out: KeySet = { keys: new Set(), complete: true, why: [] }; + const merge = (k: KeySet) => { k.keys.forEach((x) => out.keys.add(x)); if (!k.complete) { out.complete = false; out.why.push(...k.why); } }; + if (ts.isParenthesizedExpression(expr) || ts.isAsExpression(expr)) return elementKeys(expr.expression, seen); + if (ts.isArrayLiteralExpression(expr)) { + for (const el of expr.elements) merge(ts.isSpreadElement(el) ? elementKeys(el.expression, seen) : resolveKeys(el, seen)); + return out; + } + if (ts.isIdentifier(expr)) { + const d = findDeclaration(expr.text, expr); + if (d && ts.isVariableDeclaration(d) && d.initializer) return elementKeys(d.initializer, seen); + } + if (ts.isCallExpression(expr) && ts.isPropertyAccessExpression(expr.expression)) { + const m = expr.expression.name.text; + if (/^(filter|slice|concat|reverse|sort)$/.test(m)) return elementKeys(expr.expression.expression, seen); + const cb = expr.arguments[0]; + if (m === "map" && cb && (ts.isArrowFunction(cb) || ts.isFunctionExpression(cb))) return returnKeys(cb, seen); + } + return { keys: new Set(), complete: false, why: [`elements of ${expr.getText().slice(0, 40)}`] }; +} + +function enclosingFunction(n: ts.Node): ts.Node | null { + let p: ts.Node | undefined = n.parent; + while (p) { if (ts.isFunctionLike(p)) return p; p = p.parent; } + return null; +} + +/** PostgREST select list -> base columns (alias:col, col::cast, col->>path; embeds skipped). */ +export function selectColumns(select: string): { columns: string[]; star: boolean } { + const cols: string[] = []; + let star = false, depth = 0, cur = ""; + const flush = () => { + const item = cur.trim(); cur = ""; + if (!item) return; + if (item === "*") { star = true; return; } + if (item.includes("(")) return; // resource embedding rel(...) + const body = item.includes(":") && !item.includes("::") ? item.split(":").pop()! : item.replace(/^\w+:(?!:)/, ""); + const col = body.split("::")[0].split("->")[0].trim(); + if (/^\w+$/.test(col)) cols.push(col); + }; + for (const ch of select) { + if (ch === "(") depth++; + if (ch === ")") depth--; + if (ch === "," && depth === 0) flush(); else cur += ch; + } + flush(); + return { columns: cols, star }; +} + +function resolveString(expr: ts.Expression | undefined): string | null { + if (!expr) return null; + if (ts.isStringLiteralLike(expr)) return expr.text; + if (ts.isIdentifier(expr)) { + const decl = findDeclaration(expr.text, expr); + if (decl && ts.isVariableDeclaration(decl) && decl.initializer) return resolveString(decl.initializer); + } + if (ts.isBinaryExpression(expr) && expr.operatorToken.kind === ts.SyntaxKind.PlusToken) { + const l = resolveString(expr.left), r = resolveString(expr.right); + return l !== null && r !== null ? l + r : null; + } + return null; +} + +/** Inside code that only runs without Supabase (local JSON files). */ +function isFreeTierOnly(n: ts.Node): boolean { + let child: ts.Node = n; + let p: ts.Node | undefined = n.parent; + while (p) { + if (ts.isIfStatement(p)) { + const cond = p.expression.getText(); + const positive = /^hasSupabase\(\)(\s*&&.*)?$/.test(cond); + const negative = /^!hasSupabase\(\)/.test(cond); + if (positive && p.elseStatement === child) return true; + if (negative && p.thenStatement === child) return true; + } + if ((ts.isFunctionDeclaration(p) || ts.isMethodDeclaration(p)) && p.name && /Free$/.test(p.name.getText())) return true; + child = p; + p = p.parent; + } + return false; +} + +function objectProp(obj: ts.Expression | undefined, key: string): ts.Expression | undefined { + if (!obj) return undefined; + if (ts.isIdentifier(obj)) { + const d = findDeclaration(obj.text, obj); + return d && ts.isVariableDeclaration(d) ? objectProp(d.initializer, key) : undefined; + } + if (!ts.isObjectLiteralExpression(obj)) return undefined; + for (const p of obj.properties) { + if (ts.isPropertyAssignment(p) && propName(p) === key) return p.initializer; + if (ts.isShorthandPropertyAssignment(p) && p.name.text === key) { + const d = findDeclaration(key, p); + return d && ts.isVariableDeclaration(d) ? d.initializer : undefined; + } + } + return undefined; +} + +export function scanSources(srcDir: string, exclude: (file: string) => boolean = () => false): ScanResult { + const result: ScanResult = { accesses: [], passThrough: [], tables: [], unresolved: [], freeTierOnly: 0, callSites: 0 }; + const files: string[] = []; + const walk = (d: string) => { + for (const e of fs.readdirSync(d, { withFileTypes: true })) { + const p = path.join(d, e.name); + if (e.isDirectory()) walk(p); + else if (e.name.endsWith(".ts") && !e.name.endsWith(".test.ts") && !e.name.endsWith(".d.ts")) files.push(p); + } + }; + walk(srcDir); + + for (const file of files) { + const rel = path.relative(path.dirname(srcDir), file); + if (exclude(rel)) continue; + const sf = ts.createSourceFile(file, fs.readFileSync(file, "utf-8"), ts.ScriptTarget.Latest, true); + const visit = (n: ts.Node): void => { + if (ts.isCallExpression(n)) inspect(n, sf, rel, result); + ts.forEachChild(n, visit); + }; + visit(sf); + } + return result; +} + +function inspect(call: ts.CallExpression, sf: ts.SourceFile, file: string, r: ScanResult): void { + const c = calleeName(call.expression); + if (!c) return; + const isStorageUpsert = c.name === "upsert" && (c.receiver === "storage" || c.receiver === "getStorage()"); + const kind = WRITE_CALLS.has(c.name) ? "write" : PATCH_CALLS.has(c.name) ? "patch" : QUERY_CALLS.has(c.name) ? "query" + : c.name === "listRecords" ? "list" : c.name === "getRecord" ? "get" : isStorageUpsert ? "storage" : null; + if (!kind) return; + // Skip the definitions themselves (supabase-client.ts calling its own helpers with parameters). + const line = sf.getLineAndCharacterOfPosition(call.getStart()).line + 1; + const callText = c.receiver ? `${c.receiver}.${c.name}` : c.name; + r.callSites++; + if (isFreeTierOnly(call)) { r.freeTierOnly++; return; } + + const args = call.arguments; + const addCols = (table: string, cols: Iterable, k: AccessKind) => { + for (const col of cols) r.accesses.push({ file, line, call: callText, table, column: col, kind: k }); + }; + const unresolved = (what: string) => r.unresolved.push({ file, line, call: callText, what }); + + let tableExpr: ts.Expression | undefined = args[0]; + let table: string | null; + if (kind === "storage") { + const coll = args[0] && ts.isStringLiteralLike(args[0]) ? args[0].text : null; + table = coll ? DEFAULT_PREFIX + coll : null; + } else if (kind === "list") { + tableExpr = objectProp(args[0], "table"); + table = resolveTable(tableExpr); + } else { + table = resolveTable(tableExpr); + } + if (!table) { + const paramOf = (e: ts.Expression | undefined): boolean => { + if (!e) return false; + if (ts.isIdentifier(e)) { + const d = findDeclaration(e.text, e); + if (d && ts.isParameter(d)) return true; + if (d && ts.isVariableDeclaration(d)) return paramOf(d.initializer); + // shorthand `{ table }` inside listRecords({ table, ... }) + return false; + } + if (ts.isCallExpression(e) && ts.isIdentifier(e.expression) && e.expression.text === "getTableName") return paramOf(e.arguments[0]); + return false; + }; + const isParam = paramOf(tableExpr) || (kind === "list" && !tableExpr && !!args[0] && ts.isObjectLiteralExpression(args[0]) && + args[0].properties.some((p) => ts.isShorthandPropertyAssignment(p) && p.name.text === "table" && (() => { const d = findDeclaration("table", p); return !!d && ts.isParameter(d); })())); + const isStorageParam = kind === "storage" && args[0] && ts.isIdentifier(args[0]); + if (isParam || isStorageParam) r.passThrough.push({ file, line, call: callText, what: `table is parameter ${tableExpr?.getText() ?? args[0]?.getText()}` }); + else unresolved(`table ${tableExpr?.getText().slice(0, 50) ?? "(none)"}`); + return; + } + + const keysFrom = (e: ts.Expression | undefined, k: AccessKind, label: string) => { + const ks = resolveKeys(e); + addCols(table!, [...ks.keys].filter((x) => !(k === "filter" && (x === "or" || x === "and" || x === "not"))), k); + if (!ks.complete) unresolved(`${label}: ${ks.why.join("; ")}`); + }; + + switch (kind) { + case "write": + case "storage": + keysFrom(args[1], "write", "payload"); + break; + case "patch": + keysFrom(args[1], "filter", "patch filter"); + keysFrom(args[2], "write", "patch payload"); + break; + case "get": + addCols(table, ["id"], "filter"); + break; + case "query": + case "list": { + const opts = kind === "query" ? args[1] : args[0]; + const selExpr = objectProp(opts, kind === "query" ? "select" : "columns"); + if (selExpr) { + const sel = resolveString(selExpr); + if (sel === null) unresolved(`select ${selExpr.getText().slice(0, 50)}`); + else { + const { columns, star } = selectColumns(sel); + addCols(table, columns, "select"); + if (star || columns.length === 0) r.tables.push({ file, line, call: callText, table }); + } + } else { + r.tables.push({ file, line, call: callText, table }); // default select=* + } + const filt = objectProp(opts, "filters"); + if (filt) keysFrom(filt, "filter", "filters"); + const order = kind === "query" ? resolveString(objectProp(opts, "order")) : null; + if (order) addCols(table, order.split(",").map((o) => o.split(".")[0].trim()).filter(Boolean), "order"); + const orderBy = kind === "list" ? objectProp(opts, "orderBy") : undefined; + if (orderBy) { + const col = resolveString(objectProp(orderBy, "column")); + if (col) addCols(table, [col], "order"); + } + break; + } + } +} + +// ---------------------------------------------------------------- compare + +export interface Finding { + table: string; + /** null: the table itself is missing. */ + column: string | null; + kind: AccessKind | "table"; + sites: string[]; + /** Present in the current schema/setup.sql but not in v1.8.0. */ + addedAfterFloor: boolean; +} + +export function compare(scan: ScanResult, floor: Schema, current: Schema): Finding[] { + const byKey = new Map(); + const note = (table: string, column: string | null, kind: Finding["kind"], site: string) => { + const key = `${table}${column ?? ""}${column === null ? "table" : kind}`; + let f = byKey.get(key); + if (!f) { + const inCurrent = column === null ? current.has(table) : !!current.get(table)?.has(column); + f = { table, column, kind: column === null ? "table" : kind, sites: [], addedAfterFloor: inCurrent }; + byKey.set(key, f); + } + if (!f.sites.includes(site)) f.sites.push(site); + }; + for (const a of scan.accesses) { + const site = `${a.file}:${a.line}`; + if (!floor.has(a.table)) note(a.table, null, "table", site); + else if (!floor.get(a.table)!.has(a.column)) note(a.table, a.column, a.kind, site); + } + for (const t of scan.tables) if (!floor.has(t.table)) note(t.table, null, "table", `${t.file}:${t.line}`); + return [...byKey.values()].sort((x, y) => `${x.table}.${x.column}`.localeCompare(`${y.table}.${y.column}`)); +} diff --git a/tests/unit/schema-drift.test.ts b/tests/unit/schema-drift.test.ts new file mode 100644 index 0000000..d29936b --- /dev/null +++ b/tests/unit/schema-drift.test.ts @@ -0,0 +1,179 @@ +/** + * GIT-109 step 2: nothing gitmem reads or writes may be missing from the + * v1.8.0 setup.sql, unless it is allowlisted against the ticket that owns it. + * + * Standing rule (Chris, 2026-09-21): customers never run SQL after initial + * setup, so every store in the field may still be on the v1.8.0 schema. The + * pinned copy is tests/fixtures/schema/setup-v1.8.0.sql (byte-identical to + * `git show v1.8.0:schema/setup.sql`). + * + * What fails: + * - a table or column reached from src/ that v1.8.0 does not have and that + * is not on ALLOWED below; + * - an ALLOWED entry that no longer matches anything (the list only shrinks); + * - an ALLOWED entry without a GIT-### ticket; + * - a call site the analyzer cannot resolve that is not on KNOWN_UNRESOLVED + * (so coverage cannot silently decay); + * - a SESSION_COLUMNS entry missing from v1.8.0 that is not production-only. + * See tests/helpers/schema-drift.ts for what is collected and how. + */ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { parseSchema, scanSources, compare, selectColumns } from "../helpers/schema-drift.js"; +import type { Finding, ScanResult, Schema } from "../helpers/schema-drift.js"; +import { SESSION_COLUMNS, PRODUCTION_ONLY_SESSION_COLUMNS } from "../../src/services/session-columns.js"; + +const REPO = path.resolve(__dirname, "../.."); +const FLOOR_SQL = path.join(REPO, "tests/fixtures/schema/setup-v1.8.0.sql"); +const CURRENT_SQL = path.join(REPO, "schema/setup.sql"); + +interface Allowed { + table: string; + column: string | null; + kind: Finding["kind"]; + ticket: string; + reason: string; +} + +/** + * Known gaps, each owned by a ticket. Remove an entry when its ticket lands — + * the test fails on an entry that matches nothing. + */ +const ALLOWED: Allowed[] = [ + // Written only where the store has the column (store-columns.ts probe). + { table: "gitmem_learnings", column: "archived_at", kind: "write", ticket: "GIT-109", reason: "probe-guarded: archive-learning.ts writes it only if supportedColumns() finds it" }, + { table: "gitmem_sessions", column: "claude_code_session_id", kind: "write", ticket: "GIT-109", reason: "probe-guarded: production-only, dropped by filterToStoreSessionColumns() on setup.sql stores" }, + { table: "gitmem_sessions", column: "task_observations", kind: "write", ticket: "GIT-109", reason: "probe-guarded: absorb-observations.ts writes it only if supportedColumns() finds it" }, + // Scar variant A/B testing is dev-tier only (hasVariants()); customer schemas do not provision it. + { table: "variant_assignments", column: null, kind: "table", ticket: "GIT-106", reason: "dev tier only; table not in setup.sql" }, + { table: "variant_performance_metrics", column: null, kind: "table", ticket: "GIT-106", reason: "dev tier only; table not in setup.sql" }, + { table: "scar_enforcement_variants", column: "active", kind: "filter", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, + { table: "scar_enforcement_variants", column: "active", kind: "write", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, + { table: "scar_enforcement_variants", column: "description", kind: "write", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, + { table: "scar_enforcement_variants", column: "enforcement_config", kind: "write", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, + { table: "scar_enforcement_variants", column: "variant_name", kind: "write", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, + { table: "scar_enforcement_variants", column: "variant_version", kind: "write", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, +]; + +/** + * Call sites the analyzer cannot resolve statically, by file and what is + * missing. Each is checked another way or is out of reach; a NEW one fails. + */ +const KNOWN_UNRESOLVED: Array<{ file: string; what: RegExp; why: string }> = [ + { file: "src/services/thread-supabase.ts", what: /^filters: /, why: "filters come from buildScopedThreadQuery() in thread-scope.ts (another file); thread-scope tests pin them" }, + { file: "src/services/write-health.ts", what: /^table /, why: "loops over getTableName(\"learnings\" | \"decisions\"), select id only" }, + { file: "src/tools/session-close.ts", what: /sessionData not resolvable; filtered by filterToStoreSessionColumns/, why: "narrowed to SESSION_COLUMNS, checked against v1.8.0 below" }, +]; + +let floor: Schema; +let current: Schema; +let scan: ScanResult; +let findings: Finding[]; + +beforeAll(() => { + floor = parseSchema(fs.readFileSync(FLOOR_SQL, "utf-8")); + current = parseSchema(fs.readFileSync(CURRENT_SQL, "utf-8")); + scan = scanSources(path.join(REPO, "src")); + findings = compare(scan, floor, current); +}); + +const matches = (a: Allowed, f: Finding) => a.table === f.table && a.column === f.column && a.kind === f.kind; +const label = (f: Finding) => `${f.table}.${f.column ?? "(table)"} [${f.kind}]${f.addedAfterFloor ? " (added after 1.8.0)" : " (in no setup.sql)"} at ${f.sites.join(", ")}`; + +describe("schema parser", () => { + it("reads the v1.8.0 tables, ALTER-added columns and views", () => { + expect([...floor.keys()]).toEqual(expect.arrayContaining([ + "gitmem_learnings", "gitmem_sessions", "gitmem_decisions", "gitmem_scar_usage", "gitmem_threads", + "knowledge_triples", "gitmem_query_metrics", "scar_enforcement_variants", "gitmem_threads_lite", "gitmem_sessions_lite", + ])); + expect(floor.get("gitmem_learnings")).toContain("decay_multiplier"); // ALTER TABLE ... ADD COLUMN + expect(floor.get("gitmem_threads")).toContain("resolved_by_session"); + expect(floor.get("gitmem_threads")).not.toContain("CHECK"); + expect(floor.get("gitmem_sessions_lite")).toContain("close_compliance"); // view + expect(floor.get("gitmem_sessions_lite")).not.toContain("embedding"); + }); + + it("the pinned floor is the v1.8.0 schema, not a later one", () => { + expect(fs.readFileSync(FLOOR_SQL, "utf-8")).toMatch(/refresh_scar_behavioral_scores/); + // The release-B setup.sql re-creates the usage table idempotently (GIT-84); v1.8.0 did not. + expect(fs.readFileSync(FLOOR_SQL, "utf-8")).not.toBe(fs.readFileSync(CURRENT_SQL, "utf-8")); + }); + + it("parses PostgREST select lists", () => { + expect(selectColumns("id,title:t,embedding::text,metadata->>session_id,rel(a,b),*")).toEqual({ + columns: ["id", "t", "embedding", "metadata"], star: true, + }); + }); +}); + +describe("analyzer catches drift (planted in a synthetic source tree)", () => { + let dir: string; + beforeAll(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-drift-")); + fs.mkdirSync(path.join(dir, "src")); + fs.writeFileSync(path.join(dir, "src", "planted.ts"), ` + declare const supabase: any; declare function getTableName(b: string): string; declare function hasSupabase(): boolean; + declare const getStorage: () => any; + export async function a() { + const row = { id: "x", bogus_column: 1, ...(true && { also_bogus: 2 }) }; + row.assigned_later = 3; + await supabase.directUpsert(getTableName("sessions"), row); + await supabase.directQuery("knowledge_triples", { select: "id,no_such_col", filters: { nope: "eq.1" }, order: "gone.desc" }); + await supabase.directPatch(getTableName("threads"), { thread_id: "t" }, { status: "resolved", not_a_col: 1 }); + await supabase.directQuery("missing_table", { select: "id" }); + if (!hasSupabase()) await getStorage().upsert("sessions", { local_only_field: 1 }); + }`); + }); + afterAll(() => fs.rmSync(dir, { recursive: true, force: true })); + + it("reports every planted column and table, and ignores free-tier-only writes", () => { + const s = scanSources(path.join(dir, "src")); + const got = compare(s, floor, current).map((f) => `${f.table}.${f.column ?? "(table)"}:${f.kind}`).sort(); + expect(got).toEqual([ + "gitmem_sessions.also_bogus:write", + "gitmem_sessions.assigned_later:write", + "gitmem_sessions.bogus_column:write", + "gitmem_threads.not_a_col:write", + "knowledge_triples.gone:order", + "knowledge_triples.no_such_col:select", + "knowledge_triples.nope:filter", + "missing_table.(table):table", + ]); + expect(s.freeTierOnly).toBe(1); + }); +}); + +describe("src/ against the v1.8.0 schema", () => { + it("scans a meaningful surface", () => { + expect(scan.callSites).toBeGreaterThan(50); + expect(scan.accesses.length).toBeGreaterThan(300); + }); + + it("every table and column is in v1.8.0 or allowlisted by ticket", () => { + const unexpected = findings.filter((f) => !ALLOWED.some((a) => matches(a, f))); + expect(unexpected.map(label), "not in v1.8.0 setup.sql and not allowlisted").toEqual([]); + }); + + it("every allowlist entry still matches a finding, and names a ticket", () => { + for (const a of ALLOWED) { + expect(a.ticket, `${a.table}.${a.column}`).toMatch(/^GIT-\d+$/); + expect(findings.some((f) => matches(a, f)), `stale ALLOWED entry ${a.table}.${a.column} [${a.kind}] (${a.ticket}) — remove it`).toBe(true); + } + }); + + it("no call site is unresolved except the known ones", () => { + const unknown = scan.unresolved.filter((u) => !KNOWN_UNRESOLVED.some((k) => k.file === u.file && k.what.test(u.what))); + expect(unknown.map((u) => `${u.file}:${u.line} ${u.call} — ${u.what}`)).toEqual([]); + for (const k of KNOWN_UNRESOLVED) { + expect(scan.unresolved.some((u) => u.file === k.file && k.what.test(u.what)), `stale KNOWN_UNRESOLVED ${k.file}`).toBe(true); + } + }); + + it("session writes: SESSION_COLUMNS is v1.8.0 plus the probe-guarded production-only set", () => { + const sessions = floor.get("gitmem_sessions")!; + const missing = [...SESSION_COLUMNS].filter((c) => !sessions.has(c) && !PRODUCTION_ONLY_SESSION_COLUMNS.has(c)); + expect(missing, "SESSION_COLUMNS entries a v1.8.0 store rejects").toEqual([]); + }); +}); From c18e82acf34da48a67ffb20458286e8ec47b729c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 21:03:05 -0700 Subject: [PATCH 20/24] fix: hook pid check needs no ps (GIT-99) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve-root.sh decided liveness with `ps -p`. On an image without ps (e.g. node:*-slim, the natural jq-less CI runner) that call fails, so every registry entry looked dead and the Stop hook never blocked — the GIT-112 failure again, through a different missing tool. gitmem_pid_alive uses the kill -0 builtin, with EPERM counted as alive. +2 hook tests (46/46): the check with no ps on PATH, and an EPERM pid. Co-Authored-By: Claude Opus 5 --- hooks/scripts/resolve-root.sh | 16 +++++++++++++--- hooks/tests/test-hooks.sh | 10 +++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/hooks/scripts/resolve-root.sh b/hooks/scripts/resolve-root.sh index f774148..669a461 100755 --- a/hooks/scripts/resolve-root.sh +++ b/hooks/scripts/resolve-root.sh @@ -22,6 +22,7 @@ # Provides: # gitmem_live_session_ids registry session ids, newest first, skipping # entries on this host whose pid is dead +# gitmem_pid_alive whether a pid is running (no `ps` needed) # gitmem_json_escape escape a string for a JSON string literal # ============================================================================ @@ -42,10 +43,19 @@ unset _gitmem_root GITMEM_ACTIVE_SESSIONS="$GITMEM_ROOT/active-sessions.json" GITMEM_PAYLOAD_PATH="$GITMEM_ROOT/closing-payload.json" +# Is this pid a running process? `kill -0` needs no external tool (minimal +# images have no `ps`, and a missing `ps` would make every session look dead). +# EPERM ("Operation not permitted") means it exists but belongs to another user. +gitmem_pid_alive() { + local out + out=$(kill -0 "$1" 2>&1) && return 0 + case "$out" in *ermitted*) return 0 ;; esac + return 1 +} + # A registry entry whose server died is not a session anyone can be in. Only # entries on THIS host can be checked; entries without a pid, or from another -# host, are kept. `ps -p` rather than `kill -0`: kill fails with EPERM for a -# live process of another user, which is still alive. +# host, are kept. gitmem_live_session_ids() { [ -f "$GITMEM_ACTIVE_SESSIONS" ] || return 0 local host rows @@ -69,7 +79,7 @@ gitmem_live_session_ids() { # empty pid would shift the hostname into its place. while IFS='|' read -r sid pid h; do [ -n "$sid" ] || continue - if [ -n "$pid" ] && [ "$h" = "$host" ] && ! ps -p "$pid" >/dev/null 2>&1; then + if [ -n "$pid" ] && [ "$h" = "$host" ] && ! gitmem_pid_alive "$pid"; then continue fi echo "$sid" diff --git a/hooks/tests/test-hooks.sh b/hooks/tests/test-hooks.sh index 347f4af..95d5df8 100755 --- a/hooks/tests/test-hooks.sh +++ b/hooks/tests/test-hooks.sh @@ -687,7 +687,7 @@ echo "$OUTPUT" | node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()= # Registry entries whose pid is dead on this host are skipped. DEAD_PID=99999999 -while ps -p "$DEAD_PID" >/dev/null 2>&1; do DEAD_PID=$((DEAD_PID - 1)); done +while kill -0 "$DEAD_PID" 2>/dev/null; do DEAD_PID=$((DEAD_PID - 1)); done HOST=$(hostname) setup_state 10 60 echo "{\"sessions\":[{\"session_id\":\"dead-one\",\"pid\":$DEAD_PID,\"hostname\":\"$HOST\",\"started_at\":\"2026-09-21T00:00:00Z\"}]}" > "$GITMEM_DIR/active-sessions.json" @@ -716,6 +716,14 @@ OUTPUT=$(echo '{"tool_name":"Bash","tool_input":{"command":"git push origin main echo "$OUTPUT" | grep -q "block" && pass "recall-check reads the live session's scars (dead newer entry skipped)" \ || fail "recall-check live session" "block on unconfirmed scar of live-old" "$OUTPUT" +# The pid check needs no `ps` (minimal images have none). +GOT=$(PATH="$TMPDIR/no-ps-bin" "$BASH" -c '. "$0/scripts/resolve-root.sh"; gitmem_pid_alive $$ && echo alive; gitmem_pid_alive '"$DEAD_PID"' || echo dead' "$SCRIPT_DIR" 2>/dev/null | tr "\n" " ") +[ "$GOT" = "alive dead " ] && pass "gitmem_pid_alive works with no ps on PATH" \ + || fail "pid check without ps" "alive dead" "$GOT" +GOT=$(bash -c '. "$0/scripts/resolve-root.sh"; gitmem_pid_alive 1 && echo alive' "$SCRIPT_DIR") +[ "$GOT" = "alive" ] && pass "a live pid owned by another user (EPERM) counts as alive" \ + || fail "EPERM pid alive" "alive" "$GOT" + remove_session_registry # ============================================================================ From 28b3e9d609307bf86cef484643e2b6d9e8d60a08 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 21:06:08 -0700 Subject: [PATCH 21/24] =?UTF-8?q?ci:=20release=20gate=20=E2=80=94=20venue?= =?UTF-8?q?=20driver=20on=20the=20v1.8.0=20then=20current=20schema,=20hook?= =?UTF-8?q?=20tests=20without=20jq=20(GIT-83,=20GIT-112)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - venue-gate (v* tags and workflow_dispatch): puts the disposable venue on `git show v1.8.0:schema/setup.sql`, runs the driver (flow, egress --usage 5, projects when the driver has it), applies the current setup.sql twice on top, and runs flow again. Missing secrets fail the job; a concurrency group keeps two runs from wiping each other's venue data. Driver reports are uploaded (netlogs hold method/path/status/size only). - scripts/venue-schema.sh: reset-to / apply via psql, one transaction, ON_ERROR_STOP, PostgREST schema reload. Refuses production (by ref and by URL) and any DB URL that does not name VENUE_REF, before connecting. - tests/e2e/venue/reset-gitmem-objects.sql: drops only what setup.sql creates. - hooks-without-jq (every run): hooks/tests/test-hooks.sh in node:20-bookworm-slim, asserting neither jq nor ps is installed (GIT-112). - publish needs build, hooks-without-jq and venue-gate. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 114 ++++++++++++++++++++++- scripts/venue-schema.sh | 48 ++++++++++ tests/e2e/venue/reset-gitmem-objects.sql | 21 +++++ 3 files changed, 182 insertions(+), 1 deletion(-) create mode 100755 scripts/venue-schema.sh create mode 100644 tests/e2e/venue/reset-gitmem-objects.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f672779..a92f871 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,9 @@ on: tags: ['v*'] pull_request: branches: [main] + # GIT-83: run the venue gate by hand (Actions → CI → Run workflow) before + # tagging. Publishing still happens only on a v* tag. + workflow_dispatch: jobs: build: @@ -51,8 +54,117 @@ jobs: - name: Dependency audit (no unused deps) run: npx depcheck --ignores="@types/*" --json | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8')); const unused=d.dependencies||[]; if(unused.length){console.error('Unused dependencies:',unused);process.exit(1)}" + # GIT-83 / GIT-112: the hook suite on a runner WITHOUT jq (or ps). The hooks + # fall back to node and bash builtins there; on 1.10.0 that fallback left + # Stop enforcement inactive and nothing noticed, because every runner and + # dev machine the suite had run on had jq. + hooks-without-jq: + runs-on: ubuntu-latest + container: node:20-bookworm-slim + steps: + - uses: actions/checkout@v4 + + - name: Runner has no jq and no ps + run: | + if command -v jq >/dev/null; then echo "::error::jq is installed; this job must run without it"; exit 1; fi + if command -v ps >/dev/null; then echo "::error::ps is installed; this job must run without it"; exit 1; fi + bash --version | head -1; node --version + + - name: Hook tests + run: bash hooks/tests/test-hooks.sh + + # GIT-83: the release gate. Customers never run SQL after setup (standing + # rule, 2026-09-21), so a release must work on a store still on the v1.8.0 + # setup.sql — the venue is put on exactly that schema first, then the + # current one. Runs on v* tags (publish depends on it) and on demand. + # + # Needs repository secrets, all for the DISPOSABLE venue (never production; + # scripts/venue-schema.sh and the driver both refuse it): + # VENUE_REF project ref + # VENUE_SUPABASE_URL https://.supabase.co + # VENUE_SERVICE_ROLE_KEY service role key + # VENUE_DB_URL postgres:// URL (session pooler) for schema resets + venue-gate: + if: startsWith(github.ref, 'refs/tags/v') || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + timeout-minutes: 30 + # One venue: two runs would wipe each other's data. + concurrency: + group: gitmem-venue + cancel-in-progress: false + env: + VENUE_REF: ${{ secrets.VENUE_REF }} + VENUE_DB_URL: ${{ secrets.VENUE_DB_URL }} + VENUE_SUPABASE_URL: ${{ secrets.VENUE_SUPABASE_URL }} + VENUE_SERVICE_ROLE_KEY: ${{ secrets.VENUE_SERVICE_ROLE_KEY }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 # the v1.8.0 tag supplies the floor schema + + - uses: actions/setup-node@v4 + with: + node-version: 20 + cache: npm + + - run: npm ci --legacy-peer-deps + - run: npm run build + + # Missing secrets fail the gate — a skipped gate would let publish through. + - name: Venue configured + run: | + missing="" + for v in VENUE_REF VENUE_DB_URL VENUE_SUPABASE_URL VENUE_SERVICE_ROLE_KEY; do + [ -n "${!v}" ] || missing="$missing $v" + done + if [ -n "$missing" ]; then echo "::error::venue gate cannot run, missing secrets:$missing"; exit 1; fi + umask 077 + printf 'SUPABASE_URL=%s\nSUPABASE_SERVICE_ROLE_KEY=%s\nVENUE_REF=%s\n' \ + "$VENUE_SUPABASE_URL" "$VENUE_SERVICE_ROLE_KEY" "$VENUE_REF" > "$RUNNER_TEMP/venue.env" + echo "VENUE_ENV=$RUNNER_TEMP/venue.env" >> "$GITHUB_ENV" + echo "RUNS=$RUNNER_TEMP/venue-runs" >> "$GITHUB_ENV" + mkdir -p "$RUNNER_TEMP/venue-runs" + git show v1.8.0:schema/setup.sql > "$RUNNER_TEMP/setup-v1.8.0.sql" + + - name: Venue on the v1.8.0 setup.sql + run: scripts/venue-schema.sh reset-to "$RUNNER_TEMP/setup-v1.8.0.sql" + + - name: Driver on v1.8.0 — flow + run: node tests/e2e/blank-supabase.mjs flow --label v180-flow --out "$RUNS" + + - name: Driver on v1.8.0 — egress + run: node tests/e2e/blank-supabase.mjs egress --label v180-egress --out "$RUNS" --rows 250 --usage 5 + + - name: Driver on v1.8.0 — projects (when the driver has it) + run: | + # The usage line lists the modes; a bare "projects" string also appears in a path. + if grep -q 'flow|egress|projects' tests/e2e/blank-supabase.mjs; then + node tests/e2e/blank-supabase.mjs projects --label v180-projects --out "$RUNS" + else + echo "driver has no projects mode in this revision" + fi + + # The current setup.sql must apply on top of v1.8.0, and apply again. + - name: Venue on the current setup.sql (applied twice) + run: | + scripts/venue-schema.sh apply schema/setup.sql + scripts/venue-schema.sh apply schema/setup.sql + + - name: Driver on the current schema — flow + run: node tests/e2e/blank-supabase.mjs flow --label current-flow --out "$RUNS" + + - name: Driver reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: venue-gate-${{ github.run_id }} + # netlogs record method, path, status and body size only — no bodies or headers. + path: ${{ runner.temp }}/venue-runs + if-no-files-found: ignore + retention-days: 14 + publish: - needs: build + needs: [build, hooks-without-jq, venue-gate] runs-on: ubuntu-latest if: startsWith(github.ref, 'refs/tags/v') permissions: diff --git a/scripts/venue-schema.sh b/scripts/venue-schema.sh new file mode 100755 index 0000000..9e35ca5 --- /dev/null +++ b/scripts/venue-schema.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash +# GIT-83: put the disposable venue on a given schema, for the release gate. +# +# scripts/venue-schema.sh reset-to drop gitmem objects, then apply +# scripts/venue-schema.sh apply apply on top (re-applicability) +# add --dry-run to print what would run and exit +# +# Needs VENUE_REF and VENUE_DB_URL (a postgres:// URL for the venue only). +# Refuses production, and any URL that does not name VENUE_REF, before +# connecting. Uses psql with ON_ERROR_STOP, in one transaction per call. +set -euo pipefail + +DENY_REFS="cjptxyezuxdiinufgrrm" # production GitMem — never touch +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +RESET_SQL="$HERE/../tests/e2e/venue/reset-gitmem-objects.sql" + +die() { echo "[venue-schema] $*" >&2; exit 2; } + +mode="${1:-}"; sql="${2:-}"; dry=false +[ "${3:-}" = "--dry-run" ] && dry=true +case "$mode" in reset-to|apply) ;; *) die "usage: venue-schema.sh reset-to|apply [--dry-run]" ;; esac +[ -n "$sql" ] && [ -f "$sql" ] || die "schema file not found: ${sql:-}" +[ -n "${VENUE_REF:-}" ] || die "VENUE_REF is not set" +[ -n "${VENUE_DB_URL:-}" ] || die "VENUE_DB_URL is not set" +for d in $DENY_REFS; do + [ "$VENUE_REF" != "$d" ] || die "refusing: VENUE_REF is production ($d)" + case "$VENUE_DB_URL" in *"$d"*) die "refusing: VENUE_DB_URL names production ($d)" ;; esac +done +case "$VENUE_DB_URL" in + postgres://*|postgresql://*) ;; + *) die "refusing: VENUE_DB_URL is not a postgres:// URL" ;; +esac +case "$VENUE_DB_URL" in *"$VENUE_REF"*) ;; *) die "refusing: VENUE_DB_URL does not name the venue ($VENUE_REF)" ;; esac + +files=() +[ "$mode" = "reset-to" ] && files+=("$RESET_SQL") +files+=("$sql") +if $dry; then + echo "[venue-schema] would apply to venue $VENUE_REF, one transaction: ${files[*]}" + exit 0 +fi +command -v psql >/dev/null || die "psql not found" +args=() +for f in "${files[@]}"; do args+=(-f "$f"); done +# NOTIFY is delivered at commit: PostgREST reloads its schema cache before the driver runs. +PGCONNECT_TIMEOUT=20 psql "$VENUE_DB_URL" -X -q -v ON_ERROR_STOP=1 --single-transaction "${args[@]}" \ + -c "NOTIFY pgrst, 'reload schema'" +echo "[venue-schema] venue $VENUE_REF: $mode $(basename "$sql") OK" diff --git a/tests/e2e/venue/reset-gitmem-objects.sql b/tests/e2e/venue/reset-gitmem-objects.sql new file mode 100644 index 0000000..b1aed51 --- /dev/null +++ b/tests/e2e/venue/reset-gitmem-objects.sql @@ -0,0 +1,21 @@ +-- GIT-83: return the DISPOSABLE venue to a blank slate so a setup.sql lands +-- exactly as it would on a new customer project. Drops only the objects +-- schema/setup.sql creates. Run only through scripts/venue-schema.sh, which +-- refuses production and any database that is not the configured venue. +DROP TABLE IF EXISTS + gitmem_license_activations, gitmem_licenses, + gitmem_scar_usage, gitmem_decisions, gitmem_threads, knowledge_triples, + gitmem_query_metrics, scar_enforcement_variants, + gitmem_learnings, gitmem_sessions + CASCADE; -- also removes the *_lite views, triggers and policies +DO $$ +DECLARE r record; +BEGIN + FOR r IN SELECT p.oid::regprocedure AS sig FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE n.nspname = 'public' AND p.proname IN + ('gitmem_semantic_search','gitmem_scar_search','refresh_scar_behavioral_scores', + 'gitmem_update_timestamp','gitmem_validate_license','gitmem_deactivate_device') + LOOP EXECUTE 'DROP FUNCTION ' || r.sig || ' CASCADE'; END LOOP; +END $$; +-- PostgREST must see the new shape before the driver's first request. +NOTIFY pgrst, 'reload schema'; From abb772acc68db7223cc7a9f7ff2dbc213168d35e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 21:12:04 -0700 Subject: [PATCH 22/24] test: session-close mocks export findSessionById (release/C merge of #37) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GIT-86 (#37) made session_close's registry fallback look the session up by id. Four mocks of active-sessions.js exported only findSessionByHostPid — session-close-validation (named in the stack plan), session-close-payload-path (#39, whose mock header was copied from it), session-close-supabase and session-close-bridge. No test reached that fallback, so nothing failed; any future one would have thrown on the missing export. Co-Authored-By: Claude Opus 5 --- tests/unit/tools/session-close-bridge.test.ts | 1 + tests/unit/tools/session-close-payload-path.test.ts | 1 + tests/unit/tools/session-close-supabase.test.ts | 1 + tests/unit/tools/session-close-validation.test.ts | 1 + 4 files changed, 4 insertions(+) diff --git a/tests/unit/tools/session-close-bridge.test.ts b/tests/unit/tools/session-close-bridge.test.ts index 2e84f65..6224f8d 100644 --- a/tests/unit/tools/session-close-bridge.test.ts +++ b/tests/unit/tools/session-close-bridge.test.ts @@ -165,6 +165,7 @@ vi.mock("../../../src/services/gitmem-dir.js", () => ({ vi.mock("../../../src/services/active-sessions.js", () => ({ unregisterSession: () => {}, findSessionByHostPid: () => null, + findSessionById: () => null, // GIT-86: session-close's registry fallback looks up by id })); vi.mock("../../../src/services/thread-suggestions.js", () => ({ diff --git a/tests/unit/tools/session-close-payload-path.test.ts b/tests/unit/tools/session-close-payload-path.test.ts index 4fe12fd..0f89d50 100644 --- a/tests/unit/tools/session-close-payload-path.test.ts +++ b/tests/unit/tools/session-close-payload-path.test.ts @@ -136,6 +136,7 @@ vi.mock("../../../src/services/gitmem-dir.js", () => ({ vi.mock("../../../src/services/active-sessions.js", () => ({ unregisterSession: vi.fn(), findSessionByHostPid: vi.fn().mockReturnValue(null), + findSessionById: vi.fn().mockReturnValue(null), // GIT-86: session-close's registry fallback looks up by id })); vi.mock("../../../src/services/thread-suggestions.js", () => ({ diff --git a/tests/unit/tools/session-close-supabase.test.ts b/tests/unit/tools/session-close-supabase.test.ts index 70f0998..1e40800 100644 --- a/tests/unit/tools/session-close-supabase.test.ts +++ b/tests/unit/tools/session-close-supabase.test.ts @@ -158,6 +158,7 @@ vi.mock("../../../src/services/gitmem-dir.js", () => ({ vi.mock("../../../src/services/active-sessions.js", () => ({ unregisterSession: () => {}, findSessionByHostPid: () => null, + findSessionById: () => null, // GIT-86: session-close's registry fallback looks up by id })); vi.mock("../../../src/services/thread-suggestions.js", () => ({ diff --git a/tests/unit/tools/session-close-validation.test.ts b/tests/unit/tools/session-close-validation.test.ts index 98fa88e..00952b7 100644 --- a/tests/unit/tools/session-close-validation.test.ts +++ b/tests/unit/tools/session-close-validation.test.ts @@ -125,6 +125,7 @@ vi.mock("../../../src/services/gitmem-dir.js", () => ({ vi.mock("../../../src/services/active-sessions.js", () => ({ unregisterSession: vi.fn(), findSessionByHostPid: vi.fn().mockReturnValue(null), + findSessionById: vi.fn().mockReturnValue(null), // GIT-86: session-close's registry fallback looks up by id })); vi.mock("../../../src/services/thread-suggestions.js", () => ({ From f3fdae33330874ed99182674767bc5a196d18687 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 21:17:41 -0700 Subject: [PATCH 23/24] docs: CHANGELOG [Unreleased] for Release C Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71ee809..c23e303 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,62 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +**Just upgrade the package. Nothing to change on your database.** Every fix in this release works on +a project still on the `setup.sql` you first ran, back to 1.8.0. Each one was checked against a +project on exactly that schema. + +**A chat in one project no longer continues another project's session.** The desktop app runs one +gitmem process for every chat. Starting a session for project Y while a project X chat was open used +to resume X's session, swapping the project you asked for for X's and saying so only on stderr. +`session_start(project: Y)` now starts a Y session and leaves X open and resumable. Called with no +project, it resumes the most recent session and names that session's project in its output. +`force: true` carries activity forward only from a session of the same project. (GIT-86) + +**Every write now says whether it was actually saved.** `create_learning`, `create_decision`, +`create_thread`, `resolve_thread`, `archive_learning`, `record_scar_usage` (single and batch), +`session_close`, `promote_suggestion` and `save_transcript` now report `durable` and `stored_in` +alongside `success`. `success` means the record reached your database. Before this, +`resolve_thread` reported success even when your database never recorded the resolve, so other +sessions still saw the thread as open. Now it says `RESOLVED LOCALLY ONLY — not durable`. (GIT-101) + +**Stop enforcement was inactive on machines without jq; fixed.** (GIT-112) + +### Fixed + +- **Sessions start without re-downloading the whole index.** On Pro, nearly every `session_start` + changes a few scar scores, and the next start used to download every learning with its embedding + again. Now only the changed rows are fetched: in testing, 42 KB instead of 922 KB for 250 + learnings. (GIT-98) +- **The hooks read the same store as the server.** They looked for `.gitmem` in the current directory + instead of `GITMEM_DIR`, then `GITMEM_HOME`, then `~/.gitmem`, so they could see no session or + someone else's. They now skip sessions whose server process has exited, and the Stop hook prints + the exact path of `closing-payload.json`. (GIT-99) +- **`session_close` says where it looked for your payload.** With no `closing-payload.json` and no + inline reflection, it now reports `closing-payload.json not found at ` instead of asking you + to answer the closing questions again. (GIT-99) +- **The startup write check can no longer hang silently.** It gives up after 10 seconds and reports + "timed out, durability UNVERIFIED", and a database that refuses the connection is reported as + unreachable rather than OK. `health` now shows the latest verdict. (GIT-102) +- **`session_close` warnings say what failed.** Each warning names the part that failed and why, and + says whether the session itself was saved. Before, a close showed only `WARN N write failures`. + (GIT-102) +- **No more `metrics` failure at `session_start`.** A metrics row now waits for its session row to + exist. (GIT-73, listed under *Known issues* in 1.10.0) +- **Knowledge-graph links for threads are saved.** They wrote a thread id where the database expects a + UUID, so every one was rejected. (GIT-105, listed under *Known issues* in 1.10.0) +- **`migrate-root` merges your memory instead of skipping it.** If your home store already had + learnings, threads, decisions or sessions, the project store's were skipped as "already exists". They + are now merged record by record: the newer version wins and the other is written to + `migrate-root-conflicts.json`, after a backup. Running it again changes nothing. (GIT-100) +- **No false "Memory store found but NOT being read" on macOS.** Paths under `/var` and + `/private/var` (and any symlink) were compared as text, so the store gitmem was reading could be + reported as unread. (GIT-107) + +### Changed + +- **Releases are gated on a database still on 1.8.0's `setup.sql`, and on the hooks passing + without jq.** Nothing is published unless both pass. (GIT-83, GIT-109) + ## [1.10.0] - 2026-09-21 **Sessions that weren't being saved now save.** On your own Supabase project, a `session_close` was From 7a3ac5c55fd302737494455c88e276efed423849 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 21:36:38 -0700 Subject: [PATCH 24/24] chore: bump version to v1.11.0 CHANGELOG [Unreleased] -> [1.11.0] - 2026-09-21, fresh [Unreleased] above. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 2 ++ package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c23e303..96a05f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.11.0] - 2026-09-21 + **Just upgrade the package. Nothing to change on your database.** Every fix in this release works on a project still on the `setup.sql` you first ran, back to 1.8.0. Each one was checked against a project on exactly that schema. diff --git a/package-lock.json b/package-lock.json index 9b91cac..31aee33 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "gitmem-mcp", - "version": "1.10.0", + "version": "1.11.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "gitmem-mcp", - "version": "1.10.0", + "version": "1.11.0", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.0.0", diff --git a/package.json b/package.json index 3e0955f..70b18cf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gitmem-mcp", - "version": "1.10.0", + "version": "1.11.0", "mcpName": "io.github.gitmem-dev/gitmem", "description": "Persistent learning memory for AI coding agents. Memory that compounds.", "type": "module",