From 72869f2d9ec169c76ab5cf436f41e5f47fd4472d Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Mon, 7 Sep 2026 22:58:05 -0500 Subject: [PATCH 01/14] docs: make AGENTS.md the canonical agent guide for Codex/Astra (#1742) Codex reads AGENTS.md by default, not CLAUDE.md. Move project guidance to AGENTS.md (trim/nest long validation notes under docs/AGENTS.md), leave CLAUDE.md as a thin @AGENTS.md wrapper for Claude Code, and document the Codex project_doc_max_bytes raise needed for the ~35KiB root file. Co-authored-by: Colby McHenry --- AGENTS.md | 283 ++++++++++++++++++++++++++++++++++++++++++++++ CHANGELOG.md | 2 + CLAUDE.md | 300 ++----------------------------------------------- docs/AGENTS.md | 36 ++++++ 4 files changed, 328 insertions(+), 293 deletions(-) create mode 100644 AGENTS.md create mode 100644 docs/AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..ea6186f40 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,283 @@ +# AGENTS.md + +Canonical project guidance for coding agents working in this repository (Codex/Astra, Claude Code via `@AGENTS.md`, Cursor, etc.). + +**Codex size note:** root `AGENTS.md` is intentionally kept near ~35 KiB (critical build/test/arch/retrieval rules retained). Longer validation methodology + the Excalidraw worked example live in `docs/AGENTS.md`. This environment sets `project_doc_max_bytes = 49152` so root (and root+`docs/` when cwd is under `docs/`) are not silently truncated at the 32 KiB default. + +## Project Overview + +CodeGraph is a local-first code intelligence library + CLI + MCP server. It parses any supported codebase with tree-sitter, stores symbols/edges/files in SQLite (FTS5), and exposes a knowledge graph to AI agents (Claude Code, Cursor, Codex CLI, opencode) over MCP. Per-project data lives in `.codegraph/`. Extraction is deterministic — derived from AST, not LLM-summarized. + +Distributed as `@colbymchenry/codegraph` on npm; same binary serves as installer, indexer, and MCP server. + +## Build, Test, Run + +```bash +npm run build # tsc + copy schema.sql and *.wasm + build the viewer into dist/; chmods dist/bin/codegraph.js +npm run build:lib # the viewer's components as @colbymchenry/codegraph-ui (ui/dist) — NOT part of `build` +npm run dev # tsc --watch +npm run clean # rm -rf dist + +npm test # vitest run (all) +npm run test:watch +npm run test:eval # only __tests__/evaluation/ +npm run eval # build then run __tests__/evaluation/runner.ts via tsx + +npm run cli # build then run the local dist binary + +# Single test file / pattern +npx vitest run __tests__/installer-targets.test.ts +npx vitest run __tests__/extraction.test.ts -t "TypeScript" +``` + +`copy-assets` (called from `build`) copies `src/db/schema.sql` and all `src/extraction/wasm/*.wasm` files into `dist/`. **Any new SQL or grammar wasm must be copied or it won't ship.** + +One other build step writes into `dist/` and is subject to the same rule: `build:ui` builds the +browser viewer into `dist/viewer/` (never `dist/ui/` — that's the terminal ui). +`scripts/check-ui-build.mjs` asserts both `dist/viewer/` and the copied grammars in +`dist/extraction/wasm/` after every build and inside every release archive — the viewer's syntax +highlighting reads a file with the same grammar the engine indexed it with, so a missing wasm is an +unhighlighted screen as well as an extraction gap. + +`npm run build:lib` is separate and does NOT run as part of `npm run build`: it compiles the same +`ui/src` tree a second way, with `svelte-package`, into `ui/dist` — the `@colbymchenry/codegraph-ui` +component library the Pro app imports (task CG-61). `scripts/check-ui-package.mjs` then prunes the +standalone app's shell out of it, resolves the extensionless import specifiers `svelte-package` +leaves behind, and asserts the seam: nothing outside `lib/adapter.js` may reach the network. The +package is **prepared, not published** — `ui/package.json` carries `"private": true` deliberately, +and `scripts/pack-npm.sh` only packs a tarball when `CODEGRAPH_PACK_UI=1`. + +Tests run as **two vitest projects** (`vitest.workspace.mts`): `engine` (node) and `ui` (jsdom, the +Svelte plugin, `resolve.conditions: ['browser']`) for the single `__tests__/ui-package.test.ts`. +`npm test` still runs both. The split is not cosmetic — `browser` is a package-resolution +condition, and applied globally it hands the engine's suites the browser builds of +`web-tree-sitter` and friends. The root config (`vitest.config.mts`, `.mts` because the plugin is +ESM-only and the repo is CJS) is the shared base; note that a workspace project **concatenates** +the base's `include` with its own, which is why the `ui` project does not `extends` it. + +Node engines: `>=20.0.0 <25.0.0`. There is a hard exit on Node 25.x and below 20 (see `src/bin/node-version-check.ts`). + +## Architecture + +### Layered pipeline + +``` +files → ExtractionOrchestrator (tree-sitter) → DB (nodes/edges/files) + ↓ + ReferenceResolver (imports, name-matching, framework patterns) + ↓ + GraphQueryManager / GraphTraverser (callers, callees, impact) + ↓ + ContextBuilder (markdown/JSON for AI consumption) +``` + +The public API surface is `src/index.ts` — the `CodeGraph` class wires all the layers and re-exports types. Library users only touch this file; the MCP server and CLI also drive it. + +### Module layout + +- `src/index.ts` — `CodeGraph` class: `init`/`open`/`close`, `indexAll`, `sync`, `searchNodes`, `getCallers`/`getCallees`, `getImpactRadius`, `buildContext`, `watch`/`unwatch`. +- `src/db/` — `DatabaseConnection`, `QueryBuilder` (prepared statements), `schema.sql`, `sqlite-adapter.ts`. Backed by Node's built-in **`node:sqlite`** (`DatabaseSync`) — real SQLite with WAL + FTS5, exposed through a thin better-sqlite3-shaped adapter. The bundled runtime always ships Node ≥22.5, so `node:sqlite` is always available: **no native build step and no wasm fallback**. (Running from source needs Node ≥22.5.) `codegraph status` reports the live backend (`node-sqlite`, the sole backend). +- `src/extraction/` — `ExtractionOrchestrator`, tree-sitter wrappers, per-language extractors under `languages/` (one file per language), plus standalone extractors for non-tree-sitter formats (`svelte-extractor.ts`, `vue-extractor.ts`, `liquid-extractor.ts`, `dfm-extractor.ts` for Delphi). `parse-worker.ts` runs heavy parsing off the main thread. +- `src/resolution/` — `ReferenceResolver` orchestrates `import-resolver.ts` (with `path-aliases.ts` for tsconfig path aliases + cargo workspace member globs), `name-matcher.ts`, and `frameworks/` (Express, Laravel, Rails, FastAPI, Django, Flask, Spring, Gin, Axum, ASP.NET, Vapor, React Router, Next.js — `nextjs.ts`: pages and `route.ts` handlers from files, `router.push` / `redirect` / `NextResponse.redirect` as `navigates` edges, with `next-router-synthesizer.ts` for `` — Expo Router, SvelteKit, Vue/Nuxt, Cargo workspaces). Frameworks emit `route` nodes and `references` edges. `callback-synthesizer.ts` holds the whole-graph synthesis passes (`SYNTH_PASSES`, merged in registry order — first-seen wins a duplicate pair) with the language gates; `tier-synthesizer.ts` is the cross-tier pass (a client's literal `fetch`/`axios` path onto its own route, a queue job onto its consumer, a bus / socket event onto its handler — `channel`, `tier`, `registeredAt` on every edge; registered before the in-process emitter pass so its more specific edge wins); `synth-utils.ts` has the helpers they share (`enclosingFn`, `enclosingValue`, `makeLineAt`). Express's `postExtract` composes `app.use('/prefix', router)` mounts onto a mounted file's route names, idempotently (the original path stays in `qualifiedName`). +- `src/graph/` — `GraphTraverser` (BFS/DFS, impact radius, path finding) and `GraphQueryManager` (high-level queries), plus the shared query-time derivations more than one surface renders: `named-symbol-flow.ts` (the one path finder, behind `codegraph_explore`'s Flow section and the viewer's Flow strip), `dynamic-boundary-report.ts` (where the graph stops), `type-hierarchy.ts` (ancestors/subtypes and the implementation count explore prints and the viewer draws), + `dead-code.ts` (unreferenced symbols, and every reason a candidate is NOT claimed). A derivation that two callers render must live here, not in `ToolHandler` — two derivations eventually disagree. +- `src/context/` — `ContextBuilder` + formatter for markdown/JSON output. +- `src/search/` — full-text query parser and helpers for FTS5. +- `src/sync/` — `FileWatcher` (native FSEvents/inotify/RDCW) with debounce + filter, and git-hook helpers. +- `src/mcp/` — MCP server (`MCPServer`, `tools.ts`, `transport.ts`). `server-instructions.ts` is what the server returns in the MCP `initialize` response — keep it in sync with the user-facing tool guidance. +- `src/installer/` — see below. +- `src/bin/codegraph.ts` — CLI (commander). Subcommands: `install`, `init`, `uninit`, `index`, `sync`, `status`, `query`, `files`, `context`, `affected`, `serve --mcp`. +- `src/ui/` — terminal UI (shimmer progress, worker). +- `src/ui-server/` -- read-only JSON API for the `codegraph ui` browser viewer (`api/`: `node`, `flow`, `map`, `screens`, `steps`, `deadcode`, `trails`, `program`, ...) plus static server; Svelte viewer lives in `ui/` (see `docs/design/codegraph-ui-design-spec.md`). `screens`/`steps`/`program` share one fold (`via`/`when` via `graph/branch-guards.ts`); `api/effects.ts` curates calls that leave the index; `api/route-roots.ts` names where a route's code starts. Derivations rendered by more than one surface belong in `src/graph/`, not `ToolHandler`. + +### NodeKind / EdgeKind + +Defined in `src/types.ts`. Both extractors and resolvers must use these exact strings. + +- **NodeKind**: `file`, `module`, `class`, `struct`, `interface`, `trait`, `protocol`, `function`, `method`, `property`, `field`, `variable`, `constant`, `enum`, `enum_member`, `type_alias`, `namespace`, `parameter`, `import`, `export`, `route`, `component`, `union`. +- **EdgeKind**: `contains`, `calls`, `imports`, `exports`, `extends`, `implements`, `references`, `type_of`, `returns`, `instantiates`, `overrides`, `decorates`. + +### Multi-agent installer + +`src/installer/` is the entry point for `codegraph install` (and the bare `codegraph`/`npx @colbymchenry/codegraph` invocation). Architecture: + +- `targets/registry.ts` lists every supported agent. +- `targets/types.ts` defines the `AgentTarget` interface — adding a 5th agent (Continue, Zed, Windsurf…) is **one new file in `targets/` + one entry in `registry.ts`**. Each target owns its config-file location and MCP-server JSON/TOML/JSONC writing. (Targets no longer write an instructions file — see below.) +- Current targets: `claude.ts`, `cursor.ts`, `codex.ts`, `opencode.ts`. +- `targets/toml.ts` is a hand-rolled TOML serializer scoped to `[mcp_servers.codegraph]` (used by Codex). Sibling tables and `[[array_of_tables]]` are preserved verbatim. No new dependency. +- opencode reads `opencode.jsonc` by default; the installer prefers existing `.jsonc`, falls back to `.json`, and creates `.jsonc` for greenfield installs. Edits are surgical via `jsonc-parser` so user comments and formatting survive install/re-install/uninstall round-trips. +- `instructions-template.ts` no longer holds an instructions body — it exports only the ``/`` markers. The installer **stopped writing** a `## CodeGraph` block into each agent's instructions file (`CLAUDE.md` / `~/.codex/AGENTS.md` / `~/.config/opencode/AGENTS.md` / `~/.gemini/GEMINI.md` / `.cursor/rules/codegraph.mdc` / Kiro steering doc) because it duplicated the MCP `initialize` instructions verbatim (issue #529). Each target's `install` (self-heal on upgrade) and `uninstall` use the markers to **strip** a block a previous install left behind. `server-instructions.ts` is the single source of truth for agent-facing guidance. +- All installer changes need matching coverage in `__tests__/installer-targets.test.ts` — there are ~47 parameterized contract tests covering install idempotency, sibling preservation, uninstall reverses install, byte-equal re-runs returning `unchanged`, and partial-state recovery for Codex. + +### Cursor MCP working-directory quirk + +Cursor launches MCP subprocesses with the wrong cwd and doesn't pass `rootUri` in `initialize`. The installer injects `--path` into Cursor's MCP args — absolute path for local installs, `${workspaceFolder}` for global installs. If you touch Cursor wiring, preserve this. + +### MCP server instructions + +`src/mcp/server-instructions.ts` is sent back to the agent in the MCP `initialize` response. This is the *first* thing every agent sees about how to use the tools, and as of issue #529 it is the **single source of truth** for agent-facing tool guidance — the installer no longer writes a duplicate `## CodeGraph` instructions block into `CLAUDE.md` / `AGENTS.md` / `.cursor/rules/codegraph.mdc`. Edit tool guidance here and nowhere else. + +## Retrieval performance & dynamic-dispatch coverage (do not regress) + +CodeGraph's core value is letting an agent answer **structural/flow** questions ("how does X reach Y", trace, impact, callers) with a few **fast** codegraph calls and **zero Read/Grep**. The optimization target is **wall-clock latency + tool-call count** — *don't optimize for token cost*. (Cost is **lower**, not "flat" as earlier framing claimed: a current-build with-vs-without A/B across the 7 README repos, median of 4, saved on average **35% cost · 57% tokens · 46% time · 71% tool calls** — reproducing the published README. The mechanism is **far fewer turns over a much smaller accumulated context** — NOT cache-ability: the without-arm's huge token volume is *mostly* cheap cache-reads, which is why token-count savings (57%) look bigger than cost savings (35%). Measure tokens by **summing per-turn assistant usage**, not `result.usage` (last-turn only in current Claude Code). See `docs/benchmarks/call-sequence-analysis.md`.) The mechanism that drives everything here: **an agent falls back to Read/Grep the instant a codegraph answer is insufficient.** So every change is judged by one question — is codegraph's answer sufficient enough to *stop* the agent from reading? + +**Target behavior:** a flow question resolves in **1 codegraph call on small repos, scaling to 3–5 on large**, with **Read/Grep = 0**. When reviewing a PR or trying something new, do not regress this. + +### Adapt the tool to the agent — don't try to change the agent + +The lever that decides whether a retrieval change lands. **Test before building anything here: does this make a tool the agent _already calls_ do more with the input it _already gives_? If it instead needs the agent to behave differently — pick a different tool, query differently, learn from examples — it hits the low-salience wall and won't land.** + +CodeGraph's only channels to influence the agent are low-salience: the MCP `initialize` instructions (`server-instructions.ts`) and the tool descriptions. Changing them does **not** reliably move the agent's tool _choice_ or query style — validated: trace-first steering ported into the server-instructions + tool descriptions (3 wording variants) never reproduced what a CLI `--append-system-prompt` achieved, and **regressed** wall-clock vs baseline. New tools fare worse (rarely chosen — the agent under-picks even `trace`); "better examples" is the same steering. The agent's tool-choice does improve on its own as host models get better at tool use — but that is not ours to force. + +What works is meeting the agent where it already is: +- **explore-flow** — `codegraph_explore` is the PRIMARY tool the agent reliably calls; its query is a precise bag of symbol names (incl. qualified `Class.method`) spanning the flow the agent is after; explore finds the call path _among those named symbols_ (riding synthesized edges) and leads its output with it. (`buildFlowFromNamedSymbols`: segment/co-naming disambiguation; ≤1 unnamed bridge so it never wanders a god-function's fan-out. Overload-aware: a PascalCase type token in the query biases an overloaded name to that type's own def — `DataRequest task` → DataRequest's `task`, not the abstract base; named-symbol files sort first.) +- **Sufficiency** — make the tool's output complete enough that the agent stops. `codegraph_node` returns the full body + the caller/callee trail, and for an AMBIGUOUS name returns **every overload's body in one call** (so the agent never Reads a file to find the right overload — validated on Alamofire/gin). This is the after-explore depth tool (labeled SECONDARY). +- **Errors teach abandonment** — one or two `isError: true` responses early in a session and the agent stops calling codegraph entirely (maintainer-observed, repeatedly). `isError` is reserved for genuine "stop trying" cases: security refusals (`PathRefusalError`) and real malfunctions (which carry a retry-once note). Every expected/recoverable condition — project not indexed, symbol not found, file not in the index — returns a **SUCCESS-shaped response carrying the guidance** (`NotIndexedError` → `textResult`, see `ToolHandler.execute`'s catch). The same principle is why the tool surface is **always exposed, even at an un-indexed root** (the old empty-`tools/list` gate was removed in #964 — it broke monorepos where only sub-projects carry a `.codegraph/`, and hid the tools from a session that started before `codegraph init`): safety comes from the response SHAPE (success-shaped guidance, never `isError`), not from hiding tools. An un-indexed root's `initialize` sends a per-project variant (`SERVER_INSTRUCTIONS_NO_ROOT_INDEX` — "pass `projectPath` to a project that has a `.codegraph/`"), not an "inactive" note; indexing is still deliberately the user's call, never the agent's. + +What fails is the inverse — folding a precise answer into a **fuzzy-input** tool: the now-removed `codegraph_context` took a description, not symbols, so it couldn't disambiguate a flow's endpoints and surfaced the _wrong feature_ (which is why it was cut). Precise output needs precise input — explore takes a symbol bag for exactly this reason. (`codegraph_trace` was likewise removed: explore-flow does its job and the agent under-picked it.) + +The remaining lever under this axis is **coverage**: every flow made to connect statically (a new dynamic-dispatch synthesizer, or extracting symbols static parsing skipped — e.g. object-literal store actions in `create((set,get)=>({...}))`) is then surfaced automatically by explore-flow, no agent change needed. Reactive/reconciler runtimes (Halo's `ReactiveExtensionClient`, MediatR, Vue Proxy) are the frontier — flows there have no static edges, so nothing surfaces (correctly — silent beats wrong). Full investigation + A/B record: `docs/benchmarks/call-sequence-analysis.md` + auto-memory `project_codegraph_read_displacement`. + +### Explore budget — keep BOTH budgets monotonic with repo size + +Two functions in `src/mcp/tools.ts` scale explore with indexed file count. This is the expected resolution (a regression here silently forces agents back to Read): + +| Repo | files | explore calls | chars/call | per-file | +|---|---|---|---|---| +| express (small) | 147 | 1 | 18K | 3800 | +| excalidraw/django (medium) | 643–3043 | 2 | 28K | 6500 | +| vscode (large) | 10446 | 3 | 35K | 7000 | +| ~20k / ~40k | — | 4 / 5 | 38K | 7000 | + +- `getExploreBudget(fileCount)` → **call** budget: `<500→1, <5000→2, <15000→3, <25000→4, ≥25000→5` (max 5). +- `getExploreOutputBudget(fileCount)` → **per-call** output (chars / files / per-file). **Invariant: a larger tier must never get a smaller `maxCharsPerFile` than a smaller tier.** (Regression that motivated this doc: the `<5000` tier's 2500 was *below* the `<500` tier's 3800, so on a god-file repo — excalidraw's 415 KB `App.tsx` — one explore returned <1% of the file and forced a Read.) +- Explore output must **never tell the agent to "use Read"** — steer to another `codegraph_explore` and "treat returned source as already Read." + +### Dynamic-dispatch coverage — the flow must EXIST in the graph end-to-end + +Static tree-sitter extraction misses computed/indirect calls, so flows break at dynamic dispatch and the agent reads to reconstruct them. Synthesizers/resolvers bridge these so `codegraph_explore` connects them end-to-end (`src/resolution/callback-synthesizer.ts`, `src/resolution/frameworks/`). Channels today: callback/observer, EventEmitter, **React re-render** (`setState`→`render`), **JSX child** (`render`→child component), **React Native native→JS events** (`sendEvent(withName:)` / JVM `emit` → the `addListener` handler, named or inline, `rn-event-channel`), django ORM descriptor. The JS→native direction is a *resolver* (`frameworks/react-native.ts`: `RCT_EXPORT_METHOD`, `RCT_EXTERN_MODULE` Swift shims, TurboModules), which trusts receiver evidence — an alias bound to `NativeModules.X` — over the import resolver. All synthesized edges are `provenance:'heuristic'` with `metadata.synthesizedBy` + `registeredAt` (the wiring site), surfaced inline in `codegraph_explore`'s Flow section and the `codegraph_node` trail. + +**Principle: partial coverage is WORSE than none.** Bridging one boundary but not the next reveals a hop the agent then drills + reads to finish. Measured on excalidraw: react-render alone *raised* reads to 5–7; only completing the flow (adding the jsx-child hop) dropped it to 0–1. **Always close the flow end-to-end and re-measure** — never ship a half-bridged flow. + + +### Validation methodology & worked examples + +**Required** for every new language/framework: validate on small/medium/large real repos with >=3 flow prompts; deterministic probes (`scripts/agent-eval/probe-*.mjs`) then agent A/B (`scripts/agent-eval/run-all.sh` / `ab-new-vs-baseline.sh`). Pass bar: ~0 Read/Grep within the explore-call budget, faster than without-codegraph, no control-repo regression. + +Full methodology (feedback metrics, CLI contamination guard, Sonnet/`--effort high` model policy, daemon pre-warm), the Excalidraw worked example, and coverage matrix live in: +- `docs/AGENTS.md` (nested; also loaded when cwd is under `docs/`) +- `docs/design/dynamic-dispatch-coverage-playbook.md` +- `docs/design/callback-edge-synthesis.md` +- `docs/benchmarks/call-sequence-analysis.md` / `docs/benchmarks/agent-eval-feedback-metrics.md` + + +Tests live in `__tests__/` and mirror the module they cover. Notable ones beyond the obvious: + +- `installer-targets.test.ts` — parameterized contract suite across all 4 agent targets (see installer notes above). +- `evaluation/` — `runner.ts` + `test-cases.ts` exercise codegraph against synthetic projects and score the results; run via `npm run eval` (builds first). Not part of `npm test`. +- `sqlite-backend.test.ts` / `node-sqlite-backend.test.ts` — pin that `node:sqlite` is the sole backend: `getBackend()` reports `node-sqlite` and the DB comes up in WAL. +- `pr19-improvements.test.ts`, `frameworks-integration.test.ts` — regression coverage for specific past PRs/incidents; don't rename these, the names anchor to git history. + +Tests create temp dirs with `fs.mkdtempSync` and clean up in `afterEach`. They write real files and exercise real SQLite — there is no DB mocking. + +### Windows-gated tests + +Behavior that differs by platform (path resolution, drive letters, `SENSITIVE_PATHS`, `%APPDATA%` config dirs, CRLF) must be gated, not assumed. Use `it.runIf(process.platform === 'win32')(...)` for Windows-only assertions and `it.runIf(process.platform !== 'win32')(...)` for POSIX-only ones — e.g. `/etc` is sensitive on POSIX but resolves to `C:\etc` (non-existent) on Windows, so an ungated `/etc` assertion fails on Windows. Validate the Windows side for real (see below); don't merge a Windows-gated test you haven't seen run. + +## Cross-platform validation + +The dev machine — and the default `npm test` target — is **macOS**, so local runs cover the macOS path. The other two platforms aren't here; when a change is platform-sensitive (file watching, sockets / named pipes, path & symlink handling, process lifecycle, inotify budget) validate them for real rather than guessing. + +### Linux (Docker) + +When asked to test or validate on Linux, use **Docker** — there's no Linux box, but Docker runs on the macOS host. Build a throwaway image from the repo and run the suite inside it: + +- `FROM node:22-bookworm`; `COPY` the repo with a `.dockerignore` excluding `node_modules`/`dist`/`.git`/`.codegraph`; `RUN npm ci && npm run build`. Don't reuse the Mac `node_modules` — `esbuild`/`rollup` ship platform-specific binaries. +- Run with **`docker run --rm --init`**. The `--init` is load-bearing for any process-lifecycle test (daemon reaping, the #277 PPID watchdog, idle-timeout): without a zombie-reaping PID 1, a SIGKILL'd/exited process lingers as a zombie and `process.kill(pid, 0)` still reports it *alive*, so exit-detection assertions false-fail even though the process did exit. +- Linux is where the inotify watch budget actually bites: count a process's watches via `/proc//fdinfo/*` (sum `^inotify ` lines on the fd whose `readlink` is `anon_inode:inotify`). + +### Windows (Parallels VM + SSH) + +For any Windows-specific PR, bug, or implementation, validate it on the real Windows VM rather than guessing. Connection details live in the gitignored **`.parallels`** file at the repo root (VM name, guest IP, SSH user/key). `prlctl exec` needs Parallels Pro and is unavailable, so SSH is the bridge. + +- Connect / run from the Mac host: `ssh @ "..."`. For multi-line work, pipe PowerShell over stdin and **refresh PATH from the registry** first (sshd's session has a stale PATH after winget installs): + ``` + ssh colby@10.211.55.3 "powershell -NoProfile -ExecutionPolicy Bypass -Command -" <<'PS' + $env:Path = [Environment]::GetEnvironmentVariable("Path","Machine") + ";" + [Environment]::GetEnvironmentVariable("Path","User") + Set-Location C:\dev\codegraph + PS + ``` +- Clone fresh into a **Windows-local** path (`C:\dev\codegraph`) and `npm ci` there — never run npm against the shared Mac repo, since `esbuild`/`rollup` ship platform-specific binaries. +- Guest toolchain (winget): Node LTS, Git, and the **VC++ ARM64 redistributable** (required by `@rollup/rollup-win32-arm64-msvc`, which vitest pulls in). +- Fetch a contributor PR head straight from their fork to dodge `pull//head` lag: `git fetch ` then `git checkout -f FETCH_HEAD`. +- Known pre-existing Windows failures (they reproduce on `main`, unrelated to your change — confirm against `origin/main` before blaming your PR, and don't let them mask new regressions): `security.test.ts > Session marker symlink resistance > does not follow a pre-planted symlink` (symlink creation needs privileges on Windows); and the `mcp-initialize.test.ts` / `mcp-roots.test.ts` suites, which fail in `afterEach` with `EPERM` removing the temp dir because a spawned `serve --mcp` (its `--liftoff-only` re-exec grandchild) still holds the cwd / SQLite file open — a Windows file-locking quirk, not a logic bug. + +## Releases + +Released to npm and mirrored as [GitHub Releases](https://github.com/colbymchenry/codegraph/releases). `CHANGELOG.md` is the source of truth; GitHub Release notes are extracted from it. + +### Writing changelog entries + +**Default: write entries under `## [Unreleased]`** — that's the section reserved for work landing between releases. **Don't pre-create a `## [X.Y.Z]` block** for the next release: the Release workflow's first step is `scripts/prepare-release.mjs`, which automatically promotes everything under `[Unreleased]` into a new `## [X.Y.Z] - ` block at release time (or merges into a pre-existing `[X.Y.Z]` block if one exists — but you don't need one). Pre-staging is what caused the v0.9.5 sparse-release-notes incident: a sparse `[0.9.5]` block hand-added before the rest of the work landed got picked by the extractor over the much-larger `[Unreleased]` section above it. Don't do that. + +Formatting rules for any entry (anywhere — `[Unreleased]` or otherwise): + +1. **Write friendly, user-facing notes — not engineer-facing ones.** Group under `### New Features` and `### Fixes` (sentence-case). Surface `### Breaking Changes` and `### Security` as their own sections **only when the release has them**; fold improvement-flavored changes into New Features. Omit empty sections. (This replaces the old Keep-a-Changelog `Added/Changed/Fixed/Removed/Deprecated` grouping: the GitHub Release page extracts each version block **verbatim** via `scripts/extract-release-notes.mjs`, and the old dense, implementation-focused entries rendered as an unreadable wall of text — so the whole CHANGELOG was rewritten to this format and every published release re-noted to match.) +2. **One plain-language sentence per bullet:** what changed and why it matters to a user. Lead with the capability, or with the symptom that's now fixed. +3. **Strip the internals.** No internal file paths (`src/...`), no internal symbol / function / class names, no benchmark numbers / percentages / node-or-edge counts. **Keep:** language & framework names (Go, Spring, NestJS, …), things a user types or sets (`codegraph install`, `codegraph_explore`, the `CODEGRAPH_*` env vars), agent / IDE names (Claude Code, Cursor, opencode, Kiro, …), and a brief `Thanks @user` when a contributor is credited. +4. Issue / PR references in entries are by number (`(#403)` etc.); the GitHub renderer auto-links them in the published release notes. +5. **Don't add a `[X.Y.Z]: https://...` link reference yourself** — `prepare-release.mjs` appends it automatically when it promotes the version (idempotent: a re-run is a no-op if it already exists). +6. **Every release opens with a `### Highlights` block — the only part most people read.** At most ~8 one-line bullets, in plain language for someone who doesn't read code, ordered by what a typical user notices first (new agent/IDE support and setup changes, then answer quality, then reliability), plus a one-sentence upgrade note when a re-index is needed. Write or refresh it in `[Unreleased]` when a release is being prepared — not per PR — and keep the detailed `### New Features` / `### Fixes` entries below it. When `### Fixes` grows past ~15 entries, group them under `####` sub-headings (`Better answers from codegraph_explore`, `Finding your project, live updates, and the CLI`, `Indexing reliability and disk usage`, `Language and framework accuracy`) so a skimmer can find their area. + +Multi-word headings like `### New Features` are safe on the normal release path: `prepare-release.mjs` **Case A** moves the whole `[Unreleased]` body verbatim into `[X.Y.Z]`. (Only its rarely-used **Case B** *merge* splits sub-sections with a single-word `^### (\w+)$` regex that wouldn't match them — and Case B fires only if a `[X.Y.Z]` block was pre-created, which rule above already forbids.) + +### Release flow (the user runs these) + +Releases are built and published by the **GitHub Actions "Release" workflow** +(`.github/workflows/release.yml`). It runs `scripts/prepare-release.mjs` to +promote `[Unreleased]` into `[]` (and auto-commit + push that +CHANGELOG change back to `main` so on-disk truth matches the published +notes), then bundles a Node runtime per platform (`scripts/build-bundle.sh`) +and publishes both the GitHub Release and the npm thin-installer +(`scripts/pack-npm.sh`: a shim package + per-platform packages). +Publishing manually is **wrong** now — a plain `npm publish` ships the root +package (non-bundled), which breaks anyone on Node < 22.5. + +**Claude does NOT bump the version unless explicitly asked.** The maintainer +typically does it themselves — often by editing `package.json` directly via +the GitHub web UI. Don't proactively commit a version bump as part of +unrelated work, and don't propose one when summarizing a PR. + +When the maintainer DOES bump the version, the only edit strictly required is +to `package.json` — the workflow's "Sync package-lock.json" step detects a +mismatch between `package.json` and `package-lock.json`, runs +`npm install --package-lock-only --ignore-scripts` to rewrite the lock file's +version fields (top-level + `packages.""`), and auto-commits + pushes the +result back to `main` with `[skip ci]`. So a GitHub-web-UI single-file edit to +`package.json` is enough to kick off a clean release. (If they edit both files +locally, that's fine too — the sync step no-ops.) + +Once `package.json` is at the target version on `main`, trigger +**Actions → Release → Run workflow** (on `main`). The workflow: + +1. Syncs `package-lock.json` to `package.json`'s version if they've drifted; commits + pushes that change. +2. Runs `prepare-release.mjs ` → promotes `[Unreleased]` → `[X.Y.Z] - ` in `CHANGELOG.md`, appends the link reference, commits + pushes the move with `[skip ci]`. +3. Builds every platform bundle on one runner, generates `SHA256SUMS`. +4. Creates the GitHub Release with notes from the freshly-promoted `[X.Y.Z]` block. +5. Publishes the npm shim + per-platform packages. Requires the `NPM_TOKEN` repo secret. + +**Do not run `npm publish`, `git push`, or `git tag` yourself** — these are +publish actions on shared state. Write the files, hand the user the commands. + +## House rules + +- The `0.7.x` line is in active multi-agent rollout. Any change to `src/installer/` (especially `targets/`) needs corresponding test coverage and a CHANGELOG entry — installer regressions break every new install silently. +- When changing what the MCP tools do or how agents should use them, edit `src/mcp/server-instructions.ts` — it is the **single source of truth** for agent-facing tool guidance (issue #529). The installer no longer writes a duplicate instructions block into `CLAUDE.md` / `AGENTS.md` / `GEMINI.md` / `.cursor/rules/codegraph.mdc` / Kiro steering, so there's nothing to keep in sync anymore. (The repo's own checked-in `.cursor/rules/codegraph.mdc` is dogfooding config — update it too if you use Cursor on this repo, but it ships nowhere.) +- **Before adding or extending a router, a web framework, or a language's `WHEN` rules, read `docs/design/framework-coverage.md`.** It is the standing answer to "what is supported and what is left" across the three axes (route nodes → Entry points, `navigates` edges → Screens, branch-guard rules → the `WHEN` labels), with what each remaining item needs, the traps that have already cost debugging time, and the queries to re-verify it. Update it in the same change that moves a row. +- CodeGraph provides **code context**, not product requirements. For new features, ask the user about UX, edge cases, and acceptance criteria — the graph won't tell you. +- **When the user references issues, PR comments, or external reports, anchor them to a date and version before drawing conclusions.** Check the comment's `createdAt` against: + - The **last released version** — `grep -m1 '^## \[' CHANGELOG.md` shows the top-of-file version (older releases follow). A comment dated before the latest `## [X.Y.Z] - YYYY-MM-DD` is reacting to *released* state — work that's only on `main` or on an unmerged branch doesn't apply. + - The **last main commit** — `git log --first-parent main -1 --format='%ai %h %s'`. A comment after the last release but before a fix on main may already be addressed there but unreleased. + - The **current branch's tip** — your own unmerged work obviously can't be what the comment is reacting to. + Always disambiguate "released," "merged-but-unreleased," and "in-progress" before agreeing that a user-reported problem is unfixed (or that a fix is incomplete). A user saying "your fix only covers X" about a recent PR is usually pointing at the *released* shortcomings — your in-flight branch may already address them but they have no way to know that. +- **Version-tag every image referenced in `README.md`.** GitHub caches README images (`raw.githubusercontent.com` with a 5-minute TTL; third-party hosts sit behind the long-lived camo proxy), so updating an asset in place can keep showing the stale version. Give each README image URL a `?v=N` query tag and **bump `N` in the same commit whenever the asset bytes change** — e.g. `assets/waitlist.svg?v=2`. The changed URL sidesteps every cache so the new image shows immediately instead of waiting on a TTL to expire. diff --git a/CHANGELOG.md b/CHANGELOG.md index 1815c4150..220298c48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### New Features +- **Codex and Astra read project guidance from `AGENTS.md`.** The canonical agent guide now lives in `AGENTS.md` (with a nested `docs/AGENTS.md` for long validation notes); `CLAUDE.md` is a thin `@AGENTS.md` wrapper for Claude Code. Codex/Astra no longer miss the old CLAUDE-only instructions. + - **A busy screen's picture is laid out by the parts of the screen.** A screen is a set of handlers with no order between them, so on a hub screen the old rows-by-distance collapsed into one enormous row — the main screen of one app put 89 boxes side by side on a canvas over 28,000px wide, every line a near-horizontal sweep across all of it. The Steps tab now groups a screen's picture by region — the component that owns each handler, named in a small caption over its boxes — with each region a column where a step sits above what it sets in motion, tiled in the screen's own source order. At rest the picture hides only two things: the screen's own fan-out — one line into each region stands in for it — and lines that point back up; every other line draws where it leads, between two regions included, and selecting a step brings out its whole story in the side panel, link by link. A box nothing points at is the screen's own doing — run on render or mount, or from a binding written inline — the key says so, and selecting it lights its line from the screen with what fires it. The same app's widest screen now lays out under 3,500px with every line local, and the whole picture fits on screen when it opens. Endpoints, handlers and the in-order reading are untouched, and nothing needs a re-index: the regions come from the same walk that draws the steps. - **Where the code chooses, the picture says so once.** A helper that ends `return (await hasSeenWelcome(id)) ? '/home/' : '/welcome/'` sends the app to one of two screens, but the Steps picture drew that as two separate arrows, each carrying the whole condition with one of them negated and both cut off at the same forty characters — and before you clicked anything, neither arrow was labelled at all, so nothing said it was a choice. Now sibling arrows out of one box that are the arms of one `if`, `switch` or ternary are drawn as the choice they are: the condition is written once under the box that decides it, and each arrow out says only which way it is — `yes`, `no`, or a case's own value. They are the only arrows labelled before you select anything, so the picture reads at a glance without becoming a wall of text. A one-sided guard — an early exit, an `if` with only one side drawn — still carries its condition on the arrow, and an arrow that is reached whether or not the condition holds never claims a side. Nothing needs a re-index: the decision is read from the source at request time. diff --git a/CLAUDE.md b/CLAUDE.md index f91f170a9..217d62d2e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,299 +1,13 @@ # CLAUDE.md -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. +Claude Code project guidance for this repository. -## Project Overview +Primary instructions live in the canonical agent guide — import it: -CodeGraph is a local-first code intelligence library + CLI + MCP server. It parses any supported codebase with tree-sitter, stores symbols/edges/files in SQLite (FTS5), and exposes a knowledge graph to AI agents (Claude Code, Cursor, Codex CLI, opencode) over MCP. Per-project data lives in `.codegraph/`. Extraction is deterministic — derived from AST, not LLM-summarized. +@AGENTS.md -Distributed as `@colbymchenry/codegraph` on npm; same binary serves as installer, indexer, and MCP server. +## Claude-only notes -## Build, Test, Run - -```bash -npm run build # tsc + copy schema.sql and *.wasm + build the viewer into dist/; chmods dist/bin/codegraph.js -npm run build:lib # the viewer's components as @colbymchenry/codegraph-ui (ui/dist) — NOT part of `build` -npm run dev # tsc --watch -npm run clean # rm -rf dist - -npm test # vitest run (all) -npm run test:watch -npm run test:eval # only __tests__/evaluation/ -npm run eval # build then run __tests__/evaluation/runner.ts via tsx - -npm run cli # build then run the local dist binary - -# Single test file / pattern -npx vitest run __tests__/installer-targets.test.ts -npx vitest run __tests__/extraction.test.ts -t "TypeScript" -``` - -`copy-assets` (called from `build`) copies `src/db/schema.sql` and all `src/extraction/wasm/*.wasm` files into `dist/`. **Any new SQL or grammar wasm must be copied or it won't ship.** - -One other build step writes into `dist/` and is subject to the same rule: `build:ui` builds the -browser viewer into `dist/viewer/` (never `dist/ui/` — that's the terminal ui). -`scripts/check-ui-build.mjs` asserts both `dist/viewer/` and the copied grammars in -`dist/extraction/wasm/` after every build and inside every release archive — the viewer's syntax -highlighting reads a file with the same grammar the engine indexed it with, so a missing wasm is an -unhighlighted screen as well as an extraction gap. - -`npm run build:lib` is separate and does NOT run as part of `npm run build`: it compiles the same -`ui/src` tree a second way, with `svelte-package`, into `ui/dist` — the `@colbymchenry/codegraph-ui` -component library the Pro app imports (task CG-61). `scripts/check-ui-package.mjs` then prunes the -standalone app's shell out of it, resolves the extensionless import specifiers `svelte-package` -leaves behind, and asserts the seam: nothing outside `lib/adapter.js` may reach the network. The -package is **prepared, not published** — `ui/package.json` carries `"private": true` deliberately, -and `scripts/pack-npm.sh` only packs a tarball when `CODEGRAPH_PACK_UI=1`. - -Tests run as **two vitest projects** (`vitest.workspace.mts`): `engine` (node) and `ui` (jsdom, the -Svelte plugin, `resolve.conditions: ['browser']`) for the single `__tests__/ui-package.test.ts`. -`npm test` still runs both. The split is not cosmetic — `browser` is a package-resolution -condition, and applied globally it hands the engine's suites the browser builds of -`web-tree-sitter` and friends. The root config (`vitest.config.mts`, `.mts` because the plugin is -ESM-only and the repo is CJS) is the shared base; note that a workspace project **concatenates** -the base's `include` with its own, which is why the `ui` project does not `extends` it. - -Node engines: `>=20.0.0 <25.0.0`. There is a hard exit on Node 25.x and below 20 (see `src/bin/node-version-check.ts`). - -## Architecture - -### Layered pipeline - -``` -files → ExtractionOrchestrator (tree-sitter) → DB (nodes/edges/files) - ↓ - ReferenceResolver (imports, name-matching, framework patterns) - ↓ - GraphQueryManager / GraphTraverser (callers, callees, impact) - ↓ - ContextBuilder (markdown/JSON for AI consumption) -``` - -The public API surface is `src/index.ts` — the `CodeGraph` class wires all the layers and re-exports types. Library users only touch this file; the MCP server and CLI also drive it. - -### Module layout - -- `src/index.ts` — `CodeGraph` class: `init`/`open`/`close`, `indexAll`, `sync`, `searchNodes`, `getCallers`/`getCallees`, `getImpactRadius`, `buildContext`, `watch`/`unwatch`. -- `src/db/` — `DatabaseConnection`, `QueryBuilder` (prepared statements), `schema.sql`, `sqlite-adapter.ts`. Backed by Node's built-in **`node:sqlite`** (`DatabaseSync`) — real SQLite with WAL + FTS5, exposed through a thin better-sqlite3-shaped adapter. The bundled runtime always ships Node ≥22.5, so `node:sqlite` is always available: **no native build step and no wasm fallback**. (Running from source needs Node ≥22.5.) `codegraph status` reports the live backend (`node-sqlite`, the sole backend). -- `src/extraction/` — `ExtractionOrchestrator`, tree-sitter wrappers, per-language extractors under `languages/` (one file per language), plus standalone extractors for non-tree-sitter formats (`svelte-extractor.ts`, `vue-extractor.ts`, `liquid-extractor.ts`, `dfm-extractor.ts` for Delphi). `parse-worker.ts` runs heavy parsing off the main thread. -- `src/resolution/` — `ReferenceResolver` orchestrates `import-resolver.ts` (with `path-aliases.ts` for tsconfig path aliases + cargo workspace member globs), `name-matcher.ts`, and `frameworks/` (Express, Laravel, Rails, FastAPI, Django, Flask, Spring, Gin, Axum, ASP.NET, Vapor, React Router, Next.js — `nextjs.ts`: pages and `route.ts` handlers from files, `router.push` / `redirect` / `NextResponse.redirect` as `navigates` edges, with `next-router-synthesizer.ts` for `` — Expo Router, SvelteKit, Vue/Nuxt, Cargo workspaces). Frameworks emit `route` nodes and `references` edges. `callback-synthesizer.ts` holds the whole-graph synthesis passes (`SYNTH_PASSES`, merged in registry order — first-seen wins a duplicate pair) with the language gates; `tier-synthesizer.ts` is the cross-tier pass (a client's literal `fetch`/`axios` path onto its own route, a queue job onto its consumer, a bus / socket event onto its handler — `channel`, `tier`, `registeredAt` on every edge; registered before the in-process emitter pass so its more specific edge wins); `synth-utils.ts` has the helpers they share (`enclosingFn`, `enclosingValue`, `makeLineAt`). Express's `postExtract` composes `app.use('/prefix', router)` mounts onto a mounted file's route names, idempotently (the original path stays in `qualifiedName`). -- `src/graph/` — `GraphTraverser` (BFS/DFS, impact radius, path finding) and `GraphQueryManager` (high-level queries), plus the shared query-time derivations more than one surface renders: `named-symbol-flow.ts` (the one path finder, behind `codegraph_explore`'s Flow section and the viewer's Flow strip), `dynamic-boundary-report.ts` (where the graph stops), `type-hierarchy.ts` (ancestors/subtypes and the implementation count explore prints and the viewer draws), - `dead-code.ts` (unreferenced symbols, and every reason a candidate is NOT claimed). A derivation that two callers render must live here, not in `ToolHandler` — two derivations eventually disagree. -- `src/context/` — `ContextBuilder` + formatter for markdown/JSON output. -- `src/search/` — full-text query parser and helpers for FTS5. -- `src/sync/` — `FileWatcher` (native FSEvents/inotify/RDCW) with debounce + filter, and git-hook helpers. -- `src/mcp/` — MCP server (`MCPServer`, `tools.ts`, `transport.ts`). `server-instructions.ts` is what the server returns in the MCP `initialize` response — keep it in sync with the user-facing tool guidance. -- `src/installer/` — see below. -- `src/bin/codegraph.ts` — CLI (commander). Subcommands: `install`, `init`, `uninit`, `index`, `sync`, `status`, `query`, `files`, `context`, `affected`, `serve --mcp`. -- `src/ui/` — terminal UI (shimmer progress, worker). -- `src/ui-server/` — the `codegraph ui` browser viewer's read-only JSON API (`api/`: one module per endpoint — `node`, `flow`, `map`, `screens`, `steps`, `deadcode`, `trails`…) and static server; the Svelte viewer itself lives in `ui/` (see `docs/design/codegraph-ui-design-spec.md`). `api/screens.ts` (the app as screens and transitions) and `api/steps.ts` (what happens from a screen, an endpoint or a symbol, as typed steps — screens, handlers, native bridge calls and events, store actions, calls that leave the index) share one fold: everything between two boxes is `via`, and the branch guards along it join into `when` (`graph/branch-guards.ts`, read at request time). `api/program.ts` is the SECOND reading of that same walk (spec §3.13.1): the anchor's body as a block tree — items in source order, a fork wherever two sites are arms of one decision, a helper drawn in place, an arm that answers or leaves ending there — which `ui/src/lib/program-model.ts` turns into the canvas's graph of what happens NEXT (a line means "and then", a row down is one more thing already done). It is pure over the records `steps.ts` keeps while it walks (`ProgramSite`), so the rail and the tree can never hold different steps; what makes the fold possible is that a guard names the DECISION it belongs to (`BranchGuard.branch`), not only its own words. Two helpers sit beside them: `api/route-roots.ts` (where a route's code starts — the handler a resolver named, the page a screen file exports, or the route itself for an inline handler; one rule for every framework) and `api/effects.ts` (the curated table of calls that leave the index — database / response / queue / email / payments / cache / auth / process / network / storage / device / telemetry — matched on the call **as written**, per language family, plus the model / read-write and the response status). `graph/branch-guards.ts` reads, from one cached tree per file, the conditions a site runs under (each with the branching construct it belongs to and how its arm leaves), the loops it is written inside, what it passes, the call as written (the index keeps only the last segment of a deep member chain), the decorators on a definition and the declared types of a class's members — for JS/TS, Swift, Python, Java, Kotlin, C#, Go and C; a language without rules yields nothing, never a wrong label. `steps.ts`'s `crossing()` reads an edge's `tier` / `channel` marker before the languages, so a synthesized cross-tier hop between two TS files draws as a bridge (an endpoint reached over HTTP — a boundary like another screen, entered with `through=1`) or an event (a job, an event, a message arriving); a Next server action is marked at request time from its `'use server'` directive (`api/when.ts`'s `directive`). - -### NodeKind / EdgeKind - -Defined in `src/types.ts`. Both extractors and resolvers must use these exact strings. - -- **NodeKind**: `file`, `module`, `class`, `struct`, `interface`, `trait`, `protocol`, `function`, `method`, `property`, `field`, `variable`, `constant`, `enum`, `enum_member`, `type_alias`, `namespace`, `parameter`, `import`, `export`, `route`, `component`, `union`. -- **EdgeKind**: `contains`, `calls`, `imports`, `exports`, `extends`, `implements`, `references`, `type_of`, `returns`, `instantiates`, `overrides`, `decorates`. - -### Multi-agent installer - -`src/installer/` is the entry point for `codegraph install` (and the bare `codegraph`/`npx @colbymchenry/codegraph` invocation). Architecture: - -- `targets/registry.ts` lists every supported agent. -- `targets/types.ts` defines the `AgentTarget` interface — adding a 5th agent (Continue, Zed, Windsurf…) is **one new file in `targets/` + one entry in `registry.ts`**. Each target owns its config-file location and MCP-server JSON/TOML/JSONC writing. (Targets no longer write an instructions file — see below.) -- Current targets: `claude.ts`, `cursor.ts`, `codex.ts`, `opencode.ts`. -- `targets/toml.ts` is a hand-rolled TOML serializer scoped to `[mcp_servers.codegraph]` (used by Codex). Sibling tables and `[[array_of_tables]]` are preserved verbatim. No new dependency. -- opencode reads `opencode.jsonc` by default; the installer prefers existing `.jsonc`, falls back to `.json`, and creates `.jsonc` for greenfield installs. Edits are surgical via `jsonc-parser` so user comments and formatting survive install/re-install/uninstall round-trips. -- `instructions-template.ts` no longer holds an instructions body — it exports only the ``/`` markers. The installer **stopped writing** a `## CodeGraph` block into each agent's instructions file (`CLAUDE.md` / `~/.codex/AGENTS.md` / `~/.config/opencode/AGENTS.md` / `~/.gemini/GEMINI.md` / `.cursor/rules/codegraph.mdc` / Kiro steering doc) because it duplicated the MCP `initialize` instructions verbatim (issue #529). Each target's `install` (self-heal on upgrade) and `uninstall` use the markers to **strip** a block a previous install left behind. `server-instructions.ts` is the single source of truth for agent-facing guidance. -- All installer changes need matching coverage in `__tests__/installer-targets.test.ts` — there are ~47 parameterized contract tests covering install idempotency, sibling preservation, uninstall reverses install, byte-equal re-runs returning `unchanged`, and partial-state recovery for Codex. - -### Cursor MCP working-directory quirk - -Cursor launches MCP subprocesses with the wrong cwd and doesn't pass `rootUri` in `initialize`. The installer injects `--path` into Cursor's MCP args — absolute path for local installs, `${workspaceFolder}` for global installs. If you touch Cursor wiring, preserve this. - -### MCP server instructions - -`src/mcp/server-instructions.ts` is sent back to the agent in the MCP `initialize` response. This is the *first* thing every agent sees about how to use the tools, and as of issue #529 it is the **single source of truth** for agent-facing tool guidance — the installer no longer writes a duplicate `## CodeGraph` instructions block into `CLAUDE.md` / `AGENTS.md` / `.cursor/rules/codegraph.mdc`. Edit tool guidance here and nowhere else. - -## Retrieval performance & dynamic-dispatch coverage (do not regress) - -CodeGraph's core value is letting an agent answer **structural/flow** questions ("how does X reach Y", trace, impact, callers) with a few **fast** codegraph calls and **zero Read/Grep**. The optimization target is **wall-clock latency + tool-call count** — *don't optimize for token cost*. (Cost is **lower**, not "flat" as earlier framing claimed: a current-build with-vs-without A/B across the 7 README repos, median of 4, saved on average **35% cost · 57% tokens · 46% time · 71% tool calls** — reproducing the published README. The mechanism is **far fewer turns over a much smaller accumulated context** — NOT cache-ability: the without-arm's huge token volume is *mostly* cheap cache-reads, which is why token-count savings (57%) look bigger than cost savings (35%). Measure tokens by **summing per-turn assistant usage**, not `result.usage` (last-turn only in current Claude Code). See `docs/benchmarks/call-sequence-analysis.md`.) The mechanism that drives everything here: **an agent falls back to Read/Grep the instant a codegraph answer is insufficient.** So every change is judged by one question — is codegraph's answer sufficient enough to *stop* the agent from reading? - -**Target behavior:** a flow question resolves in **1 codegraph call on small repos, scaling to 3–5 on large**, with **Read/Grep = 0**. When reviewing a PR or trying something new, do not regress this. - -### Adapt the tool to the agent — don't try to change the agent - -The lever that decides whether a retrieval change lands. **Test before building anything here: does this make a tool the agent _already calls_ do more with the input it _already gives_? If it instead needs the agent to behave differently — pick a different tool, query differently, learn from examples — it hits the low-salience wall and won't land.** - -CodeGraph's only channels to influence the agent are low-salience: the MCP `initialize` instructions (`server-instructions.ts`) and the tool descriptions. Changing them does **not** reliably move the agent's tool _choice_ or query style — validated: trace-first steering ported into the server-instructions + tool descriptions (3 wording variants) never reproduced what a CLI `--append-system-prompt` achieved, and **regressed** wall-clock vs baseline. New tools fare worse (rarely chosen — the agent under-picks even `trace`); "better examples" is the same steering. The agent's tool-choice does improve on its own as host models get better at tool use — but that is not ours to force. - -What works is meeting the agent where it already is: -- **explore-flow** — `codegraph_explore` is the PRIMARY tool the agent reliably calls; its query is a precise bag of symbol names (incl. qualified `Class.method`) spanning the flow the agent is after; explore finds the call path _among those named symbols_ (riding synthesized edges) and leads its output with it. (`buildFlowFromNamedSymbols`: segment/co-naming disambiguation; ≤1 unnamed bridge so it never wanders a god-function's fan-out. Overload-aware: a PascalCase type token in the query biases an overloaded name to that type's own def — `DataRequest task` → DataRequest's `task`, not the abstract base; named-symbol files sort first.) -- **Sufficiency** — make the tool's output complete enough that the agent stops. `codegraph_node` returns the full body + the caller/callee trail, and for an AMBIGUOUS name returns **every overload's body in one call** (so the agent never Reads a file to find the right overload — validated on Alamofire/gin). This is the after-explore depth tool (labeled SECONDARY). -- **Errors teach abandonment** — one or two `isError: true` responses early in a session and the agent stops calling codegraph entirely (maintainer-observed, repeatedly). `isError` is reserved for genuine "stop trying" cases: security refusals (`PathRefusalError`) and real malfunctions (which carry a retry-once note). Every expected/recoverable condition — project not indexed, symbol not found, file not in the index — returns a **SUCCESS-shaped response carrying the guidance** (`NotIndexedError` → `textResult`, see `ToolHandler.execute`'s catch). The same principle is why the tool surface is **always exposed, even at an un-indexed root** (the old empty-`tools/list` gate was removed in #964 — it broke monorepos where only sub-projects carry a `.codegraph/`, and hid the tools from a session that started before `codegraph init`): safety comes from the response SHAPE (success-shaped guidance, never `isError`), not from hiding tools. An un-indexed root's `initialize` sends a per-project variant (`SERVER_INSTRUCTIONS_NO_ROOT_INDEX` — "pass `projectPath` to a project that has a `.codegraph/`"), not an "inactive" note; indexing is still deliberately the user's call, never the agent's. - -What fails is the inverse — folding a precise answer into a **fuzzy-input** tool: the now-removed `codegraph_context` took a description, not symbols, so it couldn't disambiguate a flow's endpoints and surfaced the _wrong feature_ (which is why it was cut). Precise output needs precise input — explore takes a symbol bag for exactly this reason. (`codegraph_trace` was likewise removed: explore-flow does its job and the agent under-picked it.) - -The remaining lever under this axis is **coverage**: every flow made to connect statically (a new dynamic-dispatch synthesizer, or extracting symbols static parsing skipped — e.g. object-literal store actions in `create((set,get)=>({...}))`) is then surfaced automatically by explore-flow, no agent change needed. Reactive/reconciler runtimes (Halo's `ReactiveExtensionClient`, MediatR, Vue Proxy) are the frontier — flows there have no static edges, so nothing surfaces (correctly — silent beats wrong). Full investigation + A/B record: `docs/benchmarks/call-sequence-analysis.md` + auto-memory `project_codegraph_read_displacement`. - -### Explore budget — keep BOTH budgets monotonic with repo size - -Two functions in `src/mcp/tools.ts` scale explore with indexed file count. This is the expected resolution (a regression here silently forces agents back to Read): - -| Repo | files | explore calls | chars/call | per-file | -|---|---|---|---|---| -| express (small) | 147 | 1 | 18K | 3800 | -| excalidraw/django (medium) | 643–3043 | 2 | 28K | 6500 | -| vscode (large) | 10446 | 3 | 35K | 7000 | -| ~20k / ~40k | — | 4 / 5 | 38K | 7000 | - -- `getExploreBudget(fileCount)` → **call** budget: `<500→1, <5000→2, <15000→3, <25000→4, ≥25000→5` (max 5). -- `getExploreOutputBudget(fileCount)` → **per-call** output (chars / files / per-file). **Invariant: a larger tier must never get a smaller `maxCharsPerFile` than a smaller tier.** (Regression that motivated this doc: the `<5000` tier's 2500 was *below* the `<500` tier's 3800, so on a god-file repo — excalidraw's 415 KB `App.tsx` — one explore returned <1% of the file and forced a Read.) -- Explore output must **never tell the agent to "use Read"** — steer to another `codegraph_explore` and "treat returned source as already Read." - -### Dynamic-dispatch coverage — the flow must EXIST in the graph end-to-end - -Static tree-sitter extraction misses computed/indirect calls, so flows break at dynamic dispatch and the agent reads to reconstruct them. Synthesizers/resolvers bridge these so `codegraph_explore` connects them end-to-end (`src/resolution/callback-synthesizer.ts`, `src/resolution/frameworks/`). Channels today: callback/observer, EventEmitter, **React re-render** (`setState`→`render`), **JSX child** (`render`→child component), **React Native native→JS events** (`sendEvent(withName:)` / JVM `emit` → the `addListener` handler, named or inline, `rn-event-channel`), django ORM descriptor. The JS→native direction is a *resolver* (`frameworks/react-native.ts`: `RCT_EXPORT_METHOD`, `RCT_EXTERN_MODULE` Swift shims, TurboModules), which trusts receiver evidence — an alias bound to `NativeModules.X` — over the import resolver. All synthesized edges are `provenance:'heuristic'` with `metadata.synthesizedBy` + `registeredAt` (the wiring site), surfaced inline in `codegraph_explore`'s Flow section and the `codegraph_node` trail. - -**Principle: partial coverage is WORSE than none.** Bridging one boundary but not the next reveals a hop the agent then drills + reads to finish. Measured on excalidraw: react-render alone *raised* reads to 5–7; only completing the flow (adding the jsx-child hop) dropped it to 0–1. **Always close the flow end-to-end and re-measure** — never ship a half-bridged flow. - -### Validation methodology (REQUIRED for every new language/framework) - -For each **language × framework**, validate on **small, medium, and large** real repos with **≥3 different flow prompts** each: - -1. **Pick the canonical flow** for the framework ("how does X reach Y": state→render, request→handler→view, query→SQL, action→reducer→store…). -2. **Deterministic probes** (`scripts/agent-eval/probe-{node,explore}.mjs` against the built `dist/`): `codegraph_explore` with the flow's symbol names connects from→to end-to-end with no break (its Flow section shows the path); **no node explosion** (`select count(*) from nodes` stable before/after re-index); synthesized-edge **precision** spot-check (`select … where provenance='heuristic'`). -3. **Agent A/B** (`scripts/agent-eval/run-all.sh ""`): with vs without codegraph, **≥2 runs/arm** (run-to-run variance is large — never conclude from n=1). Record **duration, total tool calls, Read, Grep**. Optional forced-Read-0 sufficiency proof via the block-read hook (`scripts/agent-eval/hook-settings.json`). - - **Every run also reports three feedback metrics** — residual context occupancy, explore sufficiency (what the agent did NEXT after each explore), and allocation efficiency (share of returned bytes the answer cited) — under each run, plus a side-by-side arm table (`compare-arms.mjs`). Entry point: `docs/benchmarks/agent-eval-feedback-metrics.md`. Reading them: `Read a file we returned` is an allocation miss, `Read a file we did NOT return`/`Grep` is recall; allocation efficiency is **relative** (attribution is by citation) so it is only valid between builds on the same question; occupancy *shares* are Claude Code / 200k and don't transfer to another host — the arm ratio does. - - **The `codegraph` CLI is blocked in every arm** (`no-cli-shim.sh`: sanitized PATH + a PreToolUse hook, shared by both harnesses). Without it 14 of 15 without-arm runs in one 7-repo pass reached codegraph through Bash. Check the contamination row before believing any number: `CLI calls that RETURNED output` > 0 invalidates the run (in a new-vs-baseline A/B it silently drops calls from all three metrics, since a CLI explore is not a tool call). - - **Model policy — every A/B arm runs Claude with `--model sonnet --effort high`. Always. Never Opus/Fable.** All `scripts/agent-eval/*.sh` default to this (`MODEL`/`EFFORT` env override exists — don't raise it without an explicit reason from the maintainer). Two reasons, and the second matters more than cost: (a) Sonnet doesn't burn tokens; (b) **Sonnet is the deliberate floor model** — codegraph's real users attach it to whatever agent they already run (Cursor Composer, Gemini, etc.), so we validate on a "dumber" model on purpose: a stronger model's tool-use covers up the salience/sufficiency problems a weaker one exposes. An affordance that lands on Sonnet generalizes up to every host; one that only works on Opus/Fable doesn't generalize down to the agents most users actually have. Both arms always use the same model. - - **MCP attach is a startup-latency issue, not a hard block.** On a multi-step task the agent dives into Read/grep before codegraph finishes its ~2-3s startup (worse when the eval is itself run nested inside a Claude session, under CPU contention), so it runs with no codegraph. Fix: **pre-warm a persistent daemon** for the target (`CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS` high; spawn `serve --mcp --path "" [baseline-ref]` (it bakes in the pre-warm). -4. **Pass bar:** a normal flow question reaches **~0 Read/Grep within the repo's explore-call budget**, runs **faster** than without-codegraph, and shows **no regression on a control repo**. Record the numbers in `docs/design/dynamic-dispatch-coverage-playbook.md` (the coverage matrix). - -Full playbook + per-mechanism design: `docs/design/dynamic-dispatch-coverage-playbook.md` and `docs/design/callback-edge-synthesis.md`. - -### Worked example — Excalidraw (TS/React, medium, 643 files) - -The template to replicate per language/framework. Question: *"how does updating an element re-render the canvas on screen?"* (the full flow crosses three React boundaries: observer callback, `setState`→`render`, and JSX child). - -| Stage | duration | Read | Grep | codegraph | -|---|---|---|---|---| -| Without codegraph | 115–139s | 9–10 | 10–11 | 0 | -| Broken (explore-budget regression) | 131–139s | 5–10 | 3–5 | 6–14 | -| Fixed (budget + msgs + synthesis) | 64–112s | 0–2 | 2–4 | 3–**10** | -| + trace-first steering | **51–74s** | **0–2** | 0–4 | **3–4** | - -n=4 unhooked runs/stage, same prompt. After steering flow questions to `codegraph_trace` first: **best run 0 Read / 0 Grep / 3 codegraph / 51s**; **2 of 4 fully clean** (0 Read, 0 Grep). Steering eliminated the over-drill variance — call count tightened from 3–10 to 3–4, trace adoption went 3/4 → 4/4, and the `search`+`callers` path-reconstruction floundering dropped to 0. Run-to-run variance is still real; report the range, never a single run. **Residual reads/greps are all the nonce data-flow** (`canvasNonce` — a local prop with no graph edges); that's the def-use/data-flow frontier, left deliberately uncovered (tracking every local would explode the graph). Validated: `trace(mutateElement, renderStaticScene)` connects in **6 hops** across all three boundaries (`mutateElement → triggerUpdate → [callback] triggerRender → [react-render] render → [jsx] StaticCanvas → renderStaticScene`), each hop showing inline source + the wiring site; node count stable at 9,289; 1 callback + 46 react-render + 280 jsx-render synthesized edges (no explosion, precision-checked). - -## Tests - -Tests live in `__tests__/` and mirror the module they cover. Notable ones beyond the obvious: - -- `installer-targets.test.ts` — parameterized contract suite across all 4 agent targets (see installer notes above). -- `evaluation/` — `runner.ts` + `test-cases.ts` exercise codegraph against synthetic projects and score the results; run via `npm run eval` (builds first). Not part of `npm test`. -- `sqlite-backend.test.ts` / `node-sqlite-backend.test.ts` — pin that `node:sqlite` is the sole backend: `getBackend()` reports `node-sqlite` and the DB comes up in WAL. -- `pr19-improvements.test.ts`, `frameworks-integration.test.ts` — regression coverage for specific past PRs/incidents; don't rename these, the names anchor to git history. - -Tests create temp dirs with `fs.mkdtempSync` and clean up in `afterEach`. They write real files and exercise real SQLite — there is no DB mocking. - -### Windows-gated tests - -Behavior that differs by platform (path resolution, drive letters, `SENSITIVE_PATHS`, `%APPDATA%` config dirs, CRLF) must be gated, not assumed. Use `it.runIf(process.platform === 'win32')(...)` for Windows-only assertions and `it.runIf(process.platform !== 'win32')(...)` for POSIX-only ones — e.g. `/etc` is sensitive on POSIX but resolves to `C:\etc` (non-existent) on Windows, so an ungated `/etc` assertion fails on Windows. Validate the Windows side for real (see below); don't merge a Windows-gated test you haven't seen run. - -## Cross-platform validation - -The dev machine — and the default `npm test` target — is **macOS**, so local runs cover the macOS path. The other two platforms aren't here; when a change is platform-sensitive (file watching, sockets / named pipes, path & symlink handling, process lifecycle, inotify budget) validate them for real rather than guessing. - -### Linux (Docker) - -When asked to test or validate on Linux, use **Docker** — there's no Linux box, but Docker runs on the macOS host. Build a throwaway image from the repo and run the suite inside it: - -- `FROM node:22-bookworm`; `COPY` the repo with a `.dockerignore` excluding `node_modules`/`dist`/`.git`/`.codegraph`; `RUN npm ci && npm run build`. Don't reuse the Mac `node_modules` — `esbuild`/`rollup` ship platform-specific binaries. -- Run with **`docker run --rm --init`**. The `--init` is load-bearing for any process-lifecycle test (daemon reaping, the #277 PPID watchdog, idle-timeout): without a zombie-reaping PID 1, a SIGKILL'd/exited process lingers as a zombie and `process.kill(pid, 0)` still reports it *alive*, so exit-detection assertions false-fail even though the process did exit. -- Linux is where the inotify watch budget actually bites: count a process's watches via `/proc//fdinfo/*` (sum `^inotify ` lines on the fd whose `readlink` is `anon_inode:inotify`). - -### Windows (Parallels VM + SSH) - -For any Windows-specific PR, bug, or implementation, validate it on the real Windows VM rather than guessing. Connection details live in the gitignored **`.parallels`** file at the repo root (VM name, guest IP, SSH user/key). `prlctl exec` needs Parallels Pro and is unavailable, so SSH is the bridge. - -- Connect / run from the Mac host: `ssh @ "..."`. For multi-line work, pipe PowerShell over stdin and **refresh PATH from the registry** first (sshd's session has a stale PATH after winget installs): - ``` - ssh colby@10.211.55.3 "powershell -NoProfile -ExecutionPolicy Bypass -Command -" <<'PS' - $env:Path = [Environment]::GetEnvironmentVariable("Path","Machine") + ";" + [Environment]::GetEnvironmentVariable("Path","User") - Set-Location C:\dev\codegraph - PS - ``` -- Clone fresh into a **Windows-local** path (`C:\dev\codegraph`) and `npm ci` there — never run npm against the shared Mac repo, since `esbuild`/`rollup` ship platform-specific binaries. -- Guest toolchain (winget): Node LTS, Git, and the **VC++ ARM64 redistributable** (required by `@rollup/rollup-win32-arm64-msvc`, which vitest pulls in). -- Fetch a contributor PR head straight from their fork to dodge `pull//head` lag: `git fetch ` then `git checkout -f FETCH_HEAD`. -- Known pre-existing Windows failures (they reproduce on `main`, unrelated to your change — confirm against `origin/main` before blaming your PR, and don't let them mask new regressions): `security.test.ts > Session marker symlink resistance > does not follow a pre-planted symlink` (symlink creation needs privileges on Windows); and the `mcp-initialize.test.ts` / `mcp-roots.test.ts` suites, which fail in `afterEach` with `EPERM` removing the temp dir because a spawned `serve --mcp` (its `--liftoff-only` re-exec grandchild) still holds the cwd / SQLite file open — a Windows file-locking quirk, not a logic bug. - -## Releases - -Released to npm and mirrored as [GitHub Releases](https://github.com/colbymchenry/codegraph/releases). `CHANGELOG.md` is the source of truth; GitHub Release notes are extracted from it. - -### Writing changelog entries - -**Default: write entries under `## [Unreleased]`** — that's the section reserved for work landing between releases. **Don't pre-create a `## [X.Y.Z]` block** for the next release: the Release workflow's first step is `scripts/prepare-release.mjs`, which automatically promotes everything under `[Unreleased]` into a new `## [X.Y.Z] - ` block at release time (or merges into a pre-existing `[X.Y.Z]` block if one exists — but you don't need one). Pre-staging is what caused the v0.9.5 sparse-release-notes incident: a sparse `[0.9.5]` block hand-added before the rest of the work landed got picked by the extractor over the much-larger `[Unreleased]` section above it. Don't do that. - -Formatting rules for any entry (anywhere — `[Unreleased]` or otherwise): - -1. **Write friendly, user-facing notes — not engineer-facing ones.** Group under `### New Features` and `### Fixes` (sentence-case). Surface `### Breaking Changes` and `### Security` as their own sections **only when the release has them**; fold improvement-flavored changes into New Features. Omit empty sections. (This replaces the old Keep-a-Changelog `Added/Changed/Fixed/Removed/Deprecated` grouping: the GitHub Release page extracts each version block **verbatim** via `scripts/extract-release-notes.mjs`, and the old dense, implementation-focused entries rendered as an unreadable wall of text — so the whole CHANGELOG was rewritten to this format and every published release re-noted to match.) -2. **One plain-language sentence per bullet:** what changed and why it matters to a user. Lead with the capability, or with the symptom that's now fixed. -3. **Strip the internals.** No internal file paths (`src/...`), no internal symbol / function / class names, no benchmark numbers / percentages / node-or-edge counts. **Keep:** language & framework names (Go, Spring, NestJS, …), things a user types or sets (`codegraph install`, `codegraph_explore`, the `CODEGRAPH_*` env vars), agent / IDE names (Claude Code, Cursor, opencode, Kiro, …), and a brief `Thanks @user` when a contributor is credited. -4. Issue / PR references in entries are by number (`(#403)` etc.); the GitHub renderer auto-links them in the published release notes. -5. **Don't add a `[X.Y.Z]: https://...` link reference yourself** — `prepare-release.mjs` appends it automatically when it promotes the version (idempotent: a re-run is a no-op if it already exists). -6. **Every release opens with a `### Highlights` block — the only part most people read.** At most ~8 one-line bullets, in plain language for someone who doesn't read code, ordered by what a typical user notices first (new agent/IDE support and setup changes, then answer quality, then reliability), plus a one-sentence upgrade note when a re-index is needed. Write or refresh it in `[Unreleased]` when a release is being prepared — not per PR — and keep the detailed `### New Features` / `### Fixes` entries below it. When `### Fixes` grows past ~15 entries, group them under `####` sub-headings (`Better answers from codegraph_explore`, `Finding your project, live updates, and the CLI`, `Indexing reliability and disk usage`, `Language and framework accuracy`) so a skimmer can find their area. - -Multi-word headings like `### New Features` are safe on the normal release path: `prepare-release.mjs` **Case A** moves the whole `[Unreleased]` body verbatim into `[X.Y.Z]`. (Only its rarely-used **Case B** *merge* splits sub-sections with a single-word `^### (\w+)$` regex that wouldn't match them — and Case B fires only if a `[X.Y.Z]` block was pre-created, which rule above already forbids.) - -### Release flow (the user runs these) - -Releases are built and published by the **GitHub Actions "Release" workflow** -(`.github/workflows/release.yml`). It runs `scripts/prepare-release.mjs` to -promote `[Unreleased]` into `[]` (and auto-commit + push that -CHANGELOG change back to `main` so on-disk truth matches the published -notes), then bundles a Node runtime per platform (`scripts/build-bundle.sh`) -and publishes both the GitHub Release and the npm thin-installer -(`scripts/pack-npm.sh`: a shim package + per-platform packages). -Publishing manually is **wrong** now — a plain `npm publish` ships the root -package (non-bundled), which breaks anyone on Node < 22.5. - -**Claude does NOT bump the version unless explicitly asked.** The maintainer -typically does it themselves — often by editing `package.json` directly via -the GitHub web UI. Don't proactively commit a version bump as part of -unrelated work, and don't propose one when summarizing a PR. - -When the maintainer DOES bump the version, the only edit strictly required is -to `package.json` — the workflow's "Sync package-lock.json" step detects a -mismatch between `package.json` and `package-lock.json`, runs -`npm install --package-lock-only --ignore-scripts` to rewrite the lock file's -version fields (top-level + `packages.""`), and auto-commits + pushes the -result back to `main` with `[skip ci]`. So a GitHub-web-UI single-file edit to -`package.json` is enough to kick off a clean release. (If they edit both files -locally, that's fine too — the sync step no-ops.) - -Once `package.json` is at the target version on `main`, trigger -**Actions → Release → Run workflow** (on `main`). The workflow: - -1. Syncs `package-lock.json` to `package.json`'s version if they've drifted; commits + pushes that change. -2. Runs `prepare-release.mjs ` → promotes `[Unreleased]` → `[X.Y.Z] - ` in `CHANGELOG.md`, appends the link reference, commits + pushes the move with `[skip ci]`. -3. Builds every platform bundle on one runner, generates `SHA256SUMS`. -4. Creates the GitHub Release with notes from the freshly-promoted `[X.Y.Z]` block. -5. Publishes the npm shim + per-platform packages. Requires the `NPM_TOKEN` repo secret. - -**Do not run `npm publish`, `git push`, or `git tag` yourself** — these are -publish actions on shared state. Write the files, hand the user the commands. - -## House rules - -- The `0.7.x` line is in active multi-agent rollout. Any change to `src/installer/` (especially `targets/`) needs corresponding test coverage and a CHANGELOG entry — installer regressions break every new install silently. -- When changing what the MCP tools do or how agents should use them, edit `src/mcp/server-instructions.ts` — it is the **single source of truth** for agent-facing tool guidance (issue #529). The installer no longer writes a duplicate instructions block into `CLAUDE.md` / `AGENTS.md` / `GEMINI.md` / `.cursor/rules/codegraph.mdc` / Kiro steering, so there's nothing to keep in sync anymore. (The repo's own checked-in `.cursor/rules/codegraph.mdc` is dogfooding config — update it too if you use Cursor on this repo, but it ships nowhere.) -- **Before adding or extending a router, a web framework, or a language's `WHEN` rules, read `docs/design/framework-coverage.md`.** It is the standing answer to "what is supported and what is left" across the three axes (route nodes → Entry points, `navigates` edges → Screens, branch-guard rules → the `WHEN` labels), with what each remaining item needs, the traps that have already cost debugging time, and the queries to re-verify it. Update it in the same change that moves a row. -- CodeGraph provides **code context**, not product requirements. For new features, ask the user about UX, edge cases, and acceptance criteria — the graph won't tell you. -- **When the user references issues, PR comments, or external reports, anchor them to a date and version before drawing conclusions.** Check the comment's `createdAt` against: - - The **last released version** — `grep -m1 '^## \[' CHANGELOG.md` shows the top-of-file version (older releases follow). A comment dated before the latest `## [X.Y.Z] - YYYY-MM-DD` is reacting to *released* state — work that's only on `main` or on an unmerged branch doesn't apply. - - The **last main commit** — `git log --first-parent main -1 --format='%ai %h %s'`. A comment after the last release but before a fix on main may already be addressed there but unreleased. - - The **current branch's tip** — your own unmerged work obviously can't be what the comment is reacting to. - Always disambiguate "released," "merged-but-unreleased," and "in-progress" before agreeing that a user-reported problem is unfixed (or that a fix is incomplete). A user saying "your fix only covers X" about a recent PR is usually pointing at the *released* shortcomings — your in-flight branch may already address them but they have no way to know that. -- **Version-tag every image referenced in `README.md`.** GitHub caches README images (`raw.githubusercontent.com` with a 5-minute TTL; third-party hosts sit behind the long-lived camo proxy), so updating an asset in place can keep showing the stale version. Give each README image URL a `?v=N` query tag and **bump `N` in the same commit whenever the asset bytes change** — e.g. `assets/waitlist.svg?v=2`. The changed URL sidesteps every cache so the new image shows immediately instead of waiting on a TTL to expire. +- Prefer the repo-root `AGENTS.md` as the source of truth for build/test/architecture/house rules. Edit that file (not this wrapper) when guidance changes. +- Claude Code can `@`-import nested guides too (e.g. `@docs/AGENTS.md`) when working on design/eval docs; Codex loads nested `AGENTS.md` automatically when the session cwd is under that directory. +- Do not reintroduce a duplicated `## CodeGraph` MCP tool-guidance block here — `src/mcp/server-instructions.ts` is the single source of truth (issue #529); the installer strips legacy marker blocks on upgrade. diff --git a/docs/AGENTS.md b/docs/AGENTS.md new file mode 100644 index 000000000..4ce220c2d --- /dev/null +++ b/docs/AGENTS.md @@ -0,0 +1,36 @@ +# AGENTS.md (docs/) + +Nested Codex guidance under `docs/`. Loaded with the repo-root `AGENTS.md` when cwd is under `docs/` (Codex walks root to cwd; shared `project_doc_max_bytes` budget). + +Root `AGENTS.md` already carries the non-negotiable retrieval principles (adapt-the-tool, explore budgets, end-to-end synthesis). This file holds the longer validation methodology and Excalidraw worked example that were moved out of root to fit the budget. + +### Validation methodology (REQUIRED for every new language/framework) + +For each **language × framework**, validate on **small, medium, and large** real repos with **≥3 different flow prompts** each: + +1. **Pick the canonical flow** for the framework ("how does X reach Y": state→render, request→handler→view, query→SQL, action→reducer→store…). +2. **Deterministic probes** (`scripts/agent-eval/probe-{node,explore}.mjs` against the built `dist/`): `codegraph_explore` with the flow's symbol names connects from→to end-to-end with no break (its Flow section shows the path); **no node explosion** (`select count(*) from nodes` stable before/after re-index); synthesized-edge **precision** spot-check (`select … where provenance='heuristic'`). +3. **Agent A/B** (`scripts/agent-eval/run-all.sh ""`): with vs without codegraph, **≥2 runs/arm** (run-to-run variance is large — never conclude from n=1). Record **duration, total tool calls, Read, Grep**. Optional forced-Read-0 sufficiency proof via the block-read hook (`scripts/agent-eval/hook-settings.json`). + - **Every run also reports three feedback metrics** — residual context occupancy, explore sufficiency (what the agent did NEXT after each explore), and allocation efficiency (share of returned bytes the answer cited) — under each run, plus a side-by-side arm table (`compare-arms.mjs`). Entry point: `docs/benchmarks/agent-eval-feedback-metrics.md`. Reading them: `Read a file we returned` is an allocation miss, `Read a file we did NOT return`/`Grep` is recall; allocation efficiency is **relative** (attribution is by citation) so it is only valid between builds on the same question; occupancy *shares* are Claude Code / 200k and don't transfer to another host — the arm ratio does. + - **The `codegraph` CLI is blocked in every arm** (`no-cli-shim.sh`: sanitized PATH + a PreToolUse hook, shared by both harnesses). Without it 14 of 15 without-arm runs in one 7-repo pass reached codegraph through Bash. Check the contamination row before believing any number: `CLI calls that RETURNED output` > 0 invalidates the run (in a new-vs-baseline A/B it silently drops calls from all three metrics, since a CLI explore is not a tool call). + - **Model policy — every A/B arm runs Claude with `--model sonnet --effort high`. Always. Never Opus/Fable.** All `scripts/agent-eval/*.sh` default to this (`MODEL`/`EFFORT` env override exists — don't raise it without an explicit reason from the maintainer). Two reasons, and the second matters more than cost: (a) Sonnet doesn't burn tokens; (b) **Sonnet is the deliberate floor model** — codegraph's real users attach it to whatever agent they already run (Cursor Composer, Gemini, etc.), so we validate on a "dumber" model on purpose: a stronger model's tool-use covers up the salience/sufficiency problems a weaker one exposes. An affordance that lands on Sonnet generalizes up to every host; one that only works on Opus/Fable doesn't generalize down to the agents most users actually have. Both arms always use the same model. + - **MCP attach is a startup-latency issue, not a hard block.** On a multi-step task the agent dives into Read/grep before codegraph finishes its ~2-3s startup (worse when the eval is itself run nested inside a Claude session, under CPU contention), so it runs with no codegraph. Fix: **pre-warm a persistent daemon** for the target (`CODEGRAPH_DAEMON_IDLE_TIMEOUT_MS` high; spawn `serve --mcp --path "" [baseline-ref]` (it bakes in the pre-warm). +4. **Pass bar:** a normal flow question reaches **~0 Read/Grep within the repo's explore-call budget**, runs **faster** than without-codegraph, and shows **no regression on a control repo**. Record the numbers in `docs/design/dynamic-dispatch-coverage-playbook.md` (the coverage matrix). + +Full playbook + per-mechanism design: `docs/design/dynamic-dispatch-coverage-playbook.md` and `docs/design/callback-edge-synthesis.md`. + +### Worked example — Excalidraw (TS/React, medium, 643 files) + +The template to replicate per language/framework. Question: *"how does updating an element re-render the canvas on screen?"* (the full flow crosses three React boundaries: observer callback, `setState`→`render`, and JSX child). + +| Stage | duration | Read | Grep | codegraph | +|---|---|---|---|---| +| Without codegraph | 115–139s | 9–10 | 10–11 | 0 | +| Broken (explore-budget regression) | 131–139s | 5–10 | 3–5 | 6–14 | +| Fixed (budget + msgs + synthesis) | 64–112s | 0–2 | 2–4 | 3–**10** | +| + trace-first steering | **51–74s** | **0–2** | 0–4 | **3–4** | + +n=4 unhooked runs/stage, same prompt. After steering flow questions to `codegraph_trace` first: **best run 0 Read / 0 Grep / 3 codegraph / 51s**; **2 of 4 fully clean** (0 Read, 0 Grep). Steering eliminated the over-drill variance — call count tightened from 3–10 to 3–4, trace adoption went 3/4 → 4/4, and the `search`+`callers` path-reconstruction floundering dropped to 0. Run-to-run variance is still real; report the range, never a single run. **Residual reads/greps are all the nonce data-flow** (`canvasNonce` — a local prop with no graph edges); that's the def-use/data-flow frontier, left deliberately uncovered (tracking every local would explode the graph). Validated: `trace(mutateElement, renderStaticScene)` connects in **6 hops** across all three boundaries (`mutateElement → triggerUpdate → [callback] triggerRender → [react-render] render → [jsx] StaticCanvas → renderStaticScene`), each hop showing inline source + the wiring site; node count stable at 9,289; 1 callback + 46 react-render + 280 jsx-render synthesized edges (no explosion, precision-checked). + + +Also see: `docs/design/dynamic-dispatch-coverage-playbook.md`, `docs/design/callback-edge-synthesis.md`, `docs/benchmarks/call-sequence-analysis.md`, `docs/benchmarks/agent-eval-feedback-metrics.md`. From df435d50d1b58612b4dcf2a652406aab5ba015e1 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Mon, 7 Sep 2026 23:43:33 -0500 Subject: [PATCH 02/14] fix(extraction): index TS/JS generator function declarations and expressions (#1741) (#1743) Tree-sitter kinds generator_function_declaration / generator_function were missing from both the wasm and native kernel function-type lists, so function* / async function* (and const g = function* () {}) produced no nodes. Add the kinds on both paths and cover TS+JS declaration/expression forms in extraction tests. Co-authored-by: Colby McHenry --- __tests__/extraction.test.ts | 57 +++++++++++++++++++++++++ codegraph-kernel/src/tsjs/extractors.rs | 6 +-- codegraph-kernel/src/tsjs/mod.rs | 4 +- src/extraction/languages/javascript.ts | 2 +- src/extraction/languages/typescript.ts | 2 +- src/extraction/tree-sitter.ts | 6 +-- 6 files changed, 67 insertions(+), 10 deletions(-) diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index ad0ba2374..158cda835 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -745,6 +745,63 @@ export const fetchData = async () => { }); }); +describe('Generator Function Extraction (#1741)', () => { + const functionNames = (file: string, code: string) => + extractFromSource(file, code) + .nodes.filter((n) => n.kind === 'function') + .map((n) => n.name) + .sort(); + + it('extracts function* and async function* declarations in TypeScript', () => { + process.env.CODEGRAPH_KERNEL = '0'; + const code = ` +function plain() { return 1; } +function* gen() { yield 2; } +async function asyncFn() { return 3; } +async function* asyncGen() { yield 4; } +`; + expect(functionNames('gens.ts', code)).toEqual(['asyncFn', 'asyncGen', 'gen', 'plain']); + }); + + it('extracts function* and async function* declarations in JavaScript', () => { + process.env.CODEGRAPH_KERNEL = '0'; + const code = ` +function plain() { return 1; } +function* gen() { yield 2; } +async function asyncFn() { return 3; } +async function* asyncGen() { yield 4; } +`; + expect(functionNames('gens.js', code)).toEqual(['asyncFn', 'asyncGen', 'gen', 'plain']); + }); + + it('extracts const-assigned generator and async generator expressions (TS)', () => { + process.env.CODEGRAPH_KERNEL = '0'; + const code = ` +const g = function* () { yield 1; }; +const ag = async function* () { yield 2; }; +export const exportedGen = function* () { yield 3; }; +`; + const result = extractFromSource('gen-expr.ts', code); + const names = result.nodes.filter((n) => n.kind === 'function').map((n) => n.name).sort(); + expect(names).toEqual(['ag', 'exportedGen', 'g']); + expect(result.nodes.find((n) => n.name === 'exportedGen')?.isExported).toBe(true); + expect(result.nodes.find((n) => n.name === 'g')?.isExported).toBeFalsy(); + }); + + it('extracts const-assigned generator and async generator expressions (JS)', () => { + process.env.CODEGRAPH_KERNEL = '0'; + const code = ` +const g = function* () { yield 1; }; +const ag = async function* () { yield 2; }; +export const exportedGen = function* () { yield 3; }; +`; + const result = extractFromSource('gen-expr.js', code); + const names = result.nodes.filter((n) => n.kind === 'function').map((n) => n.name).sort(); + expect(names).toEqual(['ag', 'exportedGen', 'g']); + expect(result.nodes.find((n) => n.name === 'exportedGen')?.isExported).toBe(true); + }); +}); + describe('Type Alias Extraction', () => { it('should extract exported type aliases in TypeScript', () => { const code = ` diff --git a/codegraph-kernel/src/tsjs/extractors.rs b/codegraph-kernel/src/tsjs/extractors.rs index f240c5e79..577a879e6 100644 --- a/codegraph-kernel/src/tsjs/extractors.rs +++ b/codegraph-kernel/src/tsjs/extractors.rs @@ -23,7 +23,7 @@ impl<'t> Walker<'t> { // variable_declarator (`export const useAuth = () => {}`). if name_override.is_none() && name == "" - && matches!(node.kind(), "arrow_function" | "function_expression") + && matches!(node.kind(), "arrow_function" | "function_expression" | "generator_function") { if let Some(parent) = node.parent() { if parent.kind() == "variable_declarator" { @@ -342,9 +342,9 @@ impl<'t> Walker<'t> { } let name = self.text(name_node).to_string(); - // Arrow/function values extract as functions, named by the declarator. + // Arrow/function/generator values extract as functions, named by the declarator. if let Some(v) = value { - if matches!(v.kind(), "arrow_function" | "function_expression") { + if matches!(v.kind(), "arrow_function" | "function_expression" | "generator_function") { self.extract_function(v, None); continue; } diff --git a/codegraph-kernel/src/tsjs/mod.rs b/codegraph-kernel/src/tsjs/mod.rs index afe6361d5..7ce4dcd99 100644 --- a/codegraph-kernel/src/tsjs/mod.rs +++ b/codegraph-kernel/src/tsjs/mod.rs @@ -60,7 +60,7 @@ fn is_method_type(v: Variant, kind: &str) -> bool { } fn is_function_type(kind: &str) -> bool { - matches!(kind, "function_declaration" | "arrow_function" | "function_expression") + matches!(kind, "function_declaration" | "generator_function_declaration" | "arrow_function" | "function_expression" | "generator_function") } fn is_class_type(v: Variant, kind: &str) -> bool { @@ -792,7 +792,7 @@ impl<'t> Walker<'t> { if let Some(name_node) = node.child_by_field_name("name") { return self.text(name_node).to_string(); } - if matches!(node.kind(), "arrow_function" | "function_expression") { + if matches!(node.kind(), "arrow_function" | "function_expression" | "generator_function") { return "".to_string(); } for i in 0..node.named_child_count() { diff --git a/src/extraction/languages/javascript.ts b/src/extraction/languages/javascript.ts index 3b36348b3..899e1530a 100644 --- a/src/extraction/languages/javascript.ts +++ b/src/extraction/languages/javascript.ts @@ -3,7 +3,7 @@ import type { LanguageExtractor } from '../tree-sitter-types'; import { classifyTsClassMember } from './typescript'; export const javascriptExtractor: LanguageExtractor = { - functionTypes: ['function_declaration', 'arrow_function', 'function_expression'], + functionTypes: ['function_declaration', 'generator_function_declaration', 'arrow_function', 'function_expression', 'generator_function'], classTypes: ['class_declaration'], methodTypes: ['method_definition', 'field_definition'], // JS `field_definition` ≙ TS `public_field_definition`: plain fields are diff --git a/src/extraction/languages/typescript.ts b/src/extraction/languages/typescript.ts index 72059114f..133aefd05 100644 --- a/src/extraction/languages/typescript.ts +++ b/src/extraction/languages/typescript.ts @@ -39,7 +39,7 @@ export function classifyTsClassMember(node: SyntaxNode): 'method' | 'property' { } export const typescriptExtractor: LanguageExtractor = { - functionTypes: ['function_declaration', 'arrow_function', 'function_expression'], + functionTypes: ['function_declaration', 'generator_function_declaration', 'arrow_function', 'function_expression', 'generator_function'], classTypes: ['class_declaration', 'abstract_class_declaration'], methodTypes: ['method_definition', 'public_field_definition'], classifyMethodNode: classifyTsClassMember, diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 7ef90c273..079b9653e 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -171,7 +171,7 @@ function extractNameRaw(node: SyntaxNode, source: string, extractor: LanguageExt // not from identifiers in their body. Without this, single-expression arrow // functions like `const fn = () => someIdentifier` get named "someIdentifier" // instead of "fn", because the fallback below finds the body identifier. - if (node.type === 'arrow_function' || node.type === 'function_expression') { + if (node.type === 'arrow_function' || node.type === 'function_expression' || node.type === 'generator_function') { return ''; } @@ -1556,7 +1556,7 @@ export class TreeSitterExtractor { if ( !nameOverride && name === '' && - (node.type === 'arrow_function' || node.type === 'function_expression') + (node.type === 'arrow_function' || node.type === 'function_expression' || node.type === 'generator_function') ) { const parent = node.parent; if (parent?.type === 'variable_declarator') { @@ -2623,7 +2623,7 @@ export class TreeSitterExtractor { } const name = getNodeText(nameNode, this.source); // Arrow functions / function expressions: extract as function instead of variable - if (valueNode && (valueNode.type === 'arrow_function' || valueNode.type === 'function_expression')) { + if (valueNode && (valueNode.type === 'arrow_function' || valueNode.type === 'function_expression' || valueNode.type === 'generator_function')) { this.extractFunction(valueNode); continue; } From 7440d2c475c8526e8de62f0d4127b4d47dd621d2 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Mon, 7 Sep 2026 23:56:52 -0500 Subject: [PATCH 03/14] fix(mcp): fail fast on a second direct-mode writer per project (#1740) (#1744) Concurrent CODEGRAPH_NO_DAEMON / in-process fallback serve --mcp instances each started a FileWatcher and contended on codegraph.lock until auto-sync degraded. Add an exclusive .codegraph/writer.pid lock held by the daemon or the single direct writer; a second writer exits with actionable guidance. Daemon mode still multiplexes N proxies onto one writer. Co-authored-by: Colby McHenry --- CHANGELOG.md | 4 + README.md | 2 + __tests__/mcp-writer-lock.test.ts | 128 ++++++++++++++++++++ __tests__/writer-lock.test.ts | 87 ++++++++++++++ src/mcp/daemon.ts | 12 ++ src/mcp/engine.ts | 25 ++++ src/mcp/index.ts | 21 ++++ src/mcp/writer-lock.ts | 188 ++++++++++++++++++++++++++++++ 8 files changed, 467 insertions(+) create mode 100644 __tests__/mcp-writer-lock.test.ts create mode 100644 __tests__/writer-lock.test.ts create mode 100644 src/mcp/writer-lock.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 220298c48..146c20bcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +#### MCP / indexing + +- **A second `codegraph serve --mcp` on the same project no longer silently kills auto-sync (#1740).** Direct mode (`CODEGRAPH_NO_DAEMON=1` or proxy→in-process fallback) now takes an exclusive `.codegraph/writer.pid` lock; a second writer exits immediately with guidance to stop the other server or unset `CODEGRAPH_NO_DAEMON` so clients share the daemon. The shared daemon already multiplexes N clients onto one watcher — this closes the same-OS dual-direct gap the docs warned about for Windows/WSL but did not guard. + #### Screens, links and navigation - **Where the app goes after login is a fork, not two always-es.** A navigation whose destination comes back from a helper — `router.replace(await resolvePostLoginRoute())` over `return (await hasSeenWelcome(…)) ? '/home/' : '/welcome/'` — drew both screens with no condition, reading as if the welcome screen always shows. The two arms share a line, and only a column can tell them apart; each synthesized edge now carries its literal's own position, so the guard reader says which arm it is: `WHEN await hasSeenWelcome(…)` → home, and its negation → welcome. And the scan starts at the helper's body, so a literal-union return type — `Promise<'/welcome/' | '/home/'>`, whose routes are string literals too, written first — no longer stands in for the navigation itself. Re-index after upgrading to pick the positions up. diff --git a/README.md b/README.md index a73d3b2bc..80fa50696 100644 --- a/README.md +++ b/README.md @@ -944,6 +944,8 @@ Framework routing is validated the same way, on a canonical app per framework: E **MCP server not connecting** — Your agent starts the server itself, so you don't launch it by hand. Make sure the project is initialized and indexed (`codegraph status`) and that the path in your MCP config is correct. If it still won't connect, re-run `codegraph install` to rewrite the config. +**Two `codegraph serve --mcp` on one project fight over the index / auto-sync stops** — CodeGraph allows one live MCP *writer* per project (the shared background daemon, or a single direct-mode process). Extra clients should proxy to that daemon. If you set `CODEGRAPH_NO_DAEMON=1`, run only one `serve --mcp` for that project; a second instance exits with a clear writer-lock error (see `writer.pid` under `.codegraph/`). Prefer leaving the daemon enabled so multiple MCP hosts share one watcher. + **MCP tool calls fail with `Transport closed` while `codegraph status`/`sync` are healthy** — almost always WSL2 with the project on a Windows drive (a `/mnt/c` or `/mnt/d` path), where the local socket CodeGraph uses to share one background server across sessions is unreliable. CodeGraph now falls back to serving the session in-process instead of dropping the connection, but if you still hit it, set `CODEGRAPH_NO_DAEMON=1` in your MCP server's environment to skip the shared server entirely (each session runs in its own process). Moving the project onto the Linux-native filesystem (e.g. under `~/` instead of `/mnt/`) restores the shared server. **Missing symbols** — The MCP server auto-syncs on save (wait a couple seconds). Run `codegraph sync` manually if needed. Check that the file's language is supported and isn't inside a `.gitignore`d or default-excluded directory (e.g. `node_modules`, `dist`). diff --git a/__tests__/mcp-writer-lock.test.ts b/__tests__/mcp-writer-lock.test.ts new file mode 100644 index 000000000..be68f944c --- /dev/null +++ b/__tests__/mcp-writer-lock.test.ts @@ -0,0 +1,128 @@ +/** + * Issue #1740 — concurrent direct-mode serve --mcp must fail fast on the + * second writer instead of silently degrading auto-sync. + */ + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { ChildProcessWithoutNullStreams, spawn } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { getWriterPidPath } from '../src/mcp/writer-lock'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +function sleep(ms: number): Promise { + return new Promise((r) => setTimeout(r, ms)); +} + +function spawnMcp( + cwd: string, + env: NodeJS.ProcessEnv, +): { child: ChildProcessWithoutNullStreams; getStderr: () => string } { + const child = spawn(process.execPath, [BIN, 'serve', '--mcp'], { + cwd, + stdio: ['pipe', 'pipe', 'pipe'], + env: { ...process.env, ...env }, + }) as ChildProcessWithoutNullStreams; + child.on('error', () => {}); + child.stdin.on('error', () => {}); + let stderr = ''; + child.stderr.on('data', (c: Buffer) => { stderr += c.toString('utf8'); }); + child.stdout.on('data', () => {}); + return { child, getStderr: () => stderr }; +} + +describe('issue #1740 — direct-mode writer lock', () => { + let tempDir: string; + let realRoot: string; + const children: ChildProcessWithoutNullStreams[] = []; + + beforeEach(async () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg1740-mcp-')); + realRoot = fs.realpathSync(tempDir); + fs.mkdirSync(path.join(realRoot, 'src')); + fs.writeFileSync(path.join(realRoot, 'src/a.ts'), 'export function a() { return 1; }\n'); + const cg = await CodeGraph.init(realRoot); + await cg.indexAll(); + cg.close(); + }); + + afterEach(async () => { + for (const c of children) { + try { c.kill('SIGTERM'); } catch { /* ignore */ } + } + children.length = 0; + await sleep(300); + try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('second CODEGRAPH_NO_DAEMON serve --mcp exits with writer-lock error', async () => { + const env = { + CODEGRAPH_NO_DAEMON: '1', + CODEGRAPH_MCP_DEBUG: '1', + CODEGRAPH_NO_WATCHDOG: '1', + CODEGRAPH_STARTUP_HANDSHAKE_TIMEOUT_MS: '0', + // Avoid wasm --liftoff-only re-exec so lock.pid matches the spawned pid. + CODEGRAPH_NO_RELAUNCH: '1', + CODEGRAPH_WASM_RELAUNCHED: '1', + }; + const first = spawnMcp(realRoot, env); + children.push(first.child); + + const lockPath = getWriterPidPath(realRoot); + const deadline = Date.now() + 10000; + while (Date.now() < deadline && !fs.existsSync(lockPath)) { + await sleep(50); + } + expect(fs.existsSync(lockPath)).toBe(true); + expect(first.child.exitCode).toBeNull(); + + const second = spawnMcp(realRoot, env); + children.push(second.child); + + const code = await new Promise((resolve) => { + const timer = setTimeout(() => resolve(second.child.exitCode), 10000); + second.child.on('close', (c) => { + clearTimeout(timer); + resolve(c); + }); + }); + + expect(code).toBe(1); + expect(second.getStderr()).toMatch(/writer lock held/i); + expect(second.getStderr()).toMatch(/CODEGRAPH_NO_DAEMON/); + expect(first.child.exitCode).toBeNull(); + const lock = JSON.parse(fs.readFileSync(lockPath, 'utf8')) as { pid: number }; + expect(lock.pid).toBe(first.child.pid); + }, 20000); + + it('default daemon mode still allows two proxies to share one writer', async () => { + const env = { + CODEGRAPH_MCP_LOG_ATTACH: '1', + CODEGRAPH_NO_WATCHDOG: '1', + CODEGRAPH_STARTUP_HANDSHAKE_TIMEOUT_MS: '0', + CODEGRAPH_NO_RELAUNCH: '1', + CODEGRAPH_WASM_RELAUNCHED: '1', + }; + const a = spawnMcp(realRoot, env); + const b = spawnMcp(realRoot, env); + children.push(a.child, b.child); + + const lockPath = getWriterPidPath(realRoot); + const deadline = Date.now() + 15000; + while (Date.now() < deadline && !fs.existsSync(lockPath)) { + await sleep(50); + } + expect(fs.existsSync(lockPath)).toBe(true); + await sleep(1000); + expect(a.child.exitCode).toBeNull(); + expect(b.child.exitCode).toBeNull(); + + const lock = JSON.parse(fs.readFileSync(lockPath, 'utf8')) as { pid: number; mode: string }; + expect(lock.mode).toBe('daemon'); + expect(lock.pid).not.toBe(a.child.pid); + expect(lock.pid).not.toBe(b.child.pid); + }, 25000); +}); diff --git a/__tests__/writer-lock.test.ts b/__tests__/writer-lock.test.ts new file mode 100644 index 000000000..c3ba44723 --- /dev/null +++ b/__tests__/writer-lock.test.ts @@ -0,0 +1,87 @@ +/** + * Project writer lock (#1740) — unit coverage for acquire / re-entrant / + * stale-dead-pid / live-holder refusal. + */ + +import { afterEach, describe, expect, it } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { + decodeWriterLockInfo, + getWriterPidPath, + releaseWriterLock, + tryAcquireWriterLock, + writerLockHeldMessage, +} from '../src/mcp/writer-lock'; + +describe('writer lock (#1740)', () => { + let dir: string; + + afterEach(() => { + if (dir) { + releaseWriterLock(dir); + try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + } + }); + + function makeProject(): string { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg1740-lock-')); + fs.mkdirSync(path.join(dir, '.codegraph'), { recursive: true }); + return dir; + } + + it('acquires and releases writer.pid', () => { + const root = makeProject(); + const r = tryAcquireWriterLock(root, 'direct'); + expect(r.kind).toBe('acquired'); + expect(fs.existsSync(getWriterPidPath(root))).toBe(true); + const info = decodeWriterLockInfo(fs.readFileSync(getWriterPidPath(root), 'utf8')); + expect(info?.pid).toBe(process.pid); + expect(info?.mode).toBe('direct'); + releaseWriterLock(root); + expect(fs.existsSync(getWriterPidPath(root))).toBe(false); + }); + + it('is re-entrant for the same pid', () => { + const root = makeProject(); + expect(tryAcquireWriterLock(root, 'daemon').kind).toBe('acquired'); + const again = tryAcquireWriterLock(root, 'fallback'); + expect(again.kind).toBe('acquired'); + releaseWriterLock(root); + }); + + it('reports taken when a live foreign pid holds the lock', () => { + const root = makeProject(); + // Use our own pid first, then overwrite with a fake live-looking pid by + // writing a pid that is alive: process.pid of this test — simulate foreign + // by writing a different alive pid. On Linux, PID 1 is almost always alive. + fs.writeFileSync( + getWriterPidPath(root), + JSON.stringify({ pid: 1, mode: 'direct', startedAt: Date.now() }) + '\n', + { flag: 'wx' }, + ); + const r = tryAcquireWriterLock(root, 'direct'); + expect(r.kind).toBe('taken'); + if (r.kind === 'taken') { + expect(r.existing?.pid).toBe(1); + const msg = writerLockHeldMessage(r.existing, r.pidPath); + expect(msg).toMatch(/writer lock held/i); + expect(msg).toMatch(/CODEGRAPH_NO_DAEMON/); + expect(msg).toMatch(/daemon stop/); + } + }); + + it('clears a stale dead-pid lock and acquires', () => { + const root = makeProject(); + // Pick a pid that is extremely unlikely to be alive. + const deadPid = 2147483646; + fs.writeFileSync( + getWriterPidPath(root), + JSON.stringify({ pid: deadPid, mode: 'direct', startedAt: Date.now() }) + '\n', + ); + const r = tryAcquireWriterLock(root, 'direct'); + expect(r.kind).toBe('acquired'); + releaseWriterLock(root); + }); +}); diff --git a/src/mcp/daemon.ts b/src/mcp/daemon.ts index 500c48a8c..a5735d168 100644 --- a/src/mcp/daemon.ts +++ b/src/mcp/daemon.ts @@ -55,6 +55,7 @@ import { getDaemonSocketPath, } from './daemon-paths'; import { CodeGraphPackageVersion } from './version'; +import { releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage } from './writer-lock'; import { registerDaemon, deregisterDaemon } from './daemon-registry'; /** Default idle linger after the last client disconnects. */ @@ -204,6 +205,16 @@ export class Daemon { * listening — the daemon then sticks around until idle/shutdown. */ async start(): Promise { + // #1740: claim the project writer lock before opening/watching so a + // concurrent direct-mode serve --mcp cannot start a second watcher. + const writer = tryAcquireWriterLock(this.projectRoot, 'daemon'); + if (writer.kind === 'taken') { + const msg = writerLockHeldMessage(writer.existing, writer.pidPath); + process.stderr.write(`[CodeGraph daemon] ${msg}\n`); + this.cleanupLockfile(); + throw new Error(msg); + } + // Engine init is deliberately backgrounded — see #172. The first session // to land waits on `ensureInitialized` either way, and unloaded sessions // (cross-project tool calls only) shouldn't pay any open cost. @@ -498,6 +509,7 @@ export class Daemon { } private cleanupLockfile(): void { + releaseWriterLock(this.projectRoot); try { if (fs.existsSync(this.pidPath)) { // Only remove if it still belongs to us — another daemon may have diff --git a/src/mcp/engine.ts b/src/mcp/engine.ts index 8f2e5b6ef..734fafa4f 100644 --- a/src/mcp/engine.ts +++ b/src/mcp/engine.ts @@ -16,6 +16,7 @@ import type CodeGraph from '../index'; import { resolveServerRoot } from '../directory'; import { watchDisabledReason } from '../sync'; import { ToolHandler } from './tools'; +import { releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage } from './writer-lock'; import { QueryPool, resolvePoolSize } from './query-pool'; // Lazy-load the heavy CodeGraph chain (sqlite + query/graph/context layers) OFF @@ -67,6 +68,8 @@ export class MCPEngine { // bounded but shouldn't run on every tool call in the no-default state. private lastRetrySubScanAt = 0; private watcherStarted = false; + /** Set when this engine holds writer.pid (#1740). */ + private writerLockRoot: string | null = null; private opts: Required; private closed = false; // Off-loop read-tool pool (daemon mode only). Created lazily once the default @@ -203,6 +206,10 @@ export class MCPEngine { stop(): void { if (this.closed) return; this.closed = true; + if (this.writerLockRoot) { + releaseWriterLock(this.writerLockRoot); + this.writerLockRoot = null; + } // Detach + terminate the worker pool first so no tool call routes to a // worker mid-teardown; outstanding pool calls resolve with graceful guidance. this.toolHandler.setQueryPool(null); @@ -278,6 +285,24 @@ export class MCPEngine { private startWatching(): void { if (!this.cg || this.watcherStarted || !this.opts.watch) return; + // #1740: only one live watcher/writer per project. Daemon and startDirect + // usually already hold writer.pid (re-entrant for this pid). Proxy + // in-process fallback acquires here; if another writer holds it, skip the + // watcher so we never contend on codegraph.lock until auto-sync degrades. + const lockRoot = this.projectPath; + if (lockRoot) { + const writer = tryAcquireWriterLock(lockRoot, 'fallback'); + if (writer.kind === 'taken') { + const msg = writerLockHeldMessage(writer.existing, writer.pidPath); + process.stderr.write( + `[CodeGraph MCP] File watcher not started — ${msg}\n` + ); + this.watcherStarted = true; + return; + } + this.writerLockRoot = lockRoot; + } + const disabledReason = watchDisabledReason(this.projectPath ?? process.cwd()); if (disabledReason) { process.stderr.write( diff --git a/src/mcp/index.ts b/src/mcp/index.ts index 3f57024d2..e4a14a4fb 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -48,6 +48,7 @@ import { tryAcquireDaemonLock, } from './daemon'; import { connectWithHello, runLocalHandshakeProxy } from './proxy'; +import { releaseWriterLock, tryAcquireWriterLock, writerLockHeldMessage } from './writer-lock'; import { getDaemonSocketCandidates, probeDaemonIdentity } from './daemon-paths'; import { getTelemetry } from '../telemetry'; import { checkForUpdateInBackground } from '../upgrade/update-check'; @@ -252,6 +253,8 @@ export class MCPServer { // Idempotency guard for stop(). private stopped = false; private mode: 'unstarted' | 'direct' | 'proxy' | 'daemon' = 'unstarted'; + /** Project root whose writer.pid we hold in direct mode (#1740); released on stop. */ + private writerLockRoot: string | null = null; constructor(projectPath?: string) { this.projectPath = projectPath || null; @@ -329,6 +332,10 @@ export class MCPServer { stop(): void { if (this.stopped) return; this.stopped = true; + if (this.writerLockRoot) { + releaseWriterLock(this.writerLockRoot); + this.writerLockRoot = null; + } if (this.ppidWatchdog) { clearInterval(this.ppidWatchdog); this.ppidWatchdog = null; @@ -358,6 +365,20 @@ export class MCPServer { if (reason && process.env.CODEGRAPH_MCP_DEBUG) { process.stderr.write(`[CodeGraph MCP] Direct mode: ${reason}.\n`); } + + // #1740: refuse a second direct writer on an initialized project. Daemon + // mode multiplexes clients; direct mode is single-writer-per-project. + const writerRoot = resolveDaemonRoot(this.projectPath); + if (writerRoot) { + const writer = tryAcquireWriterLock(writerRoot, 'direct'); + if (writer.kind === 'taken') { + const msg = writerLockHeldMessage(writer.existing, writer.pidPath); + process.stderr.write(`[CodeGraph MCP] ${msg}\n`); + process.exit(1); + } + this.writerLockRoot = writerRoot; + } + this.engine = new MCPEngine(); const transport = new StdioTransport(); this.session = new MCPSession(transport, this.engine, { diff --git a/src/mcp/writer-lock.ts b/src/mcp/writer-lock.ts new file mode 100644 index 000000000..919e630c5 --- /dev/null +++ b/src/mcp/writer-lock.ts @@ -0,0 +1,188 @@ +/** + * Project writer lock (#1740). + * + * At most one long-lived MCP *writer* (shared daemon OR direct-mode / + * in-process engine that owns the FileWatcher) may serve a given project. + * The shared daemon already multiplexes N stdio proxies onto one writer; this + * lock closes the same-OS gap where two direct-mode `serve --mcp` processes + * (via `CODEGRAPH_NO_DAEMON=1` or proxy→in-process fallback) each start a + * watcher, contend on `codegraph.lock`, and degrade auto-sync. + * + * Deliberately separate from `daemon.pid`: proxies probe the daemon socket + * and may clear a live pid that has no socket. A direct-mode holder must not + * look like a daemon. `writer.pid` is only about "who owns live auto-sync". + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import { getCodeGraphDir } from '../directory'; +/** Signal-0 liveness (EPERM ⇒ alive). Local copy to avoid a daemon↔writer cycle. */ +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (err: unknown) { + const e = err as NodeJS.ErrnoException; + if (e.code === 'EPERM') return true; + return false; + } +} + + +/** Absolute path to the writer pid lockfile for `projectRoot`. */ +export function getWriterPidPath(projectRoot: string): string { + let root = projectRoot; + try { root = fs.realpathSync(projectRoot); } catch { /* keep lexical */ } + return path.join(getCodeGraphDir(root), 'writer.pid'); +} + +/** Structured contents of the writer pidfile. */ +export interface WriterLockInfo { + pid: number; + /** `direct` | `daemon` | `fallback` — for actionable error text only. */ + mode: string; + startedAt: number; +} + +export type WriterAcquireResult = + | { kind: 'acquired'; pidPath: string; info: WriterLockInfo } + | { kind: 'taken'; existing: WriterLockInfo | null; pidPath: string }; + +function encode(info: WriterLockInfo): string { + return JSON.stringify(info) + '\n'; +} + +export function decodeWriterLockInfo(raw: string): WriterLockInfo | null { + try { + const parsed = JSON.parse(raw.trim()) as Partial; + if (typeof parsed.pid !== 'number' || typeof parsed.mode !== 'string') return null; + return { + pid: parsed.pid, + mode: parsed.mode, + startedAt: typeof parsed.startedAt === 'number' ? parsed.startedAt : 0, + }; + } catch { + return null; + } +} + +/** + * Atomically create `writer.pid` (link-into-place, O_EXCL fallback). If held + * by a dead PID, clear and retry once. Does not steal from a live holder. + */ +export function tryAcquireWriterLock( + projectRoot: string, + mode: string, +): WriterAcquireResult { + const pidPath = getWriterPidPath(projectRoot); + fs.mkdirSync(path.dirname(pidPath), { recursive: true }); + + const info: WriterLockInfo = { + pid: process.pid, + mode, + startedAt: Date.now(), + }; + + const attempt = (): WriterAcquireResult => { + const tmp = `${pidPath}.${process.pid}.tmp`; + let acquired = false; + try { + fs.writeFileSync(tmp, encode(info), { mode: 0o600 }); + try { + fs.linkSync(tmp, pidPath); + acquired = true; + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'EEXIST') { + // taken + } else { + // No hard links — O_EXCL create. + try { + const fd = fs.openSync(pidPath, 'wx', 0o600); + try { + fs.writeSync(fd, encode(info)); + acquired = true; + } finally { + fs.closeSync(fd); + } + } catch (e2: unknown) { + if ((e2 as NodeJS.ErrnoException).code !== 'EEXIST') throw e2; + } + } + } + } finally { + try { fs.unlinkSync(tmp); } catch { /* ignore */ } + } + + if (acquired) return { kind: 'acquired', pidPath, info }; + + let existing: WriterLockInfo | null = null; + try { + existing = decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); + } catch { /* unreadable */ } + return { kind: 'taken', existing, pidPath }; + }; + + let result = attempt(); + if (result.kind === 'taken' && result.existing && result.existing.pid === process.pid) { + // Same process already holds it (daemon acquired before engine watch). + return { kind: 'acquired', pidPath: result.pidPath, info: result.existing }; + } + if (result.kind === 'taken') { + const existing = result.existing; + if (!existing || existing.pid <= 0 || !isProcessAlive(existing.pid)) { + // Stale — clear (pid-verified) and retry once. + try { + const raw = fs.readFileSync(pidPath, 'utf8'); + const cur = decodeWriterLockInfo(raw); + if (!cur || cur.pid === existing?.pid) { + if (!cur || cur.pid <= 0 || !isProcessAlive(cur.pid)) { + fs.unlinkSync(pidPath); + } + } + } catch { /* ENOENT ok */ } + result = attempt(); + } + } + return result; +} + +/** Release if we still own the lock (pid match). */ +export function releaseWriterLock(projectRoot: string): void { + const pidPath = getWriterPidPath(projectRoot); + try { + if (!fs.existsSync(pidPath)) return; + const info = decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); + if (info && info.pid === process.pid) { + fs.unlinkSync(pidPath); + } + } catch { /* best-effort */ } +} + +/** Read current lock without acquiring. */ +export function readWriterLock(projectRoot: string): WriterLockInfo | null { + const pidPath = getWriterPidPath(projectRoot); + try { + return decodeWriterLockInfo(fs.readFileSync(pidPath, 'utf8')); + } catch { + return null; + } +} + +/** + * Actionable message when another live process owns the writer lock (#1740). + */ +export function writerLockHeldMessage( + existing: WriterLockInfo | null, + pidPath: string, +): string { + const who = existing && existing.pid > 0 + ? `PID ${existing.pid} (${existing.mode || 'unknown'} mode)` + : 'another process'; + return ( + 'CodeGraph writer lock held by ' + who + '. ' + + 'Only one live MCP writer may serve a project (auto-sync / index). ' + + 'Stop the other server (codegraph daemon stop if a shared daemon, or end the other MCP session), ' + + 'or unset CODEGRAPH_NO_DAEMON so additional clients proxy to the shared daemon. ' + + 'If this is stale, delete ' + pidPath + ); +} From 2c251e2c61d5e3a8789e3815dafd04cf13b765ed Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Tue, 8 Sep 2026 00:00:39 -0500 Subject: [PATCH 04/14] fix(resolution): a definition its language makes file-local is not a cross-file target (#1731) (#1745) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(resolution): a definition its language makes file-local is not a cross-file target Name matching accepted any same-named definition as the target of a call from another file, however the language scopes it. isVisibleAcrossFiles now declines, for a candidate in another file: C / C++ a function whose definition line carries `static` (read from source — the extractor records no storage class, and the kernel arm would need the same field) Kotlin, Java, C#, Swift, Scala, Dart, PHP visibility === 'private' Go a lowercase identifier from another directory (by the name's case: the extractor's isExported is unset for every Go method) Rust a non-`pub` item unless the reference is in the item's module subtree (a child sees its ancestors' private items via super::); a method in an `impl Trait for Type` block has the trait's visibility and is exempt The test runs in ReferenceResolver on the target the whole name-matching pipeline settled on, so a rejection ends the reference unresolved. Declining inside matchByExactName instead let the ref fall through to matchFuzzy, which committed to a same-language namesake the ranking had passed over — eight edges on one tree, all onto a local `const fail = …` arrow the graph does not hold. matchFuzzy checks its own survivor too; nothing runs after it. Five corpora, all against b9ca4b7, wasm arm, edge rows keyed with resolvedBy: betaflight fork (2,109 C files) LOST 4,451 GAINED 0 (#1730) Android/Go/JS app (114 kt, 42 go) LOST 142 GAINED 0 (#1731) emmc-reader-gui (71 rs) LOST 195 GAINED 0 skylab_hub (35 rs) LOST 92 GAINED 0 vitejs/vite (JS/TS only) LOST 0 GAINED 0 Samples read back: `Vec::new()` onto a private `fn new` in another crate, `ui.add(…)` (egui) onto a private `add`, `latch.await()` onto a test file's `private fun await`, `leaflet.js` onto an unexported Go `func add`, `usbd_get_descriptor` onto a `static get_device_descriptor` in a USB class file it never links. Fixes #1730. Fixes #1731. Co-Authored-By: Claude Fable 5.1 * fix(resolution): a static in a header is part of every unit that includes it The C rule declined any `static` function defined in another file. A `static` in a SOURCE file is local to that translation unit and the rule is right there; a `static` in a header — `static inline`, the whole of MAVLink's generated `mavlink_msg_*.h` — is textually included into every unit that names it, and the call is real. On the betaflight tree 4,306 of the 4,451 rows the first cut removed were exactly that: `testsuite.h` and `mavlink_msg_*.h` calling `protocol.h`'s `_mav_put_char_array`, `mav_array_assign_char` and each other's `_pack` / `_decode` helpers. The rule now applies only to a candidate whose file is a translation unit (`.c .cc .cpp .cxx .c++ .m .mm`). Same tree: LOST 145, GAINED 0, every one onto a `static` in another `.c` — STM32 USB class sources onto GD32's `usbd_enum.c`, and the USB descriptor table shape from #1730. Header targets in the removed set: 0. Co-Authored-By: Claude Fable 5.1 * docs(changelog): cite #1731 and narrow the C file-local note Rebased #1732 onto latest main. Clarify that only a static in another source file is declined (header static inline stays), name the Kotlin/Go/Rust shapes from #1731, and note the post-pipeline placement that avoids fuzzy fallback. --------- Co-authored-by: danusha2345 Co-authored-by: Claude Fable 5.1 Co-authored-by: Colby McHenry --- CHANGELOG.md | 2 + __tests__/cross-file-visibility.test.ts | 150 ++++++++++++++++++++++++ src/resolution/index.ts | 10 +- src/resolution/name-matcher.ts | 148 ++++++++++++++++++++++- 4 files changed, 307 insertions(+), 3 deletions(-) create mode 100644 __tests__/cross-file-visibility.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 146c20bcf..272f42b52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -207,6 +207,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### Symbols, tests and the viewer +- **A definition its language makes file-local no longer captures calls from other files.** A C `static` in another source file (`.c`/`.cc`/… — not a header's `static inline`, which is textually included), a Kotlin/Java/C#/Swift/Scala/Dart/PHP `private` member, a Go unexported name in another package, and a Rust non-`pub` item outside its module subtree cannot be what a name in another file means, but name matching accepted them whenever the names agreed: an Android `editor.apply()` onto an unrelated class's `private fun apply`, a JavaScript `fail(...)` onto a Go `func fail`, a Rust `.count()` onto a private `fn count` in another crate, and C USB helpers onto a `static` in a `.c` they never link. Such a target is now declined after the whole name-matching pipeline settles — the reference stays unresolved rather than falling through to a fuzzy namesake. Same-file definitions, a child Rust module reaching its ancestors' private items, and Rust `impl Trait for Type` methods stay resolvable. Re-index after upgrading. (#1730, #1731) + - **Files under an `e2e/` directory count as tests.** Their calls no longer appear as production callers in Steps, dead-code and test badges. - **Production code under a `samples` or `examples` package path is no longer treated as test code.** A Kotlin or Java project whose package path runs through `com/google/samples/…` (Now in Android, for one) had nearly every file counted as a fixture, so the Map opened on `build-logic`, the entry points hid the app, and dead-code and test badges were wrong. Only the project layout above a `src/` folder decides now; the package path below it never does. diff --git a/__tests__/cross-file-visibility.test.ts b/__tests__/cross-file-visibility.test.ts new file mode 100644 index 000000000..1cb0007c9 --- /dev/null +++ b/__tests__/cross-file-visibility.test.ts @@ -0,0 +1,150 @@ +/** + * A definition the language makes file-local is not a candidate for a + * cross-file name match: a C `static`, a Kotlin `private fun`, a Go unexported + * identifier in another package, a Rust non-`pub` item outside its module + * subtree. Each case pairs the invisible shape with the visible one of + * identical form, so the assertion discriminates on visibility alone. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; + +let tempDir: string; +let cg: CodeGraph | null = null; + +function project(files: Record): void { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-visibility-')); + for (const [rel, content] of Object.entries(files)) { + const abs = path.join(tempDir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } +} + +/** `calls` targets of the function named `caller`, as `file:name` strings. */ +async function calleesOf(caller: string): Promise { + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const from = cg.getNodesByKind('function').concat(cg.getNodesByKind('method')).find((n) => n.name === caller)!; + expect(from).toBeDefined(); + return cg + .getOutgoingEdges(from.id) + .filter((e) => e.kind === 'calls') + .map((e) => cg!.getNode(e.target)) + .filter((n): n is NonNullable => !!n) + .map((n) => `${n.filePath}:${n.name}`); +} + +afterEach(() => { + cg?.close(); + cg = null; + fs.rmSync(tempDir, { recursive: true, force: true }); +}); + +describe('C: a static function is local to its translation unit', () => { + it('does not resolve a call onto a static in another file', async () => { + project({ + 'core.c': 'void coreRun(void)\n{\n usbGetDescriptor();\n}\n', + 'usb_audio.c': 'static void usbGetDescriptor(void)\n{\n}\n', + }); + expect(await calleesOf('coreRun')).not.toContain('usb_audio.c:usbGetDescriptor'); + }); + + it('still resolves onto a non-static function in another file', async () => { + project({ + 'core.c': 'void coreRun(void)\n{\n usbGetDescriptor();\n}\n', + 'usb_audio.c': 'void usbGetDescriptor(void)\n{\n}\n', + }); + expect(await calleesOf('coreRun')).toContain('usb_audio.c:usbGetDescriptor'); + }); + + it('keeps a static inline defined in a header: it lives in every unit that includes it', async () => { + project({ + 'protocol.h': 'static inline void mav_put_char(char *buf, char c)\n{\n buf[0] = c;\n}\n', + 'core.c': '#include "protocol.h"\n\nvoid coreRun(char *b)\n{\n mav_put_char(b, 0);\n}\n', + }); + expect(await calleesOf('coreRun')).toContain('protocol.h:mav_put_char'); + }); + + it('keeps a same-file static, whichever line the keyword is on', async () => { + project({ + 'core.c': 'static void\nhelper(void)\n{\n}\n\nvoid coreRun(void)\n{\n helper();\n}\n', + 'other.c': 'static void helper(void)\n{\n}\n', + }); + expect(await calleesOf('coreRun')).toEqual(['core.c:helper']); + }); +}); + +describe('Kotlin: a private function is class- or file-local', () => { + it('does not resolve an SDK-style call onto another file\'s private fun', async () => { + project({ + 'Budget.kt': 'class Budget {\n private fun apply(bps: Long): Long = bps\n}\n', + 'Main.kt': 'class Main {\n fun onCreate(editor: Editor) {\n editor.apply()\n }\n}\n', + }); + expect(await calleesOf('onCreate')).not.toContain('Budget.kt:apply'); + }); + + it('still resolves onto a public fun in another file', async () => { + project({ + 'Budget.kt': 'class Budget {\n fun apply(bps: Long): Long = bps\n}\n', + 'Main.kt': 'class Main {\n fun onCreate(budget: Budget) {\n budget.apply(1L)\n }\n}\n', + }); + expect(await calleesOf('onCreate')).toContain('Budget.kt:apply'); + }); +}); + +describe('Go: an unexported identifier is package-local', () => { + it('does not resolve a call onto an unexported func in another package', async () => { + project({ + 'cmd/probe/main.go': 'package main\n\nfunc fail(msg string) {}\n', + 'server/turn.go': 'package server\n\nfunc Run() {\n\tfail("x")\n}\n', + }); + expect(await calleesOf('Run')).not.toContain('cmd/probe/main.go:fail'); + }); + + it('still resolves within the package and onto an exported func elsewhere', async () => { + project({ + 'server/util.go': 'package server\n\nfunc fail(msg string) {}\n', + 'server/turn.go': 'package server\n\nfunc Run() {\n\tfail("x")\n\tReport()\n}\n', + 'report/report.go': 'package report\n\nfunc Report() {}\n', + }); + const callees = await calleesOf('Run'); + expect(callees).toContain('server/util.go:fail'); + expect(callees).toContain('report/report.go:Report'); + }); +}); + +describe('Rust: a non-pub item is visible to its module subtree only', () => { + it('does not resolve a sibling module\'s private fn, nor another crate\'s', async () => { + project({ + 'src/main.rs': 'mod util;\nmod net;\nfn main() {}\n', + 'src/util.rs': 'fn count() -> usize { 0 }\n', + 'src/net.rs': 'pub fn run() -> usize {\n count()\n}\n', + }); + expect(await calleesOf('run')).not.toContain('src/util.rs:count'); + }); + + it('keeps a trait-impl method, which has the trait\'s visibility', async () => { + project({ + 'src/main.rs': 'mod shape;\nmod draw;\nfn main() {}\n', + 'src/shape.rs': 'pub struct Circle;\npub trait Area { fn area(&self) -> f64; }\nimpl Area for Circle {\n fn area(&self) -> f64 { 1.0 }\n}\n', + 'src/draw.rs': 'use crate::shape::{Area, Circle};\npub fn render(c: &Circle) -> f64 {\n c.area()\n}\n', + }); + expect(await calleesOf('render')).toContain('src/shape.rs:area'); + }); + + it('still resolves a parent module\'s private fn from a child, and any pub fn', async () => { + project({ + 'src/main.rs': 'mod net;\nmod util;\nfn main() {}\n', + 'src/net.rs': 'pub mod tcp;\nfn shared() {}\n', + 'src/net/tcp.rs': 'use super::shared;\nuse crate::util::exported;\npub fn open() {\n shared();\n exported();\n}\n', + 'src/util.rs': 'pub fn exported() {}\n', + }); + const callees = await calleesOf('open'); + expect(callees).toContain('src/net.rs:shared'); + expect(callees).toContain('src/util.rs:exported'); + }); +}); diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 7988c1bcf..2a995b985 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -16,7 +16,7 @@ import { FrameworkResolver, ImportMapping, } from './types'; -import { matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesKnownFamily, dumpNameMatcherProfile, clearNameMatcherMemos } from './name-matcher'; +import { isVisibleAcrossFiles, matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesKnownFamily, dumpNameMatcherProfile, clearNameMatcherMemos } from './name-matcher'; import { resolveViaImport, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, clearImportResolverMemos } from './import-resolver'; import { ResolverPool, minRefsForPool } from './resolver-pool'; import { detectFrameworks } from './frameworks'; @@ -1012,7 +1012,13 @@ export class ReferenceResolver { // binding it happened to pick. Same-file matches only. if (nameResult) { const target = this.queries.getNodeById(nameResult.targetNodeId); - if (ref.language === 'nix') { + // A definition its language makes file-local — a C `static`, a Kotlin + // `private fun`, a Go unexported name in another package, a Rust + // non-`pub` item outside its module subtree — cannot be what a name in + // another file means, whichever strategy chose it (#1730). + if (target && !isVisibleAcrossFiles(target, ref, this.context)) { + nameResult = null; + } else if (ref.language === 'nix') { if (!target || target.filePath !== ref.filePath) { nameResult = null; } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index c74d8f272..6a1863bff 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -4,6 +4,7 @@ * Handles symbol name matching for reference resolution. */ +import * as path from 'path'; import { Language, Node } from '../types'; import { UnresolvedRef, ResolvedRef, ResolutionContext } from './types'; @@ -388,6 +389,149 @@ function isLexicallyReachable( ); } +/** + * Languages in which `visibility: 'private'` on a definition means no other + * FILE can name it: a Kotlin `private fun` is file- or class-local, and the + * same holds for Java, C#, Swift, Scala, Dart and PHP members. + */ +const PRIVATE_IS_FILE_LOCAL = new Set(['kotlin', 'java', 'csharp', 'swift', 'scala', 'dart', 'php']); + +/** Per-context memo: node id → "this C/C++ function is declared `static`". */ +const C_STATIC_MEMO = new WeakMap>(); + +/** + * A C/C++ file that IS a translation unit. A `static` defined here is local + * to it. A `static` (typically `static inline`) in a header is a different + * thing: the header is textually included, so the function exists in every + * unit that includes it and is callable from each — MAVLink's generated + * `mavlink_msg_*.h` are nothing but such functions, 4,306 real calls on one + * betaflight tree. + */ +const C_SOURCE_EXT = /\.(c|cc|cpp|cxx|c\+\+|m|mm)$/i; + +/** + * Whether a C/C++ function definition carries the `static` storage class — + * read from its first source line(s), since the extractor records no storage + * class and the kernel arm would need the same field. `static` on the line + * above the name (`static void\nfoo(void)`) is the common alternative layout. + */ +function isStaticCFunction(candidate: Node, context: ResolutionContext): boolean { + let memo = C_STATIC_MEMO.get(context); + if (!memo) { + memo = new Map(); + C_STATIC_MEMO.set(context, memo); + } + const hit = memo.get(candidate.id); + if (hit !== undefined) return hit; + const lines = context.getFileLines?.(candidate.filePath) ?? context.readFile(candidate.filePath)?.split('\n') ?? []; + const head = [lines[candidate.startLine - 2] ?? '', lines[candidate.startLine - 1] ?? ''].join('\n'); + const isStatic = /(^|[\s;}])static\s/.test(head); + memo.set(candidate.id, isStatic); + return isStatic; +} + +/** Per-context memo: node id → "this Rust method implements a trait". */ +const RUST_TRAIT_IMPL_MEMO = new WeakMap>(); + +/** + * Whether a Rust method sits in an `impl Trait for Type` block. Such a method + * carries no `pub` — the trait decides its visibility — so the extractor + * records it as private; it is reachable wherever the trait is. Read from the + * nearest enclosing `impl` header above the method, memoised per node. + */ +function isRustTraitImplMethod(candidate: Node, context: ResolutionContext): boolean { + if (candidate.kind !== 'method') return false; + let memo = RUST_TRAIT_IMPL_MEMO.get(context); + if (!memo) { + memo = new Map(); + RUST_TRAIT_IMPL_MEMO.set(context, memo); + } + const hit = memo.get(candidate.id); + if (hit !== undefined) return hit; + const lines = context.getFileLines?.(candidate.filePath) ?? context.readFile(candidate.filePath)?.split('\n') ?? []; + let isTrait = false; + for (let i = candidate.startLine - 2; i >= 0; i--) { + const line = lines[i] ?? ''; + if (/^\s*(pub(\([^)]*\))?\s+)?(unsafe\s+)?impl\b/.test(line)) { + isTrait = /\sfor\s/.test(line.replace(/\/\/.*$/, '')); + break; + } + // A top-level item above the method means it was not inside an impl. + if (/^(pub(\([^)]*\))?\s+)?(fn|struct|enum|mod|trait|const|static|type)\b/.test(line)) break; + } + memo.set(candidate.id, isTrait); + return isTrait; +} + +/** + * The directory a Rust file's private items are visible from: the file's own + * module subtree. `src/net.rs` and `src/net/mod.rs` own `src/net/`; a crate + * root (`lib.rs` / `main.rs`) owns its directory. A child module reaches its + * ancestors' private items (`super::`), a sibling or another crate never does. + */ +function rustModuleDir(filePath: string): string { + const base = path.posix.basename(filePath); + const dir = path.posix.dirname(filePath); + if (base === 'mod.rs' || base === 'lib.rs' || base === 'main.rs') return dir; + return path.posix.join(dir, base.replace(/\.rs$/, '')); +} + +/** + * Whether `candidate` can be NAMED from a reference in `ref`'s file at all, + * given what its language says about the definition's visibility. A + * definition the language makes file-local is not a candidate for a + * cross-file name match, however well the names agree: + * + * - **C / C++**: a `static` function defined in a SOURCE file is local to + * that translation unit; one in a header is part of every unit that + * includes it and stays visible. On a 2,109-file betaflight tree 145 + * cross-file calls resolved onto a `static` in another `.c` (#1730) — + * `usbd_get_descriptor` onto the `static get_device_descriptor` of + * whichever USB class file ranked first. + * - **Kotlin, Java, C#, Swift, Scala, Dart, PHP**: `private` is class- or + * file-local. An Android `editor.apply()` resolved onto an unrelated class's + * `private fun apply`. + * - **Go**: an unexported (lowercase) identifier is package-local, and a + * package is a directory. Judged by the name's case: the extractor's + * `isExported` is unset for every Go method. + * - **Rust**: a non-`pub` item is visible to its module and that module's + * descendants, never to a sibling module or another crate — `.count()` on + * an iterator resolved onto a `fn count` in a different crate. A method in + * an `impl Trait for Type` block has the trait's visibility, not `private`. + * + * Same-file candidates are always visible. Applied by ReferenceResolver to + * the target the whole name-matching pipeline settled on, so a rejection ends + * the reference unresolved: declining inside matchByExactName instead let the + * ref fall through to matchFuzzy, which then committed to a same-language + * namesake the ranking had passed over — eight such edges on one tree, all + * onto a local `const fail = …` arrow the graph does not hold. matchFuzzy + * checks its own survivor as well, since nothing runs after it. + */ +export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, context: ResolutionContext): boolean { + if (candidate.filePath === ref.filePath) return true; + const lang = candidate.language as string; + if (lang === 'c' || lang === 'cpp') { + return ( + candidate.kind !== 'function' || + !C_SOURCE_EXT.test(candidate.filePath) || + !isStaticCFunction(candidate, context) + ); + } + if (lang === 'go') { + // By the name's first letter, not the extractor's flag: the flag is unset + // for every Go method, exported or not. + return /^[A-Z]/.test(candidate.name) || path.posix.dirname(candidate.filePath) === path.posix.dirname(ref.filePath); + } + if (lang === 'rust') { + if (candidate.visibility !== 'private') return true; + if (isRustTraitImplMethod(candidate, context)) return true; + const owner = rustModuleDir(candidate.filePath); + return ref.filePath.startsWith(owner + '/'); + } + if (PRIVATE_IS_FILE_LOCAL.has(lang)) return candidate.visibility !== 'private'; + return true; +} + /** * Try to resolve a reference by exact name match */ @@ -1299,6 +1443,8 @@ function getInferScanStates(context: ResolutionContext): Map RegExp[]): RegExp[] { @@ -2418,7 +2564,7 @@ export function matchFuzzy( const sameLanguageCandidates = callableCandidates.filter(n => n.language === ref.language); const finalCandidates = sameLanguageCandidates.length > 0 ? sameLanguageCandidates : callableCandidates; - if (finalCandidates.length === 1) { + if (finalCandidates.length === 1 && isVisibleAcrossFiles(finalCandidates[0]!, ref, context)) { const isCrossLanguage = finalCandidates[0]!.language !== ref.language; return { original: ref, From bffd50e4f1c67321c75e7590f9e2df981c45f018 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Tue, 8 Sep 2026 00:13:49 -0500 Subject: [PATCH 05/14] fix(resolution): a binding in a module that exports nothing is not a cross-file candidate (#1719) (#1746) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(resolution): a binding in a module that exports nothing is not a cross-file candidate On vitejs/vite, 157 cross-file `imports` refs — every `import { defineConfig } from 'vite'` in the playground and the create-vite templates — resolved onto `playground/ssr-html/test-stacktrace.js::vite`, which is `const vite = await createServer(...)` at module scope in a file with zero exports. Neither existing guard can see it. `isLexicallyReachable` returns early for any candidate that is not a `function`, and the bare-import guard correctly declines because `vite` IS a workspace member, so the specifier really is project-local. What is wrong is only which node the name lands on. A JS/TS file that contains an `import` statement and no export of any form offers nothing to any other file, so none of its bindings is a candidate for a cross-file name match. Applied in both name-based strategies: declining in matchByExactName alone just hands the same target to matchFuzzy, which resolves a unique candidate on its own. Narrow on three axes, each a class this would otherwise get wrong in the opposite direction: a classic script is exempt (a top-level binding really is a reachable global), CommonJS is exempt (`module.exports` and `exports.x` count as exports), and every non-JS/TS language is exempt. The export test reads source rather than the node's `isExported` flag, because that flag is set only from an `export_statement` ancestor and so reads false for `const x = ...; export { x }`. * fix(resolution): count bracket CommonJS exports and `declare global` as exports A file writing `exports["x"] = …` exports x, and a file with a `declare global` block contributes every name in it to every other file whether or not it exports anything of its own — the extractor emits nodes for the ambient `var` and `interface` members, so sealing such a file would hide names that really are reachable everywhere. Neither shape occurs on the vite corpus, so this changes no measured count; both are now covered by the test. * test(resolution): bind the #1719 fixture without a bare import The consumer bound every name from 'some-external-pkg'. A bare specifier names a package that is not in the graph, so no project node is the right target for such a reference and #1715 declines it -- which made four of the five positive assertions depend on a resolution that should not happen, and they failed the moment this branch was stacked on #1715. Free references reach the same exact-match path without asserting that. `strayVar` was not testable at all: a bare identifier read emits no edge, so that assertion only ever passed through the bare-import binding. The `declare global` coverage moves to an interface reached through a type annotation, paired with an identical file whose interface is not in a `declare global` -- so the assertion turns on that clause rather than passing whichever way the guard goes. * docs(changelog): record the sealed-module guard under Unreleased * fix(resolution): the sealed test rejects fuzzy's survivor, never filters its set matchFuzzy declines an ambiguous name outright, so filtering sealed candidates out of its set can leave a lone survivor and manufacture a 0.5 edge from an ambiguity that would have been declined. Testing the single survivor instead closes that path; matchByExactName keeps the filter, because it ranks a crowd rather than declining one. No instance on vitejs/vite either way (row-identical, LOST 0 / GAINED 0 per #1720 review). It also declines one shape the filter form resolved: a sealed same-language survivor no longer yields to a cross-language candidate at 0.3. * fix(resolution): reject invalid fallback targets without retargeting --------- Co-authored-by: Aaron Queen Co-authored-by: Colby McHenry --- CHANGELOG.md | 1 + __tests__/frameworks-integration.test.ts | 52 +++++ __tests__/resolution.test.ts | 267 ++++++++++++++++++++++- src/mcp/dynamic-boundaries.ts | 39 +--- src/resolution/index.ts | 2 + src/resolution/name-matcher.ts | 137 +++++++++++- src/resolution/strip-comments.ts | 46 ++++ 7 files changed, 501 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 272f42b52..877bd3fbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -208,6 +208,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### Symbols, tests and the viewer - **A definition its language makes file-local no longer captures calls from other files.** A C `static` in another source file (`.c`/`.cc`/… — not a header's `static inline`, which is textually included), a Kotlin/Java/C#/Swift/Scala/Dart/PHP `private` member, a Go unexported name in another package, and a Rust non-`pub` item outside its module subtree cannot be what a name in another file means, but name matching accepted them whenever the names agreed: an Android `editor.apply()` onto an unrelated class's `private fun apply`, a JavaScript `fail(...)` onto a Go `func fail`, a Rust `.count()` onto a private `fn count` in another crate, and C USB helpers onto a `static` in a `.c` they never link. Such a target is now declined after the whole name-matching pipeline settles — the reference stays unresolved rather than falling through to a fuzzy namesake. Same-file definitions, a child Rust module reaching its ancestors' private items, and Rust `impl Trait for Type` methods stay resolvable. Re-index after upgrading. (#1730, #1731) +- **A binding in a module that exports nothing is no longer a cross-file target.** On vite, every `import { defineConfig } from 'vite'` across the playground resolved onto a `const vite = await createServer(…)` sitting at module scope in `playground/ssr-html/test-stacktrace.js` — a file with an import and no export, so that binding is reachable from nowhere but itself. Name matching commits as soon as one candidate survives, and nothing asked whether an import could reach the survivor; that one binding took 157 edges. A JS/TS file holding an `import` and no export of any kind now offers its locals to no other file. Classic scripts, CommonJS (including `exports["x"] = …`), a later `export { … }`, and names contributed through `declare global` are all unaffected. Across vite this removed 320 wrong edges and added 18, each addition a reference that was previously ambiguous rather than newly invented. Re-index after upgrading. (#1719) - **Files under an `e2e/` directory count as tests.** Their calls no longer appear as production callers in Steps, dead-code and test badges. diff --git a/__tests__/frameworks-integration.test.ts b/__tests__/frameworks-integration.test.ts index 3df4f2d88..be7e3aea8 100644 --- a/__tests__/frameworks-integration.test.ts +++ b/__tests__/frameworks-integration.test.ts @@ -3,6 +3,10 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { CodeGraph } from '../src'; +import { DatabaseConnection, getDatabasePath } from '../src/db'; +import { QueryBuilder } from '../src/db/queries'; +import { createResolver } from '../src/resolution'; +import type { Node } from '../src/types'; import { initGrammars, loadAllGrammars } from '../src/extraction/grammars'; beforeAll(async () => { @@ -10,6 +14,54 @@ beforeAll(async () => { await loadAllGrammars(); }); +describe('Express middleware imports', () => { + it('does not resolve package imports into license headings', async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-express-doc-import-')); + let cg: CodeGraph | undefined; + try { + fs.writeFileSync(path.join(tmpDir, 'package.json'), JSON.stringify({ dependencies: { express: '*', cors: '*' } })); + fs.writeFileSync(path.join(tmpDir, 'LICENSE.md'), '# cors\n\n# host-validation-middleware\n'); + fs.writeFileSync(path.join(tmpDir, 'local.js'), 'export function localMiddleware() {}\n'); + fs.writeFileSync(path.join(tmpDir, 'server.js'), [ + "import corsMiddleware from 'cors'", + "import { hostValidationMiddleware as originalHostValidationMiddleware } from 'host-validation-middleware'", + "import { localMiddleware } from './local.js'", + 'localMiddleware()', + ].join('\n')); + cg = await CodeGraph.init(tmpDir, { index: true }); + const local = cg.getNodesByKind('function').find((n) => n.name === 'localMiddleware'); + expect(local).toBeDefined(); + expect(cg.getIncomingEdges(local!.id).some((e) => e.kind === 'imports')).toBe(true); + expect(cg.getIncomingEdges(local!.id).some((e) => e.kind === 'calls')).toBe(true); + cg.close(); + cg = undefined; + const db = DatabaseConnection.open(getDatabasePath(tmpDir)); + try { + const queries = new QueryBuilder(db.getDb()); + for (const name of ['cors', 'host-validation-middleware']) { + queries.insertNode({ + id: `heading:${name}`, name, qualifiedName: `LICENSE.md#${name}`, + kind: 'module', language: 'markdown' as Node['language'], filePath: 'LICENSE.md', + startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0, + }); + } + const resolver = createResolver(tmpDir, queries); + for (const referenceName of ['cors', 'corsMiddleware', 'host-validation-middleware']) { + expect(resolver.resolveOne({ + fromNodeId: 'file:server.js', referenceName, referenceKind: 'imports', + filePath: 'server.js', language: 'javascript', line: 1, column: 0, + })).toBeNull(); + } + } finally { + db.close(); + } + } finally { + cg?.close(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); + describe('Django end-to-end framework extraction', () => { let tmpDir: string | undefined; afterEach(() => { diff --git a/__tests__/resolution.test.ts b/__tests__/resolution.test.ts index decaadee5..cb9062019 100644 --- a/__tests__/resolution.test.ts +++ b/__tests__/resolution.test.ts @@ -11,7 +11,7 @@ import * as os from 'os'; import { CodeGraph } from '../src'; import { Node, UnresolvedReference } from '../src/types'; import { ReferenceResolver, createResolver, ResolutionContext } from '../src/resolution'; -import { matchReference, resolveMethodOnType, matchByQualifiedName, preferCallSiteFile, matchMethodCall } from '../src/resolution/name-matcher'; +import { matchReference, resolveMethodOnType, matchByQualifiedName, matchByExactName, preferCallSiteFile, matchMethodCall } from '../src/resolution/name-matcher'; import { resolveImportPath, extractImportMappings, resolveJvmImport, loadCppIncludeDirs, clearCppIncludeDirCache, isPhpIncludePathRef } from '../src/resolution/import-resolver'; import type { UnresolvedRef } from '../src/resolution/types'; import { detectFrameworks, getAllFrameworkResolvers } from '../src/resolution/frameworks'; @@ -5386,4 +5386,269 @@ in expect(importedFilePaths('main.nix')).toEqual([]); }); }); + + describe('Bindings in a module that exports nothing (#1719)', () => { + it('does not treat documentation headings as package imports', () => { + // Inject the planned Markdown node shape without depending on its extractor. + const heading: Node = { + id: 'heading:vite', name: 'vite', qualifiedName: 'guide.md#vite', + kind: 'module', language: 'markdown' as Node['language'], filePath: 'guide.md', + startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0, + }; + const context = { + getNodesByName: () => [heading], getNodesInFile: () => [], + getNodesByQualifiedName: () => [], getNodesByKind: () => [], + fileExists: () => false, readFile: () => null, + getProjectRoot: () => tempDir, getAllFiles: () => [], + } as ResolutionContext; + const ref: UnresolvedRef = { + fromNodeId: 'file:consumer.ts', referenceName: 'vite', referenceKind: 'imports', + filePath: 'consumer.ts', language: 'typescript', line: 1, column: 0, + }; + expect(matchByExactName(ref, context)).toBeNull(); + expect(matchByExactName({ ...ref, language: 'markdown' as Node['language'] }, context)?.targetNodeId).toBe(heading.id); + context.getNodesByName = () => [{ ...heading, id: 'fn:vite', kind: 'function', language: 'typescript', filePath: 'vite.ts' }]; + expect(matchByExactName(ref, context)?.targetNodeId).toBe('fn:vite'); + }); + + it('ignores export examples in strings and comments when checking module visibility', async () => { + fs.mkdirSync(path.join(tempDir, 'src')); + fs.writeFileSync(path.join(tempDir, 'src/private.js'), [ + "import fs from 'node:fs'", + 'const example = `', + 'export const example = 1', + '`', + '/*', + 'export { hidden }', + '*/', + 'function hidden() { return fs }', + 'hidden()', + ].join('\n')); + fs.writeFileSync(path.join(tempDir, 'src/consumer.js'), 'hidden()'); + fs.mkdirSync(path.join(tempDir, 'legacy')); + fs.writeFileSync(path.join(tempDir, 'legacy/global.js'), 'function hidden() { return 1 }'); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const hidden = cg.getNodesByKind('function').find((n) => n.name === 'hidden' && n.filePath === 'src/private.js'); + expect(hidden).toBeDefined(); + const callers = cg.getIncomingEdges(hidden!.id).filter((e) => e.kind === 'calls'); + expect(callers.some((e) => cg.getNode(e.source)?.filePath === 'src/consumer.js')).toBe(false); + expect(callers.some((e) => cg.getNode(e.source)?.filePath === 'src/private.js')).toBe(true); + const consumer = cg.getNodesByKind('file').find((n) => n.filePath === 'src/consumer.js'); + expect(cg.getOutgoingEdges(consumer!.id).filter((e) => e.kind === 'calls')).toEqual([]); + }); + + it('does not name-match a method call to another file\'s JSON value', async () => { + fs.writeFileSync(path.join(tempDir, 'data.json'), '{"content": "hello"}'); + fs.writeFileSync(path.join(tempDir, 'data.js'), "const content = require('./data.json')\nmodule.exports = { content }\n"); + fs.writeFileSync(path.join(tempDir, 'consumer.js'), 'export async function read(page) { return page.frame("main").content() }'); + fs.writeFileSync(path.join(tempDir, 'use-data.js'), "import { content } from './data'\nconsole.log(content)\n"); + fs.writeFileSync(path.join(tempDir, 'callback.js'), "const callback = require('./handler.js')\nmodule.exports = { callback }\n"); + fs.writeFileSync(path.join(tempDir, 'call.js'), 'callback()'); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const content = cg.getNodesByKind('constant').find((n) => n.name === 'content'); + expect(content).toBeDefined(); + expect(cg.getIncomingEdges(content!.id).filter((e) => e.kind === 'calls')).toEqual([]); + expect(cg.getIncomingEdges(content!.id).some((e) => e.kind === 'imports')).toBe(true); + const callback = cg.getNodesByKind('constant').find((n) => n.name === 'callback'); + expect(callback).toBeDefined(); + expect(cg.getIncomingEdges(callback!.id).some((e) => e.kind === 'calls')).toBe(true); + }); + + it('keeps a local file dependency import when a closer private name collides', async () => { + fs.writeFileSync(path.join(tempDir, 'package.json'), JSON.stringify({ dependencies: { 'local-dep': 'file:./dep' } })); + fs.mkdirSync(path.join(tempDir, 'dep')); + fs.mkdirSync(path.join(tempDir, 'src')); + fs.writeFileSync(path.join(tempDir, 'dep/package.json'), JSON.stringify({ name: 'local-dep', main: 'index.js' })); + fs.writeFileSync(path.join(tempDir, 'dep/index.js'), "export const msg = 'local'\n"); + fs.writeFileSync(path.join(tempDir, 'src/private.js'), "import fs from 'node:fs'\nconst msg = 'private'\n"); + fs.writeFileSync(path.join(tempDir, 'src/consumer.js'), "import { msg } from 'local-dep'\nconsole.log(msg)\n"); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const msg = cg.getNodesByKind('constant').find((n) => n.name === 'msg' && n.filePath === 'dep/index.js'); + expect(msg).toBeDefined(); + expect(cg.getIncomingEdges(msg!.id).some((e) => e.kind === 'imports')).toBe(true); + }); + + it('preserves executable CommonJS exports inside nested template interpolations', async () => { + fs.writeFileSync(path.join(tempDir, 'cjs.js'), [ + "import fs from 'node:fs'", + 'function helper() { return fs }', + 'const text = `outer ${`inner ${module.exports = { helper }}`}`', + ].join('\n')); + fs.writeFileSync(path.join(tempDir, 'consumer.js'), 'helper()'); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const helper = cg.getNodesByKind('function').find((n) => n.name === 'helper'); + expect(helper).toBeDefined(); + expect(cg.getIncomingEdges(helper!.id).some((e) => + e.kind === 'calls' && cg.getNode(e.source)?.filePath === 'consumer.js')).toBe(true); + }); + + it.each(['export function visible() { return fs }', 'function visible() { return fs }\nexport { visible }'])('preserves real exports after a regex containing a backtick: %s', async (declaration) => { + fs.writeFileSync(path.join(tempDir, 'exported.js'), "import fs from 'node:fs'\nconst re = /`/\nif (fs) /`/.test('text')\nelse /`/.test('other')\nconst make = () => /`/\n" + declaration + '\n'); + fs.writeFileSync(path.join(tempDir, 'consumer.js'), 'visible()'); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const visible = cg.getNodesByKind('function').find((n) => n.name === 'visible'); + expect(visible).toBeDefined(); + expect(cg.getIncomingEdges(visible!.id).some((e) => e.kind === 'calls')).toBe(true); + }); + + // On vitejs/vite, every `import { defineConfig } from 'vite'` across the + // playground resolved onto `playground/ssr-html/test-stacktrace.js::vite` + // — `const vite = await createServer(…)` at module scope in a file with + // zero exports — because exact-match commits whenever one candidate + // survives, and nothing asked whether an import could reach it. Only + // `sealed.js` may be filtered; every other file here is a class that must + // NOT be — a classic script (a top-level binding really is a reachable + // global), a CommonJS module, one exporting through `exports["x"]`, an ESM + // file whose export is a later `export { … }` statement (which leaves + // `isExported` false on the declaration's node), and one contributing a + // name through `declare global` while exporting nothing of its own. + let tmpDir: string; + let cg: CodeGraph; + + afterEach(() => { + cg?.close(); + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('drops them as cross-file candidates, and keeps scripts, CJS and later exports', async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1719-')); + fs.writeFileSync( + path.join(tmpDir, 'sealed.js'), + `import fsp from 'node:fs/promises' + +function widget() { + return fsp +} + +widget() +` + ); + fs.writeFileSync( + path.join(tmpDir, 'script.js'), + `function gadget() { + return 1 +} +` + ); + fs.writeFileSync( + path.join(tmpDir, 'cjs.js'), + `import osp from 'node:os' + +function helper() { + return osp +} + +module.exports = { helper } +` + ); + fs.writeFileSync( + path.join(tmpDir, 'later.js'), + `import pathp from 'node:path' + +function parser() { + return pathp +} + +export { parser } +` + ); + // `exports["x"]` is a CommonJS export too, and a file declaring globals + // offers them to every other file whether or not it exports anything of + // its own. Both would read as sealed on a test that looked only for + // `export …`, `module.exports` and `exports.x`. + fs.writeFileSync( + path.join(tmpDir, 'bracket.js'), + `import urlp from 'node:url' + +function bracketed() { + return urlp +} + +exports["bracketed"] = bracketed +` + ); + // A module with imports and no export of its own still contributes every + // name in `declare global` to every other file. `plain.ts` is the control + // that makes the assertion mean something: it is the same "import, no + // export" shape holding the same kind of declaration, so the pair differs + // only by the `declare global`, and an assertion on StrayFace alone would + // pass whatever the guard did. + fs.writeFileSync( + path.join(tmpDir, 'ambient.ts'), + `import './later' + +declare global { + interface StrayFace { + a: number + } +} +` + ); + fs.writeFileSync( + path.join(tmpDir, 'plain.ts'), + `import './later' + +interface HiddenFace { + a: number +} + +const unused: HiddenFace = { a: 1 } +` + ); + // A type annotation is the reference here, so this consumer must be .ts. + fs.writeFileSync( + path.join(tmpDir, 'consumer.ts'), + `const face: StrayFace = { a: 1 } +const hidden: HiddenFace = { a: 2 } + +export function use(): number { + return face.a + hidden.a +} +` + ); + // Nothing here is bound by an import, so every name is a free reference + // that falls through to exact name matching — the path this rule sits on. + // A bare import would reach that path too, but a bare specifier names a + // package outside the graph, so no project node is the right target for + // it and such a fixture would assert a resolution nothing should make. + fs.writeFileSync( + path.join(tmpDir, 'consumer.js'), + `widget() +gadget() +helper() +parser() +bracketed() +` + ); + + cg = await CodeGraph.init(tmpDir, { index: true }); + cg.resolveReferences(); + + // Incoming edges rather than callers, so the interfaces are asked the + // same question as the functions: a type annotation is a reference, not + // a call. + const reachedFrom = (consumer: string, name: string): boolean => { + const target = cg + .searchNodes(name, { limit: 10 }) + .find((r) => r.node.name === name && r.node.filePath !== consumer); + expect(target, `no node named ${name}`).toBeDefined(); + return cg + .getIncomingEdges(target!.node.id) + .some((e) => cg.getNode(e.source)?.filePath === consumer); + }; + + expect(reachedFrom('consumer.js', 'widget')).toBe(false); + expect(reachedFrom('consumer.js', 'gadget')).toBe(true); + expect(reachedFrom('consumer.js', 'helper')).toBe(true); + expect(reachedFrom('consumer.js', 'parser')).toBe(true); + expect(reachedFrom('consumer.js', 'bracketed')).toBe(true); + expect(reachedFrom('consumer.ts', 'StrayFace')).toBe(true); + expect(reachedFrom('consumer.ts', 'HiddenFace')).toBe(false); + }, 30000); + }); }); diff --git a/src/mcp/dynamic-boundaries.ts b/src/mcp/dynamic-boundaries.ts index 2378ddaeb..7307bd3fe 100644 --- a/src/mcp/dynamic-boundaries.ts +++ b/src/mcp/dynamic-boundaries.ts @@ -21,7 +21,8 @@ * inside a string is a false positive, so {@link blankStringContents} blanks * them too, quotes preserved.) */ -import { stripCommentsForRegex, type CommentLang } from '../resolution/strip-comments'; +import { blankStringContents, stripCommentsForRegex, type CommentLang } from '../resolution/strip-comments'; +export { blankStringContents } from '../resolution/strip-comments'; export interface BoundaryMatch { /** Stable form id, e.g. 'computed-call' — used for per-form dedupe. */ @@ -222,42 +223,6 @@ function commentLang(language: string): CommentLang | null { const MAX_MATCHES_PER_BODY = 3; const MAX_BODY_CHARS = 60_000; // a god-function tail is still scannable; beyond this, truncate -/** - * Blank the CONTENTS of string literals (quotes preserved, offsets preserved) - * so dispatch-shaped prose — docs, error messages, template text — can't fire - * a matcher. Run AFTER comment stripping (comments are already spaces). - * Backslash escapes are honored; `'`/`"` strings end at a newline (treated as - * unterminated, matching the comment stripper); backticks span lines, and - * `${...}` interpolations inside them are blanked too — missing a dispatch - * inside a template literal is acceptable, false-firing on prose is not. - */ -export function blankStringContents(text: string): string { - const out = text.split(''); - let i = 0; - const n = text.length; - while (i < n) { - const c = text[i]!; - if (c === '"' || c === "'" || c === '`') { - const quote = c; - i++; - while (i < n && text[i] !== quote) { - if (text[i] === '\\' && i + 1 < n) { - out[i] = ' '; - out[i + 1] = ' '; - i += 2; - continue; - } - if (quote !== '`' && text[i] === '\n') break; // unterminated — stop blanking - if (text[i] !== '\n') out[i] = ' '; // keep newlines for line math - i++; - } - if (i < n && text[i] === quote) i++; - continue; - } - i++; - } - return out.join(''); -} /** * Scan one symbol's body for dynamic-dispatch sites. diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 2a995b985..16bdd0d89 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -2458,6 +2458,8 @@ export class ReferenceResolver { if (!result) return result; if (ref.referenceKind !== 'references' && ref.referenceKind !== 'imports') return result; const tgt = this.getLanguageFromNodeId(result.targetNodeId); + // Package imports cannot target prose found by a framework's name lookup. + if (ref.referenceKind === 'imports' && (tgt as string) === 'markdown' && (ref.language as string) !== 'markdown') return null; if (tgt && ref.language && crossesKnownFamily(tgt, ref.language)) return null; return result; } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 6a1863bff..9c00b9fe7 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -7,6 +7,7 @@ import * as path from 'path'; import { Language, Node } from '../types'; import { UnresolvedRef, ResolvedRef, ResolutionContext } from './types'; +import { blankStringContents, stripCommentsForRegex } from './strip-comments'; /** * Ceiling on how many same-named definitions a FUZZY name-match strategy will @@ -389,6 +390,108 @@ function isLexicallyReachable( ); } +/** Languages whose module boundary is `import`/`export` (or CommonJS). */ +const ESM_FAMILY = new Set(['typescript', 'tsx', 'javascript', 'jsx', 'arkts']); + +/** + * A line-initial `import` statement — the marker that a JS/TS file is a MODULE + * rather than a classic script. Line-anchored and followed by a name, brace, + * star or quote, so a dynamic `import(` and the word inside a comment or string + * do not match. + */ +const HAS_IMPORT_STATEMENT = /^[ \t]*import[\s{*'"]/m; + +/** + * Anything the file could offer another file, in every form the extractor's own + * `isExported` flag misses. `^export` covers the declaration and later forms + * (`export const`, `export { x }`, `export default x`, `export *`); the + * CommonJS shapes cover files that never use ESM syntax at all, in both the dot + * and the bracket form; and `declare global` contributes names to every file + * whether or not the module exports anything of its own. Kept as a source test + * rather than a node scan precisely because `isExported` is set only from an + * `export_statement` ancestor, so `const x = …; export { x }` and + * `module.exports = { x }` both read as unexported on the node. + */ +const HAS_ESM_EXPORT = /^[ \t]*export[\s{*]|^[ \t]*declare\s+global\b/m; +const HAS_CJS_EXPORT = /\bmodule\.exports\b|\bexports\s*[.[]/; + +/** + * Per-context memo of "this file is a module that exports nothing", asked once + * per candidate FILE rather than once per reference. Derived from file source, + * so it drops with the context's file caches — clearNameMatcherMemos deletes it + * alongside INFER_SCAN_STATES. + */ +const SEALED_MODULES = new WeakMap>(); + +/** + * Whether `filePath` is a JS/TS module that exports NOTHING — an import + * statement present, no export of any form. No reference from another file can + * reach any binding in such a file, so every one of its symbols is a false + * candidate for a cross-file name match. + * + * This is the general case behind a package name capturing a same-named local: + * on `vitejs/vite`, 157 cross-file `imports` refs — every `import { defineConfig + * } from 'vite'` in the playground and the create-vite templates — resolved onto + * `playground/ssr-html/test-stacktrace.js::vite`, which is `const vite = await + * createServer(…)` at module scope in a file with zero exports. The existing + * guards cannot see it: `isLexicallyReachable` returns early for any candidate + * that is not a `function`, and the bare-import guard correctly declines because + * `vite` IS a workspace member, so the specifier really is project-local. What + * is wrong is only which node the name lands on. + * + * Deliberately narrow on three axes, because each is a class this would + * otherwise resolve wrongly in the opposite direction: + * + * - **A classic script is exempt.** Requiring an `import` statement means a + * non-module `.js` file — concatenated globals, a browser `