Skip to content

revert(workflow): remove codegraph ast-grep mcp integration - #909

Merged
lavaman131 merged 1 commit into
mainfrom
revert/codegraph-ast-grep-mcp
May 10, 2026
Merged

revert(workflow): remove codegraph ast-grep mcp integration#909
lavaman131 merged 1 commit into
mainfrom
revert/codegraph-ast-grep-mcp

Conversation

@lavaman131

@lavaman131 lavaman131 commented May 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Reverts the CodeGraph/ast-grep MCP integration across all bundled agent configurations and the `deep-research-codebase` workflow, restoring the pre-CodeGraph implementation based on deterministic TypeScript-based scout/scratch synthesis and batched specialist-agent fan-out.

Changes

Agent Configurations

  • Removed CodeGraph and ast-grep MCP server entries from all 6 bundled codebase-* agents (.claude/agents/, .github/agents/, .opencode/agents/)
  • Removed CodeGraph exploration instructions and ast-grep rule development guidance from all agent prompts
  • Deleted all .opencode/agents/ files and CodeGraph/ast-grep entries from .opencode/opencode.json

deep-research-codebase Workflow

  • Removed CodeGraph orchestration helpers: codegraph-contract, codegraph-probe, file-discovery, orchestration, preflight (with all associated tests)
  • Restored claude/index.ts, copilot/index.ts, and opencode/index.ts provider flows without CodeGraph preflight/orchestration steps
  • Simplified scout.ts and scratch.ts back to direct TypeScript-based synthesis
  • Removed generated snippets file and snippet-generation infrastructure

Dependencies & Tooling

  • Dropped @colbymchenry/codegraph dependency from packages/atomic-sdk
  • Removed uv/uvx installer scaffolding from spawn.ts
  • Removed lint:mcp, lint:file-discovery, and build:snippets scripts (and all supporting implementations/tests) from atomic-sdk
  • Removed bunfig.toml CodeGraph-related configuration

CI & Docs

  • Removed uv-install CI regression test step and tests/ci/uv-install.test.ts
  • Removed tests/ci/snippet-drift.test.ts
  • Removed CodeGraph migration doc and docs/agent-snippets/codegraph.md + ast-grep-rule-development.md

Validation

  • bun typecheck — passes
  • AGENT=1 bun test --parallel packages tests — 2391 pass, 3 skip, 0 fail

Restore deep-research-codebase and bundled codebase agents to the pre-CodeGraph/ast-grep MCP implementation. Remove CodeGraph package installation, uv auto-installation, and related snippet/lint/CI scaffolding.

Assistant-model: ChatGPT
@lavaman131
lavaman131 merged commit eb96c8b into main May 10, 2026
24 checks passed
@lavaman131
lavaman131 deleted the revert/codegraph-ast-grep-mcp branch May 10, 2026 08:29
@claude

claude Bot commented May 10, 2026

Copy link
Copy Markdown

Code Review — revert/codegraph-ast-grep-mcp

Reviewed against CLAUDE.md conventions (Bun-only, no any/unknown, modular, TDD). The mechanical revert is clean and the agent-config / dep / CI removals all match. Specific feedback below.

Code quality

  • Triplicated wave-assembly logic. claude/index.ts, copilot/index.ts, and opencode/index.ts now each duplicate safeReadFile, logBatchRejections, the partitionPaths builder, the wave1Tasks / wave2Tasks flatMaps, the locatorOutputs map fill, and the explorerHandles synthesis loop. The pre-codegraph code presumably had the same shape, but consider keeping a thin helpers/orchestration.ts that exports just these provider-agnostic pieces (buildPartitionPaths, buildLayer1Tasks, buildLayer2Tasks, safeReadFile, logBatchRejections, synthesizeExplorerHandles) — without the codegraph coupling. The revert removed the bad parts of that helper but also threw away the legitimate deduplication. Any future tweak to wave shape now has to be applied to three files in lock-step.
  • scout.ts:14 vs scout.ts:17. The file-level docstring says "pure TypeScript + child_process" but line 17 is a stray bare comment saying you actually use Bun.spawnSync. Fold that into the docstring rather than leaving the contradicting comment + correction one line apart.
  • scout.ts:217 isCodeFile was hoisted above listAllFiles / below the original docstring — minor, but the visual flow now is: walker → FileStats type → getCodebaseRootisCodeFilelistAllFiles. Co-locating isCodeFile next to listAllFiles (its only caller) would read more naturally.
  • prompts.ts lost the trimOr helper. Five call sites now repeat opts.x.trim().length > 0 ? opts.x.trim() : \"…\". The pre-revert code used a local trimOr(text, fallback) helper. Worth restoring — it's pure, tested implicitly by every prompt call, and makes the builders shorter and consistent.
  • renderArchitecturalOrientation helper removed too. Same tradeoff: three specialist prompts now inline the same 7-line <ARCHITECTURAL_ORIENTATION> block. A small private helper kept these in sync.

Potential bugs

  • scout.ts:listAllFiles has no stdout-size guard. Bun.spawnSync with stdout: \"pipe\" will buffer the entire git ls-files / rg --files output. On very large monorepos this can OOM the process. The previous file-discovery.ts may have streamed; worth confirming. If you're keeping the pipe buffer, add a comment acknowledging the cap.
  • walkWithIgnore posixPath.relative(scope.basePath, posix) for directory probes. When entry.isDirectory() is true you append \"/\" to the result, but posixPath.relative may return an empty string when scope.basePath === posix (i.e., the directory is the scope itself). That becomes \"/\" and ignore.ignores(\"/\") is undefined behavior in the ignore package. Edge case but worth a guarded test.
  • No fallback when wc -l is missing on Windows. scout.ts:countLines does have an in-process fallback (good), but it only triggers on Bun.spawnSync throw. On Windows where wc isn't installed, Bun.spawnSync returns success: false with empty stdout rather than throwing — wcOk stays false and the fallback runs, so this is fine. Just confirm in a unit test.

Performance

  • The new walkWithIgnore builds a fresh inherited-scopes array on every directory descent ([...inheritedScopes, { basePath, matcher: here }]). For deeply nested projects this is O(depth²) array allocation. Replace with a linked-list / index-based scope chain if it ever shows up in profiles — fine to defer.

Security

  • Net positive: drops the curl https://astral.sh/uv/install.sh | sh install path and the uvx ... git+https://github.com/ast-grep/ast-grep-mcp clone-and-execute path from agent configs. The GIT_CONFIG_GLOBAL=/dev/null neutralization is no longer needed because the codegen path it protected is gone. ✓

Test coverage

  • heuristic.test.ts deletion lost still-applicable coverage. The deleted file tested calculateExplorerCount for loc=0/-1/NaN/1000/10_000/25_000/100_000/15_000. Even after dropping the codegraphHealthy branches, the no-opts cases (expect(calculateExplorerCount(0)).toBe(2), 100_000 → 20, etc.) still apply and exercise the Math.max(2, …) and ceil rounding. Restore the no-opts cases as a small heuristic.test.ts.
  • scout.test.ts had no coverage for the new walkWithIgnore. The deleted file tested codegraph-specific listSourceFiles, so deletion is fine — but walkWithIgnore, listAllFiles, and the new in-process LOC fallback in countLines are net-new code with zero tests. A small fixture-based test (tmpdir + a few files + nested .gitignore) would protect against regressions in the fallback path that's hardest to hit on CI.
  • prompts.test.ts deletion is fine (its assertions targeted the Callers/Impact codegraph wording that no longer exists), but consider a single regression test that buildScoutPrompt / buildLocatorPrompt / etc. embed the new AST_GREP_ENV_NOTICE string. Otherwise a future refactor can silently drop the tooling notice.
  • spawn.test.ts lost hasUv PATH-cache regression test. Unavoidable since hasUv is gone, but the underlying Bun.which PATH-caching gotcha that test guarded against still applies to other callers (runtime/tmux.ts, providers/claude.ts per the deleted comment). Make sure another test in the suite still covers resolveCommandFromCurrentPath for the same gotcha.

CLAUDE.md compliance

  • All bun / no-npm rules respected. ✓
  • No any / unknown in new code. ✓
  • bun typecheck and AGENT=1 bun test reportedly green per PR description.

Nits

  • auto-sync.ts:14-22 numbered list — fine, matches the new step ordering.
  • package.json lint script line removed lint:mcp and lint:file-discovery, both correct (the scripts are deleted).
  • bunfig.toml coverage-ignore entries for the deleted scripts are gone — correct.

Overall: the revert is mechanically correct and removes a lot of code that didn't earn its keep. The main asks are (1) keep a slim orchestration.ts so the three provider workflows don't drift, (2) restore the trimOr / renderArchitecturalOrientation micro-helpers in prompts.ts, and (3) add back the no-opts calculateExplorerCount tests + a tiny walkWithIgnore fixture test.

flora131 pushed a commit that referenced this pull request May 10, 2026
origin/main reverted the codegraph ast-grep mcp integration (#909) which
removed hasUv/prependUvInstallPaths/refreshWindowsUvPath/uvInstallPathCandidates
from spawn.ts, but the refactor still referenced ensureUvInstalled. Remove
the now-orphaned ensureUvInstalled function and its callers in auto-sync.
flora131 added a commit that referenced this pull request May 10, 2026
origin/main reverted the codegraph ast-grep mcp integration (#909) which
removed hasUv/prependUvInstallPaths/refreshWindowsUvPath/uvInstallPathCandidates
from spawn.ts, but the refactor still referenced ensureUvInstalled. Remove
the now-orphaned ensureUvInstalled function and its callers in auto-sync.
flora131 added a commit that referenced this pull request May 10, 2026
origin/main reverted the codegraph ast-grep mcp integration (#909) which
removed hasUv/prependUvInstallPaths/refreshWindowsUvPath/uvInstallPathCandidates
from spawn.ts, but the refactor still referenced ensureUvInstalled. Remove
the now-orphaned ensureUvInstalled function and its callers in auto-sync.
flora131 added a commit that referenced this pull request May 10, 2026
origin/main reverted the codegraph ast-grep mcp integration (#909) which
removed hasUv/prependUvInstallPaths/refreshWindowsUvPath/uvInstallPathCandidates
from spawn.ts, but the refactor still referenced ensureUvInstalled. Remove
the now-orphaned ensureUvInstalled function and its callers in auto-sync.
lavaman131 added a commit that referenced this pull request Jun 29, 2026
Restore deep-research-codebase and bundled codebase agents to the pre-CodeGraph/ast-grep MCP implementation. Remove CodeGraph package installation, uv auto-installation, and related snippet/lint/CI scaffolding.

Assistant-model: ChatGPT

Co-authored-by: Flora <nlavaee@umich.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants