diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f672779..a92f871 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,9 @@ on: tags: ['v*'] pull_request: branches: [main] + # GIT-83: run the venue gate by hand (Actions → CI → Run workflow) before + # tagging. Publishing still happens only on a v* tag. + workflow_dispatch: jobs: build: @@ -51,8 +54,117 @@ jobs: - name: Dependency audit (no unused deps) run: npx depcheck --ignores="@types/*" --json | node -e "const d=JSON.parse(require('fs').readFileSync('/dev/stdin','utf8')); const unused=d.dependencies||[]; if(unused.length){console.error('Unused dependencies:',unused);process.exit(1)}" + # GIT-83 / GIT-112: the hook suite on a runner WITHOUT jq (or ps). The hooks + # fall back to node and bash builtins there; on 1.10.0 that fallback left + # Stop enforcement inactive and nothing noticed, because every runner and + # dev machine the suite had run on had jq. + hooks-without-jq: + runs-on: ubuntu-latest + container: node:20-bookworm-slim + steps: + - uses: actions/checkout@v4 + + - name: Runner has no jq and no ps + run: | + if command -v jq >/dev/null; then echo "::error::jq is installed; this job must run without it"; exit 1; fi + if command -v ps >/dev/null; then echo "::error::ps is installed; this job must run without it"; exit 1; fi + bash --version | head -1; node --version + + - name: Hook tests + run: bash hooks/tests/test-hooks.sh + + # GIT-83: the release gate. Customers never run SQL after setup (standing + # rule, 2026-09-21), so a release must work on a store still on the v1.8.0 + # setup.sql — the venue is put on exactly that schema first, then the + # current one. Runs on v* tags (publish depends on it) and on demand. + # + # Needs repository secrets, all for the DISPOSABLE venue (never production; + # scripts/venue-schema.sh and the driver both refuse it): + # VENUE_REF project ref + # VENUE_SUPABASE_URL https://.supabase.co + # VENUE_SERVICE_ROLE_KEY service role key + # VENUE_DB_URL postgres:// URL (session pooler) for schema resets + venue-gate: + if: startsWith(github.ref, 'refs/tags/v') || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + timeout-minutes: 30 + # One venue: two runs would wipe each other's data. + concurrency: + group: gitmem-venue + cancel-in-progress: false + env: + VENUE_REF: ${{ secrets.VENUE_REF }} + VENUE_DB_URL: ${{ secrets.VENUE_DB_URL }} + VENUE_SUPABASE_URL: ${{ secrets.VENUE_SUPABASE_URL }} + VENUE_SERVICE_ROLE_KEY: ${{ secrets.VENUE_SERVICE_ROLE_KEY }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 # the v1.8.0 tag supplies the floor schema + + - uses: actions/setup-node@v4 + with: + node-version: 20 + cache: npm + + - run: npm ci --legacy-peer-deps + - run: npm run build + + # Missing secrets fail the gate — a skipped gate would let publish through. + - name: Venue configured + run: | + missing="" + for v in VENUE_REF VENUE_DB_URL VENUE_SUPABASE_URL VENUE_SERVICE_ROLE_KEY; do + [ -n "${!v}" ] || missing="$missing $v" + done + if [ -n "$missing" ]; then echo "::error::venue gate cannot run, missing secrets:$missing"; exit 1; fi + umask 077 + printf 'SUPABASE_URL=%s\nSUPABASE_SERVICE_ROLE_KEY=%s\nVENUE_REF=%s\n' \ + "$VENUE_SUPABASE_URL" "$VENUE_SERVICE_ROLE_KEY" "$VENUE_REF" > "$RUNNER_TEMP/venue.env" + echo "VENUE_ENV=$RUNNER_TEMP/venue.env" >> "$GITHUB_ENV" + echo "RUNS=$RUNNER_TEMP/venue-runs" >> "$GITHUB_ENV" + mkdir -p "$RUNNER_TEMP/venue-runs" + git show v1.8.0:schema/setup.sql > "$RUNNER_TEMP/setup-v1.8.0.sql" + + - name: Venue on the v1.8.0 setup.sql + run: scripts/venue-schema.sh reset-to "$RUNNER_TEMP/setup-v1.8.0.sql" + + - name: Driver on v1.8.0 — flow + run: node tests/e2e/blank-supabase.mjs flow --label v180-flow --out "$RUNS" + + - name: Driver on v1.8.0 — egress + run: node tests/e2e/blank-supabase.mjs egress --label v180-egress --out "$RUNS" --rows 250 --usage 5 + + - name: Driver on v1.8.0 — projects (when the driver has it) + run: | + # The usage line lists the modes; a bare "projects" string also appears in a path. + if grep -q 'flow|egress|projects' tests/e2e/blank-supabase.mjs; then + node tests/e2e/blank-supabase.mjs projects --label v180-projects --out "$RUNS" + else + echo "driver has no projects mode in this revision" + fi + + # The current setup.sql must apply on top of v1.8.0, and apply again. + - name: Venue on the current setup.sql (applied twice) + run: | + scripts/venue-schema.sh apply schema/setup.sql + scripts/venue-schema.sh apply schema/setup.sql + + - name: Driver on the current schema — flow + run: node tests/e2e/blank-supabase.mjs flow --label current-flow --out "$RUNS" + + - name: Driver reports + if: always() + uses: actions/upload-artifact@v4 + with: + name: venue-gate-${{ github.run_id }} + # netlogs record method, path, status and body size only — no bodies or headers. + path: ${{ runner.temp }}/venue-runs + if-no-files-found: ignore + retention-days: 14 + publish: - needs: build + needs: [build, hooks-without-jq, venue-gate] runs-on: ubuntu-latest if: startsWith(github.ref, 'refs/tags/v') permissions: diff --git a/CHANGELOG.md b/CHANGELOG.md index 71ee809..96a05f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,64 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.11.0] - 2026-09-21 + +**Just upgrade the package. Nothing to change on your database.** Every fix in this release works on +a project still on the `setup.sql` you first ran, back to 1.8.0. Each one was checked against a +project on exactly that schema. + +**A chat in one project no longer continues another project's session.** The desktop app runs one +gitmem process for every chat. Starting a session for project Y while a project X chat was open used +to resume X's session, swapping the project you asked for for X's and saying so only on stderr. +`session_start(project: Y)` now starts a Y session and leaves X open and resumable. Called with no +project, it resumes the most recent session and names that session's project in its output. +`force: true` carries activity forward only from a session of the same project. (GIT-86) + +**Every write now says whether it was actually saved.** `create_learning`, `create_decision`, +`create_thread`, `resolve_thread`, `archive_learning`, `record_scar_usage` (single and batch), +`session_close`, `promote_suggestion` and `save_transcript` now report `durable` and `stored_in` +alongside `success`. `success` means the record reached your database. Before this, +`resolve_thread` reported success even when your database never recorded the resolve, so other +sessions still saw the thread as open. Now it says `RESOLVED LOCALLY ONLY — not durable`. (GIT-101) + +**Stop enforcement was inactive on machines without jq; fixed.** (GIT-112) + +### Fixed + +- **Sessions start without re-downloading the whole index.** On Pro, nearly every `session_start` + changes a few scar scores, and the next start used to download every learning with its embedding + again. Now only the changed rows are fetched: in testing, 42 KB instead of 922 KB for 250 + learnings. (GIT-98) +- **The hooks read the same store as the server.** They looked for `.gitmem` in the current directory + instead of `GITMEM_DIR`, then `GITMEM_HOME`, then `~/.gitmem`, so they could see no session or + someone else's. They now skip sessions whose server process has exited, and the Stop hook prints + the exact path of `closing-payload.json`. (GIT-99) +- **`session_close` says where it looked for your payload.** With no `closing-payload.json` and no + inline reflection, it now reports `closing-payload.json not found at ` instead of asking you + to answer the closing questions again. (GIT-99) +- **The startup write check can no longer hang silently.** It gives up after 10 seconds and reports + "timed out, durability UNVERIFIED", and a database that refuses the connection is reported as + unreachable rather than OK. `health` now shows the latest verdict. (GIT-102) +- **`session_close` warnings say what failed.** Each warning names the part that failed and why, and + says whether the session itself was saved. Before, a close showed only `WARN N write failures`. + (GIT-102) +- **No more `metrics` failure at `session_start`.** A metrics row now waits for its session row to + exist. (GIT-73, listed under *Known issues* in 1.10.0) +- **Knowledge-graph links for threads are saved.** They wrote a thread id where the database expects a + UUID, so every one was rejected. (GIT-105, listed under *Known issues* in 1.10.0) +- **`migrate-root` merges your memory instead of skipping it.** If your home store already had + learnings, threads, decisions or sessions, the project store's were skipped as "already exists". They + are now merged record by record: the newer version wins and the other is written to + `migrate-root-conflicts.json`, after a backup. Running it again changes nothing. (GIT-100) +- **No false "Memory store found but NOT being read" on macOS.** Paths under `/var` and + `/private/var` (and any symlink) were compared as text, so the store gitmem was reading could be + reported as unread. (GIT-107) + +### Changed + +- **Releases are gated on a database still on 1.8.0's `setup.sql`, and on the hooks passing + without jq.** Nothing is published unless both pass. (GIT-83, GIT-109) + ## [1.10.0] - 2026-09-21 **Sessions that weren't being saved now save.** On your own Supabase project, a `session_close` was 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..669a461 --- /dev/null +++ b/hooks/scripts/resolve-root.sh @@ -0,0 +1,91 @@ +#!/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_pid_alive whether a pid is running (no `ps` needed) +# 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" + +# 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. +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" ] && ! gitmem_pid_alive "$pid"; 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 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" +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" + +# 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 + # ============================================================================ # Summary # ============================================================================ diff --git a/package-lock.json b/package-lock.json index 9b91cac..31aee33 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "gitmem-mcp", - "version": "1.10.0", + "version": "1.11.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "gitmem-mcp", - "version": "1.10.0", + "version": "1.11.0", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.0.0", diff --git a/package.json b/package.json index 3e0955f..70b18cf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gitmem-mcp", - "version": "1.10.0", + "version": "1.11.0", "mcpName": "io.github.gitmem-dev/gitmem", "description": "Persistent learning memory for AI coding agents. Memory that compounds.", "type": "module", diff --git a/scripts/venue-schema.sh b/scripts/venue-schema.sh new file mode 100755 index 0000000..9e35ca5 --- /dev/null +++ b/scripts/venue-schema.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash +# GIT-83: put the disposable venue on a given schema, for the release gate. +# +# scripts/venue-schema.sh reset-to drop gitmem objects, then apply +# scripts/venue-schema.sh apply apply on top (re-applicability) +# add --dry-run to print what would run and exit +# +# Needs VENUE_REF and VENUE_DB_URL (a postgres:// URL for the venue only). +# Refuses production, and any URL that does not name VENUE_REF, before +# connecting. Uses psql with ON_ERROR_STOP, in one transaction per call. +set -euo pipefail + +DENY_REFS="cjptxyezuxdiinufgrrm" # production GitMem — never touch +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +RESET_SQL="$HERE/../tests/e2e/venue/reset-gitmem-objects.sql" + +die() { echo "[venue-schema] $*" >&2; exit 2; } + +mode="${1:-}"; sql="${2:-}"; dry=false +[ "${3:-}" = "--dry-run" ] && dry=true +case "$mode" in reset-to|apply) ;; *) die "usage: venue-schema.sh reset-to|apply [--dry-run]" ;; esac +[ -n "$sql" ] && [ -f "$sql" ] || die "schema file not found: ${sql:-}" +[ -n "${VENUE_REF:-}" ] || die "VENUE_REF is not set" +[ -n "${VENUE_DB_URL:-}" ] || die "VENUE_DB_URL is not set" +for d in $DENY_REFS; do + [ "$VENUE_REF" != "$d" ] || die "refusing: VENUE_REF is production ($d)" + case "$VENUE_DB_URL" in *"$d"*) die "refusing: VENUE_DB_URL names production ($d)" ;; esac +done +case "$VENUE_DB_URL" in + postgres://*|postgresql://*) ;; + *) die "refusing: VENUE_DB_URL is not a postgres:// URL" ;; +esac +case "$VENUE_DB_URL" in *"$VENUE_REF"*) ;; *) die "refusing: VENUE_DB_URL does not name the venue ($VENUE_REF)" ;; esac + +files=() +[ "$mode" = "reset-to" ] && files+=("$RESET_SQL") +files+=("$sql") +if $dry; then + echo "[venue-schema] would apply to venue $VENUE_REF, one transaction: ${files[*]}" + exit 0 +fi +command -v psql >/dev/null || die "psql not found" +args=() +for f in "${files[@]}"; do args+=(-f "$f"); done +# NOTIFY is delivered at commit: PostgREST reloads its schema cache before the driver runs. +PGCONNECT_TIMEOUT=20 psql "$VENUE_DB_URL" -X -q -v ON_ERROR_STOP=1 --single-transaction "${args[@]}" \ + -c "NOTIFY pgrst, 'reload schema'" +echo "[venue-schema] venue $VENUE_REF: $mode $(basename "$sql") OK" diff --git a/src/commands/migrate-root.ts b/src/commands/migrate-root.ts index faa4241..de5ff93 100644 --- a/src/commands/migrate-root.ts +++ b/src/commands/migrate-root.ts @@ -18,10 +18,21 @@ * is wrong about anything, the user still has their data where it was. Moving * would make a bad merge unrecoverable. * - * NEVER OVERWRITE. A file that already exists at the destination wins. The - * destination is the live store; the source is, by definition, the one that - * has not been read recently. Clobbering current memory with stale memory is - * worse than skipping. + * NEVER OVERWRITE a non-memory file. One that already exists at the + * destination wins; the destination is the live store. + * + * MERGE THE MEMORY FILES (GIT-100). learnings.json, threads.json, + * decisions.json and sessions.json used to be skipped as "already exists" — + * so a store with ANY local memory at the destination migrated none of the + * project store's memory, and the report listed the user's scars under + * "skipped". They are now merged by record id: new ids are added; where both + * sides hold a record, the newer (updated_at, else resolved_at / + * last_touched_at / created_at) wins, the destination on a tie or when + * neither is dated; every loser is written to migrate-root-conflicts.json. + * The destination's memory files are backed up before the first write. + * Re-running is idempotent: nothing changes, and a conflict is logged once. + * A memory file that cannot be read as a list of records is reported as NOT + * merged — never as skipped. * * REPORT EVERY SKIP. A silent partial migration would leave the user believing * they had merged when they had not — the failure class GIT-93 was about. @@ -29,13 +40,119 @@ import * as fs from "fs"; import * as path from "path"; -import { findStrandedProjectRoots, getHomeGitmemDir } from "../services/gitmem-dir.js"; +import { findStrandedProjectRoots, getHomeGitmemDir, sameDirectory } from "../services/gitmem-dir.js"; + +/** Memory files merged by record id instead of skipped (GIT-100). */ +export const MERGED_FILES = ["learnings.json", "threads.json", "decisions.json", "sessions.json"] as const; +const CONFLICTS_FILE = "migrate-root-conflicts.json"; + +export interface MergeReport { + file: string; + added: number; + updated: number; + unchanged: number; + /** Both sides held the record and the destination's was kept. */ + kept_destination: number; + /** Conflicts written to the conflicts file by this run (already-logged ones are not repeated). */ + conflicts_logged: number; +} interface MigrationPlan { source: string; destination: string; copied: string[]; skipped: Array<{ file: string; reason: string }>; + merged: MergeReport[]; + /** Memory files that could not be merged. Never silently skipped. */ + failed: Array<{ file: string; reason: string }>; + /** Where the destination's memory files were backed up, when anything was written. */ + backup?: string; + conflicts_file?: string; +} + +type MemoryRecord = Record & { id: string }; + +interface ConflictEntry { + file: string; + id: string; + kept: "destination" | "source"; + kept_timestamp: string | null; + lost_timestamp: string | null; + lost_record: unknown; + logged_at: string; +} + +const RECENCY_FIELDS = ["updated_at", "resolved_at", "last_touched_at", "created_at"] as const; + +function recency(r: MemoryRecord): { at: number | null; raw: string | null } { + for (const f of RECENCY_FIELDS) { + const v = r[f]; + if (typeof v === "string") { + const at = Date.parse(v); + if (Number.isFinite(at)) return { at, raw: v }; + } + } + return { at: null, raw: null }; +} + +/** Key-order-independent JSON, so re-serialized records compare equal. */ +function stable(v: unknown): string { + if (Array.isArray(v)) return `[${v.map(stable).join(",")}]`; + if (v && typeof v === "object") { + return `{${Object.keys(v as object).sort().map((k) => `${JSON.stringify(k)}:${stable((v as Record)[k])}`).join(",")}}`; + } + return JSON.stringify(v); +} + +function readRecords(file: string): { ok: true; records: MemoryRecord[] } | { ok: false; reason: string } { + let data: unknown; + try { + data = JSON.parse(fs.readFileSync(file, "utf-8")); + } catch (e) { + return { ok: false, reason: `unreadable JSON (${e instanceof Error ? e.message : String(e)})` }; + } + if (!Array.isArray(data)) return { ok: false, reason: "not a list of records" }; + const bad = data.findIndex((r) => !r || typeof r !== "object" || typeof (r as { id?: unknown }).id !== "string"); + if (bad !== -1) return { ok: false, reason: `record ${bad} has no string id` }; + return { ok: true, records: data as MemoryRecord[] }; +} + +interface PlannedMerge { + report: MergeReport; + merged: MemoryRecord[]; + conflicts: Omit[]; + changed: boolean; +} + +/** Merge source into destination by id. Pure: returns the result, writes nothing. */ +function planMerge(file: string, dest: MemoryRecord[], src: MemoryRecord[]): PlannedMerge { + const report: MergeReport = { file, added: 0, updated: 0, unchanged: 0, kept_destination: 0, conflicts_logged: 0 }; + const conflicts: Omit[] = []; + const merged = dest.slice(); + const index = new Map(merged.map((r, i) => [r.id, i])); + + for (const s of src) { + const i = index.get(s.id); + if (i === undefined) { + index.set(s.id, merged.length); + merged.push(s); + report.added++; + continue; + } + const d = merged[i]; + if (stable(d) === stable(s)) { report.unchanged++; continue; } + const rd = recency(d), rs = recency(s); + const sourceNewer = rs.at !== null && (rd.at === null || rs.at > rd.at); + if (sourceNewer) { + merged[i] = s; + report.updated++; + conflicts.push({ file, id: s.id, kept: "source", kept_timestamp: rs.raw, lost_timestamp: rd.raw, lost_record: d }); + } else { + report.kept_destination++; + conflicts.push({ file, id: s.id, kept: "destination", kept_timestamp: rd.raw, lost_timestamp: rs.raw, lost_record: s }); + } + } + return { report, merged, conflicts, changed: report.added > 0 || report.updated > 0 }; } /** @@ -47,19 +164,26 @@ export function findProjectRoots(): string[] { return findStrandedProjectRoots(); } +const isPerInstall = (relative: string, name: string) => + relative === "" && (name === "cache" || name === "license-cache.json" || name === "backups" || name === CONFLICTS_FILE); + +const isMergedFile = (relative: string, name: string) => + relative === "" && (MERGED_FILES as readonly string[]).includes(name); + /** - * Recursively copy `from` into `to`, never overwriting an existing file. - * - * Returns what was copied and what was left alone, so the caller can report both - * rather than claiming a clean merge. + * Walk `from`, planning (and unless dryRun, performing) the copy of every + * non-memory file that does not exist at `to`. Memory files present on both + * sides are collected for merging instead. */ -function copyTree( +function walk( from: string, to: string, plan: MigrationPlan, + dryRun: boolean, + toMerge: string[], relative = "" ): void { - fs.mkdirSync(to, { recursive: true }); + if (!dryRun) fs.mkdirSync(to, { recursive: true }); for (const entry of fs.readdirSync(from, { withFileTypes: true })) { const rel = relative ? path.join(relative, entry.name) : entry.name; const src = path.join(from, entry.name); @@ -67,50 +191,101 @@ function copyTree( // Caches and license state are per-install, not memory. Copying them would // move a license binding between roots, which is not this command's job. - if (relative === "" && (entry.name === "cache" || entry.name === "license-cache.json")) { + // Backups and the conflicts log belong to the root they were written in. + if (isPerInstall(relative, entry.name)) { plan.skipped.push({ file: rel, reason: "per-install state, not memory" }); continue; } if (entry.isDirectory()) { - copyTree(src, dst, plan, rel); + walk(src, dst, plan, dryRun, toMerge, rel); continue; } if (fs.existsSync(dst)) { - plan.skipped.push({ file: rel, reason: "already exists in destination" }); + if (isMergedFile(relative, entry.name)) { + toMerge.push(entry.name); + } else { + plan.skipped.push({ file: rel, reason: "already exists in destination" }); + } continue; } - fs.copyFileSync(src, dst); + if (!dryRun) fs.copyFileSync(src, dst); plan.copied.push(rel); } } +function loadConflicts(file: string): ConflictEntry[] { + try { + const data = JSON.parse(fs.readFileSync(file, "utf-8")); + return Array.isArray(data) ? data : []; + } catch { + return []; + } +} + +const conflictKey = (c: Omit) => `${c.file}\n${c.id}\n${stable(c.lost_record)}`; + export function migrateRoot(source: string, destination: string, dryRun: boolean): MigrationPlan { - const plan: MigrationPlan = { source, destination, copied: [], skipped: [] }; - - if (dryRun) { - // Walk the same tree without writing, so --dry-run reports the real plan - // rather than a guess at one. - const probe = (from: string, to: string, rel = ""): void => { - for (const entry of fs.readdirSync(from, { withFileTypes: true })) { - const r = rel ? path.join(rel, entry.name) : entry.name; - if (rel === "" && (entry.name === "cache" || entry.name === "license-cache.json")) { - plan.skipped.push({ file: r, reason: "per-install state, not memory" }); - continue; - } - if (entry.isDirectory()) { probe(path.join(from, entry.name), path.join(to, entry.name), r); continue; } - if (fs.existsSync(path.join(to, entry.name))) { - plan.skipped.push({ file: r, reason: "already exists in destination" }); - continue; - } - plan.copied.push(r); - } - }; - probe(source, destination); - return plan; + const plan: MigrationPlan = { source, destination, copied: [], skipped: [], merged: [], failed: [] }; + const toMerge: string[] = []; + + // Walk the same tree either way, so --dry-run reports the real plan rather + // than a guess at one. + walk(source, destination, plan, dryRun, toMerge); + + // Memory files on both sides: merge by record id (GIT-100). + const planned: Array = []; + for (const name of toMerge) { + const d = readRecords(path.join(destination, name)); + const s = readRecords(path.join(source, name)); + if (!d.ok || !s.ok) { + const reason = !d.ok ? `destination ${d.reason}` : `source ${(s as { reason: string }).reason}`; + plan.failed.push({ file: name, reason: `NOT merged: ${reason}` }); + continue; + } + planned.push({ name, ...planMerge(name, d.records, s.records) }); + } + + const conflictsPath = path.join(destination, CONFLICTS_FILE); + const logged = loadConflicts(conflictsPath); + const seen = new Set(logged.map(conflictKey)); + const newConflicts: ConflictEntry[] = []; + const now = new Date().toISOString(); + for (const p of planned) { + for (const c of p.conflicts) { + const k = conflictKey(c); + if (seen.has(k)) continue; + seen.add(k); + newConflicts.push({ ...c, logged_at: now }); + p.report.conflicts_logged++; + } + plan.merged.push(p.report); } + if (newConflicts.length > 0 || logged.length > 0) plan.conflicts_file = conflictsPath; - copyTree(source, destination, plan); + const writes = planned.filter((p) => p.changed); + if (dryRun || (writes.length === 0 && newConflicts.length === 0)) return plan; + + // Back up the destination's memory files before the first write. + if (writes.length > 0) { + const backup = path.join(destination, "backups", `migrate-root-${now.replace(/[:.]/g, "-")}`); + fs.mkdirSync(backup, { recursive: true }); + for (const name of MERGED_FILES) { + const f = path.join(destination, name); + if (fs.existsSync(f)) fs.copyFileSync(f, path.join(backup, name)); + } + plan.backup = backup; + } + + for (const p of writes) { + const target = path.join(destination, p.name); + const tmpFile = `${target}.migrate-${process.pid}.tmp`; + fs.writeFileSync(tmpFile, JSON.stringify(p.merged, null, 2)); + fs.renameSync(tmpFile, target); + } + if (newConflicts.length > 0) { + fs.writeFileSync(conflictsPath, JSON.stringify([...logged, ...newConflicts], null, 2)); + } return plan; } @@ -144,7 +319,7 @@ export function main(args: string[]): void { } const source = sources[0]; - if (source === home) { + if (sameDirectory(source, home)) { // GIT-107: symlinks, /var vs /private/var console.log(`Source and destination are the same (${home}). Nothing to do.`); return; } @@ -156,6 +331,23 @@ export function main(args: string[]): void { const plan = migrateRoot(source, home, dryRun); console.log(`${plan.copied.length} file(s) ${dryRun ? "would be " : ""}copied.`); + if (plan.merged.length > 0) { + console.log(`${plan.merged.length} memory file(s) ${dryRun ? "would be " : ""}merged by record id:`); + for (const m of plan.merged) { + console.log( + ` ${m.file} — ${m.added} added, ${m.updated} updated from the source, ${m.unchanged} unchanged, ` + + `${m.kept_destination} kept from the destination` + + (m.conflicts_logged > 0 ? `, ${m.conflicts_logged} conflict(s) ${dryRun ? "would be " : ""}logged` : "") + ); + } + } + if (plan.failed.length > 0) { + console.log(`${plan.failed.length} memory file(s) NOT merged — fix these and re-run:`); + for (const f of plan.failed) console.log(` ${f.file} — ${f.reason}`); + process.exitCode = 1; + } + if (plan.backup) console.log(`Destination memory files backed up to ${plan.backup}`); + if (plan.conflicts_file) console.log(`Conflicting records (the version not kept) are in ${plan.conflicts_file}`); if (plan.skipped.length > 0) { console.log(`${plan.skipped.length} skipped:`); for (const s of plan.skipped) console.log(` ${s.file} — ${s.reason}`); diff --git a/src/server.ts b/src/server.ts index 956114d..9a5d3b6 100644 --- a/src/server.ts +++ b/src/server.ts @@ -64,7 +64,7 @@ import { flushCache, startBackgroundInit, } from "./services/startup.js"; -import { checkWritePath } from "./services/write-health.js"; +import { checkWritePathWithTimeout, getLastWritePathVerdict, formatWritePathLine } from "./services/write-health.js"; import { getEffectTracker } from "./services/effect-tracker.js"; import { RIPPLE, ANSI } from "./services/display-protocol.js"; import { getProject } from "./services/session-state.js"; @@ -371,9 +371,13 @@ export function createServer(): Server { const tracker = getEffectTracker(); const report = tracker.getHealthReport(failureLimit); const summary = tracker.formatSummary(); + // GIT-102: the startup write-path verdict, including "timed out, + // durability UNVERIFIED" — previously visible only on stderr, if at all. + const writePath = getLastWritePathVerdict(); result = { ...report, - text: summary || "No tracked effects this session.", + write_path: writePath, + text: `${formatWritePathLine(writePath)}\n\n${summary || "No tracked effects this session."}`, }; break; } @@ -501,14 +505,14 @@ export async function runServer(): Promise { } // Verify the write path with the FINAL tier (post-validation): catches an // invalid license silently downgrading writes to local files. - void checkWritePath(); + void checkWritePathWithTimeout(); // GIT-102: bounded; verdict kept for health }).catch((err) => { console.error(`[gitmem:license] Validation error: ${err}`); }); } else { // No license key to validate — verify the write path with the detected tier // (catches a GITMEM_TABLE_PREFIX / schema mismatch on the backward-compat path). - void checkWritePath(); + void checkWritePathWithTimeout(); // GIT-102: bounded; verdict kept for health } if (hasSupabase()) { diff --git a/src/services/active-sessions.ts b/src/services/active-sessions.ts index 9be4ac0..6f3ad0b 100644 --- a/src/services/active-sessions.ts +++ b/src/services/active-sessions.ts @@ -126,25 +126,38 @@ function writeRegistry(registry: ActiveSessionsRegistry): void { // --- CRUD operations --- +/** + * GIT-86: the slot a session occupies in the registry is hostname+pid+project. + * + * One desktop process serves every chat, so hostname+pid alone does not name a + * session — two chats in two projects share it. Keying the slot on pid alone + * let session_start(Y) displace an open X session, mark it superseded, and + * leave its directory for pruneOrphanedDirs to delete. + */ +function sameSlot(a: ActiveSessionEntry, b: ActiveSessionEntry): boolean { + return a.hostname === b.hostname && a.pid === b.pid && a.project === b.project; +} + /** * Register a new session in the active-sessions registry. * Idempotent: re-registering the same session_id replaces the entry. - * Returns session IDs that were displaced (different session_id, same hostname+pid). + * Returns session IDs that were displaced (different session_id, same + * hostname+pid+project). Sessions of other projects on the same process are + * left open (GIT-86). */ export function registerSession(entry: ActiveSessionEntry): string[] { const displaced: string[] = []; withLockSync(getLockPath(), () => { const registry = readRegistry(); - // Collect displaced session IDs (same hostname+pid, different session_id) + // Collect displaced session IDs (same slot, different session_id) for (const s of registry.sessions) { - if (s.session_id !== entry.session_id && s.hostname === entry.hostname && s.pid === entry.pid) { + if (s.session_id !== entry.session_id && sameSlot(s, entry)) { displaced.push(s.session_id); } } - // Remove by session_id AND by hostname+pid to prevent duplicates + // Remove by session_id AND by slot to prevent duplicates registry.sessions = registry.sessions.filter((s) => - s.session_id !== entry.session_id && - !(s.hostname === entry.hostname && s.pid === entry.pid) + s.session_id !== entry.session_id && !sameSlot(s, entry) ); registry.sessions.push(entry); writeRegistry(registry); @@ -197,10 +210,16 @@ export function listActiveSessions(): ActiveSessionEntry[] { /** * Find a session by hostname and PID. * Used by session_start to detect if this process already has a registered session. + * + * GIT-86: with `project`, only a session of that project matches — a process + * can hold one open session per project. Without it, the most recently started + * session of the process is returned. */ -export function findSessionByHostPid(hostname: string, pid: number): ActiveSessionEntry | null { +export function findSessionByHostPid(hostname: string, pid: number, project?: string): ActiveSessionEntry | null { const registry = readRegistry(); - return registry.sessions.find((s) => s.hostname === hostname && s.pid === pid) || null; + return registry.sessions + .filter((s) => s.hostname === hostname && s.pid === pid && (project === undefined || s.project === project)) + .sort((a, b) => new Date(b.started_at).getTime() - new Date(a.started_at).getTime())[0] || null; } /** @@ -264,7 +283,7 @@ function toAgentIdentity(value: unknown): AgentIdentity { return AGENT_IDENTITIES.find((id) => id === normalized) ?? "Unknown"; } -export function findResumableSessionOnDisk(): ActiveSessionEntry | null { +export function findResumableSessionOnDisk(project?: string): ActiveSessionEntry | null { const currentHostname = os.hostname(); const currentPid = process.pid; const gitmemDir = getGitmemDir(); @@ -304,6 +323,11 @@ export function findResumableSessionOnDisk(): ActiveSessionEntry | null { const age = now - new Date(startedAt).getTime(); if (!Number.isFinite(age) || age > STALE_THRESHOLD_MS) continue; + // GIT-86: a session of another project is never the answer to a request + // for this one, whatever its pid says. + const sessionProject = typeof data.project === "string" ? data.project : "default"; + if (project !== undefined && sessionProject !== project) continue; + const pid = typeof data.pid === "number" ? data.pid : -1; candidates.push({ entry: { @@ -312,7 +336,7 @@ export function findResumableSessionOnDisk(): ActiveSessionEntry | null { started_at: startedAt, hostname: currentHostname, pid, - project: typeof data.project === "string" ? data.project : "default", + project: sessionProject, }, pidAlive: pid > 0 && pid !== currentPid && isPidAlive(pid), }); @@ -331,7 +355,7 @@ export function findResumableSessionOnDisk(): ActiveSessionEntry | null { } // 2. Orphaned by a restart. Adopt at most one — rebinding every dead-PID - // session would leave several rows sharing hostname+pid. + // session would leave several rows sharing hostname+pid+project. const orphaned = candidates.filter((c) => !c.pidAlive).sort(newestFirst)[0]; if (!orphaned) return null; @@ -382,9 +406,7 @@ function reconcileRegistryEntry(entry: ActiveSessionEntry): void { return; // already agrees — no write } registry.sessions = registry.sessions.filter( - (s) => - s.session_id !== entry.session_id && - !(s.hostname === entry.hostname && s.pid === entry.pid) + (s) => s.session_id !== entry.session_id && !sameSlot(s, entry) ); registry.sessions.push(entry); writeRegistry(registry); diff --git a/src/services/gitmem-dir.ts b/src/services/gitmem-dir.ts index 4451b7e..feba098 100644 --- a/src/services/gitmem-dir.ts +++ b/src/services/gitmem-dir.ts @@ -179,15 +179,52 @@ export function describeGitmemRoot(root: string): GitmemRootContents { }; } +/** + * GIT-107: a path's canonical spelling, for comparing directories. + * + * Roots used to be compared as strings, but one directory has many spellings: + * on macOS /var is a symlink to /private/var, so a home under $TMPDIR + * (/var/folders/...) and a cwd reported as /private/var/folders/... named the + * same store and still read as different — and session_start announced + * "Memory store found but NOT being read" about the store it was reading. + * + * realpathSync.native resolves every symlink. A path that does not exist yet + * (a destination root before its first write) resolves through its nearest + * existing ancestor, so /var/x/.gitmem and /private/var/x/.gitmem still agree. + */ +export function canonicalPath(p: string): string { + const abs = path.resolve(p); + const rest: string[] = []; + let dir = abs; + for (;;) { + try { + return path.join(fs.realpathSync.native(dir), ...rest.reverse()); + } catch { + const parent = path.dirname(dir); + if (parent === dir) return abs; // nothing on the path exists + rest.push(path.basename(dir)); + dir = parent; + } + } +} + +/** GIT-107: the same directory, however it is spelled. */ +export function sameDirectory(a: string, b: string): boolean { + return canonicalPath(a) === canonicalPath(b); +} + export function findStrandedProjectRoots(): string[] { try { - const home = getHomeGitmemDir(); + // A root is only stranded if it is not the one being read: neither the + // developer-scoped home nor an explicit GITMEM_DIR. (process.env, not + // getGitmemDir(): that call is what triggers this scan.) + const readRoots = [getHomeGitmemDir(), ...(process.env.GITMEM_DIR ? [process.env.GITMEM_DIR] : [])]; const stranded: string[] = []; let dir = process.cwd(); const fsRoot = path.parse(dir).root; while (dir !== fsRoot) { const candidate = path.join(dir, ".gitmem"); - if (candidate !== home && isLiveGitmemRoot(candidate)) stranded.push(candidate); + if (!readRoots.some((r) => sameDirectory(candidate, r)) && isLiveGitmemRoot(candidate)) stranded.push(candidate); dir = path.dirname(dir); } return stranded; diff --git a/src/services/metrics.ts b/src/services/metrics.ts index 0211b88..16eac35 100644 --- a/src/services/metrics.ts +++ b/src/services/metrics.ts @@ -135,6 +135,48 @@ export class Timer { } } +/** + * GIT-73: session rows still being created, by session id. + * + * session_start creates its session row fire-and-forget and records its own + * metrics at the same moment, and gitmem_query_metrics.session_id references + * gitmem_sessions(id). Whichever request reached the store first decided + * whether the metrics insert 409'd (FK violation). recall right after + * session_start raced the same way. + * + * A metrics write for a session registered here waits for its row. If the row + * did not land (or takes longer than the wait budget), the metrics row is + * written without the FK and keeps the id in metadata.session_id — where + * session_close already looks for it (GIT-109) — instead of being lost to a 409. + * Entries stay after they settle so later writes know the answer too. + */ +const pendingSessionRows = new Map>(); +const SESSION_ROW_WAIT_MS = 15_000; + +export function registerPendingSessionRow(sessionId: string, created: Promise): void { + pendingSessionRows.set(sessionId, created.catch(() => false)); +} + +/** For tests. */ +export function clearPendingSessionRows(): void { + pendingSessionRows.clear(); +} + +async function sessionRowLanded(sessionId: string): Promise { + const pending = pendingSessionRows.get(sessionId); + if (!pending) return undefined; // not created by this process — nothing to wait for + let timer: ReturnType | undefined; + const budget = new Promise((resolve) => { + timer = setTimeout(() => resolve(false), SESSION_ROW_WAIT_MS); + timer.unref?.(); + }); + try { + return await Promise.race([pending, budget]); + } finally { + if (timer) clearTimeout(timer); + } +} + /** * Track a query and record metrics. * @@ -146,9 +188,17 @@ export async function recordMetrics(metrics: QueryMetrics): Promise { // Auto-detect agent if not provided by caller const agent = metrics.agent || getAgentIdentity() || null; + // GIT-73: never reference a session row that is not there yet. + let sessionId = metrics.session_id || null; + let metadata = metrics.metadata || {}; + if (sessionId && (await sessionRowLanded(sessionId)) === false) { + metadata = { ...metadata, session_id: sessionId }; + sessionId = null; + } + const record: Record = { id: metrics.id, - session_id: metrics.session_id || null, + session_id: sessionId, agent, tool_name: metrics.tool_name, query_text: metrics.query_text || null, @@ -160,7 +210,7 @@ export async function recordMetrics(metrics: QueryMetrics): Promise { phase_tag: metrics.phase_tag || null, linear_issue: metrics.linear_issue || null, memories_surfaced: metrics.memories_surfaced || null, - metadata: metrics.metadata || {}, + metadata, created_at: new Date().toISOString(), }; diff --git a/src/services/session-state.ts b/src/services/session-state.ts index 7d342de..fc021bf 100644 --- a/src/services/session-state.ts +++ b/src/services/session-state.ts @@ -78,12 +78,12 @@ let lastFailedRecoveryFingerprint: number | null | undefined = undefined; * Set the current active session * Called by session_start */ -export function setCurrentSession(context: Omit & { surfacedScars?: SurfacedScar[]; observations?: Observation[]; children?: SessionChild[]; threads?: ThreadObject[] }): void { +export function setCurrentSession(context: Omit & { surfacedScars?: SurfacedScar[]; confirmations?: ScarConfirmation[]; observations?: Observation[]; children?: SessionChild[]; threads?: ThreadObject[] }): void { currentSession = { ...context, recallCalled: false, surfacedScars: context.surfacedScars || [], - confirmations: [], + confirmations: context.confirmations || [], reflections: [], observations: context.observations || [], children: context.children || [], diff --git a/src/services/startup.ts b/src/services/startup.ts index 3cd2232..f9bee20 100644 --- a/src/services/startup.ts +++ b/src/services/startup.ts @@ -15,8 +15,9 @@ import * as fs from "fs"; import * as path from "path"; -import { isConfigured, loadScarsWithEmbeddings, getLearningsFingerprint, getSupabaseUrl } from "./supabase-client.js"; -import { computeStoreKey, readVectorCache, writeVectorCache, tryAcquireDownloadLock, waitForLeaderCache } from "./vector-disk-cache.js"; +import { isConfigured, loadScarsWithEmbeddings, loadLearningsManifest, loadLearningsByIds, getLearningsFingerprint, getSupabaseUrl } from "./supabase-client.js"; +import { computeStoreKey, readVectorCache, readVectorCacheBase, planDelta, assembleDelta, writeVectorCache, tryAcquireDownloadLock, waitForLeaderCache } from "./vector-disk-cache.js"; +import type { StoreFingerprint } from "./vector-disk-cache.js"; import { getTableName } from "./tier.js"; import { getGitmemDir } from "./gitmem-dir.js"; import { @@ -103,6 +104,38 @@ interface ScarWithEmbedding { [key: string]: unknown; } +/** Rows the bulk load and the delta manifest both cap at. */ +const INDEX_ROW_LIMIT = 500; + +/** + * GIT-98 per-row delta: bring a cache from this store up to date by fetching + * only the rows whose updated_at moved. Returns null — and the caller does the + * full download — when there is no base to reuse or anything goes wrong. + */ +async function deltaSyncFromCache(storeKey: string, fingerprint: StoreFingerprint): Promise { + const base = readVectorCacheBase(storeKey); + if (!base || base.length === 0) return null; + const startTime = Date.now(); + try { + const manifest = await loadLearningsManifest(INDEX_ROW_LIMIT); + // An empty manifest is more likely a transient failure than an emptied + // store; let the full download decide. + if (manifest.length === 0) return null; + const plan = planDelta(base, manifest); + const fetched = plan.fetchIds.length > 0 ? await loadLearningsByIds(plan.fetchIds) : []; + const rows = assembleDelta(manifest, plan, fetched); + console.error( + `[startup] Vector cache DELTA: ${rows.length} learnings in ${Date.now() - startTime}ms — ` + + `${plan.reuse.size} reused from disk, ${fetched.length} fetched, ${plan.dropped} dropped (bulk download skipped)` + ); + if (rows.length > 0) writeVectorCache(storeKey, fingerprint, rows); + return rows; + } catch (error) { + console.error("[startup] Delta sync failed, falling back to full download:", error instanceof Error ? error.message : error); + return null; + } +} + /** * Load all learnings with embeddings from Supabase * @@ -112,7 +145,8 @@ interface ScarWithEmbedding { * NOTE: Now loads all learning types (scars, patterns, wins, anti-patterns), * not just scars. This fixes the issue where ~64 patterns were being ignored. */ -async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean } = {}): Promise<{ +/** @internal exported for tests (GIT-98 delta sync). */ +export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean } = {}): Promise<{ scars: ScarWithEmbedding[]; latestUpdatedAt: string | null; source: "disk" | "network"; @@ -155,11 +189,20 @@ async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean } = {} } releaseLock = releaseLock ?? (() => {}); - console.error(`[startup] Loading ALL learnings with embeddings from Supabase (cross-project)`); - try { + // GIT-98: a miss is usually a handful of changed rows, not a new store. + // cache-flush (bypassDiskCache) always takes the full download. + if (!options.bypassDiskCache && fingerprint.count >= 0) { + const synced = await deltaSyncFromCache(storeKey, fingerprint); + if (synced) { + return { scars: synced, latestUpdatedAt: synced[0]?.updated_at || null, source: "network" }; + } + } + + console.error(`[startup] Loading ALL learnings with embeddings from Supabase (cross-project)`); + // Load ALL learnings across projects — semantic similarity handles relevance - const learnings = await loadScarsWithEmbeddings(undefined, 500); + const learnings = await loadScarsWithEmbeddings(undefined, INDEX_ROW_LIMIT); const elapsed = Date.now() - startTime; console.error(`[startup] Loaded ${learnings.length} learnings in ${elapsed}ms`); diff --git a/src/services/supabase-client.ts b/src/services/supabase-client.ts index a420f69..ca1855a 100644 --- a/src/services/supabase-client.ts +++ b/src/services/supabase-client.ts @@ -518,6 +518,18 @@ export async function directPatch( * * NOTE: Changed from "scars only" to "all learning types" */ +/** + * The learnings the local index is built from. The bulk load, the fingerprint, + * the delta manifest and the delta fetch MUST all use these, or they describe + * different sets (GIT-98). + */ +const INDEXED_LEARNING_FILTERS: Record = { + learning_type: "in.(scar,pattern,win,anti_pattern)", + is_active: "eq.true", +}; +const INDEXED_LEARNING_COLUMNS = + "id,title,description,severity,counter_arguments,applies_when,source_linear_issue,project,embedding,updated_at,learning_type,decay_multiplier"; + export async function loadScarsWithEmbeddings( project?: string, limit = 500 @@ -526,16 +538,13 @@ export async function loadScarsWithEmbeddings( const startTime = Date.now(); try { - const filters: Record = { - learning_type: "in.(scar,pattern,win,anti_pattern)", - is_active: "eq.true", - }; + const filters: Record = { ...INDEXED_LEARNING_FILTERS }; if (project) { filters.project = project; } const learnings = await directQuery(getTableName("learnings"), { - select: "id,title,description,severity,counter_arguments,applies_when,source_linear_issue,project,embedding,updated_at,learning_type,decay_multiplier", + select: INDEXED_LEARNING_COLUMNS, filters, order: "updated_at.desc", limit, @@ -551,6 +560,40 @@ export async function loadScarsWithEmbeddings( } } +/** + * Delta manifest (GIT-98): the id and updated_at of every row the bulk load + * would return — same filters, order and limit — and nothing else. About 70 + * bytes a row, versus ~20 KB a row with its 1536-dim embedding. + */ +export async function loadLearningsManifest(limit = 500): Promise> { + return directQuery<{ id: string; updated_at: string | null }>(getTableName("learnings"), { + select: "id,updated_at", + filters: { ...INDEXED_LEARNING_FILTERS }, + order: "updated_at.desc", + limit, + }); +} + +/** Ids per delta request. A UUID is 36 characters, so this keeps URLs near 4 KB. */ +const DELTA_FETCH_CHUNK = 100; + +/** + * Delta fetch (GIT-98): full rows, embedding included, for the given ids only. + * The indexed-learning filters still apply, so a row deactivated since the + * manifest was read is simply absent. + */ +export async function loadLearningsByIds(ids: string[]): Promise { + const rows: T[] = []; + for (let i = 0; i < ids.length; i += DELTA_FETCH_CHUNK) { + const chunk = ids.slice(i, i + DELTA_FETCH_CHUNK); + rows.push(...await directQuery(getTableName("learnings"), { + select: INDEXED_LEARNING_COLUMNS, + filters: { ...INDEXED_LEARNING_FILTERS, id: `in.(${chunk.join(",")})` }, + })); + } + return rows; +} + /** * Cheap fingerprint of the learnings the local index is built from (GIT-98). * @@ -569,8 +612,7 @@ export async function getLearningsFingerprint(): Promise<{ count: number; latest try { const url = new URL(`${SUPABASE_REST_URL}/${getTableName("learnings")}`); url.searchParams.set("select", "updated_at"); - url.searchParams.set("learning_type", "in.(scar,pattern,win,anti_pattern)"); - url.searchParams.set("is_active", "eq.true"); + for (const [key, value] of Object.entries(INDEXED_LEARNING_FILTERS)) url.searchParams.set(key, value); url.searchParams.set("order", "updated_at.desc"); url.searchParams.set("limit", "1"); diff --git a/src/services/triple-writer.ts b/src/services/triple-writer.ts index 81d8410..009560c 100644 --- a/src/services/triple-writer.ts +++ b/src/services/triple-writer.ts @@ -12,7 +12,7 @@ import { v4 as uuidv4 } from "uuid"; import * as supabase from "./supabase-client.js"; -import { hasSupabase } from "./tier.js"; +import { hasSupabase, getTableName } from "./tier.js"; // --- Types --- @@ -32,7 +32,8 @@ interface TripleCandidate { predicate: Predicate; object: string; source_type: string; - source_id: string; + /** knowledge_triples.source_id is a UUID column: a row id, or null. */ + source_id: string | null; source_linear_issue?: string; domain?: string[]; project: string; @@ -282,11 +283,24 @@ export function extractDecisionTriples(params: DecisionTripleParams): TripleCand * failure (GIT-104) — callers run this under getEffectTracker().track(), which * never rethrows. */ +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + export async function writeTriples(candidates: TripleCandidate[]): Promise { if (candidates.length === 0 || !hasSupabase()) { return 0; } + // GIT-105: source_id is a UUID column in every schema. A non-UUID value + // (thread ids are "t-xxxxxxxx") failed the whole row with 22P02, so every + // thread triple 400'd and was lost. Never send one; the subject/object + // labels still carry the relation. + for (const c of candidates) { + if (c.source_id !== null && !UUID_RE.test(c.source_id)) { + console.error(`[triple-writer] source_id "${c.source_id}" is not a UUID — writing the triple with source_id null`); + c.source_id = null; + } + } + let written = 0; const errors: string[] = []; @@ -356,6 +370,12 @@ export function writeTriplesForDecision(params: DecisionTripleParams): Promise { - const triples = extractThreadCreationTriples(params); +export async function writeTriplesForThreadCreation(params: ThreadCreationTripleParams): Promise { + const triples = extractThreadCreationTriples(await withThreadRowId(params)); return writeTriples(triples); } +/** + * GIT-105: fill thread_row_id from the store when the caller did not have it. + * A failed or empty lookup leaves it null — the triple is still written. + */ +async function withThreadRowId(params: T): Promise { + if (params.thread_row_id !== undefined || !hasSupabase()) return params; + try { + const rows = await supabase.directQuery<{ id: string }>(getTableName("threads"), { + select: "id", + filters: { thread_id: params.thread_id }, + limit: 1, + }); + return { ...params, thread_row_id: rows[0]?.id ?? null }; + } catch (error) { + console.error(`[triple-writer] Could not look up the row id of ${params.thread_id}:`, error instanceof Error ? error.message : error); + return { ...params, thread_row_id: null }; + } +} + /** * Generate and write triples for a resolved thread. * Fire-and-forget — run under getEffectTracker().track(); rejects if any triple failed. */ -export function writeTriplesForThreadResolution(params: ThreadResolutionTripleParams): Promise { - const triples = extractThreadResolutionTriples(params); +export async function writeTriplesForThreadResolution(params: ThreadResolutionTripleParams): Promise { + const triples = extractThreadResolutionTriples(await withThreadRowId(params)); return writeTriples(triples); } diff --git a/src/services/vector-disk-cache.ts b/src/services/vector-disk-cache.ts index 91853b9..490a4ba 100644 --- a/src/services/vector-disk-cache.ts +++ b/src/services/vector-disk-cache.ts @@ -12,7 +12,21 @@ * 1. Ask the store for a FINGERPRINT — row count + newest updated_at. One * request, one row, a few hundred bytes (PostgREST `Prefer: count=exact`). * 2. If a cache file with the same fingerprint exists, load vectors from disk. - * 3. Otherwise download as before and rewrite the cache. + * 3. Otherwise sync the difference (below) and rewrite the cache. + * + * ## Per-row delta sync + * + * A fingerprint miss used to mean a full download, and on a Pro store it + * misses on nearly every session_start: refresh_scar_behavioral_scores() + * rewrites every scar with enough recent usage and bumps its updated_at. So + * a handful of changed rows cost the whole index. + * + * On a miss with a usable cache from this store, only the manifest (id, + * updated_at; no vectors) is downloaded. A row whose id and updated_at match + * the cache is reused from disk; only changed and new ids are fetched in full; + * rows no longer in the manifest are dropped. The result is in manifest order, + * exactly what a full download would have returned, given that updated_at + * moves on every edit (the same assumption the fingerprint already makes). * * ## What the fingerprint can and cannot see * @@ -105,31 +119,104 @@ export function readVectorCache(storeKey: string, expected: StoreFingerprint) // failures would otherwise "agree" and serve whatever is on disk. if (expected.count < 0) return { hit: false, reason: "store fingerprint unavailable" }; + const file = readCacheFile(storeKey); + if (!file.ok) return { hit: false, reason: file.reason }; + if (!file.parsed.fingerprint || !fingerprintsMatch(file.parsed.fingerprint, expected)) { + return { hit: false, reason: "store changed since cache was written" }; + } + return { hit: true, rows: file.parsed.rows, bytesOnDisk: file.bytesOnDisk }; +} + +/** + * The cached rows of this store, whatever fingerprint they were written under: + * the base a delta sync reuses unchanged rows from. Never served as-is — every + * row is checked against the store's manifest first. + */ +export function readVectorCacheBase(storeKey: string): T[] | null { + if (!isVectorDiskCacheEnabled()) return null; + const file = readCacheFile(storeKey); + return file.ok ? file.parsed.rows : null; +} + +export interface ManifestRow { + id: string; + updated_at: string | null; +} + +export interface DeltaPlan { + /** Cached rows still current in the store, by id. */ + reuse: Map; + /** Ids to fetch in full: changed since the cache was written, or new. */ + fetchIds: string[]; + /** Cached rows the store no longer returns. */ + dropped: number; +} + +/** + * Compare the store's manifest with the cached rows. A row is reused only when + * its id AND updated_at match; a cached row without updated_at is refetched. + */ +export function planDelta( + cached: T[], + manifest: ManifestRow[] +): DeltaPlan { + const cachedById = new Map(cached.map((r) => [r.id, r])); + const reuse = new Map(); + const fetchIds: string[] = []; + for (const m of manifest) { + const c = cachedById.get(m.id); + if (c && m.updated_at != null && c.updated_at === m.updated_at) reuse.set(m.id, c); + else fetchIds.push(m.id); + } + const inManifest = new Set(manifest.map((m) => m.id)); + const dropped = cached.filter((r) => !inManifest.has(r.id)).length; + return { reuse, fetchIds, dropped }; +} + +/** + * Rows in manifest order: reused where current, fetched otherwise. A manifest + * id that is in neither (deactivated between the manifest and the fetch) is + * left out, as a full download at that moment would have. + */ +export function assembleDelta( + manifest: ManifestRow[], + plan: DeltaPlan, + fetched: T[] +): T[] { + const fetchedById = new Map(fetched.map((r) => [r.id, r])); + const rows: T[] = []; + for (const m of manifest) { + const row = fetchedById.get(m.id) ?? plan.reuse.get(m.id); + if (row) rows.push(row); + } + return rows; +} + +function readCacheFile(storeKey: string): + | { ok: true; parsed: VectorCacheFile; bytesOnDisk: number } + | { ok: false; reason: string } { const file = getVectorCachePath(storeKey); let raw: string; try { - if (!fs.existsSync(file)) return { hit: false, reason: "no cache file" }; + if (!fs.existsSync(file)) return { ok: false, reason: "no cache file" }; raw = fs.readFileSync(file, "utf-8"); } catch (error) { - return { hit: false, reason: `unreadable: ${error instanceof Error ? error.message : String(error)}` }; + return { ok: false, reason: `unreadable: ${error instanceof Error ? error.message : String(error)}` }; } let parsed: VectorCacheFile; try { parsed = JSON.parse(raw) as VectorCacheFile; } catch { - return { hit: false, reason: "corrupt JSON" }; + return { ok: false, reason: "corrupt JSON" }; } - if (!parsed || typeof parsed !== "object") return { hit: false, reason: "corrupt shape" }; - if (parsed.format !== VECTOR_CACHE_FORMAT) return { hit: false, reason: `format ${parsed.format} != ${VECTOR_CACHE_FORMAT}` }; - if (parsed.storeKey !== storeKey) return { hit: false, reason: "written for a different store" }; - if (!Array.isArray(parsed.rows)) return { hit: false, reason: "corrupt rows" }; - if (!parsed.fingerprint || !fingerprintsMatch(parsed.fingerprint, expected)) { - return { hit: false, reason: "store changed since cache was written" }; - } + if (!parsed || typeof parsed !== "object") return { ok: false, reason: "corrupt shape" }; + if (parsed.format !== VECTOR_CACHE_FORMAT) return { ok: false, reason: `format ${parsed.format} != ${VECTOR_CACHE_FORMAT}` }; + if (parsed.storeKey !== storeKey) return { ok: false, reason: "written for a different store" }; + if (!Array.isArray(parsed.rows)) return { ok: false, reason: "corrupt rows" }; - return { hit: true, rows: parsed.rows, bytesOnDisk: Buffer.byteLength(raw) }; + return { ok: true, parsed, bytesOnDisk: Buffer.byteLength(raw) }; } /** diff --git a/src/services/write-health.ts b/src/services/write-health.ts index a424d76..40500d0 100644 --- a/src/services/write-health.ts +++ b/src/services/write-health.ts @@ -17,6 +17,12 @@ * Fire-and-forget: never throws (whole body is guarded), never blocks startup. * Logs a loud warning to stderr when misconfigured, a one-line confirmation * otherwise. + * + * GIT-102: the probe had no timeout. A store that accepts the connection and + * never answers left it pending forever — no warning, no confirmation, and + * nothing anywhere saying durability had not been checked. The server now runs + * it through checkWritePathWithTimeout(): raced against 10 s, with a + * "timed_out" verdict that says so, and the last verdict kept for `health`. */ import { getTier, hasSupabase, getTablePrefix, getTableName } from "./tier.js"; @@ -30,12 +36,115 @@ export type WritePathMode = | "free_with_credentials" // creds present but tier free — writes silently local (bug) | "missing_tables" // pro/dev but resolved tables absent (bug) | "supabase" // healthy: pro/dev with tables present - | "skipped"; // unexpected internal error — stayed silent, did not block startup + | "skipped" // unexpected internal error — stayed silent, did not block startup + | "timed_out" // GIT-102: no answer within the budget — durability UNVERIFIED + | "unreachable"; // GIT-102: probes errored (network/auth) — durability UNVERIFIED export interface WritePathResult { ok: boolean; mode: WritePathMode; missing?: string[]; + /** unreachable: the probe error, as the store reported it. */ + error?: string; +} + +/** The most recent write-path verdict, for `health` (GIT-102). */ +export interface WritePathVerdict extends WritePathResult { + /** One line a person can act on. */ + summary: string; + checked_at: string; + duration_ms: number; + /** A check that finished after its timeout had already been reported. */ + late?: boolean; +} + +export const WRITE_PATH_TIMEOUT_MS = 10_000; + +let lastVerdict: WritePathVerdict | null = null; + +/** The last write-path verdict, or null when no check has finished or timed out yet. */ +export function getLastWritePathVerdict(): WritePathVerdict | null { + return lastVerdict; +} + +/** For tests. */ +export function resetWritePathVerdict(): void { + lastVerdict = null; +} + +/** The `health` line for a verdict (GIT-102). */ +export function formatWritePathLine(v: WritePathVerdict | null): string { + if (!v) return "Write path: not checked yet (the startup check has not finished or timed out)"; + return `Write path: ${v.summary} (${v.ok ? "ok" : "NOT OK"}, checked ${v.checked_at}, ${v.duration_ms} ms` + + `${v.late ? ", finished after its timeout" : ""})`; +} + +function summarize(r: WritePathResult, timeoutMs: number): string { + switch (r.mode) { + case "supabase": return "Supabase — learnings/decisions tables present, writes are durable"; + case "local": return "local files (free tier, no Supabase configured) — intended"; + case "free_with_credentials": return "Supabase is configured but the tier resolved to FREE — writes are going to local files, NOT Supabase"; + case "missing_tables": return `tables missing on the Supabase backend (${(r.missing || []).join(", ")}) — create_learning / create_decision will fail`; + case "skipped": return "check skipped after an internal error — durability UNVERIFIED"; + case "timed_out": return `timed out after ${Math.round(timeoutMs / 1000)} s, durability UNVERIFIED`; + case "unreachable": return `store unreachable (${r.error ?? "probe failed"}), durability UNVERIFIED`; + } +} + +/** + * Run checkWritePath() against a deadline and record the verdict. + * + * Resolves by the deadline at the latest. If the check finishes afterwards, + * its real verdict replaces "timed_out" (marked late) — the store answered + * slowly, and the latest word on it is the one health should show. + */ +export async function checkWritePathWithTimeout( + timeoutMs: number = WRITE_PATH_TIMEOUT_MS, + check: () => Promise = checkWritePath +): Promise { + const started = Date.now(); + let timedOut = false; + let timer: ReturnType | undefined; + + const record = (r: WritePathResult, late = false): WritePathVerdict => { + lastVerdict = { + ...r, + summary: summarize(r, timeoutMs), + checked_at: new Date().toISOString(), + duration_ms: Date.now() - started, + ...(late && { late: true }), + }; + return lastVerdict; + }; + + const checked = check().then((r) => { + if (timedOut) { + const v = record(r, true); + console.error(`[gitmem] Write-path check finished late (${v.duration_ms} ms): ${v.summary}`); + return v; + } + return record(r); + }); + + const deadline = new Promise((resolve) => { + timer = setTimeout(() => { + timedOut = true; + const v = record({ ok: false, mode: "timed_out" }); + console.error( + `\n\u26a0\ufe0f [gitmem] WRITE PATH: ${v.summary}.\n` + + " The Supabase store did not answer the write-path probe. Writes may or may not be landing;\n" + + " check connectivity to SUPABASE_URL. `health` shows the latest verdict.\n" + ); + resolve(v); + }, timeoutMs); + timer.unref?.(); + }); + + try { + return await Promise.race([checked, deadline]); + } finally { + if (timer) clearTimeout(timer); + } } export async function checkWritePath(): Promise { @@ -65,6 +174,7 @@ export async function checkWritePath(): Promise { const prefix = getTablePrefix(); const prefixSource = process.env.GITMEM_TABLE_PREFIX ? "GITMEM_TABLE_PREFIX" : "default"; const missing: string[] = []; + const probeErrors: string[] = []; for (const base of ["learnings", "decisions"]) { const table = getTableName(base); @@ -73,7 +183,9 @@ export async function checkWritePath(): Promise { } catch (err) { const msg = err instanceof Error ? err.message : String(err); if (SCHEMA_MISS.test(msg)) missing.push(table); - // Transient network/auth errors are out of scope for this check. + // GIT-102: a network/auth error used to be ignored here, so a store + // that refused every connection was reported as "Write-path OK". + else probeErrors.push(`${table}: ${msg}`); } } @@ -89,6 +201,15 @@ export async function checkWritePath(): Promise { return { ok: false, mode: "missing_tables", missing }; } + if (probeErrors.length > 0) { + const error = probeErrors.join("; ").slice(0, 300); + console.error( + "\n\u26a0\ufe0f [gitmem] WRITE PATH: the Supabase store could not be probed — durability UNVERIFIED.\n" + + ` ${error}\n` + ); + return { ok: false, mode: "unreachable", error }; + } + console.error( `[gitmem] Write-path OK (tier ${getTier()}, prefix "${prefix}", learnings/decisions present).` ); diff --git a/src/services/write-result.ts b/src/services/write-result.ts new file mode 100644 index 0000000..850fb8a --- /dev/null +++ b/src/services/write-result.ts @@ -0,0 +1,47 @@ +/** + * WriteResult — the one contract every write tool answers with (GIT-101). + * + * create_thread established it (GIT-63/GIT-67, R3/R4): say where the record + * landed and whether it survives this machine, and only call it a success + * when it reached the store of truth for this tier. Other write tools grew + * their own shapes — resolve_thread returned success: true whether or not the + * durable store accepted the write — so a caller could not tell a saved + * record from a lost one without reading prose. + * + * durable the record is in Supabase (pro/dev). Never true on free. + * stored_in "supabase" | "local" (free: the local store IS the truth) | + * "local_only" (pro: kept on this machine only, the durable + * write failed) | null (stored nowhere). + * success durable || !hasSupabase() — and false whenever nothing was + * stored at all. + */ + +import { hasSupabase } from "./tier.js"; +import type { WriteResult, StoredIn } from "../types/index.js"; + +export type { WriteResult, StoredIn }; + +/** + * The outcome of a write. + * + * @param durable the durable store (Supabase) accepted the write + * @param storedLocally a local copy exists (threads.json, the free-tier + * store, session state). Default: true on free, where + * reaching this call means the local write succeeded; + * false on pro, where a failed durable write keeps + * nothing unless the tool says it did. + */ +export function writeResult(durable: boolean, storedLocally: boolean = !hasSupabase()): WriteResult { + if (durable && hasSupabase()) return { success: true, durable: true, stored_in: "supabase" }; + if (!hasSupabase()) { + return storedLocally + ? { success: true, durable: false, stored_in: "local" } + : notStored(); + } + return { success: false, durable: false, stored_in: storedLocally ? "local_only" : null }; +} + +/** Nothing was stored anywhere. */ +export function notStored(): WriteResult { + return { success: false, durable: false, stored_in: null }; +} diff --git a/src/tools/archive-learning.ts b/src/tools/archive-learning.ts index 898413d..eeb020e 100644 --- a/src/tools/archive-learning.ts +++ b/src/tools/archive-learning.ts @@ -16,6 +16,8 @@ import { getStorage } from "../services/storage.js"; import { flushCache } from "../services/startup.js"; import { Timer } from "../services/metrics.js"; import { wrapDisplay } from "../services/display-protocol.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import type { WriteResult } from "../types/index.js"; export interface ArchiveLearningParams { /** UUID or short ID prefix of the learning to archive */ @@ -24,8 +26,7 @@ export interface ArchiveLearningParams { reason?: string; } -export interface ArchiveLearningResult { - success: boolean; +export interface ArchiveLearningResult extends WriteResult { id: string; archived_at?: string; reason?: string; @@ -95,7 +96,7 @@ export async function archiveLearning(params: ArchiveLearningParams): Promise {}); return { - success: true, + ...writeResult(hasSupabase()), decision_id: decisionId, performance: perfData, display: wrapDisplay(`Decision logged: "${params.title}"\nID: ${decisionId}`), @@ -151,7 +152,7 @@ export async function createDecision( const latencyMs = timer.stop(); const perfData = buildPerformanceData("create_decision", latencyMs, 0); return { - success: false, + ...notStored(), decision_id: "", performance: perfData, display: wrapDisplay(`Failed to log decision`), diff --git a/src/tools/create-learning.ts b/src/tools/create-learning.ts index b65a660..db9b929 100644 --- a/src/tools/create-learning.ts +++ b/src/tools/create-learning.ts @@ -32,6 +32,7 @@ import type { PerformanceBreakdown, ComponentPerformance, } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; /** * Validate scar-specific requirements @@ -85,7 +86,7 @@ export async function createLearning( const latencyMs = timer.stop(); const perfData = buildPerformanceData("create_learning", latencyMs, 0); return { - success: false, + ...notStored(), learning_id: "", embedding_generated: false, errors, @@ -314,7 +315,7 @@ export async function createLearning( const se = params.severity ? (SEV[params.severity] || "") + " " : ""; return { - success: true, + ...writeResult(hasSupabase()), learning_id: learningId, embedding_generated: embeddingGenerated, performance: perfData, @@ -327,7 +328,7 @@ export async function createLearning( const latencyMs = timer.stop(); const perfData = buildPerformanceData("create_learning", latencyMs, 0); return { - success: false, + ...notStored(), learning_id: "", embedding_generated: false, errors: [errorMsg], diff --git a/src/tools/create-thread.ts b/src/tools/create-thread.ts index 691de05..8ac846d 100644 --- a/src/tools/create-thread.ts +++ b/src/tools/create-thread.ts @@ -41,7 +41,7 @@ import { import { wrapDisplay, truncate } from "../services/display-protocol.js"; import { formatThreadForDisplay } from "../services/timezone.js"; import type { ThreadWithEmbedding } from "../services/thread-dedup.js"; -import type { ThreadObject, PerformanceData, Project } from "../types/index.js"; +import type { ThreadObject, PerformanceData, Project, WriteResult, StoredIn } from "../types/index.js"; // --- Types --- @@ -73,21 +73,17 @@ export type NotStoredReason = /** * Where a thread actually landed (R3: every response names its store). * `local_only` is the honest middle state from R4 — written somewhere, but - * not somewhere durable. + * not somewhere durable. GIT-101: this contract is now shared by every write + * tool (WriteResult). */ -export type StoredIn = "supabase" | "local" | "local_only" | null; +export type { StoredIn } from "../types/index.js"; -export interface CreateThreadResult { - success: boolean; +export interface CreateThreadResult extends WriteResult { /** * Whether a row exists. Distinct from `success` on purpose: a dedup refusal * is a well-formed answer to a well-formed request, not a crash. */ stored: boolean; - /** Whether what was stored survives this machine. */ - durable: boolean; - /** R3: names the store in-band, which also kills the f104e10d silent-local trap. */ - stored_in: StoredIn; reason?: NotStoredReason; thread?: ThreadObject; error?: string; @@ -296,12 +292,14 @@ export async function createThread( // Write to Supabase (source of truth) with embedding. let supabaseSynced = false; let supabaseError: string | undefined; + let threadRowId: string | null = null; // GIT-105: gitmem_threads.id, for triple source_id const embeddingJson = newEmbedding ? JSON.stringify(newEmbedding) : null; if (hasSupabase()) { try { const supabaseResult = await createThreadInSupabase(thread, project, embeddingJson); supabaseSynced = Boolean(supabaseResult); + threadRowId = supabaseResult?.id ?? null; if (!supabaseSynced) { supabaseError = "write returned no row"; } @@ -347,6 +345,7 @@ export async function createThread( getEffectTracker().track("triple_write", "thread_creation", () => writeTriplesForThreadCreation({ thread_id: thread.id, + thread_row_id: threadRowId, text: trimmedText, linear_issue: params.linear_issue, session_id: sessionId, diff --git a/src/tools/promote-suggestion.ts b/src/tools/promote-suggestion.ts index 1d00b5f..1b359d2 100644 --- a/src/tools/promote-suggestion.ts +++ b/src/tools/promote-suggestion.ts @@ -22,6 +22,8 @@ import { buildPerformanceData, } from "../services/metrics.js"; import type { ThreadObject, PerformanceData, Project, ThreadSuggestion } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import type { WriteResult } from "../types/index.js"; // --- Types --- @@ -30,8 +32,7 @@ export interface PromoteSuggestionParams { project?: Project; } -export interface PromoteSuggestionResult { - success: boolean; +export interface PromoteSuggestionResult extends WriteResult { thread?: ThreadObject; suggestion?: ThreadSuggestion; error?: string; @@ -49,7 +50,7 @@ export async function promoteSuggestion( if (!params.suggestion_id) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "suggestion_id is required", performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 0), display: wrapDisplay(`Failed: suggestion_id is required`), @@ -64,7 +65,7 @@ export async function promoteSuggestion( if (!target) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: `Pending suggestion not found: "${params.suggestion_id}"`, performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 0), display: wrapDisplay(`Suggestion not found: ${params.suggestion_id}`), @@ -79,8 +80,12 @@ export async function promoteSuggestion( if (!threadResult.success || !threadResult.thread) { const latencyMs = timer.stop(); + // GIT-101: pass create_thread's own outcome through (a local_only thread + // is not a promotion), rather than restating it. return { - success: false, + ...notStored(), + durable: threadResult.durable, + stored_in: threadResult.stored_in, error: `Thread creation failed: ${threadResult.error || "unknown"}`, performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 0), display: wrapDisplay(`Failed to promote suggestion`), @@ -93,7 +98,9 @@ export async function promoteSuggestion( const latencyMs = timer.stop(); return { - success: true, + success: threadResult.success, + durable: threadResult.durable, + stored_in: threadResult.stored_in, thread: threadResult.thread, suggestion: target, performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 1), diff --git a/src/tools/record-scar-usage-batch.ts b/src/tools/record-scar-usage-batch.ts index d364a08..5c634b7 100644 --- a/src/tools/record-scar-usage-batch.ts +++ b/src/tools/record-scar-usage-batch.ts @@ -15,6 +15,7 @@ import type { ScarUsageEntry, Project, } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; const TARGET_LATENCY_MS = 2000; // Target for batch operation @@ -98,9 +99,13 @@ export async function recordScarUsageBatch( const metricsId = uuidv4(); if (!hasSupabase()) { - // Free tier: scar usage tracked locally via record-scar-usage (single), not batch + // Free tier: scar usage tracked locally via record-scar-usage (single), not batch. + // GIT-101: nothing is stored here, and the result says so (stored_in: null); + // success stays true because there is no durable store to have missed. return { success: true, + durable: false, + stored_in: null, usage_ids: [], resolved_count: 0, failed_count: 0, @@ -191,7 +196,7 @@ export async function recordScarUsageBatch( }); return { - success: true, + ...writeResult(hasSupabase()), usage_ids: usageIds, resolved_count: resolvedCount, failed_count: failedIdentifiers.length, @@ -205,7 +210,7 @@ export async function recordScarUsageBatch( console.error("[record-scar-usage-batch] Error recording batch:", error); return { - success: false, + ...notStored(), error: error instanceof Error ? error.message : String(error), usage_ids: usageIds, resolved_count: resolvedCount, diff --git a/src/tools/record-scar-usage.ts b/src/tools/record-scar-usage.ts index 40ca688..9b5daa9 100644 --- a/src/tools/record-scar-usage.ts +++ b/src/tools/record-scar-usage.ts @@ -24,6 +24,7 @@ import type { RecordScarUsageResult, PerformanceBreakdown, } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; /** * Execute record_scar_usage tool @@ -101,7 +102,7 @@ export async function recordScarUsage( }).catch(() => {}); return { - success: true, + ...writeResult(hasSupabase()), usage_id: usageId, performance: perfData, display: wrapDisplay(`Scar usage recorded\nID: ${usageId}`), @@ -112,7 +113,7 @@ export async function recordScarUsage( const latencyMs = timer.stop(); const perfData = buildPerformanceData("record_scar_usage", latencyMs, 0); return { - success: false, + ...notStored(), usage_id: "", errors: [errorMsg], performance: perfData, diff --git a/src/tools/resolve-thread.ts b/src/tools/resolve-thread.ts index 6e0e0fb..3fbbc78 100644 --- a/src/tools/resolve-thread.ts +++ b/src/tools/resolve-thread.ts @@ -44,6 +44,8 @@ import { buildPerformanceData, } from "../services/metrics.js"; import { wrapDisplay } from "../services/display-protocol.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import { hasSupabase } from "../services/tier.js"; import { formatThreadForDisplay } from "../services/timezone.js"; import type { ResolveThreadParams, ResolveThreadResult, ThreadObject } from "../types/index.js"; @@ -57,7 +59,7 @@ export async function resolveThread( if (!params.thread_id && !params.text_match) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "Either thread_id or text_match is required", performance: buildPerformanceData("resolve_thread", latencyMs, 0), display: wrapDisplay(`Either thread_id or text_match is required`), @@ -87,7 +89,7 @@ export async function resolveThread( if (pos < 1 || pos > openThreads.length) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: `Thread #${pos} out of range (${openThreads.length} open threads)`, performance: buildPerformanceData("resolve_thread", latencyMs, 0), display: wrapDisplay(`Thread #${pos} out of range (${openThreads.length} open threads)`), @@ -162,7 +164,7 @@ export async function resolveThread( : `Thread not found: "${searchKey}"`; return { - success: false, + ...notStored(), error: message, performance: buildPerformanceData("resolve_thread", latencyMs, 0), display: wrapDisplay(message), @@ -193,7 +195,9 @@ export async function resolveThread( // Persist to local file (cache) saveThreadsFile(threads); - // Update Supabase (source of truth) — graceful fallback on failure + // Update Supabase (source of truth). GIT-101: the outcome is the result — + // it used to be computed, logged to metrics and then dropped, so a resolve + // the durable store never saw still answered success: true. let supabaseSynced = false; const supabaseSuccess = await resolveThreadInSupabase(resolved.id, { resolvedAt: resolved.resolved_at, @@ -275,8 +279,18 @@ export async function resolveThread( if (alsoResolved.length > 0) resolveMsg += `\nAlso resolved: ${alsoResolved.map(t => t.id).join(", ")}`; + // The local file was written above either way; on pro that alone is not a success. + const outcome = writeResult(supabaseSynced, true); + if (hasSupabase() && !outcome.durable) { + resolveMsg = + `RESOLVED LOCALLY ONLY — not durable (stored_in: local_only, durable: false).\n` + + `${resolveMsg}\n` + + `The durable store did not accept this resolve, so other sessions still see ${resolved.id} as open. ` + + `Retry when the store is reachable.`; + } + return { - success: true, + ...outcome, resolved_thread: formatThreadForDisplay(resolved), ...(alsoResolved.length > 0 && { also_resolved: alsoResolved.map(formatThreadForDisplay), diff --git a/src/tools/save-transcript.ts b/src/tools/save-transcript.ts index 88a487d..b48e6f5 100644 --- a/src/tools/save-transcript.ts +++ b/src/tools/save-transcript.ts @@ -12,6 +12,8 @@ import * as supabase from "../services/supabase-client.js"; import { Timer, buildPerformanceData } from "../services/metrics.js"; import { getProject } from "../services/session-state.js"; import type { Project, PerformanceData, PerformanceBreakdown } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import type { WriteResult } from "../types/index.js"; export interface SaveTranscriptParams { session_id: string; @@ -20,8 +22,7 @@ export interface SaveTranscriptParams { project?: Project; } -export interface SaveTranscriptResult { - success: boolean; +export interface SaveTranscriptResult extends WriteResult { transcript_path?: string; size_bytes?: number; size_kb?: number; @@ -43,7 +44,7 @@ export async function saveTranscript( if (!params.session_id) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "session_id is required", performance: buildPerformanceData("save_transcript", latencyMs, 0), }; @@ -52,7 +53,7 @@ export async function saveTranscript( if (!params.transcript) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "transcript content is required", performance: buildPerformanceData("save_transcript", latencyMs, 0), }; @@ -108,7 +109,8 @@ export async function saveTranscript( const estimatedTokens = Math.ceil(result.size_bytes / 4); return { - success: true, + // Transcripts only go to the durable store; reaching here means it took them. + ...writeResult(true), transcript_path: result.transcript_path, size_bytes: result.size_bytes, size_kb: Math.round(result.size_bytes / 1024 * 10) / 10, @@ -122,7 +124,7 @@ export async function saveTranscript( const latencyMs = timer.stop(); const errorMessage = error instanceof Error ? error.message : String(error); return { - success: false, + ...notStored(), error: `Failed to save transcript: ${errorMessage}`, performance: buildPerformanceData("save_transcript", latencyMs, 0), }; diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index 2388a4c..f398efc 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -36,6 +36,7 @@ import { import { wrapDisplay, truncate, productLine, boldText, dimText, STATUS, ANSI } from "../services/display-protocol.js"; import { queryScarUsageByDateRange, enrichScarUsageTitles, formatBlindspotSnippet } from "../services/analytics.js"; import { recordScarUsageBatch } from "./record-scar-usage-batch.js"; +import { writeResult, notStored } from "../services/write-result.js"; import { getEffectTracker } from "../services/effect-tracker.js"; import { saveTranscript } from "./save-transcript.js"; import { processTranscript } from "../services/transcript-chunker.js"; @@ -43,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 } from "../services/active-sessions.js"; +import { unregisterSession, findSessionByHostPid, findSessionById } from "../services/active-sessions.js"; import { loadSuggestions, saveSuggestions, detectSuggestedThreads, loadRecentSessionEmbeddings } from "../services/thread-suggestions.js"; import { writeAgentBriefing } from "../services/agent-briefing.js"; import type { @@ -318,7 +319,7 @@ async function sessionCloseFree( const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, true); return { - success: true, + ...writeResult(false), session_id: sessionId, close_compliance: closeCompliance, performance: perfData, @@ -329,10 +330,10 @@ async function sessionCloseFree( clearCurrentSession(); const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); - const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`]); + const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], undefined, undefined, false); return { - success: false, + ...notStored(), session_id: sessionId, close_compliance: closeCompliance, validation_errors: [`Failed to persist session: ${errorMessage}`], @@ -363,6 +364,8 @@ function formatCloseDisplay( errors?: string[], transcriptStatus?: TranscriptStatus, blindspotSnippet?: string | null, + /** GIT-102: whether the session content itself landed; defaults to `success`. */ + sessionStored: boolean = success, ): string { const lines: string[] = []; @@ -434,15 +437,55 @@ function formatCloseDisplay( } // Write health — only on failure - const healthReport = getEffectTracker().getHealthReport(); - if (healthReport.overall.failed > 0) { + const warnings = formatWriteWarnings(getEffectTracker().getHealthReport(), sessionStored); + if (warnings.length > 0) { lines.push(""); - lines.push(`${STATUS.warn} ${healthReport.overall.failed} write failure${healthReport.overall.failed > 1 ? "s" : ""}`); + for (const w of warnings) lines.push(`${STATUS.warn} ${w}`); } return wrapDisplay(lines.join("\n")); } +/** GIT-102: what each tracked write path is, in words. */ +const SUBSYSTEM_NAMES: Record = { + triple_write: "knowledge graph (triples)", + embedding: "session embedding", + scar_usage: "scar usage records", + transcript: "transcript upload", + metrics: "query metrics", + relevance_update: "relevance feedback", + variant_generation: "scar variants", + cache_set: "local cache", +}; + +/** + * GIT-102: one self-describing line per failing write subsystem. + * + * The close used to print "WARN 3 write failures" and nothing else: not which + * writes, not why, and — the question every reader actually has — not whether + * the session itself was saved. Each line now names the subsystem, the cause + * (its most recent error), and states whether the session content was stored. + * That last clause is mandatory on every line, so no WARN can be read as + * "the close failed" when it didn't, or as harmless when it wasn't. + */ +export function formatWriteWarnings( + report: ReturnType["getHealthReport"]>, + sessionStored: boolean +): string[] { + const where = hasSupabase() ? "" : " (local)"; + const stored = sessionStored ? `session content stored OK${where}` : "session content NOT stored"; + const lines: string[] = []; + for (const [path, s] of Object.entries(report.byPath)) { + if (s.failed === 0) continue; + const name = SUBSYSTEM_NAMES[path] ?? path; + const cause = s.lastFailure?.error + ? truncate(s.lastFailure.error.replace(/\s+/g, " ").trim(), 160) + : "no error message recorded"; + lines.push(`${name}: ${s.failed} of ${s.attempted} write${s.attempted > 1 ? "s" : ""} failed — ${cause} · ${stored}`); + } + return lines; +} + /** * GIT-21: Clean up all session files for a closed session. * Unregisters from registry, deletes per-session directory, and removes legacy file. @@ -862,7 +905,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: params.session_id, close_compliance: { close_type: params.close_type, @@ -908,7 +951,7 @@ export async function sessionClose( if (!params.session_id && params.close_type !== "retroactive") { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), session_id: "", close_compliance: { close_type: params.close_type, @@ -932,6 +975,11 @@ 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; + // Whether the session row reached the durable store. Set only after the + // upsert resolves, so every exit can say so: its WriteResult (GIT-101) and + // the "session content stored OK / NOT stored" clause on each WARN (GIT-102). + let sessionRowStored = false; try { if (fs.existsSync(payloadPath)) { const filePayload = JSON.parse(fs.readFileSync(payloadPath, "utf-8")) as Partial; @@ -944,6 +992,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 { + ...notStored(), // pre-write exit: nothing stored + 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 @@ -1066,7 +1144,7 @@ export async function sessionClose( // Hard gate: quick close requires session under 30 minutes if (params.close_type === "quick" && activity.duration_min >= 30) { return { - success: false, + ...notStored(), session_id: params.session_id || "", close_compliance: { close_type: "quick", @@ -1099,7 +1177,7 @@ export async function sessionClose( // Exemptions: quick (micro sessions), autonomous (CODA-1), hasReflection (agent already wrote full reflection) if (params.close_type === "standard" && !isRecallCalled() && !hasReflection) { return { - success: false, + ...notStored(), session_id: params.session_id || "", close_compliance: { close_type: "standard", @@ -1155,7 +1233,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: "", // Empty string when no session found close_compliance: { close_type: params.close_type, @@ -1179,7 +1257,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: "", // Empty string when search fails close_compliance: { close_type: params.close_type, @@ -1206,7 +1284,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: params.session_id, close_compliance: { close_type: params.close_type, @@ -1285,8 +1363,10 @@ export async function sessionClose( // Fall back to active-sessions registry as last resort if (!existingSession) { try { - const mySession = findSessionByHostPid(os.hostname(), process.pid); - if (mySession && mySession.session_id === sessionId) { + // GIT-86: look up by id — a process can hold one session per project, + // so "this process's session" is not a single entry. + const mySession = findSessionById(sessionId); + if (mySession && mySession.hostname === os.hostname() && mySession.pid === process.pid) { existingSession = { id: sessionId, session_date: mySession.started_at?.split("T")[0] || new Date().toISOString().split("T")[0], @@ -1304,7 +1384,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: sessionId, close_compliance: closeCompliance, validation_errors: [`Session ${sessionId} not found in Supabase, local files, or registry. Was session_start called?`], @@ -1470,6 +1550,7 @@ export async function sessionClose( supabase.directUpsert(sessionsTable, sessionRow), blindspotPromise, ]); + sessionRowStored = true; // Tracked fire-and-forget embedding generation + session update + thread detection if (isEmbeddingAvailable()) { @@ -1642,9 +1723,12 @@ export async function sessionClose( const allErrors = [...persistErrors, ...coverageWarnings, ...validation.warnings]; - const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, !partialPersist, allErrors.length > 0 ? allErrors : undefined, transcriptStatus, blindspotSnippet); + // The session row is upserted above; a partial persist is threads, not the session. + const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, !partialPersist, allErrors.length > 0 ? allErrors : undefined, transcriptStatus, blindspotSnippet, true); return { + // The session row is durable; success also requires every thread to sync. + ...writeResult(true), success: !partialPersist, session_id: sessionId, close_compliance: closeCompliance, @@ -1660,9 +1744,10 @@ export async function sessionClose( // Clear session state even on error (session is done either way) clearCurrentSession(); - const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], transcriptStatus, blindspotSnippet); + const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], transcriptStatus, blindspotSnippet, sessionRowStored); return { + ...writeResult(sessionRowStored), success: false, session_id: sessionId, close_compliance: closeCompliance, diff --git a/src/tools/session-start.ts b/src/tools/session-start.ts index 28ffedd..b177c3c 100644 --- a/src/tools/session-start.ts +++ b/src/tools/session-start.ts @@ -24,6 +24,7 @@ import { getStorage } from "../services/storage.js"; import { Timer, recordMetrics, + registerPendingSessionRow, calculateContextBytes, PERFORMANCE_TARGETS, buildPerformanceData, @@ -42,7 +43,7 @@ import * as os from "os"; import { formatDate } from "../services/timezone.js"; import { productLine, dimText, boldText } from "../services/display-protocol.js"; // Suggested threads removed from start display -import type { PerformanceBreakdown, ComponentPerformance, SurfacedScar, Observation, SessionChild } from "../types/index.js"; +import type { PerformanceBreakdown, ComponentPerformance, SurfacedScar, ScarConfirmation, Observation, SessionChild } from "../types/index.js"; import { querySessionsByDateRange, queryScarUsageByDateRange, @@ -426,14 +427,14 @@ async function createSessionRecord( project: Project, linearIssue?: string, preGeneratedId?: string // Accept pre-generated UUID for fire-and-forget pattern -): Promise<{ session_id: string; latency_ms: number; network_call: boolean }> { +): Promise<{ session_id: string; latency_ms: number; network_call: boolean; stored: boolean }> { const sessionId = preGeneratedId || uuidv4(); const today = new Date().toISOString().split("T")[0]; const timer = new Timer(); if (!hasSupabase()) { // Free tier: session tracked locally only - return { session_id: sessionId, latency_ms: timer.stop(), network_call: false }; + return { session_id: sessionId, latency_ms: timer.stop(), network_call: false, stored: false }; } try { @@ -458,6 +459,7 @@ async function createSessionRecord( session_id: sessionId, latency_ms: timer.stop(), network_call: true, // Always writes to Supabase + stored: true, }; } catch (error) { console.error("[session_start] Failed to create session record:", error); @@ -466,6 +468,7 @@ async function createSessionRecord( session_id: sessionId, latency_ms: timer.stop(), network_call: true, // Network was attempted + stored: false, }; } } @@ -515,7 +518,7 @@ async function sessionStartFree( metricsId: string, existingSessionId?: string, existingStartedAt?: Date, - forceCarryActivity?: { surfacedScars: SurfacedScar[]; observations: Observation[]; children: SessionChild[] }, + forceCarryActivity?: { surfacedScars: SurfacedScar[]; confirmations: ScarConfirmation[]; observations: Observation[]; children: SessionChild[] }, ): Promise { const storage = getStorage(); const isResuming = !!existingSessionId; @@ -613,7 +616,8 @@ async function sessionStartFree( // writeSessionFiles merges with existing file threads to preserve mid-session creations let freeMergedThreads = freeAggregatedThreads; try { - freeMergedThreads = writeSessionFiles(sessionId, agent, project, surfacedScars, freeAggregatedThreads, undefined, false, false, isResuming ? existingStartedAt : undefined); + // existingStartedAt is set only on resume or a same-project force carry-forward. + freeMergedThreads = writeSessionFiles(sessionId, agent, project, surfacedScars, freeAggregatedThreads, undefined, false, false, existingStartedAt); } catch (error) { console.warn("[session_start] Failed to persist session files:", error); } @@ -624,8 +628,12 @@ async function sessionStartFree( sessionId, linearIssue: params.linear_issue, agent, - startedAt: (isResuming && existingStartedAt) || new Date(), + project, + // GIT-86: the force carry-forward startedAt was passed in but only honoured + // on resume, so a forced free-tier session restarted its duration clock. + startedAt: existingStartedAt || new Date(), surfacedScars: freeMergedScars, + confirmations: forceCarryActivity?.confirmations, observations: forceCarryActivity?.observations, children: forceCarryActivity?.children, threads: freeMergedThreads, @@ -636,6 +644,7 @@ async function sessionStartFree( session_id: sessionId, agent, ...(isResuming && { resumed: true }), + ...(isResuming && !params.project && { project_from_resumed_session: true }), detected_environment: env, last_session: lastSession, ...(projectState && { project_state: projectState }), @@ -714,10 +723,16 @@ function restoreSessionState( * Uses the active-sessions registry (hostname+PID) to identify THIS process's * session, preventing cross-process session theft on shared filesystems. * Falls back to legacy active-session.json for backward compatibility. + * + * GIT-86: `project` is the project the caller asked for. When set, only a + * session of that project is resumed; one desktop process serves every chat, + * so this process's other sessions belong to other conversations. When + * absent, the most recent session is resumed, whatever its project. */ function checkExistingSession( agent: AgentIdentity, - force?: boolean + force?: boolean, + project?: Project ): { sessionId: string; agent: AgentIdentity; linearIssue?: string; startedAt?: Date; project?: Project } | null { if (force) { console.error("[session_start] force=true, skipping active session guard"); @@ -736,7 +751,7 @@ function checkExistingSession( // answer — a lost or diverged registry used to make an intact session on // disk invisible, so session_start would open a second session alongside it. const mySession = - findSessionByHostPid(os.hostname(), process.pid) ?? findResumableSessionOnDisk(); + findSessionByHostPid(os.hostname(), process.pid, project) ?? findResumableSessionOnDisk(project); 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); @@ -913,6 +928,12 @@ function formatStartDisplay(result: SessionStartResult, displayInfoMap?: Map {}); + // Fire-and-forget: don't await the Supabase write. GIT-73: metrics writes + // for this session wait on it instead of racing it to a 409. + registerPendingSessionRow( + sessionId, + createSessionRecord(agent, project, params.linear_issue, sessionId).then((r) => r.stored) + ); // Mark prior in-memory session as superseded (force=true path) // Registry displacement in writeSessionFiles handles the registry case, // but priorSession may not be in the registry (e.g., after MCP restart) @@ -1203,6 +1234,7 @@ export async function sessionStart( project, startedAt: (isResuming && existingSession?.startedAt) || forceCarryStartedAt || new Date(), surfacedScars: mergedScars, + confirmations: forceCarryConfirmations, observations: forceCarryObservations, children: forceCarryChildren, threads: mergedThreads, @@ -1222,6 +1254,7 @@ export async function sessionStart( session_id: sessionId, agent, ...(isResuming && { resumed: true }), + ...(isResuming && !params.project && { project_from_resumed_session: true }), detected_environment: env, last_session: slimLastSession, ...(projectState && { project_state: projectState }), @@ -1311,7 +1344,7 @@ export async function sessionRefresh( project = params.project || currentSession.project || "default"; } else { // Fallback — check registry for this process, then legacy file - const mySession = findSessionByHostPid(os.hostname(), process.pid); + const mySession = findSessionByHostPid(os.hostname(), process.pid, params.project); let raw: Record | null = null; if (mySession) { diff --git a/src/types/index.ts b/src/types/index.ts index 61654e7..6de5c9b 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -198,6 +198,11 @@ export interface SessionStartResult { performance?: PerformanceData; /** Whether this session was resumed from an existing active session */ resumed?: boolean; + /** + * GIT-86: no project was passed, so the resumed session's project was used. + * The display names it, so the caller can see which namespace it is in. + */ + project_from_resumed_session?: boolean; /** Whether this result is from a mid-session refresh (no new session created) */ refreshed?: boolean; /** Message explaining session state */ @@ -291,8 +296,21 @@ export interface CloseCompliance { retroactive?: boolean; // Optional: marks sessions created post-mortem } -export interface SessionCloseResult { +/** + * GIT-101: the contract every write tool answers with — where the record + * landed and whether it survives this machine. See services/write-result.ts. + */ +export type StoredIn = "supabase" | "local" | "local_only" | null; +export interface WriteResult { + /** durable || !hasSupabase(), and false when nothing was stored. */ success: boolean; + /** Whether what was stored survives this machine. */ + durable: boolean; + /** Names the store in-band. */ + stored_in: StoredIn; +} + +export interface SessionCloseResult extends WriteResult { session_id: string; close_compliance: CloseCompliance; validation_errors?: string[]; @@ -325,8 +343,7 @@ export interface CreateLearningParams { self_check_criteria?: string[]; } -export interface CreateLearningResult { - success: boolean; +export interface CreateLearningResult extends WriteResult { learning_id: string; embedding_generated: boolean; /** Error details when success=false */ @@ -348,8 +365,7 @@ export interface CreateDecisionParams { project?: Project; } -export interface CreateDecisionResult { - success: boolean; +export interface CreateDecisionResult extends WriteResult { decision_id: string; display?: string; performance: PerformanceData; @@ -445,8 +461,7 @@ export interface RecordScarUsageParams { variant_id?: string; // UUID of assigned variant for A/B testing } -export interface RecordScarUsageResult { - success: boolean; +export interface RecordScarUsageResult extends WriteResult { usage_id: string; /** Error details when success=false */ errors?: string[]; @@ -474,8 +489,7 @@ export interface RecordScarUsageBatchParams { project?: Project; } -export interface RecordScarUsageBatchResult { - success: boolean; +export interface RecordScarUsageBatchResult extends WriteResult { usage_ids: string[]; resolved_count: number; failed_count: number; @@ -598,8 +612,7 @@ export interface ResolveThreadParams { resolution_note?: string; } -export interface ResolveThreadResult { - success: boolean; +export interface ResolveThreadResult extends WriteResult { resolved_thread?: ThreadObject; /** Threads that were also resolved via duplicate cascade */ also_resolved?: ThreadObject[]; diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..0a16761 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -14,6 +14,7 @@ * Usage (build first: npm run build): * VENUE_ENV=/path/outside/repo/.env node tests/e2e/blank-supabase.mjs flow --label main --out * VENUE_ENV=/path/outside/repo/.env node tests/e2e/blank-supabase.mjs egress --label pr31 --out [--rows 250] [--usage 5] + * VENUE_ENV=/path/outside/repo/.env node tests/e2e/blank-supabase.mjs projects --label git86 --out * * flow session_start -> list_threads -> recall -> confirm_scars -> create_thread * -> list_threads -> session_close -> health, with a local threads.json that @@ -21,6 +22,12 @@ * egress bytes received from the venue on a cold start, a warm start, and around * two session_starts (GIT-98 disk cache). --usage N seeds N scars with * >=3 usage rows so refresh_scar_behavioral_scores() has work to do. + * Fails if any start after the cold one downloads the whole index again + * (GIT-98 per-row delta). + * projects one server process serving two projects, as the desktop app does (GIT-86): + * session_start(X), session_start(Y), session_start(X), session_start({}), + * session_start(X, force). A session of one project is never resumed, + * displaced or superseded by a call for another. * * VENUE_ENV is a KEY=VALUE file OUTSIDE the repo defining SUPABASE_URL, * SUPABASE_SERVICE_ROLE_KEY and VENUE_REF. Credentials are never committed. @@ -48,7 +55,7 @@ const DENY_REFS = new Set(["cjptxyezuxdiinufgrrm"]); // production GitMem — ne * Known venue failures. Any other 4xx/5xx from /rest/v1/ or /functions/v1/ * fails the run. Each entry names the ticket that removes it — delete the entry * when that ticket merges, so this list only shrinks. An entry allows a failure; - * it does not require one (e.g. the GIT-73 race is intermittent). + * it does not require one (a failure may be intermittent). */ /** * Not defects: the store capability probe (src/services/store-columns.ts) is a @@ -72,8 +79,6 @@ function isCapabilityProbe(r) { } const EXPECTED_FAILURES = [ - { ticket: "GIT-73", what: "metrics/session_start FK race", method: "POST", path: /^\/rest\/v1\/gitmem_query_metrics$/, status: 409 }, - { ticket: "GIT-105", what: "knowledge-triple thread id into a uuid column", method: "POST", path: /^\/rest\/v1\/knowledge_triples$/, status: 400 }, ]; const HERE = dirname(fileURLToPath(import.meta.url)); const REPO = resolve(HERE, "../.."); @@ -96,8 +101,8 @@ function configError(message) { console.error(`[driver] ${message}`); process.exit(2); } -if (!["flow", "egress"].includes(mode)) { - configError("usage: blank-supabase.mjs flow|egress --label --out [--rows N] [--usage N]"); +if (!["flow", "egress", "projects"].includes(mode)) { + configError("usage: blank-supabase.mjs flow|egress|projects --label --out [--rows N] [--usage N]"); } const venueEnvPath = process.env.VENUE_ENV; @@ -366,6 +371,24 @@ async function flow() { const lt = await step("list_threads", { project: PROJECT }); const ltOpen = Number((lt.match(/(\d+) open/) || [])[1] ?? -1); + // GIT-101: resolve a second driver thread; the durable store must record it + // and the response must not claim a local-only resolve as done. + const ct2 = await step("create_thread", { text: `Close out the quarterly vendor audit checklist (${label})`, allow_duplicate: true }); + const resolveId = (ct2.match(/\bt-[0-9a-f]{8}\b/) || [])[0] ?? null; + const rt = resolveId ? await step("resolve_thread", { thread_id: resolveId, resolution_note: "venue driver" }) : ""; + const resolvedRow = resolveId + ? await rest("GET", `gitmem_threads?select=status,resolved_by_session&thread_id=eq.${resolveId}`, undefined, { Prefer: "" }).then((r) => r.json()) + : []; + const resolveDurable = !!resolveId && /Thread resolved/.test(rt) && !/LOCALLY ONLY/.test(rt) + && resolvedRow[0]?.status === "resolved"; + + // 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: { @@ -377,6 +400,16 @@ async function flow() { await waitQuiet(netlog, { minMs: 2000, quietMs: 3000 }); const health = await step("health", { failure_limit: 20 }); + // GIT-102: every WARN on the close names a subsystem and cause and says + // whether the session content was stored; health reports the write-path verdict. + const closeWarns = sc.replace(/\x1b\[[0-9;]*m/g, "").split("\n").filter((l) => /^WARN /.test(l)); + const closeWarnsSelfDescribing = closeWarns.every((l) => /^WARN .+: \d+ of \d+ writes? failed — .+ · session content (stored OK|NOT stored)/.test(l)); + const healthHasWritePath = /^Write path: /m.test(health); + // GIT-105: thread triples land, and their source_id is a real thread row id. + const threadTriples = await rest("GET", "knowledge_triples?select=predicate,source_id&source_type=eq.thread", undefined, { Prefer: "" }).then((r) => r.json()); + const threadRowIds = new Set((await rest("GET", "gitmem_threads?select=id", undefined, { Prefer: "" }).then((r) => r.json())).map((t) => t.id)); + const threadTriplesLinked = threadTriples.length > 0 && threadTriples.every((t) => threadRowIds.has(t.source_id)); + // Did the close land? Is relevance readable from the store (GIT-109)? const closedRows = sessionId ? await rest("GET", `gitmem_sessions?select=id,closing_reflection&id=eq.${sessionId}`, undefined, { Prefer: "" }).then((r) => r.json()) @@ -408,6 +441,16 @@ 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, + resolve_thread_durable: resolveDurable, + close_warns: closeWarns, + close_warns_self_describing: closeWarnsSelfDescribing, + health_has_write_path: healthHasWritePath, + thread_triples: threadTriples.length, + thread_triples_linked: threadTriplesLinked, + // GIT-107: the driver creates no stranded store; its GITMEM_DIR lives under + // $TMPDIR, i.e. /var/... vs /private/var/... on macOS. + false_stranded_notice: /NOT being read/.test(ss), relevance_readable: relevance.some((r) => scarIds.some((id) => (r.memories_applied || []).includes(id) && r.memory_relevance?.[id])), relevance, health_failed_total: failed, @@ -514,12 +557,97 @@ async function egress() { return summary; } -const result = mode === "flow" ? await flow() : await egress(); +// ---------------------------------------------------------------- PROJECTS (GIT-86) +async function projects() { + await wipeVenue(); + const X = `${PROJECT}-x`, Y = `${PROJECT}-y`; + const { home, gitmemDir } = isolatedHome(`${label}-projects`); + const ollama = await startFakeOllama(); + const netlog = join(outDir, `netlog-${label}-projects.jsonl`); + writeFileSync(netlog, ""); + const stderr = []; + const srv = await startServer({ home, gitmemDir, ollamaUrl: ollama.url, netlog, stderrSink: stderr }); + await waitQuiet(netlog, { minMs: 2000 }); + + const steps = []; + const start = async (name, args) => { + const text = await srv.call("session_start", { agent_identity: "desktop", ...args }); + const id = (text.match(UUID) || [])[0] ?? null; + const step = { name, args, session_id: id, resumed: /\bresumed\b/.test(text.split("\n")[0]), + names_project: (text.match(/Resumed project: (\S+)/) || [])[1] ?? null }; + steps.push(step); + console.log(`[driver] ${name}: ${id?.slice(0, 8)} resumed=${step.resumed}${step.names_project ? ` names=${step.names_project}` : ""}`); + await waitQuiet(netlog, { minMs: 1500, quietMs: 2000 }); + return step; + }; + const x1 = await start("X", { project: X }); + const y1 = await start("Y while X open", { project: Y }); + const x2 = await start("X again", { project: X }); + const bare = await start("no project", {}); + const xf = await start("X force (Y in memory)", { project: X, force: true }); + await srv.close(); + ollama.close(); + allVenueRequests.push(...venueReqs(readNet(netlog))); + writeFileSync(join(outDir, `stderr-${label}-projects.log`), stderr.join("")); + + const rows = await rest("GET", `gitmem_sessions?select=id,project,close_compliance&id=in.(${[x1, y1, xf].map((s) => s.session_id).join(",")})`, undefined, { Prefer: "" }).then((r) => r.json()); + const row = (id) => rows.find((r) => r.id === id); + const superseded = (id) => row(id)?.close_compliance?.close_type === "superseded"; + const registry = JSON.parse(readFileSync(join(gitmemDir, "active-sessions.json"), "utf8")).sessions; + const dirExists = (id) => existsSync(join(gitmemDir, "sessions", id, "session.json")); + + const checks = { + "Y is a new session, not X resumed": y1.session_id !== x1.session_id && !y1.resumed, + "X row stored under X": row(x1.session_id)?.project === X, + "Y row stored under Y": row(y1.session_id)?.project === Y, + "X again resumes X": x2.session_id === x1.session_id && x2.resumed, + "no project resumes the most recent session (Y)": bare.session_id === y1.session_id && bare.resumed, + "no project names the resumed project": bare.names_project === Y, + "X force is a new session": xf.session_id !== x1.session_id && xf.session_id !== y1.session_id && !xf.resumed, + "Y never superseded by an X call": !superseded(y1.session_id), + "X superseded only by the same-project force": superseded(x1.session_id) && row(x1.session_id)?.close_compliance?.superseded_by === xf.session_id, + "Y still registered and on disk": registry.some((e) => e.session_id === y1.session_id && e.project === Y) && dirExists(y1.session_id), + "no cross-project override on stderr": !/Project override on resume/.test(stderr.join("")), + }; + const failed = Object.entries(checks).filter(([, ok]) => !ok).map(([k]) => k); + const summary = { label, mode, venue: SUPABASE_URL, steps, rows, registry, checks, failed }; + writeFileSync(join(outDir, `summary-${label}-projects.json`), JSON.stringify(summary, null, 2)); + return summary; +} + +const result = mode === "flow" ? await flow() : mode === "projects" ? await projects() : await egress(); const failureCheck = checkFailures(allVenueRequests); // Invariants the edge-function removal (GIT-97) guarantees, whatever the status code. 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.resolve_thread_durable) { + failureCheck.unexpected.push("resolve_thread did not land durably in gitmem_threads, or claimed it did not (GIT-101)"); +} +if (mode === "flow" && !result.missing_payload_named) { + failureCheck.unexpected.push("session_close without a payload did not name the absolute closing-payload.json path (GIT-99)"); +} +// GIT-98 per-row delta: after the cold start, a changed store costs the changed +// rows, never the whole index again. +if (mode === "egress") { + for (const st of result.starts.slice(1)) { + if (st.cache_log.some((l) => /Loading ALL learnings/.test(l))) { + failureCheck.unexpected.push(`full index download after the cold start (GIT-98): ${st.start}, ${st.total_bytes} B`); + } + } +} +if (mode === "flow" && !result.close_warns_self_describing) { + failureCheck.unexpected.push(`session_close WARN is not self-describing (GIT-102): ${result.close_warns.join(" | ")}`); +} +if (mode === "flow" && !result.health_has_write_path) { + failureCheck.unexpected.push("health does not report the write-path verdict (GIT-102)"); +} +if (mode === "flow" && !result.thread_triples_linked) { + failureCheck.unexpected.push(`thread triples missing or not linked to a gitmem_threads row (GIT-105): ${result.thread_triples} found`); +} +if (mode === "flow" && result.false_stranded_notice) { + failureCheck.unexpected.push("session_start reported the store it reads as 'NOT being read' (GIT-107)"); +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } @@ -531,6 +659,7 @@ if (mode === "flow" && !result.thread_counts_match) { `thread panel mismatch (GIT-97): session_start ${result.session_start_thread_count} != list_threads ${result.list_threads_open_right_after_session_start}` ); } +if (mode === "projects") failureCheck.unexpected.push(...result.failed.map((f) => `cross-project resume (GIT-86): ${f}`)); writeFileSync(join(outDir, `failure-check-${label}-${mode}.json`), JSON.stringify(failureCheck, null, 2)); console.log(JSON.stringify({ ...result, health_text: undefined, stderr_error_lines: undefined, failure_check: failureCheck }, null, 2)); if (failureCheck.unexpected.length > 0) { diff --git a/tests/e2e/venue/reset-gitmem-objects.sql b/tests/e2e/venue/reset-gitmem-objects.sql new file mode 100644 index 0000000..b1aed51 --- /dev/null +++ b/tests/e2e/venue/reset-gitmem-objects.sql @@ -0,0 +1,21 @@ +-- GIT-83: return the DISPOSABLE venue to a blank slate so a setup.sql lands +-- exactly as it would on a new customer project. Drops only the objects +-- schema/setup.sql creates. Run only through scripts/venue-schema.sh, which +-- refuses production and any database that is not the configured venue. +DROP TABLE IF EXISTS + gitmem_license_activations, gitmem_licenses, + gitmem_scar_usage, gitmem_decisions, gitmem_threads, knowledge_triples, + gitmem_query_metrics, scar_enforcement_variants, + gitmem_learnings, gitmem_sessions + CASCADE; -- also removes the *_lite views, triggers and policies +DO $$ +DECLARE r record; +BEGIN + FOR r IN SELECT p.oid::regprocedure AS sig FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace + WHERE n.nspname = 'public' AND p.proname IN + ('gitmem_semantic_search','gitmem_scar_search','refresh_scar_behavioral_scores', + 'gitmem_update_timestamp','gitmem_validate_license','gitmem_deactivate_device') + LOOP EXECUTE 'DROP FUNCTION ' || r.sig || ' CASCADE'; END LOOP; +END $$; +-- PostgREST must see the new shape before the driver's first request. +NOTIFY pgrst, 'reload schema'; diff --git a/tests/fixtures/schema/setup-v1.8.0.sql b/tests/fixtures/schema/setup-v1.8.0.sql new file mode 100644 index 0000000..badd255 --- /dev/null +++ b/tests/fixtures/schema/setup-v1.8.0.sql @@ -0,0 +1,657 @@ +-- GitMem Schema Setup +-- Run this SQL in your Supabase SQL Editor to create the required tables. +-- Dashboard → SQL Editor → New query → Paste → Run + +-- Enable pgvector extension for semantic search +CREATE EXTENSION IF NOT EXISTS vector; + +-- ============================================================================ +-- Learnings table (scars, wins, patterns) +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_learnings ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + learning_type TEXT NOT NULL CHECK (learning_type IN ('scar', 'win', 'pattern', 'anti_pattern')), + title TEXT NOT NULL, + description TEXT NOT NULL, + severity TEXT CHECK (severity IN ('critical', 'high', 'medium', 'low')), + scar_type TEXT DEFAULT 'operational', + counter_arguments TEXT[] DEFAULT '{}', + problem_context TEXT DEFAULT '', + solution_approach TEXT DEFAULT '', + applies_when TEXT[] DEFAULT '{}', + keywords TEXT[] DEFAULT '{}', + domain TEXT[] DEFAULT '{}', + embedding vector(1536), + project TEXT DEFAULT 'default', + source_date DATE DEFAULT CURRENT_DATE, + source_linear_issue TEXT, + persona_name TEXT, + why_this_matters TEXT, + action_protocol TEXT, + self_check_criteria TEXT, + is_active BOOLEAN DEFAULT true, + decay_multiplier FLOAT DEFAULT 1.0, + repeat_mistake BOOLEAN DEFAULT false, + related_scar_id UUID, + repeat_mistake_details JSONB, + created_at TIMESTAMPTZ DEFAULT NOW(), + updated_at TIMESTAMPTZ DEFAULT NOW() +); + +-- Migration: ensure all columns exist (idempotent for upgrades) +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS embedding vector(1536); +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS source_linear_issue TEXT; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS persona_name TEXT; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS why_this_matters TEXT; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS action_protocol TEXT; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS self_check_criteria TEXT; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS is_active BOOLEAN DEFAULT true; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS decay_multiplier FLOAT DEFAULT 1.0; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS repeat_mistake BOOLEAN DEFAULT false; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS related_scar_id UUID; +ALTER TABLE gitmem_learnings ADD COLUMN IF NOT EXISTS repeat_mistake_details JSONB; + +-- Indexes (created after migration ensures columns exist) +CREATE INDEX IF NOT EXISTS idx_gitmem_learnings_embedding + ON gitmem_learnings USING ivfflat (embedding vector_cosine_ops) + WITH (lists = 10); + +CREATE INDEX IF NOT EXISTS idx_gitmem_learnings_type + ON gitmem_learnings (learning_type); + +CREATE INDEX IF NOT EXISTS idx_gitmem_learnings_project + ON gitmem_learnings (project); + +-- ============================================================================ +-- Sessions table +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_sessions ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + session_title TEXT DEFAULT 'Interactive Session', + session_date DATE DEFAULT CURRENT_DATE, + agent TEXT DEFAULT 'Unknown', + project TEXT DEFAULT 'default', + linear_issue TEXT, + recording_path TEXT, + transcript_path TEXT, + decisions TEXT[] DEFAULT '{}', + open_threads TEXT[] DEFAULT '{}', + closing_reflection JSONB, + close_compliance JSONB, + rapport_summary TEXT, + embedding vector(1536), + created_at TIMESTAMPTZ DEFAULT NOW(), + updated_at TIMESTAMPTZ DEFAULT NOW() +); + +-- Migration: ensure all columns exist +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS embedding vector(1536); +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS linear_issue TEXT; +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS recording_path TEXT; +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS transcript_path TEXT; +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS close_compliance JSONB; +ALTER TABLE gitmem_sessions ADD COLUMN IF NOT EXISTS rapport_summary TEXT; + +CREATE INDEX IF NOT EXISTS idx_gitmem_sessions_agent + ON gitmem_sessions (agent); + +CREATE INDEX IF NOT EXISTS idx_gitmem_sessions_created + ON gitmem_sessions (created_at DESC); + +-- ============================================================================ +-- Decisions table +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_decisions ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + decision_date DATE DEFAULT CURRENT_DATE, + title TEXT NOT NULL, + decision TEXT NOT NULL, + rationale TEXT NOT NULL, + alternatives_considered TEXT[] DEFAULT '{}', + personas_involved TEXT[] DEFAULT '{}', + docs_affected TEXT[] DEFAULT '{}', + linear_issue TEXT, + session_id UUID REFERENCES gitmem_sessions(id), + project TEXT DEFAULT 'default', + embedding vector(1536), + created_at TIMESTAMPTZ DEFAULT NOW() +); + +-- Migration: ensure all columns exist +ALTER TABLE gitmem_decisions ADD COLUMN IF NOT EXISTS embedding vector(1536); +ALTER TABLE gitmem_decisions ADD COLUMN IF NOT EXISTS personas_involved TEXT[] DEFAULT '{}'; +ALTER TABLE gitmem_decisions ADD COLUMN IF NOT EXISTS docs_affected TEXT[] DEFAULT '{}'; +ALTER TABLE gitmem_decisions ADD COLUMN IF NOT EXISTS linear_issue TEXT; + +CREATE INDEX IF NOT EXISTS idx_gitmem_decisions_session + ON gitmem_decisions (session_id); + +-- ============================================================================ +-- Scar usage tracking +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_scar_usage ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + scar_id UUID REFERENCES gitmem_learnings(id), + session_id UUID REFERENCES gitmem_sessions(id), + agent TEXT DEFAULT 'Unknown', + issue_id TEXT, + issue_identifier TEXT, + reference_type TEXT CHECK (reference_type IN ('explicit', 'implicit', 'acknowledged', 'refuted', 'none')), + reference_context TEXT, + surfaced_at TIMESTAMPTZ, + acknowledged_at TIMESTAMPTZ, + referenced BOOLEAN, + execution_successful BOOLEAN, + variant_id UUID, + created_at TIMESTAMPTZ DEFAULT NOW() +); + +CREATE INDEX IF NOT EXISTS idx_gitmem_scar_usage_scar + ON gitmem_scar_usage (scar_id); + +CREATE INDEX IF NOT EXISTS idx_gitmem_scar_usage_session + ON gitmem_scar_usage (session_id); + +-- Migration: add columns for older installs +ALTER TABLE gitmem_scar_usage ADD COLUMN IF NOT EXISTS issue_id TEXT; +ALTER TABLE gitmem_scar_usage ADD COLUMN IF NOT EXISTS issue_identifier TEXT; +ALTER TABLE gitmem_scar_usage ADD COLUMN IF NOT EXISTS acknowledged_at TIMESTAMPTZ; +ALTER TABLE gitmem_scar_usage ADD COLUMN IF NOT EXISTS referenced BOOLEAN; +ALTER TABLE gitmem_scar_usage ADD COLUMN IF NOT EXISTS variant_id UUID; + +-- ============================================================================ +-- Threads table (cross-session work tracking) +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_threads ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + thread_id TEXT NOT NULL UNIQUE, + text TEXT NOT NULL, + status TEXT NOT NULL DEFAULT 'active' CHECK (status IN ('emerging', 'active', 'cooling', 'dormant', 'archived', 'resolved')), + thread_class TEXT DEFAULT 'operational' CHECK (thread_class IN ('operational', 'backlog')), + vitality_score FLOAT DEFAULT 1.0, + last_touched_at TIMESTAMPTZ DEFAULT NOW(), + touch_count INT DEFAULT 1, + resolved_at TIMESTAMPTZ, + resolution_note TEXT, + source_session UUID REFERENCES gitmem_sessions(id), + resolved_by_session UUID REFERENCES gitmem_sessions(id), + related_issues TEXT[] DEFAULT '{}', + domain TEXT[] DEFAULT '{}', + project TEXT DEFAULT 'default', + metadata JSONB DEFAULT '{}', + embedding vector(1536), + created_at TIMESTAMPTZ DEFAULT NOW(), + updated_at TIMESTAMPTZ DEFAULT NOW() +); + +CREATE INDEX IF NOT EXISTS idx_gitmem_threads_status + ON gitmem_threads (status); + +CREATE INDEX IF NOT EXISTS idx_gitmem_threads_project + ON gitmem_threads (project); + +-- ============================================================================ +-- Knowledge triples (knowledge graph) +-- ============================================================================ +CREATE TABLE IF NOT EXISTS knowledge_triples ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + subject TEXT NOT NULL, + predicate TEXT NOT NULL CHECK (predicate IN ('created_in', 'influenced_by', 'supersedes', 'demonstrates', 'affects_doc', 'created_thread', 'resolves_thread', 'relates_to_thread')), + object TEXT NOT NULL, + event_time TIMESTAMPTZ DEFAULT NOW(), + decay_weight FLOAT DEFAULT 1.0, + half_life_days INT DEFAULT 9999, + decay_floor FLOAT DEFAULT 0.1, + source_type TEXT, + source_id UUID, + source_linear_issue TEXT, + domain TEXT[] DEFAULT '{}', + project TEXT DEFAULT 'default', + created_by TEXT, + created_at TIMESTAMPTZ DEFAULT NOW(), + updated_at TIMESTAMPTZ DEFAULT NOW() +); + +CREATE INDEX IF NOT EXISTS idx_gitmem_triples_subject + ON knowledge_triples (subject); + +CREATE INDEX IF NOT EXISTS idx_gitmem_triples_project + ON knowledge_triples (project); + +-- ============================================================================ +-- Query metrics (performance tracking) +-- ============================================================================ +CREATE TABLE IF NOT EXISTS gitmem_query_metrics ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + session_id UUID REFERENCES gitmem_sessions(id), + agent TEXT, + tool_name TEXT NOT NULL, + query_text TEXT, + tables_searched TEXT[], + latency_ms INT, + result_count INT, + similarity_scores FLOAT[], + context_bytes INT, + phase_tag TEXT, + linear_issue TEXT, + memories_surfaced UUID[], + metadata JSONB DEFAULT '{}', + created_at TIMESTAMPTZ DEFAULT NOW() +); + +-- ============================================================================ +-- Scar enforcement variants (A/B testing) +-- ============================================================================ +CREATE TABLE IF NOT EXISTS scar_enforcement_variants ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + scar_id UUID NOT NULL REFERENCES gitmem_learnings(id), + variant_type TEXT NOT NULL, + variant_config JSONB, + metadata JSONB DEFAULT '{}', + created_at TIMESTAMPTZ DEFAULT NOW(), + updated_at TIMESTAMPTZ DEFAULT NOW() +); + +-- ============================================================================ +-- Lite views (exclude embedding columns for reduced context) +-- ============================================================================ +CREATE OR REPLACE VIEW gitmem_learnings_lite AS + SELECT id, learning_type, title, description, severity, scar_type, + counter_arguments, problem_context, solution_approach, applies_when, + keywords, domain, project, source_date, source_linear_issue, + persona_name, is_active, decay_multiplier, created_at, updated_at + FROM gitmem_learnings; + +CREATE OR REPLACE VIEW gitmem_sessions_lite AS + SELECT id, session_title, session_date, agent, project, linear_issue, + decisions, open_threads, closing_reflection, close_compliance, + rapport_summary, created_at, updated_at + FROM gitmem_sessions; + +CREATE OR REPLACE VIEW gitmem_decisions_lite AS + SELECT id, decision_date, title, decision, rationale, + alternatives_considered, personas_involved, docs_affected, + linear_issue, session_id, project, created_at + FROM gitmem_decisions; + +CREATE OR REPLACE VIEW gitmem_threads_lite AS + SELECT id, thread_id, text, status, thread_class, vitality_score, + last_touched_at, touch_count, resolved_at, resolution_note, + source_session, resolved_by_session, related_issues, domain, + project, metadata, created_at, updated_at + FROM gitmem_threads; + +-- ============================================================================ +-- Semantic search RPC function +-- ============================================================================ +CREATE OR REPLACE FUNCTION gitmem_semantic_search( + query_embedding vector(1536), + match_count INT DEFAULT 5, + similarity_threshold FLOAT DEFAULT 0.0 +) +RETURNS TABLE ( + id UUID, + title TEXT, + description TEXT, + severity TEXT, + counter_arguments TEXT[], + similarity FLOAT +) +LANGUAGE plpgsql +AS $$ +BEGIN + RETURN QUERY + SELECT + l.id, + l.title, + l.description, + l.severity, + l.counter_arguments, + 1 - (l.embedding <=> query_embedding) AS similarity + FROM gitmem_learnings l + WHERE l.embedding IS NOT NULL + AND 1 - (l.embedding <=> query_embedding) > similarity_threshold + ORDER BY l.embedding <=> query_embedding + LIMIT match_count; +END; +$$; + +-- ============================================================================ +-- Scar search with temporal + behavioral decay weighting +-- ============================================================================ +CREATE OR REPLACE FUNCTION gitmem_scar_search( + query_embedding vector(1536), + match_count INT DEFAULT 5, + similarity_threshold FLOAT DEFAULT 0.0 +) +RETURNS TABLE ( + id UUID, + title TEXT, + description TEXT, + severity TEXT, + scar_type TEXT, + counter_arguments TEXT[], + decay_multiplier FLOAT, + similarity FLOAT, + weighted_similarity FLOAT +) +LANGUAGE plpgsql +AS $$ +DECLARE + decay_days INT; + temporal_weight FLOAT; +BEGIN + RETURN QUERY + SELECT + l.id, + l.title, + l.description, + l.severity, + l.scar_type, + l.counter_arguments, + COALESCE(l.decay_multiplier, 1.0) AS decay_multiplier, + (1 - (l.embedding <=> query_embedding))::FLOAT AS similarity, + -- Weighted similarity: raw * temporal_decay * behavioral_decay + ( + (1 - (l.embedding <=> query_embedding)) * + -- Temporal decay based on scar_type + CASE + WHEN l.scar_type = 'process' THEN 1.0 -- permanent + WHEN l.scar_type = 'incident' THEN + GREATEST(0.1, 1.0 - 0.9 * (EXTRACT(EPOCH FROM (NOW() - l.created_at)) / (180.0 * 86400))) + WHEN l.scar_type = 'context' THEN + GREATEST(0.1, 1.0 - 0.9 * (EXTRACT(EPOCH FROM (NOW() - l.created_at)) / (30.0 * 86400))) + ELSE 1.0 -- default: no decay + END * + -- Behavioral decay multiplier + COALESCE(l.decay_multiplier, 1.0) + )::FLOAT AS weighted_similarity + FROM gitmem_learnings l + WHERE l.embedding IS NOT NULL + AND COALESCE(l.is_active, true) = true + AND (1 - (l.embedding <=> query_embedding)) > similarity_threshold + ORDER BY weighted_similarity DESC + LIMIT match_count; +END; +$$; + +-- ============================================================================ +-- Refresh behavioral decay scores from scar_usage patterns +-- Aggregates last 90 days of usage, computes dismiss rate, +-- updates decay_multiplier on learnings (minimum 3 surfacings required) +-- ============================================================================ +CREATE OR REPLACE FUNCTION refresh_scar_behavioral_scores() +RETURNS TABLE (scars_updated INT, scars_scanned INT) +LANGUAGE plpgsql +AS $$ +DECLARE + v_scanned INT := 0; + v_updated INT := 0; +BEGIN + -- Aggregate scar_usage from last 90 days, compute dismiss rate + WITH usage_stats AS ( + SELECT + su.scar_id, + COUNT(*) AS times_surfaced, + COUNT(*) FILTER (WHERE su.reference_type IN ('none', 'refuted')) AS times_dismissed + FROM gitmem_scar_usage su + WHERE su.surfaced_at >= NOW() - INTERVAL '90 days' + GROUP BY su.scar_id + HAVING COUNT(*) >= 3 -- minimum surfacings for meaningful signal + ) + UPDATE gitmem_learnings l + SET decay_multiplier = GREATEST(0.1, 1.0 - (us.times_dismissed::FLOAT / us.times_surfaced::FLOAT) * 0.8), + updated_at = NOW() + FROM usage_stats us + WHERE l.id = us.scar_id + AND COALESCE(l.is_active, true) = true; + + GET DIAGNOSTICS v_updated = ROW_COUNT; + + SELECT COUNT(DISTINCT scar_id) INTO v_scanned + FROM gitmem_scar_usage + WHERE surfaced_at >= NOW() - INTERVAL '90 days'; + + RETURN QUERY SELECT v_updated, v_scanned; +END; +$$; + +-- ============================================================================ +-- Row Level Security +-- ============================================================================ + +-- Enable RLS on all tables +ALTER TABLE gitmem_learnings ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_sessions ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_decisions ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_scar_usage ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_threads ENABLE ROW LEVEL SECURITY; +ALTER TABLE knowledge_triples ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_query_metrics ENABLE ROW LEVEL SECURITY; +ALTER TABLE scar_enforcement_variants ENABLE ROW LEVEL SECURITY; + +-- Service role has full access (used by the MCP server) +-- Drop-then-create for idempotency (CREATE POLICY has no IF NOT EXISTS) +DO $$ BEGIN + -- gitmem_learnings + DROP POLICY IF EXISTS "Service role full access" ON gitmem_learnings; + CREATE POLICY "Service role full access" ON gitmem_learnings FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_learnings; + CREATE POLICY "Block anonymous access" ON gitmem_learnings FOR ALL USING (auth.role() != 'anon'); + -- gitmem_sessions + DROP POLICY IF EXISTS "Service role full access" ON gitmem_sessions; + CREATE POLICY "Service role full access" ON gitmem_sessions FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_sessions; + CREATE POLICY "Block anonymous access" ON gitmem_sessions FOR ALL USING (auth.role() != 'anon'); + -- gitmem_decisions + DROP POLICY IF EXISTS "Service role full access" ON gitmem_decisions; + CREATE POLICY "Service role full access" ON gitmem_decisions FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_decisions; + CREATE POLICY "Block anonymous access" ON gitmem_decisions FOR ALL USING (auth.role() != 'anon'); + -- gitmem_scar_usage + DROP POLICY IF EXISTS "Service role full access" ON gitmem_scar_usage; + CREATE POLICY "Service role full access" ON gitmem_scar_usage FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_scar_usage; + CREATE POLICY "Block anonymous access" ON gitmem_scar_usage FOR ALL USING (auth.role() != 'anon'); + -- gitmem_threads + DROP POLICY IF EXISTS "Service role full access" ON gitmem_threads; + CREATE POLICY "Service role full access" ON gitmem_threads FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_threads; + CREATE POLICY "Block anonymous access" ON gitmem_threads FOR ALL USING (auth.role() != 'anon'); + -- knowledge_triples + DROP POLICY IF EXISTS "Service role full access" ON knowledge_triples; + CREATE POLICY "Service role full access" ON knowledge_triples FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON knowledge_triples; + CREATE POLICY "Block anonymous access" ON knowledge_triples FOR ALL USING (auth.role() != 'anon'); + -- gitmem_query_metrics + DROP POLICY IF EXISTS "Service role full access" ON gitmem_query_metrics; + CREATE POLICY "Service role full access" ON gitmem_query_metrics FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON gitmem_query_metrics; + CREATE POLICY "Block anonymous access" ON gitmem_query_metrics FOR ALL USING (auth.role() != 'anon'); + -- scar_enforcement_variants + DROP POLICY IF EXISTS "Service role full access" ON scar_enforcement_variants; + CREATE POLICY "Service role full access" ON scar_enforcement_variants FOR ALL USING (auth.role() = 'service_role'); + DROP POLICY IF EXISTS "Block anonymous access" ON scar_enforcement_variants; + CREATE POLICY "Block anonymous access" ON scar_enforcement_variants FOR ALL USING (auth.role() != 'anon'); +END $$; + +-- ============================================================================ +-- Auto-update timestamps +-- ============================================================================ +CREATE OR REPLACE FUNCTION gitmem_update_timestamp() +RETURNS TRIGGER AS $$ +BEGIN + NEW.updated_at = NOW(); + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS gitmem_learnings_updated ON gitmem_learnings; +CREATE TRIGGER gitmem_learnings_updated + BEFORE UPDATE ON gitmem_learnings + FOR EACH ROW EXECUTE FUNCTION gitmem_update_timestamp(); + +DROP TRIGGER IF EXISTS gitmem_sessions_updated ON gitmem_sessions; +CREATE TRIGGER gitmem_sessions_updated + BEFORE UPDATE ON gitmem_sessions + FOR EACH ROW EXECUTE FUNCTION gitmem_update_timestamp(); + +-- ============================================================================ +-- License Management Tables (deployed on GitMem's infrastructure, not user's) +-- These tables are included here for reference and for our own deployment. +-- Users do NOT need to run these — they exist on gitmem-api.supabase.co +-- ============================================================================ + +CREATE TABLE IF NOT EXISTS gitmem_licenses ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + api_key TEXT UNIQUE NOT NULL, + tier TEXT NOT NULL DEFAULT 'pro' CHECK (tier IN ('pro', 'dev')), + owner_email TEXT, + is_active BOOLEAN NOT NULL DEFAULT true, + max_activations INTEGER NOT NULL DEFAULT 3, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + expires_at TIMESTAMPTZ, + last_validated_at TIMESTAMPTZ, + metadata JSONB DEFAULT '{}' +); + +CREATE TABLE IF NOT EXISTS gitmem_license_activations ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + license_id UUID NOT NULL REFERENCES gitmem_licenses(id) ON DELETE CASCADE, + install_id TEXT NOT NULL, + activated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + last_seen_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + device_info JSONB DEFAULT '{}', + UNIQUE(license_id, install_id) +); + +-- RLS: service role only (these are admin tables on our infrastructure) +ALTER TABLE gitmem_licenses ENABLE ROW LEVEL SECURITY; +ALTER TABLE gitmem_license_activations ENABLE ROW LEVEL SECURITY; + +CREATE POLICY "Service role full access" ON gitmem_licenses + FOR ALL USING (auth.role() = 'service_role'); + +CREATE POLICY "Service role full access" ON gitmem_license_activations + FOR ALL USING (auth.role() = 'service_role'); + +-- Index for fast key lookup +CREATE INDEX IF NOT EXISTS idx_gitmem_licenses_api_key + ON gitmem_licenses (api_key); + +CREATE INDEX IF NOT EXISTS idx_gitmem_license_activations_license + ON gitmem_license_activations (license_id); + +-- ============================================================================ +-- License Validation RPC +-- Checks key validity, device limit, registers/updates activation +-- ============================================================================ +CREATE OR REPLACE FUNCTION gitmem_validate_license(p_api_key TEXT, p_install_id TEXT) +RETURNS TABLE(tier TEXT, valid BOOLEAN, message TEXT) +LANGUAGE plpgsql +SECURITY DEFINER +AS $$ +DECLARE + v_license_id UUID; + v_tier TEXT; + v_is_active BOOLEAN; + v_max_activations INTEGER; + v_expires_at TIMESTAMPTZ; + v_current_activations INTEGER; +BEGIN + -- Look up the license + SELECT l.id, l.tier, l.is_active, l.max_activations, l.expires_at + INTO v_license_id, v_tier, v_is_active, v_max_activations, v_expires_at + FROM gitmem_licenses l + WHERE l.api_key = p_api_key; + + -- Key not found + IF v_license_id IS NULL THEN + RETURN QUERY SELECT NULL::TEXT, false, 'Invalid license key'::TEXT; + RETURN; + END IF; + + -- Key deactivated + IF NOT v_is_active THEN + RETURN QUERY SELECT NULL::TEXT, false, 'License has been deactivated'::TEXT; + RETURN; + END IF; + + -- Key expired + IF v_expires_at IS NOT NULL AND v_expires_at < NOW() THEN + RETURN QUERY SELECT NULL::TEXT, false, 'License has expired'::TEXT; + RETURN; + END IF; + + -- Count current activations (excluding this install_id if already registered) + SELECT COUNT(*) + INTO v_current_activations + FROM gitmem_license_activations a + WHERE a.license_id = v_license_id + AND a.install_id != p_install_id; + + -- Check device limit (if this is a new device) + IF v_current_activations >= v_max_activations THEN + -- Check if this install_id already exists (re-validation) + IF NOT EXISTS ( + SELECT 1 FROM gitmem_license_activations a + WHERE a.license_id = v_license_id AND a.install_id = p_install_id + ) THEN + RETURN QUERY SELECT NULL::TEXT, false, + format('Device limit reached (%s/%s). Deactivate another device or contact support.', v_current_activations, v_max_activations)::TEXT; + RETURN; + END IF; + END IF; + + -- Register or update activation + INSERT INTO gitmem_license_activations (license_id, install_id, last_seen_at) + VALUES (v_license_id, p_install_id, NOW()) + ON CONFLICT (license_id, install_id) + DO UPDATE SET last_seen_at = NOW(); + + -- Update last_validated_at on the license + UPDATE gitmem_licenses SET last_validated_at = NOW() WHERE id = v_license_id; + + -- Success + RETURN QUERY SELECT v_tier, true, 'Valid'::TEXT; +END; +$$; + +-- ============================================================================ +-- Device Deactivation RPC +-- Removes a device activation for a license key + install_id pair. +-- Called by `gitmem-mcp deactivate` to free up a device slot. +-- ============================================================================ +CREATE OR REPLACE FUNCTION gitmem_deactivate_device(p_api_key TEXT, p_install_id TEXT) +RETURNS TABLE(success BOOLEAN, message TEXT) +LANGUAGE plpgsql +SECURITY DEFINER +AS $$ +DECLARE + v_license_id UUID; + v_deleted INTEGER; +BEGIN + -- Look up the license + SELECT l.id INTO v_license_id + FROM gitmem_licenses l + WHERE l.api_key = p_api_key; + + IF v_license_id IS NULL THEN + RETURN QUERY SELECT false, 'Invalid license key'::TEXT; + RETURN; + END IF; + + -- Delete the activation for this install_id + DELETE FROM gitmem_license_activations a + WHERE a.license_id = v_license_id + AND a.install_id = p_install_id; + + GET DIAGNOSTICS v_deleted = ROW_COUNT; + + IF v_deleted > 0 THEN + RETURN QUERY SELECT true, 'Device deactivated'::TEXT; + ELSE + RETURN QUERY SELECT true, 'Device was not registered (already deactivated)'::TEXT; + END IF; +END; +$$; diff --git a/tests/helpers/schema-drift.ts b/tests/helpers/schema-drift.ts new file mode 100644 index 0000000..67b19dc --- /dev/null +++ b/tests/helpers/schema-drift.ts @@ -0,0 +1,529 @@ +/** + * GIT-109 step 2: static schema-drift analysis. + * + * Customers never run SQL after initial setup (standing rule, 2026-09-21), so + * every table and column gitmem touches must exist in the v1.8.0 setup.sql — + * or be deliberately guarded. Release B found closes lost to exactly this: the + * code wrote production-only columns a setup.sql store does not have. + * + * This walks src/ with the TypeScript compiler API and collects, per call site: + * writes — table + payload keys reaching directUpsert / directPatch / + * upsertRecord / storage.upsert + * reads — table + select / filter / order keys reaching directQuery, + * directQueryAll, listRecords, getRecord + * and compares them with a parsed setup.sql (table -> columns, views included). + * + * Resolution is deliberately conservative: a table or payload it cannot + * resolve statically is reported as UNRESOLVED, never assumed fine. + */ +import * as fs from "fs"; +import * as path from "path"; +import ts from "typescript"; + +// ---------------------------------------------------------------- schema + +export type Schema = Map>; + +const CONSTRAINT_WORDS = /^(PRIMARY|UNIQUE|CONSTRAINT|FOREIGN|CHECK|EXCLUDE)\b/i; + +/** setup.sql -> table/view name -> columns. */ +export function parseSchema(sql: string): Schema { + const schema: Schema = new Map(); + const clean = sql.replace(/--[^\n]*/g, ""); + const add = (t: string, c: string) => { + if (!schema.has(t)) schema.set(t, new Set()); + schema.get(t)!.add(c); + }; + + // CREATE TABLE ... ( body ); — body split on top-level commas + const tableRe = /CREATE TABLE(?: IF NOT EXISTS)?\s+(?:public\.)?(\w+)\s*\(/gi; + let m: RegExpExecArray | null; + while ((m = tableRe.exec(clean))) { + const table = m[1]; + let depth = 1, i = tableRe.lastIndex, start = i; + const parts: string[] = []; + for (; i < clean.length && depth > 0; i++) { + const ch = clean[i]; + if (ch === "(") depth++; + else if (ch === ")") { depth--; if (depth === 0) parts.push(clean.slice(start, i)); } + else if (ch === "," && depth === 1) { parts.push(clean.slice(start, i)); start = i + 1; } + } + if (!schema.has(table)) schema.set(table, new Set()); + for (const p of parts) { + const line = p.trim(); + if (!line || CONSTRAINT_WORDS.test(line)) continue; + const col = line.match(/^"?(\w+)"?/); + if (col) add(table, col[1]); + } + } + + const alterRe = /ALTER TABLE\s+(?:IF EXISTS\s+)?(?:public\.)?(\w+)\s+ADD COLUMN(?: IF NOT EXISTS)?\s+"?(\w+)"?/gi; + while ((m = alterRe.exec(clean))) add(m[1], m[2]); + + const viewRe = /CREATE (?:OR REPLACE )?VIEW\s+(?:public\.)?(\w+)\s+AS\s+SELECT\s+([\s\S]+?)\s+FROM\s+(?:public\.)?(\w+)/gi; + while ((m = viewRe.exec(clean))) { + const [, view, list, base] = m; + if (!schema.has(view)) schema.set(view, new Set()); + for (const item of list.split(",").map((s) => s.trim())) { + if (item === "*") { for (const c of schema.get(base) ?? []) add(view, c); continue; } + const alias = item.match(/\bAS\s+(\w+)$/i); + add(view, alias ? alias[1] : item.replace(/^\w+\./, "").match(/^(\w+)/)?.[1] ?? item); + } + } + return schema; +} + +// ---------------------------------------------------------------- code scan + +export type AccessKind = "write" | "select" | "filter" | "order"; + +export interface Access { + file: string; + line: number; + call: string; + table: string; + column: string; + kind: AccessKind; +} + +export interface Unresolved { + file: string; + line: number; + call: string; + what: string; +} + +export interface ScanResult { + accesses: Access[]; + /** Generic helpers whose table is a parameter (storage.ts, supabase-client.ts): checked at their callers. */ + passThrough: Unresolved[]; + /** Tables reached with no column information (e.g. a bare select=*). */ + tables: Array<{ file: string; line: number; call: string; table: string }>; + unresolved: Unresolved[]; + /** Call sites in free-tier-only code (local JSON files, not the store). */ + freeTierOnly: number; + callSites: number; +} + +const WRITE_CALLS = new Set(["directUpsert", "upsertRecord"]); +const PATCH_CALLS = new Set(["directPatch"]); +const QUERY_CALLS = new Set(["directQuery", "directQueryAll"]); +const DEFAULT_PREFIX = "gitmem_"; + +function calleeName(expr: ts.LeftHandSideExpression): { name: string; receiver?: string } | null { + if (ts.isIdentifier(expr)) return { name: expr.text }; + if (ts.isPropertyAccessExpression(expr)) { + const recv = expr.expression; + const receiver = ts.isIdentifier(recv) ? recv.text : ts.isCallExpression(recv) && ts.isIdentifier(recv.expression) ? `${recv.expression.text}()` : undefined; + return { name: expr.name.text, receiver }; + } + return null; +} + +/** Find the nearest declaration of `name` visible from `from` (same file). */ +function findDeclaration(name: string, from: ts.Node): ts.VariableDeclaration | ts.ParameterDeclaration | null { + let scope: ts.Node | undefined = from; + while (scope) { + let found: ts.VariableDeclaration | ts.ParameterDeclaration | null = null; + const visit = (n: ts.Node): void => { + if (found) return; + if (ts.isVariableDeclaration(n) && ts.isIdentifier(n.name) && n.name.text === name && n.pos < from.pos) found = n; + else if (ts.isParameter(n) && ts.isIdentifier(n.name) && n.name.text === name) found = n; + if (n !== scope && (ts.isFunctionLike(n) || ts.isClassLike(n))) { + // parameters of the enclosing function are visited via the scope itself + return; + } + ts.forEachChild(n, visit); + }; + if (ts.isFunctionLike(scope)) for (const p of scope.parameters) visit(p); + ts.forEachChild(scope, visit); + if (found) return found; + scope = scope.parent; + } + return null; +} + +function resolveTable(expr: ts.Expression | undefined): string | null { + if (!expr) return null; + if (ts.isStringLiteralLike(expr)) return expr.text; + if (ts.isParenthesizedExpression(expr) || ts.isAsExpression(expr) || ts.isNonNullExpression(expr)) return resolveTable(expr.expression); + if (ts.isCallExpression(expr) && ts.isIdentifier(expr.expression) && expr.expression.text === "getTableName") { + const arg = expr.arguments[0]; + return arg && ts.isStringLiteralLike(arg) ? DEFAULT_PREFIX + arg.text : null; + } + if (ts.isIdentifier(expr)) { + const decl = findDeclaration(expr.text, expr); + if (decl && ts.isVariableDeclaration(decl) && decl.initializer) return resolveTable(decl.initializer); + } + return null; +} + +interface KeySet { keys: Set; complete: boolean; why: string[] } + +function propName(p: ts.ObjectLiteralElementLike): string | null { + const n = p.name; + if (!n) return null; + if (ts.isIdentifier(n) || ts.isStringLiteralLike(n) || ts.isNumericLiteral(n)) return n.text; + return null; +} + +/** Keys of an object-valued expression, following spreads, conditionals and later assignments. */ +function resolveKeys(expr: ts.Expression | undefined, seen = new Set()): KeySet { + const out: KeySet = { keys: new Set(), complete: true, why: [] }; + const merge = (k: KeySet) => { k.keys.forEach((x) => out.keys.add(x)); if (!k.complete) { out.complete = false; out.why.push(...k.why); } }; + if (!expr) return { keys: new Set(), complete: false, why: ["no payload"] }; + if (seen.has(expr)) return out; + seen.add(expr); + + if (ts.isParenthesizedExpression(expr) || ts.isAsExpression(expr) || ts.isNonNullExpression(expr) || ts.isSatisfiesExpression?.(expr)) { + return resolveKeys((expr as ts.ParenthesizedExpression).expression, seen); + } + if (ts.isObjectLiteralExpression(expr)) { + for (const p of expr.properties) { + if (ts.isSpreadAssignment(p)) merge(resolveKeys(p.expression, seen)); + else if (ts.isShorthandPropertyAssignment(p)) out.keys.add(p.name.text); + else { + const n = propName(p); + if (n) out.keys.add(n); else { out.complete = false; out.why.push(`computed key ${p.getText().slice(0, 40)}`); } + } + } + return out; + } + // `cond && {…}` / `cond ? {…} : {…}` + if (ts.isBinaryExpression(expr) && (expr.operatorToken.kind === ts.SyntaxKind.AmpersandAmpersandToken || expr.operatorToken.kind === ts.SyntaxKind.BarBarToken || expr.operatorToken.kind === ts.SyntaxKind.QuestionQuestionToken)) { + if (ts.isObjectLiteralExpression(expr.right) || ts.isParenthesizedExpression(expr.right)) return resolveKeys(expr.right, seen); + } + if (ts.isConditionalExpression(expr)) { + merge(resolveKeys(expr.whenTrue, seen)); + merge(resolveKeys(expr.whenFalse, seen)); + return out; + } + if (ts.isIdentifier(expr)) { + const decl = findDeclaration(expr.text, expr); + // A callback parameter: `rows.map((r) => write(r))` — the elements of rows. + if (decl && ts.isParameter(decl) && ts.isIdentifier(decl.name)) { + const fn = decl.parent; + const call = fn.parent; + if ((ts.isArrowFunction(fn) || ts.isFunctionExpression(fn)) && ts.isCallExpression(call) && call.arguments[0] === fn && + ts.isPropertyAccessExpression(call.expression) && /^(map|forEach|flatMap)$/.test(call.expression.name.text)) { + return elementKeys(call.expression.expression, seen); + } + } + if (decl && ts.isVariableDeclaration(decl) && decl.initializer) { + merge(resolveKeys(decl.initializer, seen)); + // later `x.prop = …` / `x["prop"] = …` in the same function + const fn = enclosingFunction(decl) ?? decl.getSourceFile(); + const visit = (n: ts.Node): void => { + if (ts.isBinaryExpression(n) && n.operatorToken.kind === ts.SyntaxKind.EqualsToken) { + const l = n.left; + if (ts.isPropertyAccessExpression(l) && ts.isIdentifier(l.expression) && l.expression.text === expr.text) out.keys.add(l.name.text); + if (ts.isElementAccessExpression(l) && ts.isIdentifier(l.expression) && l.expression.text === expr.text && ts.isStringLiteralLike(l.argumentExpression)) out.keys.add(l.argumentExpression.text); + } + ts.forEachChild(n, visit); + }; + visit(fn); + return out; + } + return { keys: new Set(), complete: false, why: [`payload ${expr.text} not resolvable`] }; + } + if (ts.isCallExpression(expr)) { + // Column filters narrow a payload; the payload is their first argument. + const name = ts.isIdentifier(expr.expression) ? expr.expression.text : ts.isPropertyAccessExpression(expr.expression) ? expr.expression.name.text : ""; + if (/^filterTo|Columns$/.test(name) && expr.arguments[0]) { + const k = resolveKeys(expr.arguments[0], seen); + k.why.push(`filtered by ${name}`); + return k; + } + // A function in this file that returns an object literal. + const local = ts.isIdentifier(expr.expression) ? findLocalFunction(expr.expression.text, expr.getSourceFile()) : null; + if (local) return returnKeys(local, seen); + if (ts.isAwaitExpression(expr.parent)) {/* fallthrough */} + return { keys: new Set(), complete: false, why: [`payload from ${name || "call"}()`] }; + } + if (ts.isAwaitExpression(expr)) return resolveKeys(expr.expression, seen); + return { keys: new Set(), complete: false, why: [`payload ${ts.SyntaxKind[expr.kind]}`] }; +} + +function findLocalFunction(name: string, sf: ts.SourceFile): ts.FunctionLikeDeclaration | null { + let found: ts.FunctionLikeDeclaration | null = null; + const visit = (n: ts.Node): void => { + if (found) return; + if (ts.isFunctionDeclaration(n) && n.name?.text === name && n.body) found = n; + else if (ts.isVariableDeclaration(n) && ts.isIdentifier(n.name) && n.name.text === name && n.initializer && + (ts.isArrowFunction(n.initializer) || ts.isFunctionExpression(n.initializer))) found = n.initializer; + else ts.forEachChild(n, visit); + }; + visit(sf); + return found; +} + +/** Keys of every object a function returns (its own returns, not nested functions'). */ +function returnKeys(fn: ts.FunctionLikeDeclaration, seen: Set): KeySet { + const out: KeySet = { keys: new Set(), complete: true, why: [] }; + const merge = (k: KeySet) => { k.keys.forEach((x) => out.keys.add(x)); if (!k.complete) { out.complete = false; out.why.push(...k.why); } }; + if (fn.body && !ts.isBlock(fn.body)) { merge(resolveKeys(fn.body as ts.Expression, seen)); return out; } + const visit = (n: ts.Node): void => { + if (n !== fn && ts.isFunctionLike(n)) return; + if (ts.isReturnStatement(n) && n.expression) merge(resolveKeys(n.expression, seen)); + ts.forEachChild(n, visit); + }; + if (fn.body) visit(fn.body); + if (out.keys.size === 0 && out.complete) return { keys: out.keys, complete: false, why: ["function returns no object literal"] }; + return out; +} + +/** Keys of the elements of an array-valued expression. */ +function elementKeys(expr: ts.Expression, seen: Set): KeySet { + const out: KeySet = { keys: new Set(), complete: true, why: [] }; + const merge = (k: KeySet) => { k.keys.forEach((x) => out.keys.add(x)); if (!k.complete) { out.complete = false; out.why.push(...k.why); } }; + if (ts.isParenthesizedExpression(expr) || ts.isAsExpression(expr)) return elementKeys(expr.expression, seen); + if (ts.isArrayLiteralExpression(expr)) { + for (const el of expr.elements) merge(ts.isSpreadElement(el) ? elementKeys(el.expression, seen) : resolveKeys(el, seen)); + return out; + } + if (ts.isIdentifier(expr)) { + const d = findDeclaration(expr.text, expr); + if (d && ts.isVariableDeclaration(d) && d.initializer) return elementKeys(d.initializer, seen); + } + if (ts.isCallExpression(expr) && ts.isPropertyAccessExpression(expr.expression)) { + const m = expr.expression.name.text; + if (/^(filter|slice|concat|reverse|sort)$/.test(m)) return elementKeys(expr.expression.expression, seen); + const cb = expr.arguments[0]; + if (m === "map" && cb && (ts.isArrowFunction(cb) || ts.isFunctionExpression(cb))) return returnKeys(cb, seen); + } + return { keys: new Set(), complete: false, why: [`elements of ${expr.getText().slice(0, 40)}`] }; +} + +function enclosingFunction(n: ts.Node): ts.Node | null { + let p: ts.Node | undefined = n.parent; + while (p) { if (ts.isFunctionLike(p)) return p; p = p.parent; } + return null; +} + +/** PostgREST select list -> base columns (alias:col, col::cast, col->>path; embeds skipped). */ +export function selectColumns(select: string): { columns: string[]; star: boolean } { + const cols: string[] = []; + let star = false, depth = 0, cur = ""; + const flush = () => { + const item = cur.trim(); cur = ""; + if (!item) return; + if (item === "*") { star = true; return; } + if (item.includes("(")) return; // resource embedding rel(...) + const body = item.includes(":") && !item.includes("::") ? item.split(":").pop()! : item.replace(/^\w+:(?!:)/, ""); + const col = body.split("::")[0].split("->")[0].trim(); + if (/^\w+$/.test(col)) cols.push(col); + }; + for (const ch of select) { + if (ch === "(") depth++; + if (ch === ")") depth--; + if (ch === "," && depth === 0) flush(); else cur += ch; + } + flush(); + return { columns: cols, star }; +} + +function resolveString(expr: ts.Expression | undefined): string | null { + if (!expr) return null; + if (ts.isStringLiteralLike(expr)) return expr.text; + if (ts.isIdentifier(expr)) { + const decl = findDeclaration(expr.text, expr); + if (decl && ts.isVariableDeclaration(decl) && decl.initializer) return resolveString(decl.initializer); + } + if (ts.isBinaryExpression(expr) && expr.operatorToken.kind === ts.SyntaxKind.PlusToken) { + const l = resolveString(expr.left), r = resolveString(expr.right); + return l !== null && r !== null ? l + r : null; + } + return null; +} + +/** Inside code that only runs without Supabase (local JSON files). */ +function isFreeTierOnly(n: ts.Node): boolean { + let child: ts.Node = n; + let p: ts.Node | undefined = n.parent; + while (p) { + if (ts.isIfStatement(p)) { + const cond = p.expression.getText(); + const positive = /^hasSupabase\(\)(\s*&&.*)?$/.test(cond); + const negative = /^!hasSupabase\(\)/.test(cond); + if (positive && p.elseStatement === child) return true; + if (negative && p.thenStatement === child) return true; + } + if ((ts.isFunctionDeclaration(p) || ts.isMethodDeclaration(p)) && p.name && /Free$/.test(p.name.getText())) return true; + child = p; + p = p.parent; + } + return false; +} + +function objectProp(obj: ts.Expression | undefined, key: string): ts.Expression | undefined { + if (!obj) return undefined; + if (ts.isIdentifier(obj)) { + const d = findDeclaration(obj.text, obj); + return d && ts.isVariableDeclaration(d) ? objectProp(d.initializer, key) : undefined; + } + if (!ts.isObjectLiteralExpression(obj)) return undefined; + for (const p of obj.properties) { + if (ts.isPropertyAssignment(p) && propName(p) === key) return p.initializer; + if (ts.isShorthandPropertyAssignment(p) && p.name.text === key) { + const d = findDeclaration(key, p); + return d && ts.isVariableDeclaration(d) ? d.initializer : undefined; + } + } + return undefined; +} + +export function scanSources(srcDir: string, exclude: (file: string) => boolean = () => false): ScanResult { + const result: ScanResult = { accesses: [], passThrough: [], tables: [], unresolved: [], freeTierOnly: 0, callSites: 0 }; + const files: string[] = []; + const walk = (d: string) => { + for (const e of fs.readdirSync(d, { withFileTypes: true })) { + const p = path.join(d, e.name); + if (e.isDirectory()) walk(p); + else if (e.name.endsWith(".ts") && !e.name.endsWith(".test.ts") && !e.name.endsWith(".d.ts")) files.push(p); + } + }; + walk(srcDir); + + for (const file of files) { + const rel = path.relative(path.dirname(srcDir), file); + if (exclude(rel)) continue; + const sf = ts.createSourceFile(file, fs.readFileSync(file, "utf-8"), ts.ScriptTarget.Latest, true); + const visit = (n: ts.Node): void => { + if (ts.isCallExpression(n)) inspect(n, sf, rel, result); + ts.forEachChild(n, visit); + }; + visit(sf); + } + return result; +} + +function inspect(call: ts.CallExpression, sf: ts.SourceFile, file: string, r: ScanResult): void { + const c = calleeName(call.expression); + if (!c) return; + const isStorageUpsert = c.name === "upsert" && (c.receiver === "storage" || c.receiver === "getStorage()"); + const kind = WRITE_CALLS.has(c.name) ? "write" : PATCH_CALLS.has(c.name) ? "patch" : QUERY_CALLS.has(c.name) ? "query" + : c.name === "listRecords" ? "list" : c.name === "getRecord" ? "get" : isStorageUpsert ? "storage" : null; + if (!kind) return; + // Skip the definitions themselves (supabase-client.ts calling its own helpers with parameters). + const line = sf.getLineAndCharacterOfPosition(call.getStart()).line + 1; + const callText = c.receiver ? `${c.receiver}.${c.name}` : c.name; + r.callSites++; + if (isFreeTierOnly(call)) { r.freeTierOnly++; return; } + + const args = call.arguments; + const addCols = (table: string, cols: Iterable, k: AccessKind) => { + for (const col of cols) r.accesses.push({ file, line, call: callText, table, column: col, kind: k }); + }; + const unresolved = (what: string) => r.unresolved.push({ file, line, call: callText, what }); + + let tableExpr: ts.Expression | undefined = args[0]; + let table: string | null; + if (kind === "storage") { + const coll = args[0] && ts.isStringLiteralLike(args[0]) ? args[0].text : null; + table = coll ? DEFAULT_PREFIX + coll : null; + } else if (kind === "list") { + tableExpr = objectProp(args[0], "table"); + table = resolveTable(tableExpr); + } else { + table = resolveTable(tableExpr); + } + if (!table) { + const paramOf = (e: ts.Expression | undefined): boolean => { + if (!e) return false; + if (ts.isIdentifier(e)) { + const d = findDeclaration(e.text, e); + if (d && ts.isParameter(d)) return true; + if (d && ts.isVariableDeclaration(d)) return paramOf(d.initializer); + // shorthand `{ table }` inside listRecords({ table, ... }) + return false; + } + if (ts.isCallExpression(e) && ts.isIdentifier(e.expression) && e.expression.text === "getTableName") return paramOf(e.arguments[0]); + return false; + }; + const isParam = paramOf(tableExpr) || (kind === "list" && !tableExpr && !!args[0] && ts.isObjectLiteralExpression(args[0]) && + args[0].properties.some((p) => ts.isShorthandPropertyAssignment(p) && p.name.text === "table" && (() => { const d = findDeclaration("table", p); return !!d && ts.isParameter(d); })())); + const isStorageParam = kind === "storage" && args[0] && ts.isIdentifier(args[0]); + if (isParam || isStorageParam) r.passThrough.push({ file, line, call: callText, what: `table is parameter ${tableExpr?.getText() ?? args[0]?.getText()}` }); + else unresolved(`table ${tableExpr?.getText().slice(0, 50) ?? "(none)"}`); + return; + } + + const keysFrom = (e: ts.Expression | undefined, k: AccessKind, label: string) => { + const ks = resolveKeys(e); + addCols(table!, [...ks.keys].filter((x) => !(k === "filter" && (x === "or" || x === "and" || x === "not"))), k); + if (!ks.complete) unresolved(`${label}: ${ks.why.join("; ")}`); + }; + + switch (kind) { + case "write": + case "storage": + keysFrom(args[1], "write", "payload"); + break; + case "patch": + keysFrom(args[1], "filter", "patch filter"); + keysFrom(args[2], "write", "patch payload"); + break; + case "get": + addCols(table, ["id"], "filter"); + break; + case "query": + case "list": { + const opts = kind === "query" ? args[1] : args[0]; + const selExpr = objectProp(opts, kind === "query" ? "select" : "columns"); + if (selExpr) { + const sel = resolveString(selExpr); + if (sel === null) unresolved(`select ${selExpr.getText().slice(0, 50)}`); + else { + const { columns, star } = selectColumns(sel); + addCols(table, columns, "select"); + if (star || columns.length === 0) r.tables.push({ file, line, call: callText, table }); + } + } else { + r.tables.push({ file, line, call: callText, table }); // default select=* + } + const filt = objectProp(opts, "filters"); + if (filt) keysFrom(filt, "filter", "filters"); + const order = kind === "query" ? resolveString(objectProp(opts, "order")) : null; + if (order) addCols(table, order.split(",").map((o) => o.split(".")[0].trim()).filter(Boolean), "order"); + const orderBy = kind === "list" ? objectProp(opts, "orderBy") : undefined; + if (orderBy) { + const col = resolveString(objectProp(orderBy, "column")); + if (col) addCols(table, [col], "order"); + } + break; + } + } +} + +// ---------------------------------------------------------------- compare + +export interface Finding { + table: string; + /** null: the table itself is missing. */ + column: string | null; + kind: AccessKind | "table"; + sites: string[]; + /** Present in the current schema/setup.sql but not in v1.8.0. */ + addedAfterFloor: boolean; +} + +export function compare(scan: ScanResult, floor: Schema, current: Schema): Finding[] { + const byKey = new Map(); + const note = (table: string, column: string | null, kind: Finding["kind"], site: string) => { + const key = `${table}${column ?? ""}${column === null ? "table" : kind}`; + let f = byKey.get(key); + if (!f) { + const inCurrent = column === null ? current.has(table) : !!current.get(table)?.has(column); + f = { table, column, kind: column === null ? "table" : kind, sites: [], addedAfterFloor: inCurrent }; + byKey.set(key, f); + } + if (!f.sites.includes(site)) f.sites.push(site); + }; + for (const a of scan.accesses) { + const site = `${a.file}:${a.line}`; + if (!floor.has(a.table)) note(a.table, null, "table", site); + else if (!floor.get(a.table)!.has(a.column)) note(a.table, a.column, a.kind, site); + } + for (const t of scan.tables) if (!floor.has(t.table)) note(t.table, null, "table", `${t.file}:${t.line}`); + return [...byKey.values()].sort((x, y) => `${x.table}.${x.column}`.localeCompare(`${y.table}.${y.column}`)); +} diff --git a/tests/unit/commands/migrate-root.test.ts b/tests/unit/commands/migrate-root.test.ts index e26adf7..01f6ade 100644 --- a/tests/unit/commands/migrate-root.test.ts +++ b/tests/unit/commands/migrate-root.test.ts @@ -10,7 +10,10 @@ * merely fail: * * copies, never moves — a wrong call leaves the original intact - * never overwrites — stale memory cannot clobber current memory + * never overwrites — a non-memory file at the destination wins + * merges memory by id — GIT-100: learnings/threads/decisions/sessions + * .json were skipped as "already exists", so a + * destination with any memory received none * reports every skip — a partial merge is never reported as complete * writes where gitmem reads — this one was a real bug: main() computed the * destination with os.homedir() instead of the @@ -67,24 +70,27 @@ describe("GIT-91: migrate-root", () => { expect(read(path.join(source, "learnings.json"))).toBe(before); }); - it("never overwrites a file that already exists in the destination", () => { - // The destination is the live store; the source is by definition the one - // that has not been read recently. Stale must not win. - fs.writeFileSync(path.join(destination, "learnings.json"), '{"learnings":["CURRENT"]}'); + it("never overwrites a non-memory file that already exists in the destination", () => { + // GIT-100: this used learnings.json and asserted it was skipped, i.e. the + // project store's memory was dropped whenever the destination had any. + // Memory files are merged now (below); other files keep this rule. + fs.writeFileSync(path.join(source, "config.json"), '{"project":"old"}'); + fs.writeFileSync(path.join(destination, "config.json"), '{"project":"CURRENT"}'); const plan = migrateRoot(source, destination, false); - expect(read(path.join(destination, "learnings.json"))).toBe('{"learnings":["CURRENT"]}'); - expect(plan.copied).not.toContain("learnings.json"); - expect(plan.skipped.map((s) => s.file)).toContain("learnings.json"); + expect(read(path.join(destination, "config.json"))).toBe('{"project":"CURRENT"}'); + expect(plan.copied).not.toContain("config.json"); + expect(plan.skipped.map((s) => s.file)).toContain("config.json"); }); it("reports why each file was skipped", () => { - fs.writeFileSync(path.join(destination, "learnings.json"), "{}"); + fs.writeFileSync(path.join(source, "config.json"), "{}"); + fs.writeFileSync(path.join(destination, "config.json"), "{}"); const plan = migrateRoot(source, destination, false); - const skip = plan.skipped.find((s) => s.file === "learnings.json"); + const skip = plan.skipped.find((s) => s.file === "config.json"); // A silent partial merge would read as a complete one. expect(skip?.reason).toMatch(/already exists/i); }); @@ -111,3 +117,145 @@ describe("GIT-91: migrate-root", () => { expect(dry.copied.sort()).toEqual(applied.copied.sort()); }); }); + +// =========================================================================== +// GIT-100: memory files are merged by record id, not skipped +// =========================================================================== + +const ts = (h: number) => `2026-09-${String(h).padStart(2, "0")}T00:00:00.000Z`; +const writeJson = (p: string, v: unknown) => fs.writeFileSync(p, JSON.stringify(v, null, 2)); +const readJson = (p: string) => JSON.parse(read(p)); +const ids = (p: string) => readJson(p).map((r: { id: string }) => r.id); + +describe("GIT-100: migrate-root merges memory files by record id", () => { + beforeEach(() => { + for (const f of ["learnings.json", "threads.json", "decisions.json", "sessions.json"]) { + fs.rmSync(path.join(source, f), { force: true }); + } + writeJson(path.join(source, "learnings.json"), [ + { id: "L-src-only", title: "only in project store", updated_at: ts(1) }, + { id: "L-both-src-newer", title: "project edit", updated_at: ts(10) }, + { id: "L-both-dst-newer", title: "stale project copy", updated_at: ts(2) }, + { id: "L-same", title: "identical", updated_at: ts(3) }, + ]); + writeJson(path.join(destination, "learnings.json"), [ + { id: "L-dst-only", title: "only in home store", updated_at: ts(4) }, + { id: "L-both-src-newer", title: "old home copy", updated_at: ts(5) }, + { id: "L-both-dst-newer", title: "home edit", updated_at: ts(9) }, + { updated_at: ts(3), title: "identical", id: "L-same" }, // same record, different key order + ]); + writeJson(path.join(source, "threads.json"), [{ id: "t-src", text: "project thread", created_at: ts(1) }]); + writeJson(path.join(destination, "threads.json"), [{ id: "t-dst", text: "home thread", created_at: ts(2) }]); + writeJson(path.join(source, "decisions.json"), [{ id: "D1", title: "undated A" }]); + writeJson(path.join(destination, "decisions.json"), [{ id: "D1", title: "undated B" }]); + writeJson(path.join(source, "sessions.json"), [{ id: "S-src", session_date: "2026-09-01" }]); + }); + + it("never lists a memory file under skipped", () => { + const plan = migrateRoot(source, destination, false); + const skipped = plan.skipped.map((s) => s.file); + for (const f of ["learnings.json", "threads.json", "decisions.json", "sessions.json"]) expect(skipped).not.toContain(f); + }); + + it("adds new ids, keeps destination-only ids, and the newer record wins", () => { + const plan = migrateRoot(source, destination, false); + const merged = readJson(path.join(destination, "learnings.json")); + const byId = Object.fromEntries(merged.map((r: { id: string; title: string }) => [r.id, r.title])); + + expect(byId).toEqual({ + "L-dst-only": "only in home store", + "L-both-src-newer": "project edit", // source newer: replaced + "L-both-dst-newer": "home edit", // destination newer: kept + "L-same": "identical", + "L-src-only": "only in project store", + }); + expect(plan.merged.find((m) => m.file === "learnings.json")).toMatchObject({ + added: 1, updated: 1, unchanged: 1, kept_destination: 1, conflicts_logged: 2, + }); + expect(ids(path.join(destination, "threads.json")).sort()).toEqual(["t-dst", "t-src"]); + }); + + it("a memory file only in the source is copied", () => { + const plan = migrateRoot(source, destination, false); + expect(plan.copied).toContain("sessions.json"); + expect(ids(path.join(destination, "sessions.json"))).toEqual(["S-src"]); + }); + + it("undated records that differ: the destination is kept and the loser logged", () => { + migrateRoot(source, destination, false); + expect(readJson(path.join(destination, "decisions.json"))).toEqual([{ id: "D1", title: "undated B" }]); + const conflicts = readJson(path.join(destination, "migrate-root-conflicts.json")); + expect(conflicts.find((c: { id: string }) => c.id === "D1")).toMatchObject({ kept: "destination", lost_record: { id: "D1", title: "undated A" } }); + }); + + it("logs every loser, with which side was kept", () => { + migrateRoot(source, destination, false); + const conflicts = readJson(path.join(destination, "migrate-root-conflicts.json")); + const learningConflicts = conflicts.filter((c: { file: string }) => c.file === "learnings.json"); + expect(learningConflicts).toHaveLength(2); + expect(learningConflicts.find((c: { id: string }) => c.id === "L-both-src-newer")).toMatchObject({ + kept: "source", lost_record: { title: "old home copy" }, kept_timestamp: ts(10), lost_timestamp: ts(5), + }); + expect(learningConflicts.find((c: { id: string }) => c.id === "L-both-dst-newer")).toMatchObject({ + kept: "destination", lost_record: { title: "stale project copy" }, + }); + }); + + it("backs up the destination's memory files before writing", () => { + const before = read(path.join(destination, "learnings.json")); + const plan = migrateRoot(source, destination, false); + expect(plan.backup).toBeDefined(); + expect(read(path.join(plan.backup!, "learnings.json"))).toBe(before); + expect(fs.existsSync(path.join(plan.backup!, "threads.json"))).toBe(true); + }); + + it("is idempotent: a re-run changes nothing, backs up nothing, logs nothing new", () => { + migrateRoot(source, destination, false); + const snapshot = ["learnings.json", "threads.json", "decisions.json", "sessions.json", "migrate-root-conflicts.json"] + .map((f) => read(path.join(destination, f))); + + const again = migrateRoot(source, destination, false); + + expect(["learnings.json", "threads.json", "decisions.json", "sessions.json", "migrate-root-conflicts.json"] + .map((f) => read(path.join(destination, f)))).toEqual(snapshot); + expect(again.backup).toBeUndefined(); + for (const m of again.merged) { + expect(m.added).toBe(0); + expect(m.updated).toBe(0); + expect(m.conflicts_logged).toBe(0); + } + }); + + it("leaves the source intact", () => { + const before = read(path.join(source, "learnings.json")); + migrateRoot(source, destination, false); + expect(read(path.join(source, "learnings.json"))).toBe(before); + }); + + it("dry-run writes nothing but reports the same merge", () => { + const before = read(path.join(destination, "learnings.json")); + const dry = migrateRoot(source, destination, true); + expect(read(path.join(destination, "learnings.json"))).toBe(before); + expect(fs.existsSync(path.join(destination, "migrate-root-conflicts.json"))).toBe(false); + expect(fs.existsSync(path.join(destination, "backups"))).toBe(false); + + const applied = migrateRoot(source, destination, false); + expect(dry.merged).toEqual(applied.merged); + }); + + it("an unreadable memory file is reported as NOT merged, never skipped, and left untouched", () => { + fs.writeFileSync(path.join(destination, "threads.json"), "{ not json"); + const plan = migrateRoot(source, destination, false); + expect(plan.failed).toEqual([{ file: "threads.json", reason: expect.stringMatching(/^NOT merged: destination unreadable JSON/) }]); + expect(plan.skipped.map((s) => s.file)).not.toContain("threads.json"); + expect(read(path.join(destination, "threads.json"))).toBe("{ not json"); + }); + + it("does not migrate backups or the conflicts log out of a source that has them", () => { + fs.mkdirSync(path.join(source, "backups", "old"), { recursive: true }); + fs.writeFileSync(path.join(source, "migrate-root-conflicts.json"), "[]"); + const plan = migrateRoot(source, destination, false); + expect(plan.copied.some((f) => f.startsWith("backups"))).toBe(false); + expect(plan.copied).not.toContain("migrate-root-conflicts.json"); + }); +}); diff --git a/tests/unit/schema-drift.test.ts b/tests/unit/schema-drift.test.ts new file mode 100644 index 0000000..d29936b --- /dev/null +++ b/tests/unit/schema-drift.test.ts @@ -0,0 +1,179 @@ +/** + * GIT-109 step 2: nothing gitmem reads or writes may be missing from the + * v1.8.0 setup.sql, unless it is allowlisted against the ticket that owns it. + * + * Standing rule (Chris, 2026-09-21): customers never run SQL after initial + * setup, so every store in the field may still be on the v1.8.0 schema. The + * pinned copy is tests/fixtures/schema/setup-v1.8.0.sql (byte-identical to + * `git show v1.8.0:schema/setup.sql`). + * + * What fails: + * - a table or column reached from src/ that v1.8.0 does not have and that + * is not on ALLOWED below; + * - an ALLOWED entry that no longer matches anything (the list only shrinks); + * - an ALLOWED entry without a GIT-### ticket; + * - a call site the analyzer cannot resolve that is not on KNOWN_UNRESOLVED + * (so coverage cannot silently decay); + * - a SESSION_COLUMNS entry missing from v1.8.0 that is not production-only. + * See tests/helpers/schema-drift.ts for what is collected and how. + */ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { parseSchema, scanSources, compare, selectColumns } from "../helpers/schema-drift.js"; +import type { Finding, ScanResult, Schema } from "../helpers/schema-drift.js"; +import { SESSION_COLUMNS, PRODUCTION_ONLY_SESSION_COLUMNS } from "../../src/services/session-columns.js"; + +const REPO = path.resolve(__dirname, "../.."); +const FLOOR_SQL = path.join(REPO, "tests/fixtures/schema/setup-v1.8.0.sql"); +const CURRENT_SQL = path.join(REPO, "schema/setup.sql"); + +interface Allowed { + table: string; + column: string | null; + kind: Finding["kind"]; + ticket: string; + reason: string; +} + +/** + * Known gaps, each owned by a ticket. Remove an entry when its ticket lands — + * the test fails on an entry that matches nothing. + */ +const ALLOWED: Allowed[] = [ + // Written only where the store has the column (store-columns.ts probe). + { table: "gitmem_learnings", column: "archived_at", kind: "write", ticket: "GIT-109", reason: "probe-guarded: archive-learning.ts writes it only if supportedColumns() finds it" }, + { table: "gitmem_sessions", column: "claude_code_session_id", kind: "write", ticket: "GIT-109", reason: "probe-guarded: production-only, dropped by filterToStoreSessionColumns() on setup.sql stores" }, + { table: "gitmem_sessions", column: "task_observations", kind: "write", ticket: "GIT-109", reason: "probe-guarded: absorb-observations.ts writes it only if supportedColumns() finds it" }, + // Scar variant A/B testing is dev-tier only (hasVariants()); customer schemas do not provision it. + { table: "variant_assignments", column: null, kind: "table", ticket: "GIT-106", reason: "dev tier only; table not in setup.sql" }, + { table: "variant_performance_metrics", column: null, kind: "table", ticket: "GIT-106", reason: "dev tier only; table not in setup.sql" }, + { table: "scar_enforcement_variants", column: "active", kind: "filter", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, + { table: "scar_enforcement_variants", column: "active", kind: "write", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, + { table: "scar_enforcement_variants", column: "description", kind: "write", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, + { table: "scar_enforcement_variants", column: "enforcement_config", kind: "write", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, + { table: "scar_enforcement_variants", column: "variant_name", kind: "write", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, + { table: "scar_enforcement_variants", column: "variant_version", kind: "write", ticket: "GIT-106", reason: "dev tier only; setup.sql's table has a different shape" }, +]; + +/** + * Call sites the analyzer cannot resolve statically, by file and what is + * missing. Each is checked another way or is out of reach; a NEW one fails. + */ +const KNOWN_UNRESOLVED: Array<{ file: string; what: RegExp; why: string }> = [ + { file: "src/services/thread-supabase.ts", what: /^filters: /, why: "filters come from buildScopedThreadQuery() in thread-scope.ts (another file); thread-scope tests pin them" }, + { file: "src/services/write-health.ts", what: /^table /, why: "loops over getTableName(\"learnings\" | \"decisions\"), select id only" }, + { file: "src/tools/session-close.ts", what: /sessionData not resolvable; filtered by filterToStoreSessionColumns/, why: "narrowed to SESSION_COLUMNS, checked against v1.8.0 below" }, +]; + +let floor: Schema; +let current: Schema; +let scan: ScanResult; +let findings: Finding[]; + +beforeAll(() => { + floor = parseSchema(fs.readFileSync(FLOOR_SQL, "utf-8")); + current = parseSchema(fs.readFileSync(CURRENT_SQL, "utf-8")); + scan = scanSources(path.join(REPO, "src")); + findings = compare(scan, floor, current); +}); + +const matches = (a: Allowed, f: Finding) => a.table === f.table && a.column === f.column && a.kind === f.kind; +const label = (f: Finding) => `${f.table}.${f.column ?? "(table)"} [${f.kind}]${f.addedAfterFloor ? " (added after 1.8.0)" : " (in no setup.sql)"} at ${f.sites.join(", ")}`; + +describe("schema parser", () => { + it("reads the v1.8.0 tables, ALTER-added columns and views", () => { + expect([...floor.keys()]).toEqual(expect.arrayContaining([ + "gitmem_learnings", "gitmem_sessions", "gitmem_decisions", "gitmem_scar_usage", "gitmem_threads", + "knowledge_triples", "gitmem_query_metrics", "scar_enforcement_variants", "gitmem_threads_lite", "gitmem_sessions_lite", + ])); + expect(floor.get("gitmem_learnings")).toContain("decay_multiplier"); // ALTER TABLE ... ADD COLUMN + expect(floor.get("gitmem_threads")).toContain("resolved_by_session"); + expect(floor.get("gitmem_threads")).not.toContain("CHECK"); + expect(floor.get("gitmem_sessions_lite")).toContain("close_compliance"); // view + expect(floor.get("gitmem_sessions_lite")).not.toContain("embedding"); + }); + + it("the pinned floor is the v1.8.0 schema, not a later one", () => { + expect(fs.readFileSync(FLOOR_SQL, "utf-8")).toMatch(/refresh_scar_behavioral_scores/); + // The release-B setup.sql re-creates the usage table idempotently (GIT-84); v1.8.0 did not. + expect(fs.readFileSync(FLOOR_SQL, "utf-8")).not.toBe(fs.readFileSync(CURRENT_SQL, "utf-8")); + }); + + it("parses PostgREST select lists", () => { + expect(selectColumns("id,title:t,embedding::text,metadata->>session_id,rel(a,b),*")).toEqual({ + columns: ["id", "t", "embedding", "metadata"], star: true, + }); + }); +}); + +describe("analyzer catches drift (planted in a synthetic source tree)", () => { + let dir: string; + beforeAll(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-drift-")); + fs.mkdirSync(path.join(dir, "src")); + fs.writeFileSync(path.join(dir, "src", "planted.ts"), ` + declare const supabase: any; declare function getTableName(b: string): string; declare function hasSupabase(): boolean; + declare const getStorage: () => any; + export async function a() { + const row = { id: "x", bogus_column: 1, ...(true && { also_bogus: 2 }) }; + row.assigned_later = 3; + await supabase.directUpsert(getTableName("sessions"), row); + await supabase.directQuery("knowledge_triples", { select: "id,no_such_col", filters: { nope: "eq.1" }, order: "gone.desc" }); + await supabase.directPatch(getTableName("threads"), { thread_id: "t" }, { status: "resolved", not_a_col: 1 }); + await supabase.directQuery("missing_table", { select: "id" }); + if (!hasSupabase()) await getStorage().upsert("sessions", { local_only_field: 1 }); + }`); + }); + afterAll(() => fs.rmSync(dir, { recursive: true, force: true })); + + it("reports every planted column and table, and ignores free-tier-only writes", () => { + const s = scanSources(path.join(dir, "src")); + const got = compare(s, floor, current).map((f) => `${f.table}.${f.column ?? "(table)"}:${f.kind}`).sort(); + expect(got).toEqual([ + "gitmem_sessions.also_bogus:write", + "gitmem_sessions.assigned_later:write", + "gitmem_sessions.bogus_column:write", + "gitmem_threads.not_a_col:write", + "knowledge_triples.gone:order", + "knowledge_triples.no_such_col:select", + "knowledge_triples.nope:filter", + "missing_table.(table):table", + ]); + expect(s.freeTierOnly).toBe(1); + }); +}); + +describe("src/ against the v1.8.0 schema", () => { + it("scans a meaningful surface", () => { + expect(scan.callSites).toBeGreaterThan(50); + expect(scan.accesses.length).toBeGreaterThan(300); + }); + + it("every table and column is in v1.8.0 or allowlisted by ticket", () => { + const unexpected = findings.filter((f) => !ALLOWED.some((a) => matches(a, f))); + expect(unexpected.map(label), "not in v1.8.0 setup.sql and not allowlisted").toEqual([]); + }); + + it("every allowlist entry still matches a finding, and names a ticket", () => { + for (const a of ALLOWED) { + expect(a.ticket, `${a.table}.${a.column}`).toMatch(/^GIT-\d+$/); + expect(findings.some((f) => matches(a, f)), `stale ALLOWED entry ${a.table}.${a.column} [${a.kind}] (${a.ticket}) — remove it`).toBe(true); + } + }); + + it("no call site is unresolved except the known ones", () => { + const unknown = scan.unresolved.filter((u) => !KNOWN_UNRESOLVED.some((k) => k.file === u.file && k.what.test(u.what))); + expect(unknown.map((u) => `${u.file}:${u.line} ${u.call} — ${u.what}`)).toEqual([]); + for (const k of KNOWN_UNRESOLVED) { + expect(scan.unresolved.some((u) => u.file === k.file && k.what.test(u.what)), `stale KNOWN_UNRESOLVED ${k.file}`).toBe(true); + } + }); + + it("session writes: SESSION_COLUMNS is v1.8.0 plus the probe-guarded production-only set", () => { + const sessions = floor.get("gitmem_sessions")!; + const missing = [...SESSION_COLUMNS].filter((c) => !sessions.has(c) && !PRODUCTION_ONLY_SESSION_COLUMNS.has(c)); + expect(missing, "SESSION_COLUMNS entries a v1.8.0 store rejects").toEqual([]); + }); +}); diff --git a/tests/unit/services/active-sessions.test.ts b/tests/unit/services/active-sessions.test.ts index 8e163a2..2452783 100644 --- a/tests/unit/services/active-sessions.test.ts +++ b/tests/unit/services/active-sessions.test.ts @@ -123,6 +123,17 @@ describe("registerSession", () => { expect(listActiveSessions()[0].session_id).toBe("22222222-2222-2222-2222-222222222222"); }); + it("does not displace a session of another project on the same hostname+pid (GIT-86)", () => { + // One desktop process serves every chat: two projects share hostname+pid. + registerSession(makeEntry({ session_id: "11111111-1111-1111-1111-111111111111", hostname: "host-a", pid: 100, project: "proj-x" })); + const displaced = registerSession( + makeEntry({ session_id: "22222222-2222-2222-2222-222222222222", hostname: "host-a", pid: 100, project: "proj-y" }) + ); + + expect(displaced).toEqual([]); + expect(listActiveSessions().map((s) => s.project).sort()).toEqual(["proj-x", "proj-y"]); + }); + it("returns empty array when no sessions displaced", () => { const entry = makeEntry(); const displaced = registerSession(entry); @@ -224,6 +235,20 @@ describe("findSessionByHostPid", () => { expect(findSessionByHostPid("test-host", 1111)).toBeNull(); }); + it("with a project, matches only a session of that project (GIT-86)", () => { + registerSession(makeEntry({ session_id: "11111111-1111-1111-1111-111111111111", hostname: "h", pid: 7, project: "proj-x" })); + + expect(findSessionByHostPid("h", 7, "proj-y")).toBeNull(); + expect(findSessionByHostPid("h", 7, "proj-x")?.session_id).toBe("11111111-1111-1111-1111-111111111111"); + }); + + it("without a project, returns the most recently started session of the process (GIT-86)", () => { + registerSession(makeEntry({ session_id: "22222222-2222-2222-2222-222222222222", hostname: "h", pid: 7, project: "proj-y", started_at: new Date().toISOString() })); + registerSession(makeEntry({ session_id: "11111111-1111-1111-1111-111111111111", hostname: "h", pid: 7, project: "proj-x", started_at: new Date(Date.now() - 60_000).toISOString() })); + + expect(findSessionByHostPid("h", 7)?.session_id).toBe("22222222-2222-2222-2222-222222222222"); + }); + it("requires both hostname AND pid to match", () => { registerSession(makeEntry({ hostname: "host-A", pid: 100 })); registerSession(makeEntry({ @@ -632,6 +657,16 @@ describe("findResumableSessionOnDisk (GIT-89)", () => { ); }); + it("with a project, never resolves a session of another project (GIT-86)", () => { + seedDiskOnlySession("11111111-1111-1111-1111-111111111111", { project: "proj-x" }); + seedDiskOnlySession("22222222-2222-2222-2222-222222222222", { project: "proj-y", pid: DEAD_PID }); + + expect(findResumableSessionOnDisk("proj-z")).toBeNull(); + expect(findResumableSessionOnDisk("proj-x")?.session_id).toBe("11111111-1111-1111-1111-111111111111"); + // proj-y's orphan is adoptable for proj-y and left alone by the proj-x lookup above. + expect(findResumableSessionOnDisk("proj-y")?.session_id).toBe("22222222-2222-2222-2222-222222222222"); + }); + it("returns null when there is nothing on disk", () => { expect(findResumableSessionOnDisk()).toBeNull(); }); diff --git a/tests/unit/services/gitmem-root-symlink.test.ts b/tests/unit/services/gitmem-root-symlink.test.ts new file mode 100644 index 0000000..a7ec2cc --- /dev/null +++ b/tests/unit/services/gitmem-root-symlink.test.ts @@ -0,0 +1,103 @@ +/** + * GIT-107: roots are compared as directories, not strings. + * + * On macOS /var is a symlink to /private/var. process.cwd() reports the + * resolved /private/var/... while a home under $TMPDIR is spelled /var/..., so + * the store being read was announced as "Memory store found but NOT being + * read". Any symlink in either path does the same. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { findStrandedProjectRoots, sameDirectory, canonicalPath, clearGitmemDirCache } from "../../../src/services/gitmem-dir.js"; + +let tmp: string; +const saved = { home: process.env.GITMEM_HOME, dir: process.env.GITMEM_DIR }; + +/** A live project-scoped store: config.json marks a deliberate install. */ +function liveStore(dir: string): string { + const g = path.join(dir, ".gitmem"); + fs.mkdirSync(g, { recursive: true }); + fs.writeFileSync(path.join(g, "config.json"), JSON.stringify({ project: "p" })); + return g; +} + +beforeEach(() => { + tmp = fs.realpathSync.native(fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git107-"))); + delete process.env.GITMEM_DIR; + clearGitmemDirCache(); + vi.spyOn(console, "error").mockImplementation(() => {}); +}); + +afterEach(() => { + vi.restoreAllMocks(); + if (saved.home === undefined) delete process.env.GITMEM_HOME; else process.env.GITMEM_HOME = saved.home; + if (saved.dir === undefined) delete process.env.GITMEM_DIR; else process.env.GITMEM_DIR = saved.dir; + clearGitmemDirCache(); + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +describe("GIT-107: root comparison survives symlinks", () => { + it("home reached through a symlink is not reported as a stranded store", () => { + const real = path.join(tmp, "real-home"); + fs.mkdirSync(real); + liveStore(real); // the store gitmem reads + const link = path.join(tmp, "link-home"); + fs.symlinkSync(real, link, "dir"); + + process.env.GITMEM_HOME = link; // home spelled via the symlink + vi.spyOn(process, "cwd").mockReturnValue(path.join(real, "work")); // cwd spelled resolved + + expect(findStrandedProjectRoots()).toEqual([]); + }); + + it("a genuinely different live store is still reported", () => { + const home = path.join(tmp, "home"); + fs.mkdirSync(home); + const project = path.join(tmp, "project"); + const stranded = liveStore(project); + process.env.GITMEM_HOME = home; + vi.spyOn(process, "cwd").mockReturnValue(path.join(project, "src")); + + expect(findStrandedProjectRoots()).toEqual([stranded]); + }); + + it("an explicit GITMEM_DIR naming the project store is the store being read, not a stranded one", () => { + const project = path.join(tmp, "project"); + const store = liveStore(project); + process.env.GITMEM_HOME = path.join(tmp, "home"); + process.env.GITMEM_DIR = path.join(tmp, "project", ".", ".gitmem"); + vi.spyOn(process, "cwd").mockReturnValue(project); + + expect(findStrandedProjectRoots()).not.toContain(store); + }); + + it("sameDirectory: symlinks, trailing slashes, dot segments and not-yet-existing leaves", () => { + const real = path.join(tmp, "r"); + fs.mkdirSync(real); + const link = path.join(tmp, "l"); + fs.symlinkSync(real, link, "dir"); + + expect(sameDirectory(link, real)).toBe(true); + expect(sameDirectory(`${real}/`, path.join(real, ".", "."))).toBe(true); + // Destination roots may not exist yet; they resolve through their parent. + expect(sameDirectory(path.join(link, ".gitmem"), path.join(real, ".gitmem"))).toBe(true); + expect(canonicalPath(path.join(link, "a", "b"))).toBe(path.join(real, "a", "b")); + expect(sameDirectory(path.join(tmp, "x"), path.join(tmp, "y"))).toBe(false); + }); + + // The reported case: macOS /var -> /private/var. + const tmpdirReal = fs.realpathSync.native(os.tmpdir()); + it.skipIf(tmpdirReal === path.resolve(os.tmpdir()))("macOS: /var/... and /private/var/... are one store", () => { + const spelledTmp = path.join(os.tmpdir(), path.basename(tmp)); // /var/folders/... spelling + const real = path.join(tmp, "home"); + fs.mkdirSync(real); + liveStore(real); + process.env.GITMEM_HOME = path.join(spelledTmp, "home"); + vi.spyOn(process, "cwd").mockReturnValue(real); // what process.cwd() reports: /private/var/... + + expect(process.env.GITMEM_HOME).not.toBe(real); + expect(findStrandedProjectRoots()).toEqual([]); + }); +}); diff --git a/tests/unit/services/metrics-session-fk.test.ts b/tests/unit/services/metrics-session-fk.test.ts new file mode 100644 index 0000000..3ff787a --- /dev/null +++ b/tests/unit/services/metrics-session-fk.test.ts @@ -0,0 +1,74 @@ +/** + * GIT-73: a metrics row never references a session row that is not there yet. + * + * session_start created its session row fire-and-forget and wrote its metrics + * row at the same moment; gitmem_query_metrics.session_id is an FK to + * gitmem_sessions(id), so whichever arrived first decided whether the metrics + * insert 409'd. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const writes = vi.hoisted(() => ({ log: [] as Array<{ table: string; row: Record; at: number }> })); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + directUpsert: vi.fn(async (table: string, row: Record) => { + writes.log.push({ table, row, at: Date.now() }); + return row; + }), +})); +vi.mock("../../../src/services/tier.js", async (orig) => ({ + ...(await orig()), + hasSupabase: () => true, +})); + +import { recordMetrics, registerPendingSessionRow, clearPendingSessionRows } from "../../../src/services/metrics.js"; + +const SID = "393adb34-a80c-4c3a-b71a-bc0053b7a7ea"; +const metric = (extra: Record = {}) => ({ + id: "m-" + Math.random().toString(16).slice(2), session_id: SID, tool_name: "session_start", + latency_ms: 1, result_count: 0, metadata: { project: "p" }, ...extra, +}) as never; + +beforeEach(() => { writes.log = []; clearPendingSessionRows(); }); + +describe("metrics wait for their session row (GIT-73)", () => { + it("waits until the session row has landed, then keeps the FK", async () => { + let land!: (v: boolean) => void; + registerPendingSessionRow(SID, new Promise((r) => (land = r))); + + const pending = recordMetrics(metric()); + await new Promise((r) => setTimeout(r, 30)); + expect(writes.log).toHaveLength(0); // held back while the session row is in flight + + land(true); + await pending; + expect(writes.log).toHaveLength(1); + expect(writes.log[0].row.session_id).toBe(SID); + }); + + it("a session row that did not land: no FK, id kept in metadata.session_id", async () => { + registerPendingSessionRow(SID, Promise.resolve(false)); + await recordMetrics(metric()); + expect(writes.log[0].row.session_id).toBeNull(); + expect(writes.log[0].row.metadata).toEqual({ project: "p", session_id: SID }); + }); + + it("a session row whose creation threw is treated as not landed", async () => { + registerPendingSessionRow(SID, Promise.reject(new Error("store down"))); + await recordMetrics(metric()); + expect(writes.log[0].row.session_id).toBeNull(); + }); + + it("later writes for the same session reuse the settled answer", async () => { + registerPendingSessionRow(SID, Promise.resolve(true)); + await recordMetrics(metric()); + await recordMetrics(metric({ tool_name: "recall" })); + expect(writes.log.map((w) => w.row.session_id)).toEqual([SID, SID]); + }); + + it("a session this process did not create (resumed, or another process's) is written immediately", async () => { + await recordMetrics(metric()); + expect(writes.log).toHaveLength(1); + expect(writes.log[0].row.session_id).toBe(SID); + }); +}); diff --git a/tests/unit/services/thread-triples-write.test.ts b/tests/unit/services/thread-triples-write.test.ts new file mode 100644 index 0000000..cb7f84a --- /dev/null +++ b/tests/unit/services/thread-triples-write.test.ts @@ -0,0 +1,68 @@ +/** + * GIT-105: thread triples reach knowledge_triples on a v1.8.0 store. + * + * source_id is a UUID column; thread ids are "t-xxxxxxxx". Every + * create_thread / resolve_thread triple 400'd (22P02) and the failure was + * swallowed. The writer now stores the thread's row id (gitmem_threads.id), + * looking it up when the caller does not have it, and never sends a non-UUID. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const store = vi.hoisted(() => ({ + upserts: [] as Array>, + queries: [] as Array<{ table: string; filters: Record }>, + threadRows: {} as Record, + queryFails: false, +})); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + directUpsert: vi.fn(async (_t: string, row: Record) => { store.upserts.push(row); return row; }), + directQuery: vi.fn(async (table: string, opts: { filters: Record }) => { + store.queries.push({ table, filters: opts.filters }); + if (store.queryFails) throw new Error("store down"); + const id = store.threadRows[opts.filters.thread_id]; + return id ? [{ id }] : []; + }), +})); +vi.mock("../../../src/services/tier.js", async (orig) => ({ + ...(await orig()), + hasSupabase: () => true, +})); + +import { writeTriplesForThreadCreation, writeTriplesForThreadResolution, writeTriples } from "../../../src/services/triple-writer.js"; + +const ROW = "7d1c9a52-3f4e-4b8a-9c21-5e6f7a8b9c0d"; +const SESSION = "550e8400-e29b-41d4-a716-446655440000"; +const base = { thread_id: "t-abc12345", text: "Fix auth timeout", session_id: SESSION, project: "p", agent: "cli" }; + +beforeEach(() => { + store.upserts = []; store.queries = []; store.threadRows = {}; store.queryFails = false; +}); + +describe("thread triples on a UUID source_id column (GIT-105)", () => { + it("creation: uses the row id create_thread already has — no lookup", async () => { + await writeTriplesForThreadCreation({ ...base, thread_row_id: ROW }); + expect(store.queries).toHaveLength(0); + expect(store.upserts.map((r) => r.source_id)).toEqual([ROW]); + }); + + it("resolution: looks the row id up by thread_id", async () => { + store.threadRows["t-abc12345"] = ROW; + await writeTriplesForThreadResolution(base); + expect(store.queries).toEqual([{ table: "gitmem_threads", filters: { thread_id: "t-abc12345" } }]); + expect(store.upserts.map((r) => r.source_id)).toEqual([ROW]); + }); + + it("thread not in the store, or the lookup fails: source_id null, triple still written", async () => { + await writeTriplesForThreadResolution(base); + store.queryFails = true; + await writeTriplesForThreadResolution(base); + expect(store.upserts.map((r) => r.source_id)).toEqual([null, null]); + }); + + it("never sends a non-UUID source_id, whatever the caller passed", async () => { + await writeTriples([{ subject: "s", predicate: "created_thread", object: "o", source_type: "thread", + source_id: "t-abc12345", project: "p", half_life_days: 9999, created_by: "cli" }]); + expect(store.upserts[0].source_id).toBeNull(); + }); +}); diff --git a/tests/unit/services/thread-triples.test.ts b/tests/unit/services/thread-triples.test.ts index 8de0a03..fd2853c 100644 --- a/tests/unit/services/thread-triples.test.ts +++ b/tests/unit/services/thread-triples.test.ts @@ -17,8 +17,13 @@ import type { // ---------- Helpers ---------- +// GIT-105: source_id is the thread's row id (gitmem_threads.id, a UUID), not +// its "t-" thread_id — knowledge_triples.source_id is a UUID column. +const THREAD_ROW_ID = "7d1c9a52-3f4e-4b8a-9c21-5e6f7a8b9c0d"; + const BASE_CREATION_PARAMS: ThreadCreationTripleParams = { thread_id: "t-abc12345", + thread_row_id: THREAD_ROW_ID, text: "Fix auth timeout in production", session_id: "550e8400-e29b-41d4-a716-446655440000", linear_issue: "PROJ-123", @@ -28,6 +33,7 @@ const BASE_CREATION_PARAMS: ThreadCreationTripleParams = { const BASE_RESOLUTION_PARAMS: ThreadResolutionTripleParams = { thread_id: "t-abc12345", + thread_row_id: THREAD_ROW_ID, text: "Fix auth timeout in production", resolution_note: "Fixed by increasing timeout to 30s", session_id: "660e8400-e29b-41d4-a716-446655440000", @@ -88,7 +94,8 @@ describe("extractThreadCreationTriples", () => { for (const triple of triples) { expect(triple.source_type).toBe("thread"); - expect(triple.source_id).toBe("t-abc12345"); + // GIT-105: was "t-abc12345", which the UUID column rejects (22P02). + expect(triple.source_id).toBe(THREAD_ROW_ID); expect(triple.half_life_days).toBe(9999); expect(triple.project).toBe("test-project"); expect(triple.created_by).toBe("CLI"); @@ -129,7 +136,17 @@ describe("extractThreadResolutionTriples", () => { expect(triple.subject).toMatch(/^Session: /); expect(triple.object).toMatch(/^Thread: /); expect(triple.source_type).toBe("thread"); - expect(triple.source_id).toBe("t-abc12345"); + // GIT-105: was "t-abc12345", which the UUID column rejects (22P02). + expect(triple.source_id).toBe(THREAD_ROW_ID); + }); + + it("without a row id, source_id is null — never the thread_id (GIT-105)", () => { + for (const t of [ + ...extractThreadCreationTriples({ ...BASE_CREATION_PARAMS, thread_row_id: null }), + ...extractThreadResolutionTriples({ ...BASE_RESOLUTION_PARAMS, thread_row_id: undefined }), + ]) { + expect(t.source_id).toBeNull(); + } }); }); diff --git a/tests/unit/services/vector-delta-sync.test.ts b/tests/unit/services/vector-delta-sync.test.ts new file mode 100644 index 0000000..332521b --- /dev/null +++ b/tests/unit/services/vector-delta-sync.test.ts @@ -0,0 +1,180 @@ +/** + * GIT-98: per-row delta sync. + * + * A fingerprint miss used to cost the whole index. On a Pro store it misses on + * nearly every session_start, because refresh_scar_behavioral_scores() bumps + * updated_at on every scar with enough recent usage. Now a miss with a cache + * from this store downloads the manifest (id, updated_at) and only the rows + * whose updated_at moved. + * + * Real filesystem cache, isolated GITMEM_DIR; the store is mocked at the + * supabase-client boundary so each call can be counted. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +const store = vi.hoisted(() => ({ + rows: [] as Array<{ id: string; title: string; updated_at: string; embedding: number[] }>, + manifestFails: false, + calls: { fingerprint: 0, manifest: 0, byIds: [] as string[][], bulk: 0 }, +})); + +const active = () => [...store.rows].sort((a, b) => b.updated_at.localeCompare(a.updated_at)); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + isConfigured: () => true, + getSupabaseUrl: () => "https://venue.example.supabase.co", + getLearningsFingerprint: vi.fn(async () => { + store.calls.fingerprint++; + const rows = active(); + return { count: rows.length, latestUpdatedAt: rows[0]?.updated_at ?? null }; + }), + loadLearningsManifest: vi.fn(async () => { + store.calls.manifest++; + if (store.manifestFails) throw new Error("manifest 500"); + return active().map(({ id, updated_at }) => ({ id, updated_at })); + }), + loadLearningsByIds: vi.fn(async (ids: string[]) => { + store.calls.byIds.push(ids); + return active().filter((r) => ids.includes(r.id)).map((r) => ({ ...r })); + }), + loadScarsWithEmbeddings: vi.fn(async () => { + store.calls.bulk++; + return active().map((r) => ({ ...r })); + }), +})); + +import { loadScarsFromSupabase } from "../../../src/services/startup.js"; +import { planDelta, assembleDelta } from "../../../src/services/vector-disk-cache.js"; + +let dir: string; +const ts = (n: number) => new Date(Date.UTC(2026, 8, 21, 0, 0, n)).toISOString(); +const vec = (i: number) => Array.from({ length: 8 }, (_, k) => i + k / 10); + +function seed(n: number) { + store.rows = Array.from({ length: n }, (_, i) => ({ id: `id-${String(i).padStart(3, "0")}`, title: `t${i}`, updated_at: ts(i), embedding: vec(i) })); +} +/** What refresh_scar_behavioral_scores() does to a scar: new score, new updated_at. */ +function rewrite(ids: string[], at: number) { + for (const r of store.rows) if (ids.includes(r.id)) { r.updated_at = ts(at); r.title = `${r.title}*`; r.embedding = vec(1000 + at); } +} +const resetCalls = () => { store.calls = { fingerprint: 0, manifest: 0, byIds: [], bulk: 0 }; }; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-vdelta-")); + vi.stubEnv("GITMEM_DIR", dir); + vi.stubEnv("GITMEM_VECTOR_DISK_CACHE", ""); + store.manifestFails = false; + seed(250); + resetCalls(); +}); + +afterEach(() => { + vi.unstubAllEnvs(); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +describe("planDelta / assembleDelta", () => { + const cached = [ + { id: "a", updated_at: "1", v: "a-old" }, + { id: "b", updated_at: "1", v: "b-old" }, + { id: "gone", updated_at: "1", v: "gone" }, + { id: "no-ts", v: "no-ts" }, + ]; + const manifest = [ + { id: "new", updated_at: "3" }, + { id: "b", updated_at: "2" }, + { id: "a", updated_at: "1" }, + { id: "no-ts", updated_at: "1" }, + ]; + + it("reuses only id+updated_at matches, fetches changed, new and timestamp-less rows, counts dropped ones", () => { + const plan = planDelta(cached as never[], manifest); + expect([...plan.reuse.keys()]).toEqual(["a"]); + expect(plan.fetchIds).toEqual(["new", "b", "no-ts"]); + expect(plan.dropped).toBe(1); + }); + + it("assembles in manifest order and omits ids that vanished before the fetch", () => { + const plan = planDelta(cached as never[], manifest); + const rows = assembleDelta(manifest, plan, [{ id: "b", v: "b-new" }, { id: "no-ts", v: "no-ts-new" }] as never[]); + expect(rows.map((r: { v: string }) => r.v)).toEqual(["b-new", "a-old", "no-ts-new"]); + }); +}); + +describe("loadScarsFromSupabase delta sync (GIT-98)", () => { + it("cold start: one full download, then a warm start costs only the fingerprint", async () => { + const cold = await loadScarsFromSupabase(); + expect(cold.scars).toHaveLength(250); + expect(store.calls.bulk).toBe(1); + + resetCalls(); + const warm = await loadScarsFromSupabase(); + expect(warm.source).toBe("disk"); + expect(store.calls).toEqual({ fingerprint: 1, manifest: 0, byIds: [], bulk: 0 }); + }); + + it("5 rows rewritten: fetches exactly those 5 vectors, not 250, and matches a full download", async () => { + await loadScarsFromSupabase(); + const changed = ["id-007", "id-042", "id-099", "id-150", "id-249"]; + rewrite(changed, 5000); + resetCalls(); + + const synced = await loadScarsFromSupabase(); + + expect(store.calls.bulk).toBe(0); + expect(store.calls.manifest).toBe(1); + expect(store.calls.byIds.flat().sort()).toEqual([...changed].sort()); + // Identical to what a full download would have returned, order included. + expect(synced.scars).toEqual(active()); + expect(synced.latestUpdatedAt).toBe(ts(5000)); + + // The rewritten cache serves the next start from disk. + resetCalls(); + expect((await loadScarsFromSupabase()).source).toBe("disk"); + expect(store.calls.byIds).toEqual([]); + }); + + it("inserts and deactivations: fetches the new row, drops the gone one", async () => { + await loadScarsFromSupabase(); + store.rows = store.rows.filter((r) => r.id !== "id-010"); + store.rows.push({ id: "id-new", title: "fresh", updated_at: ts(6000), embedding: vec(6000) }); + resetCalls(); + + const synced = await loadScarsFromSupabase(); + expect(store.calls.bulk).toBe(0); + expect(store.calls.byIds.flat()).toEqual(["id-new"]); + expect(synced.scars.map((r) => r.id)).toEqual(active().map((r) => r.id)); + expect(synced.scars.some((r) => r.id === "id-010")).toBe(false); + }); + + it("a failed manifest falls back to the full download", async () => { + await loadScarsFromSupabase(); + rewrite(["id-001"], 7000); + store.manifestFails = true; + resetCalls(); + + const loaded = await loadScarsFromSupabase(); + expect(store.calls.bulk).toBe(1); + expect(loaded.scars).toEqual(active()); + }); + + it("cache-flush (bypassDiskCache) always takes the full download", async () => { + await loadScarsFromSupabase(); + rewrite(["id-001"], 8000); + resetCalls(); + + await loadScarsFromSupabase({ bypassDiskCache: true }); + expect(store.calls.manifest).toBe(0); + expect(store.calls.bulk).toBe(1); + }); + + it("no cache from this store: full download, no manifest", async () => { + const loaded = await loadScarsFromSupabase(); + expect(store.calls.manifest).toBe(0); + expect(store.calls.bulk).toBe(1); + expect(loaded.scars).toHaveLength(250); + }); +}); diff --git a/tests/unit/services/write-health-probe.test.ts b/tests/unit/services/write-health-probe.test.ts new file mode 100644 index 0000000..b3e7b8a --- /dev/null +++ b/tests/unit/services/write-health-probe.test.ts @@ -0,0 +1,48 @@ +/** + * GIT-102: checkWritePath's verdicts, store mocked at the client. + * + * A probe that errors for any reason other than a missing table used to be + * ignored, so a store refusing every connection was reported "Write-path OK". + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const probe = vi.hoisted(() => ({ impl: async (): Promise => [] })); + +vi.mock("../../../src/services/supabase-client.js", () => ({ + isConfigured: () => true, + directQuery: vi.fn(() => probe.impl()), +})); +vi.mock("../../../src/services/tier.js", () => ({ + getTier: () => "pro", + hasSupabase: () => true, + getTablePrefix: () => "gitmem_", + getTableName: (b: string) => `gitmem_${b}`, +})); + +import { checkWritePath } from "../../../src/services/write-health.js"; + +beforeEach(() => { probe.impl = async () => []; }); + +describe("checkWritePath verdicts (GIT-102)", () => { + it("tables answer: supabase, ok", async () => { + expect(await checkWritePath()).toEqual({ ok: true, mode: "supabase" }); + }); + + it("a missing table: missing_tables", async () => { + probe.impl = async () => { throw new Error("PGRST205 Could not find the table"); }; + expect(await checkWritePath()).toMatchObject({ ok: false, mode: "missing_tables", missing: ["gitmem_learnings", "gitmem_decisions"] }); + }); + + it("the store refuses the connection: unreachable, NOT ok (was: Write-path OK)", async () => { + probe.impl = async () => { throw new TypeError("fetch failed: ECONNREFUSED"); }; + const r = await checkWritePath(); + expect(r.ok).toBe(false); + expect(r.mode).toBe("unreachable"); + expect(r.error).toContain("ECONNREFUSED"); + }); + + it("an auth failure is unreachable too, not OK", async () => { + probe.impl = async () => { throw new Error("Supabase query error: 401 - Invalid API key"); }; + expect(await checkWritePath()).toMatchObject({ ok: false, mode: "unreachable" }); + }); +}); diff --git a/tests/unit/services/write-health-timeout.test.ts b/tests/unit/services/write-health-timeout.test.ts new file mode 100644 index 0000000..b573ab7 --- /dev/null +++ b/tests/unit/services/write-health-timeout.test.ts @@ -0,0 +1,111 @@ +/** + * GIT-102: diagnostics that say what they know and what they don't. + * + * 1. The startup write-path probe is raced against a deadline. A store that + * never answers yields "timed out, durability UNVERIFIED" — not silence. + * The last verdict is kept for `health`, and a late answer replaces it. + * 2. session_close's WARN names the subsystem, the cause, and always whether + * the session content itself was stored. + */ +import { describe, it, expect, beforeEach } from "vitest"; +import { + checkWritePathWithTimeout, + getLastWritePathVerdict, + resetWritePathVerdict, + formatWritePathLine, + WRITE_PATH_TIMEOUT_MS, +} from "../../../src/services/write-health.js"; +import type { WritePathResult } from "../../../src/services/write-health.js"; +import { formatWriteWarnings } from "../../../src/tools/session-close.js"; + +const never = () => new Promise(() => {}); +const after = (ms: number, r: WritePathResult) => () => new Promise((res) => setTimeout(() => res(r), ms)); + +beforeEach(() => resetWritePathVerdict()); + +describe("write-path check timeout (GIT-102)", () => { + it("defaults to a 10 s budget", () => { + expect(WRITE_PATH_TIMEOUT_MS).toBe(10_000); + }); + + it("a probe that never answers resolves at the deadline as timed_out, durability UNVERIFIED", async () => { + const t0 = Date.now(); + const v = await checkWritePathWithTimeout(60, never); + expect(Date.now() - t0).toBeLessThan(1000); + expect(v.ok).toBe(false); + expect(v.mode).toBe("timed_out"); + expect(v.summary).toMatch(/timed out after .* durability UNVERIFIED/); + expect(getLastWritePathVerdict()?.mode).toBe("timed_out"); + }); + + it("names the configured budget in seconds", async () => { + const v = await checkWritePathWithTimeout(10_000, () => Promise.resolve({ ok: false, mode: "timed_out" })); + expect(v.summary).toBe("timed out after 10 s, durability UNVERIFIED"); + }); + + it("a fast probe's verdict is recorded as-is", async () => { + const v = await checkWritePathWithTimeout(1000, () => Promise.resolve({ ok: true, mode: "supabase" })); + expect(v.mode).toBe("supabase"); + expect(v.late).toBeUndefined(); + expect(getLastWritePathVerdict()).toEqual(v); + }); + + it("a probe that answers after the deadline replaces timed_out, marked late", async () => { + const v = await checkWritePathWithTimeout(30, after(90, { ok: false, mode: "missing_tables", missing: ["gitmem_learnings"] })); + expect(v.mode).toBe("timed_out"); + await new Promise((r) => setTimeout(r, 120)); + const latest = getLastWritePathVerdict()!; + expect(latest.mode).toBe("missing_tables"); + expect(latest.late).toBe(true); + expect(latest.summary).toContain("gitmem_learnings"); + }); + + it("health's line says not-checked, UNVERIFIED, or ok — never nothing", async () => { + expect(formatWritePathLine(null)).toMatch(/not checked yet/); + const timedOut = await checkWritePathWithTimeout(20, never); + expect(formatWritePathLine(timedOut)).toMatch(/^Write path: timed out after .* durability UNVERIFIED \(NOT OK, checked /); + const ok = await checkWritePathWithTimeout(1000, () => Promise.resolve({ ok: true, mode: "supabase" })); + expect(formatWritePathLine(ok)).toMatch(/^Write path: Supabase — .* \(ok, checked /); + }); +}); + +describe("session_close WARN lines (GIT-102)", () => { + const report = (byPath: Record) => ({ + byPath: Object.fromEntries(Object.entries(byPath).map(([k, v]) => [k, { + attempted: v.attempted, succeeded: v.attempted - v.failed, failed: v.failed, successRate: "", avgDurationMs: 0, + ...(v.error && { lastFailure: { error: v.error, timestamp: "" } }), + }])), + overall: { attempted: 0, succeeded: 0, failed: 0, successRate: "", paths_with_failures: [] }, + recentFailures: [], + uptimeMs: 0, + }); + + it("names subsystem and cause, and says the session content was stored", () => { + const lines = formatWriteWarnings(report({ + triple_write: { attempted: 3, failed: 3, error: 'POST knowledge_triples 400: invalid input syntax for type uuid: "t-abc"' }, + embedding: { attempted: 1, failed: 0 }, + }), true); + expect(lines).toHaveLength(1); + expect(lines[0]).toMatch(/^knowledge graph \(triples\): 3 of 3 writes failed — POST knowledge_triples 400: invalid input syntax/); + expect(lines[0]).toMatch(/· session content stored OK( \(local\))?$/); + }); + + it("says NOT stored when the session row did not land — on every line", () => { + const lines = formatWriteWarnings(report({ + triple_write: { attempted: 1, failed: 1, error: "boom" }, + transcript: { attempted: 1, failed: 1, error: "bucket missing" }, + }), false); + expect(lines).toHaveLength(2); + for (const l of lines) expect(l).toMatch(/· session content NOT stored$/); + expect(lines[1]).toMatch(/^transcript upload: 1 of 1 write failed — bucket missing/); + }); + + it("an unknown subsystem is named by its path, a missing cause is said to be missing", () => { + const [line] = formatWriteWarnings(report({ new_path: { attempted: 2, failed: 1 } }), true); + expect(line).toMatch(/^new_path: 1 of 2 writes failed — no error message recorded · session content stored OK/); + }); + + it("no failures, no WARN", () => { + expect(formatWriteWarnings(report({ triple_write: { attempted: 2, failed: 0 } }), true)).toEqual([]); + }); +}); diff --git a/tests/unit/tools/session-close-bridge.test.ts b/tests/unit/tools/session-close-bridge.test.ts index 2e84f65..6224f8d 100644 --- a/tests/unit/tools/session-close-bridge.test.ts +++ b/tests/unit/tools/session-close-bridge.test.ts @@ -165,6 +165,7 @@ vi.mock("../../../src/services/gitmem-dir.js", () => ({ vi.mock("../../../src/services/active-sessions.js", () => ({ unregisterSession: () => {}, findSessionByHostPid: () => null, + findSessionById: () => null, // GIT-86: session-close's registry fallback looks up by id })); vi.mock("../../../src/services/thread-suggestions.js", () => ({ 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..0f89d50 --- /dev/null +++ b/tests/unit/tools/session-close-payload-path.test.ts @@ -0,0 +1,204 @@ +/** + * 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), + findSessionById: vi.fn().mockReturnValue(null), // GIT-86: session-close's registry fallback looks up by id +})); + +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..1e40800 100644 --- a/tests/unit/tools/session-close-supabase.test.ts +++ b/tests/unit/tools/session-close-supabase.test.ts @@ -158,6 +158,7 @@ vi.mock("../../../src/services/gitmem-dir.js", () => ({ vi.mock("../../../src/services/active-sessions.js", () => ({ unregisterSession: () => {}, findSessionByHostPid: () => null, + findSessionById: () => null, // GIT-86: session-close's registry fallback looks up by id })); vi.mock("../../../src/services/thread-suggestions.js", () => ({ @@ -347,6 +348,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 +363,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 +380,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 +397,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" }, diff --git a/tests/unit/tools/session-close-validation.test.ts b/tests/unit/tools/session-close-validation.test.ts index 98fa88e..00952b7 100644 --- a/tests/unit/tools/session-close-validation.test.ts +++ b/tests/unit/tools/session-close-validation.test.ts @@ -125,6 +125,7 @@ vi.mock("../../../src/services/gitmem-dir.js", () => ({ 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 })); vi.mock("../../../src/services/thread-suggestions.js", () => ({ diff --git a/tests/unit/tools/session-start-cross-project.test.ts b/tests/unit/tools/session-start-cross-project.test.ts new file mode 100644 index 0000000..ce158ee --- /dev/null +++ b/tests/unit/tools/session-start-cross-project.test.ts @@ -0,0 +1,141 @@ +/** + * GIT-86: session_start must never resume a session of another project. + * + * One desktop process serves every chat, so this process's registry entries + * and session directories belong to several conversations at once. Before the + * fix, session_start(Y) found X by hostname+pid, overwrote the caller's + * project with X's (stderr only) and continued the X session in the Y chat. + * + * Runs the real session_start on the free tier against a temp .gitmem root. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +vi.mock("../../../src/services/agent-detection.js", () => ({ + detectAgent: () => ({ agent: "desktop", entrypoint: "claude-desktop", docker: false, hostname: os.hostname() }), +})); + +import { sessionStart } from "../../../src/tools/session-start.js"; +import { setGitmemDir, clearGitmemDirCache } from "../../../src/services/gitmem-dir.js"; +import { resetTier } from "../../../src/services/tier.js"; +import { listActiveSessions, findResumableSessionOnDisk, pruneStale } from "../../../src/services/active-sessions.js"; +import { clearCurrentSession, getCurrentSession, addSurfacedScars, addConfirmations } from "../../../src/services/session-state.js"; + +let tmpDir: string; + +const sessionDirExists = (id: string) => fs.existsSync(path.join(tmpDir, "sessions", id, "session.json")); + +beforeEach(() => { + vi.stubEnv("GITMEM_TIER", "free"); + resetTier(); + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git86-")); + setGitmemDir(tmpDir); + clearCurrentSession(); +}); + +afterEach(() => { + clearCurrentSession(); + clearGitmemDirCache(); + resetTier(); + vi.unstubAllEnvs(); + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +describe("session_start across projects (GIT-86)", () => { + it("X open; session_start(Y) starts a new Y session and X stays resumable", async () => { + const x = await sessionStart({ project: "proj-x" }); + expect(x.project).toBe("proj-x"); + + const y = await sessionStart({ project: "proj-y" }); + expect(y.session_id).not.toBe(x.session_id); + expect(y.project).toBe("proj-y"); + expect(y.resumed).toBeUndefined(); + + // X left open: registry entry, session directory, and a prune pass keep it. + pruneStale(); + expect(sessionDirExists(x.session_id)).toBe(true); + expect(listActiveSessions().map((s) => [s.session_id, s.project]).sort()).toEqual( + [[x.session_id, "proj-x"], [y.session_id, "proj-y"]].sort() + ); + + // ...and resumable by asking for X again. + const xAgain = await sessionStart({ project: "proj-x" }); + expect(xAgain.session_id).toBe(x.session_id); + expect(xAgain.resumed).toBe(true); + expect(xAgain.project).toBe("proj-x"); + + // Y, likewise, was not displaced by resuming X. + const yAgain = await sessionStart({ project: "proj-y" }); + expect(yAgain.session_id).toBe(y.session_id); + expect(yAgain.resumed).toBe(true); + }); + + it("resumes the same project's session as before", async () => { + const first = await sessionStart({ project: "proj-x" }); + const second = await sessionStart({ project: "proj-x" }); + expect(second.session_id).toBe(first.session_id); + expect(second.resumed).toBe(true); + expect(second.project_from_resumed_session).toBeUndefined(); + }); + + it("no project passed: resumes the most recent session and names its project in the display", async () => { + await sessionStart({ project: "proj-x" }); + await new Promise((r) => setTimeout(r, 5)); // distinct started_at + const y = await sessionStart({ project: "proj-y" }); + + const bare = await sessionStart({}); + expect(bare.session_id).toBe(y.session_id); + expect(bare.resumed).toBe(true); + expect(bare.project).toBe("proj-y"); + expect(bare.project_from_resumed_session).toBe(true); + expect(bare.display).toContain("Resumed project: proj-y"); + }); + + it("the disk scan honours the requested project when the registry is lost", async () => { + const x = await sessionStart({ project: "proj-x" }); + fs.writeFileSync(path.join(tmpDir, "active-sessions.json"), JSON.stringify({ sessions: [] })); + + expect(findResumableSessionOnDisk("proj-y")).toBeNull(); + expect(findResumableSessionOnDisk("proj-x")?.session_id).toBe(x.session_id); + + const y = await sessionStart({ project: "proj-y" }); + expect(y.session_id).not.toBe(x.session_id); + expect(y.resumed).toBeUndefined(); + }); + + it("force:true carries activity forward from a same-project session only", async () => { + const scar = { scar_id: "s-1", scar_title: "t", scar_severity: "medium", surfaced_at: new Date().toISOString(), source: "recall" as const }; + + // Different project in memory: nothing carried, the X session stays open. + const conf = (id: string) => ({ scar_id: id, scar_title: "t", decision: "APPLYING" as const, evidence: "e", confirmed_at: new Date().toISOString() }); + const x = await sessionStart({ project: "proj-x" }); + addSurfacedScars([scar]); + addConfirmations([conf("s-1")]); + const xStartedAt = getCurrentSession()!.startedAt; + await new Promise((r) => setTimeout(r, 5)); + const y = await sessionStart({ project: "proj-y", force: true }); + expect(y.session_id).not.toBe(x.session_id); + expect(getCurrentSession()!.surfacedScars).toHaveLength(0); + expect(getCurrentSession()!.confirmations).toHaveLength(0); + expect(getCurrentSession()!.startedAt.getTime()).toBeGreaterThan(xStartedAt.getTime()); + expect(sessionDirExists(x.session_id)).toBe(true); + expect(listActiveSessions().some((s) => s.session_id === x.session_id)).toBe(true); + + // Same project in memory: carried as before (t-f7c2fa01). + addSurfacedScars([{ ...scar, scar_id: "s-2" }]); + addConfirmations([conf("s-2")]); + const yStartedAt = getCurrentSession()!.startedAt; + const y2 = await sessionStart({ project: "proj-y", force: true }); + expect(y2.session_id).not.toBe(y.session_id); + expect(getCurrentSession()!.surfacedScars.map((s) => s.scar_id)).toEqual(["s-2"]); + expect(getCurrentSession()!.confirmations.map((c) => c.scar_id)).toEqual(["s-2"]); + expect(getCurrentSession()!.startedAt.getTime()).toBe(yStartedAt.getTime()); + // The same-project predecessor is displaced; X is still untouched. + const ids = listActiveSessions().map((s) => s.session_id); + expect(ids).toContain(x.session_id); + expect(ids).not.toContain(y.session_id); + }); +}); diff --git a/tests/unit/tools/write-result-contract.test.ts b/tests/unit/tools/write-result-contract.test.ts new file mode 100644 index 0000000..de7455e --- /dev/null +++ b/tests/unit/tools/write-result-contract.test.ts @@ -0,0 +1,137 @@ +/** + * GIT-101: every write tool answers with WriteResult, and none of them calls a + * write the durable store never saw a success. + * + * One parameterized test: Pro tier, Supabase configured, and every request to + * it fails at the network. Each tool runs its real write path and must report + * success: false, durable: false — and a stored_in that names what actually + * survived (local_only when a local copy was kept, null when nothing was). + * + * resolve_thread used to return success: true here unconditionally. + */ +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +const saved = vi.hoisted(() => { + const keys = ["GITMEM_TIER", "SUPABASE_URL", "SUPABASE_SERVICE_ROLE_KEY", "GITMEM_DIR", "OPENAI_API_KEY", "OPENROUTER_API_KEY", "GITMEM_EMBEDDING_PROVIDER"]; + const before = Object.fromEntries(keys.map((k) => [k, process.env[k]])); + const dir = require("fs").mkdtempSync(require("path").join(require("os").tmpdir(), "gitmem-git101-")); + Object.assign(process.env, { + GITMEM_TIER: "pro", + SUPABASE_URL: "https://stubbed-venue.supabase.co", + SUPABASE_SERVICE_ROLE_KEY: "stub-key", + GITMEM_DIR: dir, + }); + delete process.env.OPENAI_API_KEY; + delete process.env.OPENROUTER_API_KEY; + delete process.env.GITMEM_EMBEDDING_PROVIDER; + return { before, dir }; +}); + +import { resetTier, hasSupabase } from "../../../src/services/tier.js"; +import { setCurrentSession, clearCurrentSession } from "../../../src/services/session-state.js"; +import { saveThreadsFile } from "../../../src/services/thread-manager.js"; +import { saveSuggestions } from "../../../src/services/thread-suggestions.js"; +import { createLearning } from "../../../src/tools/create-learning.js"; +import { createDecision } from "../../../src/tools/create-decision.js"; +import { createThread } from "../../../src/tools/create-thread.js"; +import { resolveThread } from "../../../src/tools/resolve-thread.js"; +import { archiveLearning } from "../../../src/tools/archive-learning.js"; +import { recordScarUsage } from "../../../src/tools/record-scar-usage.js"; +import { recordScarUsageBatch } from "../../../src/tools/record-scar-usage-batch.js"; +import { sessionClose } from "../../../src/tools/session-close.js"; +import { promoteSuggestion } from "../../../src/tools/promote-suggestion.js"; +import { saveTranscript } from "../../../src/tools/save-transcript.js"; + +const SID = "393adb34-a80c-4c3a-b71a-bc0053b7a7ea"; +const SCAR = "11111111-2222-4333-8444-555555555555"; +const now = () => new Date().toISOString(); +let supabaseCalls = 0; + +type Case = { + tool: string; + run: () => Promise<{ success: boolean; durable: boolean; stored_in: unknown; display?: string }>; + /** What a failed durable write leaves behind. */ + storedIn: "local_only" | null; + setup?: () => void; +}; + +const CASES: Case[] = [ + { tool: "create_learning", storedIn: null, + run: () => createLearning({ learning_type: "win", title: "t", description: "d" } as never) }, + { tool: "create_decision", storedIn: null, + run: () => createDecision({ title: "t", decision: "d", rationale: "r" }) }, + { tool: "create_thread", storedIn: "local_only", + run: () => createThread({ text: "GIT-101 contract thread", allow_duplicate: true }) }, + { tool: "resolve_thread", storedIn: "local_only", + setup: () => saveThreadsFile([{ id: "t-git10101", text: "resolve me", status: "open", created_at: now() } as never]), + run: () => resolveThread({ thread_id: "t-git10101" }) }, + { tool: "archive_learning", storedIn: null, + run: () => archiveLearning({ id: SCAR }) }, + { tool: "record_scar_usage", storedIn: null, + run: () => recordScarUsage({ scar_id: SCAR, surfaced_at: now(), reference_type: "explicit", reference_context: "c" }) }, + { tool: "record_scar_usage_batch", storedIn: null, + run: () => recordScarUsageBatch({ scars: [{ scar_identifier: SCAR, surfaced_at: now(), reference_type: "explicit", reference_context: "c" }] } as never) }, + { tool: "session_close", storedIn: null, + run: () => sessionClose({ session_id: SID, close_type: "standard", human_corrections: "none", + closing_reflection: { what_broke: "x", what_took_longer: "x", do_differently: "x", what_worked: "x", + wrong_assumption: "x", scars_applied: [], institutional_memory_items: "x", collaborative_dynamic: "x", rapport_notes: "x" } } as never) }, + { tool: "promote_suggestion", storedIn: "local_only", + setup: () => saveSuggestions([{ id: "ts-git10101", text: "promote me", embedding: null, evidence_sessions: ["a", "b", "c"], + similarity_score: 0.9, status: "pending", created_at: now() } as never]), + run: () => promoteSuggestion({ suggestion_id: "ts-git10101" }) }, + { tool: "save_transcript", storedIn: null, + run: () => saveTranscript({ session_id: SID, transcript: "hello" }) }, +]; + +beforeAll(() => { + resetTier(); + vi.stubGlobal("fetch", vi.fn(async (input: unknown) => { + const url = String(input instanceof Request ? input.url : input); + if (url.includes("stubbed-venue.supabase.co")) supabaseCalls++; + throw new TypeError("fetch failed (GIT-101 stub: store unreachable)"); + })); +}); + +afterAll(() => { + vi.unstubAllGlobals(); + clearCurrentSession(); + for (const [k, v] of Object.entries(saved.before)) { + if (v === undefined) delete process.env[k]; else process.env[k] = v; + } + resetTier(); + fs.rmSync(saved.dir, { recursive: true, force: true }); +}); + +beforeEach(() => { + supabaseCalls = 0; + setCurrentSession({ sessionId: SID, agent: "cli", project: "default", startedAt: new Date() }); +}); + +describe("WriteResult contract with Supabase failing (GIT-101)", () => { + it("runs on the pro tier", () => { + expect(hasSupabase()).toBe(true); + }); + + it.each(CASES)("$tool: success false, durable false, stored_in $storedIn", async ({ run, setup, storedIn }) => { + setup?.(); + const result = await run(); + + expect(supabaseCalls).toBeGreaterThan(0); // the durable store was actually tried + expect(result.success).toBe(false); + expect(result.durable).toBe(false); + expect(result.stored_in).toBe(storedIn); + // Over MCP a client sees only `display` when a tool sets one (server.ts), + // so the display must carry the same verdict as the fields. + const display = (result as { display?: string }).display; + if (display !== undefined) expect(display).toMatch(/fail|not stored|not durable|locally only/i); + }); + + it("resolve_thread says so in its display, not just its fields", async () => { + saveThreadsFile([{ id: "t-git10102", text: "resolve me too", status: "open", created_at: now() } as never]); + const result = await resolveThread({ thread_id: "t-git10102" }); + expect(result.display).toContain("RESOLVED LOCALLY ONLY — not durable"); + }); +});