YNU-912: Package SDK MCP for npm publishing#716
Conversation
📝 WalkthroughWalkthroughThe PR establishes a complete publishing pipeline for the Yellow SDK MCP server package. It adds a GitHub Actions workflow that validates package contents, performs smoke tests, publishes to npm, and registers with the MCP registry. Supporting changes include package metadata restructuring, content preparation scripts, runtime logic for supporting both monorepo and packaged content modes, and expanded documentation covering multiple client integrations. ChangesPackage Publishing & Content Management
Sequence DiagramsequenceDiagram
participant Trigger as Git Tag<br/>mcp-v*
participant GHA as GitHub Actions
participant Pack as npm pack
participant Test as Smoke Test<br/>(stdio)
participant NPM as npm Registry
participant Verify as verify-release<br/>-preflight
participant Registry as MCP Registry
Trigger->>GHA: Trigger on mcp-v* tag
GHA->>GHA: Install deps, build
GHA->>Pack: Run npm pack
Pack-->>GHA: tarball created
GHA->>Test: Install tarball, connect to server
Test->>Test: Assert tools, versions, content mode
Test-->>GHA: Smoke test passed
GHA->>Verify: Validate versions across<br/>package.json, server.json,<br/>release.json
Verify-->>GHA: Preflight passed
GHA->>NPM: Publish with NPM_TOKEN<br/>+ provenance
NPM-->>GHA: Published
GHA->>Registry: Download mcp-publisher,<br/>login with github-oidc
GHA->>Registry: Publish MCP registry<br/>metadata
Registry-->>GHA: Metadata published
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
59ef506 to
daebf7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
sdk/mcp/src/index.ts (1)
28-29: ⚡ Quick winAvoid hardcoded release versions in runtime metadata and scaffolds.
'1.2.1'/'v1.2.1'literals can drift on the next release and makeserver_infoplus generatedgo.modinaccurate. Derive these from runtime package versions instead.Suggested fix
-const GO_MODULE_VERSION = 'v1.2.1'; +let GO_MODULE_VERSION = 'unknown'; ... -const serverVersion = readPackageVersion(resolve(PACKAGE_ROOT, 'package.json')) ?? '1.2.1'; +const serverVersion = readPackageVersion(resolve(PACKAGE_ROOT, 'package.json')) ?? 'unknown'; const sdkVersion = readPackageVersion(resolve(SDK_ROOT, 'package.json')) ?? 'unknown'; const compatVersion = readPackageVersion(resolve(COMPAT_ROOT, 'package.json')) ?? 'unknown'; +if (sdkVersion !== 'unknown') GO_MODULE_VERSION = `v${sdkVersion}`;Also applies to: 974-977, 1761-1763, 2056-2056
🤖 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 28 - 29, Replace hardcoded version and name literals by deriving them from package/module metadata at runtime: update GO_MODULE_VERSION and SERVER_NAME in sdk/mcp/src/index.ts (and other occurrences around the noted locations) to read the package version and name (e.g., from package.json or process env populated from build) and use those values when composing server_info and generating go.mod; ensure any code that writes go.mod or server_info uses this derived value so releases don’t drift from actual package version.
🤖 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/package.json`:
- Line 21: The "build" npm script uses a platform-specific chmod call which
breaks on Windows; update the package.json "build" script (and ensure
"prepublishOnly" still chains correctly) to avoid direct use of chmod by either
1) removing the chmod step and marking the executable via package.json "bin"
metadata, or 2) replacing the chmod call with a cross-platform approach such as
adding a devDependency like "chmod-cli" and invoking it in the "build" script,
or invoking a small Node script that calls fs.chmodSync("dist/index.js", 0o755)
(create a scripts/set-executable.js and call it from "build"); pick one approach
and update package.json scripts and devDependencies accordingly so builds
succeed on Windows and Unix.
In `@sdk/mcp/scripts/prepare-package-content.mjs`:
- Around line 54-56: The code currently logs and continues when a required
sourcePath is missing (the if block using existsSync(sourcePath)), which allows
creating an incomplete snapshot; change this to fail fast by throwing an Error
or calling process.exit(1) with a descriptive message that includes the
relative(repoRoot, sourcePath) so the build aborts immediately; update the block
around existsSync(sourcePath) in prepare-package-content.mjs to stop execution
rather than continue.
---
Nitpick comments:
In `@sdk/mcp/src/index.ts`:
- Around line 28-29: Replace hardcoded version and name literals by deriving
them from package/module metadata at runtime: update GO_MODULE_VERSION and
SERVER_NAME in sdk/mcp/src/index.ts (and other occurrences around the noted
locations) to read the package version and name (e.g., from package.json or
process env populated from build) and use those values when composing
server_info and generating go.mod; ensure any code that writes go.mod or
server_info uses this derived value so releases don’t drift from actual package
version.
🪄 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: b7bf73bc-a523-4a02-a343-12750994641e
⛔ Files ignored due to path filters (1)
sdk/mcp/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.github/workflows/publish-sdk-mcp.ymlsdk/mcp/.gitignoresdk/mcp/README.mdsdk/mcp/package.jsonsdk/mcp/scripts/prepare-package-content.mjssdk/mcp/server.jsonsdk/mcp/src/index.ts
daebf7f to
d565102
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
sdk/mcp/scripts/verify-release-preflight.mjs (1)
52-53: 💤 Low valueOptional: Mirror
verify-package-content.mjs's explicit npm package presence assertion for clearer errors.If
serverJson.packageshappens to omit an npm entry,npmPackage?.version === versionfails with the generic "version must match" message even though the entry itself is missing. The companionverify-package-content.mjsasserts presence explicitly (line 68–69) — aligning the two keeps error messages on missing/unmatched entries unambiguous.♻️ Proposed alignment
const npmPackage = serverJson.packages.find((entry) => entry.registryType === 'npm'); -assert(npmPackage?.version === version, 'server.json npm package version must match package.json version'); +assert(npmPackage, 'server.json is missing an npm package entry'); +assert(npmPackage.version === version, 'server.json npm package version must match package.json version');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/mcp/scripts/verify-release-preflight.mjs` around lines 52 - 53, The current assertion uses npmPackage?.version which yields a vague failure if the npm entry is missing; update the check in verify-release-preflight.mjs to first assert that an npm package entry exists (e.g., assert(npmPackage, 'server.json must contain an npm package entry')) and only then assert npmPackage.version === version so the error messages distinguish "missing npm package" from "version mismatch"; reference the npmPackage variable and serverJson.packages find logic when making the change..github/workflows/publish-sdk-mcp.yml (1)
126-137: ⚡ Quick winBrittle exact-count assertions on tools/resources will block unrelated releases.
tools.tools.length !== 9andresources.resources.length !== 31will fail the entire publish pipeline the moment anyone adds (or temporarily removes) a tool or resource — even though that change is exactly the kind of thing that should ship in a release. This is inconsistent with the much more forgiving>= 70floor used at line 162 forcontentManifestFiles.Prefer asserting presence of the tools/resources you actually depend on by name (you already do this for
server_info), plus a sane lower-bound on count. That preserves the regression signal (catastrophically empty registries) without coupling CI to incidental inventory.♻️ Suggested change
const tools = await client.listTools(); - if (tools.tools.length !== 9) { - throw new Error(`expected 9 tools, got ${tools.tools.length}`); - } - if (!tools.tools.some((tool) => tool.name === 'server_info')) { - throw new Error('server_info tool missing'); - } + const requiredTools = [ + 'server_info', + 'lookup_method', + 'lookup_type', + 'lookup_rpc_method', + 'scaffold_project', + ]; + for (const name of requiredTools) { + if (!tools.tools.some((tool) => tool.name === name)) { + throw new Error(`required tool missing: ${name}`); + } + } + if (tools.tools.length < requiredTools.length) { + throw new Error(`unexpected tool count ${tools.tools.length}`); + } const resources = await client.listResources(); - if (resources.resources.length !== 31) { - throw new Error(`expected 31 resources, got ${resources.resources.length}`); + if (resources.resources.length < 20) { + throw new Error(`unexpected resource count ${resources.resources.length}`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-sdk-mcp.yml around lines 126 - 137, Replace the brittle exact-count checks on tools and resources with presence checks for the specific dependencies and a reasonable lower-bound count: when calling client.listTools() keep the existing presence assertion for 'server_info' (tools.tools.some(...)) and change the equality check on tools.tools.length !== 9 to a floor check (e.g., tools.tools.length >= <sane_min_tools>) so the pipeline only fails on catastrophically low inventories; do the same for client.listResources() (keep any name-based assertions you depend on and change resources.resources.length !== 31 to resources.resources.length >= <sane_min_resources>); this mirrors the tolerant approach used for contentManifestFiles and avoids blocking unrelated releases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/publish-sdk-mcp.yml:
- Around line 126-137: Replace the brittle exact-count checks on tools and
resources with presence checks for the specific dependencies and a reasonable
lower-bound count: when calling client.listTools() keep the existing presence
assertion for 'server_info' (tools.tools.some(...)) and change the equality
check on tools.tools.length !== 9 to a floor check (e.g., tools.tools.length >=
<sane_min_tools>) so the pipeline only fails on catastrophically low
inventories; do the same for client.listResources() (keep any name-based
assertions you depend on and change resources.resources.length !== 31 to
resources.resources.length >= <sane_min_resources>); this mirrors the tolerant
approach used for contentManifestFiles and avoids blocking unrelated releases.
In `@sdk/mcp/scripts/verify-release-preflight.mjs`:
- Around line 52-53: The current assertion uses npmPackage?.version which yields
a vague failure if the npm entry is missing; update the check in
verify-release-preflight.mjs to first assert that an npm package entry exists
(e.g., assert(npmPackage, 'server.json must contain an npm package entry')) and
only then assert npmPackage.version === version so the error messages
distinguish "missing npm package" from "version mismatch"; reference the
npmPackage variable and serverJson.packages find logic when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b893ca4-ec22-4eab-8b44-006f75760f2c
⛔ Files ignored due to path filters (1)
sdk/mcp/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.github/workflows/publish-sdk-mcp.ymlsdk/mcp/.gitignoresdk/mcp/README.mdsdk/mcp/package.jsonsdk/mcp/scripts/prepare-package-content.mjssdk/mcp/scripts/set-executable.mjssdk/mcp/scripts/verify-package-content.mjssdk/mcp/scripts/verify-release-preflight.mjssdk/mcp/server.jsonsdk/mcp/src/index.ts
Summary
Packages
sdk/mcpfor public npm distribution as@yellow-org/sdk-mcp, so Claude, Codex, Cursor, VS Code, and other stdio MCP clients can run the Yellow SDK / Nitrolite protocol context server without cloning the Nitrolite monorepo.What changed
sdk/mcp: compileddistentrypoint, executableyellow-sdk-mcpbin, public publish config, minimal packagefiles, andmcpName.content/snapshot builder so published installs can serve SDK/protocol docs from packaged files while local monorepo development still reads live source first.server_infofor package, SDK, compat, Go module, protocol, transport, and content-mode debugging.server.jsonand a tag-triggeredmcp-v*workflow that builds, packs, validates, smoke-tests, publishes to npm, then publishes registry metadata.Release safety
The workflow now publishes the exact tarball it verifies. It asserts critical package paths, blocks source/test file leakage, installs the packed tarball into a temp app, and exercises packaged-snapshot mode through
server_info,lookup_method, andlookup_rpc_methodbefore publish.mcp-publisheris pinned tov1.7.6with SHA256 verification instead of using a floatinglatestdownload.Validation run locally
npm run typecheckfromsdk/mcpnpm run buildfromsdk/mcpnpm pack --jsonserver_info,lookup_method, andlookup_rpc_methodgit diff --cached --checkNotes for reviewers
@yellow-org/sdk(1.2.1) so@yellow-org/sdk-mcp@^1tracks v1 SDK docs.--no-gpg-signbecause the local 1Password SSH signing agent failed during commit creation.Summary by CodeRabbit
Release Notes
New Features
server_infotool providing server metadata and version information.Documentation
Chores