From a8de9532a0837ddc37f04f1f3650a524ada03d7d Mon Sep 17 00:00:00 2001 From: Utkarsh Singh <6995377+vib795@users.noreply.github.com> Date: Sat, 29 Aug 2026 11:41:17 -0500 Subject: [PATCH] feat: route the capture signal to the model instead of to doctor (0.7.0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `captureGap` has known since 0.5 how far a repository has moved with nothing written down in it. That answer went only to `doctor` — a command a person runs on purpose, which is precisely the person who did not need telling. The signal never reached the one reader who could act on it mid-conversation. Tier 1 — the `remember` description is now regenerated from store state, the same mechanism that has always fed `recall`. It reads as a fact, not an instruction: "51 commits of history and nothing captured here" is something the model can weigh against what just happened, where "remember to capture things" is wallpaper it stops seeing by the third turn. It goes quiet on a covered repository, because a line that nags at a current store teaches the reader to discount it before the day it matters. `compact` now picks the text per skill rather than writing one digest to every registered path, which is what makes registering a second skill safe at all. Tier 2 — `agent-memory brief`, the mirror of `tree`, scoped for a writer. One read-only call before composing. It answers three things the skill previously guessed at: what is already here (dedup is by exact content hash, so a paraphrase becomes a second node), which ids are real (a missing `edges[].dst` is legal and silently never connects), and which types are empty. No hooks. Copilot has no equivalent to Claude Code's, and every repo-local instruction file lives in the user's project, which this package does not write to. The skill description and the invocation path are the two surfaces that work everywhere, so both tiers ride on those. Zero dependencies, as before. Nothing added to package.json. Also: - doctor checks the nudge against the same cap as the digest; over it, Tier 1 silently degrades to generic text and nothing said so - README described `skills/recall/SKILL.md` showing as modified after a clone install. `insideCheckout` has refused that write since 0.6.x — the docs described a bug that was already fixed 105 tests, up from 97. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014fSLBRUVVhAauWDuzJM4mc --- .claude-plugin/marketplace.json | 2 +- .claude-plugin/plugin.json | 2 +- ARCHITECTURE.md | 21 ++- README.md | 24 +-- package-lock.json | 4 +- package.json | 2 +- skills/remember/SKILL.md | 26 +++ src/cli.js | 33 +++- src/compact.js | 43 ++++- src/config.js | 1 + src/digest.js | 277 +++++++++++++++++++++++++++++++- src/setup.js | 31 +++- test/integration.test.js | 269 +++++++++++++++++++++++++++++-- 13 files changed, 691 insertions(+), 44 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 8bacf4c..ab84f19 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.6.5", + "version": "0.7.0", "author": { "name": "Utkarsh Singh", "url": "https://github.com/vib795" diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 552e0a0..3551f4b 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "agent-memory", "displayName": "agent-memory", - "version": "0.6.5", + "version": "0.7.0", "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/ARCHITECTURE.md b/ARCHITECTURE.md index caf2145..cca6e10 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -250,7 +250,8 @@ premium request budget behind it. ```mermaid flowchart TB subgraph t1["TIER 1 — standing cost, every chat"] - D["recall skill description
≤ 400 chars
'Durable project knowledge: 5 notes, 1 constraint
across agent-memory. Topics: … Use when …'"] + D["recall description
≤ 400 chars — what the store knows
'Durable project knowledge: 5 notes, 1 constraint
across agent-memory. Topics: … Use when …'"] + R["remember description
≤ 400 chars — what it is missing
'340 commits since anything was captured
for agent-memory. Use when …'"] end subgraph t2["TIER 2 — per-invocation, only when recall fires"] TR["routing tree
≤ 80 lines
type · id · title, ordered by
constraint-first then degree"] @@ -266,14 +267,28 @@ flowchart TB ``` Tier 1 is loaded into every chat whether or not memory is ever used, so it has to read -like a description rather than a document. It is composed from constraint count, then -repos by note count, then topics by edge degree — and on overflow it sheds the +like a description rather than a document. `recall`'s is composed from constraint count, +then repos by note count, then topics by edge degree — and on overflow it sheds the lowest-degree topics first, then repos, **one whole item at a time**. Cutting mid-word would leave the description looking corrupted, which is worse than saying less. Two pieces are structural and never dropped: the constraint count, and the closing "use when" clause — which is the entire reason an agent decides to invoke at all. +**Tier 1 has two occupants.** `recall`'s description advertises what the store knows; +`remember`'s advertises what it is missing. The second is `captureGap` — the distance +in commits from HEAD to the nearest capture, which the store has always been able to +compute — routed to the one surface that is loaded at the moment capture is worth +doing. Before 0.7 it went only to `doctor`, a command run by the person who least +needed telling. + +It is written as a *state*, never as an instruction: "340 commits since anything was +captured here" is a fact the model can weigh against what just happened in the +conversation, where "remember to capture things" is wallpaper it stops seeing. Code +supplies the timing signal; the model still decides whether anything durable happened. +On a covered repository it goes quiet and simply reports the count — a line that nags +at a current store teaches the reader to discount it before the day it matters. + Neither tier costs a premium request. A request is charged per prompt, not per tool call, so both ride inside a turn that was already paid for. diff --git a/README.md b/README.md index 04fc10f..80a4499 100644 --- a/README.md +++ b/README.md @@ -349,11 +349,12 @@ lives under `~/.agents/memory`, nothing is written into the projects you point i at, and there is no per-repo setup step. `cd` between projects freely: the store does not move, split, or reset. -One exception, and it is this repository rather than yours: if you installed from a -clone, `npm install -g .` symlinks rather than copies, so `compact` regenerating the -skill descriptions lands in your working tree and `skills/recall/SKILL.md` shows as -modified. That is generated state, and [From a clone](#from-a-clone) says so. No -project repository is ever written to. +One note, and it is about this repository rather than yours: if you installed from a +clone, `npm install -g .` symlinks rather than copies, so the skill links resolve back +into your working tree. `compact` detects that and refuses to write there — the files +are tracked, and one developer's note count committed and published is exactly what +happened for twenty releases. It reports them as skipped instead. No project repository +is ever written to. What the working directory changes is *scope*, never location. @@ -366,6 +367,7 @@ What the working directory changes is *scope*, never location. | `search` | whole store | nothing — full text hits every note in every repo | | `get` | whole store | the staleness line only; the note is found by id either way | | `tree` | whole store | **filters it** — defaults to the current repo | +| `brief` | whole store | **filters it** — the same scoping as `tree` | | `write` | whole store | **is stamped into the note** — see below | The current repo is `git rev-parse --show-toplevel` reduced to its directory name. @@ -403,6 +405,7 @@ Only `init` is a once-per-machine command, and `agent-memory setup` already ran | `init` | once, via `setup`. Again only to register extra skill paths | | `write` | every capture | | `tree`, `get`, `search` | every lookup | +| `brief` | every capture, before `write` — what is already known here | | `index` | repair only — `write` reindexes on every call. Run it after hand-editing or deleting notes, or after deleting `index.db` | | `compact` | occasionally. Nothing schedules it: no daemon, no cron, no hook | | `doctor` | after install, after an upgrade, and whenever something looks wrong | @@ -550,14 +553,15 @@ powershell -ExecutionPolicy Bypass -File .\install.ps1 # Windows Both are thin wrappers over `agent-memory setup`; the linking logic lives in `src/setup.js` so there is one implementation rather than three that drift. -Note that `npm install -g .` from a clone *symlinks* rather than copies, so -`compact` regenerates the description in your working tree and -`skills/recall/SKILL.md` will show as modified. That is expected — the description -is generated state, and the committed value is only a placeholder. +Note that `npm install -g .` from a clone *symlinks* rather than copies, so the skill +links point back into your working tree. `compact` will not regenerate a description +there — `skills/recall/SKILL.md` and `skills/remember/SKILL.md` are tracked files, and +their committed descriptions are deliberately generic placeholders. `compact` prints +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 (91 tests, no dependencies). CI runs it on Linux, +Run `npm test` for the suite (105 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 9090e47..80eb2ec 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@vib795/agent-memory", - "version": "0.6.5", + "version": "0.7.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@vib795/agent-memory", - "version": "0.6.5", + "version": "0.7.0", "license": "MIT", "bin": { "agent-memory": "src/cli.js" diff --git a/package.json b/package.json index cd1288a..c522016 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@vib795/agent-memory", - "version": "0.6.5", + "version": "0.7.0", "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/skills/remember/SKILL.md b/skills/remember/SKILL.md index 38da004..3415a38 100644 --- a/skills/remember/SKILL.md +++ b/skills/remember/SKILL.md @@ -79,6 +79,32 @@ most turns. --- +## Step 0 — Read what is already known (ONE terminal call) + +```bash +agent-memory brief +``` + +Read-only, safe on every shell including PowerShell, and it costs no request of its +own — it rides inside the turn you are already answering. + +It answers three things you would otherwise guess at: + +- **What is already here.** Deduplication is by exact content, so the same claim in + different words becomes a second node. If the brief already lists it, either say + nothing or update that note by its id. +- **Which ids are real.** An `edges[].dst` or `supersedes` pointing at an id you + invented is accepted and then silently never connects. Take targets from the brief. +- **Which types are empty.** A store with no `constraint` has not recorded what the + environment forbids, which is the type that saves a future session a wasted retry. + +If the brief lists an id under "already covered", that juncture was captured minutes +ago. Do not capture it again. + +Skip this step only when the user named exactly what to write and it is plainly new. + +--- + ## Step 1 — Select what is durable diff --git a/src/cli.js b/src/cli.js index bb2b2c3..28d9bd5 100755 --- a/src/cli.js +++ b/src/cli.js @@ -10,7 +10,9 @@ import { openDb, reindex, searchNodes, getNodeRow, markAccessed, nodeCount, hasFts, } from './index-db.js'; import { neighborhood, applyBudget } from './graph.js'; -import { buildTree, renderTree, buildDigest } from './digest.js'; +import { + buildTree, renderTree, buildDigest, buildCaptureNudge, buildBrief, renderBrief, +} from './digest.js'; import { compact, maybeCompact } from './compact.js'; import { staleness, currentRepo, reviewCandidates, captureGap } from './staleness.js'; import { setup as runSetup, unlinkSkills, danglingSkillLinks, SKILLS } from './setup.js'; @@ -121,6 +123,7 @@ const USAGE = `agent-memory — durable cross-repo knowledge for coding agents search [--limit N] full-text fallback when the tree misses write --from-json validated upsert; used by the skills [--source ] [--repo ] + brief [--repo ] what is already known here, before capturing compact dedup, decay, reindex, regenerate doctor preflight and health report export [--scope global|repo|all] knowledge worth carrying to another machine @@ -217,7 +220,7 @@ function cmdSetup() { if (r.compactError) { lines.push( '', - ` Skills are installed, but refreshing the /recall description failed: ${r.compactError}`, + ` Skills are installed, but refreshing the /recall and /remember descriptions failed: ${r.compactError}`, ' Run `agent-memory index` then `agent-memory compact` to retry just that step.', ); } @@ -286,6 +289,23 @@ function cmdTree(opts) { return { ok: true, ...result, gap, text: renderTree({ ...result, gap }) }; } +/** + * Tier 2 of the capture pipeline: what the store already knows, before writing to it. + * + * The counterpart to \`tree\`. Same scoping, same budget, opposite reader: \`tree\` tells an + * agent which note answers a question, this tells it which note it is about to write + * twice. One call, in a turn already paid for. + */ +function cmdBrief(opts) { + const cfg = loadConfig(); + const db = openDb(); + // \`--repo\` with no value means every repo, matching tree. Anything else names one. + const repo = opts.repo === true ? null : (opts.repo ?? currentRepo()); + const result = buildBrief(db, { repo, cfg }); + db.close(); + return { ok: true, ...result, text: renderBrief(result) }; +} + function cmdGet(opts) { const cfg = loadConfig(); const id = opts._[0]; @@ -481,6 +501,7 @@ function cmdCompact() { ...r.decayed.map((d) => `archived ${d.id}, last seen ${d.lastSeen}`), ...r.malformed.map((m) => `warning: unparseable ${m.path}`), `digest ${r.digestChars} chars`, + `capture nudge ${r.nudgeChars} chars`, ...r.skills.map((s) => `updated description in ${s}`), ...(r.skipped || []).map( (s) => `skipped ${s}: inside this package's git checkout, so the file is tracked` @@ -553,6 +574,11 @@ function cmdDoctor() { const digest = buildDigest(db, { cfg }); add('digest within cap', digest.length <= cfg.digestChars, `${digest.length}/${cfg.digestChars} chars`); + // The nudge shares the cap and the silent fallback: over it, Tier 1 quietly becomes + // the generic string, which reads exactly like a store with nothing to report. + const nudge = buildCaptureNudge(db, { cfg }); + add('capture nudge within cap', nudge.length <= cfg.digestChars, `${nudge.length}/${cfg.digestChars} chars`); + const registered = cfg.skillPaths || []; const missing = registered.filter((p) => !existsSync(p)); add( @@ -640,7 +666,7 @@ function cmdDoctor() { // Staleness is a report, not a failure. Being told about it is the whole feature. const advisory = new Set(['staleness', 'capture gap', 'skills linked']); const fatal = checks.filter((c) => !c.ok && !advisory.has(c.name)); - return { ok: fatal.length === 0, checks, stale, gap, digest, text }; + return { ok: fatal.length === 0, checks, stale, gap, digest, nudge, text }; } // --- dispatch --------------------------------------------------------------- @@ -974,6 +1000,7 @@ const COMMANDS = { init: cmdInit, index: cmdIndex, tree: cmdTree, + brief: cmdBrief, get: cmdGet, search: cmdSearch, write: cmdWrite, diff --git a/src/compact.js b/src/compact.js index 2fc28aa..b8ce852 100644 --- a/src/compact.js +++ b/src/compact.js @@ -1,11 +1,11 @@ import { readFileSync, existsSync, realpathSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; -import { join, sep } from 'node:path'; +import { join, sep, basename, dirname } from 'node:path'; import { loadConfig, paths } from './config.js'; import { listNotes, archiveNote, contentHash, nowIso, serializeNote } from './store.js'; import { atomicWrite } from './atomic.js'; import { openDb, reindex } from './index-db.js'; -import { buildDigest, buildTree, renderTree } from './digest.js'; +import { buildDigest, buildCaptureNudge, buildTree, renderTree } from './digest.js'; /** * Compaction is pure code. No model is involved, and none should be. @@ -203,9 +203,33 @@ export function writeSkillDescription(skillPath, description) { return true; } -/** Regenerate everything derived: ROUTING.md and each installed skill description. */ +/** + * Which skill a registered path belongs to. + * + * Derived from the path rather than stored beside it, because `setup` is what creates + * these paths and it only ever creates the two shapes below. Keeping `skillPaths` a + * flat list of strings means no config migration and no second source of truth about + * which skill is which — the layout already answers it. + */ +export function skillNameFromPath(p) { + const file = basename(p); + if (file === 'SKILL.md') return basename(dirname(p)); + const m = file.match(/^(.+)\.prompt\.md$/); + return m ? m[1] : null; +} + +/** + * Regenerate everything derived: ROUTING.md and each installed skill description. + * + * Two descriptions are generated, not one, and which a path receives is decided by the + * skill it belongs to. `remember` gets the capture nudge; everything else registered + * gets the digest. Before this, one text was written to every registered path, which is + * why `setup` could only ever register `recall` — handing `remember` the digest would + * have replaced a good description with a description of the wrong thing. + */ function regenerate(db, cfg) { const digest = buildDigest(db, { cfg }); + const nudge = buildCaptureNudge(db, { cfg }); const tree = buildTree(db, { all: true, cfg }); const routing = [ @@ -227,9 +251,18 @@ function regenerate(db, cfg) { skipped.push(p); continue; } - if (writeSkillDescription(p, digest)) skills.push(p); + const text = skillNameFromPath(p) === 'remember' ? nudge : digest; + if (writeSkillDescription(p, text)) skills.push(p); } - return { digest, digestChars: digest.length, routing: paths.routing, skills, skipped }; + return { + digest, + digestChars: digest.length, + nudge, + nudgeChars: nudge.length, + routing: paths.routing, + skills, + skipped, + }; } /** diff --git a/src/config.js b/src/config.js index 0bd84be..fb9b61b 100644 --- a/src/config.js +++ b/src/config.js @@ -102,6 +102,7 @@ export const DEFAULTS = { staleReviewCommits: 100, // above this, doctor flags it for review captureGapCommits: 50, // repo movement with no capture at all before it is worth saying compactThreshold: 10, // node-count delta that triggers an automatic compact + briefRecentMinutes: 120, // window the capture brief calls already covered }; // Written by the installer: every SKILL.md whose description compact regenerates. diff --git a/src/digest.js b/src/digest.js index 4dc1937..b6ef87e 100644 --- a/src/digest.js +++ b/src/digest.js @@ -1,4 +1,5 @@ -import { loadConfig } from './config.js'; +import { loadConfig, NOTE_TYPES } from './config.js'; +import { captureGap, currentRepo } from './staleness.js'; /** * Two-tier routing. @@ -12,6 +13,11 @@ import { loadConfig } from './config.js'; * * Neither tier costs a premium request. A request is charged per prompt, not per * tool call, so both of these ride inside a turn that was already paid for. + * + * Tier 1 has two occupants, not one. `recall`'s description advertises what the store + * knows; `remember`'s advertises what it is missing. Both are the same mechanism — a + * line of frontmatter that code regenerates and every conversation loads — pointed at + * opposite halves of the same problem. */ // Never dropped from either tier. A constraint is what stops an agent from burning @@ -126,6 +132,111 @@ export function buildDigest(db, { cfg = loadConfig() } = {}) { return out; } +// The routing clause for `remember`, and the mirror of USE_WHEN. It carries both the +// asked form and the unasked one, because the unasked one is the entire reason this +// description is regenerated at all: a memory that only grows when someone remembers +// to grow it stays thin. Never dropped, for the same reason USE_WHEN is never dropped. +const CAPTURE_WHEN = + 'Use when the user says remember this, save this or note this down — and invoke it ' + + 'unasked the moment a decision settles, a constraint surfaces, a root cause is ' + + 'found or a convention is agreed.'; + +const NUDGE_GENERIC = 'Capture durable project knowledge into a cross-repo memory graph.'; + +/** + * Notes that speak for this repository, counted the way `captureGap` scopes them. + * + * A node with no repos is global and applies everywhere, so it counts here; a node + * claiming other repos does not. This mirrors `captureGap`'s filter deliberately — + * two different answers to "does this note cover me" in one description would be a + * bug the reader could see. + */ +function repoScopedCount(db, repo, type = null) { + // Bound in SQL order: the type predicate precedes the repo one in the statement. + return db + .prepare(` + SELECT COUNT(*) AS c + FROM nodes n + WHERE n.archived = 0 ${type ? 'AND n.type = ?' : ''} + AND (NOT EXISTS (SELECT 1 FROM node_repos r WHERE r.node_id = n.id) + OR EXISTS (SELECT 1 FROM node_repos r WHERE r.node_id = n.id AND r.repo = ?)) + `) + .get(...(type ? [type, repo] : [repo])).c; +} + +/** + * Tier 1, second occupant: the `remember` skill description. + * + * `captureGap` has known since 0.5 how far a repository has moved with nothing written + * down in it, and that answer went only to `doctor` — a command a person runs on + * purpose, which is precisely the person who did not need telling. The signal never + * reached the one reader who could act on it mid-conversation. This routes it to the + * surface that is already loaded into every turn. + * + * Written as a *state*, never as an instruction. "340 commits since anything was + * captured here" is a fact the model can weigh against what just happened in the + * conversation; "remember to capture things" is wallpaper it stops seeing by the third + * turn. The distinction is the whole design: code supplies the timing signal, the model + * still decides whether anything durable actually happened. + * + * It goes quiet on a covered repository. A description that nags at a store which is + * already current teaches the reader to discount the line, and then it is worth nothing + * on the day it has something to say. + * + * Scoping caveat, stated because it is visible in the output: the gap is per repository + * and `compact` runs wherever the user happens to be, so this describes the repo where + * compaction last ran. That is why every variant names the repo out loud — a reader in + * a different tree can see the mismatch rather than act on a number that is not theirs. + * `maybeCompact` re-points it on the next write, so it self-corrects with use. + */ +export function buildCaptureNudge(db, { cfg = loadConfig(), cwd = process.cwd(), repo, gap } = {}) { + const g = gap !== undefined ? gap : captureGap(db, { cwd, cfg, repo }); + + const compose = (head) => { + const out = head ? `${head} ${CAPTURE_WHEN}` : `${NUDGE_GENERIC} ${CAPTURE_WHEN}`; + // Nothing here is a list, so there is no elastic middle to shed one item at a + // time. Over the cap, drop the whole head rather than truncate mid-sentence: a + // description that stops in the middle of a number reads as corrupted, and the + // routing clause is the part that must survive either way. + return out.length > cfg.digestChars ? `${NUDGE_GENERIC} ${CAPTURE_WHEN}` : out; + }; + + // Outside a repository there is no gap to report and no repo to name. Say what the + // skill is for and stop, rather than inventing a number. + if (!g) return compose(null); + + // Never captured in this repository. `captureGap` counts only notes carrying a + // `captured_sha`, so this is its answer to "has anyone written anything down here", + // and the commit branches below stay on that same definition. The broader count is + // reserved for the covered branches, where the question is how much knowledge applies + // here rather than how much of it was captured here. + if (g.notes === 0) { + // captureGap withholds its note on a repository too young for the absence to mean + // anything. Stay silent with it: nagging on commit three is how a signal gets + // discounted long before the day it matters. + return compose( + g.note ? `Nothing has ever been captured for ${g.repo} — ${g.commits} commits of history.` : null, + ); + } + + // Captured, but the repository has moved a long way since the most recent one. + if (g.note) return compose(`${g.commits} commits since anything was captured for ${g.repo}.`); + + // Current on commits, but blind in the type that matters most. `digest` privileges + // constraints and never drops them from Tier 1; a store holding none has not recorded + // the thing most likely to save a future session a wasted retry loop. + const notes = repoScopedCount(db, g.repo); + if (repoScopedCount(db, g.repo, PRIVILEGED) === 0) { + return compose( + `${notes} note${notes === 1 ? '' : 's'} for ${g.repo} and no constraint recorded — ` + + 'what this environment forbids has never been written down.', + ); + } + + // Covered. Report the state plainly and let the routing clause do the rest. + return compose(`${notes} note${notes === 1 ? '' : 's'} for ${g.repo}, capture is current.`); +} + /** * Tier 2: the routing tree, scoped. * @@ -207,3 +318,167 @@ export function renderTree(result) { if (result.gap?.note) out.push(result.gap.note); return out.join('\n'); } + +/** + * What each type is for, shown only where one is missing. + * + * Taken from the table in the remember skill so there is one wording, not two that + * drift. Printed against a zero count it answers the question the count raises: not + * "you have none of these" but "here is what one would have said". + */ +const TYPE_HINT = { + constraint: 'what the environment or the org forbids', + decision: 'what was chosen, why, and what was rejected', + convention: 'how this codebase does something, and the gotcha', + system: 'how a thing works, anchored to a path:line', +}; + +/** Notes per type under the same scoping the tree uses, zeros included. */ +function typeCounts(db, repo) { + const rows = repo + ? db + .prepare(` + SELECT n.type AS type, COUNT(DISTINCT n.id) AS c + FROM nodes n + LEFT JOIN node_repos r ON r.node_id = n.id + WHERE n.archived = 0 AND (n.scope = 'global' OR r.repo = ?) + GROUP BY n.type + `) + .all(repo) + : db.prepare('SELECT type, COUNT(*) AS c FROM nodes WHERE archived = 0 GROUP BY type').all(); + + // Seeded from NOTE_TYPES so a type with no rows reports 0 rather than going absent. + // The absent ones are the entire point of this section. + const counts = new Map(NOTE_TYPES.map((t) => [t, 0])); + for (const r of rows) counts.set(r.type, r.c); + return counts; +} + +/** + * Ids written recently enough that capturing them again would be a duplicate. + * + * The skill already forbids re-capturing a juncture that came up twice in one + * conversation, and that rule currently depends on the model remembering across a + * long turn. This makes it data instead. + * + * Recency is a proxy and is labelled as one: the CLI has no notion of a session, and + * inventing one would mean tracking state this tool deliberately does not keep. A + * window wide enough to cover the working session is the honest approximation. + */ +function recentlyCaptured(db, repo, cfg, now) { + // Same format store.js writes, so a lexicographic compare is a chronological one. + const cutoff = new Date(now - cfg.briefRecentMinutes * 60000) + .toISOString() + .replace(/\.\d{3}Z$/, 'Z'); + const rows = repo + ? db + .prepare(` + SELECT DISTINCT n.id AS id, n.updated AS updated + FROM nodes n + LEFT JOIN node_repos r ON r.node_id = n.id + WHERE n.archived = 0 AND n.updated >= ? AND (n.scope = 'global' OR r.repo = ?) + ORDER BY n.updated DESC, n.id + `) + .all(cutoff, repo) + : db + .prepare(` + SELECT id, updated FROM nodes + WHERE archived = 0 AND updated >= ? ORDER BY updated DESC, id + `) + .all(cutoff); + return rows.map((r) => r.id); +} + +/** + * Tier 2 of the capture pipeline: what `remember` reads before it composes. + * + * The mirror of `buildTree`, scoped for a writer rather than a reader. Without it the + * skill composes blind, and blind composition has three failure modes this answers + * directly: it rewords a note that already exists (dedup is by exact content hash, so + * a paraphrase becomes a second node), it invents an `edges[].dst` id that never + * connects because a missing dst is legal, and it cannot see which type the store is + * missing at the one moment it is about to write something. + * + * One call, inside a turn that was already paid for. That economy is why the brief is + * a command the skill runs rather than anything loaded standing — and it matters more + * on Copilot, where a second round trip is a second premium request. + * + * The list is `buildTree`'s, deliberately: two answers to "which notes speak for this + * repo" in one tool would be a bug the reader could see. + */ +export function buildBrief(db, { repo, cwd = process.cwd(), cfg = loadConfig(), gap, now = Date.now() } = {}) { + const here = repo === undefined ? currentRepo(cwd) : repo; + const g = gap !== undefined ? gap : here ? captureGap(db, { cwd, cfg, repo: here }) : null; + const tree = buildTree(db, { repo: here, cfg }); + const counts = typeCounts(db, here); + + return { + repo: here, + gap: g, + tree, + counts: Object.fromEntries(counts), + missing: NOTE_TYPES.filter((t) => counts.get(t) === 0), + recent: recentlyCaptured(db, here, cfg, now), + recentMinutes: cfg.briefRecentMinutes, + total: tree.total, + }; +} + +export function renderBrief(result) { + const { repo, gap, tree, counts, missing, recent, recentMinutes } = result; + const scope = repo ?? 'all repos'; + + // The header carries the same signal the remember description does, at the same + // moment it is being acted on. A brief that opens with a note count while the repo + // has moved 300 commits since anyone wrote anything is burying its own headline. + // Composed from the gap fields rather than reusing \`gap.note\` verbatim: that string + // names the repo, and the header already has, so borrowing it stutters. + const state = !repo + ? `${tree.total} notes in the store` + : gap?.note && gap.notes === 0 + ? `nothing captured here yet, over ${gap.commits} commits of history` + : gap?.note + ? `${gap.commits} commits since anything was captured here` + : `${tree.total} note${tree.total === 1 ? '' : 's'} here, capture is current`; + const out = [`# capture brief: ${scope} — ${state}`]; + + if (tree.lines.length) { + out.push('', 'already known here — write the gap, not these'); + const width = tree.lines.reduce((w, e) => Math.max(w, e.id.length), 0); + for (const e of tree.lines) { + out.push(`${e.type.padEnd(10)} ${e.id.padEnd(width)} ${e.title}${e.archived ? ' (archived)' : ''}`); + } + // Never silent. A truncated list that reads as the whole store is how a duplicate + // gets written against a note that was there the entire time. + if (tree.omitted.length) { + out.push(`${tree.omitted.length} more not shown, run agent-memory tree --all`); + } + out.push('', 'These ids are real. Use them as an `edges[].dst` or `supersedes` target;'); + out.push('an id you invent is accepted and then never connects to anything.'); + } + + if (missing.length) { + out.push('', 'nothing captured yet in these types'); + const width = missing.reduce((w, t) => Math.max(w, t.length), 0); + for (const t of missing) out.push(` ${t.padEnd(width)} ${TYPE_HINT[t]}`); + } + + const present = Object.entries(counts).filter(([, c]) => c > 0); + if (present.length) { + out.push('', `counts: ${present.map(([t, c]) => `${t} ${c}`).join(', ')}`); + } + + if (recent.length) { + // Written as the reason rather than the rule, because the rule is already in the + // skill and the model is being asked to apply it, not to learn it again. + out.push( + '', + `captured in the last ${recentMinutes} minutes, so already covered: ${recent.join(', ')}`, + ); + } + + if (!tree.lines.length && !recent.length) { + out.push('', 'The store holds nothing for this repository yet. Anything durable is new.'); + } + return out.join('\n'); +} diff --git a/src/setup.js b/src/setup.js index 4af9644..7b8a80f 100644 --- a/src/setup.js +++ b/src/setup.js @@ -200,14 +200,27 @@ export function unlinkSkills() { return { removed, kept }; } +/** + * Skills whose description `compact` regenerates from store state. + * + * `recall` advertises what the store knows. `remember` advertises what it is missing, + * which is the only form of that signal that reaches the model at the moment capture is + * worth doing — `captureGap` had computed it since 0.5 and sent it only to `doctor`, a + * command run by the one person who did not need telling. + * + * `handoff` is deliberately absent: it describes itself, and there is no store-derived + * state that would make its description more useful than the one it ships with. + */ +export const REGENERATED = ['recall', 'remember']; + /** * Install into every agent present on this machine, then build the store. * - * Only `recall` is registered for description regeneration. `compact` overwrites the - * description of every path it is given, and handoff and remember describe - * themselves; registering all three would replace two good descriptions with a third. - * Copies and prompt files register their own path, because rewriting the packaged - * original would never reach them. + * Only the skills in `REGENERATED` are registered, and `compact` now picks the text per + * skill rather than writing one digest to every path it is given. That is what makes + * registering a second skill safe; before it, doing so would have replaced a good + * description with a description of the wrong thing. Copies and prompt files register + * their own path, because rewriting the packaged original would never reach them. */ export function setup({ compactFn } = {}) { const targets = installableTargets(); @@ -241,9 +254,11 @@ export function setup({ compactFn } = {}) { } const skillPaths = [ - join(packagedSkillsDir(), 'recall', 'SKILL.md'), - ...copies.filter((c) => c.name === 'recall').map((c) => join(c.path, 'SKILL.md')), - ...installed.filter((i) => i.mode === 'prompt' && i.name === 'recall').map((i) => i.path), + ...REGENERATED.map((name) => join(packagedSkillsDir(), name, 'SKILL.md')), + ...copies.filter((c) => REGENERATED.includes(c.name)).map((c) => join(c.path, 'SKILL.md')), + ...installed + .filter((i) => i.mode === 'prompt' && REGENERATED.includes(i.name)) + .map((i) => i.path), ]; // Drop registrations whose file is gone before adding the current ones. Renaming // the checkout, moving it, or reinstalling under a different prefix each leave a diff --git a/test/integration.test.js b/test/integration.test.js index 1a0d50c..c61a718 100644 --- a/test/integration.test.js +++ b/test/integration.test.js @@ -13,12 +13,14 @@ const ROOT = mkdtempSync(join(tmpdir(), 'agent-memory-int-')); process.env.AGENT_MEMORY_HOME = ROOT; process.on('exit', () => rmSync(ROOT, { recursive: true, force: true })); -const { writeNote, listNotes, notePath, readNote } = await import('../src/store.js'); +const { writeNote, listNotes, notePath, readNote, archiveNote } = await import('../src/store.js'); const idx = await import('../src/index-db.js'); const { searchNodes } = idx; const { neighborhood, applyBudget } = await import('../src/graph.js'); -const { buildTree, buildDigest, renderTree } = await import('../src/digest.js'); -const { compact, writeSkillDescription, insideCheckout } = await import('../src/compact.js'); +const { buildTree, buildDigest, buildCaptureNudge, renderTree, buildBrief, renderBrief } = + await import('../src/digest.js'); +const { compact, writeSkillDescription, insideCheckout, skillNameFromPath } = + await import('../src/compact.js'); const stale = await import('../src/staleness.js'); const { setup, unlinkSkills, danglingSkillLinks, skillTargets, packagedSkillsDir, SKILLS } = await import('../src/setup.js'); @@ -353,6 +355,142 @@ test('compact regenerates ROUTING.md and the registered skill description', () = assert.ok(readFileSync(paths.routing, 'utf8').includes('auth-service')); }); +// --- capture nudge (Tier 1, second occupant) ----------------------------------- + +test('the capture nudge reports store state and always keeps its routing clause', () => { + // The whole point of regenerating this line is that it changes. A description that + // reads the same on an empty store and a covered one is wallpaper, and wallpaper is + // what the model stops seeing by the third turn. + const repo = mkdtempSync(join(tmpdir(), 'agent-memory-nudge-')); + 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 commit = (n) => { + writeFileSync(join(repo, 'a.txt'), `v${n}`, 'utf8'); + git('add', '.'); + git('commit', '-qm', `c${n}`); + }; + commit(0); + const first = git('rev-parse', 'HEAD'); + + const name = basename(repo); + const db = seed(); + const cfg = { ...DEFAULTS, captureGapCommits: 10 }; + const nudge = () => buildCaptureNudge(db, { cwd: repo, cfg, repo: name }); + const ROUTING = /Use when the user says remember this/; + + try { + stale.resetCache(); + + // A young repo with nothing captured stays quiet, exactly as captureGap does. + // Silence here is a claim that there is nothing to act on, so it has to be earned. + assert.doesNotMatch(nudge(), /commits/, 'must not nag on a three-commit repo'); + assert.match(nudge(), ROUTING, 'the routing clause ships in every variant'); + + for (let i = 1; i <= 15; i++) commit(i); + stale.resetCache(); + assert.match(nudge(), /Nothing has ever been captured for .* — 16 commits of history\./); + + // Captured once, long ago. The number is the distance to the nearest capture. + writeNote({ + id: 'nudge-note', type: 'system', title: 'Captured once', body: 'Long ago.', + repos: [name], captured_sha: first, + }); + idx.reindex(db); + stale.resetCache(); + assert.match(nudge(), /15 commits since anything was captured for /); + + // Current on commits. seed() carries a global constraint, which applies to every + // repo, so the covered branch is what should speak here. + writeNote({ + id: 'nudge-note-2', type: 'system', title: 'Captured now', body: 'Current.', + repos: [name], captured_sha: git('rev-parse', 'HEAD'), + }); + idx.reindex(db); + stale.resetCache(); + assert.match(nudge(), /capture is current\./); + assert.doesNotMatch(nudge(), /no constraint recorded/); + + // Covered on commits but holding no constraint: the type the digest privileges and + // never drops, and the one that stops a future session repeating a blocked approach. + archiveNote('constraint', 'no-external-db'); + idx.reindex(db); + stale.resetCache(); + assert.match(nudge(), /no constraint recorded — what this environment forbids/); + assert.match(nudge(), ROUTING); + + // Outside a repository there is no gap and no repo to name. Say what the skill is + // for rather than inventing a number about a tree we are not in. + const outside = buildCaptureNudge(db, { cfg, gap: null }); + assert.doesNotMatch(outside, /commits|notes? for/); + assert.match(outside, ROUTING); + + // Standing context cost, same ceiling as the digest. + for (const text of [nudge(), outside]) { + assert.ok(text.length <= cfg.digestChars, `nudge was ${text.length} chars`); + } + } finally { + db.close(); + rmSync(repo, { recursive: true, force: true }); + stale.resetCache(); + } +}); + +test('compact writes the nudge to remember and the digest to recall', () => { + // One text used to go to every registered path, which is why only recall could be + // registered. Sending remember the digest would describe the wrong thing entirely. + seed().close(); + const dir = mkdtempSync(join(tmpdir(), 'agent-memory-two-')); + const frontmatter = (n) => `---\nname: ${n}\ndescription: placeholder\n---\n\n# body\n`; + const recall = join(dir, 'recall', 'SKILL.md'); + const remember = join(dir, 'remember', 'SKILL.md'); + const prompt = join(dir, 'remember.prompt.md'); + for (const p of [recall, remember]) mkdirSync(join(p, '..'), { recursive: true }); + writeFileSync(recall, frontmatter('recall'), 'utf8'); + writeFileSync(remember, frontmatter('remember'), 'utf8'); + writeFileSync(prompt, frontmatter('remember'), 'utf8'); + + try { + const r = compact({ cfg: { ...DEFAULTS, skillPaths: [recall, remember, prompt] } }); + assert.notEqual(r.digest, r.nudge, 'the two descriptions must not be the same text'); + assert.equal(r.nudgeChars, r.nudge.length); + + assert.ok(readFileSync(recall, 'utf8').includes(`description: ${JSON.stringify(r.digest)}`)); + assert.ok(readFileSync(remember, 'utf8').includes(`description: ${JSON.stringify(r.nudge)}`)); + // Both install layouts route by name, so a Copilot prompt file gets it too. + assert.ok(readFileSync(prompt, 'utf8').includes(`description: ${JSON.stringify(r.nudge)}`)); + assert.ok(readFileSync(remember, 'utf8').includes('# body'), 'the body is untouched'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test('skillNameFromPath reads both install layouts and rejects neither-shape', () => { + // Derived from the path rather than stored beside it: setup creates these two shapes + // and nothing else, so the layout is already the answer and a second source of truth + // about which skill is which would be one more thing to drift. + assert.equal(skillNameFromPath(join('a', 'b', 'remember', 'SKILL.md')), 'remember'); + assert.equal(skillNameFromPath(join('a', 'b', 'recall', 'SKILL.md')), 'recall'); + assert.equal(skillNameFromPath(join('a', 'prompts', 'remember.prompt.md')), 'remember'); + assert.equal(skillNameFromPath(join('a', 'ROUTING.md')), null); +}); + +test('the shipped remember description is generic, not one machine capture gap', () => { + // The same backstop recall has. This description is regenerated on install, so the + // one in the repo is what every reader gets before their first compact -- and a note + // count or a commit distance in it is a false claim about a store they do not have. + const skill = fileURLToPath(new URL('../skills/remember/SKILL.md', import.meta.url)); + const m = readFileSync(skill, 'utf8').match(/^description: (.*)$/m); + assert.ok(m, 'remember must carry a description'); + const d = m[1]; + + assert.doesNotMatch(d, /\d+ notes?\b/, 'note counts describe one machine, not the reader'); + assert.doesNotMatch(d, /\d+ commits?\b/, 'a commit distance is one machine, not the reader'); + assert.match(d, /remember this/, 'the routing triggers must ship'); +}); + test('writeSkillDescription refuses a file without frontmatter', () => { const plain = join(ROOT, 'PLAIN.md'); writeFileSync(plain, '# no frontmatter\n', 'utf8'); @@ -694,18 +832,21 @@ test('a VS Code install gets prompt files, generated from the skills', () => { } }); -test('setup registers recall and nothing else', () => { +test('setup registers recall and remember, and never handoff', () => { seed().close(); const home = mkdtempSync(join(tmpdir(), 'agent-memory-home-')); process.env.AGENT_MEMORY_SKILLS_HOME = home; try { saveConfig({ skillPaths: [] }); const r = setup({}); - // compact overwrites the description of every registered path. handoff and - // remember describe themselves, so registering them would destroy both. - assert.deepEqual(r.skillPaths, [join(packagedSkillsDir(), 'recall', 'SKILL.md')]); + // Both are Tier 1 and both are regenerated from store state: recall advertises what + // the store knows, remember what it is missing. handoff describes itself and has no + // store-derived state, so registering it would destroy a good description. + assert.deepEqual(r.skillPaths, [ + join(packagedSkillsDir(), 'recall', 'SKILL.md'), + join(packagedSkillsDir(), 'remember', 'SKILL.md'), + ]); assert.ok(!JSON.stringify(loadConfig().skillPaths).includes('handoff')); - assert.ok(!JSON.stringify(loadConfig().skillPaths).includes('remember')); } finally { delete process.env.AGENT_MEMORY_SKILLS_HOME; rmSync(home, { recursive: true, force: true }); @@ -726,7 +867,10 @@ test('setup forgets a registered skill whose file is gone', () => { setup({}); const after = loadConfig().skillPaths; assert.ok(!after.includes(dead), `stale path survived: ${JSON.stringify(after)}`); - assert.deepEqual(after, [join(packagedSkillsDir(), 'recall', 'SKILL.md')]); + assert.deepEqual(after, [ + join(packagedSkillsDir(), 'recall', 'SKILL.md'), + join(packagedSkillsDir(), 'remember', 'SKILL.md'), + ]); } finally { delete process.env.AGENT_MEMORY_SKILLS_HOME; rmSync(home, { recursive: true, force: true }); @@ -1330,3 +1474,110 @@ test('export removes personal identifiers and says what it removed', () => { rmSync(home, { recursive: true, force: true }); } }); + +// --- capture brief (Tier 2 of the capture pipeline) ----------------------------- + +test('the brief shows real ids, the empty types, and what was just captured', () => { + // Without this the skill composes blind, and blind composition reworded a note that + // already existed into a second node -- dedup is by exact content hash, so a + // paraphrase is not caught. + const db = seed(); + try { + // Well past the recency window, so seed()'s notes are history rather than + // duplicates-in-waiting. A clock near today would put them inside it. + const now = Date.parse('2027-01-01T00:00:00Z'); + const b = buildBrief(db, { repo: 'repo-a', gap: null, now }); + const text = renderBrief(b); + + // Real ids, so an edges[].dst can point at something that exists. A missing dst is + // legal and simply never connects, which is why guessing one is worse than silence. + assert.match(text, /auth-service/); + assert.match(text, /no-external-db/); + assert.match(text, /edges\[\]\.dst/, 'the brief has to say why the ids are there'); + + // seed() holds system, decision and constraint -- convention is the gap. + assert.deepEqual(b.missing, ['convention']); + assert.match(text, /nothing captured yet in these types/); + assert.match(text, /convention {2}how this codebase does something/); + assert.doesNotMatch(text, /^ {2}system /m, 'a type that exists is not listed as missing'); + + assert.deepEqual(b.recent, [], 'notes older than the window are not called recent'); + assert.doesNotMatch(text, /already covered/); + } finally { + db.close(); + } +}); + +test('the brief calls a fresh note already covered, and forgets it once the window passes', () => { + // The skill forbids capturing the same juncture twice in one conversation, and that + // rule currently rests on the model remembering across a long turn. This is the same + // rule as data. Recency is an approximation of a session and is labelled as one. + const db = seed(); + try { + writeNote({ id: 'just-now', type: 'convention', title: 'Written this minute', body: 'x', repos: ['repo-a'] }); + idx.reindex(db); + + const fresh = buildBrief(db, { repo: 'repo-a', gap: null, now: Date.now() }); + assert.ok(fresh.recent.includes('just-now')); + assert.match(renderBrief(fresh), /already covered: .*just-now/); + + // Two hours on, at the default window, the same note is history rather than a + // duplicate risk -- re-capturing a juncture from last week is a legitimate update. + const later = buildBrief(db, { + repo: 'repo-a', gap: null, now: Date.now() + (DEFAULTS.briefRecentMinutes + 1) * 60000, + }); + assert.deepEqual(later.recent, []); + } finally { + db.close(); + } +}); + +test('the brief never truncates silently and stays on the tree budget', () => { + // The failure this prevents is precise: a clipped list reads as the whole store, so + // a duplicate gets written against a note that was there the entire time. + const db = seed(); + try { + for (let i = 0; i < 30; i++) { + writeNote({ id: `filler-${i}`, type: 'system', title: `Filler ${i}`, body: 'b', repos: ['repo-a'] }); + } + idx.reindex(db); + const cfg = { ...DEFAULTS, treeLines: 12 }; + const text = renderBrief(buildBrief(db, { repo: 'repo-a', cfg, gap: null })); + assert.match(text, /more not shown, run agent-memory tree --all/); + // The constraint survives any cap, here as everywhere else. + assert.match(text, /no-external-db/); + } finally { + db.close(); + } +}); + +test('brief runs as a command, scopes to the repo, and reports empty honestly', () => { + const home = mkdtempSync(join(tmpdir(), 'agent-memory-brief-')); + const cli = fileURLToPath(new URL('../src/cli.js', import.meta.url)); + const run = (...args) => + spawnSync(process.execPath, [cli, ...args], { + env: { ...process.env, AGENT_MEMORY_HOME: home }, encoding: 'utf8', + }); + try { + assert.equal(run('init').status, 0); + + const empty = run('brief'); + assert.equal(empty.status, 0, empty.stderr); + assert.match(empty.stdout, /capture brief/); + // All four types absent is the honest reading of an empty store, and the one a + // fresh install has to survive without looking broken. + for (const t of ['system', 'decision', 'convention', 'constraint']) { + assert.match(empty.stdout, new RegExp(`^ {2}${t}`, 'm'), `${t} missing from the empty brief`); + } + + const json = run('brief', '--json'); + assert.equal(json.status, 0, json.stderr); + const parsed = JSON.parse(json.stdout); + assert.equal(parsed.ok, true); + assert.equal(parsed.total, 0); + assert.deepEqual(parsed.missing, ['system', 'decision', 'convention', 'constraint']); + assert.equal(parsed.recentMinutes, DEFAULTS.briefRecentMinutes); + } finally { + rmSync(home, { recursive: true, force: true }); + } +});