diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a92f871..af30f7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,6 +51,17 @@ jobs: - name: E2E — session identity survives MCP restart run: npx vitest run --config vitest.e2e.config.ts tests/e2e/git-89-session-identity.test.ts + # GIT-120: install-hooks / uninstall-hooks keep foreign hooks. Runs the + # real CLI in a scratch repo; no server, Docker or credentials. + - name: E2E — hooks install/uninstall keep foreign hooks + run: npx vitest run --config vitest.e2e.config.ts tests/e2e/git-120-hooks-merge.test.ts + + # GIT-115: a fresh install in a clean HOME — init, the SessionStart hook, + # the built server — plus the CLI/wizard suite. Both run the real CLI in + # scratch directories with a scratch HOME; no Docker or credentials. + - name: E2E — fresh install reads what init wrote; CLI and wizard + run: npx vitest run --config vitest.e2e.config.ts tests/e2e/git-115-fresh-install.test.ts tests/e2e/cli-fresh-install.test.ts + - 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)}" @@ -73,6 +84,31 @@ jobs: - name: Hook tests run: bash hooks/tests/test-hooks.sh + # GIT-116: the same suite on stock macOS. The runner has Homebrew (and may + # have coreutils' timeout), so the suite runs with the system directories + # plus a directory holding only node — what a Mac without Homebrew has. + hooks-macos: + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 20 + + - name: Hook tests on a stock macOS PATH (no Homebrew, no timeout) + run: | + mkdir -p "$RUNNER_TEMP/nodeonly" + ln -s "$(command -v node)" "$RUNNER_TEMP/nodeonly/node" + STOCK="/usr/bin:/bin:/usr/sbin:/sbin:$RUNNER_TEMP/nodeonly" + for t in timeout gtimeout; do + if PATH="$STOCK" command -v "$t" >/dev/null; then echo "::error::$t is on the stock PATH"; exit 1; fi + done + # macOS 15+ ships /usr/bin/jq; older macOS does not. The jq-less path + # is covered by hooks-without-jq (Linux); report which this runner is. + PATH="$STOCK" command -v jq >/dev/null && echo "stock jq present: $(PATH="$STOCK" jq --version)" || echo "stock jq absent" + env -i HOME="$HOME" PATH="$STOCK" /bin/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 @@ -164,7 +200,7 @@ jobs: retention-days: 14 publish: - needs: [build, hooks-without-jq, venue-gate] + needs: [build, hooks-without-jq, hooks-macos, venue-gate] runs-on: ubuntu-latest if: startsWith(github.ref, 'refs/tags/v') permissions: diff --git a/CHANGELOG.md b/CHANGELOG.md index 96a05f1..3f102de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,54 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.11.1] - 2026-09-22 + +**Just upgrade the package. Nothing to change on your database.** Every fix below works on a project +still on the `setup.sql` you first ran, back to 1.8.0, and was checked against a project on exactly +that schema and on the current one. + +**Recall works when it has to ask your database.** When the local index was not ready, or with +`GITMEM_SEARCH_MODE=remote`, recall called a database function that `setup.sql` never created. It +got an error and returned no scars. It now calls `gitmem_scar_search`, which every project has, and +falls back to the older function only where that exists. (GIT-114) + +**A fresh install now finds its starter lessons.** `gitmem init` put your memory in the repo's +`.gitmem/`, but gitmem reads `~/.gitmem`, so a new install started with none of its starter lessons +and was told its memory was "NOT being read". `init` now puts your memory where gitmem reads it +(`GITMEM_DIR`, else `GITMEM_HOME/.gitmem`, else `~/.gitmem`). The repo's `.gitmem/` keeps only +`config.json`, with the project name, and the hook scripts. A repo set up that way no longer +triggers the warning. `gitmem uninstall` removes the repo's files and keeps your memory, which every +project shares, unless you pass `--all`. The session-start hook now finds the project name without +jq. (GIT-115) + +If an older `init` put lessons of your own in a repo's `.gitmem/`, session_start still points them +out. `npx gitmem-mcp migrate-root` copies them into `~/.gitmem`. + +**`install-hooks` and `uninstall-hooks` keep your own hooks.** They used to replace or delete every +hook in `.claude/settings.json` or `.cursor/hooks.json`. Now they add or remove only gitmem's hooks, +back the file up first, and print what changed. A file that is not valid JSON is left alone instead +of being replaced. `gitmem init` and `gitmem uninstall` use the same matching, so a hook of yours +that shares a matcher with gitmem's, or lives in a folder named gitmem, is no longer removed or +mistaken for gitmem's. (GIT-120, GIT-115) + +### Fixed + +- **Hooks work on a stock Mac.** The prompt hook ran `timeout`, which macOS does not have, so it + never returned scars there. The time limit now lives inside gitmem. When gitmem or node cannot be + found, the hook says so instead of staying silent. (GIT-116) +- **A thread that failed to sync is no longer marked synced.** `session_close` reports PARTIAL and + names the thread ids that did not reach your database. Those threads stay in `threads.json` and + the next session tries again. (GIT-117) +- **A failed index reload no longer empties recall.** If reloading scars from your database fails, + gitmem keeps the index it had, falls back to the on-disk cache, and retries in the background. + `create_learning` says when the new learning is saved but not yet searchable. (GIT-118) +- **`archive_learning` accepts a short id.** Archiving by id prefix failed on every project with + "operator does not exist". Archiving an id that does not exist used to report success; it now says + nothing was archived. (GIT-119) +- **Updates that change no row are reported as not saved.** This covers thread resolve, touch and + auto-archive, session embeddings, superseded sessions, relevance data and transcript paths. Each + one used to count as success. (GIT-119) + ## [1.11.0] - 2026-09-21 **Just upgrade the package. Nothing to change on your database.** Every fix in this release works on diff --git a/bin/gitmem-root.js b/bin/gitmem-root.js new file mode 100644 index 0000000..5ddbb61 --- /dev/null +++ b/bin/gitmem-root.js @@ -0,0 +1,34 @@ +/** + * GIT-115: where the CLI puts things, split the same way the server reads them. + * + * The store (learnings, threads, sessions, decisions, the closing payload and + * its template) belongs in the root the server reads. That is resolved exactly + * as src/services/gitmem-dir.ts does, without a running session: GITMEM_DIR, + * then GITMEM_HOME/.gitmem, then ~/.gitmem. `init` used to seed /.gitmem, + * which the server has not read since GIT-91, so a fresh install saw no + * starter lessons and was told its store was "NOT being read". + * + * The repo keeps its own .gitmem/ for two things only: config.json with the + * project name (the SessionStart hook reads it from the repo) and the hook + * scripts (committed settings refer to them by relative path). + */ + +import { join } from "path"; +import { homedir } from "os"; + +/** The root the server reads. */ +export function storeRoot() { + if (process.env.GITMEM_DIR) return process.env.GITMEM_DIR; + return join(process.env.GITMEM_HOME || homedir(), ".gitmem"); +} + +/** The repo's .gitmem/: config.json and hooks/ only. */ +export function repoGitmemDir(cwd = process.cwd()) { + return join(cwd, ".gitmem"); +} + +/** A path shortened to ~ for display. */ +export function displayPath(p) { + const home = homedir(); + return p === home || p.startsWith(home + "/") ? "~" + p.slice(home.length) : p; +} diff --git a/bin/gitmem.js b/bin/gitmem.js index e2294bc..2d039ec 100755 --- a/bin/gitmem.js +++ b/bin/gitmem.js @@ -28,6 +28,8 @@ import { import { dirname, join } from "path"; import { fileURLToPath } from "url"; import { homedir } from "os"; +import { hasGitmemHooks, hooksShapeError, mergeGitmemHooks, removeGitmemHooks, backupFile } from "./hooks-merge.js"; +import { storeRoot, displayPath } from "./gitmem-root.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); const command = process.argv[2]; @@ -42,7 +44,7 @@ Usage: npx gitmem-mcp init --dry-run Show what would be configured npx gitmem-mcp init --client cursor Set up for Cursor IDE npx gitmem-mcp uninstall Clean removal of gitmem from project - npx gitmem-mcp uninstall --all Also delete .gitmem/ data directory + npx gitmem-mcp uninstall --all Also delete the memory store (~/.gitmem, shared by all projects) Pro Tier Activation: npx gitmem-mcp activate Activate Pro tier (license key + credentials wizard) @@ -115,25 +117,25 @@ async function cmdInit() { } if (!supabaseUrl || !supabaseKey) { - // Free tier: copy starter scars to local .gitmem/ directory + // Free tier: copy starter scars into the store the server reads (GIT-115). console.log("No Supabase credentials found — initializing free tier (local storage)."); console.log(""); - const gitmemDir = join(process.cwd(), ".gitmem"); + const gitmemDir = storeRoot(); + const storeName = displayPath(gitmemDir); if (!existsSync(gitmemDir)) { mkdirSync(gitmemDir, { recursive: true }); } - // Write config.json (with project if specified via --project) - const configPath = join(gitmemDir, "config.json"); + // The project name stays in the repo's .gitmem/config.json, where the + // SessionStart hook reads it. + const repoDir = join(process.cwd(), ".gitmem"); + const configPath = join(repoDir, "config.json"); if (!existsSync(configPath)) { - const config = {}; - if (projectArg) config.project = projectArg; - writeFileSync(configPath, JSON.stringify(config, null, 2)); if (projectArg) { + mkdirSync(repoDir, { recursive: true }); + writeFileSync(configPath, JSON.stringify({ project: projectArg }, null, 2)); console.log(` + Created .gitmem/config.json (project: "${projectArg}")`); - } else { - console.log(" + Created .gitmem/config.json"); } } else if (projectArg) { // Config exists — update project field @@ -205,7 +207,7 @@ async function cmdInit() { } console.log(""); - console.log(`Done: ${added} new scars added to .gitmem/learnings.json`); + console.log(`Done: ${added} new scars added to ${storeName}/learnings.json`); console.log(""); console.log("Add .gitmem/ to your .gitignore:"); console.log(" echo '.gitmem/' >> .gitignore"); @@ -479,6 +481,39 @@ async function cmdSessionRefresh() { } } +/** + * GIT-120: read a hooks-bearing JSON file for an in-place edit. A file that + * exists but cannot be parsed, or whose hooks have an unexpected shape, is + * never rewritten — that used to replace the user's file with a fresh one. + */ +function readHooksFile(path, name) { + if (!existsSync(path)) return {}; + let cfg; + try { + cfg = JSON.parse(readFileSync(path, "utf-8")); + } catch (err) { + console.error(`Error: ${name} is not valid JSON (${err.message}). Nothing was changed.`); + console.error(`Fix or move ${name}, then run this again.`); + process.exit(1); + } + const shapeError = cfg && typeof cfg === "object" && !Array.isArray(cfg) ? hooksShapeError(cfg.hooks) : "file is not a JSON object"; + if (shapeError) { + console.error(`Error: ${name}: ${shapeError}. Nothing was changed.`); + process.exit(1); + } + return cfg; +} + +/** Back up, write, and print what changed. */ +function writeHooksFile(path, name, cfg, changes) { + const backup = backupFile(path); + writeFileSync(path, JSON.stringify(cfg, null, 2)); + console.log(`Changes to ${name}:`); + for (const line of changes) console.log(` ${line}`); + console.log(backup ? `Backup of the previous file: ${backup}` : "(new file, nothing to back up)"); + console.log(""); +} + /** * Install gitmem hooks as project-level hooks. * @@ -560,30 +595,21 @@ function cmdInstallHooks() { stop: [{ command: `bash ${relScripts}/session-close-check.sh`, timeout: 5000 }], }; - let config = {}; - if (existsSync(hooksPath)) { - try { - config = JSON.parse(readFileSync(hooksPath, "utf-8")); - } catch { - console.warn(" Warning: Could not parse existing .cursor/hooks.json, creating fresh"); - } - } else { - mkdirSync(cursorDir, { recursive: true }); - } + const config = readHooksFile(hooksPath, ".cursor/hooks.json"); + if (!existsSync(cursorDir)) mkdirSync(cursorDir, { recursive: true }); - if (config.hooks && !force) { - const hasGitmem = JSON.stringify(config.hooks).includes("gitmem"); - if (hasGitmem) { - console.log("GitMem hooks already installed in .cursor/hooks.json"); - console.log(""); - console.log("To reinstall (overwrite), run:"); - console.log(" npx gitmem-mcp install-hooks --client cursor --force"); - return; - } + if (!force && hasGitmemHooks(config.hooks)) { + console.log("GitMem hooks already installed in .cursor/hooks.json"); + console.log(""); + console.log("To reinstall (replaces only gitmem's hooks), run:"); + console.log(" npx gitmem-mcp install-hooks --client cursor --force"); + return; } - config.hooks = gitmemHooks; - writeFileSync(hooksPath, JSON.stringify(config, null, 2)); + // GIT-120: merge per event; other hooks stay as they are. + const merged = mergeGitmemHooks(config.hooks, gitmemHooks); + config.hooks = merged.hooks; + writeHooksFile(hooksPath, ".cursor/hooks.json", config, merged.changes); console.log("Hooks written to .cursor/hooks.json"); console.log(`Scripts at: ${relScripts}/`); @@ -634,30 +660,21 @@ function cmdInstallHooks() { ], }; - let settings = {}; - if (existsSync(settingsPath)) { - try { - settings = JSON.parse(readFileSync(settingsPath, "utf-8")); - } catch { - console.warn(" Warning: Could not parse existing .claude/settings.json, creating fresh"); - } - } else { - mkdirSync(claudeDir, { recursive: true }); - } + const settings = readHooksFile(settingsPath, ".claude/settings.json"); + if (!existsSync(claudeDir)) mkdirSync(claudeDir, { recursive: true }); - if (settings.hooks && !force) { - const hasGitmem = JSON.stringify(settings.hooks).includes("gitmem"); - if (hasGitmem) { - console.log("GitMem hooks already installed in .claude/settings.json"); - console.log(""); - console.log("To reinstall (overwrite), run:"); - console.log(" npx gitmem-mcp install-hooks --force"); - return; - } + if (!force && hasGitmemHooks(settings.hooks)) { + console.log("GitMem hooks already installed in .claude/settings.json"); + console.log(""); + console.log("To reinstall (replaces only gitmem's hooks), run:"); + console.log(" npx gitmem-mcp install-hooks --force"); + return; } - settings.hooks = gitmemHooks; - writeFileSync(settingsPath, JSON.stringify(settings, null, 2)); + // GIT-120: merge per event; other hooks stay as they are. + const merged = mergeGitmemHooks(settings.hooks, gitmemHooks); + settings.hooks = merged.hooks; + writeHooksFile(settingsPath, ".claude/settings.json", settings, merged.changes); console.log("Hooks written to .claude/settings.json"); console.log(`Scripts at: ${relScripts}/`); @@ -732,6 +749,25 @@ function promptTelemetryOptIn() { * Also cleans up legacy plugin directories and temp state. * Use --client to target a specific IDE. */ +/** + * GIT-120: take out only the commands gitmem installed. Other hooks, and the + * hooks key itself while anything is left in it, stay. + */ +function uninstallFrom(path, name, cfg, extraChanges = []) { + if (!cfg.hooks && extraChanges.length === 0) { + console.log(`[uninstall] No hooks found in ${name}`); + return; + } + const result = removeGitmemHooks(cfg.hooks); + if (result.removed === 0 && extraChanges.length === 0) { + console.log(`[uninstall] No gitmem hooks found in ${name}${result.kept ? ` (${result.kept} other hook(s) left untouched)` : ""}`); + return; + } + if (result.hooks) cfg.hooks = result.hooks; else delete cfg.hooks; + writeHooksFile(path, name, cfg, [...result.changes, ...extraChanges]); + console.log(`[uninstall] Removed ${result.removed} gitmem hook(s) from ${name}; kept ${result.kept} other`); +} + function cmdUninstallHooks() { const clientIdx = process.argv.indexOf("--client"); const clientArg = clientIdx !== -1 ? process.argv[clientIdx + 1]?.toLowerCase() : null; @@ -759,40 +795,8 @@ function cmdUninstallHooks() { // Remove hooks from .cursor/hooks.json const hooksPath = join(process.cwd(), ".cursor", "hooks.json"); if (existsSync(hooksPath)) { - try { - const cfg = JSON.parse(readFileSync(hooksPath, "utf-8")); - if (cfg.hooks) { - // Filter out gitmem hooks, preserve others - const cleaned = {}; - let removed = 0; - for (const [eventType, entries] of Object.entries(cfg.hooks)) { - if (!Array.isArray(entries)) continue; - const nonGitmem = entries.filter((e) => { - if (typeof e.command === "string" && e.command.includes("gitmem")) { - removed++; - return false; - } - return true; - }); - if (nonGitmem.length > 0) cleaned[eventType] = nonGitmem; - } - if (removed > 0) { - if (Object.keys(cleaned).length > 0) { - cfg.hooks = cleaned; - } else { - delete cfg.hooks; - } - writeFileSync(hooksPath, JSON.stringify(cfg, null, 2)); - console.log(`[uninstall] Removed ${removed} gitmem hooks from .cursor/hooks.json`); - } else { - console.log("[uninstall] No gitmem hooks found in .cursor/hooks.json"); - } - } else { - console.log("[uninstall] No hooks found in .cursor/hooks.json"); - } - } catch { - // ignore parse errors - } + const cfg = readHooksFile(hooksPath, ".cursor/hooks.json"); + uninstallFrom(hooksPath, ".cursor/hooks.json", cfg); } else { console.log("[uninstall] No .cursor/hooks.json found"); } @@ -800,28 +804,12 @@ function cmdUninstallHooks() { // Remove hooks from .claude/settings.json const settingsPath = join(process.cwd(), ".claude", "settings.json"); if (existsSync(settingsPath)) { - try { - const cfg = JSON.parse(readFileSync(settingsPath, "utf-8")); - if (cfg.hooks) { - delete cfg.hooks; - writeFileSync(settingsPath, JSON.stringify(cfg, null, 2)); - console.log("[uninstall] Removed hooks from .claude/settings.json"); - } else { - console.log("[uninstall] No hooks found in .claude/settings.json"); - } - // Also clean legacy enabledPlugins - if (cfg.enabledPlugins) { - for (const key of Object.keys(cfg.enabledPlugins)) { - if (key.startsWith("gitmem-hooks")) { - delete cfg.enabledPlugins[key]; - writeFileSync(settingsPath, JSON.stringify(cfg, null, 2)); - console.log("[cleanup] Removed legacy enabledPlugins entry"); - } - } - } - } catch { - // ignore parse errors - } + const cfg = readHooksFile(settingsPath, ".claude/settings.json"); + // Legacy enabledPlugins entries from the old plugin install. + const legacyPlugins = Object.keys(cfg.enabledPlugins || {}).filter((k) => k.startsWith("gitmem-hooks")); + for (const key of legacyPlugins) delete cfg.enabledPlugins[key]; + uninstallFrom(settingsPath, ".claude/settings.json", cfg, + legacyPlugins.map((k) => `enabledPlugins: removed legacy entry ${k}`)); } } diff --git a/bin/hooks-merge.js b/bin/hooks-merge.js new file mode 100644 index 0000000..9a8b11c --- /dev/null +++ b/bin/hooks-merge.js @@ -0,0 +1,130 @@ +/** + * GIT-120: merge gitmem's hooks into a settings file without touching anyone + * else's. + * + * install-hooks used to replace `settings.hooks` wholesale and uninstall-hooks + * deleted the key, so every hook the user or another tool had configured was + * lost. These helpers work per event and per command: only commands gitmem + * wrote are replaced or removed, and the file is backed up before any write. + * + * Two shapes are supported: + * Claude Code hooks[event] = [{ matcher?, hooks: [{ type, command, ... }] }] + * Cursor hooks[event] = [{ command, ... }] + */ + +import { copyFileSync, existsSync } from "fs"; + +/** + * A command gitmem installed: the per-repo copy (.gitmem/hooks/), the older + * node_modules path, or the legacy plugin. A bare "gitmem" substring is not + * enough — a user's own script can live in a directory called gitmem. + */ +export function isGitmemCommand(command) { + if (typeof command !== "string") return false; + return /(^|[\s"'=/])\.gitmem\/hooks\/[\w.-]+\.sh\b/.test(command) + || /gitmem-mcp\/hooks\/scripts\/[\w.-]+\.sh\b/.test(command) + || /\bgitmem-hooks\b/.test(command); +} + +const isGroup = (entry) => entry && typeof entry === "object" && Array.isArray(entry.hooks); + +/** Commands in one event's entries, as [gitmem, other] counts. */ +function countEntries(entries) { + let gitmem = 0; + let other = 0; + for (const entry of Array.isArray(entries) ? entries : []) { + const commands = isGroup(entry) ? entry.hooks : [entry]; + for (const h of commands) { + if (isGitmemCommand(h?.command)) gitmem++; else other++; + } + } + return { gitmem, other }; +} + +/** + * One event's entries with gitmem's commands taken out. A Claude group keeps + * its other commands; a group left empty is dropped. + */ +function withoutGitmem(entries) { + const kept = []; + for (const entry of Array.isArray(entries) ? entries : []) { + if (isGroup(entry)) { + const others = entry.hooks.filter((h) => !isGitmemCommand(h?.command)); + if (others.length === entry.hooks.length) kept.push(entry); + else if (others.length > 0) kept.push({ ...entry, hooks: others }); + } else if (!isGitmemCommand(entry?.command)) { + kept.push(entry); + } + } + return kept; +} + +/** The hooks object must be a plain object of arrays, or absent. */ +export function hooksShapeError(hooks) { + if (hooks === undefined) return null; + if (!hooks || typeof hooks !== "object" || Array.isArray(hooks)) return "hooks is not an object"; + for (const [event, entries] of Object.entries(hooks)) { + if (!Array.isArray(entries)) return `hooks.${event} is not an array`; + } + return null; +} + +/** True when any command in the hooks is one gitmem installed. */ +export function hasGitmemHooks(hooks) { + return Object.values(hooks || {}).some((entries) => countEntries(entries).gitmem > 0); +} + +/** + * Merge gitmem's hooks in: per event, drop the gitmem commands already there, + * keep everything else, append gitmem's current entries. + * Returns the new hooks and one change line per event touched. + */ +export function mergeGitmemHooks(existing, gitmemHooks) { + const hooks = { ...(existing || {}) }; + const changes = []; + for (const [event, entries] of Object.entries(gitmemHooks)) { + const before = countEntries(hooks[event]); + const added = countEntries(entries).gitmem; + hooks[event] = [...withoutGitmem(hooks[event]), ...entries]; + changes.push(`${event}: ${before.gitmem ? `replaced ${before.gitmem} gitmem hook(s) with ${added}` : `added ${added} gitmem hook(s)`}${before.other ? `, kept ${before.other} other` : ""}`); + } + // Events this installer does not manage are left exactly as they are. That + // includes gitmem's own UserPromptSubmit hook, which `gitmem init` installs + // and install-hooks does not. + for (const [event, entries] of Object.entries(hooks)) { + if (event in gitmemHooks) continue; + const c = countEntries(entries); + if (c.gitmem + c.other) changes.push(`${event}: kept ${c.gitmem + c.other} (untouched)`); + } + return { hooks, changes }; +} + +/** + * Take gitmem's commands out of every event. Events left empty are dropped; + * hooks is undefined when nothing remains. + */ +export function removeGitmemHooks(existing) { + const hooks = {}; + const changes = []; + let removed = 0; + let kept = 0; + for (const [event, entries] of Object.entries(existing || {})) { + const c = countEntries(entries); + removed += c.gitmem; + kept += c.other; + const rest = withoutGitmem(entries); + if (rest.length) hooks[event] = rest; + if (c.gitmem) changes.push(`${event}: removed ${c.gitmem} gitmem hook(s)${c.other ? `, kept ${c.other} other` : ""}`); + else if (c.other) changes.push(`${event}: kept ${c.other} other (untouched)`); + } + return { hooks: Object.keys(hooks).length ? hooks : undefined, removed, kept, changes }; +} + +/** Copy a file aside before it is rewritten; returns the backup path or null. */ +export function backupFile(path) { + if (!existsSync(path)) return null; + const stamp = new Date().toISOString().replace(/[:.]/g, "-"); + const backup = `${path}.gitmem-backup-${stamp}`; + copyFileSync(path, backup); + return backup; +} diff --git a/bin/init-wizard.js b/bin/init-wizard.js index 261d4f1..e54f115 100644 --- a/bin/init-wizard.js +++ b/bin/init-wizard.js @@ -20,6 +20,8 @@ import { import { dirname, join } from "path"; import { fileURLToPath } from "url"; import { createInterface } from "readline/promises"; +import { storeRoot, repoGitmemDir, displayPath } from "./gitmem-root.js"; +import { hasGitmemHooks, mergeGitmemHooks, removeGitmemHooks, backupFile } from "./hooks-merge.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); const cwd = process.cwd(); @@ -166,7 +168,11 @@ const CLIENT_CONFIGS = { }; // Shared paths -const gitmemDir = join(cwd, ".gitmem"); +// GIT-115: the store goes where the server reads it; the repo keeps only +// config.json (project name, read by the SessionStart hook) and the hooks. +const repoDir = repoGitmemDir(cwd); +const storeDir = storeRoot(); +const storeName = displayPath(storeDir); const gitignorePath = join(cwd, ".gitignore"); const starterScarsPath = join(__dirname, "..", "schema", "starter-scars.json"); const hooksScriptsDir = join(__dirname, "..", "hooks", "scripts"); @@ -351,14 +357,9 @@ function buildCursorHooks() { }; } -function isGitmemHook(entry) { - if (entry.hooks && Array.isArray(entry.hooks)) { - return entry.hooks.some((h) => typeof h.command === "string" && h.command.includes("gitmem")); - } - if (typeof entry.command === "string") { - return entry.command.includes("gitmem"); - } - return false; +/** Commands in a hooks object that are not gitmem's (GIT-120 matching). */ +function countOtherHooks(hooks) { + return removeGitmemHooks(hooks).kept; } function getInstructionsTemplate() { @@ -370,7 +371,7 @@ function getInstructionsTemplate() { } function copyHookScripts() { - const destHooksDir = join(gitmemDir, "hooks"); + const destHooksDir = join(repoDir, "hooks"); if (!existsSync(destHooksDir)) { mkdirSync(destHooksDir, { recursive: true }); } @@ -393,8 +394,22 @@ function copyHookScripts() { // ── Steps ── // Each returns { done: bool } so main can track progress +/** + * GIT-115: the repo's .gitmem/config.json carries the project name, which the + * SessionStart hook reads from the repo. It is not the store's config.json. + */ +function writeRepoConfig() { + if (dryRun) return; + const configPath = join(repoDir, "config.json"); + const config = readJson(configPath) || {}; + if (projectName) config.project = projectName; + else if (existsSync(configPath)) return; + mkdirSync(repoDir, { recursive: true }); + writeJson(configPath, config); +} + async function stepMemoryStore() { - const learningsPath = join(gitmemDir, "learnings.json"); + const learningsPath = join(storeDir, "learnings.json"); const exists = existsSync(learningsPath); let existingCount = 0; if (exists) { @@ -412,15 +427,16 @@ async function stepMemoryStore() { } if (exists && existingCount >= starterScars.length) { - log(CHECK, `Memory store already set up ${C.dim}(${existingCount} lessons in .gitmem/)${C.reset}`); + writeRepoConfig(); + log(CHECK, `Memory store already set up ${C.dim}(${existingCount} lessons in ${storeName})${C.reset}`); return { done: false }; } // Needs merge — prompt if existing data OR interactive mode if (exists || interactive) { const prompt = exists - ? `Merge ${starterScars.length} lessons into .gitmem/? (${existingCount} existing)` - : `Create .gitmem/ with ${starterScars.length} starter lessons?`; + ? `Merge ${starterScars.length} lessons into ${storeName}? (${existingCount} existing)` + : `Create ${storeName} with ${starterScars.length} starter lessons?`; if (!(await confirm(prompt))) { log(SKIP, "Memory store skipped"); return { done: false }; @@ -428,28 +444,18 @@ async function stepMemoryStore() { } if (dryRun) { - log(CHECK, `Would create .gitmem/ with ${starterScars.length} starter lessons`, "[dry-run]"); + log(CHECK, `Would create ${storeName} with ${starterScars.length} starter lessons`, "[dry-run]"); return { done: true }; } - if (!existsSync(gitmemDir)) { - mkdirSync(gitmemDir, { recursive: true }); + if (!existsSync(storeDir)) { + mkdirSync(storeDir, { recursive: true }); } - // Config - const configPath = join(gitmemDir, "config.json"); - if (!existsSync(configPath)) { - const config = { feedback_enabled: false, telemetry_enabled: false }; - if (projectName) config.project = projectName; - writeJson(configPath, config); - } else if (projectName) { - const config = readJson(configPath) || {}; - config.project = projectName; - writeJson(configPath, config); - } + writeRepoConfig(); // Closing payload template (prevents permission prompt on first session close) - const payloadPath = join(gitmemDir, "closing-payload.json"); + const payloadPath = join(storeDir, "closing-payload.json"); if (!existsSync(payloadPath)) { writeJson(payloadPath, { closing_reflection: { @@ -495,7 +501,7 @@ async function stepMemoryStore() { writeJson(learningsPath, existing); // Starter thread — nudges user to add their own project-specific scar - const threadsPath = join(gitmemDir, "threads.json"); + const threadsPath = join(storeDir, "threads.json"); if (!existsSync(threadsPath)) { writeJson(threadsPath, [ { @@ -509,14 +515,14 @@ async function stepMemoryStore() { // Empty collection files for (const file of ["sessions.json", "decisions.json", "scar-usage.json"]) { - const filePath = join(gitmemDir, file); + const filePath = join(storeDir, file); if (!existsSync(filePath)) { writeFileSync(filePath, "[]"); } } // Closing payload template - const templatePath = join(gitmemDir, "closing-payload-template.json"); + const templatePath = join(storeDir, "closing-payload-template.json"); if (!existsSync(templatePath)) { writeJson(templatePath, { closing_reflection: { @@ -541,7 +547,7 @@ async function stepMemoryStore() { : ""; log(CHECK, - "Created .gitmem/ \u2014 your local memory store", + `Created ${storeName} \u2014 your local memory store`, `${starterScars.length} lessons from common mistakes included${mergeNote}` ); return { done: true }; @@ -722,19 +728,14 @@ async function stepHooks() { async function stepHooksClaude() { const existing = readJson(cc.settingsFile); const hooks = existing?.hooks || {}; - const hasGitmem = JSON.stringify(hooks).includes("gitmem"); + const hasGitmem = hasGitmemHooks(hooks); if (hasGitmem) { log(CHECK, `Automatic memory hooks already configured`); return { done: false }; } - let existingHookCount = 0; - for (const entries of Object.values(hooks)) { - if (Array.isArray(entries)) { - existingHookCount += entries.filter((e) => !isGitmemHook(e)).length; - } - } + const existingHookCount = countOtherHooks(hooks); // Existing hooks — prompt for merge if (existingHookCount > 0 || interactive) { @@ -759,16 +760,9 @@ async function stepHooksClaude() { mkdirSync(cc.configDir, { recursive: true }); } - const gitmemHooks = buildClaudeHooks(); - const merged = { ...(settings.hooks || {}) }; - - for (const [eventType, gitmemEntries] of Object.entries(gitmemHooks)) { - const existingEntries = merged[eventType] || []; - const nonGitmem = existingEntries.filter((e) => !isGitmemHook(e)); - merged[eventType] = [...nonGitmem, ...gitmemEntries]; - } - - settings.hooks = merged; + // GIT-120 merge: per event and per command; other hooks untouched. + settings.hooks = mergeGitmemHooks(settings.hooks, buildClaudeHooks()).hooks; + backupFile(cc.settingsFile); writeJson(cc.settingsFile, settings); const preserveNote = existingHookCount > 0 @@ -795,21 +789,14 @@ async function stepHooksCursor() { const hooksName = cc.hooksFileName; const existing = readJson(hooksPath); - const hasGitmem = existing ? JSON.stringify(existing).includes("gitmem") : false; + const hasGitmem = hasGitmemHooks(existing?.hooks); if (hasGitmem) { log(CHECK, `Automatic memory hooks already configured ${C.dim}(${hooksName})${C.reset}`); return { done: false }; } - let existingHookCount = 0; - if (existing?.hooks) { - for (const entries of Object.values(existing.hooks)) { - if (Array.isArray(entries)) { - existingHookCount += entries.filter((e) => !isGitmemHook(e)).length; - } - } - } + const existingHookCount = countOtherHooks(existing?.hooks); if (existingHookCount > 0 || interactive) { const prompt = existingHookCount > 0 @@ -832,17 +819,9 @@ async function stepHooksCursor() { mkdirSync(cc.configDir, { recursive: true }); } - const gitmemHooks = buildCursorHooks(); const config = existing || {}; - const merged = { ...(config.hooks || {}) }; - - for (const [eventType, gitmemEntries] of Object.entries(gitmemHooks)) { - const existingEntries = merged[eventType] || []; - const nonGitmem = existingEntries.filter((e) => !isGitmemHook(e)); - merged[eventType] = [...nonGitmem, ...gitmemEntries]; - } - - config.hooks = merged; + config.hooks = mergeGitmemHooks(config.hooks, buildCursorHooks()).hooks; + backupFile(hooksPath); writeJson(hooksPath, config); const preserveNote = existingHookCount > 0 @@ -937,7 +916,8 @@ async function stepAgentsMd() { } async function stepFeedbackOptIn() { - const configPath = join(gitmemDir, "config.json"); + // The server reads feedback_enabled from the store's config.json. + const configPath = join(storeDir, "config.json"); const config = readJson(configPath) || {}; // Already opted in — skip @@ -950,7 +930,7 @@ async function stepFeedbackOptIn() { if (!interactive && autoYes) { log(CHECK, "Anonymous feedback sharing is off", - "Run with --interactive to enable, or set feedback_enabled in .gitmem/config.json" + `Run with --interactive to enable, or set feedback_enabled in ${storeName}/config.json` ); return { done: false }; } @@ -964,7 +944,7 @@ async function stepFeedbackOptIn() { if (!accepted) { log(CHECK, "Anonymous feedback sharing is off", - "You can enable it later in .gitmem/config.json" + `You can enable it later in ${storeName}/config.json` ); return { done: false }; } @@ -975,6 +955,7 @@ async function stepFeedbackOptIn() { } config.feedback_enabled = true; + mkdirSync(storeDir, { recursive: true }); writeJson(configPath, config); log(CHECK, diff --git a/bin/uninstall.js b/bin/uninstall.js index f4b12ff..02c5224 100644 --- a/bin/uninstall.js +++ b/bin/uninstall.js @@ -14,10 +14,13 @@ import { writeFileSync, existsSync, rmSync, + readdirSync, } from "fs"; import { dirname, join } from "path"; import { fileURLToPath } from "url"; import { createInterface } from "readline/promises"; +import { storeRoot, repoGitmemDir, displayPath } from "./gitmem-root.js"; +import { removeGitmemHooks, backupFile } from "./hooks-merge.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); const cwd = process.cwd(); @@ -122,7 +125,10 @@ function detectClient() { const client = detectClient(); const cc = CLIENT_CONFIGS[client]; -const gitmemDir = join(cwd, ".gitmem"); +// GIT-115: the repo's .gitmem/ (config.json + hooks) and the store the +// server reads are separate, and handled separately below. +const repoDir = repoGitmemDir(cwd); +const storeDir = storeRoot(); const gitignorePath = join(cwd, ".gitignore"); let rl; @@ -153,18 +159,6 @@ function writeJson(path, data) { writeFileSync(path, JSON.stringify(data, null, 2) + "\n"); } -function isGitmemHook(entry) { - if (entry.hooks && Array.isArray(entry.hooks)) { - return entry.hooks.some( - (h) => typeof h.command === "string" && h.command.includes("gitmem") - ); - } - if (typeof entry.command === "string") { - return entry.command.includes("gitmem"); - } - return false; -} - // ── Steps ── function stepInstructions() { @@ -232,94 +226,34 @@ function stepHooks() { return stepHooksCursor(); } -function stepHooksClaude() { - const settings = readJson(cc.settingsFile); - if (!settings?.hooks) { - log(SKIP, "No hooks in .claude/settings.json"); +/** + * GIT-120 matching: remove only the commands gitmem installed. A matcher group + * keeps its other commands; the hooks key goes only when nothing is left. + */ +function removeHooksFrom(file, name) { + const cfg = readJson(file); + if (!cfg?.hooks) { + log(SKIP, `No hooks in ${name}`); return; } - - let removed = 0; - let preserved = 0; - const cleaned = {}; - - for (const [eventType, entries] of Object.entries(settings.hooks)) { - if (!Array.isArray(entries)) continue; - const nonGitmem = entries.filter((e) => { - if (isGitmemHook(e)) { - removed++; - return false; - } - preserved++; - return true; - }); - if (nonGitmem.length > 0) { - cleaned[eventType] = nonGitmem; - } - } - - if (removed === 0) { + const result = removeGitmemHooks(cfg.hooks); + if (result.removed === 0) { log(SKIP, "No gitmem hooks found"); return; } + if (result.hooks) cfg.hooks = result.hooks; else delete cfg.hooks; + const backup = backupFile(file); + writeJson(file, cfg); + const kept = result.kept > 0 ? `(${result.kept} other hook${result.kept !== 1 ? "s" : ""} preserved)` : ""; + log(CHECK, "Removed automatic memory hooks", [kept, backup ? `backup: ${displayPath(backup)}` : ""].filter(Boolean).join(" ")); +} - if (Object.keys(cleaned).length > 0) { - settings.hooks = cleaned; - } else { - delete settings.hooks; - } - - writeJson(cc.settingsFile, settings); - if (preserved > 0) { - log(CHECK, "Removed automatic memory hooks", `(${preserved} other hook${preserved !== 1 ? "s" : ""} preserved)`); - } else { - log(CHECK, "Removed automatic memory hooks"); - } +function stepHooksClaude() { + removeHooksFrom(cc.settingsFile, ".claude/settings.json"); } function stepHooksCursor() { - const config = readJson(cc.hooksFile); - if (!config?.hooks) { - log(SKIP, `No hooks in ${cc.hooksFileName}`); - return; - } - - let removed = 0; - let preserved = 0; - const cleaned = {}; - - for (const [eventType, entries] of Object.entries(config.hooks)) { - if (!Array.isArray(entries)) continue; - const nonGitmem = entries.filter((e) => { - if (isGitmemHook(e)) { - removed++; - return false; - } - preserved++; - return true; - }); - if (nonGitmem.length > 0) { - cleaned[eventType] = nonGitmem; - } - } - - if (removed === 0) { - log(SKIP, "No gitmem hooks found"); - return; - } - - if (Object.keys(cleaned).length > 0) { - config.hooks = cleaned; - } else { - delete config.hooks; - } - - writeJson(cc.hooksFile, config); - if (preserved > 0) { - log(CHECK, "Removed automatic memory hooks", `(${preserved} other hook${preserved !== 1 ? "s" : ""} preserved)`); - } else { - log(CHECK, "Removed automatic memory hooks"); - } + removeHooksFrom(cc.hooksFile, cc.hooksFileName); } function stepPermissions() { @@ -359,23 +293,51 @@ function stepPermissions() { log(CHECK, "Removed tool permissions"); } +/** + * GIT-115: the repo's .gitmem/ holds gitmem's config.json and hooks/, which + * are this repo's integration and go with it. Anything else in there (a store + * from an older install) is left alone. + */ +function stepRepoDir() { + if (!existsSync(repoDir)) { + log(SKIP, "No .gitmem/ in this repo"); + return; + } + for (const entry of ["config.json", "hooks"]) { + rmSync(join(repoDir, entry), { recursive: true, force: true }); + } + const rest = readdirSync(repoDir); + if (rest.length === 0) { + rmSync(repoDir, { recursive: true, force: true }); + log(CHECK, "Removed .gitmem/ from this repo", "(config.json and hooks)"); + } else { + log(CHECK, "Removed .gitmem/config.json and .gitmem/hooks/", + `(${rest.length} other file${rest.length !== 1 ? "s" : ""} left in .gitmem/)`); + } +} + +/** + * The memory store is shared by every project on this machine, so it is kept + * unless --all is given or the user says to delete it. + */ async function stepGitmemDir() { - if (!existsSync(gitmemDir)) { - log(SKIP, "No .gitmem/ directory"); + const name = displayPath(storeDir); + if (!existsSync(storeDir)) { + log(SKIP, `No memory store at ${name}`); return; } if (deleteAll) { - rmSync(gitmemDir, { recursive: true, force: true }); - log(CHECK, "Deleted .gitmem/ directory"); + rmSync(storeDir, { recursive: true, force: true }); + log(CHECK, `Deleted memory store ${name}`); return; } - if (await confirm("Keep .gitmem/ memory data for future use?", true)) { - log(CHECK, ".gitmem/ preserved — your memories will be here if you reinstall"); + if (await confirm(`Keep memory store ${name}? (shared by every project on this machine)`, true)) { + log(CHECK, `${name} preserved — your memories will be there if you reinstall`); } else { - rmSync(gitmemDir, { recursive: true, force: true }); - log(CHECK, "Deleted .gitmem/ directory"); + rmSync(storeDir, { recursive: true, force: true }); + log(CHECK, `Deleted memory store ${name}`); } } @@ -406,6 +368,7 @@ async function main() { stepMcpJson(); stepHooks(); stepPermissions(); + stepRepoDir(); await stepGitmemDir(); stepGitignore(); diff --git a/hooks/scripts/auto-retrieve-hook.sh b/hooks/scripts/auto-retrieve-hook.sh index 7bdfabb..d1befad 100755 --- a/hooks/scripts/auto-retrieve-hook.sh +++ b/hooks/scripts/auto-retrieve-hook.sh @@ -36,7 +36,20 @@ HOOK_INPUT=$(cat -) # Extract prompt from hook input # ============================================================================ -# Use node if available, fallback to basic extraction +# GIT-116: a missing tool is a visible, non-blocking hook error (exit 1 with a +# reason on stderr; exit 2 would block the prompt), never a silent "nothing +# relevant". Retrieval needs node, which the MCP server needs anyway — but the +# hook runs with the launcher's PATH, where an nvm/volta node may be absent. +gitmem_unavailable() { + echo "gitmem auto-retrieve is not running: $1" >&2 + exit 1 +} + +if ! command -v node &>/dev/null; then + gitmem_unavailable "node is not on the hook's PATH (PATH=$PATH)" +fi + +# Extract the prompt (node is required; checked above) PROMPT="" if command -v node &>/dev/null; then PROMPT=$(echo "$HOOK_INPUT" | node -e " @@ -134,10 +147,6 @@ fi # Invoke quick-retrieve (Node module) # ============================================================================ -# Require node for retrieval -if ! command -v node &>/dev/null; then - exit 0 -fi # Locate quick-retrieve.js relative to plugin root # Hook scripts are at: ${CLAUDE_PLUGIN_ROOT}/scripts/ @@ -150,13 +159,17 @@ if [ ! -f "$QUICK_RETRIEVE" ]; then QUICK_RETRIEVE="${CLAUDE_PLUGIN_ROOT}/../dist/hooks/quick-retrieve.js" fi -# If still not found, try the gitmem package location +# Hooks copied into /.gitmem/hooks (init, install-hooks) have no dist/ +# beside them, so look where the package itself is installed. GIT-116: this +# searched for a package called "gitmem"; it is "gitmem-mcp". Most installs run +# it through npx, so npx's cache is searched too (newest first). if [ ! -f "$QUICK_RETRIEVE" ]; then - # Check common install locations + REL="gitmem-mcp/dist/hooks/quick-retrieve.js" + BIN="$(command -v gitmem-mcp 2>/dev/null || true)" for CANDIDATE in \ - "/workspace/gitmem/dist/hooks/quick-retrieve.js" \ - "$(npm root -g 2>/dev/null)/gitmem/dist/hooks/quick-retrieve.js" \ - "$(dirname "$(which gitmem 2>/dev/null)")/../lib/node_modules/gitmem/dist/hooks/quick-retrieve.js"; do + ${BIN:+"$(dirname "$BIN")/../lib/node_modules/$REL"} \ + $(ls -1t "$HOME"/.npm/_npx/*/node_modules/$REL 2>/dev/null) \ + "$(npm root -g 2>/dev/null)/$REL"; do if [ -f "$CANDIDATE" ]; then QUICK_RETRIEVE="$CANDIDATE" break @@ -165,13 +178,13 @@ if [ ! -f "$QUICK_RETRIEVE" ]; then fi if [ ! -f "$QUICK_RETRIEVE" ]; then - # Can't find quick-retrieve module — fail open - exit 0 + gitmem_unavailable "quick-retrieve.js not found next to the hook, in npx's cache, or in the global node_modules" fi -# Call quick-retrieve with prompt and level -# Timeout: 2.5s (leave 500ms buffer within 3s hook timeout) -RESULT=$(timeout 2.5 node "$QUICK_RETRIEVE" "$PROMPT" "$RETRIEVAL_LEVEL" 2>/dev/null) || true +# Call quick-retrieve with prompt and level. It bounds its own run (2.5 s, +# inside the 3 s hook budget): GIT-116 dropped the `timeout` wrapper, which +# macOS does not ship. +RESULT=$(node "$QUICK_RETRIEVE" "$PROMPT" "$RETRIEVAL_LEVEL" 2>/dev/null) || true # Empty result → nothing relevant found if [ -z "$RESULT" ]; then diff --git a/hooks/scripts/credential-guard.sh b/hooks/scripts/credential-guard.sh index 83361d3..0ab006f 100755 --- a/hooks/scripts/credential-guard.sh +++ b/hooks/scripts/credential-guard.sh @@ -111,6 +111,10 @@ if [ "$TOOL_NAME" = "Read" ]; then # Block reading known credential files if echo "$BASENAME" | grep -qEi "$CREDENTIAL_FILES_PATTERN"; then + # GIT-116: escape for the JSON below. A quote or backslash in the path + # made the output invalid JSON, which Claude Code treats as "allow". + FILE_PATH=$(printf '%s' "$FILE_PATH" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g') + BASENAME=$(printf '%s' "$BASENAME" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g') cat </dev/null && return 0 + if [ -d /proc/self ]; then + [ -d "/proc/$1" ] && return 0 || return 1 + fi + if command -v ps >/dev/null 2>&1; then + ps -p "$1" >/dev/null 2>&1 && return 0 || return 1 + fi local out - out=$(kill -0 "$1" 2>&1) && return 0 + out=$(LC_ALL=C kill -0 "$1" 2>&1) case "$out" in *ermitted*) return 0 ;; esac return 1 } @@ -59,7 +68,8 @@ gitmem_pid_alive() { gitmem_live_session_ids() { [ -f "$GITMEM_ACTIVE_SESSIONS" ] || return 0 local host rows - host=$(hostname 2>/dev/null || echo "") + # bash's own HOSTNAME is gethostname(), what the server's os.hostname() is. + host="${HOSTNAME:-$(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("|")' \ diff --git a/hooks/scripts/session-start.sh b/hooks/scripts/session-start.sh index 8e11fbc..246ff67 100755 --- a/hooks/scripts/session-start.sh +++ b/hooks/scripts/session-start.sh @@ -7,13 +7,16 @@ set -e -# Debug logging — verify hook actually fires -PLUGIN_LOG="/tmp/gitmem-hooks-plugin-debug.log" -echo "[$(date)] PLUGIN SessionStart hook invoked" >> "$PLUGIN_LOG" +# Debug logging — verify hook actually fires. GIT-116: per user and in +# $TMPDIR (macOS gives each user its own), and never fatal: under `set -e` an +# unwritable log — e.g. /tmp/…debug.log created by another user — used to kill +# the whole SessionStart hook. +PLUGIN_LOG="${TMPDIR:-/tmp}/gitmem-hooks-plugin-debug-$(id -u).log" +echo "[$(date)] PLUGIN SessionStart hook invoked" >> "$PLUGIN_LOG" 2>/dev/null || true # Read hook input from stdin HOOK_INPUT=$(cat -) -echo "[$(date)] Input: $HOOK_INPUT" >> "$PLUGIN_LOG" +echo "[$(date)] Input: $HOOK_INPUT" >> "$PLUGIN_LOG" 2>/dev/null || true # ============================================================================ # Detect gitmem MCP server @@ -98,8 +101,7 @@ fi if [ "$GITMEM_DETECTED" = "false" ]; then for SERVER_PATH in \ "${GITMEM_SERVER_PATH:-}" \ - "$(which gitmem-mcp 2>/dev/null || echo '')" \ - "$(which gitmem 2>/dev/null || echo '')"; do + "$(command -v gitmem-mcp 2>/dev/null || echo '')"; do if [ -n "$SERVER_PATH" ] && [ -f "$SERVER_PATH" ]; then GITMEM_DETECTED=true DETECT_SOURCE="binary $SERVER_PATH" @@ -122,13 +124,13 @@ fi # MCP handshake completion. Total budget: ~8s (within 10s hook timeout). if [ "$GITMEM_DETECTED" = "true" ]; then - GATE_START=$(date +%s%N 2>/dev/null || date +%s) + GATE_START=$(date +%s) MAX_WAIT_SECS=7 POLL_INTERVAL=0.3 HANDSHAKE_BUFFER=0.5 SERVER_FOUND=false - echo "[$(date)] MCP readiness gate: waiting up to ${MAX_WAIT_SECS}s for gitmem server process..." >> "$PLUGIN_LOG" + echo "[$(date)] MCP readiness gate: waiting up to ${MAX_WAIT_SECS}s for gitmem server process..." >> "$PLUGIN_LOG" 2>/dev/null || true # Build list of patterns to match the gitmem server process # Covers: direct node invocation, npx, and symlinked binaries @@ -140,12 +142,19 @@ if [ "$GITMEM_DETECTED" = "true" ]; then # Poll every 0.3s. Max iterations = MAX_WAIT_SECS / 0.3 ≈ 23 MAX_POLLS=$(( MAX_WAIT_SECS * 10 / 3 )) POLL=0 + # GIT-116: pgrep ships with macOS and procps, but not with every slim + # image. Without it every poll failed and the gate stalled for its full + # 7 s before continuing; skip the wait instead, and say so. + if ! command -v pgrep >/dev/null 2>&1; then + echo "[$(date)] MCP readiness gate: pgrep not available — not waiting for the server process" >> "$PLUGIN_LOG" 2>/dev/null || true + MAX_POLLS=0 + fi while [ "$POLL" -lt "$MAX_POLLS" ]; do for PATTERN in "${GITMEM_PATTERNS[@]}"; do if pgrep -f "$PATTERN" > /dev/null 2>&1; then SERVER_FOUND=true WAIT_SECS=$(( POLL * 3 / 10 )) - echo "[$(date)] MCP readiness gate: gitmem process found (pattern: $PATTERN) after ~${WAIT_SECS}.$(( POLL * 3 % 10 ))s" >> "$PLUGIN_LOG" + echo "[$(date)] MCP readiness gate: gitmem process found (pattern: $PATTERN) after ~${WAIT_SECS}.$(( POLL * 3 % 10 ))s" >> "$PLUGIN_LOG" 2>/dev/null || true # Buffer for MCP protocol handshake to complete sleep "$HANDSHAKE_BUFFER" break 2 @@ -156,11 +165,11 @@ if [ "$GITMEM_DETECTED" = "true" ]; then done if [ "$SERVER_FOUND" = "false" ]; then - echo "[$(date)] MCP readiness gate: gitmem process NOT found after ${MAX_WAIT_SECS}s — proceeding anyway (config detected, server may use different process name)" >> "$PLUGIN_LOG" + echo "[$(date)] MCP readiness gate: gitmem process NOT found after ${MAX_WAIT_SECS}s — proceeding anyway (config detected, server may use different process name)" >> "$PLUGIN_LOG" 2>/dev/null || true fi - GATE_END=$(date +%s%N 2>/dev/null || date +%s) - echo "[$(date)] MCP readiness gate completed" >> "$PLUGIN_LOG" + GATE_END=$(date +%s) + echo "[$(date)] MCP readiness gate completed" >> "$PLUGIN_LOG" 2>/dev/null || true fi # ============================================================================ @@ -179,7 +188,7 @@ echo "0" > "$STATE_DIR/last_nag_time" # ============================================================================ if [ "$GITMEM_DETECTED" = "true" ]; then - echo "[$(date)] Gitmem DETECTED via ${DETECT_SOURCE} — instructing agent to call session_start via MCP" >> "$PLUGIN_LOG" + echo "[$(date)] Gitmem DETECTED via ${DETECT_SOURCE} — instructing agent to call session_start via MCP" >> "$PLUGIN_LOG" 2>/dev/null || true # Instruct agent to call session_start via MCP. # The MCP server creates sessions with its own PID, enabling multi-session @@ -191,13 +200,19 @@ if [ "$GITMEM_DETECTED" = "true" ]; then # # Read project from .gitmem/config.json if available. # Fallback: let session_start default (reads config.json server-side too). + # GIT-115: this is the repo's config.json, not the store's; it stays in the + # repo so each repo keeps its own project. node when jq is absent. GITMEM_PROJECT="" - if [ -f ".gitmem/config.json" ] && command -v jq &>/dev/null; then - GITMEM_PROJECT=$(jq -r '.project // empty' .gitmem/config.json 2>/dev/null || true) + if [ -f ".gitmem/config.json" ]; then + if command -v jq &>/dev/null; then + GITMEM_PROJECT=$(jq -r '.project // empty' .gitmem/config.json 2>/dev/null || true) + elif command -v node &>/dev/null; then + GITMEM_PROJECT=$(node -e "try{const p=JSON.parse(require('fs').readFileSync('.gitmem/config.json','utf8')).project;if(typeof p==='string')process.stdout.write(p)}catch(e){}" 2>/dev/null || true) + fi fi if [ -n "$GITMEM_PROJECT" ]; then - echo "[$(date)] Project from config: ${GITMEM_PROJECT}" >> "$PLUGIN_LOG" + echo "[$(date)] Project from config: ${GITMEM_PROJECT}" >> "$PLUGIN_LOG" 2>/dev/null || true cat <> "$PLUGIN_LOG" + echo "[$(date)] Gitmem NOT detected (checked: project .mcp.json, mcp-config, ~/.claude.json, disk)" >> "$PLUGIN_LOG" 2>/dev/null || true echo "GITMEM PLUGIN: GitMem MCP server not detected. Session lifecycle hooks are inactive. To enable, ensure gitmem is configured via --mcp-config, project .mcp.json, or set GITMEM_ENABLED=true." fi -echo "[$(date)] Hook completed, exit 0" >> "$PLUGIN_LOG" +echo "[$(date)] Hook completed, exit 0" >> "$PLUGIN_LOG" 2>/dev/null || true exit 0 diff --git a/hooks/tests/test-hooks.sh b/hooks/tests/test-hooks.sh index 95d5df8..8113fee 100755 --- a/hooks/tests/test-hooks.sh +++ b/hooks/tests/test-hooks.sh @@ -128,6 +128,22 @@ else pass "Output is plain text, not JSON" fi +# Test 1.2b (GIT-115): the project hint comes from the repo's .gitmem/config.json, +# with or without jq (node fallback). +# A repo of its own: the suite's GITMEM_DIR ($TMPDIR/.gitmem) is the store. +mkdir -p "$TMPDIR/repo-git115/.gitmem" +cp "$TMPDIR/.mcp.json" "$TMPDIR/repo-git115/.mcp.json" +echo '{"project":"acme-hooks"}' > "$TMPDIR/repo-git115/.gitmem/config.json" +OUTPUT=$(cd "$TMPDIR/repo-git115" && echo '{}' | bash "$SCRIPT_DIR/scripts/session-start.sh" 2>/dev/null) +if echo "$OUTPUT" | grep -q 'session_start(project: "acme-hooks")'; then + pass "Project hint read from the repo's .gitmem/config.json" +else + fail "Project hint read from the repo's .gitmem/config.json" \ + 'Contains session_start(project: "acme-hooks")' \ + "$OUTPUT" +fi +rm -rf "$TMPDIR/repo-git115" + # Test 1.3: Gitmem NOT in .mcp.json (may still detect via disk fallback) rm "$TMPDIR/.mcp.json" echo '{"mcpServers":{"other-tool":{}}}' > "$TMPDIR/.mcp.json" @@ -726,6 +742,92 @@ GOT=$(bash -c '. "$0/scripts/resolve-root.sh"; gitmem_pid_alive 1 && echo alive' remove_session_registry +# ============================================================================ +# TEST GROUP: portability (GIT-116) +# ============================================================================ + +echo "" +echo -e "${YELLOW}=== Portability (GIT-116) ===${NC}" + +# A PATH holding every command on the current PATH except the ones named. +path_without() { + local out="$1"; shift + mkdir -p "$out" + local d f n old_ifs="$IFS" + IFS=: + for d in $PATH; do + [ -d "$d" ] || continue + for f in "$d"/*; do + n=$(basename "$f") + [ -x "$f" ] && [ ! -e "$out/$n" ] && ln -s "$f" "$out/$n" 2>/dev/null || true + done + done + IFS="$old_ifs" + for n in "$@"; do rm -f "$out/$n"; done + echo "$out" +} + +# A fake installed package: hooks/scripts next to dist/hooks/quick-retrieve.js. +PKG="$TMPDIR/pkg" +mkdir -p "$PKG/hooks/scripts" "$PKG/dist/hooks" +cp "$SCRIPT_DIR"/scripts/*.sh "$PKG/hooks/scripts/" +cat > "$PKG/dist/hooks/quick-retrieve.js" <<'STUB' +process.stdout.write("STUB-SCAR for: " + process.argv[2]); +STUB +PROMPT_JSON='{"prompt":"add a retry to the deploy script before the migration"}' + +NO_TIMEOUT_PATH=$(path_without "$TMPDIR/bin-no-timeout" timeout gtimeout) +OUT=$(echo "$PROMPT_JSON" | PATH="$NO_TIMEOUT_PATH" bash "$PKG/hooks/scripts/auto-retrieve-hook.sh" 2>/dev/null) || true +echo "$OUT" | grep -q "STUB-SCAR" && pass "auto-retrieve works with no timeout/gtimeout on PATH (stock macOS)" \ + || fail "auto-retrieve without timeout" "STUB-SCAR in additionalContext" "$OUT" + +NO_NODE_PATH=$(path_without "$TMPDIR/bin-no-node" node) +set +e +ERR=$(echo "$PROMPT_JSON" | PATH="$NO_NODE_PATH" bash "$PKG/hooks/scripts/auto-retrieve-hook.sh" 2>&1 >/dev/null); CODE=$? +set -e +[ "$CODE" = "1" ] && echo "$ERR" | grep -q "node is not on the hook's PATH" \ + && pass "no node: visible non-blocking error (exit 1, reason on stderr)" \ + || fail "no node visible" "exit 1 + reason" "exit=$CODE err=$ERR" + +# Hooks copied into a repo (init / install-hooks): no dist beside them. +COPIED="$TMPDIR/repo/.gitmem/hooks"; mkdir -p "$COPIED"; cp "$SCRIPT_DIR"/scripts/*.sh "$COPIED/" +EMPTY_HOME="$TMPDIR/empty-home"; mkdir -p "$EMPTY_HOME" +NO_PKG_PATH=$(path_without "$TMPDIR/bin-no-pkg" gitmem-mcp npm) +set +e +ERR=$(echo "$PROMPT_JSON" | HOME="$EMPTY_HOME" PATH="$NO_PKG_PATH" bash "$COPIED/auto-retrieve-hook.sh" 2>&1 >/dev/null); CODE=$? +set -e +[ "$CODE" = "1" ] && echo "$ERR" | grep -q "quick-retrieve.js not found" \ + && pass "module not found: visible non-blocking error, not a silent 'nothing relevant'" \ + || fail "module not found visible" "exit 1 + reason" "exit=$CODE err=$ERR" + +NPX_HOME="$TMPDIR/npx-home" +mkdir -p "$NPX_HOME/.npm/_npx/abc123/node_modules/gitmem-mcp/dist/hooks" +cp "$PKG/dist/hooks/quick-retrieve.js" "$NPX_HOME/.npm/_npx/abc123/node_modules/gitmem-mcp/dist/hooks/" +OUT=$(echo "$PROMPT_JSON" | HOME="$NPX_HOME" PATH="$NO_PKG_PATH" bash "$COPIED/auto-retrieve-hook.sh" 2>/dev/null) || true +echo "$OUT" | grep -q "STUB-SCAR" && pass "copied hooks find gitmem-mcp in npx's cache" \ + || fail "npx cache lookup" "STUB-SCAR" "$OUT" + +# session-start: no pgrep must not stall the 10 s hook for the 7 s gate. +NO_PGREP_PATH=$(path_without "$TMPDIR/bin-no-pgrep" pgrep) +T0=$(date +%s) +OUT=$(echo '{}' | GITMEM_ENABLED=true PATH="$NO_PGREP_PATH" bash "$SCRIPT_DIR/scripts/session-start.sh" 2>/dev/null) || true +ELAPSED=$(( $(date +%s) - T0 )) +[ "$ELAPSED" -lt 4 ] && echo "$OUT" | grep -q "SESSION START" \ + && pass "session-start without pgrep does not stall (${ELAPSED}s)" \ + || fail "session-start without pgrep" "<4s and SESSION START" "${ELAPSED}s: $OUT" + +# session-start: an unwritable debug log must not kill the hook under set -e. +BAD_TMP="$TMPDIR/bad-tmp"; mkdir -p "$BAD_TMP/gitmem-hooks-plugin-debug-$(id -u).log" +OUT=$(echo '{}' | GITMEM_ENABLED=true TMPDIR="$BAD_TMP" PATH="$NO_PGREP_PATH" bash "$SCRIPT_DIR/scripts/session-start.sh" 2>/dev/null) || true +echo "$OUT" | grep -q "SESSION START" && pass "session-start survives an unwritable debug log" \ + || fail "unwritable debug log" "SESSION START" "$OUT" + +# credential-guard: a quote or backslash in the path must still produce JSON that blocks. +OUT=$(printf '{"tool_name":"Read","tool_input":{"file_path":"/x/we\\"ird\\\\d/.env"}}' | bash "$SCRIPT_DIR/scripts/credential-guard.sh" 2>/dev/null) || true +echo "$OUT" | node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{process.exit(JSON.parse(d).decision==="block"?0:1)})' 2>/dev/null \ + && pass "credential-guard block is valid JSON for a path with quotes" \ + || fail "credential-guard JSON" "valid JSON, decision block" "$OUT" + # ============================================================================ # Summary # ============================================================================ diff --git a/package-lock.json b/package-lock.json index 31aee33..acbbc94 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "gitmem-mcp", - "version": "1.11.0", + "version": "1.11.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "gitmem-mcp", - "version": "1.11.0", + "version": "1.11.1", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.0.0", diff --git a/package.json b/package.json index 70b18cf..f111369 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gitmem-mcp", - "version": "1.11.0", + "version": "1.11.1", "mcpName": "io.github.gitmem-dev/gitmem", "description": "Persistent learning memory for AI coding agents. Memory that compounds.", "type": "module", diff --git a/src/commands/telemetry.ts b/src/commands/telemetry.ts index 80068a5..b01838c 100644 --- a/src/commands/telemetry.ts +++ b/src/commands/telemetry.ts @@ -9,6 +9,8 @@ */ import { getTelemetry, Telemetry } from "../lib/telemetry.js"; +// GIT-115: telemetry state lives in the store the server reads, not /.gitmem. +import { getGitmemDir } from "../services/gitmem-dir.js"; import { join } from "path"; import { readFileSync } from "fs"; @@ -23,9 +25,6 @@ function getPackageVersion(): string { } } -function getGitmemDir(): string { - return join(process.cwd(), ".gitmem"); -} export function main(args: string[]): void { const subcommand = args[0]; @@ -117,7 +116,7 @@ function cmdEnable(telemetry: Telemetry): void { console.log(" ✗ IP addresses or persistent IDs"); console.log(""); console.log("Transparency:"); - console.log(" • All events logged to .gitmem/telemetry.log"); + console.log(` • All events logged to ${join(getGitmemDir(), "telemetry.log")}`); console.log(" • View before sending: gitmem telemetry show"); console.log(" • Disable anytime: gitmem telemetry disable"); console.log(""); @@ -139,7 +138,7 @@ function cmdEnable(telemetry: Telemetry): void { telemetry.enable(); console.log(""); console.log("\x1b[32m✓\x1b[0m Telemetry enabled"); - console.log(" Data logged to: .gitmem/telemetry.log"); + console.log(` Data logged to: ${join(getGitmemDir(), "telemetry.log")}`); console.log(" Review anytime: gitmem telemetry show"); console.log(" Disable anytime: gitmem telemetry disable"); } else { @@ -160,7 +159,7 @@ function cmdDisable(telemetry: Telemetry): void { console.log("==========================\n"); console.log("\x1b[32m✓\x1b[0m Telemetry disabled"); console.log(" Pending events: will not be sent"); - console.log(" Local logs: preserved at .gitmem/telemetry.log"); + console.log(` Local logs: preserved at ${join(getGitmemDir(), "telemetry.log")}`); console.log(""); console.log("To re-enable: gitmem telemetry enable"); } diff --git a/src/hooks/quick-retrieve.ts b/src/hooks/quick-retrieve.ts index c414b33..41cfc4b 100644 --- a/src/hooks/quick-retrieve.ts +++ b/src/hooks/quick-retrieve.ts @@ -148,12 +148,37 @@ function toFormattable(scar: RelevantScar): FormattableScar { }; } +// --- Time bound (GIT-116) --- +// The UserPromptSubmit hook has a 3 s budget. The hook used to enforce it with +// `timeout 2.5 node …`, but macOS ships no `timeout`, so on a stock Mac the +// call failed, the error was discarded and nothing was ever retrieved. The +// bound lives here now: node is the one tool the hook already requires. + +export const QUICK_RETRIEVE_DEADLINE_MS = 2500; + +/** + * Exit quietly (no context injected) once `ms` have passed. unref'd, so a + * run that finishes first is not held open by the timer. + */ +export function armDeadline( + ms: number, + onExpire: () => void = () => { + process.stderr.write(`gitmem quick-retrieve: no result within ${ms} ms; nothing injected\n`); + process.exit(0); + } +): ReturnType { + const timer = setTimeout(onExpire, ms); + timer.unref?.(); + return timer; +} + // --- CLI entry point --- // Called from hook: node dist/hooks/quick-retrieve.js [token_budget] const isMainModule = process.argv[1]?.endsWith("quick-retrieve.js"); if (isMainModule) { + armDeadline(Number(process.env.GITMEM_QUICK_RETRIEVE_DEADLINE_MS) || QUICK_RETRIEVE_DEADLINE_MS); const prompt = process.argv[2]; const level = process.argv[3] || "scars"; const budget = process.argv[4] ? parseInt(process.argv[4], 10) : undefined; diff --git a/src/services/display-protocol.ts b/src/services/display-protocol.ts index ec712e8..5e0cd2c 100644 --- a/src/services/display-protocol.ts +++ b/src/services/display-protocol.ts @@ -156,6 +156,7 @@ export const STATUS = { rejected: `${red}REJECTED${reset}`, complete: `${green}COMPLETE${reset}`, failed: `${red}FAILED${reset}`, + partial: `${yellow}PARTIAL${reset}`, pass: `${green}+${reset}`, miss: `${red}-${reset}`, } as const; diff --git a/src/services/gitmem-dir.ts b/src/services/gitmem-dir.ts index feba098..3f11d8d 100644 --- a/src/services/gitmem-dir.ts +++ b/src/services/gitmem-dir.ts @@ -15,6 +15,7 @@ import * as path from "path"; import * as fs from "fs"; import * as os from "os"; +import { fileURLToPath } from "url"; /** * Validate a string intended for use as a single path component (directory name or filename). @@ -272,9 +273,16 @@ function warnAboutStrandedProjectRoots(home: string): void { * without ever having held a session. * * Any ONE of these counts: - * - config.json a deliberate project-scoped install * - a registered session the registry names at least one * - a real session dir sessions//session.json parses with a session_id + * - a real record a learning, thread or decision that `init` did not seed + * + * GIT-115: config.json no longer counts. A repo's .gitmem/ now holds its + * config.json (the project name the SessionStart hook reads) and the hook + * scripts on purpose, while the store lives in the root the server reads. A + * repo set up that way is not a stranded store. Starter lessons and the + * welcome thread are not evidence either: every older `init` seeded them into + * the repo, and the root being read has its own copy. * * Exported for tests and diagnostics; the resolution path is the only caller * that matters. @@ -283,8 +291,7 @@ export function isLiveGitmemRoot(candidate: string): boolean { try { if (!fs.existsSync(candidate)) return false; - // A project-scoped install is deliberate — honour it even when idle. - if (fs.existsSync(path.join(candidate, "config.json"))) return true; + if (holdsRealRecords(candidate)) return true; const registryPath = path.join(candidate, "active-sessions.json"); if (fs.existsSync(registryPath)) { @@ -317,6 +324,43 @@ export function isLiveGitmemRoot(candidate: string): boolean { } } +/** What `init` seeds: starter lesson ids and the welcome thread (GIT-115). */ +const WELCOME_THREAD_ID = "t-welcome01"; +let starterIds: Set | null = null; +function starterLearningIds(): Set { + if (starterIds) return starterIds; + starterIds = new Set(); + try { + // src/services or dist/services → /schema/starter-scars.json + const here = path.dirname(fileURLToPath(import.meta.url)); + const file = path.join(here, "..", "..", "schema", "starter-scars.json"); + for (const s of JSON.parse(fs.readFileSync(file, "utf-8"))) if (s?.id) starterIds.add(String(s.id)); + } catch { + // Unreadable: every learning then counts, which errs toward reporting. + } + return starterIds; +} + +/** Entries of a store file that may be a bare array or {key: array}. */ +function readCollection(file: string, key: string): Array> { + try { + if (!fs.existsSync(file)) return []; + const parsed = JSON.parse(fs.readFileSync(file, "utf-8")); + const list = Array.isArray(parsed) ? parsed : parsed?.[key]; + return Array.isArray(list) ? list : []; + } catch { + return []; + } +} + +/** GIT-115: a learning, thread or decision that `init` did not seed. */ +function holdsRealRecords(root: string): boolean { + const starters = starterLearningIds(); + if (readCollection(path.join(root, "learnings.json"), "learnings").some((l) => !starters.has(String(l?.id)))) return true; + if (readCollection(path.join(root, "threads.json"), "threads").some((t) => String(t?.id) !== WELCOME_THREAD_ID)) return true; + return readCollection(path.join(root, "decisions.json"), "decisions").length > 0; +} + /** * Get a file path within the .gitmem directory */ diff --git a/src/services/local-vector-search.ts b/src/services/local-vector-search.ts index 99dbea1..d34c110 100644 --- a/src/services/local-vector-search.ts +++ b/src/services/local-vector-search.ts @@ -211,6 +211,11 @@ export class LocalVectorSearch { return this.scars.length; } + /** Ids of the indexed scars (GIT-119: prefix resolution without a query). */ + getScarIds(): string[] { + return this.scars.map((s) => s.scar.id); + } + /** * Search for scars similar to the query * diff --git a/src/services/metrics.ts b/src/services/metrics.ts index 16eac35..bafda1b 100644 --- a/src/services/metrics.ts +++ b/src/services/metrics.ts @@ -443,9 +443,11 @@ export async function updateRelevanceData( ); if (applied.length === 0 && Object.keys(relevance).length === 0) continue; - await supabase.directPatch("gitmem_query_metrics", { id: `eq.${metric.id}` }, { + const patched = await supabase.directPatch("gitmem_query_metrics", { id: `eq.${metric.id}` }, { metadata: { ...(metric.metadata ?? {}), memories_applied: applied, memory_relevance: relevance }, }); + // GIT-119: 0 rows = relevance not recorded; throw so the tracker counts it. + if (patched.count === 0) throw new Error(`relevance for metrics row ${metric.id} updated no row`); } }); } diff --git a/src/services/startup.ts b/src/services/startup.ts index f9bee20..65f6820 100644 --- a/src/services/startup.ts +++ b/src/services/startup.ts @@ -149,7 +149,14 @@ async function deltaSyncFromCache(storeKey: string, fingerprint: StoreFingerprin export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean } = {}): Promise<{ scars: ScarWithEmbedding[]; latestUpdatedAt: string | null; - source: "disk" | "network"; + source: "disk" | "network" | "disk-stale"; + /** + * GIT-118: false when the store could not be read. `scars` is then the last + * disk cache (possibly stale) or [] — never to be mistaken for "the store is + * empty", which is ok: true with no rows. + */ + ok: boolean; + error?: string; }> { const startTime = Date.now(); @@ -167,7 +174,7 @@ export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean `[startup] Vector cache HIT: ${cached.rows.length} learnings from disk in ${Date.now() - startTime}ms ` + `(${cached.bytesOnDisk} bytes on disk, bulk download skipped)` ); - return { scars: cached.rows, latestUpdatedAt: fingerprint.latestUpdatedAt, source: "disk" }; + return { scars: cached.rows, latestUpdatedAt: fingerprint.latestUpdatedAt, source: "disk", ok: true }; } console.error(`[startup] Vector cache MISS: ${cached.reason}`); } @@ -183,7 +190,7 @@ export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean console.error( `[startup] Vector cache HIT (via leader): ${fromLeader.rows.length} learnings in ${Date.now() - startTime}ms, bulk download skipped` ); - return { scars: fromLeader.rows, latestUpdatedAt: fingerprint.latestUpdatedAt, source: "disk" }; + return { scars: fromLeader.rows, latestUpdatedAt: fingerprint.latestUpdatedAt, source: "disk", ok: true }; } console.error("[startup] Leader did not produce a usable cache — downloading"); } @@ -195,7 +202,7 @@ export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean 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" }; + return { scars: synced, latestUpdatedAt: synced[0]?.updated_at || null, source: "network", ok: true }; } } @@ -221,10 +228,18 @@ export async function loadScarsFromSupabase(options: { bypassDiskCache?: boolean writeVectorCache(storeKey, fingerprint, learnings); } - return { scars: learnings, latestUpdatedAt, source: "network" }; + return { scars: learnings, latestUpdatedAt, source: "network", ok: true }; } catch (error) { - console.error("[startup] Failed to load learnings:", error); - return { scars: [], latestUpdatedAt: null, source: "network" }; + // GIT-118: this returned [] with no flag, and every caller reinitialised + // the index and the hooks' scar cache with it — one failed request emptied + // recall until the next successful load. Say it failed, and offer the last + // disk cache (possibly stale) for a caller that has nothing better. + const message = error instanceof Error ? error.message : String(error); + console.error("[startup] Failed to load learnings:", message); + const stale = readVectorCacheBase(storeKey); + return stale && stale.length > 0 + ? { scars: stale, latestUpdatedAt: stale[0]?.updated_at || null, source: "disk-stale", ok: false, error: message } + : { scars: [], latestUpdatedAt: null, source: "network", ok: false, error: message }; } finally { releaseLock(); } @@ -312,10 +327,33 @@ export async function initializeGitMem(_project?: Project): Promise<{ try { // Load ALL scars from Supabase (cross-project unified cache) - const { scars, latestUpdatedAt } = await loadScarsFromSupabase(); + const loaded = await loadScarsFromSupabase(); + const { scars, latestUpdatedAt } = loaded; + + if (!loaded.ok) { + // GIT-118: the store could not be read. Serve the last disk cache if + // there is one (stale beats empty), leave the hooks' cache alone, report + // failure so ensureInitialized tries again, and retry in the background. + if (scars.length > 0) await initializeLocalSearch(scars, undefined, latestUpdatedAt || undefined); + scheduleReloadRetry(loaded.error ?? "store unreachable"); + return { + success: false, + scar_count: getLocalVectorSearch().getScarCount(), + elapsed_ms: Date.now() - startTime, + search_mode: "local", + error: `could not load learnings from the store (${loaded.error})${scars.length > 0 ? `; serving ${scars.length} from the disk cache, possibly stale` : ""}`, + }; + } - // Initialize unified local vector search - await initializeLocalSearch(scars, undefined, latestUpdatedAt || undefined); + // Initialize unified local vector search. GIT-118: if an earlier failed + // start left the stale disk copy in the index, replace it — initialize() + // is a no-op on an index that is already initialized. + if (getLocalVectorSearch().getScarCount() > 0) { + await reinitializeLocalSearch(scars, undefined, latestUpdatedAt || undefined); + } else { + await initializeLocalSearch(scars, undefined, latestUpdatedAt || undefined); + } + clearReloadRetry(); // Persist scars to disk for hook processes (no embeddings) persistScarsForHooks(scars); @@ -370,6 +408,9 @@ export async function ensureInitialized(_project?: Project): Promise { startupCompleted.set(UNIFIED_KEY, result.success); if (!result.success) { console.warn(`[startup] GitMem not fully initialized: ${result.error}`); + // GIT-118: a failed init was cached as this promise forever, so no later + // call ever tried again. Forget it; the next call (or the retry) loads. + startupPromises.delete(UNIFIED_KEY); } })(); @@ -653,9 +694,29 @@ async function checkThreadHealth(project?: Project): Promise } /** - * Flush and reload the cache + * Flush and reload the cache. A flush is the user saying "I do not trust what + * you have": the disk cache is bypassed (GIT-98) and rewritten. */ export async function flushCache(_project?: Project): Promise { + return reloadIndex({ bypassDiskCache: true }); +} + +/** + * GIT-118: bring the index up to date after a write (create_learning). Uses + * the per-row delta (GIT-98), so a new learning costs its own row rather than + * the whole index — cheap enough for the tool to await and report. + */ +export async function refreshIndexAfterWrite(_project?: Project): Promise { + return reloadIndex({ bypassDiskCache: false }); +} + +/** + * Reload the index from the store — or, if the store cannot be read, keep + * what is there (GIT-118). A failed load used to reinitialise the index and + * overwrite the hooks' scar cache with [] and report success, so one failed + * request emptied recall until the next successful load. + */ +async function reloadIndex(options: { bypassDiskCache: boolean }): Promise { const startTime = Date.now(); const config = getConfig(); @@ -672,15 +733,32 @@ export async function flushCache(_project?: Project): Promise const previousCount = getLocalVectorSearch().getScarCount(); try { - // Load ALL fresh scars (cross-project). A flush is the user saying "I do not - // trust what you have" — bypass the disk cache (GIT-98) and rewrite it. - const { scars, latestUpdatedAt } = await loadScarsFromSupabase({ bypassDiskCache: true }); + const loaded = await loadScarsFromSupabase({ bypassDiskCache: options.bypassDiskCache }); + + if (!loaded.ok) { + // Keep the index and the hooks' cache. Only an EMPTY index takes the + // stale disk copy: it has nothing better to lose. + if (previousCount === 0 && loaded.scars.length > 0) { + await reinitializeLocalSearch(loaded.scars, undefined, loaded.latestUpdatedAt || undefined); + } + scheduleReloadRetry(loaded.error ?? "store unreachable"); + const kept = getLocalVectorSearch().getScarCount(); + console.error(`[cache] Reload failed (${loaded.error}); kept ${kept} scars, hook cache unchanged`); + return { + success: false, + previous_scar_count: previousCount, + new_scar_count: kept, + elapsed_ms: Date.now() - startTime, + error: loaded.error ?? "store unreachable", + }; + } // Reinitialize the unified index - await reinitializeLocalSearch(scars, undefined, latestUpdatedAt || undefined); + await reinitializeLocalSearch(loaded.scars, undefined, loaded.latestUpdatedAt || undefined); // Update disk cache for hook processes too - persistScarsForHooks(scars); + persistScarsForHooks(loaded.scars); + clearReloadRetry(); const newCount = getLocalVectorSearch().getScarCount(); const elapsed = Date.now() - startTime; @@ -697,17 +775,59 @@ export async function flushCache(_project?: Project): Promise const elapsed = Date.now() - startTime; const errorMsg = error instanceof Error ? error.message : String(error); console.error("[cache] Flush failed:", errorMsg); + scheduleReloadRetry(errorMsg); return { success: false, previous_scar_count: previousCount, - new_scar_count: previousCount, + new_scar_count: getLocalVectorSearch().getScarCount(), elapsed_ms: elapsed, error: errorMsg, }; } } +// --------------------------------------------------------------------------- +// GIT-118: retry a failed load with backoff (30 s, doubling, capped at 10 min) +// --------------------------------------------------------------------------- + +const RELOAD_RETRY_BASE_MS = 30_000; +const RELOAD_RETRY_MAX_MS = 10 * 60_000; +let reloadRetryTimer: ReturnType | null = null; +let reloadRetryAttempt = 0; + +function scheduleReloadRetry(reason: string): void { + if (reloadRetryTimer) return; // one pending retry at a time + const delay = Math.min(RELOAD_RETRY_BASE_MS * 2 ** reloadRetryAttempt, RELOAD_RETRY_MAX_MS); + reloadRetryAttempt++; + console.error(`[startup] Index reload failed (${reason}); retry ${reloadRetryAttempt} in ${Math.round(delay / 1000)} s`); + reloadRetryTimer = setTimeout(() => { + reloadRetryTimer = null; + reloadIndex({ bypassDiskCache: false }) + .then((r) => { + if (r.success) startupCompleted.set(UNIFIED_KEY, true); + }) + .catch(() => { /* reloadIndex reschedules itself */ }); + }, delay); + reloadRetryTimer.unref?.(); +} + +function clearReloadRetry(): void { + if (reloadRetryTimer) clearTimeout(reloadRetryTimer); + reloadRetryTimer = null; + reloadRetryAttempt = 0; +} + +/** For tests and health. */ +export function getReloadRetryState(): { pending: boolean; attempt: number } { + return { pending: reloadRetryTimer !== null, attempt: reloadRetryAttempt }; +} + +/** For tests. */ +export function resetReloadRetry(): void { + clearReloadRetry(); +} + /** * Check if cache needs refresh based on TTL or staleness * Returns true if a refresh is recommended diff --git a/src/services/supabase-client.ts b/src/services/supabase-client.ts index ca1855a..d02d27d 100644 --- a/src/services/supabase-client.ts +++ b/src/services/supabase-client.ts @@ -14,7 +14,7 @@ import type { SupabaseSearchOptions, } from "../types/index.js"; import { getCache } from "./cache.js"; -import { getTableName } from "./tier.js"; +import { getTableName, getTablePrefix } from "./tier.js"; // --- PostgREST Input Sanitization --- @@ -159,61 +159,6 @@ export async function upsertRecord( return directUpsert(table, data); } -/** - * Semantic search across tables - * - * Generates an embedding for the query, then calls the match_ - * RPC function directly via PostgREST. - */ -export async function semanticSearch( - options: SupabaseSearchOptions -): Promise { - if (!isConfigured()) { - throw new Error("Supabase not configured"); - } - - const { query, match_count = 10 } = options; - - // Generate embedding for the query - const { embed } = await import("./embedding.js"); - const embedding = await embed(query); - - if (!embedding) { - console.error("[semantic-search] No embedding provider configured — cannot run semantic search"); - return []; - } - - // GIT-93: same defect as scarSearch below — this built `${prefix}_semantic_search`, - // which exists under no prefix. The deployed function is match_
, and it - // takes similarity_threshold (unlike the _weighted variant), which is what the - // body already sends. - const rpcName = `match_${getTableName("learnings")}`; - const url = `${SUPABASE_URL}/rest/v1/rpc/${rpcName}`; - - const response = await fetch(url, { - method: "POST", - headers: { - "Content-Type": "application/json", - apikey: SUPABASE_KEY, - Authorization: `Bearer ${SUPABASE_KEY}`, - }, - body: JSON.stringify({ - query_embedding: `[${embedding.join(",")}]`, - match_count, - similarity_threshold: 0.0, - }), - signal: AbortSignal.timeout(15_000), - }); - - if (!response.ok) { - const text = await response.text(); - throw new Error(`Supabase RPC error: ${response.status} - ${text.slice(0, 200)}`); - } - - const rows = (await response.json()) as T[]; - return rows || []; -} - // ============================================================================ // DIRECT SUPABASE QUERIES // ============================================================================ @@ -473,11 +418,22 @@ export async function directUpsert( * @param filters PostgREST filter to identify the row(s) to update * @param data Fields to update (partial — omitted columns stay unchanged) */ +/** + * GIT-119: what a PATCH changed. PostgREST answers 200 with [] when the filter + * matched nothing (a wrong id, a row not created yet, RLS), so a PATCH that + * returns is not evidence of a write: `count` is. All callers treat 0 as not + * durable. + */ +export interface PatchResult { + count: number; + rows: T[]; +} + export async function directPatch( table: string, filters: Record, data: Record -): Promise { +): Promise> { if (!isConfigured()) { throw new Error("Supabase not configured"); } @@ -507,7 +463,8 @@ export async function directPatch( throw new Error(`Supabase patch error: ${response.status} - ${text.slice(0, 200)}`); } - return response.json() as Promise; + const rows = ((await response.json()) as T[]) || []; + return { count: Array.isArray(rows) ? rows.length : 0, rows: Array.isArray(rows) ? rows : [] }; } /** @@ -677,49 +634,107 @@ export async function scarSearch( return []; } - // GIT-93: the RPC is named after the TABLE it searches, not after the prefix - // with a verb appended. This built `${prefix}_scar_search` — "orchestra_scar_search" - // under GITMEM_TABLE_PREFIX=orchestra_, and "gitmem_scar_search" by default. - // Neither exists: PostgREST exposes match_orchestra_learnings_weighted, and a - // survey of the deployed functions found no *_scar_search under any prefix. So - // this fallback returned PGRST202 on every call, on every deployment, since it - // was written — invisible because it is only reached while the local vector - // index is cold. - // - // _weighted is the right one of the pair: this function is documented as scar - // search WITH SEVERITY WEIGHTING, and it is the variant that returns - // decay_multiplier, which recall consumes. Note it takes match_threshold, not - // similarity_threshold — the unweighted variant takes the latter. - const rpcName = `match_${getTableName("learnings")}_weighted`; - const url = `${SUPABASE_URL}/rest/v1/rpc/${rpcName}`; + const rows = await callScarSearchRpc(embedding, matchCount); + return (await withLearningColumns(rows)) as T[]; +} - const response = await fetch(url, { - method: "POST", - headers: { - "Content-Type": "application/json", - apikey: SUPABASE_KEY, - Authorization: `Bearer ${SUPABASE_KEY}`, - }, - body: JSON.stringify({ - query_embedding: `[${embedding.join(",")}]`, - match_count: matchCount, - match_threshold: 0.0, - // project_filter is deliberately not sent. The primary path this falls back - // from is the unified CROSS-PROJECT vector cache, so filtering here would - // make the fallback return a different, narrower result set than the path - // it stands in for — a silent behaviour change on exactly the cold-start - // calls that are hardest to notice. - }), - signal: AbortSignal.timeout(15_000), - }); +/** + * GIT-114: which scar-search function this store has. + * + * setup.sql (every version a customer can have) defines gitmem_scar_search, + * taking similarity_threshold. It does NOT define match_
_weighted — + * GIT-93 renamed the call to that after surveying nTEG's store, which has it, + * so every customer's cold-index recall got PGRST202. + * + * Order of preference: + * - default prefix: gitmem_scar_search (setup.sql), then + * match_gitmem_learnings_weighted for a store that predates or lacks it; + * - any other prefix: only match_
_weighted — gitmem_scar_search reads + * gitmem_learnings by name and would search the wrong table. + * The first function that exists is remembered for the process. + */ +interface ScarSearchRpc { + name: string; + thresholdParam: "similarity_threshold" | "match_threshold"; +} + +let resolvedScarSearchRpc: ScarSearchRpc | null = null; + +/** For tests. */ +export function resetScarSearchRpc(): void { + resolvedScarSearchRpc = null; +} + +function scarSearchCandidates(): ScarSearchRpc[] { + const weighted: ScarSearchRpc = { name: `match_${getTableName("learnings")}_weighted`, thresholdParam: "match_threshold" }; + return getTablePrefix() === "gitmem_" + ? [{ name: "gitmem_scar_search", thresholdParam: "similarity_threshold" }, weighted] + : [weighted]; +} + +async function callScarSearchRpc(embedding: number[], matchCount: number): Promise { + const candidates = resolvedScarSearchRpc ? [resolvedScarSearchRpc] : scarSearchCandidates(); + let lastError = ""; + for (const rpc of candidates) { + const response = await fetch(`${SUPABASE_URL}/rest/v1/rpc/${rpc.name}`, { + method: "POST", + headers: { + "Content-Type": "application/json", + apikey: SUPABASE_KEY, + Authorization: `Bearer ${SUPABASE_KEY}`, + }, + body: JSON.stringify({ + query_embedding: `[${embedding.join(",")}]`, + match_count: matchCount, + [rpc.thresholdParam]: 0.0, + // project_filter is deliberately not sent. The primary path this falls + // back from is the unified CROSS-PROJECT vector cache, so filtering here + // would make the fallback return a narrower set than the path it stands + // in for — a silent behaviour change on the cold-start calls. + }), + signal: AbortSignal.timeout(15_000), + }); + + if (response.ok) { + resolvedScarSearchRpc = rpc; + const rows = (await response.json()) as T[]; + return rows || []; + } - if (!response.ok) { const text = await response.text(); - throw new Error(`Supabase RPC error: ${response.status} - ${text.slice(0, 200)}`); + lastError = `Supabase RPC error: ${response.status} - ${text.slice(0, 200)}`; + // PGRST202: no such function (with these parameters) — try the next name. + // Anything else is a real failure of a function that exists. + if (!(response.status === 404 && text.includes("PGRST202"))) break; + console.error(`[scar-search] ${rpc.name} not on this store (PGRST202)`); } + throw new Error(lastError); +} - const rows = (await response.json()) as T[]; - return rows || []; +/** + * gitmem_scar_search returns id, title, description, severity, scar_type, + * counter_arguments, decay_multiplier and similarity — not the fields recall + * renders from (learning_type, applies_when, why_this_matters, …). Fetch them + * for the returned ids in one request. If that request fails the rows are + * still returned: recall renders what it has rather than losing the result. + */ +const RECALL_EXTRA_COLUMNS = + "id,learning_type,problem_context,solution_approach,applies_when,domain,keywords,source_linear_issue,why_this_matters,action_protocol,self_check_criteria,project"; + +async function withLearningColumns(rows: T[]): Promise { + if (rows.length === 0 || rows.every((r) => r.learning_type !== undefined)) return rows; + try { + const extra = await directQuery & { id: string }>(getTableName("learnings"), { + select: RECALL_EXTRA_COLUMNS, + filters: { id: `in.(${rows.map((r) => r.id).join(",")})` }, + }); + const byId = new Map(extra.map((e) => [e.id, e])); + return rows.map((r) => ({ ...(byId.get(r.id) ?? {}), ...r })); + } catch (error) { + console.error("[scar-search] Could not fetch recall columns for the matched scars (returning without them):", + error instanceof Error ? error.message : error); + return rows; + } } /** @@ -888,10 +903,12 @@ export async function saveTranscript( // Update the session record with transcript_path (direct REST API) let patch_warning: string | undefined; try { - await directPatch(getTableName("sessions"), + const patched = await directPatch(getTableName("sessions"), { id: sessionId }, { transcript_path: path } ); + // GIT-119: 0 rows = no session row to update, not a success. + if (patched.count === 0) throw new Error(`no session row ${sessionId} to update`); } catch (error) { // File is saved; session record update failed — warn, don't fail const msg = error instanceof Error ? error.message : String(error); diff --git a/src/services/thread-supabase.ts b/src/services/thread-supabase.ts index 1494521..3eb723c 100644 --- a/src/services/thread-supabase.ts +++ b/src/services/thread-supabase.ts @@ -205,19 +205,24 @@ export async function createThreadInSupabase( } try { - const row = threadObjectToRow(thread, project, embedding); - const result = await supabase.directUpsert( - getTableName("threads"), - row - ); - console.error(`[thread-supabase] Created thread ${thread.id} in Supabase`); - return result; + return await writeThreadRow(thread, project, embedding); } catch (error) { console.error("[thread-supabase] Failed to create thread:", error instanceof Error ? error.message : error); return null; } } +/** + * GIT-117: create a thread row, or throw. directUpsert asks for the stored + * representation and throws on an empty one, so a return is a confirmed row. + */ +async function writeThreadRow(thread: ThreadObject, project: Project, embedding?: string | null): Promise { + const row = threadObjectToRow(thread, project, embedding); + const result = await supabase.directUpsert(getTableName("threads"), row); + console.error(`[thread-supabase] Created thread ${thread.id} in Supabase`); + return result; +} + /** * Resolve a thread in Supabase by thread_id. * Updates status, resolved_at, resolution_note, resolved_by_session. @@ -236,33 +241,7 @@ export async function resolveThreadInSupabase( } try { - // First, find the UUID primary key for this thread_id - const rows = await supabase.directQuery(getTableName("threads"), { - select: "id,thread_id", - filters: { thread_id: threadId }, - limit: 1, - }); - - if (rows.length === 0) { - console.error(`[thread-supabase] Thread ${threadId} not found in Supabase (will proceed with local-only)`); - return false; - } - - const uuid = rows[0].id; - const patchData: Record = { - status: "resolved", - resolved_at: options.resolvedAt || new Date().toISOString(), - }; - - if (options.resolutionNote) { - patchData.resolution_note = options.resolutionNote; - } - if (options.resolvedBySession) { - patchData.resolved_by_session = options.resolvedBySession; - } - - await supabase.directPatch(getTableName("threads"), { id: uuid }, patchData); - console.error(`[thread-supabase] Resolved thread ${threadId} in Supabase`); + await resolveThreadRow(threadId, options); return true; } catch (error) { console.error("[thread-supabase] Failed to resolve thread:", error instanceof Error ? error.message : error); @@ -270,6 +249,34 @@ export async function resolveThreadInSupabase( } } +/** + * GIT-117: resolve a thread row, or throw. Confirmed only when the PATCH + * returns the row: an empty result means nothing was updated. + */ +async function resolveThreadRow( + threadId: string, + options: { resolvedAt?: string; resolutionNote?: string; resolvedBySession?: string } +): Promise { + const rows = await supabase.directQuery(getTableName("threads"), { + select: "id,thread_id", + filters: { thread_id: threadId }, + limit: 1, + }); + if (rows.length === 0) throw new Error(`thread ${threadId} not found in the store`); + + const patchData: Record = { + status: "resolved", + resolved_at: options.resolvedAt || new Date().toISOString(), + }; + if (options.resolutionNote) patchData.resolution_note = options.resolutionNote; + if (options.resolvedBySession) patchData.resolved_by_session = options.resolvedBySession; + + const patched = await supabase.directPatch(getTableName("threads"), { id: rows[0].id }, patchData); + // GIT-119: directPatch reports the rows it updated; 0 = not resolved. + if (patched.count === 0) throw new Error(`resolve of ${threadId} updated no row`); + console.error(`[thread-supabase] Resolved thread ${threadId} in Supabase`); +} + /** * Fetch a single thread from Supabase by its thread_id ("t-XXXX"). * @@ -508,9 +515,10 @@ export async function loadActiveThreadsFromSupabase( */ export async function touchThreadsInSupabase( threadIds: string[] -): Promise { +): Promise<{ touched: string[]; failed: { id: string; error: string }[] }> { + const outcome = { touched: [] as string[], failed: [] as { id: string; error: string }[] }; if (!hasSupabase() || !supabase.isConfigured() || threadIds.length === 0) { - return; + return outcome; } for (const threadId of threadIds) { @@ -522,12 +530,17 @@ export async function touchThreadsInSupabase( limit: 1, }); - if (rows.length === 0) continue; + // GIT-117: a thread we believe exists but the store does not have is a + // failed write, not a no-op. + if (rows.length === 0) throw new Error(`thread ${threadId} not found in the store`); const row = rows[0]; // Skip resolved/archived threads — no point recomputing vitality - if (row.status === "resolved" || row.status === "archived") continue; + if (row.status === "resolved" || row.status === "archived") { + outcome.touched.push(threadId); + continue; + } const now = new Date(); const newTouchCount = (row.touch_count || 0) + 1; @@ -551,18 +564,24 @@ export async function touchThreadsInSupabase( delete metadata.dormant_since; } - await supabase.directPatch(getTableName("threads"), { id: row.id }, { + const patched = await supabase.directPatch(getTableName("threads"), { id: row.id }, { touch_count: newTouchCount, last_touched_at: nowIso, vitality_score: vitality.vitality_score, status: lifecycle_status, metadata, }); + // GIT-119: 0 rows = not touched. + if (patched.count === 0) throw new Error(`touch of ${threadId} updated no row`); + outcome.touched.push(threadId); } catch (error) { - console.error(`[thread-supabase] Failed to touch thread ${threadId}:`, error instanceof Error ? error.message : error); + const message = error instanceof Error ? error.message : String(error); + console.error(`[thread-supabase] Failed to touch thread ${threadId}:`, message); + outcome.failed.push({ id: threadId, error: message }); // Continue with other threads } } + return outcome; } /** @@ -665,13 +684,18 @@ export async function syncThreadsToSupabase( const normalizedNewText = normalizeText(thread.text || ""); const matchedThreadId = normalizedNewText ? textToExistingId.get(normalizedNewText) : undefined; + // GIT-117: every write below throws unless the store confirmed a row. + // The helpers this loop called before swallowed their failures and + // returned null / false / void, so a thread was counted as synced — + // and pruned from threads.json — when nothing had been written. if (matchedThreadId) { // Duplicate text found — touch existing instead of creating console.error(`[thread-supabase] Dedup: "${thread.id}" matches existing "${matchedThreadId}" by text — touching instead of creating`); - await touchThreadsInSupabase([matchedThreadId]); + const touched = await touchThreadsInSupabase([matchedThreadId]); + if (touched.failed.length > 0) throw new Error(touched.failed[0].error); } else { // Genuinely new thread — create it - await createThreadInSupabase(thread, project); + await writeThreadRow(thread, project); // Register in lookup so subsequent threads in this batch also dedup if (normalizedNewText) { textToExistingId.set(normalizedNewText, thread.id); @@ -679,14 +703,15 @@ export async function syncThreadsToSupabase( } } else if (thread.status === "resolved" && existing[0].status !== "resolved") { // Thread was resolved during this session - await resolveThreadInSupabase(thread.id, { + await resolveThreadRow(thread.id, { resolvedAt: thread.resolved_at, resolutionNote: thread.resolution_note, resolvedBySession: thread.resolved_by_session || sessionId, }); } else { // Existing thread, just touch it - await touchThreadsInSupabase([thread.id]); + const touched = await touchThreadsInSupabase([thread.id]); + if (touched.failed.length > 0) throw new Error(touched.failed[0].error); } synced.push(thread.id); } catch (error) { @@ -752,10 +777,12 @@ export async function archiveDormantThreads( const daysDormant = (now.getTime() - dormantStart.getTime()) / (1000 * 60 * 60 * 24); if (daysDormant >= dormantDays) { - await supabase.directPatch(getTableName("threads"), { id: row.id }, { + const patched = await supabase.directPatch(getTableName("threads"), { id: row.id }, { status: "archived", }); - archived_ids.push(row.thread_id); + // GIT-119: count only rows the store actually archived. + if (patched.count > 0) archived_ids.push(row.thread_id); + else console.error(`[thread-supabase] Archive of ${row.thread_id} updated no row`); } } diff --git a/src/tools/archive-learning.ts b/src/tools/archive-learning.ts index eeb020e..cb4dd3a 100644 --- a/src/tools/archive-learning.ts +++ b/src/tools/archive-learning.ts @@ -14,6 +14,7 @@ import { supportedColumns } from "../services/store-columns.js"; import { hasSupabase, getTableName } from "../services/tier.js"; import { getStorage } from "../services/storage.js"; import { flushCache } from "../services/startup.js"; +import { getLocalVectorSearch } from "../services/local-vector-search.js"; import { Timer } from "../services/metrics.js"; import { wrapDisplay } from "../services/display-protocol.js"; import { writeResult, notStored } from "../services/write-result.js"; @@ -37,6 +38,25 @@ export interface ArchiveLearningResult extends WriteResult { } const FULL_UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** + * GIT-119: the UUIDs a hex prefix covers, as an inclusive range. uuid values + * order bytewise, i.e. by their hex digits, so prefix+"000…" .. prefix+"fff…" + * is exactly the set that starts with the prefix. + */ +export function uuidRangeForPrefix(prefix: string): [string, string] { + const fmt = (hex: string) => `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}`; + return [fmt(prefix.padEnd(32, "0")), fmt(prefix.padEnd(32, "f"))]; +} + +/** Ids in the warm local vector index (empty when it is not loaded). */ +function localIndexIds(): string[] { + try { + return getLocalVectorSearch().getScarIds(); + } catch { + return []; + } +} const HEX_PREFIX_RE = /^[0-9a-f]{4,32}$/i; /** @@ -58,10 +78,19 @@ async function resolveIdPrefix(input: string): Promise<{ id: string } | { error: const prefix = input.toLowerCase(); if (hasSupabase() && isConfigured()) { - // Supabase: use PostgREST like filter + // GIT-119: this sent id=like.% — Postgres has no LIKE on a UUID + // column (42883 "operator does not exist: uuid ~~ unknown"), so prefix + // archiving failed on every store. Resolve from the local index when it + // has the answer, else ask the store for the UUID range the prefix spans. + const local = localIndexIds().filter((id) => id.replace(/-/g, "").startsWith(prefix)); + if (local.length === 1) return { id: local[0] }; + if (local.length > 1) { + return { error: `Ambiguous prefix "${prefix}" — matches multiple learnings: ${local.slice(0, 2).map((m) => m.slice(0, 12) + "…").join(", ")}` }; + } + const [low, high] = uuidRangeForPrefix(prefix); const matches = await directQuery<{ id: string }>(getTableName("learnings"), { select: "id", - filters: { id: `like.${prefix}%` }, + filters: { and: `(id.gte.${low},id.lte.${high})` }, limit: 2, }); @@ -130,10 +159,24 @@ export async function archiveLearning(params: ArchiveLearningParams): Promise { - console.warn("[create_learning] Cache invalidation failed (non-fatal):", err); - }); + try { + const refreshed = await refreshIndexAfterWrite(project); + if (!refreshed.success) indexNotRefreshed = refreshed.error ?? "reload failed"; + } catch (err) { + indexNotRefreshed = err instanceof Error ? err.message : String(err); + } } else { // Free tier: Store locally without embedding console.error(`[create_learning] Storing locally: ${learningId}`); @@ -319,7 +328,13 @@ export async function createLearning( learning_id: learningId, embedding_generated: embeddingGenerated, performance: perfData, - display: wrapDisplay(`${te} Created ${params.learning_type}: "${params.title}"\n${se}ID: ${learningId}`), + ...(indexNotRefreshed !== null && { index_refreshed: false }), + display: wrapDisplay( + `${te} Created ${params.learning_type}: "${params.title}"\n${se}ID: ${learningId}` + + (indexNotRefreshed !== null + ? `\nRecall index NOT refreshed (${indexNotRefreshed}). The learning is saved; recall will find it after the next successful reload (retrying in the background).` + : "") + ), }; } catch (error) { const errorMsg = error instanceof Error ? error.message : String(error); diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index f398efc..ffe352a 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -16,7 +16,7 @@ import { getStorage } from "../services/storage.js"; import { filterToStoreSessionColumns } from "../services/session-columns.js"; import { storeHasTable } from "../services/store-columns.js"; import { clearCurrentSession, resolveCurrentSession, getSurfacedScars, getConfirmations, getReflections, getObservations, getChildren, getThreads, getSessionActivity, isRecallCalled } from "../services/session-state.js"; -import { normalizeThreads, mergeThreadStates, migrateStringThread, saveThreadsFile } from "../services/thread-manager.js"; // +import { normalizeThreads, mergeThreadStates, migrateStringThread, saveThreadsFile, loadThreadsFile } from "../services/thread-manager.js"; // import { deduplicateThreadList } from "../services/thread-dedup.js"; import { syncThreadsToSupabase, loadOpenThreadEmbeddings } from "../services/thread-supabase.js"; import type { ThreadSyncResult } from "../services/thread-supabase.js"; @@ -369,8 +369,9 @@ function formatCloseDisplay( ): string { const lines: string[] = []; - // Header: branded product line - const status = success ? STATUS.complete : STATUS.failed; + // Header: branded product line. GIT-117: the session saved but some threads + // did not — PARTIAL, not FAILED (the session is there) and not COMPLETE. + const status = success ? STATUS.complete : sessionStored ? STATUS.partial : STATUS.failed; lines.push(productLine("close", status)); // Stats line: compact one-liner with key counts @@ -486,6 +487,15 @@ export function formatWriteWarnings( return lines; } +/** + * Store a session's embedding (PATCH, not upsert — the row already exists). + * GIT-119: 0 rows = not saved; throwing lets the effect tracker record it. + */ +export async function saveSessionEmbedding(sessionId: string, embeddingJson: string): Promise { + const patched = await supabase.directPatch(getTableName("sessions"), { id: sessionId }, { embedding: embeddingJson }); + if (patched.count === 0) throw new Error(`session ${sessionId.slice(0, 8)}: embedding PATCH updated no row`); +} + /** * GIT-21: Clean up all session files for a closed session. * Unregisters from registry, deletes per-session directory, and removes legacy file. @@ -1445,8 +1455,19 @@ export async function sessionClose( `[session_close] Local thread prune SKIPPED — ${threadSync.failed.length} thread(s) did not reach Supabase (${threadSync.failed.map((f) => f.id).join(", ")}). ` + `threads.json retained in full; it is the only record of the unsynced threads.` ); + // GIT-117: mark what did not land so the next session_start keeps it + // (whatever its age) and the next close writes it again. + const failedIds = new Set(threadSync.failed.map((f) => f.id)); + const fileThreads = loadThreadsFile(); + const byId = new Map(fileThreads.map((t) => [t.id, t])); + for (const t of closeThreads) { + if (failedIds.has(t.id)) byId.set(t.id, { ...(byId.get(t.id) ?? {}), ...t, sync_pending: true }); + } + saveThreadsFile([...byId.values()]); } else { - const openThreadsOnly = closeThreads.filter(t => t.status === "open" || !t.status); + const openThreadsOnly = closeThreads + .filter(t => t.status === "open" || !t.status) + .map(({ sync_pending: _written, ...t }) => t); // GIT-117: written now saveThreadsFile(openThreadsOnly); console.error(`[session_close] Pruned threads.json: ${openThreadsOnly.length} open threads (removed ${closeThreads.length - openThreadsOnly.length} resolved/archived)`); } @@ -1566,11 +1587,7 @@ export async function sessionClose( const embeddingVector = await embed(embeddingText); if (embeddingVector) { const embeddingJson = JSON.stringify(embeddingVector); - // Update session with embedding (PATCH, not upsert — row already exists) - await supabase.directPatch(getTableName("sessions"), - { id: sessionId }, - { embedding: embeddingJson } - ); + await saveSessionEmbedding(sessionId, embeddingJson); console.error("[session_close] Embedding saved to session"); // Phase 5: Implicit thread detection (chained after embedding) @@ -1702,8 +1719,8 @@ export async function sessionClose( const persistErrors = partialPersist ? [ - `Thread sync incomplete: ${threadSync.failed.length} of ${threadSync.attempted} thread(s) did not reach Supabase — ${threadSync.failed.map((f) => `${f.id} (${f.error})`).join("; ")}. ` + - `threads.json was NOT pruned and session files were retained; re-run session_close to retry.`, + `PARTIAL: the session saved, but ${threadSync.failed.length} of ${threadSync.attempted} thread(s) did not reach Supabase — ${threadSync.failed.map((f) => `${f.id} (${f.error})`).join("; ")}. ` + + `They are kept in threads.json and carried into the next session, whose close writes them again; session files were retained.`, ] : []; diff --git a/src/tools/session-start.ts b/src/tools/session-start.ts index b177c3c..a167a6c 100644 --- a/src/tools/session-start.ts +++ b/src/tools/session-start.ts @@ -478,7 +478,8 @@ async function createSessionRecord( * Fire-and-forget — failures logged but don't block session_start. * Only sets close_compliance if it's currently null (truly abandoned). */ -async function markSessionSuperseded(oldSessionId: string, newSessionId: string): Promise { +/** @internal exported for the GIT-119 zero-row test. */ +export async function markSessionSuperseded(oldSessionId: string, newSessionId: string): Promise { if (!hasSupabase()) return; // Free tier: no remote session tracking try { // Check if session already has close_compliance (was properly closed) @@ -490,7 +491,7 @@ async function markSessionSuperseded(oldSessionId: string, newSessionId: string) // Already closed — don't overwrite return; } - await supabase.directPatch(getTableName("sessions"), + const patched = await supabase.directPatch(getTableName("sessions"), { id: oldSessionId }, { close_compliance: { @@ -500,6 +501,8 @@ async function markSessionSuperseded(oldSessionId: string, newSessionId: string) }, } ); + // GIT-119: 0 rows = the superseded row isn't there; not marked. + if (patched.count === 0) throw new Error(`no session row ${oldSessionId.slice(0, 8)} to mark superseded`); console.error(`[session_start] Marked session ${oldSessionId.slice(0, 8)} as superseded by ${newSessionId.slice(0, 8)}`); } catch (error) { console.error(`[session_start] Failed to mark session ${oldSessionId.slice(0, 8)} as superseded:`, error); @@ -870,9 +873,17 @@ function writeSessionFiles( // because the NOT-IN query excludes them, making them look "local-only". const supabaseIds = new Set(threads.map(t => t.id)); const cutoff = Date.now() - 24 * 60 * 60 * 1000; + // GIT-117: a thread a close could not write is kept whatever its age, and + // its local state (e.g. a resolution) wins over the store's copy until a + // later close writes it. + const pending = new Map(existingFileThreads.filter(t => t.sync_pending && t.id).map(t => [t.id, t])); + if (pending.size > 0) { + console.error(`[session_start] Carrying forward ${pending.size} thread(s) a previous close could not write: ${[...pending.keys()].join(", ")}`); + } const localOnlyThreads = existingFileThreads.filter(t => { if (supabaseIds.has(t.id)) return false; // exists in Supabase active set if (!t.id) return false; // no ID = malformed, drop + if (t.sync_pending) return true; const created = t.created_at ? new Date(t.created_at).getTime() : 0; return created > cutoff; // only keep if created within last 24h }); @@ -883,7 +894,7 @@ function writeSessionFiles( if (dropped > 0) { console.error(`[session_start] Dropped ${dropped} stale local-only threads (resolved/archived in Supabase)`); } - merged = deduplicateThreadList([...threads, ...localOnlyThreads]); + merged = deduplicateThreadList([...threads.map(t => pending.get(t.id) ?? t), ...localOnlyThreads]); } else { // Fallback (free tier / Supabase offline): merge with existing file merged = existingFileThreads.length > 0 diff --git a/src/types/index.ts b/src/types/index.ts index 6de5c9b..7f9a12a 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -86,6 +86,12 @@ export interface ThreadObject { source_session?: string; /** Session ID that resolved this thread */ resolved_by_session?: string; + /** + * GIT-117: a close could not write this thread (or its resolution) to the + * durable store. Kept locally, whatever its age, and preferred over the + * store's copy until a later close writes it. + */ + sync_pending?: boolean; /** Brief resolution note */ resolution_note?: string; } @@ -346,6 +352,8 @@ export interface CreateLearningParams { export interface CreateLearningResult extends WriteResult { learning_id: string; embedding_generated: boolean; + /** GIT-118: false when the recall index could not be refreshed after the write. */ + index_refreshed?: boolean; /** Error details when success=false */ errors?: string[]; display?: string; diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 0a16761..f93f69c 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -200,7 +200,7 @@ function isolatedHome(tag) { return { home, gitmemDir }; } const STRIP = /^(OPENAI_API_KEY|OPENROUTER_API_KEY|GITMEM_|SUPABASE_|OLLAMA_|NODE_OPTIONS$|CLAUDE_CODE_ENTRYPOINT$)/; -async function startServer({ home, gitmemDir, ollamaUrl, netlog, stderrSink }) { +async function startServer({ home, gitmemDir, ollamaUrl, netlog, stderrSink, extraEnv = {} }) { const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => !STRIP.test(k))); Object.assign(env, { HOME: home, @@ -216,6 +216,7 @@ async function startServer({ home, gitmemDir, ollamaUrl, netlog, stderrSink }) { GITMEM_NETLOG: netlog, NODE_OPTIONS: `--import=${pathToFileURL(NETLOG_PRELOAD).href}`, NO_COLOR: "1", + ...extraEnv, }); const transport = new StdioClientTransport({ command: process.execPath, args: [SERVER], env, cwd: home, stderr: "pipe" }); transport.stderr?.on("data", (d) => stderrSink.push(d.toString())); @@ -400,6 +401,25 @@ async function flow() { await waitQuiet(netlog, { minMs: 2000, quietMs: 3000 }); const health = await step("health", { failure_limit: 20 }); + // GIT-119: archive_learning by id prefix sent id=like.% against a + // uuid column (42883), and a full UUID that matched no row still reported + // "Archived". The row below is inserted after the server loaded its index, + // so the prefix must be resolved by the store (UUID range), not locally. + const archiveRowId = randomUUID(); + await rest("POST", "gitmem_learnings", [{ + id: archiveRowId, learning_type: "scar", title: `Driver archive target (${label})`, + description: "Inserted after startup so archive_learning resolves its prefix from the store.", + severity: "low", counter_arguments: ["You might think it is permanent — but it is archived here."], + project: PROJECT, is_active: true, + }]); + const archivePrefix = archiveRowId.slice(0, 8); + const ap = await step("archive_learning", { id: archivePrefix, reason: "venue driver (GIT-119)" }); + const archivedRow = await rest("GET", `gitmem_learnings?select=is_active&id=eq.${archiveRowId}`, undefined, { Prefer: "" }).then((r) => r.json()); + const archivePrefixDurable = /Archived learning/.test(ap) && archivedRow[0]?.is_active === false; + const ghostId = "00000000-0000-4000-8000-00000000d119"; + const ag = await step("archive_learning", { id: ghostId, reason: "venue driver (GIT-119)" }); + const archiveGhostHonest = !/Archived learning/.test(ag) && /nothing was archived|not found|No learning/i.test(ag); + // 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)); @@ -422,6 +442,131 @@ async function flow() { const relevance = relevanceRows.map((m) => ({ tool: m.tool_name, memories_applied: m.metadata.memories_applied, memory_relevance: m.metadata.memory_relevance })); await waitQuiet(netlog, { minMs: 1000, quietMs: 2000 }); await srv.close(); + + // GIT-114: recall served by the store, not the local index. (1) Forced with + // GITMEM_SEARCH_MODE=remote. (2) A fresh server asked before its index has + // loaded — the cold-start path every customer hits first. Both must return a + // seeded scar; the store must answer the RPC (a 404 fails the run below). + const recallProbe = async (tag, extraEnv) => { + const h = isolatedHome(`${label}-${tag}`); + const probeLog = join(outDir, `netlog-${label}-${tag}.jsonl`); + writeFileSync(probeLog, ""); + const errs = []; + const p = await startServer({ home: h.home, gitmemDir: h.gitmemDir, ollamaUrl: ollama.url, netlog: probeLog, stderrSink: errs, extraEnv }); + let text = ""; + try { + text = await p.call("recall", { plan: "Supabase migrations must be dry-run before push", project: PROJECT, match_count: 3 }); + } catch (e) { text = `ERROR ${e}`; } + await p.close(); + allVenueRequests.push(...venueReqs(readNet(probeLog))); + const errText = errs.join(""); + return { + found_seeded_scar: /Supabase migrations must be dry-run/.test(text), + used_store_rpc: venueReqs(readNet(probeLog)).some((r) => r.path.includes("/rpc/") && r.status < 300), + fell_back_from_local: /Local cache not ready/.test(errText), + text: text.slice(0, 300), + }; + }; + const recallRemote = await recallProbe("recall-remote", { GITMEM_SEARCH_MODE: "remote" }); + const recallCold = await recallProbe("recall-cold", {}); + // GIT-117: a close whose thread write the store rejects. A thread naming a + // session the store has never seen violates the source_session FK (23503). + // The close must say PARTIAL and name it, keep it in threads.json marked + // sync_pending, and the next session_start must carry it forward. + const threadSyncProbe = async () => { + const h = isolatedHome(`${label}-threadsync`); + const probeLog = join(outDir, `netlog-${label}-threadsync.jsonl`); + writeFileSync(probeLog, ""); + const badId = `t-fk${randomUUID().slice(0, 6)}`; + const errs1 = []; + const p1 = await startServer({ home: h.home, gitmemDir: h.gitmemDir, ollamaUrl: ollama.url, netlog: probeLog, stderrSink: errs1 }); + const ss1 = await p1.call("session_start", { project: PROJECT, agent_identity: "cli", force: true }); + const sid1 = (ss1.match(UUID) || [])[0]; + await waitQuiet(probeLog, { minMs: 1500, quietMs: 2000 }); + const close = await p1.call("session_close", { + session_id: sid1, close_type: "standard", human_corrections: "none", + open_threads: [{ id: badId, text: `Thread whose write the store rejects (${label})`, status: "open", + created_at: new Date().toISOString(), source_session: randomUUID() }], + closing_reflection: { + what_broke: "n/a", what_took_longer: "n/a", do_differently: "n/a", what_worked: "n/a", + wrong_assumption: "n/a", scars_applied: [], institutional_memory_items: "n/a", + collaborative_dynamic: "n/a", rapport_notes: "n/a", + }, + }); + await p1.close(); + const threadsFile = join(h.gitmemDir, "threads.json"); + const local = existsSync(threadsFile) ? JSON.parse(readFileSync(threadsFile, "utf8")) : []; + const localEntry = local.find((t) => t.id === badId); + const inStore = await rest("GET", `gitmem_threads?select=id&thread_id=eq.${badId}`, undefined, { Prefer: "" }).then((r) => r.json()); + const errs2 = []; + const p2 = await startServer({ home: h.home, gitmemDir: h.gitmemDir, ollamaUrl: ollama.url, netlog: probeLog, stderrSink: errs2 }); + await p2.call("session_start", { project: PROJECT, agent_identity: "cli" }); + await p2.close(); + allVenueRequests.push(...venueReqs(readNet(probeLog)).filter((r) => !(r.method === "POST" && /gitmem_threads$/.test(r.path) && r.status === 409))); + const after = existsSync(threadsFile) ? JSON.parse(readFileSync(threadsFile, "utf8")) : []; + writeFileSync(join(outDir, `stderr-${label}-threadsync-restart.log`), errs2.join("")); + writeFileSync(join(outDir, `threads-${label}-threadsync-after.json`), JSON.stringify(after, null, 2)); + const plain = close.replace(/\x1b\[[0-9;]*m/g, ""); + return { + bad_thread: badId, + store_rejected_it: inStore.length === 0, + close_says_partial: /close · PARTIAL/.test(plain) && plain.includes(badId), + kept_locally_pending: !!localEntry && localEntry.sync_pending === true, + carried_into_next_session: after.some((t) => t.id === badId) && + errs2.join("").split("\n").some((l) => l.includes("Carrying forward") && l.includes(badId)), + close_head: plain.split("\n").slice(0, 2).join(" | "), + }; + }; + const threadSync = await threadSyncProbe(); + // GIT-118: the store fails the index READS while writes still land. A + // forwarding proxy in front of the venue fails GET gitmem_learnings… on + // demand. create_learning must save and say its index refresh failed; a + // recall right after must still find the seeded scars (index kept). + const reloadFailureProbe = async () => { + let failIndexReads = false; + const proxy = createServer(async (req, res) => { + const chunks = []; + for await (const c of req) chunks.push(c); + if (failIndexReads && req.method === "GET" && /^\/rest\/v1\/gitmem_learnings(\?|$)/.test(req.url)) { + res.writeHead(503, { "content-type": "application/json" }); + res.end('{"message":"injected by the venue driver (GIT-118)"}'); + return; + } + const headers = Object.fromEntries(Object.entries(req.headers).filter(([k]) => !["host", "connection", "content-length", "accept-encoding"].includes(k))); + const upstream = await fetch(`${SUPABASE_URL}${req.url}`, { method: req.method, headers, body: chunks.length ? Buffer.concat(chunks) : undefined }); + const body = Buffer.from(await upstream.arrayBuffer()); + const out = {}; + upstream.headers.forEach((v, k) => { if (!["content-encoding", "content-length", "transfer-encoding", "connection"].includes(k)) out[k] = v; }); + res.writeHead(upstream.status, out); + res.end(body); + }); + await new Promise((r) => proxy.listen(0, "127.0.0.1", r)); + const h = isolatedHome(`${label}-reload`); + const probeLog = join(outDir, `netlog-${label}-reload.jsonl`); + writeFileSync(probeLog, ""); + const errs = []; + const p = await startServer({ home: h.home, gitmemDir: h.gitmemDir, ollamaUrl: ollama.url, netlog: probeLog, stderrSink: errs, + extraEnv: { SUPABASE_URL: `http://127.0.0.1:${proxy.address().port}`, GITMEM_SEARCH_MODE: "local" } }); + await p.call("session_start", { project: PROJECT, agent_identity: "cli", force: true }); + for (let i = 0; i < 60 && !/GitMem initialized: [1-9]\d* scars/.test(errs.join("")); i++) await new Promise((r) => setTimeout(r, 500)); + failIndexReads = true; + const created = await p.call("create_learning", { learning_type: "win", title: `Reload probe win (${label}) ${randomUUID().slice(0, 8)}`, description: "written while the index reads fail", project: PROJECT }); + // Let any background reload the write started finish before the next + // prompt's recall (1.11.0 did not await it; a user's next prompt comes later). + await waitQuiet(probeLog, { minMs: 3000, quietMs: 2500 }); + const recall = await p.call("recall", { plan: "Supabase migrations must be dry-run before push", project: PROJECT, match_count: 3 }); + failIndexReads = false; + await p.close(); + proxy.close(); + writeFileSync(join(outDir, `stderr-${label}-reload.log`), errs.join("")); + return { + learning_saved: /Created win/.test(created), + create_says_not_refreshed: /Recall index NOT refreshed/.test(created), + recall_still_finds_seeded_scar: /Supabase migrations must be dry-run/.test(recall), + create_head: created.split("\n").slice(0, 3).join(" | ").slice(0, 240), + }; + }; + const reloadFailure = await reloadFailureProbe(); ollama.close(); const reqs = readNet(netlog); @@ -441,8 +586,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, + recall_remote: recallRemote, + recall_cold: recallCold, + thread_sync: threadSync, + reload_failure: reloadFailure, missing_payload_named: missingPayloadNamed, resolve_thread_durable: resolveDurable, + archive_prefix_durable: archivePrefixDurable, + archive_prefix_text: ap.slice(0, 300), + archive_ghost_honest: archiveGhostHonest, + archive_ghost_text: ag.slice(0, 300), close_warns: closeWarns, close_warns_self_describing: closeWarnsSelfDescribing, health_has_write_path: healthHasWritePath, @@ -624,6 +777,12 @@ if (edgeCalls.length) failureCheck.unexpected.push(...edgeCalls.map((c) => `edge 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.archive_prefix_durable) { + failureCheck.unexpected.push(`archive_learning by id prefix did not archive the row (GIT-119): ${result.archive_prefix_text}`); +} +if (mode === "flow" && !result.archive_ghost_honest) { + failureCheck.unexpected.push(`archive_learning of a nonexistent id claimed success (GIT-119): ${result.archive_ghost_text}`); +} 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)"); } @@ -648,6 +807,25 @@ if (mode === "flow" && !result.thread_triples_linked) { 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.recall_remote.found_seeded_scar && result.recall_remote.used_store_rpc)) { + failureCheck.unexpected.push(`recall with GITMEM_SEARCH_MODE=remote did not return a seeded scar from the store (GIT-114): ${result.recall_remote.text}`); +} +if (mode === "flow" && !result.recall_cold.found_seeded_scar) { + failureCheck.unexpected.push(`cold-start recall returned no seeded scar (GIT-114): ${result.recall_cold.text}`); +} +if (mode === "flow") { + const t = result.thread_sync; + if (!t.store_rejected_it) failureCheck.unexpected.push(`thread-sync probe: the store accepted ${t.bad_thread}; the probe did not exercise a failed write (GIT-117)`); + else if (!(t.close_says_partial && t.kept_locally_pending && t.carried_into_next_session)) { + failureCheck.unexpected.push(`a rejected thread write was not reported PARTIAL / kept pending / carried forward (GIT-117): ${JSON.stringify(t)}`); + } +} +if (mode === "flow") { + const r = result.reload_failure; + if (!(r.learning_saved && r.create_says_not_refreshed && r.recall_still_finds_seeded_scar)) { + failureCheck.unexpected.push(`a failed index reload was hidden or emptied recall (GIT-118): ${JSON.stringify(r)}`); + } +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } diff --git a/tests/e2e/cli-fresh-install.test.ts b/tests/e2e/cli-fresh-install.test.ts index d7d6e3e..9a8e88e 100644 --- a/tests/e2e/cli-fresh-install.test.ts +++ b/tests/e2e/cli-fresh-install.test.ts @@ -17,6 +17,7 @@ import { execFile as execFileCb } from "child_process"; import { promisify } from "util"; import { mkdirSync, + mkdtempSync, rmSync, existsSync, readFileSync, @@ -38,6 +39,11 @@ import { } from "./mcp-client.js"; const execFile = promisify(execFileCb); + +// GIT-115: init writes the store to the root the server reads (~/.gitmem). +// Every CLI call here runs against a scratch HOME, never the developer's. +const SANDBOX_HOME = mkdtempSync(join(tmpdir(), "gitmem-cli-home-")); +afterAll(() => rmSync(SANDBOX_HOME, { recursive: true, force: true })); const GITMEM_BIN = join(__dirname, "../../bin/gitmem.js"); /** @@ -52,6 +58,9 @@ async function runGitmem( ): Promise<{ stdout: string; stderr: string; exitCode: number }> { const env = { ...process.env, + HOME: SANDBOX_HOME, + GITMEM_DIR: "", + GITMEM_HOME: "", // Ensure free tier unless overridden SUPABASE_URL: "", SUPABASE_SERVICE_ROLE_KEY: "", @@ -90,15 +99,16 @@ describe("Fresh Install: Free Tier CLI", () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); }); - it("gitmem init creates .gitmem/ with starter scars", async () => { + it("gitmem init creates the store in ~/.gitmem with starter scars", async () => { const { stdout, exitCode } = await runGitmem(["init", "--yes"], { cwd: TEST_DIR }); expect(exitCode).toBe(0); - expect(stdout.toLowerCase()).toContain("free"); + expect(stdout).toContain("Created ~/.gitmem"); - // .gitmem/ directory created - const gitmemDir = join(TEST_DIR, ".gitmem"); + // GIT-115: the store is where the server reads it, not /.gitmem + const gitmemDir = join(SANDBOX_HOME, ".gitmem"); expect(existsSync(gitmemDir)).toBe(true); + expect(existsSync(join(TEST_DIR, ".gitmem", "learnings.json"))).toBe(false); // learnings.json has starter scars const learningsPath = join(gitmemDir, "learnings.json"); @@ -112,20 +122,20 @@ describe("Fresh Install: Free Tier CLI", () => { expect(existsSync(join(gitmemDir, "scar-usage.json"))).toBe(true); // stdout shows scar count - const countMatch = stdout.match(/(\d+) starter scars/) || stdout.match(/(\d+) new scars added/); + const countMatch = stdout.match(/(\d+) lessons from common mistakes/); expect(countMatch).not.toBeNull(); expect(parseInt(countMatch![1])).toBeGreaterThan(0); }); it("gitmem init is idempotent (no duplicates on re-run)", async () => { // Count scars before second run - const learningsPath = join(TEST_DIR, ".gitmem", "learnings.json"); + const learningsPath = join(SANDBOX_HOME, ".gitmem", "learnings.json"); const beforeCount = JSON.parse(readFileSync(learningsPath, "utf-8")).length; const { stdout, exitCode } = await runGitmem(["init", "--yes"], { cwd: TEST_DIR }); expect(exitCode).toBe(0); - expect(stdout.toLowerCase()).toContain("already configured"); + expect(stdout.toLowerCase()).toContain("already set up"); // Same count — no duplicates const afterCount = JSON.parse(readFileSync(learningsPath, "utf-8")).length; @@ -295,9 +305,10 @@ describe("Fresh Install: Hooks CLI", () => { // Permissions preserved expect(settings.permissions.allow).toContain("mcp__gitmem__*"); expect(settings.permissions.allow).toContain("custom_tool"); - // Hooks replaced with new ones + // gitmem's hooks added; the foreign "echo old" hook kept (GIT-120) const cmd = JSON.stringify(settings.hooks.SessionStart); expect(cmd).toContain("session-start.sh"); + expect(cmd).toContain("echo old"); }); it("uninstall-hooks removes hooks from settings", async () => { @@ -308,10 +319,11 @@ describe("Fresh Install: Hooks CLI", () => { expect(exitCode).toBe(0); expect(stdout).toContain("Uninstall complete"); - // settings.json still exists but hooks key removed + // settings.json still exists; only gitmem's hooks removed, the foreign + // "echo old" hook from the previous test kept (GIT-120) expect(existsSync(SETTINGS_PATH)).toBe(true); const settings = JSON.parse(readFileSync(SETTINGS_PATH, "utf-8")); - expect(settings.hooks).toBeUndefined(); + expect(settings.hooks).toEqual({ SessionStart: [{ hooks: [{ type: "command", command: "echo old" }] }] }); // Permissions preserved expect(settings.permissions).toBeDefined(); @@ -323,7 +335,7 @@ describe("Fresh Install: Hooks CLI", () => { }); expect(exitCode).toBe(0); - expect(stdout).toContain("No hooks found"); + expect(stdout).toContain("No gitmem hooks found"); }); }); @@ -338,15 +350,16 @@ describe("Fresh Install: MCP Server Lifecycle", () => { if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); mkdirSync(TEST_DIR, { recursive: true }); - // Run init to populate .gitmem/ + // Run init to populate the store (GIT-115: ~/.gitmem, here the sandbox HOME) await runGitmem(["init", "--yes"], { cwd: TEST_DIR }); - // Start MCP server with CWD in the initialized directory - // so it walks up and finds .gitmem/ with starter scars + // Start the MCP server on the same HOME, so it reads what init wrote mcp = await createMcpClient( { ...createTierEnv("free"), - HOME: TEST_DIR, + HOME: SANDBOX_HOME, + GITMEM_DIR: "", + GITMEM_HOME: "", }, { cwd: TEST_DIR } ); @@ -556,6 +569,9 @@ describe("Fresh Install: Hook Script Output", () => { ...process.env, CLAUDE_SESSION_ID: sessionId, HOME: TEST_DIR, + // GIT-99: the hook reads the registry from the server's root, not + // its cwd; name the test's root explicitly. + GITMEM_DIR: join(sessionDir, ".gitmem"), }, timeout: 10_000, } diff --git a/tests/e2e/git-115-fresh-install.test.ts b/tests/e2e/git-115-fresh-install.test.ts new file mode 100644 index 0000000..429c80d --- /dev/null +++ b/tests/e2e/git-115-fresh-install.test.ts @@ -0,0 +1,156 @@ +/** + * GIT-115: a fresh install reads what init wrote. + * + * init seeded /.gitmem while the server reads ~/.gitmem (GIT-91), so a + * new user's first session had no starter lessons and was told its store was + * "NOT being read". Now the store goes to the root the server reads, and the + * repo keeps only config.json (the project name) and the hook scripts. + * + * Clean HOME, clean repo, the real CLI, the real SessionStart hook and the + * built server. No Supabase, no Docker: safe for CI. + */ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { execFile as execFileCb } from "child_process"; +import { promisify } from "util"; +import { mkdtempSync, mkdirSync, rmSync, readFileSync, writeFileSync, readdirSync, existsSync, realpathSync } from "fs"; +import { join } from "path"; +import { tmpdir } from "os"; +import { createMcpClient, callTool, getToolResultText, isToolError, type McpTestClient } from "./mcp-client.js"; + +const execFile = promisify(execFileCb); +const GITMEM_BIN = join(__dirname, "../../bin/gitmem.js"); +const STARTER_COUNT = JSON.parse(readFileSync(join(__dirname, "../../schema/starter-scars.json"), "utf-8")).length; +const PROJECT = "acme-git115"; + +let home: string; +let repo: string; +let mcp: McpTestClient | undefined; + +/** The environment of a new user: nothing gitmem-specific set. */ +function cleanEnv(): Record { + const env: Record = {}; + for (const [k, v] of Object.entries(process.env)) { + if (v !== undefined && !/^(GITMEM_|SUPABASE_|OPENAI_|OPENROUTER_)/.test(k)) env[k] = v; + } + // GITMEM_DIR/GITMEM_HOME blank as well: createMcpClient layers process.env underneath. + return { ...env, HOME: home, GITMEM_DIR: "", GITMEM_HOME: "", NO_COLOR: "1", GITMEM_TIER: "free", SUPABASE_URL: "", SUPABASE_SERVICE_ROLE_KEY: "" }; +} + +async function run(cmd: string, args: string[], input?: string) { + const p = execFile(cmd, args, { cwd: repo, env: cleanEnv(), timeout: 60_000 }); + if (input !== undefined) { p.child.stdin?.end(input); } + try { + const { stdout, stderr } = await p; + return { stdout, stderr, code: 0 }; + } catch (e: any) { + return { stdout: e.stdout ?? "", stderr: e.stderr ?? "", code: typeof e.code === "number" ? e.code : 1 }; + } +} + +let initOut = ""; +let hookOut = ""; +let startText = ""; +let recallText = ""; + +beforeAll(async () => { + home = realpathSync(mkdtempSync(join(tmpdir(), "gitmem-git115-home-"))); + repo = realpathSync(mkdtempSync(join(tmpdir(), "gitmem-git115-repo-"))); + + const init = await run("node", [GITMEM_BIN, "init", "--yes", "--project", PROJECT, "--client", "claude"]); + initOut = init.stdout + init.stderr; + + // The SessionStart hook, as Claude Code runs it: from the repo, JSON on stdin. + hookOut = (await run("bash", [".gitmem/hooks/session-start.sh"], "{}")).stdout; + + // The project the hook tells the agent to use, straight from the repo config. + const hinted = (hookOut.match(/session_start\(project: "([^"]+)"\)/) || [])[1]; + + mcp = await createMcpClient(cleanEnv(), { cwd: repo }); + const result = await callTool(mcp.client, "session_start", { + agent_identity: "cli", force: true, ...(hinted ? { project: hinted } : {}), + }); + startText = isToolError(result) ? `ERROR ${getToolResultText(result)}` : getToolResultText(result); + const rc = await callTool(mcp.client, "recall", { plan: "run the database migration without a rollback plan", project: PROJECT }); + recallText = isToolError(rc) ? `ERROR ${getToolResultText(rc)}` : getToolResultText(rc); +}, 120_000); + +afterAll(async () => { + if (mcp) await mcp.cleanup(); + rmSync(home, { recursive: true, force: true }); + rmSync(repo, { recursive: true, force: true }); +}); + +describe("GIT-115: fresh install, clean HOME", () => { + it("init seeds the starter lessons into the root the server reads", () => { + const learnings = JSON.parse(readFileSync(join(home, ".gitmem", "learnings.json"), "utf-8")); + expect(learnings).toHaveLength(STARTER_COUNT); + expect(existsSync(join(home, ".gitmem", "threads.json"))).toBe(true); + expect(existsSync(join(home, ".gitmem", "closing-payload-template.json"))).toBe(true); + expect(initOut).toContain("~/.gitmem"); + }); + + it("the repo keeps only config.json (with the project) and the hooks", () => { + expect(readdirSync(join(repo, ".gitmem")).sort()).toEqual(["config.json", "hooks"]); + expect(JSON.parse(readFileSync(join(repo, ".gitmem", "config.json"), "utf-8"))).toEqual({ project: PROJECT }); + }); + + it("the SessionStart hook names the project from the repo config", () => { + expect(hookOut).toContain(`session_start(project: "${PROJECT}")`); + }); + + it("session_start reads what init seeded, under the repo's project", () => { + expect(startText).not.toMatch(/^ERROR/); + expect(startText).toContain(`· ${PROJECT}`); + // The welcome thread init wrote into the store. + expect(startText).toContain("Add your first project-specific scar"); + }); + + it("recall in that session returns a starter scar", () => { + expect(recallText).not.toMatch(/^ERROR/); + expect(recallText).toContain("Database Migration Without Rollback Plan"); + }); + + it("session_start does not report the repo's .gitmem as a store NOT being read", () => { + expect(startText).not.toContain("NOT being read"); + }); +}); + +describe("GIT-115: uninstall follows the same split", () => { + it("init installs its hooks beside a foreign hook under a gitmem-named path (GIT-120 matching)", async () => { + // The wizard's old substring match took this hook for gitmem's own and + // skipped installing gitmem's hooks altogether. + const other = realpathSync(mkdtempSync(join(tmpdir(), "gitmem-git115-repo2-"))); + try { + const foreign = { hooks: { PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "bash ~/code/gitmem-notes/lint.sh" }] }] } }; + mkdirSync(join(other, ".claude")); + writeFileSync(join(other, ".claude", "settings.json"), JSON.stringify(foreign, null, 2)); + const r = await execFile("node", [GITMEM_BIN, "init", "--yes", "--client", "claude"], { cwd: other, env: cleanEnv(), timeout: 60_000 }); + expect(r.stdout).toContain("Added automatic memory hooks"); + const text = readFileSync(join(other, ".claude", "settings.json"), "utf-8"); + expect(text).toContain("gitmem-notes/lint.sh"); + expect(text).toContain(".gitmem/hooks/session-start.sh"); + } finally { + rmSync(other, { recursive: true, force: true }); + } + }); + + it("uninstall removes the repo's config and hooks, keeps foreign hooks, and keeps the shared store", async () => { + const settingsPath = join(repo, ".claude", "settings.json"); + const settings = JSON.parse(readFileSync(settingsPath, "utf-8")); + settings.hooks.PreToolUse.find((g: any) => g.matcher === "Bash").hooks.push({ type: "command", command: "bash ~/code/gitmem-notes/lint.sh" }); + writeFileSync(settingsPath, JSON.stringify(settings, null, 2)); + const r = await run("node", [GITMEM_BIN, "uninstall", "--yes", "--client", "claude"]); + expect(r.code).toBe(0); + expect(existsSync(join(repo, ".gitmem"))).toBe(false); + const hooks = JSON.parse(readFileSync(join(repo, ".claude", "settings.json"), "utf-8")).hooks; + expect(JSON.stringify(hooks)).toContain("gitmem-notes/lint.sh"); + expect(JSON.stringify(hooks)).not.toContain(".gitmem/hooks/"); + expect(existsSync(join(home, ".gitmem", "learnings.json"))).toBe(true); + }); + + it("uninstall --all deletes the store", async () => { + const r = await run("node", [GITMEM_BIN, "uninstall", "--yes", "--all", "--client", "claude"]); + expect(r.code).toBe(0); + expect(existsSync(join(home, ".gitmem"))).toBe(false); + }); +}); diff --git a/tests/e2e/git-120-hooks-merge.test.ts b/tests/e2e/git-120-hooks-merge.test.ts new file mode 100644 index 0000000..4cc995d --- /dev/null +++ b/tests/e2e/git-120-hooks-merge.test.ts @@ -0,0 +1,187 @@ +/** + * GIT-120: install-hooks and uninstall-hooks keep other people's hooks. + * + * install-hooks replaced settings.hooks wholesale and uninstall-hooks deleted + * the key, so a user's own hooks (or another tool's) vanished. These run the + * real CLI in a scratch repo that already has foreign hooks, and check that + * only gitmem's commands come and go, that the file is backed up first, and + * that the output says what changed. + * + * No server, no Supabase, no Docker: safe for CI. + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { execFile as execFileCb } from "child_process"; +import { promisify } from "util"; +import { mkdtempSync, rmSync, readFileSync, writeFileSync, mkdirSync, readdirSync } from "fs"; +import { join } from "path"; +import { tmpdir } from "os"; + +const execFile = promisify(execFileCb); +const GITMEM_BIN = join(__dirname, "../../bin/gitmem.js"); + +async function gitmem(args: string[], cwd: string, home: string) { + try { + const { stdout, stderr } = await execFile("node", [GITMEM_BIN, ...args], { + cwd, + env: { ...process.env, HOME: home, NO_COLOR: "1", SUPABASE_URL: "", SUPABASE_SERVICE_ROLE_KEY: "", GITMEM_TIER: "free" }, + timeout: 30_000, + }); + return { stdout, stderr, code: 0 }; + } catch (e: any) { + return { stdout: e.stdout ?? "", stderr: e.stderr ?? "", code: typeof e.code === "number" ? e.code : 1 }; + } +} + +/** A user's own hooks, including one whose path happens to contain "gitmem". */ +const FOREIGN_CLAUDE = { + SessionStart: [{ hooks: [{ type: "command", command: "echo foreign-session-start" }] }], + PreToolUse: [ + { matcher: "Bash", hooks: [{ type: "command", command: "bash ~/code/gitmem-notes/lint.sh" }] }, + ], + Notification: [{ hooks: [{ type: "command", command: "say done" }] }], +}; + +const allCommands = (hooks: Record = {}) => + Object.values(hooks).flat().flatMap((e: any) => (Array.isArray(e.hooks) ? e.hooks : [e])).map((h: any) => h.command as string); +const gitmemCommands = (hooks?: Record) => allCommands(hooks).filter((c) => c.includes(".gitmem/hooks/")); +const foreignCommands = (hooks?: Record) => allCommands(hooks).filter((c) => !c.includes(".gitmem/hooks/")); +const backups = (dir: string, base: string) => readdirSync(dir).filter((f) => f.startsWith(`${base}.gitmem-backup-`)); + +let repo: string; +let home: string; +let settingsPath: string; + +beforeEach(() => { + repo = mkdtempSync(join(tmpdir(), "gitmem-git120-repo-")); + home = mkdtempSync(join(tmpdir(), "gitmem-git120-home-")); + mkdirSync(join(repo, ".claude")); + settingsPath = join(repo, ".claude", "settings.json"); +}); + +afterEach(() => { + rmSync(repo, { recursive: true, force: true }); + rmSync(home, { recursive: true, force: true }); +}); + +describe("GIT-120: Claude Code settings.json", () => { + const original = () => ({ permissions: { allow: ["Bash(ls:*)"] }, hooks: structuredClone(FOREIGN_CLAUDE) }); + + it("install-hooks keeps foreign hooks, backs up first, and prints what changed", async () => { + const before = JSON.stringify(original(), null, 2); + writeFileSync(settingsPath, before); + + const r = await gitmem(["install-hooks"], repo, home); + expect(r.code).toBe(0); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + + expect(foreignCommands(after.hooks).sort()).toEqual(allCommands(FOREIGN_CLAUDE).sort()); + expect(after.hooks.Notification).toEqual(FOREIGN_CLAUDE.Notification); + expect(gitmemCommands(after.hooks).length).toBeGreaterThan(0); + expect(after.permissions).toEqual({ allow: ["Bash(ls:*)"] }); + + const b = backups(join(repo, ".claude"), "settings.json"); + expect(b).toHaveLength(1); + expect(readFileSync(join(repo, ".claude", b[0]), "utf-8")).toBe(before); + + expect(r.stdout).toContain("Changes to .claude/settings.json:"); + expect(r.stdout).toMatch(/SessionStart: added \d+ gitmem hook\(s\), kept 1 other/); + expect(r.stdout).toMatch(/Notification: kept 1 \(untouched\)/); + expect(r.stdout).toContain("Backup of the previous file:"); + }); + + it("install-hooks --force twice replaces gitmem's hooks without duplicating them", async () => { + writeFileSync(settingsPath, JSON.stringify(original(), null, 2)); + await gitmem(["install-hooks"], repo, home); + const once = gitmemCommands(JSON.parse(readFileSync(settingsPath, "utf-8")).hooks).length; + + const r = await gitmem(["install-hooks", "--force"], repo, home); + expect(r.code).toBe(0); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(gitmemCommands(after.hooks)).toHaveLength(once); + expect(foreignCommands(after.hooks).sort()).toEqual(allCommands(FOREIGN_CLAUDE).sort()); + expect(r.stdout).toMatch(/SessionStart: replaced \d+ gitmem hook\(s\) with \d+, kept 1 other/); + }); + + it("install-hooks without --force leaves an installed file alone", async () => { + writeFileSync(settingsPath, JSON.stringify(original(), null, 2)); + await gitmem(["install-hooks"], repo, home); + const snapshot = readFileSync(settingsPath, "utf-8"); + const r = await gitmem(["install-hooks"], repo, home); + expect(r.stdout).toContain("already installed"); + expect(readFileSync(settingsPath, "utf-8")).toBe(snapshot); + }); + + it("uninstall-hooks removes only gitmem's commands and leaves the foreign hooks as they were", async () => { + writeFileSync(settingsPath, JSON.stringify(original(), null, 2)); + await gitmem(["install-hooks"], repo, home); + + const r = await gitmem(["uninstall-hooks"], repo, home); + expect(r.code).toBe(0); + const after = JSON.parse(readFileSync(settingsPath, "utf-8")); + expect(after.hooks).toEqual(FOREIGN_CLAUDE); + expect(after.permissions).toEqual({ allow: ["Bash(ls:*)"] }); + expect(r.stdout).toMatch(/Removed \d+ gitmem hook\(s\) from \.claude\/settings\.json; kept 3 other/); + expect(backups(join(repo, ".claude"), "settings.json")).toHaveLength(2); + }); + + it("uninstall-hooks keeps a foreign command that shares a matcher group with gitmem's", async () => { + const mixed = { + hooks: { + PreToolUse: [{ matcher: "Bash", hooks: [ + { type: "command", command: "bash .gitmem/hooks/credential-guard.sh", timeout: 3000 }, + { type: "command", command: "echo mine" }, + ] }], + }, + }; + writeFileSync(settingsPath, JSON.stringify(mixed, null, 2)); + const r = await gitmem(["uninstall-hooks"], repo, home); + expect(r.code).toBe(0); + expect(JSON.parse(readFileSync(settingsPath, "utf-8")).hooks).toEqual({ + PreToolUse: [{ matcher: "Bash", hooks: [{ type: "command", command: "echo mine" }] }], + }); + }); + + it("install-hooks leaves events it does not manage alone, including init's auto-retrieve hook", async () => { + const autoRetrieve = { UserPromptSubmit: [{ hooks: [{ type: "command", command: "bash .gitmem/hooks/auto-retrieve-hook.sh", timeout: 3000 }] }] }; + writeFileSync(settingsPath, JSON.stringify({ hooks: autoRetrieve }, null, 2)); + const r = await gitmem(["install-hooks", "--force"], repo, home); + expect(r.code).toBe(0); + expect(JSON.parse(readFileSync(settingsPath, "utf-8")).hooks.UserPromptSubmit).toEqual(autoRetrieve.UserPromptSubmit); + }); + + it("uninstall-hooks drops the hooks key only when nothing else is left", async () => { + await gitmem(["install-hooks"], repo, home); + await gitmem(["uninstall-hooks"], repo, home); + expect(JSON.parse(readFileSync(settingsPath, "utf-8")).hooks).toBeUndefined(); + }); + + it("an unparseable settings.json is not overwritten", async () => { + writeFileSync(settingsPath, "{ not json"); + for (const cmd of ["install-hooks", "uninstall-hooks"]) { + const r = await gitmem([cmd], repo, home); + expect(r.code).toBe(1); + expect(r.stderr).toContain("Nothing was changed"); + expect(readFileSync(settingsPath, "utf-8")).toBe("{ not json"); + } + }); +}); + +describe("GIT-120: Cursor hooks.json", () => { + it("install and uninstall keep a foreign Cursor hook", async () => { + mkdirSync(join(repo, ".cursor")); + const hooksPath = join(repo, ".cursor", "hooks.json"); + const foreign = { version: 1, hooks: { stop: [{ command: "echo cursor-stop" }], afterFileEdit: [{ command: "prettier --write" }] } }; + writeFileSync(hooksPath, JSON.stringify(foreign, null, 2)); + + const i = await gitmem(["install-hooks", "--client", "cursor"], repo, home); + expect(i.code).toBe(0); + const installed = JSON.parse(readFileSync(hooksPath, "utf-8")); + expect(foreignCommands(installed.hooks).sort()).toEqual(["echo cursor-stop", "prettier --write"]); + expect(gitmemCommands(installed.hooks).length).toBeGreaterThan(0); + expect(installed.version).toBe(1); + + const u = await gitmem(["uninstall-hooks", "--client", "cursor"], repo, home); + expect(u.code).toBe(0); + expect(JSON.parse(readFileSync(hooksPath, "utf-8"))).toEqual(foreign); + }); +}); diff --git a/tests/helpers/schema-drift.ts b/tests/helpers/schema-drift.ts index 67b19dc..bfd092f 100644 --- a/tests/helpers/schema-drift.ts +++ b/tests/helpers/schema-drift.ts @@ -527,3 +527,99 @@ export function compare(scan: ScanResult, floor: Schema, current: Schema): Findi 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}`)); } + +// ---------------------------------------------------------------- RPCs (GIT-114) + +/** setup.sql -> function names (CREATE [OR REPLACE] FUNCTION name(...)). */ +export function parseFunctions(sql: string): Set { + const names = new Set(); + const re = /CREATE\s+(?:OR\s+REPLACE\s+)?FUNCTION\s+(?:public\.)?(\w+)\s*\(/gi; + let m: RegExpExecArray | null; + const clean = sql.replace(/--[^\n]*/g, ""); + while ((m = re.exec(clean))) names.add(m[1]); + return names; +} + +export interface RpcRef { + file: string; + line: number; + name: string; + /** A fixed https:// URL — another project (e.g. licensing), not the customer's store. */ + external: boolean; +} + +/** + * Text of a string or template literal, with getTableName("x") substitutions + * expanded to the default table name; any other substitution becomes "\0". + */ +function literalText(n: ts.Node): string | null { + if (ts.isStringLiteralLike(n)) return n.text; + if (ts.isTemplateExpression(n)) { + let out = n.head.text; + for (const span of n.templateSpans) { + const e = span.expression; + out += ts.isCallExpression(e) && ts.isIdentifier(e.expression) && e.expression.text === "getTableName" && + e.arguments[0] && ts.isStringLiteralLike(e.arguments[0]) + ? DEFAULT_PREFIX + e.arguments[0].text + : "\0"; + out += span.literal.text; + } + return out; + } + return null; +} + +/** + * Every RPC name src/ can call. Two shapes: + * - a literal or template URL containing "/rpc/"; + * - in a file that builds "/rpc/${…}" dynamically, every `name:` property of + * an object literal (the candidate list the URL is built from). + * Returns the refs and the dynamic call sites whose names could not be resolved. + */ +export function scanRpcNames(srcDir: string): { refs: RpcRef[]; unresolved: Unresolved[] } { + const refs: RpcRef[] = []; + const unresolved: Unresolved[] = []; + 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); + const sf = ts.createSourceFile(file, fs.readFileSync(file, "utf-8"), ts.ScriptTarget.Latest, true); + const lineOf = (n: ts.Node) => sf.getLineAndCharacterOfPosition(n.getStart()).line + 1; + const dynamicSites: ts.Node[] = []; + const nameProps: ts.PropertyAssignment[] = []; + const visit = (n: ts.Node): void => { + const text = literalText(n); + if (text !== null && text.includes("/rpc/")) { + const seg = text.slice(text.indexOf("/rpc/") + 5).split(/[/?"'\s]/)[0]; + if (seg && !seg.includes("\0")) { + refs.push({ file: rel, line: lineOf(n), name: seg, external: /^https?:\/\//.test(text) }); + } else { + dynamicSites.push(n); + } + } + if (ts.isPropertyAssignment(n) && propName(n) === "name") nameProps.push(n); + ts.forEachChild(n, visit); + }; + visit(sf); + if (dynamicSites.length === 0) continue; + let found = 0; + for (const p of nameProps) { + const t = literalText(p.initializer); + if (t && /^[a-z][a-z0-9_]*$/.test(t)) { + refs.push({ file: rel, line: lineOf(p), name: t, external: false }); + found++; + } + } + if (found === 0) { + for (const d of dynamicSites) unresolved.push({ file: rel, line: lineOf(d), call: "rpc", what: `dynamic RPC name ${d.getText().slice(0, 60)}` }); + } + } + return { refs, unresolved }; +} diff --git a/tests/unit/hooks/quick-retrieve-deadline.test.ts b/tests/unit/hooks/quick-retrieve-deadline.test.ts new file mode 100644 index 0000000..2d34ef1 --- /dev/null +++ b/tests/unit/hooks/quick-retrieve-deadline.test.ts @@ -0,0 +1,41 @@ +/** + * GIT-116: quick-retrieve bounds its own run. The hook used to wrap it in + * `timeout 2.5`, which macOS does not ship, so on a stock Mac nothing was + * ever retrieved. + */ +import { describe, it, expect, vi, afterEach } from "vitest"; +import * as fs from "fs"; +import * as path from "path"; +import { armDeadline, QUICK_RETRIEVE_DEADLINE_MS } from "../../../src/hooks/quick-retrieve.js"; + +afterEach(() => { vi.useRealTimers(); }); + +describe("quick-retrieve deadline (GIT-116)", () => { + it("fires after the budget and not before", () => { + vi.useFakeTimers(); + const expire = vi.fn(); + armDeadline(100, expire); + vi.advanceTimersByTime(99); + expect(expire).not.toHaveBeenCalled(); + vi.advanceTimersByTime(1); + expect(expire).toHaveBeenCalledOnce(); + }); + + it("does not keep a finished run alive (unref'd)", () => { + const timer = armDeadline(60_000, () => {}); + expect(timer.hasRef()).toBe(false); + clearTimeout(timer); + }); + + it("leaves 500 ms of the hook's 3 s budget", () => { + expect(QUICK_RETRIEVE_DEADLINE_MS).toBe(2500); + }); + + it("no hook script calls timeout/gtimeout any more", () => { + const dir = path.resolve(__dirname, "../../../hooks/scripts"); + for (const f of fs.readdirSync(dir).filter((f) => f.endsWith(".sh"))) { + const code = fs.readFileSync(path.join(dir, f), "utf-8").split("\n").filter((l) => !l.trim().startsWith("#")); + expect(code.filter((l) => /(^|[\s;(|&`$])g?timeout\s+[0-9]/.test(l)), f).toEqual([]); + } + }); +}); diff --git a/tests/unit/schema-drift.test.ts b/tests/unit/schema-drift.test.ts index d29936b..83bb05e 100644 --- a/tests/unit/schema-drift.test.ts +++ b/tests/unit/schema-drift.test.ts @@ -21,7 +21,7 @@ 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 { parseSchema, scanSources, compare, selectColumns, parseFunctions, scanRpcNames } 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"; @@ -177,3 +177,63 @@ describe("src/ against the v1.8.0 schema", () => { expect(missing, "SESSION_COLUMNS entries a v1.8.0 store rejects").toEqual([]); }); }); + +// --------------------------------------------------------------------------- +// GIT-114: RPC names +// --------------------------------------------------------------------------- + +/** + * RPCs src/ may call that v1.8.0's setup.sql does not define, each owned by a + * ticket. Remove an entry when it goes; a stale entry fails. + */ +const ALLOWED_RPCS: Array<{ name: string; ticket: string; reason: string }> = [ + { name: "match_gitmem_learnings_weighted", ticket: "GIT-114", reason: "fallback only after gitmem_scar_search answers PGRST202 (a store without the setup.sql function, e.g. nTEG's)" }, + { name: "match_transcript_chunks", ticket: "GIT-106", reason: "transcript search: dev tier only; setup.sql creates neither the table nor the function" }, +]; + +describe("RPC names against the v1.8.0 schema (GIT-114)", () => { + const functions = parseFunctions(fs.readFileSync(FLOOR_SQL, "utf-8")); + const rpc = scanRpcNames(path.join(REPO, "src")); + const storeRefs = rpc.refs.filter((r) => !r.external); + + it("parses setup.sql's functions", () => { + expect([...functions]).toEqual(expect.arrayContaining([ + "gitmem_scar_search", "gitmem_semantic_search", "refresh_scar_behavioral_scores", + ])); + }); + + it("finds the RPC call sites, including the scar-search candidate list", () => { + const names = storeRefs.map((r) => r.name); + expect(names).toEqual(expect.arrayContaining(["gitmem_scar_search", "refresh_scar_behavioral_scores"])); + expect(rpc.unresolved).toEqual([]); + }); + + it("every RPC on the customer's store is defined by setup.sql or allowlisted by ticket", () => { + const missing = storeRefs.filter((r) => !functions.has(r.name) && !ALLOWED_RPCS.some((a) => a.name === r.name)); + expect(missing.map((r) => `${r.name} at ${r.file}:${r.line}`)).toEqual([]); + }); + + it("every allowlisted RPC is still referenced and names a ticket", () => { + for (const a of ALLOWED_RPCS) { + expect(a.ticket).toMatch(/^GIT-\d+$/); + expect(storeRefs.some((r) => r.name === a.name), `stale ALLOWED_RPCS entry ${a.name}`).toBe(true); + } + }); + + it("catches a planted call to a function setup.sql lacks, and a dynamic name it cannot resolve", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-rpc-drift-")); + try { + fs.mkdirSync(path.join(dir, "src")); + fs.writeFileSync(path.join(dir, "src", "a.ts"), ` + declare const U: string; declare function getTableName(b: string): string; declare const x: string; + export const a = \`\${U}/rest/v1/rpc/match_\${getTableName("learnings")}_weighted\`; + export const b = "https://example.supabase.co/rest/v1/rpc/licence_check"; + export const c = \`\${U}/rest/v1/rpc/\${x}\`;`); + const r = scanRpcNames(path.join(dir, "src")); + expect(r.refs.map((x) => `${x.name}:${x.external}`).sort()).toEqual(["licence_check:true", "match_gitmem_learnings_weighted:false"]); + expect(r.unresolved).toHaveLength(1); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/unit/services/directpatch-zero-rows.test.ts b/tests/unit/services/directpatch-zero-rows.test.ts new file mode 100644 index 0000000..e27e4fe --- /dev/null +++ b/tests/unit/services/directpatch-zero-rows.test.ts @@ -0,0 +1,131 @@ +/** + * GIT-119: a PATCH that matched no row is not a write. + * + * PostgREST answers 200 with [] when a PATCH's filter matches nothing (a wrong + * id, a row not created yet, RLS). directPatch returned that array and all 8 + * callers ignored it, so each reported or assumed success for a write that did + * not happen. directPatch now returns { count, rows } and every caller treats + * count 0 as not durable, each in the way its result can carry it. + * + * One parameterized test over all 8 callers. The store is stubbed at fetch: + * every PATCH answers 200 [], every GET answers the rows the caller needs to + * reach its PATCH. + */ +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach } from "vitest"; + +const env = vi.hoisted(() => { + const keys = ["GITMEM_TIER", "SUPABASE_URL", "SUPABASE_SERVICE_ROLE_KEY", "GITMEM_DIR"]; + const before = Object.fromEntries(keys.map((k) => [k, process.env[k]])); + const dir = require("fs").mkdtempSync(require("path").join(require("os").tmpdir(), "gitmem-git119-")); + Object.assign(process.env, { + GITMEM_TIER: "pro", + SUPABASE_URL: "https://stubbed.supabase.co", + SUPABASE_SERVICE_ROLE_KEY: "stub", + GITMEM_DIR: dir, + }); + return { before, dir }; +}); + +import { resetTier } from "../../../src/services/tier.js"; +import { archiveLearning } from "../../../src/tools/archive-learning.js"; +import { saveSessionEmbedding } from "../../../src/tools/session-close.js"; +import { markSessionSuperseded } from "../../../src/tools/session-start.js"; +import { resolveThreadInSupabase, touchThreadsInSupabase, archiveDormantThreads } from "../../../src/services/thread-supabase.js"; +import { updateRelevanceData } from "../../../src/services/metrics.js"; +import { saveTranscript } from "../../../src/services/supabase-client.js"; +import { getEffectTracker } from "../../../src/services/effect-tracker.js"; + +const UUID = "a501c95e-1234-4678-9abc-def012345678"; +const SCAR = "b1111111-2222-4333-8444-555555555555"; +const patches: string[] = []; + +/** Rows a GET returns, by table, so each caller reaches its PATCH. */ +function getRows(url: URL): unknown[] { + const table = url.pathname.split("/").pop() ?? ""; + if (url.searchParams.get("limit") === "1" && url.searchParams.get("select") === "archived_at") return [{ archived_at: null }]; // column probe + if (table.endsWith("sessions")) return [{ close_compliance: null }]; + if (table.endsWith("threads")) { + if (url.searchParams.get("status") === "eq.dormant") { + return [{ id: UUID, thread_id: "t-dormant", metadata: { dormant_since: new Date(Date.now() - 60 * 86400_000).toISOString() } }]; + } + return [{ id: UUID, thread_id: "t-x", touch_count: 1, created_at: new Date().toISOString(), thread_class: "backlog", status: "active" }]; + } + if (table === "gitmem_query_metrics") return [{ id: UUID, memories_surfaced: [SCAR], metadata: {} }]; + return []; +} + +beforeAll(() => { + resetTier(); + vi.stubGlobal("fetch", vi.fn(async (input: unknown, init?: { method?: string }) => { + const url = new URL(String(input)); + const method = init?.method ?? "GET"; + const body = method === "PATCH" ? (patches.push(url.pathname), []) : method === "GET" ? getRows(url) : { Key: "ok" }; + return new Response(JSON.stringify(body), { status: 200, headers: { "content-type": "application/json" } }); + })); +}); + +afterAll(() => { + vi.unstubAllGlobals(); + for (const [k, v] of Object.entries(env.before)) { + if (v === undefined) delete process.env[k]; else process.env[k] = v; + } + resetTier(); + require("fs").rmSync(env.dir, { recursive: true, force: true }); +}); + +beforeEach(() => { patches.length = 0; }); + +/** A background write's failure is visible in stderr or the effect tracker. */ +const errorsLogged = () => (console.error as unknown as { mock: { calls: unknown[][] } }).mock.calls.map((c) => c.join(" ")).join("\n"); +const trackerFailures = (path: string) => getEffectTracker().getHealthReport().byPath[path]?.failed ?? 0; + +type Case = { caller: string; run: () => Promise }; + +const CASES: Case[] = [ + { caller: "archive_learning (full UUID)", run: async () => { + const r = await archiveLearning({ id: UUID }); + expect(r.success).toBe(false); + expect(r.durable).toBe(false); + } }, + { caller: "session_close: session embedding", run: async () => { + await expect(saveSessionEmbedding(UUID, "[0.1]")).rejects.toThrow(/updated no row/); + } }, + { caller: "session_start: mark superseded", run: async () => { + await markSessionSuperseded(UUID, SCAR); + expect(errorsLogged()).toMatch(/Failed to mark session .* superseded[\s\S]*no session row/); + } }, + { caller: "thread resolve", run: async () => { + expect(await resolveThreadInSupabase("t-x")).toBe(false); + } }, + { caller: "thread touch", run: async () => { + await touchThreadsInSupabase(["t-x"]); + expect(errorsLogged()).toMatch(/Failed to touch thread t-x[\s\S]*updated no row/); + } }, + { caller: "thread auto-archive", run: async () => { + const r = await archiveDormantThreads("default", 30); + expect(r.archived_count).toBe(0); + expect(r.archived_ids).toEqual([]); + } }, + { caller: "relevance update (metrics)", run: async () => { + const before = trackerFailures("relevance_update"); + await updateRelevanceData(UUID, [SCAR], { [SCAR]: "high" }); + expect(trackerFailures("relevance_update")).toBe(before + 1); + } }, + { caller: "saveTranscript: transcript_path", run: async () => { + const r = await saveTranscript(UUID, "hello"); + expect(r.patch_warning).toMatch(/no session row/); + } }, +]; + +describe("GIT-119: every directPatch caller treats 0 rows as not durable", () => { + it("covers all 8 callers", () => { + expect(CASES).toHaveLength(8); + }); + + it.each(CASES)("$caller", async ({ run }) => { + vi.spyOn(console, "error").mockImplementation(() => {}); + await run(); + expect(patches.length).toBeGreaterThan(0); // the PATCH was really sent + vi.restoreAllMocks(); + }); +}); diff --git a/tests/unit/services/gitmem-root-resolution.test.ts b/tests/unit/services/gitmem-root-resolution.test.ts index cdd086e..1bf6259 100644 --- a/tests/unit/services/gitmem-root-resolution.test.ts +++ b/tests/unit/services/gitmem-root-resolution.test.ts @@ -140,10 +140,38 @@ describe("GIT-91: isLiveGitmemRoot only counts real evidence", () => { vi.restoreAllMocks(); }); - it("counts a deliberate project-scoped install (config.json)", () => { + // GIT-115: a repo's .gitmem/ holds config.json (the project name) and the + // hook scripts on purpose; the store lives in the root the server reads. + it("does not count a repo .gitmem/ holding only config.json and hooks", () => { const gitmem = path.join(tmp, ".gitmem"); - fs.mkdirSync(gitmem, { recursive: true }); + fs.mkdirSync(path.join(gitmem, "hooks"), { recursive: true }); fs.writeFileSync(path.join(gitmem, "config.json"), JSON.stringify({ project: "x" })); + fs.writeFileSync(path.join(gitmem, "hooks", "session-start.sh"), "#!/bin/bash\n"); + + expect(isLiveGitmemRoot(gitmem)).toBe(false); + }); + + it("does not count what an older init seeded: starter lessons and the welcome thread", () => { + const gitmem = path.join(tmp, ".gitmem"); + fs.mkdirSync(gitmem, { recursive: true }); + const starters = JSON.parse(fs.readFileSync(path.join(__dirname, "../../../schema/starter-scars.json"), "utf-8")); + fs.writeFileSync(path.join(gitmem, "learnings.json"), JSON.stringify(starters)); + fs.writeFileSync(path.join(gitmem, "threads.json"), JSON.stringify([{ id: "t-welcome01", text: "welcome" }])); + fs.writeFileSync(path.join(gitmem, "decisions.json"), "[]"); + fs.writeFileSync(path.join(gitmem, "config.json"), "{}"); + + expect(isLiveGitmemRoot(gitmem)).toBe(false); + }); + + it.each([ + ["a learning of the user's own", "learnings.json", [{ id: "user-learning-1", title: "mine" }]], + ["a thread of the user's own", "threads.json", [{ id: "t-abc12345", text: "mine" }]], + ["a decision", "decisions.json", [{ id: "d-1", title: "chose X" }]], + ["records in the {key: array} shape", "learnings.json", { learnings: [{ id: "user-learning-2" }] }], + ])("counts %s", (_label, file, content) => { + const gitmem = path.join(tmp, ".gitmem"); + fs.mkdirSync(gitmem, { recursive: true }); + fs.writeFileSync(path.join(gitmem, file), JSON.stringify(content)); expect(isLiveGitmemRoot(gitmem)).toBe(true); }); diff --git a/tests/unit/services/gitmem-root-symlink.test.ts b/tests/unit/services/gitmem-root-symlink.test.ts index a7ec2cc..19eca86 100644 --- a/tests/unit/services/gitmem-root-symlink.test.ts +++ b/tests/unit/services/gitmem-root-symlink.test.ts @@ -15,11 +15,15 @@ import { findStrandedProjectRoots, sameDirectory, canonicalPath, clearGitmemDirC 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. */ +/** + * A live project-scoped store: it holds a learning of the user's own. + * (GIT-115: config.json alone no longer marks one; a repo keeps its + * config.json and hooks in .gitmem/ while the store lives elsewhere.) + */ 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" })); + fs.writeFileSync(path.join(g, "learnings.json"), JSON.stringify([{ id: "user-learning", title: "mine" }])); return g; } diff --git a/tests/unit/services/scar-search-rpc-name.test.ts b/tests/unit/services/scar-search-rpc-name.test.ts index 2754cfd..8fe94e5 100644 --- a/tests/unit/services/scar-search-rpc-name.test.ts +++ b/tests/unit/services/scar-search-rpc-name.test.ts @@ -97,22 +97,96 @@ describe("GIT-93: retrieval RPC names are derived from the table, not the prefix expect(calls[0].body).not.toHaveProperty("project_filter"); }); - it("semanticSearch calls match_
", async () => { + // GIT-114: this used to assert match_gitmem_learnings_weighted for the + // default prefix — the name GIT-93 took from nTEG's store. No setup.sql + // defines it, so every customer's cold-index recall got PGRST202. The + // default prefix now calls setup.sql's function first. + it("default prefix: calls setup.sql's gitmem_scar_search with similarity_threshold", async () => { const { calls } = stubFetch(); - const client = await loadClient("orchestra_"); + const client = await loadClient("gitmem_"); - await client.semanticSearch({ query: "any query", match_count: 5 }); + await client.scarSearch("any query", 3); - expect(calls[0].url).toContain("/rest/v1/rpc/match_orchestra_learnings"); - expect(calls[0].url).not.toContain("orchestra_semantic_search"); + expect(calls[0].url).toMatch(/\/rest\/v1\/rpc\/gitmem_scar_search$/); + expect(calls[0].body).toHaveProperty("similarity_threshold", 0); + expect(calls[0].body).not.toHaveProperty("match_threshold"); + expect(calls[0].body).not.toHaveProperty("project_filter"); }); +}); - it("tracks the prefix rather than hardcoding one deployment's table", async () => { - const { calls } = stubFetch(); +// --------------------------------------------------------------------------- +// GIT-114: fallback and missing columns +// --------------------------------------------------------------------------- + +type Reply = { status: number; body: unknown }; +function scriptedFetch(replies: Array<(url: string) => Reply | null>) { + const calls: Array<{ url: string; body: Record }> = []; + vi.stubGlobal("fetch", vi.fn(async (url: string, init?: { body?: string }) => { + calls.push({ url: String(url), body: init?.body ? JSON.parse(init.body) : {} }); + let r: Reply | null = null; + for (const f of replies) { r = f(String(url)); if (r) break; } + r = r ?? { status: 200, body: [] }; + const text = typeof r.body === "string" ? r.body : JSON.stringify(r.body); + return { ok: r.status < 300, status: r.status, json: async () => JSON.parse(text), text: async () => text } as unknown as Response; + })); + return { calls }; +} +const PGRST202 = { status: 404, body: '{"code":"PGRST202","message":"Could not find the function"}' }; +const SCAR = { id: "11111111-2222-4333-8444-555555555555", title: "t", description: "d", severity: "high", similarity: 0.8 }; + +describe("GIT-114: remote scar search on a setup.sql store", () => { + beforeEach(() => { vi.resetModules(); }); + afterEach(() => { + vi.unstubAllGlobals(); + vi.doUnmock("../../../src/services/embedding.js"); + process.env = { ...ORIGINAL_ENV }; + }); + + it("a store without gitmem_scar_search (PGRST202) falls back to match_gitmem_learnings_weighted, and remembers it", async () => { + const { calls } = scriptedFetch([ + (u) => (u.endsWith("/rpc/gitmem_scar_search") ? PGRST202 : null), + (u) => (u.endsWith("/rpc/match_gitmem_learnings_weighted") ? { status: 200, body: [{ ...SCAR, learning_type: "scar" }] } : null), + ]); const client = await loadClient("gitmem_"); - await client.scarSearch("any query", 3); + const rows = await client.scarSearch("q", 3); + expect(rows).toHaveLength(1); + expect(calls.map((c) => c.url.split("/rpc/")[1])).toEqual(["gitmem_scar_search", "match_gitmem_learnings_weighted"]); + expect(calls[1].body).toHaveProperty("match_threshold"); + + calls.length = 0; + await client.scarSearch("q", 3); + expect(calls.map((c) => c.url.split("/rpc/")[1])).toEqual(["match_gitmem_learnings_weighted"]); + }); + + it("a real error from an existing function is not retried under another name", async () => { + const { calls } = scriptedFetch([(u) => (u.includes("/rpc/") ? { status: 500, body: "boom" } : null)]); + const client = await loadClient("gitmem_"); + await expect(client.scarSearch("q", 3)).rejects.toThrow(/500/); + expect(calls).toHaveLength(1); + }); + + it("fetches the columns gitmem_scar_search does not return, by id, in one request", async () => { + const { calls } = scriptedFetch([ + (u) => (u.endsWith("/rpc/gitmem_scar_search") ? { status: 200, body: [SCAR] } : null), + (u) => (u.includes("/rest/v1/gitmem_learnings?") ? { status: 200, body: [{ id: SCAR.id, learning_type: "scar", applies_when: ["deploy"], why_this_matters: "w" }] } : null), + ]); + const client = await loadClient("gitmem_"); + + const [row] = await client.scarSearch>("q", 3); + expect(row).toMatchObject({ ...SCAR, learning_type: "scar", applies_when: ["deploy"], why_this_matters: "w" }); + const follow = new URL(calls[1].url); + expect(follow.searchParams.get("id")).toBe(`in.(${SCAR.id})`); + expect(follow.searchParams.get("select")).toContain("learning_type"); + }); - expect(calls[0].url).toContain("/rest/v1/rpc/match_gitmem_learnings_weighted"); + it("if the follow-up select fails, the matched scars are still returned", async () => { + scriptedFetch([ + (u) => (u.endsWith("/rpc/gitmem_scar_search") ? { status: 200, body: [SCAR] } : null), + (u) => (u.includes("/rest/v1/gitmem_learnings?") ? { status: 503, body: "down" } : null), + ]); + const client = await loadClient("gitmem_"); + const rows = await client.scarSearch>("q", 3); + expect(rows).toEqual([SCAR]); }); }); diff --git a/tests/unit/services/startup-reload-failure.test.ts b/tests/unit/services/startup-reload-failure.test.ts new file mode 100644 index 0000000..9a2b1a9 --- /dev/null +++ b/tests/unit/services/startup-reload-failure.test.ts @@ -0,0 +1,170 @@ +/** + * GIT-118: a failed load never empties recall. + * + * loadScarsFromSupabase returned [] on any error with no flag, and every + * caller reinitialised the index and overwrote the hooks' scar cache with it. + * create_learning triggers a reload after every write, so one failed request + * emptied recall — and reported success — until the next good load, which a + * failed init never retried. + * + * Store mocked at the supabase-client boundary; the disk vector cache, the + * local index and the hook cache are real (temp GITMEM_DIR). Each test gets a + * fresh module graph, i.e. a fresh process. + */ +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 store = vi.hoisted(() => ({ + rows: [] as Array<{ id: string; title: string; description: string; severity: string; learning_type: string; updated_at: string; embedding: number[] }>, + down: false, + calls: { bulk: 0, manifest: 0 }, +})); +const active = () => [...store.rows].sort((a, b) => b.updated_at.localeCompare(a.updated_at)); +const fail = () => { throw new TypeError("fetch failed: ECONNRESET"); }; + +vi.mock("../../../src/services/supabase-client.js", () => ({ + isConfigured: () => true, + getSupabaseUrl: () => "https://venue.example.supabase.co", + getLearningsFingerprint: vi.fn(async () => { + if (store.down) return { count: -1, latestUpdatedAt: null }; // the real one's failure shape + const r = active(); + return { count: r.length, latestUpdatedAt: r[0]?.updated_at ?? null }; + }), + loadLearningsManifest: vi.fn(async () => { store.calls.manifest++; if (store.down) fail(); return active().map(({ id, updated_at }) => ({ id, updated_at })); }), + loadLearningsByIds: vi.fn(async (ids: string[]) => { if (store.down) fail(); return active().filter((r) => ids.includes(r.id)); }), + loadScarsWithEmbeddings: vi.fn(async () => { store.calls.bulk++; if (store.down) fail(); return active().map((r) => ({ ...r })); }), + directQuery: vi.fn(async () => []), +})); + +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: 1536 }, (_, k) => Math.sin(i + k)); // the index keeps only 1536-dim rows +const seed = (n: number, at = 0) => { + store.rows = Array.from({ length: n }, (_, i) => ({ + id: `id-${String(i).padStart(3, "0")}`, title: `scar ${i}`, description: "d", severity: "medium", + learning_type: "scar", updated_at: ts(at + i), embedding: vec(i), + })); +}; +const hookCache = () => JSON.parse(fs.readFileSync(path.join(dir, "cache", "hook-scars.json"), "utf-8")); + +async function freshProcess() { + vi.resetModules(); + const startup = await import("../../../src/services/startup.js"); + const lvs = await import("../../../src/services/local-vector-search.js"); + return { startup, count: () => lvs.getLocalVectorSearch().getScarCount() }; +} + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), "gitmem-git118-")); + vi.stubEnv("GITMEM_DIR", dir); + vi.stubEnv("GITMEM_TIER", "pro"); + vi.stubEnv("SUPABASE_URL", "https://venue.example.supabase.co"); + vi.stubEnv("SUPABASE_SERVICE_ROLE_KEY", "k"); + vi.stubEnv("GITMEM_SEARCH_MODE", "local"); + vi.stubEnv("GITMEM_VECTOR_DISK_CACHE", ""); + store.down = false; + store.calls = { bulk: 0, manifest: 0 }; + seed(20); +}); + +afterEach(async () => { + const { resetReloadRetry } = await import("../../../src/services/startup.js"); + resetReloadRetry?.(); + vi.useRealTimers(); + vi.unstubAllEnvs(); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +describe("GIT-118: a failed reload keeps what recall has", () => { + it("flush with the store down: success false, index and hook cache kept", async () => { + const { startup, count } = await freshProcess(); + expect((await startup.initializeGitMem()).success).toBe(true); + expect(count()).toBe(20); + expect(hookCache()).toHaveLength(20); + + store.down = true; + const r = await startup.flushCache(); + + expect(r.success).toBe(false); + expect(r.error).toMatch(/ECONNRESET/); + expect(count()).toBe(20); + expect(hookCache()).toHaveLength(20); + }); + + it("the post-write refresh uses the per-row delta, not a full download", async () => { + const { startup } = await freshProcess(); + await startup.initializeGitMem(); + store.calls = { bulk: 0, manifest: 0 }; + store.rows.push({ ...store.rows[0], id: "id-new", updated_at: ts(999) }); + + const r = await startup.refreshIndexAfterWrite(); + expect(r.success).toBe(true); + expect(r.new_scar_count).toBe(21); + expect(store.calls).toEqual({ bulk: 0, manifest: 1 }); + }); + + it("cold start with the store down serves the disk cache (stale) and says it failed", async () => { + const first = await freshProcess(); + await first.startup.initializeGitMem(); // leaves a disk cache behind + + store.down = true; + const { startup, count } = await freshProcess(); + const r = await startup.initializeGitMem(); + + expect(r.success).toBe(false); + expect(r.error).toMatch(/serving 20 from the disk cache/); + expect(count()).toBe(20); + }); + + it("a genuinely empty store is success with 0, not a failure", async () => { + store.rows = []; + const { startup, count } = await freshProcess(); + const r = await startup.initializeGitMem(); + expect(r.success).toBe(true); + expect(count()).toBe(0); + }); + + it("a failed load retries with backoff and recovers when the store does", async () => { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + const { startup, count } = await freshProcess(); + await startup.initializeGitMem(); + store.down = true; + await startup.flushCache(); + expect(startup.getReloadRetryState()).toEqual({ pending: true, attempt: 1 }); + + store.down = false; + seed(25, 100); + await vi.advanceTimersByTimeAsync(30_000); + + expect(count()).toBe(25); + expect(startup.getReloadRetryState()).toEqual({ pending: false, attempt: 0 }); + }); + + it("the second retry waits twice as long", async () => { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] }); + const { startup } = await freshProcess(); + await startup.initializeGitMem(); + store.down = true; + await startup.flushCache(); + await vi.advanceTimersByTimeAsync(30_000); // retry 1 fails, schedules retry 2 at 60 s + expect(startup.getReloadRetryState()).toEqual({ pending: true, attempt: 2 }); + store.calls = { bulk: 0, manifest: 0 }; + await vi.advanceTimersByTimeAsync(59_000); + expect(store.calls.manifest + store.calls.bulk).toBe(0); + await vi.advanceTimersByTimeAsync(1_000); + expect(store.calls.manifest + store.calls.bulk).toBeGreaterThan(0); + }); + + it("ensureInitialized tries again after a failed init (it cached the failure forever)", async () => { + store.down = true; + const { startup, count } = await freshProcess(); + await startup.ensureInitialized(); + expect(count()).toBe(0); + + store.down = false; + await startup.ensureInitialized(); + expect(count()).toBe(20); + }); +}); diff --git a/tests/unit/services/thread-sync-result.test.ts b/tests/unit/services/thread-sync-result.test.ts index ba5e976..21055c4 100644 --- a/tests/unit/services/thread-sync-result.test.ts +++ b/tests/unit/services/thread-sync-result.test.ts @@ -16,6 +16,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; const mockDirectQuery = vi.fn(); const mockDirectQueryAll = vi.fn(); const mockDirectUpsert = vi.fn(); +const mockDirectPatch = vi.fn(); const mockIsConfigured = vi.fn(() => true); const mockHasSupabase = vi.fn(() => true); @@ -24,6 +25,7 @@ vi.mock("../../../src/services/supabase-client.js", () => ({ directQuery: (...a: unknown[]) => mockDirectQuery(...a), directQueryAll: (...a: unknown[]) => mockDirectQueryAll(...a), directUpsert: (...a: unknown[]) => mockDirectUpsert(...a), + directPatch: (...a: unknown[]) => mockDirectPatch(...a), upsertRecord: vi.fn(), listRecords: vi.fn(), })); @@ -55,6 +57,8 @@ beforeEach(() => { mockIsConfigured.mockReturnValue(true); mockHasSupabase.mockReturnValue(true); mockDirectUpsert.mockResolvedValue([{ id: "row" }]); + mockDirectPatch.mockReset(); + mockDirectPatch.mockResolvedValue({ count: 1, rows: [{ id: "row" }] }); // GIT-119 shape }); describe("tier awareness (R3)", () => { @@ -236,3 +240,81 @@ describe("dedup candidate coverage (GIT-70)", () => { expect(r.dedup_coverage).not.toBe("complete"); }); }); + +// --------------------------------------------------------------------------- +// GIT-117: the WRITE fails, not the lookup +// --------------------------------------------------------------------------- +// The tests above fail only directQuery, the one call whose error propagated. +// The create / resolve / touch helpers caught their own failures and returned +// null / false / void, which the loop never read — so a thread whose write +// failed was counted as synced, and session_close pruned it from threads.json. + +describe("GIT-117: a thread counts as synced only when the store confirms a row", () => { + it("create rejected by the store -> failed, not synced", async () => { + mockDirectQuery.mockResolvedValue([]); // not in the store yet -> create + mockDirectUpsert.mockRejectedValue(new Error("Supabase upsert error: 409 - violates foreign key constraint")); + + const r = await syncThreadsToSupabase([thread("t-new")], "gitmem", "s-1"); + + expect(r.synced).toEqual([]); + expect(r.failed.map((f) => f.id)).toEqual(["t-new"]); + expect(r.failed[0].error).toMatch(/foreign key/); + expect(r.all_synced).toBe(false); + }); + + it("resolve whose PATCH updates no row -> failed", async () => { + mockDirectQuery + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r", status: "open" }]) // existence + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r" }]); // resolve lookup + mockDirectPatch.mockResolvedValue({ count: 0, rows: [] }); + + const r = await syncThreadsToSupabase([thread("t-r", "resolved")], "gitmem", "s-1"); + + expect(r.failed.map((f) => f.id)).toEqual(["t-r"]); + expect(r.failed[0].error).toMatch(/updated no row/); + expect(r.all_synced).toBe(false); + }); + + it("resolve rejected by the store -> failed", async () => { + mockDirectQuery + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r", status: "open" }]) + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r" }]); + mockDirectPatch.mockRejectedValue(new Error("Supabase patch error: 503")); + + const r = await syncThreadsToSupabase([thread("t-r", "resolved")], "gitmem", "s-1"); + expect(r.failed.map((f) => f.id)).toEqual(["t-r"]); + }); + + it("touch of an existing thread whose PATCH fails -> failed", async () => { + mockDirectQuery + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-o", status: "active" }]) // existence + .mockResolvedValueOnce([{ id: "u1", touch_count: 1, created_at: "2026-08-01T00:00:00Z", thread_class: "backlog", status: "active" }]); + mockDirectPatch.mockRejectedValue(new Error("Supabase patch error: 500")); + + const r = await syncThreadsToSupabase([thread("t-o")], "gitmem", "s-1"); + expect(r.failed.map((f) => f.id)).toEqual(["t-o"]); + expect(r.all_synced).toBe(false); + }); + + it("one failure among several: the others are synced, only the failure is reported", async () => { + mockDirectQuery.mockResolvedValue([]); + mockDirectUpsert + .mockResolvedValueOnce([{ id: "a" }]) + .mockRejectedValueOnce(new Error("boom")) + .mockResolvedValueOnce([{ id: "c" }]); + + const r = await syncThreadsToSupabase([thread("t-a"), thread("t-b"), thread("t-c")], "gitmem", "s-1"); + expect(r.synced).toEqual(["t-a", "t-c"]); + expect(r.failed.map((f) => f.id)).toEqual(["t-b"]); + }); + + it("every write confirmed -> all synced (the contract did not get stricter than the store)", async () => { + mockDirectQuery + .mockResolvedValueOnce([]) // t-new: create + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r", status: "open" }]) // t-r: exists + .mockResolvedValueOnce([{ id: "u1", thread_id: "t-r" }]); // t-r: resolve lookup + const r = await syncThreadsToSupabase([thread("t-new"), thread("t-r", "resolved")], "gitmem", "s-1"); + expect(r.failed).toEqual([]); + expect(r.all_synced).toBe(true); + }); +}); diff --git a/tests/unit/tools/archive-learning.test.ts b/tests/unit/tools/archive-learning.test.ts index 7a004be..211ea06 100644 --- a/tests/unit/tools/archive-learning.test.ts +++ b/tests/unit/tools/archive-learning.test.ts @@ -7,7 +7,9 @@ import { SUPABASE_TIER_MOCKS } from "../../helpers/supabase-mocks.js"; // --- Mocks --- -const mockDirectPatch = vi.fn().mockResolvedValue(undefined); +// GIT-119: directPatch returns what it changed; the default is one row. +const PATCHED_ONE = { count: 1, rows: [{ id: "a501c95e-1234-5678-9abc-def012345678" }] }; +const mockDirectPatch = vi.fn().mockResolvedValue(PATCHED_ONE); const mockDirectQuery = vi.fn().mockResolvedValue([]); const mockIsConfigured = vi.fn(() => true); @@ -44,6 +46,11 @@ vi.mock("../../../src/services/startup.js", () => ({ flushCache: vi.fn().mockResolvedValue(undefined), })); +const localIds = vi.hoisted(() => ({ ids: [] as string[] })); +vi.mock("../../../src/services/local-vector-search.js", () => ({ + getLocalVectorSearch: () => ({ getScarIds: () => localIds.ids }), +})); + vi.mock("../../../src/services/metrics.js", () => ({ Timer: class { stop() { return 42; } }, })); @@ -52,7 +59,7 @@ vi.mock("../../../src/services/display-protocol.js", () => ({ wrapDisplay: (msg: string) => msg, })); -import { archiveLearning } from "../../../src/tools/archive-learning.js"; +import { archiveLearning, uuidRangeForPrefix } from "../../../src/tools/archive-learning.js"; import { hasSupabase } from "../../../src/services/tier.js"; const FULL_UUID = "a501c95e-1234-5678-9abc-def012345678"; @@ -60,6 +67,8 @@ const FULL_UUID = "a501c95e-1234-5678-9abc-def012345678"; describe("archive_learning prefix resolution", () => { beforeEach(() => { vi.clearAllMocks(); + mockDirectPatch.mockResolvedValue(PATCHED_ONE); + localIds.ids = []; vi.mocked(hasSupabase).mockReturnValue(true); mockIsConfigured.mockReturnValue(true); }); @@ -107,19 +116,21 @@ describe("archive_learning prefix resolution", () => { // --- Supabase prefix resolution --- - it("resolves 8-char prefix via Supabase like filter", async () => { + // GIT-119: these asserted id=like.%. Postgres has no LIKE on a UUID + // column (42883 on every store), so they pinned the bug; the store is now + // asked for the UUID range the prefix spans. + it("resolves 8-char prefix via a UUID range query", async () => { mockDirectQuery.mockResolvedValueOnce([{ id: FULL_UUID }]); const result = await archiveLearning({ id: "a501c95e" }); expect(result.success).toBe(true); expect(result.id).toBe(FULL_UUID); - // Check directQuery was called with like filter expect(mockDirectQuery).toHaveBeenCalledWith( expect.any(String), expect.objectContaining({ select: "id", - filters: { id: "like.a501c95e%" }, + filters: { and: "(id.gte.a501c95e-0000-0000-0000-000000000000,id.lte.a501c95e-ffff-ffff-ffff-ffffffffffff)" }, limit: 2, }), ); @@ -141,7 +152,7 @@ describe("archive_learning prefix resolution", () => { expect(mockDirectQuery).toHaveBeenCalledWith( expect.any(String), expect.objectContaining({ - filters: { id: "like.a501%" }, + filters: { and: "(id.gte.a5010000-0000-0000-0000-000000000000,id.lte.a501ffff-ffff-ffff-ffff-ffffffffffff)" }, }), ); }); @@ -224,3 +235,42 @@ describe("archive_learning prefix resolution", () => { expect(result.display).toContain("superseded"); }); }); + +describe("GIT-119: archive_learning on a store that has no LIKE for UUIDs", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockDirectPatch.mockResolvedValue(PATCHED_ONE); + localIds.ids = []; + vi.mocked(hasSupabase).mockReturnValue(true); + mockIsConfigured.mockReturnValue(true); + }); + + it("uuidRangeForPrefix spans exactly the ids with that prefix", () => { + expect(uuidRangeForPrefix("a501c95e1")).toEqual([ + "a501c95e-1000-0000-0000-000000000000", + "a501c95e-1fff-ffff-ffff-ffffffffffff", + ]); + const [lo, hi] = uuidRangeForPrefix("a501"); + for (const id of ["a5010000-0000-0000-0000-000000000000", FULL_UUID, "a501ffff-ffff-ffff-ffff-ffffffffffff"]) { + expect(id >= lo && id <= hi).toBe(true); + } + for (const id of ["a500ffff-ffff-ffff-ffff-ffffffffffff", "a5020000-0000-0000-0000-000000000000"]) { + expect(id >= lo && id <= hi).toBe(false); + } + }); + + it("resolves from the warm local index without a query", async () => { + localIds.ids = [FULL_UUID, "b1111111-1111-1111-1111-111111111111"]; + const result = await archiveLearning({ id: "a501c9" }); + expect(result.id).toBe(FULL_UUID); + expect(mockDirectQuery).not.toHaveBeenCalled(); + }); + + it("an id the PATCH matched no row for is not archived, and not durable", async () => { + mockDirectPatch.mockResolvedValueOnce({ count: 0, rows: [] }); + const result = await archiveLearning({ id: FULL_UUID }); + expect(result.success).toBe(false); + expect(result.durable).toBe(false); + expect(result.error).toMatch(/nothing was archived/); + }); +}); diff --git a/tests/unit/tools/create-learning-errors.test.ts b/tests/unit/tools/create-learning-errors.test.ts index 3db0a6b..a6e116d 100644 --- a/tests/unit/tools/create-learning-errors.test.ts +++ b/tests/unit/tools/create-learning-errors.test.ts @@ -40,6 +40,9 @@ vi.mock("../../../src/services/display-protocol.js", () => ({ vi.mock("../../../src/services/startup.js", () => ({ flushCache: vi.fn(() => Promise.resolve()), + // GIT-118: create_learning awaits this; omitting it made every call report + // "index NOT refreshed" through a swallowed mock-export error. + refreshIndexAfterWrite: vi.fn(() => Promise.resolve({ success: true, previous_scar_count: 0, new_scar_count: 1, elapsed_ms: 1 })), })); vi.mock("../../../src/services/triple-writer.js", () => ({ @@ -190,3 +193,25 @@ describe("create_learning: DB error surfacing", () => { expect(result.errors).toBeUndefined(); }); }); + +// GIT-118: the post-write index refresh is awaited and its failure reported. +describe("create_learning: recall index refresh after the write (GIT-118)", () => { + it("says so in the display when the index could not be refreshed; the learning is still saved", async () => { + const startup = await import("../../../src/services/startup.js"); + vi.mocked(startup.refreshIndexAfterWrite).mockResolvedValueOnce({ + success: false, previous_scar_count: 5, new_scar_count: 5, elapsed_ms: 1, error: "fetch failed: ECONNRESET", + }); + const result = await createLearning({ + learning_type: "win", title: "t", description: "d", + } as never); + expect(result.success).toBe(true); + expect(result.index_refreshed).toBe(false); + expect(result.display).toMatch(/Recall index NOT refreshed \(fetch failed: ECONNRESET\)\. The learning is saved/); + }); + + it("says nothing extra when the refresh succeeds", async () => { + const result = await createLearning({ learning_type: "win", title: "t", description: "d" } as never); + expect(result.index_refreshed).toBeUndefined(); + expect(result.display).not.toMatch(/NOT refreshed/); + }); +}); diff --git a/tests/unit/tools/create-learning-field-persistence.test.ts b/tests/unit/tools/create-learning-field-persistence.test.ts index 6275ade..97a0267 100644 --- a/tests/unit/tools/create-learning-field-persistence.test.ts +++ b/tests/unit/tools/create-learning-field-persistence.test.ts @@ -55,6 +55,7 @@ vi.mock("../../../src/services/display-protocol.js", () => ({ vi.mock("../../../src/services/startup.js", () => ({ flushCache: vi.fn(() => Promise.resolve()), + refreshIndexAfterWrite: vi.fn(() => Promise.resolve({ success: true, previous_scar_count: 0, new_scar_count: 1, elapsed_ms: 1 })), // GIT-118 })); vi.mock("../../../src/services/triple-writer.js", () => ({ diff --git a/tests/unit/tools/variants-dev-only.test.ts b/tests/unit/tools/variants-dev-only.test.ts index 0bc6bc7..8f7c244 100644 --- a/tests/unit/tools/variants-dev-only.test.ts +++ b/tests/unit/tools/variants-dev-only.test.ts @@ -49,7 +49,10 @@ vi.mock("../../../src/services/embedding.js", () => ({ embed: async () => null, isEmbeddingAvailable: () => false, })); -vi.mock("../../../src/services/startup.js", () => ({ flushCache: async () => undefined })); +vi.mock("../../../src/services/startup.js", () => ({ + flushCache: async () => undefined, + refreshIndexAfterWrite: async () => ({ success: true, previous_scar_count: 0, new_scar_count: 1, elapsed_ms: 1 }), // GIT-118 +})); vi.mock("../../../src/services/triple-writer.js", () => ({ writeTriplesForLearning: async () => 0 })); const { resetTier, hasVariants } = await import("../../../src/services/tier.js");