From 45d7a1b4bdca9a824d2e44461ffdf0e45de694aa Mon Sep 17 00:00:00 2001 From: raymondchins Date: Mon, 27 Jul 2026 22:14:23 +0700 Subject: [PATCH 01/12] docs(benchmark): disclose the D+F skew and reconcile blast radius with EVAL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 98.3% headline is carried by two of its seven rows (repo dump and blast radius). Only the repo-dump skew was disclosed. Adds the excl-D+F column to every run — 89.8% ai-chatbot, 96.4% zod, 73.1% taxonomy, 93.7% pooled — so the routine-case figure is readable without recomputing it. Also reconciles the two files that looked like they contradicted each other: RESULTS.md prices scenario D against an agent that cats all 65 dependents, EVAL.md prices --relates against grep -l. Both are true; the cheaper baseline is cheaper because it is 59.9% precise. Stated from both sides. Validates chars/4 against o200k_base for the first time on the three pinned eval repos: absolute error is 1.1-12.5% (wider than the +/-10% claimed), but the published saved-% moves only 0.03-0.08pp. Caveat corrected to match. The tokenizer was installed outside the repo; package.json is unchanged. --- EVAL.md | 8 +++++++ README.md | 15 ++++++++----- benchmark/RESULTS.md | 51 +++++++++++++++++++++++++++++++++++++------- 3 files changed, 61 insertions(+), 13 deletions(-) diff --git a/EVAL.md b/EVAL.md index 6097419..1d46b6d 100644 --- a/EVAL.md +++ b/EVAL.md @@ -74,6 +74,14 @@ where naive grep returns a noisy superset (high recall, low precision) — and ` actually costs **more** tokens than `grep -l` because it returns the full blast radius (exports + imports + dependents + related), not just the file list. +> **Reconciling this with `RESULTS.md`'s 99.2% blast-radius row.** Both numbers are real; +> they price different baselines. `RESULTS.md` scenario D compares against an agent that +> `cat`s every dependent file — agentmap wins by ~99%. This eval compares against `grep -l`, +> which returns a file list and nothing else — agentmap loses on tokens. The list is cheaper +> because it is *less* correct: at **59.9%** precision, roughly 4 of every 10 paths on it are +> not dependents, and the agent pays for them on the next turn when it opens them. Neither +> file is the whole picture on its own; quote them together. + ### Per fixture | Repo | commit | def n | agentmap top1/top3 | grep top1/top3 | deps n | agentmap recall/prec | grep recall/prec | diff --git a/README.md b/README.md index 079646f..ce887c5 100644 --- a/README.md +++ b/README.md @@ -133,12 +133,17 @@ follows the chain and names the file that actually declares it. - **The win scales with the work.** The 63% and 11% rows are the floor. A *trivial single-file* lookup can cost **more** than `cat` + `grep` — taxonomy's file-import task hit **−313%**, and it stays in the table. -- **The 98.3% combined figure is skewed** by the whole-repo row (150 K vs 1 K). Excluding it, - the per-task average is ~32× rather than 58×. Both are real; the headline captures the - common worst case (repo dump on session start). +- **The 98.3% headline is carried by its two biggest rows** — repo dump (150,281 → 1,127) and + blast radius (81,038 → 616). Drop the repo dump and it's **96.9%**; drop both and it's + **89.8%** here, **93.7%** pooled across all three repos, and **73.1%** on the smallest one. + All of those are real — they answer different questions. The headline is the common worst + case: an agent dumping the repo at session start. - **`--relates` returns the full blast radius**, so it costs *more* than a bare `grep -l` file - list. Complete-and-correct over short-and-wrong — but it is a trade, stated in - [EVAL.md](./EVAL.md). + list. That's why the same command reads as 99.2% *saved* in the benchmark and *more + expensive* in the eval: the benchmark's baseline is an agent that `cat`s all 65 dependent + files, the eval's is a file list nobody reads. Against the list, agentmap trades tokens for + precision — 100% vs 59.9%, so ~4 in 10 files on the grep list don't belong. + Complete-and-correct over short-and-wrong, but it is a trade → [EVAL.md](./EVAL.md). - **Numbers are context-token volume**, not answer quality or wall-clock. - **Token counts are estimates** (`chars / 4`), applied identically to both sides. - **TypeScript/JavaScript only** (+ Vue SFC) — see [Scope & limitations](#scope--limitations). diff --git a/benchmark/RESULTS.md b/benchmark/RESULTS.md index ab70ede..4a5732f 100644 --- a/benchmark/RESULTS.md +++ b/benchmark/RESULTS.md @@ -6,14 +6,22 @@ raw files with `cat` / `grep` / `find`. Measured across **7 agent tasks on 3 rea public repos**, fully reproducible. Every number below is captured tool output — no hand-tuned figures. -| Repo | Files | Total saved | Standout task | -|------|------:|------------:|---------------| -| [vercel/ai-chatbot](https://github.com/vercel/ai-chatbot) | 154 | **98.3%** | reuse lookup 99.9% | -| [colinhacks/zod](https://github.com/colinhacks/zod) | 367 | **99.2%** | whole-repo map 99.8% | -| [shadcn-ui/taxonomy](https://github.com/shadcn-ui/taxonomy) | 125 | **96.0%** | reuse lookup 99.3% | +| Repo | Files | Total saved | Excl. D+F | Standout task | +|------|------:|------------:|----------:|---------------| +| [vercel/ai-chatbot](https://github.com/vercel/ai-chatbot) | 154 | **98.3%** | 89.8% | reuse lookup 99.9% | +| [colinhacks/zod](https://github.com/colinhacks/zod) | 367 | **99.2%** | 96.4% | whole-repo map 99.8% | +| [shadcn-ui/taxonomy](https://github.com/shadcn-ui/taxonomy) | 125 | **96.0%** | 73.1% | reuse lookup 99.3% | +| **Pooled** | 646 | **98.7%** | **93.7%** | — | Per-task peaks across the three repos: **whole-repo map 99.8%**, **reuse-before-rebuild 99.9%**, **blast-radius 99.2%**, **find-symbol 99%**. +**Read the second column before quoting the first.** Scenarios **D** (blast radius) and +**F** (map whole repo) are the two largest rows in every run, and they carry the totals — +see caveats 7 and 8. The `Excl. D+F` column is the same runs with both dropped: it is the +figure to quote for the *routine* case, and the totals are the figure for the case the tool +exists to prevent (an agent dumping a repo into context). Both columns come from the same +captured output below; neither is re-weighted. + ## Captured runs (`bench.mjs`) ### vercel/ai-chatbot — 154 files, sha `2becdb4` @@ -73,9 +81,18 @@ ts-morph-mappable repo. ## Honest caveats — read before quoting the number 1. **Token estimate is `chars / 4`** — a rough heuristic (the same one agentmap - uses), applied to **both** sides, so the saved-% *ratio* is robust even though - absolute token figures are ±10%. Raw char counts live in each run's `@@JSON@@` - footer. + uses), applied to **both** sides. **Checked once against a real tokenizer** + (`o200k_base`, 2026-07-27, on the three pinned `EVAL.md` repos): `chars/4` + **undercounts** real tokens by **1.1–12.5%** on raw source, and is within + **±4.6%** on agentmap's own `--map`/`--hubs` output. So the *absolute* counts + are looser than the ±10% this caveat used to claim — treat them as ±13%. + The *ratio* survives, which is the part that gets published: agentmap's output + tokenizes ~4–10% more densely than raw source, and recomputing the saved-% on + real tokens moves it by **0.03–0.08 percentage points** (zod 99.6 → 99.7, + hono 99.6 → 99.6, zustand 98.0 → 98.1). Quote the percentages, not the counts. + The tokenizer is **not** a dependency — it was installed once outside the repo + for this check and nothing in `package.json` changed. Raw char counts live in + each run's `@@JSON@@` footer. 2. **One result is negative, and we left it in.** taxonomy scenario **A = −313%**: for a *trivial single-file* dependency lookup, `cat` + a tiny `grep` is cheaper than agentmap's structured block. The tool pays off **at scale** (more files, @@ -93,6 +110,24 @@ ts-morph-mappable repo. there (unusual layout its ts-morph pass didn't pick up), so `--map` emitted nothing and the "100%" was an empty-output artifact, not a real saving. Only repos agentmap actually indexes are reported. +7. **Two scenarios carry the totals.** D and F are the two largest rows in all + three runs — on ai-chatbot they are 90% of the baseline by themselves. Dropping + both takes 98.3% → **89.8%** (ai-chatbot), 99.2% → **96.4%** (zod), and + 96.0% → **73.1%** (taxonomy); pooled, 98.7% → **93.7%**. That is not a + correction — a repo dump and a blast-radius walk are things agents really do, + and they are exactly where a map wins most. It does mean a total is a statement + about a *mix of tasks*, so quoting it for a single routine query overstates it. + The `Excl. D+F` column exists so nobody has to recompute this to check. +8. **Scenario D's baseline is `cat` every dependent, not `grep -l`.** The 99.2% is + measured against an agent that opens all 65 dependent files. Against an agent + that runs `grep -l` and reads nothing, `--relates` costs **more** tokens, not + fewer — [`../EVAL.md`](../EVAL.md) measures and states that. The two are not in + conflict; they price different baselines. What the eval adds is why the cheaper + baseline is not free: `grep -l` scores **59.9%** precision against agentmap's + **100%** (n=42), so roughly 4 in 10 files on that list are not dependents, and + the agent pays for them on the next turn when it reads them. ⚠️ `bench.mjs` does + **not** yet ship a `grep -l` baseline row for D — until it does, this caveat is + the reconciliation, not a measured third column. ## Reproduce From 1369540003eb87982e50b614ae27a69301455f04 Mon Sep 17 00:00:00 2001 From: raymondchins Date: Mon, 27 Jul 2026 22:19:10 +0700 Subject: [PATCH 02/12] fix(security): neutralise terminal control sequences in content-search output Content-search lines are the only raw repository bytes agentmap echoes back; everything else it prints is its own metadata. git grep -I skips binary files, so what survives is a TEXT file with escapes embedded on purpose: ESC[2J blanks the terminal, and a cursor-up run can overwrite the file:line prefix so a hit appears to come from a file it did not. C0 controls and DEL are now replaced with U+FFFD inside contentSearch(), so prose and --json get the same sanitised bytes. Tab and newline are kept - stripping tabs would mangle every tab-indented repo. Doing it at the source rather than at the two print sites matters because a --json consumer that parses and echoes a line is exposed to the same trick; MCP's JSON.stringify protects the model, not that consumer. Test asserts both surfaces and is proven to fail with the sanitiser stubbed out. A second test pins the tab carve-out. Also documents the two content-search guards in SECURITY.md - the MCP data fence was implemented but never written down. --- SECURITY.md | 7 +++++ agentmap.mjs | 16 +++++++++++- test/injection-safety.test.mjs | 47 ++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/SECURITY.md b/SECURITY.md index 1b66789..9494941 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -56,6 +56,13 @@ When no graph match is found, agentmap falls back to a live `git grep` over trac This denylist is a best-effort guard for conventionally-named secret files, not a guarantee — a secret stored in an unmatched filename can still be surfaced. (It deliberately does **not** match a bare `token` substring, which would over-exclude ordinary source like `tokenizer.ts`.) agentmap does **not** transmit file contents anywhere; all processing is local. +**Matched lines are untrusted repository bytes**, and are the only place agentmap echoes repository content back out — everything else it prints is its own metadata. Two guards apply: + +- **Terminal control sequences are neutralised.** C0 control characters and `DEL` are replaced with `U+FFFD` before the lines are printed or serialised, on both the prose and `--json` surfaces. Tab and newline are preserved. Without this, a text file carrying `ESC[2J` or a cursor-up run could blank the terminal or overwrite the `file:line` prefix so a hit appears to come from a file it did not. +- **Over MCP, the lines are fenced as data.** The `any` tool appends a second content block marking the result as raw untrusted repository content, so a planted "ignore previous instructions" in an ordinary source or markdown file reads to the model as data rather than as a command. Structural results (file / symbol / feature) are agentmap's own metadata and are not fenced; the CLI path writes to a terminal, not to a model, and is not fenced either. + +Neither guard makes untrusted repository content safe to execute. They reduce the two ways a matched line can act on something other than the reader's eyes. + ### Trust boundaries | Boundary | Notes | diff --git a/agentmap.mjs b/agentmap.mjs index 3e82d02..5c90e07 100755 --- a/agentmap.mjs +++ b/agentmap.mjs @@ -277,9 +277,23 @@ const SENSITIVE_EXCLUDES = [ // password.txt / passwords.json is excluded, not just foo.password.ts. ":(exclude,icase)*secret*", ":(exclude,icase)*credential*", ":(exclude,icase)*password*", ]; +// Neutralise terminal control sequences in content-search output. These lines are +// the ONLY place agentmap echoes raw repository bytes back out — everything else it +// prints is its own metadata. `git grep -I` already skips binary files, so what is +// left is a TEXT file with escapes deliberately embedded in it: an ESC[2J or a +// cursor-up run can blank the terminal or overwrite the file:line prefix, making a +// hit appear to come from a file it did not. Replaces C0 controls (keeping \t) and +// DEL with U+FFFD, so the line count and column alignment survive and the escape +// becomes visible instead of executable. Applied inside contentSearch() rather than +// at the two print sites, so prose AND --json get the same sanitised bytes — a JSON +// consumer that parses and echoes a line is exposed to exactly the same trick, and +// MCP's JSON.stringify escaping protects the model but not that consumer. +// eslint-disable-next-line no-control-regex +const CONTROL_CHARS = /[\x00-\x08\x0B-\x1F\x7F]/g; +const sanitizeContentLines = (s) => s.replace(CONTROL_CHARS, "�"); const contentSearch = (q) => { try { - return execFileSync("git", ["-c", "core.quotePath=off", "grep", "-F", "--untracked", "-n", "-i", "-I", "-e", q, "--", ".", ":!.claude/agentmap/", ...SENSITIVE_EXCLUDES], { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], maxBuffer: MAXBUF }).trim(); + return sanitizeContentLines(execFileSync("git", ["-c", "core.quotePath=off", "grep", "-F", "--untracked", "-n", "-i", "-I", "-e", q, "--", ".", ":!.claude/agentmap/", ...SENSITIVE_EXCLUDES], { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"], maxBuffer: MAXBUF }).trim()); } catch { return ""; } }; const currentSha = () => sh("git rev-parse --short HEAD"); diff --git a/test/injection-safety.test.mjs b/test/injection-safety.test.mjs index a9a1491..3707155 100644 --- a/test/injection-safety.test.mjs +++ b/test/injection-safety.test.mjs @@ -109,3 +109,50 @@ test('a literal that DOES exist still matches inertly (positive control)', () => assert.match(r.stdout, /SAFE_LITERAL_TOKEN/, "benign literal not found via content search"); cleanup(dir); }); + +test("terminal control sequences in matched source are neutralised, not echoed", () => { + // Content-search lines are the only raw repository bytes agentmap prints. A text + // file can carry ESC sequences that `git grep -I` happily passes through: ESC[2J + // clears the screen, and a cursor-up run can overwrite the file:line prefix so a + // hit looks like it came from somewhere else. Assert the escape never reaches + // stdout on EITHER surface — prose or --json — while the match still lands. + const dir = makeRepo({ + ...FIXTURE, + // \x1b[2J = clear screen, \x1b[1A = cursor up, \x07 = bell, \x7f = DEL. + "src/evil.ts": `export const E = "ESCAPE_PROBE_LITERAL\x1b[2J\x1b[1A\x07\x7fspoofed";`, + }); + gitInit(dir, { commit: true }); + + const prose = run(dir, "--any", "ESCAPE_PROBE_LITERAL"); + assert.equal(prose.status, 0, `probe literal should be found, got ${prose.status}: ${prose.stderr}`); + assert.match(prose.stdout, /evil\.ts/, "the match itself was lost while sanitising"); + assert.doesNotMatch(prose.stdout, /[\x00-\x08\x0B-\x1F\x7F]/, "a raw control character reached prose stdout"); + + const json = run(dir, "--any", "ESCAPE_PROBE_LITERAL", "--json"); + assert.equal(json.status, 0, `--json probe should exit 0, got ${json.status}: ${json.stderr}`); + const obj = JSON.parse(json.stdout); + assert.equal(obj.kind, "content", `expected a content result, got ${obj.kind}`); + const joined = obj.lines.join("\n"); + // Parsed back out, so this catches the JSON-escaped form too — JSON.stringify + // would have written a \\u001b escape and a consumer echoing the parsed value is exposed. + assert.doesNotMatch(joined, /[\x00-\x08\x0B-\x1F\x7F]/, "a raw control character survived into --json lines"); + assert.match(joined, /ESCAPE_PROBE_LITERAL/, "the match itself was lost in --json"); + assert.match(joined, /�/, "controls should be replaced with U+FFFD, not silently dropped"); + + cleanup(dir); +}); + +test("sanitising controls does not eat ordinary tabs in matched lines", () => { + // \t is deliberately NOT stripped — it is ordinary source indentation, and + // removing it would mangle every tab-indented repo's content-search output. + const dir = makeRepo({ + ...FIXTURE, + "src/tabbed.ts": `export function f() {\n\treturn "TAB_PROBE_LITERAL";\n}`, + }); + gitInit(dir, { commit: true }); + const r = run(dir, "--any", "TAB_PROBE_LITERAL", "--json"); + assert.equal(r.status, 0, `tab probe should exit 0, got ${r.status}: ${r.stderr}`); + const line = JSON.parse(r.stdout).lines.join("\n"); + assert.match(line, /\t/, "the tab was stripped along with the control characters"); + cleanup(dir); +}); From 223b16e820e131f901799737234e5bcca01d81a6 Mon Sep 17 00:00:00 2001 From: raymondchins Date: Mon, 27 Jul 2026 22:22:04 +0700 Subject: [PATCH 03/12] ci: run the suite on windows and macos, not just linux The 477 tests had only ever run on ubuntu, while the code carries real per-platform branches (win32 paths in skills/install.mjs, path separators through the resolver) and the docs target Windows. Node versions still fan out on Linux only; Windows and macOS get one pinned version each via include, so this adds two jobs rather than turning a 3-version matrix into nine. Guards the one file that cannot work there: post-commit-hook.test.mjs drives a POSIX /bin/sh script by spawning sh directly. Git for Windows runs that hook through its own bundled bash, so the hook is not what is uncovered on win32 - the harness is. Skipped with that reason attached rather than reshaped into something that passes without executing anything. install-smoke stays Linux-only and now says why in the file: every step is a bash script with absolute /tmp paths, and a half-ported PowerShell version that quietly skips a step is worse than a stated gap. --- .github/workflows/ci.yml | 28 ++++++++++++++++++++++++++-- test/post-commit-hook.test.mjs | 17 ++++++++++++----- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1945830..978ae70 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,15 +23,31 @@ permissions: jobs: test: - name: test (node ${{ matrix.node-version }}) + name: test (${{ matrix.os }}, node ${{ matrix.node-version }}) # The schedule trigger exists only to re-run `audit` against a moving # advisory DB; the code is unchanged between crons, so skip the rest. if: github.event_name != 'schedule' - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} + # Node versions fan out on Linux only; Windows and macOS get one pinned + # version each via `include`. A full 3x3 cross-product would be nine jobs to + # re-answer a question the Linux column already answers — what these two add + # is the PLATFORM, not another Node. + # + # They are here because the code has real per-platform branches (win32 paths + # in skills/install.mjs, path separators through the resolver, git behaviour + # differences) and the docs target Windows, yet every one of the 477 tests + # had only ever run on Linux. macOS is the maintainer's own platform and is + # exercised locally; Windows genuinely was not covered anywhere. strategy: fail-fast: false matrix: + os: [ubuntu-latest] node-version: [20, 22, 24] + include: + - os: windows-latest + node-version: 22 + - os: macos-latest + node-version: 22 steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -73,6 +89,14 @@ jobs: # So: pack, install from the tarball, and drive the real binary. Exit status is # NOT sufficient evidence here — exit 0 with empty stdout was the bug's exact # signature, so every step below asserts on OUTPUT. + # + # Deliberately Linux-only, unlike the `test` job above. Every step here is a + # bash script with `set -euo pipefail`, absolute /tmp paths and $GITHUB_ENV + # export syntax; on windows-latest the default shell is PowerShell, so porting + # this means `shell: bash` plus rewriting the paths, and a half-ported version + # that silently skips a step is worse than an honest gap. The Windows install + # path is therefore NOT covered — recorded here rather than left to be inferred + # from the matrix. install-smoke: name: install smoke (node ${{ matrix.node-version }}) if: github.event_name != 'schedule' diff --git a/test/post-commit-hook.test.mjs b/test/post-commit-hook.test.mjs index a9b3040..ba1e9da 100644 --- a/test/post-commit-hook.test.mjs +++ b/test/post-commit-hook.test.mjs @@ -14,6 +14,13 @@ import { makeRepo, writeFiles, gitInit, cleanup } from "./helpers.mjs"; const HERE = dirname(fileURLToPath(import.meta.url)); const HOOK_SRC = join(HERE, "..", "hooks", "post-commit"); +// hooks/post-commit is a POSIX /bin/sh script, and every test below drives it by +// spawning `sh` directly. `sh` is not a Windows platform binary — git for Windows +// ships its own bash and runs the hook through that, so what is unavailable on +// win32 is this harness, not the hook. Skipped explicitly rather than left to fail +// or, worse, rewritten into something that passes without executing anything. +const POSIX_ONLY = { skip: process.platform === "win32" ? "hooks/post-commit is a POSIX sh script; sh is unavailable on win32" : false }; + // Install the real hook into dir/.git/hooks/post-commit, overriding the // core.hooksPath=/dev/null that gitInit sets so the hook actually fires. function installHook(dir) { @@ -27,7 +34,7 @@ function installHook(dir) { // (the hook runs it via `node ./agentmap.mjs`, which treats .mjs as a module). const PAYLOAD = 'import{writeFileSync}from"node:fs";writeFileSync("PWNED","x")\n'; -test("planted ./agentmap.mjs is NOT executed by the post-commit hook by default", () => { +test("planted ./agentmap.mjs is NOT executed by the post-commit hook by default", POSIX_ONLY, () => { const dir = makeRepo({ "agentmap.mjs": PAYLOAD, "a.ts": "export const a = 1;\n" }); gitInit(dir); installHook(dir); @@ -41,7 +48,7 @@ test("planted ./agentmap.mjs is NOT executed by the post-commit hook by default" cleanup(dir); }); -test("AGENTMAP_HOOK_ALLOW_LOCAL=1 opts in to running ./agentmap.mjs", () => { +test("AGENTMAP_HOOK_ALLOW_LOCAL=1 opts in to running ./agentmap.mjs", POSIX_ONLY, () => { const dir = makeRepo({ "agentmap.mjs": PAYLOAD, "a.ts": "export const a = 1;\n" }); gitInit(dir); installHook(dir); @@ -70,7 +77,7 @@ const runHook = (dir) => execFileSync("sh", [join(dir, ".git", "hooks", "post-co cwd: dir, env: { ...process.env, AGENTMAP_HOOK_ALLOW_LOCAL: "1" }, stdio: "ignore", }); -test("a held lock makes the hook skip instead of piling on", () => { +test("a held lock makes the hook skip instead of piling on", POSIX_ONLY, () => { const dir = makeRepo({ "agentmap.mjs": PAYLOAD, "a.ts": "export const a = 1;\n" }); gitInit(dir); installHook(dir); @@ -82,7 +89,7 @@ test("a held lock makes the hook skip instead of piling on", () => { cleanup(dir); }); -test("a lock older than 10 minutes is cleared so refresh cannot stay dead", () => { +test("a lock older than 10 minutes is cleared so refresh cannot stay dead", POSIX_ONLY, () => { // Without this, one killed rebuild would disable auto-refresh permanently — // a worse failure than the leak it guards against, and a silent one. const dir = makeRepo({ "agentmap.mjs": PAYLOAD, "a.ts": "export const a = 1;\n" }); @@ -123,7 +130,7 @@ const WRAPPER = [ 'setInterval(()=>{},1000);\n', ].join(""); -test("the timeout reaps a hung runner's grandchild, not just the wrapper", () => { +test("the timeout reaps a hung runner's grandchild, not just the wrapper", POSIX_ONLY, () => { const dir = makeRepo({ "agentmap.mjs": WRAPPER, "a.ts": "export const a = 1;\n" }); const beat = join(dir, "HEARTBEAT"); try { From 374b20db02b95bb0d47038768d35d79417277168 Mon Sep 17 00:00:00 2001 From: raymondchins Date: Mon, 27 Jul 2026 22:28:31 +0700 Subject: [PATCH 04/12] fix: honest --symbols count, real Gemini path on Windows, no partial installs Four separate correctness gaps, all of the same shape: the tool reported success while doing something other than what it said. --symbols N: map.json persists only the top 80, so --symbols 200 printed "top 200 ranked symbols" above at most 80 rows. n beyond what was persisted now re-ranks from the cached file map - the same ts-morph-free recompute --map --focus already does - so 200 returns 200 where the repo has them. Verified on zod: 200 requested, 200 returned. Where a repo genuinely ranks fewer, the header says so and --json carries requested/shown/truncated. Gemini global install on Windows wrote to ~/.agents/, which is the Amp/legacy convention; Gemini CLI resolves global config under homedir()/.gemini on every platform and never reads .agents. Two files written where nothing loads them, reported as installed. The win32 branch is gone - one path everywhere. settings.json with a non-object "hooks" threw ".some is not a function" partway through a multi-platform install, after earlier platforms were already on disk. Now validated with an error naming the file and the key, and installSkill() preflights every platform in dry-run mode before the first write, so a bad config leaves the repo untouched instead of half-installed. JSONC comments were silently deleted on rewrite, since JSON.stringify cannot round-trip them. Still deleted - a real splice needs a JSONC writer - but now the user is told which file to check. 5 tests, README updated for the --symbols contract change. --- README.md | 10 ++++++ agentmap.mjs | 31 ++++++++++++++--- skills/install-helpers.mjs | 51 ++++++++++++++++++++++----- skills/install.mjs | 52 ++++++++++++++++++++-------- test/install-skill.test.mjs | 69 ++++++++++++++++++++++++++++++++++++- 5 files changed, 184 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index ce887c5..291e7b1 100644 --- a/README.md +++ b/README.md @@ -757,6 +757,16 @@ top 10 ranked symbols (Aider-style): 0.015034 lib/errors.ts → ErrorCode (TypeAliasDeclaration) ``` +`map.json` persists the top 80. Asking for more re-ranks from the cached map rather than +truncating, so `--symbols 200` really does return 200 where the repo has them. When a repo +has fewer ranked symbols than you asked for, the header says so and `--json` carries +`requested` / `shown` / `truncated`: + +``` +$ node agentmap.mjs --symbols 200 +top 62 ranked symbols (Aider-style) — asked for 200, this repo only ranks 62: +``` + ### `--map [--tokens N] [--focus ]` — token-budgeted ranked digest The token-budgeted digest (Aider's killer feature): a ranked, files-and-symbols summary diff --git a/agentmap.mjs b/agentmap.mjs index 5c90e07..51e3b79 100755 --- a/agentmap.mjs +++ b/agentmap.mjs @@ -1818,6 +1818,27 @@ function assemble(files, { target = MAP, extra = null, t0 = Date.now() } = {}) { return out; } +// Resolve `--symbols n` / the `symbols` MCP tool against a cached map. +// +// `map.json` persists only the top RANKED_SYMBOLS_LIMIT (80) symbols, so asking +// for more used to print "top 200 ranked symbols" above at most 80 rows: the +// count described the REQUEST, not the output, with nothing saying so. When n +// exceeds what was persisted, re-rank from the cached file map rather than +// truncating — rankSymbols() reads `files` only, which is the same ts-morph-free +// recompute the `--map --focus` path already performs, so this costs a graph walk +// over data already in memory and no rebuild. Falls back to the stored slice if +// the recompute cannot run (a pre-schema-7 cache with no `files`), because a +// short answer beats a thrown one. Callers report `shown` so a repo that simply +// has fewer ranked symbols than requested is distinguishable from a cap. +function rankedSymbolsFor(data, n) { + const stored = data.rankedSymbols || []; + if (n <= stored.length) return stored.slice(0, n); + try { + const full = rankSymbols(data.files || {}, null); + return full.length >= stored.length ? full.slice(0, n) : stored.slice(0, n); + } catch { return stored.slice(0, n); } +} + // Build the Aider-style identifier graph from the file map and return a // ranked list of { file, name, kind, rank }. `focus` (Set of paths) + // derived mentioned idents personalize the ranking when given. @@ -3400,9 +3421,9 @@ async function main() { } else if (has("--symbols")) { const data = ensureFresh(); const sn = parseInt(arg("--symbols") ?? "", 10); const n = Number.isFinite(sn) && sn > 0 ? sn : DEFAULT_SYMBOLS; - const syms = (data.rankedSymbols || []).slice(0, n); - out({ command: "symbols", symbols: syms.map((s) => ({ rank: s.rank, file: s.file, name: s.name, kind: s.kind })) }, () => { - console.log(`top ${n} ranked symbols (Aider-style):`); + const syms = rankedSymbolsFor(data, n); + out({ command: "symbols", requested: n, shown: syms.length, truncated: syms.length < n, symbols: syms.map((s) => ({ rank: s.rank, file: s.file, name: s.name, kind: s.kind })) }, () => { + console.log(`top ${syms.length} ranked symbols (Aider-style)${syms.length < n ? ` — asked for ${n}, this repo only ranks ${syms.length}` : ""}:`); for (const s of syms) console.log(` ${s.rank} ${s.file} → ${s.name} (${s.kind})`); }); } else if (has("--feature")) { @@ -4487,8 +4508,8 @@ function mcpQuery(name, args) { const data = mcpEnsureFresh(); const snRaw = (a.n != null && Number.isFinite(Number(a.n))) ? String(Math.trunc(Number(a.n))) : ""; const sn = parseInt(snRaw, 10); const n = Number.isFinite(sn) && sn > 0 ? sn : DEFAULT_SYMBOLS; - const syms = (data.rankedSymbols || []).slice(0, n); - return ok({ command: "symbols", symbols: syms.map((s) => ({ rank: s.rank, file: s.file, name: s.name, kind: s.kind })) }); + const syms = rankedSymbolsFor(data, n); + return ok({ command: "symbols", requested: n, shown: syms.length, truncated: syms.length < n, symbols: syms.map((s) => ({ rank: s.rank, file: s.file, name: s.name, kind: s.kind })) }); } default: return err(2, `unknown tool: ${name}`); diff --git a/skills/install-helpers.mjs b/skills/install-helpers.mjs index 3fb544c..2bc7426 100644 --- a/skills/install-helpers.mjs +++ b/skills/install-helpers.mjs @@ -43,14 +43,40 @@ function stripJsonComments(src) { return out; } +// Parse a settings file that may be JSONC. Returns { settings, hadComments } — +// `hadComments` is true when the file only parsed after comments were stripped, +// which is the caller's cue to warn: we re-serialise with JSON.stringify, so +// every comment in the user's file is dropped on write. Preserving them would +// mean a real JSONC-aware splice; warning is the honest cheap option, and it +// beats the previous behaviour of deleting a user's annotations in silence. function parseSettings(text, settingsPath) { - try { return JSON.parse(text) || {}; } + try { return { settings: JSON.parse(text) || {}, hadComments: false }; } catch { - try { return JSON.parse(stripJsonComments(text)) || {}; } + try { return { settings: JSON.parse(stripJsonComments(text)) || {}, hadComments: /\/\/|\/\*/.test(text) }; } catch { throw new Error(`${settingsPath} is not valid JSON — fix or remove it, then re-run`); } } } +// Fetch settings.hooks[event] as an array, creating it when absent and REJECTING +// a conflicting shape with a message that names the file and the key. +// +// `settings.hooks ??= {}` only fills null/undefined, so a settings.json where +// `hooks` is a string, a number or an array sailed through and blew up two lines +// later on `.some is not a function` — an opaque TypeError, thrown mid-install +// after earlier platforms had already been written to disk. The user saw a stack +// trace and a half-installed skill, with nothing pointing at the actual cause. +function hookArray(settings, event, settingsPath) { + if (settings.hooks === undefined || settings.hooks === null) settings.hooks = {}; + if (typeof settings.hooks !== "object" || Array.isArray(settings.hooks)) { + throw new Error(`${settingsPath}: "hooks" must be an object, found ${Array.isArray(settings.hooks) ? "an array" : typeof settings.hooks} — fix it, then re-run`); + } + if (settings.hooks[event] === undefined || settings.hooks[event] === null) settings.hooks[event] = []; + if (!Array.isArray(settings.hooks[event])) { + throw new Error(`${settingsPath}: "hooks.${event}" must be an array, found ${typeof settings.hooks[event]} — fix it, then re-run`); + } + return settings.hooks[event]; +} + export function readGuidanceSection() { if (!existsSync(GUIDANCE)) throw new Error(`packaged guidance missing: ${GUIDANCE}`); return readFileSync(GUIDANCE, "utf8"); @@ -74,14 +100,17 @@ export function installGeminiHooks(root, dryRun) { const NUDGE_CMD = `node "$GEMINI_PROJECT_DIR/.gemini/hooks/agentmap-nudge.mjs"`; const targets = [nudgeRel, settingsPath]; - let settings = {}; + let settings = {}, hadComments = false; if (existsSync(settingsPath)) { - settings = parseSettings(readFileSync(settingsPath, "utf8"), settingsPath); + ({ settings, hadComments } = parseSettings(readFileSync(settingsPath, "utf8"), settingsPath)); } - settings.hooks ??= {}; - settings.hooks.BeforeTool ??= []; + // Validates shape and throws a named error before anything is written. Runs on + // the dry-run path too, on purpose: installSkill() preflights every platform + // with dryRun=true so a broken settings.json fails before the FIRST file lands, + // instead of halfway through a multi-platform install. + const beforeTool = hookArray(settings, "BeforeTool", settingsPath); const matcher = "run_shell_command|grep|search"; - const already = settings.hooks.BeforeTool.some( + const already = beforeTool.some( (e) => e?.matcher === matcher && Array.isArray(e?.hooks) && e.hooks.some((h) => typeof h?.command === "string" && h.command.includes("agentmap-nudge")), ); @@ -92,7 +121,7 @@ export function installGeminiHooks(root, dryRun) { writeFileSync(nudgeDest, readFileSync(GEMINI_NUDGE_SRC, "utf8")); if (!already) { - settings.hooks.BeforeTool.push({ + beforeTool.push({ matcher, hooks: [{ name: "agentmap-nudge", @@ -102,6 +131,12 @@ export function installGeminiHooks(root, dryRun) { description: "Nudge structural searches toward agentmap", }], }); + // Say so before the rewrite, not after. JSON.stringify cannot round-trip + // JSONC, so the user's comments are about to be gone and the only kind thing + // to do is name the file they should check. + if (hadComments) { + console.warn(` ⚠ ${settingsPath} contained comments — JSON has no way to keep them, so they were dropped when agentmap added its hook. Re-add them if you need them.`); + } atomicWrite(settingsPath, JSON.stringify(settings, null, 2) + "\n"); } return targets; diff --git a/skills/install.mjs b/skills/install.mjs index 5eddded..3250db5 100644 --- a/skills/install.mjs +++ b/skills/install.mjs @@ -2,7 +2,7 @@ // --install-skill: skill files + always-on docs/hooks per platform (project or global). import { readFileSync, writeFileSync, existsSync } from "node:fs"; -import { homedir, platform as osPlatform } from "node:os"; +import { homedir } from "node:os"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; import { @@ -69,16 +69,17 @@ const PLATFORMS = { gemini: { label: "Gemini CLI", src: SKILL_MD, - dest: (root, globalScope) => { - if (!globalScope) return skillPath(root, false, ".gemini", "skills", "agentmap", "SKILL.md"); - if (osPlatform() === "win32") return skillPath(root, true, ".agents", "skills", "agentmap", "SKILL.md"); - return skillPath(root, true, ".gemini", "skills", "agentmap", "SKILL.md"); - }, - docs: (root, globalScope) => { - if (!globalScope) return join(root, "GEMINI.md"); - if (osPlatform() === "win32") return join(root, ".agents", "GEMINI.md"); - return join(root, ".gemini", "GEMINI.md"); - }, + // No win32 special case, deliberately. A previous version sent the GLOBAL + // Windows install to `~/.agents/` instead of `~/.gemini/`; `.agents` is the + // Amp/legacy convention (see the `agents` entry below), and Gemini CLI does + // not read it on any platform — it resolves global config under + // `homedir()/.gemini` everywhere. So the special case wrote two real files to + // a path nothing loads, and the install reported success: a Windows user got + // a silent no-op rather than an error. One path on every platform. + dest: (root, globalScope) => + skillPath(root, globalScope, ".gemini", "skills", "agentmap", "SKILL.md"), + docs: (root, globalScope) => + globalScope ? join(root, ".gemini", "GEMINI.md") : join(root, "GEMINI.md"), hooks: true, }, antigravity: { @@ -178,11 +179,11 @@ function installDocsForPlatform(cfg, { root, globalScope, dryRun, guidance, merg targets.push(dest); } -function installExtrasForPlatform(name, cfg, { root, globalScope, dryRun, targets }) { +function installExtrasForPlatform(name, cfg, { root, globalScope, dryRun, targets, quiet = false }) { if (cfg.hooks && !globalScope) { const hookTargets = installGeminiHooks(root, dryRun); for (const t of hookTargets) { - if (dryRun) console.log(` ${cfg.label} hooks: ${t}`); + if (dryRun) { if (!quiet) console.log(` ${cfg.label} hooks: ${t}`); } else { console.log(` ${cfg.label} hooks → ${t}`); targets.push(t); @@ -192,7 +193,7 @@ function installExtrasForPlatform(name, cfg, { root, globalScope, dryRun, target if (cfg.codexHooks && !globalScope) { const hookTargets = installCodexHooks(root, dryRun); for (const t of hookTargets) { - if (dryRun) console.log(` ${cfg.label} hooks: ${t}`); + if (dryRun) { if (!quiet) console.log(` ${cfg.label} hooks: ${t}`); } else { console.log(` ${cfg.label} hooks → ${t}`); targets.push(t); @@ -202,7 +203,7 @@ function installExtrasForPlatform(name, cfg, { root, globalScope, dryRun, target if (cfg.plugin && !globalScope) { const pluginTargets = installOpencodePlugin(root, dryRun); for (const t of pluginTargets) { - if (dryRun) console.log(` ${cfg.label} plugin: ${t}`); + if (dryRun) { if (!quiet) console.log(` ${cfg.label} plugin: ${t}`); } else { console.log(` ${cfg.label} plugin → ${t}`); targets.push(t); @@ -227,6 +228,27 @@ export function installSkill({ platforms: platformsArg = "all", project = true, if (dryRun) console.log(`--dry-run: would install agentmap skill (${scope} scope):`); + // Preflight: validate EVERY platform before writing the FIRST file. + // + // The loop below writes as it goes, so a platform whose config is malformed + // used to throw partway through and leave the earlier platforms installed and + // the later ones not — a state that is neither "installed" nor "unchanged", + // from a command the user will now re-run against a repo it has already + // half-modified. The extras are where that happens: they parse the user's + // settings.json / config.toml, which is the one input agentmap does not + // control. Running them in dry-run mode first performs the same parse and + // shape validation and writes nothing, so a bad file fails clean. + // + // Skipped when already dry-running (the loop below does this work anyway) and + // silenced via `quiet`, since these are checks, not a plan for the user to read. + if (!dryRun) { + for (const name of names) { + const cfg = PLATFORMS[name]; + if (cfg.projectOnly && globalScope) continue; + installExtrasForPlatform(name, cfg, { root, globalScope, dryRun: true, targets: [], quiet: true }); + } + } + for (const name of names) { const cfg = PLATFORMS[name]; if (cfg.projectOnly && globalScope) { diff --git a/test/install-skill.test.mjs b/test/install-skill.test.mjs index 14b16ef..bd0ccd7 100644 --- a/test/install-skill.test.mjs +++ b/test/install-skill.test.mjs @@ -3,7 +3,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; -import { makeRepo, run, cleanup } from "./helpers.mjs"; +import { makeRepo, run, runErr, cleanup } from "./helpers.mjs"; const PKG_VERSION = JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf8")).version; @@ -139,3 +139,70 @@ test("--install-skill is idempotent", () => { assert.equal(run(dir, "--install-skill", "--platform", "claude").status, 0); cleanup(dir); }); + +// --- installer robustness: a malformed settings.json must not half-install ---- + +test("a non-object hooks key fails with a named error, not an opaque TypeError", () => { + // `settings.hooks ??= {}` only fills null/undefined, so a string sailed through + // and threw ".some is not a function" two lines later. + const dir = makeRepo({ ".gemini/settings.json": JSON.stringify({ hooks: "nope" }, null, 2) }); + const r = run(dir, "--install-skill", "--platform", "gemini"); + assert.notEqual(r.status, 0, "a malformed settings.json should fail the install"); + const msg = r.stdout + r.stderr; + assert.match(msg, /\.gemini\/settings\.json/, "the error does not name the offending file"); + assert.match(msg, /"hooks" must be an object/, "the error does not name the offending key"); + assert.doesNotMatch(msg, /is not a function/, "still throwing the opaque TypeError"); + cleanup(dir); +}); + +test("a non-array hooks.BeforeTool fails with a named error", () => { + const dir = makeRepo({ ".gemini/settings.json": JSON.stringify({ hooks: { BeforeTool: 42 } }, null, 2) }); + const r = run(dir, "--install-skill", "--platform", "gemini"); + assert.notEqual(r.status, 0, "a malformed hooks.BeforeTool should fail the install"); + assert.match(r.stdout + r.stderr, /"hooks\.BeforeTool" must be an array/, "the error does not name the offending key"); + cleanup(dir); +}); + +test("a malformed config for ONE platform installs nothing for the others", () => { + // The whole point of the preflight. Before it, `--install-skill` (all platforms) + // wrote Claude/Cursor/Codex first and only then hit Gemini's broken settings.json, + // leaving a repo that was neither installed nor untouched. + const dir = makeRepo({ + "src/index.ts": "export function x() { return 1; }", + ".gemini/settings.json": JSON.stringify({ hooks: "nope" }, null, 2), + }); + const r = run(dir, "--install-skill"); + assert.notEqual(r.status, 0, "install should fail while any platform's config is malformed"); + for (const p of [ + join(dir, ".claude", "skills", "agentmap", "SKILL.md"), + join(dir, ".cursor", "rules", "agentmap.mdc"), + join(dir, ".codex", "skills", "agentmap", "SKILL.md"), + join(dir, ".opencode", "skills", "agentmap", "SKILL.md"), + ]) { + assert.equal(existsSync(p), false, `partial install: ${p} was written despite the failure`); + } + cleanup(dir); +}); + +test("comments in settings.json are reported as dropped, not deleted in silence", () => { + // JSON.stringify cannot round-trip JSONC. The comments go; the user gets told. + const dir = makeRepo({ + ".gemini/settings.json": '{\n // keep an eye on this\n "theme": "dark"\n}\n', + }); + const r = runErr(dir, "--install-skill", "--platform", "gemini"); + assert.equal(r.status, 0, r.stderr); + const msg = r.stdout + r.stderr; + assert.match(msg, /contained comments/i, "no warning that JSONC comments were dropped"); + const after = JSON.parse(readFileSync(join(dir, ".gemini", "settings.json"), "utf8")); + assert.equal(after.theme, "dark", "the surrounding settings were not preserved"); + assert.ok(Array.isArray(after.hooks?.BeforeTool), "the hook was not actually registered"); + cleanup(dir); +}); + +test("a comment-free settings.json triggers no comment warning", () => { + const dir = makeRepo({ ".gemini/settings.json": JSON.stringify({ theme: "dark" }, null, 2) }); + const r = runErr(dir, "--install-skill", "--platform", "gemini"); + assert.equal(r.status, 0, r.stderr); + assert.doesNotMatch(r.stdout + r.stderr, /contained comments/i, "false comment warning on plain JSON"); + cleanup(dir); +}); From 65bc851b10b07c4bb1eba9ff0510646de30b7c84 Mon Sep 17 00:00:00 2001 From: raymondchins Date: Mon, 27 Jul 2026 22:34:55 +0700 Subject: [PATCH 05/12] docs(roadmap): close three Batch 3 perf items with measurements None of the three produced code, and two of them should not. Prune rankSymbols cross-product: refuted on five repos (49-409 files, including a Next.js app, the shape most likely to duplicate export names). Largest edge list is 2,533; the most definers any identifier attracts is 17. Pruning at defCount>20 drops zero edges everywhere, and pruning low enough to matter would delete 78.7% of zod's graph including real identifiers. Two of the item's premises were already false: default is excluded from references, and identMul's RARE_PENALTY already discounts the case it wanted removed. Memory ceiling: the rescoped heapUsed sampler was built, measured and reverted. Under a forced 150MB ceiling on zod, a probe firing every iteration logged exactly ONE sample before the OOM - the process dies inside a single getExportedDeclarations(), finer than one loop iteration - and that last sample read 42% of the limit, because V8 collects harder until it gives up. No rate and no threshold can catch it. A guard that never fires still reads as protection, so it is gone; agentmap.mjs is byte-identical. The measured envelope and the --max-old-space-size remedy go to README Troubleshooting instead. Post-commit hook: the lock half already shipped (atomic mkdir, 10-minute orphan clear, process-tree kill). The incremental half stays gated on Tier 2 becoming default-on - it is still experimental with a known reparse tail, and this hook runs on every commit in every consumer repo. --- ROADMAP.md | 63 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 48ef074..314297f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -232,10 +232,20 @@ behavior into a real competitive claim vs CodeGraph's 2s sync. `./types` → `types.d.ts`, and for directory imports resolving via a nested `package.json` `"main"`. Trying ts-morph first with `resolveSpec` as fallback is already the correct design. *(performance/high)* -- [ ] **Incremental post-commit rebuild + lock** — `hooks/post-commit:67`: the - hook re-parses the entire repo on every commit and concurrent rebuilds duplicate - work with no locking. Diff `HEAD~1..HEAD` and re-parse only changed files + their - direct dependents; add a lockfile / compare-and-skip on in-progress HEAD build. +- [~] **Incremental post-commit rebuild + lock** — **LOCK DONE; incremental + deliberately NOT taken yet.** The locking half shipped: `hooks/post-commit` holds + a single-instance lock via atomic `mkdir` (`:92`), clears a lock orphaned by a + killed run after 10 minutes (`:95`) so one bad commit cannot wedge refresh + permanently, and caps the run with a process-tree kill. Concurrent rebuilds no + longer duplicate work. + The incremental half is **gated on Tier 2 going default-on**, and should stay + gated. Tier 2 (`AGENTMAP_INCREMENTAL=1`) is still EXPERIMENTAL because three + adversarial rounds left a residual isolated-reparse tail (`.d.ts` edges, + package.json `exports`, barrel+target). This hook ships to every consumer and runs + on every commit they make, so wiring it to an opt-in-because-not-yet-trusted path + would push exactly that tail onto people who never opted in — and a wrong map + written by a background hook is the hardest kind to notice. Revisit when Tier 2's + tail closes and it becomes the default dirty path. *(performance/medium — depends on Batch 2 incremental machinery)* - [ ] **Memory ceiling** — ⚠ **THE REMEDY IN THE ORIGINAL ITEM IS REFUTED. Do not implement it.** Measured on content-os (393 files), four loop variants doing @@ -256,19 +266,50 @@ behavior into a real competitive claim vs CodeGraph's 2s sync. closure (~300MB, ~1,800 extra program files on content-os) dominates and is independent of repo size. A file-count envelope would cry wolf on small dep-heavy repos and stay silent on the big repo it exists for. - **Still open, rescoped:** sample real `heapUsed` during the parse and print one - actionable warning (with the `--max-old-space-size` fix) before an OOM kills the - build with no map at all; document the measured envelope. *(performance/medium)* + **The rescoped remedy is now ALSO refuted — built, measured, reverted.** Sampling + `heapUsed` against `getHeapStatistics().heap_size_limit` during the parse loop + cannot fire in time, for two independent reasons measured on zod (409 files) under + a forced `--max-old-space-size=150`: + - **The fatal allocation is inside ONE file's work, not spread across files.** With + a probe printing on every iteration, the process died having logged **exactly one + sample** — it OOM'd during the first file's `getExportedDeclarations()`. Sampling + every 64 files logged **zero** samples before death. No sampling rate helps when + the granularity of the blow-up is finer than one loop iteration. + - **The reading immediately before death is not elevated.** That single sample read + `heapUsed` **104MB against a 246MB limit — 42%**, nowhere near any threshold + worth warning on. V8 keeps `heapUsed` low by collecting harder right up until it + gives up, so the ratio is flat and then the process is gone. + A guard that never fires is worse than none: it reads as protection in the source + and in a review. The implementation was therefore reverted (`agentmap.mjs` is + byte-identical to before it), and what survives is the half that is real — the + measured envelope and the `--max-old-space-size` remedy, documented in the README + Troubleshooting section where a user hitting the OOM will search for it. + **Still open:** nothing in-process. A supervisor that spawns the build and maps + exit 134 / `SIGABRT` to the remedy would work, and `mcp.mjs` already spawns + agentmap so it could do this for the MCP path — but the plain CLI has no parent, + and adding one is an architecture change, not a warning. *(performance/medium)* - [x] **Cap unbounded symbol matches** — DONE. `--find`/`--any` symbol matches are ranked by the containing file's PageRank and capped to `SYMBOL_MATCH_LIMIT` (50), with a "showing top N of M by pagerank — narrow your query" footer in prose and `total`/`shown`/`truncated` (`--find`) / `symbolsTotal`/`symbolsTruncated` (`--any`) in JSON. Ranking keeps the important matches when truncated. *(performance/medium)* -- [ ] **Prune rankSymbols cross-product** — `agentmap.mjs:736`: refs×defs edge - list per identifier is quadratic on duplicated export names. Skip identifiers - whose definer count exceeds a threshold (near-zero signal after the 0.1 - multiplier) or aggregate into per-defFile summary edges. *(performance/low)* +- [x] **Prune rankSymbols cross-product** — **CLOSED: REFUTED. Do not implement.** + Measured on five repos (agentmap 77 files, zustand 49, hono 385, zod 409, + a 392-file Next.js app) by replaying the edge-building loop over each cached map. + The cross-product is not quadratic in practice: the largest edge list is **2,533** + (hono), and the most definers any single identifier attracts is **17**. Pruning at + `defCount > 20` drops **0 edges on every one of the five**, so the threshold the + item implies is a no-op. Pruning low enough to matter is actively harmful — + `defCount > 5` would drop **78.7% of zod's graph**, including `util` + (12 definers × 34 referencing files), which is a real identifier the ranking wants. + The app-shaped repo, the case most likely to duplicate export names, peaked at + **3** definers (`NotificationsPage`) and 1,292 edges. + Two premises were wrong: `default` is already excluded from references + (`agentmap.mjs:1859`), which removes the one identifier that would genuinely fan + out, and `identMul`'s `RARE_PENALTY` already discounts the high-definer case it + proposed to delete. The real cost driver is `getExportedDeclarations()` at + ~O(N^2.7), recorded in the wall-clock-budget item above. *(performance/low)* **Acceptance:** a second query on an unchanged dirty tree does not re-parse; a pathological deep-chain repo finishes within the budget with skipped files From 017c6e1a8713490b26cb335d82f9a77a42846b49 Mon Sep 17 00:00:00 2001 From: raymondchins Date: Mon, 27 Jul 2026 22:43:55 +0700 Subject: [PATCH 06/12] test: assert ranking ORDER, isolate the env, add typecheck and coverage gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ranking quality had no guard. determinism.test.mjs proves two builds agree and that a known file appears somewhere in hubs — invert the hub comparator and both assertions still pass. Proven: with .sort() reversed, determinism reports 2/2 green while the new ranking-quality suite fails 3 of 5. Fixture is a constructed star (in-degrees 5/2/0) so first place, monotonicity and leaf-vs-hub are all knowable in advance, plus the same check one level down on --symbols. Env isolation: the two --global tests resolved paths against whatever $HOME the runner had, and now go through runWithHome() and assert the fake home appears in the output. git() no longer inherits the developer's ~/.gitconfig or /etc/gitconfig — GIT_CONFIG_GLOBAL/SYSTEM point at a path that does not exist, rather than /dev/null, which is not a path on the Windows runner just added. Typecheck gate: the roadmap assumed typescript already came via ts-morph. It does not - @ts-morph/common vendors it with no tsc bin - so this adds typescript and @types/node as DEV dependencies. The near-zero-deps rule governs runtime deps and the tarball; neither ships (package.json files is an allowlist, and npm pack --dry-run confirms). strict is off with a measured reason recorded in jsconfig.json: strict reports 505 errors, 272 of them 'annotate this parameter'. Non-strict reported 2, both real drift - a JSDoc @type for PLATFORMS that had fallen behind the object, and a dynamic import TS could not follow. Both fixed. Verified jsconfig.json does not perturb agentmap's own map: byte-identical with and without it. Coverage floor uses node --test's built-in coverage, not c8 - one less dep. Measured 93.77% lines / 76.12% branches; floors set at 90/70 so the gate is a regression alarm rather than a target that reddens on ordinary work. Own CI job pinned to Node 24 because the threshold flags do not exist on Node 20. ESLint deliberately NOT added - reasoning recorded in ROADMAP. --- .github/workflows/ci.yml | 37 ++++ CONTRIBUTING.md | 17 +- agentmap.mjs | 5 +- jsconfig.json | 29 +++ package-lock.json | 400 +++++++++++++++++++++++++++++++++- package.json | 10 +- skills/install.mjs | 1 + test/helpers.mjs | 22 +- test/install-skill.test.mjs | 10 +- test/ranking-quality.test.mjs | 103 +++++++++ 10 files changed, 623 insertions(+), 11 deletions(-) create mode 100644 jsconfig.json create mode 100644 test/ranking-quality.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 978ae70..304693b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,6 +61,14 @@ jobs: - name: Install dependencies run: npm ci + # checkJs over ~250KB of hand-written JS — the only static analysis that + # reads this codebase, since none of it is TypeScript. Deliberately runs on + # every OS: the two errors it caught when introduced were a stale JSDoc type + # and an unfollowable dynamic import, and path-shaped mistakes are exactly + # the kind that differ per platform. + - name: Typecheck + run: npm run typecheck + # Black-box suite — drives the real CLI as a subprocess against temp repos. # Runs via npm so CI uses the same glob set as `npm test` locally # (test/*.test.mjs alone silently skipped the test/vue-sfc/ suite). @@ -189,6 +197,35 @@ jobs: echo "generatedSha $BEFORE -> $AFTER" test "$AFTER" != "$BEFORE" || { echo "::error::post-commit hook did not refresh the map — auto-refresh is dead"; exit 1; } + # Coverage floor, so a shipped file that nothing executes stays visible. + # + # Uses node --test's own coverage rather than c8: the thresholds land natively + # from Node 22, and the near-zero-deps rule is easier to keep than to argue with. + # That is also why this is its own job pinned to one version — the flags do not + # exist on Node 20, which the test matrix still supports. + # + # Floors sit BELOW the measured 93.77% lines / 75.92% branches on purpose. They + # are a regression alarm, not a target: a gate set at the current number reddens + # on ordinary work and gets raised until someone stops reading it. + coverage: + name: Coverage floor + if: github.event_name != 'schedule' + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Setup Node + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: 24 + + - name: Install dependencies + run: npm ci + + - name: Run tests under coverage + run: npm run coverage + # Audit for high-severity vulnerabilities in the dependency tree. Deliberately # its own job rather than a step in the matrix above: it depends only on the # lockfile, so running it per Node version was three identical checks, and a diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3cd45b9..ebde1ea 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -120,9 +120,22 @@ When you touch caching, building, or the schema: ## Submitting a PR 1. For anything non-trivial, open (or link) an issue describing the change first. -2. Branch, make the change, run `npm test` — all green on Node 20+. +2. Branch, make the change, then run all three gates — CI runs the same ones: + + ```bash + npm test # black-box suite, all green on Node 20+ + npm run typecheck # checkJs over the .mjs sources (see jsconfig.json) + npm run coverage # suite again with the line/branch floor enforced + ``` + + `typecheck` is intentionally non-strict — `jsconfig.json` explains what that + buys and what it gives up. `coverage` needs Node 22+ for the threshold flags; + `npm test` alone is fine on Node 20. 3. Tests are dependency-free black-box drivers over throwaway git repos (see - `test/helpers.mjs`). New behavior needs a test in that style. + `test/helpers.mjs`). New behavior needs a test in that style. Assert what the + output *should be*, not merely that two runs agree — `test/determinism.test.mjs` + passes unchanged with the hub comparator inverted, which is why + `test/ranking-quality.test.mjs` exists. 4. Keep the diff minimal and the output byte-identical for existing commands — unless the change *is* the output (then call it out). 5. Fill in the PR checklist. Maintainers may decline in-scope-but-bloating diff --git a/agentmap.mjs b/agentmap.mjs index 51e3b79..a558e66 100755 --- a/agentmap.mjs +++ b/agentmap.mjs @@ -3048,7 +3048,10 @@ async function main() { // `node mcp.mjs` run, which is not cyclic). if (has("--mcp")) { try { - const m = await import(new URL("./mcp.mjs", import.meta.url)); + // .href, not the URL object: both resolve identically at runtime, but the + // dynamic-import signature is typed as string, so the object form is the + // one thing in this file the typecheck gate cannot see past. + const m = await import(new URL("./mcp.mjs", import.meta.url).href); await m.serve(mcpQuery); } catch (e) { console.error(`agentmap --mcp failed: ${e?.message || e}`); diff --git a/jsconfig.json b/jsconfig.json new file mode 100644 index 0000000..62c7d3c --- /dev/null +++ b/jsconfig.json @@ -0,0 +1,29 @@ +{ + "//": [ + "Typecheck gate for a codebase with no .ts in it. agentmap.mjs is ~250KB of", + "hand-written JavaScript; `checkJs` is the only static analysis that reads it.", + "", + "strict is OFF on purpose, and that is the whole reason this file is usable.", + "Measured 2026-07-27: strict reports 505 errors, of which 272 are TS7006", + "(annotate every parameter) — a rewrite, not a gate. Non-strict reports 2, and", + "both were real drift: a JSDoc @type for PLATFORMS that had fallen behind the", + "object it describes, and a dynamic import TypeScript could not follow. The", + "gate earns its place by catching that class — wrong property, wrong arity,", + "stale annotation — not by demanding annotations.", + "", + "Tighten it when the code is ready, not by loosening what it reports." + ], + "compilerOptions": { + "checkJs": true, + "allowJs": true, + "noEmit": true, + "target": "ES2022", + "module": "ESNext", + "moduleResolution": "bundler", + "strict": false, + "noImplicitAny": false, + "skipLibCheck": true, + "types": ["node"] + }, + "include": ["agentmap.mjs", "mcp.mjs", "skills/*.mjs", "hooks/*.mjs"] +} diff --git a/package-lock.json b/package-lock.json index 2a261fc..c865aa8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@raymondchins/agentmap", - "version": "0.17.0", + "version": "0.20.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@raymondchins/agentmap", - "version": "0.17.0", + "version": "0.20.1", "license": "MIT", "dependencies": { "ts-morph": "28.0.0" @@ -14,6 +14,10 @@ "bin": { "agentmap": "agentmap.mjs" }, + "devDependencies": { + "@types/node": "^26.1.1", + "typescript": "^7.0.2" + }, "engines": { "node": ">=20" } @@ -29,6 +33,356 @@ "tinyglobby": "^0.2.14" } }, + "node_modules/@types/node": { + "version": "26.1.1", + "resolved": "https://registry.npmjs.org/@types/node/-/node-26.1.1.tgz", + "integrity": "sha512-nxAkRSVkN1Y0JC1W8ky/fTfkGsMmcrRsbx+3XoZE+rMOX71kLYTV7fLXpqud1GpbpP5TuffXFqfX7fH2GgZREw==", + "dev": true, + "license": "MIT", + "dependencies": { + "undici-types": "~8.3.0" + } + }, + "node_modules/@typescript/typescript-aix-ppc64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-aix-ppc64/-/typescript-aix-ppc64-7.0.2.tgz", + "integrity": "sha512-MTKKkWB7p/0E9xi1d1tHtZ5PiLkGEMIq88pK2CubZjOsLtYTLqhgIgi6zepFa+9GHZ6h05NMCkQxGKiPXMxXtQ==", + "cpu": [ + "ppc64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "aix" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-darwin-arm64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-darwin-arm64/-/typescript-darwin-arm64-7.0.2.tgz", + "integrity": "sha512-gowzar9MwS/aRWp6f3a4KUqzRjAZjOsmGNCM6LcTgXum+dBfgsBVMN+AgvOCCbguXyick6LJhpBszxMebJ8syA==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-darwin-x64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-darwin-x64/-/typescript-darwin-x64-7.0.2.tgz", + "integrity": "sha512-SZ9xZInqApNlNGc9s0W1VSsktYSOe9cFqNOIqmN1Gs8SmkjKZYFt017G4VwPxASInODuAdbTW7sXiFUf893RgA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-freebsd-arm64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-freebsd-arm64/-/typescript-freebsd-arm64-7.0.2.tgz", + "integrity": "sha512-W5NH4y/J0plIIS5b2xvTEkU7JFxyqdMAOgf+Ilhl0vHQXKO5dZoxd+C/jEtq56c4F3wk71RB4BMRQ2XdI+bwYQ==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-freebsd-x64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-freebsd-x64/-/typescript-freebsd-x64-7.0.2.tgz", + "integrity": "sha512-UMGDx5sTpzNw3WiPebH7l90IWfJggEd+egHt/q6p7/Cm3zqoV7VxkGXt+3DxPIw8CcmvAB0j3sVVfbhX+M4Tpw==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-linux-arm": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-linux-arm/-/typescript-linux-arm-7.0.2.tgz", + "integrity": "sha512-gffT3xPz9sR7j/YJExkyPntrI0P2EP9XbOyWzth2/Gs0RstK+90RBcO0ncXoXy/beYll1SXw846Nf2zdnEz0QQ==", + "cpu": [ + "arm" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-linux-arm64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-linux-arm64/-/typescript-linux-arm64-7.0.2.tgz", + "integrity": "sha512-Qh4eU4/y3yDjnfjjyPYihMj5/ODIlmt+Bzu17OI+fiSRDW57QmU5SiN63exPRNJPKUzcc1INa1NXdrJ+MqHjUQ==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-linux-loong64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-linux-loong64/-/typescript-linux-loong64-7.0.2.tgz", + "integrity": "sha512-uEHck9i8hoAzXPiYRib1O7miOnz23SxIeVl6F4LXox+qov1K35jHcEW6VHKvZI+pyvl7fZEP4MCU5LYvIq1GuQ==", + "cpu": [ + "loong64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-linux-mips64el": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-linux-mips64el/-/typescript-linux-mips64el-7.0.2.tgz", + "integrity": "sha512-R4KvAMnE43W5Qeqb0Ly56O3mWMWIAgsMyz36DCaycd5nbg/9kzm0liw3JocfRqyJY0KPmzFjbswozXyW0DnIYA==", + "cpu": [ + "mips64el" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-linux-ppc64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-linux-ppc64/-/typescript-linux-ppc64-7.0.2.tgz", + "integrity": "sha512-DORx5b3sd/4S7eayxm4FQv+A7CrkUIGRaHiwI8oiHTAI1fAPWhF4J0vAlkC8biAlHSVVwxMQ3tjZ2/DVbnQiiA==", + "cpu": [ + "ppc64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-linux-riscv64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-linux-riscv64/-/typescript-linux-riscv64-7.0.2.tgz", + "integrity": "sha512-wf0jqEDOjrPRnKwYRyyJDRo11KMbvMFrU+q4zqKyChODBzvlkbhNQfKvLxQCcwTpdDaXSHZTVuh0JoCrKCUMHQ==", + "cpu": [ + "riscv64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-linux-s390x": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-linux-s390x/-/typescript-linux-s390x-7.0.2.tgz", + "integrity": "sha512-IkwJc3L7yhytWd/ewjyxNDfOmswCm9GWMJT/ue/dU4aZNbwZeYAetq42VyLmsmSjvoX7z74X6ZaYCtzAr0EuGw==", + "cpu": [ + "s390x" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-linux-x64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-linux-x64/-/typescript-linux-x64-7.0.2.tgz", + "integrity": "sha512-EYdf2cNg7rgCWJnxCdJ+F3V39O8ihb37eHAu1LK8oAFizgTQbPOK7zHHXbPt8rX24COqODXeI3sIf0fCXG7H/A==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-netbsd-arm64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-netbsd-arm64/-/typescript-netbsd-arm64-7.0.2.tgz", + "integrity": "sha512-+polYF4MF04aPpO5FTkHran9yUQDSXqy5GiSDKpsll5jy3l3+g9QLhpf39T+ePtefhXLOGrLl0QIjkQP6VnelA==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "netbsd" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-netbsd-x64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-netbsd-x64/-/typescript-netbsd-x64-7.0.2.tgz", + "integrity": "sha512-8YIT0EHM/3dq10ZOVF/A7pc/YSMtbcecct4rWtexrnSCHOPcpC2KTLXfTCR6vDpnSiY12heNb1GiN/wu+T/FyA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "netbsd" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-openbsd-arm64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-openbsd-arm64/-/typescript-openbsd-arm64-7.0.2.tgz", + "integrity": "sha512-APT8+ClYnuYm1u9+kgGXoMj2VzWzcymwh2gNSQVySHfkRDGOTVkoWLjCmOQSaO+PoqQ57B0flRp9SA+7GnnkzQ==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "openbsd" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-openbsd-x64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-openbsd-x64/-/typescript-openbsd-x64-7.0.2.tgz", + "integrity": "sha512-yX7s+Q0Dln0Dt9tEzZsAjXXR/+ytBM7AlglaqyeMPxQszJ1JhlJdZ6jLA+IzldHtflX81em7lDao1xXu+aRRkg==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "openbsd" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-sunos-x64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-sunos-x64/-/typescript-sunos-x64-7.0.2.tgz", + "integrity": "sha512-dLJDGaLZ1D4HPQn62u1n8mBDkJREwMsAkCdkwd4Ieqw+x3TUyTsqY0YiBCtE6H6OzzgGk3iuZ3vFWRS+E8/d1g==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "sunos" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-win32-arm64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-win32-arm64/-/typescript-win32-arm64-7.0.2.tgz", + "integrity": "sha512-Gyl1Vy6OsWesLzmq+EP0Fb7b4Nid5232AvcA2SFcdYreldpNtYFFofPjnt62y9hQy7VTaZp65ICJjuAQRaVcIQ==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=16.20.0" + } + }, + "node_modules/@typescript/typescript-win32-x64": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@typescript/typescript-win32-x64/-/typescript-win32-x64-7.0.2.tgz", + "integrity": "sha512-0BQ3HkAHHlKLSp1qRvf3SUhGpGsDuhB/jgFw75guyqbxJqEaS0Cw/VFO8i2nHglJUzQCRtMMR/IBAKE3ETMC4g==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "Apache-2.0", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=16.20.0" + } + }, "node_modules/balanced-match": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", @@ -131,6 +485,48 @@ "@ts-morph/common": "~0.29.0", "code-block-writer": "^13.0.3" } + }, + "node_modules/typescript": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-7.0.2.tgz", + "integrity": "sha512-8FYau96o3NKOhbjKi/qNvG/W5jhzxkbdm5sj9AbZ/5T5sWqn3hJgLfGx27sRKZWTvyzCP8dLRBTf5tBTSRVUNA==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "tsc": "bin/tsc" + }, + "engines": { + "node": ">=16.20.0" + }, + "optionalDependencies": { + "@typescript/typescript-aix-ppc64": "7.0.2", + "@typescript/typescript-darwin-arm64": "7.0.2", + "@typescript/typescript-darwin-x64": "7.0.2", + "@typescript/typescript-freebsd-arm64": "7.0.2", + "@typescript/typescript-freebsd-x64": "7.0.2", + "@typescript/typescript-linux-arm": "7.0.2", + "@typescript/typescript-linux-arm64": "7.0.2", + "@typescript/typescript-linux-loong64": "7.0.2", + "@typescript/typescript-linux-mips64el": "7.0.2", + "@typescript/typescript-linux-ppc64": "7.0.2", + "@typescript/typescript-linux-riscv64": "7.0.2", + "@typescript/typescript-linux-s390x": "7.0.2", + "@typescript/typescript-linux-x64": "7.0.2", + "@typescript/typescript-netbsd-arm64": "7.0.2", + "@typescript/typescript-netbsd-x64": "7.0.2", + "@typescript/typescript-openbsd-arm64": "7.0.2", + "@typescript/typescript-openbsd-x64": "7.0.2", + "@typescript/typescript-sunos-x64": "7.0.2", + "@typescript/typescript-win32-arm64": "7.0.2", + "@typescript/typescript-win32-x64": "7.0.2" + } + }, + "node_modules/undici-types": { + "version": "8.3.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-8.3.0.tgz", + "integrity": "sha512-j375ScV60dom+YkPFIfTLcOiPxkN/buHz5GobjLhixFuANaNs3C9l4GmrWqejgXWJ7BbJcFYpTEUkS1Ge8bpZQ==", + "dev": true, + "license": "MIT" } } } diff --git a/package.json b/package.json index 40ea509..aa1c357 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,9 @@ "scripts": { "map": "node agentmap.mjs", "test": "node --test test/*.test.mjs test/**/*.test.mjs", - "eval": "node eval/eval.mjs" + "eval": "node eval/eval.mjs", + "typecheck": "tsc -p jsconfig.json", + "coverage": "node --test --experimental-test-coverage --test-coverage-lines=90 --test-coverage-branches=70 test/*.test.mjs test/**/*.test.mjs" }, "engines": { "node": ">=20" @@ -75,5 +77,9 @@ "url": "https://github.com/raymondchins/agentmap/issues" }, "author": "Raymond Surya Chin", - "license": "MIT" + "license": "MIT", + "devDependencies": { + "@types/node": "^26.1.1", + "typescript": "^7.0.2" + } } diff --git a/skills/install.mjs b/skills/install.mjs index 3250db5..9fdc675 100644 --- a/skills/install.mjs +++ b/skills/install.mjs @@ -33,6 +33,7 @@ function skillPath(root, _globalScope, ...segments) { * legacy?: boolean; * docs?: (root: string, globalScope: boolean) => string; * hooks?: boolean; + * codexHooks?: boolean; * plugin?: boolean; * }>} */ const PLATFORMS = { diff --git a/test/helpers.mjs b/test/helpers.mjs index 5806cb7..d07c6be 100644 --- a/test/helpers.mjs +++ b/test/helpers.mjs @@ -49,9 +49,29 @@ export function gitInit(dir, { commit = false, message = "init" } = {}) { if (commit) { g("add", "-A"); g("commit", "-q", "-m", message, "--no-verify"); } } +// Detach every test's git from the DEVELOPER's git. Without this the suite +// inherits whatever is in ~/.gitconfig and /etc/gitconfig — `init.defaultBranch`, +// `core.autocrlf`, `commit.gpgsign`, an `includeIf`, a global `core.hooksPath` — +// so a test can pass on one machine and fail on another for reasons no assertion +// mentions. gitInit() already pins the few settings it cares about locally; this +// closes the rest. +// +// Points at a path that does not exist rather than /dev/null: git treats a missing +// config file as empty everywhere, while /dev/null is not a path on Windows, which +// the CI matrix now covers. +const NO_GIT_CONFIG = join(tmpdir(), "agentmap-no-such-gitconfig"); +const GIT_ENV = { + GIT_CONFIG_GLOBAL: NO_GIT_CONFIG, + GIT_CONFIG_SYSTEM: NO_GIT_CONFIG, + GIT_CONFIG_NOSYSTEM: "1", +}; + // Run a raw git command in `dir`. Throws on failure (callers expect git to work). export function git(dir, ...args) { - return execFileSync("git", args, { cwd: dir, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); + return execFileSync("git", args, { + cwd: dir, encoding: "utf8", stdio: ["ignore", "pipe", "pipe"], + env: { ...process.env, ...GIT_ENV }, + }); } // Every synchronous CLI invocation below gets a hard ceiling: if agentmap ever diff --git a/test/install-skill.test.mjs b/test/install-skill.test.mjs index bd0ccd7..e09aeea 100644 --- a/test/install-skill.test.mjs +++ b/test/install-skill.test.mjs @@ -3,7 +3,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; -import { makeRepo, run, runErr, cleanup } from "./helpers.mjs"; +import { makeRepo, run, runErr, runWithHome, cleanup } from "./helpers.mjs"; const PKG_VERSION = JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf8")).version; @@ -83,7 +83,9 @@ test("--install-skill --platform gemini installs GEMINI.md and hooks", () => { test("--install-skill --global --platform antigravity --dry-run targets ~/.gemini/config/skills", () => { const dir = makeRepo({}); - const r = run(dir, "--install-skill", "--global", "--platform", "antigravity", "--dry-run"); + const home = makeRepo({}); + const r = runWithHome(dir, home, "--install-skill", "--global", "--platform", "antigravity", "--dry-run"); + assert.ok(r.stdout.includes(home), "global paths did not resolve against the fake HOME"); assert.equal(r.status, 0, r.stderr); assert.match(r.stdout, /\.gemini[/\\]config[/\\]skills[/\\]agentmap[/\\]SKILL\.md/); cleanup(dir); @@ -91,7 +93,9 @@ test("--install-skill --global --platform antigravity --dry-run targets ~/.gemin test("--install-skill --global --platform opencode --dry-run targets ~/.config/opencode/skills and AGENTS.md", () => { const dir = makeRepo({}); - const r = run(dir, "--install-skill", "--global", "--platform", "opencode", "--dry-run"); + const home = makeRepo({}); + const r = runWithHome(dir, home, "--install-skill", "--global", "--platform", "opencode", "--dry-run"); + assert.ok(r.stdout.includes(home), "global paths did not resolve against the fake HOME"); assert.equal(r.status, 0, r.stderr); assert.match(r.stdout, /\.config[/\\]opencode[/\\]skills[/\\]agentmap[/\\]SKILL\.md/); assert.match(r.stdout, /\.config[/\\]opencode[/\\]AGENTS\.md/); diff --git a/test/ranking-quality.test.mjs b/test/ranking-quality.test.mjs new file mode 100644 index 0000000..b9b99b1 --- /dev/null +++ b/test/ranking-quality.test.mjs @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: MIT +// Contract — ranking QUALITY, not just ranking stability. +// +// determinism.test.mjs proves two builds agree with each other and that a known +// file appears SOMEWHERE in hubs. Neither catches a ranking that is stably, +// reproducibly wrong: invert the comparator and every one of those assertions +// still passes. These tests assert ORDER against a fixture whose in-degrees are +// constructed and therefore known in advance. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { makeRepo, gitInit, run, cleanup } from "./helpers.mjs"; + +const MAP = ".claude/agentmap/map.json"; + +// A deliberate star: one file everything imports, one middling file, one leaf +// nobody imports. In-degrees are 5 / 2 / 0, unambiguous by construction. +const FIXTURE = { + "tsconfig.json": JSON.stringify({ compilerOptions: { allowJs: true }, include: ["**/*.ts"] }), + "src/core.ts": `export const core = 1;`, + "src/mid.ts": `export const mid = 2;`, + "src/leaf.ts": `export const leaf = 3;`, + "src/a.ts": `import { core } from "./core";\nimport { mid } from "./mid";\nexport const a = core + mid;`, + "src/b.ts": `import { core } from "./core";\nimport { mid } from "./mid";\nexport const b = core + mid;`, + "src/c.ts": `import { core } from "./core";\nexport const c = core;`, + "src/d.ts": `import { core } from "./core";\nexport const d = core;`, + "src/e.ts": `import { core } from "./core";\nexport const e = core;`, +}; + +// hubs entries are display strings: "path (deg N, pr X)". Rank = array position. +const rankOf = (hubs, path) => hubs.findIndex((h) => h.startsWith(`${path} `) || h === path); +const degOf = (hubs, path) => { + const e = hubs.find((h) => h.startsWith(`${path} `)); + const m = e && e.match(/deg (\d+)/); + return m ? Number(m[1]) : null; +}; + +function buildHubs() { + const dir = makeRepo(FIXTURE); + gitInit(dir, { commit: true }); + const r = run(dir); + assert.equal(r.status, 0, `build failed: ${r.stderr}`); + const hubs = JSON.parse(readFileSync(join(dir, MAP), "utf8")).hubs; + return { dir, hubs }; +} + +test("the most-imported file ranks FIRST, not merely somewhere in hubs", () => { + const { dir, hubs } = buildHubs(); + assert.ok(hubs.length > 0, "no hubs produced"); + assert.ok( + hubs[0].startsWith("src/core.ts "), + `hubs[0] should be the 5-importer file, got: ${hubs[0]}\nfull: ${JSON.stringify(hubs)}`, + ); + cleanup(dir); +}); + +test("in-degree is reported accurately for the constructed graph", () => { + // Guards the number the ordering is derived from, so a correct order built on a + // wrong degree still fails loudly. + const { dir, hubs } = buildHubs(); + assert.equal(degOf(hubs, "src/core.ts"), 5, `core.ts should have 5 dependents: ${JSON.stringify(hubs)}`); + assert.equal(degOf(hubs, "src/mid.ts"), 2, `mid.ts should have 2 dependents: ${JSON.stringify(hubs)}`); + cleanup(dir); +}); + +test("a leaf nobody imports never outranks a file with importers", () => { + const { dir, hubs } = buildHubs(); + const leaf = rankOf(hubs, "src/leaf.ts"); + const mid = rankOf(hubs, "src/mid.ts"); + assert.notEqual(mid, -1, `mid.ts (2 importers) missing from hubs: ${JSON.stringify(hubs)}`); + // Absent is fine — a leaf has no claim on the list at all. Present-and-higher is not. + if (leaf !== -1) { + assert.ok(leaf > mid, `leaf.ts (0 importers) outranked mid.ts (2 importers): ${JSON.stringify(hubs)}`); + } + cleanup(dir); +}); + +test("hub order is monotonic in in-degree for this fixture", () => { + // The whole point: an inverted or scrambled comparator survives every + // determinism assertion and dies here. + const { dir, hubs } = buildHubs(); + const degs = hubs.map((h) => { const m = h.match(/deg (\d+)/); return m ? Number(m[1]) : null; }) + .filter((d) => d !== null); + for (let i = 1; i < degs.length; i++) { + assert.ok(degs[i] <= degs[i - 1], `hubs are not ordered by descending degree at ${i}: ${JSON.stringify(hubs)}`); + } + cleanup(dir); +}); + +test("--symbols ranks the most-referenced export above a single-use one", () => { + // Same idea one level down: symbol ranking, not file ranking. + const { dir } = buildHubs(); + const r = run(dir, "--symbols", "20", "--json"); + assert.equal(r.status, 0, `--symbols failed: ${r.stderr}`); + const names = JSON.parse(r.stdout).symbols.map((s) => s.name); + const core = names.indexOf("core"), mid = names.indexOf("mid"); + assert.notEqual(core, -1, `"core" missing from ranked symbols: ${JSON.stringify(names)}`); + if (mid !== -1) { + assert.ok(core < mid, `"core" (5 referencing files) ranked below "mid" (2): ${JSON.stringify(names)}`); + } + cleanup(dir); +}); From 135adc09976ab50c18ba312a429c77027f92e294 Mon Sep 17 00:00:00 2001 From: raymondchins Date: Mon, 27 Jul 2026 22:47:01 +0700 Subject: [PATCH 07/12] docs: correct the Gemini hook row, add the missing troubleshooting cases The onboarding matrix, uninstall table and troubleshooting section already existed - that roadmap item's anchors were stale. What was actually wrong or absent: The Gemini row claimed the nudge fires on AfterTool. It does not: the hook is registered on BeforeTool and emits a top-level systemMessage. What Gemini drops on BeforeTool is hookSpecificOutput.additionalContext, which is why the nudge used to vanish - the row had inverted the cause and the cure. Four troubleshooting rows added, each for a failure with no visible cause: nvm (the hook and the agent do not source nvm.sh, so node is absent from a PATH that works fine in your shell); the OOM, with the measured envelope showing repo size is the wrong axis - a 252-file app peaks at 683MB against 756MB for 4,000 dependency-free files, because the .d.ts closure dominates; stale skill detection, which --doctor already reports and nothing documented; and 0-files, stated precisely as git ls-files --cached --others --exclude-standard, so the gitignore case is obvious rather than mysterious. Competitor table: every row now links to its project, Repomix's agent-loop cell corrected from 'No' to 'MCP server (no auto-refresh, no nudge)', and an as-of date with an invitation to file drift as a bug. --- README.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 291e7b1..c2b28e5 100644 --- a/README.md +++ b/README.md @@ -169,17 +169,20 @@ OS-event file watcher (FSEvents/inotify) with debounced auto-sync and an install auto-configures eight agent CLIs. agentmap's honest edge over the multi-language graph tools is narrower and sharper: **TS/JS resolution the others approximate, with a published accuracy eval.** -| | **agentmap** | Aider repo map | RepoMapper | Repomix | code2prompt | +| | **agentmap** | [Aider repo map](https://github.com/Aider-AI/aider) | [RepoMapper](https://github.com/nuptcode/repomapper) | [Repomix](https://github.com/yamadashy/repomix) | [code2prompt](https://github.com/mufeedvh/code2prompt) | | --- | --- | --- | --- | --- | --- | | **Ranking algorithm** | Personalized PageRank (file + symbol graphs) | PageRank (graph ranking) | Importance heuristics | None (file order) | None (file order) | | **Languages** | TS/JS + Vue SFC (via ts-morph) | Many (tree-sitter) | Many (tree-sitter) | Language-agnostic (text) | Language-agnostic (text) | | **Token-budget output** | Yes — `--map [--tokens N]` ranked digest | Yes (built into Aider's context) | Partial | Yes (size caps) | Yes (templates/caps) | | **TS/JS resolution depth** | **Compiler-grade — `tsconfig` paths + `vite`/`webpack` alias + `#imports` + workspaces (ts-morph)** | Basename/regex heuristics | Basename/regex heuristics | N/A (text) | N/A (text) | | **Retrieval-accuracy eval** | **Yes — published [`EVAL.md`](./EVAL.md) vs live ground truth** | No | No | No | No | -| **Agent-loop wiring** | Yes — post-commit auto-refresh + PreToolUse hook | In-process (Aider only) | No | No | No | +| **Agent-loop wiring** | Yes — post-commit auto-refresh + PreToolUse hook | In-process (Aider only) | No | MCP server (no auto-refresh, no nudge) | No | | **Dependencies** | `ts-morph` only | Python + tree-sitter stack | Python + tree-sitter | Node | Rust binary | | **Install** | `npx @raymondchins/agentmap` | `pip install aider-chat` | `pip install` | `npx`/global | `cargo`/binary | +Comparison as of 2026-07-27, from each project's own docs. These are moving targets — if a +cell is out of date, that's a bug: open an issue. + What that table is **not** claiming: agentmap is TS/JS-only (the others are multi-language), and it's a **file-level import graph**, not a full call-site/reference resolver (see [Scope & limitations](#scope--limitations)). The differentiators are narrow and honest: @@ -358,7 +361,7 @@ skill/rule the agent may or may not consult). Honest matrix: | Platform | Install | Enforcement | Known gaps | |----------|---------|-------------|------------| | **Claude Code** | `/plugin install agentmap@agentmap` (or `--install-hooks`) | **live hook** — `PreToolUse` nudge on `Grep` + Bash searchers | non-blocking (never denies grep); bare-symbol `Grep` nudge requires the #3 hook fix | -| **Gemini CLI** | `--install-skill --platform gemini` | **live hook** — `.gemini/settings.json` nudge | fires on the `AfterTool`/`systemMessage` path (the earlier `BeforeTool` + `additionalContext` combo was silently dropped — fixed in #4) | +| **Gemini CLI** | `--install-skill --platform gemini` | **live hook** — `.gemini/settings.json` nudge | fires on `BeforeTool` and emits a top-level `systemMessage`; Gemini parses and then **drops** `hookSpecificOutput.additionalContext` on `BeforeTool`, which is why the nudge used to vanish silently | | **OpenCode** | `--install-skill --platform opencode` | **log-only** — `.opencode/plugins/agentmap-nudge.js` writes to the log, does not inject context | plugin can't steer the model; relies on the `AGENTS.md` block being read | | **Cursor** | `--install-skill --platform cursor` + `.cursor/mcp.json` (below) | **MCP + docs** — `alwaysApply` rule + the MCP server | Cursor's own hooks aren't wired; the rule is advisory | | **Codex CLI** | `--install-skill --platform codex` | **live gate** — `.codex/config.toml` PreToolUse hook | denies only high-confidence structural greps; allow-fallback for logs/pipes/non-TS-JS; `AGENTMAP_CODEX_GATE=0` bypasses; needs a trusted dir + Codex hooks-GA | @@ -408,6 +411,10 @@ leaves the rest of your `AGENTS.md` / `GEMINI.md` intact. | Codex/Gemini nudge never fires | Codex's gate is opt-in — set `[features] hooks = true` in `.codex/config.toml` (`AGENTMAP_CODEX_GATE=0` disables it). Gemini needs the `BeforeTool` hook that `--install-skill` writes. | | Installed the wrong `agentmap` | This is **`@raymondchins/agentmap`** (npm scope) — not the unrelated unscoped `agentmap` packages. | | Cursor MCP tools missing | `--mcp` doesn't auto-wire Cursor; add the copy-paste `.cursor/mcp.json` from the matrix above and restart Cursor. | +| Hook works in your shell, not in the agent | Almost always **nvm**. Your interactive shell sources `~/.nvm/nvm.sh`; the git hook and the agent's tool runner do not, so `node` isn't on their `PATH`. Point the hook at an absolute node (`which node`) or install a system-wide node. | +| `JavaScript heap out of memory` | Raise the ceiling — the parse peaks and there is no in-process warning that can fire in time (the process dies inside a single call, with heap use still at ~40% one sample earlier). Re-run as `NODE_OPTIONS=--max-old-space-size=8192 npx @raymondchins/agentmap`. Repo **size is not the axis**: measured, a 252-file Next.js app peaks at 683 MB while 4,000 dependency-free files peak at 756 MB, because the dependency `.d.ts` closure (~300 MB, ~1,800 extra program files on a 393-file app) dominates. A small repo with heavy `@types` can need more than a large plain one. | +| Skill file looks out of date | Each installed skill dir carries a `.agentmap_version`. `agentmap --doctor` compares it against the running version and flags the drift; `--install-skill` again overwrites it. | +| `0 files mapped` | agentmap indexes `git ls-files --cached --others --exclude-standard`, so uncommitted files *are* included but **`.gitignore`d ones are not** — a source tree matched by an ignore rule maps to nothing, as does a directory that is not a git repo at all. Confirm with `git ls-files --others --exclude-standard \| head`. | --- From d3bab6525343a412a50f3da57ed4477cf0c10acd Mon Sep 17 00:00:00 2001 From: raymondchins Date: Mon, 27 Jul 2026 22:50:28 +0700 Subject: [PATCH 08/12] refactor: one source walk instead of two, add a code of conduct The recursive tree walk existed twice - sourceFingerprint() and makeProject()'s non-git .vue fallback - identical except for what each does with a file, with every safety rule restated in both copies. Four of those rules are load-bearing and non-obvious (depth cap, per-directory try/catch, lstat-not-stat, skip symlinks), so two copies meant a fix could land in one and not the other. Extracted as walkSources(dir, onFile) with the reasoning written down once. Verified rather than assumed: 43 vue-sfc tests green (the fallback walk is what they exercise), a repo containing a circular AND a dangling symlink still terminates with exit 0, and a non-git .vue tree still resolves its import edge. Two other housekeeping items were already done and only the checkbox was stale: the dead statSync import is gone, and readPackageVersion is one definition with two callers, not the four divergent copies recorded. CI actions were already SHA-pinned. Adds CODE_OF_CONDUCT.md - links the Contributor Covenant rather than copying it, and states explicitly that being told a patch is wrong or that a number does not reproduce is the process working, since this repo closes roadmap items as refuted in writing. No FUNDING.yml: sponsorship handles are the maintainer's call, not something to invent. --- CODE_OF_CONDUCT.md | 41 ++++++++++++++++++++++++++ agentmap.mjs | 71 ++++++++++++++++++++++------------------------ 2 files changed, 75 insertions(+), 37 deletions(-) create mode 100644 CODE_OF_CONDUCT.md diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..63cfe24 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,41 @@ +# Code of Conduct + +## The standard + +This project adopts the [Contributor Covenant v2.1](https://www.contributor-covenant.org/version/2/1/code_of_conduct/). +Read it there — it is the authoritative text, and restating it here only creates a +copy to drift. + +In short: be respectful, assume good faith, and keep discussion about the work. +Harassment, personal attacks, and demeaning comments are not welcome, in issues, +pull requests, commit messages, or anywhere else this project is discussed. + +## Scope + +Applies in all project spaces — issues, pull requests, discussions, commits — and +when representing the project publicly. + +## Technical disagreement is not a violation + +This repository argues with itself in writing. `ROADMAP.md` records items closed as +**refuted** with the measurements that refuted them, several of which contradict +what a maintainer previously believed. `CONTRIBUTING.md` says maintainers may decline +in-scope changes. Being told your patch is wrong, or that a number does not +reproduce, is the process working. + +What is not on that list: making it personal, or about the person. + +## Reporting + +Report anything that crosses the line to **raymondchin.s@gmail.com**, the address +already on every commit in this repository. Reports are handled privately, and the +reporter's identity is not shared with the person reported. + +If the report concerns the maintainer, GitHub's own +[reporting channels](https://docs.github.com/en/communities/maintaining-your-safety-on-github/reporting-abuse-or-spam) +exist for exactly that reason and are the right escalation. + +## Enforcement + +Responses are proportionate: a private correction, a warning, removal of a comment, +or a block. The maintainer decides, and will say which is being applied and why. diff --git a/agentmap.mjs b/agentmap.mjs index a558e66..10fae55 100755 --- a/agentmap.mjs +++ b/agentmap.mjs @@ -438,30 +438,39 @@ function bm25Search(lexical, files, rawQuery, { limit = SYMBOL_MATCH_LIMIT } = { // without a full reparse. Skips node_modules/.git/.next. Any error ⇒ "" (caller // falls through to build, i.e. current behavior). Never used on the git path. // SOURCE_EXT_RE includes `.vue` so editing a Vue SFC invalidates the cache too. +// The one recursive source walk. Two callers needed exactly this traversal and +// differed only in what they do with a file, so it existed twice with the safety +// rules restated in both — the failure mode being a fix applied to one copy. The +// rules are load-bearing and each is here for a specific reason: +// • depth cap 40 — don't fully walk a pathologically deep tree; +// • per-directory try/catch — one permission-denied subdir must NOT abort the +// WHOLE walk. In sourceFingerprint() that would return "" and silently +// disable caching, which looks like a performance mystery, not an error; +// • lstatSync, NOT statSync, so a symlink reports as itself rather than its +// target, and symlinked entries are skipped entirely — never recursed into, +// never stat'd through — so a circular symlink cannot recurse until the +// stack overflows; +// • node_modules/.git/.next pruned before any stat. +// `onFile(fullPath, name, stat)` is called for every non-directory survivor. +function walkSources(dir, onFile, depth = 0) { + if (depth > 40) return; + let names; try { names = readdirSync(dir); } catch { return; } + for (const name of names) { + if (name === "node_modules" || name === ".git" || name === ".next") continue; + const full = dir + "/" + name; + let st; try { st = lstatSync(full); } catch { continue; } + if (st.isSymbolicLink()) continue; + if (st.isDirectory()) walkSources(full, onFile, depth + 1); + else onFile(full, name, st); + } +} + function sourceFingerprint() { try { const entries = []; - const walk = (dir, depth) => { - if (depth > 40) return; // depth cap — don't fully walk a pathologically deep tree - // per-directory try/catch: a single permission-denied subdir must NOT abort - // the WHOLE walk (that would return "" and silently disable caching) — skip - // the unreadable dir and keep going so the fingerprint stays usable. - let names; try { names = readdirSync(dir); } catch { return; } - for (const name of names) { - if (name === "node_modules" || name === ".git" || name === ".next") continue; - const full = dir + "/" + name; - let st; - // lstatSync (NOT statSync) so a symlink reports as a symlink instead of - // its target. Symlinked entries are SKIPPED entirely — never recursed - // into, never stat'd through — so a circular symlink can't cause infinite - // recursion / stack overflow. - try { st = lstatSync(full); } catch { continue; } - if (st.isSymbolicLink()) continue; - if (st.isDirectory()) walk(full, depth + 1); - else if (SOURCE_EXT_RE.test(name)) entries.push(`${full}:${st.mtimeMs}:${st.size}`); - } - }; - walk(".", 0); + walkSources(".", (full, name, st) => { + if (SOURCE_EXT_RE.test(name)) entries.push(`${full}:${st.mtimeMs}:${st.size}`); + }); entries.sort(); return createHash("sha1").update(entries.join("\n")).digest("hex"); } catch { return ""; } @@ -1150,23 +1159,11 @@ function makeProject(inc = null) { `components/**/*.${g}`, `lib/**/*.${g}`, `pages/**/*.${g}`, `*.${g}`, ]); - // Non-git `.vue` fallback: walk the tree like sourceFingerprint() does. + // Non-git `.vue` fallback: same traversal as sourceFingerprint(), different leaf. try { - const walk = (dir, depth) => { - if (depth > 40) return; // depth cap, matching sourceFingerprint() - let names; try { names = readdirSync(dir); } catch { return; } // skip unreadable dir, don't abort the whole walk - for (const name of names) { - if (name === "node_modules" || name === ".git" || name === ".next") continue; - const full = dir + "/" + name; - // lstatSync (NOT statSync) + skip symlinks, matching sourceFingerprint(): - // a circular symlink would otherwise recurse until the stack overflows. - let st; try { st = lstatSync(full); } catch { continue; } - if (st.isSymbolicLink()) continue; - if (st.isDirectory()) walk(full, depth + 1); - else if (name.endsWith(".vue")) vueFiles.push(full.replace(/^\.\//, "")); - } - }; - walk(".", 0); + walkSources(".", (full, name) => { + if (name.endsWith(".vue")) vueFiles.push(full.replace(/^\.\//, "")); + }); } catch { /* ignore — proceed without Vue */ } } // Build the virtual→real map and register each `