Fix v2 single-package publish bundle + add pack:verify

[cliffhall]

Refs #1636 (packaging half). Does not close it: this PR delivers the single-package packaging correctness + pack:verify; the main.yml release-job wiring is tracked separately in #1640 (lands at the v2/main → main cutover), and the standalone core package remains deferred on #1636. The web version display is #1639.

⚠️ This PR targets v2/main (a non-default branch), so closing keywords would not auto-close anyway — the linked issues are closed manually on merge per AGENTS.md.

Prepares the single-package v2 publish bundle and adds a script that verifies the real published tarball end to end — catching "works in --dev, breaks under npx @modelcontextprotocol/inspector" failures before they ship.

Per the decisions on #1636: one @modelcontextprotocol/inspector package with everything, a single version number, no standalone core package for now, source maps dropped, and clients/web/build fixed. Actual main.yml release-job wiring still lands at the v2/main → main cutover; this lands the packaging correctness + local verification now.

What changed

• scripts/pack-and-verify.mjs (npm run pack:verify) — builds, npm packs the publishable tarball, installs it into a clean throwaway consumer (real npm install <tgz>, runs postinstall), and drives the installed mcp-inspector bin: --help dispatch, a real --cli tools/list over stdio, and a prod --web boot that must serve / (HTTP 200, injected token) from the shipped dist. Also asserts no source maps ship and that the runtime-required files are present. Local/release tool (the install needs network), so it is intentionally not part of the fast validate/ci loop.
• Source maps off in the three client bundlers (clients/{cli,tui,web} tsup configs). Vite and the launcher tsc already emit none. This ~halves the unpacked payload.
• clients/web/.npmignore so clients/web/build (the prod web-server runner) actually ships. npm's packlist was honoring clients/web/.gitignore's build/ entry over the root "files" allowlist, silently dropping the runner while dist/ slipped through.
• Ship clients/cli/package.json and clients/tui/package.json (added to root "files") — both bundles read their own package.json at runtime (CLI client identity, TUI header). Without them the installed CLI crashed on connect and the installed TUI crashed on launch, though both worked in-repo.

Two real publish-only bugs this caught

pack:verify surfaced both while being written — each worked in every existing smoke (which run against the in-repo tree) but crashed from an installed package:

1. TUI App.tsx read ../../package.json (→ clients/tui/package.json) → ENOENT on launch.
2. CLI cli.ts imported ../package.json (→ clients/cli/package.json) → ERR_MODULE_NOT_FOUND on connect (--help worked; tools/list didn't).

Result

Publishable tarball: 26 files, ~1.5 MB compressed / 4.44 MB unpacked (was 6.6 MB), no source maps, clients/web/{build,dist} + both client package.jsons present. npm run pack:verify passes: install-clean + web/cli/tui driven end to end from the installed bin.

Test plan

• npm run pack:verify — green end to end.
• npm run ci — validate + coverage gate + smokes + Storybook.

🤖 Generated with Claude Code

[cliffhall]

Added a root README.md and moved the publishing/packaging documentation out of the launcher README into it (whole-repo packaging belongs at the root, not in the launcher client). Also fixes a dangling include — CLAUDE.md referenced @./README.md, which didn't exist.

The root README covers project layout, setup, the @inspector/core shared package + architecture, the web "dumb components" + Storybook approach, test-servers configuration, the build process, the testing/quality gate, publishing (relocated here), and how AGENTS.md/CLAUDE.md govern contributions for humans and agents alike. The launcher README's Publishing section is now a one-line pointer to the root.

[cliffhall]

Single-sourced the version from the root package.json. Following up on the packaging fixes: rather than shipping each client's package.json so its bundle could read its own version, all clients now read the version from the root package.json — the single source of truth for the one-package/one-version release.

What changed:

• New shared reader core/node/version.ts — a Node-only readInspectorVersion() that walks up to the root manifest (always in the tarball). 100% covered by a new integration test.
• CLI & TUI read the version via the shared reader; TUI now passes a real clientIdentity instead of falling through to the core default.
• Found and fixed a live bug: core/mcp/inspectorClient.ts hardcoded a stale 0.20.0, and because the TUI and web don't pass a clientIdentity, that's the version they were actually reporting to servers. Replaced with a neutral 0.0.0 placeholder; real clients pass their own identity (web sources it from GET /api/config, see Web: show clients/web version in lower-right corner (grey) #1639).
• Dropped version from all four client package.jsons (root keeps the only one) and stopped shipping clients/{cli,tui}/package.json — this supersedes the earlier "ship them so they can be read" fix, since nothing reads a client manifest at runtime anymore. pack:verify and the README packaging invariants updated to match.

Verified with npm run pack:verify (installed CLI/TUI resolve the version from the root manifest with no client package.json present) and a green npm run ci (core/node/version.ts at 100% coverage).

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 17s —— View job

---

Review: PR #1637 — v2 single-package publish bundle + pack:verify

• Read the diff against origin/v2/main
• Review scripts/pack-and-verify.mjs
• Review version single-sourcing (core/node/version.ts, CLI/TUI wiring)
• Review packaging changes (.npmignore, files, sourcemaps)
• Review docs (root README)
• Post review feedback

---

Overall this is a strong, well-tested PR. The packaging fixes are correct and — crucially — the pack:verify harness is the right tool to keep them from silently regressing, since it exercises the actual published tarball rather than the in-repo tree. Version single-sourcing is clean and the walk-up logic is sound. No blocking issues found; a few small observations below.

✅ What's solid

• core/node/version.ts — the walk-up is correct for both trees: dev (clients/cli/build → root) and installed (node_modules/@modelcontextprotocol/inspector/clients/cli/build → package root). The name-match guard (manifest?.name === ROOT_PACKAGE_NAME && manifest.version) correctly skips stray client manifests, and the loop checks the current dir before the dir === root break, so a root-level manifest is never missed. Branch coverage looks complete (name-mismatch skip, name-match-no-version skip, catch for dirless levels, and the terminal throw are all hit by the two tests). Good call using a neutral 0.0.0 placeholder over a number that would drift.
• .npmignore fix is precisely targeted — I confirmed only clients/web/.gitignore lists build; cli/tui/launcher have no nested .gitignore, so the root files allowlist ships their build/ unimpeded. The verbose comment explaining why the file exists is exactly the kind of thing that prevents a future "why is this here?" deletion.
• pack:verify genuinely closes the in-repo-vs-published gap and caught the two real path-resolution bugs described. Driving --cli tools/list over stdio and a prod --web boot from the installed location is the correct depth of check.

🔍 Minor observations (non-blocking)

1. Web now reports mcp-inspector 0.0.0 as its client identity to servers until Web: show clients/web version in lower-right corner (grey) #1639 wires the real version through GET /api/config. This is the fallback in core/mcp/inspectorClient.ts:38 and only web hits it (CLI/TUI now pass their own identity). It's a documented, intentional placeholder — just flagging it explicitly for reviewers, since a server-side connection log will show 0.0.0 for the web client in the interim. Previously it (mis)reported 0.20.0, so this is arguably more honest, not a regression in substance.

2. scripts/pack-and-verify.mjs:154 — cleanupTarball is dead logic. It's initialized true and the catch sets it true again, so the finally's if (cleanupTarball) rmSync(tarball) always runs. If the intent was to retain the tarball on failure for debugging, the catch defeats that; if not, the variable can go and the finally can unconditionally remove the tarball. Purely cosmetic. Fix this →

3. TUI evaluates readInspectorVersion(import.meta.url) at module top-level (App.tsx:APP_VERSION), whereas the CLI calls it lazily inside callMethod. In the installed/dev tree the root is always reachable so this is fine, but the eager version means any future scenario where the root manifest isn't resolvable would surface as an import-time crash rather than a handled error. Not worth changing now — just noting the asymmetry.

4. verifyWeb hardcodes port 6399 (env-overridable via PACK_VERIFY_WEB_PORT). Since pack:verify is a local/release tool, a rare port clash is acceptable, but worth keeping the override documented if this ever runs in a shared CI runner.

None of these need to hold up the merge. Nice work — the pack:verify investment pays for itself the first time it catches a packlist regression.

[cliffhall]

Thanks for the review! Responses to the four observations:

1. Web reports 0.0.0 until Web: show clients/web version in lower-right corner (grey) #1639 — correct and intentional. As you noted, it's more honest than the previous 0.20.0, and Web: show clients/web version in lower-right corner (grey) #1639 will wire the real version through GET /api/config and pass it as the web clientIdentity. Leaving as-is; flagged in the code comment and on Web: show clients/web version in lower-right corner (grey) #1639. No change.

2. Dead cleanupTarball logic — fixed in c1b168e. Rather than delete the flag, I made it meaningful: the catch now sets it false so the packed tarball is retained on failure for post-mortem inspection (unpack it / diff the file list), and logs the path; the happy path still removes it. That's the more useful of the two options you offered for a verify tool.

3. TUI eager top-level readInspectorVersion vs CLI lazy — real asymmetry, leaving it. The TUI genuinely needs the version at module scope (the header constant APP_VERSION and the clientIdentity it passes), and the root manifest is always resolvable in both the dev tree and the installed package, so an import-time throw can't actually occur in practice. Not worth restructuring the module for a can't-happen path. No change.

4. verifyWeb hardcodes 6399 — acceptable for a local/release tool, and it's already overridable via PACK_VERIFY_WEB_PORT (documented in the function). No change.

Only #2 was an actual code fix; the rest are intentional/documented. Re-requesting a review to confirm nothing else.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 31s —— View job

---

Re-review: cleanupTarball fix (c1b168e)

• Verify the cleanupTarball fix
• Re-scan the rest of the diff
• Post findings

---

Thanks for the follow-ups. #1, #3, #4 are all fine as answered. On #2, though — the fix doesn't do what the commit says, because it collides with fail() calling process.exit(1).

🔴 The retain-on-failure fix is inert; the temp work dir now leaks on failure

The failure path is:

} catch (err) {
  cleanupTarball = false;                       // line 259
  console.error(`… tarball retained … ${tarball}`);
  fail(err …);                                  // line 261 → process.exit(1)
} finally {
  rmSync(work, { recursive: true, force: true });   // line 263
  if (cleanupTarball) rmSync(tarball, …);           // line 264
}

fail() (line 54) calls process.exit(1), and process.exit() does not run finally blocks — I confirmed it in this environment:

$ node -e 'try{throw 0}catch(e){process.exit(3)}finally{console.log("FINALLY")}'
# prints nothing from finally; exits 3

So on the failure path the whole finally is skipped. Two consequences:

1. cleanupTarball = false is still dead logic. The finally only ever runs on the success path, where cleanupTarball is always true. The branch if (cleanupTarball) is never evaluated with false. The tarball is retained on failure — but only as an accidental side effect of process.exit() skipping the finally, not because of the flag. This is the same dead-variable the commit set out to fix, just relocated.
2. rmSync(work, …) is also skipped on failure, so the temp work dir (the clean consumer with its full node_modules from the real npm install <tgz>) now leaks on every failed run. That's the more material regression — previously it leaked too (the old catch also fail()ed), so this isn't newly broken, but the commit's framing ("happy path still removes it") implies the finally runs on failure, and it doesn't.

To actually get the intended behavior (retain tarball, still clean the work dir, on failure), do the cleanup explicitly in the catch rather than relying on a finally that process.exit() bypasses — e.g.:

} catch (err) {
  rmSync(work, { recursive: true, force: true });   // still clean the heavy work dir
  console.error(`pack:verify — tarball retained for inspection at ${tarball}`);
  fail(err instanceof Error ? err.message : String(err));  // exits 1, tarball left in place
} finally {
  rmSync(work, { recursive: true, force: true });   // success path
  rmSync(tarball, { force: true });                 // success: always remove
}

(or make fail() throw and do a single top-level handler, but the above is the smaller change). Either way the cleanupTarball variable can go. Fix this →

Everything else

The rest of the PR is unchanged from my last pass and still looks good — version single-sourcing (core/node/version.ts), the .npmignore fix, and the pack:verify harness itself. This one item is the only thing outstanding, and it's minor (a temp-dir leak in a local-only tool on the failure path). Not a merge blocker, but the commit doesn't accomplish its stated goal, so worth a small follow-up.

[cliffhall]

Sharp catch — you're right, my previous fix was inert. fail() calls process.exit(1), which skips finally, so cleanupTarball = false never took effect and the temp work dir leaked on every failed run (and since fail() is called from all over, most failures never reached the finally at all).

Fixed properly in 3dead1f: moved cleanup into fail() itself — the single failure-exit point. On failure it removes the heavy work dir (the full node_modules from the real install) and leaves the packed tarball in place for inspection; the success path removes both inline. Dropped the cleanupTarball flag entirely.

Verified both paths this time rather than reasoning about them:

• Success: work dir + tarball removed, exit 0.
• Forced failure (temporary injected fail() after the work dir is created): work dirs count stayed flat (this run's dir cleaned), tarball retained, exit 1, and it logged tarball retained for inspection at ….

Re-requesting review.

@claude review

[claude]

Claude finished @cliffhall's task in 1m 18s —— View job

---

Re-review: fail()-based cleanup fix (3dead1f)

• Verify the cleanup-in-fail() fix
• Re-scan the rest of the diff
• Post findings

---

✅ The fix is correct this time — verified by tracing both paths. Moving cleanup into fail() is the right call, since fail() is the single exit point and process.exit() skips finally. The dead finally and the cleanupTarball flag are both gone.

• Failure path (fail(), pack-and-verify.mjs:62): removes workDir (the heavy node_modules consumer), retains the tarball, logs the retention path, exits 1. Matches the stated goal.
• Success path (:267-268): removes both work and tarball inline, natural exit 0. No finally to be skipped.
• Unexpected throw: the catch at :274 routes through fail(), so an out-of-band error gets the same consistent cleanup + exit. Good.
• The TDZ concern is handled and documented: workDir is only assigned (:167) after const tarball (:124), so the if (workDir) guard guarantees tarball is initialized before it's referenced in fail(). The comment at :66 calls this out explicitly. Confirmed no fail() call before :124 ever enters the workDir block (they all run with workDir === null).

🔍 One residual instance of the same pattern (minor, non-blocking)

verifyWeb still relies on a finally { stop() } (:338-340) to kill the spawned --web child — but every failure inside its try goes through fail() → process.exit(1), which skips that finally. So on a --web-phase failure (:317, :326, :329, :333, :336), child.kill() never runs and the web server child is orphaned (reparented to init on POSIX), left listening on the port.

Two consequences, both minor for a local/release tool:

1. A leaked server process after a failed run.
2. More subtly, a subsequent pack:verify run's verifyWeb could fetch the stale orphaned server on the same port and get a false 200 — masking a real regression until the port is freed.

Since this is exactly the process.exit()-skips-finally class you just eliminated in the main flow, the tidy fix is to have fail() also stop any live child before exiting — e.g. register the child so fail() can child.kill("SIGTERM") it alongside the workDir cleanup. Fix this →

Everything else in the PR is unchanged and still looks good (version single-sourcing, .npmignore, the packaging assertions). The core cleanup fix accomplishes its stated goal now — this last item is optional polish on the same failure-path theme.
· 1636-v2-publish-bundle

[cliffhall]

Good catch — same process.exit()-skips-finally pattern, and the stale-server-serves-false-200 consequence is a real correctness risk for a verify tool, so worth fixing.

Fixed in 9324f9f: track the live --web child in a module webChild, and fail() now SIGTERMs it before exiting (alongside the workDir cleanup); stop() clears it on the success path.

Verified: a forced web-phase failure exits 1, retains the tarball, and leaves no listener on the port (child killed, not orphaned) — before the fix that server would have lingered.

@claude review

[claude]

Claude finished @cliffhall's task in 44s —— View job

---

Re-review: webChild kill-in-fail() fix (9324f9f)

• Verify the web-child cleanup fix
• Re-scan the rest of the diff
• Post findings

---

✅ The fix is correct — the last process.exit()-skips-finally orphan is closed. Traced every path:

• webChild is tracked and killed at the single exit point. verifyWeb assigns webChild = child right after spawn (pack-and-verify.mjs:312), and fail() now SIGTERMs it before exiting (:69-71), guarded by webChild.exitCode === null so it never signals an already-dead child. This is exactly right, since fail() → process.exit() bypasses the finally { stop() } at :350.
• Success path clears the handle. stop() (:320-323) sets webChild = null after killing, so once verifyWeb returns cleanly the module has no dangling reference — a later success-path cleanup (or an unrelated fail()) won't touch a reaped child. Good.
• The child-already-exited case is handled twice over. If --web dies before serving, :328 routes to fail() — and by then child.exitCode is non-null, so the new :69 guard correctly skips the kill. No double-signal, no crash.
• Ordering/TDZ still sound. fail()'s webChild block runs before the workDir/tarball block, and webChild is null until verifyWeb runs (well after both tarball and workDir are initialized), so no temporal-dead-zone hazard is introduced.

The three failure-path concerns from the prior passes are now all resolved: the tarball is genuinely retained on failure, the heavy work dir is cleaned, and the --web child is no longer orphaned (so no stale-server false-200 can mask a regression in a later run).

Minor note (non-blocking, no change needed)

fail() sends SIGTERM and immediately process.exit(1) without awaiting the child's teardown, so there's a sub-second window where the port may not yet be free. In practice the signal is delivered before the parent dies (vs. the old orphan that lingered indefinitely), and the next run's verifyWeb polls / for up to 60s, so this is not worth adding an async wait for in a local/release tool. Flagging only for completeness.

Nothing outstanding. This closes the failure-path cleanup theme cleanly across all three follow-ups (fail()-based tarball retention → work-dir cleanup → web-child kill). LGTM. 👍
· 1636-v2-publish-bundle