From 936cc4fabcab6bf30fb298eb61a50f3e3770d02a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:20:03 -0700 Subject: [PATCH 1/4] fix: hooks resolve .gitmem like the server, skip dead-pid sessions, print the payload path (GIT-99) Since GIT-91 the server resolves its root independently of cwd, but the hooks still read ".gitmem/..." relative to theirs. A hook running in a repo read a different store from the one the server writes: no session, or someone else's, and a closing-payload.json the server never reads. - hooks/scripts/resolve-root.sh: GITMEM_DIR, then GITMEM_HOME/.gitmem, then ~/.gitmem, made absolute. Sourced by recall-check, post-tool-use and session-close-check. - Registry entries on this host whose pid is dead are skipped; entries without a pid or from another host are kept. recall-check takes the newest live session instead of sessions[0]. - The Stop hook prints the resolved absolute payload path, JSON-escaped. - Fixes a pre-existing break: without jq, session-close-check's node fallback was brace-expanded by bash into two broken node calls, so the Stop hook never blocked (3 of 32 hook tests red on main on a jq-less machine). test-hooks.sh: the existing tests relied on cwd resolution (cd $TMPDIR finding $TMPDIR/.gitmem); they now export GITMEM_DIR to the same store. +12 tests (44/44; main 31/44). Co-Authored-By: Claude Opus 5 --- hooks/scripts/post-tool-use.sh | 5 +- hooks/scripts/recall-check.sh | 17 ++--- hooks/scripts/resolve-root.sh | 81 +++++++++++++++++++++++ hooks/scripts/session-close-check.sh | 19 +++--- hooks/tests/test-hooks.sh | 96 ++++++++++++++++++++++++++++ 5 files changed, 195 insertions(+), 23 deletions(-) create mode 100755 hooks/scripts/resolve-root.sh diff --git a/hooks/scripts/post-tool-use.sh b/hooks/scripts/post-tool-use.sh index ae6c45a..86fb705 100755 --- a/hooks/scripts/post-tool-use.sh +++ b/hooks/scripts/post-tool-use.sh @@ -22,8 +22,9 @@ HOOK_INPUT=$(cat -) # ============================================================================ # Check active sessions registry -ACTIVE_SESSIONS=".gitmem/active-sessions.json" -if [ ! -f "$ACTIVE_SESSIONS" ]; then +# GIT-99: resolve .gitmem the way the server does, not from cwd. +. "$(dirname "${BASH_SOURCE[0]}")/resolve-root.sh" +if [ -z "$(gitmem_live_session_ids)" ]; then exit 0 fi diff --git a/hooks/scripts/recall-check.sh b/hooks/scripts/recall-check.sh index 1750ae1..71a9e82 100755 --- a/hooks/scripts/recall-check.sh +++ b/hooks/scripts/recall-check.sh @@ -31,20 +31,17 @@ HOOK_INPUT=$(cat -) # Resolve active session from registry # ============================================================================ -ACTIVE_SESSIONS=".gitmem/active-sessions.json" -if [ ! -f "$ACTIVE_SESSIONS" ]; then +# GIT-99: resolve .gitmem the way the server does, not from cwd. +. "$(dirname "${BASH_SOURCE[0]}")/resolve-root.sh" +if [ ! -f "$GITMEM_ACTIVE_SESSIONS" ]; then exit 0 fi -# Get first session ID from registry to find per-session data file +# Newest live session in the registry -> its per-session data file. +# Entries whose server pid is dead are skipped (GIT-99). SESSION_FILE="" -if command -v jq &>/dev/null; then - SID=$(jq -r '.sessions[0].session_id // empty' "$ACTIVE_SESSIONS" 2>/dev/null) - [ -n "$SID" ] && SESSION_FILE=".gitmem/sessions/${SID}/session.json" -elif command -v node &>/dev/null; then - SID=$(node -e "const fs=require('fs');try{const r=JSON.parse(fs.readFileSync('$ACTIVE_SESSIONS','utf8'));const s=(r.sessions||[])[0];process.stdout.write(s?.session_id||'')}catch(e){}" 2>/dev/null) - [ -n "$SID" ] && SESSION_FILE=".gitmem/sessions/${SID}/session.json" -fi +SID=$(gitmem_live_session_ids | head -n 1) +[ -n "$SID" ] && SESSION_FILE="$GITMEM_ROOT/sessions/${SID}/session.json" if [ -z "$SESSION_FILE" ] || [ ! -f "$SESSION_FILE" ]; then exit 0 diff --git a/hooks/scripts/resolve-root.sh b/hooks/scripts/resolve-root.sh new file mode 100755 index 0000000..f774148 --- /dev/null +++ b/hooks/scripts/resolve-root.sh @@ -0,0 +1,81 @@ +#!/bin/bash +# ============================================================================ +# gitmem-hooks — resolve the .gitmem root the way the MCP server does (GIT-99) +# +# Sourced, not executed: . "$(dirname "$0")/resolve-root.sh" +# +# The hooks used to read ".gitmem/..." relative to their cwd. Since GIT-91 the +# server resolves its root independently of cwd, so a hook running in a repo +# read a different store from the one the server writes: it saw no session, or +# someone else's, and pointed the agent at a closing-payload.json the server +# would never read. +# +# Precedence mirrors src/services/gitmem-dir.ts: +# 1. $GITMEM_DIR — names the .gitmem directory itself +# 2. $GITMEM_HOME/.gitmem — relocates the home the fallback uses +# 3. $HOME/.gitmem +# +# Sets (all absolute): +# GITMEM_ROOT the resolved .gitmem directory +# GITMEM_ACTIVE_SESSIONS $GITMEM_ROOT/active-sessions.json +# GITMEM_PAYLOAD_PATH $GITMEM_ROOT/closing-payload.json +# Provides: +# gitmem_live_session_ids registry session ids, newest first, skipping +# entries on this host whose pid is dead +# gitmem_json_escape escape a string for a JSON string literal +# ============================================================================ + +if [ -n "${GITMEM_DIR:-}" ]; then + _gitmem_root="$GITMEM_DIR" +elif [ -n "${GITMEM_HOME:-}" ]; then + _gitmem_root="$GITMEM_HOME/.gitmem" +else + _gitmem_root="$HOME/.gitmem" +fi +# Absolute, without requiring the directory to exist yet. +case "$_gitmem_root" in + /*) ;; + *) _gitmem_root="$PWD/$_gitmem_root" ;; +esac +GITMEM_ROOT="${_gitmem_root%/}" +unset _gitmem_root +GITMEM_ACTIVE_SESSIONS="$GITMEM_ROOT/active-sessions.json" +GITMEM_PAYLOAD_PATH="$GITMEM_ROOT/closing-payload.json" + +# A registry entry whose server died is not a session anyone can be in. Only +# entries on THIS host can be checked; entries without a pid, or from another +# host, are kept. `ps -p` rather than `kill -0`: kill fails with EPERM for a +# live process of another user, which is still alive. +gitmem_live_session_ids() { + [ -f "$GITMEM_ACTIVE_SESSIONS" ] || return 0 + local host rows + host=$(hostname 2>/dev/null || echo "") + if command -v jq &>/dev/null; then + rows=$(jq -r '(.sessions // []) | sort_by(.started_at // "") | reverse | .[] + | [(.session_id // ""), ((.pid // "") | tostring), (.hostname // "")] | join("|")' \ + "$GITMEM_ACTIVE_SESSIONS" 2>/dev/null) || return 0 + elif command -v node &>/dev/null; then + rows=$(GITMEM_REG="$GITMEM_ACTIVE_SESSIONS" node -e ' + try { + const r = JSON.parse(require("fs").readFileSync(process.env.GITMEM_REG, "utf8")); + const s = (r.sessions || []).slice().sort((a, b) => String(b.started_at || "").localeCompare(String(a.started_at || ""))); + process.stdout.write(s.map((e) => [e.session_id || "", e.pid ?? "", e.hostname || ""].join("|")).join("\n")); + } catch (e) {}' 2>/dev/null) || return 0 + else + return 0 + fi + local sid pid h + # "|" rather than a tab: read collapses runs of whitespace separators, so an + # empty pid would shift the hostname into its place. + while IFS='|' read -r sid pid h; do + [ -n "$sid" ] || continue + if [ -n "$pid" ] && [ "$h" = "$host" ] && ! ps -p "$pid" >/dev/null 2>&1; then + continue + fi + echo "$sid" + done <<< "$rows" +} + +gitmem_json_escape() { + printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' +} diff --git a/hooks/scripts/session-close-check.sh b/hooks/scripts/session-close-check.sh index ccbb57a..0b25075 100755 --- a/hooks/scripts/session-close-check.sh +++ b/hooks/scripts/session-close-check.sh @@ -35,16 +35,12 @@ fi IS_MEANINGFUL=false -ACTIVE_SESSIONS=".gitmem/active-sessions.json" +# GIT-99: resolve .gitmem the way the server does, not from cwd. +. "$(dirname "${BASH_SOURCE[0]}")/resolve-root.sh" +# A live registry entry means a session is open. Entries whose server pid is +# dead are skipped: nobody can close those from here (GIT-99). SESSION_STARTED=false -if [ -f "$ACTIVE_SESSIONS" ]; then - # Check if registry has active session entries - if command -v jq &>/dev/null; then - [ "$(jq '.sessions | length' "$ACTIVE_SESSIONS" 2>/dev/null || echo 0)" -gt 0 ] 2>/dev/null && SESSION_STARTED=true - elif command -v node &>/dev/null; then - [ "$(node -e "const fs=require('fs');try{const r=JSON.parse(fs.readFileSync('$ACTIVE_SESSIONS','utf8'));process.stdout.write(String((r.sessions||[]).length))}catch(e){process.stdout.write('0')}" 2>/dev/null)" -gt 0 ] 2>/dev/null && SESSION_STARTED=true - fi -fi +[ -n "$(gitmem_live_session_ids)" ] && SESSION_STARTED=true # Without state dir, we have no tracking data — can't determine meaningfulness. # This happens when the SessionStart hook didn't fire (plugin hooks issue). @@ -98,10 +94,11 @@ if [ "$SESSION_STARTED" = "true" ]; then mkdir -p "$STATE_DIR" touch "$STATE_DIR/stop_hook_active" - cat <<'HOOKJSON' + PAYLOAD_JSON=$(gitmem_json_escape "$GITMEM_PAYLOAD_PATH") + cat < "$R_CWD/.gitmem/active-sessions.json" +OUTPUT=$( cd "$R_CWD" && echo '{}' | bash "$SCRIPT_DIR/scripts/session-close-check.sh" 2>/dev/null ) +echo "$OUTPUT" | grep -q '"decision": "block"' && pass "Stop hook reads GITMEM_DIR from an unrelated cwd (decoy cwd/.gitmem ignored)" \ + || fail "Stop hook cwd independence" "block" "$OUTPUT" + +# The Stop hook names the absolute payload path it resolved. +PAYLOAD_EXPECTED="$GITMEM_DIR/closing-payload.json" +echo "$OUTPUT" | grep -qF "WRITE structured payload to $PAYLOAD_EXPECTED (this exact absolute path" \ + && pass "Stop hook text prints the resolved absolute payload path" \ + || fail "Stop hook payload path" "$PAYLOAD_EXPECTED" "$OUTPUT" +echo "$OUTPUT" | (command -v node >/dev/null && node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{JSON.parse(d)})' 2>/dev/null) \ + && pass "Stop hook output with the path is valid JSON" \ + || fail "Stop hook JSON" "parseable JSON" "$OUTPUT" + +# A path that needs escaping still yields valid JSON. +Q_DIR="$TMPDIR/q\"uote/.gitmem"; mkdir -p "$Q_DIR" +echo '{"sessions":[{"session_id":"q"}]}' > "$Q_DIR/active-sessions.json" +setup_state 10 60 +OUTPUT=$( GITMEM_DIR="$Q_DIR" bash -c 'echo "{}" | bash "$0/scripts/session-close-check.sh"' "$SCRIPT_DIR" 2>/dev/null ) +echo "$OUTPUT" | node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{const j=JSON.parse(d);if(!j.reason.includes(process.argv[1]))process.exit(1)})' "$Q_DIR/closing-payload.json" 2>/dev/null \ + && pass "payload path with a quote is JSON-escaped" \ + || fail "payload path escaping" "valid JSON containing $Q_DIR/closing-payload.json" "$OUTPUT" + +# Registry entries whose pid is dead on this host are skipped. +DEAD_PID=99999999 +while ps -p "$DEAD_PID" >/dev/null 2>&1; do DEAD_PID=$((DEAD_PID - 1)); done +HOST=$(hostname) +setup_state 10 60 +echo "{\"sessions\":[{\"session_id\":\"dead-one\",\"pid\":$DEAD_PID,\"hostname\":\"$HOST\",\"started_at\":\"2026-09-21T00:00:00Z\"}]}" > "$GITMEM_DIR/active-sessions.json" +OUTPUT=$(echo '{}' | bash "$SCRIPT_DIR/scripts/session-close-check.sh" 2>/dev/null) +! echo "$OUTPUT" | grep -q "block" && pass "Stop hook ignores a registry entry whose server pid is dead" \ + || fail "dead pid skipped (Stop)" "no block" "$OUTPUT" + +setup_state 10 60 +echo "{\"sessions\":[{\"session_id\":\"dead-one\",\"pid\":$DEAD_PID,\"hostname\":\"other-host-$HOST\",\"started_at\":\"2026-09-21T00:00:00Z\"}]}" > "$GITMEM_DIR/active-sessions.json" +OUTPUT=$(echo '{}' | bash "$SCRIPT_DIR/scripts/session-close-check.sh" 2>/dev/null) +echo "$OUTPUT" | grep -q "block" && pass "an entry from another host is kept (its pid cannot be checked here)" \ + || fail "other-host entry kept" "block" "$OUTPUT" + +# recall-check uses the newest LIVE session, not sessions[0]. +remove_session_registry +echo "{\"sessions\":[ + {\"session_id\":\"live-old\",\"pid\":$$,\"hostname\":\"$HOST\",\"started_at\":\"2026-09-21T00:00:00Z\"}, + {\"session_id\":\"dead-new\",\"pid\":$DEAD_PID,\"hostname\":\"$HOST\",\"started_at\":\"2026-09-21T09:00:00Z\"}]}" > "$GITMEM_DIR/active-sessions.json" +create_session_data "live-old" '[{"scar_id":"s1","scar_title":"Unconfirmed","source":"recall"}]' '[]' +create_session_data "dead-new" '[]' '[]' +GOT=$(bash -c '. "$0/scripts/resolve-root.sh"; gitmem_live_session_ids | tr "\n" " "' "$SCRIPT_DIR") +[ "$GOT" = "live-old " ] && pass "gitmem_live_session_ids: newest first, dead pid skipped" \ + || fail "live session ids" "live-old" "$GOT" +setup_state 0 0 +OUTPUT=$(echo '{"tool_name":"Bash","tool_input":{"command":"git push origin main"}}' | bash "$SCRIPT_DIR/scripts/recall-check.sh" 2>/dev/null) +echo "$OUTPUT" | grep -q "block" && pass "recall-check reads the live session's scars (dead newer entry skipped)" \ + || fail "recall-check live session" "block on unconfirmed scar of live-old" "$OUTPUT" + +remove_session_registry + # ============================================================================ # Summary # ============================================================================ From 709aa84f034c2551941b9cadca5267eadfd1ca10 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:20:03 -0700 Subject: [PATCH 2/4] fix: session_close names the payload path when there is nothing to close with (GIT-99) A standard close with no closing-payload.json and no inline reflection now returns "closing-payload.json not found at " instead of the "requires task_completion / closing_reflection with N answers" family, which sent agents to rewrite answers they had already written to the wrong root. An unparseable payload is reported as found but unreadable, at its path. session-close-supabase.test.ts: three title/decision tests closed "standard" with neither payload nor reflection, reaching persistence only because validation is mocked. That input is now rejected up front, so they carry a minimal reflection to keep testing what they test. +5 tests. Co-Authored-By: Claude Opus 5 --- src/tools/session-close.ts | 31 +++ .../tools/session-close-payload-path.test.ts | 203 ++++++++++++++++++ .../unit/tools/session-close-supabase.test.ts | 9 + 3 files changed, 243 insertions(+) create mode 100644 tests/unit/tools/session-close-payload-path.test.ts diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index 2388a4c..b21d73e 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -932,6 +932,7 @@ export async function sessionClose( // This keeps the visible MCP tool call small: just session_id + close_type. const payloadPath = getGitmemPath("closing-payload.json"); let payloadConsumed = false; + let payloadReadError: string | null = null; try { if (fs.existsSync(payloadPath)) { const filePayload = JSON.parse(fs.readFileSync(payloadPath, "utf-8")) as Partial; @@ -944,6 +945,36 @@ export async function sessionClose( } } catch (error) { console.warn("[session_close] Failed to read closing-payload.json:", error); + payloadReadError = + `closing-payload.json at ${path.resolve(payloadPath)} could not be read: ` + + `${error instanceof Error ? error.message : String(error)}. Fix or rewrite it, then call session_close again.`; + } + + // GIT-99: a standard close with no reflection at all. The agent almost always + // wrote the payload — to a root this server does not read (the hook and the + // server resolved .gitmem differently). Validation would then report + // "requires task_completion" / "requires closing_reflection with N answers", + // which sends the agent to rewrite answers it already wrote. Name the path. + if (params.close_type === "standard" && !payloadConsumed && !params.closing_reflection) { + return { + success: false, + session_id: params.session_id || "", + close_compliance: { + close_type: "standard", + agent: detectAgent().agent, + checklist_displayed: false, + questions_answered_by_agent: false, + human_asked_for_corrections: false, + learnings_stored: 0, + scars_applied: 0, + }, + validation_errors: [ + payloadReadError ?? + `closing-payload.json not found at ${path.resolve(payloadPath)}. ` + + `Write the closing payload to exactly that path (or pass closing_reflection inline), then call session_close again.`, + ], + performance: buildPerformanceData("session_close", timer.stop(), 0), + }; } // Sanitize scars_to_record: agents frequently write create_learning shape diff --git a/tests/unit/tools/session-close-payload-path.test.ts b/tests/unit/tools/session-close-payload-path.test.ts new file mode 100644 index 0000000..4fe12fd --- /dev/null +++ b/tests/unit/tools/session-close-payload-path.test.ts @@ -0,0 +1,203 @@ +/** + * GIT-99: a standard close with neither closing-payload.json nor an inline + * reflection names the path it looked at. + * + * The agent has usually written the payload — to a .gitmem root this server + * does not read. It used to get "requires task_completion" / "requires + * closing_reflection with N answers", which sends it to rewrite answers it + * already wrote instead of to the path mismatch. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +const root = vi.hoisted(() => ({ dir: "" })); + +// Mock dependencies before importing session-close +vi.mock("../../../src/services/agent-detection.js", () => ({ + detectAgent: () => ({ agent: "CLI", entrypoint: "cli", docker: true, hostname: "test" }), +})); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + listRecords: vi.fn().mockResolvedValue([]), + getRecord: vi.fn().mockResolvedValue(null), + directUpsert: vi.fn().mockResolvedValue(undefined), + directPatch: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../../src/services/embedding.js", () => ({ + embed: vi.fn().mockResolvedValue(null), + isEmbeddingAvailable: () => false, +})); + +vi.mock("../../../src/services/tier.js", () => ({ + hasSupabase: () => false, + hasBatchOperations: () => false, + hasTranscripts: () => false, + hasCacheManagement: () => false, + getTableName: (base: string) => `orchestra_${base}`, + hasProInsights: () => false, +})); + +vi.mock("../../../src/services/analytics.js", () => ({ + queryScarUsageByDateRange: vi.fn().mockResolvedValue([]), + enrichScarUsageTitles: vi.fn().mockResolvedValue([]), + formatBlindspotSnippet: vi.fn().mockReturnValue(null), + querySessionsByDateRange: vi.fn().mockResolvedValue([]), + computeLightweightSummary: vi.fn().mockReturnValue(null), +})); + +vi.mock("../../../src/services/storage.js", () => ({ + getStorage: () => ({ + get: vi.fn().mockResolvedValue(null), + upsert: vi.fn().mockResolvedValue(undefined), + }), +})); + +vi.mock("../../../src/services/session-state.js", () => ({ + clearCurrentSession: vi.fn(), + getSurfacedScars: () => [], + getObservations: () => [], + getChildren: () => [], + getThreads: () => [], + getSessionActivity: () => null, + isRecallCalled: () => true, +})); + +vi.mock("../../../src/services/thread-manager.js", () => ({ + normalizeThreads: vi.fn().mockReturnValue([]), + mergeThreadStates: vi.fn().mockReturnValue([]), + migrateStringThread: vi.fn().mockReturnValue({ id: "t-test", text: "test", status: "open", created_at: new Date().toISOString() }), + saveThreadsFile: vi.fn(), +})); + +vi.mock("../../../src/services/thread-dedup.js", () => ({ + deduplicateThreadList: vi.fn().mockImplementation((threads) => threads), +})); + +vi.mock("../../../src/services/thread-supabase.js", () => ({ + syncThreadsToSupabase: vi.fn().mockResolvedValue(undefined), + loadOpenThreadEmbeddings: vi.fn().mockResolvedValue([]), +})); + +vi.mock("../../../src/services/compliance-validator.js", () => ({ + validateSessionClose: () => ({ valid: true, errors: [], warnings: [] }), + buildCloseCompliance: vi.fn().mockReturnValue({ + close_type: "quick", + agent: "CLI", + checklist_displayed: true, + questions_answered_by_agent: false, + human_asked_for_corrections: false, + learnings_stored: 0, + scars_applied: 0, + }), +})); + +vi.mock("../../../src/services/metrics.js", () => ({ + Timer: class { stop() { return 100; } }, + recordMetrics: vi.fn().mockResolvedValue(undefined), + buildPerformanceData: (name: string, latency: number, count: number) => ({ + latency_ms: latency, + target_ms: 3000, + meets_target: latency < 3000, + result_count: count, + }), + updateRelevanceData: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../../src/tools/record-scar-usage-batch.js", () => ({ + recordScarUsageBatch: vi.fn().mockResolvedValue({ success: true }), +})); + +vi.mock("../../../src/services/effect-tracker.js", () => ({ + getEffectTracker: () => ({ + track: vi.fn(), + formatSummary: () => "No tracked effects this session.", + getHealthReport: () => ({ overall: { attempted: 0, succeeded: 0, failed: 0, successRate: "N/A", paths_with_failures: [] }, byPath: {}, recentFailures: [] }), + }), +})); + +vi.mock("../../../src/tools/save-transcript.js", () => ({ + saveTranscript: vi.fn().mockResolvedValue({ success: false }), +})); + +vi.mock("../../../src/services/transcript-chunker.js", () => ({ + processTranscript: vi.fn().mockResolvedValue({ success: false }), +})); + +vi.mock("../../../src/services/gitmem-dir.js", () => ({ + getGitmemPath: (filename: string) => `${root.dir}/${filename}`, + getGitmemDir: () => root.dir, + getSessionPath: (sid: string, filename: string) => `${root.dir}/sessions/${sid}/${filename}`, +})); + +vi.mock("../../../src/services/active-sessions.js", () => ({ + unregisterSession: vi.fn(), + findSessionByHostPid: vi.fn().mockReturnValue(null), +})); + +vi.mock("../../../src/services/thread-suggestions.js", () => ({ + loadSuggestions: vi.fn().mockReturnValue([]), + saveSuggestions: vi.fn(), + detectSuggestedThreads: vi.fn().mockReturnValue([]), + loadRecentSessionEmbeddings: vi.fn().mockResolvedValue(null), +})); + +import { sessionClose } from "../../../src/tools/session-close.js"; + +const SID = "393adb34-a80c-4c3a-b71a-bc0053b7a7ea"; +const payloadFile = () => path.join(root.dir, "closing-payload.json"); + +beforeEach(() => { + vi.clearAllMocks(); + root.dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git99-")); +}); + +afterEach(() => { + fs.rmSync(root.dir, { recursive: true, force: true }); +}); + +describe("session_close missing payload (GIT-99)", () => { + it("standard close, no payload, no inline reflection: names the absolute path", async () => { + const result = await sessionClose({ session_id: SID, close_type: "standard" }); + + expect(result.success).toBe(false); + expect(result.validation_errors).toHaveLength(1); + expect(result.validation_errors![0]).toBe( + `closing-payload.json not found at ${payloadFile()}. ` + + `Write the closing payload to exactly that path (or pass closing_reflection inline), then call session_close again.` + ); + expect(path.isAbsolute(result.validation_errors![0].split(" not found at ")[1].split(". Write")[0])).toBe(true); + expect(result.validation_errors!.join(" ")).not.toMatch(/answers|task_completion|recall\(\)/); + }); + + it("an unparseable payload is reported as found-but-unreadable, at its path", async () => { + fs.writeFileSync(payloadFile(), "{ not json"); + const result = await sessionClose({ session_id: SID, close_type: "standard" }); + + expect(result.success).toBe(false); + expect(result.validation_errors![0]).toContain(`closing-payload.json at ${payloadFile()} could not be read:`); + }); + + it("an inline reflection is enough — no payload file needed", async () => { + const result = await sessionClose({ + session_id: SID, + close_type: "standard", + closing_reflection: { what_broke: "x", what_worked: "y", scars_applied: [] }, + }); + expect((result.validation_errors || []).join(" ")).not.toContain("closing-payload.json not found"); + }); + + it("a payload at the path is consumed", async () => { + fs.writeFileSync(payloadFile(), JSON.stringify({ closing_reflection: { what_broke: "x", what_worked: "y", scars_applied: [] } })); + const result = await sessionClose({ session_id: SID, close_type: "standard" }); + expect((result.validation_errors || []).join(" ")).not.toContain("closing-payload.json"); + }); + + it("quick close never needs a payload", async () => { + const result = await sessionClose({ session_id: SID, close_type: "quick" }); + expect((result.validation_errors || []).join(" ")).not.toContain("closing-payload.json"); + }); +}); diff --git a/tests/unit/tools/session-close-supabase.test.ts b/tests/unit/tools/session-close-supabase.test.ts index d0d2872..70f0998 100644 --- a/tests/unit/tools/session-close-supabase.test.ts +++ b/tests/unit/tools/session-close-supabase.test.ts @@ -347,6 +347,12 @@ describe("session_close Supabase path", () => { }); }); + // GIT-99: a standard close with neither a payload nor an inline reflection is + // now rejected up front ("closing-payload.json not found at "). These + // tests are about titles and decisions, not validation (which is mocked), so + // they carry a reflection to reach the persistence path they check. + const REFLECTION = { what_broke: "n/a", what_worked: "n/a", scars_applied: [] }; + describe("session title update", () => { it("updates generic title with Linear issue + decision", async () => { vi.mocked(supabase.getRecord).mockResolvedValue( @@ -356,6 +362,7 @@ describe("session_close Supabase path", () => { await sessionClose({ session_id: VALID_UUID, close_type: "standard", + closing_reflection: REFLECTION, linear_issue: "PROJ-123", decisions: [{ title: "Chose X over Y", decision: "X", rationale: "because" }], }); @@ -372,6 +379,7 @@ describe("session_close Supabase path", () => { await sessionClose({ session_id: VALID_UUID, close_type: "standard", + closing_reflection: REFLECTION, linear_issue: "PROJ-123", decisions: [{ title: "Chose X over Y", decision: "X", rationale: "because" }], }); @@ -388,6 +396,7 @@ describe("session_close Supabase path", () => { await sessionClose({ session_id: VALID_UUID, close_type: "standard", + closing_reflection: REFLECTION, decisions: [ { title: "Decision A", decision: "Chose A", rationale: "faster" }, { title: "Decision B", decision: "Chose B", rationale: "simpler" }, From f1a18421528627a787f59ece4158dd02d4b79c66 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 20:20:03 -0700 Subject: [PATCH 3/4] test: venue flow checks a payload-less close names the absolute payload path (GIT-99) Exits 1 on 1.10.0 ("requires closing_reflection with 7 answers"); exits 0 with the fix. Co-Authored-By: Claude Opus 5 --- tests/e2e/blank-supabase.mjs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..cb53a70 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -366,6 +366,13 @@ async function flow() { const lt = await step("list_threads", { project: PROJECT }); const ltOpen = Number((lt.match(/(\d+) open/) || [])[1] ?? -1); + // GIT-99: a standard close with no payload file and no inline reflection + // names the absolute path this server reads, instead of "requires N answers". + const expectedPayloadPath = join(gitmemDir, "closing-payload.json"); + const scNoPayload = await step("session_close", { session_id: sessionId, close_type: "standard" }); + const missingPayloadNamed = scNoPayload.includes(`closing-payload.json not found at ${expectedPayloadPath}`) + && !/requires closing_reflection|requires task_completion/.test(scNoPayload); + const sc = await step("session_close", { session_id: sessionId, close_type: "standard", human_corrections: "none", closing_reflection: { @@ -408,6 +415,7 @@ async function flow() { remote_sessions_after: await count("gitmem_sessions"), remote_scar_usage_after: await count("gitmem_scar_usage"), session_close_persisted: closedRows.length === 1 && closedRows[0].closing_reflection != null, + missing_payload_named: missingPayloadNamed, relevance_readable: relevance.some((r) => scarIds.some((id) => (r.memories_applied || []).includes(id) && r.memory_relevance?.[id])), relevance, health_failed_total: failed, @@ -520,6 +528,9 @@ const failureCheck = checkFailures(allVenueRequests); const edgeCalls = allVenueRequests.filter((r) => r.path.startsWith("/functions/v1/")) .map((r) => `${r.method} ${r.path} -> ${r.status}`); if (edgeCalls.length) failureCheck.unexpected.push(...edgeCalls.map((c) => `edge function called (GIT-97): ${c}`)); +if (mode === "flow" && !result.missing_payload_named) { + failureCheck.unexpected.push("session_close without a payload did not name the absolute closing-payload.json path (GIT-99)"); +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } From c18e82acf34da48a67ffb20458286e8ec47b729c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 21:03:05 -0700 Subject: [PATCH 4/4] fix: hook pid check needs no ps (GIT-99) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve-root.sh decided liveness with `ps -p`. On an image without ps (e.g. node:*-slim, the natural jq-less CI runner) that call fails, so every registry entry looked dead and the Stop hook never blocked — the GIT-112 failure again, through a different missing tool. gitmem_pid_alive uses the kill -0 builtin, with EPERM counted as alive. +2 hook tests (46/46): the check with no ps on PATH, and an EPERM pid. Co-Authored-By: Claude Opus 5 --- hooks/scripts/resolve-root.sh | 16 +++++++++++++--- hooks/tests/test-hooks.sh | 10 +++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/hooks/scripts/resolve-root.sh b/hooks/scripts/resolve-root.sh index f774148..669a461 100755 --- a/hooks/scripts/resolve-root.sh +++ b/hooks/scripts/resolve-root.sh @@ -22,6 +22,7 @@ # Provides: # gitmem_live_session_ids registry session ids, newest first, skipping # entries on this host whose pid is dead +# gitmem_pid_alive whether a pid is running (no `ps` needed) # gitmem_json_escape escape a string for a JSON string literal # ============================================================================ @@ -42,10 +43,19 @@ unset _gitmem_root GITMEM_ACTIVE_SESSIONS="$GITMEM_ROOT/active-sessions.json" GITMEM_PAYLOAD_PATH="$GITMEM_ROOT/closing-payload.json" +# Is this pid a running process? `kill -0` needs no external tool (minimal +# images have no `ps`, and a missing `ps` would make every session look dead). +# EPERM ("Operation not permitted") means it exists but belongs to another user. +gitmem_pid_alive() { + local out + out=$(kill -0 "$1" 2>&1) && return 0 + case "$out" in *ermitted*) return 0 ;; esac + return 1 +} + # A registry entry whose server died is not a session anyone can be in. Only # entries on THIS host can be checked; entries without a pid, or from another -# host, are kept. `ps -p` rather than `kill -0`: kill fails with EPERM for a -# live process of another user, which is still alive. +# host, are kept. gitmem_live_session_ids() { [ -f "$GITMEM_ACTIVE_SESSIONS" ] || return 0 local host rows @@ -69,7 +79,7 @@ gitmem_live_session_ids() { # empty pid would shift the hostname into its place. while IFS='|' read -r sid pid h; do [ -n "$sid" ] || continue - if [ -n "$pid" ] && [ "$h" = "$host" ] && ! ps -p "$pid" >/dev/null 2>&1; then + if [ -n "$pid" ] && [ "$h" = "$host" ] && ! gitmem_pid_alive "$pid"; then continue fi echo "$sid" diff --git a/hooks/tests/test-hooks.sh b/hooks/tests/test-hooks.sh index 347f4af..95d5df8 100755 --- a/hooks/tests/test-hooks.sh +++ b/hooks/tests/test-hooks.sh @@ -687,7 +687,7 @@ echo "$OUTPUT" | node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()= # Registry entries whose pid is dead on this host are skipped. DEAD_PID=99999999 -while ps -p "$DEAD_PID" >/dev/null 2>&1; do DEAD_PID=$((DEAD_PID - 1)); done +while kill -0 "$DEAD_PID" 2>/dev/null; do DEAD_PID=$((DEAD_PID - 1)); done HOST=$(hostname) setup_state 10 60 echo "{\"sessions\":[{\"session_id\":\"dead-one\",\"pid\":$DEAD_PID,\"hostname\":\"$HOST\",\"started_at\":\"2026-09-21T00:00:00Z\"}]}" > "$GITMEM_DIR/active-sessions.json" @@ -716,6 +716,14 @@ OUTPUT=$(echo '{"tool_name":"Bash","tool_input":{"command":"git push origin main echo "$OUTPUT" | grep -q "block" && pass "recall-check reads the live session's scars (dead newer entry skipped)" \ || fail "recall-check live session" "block on unconfirmed scar of live-old" "$OUTPUT" +# The pid check needs no `ps` (minimal images have none). +GOT=$(PATH="$TMPDIR/no-ps-bin" "$BASH" -c '. "$0/scripts/resolve-root.sh"; gitmem_pid_alive $$ && echo alive; gitmem_pid_alive '"$DEAD_PID"' || echo dead' "$SCRIPT_DIR" 2>/dev/null | tr "\n" " ") +[ "$GOT" = "alive dead " ] && pass "gitmem_pid_alive works with no ps on PATH" \ + || fail "pid check without ps" "alive dead" "$GOT" +GOT=$(bash -c '. "$0/scripts/resolve-root.sh"; gitmem_pid_alive 1 && echo alive' "$SCRIPT_DIR") +[ "$GOT" = "alive" ] && pass "a live pid owned by another user (EPERM) counts as alive" \ + || fail "EPERM pid alive" "alive" "$GOT" + remove_session_registry # ============================================================================