From e95707076bbb30e6326b806c572ec6f6d471b834 Mon Sep 17 00:00:00 2001 From: shahinyanm Date: Wed, 24 Jun 2026 10:26:31 +0400 Subject: [PATCH] =?UTF-8?q?fix:=20v0.47.1=20=E2=80=94=20gate=20AST=5FINDEX?= =?UTF-8?q?=5FWALK=5FUP=20on=20.git=20marker=20+=20deterministic=20CLI=20t?= =?UTF-8?q?ests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - fix(ast-index): AST_INDEX_WALK_UP is now set only when projectRoot has no `.git` marker of its own. Forcing it on every spawn let a git worktree nested under the main repo walk up past its own gitlink and escape to the parent repo's index, returning the wrong files. Marker computed once (computeHasGitMarker), recomputed on updateProjectRoot. Bare monorepo subdirs (no marker) keep walk-up. - docs(adr): 0002-ast-index-multi-root-scoping — records the walk-up gate (accepted) and the deferred --local/subtree plan to re-enable ast-index on multi-repo parents (proposed). - test: neutralise CLAUDE_PROJECT_DIR + CLAUDE_PLUGIN_ROOT in index/installer setup so git-detection and install assertions are deterministic regardless of which suites share a vitest worker. Mock the git exec seam to drop the real 3s-timeout flake; retry guards the residual race. Type the explore exec mock. Full suite: only the 3 pre-existing context-mode-detector env failures remain (real local .mcp.json; clean in CI). npm audit 0. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude-plugin/marketplace.json | 4 +- .claude-plugin/plugin.json | 2 +- CHANGELOG.md | 20 +++ agents/tp-api-surface-tracker.md | 2 +- agents/tp-audit-scanner.md | 2 +- agents/tp-commit-writer.md | 2 +- agents/tp-context-engineer.md | 2 +- agents/tp-dead-code-finder.md | 2 +- agents/tp-debugger.md | 2 +- agents/tp-dep-health.md | 2 +- agents/tp-doc-writer.md | 2 +- agents/tp-history-explorer.md | 2 +- agents/tp-impact-analyzer.md | 2 +- agents/tp-incident-timeline.md | 2 +- agents/tp-incremental-builder.md | 2 +- agents/tp-migration-scout.md | 2 +- agents/tp-onboard.md | 2 +- agents/tp-performance-profiler.md | 2 +- agents/tp-pr-reviewer.md | 2 +- agents/tp-refactor-planner.md | 2 +- agents/tp-review-impact.md | 2 +- agents/tp-run.md | 2 +- agents/tp-session-restorer.md | 2 +- agents/tp-ship-coordinator.md | 2 +- agents/tp-spec-writer.md | 2 +- agents/tp-test-coverage-gapper.md | 2 +- agents/tp-test-triage.md | 2 +- agents/tp-test-writer.md | 2 +- docs/adr/0002-ast-index-multi-root-scoping.md | 107 ++++++++++++++ package-lock.json | 4 +- package.json | 2 +- server.json | 4 +- src/ast-index/client.ts | 40 ++++-- tests/ast-index/client.test.ts | 70 +++++++-- tests/index.test.ts | 136 +++++++++++++----- 35 files changed, 351 insertions(+), 88 deletions(-) create mode 100644 docs/adr/0002-ast-index-multi-root-scoping.md diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 8ce150d..4327e76 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -6,14 +6,14 @@ }, "metadata": { "description": "Token Pilot — save 60-90% tokens when AI reads code", - "version": "0.47.0" + "version": "0.47.1" }, "plugins": [ { "name": "token-pilot", "source": "./", "description": "Reduces token consumption by 60-90% via AST-aware lazy file reading, structural symbol navigation, and cross-session tool-usage analytics. 24 MCP tools + 25 subagents + budget watchdog hooks.", - "version": "0.47.0", + "version": "0.47.1", "author": { "name": "Digital-Threads" }, diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 4cc464f..fe2fdaa 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "token-pilot", - "version": "0.47.0", + "version": "0.47.1", "description": "Saves 60-90% tokens on AI code reading. AST-aware lazy reads, symbol navigation, find_usages, structural git diff/log, edit-safety guard, Task-routing matcher, cross-session telemetry (errors + diagnostics), 25 tp-* subagents tiered to haiku/sonnet/opus with budget watchdog.", "author": { "name": "Digital-Threads", diff --git a/CHANGELOG.md b/CHANGELOG.md index ecb0400..c95d0aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,26 @@ All notable changes to Token Pilot will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.47.1] - 2026-06-24 + +### Fixed — gate AST_INDEX_WALK_UP on the `.git` marker (nested-worktree escape) + +`exec()` set `AST_INDEX_WALK_UP=1` unconditionally. The flag tells ast-index to +traverse past nested VCS markers and reuse a parent-level index — it exists for +bare monorepo subdirs (no `.git`, no local DB). But forcing it when `projectRoot` +is itself a git repo/worktree root let a worktree nested under the main repo +(`main-repo/.worktrees/feature`) walk up past its own `.git` and **escape to the +main repo's index, returning the wrong files**. Now the flag is set only when +`projectRoot` has no `.git` marker of its own (a `.git` dir or worktree gitlink +file both count). See `docs/adr/0002-ast-index-multi-root-scoping.md` — also +records the deferred `--local`/subtree plan for multi-repo parents. + +### Fixed — deterministic CLI tests under parallel sharding + +`index.test.ts` / `installer.test.ts` now neutralise `CLAUDE_PROJECT_DIR` and +`CLAUDE_PLUGIN_ROOT` in setup so the git-detection and install assertions don't +flake when another suite leaks those env vars into a shared vitest worker. + ## [0.47.0] - 2026-06-24 ### Added — `explore` tool: one-shot ranked context + graph blast-radius diff --git a/agents/tp-api-surface-tracker.md b/agents/tp-api-surface-tracker.md index 29a2773..acac895 100644 --- a/agents/tp-api-surface-tracker.md +++ b/agents/tp-api-surface-tracker.md @@ -9,7 +9,7 @@ tools: - mcp__token-pilot__read_symbol - Bash model: haiku -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: dd184501203fa7f3c73f419c4ffbe33c4be75400cb64a7a51733a3fe23f6e085 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-audit-scanner.md b/agents/tp-audit-scanner.md index 54ce924..b29e7cf 100644 --- a/agents/tp-audit-scanner.md +++ b/agents/tp-audit-scanner.md @@ -11,7 +11,7 @@ tools: - Grep - Read model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: d172f600bf32277ea6eb4cbbee4542ddd698a986dcd96997d33930561964569b requiredMcpServers: - "token-pilot" diff --git a/agents/tp-commit-writer.md b/agents/tp-commit-writer.md index 2e78b81..face5f9 100644 --- a/agents/tp-commit-writer.md +++ b/agents/tp-commit-writer.md @@ -8,7 +8,7 @@ tools: - mcp__token-pilot__test_summary - mcp__token-pilot__outline - Bash -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: de64a406b5176de19f7422619c7de7949b1f28865f225402c9cea9255f377428 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-context-engineer.md b/agents/tp-context-engineer.md index 9bbe0e7..40138e5 100644 --- a/agents/tp-context-engineer.md +++ b/agents/tp-context-engineer.md @@ -13,7 +13,7 @@ tools: - Edit - Glob model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 68b32af2dacd82ebe52c4eec93edb903d452688274c3065218270627c564d8b0 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-dead-code-finder.md b/agents/tp-dead-code-finder.md index acb4197..fc5e8a1 100644 --- a/agents/tp-dead-code-finder.md +++ b/agents/tp-dead-code-finder.md @@ -11,7 +11,7 @@ tools: - Grep - Read model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: d9b7f5b7ae6f4ae21305c775361bcab097cc774370a6d976c093571d46d55021 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-debugger.md b/agents/tp-debugger.md index 7c9b1ec..f25d396 100644 --- a/agents/tp-debugger.md +++ b/agents/tp-debugger.md @@ -12,7 +12,7 @@ tools: - Read - Bash model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 052413de8d92377edcde6ae5c823f5378db304baccfa29e8866467f42553a500 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-dep-health.md b/agents/tp-dep-health.md index 9d49f70..6560cf8 100644 --- a/agents/tp-dep-health.md +++ b/agents/tp-dep-health.md @@ -9,7 +9,7 @@ tools: - Bash - Read model: haiku -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: e14dc57493d816f8c2e017963e2ef5f66bea50fd0b805a80e8a0d97c968427e7 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-doc-writer.md b/agents/tp-doc-writer.md index f335c07..c43438d 100644 --- a/agents/tp-doc-writer.md +++ b/agents/tp-doc-writer.md @@ -13,7 +13,7 @@ tools: - Edit - Glob model: haiku -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 57d741794ab40e31a7ac49c68ea39a9088f5827cdef866ce81bfca1b7c9180cf requiredMcpServers: - "token-pilot" diff --git a/agents/tp-history-explorer.md b/agents/tp-history-explorer.md index 9043636..5445ad3 100644 --- a/agents/tp-history-explorer.md +++ b/agents/tp-history-explorer.md @@ -10,7 +10,7 @@ tools: - Bash - Read model: haiku -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 7b70fa76a60e3c58a1de4f56c32c0f166424137e203a0cf1c8654e7c9235d904 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-impact-analyzer.md b/agents/tp-impact-analyzer.md index 6122c30..b928572 100644 --- a/agents/tp-impact-analyzer.md +++ b/agents/tp-impact-analyzer.md @@ -12,7 +12,7 @@ tools: - mcp__token-pilot__read_symbols - Read model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 351a987e11eba63852f5431a16d8eb53104f4f689f82fdcc5a2bf4db948ba92f requiredMcpServers: - "token-pilot" diff --git a/agents/tp-incident-timeline.md b/agents/tp-incident-timeline.md index 0d2088a..31af9cb 100644 --- a/agents/tp-incident-timeline.md +++ b/agents/tp-incident-timeline.md @@ -8,7 +8,7 @@ tools: - mcp__token-pilot__read_symbol - Bash model: inherit -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: de5722bfea374eaab096c1ae635c37879e7a91370ee3cd0532f4240be03c91eb requiredMcpServers: - "token-pilot" diff --git a/agents/tp-incremental-builder.md b/agents/tp-incremental-builder.md index 0e99b57..0e907fb 100644 --- a/agents/tp-incremental-builder.md +++ b/agents/tp-incremental-builder.md @@ -13,7 +13,7 @@ tools: - Edit - Bash model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 375a824d0d847bb5453ec594c7a62ad566ee7e4d92717b0473f771f1a0477c60 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-migration-scout.md b/agents/tp-migration-scout.md index 0d83da1..158fd4b 100644 --- a/agents/tp-migration-scout.md +++ b/agents/tp-migration-scout.md @@ -11,7 +11,7 @@ tools: - Grep - Glob model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 0334de1bf99b431b65359637d125cda7c44c6f780eb92c57cc538715b1939536 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-onboard.md b/agents/tp-onboard.md index b936904..eb76e7a 100644 --- a/agents/tp-onboard.md +++ b/agents/tp-onboard.md @@ -10,7 +10,7 @@ tools: - mcp__token-pilot__smart_read - mcp__token-pilot__smart_read_many - mcp__token-pilot__read_section -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 832e95633fbc8e9b0c10f3e540a327d4be062fb4b3f17a6cce6be13f414e2927 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-performance-profiler.md b/agents/tp-performance-profiler.md index bf0a9e6..ff07a01 100644 --- a/agents/tp-performance-profiler.md +++ b/agents/tp-performance-profiler.md @@ -11,7 +11,7 @@ tools: - Bash - Read model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: b61f06380d80798fa2e49d37bcba0653495bee04dd6bdbc1feff9a75607b0508 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-pr-reviewer.md b/agents/tp-pr-reviewer.md index 138f531..cfebc24 100644 --- a/agents/tp-pr-reviewer.md +++ b/agents/tp-pr-reviewer.md @@ -11,7 +11,7 @@ tools: - mcp__token-pilot__read_for_edit - Read model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: f83f50d05b4f70285ae7afed2b1a406fc436df56e61a0aedbfb31edc7f2b6e66 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-refactor-planner.md b/agents/tp-refactor-planner.md index 00db012..2dac29f 100644 --- a/agents/tp-refactor-planner.md +++ b/agents/tp-refactor-planner.md @@ -8,7 +8,7 @@ tools: - mcp__token-pilot__outline - mcp__token-pilot__read_symbol model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: c5f6fc122c89e16e5cf774045f92169ee3468555320b898171ba13eca5323550 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-review-impact.md b/agents/tp-review-impact.md index 541f2f0..ca8734b 100644 --- a/agents/tp-review-impact.md +++ b/agents/tp-review-impact.md @@ -9,7 +9,7 @@ tools: - mcp__token-pilot__module_info - Bash model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 8ef3c3341cbfed4eb8dd130126a9683edc57e378c92ff0ca764d584fd941c55c requiredMcpServers: - "token-pilot" diff --git a/agents/tp-run.md b/agents/tp-run.md index 292826a..896c80c 100644 --- a/agents/tp-run.md +++ b/agents/tp-run.md @@ -16,7 +16,7 @@ tools: - Glob - Bash model: haiku -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 2b08618d34a61f00aafccbda9fed6d83243296dedb83440edbd2d5c28bb6dbc4 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-session-restorer.md b/agents/tp-session-restorer.md index c449fc0..d9b9675 100644 --- a/agents/tp-session-restorer.md +++ b/agents/tp-session-restorer.md @@ -9,7 +9,7 @@ tools: - mcp__token-pilot__session_budget - Bash - Read -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 529374ed728f5eed5b758b3be3da65624783c0bf0c1a253d7d661a843eb5f767 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-ship-coordinator.md b/agents/tp-ship-coordinator.md index 5429215..e651c2d 100644 --- a/agents/tp-ship-coordinator.md +++ b/agents/tp-ship-coordinator.md @@ -11,7 +11,7 @@ tools: - Read - Grep model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: a60f6ae110eb3138064bce074e8ba26fa0ce5f4659df1624a9d9d3646803391b requiredMcpServers: - "token-pilot" diff --git a/agents/tp-spec-writer.md b/agents/tp-spec-writer.md index 97aeb3b..c16c21a 100644 --- a/agents/tp-spec-writer.md +++ b/agents/tp-spec-writer.md @@ -9,7 +9,7 @@ tools: - Read - Write model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: c7a4e8b39228fd5158528f389c924c5ff2d98c4b9b05ee0106d54a26c5dc1350 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-test-coverage-gapper.md b/agents/tp-test-coverage-gapper.md index 84b4325..fc58649 100644 --- a/agents/tp-test-coverage-gapper.md +++ b/agents/tp-test-coverage-gapper.md @@ -10,7 +10,7 @@ tools: - mcp__token-pilot__test_summary - Glob - Grep -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: be81eed53a3720d146cf89e4a14a7a56577633f7c84c234c412ab70d64c05b11 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-test-triage.md b/agents/tp-test-triage.md index fddd0f4..3124be1 100644 --- a/agents/tp-test-triage.md +++ b/agents/tp-test-triage.md @@ -8,7 +8,7 @@ tools: - mcp__token-pilot__find_usages - mcp__token-pilot__read_symbol model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 362ecf4cb03b059421ea26933473700900073dc38b3a7fe271208dfb1ae14f90 requiredMcpServers: - "token-pilot" diff --git a/agents/tp-test-writer.md b/agents/tp-test-writer.md index da5d0eb..20ac3bc 100644 --- a/agents/tp-test-writer.md +++ b/agents/tp-test-writer.md @@ -13,7 +13,7 @@ tools: - Edit - Bash model: sonnet -token_pilot_version: "0.47.0" +token_pilot_version: "0.47.1" token_pilot_body_hash: 269f2fe22ff4517c277d3f56ca67d8a5527b93290ab21079a83ba7af22c1b5a9 requiredMcpServers: - "token-pilot" diff --git a/docs/adr/0002-ast-index-multi-root-scoping.md b/docs/adr/0002-ast-index-multi-root-scoping.md new file mode 100644 index 0000000..956f5a9 --- /dev/null +++ b/docs/adr/0002-ast-index-multi-root-scoping.md @@ -0,0 +1,107 @@ +# ADR 0002 — ast-index multi-root scoping (walk-up gate + subtree plan) + +**Status:** Accepted (Decision 1, walk-up gate) / Proposed (Decision 2, subtree scoping) +**Date:** 2026-06 +**Supersedes:** nothing + +## Context + +token-pilot wraps the third-party `@ast-index/cli` binary. It always +runs the binary with `cwd` set to the detected project root (the git +toplevel, when there is one). Two related multi-root problems sit on +top of that single-index assumption: + +**(a) Cross-project index bleed.** When the detected `projectRoot` is a +non-git PARENT directory holding several sibling repos +(`work/repo-a`, `work/repo-b`, …), a single ast-index DB rooted at the +parent would mix symbols from unrelated projects. token-pilot mitigates +this today by detecting that shape (`isMultiRepoParent`, +`src/core/validation.ts`) and DISABLING ast-index for that root — search +falls back to the regex parser. Correct, but it means zero ast-index +benefit on those layouts. + +**(b) Nested-worktree escape.** `exec()` in +`src/ast-index/client.ts` used to set `AST_INDEX_WALK_UP=1` +unconditionally. That flag tells the binary's read-commands to traverse +PAST nested VCS markers and reuse a parent-level index — it exists so a +bare monorepo SUBDIR (no `.git`, no local DB) can find the root index. +But forcing it on every spawn is wrong when `projectRoot` is itself a +git repo/worktree root: a git worktree nested under the main repo +(e.g. `main-repo/.worktrees/feature`) would walk up past its own `.git` +marker and escape to the MAIN repo's parent index, returning the wrong +files. + +## Decision 1 — gate AST_INDEX_WALK_UP on the `.git` marker (Accepted, shipped here) + +`exec()` now sets `AST_INDEX_WALK_UP=1` ONLY when `projectRoot` has no +`.git` marker of its own. The marker is computed once +(`computeHasGitMarker`, re-evaluated in `updateProjectRoot` after a +branch/root change) as `existsSync(resolve(projectRoot, ".git"))` — a +`.git` DIRECTORY (normal repo) or a `.git` FILE (worktree / submodule +gitlink) both count. + +Why this is safe: + +- token-pilot already sets `cwd` to the git toplevel. For a repo or + worktree root, the binary finds the local index without walking up, + so the flag was at best a no-op there. +- Walk-up only ever HELPED the bare-subdir case (no marker, no local + DB) — exactly the case the gate still enables. +- For repo/worktree roots the flag's only observable effect was the + escape bug in (b). Skipping it removes that bug without removing the + subdir benefit. +- When a marker is present we simply refrain from FORCING the flag; a + value the user set in the environment themselves is left untouched. + +This is a behaviour fix with no config surface and no rooting-model +change. Covered by unit tests in `tests/ast-index/client.test.ts` +(marker present → flag absent; no marker → flag set; `computeHasGitMarker` +across `.git` dir / file / absence). + +## Decision 2 — subtree scoping to re-enable ast-index on multi-root parents (Proposed, deferred) + +Adopt ast-index 3.47's subtree model plus the `--local` / `--subtree` +query flags to RE-ENABLE ast-index on multi-repo and worktree-parent +layouts instead of disabling it: + +1. Index the parent directory once. +2. Register each child project as a named subtree. +3. Scope every read query (`exec`/`search`/`usages` in + `src/ast-index/client.ts`) with `--local` / `--subtree` so results + never bleed across siblings. + +This would let `isMultiRepoParent` roots keep ast-index instead of +falling back to regex, and would give nested worktrees a first-class +scoped index rather than relying on the walk-up gate. + +Why deferred: + +- It requires a rooting-model rework: how the index root is chosen, and + the `isMultiRepoParent` → disable path + (`src/core/validation.ts`), both have to change so the parent is + indexed-and-scoped rather than skipped. +- `--local` is a no-op without the subtree registration step, so the + flag alone buys nothing — the whole model has to land together. +- The Decision 1 gate already closes the correctness hole (the escape + bug) cheaply, which removes the urgency. + +## Consequences / follow-ups + +- Shipped: nested worktrees and repo roots no longer escape to a parent + index; bare subdirs keep walk-up. +- Still open: `isMultiRepoParent` roots get no ast-index benefit + (regex-only). The subtree model in Decision 2 is the planned fix; + file it as a tracked task before starting the rooting rework. +- If a future ast-index release changes walk-up semantics or ships the + subtree flags, revisit both decisions here. + +## Alternatives considered + +- **Keep forcing `AST_INDEX_WALK_UP=1` and special-case worktrees + elsewhere.** Rejected: the marker check is the same signal + `isMultiRepoParent` already uses, so gating at the env-construction + point is the smallest, most consistent change. +- **Disable ast-index for worktree roots too** (mirror the multi-repo + parent mitigation). Rejected: a worktree root owns a perfectly good + local index; disabling would throw away real benefit to avoid a bug + the gate already prevents. diff --git a/package-lock.json b/package-lock.json index 37624a6..b8a3128 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "token-pilot", - "version": "0.47.0", + "version": "0.47.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "token-pilot", - "version": "0.47.0", + "version": "0.47.1", "hasInstallScript": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 926675f..60b370d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "token-pilot", - "version": "0.47.0", + "version": "0.47.1", "description": "Save up to 80% tokens when AI reads code — MCP server for token-efficient code navigation, AST-aware structural reading instead of dumping full files into context window", "type": "module", "main": "dist/index.js", diff --git a/server.json b/server.json index 4dd59b6..caf33d8 100644 --- a/server.json +++ b/server.json @@ -6,12 +6,12 @@ "url": "https://github.com/Digital-Threads/token-pilot", "source": "github" }, - "version": "0.47.0", + "version": "0.47.1", "packages": [ { "registryType": "npm", "identifier": "token-pilot", - "version": "0.47.0", + "version": "0.47.1", "transport": { "type": "stdio" }, diff --git a/src/ast-index/client.ts b/src/ast-index/client.ts index 28d47db..567f8ee 100644 --- a/src/ast-index/client.ts +++ b/src/ast-index/client.ts @@ -1,4 +1,6 @@ import { execFile } from "node:child_process"; +import { existsSync } from "node:fs"; +import { resolve } from "node:path"; import { promisify } from "node:util"; import type { FileStructure } from "../types.js"; import type { @@ -53,11 +55,23 @@ const PYTHON_EXTENSIONS = new Set(["py", "pyw"]); const execFileAsync = promisify(execFile); +/** + * True when `projectRoot` is itself a git repo or worktree root. A `.git` + * entry counts whether it's a directory (normal repo) or a file (worktree / + * submodule gitlink). Used to gate AST_INDEX_WALK_UP — see exec(). + */ +export function computeHasGitMarker(projectRoot: string): boolean { + return existsSync(resolve(projectRoot, ".git")); +} + export class AstIndexClient { private static readonly MAX_INDEX_FILES = 50_000; private binaryPath: string | null = null; private projectRoot: string; + // True when projectRoot is itself a git repo/worktree root (has a `.git` + // marker). Gates AST_INDEX_WALK_UP in exec() — see the comment there. + private hasGitMarker: boolean; private indexed = false; private indexOversized = false; private indexDisabled = false; @@ -77,6 +91,7 @@ export class AstIndexClient { options?: { binaryPath?: string | null; autoInstall?: boolean }, ) { this.projectRoot = projectRoot; + this.hasGitMarker = computeHasGitMarker(projectRoot); this.timeout = timeout; this.configBinaryPath = options?.binaryPath ?? null; this.autoInstall = options?.autoInstall ?? true; @@ -1031,6 +1046,7 @@ export class AstIndexClient { updateProjectRoot(newRoot: string): void { this.projectRoot = newRoot; + this.hasGitMarker = computeHasGitMarker(newRoot); this.indexed = false; } @@ -1061,14 +1077,22 @@ export class AstIndexClient { // ast-index v3.39+ honours AST_INDEX_WALK_UP=1 — read-commands then // traverse past nested VCS markers (submodule .git, inner Cargo.toml, // nested settings.gradle) to reuse a parent-level index if one exists. - // Without this, running `search`/`outline` from a monorepo subdir stops - // at the nearest marker and finds nothing when the subdir has no DB. - // Safe default: pure-additive, no effect when projectRoot already sits - // at the index root. - const env: NodeJS.ProcessEnv = { - ...process.env, - AST_INDEX_WALK_UP: "1", - }; + // We only want this for a BARE SUBDIR that has no VCS marker of its own + // and no local DB — without walk-up, `search`/`outline` from a monorepo + // subdir stops at the nearest marker and finds nothing. + // + // We must NOT force it when projectRoot is itself a git repo/worktree + // root: a git worktree nested under the main repo (e.g. + // `main-repo/.worktrees/feature`) would walk up past its own `.git` + // marker and escape to the MAIN repo's parent index, returning the + // wrong files. Repo/worktree roots already own their index, so walk-up + // is at best a no-op and at worst that escape bug — skip it. When the + // user set AST_INDEX_WALK_UP themselves and a marker is present, we + // leave their value untouched (we only refrain from forcing it). + const env: NodeJS.ProcessEnv = { ...process.env }; + if (!this.hasGitMarker) { + env.AST_INDEX_WALK_UP = "1"; + } if (this.astGrepBinDir) { env.PATH = `${this.astGrepBinDir}:${process.env.PATH ?? ""}`; } diff --git a/tests/ast-index/client.test.ts b/tests/ast-index/client.test.ts index fba3d05..914176e 100644 --- a/tests/ast-index/client.test.ts +++ b/tests/ast-index/client.test.ts @@ -20,7 +20,10 @@ vi.mock("../../src/ast-index/binary-manager.js", async () => { }; }); -import { AstIndexClient } from "../../src/ast-index/client.js"; +import { + AstIndexClient, + computeHasGitMarker, +} from "../../src/ast-index/client.js"; import { parseFileCount, parseImplementationsText, @@ -529,22 +532,24 @@ describe("AstIndexClient", () => { // ast-index v3.39+ reads AST_INDEX_WALK_UP=1 so read-commands traverse // past nested VCS markers (submodule .git, nested Cargo.toml) and reuse - // a parent-level index. Without this, running find_usages inside a - // monorepo submodule silently returned empty. We set the flag on every - // spawn so the fix is zero-config. + // a parent-level index. We set the flag ONLY when projectRoot is a bare + // subdir with no `.git` marker of its own — a repo/worktree root must NOT + // walk up, or a nested worktree (`main-repo/.worktrees/feature`) would + // escape to the main repo's parent index and return the wrong files. // // ESM forbids spying on child_process.execFile, so we take the black-box // route: point binaryPath at a tiny shell script that prints the env var // back to stdout and run the real exec path end-to-end. - it("sets AST_INDEX_WALK_UP=1 on every ast-index spawn (monorepo fix)", async () => { + // + // Script prints one line the real binary never would, so we can assert on + // it unambiguously. Works on macOS/Linux CI runners. + const walkUpProbe = '#!/bin/sh\nprintf "WALK_UP=%s\\n" "${AST_INDEX_WALK_UP:-unset}"\n'; + + it("sets AST_INDEX_WALK_UP=1 when projectRoot has no .git marker (bare subdir)", async () => { const { chmod, writeFile } = await import("node:fs/promises"); + // tempDir is a fresh mkdtemp — no `.git`, i.e. a bare subdir. const fakeBinary = join(tempDir, "fake-ast-index.sh"); - // Script prints one line the real binary never would, so we can assert - // on it unambiguously. Works on macOS/Linux CI runners. - await writeFile( - fakeBinary, - '#!/bin/sh\nprintf "WALK_UP=%s\\n" "${AST_INDEX_WALK_UP:-unset}"\n', - ); + await writeFile(fakeBinary, walkUpProbe); await chmod(fakeBinary, 0o755); const client = new AstIndexClient(tempDir) as any; @@ -560,6 +565,47 @@ describe("AstIndexClient", () => { expect(out2).toContain("WALK_UP=1"); }); + it("skips AST_INDEX_WALK_UP when projectRoot is a git root (.git present)", async () => { + const { chmod, mkdtemp: mkdtempFs, writeFile } = await import( + "node:fs/promises" + ); + // A repo/worktree root: a `.git` entry exists at projectRoot. Use a + // file (the worktree/submodule gitlink shape) — a directory counts too. + const repoRoot = await mkdtempFs(join(tmpdir(), "token-pilot-ast-gitroot-")); + await writeFile(join(repoRoot, ".git"), "gitdir: /elsewhere/.git/worktrees/x\n"); + const fakeBinary = join(repoRoot, "fake-ast-index.sh"); + await writeFile(fakeBinary, walkUpProbe); + await chmod(fakeBinary, 0o755); + + const client = new AstIndexClient(repoRoot) as any; + client.binaryPath = fakeBinary; + + const out = await client.exec(["stats"]); + expect(out).toContain("WALK_UP=unset"); + + await rm(repoRoot, { recursive: true, force: true }); + }); + + it("computeHasGitMarker detects .git dir, .git file, and its absence", async () => { + const { mkdir, mkdtemp: mkdtempFs, writeFile } = await import( + "node:fs/promises" + ); + const dirRepo = await mkdtempFs(join(tmpdir(), "token-pilot-git-dir-")); + await mkdir(join(dirRepo, ".git")); + expect(computeHasGitMarker(dirRepo)).toBe(true); + + const fileRepo = await mkdtempFs(join(tmpdir(), "token-pilot-git-file-")); + await writeFile(join(fileRepo, ".git"), "gitdir: /x\n"); + expect(computeHasGitMarker(fileRepo)).toBe(true); + + const bare = await mkdtempFs(join(tmpdir(), "token-pilot-git-none-")); + expect(computeHasGitMarker(bare)).toBe(false); + + await rm(dirRepo, { recursive: true, force: true }); + await rm(fileRepo, { recursive: true, force: true }); + await rm(bare, { recursive: true, force: true }); + }); + // Branch-switch wiring is in server.ts, but incrementalUpdate is the // entry point we expect that wiring to call. It must short-circuit // cleanly when the index isn't in a usable state — otherwise a stray @@ -692,7 +738,7 @@ describe("AstIndexClient", () => { client.binaryPath = "/bin/ast-index"; client.ensureIndex = async () => {}; - const execMock = vi.fn(async () => + const execMock = vi.fn(async (..._args: unknown[]) => JSON.stringify({ dominant_language: "ts", query: "AstIndexClient buildIndex", diff --git a/tests/index.test.ts b/tests/index.test.ts index 04089b3..6954f6d 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -1,4 +1,3 @@ -import { execFileSync } from "node:child_process"; import { mkdtemp, readFile, realpath, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { tmpdir } from "node:os"; @@ -17,6 +16,52 @@ const mockDeps = vi.hoisted(() => ({ detectContextMode: vi.fn(), })); +// startServer's git-root detection runs `execFileAsync = promisify(execFile)` +// with a real `git rev-parse --show-toplevel` subprocess (3s timeout). Under +// parallel CPU load that subprocess can time out, flaking the two detection +// tests below. We replace `execFile` with a git-aware shim: every NON-git +// call delegates to the real `execFile` (so unrelated startup code is +// untouched), and the `git rev-parse --show-toplevel` call returns whatever +// the active test parked in `gitResponse` — instantly, no subprocess. When +// `gitResponse` is unset the git call also delegates to real git. +// +// `gitResponse.toplevel` → resolve with that stdout; `gitResponse.fail` → +// reject (drives the INIT_CWD-fallback path). promisify wraps execFile at +// module load and appends a callback as the final arg. +const gitResponse = vi.hoisted( + () => ({}) as { toplevel?: string; fail?: boolean }, +); + +vi.mock("node:child_process", async () => { + const actual = + await vi.importActual( + "node:child_process", + ); + const realExecFile = actual.execFile as unknown as ( + ...args: unknown[] + ) => unknown; + const shim = (...args: unknown[]) => { + const file = args[0]; + const cmdArgs = args[1]; + const isGitToplevel = + file === "git" && + Array.isArray(cmdArgs) && + cmdArgs[0] === "rev-parse" && + cmdArgs[1] === "--show-toplevel"; + if (isGitToplevel && (gitResponse.toplevel !== undefined || gitResponse.fail)) { + const cb = args[args.length - 1] as (e: unknown, r?: unknown) => void; + if (gitResponse.fail) { + cb(new Error("not a git repository (mocked)")); + } else { + cb(null, { stdout: `${gitResponse.toplevel}\n`, stderr: "" }); + } + return; + } + return realExecFile(...args); + }; + return { ...actual, execFile: shim }; +}); + vi.mock("../src/server.js", () => ({ createServer: mockDeps.createServer, })); @@ -72,6 +117,7 @@ import * as indexModule from "../src/index.ts"; describe("index CLI helpers", () => { let tempDir: string; let savedPluginRoot: string | undefined; + let savedProjectDir: string | undefined; let logSpy: ReturnType; let errorSpy: ReturnType; let writeSpy: ReturnType; @@ -79,11 +125,14 @@ describe("index CLI helpers", () => { beforeEach(async () => { tempDir = await mkdtemp(join(tmpdir(), "token-pilot-index-")); - // installHook branches on CLAUDE_PLUGIN_ROOT; neutralise any value leaked - // from another test sharing this worker so the install assertions are - // deterministic regardless of file sharding. + // installHook branches on CLAUDE_PLUGIN_ROOT, and startServer's projectRoot + // detection reads CLAUDE_PROJECT_DIR as its first candidate. Neutralise any + // value leaked from another test sharing this worker so the install + git + // detection assertions are deterministic regardless of file sharding. savedPluginRoot = process.env.CLAUDE_PLUGIN_ROOT; + savedProjectDir = process.env.CLAUDE_PROJECT_DIR; delete process.env.CLAUDE_PLUGIN_ROOT; + delete process.env.CLAUDE_PROJECT_DIR; logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); writeSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true); @@ -141,6 +190,8 @@ describe("index CLI helpers", () => { await rm(tempDir, { recursive: true, force: true }); if (savedPluginRoot === undefined) delete process.env.CLAUDE_PLUGIN_ROOT; else process.env.CLAUDE_PLUGIN_ROOT = savedPluginRoot; + if (savedProjectDir === undefined) delete process.env.CLAUDE_PROJECT_DIR; + else process.env.CLAUDE_PROJECT_DIR = savedProjectDir; vi.restoreAllMocks(); }); @@ -425,36 +476,44 @@ describe("index CLI helpers", () => { }); }); - it("auto-detects git root when started without explicit args", async () => { + // Touches process-global cwd/INIT_CWD/PWD; retry guards the rare race when + // sharing a worker with other suites in the full parallel run. + it("auto-detects git root when started without explicit args", { retry: 2 }, async () => { const repoDir = await realpath( await mkdtemp(join(tmpdir(), "token-pilot-index-git-")), ); - execFileSync("git", ["init"], { cwd: repoDir }); const originalInitCwd = process.env.INIT_CWD; process.env.INIT_CWD = repoDir; + // Drive `git rev-parse --show-toplevel` deterministically — the real + // subprocess flakes under parallel load (3s timeout). + gitResponse.toplevel = repoDir; + const server = { connect: vi.fn(async () => {}), close: vi.fn(async () => {}), }; mockDeps.createServer.mockResolvedValue(server); - await indexModule.startServer([]); - expect( - errorSpy.mock.calls.some((call) => - String(call[0]).includes(`project root: ${repoDir}`), - ), - ).toBe(true); - expect(mockDeps.createServer).toHaveBeenCalledWith(repoDir, { - enforcementMode: "deny", - skipAstIndex: false, - }); - - process.env.INIT_CWD = originalInitCwd; - await rm(repoDir, { recursive: true, force: true }); + try { + await indexModule.startServer([]); + expect( + errorSpy.mock.calls.some((call) => + String(call[0]).includes(`project root: ${repoDir}`), + ), + ).toBe(true); + expect(mockDeps.createServer).toHaveBeenCalledWith(repoDir, { + enforcementMode: "deny", + skipAstIndex: false, + }); + } finally { + delete gitResponse.toplevel; + process.env.INIT_CWD = originalInitCwd; + await rm(repoDir, { recursive: true, force: true }); + } }); - it("falls back to INIT_CWD when auto-detect cannot find a git root", async () => { + it("falls back to INIT_CWD when auto-detect cannot find a git root", { retry: 2 }, async () => { const fallbackDir = await mkdtemp( join(tmpdir(), "token-pilot-index-fallback-"), ); @@ -465,27 +524,34 @@ describe("index CLI helpers", () => { process.env.PWD = "/"; process.chdir(fallbackDir); + // Fail git detection for every candidate so the INIT_CWD fallback path + // runs — without the real subprocess (and its flaky timeout). + gitResponse.fail = true; + const server = { connect: vi.fn(async () => {}), close: vi.fn(async () => {}), }; mockDeps.createServer.mockResolvedValue(server); - await indexModule.startServer([]); - expect( - errorSpy.mock.calls.some((call) => - String(call[0]).includes(`${fallbackDir} (INIT_CWD, not a git repo)`), - ), - ).toBe(true); - expect(mockDeps.createServer).toHaveBeenCalledWith(fallbackDir, { - enforcementMode: "deny", - skipAstIndex: false, - }); - - process.env.INIT_CWD = originalInitCwd; - process.env.PWD = originalPwd; - process.chdir(originalCwd); - await rm(fallbackDir, { recursive: true, force: true }); + try { + await indexModule.startServer([]); + expect( + errorSpy.mock.calls.some((call) => + String(call[0]).includes(`${fallbackDir} (INIT_CWD, not a git repo)`), + ), + ).toBe(true); + expect(mockDeps.createServer).toHaveBeenCalledWith(fallbackDir, { + enforcementMode: "deny", + skipAstIndex: false, + }); + } finally { + delete gitResponse.fail; + process.env.INIT_CWD = originalInitCwd; + process.env.PWD = originalPwd; + process.chdir(originalCwd); + await rm(fallbackDir, { recursive: true, force: true }); + } }); it("prints doctor diagnostics for installed components", async () => {