From e32ef682031502c2c8feaff08a2c17656e62c435 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 10:08:18 -0500 Subject: [PATCH 1/4] fix: session_close honours or refuses an explicit session_id (GIT-122) Co-Authored-By: Claude Sonnet 5.5 --- src/schemas/session-close.ts | 1 + src/tools/definitions.ts | 4 + src/tools/session-close.ts | 152 +++++++- src/types/index.ts | 4 + .../tools/session-close-id-invariant.test.ts | 338 ++++++++++++++++++ .../tools/session-close-payload-path.test.ts | 5 +- 6 files changed, 496 insertions(+), 8 deletions(-) create mode 100644 tests/unit/tools/session-close-id-invariant.test.ts diff --git a/src/schemas/session-close.ts b/src/schemas/session-close.ts index 0a2fc5e..9641fd6 100644 --- a/src/schemas/session-close.ts +++ b/src/schemas/session-close.ts @@ -107,6 +107,7 @@ export const SessionCloseParamsSchema = z.object({ scars_to_record: z.array(ScarUsageEntrySchema).optional(), capture_transcript: z.boolean().optional(), transcript_path: SafeTranscriptPathSchema.optional(), + reclose: z.boolean().optional(), }); export type SessionCloseParams = z.infer; diff --git a/src/tools/definitions.ts b/src/tools/definitions.ts index cfdcbce..a458cc9 100644 --- a/src/tools/definitions.ts +++ b/src/tools/definitions.ts @@ -205,6 +205,10 @@ export const TOOLS = [ type: "number", description: "End-to-end ceremony duration from agent perspective (in milliseconds)", }, + reclose: { + type: "boolean", + description: "Replace the close of a session that is already closed. Without it, closing a closed session is refused.", + }, }, required: ["session_id", "close_type"], }, diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index a740096..a44bf64 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -44,7 +44,7 @@ import * as fs from "fs"; import * as path from "path"; import * as os from "os"; import { getGitmemPath, getGitmemDir, getSessionPath, getSessionDir } from "../services/gitmem-dir.js"; -import { unregisterSession, findSessionByHostPid, findSessionById } from "../services/active-sessions.js"; +import { unregisterSession, findSessionByHostPid, findSessionById, listActiveSessions } from "../services/active-sessions.js"; import { loadSuggestions, saveSuggestions, detectSuggestedThreads, loadRecentSessionEmbeddings } from "../services/thread-suggestions.js"; import { writeAgentBriefing } from "../services/agent-briefing.js"; import type { @@ -244,6 +244,12 @@ async function sessionCloseFree( try { // Load existing session if available const existingSession = await storage.get>("sessions", sessionId); + // GIT-122: same rule as the Pro path — no silent second close. + if (existingSession?.close_compliance && !params.reclose) { + return refuseClose(params, sessionId, + `Session ${sessionId.slice(0, 8)} is already closed. Nothing was written. ` + + `To replace its close, call again with reclose: true.`, timer); + } const sessionData: Record = { ...(existingSession || {}), @@ -386,7 +392,7 @@ function formatCloseDisplay( if (stats.length > 0) { lines.push(stats.join(" · ")); } - lines.push(dimText(`${sessionId.slice(0, 8)} · ${compliance.agent} · ${compliance.close_type}`)); + lines.push(dimText(`${sessionId} · ${compliance.agent} · ${compliance.close_type}`)); // Errors — only on failure if (!success && errors?.length) { @@ -530,6 +536,41 @@ function isValidSessionId(id: string): boolean { return UUID_REGEX.test(id) || SHORT_ID_REGEX.test(id); } +/** + * GIT-122: does the id that was written match the id the caller asked for? + * The short form (8 hex) names the session whose full id starts with it. + */ +function sessionIdsMatch(requested: string, resolved: string): boolean { + const a = requested.toLowerCase(); + const b = resolved.toLowerCase(); + return a === b || (SHORT_ID_REGEX.test(a) && b.startsWith(a)) || (SHORT_ID_REGEX.test(b) && a.startsWith(b)); +} + +/** GIT-122: a close that is refused before anything is written. */ +function refuseClose( + params: SessionCloseParams, + sessionId: string, + error: string, + timer: Timer, +): SessionCloseResult { + console.error(`[session_close] REFUSED: ${error}`); + return { + ...notStored(), + session_id: sessionId, + close_compliance: { + close_type: params.close_type, + agent: "Unknown", + checklist_displayed: false, + questions_answered_by_agent: false, + human_asked_for_corrections: false, + learnings_stored: 0, + scars_applied: 0, + }, + validation_errors: [error], + performance: buildPerformanceData("session_close", timer.stop(), 0), + }; +} + /** * Build the session data record from params and existing session state. * Handles retroactive vs normal mode, reflection, decisions, threads, @@ -910,6 +951,12 @@ export async function sessionClose( const timer = new Timer(); const metricsId = uuidv4(); + // GIT-122: the id the caller asked for, before any merge or recovery can + // touch params. An explicit id is either the session that gets closed or the + // call is refused. Logged raw so the next occurrence is diagnosable. + const requestedId: string | undefined = params.session_id || undefined; + console.error(`[session_close] incoming session_id=${requestedId ?? "(none)"}`); + // Validate session_id format before any DB calls if (params.session_id && !isValidSessionId(params.session_id)) { const latencyMs = timer.stop(); @@ -935,6 +982,25 @@ 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") { + // GIT-122 (GIT-103 rule 3): with no id and more than one live session on + // this machine, "the current session" is a guess. Guessing closed the wrong + // row on 2026-10-04. Refuse and name the candidates. + try { + const live = listActiveSessions(); + if (live.length > 1) { + const ids = live.map((x) => x.session_id.slice(0, 8)).join(", "); + return refuseClose( + params, + "", + `No session_id provided and ${live.length} sessions are live on this machine (${ids}). ` + + `Pass session_id explicitly so the close lands on the session you mean.`, + timer, + ); + } + } catch (error) { + console.warn("[session_close] Failed to list live sessions:", error); + } + // 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. @@ -980,10 +1046,33 @@ export async function sessionClose( }; } - // 0a. File-based payload handoff: if .gitmem/closing-payload.json exists, - // merge it with inline params (inline params take precedence). - // This keeps the visible MCP tool call small: just session_id + close_type. - const payloadPath = getGitmemPath("closing-payload.json"); + // 0a. File-based payload handoff: merge the closing payload with inline + // params (inline params take precedence). This keeps the visible MCP tool + // call small: just session_id + close_type. + // + // GIT-122: the payload lives in the closing session's own folder, + // /sessions//closing-payload.json. The old single file at the + // .gitmem root is shared by every session on the machine, so one session's + // close merged and deleted another's payload. The root file is still read for + // one release, and only when exactly one session is live, so nobody is + // handed a payload that might belong to someone else. + const legacyPayloadPath = getGitmemPath("closing-payload.json"); + const scopedPayloadPath = params.session_id + ? path.join(getGitmemDir(), "sessions", params.session_id, "closing-payload.json") + : null; + let payloadPath = scopedPayloadPath ?? legacyPayloadPath; + let legacyPayloadSkipped = false; + if (scopedPayloadPath && !fs.existsSync(scopedPayloadPath) && fs.existsSync(legacyPayloadPath)) { + let liveCount = 0; + try { liveCount = listActiveSessions().length; } catch { /* registry unreadable: treat as unknown */ } + if (liveCount <= 1) { + payloadPath = legacyPayloadPath; + console.error(`[session_close] Using legacy ${legacyPayloadPath}; write payloads to ${scopedPayloadPath}`); + } else { + legacyPayloadSkipped = true; + console.error(`[session_close] Ignoring legacy ${legacyPayloadPath}: ${liveCount} sessions are live and it carries no owner`); + } + } let payloadConsumed = false; let payloadReadError: string | null = null; // Whether the session row reached the durable store. Set only after the @@ -993,6 +1082,17 @@ export async function sessionClose( try { if (fs.existsSync(payloadPath)) { const filePayload = JSON.parse(fs.readFileSync(payloadPath, "utf-8")) as Partial; + // GIT-122: the payload must not carry another session's identity. + if (filePayload.session_id && params.session_id && !sessionIdsMatch(filePayload.session_id, params.session_id)) { + return refuseClose( + params, + params.session_id, + `${path.resolve(payloadPath)} belongs to session ${filePayload.session_id.slice(0, 8)}, ` + + `but this call closes ${params.session_id.slice(0, 8)}. Nothing was written. ` + + `Move the payload into the right session's folder or remove it.`, + timer, + ); + } // File provides defaults; inline params override params = { ...filePayload, ...params }; payloadConsumed = true; @@ -1028,7 +1128,10 @@ export async function sessionClose( validation_errors: [ payloadReadError ?? `closing-payload.json not found at ${path.resolve(payloadPath)}. ` + - `Write the closing payload to exactly that path (or pass closing_reflection inline), then call session_close again.`, + `Write the closing payload to exactly that path (or pass closing_reflection inline), then call session_close again.` + + (legacyPayloadSkipped + ? ` A payload at ${path.resolve(legacyPayloadPath)} was ignored because several sessions are live and that file has no owner.` + : ""), ], performance: buildPerformanceData("session_close", timer.stop(), 0), }; @@ -1401,6 +1504,29 @@ export async function sessionClose( performance: perfData, }; } + + // GIT-122: the invariant. Refuse, before thread sync or any write, if the + // session about to be written is not the one that was asked for, or the row + // found is some other row. + if (requestedId && !sessionIdsMatch(requestedId, sessionId)) { + return refuseClose(params, sessionId, + `Requested session ${requestedId} but the close resolved to ${sessionId}. Nothing was written.`, timer); + } + const foundId = existingSession.id; + if (typeof foundId === "string" && foundId !== sessionId) { + return refuseClose(params, sessionId, + `Session lookup for ${sessionId} returned the record of ${foundId}. Nothing was written.`, timer); + } + + // GIT-122: a session that already has a close is not closed again by + // accident. 2026-10-04 overwrote ed6b400e's close (scars_applied 3 → 6). + // A close that only partly persisted (threads not synced) stays retryable. + const priorClose = existingSession.close_compliance as Record | null | undefined; + if (priorClose && !priorClose.partial_persist && !params.reclose) { + return refuseClose(params, sessionId, + `Session ${sessionId.slice(0, 8)} is already closed. Nothing was written. ` + + `To replace its close, call again with reclose: true.`, timer); + } } // 5. Build session data (merge with existing or create from scratch) @@ -1454,6 +1580,10 @@ export async function sessionClose( } } + // GIT-122: a close whose threads did not all land is retryable; mark the row + // so the already-closed refusal lets the retry through. + if (!threadSync.all_synced) closeCompliance.partial_persist = true; + // Prune threads.json: only keep open threads — ONLY if the durable store // agrees. A partial sync means the local file is the only record of the // threads that did not land; discarding it would destroy them. @@ -1575,6 +1705,14 @@ export async function sessionClose( // provisioned from setup.sql they used to fail the entire close with PGRST204. const sessionsTable = getTableName("sessions"); const sessionRow = await filterToStoreSessionColumns(sessionData, sessionsTable); + // GIT-122: assert on the row id about to be written, not on a promise that + // resolves. COMPLETE for the wrong row is the failure this guards. + // Retroactive closes write a new row under a fresh id on purpose. + if (sessionRow.id !== sessionId || (!isRetroactive && requestedId && !sessionIdsMatch(requestedId, String(sessionRow.id)))) { + throw new Error( + `refusing to write: row id ${String(sessionRow.id)} does not match the session being closed (${sessionId}, requested ${requestedId ?? "none"})` + ); + } await Promise.all([ supabase.directUpsert(sessionsTable, sessionRow), blindspotPromise, diff --git a/src/types/index.ts b/src/types/index.ts index 7f9a12a..a9c5338 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -288,6 +288,8 @@ export interface SessionCloseParams { capture_transcript?: boolean; /** Explicit transcript file path (overrides automatic detection) */ transcript_path?: string; + /** GIT-122: replace an existing close. Without it, closing a closed session is refused. */ + reclose?: boolean; } export interface CloseCompliance { @@ -300,6 +302,8 @@ export interface CloseCompliance { scars_applied: number; ceremony_duration_ms?: number; // Optional: end-to-end ceremony duration retroactive?: boolean; // Optional: marks sessions created post-mortem + /** GIT-122: the session saved but some threads did not sync; the close may be retried. */ + partial_persist?: boolean; } /** diff --git a/tests/unit/tools/session-close-id-invariant.test.ts b/tests/unit/tools/session-close-id-invariant.test.ts new file mode 100644 index 0000000..925cb35 --- /dev/null +++ b/tests/unit/tools/session-close-id-invariant.test.ts @@ -0,0 +1,338 @@ +/** + * GIT-122: an explicit session_id is either the session that gets closed, or + * the call is refused. A close never lands on a different session's row, never + * silently overwrites an already-closed session, and never merges another + * session's closing-payload.json. + * + * Incident 2026-10-04: Brain closed 279cb65a through a process bound to CLI's + * already-closed ed6b400e. COMPLETE was reported, ed6b400e's close_compliance was + * overwritten, and 279cb65a stayed open. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +const root = vi.hoisted(() => ({ dir: "" })); +const live = vi.hoisted(() => ({ list: (..._a: unknown[]): unknown[] => [] })); +const store = vi.hoisted(() => ({ rows: {} as Record>, upserts: [] as Record[] })); +const bound = vi.hoisted(() => ({ id: null as string | null })); + +// Mock dependencies before importing session-close +vi.mock("../../../src/services/agent-detection.js", () => ({ + detectAgent: () => ({ agent: "CLI", entrypoint: "cli", docker: true, hostname: "test" }), +})); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + listRecords: vi.fn().mockResolvedValue([]), + getRecord: vi.fn().mockImplementation(async (_t: string, id: string) => store.rows[id] ?? null), + directUpsert: vi.fn().mockImplementation(async (_t: string, row: Record) => { store.upserts.push(row); }), + directPatch: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../../src/services/embedding.js", () => ({ + embed: vi.fn().mockResolvedValue(null), + isEmbeddingAvailable: () => false, +})); + +vi.mock("../../../src/services/tier.js", () => ({ + hasSupabase: () => true, + hasBatchOperations: () => false, + hasTranscripts: () => false, + hasCacheManagement: () => false, + getTableName: (base: string) => `orchestra_${base}`, + hasProInsights: () => false, +})); + +vi.mock("../../../src/services/analytics.js", () => ({ + queryScarUsageByDateRange: vi.fn().mockResolvedValue([]), + enrichScarUsageTitles: vi.fn().mockResolvedValue([]), + formatBlindspotSnippet: vi.fn().mockReturnValue(null), + querySessionsByDateRange: vi.fn().mockResolvedValue([]), + computeLightweightSummary: vi.fn().mockReturnValue(null), +})); + +vi.mock("../../../src/services/storage.js", () => ({ + getStorage: () => ({ + get: vi.fn().mockResolvedValue(null), + upsert: vi.fn().mockResolvedValue(undefined), + }), +})); + +vi.mock("../../../src/services/session-state.js", () => ({ + clearCurrentSession: vi.fn(), + getSurfacedScars: () => [], + getObservations: () => [], + getChildren: () => [], + getThreads: () => [], + getSessionActivity: () => null, + isRecallCalled: () => true, + getConfirmations: () => [], + resolveCurrentSession: () => (bound.id ? { sessionId: bound.id, surfacedScars: [], recallCalled: true } : null), +})); + +vi.mock("../../../src/services/thread-manager.js", () => ({ + normalizeThreads: vi.fn().mockReturnValue([]), + mergeThreadStates: vi.fn().mockReturnValue([]), + migrateStringThread: vi.fn().mockReturnValue({ id: "t-test", text: "test", status: "open", created_at: new Date().toISOString() }), + saveThreadsFile: vi.fn(), +})); + +vi.mock("../../../src/services/thread-dedup.js", () => ({ + deduplicateThreadList: vi.fn().mockImplementation((threads) => threads), +})); + +vi.mock("../../../src/services/thread-supabase.js", () => ({ + syncThreadsToSupabase: vi.fn().mockResolvedValue(undefined), + loadOpenThreadEmbeddings: vi.fn().mockResolvedValue([]), +})); + +vi.mock("../../../src/services/compliance-validator.js", () => ({ + validateSessionClose: () => ({ valid: true, errors: [], warnings: [] }), + buildCloseCompliance: vi.fn().mockReturnValue({ + close_type: "quick", + agent: "CLI", + checklist_displayed: true, + questions_answered_by_agent: false, + human_asked_for_corrections: false, + learnings_stored: 0, + scars_applied: 0, + }), +})); + +vi.mock("../../../src/services/metrics.js", () => ({ + Timer: class { stop() { return 100; } }, + recordMetrics: vi.fn().mockResolvedValue(undefined), + buildPerformanceData: (name: string, latency: number, count: number) => ({ + latency_ms: latency, + target_ms: 3000, + meets_target: latency < 3000, + result_count: count, + }), + updateRelevanceData: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../../src/tools/record-scar-usage-batch.js", () => ({ + recordScarUsageBatch: vi.fn().mockResolvedValue({ success: true }), +})); + +vi.mock("../../../src/services/effect-tracker.js", () => ({ + getEffectTracker: () => ({ + track: vi.fn(), + formatSummary: () => "No tracked effects this session.", + getHealthReport: () => ({ overall: { attempted: 0, succeeded: 0, failed: 0, successRate: "N/A", paths_with_failures: [] }, byPath: {}, recentFailures: [] }), + }), +})); + +vi.mock("../../../src/tools/save-transcript.js", () => ({ + saveTranscript: vi.fn().mockResolvedValue({ success: false }), +})); + +vi.mock("../../../src/services/transcript-chunker.js", () => ({ + processTranscript: vi.fn().mockResolvedValue({ success: false }), +})); + +vi.mock("../../../src/services/gitmem-dir.js", () => ({ + getGitmemPath: (filename: string) => `${root.dir}/${filename}`, + getGitmemDir: () => root.dir, + getSessionPath: (sid: string, filename: string) => `${root.dir}/sessions/${sid}/${filename}`, + getSessionDir: (sid: string) => `${root.dir}/sessions/${sid}`, +})); + +vi.mock("../../../src/services/active-sessions.js", () => ({ + unregisterSession: vi.fn(), + findSessionByHostPid: vi.fn().mockReturnValue(null), + findSessionById: vi.fn().mockReturnValue(null), // GIT-86: session-close's registry fallback looks up by id + listActiveSessions: (...a: unknown[]) => live.list(...a), +})); + +vi.mock("../../../src/services/thread-suggestions.js", () => ({ + loadSuggestions: vi.fn().mockReturnValue([]), + saveSuggestions: vi.fn(), + detectSuggestedThreads: vi.fn().mockReturnValue([]), + loadRecentSessionEmbeddings: vi.fn().mockResolvedValue(null), +})); + +import { sessionClose } from "../../../src/tools/session-close.js"; + +const A = "aaaaaaaa-1111-4111-8111-aaaaaaaaaaaa"; +const B = "bbbbbbbb-2222-4222-8222-bbbbbbbbbbbb"; +const REFLECTION = { what_broke: "x", what_took_longer: "x", do_differently: "x", what_worked: "y", wrong_assumption: "x", scars_applied: [] }; +const reflection = (tag: string) => ({ ...REFLECTION, what_worked: tag }); +const sessionDir = (id: string) => path.join(root.dir, "sessions", id); +const writePayload = (file: string, body: unknown) => { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, JSON.stringify(body)); +}; +const open = (id: string) => ({ id, agent: "cli", project: "gitmem", session_date: "2026-10-04" }); +const closed = (id: string) => ({ ...open(id), close_compliance: { close_type: "standard", agent: "cli" } }); +const written = () => store.upserts.map((r) => r.id); + +beforeEach(() => { + vi.clearAllMocks(); + root.dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git122-")); + store.rows = {}; + store.upserts = []; + bound.id = null; + live.list = () => []; +}); + +afterEach(() => { + fs.rmSync(root.dir, { recursive: true, force: true }); +}); + +describe("session_close identity invariant (GIT-122)", () => { + it("explicit id A + payload file carrying B's identity: refused, nothing written", async () => { + store.rows[A] = open(A); + store.rows[B] = open(B); + bound.id = B; + writePayload(path.join(root.dir, "closing-payload.json"), { session_id: B, closing_reflection: reflection("B-reflection") }); + + const result = await sessionClose({ session_id: A, close_type: "standard" }); + + expect(result.success).toBe(false); + expect(result.validation_errors!.join(" ")).toContain(A.slice(0, 8)); + expect(result.validation_errors!.join(" ")).toContain(B.slice(0, 8)); + expect(written()).toEqual([]); + }); + + it("per-session payload carrying another session's id: refused, nothing written", async () => { + store.rows[A] = open(A); + writePayload(path.join(sessionDir(A), "closing-payload.json"), { session_id: B, closing_reflection: reflection("B-reflection") }); + + const result = await sessionClose({ session_id: A, close_type: "standard", closing_reflection: reflection("A-inline") }); + + expect(result.success).toBe(false); + expect(written()).toEqual([]); + }); + + it("explicit id of an already-closed session: refused, row untouched", async () => { + store.rows[A] = closed(A); + + const result = await sessionClose({ session_id: A, close_type: "quick" }); + + expect(result.success).toBe(false); + expect(result.validation_errors!.join(" ")).toMatch(/already closed/i); + expect(result.validation_errors!.join(" ")).toContain("reclose"); + expect(written()).toEqual([]); + }); + + it("reclose: true is the explicit way to close it again, and lands on that row", async () => { + store.rows[A] = closed(A); + + const result = await sessionClose({ session_id: A, close_type: "quick", reclose: true }); + + expect(written()).toEqual([A]); + expect(result.session_id).toBe(A); + }); + + it("a partial-persist close can be retried without reclose", async () => { + store.rows[A] = { ...open(A), close_compliance: { close_type: "standard", agent: "cli", partial_persist: true } }; + + await sessionClose({ session_id: A, close_type: "quick" }); + + expect(written()).toEqual([A]); + }); + + it("no id with two live sessions: refused, names both, writes nothing", async () => { + store.rows[A] = open(A); + store.rows[B] = open(B); + bound.id = B; + live.list = () => [{ session_id: A }, { session_id: B }]; + + const result = await sessionClose({ close_type: "quick" }); + + expect(result.success).toBe(false); + expect(result.validation_errors!.join(" ")).toContain(A.slice(0, 8)); + expect(result.validation_errors!.join(" ")).toContain(B.slice(0, 8)); + expect(written()).toEqual([]); + }); + + it("no id with exactly one live session still recovers it", async () => { + store.rows[B] = open(B); + bound.id = B; + live.list = () => [{ session_id: B }]; + + await sessionClose({ close_type: "quick" }); + + expect(written()).toEqual([B]); + }); + + it("explicit id A closes A even when the process is bound to B", async () => { + store.rows[A] = open(A); + store.rows[B] = open(B); + bound.id = B; + + const result = await sessionClose({ session_id: A, close_type: "quick" }); + + expect(written()).toEqual([A]); + expect(result.session_id).toBe(A); + }); + + it("result and display show the full id actually written", async () => { + store.rows[A] = open(A); + + const result = await sessionClose({ session_id: A, close_type: "quick" }); + + expect(result.session_id).toBe(A); + expect(result.display).toContain(A); + }); + + it("logs the raw incoming session_id to stderr on entry", async () => { + const err = vi.spyOn(console, "error").mockImplementation(() => {}); + store.rows[A] = open(A); + + await sessionClose({ session_id: A, close_type: "quick" }); + + expect(err.mock.calls.map((c) => String(c[0]))).toContain(`[session_close] incoming session_id=${A}`); + err.mockRestore(); + }); +}); + +describe("closing payload is scoped per session (GIT-122)", () => { + it("reads //closing-payload.json", async () => { + store.rows[A] = open(A); + writePayload(path.join(sessionDir(A), "closing-payload.json"), { closing_reflection: reflection("A-reflection") }); + + await sessionClose({ session_id: A, close_type: "standard" }); + + expect(written()).toEqual([A]); + expect(JSON.stringify(store.upserts[0])).toContain("A-reflection"); + }); + + it("B's payload is untouched and B stays open when A closes", async () => { + store.rows[A] = open(A); + store.rows[B] = open(B); + writePayload(path.join(sessionDir(A), "closing-payload.json"), { closing_reflection: reflection("A-reflection") }); + writePayload(path.join(sessionDir(B), "closing-payload.json"), { closing_reflection: reflection("B-reflection") }); + + await sessionClose({ session_id: A, close_type: "standard" }); + + expect(written()).toEqual([A]); + expect(JSON.stringify(store.upserts[0])).not.toContain("B-reflection"); + expect(fs.existsSync(path.join(sessionDir(B), "closing-payload.json"))).toBe(true); + }); + + it("falls back to the legacy root file when exactly one session is live", async () => { + store.rows[A] = open(A); + live.list = () => [{ session_id: A }]; + writePayload(path.join(root.dir, "closing-payload.json"), { closing_reflection: reflection("legacy-reflection") }); + + await sessionClose({ session_id: A, close_type: "standard" }); + + expect(JSON.stringify(store.upserts[0])).toContain("legacy-reflection"); + }); + + it("ignores the legacy root file when several sessions are live", async () => { + store.rows[A] = open(A); + live.list = () => [{ session_id: A }, { session_id: B }]; + writePayload(path.join(root.dir, "closing-payload.json"), { closing_reflection: reflection("legacy-reflection") }); + + const result = await sessionClose({ session_id: A, close_type: "standard" }); + + expect(result.success).toBe(false); + expect(written()).toEqual([]); + expect(fs.existsSync(path.join(root.dir, "closing-payload.json"))).toBe(true); + }); +}); diff --git a/tests/unit/tools/session-close-payload-path.test.ts b/tests/unit/tools/session-close-payload-path.test.ts index 0f89d50..7c8f5d9 100644 --- a/tests/unit/tools/session-close-payload-path.test.ts +++ b/tests/unit/tools/session-close-payload-path.test.ts @@ -149,7 +149,8 @@ vi.mock("../../../src/services/thread-suggestions.js", () => ({ import { sessionClose } from "../../../src/tools/session-close.js"; const SID = "393adb34-a80c-4c3a-b71a-bc0053b7a7ea"; -const payloadFile = () => path.join(root.dir, "closing-payload.json"); +// GIT-122: the payload lives in the closing session's own folder. +const payloadFile = () => path.join(root.dir, "sessions", SID, "closing-payload.json"); beforeEach(() => { vi.clearAllMocks(); @@ -175,6 +176,7 @@ describe("session_close missing payload (GIT-99)", () => { }); it("an unparseable payload is reported as found-but-unreadable, at its path", async () => { + fs.mkdirSync(path.dirname(payloadFile()), { recursive: true }); fs.writeFileSync(payloadFile(), "{ not json"); const result = await sessionClose({ session_id: SID, close_type: "standard" }); @@ -192,6 +194,7 @@ describe("session_close missing payload (GIT-99)", () => { }); it("a payload at the path is consumed", async () => { + fs.mkdirSync(path.dirname(payloadFile()), { recursive: true }); fs.writeFileSync(payloadFile(), JSON.stringify({ closing_reflection: { what_broke: "x", what_worked: "y", scars_applied: [] } })); const result = await sessionClose({ session_id: SID, close_type: "standard" }); expect((result.validation_errors || []).join(" ")).not.toContain("closing-payload.json"); From 06baf49b58457f70f64d7956f520d2c09c6135cd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 14:10:21 -0500 Subject: [PATCH 2/4] feat: gitmem_session_id alias for session_close, for clients that strip session_id (GIT-122) Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 11 ++++ src/schemas/session-close.ts | 3 + src/tools/definitions.ts | 6 +- src/tools/session-close.ts | 19 +++++++ src/types/index.ts | 2 + .../tools/session-close-id-invariant.test.ts | 56 +++++++++++++++++++ 6 files changed, 96 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4018ecb..ea16f3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `session_close` now closes the session you name, or refuses (GIT-122). On 2026-10-04 a close that + named one session landed on another, already-closed one and overwrote its close. The Claude + remote-devices bridge strips a parameter called `session_id`, so the server received no id and + silently fell back to its own bound session. Now: `session_close` accepts the same id as + `gitmem_session_id`, a name a proxy will not claim (if both are given and differ, it refuses); + closing a session that is already closed is refused unless you pass `reclose: true`; a call with no + id is refused while several sessions are live; a resolved id that differs from the requested one is + refused before anything is written; the result and display show the full id actually written; and + the incoming id is logged to stderr. `closing-payload.json` now lives in the session's own folder + (`sessions//closing-payload.json`) and is refused if it names another session; the old shared + file at the store root is still read for one release, and only when a single session is live. - Running the e2e suite no longer deletes your real `~/.gitmem`. `cursor-cross-tool.test.ts` ran `gitmem uninstall --all` with your real HOME, and `--all` removes the memory store (session registry, per-session files, local `threads.json`, closing payloads). Those tests now run against diff --git a/src/schemas/session-close.ts b/src/schemas/session-close.ts index 9641fd6..189296a 100644 --- a/src/schemas/session-close.ts +++ b/src/schemas/session-close.ts @@ -94,6 +94,9 @@ export const SessionCloseParamsSchema = z.object({ // could never execute. The agent had to already know the id, which is exactly // what a restart (and context compaction) takes away. session_id: SessionIdSchema.optional(), + // GIT-122: same id under a name a proxy will not claim (the remote-devices + // bridge strips `session_id`). Both given and different is refused at close. + gitmem_session_id: SessionIdSchema.optional(), close_type: CloseTypeSchema, task_completion: TaskCompletionSchema.optional(), closing_reflection: ClosingReflectionSchema.optional(), diff --git a/src/tools/definitions.ts b/src/tools/definitions.ts index a458cc9..51c256c 100644 --- a/src/tools/definitions.ts +++ b/src/tools/definitions.ts @@ -184,6 +184,10 @@ export const TOOLS = [ type: "string", description: "Session ID from session_start", }, + gitmem_session_id: { + type: "string", + description: "Same as session_id — prefer this one. Some MCP clients and proxies strip a parameter named session_id, so the close would not know which session you mean. If both are given and differ, the close is refused.", + }, close_type: { type: "string", enum: ["standard", "quick", "autonomous"], @@ -210,7 +214,7 @@ export const TOOLS = [ description: "Replace the close of a session that is already closed. Without it, closing a closed session is refused.", }, }, - required: ["session_id", "close_type"], + required: ["close_type"], }, }, { diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index a44bf64..1d4b29e 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -954,6 +954,25 @@ export async function sessionClose( // GIT-122: the id the caller asked for, before any merge or recovery can // touch params. An explicit id is either the session that gets closed or the // call is refused. Logged raw so the next occurrence is diagnosable. + // + // `gitmem_session_id` is the same id under a name a proxy will not claim: the + // Claude remote-devices bridge strips a parameter called `session_id`, so the + // 2026-10-04 close arrived with no id at all. If both are given and differ, + // refuse. + if (params.gitmem_session_id) { + if (params.session_id && !sessionIdsMatch(params.session_id, params.gitmem_session_id)) { + console.error(`[session_close] incoming session_id=${params.session_id} gitmem_session_id=${params.gitmem_session_id}`); + return refuseClose( + params, + "", + `session_id ${params.session_id} and gitmem_session_id ${params.gitmem_session_id} name different sessions. ` + + `Pass one of them, or the same id in both. Nothing was written.`, + timer, + ); + } + const { gitmem_session_id: alias, ...rest } = params; + params = { ...rest, session_id: params.session_id || alias }; + } const requestedId: string | undefined = params.session_id || undefined; console.error(`[session_close] incoming session_id=${requestedId ?? "(none)"}`); diff --git a/src/types/index.ts b/src/types/index.ts index a9c5338..ffff71e 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -271,6 +271,8 @@ export interface SessionDecision { export interface SessionCloseParams { session_id: string; + /** GIT-122: alias of session_id for clients/proxies that strip a parameter named session_id. */ + gitmem_session_id?: string; close_type: CloseType; /** Task completion proof - REQUIRED for standard close */ task_completion?: TaskCompletion; diff --git a/tests/unit/tools/session-close-id-invariant.test.ts b/tests/unit/tools/session-close-id-invariant.test.ts index 925cb35..eaf8b97 100644 --- a/tests/unit/tools/session-close-id-invariant.test.ts +++ b/tests/unit/tools/session-close-id-invariant.test.ts @@ -336,3 +336,59 @@ describe("closing payload is scoped per session (GIT-122)", () => { expect(fs.existsSync(path.join(root.dir, "closing-payload.json"))).toBe(true); }); }); + +describe("gitmem_session_id alias (GIT-122)", () => { + // The remote-devices bridge drops a parameter named `session_id`; the alias carries the same id. + it("alias alone closes that session, even when the process is bound to another", async () => { + store.rows[A] = open(A); + store.rows[B] = open(B); + bound.id = B; + live.list = () => [{ session_id: B }]; + + const result = await sessionClose({ gitmem_session_id: A, close_type: "quick" }); + + expect(written()).toEqual([A]); + expect(result.session_id).toBe(A); + }); + + it("both given and equal: closes it", async () => { + store.rows[A] = open(A); + await sessionClose({ session_id: A, gitmem_session_id: A, close_type: "quick" }); + expect(written()).toEqual([A]); + }); + + it("both given and different: refused, nothing written, both ids named", async () => { + store.rows[A] = open(A); + store.rows[B] = open(B); + + const result = await sessionClose({ session_id: A, gitmem_session_id: B, close_type: "quick" }); + + expect(result.success).toBe(false); + expect(result.validation_errors!.join(" ")).toContain(A.slice(0, 8)); + expect(result.validation_errors!.join(" ")).toContain(B.slice(0, 8)); + expect(written()).toEqual([]); + }); + + it("alias is subject to the same checks: a closed session is refused", async () => { + store.rows[A] = closed(A); + const result = await sessionClose({ gitmem_session_id: A, close_type: "quick" }); + expect(result.success).toBe(false); + expect(written()).toEqual([]); + }); + + it("a malformed alias is rejected, not ignored", async () => { + store.rows[B] = open(B); + bound.id = B; + const result = await sessionClose({ gitmem_session_id: "../../etc/passwd", close_type: "quick" }); + expect(result.success).toBe(false); + expect(written()).toEqual([]); + }); + + it("the alias id is logged on entry", async () => { + const err = vi.spyOn(console, "error").mockImplementation(() => {}); + store.rows[A] = open(A); + await sessionClose({ gitmem_session_id: A, close_type: "quick" }); + expect(err.mock.calls.map((c) => String(c[0]))).toContain(`[session_close] incoming session_id=${A}`); + err.mockRestore(); + }); +}); From 640a9fd376a0835a1901deacec8bd95ed6693013 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 15:56:49 -0500 Subject: [PATCH 3/4] test: venue driver expects the per-session payload path (GIT-122) Co-Authored-By: Claude Sonnet 5.5 --- tests/e2e/blank-supabase.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index f93f69c..567e310 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -385,7 +385,8 @@ async function flow() { // GIT-99: a standard close with no payload file and no inline reflection // names the absolute path this server reads, instead of "requires N answers". - const expectedPayloadPath = join(gitmemDir, "closing-payload.json"); + // GIT-122: the payload lives in the closing session's own folder. + const expectedPayloadPath = join(gitmemDir, "sessions", sessionId, "closing-payload.json"); const scNoPayload = await step("session_close", { session_id: sessionId, close_type: "standard" }); const missingPayloadNamed = scNoPayload.includes(`closing-payload.json not found at ${expectedPayloadPath}`) && !/requires closing_reflection|requires task_completion/.test(scNoPayload); From e17888bdccff864e04c1ac5a4d3e883c92c0f2cd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 16:09:45 -0500 Subject: [PATCH 4/4] fix: root closing-payload.json is read when it names the closing session; hook and templates say to include session_id (GIT-122) Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 6 +- CLAUDE.md.template | 1 + bin/init-wizard.js | 2 + copilot-instructions.template | 1 + cursorrules.template | 1 + hooks/scripts/session-close-check.sh | 2 +- hooks/tests/test-hooks.sh | 5 ++ src/tools/session-close.ts | 54 +++++++----- src/tools/session-start.ts | 3 + .../tools/session-close-id-invariant.test.ts | 82 +++++++++++++++++++ windsurfrules.template | 1 + 11 files changed, 133 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea16f3e..984b20d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,8 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 id is refused while several sessions are live; a resolved id that differs from the requested one is refused before anything is written; the result and display show the full id actually written; and the incoming id is logged to stderr. `closing-payload.json` now lives in the session's own folder - (`sessions//closing-payload.json`) and is refused if it names another session; the old shared - file at the store root is still read for one release, and only when a single session is live. + (`sessions//closing-payload.json`) and is refused if it names another session. The old shared + file at the store root is still read for one release when it carries a `session_id` naming the + session being closed (the Stop hook, `gitmem init` and the agent templates now tell agents to + include it), or, with no `session_id`, only while a single session is live. - Running the e2e suite no longer deletes your real `~/.gitmem`. `cursor-cross-tool.test.ts` ran `gitmem uninstall --all` with your real HOME, and `--all` removes the memory store (session registry, per-session files, local `threads.json`, closing payloads). Those tests now run against diff --git a/CLAUDE.md.template b/CLAUDE.md.template index b8a0422..17cbdab 100644 --- a/CLAUDE.md.template +++ b/CLAUDE.md.template @@ -67,6 +67,7 @@ On "closing", "done for now", or "wrapping up": Payload schema (`.gitmem/closing-payload.json`): ```json { + "session_id": "", "closing_reflection": { "what_broke": "...", "what_took_longer": "...", diff --git a/bin/init-wizard.js b/bin/init-wizard.js index e54f115..7a1534c 100644 --- a/bin/init-wizard.js +++ b/bin/init-wizard.js @@ -458,6 +458,7 @@ async function stepMemoryStore() { const payloadPath = join(storeDir, "closing-payload.json"); if (!existsSync(payloadPath)) { writeJson(payloadPath, { + session_id: "", closing_reflection: { what_broke: "", what_took_longer: "", @@ -525,6 +526,7 @@ async function stepMemoryStore() { const templatePath = join(storeDir, "closing-payload-template.json"); if (!existsSync(templatePath)) { writeJson(templatePath, { + session_id: "your session id from session_start (required)", closing_reflection: { what_broke: "", what_took_longer: "", do_differently: "", what_worked: "", wrong_assumption: "", scars_applied: [], diff --git a/copilot-instructions.template b/copilot-instructions.template index df40513..f824476 100644 --- a/copilot-instructions.template +++ b/copilot-instructions.template @@ -61,6 +61,7 @@ Safe alternatives: `env | grep -c VARNAME` (count only), `[ -n "$VARNAME" ] && e Payload schema (`.gitmem/closing-payload.json`): ```json { + "session_id": "", "closing_reflection": { "what_broke": "...", "what_took_longer": "...", diff --git a/cursorrules.template b/cursorrules.template index af6cb9c..f43e424 100644 --- a/cursorrules.template +++ b/cursorrules.template @@ -61,6 +61,7 @@ On "closing", "done for now", or "wrapping up": Payload schema (`.gitmem/closing-payload.json`): ```json { + "session_id": "", "closing_reflection": { "what_broke": "...", "what_took_longer": "...", diff --git a/hooks/scripts/session-close-check.sh b/hooks/scripts/session-close-check.sh index 0b25075..cd30276 100755 --- a/hooks/scripts/session-close-check.sh +++ b/hooks/scripts/session-close-check.sh @@ -98,7 +98,7 @@ if [ "$SESSION_STARTED" = "true" ]; then cat </dev/null && node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{JSON.parse(d)})' 2>/dev/null) \ && pass "Stop hook output with the path is valid JSON" \ || fail "Stop hook JSON" "parseable JSON" "$OUTPUT" diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index 1d4b29e..dcc355b 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -1070,11 +1070,15 @@ export async function sessionClose( // call small: just session_id + close_type. // // GIT-122: the payload lives in the closing session's own folder, - // /sessions//closing-payload.json. The old single file at the - // .gitmem root is shared by every session on the machine, so one session's - // close merged and deleted another's payload. The root file is still read for - // one release, and only when exactly one session is live, so nobody is - // handed a payload that might belong to someone else. + // /sessions//closing-payload.json, and that wins when present. + // The single file at the .gitmem root is shared by every session on the + // machine, so one session's close used to merge and delete another's payload. + // The Stop hook and the init templates still name the root file, so it is + // still read (for one release), under one rule: it must say whose it is. + // - it names this session (`session_id`) -> used, however many sessions are live; + // - it names another session -> refused (below); + // - it names nobody -> used only while a single session + // is live, otherwise ignored. const legacyPayloadPath = getGitmemPath("closing-payload.json"); const scopedPayloadPath = params.session_id ? path.join(getGitmemDir(), "sessions", params.session_id, "closing-payload.json") @@ -1082,15 +1086,7 @@ export async function sessionClose( let payloadPath = scopedPayloadPath ?? legacyPayloadPath; let legacyPayloadSkipped = false; if (scopedPayloadPath && !fs.existsSync(scopedPayloadPath) && fs.existsSync(legacyPayloadPath)) { - let liveCount = 0; - try { liveCount = listActiveSessions().length; } catch { /* registry unreadable: treat as unknown */ } - if (liveCount <= 1) { - payloadPath = legacyPayloadPath; - console.error(`[session_close] Using legacy ${legacyPayloadPath}; write payloads to ${scopedPayloadPath}`); - } else { - legacyPayloadSkipped = true; - console.error(`[session_close] Ignoring legacy ${legacyPayloadPath}: ${liveCount} sessions are live and it carries no owner`); - } + payloadPath = legacyPayloadPath; } let payloadConsumed = false; let payloadReadError: string | null = null; @@ -1112,12 +1108,26 @@ export async function sessionClose( timer, ); } - // File provides defaults; inline params override - params = { ...filePayload, ...params }; - payloadConsumed = true; - console.error(`[session_close] Loaded closing payload from ${payloadPath}`); - // Payload file is cleaned up AFTER successful close (see end of function). - // If the tool crashes, the payload survives for retry. + let ownerless = false; + if (payloadPath === legacyPayloadPath && scopedPayloadPath && !filePayload.session_id) { + let liveCount = 0; + try { liveCount = listActiveSessions().length; } catch { /* registry unreadable: treat as unknown */ } + ownerless = liveCount > 1; + if (ownerless) { + legacyPayloadSkipped = true; + console.error(`[session_close] Ignoring ${legacyPayloadPath}: ${liveCount} sessions are live and it names no session_id`); + } else { + console.error(`[session_close] Using legacy ${legacyPayloadPath}; write payloads to ${scopedPayloadPath}`); + } + } + if (!ownerless) { + // File provides defaults; inline params override + params = { ...filePayload, ...params }; + payloadConsumed = true; + console.error(`[session_close] Loaded closing payload from ${payloadPath}`); + // Payload file is cleaned up AFTER successful close (see end of function). + // If the tool crashes, the payload survives for retry. + } } } catch (error) { console.warn("[session_close] Failed to read closing-payload.json:", error); @@ -1146,10 +1156,10 @@ export async function sessionClose( }, validation_errors: [ payloadReadError ?? - `closing-payload.json not found at ${path.resolve(payloadPath)}. ` + + `closing-payload.json not found at ${path.resolve(legacyPayloadSkipped && scopedPayloadPath ? scopedPayloadPath : payloadPath)}. ` + `Write the closing payload to exactly that path (or pass closing_reflection inline), then call session_close again.` + (legacyPayloadSkipped - ? ` A payload at ${path.resolve(legacyPayloadPath)} was ignored because several sessions are live and that file has no owner.` + ? ` A payload at ${path.resolve(legacyPayloadPath)} was ignored because several sessions are live and it has no \"session_id\" field naming this session; add \"session_id\": \"${params.session_id}\" to it, or write it to the path above.` : ""), ], performance: buildPerformanceData("session_close", timer.stop(), 0), diff --git a/src/tools/session-start.ts b/src/tools/session-start.ts index a167a6c..0084e52 100644 --- a/src/tools/session-start.ts +++ b/src/tools/session-start.ts @@ -66,6 +66,9 @@ import type { * know the exact field names for closing-payload.json without guessing. */ const CLOSING_PAYLOAD_SCHEMA: Record = { + // GIT-122: name your session. A payload at the shared root is read only if it says whose it is + // (always, or when a single session is live); concurrent sessions would otherwise read each other's. + session_id: "this session's id, from session_start", closing_reflection: { what_broke: "Q1: What broke that you didn't expect?", what_took_longer: "Q2: What took longer than it should have?", diff --git a/tests/unit/tools/session-close-id-invariant.test.ts b/tests/unit/tools/session-close-id-invariant.test.ts index eaf8b97..12112db 100644 --- a/tests/unit/tools/session-close-id-invariant.test.ts +++ b/tests/unit/tools/session-close-id-invariant.test.ts @@ -13,6 +13,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import * as fs from "fs"; import * as os from "os"; import * as path from "path"; +import { execFileSync } from "child_process"; const root = vi.hoisted(() => ({ dir: "" })); const live = vi.hoisted(() => ({ list: (..._a: unknown[]): unknown[] => [] })); @@ -334,6 +335,87 @@ describe("closing payload is scoped per session (GIT-122)", () => { expect(result.success).toBe(false); expect(written()).toEqual([]); expect(fs.existsSync(path.join(root.dir, "closing-payload.json"))).toBe(true); + expect(result.validation_errors![0]).toContain(`closing-payload.json not found at ${path.join(sessionDir(A), "closing-payload.json")}`); + expect(result.validation_errors![0]).toMatch(/was ignored because several sessions are live/); + }); + + it("two live sessions, root payload naming this session: used, then removed", async () => { + store.rows[A] = open(A); + store.rows[B] = open(B); + live.list = () => [{ session_id: A }, { session_id: B }]; + writePayload(path.join(root.dir, "closing-payload.json"), { session_id: A, closing_reflection: reflection("root-A-reflection") }); + + const result = await sessionClose({ session_id: A, close_type: "standard" }); + + expect(written()).toEqual([A]); + expect(JSON.stringify(store.upserts[0])).toContain("root-A-reflection"); + expect(result.success).toBe(true); + expect(fs.existsSync(path.join(root.dir, "closing-payload.json"))).toBe(false); + }); + + it("two live sessions, root payload naming the other session: refused, file kept", async () => { + store.rows[A] = open(A); + store.rows[B] = open(B); + live.list = () => [{ session_id: A }, { session_id: B }]; + writePayload(path.join(root.dir, "closing-payload.json"), { session_id: B, closing_reflection: reflection("root-B-reflection") }); + + const result = await sessionClose({ session_id: A, close_type: "standard", closing_reflection: reflection("A-inline") }); + + expect(result.success).toBe(false); + expect(written()).toEqual([]); + expect(fs.existsSync(path.join(root.dir, "closing-payload.json"))).toBe(true); + }); + + it("the per-session payload still wins over a root payload", async () => { + store.rows[A] = open(A); + live.list = () => [{ session_id: A }, { session_id: B }]; + writePayload(path.join(sessionDir(A), "closing-payload.json"), { closing_reflection: reflection("scoped-reflection") }); + writePayload(path.join(root.dir, "closing-payload.json"), { session_id: A, closing_reflection: reflection("root-reflection") }); + + await sessionClose({ session_id: A, close_type: "standard" }); + + expect(JSON.stringify(store.upserts[0])).toContain("scoped-reflection"); + expect(JSON.stringify(store.upserts[0])).not.toContain("root-reflection"); + }); +}); + +describe("the Stop hook's instruction and the server's reader agree (GIT-122)", () => { + const HOOK = path.join(__dirname, "../../../hooks/scripts/session-close-check.sh"); + + it("following the hook's instruction with two live sessions closes the right one", async () => { + store.rows[A] = open(A); + store.rows[B] = open(B); + live.list = () => [{ session_id: A }, { session_id: B }]; + fs.writeFileSync(path.join(root.dir, "active-sessions.json"), JSON.stringify({ sessions: [{ session_id: A }, { session_id: B }] })); + + const hookSession = `git122-${process.pid}`; + const state = `/tmp/gitmem-hooks-${hookSession}`; + fs.mkdirSync(state, { recursive: true }); + fs.writeFileSync(path.join(state, "tool_call_count"), "10"); + fs.writeFileSync(path.join(state, "start_time"), String(Math.floor(Date.now() / 1000) - 600)); + let reason: string; + try { + const out = execFileSync("bash", [HOOK], { + input: "{}", + env: { PATH: process.env.PATH!, HOME: root.dir, GITMEM_DIR: root.dir, CLAUDE_SESSION_ID: hookSession }, + }).toString(); + reason = JSON.parse(out).reason; + } finally { + fs.rmSync(state, { recursive: true, force: true }); + } + + // What the hook tells the agent to do... + const target = /WRITE structured payload to (\S+) \(this exact absolute path/.exec(reason)![1]; + expect(reason).toContain('with "session_id" (YOUR session id from session_start'); + // ...done literally: the path it names, with this session's id in it. + writePayload(target, { session_id: A, closing_reflection: reflection("hook-instructed") }); + + // The server reads exactly that file. + const result = await sessionClose({ session_id: A, close_type: "standard" }); + + expect(written()).toEqual([A]); + expect(JSON.stringify(store.upserts[0])).toContain("hook-instructed"); + expect(result.success).toBe(true); }); }); diff --git a/windsurfrules.template b/windsurfrules.template index b77d643..7617f04 100644 --- a/windsurfrules.template +++ b/windsurfrules.template @@ -61,6 +61,7 @@ Safe alternatives: `env | grep -c VARNAME` (count only), `[ -n "$VARNAME" ] && e Payload schema (`.gitmem/closing-payload.json`): ```json { + "session_id": "", "closing_reflection": { "what_broke": "...", "what_took_longer": "...",