Skip to content

Refactor project after addition of CLI#316

Merged
cliffhall merged 8 commits into
modelcontextprotocol:mainfrom
cliffhall:fix-cli-build
Apr 16, 2025
Merged

Refactor project after addition of CLI#316
cliffhall merged 8 commits into
modelcontextprotocol:mainfrom
cliffhall:fix-cli-build

Conversation

@cliffhall
Copy link
Copy Markdown
Member

@cliffhall cliffhall commented Apr 15, 2025 •
edited
Loading

Description

  • Remove bin folder, leaving cli, server, and client
  • This fixes New CLI feature needs organizational work #315
  • In .gitignore,
    • add .idea
    • remove bin/build
  • Remove bin and bin/cli.js
  • Remove bin/scripts/copy-cli.js
  • Refactor/move bin/scripts to cli/scripts
  • Refactor/move bin/src/index.ts to cli/src/cli.ts
  • Refactor/renamed client/bin/cli.js to client/bin/client.js
  • In .github/workflows/main.yml,
    • add run of cli tests
  • In cli/pacakge.json
    • change main and bin/mcp-inspector-cli properties to build/cli.js
  • In client/package.json,
    • change bin/mcp-inspector-client properties to build/start.js
  • In pacakge.json
    • change bin/mcp-inspector property to ./cli/build/cli.js
    • removed bin and cli/bin from files list
    • removed @modelcontextprotocol/inspector-bin dependency
    • rearranged and corrected scripts

Motivation and Context

See #315 for issue details

How Has This Been Tested?

Build, test, test-cli, start, manually test UI, and npm publish --dry-run to check package files.

npm publish --dry-run

npm notice
npm notice 📦  @modelcontextprotocol/inspector@0.9.0
npm notice Tarball Contents
npm notice 1.1kB LICENSE
npm notice 8.6kB README.md
npm notice 6.7kB cli/build/cli.js
npm notice 922B cli/build/client/connection.js
npm notice 199B cli/build/client/index.js
npm notice 664B cli/build/client/prompts.js
npm notice 961B cli/build/client/resources.js
npm notice 2.0kB cli/build/client/tools.js
npm notice 11B cli/build/client/types.js
npm notice 405B cli/build/error-handler.js
npm notice 6.4kB cli/build/index.js
npm notice 1.6kB cli/build/transport.js
npm notice 858B client/bin/client.js
npm notice 2.7kB client/bin/start.js
npm notice 1.0MB client/dist/assets/index-C3EbTOeA.js
npm notice 46.8kB client/dist/assets/index-D5WIP3OH.css
npm notice 1.6kB client/dist/assets/OAuthCallback-DKtKHu2h.js
npm notice 459B client/dist/index.html
npm notice 973B client/dist/mcp.svg
npm notice 1.9kB package.json
npm notice 5.8kB server/build/index.js
npm notice 1.1kB server/build/mcpProxy.js
npm notice Tarball Details
npm notice name: @modelcontextprotocol/inspector
npm notice version: 0.9.0
npm notice filename: modelcontextprotocol-inspector-0.9.0.tgz
npm notice package size: 241.0 kB
npm notice unpacked size: 1.1 MB
npm notice shasum: 174229d7dc7751fff07b651091b136a7042e4b3d
npm notice integrity: sha512-OTb94ukh1EsUT[...]ZQ69A+XeNwQ+g==
npm notice total files: 22
npm notice
npm notice Publishing to https://registry.npmjs.org/ with tag latest and default access (dry-run)

Breaking Changes

Nope.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@cliffhall
Refactor project components
6a99fea 
* Remove bin folder, leaving cli, server, and client
* This fixes modelcontextprotocol#315
* In .gitignore,
  - add .idea
  - remove bin/build
* Remove bin and bin/cli.js
* Remove bin/scripts/copy-cli.js
* Refactor/move bin/scripts to cli/scripts
* Refactor/move bin/src/index.ts to cli/src/cli.ts
* Refactor/renamed client/bin/cli.js to client/bin/client.js
* In .github/workflows/main.yml,
  - add run of cli tests
* In cli/pacakge.json
  - change main and bin/mcp-inspector-cli properties to build/cli.js
* In client/package.json,
  - change bin/mcp-inspector-client properties to build/start.js
* In pacakge.json
  - change bin/mcp-inspector property to ./cli/build/cli.js
  - removed bin and cli/bin from files list
  - removed @modelcontextprotocol/inspector-bin dependency
  - rearranged and corrected scripts
@cliffhall
Remove cli tests from build (build has to happen first for those tests)
b19c78b
@cliffhall cliffhall requested a review from olaservo April 15, 2025 22:09
olaservo
olaservo reviewed Apr 16, 2025
View reviewed changes
Comment thread package.json
@olaservo
Copy link
Copy Markdown
Member

olaservo commented Apr 16, 2025

When I run this on my local machine and do npm install I end up with dependency conflicts between the Express version required by the typescript MCP SDK (which shows as ^5.0.1 in the updated package-lock.json) and the Express version specified in the package.json for the proxy server which is set to ^4.21.0.

So I think that means we need to update the proxy server package.json to use ^5.0.1 which appears to resolve my errors when trying to run npm run dev:windows.

@olaservo
Copy link
Copy Markdown
Member

olaservo commented Apr 16, 2025

The run dev:windows command is working for me now after updating the proxy server Express version to 5x. 🎉

One more thing, I noticed in the description you mentioned

In .github/workflows/main.yml,
add run of cli tests

But I don't see any changes to main.yml in this diff?

@cliffhall
Copy link
Copy Markdown
Member Author

cliffhall commented Apr 16, 2025 •
edited
Loading

The run dev:windows command is working for me now after updating the proxy server Express version to 5x. 🎉

Noice!

One more thing, I noticed in the description you mentioned

In .github/workflows/main.yml,
add run of cli tests

But I don't see any changes to main.yml in this diff?

Yes, I had to pull that out. It wouldn't run in CI. Decided not to grapple with that in this PR.

olaservo
olaservo approved these changes Apr 16, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@olaservo olaservo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, looks good to me!

@cliffhall cliffhall merged commit 49dddfb into modelcontextprotocol:main Apr 16, 2025
2 checks passed
@cliffhall cliffhall deleted the fix-cli-build branch April 16, 2025 17:09
IgnacioC44 referenced this pull request in MCPJam/inspector Jun 21, 2025
@cliffhall
Merge pull request #316 from cliffhall/fix-cli-build
226ad2e 
Refactor project after addition of CLI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@olaservo olaservo olaservo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

New CLI feature needs organizational work

2 participants

@cliffhall @olaservo