-
-
Notifications
You must be signed in to change notification settings - Fork 27
feat: Add separate MCP server package #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The failure is a Mocha timeout. Seems intermittent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new @eslint/mcp package that implements an MCP server for ESLint along with its CLI and accompanying tests and configuration updates.
- Port the existing mcp-server.js into the new package with a corresponding CLI executable.
- Add tests for both the MCP server and CLI, as well as update documentation, workflows, and issue templates to support the new package.
Reviewed Changes
Copilot reviewed 15 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/mcp/tests/mcp-server.test.js | Added tests for MCP server tools functionality |
packages/mcp/tests/mcp-cli.js | Added CLI tests to verify startup behavior |
packages/mcp/tests/fixtures/* | Added fixture files for testing linting outcomes |
packages/mcp/src/mcp-server.js | Implemented the MCP server using ESLint integration |
packages/mcp/src/mcp-cli.js | Provided a CLI to initialize and run the MCP server |
packages/mcp/README.md | Updated documentation to include usage for the new package |
Other configuration files (.github/workflows, etc.) | Updated CI/workflows and issue templates to include the MCP package |
Files not reviewed (4)
- .release-please-manifest.json: Language not supported
- packages/mcp/package.json: Language not supported
- packages/mcp/tsconfig.json: Language not supported
- release-please-config.json: Language not supported
Co-authored-by: Milos Djermanovic <[email protected]>
Co-authored-by: Milos Djermanovic <[email protected]>
Co-authored-by: Milos Djermanovic <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! Leaving open for @fasttime to verify.
// Important: Cursor throws an error when `describe()` is used in the schema. | ||
const filePathsSchema = { | ||
filePaths: z.array(z.string().min(1)).nonempty(), | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Important: Cursor throws an error when `describe()` is used in the schema. | |
const filePathsSchema = { | |
filePaths: z.array(z.string().min(1)).nonempty(), | |
}; | |
const filePathsSchema = { | |
filePaths: z | |
.array( | |
z | |
.string() | |
.min(1) | |
.describe("Full filesystem path of a file to be analyzed"), | |
) | |
.nonempty(), | |
}; |
I can't reproduce the error in Cursor anymore. I tried to re-add the description from this commit and I configured Cursor to start the server directly from the path of mcp-cli.js
. I can see that lint-file
is working as expected now. How is it for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to stick with what we knew already worked in the previous release in this PR. We can make other changes like this as improvements later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Prerequisites checklist
What is the purpose of this pull request?
Adds a new
@eslint/mcp
package.What changes did you make? (Give an overview)
mcp-server.js
from theeslint
repo, along with tests.Related Issues
refs eslint/eslint#19682
Is there anything you'd like reviewers to focus on?
This package has no JSR equivalent because it's a binary.
Also, because it's a binary, I'm not rolling up or converting to CommonJS. It stays ESM and the
src
directory is published.These were simplifying decisions based on the uniqueness of this package. I think this is okay because there is no API exposed through the package.