feat(mcp): surface project root in codegraph_status + actionable errors#126
Closed
andreinknv wants to merge 2 commits into
Closed
feat(mcp): surface project root in codegraph_status + actionable errors#126andreinknv wants to merge 2 commits into
andreinknv wants to merge 2 commits into
Conversation
…flicts
Today every PR adding an MCP tool conflicts on the same two
shared lists in src/mcp/tools.ts: the tools[] array (the
list_tools surface) and the case switch in execute(). After this
refactor:
Adding a new MCP tool:
1. Drop a file at src/mcp/tools/<name>.ts exporting a
<NAME>_TOOL: ToolModule (definition + handlerKey).
2. Add one import line and one array entry to
src/mcp/tools/registry.ts.
3. Implement handle<Name>(args) on ToolHandler in tools.ts and
add the new key to HandlerKey in tools/types.ts.
Step 3 is the only remaining "shared method on a single class"
conflict surface. Extracting handler bodies into per-tool files
(making step 3 also a single-file addition) is left as a
follow-up — the cost/benefit favors landing this incremental win
now and finishing the body extraction once language and migration
refactors land.
## What's new
- **src/mcp/tool-types.ts** — extracted ToolDefinition, ToolResult,
PropertySchema, projectPathProperty into a shared module so
per-tool files can import without circular dependency.
- **src/mcp/tools/types.ts** — ToolModule interface, HandlerKey
string union, and ToolHandlerLike (a structural type that
ToolHandler now `implements`, providing compile-time guarantee
that every HandlerKey maps to a real method).
- **src/mcp/tools/<name>.ts × 9** — one file per existing tool
(callees, callers, context, explore, files, impact, node, search,
status). Each ~25-30 lines: import + definition literal +
handlerKey reference.
- **src/mcp/tools/registry.ts** — static-import barrel, sorted
alphabetically. Exports getToolModules(), getToolModule(name),
and the derived `tools[]` array.
- **src/mcp/tools.ts** — ~200 lines deleted from the top
(inline types + tools[] array + projectPathProperty).
execute()'s case-switch replaced with a registry lookup +
type-safe `this[mod.handlerKey](args)` dispatch (now compile-
time-checked thanks to `implements ToolHandlerLike`).
All `private async handle*` methods now public to match the
interface. errorResult/textResult also public for the same reason.
- **src/mcp/index.ts** — MCPServer's tool-existence check switched
from a linear `tools.find()` scan to the O(1) `getToolModule()`
Map lookup, eliminating two parallel lookup paths.
## Tests
387/387 pass. **7 new tests** in __tests__/mcp-tool-registry.test.ts:
- Definitions are well-formed (name shape, description length).
- handlerKey shape (`handle<UpperCase>`).
- Every registered handlerKey resolves to a real method on
ToolHandler.
- Exported `tools[]` exactly mirrors the registry.
- Canonical 9 main-line tools regression guard.
- execute() unknown-tool error path.
- **End-to-end dispatch smoke test**: execute('codegraph_status', {})
reaches the real handler body (no broken `this` binding) — would
fail loudly if the dynamic dispatch chain ever breaks.
## Reviewer pass
Independent reviewer ran once. 2 REQUEST_CHANGES + 2 INFO addressed:
1. ToolHandlerLike was defined but never enforced —
ToolHandler now `implements ToolHandlerLike`. Eliminates the
`(this as unknown as Record<...>)` cast in execute(); dispatch
is fully compile-time-checked.
2. No end-to-end dispatch test — added one (see Tests above).
3. MCPServer.handleToolsCall used a linear `tools.find()` scan
while execute() used Map lookup — switched to getToolModule()
for parity.
4. Removed redundant .slice() in registry.ts (map() already
returns a fresh array).
## Backward compat
src/mcp/tools.ts still re-exports ToolDefinition, ToolResult, the
mutable `tools[]` array, ToolHandler, and getExploreBudget. Every
existing consumer (`import { ToolDefinition, ToolResult, tools,
ToolHandler } from './tools'`) keeps working unchanged.
## Affected open PRs
- colbymchenry#110 (review-context): rebases to 1 new file in tools/ + 2
lines in registry.ts + 1 method on ToolHandler + 1 line in
HandlerKey.
- colbymchenry#112 (centrality+churn): same shape for the codegraph_hotspots
tool.
- colbymchenry#114 (config-refs): same shape for codegraph_config.
- colbymchenry#115 (sql-refs): same shape for codegraph_sql.
Each goes from 4-way conflict (tools[] + case + handler + helpers)
down to 1-way conflict (HandlerKey + handler method on ToolHandler,
both in tools.ts).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Merge guide: see #120 for the full backlog merge order. Short version for this PR: ✅ already in post-refactor shape (sits on top of the per-tool registry from #117). Stacked on #117 — until that lands, this PR's diff transitively includes the refactor commit. Independent reviewer pass caught and fixed three issues: unreachable sourceLabel branch, SQLite-handle leak in tests, macOS/Windows case-FS edge case. Live smoke test confirmed the four user-facing strings render correctly. Pairs naturally with #121 (MCP playbook): playbook tells the agent which tool to call; this PR makes status's first line answer which project it's about. |
Friction point this addresses: when an MCP server's default project
(the directory it was launched from) doesn't match what the agent
wants to query, every tool call has to pass `projectPath` -- but the
agent has no way of knowing whether to do that, because no tool
surfaces the default. The agent guesses, gets a "not initialized"
error, then guesses again. The error message even pointed at the
wrong remediation (`codegraph init` rather than `pass projectPath`).
Changes:
- `handleStatus` now starts with `**Project root:** <path>` plus a
label: `(default -- server CWD at startup)` or `(from
\`projectPath\` argument)`. The agent's first call to status
answers "is the default what I want?" with one line.
- `handleStatus` lists "Other projects this server has open" --
anything in `projectCache` plus the default if it differs from
the queried one. Helpful for monorepo / adjacent-repo workflows.
- `getCodeGraph()` errors are rewritten:
No default + no projectPath -> "No default codegraph project
for this MCP server. Either: (a) restart the MCP server
from a directory containing .codegraph/, (b) run `codegraph
init` in the current directory, or (c) pass `projectPath`
pointing to a directory that already has .codegraph/."
projectPath supplied but no .codegraph/ found -> "No
.codegraph/ found at or above {path}. Run `codegraph init`
in that project first, or pass a different projectPath."
- `STATUS_TOOL` description rewritten so agents call status FIRST
when the default project is unknown.
Independent reviewer pass caught and fixed three issues before
shipping: (1) the third "(resolved from cache)" sourceLabel branch
was unreachable -- collapsed to a binary ternary; (2) the test
suite leaked SQLite handles by not calling `handler.closeAll()` --
fixed via afterEach + idempotent close handling; (3) added a comment
documenting the macOS/Windows case-insensitive-FS edge case where
strict-string project-root equality could lie.
Tested:
- 5 unit tests in __tests__/mcp-status.test.ts covering both
sourceLabel branches, the multi-project list, and both
actionable-error paths.
- 799/799 full suite passes.
- Live smoke test against codegraph's own indexed tree confirmed
all four user-facing strings render correctly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
andreinknv
force-pushed
the
feat/mcp-default-project-hint
branch
from
f4620ed to
c1cbb01
Compare
Owner
|
Closing — the intrinsic change here (status surface + error wording) is reasonable, but it's stacked on #117's MCP tool-registry refactor and can't be reviewed in isolation. The trunk decision lives in #117; if I accept that refactor, this is welcome to come back as a tiny PR on top of the new structure. Looking at #117 next. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes a real ergonomics gap I hit running the MCP server in an agentic flow: when the server's default project doesn't match the codebase the agent wants to query, every tool call has to pass
projectPath, but the agent has no way of knowing whether to do that — no tool surfaces the default. The agent guesses, gets backCodeGraph not initialized, guesses again. The error even pointed at the wrong remediation (codegraph initinstead ofpass projectPath).What changes
codegraph_statusnow leads with the project identity:The label is
(default — server CWD at startup)or(from \projectPath` argument). One look atstatus` answers "do I need to start passing projectPath?" — which previously required an unrelated tool call to fail first.Errors are rewritten with concrete next steps:
projectPathCodeGraph not initialized for this project. Run 'codegraph init' first.codegraph init/ passprojectPathprojectPathsupplied, no.codegraph/foundCodeGraph not initialized in {path}. Run 'codegraph init' in that project first.No .codegraph/ found at or above {path}. Run \codegraph init` in that project first, or pass a different projectPath.`STATUS_TOOLdescription nudges agents to call status FIRST in unfamiliar sessions, so the project-identity check is the natural opening move.Independent reviewer pass
Caught and fixed three issues before opening:
'(resolved from cache)'sourceLabel branch was unreachable —getCodeGraph(undefined)always returnsthis.cgor throws, so whenargs.projectPathis undefined the project IS the default. Collapsed to a binary ternary with a comment explaining why.handler.closeAll()was never called, leaving cached project connections open across tests. Fixed viaafterEachcleanup + idempotent double-close handling.path.resolvedoesn't normalise case, so a client-suppliedrootUriwith different capitalisation thanprocess.cwd()could compare unequal under strict-string equality. Documented as a comment; deferred arealpath-based fix until someone actually hits it.Test plan
npx tsc --noEmitcleannpx vitest run— 799/799 tests pass__tests__/mcp-status.test.ts(5 cases):projectPath-argument labelprojectPath.codegraph/is missing there__tests__/mcp-tool-registry.test.tsregex for the new error wordingFiles changed
src/mcp/tools.tshandleStatusrewrite +getCodeGrapherror rewritesrc/mcp/tools/status.ts__tests__/mcp-status.test.ts__tests__/mcp-tool-registry.test.tsPairs naturally with #121
The MCP-instructions playbook from #121 tells the agent which tool to call for each intent. This PR makes sure the answer to "is the project root what I expect?" is always one tool call away and self-explanatory — exactly the kind of "tier-1 first" check the playbook recommends.
🤖 Generated with Claude Code