diff --git a/CHANGELOG.md b/CHANGELOG.md index 48d7c14..6547d41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,69 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.8.0] - 2026-08-09 + +**No one loses any information.** Nothing is deleted, nothing is moved, nothing is overwritten. + +gitmem now reads one memory store — `~/.gitmem` — no matter which directory a process starts in. +That has been the default since v1.0.10 in February, so for almost everyone this changes nothing +visible. If you are one of the rare installs that still keeps memory in a project-local `.gitmem/`, +your first `session_start` after upgrading will show you exactly where it is, how many learnings, +threads and sessions are in it, and the one command that copies them across. `migrate-root` copies +and leaves the original untouched. Pro users' Supabase memory is unaffected either way. + +The rest of this release is about a quieter problem: gitmem was reporting success for failures. A +session that survived an MCP restart was told it had none. A scar search that never reached the +store was answered with "proceed freely". Both are fixed, and both now say what actually happened. + +### Fixed + +- **A session no longer loses its identity when the MCP server restarts.** Identity was bound to + `process.pid` and looked up through the active-sessions registry, so any restart — an app update, + a rebuild, a relaunch — orphaned the entry and every session-required tool reported "No active + session" for the rest of the session, while writes continued to land correctly. Identity now + resolves from the durable per-session store on disk; PID is only a disambiguator, and the registry + is repaired from disk rather than gating access to it. A live session belonging to another server + is still never claimed, so concurrent sessions remain isolated. `session_close` also no longer + requires you to pass `session_id` — it resolves the session itself, which is the case a restart + exists to break. (GIT-89) +- **Scar retrieval failed on every call whenever the local index was cold.** The Supabase fallback + built its RPC name from the table prefix and a verb, producing a function that exists under no + prefix, on any deployment — so a `recall` issued before the in-memory index finished loading + returned nothing at all. That window includes the first `recall` of a session. The RPCs are now + called by their deployed names. (GIT-93) +- **`confirm_scars` reported a failed retrieval as a clean check.** With nothing surfaced it replied + "No recall-surfaced scars to confirm. Proceed freely" — the same answer whether the search had run + and matched nothing or had never reached the store. It now distinguishes the two and names the + underlying error, and the distinction survives a restart. This is why the retrieval defect above + could persist unnoticed. (GIT-93) +- **The pre-publish clean-room images could not build.** Every clean-room Dockerfile installed + `npm@latest` onto a Node 20 base, which stopped working once npm began requiring Node 22.22+. The + gate that tests the packaged tarball the way a user installs it had been failing silently, because + a gate only run by hand has no failure signal between uses. (GIT-91) + +### Added + +- **`npx gitmem-mcp migrate-root`** — copies a project-local memory store into `~/.gitmem`. It copies + rather than moves, never overwrites a file that already exists at the destination, and reports + every file it skipped and why. `--dry-run` shows the exact plan first. (GIT-91) +- **A first-run signpost for project-local stores.** If one is found, `session_start` names the path, + the record counts it holds, and the one command that copies it in. Detection only — the store is + never read from behind your back, and never silently unread either. (GIT-91) +- **`GITMEM_HOME`** — relocates the developer-scoped root without short-circuiting resolution the way + `GITMEM_DIR` does. Useful for containers and CI. (GIT-91) + +### Changed + +- **The `.gitmem` root no longer depends on the working directory.** Resolution used to walk up from + `process.cwd()`, which meant the MCP server and the SessionStart hook — which do not share a + directory — could bind one session to two different stores, with writes landing where identity + resolution never looked. Project-local stores are still fully supported and are now selected + explicitly with `GITMEM_DIR`. (GIT-91) +- **CI gates publishing on a real restart.** The release pipeline now runs an end-to-end test that + kills the MCP server process and drives a recovered session over the MCP protocol, so the identity + fix above cannot regress into a release. (GIT-89) + ## [1.7.0] - 2026-08-07 **No destructive changes, no data loss, no migration.** But if you start seeing errors after diff --git a/bin/gitmem.js b/bin/gitmem.js index d114d49..e2294bc 100755 --- a/bin/gitmem.js +++ b/bin/gitmem.js @@ -52,6 +52,8 @@ Other commands: npx gitmem-mcp setup Output SQL for Supabase schema setup (pro/dev tier) npx gitmem-mcp configure Generate .mcp.json config for Claude Code / Cursor npx gitmem-mcp check Run diagnostic health check + npx gitmem-mcp migrate-root Copy a project-scoped .gitmem into ~/.gitmem + npx gitmem-mcp migrate-root --dry-run Show what would be copied npx gitmem-mcp check --full Full diagnostic with benchmarks npx gitmem-mcp install-hooks Install hooks (standalone) npx gitmem-mcp uninstall-hooks Remove hooks (standalone) @@ -902,6 +904,9 @@ switch (command) { case "telemetry": import("../dist/commands/telemetry.js").then((m) => m.main(process.argv.slice(3))); break; + case "migrate-root": + import("../dist/commands/migrate-root.js").then((m) => m.main(process.argv.slice(3))); + break; case "install-hooks": cmdInstallHooks(); break; diff --git a/package.json b/package.json index 4710801..53de28e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gitmem-mcp", - "version": "1.7.0", + "version": "1.8.0", "mcpName": "io.github.gitmem-dev/gitmem", "description": "Persistent learning memory for AI coding agents. Memory that compounds.", "type": "module", diff --git a/src/commands/migrate-root.ts b/src/commands/migrate-root.ts new file mode 100644 index 0000000..faa4241 --- /dev/null +++ b/src/commands/migrate-root.ts @@ -0,0 +1,169 @@ +/** + * GIT-91: copy a project-scoped .gitmem store into the developer-scoped root. + * + * Before v1.0.10 gitmem stored data in /.gitmem. That release moved the + * default to ~/.gitmem and kept a cwd walk-up so existing stores were still + * found. GIT-91 removed the walk-up: deriving the root from process.cwd() meant + * the MCP server and the SessionStart hook — which do not share a cwd — resolved + * different stores for one session. + * + * The consequence for anyone still on a pre-1.0.10 layout is that their store is + * no longer read. On the free tier that store IS the memory (learnings.json, + * threads.json), so "my scars vanished after an upgrade" is the experience this + * command exists to prevent. + * + * Design constraints, in order of importance: + * + * COPY, NEVER MOVE. The source is left byte-for-byte intact. If this command + * is wrong about anything, the user still has their data where it was. Moving + * would make a bad merge unrecoverable. + * + * NEVER OVERWRITE. A file that already exists at the destination wins. The + * destination is the live store; the source is, by definition, the one that + * has not been read recently. Clobbering current memory with stale memory is + * worse than skipping. + * + * REPORT EVERY SKIP. A silent partial migration would leave the user believing + * they had merged when they had not — the failure class GIT-93 was about. + */ + +import * as fs from "fs"; +import * as path from "path"; +import { findStrandedProjectRoots, getHomeGitmemDir } from "../services/gitmem-dir.js"; + +interface MigrationPlan { + source: string; + destination: string; + copied: string[]; + skipped: Array<{ file: string; reason: string }>; +} + +/** + * Project-scoped roots holding live state. Delegates to the shared detector so + * this command and the session_start notice can never disagree about what + * counts as a store worth migrating. + */ +export function findProjectRoots(): string[] { + return findStrandedProjectRoots(); +} + +/** + * Recursively copy `from` into `to`, never overwriting an existing file. + * + * Returns what was copied and what was left alone, so the caller can report both + * rather than claiming a clean merge. + */ +function copyTree( + from: string, + to: string, + plan: MigrationPlan, + relative = "" +): void { + fs.mkdirSync(to, { recursive: true }); + for (const entry of fs.readdirSync(from, { withFileTypes: true })) { + const rel = relative ? path.join(relative, entry.name) : entry.name; + const src = path.join(from, entry.name); + const dst = path.join(to, entry.name); + + // Caches and license state are per-install, not memory. Copying them would + // move a license binding between roots, which is not this command's job. + if (relative === "" && (entry.name === "cache" || entry.name === "license-cache.json")) { + plan.skipped.push({ file: rel, reason: "per-install state, not memory" }); + continue; + } + + if (entry.isDirectory()) { + copyTree(src, dst, plan, rel); + continue; + } + if (fs.existsSync(dst)) { + plan.skipped.push({ file: rel, reason: "already exists in destination" }); + continue; + } + fs.copyFileSync(src, dst); + plan.copied.push(rel); + } +} + +export function migrateRoot(source: string, destination: string, dryRun: boolean): MigrationPlan { + const plan: MigrationPlan = { source, destination, copied: [], skipped: [] }; + + if (dryRun) { + // Walk the same tree without writing, so --dry-run reports the real plan + // rather than a guess at one. + const probe = (from: string, to: string, rel = ""): void => { + for (const entry of fs.readdirSync(from, { withFileTypes: true })) { + const r = rel ? path.join(rel, entry.name) : entry.name; + if (rel === "" && (entry.name === "cache" || entry.name === "license-cache.json")) { + plan.skipped.push({ file: r, reason: "per-install state, not memory" }); + continue; + } + if (entry.isDirectory()) { probe(path.join(from, entry.name), path.join(to, entry.name), r); continue; } + if (fs.existsSync(path.join(to, entry.name))) { + plan.skipped.push({ file: r, reason: "already exists in destination" }); + continue; + } + plan.copied.push(r); + } + }; + probe(source, destination); + return plan; + } + + copyTree(source, destination, plan); + return plan; +} + +export function main(args: string[]): void { + const dryRun = args.includes("--dry-run"); + // Must come from the resolver the server uses, not os.homedir() directly. + // GITMEM_HOME relocates the developer-scoped root, and computing the + // destination independently sent this command to a different store than the + // one gitmem reads — under a GITMEM_HOME override it copied into the real + // ~/.gitmem instead. A migration tool that writes somewhere the product does + // not read is worse than no tool. + const home = getHomeGitmemDir(); + + const explicitIdx = args.indexOf("--from"); + const explicit = explicitIdx !== -1 ? args[explicitIdx + 1] : null; + + const sources = explicit ? [path.resolve(explicit)] : findProjectRoots(); + + if (sources.length === 0) { + console.log("No project-scoped .gitmem store found above the current directory."); + console.log(`Nothing to migrate — ${home} is already the store gitmem reads.`); + return; + } + + if (sources.length > 1) { + console.log(`Found ${sources.length} project-scoped stores:\n`); + sources.forEach((s) => console.log(` ${s}`)); + console.log(`\nMigrate them one at a time so each result is reviewable:`); + console.log(` npx gitmem-mcp migrate-root --from ${sources[0]}`); + return; + } + + const source = sources[0]; + if (source === home) { + console.log(`Source and destination are the same (${home}). Nothing to do.`); + return; + } + + console.log(`${dryRun ? "Would copy" : "Copying"} gitmem store`); + console.log(` from: ${source}`); + console.log(` to: ${home}\n`); + + const plan = migrateRoot(source, home, dryRun); + + console.log(`${plan.copied.length} file(s) ${dryRun ? "would be " : ""}copied.`); + if (plan.skipped.length > 0) { + console.log(`${plan.skipped.length} skipped:`); + for (const s of plan.skipped) console.log(` ${s.file} — ${s.reason}`); + } + + console.log( + `\nThe source was NOT modified. ${source} is still intact — verify the result ` + + `before deleting anything.` + ); + if (dryRun) console.log("\nRe-run without --dry-run to apply."); +} diff --git a/src/services/gitmem-dir.ts b/src/services/gitmem-dir.ts index f5340e3..4451b7e 100644 --- a/src/services/gitmem-dir.ts +++ b/src/services/gitmem-dir.ts @@ -47,8 +47,12 @@ export function setGitmemDir(dir: string): void { * Resolution order: * 1. GITMEM_DIR env var (explicit override) * 2. Cached path from session_start (most reliable) - * 3. Walk up from CWD looking for existing .gitmem/ sentinels (backward compat) - * 4. Fall back to ~/.gitmem (developer-scoped, survives across projects/containers) + * 3. ~/.gitmem — authoritative, and independent of cwd. + * + * GIT-91 removed a cwd walk-up that sat between 2 and 3. Because it derived the + * answer from process.cwd(), the MCP server and the SessionStart hook — which do + * not share a cwd — resolved different roots for the same session. Project-scoped + * roots are still supported, but must be named explicitly via GITMEM_DIR. */ export function getGitmemDir(): string { // 1. GITMEM_DIR env var (explicit override, highest priority) @@ -66,30 +70,214 @@ export function getGitmemDir(): string { return cachedGitmemDir; } - // 3. Walk up from CWD looking for existing .gitmem directory - // Backward compat: finds project-scoped .gitmem/ from older installations. - // Sentinel files checked in priority order: - // - active-sessions.json (multi-session registry, GIT-19) - // - config.json (project-level gitmem config) - const sentinels = ["active-sessions.json", "config.json"]; - let dir = process.cwd(); - const root = path.parse(dir).root; - while (dir !== root) { - const candidate = path.join(dir, ".gitmem"); - for (const sentinel of sentinels) { - if (fs.existsSync(path.join(candidate, sentinel))) { - cachedGitmemDir = candidate; - console.error(`[gitmem-dir] Found .gitmem via walk-up (${sentinel}): ${candidate}`); - return candidate; + // 3. ~/.gitmem is authoritative. Not a fallback — the answer. + // + // GIT-91: resolution used to walk up from process.cwd() and adopt any + // directory containing active-sessions.json or config.json. That makes the + // answer a function of cwd, and the processes sharing a session do not + // share a cwd: the MCP server runs from wherever the client launched it, + // while the SessionStart hook runs in the repo. So one logical session bound + // to two different stores, and writes landed in a root that identity + // resolution never read. + // + // No cwd-derived rule can fix that. Tightening the sentinel to require live + // state was tried first and does not hold: stale test-written sessions carry + // a structurally valid session.json (GIT-92), so the repo still qualified — + // and even a perfect liveness test cannot make two processes with different + // cwds agree. The only property that guarantees agreement is not depending + // on cwd at all. + // + // Project-scoped roots remain reachable, but only by saying so explicitly + // via GITMEM_DIR. Nothing is moved or deleted; a project root that still + // holds live state is reported loudly, with the exact way to select it. + const home = getHomeGitmemDir(); + warnAboutStrandedProjectRoots(home); + return home; +} + +/** + * The developer-scoped root: `/.gitmem`. + * + * GITMEM_HOME overrides the base directory. It is distinct from GITMEM_DIR: + * GITMEM_DIR names the `.gitmem` directory itself and short-circuits resolution + * entirely, while GITMEM_HOME only relocates the home the fallback is computed + * from, leaving the precedence chain intact. + * + * It exists because os.homedir() reads the OS-level environment, which cannot be + * redirected from inside a worker thread — process.env there is a JS-level copy + * that never reaches getenv(). The test suite runs on `pool: "threads"` and + * writes real session state, so without this there is no way to keep it off the + * developer's store (GIT-92). The same lever is useful for containers and CI, + * where HOME is often not where state should live. + */ +export function getHomeGitmemDir(): string { + const base = process.env.GITMEM_HOME || os.homedir(); + return path.join(base, ".gitmem"); +} + +/** Report at most one stranded root per process — this runs on a hot path. */ +let strandedWarningIssued = false; + +/** + * GIT-91: project-scoped roots above the cwd that hold live state and are no + * longer read. + * + * Exported because stderr is invisible in most MCP clients: session_start puts + * this in its display, where the user will actually see it. Returns [] on any + * error — a diagnostic must never break resolution. + */ +/** What a stranded root actually holds, for the session_start notice (R18). */ +export interface GitmemRootContents { + root: string; + learnings: number; + threads: number; + sessions: number; +} + +/** Count entries in a store file that may be a bare array or {key: array}. */ +function countCollection(file: string, key: string): number { + try { + if (!fs.existsSync(file)) return 0; + const parsed = JSON.parse(fs.readFileSync(file, "utf-8")); + if (Array.isArray(parsed)) return parsed.length; + if (parsed && Array.isArray(parsed[key])) return parsed[key].length; + return 0; + } catch { + return 0; + } +} + +/** + * GIT-91 / R18: what a stranded root contains. + * + * The notice has to state counts, not just a path. "Your memory is at another + * path" is abstract enough to scroll past; "142 learnings, 6 threads are sitting + * at this path" is not. Detection-without-use only works if the detection says + * something a user can weigh. + * + * Counts are best-effort by design — an unreadable or unexpected file yields 0 + * rather than throwing. A notice that fails to render because one file is + * malformed would reintroduce exactly the silence this exists to prevent. + */ +export function describeGitmemRoot(root: string): GitmemRootContents { + let sessions = 0; + try { + const sessionsDir = path.join(root, "sessions"); + if (fs.existsSync(sessionsDir)) { + for (const entry of fs.readdirSync(sessionsDir)) { + if (fs.existsSync(path.join(sessionsDir, entry, "session.json"))) sessions++; } } - dir = path.dirname(dir); + } catch { + // best-effort + } + return { + root, + learnings: countCollection(path.join(root, "learnings.json"), "learnings"), + threads: countCollection(path.join(root, "threads.json"), "threads"), + sessions, + }; +} + +export function findStrandedProjectRoots(): string[] { + try { + const home = getHomeGitmemDir(); + const stranded: string[] = []; + let dir = process.cwd(); + const fsRoot = path.parse(dir).root; + while (dir !== fsRoot) { + const candidate = path.join(dir, ".gitmem"); + if (candidate !== home && isLiveGitmemRoot(candidate)) stranded.push(candidate); + dir = path.dirname(dir); + } + return stranded; + } catch { + return []; } +} + +/** + * GIT-91: warn when a project-scoped root still holds live state. + * + * Resolution no longer walks up, so such a root is no longer read. It is not + * touched either — moving a user's memory store is a far worse failure than not + * reading it. Instead: name the path and the one-line fix, once per process. + * + * Silence here would be the same defect as GIT-93's "Proceed freely" — a system + * reporting a clean state while a store it used to read sits unread. + */ +function warnAboutStrandedProjectRoots(home: string): void { + if (strandedWarningIssued) return; + strandedWarningIssued = true; + + try { + const stranded = findStrandedProjectRoots(); + if (stranded.length === 0) return; + + console.error( + `[gitmem-dir] Project-scoped .gitmem found with live state, NOT being used: ` + + `${stranded.join(", ")}. gitmem now resolves ${home} regardless of cwd, so every ` + + `process in a session agrees on one store (GIT-91). To use a project-scoped root, ` + + `set GITMEM_DIR= explicitly. Nothing has been moved or deleted.` + ); + } catch { + // Diagnostics must never break resolution. + } +} - // 4. Fall back to ~/.gitmem (developer-scoped — survives across projects and containers) - const fallback = path.join(os.homedir(), ".gitmem"); - console.error(`[gitmem-dir] Falling back to home dir: ${fallback}`); - return fallback; +/** + * GIT-91: does this directory hold a gitmem store that is actually in use? + * + * Presence of a file is not evidence. The registry in particular is present and + * empty on any tree a gitmem process has merely passed through, and empty means + * the opposite of "sessions live here". Test runs leave the same residue + * (GIT-92), so an unrelated repo can acquire a convincing-looking .gitmem/ + * 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 + * + * Exported for tests and diagnostics; the resolution path is the only caller + * that matters. + */ +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; + + const registryPath = path.join(candidate, "active-sessions.json"); + if (fs.existsSync(registryPath)) { + try { + const registry = JSON.parse(fs.readFileSync(registryPath, "utf-8")); + if (Array.isArray(registry.sessions) && registry.sessions.length > 0) return true; + } catch { + // Unreadable registry is not evidence of anything. Fall through to the + // session directories, which are the durable store (GIT-89). + } + } + + const sessionsDir = path.join(candidate, "sessions"); + if (!fs.existsSync(sessionsDir)) return false; + for (const entry of fs.readdirSync(sessionsDir)) { + const sessionFile = path.join(sessionsDir, entry, "session.json"); + if (!fs.existsSync(sessionFile)) continue; // fixture dir, not a session + try { + const data = JSON.parse(fs.readFileSync(sessionFile, "utf-8")); + if (data && typeof data.session_id === "string" && data.session_id) return true; + } catch { + // Malformed session file — not evidence. + } + } + return false; + } catch { + // Unreadable candidate (permissions, race). Treat as not-live rather than + // throwing: resolution must always yield a usable root. + return false; + } } /** @@ -175,4 +363,7 @@ export function getInstallId(): string | null { */ export function clearGitmemDirCache(): void { cachedGitmemDir = null; + // GIT-91: the stranded-root notice is once-per-process, which would otherwise + // leak between tests that share a module instance. + strandedWarningIssued = false; } diff --git a/src/tools/session-start.ts b/src/tools/session-start.ts index 5b53148..c6d034e 100644 --- a/src/tools/session-start.ts +++ b/src/tools/session-start.ts @@ -36,7 +36,7 @@ import { loadActiveThreadsFromSupabase, archiveDormantThreads } from "../service import { resolveThreadScope, computePanelOmission, formatOmissionLine } from "../services/thread-scope.js"; import type { ThreadScopeCounts } from "../services/thread-scope.js"; import type { ThreadDisplayInfo } from "../services/thread-supabase.js"; -import { setGitmemDir, getGitmemDir, getSessionPath, getConfigProject } from "../services/gitmem-dir.js"; +import { setGitmemDir, getGitmemDir, getSessionPath, getConfigProject, findStrandedProjectRoots, describeGitmemRoot } from "../services/gitmem-dir.js"; import { registerSession, findSessionByHostPid, findResumableSessionOnDisk, pruneStale, migrateFromLegacy } from "../services/active-sessions.js"; import * as os from "os"; import { formatDate } from "../services/timezone.js"; @@ -908,6 +908,40 @@ function formatStartDisplay(result: SessionStartResult, displayInfoMap?: Map/.gitmem, and a cwd walk-up + // kept finding it afterwards. GIT-91 removed that walk-up, so such a store is + // now unread — on the free tier that store IS the memory, learnings and all. + // + // This is reported here, not only to stderr, because stderr is invisible in + // most MCP clients. A user whose memory silently emptied after an upgrade + // would have no way to connect it to anything. Placed above threads so it is + // not pushed off the end of a long block. + // R18: state the COUNTS, not just the path. "Your memory is at another path" + // is abstract enough to scroll past; "142 learnings are sitting here" is not. + // Detection-without-use only works if the detection says something the user + // can weigh — the store is never silently unread, it is loudly findable. + const stranded = findStrandedProjectRoots(); + if (stranded.length > 0) { + visual.push(""); + visual.push(boldText("Memory store found but NOT being read")); + for (const root of stranded) { + const c = describeGitmemRoot(root); + const held = [ + c.learnings > 0 ? `${c.learnings} learnings` : null, + c.threads > 0 ? `${c.threads} threads` : null, + c.sessions > 0 ? `${c.sessions} sessions` : null, + ].filter(Boolean).join(", "); + visual.push(dimText(` ${root}`)); + visual.push(dimText(` holds: ${held || "no countable records"}`)); + } + visual.push(dimText(` gitmem reads ${getGitmemDir()} regardless of directory (1.8.0).`)); + visual.push(dimText(` Nothing was moved or deleted. Copy it in:`)); + visual.push(dimText(` npx gitmem-mcp migrate-root --dry-run`)); + visual.push(dimText(` Or keep using that store: set GITMEM_DIR=`)); + } + // Threads section — top 5 by vitality, truncated to 60 chars const hasThreads = result.open_threads && result.open_threads.length > 0; const hasDecisions = result.recent_decisions && result.recent_decisions.length > 0; diff --git a/tests/setup/isolate-gitmem-root.ts b/tests/setup/isolate-gitmem-root.ts new file mode 100644 index 0000000..5f4a7f8 --- /dev/null +++ b/tests/setup/isolate-gitmem-root.ts @@ -0,0 +1,34 @@ +/** + * GIT-92 / GIT-91: assert the suite cannot reach the developer's real store. + * + * The isolation itself is in vitest.config.ts, which hands every worker a + * throwaway GITMEM_HOME. HOME cannot be used: vitest runs on pool "threads", + * where process.env is a JS-level copy that never reaches native getenv(), so + * os.homedir() is unaffected by anything set from inside a worker. + * + * Why this matters: tests call setCurrentSession() with literal ids and those + * writes follow getGitmemDir(). That used to land in /.gitmem, which was + * already wrong — the suite left directories named original-session, + * test-session-2/3 and test-session-clean in the developer's tree. GIT-91 + * removed the cwd walk-up, so the same writes now resolve ~/.gitmem: real + * sessions, real threads, and on the free tier every learning ever captured. + * The pollution did not appear with that change; it moved somewhere far worse. + * + * A run that can still see the real store is a broken harness, not a warning to + * scroll past — so this throws rather than logs. + */ + +import * as os from "os"; +import * as path from "path"; +import { getHomeGitmemDir } from "../../src/services/gitmem-dir.js"; + +const resolved = path.resolve(getHomeGitmemDir()); +const tmp = path.resolve(os.tmpdir()); + +if (!resolved.startsWith(tmp)) { + throw new Error( + `[test-setup] the developer-scoped root resolves to ${resolved}, outside ${tmp}. ` + + `The suite writes session state through getGitmemDir() and would touch the real ` + + `store. Refusing to run — check the env block in vitest.config.ts.` + ); +} diff --git a/tests/unit/commands/migrate-root.test.ts b/tests/unit/commands/migrate-root.test.ts new file mode 100644 index 0000000..e26adf7 --- /dev/null +++ b/tests/unit/commands/migrate-root.test.ts @@ -0,0 +1,113 @@ +/** + * GIT-91: migrate-root copies a stranded project store into the root gitmem reads. + * + * Removing the cwd walk-up leaves pre-v1.0.10 stores unread. On the free tier + * that store IS the memory, so this command is the difference between "gitmem + * changed where it looks" and "my scars vanished after an upgrade". + * + * The properties below are the ones that make it safe to run on a store you + * cannot afford to lose. Each is a way this could destroy data rather than + * merely fail: + * + * copies, never moves — a wrong call leaves the original intact + * never overwrites — stale memory cannot clobber current memory + * reports every skip — a partial merge is never reported as complete + * writes where gitmem reads — this one was a real bug: main() computed the + * destination with os.homedir() instead of the + * shared resolver, so under a GITMEM_HOME + * override it wrote into the real ~/.gitmem. + */ + +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import * as fs from "fs"; +import * as path from "path"; +import * as os from "os"; +import { migrateRoot } from "../../../src/commands/migrate-root.js"; + +let tmp: string; +let source: string; +let destination: string; + +const read = (p: string): string => fs.readFileSync(p, "utf-8"); + +beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-migrate-")); + source = path.join(tmp, "project", ".gitmem"); + destination = path.join(tmp, "home", ".gitmem"); + fs.mkdirSync(path.join(source, "sessions", "s1"), { recursive: true }); + fs.mkdirSync(destination, { recursive: true }); + fs.writeFileSync(path.join(source, "learnings.json"), '{"learnings":["old"]}'); + fs.writeFileSync(path.join(source, "threads.json"), '{"threads":["old-thread"]}'); + fs.writeFileSync(path.join(source, "sessions", "s1", "session.json"), '{"session_id":"s1"}'); +}); + +afterEach(() => fs.rmSync(tmp, { recursive: true, force: true })); + +describe("GIT-91: migrate-root", () => { + it("copies memory files into the destination", () => { + const plan = migrateRoot(source, destination, false); + + expect(plan.copied).toContain("learnings.json"); + expect(plan.copied).toContain("threads.json"); + expect(read(path.join(destination, "learnings.json"))).toBe('{"learnings":["old"]}'); + }); + + it("copies nested session directories", () => { + migrateRoot(source, destination, false); + + expect(fs.existsSync(path.join(destination, "sessions", "s1", "session.json"))).toBe(true); + }); + + it("leaves the source byte-for-byte intact", () => { + const before = read(path.join(source, "learnings.json")); + + migrateRoot(source, destination, false); + + expect(fs.existsSync(path.join(source, "learnings.json"))).toBe(true); + expect(read(path.join(source, "learnings.json"))).toBe(before); + }); + + it("never overwrites a file that already exists in the destination", () => { + // The destination is the live store; the source is by definition the one + // that has not been read recently. Stale must not win. + fs.writeFileSync(path.join(destination, "learnings.json"), '{"learnings":["CURRENT"]}'); + + const plan = migrateRoot(source, destination, false); + + expect(read(path.join(destination, "learnings.json"))).toBe('{"learnings":["CURRENT"]}'); + expect(plan.copied).not.toContain("learnings.json"); + expect(plan.skipped.map((s) => s.file)).toContain("learnings.json"); + }); + + it("reports why each file was skipped", () => { + fs.writeFileSync(path.join(destination, "learnings.json"), "{}"); + + const plan = migrateRoot(source, destination, false); + + const skip = plan.skipped.find((s) => s.file === "learnings.json"); + // A silent partial merge would read as a complete one. + expect(skip?.reason).toMatch(/already exists/i); + }); + + it("does not carry per-install state across roots", () => { + fs.mkdirSync(path.join(source, "cache"), { recursive: true }); + fs.writeFileSync(path.join(source, "cache", "hook-scars.json"), "[]"); + fs.writeFileSync(path.join(source, "license-cache.json"), "{}"); + + const plan = migrateRoot(source, destination, false); + + // Moving a license binding between roots is not this command's job. + expect(fs.existsSync(path.join(destination, "license-cache.json"))).toBe(false); + expect(plan.skipped.map((s) => s.file)).toContain("license-cache.json"); + expect(plan.skipped.map((s) => s.file)).toContain("cache"); + }); + + it("writes nothing in dry-run, and reports the same plan it would apply", () => { + const dry = migrateRoot(source, destination, true); + + expect(fs.existsSync(path.join(destination, "learnings.json"))).toBe(false); + + const applied = migrateRoot(source, destination, false); + expect(dry.copied.sort()).toEqual(applied.copied.sort()); + }); +}); diff --git a/tests/unit/no-console-log.test.ts b/tests/unit/no-console-log.test.ts index 6fc9bd9..e22cfee 100644 --- a/tests/unit/no-console-log.test.ts +++ b/tests/unit/no-console-log.test.ts @@ -34,6 +34,9 @@ const CLI_COMMAND_ALLOWLIST = new Set([ "src/commands/activate.ts", "src/commands/deactivate.ts", "src/commands/migrate-local.ts", + // GIT-91: invoked only via bin/gitmem.js `migrate-root`, never imported by the + // server, so its output cannot reach the MCP stdio stream. + "src/commands/migrate-root.ts", ]); describe("no console.log in src/", () => { diff --git a/tests/unit/services/gitmem-dir-multisession.test.ts b/tests/unit/services/gitmem-dir-multisession.test.ts index f84e7c3..bcafc34 100644 --- a/tests/unit/services/gitmem-dir-multisession.test.ts +++ b/tests/unit/services/gitmem-dir-multisession.test.ts @@ -14,6 +14,7 @@ import { getSessionPath, setGitmemDir, clearGitmemDirCache, + getHomeGitmemDir, } from "../../../src/services/gitmem-dir.js"; let tmpDir: string; @@ -76,7 +77,14 @@ describe("getSessionPath", () => { }); describe("getGitmemDir walk-up with multiple sentinels", () => { - it("finds .gitmem with active-sessions.json sentinel", () => { + // GIT-91: the walk-up these tests described has been removed. Deriving the + // root from process.cwd() meant the MCP server and the SessionStart hook — + // which do not share a cwd — resolved different stores for the same session. + // The three cases below asserted exactly that behaviour, so they now assert + // its replacement: cwd is ignored, and a project-scoped root is selected only + // by naming it in GITMEM_DIR. + + it("ignores an active-sessions.json sentinel in a parent directory", () => { const projectDir = path.join(tmpDir, "project"); const subDir = path.join(projectDir, "sub", "deep"); const gitmemDir = path.join(projectDir, ".gitmem"); @@ -87,11 +95,10 @@ describe("getGitmemDir walk-up with multiple sentinels", () => { vi.spyOn(process, "cwd").mockReturnValue(subDir); - const result = getGitmemDir(); - expect(result).toBe(gitmemDir); + expect(getGitmemDir()).toBe(getHomeGitmemDir()); }); - it("finds .gitmem with config.json sentinel", () => { + it("ignores a config.json sentinel in a parent directory", () => { const projectDir = path.join(tmpDir, "project"); const subDir = path.join(projectDir, "sub"); const gitmemDir = path.join(projectDir, ".gitmem"); @@ -102,8 +109,7 @@ describe("getGitmemDir walk-up with multiple sentinels", () => { vi.spyOn(process, "cwd").mockReturnValue(subDir); - const result = getGitmemDir(); - expect(result).toBe(gitmemDir); + expect(getGitmemDir()).toBe(getHomeGitmemDir()); }); it("does NOT use legacy active-session.json as sentinel (removed in multi-session)", () => { @@ -117,12 +123,10 @@ describe("getGitmemDir walk-up with multiple sentinels", () => { vi.spyOn(process, "cwd").mockReturnValue(subDir); - const result = getGitmemDir(); - // Falls back to ~/.gitmem since active-session.json is no longer a sentinel - expect(result).toBe(path.join(os.homedir(), ".gitmem")); + expect(getGitmemDir()).toBe(getHomeGitmemDir()); }); - it("prefers active-sessions.json over config.json at same level", () => { + it("selects a project-scoped root when GITMEM_DIR names it", () => { const projectDir = path.join(tmpDir, "project"); const gitmemDir = path.join(projectDir, ".gitmem"); @@ -131,10 +135,16 @@ describe("getGitmemDir walk-up with multiple sentinels", () => { fs.writeFileSync(path.join(gitmemDir, "config.json"), "{}"); vi.spyOn(process, "cwd").mockReturnValue(projectDir); - - // This always works since active-sessions.json is checked first - const result = getGitmemDir(); - expect(result).toBe(gitmemDir); + const previous = process.env.GITMEM_DIR; + process.env.GITMEM_DIR = gitmemDir; + try { + clearGitmemDirCache(); + expect(getGitmemDir()).toBe(gitmemDir); + } finally { + if (previous === undefined) delete process.env.GITMEM_DIR; + else process.env.GITMEM_DIR = previous; + clearGitmemDirCache(); + } }); it("falls back to ~/.gitmem when no sentinel found", () => { @@ -144,6 +154,6 @@ describe("getGitmemDir walk-up with multiple sentinels", () => { vi.spyOn(process, "cwd").mockReturnValue(emptyDir); const result = getGitmemDir(); - expect(result).toBe(path.join(os.homedir(), ".gitmem")); + expect(result).toBe(getHomeGitmemDir()); }); }); diff --git a/tests/unit/services/gitmem-root-resolution.test.ts b/tests/unit/services/gitmem-root-resolution.test.ts new file mode 100644 index 0000000..cdd086e --- /dev/null +++ b/tests/unit/services/gitmem-root-resolution.test.ts @@ -0,0 +1,250 @@ +/** + * GIT-91: the .gitmem root must not depend on process.cwd(). + * + * Resolution used to walk up from cwd and adopt any directory containing + * active-sessions.json or config.json. The processes that share a session do not + * share a cwd — the MCP server runs from wherever the client launched it, the + * SessionStart hook runs in the repo — so one logical session bound to two + * different stores, and writes landed in a root that identity resolution never + * read. On the machine where this was found there were three such roots. + * + * Tightening the sentinel to require live state was the first attempt and does + * not hold: test-written sessions carry a structurally valid session.json + * (GIT-92), so a repo containing only stale test residue still qualified. And + * even a perfect liveness check cannot make two processes with different cwds + * agree — the dependency on cwd is the defect, not the strictness of the test. + * + * So the invariant under test is not "picks the best root". It is "picks the + * SAME root from anywhere". Everything else here supports that. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import * as fs from "fs"; +import * as path from "path"; +import * as os from "os"; +import { + getGitmemDir, + clearGitmemDirCache, + getHomeGitmemDir, + isLiveGitmemRoot, + describeGitmemRoot, +} from "../../../src/services/gitmem-dir.js"; + +const HOME_ROOT = getHomeGitmemDir(); +let tmp: string; +const originalEnv = process.env.GITMEM_DIR; + +/** + * vitest workers forbid changing the working directory, so cwd is stubbed. + * That is closer to the real defect anyway: the point is that resolution READS + * cwd, and the processes sharing a session report different values for it. + */ +function atCwd(dir: string): void { + vi.spyOn(process, "cwd").mockReturnValue(dir); + clearGitmemDirCache(); +} + +/** A .gitmem holding one structurally valid session — what tests leave behind. */ +function seedSessionRoot(dir: string, sessionId = "aaaaaaaa-1111-2222-3333-444444444444"): string { + const gitmem = path.join(dir, ".gitmem"); + fs.mkdirSync(path.join(gitmem, "sessions", sessionId), { recursive: true }); + fs.writeFileSync(path.join(gitmem, "active-sessions.json"), JSON.stringify({ sessions: [] })); + fs.writeFileSync( + path.join(gitmem, "sessions", sessionId, "session.json"), + JSON.stringify({ session_id: sessionId, agent: "cli", project: "gitmem_test" }) + ); + return gitmem; +} + +describe("GIT-91: .gitmem root resolution is independent of cwd", () => { + beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-root-")); + delete process.env.GITMEM_DIR; + clearGitmemDirCache(); + vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + clearGitmemDirCache(); + if (originalEnv === undefined) delete process.env.GITMEM_DIR; + else process.env.GITMEM_DIR = originalEnv; + fs.rmSync(tmp, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it("resolves the same root from inside a project that has its own .gitmem", () => { + seedSessionRoot(tmp); + + atCwd(tmp); + const fromProject = getGitmemDir(); + + atCwd(os.tmpdir()); + const fromElsewhere = getGitmemDir(); + + // The whole bug in one assertion: these used to differ. + expect(fromProject).toBe(fromElsewhere); + expect(fromProject).toBe(HOME_ROOT); + }); + + it("does not adopt a project root left behind by a test run", () => { + // Structurally valid session, stale — indistinguishable from a real one, + // which is why a liveness check was not enough (GIT-92). + seedSessionRoot(tmp); + atCwd(tmp); + + expect(getGitmemDir()).toBe(HOME_ROOT); + }); + + it("does not adopt a project root that merely contains an empty registry", () => { + const gitmem = path.join(tmp, ".gitmem"); + fs.mkdirSync(gitmem, { recursive: true }); + fs.writeFileSync(path.join(gitmem, "active-sessions.json"), JSON.stringify({ sessions: [] })); + atCwd(tmp); + + expect(getGitmemDir()).toBe(HOME_ROOT); + }); + + it("honours GITMEM_DIR, which is now the only way to select a project root", () => { + const explicit = path.join(tmp, "explicit-root"); + fs.mkdirSync(explicit, { recursive: true }); + process.env.GITMEM_DIR = explicit; + atCwd(os.tmpdir()); + + expect(getGitmemDir()).toBe(explicit); + }); + + it("reports a stranded project root instead of silently ignoring it", () => { + seedSessionRoot(tmp); + atCwd(tmp); + const errors: string[] = []; + (console.error as unknown as { mockImplementation: (f: (m: string) => void) => void }) + .mockImplementation((m: string) => { errors.push(String(m)); }); + + getGitmemDir(); + + const notice = errors.find((e) => e.includes("NOT being used")); + expect(notice, "a root holding live state that is no longer read must be reported").toBeTruthy(); + expect(notice).toContain("GITMEM_DIR"); + // Never move or delete a user's memory store to make resolution tidy. + expect(notice).toContain("Nothing has been moved or deleted"); + }); +}); + +describe("GIT-91: isLiveGitmemRoot only counts real evidence", () => { + beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-live-")); + vi.spyOn(console, "error").mockImplementation(() => {}); + }); + afterEach(() => { + fs.rmSync(tmp, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it("counts a deliberate project-scoped install (config.json)", () => { + const gitmem = path.join(tmp, ".gitmem"); + fs.mkdirSync(gitmem, { recursive: true }); + fs.writeFileSync(path.join(gitmem, "config.json"), JSON.stringify({ project: "x" })); + + expect(isLiveGitmemRoot(gitmem)).toBe(true); + }); + + it("counts a registry naming at least one session", () => { + const gitmem = path.join(tmp, ".gitmem"); + fs.mkdirSync(gitmem, { recursive: true }); + fs.writeFileSync( + path.join(gitmem, "active-sessions.json"), + JSON.stringify({ sessions: [{ session_id: "abc", pid: 1 }] }) + ); + + expect(isLiveGitmemRoot(gitmem)).toBe(true); + }); + + it("does not count an empty registry — that means the opposite", () => { + const gitmem = path.join(tmp, ".gitmem"); + fs.mkdirSync(gitmem, { recursive: true }); + fs.writeFileSync(path.join(gitmem, "active-sessions.json"), JSON.stringify({ sessions: [] })); + + expect(isLiveGitmemRoot(gitmem)).toBe(false); + }); + + it("does not count session directories with no session.json", () => { + const gitmem = path.join(tmp, ".gitmem"); + // Exactly the residue found in the wild: named like sessions, empty inside. + for (const name of ["original-session", "test-session-2", "test-session-clean"]) { + fs.mkdirSync(path.join(gitmem, "sessions", name), { recursive: true }); + } + + expect(isLiveGitmemRoot(gitmem)).toBe(false); + }); + + it("does not throw on a missing or unreadable candidate", () => { + expect(isLiveGitmemRoot(path.join(tmp, "does-not-exist"))).toBe(false); + }); +}); + +/** + * R18 acceptance: detection-without-use must state what the stranded store + * HOLDS, not merely that one exists. + * + * The ruling rejected a read-only compatibility fallback — reading the legacy + * path with a warning keeps the cross-process disagreement the fix exists to + * kill, because the hook and the server still resolve differently. Detection + * replaces it, and detection only works if the user can weigh what they are + * told: "your memory is at another path" is abstract enough to scroll past, + * "3 learnings, 2 sessions are sitting there" is not. + */ +describe("R18: describeGitmemRoot reports what a stranded store holds", () => { + let store: string; + + beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-counts-")); + store = path.join(tmp, ".gitmem"); + }); + afterEach(() => fs.rmSync(tmp, { recursive: true, force: true })); + + it("counts learnings, threads and sessions in a populated legacy store", () => { + fs.mkdirSync(path.join(store, "sessions", "s1"), { recursive: true }); + fs.mkdirSync(path.join(store, "sessions", "s2"), { recursive: true }); + fs.writeFileSync(path.join(store, "sessions", "s1", "session.json"), '{"session_id":"s1"}'); + fs.writeFileSync(path.join(store, "sessions", "s2", "session.json"), '{"session_id":"s2"}'); + fs.writeFileSync(path.join(store, "learnings.json"), JSON.stringify([{ id: 1 }, { id: 2 }, { id: 3 }])); + fs.writeFileSync(path.join(store, "threads.json"), JSON.stringify([{ id: "t1" }])); + + const c = describeGitmemRoot(store); + + expect(c.learnings).toBe(3); + expect(c.threads).toBe(1); + expect(c.sessions).toBe(2); + }); + + it("does not count a session directory with no session.json", () => { + fs.mkdirSync(path.join(store, "sessions", "test-session-2"), { recursive: true }); + + expect(describeGitmemRoot(store).sessions).toBe(0); + }); + + it("handles the {key: array} file shape as well as a bare array", () => { + fs.mkdirSync(store, { recursive: true }); + fs.writeFileSync( + path.join(store, "learnings.json"), + JSON.stringify({ learnings: [{ id: 1 }, { id: 2 }] }) + ); + + expect(describeGitmemRoot(store).learnings).toBe(2); + }); + + it("returns zeros rather than throwing on a malformed store", () => { + fs.mkdirSync(store, { recursive: true }); + fs.writeFileSync(path.join(store, "learnings.json"), "not json at all"); + + // A notice that failed to render because one file is corrupt would + // reintroduce exactly the silence detection exists to prevent. + expect(() => describeGitmemRoot(store)).not.toThrow(); + expect(describeGitmemRoot(store).learnings).toBe(0); + }); + + it("reports zeros for a root that does not exist", () => { + expect(describeGitmemRoot(path.join(tmp, "nope", ".gitmem"))) + .toMatchObject({ learnings: 0, threads: 0, sessions: 0 }); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 2982209..215f53d 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,4 +1,22 @@ import { defineConfig } from "vitest/config"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +/** + * GIT-92/GIT-91: every worker gets a throwaway HOME. + * + * The suite writes real session state through getGitmemDir(), and since GIT-91 + * removed the cwd walk-up that resolves ~/.gitmem — the developer's actual + * store. Redirecting HOME (rather than GITMEM_DIR) moves only the final + * fallback, so the precedence chain GITMEM_DIR > cache > home stays intact and + * suites that point at their own temp root via setGitmemDir() keep working. + * + * GITMEM_HOME rather than HOME: vitest runs on pool "threads", where + * process.env is a JS-level copy that never reaches native getenv(), so + * os.homedir() cannot be redirected from inside a worker at all. + */ +const TEST_HOME = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-test-home-")); /** * Vitest configuration for Tier 1 unit tests. @@ -42,6 +60,15 @@ export default defineConfig({ // Environment environment: "node", + // GIT-92/GIT-91: pin every worker to a throwaway .gitmem root before any + // test imports gitmem-dir. Without this the suite writes session + // directories into the developer's real store — and since GIT-91 removed + // the cwd walk-up, "real store" means ~/.gitmem, not a repo-local one. + setupFiles: ["tests/setup/isolate-gitmem-root.ts"], + + // See TEST_HOME above — this is what actually isolates the store. + env: { GITMEM_HOME: TEST_HOME }, + // Clear mocks between tests clearMocks: true, restoreMocks: true,