feat: expose individual graph types as separate MCP tools #84
Conversation
Adds four new MCP tools for targeted graph generation: - get_call_graph: Function-level call relationships - get_dependency_graph: Module import relationships - get_domain_graph: High-level domain classification - get_parse_graph: AST-level code structure Each tool calls its corresponding API endpoint, allowing agents to request only the graph type relevant to their task. This provides: - Faster responses (smaller graphs) - Lower token costs (focused data) - Better tool discoverability Also fixes build issue where SupermodelClient was referenced but doesn't exist in the SDK - now uses DefaultApi directly. Closes #81
WalkthroughThis PR adds four new graph-analysis MCP tools, centralizes API helper utilities, updates server tool registration/dispatch to use a dynamic tool map with duplicate-name checks, adds integration and unit tests, and expands README documentation and CLI entrypoint handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as MCP Client
participant Server as MCP Server
participant FS as Filesystem
participant API as Supermodel API
Client->>Server: tools/call (toolName="get_call_graph", directory, jq_filter?)
Server->>Server: lookup toolMap[toolName] -> handler
Server->>Server: validate inputs (directory, jq_filter)
Server->>FS: zip target directory -> /tmp/repo.zip
FS-->>Server: zipPath
Server->>API: POST /v1/graphs/call (zip blob + idempotencyKey)
API-->>Server: graph JSON result or error
Server->>Server: maybeFilter(jq_filter, result)
Server-->>Client: result or structured error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/tools/graph-tools.ts`:
- Around line 255-279: The handler currently treats non-string directory values
(e.g., directory: 123) as missing; update the validation in the handler function
to explicitly check the type of providedDirectory before falling back to
defaultWorkdir and return an asErrorResult with type 'validation_error' (and a
clear message like "Invalid 'directory' parameter: expected a string") when
providedDirectory exists but is not a string; ensure you reference the existing
variables providedDirectory, directory, args and reuse the existing error shape
(code 'MISSING_DIRECTORY' or a new code like 'INVALID_DIRECTORY') so callers get
a direct, descriptive validation error instead of "missing directory."
🧹 Nitpick comments (1)
src/tools/graph-tools.ts (1)
106-216: Consider extracting shared graph helper utilities to avoid drift
generateIdempotencyKey,formatBytes, andclassifyApiErrorlargely mirror logic insrc/tools/create-supermodel-graph.ts. A small shared helper module would keep behavior consistent and reduce future bugs from mismatched changes.
Root Cause Analysis: CI Build FailureThe ErrorSDK ArchitectureThe SDK has two layers:
What Went WrongThis PR switched from // Main uses SupermodelClient (returns SupermodelIR directly)
const response = await client.graphs.generateSupermodelGraph(file);
response.summary // ✓ Works
// PR uses DefaultApi (returns SupermodelIRAsync wrapper)
const response = await client.api.generateSupermodelGraph({...});
response.summary // ✗ Fails - data is at response.result?.summaryWhy Local Typecheck PassedLocal FixKeep using |
DefaultApi returns async job wrappers (SupermodelIRAsync) that require manual polling. SupermodelClient handles polling automatically and returns unwrapped results with .summary property. Fixes CI build failure: Property 'summary' does not exist on type 'SupermodelIRAsync'
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/tools/graph-tools.ts`:
- Around line 336-339: The finally block currently awaits cleanup() directly
which can throw and mask a successful response; wrap the cleanup() call in its
own try/catch inside the finally so any error is caught and logged (e.g.,
console.error or the module's logger) and not rethrown—replace "await
cleanup();" with a try { await cleanup(); } catch (err) { /* log err only */ }
to ensure cleanup failures don't propagate.
Add explicit type check before falling back to defaultWorkdir. Returns INVALID_DIRECTORY error for values like `directory: 123` instead of confusing "missing directory" message.
Wrap cleanup() in try/catch inside finally block. Logs warning instead of throwing, so a successful graph generation isn't masked by a temp file cleanup error.
- Document get_call_graph, get_dependency_graph, get_domain_graph, get_parse_graph - Add "Choosing the Right Tool" comparison table - Add Local Development & Testing section with build/test instructions - Include test-local.js script for verifying server functionality
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/tools/graph-tools.ts`:
- Around line 264-277: The code currently validates providedDirectory but lacks
a type guard for jq_filter; add a check after the args destructuring to ensure
jq_filter is either undefined or a string and if not return asErrorResult with
type 'validation_error' (similar shape to the directory error) so invalid inputs
(e.g., jq_filter: 123) produce a clear validation error before calling
maybeFilter; reference the jq_filter variable, the maybeFilter call, and the
asErrorResult helper when implementing this guard.
- Around line 327-330: The code constructs a Blob (new Blob([...])) in the try
block around readFile(zipPath), which is only available globally in Node 18+;
either enforce Node 18+ by adding an "engines": { "node": ">=18" } entry to
package.json, or (preferred for older Node support) import Blob from the buffer
module and use that: add import { Blob } from 'buffer' at the top of the module
so the new Blob([...]) call in src/tools/graph-tools.ts works on Node versions
<18; update tests/CI if you change the engine requirement.
Move duplicated code from graph-tools.ts and create-supermodel-graph.ts: - formatBytes() - generateIdempotencyKey() - classifyApiError() - REPORT_REPO / REPORT_SUGGESTION constants Uses the more comprehensive classifyApiError from create-supermodel-graph.ts which handles 404, timeout, and network errors with detailed context.
There was a problem hiding this comment.
Required before merge:
- Add
jq_filtertype validation (similar todirectorycheck at line 277):
if (jq_filter !== undefined && typeof jq_filter !== 'string') {
return asErrorResult({
type: 'validation_error',
message: 'Invalid "jq_filter" parameter. Provide a jq filter string.',
code: 'INVALID_JQ_FILTER',
recoverable: false,
suggestion: 'Pass jq_filter as a string, e.g. jq_filter=".nodes".',
});
}- Add Node version constraint or import Blob explicitly (line 330):
- Option A: Add
"engines": { "node": ">=18" }to package.json - Option B: Add
import { Blob } from 'buffer';at top of graph-tools.ts
- Option A: Add
Optional follow-up:
- Consider extracting shared utilities (
generateIdempotencyKey,formatBytes,classifyApiError) to reduce duplication withcreate-supermodel-graph.ts
Address PR review feedback: - Add type validation for jq_filter parameter (returns INVALID_JQ_FILTER error) - Import Blob from 'buffer' module for Node.js <18 compatibility
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tools/create-supermodel-graph.ts (1)
698-701: ImportBlobfrombufferor enforce Node >=18.This code uses
new Blob(...)on line 700 without importing it. While Blob is a global in Node 18+, it doesn't exist in Node 16—users on that version will hit a runtime crash saying "Blob is not defined." Sincepackage.jsondoesn't specify a minimum Node version, this will break things for anyone still on Node 16.Pick one fix: Either import
Blobfrom thebuffermodule, or add"engines": { "node": ">=18" }topackage.jsonto make it clear this project needs Node 18+.🔧 Suggested fix (import Blob)
import { readFile } from 'fs/promises'; import { basename } from 'path'; +import { Blob } from 'buffer';
🤖 Fix all issues with AI agents
In `@src/tools/graph-tools.ts`:
- Around line 236-241: The catch block after calling maybeFilter should detect
jq parse errors and return a validation error instead of treating them as API
errors: when catching an error, call isJqError(error) and if true return
asErrorResult({ code: "INVALID_JQ_FILTER", message: error.message }) (or
whatever shape asErrorResult expects for validation errors) rather than
classifyApiError; otherwise keep the existing logger.error(`[${config.toolName}]
API error:`, error.message) and return asErrorResult(classifyApiError(error)).
Ensure you reference maybeFilter, isJqError, asErrorResult and classifyApiError
so the jq syntax issue yields a clear INVALID_JQ_FILTER result.
- Import Blob from 'buffer' in create-supermodel-graph.ts for Node <18 - Handle jq filter errors separately from API errors in graph-tools.ts using isJqError() to return INVALID_JQ_FILTER validation error
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/tools/graph-tools.ts`:
- Around line 141-185: The code silently treats an empty providedDirectory as
missing by using "providedDirectory || defaultWorkdir"; change the assignment so
the handler uses nullish coalescing (directory = providedDirectory ??
defaultWorkdir) so an explicit empty string stays as-is and the existing
validation (the subsequent check against !directory / typeof directory) will
catch and return the validation error; update the line that declares "const
directory = providedDirectory || defaultWorkdir;" accordingly in the handler.
- Add 19 unit tests covering tool exports, metadata, input schema,
and parameter validation for all graph tools
- Replace fragile array indexing (GRAPH_TYPES[0]) with safe lookup
using getGraphType('call') helper function
- Tests verify consistent validation across all 4 graph tools
Test count: 95 -> 114
Change || to ?? so empty string directory explicitly passed by caller triggers validation error instead of silently falling back to default.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tools/create-supermodel-graph.ts (1)
727-730: SDK layer mismatch — this is the CI error you mentioned.Based on the PR comments, if
ClientContextwas changed intypes.tsto exposeDefaultApidirectly, thenclient.graphsmight not be theSupermodelClientanymore. TheSupermodelClienthandles polling and returns the unwrappedSupermodelIRtype where.summaryexists. ButDefaultApi.generateSupermodelGraph()returnsSupermodelIRAsync— an async wrapper where the actual payload lives under.result.So when you do:
const summary = response.summary; // line 744TypeScript complains because
SupermodelIRAsyncdoesn't have.summaryat the top level — you'd needresponse.result?.summary.Your proposed fix is correct: revert
ClientContextto useSupermodelClient(client.graphs) so polling/unwrapping is handled automatically and.summaryis directly accessible.Also, the
fileBlob as anycast on line 728 is a bit of a smell — it might be hiding type mismatches between what the SDK expects and what you're passing. Once you fix the client layer, double-check if this cast is still needed.Also applies to: 744-749
Manual Testing with Claude CodeTested PR branch locally by cloning, building, and running the MCP server against its own codebase ( Setupgit clone https://github.com/supermodeltools/mcp.git /tmp/supermodel-mcp-test
cd /tmp/supermodel-mcp-test
git fetch origin pull/84/head:pr-84 && git checkout pr-84
npm install && npm run buildTools RegisteredVerified all 5 tools are exposed via
API Call Tests
Validation Error Tests
ConclusionAll new tools work correctly with real API calls. Error handling covers all validation cases with actionable suggestions. Ready to merge. |
greynewell
left a comment
greynewell
left a comment
There was a problem hiding this comment.
This looks good. I would prefer the test script be in JS, ideally a jest test. Also, we need to update the server instructions as well as the readme. I would also double check the tool descriptions for safety
scripts/test-local.js
Outdated
There was a problem hiding this comment.
I would prefer this as a jest test if possible
- Remove scripts/test-local.js, add Jest tests for tool safety instead - Update server instructions to document all graph tools - Add tool description safety tests (no unsafe instructions) - Update README to remove test-local.js references
This reverts commit 6d96d66.
Address review feedback from Grey: - Convert scripts/test-local.js to Jest integration test - Add 11 integration tests for MCP protocol and tools - Update server instructions to document available tools - Update README to reference Jest tests instead of script Test count: 114 -> 125
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 311-367: The README marks the `directory` parameter as required
for get_dependency_graph, get_domain_graph, and get_parse_graph even though the
server can use a configured default working directory; update each parameters
table for `get_dependency_graph`, `get_domain_graph`, and `get_parse_graph` to
mark `directory` as optional (or indicate "Optional if default workdir is set"),
keep `jq_filter` optional, and add a short clarifying sentence/note explaining
that if a default workdir is configured the `directory` argument may be omitted
(include a brief CLI example like `node dist/index.js /path/to/repo` or the
equivalent to show omission). Ensure the wording is consistent across all three
function docs.
In `@src/tools/graph-tools.test.ts`:
- Around line 1-149: Tests use hard-coded Unix paths ('/nonexistent/path/xyz123'
and '/default/workdir') which break on Windows or if the path exists; change
those tests in src/tools/graph-tools.test.ts to build OS-safe non-existent paths
using Node's os.tmpdir() and path.join (e.g., path.join(os.tmpdir(),
`nonexistent-${Date.now()}-${Math.random()}`)) so they are portable; update the
two cases that call callGraphTool.handler (the DIRECTORY_NOT_FOUND and
default-workdir test) to pass these generated paths instead of the hard-coded
strings.
🧹 Nitpick comments (2)
scripts/test-local.js (1)
21-25: Handle server start failures explicitly.If
dist/index.jsis missing or crashes immediately, writes to stdin can throw EPIPE and the script keeps going with confusing output. A tinyerrorhandler makes failures obvious.Suggested tweak
const server = spawn('node', ['dist/index.js'], { stdio: ['pipe', 'pipe', 'inherit'], env: { ...process.env } }); + +server.on('error', (err) => { + console.error('Failed to start MCP server:', err); + process.exit(1); +});src/server.ts (1)
121-131: Guard against duplicate tool names.If two tools accidentally share a name, the later one silently overwrites the earlier in the
Map. A quick check prevents hard‑to‑trace routing bugs (e.g.,get_call_graphhiding another tool).Example guard
- const toolMap = new Map( - allTools.map(t => [t.tool.name, t]) - ); + const toolMap = new Map(); + for (const t of allTools) { + if (toolMap.has(t.tool.name)) { + throw new Error(`Duplicate tool name: ${t.tool.name}`); + } + toolMap.set(t.tool.name, t); + }
| **Parameters:** | ||
|
|
||
| | Argument | Type | Required | Description | | ||
| |----------|------|----------|-------------| | ||
| | `directory` | string | Yes | Path to repository directory | | ||
| | `jq_filter` | string | No | jq filter for custom data extraction | | ||
|
|
||
| #### `get_dependency_graph` | ||
|
|
||
| Generate a module-level dependency graph showing import relationships. | ||
|
|
||
| **Use this to:** | ||
| - Understand module dependencies | ||
| - Find circular dependencies | ||
| - Identify tightly coupled modules | ||
| - Plan module extraction or refactoring | ||
|
|
||
| **Parameters:** | ||
|
|
||
| | Argument | Type | Required | Description | | ||
| |----------|------|----------|-------------| | ||
| | `directory` | string | Yes | Path to repository directory | | ||
| | `jq_filter` | string | No | jq filter for custom data extraction | | ||
|
|
||
| #### `get_domain_graph` | ||
|
|
||
| Generate a high-level domain classification graph. | ||
|
|
||
| **Use this to:** | ||
| - Understand the architectural domains in a codebase | ||
| - See how code is organized into logical areas | ||
| - Get a bird's-eye view of system structure | ||
| - Identify domain boundaries | ||
|
|
||
| **Parameters:** | ||
|
|
||
| | Argument | Type | Required | Description | | ||
| |----------|------|----------|-------------| | ||
| | `directory` | string | Yes | Path to repository directory | | ||
| | `jq_filter` | string | No | jq filter for custom data extraction | | ||
|
|
||
| #### `get_parse_graph` | ||
|
|
||
| Generate an AST-level parse graph with fine-grained code structure. | ||
|
|
||
| **Use this to:** | ||
| - Analyze detailed code structure | ||
| - Find specific syntax patterns | ||
| - Understand class/function definitions at AST level | ||
| - Support precise refactoring operations | ||
|
|
||
| **Parameters:** | ||
|
|
||
| | Argument | Type | Required | Description | | ||
| |----------|------|----------|-------------| | ||
| | `directory` | string | Yes | Path to repository directory | | ||
| | `jq_filter` | string | No | jq filter for custom data extraction | |
There was a problem hiding this comment.
Clarify that directory can be omitted when a default workdir is set.
The server supports a default working directory and passes it to tools, but these tables mark directory as strictly required. This can confuse users who rely on the CLI default (e.g., node dist/index.js /path/to/repo).
Doc tweak example
-| `directory` | string | Yes | Path to repository directory |
+| `directory` | string | Yes* | Path to repository directory |
...
+* If the server was started with a default working directory, `directory` can be omitted.🤖 Prompt for AI Agents
In `@README.md` around lines 311 - 367, The README marks the `directory` parameter
as required for get_dependency_graph, get_domain_graph, and get_parse_graph even
though the server can use a configured default working directory; update each
parameters table for `get_dependency_graph`, `get_domain_graph`, and
`get_parse_graph` to mark `directory` as optional (or indicate "Optional if
default workdir is set"), keep `jq_filter` optional, and add a short clarifying
sentence/note explaining that if a default workdir is configured the `directory`
argument may be omitted (include a brief CLI example like `node dist/index.js
/path/to/repo` or the equivalent to show omission). Ensure the wording is
consistent across all three function docs.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/server.integration.test.ts`:
- Around line 6-72: The test currently assumes dist/index.js exists and the
server is ready after a fixed 500ms wait which causes flaky timeouts; update the
beforeAll setup to first check existence of distPath (use fs.existsSync on the
distPath variable and fail the test early with a clear error if missing) and
replace the fixed await new Promise(r => setTimeout(r, 500)) with a configurable
startup wait or retry loop that polls server readiness (e.g., watch
server.stdout for a ready/initialized line or attempt a lightweight JSON-RPC
ping via sendRequest with retries using requestId/responseQueue) before
proceeding; ensure server is killed on failure and include clear error messages
referencing distPath, beforeAll, sendRequest, server, and responseQueue so CI
failures are fast-failing and deterministic.
- Add JSDoc comments to filtering.ts, index.ts, and server.ts for docstring coverage - Add guard against duplicate tool names in server.ts toolMap - Use OS-safe paths in graph-tools.test.ts (os.tmpdir() instead of hardcoded Unix paths) - Improve integration test startup detection with dist/index.js existence check and retry loop
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/server.integration.test.ts`:
- Around line 6-11: Replace the hardcoded Unix path '/nonexistent/path/xyz123'
with an OS-safe temp-based path: add "import * as os from 'os'" (since path is
already imported) and create a variable like nonExistentDir =
path.join(os.tmpdir(),
`nonexistent-${Date.now()}-${Math.random().toString(36).slice(2)}`) and use
nonExistentDir wherever '/nonexistent/path/xyz123' appears (including the
occurrences around lines 231-235) so the tests are portable across
Windows/macOS/Linux.
- Around line 94-107: The startup wait loop can silently exit without the server
being ready — add a boolean ready flag (e.g., let ready = false) before the
loop, set ready = true when server.stdin?.writable is detected inside the loop,
and after the loop check if (!ready) throw a descriptive error (including
SERVER_STARTUP_TIMEOUT_MS or server.exitCode) to fail fast; reference the
existing symbols SERVER_STARTUP_TIMEOUT_MS, STARTUP_POLL_INTERVAL_MS,
server.stdin?.writable and server.exitCode when implementing the check.
🧹 Nitpick comments (2)
src/filtering.ts (2)
6-6: The@ts-nocheckdisables all TypeScript checking for this file.Hey! So this directive basically tells TypeScript: "Don't look at this file, trust me bro." 😅 The problem is it can hide real bugs — like if someone passes the wrong type to
maybeFilter, TypeScript won't catch it.Usually
@ts-nocheckgets added when there's a tricky library (likejq-webhere) that doesn't have great type definitions. If that's the case, a more surgical approach is to use@ts-expect-erroror@ts-ignoreonly on the specific line that needs it (line 30), rather than the whole file.Example — instead of blanket-disabling checks:
// `@ts-expect-error` jq-web types are incomplete return (await initJq).json(json, jqFilter);This way you still get type safety for the rest of the file. Worth considering if you have time!
38-39: Type guard could be more precise to capture thestderrproperty.So right now the function checks for
'stderr' in error, but the return type only narrows toError. This means if someone uses this guard, TypeScript won't know thatstderrexists on the error object — they'd still get a type error trying to accesserror.stderr.A quick fix is to make the return type more specific:
♻️ Suggested improvement
-export function isJqError(error: any): error is Error { +export function isJqError(error: any): error is Error & { stderr: string } { return error instanceof Error && 'stderr' in error; }Then consumers can safely do:
if (isJqError(err)) { console.log(err.stderr); // TypeScript knows stderr exists! }
| import { describe, it, expect, beforeAll, afterAll } from '@jest/globals'; | ||
| import { spawn, ChildProcess } from 'child_process'; | ||
| import { existsSync } from 'fs'; | ||
| import * as readline from 'readline'; | ||
| import * as path from 'path'; | ||
|
|
There was a problem hiding this comment.
Make the “non‑existent directory” OS‑safe.
/nonexistent/path/xyz123 is Unix‑ish; on Windows it resolves to a drive root and could theoretically exist. Using os.tmpdir() keeps it portable (same idea you used in the unit tests).
🛠️ Suggested fix
-import { existsSync } from 'fs';
+import { existsSync } from 'fs';
+import * as os from 'os';
import * as readline from 'readline';
import * as path from 'path';
@@
it('should return not_found error for non-existent directory', async () => {
+ const missingDir = path.join(
+ os.tmpdir(),
+ `supermodel-missing-${Date.now()}-${Math.random().toString(36).slice(2)}`
+ );
const result = await sendRequest('tools/call', {
name: 'get_call_graph',
- arguments: { directory: '/nonexistent/path/xyz123' }
+ arguments: { directory: missingDir }
});Also applies to: 231-235
🤖 Prompt for AI Agents
In `@src/server.integration.test.ts` around lines 6 - 11, Replace the hardcoded
Unix path '/nonexistent/path/xyz123' with an OS-safe temp-based path: add
"import * as os from 'os'" (since path is already imported) and create a
variable like nonExistentDir = path.join(os.tmpdir(),
`nonexistent-${Date.now()}-${Math.random().toString(36).slice(2)}`) and use
nonExistentDir wherever '/nonexistent/path/xyz123' appears (including the
occurrences around lines 231-235) so the tests are portable across
Windows/macOS/Linux.
| // Wait for server to be ready with retry loop | ||
| const startTime = Date.now(); | ||
| while (Date.now() - startTime < SERVER_STARTUP_TIMEOUT_MS) { | ||
| // Check if server has exited unexpectedly | ||
| if (server.exitCode !== null) { | ||
| throw new Error(`Server exited unexpectedly with code ${server.exitCode}`); | ||
| } | ||
| // Small delay between checks | ||
| await new Promise(r => setTimeout(r, STARTUP_POLL_INTERVAL_MS)); | ||
| // Server is ready when stdin is writable | ||
| if (server.stdin?.writable) { | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Fail fast if the server never becomes ready.
Right now the loop just stops after 5s even if the server isn’t ready, so the next test fails later with a less obvious timeout (e.g., “Request initialize timed out”). Add a ready flag and throw right after the loop when startup doesn’t complete.
🛠️ Suggested fix
- const startTime = Date.now();
- while (Date.now() - startTime < SERVER_STARTUP_TIMEOUT_MS) {
+ const startTime = Date.now();
+ let ready = false;
+ while (Date.now() - startTime < SERVER_STARTUP_TIMEOUT_MS) {
// Check if server has exited unexpectedly
if (server.exitCode !== null) {
throw new Error(`Server exited unexpectedly with code ${server.exitCode}`);
}
// Small delay between checks
await new Promise(r => setTimeout(r, STARTUP_POLL_INTERVAL_MS));
// Server is ready when stdin is writable
if (server.stdin?.writable) {
- break;
+ ready = true;
+ break;
}
}
+ if (!ready) {
+ throw new Error(`Server did not become ready within ${SERVER_STARTUP_TIMEOUT_MS}ms`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Wait for server to be ready with retry loop | |
| const startTime = Date.now(); | |
| while (Date.now() - startTime < SERVER_STARTUP_TIMEOUT_MS) { | |
| // Check if server has exited unexpectedly | |
| if (server.exitCode !== null) { | |
| throw new Error(`Server exited unexpectedly with code ${server.exitCode}`); | |
| } | |
| // Small delay between checks | |
| await new Promise(r => setTimeout(r, STARTUP_POLL_INTERVAL_MS)); | |
| // Server is ready when stdin is writable | |
| if (server.stdin?.writable) { | |
| break; | |
| } | |
| } | |
| // Wait for server to be ready with retry loop | |
| const startTime = Date.now(); | |
| let ready = false; | |
| while (Date.now() - startTime < SERVER_STARTUP_TIMEOUT_MS) { | |
| // Check if server has exited unexpectedly | |
| if (server.exitCode !== null) { | |
| throw new Error(`Server exited unexpectedly with code ${server.exitCode}`); | |
| } | |
| // Small delay between checks | |
| await new Promise(r => setTimeout(r, STARTUP_POLL_INTERVAL_MS)); | |
| // Server is ready when stdin is writable | |
| if (server.stdin?.writable) { | |
| ready = true; | |
| break; | |
| } | |
| } | |
| if (!ready) { | |
| throw new Error(`Server did not become ready within ${SERVER_STARTUP_TIMEOUT_MS}ms`); | |
| } |
🤖 Prompt for AI Agents
In `@src/server.integration.test.ts` around lines 94 - 107, The startup wait loop
can silently exit without the server being ready — add a boolean ready flag
(e.g., let ready = false) before the loop, set ready = true when
server.stdin?.writable is detected inside the loop, and after the loop check if
(!ready) throw a descriptive error (including SERVER_STARTUP_TIMEOUT_MS or
server.exitCode) to fail fast; reference the existing symbols
SERVER_STARTUP_TIMEOUT_MS, STARTUP_POLL_INTERVAL_MS, server.stdin?.writable and
server.exitCode when implementing the check.
…API info - Describe actual return types for each graph tool based on API verification - Remove incorrect AST references from parse graph description - Add "Returns:" section to each tool showing actual output structure - Simplify server instructions with scannable table format - Filed bug for dependency graph returning full graph instead of imports only
Summary
Adds four new MCP tools for targeted graph generation, allowing agents to request only the graph type relevant to their task:
get_call_graph/v1/graphs/callget_dependency_graph/v1/graphs/dependencyget_domain_graph/v1/graphs/domainget_parse_graph/v1/graphs/parseThe existing
explore_codebasetool (full supermodel graph) remains available for comprehensive analysis.Motivation
Changes
src/tools/graph-tools.ts- New file with factory for creating graph-type-specific toolssrc/server.ts- Register all tools (existing + new)src/types.ts- Update ClientContext to use DefaultApi directlysrc/tools/create-supermodel-graph.ts- Update to use DefaultApiTest Plan
npm run buildpassesnpm testpasses (95 tests)Closes #81
Summary by CodeRabbit
New Features
Bug Fixes & Reliability
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.