diff --git a/.agents/lessons.md b/.agents/lessons.md index d9722cd..579fe2b 100644 --- a/.agents/lessons.md +++ b/.agents/lessons.md @@ -15,3 +15,6 @@ Each entry is a single bullet: `- **** — `. Newest entries at t - **PR / issue / comment bodies always go through a temp file** — never pass markdown bodies via shell heredoc to `gh pr create --body "$(cat <<'EOF'…)"` / `gh pr edit --body …` / `gh pr comment --body …` / `gh issue create --body …` / `gh api` `--field body=…`. Backticks inside the heredoc (every code span and code fence) get shell-escaped to `\`` and render literally on GitHub — every recipe id, file path, flag, SQL fragment, and code fence in the rendered body comes out as `\`coverage\``instead of`coverage`. Pattern: write the body to a temp file (`Write`to`/tmp/pr--body.md`), pass `--body-file /tmp/pr--body.md`, then delete the temp file. Cost is one extra tool call; saves redoing every PR body that has more than a few backticks. Hit on PR #57 — final body was a wall of `\`` artifacts until rewritten via temp file. - **Never commit absolute local user paths** — no `/Users//…`, `/home//…`, `~/…`, or `file:///` URIs in any tracked doc, code, comment, or PR body. Reasons: (1) leaks the maintainer's directory structure / username to public mirrors; (2) every other contributor's paths differ — the reference is dead on their machine; (3) a `git clone` of someone else's machine isn't a fact we can cite as a "source for deep-dives" — public upstream URLs are. Pattern: cite `https://github.com//` (with optional `/tree//`) for upstream sources; use repo-relative paths (`docs/foo.md`, `src/bar.ts`) for in-tree references. Hit on PR #58 first draft — referenced a local peer-repo clone path in a research note before the user caught it. - **Prescriptive research notes pin every concrete claim before recommending a ship sequence** — when a research/plan-shape doc proposes work (effort estimates, capability inventories, "we already do X" framing), every concrete claim needs a `file:line` / `codemap query` / `rg` / `--recipes-json` reference a reviewer can re-run. Reasoning-from-substrate intuition without pinning ships errors: "the AST walker already counts nodes" / "fan-in detects orphans" / "the `re_export_source` column doesn't exist" — all real errors caught on PR #58 by triangulating against the codebase. Don't ship a peer / parallel "descriptive baseline" doc to triangulate against (Rule 1 violation — it duplicates `architecture.md` / `db.ts` / `--recipes-json`); instead, either (a) pin claims in the prescriptive doc itself, or (b) self-audit by re-running every claim against the canonical home before committing. Either path beats the "dual descriptive + prescriptive doc" pattern on docs-governance grounds. +- **Semicolons inside `--` line comments in `db.ts` DDL strings break Node CI** — `architecture.md` notes this but the lesson wasn't in this file. `runSql()` on Node splits multi-statement SQL on `;` (because `better-sqlite3` allows one statement per `prepare()`); a `;` inside an inner `--` comment (e.g. `-- recipe_id is loose (matches a or b; no FK)`) creates a comment-only fragment, which `prepare()` rejects with `RangeError: The supplied SQL string contains no statements`. **Bun tests don't catch this** — `bun:sqlite` accepts multi-statement SQL natively, so `bun test` + `bun run check` are both green; the failure surfaces only when CI runs `node dist/index.mjs --full`. **Pattern:** rewrite comment-side `;` to a comma or em-dash; sanity-check with a tiny harness that splits the createTables template on `;` and prints any fragment that's all-`--`-comments. Hit on PR #76. The validator can be a one-liner — adding it as `bun run smoke:node` is a candidate roadmap item. +- **`process.exitCode` is NOT a safe success oracle inside CLI verbs** — when a CLI handler in `runQueryCmd` (or any sibling that supports `--ci`) needs to know "did the work succeed?" for a side-effect decision (recency tracking, telemetry, follow-up writes), do NOT key off `process.exitCode !== 1`. Two failure modes: (1) `--ci` deliberately sets `exitCode = 1` on findings as the CI gating signal even though the recipe ran cleanly — keying off exitCode then treats success as failure (PR #76 audit Finding 1: `--ci + --recipe X --format sarif` exits 1 → recency NOT recorded → undercounts every CI run). (2) The exit code is process-global and survives across calls when an exported helper is invoked multiple times in one process (tests, programmatic use); a prior failure poisons later success. **Pattern:** track an explicit `recipeQuerySucceeded` (or domain-named) local flag, set true at each successful exit point inside the try block; check it in the finally. For helpers that conflate "ran cleanly" with "exit 1 because findings" (like `printFormattedQuery`), refactor to a discriminated-union return shape (`{ ok: true, exitCode: 0 | 1 } | { ok: false }`) so callers can disambiguate. Hit on PR #76 — caught by 2/3 PR-review agents (Codex + gpt-5.5) with concrete repros. +- **Read-side substrate must be pure even when "lazy on read" looks cheap** — when a write-side cleanup (DELETE-on-prune, GC sweep, cache invalidation) seems cheaper to run lazily on the read path, weigh that against the documented contract of the read site. If the read surface promises "no side effects" / "no DB required" / "read resource" semantics (per CLI help text, README, MCP resource conventions), an in-place DELETE breaks the contract — even under WAL where the write doesn't block readers. **Pattern:** filter at SELECT time (`WHERE last_run_at >= cutoff`) for read paths, do eager prune on the write path (typically cheap on tiny domain tables — index-bounded scan is microseconds). The "hot path" cost concern that motivates lazy-on-read usually doesn't hold for tiny tables, but the read-purity invariant is durable across N agents reading the resource. Hit on PR #76 — Q3 of the recipe-recency plan locked lazy-on-read with reasoning that didn't model the "No DB required" `--recipes-json` contract; gpt-5.5 audit caught it. diff --git a/.agents/rules/codemap.md b/.agents/rules/codemap.md index 7a29e83..a4b44df 100644 --- a/.agents/rules/codemap.md +++ b/.agents/rules/codemap.md @@ -23,7 +23,7 @@ A local database (default **`.codemap/index.db`**) indexes structure: symbols, i | Parametrised recipe | — | `bun src/index.ts query --json --recipe find-symbol-by-kind --params kind=function,name_pattern=%Query%` — params declared in recipe `.md` frontmatter and validated before SQL binding. | | Boundary violations | — | `bun src/index.ts query --json --recipe boundary-violations` — joins `dependencies` × `boundary_rules` (config-driven) via SQLite `GLOB`. `.codemap/config.ts` `boundaries: [{name, from_glob, to_glob, action?}]`; default `action: "deny"`. SARIF / annotations work via the `file_path` alias. | | Rename preview | — | `bun src/index.ts query --recipe rename-preview --params old=usePermissions,new=useAccess,kind=function --format diff` — read-only unified diff; codemap never writes files. | -| Recipe catalog / SQL | — | `bun src/index.ts query --recipes-json` · `bun src/index.ts query --print-sql fan-out` | +| Recipe catalog / SQL | — | `bun src/index.ts query --recipes-json` (every entry includes `last_run_at: number \| null` + `run_count: number` recency fields; rank with `jq 'sort_by(.last_run_at // 0) \| reverse'`; opt-out via `.codemap/config` `recipeRecency: false`) · `bun src/index.ts query --print-sql fan-out` | | Counts only | — | `bun src/index.ts query --json --summary -r deprecated-symbols` | | PR-scoped rows | — | `bun src/index.ts query --json --changed-since origin/main -r fan-out` | | Bucket by owner / dir / pkg | — | `bun src/index.ts query --json --group-by directory -r fan-in` | @@ -75,12 +75,12 @@ Validation: SQL is rejected at load time if it starts with DML/DDL (DELETE/DROP/ **Impact (`bun src/index.ts impact `)**: symbol/file blast-radius walker — replaces hand-composed `WITH RECURSIVE` queries that agents struggle to write. Target auto-resolves: contains `/` or matches `files.path` → file target; otherwise symbol (case-sensitive). Walks compatible graphs by target kind: **symbol** → `calls` (callers / callees by name); **file** → `dependencies` + `imports` (`resolved_path` only). `--via ` overrides; mismatched explicit choices land in `skipped_backends` (no error). Cycle-detected via `WITH RECURSIVE` path-string + `instr` check; bounded by `--depth N` (default 3, `0` = unbounded but still cycle-detected and limit-capped) and `--limit N` (default 500). Output envelope: `{target, direction, via, depth_limit, matches: [{depth, direction, edge, kind, name?, file_path}], summary: {nodes, max_depth_reached, by_kind, terminated_by: 'depth'|'limit'|'exhausted'}}`. `--summary` trims `matches` for cheap CI-gate consumption (`jq '.summary.nodes'`) but preserves the count. SARIF / annotations not supported (graph traversal, not findings). Pure transport-agnostic engine in `application/impact-engine.ts`; CLI / MCP / HTTP all dispatch the same `findImpact` function. -**MCP server (`bun src/index.ts mcp`)**: stdio MCP (Model Context Protocol) server — agents call codemap as JSON-RPC tools instead of shelling out to the CLI on every read. v1 ships one tool per CLI verb plus four lazy-cached resources: +**MCP server (`bun src/index.ts mcp`)**: stdio MCP (Model Context Protocol) server — agents call codemap as JSON-RPC tools instead of shelling out to the CLI on every read. v1 ships one tool per CLI verb plus six resources (`codemap://recipes` + `codemap://recipes/{id}` are live read every call so inline `last_run_at` / `run_count` recency stays fresh; `codemap://schema` + `codemap://skill` lazy-cache; `codemap://files/{path}` + `codemap://symbols/{name}` always live): - **Tools:** `query` / `query_batch` / `query_recipe` / `audit` / `save_baseline` / `list_baselines` / `drop_baseline` / `context` / `validate` / `show` / `snippet` / `impact`. Snake_case keys (Codemap convention matching MCP spec examples + reference servers — spec is convention-agnostic; CLI stays kebab). - **`query_batch` (MCP-only):** N statements in one round-trip. Items are `string | {sql, summary?, changed_since?, group_by?}` — string form inherits batch-wide flag defaults, object form overrides on a per-key basis. Per-statement errors are isolated. - **`save_baseline` (polymorphic):** one tool, `{name, sql? | recipe?}` with runtime exclusivity check (mirrors the CLI's single `--save-baseline=` verb). -- **Resources:** `codemap://recipes` (catalog), `codemap://recipes/{id}` (one recipe), `codemap://schema` (live DDL from `sqlite_schema`), `codemap://skill` (bundled SKILL.md text), `codemap://files/{path}` (per-file roll-up: symbols, imports, exports, coverage), `codemap://symbols/{name}` (symbol lookup with `{matches, disambiguation?}` envelope; `?in=` filter mirrors `show --in`). Catalog resources lazy-cache on first `read_resource`; `files/` and `symbols/` read live every call (no caching) since the index can change between requests under `--watch`. +- **Resources:** `codemap://recipes` (catalog — live), `codemap://recipes/{id}` (one recipe — live), `codemap://schema` (live DDL from `sqlite_schema`; lazy-cached), `codemap://skill` (bundled SKILL.md text; lazy-cached), `codemap://files/{path}` (per-file roll-up: symbols, imports, exports, coverage — live), `codemap://symbols/{name}` (symbol lookup with `{matches, disambiguation?}` envelope; `?in=` filter mirrors `show --in` — live). Recipe catalogs read live every call so inline `last_run_at` / `run_count` recency reflects mutations during the server lifetime; `schema` / `skill` cache because their inputs don't change mid-session. - **Output shape uniformity:** every tool returns the JSON envelope its CLI counterpart's `--json` would print — no re-mapping. Schema additions to the CLI envelope propagate to MCP automatically. For developing the MCP server itself: `src/cli/cmd-mcp.ts` (CLI shell) + `src/application/mcp-server.ts` (engine). See [`docs/architecture.md` § MCP wiring](../../docs/architecture.md#cli-usage). diff --git a/.agents/skills/codemap/SKILL.md b/.agents/skills/codemap/SKILL.md index 21ef321..ee0a99d 100644 --- a/.agents/skills/codemap/SKILL.md +++ b/.agents/skills/codemap/SKILL.md @@ -38,7 +38,7 @@ Replace placeholders (`'...'`) with your module path, file glob, or symbol name. **Suppressions (opt-in):** `// codemap-ignore-next-line ` and `// codemap-ignore-file ` (also `#`, `--`, `