chore: add Claude Code config + custom SDK MCP server#635
chore: add Claude Code config + custom SDK MCP server#635
Conversation
Set up AI-assisted development tooling: - Root CLAUDE.md + SDK-specific CLAUDE.md files (ts, ts-compat, go) - .claude/settings.json with permission allow/deny lists - .claude/rules/ for TypeScript, Go, and Solidity conventions - .claude/skills/ for /test, /typecheck, /lint, /build-sdk, /review-pr - .mcp.json for GitHub MCP + custom Nitrolite SDK MCP server - sdk/mcp/ — custom MCP server exposing SDK API surface (resources, tools, prompts) - Root .gitignore for local Claude files
ihsraham
requested review from
dimast-x,
nksazonov and
philanton
as code owners
📝 WalkthroughWalkthroughAdds repository-wide Claude rules, skills (build/lint/test/typecheck/review), MCP server tooling that indexes the TypeScript SDK and exposes query tools, and documentation/config for Go, TypeScript, and Solidity packages. Also adds MCP package and related configs and .gitignore entries. Changes
Sequence Diagram(s)sequenceDiagram
participant FS as File System (sdk/ts, sdk/ts-compat)
participant Parser as Source Parser/Indexer
participant Mcp as McpServer (nitrolite-sdk)
participant Transport as StdioServerTransport
participant Client as MCP Client
Note over FS,Parser: startup
FS->>Parser: read SDK source files
Parser->>Parser: extract methods, types, barrels, docs
Parser->>Mcp: register resources (methods, types, enums, examples)
Parser->>Mcp: register tools (fuzzy search, type lookup, export validation)
Mcp->>Transport: start listening over stdio
Client->>Transport: query (fuzzy search / lookup)
Transport->>Mcp: forward request
Mcp->>Parser: lookup/index query
Mcp->>Transport: return results
Transport->>Client: deliver response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 11
🧹 Nitpick comments (3)
sdk/mcp/tsconfig.json (1)
1-14: LGTM with a minor note onmoduleResolution.The TypeScript configuration is solid with strict mode enabled and proper output settings. The
moduleResolution: "bundler"option is well-suited for modern bundler workflows, though"node16"or"nodenext"might be slightly more conventional for a Node.js CLI tool executed viatsx. Since the MCP server is always invoked throughtsx(per.mcp.jsonandpackage.jsonscripts), the current choice works fine.📦 Optional: Consider moduleResolution for CLI context
If this package will be published and consumed as a CLI tool, consider using
"moduleResolution": "node16"or"nodenext"for better alignment with Node.js module resolution semantics:"compilerOptions": { "target": "es2022", "module": "ESNext", - "moduleResolution": "bundler", + "moduleResolution": "node16", "strict": true,However, since the current setup uses
tsxfor all execution paths, the existing configuration is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/mcp/tsconfig.json` around lines 1 - 14, The tsconfig.json uses "moduleResolution": "bundler" which is fine for tsx but might be more conventional to use Node resolution for a CLI; update the compilerOptions.moduleResolution value to "node16" or "nodenext" in tsconfig.json (or keep "bundler" if you intentionally rely on tsx) and ensure this aligns with how the MCP server is invoked in package.json/.mcp.json; change only the "moduleResolution" setting in the existing tsconfig.json to the chosen value..claude/skills/build-sdk/SKILL.md (1)
18-18:ts-compatbuild gate ondist/existence is brittle.On Line 18, checking only folder existence can accept stale
sdk/tsartifacts. Prefer always buildingsdk/tsbeforesdk/ts-compatfor deterministic results.Suggested wording change
- - "ts-compat" or "compat" -> check sdk/ts/dist/ exists first; if not, build sdk/ts first, then `cd sdk/ts-compat && npm run build` + - "ts-compat" or "compat" -> always build `sdk/ts` first, then `cd sdk/ts-compat && npm run build`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/build-sdk/SKILL.md at line 18, Replace the brittle "check sdk/ts/dist/ exists" gate by always building the TypeScript SDK before the compatibility package: ensure the docs/instructions tell to run the build for sdk/ts first (e.g., run the existing build step for the sdk/ts project) and then run the ts-compat build (the step that currently reads `cd sdk/ts-compat && npm run build`), removing any conditional that skips building sdk/ts based solely on the presence of sdk/ts/dist so the sequence is deterministic.sdk/ts-compat/CLAUDE.md (1)
28-28: Avoid source line numbers in this guide.
src/index.tslines 154-158 will drift the next time exports move around. Point to the relevant symbol or section instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/ts-compat/CLAUDE.md` at line 28, Replace the brittle "src/index.ts lines 154-158" reference with a stable reference to the exported symbol(s) or section name instead (e.g., the export name(s) from src/index.ts such as the relevant function/class/constant or a heading like "Exports" or "Public API"); update CLAUDE.md to mention the specific symbol(s) exported by src/index.ts rather than line numbers so the doc stays correct when code shifts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.json:
- Around line 3-25: Add an allowlist entry for the MCP startup command so Claude
can run the referenced test plan; update the "allow" array in
.claude/settings.json to include a pattern matching the MCP startup command
(e.g., add an entry like Bash(npx tsx sdk/mcp/src/index.ts*) near the other npx
entries such as Bash(npx jest*) and Bash(npx prettier*)). Ensure the pattern
covers optional args/flags and follows the same quoting/format as the existing
entries so the MCP verification command is permitted.
In @.claude/skills/lint/SKILL.md:
- Line 12: The /lint route for sdk/ts-compat currently invokes "npm run
typecheck" which does not perform linting/formatting; update the route mapping
for "/lint" (the sdk/ts-compat entry) to call a real lint/format script if one
exists (e.g., "npm run lint" or "npm run format"), otherwise change the SKILL.md
entry for the sdk/ts-compat mapping to explicitly document that "/lint" is a
temporary fallback to "npm run typecheck" and state its limitations (type-only
checks, not style/format validation) so callers know it does not perform
linting.
In @.claude/skills/test/SKILL.md:
- Line 21: Update the wording that currently says "**Repo root with no
argument** -> run `go test ./...`" to clarify this only runs Go packages and
does not run TypeScript SDKs, contracts, or integration tests; explicitly state
that `go test ./...` skips `sdk/ts`, `sdk/ts-compat`, `contracts`, and
`test/integration` and add a suggested routing note to run TS tests separately
(e.g., `cd sdk/ts && npm test` or `pnpm -w test` / appropriate commands for
`sdk/ts-compat`), and call out running integration tests in `test/integration`
with their required command so the README entry in SKILL.md accurately reflects
a full-project test procedure.
- Around line 23-26: Summary: The Go recipe incorrectly tries to scope by file
using "go test -run TestName ./sdk/go/..." which filters by test name across
packages rather than selecting a specific file. Fix: update the recipe to
resolve the target file's containing package (use the file path to determine the
package path for the package that contains the _test.go), then invoke go test
against that package path with -run TestName to filter the test function;
reference the file name and its containing package when constructing the
command. Ensure the documentation examples reference go test, the -run flag, the
resolved package path (not ./sdk/go/...), and the target test function name.
In @.mcp.json:
- Around line 7-10: The MCP entry "nitrolite-sdk" currently runs "npx tsx
sdk/mcp/src/index.ts" without ensuring sdk/mcp dependencies are installed; fix
by adding a small startup wrapper or documentation: create a startup script in
sdk/mcp (e.g., start.sh or start.js) that changes to sdk/mcp, checks for
node_modules (or runs npm ci) and installs if missing, then invokes "npx tsx
src/index.ts", and update the ".mcp.json" "nitrolite-sdk" command/args to call
that wrapper; alternatively add a clear setup note in sdk/mcp/CLAUDE.md or the
repo README instructing developers to run npm install in sdk/mcp before starting
the MCP server.
In `@CLAUDE.md`:
- Around line 69-77: The fenced code block containing the commit message
examples (the line starting with "feat|fix|chore|test|docs(scope): description"
and the three example lines) is missing a language tag which triggers MD040;
update the opening fence to include a language identifier such as "text" or
"bash" (e.g., change "```" to "```text") so the block is properly tagged for
markdownlint.
In `@sdk/mcp/package.json`:
- Around line 6-9: The package.json currently points "main" and the "bin" entry
"nitrolite-sdk-mcp" at the TypeScript source (src/index.ts); change both to the
compiled output (e.g., dist/index.js) so consumers get built artifacts after
running the existing "build" (tsc) script, and optionally add a "files" array
(e.g., ["dist","README.md"]) to control published files; update the "main" key
and the "bin" mapping in package.json accordingly and ensure the build produces
dist/index.js.
In `@sdk/mcp/src/index.ts`:
- Around line 371-403: The tool "search_api" currently only searches the
in-memory arrays methods and types but its description mentions examples; update
the implementation to include the examples corpus (e.g., an examples array or
resource used elsewhere) in the fuzzy search and add the relevant fields (title,
body, tags) to the query match, or alternatively change the tool description to
remove "examples" if you don't want to search them; specifically modify the
async handler in search_api to filter an examples collection (e.g.,
examples.filter(e => e.title.toLowerCase().includes(q) ||
e.body.toLowerCase().includes(q) || e.tags.join(' ').toLowerCase().includes(q)))
and append a "## Examples" section to the generated text, or adjust the
description string passed to server.tool to reflect only methods and types.
- Around line 30-44: extractExports currently returns only strings and strips
the `type` keyword; change it to return structured metadata like { name: string,
isType: boolean } by detecting whether each export token was declared as `type`
(and preserving the original exported identifier after removing `as` aliases),
and keep the wildcard handling as a distinct record. Then update validate_import
to match symbols using exact equality against the exported name (not .includes)
and consult the isType flag to recommend `import type { ... }` for type-only
exports and regular `import { ... }` for value exports; reference the functions
extractExports and validate_import when making these changes.
- Around line 190-207: The examples in the server.resource('examples-channels',
'nitrolite://examples/channels', ...) payload use new Decimal(...) but do not
import Decimal, so pasted snippets fail; update the example blocks referenced
(the channel example and the other snippets at the other example resources) to
include an import for Decimal from decimal.js (or replace usages with the actual
accepted runtime numeric type) at the top of each snippet so the examples are
copy-paste runnable; specifically edit the example code inside
server.resource('examples-channels', ...) and the other example resources
covering the snippets noted (lines ~190–207, ~229–236, ~256–269) to add the
Decimal import or switch to the correct type.
- Around line 69-89: The loadClientMethods() parser incorrectly forces every
signature to Promise by building signature as `${name}(${params}):
Promise<${returnType}>`—change it to use the parsed returnType directly (e.g.,
`${name}(${params}): ${returnType}`) or replace the regex approach with
TypeScript AST parsing to preserve JSDoc and exact signatures; additionally, add
a missing Decimal import (import Decimal from 'decimal.js') where new
Decimal(...) is used, correct the search_api description to match actual
behavior or expand the search implementation to include examples (update the
text that claims it searches "methods, types, and examples"), and fix
validate_import to use exact symbol matching (use word-boundary regex or exact
token comparison instead of e.includes(symbol) and sdkBarrel.includes(symbol))
so partial substrings no longer produce false positives.
---
Nitpick comments:
In @.claude/skills/build-sdk/SKILL.md:
- Line 18: Replace the brittle "check sdk/ts/dist/ exists" gate by always
building the TypeScript SDK before the compatibility package: ensure the
docs/instructions tell to run the build for sdk/ts first (e.g., run the existing
build step for the sdk/ts project) and then run the ts-compat build (the step
that currently reads `cd sdk/ts-compat && npm run build`), removing any
conditional that skips building sdk/ts based solely on the presence of
sdk/ts/dist so the sequence is deterministic.
In `@sdk/mcp/tsconfig.json`:
- Around line 1-14: The tsconfig.json uses "moduleResolution": "bundler" which
is fine for tsx but might be more conventional to use Node resolution for a CLI;
update the compilerOptions.moduleResolution value to "node16" or "nodenext" in
tsconfig.json (or keep "bundler" if you intentionally rely on tsx) and ensure
this aligns with how the MCP server is invoked in package.json/.mcp.json; change
only the "moduleResolution" setting in the existing tsconfig.json to the chosen
value.
In `@sdk/ts-compat/CLAUDE.md`:
- Line 28: Replace the brittle "src/index.ts lines 154-158" reference with a
stable reference to the exported symbol(s) or section name instead (e.g., the
export name(s) from src/index.ts such as the relevant function/class/constant or
a heading like "Exports" or "Public API"); update CLAUDE.md to mention the
specific symbol(s) exported by src/index.ts rather than line numbers so the doc
stays correct when code shifts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2efaa07-4130-4373-b200-40acf2027214
⛔ Files ignored due to path filters (1)
sdk/mcp/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.claude/rules/go.md.claude/rules/solidity.md.claude/rules/typescript.md.claude/settings.json.claude/skills/build-sdk/SKILL.md.claude/skills/lint/SKILL.md.claude/skills/review-pr/SKILL.md.claude/skills/test/SKILL.md.claude/skills/typecheck/SKILL.md.gitignore.mcp.jsonCLAUDE.mdsdk/go/CLAUDE.mdsdk/mcp/.gitignoresdk/mcp/package.jsonsdk/mcp/src/index.tssdk/mcp/tsconfig.jsonsdk/ts-compat/CLAUDE.mdsdk/ts/CLAUDE.md
| Route to the correct linter based on context: | ||
|
|
||
| - **sdk/ts/** -> `cd sdk/ts && npm run lint && npx prettier --check .` | ||
| - **sdk/ts-compat/** -> `cd sdk/ts-compat && npm run typecheck` (no dedicated lint script; typecheck is the closest) |
There was a problem hiding this comment.
/lint route for sdk/ts-compat is doing typecheck, not linting.
On Line 12, routing lint to npm run typecheck can silently skip formatting/style validation. Please either run an actual lint/format command (if available) or explicitly document this as a temporary fallback with clear limitation text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/lint/SKILL.md at line 12, The /lint route for sdk/ts-compat
currently invokes "npm run typecheck" which does not perform linting/formatting;
update the route mapping for "/lint" (the sdk/ts-compat entry) to call a real
lint/format script if one exists (e.g., "npm run lint" or "npm run format"),
otherwise change the SKILL.md entry for the sdk/ts-compat mapping to explicitly
document that "/lint" is a temporary fallback to "npm run typecheck" and state
its limitations (type-only checks, not style/format validation) so callers know
it does not perform linting.
.claude/skills/test/SKILL.md
Outdated
| - **contracts/** or .sol files -> `cd contracts && forge test` | ||
| - **clearnode/** -> `go test ./clearnode/... -v` (from repo root) | ||
| - **test/integration/** -> `cd test/integration && npm test` | ||
| - **Repo root with no argument** -> run `go test ./...` and note TS tests need separate runs |
There was a problem hiding this comment.
Repo-root /test is not a whole-project run.
go test ./... only exercises Go packages, so this branch skips sdk/ts, sdk/ts-compat, contracts, and test/integration while still sounding like a full-project pass.
Suggested routing
-- **Repo root with no argument** -> run `go test ./...` and note TS tests need separate runs
+- **Repo root with no argument** -> either run every top-level suite (`go test ./sdk/go/... -v`, `go test ./clearnode/... -v`, `cd sdk/ts && npm test`, `cd sdk/ts-compat && npm test`, `cd contracts && forge test`, `cd test/integration && npm test`) or ask the user to choose a target📝 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.
| - **Repo root with no argument** -> run `go test ./...` and note TS tests need separate runs | |
| - **Repo root with no argument** -> either run every top-level suite (`go test ./sdk/go/... -v`, `go test ./clearnode/... -v`, `cd sdk/ts && npm test`, `cd sdk/ts-compat && npm test`, `cd contracts && forge test`, `cd test/integration && npm test`) or ask the user to choose a target |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/test/SKILL.md at line 21, Update the wording that currently
says "**Repo root with no argument** -> run `go test ./...`" to clarify this
only runs Go packages and does not run TypeScript SDKs, contracts, or
integration tests; explicitly state that `go test ./...` skips `sdk/ts`,
`sdk/ts-compat`, `contracts`, and `test/integration` and add a suggested routing
note to run TS tests separately (e.g., `cd sdk/ts && npm test` or `pnpm -w test`
/ appropriate commands for `sdk/ts-compat`), and call out running integration
tests in `test/integration` with their required command so the README entry in
SKILL.md accurately reflects a full-project test procedure.
| If a specific test file is given (e.g., `sdk/ts/test/unit/utils.test.ts`), run only that file: | ||
| - For Jest: `cd sdk/ts && npx jest test/unit/utils.test.ts` | ||
| - For Go: `go test -run TestName ./sdk/go/...` | ||
| - For Forge: `cd contracts && forge test --match-path test/MyTest.t.sol` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does go test -runsupport executing a single*_test.go file by path, or does it only filter test function names inside the selected package(s)?
💡 Result:
No, go test -run does not support executing a single *_test.go file by path. It only filters test function names (using a regular expression) inside the selected package(s). To run tests from a specific *_test.go file, use go test <file_path>, which compiles and runs only the tests in that file (treated as a package). The -run flag then filters which test functions within that file/package to execute. Official documentation confirms -run regexp "Run only those tests and examples matching the regular expression" with no mention of file paths. All sources distinguish file selection (via arguments) from function filtering (via -run).
Citations:
- 1: https://go.dev/cmd/go/internal/test%40go1.25.1
- 2: https://pkg.go.dev/testing/
- 3: http://golang.org/cmd/gotest
- 4: https://pkg.go.dev/testing@go1.22.2
- 5: https://manpages.ubuntu.com/manpages/lunar/en/man7/go-testflag.7.html
- 6: https://pkg.go.dev/testing@go1.25.5
The Go single-file recipe will not scope to one file.
go test -run TestName ./sdk/go/... filters test functions across the selected package set; it does not select a specific _test.go file. To run a test from a specific file, resolve the file's containing package first, then use that package path with -run to filter the test function name.
Suggested wording
-- For Go: `go test -run TestName ./sdk/go/...`
+- For Go: resolve the file's containing package and run that package. If you also want one test, extract its concrete `Test...` name first and then use `go test ./sdk/go/<pkg> -run '^TestName$'`📝 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.
| If a specific test file is given (e.g., `sdk/ts/test/unit/utils.test.ts`), run only that file: | |
| - For Jest: `cd sdk/ts && npx jest test/unit/utils.test.ts` | |
| - For Go: `go test -run TestName ./sdk/go/...` | |
| - For Forge: `cd contracts && forge test --match-path test/MyTest.t.sol` | |
| If a specific test file is given (e.g., `sdk/ts/test/unit/utils.test.ts`), run only that file: | |
| - For Jest: `cd sdk/ts && npx jest test/unit/utils.test.ts` | |
| - For Go: resolve the file's containing package and run that package. If you also want one test, extract its concrete `Test...` name first and then use `go test ./sdk/go/<pkg> -run '^TestName$'` | |
| - For Forge: `cd contracts && forge test --match-path test/MyTest.t.sol` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/test/SKILL.md around lines 23 - 26, Summary: The Go recipe
incorrectly tries to scope by file using "go test -run TestName ./sdk/go/..."
which filters by test name across packages rather than selecting a specific
file. Fix: update the recipe to resolve the target file's containing package
(use the file path to determine the package path for the package that contains
the _test.go), then invoke go test against that package path with -run TestName
to filter the test function; reference the file name and its containing package
when constructing the command. Ensure the documentation examples reference go
test, the -run flag, the resolved package path (not ./sdk/go/...), and the
target test function name.
| "main": "src/index.ts", | ||
| "bin": { | ||
| "nitrolite-sdk-mcp": "src/index.ts" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for publish-related config or scripts
rg -i "npm publish|publishConfig" sdk/mcp/package.json
# Check if there's a README or publishing documentation
fd -t f "README" sdk/mcp/Repository: layer-3/nitrolite
Length of output: 43
🏁 Script executed:
# Read the full package.json to see current configuration
cat sdk/mcp/package.jsonRepository: layer-3/nitrolite
Length of output: 677
🏁 Script executed:
# Check if src/index.ts exists
test -f sdk/mcp/src/index.ts && echo "src/index.ts exists" || echo "src/index.ts does not exist"
# Check if dist directory exists
test -d sdk/mcp/dist && echo "dist/ exists" || echo "dist/ does not exist"
# Look for build-related files in sdk/mcp
fd -t f "tsconfig|build|webpack|vite|esbuild" sdk/mcp/ --max-depth 2Repository: layer-3/nitrolite
Length of output: 123
🏁 Script executed:
# Check if there are any build scripts in package.json and explore broader context
head -50 sdk/mcp/package.json
# Check if this is a monorepo and how other packages handle this
rg '"main".*"' sdk/ -A 1 -B 1 | head -40Repository: layer-3/nitrolite
Length of output: 1114
main and bin point to TypeScript source instead of built output.
The package has a build script configured (tsc), but main and bin reference src/index.ts instead of the compiled output. This breaks npm distribution—consumers installing the package will try to import TypeScript source directly without access to the compiler. Other packages in this monorepo (sdk/ts, sdk/ts-compat) already follow the dist/ pattern, confirming the expected structure.
Update to point to built artifacts:
Proposed fix
"type": "module",
- "main": "src/index.ts",
+ "main": "dist/index.js",
"bin": {
- "nitrolite-sdk-mcp": "src/index.ts"
+ "nitrolite-sdk-mcp": "dist/index.js"
},Optionally add to control published files:
"files": ["dist", "README.md"],📝 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.
| "main": "src/index.ts", | |
| "bin": { | |
| "nitrolite-sdk-mcp": "src/index.ts" | |
| }, | |
| "main": "dist/index.js", | |
| "bin": { | |
| "nitrolite-sdk-mcp": "dist/index.js" | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/mcp/package.json` around lines 6 - 9, The package.json currently points
"main" and the "bin" entry "nitrolite-sdk-mcp" at the TypeScript source
(src/index.ts); change both to the compiled output (e.g., dist/index.js) so
consumers get built artifacts after running the existing "build" (tsc) script,
and optionally add a "files" array (e.g., ["dist","README.md"]) to control
published files; update the "main" key and the "bin" mapping in package.json
accordingly and ensure the build produces dist/index.js.
| /** Extract named exports from a barrel file */ | ||
| function extractExports(content: string): string[] { | ||
| const names: string[] = []; | ||
| // Match: export { Foo, Bar } from '...' and export type { Baz } from '...' | ||
| for (const m of content.matchAll(/export\s+(?:type\s+)?\{([^}]+)\}/g)) { | ||
| for (const name of m[1].split(',')) { | ||
| const clean = name.replace(/\s+as\s+\w+/, '').replace(/type\s+/, '').trim(); | ||
| if (clean && !clean.startsWith('//')) names.push(clean); | ||
| } | ||
| } | ||
| // Match: export * from '...' | ||
| for (const m of content.matchAll(/export\s+\*\s+from\s+'([^']+)'/g)) { | ||
| names.push(`* from '${m[1]}'`); | ||
| } | ||
| return names; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Type-only exports in sdk/ts-compat/src/index.ts:"
rg -n 'export.*\btype\b' sdk/ts-compat/src/index.ts || true
echo
echo "Current validator logic in sdk/mcp/src/index.ts:"
rg -n 'replace\(/type|includes\(symbol\)|import \{ \$\{symbol\} \}' sdk/mcp/src/index.tsRepository: layer-3/nitrolite
Length of output: 1213
🏁 Script executed:
cat -n sdk/mcp/src/index.ts | sed -n '25,50p'Repository: layer-3/nitrolite
Length of output: 1247
🏁 Script executed:
cat -n sdk/mcp/src/index.ts | sed -n '435,460p'Repository: layer-3/nitrolite
Length of output: 1756
🏁 Script executed:
cat -n sdk/ts-compat/src/index.ts | head -20Repository: layer-3/nitrolite
Length of output: 925
🏁 Script executed:
cat sdk/ts-compat/src/index.ts | rg -A 2 'export.*type'Repository: layer-3/nitrolite
Length of output: 470
🏁 Script executed:
# Check for any tests related to validate_import or extractExports
fd -t f '\.(test|spec)\.(ts|js)$' sdk/mcp/src | head -5Repository: layer-3/nitrolite
Length of output: 43
🏁 Script executed:
# Check if there are any actual cases where substring matching would cause issues
grep -o 'export.*\b\w\+\b' sdk/ts-compat/src/index.ts | sort -u | head -30Repository: layer-3/nitrolite
Length of output: 510
🏁 Script executed:
# Get the complete type-only exports section
cat -n sdk/ts-compat/src/index.ts | sed -n '110,125p'Repository: layer-3/nitrolite
Length of output: 453
🏁 Script executed:
# Check for potential substring match issues - symbols that could be substrings of others
cat sdk/ts-compat/src/index.ts | rg -o '\b[A-Z][a-zA-Z0-9]*\b' | sort -u | head -40Repository: layer-3/nitrolite
Length of output: 677
🏁 Script executed:
# Test if the current logic would incorrectly match "Client" in "NitroliteClient"
cat sdk/ts-compat/src/index.ts | grep -E '(Client|Signer|Poller|Config)' | head -15Repository: layer-3/nitrolite
Length of output: 628
Refactor extractExports() to preserve type metadata and use exact symbol matching in validate_import().
The current implementation has two critical flaws:
-
Type-only exports lose semantics: The function strips the
typekeyword (line 36) without tracking it, so type-only exports likeAccountInfo,LedgerChannelfrom lines 110–117 will be recommended as value imports (import { AccountInfo }) instead of type imports (import type { AccountInfo }). -
Substring matching causes false positives: The validation logic uses
.includes(symbol)on line 444, so a query for"Client"will match"NitroliteClient"as a false positive. Real exports include symbols likeNitroliteClient,WalletStateSigner,EventPoller, andCompatClientConfigwhere partial matches would occur.
Fix: Store structured metadata {name: string, isType: boolean} in extractExports() and use exact equality matching in validate_import() to recommend the correct import form for both type and value exports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/mcp/src/index.ts` around lines 30 - 44, extractExports currently returns
only strings and strips the `type` keyword; change it to return structured
metadata like { name: string, isType: boolean } by detecting whether each export
token was declared as `type` (and preserving the original exported identifier
after removing `as` aliases), and keep the wildcard handling as a distinct
record. Then update validate_import to match symbols using exact equality
against the exported name (not .includes) and consult the isType flag to
recommend `import type { ... }` for type-only exports and regular `import { ...
}` for value exports; reference the functions extractExports and validate_import
when making these changes.
- Add npx tsx to settings.json allowlist (unblocks MCP server startup) - Add language tag to CLAUDE.md code block (markdownlint MD040) - Add MCP setup note to CLAUDE.md Important Notes section - Clarify /test skill repo-root behavior (Go-only, not full suite) - Fix MCP method parser to detect sync vs async return types - Replace Decimal usage with string amounts in MCP examples - Narrow search_api tool description to match implementation
There was a problem hiding this comment.
🧹 Nitpick comments (4)
sdk/mcp/src/index.ts (4)
454-455: SDK barrel is re-read on everyvalidate_importcall.Unlike
compatExportswhich is loaded once at startup, the SDK barrel is read from disk on every tool invocation. Cache it at startup for consistency and performance.♻️ Proposed fix to cache SDK exports at startup
const methods: MethodInfo[] = []; const types: TypeInfo[] = []; const compatExports: string[] = []; +const sdkExports: string[] = []; // ... existing load functions ... +function loadSdkExports(): void { + const content = readFile(resolve(SDK_ROOT, 'src/index.ts')); + sdkExports.push(...extractExports(content)); +} + loadClientMethods(); loadTypes(); loadCompatExports(); +loadSdkExports();Then in
validate_import:// Check if it's in the main SDK - const sdkBarrel = readFile(resolve(SDK_ROOT, 'src/index.ts')); - const inSdk = sdkBarrel.includes(symbol); + const inSdk = sdkExports.some(e => e === symbol);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/mcp/src/index.ts` around lines 454 - 455, The SDK barrel is being re-read on every validate_import call (the readFile(resolve(SDK_ROOT, 'src/index.ts')) that sets sdkBarrel); instead, read and cache the SDK barrel once at module initialization (like compatExports) into a top-level variable (e.g., sdkBarrelCached) and replace the per-call read with a lookup against that cached variable inside validate_import (referencing sdkBarrel -> sdkBarrelCached and validate_import) so subsequent invocations reuse the in-memory content for consistency and performance.
25-28: Consider wrappingreadFileSyncin try-catch for robustness.While
existsSyncguards against missing files,readFileSynccan still throw on permission errors or other I/O failures. Since startup failures are handled bymain().catch(), this is acceptable, but explicit error handling would provide clearer diagnostics.♻️ Optional fix for clearer error handling
function readFile(path: string): string { - if (!existsSync(path)) return ''; - return readFileSync(path, 'utf-8'); + try { + if (!existsSync(path)) return ''; + return readFileSync(path, 'utf-8'); + } catch (err) { + console.error(`Warning: Failed to read ${path}:`, err); + return ''; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/mcp/src/index.ts` around lines 25 - 28, The readFile function should guard against I/O errors from readFileSync by wrapping the call in a try-catch: keep the existsSync check, but call readFileSync inside a try block and in the catch either throw a new Error that includes the file path and original error (so main().catch() gets clear diagnostics) or log the error and return '' if you prefer non-fatal behavior; update the readFile function to catch (err) and include the original error message/stack when rethrowing so debugging is easier.
117-127: Regex patterns for interfaces/types are fragile with nested braces.The patterns
\{([^}]+)\}will truncate at the first}, breaking on interfaces with nested object types. This works for the current SDK structure (simple single-brace bodies), but could silently fail if interfaces gain nested type definitions.For a more robust approach, consider using the TypeScript compiler API or a simple brace-counting parser. For now, this is acceptable given the SDK's current type patterns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/mcp/src/index.ts` around lines 117 - 127, The current regexes used in content.matchAll for enums, interfaces, and type aliases (the patterns using \{([^}]+)\} and /export\s+type\s+.../) are fragile and break on nested braces; replace the regex-based extraction in the parsing loop (the blocks that push into types with name/m[1], kind, fields, source) with a small parser that scans the file and uses brace-counting to capture balanced { ... } bodies (or switch to the TypeScript compiler API to get exported enums/interfaces/types), then populate types with the same shape (name, kind, fields, source) using the correctly captured body so nested object types are handled safely.
410-441: Consider generating RPC method formats from a single source to prevent future staleness.The documented wire formats are currently accurate and match the parsing logic in
sdk/ts-compat/src/rpc.ts. However, hardcoding these mappings risks documentation drift as the protocol evolves. Consider centralizing the format definitions (e.g., insdk/ts-compat/src/types.ts) and either importing them here or documenting a reference to keep them synchronized.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/mcp/src/index.ts` around lines 410 - 441, The rpcMethods mapping in the server.tool('get_rpc_method') handler is hardcoded and can drift from the canonical RPC parsing/types in sdk/ts-compat/src/rpc.ts; refactor by moving the canonical request/response format definitions into a single source (e.g., sdk/ts-compat/src/types.ts or an exported constant in rpc.ts) and import that into this file instead of the local rpcMethods literal; update server.tool('get_rpc_method') to build its response from the shared definitions (keeping keys like ping, get_channels, transfer, create_app_session, etc.) so documentation stays synchronized with the parsing logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@sdk/mcp/src/index.ts`:
- Around line 454-455: The SDK barrel is being re-read on every validate_import
call (the readFile(resolve(SDK_ROOT, 'src/index.ts')) that sets sdkBarrel);
instead, read and cache the SDK barrel once at module initialization (like
compatExports) into a top-level variable (e.g., sdkBarrelCached) and replace the
per-call read with a lookup against that cached variable inside validate_import
(referencing sdkBarrel -> sdkBarrelCached and validate_import) so subsequent
invocations reuse the in-memory content for consistency and performance.
- Around line 25-28: The readFile function should guard against I/O errors from
readFileSync by wrapping the call in a try-catch: keep the existsSync check, but
call readFileSync inside a try block and in the catch either throw a new Error
that includes the file path and original error (so main().catch() gets clear
diagnostics) or log the error and return '' if you prefer non-fatal behavior;
update the readFile function to catch (err) and include the original error
message/stack when rethrowing so debugging is easier.
- Around line 117-127: The current regexes used in content.matchAll for enums,
interfaces, and type aliases (the patterns using \{([^}]+)\} and
/export\s+type\s+.../) are fragile and break on nested braces; replace the
regex-based extraction in the parsing loop (the blocks that push into types with
name/m[1], kind, fields, source) with a small parser that scans the file and
uses brace-counting to capture balanced { ... } bodies (or switch to the
TypeScript compiler API to get exported enums/interfaces/types), then populate
types with the same shape (name, kind, fields, source) using the correctly
captured body so nested object types are handled safely.
- Around line 410-441: The rpcMethods mapping in the
server.tool('get_rpc_method') handler is hardcoded and can drift from the
canonical RPC parsing/types in sdk/ts-compat/src/rpc.ts; refactor by moving the
canonical request/response format definitions into a single source (e.g.,
sdk/ts-compat/src/types.ts or an exported constant in rpc.ts) and import that
into this file instead of the local rpcMethods literal; update
server.tool('get_rpc_method') to build its response from the shared definitions
(keeping keys like ping, get_channels, transfer, create_app_session, etc.) so
documentation stays synchronized with the parsing logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d8adf8a-fbbe-4a77-92d0-4a96014e6622
📒 Files selected for processing (4)
.claude/settings.json.claude/skills/test/SKILL.mdCLAUDE.mdsdk/mcp/src/index.ts
✅ Files skipped from review due to trivial changes (3)
- .claude/settings.json
- CLAUDE.md
- .claude/skills/test/SKILL.md
2 participants
Summary
CLAUDE.md(root + SDK-specific),.claude/settings.json,.mcp.json,.gitignore.claude/rules/for TypeScript, Go, and Solidity conventions/test,/typecheck,/lint,/build-sdk,/review-pr— smart routing to correct toolchainsdk/mcp/): Exposes Nitrolite SDK API surface to AI agents/IDEs via Model Context Protocol — 8 resources, 5 tools, 2prompts
MCP Server Capabilities
lookup_method,lookup_type,search_api,get_rpc_method,validate_importcreate-channel-app,migrate-from-v053Test plan
CLAUDE.mdloads automatically/test sdk/tsroutes tocd sdk/ts && npm test/review-prreviews uncommitted changesnpx tsx sdk/mcp/src/index.tsstarts MCP server without errornpm run build/npm testinsdk/tsauto-allowed (no permission prompt)git push --forceblocked by deny listSummary by CodeRabbit
New Features
Documentation
Chores