diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index ab84f19..57e884f 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "name": "agent-memory", "source": "./", "description": "Durable cross-repository memory for coding agents — built to survive an IT security review. Zero runtime dependencies, zero dev dependencies, no install script: nothing runs when you install it, and granting it your agents is a separate explicit command. Independently scanned, with a passing verdict. Adds /handoff, /remember and /recall over one local markdown store. Requires the CLI: npm install -g @vib795/agent-memory (Node >= 22.5).", - "version": "0.7.0", + "version": "0.7.1", "author": { "name": "Utkarsh Singh", "url": "https://github.com/vib795" diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 3551f4b..3b9bfa6 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "agent-memory", "displayName": "agent-memory", - "version": "0.7.0", + "version": "0.7.1", "description": "Durable cross-repository memory for coding agents — built to survive an IT security review. Zero runtime dependencies, zero dev dependencies, no install script: nothing runs when you install it, and granting it your agents is a separate explicit command. Independently scanned, with a passing verdict. Adds /handoff, /remember and /recall over one local markdown store. Requires the CLI: npm install -g @vib795/agent-memory (Node >= 22.5).", "author": { "name": "Utkarsh Singh", diff --git a/README.md b/README.md index 80a4499..0c6e586 100644 --- a/README.md +++ b/README.md @@ -561,7 +561,7 @@ them as skipped, which is the intended outcome, not a failure. Needs Node 22.5 or newer; `doctor` says so plainly if the version is too old. -Run `npm test` for the suite (105 tests, no dependencies). CI runs it on Linux, +Run `npm test` for the suite (106 tests, no dependencies). CI runs it on Linux, macOS and Windows across Node 22 and 24, and separately installs the packed tarball and exercises it end to end on all three. diff --git a/package-lock.json b/package-lock.json index 80eb2ec..af8a3cd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@vib795/agent-memory", - "version": "0.7.0", + "version": "0.7.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@vib795/agent-memory", - "version": "0.7.0", + "version": "0.7.1", "license": "MIT", "bin": { "agent-memory": "src/cli.js" diff --git a/package.json b/package.json index c522016..0b5e67b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@vib795/agent-memory", - "version": "0.7.0", + "version": "0.7.1", "description": "Durable cross-repo knowledge graph for GitHub Copilot and Claude Code. Markdown source of truth, disposable SQLite index, zero runtime dependencies.", "keywords": [ "github-copilot", diff --git a/src/compact.js b/src/compact.js index b8ce852..be5a5b5 100644 --- a/src/compact.js +++ b/src/compact.js @@ -1,4 +1,5 @@ import { readFileSync, existsSync, realpathSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; import { fileURLToPath } from 'node:url'; import { join, sep, basename, dirname } from 'node:path'; import { loadConfig, paths } from './config.js'; @@ -152,28 +153,78 @@ function decay(active, cfg, now) { * what it now knows without costing anything at chat time. */ // The directory this package was installed into. `setup` links skill directories at -// `/skills/`, so every description write lands in the package's own +// `/skills/`, so a description write normally lands in the package's own // files. That is correct for an installed package and wrong for a git checkout, where // those files are tracked: one developer's digest gets committed and then published to // everyone. It shipped that way for twenty releases, advertising one machine's five // notes to every user who installed the plugin. const PACKAGE_ROOT = fileURLToPath(new URL('..', import.meta.url)); +// One process, one answer, so a per-run cache cannot go stale. `compact` asks about +// every registered path on every call, and most of them resolve to the same few trees. +const trackedCache = new Map(); + /** - * Is this path inside the package's own git checkout? + * Does git consider this exact file tracked? * - * Resolved through `realpathSync` because the path arrives as a symlink planted by - * `setup`; the link sits outside the checkout even when its target is inside it, which - * is the whole reason this went unnoticed. The `.git` test separates a developer's - * working tree from an ordinary install, which has no `.git` and must keep being - * written to — that write is the Tier-1 mechanism, not a bug. + * `true` and `false` are answers; `null` means git could not be asked and the caller + * has to fall back. A non-zero exit covers both "not tracked" and "not in a repository", + * and those mean the same thing here: writing the file publishes nothing. + */ +function isTracked(real) { + if (trackedCache.has(real)) return trackedCache.get(real); + let answer; + try { + execFileSync('git', ['ls-files', '--error-unmatch', '--', basename(real)], { + cwd: dirname(real), + stdio: ['ignore', 'ignore', 'ignore'], + timeout: 5000, + }); + answer = true; + } catch (err) { + // ENOENT is git missing, which is not an answer about the file. Anything else is + // git having run and said no. + answer = err.code === 'ENOENT' ? null : false; + } + trackedCache.set(real, answer); + return answer; +} + +/** + * Would writing this file commit local state into someone's repository? + * + * The question is about the **target**, not about where this code is running from, and + * that distinction is the bug this replaced. The old test asked whether the running + * package had a `.git`, which is true from a checkout and false from an installed + * package — so a registered path pointing into a checkout was refused in dev mode and + * silently written in normal mode. Switching a machine from `npm install -g .` to the + * published package leaves exactly such a path behind, and the next `compact` wrote a + * machine-specific digest into a tracked file. The same failure that shipped one + * machine's note count for twenty releases, reached from the other direction. + * + * Tracked-ness is the property that actually matters, so ask git directly. Resolved + * through `realpathSync` first because the path arrives as a symlink planted by + * `setup`: the link sits outside the repository even when its target is inside it, and + * asking about the link would answer "not tracked" and then write straight through it. + * + * Without git, fall back to the old package-root heuristic. It is narrower than the + * real question but it is what this shipped with, and a machine with no git also has + * no tracked file to damage. */ export function insideCheckout(file) { - if (!existsSync(join(PACKAGE_ROOT, '.git'))) return false; let real; - let root; try { real = realpathSync(file); + } catch { + return false; + } + + const tracked = isTracked(real); + if (tracked !== null) return tracked; + + if (!existsSync(join(PACKAGE_ROOT, '.git'))) return false; + let root; + try { root = realpathSync(PACKAGE_ROOT); } catch { return false; @@ -182,6 +233,11 @@ export function insideCheckout(file) { return real === root || real.startsWith(root + sep); } +/** Testing seam: the per-process cache would otherwise outlive a fixture repo. */ +export function resetTrackedCache() { + trackedCache.clear(); +} + export function writeSkillDescription(skillPath, description) { if (!existsSync(skillPath)) return false; if (insideCheckout(skillPath)) return false; diff --git a/test/integration.test.js b/test/integration.test.js index c61a718..6d90d79 100644 --- a/test/integration.test.js +++ b/test/integration.test.js @@ -19,7 +19,7 @@ const { searchNodes } = idx; const { neighborhood, applyBudget } = await import('../src/graph.js'); const { buildTree, buildDigest, buildCaptureNudge, renderTree, buildBrief, renderBrief } = await import('../src/digest.js'); -const { compact, writeSkillDescription, insideCheckout, skillNameFromPath } = +const { compact, writeSkillDescription, insideCheckout, skillNameFromPath, resetTrackedCache } = await import('../src/compact.js'); const stale = await import('../src/staleness.js'); const { setup, unlinkSkills, danglingSkillLinks, skillTargets, packagedSkillsDir, SKILLS } = @@ -1581,3 +1581,53 @@ test('brief runs as a command, scopes to the repo, and reports empty honestly', rmSync(home, { recursive: true, force: true }); } }); + +test('compact refuses a tracked file in any repo, not just the running package', () => { + // The bug this replaced: the guard asked whether the *running package* had a .git, + // which is true from a checkout and false from an installed package. So a registered + // path pointing into a checkout was refused in dev mode and silently written in + // normal mode. Switching a machine from `npm install -g .` to the published package + // leaves exactly such a path behind, and the next compact wrote a machine-specific + // digest into a tracked file -- observed on a real install, not hypothesised. + seed().close(); + const repo = mkdtempSync(join(tmpdir(), 'agent-memory-tracked-')); + const git = (...args) => + execFileSync('git', args, { cwd: repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); + git('init', '-q', '-b', 'main'); + git('config', 'user.email', 'test@example.com'); + git('config', 'user.name', 'Test'); + + const body = '---\nname: recall\ndescription: placeholder\n---\n\n# body\n'; + const tracked = join(repo, 'skills', 'recall', 'SKILL.md'); + mkdirSync(join(repo, 'skills', 'recall'), { recursive: true }); + writeFileSync(tracked, body, 'utf8'); + // Untracked, in the same repository. Writing it publishes nothing, so it is fair + // game -- the guard is about tracked-ness, not about being near a .git. + const untracked = join(repo, 'skills', 'scratch.prompt.md'); + writeFileSync(untracked, '---\nname: recall\ndescription: placeholder\n---\n\n# body\n', 'utf8'); + git('add', 'skills/recall/SKILL.md'); + git('commit', '-qm', 'add skill'); + + try { + resetTrackedCache(); + assert.equal(insideCheckout(tracked), true, 'a tracked file is refused'); + assert.equal(insideCheckout(untracked), false, 'an untracked file is writable'); + + // Reached through the symlink setup actually plants. Asking about the link would + // answer "not tracked" and then write straight through it into the repository. + const dir = mkdtempSync(join(tmpdir(), 'agent-memory-tlink-')); + const link = join(dir, 'SKILL.md'); + symlinkSync(tracked, link); + resetTrackedCache(); + assert.equal(insideCheckout(link), true, 'a symlink must be resolved before asking git'); + + const r = compact({ cfg: { ...DEFAULTS, skillPaths: [tracked, untracked, link] } }); + assert.equal(readFileSync(tracked, 'utf8'), body, 'the tracked file must be byte-identical'); + assert.ok(r.skipped.includes(tracked), 'and the refusal is reported, never silent'); + assert.ok(r.skills.includes(untracked), 'the untracked file still gets its description'); + rmSync(dir, { recursive: true, force: true }); + } finally { + rmSync(repo, { recursive: true, force: true }); + resetTrackedCache(); + } +});