fix(release): correct npm package metadata and gate publishing on engine assets - #18
Open
anvanster wants to merge 3 commits into
Open
fix(release): correct npm package metadata and gate publishing on engine assets#18anvanster wants to merge 3 commits into
anvanster wants to merge 3 commits into
Conversation
npm rewrote four `bin` paths and `repository.url` on every publish and reported the bin entries as "invalid and removed". The entries in fact survived - 0.19.0 and 0.19.1 both carry all of theirs on the registry - but the warning was indistinguishable from a real failure and cost a publish that was aborted on the assumption it had broken something. Store the forms npm normalizes to, so a clean publish is silent and any future warning means something. package-npm.sh treated a package.json/server.json version mismatch as a warning, printed into the middle of npm pack output where it scrolls past. The two files are published to two different registries under one version, and republishing that version cannot correct a disagreement between them, so this now aborts instead. The package ships no engine: every install fetches one from the release tagged with the engine's version. Publishing ahead of those assets makes a package that installs cleanly and then has nothing to run, so the assets are probed before packing rather than trusted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017rVbt7rENTwXkdHt3Bpgb5
🔍 CodeGraph PR Review2 files changed (+86/−11, 0 functions) · Risk: 🟢 low Suggested reviewersAndrey Vasilevsky (9 lines), anvanster (2 lines) Suggested commit: |
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
1 participant
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.
Intent
The developer was finishing the 0.20.0 release of CodeGraph across all three distribution channels (GitHub release binaries, the npm package @astudioplus/codegraph-mcp, and the MCP Registry) and asked the agent to double-check scripts/package-npm.sh for correctness before running it manually, since npmjs 2FA meant the developer had to execute the publish themselves. During the actual publish, npm emitted warnings that it had auto-corrected package.json - the four bin entries were declared with a "./bin/..." prefix that npm rejected and normalized, and repository.url was normalized to "git+https://github.com/codegraph-ai/CodeGraph.git" - so the developer aborted the publish and asked for those metadata errors to be fixed in the scripts before retrying. The intent was to make the published npm metadata match exactly what npm expects so publishing runs clean with no warnings, and to harden the packaging script itself: a version mismatch between package.json and server.json should be fatal rather than a printed warning, and the script should verify the GitHub release engine assets exist before publishing, since the package ships no bundled engine and an install would otherwise succeed with nothing to run. The developer explicitly did not want a version bump (0.20.0 had in fact published successfully; only the follow-on MCP Registry step had failed on auth). Finally the developer asked to commit the two changed files - mcp-package/package.json and scripts/package-npm.sh - so the repository would record the metadata that actually produced the live 0.20.0 release, and then to push, which per their setup goes through the no-mistakes gate rather than directly to origin.
What Changed
mcp-package/package.json: the fourbinentries drop their./prefix andrepository.urlbecomesgit+https://github.com/codegraph-ai/CodeGraph.git, so the published metadata matches what npm normalizes to instead of being auto-corrected with warnings at publish time.scripts/package-npm.sh: a version mismatch is now fatal instead of a printed warning, and the check coversserver.json's nestedpackages[].version(the field the MCP Registry resolves the tarball from) in addition to the top-levelversion.scripts/package-npm.sh: before packing, the script probes the GitHub release for the engine version pinned inbin/fetch-engine.jsand fails if any of the five engine assets or their.sha256files are missing, since the package bundles no engine and an install would otherwise succeed with nothing to run. Header comments were updated to describe the new gate. A follow-up note from review: the probes have nocurl --retry, so a transient network failure reports a complete release as missing (fail-closed, re-runnable).Risk Assessment
✅ Low: All three round-1 findings are correctly fixed and the change remains confined to release tooling plus npm metadata that matches npm's own normalization, with the asset list verified to mirror publish-release-assets.sh exactly and the stricter gate confirmed to pass against the live v0.20.0 release.
Testing
Ran the mcp-package test suite (the same suite the packaging script gates on) plus targeted end-to-end verification of the actual user-visible intent:
npm publish --dry-runreproduces the five auto-correction warnings on the base commit's package.json and emits none on the fixed one; the committed bin map and repository.url match exactly what npmjs.com stores for the live 0.20.0 release; and installing the packed tarball creates all four working bin shims. Exercised the packaging script in temp repo copies - the happy path probes the real v0.20.0 GitHub release and finds all five engine assets with their checksums (exit 0), while a server.json top-level mismatch, a packages[].version mismatch, and an unpublished engine version each abort with exit 1, versus the base script which warned and packaged anyway at exit 0. This change is CLI/packaging-only with no rendered UI surface, so the evidence is publish transcripts and install output rather than screenshots. Everything passed; the worktree was left clean with no stray tarballs or binaries.Evidence: npm publish --dry-run BEFORE (base commit metadata) - the aborted-publish warnings
npm warn publish npm auto-corrected some errors in your package.json when publishing. Please run "npm pkg fix" to address these errors. npm warn publish errors corrected: npm warn publish "bin[codegraph-mcp]" script name bin/codegraph-mcp.js was invalid and removed npm warn publish "bin[codegraph-daemon]" script name bin/codegraph-daemon.js was invalid and removed npm warn publish "bin[codegraph-mcp-install-hooks]" script name bin/install-hooks.js was invalid and removed npm warn publish "bin[codegraph-mcp-fetch-engine]" script name bin/fetch-engine-cli.js was invalid and removed npm warn publish "repository.url" was normalized to "git+https://github.com/codegraph-ai/CodeGraph.git"Evidence: npm publish --dry-run AFTER (fixed metadata) - no warnings, clean manifest
npm notice 📦 @astudioplus/codegraph-mcp@0.20.0 npm notice ... (11 files) npm notice name: @astudioplus/codegraph-mcp npm notice version: 0.20.0 npm notice filename: astudioplus-codegraph-mcp-0.20.0.tgz npm notice package size: 22.2 kB (zeronpm warn publishlines; the only error is the expected "You cannot publish over the previously published versions: 0.20.0", since 0.20.0 is already live)Evidence: Committed package.json matches the live 0.20.0 registry metadata
# npm view @astudioplus/codegraph-mcp@0.20.0 bin repository.url bin = { 'codegraph-mcp': 'bin/codegraph-mcp.js', 'codegraph-daemon': 'bin/codegraph-daemon.js', 'codegraph-mcp-install-hooks': 'bin/install-hooks.js', 'codegraph-mcp-fetch-engine': 'bin/fetch-engine-cli.js' } repository.url = 'git+https://github.com/codegraph-ai/CodeGraph.git' # mcp-package/package.json now records the identical values.Evidence: Installed tarball produces working bin shims
Evidence: package-npm.sh happy path - live engine-asset verification against v0.20.0
package.json version: 0.20.0 server.json version: 0.20.0 server.json npm package: 0.20.0 engine version: 0.20.0 (fetched at install time) Checking published engine assets for v0.20.0... ✓ codegraph-server-darwin-arm64 ✓ codegraph-server-darwin-x64 ✓ codegraph-server-linux-x64 ✓ codegraph-server-win32-x64.exe ✓ onnxruntime.dll ✓ every engine asset is published for v0.20.0 ✓ Created: mcp-package/astudioplus-codegraph-mcp-0.20.0.tgz ( 24K)Evidence: Gate exit codes: new script aborts where the base script warned and continued
# target commit (18c36df) happy path -> exit 0 server.json version mismatch -> exit 1 packages[].version mismatch -> exit 1 engine assets not published -> exit 1 # base commit (284c8e5), same server.json version mismatch package.json version: 0.20.0 server.json version: 0.19.9 WARNING: version mismatch between package.json and server.json Packing... ✓ Created: mcp-package/astudioplus-codegraph-mcp-0.20.0.tgz ( 24K) base script exit with version mismatch -> 0 (packaging proceeded)Evidence: Base-script behavior on version mismatch (full transcript)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
scripts/package-npm.sh:66- The newly-fatal version check compares package.json.version only against server.json's top-levelversion, but the MCP Registry resolves the npm tarball frompackages[0].version(mcp-package/server.json:14), which is left unchecked. Failing sequence: a release bumps mcp-package/package.json and server.json's top-levelversionto 0.21.0 but missespackages[0].version(still 0.20.0) - no script syncs these three fields, they are hand-edited. The gate passes, npm publishes 0.21.0, andmcp-publisher publish(line 106) registers a 0.21.0 server entry that points MCP clients at the 0.20.0 npm tarball. Registry validation still succeeds because that older tarball exists and carries the right mcpName, so the mismatch is silent - and per the change's own comment it cannot be corrected by republishing the same version. This is the exact 'npmjs.com and the MCP Registry disagreeing about what this release is' failure the fatal check was added to prevent. Fix: extend the comparison torequire(server.json).packages[0].version(and error on it the same way) before packing.scripts/package-npm.sh:76- The new pre-publish engine-asset gate probes a single asset, codegraph-server-linux-x64.sha256, while fetch-engine.js:120-133 resolves four distinct assets (codegraph-server-darwin-arm64, codegraph-server-darwin-x64, codegraph-server-linux-x64, codegraph-server-win32-x64.exe) plus the onnxruntime.dll sidecar. A partially-populated release is reachable: scripts/publish-release-assets.sh:219 uploads every staged file in onegh release uploadinvocation, and underset -euo pipefaila mid-upload failure (network drop, rate limit) aborts the script with some assets already attached. If linux-x64 landed and darwin/win32 did not, re-running package-npm.sh --publish passes this probe and publishes the npm package; macOS and Windows users then 404 in bin/postinstall.js and get 'a package that installs cleanly and then has nothing to run' - the precise outcome this gate was introduced to prevent. Fix: loop the probe over the same asset list publish-release-assets.sh:43-46 defines, failing on the first missing one.scripts/package-npm.sh:71- The new comment states 'every install fetches one from the release tagged with this version', which reads as the package version being published. mcp-package/bin/fetch-engine.js:64-77 documents the opposite as deliberate: ENGINE_VERSION is intentionally decoupled from the client version so a client-only patch release does not request a tag that was never published. The code below is correct (it reads ENGINE_VERSION, not PKG_VERSION); only the comment is misleading, and it could prompt a future maintainer to 'fix' the check to use PKG_VERSION and break client-only patch releases. Reword to name the pinned engine version explicitly.🔧 Fix: harden npm packaging version and engine-asset gates
1 info still open:
scripts/package-npm.sh:124- The asset gate now issues 10 sequential unauthenticated curl requests (5 assets x binary + .sha256) with no--retry, so a single transient failure - DNS blip, CDN 5xx, dropped connection - marks the asset missing and aborts withERROR: the release v${ENGINE_VERSION} is missing engine assets, which asserts something untrue and points the operator atpublish-release-assets.sh --publishfor a release that is in fact complete. Behaviour is fail-closed and the operator can simply re-run, so impact is limited to a misleading message; the flake surface is however 10x what the single-probe version had. Adding--retry 3 --retry-connrefused(and optionally--max-time) to both probes would keep the gate strict while making a reported miss actually mean a miss. Verified read-only against the live v0.20.0 release that all 10 URLs currently return 206, so the stricter gate does not block the pending republish and the-r 0-0range request is honoured through GitHub's asset-CDN redirect.✅ **Test** - passed
✅ No issues found.
cd mcp-package && npm test(fetch-engine + wrapper-args suites, 0 failures)npm publish --dry-runin mcp-package with the fixed package.json - zero warn linesnpm publish --dry-runon a temp copy withgit show 284c8e5:mcp-package/package.json- reproduces the 5 auto-correction warningsnpm pkg fixdiff on the base package.json - shows npm rewriting the 4./bin/...entries and repository.urlnpm packthe fixed package, thennpm install --ignore-scripts <tarball>in a scratch project and inspectnode_modules/.bin/shim targetsnpm view @astudioplus/codegraph-mcp@0.20.0 bin repository.urlcompared against the committed mcp-package/package.json./scripts/package-npm.shhappy path in a temp repo copy - live engine-asset probe against the v0.20.0 release, exit 0./scripts/package-npm.shwith server.json top-level version set to 0.19.9 - exit 1./scripts/package-npm.shwith server.json packages[0].version set to 0.19.9 - exit 1./scripts/package-npm.shwith fetch-engine.js ENGINE_VERSION set to 99.99.99 - all 5 assets reported missing, exit 1base-commitscripts/package-npm.shwith the same version mismatch - WARNING printed, packaging proceeded, exit 0CHANGELOG.md:7- Out-of-scope follow-up: CHANGELOG.md's newest entry is 0.17.0 while the shipped version is 0.20.0 (mcp-package/package.json, server.json, and bin/fetch-engine.js ENGINE_VERSION all read 0.20.0). This gap predates this change and this change is release-tooling only with no user-facing behavior, so no entry is owed here. Flagging it because a release-metadata commit is where the drift becomes visible; backfilling 0.18-0.20 deserves its own change.scripts/package-npm.sh:95- shellcheck/shfmt are not installed and the repo configures no shell linter, so static analysis of the ~80 added lines in scripts/package-npm.sh was limited tobash -nsyntax checking plus manual review. Manual review found no quoting, word-splitting, or set -euo pipefail interaction problems in the added code. If shell static analysis matters for release scripts, adding shellcheck to the toolchain would be a reasonable follow-up.✅ **Push** - passed
✅ No issues found.