diff --git a/.gitignore b/.gitignore index 8033af9..b33b3c3 100644 --- a/.gitignore +++ b/.gitignore @@ -14,5 +14,9 @@ dist/ .npmrc CLAUDE.md +# Credentials — never commit +.mcpregistry_github_token +.mcpregistry_registry_token + # Internal admin docs (not shipped) docs/admin-license-management.md diff --git a/src/services/active-sessions.ts b/src/services/active-sessions.ts index 79c6224..4ff0348 100644 --- a/src/services/active-sessions.ts +++ b/src/services/active-sessions.ts @@ -22,7 +22,6 @@ import { withLockSync } from "./file-lock.js"; const REGISTRY_FILENAME = "active-sessions.json"; const LOCK_FILENAME = "active-sessions.lock"; const STALE_THRESHOLD_MS = 24 * 60 * 60 * 1000; // 24 hours -const ADOPT_THRESHOLD_MS = 2 * 60 * 60 * 1000; // 2 hours — adopt dead-PID sessions if recent // --- Atomic write utility --- @@ -54,6 +53,25 @@ function getRegistryPath(): string { return path.join(getGitmemDir(), REGISTRY_FILENAME); } +/** + * A cheap fingerprint of the registry's current state (GIT-51). + * + * Session recovery needs to know whether it is worth retrying. The registry is + * written by OTHER processes — notably the SessionStart hook, which runs as its + * own CLI process (scar 55d1bccd) — so "I already failed to recover" is only + * valid until someone else touches the file. + * + * Returns null when the registry does not exist, which is itself a stable + * state worth caching against. + */ +export function getRegistryFingerprint(): number | null { + try { + return fs.statSync(getRegistryPath()).mtimeMs; + } catch { + return null; + } +} + function getLockPath(): string { return path.join(getGitmemDir(), LOCK_FILENAME); } @@ -181,13 +199,90 @@ export function findSessionById(sessionId: string): ActiveSessionEntry | null { return registry.sessions.find((s) => s.session_id === sessionId) || null; } +/** + * Check whether a PID is currently running. + * EPERM means the process exists but belongs to another user — still alive. + */ +function isPidAlive(pid: number): boolean { + try { + process.kill(pid, 0); // Signal 0 = check existence only + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === "EPERM"; + } +} + +/** + * GIT-51: Adopt a session left behind by a previous incarnation of this MCP + * server process, rebinding it to the current PID. + * + * The MCP server restarts routinely mid-session (context compaction, rebuild, + * client restart). The hostname stays the same but the PID changes, so + * findSessionByHostPid() no longer matches and the session looks gone. A + * registry entry on this host whose PID is dead, with its session file still on + * disk, is that session. + * + * Deliberately narrow: + * - Adopts at most ONE entry. Rebinding every dead-PID entry would leave + * multiple rows sharing hostname+pid, which findSessionByHostPid() then + * resolves arbitrarily by array order. + * - Never adopts a session whose PID is still alive (GIT-20: "never resume + * another process's session"). + * - Not gated on session age beyond the 24h stale horizon. The old 2h adoption + * window was shorter than a normal working session, so the sessions most in + * need of recovery were the ones excluded from it. + * + * Returns the adopted entry (with the updated PID), or null if there is nothing + * to adopt. + */ +export function adoptSessionForCurrentProcess(): ActiveSessionEntry | null { + const currentHostname = os.hostname(); + const currentPid = process.pid; + const gitmemDir = getGitmemDir(); + + return withLockSync(getLockPath(), () => { + const registry = readRegistry(); + const now = Date.now(); + + const candidates = registry.sessions + .filter((entry) => { + if (entry.hostname !== currentHostname) return false; + if (isPidAlive(entry.pid)) return false; + + const age = now - new Date(entry.started_at).getTime(); + if (!Number.isFinite(age) || age > STALE_THRESHOLD_MS) return false; + + // The session file is the evidence that the session is real and resumable. + const sessionFile = path.join(gitmemDir, "sessions", entry.session_id, "session.json"); + return fs.existsSync(sessionFile); + }) + .sort((a, b) => new Date(b.started_at).getTime() - new Date(a.started_at).getTime()); + + const adopted = candidates[0]; + if (!adopted) return null; + + console.error( + `[active-sessions] Adopting orphaned session ${adopted.session_id.slice(0, 8)} (dead pid ${adopted.pid} → ${currentPid})` + ); + adopted.pid = currentPid; + writeRegistry(registry); + + return { ...adopted }; + }); +} + /** * Prune stale sessions from the registry (GIT-22 enhanced). * * A session is stale if: * 1. Its started_at is older than 24 hours, OR - * 2. Its PID no longer exists on this hostname (process died without cleanup), OR - * 3. Its per-session directory/session.json is missing (orphaned registry entry) + * 2. Its per-session directory/session.json is missing (orphaned registry entry) + * + * GIT-51: A dead PID is NOT on its own grounds for pruning. The MCP server + * restarting mid-session leaves exactly that signature, and deleting the entry + * (and its session directory) destroys a live session's state. Dead-PID entries + * are left for adoptSessionForCurrentProcess() to recover, and fall out on the + * 24h age path if nobody claims them. * * Also cleans up per-session directories for pruned sessions, * and removes orphaned session directories with no registry entry. @@ -197,11 +292,8 @@ export function pruneStale(): number { return withLockSync(getLockPath(), () => { const registry = readRegistry(); const now = Date.now(); - const currentHostname = os.hostname(); - const currentPid = process.pid; const before = registry.sessions.length; const gitmemDir = getGitmemDir(); - let adopted = false; registry.sessions = registry.sessions.filter((entry) => { // GIT-22: Check for orphaned registry entry (session file missing) @@ -229,39 +321,15 @@ export function pruneStale(): number { return false; } - // Check if PID is alive (only for sessions on the same host) - if (entry.hostname === currentHostname) { - try { - process.kill(entry.pid, 0); // Signal 0 = check existence only - } catch { - // PID is dead. If session is recent, adopt it into the current process - // instead of pruning. This handles MCP server restarts during context - // compaction where the hostname stays the same but the PID changes. - if (age < ADOPT_THRESHOLD_MS) { - console.error( - `[active-sessions] Adopting orphaned session ${entry.session_id.slice(0, 8)} (dead pid ${entry.pid} → ${currentPid})` - ); - entry.pid = currentPid; - adopted = true; - return true; // Keep in registry with updated PID - } - console.error( - `[active-sessions] Pruning dead session ${entry.session_id.slice(0, 8)} (pid ${entry.pid} no longer running)` - ); - cleanupSessionDir(gitmemDir, entry.session_id); - return false; - } - } - + // GIT-51: dead PID on this host = restarted server, not a dead session. + // Left in place for adoptSessionForCurrentProcess() to recover. return true; }); const pruned = before - registry.sessions.length; - if (pruned > 0 || adopted) { + if (pruned > 0) { writeRegistry(registry); - if (pruned > 0) { - console.error(`[active-sessions] Pruned ${pruned} stale session(s)`); - } + console.error(`[active-sessions] Pruned ${pruned} stale session(s)`); } // GIT-22: Clean up orphaned session directories (dir exists but no registry entry) diff --git a/src/services/session-state.ts b/src/services/session-state.ts index 0c00cc1..e9957b0 100644 --- a/src/services/session-state.ts +++ b/src/services/session-state.ts @@ -16,7 +16,7 @@ import fs from "fs"; import * as os from "os"; import type { SurfacedScar, ScarConfirmation, ScarReflection, Observation, SessionChild, ThreadObject } from "../types/index.js"; import { getSessionPath } from "./gitmem-dir.js"; -import { listActiveSessions } from "./active-sessions.js"; +import { findSessionByHostPid, adoptSessionForCurrentProcess, getRegistryFingerprint } from "./active-sessions.js"; interface SessionContext { sessionId: string; @@ -32,11 +32,37 @@ interface SessionContext { children: SessionChild[]; // v2 Phase 2: Child agent records threads: ThreadObject[]; // : Working thread state feedbackSubmitCount: number; // Rate limit counter for contribute_feedback + /** + * GIT-51: set when this session was recovered from disk and session.json + * disagreed with the registry about which project it belongs to. Carried on + * the context so a consumer can see that its scope rests on a resolved + * conflict rather than on agreement. + */ + recoveryConflict?: boolean; } // Global session state (single active session per MCP server instance) let currentSession: SessionContext | null = null; +// GIT-51: recovery reads the registry and a session file, so it does not repeat +// while nothing has changed. Reset whenever state is set or cleared. +let recoveryAttempted = false; + +// GIT-51 (reconciliation): the registry fingerprint at the last FAILED recovery. +// +// A plain boolean latch was permanently fail-closed in one real scenario: the +// SessionStart hook runs as a separate CLI process (scar 55d1bccd), so a +// session can appear in the registry AFTER this process has already tried and +// failed to recover. With GIT-67's R5 guard now refusing sessionless writes on +// pro, that combination turns one early miss into every subsequent write being +// refused for the life of the process — a fail-closed bug replacing a fail-open +// one. +// +// Keying the latch on the registry's mtime keeps the I/O-avoidance the boolean +// was for (no repeated reads while nothing changed) without making the failure +// permanent. +let lastFailedRecoveryFingerprint: number | null | undefined = undefined; + /** * Set the current active session * Called by session_start @@ -53,15 +79,111 @@ export function setCurrentSession(context: Omit 0) { - return currentSession.surfacedScars; + if (session?.surfacedScars && session.surfacedScars.length > 0) { + return session.surfacedScars; } - // Fallback 1: recover from per-session file if in-memory was lost (MCP restart) - if (currentSession?.sessionId) { + // Fallback: re-read the resolved session's file. Covers the case where + // identity survived but the scars array did not — e.g. scars were appended by + // another process, or by this session before an in-place refresh. + // + // GIT-51: this deliberately reads only the *resolved* session's file. The + // previous "newest session on this host" fallback could hand one session + // another concurrent session's scars. + if (session?.sessionId) { try { - const sessionFilePath = getSessionPath(currentSession.sessionId, "session.json"); + const sessionFilePath = getSessionPath(session.sessionId, "session.json"); if (fs.existsSync(sessionFilePath)) { const data = JSON.parse(fs.readFileSync(sessionFilePath, "utf-8")); if (data.surfaced_scars && Array.isArray(data.surfaced_scars) && data.surfaced_scars.length > 0) { - currentSession.surfacedScars = data.surfaced_scars; + session.surfacedScars = data.surfaced_scars; console.error(`[session-state] Recovered ${data.surfaced_scars.length} surfaced scars from file`); return data.surfaced_scars; } @@ -155,33 +288,6 @@ export function getSurfacedScars(): SurfacedScar[] { } } - // Fallback 2: if currentSession is null (MCP restarted completely), find session from registry. - // After MCP restart the PID changes, but the active-sessions registry may still have - // the previous session entry (pruneStale adopts dead-PID sessions on same hostname). - if (!currentSession) { - try { - const hostname = os.hostname(); - const sessions = listActiveSessions(); - const candidates = sessions - .filter(s => s.hostname === hostname) - .sort((a, b) => new Date(b.started_at).getTime() - new Date(a.started_at).getTime()); - - if (candidates.length > 0) { - const recoveredId = candidates[0].session_id; - const sessionFilePath = getSessionPath(recoveredId, "session.json"); - if (fs.existsSync(sessionFilePath)) { - const data = JSON.parse(fs.readFileSync(sessionFilePath, "utf-8")); - if (data.surfaced_scars && Array.isArray(data.surfaced_scars) && data.surfaced_scars.length > 0) { - console.error(`[session-state] Recovered ${data.surfaced_scars.length} surfaced scars from registry session ${recoveredId.slice(0, 8)}`); - return data.surfaced_scars; - } - } - } - } catch (error) { - console.warn("[session-state] Failed to recover surfaced scars from registry:", error); - } - } - return []; } diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index cb70bec..edbef8c 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -13,7 +13,7 @@ import * as supabase from "../services/supabase-client.js"; import { embed, isEmbeddingAvailable } from "../services/embedding.js"; import { hasSupabase, hasProInsights, getTableName } from "../services/tier.js"; import { getStorage } from "../services/storage.js"; -import { clearCurrentSession, getSurfacedScars, getConfirmations, getReflections, getObservations, getChildren, getThreads, getSessionActivity, isRecallCalled } from "../services/session-state.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 { deduplicateThreadList } from "../services/thread-dedup.js"; import { syncThreadsToSupabase, loadOpenThreadEmbeddings } from "../services/thread-supabase.js"; @@ -854,9 +854,14 @@ export async function sessionClose( // GIT-21: Recover session_id from active-sessions registry (hostname+PID) or legacy file if (!params.session_id && params.close_type !== "retroactive") { - // Try registry first (GIT-20 writes here) + // Try registry first (GIT-20 writes here). + // GIT-51: resolveCurrentSession() also adopts a session orphaned by an MCP + // restart, so close still finds the original session_id after a restart. try { - const mySession = findSessionByHostPid(os.hostname(), process.pid); + const resolved = resolveCurrentSession(); + const mySession = resolved + ? { session_id: resolved.sessionId } + : findSessionByHostPid(os.hostname(), process.pid); if (mySession) { console.error(`[session_close] Recovered session_id from registry: ${mySession.session_id}`); params = { ...params, session_id: mySession.session_id }; diff --git a/src/tools/session-start.ts b/src/tools/session-start.ts index 637ca31..91fd1b3 100644 --- a/src/tools/session-start.ts +++ b/src/tools/session-start.ts @@ -37,7 +37,7 @@ import { resolveThreadScope, computePanelOmission, formatOmissionLine } from ".. 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 { registerSession, findSessionByHostPid, pruneStale, migrateFromLegacy } from "../services/active-sessions.js"; +import { registerSession, findSessionByHostPid, adoptSessionForCurrentProcess, pruneStale, migrateFromLegacy } from "../services/active-sessions.js"; import * as os from "os"; import { formatDate } from "../services/timezone.js"; import { productLine, dimText, boldText } from "../services/display-protocol.js"; @@ -711,8 +711,11 @@ function checkExistingSession( // GIT-20: Prune stale sessions from crashed/dead containers pruneStale(); - // GIT-20: Check registry for THIS process's session (hostname + PID match) - const mySession = findSessionByHostPid(os.hostname(), process.pid); + // GIT-20: Check registry for THIS process's session (hostname + PID match). + // GIT-51: if the server restarted, the PID no longer matches — adopt the + // session this process left behind rather than starting a second one. + const mySession = + findSessionByHostPid(os.hostname(), process.pid) ?? adoptSessionForCurrentProcess(); if (mySession) { console.error(`[session_start] Found own session in registry: ${mySession.session_id} (host: ${mySession.hostname}, pid: ${mySession.pid})`); const data = readSessionFile(mySession.session_id); diff --git a/tests/unit/services/active-sessions.test.ts b/tests/unit/services/active-sessions.test.ts index f201def..64924c2 100644 --- a/tests/unit/services/active-sessions.test.ts +++ b/tests/unit/services/active-sessions.test.ts @@ -16,6 +16,7 @@ import { listActiveSessions, findSessionByHostPid, findSessionById, + adoptSessionForCurrentProcess, pruneStale, migrateFromLegacy, resetMigrationFlag, @@ -255,40 +256,42 @@ describe("pruneStale", () => { expect(sessions[0].session_id).toBe(fresh.session_id); }); - it("adopts recent sessions with dead PIDs on same host (MCP server restart)", () => { + it("keeps dead-PID sessions on this host for adoption (GIT-51)", () => { + // A dead PID on this host is the signature of an MCP server restart, not a + // dead session. Pruning it here deleted the session directory out from + // under a session that was still running. const dead = makeEntry({ session_id: "11111111-1111-1111-1111-111111111111", hostname: os.hostname(), pid: 99999999, // very unlikely to be a real PID - started_at: new Date().toISOString(), // recent — should be adopted + started_at: new Date().toISOString(), }); registerSession(dead); createSessionFile(dead.session_id); // file exists but PID is dead const pruned = pruneStale(); - expect(pruned).toBe(0); // adopted, not pruned + expect(pruned).toBe(0); const sessions = listActiveSessions(); expect(sessions).toHaveLength(1); expect(sessions[0].session_id).toBe(dead.session_id); - expect(sessions[0].pid).toBe(process.pid); // PID updated to current process + expect(fs.existsSync(path.join(tmpDir, "sessions", dead.session_id))).toBe(true); }); - it("prunes old sessions with dead PIDs on same host", () => { + it("keeps dead-PID sessions older than the old 2h adoption window (GIT-51)", () => { const oldDead = makeEntry({ session_id: "11111111-1111-1111-1111-111111111111", hostname: os.hostname(), - pid: 99999999, // very unlikely to be a real PID - started_at: new Date(Date.now() - 3 * 60 * 60 * 1000).toISOString(), // 3h ago — past adopt threshold + pid: 99999999, + started_at: new Date(Date.now() - 3 * 60 * 60 * 1000).toISOString(), // 3h ago }); registerSession(oldDead); createSessionFile(oldDead.session_id); - const pruned = pruneStale(); - expect(pruned).toBe(1); - expect(listActiveSessions()).toHaveLength(0); + expect(pruneStale()).toBe(0); + expect(listActiveSessions()).toHaveLength(1); }); it("keeps sessions with live PIDs", () => { @@ -414,6 +417,126 @@ describe("pruneStale", () => { }); }); +describe("adoptSessionForCurrentProcess (GIT-51)", () => { + it("adopts a session left by a previous incarnation of this process", () => { + const dead = makeEntry({ + session_id: "11111111-1111-1111-1111-111111111111", + hostname: os.hostname(), + pid: 99999999, + started_at: new Date().toISOString(), + }); + registerSession(dead); + createSessionFile(dead.session_id); + + const adopted = adoptSessionForCurrentProcess(); + + expect(adopted).not.toBeNull(); + expect(adopted!.session_id).toBe(dead.session_id); + expect(adopted!.pid).toBe(process.pid); + expect(listActiveSessions()[0].pid).toBe(process.pid); + }); + + it("adopts a session older than 2 hours", () => { + // The old ADOPT_THRESHOLD_MS window was shorter than a normal working + // session, so long sessions — the ones that most need recovery — were + // excluded from it. + const dead = makeEntry({ + session_id: "11111111-1111-1111-1111-111111111111", + hostname: os.hostname(), + pid: 99999999, + started_at: new Date(Date.now() - 6 * 60 * 60 * 1000).toISOString(), + }); + registerSession(dead); + createSessionFile(dead.session_id); + + expect(adoptSessionForCurrentProcess()?.session_id).toBe(dead.session_id); + }); + + it("adopts at most one entry — never leaves two rows sharing hostname+pid", () => { + const first = makeEntry({ + session_id: "11111111-1111-1111-1111-111111111111", + hostname: os.hostname(), + pid: 99999998, + started_at: new Date(Date.now() - 60 * 60 * 1000).toISOString(), + }); + const second = makeEntry({ + session_id: "22222222-2222-2222-2222-222222222222", + hostname: os.hostname(), + pid: 99999999, + started_at: new Date().toISOString(), + }); + registerSession(first); + registerSession(second); + createSessionFile(first.session_id); + createSessionFile(second.session_id); + + const adopted = adoptSessionForCurrentProcess(); + + // Most recently started wins — deterministic, not array order. + expect(adopted!.session_id).toBe(second.session_id); + + const sessions = listActiveSessions(); + const mine = sessions.filter((s) => s.hostname === os.hostname() && s.pid === process.pid); + expect(mine).toHaveLength(1); + expect(mine[0].session_id).toBe(second.session_id); + }); + + it("never claims a session whose PID is still alive", () => { + const live = makeEntry({ + session_id: "11111111-1111-1111-1111-111111111111", + hostname: os.hostname(), + pid: process.pid, // this process is alive + }); + registerSession(live); + createSessionFile(live.session_id); + + // findSessionByHostPid already resolves our own session; adoption must not + // also claim it (nor any other live process's session). + expect(adoptSessionForCurrentProcess()).toBeNull(); + }); + + it("ignores dead-PID sessions on other hosts", () => { + const remote = makeEntry({ + session_id: "11111111-1111-1111-1111-111111111111", + hostname: "some-other-container", + pid: 99999999, + }); + registerSession(remote); + createSessionFile(remote.session_id); + + expect(adoptSessionForCurrentProcess()).toBeNull(); + }); + + it("ignores entries with no session file on disk", () => { + registerSession( + makeEntry({ + session_id: "11111111-1111-1111-1111-111111111111", + hostname: os.hostname(), + pid: 99999999, + }) + ); + + expect(adoptSessionForCurrentProcess()).toBeNull(); + }); + + it("ignores entries past the 24h stale horizon", () => { + const ancient = makeEntry({ + session_id: "11111111-1111-1111-1111-111111111111", + hostname: os.hostname(), + pid: 99999999, + started_at: new Date(Date.now() - 25 * 60 * 60 * 1000).toISOString(), + }); + registerSession(ancient); + createSessionFile(ancient.session_id); + + expect(adoptSessionForCurrentProcess()).toBeNull(); + }); + + it("returns null on an empty registry", () => { + expect(adoptSessionForCurrentProcess()).toBeNull(); + }); +}); + describe("registry corruption recovery", () => { it("handles empty file gracefully", () => { fs.writeFileSync(path.join(tmpDir, "active-sessions.json"), ""); diff --git a/tests/unit/services/session-state-recovery.test.ts b/tests/unit/services/session-state-recovery.test.ts index 9522688..b4e671f 100644 --- a/tests/unit/services/session-state-recovery.test.ts +++ b/tests/unit/services/session-state-recovery.test.ts @@ -1,54 +1,47 @@ /** - * Tests for getSurfacedScars() registry-based recovery (Fallback 2). + * GIT-51: session identity recovery after an MCP server restart. * - * When MCP restarts completely (currentSession is null), getSurfacedScars() - * should recover scars from the active-sessions registry by finding the - * most recent session for the current hostname and reading its session.json. + * Session identity lives in an in-memory `currentSession` that dies with the + * process. The MCP server restarts routinely mid-session (context compaction, + * rebuild, client restart), and until GIT-51 nothing rebuilt that state except + * session_start — which agents don't call again mid-session. The result was + * "No active session" for every session-required tool, and a close that could + * not resolve which session to close. + * + * These tests exercise the real registry and real session files (temp dir) + * rather than module mocks, because the defect was in how those two stores + * relate to each other. */ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import * as os from "os"; - -// Mock fs at module level — vi.spyOn(fs, "existsSync") fails because ESM re-exports are read-only. -// session-state.ts uses `import fs from "fs"` (default import), so we must set `default` explicitly. -vi.mock("fs", async (importOriginal) => { - const actual = await importOriginal(); - const mocked = { - ...actual, - existsSync: vi.fn().mockReturnValue(false), - readFileSync: vi.fn().mockReturnValue("{}"), - }; - return { ...mocked, default: mocked }; -}); - -vi.mock("../../../src/services/active-sessions.js", () => ({ - listActiveSessions: vi.fn().mockReturnValue([]), -})); - -vi.mock("../../../src/services/gitmem-dir.js", () => ({ - getSessionPath: vi.fn().mockReturnValue("/tmp/nonexistent/session.json"), -})); - +import { describe, it, expect, beforeEach, afterEach } from "vitest"; import * as fs from "fs"; +import * as path from "path"; +import * as os from "os"; +import { setGitmemDir, clearGitmemDirCache } from "../../../src/services/gitmem-dir.js"; +import { registerSession, listActiveSessions } from "../../../src/services/active-sessions.js"; import { setCurrentSession, clearCurrentSession, + resolveCurrentSession, + getCurrentSession, getSurfacedScars, addSurfacedScars, } from "../../../src/services/session-state.js"; -import { listActiveSessions } from "../../../src/services/active-sessions.js"; -import { getSessionPath } from "../../../src/services/gitmem-dir.js"; +import type { SurfacedScar } from "../../../src/types/index.js"; const HOSTNAME = os.hostname(); +const DEAD_PID = 99999999; // very unlikely to be a real PID + +let tmpDir: string; -const MOCK_SCARS = [ +const MOCK_SCARS: SurfacedScar[] = [ { scar_id: "aaaa1111-1111-1111-1111-111111111111", scar_title: "Trace execution path first", title: "Trace execution path first", severity: "high", surfaced_at: "2026-02-22T10:00:00.000Z", - source: "recall" as const, + source: "recall", }, { scar_id: "bbbb2222-2222-2222-2222-222222222222", @@ -56,160 +49,318 @@ const MOCK_SCARS = [ title: "Done != Deployed", severity: "high", surfaced_at: "2026-02-22T10:00:00.000Z", - source: "recall" as const, + source: "recall", }, -]; +] as SurfacedScar[]; -describe("getSurfacedScars() registry recovery", () => { - beforeEach(() => { - clearCurrentSession(); - vi.mocked(listActiveSessions).mockReturnValue([]); - vi.mocked(getSessionPath).mockReturnValue("/tmp/nonexistent/session.json"); - vi.mocked(fs.existsSync).mockReturnValue(false); - vi.mocked(fs.readFileSync).mockReturnValue("{}"); +/** + * Simulate a session that was started by a now-dead server process: + * a registry entry with a dead PID plus the session file it wrote. + */ +function seedOrphanedSession(opts: { + sessionId: string; + startedAt?: string; + pid?: number; + scars?: SurfacedScar[]; + project?: string; +}): void { + const startedAt = opts.startedAt || new Date().toISOString(); + registerSession({ + session_id: opts.sessionId, + agent: "cli", + started_at: startedAt, + hostname: HOSTNAME, + pid: opts.pid ?? DEAD_PID, + project: opts.project || "gitmem", }); - it("returns in-memory scars when session is active", () => { - setCurrentSession({ - sessionId: "test-session-id", - agent: "CLI", - startedAt: new Date(), + const sessionDir = path.join(tmpDir, "sessions", opts.sessionId); + fs.mkdirSync(sessionDir, { recursive: true }); + fs.writeFileSync( + path.join(sessionDir, "session.json"), + JSON.stringify({ + session_id: opts.sessionId, + agent: "cli", + started_at: startedAt, + project: opts.project || "gitmem", + hostname: HOSTNAME, + pid: opts.pid ?? DEAD_PID, + surfaced_scars: opts.scars ?? MOCK_SCARS, + threads: [], + }) + ); +} + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git51-")); + setGitmemDir(tmpDir); + clearCurrentSession(); +}); + +afterEach(() => { + clearCurrentSession(); + clearGitmemDirCache(); + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +describe("resolveCurrentSession() — restart recovery", () => { + it("re-binds identity after a restart (dead PID + session file on disk)", () => { + const sessionId = "11111111-1111-1111-1111-111111111111"; + seedOrphanedSession({ sessionId }); + + // In-memory state is null — exactly what a restarted server sees. + const resolved = resolveCurrentSession(); + + expect(resolved).not.toBeNull(); + expect(resolved!.sessionId).toBe(sessionId); + expect(resolved!.surfacedScars).toHaveLength(2); + expect(resolved!.project).toBe("gitmem"); + }); + + it("recovers a session older than the old 2h adoption window", () => { + // Regression test for ADOPT_THRESHOLD_MS: real sessions routinely run + // longer than two hours, so the sessions most in need of recovery were + // precisely the ones the old window excluded. + const sessionId = "22222222-2222-2222-2222-222222222222"; + seedOrphanedSession({ + sessionId, + startedAt: new Date(Date.now() - 6 * 60 * 60 * 1000).toISOString(), }); - addSurfacedScars(MOCK_SCARS); - const result = getSurfacedScars(); - expect(result.length).toBe(2); - expect(result[0].scar_id).toBe(MOCK_SCARS[0].scar_id); + const resolved = resolveCurrentSession(); + + expect(resolved).not.toBeNull(); + expect(resolved!.sessionId).toBe(sessionId); + expect(resolved!.surfacedScars).toHaveLength(2); }); - it("returns empty array when no session and no registry entries", () => { - vi.mocked(listActiveSessions).mockReturnValue([]); + it("rebinds the registry entry to the current PID so close can find it", () => { + const sessionId = "33333333-3333-3333-3333-333333333333"; + seedOrphanedSession({ sessionId }); - const result = getSurfacedScars(); - expect(result).toEqual([]); + resolveCurrentSession(); + + const entries = listActiveSessions(); + expect(entries).toHaveLength(1); + expect(entries[0].session_id).toBe(sessionId); + expect(entries[0].pid).toBe(process.pid); }); - it("recovers scars from registry when currentSession is null", () => { - const sessionId = "recovered-session-id-1234-567890abcdef"; - const sessionFilePath = `/tmp/.gitmem/sessions/${sessionId}/session.json`; + it("preserves surfaced_scars through the restart", () => { + const sessionId = "44444444-4444-4444-4444-444444444444"; + seedOrphanedSession({ sessionId }); - vi.mocked(listActiveSessions).mockReturnValue([ - { - session_id: sessionId, - hostname: HOSTNAME, - pid: 99999, - agent: "CLI", - started_at: new Date().toISOString(), - }, + const scars = getSurfacedScars(); + + expect(scars.map((s) => s.scar_id)).toEqual([ + "aaaa1111-1111-1111-1111-111111111111", + "bbbb2222-2222-2222-2222-222222222222", ]); + }); - vi.mocked(getSessionPath).mockReturnValue(sessionFilePath); - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue( - JSON.stringify({ surfaced_scars: MOCK_SCARS }) - ); + it("returns the in-memory session without touching disk when one is active", () => { + setCurrentSession({ + sessionId: "live-session", + agent: "cli", + startedAt: new Date(), + }); + addSurfacedScars(MOCK_SCARS); - const result = getSurfacedScars(); + // A stale orphan exists, but the live session must win. + seedOrphanedSession({ sessionId: "55555555-5555-5555-5555-555555555555" }); - expect(result.length).toBe(2); - expect(result[0].scar_title).toBe("Trace execution path first"); - expect(result[1].scar_title).toBe("Done != Deployed"); + expect(getCurrentSession()!.sessionId).toBe("live-session"); }); - it("picks most recent session from registry when multiple exist", () => { - const oldSessionId = "old-session-aaaa-bbbb-ccccddddeeee"; - const newSessionId = "new-session-1111-2222-333344445555"; + it("returns null when there is genuinely no session", () => { + expect(resolveCurrentSession()).toBeNull(); + expect(getSurfacedScars()).toEqual([]); + }); - vi.mocked(listActiveSessions).mockReturnValue([ - { - session_id: oldSessionId, - hostname: HOSTNAME, - pid: 11111, - agent: "CLI", - started_at: "2026-02-22T08:00:00.000Z", - }, - { - session_id: newSessionId, - hostname: HOSTNAME, - pid: 22222, - agent: "CLI", - started_at: "2026-02-22T10:00:00.000Z", - }, - ]); + it("does not invent a session from a registry entry with no session file", () => { + registerSession({ + session_id: "66666666-6666-6666-6666-666666666666", + agent: "cli", + started_at: new Date().toISOString(), + hostname: HOSTNAME, + pid: DEAD_PID, + project: "gitmem", + }); - vi.mocked(getSessionPath).mockImplementation((sid: string) => - `/tmp/.gitmem/sessions/${sid}/session.json` - ); + expect(resolveCurrentSession()).toBeNull(); + }); - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockImplementation((filePath) => { - const p = filePath as string; - if (p.includes(newSessionId)) { - return JSON.stringify({ surfaced_scars: MOCK_SCARS }); - } - return JSON.stringify({ surfaced_scars: [] }); + it("ignores sessions belonging to other hosts", () => { + const sessionId = "77777777-7777-7777-7777-777777777777"; + registerSession({ + session_id: sessionId, + agent: "cli", + started_at: new Date().toISOString(), + hostname: "some-other-container", + pid: DEAD_PID, + project: "gitmem", }); + const sessionDir = path.join(tmpDir, "sessions", sessionId); + fs.mkdirSync(sessionDir, { recursive: true }); + fs.writeFileSync( + path.join(sessionDir, "session.json"), + JSON.stringify({ session_id: sessionId, surfaced_scars: MOCK_SCARS }) + ); + + expect(resolveCurrentSession()).toBeNull(); + }); - const result = getSurfacedScars(); - expect(result.length).toBe(2); + it("handles a corrupted session.json without throwing", () => { + const sessionId = "88888888-8888-8888-8888-888888888888"; + seedOrphanedSession({ sessionId }); + fs.writeFileSync( + path.join(tmpDir, "sessions", sessionId, "session.json"), + "not valid json{{{" + ); + + expect(resolveCurrentSession()).toBeNull(); + expect(getSurfacedScars()).toEqual([]); }); +}); - it("ignores registry sessions from different hostnames", () => { - vi.mocked(listActiveSessions).mockReturnValue([ +describe("getSurfacedScars() — cross-session isolation", () => { + it("never hands one session another concurrent session's scars", () => { + // Two sessions on one host. The live one owns this PID; the other is a + // different agent's orphan. The pre-GIT-51 fallback picked "newest session + // on this host" regardless of PID, which could return the wrong scars. + const otherScars = [ { - session_id: "other-host-session", - hostname: "different-host", - pid: 33333, - agent: "CLI", - started_at: new Date().toISOString(), + scar_id: "cccc3333-3333-3333-3333-333333333333", + scar_title: "Someone else's scar", + title: "Someone else's scar", + severity: "high", + surfaced_at: "2026-02-22T12:00:00.000Z", + source: "recall", }, + ] as SurfacedScar[]; + + const mineId = "aaaaaaaa-0000-0000-0000-000000000001"; + const theirsId = "bbbbbbbb-0000-0000-0000-000000000002"; + + // Mine started earlier; theirs is newer — so "newest wins" would pick theirs. + seedOrphanedSession({ + sessionId: mineId, + pid: process.pid, + startedAt: new Date(Date.now() - 60 * 60 * 1000).toISOString(), + }); + seedOrphanedSession({ + sessionId: theirsId, + startedAt: new Date().toISOString(), + scars: otherScars, + }); + + const resolved = resolveCurrentSession(); + + expect(resolved!.sessionId).toBe(mineId); + expect(getSurfacedScars().map((s) => s.scar_id)).toEqual([ + "aaaa1111-1111-1111-1111-111111111111", + "bbbb2222-2222-2222-2222-222222222222", ]); + }); +}); - const result = getSurfacedScars(); - expect(result).toEqual([]); +// --------------------------------------------------------------------------- +// GIT-51 reconciliation (GIT-69 sprint): the two ruled fixes +// --------------------------------------------------------------------------- + +/** Seed a session whose session.json and registry entry disagree on project. */ +function seedConflictingSession(opts: { + sessionId: string; + registryProject: string; + fileProject: string; +}): void { + const startedAt = new Date().toISOString(); + registerSession({ + session_id: opts.sessionId, + agent: "cli", + started_at: startedAt, + hostname: HOSTNAME, + pid: DEAD_PID, + project: opts.registryProject, }); - it("handles corrupted session.json gracefully", () => { - const sessionId = "corrupt-session-1234"; + const sessionDir = path.join(tmpDir, "sessions", opts.sessionId); + fs.mkdirSync(sessionDir, { recursive: true }); + fs.writeFileSync( + path.join(sessionDir, "session.json"), + JSON.stringify({ + session_id: opts.sessionId, + agent: "cli", + started_at: startedAt, + project: opts.fileProject, + hostname: HOSTNAME, + pid: DEAD_PID, + surfaced_scars: [], + threads: [], + }) + ); +} + +describe("recovery retries when the registry changes", () => { + it("finds a session registered AFTER an earlier failed recovery", () => { + // The SessionStart hook runs as a separate CLI process (scar 55d1bccd), so + // a session can appear in the registry after this process already tried and + // failed. A plain boolean latch made that permanent — and with GIT-67's R5 + // guard refusing sessionless writes, one early miss would refuse every + // write for the life of the process. Fail-closed replacing fail-open. + expect(resolveCurrentSession()).toBeNull(); + + seedOrphanedSession({ sessionId: "cccc3333-3333-3333-3333-333333333333" }); + + const recovered = resolveCurrentSession(); + expect(recovered).not.toBeNull(); + expect(recovered?.sessionId).toBe("cccc3333-3333-3333-3333-333333333333"); + }); - vi.mocked(listActiveSessions).mockReturnValue([ - { - session_id: sessionId, - hostname: HOSTNAME, - pid: 44444, - agent: "CLI", - started_at: new Date().toISOString(), - }, - ]); + it("does not re-read while the registry is unchanged", () => { + // The latch still exists — it just is not permanent. Two consecutive + // failures with no registry change stay null without thrashing the disk. + expect(resolveCurrentSession()).toBeNull(); + expect(resolveCurrentSession()).toBeNull(); + }); +}); + +describe("project disagreement between session.json and the registry", () => { + it("flags the conflict instead of resolving it silently", () => { + seedConflictingSession({ + sessionId: "dddd4444-4444-4444-4444-444444444444", + registryProject: "weekend_warrior", + fileProject: "gitmem", + }); - vi.mocked(getSessionPath).mockReturnValue(`/tmp/.gitmem/sessions/${sessionId}/session.json`); - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue("not valid json{{{"); + const recovered = resolveCurrentSession(); - const result = getSurfacedScars(); - expect(result).toEqual([]); + expect(recovered?.recoveryConflict).toBe(true); }); - it("returns empty when session.json has no surfaced_scars field", () => { - const sessionId = "no-scars-session-1234"; + it("keeps session.json authoritative, as documented", () => { + seedConflictingSession({ + sessionId: "eeee5555-5555-5555-5555-555555555555", + registryProject: "weekend_warrior", + fileProject: "gitmem", + }); - vi.mocked(listActiveSessions).mockReturnValue([ - { - session_id: sessionId, - hostname: HOSTNAME, - pid: 55555, - agent: "CLI", - started_at: new Date().toISOString(), - }, - ]); + // Precedence is unchanged from the original `||` — what changed is that it + // is now stated and the disagreement is recorded. GIT-69's scope resolver + // keys on this project, so a silent wrong value scopes the whole session to + // the wrong namespace. + expect(resolveCurrentSession()?.project).toBe("gitmem"); + }); - vi.mocked(getSessionPath).mockReturnValue(`/tmp/.gitmem/sessions/${sessionId}/session.json`); - vi.mocked(fs.existsSync).mockReturnValue(true); - vi.mocked(fs.readFileSync).mockReturnValue( - JSON.stringify({ agent: "CLI", started_at: new Date().toISOString() }) - ); + it("does not flag a conflict when the two agree", () => { + seedOrphanedSession({ + sessionId: "ffff6666-6666-6666-6666-666666666666", + project: "gitmem", + }); + + const recovered = resolveCurrentSession(); - const result = getSurfacedScars(); - expect(result).toEqual([]); + expect(recovered?.project).toBe("gitmem"); + expect(recovered?.recoveryConflict).toBeFalsy(); }); });