chore: add Claude Code config + custom SDK MCP server#635
Merged
Conversation
ihsraham
requested review from
dimast-x,
nksazonov and
philanton
as code owners
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds AI-oriented tooling and documentation plus a new MCP server: language rules and Claude skills, repo-wide AI agent guidance, extensive protocol/SDK docs, and a TypeScript MCP server that indexes SDK/docs and exposes lookup/validation/scaffold tools over stdio. Changes
Sequence Diagram(s)sequenceDiagram
participant DevDocs as Docs/SDK Sources
participant Parser as MCP Indexer
participant McpServer as McpServer
participant Transport as StdioTransport
participant Client as MCP Client
Note right of DevDocs: `sdk/ts`, `sdk/go`, `docs/*`, `ts-compat`
DevDocs->>Parser: Read & parse TypeScript docs/types, Go signatures, protocol YAML/MD
Parser-->>McpServer: Register resources (methods, types, examples, tools)
McpServer->>Transport: Start stdio server (listen)
Client->>Transport: Send request (lookup/search/scaffold)
Transport->>McpServer: Deliver request
McpServer->>Parser: Query index / run tool (validation/scaffold)
Parser-->>McpServer: Return result
McpServer->>Transport: Respond with generated Markdown/JSON
Transport->>Client: Return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 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}
Contributor
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
ihsraham
added a commit
that referenced
this pull request
Mar 30, 2026
- 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
Contributor
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
philanton
approved these changes
Mar 30, 2026
ihsraham
added a commit
that referenced
this pull request
Apr 9, 2026
- 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
ihsraham
force-pushed
the
chore/claude-code-setup
branch
from
965afb0 to
71eaeaa
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
sdk/ts-mcp/src/index.ts (2)
88-113: Method parsing regex may miss some signatures.The regex on line 90 attempts to parse JSDoc + method signatures but has limitations:
- It captures
Promise<X>return types but may not handle nested generics likePromise<{foo: Bar}>- Multi-line parameters aren't fully captured since
[^)]*stops at the first)- The async detection logic (lines 102-103) checks a small window before the match, which could give false positives/negatives
This is acceptable for documentation purposes but may produce incomplete signatures for complex methods.
Consider adding a comment noting these limitations, or using a more robust TypeScript AST parser if higher fidelity is needed in the future.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/ts-mcp/src/index.ts` around lines 88 - 113, The regex in loadClientMethods that parses JSDoc and method signatures is brittle (the RegExp variable re and the surrounding async detection logic) and can miss nested generics, multiline parameter lists, and mis-detect async; update the code by either (a) adding a concise inline comment above loadClientMethods/re explaining these specific limitations (nested generics, multiline params, narrow async window) and recommending migrating to a TypeScript AST parser for full fidelity later, or (b) replace the ad-hoc regex approach with a proper TS AST parse of src/client.ts to extract method name, params and return type reliably (use the AST to populate methods array and maintain categorizeMethod usage); prefer the comment if you want a minimal change, otherwise implement the AST-based parsing for correctness.
1064-1095: Hardcoded RPC wire formats may drift from actual implementation.The
get_rpc_methodtool contains a hardcoded map of RPC method wire formats (lines 1069-1085). This duplicates information that exists in clearnode docs and could become stale. Unlikelookup_rpc_methodwhich reads from API.md, this provides static format strings.Consider:
- Adding a code comment explaining these are v0.5.3 compat-layer formats that are intentionally static
- Or consolidating with
lookup_rpc_methodto reduce duplicationThe current approach works but requires manual updates if wire formats change.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/ts-mcp/src/index.ts` around lines 1064 - 1095, The get_rpc_method tool currently hardcodes RPC wire formats in the rpcMethods map inside the server.tool handler named 'get_rpc_method', which can drift from docs; either (A) add a clear code comment above rpcMethods stating these strings are intentionally the v0.5.3 compat-layer static formats (so future maintainers know they must be updated manually), or (B) refactor get_rpc_method to reuse the existing lookup_rpc_method logic (call or import lookup_rpc_method or the same API.md parsing used by lookup_rpc_method) to derive formats dynamically; update references to rpcMethods and any returned text construction in the 'get_rpc_method' handler accordingly.sdk/mcp-go/main.go (1)
194-201: HardcodedNewClientsignature may drift from actual SDK implementation.The signature for
NewClientis manually specified here rather than being parsed from the source like other methods. If the actual SDK signature changes, this documentation will become stale.Consider adding a comment noting this is manually maintained, or extending
loadGoSDKMethodsto also parse standalone functions likeNewClientfrom the source.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/mcp-go/main.go` around lines 194 - 201, The code hardcodes the NewClient signature instead of parsing it from source, so update loadGoSDKMethods to also detect standalone top-level functions (like NewClient) or, if you prefer the minimal change, add an explicit TODO comment above the NewClient block noting the signature is manually maintained and must be updated when SDK changes; if implementing parsing, extend the existing parsing routine used for methods to also scan for "func NewClient(" declarations and populate methodInfo with Name "NewClient", Signature extracted from the source, Comment and Category, ensuring you reuse existing parsing helpers to avoid duplication and keep behavior consistent with other parsed methods.sdk/mcp-go/README.md (1)
41-106: Consider adding language specifiers to fenced code blocks.The example output blocks showing tool request/response pairs lack language specifiers. While these aren't traditional code, adding a language tag (e.g.,
textorplaintext) would satisfy markdown linters and improve consistency.This is a minor lint warning (MD040) and doesn't affect functionality. The examples are clear and well-documented as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/mcp-go/README.md` around lines 41 - 106, The fenced example blocks (e.g., the lookup_method blocks containing lookup_method({ name: "Deposit" }), lookup_method({ name: "Transfer" }), lookup_method({ name: "CreateAppSession" }), lookup_method({ name: "SubmitAppState" }), and lookup_method({ name: "Checkpoint" })) should include a language specifier to satisfy markdown linters; update each triple-backtick fence to use a neutral tag like ```text or ```plaintext for those tool request/response examples so the README.md passes MD040 and remains consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@llms-full.txt`:
- Around line 177-213: Summary: Example imports and method list are inconsistent
with the real SDK API. Fix the example by replacing the incorrect
NitroliteClient usage with the SDK factory pattern: use createSigners and
Client.create (and the withBlockchainRPC helper) instead of importing
NitroliteClient; locate the example import/instantiation that references
NitroliteClient and update to use createSigners, Client.create, and
withBlockchainRPC. Also remove the nonexistent closeAppSession entry from the
key methods table and clarify that closing an app session is performed via
submitAppState with a Close intent (replace the closeAppSession row with a note
that submitAppState handles session closure); reference symbols:
NitroliteClient, createSigners, Client.create, withBlockchainRPC,
closeAppSession, submitAppState.
In `@sdk/mcp-go/main.go`:
- Around line 58-74: The findRepoRoot function currently ignores the error
returned by os.Getwd(), which can leave dir empty and break the upward
traversal; update findRepoRoot to capture the error from os.Getwd(), handle it
(e.g., if err != nil return the existing fallback path filepath.Join(".", "..",
"..") or propagate the error up by changing the signature), and ensure you don't
proceed with an empty dir before the loop; modify the function body around the
os.Getwd() call and any return path so error handling is explicit and safe.
In `@sdk/ts-mcp/README.md`:
- Around line 128-170: Add a new README section documenting the
lookup_rpc_method tool (alongside get_rpc_method) describing that
lookup_rpc_method (exported from index.ts) returns clearnode RPC metadata pulled
from API.md including method name, description, access level, and parameters;
include a usage example block showing a call like lookup_rpc_method({ method:
"create_channel" }) and sample formatted output (method, description, access
level, request/response shapes) matching the style and verbosity used for
get_rpc_method so readers can see how the two tools differ and when to use each.
- Around line 29-31: Add a README section documenting the missing tool
lookup_rpc_method: explain what lookup_rpc_method does, show a short example of
calling it (input and expected output), and explicitly contrast it with
get_rpc_method (e.g., lookup_rpc_method performs a broader search/heuristic
lookup across service/protocol metadata while get_rpc_method fetches a single
method by exact identifier). Update the "8 tools" list to include
lookup_rpc_method and ensure the examples mirror the style used for the other
tools so readers can see usage and the key behavioral difference from
get_rpc_method.
---
Nitpick comments:
In `@sdk/mcp-go/main.go`:
- Around line 194-201: The code hardcodes the NewClient signature instead of
parsing it from source, so update loadGoSDKMethods to also detect standalone
top-level functions (like NewClient) or, if you prefer the minimal change, add
an explicit TODO comment above the NewClient block noting the signature is
manually maintained and must be updated when SDK changes; if implementing
parsing, extend the existing parsing routine used for methods to also scan for
"func NewClient(" declarations and populate methodInfo with Name "NewClient",
Signature extracted from the source, Comment and Category, ensuring you reuse
existing parsing helpers to avoid duplication and keep behavior consistent with
other parsed methods.
In `@sdk/mcp-go/README.md`:
- Around line 41-106: The fenced example blocks (e.g., the lookup_method blocks
containing lookup_method({ name: "Deposit" }), lookup_method({ name: "Transfer"
}), lookup_method({ name: "CreateAppSession" }), lookup_method({ name:
"SubmitAppState" }), and lookup_method({ name: "Checkpoint" })) should include a
language specifier to satisfy markdown linters; update each triple-backtick
fence to use a neutral tag like ```text or ```plaintext for those tool
request/response examples so the README.md passes MD040 and remains consistent.
In `@sdk/ts-mcp/src/index.ts`:
- Around line 88-113: The regex in loadClientMethods that parses JSDoc and
method signatures is brittle (the RegExp variable re and the surrounding async
detection logic) and can miss nested generics, multiline parameter lists, and
mis-detect async; update the code by either (a) adding a concise inline comment
above loadClientMethods/re explaining these specific limitations (nested
generics, multiline params, narrow async window) and recommending migrating to a
TypeScript AST parser for full fidelity later, or (b) replace the ad-hoc regex
approach with a proper TS AST parse of src/client.ts to extract method name,
params and return type reliably (use the AST to populate methods array and
maintain categorizeMethod usage); prefer the comment if you want a minimal
change, otherwise implement the AST-based parsing for correctness.
- Around line 1064-1095: The get_rpc_method tool currently hardcodes RPC wire
formats in the rpcMethods map inside the server.tool handler named
'get_rpc_method', which can drift from docs; either (A) add a clear code comment
above rpcMethods stating these strings are intentionally the v0.5.3 compat-layer
static formats (so future maintainers know they must be updated manually), or
(B) refactor get_rpc_method to reuse the existing lookup_rpc_method logic (call
or import lookup_rpc_method or the same API.md parsing used by
lookup_rpc_method) to derive formats dynamically; update references to
rpcMethods and any returned text construction in the 'get_rpc_method' handler
accordingly.
🪄 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: 7d631942-4da2-4def-81d0-881a7307f0e7
⛔ Files ignored due to path filters (2)
sdk/mcp-go/go.sumis excluded by!**/*.sumsdk/ts-mcp/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
.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.cursorrules.github/copilot-instructions.md.gitignore.mcp.json.windsurfrulesCLAUDE.mdllms-full.txtllms.txtsdk/go/CLAUDE.mdsdk/mcp-go/.gitignoresdk/mcp-go/README.mdsdk/mcp-go/go.modsdk/mcp-go/main.gosdk/ts-compat/CLAUDE.mdsdk/ts-mcp/.gitignoresdk/ts-mcp/README.mdsdk/ts-mcp/package.jsonsdk/ts-mcp/src/index.tssdk/ts-mcp/tsconfig.jsonsdk/ts/CLAUDE.md
✅ Files skipped from review due to trivial changes (23)
- .gitignore
- sdk/mcp-go/.gitignore
- .claude/rules/solidity.md
- .claude/rules/go.md
- .mcp.json
- sdk/mcp-go/go.mod
- .claude/skills/review-pr/SKILL.md
- sdk/ts-mcp/.gitignore
- .windsurfrules
- .claude/rules/typescript.md
- .claude/skills/typecheck/SKILL.md
- .github/copilot-instructions.md
- llms.txt
- sdk/ts-mcp/package.json
- .cursorrules
- .claude/skills/lint/SKILL.md
- CLAUDE.md
- sdk/ts/CLAUDE.md
- sdk/ts-mcp/tsconfig.json
- sdk/go/CLAUDE.md
- sdk/ts-compat/CLAUDE.md
- .claude/skills/build-sdk/SKILL.md
- .claude/settings.json
🚧 Files skipped from review as they are similar to previous changes (1)
- .claude/skills/test/SKILL.md
nksazonov
reviewed
Apr 9, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
llms-full.txt (2)
149-176:⚠️ Potential issue | 🟠 MajorRPC method table mixes incompatible method models.
Line 170 documents
close_app_session, while Line 226 says session close is via app-state close intent. Keep one canonical model in the table (prefer exact method names from the active v1 router/docs) to prevent tool confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@llms-full.txt` around lines 149 - 176, The RPC method table mixes two incompatible models — e.g., it lists close_app_session while elsewhere close is handled via an app-state close intent — so update the table to use the single canonical v1 router method names from the active docs: remove or replace legacy/ambiguous entries (like close_app_session) with the exact v1 methods (e.g., app_sessions.v1.submit_session_key_state, app_sessions.v1.get_last_key_states or the documented app-state close intent name), ensure method names match the active router docs exactly, and eliminate duplicate/conflicting descriptions to keep one consistent model.
84-90:⚠️ Potential issue | 🟠 MajorAuth/JWT flow appears out of sync with current RPC surface.
Line 84-Line 90 and Line 151-Line 153 describe
auth_request/auth_challenge/auth_verify+ JWT issuance, but the registered methods shown inclearnode/api/rpc_router.go:111-150do not include these handlers. Please align this section to the currently exposed v1 RPC methods to avoid invalid client scaffolding.Also applies to: 151-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@llms-full.txt` around lines 84 - 90, The docs describe an auth_request/auth_challenge/auth_verify challenge-response + JWT flow that doesn't match the actual registered RPC handlers; either update the documentation to reflect the current v1 RPC surface or implement the missing handlers. Specifically, reconcile the auth flow with the RPC registration code (the function that registers v1 methods, e.g., registerV1RPCMethods or RegisterRPCHandlers in rpc_router.go) by either (A) adding and wiring auth_request, auth_challenge, and auth_verify handlers that issue JWTs, ensuring they are exported/registered with the same method names, or (B) removing/revising those auth_* entries from the docs and replacing them with the exact RPC method names currently registered; update any references to JWT issuance to match the actual handler that issues tokens.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@llms-full.txt`:
- Around line 68-76: The docs show 4-element request/response tuples but the
implementation and envelope (pkg/rpc/message.go) use a 5-element tuple including
Type, causing mismatched frames; update the documented JSON examples for
"Request Format" and "Response Format" to include the leading TYPE element so
they read { "req": [TYPE, REQUEST_ID, METHOD, PARAMS, TIMESTAMP], "sig": [...] }
and { "res": [TYPE, REQUEST_ID, METHOD, DATA, TIMESTAMP], "sig": [...] }
respectively, and ensure any human-readable examples reference the same TYPE
position as the Type field used in pkg/rpc/message.go and any code that
constructs frames (e.g., places where req/res tuples are assembled).
---
Duplicate comments:
In `@llms-full.txt`:
- Around line 149-176: The RPC method table mixes two incompatible models —
e.g., it lists close_app_session while elsewhere close is handled via an
app-state close intent — so update the table to use the single canonical v1
router method names from the active docs: remove or replace legacy/ambiguous
entries (like close_app_session) with the exact v1 methods (e.g.,
app_sessions.v1.submit_session_key_state, app_sessions.v1.get_last_key_states or
the documented app-state close intent name), ensure method names match the
active router docs exactly, and eliminate duplicate/conflicting descriptions to
keep one consistent model.
- Around line 84-90: The docs describe an
auth_request/auth_challenge/auth_verify challenge-response + JWT flow that
doesn't match the actual registered RPC handlers; either update the
documentation to reflect the current v1 RPC surface or implement the missing
handlers. Specifically, reconcile the auth flow with the RPC registration code
(the function that registers v1 methods, e.g., registerV1RPCMethods or
RegisterRPCHandlers in rpc_router.go) by either (A) adding and wiring
auth_request, auth_challenge, and auth_verify handlers that issue JWTs, ensuring
they are exported/registered with the same method names, or (B)
removing/revising those auth_* entries from the docs and replacing them with the
exact RPC method names currently registered; update any references to JWT
issuance to match the actual handler that issues tokens.
🪄 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: 0e4cd2d7-3b00-45ce-9c83-ada0ce9a2e60
📒 Files selected for processing (10)
.claude/rules/solidity.md.claude/skills/test/SKILL.md.cursorrules.github/copilot-instructions.md.windsurfrulesCLAUDE.mdllms-full.txtsdk/mcp-go/README.mdsdk/mcp-go/main.gosdk/ts-mcp/README.md
✅ Files skipped from review due to trivial changes (7)
- .claude/rules/solidity.md
- .cursorrules
- CLAUDE.md
- sdk/ts-mcp/README.md
- sdk/mcp-go/main.go
- .windsurfrules
- sdk/mcp-go/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .claude/skills/test/SKILL.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@llms-full.txt`:
- Line 173: Remove the `close_app_session` row from the RPC methods table so the
documentation matches the actual RPC registry; specifically delete the
`close_app_session` entry and ensure the table lists only the implemented
methods (e.g.,
SubmitDepositState/SubmitAppState/CreateAppSession/GetAppDefinition/GetAppSessions/SubmitSessionKeyState/GetLastKeyStates
and conditionally RebalanceAppSessions) and mention that app session termination
is performed via `submitAppState`/`SubmitAppState` with a Close intent rather
than a `close_app_session` RPC.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@llms-full.txt`:
- Around line 372-377: The example flow is missing the on-chain settlement
steps: after calling Deposit and after calling Transfer you must invoke the
Checkpoint method to perform settlement; update the sequence around the existing
Deposit, Transfer, and CloseHomeChannel calls so it calls Checkpoint(ctx) (or
the appropriate client.Checkpoint(ctx)) immediately after Deposit and again
after Transfer before CloseHomeChannel to follow the documented two-step pattern
(references: Deposit, Transfer, Checkpoint, CloseHomeChannel).
In `@sdk/ts-mcp/src/index.ts`:
- Around line 589-617: The JSON examples for quorum patterns use "weight" but
the type AppDefinitionV1 / AppParticipantV1 defines the field as
signatureWeight; update each example to use "signatureWeight" (or explicitly
label the examples as pseudocode) so the snippets match the AppDefinitionV1
shape; ensure references to participants, quorum, and nonce remain consistent
and that any documentation mentioning AppParticipantV1 or signatureWeight uses
the exact property name.
- Around line 920-922: The deposit is initiated with client.deposit(CHAIN_ID,
'usdc', new Decimal(10)) but the resulting on-chain state isn't settled; after
the deposit call, invoke and await the checkpoint operation (e.g.,
client.checkpoint(CHAIN_ID)) to finalize the deposit on-chain before proceeding
to create the app session or any subsequent operations that rely on the settled
balance.
- Around line 1358-1361: The .env.example generation always starts with
"PRIVATE_KEY" which mismatches the ai-agent scaffold that expects
"AGENT_PRIVATE_KEY"; update the envExample construction in
sdk/ts-mcp/src/index.ts (the variable envExample and the surrounding
template-based logic) to emit the correct key name based on the selected
template: emit "AGENT_PRIVATE_KEY=0x..." when template === 'ai-agent', emit
"PRIVATE_KEY=0x..." for templates that actually use PRIVATE_KEY (e.g.,
transfer-app or default), and keep the other conditional lines (RECIPIENT,
PEER_ADDRESS, CLEARNODE_URL, RPC_URL) unchanged so the generated .env.example
matches the scaffolded source.
🪄 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: 6f7d1e6b-cfb3-4c18-aa96-f6a107afc895
📒 Files selected for processing (8)
.cursorrules.github/copilot-instructions.md.windsurfrulesCLAUDE.mdllms-full.txtllms.txtsdk/mcp-go/main.gosdk/ts-mcp/src/index.ts
✅ Files skipped from review due to trivial changes (4)
- llms.txt
- CLAUDE.md
- .github/copilot-instructions.md
- sdk/mcp-go/main.go
🚧 Files skipped from review as they are similar to previous changes (2)
- .cursorrules
- .windsurfrules
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
sdk/ts-mcp/src/index.ts (1)
1269-1310:⚠️ Potential issue | 🟠 MajorFund the channel before the app-session scaffold uses allocations.
Lines 1274-1309 create an app session and submit
usdcallocations, but the generated starter never deposits or checkpoints any funds first. The full app-session example in this same file already does that on Lines 920-922, so the scaffolded project won't be runnable as-is.Proposed fix
async function main() { const { stateSigner, txSigner } = createSigners(PRIVATE_KEY); const myAddress = stateSigner.address; const client = await Client.create(CLEARNODE_URL, stateSigner, txSigner, withBlockchainRPC(CHAIN_ID, RPC_URL)); + + // Fund the channel before creating the app session + await client.deposit(CHAIN_ID, 'usdc', new Decimal(10)); + await client.checkpoint('usdc'); // Define app session const definition: app.AppDefinitionV1 = {Based on learnings, Key SDK methods include: Deposit, Transfer, Checkpoint (required after deposit/withdraw/close), CloseHomeChannel (follow with Checkpoint), CreateAppSession, SubmitAppState.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/ts-mcp/src/index.ts` around lines 1269 - 1310, The snippet creates an app session with usdc allocations but never funds or checkpoints the home channel first, so add calls to deposit and checkpoint before createAppSession and ensure proper checkpoint/close after closing the app: use client.deposit(...) to deposit the required usdc amounts for participants (matching allocations), then call client.checkpoint(...) to persist the deposit, then proceed to client.createAppSession(...) and client.submitAppState(...); after submitting the Close intent, call client.closeHomeChannel(...) if needed and client.checkpoint(...) again to finalize balances. Ensure you reference the same asset string ('usdc') and the methods client.deposit, client.checkpoint, client.createAppSession, client.submitAppState, and client.closeHomeChannel when adding these steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/ts-mcp/src/index.ts`:
- Around line 220-222: loadV1API() currently only finds the "api:" block by
searching for '\napi:\n', so if docs/api.yaml begins with "api:" on the first
line it returns early and leaves rpcMethodDocs empty; change the detection logic
in index.ts to treat an "api:" at the very start as valid—e.g., when apiStart is
-1, check content.startsWith('api:\n') (or use a regex /^api:\n/m) and set
apiStart accordingly before slicing; update the code paths around the apiStart
variable and the call to loadV1API() so rpcMethodDocs is populated and
protocol-rpc-methods / lookup_rpc_method retain indexed methods.
---
Duplicate comments:
In `@sdk/ts-mcp/src/index.ts`:
- Around line 1269-1310: The snippet creates an app session with usdc
allocations but never funds or checkpoints the home channel first, so add calls
to deposit and checkpoint before createAppSession and ensure proper
checkpoint/close after closing the app: use client.deposit(...) to deposit the
required usdc amounts for participants (matching allocations), then call
client.checkpoint(...) to persist the deposit, then proceed to
client.createAppSession(...) and client.submitAppState(...); after submitting
the Close intent, call client.closeHomeChannel(...) if needed and
client.checkpoint(...) again to finalize balances. Ensure you reference the same
asset string ('usdc') and the methods client.deposit, client.checkpoint,
client.createAppSession, client.submitAppState, and client.closeHomeChannel when
adding these steps.
🪄 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: a89c4f08-1bf1-4e57-9fb1-a6a5e1842e8b
📒 Files selected for processing (2)
llms-full.txtsdk/ts-mcp/src/index.ts
ihsraham
force-pushed
the
chore/claude-code-setup
branch
from
5e694e7 to
5a91cdd
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/mcp-test-plan.md`:
- Around line 13-27: Replace the machine-specific absolute path
"/Users/maharshimishra/Documents/nitrolite" and the chained cd commands that
leave the shell inside sdk/ with portable, repo-root-relative commands;
specifically, change the hardcoded cd to use the repo root (.) or no cd at all,
and replace "cd sdk/mcp-test-results && npm install && cd .." and "cd sdk/mcp &&
npm install && cd .." with either npm --prefix invocations or subshells (e.g.,
(cd sdk/mcp-test-results && npm install)) or pushd/popd so the working directory
isn’t mutated, and ensure the final run command stays correct: npm --prefix
sdk/mcp-test-results exec -- tsx sdk/mcp-test-results/run.ts.
- Around line 151-155: The acceptance rule for explain_concept("clearnode") in
sdk/mcp-test-plan.md conflicts with recorded behavior in
sdk/mcp-test-results/go-server.md; update the plan to match the recorded PASS
behavior by changing the requirement for explain_concept("clearnode") from
“meaningful response” to accept a graceful "not found" response (or
alternatively, if the intent is to require a meaningful response, update
sdk/mcp-test-results/go-server.md to record the server as failing for
clearnode); ensure you reference the explain_concept("clearnode") test entry and
adjust the acceptance criteria so both the plan and go-server results are
consistent.
In `@sdk/mcp-test-results/go-server.md`:
- Around line 450-456: The test result for explain_concept("clearnode")
currently marks "No concept matching 'clearnode' found" as PASS, which conflicts
with the test plan expectation in sdk/mcp-test-plan.md that a meaningful
explanation should be returned; fix by making the behavior consistent: either
update the test result block for explain_concept("clearnode") to FAIL (or
WARNING) and include the expected explanation text or remediation steps, or
modify the test plan entry for clearnode to allow "not found" as an acceptable
PASS; locate the explain_concept("clearnode") entry in the results file and the
corresponding clearnode expectation in sdk/mcp-test-plan.md and change one side
so both the result and plan match.
In `@sdk/THE_WAY_AHEAD.md`:
- Line 11: Update the resource count in the table row "Unified MCP server
(`sdk/mcp/`)" from 33 to 30 to match the current test plan/results in this PR;
locate the string "| Unified MCP server (`sdk/mcp/`) | Works locally, 33
resources, 8 tools, 3 prompts — covers TypeScript and Go SDKs |" in
sdk/THE_WAY_AHEAD.md and change the numeric value to 30 (and scan the file for
any other occurrences of "33 resources" to keep counts consistent).
🪄 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: 19231b60-5fb1-483a-80bf-c99b536c59c9
⛔ Files ignored due to path filters (2)
sdk/mcp-test-results/package-lock.jsonis excluded by!**/package-lock.jsonsdk/mcp/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
.mcp.jsonCLAUDE.mdsdk/THE_WAY_AHEAD.mdsdk/mcp-test-cases.mdsdk/mcp-test-plan.mdsdk/mcp-test-results/.gitignoresdk/mcp-test-results/go-server.mdsdk/mcp-test-results/package.jsonsdk/mcp-test-results/run.tssdk/mcp-test-results/ts-server.mdsdk/mcp/.gitignoresdk/mcp/README.mdsdk/mcp/package.jsonsdk/mcp/src/index.tssdk/mcp/tsconfig.jsonyellow-docs
✅ Files skipped from review due to trivial changes (3)
- sdk/mcp-test-results/.gitignore
- CLAUDE.md
- sdk/mcp-test-cases.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .mcp.json
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
- 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
…E context files Replace sdk/mcp/ with sdk/ts-mcp/ (TS) and sdk/mcp-go/ (Go) MCP servers. Both now emit correct, current API signatures verified against SDK source. Key fixes from review: - Go method regex: 11/44 → 44/44 public methods indexed - TS Client.create regex: now captures static async methods - All examples use createSigners + Decimal (not walletClient + strings) - RPC wire formats match clearnode/docs/API.md - validate_import uses exact match (no substring false positives) - App session examples use correct definition shape, quorum sigs, versioned updates - closeHomeChannel always followed by checkpoint() - No references to nonexistent closeAppSession() New files: - sdk/mcp-go/ — Go MCP server (19 resources, 5 tools, 2 prompts) - .cursorrules, .windsurfrules, .github/copilot-instructions.md - llms.txt, llms-full.txt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
llms-full.txt: - Rewrite auth section: emphasize request signing, EIP-712, JWT - Merge RPC table into single table with Access column (matches API.md) - Add security token locking methods to both SDK method tables - Expand state invariants with version uniqueness, allocation backing, no retrogression - Add migration transitions (InitiateMigration/FinalizeMigration) - Reference contracts/SECURITY.md for full invariant list Context files (.cursorrules, .windsurfrules, .github/copilot-instructions.md): - Fix SDK method signatures (quorumSigs, wallet params, no CloseAppSession) - Add key reference files section (protocol-description.md, SECURITY.md, ChannelHub.sol, clearnode docs) - Add Solidity style guide + development practices links - Fix .windsurfrules heading Code: - Handle os.Getwd() error in Go MCP findRepoRoot() - Fix Go test skill recipe (resolve package before -run) - Add npm install to CLAUDE.md TS build commands - Highlight TS-only in ts-mcp README - Document lookup_rpc_method tool in ts-mcp README Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rnode API The message envelope description said 5-element tuple with Type field, but the actual wire format uses 4-element tuples where the outer key (req/res) is the type discriminator. Fixed to match clearnode/docs/API.md.
The v1 source of truth is docs/api.yaml and docs/protocol/. clearnode/docs/API.md documents the 0.5.x compat surface, not v1. llms-full.txt: - Rewrite RPC table from docs/api.yaml with v1 grouped methods - Remove auth_request/auth_challenge/auth_verify (0.5.x flow) - Remove close_app_session from RPC table (not in v1 router) - Replace auth section with request signing description - Expand compat layer section explaining what it bridges and why Context files (.cursorrules, .windsurfrules, copilot-instructions.md): - Add v1 source of truth guidance (docs/api.yaml, not clearnode/docs/API.md) - Clarify sdk vs sdk-compat roles CLAUDE.md: - Add "V1 Protocol Source of Truth" section - Document sdk vs compat distinction - Warn against using clearnode/docs/API.md for v1 methods MCP servers: - Remove close_app_session from TS get_rpc_method tool - Mark get_rpc_method as 0.5.x compat tool, point to docs/api.yaml for v1 - Rewrite Go auth flow resource for v1 request signing model
…yaml v1 source
The MCP server was sourcing RPC method data and auth flow content from
clearnode/docs/API.md (0.5.x protocol). Now all protocol content comes
exclusively from docs/api.yaml and docs/protocol/, the canonical v1 sources.
…eight and env template - llms-full.txt: add Checkpoint() calls to Go transfer flow example - ts-mcp: fix quorum JSON examples to use signatureWeight (matches AppParticipantV1) - ts-mcp: add missing checkpoint() after deposit in app-session scaffold - ts-mcp: emit correct ANT_PRIVATE_KEY in .env.example for ai-agent template
- Rename sdk/ts-mcp → sdk/mcp; unified server covers both TypeScript and
Go SDKs (30 resources, 8 tools, 3 prompts)
- Add Go SDK method/type parsing (loadGoSdkMethods, loadGoTypes) reading
from sdk/go/ and pkg/ source files at startup
- Extend lookup_method, lookup_type, search_api with optional language
param ('typescript' | 'go' | 'both'); default 'typescript' preserves
existing behaviour
- Add go-* scaffold templates (go-transfer-app, go-app-session, go-ai-agent)
- Add Go-specific resources: nitrolite://go-api/methods, go-api/types,
go-examples/full-transfer-script, go-examples/full-app-session-script
- Add nitrolite://protocol/auth-flow resource (was missing from TS server)
- Expand create-channel-app and build-ai-agent-app prompts with Go sections
- Update .mcp.json: single 'nitrolite' entry pointing at sdk/mcp
- Update CLAUDE.md: unified row replacing two separate MCP server rows
- Add sdk/mcp-test-results/ harness (run.ts + package.json) covering all
115 TS + Go scenarios; 115/115 passing
- Add sdk/mcp-test-plan.md and sdk/mcp-test-cases.md vetting checklists
- Add sdk/THE_WAY_AHEAD.md: roadmap updated, Phase 2 Go binary replaced
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ihsraham
force-pushed
the
chore/claude-code-setup
branch
from
5a91cdd to
7e30cc4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (2)
sdk/THE_WAY_AHEAD.md (1)
9-12:⚠️ Potential issue | 🟡 MinorUpdate the unified-server resource count.
The server currently registers 30 resources, and both vetting reports record 30 as well, so “33 resources” is already stale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/THE_WAY_AHEAD.md` around lines 9 - 12, Update the resource count for the unified MCP server entry in THE_WAY_AHEAD.md: change the table cell that currently reads "33 resources" next to "Unified MCP server (`sdk/mcp/`)" to "30 resources" so it matches the actual registered count and the vetting reports; ensure the rest of the text ("8 tools, 3 prompts — covers TypeScript and Go SDKs") remains unchanged.sdk/mcp/src/index.ts (1)
434-437:⚠️ Potential issue | 🟠 MajorHandle
api:when it is the first line ofdocs/api.yaml.
content.indexOf('\napi:\n')still returns-1when the file starts withapi:on Line 1, which leavesrpcMethodDocsempty and breaks bothlookup_rpc_methodandnitrolite://protocol/rpc-methods.🔧 Minimal fix
- const apiStart = content.indexOf('\napi:\n'); - if (apiStart === -1) return; - const lines = content.slice(apiStart).split('\n'); + const apiStart = content.startsWith('api:\n') ? 0 : content.indexOf('\napi:\n'); + if (apiStart === -1) return; + const lines = content.slice(apiStart === 0 ? 0 : apiStart + 1).split('\n');🤖 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 434 - 437, The code uses content.indexOf('\napi:\n') to find the api section which fails when api: is the very first line; update the logic in the block that computes apiStart (and subsequently lines) to detect both cases by checking content.startsWith('api:\n') or searching for 'api:\n' (falling back to 0 when at start) so apiStart is set correctly; ensure rpcMethodDocs population code that uses apiStart and lines (and consumers like lookup_rpc_method and the nitrolite://protocol/rpc-methods generator) still reads from the correct slice when api: is at the top of the file.
🧹 Nitpick comments (1)
sdk/mcp/src/index.ts (1)
159-168: Use glob patterns instead of hardcoded file lists for Go type discovery.
loadGoSdkMethods()already scanssdk/go/client.goandsdk/go/node.go, butloadGoTypes()only covers a fixed subset of files and misses many exported types in both directories. This causes the Go type inventory exposed vianitrolite://go-api/typesand thelookup_typetool to silently drift from the actual SDK API surface. The initial check found exported types in:
pkg/core/: channel_signer.go, event.go, interface.go, session_key.go, state_advancer.go, state_packer.gopkg/app/: app_v1.go, session_key_v1.gopkg/rpc/: api.go, client.go, connection.go, context.go, dialer.go, error.go, message.go, methods.go, node.gosdk/go/: client.go, mock_dialer_test.goReplace the hardcoded
fileSetsarray with glob patterns for each directory root to maintain consistency withloadGoSdkMethods().🤖 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 159 - 168, The current fileSets array in loadGoTypes() enumerates a fixed list of files and misses many exported Go types; update loadGoTypes() to mirror loadGoSdkMethods() by replacing the hardcoded fileSets with glob patterns (e.g. resolve(PKG_ROOT, '**/*.go') and resolve(GO_SDK_ROOT, '**/*.go') or directory-root globs per pkg/core, pkg/app, pkg/rpc, sdk/go) so all .go files under those roots are scanned, ensuring loadGoTypes() discovers the same API surface as loadGoSdkMethods(); adjust any file-filtering logic around fileSets accordingly to still exclude vendor/test files if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@llms-full.txt`:
- Around line 352-353: Update the documentation table entries that list
`sdk/ts-mcp/` and `sdk/mcp-go/` to the unified path `sdk/mcp/`; locate the two
rows containing the markers `sdk/ts-mcp/` and `sdk/mcp-go/` and replace them
with a single row `sdk/mcp/` and the appropriate description ("MCP server for AI
agents") and language column (e.g., "Go/TypeScript" or a neutral label),
ensuring the table formatting remains consistent with surrounding rows.
In `@sdk/mcp-go/main.go`:
- Around line 843-848: The starter code is ignoring errors from
sign.NewEthereumMsgSigner, sign.NewEthereumRawSigner, sdk.NewClient, and
strconv.ParseUint; update the call sites to capture the returned error values,
check for non-nil errors, and propagate or handle them (return the error from
main/init or log and exit) instead of using `_`. Specifically, replace
constructs like "stateSigner, _ := sign.NewEthereumMsgSigner(agentPrivateKey)"
and "txSigner, _ := sign.NewEthereumRawSigner(agentPrivateKey)" with error-aware
variants that validate the signer and bail on failure, and change "client, _ :=
sdk.NewClient(...)" to capture err and handle it; do the same for any
strconv.ParseUint calls so invalid inputs produce a handled error rather than a
zero value. Ensure error checks return or exit with a clear message so invalid
keys/configurations do not yield nil client or silent defaults.
- Around line 508-510: The handlers handleSearchAPI, handleLookupMethod,
handleExplainConcept and handleLookupRPCMethod are allowing empty/whitespace
tool arguments to match everything because blank strings aren't validated (and
two handlers also omit TrimSpace); update each handler to TrimSpace the incoming
parameter (use strings.TrimSpace on req.GetString(...)), then reject when the
trimmed value == "" by returning a validation/invalid-argument error instead of
proceeding to perform substring matching; ensure you apply this to the specific
params used in those functions (e.g., the "query" param in handleSearchAPI) so
blank or missing inputs are rejected.
In `@sdk/mcp-test-results/run.ts`:
- Around line 173-179: The inventory checks currently allow loose matches;
update the check(...) calls (the calls with ID '0.1', '0.2', '0.3' that
reference tsResults, resources.resources.length, resourceUris,
tools.tools.length, toolNames, prompts.prompts.length and promptNames) to assert
exact counts rather than using minLength/contains. Replace the
free-text/count-in-string checks with explicit assertions: compare the numeric
lengths (resources.resources.length, tools.tools.length, prompts.prompts.length)
against the expected integers (e.g., 30, 8, 3) and change the check(...) options
to require equality (e.g., use an equals or exact match option on the count
field or make the expected message exactly "30 resources", "8 tools", "3
prompts"). Apply the same precise-count changes to the similar checks around the
other block mentioned (lines ~459-466).
- Around line 629-661: If connect() throws, the catch currently only logs and
leaves tsResults/goResults empty so failed === 0 and process exits 0; update the
catch around connect() to ensure the harness fails: capture the error in the
catch for connect(), log it (using e.message), and then either rethrow or set a
non-zero failure flag (e.g. set a local variable like connectionFailed = true or
increment failedCounter) so that process.exit is called with a non-zero code;
reference the connect() call, the catch block, the failed variable/logic, and
process.exit to locate where to set the failure state and ensure the process
exits non-zero when the server never starts.
In `@sdk/mcp/README.md`:
- Around line 35-122: The README contains unlabeled fenced code blocks (MD040)
for the examples and CLI snippets (e.g., the lookup_method, lookup_type,
search_api, scaffold_project examples and their inline command blocks), so add
appropriate language tags to each fence (for example "bash" or "sh" for
shell/REPL commands, "text" for plain output, "json" where applicable) so every
triple-backtick block in those sections has a language identifier to satisfy
markdownlint.
In `@sdk/mcp/src/index.ts`:
- Around line 615-675: The walkthrough skips funding the app session before
submitting a non-zero allocation, which will cause the SubmitAppState calls to
be rejected; after CreateAppSession (when you have sessionID) call the session
deposit helper (SubmitAppSessionDeposit or submitAppSessionDeposit) to commit
funds into the app channel (e.g., deposit USDC amount matching your intended
allocations) and wait for its confirmation before calling SubmitAppState with
non-zero Allocations; update the sequences around CreateAppSession, sessionID,
SubmitAppSessionDeposit, and SubmitAppState so every example includes the
deposit step (matching the TS example that deposits 10 USDC prior to submitting
the 15/5 allocation).
- Around line 112-131: The current regex in variable re is not matching access
modifiers before extracting JSDoc/methods and thus can splice helper text into
docs; update the pattern (or add a pre-check) to detect and skip methods with
access modifiers by matching an optional access modifier group
(?:private|protected|public)\s+ between the JSDoc and the method declaration
(i.e. change re to include (?:\s*(?:private|protected|public)\s+)? before
(?:static\s+)?(?:async\s+)? or, alternatively, inspect the substring between
match.index and match.index+match[0].length for those keywords and continue the
loop when found) so that categorizeMethod, isAsync detection, and the
methods.push entry only run for public methods.
- Around line 821-823: After calling client.Deposit(ctx, 11155111, "usdc",
decimal.NewFromInt(50)) you must immediately call the client's Checkpoint
operation to persist the signed state before proceeding; add a call to
client.Checkpoint(ctx) (or the appropriate Checkpoint signature used in this
SDK) right after Deposit, check its returned error and handle it (log.Fatal on
error) before logging "Agent funded with 50 USDC" or starting any payment
activity.
In `@sdk/THE_WAY_AHEAD.md`:
- Around line 14-15: Update the runtime input list used by the MCP server to
remove the old clearnode/docs/ path and include docs/api.yaml instead: find the
logic that enumerates runtime sources (look for references to "clearnode/docs/"
or the runtime input list in the MCP server initialization or embedding setup)
and replace that entry with "docs/api.yaml" so the server reads protocol
markdown plus docs/api.yaml as the canonical v1 API definitions for RPC methods,
types, and schemas.
---
Duplicate comments:
In `@sdk/mcp/src/index.ts`:
- Around line 434-437: The code uses content.indexOf('\napi:\n') to find the api
section which fails when api: is the very first line; update the logic in the
block that computes apiStart (and subsequently lines) to detect both cases by
checking content.startsWith('api:\n') or searching for 'api:\n' (falling back to
0 when at start) so apiStart is set correctly; ensure rpcMethodDocs population
code that uses apiStart and lines (and consumers like lookup_rpc_method and the
nitrolite://protocol/rpc-methods generator) still reads from the correct slice
when api: is at the top of the file.
In `@sdk/THE_WAY_AHEAD.md`:
- Around line 9-12: Update the resource count for the unified MCP server entry
in THE_WAY_AHEAD.md: change the table cell that currently reads "33 resources"
next to "Unified MCP server (`sdk/mcp/`)" to "30 resources" so it matches the
actual registered count and the vetting reports; ensure the rest of the text ("8
tools, 3 prompts — covers TypeScript and Go SDKs") remains unchanged.
---
Nitpick comments:
In `@sdk/mcp/src/index.ts`:
- Around line 159-168: The current fileSets array in loadGoTypes() enumerates a
fixed list of files and misses many exported Go types; update loadGoTypes() to
mirror loadGoSdkMethods() by replacing the hardcoded fileSets with glob patterns
(e.g. resolve(PKG_ROOT, '**/*.go') and resolve(GO_SDK_ROOT, '**/*.go') or
directory-root globs per pkg/core, pkg/app, pkg/rpc, sdk/go) so all .go files
under those roots are scanned, ensuring loadGoTypes() discovers the same API
surface as loadGoSdkMethods(); adjust any file-filtering logic around fileSets
accordingly to still exclude vendor/test files if needed.
🪄 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: 67c3d40b-4c9e-4b1c-ad54-7d1a3f3d7d82
⛔ Files ignored due to path filters (3)
sdk/mcp-go/go.sumis excluded by!**/*.sumsdk/mcp-test-results/package-lock.jsonis excluded by!**/package-lock.jsonsdk/mcp/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (37)
.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.cursorrules.github/copilot-instructions.md.gitignore.mcp.json.windsurfrulesCLAUDE.mdllms-full.txtllms.txtsdk/THE_WAY_AHEAD.mdsdk/go/CLAUDE.mdsdk/mcp-go/.gitignoresdk/mcp-go/README.mdsdk/mcp-go/go.modsdk/mcp-go/main.gosdk/mcp-test-cases.mdsdk/mcp-test-plan.mdsdk/mcp-test-results/.gitignoresdk/mcp-test-results/go-server.mdsdk/mcp-test-results/package.jsonsdk/mcp-test-results/run.tssdk/mcp-test-results/ts-server.mdsdk/mcp/.gitignoresdk/mcp/README.mdsdk/mcp/package.jsonsdk/mcp/src/index.tssdk/mcp/tsconfig.jsonsdk/ts-compat/CLAUDE.mdsdk/ts/CLAUDE.md
✅ Files skipped from review due to trivial changes (27)
- sdk/mcp-test-results/.gitignore
- sdk/mcp/.gitignore
- sdk/mcp-go/.gitignore
- .gitignore
- .claude/rules/solidity.md
- .mcp.json
- sdk/mcp-test-results/package.json
- .claude/rules/typescript.md
- .claude/skills/typecheck/SKILL.md
- .claude/rules/go.md
- sdk/mcp-go/go.mod
- sdk/mcp/tsconfig.json
- .claude/settings.json
- .claude/skills/build-sdk/SKILL.md
- llms.txt
- .claude/skills/lint/SKILL.md
- sdk/mcp/package.json
- CLAUDE.md
- sdk/mcp-test-plan.md
- sdk/go/CLAUDE.md
- sdk/ts/CLAUDE.md
- sdk/ts-compat/CLAUDE.md
- sdk/mcp-go/README.md
- .cursorrules
- .github/copilot-instructions.md
- sdk/mcp-test-cases.md
- .windsurfrules
🚧 Files skipped from review as they are similar to previous changes (2)
- .claude/skills/review-pr/SKILL.md
- .claude/skills/test/SKILL.md
philanton
reviewed
Apr 14, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
sdk/mcp/src/index.ts (3)
597-690:⚠️ Potential issue | 🟠 MajorFund the home channel before these app-session starter flows.
These snippets jump to
SubmitAppSessionDeposit/submitAppSessionDeposit, but they never first put funds into the caller's channel. The full TypeScript example later in this file already doesdeposit()+checkpoint()before the app-session flow, so these shorter examples/templates currently teach a path that has no channel balance to commit into the session.Also applies to: 734-815, 1013-1075, 1997-2061
🤖 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 597 - 690, The examples jump straight to CreateAppSession / SubmitAppSessionDeposit without ensuring the caller's home channel has on-chain funds; before calling client.CreateAppSession and client.SubmitAppSessionDeposit (or submitAppSessionDeposit), add the same home-channel funding steps used in the TypeScript example — perform a deposit() (on the client/home channel) and a checkpoint() so the channel balance is committed — or call a helper like EnsureHomeChannelFunded; update all affected snippets (the blocks around CreateAppSession/SubmitAppSessionDeposit at the listed ranges) to include these calls prior to submitting app-session deposits.
1018-1050:⚠️ Potential issue | 🟡 MinorAdd the missing
Decimalimport to the first app-session snippet.This block uses
new Decimal(...)on Line 1045, Line 1046, and Line 1050, but only importsapp, so the copied example will not type-check.Minimal fix
-import { app } from '@yellow-org/sdk'; +import { app } from '@yellow-org/sdk'; +import Decimal from 'decimal.js';🤖 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 1018 - 1050, The example uses new Decimal(...) but never imports Decimal, causing the snippet to fail type-check; add the Decimal import at the top of the snippet (e.g., import Decimal from 'decimal.js' or your project's Decimal module) so the usage in the AppDefinition/AppStateUpdate example (symbols: AppDefinitionV1, AppStateUpdateV1, createAppSession, submitAppSessionDeposit) compiles correctly.
35-49:⚠️ Potential issue | 🟠 MajorKeep
typeexports typed in the barrel index.
extractExports()removes thetypequalifier, sovalidate_import()can only emitimport { Foo } ...even when the barrel only exportsFooas a type. That makes the MCP guidance wrong for type-only symbols fromsdk-compatand the main SDK.#!/bin/bash set -euo pipefail echo "Type-only exports in sdk/ts-compat:" rg -n 'export\s+type\s+\{' sdk/ts-compat/src/index.ts || true echo echo "Type-only exports in sdk/ts:" rg -n 'export\s+type\s+\{' sdk/ts/src/index.ts || true echo echo "Current extractor/validator implementation:" sed -n '35,49p' sdk/mcp/src/index.ts echo sed -n '1831,1847p' sdk/mcp/src/index.tsAlso applies to: 1831-1847
🤖 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 35 - 49, The extractor extractExports currently strips the "type" qualifier so downstream validate_import and guidance produce value imports for symbols that are type-only; update extractExports to preserve the "type" token (e.g., capture optional "type" in the regex and include it in the pushed name like "type Foo") so exported names retain their type-only status, and apply the same preservation in the other extractor instance around the validate_import-related logic so type-only exports from sdk-compat/sdk are emitted as "type" imports by validate_import.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/mcp/src/index.ts`:
- Around line 1941-1945: The go.mod scaffold currently pins nitrolite to the
non-existent stub version "github.com/layer-3/nitrolite v0.0.0" (constructed in
the goMod template variable); update the goMod generation to use a real
published release (e.g. "v1.2.0") or a configurable constant so generated
projects can run `go mod tidy`; modify the goMod string in the code that builds
goMod (referencing goMod, template, and goTemplateMap) to include the real
version (or read it from a new constant like NITROLITE_VERSION) and ensure the
returned text uses that updated goMod value.
In `@sdk/THE_WAY_AHEAD.md`:
- Around line 87-93: The guidance incorrectly suggests adding a postbuild step
to inject a shebang into dist/index.js because tsc strips shebangs; instead
remove that postbuild workaround from THE_WAY_AHEAD.md and update the note to
state that TypeScript preserves shebangs from source files (e.g.,
sdk/mcp/src/index.ts contains #!/usr/bin/env node and will be emitted to
dist/index.js), and optionally mention tsup as an alternative build tool that
also handles shebangs—not as a required fix; update references to dist/index.js
and sdk/mcp/src/index.ts accordingly.
---
Duplicate comments:
In `@sdk/mcp/src/index.ts`:
- Around line 597-690: The examples jump straight to CreateAppSession /
SubmitAppSessionDeposit without ensuring the caller's home channel has on-chain
funds; before calling client.CreateAppSession and client.SubmitAppSessionDeposit
(or submitAppSessionDeposit), add the same home-channel funding steps used in
the TypeScript example — perform a deposit() (on the client/home channel) and a
checkpoint() so the channel balance is committed — or call a helper like
EnsureHomeChannelFunded; update all affected snippets (the blocks around
CreateAppSession/SubmitAppSessionDeposit at the listed ranges) to include these
calls prior to submitting app-session deposits.
- Around line 1018-1050: The example uses new Decimal(...) but never imports
Decimal, causing the snippet to fail type-check; add the Decimal import at the
top of the snippet (e.g., import Decimal from 'decimal.js' or your project's
Decimal module) so the usage in the AppDefinition/AppStateUpdate example
(symbols: AppDefinitionV1, AppStateUpdateV1, createAppSession,
submitAppSessionDeposit) compiles correctly.
- Around line 35-49: The extractor extractExports currently strips the "type"
qualifier so downstream validate_import and guidance produce value imports for
symbols that are type-only; update extractExports to preserve the "type" token
(e.g., capture optional "type" in the regex and include it in the pushed name
like "type Foo") so exported names retain their type-only status, and apply the
same preservation in the other extractor instance around the
validate_import-related logic so type-only exports from sdk-compat/sdk are
emitted as "type" imports by validate_import.
🪄 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: 800a947d-32ab-4134-bcc4-3a17cc6b51b3
📒 Files selected for processing (5)
.gitignorellms-full.txtsdk/THE_WAY_AHEAD.mdsdk/mcp/README.mdsdk/mcp/src/index.ts
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- sdk/mcp/README.md
philanton
approved these changes
Apr 14, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
sdk/mcp/src/index.ts (2)
293-318: Go method indexing requires documentation comments.The regex
((?:\/\/[^\n]*\n)+)requires at least one comment line before each method. Public methods without doc comments won't appear in the MCP index. If this is intentional (only index documented methods), no action needed. Otherwise, make the comment group optional:((?:\/\/[^\n]*\n)*)♻️ Optional fix to index undocumented methods
- const methodRe = /((?:\/\/[^\n]*\n)+)func \(c \*Client\) (\w+)\(([^)]*)\)\s*(.*)/g; + const methodRe = /((?:\/\/[^\n]*\n)*)func \(c \*Client\) (\w+)\(([^)]*)\)\s*(.*)/g;Then update lines 301-305 to handle empty comments:
const rawComment = match[1].trim(); - const comment = rawComment.split('\n') + const comment = rawComment ? rawComment.split('\n') .map(l => l.replace(/^\/\/ ?/, '').trim()) .filter(Boolean) - .join(' '); + .join(' ') : `${name} method`;🤖 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 293 - 318, The current regex methodRe requires at least one leading "//" comment so public Go methods without doc comments are skipped; make the comment capture optional by changing the pattern to allow zero comment lines (i.e., use ((?:\/\/[^\n]*\n)*)) and then update the parsing after match to handle an empty rawComment (set comment to empty string when rawComment is missing instead of splitting), ensuring goMethods still gets pushed with name, signature, comment ('' when none), and category (use category || categorizeGoMethod(name)).
377-388: Consider a more robust parser for complex TypeScript types.The interface regex
\{([^}]+)\}(line 381) stops at the first}, so interfaces with nested object types (e.g.,bar: { baz: string }) will be truncated. For documentation purposes this is often acceptable, but if you encounter missing fields in the MCP output, switching to TypeScript's compiler API (ts.createSourceFile+ AST traversal) would capture the full structure.🤖 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 377 - 388, The current parsing using content.matchAll with regexes (e.g., the interface pattern /\{([^}]+)\}/ in the loop that pushes into the types array) fails on nested object types; replace the regex-based extraction with a TypeScript Compiler API approach: use ts.createSourceFile to parse the file content, traverse the AST to find exported EnumDeclaration, InterfaceDeclaration and TypeAliasDeclaration nodes, and for each push the same shape into the existing types array (name, kind, fields as a string or reconstructed node text, and source); ensure you import 'typescript' and extract the full node text (or serialize members) so nested objects and complex type expressions are captured correctly instead of being truncated by the regex.
🤖 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 293-318: The current regex methodRe requires at least one leading
"//" comment so public Go methods without doc comments are skipped; make the
comment capture optional by changing the pattern to allow zero comment lines
(i.e., use ((?:\/\/[^\n]*\n)*)) and then update the parsing after match to
handle an empty rawComment (set comment to empty string when rawComment is
missing instead of splitting), ensuring goMethods still gets pushed with name,
signature, comment ('' when none), and category (use category ||
categorizeGoMethod(name)).
- Around line 377-388: The current parsing using content.matchAll with regexes
(e.g., the interface pattern /\{([^}]+)\}/ in the loop that pushes into the
types array) fails on nested object types; replace the regex-based extraction
with a TypeScript Compiler API approach: use ts.createSourceFile to parse the
file content, traverse the AST to find exported EnumDeclaration,
InterfaceDeclaration and TypeAliasDeclaration nodes, and for each push the same
shape into the existing types array (name, kind, fields as a string or
reconstructed node text, and source); ensure you import 'typescript' and extract
the full node text (or serialize members) so nested objects and complex type
expressions are captured correctly instead of being truncated by the regex.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6e4a669-2a08-485d-b071-0b19de2cf797
📒 Files selected for processing (2)
sdk/THE_WAY_AHEAD.mdsdk/mcp/src/index.ts
nksazonov
pushed a commit
that referenced
this pull request
Apr 16, 2026
## Summary
Consolidates the two separate MCP servers (`sdk/ts-mcp` and
`sdk/mcp-go`) into a single unified TypeScript server at `sdk/mcp` that
covers both the TypeScript and Go SDKs. Updates `.mcp.json` to a single
`nitrolite` entry.
### What changed
**`sdk/ts-mcp` → `sdk/mcp` (unified server)**
- 30 resources, 8 tools, 3 prompts — up from 25 TS + 19 Go across two
separate servers
- Go SDK method parsing (`loadGoSdkMethods`) reads `sdk/go/*.go` at
startup, indexes all `*Client` methods
- Go type parsing (`loadGoTypes`) reads `pkg/core`, `pkg/app`,
`pkg/rpc`, `sdk/go/*.go` for structs/enums
- `language` param added to `lookup_method`, `lookup_type`, `search_api`
(`'typescript'` default, `'go'`, `'both'`)
- Go scaffold templates: `go-transfer-app`, `go-app-session`,
`go-ai-agent`
- New resources: `nitrolite://go-api/methods`,
`nitrolite://go-api/types`,
`nitrolite://go-examples/full-transfer-script`,
`nitrolite://go-examples/full-app-session-script`,
`nitrolite://protocol/auth-flow`
- Expanded `create-channel-app` and `build-ai-agent-app` prompts with Go
sections alongside existing TS content
**`.mcp.json`** — single `nitrolite` entry replacing the previous
`nitrolite-ts` + `nitrolite-go` pair
**`CLAUDE.md`** — unified `sdk/mcp/` row replacing two separate MCP
server rows
### Correctness (from prior review rounds)
- All RPC formats sourced from `docs/api.yaml` v1 — not the legacy
`clearnode/docs/API.md`
- `CloseAppSession` absent from both TS and Go (method doesn't exist in
v1)
- TS default behaviour unchanged — `lookup_method({ name: 'transfer' })`
still returns TS `Decimal` signature with no Go bleed-through
- `validate_import` uses exact match (no substring false positives)
### Test harness
`sdk/mcp-test-results/run.ts` — 115 scenarios across TS (A–L) and Go (H,
F, G, J, K, L)
```
TOTAL: 115 passed, 0 failed, 115 total
```
Surface inventory verified: exactly 30 resources, 8 tools, 3 prompts.
### Follow-ups (not in this PR)
- `sdk/mcp-go/` deletion — can be removed after this merges
- `yellow-docs` docs update — separate PR in that repo
## Test plan
- [ ] `cd sdk/mcp && npm install && npm start` — server starts, logs 30
resources + 8 tools
- [ ] `lookup_method({ name: "transfer" })` → returns TS `Decimal`
signature
- [ ] `lookup_method({ name: "Transfer", language: "go" })` → returns Go
`func (c *Client) Transfer(...)`
- [ ] `lookup_method({ name: "closeAppSession" })` → "not found"
- [ ] `scaffold_project({ template: "go-transfer-app" })` → valid Go
code with `Checkpoint`, no TS sections
- [ ] `npm --prefix sdk/mcp-test-results exec -- tsx
sdk/mcp-test-results/run.ts` → 115/115 pass
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added a unified MCP server for AI assistants to browse protocol docs,
SDK references, and scaffolding tools.
* **Documentation**
* Added extensive guides covering protocol design, SDKs (TypeScript, Go,
Solidity), workflows, rules, and an AI reference.
* **Chores**
* Introduced repository settings, CI/automation skill definitions,
package config, and ignore rules to standardize developer tooling and
policies.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
3 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
Consolidates the two separate MCP servers (
sdk/ts-mcpandsdk/mcp-go) into a single unified TypeScript server atsdk/mcpthat covers both the TypeScript and Go SDKs. Updates.mcp.jsonto a singlenitroliteentry.What changed
sdk/ts-mcp→sdk/mcp(unified server)loadGoSdkMethods) readssdk/go/*.goat startup, indexes all*ClientmethodsloadGoTypes) readspkg/core,pkg/app,pkg/rpc,sdk/go/*.gofor structs/enumslanguageparam added tolookup_method,lookup_type,search_api('typescript'default,'go','both')go-transfer-app,go-app-session,go-ai-agentnitrolite://go-api/methods,nitrolite://go-api/types,nitrolite://go-examples/full-transfer-script,nitrolite://go-examples/full-app-session-script,nitrolite://protocol/auth-flowcreate-channel-appandbuild-ai-agent-appprompts with Go sections alongside existing TS content.mcp.json— singlenitroliteentry replacing the previousnitrolite-ts+nitrolite-gopairCLAUDE.md— unifiedsdk/mcp/row replacing two separate MCP server rowsCorrectness (from prior review rounds)
docs/api.yamlv1 — not the legacyclearnode/docs/API.mdCloseAppSessionabsent from both TS and Go (method doesn't exist in v1)lookup_method({ name: 'transfer' })still returns TSDecimalsignature with no Go bleed-throughvalidate_importuses exact match (no substring false positives)Test harness
sdk/mcp-test-results/run.ts— 115 scenarios across TS (A–L) and Go (H, F, G, J, K, L)Surface inventory verified: exactly 30 resources, 8 tools, 3 prompts.
Follow-ups (not in this PR)
sdk/mcp-go/deletion — can be removed after this mergesyellow-docsdocs update — separate PR in that repoTest plan
cd sdk/mcp && npm install && npm start— server starts, logs 30 resources + 8 toolslookup_method({ name: "transfer" })→ returns TSDecimalsignaturelookup_method({ name: "Transfer", language: "go" })→ returns Gofunc (c *Client) Transfer(...)lookup_method({ name: "closeAppSession" })→ "not found"scaffold_project({ template: "go-transfer-app" })→ valid Go code withCheckpoint, no TS sectionsnpm --prefix sdk/mcp-test-results exec -- tsx sdk/mcp-test-results/run.ts→ 115/115 pass🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores