From 4a6ad098afab3135ba2e5c9dd6da69d3cad01435 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:12:59 -0700 Subject: [PATCH 01/17] fix: remote recall calls setup.sql's gitmem_scar_search (GIT-114) When the local vector index isn't ready (every server start until warm-up, a new or empty store, a failed load, GITMEM_SEARCH_MODE=remote), recall called match_gitmem_learnings_weighted. No setup.sql defines it: GIT-93 took the name from nTEG's store. Every customer's cold recall got PGRST202 and returned nothing. - Default prefix: call gitmem_scar_search with similarity_threshold. If the store answers PGRST202 (no such function, e.g. nTEG's), fall back to match_gitmem_learnings_weighted with match_threshold. The first that exists is remembered for the process; any other error is not retried. - Non-default prefix: only match__weighted. gitmem_scar_search reads gitmem_learnings by name and would search the wrong table. - gitmem_scar_search doesn't return learning_type, applies_when, why_this_matters and the other fields recall renders. They are fetched for the matched ids in one select; if that fails, the rows are returned without them. - Remove the dead semanticSearch (match_gitmem_learnings, no callers). scar-search-rpc-name.test.ts: "tracks the prefix" asserted match_gitmem_learnings_weighted for the default prefix, the bug itself; it now asserts gitmem_scar_search. The semanticSearch test goes with the function. +4 tests: PGRST202 fallback, no retry on a real error, the follow-up select, and tolerating its failure. Co-Authored-By: Claude Opus 5.5 --- src/services/supabase-client.ts | 191 +++++++++--------- .../services/scar-search-rpc-name.test.ts | 92 ++++++++- 2 files changed, 180 insertions(+), 103 deletions(-) diff --git a/src/services/supabase-client.ts b/src/services/supabase-client.ts index ca1855a..d56b8cd 100644 --- a/src/services/supabase-client.ts +++ b/src/services/supabase-client.ts @@ -14,7 +14,7 @@ import type { SupabaseSearchOptions, } from "../types/index.js"; import { getCache } from "./cache.js"; -import { getTableName } from "./tier.js"; +import { getTableName, getTablePrefix } from "./tier.js"; // --- PostgREST Input Sanitization --- @@ -159,61 +159,6 @@ export async function upsertRecord( return directUpsert(table, data); } -/** - * Semantic search across tables - * - * Generates an embedding for the query, then calls the match_
- * RPC function directly via PostgREST. - */ -export async function semanticSearch( - options: SupabaseSearchOptions -): Promise { - if (!isConfigured()) { - throw new Error("Supabase not configured"); - } - - const { query, match_count = 10 } = options; - - // Generate embedding for the query - const { embed } = await import("./embedding.js"); - const embedding = await embed(query); - - if (!embedding) { - console.error("[semantic-search] No embedding provider configured — cannot run semantic search"); - return []; - } - - // GIT-93: same defect as scarSearch below — this built `${prefix}_semantic_search`, - // which exists under no prefix. The deployed function is match_
, and it - // takes similarity_threshold (unlike the _weighted variant), which is what the - // body already sends. - const rpcName = `match_${getTableName("learnings")}`; - const url = `${SUPABASE_URL}/rest/v1/rpc/${rpcName}`; - - const response = await fetch(url, { - method: "POST", - headers: { - "Content-Type": "application/json", - apikey: SUPABASE_KEY, - Authorization: `Bearer ${SUPABASE_KEY}`, - }, - body: JSON.stringify({ - query_embedding: `[${embedding.join(",")}]`, - match_count, - similarity_threshold: 0.0, - }), - signal: AbortSignal.timeout(15_000), - }); - - if (!response.ok) { - const text = await response.text(); - throw new Error(`Supabase RPC error: ${response.status} - ${text.slice(0, 200)}`); - } - - const rows = (await response.json()) as T[]; - return rows || []; -} - // ============================================================================ // DIRECT SUPABASE QUERIES // ============================================================================ @@ -677,49 +622,107 @@ export async function scarSearch( return []; } - // GIT-93: the RPC is named after the TABLE it searches, not after the prefix - // with a verb appended. This built `${prefix}_scar_search` — "orchestra_scar_search" - // under GITMEM_TABLE_PREFIX=orchestra_, and "gitmem_scar_search" by default. - // Neither exists: PostgREST exposes match_orchestra_learnings_weighted, and a - // survey of the deployed functions found no *_scar_search under any prefix. So - // this fallback returned PGRST202 on every call, on every deployment, since it - // was written — invisible because it is only reached while the local vector - // index is cold. - // - // _weighted is the right one of the pair: this function is documented as scar - // search WITH SEVERITY WEIGHTING, and it is the variant that returns - // decay_multiplier, which recall consumes. Note it takes match_threshold, not - // similarity_threshold — the unweighted variant takes the latter. - const rpcName = `match_${getTableName("learnings")}_weighted`; - const url = `${SUPABASE_URL}/rest/v1/rpc/${rpcName}`; + const rows = await callScarSearchRpc(embedding, matchCount); + return (await withLearningColumns(rows)) as T[]; +} - const response = await fetch(url, { - method: "POST", - headers: { - "Content-Type": "application/json", - apikey: SUPABASE_KEY, - Authorization: `Bearer ${SUPABASE_KEY}`, - }, - body: JSON.stringify({ - query_embedding: `[${embedding.join(",")}]`, - match_count: matchCount, - match_threshold: 0.0, - // project_filter is deliberately not sent. The primary path this falls back - // from is the unified CROSS-PROJECT vector cache, so filtering here would - // make the fallback return a different, narrower result set than the path - // it stands in for — a silent behaviour change on exactly the cold-start - // calls that are hardest to notice. - }), - signal: AbortSignal.timeout(15_000), - }); +/** + * GIT-114: which scar-search function this store has. + * + * setup.sql (every version a customer can have) defines gitmem_scar_search, + * taking similarity_threshold. It does NOT define match_
_weighted — + * GIT-93 renamed the call to that after surveying nTEG's store, which has it, + * so every customer's cold-index recall got PGRST202. + * + * Order of preference: + * - default prefix: gitmem_scar_search (setup.sql), then + * match_gitmem_learnings_weighted for a store that predates or lacks it; + * - any other prefix: only match_
_weighted — gitmem_scar_search reads + * gitmem_learnings by name and would search the wrong table. + * The first function that exists is remembered for the process. + */ +interface ScarSearchRpc { + name: string; + thresholdParam: "similarity_threshold" | "match_threshold"; +} + +let resolvedScarSearchRpc: ScarSearchRpc | null = null; + +/** For tests. */ +export function resetScarSearchRpc(): void { + resolvedScarSearchRpc = null; +} + +function scarSearchCandidates(): ScarSearchRpc[] { + const weighted: ScarSearchRpc = { name: `match_${getTableName("learnings")}_weighted`, thresholdParam: "match_threshold" }; + return getTablePrefix() === "gitmem_" + ? [{ name: "gitmem_scar_search", thresholdParam: "similarity_threshold" }, weighted] + : [weighted]; +} + +async function callScarSearchRpc(embedding: number[], matchCount: number): Promise { + const candidates = resolvedScarSearchRpc ? [resolvedScarSearchRpc] : scarSearchCandidates(); + let lastError = ""; + for (const rpc of candidates) { + const response = await fetch(`${SUPABASE_URL}/rest/v1/rpc/${rpc.name}`, { + method: "POST", + headers: { + "Content-Type": "application/json", + apikey: SUPABASE_KEY, + Authorization: `Bearer ${SUPABASE_KEY}`, + }, + body: JSON.stringify({ + query_embedding: `[${embedding.join(",")}]`, + match_count: matchCount, + [rpc.thresholdParam]: 0.0, + // project_filter is deliberately not sent. The primary path this falls + // back from is the unified CROSS-PROJECT vector cache, so filtering here + // would make the fallback return a narrower set than the path it stands + // in for — a silent behaviour change on the cold-start calls. + }), + signal: AbortSignal.timeout(15_000), + }); + + if (response.ok) { + resolvedScarSearchRpc = rpc; + const rows = (await response.json()) as T[]; + return rows || []; + } - if (!response.ok) { const text = await response.text(); - throw new Error(`Supabase RPC error: ${response.status} - ${text.slice(0, 200)}`); + lastError = `Supabase RPC error: ${response.status} - ${text.slice(0, 200)}`; + // PGRST202: no such function (with these parameters) — try the next name. + // Anything else is a real failure of a function that exists. + if (!(response.status === 404 && text.includes("PGRST202"))) break; + console.error(`[scar-search] ${rpc.name} not on this store (PGRST202)`); } + throw new Error(lastError); +} - const rows = (await response.json()) as T[]; - return rows || []; +/** + * gitmem_scar_search returns id, title, description, severity, scar_type, + * counter_arguments, decay_multiplier and similarity — not the fields recall + * renders from (learning_type, applies_when, why_this_matters, …). Fetch them + * for the returned ids in one request. If that request fails the rows are + * still returned: recall renders what it has rather than losing the result. + */ +const RECALL_EXTRA_COLUMNS = + "id,learning_type,problem_context,solution_approach,applies_when,domain,keywords,source_linear_issue,why_this_matters,action_protocol,self_check_criteria,project"; + +async function withLearningColumns(rows: T[]): Promise { + if (rows.length === 0 || rows.every((r) => r.learning_type !== undefined)) return rows; + try { + const extra = await directQuery & { id: string }>(getTableName("learnings"), { + select: RECALL_EXTRA_COLUMNS, + filters: { id: `in.(${rows.map((r) => r.id).join(",")})` }, + }); + const byId = new Map(extra.map((e) => [e.id, e])); + return rows.map((r) => ({ ...(byId.get(r.id) ?? {}), ...r })); + } catch (error) { + console.error("[scar-search] Could not fetch recall columns for the matched scars (returning without them):", + error instanceof Error ? error.message : error); + return rows; + } } /** diff --git a/tests/unit/services/scar-search-rpc-name.test.ts b/tests/unit/services/scar-search-rpc-name.test.ts index 2754cfd..8fe94e5 100644 --- a/tests/unit/services/scar-search-rpc-name.test.ts +++ b/tests/unit/services/scar-search-rpc-name.test.ts @@ -97,22 +97,96 @@ describe("GIT-93: retrieval RPC names are derived from the table, not the prefix expect(calls[0].body).not.toHaveProperty("project_filter"); }); - it("semanticSearch calls match_
", async () => { + // GIT-114: this used to assert match_gitmem_learnings_weighted for the + // default prefix — the name GIT-93 took from nTEG's store. No setup.sql + // defines it, so every customer's cold-index recall got PGRST202. The + // default prefix now calls setup.sql's function first. + it("default prefix: calls setup.sql's gitmem_scar_search with similarity_threshold", async () => { const { calls } = stubFetch(); - const client = await loadClient("orchestra_"); + const client = await loadClient("gitmem_"); - await client.semanticSearch({ query: "any query", match_count: 5 }); + await client.scarSearch("any query", 3); - expect(calls[0].url).toContain("/rest/v1/rpc/match_orchestra_learnings"); - expect(calls[0].url).not.toContain("orchestra_semantic_search"); + expect(calls[0].url).toMatch(/\/rest\/v1\/rpc\/gitmem_scar_search$/); + expect(calls[0].body).toHaveProperty("similarity_threshold", 0); + expect(calls[0].body).not.toHaveProperty("match_threshold"); + expect(calls[0].body).not.toHaveProperty("project_filter"); }); +}); - it("tracks the prefix rather than hardcoding one deployment's table", async () => { - const { calls } = stubFetch(); +// --------------------------------------------------------------------------- +// GIT-114: fallback and missing columns +// --------------------------------------------------------------------------- + +type Reply = { status: number; body: unknown }; +function scriptedFetch(replies: Array<(url: string) => Reply | null>) { + const calls: Array<{ url: string; body: Record }> = []; + vi.stubGlobal("fetch", vi.fn(async (url: string, init?: { body?: string }) => { + calls.push({ url: String(url), body: init?.body ? JSON.parse(init.body) : {} }); + let r: Reply | null = null; + for (const f of replies) { r = f(String(url)); if (r) break; } + r = r ?? { status: 200, body: [] }; + const text = typeof r.body === "string" ? r.body : JSON.stringify(r.body); + return { ok: r.status < 300, status: r.status, json: async () => JSON.parse(text), text: async () => text } as unknown as Response; + })); + return { calls }; +} +const PGRST202 = { status: 404, body: '{"code":"PGRST202","message":"Could not find the function"}' }; +const SCAR = { id: "11111111-2222-4333-8444-555555555555", title: "t", description: "d", severity: "high", similarity: 0.8 }; + +describe("GIT-114: remote scar search on a setup.sql store", () => { + beforeEach(() => { vi.resetModules(); }); + afterEach(() => { + vi.unstubAllGlobals(); + vi.doUnmock("../../../src/services/embedding.js"); + process.env = { ...ORIGINAL_ENV }; + }); + + it("a store without gitmem_scar_search (PGRST202) falls back to match_gitmem_learnings_weighted, and remembers it", async () => { + const { calls } = scriptedFetch([ + (u) => (u.endsWith("/rpc/gitmem_scar_search") ? PGRST202 : null), + (u) => (u.endsWith("/rpc/match_gitmem_learnings_weighted") ? { status: 200, body: [{ ...SCAR, learning_type: "scar" }] } : null), + ]); const client = await loadClient("gitmem_"); - await client.scarSearch("any query", 3); + const rows = await client.scarSearch("q", 3); + expect(rows).toHaveLength(1); + expect(calls.map((c) => c.url.split("/rpc/")[1])).toEqual(["gitmem_scar_search", "match_gitmem_learnings_weighted"]); + expect(calls[1].body).toHaveProperty("match_threshold"); + + calls.length = 0; + await client.scarSearch("q", 3); + expect(calls.map((c) => c.url.split("/rpc/")[1])).toEqual(["match_gitmem_learnings_weighted"]); + }); + + it("a real error from an existing function is not retried under another name", async () => { + const { calls } = scriptedFetch([(u) => (u.includes("/rpc/") ? { status: 500, body: "boom" } : null)]); + const client = await loadClient("gitmem_"); + await expect(client.scarSearch("q", 3)).rejects.toThrow(/500/); + expect(calls).toHaveLength(1); + }); + + it("fetches the columns gitmem_scar_search does not return, by id, in one request", async () => { + const { calls } = scriptedFetch([ + (u) => (u.endsWith("/rpc/gitmem_scar_search") ? { status: 200, body: [SCAR] } : null), + (u) => (u.includes("/rest/v1/gitmem_learnings?") ? { status: 200, body: [{ id: SCAR.id, learning_type: "scar", applies_when: ["deploy"], why_this_matters: "w" }] } : null), + ]); + const client = await loadClient("gitmem_"); + + const [row] = await client.scarSearch>("q", 3); + expect(row).toMatchObject({ ...SCAR, learning_type: "scar", applies_when: ["deploy"], why_this_matters: "w" }); + const follow = new URL(calls[1].url); + expect(follow.searchParams.get("id")).toBe(`in.(${SCAR.id})`); + expect(follow.searchParams.get("select")).toContain("learning_type"); + }); - expect(calls[0].url).toContain("/rest/v1/rpc/match_gitmem_learnings_weighted"); + it("if the follow-up select fails, the matched scars are still returned", async () => { + scriptedFetch([ + (u) => (u.endsWith("/rpc/gitmem_scar_search") ? { status: 200, body: [SCAR] } : null), + (u) => (u.includes("/rest/v1/gitmem_learnings?") ? { status: 503, body: "down" } : null), + ]); + const client = await loadClient("gitmem_"); + const rows = await client.scarSearch>("q", 3); + expect(rows).toEqual([SCAR]); }); }); From 65cbba4093c1dbfdfa24815fdd18b75cb6eea7c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:12:59 -0700 Subject: [PATCH 02/17] test: drift analyzer checks RPC names against v1.8.0's setup.sql (GIT-114) scanRpcNames resolves "/rpc/" in string and template literals (getTableName expanded) and, where the URL is built from a variable, the name properties of the candidate objects in the same file. Fixed https:// URLs (licensing) are classified external. Every store RPC must be defined by setup.sql or allowlisted by ticket; stale entries and unresolved dynamic names fail. Planted-drift self-test included. +5 tests. Co-Authored-By: Claude Opus 5.5 --- tests/helpers/schema-drift.ts | 96 +++++++++++++++++++++++++++++++++ tests/unit/schema-drift.test.ts | 62 ++++++++++++++++++++- 2 files changed, 157 insertions(+), 1 deletion(-) diff --git a/tests/helpers/schema-drift.ts b/tests/helpers/schema-drift.ts index 67b19dc..bfd092f 100644 --- a/tests/helpers/schema-drift.ts +++ b/tests/helpers/schema-drift.ts @@ -527,3 +527,99 @@ export function compare(scan: ScanResult, floor: Schema, current: Schema): Findi 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}`)); } + +// ---------------------------------------------------------------- RPCs (GIT-114) + +/** setup.sql -> function names (CREATE [OR REPLACE] FUNCTION name(...)). */ +export function parseFunctions(sql: string): Set { + const names = new Set(); + const re = /CREATE\s+(?:OR\s+REPLACE\s+)?FUNCTION\s+(?:public\.)?(\w+)\s*\(/gi; + let m: RegExpExecArray | null; + const clean = sql.replace(/--[^\n]*/g, ""); + while ((m = re.exec(clean))) names.add(m[1]); + return names; +} + +export interface RpcRef { + file: string; + line: number; + name: string; + /** A fixed https:// URL — another project (e.g. licensing), not the customer's store. */ + external: boolean; +} + +/** + * Text of a string or template literal, with getTableName("x") substitutions + * expanded to the default table name; any other substitution becomes "\0". + */ +function literalText(n: ts.Node): string | null { + if (ts.isStringLiteralLike(n)) return n.text; + if (ts.isTemplateExpression(n)) { + let out = n.head.text; + for (const span of n.templateSpans) { + const e = span.expression; + out += ts.isCallExpression(e) && ts.isIdentifier(e.expression) && e.expression.text === "getTableName" && + e.arguments[0] && ts.isStringLiteralLike(e.arguments[0]) + ? DEFAULT_PREFIX + e.arguments[0].text + : "\0"; + out += span.literal.text; + } + return out; + } + return null; +} + +/** + * Every RPC name src/ can call. Two shapes: + * - a literal or template URL containing "/rpc/"; + * - in a file that builds "/rpc/${…}" dynamically, every `name:` property of + * an object literal (the candidate list the URL is built from). + * Returns the refs and the dynamic call sites whose names could not be resolved. + */ +export function scanRpcNames(srcDir: string): { refs: RpcRef[]; unresolved: Unresolved[] } { + const refs: RpcRef[] = []; + const unresolved: Unresolved[] = []; + 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); + const sf = ts.createSourceFile(file, fs.readFileSync(file, "utf-8"), ts.ScriptTarget.Latest, true); + const lineOf = (n: ts.Node) => sf.getLineAndCharacterOfPosition(n.getStart()).line + 1; + const dynamicSites: ts.Node[] = []; + const nameProps: ts.PropertyAssignment[] = []; + const visit = (n: ts.Node): void => { + const text = literalText(n); + if (text !== null && text.includes("/rpc/")) { + const seg = text.slice(text.indexOf("/rpc/") + 5).split(/[/?"'\s]/)[0]; + if (seg && !seg.includes("\0")) { + refs.push({ file: rel, line: lineOf(n), name: seg, external: /^https?:\/\//.test(text) }); + } else { + dynamicSites.push(n); + } + } + if (ts.isPropertyAssignment(n) && propName(n) === "name") nameProps.push(n); + ts.forEachChild(n, visit); + }; + visit(sf); + if (dynamicSites.length === 0) continue; + let found = 0; + for (const p of nameProps) { + const t = literalText(p.initializer); + if (t && /^[a-z][a-z0-9_]*$/.test(t)) { + refs.push({ file: rel, line: lineOf(p), name: t, external: false }); + found++; + } + } + if (found === 0) { + for (const d of dynamicSites) unresolved.push({ file: rel, line: lineOf(d), call: "rpc", what: `dynamic RPC name ${d.getText().slice(0, 60)}` }); + } + } + return { refs, unresolved }; +} diff --git a/tests/unit/schema-drift.test.ts b/tests/unit/schema-drift.test.ts index d29936b..83bb05e 100644 --- a/tests/unit/schema-drift.test.ts +++ b/tests/unit/schema-drift.test.ts @@ -21,7 +21,7 @@ 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 { parseSchema, scanSources, compare, selectColumns, parseFunctions, scanRpcNames } 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"; @@ -177,3 +177,63 @@ describe("src/ against the v1.8.0 schema", () => { expect(missing, "SESSION_COLUMNS entries a v1.8.0 store rejects").toEqual([]); }); }); + +// --------------------------------------------------------------------------- +// GIT-114: RPC names +// --------------------------------------------------------------------------- + +/** + * RPCs src/ may call that v1.8.0's setup.sql does not define, each owned by a + * ticket. Remove an entry when it goes; a stale entry fails. + */ +const ALLOWED_RPCS: Array<{ name: string; ticket: string; reason: string }> = [ + { name: "match_gitmem_learnings_weighted", ticket: "GIT-114", reason: "fallback only after gitmem_scar_search answers PGRST202 (a store without the setup.sql function, e.g. nTEG's)" }, + { name: "match_transcript_chunks", ticket: "GIT-106", reason: "transcript search: dev tier only; setup.sql creates neither the table nor the function" }, +]; + +describe("RPC names against the v1.8.0 schema (GIT-114)", () => { + const functions = parseFunctions(fs.readFileSync(FLOOR_SQL, "utf-8")); + const rpc = scanRpcNames(path.join(REPO, "src")); + const storeRefs = rpc.refs.filter((r) => !r.external); + + it("parses setup.sql's functions", () => { + expect([...functions]).toEqual(expect.arrayContaining([ + "gitmem_scar_search", "gitmem_semantic_search", "refresh_scar_behavioral_scores", + ])); + }); + + it("finds the RPC call sites, including the scar-search candidate list", () => { + const names = storeRefs.map((r) => r.name); + expect(names).toEqual(expect.arrayContaining(["gitmem_scar_search", "refresh_scar_behavioral_scores"])); + expect(rpc.unresolved).toEqual([]); + }); + + it("every RPC on the customer's store is defined by setup.sql or allowlisted by ticket", () => { + const missing = storeRefs.filter((r) => !functions.has(r.name) && !ALLOWED_RPCS.some((a) => a.name === r.name)); + expect(missing.map((r) => `${r.name} at ${r.file}:${r.line}`)).toEqual([]); + }); + + it("every allowlisted RPC is still referenced and names a ticket", () => { + for (const a of ALLOWED_RPCS) { + expect(a.ticket).toMatch(/^GIT-\d+$/); + expect(storeRefs.some((r) => r.name === a.name), `stale ALLOWED_RPCS entry ${a.name}`).toBe(true); + } + }); + + it("catches a planted call to a function setup.sql lacks, and a dynamic name it cannot resolve", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-rpc-drift-")); + try { + fs.mkdirSync(path.join(dir, "src")); + fs.writeFileSync(path.join(dir, "src", "a.ts"), ` + declare const U: string; declare function getTableName(b: string): string; declare const x: string; + export const a = \`\${U}/rest/v1/rpc/match_\${getTableName("learnings")}_weighted\`; + export const b = "https://example.supabase.co/rest/v1/rpc/licence_check"; + export const c = \`\${U}/rest/v1/rpc/\${x}\`;`); + const r = scanRpcNames(path.join(dir, "src")); + expect(r.refs.map((x) => `${x.name}:${x.external}`).sort()).toEqual(["licence_check:true", "match_gitmem_learnings_weighted:false"]); + expect(r.unresolved).toHaveLength(1); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); From ea7c6e822efd0339a105e1279abc3f2fef6857ad Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:12:59 -0700 Subject: [PATCH 03/17] test: venue flow recalls with GITMEM_SEARCH_MODE=remote and from a cold server (GIT-114) Both must return a seeded scar through the store's RPC. On 1.11.0 the RPC 404s (PGRST202) and both probes find nothing: exit 1. Co-Authored-By: Claude Opus 5.5 --- tests/e2e/blank-supabase.mjs | 38 +++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 0a16761..39e5a63 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -200,7 +200,7 @@ function isolatedHome(tag) { return { home, gitmemDir }; } const STRIP = /^(OPENAI_API_KEY|OPENROUTER_API_KEY|GITMEM_|SUPABASE_|OLLAMA_|NODE_OPTIONS$|CLAUDE_CODE_ENTRYPOINT$)/; -async function startServer({ home, gitmemDir, ollamaUrl, netlog, stderrSink }) { +async function startServer({ home, gitmemDir, ollamaUrl, netlog, stderrSink, extraEnv = {} }) { const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => !STRIP.test(k))); Object.assign(env, { HOME: home, @@ -216,6 +216,7 @@ async function startServer({ home, gitmemDir, ollamaUrl, netlog, stderrSink }) { GITMEM_NETLOG: netlog, NODE_OPTIONS: `--import=${pathToFileURL(NETLOG_PRELOAD).href}`, NO_COLOR: "1", + ...extraEnv, }); const transport = new StdioClientTransport({ command: process.execPath, args: [SERVER], env, cwd: home, stderr: "pipe" }); transport.stderr?.on("data", (d) => stderrSink.push(d.toString())); @@ -422,6 +423,33 @@ async function flow() { const relevance = relevanceRows.map((m) => ({ tool: m.tool_name, memories_applied: m.metadata.memories_applied, memory_relevance: m.metadata.memory_relevance })); await waitQuiet(netlog, { minMs: 1000, quietMs: 2000 }); await srv.close(); + + // GIT-114: recall served by the store, not the local index. (1) Forced with + // GITMEM_SEARCH_MODE=remote. (2) A fresh server asked before its index has + // loaded — the cold-start path every customer hits first. Both must return a + // seeded scar; the store must answer the RPC (a 404 fails the run below). + const recallProbe = async (tag, extraEnv) => { + const h = isolatedHome(`${label}-${tag}`); + const probeLog = join(outDir, `netlog-${label}-${tag}.jsonl`); + writeFileSync(probeLog, ""); + const errs = []; + const p = await startServer({ home: h.home, gitmemDir: h.gitmemDir, ollamaUrl: ollama.url, netlog: probeLog, stderrSink: errs, extraEnv }); + let text = ""; + try { + text = await p.call("recall", { plan: "Supabase migrations must be dry-run before push", project: PROJECT, match_count: 3 }); + } catch (e) { text = `ERROR ${e}`; } + await p.close(); + allVenueRequests.push(...venueReqs(readNet(probeLog))); + const errText = errs.join(""); + return { + found_seeded_scar: /Supabase migrations must be dry-run/.test(text), + used_store_rpc: venueReqs(readNet(probeLog)).some((r) => r.path.includes("/rpc/") && r.status < 300), + fell_back_from_local: /Local cache not ready/.test(errText), + text: text.slice(0, 300), + }; + }; + const recallRemote = await recallProbe("recall-remote", { GITMEM_SEARCH_MODE: "remote" }); + const recallCold = await recallProbe("recall-cold", {}); ollama.close(); const reqs = readNet(netlog); @@ -441,6 +469,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, + recall_remote: recallRemote, + recall_cold: recallCold, missing_payload_named: missingPayloadNamed, resolve_thread_durable: resolveDurable, close_warns: closeWarns, @@ -648,6 +678,12 @@ if (mode === "flow" && !result.thread_triples_linked) { 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.recall_remote.found_seeded_scar && result.recall_remote.used_store_rpc)) { + failureCheck.unexpected.push(`recall with GITMEM_SEARCH_MODE=remote did not return a seeded scar from the store (GIT-114): ${result.recall_remote.text}`); +} +if (mode === "flow" && !result.recall_cold.found_seeded_scar) { + failureCheck.unexpected.push(`cold-start recall returned no seeded scar (GIT-114): ${result.recall_cold.text}`); +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } From b6037820e21690bed6e2052b0e35f355fb7d4fd9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:21:03 -0700 Subject: [PATCH 04/17] fix: hooks need only bash and node; missing tools fail visibly (GIT-116) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit auto-retrieve-hook.sh ran `timeout 2.5 node …`. macOS ships no timeout, the error went to /dev/null, and the hook exited as "nothing relevant", so auto-retrieve never ran on a stock Mac. - quick-retrieve bounds its own run (armDeadline, 2.5 s, unref'd); the hook calls node directly. - Hooks copied into /.gitmem/hooks looked for a package named "gitmem"; it is gitmem-mcp. The lookup now tries the gitmem-mcp bin, npx's cache (how most installs run it) and the global node_modules. - A missing node or quick-retrieve module is a visible non-blocking hook error (exit 1, reason on stderr), not a silent empty result. The node check runs before prompt extraction, which needed node too. - session-start.sh: debug log per user in $TMPDIR and never fatal under set -e; no pgrep skips the 7 s readiness gate instead of stalling; `which` -> `command -v`; no GNU-only `date +%N`. - resolve-root.sh: pid liveness via kill -0, then /proc (Linux) or ps (macOS), not kill's English error text; host from bash's $HOSTNAME. - credential-guard.sh: the path is JSON-escaped. A quote in it made the block output invalid JSON, which Claude Code treats as allow. test-hooks.sh: +7 portability tests using PATHs with the tool removed (53/53; main fails 6). Unit: +4 (deadline, unref, no timeout call left). Co-Authored-By: Claude Opus 5.5 --- hooks/scripts/auto-retrieve-hook.sh | 43 ++++++---- hooks/scripts/credential-guard.sh | 4 + hooks/scripts/resolve-root.sh | 20 +++-- hooks/scripts/session-start.sh | 41 +++++---- hooks/tests/test-hooks.sh | 86 +++++++++++++++++++ src/hooks/quick-retrieve.ts | 25 ++++++ .../hooks/quick-retrieve-deadline.test.ts | 41 +++++++++ 7 files changed, 224 insertions(+), 36 deletions(-) create mode 100644 tests/unit/hooks/quick-retrieve-deadline.test.ts diff --git a/hooks/scripts/auto-retrieve-hook.sh b/hooks/scripts/auto-retrieve-hook.sh index 7bdfabb..d1befad 100755 --- a/hooks/scripts/auto-retrieve-hook.sh +++ b/hooks/scripts/auto-retrieve-hook.sh @@ -36,7 +36,20 @@ HOOK_INPUT=$(cat -) # Extract prompt from hook input # ============================================================================ -# Use node if available, fallback to basic extraction +# GIT-116: a missing tool is a visible, non-blocking hook error (exit 1 with a +# reason on stderr; exit 2 would block the prompt), never a silent "nothing +# relevant". Retrieval needs node, which the MCP server needs anyway — but the +# hook runs with the launcher's PATH, where an nvm/volta node may be absent. +gitmem_unavailable() { + echo "gitmem auto-retrieve is not running: $1" >&2 + exit 1 +} + +if ! command -v node &>/dev/null; then + gitmem_unavailable "node is not on the hook's PATH (PATH=$PATH)" +fi + +# Extract the prompt (node is required; checked above) PROMPT="" if command -v node &>/dev/null; then PROMPT=$(echo "$HOOK_INPUT" | node -e " @@ -134,10 +147,6 @@ fi # Invoke quick-retrieve (Node module) # ============================================================================ -# Require node for retrieval -if ! command -v node &>/dev/null; then - exit 0 -fi # Locate quick-retrieve.js relative to plugin root # Hook scripts are at: ${CLAUDE_PLUGIN_ROOT}/scripts/ @@ -150,13 +159,17 @@ if [ ! -f "$QUICK_RETRIEVE" ]; then QUICK_RETRIEVE="${CLAUDE_PLUGIN_ROOT}/../dist/hooks/quick-retrieve.js" fi -# If still not found, try the gitmem package location +# Hooks copied into /.gitmem/hooks (init, install-hooks) have no dist/ +# beside them, so look where the package itself is installed. GIT-116: this +# searched for a package called "gitmem"; it is "gitmem-mcp". Most installs run +# it through npx, so npx's cache is searched too (newest first). if [ ! -f "$QUICK_RETRIEVE" ]; then - # Check common install locations + REL="gitmem-mcp/dist/hooks/quick-retrieve.js" + BIN="$(command -v gitmem-mcp 2>/dev/null || true)" for CANDIDATE in \ - "/workspace/gitmem/dist/hooks/quick-retrieve.js" \ - "$(npm root -g 2>/dev/null)/gitmem/dist/hooks/quick-retrieve.js" \ - "$(dirname "$(which gitmem 2>/dev/null)")/../lib/node_modules/gitmem/dist/hooks/quick-retrieve.js"; do + ${BIN:+"$(dirname "$BIN")/../lib/node_modules/$REL"} \ + $(ls -1t "$HOME"/.npm/_npx/*/node_modules/$REL 2>/dev/null) \ + "$(npm root -g 2>/dev/null)/$REL"; do if [ -f "$CANDIDATE" ]; then QUICK_RETRIEVE="$CANDIDATE" break @@ -165,13 +178,13 @@ if [ ! -f "$QUICK_RETRIEVE" ]; then fi if [ ! -f "$QUICK_RETRIEVE" ]; then - # Can't find quick-retrieve module — fail open - exit 0 + gitmem_unavailable "quick-retrieve.js not found next to the hook, in npx's cache, or in the global node_modules" fi -# Call quick-retrieve with prompt and level -# Timeout: 2.5s (leave 500ms buffer within 3s hook timeout) -RESULT=$(timeout 2.5 node "$QUICK_RETRIEVE" "$PROMPT" "$RETRIEVAL_LEVEL" 2>/dev/null) || true +# Call quick-retrieve with prompt and level. It bounds its own run (2.5 s, +# inside the 3 s hook budget): GIT-116 dropped the `timeout` wrapper, which +# macOS does not ship. +RESULT=$(node "$QUICK_RETRIEVE" "$PROMPT" "$RETRIEVAL_LEVEL" 2>/dev/null) || true # Empty result → nothing relevant found if [ -z "$RESULT" ]; then diff --git a/hooks/scripts/credential-guard.sh b/hooks/scripts/credential-guard.sh index 83361d3..0ab006f 100755 --- a/hooks/scripts/credential-guard.sh +++ b/hooks/scripts/credential-guard.sh @@ -111,6 +111,10 @@ if [ "$TOOL_NAME" = "Read" ]; then # Block reading known credential files if echo "$BASENAME" | grep -qEi "$CREDENTIAL_FILES_PATTERN"; then + # GIT-116: escape for the JSON below. A quote or backslash in the path + # made the output invalid JSON, which Claude Code treats as "allow". + FILE_PATH=$(printf '%s' "$FILE_PATH" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g') + BASENAME=$(printf '%s' "$BASENAME" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g') cat </dev/null && return 0 + if [ -d /proc/self ]; then + [ -d "/proc/$1" ] && return 0 || return 1 + fi + if command -v ps >/dev/null 2>&1; then + ps -p "$1" >/dev/null 2>&1 && return 0 || return 1 + fi local out - out=$(kill -0 "$1" 2>&1) && return 0 + out=$(LC_ALL=C kill -0 "$1" 2>&1) case "$out" in *ermitted*) return 0 ;; esac return 1 } @@ -59,7 +68,8 @@ gitmem_pid_alive() { gitmem_live_session_ids() { [ -f "$GITMEM_ACTIVE_SESSIONS" ] || return 0 local host rows - host=$(hostname 2>/dev/null || echo "") + # bash's own HOSTNAME is gethostname(), what the server's os.hostname() is. + host="${HOSTNAME:-$(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("|")' \ diff --git a/hooks/scripts/session-start.sh b/hooks/scripts/session-start.sh index 8e11fbc..7a59048 100755 --- a/hooks/scripts/session-start.sh +++ b/hooks/scripts/session-start.sh @@ -7,13 +7,16 @@ set -e -# Debug logging — verify hook actually fires -PLUGIN_LOG="/tmp/gitmem-hooks-plugin-debug.log" -echo "[$(date)] PLUGIN SessionStart hook invoked" >> "$PLUGIN_LOG" +# Debug logging — verify hook actually fires. GIT-116: per user and in +# $TMPDIR (macOS gives each user its own), and never fatal: under `set -e` an +# unwritable log — e.g. /tmp/…debug.log created by another user — used to kill +# the whole SessionStart hook. +PLUGIN_LOG="${TMPDIR:-/tmp}/gitmem-hooks-plugin-debug-$(id -u).log" +echo "[$(date)] PLUGIN SessionStart hook invoked" >> "$PLUGIN_LOG" 2>/dev/null || true # Read hook input from stdin HOOK_INPUT=$(cat -) -echo "[$(date)] Input: $HOOK_INPUT" >> "$PLUGIN_LOG" +echo "[$(date)] Input: $HOOK_INPUT" >> "$PLUGIN_LOG" 2>/dev/null || true # ============================================================================ # Detect gitmem MCP server @@ -98,8 +101,7 @@ fi if [ "$GITMEM_DETECTED" = "false" ]; then for SERVER_PATH in \ "${GITMEM_SERVER_PATH:-}" \ - "$(which gitmem-mcp 2>/dev/null || echo '')" \ - "$(which gitmem 2>/dev/null || echo '')"; do + "$(command -v gitmem-mcp 2>/dev/null || echo '')"; do if [ -n "$SERVER_PATH" ] && [ -f "$SERVER_PATH" ]; then GITMEM_DETECTED=true DETECT_SOURCE="binary $SERVER_PATH" @@ -122,13 +124,13 @@ fi # MCP handshake completion. Total budget: ~8s (within 10s hook timeout). if [ "$GITMEM_DETECTED" = "true" ]; then - GATE_START=$(date +%s%N 2>/dev/null || date +%s) + GATE_START=$(date +%s) MAX_WAIT_SECS=7 POLL_INTERVAL=0.3 HANDSHAKE_BUFFER=0.5 SERVER_FOUND=false - echo "[$(date)] MCP readiness gate: waiting up to ${MAX_WAIT_SECS}s for gitmem server process..." >> "$PLUGIN_LOG" + echo "[$(date)] MCP readiness gate: waiting up to ${MAX_WAIT_SECS}s for gitmem server process..." >> "$PLUGIN_LOG" 2>/dev/null || true # Build list of patterns to match the gitmem server process # Covers: direct node invocation, npx, and symlinked binaries @@ -140,12 +142,19 @@ if [ "$GITMEM_DETECTED" = "true" ]; then # Poll every 0.3s. Max iterations = MAX_WAIT_SECS / 0.3 ≈ 23 MAX_POLLS=$(( MAX_WAIT_SECS * 10 / 3 )) POLL=0 + # GIT-116: pgrep ships with macOS and procps, but not with every slim + # image. Without it every poll failed and the gate stalled for its full + # 7 s before continuing; skip the wait instead, and say so. + if ! command -v pgrep >/dev/null 2>&1; then + echo "[$(date)] MCP readiness gate: pgrep not available — not waiting for the server process" >> "$PLUGIN_LOG" 2>/dev/null || true + MAX_POLLS=0 + fi while [ "$POLL" -lt "$MAX_POLLS" ]; do for PATTERN in "${GITMEM_PATTERNS[@]}"; do if pgrep -f "$PATTERN" > /dev/null 2>&1; then SERVER_FOUND=true WAIT_SECS=$(( POLL * 3 / 10 )) - echo "[$(date)] MCP readiness gate: gitmem process found (pattern: $PATTERN) after ~${WAIT_SECS}.$(( POLL * 3 % 10 ))s" >> "$PLUGIN_LOG" + echo "[$(date)] MCP readiness gate: gitmem process found (pattern: $PATTERN) after ~${WAIT_SECS}.$(( POLL * 3 % 10 ))s" >> "$PLUGIN_LOG" 2>/dev/null || true # Buffer for MCP protocol handshake to complete sleep "$HANDSHAKE_BUFFER" break 2 @@ -156,11 +165,11 @@ if [ "$GITMEM_DETECTED" = "true" ]; then done if [ "$SERVER_FOUND" = "false" ]; then - echo "[$(date)] MCP readiness gate: gitmem process NOT found after ${MAX_WAIT_SECS}s — proceeding anyway (config detected, server may use different process name)" >> "$PLUGIN_LOG" + echo "[$(date)] MCP readiness gate: gitmem process NOT found after ${MAX_WAIT_SECS}s — proceeding anyway (config detected, server may use different process name)" >> "$PLUGIN_LOG" 2>/dev/null || true fi - GATE_END=$(date +%s%N 2>/dev/null || date +%s) - echo "[$(date)] MCP readiness gate completed" >> "$PLUGIN_LOG" + GATE_END=$(date +%s) + echo "[$(date)] MCP readiness gate completed" >> "$PLUGIN_LOG" 2>/dev/null || true fi # ============================================================================ @@ -179,7 +188,7 @@ echo "0" > "$STATE_DIR/last_nag_time" # ============================================================================ if [ "$GITMEM_DETECTED" = "true" ]; then - echo "[$(date)] Gitmem DETECTED via ${DETECT_SOURCE} — instructing agent to call session_start via MCP" >> "$PLUGIN_LOG" + echo "[$(date)] Gitmem DETECTED via ${DETECT_SOURCE} — instructing agent to call session_start via MCP" >> "$PLUGIN_LOG" 2>/dev/null || true # Instruct agent to call session_start via MCP. # The MCP server creates sessions with its own PID, enabling multi-session @@ -197,7 +206,7 @@ if [ "$GITMEM_DETECTED" = "true" ]; then fi if [ -n "$GITMEM_PROJECT" ]; then - echo "[$(date)] Project from config: ${GITMEM_PROJECT}" >> "$PLUGIN_LOG" + echo "[$(date)] Project from config: ${GITMEM_PROJECT}" >> "$PLUGIN_LOG" 2>/dev/null || true cat <> "$PLUGIN_LOG" + echo "[$(date)] Gitmem NOT detected (checked: project .mcp.json, mcp-config, ~/.claude.json, disk)" >> "$PLUGIN_LOG" 2>/dev/null || true echo "GITMEM PLUGIN: GitMem MCP server not detected. Session lifecycle hooks are inactive. To enable, ensure gitmem is configured via --mcp-config, project .mcp.json, or set GITMEM_ENABLED=true." fi -echo "[$(date)] Hook completed, exit 0" >> "$PLUGIN_LOG" +echo "[$(date)] Hook completed, exit 0" >> "$PLUGIN_LOG" 2>/dev/null || true exit 0 diff --git a/hooks/tests/test-hooks.sh b/hooks/tests/test-hooks.sh index 95d5df8..0e24960 100755 --- a/hooks/tests/test-hooks.sh +++ b/hooks/tests/test-hooks.sh @@ -726,6 +726,92 @@ GOT=$(bash -c '. "$0/scripts/resolve-root.sh"; gitmem_pid_alive 1 && echo alive' remove_session_registry +# ============================================================================ +# TEST GROUP: portability (GIT-116) +# ============================================================================ + +echo "" +echo -e "${YELLOW}=== Portability (GIT-116) ===${NC}" + +# A PATH holding every command on the current PATH except the ones named. +path_without() { + local out="$1"; shift + mkdir -p "$out" + local d f n old_ifs="$IFS" + IFS=: + for d in $PATH; do + [ -d "$d" ] || continue + for f in "$d"/*; do + n=$(basename "$f") + [ -x "$f" ] && [ ! -e "$out/$n" ] && ln -s "$f" "$out/$n" 2>/dev/null || true + done + done + IFS="$old_ifs" + for n in "$@"; do rm -f "$out/$n"; done + echo "$out" +} + +# A fake installed package: hooks/scripts next to dist/hooks/quick-retrieve.js. +PKG="$TMPDIR/pkg" +mkdir -p "$PKG/hooks/scripts" "$PKG/dist/hooks" +cp "$SCRIPT_DIR"/scripts/*.sh "$PKG/hooks/scripts/" +cat > "$PKG/dist/hooks/quick-retrieve.js" <<'STUB' +process.stdout.write("STUB-SCAR for: " + process.argv[2]); +STUB +PROMPT_JSON='{"prompt":"add a retry to the deploy script before the migration"}' + +NO_TIMEOUT_PATH=$(path_without "$TMPDIR/bin-no-timeout" timeout gtimeout) +OUT=$(echo "$PROMPT_JSON" | PATH="$NO_TIMEOUT_PATH" bash "$PKG/hooks/scripts/auto-retrieve-hook.sh" 2>/dev/null) || true +echo "$OUT" | grep -q "STUB-SCAR" && pass "auto-retrieve works with no timeout/gtimeout on PATH (stock macOS)" \ + || fail "auto-retrieve without timeout" "STUB-SCAR in additionalContext" "$OUT" + +NO_NODE_PATH=$(path_without "$TMPDIR/bin-no-node" node) +set +e +ERR=$(echo "$PROMPT_JSON" | PATH="$NO_NODE_PATH" bash "$PKG/hooks/scripts/auto-retrieve-hook.sh" 2>&1 >/dev/null); CODE=$? +set -e +[ "$CODE" = "1" ] && echo "$ERR" | grep -q "node is not on the hook's PATH" \ + && pass "no node: visible non-blocking error (exit 1, reason on stderr)" \ + || fail "no node visible" "exit 1 + reason" "exit=$CODE err=$ERR" + +# Hooks copied into a repo (init / install-hooks): no dist beside them. +COPIED="$TMPDIR/repo/.gitmem/hooks"; mkdir -p "$COPIED"; cp "$SCRIPT_DIR"/scripts/*.sh "$COPIED/" +EMPTY_HOME="$TMPDIR/empty-home"; mkdir -p "$EMPTY_HOME" +NO_PKG_PATH=$(path_without "$TMPDIR/bin-no-pkg" gitmem-mcp npm) +set +e +ERR=$(echo "$PROMPT_JSON" | HOME="$EMPTY_HOME" PATH="$NO_PKG_PATH" bash "$COPIED/auto-retrieve-hook.sh" 2>&1 >/dev/null); CODE=$? +set -e +[ "$CODE" = "1" ] && echo "$ERR" | grep -q "quick-retrieve.js not found" \ + && pass "module not found: visible non-blocking error, not a silent 'nothing relevant'" \ + || fail "module not found visible" "exit 1 + reason" "exit=$CODE err=$ERR" + +NPX_HOME="$TMPDIR/npx-home" +mkdir -p "$NPX_HOME/.npm/_npx/abc123/node_modules/gitmem-mcp/dist/hooks" +cp "$PKG/dist/hooks/quick-retrieve.js" "$NPX_HOME/.npm/_npx/abc123/node_modules/gitmem-mcp/dist/hooks/" +OUT=$(echo "$PROMPT_JSON" | HOME="$NPX_HOME" PATH="$NO_PKG_PATH" bash "$COPIED/auto-retrieve-hook.sh" 2>/dev/null) || true +echo "$OUT" | grep -q "STUB-SCAR" && pass "copied hooks find gitmem-mcp in npx's cache" \ + || fail "npx cache lookup" "STUB-SCAR" "$OUT" + +# session-start: no pgrep must not stall the 10 s hook for the 7 s gate. +NO_PGREP_PATH=$(path_without "$TMPDIR/bin-no-pgrep" pgrep) +T0=$(date +%s) +OUT=$(echo '{}' | GITMEM_ENABLED=true PATH="$NO_PGREP_PATH" bash "$SCRIPT_DIR/scripts/session-start.sh" 2>/dev/null) || true +ELAPSED=$(( $(date +%s) - T0 )) +[ "$ELAPSED" -lt 4 ] && echo "$OUT" | grep -q "SESSION START" \ + && pass "session-start without pgrep does not stall (${ELAPSED}s)" \ + || fail "session-start without pgrep" "<4s and SESSION START" "${ELAPSED}s: $OUT" + +# session-start: an unwritable debug log must not kill the hook under set -e. +BAD_TMP="$TMPDIR/bad-tmp"; mkdir -p "$BAD_TMP/gitmem-hooks-plugin-debug-$(id -u).log" +OUT=$(echo '{}' | GITMEM_ENABLED=true TMPDIR="$BAD_TMP" PATH="$NO_PGREP_PATH" bash "$SCRIPT_DIR/scripts/session-start.sh" 2>/dev/null) || true +echo "$OUT" | grep -q "SESSION START" && pass "session-start survives an unwritable debug log" \ + || fail "unwritable debug log" "SESSION START" "$OUT" + +# credential-guard: a quote or backslash in the path must still produce JSON that blocks. +OUT=$(printf '{"tool_name":"Read","tool_input":{"file_path":"/x/we\\"ird\\\\d/.env"}}' | bash "$SCRIPT_DIR/scripts/credential-guard.sh" 2>/dev/null) || true +echo "$OUT" | node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{process.exit(JSON.parse(d).decision==="block"?0:1)})' 2>/dev/null \ + && pass "credential-guard block is valid JSON for a path with quotes" \ + || fail "credential-guard JSON" "valid JSON, decision block" "$OUT" + # ============================================================================ # Summary # ============================================================================ diff --git a/src/hooks/quick-retrieve.ts b/src/hooks/quick-retrieve.ts index c414b33..41cfc4b 100644 --- a/src/hooks/quick-retrieve.ts +++ b/src/hooks/quick-retrieve.ts @@ -148,12 +148,37 @@ function toFormattable(scar: RelevantScar): FormattableScar { }; } +// --- Time bound (GIT-116) --- +// The UserPromptSubmit hook has a 3 s budget. The hook used to enforce it with +// `timeout 2.5 node …`, but macOS ships no `timeout`, so on a stock Mac the +// call failed, the error was discarded and nothing was ever retrieved. The +// bound lives here now: node is the one tool the hook already requires. + +export const QUICK_RETRIEVE_DEADLINE_MS = 2500; + +/** + * Exit quietly (no context injected) once `ms` have passed. unref'd, so a + * run that finishes first is not held open by the timer. + */ +export function armDeadline( + ms: number, + onExpire: () => void = () => { + process.stderr.write(`gitmem quick-retrieve: no result within ${ms} ms; nothing injected\n`); + process.exit(0); + } +): ReturnType { + const timer = setTimeout(onExpire, ms); + timer.unref?.(); + return timer; +} + // --- CLI entry point --- // Called from hook: node dist/hooks/quick-retrieve.js [token_budget] const isMainModule = process.argv[1]?.endsWith("quick-retrieve.js"); if (isMainModule) { + armDeadline(Number(process.env.GITMEM_QUICK_RETRIEVE_DEADLINE_MS) || QUICK_RETRIEVE_DEADLINE_MS); const prompt = process.argv[2]; const level = process.argv[3] || "scars"; const budget = process.argv[4] ? parseInt(process.argv[4], 10) : undefined; diff --git a/tests/unit/hooks/quick-retrieve-deadline.test.ts b/tests/unit/hooks/quick-retrieve-deadline.test.ts new file mode 100644 index 0000000..2d34ef1 --- /dev/null +++ b/tests/unit/hooks/quick-retrieve-deadline.test.ts @@ -0,0 +1,41 @@ +/** + * GIT-116: quick-retrieve bounds its own run. The hook used to wrap it in + * `timeout 2.5`, which macOS does not ship, so on a stock Mac nothing was + * ever retrieved. + */ +import { describe, it, expect, vi, afterEach } from "vitest"; +import * as fs from "fs"; +import * as path from "path"; +import { armDeadline, QUICK_RETRIEVE_DEADLINE_MS } from "../../../src/hooks/quick-retrieve.js"; + +afterEach(() => { vi.useRealTimers(); }); + +describe("quick-retrieve deadline (GIT-116)", () => { + it("fires after the budget and not before", () => { + vi.useFakeTimers(); + const expire = vi.fn(); + armDeadline(100, expire); + vi.advanceTimersByTime(99); + expect(expire).not.toHaveBeenCalled(); + vi.advanceTimersByTime(1); + expect(expire).toHaveBeenCalledOnce(); + }); + + it("does not keep a finished run alive (unref'd)", () => { + const timer = armDeadline(60_000, () => {}); + expect(timer.hasRef()).toBe(false); + clearTimeout(timer); + }); + + it("leaves 500 ms of the hook's 3 s budget", () => { + expect(QUICK_RETRIEVE_DEADLINE_MS).toBe(2500); + }); + + it("no hook script calls timeout/gtimeout any more", () => { + const dir = path.resolve(__dirname, "../../../hooks/scripts"); + for (const f of fs.readdirSync(dir).filter((f) => f.endsWith(".sh"))) { + const code = fs.readFileSync(path.join(dir, f), "utf-8").split("\n").filter((l) => !l.trim().startsWith("#")); + expect(code.filter((l) => /(^|[\s;(|&`$])g?timeout\s+[0-9]/.test(l)), f).toEqual([]); + } + }); +}); From 63bdba54c6cc4370530c9c99bc0dad8d1679330d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:21:04 -0700 Subject: [PATCH 05/17] ci: hook tests on stock macOS (no Homebrew, no timeout, no jq); publish depends on it (GIT-116) Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a92f871..8e2e7f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,6 +73,28 @@ jobs: - name: Hook tests run: bash hooks/tests/test-hooks.sh + # GIT-116: the same suite on stock macOS. The runner has Homebrew (and may + # have coreutils' timeout), so the suite runs with the system directories + # plus a directory holding only node — what a Mac without Homebrew has. + hooks-macos: + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 20 + + - name: Hook tests on a stock macOS PATH (no Homebrew, no timeout, no jq) + run: | + mkdir -p "$RUNNER_TEMP/nodeonly" + ln -s "$(command -v node)" "$RUNNER_TEMP/nodeonly/node" + STOCK="/usr/bin:/bin:/usr/sbin:/sbin:$RUNNER_TEMP/nodeonly" + for t in timeout gtimeout jq; do + if PATH="$STOCK" command -v "$t" >/dev/null; then echo "::error::$t is on the stock PATH"; exit 1; fi + done + env -i HOME="$HOME" PATH="$STOCK" /bin/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 @@ -164,7 +186,7 @@ jobs: retention-days: 14 publish: - needs: [build, hooks-without-jq, venue-gate] + needs: [build, hooks-without-jq, hooks-macos, venue-gate] runs-on: ubuntu-latest if: startsWith(github.ref, 'refs/tags/v') permissions: From b4aa396f42652bd184a830c9a1af3795cf9b6d66 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:28:24 -0700 Subject: [PATCH 06/17] fix: a thread counts as synced only when the store confirms its row (GIT-117) syncThreadsToSupabase called create/resolve/touch helpers that caught their own failures and returned null/false/void, which the loop never read. So a thread whose write the store rejected was counted as synced: session_close reported COMPLETE, pruned threads.json and deleted the session files, and a local-only thread was dropped as stale 24 h later. - The loop uses throwing variants: a create counts when directUpsert returns the row; a resolve or touch counts only when the PATCH returns a row (0 rows = failed); a thread the store doesn't have is a failure, not a no-op. touchThreadsInSupabase reports per-thread outcomes. - Partial sync: the close says PARTIAL (session saved, threads not) and names the failed ids; threads.json keeps them marked sync_pending. - session_start keeps sync_pending threads whatever their age and prefers their local state over the store's (so an unwritten resolution survives) until a later close writes them; a full sync clears the flag. thread-sync-result.test.ts: the GIT-70 tests only failed the lookup, the one call that propagated. +6 tests fail the writes; 5 fail on main. Co-Authored-By: Claude Opus 5.5 --- src/services/display-protocol.ts | 1 + src/services/thread-supabase.ts | 111 +++++++++++------- src/tools/session-close.ts | 24 +++- src/tools/session-start.ts | 10 +- src/types/index.ts | 6 + .../unit/services/thread-sync-result.test.ts | 82 +++++++++++++ 6 files changed, 183 insertions(+), 51 deletions(-) diff --git a/src/services/display-protocol.ts b/src/services/display-protocol.ts index ec712e8..5e0cd2c 100644 --- a/src/services/display-protocol.ts +++ b/src/services/display-protocol.ts @@ -156,6 +156,7 @@ export const STATUS = { rejected: `${red}REJECTED${reset}`, complete: `${green}COMPLETE${reset}`, failed: `${red}FAILED${reset}`, + partial: `${yellow}PARTIAL${reset}`, pass: `${green}+${reset}`, miss: `${red}-${reset}`, } as const; diff --git a/src/services/thread-supabase.ts b/src/services/thread-supabase.ts index 1494521..9674ae5 100644 --- a/src/services/thread-supabase.ts +++ b/src/services/thread-supabase.ts @@ -205,19 +205,24 @@ export async function createThreadInSupabase( } try { - const row = threadObjectToRow(thread, project, embedding); - const result = await supabase.directUpsert( - getTableName("threads"), - row - ); - console.error(`[thread-supabase] Created thread ${thread.id} in Supabase`); - return result; + return await writeThreadRow(thread, project, embedding); } catch (error) { console.error("[thread-supabase] Failed to create thread:", error instanceof Error ? error.message : error); return null; } } +/** + * GIT-117: create a thread row, or throw. directUpsert asks for the stored + * representation and throws on an empty one, so a return is a confirmed row. + */ +async function writeThreadRow(thread: ThreadObject, project: Project, embedding?: string | null): Promise { + const row = threadObjectToRow(thread, project, embedding); + const result = await supabase.directUpsert(getTableName("threads"), row); + console.error(`[thread-supabase] Created thread ${thread.id} in Supabase`); + return result; +} + /** * Resolve a thread in Supabase by thread_id. * Updates status, resolved_at, resolution_note, resolved_by_session. @@ -236,33 +241,7 @@ export async function resolveThreadInSupabase( } try { - // First, find the UUID primary key for this thread_id - const rows = await supabase.directQuery(getTableName("threads"), { - select: "id,thread_id", - filters: { thread_id: threadId }, - limit: 1, - }); - - if (rows.length === 0) { - console.error(`[thread-supabase] Thread ${threadId} not found in Supabase (will proceed with local-only)`); - return false; - } - - const uuid = rows[0].id; - const patchData: Record = { - status: "resolved", - resolved_at: options.resolvedAt || new Date().toISOString(), - }; - - if (options.resolutionNote) { - patchData.resolution_note = options.resolutionNote; - } - if (options.resolvedBySession) { - patchData.resolved_by_session = options.resolvedBySession; - } - - await supabase.directPatch(getTableName("threads"), { id: uuid }, patchData); - console.error(`[thread-supabase] Resolved thread ${threadId} in Supabase`); + await resolveThreadRow(threadId, options); return true; } catch (error) { console.error("[thread-supabase] Failed to resolve thread:", error instanceof Error ? error.message : error); @@ -270,6 +249,33 @@ export async function resolveThreadInSupabase( } } +/** + * GIT-117: resolve a thread row, or throw. Confirmed only when the PATCH + * returns the row: an empty result means nothing was updated. + */ +async function resolveThreadRow( + threadId: string, + options: { resolvedAt?: string; resolutionNote?: string; resolvedBySession?: string } +): Promise { + const rows = await supabase.directQuery(getTableName("threads"), { + select: "id,thread_id", + filters: { thread_id: threadId }, + limit: 1, + }); + if (rows.length === 0) throw new Error(`thread ${threadId} not found in the store`); + + const patchData: Record = { + status: "resolved", + resolved_at: options.resolvedAt || new Date().toISOString(), + }; + if (options.resolutionNote) patchData.resolution_note = options.resolutionNote; + if (options.resolvedBySession) patchData.resolved_by_session = options.resolvedBySession; + + const patched = await supabase.directPatch(getTableName("threads"), { id: rows[0].id }, patchData); + if (!Array.isArray(patched) || patched.length === 0) throw new Error(`resolve of ${threadId} updated no row`); + console.error(`[thread-supabase] Resolved thread ${threadId} in Supabase`); +} + /** * Fetch a single thread from Supabase by its thread_id ("t-XXXX"). * @@ -508,9 +514,10 @@ export async function loadActiveThreadsFromSupabase( */ export async function touchThreadsInSupabase( threadIds: string[] -): Promise { +): Promise<{ touched: string[]; failed: { id: string; error: string }[] }> { + const outcome = { touched: [] as string[], failed: [] as { id: string; error: string }[] }; if (!hasSupabase() || !supabase.isConfigured() || threadIds.length === 0) { - return; + return outcome; } for (const threadId of threadIds) { @@ -522,12 +529,17 @@ export async function touchThreadsInSupabase( limit: 1, }); - if (rows.length === 0) continue; + // GIT-117: a thread we believe exists but the store does not have is a + // failed write, not a no-op. + if (rows.length === 0) throw new Error(`thread ${threadId} not found in the store`); const row = rows[0]; // Skip resolved/archived threads — no point recomputing vitality - if (row.status === "resolved" || row.status === "archived") continue; + if (row.status === "resolved" || row.status === "archived") { + outcome.touched.push(threadId); + continue; + } const now = new Date(); const newTouchCount = (row.touch_count || 0) + 1; @@ -551,18 +563,23 @@ export async function touchThreadsInSupabase( delete metadata.dormant_since; } - await supabase.directPatch(getTableName("threads"), { id: row.id }, { + const patched = await supabase.directPatch(getTableName("threads"), { id: row.id }, { touch_count: newTouchCount, last_touched_at: nowIso, vitality_score: vitality.vitality_score, status: lifecycle_status, metadata, }); + if (!Array.isArray(patched) || patched.length === 0) throw new Error(`touch of ${threadId} updated no row`); + outcome.touched.push(threadId); } catch (error) { - console.error(`[thread-supabase] Failed to touch thread ${threadId}:`, error instanceof Error ? error.message : error); + const message = error instanceof Error ? error.message : String(error); + console.error(`[thread-supabase] Failed to touch thread ${threadId}:`, message); + outcome.failed.push({ id: threadId, error: message }); // Continue with other threads } } + return outcome; } /** @@ -665,13 +682,18 @@ export async function syncThreadsToSupabase( const normalizedNewText = normalizeText(thread.text || ""); const matchedThreadId = normalizedNewText ? textToExistingId.get(normalizedNewText) : undefined; + // GIT-117: every write below throws unless the store confirmed a row. + // The helpers this loop called before swallowed their failures and + // returned null / false / void, so a thread was counted as synced — + // and pruned from threads.json — when nothing had been written. if (matchedThreadId) { // Duplicate text found — touch existing instead of creating console.error(`[thread-supabase] Dedup: "${thread.id}" matches existing "${matchedThreadId}" by text — touching instead of creating`); - await touchThreadsInSupabase([matchedThreadId]); + const touched = await touchThreadsInSupabase([matchedThreadId]); + if (touched.failed.length > 0) throw new Error(touched.failed[0].error); } else { // Genuinely new thread — create it - await createThreadInSupabase(thread, project); + await writeThreadRow(thread, project); // Register in lookup so subsequent threads in this batch also dedup if (normalizedNewText) { textToExistingId.set(normalizedNewText, thread.id); @@ -679,14 +701,15 @@ export async function syncThreadsToSupabase( } } else if (thread.status === "resolved" && existing[0].status !== "resolved") { // Thread was resolved during this session - await resolveThreadInSupabase(thread.id, { + await resolveThreadRow(thread.id, { resolvedAt: thread.resolved_at, resolutionNote: thread.resolution_note, resolvedBySession: thread.resolved_by_session || sessionId, }); } else { // Existing thread, just touch it - await touchThreadsInSupabase([thread.id]); + const touched = await touchThreadsInSupabase([thread.id]); + if (touched.failed.length > 0) throw new Error(touched.failed[0].error); } synced.push(thread.id); } catch (error) { diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index f398efc..ff0ee76 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -16,7 +16,7 @@ import { getStorage } from "../services/storage.js"; import { filterToStoreSessionColumns } from "../services/session-columns.js"; import { storeHasTable } from "../services/store-columns.js"; import { clearCurrentSession, resolveCurrentSession, getSurfacedScars, getConfirmations, getReflections, getObservations, getChildren, getThreads, getSessionActivity, isRecallCalled } from "../services/session-state.js"; -import { normalizeThreads, mergeThreadStates, migrateStringThread, saveThreadsFile } from "../services/thread-manager.js"; // +import { normalizeThreads, mergeThreadStates, migrateStringThread, saveThreadsFile, loadThreadsFile } from "../services/thread-manager.js"; // import { deduplicateThreadList } from "../services/thread-dedup.js"; import { syncThreadsToSupabase, loadOpenThreadEmbeddings } from "../services/thread-supabase.js"; import type { ThreadSyncResult } from "../services/thread-supabase.js"; @@ -369,8 +369,9 @@ function formatCloseDisplay( ): string { const lines: string[] = []; - // Header: branded product line - const status = success ? STATUS.complete : STATUS.failed; + // Header: branded product line. GIT-117: the session saved but some threads + // did not — PARTIAL, not FAILED (the session is there) and not COMPLETE. + const status = success ? STATUS.complete : sessionStored ? STATUS.partial : STATUS.failed; lines.push(productLine("close", status)); // Stats line: compact one-liner with key counts @@ -1445,8 +1446,19 @@ export async function sessionClose( `[session_close] Local thread prune SKIPPED — ${threadSync.failed.length} thread(s) did not reach Supabase (${threadSync.failed.map((f) => f.id).join(", ")}). ` + `threads.json retained in full; it is the only record of the unsynced threads.` ); + // GIT-117: mark what did not land so the next session_start keeps it + // (whatever its age) and the next close writes it again. + const failedIds = new Set(threadSync.failed.map((f) => f.id)); + const fileThreads = loadThreadsFile(); + const byId = new Map(fileThreads.map((t) => [t.id, t])); + for (const t of closeThreads) { + if (failedIds.has(t.id)) byId.set(t.id, { ...(byId.get(t.id) ?? {}), ...t, sync_pending: true }); + } + saveThreadsFile([...byId.values()]); } else { - const openThreadsOnly = closeThreads.filter(t => t.status === "open" || !t.status); + const openThreadsOnly = closeThreads + .filter(t => t.status === "open" || !t.status) + .map(({ sync_pending: _written, ...t }) => t); // GIT-117: written now saveThreadsFile(openThreadsOnly); console.error(`[session_close] Pruned threads.json: ${openThreadsOnly.length} open threads (removed ${closeThreads.length - openThreadsOnly.length} resolved/archived)`); } @@ -1702,8 +1714,8 @@ export async function sessionClose( const persistErrors = partialPersist ? [ - `Thread sync incomplete: ${threadSync.failed.length} of ${threadSync.attempted} thread(s) did not reach Supabase — ${threadSync.failed.map((f) => `${f.id} (${f.error})`).join("; ")}. ` + - `threads.json was NOT pruned and session files were retained; re-run session_close to retry.`, + `PARTIAL: the session saved, but ${threadSync.failed.length} of ${threadSync.attempted} thread(s) did not reach Supabase — ${threadSync.failed.map((f) => `${f.id} (${f.error})`).join("; ")}. ` + + `They are kept in threads.json and carried into the next session, whose close writes them again; session files were retained.`, ] : []; diff --git a/src/tools/session-start.ts b/src/tools/session-start.ts index b177c3c..0d8ce51 100644 --- a/src/tools/session-start.ts +++ b/src/tools/session-start.ts @@ -870,9 +870,17 @@ function writeSessionFiles( // because the NOT-IN query excludes them, making them look "local-only". const supabaseIds = new Set(threads.map(t => t.id)); const cutoff = Date.now() - 24 * 60 * 60 * 1000; + // GIT-117: a thread a close could not write is kept whatever its age, and + // its local state (e.g. a resolution) wins over the store's copy until a + // later close writes it. + const pending = new Map(existingFileThreads.filter(t => t.sync_pending && t.id).map(t => [t.id, t])); + if (pending.size > 0) { + console.error(`[session_start] Carrying forward ${pending.size} thread(s) a previous close could not write: ${[...pending.keys()].join(", ")}`); + } const localOnlyThreads = existingFileThreads.filter(t => { if (supabaseIds.has(t.id)) return false; // exists in Supabase active set if (!t.id) return false; // no ID = malformed, drop + if (t.sync_pending) return true; const created = t.created_at ? new Date(t.created_at).getTime() : 0; return created > cutoff; // only keep if created within last 24h }); @@ -883,7 +891,7 @@ function writeSessionFiles( if (dropped > 0) { console.error(`[session_start] Dropped ${dropped} stale local-only threads (resolved/archived in Supabase)`); } - merged = deduplicateThreadList([...threads, ...localOnlyThreads]); + merged = deduplicateThreadList([...threads.map(t => pending.get(t.id) ?? t), ...localOnlyThreads]); } else { // Fallback (free tier / Supabase offline): merge with existing file merged = existingFileThreads.length > 0 diff --git a/src/types/index.ts b/src/types/index.ts index 6de5c9b..383344a 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -86,6 +86,12 @@ export interface ThreadObject { source_session?: string; /** Session ID that resolved this thread */ resolved_by_session?: string; + /** + * GIT-117: a close could not write this thread (or its resolution) to the + * durable store. Kept locally, whatever its age, and preferred over the + * store's copy until a later close writes it. + */ + sync_pending?: boolean; /** Brief resolution note */ resolution_note?: string; } diff --git a/tests/unit/services/thread-sync-result.test.ts b/tests/unit/services/thread-sync-result.test.ts index ba5e976..338ed91 100644 --- a/tests/unit/services/thread-sync-result.test.ts +++ b/tests/unit/services/thread-sync-result.test.ts @@ -16,6 +16,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; const mockDirectQuery = vi.fn(); const mockDirectQueryAll = vi.fn(); const mockDirectUpsert = vi.fn(); +const mockDirectPatch = vi.fn(); const mockIsConfigured = vi.fn(() => true); const mockHasSupabase = vi.fn(() => true); @@ -24,6 +25,7 @@ vi.mock("../../../src/services/supabase-client.js", () => ({ directQuery: (...a: unknown[]) => mockDirectQuery(...a), directQueryAll: (...a: unknown[]) => mockDirectQueryAll(...a), directUpsert: (...a: unknown[]) => mockDirectUpsert(...a), + directPatch: (...a: unknown[]) => mockDirectPatch(...a), upsertRecord: vi.fn(), listRecords: vi.fn(), })); @@ -55,6 +57,8 @@ beforeEach(() => { mockIsConfigured.mockReturnValue(true); mockHasSupabase.mockReturnValue(true); mockDirectUpsert.mockResolvedValue([{ id: "row" }]); + mockDirectPatch.mockReset(); + mockDirectPatch.mockResolvedValue([{ id: "row" }]); }); describe("tier awareness (R3)", () => { @@ -236,3 +240,81 @@ describe("dedup candidate coverage (GIT-70)", () => { expect(r.dedup_coverage).not.toBe("complete"); }); }); + +// --------------------------------------------------------------------------- +// GIT-117: the WRITE fails, not the lookup +// --------------------------------------------------------------------------- +// The tests above fail only directQuery, the one call whose error propagated. +// The create / resolve / touch helpers caught their own failures and returned +// null / false / void, which the loop never read — so a thread whose write +// failed was counted as synced, and session_close pruned it from threads.json. + +describe("GIT-117: a thread counts as synced only when the store confirms a row", () => { + it("create rejected by the store -> failed, not synced", async () => { + mockDirectQuery.mockResolvedValue([]); // not in the store yet -> create + mockDirectUpsert.mockRejectedValue(new Error("Supabase upsert error: 409 - violates foreign key constraint")); + + const r = await syncThreadsToSupabase([thread("t-new")], "gitmem", "s-1"); + + expect(r.synced).toEqual([]); + expect(r.failed.map((f) => f.id)).toEqual(["t-new"]); + expect(r.failed[0].error).toMatch(/foreign key/); + expect(r.all_synced).toBe(false); + }); + + it("resolve whose PATCH updates no row -> failed", async () => { + mockDirectQuery + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r", status: "open" }]) // existence + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r" }]); // resolve lookup + mockDirectPatch.mockResolvedValue([]); + + const r = await syncThreadsToSupabase([thread("t-r", "resolved")], "gitmem", "s-1"); + + expect(r.failed.map((f) => f.id)).toEqual(["t-r"]); + expect(r.failed[0].error).toMatch(/updated no row/); + expect(r.all_synced).toBe(false); + }); + + it("resolve rejected by the store -> failed", async () => { + mockDirectQuery + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r", status: "open" }]) + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r" }]); + mockDirectPatch.mockRejectedValue(new Error("Supabase patch error: 503")); + + const r = await syncThreadsToSupabase([thread("t-r", "resolved")], "gitmem", "s-1"); + expect(r.failed.map((f) => f.id)).toEqual(["t-r"]); + }); + + it("touch of an existing thread whose PATCH fails -> failed", async () => { + mockDirectQuery + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-o", status: "active" }]) // existence + .mockResolvedValueOnce([{ id: "u1", touch_count: 1, created_at: "2026-08-01T00:00:00Z", thread_class: "backlog", status: "active" }]); + mockDirectPatch.mockRejectedValue(new Error("Supabase patch error: 500")); + + const r = await syncThreadsToSupabase([thread("t-o")], "gitmem", "s-1"); + expect(r.failed.map((f) => f.id)).toEqual(["t-o"]); + expect(r.all_synced).toBe(false); + }); + + it("one failure among several: the others are synced, only the failure is reported", async () => { + mockDirectQuery.mockResolvedValue([]); + mockDirectUpsert + .mockResolvedValueOnce([{ id: "a" }]) + .mockRejectedValueOnce(new Error("boom")) + .mockResolvedValueOnce([{ id: "c" }]); + + const r = await syncThreadsToSupabase([thread("t-a"), thread("t-b"), thread("t-c")], "gitmem", "s-1"); + expect(r.synced).toEqual(["t-a", "t-c"]); + expect(r.failed.map((f) => f.id)).toEqual(["t-b"]); + }); + + it("every write confirmed -> all synced (the contract did not get stricter than the store)", async () => { + mockDirectQuery + .mockResolvedValueOnce([]) // t-new: create + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r", status: "open" }]) // t-r: exists + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r" }]); // t-r: resolve lookup + const r = await syncThreadsToSupabase([thread("t-new"), thread("t-r", "resolved")], "gitmem", "s-1"); + expect(r.failed).toEqual([]); + expect(r.all_synced).toBe(true); + }); +}); From 6c2124b1e38f8bc2e96509eeede4a2d78227cb5f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:28:24 -0700 Subject: [PATCH 07/17] test: venue flow closes with a thread the store rejects (FK) and checks PARTIAL + carry-forward (GIT-117) On 1.11.0 the store rejects the thread and the close says COMPLETE with no pending marker: exit 1. Co-Authored-By: Claude Opus 5.5 --- tests/e2e/blank-supabase.mjs | 58 ++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 0a16761..5b61093 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -422,6 +422,56 @@ async function flow() { const relevance = relevanceRows.map((m) => ({ tool: m.tool_name, memories_applied: m.metadata.memories_applied, memory_relevance: m.metadata.memory_relevance })); await waitQuiet(netlog, { minMs: 1000, quietMs: 2000 }); await srv.close(); + + // GIT-117: a close whose thread write the store rejects. A thread naming a + // session the store has never seen violates the source_session FK (23503). + // The close must say PARTIAL and name it, keep it in threads.json marked + // sync_pending, and the next session_start must carry it forward. + const threadSyncProbe = async () => { + const h = isolatedHome(`${label}-threadsync`); + const probeLog = join(outDir, `netlog-${label}-threadsync.jsonl`); + writeFileSync(probeLog, ""); + const badId = `t-fk${randomUUID().slice(0, 6)}`; + const errs1 = []; + const p1 = await startServer({ home: h.home, gitmemDir: h.gitmemDir, ollamaUrl: ollama.url, netlog: probeLog, stderrSink: errs1 }); + const ss1 = await p1.call("session_start", { project: PROJECT, agent_identity: "cli", force: true }); + const sid1 = (ss1.match(UUID) || [])[0]; + await waitQuiet(probeLog, { minMs: 1500, quietMs: 2000 }); + const close = await p1.call("session_close", { + session_id: sid1, close_type: "standard", human_corrections: "none", + open_threads: [{ id: badId, text: `Thread whose write the store rejects (${label})`, status: "open", + created_at: new Date().toISOString(), source_session: randomUUID() }], + closing_reflection: { + what_broke: "n/a", what_took_longer: "n/a", do_differently: "n/a", what_worked: "n/a", + wrong_assumption: "n/a", scars_applied: [], institutional_memory_items: "n/a", + collaborative_dynamic: "n/a", rapport_notes: "n/a", + }, + }); + await p1.close(); + const threadsFile = join(h.gitmemDir, "threads.json"); + const local = existsSync(threadsFile) ? JSON.parse(readFileSync(threadsFile, "utf8")) : []; + const localEntry = local.find((t) => t.id === badId); + const inStore = await rest("GET", `gitmem_threads?select=id&thread_id=eq.${badId}`, undefined, { Prefer: "" }).then((r) => r.json()); + const errs2 = []; + const p2 = await startServer({ home: h.home, gitmemDir: h.gitmemDir, ollamaUrl: ollama.url, netlog: probeLog, stderrSink: errs2 }); + await p2.call("session_start", { project: PROJECT, agent_identity: "cli" }); + await p2.close(); + allVenueRequests.push(...venueReqs(readNet(probeLog)).filter((r) => !(r.method === "POST" && /gitmem_threads$/.test(r.path) && r.status === 409))); + const after = existsSync(threadsFile) ? JSON.parse(readFileSync(threadsFile, "utf8")) : []; + writeFileSync(join(outDir, `stderr-${label}-threadsync-restart.log`), errs2.join("")); + writeFileSync(join(outDir, `threads-${label}-threadsync-after.json`), JSON.stringify(after, null, 2)); + const plain = close.replace(/\x1b\[[0-9;]*m/g, ""); + return { + bad_thread: badId, + store_rejected_it: inStore.length === 0, + close_says_partial: /close · PARTIAL/.test(plain) && plain.includes(badId), + kept_locally_pending: !!localEntry && localEntry.sync_pending === true, + carried_into_next_session: after.some((t) => t.id === badId) && + errs2.join("").split("\n").some((l) => l.includes("Carrying forward") && l.includes(badId)), + close_head: plain.split("\n").slice(0, 2).join(" | "), + }; + }; + const threadSync = await threadSyncProbe(); ollama.close(); const reqs = readNet(netlog); @@ -441,6 +491,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, + thread_sync: threadSync, missing_payload_named: missingPayloadNamed, resolve_thread_durable: resolveDurable, close_warns: closeWarns, @@ -648,6 +699,13 @@ if (mode === "flow" && !result.thread_triples_linked) { 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") { + const t = result.thread_sync; + if (!t.store_rejected_it) failureCheck.unexpected.push(`thread-sync probe: the store accepted ${t.bad_thread}; the probe did not exercise a failed write (GIT-117)`); + else if (!(t.close_says_partial && t.kept_locally_pending && t.carried_into_next_session)) { + failureCheck.unexpected.push(`a rejected thread write was not reported PARTIAL / kept pending / carried forward (GIT-117): ${JSON.stringify(t)}`); + } +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } From f8cfe3bf4772b0effb72ed6bcc3bb701f0842fbb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:29:27 -0700 Subject: [PATCH 08/17] ci: macOS hook leg asserts no timeout/gtimeout only; macOS 15 ships jq (GIT-116) macos-latest (macOS 15) has /usr/bin/jq as a system tool, so asserting its absence failed the job. The jq-less path stays covered by hooks-without-jq on Linux; the macOS leg now reports whether stock jq is present. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e2e7f4..4f3c0af 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,14 +85,17 @@ jobs: with: node-version: 20 - - name: Hook tests on a stock macOS PATH (no Homebrew, no timeout, no jq) + - name: Hook tests on a stock macOS PATH (no Homebrew, no timeout) run: | mkdir -p "$RUNNER_TEMP/nodeonly" ln -s "$(command -v node)" "$RUNNER_TEMP/nodeonly/node" STOCK="/usr/bin:/bin:/usr/sbin:/sbin:$RUNNER_TEMP/nodeonly" - for t in timeout gtimeout jq; do + for t in timeout gtimeout; do if PATH="$STOCK" command -v "$t" >/dev/null; then echo "::error::$t is on the stock PATH"; exit 1; fi done + # macOS 15+ ships /usr/bin/jq; older macOS does not. The jq-less path + # is covered by hooks-without-jq (Linux); report which this runner is. + PATH="$STOCK" command -v jq >/dev/null && echo "stock jq present: $(PATH="$STOCK" jq --version)" || echo "stock jq absent" env -i HOME="$HOME" PATH="$STOCK" /bin/bash hooks/tests/test-hooks.sh # GIT-83: the release gate. Customers never run SQL after setup (standing From c903fe58c74836b3cb8b5fd2aa36c18b43888151 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:36:10 -0700 Subject: [PATCH 09/17] fix: a failed index reload keeps what recall has, says so, and retries (GIT-118) loadScarsFromSupabase returned [] on any error with no flag. flushCache (fired after every create_learning, and every 15 min) reinitialised the index and overwrote hook-scars.json with it, logged "N -> 0", and reported success. A failed init was cached as ensureInitialized's promise, so it was never retried. - loadScarsFromSupabase reports ok/error; on failure it offers the last disk vector cache (stale) instead of [], and never as "the store is empty". - A failed reload keeps the index and the hook cache (an empty index takes the stale disk copy) and returns success: false with the cause. - A failed load retries with backoff (30 s, doubling, 10 min cap); ensureInitialized forgets a failed init; an init after a stale start replaces the stale rows (initialize() is a no-op once initialized). - create_learning awaits a refresh and its display says when the recall index could not be refreshed (index_refreshed: false). The post-write refresh uses the per-row delta (GIT-98), not the disk-bypassing flush. +7 startup tests (6 fail on main), +2 create_learning display tests. Three test files mocked startup.js without refreshIndexAfterWrite; the missing export threw inside create_learning's try and every call reported "NOT refreshed" unseen. Their mocks now provide it. Co-Authored-By: Claude Opus 5.5 --- src/services/startup.ts | 154 ++++++++++++++-- src/tools/create-learning.ts | 27 ++- src/types/index.ts | 2 + .../services/startup-reload-failure.test.ts | 170 ++++++++++++++++++ .../unit/tools/create-learning-errors.test.ts | 25 +++ .../create-learning-field-persistence.test.ts | 1 + tests/unit/tools/variants-dev-only.test.ts | 5 +- 7 files changed, 360 insertions(+), 24 deletions(-) create mode 100644 tests/unit/services/startup-reload-failure.test.ts diff --git a/src/services/startup.ts b/src/services/startup.ts index f9bee20..65f6820 100644 --- a/src/services/startup.ts +++ b/src/services/startup.ts @@ -149,7 +149,14 @@ async function deltaSyncFromCache(storeKey: string, fingerprint: StoreFingerprin export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean } = {}): Promise<{ scars: ScarWithEmbedding[]; latestUpdatedAt: string | null; - source: "disk" | "network"; + source: "disk" | "network" | "disk-stale"; + /** + * GIT-118: false when the store could not be read. `scars` is then the last + * disk cache (possibly stale) or [] — never to be mistaken for "the store is + * empty", which is ok: true with no rows. + */ + ok: boolean; + error?: string; }> { const startTime = Date.now(); @@ -167,7 +174,7 @@ export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean `[startup] Vector cache HIT: ${cached.rows.length} learnings from disk in ${Date.now() - startTime}ms ` + `(${cached.bytesOnDisk} bytes on disk, bulk download skipped)` ); - return { scars: cached.rows, latestUpdatedAt: fingerprint.latestUpdatedAt, source: "disk" }; + return { scars: cached.rows, latestUpdatedAt: fingerprint.latestUpdatedAt, source: "disk", ok: true }; } console.error(`[startup] Vector cache MISS: ${cached.reason}`); } @@ -183,7 +190,7 @@ export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean console.error( `[startup] Vector cache HIT (via leader): ${fromLeader.rows.length} learnings in ${Date.now() - startTime}ms, bulk download skipped` ); - return { scars: fromLeader.rows, latestUpdatedAt: fingerprint.latestUpdatedAt, source: "disk" }; + return { scars: fromLeader.rows, latestUpdatedAt: fingerprint.latestUpdatedAt, source: "disk", ok: true }; } console.error("[startup] Leader did not produce a usable cache — downloading"); } @@ -195,7 +202,7 @@ export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean 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" }; + return { scars: synced, latestUpdatedAt: synced[0]?.updated_at || null, source: "network", ok: true }; } } @@ -221,10 +228,18 @@ export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean writeVectorCache(storeKey, fingerprint, learnings); } - return { scars: learnings, latestUpdatedAt, source: "network" }; + return { scars: learnings, latestUpdatedAt, source: "network", ok: true }; } catch (error) { - console.error("[startup] Failed to load learnings:", error); - return { scars: [], latestUpdatedAt: null, source: "network" }; + // GIT-118: this returned [] with no flag, and every caller reinitialised + // the index and the hooks' scar cache with it — one failed request emptied + // recall until the next successful load. Say it failed, and offer the last + // disk cache (possibly stale) for a caller that has nothing better. + const message = error instanceof Error ? error.message : String(error); + console.error("[startup] Failed to load learnings:", message); + const stale = readVectorCacheBase(storeKey); + return stale && stale.length > 0 + ? { scars: stale, latestUpdatedAt: stale[0]?.updated_at || null, source: "disk-stale", ok: false, error: message } + : { scars: [], latestUpdatedAt: null, source: "network", ok: false, error: message }; } finally { releaseLock(); } @@ -312,10 +327,33 @@ export async function initializeGitMem(_project?: Project): Promise<{ try { // Load ALL scars from Supabase (cross-project unified cache) - const { scars, latestUpdatedAt } = await loadScarsFromSupabase(); + const loaded = await loadScarsFromSupabase(); + const { scars, latestUpdatedAt } = loaded; + + if (!loaded.ok) { + // GIT-118: the store could not be read. Serve the last disk cache if + // there is one (stale beats empty), leave the hooks' cache alone, report + // failure so ensureInitialized tries again, and retry in the background. + if (scars.length > 0) await initializeLocalSearch(scars, undefined, latestUpdatedAt || undefined); + scheduleReloadRetry(loaded.error ?? "store unreachable"); + return { + success: false, + scar_count: getLocalVectorSearch().getScarCount(), + elapsed_ms: Date.now() - startTime, + search_mode: "local", + error: `could not load learnings from the store (${loaded.error})${scars.length > 0 ? `; serving ${scars.length} from the disk cache, possibly stale` : ""}`, + }; + } - // Initialize unified local vector search - await initializeLocalSearch(scars, undefined, latestUpdatedAt || undefined); + // Initialize unified local vector search. GIT-118: if an earlier failed + // start left the stale disk copy in the index, replace it — initialize() + // is a no-op on an index that is already initialized. + if (getLocalVectorSearch().getScarCount() > 0) { + await reinitializeLocalSearch(scars, undefined, latestUpdatedAt || undefined); + } else { + await initializeLocalSearch(scars, undefined, latestUpdatedAt || undefined); + } + clearReloadRetry(); // Persist scars to disk for hook processes (no embeddings) persistScarsForHooks(scars); @@ -370,6 +408,9 @@ export async function ensureInitialized(_project?: Project): Promise { startupCompleted.set(UNIFIED_KEY, result.success); if (!result.success) { console.warn(`[startup] GitMem not fully initialized: ${result.error}`); + // GIT-118: a failed init was cached as this promise forever, so no later + // call ever tried again. Forget it; the next call (or the retry) loads. + startupPromises.delete(UNIFIED_KEY); } })(); @@ -653,9 +694,29 @@ async function checkThreadHealth(project?: Project): Promise } /** - * Flush and reload the cache + * Flush and reload the cache. A flush is the user saying "I do not trust what + * you have": the disk cache is bypassed (GIT-98) and rewritten. */ export async function flushCache(_project?: Project): Promise { + return reloadIndex({ bypassDiskCache: true }); +} + +/** + * GIT-118: bring the index up to date after a write (create_learning). Uses + * the per-row delta (GIT-98), so a new learning costs its own row rather than + * the whole index — cheap enough for the tool to await and report. + */ +export async function refreshIndexAfterWrite(_project?: Project): Promise { + return reloadIndex({ bypassDiskCache: false }); +} + +/** + * Reload the index from the store — or, if the store cannot be read, keep + * what is there (GIT-118). A failed load used to reinitialise the index and + * overwrite the hooks' scar cache with [] and report success, so one failed + * request emptied recall until the next successful load. + */ +async function reloadIndex(options: { bypassDiskCache: boolean }): Promise { const startTime = Date.now(); const config = getConfig(); @@ -672,15 +733,32 @@ export async function flushCache(_project?: Project): Promise const previousCount = getLocalVectorSearch().getScarCount(); try { - // Load ALL fresh scars (cross-project). A flush is the user saying "I do not - // trust what you have" — bypass the disk cache (GIT-98) and rewrite it. - const { scars, latestUpdatedAt } = await loadScarsFromSupabase({ bypassDiskCache: true }); + const loaded = await loadScarsFromSupabase({ bypassDiskCache: options.bypassDiskCache }); + + if (!loaded.ok) { + // Keep the index and the hooks' cache. Only an EMPTY index takes the + // stale disk copy: it has nothing better to lose. + if (previousCount === 0 && loaded.scars.length > 0) { + await reinitializeLocalSearch(loaded.scars, undefined, loaded.latestUpdatedAt || undefined); + } + scheduleReloadRetry(loaded.error ?? "store unreachable"); + const kept = getLocalVectorSearch().getScarCount(); + console.error(`[cache] Reload failed (${loaded.error}); kept ${kept} scars, hook cache unchanged`); + return { + success: false, + previous_scar_count: previousCount, + new_scar_count: kept, + elapsed_ms: Date.now() - startTime, + error: loaded.error ?? "store unreachable", + }; + } // Reinitialize the unified index - await reinitializeLocalSearch(scars, undefined, latestUpdatedAt || undefined); + await reinitializeLocalSearch(loaded.scars, undefined, loaded.latestUpdatedAt || undefined); // Update disk cache for hook processes too - persistScarsForHooks(scars); + persistScarsForHooks(loaded.scars); + clearReloadRetry(); const newCount = getLocalVectorSearch().getScarCount(); const elapsed = Date.now() - startTime; @@ -697,17 +775,59 @@ export async function flushCache(_project?: Project): Promise const elapsed = Date.now() - startTime; const errorMsg = error instanceof Error ? error.message : String(error); console.error("[cache] Flush failed:", errorMsg); + scheduleReloadRetry(errorMsg); return { success: false, previous_scar_count: previousCount, - new_scar_count: previousCount, + new_scar_count: getLocalVectorSearch().getScarCount(), elapsed_ms: elapsed, error: errorMsg, }; } } +// --------------------------------------------------------------------------- +// GIT-118: retry a failed load with backoff (30 s, doubling, capped at 10 min) +// --------------------------------------------------------------------------- + +const RELOAD_RETRY_BASE_MS = 30_000; +const RELOAD_RETRY_MAX_MS = 10 * 60_000; +let reloadRetryTimer: ReturnType | null = null; +let reloadRetryAttempt = 0; + +function scheduleReloadRetry(reason: string): void { + if (reloadRetryTimer) return; // one pending retry at a time + const delay = Math.min(RELOAD_RETRY_BASE_MS * 2 ** reloadRetryAttempt, RELOAD_RETRY_MAX_MS); + reloadRetryAttempt++; + console.error(`[startup] Index reload failed (${reason}); retry ${reloadRetryAttempt} in ${Math.round(delay / 1000)} s`); + reloadRetryTimer = setTimeout(() => { + reloadRetryTimer = null; + reloadIndex({ bypassDiskCache: false }) + .then((r) => { + if (r.success) startupCompleted.set(UNIFIED_KEY, true); + }) + .catch(() => { /* reloadIndex reschedules itself */ }); + }, delay); + reloadRetryTimer.unref?.(); +} + +function clearReloadRetry(): void { + if (reloadRetryTimer) clearTimeout(reloadRetryTimer); + reloadRetryTimer = null; + reloadRetryAttempt = 0; +} + +/** For tests and health. */ +export function getReloadRetryState(): { pending: boolean; attempt: number } { + return { pending: reloadRetryTimer !== null, attempt: reloadRetryAttempt }; +} + +/** For tests. */ +export function resetReloadRetry(): void { + clearReloadRetry(); +} + /** * Check if cache needs refresh based on TTL or staleness * Returns true if a refresh is recommended diff --git a/src/tools/create-learning.ts b/src/tools/create-learning.ts index db9b929..ac36a4e 100644 --- a/src/tools/create-learning.ts +++ b/src/tools/create-learning.ts @@ -13,7 +13,7 @@ import * as supabase from "../services/supabase-client.js"; import { embed, isEmbeddingAvailable } from "../services/embedding.js"; import { getAgentIdentity } from "../services/agent-detection.js"; import { wrapDisplay, TYPE, SEV } from "../services/display-protocol.js"; -import { flushCache } from "../services/startup.js"; +import { refreshIndexAfterWrite } from "../services/startup.js"; import { writeTriplesForLearning } from "../services/triple-writer.js"; import { generateVariantsForScar } from "../services/variant-generation.js"; import { getEffectTracker } from "../services/effect-tracker.js"; @@ -182,6 +182,9 @@ export async function createLearning( learningData.severity = params.severity || "medium"; } + // GIT-118: set when the recall index could not be refreshed after the write. + let indexNotRefreshed: string | null = null; + try { let embeddingGenerated = false; const breakdown: PerformanceBreakdown = {}; @@ -272,11 +275,17 @@ export async function createLearning( ); } - // Invalidate local cache so next recall picks up the new learning + // Refresh the local index so the next recall sees the new learning. + // GIT-118: awaited and reported. It was fire-and-forget, and a failed + // reload emptied the index while create_learning said nothing. The + // learning itself is saved either way; only recall's view of it waits. const project = (params.project || getProject() || "default") as Project; - flushCache(project).catch((err) => { - console.warn("[create_learning] Cache invalidation failed (non-fatal):", err); - }); + try { + const refreshed = await refreshIndexAfterWrite(project); + if (!refreshed.success) indexNotRefreshed = refreshed.error ?? "reload failed"; + } catch (err) { + indexNotRefreshed = err instanceof Error ? err.message : String(err); + } } else { // Free tier: Store locally without embedding console.error(`[create_learning] Storing locally: ${learningId}`); @@ -319,7 +328,13 @@ export async function createLearning( learning_id: learningId, embedding_generated: embeddingGenerated, performance: perfData, - display: wrapDisplay(`${te} Created ${params.learning_type}: "${params.title}"\n${se}ID: ${learningId}`), + ...(indexNotRefreshed !== null && { index_refreshed: false }), + display: wrapDisplay( + `${te} Created ${params.learning_type}: "${params.title}"\n${se}ID: ${learningId}` + + (indexNotRefreshed !== null + ? `\nRecall index NOT refreshed (${indexNotRefreshed}). The learning is saved; recall will find it after the next successful reload (retrying in the background).` + : "") + ), }; } catch (error) { const errorMsg = error instanceof Error ? error.message : String(error); diff --git a/src/types/index.ts b/src/types/index.ts index 6de5c9b..c372436 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -346,6 +346,8 @@ export interface CreateLearningParams { export interface CreateLearningResult extends WriteResult { learning_id: string; embedding_generated: boolean; + /** GIT-118: false when the recall index could not be refreshed after the write. */ + index_refreshed?: boolean; /** Error details when success=false */ errors?: string[]; display?: string; diff --git a/tests/unit/services/startup-reload-failure.test.ts b/tests/unit/services/startup-reload-failure.test.ts new file mode 100644 index 0000000..9a2b1a9 --- /dev/null +++ b/tests/unit/services/startup-reload-failure.test.ts @@ -0,0 +1,170 @@ +/** + * GIT-118: a failed load never empties recall. + * + * loadScarsFromSupabase returned [] on any error with no flag, and every + * caller reinitialised the index and overwrote the hooks' scar cache with it. + * create_learning triggers a reload after every write, so one failed request + * emptied recall — and reported success — until the next good load, which a + * failed init never retried. + * + * Store mocked at the supabase-client boundary; the disk vector cache, the + * local index and the hook cache are real (temp GITMEM_DIR). Each test gets a + * fresh module graph, i.e. a fresh process. + */ +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 store = vi.hoisted(() => ({ + rows: [] as Array<{ id: string; title: string; description: string; severity: string; learning_type: string; updated_at: string; embedding: number[] }>, + down: false, + calls: { bulk: 0, manifest: 0 }, +})); +const active = () => [...store.rows].sort((a, b) => b.updated_at.localeCompare(a.updated_at)); +const fail = () => { throw new TypeError("fetch failed: ECONNRESET"); }; + +vi.mock("../../../src/services/supabase-client.js", () => ({ + isConfigured: () => true, + getSupabaseUrl: () => "https://venue.example.supabase.co", + getLearningsFingerprint: vi.fn(async () => { + if (store.down) return { count: -1, latestUpdatedAt: null }; // the real one's failure shape + const r = active(); + return { count: r.length, latestUpdatedAt: r[0]?.updated_at ?? null }; + }), + loadLearningsManifest: vi.fn(async () => { store.calls.manifest++; if (store.down) fail(); return active().map(({ id, updated_at }) => ({ id, updated_at })); }), + loadLearningsByIds: vi.fn(async (ids: string[]) => { if (store.down) fail(); return active().filter((r) => ids.includes(r.id)); }), + loadScarsWithEmbeddings: vi.fn(async () => { store.calls.bulk++; if (store.down) fail(); return active().map((r) => ({ ...r })); }), + directQuery: vi.fn(async () => []), +})); + +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: 1536 }, (_, k) => Math.sin(i + k)); // the index keeps only 1536-dim rows +const seed = (n: number, at = 0) => { + store.rows = Array.from({ length: n }, (_, i) => ({ + id: `id-${String(i).padStart(3, "0")}`, title: `scar ${i}`, description: "d", severity: "medium", + learning_type: "scar", updated_at: ts(at + i), embedding: vec(i), + })); +}; +const hookCache = () => JSON.parse(fs.readFileSync(path.join(dir, "cache", "hook-scars.json"), "utf-8")); + +async function freshProcess() { + vi.resetModules(); + const startup = await import("../../../src/services/startup.js"); + const lvs = await import("../../../src/services/local-vector-search.js"); + return { startup, count: () => lvs.getLocalVectorSearch().getScarCount() }; +} + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git118-")); + vi.stubEnv("GITMEM_DIR", dir); + vi.stubEnv("GITMEM_TIER", "pro"); + vi.stubEnv("SUPABASE_URL", "https://venue.example.supabase.co"); + vi.stubEnv("SUPABASE_SERVICE_ROLE_KEY", "k"); + vi.stubEnv("GITMEM_SEARCH_MODE", "local"); + vi.stubEnv("GITMEM_VECTOR_DISK_CACHE", ""); + store.down = false; + store.calls = { bulk: 0, manifest: 0 }; + seed(20); +}); + +afterEach(async () => { + const { resetReloadRetry } = await import("../../../src/services/startup.js"); + resetReloadRetry?.(); + vi.useRealTimers(); + vi.unstubAllEnvs(); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +describe("GIT-118: a failed reload keeps what recall has", () => { + it("flush with the store down: success false, index and hook cache kept", async () => { + const { startup, count } = await freshProcess(); + expect((await startup.initializeGitMem()).success).toBe(true); + expect(count()).toBe(20); + expect(hookCache()).toHaveLength(20); + + store.down = true; + const r = await startup.flushCache(); + + expect(r.success).toBe(false); + expect(r.error).toMatch(/ECONNRESET/); + expect(count()).toBe(20); + expect(hookCache()).toHaveLength(20); + }); + + it("the post-write refresh uses the per-row delta, not a full download", async () => { + const { startup } = await freshProcess(); + await startup.initializeGitMem(); + store.calls = { bulk: 0, manifest: 0 }; + store.rows.push({ ...store.rows[0], id: "id-new", updated_at: ts(999) }); + + const r = await startup.refreshIndexAfterWrite(); + expect(r.success).toBe(true); + expect(r.new_scar_count).toBe(21); + expect(store.calls).toEqual({ bulk: 0, manifest: 1 }); + }); + + it("cold start with the store down serves the disk cache (stale) and says it failed", async () => { + const first = await freshProcess(); + await first.startup.initializeGitMem(); // leaves a disk cache behind + + store.down = true; + const { startup, count } = await freshProcess(); + const r = await startup.initializeGitMem(); + + expect(r.success).toBe(false); + expect(r.error).toMatch(/serving 20 from the disk cache/); + expect(count()).toBe(20); + }); + + it("a genuinely empty store is success with 0, not a failure", async () => { + store.rows = []; + const { startup, count } = await freshProcess(); + const r = await startup.initializeGitMem(); + expect(r.success).toBe(true); + expect(count()).toBe(0); + }); + + it("a failed load retries with backoff and recovers when the store does", async () => { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + const { startup, count } = await freshProcess(); + await startup.initializeGitMem(); + store.down = true; + await startup.flushCache(); + expect(startup.getReloadRetryState()).toEqual({ pending: true, attempt: 1 }); + + store.down = false; + seed(25, 100); + await vi.advanceTimersByTimeAsync(30_000); + + expect(count()).toBe(25); + expect(startup.getReloadRetryState()).toEqual({ pending: false, attempt: 0 }); + }); + + it("the second retry waits twice as long", async () => { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + const { startup } = await freshProcess(); + await startup.initializeGitMem(); + store.down = true; + await startup.flushCache(); + await vi.advanceTimersByTimeAsync(30_000); // retry 1 fails, schedules retry 2 at 60 s + expect(startup.getReloadRetryState()).toEqual({ pending: true, attempt: 2 }); + store.calls = { bulk: 0, manifest: 0 }; + await vi.advanceTimersByTimeAsync(59_000); + expect(store.calls.manifest + store.calls.bulk).toBe(0); + await vi.advanceTimersByTimeAsync(1_000); + expect(store.calls.manifest + store.calls.bulk).toBeGreaterThan(0); + }); + + it("ensureInitialized tries again after a failed init (it cached the failure forever)", async () => { + store.down = true; + const { startup, count } = await freshProcess(); + await startup.ensureInitialized(); + expect(count()).toBe(0); + + store.down = false; + await startup.ensureInitialized(); + expect(count()).toBe(20); + }); +}); diff --git a/tests/unit/tools/create-learning-errors.test.ts b/tests/unit/tools/create-learning-errors.test.ts index 3db0a6b..a6e116d 100644 --- a/tests/unit/tools/create-learning-errors.test.ts +++ b/tests/unit/tools/create-learning-errors.test.ts @@ -40,6 +40,9 @@ vi.mock("../../../src/services/display-protocol.js", () => ({ vi.mock("../../../src/services/startup.js", () => ({ flushCache: vi.fn(() => Promise.resolve()), + // GIT-118: create_learning awaits this; omitting it made every call report + // "index NOT refreshed" through a swallowed mock-export error. + refreshIndexAfterWrite: vi.fn(() => Promise.resolve({ success: true, previous_scar_count: 0, new_scar_count: 1, elapsed_ms: 1 })), })); vi.mock("../../../src/services/triple-writer.js", () => ({ @@ -190,3 +193,25 @@ describe("create_learning: DB error surfacing", () => { expect(result.errors).toBeUndefined(); }); }); + +// GIT-118: the post-write index refresh is awaited and its failure reported. +describe("create_learning: recall index refresh after the write (GIT-118)", () => { + it("says so in the display when the index could not be refreshed; the learning is still saved", async () => { + const startup = await import("../../../src/services/startup.js"); + vi.mocked(startup.refreshIndexAfterWrite).mockResolvedValueOnce({ + success: false, previous_scar_count: 5, new_scar_count: 5, elapsed_ms: 1, error: "fetch failed: ECONNRESET", + }); + const result = await createLearning({ + learning_type: "win", title: "t", description: "d", + } as never); + expect(result.success).toBe(true); + expect(result.index_refreshed).toBe(false); + expect(result.display).toMatch(/Recall index NOT refreshed \(fetch failed: ECONNRESET\)\. The learning is saved/); + }); + + it("says nothing extra when the refresh succeeds", async () => { + const result = await createLearning({ learning_type: "win", title: "t", description: "d" } as never); + expect(result.index_refreshed).toBeUndefined(); + expect(result.display).not.toMatch(/NOT refreshed/); + }); +}); diff --git a/tests/unit/tools/create-learning-field-persistence.test.ts b/tests/unit/tools/create-learning-field-persistence.test.ts index 6275ade..97a0267 100644 --- a/tests/unit/tools/create-learning-field-persistence.test.ts +++ b/tests/unit/tools/create-learning-field-persistence.test.ts @@ -55,6 +55,7 @@ vi.mock("../../../src/services/display-protocol.js", () => ({ vi.mock("../../../src/services/startup.js", () => ({ flushCache: vi.fn(() => Promise.resolve()), + refreshIndexAfterWrite: vi.fn(() => Promise.resolve({ success: true, previous_scar_count: 0, new_scar_count: 1, elapsed_ms: 1 })), // GIT-118 })); vi.mock("../../../src/services/triple-writer.js", () => ({ diff --git a/tests/unit/tools/variants-dev-only.test.ts b/tests/unit/tools/variants-dev-only.test.ts index 0bc6bc7..8f7c244 100644 --- a/tests/unit/tools/variants-dev-only.test.ts +++ b/tests/unit/tools/variants-dev-only.test.ts @@ -49,7 +49,10 @@ vi.mock("../../../src/services/embedding.js", () => ({ embed: async () => null, isEmbeddingAvailable: () => false, })); -vi.mock("../../../src/services/startup.js", () => ({ flushCache: async () => undefined })); +vi.mock("../../../src/services/startup.js", () => ({ + flushCache: async () => undefined, + refreshIndexAfterWrite: async () => ({ success: true, previous_scar_count: 0, new_scar_count: 1, elapsed_ms: 1 }), // GIT-118 +})); vi.mock("../../../src/services/triple-writer.js", () => ({ writeTriplesForLearning: async () => 0 })); const { resetTier, hasVariants } = await import("../../../src/services/tier.js"); From 848f8c934974aab6e1abb3b030eb1603aa43f7e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:36:10 -0700 Subject: [PATCH 10/17] test: venue flow fails the index reads through a proxy and checks recall survives (GIT-118) A forwarding proxy in front of the venue fails GET gitmem_learnings* on demand while writes pass. On 1.11.0: "Flushed and reloaded: 5 -> 0", create_learning silent, recall empty; exit 1. Co-Authored-By: Claude Opus 5.5 --- tests/e2e/blank-supabase.mjs | 60 +++++++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 0a16761..3cfc90e 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -200,7 +200,7 @@ function isolatedHome(tag) { return { home, gitmemDir }; } const STRIP = /^(OPENAI_API_KEY|OPENROUTER_API_KEY|GITMEM_|SUPABASE_|OLLAMA_|NODE_OPTIONS$|CLAUDE_CODE_ENTRYPOINT$)/; -async function startServer({ home, gitmemDir, ollamaUrl, netlog, stderrSink }) { +async function startServer({ home, gitmemDir, ollamaUrl, netlog, stderrSink, extraEnv = {} }) { const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => !STRIP.test(k))); Object.assign(env, { HOME: home, @@ -216,6 +216,7 @@ async function startServer({ home, gitmemDir, ollamaUrl, netlog, stderrSink }) { GITMEM_NETLOG: netlog, NODE_OPTIONS: `--import=${pathToFileURL(NETLOG_PRELOAD).href}`, NO_COLOR: "1", + ...extraEnv, }); const transport = new StdioClientTransport({ command: process.execPath, args: [SERVER], env, cwd: home, stderr: "pipe" }); transport.stderr?.on("data", (d) => stderrSink.push(d.toString())); @@ -422,6 +423,56 @@ async function flow() { const relevance = relevanceRows.map((m) => ({ tool: m.tool_name, memories_applied: m.metadata.memories_applied, memory_relevance: m.metadata.memory_relevance })); await waitQuiet(netlog, { minMs: 1000, quietMs: 2000 }); await srv.close(); + + // GIT-118: the store fails the index READS while writes still land. A + // forwarding proxy in front of the venue fails GET gitmem_learnings… on + // demand. create_learning must save and say its index refresh failed; a + // recall right after must still find the seeded scars (index kept). + const reloadFailureProbe = async () => { + let failIndexReads = false; + const proxy = createServer(async (req, res) => { + const chunks = []; + for await (const c of req) chunks.push(c); + if (failIndexReads && req.method === "GET" && /^\/rest\/v1\/gitmem_learnings(\?|$)/.test(req.url)) { + res.writeHead(503, { "content-type": "application/json" }); + res.end('{"message":"injected by the venue driver (GIT-118)"}'); + return; + } + const headers = Object.fromEntries(Object.entries(req.headers).filter(([k]) => !["host", "connection", "content-length", "accept-encoding"].includes(k))); + const upstream = await fetch(`${SUPABASE_URL}${req.url}`, { method: req.method, headers, body: chunks.length ? Buffer.concat(chunks) : undefined }); + const body = Buffer.from(await upstream.arrayBuffer()); + const out = {}; + upstream.headers.forEach((v, k) => { if (!["content-encoding", "content-length", "transfer-encoding", "connection"].includes(k)) out[k] = v; }); + res.writeHead(upstream.status, out); + res.end(body); + }); + await new Promise((r) => proxy.listen(0, "127.0.0.1", r)); + const h = isolatedHome(`${label}-reload`); + const probeLog = join(outDir, `netlog-${label}-reload.jsonl`); + writeFileSync(probeLog, ""); + const errs = []; + const p = await startServer({ home: h.home, gitmemDir: h.gitmemDir, ollamaUrl: ollama.url, netlog: probeLog, stderrSink: errs, + extraEnv: { SUPABASE_URL: `http://127.0.0.1:${proxy.address().port}`, GITMEM_SEARCH_MODE: "local" } }); + await p.call("session_start", { project: PROJECT, agent_identity: "cli", force: true }); + for (let i = 0; i < 60 && !/GitMem initialized: [1-9]\d* scars/.test(errs.join("")); i++) await new Promise((r) => setTimeout(r, 500)); + failIndexReads = true; + const created = await p.call("create_learning", { learning_type: "win", title: `Reload probe win (${label}) ${randomUUID().slice(0, 8)}`, description: "written while the index reads fail", project: PROJECT }); + // Let any background reload the write started finish before the next + // prompt's recall (1.11.0 did not await it; a user's next prompt comes later). + await waitQuiet(probeLog, { minMs: 3000, quietMs: 2500 }); + const recall = await p.call("recall", { plan: "Supabase migrations must be dry-run before push", project: PROJECT, match_count: 3 }); + failIndexReads = false; + await p.close(); + proxy.close(); + writeFileSync(join(outDir, `stderr-${label}-reload.log`), errs.join("")); + return { + learning_saved: /Created win/.test(created), + create_says_not_refreshed: /Recall index NOT refreshed/.test(created), + recall_still_finds_seeded_scar: /Supabase migrations must be dry-run/.test(recall), + create_head: created.split("\n").slice(0, 3).join(" | ").slice(0, 240), + }; + }; + const reloadFailure = await reloadFailureProbe(); ollama.close(); const reqs = readNet(netlog); @@ -441,6 +492,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, + reload_failure: reloadFailure, missing_payload_named: missingPayloadNamed, resolve_thread_durable: resolveDurable, close_warns: closeWarns, @@ -648,6 +700,12 @@ if (mode === "flow" && !result.thread_triples_linked) { 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") { + const r = result.reload_failure; + if (!(r.learning_saved && r.create_says_not_refreshed && r.recall_still_finds_seeded_scar)) { + failureCheck.unexpected.push(`a failed index reload was hidden or emptied recall (GIT-118): ${JSON.stringify(r)}`); + } +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } From e94f7110f789431b9049b2afac2acfead16ae521 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:44:03 -0700 Subject: [PATCH 11/17] fix: archive_learning resolves prefixes by UUID range; 0-row PATCH is not durable (GIT-119) archive_learning sent id=like.% against a uuid column, which Postgres rejects (42883), so prefix archiving failed on every store. The prefix now resolves from the local index, or from the store as a UUID range when cold. directPatch returns { count, rows }. PostgREST answers 200 [] when a PATCH matched nothing; all 8 callers now treat count 0 as not durable (archive of a nonexistent id, session embedding, superseded mark, thread resolve/touch/ auto-archive, relevance update, transcript path). Tests: +12 (9 parameterized zero-row cases, 3 archive). Venue driver gains a prefix archive of a row the index has not seen, and a nonexistent full UUID. Co-Authored-By: Claude Opus 5.5 --- src/services/local-vector-search.ts | 5 + src/services/metrics.ts | 4 +- src/services/supabase-client.ts | 20 ++- src/services/thread-supabase.ts | 17 ++- src/tools/archive-learning.ts | 49 ++++++- src/tools/session-close.ts | 15 +- src/tools/session-start.ts | 7 +- tests/e2e/blank-supabase.mjs | 29 ++++ .../services/directpatch-zero-rows.test.ts | 131 ++++++++++++++++++ tests/unit/tools/archive-learning.test.ts | 62 ++++++++- 10 files changed, 315 insertions(+), 24 deletions(-) create mode 100644 tests/unit/services/directpatch-zero-rows.test.ts diff --git a/src/services/local-vector-search.ts b/src/services/local-vector-search.ts index 99dbea1..d34c110 100644 --- a/src/services/local-vector-search.ts +++ b/src/services/local-vector-search.ts @@ -211,6 +211,11 @@ export class LocalVectorSearch { return this.scars.length; } + /** Ids of the indexed scars (GIT-119: prefix resolution without a query). */ + getScarIds(): string[] { + return this.scars.map((s) => s.scar.id); + } + /** * Search for scars similar to the query * diff --git a/src/services/metrics.ts b/src/services/metrics.ts index 16eac35..bafda1b 100644 --- a/src/services/metrics.ts +++ b/src/services/metrics.ts @@ -443,9 +443,11 @@ export async function updateRelevanceData( ); if (applied.length === 0 && Object.keys(relevance).length === 0) continue; - await supabase.directPatch("gitmem_query_metrics", { id: `eq.${metric.id}` }, { + const patched = await supabase.directPatch("gitmem_query_metrics", { id: `eq.${metric.id}` }, { metadata: { ...(metric.metadata ?? {}), memories_applied: applied, memory_relevance: relevance }, }); + // GIT-119: 0 rows = relevance not recorded; throw so the tracker counts it. + if (patched.count === 0) throw new Error(`relevance for metrics row ${metric.id} updated no row`); } }); } diff --git a/src/services/supabase-client.ts b/src/services/supabase-client.ts index ca1855a..cd206f3 100644 --- a/src/services/supabase-client.ts +++ b/src/services/supabase-client.ts @@ -473,11 +473,22 @@ export async function directUpsert( * @param filters PostgREST filter to identify the row(s) to update * @param data Fields to update (partial — omitted columns stay unchanged) */ +/** + * GIT-119: what a PATCH changed. PostgREST answers 200 with [] when the filter + * matched nothing (a wrong id, a row not created yet, RLS), so a PATCH that + * returns is not evidence of a write: `count` is. All callers treat 0 as not + * durable. + */ +export interface PatchResult { + count: number; + rows: T[]; +} + export async function directPatch( table: string, filters: Record, data: Record -): Promise { +): Promise> { if (!isConfigured()) { throw new Error("Supabase not configured"); } @@ -507,7 +518,8 @@ export async function directPatch( throw new Error(`Supabase patch error: ${response.status} - ${text.slice(0, 200)}`); } - return response.json() as Promise; + const rows = ((await response.json()) as T[]) || []; + return { count: Array.isArray(rows) ? rows.length : 0, rows: Array.isArray(rows) ? rows : [] }; } /** @@ -888,10 +900,12 @@ export async function saveTranscript( // Update the session record with transcript_path (direct REST API) let patch_warning: string | undefined; try { - await directPatch(getTableName("sessions"), + const patched = await directPatch(getTableName("sessions"), { id: sessionId }, { transcript_path: path } ); + // GIT-119: 0 rows = no session row to update, not a success. + if (patched.count === 0) throw new Error(`no session row ${sessionId} to update`); } catch (error) { // File is saved; session record update failed — warn, don't fail const msg = error instanceof Error ? error.message : String(error); diff --git a/src/services/thread-supabase.ts b/src/services/thread-supabase.ts index 1494521..4b22c88 100644 --- a/src/services/thread-supabase.ts +++ b/src/services/thread-supabase.ts @@ -261,7 +261,12 @@ export async function resolveThreadInSupabase( patchData.resolved_by_session = options.resolvedBySession; } - await supabase.directPatch(getTableName("threads"), { id: uuid }, patchData); + const patched = await supabase.directPatch(getTableName("threads"), { id: uuid }, patchData); + // GIT-119: 0 rows = not resolved. + if (patched.count === 0) { + console.error(`[thread-supabase] Resolve of ${threadId} updated no row`); + return false; + } console.error(`[thread-supabase] Resolved thread ${threadId} in Supabase`); return true; } catch (error) { @@ -551,13 +556,15 @@ export async function touchThreadsInSupabase( delete metadata.dormant_since; } - await supabase.directPatch(getTableName("threads"), { id: row.id }, { + const patched = await supabase.directPatch(getTableName("threads"), { id: row.id }, { touch_count: newTouchCount, last_touched_at: nowIso, vitality_score: vitality.vitality_score, status: lifecycle_status, metadata, }); + // GIT-119: 0 rows = not touched. + if (patched.count === 0) throw new Error(`touch of ${threadId} updated no row`); } catch (error) { console.error(`[thread-supabase] Failed to touch thread ${threadId}:`, error instanceof Error ? error.message : error); // Continue with other threads @@ -752,10 +759,12 @@ export async function archiveDormantThreads( const daysDormant = (now.getTime() - dormantStart.getTime()) / (1000 * 60 * 60 * 24); if (daysDormant >= dormantDays) { - await supabase.directPatch(getTableName("threads"), { id: row.id }, { + const patched = await supabase.directPatch(getTableName("threads"), { id: row.id }, { status: "archived", }); - archived_ids.push(row.thread_id); + // GIT-119: count only rows the store actually archived. + if (patched.count > 0) archived_ids.push(row.thread_id); + else console.error(`[thread-supabase] Archive of ${row.thread_id} updated no row`); } } diff --git a/src/tools/archive-learning.ts b/src/tools/archive-learning.ts index eeb020e..cb4dd3a 100644 --- a/src/tools/archive-learning.ts +++ b/src/tools/archive-learning.ts @@ -14,6 +14,7 @@ import { supportedColumns } from "../services/store-columns.js"; import { hasSupabase, getTableName } from "../services/tier.js"; import { getStorage } from "../services/storage.js"; import { flushCache } from "../services/startup.js"; +import { getLocalVectorSearch } from "../services/local-vector-search.js"; import { Timer } from "../services/metrics.js"; import { wrapDisplay } from "../services/display-protocol.js"; import { writeResult, notStored } from "../services/write-result.js"; @@ -37,6 +38,25 @@ export interface ArchiveLearningResult extends WriteResult { } const FULL_UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** + * GIT-119: the UUIDs a hex prefix covers, as an inclusive range. uuid values + * order bytewise, i.e. by their hex digits, so prefix+"000…" .. prefix+"fff…" + * is exactly the set that starts with the prefix. + */ +export function uuidRangeForPrefix(prefix: string): [string, string] { + const fmt = (hex: string) => `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}`; + return [fmt(prefix.padEnd(32, "0")), fmt(prefix.padEnd(32, "f"))]; +} + +/** Ids in the warm local vector index (empty when it is not loaded). */ +function localIndexIds(): string[] { + try { + return getLocalVectorSearch().getScarIds(); + } catch { + return []; + } +} const HEX_PREFIX_RE = /^[0-9a-f]{4,32}$/i; /** @@ -58,10 +78,19 @@ async function resolveIdPrefix(input: string): Promise<{ id: string } | { error: const prefix = input.toLowerCase(); if (hasSupabase() && isConfigured()) { - // Supabase: use PostgREST like filter + // GIT-119: this sent id=like.% — Postgres has no LIKE on a UUID + // column (42883 "operator does not exist: uuid ~~ unknown"), so prefix + // archiving failed on every store. Resolve from the local index when it + // has the answer, else ask the store for the UUID range the prefix spans. + const local = localIndexIds().filter((id) => id.replace(/-/g, "").startsWith(prefix)); + if (local.length === 1) return { id: local[0] }; + if (local.length > 1) { + return { error: `Ambiguous prefix "${prefix}" — matches multiple learnings: ${local.slice(0, 2).map((m) => m.slice(0, 12) + "…").join(", ")}` }; + } + const [low, high] = uuidRangeForPrefix(prefix); const matches = await directQuery<{ id: string }>(getTableName("learnings"), { select: "id", - filters: { id: `like.${prefix}%` }, + filters: { and: `(id.gte.${low},id.lte.${high})` }, limit: 2, }); @@ -130,10 +159,24 @@ export async function archiveLearning(params: ArchiveLearningParams): Promise { + const patched = await supabase.directPatch(getTableName("sessions"), { id: sessionId }, { embedding: embeddingJson }); + if (patched.count === 0) throw new Error(`session ${sessionId.slice(0, 8)}: embedding PATCH updated no row`); +} + /** * GIT-21: Clean up all session files for a closed session. * Unregisters from registry, deletes per-session directory, and removes legacy file. @@ -1566,11 +1575,7 @@ export async function sessionClose( const embeddingVector = await embed(embeddingText); if (embeddingVector) { const embeddingJson = JSON.stringify(embeddingVector); - // Update session with embedding (PATCH, not upsert — row already exists) - await supabase.directPatch(getTableName("sessions"), - { id: sessionId }, - { embedding: embeddingJson } - ); + await saveSessionEmbedding(sessionId, embeddingJson); console.error("[session_close] Embedding saved to session"); // Phase 5: Implicit thread detection (chained after embedding) diff --git a/src/tools/session-start.ts b/src/tools/session-start.ts index b177c3c..dad1b95 100644 --- a/src/tools/session-start.ts +++ b/src/tools/session-start.ts @@ -478,7 +478,8 @@ async function createSessionRecord( * Fire-and-forget — failures logged but don't block session_start. * Only sets close_compliance if it's currently null (truly abandoned). */ -async function markSessionSuperseded(oldSessionId: string, newSessionId: string): Promise { +/** @internal exported for the GIT-119 zero-row test. */ +export async function markSessionSuperseded(oldSessionId: string, newSessionId: string): Promise { if (!hasSupabase()) return; // Free tier: no remote session tracking try { // Check if session already has close_compliance (was properly closed) @@ -490,7 +491,7 @@ async function markSessionSuperseded(oldSessionId: string, newSessionId: string) // Already closed — don't overwrite return; } - await supabase.directPatch(getTableName("sessions"), + const patched = await supabase.directPatch(getTableName("sessions"), { id: oldSessionId }, { close_compliance: { @@ -500,6 +501,8 @@ async function markSessionSuperseded(oldSessionId: string, newSessionId: string) }, } ); + // GIT-119: 0 rows = the superseded row isn't there; not marked. + if (patched.count === 0) throw new Error(`no session row ${oldSessionId.slice(0, 8)} to mark superseded`); console.error(`[session_start] Marked session ${oldSessionId.slice(0, 8)} as superseded by ${newSessionId.slice(0, 8)}`); } catch (error) { console.error(`[session_start] Failed to mark session ${oldSessionId.slice(0, 8)} as superseded:`, error); diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 0a16761..1d61757 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -400,6 +400,25 @@ async function flow() { await waitQuiet(netlog, { minMs: 2000, quietMs: 3000 }); const health = await step("health", { failure_limit: 20 }); + // GIT-119: archive_learning by id prefix sent id=like.% against a + // uuid column (42883), and a full UUID that matched no row still reported + // "Archived". The row below is inserted after the server loaded its index, + // so the prefix must be resolved by the store (UUID range), not locally. + const archiveRowId = randomUUID(); + await rest("POST", "gitmem_learnings", [{ + id: archiveRowId, learning_type: "scar", title: `Driver archive target (${label})`, + description: "Inserted after startup so archive_learning resolves its prefix from the store.", + severity: "low", counter_arguments: ["You might think it is permanent — but it is archived here."], + project: PROJECT, is_active: true, + }]); + const archivePrefix = archiveRowId.slice(0, 8); + const ap = await step("archive_learning", { id: archivePrefix, reason: "venue driver (GIT-119)" }); + const archivedRow = await rest("GET", `gitmem_learnings?select=is_active&id=eq.${archiveRowId}`, undefined, { Prefer: "" }).then((r) => r.json()); + const archivePrefixDurable = /Archived learning/.test(ap) && archivedRow[0]?.is_active === false; + const ghostId = "00000000-0000-4000-8000-00000000d119"; + const ag = await step("archive_learning", { id: ghostId, reason: "venue driver (GIT-119)" }); + const archiveGhostHonest = !/Archived learning/.test(ag) && /nothing was archived|not found|No learning/i.test(ag); + // 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)); @@ -443,6 +462,10 @@ async function flow() { session_close_persisted: closedRows.length === 1 && closedRows[0].closing_reflection != null, missing_payload_named: missingPayloadNamed, resolve_thread_durable: resolveDurable, + archive_prefix_durable: archivePrefixDurable, + archive_prefix_text: ap.slice(0, 300), + archive_ghost_honest: archiveGhostHonest, + archive_ghost_text: ag.slice(0, 300), close_warns: closeWarns, close_warns_self_describing: closeWarnsSelfDescribing, health_has_write_path: healthHasWritePath, @@ -624,6 +647,12 @@ if (edgeCalls.length) failureCheck.unexpected.push(...edgeCalls.map((c) => `edge 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.archive_prefix_durable) { + failureCheck.unexpected.push(`archive_learning by id prefix did not archive the row (GIT-119): ${result.archive_prefix_text}`); +} +if (mode === "flow" && !result.archive_ghost_honest) { + failureCheck.unexpected.push(`archive_learning of a nonexistent id claimed success (GIT-119): ${result.archive_ghost_text}`); +} 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)"); } diff --git a/tests/unit/services/directpatch-zero-rows.test.ts b/tests/unit/services/directpatch-zero-rows.test.ts new file mode 100644 index 0000000..e27e4fe --- /dev/null +++ b/tests/unit/services/directpatch-zero-rows.test.ts @@ -0,0 +1,131 @@ +/** + * GIT-119: a PATCH that matched no row is not a write. + * + * PostgREST answers 200 with [] when a PATCH's filter matches nothing (a wrong + * id, a row not created yet, RLS). directPatch returned that array and all 8 + * callers ignored it, so each reported or assumed success for a write that did + * not happen. directPatch now returns { count, rows } and every caller treats + * count 0 as not durable, each in the way its result can carry it. + * + * One parameterized test over all 8 callers. The store is stubbed at fetch: + * every PATCH answers 200 [], every GET answers the rows the caller needs to + * reach its PATCH. + */ +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach } from "vitest"; + +const env = vi.hoisted(() => { + const keys = ["GITMEM_TIER", "SUPABASE_URL", "SUPABASE_SERVICE_ROLE_KEY", "GITMEM_DIR"]; + const before = Object.fromEntries(keys.map((k) => [k, process.env[k]])); + const dir = require("fs").mkdtempSync(require("path").join(require("os").tmpdir(), "gitmem-git119-")); + Object.assign(process.env, { + GITMEM_TIER: "pro", + SUPABASE_URL: "https://stubbed.supabase.co", + SUPABASE_SERVICE_ROLE_KEY: "stub", + GITMEM_DIR: dir, + }); + return { before, dir }; +}); + +import { resetTier } from "../../../src/services/tier.js"; +import { archiveLearning } from "../../../src/tools/archive-learning.js"; +import { saveSessionEmbedding } from "../../../src/tools/session-close.js"; +import { markSessionSuperseded } from "../../../src/tools/session-start.js"; +import { resolveThreadInSupabase, touchThreadsInSupabase, archiveDormantThreads } from "../../../src/services/thread-supabase.js"; +import { updateRelevanceData } from "../../../src/services/metrics.js"; +import { saveTranscript } from "../../../src/services/supabase-client.js"; +import { getEffectTracker } from "../../../src/services/effect-tracker.js"; + +const UUID = "a501c95e-1234-4678-9abc-def012345678"; +const SCAR = "b1111111-2222-4333-8444-555555555555"; +const patches: string[] = []; + +/** Rows a GET returns, by table, so each caller reaches its PATCH. */ +function getRows(url: URL): unknown[] { + const table = url.pathname.split("/").pop() ?? ""; + if (url.searchParams.get("limit") === "1" && url.searchParams.get("select") === "archived_at") return [{ archived_at: null }]; // column probe + if (table.endsWith("sessions")) return [{ close_compliance: null }]; + if (table.endsWith("threads")) { + if (url.searchParams.get("status") === "eq.dormant") { + return [{ id: UUID, thread_id: "t-dormant", metadata: { dormant_since: new Date(Date.now() - 60 * 86400_000).toISOString() } }]; + } + return [{ id: UUID, thread_id: "t-x", touch_count: 1, created_at: new Date().toISOString(), thread_class: "backlog", status: "active" }]; + } + if (table === "gitmem_query_metrics") return [{ id: UUID, memories_surfaced: [SCAR], metadata: {} }]; + return []; +} + +beforeAll(() => { + resetTier(); + vi.stubGlobal("fetch", vi.fn(async (input: unknown, init?: { method?: string }) => { + const url = new URL(String(input)); + const method = init?.method ?? "GET"; + const body = method === "PATCH" ? (patches.push(url.pathname), []) : method === "GET" ? getRows(url) : { Key: "ok" }; + return new Response(JSON.stringify(body), { status: 200, headers: { "content-type": "application/json" } }); + })); +}); + +afterAll(() => { + vi.unstubAllGlobals(); + for (const [k, v] of Object.entries(env.before)) { + if (v === undefined) delete process.env[k]; else process.env[k] = v; + } + resetTier(); + require("fs").rmSync(env.dir, { recursive: true, force: true }); +}); + +beforeEach(() => { patches.length = 0; }); + +/** A background write's failure is visible in stderr or the effect tracker. */ +const errorsLogged = () => (console.error as unknown as { mock: { calls: unknown[][] } }).mock.calls.map((c) => c.join(" ")).join("\n"); +const trackerFailures = (path: string) => getEffectTracker().getHealthReport().byPath[path]?.failed ?? 0; + +type Case = { caller: string; run: () => Promise }; + +const CASES: Case[] = [ + { caller: "archive_learning (full UUID)", run: async () => { + const r = await archiveLearning({ id: UUID }); + expect(r.success).toBe(false); + expect(r.durable).toBe(false); + } }, + { caller: "session_close: session embedding", run: async () => { + await expect(saveSessionEmbedding(UUID, "[0.1]")).rejects.toThrow(/updated no row/); + } }, + { caller: "session_start: mark superseded", run: async () => { + await markSessionSuperseded(UUID, SCAR); + expect(errorsLogged()).toMatch(/Failed to mark session .* superseded[\s\S]*no session row/); + } }, + { caller: "thread resolve", run: async () => { + expect(await resolveThreadInSupabase("t-x")).toBe(false); + } }, + { caller: "thread touch", run: async () => { + await touchThreadsInSupabase(["t-x"]); + expect(errorsLogged()).toMatch(/Failed to touch thread t-x[\s\S]*updated no row/); + } }, + { caller: "thread auto-archive", run: async () => { + const r = await archiveDormantThreads("default", 30); + expect(r.archived_count).toBe(0); + expect(r.archived_ids).toEqual([]); + } }, + { caller: "relevance update (metrics)", run: async () => { + const before = trackerFailures("relevance_update"); + await updateRelevanceData(UUID, [SCAR], { [SCAR]: "high" }); + expect(trackerFailures("relevance_update")).toBe(before + 1); + } }, + { caller: "saveTranscript: transcript_path", run: async () => { + const r = await saveTranscript(UUID, "hello"); + expect(r.patch_warning).toMatch(/no session row/); + } }, +]; + +describe("GIT-119: every directPatch caller treats 0 rows as not durable", () => { + it("covers all 8 callers", () => { + expect(CASES).toHaveLength(8); + }); + + it.each(CASES)("$caller", async ({ run }) => { + vi.spyOn(console, "error").mockImplementation(() => {}); + await run(); + expect(patches.length).toBeGreaterThan(0); // the PATCH was really sent + vi.restoreAllMocks(); + }); +}); diff --git a/tests/unit/tools/archive-learning.test.ts b/tests/unit/tools/archive-learning.test.ts index 7a004be..211ea06 100644 --- a/tests/unit/tools/archive-learning.test.ts +++ b/tests/unit/tools/archive-learning.test.ts @@ -7,7 +7,9 @@ import { SUPABASE_TIER_MOCKS } from "../../helpers/supabase-mocks.js"; // --- Mocks --- -const mockDirectPatch = vi.fn().mockResolvedValue(undefined); +// GIT-119: directPatch returns what it changed; the default is one row. +const PATCHED_ONE = { count: 1, rows: [{ id: "a501c95e-1234-5678-9abc-def012345678" }] }; +const mockDirectPatch = vi.fn().mockResolvedValue(PATCHED_ONE); const mockDirectQuery = vi.fn().mockResolvedValue([]); const mockIsConfigured = vi.fn(() => true); @@ -44,6 +46,11 @@ vi.mock("../../../src/services/startup.js", () => ({ flushCache: vi.fn().mockResolvedValue(undefined), })); +const localIds = vi.hoisted(() => ({ ids: [] as string[] })); +vi.mock("../../../src/services/local-vector-search.js", () => ({ + getLocalVectorSearch: () => ({ getScarIds: () => localIds.ids }), +})); + vi.mock("../../../src/services/metrics.js", () => ({ Timer: class { stop() { return 42; } }, })); @@ -52,7 +59,7 @@ vi.mock("../../../src/services/display-protocol.js", () => ({ wrapDisplay: (msg: string) => msg, })); -import { archiveLearning } from "../../../src/tools/archive-learning.js"; +import { archiveLearning, uuidRangeForPrefix } from "../../../src/tools/archive-learning.js"; import { hasSupabase } from "../../../src/services/tier.js"; const FULL_UUID = "a501c95e-1234-5678-9abc-def012345678"; @@ -60,6 +67,8 @@ const FULL_UUID = "a501c95e-1234-5678-9abc-def012345678"; describe("archive_learning prefix resolution", () => { beforeEach(() => { vi.clearAllMocks(); + mockDirectPatch.mockResolvedValue(PATCHED_ONE); + localIds.ids = []; vi.mocked(hasSupabase).mockReturnValue(true); mockIsConfigured.mockReturnValue(true); }); @@ -107,19 +116,21 @@ describe("archive_learning prefix resolution", () => { // --- Supabase prefix resolution --- - it("resolves 8-char prefix via Supabase like filter", async () => { + // GIT-119: these asserted id=like.%. Postgres has no LIKE on a UUID + // column (42883 on every store), so they pinned the bug; the store is now + // asked for the UUID range the prefix spans. + it("resolves 8-char prefix via a UUID range query", async () => { mockDirectQuery.mockResolvedValueOnce([{ id: FULL_UUID }]); const result = await archiveLearning({ id: "a501c95e" }); expect(result.success).toBe(true); expect(result.id).toBe(FULL_UUID); - // Check directQuery was called with like filter expect(mockDirectQuery).toHaveBeenCalledWith( expect.any(String), expect.objectContaining({ select: "id", - filters: { id: "like.a501c95e%" }, + filters: { and: "(id.gte.a501c95e-0000-0000-0000-000000000000,id.lte.a501c95e-ffff-ffff-ffff-ffffffffffff)" }, limit: 2, }), ); @@ -141,7 +152,7 @@ describe("archive_learning prefix resolution", () => { expect(mockDirectQuery).toHaveBeenCalledWith( expect.any(String), expect.objectContaining({ - filters: { id: "like.a501%" }, + filters: { and: "(id.gte.a5010000-0000-0000-0000-000000000000,id.lte.a501ffff-ffff-ffff-ffff-ffffffffffff)" }, }), ); }); @@ -224,3 +235,42 @@ describe("archive_learning prefix resolution", () => { expect(result.display).toContain("superseded"); }); }); + +describe("GIT-119: archive_learning on a store that has no LIKE for UUIDs", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockDirectPatch.mockResolvedValue(PATCHED_ONE); + localIds.ids = []; + vi.mocked(hasSupabase).mockReturnValue(true); + mockIsConfigured.mockReturnValue(true); + }); + + it("uuidRangeForPrefix spans exactly the ids with that prefix", () => { + expect(uuidRangeForPrefix("a501c95e1")).toEqual([ + "a501c95e-1000-0000-0000-000000000000", + "a501c95e-1fff-ffff-ffff-ffffffffffff", + ]); + const [lo, hi] = uuidRangeForPrefix("a501"); + for (const id of ["a5010000-0000-0000-0000-000000000000", FULL_UUID, "a501ffff-ffff-ffff-ffff-ffffffffffff"]) { + expect(id >= lo && id <= hi).toBe(true); + } + for (const id of ["a500ffff-ffff-ffff-ffff-ffffffffffff", "a5020000-0000-0000-0000-000000000000"]) { + expect(id >= lo && id <= hi).toBe(false); + } + }); + + it("resolves from the warm local index without a query", async () => { + localIds.ids = [FULL_UUID, "b1111111-1111-1111-1111-111111111111"]; + const result = await archiveLearning({ id: "a501c9" }); + expect(result.id).toBe(FULL_UUID); + expect(mockDirectQuery).not.toHaveBeenCalled(); + }); + + it("an id the PATCH matched no row for is not archived, and not durable", async () => { + mockDirectPatch.mockResolvedValueOnce({ count: 0, rows: [] }); + const result = await archiveLearning({ id: FULL_UUID }); + expect(result.success).toBe(false); + expect(result.durable).toBe(false); + expect(result.error).toMatch(/nothing was archived/); + }); +}); From 21d64d5ce825e44313ed3672950e104582b8714b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:49:12 -0700 Subject: [PATCH 12/17] fix: install-hooks/uninstall-hooks merge per event and keep foreign hooks (GIT-120) install-hooks replaced settings.hooks wholesale and uninstall-hooks deleted the key, so users' own hooks were lost. Both now work per event and per command: only commands gitmem installed are replaced or removed, a matcher group keeps its other commands, events install-hooks does not manage stay untouched, the file is backed up before every write, and the output lists what changed. An unparseable file is no longer overwritten "fresh". gitmem detection matches gitmem's script paths instead of any "gitmem" substring, which made a foreign hook under a gitmem-named directory look like an existing install. Tests: new CLI e2e (9) with foreign hooks present, wired into CI. Two cli-fresh-install assertions that pinned the old wipe are updated. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 5 + bin/gitmem.js | 183 ++++++++++++------------- bin/hooks-merge.js | 130 ++++++++++++++++++ tests/e2e/cli-fresh-install.test.ts | 10 +- tests/e2e/git-120-hooks-merge.test.ts | 187 ++++++++++++++++++++++++++ 5 files changed, 413 insertions(+), 102 deletions(-) create mode 100644 bin/hooks-merge.js create mode 100644 tests/e2e/git-120-hooks-merge.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a92f871..8d9fe5a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,6 +51,11 @@ jobs: - name: E2E — session identity survives MCP restart run: npx vitest run --config vitest.e2e.config.ts tests/e2e/git-89-session-identity.test.ts + # GIT-120: install-hooks / uninstall-hooks keep foreign hooks. Runs the + # real CLI in a scratch repo; no server, Docker or credentials. + - name: E2E — hooks install/uninstall keep foreign hooks + run: npx vitest run --config vitest.e2e.config.ts tests/e2e/git-120-hooks-merge.test.ts + - 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)}" diff --git a/bin/gitmem.js b/bin/gitmem.js index e2294bc..1dc71d6 100755 --- a/bin/gitmem.js +++ b/bin/gitmem.js @@ -28,6 +28,7 @@ import { import { dirname, join } from "path"; import { fileURLToPath } from "url"; import { homedir } from "os"; +import { hasGitmemHooks, hooksShapeError, mergeGitmemHooks, removeGitmemHooks, backupFile } from "./hooks-merge.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); const command = process.argv[2]; @@ -479,6 +480,39 @@ async function cmdSessionRefresh() { } } +/** + * GIT-120: read a hooks-bearing JSON file for an in-place edit. A file that + * exists but cannot be parsed, or whose hooks have an unexpected shape, is + * never rewritten — that used to replace the user's file with a fresh one. + */ +function readHooksFile(path, name) { + if (!existsSync(path)) return {}; + let cfg; + try { + cfg = JSON.parse(readFileSync(path, "utf-8")); + } catch (err) { + console.error(`Error: ${name} is not valid JSON (${err.message}). Nothing was changed.`); + console.error(`Fix or move ${name}, then run this again.`); + process.exit(1); + } + const shapeError = cfg && typeof cfg === "object" && !Array.isArray(cfg) ? hooksShapeError(cfg.hooks) : "file is not a JSON object"; + if (shapeError) { + console.error(`Error: ${name}: ${shapeError}. Nothing was changed.`); + process.exit(1); + } + return cfg; +} + +/** Back up, write, and print what changed. */ +function writeHooksFile(path, name, cfg, changes) { + const backup = backupFile(path); + writeFileSync(path, JSON.stringify(cfg, null, 2)); + console.log(`Changes to ${name}:`); + for (const line of changes) console.log(` ${line}`); + console.log(backup ? `Backup of the previous file: ${backup}` : "(new file, nothing to back up)"); + console.log(""); +} + /** * Install gitmem hooks as project-level hooks. * @@ -560,30 +594,21 @@ function cmdInstallHooks() { stop: [{ command: `bash ${relScripts}/session-close-check.sh`, timeout: 5000 }], }; - let config = {}; - if (existsSync(hooksPath)) { - try { - config = JSON.parse(readFileSync(hooksPath, "utf-8")); - } catch { - console.warn(" Warning: Could not parse existing .cursor/hooks.json, creating fresh"); - } - } else { - mkdirSync(cursorDir, { recursive: true }); - } + const config = readHooksFile(hooksPath, ".cursor/hooks.json"); + if (!existsSync(cursorDir)) mkdirSync(cursorDir, { recursive: true }); - if (config.hooks && !force) { - const hasGitmem = JSON.stringify(config.hooks).includes("gitmem"); - if (hasGitmem) { - console.log("GitMem hooks already installed in .cursor/hooks.json"); - console.log(""); - console.log("To reinstall (overwrite), run:"); - console.log(" npx gitmem-mcp install-hooks --client cursor --force"); - return; - } + if (!force && hasGitmemHooks(config.hooks)) { + console.log("GitMem hooks already installed in .cursor/hooks.json"); + console.log(""); + console.log("To reinstall (replaces only gitmem's hooks), run:"); + console.log(" npx gitmem-mcp install-hooks --client cursor --force"); + return; } - config.hooks = gitmemHooks; - writeFileSync(hooksPath, JSON.stringify(config, null, 2)); + // GIT-120: merge per event; other hooks stay as they are. + const merged = mergeGitmemHooks(config.hooks, gitmemHooks); + config.hooks = merged.hooks; + writeHooksFile(hooksPath, ".cursor/hooks.json", config, merged.changes); console.log("Hooks written to .cursor/hooks.json"); console.log(`Scripts at: ${relScripts}/`); @@ -634,30 +659,21 @@ function cmdInstallHooks() { ], }; - let settings = {}; - if (existsSync(settingsPath)) { - try { - settings = JSON.parse(readFileSync(settingsPath, "utf-8")); - } catch { - console.warn(" Warning: Could not parse existing .claude/settings.json, creating fresh"); - } - } else { - mkdirSync(claudeDir, { recursive: true }); - } + const settings = readHooksFile(settingsPath, ".claude/settings.json"); + if (!existsSync(claudeDir)) mkdirSync(claudeDir, { recursive: true }); - if (settings.hooks && !force) { - const hasGitmem = JSON.stringify(settings.hooks).includes("gitmem"); - if (hasGitmem) { - console.log("GitMem hooks already installed in .claude/settings.json"); - console.log(""); - console.log("To reinstall (overwrite), run:"); - console.log(" npx gitmem-mcp install-hooks --force"); - return; - } + if (!force && hasGitmemHooks(settings.hooks)) { + console.log("GitMem hooks already installed in .claude/settings.json"); + console.log(""); + console.log("To reinstall (replaces only gitmem's hooks), run:"); + console.log(" npx gitmem-mcp install-hooks --force"); + return; } - settings.hooks = gitmemHooks; - writeFileSync(settingsPath, JSON.stringify(settings, null, 2)); + // GIT-120: merge per event; other hooks stay as they are. + const merged = mergeGitmemHooks(settings.hooks, gitmemHooks); + settings.hooks = merged.hooks; + writeHooksFile(settingsPath, ".claude/settings.json", settings, merged.changes); console.log("Hooks written to .claude/settings.json"); console.log(`Scripts at: ${relScripts}/`); @@ -732,6 +748,25 @@ function promptTelemetryOptIn() { * Also cleans up legacy plugin directories and temp state. * Use --client to target a specific IDE. */ +/** + * GIT-120: take out only the commands gitmem installed. Other hooks, and the + * hooks key itself while anything is left in it, stay. + */ +function uninstallFrom(path, name, cfg, extraChanges = []) { + if (!cfg.hooks && extraChanges.length === 0) { + console.log(`[uninstall] No hooks found in ${name}`); + return; + } + const result = removeGitmemHooks(cfg.hooks); + if (result.removed === 0 && extraChanges.length === 0) { + console.log(`[uninstall] No gitmem hooks found in ${name}${result.kept ? ` (${result.kept} other hook(s) left untouched)` : ""}`); + return; + } + if (result.hooks) cfg.hooks = result.hooks; else delete cfg.hooks; + writeHooksFile(path, name, cfg, [...result.changes, ...extraChanges]); + console.log(`[uninstall] Removed ${result.removed} gitmem hook(s) from ${name}; kept ${result.kept} other`); +} + function cmdUninstallHooks() { const clientIdx = process.argv.indexOf("--client"); const clientArg = clientIdx !== -1 ? process.argv[clientIdx + 1]?.toLowerCase() : null; @@ -759,40 +794,8 @@ function cmdUninstallHooks() { // Remove hooks from .cursor/hooks.json const hooksPath = join(process.cwd(), ".cursor", "hooks.json"); if (existsSync(hooksPath)) { - try { - const cfg = JSON.parse(readFileSync(hooksPath, "utf-8")); - if (cfg.hooks) { - // Filter out gitmem hooks, preserve others - const cleaned = {}; - let removed = 0; - for (const [eventType, entries] of Object.entries(cfg.hooks)) { - if (!Array.isArray(entries)) continue; - const nonGitmem = entries.filter((e) => { - if (typeof e.command === "string" && e.command.includes("gitmem")) { - removed++; - return false; - } - return true; - }); - if (nonGitmem.length > 0) cleaned[eventType] = nonGitmem; - } - if (removed > 0) { - if (Object.keys(cleaned).length > 0) { - cfg.hooks = cleaned; - } else { - delete cfg.hooks; - } - writeFileSync(hooksPath, JSON.stringify(cfg, null, 2)); - console.log(`[uninstall] Removed ${removed} gitmem hooks from .cursor/hooks.json`); - } else { - console.log("[uninstall] No gitmem hooks found in .cursor/hooks.json"); - } - } else { - console.log("[uninstall] No hooks found in .cursor/hooks.json"); - } - } catch { - // ignore parse errors - } + const cfg = readHooksFile(hooksPath, ".cursor/hooks.json"); + uninstallFrom(hooksPath, ".cursor/hooks.json", cfg); } else { console.log("[uninstall] No .cursor/hooks.json found"); } @@ -800,28 +803,12 @@ function cmdUninstallHooks() { // Remove hooks from .claude/settings.json const settingsPath = join(process.cwd(), ".claude", "settings.json"); if (existsSync(settingsPath)) { - try { - const cfg = JSON.parse(readFileSync(settingsPath, "utf-8")); - if (cfg.hooks) { - delete cfg.hooks; - writeFileSync(settingsPath, JSON.stringify(cfg, null, 2)); - console.log("[uninstall] Removed hooks from .claude/settings.json"); - } else { - console.log("[uninstall] No hooks found in .claude/settings.json"); - } - // Also clean legacy enabledPlugins - if (cfg.enabledPlugins) { - for (const key of Object.keys(cfg.enabledPlugins)) { - if (key.startsWith("gitmem-hooks")) { - delete cfg.enabledPlugins[key]; - writeFileSync(settingsPath, JSON.stringify(cfg, null, 2)); - console.log("[cleanup] Removed legacy enabledPlugins entry"); - } - } - } - } catch { - // ignore parse errors - } + const cfg = readHooksFile(settingsPath, ".claude/settings.json"); + // Legacy enabledPlugins entries from the old plugin install. + const legacyPlugins = Object.keys(cfg.enabledPlugins || {}).filter((k) => k.startsWith("gitmem-hooks")); + for (const key of legacyPlugins) delete cfg.enabledPlugins[key]; + uninstallFrom(settingsPath, ".claude/settings.json", cfg, + legacyPlugins.map((k) => `enabledPlugins: removed legacy entry ${k}`)); } } diff --git a/bin/hooks-merge.js b/bin/hooks-merge.js new file mode 100644 index 0000000..9a8b11c --- /dev/null +++ b/bin/hooks-merge.js @@ -0,0 +1,130 @@ +/** + * GIT-120: merge gitmem's hooks into a settings file without touching anyone + * else's. + * + * install-hooks used to replace `settings.hooks` wholesale and uninstall-hooks + * deleted the key, so every hook the user or another tool had configured was + * lost. These helpers work per event and per command: only commands gitmem + * wrote are replaced or removed, and the file is backed up before any write. + * + * Two shapes are supported: + * Claude Code hooks[event] = [{ matcher?, hooks: [{ type, command, ... }] }] + * Cursor hooks[event] = [{ command, ... }] + */ + +import { copyFileSync, existsSync } from "fs"; + +/** + * A command gitmem installed: the per-repo copy (.gitmem/hooks/), the older + * node_modules path, or the legacy plugin. A bare "gitmem" substring is not + * enough — a user's own script can live in a directory called gitmem. + */ +export function isGitmemCommand(command) { + if (typeof command !== "string") return false; + return /(^|[\s"'=/])\.gitmem\/hooks\/[\w.-]+\.sh\b/.test(command) + || /gitmem-mcp\/hooks\/scripts\/[\w.-]+\.sh\b/.test(command) + || /\bgitmem-hooks\b/.test(command); +} + +const isGroup = (entry) => entry && typeof entry === "object" && Array.isArray(entry.hooks); + +/** Commands in one event's entries, as [gitmem, other] counts. */ +function countEntries(entries) { + let gitmem = 0; + let other = 0; + for (const entry of Array.isArray(entries) ? entries : []) { + const commands = isGroup(entry) ? entry.hooks : [entry]; + for (const h of commands) { + if (isGitmemCommand(h?.command)) gitmem++; else other++; + } + } + return { gitmem, other }; +} + +/** + * One event's entries with gitmem's commands taken out. A Claude group keeps + * its other commands; a group left empty is dropped. + */ +function withoutGitmem(entries) { + const kept = []; + for (const entry of Array.isArray(entries) ? entries : []) { + if (isGroup(entry)) { + const others = entry.hooks.filter((h) => !isGitmemCommand(h?.command)); + if (others.length === entry.hooks.length) kept.push(entry); + else if (others.length > 0) kept.push({ ...entry, hooks: others }); + } else if (!isGitmemCommand(entry?.command)) { + kept.push(entry); + } + } + return kept; +} + +/** The hooks object must be a plain object of arrays, or absent. */ +export function hooksShapeError(hooks) { + if (hooks === undefined) return null; + if (!hooks || typeof hooks !== "object" || Array.isArray(hooks)) return "hooks is not an object"; + for (const [event, entries] of Object.entries(hooks)) { + if (!Array.isArray(entries)) return `hooks.${event} is not an array`; + } + return null; +} + +/** True when any command in the hooks is one gitmem installed. */ +export function hasGitmemHooks(hooks) { + return Object.values(hooks || {}).some((entries) => countEntries(entries).gitmem > 0); +} + +/** + * Merge gitmem's hooks in: per event, drop the gitmem commands already there, + * keep everything else, append gitmem's current entries. + * Returns the new hooks and one change line per event touched. + */ +export function mergeGitmemHooks(existing, gitmemHooks) { + const hooks = { ...(existing || {}) }; + const changes = []; + for (const [event, entries] of Object.entries(gitmemHooks)) { + const before = countEntries(hooks[event]); + const added = countEntries(entries).gitmem; + hooks[event] = [...withoutGitmem(hooks[event]), ...entries]; + changes.push(`${event}: ${before.gitmem ? `replaced ${before.gitmem} gitmem hook(s) with ${added}` : `added ${added} gitmem hook(s)`}${before.other ? `, kept ${before.other} other` : ""}`); + } + // Events this installer does not manage are left exactly as they are. That + // includes gitmem's own UserPromptSubmit hook, which `gitmem init` installs + // and install-hooks does not. + for (const [event, entries] of Object.entries(hooks)) { + if (event in gitmemHooks) continue; + const c = countEntries(entries); + if (c.gitmem + c.other) changes.push(`${event}: kept ${c.gitmem + c.other} (untouched)`); + } + return { hooks, changes }; +} + +/** + * Take gitmem's commands out of every event. Events left empty are dropped; + * hooks is undefined when nothing remains. + */ +export function removeGitmemHooks(existing) { + const hooks = {}; + const changes = []; + let removed = 0; + let kept = 0; + for (const [event, entries] of Object.entries(existing || {})) { + const c = countEntries(entries); + removed += c.gitmem; + kept += c.other; + const rest = withoutGitmem(entries); + if (rest.length) hooks[event] = rest; + if (c.gitmem) changes.push(`${event}: removed ${c.gitmem} gitmem hook(s)${c.other ? `, kept ${c.other} other` : ""}`); + else if (c.other) changes.push(`${event}: kept ${c.other} other (untouched)`); + } + return { hooks: Object.keys(hooks).length ? hooks : undefined, removed, kept, changes }; +} + +/** Copy a file aside before it is rewritten; returns the backup path or null. */ +export function backupFile(path) { + if (!existsSync(path)) return null; + const stamp = new Date().toISOString().replace(/[:.]/g, "-"); + const backup = `${path}.gitmem-backup-${stamp}`; + copyFileSync(path, backup); + return backup; +} diff --git a/tests/e2e/cli-fresh-install.test.ts b/tests/e2e/cli-fresh-install.test.ts index d7d6e3e..eee3990 100644 --- a/tests/e2e/cli-fresh-install.test.ts +++ b/tests/e2e/cli-fresh-install.test.ts @@ -295,9 +295,10 @@ describe("Fresh Install: Hooks CLI", () => { // Permissions preserved expect(settings.permissions.allow).toContain("mcp__gitmem__*"); expect(settings.permissions.allow).toContain("custom_tool"); - // Hooks replaced with new ones + // gitmem's hooks added; the foreign "echo old" hook kept (GIT-120) const cmd = JSON.stringify(settings.hooks.SessionStart); expect(cmd).toContain("session-start.sh"); + expect(cmd).toContain("echo old"); }); it("uninstall-hooks removes hooks from settings", async () => { @@ -308,10 +309,11 @@ describe("Fresh Install: Hooks CLI", () => { expect(exitCode).toBe(0); expect(stdout).toContain("Uninstall complete"); - // settings.json still exists but hooks key removed + // settings.json still exists; only gitmem's hooks removed, the foreign + // "echo old" hook from the previous test kept (GIT-120) expect(existsSync(SETTINGS_PATH)).toBe(true); const settings = JSON.parse(readFileSync(SETTINGS_PATH, "utf-8")); - expect(settings.hooks).toBeUndefined(); + expect(settings.hooks).toEqual({ SessionStart: [{ hooks: [{ type: "command", command: "echo old" }] }] }); // Permissions preserved expect(settings.permissions).toBeDefined(); @@ -323,7 +325,7 @@ describe("Fresh Install: Hooks CLI", () => { }); expect(exitCode).toBe(0); - expect(stdout).toContain("No hooks found"); + expect(stdout).toContain("No gitmem hooks found"); }); }); diff --git a/tests/e2e/git-120-hooks-merge.test.ts b/tests/e2e/git-120-hooks-merge.test.ts new file mode 100644 index 0000000..4cc995d --- /dev/null +++ b/tests/e2e/git-120-hooks-merge.test.ts @@ -0,0 +1,187 @@ +/** + * GIT-120: install-hooks and uninstall-hooks keep other people's hooks. + * + * install-hooks replaced settings.hooks wholesale and uninstall-hooks deleted + * the key, so a user's own hooks (or another tool's) vanished. These run the + * real CLI in a scratch repo that already has foreign hooks, and check that + * only gitmem's commands come and go, that the file is backed up first, and + * that the output says what changed. + * + * No server, no Supabase, no Docker: safe for CI. + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { execFile as execFileCb } from "child_process"; +import { promisify } from "util"; +import { mkdtempSync, rmSync, readFileSync, writeFileSync, mkdirSync, readdirSync } from "fs"; +import { join } from "path"; +import { tmpdir } from "os"; + +const execFile = promisify(execFileCb); +const GITMEM_BIN = join(__dirname, "../../bin/gitmem.js"); + +async function gitmem(args: string[], cwd: string, home: string) { + try { + const { stdout, stderr } = await execFile("node", [GITMEM_BIN, ...args], { + cwd, + env: { ...process.env, HOME: home, NO_COLOR: "1", SUPABASE_URL: "", SUPABASE_SERVICE_ROLE_KEY: "", GITMEM_TIER: "free" }, + timeout: 30_000, + }); + return { stdout, stderr, code: 0 }; + } catch (e: any) { + return { stdout: e.stdout ?? "", stderr: e.stderr ?? "", code: typeof e.code === "number" ? e.code : 1 }; + } +} + +/** A user's own hooks, including one whose path happens to contain "gitmem". */ +const FOREIGN_CLAUDE = { + SessionStart: [{ hooks: [{ type: "command", command: "echo foreign-session-start" }] }], + PreToolUse: [ + { matcher: "Bash", hooks: [{ type: "command", command: "bash ~/code/gitmem-notes/lint.sh" }] }, + ], + Notification: [{ hooks: [{ type: "command", command: "say done" }] }], +}; + +const allCommands = (hooks: Record = {}) => + Object.values(hooks).flat().flatMap((e: any) => (Array.isArray(e.hooks) ? e.hooks : [e])).map((h: any) => h.command as string); +const gitmemCommands = (hooks?: Record) => allCommands(hooks).filter((c) => c.includes(".gitmem/hooks/")); +const foreignCommands = (hooks?: Record) => allCommands(hooks).filter((c) => !c.includes(".gitmem/hooks/")); +const backups = (dir: string, base: string) => readdirSync(dir).filter((f) => f.startsWith(`${base}.gitmem-backup-`)); + +let repo: string; +let home: string; +let settingsPath: string; + +beforeEach(() => { + repo = mkdtempSync(join(tmpdir(), "gitmem-git120-repo-")); + home = mkdtempSync(join(tmpdir(), "gitmem-git120-home-")); + mkdirSync(join(repo, ".claude")); + settingsPath = join(repo, ".claude", "settings.json"); +}); + +afterEach(() => { + rmSync(repo, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); +}); + +describe("GIT-120: Claude Code settings.json", () => { + const original = () => ({ permissions: { allow: ["Bash(ls:*)"] }, hooks: structuredClone(FOREIGN_CLAUDE) }); + + it("install-hooks keeps foreign hooks, backs up first, and prints what changed", async () => { + const before = JSON.stringify(original(), null, 2); + writeFileSync(settingsPath, before); + + const r = await gitmem(["install-hooks"], repo, home); + expect(r.code).toBe(0); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + + expect(foreignCommands(after.hooks).sort()).toEqual(allCommands(FOREIGN_CLAUDE).sort()); + expect(after.hooks.Notification).toEqual(FOREIGN_CLAUDE.Notification); + expect(gitmemCommands(after.hooks).length).toBeGreaterThan(0); + expect(after.permissions).toEqual({ allow: ["Bash(ls:*)"] }); + + const b = backups(join(repo, ".claude"), "settings.json"); + expect(b).toHaveLength(1); + expect(readFileSync(join(repo, ".claude", b[0]), "utf-8")).toBe(before); + + expect(r.stdout).toContain("Changes to .claude/settings.json:"); + expect(r.stdout).toMatch(/SessionStart: added \d+ gitmem hook\(s\), kept 1 other/); + expect(r.stdout).toMatch(/Notification: kept 1 \(untouched\)/); + expect(r.stdout).toContain("Backup of the previous file:"); + }); + + it("install-hooks --force twice replaces gitmem's hooks without duplicating them", async () => { + writeFileSync(settingsPath, JSON.stringify(original(), null, 2)); + await gitmem(["install-hooks"], repo, home); + const once = gitmemCommands(JSON.parse(readFileSync(settingsPath, "utf-8")).hooks).length; + + const r = await gitmem(["install-hooks", "--force"], repo, home); + expect(r.code).toBe(0); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(gitmemCommands(after.hooks)).toHaveLength(once); + expect(foreignCommands(after.hooks).sort()).toEqual(allCommands(FOREIGN_CLAUDE).sort()); + expect(r.stdout).toMatch(/SessionStart: replaced \d+ gitmem hook\(s\) with \d+, kept 1 other/); + }); + + it("install-hooks without --force leaves an installed file alone", async () => { + writeFileSync(settingsPath, JSON.stringify(original(), null, 2)); + await gitmem(["install-hooks"], repo, home); + const snapshot = readFileSync(settingsPath, "utf-8"); + const r = await gitmem(["install-hooks"], repo, home); + expect(r.stdout).toContain("already installed"); + expect(readFileSync(settingsPath, "utf-8")).toBe(snapshot); + }); + + it("uninstall-hooks removes only gitmem's commands and leaves the foreign hooks as they were", async () => { + writeFileSync(settingsPath, JSON.stringify(original(), null, 2)); + await gitmem(["install-hooks"], repo, home); + + const r = await gitmem(["uninstall-hooks"], repo, home); + expect(r.code).toBe(0); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.hooks).toEqual(FOREIGN_CLAUDE); + expect(after.permissions).toEqual({ allow: ["Bash(ls:*)"] }); + expect(r.stdout).toMatch(/Removed \d+ gitmem hook\(s\) from \.claude\/settings\.json; kept 3 other/); + expect(backups(join(repo, ".claude"), "settings.json")).toHaveLength(2); + }); + + it("uninstall-hooks keeps a foreign command that shares a matcher group with gitmem's", async () => { + const mixed = { + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [ + { type: "command", command: "bash .gitmem/hooks/credential-guard.sh", timeout: 3000 }, + { type: "command", command: "echo mine" }, + ] }], + }, + }; + writeFileSync(settingsPath, JSON.stringify(mixed, null, 2)); + const r = await gitmem(["uninstall-hooks"], repo, home); + expect(r.code).toBe(0); + expect(JSON.parse(readFileSync(settingsPath, "utf-8")).hooks).toEqual({ + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "echo mine" }] }], + }); + }); + + it("install-hooks leaves events it does not manage alone, including init's auto-retrieve hook", async () => { + const autoRetrieve = { UserPromptSubmit: [{ hooks: [{ type: "command", command: "bash .gitmem/hooks/auto-retrieve-hook.sh", timeout: 3000 }] }] }; + writeFileSync(settingsPath, JSON.stringify({ hooks: autoRetrieve }, null, 2)); + const r = await gitmem(["install-hooks", "--force"], repo, home); + expect(r.code).toBe(0); + expect(JSON.parse(readFileSync(settingsPath, "utf-8")).hooks.UserPromptSubmit).toEqual(autoRetrieve.UserPromptSubmit); + }); + + it("uninstall-hooks drops the hooks key only when nothing else is left", async () => { + await gitmem(["install-hooks"], repo, home); + await gitmem(["uninstall-hooks"], repo, home); + expect(JSON.parse(readFileSync(settingsPath, "utf-8")).hooks).toBeUndefined(); + }); + + it("an unparseable settings.json is not overwritten", async () => { + writeFileSync(settingsPath, "{ not json"); + for (const cmd of ["install-hooks", "uninstall-hooks"]) { + const r = await gitmem([cmd], repo, home); + expect(r.code).toBe(1); + expect(r.stderr).toContain("Nothing was changed"); + expect(readFileSync(settingsPath, "utf-8")).toBe("{ not json"); + } + }); +}); + +describe("GIT-120: Cursor hooks.json", () => { + it("install and uninstall keep a foreign Cursor hook", async () => { + mkdirSync(join(repo, ".cursor")); + const hooksPath = join(repo, ".cursor", "hooks.json"); + const foreign = { version: 1, hooks: { stop: [{ command: "echo cursor-stop" }], afterFileEdit: [{ command: "prettier --write" }] } }; + writeFileSync(hooksPath, JSON.stringify(foreign, null, 2)); + + const i = await gitmem(["install-hooks", "--client", "cursor"], repo, home); + expect(i.code).toBe(0); + const installed = JSON.parse(readFileSync(hooksPath, "utf-8")); + expect(foreignCommands(installed.hooks).sort()).toEqual(["echo cursor-stop", "prettier --write"]); + expect(gitmemCommands(installed.hooks).length).toBeGreaterThan(0); + expect(installed.version).toBe(1); + + const u = await gitmem(["uninstall-hooks", "--client", "cursor"], repo, home); + expect(u.code).toBe(0); + expect(JSON.parse(readFileSync(hooksPath, "utf-8"))).toEqual(foreign); + }); +}); From c1cf1b1458bf1a7fe08e82421daa981365b6caa1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:51:05 -0700 Subject: [PATCH 13/17] test: thread-sync mocks use the GIT-119 directPatch shape (release/D stack) GIT-117's tests mocked directPatch as an array. After GIT-119 it returns { count, rows }; with the old mock a 0-row PATCH read as success. Co-Authored-By: Claude Opus 5.5 --- tests/unit/services/thread-sync-result.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/services/thread-sync-result.test.ts b/tests/unit/services/thread-sync-result.test.ts index 338ed91..21055c4 100644 --- a/tests/unit/services/thread-sync-result.test.ts +++ b/tests/unit/services/thread-sync-result.test.ts @@ -58,7 +58,7 @@ beforeEach(() => { mockHasSupabase.mockReturnValue(true); mockDirectUpsert.mockResolvedValue([{ id: "row" }]); mockDirectPatch.mockReset(); - mockDirectPatch.mockResolvedValue([{ id: "row" }]); + mockDirectPatch.mockResolvedValue({ count: 1, rows: [{ id: "row" }] }); // GIT-119 shape }); describe("tier awareness (R3)", () => { @@ -266,7 +266,7 @@ describe("GIT-117: a thread counts as synced only when the store confirms a row" mockDirectQuery .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r", status: "open" }]) // existence .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r" }]); // resolve lookup - mockDirectPatch.mockResolvedValue([]); + mockDirectPatch.mockResolvedValue({ count: 0, rows: [] }); const r = await syncThreadsToSupabase([thread("t-r", "resolved")], "gitmem", "s-1"); From 5cb3351e9a6c523cc8a50ad664922de746a01e09 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 20:55:42 -0700 Subject: [PATCH 14/17] docs: CHANGELOG [Unreleased] for Release D (GIT-114, 116-120) Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96a05f1..8d59a11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,38 @@ 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 below works on a project +still on the `setup.sql` you first ran, back to 1.8.0, and was checked against a project on exactly +that schema and on the current one. + +**Recall works when it has to ask your database.** When the local index was not ready, or with +`GITMEM_SEARCH_MODE=remote`, recall called a database function that `setup.sql` never created. It +got an error and returned no scars. It now calls `gitmem_scar_search`, which every project has, and +falls back to the older function only where that exists. (GIT-114) + +**`install-hooks` and `uninstall-hooks` keep your own hooks.** They used to replace or delete every +hook in `.claude/settings.json` or `.cursor/hooks.json`. Now they add or remove only gitmem's hooks, +back the file up first, and print what changed. A file that is not valid JSON is left alone instead +of being replaced. (GIT-120) + +### Fixed + +- **Hooks work on a stock Mac.** The prompt hook ran `timeout`, which macOS does not have, so it + never returned scars there. The time limit now lives inside gitmem. When gitmem or node cannot be + found, the hook says so instead of staying silent. (GIT-116) +- **A thread that failed to sync is no longer marked synced.** `session_close` reports PARTIAL and + names the thread ids that did not reach your database. Those threads stay in `threads.json` and + the next session tries again. (GIT-117) +- **A failed index reload no longer empties recall.** If reloading scars from your database fails, + gitmem keeps the index it had, falls back to the on-disk cache, and retries in the background. + `create_learning` says when the new learning is saved but not yet searchable. (GIT-118) +- **`archive_learning` accepts a short id.** Archiving by id prefix failed on every project with + "operator does not exist". Archiving an id that does not exist used to report success; it now says + nothing was archived. (GIT-119) +- **Updates that change no row are reported as not saved.** This covers thread resolve, touch and + auto-archive, session embeddings, superseded sessions, relevance data and transcript paths. Each + one used to count as success. (GIT-119) + ## [1.11.0] - 2026-09-21 **Just upgrade the package. Nothing to change on your database.** Every fix in this release works on From d590566533b5a6d26cb0f7ae6b2f33d7f8ddc5e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 21:19:28 -0700 Subject: [PATCH 15/17] fix: init writes the store where the server reads it; repo keeps config + hooks (GIT-115) init seeded /.gitmem while the server reads ~/.gitmem (GIT-91), so a fresh install had no starter lessons and was told its store was "NOT being read". The wizard and init-scars now seed learnings, threads, collections and the closing payload + template into the root the server reads (GITMEM_DIR, then GITMEM_HOME/.gitmem, then ~/.gitmem). The repo's .gitmem/ keeps only config.json (project name) and the hook scripts. - A repo .gitmem/ with only config.json and hooks, or only what an older init seeded, is not a stranded store. Real records (a user's learning, thread or decision) now count, which main did not check at all. - The SessionStart hook reads the repo's project name without jq (node). - uninstall removes the repo's config.json and hooks; the shared store is kept unless --all or the user says delete. Hook removal and the wizard's hook merge use the GIT-120 module (bin/hooks-merge.js, identical to #55). - telemetry commands use the server's root. Tests: fresh-install e2e in a clean HOME (9, all fail on main), unit +5, hook suite +1. cli-fresh-install made hermetic (scratch HOME) and repaired; both e2e files wired into CI. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 6 + bin/gitmem-root.js | 34 ++++ bin/gitmem.js | 23 +-- bin/hooks-merge.js | 130 ++++++++++++++ bin/init-wizard.js | 125 ++++++-------- bin/uninstall.js | 163 +++++++----------- hooks/scripts/session-start.sh | 10 +- hooks/tests/test-hooks.sh | 16 ++ src/commands/telemetry.ts | 11 +- src/services/gitmem-dir.ts | 50 +++++- tests/e2e/cli-fresh-install.test.ts | 36 ++-- tests/e2e/git-115-fresh-install.test.ts | 156 +++++++++++++++++ .../services/gitmem-root-resolution.test.ts | 32 +++- .../unit/services/gitmem-root-symlink.test.ts | 8 +- 14 files changed, 591 insertions(+), 209 deletions(-) create mode 100644 bin/gitmem-root.js create mode 100644 bin/hooks-merge.js create mode 100644 tests/e2e/git-115-fresh-install.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a92f871..74b46f6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,6 +51,12 @@ jobs: - name: E2E — session identity survives MCP restart run: npx vitest run --config vitest.e2e.config.ts tests/e2e/git-89-session-identity.test.ts + # GIT-115: a fresh install in a clean HOME — init, the SessionStart hook, + # the built server — plus the CLI/wizard suite. Both run the real CLI in + # scratch directories with a scratch HOME; no Docker or credentials. + - name: E2E — fresh install reads what init wrote; CLI and wizard + run: npx vitest run --config vitest.e2e.config.ts tests/e2e/git-115-fresh-install.test.ts tests/e2e/cli-fresh-install.test.ts + - 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)}" diff --git a/bin/gitmem-root.js b/bin/gitmem-root.js new file mode 100644 index 0000000..5ddbb61 --- /dev/null +++ b/bin/gitmem-root.js @@ -0,0 +1,34 @@ +/** + * GIT-115: where the CLI puts things, split the same way the server reads them. + * + * The store (learnings, threads, sessions, decisions, the closing payload and + * its template) belongs in the root the server reads. That is resolved exactly + * as src/services/gitmem-dir.ts does, without a running session: GITMEM_DIR, + * then GITMEM_HOME/.gitmem, then ~/.gitmem. `init` used to seed /.gitmem, + * which the server has not read since GIT-91, so a fresh install saw no + * starter lessons and was told its store was "NOT being read". + * + * The repo keeps its own .gitmem/ for two things only: config.json with the + * project name (the SessionStart hook reads it from the repo) and the hook + * scripts (committed settings refer to them by relative path). + */ + +import { join } from "path"; +import { homedir } from "os"; + +/** The root the server reads. */ +export function storeRoot() { + if (process.env.GITMEM_DIR) return process.env.GITMEM_DIR; + return join(process.env.GITMEM_HOME || homedir(), ".gitmem"); +} + +/** The repo's .gitmem/: config.json and hooks/ only. */ +export function repoGitmemDir(cwd = process.cwd()) { + return join(cwd, ".gitmem"); +} + +/** A path shortened to ~ for display. */ +export function displayPath(p) { + const home = homedir(); + return p === home || p.startsWith(home + "/") ? "~" + p.slice(home.length) : p; +} diff --git a/bin/gitmem.js b/bin/gitmem.js index e2294bc..bad3b47 100755 --- a/bin/gitmem.js +++ b/bin/gitmem.js @@ -28,6 +28,7 @@ import { import { dirname, join } from "path"; import { fileURLToPath } from "url"; import { homedir } from "os"; +import { storeRoot, displayPath } from "./gitmem-root.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); const command = process.argv[2]; @@ -42,7 +43,7 @@ Usage: npx gitmem-mcp init --dry-run Show what would be configured npx gitmem-mcp init --client cursor Set up for Cursor IDE npx gitmem-mcp uninstall Clean removal of gitmem from project - npx gitmem-mcp uninstall --all Also delete .gitmem/ data directory + npx gitmem-mcp uninstall --all Also delete the memory store (~/.gitmem, shared by all projects) Pro Tier Activation: npx gitmem-mcp activate Activate Pro tier (license key + credentials wizard) @@ -115,25 +116,25 @@ async function cmdInit() { } if (!supabaseUrl || !supabaseKey) { - // Free tier: copy starter scars to local .gitmem/ directory + // Free tier: copy starter scars into the store the server reads (GIT-115). console.log("No Supabase credentials found — initializing free tier (local storage)."); console.log(""); - const gitmemDir = join(process.cwd(), ".gitmem"); + const gitmemDir = storeRoot(); + const storeName = displayPath(gitmemDir); if (!existsSync(gitmemDir)) { mkdirSync(gitmemDir, { recursive: true }); } - // Write config.json (with project if specified via --project) - const configPath = join(gitmemDir, "config.json"); + // The project name stays in the repo's .gitmem/config.json, where the + // SessionStart hook reads it. + const repoDir = join(process.cwd(), ".gitmem"); + const configPath = join(repoDir, "config.json"); if (!existsSync(configPath)) { - const config = {}; - if (projectArg) config.project = projectArg; - writeFileSync(configPath, JSON.stringify(config, null, 2)); if (projectArg) { + mkdirSync(repoDir, { recursive: true }); + writeFileSync(configPath, JSON.stringify({ project: projectArg }, null, 2)); console.log(` + Created .gitmem/config.json (project: "${projectArg}")`); - } else { - console.log(" + Created .gitmem/config.json"); } } else if (projectArg) { // Config exists — update project field @@ -205,7 +206,7 @@ async function cmdInit() { } console.log(""); - console.log(`Done: ${added} new scars added to .gitmem/learnings.json`); + console.log(`Done: ${added} new scars added to ${storeName}/learnings.json`); console.log(""); console.log("Add .gitmem/ to your .gitignore:"); console.log(" echo '.gitmem/' >> .gitignore"); diff --git a/bin/hooks-merge.js b/bin/hooks-merge.js new file mode 100644 index 0000000..9a8b11c --- /dev/null +++ b/bin/hooks-merge.js @@ -0,0 +1,130 @@ +/** + * GIT-120: merge gitmem's hooks into a settings file without touching anyone + * else's. + * + * install-hooks used to replace `settings.hooks` wholesale and uninstall-hooks + * deleted the key, so every hook the user or another tool had configured was + * lost. These helpers work per event and per command: only commands gitmem + * wrote are replaced or removed, and the file is backed up before any write. + * + * Two shapes are supported: + * Claude Code hooks[event] = [{ matcher?, hooks: [{ type, command, ... }] }] + * Cursor hooks[event] = [{ command, ... }] + */ + +import { copyFileSync, existsSync } from "fs"; + +/** + * A command gitmem installed: the per-repo copy (.gitmem/hooks/), the older + * node_modules path, or the legacy plugin. A bare "gitmem" substring is not + * enough — a user's own script can live in a directory called gitmem. + */ +export function isGitmemCommand(command) { + if (typeof command !== "string") return false; + return /(^|[\s"'=/])\.gitmem\/hooks\/[\w.-]+\.sh\b/.test(command) + || /gitmem-mcp\/hooks\/scripts\/[\w.-]+\.sh\b/.test(command) + || /\bgitmem-hooks\b/.test(command); +} + +const isGroup = (entry) => entry && typeof entry === "object" && Array.isArray(entry.hooks); + +/** Commands in one event's entries, as [gitmem, other] counts. */ +function countEntries(entries) { + let gitmem = 0; + let other = 0; + for (const entry of Array.isArray(entries) ? entries : []) { + const commands = isGroup(entry) ? entry.hooks : [entry]; + for (const h of commands) { + if (isGitmemCommand(h?.command)) gitmem++; else other++; + } + } + return { gitmem, other }; +} + +/** + * One event's entries with gitmem's commands taken out. A Claude group keeps + * its other commands; a group left empty is dropped. + */ +function withoutGitmem(entries) { + const kept = []; + for (const entry of Array.isArray(entries) ? entries : []) { + if (isGroup(entry)) { + const others = entry.hooks.filter((h) => !isGitmemCommand(h?.command)); + if (others.length === entry.hooks.length) kept.push(entry); + else if (others.length > 0) kept.push({ ...entry, hooks: others }); + } else if (!isGitmemCommand(entry?.command)) { + kept.push(entry); + } + } + return kept; +} + +/** The hooks object must be a plain object of arrays, or absent. */ +export function hooksShapeError(hooks) { + if (hooks === undefined) return null; + if (!hooks || typeof hooks !== "object" || Array.isArray(hooks)) return "hooks is not an object"; + for (const [event, entries] of Object.entries(hooks)) { + if (!Array.isArray(entries)) return `hooks.${event} is not an array`; + } + return null; +} + +/** True when any command in the hooks is one gitmem installed. */ +export function hasGitmemHooks(hooks) { + return Object.values(hooks || {}).some((entries) => countEntries(entries).gitmem > 0); +} + +/** + * Merge gitmem's hooks in: per event, drop the gitmem commands already there, + * keep everything else, append gitmem's current entries. + * Returns the new hooks and one change line per event touched. + */ +export function mergeGitmemHooks(existing, gitmemHooks) { + const hooks = { ...(existing || {}) }; + const changes = []; + for (const [event, entries] of Object.entries(gitmemHooks)) { + const before = countEntries(hooks[event]); + const added = countEntries(entries).gitmem; + hooks[event] = [...withoutGitmem(hooks[event]), ...entries]; + changes.push(`${event}: ${before.gitmem ? `replaced ${before.gitmem} gitmem hook(s) with ${added}` : `added ${added} gitmem hook(s)`}${before.other ? `, kept ${before.other} other` : ""}`); + } + // Events this installer does not manage are left exactly as they are. That + // includes gitmem's own UserPromptSubmit hook, which `gitmem init` installs + // and install-hooks does not. + for (const [event, entries] of Object.entries(hooks)) { + if (event in gitmemHooks) continue; + const c = countEntries(entries); + if (c.gitmem + c.other) changes.push(`${event}: kept ${c.gitmem + c.other} (untouched)`); + } + return { hooks, changes }; +} + +/** + * Take gitmem's commands out of every event. Events left empty are dropped; + * hooks is undefined when nothing remains. + */ +export function removeGitmemHooks(existing) { + const hooks = {}; + const changes = []; + let removed = 0; + let kept = 0; + for (const [event, entries] of Object.entries(existing || {})) { + const c = countEntries(entries); + removed += c.gitmem; + kept += c.other; + const rest = withoutGitmem(entries); + if (rest.length) hooks[event] = rest; + if (c.gitmem) changes.push(`${event}: removed ${c.gitmem} gitmem hook(s)${c.other ? `, kept ${c.other} other` : ""}`); + else if (c.other) changes.push(`${event}: kept ${c.other} other (untouched)`); + } + return { hooks: Object.keys(hooks).length ? hooks : undefined, removed, kept, changes }; +} + +/** Copy a file aside before it is rewritten; returns the backup path or null. */ +export function backupFile(path) { + if (!existsSync(path)) return null; + const stamp = new Date().toISOString().replace(/[:.]/g, "-"); + const backup = `${path}.gitmem-backup-${stamp}`; + copyFileSync(path, backup); + return backup; +} diff --git a/bin/init-wizard.js b/bin/init-wizard.js index 261d4f1..e54f115 100644 --- a/bin/init-wizard.js +++ b/bin/init-wizard.js @@ -20,6 +20,8 @@ import { import { dirname, join } from "path"; import { fileURLToPath } from "url"; import { createInterface } from "readline/promises"; +import { storeRoot, repoGitmemDir, displayPath } from "./gitmem-root.js"; +import { hasGitmemHooks, mergeGitmemHooks, removeGitmemHooks, backupFile } from "./hooks-merge.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); const cwd = process.cwd(); @@ -166,7 +168,11 @@ const CLIENT_CONFIGS = { }; // Shared paths -const gitmemDir = join(cwd, ".gitmem"); +// GIT-115: the store goes where the server reads it; the repo keeps only +// config.json (project name, read by the SessionStart hook) and the hooks. +const repoDir = repoGitmemDir(cwd); +const storeDir = storeRoot(); +const storeName = displayPath(storeDir); const gitignorePath = join(cwd, ".gitignore"); const starterScarsPath = join(__dirname, "..", "schema", "starter-scars.json"); const hooksScriptsDir = join(__dirname, "..", "hooks", "scripts"); @@ -351,14 +357,9 @@ function buildCursorHooks() { }; } -function isGitmemHook(entry) { - if (entry.hooks && Array.isArray(entry.hooks)) { - return entry.hooks.some((h) => typeof h.command === "string" && h.command.includes("gitmem")); - } - if (typeof entry.command === "string") { - return entry.command.includes("gitmem"); - } - return false; +/** Commands in a hooks object that are not gitmem's (GIT-120 matching). */ +function countOtherHooks(hooks) { + return removeGitmemHooks(hooks).kept; } function getInstructionsTemplate() { @@ -370,7 +371,7 @@ function getInstructionsTemplate() { } function copyHookScripts() { - const destHooksDir = join(gitmemDir, "hooks"); + const destHooksDir = join(repoDir, "hooks"); if (!existsSync(destHooksDir)) { mkdirSync(destHooksDir, { recursive: true }); } @@ -393,8 +394,22 @@ function copyHookScripts() { // ── Steps ── // Each returns { done: bool } so main can track progress +/** + * GIT-115: the repo's .gitmem/config.json carries the project name, which the + * SessionStart hook reads from the repo. It is not the store's config.json. + */ +function writeRepoConfig() { + if (dryRun) return; + const configPath = join(repoDir, "config.json"); + const config = readJson(configPath) || {}; + if (projectName) config.project = projectName; + else if (existsSync(configPath)) return; + mkdirSync(repoDir, { recursive: true }); + writeJson(configPath, config); +} + async function stepMemoryStore() { - const learningsPath = join(gitmemDir, "learnings.json"); + const learningsPath = join(storeDir, "learnings.json"); const exists = existsSync(learningsPath); let existingCount = 0; if (exists) { @@ -412,15 +427,16 @@ async function stepMemoryStore() { } if (exists && existingCount >= starterScars.length) { - log(CHECK, `Memory store already set up ${C.dim}(${existingCount} lessons in .gitmem/)${C.reset}`); + writeRepoConfig(); + log(CHECK, `Memory store already set up ${C.dim}(${existingCount} lessons in ${storeName})${C.reset}`); return { done: false }; } // Needs merge — prompt if existing data OR interactive mode if (exists || interactive) { const prompt = exists - ? `Merge ${starterScars.length} lessons into .gitmem/? (${existingCount} existing)` - : `Create .gitmem/ with ${starterScars.length} starter lessons?`; + ? `Merge ${starterScars.length} lessons into ${storeName}? (${existingCount} existing)` + : `Create ${storeName} with ${starterScars.length} starter lessons?`; if (!(await confirm(prompt))) { log(SKIP, "Memory store skipped"); return { done: false }; @@ -428,28 +444,18 @@ async function stepMemoryStore() { } if (dryRun) { - log(CHECK, `Would create .gitmem/ with ${starterScars.length} starter lessons`, "[dry-run]"); + log(CHECK, `Would create ${storeName} with ${starterScars.length} starter lessons`, "[dry-run]"); return { done: true }; } - if (!existsSync(gitmemDir)) { - mkdirSync(gitmemDir, { recursive: true }); + if (!existsSync(storeDir)) { + mkdirSync(storeDir, { recursive: true }); } - // Config - const configPath = join(gitmemDir, "config.json"); - if (!existsSync(configPath)) { - const config = { feedback_enabled: false, telemetry_enabled: false }; - if (projectName) config.project = projectName; - writeJson(configPath, config); - } else if (projectName) { - const config = readJson(configPath) || {}; - config.project = projectName; - writeJson(configPath, config); - } + writeRepoConfig(); // Closing payload template (prevents permission prompt on first session close) - const payloadPath = join(gitmemDir, "closing-payload.json"); + const payloadPath = join(storeDir, "closing-payload.json"); if (!existsSync(payloadPath)) { writeJson(payloadPath, { closing_reflection: { @@ -495,7 +501,7 @@ async function stepMemoryStore() { writeJson(learningsPath, existing); // Starter thread — nudges user to add their own project-specific scar - const threadsPath = join(gitmemDir, "threads.json"); + const threadsPath = join(storeDir, "threads.json"); if (!existsSync(threadsPath)) { writeJson(threadsPath, [ { @@ -509,14 +515,14 @@ async function stepMemoryStore() { // Empty collection files for (const file of ["sessions.json", "decisions.json", "scar-usage.json"]) { - const filePath = join(gitmemDir, file); + const filePath = join(storeDir, file); if (!existsSync(filePath)) { writeFileSync(filePath, "[]"); } } // Closing payload template - const templatePath = join(gitmemDir, "closing-payload-template.json"); + const templatePath = join(storeDir, "closing-payload-template.json"); if (!existsSync(templatePath)) { writeJson(templatePath, { closing_reflection: { @@ -541,7 +547,7 @@ async function stepMemoryStore() { : ""; log(CHECK, - "Created .gitmem/ \u2014 your local memory store", + `Created ${storeName} \u2014 your local memory store`, `${starterScars.length} lessons from common mistakes included${mergeNote}` ); return { done: true }; @@ -722,19 +728,14 @@ async function stepHooks() { async function stepHooksClaude() { const existing = readJson(cc.settingsFile); const hooks = existing?.hooks || {}; - const hasGitmem = JSON.stringify(hooks).includes("gitmem"); + const hasGitmem = hasGitmemHooks(hooks); if (hasGitmem) { log(CHECK, `Automatic memory hooks already configured`); return { done: false }; } - let existingHookCount = 0; - for (const entries of Object.values(hooks)) { - if (Array.isArray(entries)) { - existingHookCount += entries.filter((e) => !isGitmemHook(e)).length; - } - } + const existingHookCount = countOtherHooks(hooks); // Existing hooks — prompt for merge if (existingHookCount > 0 || interactive) { @@ -759,16 +760,9 @@ async function stepHooksClaude() { mkdirSync(cc.configDir, { recursive: true }); } - const gitmemHooks = buildClaudeHooks(); - const merged = { ...(settings.hooks || {}) }; - - for (const [eventType, gitmemEntries] of Object.entries(gitmemHooks)) { - const existingEntries = merged[eventType] || []; - const nonGitmem = existingEntries.filter((e) => !isGitmemHook(e)); - merged[eventType] = [...nonGitmem, ...gitmemEntries]; - } - - settings.hooks = merged; + // GIT-120 merge: per event and per command; other hooks untouched. + settings.hooks = mergeGitmemHooks(settings.hooks, buildClaudeHooks()).hooks; + backupFile(cc.settingsFile); writeJson(cc.settingsFile, settings); const preserveNote = existingHookCount > 0 @@ -795,21 +789,14 @@ async function stepHooksCursor() { const hooksName = cc.hooksFileName; const existing = readJson(hooksPath); - const hasGitmem = existing ? JSON.stringify(existing).includes("gitmem") : false; + const hasGitmem = hasGitmemHooks(existing?.hooks); if (hasGitmem) { log(CHECK, `Automatic memory hooks already configured ${C.dim}(${hooksName})${C.reset}`); return { done: false }; } - let existingHookCount = 0; - if (existing?.hooks) { - for (const entries of Object.values(existing.hooks)) { - if (Array.isArray(entries)) { - existingHookCount += entries.filter((e) => !isGitmemHook(e)).length; - } - } - } + const existingHookCount = countOtherHooks(existing?.hooks); if (existingHookCount > 0 || interactive) { const prompt = existingHookCount > 0 @@ -832,17 +819,9 @@ async function stepHooksCursor() { mkdirSync(cc.configDir, { recursive: true }); } - const gitmemHooks = buildCursorHooks(); const config = existing || {}; - const merged = { ...(config.hooks || {}) }; - - for (const [eventType, gitmemEntries] of Object.entries(gitmemHooks)) { - const existingEntries = merged[eventType] || []; - const nonGitmem = existingEntries.filter((e) => !isGitmemHook(e)); - merged[eventType] = [...nonGitmem, ...gitmemEntries]; - } - - config.hooks = merged; + config.hooks = mergeGitmemHooks(config.hooks, buildCursorHooks()).hooks; + backupFile(hooksPath); writeJson(hooksPath, config); const preserveNote = existingHookCount > 0 @@ -937,7 +916,8 @@ async function stepAgentsMd() { } async function stepFeedbackOptIn() { - const configPath = join(gitmemDir, "config.json"); + // The server reads feedback_enabled from the store's config.json. + const configPath = join(storeDir, "config.json"); const config = readJson(configPath) || {}; // Already opted in — skip @@ -950,7 +930,7 @@ async function stepFeedbackOptIn() { if (!interactive && autoYes) { log(CHECK, "Anonymous feedback sharing is off", - "Run with --interactive to enable, or set feedback_enabled in .gitmem/config.json" + `Run with --interactive to enable, or set feedback_enabled in ${storeName}/config.json` ); return { done: false }; } @@ -964,7 +944,7 @@ async function stepFeedbackOptIn() { if (!accepted) { log(CHECK, "Anonymous feedback sharing is off", - "You can enable it later in .gitmem/config.json" + `You can enable it later in ${storeName}/config.json` ); return { done: false }; } @@ -975,6 +955,7 @@ async function stepFeedbackOptIn() { } config.feedback_enabled = true; + mkdirSync(storeDir, { recursive: true }); writeJson(configPath, config); log(CHECK, diff --git a/bin/uninstall.js b/bin/uninstall.js index f4b12ff..02c5224 100644 --- a/bin/uninstall.js +++ b/bin/uninstall.js @@ -14,10 +14,13 @@ import { writeFileSync, existsSync, rmSync, + readdirSync, } from "fs"; import { dirname, join } from "path"; import { fileURLToPath } from "url"; import { createInterface } from "readline/promises"; +import { storeRoot, repoGitmemDir, displayPath } from "./gitmem-root.js"; +import { removeGitmemHooks, backupFile } from "./hooks-merge.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); const cwd = process.cwd(); @@ -122,7 +125,10 @@ function detectClient() { const client = detectClient(); const cc = CLIENT_CONFIGS[client]; -const gitmemDir = join(cwd, ".gitmem"); +// GIT-115: the repo's .gitmem/ (config.json + hooks) and the store the +// server reads are separate, and handled separately below. +const repoDir = repoGitmemDir(cwd); +const storeDir = storeRoot(); const gitignorePath = join(cwd, ".gitignore"); let rl; @@ -153,18 +159,6 @@ function writeJson(path, data) { writeFileSync(path, JSON.stringify(data, null, 2) + "\n"); } -function isGitmemHook(entry) { - if (entry.hooks && Array.isArray(entry.hooks)) { - return entry.hooks.some( - (h) => typeof h.command === "string" && h.command.includes("gitmem") - ); - } - if (typeof entry.command === "string") { - return entry.command.includes("gitmem"); - } - return false; -} - // ── Steps ── function stepInstructions() { @@ -232,94 +226,34 @@ function stepHooks() { return stepHooksCursor(); } -function stepHooksClaude() { - const settings = readJson(cc.settingsFile); - if (!settings?.hooks) { - log(SKIP, "No hooks in .claude/settings.json"); +/** + * GIT-120 matching: remove only the commands gitmem installed. A matcher group + * keeps its other commands; the hooks key goes only when nothing is left. + */ +function removeHooksFrom(file, name) { + const cfg = readJson(file); + if (!cfg?.hooks) { + log(SKIP, `No hooks in ${name}`); return; } - - let removed = 0; - let preserved = 0; - const cleaned = {}; - - for (const [eventType, entries] of Object.entries(settings.hooks)) { - if (!Array.isArray(entries)) continue; - const nonGitmem = entries.filter((e) => { - if (isGitmemHook(e)) { - removed++; - return false; - } - preserved++; - return true; - }); - if (nonGitmem.length > 0) { - cleaned[eventType] = nonGitmem; - } - } - - if (removed === 0) { + const result = removeGitmemHooks(cfg.hooks); + if (result.removed === 0) { log(SKIP, "No gitmem hooks found"); return; } + if (result.hooks) cfg.hooks = result.hooks; else delete cfg.hooks; + const backup = backupFile(file); + writeJson(file, cfg); + const kept = result.kept > 0 ? `(${result.kept} other hook${result.kept !== 1 ? "s" : ""} preserved)` : ""; + log(CHECK, "Removed automatic memory hooks", [kept, backup ? `backup: ${displayPath(backup)}` : ""].filter(Boolean).join(" ")); +} - if (Object.keys(cleaned).length > 0) { - settings.hooks = cleaned; - } else { - delete settings.hooks; - } - - writeJson(cc.settingsFile, settings); - if (preserved > 0) { - log(CHECK, "Removed automatic memory hooks", `(${preserved} other hook${preserved !== 1 ? "s" : ""} preserved)`); - } else { - log(CHECK, "Removed automatic memory hooks"); - } +function stepHooksClaude() { + removeHooksFrom(cc.settingsFile, ".claude/settings.json"); } function stepHooksCursor() { - const config = readJson(cc.hooksFile); - if (!config?.hooks) { - log(SKIP, `No hooks in ${cc.hooksFileName}`); - return; - } - - let removed = 0; - let preserved = 0; - const cleaned = {}; - - for (const [eventType, entries] of Object.entries(config.hooks)) { - if (!Array.isArray(entries)) continue; - const nonGitmem = entries.filter((e) => { - if (isGitmemHook(e)) { - removed++; - return false; - } - preserved++; - return true; - }); - if (nonGitmem.length > 0) { - cleaned[eventType] = nonGitmem; - } - } - - if (removed === 0) { - log(SKIP, "No gitmem hooks found"); - return; - } - - if (Object.keys(cleaned).length > 0) { - config.hooks = cleaned; - } else { - delete config.hooks; - } - - writeJson(cc.hooksFile, config); - if (preserved > 0) { - log(CHECK, "Removed automatic memory hooks", `(${preserved} other hook${preserved !== 1 ? "s" : ""} preserved)`); - } else { - log(CHECK, "Removed automatic memory hooks"); - } + removeHooksFrom(cc.hooksFile, cc.hooksFileName); } function stepPermissions() { @@ -359,23 +293,51 @@ function stepPermissions() { log(CHECK, "Removed tool permissions"); } +/** + * GIT-115: the repo's .gitmem/ holds gitmem's config.json and hooks/, which + * are this repo's integration and go with it. Anything else in there (a store + * from an older install) is left alone. + */ +function stepRepoDir() { + if (!existsSync(repoDir)) { + log(SKIP, "No .gitmem/ in this repo"); + return; + } + for (const entry of ["config.json", "hooks"]) { + rmSync(join(repoDir, entry), { recursive: true, force: true }); + } + const rest = readdirSync(repoDir); + if (rest.length === 0) { + rmSync(repoDir, { recursive: true, force: true }); + log(CHECK, "Removed .gitmem/ from this repo", "(config.json and hooks)"); + } else { + log(CHECK, "Removed .gitmem/config.json and .gitmem/hooks/", + `(${rest.length} other file${rest.length !== 1 ? "s" : ""} left in .gitmem/)`); + } +} + +/** + * The memory store is shared by every project on this machine, so it is kept + * unless --all is given or the user says to delete it. + */ async function stepGitmemDir() { - if (!existsSync(gitmemDir)) { - log(SKIP, "No .gitmem/ directory"); + const name = displayPath(storeDir); + if (!existsSync(storeDir)) { + log(SKIP, `No memory store at ${name}`); return; } if (deleteAll) { - rmSync(gitmemDir, { recursive: true, force: true }); - log(CHECK, "Deleted .gitmem/ directory"); + rmSync(storeDir, { recursive: true, force: true }); + log(CHECK, `Deleted memory store ${name}`); return; } - if (await confirm("Keep .gitmem/ memory data for future use?", true)) { - log(CHECK, ".gitmem/ preserved — your memories will be here if you reinstall"); + if (await confirm(`Keep memory store ${name}? (shared by every project on this machine)`, true)) { + log(CHECK, `${name} preserved — your memories will be there if you reinstall`); } else { - rmSync(gitmemDir, { recursive: true, force: true }); - log(CHECK, "Deleted .gitmem/ directory"); + rmSync(storeDir, { recursive: true, force: true }); + log(CHECK, `Deleted memory store ${name}`); } } @@ -406,6 +368,7 @@ async function main() { stepMcpJson(); stepHooks(); stepPermissions(); + stepRepoDir(); await stepGitmemDir(); stepGitignore(); diff --git a/hooks/scripts/session-start.sh b/hooks/scripts/session-start.sh index 8e11fbc..d50f77b 100755 --- a/hooks/scripts/session-start.sh +++ b/hooks/scripts/session-start.sh @@ -191,9 +191,15 @@ if [ "$GITMEM_DETECTED" = "true" ]; then # # Read project from .gitmem/config.json if available. # Fallback: let session_start default (reads config.json server-side too). + # GIT-115: this is the repo's config.json, not the store's; it stays in the + # repo so each repo keeps its own project. node when jq is absent. GITMEM_PROJECT="" - if [ -f ".gitmem/config.json" ] && command -v jq &>/dev/null; then - GITMEM_PROJECT=$(jq -r '.project // empty' .gitmem/config.json 2>/dev/null || true) + if [ -f ".gitmem/config.json" ]; then + if command -v jq &>/dev/null; then + GITMEM_PROJECT=$(jq -r '.project // empty' .gitmem/config.json 2>/dev/null || true) + elif command -v node &>/dev/null; then + GITMEM_PROJECT=$(node -e "try{const p=JSON.parse(require('fs').readFileSync('.gitmem/config.json','utf8')).project;if(typeof p==='string')process.stdout.write(p)}catch(e){}" 2>/dev/null || true) + fi fi if [ -n "$GITMEM_PROJECT" ]; then diff --git a/hooks/tests/test-hooks.sh b/hooks/tests/test-hooks.sh index 95d5df8..c2b0d75 100755 --- a/hooks/tests/test-hooks.sh +++ b/hooks/tests/test-hooks.sh @@ -128,6 +128,22 @@ else pass "Output is plain text, not JSON" fi +# Test 1.2b (GIT-115): the project hint comes from the repo's .gitmem/config.json, +# with or without jq (node fallback). +# A repo of its own: the suite's GITMEM_DIR ($TMPDIR/.gitmem) is the store. +mkdir -p "$TMPDIR/repo-git115/.gitmem" +cp "$TMPDIR/.mcp.json" "$TMPDIR/repo-git115/.mcp.json" +echo '{"project":"acme-hooks"}' > "$TMPDIR/repo-git115/.gitmem/config.json" +OUTPUT=$(cd "$TMPDIR/repo-git115" && echo '{}' | bash "$SCRIPT_DIR/scripts/session-start.sh" 2>/dev/null) +if echo "$OUTPUT" | grep -q 'session_start(project: "acme-hooks")'; then + pass "Project hint read from the repo's .gitmem/config.json" +else + fail "Project hint read from the repo's .gitmem/config.json" \ + 'Contains session_start(project: "acme-hooks")' \ + "$OUTPUT" +fi +rm -rf "$TMPDIR/repo-git115" + # Test 1.3: Gitmem NOT in .mcp.json (may still detect via disk fallback) rm "$TMPDIR/.mcp.json" echo '{"mcpServers":{"other-tool":{}}}' > "$TMPDIR/.mcp.json" diff --git a/src/commands/telemetry.ts b/src/commands/telemetry.ts index 80068a5..b01838c 100644 --- a/src/commands/telemetry.ts +++ b/src/commands/telemetry.ts @@ -9,6 +9,8 @@ */ import { getTelemetry, Telemetry } from "../lib/telemetry.js"; +// GIT-115: telemetry state lives in the store the server reads, not /.gitmem. +import { getGitmemDir } from "../services/gitmem-dir.js"; import { join } from "path"; import { readFileSync } from "fs"; @@ -23,9 +25,6 @@ function getPackageVersion(): string { } } -function getGitmemDir(): string { - return join(process.cwd(), ".gitmem"); -} export function main(args: string[]): void { const subcommand = args[0]; @@ -117,7 +116,7 @@ function cmdEnable(telemetry: Telemetry): void { console.log(" ✗ IP addresses or persistent IDs"); console.log(""); console.log("Transparency:"); - console.log(" • All events logged to .gitmem/telemetry.log"); + console.log(` • All events logged to ${join(getGitmemDir(), "telemetry.log")}`); console.log(" • View before sending: gitmem telemetry show"); console.log(" • Disable anytime: gitmem telemetry disable"); console.log(""); @@ -139,7 +138,7 @@ function cmdEnable(telemetry: Telemetry): void { telemetry.enable(); console.log(""); console.log("\x1b[32m✓\x1b[0m Telemetry enabled"); - console.log(" Data logged to: .gitmem/telemetry.log"); + console.log(` Data logged to: ${join(getGitmemDir(), "telemetry.log")}`); console.log(" Review anytime: gitmem telemetry show"); console.log(" Disable anytime: gitmem telemetry disable"); } else { @@ -160,7 +159,7 @@ function cmdDisable(telemetry: Telemetry): void { console.log("==========================\n"); console.log("\x1b[32m✓\x1b[0m Telemetry disabled"); console.log(" Pending events: will not be sent"); - console.log(" Local logs: preserved at .gitmem/telemetry.log"); + console.log(` Local logs: preserved at ${join(getGitmemDir(), "telemetry.log")}`); console.log(""); console.log("To re-enable: gitmem telemetry enable"); } diff --git a/src/services/gitmem-dir.ts b/src/services/gitmem-dir.ts index feba098..3f11d8d 100644 --- a/src/services/gitmem-dir.ts +++ b/src/services/gitmem-dir.ts @@ -15,6 +15,7 @@ import * as path from "path"; import * as fs from "fs"; import * as os from "os"; +import { fileURLToPath } from "url"; /** * Validate a string intended for use as a single path component (directory name or filename). @@ -272,9 +273,16 @@ function warnAboutStrandedProjectRoots(home: string): void { * without ever having held a session. * * Any ONE of these counts: - * - config.json a deliberate project-scoped install * - a registered session the registry names at least one * - a real session dir sessions//session.json parses with a session_id + * - a real record a learning, thread or decision that `init` did not seed + * + * GIT-115: config.json no longer counts. A repo's .gitmem/ now holds its + * config.json (the project name the SessionStart hook reads) and the hook + * scripts on purpose, while the store lives in the root the server reads. A + * repo set up that way is not a stranded store. Starter lessons and the + * welcome thread are not evidence either: every older `init` seeded them into + * the repo, and the root being read has its own copy. * * Exported for tests and diagnostics; the resolution path is the only caller * that matters. @@ -283,8 +291,7 @@ export function isLiveGitmemRoot(candidate: string): boolean { try { if (!fs.existsSync(candidate)) return false; - // A project-scoped install is deliberate — honour it even when idle. - if (fs.existsSync(path.join(candidate, "config.json"))) return true; + if (holdsRealRecords(candidate)) return true; const registryPath = path.join(candidate, "active-sessions.json"); if (fs.existsSync(registryPath)) { @@ -317,6 +324,43 @@ export function isLiveGitmemRoot(candidate: string): boolean { } } +/** What `init` seeds: starter lesson ids and the welcome thread (GIT-115). */ +const WELCOME_THREAD_ID = "t-welcome01"; +let starterIds: Set | null = null; +function starterLearningIds(): Set { + if (starterIds) return starterIds; + starterIds = new Set(); + try { + // src/services or dist/services → /schema/starter-scars.json + const here = path.dirname(fileURLToPath(import.meta.url)); + const file = path.join(here, "..", "..", "schema", "starter-scars.json"); + for (const s of JSON.parse(fs.readFileSync(file, "utf-8"))) if (s?.id) starterIds.add(String(s.id)); + } catch { + // Unreadable: every learning then counts, which errs toward reporting. + } + return starterIds; +} + +/** Entries of a store file that may be a bare array or {key: array}. */ +function readCollection(file: string, key: string): Array> { + try { + if (!fs.existsSync(file)) return []; + const parsed = JSON.parse(fs.readFileSync(file, "utf-8")); + const list = Array.isArray(parsed) ? parsed : parsed?.[key]; + return Array.isArray(list) ? list : []; + } catch { + return []; + } +} + +/** GIT-115: a learning, thread or decision that `init` did not seed. */ +function holdsRealRecords(root: string): boolean { + const starters = starterLearningIds(); + if (readCollection(path.join(root, "learnings.json"), "learnings").some((l) => !starters.has(String(l?.id)))) return true; + if (readCollection(path.join(root, "threads.json"), "threads").some((t) => String(t?.id) !== WELCOME_THREAD_ID)) return true; + return readCollection(path.join(root, "decisions.json"), "decisions").length > 0; +} + /** * Get a file path within the .gitmem directory */ diff --git a/tests/e2e/cli-fresh-install.test.ts b/tests/e2e/cli-fresh-install.test.ts index d7d6e3e..49f81fd 100644 --- a/tests/e2e/cli-fresh-install.test.ts +++ b/tests/e2e/cli-fresh-install.test.ts @@ -17,6 +17,7 @@ import { execFile as execFileCb } from "child_process"; import { promisify } from "util"; import { mkdirSync, + mkdtempSync, rmSync, existsSync, readFileSync, @@ -38,6 +39,11 @@ import { } from "./mcp-client.js"; const execFile = promisify(execFileCb); + +// GIT-115: init writes the store to the root the server reads (~/.gitmem). +// Every CLI call here runs against a scratch HOME, never the developer's. +const SANDBOX_HOME = mkdtempSync(join(tmpdir(), "gitmem-cli-home-")); +afterAll(() => rmSync(SANDBOX_HOME, { recursive: true, force: true })); const GITMEM_BIN = join(__dirname, "../../bin/gitmem.js"); /** @@ -52,6 +58,9 @@ async function runGitmem( ): Promise<{ stdout: string; stderr: string; exitCode: number }> { const env = { ...process.env, + HOME: SANDBOX_HOME, + GITMEM_DIR: "", + GITMEM_HOME: "", // Ensure free tier unless overridden SUPABASE_URL: "", SUPABASE_SERVICE_ROLE_KEY: "", @@ -90,15 +99,16 @@ describe("Fresh Install: Free Tier CLI", () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); }); - it("gitmem init creates .gitmem/ with starter scars", async () => { + it("gitmem init creates the store in ~/.gitmem with starter scars", async () => { const { stdout, exitCode } = await runGitmem(["init", "--yes"], { cwd: TEST_DIR }); expect(exitCode).toBe(0); - expect(stdout.toLowerCase()).toContain("free"); + expect(stdout).toContain("Created ~/.gitmem"); - // .gitmem/ directory created - const gitmemDir = join(TEST_DIR, ".gitmem"); + // GIT-115: the store is where the server reads it, not /.gitmem + const gitmemDir = join(SANDBOX_HOME, ".gitmem"); expect(existsSync(gitmemDir)).toBe(true); + expect(existsSync(join(TEST_DIR, ".gitmem", "learnings.json"))).toBe(false); // learnings.json has starter scars const learningsPath = join(gitmemDir, "learnings.json"); @@ -112,20 +122,20 @@ describe("Fresh Install: Free Tier CLI", () => { expect(existsSync(join(gitmemDir, "scar-usage.json"))).toBe(true); // stdout shows scar count - const countMatch = stdout.match(/(\d+) starter scars/) || stdout.match(/(\d+) new scars added/); + const countMatch = stdout.match(/(\d+) lessons from common mistakes/); expect(countMatch).not.toBeNull(); expect(parseInt(countMatch![1])).toBeGreaterThan(0); }); it("gitmem init is idempotent (no duplicates on re-run)", async () => { // Count scars before second run - const learningsPath = join(TEST_DIR, ".gitmem", "learnings.json"); + const learningsPath = join(SANDBOX_HOME, ".gitmem", "learnings.json"); const beforeCount = JSON.parse(readFileSync(learningsPath, "utf-8")).length; const { stdout, exitCode } = await runGitmem(["init", "--yes"], { cwd: TEST_DIR }); expect(exitCode).toBe(0); - expect(stdout.toLowerCase()).toContain("already configured"); + expect(stdout.toLowerCase()).toContain("already set up"); // Same count — no duplicates const afterCount = JSON.parse(readFileSync(learningsPath, "utf-8")).length; @@ -338,15 +348,16 @@ describe("Fresh Install: MCP Server Lifecycle", () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); mkdirSync(TEST_DIR, { recursive: true }); - // Run init to populate .gitmem/ + // Run init to populate the store (GIT-115: ~/.gitmem, here the sandbox HOME) await runGitmem(["init", "--yes"], { cwd: TEST_DIR }); - // Start MCP server with CWD in the initialized directory - // so it walks up and finds .gitmem/ with starter scars + // Start the MCP server on the same HOME, so it reads what init wrote mcp = await createMcpClient( { ...createTierEnv("free"), - HOME: TEST_DIR, + HOME: SANDBOX_HOME, + GITMEM_DIR: "", + GITMEM_HOME: "", }, { cwd: TEST_DIR } ); @@ -556,6 +567,9 @@ describe("Fresh Install: Hook Script Output", () => { ...process.env, CLAUDE_SESSION_ID: sessionId, HOME: TEST_DIR, + // GIT-99: the hook reads the registry from the server's root, not + // its cwd; name the test's root explicitly. + GITMEM_DIR: join(sessionDir, ".gitmem"), }, timeout: 10_000, } diff --git a/tests/e2e/git-115-fresh-install.test.ts b/tests/e2e/git-115-fresh-install.test.ts new file mode 100644 index 0000000..429c80d --- /dev/null +++ b/tests/e2e/git-115-fresh-install.test.ts @@ -0,0 +1,156 @@ +/** + * GIT-115: a fresh install reads what init wrote. + * + * init seeded /.gitmem while the server reads ~/.gitmem (GIT-91), so a + * new user's first session had no starter lessons and was told its store was + * "NOT being read". Now the store goes to the root the server reads, and the + * repo keeps only config.json (the project name) and the hook scripts. + * + * Clean HOME, clean repo, the real CLI, the real SessionStart hook and the + * built server. No Supabase, no Docker: safe for CI. + */ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { execFile as execFileCb } from "child_process"; +import { promisify } from "util"; +import { mkdtempSync, mkdirSync, rmSync, readFileSync, writeFileSync, readdirSync, existsSync, realpathSync } from "fs"; +import { join } from "path"; +import { tmpdir } from "os"; +import { createMcpClient, callTool, getToolResultText, isToolError, type McpTestClient } from "./mcp-client.js"; + +const execFile = promisify(execFileCb); +const GITMEM_BIN = join(__dirname, "../../bin/gitmem.js"); +const STARTER_COUNT = JSON.parse(readFileSync(join(__dirname, "../../schema/starter-scars.json"), "utf-8")).length; +const PROJECT = "acme-git115"; + +let home: string; +let repo: string; +let mcp: McpTestClient | undefined; + +/** The environment of a new user: nothing gitmem-specific set. */ +function cleanEnv(): Record { + const env: Record = {}; + for (const [k, v] of Object.entries(process.env)) { + if (v !== undefined && !/^(GITMEM_|SUPABASE_|OPENAI_|OPENROUTER_)/.test(k)) env[k] = v; + } + // GITMEM_DIR/GITMEM_HOME blank as well: createMcpClient layers process.env underneath. + return { ...env, HOME: home, GITMEM_DIR: "", GITMEM_HOME: "", NO_COLOR: "1", GITMEM_TIER: "free", SUPABASE_URL: "", SUPABASE_SERVICE_ROLE_KEY: "" }; +} + +async function run(cmd: string, args: string[], input?: string) { + const p = execFile(cmd, args, { cwd: repo, env: cleanEnv(), timeout: 60_000 }); + if (input !== undefined) { p.child.stdin?.end(input); } + try { + const { stdout, stderr } = await p; + return { stdout, stderr, code: 0 }; + } catch (e: any) { + return { stdout: e.stdout ?? "", stderr: e.stderr ?? "", code: typeof e.code === "number" ? e.code : 1 }; + } +} + +let initOut = ""; +let hookOut = ""; +let startText = ""; +let recallText = ""; + +beforeAll(async () => { + home = realpathSync(mkdtempSync(join(tmpdir(), "gitmem-git115-home-"))); + repo = realpathSync(mkdtempSync(join(tmpdir(), "gitmem-git115-repo-"))); + + const init = await run("node", [GITMEM_BIN, "init", "--yes", "--project", PROJECT, "--client", "claude"]); + initOut = init.stdout + init.stderr; + + // The SessionStart hook, as Claude Code runs it: from the repo, JSON on stdin. + hookOut = (await run("bash", [".gitmem/hooks/session-start.sh"], "{}")).stdout; + + // The project the hook tells the agent to use, straight from the repo config. + const hinted = (hookOut.match(/session_start\(project: "([^"]+)"\)/) || [])[1]; + + mcp = await createMcpClient(cleanEnv(), { cwd: repo }); + const result = await callTool(mcp.client, "session_start", { + agent_identity: "cli", force: true, ...(hinted ? { project: hinted } : {}), + }); + startText = isToolError(result) ? `ERROR ${getToolResultText(result)}` : getToolResultText(result); + const rc = await callTool(mcp.client, "recall", { plan: "run the database migration without a rollback plan", project: PROJECT }); + recallText = isToolError(rc) ? `ERROR ${getToolResultText(rc)}` : getToolResultText(rc); +}, 120_000); + +afterAll(async () => { + if (mcp) await mcp.cleanup(); + rmSync(home, { recursive: true, force: true }); + rmSync(repo, { recursive: true, force: true }); +}); + +describe("GIT-115: fresh install, clean HOME", () => { + it("init seeds the starter lessons into the root the server reads", () => { + const learnings = JSON.parse(readFileSync(join(home, ".gitmem", "learnings.json"), "utf-8")); + expect(learnings).toHaveLength(STARTER_COUNT); + expect(existsSync(join(home, ".gitmem", "threads.json"))).toBe(true); + expect(existsSync(join(home, ".gitmem", "closing-payload-template.json"))).toBe(true); + expect(initOut).toContain("~/.gitmem"); + }); + + it("the repo keeps only config.json (with the project) and the hooks", () => { + expect(readdirSync(join(repo, ".gitmem")).sort()).toEqual(["config.json", "hooks"]); + expect(JSON.parse(readFileSync(join(repo, ".gitmem", "config.json"), "utf-8"))).toEqual({ project: PROJECT }); + }); + + it("the SessionStart hook names the project from the repo config", () => { + expect(hookOut).toContain(`session_start(project: "${PROJECT}")`); + }); + + it("session_start reads what init seeded, under the repo's project", () => { + expect(startText).not.toMatch(/^ERROR/); + expect(startText).toContain(`· ${PROJECT}`); + // The welcome thread init wrote into the store. + expect(startText).toContain("Add your first project-specific scar"); + }); + + it("recall in that session returns a starter scar", () => { + expect(recallText).not.toMatch(/^ERROR/); + expect(recallText).toContain("Database Migration Without Rollback Plan"); + }); + + it("session_start does not report the repo's .gitmem as a store NOT being read", () => { + expect(startText).not.toContain("NOT being read"); + }); +}); + +describe("GIT-115: uninstall follows the same split", () => { + it("init installs its hooks beside a foreign hook under a gitmem-named path (GIT-120 matching)", async () => { + // The wizard's old substring match took this hook for gitmem's own and + // skipped installing gitmem's hooks altogether. + const other = realpathSync(mkdtempSync(join(tmpdir(), "gitmem-git115-repo2-"))); + try { + const foreign = { hooks: { PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "bash ~/code/gitmem-notes/lint.sh" }] }] } }; + mkdirSync(join(other, ".claude")); + writeFileSync(join(other, ".claude", "settings.json"), JSON.stringify(foreign, null, 2)); + const r = await execFile("node", [GITMEM_BIN, "init", "--yes", "--client", "claude"], { cwd: other, env: cleanEnv(), timeout: 60_000 }); + expect(r.stdout).toContain("Added automatic memory hooks"); + const text = readFileSync(join(other, ".claude", "settings.json"), "utf-8"); + expect(text).toContain("gitmem-notes/lint.sh"); + expect(text).toContain(".gitmem/hooks/session-start.sh"); + } finally { + rmSync(other, { recursive: true, force: true }); + } + }); + + it("uninstall removes the repo's config and hooks, keeps foreign hooks, and keeps the shared store", async () => { + const settingsPath = join(repo, ".claude", "settings.json"); + const settings = JSON.parse(readFileSync(settingsPath, "utf-8")); + settings.hooks.PreToolUse.find((g: any) => g.matcher === "Bash").hooks.push({ type: "command", command: "bash ~/code/gitmem-notes/lint.sh" }); + writeFileSync(settingsPath, JSON.stringify(settings, null, 2)); + const r = await run("node", [GITMEM_BIN, "uninstall", "--yes", "--client", "claude"]); + expect(r.code).toBe(0); + expect(existsSync(join(repo, ".gitmem"))).toBe(false); + const hooks = JSON.parse(readFileSync(join(repo, ".claude", "settings.json"), "utf-8")).hooks; + expect(JSON.stringify(hooks)).toContain("gitmem-notes/lint.sh"); + expect(JSON.stringify(hooks)).not.toContain(".gitmem/hooks/"); + expect(existsSync(join(home, ".gitmem", "learnings.json"))).toBe(true); + }); + + it("uninstall --all deletes the store", async () => { + const r = await run("node", [GITMEM_BIN, "uninstall", "--yes", "--all", "--client", "claude"]); + expect(r.code).toBe(0); + expect(existsSync(join(home, ".gitmem"))).toBe(false); + }); +}); diff --git a/tests/unit/services/gitmem-root-resolution.test.ts b/tests/unit/services/gitmem-root-resolution.test.ts index cdd086e..1bf6259 100644 --- a/tests/unit/services/gitmem-root-resolution.test.ts +++ b/tests/unit/services/gitmem-root-resolution.test.ts @@ -140,10 +140,38 @@ describe("GIT-91: isLiveGitmemRoot only counts real evidence", () => { vi.restoreAllMocks(); }); - it("counts a deliberate project-scoped install (config.json)", () => { + // GIT-115: a repo's .gitmem/ holds config.json (the project name) and the + // hook scripts on purpose; the store lives in the root the server reads. + it("does not count a repo .gitmem/ holding only config.json and hooks", () => { const gitmem = path.join(tmp, ".gitmem"); - fs.mkdirSync(gitmem, { recursive: true }); + fs.mkdirSync(path.join(gitmem, "hooks"), { recursive: true }); fs.writeFileSync(path.join(gitmem, "config.json"), JSON.stringify({ project: "x" })); + fs.writeFileSync(path.join(gitmem, "hooks", "session-start.sh"), "#!/bin/bash\n"); + + expect(isLiveGitmemRoot(gitmem)).toBe(false); + }); + + it("does not count what an older init seeded: starter lessons and the welcome thread", () => { + const gitmem = path.join(tmp, ".gitmem"); + fs.mkdirSync(gitmem, { recursive: true }); + const starters = JSON.parse(fs.readFileSync(path.join(__dirname, "../../../schema/starter-scars.json"), "utf-8")); + fs.writeFileSync(path.join(gitmem, "learnings.json"), JSON.stringify(starters)); + fs.writeFileSync(path.join(gitmem, "threads.json"), JSON.stringify([{ id: "t-welcome01", text: "welcome" }])); + fs.writeFileSync(path.join(gitmem, "decisions.json"), "[]"); + fs.writeFileSync(path.join(gitmem, "config.json"), "{}"); + + expect(isLiveGitmemRoot(gitmem)).toBe(false); + }); + + it.each([ + ["a learning of the user's own", "learnings.json", [{ id: "user-learning-1", title: "mine" }]], + ["a thread of the user's own", "threads.json", [{ id: "t-abc12345", text: "mine" }]], + ["a decision", "decisions.json", [{ id: "d-1", title: "chose X" }]], + ["records in the {key: array} shape", "learnings.json", { learnings: [{ id: "user-learning-2" }] }], + ])("counts %s", (_label, file, content) => { + const gitmem = path.join(tmp, ".gitmem"); + fs.mkdirSync(gitmem, { recursive: true }); + fs.writeFileSync(path.join(gitmem, file), JSON.stringify(content)); expect(isLiveGitmemRoot(gitmem)).toBe(true); }); diff --git a/tests/unit/services/gitmem-root-symlink.test.ts b/tests/unit/services/gitmem-root-symlink.test.ts index a7ec2cc..19eca86 100644 --- a/tests/unit/services/gitmem-root-symlink.test.ts +++ b/tests/unit/services/gitmem-root-symlink.test.ts @@ -15,11 +15,15 @@ import { findStrandedProjectRoots, sameDirectory, canonicalPath, clearGitmemDirC 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. */ +/** + * A live project-scoped store: it holds a learning of the user's own. + * (GIT-115: config.json alone no longer marks one; a repo keeps its + * config.json and hooks in .gitmem/ while the store lives elsewhere.) + */ 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" })); + fs.writeFileSync(path.join(g, "learnings.json"), JSON.stringify([{ id: "user-learning", title: "mine" }])); return g; } From 0624096ee8ab1ddd7f07502cc4d170154d88bfa0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 21:23:54 -0700 Subject: [PATCH 16/17] docs: CHANGELOG [Unreleased] adds GIT-115 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d59a11..03a2326 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,10 +16,24 @@ that schema and on the current one. got an error and returned no scars. It now calls `gitmem_scar_search`, which every project has, and falls back to the older function only where that exists. (GIT-114) +**A fresh install now finds its starter lessons.** `gitmem init` put your memory in the repo's +`.gitmem/`, but gitmem reads `~/.gitmem`, so a new install started with none of its starter lessons +and was told its memory was "NOT being read". `init` now puts your memory where gitmem reads it +(`GITMEM_DIR`, else `GITMEM_HOME/.gitmem`, else `~/.gitmem`). The repo's `.gitmem/` keeps only +`config.json`, with the project name, and the hook scripts. A repo set up that way no longer +triggers the warning. `gitmem uninstall` removes the repo's files and keeps your memory, which every +project shares, unless you pass `--all`. The session-start hook now finds the project name without +jq. (GIT-115) + +If an older `init` put lessons of your own in a repo's `.gitmem/`, session_start still points them +out. `npx gitmem-mcp migrate-root` copies them into `~/.gitmem`. + **`install-hooks` and `uninstall-hooks` keep your own hooks.** They used to replace or delete every hook in `.claude/settings.json` or `.cursor/hooks.json`. Now they add or remove only gitmem's hooks, back the file up first, and print what changed. A file that is not valid JSON is left alone instead -of being replaced. (GIT-120) +of being replaced. `gitmem init` and `gitmem uninstall` use the same matching, so a hook of yours +that shares a matcher with gitmem's, or lives in a folder named gitmem, is no longer removed or +mistaken for gitmem's. (GIT-120, GIT-115) ### Fixed From f180c2085f3257c03d0920b8ecf7dd3212c10eb4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 21:44:18 -0700 Subject: [PATCH 17/17] chore: bump version to v1.11.1 CHANGELOG [Unreleased] -> [1.11.1] - 2026-09-22, fresh [Unreleased] above. Co-Authored-By: Claude Opus 5.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 03a2326..3f102de 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.1] - 2026-09-22 + **Just upgrade the package. Nothing to change on your database.** Every fix below works on a project still on the `setup.sql` you first ran, back to 1.8.0, and was checked against a project on exactly that schema and on the current one. diff --git a/package-lock.json b/package-lock.json index 31aee33..acbbc94 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "gitmem-mcp", - "version": "1.11.0", + "version": "1.11.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "gitmem-mcp", - "version": "1.11.0", + "version": "1.11.1", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.0.0", diff --git a/package.json b/package.json index 70b18cf..f111369 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gitmem-mcp", - "version": "1.11.0", + "version": "1.11.1", "mcpName": "io.github.gitmem-dev/gitmem", "description": "Persistent learning memory for AI coding agents. Memory that compounds.", "type": "module",