refactor: rename package and remove legacy 'spec-maker' branding#43
refactor: rename package and remove legacy 'spec-maker' branding#43
Conversation
Complete package rename from mcp-github-pr-review-spec-maker to mcp-github-pr-review, removing legacy "spec" terminology throughout. ## Package Changes - Rename: mcp_github_pr_review_spec_maker → mcp_github_pr_review - Console script: mcp-github-pr-review-spec-maker → mcp-github-pr-review - Update GitHub org: petersouter → cool-kids-inc - Repository: github-pr-review-mcp-server ## Code Changes - Rename class: ReviewSpecGenerator → PRReviewServer - Update server name: github_review_spec_generator → github_pr_review - Update User-Agent: mcp-pr-review-spec-maker/1.0 → mcp-pr-review/1.0 - Dynamic version from importlib.metadata - Enhanced tool descriptions with key features ## Documentation Updates - Update all module references and import paths - Update mkdocs.yml with new org and package name - Fix run-server.sh with correct paths and server name - Update README, CLAUDE.md, AGENTS.md, SECURITY.md - Polish all docs/ content for consistency - Remove obsolete "spec" terminology ## Testing - Update 190 tests with new imports and assertions - All tests passing (100%) - All quality checks passing (ruff, mypy, compile-check) This refactor provides clearer branding and aligns the package name with its actual functionality: fetching and formatting PR review comments, not creating specification files. 🤖 Generated with Claude Code
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRelocates the MCP GitHub PR review server into a new package at Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Host as MCP Host (client)
participant CLI as mcp-github-pr-review
participant Server as PRReviewServer
participant Resolver as Git Resolver
participant GitHub as GitHub API
Host->>CLI: invoke console script
CLI->>CLI: parse args & load .env
CLI->>Server: start async server
Server->>Host: register tools
Host->>Server: call fetch_pr_review_comments(pr_url?, output, limits)
alt pr_url missing
Server->>Resolver: resolve_open_pr_url(strategy)
Resolver->>GitHub: list PRs / query repo
GitHub-->>Resolver: PR metadata
Resolver-->>Server: pr_url
end
Server->>GitHub: fetch_pr_comments_graphql(owner,repo,pr_num,host)
alt graphql fails or incomplete
Server->>GitHub: fetch_pr_comments REST (paginated)
end
Server->>Server: generate_markdown(comments)
Server-->>Host: return Markdown / JSON / both
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Summary of ChangesHello @petems, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant refactoring to rename the project's Python package and internal components, shedding outdated branding and aligning the codebase with its current purpose of fetching and formatting GitHub PR review comments. The changes encompass file renames, import updates, and the introduction of a new, comprehensive documentation site, all aimed at improving maintainability, clarity, and user experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a comprehensive refactoring that successfully renames the package and removes the legacy 'spec-maker' branding. The code changes are clean and consistent, and the addition of a full documentation site with MkDocs is a great improvement. I've found a few minor typos and inconsistencies in the documentation and configuration files, which I've detailed in the comments. Overall, this is a solid pull request that significantly improves the project's clarity and maintainability.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/mcp_github_pr_review/git_pr_resolver.py (1)
222-231: Ensure we always return an HTML URLDocstring promises an HTML PR URL, but fallback returns API “url” when “html_url” is missing. Prefer constructing HTML when html_url is absent.
Apply:
- def get_url(pr_dict: dict[str, Any]) -> str: - url = pr_dict.get("html_url") or pr_dict.get("url") - if url: - return str(url) + def get_url(pr_dict: dict[str, Any]) -> str: + html = pr_dict.get("html_url") + if html: + return str(html) number = pr_dict.get("number") try: num_str = str(int(number)) if number is not None else "unknown" except (ValueError, TypeError): num_str = "unknown" return f"https://{actual_host}/{owner}/{repo}/pull/{num_str}"pyproject.toml (1)
92-92: Updateknown-first-partyto reflect the new package name.The
isortconfiguration still references the old module namemcp_server, but the package has been renamed tomcp_github_pr_review. This will cause import sorting to treat the new package as third-party rather than first-party.Apply this diff:
[tool.ruff.lint.isort] -known-first-party = ["mcp_server"] +known-first-party = ["mcp_github_pr_review"]
🧹 Nitpick comments (10)
requirements.txt (1)
5-5: Dulwich is properly integrated and secure; consider tightening the version constraint as a best practice.Dulwich is actively used in the codebase via
src/mcp_github_pr_review/git_pr_resolver.py. The version constraint 0.22.0 is safe from known advisories (all documented CVEs are patched in versions well before 0.22.0).However, consider tightening the version constraint to prevent unexpected major version breakage:
-dulwich>=0.22.0 +dulwich>=0.22.0,<1.0.0This follows packaging best practices by allowing patch/minor updates while preventing hypothetical breaking changes from a future major version release.
src/mcp_github_pr_review/git_pr_resolver.py (2)
263-269: Honor configurable per‑page size when listing PRsWe already document HTTP_PER_PAGE. Consider adding per_page to reduce payloads and align with config.
- url = ( - f"{api_base}/repos/{owner}/{repo}/pulls" - "?state=open&sort=updated&direction=desc" - ) + per_page = int(os.getenv("HTTP_PER_PAGE", "100")) + per_page = max(1, min(per_page, 100)) + url = ( + f"{api_base}/repos/{owner}/{repo}/pulls" + f"?state=open&sort=updated&direction=desc&per_page={per_page}" + )
28-37: Optional: support ssh:// form in remote URLsAdd regex for ssh://git@host/owner/repo(.git) to cover another common format.
REMOTE_REGEXES = [ # SSH: git@github.com:owner/repo.git re.compile( r"^(?:git@)(?P<host>[^:]+):(?P<owner>[^/]+)/(?P<repo>[^/]+?)(?:\.git)?$" ), + # SSH scheme: ssh://git@github.com/owner/repo(.git) + re.compile( + r"^ssh://(?:git@)?(?P<host>[^/]+)/(?P<owner>[^/]+)/(?P<repo>[^/]+?)(?:\.git)?/?$" + ), # HTTPS: https://github.com/owner/repo(.git) re.compile( r"^https?://(?P<host>[^/]+)/(?P<owner>[^/]+)/(?P<repo>[^/]+?)(?:\.git)?/?$" ), ]src/mcp_github_pr_review/github_api_constants.py (1)
6-6: User‑Agent should include dynamic package version and align with CLI nameStatic “mcp-pr-review/1.0” will go stale and doesn’t match the CLI “mcp-github-pr-review”. Propose dynamic UA with robust fallback.
- GITHUB_USER_AGENT = "mcp-pr-review/1.0" + import os + from importlib.metadata import PackageNotFoundError, version + + _UA_NAME = "mcp-github-pr-review" + try: + _pkg_ver = version("mcp_github_pr_review") + except PackageNotFoundError: + _pkg_ver = os.getenv("PACKAGE_VERSION", "0") + GITHUB_USER_AGENT = f"{_UA_NAME}/{_pkg_ver}"src/mcp_github_pr_review/__init__.py (1)
3-5: Minimal public surface LGTMRe‑exporting only create_server keeps API tight.
Optionally expose
__version__via importlib.metadata for convenience.+from importlib.metadata import PackageNotFoundError, version as _version + +try: + __version__ = _version("mcp_github_pr_review") +except PackageNotFoundError: # dev/editable fallback + __version__ = "0" + __all__ = ["create_server"]docs/guides/editor-integrations.md (1)
7-13: Add GH_HOST note and token security reminderBriefly mention GH_HOST for GHES users and advise against committing tokens; reference .env usage.
- **Environment**: Provide `GITHUB_TOKEN` if not already available in your shell. + - **Environment**: Provide `GITHUB_TOKEN` (do not commit to repo). For GitHub Enterprise, also set `GH_HOST`. @@ [mcp_servers.pr-review.env] GITHUB_TOKEN = "${GITHUB_TOKEN}" +# For GitHub Enterprise: +# GH_HOST = "ghe.example.com" @@ "env": { "GITHUB_TOKEN": "${GITHUB_TOKEN}" + // For GitHub Enterprise: + // "GH_HOST": "ghe.example.com" }Also applies to: 18-27, 30-43, 46-54, 56-56
mcp_server.py (2)
1-4: Emit a deprecation warning on importHelp downstreams notice the move to the new package path.
"""Compatibility wrapper for the legacy module path.""" -from mcp_github_pr_review.server import * # noqa: F401,F403 +import warnings +from mcp_github_pr_review.server import * # noqa: F401,F403 + +warnings.warn( + "mcp_server is deprecated; import from mcp_github_pr_review.server instead.", + DeprecationWarning, + stacklevel=2, +)
1-4: Optional: alias legacy symbols for smoother migrationIf feasible, provide old names pointing to new ones for a deprecation period.
-from mcp_github_pr_review.server import * # noqa: F401,F403 +from mcp_github_pr_review.server import * # noqa: F401,F403 +try: + # Legacy alias (no-op if not needed) + ReviewSpecGenerator = PRReviewServer # type: ignore[name-defined] +except Exception: + passSECURITY.md (1)
9-9: Fix redundant wording.The phrase "This MCP server originally included..." could be more specific. Consider using "This server" or "The GitHub PR Review MCP Server" for consistency with the corrected text above.
Apply this diff:
-This MCP server originally included functionality to create markdown markdown files on disk via a `create_review_spec_file` tool. This feature was intentionally removed due to security complexity and concerns. +This server originally included functionality to create markdown files on disk via a `create_review_spec_file` tool. This feature was intentionally removed due to security complexity and concerns.docs/guides/remote-uv-endpoint.md (1)
7-9: Optional: Add language specifier to fenced code block.The ASCII architecture diagram at lines 7-9 lacks a language specifier. While this is flagged by markdownlint, it's a minor formatting issue.
You can add
textas the language:-``` +```text client host (Claude, Codex) ──ssh──► uv-managed worker ──► GitHub API</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 28afc86453609eef77b6195477ebdc5d943c266d and 7a56f123da1c6746c6ede9d527d4cdb1dc56202c. </details> <details> <summary>⛔ Files ignored due to path filters (2)</summary> * `docs/assets/demo.gif` is excluded by `!**/*.gif` * `uv.lock` is excluded by `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (52)</summary> * `AGENTS.md` (3 hunks) * `CLAUDE.md` (3 hunks) * `README.md` (5 hunks) * `SECURITY.md` (1 hunks) * `UV_COMMANDS.md` (1 hunks) * `docs/architecture/overview.md` (1 hunks) * `docs/changelog.md` (1 hunks) * `docs/contributing/index.md` (1 hunks) * `docs/getting-started/configuration.md` (1 hunks) * `docs/getting-started/installation.md` (1 hunks) * `docs/getting-started/quickstart.md` (1 hunks) * `docs/guides/editor-integrations.md` (1 hunks) * `docs/guides/local-stdio.md` (1 hunks) * `docs/guides/remote-uv-endpoint.md` (1 hunks) * `docs/index.md` (1 hunks) * `docs/reference/cli.md` (1 hunks) * `docs/reference/environment.md` (1 hunks) * `docs/reference/mcp-manifest.md` (1 hunks) * `docs/reference/publishing.md` (1 hunks) * `docs/reference/tools.md` (1 hunks) * `docs/security/index.md` (1 hunks) * `mcp_server.py` (1 hunks) * `mkdocs.yml` (1 hunks) * `prompts/GITHUB_API_ROBUSTNESS.md` (7 hunks) * `pyproject.toml` (3 hunks) * `requirements.txt` (1 hunks) * `run-server.sh` (9 hunks) * `specs/PHASE2_ENTERPRISE_GITHUB_URLS.md` (7 hunks) * `src/mcp_github_pr_review/__init__.py` (1 hunks) * `src/mcp_github_pr_review/__main__.py` (1 hunks) * `src/mcp_github_pr_review/cli.py` (1 hunks) * `src/mcp_github_pr_review/git_pr_resolver.py` (1 hunks) * `src/mcp_github_pr_review/github_api_constants.py` (1 hunks) * `src/mcp_github_pr_review/mcp.json` (1 hunks) * `src/mcp_github_pr_review/server.py` (1 hunks) * `tests/conftest.py` (3 hunks) * `tests/test_api_headers.py` (2 hunks) * `tests/test_config_edge_cases.py` (1 hunks) * `tests/test_config_helpers.py` (1 hunks) * `tests/test_enterprise_url_support.py` (3 hunks) * `tests/test_get_pr_info.py` (1 hunks) * `tests/test_git_pr_resolver.py` (27 hunks) * `tests/test_graphql_error_handling.py` (1 hunks) * `tests/test_graphql_timeout.py` (1 hunks) * `tests/test_integration.py` (5 hunks) * `tests/test_mcp_server_tools.py` (31 hunks) * `tests/test_null_author.py` (1 hunks) * `tests/test_pagination_limits.py` (1 hunks) * `tests/test_rest_error_handling.py` (12 hunks) * `tests/test_rest_timeout.py` (1 hunks) * `tests/test_security.py` (2 hunks) * `tests/test_status_fields.py` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (9)</summary> <details> <summary>**/*.py</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > `**/*.py`: Format Python code with Ruff and adhere to a maximum line length of 88 characters > Ensure imports are sorted per Ruff’s configuration > Code must be compatible with and target Python 3.10+ features/configuration > Address security findings covered by Ruff’s bandit rules; avoid insecure patterns (e.g., unsafe subprocess, eval) > Pass static type checks with mypy; add/maintain type annotations as needed > Python files must compile without SyntaxError (compile-check must pass) > > `**/*.py`: Code must pass ruff linting with zero violations (ruff check .) > Code must be formatted with ruff format . > Static type checking must pass with mypy > Use type hints for all function parameters and return values > Follow async/await patterns for I/O operations (GitHub API calls, file operations) > Handle errors gracefully with proper exception handling and logging > Log errors to stderr using print(..., file=sys.stderr) > Use traceback.print_exc(file=sys.stderr) for debugging when emitting stack traces > Implement retry logic for transient failures honoring HTTP_MAX_RETRIES > Validate all input parameters (URLs, filenames, numeric limits) > Use safe file operations (e.g., exclusive create, path validation) > Implement rate limiting and pagination safety caps (e.g., PR_FETCH_MAX_PAGES, PR_FETCH_MAX_COMMENTS) for GitHub API usage Files: - `tests/test_status_fields.py` - `src/mcp_github_pr_review/__main__.py` - `tests/test_pagination_limits.py` - `tests/test_null_author.py` - `tests/test_graphql_error_handling.py` - `src/mcp_github_pr_review/github_api_constants.py` - `tests/test_rest_error_handling.py` - `tests/test_security.py` - `src/mcp_github_pr_review/__init__.py` - `tests/test_graphql_timeout.py` - `tests/test_git_pr_resolver.py` - `tests/test_rest_timeout.py` - `tests/test_config_edge_cases.py` - `tests/test_api_headers.py` - `src/mcp_github_pr_review/cli.py` - `src/mcp_github_pr_review/git_pr_resolver.py` - `tests/test_config_helpers.py` - `tests/test_get_pr_info.py` - `tests/test_integration.py` - `src/mcp_github_pr_review/server.py` - `tests/test_mcp_server_tools.py` - `tests/test_enterprise_url_support.py` - `tests/conftest.py` - `mcp_server.py` </details> <details> <summary>tests/**/*.py</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > `tests/**/*.py`: Use pytest for all tests and keep tests under the tests/ directory > Avoid live GitHub calls in tests by default; use mocks/fixtures, and keep real-call integration tests token-gated/conditionally skipped Files: - `tests/test_status_fields.py` - `tests/test_pagination_limits.py` - `tests/test_null_author.py` - `tests/test_graphql_error_handling.py` - `tests/test_rest_error_handling.py` - `tests/test_security.py` - `tests/test_graphql_timeout.py` - `tests/test_git_pr_resolver.py` - `tests/test_rest_timeout.py` - `tests/test_config_edge_cases.py` - `tests/test_api_headers.py` - `tests/test_config_helpers.py` - `tests/test_get_pr_info.py` - `tests/test_integration.py` - `tests/test_mcp_server_tools.py` - `tests/test_enterprise_url_support.py` - `tests/conftest.py` </details> <details> <summary>tests/test_*.py</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > `tests/test_*.py`: Place test files under tests/ with filenames matching tests/test_*.py > Name test functions with the test_* prefix > Use pytest-asyncio for async test functions Files: - `tests/test_status_fields.py` - `tests/test_pagination_limits.py` - `tests/test_null_author.py` - `tests/test_graphql_error_handling.py` - `tests/test_rest_error_handling.py` - `tests/test_security.py` - `tests/test_graphql_timeout.py` - `tests/test_git_pr_resolver.py` - `tests/test_rest_timeout.py` - `tests/test_config_edge_cases.py` - `tests/test_api_headers.py` - `tests/test_config_helpers.py` - `tests/test_get_pr_info.py` - `tests/test_integration.py` - `tests/test_mcp_server_tools.py` - `tests/test_enterprise_url_support.py` </details> <details> <summary>tests/test_pagination_limits.py</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > Maintain pagination safety tests in tests/test_pagination_limits.py Files: - `tests/test_pagination_limits.py` </details> <details> <summary>SECURITY.md</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > Read and adhere to SECURITY.md warnings before using this MCP server in agent workflows Files: - `SECURITY.md` </details> <details> <summary>tests/test_integration.py</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > Keep integration tests in tests/test_integration.py (skips when GITHUB_TOKEN is not set) Files: - `tests/test_integration.py` </details> <details> <summary>pyproject.toml</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > Project must target and enforce Python 3.10+ via pyproject.toml Files: - `pyproject.toml` </details> <details> <summary>tests/conftest.py</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > Place shared pytest fixtures (HTTP client mock, git context, temp dirs, timeouts) in tests/conftest.py > > Share common pytest fixtures in tests/conftest.py Files: - `tests/conftest.py` </details> <details> <summary>mcp_server.py</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > `mcp_server.py`: All GitHub API interactions should be async/await based > Implement robust error handling for GitHub calls (timeouts, API errors, rate limiting) with retries as appropriate > Respect environment-based configuration (GITHUB_TOKEN required; GH_HOST, GITHUB_API_URL, GITHUB_GRAPHQL_URL supported) > Enforce pagination safety limits for GitHub API calls (max pages/comments, per-page size, retry caps) > Generate markdown with dynamic fencing to safely handle nested backticks in output Files: - `mcp_server.py` </details> </details><details> <summary>🧠 Learnings (5)</summary> <details> <summary>📚 Learning: 2025-10-17T08:04:11.505Z</summary>Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : All GitHub API interactions should be async/await based**Applied to files:** - `tests/test_null_author.py` - `tests/test_graphql_error_handling.py` - `tests/test_graphql_timeout.py` - `tests/test_rest_timeout.py` - `specs/PHASE2_ENTERPRISE_GITHUB_URLS.md` - `prompts/GITHUB_API_ROBUSTNESS.md` - `tests/test_mcp_server_tools.py` </details> <details> <summary>📚 Learning: 2025-10-17T08:04:11.505Z</summary>Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : Respect environment-based configuration (GITHUB_TOKEN required; GH_HOST, GITHUB_API_URL, GITHUB_GRAPHQL_URL supported)**Applied to files:** - `docs/getting-started/configuration.md` - `tests/test_git_pr_resolver.py` - `docs/reference/environment.md` - `specs/PHASE2_ENTERPRISE_GITHUB_URLS.md` - `prompts/GITHUB_API_ROBUSTNESS.md` - `run-server.sh` </details> <details> <summary>📚 Learning: 2025-10-17T08:04:11.505Z</summary>Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Run the full quality pipeline locally before any push: ruff format, ruff check --fix, mypy, compile check, and pytest; do not push unless all pass**Applied to files:** - `CLAUDE.md` </details> <details> <summary>📚 Learning: 2025-10-17T08:04:11.505Z</summary>Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : Implement robust error handling for GitHub calls (timeouts, API errors, rate limiting) with retries as appropriate**Applied to files:** - `prompts/GITHUB_API_ROBUSTNESS.md` </details> <details> <summary>📚 Learning: 2025-10-17T08:04:11.505Z</summary>Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to tests/conftest.py : Place shared pytest fixtures (HTTP client mock, git context, temp dirs, timeouts) in tests/conftest.py**Applied to files:** - `tests/conftest.py` </details> </details><details> <summary>🧬 Code graph analysis (22)</summary> <details> <summary>tests/test_status_fields.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `generate_markdown` (661-738) </details> </blockquote></details> <details> <summary>src/mcp_github_pr_review/__main__.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/cli.py (1)</summary> * `main` (59-85) </details> </blockquote></details> <details> <summary>tests/test_pagination_limits.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `fetch_pr_comments` (455-658) </details> </blockquote></details> <details> <summary>tests/test_null_author.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `fetch_pr_comments_graphql` (261-452) </details> </blockquote></details> <details> <summary>tests/test_graphql_error_handling.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `fetch_pr_comments_graphql` (261-452) </details> </blockquote></details> <details> <summary>tests/test_rest_error_handling.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `fetch_pr_comments` (455-658) </details> </blockquote></details> <details> <summary>tests/test_security.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (2)</summary> * `escape_html_safe` (46-57) * `generate_markdown` (661-738) </details> </blockquote></details> <details> <summary>src/mcp_github_pr_review/__init__.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `create_server` (1186-1189) </details> </blockquote></details> <details> <summary>tests/test_graphql_timeout.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `fetch_pr_comments_graphql` (261-452) </details> </blockquote></details> <details> <summary>tests/test_git_pr_resolver.py (2)</summary><blockquote> <details> <summary>tests/conftest.py (1)</summary> * `FakeClient` (418-450) </details> <details> <summary>src/mcp_github_pr_review/git_pr_resolver.py (4)</summary> * `_get_repo` (73-79) * `git_detect_repo_branch` (82-127) * `graphql_url_for_host` (300-346) * `_html_pr_url` (349-350) </details> </blockquote></details> <details> <summary>tests/test_rest_timeout.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `fetch_pr_comments` (455-658) </details> </blockquote></details> <details> <summary>tests/test_config_edge_cases.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `_int_conf` (69-99) </details> </blockquote></details> <details> <summary>tests/test_api_headers.py (2)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/git_pr_resolver.py (1)</summary> * `resolve_pr_url` (161-297) </details> <details> <summary>src/mcp_github_pr_review/server.py (2)</summary> * `fetch_pr_comments` (455-658) * `fetch_pr_comments_graphql` (261-452) </details> </blockquote></details> <details> <summary>src/mcp_github_pr_review/cli.py (2)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (2)</summary> * `PRReviewServer` (744-1183) * `run` (1158-1183) </details> <details> <summary>run-server.sh (1)</summary> * `parse_args` (621-644) </details> </blockquote></details> <details> <summary>tests/test_get_pr_info.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `get_pr_info` (226-258) </details> </blockquote></details> <details> <summary>tests/test_integration.py (2)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `PRReviewServer` (744-1183) </details> <details> <summary>tests/conftest.py (4)</summary> * `mcp_server` (265-269) * `mock_http_client` (200-209) * `temp_review_specs_dir` (257-261) * `sample_pr_comments` (276-312) </details> </blockquote></details> <details> <summary>src/mcp_github_pr_review/server.py (2)</summary><blockquote> <details> <summary>tests/conftest.py (6)</summary> * `json` (408-409) * `raise_for_status` (411-415) * `post` (138-148) * `post` (440-450) * `get` (128-136) * `get` (430-438) </details> <details> <summary>tests/test_git_pr_resolver.py (9)</summary> * `post` (522-524) * `post` (565-566) * `post` (672-674) * `post` (701-703) * `get` (48-53) * `get` (311-312) * `get` (328-329) * `get` (346-347) * `get` (363-370) </details> </blockquote></details> <details> <summary>tests/test_mcp_server_tools.py (2)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (2)</summary> * `PRReviewServer` (744-1183) * `handle_call_tool` (891-1064) </details> <details> <summary>tests/conftest.py (1)</summary> * `mcp_server` (265-269) </details> </blockquote></details> <details> <summary>tests/test_enterprise_url_support.py (2)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/git_pr_resolver.py (1)</summary> * `graphql_url_for_host` (300-346) </details> <details> <summary>src/mcp_github_pr_review/server.py (3)</summary> * `fetch_pr_comments` (455-658) * `fetch_pr_comments_graphql` (261-452) * `get_pr_info` (226-258) </details> </blockquote></details> <details> <summary>tests/conftest.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `PRReviewServer` (744-1183) </details> </blockquote></details> <details> <summary>mcp_server.py (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/cli.py (1)</summary> * `main` (59-85) </details> </blockquote></details> <details> <summary>run-server.sh (1)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/server.py (1)</summary> * `run` (1158-1183) </details> </blockquote></details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/reference/mcp-manifest.md</summary> [uncategorized] ~35-~35: The official name of this software platform is spelled with a capital “H”. Context: ...son` inside the package directory (e.g. `src/mcp_github_pr_review/mcp.json`). Update `MANIFEST.... (GITHUB) </details> <details> <summary>docs/reference/cli.md</summary> [grammar] ~15-~15: Use a hyphen to join words. Context: ...s process. | | `--json-logs` | Emit JSON formatted log lines. | | `--help` | Show... (QB_NEW_EN_HYPHEN) </details> <details> <summary>docs/getting-started/configuration.md</summary> [uncategorized] ~8-~8: The official name of this software platform is spelled with a capital “H”. Context: ...l-request comments. | | `GH_HOST` | ❌ | `github.com` | GitHub Enterprise hostname. Auto... (GITHUB) </details> <details> <summary>docs/getting-started/installation.md</summary> [uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”. Context: ...view --help ``` This entry point wraps `uv run python -m mcp_github_pr_review.server` when installed from P... (GITHUB) --- [uncategorized] ~44-~44: The official name of this software platform is spelled with a capital “H”. Context: ...``` The server can then be imported as `mcp_github_pr_review` for programmatic integration... (GITHUB) </details> <details> <summary>docs/reference/environment.md</summary> [uncategorized] ~8-~8: The official name of this software platform is spelled with a capital “H”. Context: ...t:read` scope. | | `GH_HOST` | string | `github.com` | Automatically updates `GITHUB_AP... (GITHUB) </details> <details> <summary>specs/PHASE2_ENTERPRISE_GITHUB_URLS.md</summary> [uncategorized] ~24-~24: The official name of this software platform is spelled with a capital “H”. Context: ...et ### Current Hardcoded URLs (src/mcp_github_pr_review_spec_maker/server.py) These ... (GITHUB) --- [uncategorized] ~76-~76: The official name of this software platform is spelled with a capital “H”. Context: ...mport Enterprise URL Helpers in src/mcp_github_pr_review_spec_maker/server.py **File*... (GITHUB) </details> <details> <summary>prompts/GITHUB_API_ROBUSTNESS.md</summary> [uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”. Context: ...PI Headers** (both files): - Change: `Accept: application/vnd.github.v3+json` → `Accept: application/vnd.git... (GITHUB) --- [uncategorized] ~28-~28: The official name of this software platform is spelled with a capital “H”. Context: ...i-Version: 2022-11-28` - Location in `src/mcp_github_pr_review_spec_maker/server.py`: Lines ... (GITHUB) --- [uncategorized] ~32-~32: The official name of this software platform is spelled with a capital “H”. Context: ...L API Headers** (both files): - Add: `Accept: application/vnd.github+json` (currently missing) - Add: `X-... (GITHUB) --- [uncategorized] ~34-~34: The official name of this software platform is spelled with a capital “H”. Context: ...i-Version: 2022-11-28` - Location in `src/mcp_github_pr_review_spec_maker/server.py`: Lines ... (GITHUB) --- [uncategorized] ~89-~89: The official name of this software platform is spelled with a capital “H”. Context: ...se GitHub URL construction 2. **Update `src/mcp_github_pr_review_spec_maker/server.py` URL con... (GITHUB) --- [uncategorized] ~169-~169: The official name of this software platform is spelled with a capital “H”. Context: ...r GitHub support tickets - Location: `src/mcp_github_pr_review_spec_maker/server.py` lines 4... (GITHUB) --- [uncategorized] ~177-~177: The official name of this software platform is spelled with a capital “H”. Context: ...L Rate Limit Handling**: - Location: `src/mcp_github_pr_review_spec_maker/server.py` lines 2... (GITHUB) --- [uncategorized] ~235-~235: The official name of this software platform is spelled with a capital “H”. Context: ...: `min(15.0, backoff)` - Location in `src/mcp_github_pr_review_spec_maker/server.py`: Line 4... (GITHUB) </details> <details> <summary>docs/architecture/overview.md</summary> [uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”. Context: ...CP tools by registering their schema in `src/mcp_github_pr_review/server.py` and implementing a... (GITHUB) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>docs/getting-started/configuration.md</summary> 23-23: Heading levels should only increment by one level at a time Expected: h2; Actual: h3 (MD001, heading-increment) </details> <details> <summary>docs/guides/remote-uv-endpoint.md</summary> 7-7: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (65)</summary><blockquote> <details> <summary>src/mcp_github_pr_review/git_pr_resolver.py (1)</summary><blockquote> `13-17`: **Relative import LGTM** Switch to package‑relative constants is correct and keeps intra‑package refs stable. </blockquote></details> <details> <summary>mcp_server.py (1)</summary><blockquote> `1-8`: **Rename verification complete—no critical legacy references in code** The rename from `mcp_github_pr_review_spec_maker` to `mcp_github_pr_review` is comprehensive: - Directory structure: Old `src/mcp_github_pr_review_spec_maker` removed; new `src/mcp_github_pr_review` in place - Code imports: All Python files (mcp_server.py, tests, modules) use the new module name `mcp_github_pr_review` - Compatibility wrapper (mcp_server.py lines 1-8): Correctly imports from `mcp_github_pr_review.server` and `mcp_github_pr_review.cli` No legacy identifiers remain in executable code or imports. Documentation/comments referencing old paths are historical records of the refactoring, not active issues. </blockquote></details> <details> <summary>tests/test_status_fields.py (1)</summary><blockquote> `3-3`: **LGTM! Import path updated correctly.** The import path has been updated to reflect the new package structure. The `generate_markdown` function maintains the same signature and behavior. </blockquote></details> <details> <summary>tests/test_graphql_error_handling.py (1)</summary><blockquote> `8-8`: **LGTM! Import path updated correctly.** The import path has been updated to reflect the new package structure. The `fetch_pr_comments_graphql` function maintains the same signature and behavior. </blockquote></details> <details> <summary>src/mcp_github_pr_review/__main__.py (1)</summary><blockquote> `1-4`: **LGTM! Clean CLI entry point implementation.** The implementation follows Python best practices: - Uses `raise SystemExit(main())` for proper exit code handling - Includes appropriate coverage pragma for the `if __name__` block - Clean separation of concerns with CLI logic in a separate module </blockquote></details> <details> <summary>tests/test_rest_timeout.py (1)</summary><blockquote> `7-7`: **LGTM! Import path updated correctly.** The import path has been updated to reflect the new package structure. The `fetch_pr_comments` function maintains the same signature and behavior. </blockquote></details> <details> <summary>tests/test_pagination_limits.py (1)</summary><blockquote> `16-16`: **LGTM! Import path updated correctly.** The import path has been updated to reflect the new package structure. The `fetch_pr_comments` function maintains the same signature and behavior. </blockquote></details> <details> <summary>UV_COMMANDS.md (1)</summary><blockquote> `24-24`: **LGTM! Command updated to use new CLI entry point.** The command has been correctly updated to use the new `mcp-github-pr-review` CLI entry point, which aligns with the package restructuring and new CLI implementation. </blockquote></details> <details> <summary>tests/test_config_edge_cases.py (1)</summary><blockquote> `5-5`: **LGTM! Import path updated correctly.** The import path has been updated to reflect the new package structure. The `_int_conf` function maintains the same signature and behavior. </blockquote></details> <details> <summary>tests/test_graphql_timeout.py (1)</summary><blockquote> `7-7`: **LGTM!** The import path update correctly reflects the package refactoring from `mcp_server` to `mcp_github_pr_review.server`. </blockquote></details> <details> <summary>docs/changelog.md (1)</summary><blockquote> `1-8`: **LGTM!** The changelog follows the Keep a Changelog format and appropriately documents the major changes in this refactoring PR. </blockquote></details> <details> <summary>tests/test_null_author.py (1)</summary><blockquote> `7-7`: **LGTM!** The import path update is consistent with the package refactoring. </blockquote></details> <details> <summary>tests/test_config_helpers.py (1)</summary><blockquote> `7-22`: **LGTM!** The import path updates correctly reflect the package refactoring. All configuration constants and helper functions are properly imported from the new module location. </blockquote></details> <details> <summary>tests/test_get_pr_info.py (1)</summary><blockquote> `3-3`: **LGTM!** The import path update is consistent with the package refactoring. </blockquote></details> <details> <summary>docs/guides/local-stdio.md (1)</summary><blockquote> `1-27`: **LGTM!** The local stdio execution guide is clear and comprehensive. The setup steps, optional flags, and troubleshooting tips provide helpful guidance for users running the server locally. </blockquote></details> <details> <summary>AGENTS.md (1)</summary><blockquote> `124-124`: **LGTM!** The command references have been correctly updated to use the new `mcp-github-pr-review` entry point. Also applies to: 237-237 </blockquote></details> <details> <summary>docs/contributing/index.md (1)</summary><blockquote> `1-37`: **LGTM! Clear and comprehensive contributing guide.** The setup instructions and development workflow are well-documented. All commands are appropriate for the uv-based workflow described in the PR objectives. </blockquote></details> <details> <summary>tests/test_rest_error_handling.py (2)</summary><blockquote> `9-9`: **LGTM! Import path correctly updated.** The import path has been updated consistently with the package restructuring from `mcp_server` to `mcp_github_pr_review.server`. --- `56-58`: **LGTM! Mock patch targets correctly updated.** All patch targets and monkeypatch.setattr calls have been updated to reference the new module path `mcp_github_pr_review.server`, maintaining test coverage during the refactoring. Also applies to: 76-76, 91-93, 112-112, 127-129, 144-144, 157-159, 184-186, 202-204, 226-228, 244-246 </blockquote></details> <details> <summary>docs/index.md (1)</summary><blockquote> `1-14`: **LGTM! Clear and well-organized documentation landing page.** The introduction effectively orients readers and provides a logical navigation structure for the documentation suite. </blockquote></details> <details> <summary>tests/test_security.py (2)</summary><blockquote> `3-3`: **LGTM! Import path correctly updated.** The import path has been updated to `mcp_github_pr_review.server` consistent with the package restructuring. --- `120-120`: **LGTM! Test assertion updated to reflect rebranding.** The expected heading has been updated from "Pull Request Review Spec" to "Pull Request Review Comments", correctly reflecting the removal of legacy "spec-maker" terminology documented in the PR objectives. </blockquote></details> <details> <summary>docs/getting-started/quickstart.md (1)</summary><blockquote> `25-26`: **LGTM! Correct CLI entry point and verification commands.** The `mcp-github-pr-review` command and `claude mcp call pr-review list-tools` verification correctly reference the new CLI entry point and server name. Also applies to: 35-38 </blockquote></details> <details> <summary>docs/getting-started/installation.md (1)</summary><blockquote> `1-44`: **LGTM! Clear and accurate installation guide.** The installation instructions correctly document the new package name `mcp-github-pr-review` and CLI entry point `mcp-github-pr-review` across different installation methods. Note: The static analysis hints about "GitHub" capitalization are false positives—`mcp_github_pr_review` is a Python package identifier, not a reference to the GitHub platform. </blockquote></details> <details> <summary>docs/reference/mcp-manifest.md (1)</summary><blockquote> `1-44`: **LGTM! Comprehensive manifest documentation.** The manifest structure, distribution guidance, and marketplace notes are clearly documented. The example JSON correctly reflects the new `pr-review` server name and `mcp-github-pr-review` command. Note: The static analysis hint about "GitHub" capitalization on line 35 is a false positive—it's referring to a Python package path, not the GitHub platform. </blockquote></details> <details> <summary>docs/reference/publishing.md (1)</summary><blockquote> `1-20`: **LGTM! Clear and comprehensive publishing workflow.** The PyPI publishing steps are well-documented, including version synchronization across `pyproject.toml` and `mcp.json`, distribution building with Hatch, and release automation guidance. </blockquote></details> <details> <summary>CLAUDE.md (1)</summary><blockquote> `40-40`: **LGTM! Documentation accurately reflects the package rename.** The documentation updates correctly reference the new command `mcp-github-pr-review`, the renamed `PRReviewServer` class, and the updated module path `src/mcp_github_pr_review/server.py`. Also applies to: 51-51, 55-55, 90-90 </blockquote></details> <details> <summary>mkdocs.yml (1)</summary><blockquote> `1-66`: **LGTM! Well-structured MkDocs configuration.** The documentation site configuration is comprehensive and follows MkDocs best practices. The mkdocstrings plugin is correctly configured to generate API documentation from the `src` directory, and the navigation structure provides good coverage of all documentation sections. </blockquote></details> <details> <summary>docs/reference/cli.md (1)</summary><blockquote> `1-18`: **LGTM! Clear and comprehensive CLI documentation.** The CLI reference accurately documents all available options and their behavior. The note about environment variables and CLI flag precedence is helpful. </blockquote></details> <details> <summary>docs/security/index.md (1)</summary><blockquote> `1-24`: **LGTM! Well-structured security guidance.** The security documentation provides clear threat modeling, required controls, and incident response procedures. The references to SECURITY.md are appropriate for detailed mitigation strategies. </blockquote></details> <details> <summary>tests/test_api_headers.py (1)</summary><blockquote> `6-12`: **LGTM! Imports and User-Agent correctly updated.** The test file has been properly updated to import from the new `mcp_github_pr_review` package paths, and the User-Agent assertion correctly reflects the new branding `mcp-pr-review/1.0`. Also applies to: 149-149 </blockquote></details> <details> <summary>tests/conftest.py (1)</summary><blockquote> `2-2`: **LGTM! Fixture correctly updated for new server class.** The `mcp_server` fixture has been properly updated to import and instantiate `PRReviewServer` from the new `mcp_github_pr_review.server` module path. The docstring accurately reflects the change. Based on learnings Also applies to: 265-269 </blockquote></details> <details> <summary>pyproject.toml (1)</summary><blockquote> `2-2`: **LGTM! Package metadata and build configuration correctly updated.** The package has been properly renamed to `mcp-github-pr-review`, the console script entry point is configured, documentation tooling dependencies are added, and the build targets correctly include the new source structure and MCP manifest. Also applies to: 19-20, 29-32, 36-38, 45-59 </blockquote></details> <details> <summary>tests/test_enterprise_url_support.py (1)</summary><blockquote> `10-15`: **LGTM! Import paths correctly updated.** The imports now reference the new package structure (`mcp_github_pr_review.git_pr_resolver` and `mcp_github_pr_review.server`) which aligns with the package rename objective. </blockquote></details> <details> <summary>docs/reference/tools.md (1)</summary><blockquote> `1-37`: **LGTM! Clear and comprehensive tool documentation.** The documentation provides clear descriptions of the two MCP tools, their parameters, responses, and error codes. This will help users understand the API surface. </blockquote></details> <details> <summary>src/mcp_github_pr_review/mcp.json (1)</summary><blockquote> `1-29`: **LGTM! MCP manifest properly configured.** The manifest correctly references the new command name (`mcp-github-pr-review`), updated repository URLs, and includes all necessary metadata for the MCP server. </blockquote></details> <details> <summary>run-server.sh (2)</summary><blockquote> `5-5`: **LGTM! Headers and server name updated consistently.** The script headers and `SERVER_NAME` variable now reflect the new "pr-review" branding, removing the legacy "spec" terminology. Also applies to: 37-37, 654-654 --- `675-675`: **LGTM! Server path references updated correctly.** All references now point to the new location `src/mcp_github_pr_review/server.py`, consistent with the package restructuring. Also applies to: 710-712, 715-717, 720-722, 725-727, 735-737, 742-744 </blockquote></details> <details> <summary>tests/test_git_pr_resolver.py (2)</summary><blockquote> `10-15`: **LGTM! Import paths updated correctly.** Imports now reference `mcp_github_pr_review.git_pr_resolver`, consistent with the package rename. --- `56-57`: **LGTM! Monkeypatch targets updated consistently.** All patch targets now reference the new module path `mcp_github_pr_review.git_pr_resolver`, ensuring tests continue to work with the renamed package structure. Also applies to: 80-82, 100-101, 168-169, 283-284, 315-316, 332-333, 350-351, 373-374 </blockquote></details> <details> <summary>docs/architecture/overview.md (1)</summary><blockquote> `1-33`: **LGTM! Clear architecture documentation with correct module references.** The documentation provides a solid overview of the three subsystems and correctly references `src/mcp_github_pr_review/server.py` for the new package structure. </blockquote></details> <details> <summary>tests/test_mcp_server_tools.py (4)</summary><blockquote> `11-15`: **LGTM! Imports updated for new package structure.** The imports correctly reference `mcp_github_pr_review.server` and use the renamed `PRReviewServer` class. --- `21-21`: **LGTM! Test expectation updated for new branding.** The expected markdown header is now "Pull Request Review Comments" instead of "Pull Request Review Spec", removing the legacy "spec" terminology. --- `43-43`: **LGTM! Type annotations use renamed server class.** All test functions correctly use the `PRReviewServer` type annotation, consistent with the class rename from `ReviewSpecGenerator`. Also applies to: 53-53, 59-59, 68-68, 78-78, 88-88, 103-103, 114-114, 131-131, 160-160, 171-171, 225-225, 247-247, 303-303, 326-326, 345-345, 391-391, 408-408, 461-461, 480-480, 502-502, 539-539, 575-575 --- `120-121`: **LGTM! Monkeypatch targets updated consistently.** All patch targets now reference `mcp_github_pr_review.server.*`, aligning with the new package structure. Also applies to: 235-236, 267-268, 270-271, 312-312, 378-378, 394-394, 422-422, 508-508, 555-555, 591-591 </blockquote></details> <details> <summary>src/mcp_github_pr_review/cli.py (3)</summary><blockquote> `16-23`: **LGTM!** The positive integer validator correctly handles type conversion and value validation for CLI arguments. --- `26-56`: **LGTM!** The argument parser is well-structured with appropriate type validators and help text for all CLI options. --- `59-85`: **LGTM!** The main function implements a clean flow: - Proper dotenv loading with sensible override behavior - CLI arguments correctly override environment variables - Appropriate error handling for KeyboardInterrupt (exit 130) and unexpected errors - Clean server lifecycle management </blockquote></details> <details> <summary>tests/test_integration.py (3)</summary><blockquote> `21-27`: **LGTM!** Import paths correctly updated to reflect the new module structure (`mcp_github_pr_review.server`). --- `79-79`: **LGTM!** Assertion correctly updated to expect the new markdown heading that reflects the project's rebranding from "spec-maker" to "review". --- `136-146`: **LGTM!** Test fixture and mock patch paths correctly updated to reflect: - New class name (`PRReviewServer` instead of `ReviewSpecGenerator`) - New module structure (`mcp_github_pr_review.git_pr_resolver`) </blockquote></details> <details> <summary>README.md (4)</summary><blockquote> `1-20`: **LGTM!** Documentation header correctly updated with: - New project title reflecting the rebranding - Updated asset paths - Consistent command references (`mcp-github-pr-review`) - Reorganized documentation structure --- `117-134`: **LGTM!** Server execution commands correctly updated to use the new CLI entry point (`mcp-github-pr-review`) and registration name (`pr-review`). --- `154-164`: **LGTM!** Claude CLI configuration examples correctly updated with the new command structure and consistent naming. --- `166-198`: **LGTM!** Configuration examples for Codex and Gemini CLIs are consistent with the new naming convention throughout. </blockquote></details> <details> <summary>src/mcp_github_pr_review/server.py (10)</summary><blockquote> `1-43`: **LGTM!** Module initialization is well-structured: - Comprehensive imports for all required functionality - Proper dotenv loading at module level - Robust version detection with fallback for development scenarios --- `46-57`: **LGTM!** HTML escaping function implements proper security measures: - Escapes all HTML entities including quotes - Handles None values gracefully - Accepts any type and safely converts to string --- `60-123`: **LGTM!** Configuration management is well-implemented: - Clear parameter ranges defined as constants - Proper precedence: CLI override > environment variable > default - Safe type conversion with error handling - Values clamped to valid ranges to prevent invalid configurations --- `147-222`: **LGTM!** Retry logic implements industry best practices: - Exponential backoff with jitter to prevent thundering herd - Separate handling for network errors vs HTTP status errors - Flexible status handler for auth fallback and rate limiting - Appropriate delay caps and retry limits --- `226-258`: **LGTM!** PR URL parsing correctly handles: - Both GitHub.com and enterprise hosts - Optional trailing path segments, query strings, and fragments - Clear error messages for invalid formats - Proper return type with host extraction for enterprise support --- `261-452`: **LGTM!** GraphQL comment fetching is robust and feature-complete: - Enterprise GitHub support via dynamic URL construction - Comprehensive pagination with safety limits - Proper handling of null/deleted authors - Resolution and outdated status tracking - Configurable timeouts and retry logic - Appropriate error handling for timeouts and network errors --- `455-658`: **LGTM!** REST API comment fetching demonstrates production-ready implementation: - Intelligent auth fallback for fine-grained vs classic tokens - Rate limiting detection with Retry-After header support - Conservative error handling ensures reliability - Enterprise support via dynamic API base URLs - Comprehensive pagination with safety limits - Defensive URL encoding --- `661-738`: **LGTM!** Markdown generation implements comprehensive security and robustness: - Dynamic code fence selection prevents fence injection - All user content HTML-escaped to prevent XSS - Graceful handling of malformed user objects - Visual status indicators for resolution and outdated state - Updated heading reflects project rebranding --- `744-1183`: **LGTM!** PRReviewServer class demonstrates excellent design: - Proper MCP handler registration with decorator pattern - Comprehensive tool definitions with detailed schemas - Robust parameter validation rejecting invalid types - Clean error handling with helpful error messages - Auto-resolution workflow for PR URLs - Enterprise support threaded through all operations - Correct server initialization with version info --- `1186-1194`: **LGTM!** Factory function and main block provide clean entry points: - Factory enables testing and flexible instantiation - Main block supports direct module execution </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Update all MCP registration commands to use the proper entry point 'mcp-github-pr-review' instead of running mcp_server.py directly. This fixes ImportError from relative imports. Changes: - Claude CLI registration: use uv --directory with entry point - Claude Desktop config: generate uv command with proper args - Codex CLI config: use uv with --directory flag - Gemini CLI config: use uv command with proper args - Server execution: use entry point in both uv and fallback paths - Display instructions: update all example commands 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fix typos, redundant phrasing, and update stale file paths based on
feedback from gemini-code-assist and coderabbitai reviewers.
Changes:
- Fix redundant phrasing in AGENTS.md ("fetches...and fetches")
- Fix redundant text in SECURITY.md ("MCP Server MCP server")
- Remove duplicate "markdown" and "specification" references
- Fix docs/architecture/overview.md specification → document
- Fix git clone commands: remove 'uvx' prefix, correct directory names
- Fix docs/getting-started/configuration.md: MPC → MCP, heading levels,
add LOG_LEVEL default
- Update docs/guides/remote-uv-endpoint.md: use 'uv tool install'
instead of 'uv add' for standalone service installation
- Fix docs/reference/environment.md: "Currently used by future" →
"Reserved for future"
- Update all file paths: mcp_github_pr_review_spec_maker →
mcp_github_pr_review in prompts/ and specs/
- Consolidate pyproject.toml dev dependencies: remove redundant
[dependency-groups] section, keep [project.optional-dependencies]
with comprehensive dependency list
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
prompts/GITHUB_API_ROBUSTNESS.md (1)
38-38: Update User-Agent to remove legacy "spec-maker" branding.Line 38 still references
User-Agent: mcp-pr-review-spec-maker/1.0, which contradicts the package rename objectives. Update toUser-Agent: mcp-pr-review/1.0for consistency.Apply this diff:
- Maintain: `User-Agent: mcp-pr-review-spec-maker/1.0` + Maintain: `User-Agent: mcp-pr-review/1.0`pyproject.toml (1)
97-97: Updateknown-first-partyto match the new package name.The isort configuration still references the old package name
mcp_server, but the actual package is nowmcp_github_pr_review. This will cause import sorting issues in the new package.[tool.ruff.lint.isort] -known-first-party = ["mcp_server"] +known-first-party = ["mcp_github_pr_review"]
🧹 Nitpick comments (2)
docs/architecture/overview.md (1)
16-16: Capitalize "Markdown" as a proper noun.Line 16 should use the proper noun form for the file format name.
-4. Comments are transformed into a markdown document. +4. Comments are transformed into a Markdown document.docs/guides/remote-uv-endpoint.md (1)
7-9: Add language specifier to fenced code block.The ASCII diagram should specify a language (or use
textfor clarity).-``` +```text client host (Claude, Codex) ──ssh──► uv-managed worker ──► GitHub API -``` +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
AGENTS.md(3 hunks)SECURITY.md(1 hunks)docs/architecture/overview.md(1 hunks)docs/getting-started/configuration.md(1 hunks)docs/getting-started/quickstart.md(1 hunks)docs/guides/remote-uv-endpoint.md(1 hunks)docs/reference/environment.md(1 hunks)prompts/GITHUB_API_ROBUSTNESS.md(7 hunks)pyproject.toml(2 hunks)specs/PHASE2_ENTERPRISE_GITHUB_URLS.md(7 hunks)
✅ Files skipped from review due to trivial changes (2)
- AGENTS.md
- docs/getting-started/quickstart.md
🧰 Additional context used
📓 Path-based instructions (2)
pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
Project must target and enforce Python 3.10+ via pyproject.toml
Files:
pyproject.toml
SECURITY.md
📄 CodeRabbit inference engine (AGENTS.md)
Read and adhere to SECURITY.md warnings before using this MCP server in agent workflows
Files:
SECURITY.md
🧠 Learnings (3)
📚 Learning: 2025-10-17T08:04:11.505Z
Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : All GitHub API interactions should be async/await based
Applied to files:
prompts/GITHUB_API_ROBUSTNESS.mdspecs/PHASE2_ENTERPRISE_GITHUB_URLS.md
📚 Learning: 2025-10-17T08:04:11.505Z
Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : Respect environment-based configuration (GITHUB_TOKEN required; GH_HOST, GITHUB_API_URL, GITHUB_GRAPHQL_URL supported)
Applied to files:
prompts/GITHUB_API_ROBUSTNESS.mddocs/getting-started/configuration.mdspecs/PHASE2_ENTERPRISE_GITHUB_URLS.mddocs/reference/environment.md
📚 Learning: 2025-10-17T08:04:11.505Z
Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : Implement robust error handling for GitHub calls (timeouts, API errors, rate limiting) with retries as appropriate
Applied to files:
prompts/GITHUB_API_ROBUSTNESS.md
🪛 LanguageTool
docs/architecture/overview.md
[uncategorized] ~16-~16: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...ing. 4. Comments are transformed into a markdown document. 5. Response returns to the ho...
(MARKDOWN_NNP)
[uncategorized] ~31-~31: The official name of this software platform is spelled with a capital “H”.
Context: ...CP tools by registering their schema in src/mcp_github_pr_review/server.py and implementing a...
(GITHUB)
prompts/GITHUB_API_ROBUSTNESS.md
[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”.
Context: ...PI Headers** (both files): - Change: Accept: application/vnd.github.v3+json → `Accept: application/vnd.git...
(GITHUB)
[uncategorized] ~28-~28: The official name of this software platform is spelled with a capital “H”.
Context: ...i-Version: 2022-11-28 - Location insrc/mcp_github_pr_review/server.py`: Lines 380-386 ...
(GITHUB)
[uncategorized] ~32-~32: The official name of this software platform is spelled with a capital “H”.
Context: ...L API Headers** (both files): - Add: Accept: application/vnd.github+json (currently missing) - Add: `X-...
(GITHUB)
[uncategorized] ~34-~34: The official name of this software platform is spelled with a capital “H”.
Context: ...i-Version: 2022-11-28 - Location insrc/mcp_github_pr_review/server.py`: Lines 172-176 ...
(GITHUB)
[uncategorized] ~89-~89: The official name of this software platform is spelled with a capital “H”.
Context: ...se GitHub URL construction 2. Update src/mcp_github_pr_review/server.py URL construction...
(GITHUB)
[uncategorized] ~169-~169: The official name of this software platform is spelled with a capital “H”.
Context: ...r GitHub support tickets - Location: src/mcp_github_pr_review/server.py lines 458-481 (exi...
(GITHUB)
[uncategorized] ~177-~177: The official name of this software platform is spelled with a capital “H”.
Context: ...L Rate Limit Handling**: - Location: src/mcp_github_pr_review/server.py lines 260-279 -...
(GITHUB)
[uncategorized] ~235-~235: The official name of this software platform is spelled with a capital “H”.
Context: ...: min(15.0, backoff) - Location in src/mcp_github_pr_review/server.py: Line 488 - Loc...
(GITHUB)
docs/getting-started/configuration.md
[uncategorized] ~8-~8: The official name of this software platform is spelled with a capital “H”.
Context: ...l-request comments. | | GH_HOST | ❌ | github.com | GitHub Enterprise hostname. Auto...
(GITHUB)
docs/reference/environment.md
[uncategorized] ~8-~8: The official name of this software platform is spelled with a capital “H”.
Context: ...t:readscope. | |GH_HOST| string |github.com| Automatically updatesGITHUB_AP...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
docs/guides/remote-uv-endpoint.md
7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (7)
SECURITY.md (2)
3-3: Redundant text from past reviews appears to be resolved.The prior concerns about redundant wording ("MCP Server MCP server" and "markdown markdown files") have been corrected. Line 3 now cleanly reads "GitHub PR Review MCP Server" and line 9 properly states "markdown files."
Also applies to: 9-9
1-154: Security documentation comprehensively covers the package rename refactor.The document appropriately reflects the migration from spec-maker branding to the new pr-review package name, removes legacy tool references (
create_review_spec_file), and provides thorough security guidance. The emphasis on agentic workflow risks, GitHub API hardening, input validation, and recommended safeguards aligns well with the MCP server's role as a trusted data source for AI agents.prompts/GITHUB_API_ROBUSTNESS.md (1)
20-20: Package path references correctly updated.All marked lines containing
src/mcp_github_pr_review/paths reflect the new package name. The prior concerns flagged by reviewers have been addressed—no stale_spec_makerpaths remain.Also applies to: 28-28, 34-34, 62-62, 89-89, 122-122, 169-169, 177-177, 227-227, 235-235
specs/PHASE2_ENTERPRISE_GITHUB_URLS.md (1)
24-24: File paths properly updated to reflect package rename.All references to
src/mcp_github_pr_review/server.pyhave been correctly updated throughout the spec, addressing the previous review comments that flagged stale package name references. The specification document is well-structured, with clear implementation steps, comprehensive testing strategy, and proper documentation of backward compatibility and environment configuration (aligning with learnings aboutGH_HOST,GITHUB_API_URL, andGITHUB_GRAPHQL_URLsupport).Also applies to: 76-76, 78-78, 94-94, 139-139, 175-175, 211-211, 424-424
pyproject.toml (1)
10-10: Python 3.10+ enforcement is correctly configured.The project properly enforces Python 3.10+ across:
requires-python = ">=3.10"(line 10)- Ruff
target-version = "py310"(line 67)- MyPy
python_version = "3.10"(line 139)Also applies to: 67-67, 139-139
docs/reference/environment.md (1)
1-17: Environment variable documentation is complete and accurate.The table clearly documents all required and optional variables, their defaults, and notes. The past review comment about contradictory wording for
GITHUB_GRAPHQL_URLhas been resolved.docs/getting-started/configuration.md (1)
23-27: Configuration documentation is well-structured and complete.Heading levels are consistent, terminology is correct ("MCP" not "MPC"), and the
LOG_LEVELdefault is clearly documented. The table provides comprehensive coverage of configuration options.
Restore [dependency-groups] section alongside [project.optional-dependencies] to maintain compatibility with CI workflow using 'uv sync --dev'. Both sections serve different purposes: - [project.optional-dependencies]: Standard Python packaging (pip, etc.) - [dependency-groups]: uv-specific for efficient dev dependency management This resolves the GitHub Actions test failure: error: Failed to spawn: `pytest` Caused by: No such file or directory (os error 2) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Apply critical fixes from CodeRabbit automated review:
1. Update pyproject.toml isort config
- Change known-first-party from "mcp_server" to "mcp_github_pr_review"
- Ensures proper import sorting with new package name
2. Fix git_pr_resolver.py URL construction
- Ensure get_url() always returns HTML URLs, not API URLs
- Remove fallback to pr_dict.get("url") which returns API endpoint
- Construct HTML URL from PR number if html_url is missing
3. Add deprecation warning to mcp_server.py
- Emit DeprecationWarning when legacy module is imported
- Guides users to migrate to mcp_github_pr_review.server
4. Fix SECURITY.md redundant wording
- Change "This MCP server originally" to "This server originally"
- Removes unnecessary duplication
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pyproject.toml (1)
22-38: Address continued redundancy between[project.optional-dependencies]and[dependency-groups].The dev dependencies are duplicated across two sections: lines 22–38 (
[project.optional-dependencies]) and lines 168–184 ([dependency-groups]). This duplicates the single source of truth and creates maintenance risk if versions diverge. The[dependency-groups]section is Hatch-specific (PEP 735), while[project.optional-dependencies]is the standard understood by all Python tooling (pip, uv, etc.).To resolve, consolidate to the standard
[project.optional-dependencies]section alone and remove[dependency-groups], or configure Hatch to consume[project.optional-dependencies]if Hatch-specific behavior is required. Reference past review comment for context.Also applies to: 168-184
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
SECURITY.md(1 hunks)mcp_server.py(1 hunks)pyproject.toml(4 hunks)src/mcp_github_pr_review/git_pr_resolver.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- SECURITY.md
- src/mcp_github_pr_review/git_pr_resolver.py
🧰 Additional context used
📓 Path-based instructions (3)
pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
Project must target and enforce Python 3.10+ via pyproject.toml
Files:
pyproject.toml
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Ruff and adhere to a maximum line length of 88 characters
Ensure imports are sorted per Ruff’s configuration
Code must be compatible with and target Python 3.10+ features/configuration
Address security findings covered by Ruff’s bandit rules; avoid insecure patterns (e.g., unsafe subprocess, eval)
Pass static type checks with mypy; add/maintain type annotations as needed
Python files must compile without SyntaxError (compile-check must pass)
**/*.py: Code must pass ruff linting with zero violations (ruff check .)
Code must be formatted with ruff format .
Static type checking must pass with mypy
Use type hints for all function parameters and return values
Follow async/await patterns for I/O operations (GitHub API calls, file operations)
Handle errors gracefully with proper exception handling and logging
Log errors to stderr using print(..., file=sys.stderr)
Use traceback.print_exc(file=sys.stderr) for debugging when emitting stack traces
Implement retry logic for transient failures honoring HTTP_MAX_RETRIES
Validate all input parameters (URLs, filenames, numeric limits)
Use safe file operations (e.g., exclusive create, path validation)
Implement rate limiting and pagination safety caps (e.g., PR_FETCH_MAX_PAGES, PR_FETCH_MAX_COMMENTS) for GitHub API usage
Files:
mcp_server.py
mcp_server.py
📄 CodeRabbit inference engine (CLAUDE.md)
mcp_server.py: All GitHub API interactions should be async/await based
Implement robust error handling for GitHub calls (timeouts, API errors, rate limiting) with retries as appropriate
Respect environment-based configuration (GITHUB_TOKEN required; GH_HOST, GITHUB_API_URL, GITHUB_GRAPHQL_URL supported)
Enforce pagination safety limits for GitHub API calls (max pages/comments, per-page size, retry caps)
Generate markdown with dynamic fencing to safely handle nested backticks in output
Files:
mcp_server.py
🧬 Code graph analysis (1)
mcp_server.py (1)
src/mcp_github_pr_review/cli.py (1)
main(59-85)
🪛 GitHub Actions: Lint
mcp_server.py
[error] 11-11: Ruff check failed. E402: Module level import not at top of file. 'from mcp_github_pr_review.server import * # noqa: F401,F403' must be placed at the top of the file. Command: 'uv run ruff check .'
🔇 Additional comments (5)
pyproject.toml (3)
10-10: ✓ Python 3.10+ correctly enforced across build, linting, and type checking.The
requires-python,tool.ruff.target-version, andtool.mypy.python_versionare all consistently set to 3.10+, meeting the coding guideline requirement.Also applies to: 67-67, 139-139
2-2: ✓ Package rename is thorough and consistent.All references from mcp-github-pr-review-spec-maker to mcp-github-pr-review are applied uniformly: project name, description, console script entry point, GitHub URLs, build targets, and tooling configuration.
Also applies to: 4-4, 20-20, 41-43, 50-50, 97-97
49-64: ✓ Build configuration properly updated for new layout and new artifact.The wheel and sdist targets correctly reference the renamed package directory (
src/mcp_github_pr_review), exclude.git*files, and include the newmcp.jsonmanifest. Configuration is consistent across both targets.mcp_server.py (2)
1-9: LGTM! Proper deprecation pattern.The deprecation warning is correctly structured and issued before the import, which is the appropriate pattern for compatibility wrappers. The
stacklevel=2ensures the warning points to the caller's code.
13-16: LGTM! Correct CLI delegation pattern.The main block properly delegates to the new CLI entry point using
raise SystemExit(main()), which is the correct pattern for CLI programs. The pragma no cover is appropriate for the main guard.
Implements 7 nitpick suggestions from CodeRabbit review: **Code Quality:** - Tighten dulwich version constraint to prevent breaking changes (<1.0.0) - Add ssh:// URL format support to REMOTE_REGEXES for broader compatibility - Honor HTTP_PER_PAGE env var when listing PRs for consistency - Make User-Agent dynamic with package version from importlib.metadata - Expose __version__ in __init__.py for better package introspection **Documentation:** - Add GH_HOST notes and security reminders to editor-integrations.md - Add text language specifier to ASCII diagram in remote-uv-endpoint.md **Test Updates:** - Update User-Agent assertion to check dynamic version prefix - Add ssh:// URL test cases to success_cases - Remove ssh:// from unsupported format tests All changes maintain backward compatibility and follow best practices. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
mcp_server.py (1)
5-5: Fix pipeline failure: Add E402 to noqa comment.The noqa comment is missing E402, which will cause the linting pipeline to fail since this import comes after the module docstring and another import statement. As noted in the previous review, this needs to be suppressed.
Apply this diff:
-from mcp_github_pr_review.server import * # noqa: F401,F403 +from mcp_github_pr_review.server import * # noqa: F401,F403,E402
🧹 Nitpick comments (1)
requirements.txt (1)
3-3: Consider upper-bounding httpx for stability.Currently
httpx>=0.27.0has no upper bound, which allows any future version including potential breaking changes in 2.0.0 and beyond. For consistency with the dulwich constraint pattern (<1.0.0), consider adoptinghttpx>=0.27.0,<1.0.0to prevent major version upgrades unless the project intentionally allows unbounded minor/major versions for this dependency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
docs/guides/editor-integrations.md(1 hunks)docs/guides/remote-uv-endpoint.md(1 hunks)mcp_server.py(1 hunks)requirements.txt(1 hunks)src/mcp_github_pr_review/__init__.py(1 hunks)src/mcp_github_pr_review/git_pr_resolver.py(4 hunks)src/mcp_github_pr_review/github_api_constants.py(1 hunks)tests/test_api_headers.py(2 hunks)tests/test_git_pr_resolver.py(28 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/mcp_github_pr_review/init.py
- docs/guides/remote-uv-endpoint.md
- src/mcp_github_pr_review/git_pr_resolver.py
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Ruff and adhere to a maximum line length of 88 characters
Ensure imports are sorted per Ruff’s configuration
Code must be compatible with and target Python 3.10+ features/configuration
Address security findings covered by Ruff’s bandit rules; avoid insecure patterns (e.g., unsafe subprocess, eval)
Pass static type checks with mypy; add/maintain type annotations as needed
Python files must compile without SyntaxError (compile-check must pass)
**/*.py: Code must pass ruff linting with zero violations (ruff check .)
Code must be formatted with ruff format .
Static type checking must pass with mypy
Use type hints for all function parameters and return values
Follow async/await patterns for I/O operations (GitHub API calls, file operations)
Handle errors gracefully with proper exception handling and logging
Log errors to stderr using print(..., file=sys.stderr)
Use traceback.print_exc(file=sys.stderr) for debugging when emitting stack traces
Implement retry logic for transient failures honoring HTTP_MAX_RETRIES
Validate all input parameters (URLs, filenames, numeric limits)
Use safe file operations (e.g., exclusive create, path validation)
Implement rate limiting and pagination safety caps (e.g., PR_FETCH_MAX_PAGES, PR_FETCH_MAX_COMMENTS) for GitHub API usage
Files:
src/mcp_github_pr_review/github_api_constants.pytests/test_git_pr_resolver.pymcp_server.pytests/test_api_headers.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest for all tests and keep tests under the tests/ directory
Avoid live GitHub calls in tests by default; use mocks/fixtures, and keep real-call integration tests token-gated/conditionally skipped
Files:
tests/test_git_pr_resolver.pytests/test_api_headers.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: Place test files under tests/ with filenames matching tests/test_.py
Name test functions with the test_ prefix
Use pytest-asyncio for async test functions
Files:
tests/test_git_pr_resolver.pytests/test_api_headers.py
mcp_server.py
📄 CodeRabbit inference engine (CLAUDE.md)
mcp_server.py: All GitHub API interactions should be async/await based
Implement robust error handling for GitHub calls (timeouts, API errors, rate limiting) with retries as appropriate
Respect environment-based configuration (GITHUB_TOKEN required; GH_HOST, GITHUB_API_URL, GITHUB_GRAPHQL_URL supported)
Enforce pagination safety limits for GitHub API calls (max pages/comments, per-page size, retry caps)
Generate markdown with dynamic fencing to safely handle nested backticks in output
Files:
mcp_server.py
🧠 Learnings (1)
📚 Learning: 2025-10-17T08:04:11.505Z
Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : Respect environment-based configuration (GITHUB_TOKEN required; GH_HOST, GITHUB_API_URL, GITHUB_GRAPHQL_URL supported)
Applied to files:
tests/test_git_pr_resolver.py
🧬 Code graph analysis (3)
tests/test_git_pr_resolver.py (2)
tests/conftest.py (1)
FakeClient(418-450)src/mcp_github_pr_review/git_pr_resolver.py (4)
_get_repo(77-83)git_detect_repo_branch(86-131)graphql_url_for_host(306-352)_html_pr_url(355-356)
mcp_server.py (1)
src/mcp_github_pr_review/cli.py (1)
main(59-85)
tests/test_api_headers.py (2)
src/mcp_github_pr_review/git_pr_resolver.py (1)
resolve_pr_url(165-303)src/mcp_github_pr_review/server.py (2)
fetch_pr_comments(455-658)fetch_pr_comments_graphql(261-452)
🔇 Additional comments (9)
requirements.txt (1)
5-5: Verify dulwich 0.22.0 supports Git repository discovery.The version constraint
>=0.22.0,<1.0.0is reasonable for preventing breaking changes, but confirm that the minimum version (0.22.0) actually provides the repository discovery functionality needed for Git workspace resolution.Have you validated that dulwich 0.22.0 supports the repository location discovery logic used by the workspace resolver? If not, consider testing against the pinned minimum version to ensure compatibility.
docs/guides/editor-integrations.md (1)
1-63: Documentation looks well-structured and aligns with PR naming changes.The guide consistently uses the updated command name (
mcp-github-pr-review, lines 10, 20, 37, 54) and server identifier (pr-review, lines 9, 19, 36, 53) per the PR objectives. Environment variable setup is clearly documented across all platforms. Instructions for Claude Desktop, Codex CLI, and Gemini CLI appear complete and standard for MCP tool integration. The guidance to keep the server running (line 65) and restart tools to apply changes is helpful.tests/test_git_pr_resolver.py (4)
10-15: LGTM! Import path correctly updated to new package structure.The import statement correctly references the new
mcp_github_pr_review.git_pr_reviewmodule path, consistent with the package rename frommcp_github_pr_review_spec_makertomcp_github_pr_review.
56-57: LGTM! All monkeypatch targets correctly updated.All
monkeypatch.setattrcalls consistently reference the new module pathmcp_github_pr_review.git_pr_resolver, ensuring test mocking continues to work correctly with the refactored package structure.Also applies to: 80-82, 100-101, 175-176, 290-291, 322-323, 339-340, 357-358, 380-381, 419-421, 445-447, 476-482, 511-517, 713-714
134-141: LGTM! Good addition of SSH URL test coverage.The new test cases properly cover SSH URL formats with and without the git user prefix and
.gitextension. These are common Git remote URL formats that expand test coverage appropriately.
262-262: LGTM! All inline imports correctly reference the new package structure.All inline imports within test functions consistently use the
mcp_github_pr_review.git_pr_resolvermodule path, maintaining consistency across the test suite.Also applies to: 392-392, 432-432, 457-457, 492-492, 526-526, 548-548, 589-589, 609-609, 629-629, 650-650, 671-671
tests/test_api_headers.py (2)
6-12: LGTM - Import paths updated correctly.The imports have been properly updated to reference the new
mcp_github_pr_reviewpackage structure, aligning with the package rename objective.
149-151: LGTM - Appropriate test update for dynamic versioning.The test correctly validates the User-Agent prefix instead of an exact match, accommodating the dynamic version string from package metadata.
src/mcp_github_pr_review/github_api_constants.py (1)
7-8: LGTM - GitHub API header constants correctly defined.The Accept header and API version constants follow GitHub's modern API recommendations.
The Cursor configuration example was showing a standalone server object with a "name" field instead of the required top-level "mcpServers" mapping. This would cause Cursor to fail loading the MCP server. Fixed by: - Removing the "name" property - Wrapping server configuration inside "mcpServers" object - Using "pr-review" as the server key - Placing "command" and "env" under the server key This now matches the correct MCP configuration format used by Cursor and is consistent with the Gemini CLI example in the same file. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The version() calls were using the underscored module name "mcp_github_pr_review" instead of the hyphenated distribution name "mcp-github-pr-review" from pyproject.toml. This caused PackageNotFoundError and fallback to version "0". Fixed in both locations: - src/mcp_github_pr_review/github_api_constants.py (line 13) - src/mcp_github_pr_review/__init__.py (line 9) Now correctly detects version as "0.1.0" from package metadata. Verified: - __version__ = "0.1.0" ✓ - GITHUB_USER_AGENT = "mcp-github-pr-review/0.1.0" ✓ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Significantly improves test coverage by adding tests for previously untested modules: **New Test Coverage:** - cli.py: 0% → 100% (37 statements) - __main__.py: 0% → 100% (1 statement) - Overall coverage: 92% → 98% **tests/test_cli.py (22 tests):** - TestPositiveIntValidator: Tests for _positive_int validator (5 tests) - TestParseArgs: Tests for argument parsing (10 tests) - TestMain: Tests for main() function (7 tests) - Environment variable handling - CLI flag precedence - KeyboardInterrupt handling (exit 130) - Exception handling **tests/test_package_metadata.py (5 tests):** - TestMainEntry: Tests python -m mcp_github_pr_review execution - TestVersionFallback: Documents version fallback logic - TestVersionDetection: Tests version detection from metadata **Note on Remaining Coverage Gaps:** - Lines 10-11 in __init__.py (PackageNotFoundError fallback) - Lines 14-15 in github_api_constants.py (PackageNotFoundError fallback) These lines are defensive programming that catch the case where the package is not installed. They are difficult to test without breaking the test environment and are simple enough to be considered safe. Test suite: 190 → 217 tests (+27 tests) All tests pass with 0 failures 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_cli.py (1)
180-202: Simplify the assertion for unset environment variables.Lines 199-202 use a complex condition to verify that
PR_FETCH_MAX_COMMENTSwas not set:assert ( "PR_FETCH_MAX_COMMENTS" not in os.environ or os.environ.get("PR_FETCH_MAX_COMMENTS") != "None" )This is unnecessarily convoluted. The CLI only sets env vars to
str(value)whenvalue is not None, so the string"None"will never appear. The intent—verify that the key was not set by this test—is clearer with a direct assertion or by omitting the check entirely.Consider simplifying:
- # Other env vars should not be set - assert ( - "PR_FETCH_MAX_COMMENTS" not in os.environ - or os.environ.get("PR_FETCH_MAX_COMMENTS") != "None" - ) + # Other env vars should not be set by this test + assert "PR_FETCH_MAX_COMMENTS" not in os.environ or os.environ.get( + "PR_FETCH_MAX_COMMENTS" + ) != "25"Or simply remove the assertion if the test's focus is only on
PR_FETCH_MAX_PAGES.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/test_cli.py(1 hunks)tests/test_package_metadata.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Ruff and adhere to a maximum line length of 88 characters
Ensure imports are sorted per Ruff’s configuration
Code must be compatible with and target Python 3.10+ features/configuration
Address security findings covered by Ruff’s bandit rules; avoid insecure patterns (e.g., unsafe subprocess, eval)
Pass static type checks with mypy; add/maintain type annotations as needed
Python files must compile without SyntaxError (compile-check must pass)
**/*.py: Code must pass ruff linting with zero violations (ruff check .)
Code must be formatted with ruff format .
Static type checking must pass with mypy
Use type hints for all function parameters and return values
Follow async/await patterns for I/O operations (GitHub API calls, file operations)
Handle errors gracefully with proper exception handling and logging
Log errors to stderr using print(..., file=sys.stderr)
Use traceback.print_exc(file=sys.stderr) for debugging when emitting stack traces
Implement retry logic for transient failures honoring HTTP_MAX_RETRIES
Validate all input parameters (URLs, filenames, numeric limits)
Use safe file operations (e.g., exclusive create, path validation)
Implement rate limiting and pagination safety caps (e.g., PR_FETCH_MAX_PAGES, PR_FETCH_MAX_COMMENTS) for GitHub API usage
Files:
tests/test_package_metadata.pytests/test_cli.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest for all tests and keep tests under the tests/ directory
Avoid live GitHub calls in tests by default; use mocks/fixtures, and keep real-call integration tests token-gated/conditionally skipped
Files:
tests/test_package_metadata.pytests/test_cli.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: Place test files under tests/ with filenames matching tests/test_.py
Name test functions with the test_ prefix
Use pytest-asyncio for async test functions
Files:
tests/test_package_metadata.pytests/test_cli.py
🧬 Code graph analysis (2)
tests/test_package_metadata.py (1)
src/mcp_github_pr_review/server.py (1)
run(1158-1183)
tests/test_cli.py (1)
src/mcp_github_pr_review/cli.py (2)
_positive_int(16-23)main(59-85)
🔇 Additional comments (6)
tests/test_package_metadata.py (2)
7-21: LGTM!The test correctly verifies the
__main__.pyentry point by running the module with--help. The subprocess security suppression is appropriate for test code, and the 5-second timeout guards against hanging processes.
51-70: LGTM!The tests correctly verify that version metadata is detected from the installed package and properly incorporated into the User-Agent string. The format validation (split on "/" expecting exactly 2 parts) appropriately guards against malformed headers.
tests/test_cli.py (4)
13-36: LGTM!Comprehensive validation testing for the
_positive_inthelper. Good coverage of boundary cases (zero, negative) and invalid types (non-integer strings, floats).
38-101: LGTM!Thorough coverage of argument parsing including default values, individual options, combined options, and error cases. The tests correctly verify that invalid inputs trigger argparse's SystemExit behavior.
109-176: LGTM!The tests correctly verify the main CLI flow including dotenv loading, environment variable overrides, and server initialization. Good use of mocking to isolate the CLI logic from server execution.
207-258: LGTM!Excellent coverage of error handling and environment variable precedence. The tests correctly verify:
- KeyboardInterrupt returns 130 (standard Unix convention)
- Unexpected exceptions are logged to stderr and re-raised
- Existing environment variables are preserved when no CLI override is provided
| class TestVersionFallback: | ||
| """Test version detection fallback paths. | ||
|
|
||
| Note: The fallback logic (lines 10-11 in __init__.py and 14-15 in | ||
| github_api_constants.py) catches PackageNotFoundError when the package | ||
| is not installed and falls back to environment variable or "0". | ||
| This is difficult to test directly in pytest without breaking the test | ||
| environment, so these lines remain untested in coverage reports. | ||
| The fallback logic is simple and defensive programming best practice. | ||
| """ | ||
|
|
||
| def test_version_fallback_logic_exists_in_init(self) -> None: | ||
| """Document that fallback logic exists in __init__.py.""" | ||
| import mcp_github_pr_review | ||
|
|
||
| # Verify the module has version detection with fallback | ||
| assert hasattr(mcp_github_pr_review, "__version__") | ||
|
|
||
| def test_version_fallback_logic_exists_in_constants(self) -> None: | ||
| """Document that fallback logic exists in github_api_constants.py.""" | ||
| from mcp_github_pr_review import github_api_constants | ||
|
|
||
| # Verify the module has User-Agent with version | ||
| assert github_api_constants.GITHUB_USER_AGENT.startswith( | ||
| "mcp-github-pr-review/" | ||
| ) |
There was a problem hiding this comment.
Misleading test class name.
The class is named TestVersionFallback and test methods reference "fallback logic," but the tests only verify normal operation (when the package is installed). The fallback paths—where PackageNotFoundError is caught—are never exercised. While the docstring acknowledges this limitation, the naming creates a false sense of coverage.
Consider renaming to TestVersionAvailability or similar, and renaming the test methods to reflect that they document the existence of fallback logic rather than testing it:
-class TestVersionFallback:
- """Test version detection fallback paths.
+class TestVersionAvailability:
+ """Document version detection and fallback logic.
- def test_version_fallback_logic_exists_in_init(self) -> None:
- """Document that fallback logic exists in __init__.py."""
+ def test_version_attribute_exists(self) -> None:
+ """Verify __version__ is available when package is installed."""
- def test_version_fallback_logic_exists_in_constants(self) -> None:
- """Document that fallback logic exists in github_api_constants.py."""
+ def test_user_agent_includes_package_name(self) -> None:
+ """Verify User-Agent header uses correct package name."""🤖 Prompt for AI Agents
In tests/test_package_metadata.py around lines 23 to 48, the test class and
method names imply they exercise fallback behavior when they only assert that
version information is present; rename the class from TestVersionFallback to
TestVersionAvailability (or similar) and rename the two test methods to clearly
describe what they assert (e.g., test_version_attribute_exists_in_init and
test_user_agent_includes_version) so names reflect that these tests document
presence of version detection rather than exercising PackageNotFoundError
fallback paths.
The test was raising TimeoutException on every retry without mocking asyncio.sleep, causing the retry logic to actually wait through exponential backoff delays: - 1st retry: ~0.5-0.75s - 2nd retry: ~1.0-1.25s - 3rd retry: ~2.0-2.25s - 4th retry: ~4.0-4.25s Total: ~7.5-8.5 seconds, exceeding the 5-second test timeout Fixed by adding asyncio.sleep mock to avoid actual delays during retries. The test now completes in ~0.3 seconds instead of timing out. This was causing CI failures with: "tests/test_graphql_error_handling.py ... Timeout" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Disable coverage instrumentation in subprocess by setting COVERAGE_PROCESS_START="" environment variable when running `python -m mcp_github_pr_review --help` in test. This prevents exit code 139 (segmentation fault) in CI on Python 3.12/Linux caused by pytest-cov instrumenting subprocess execution. Fixes GitHub Actions CI failure where tests would segfault after test_mcp_server_tools.py completes on Python 3.12. Tests pass locally on Python 3.10 and now should pass on all Python versions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…error Add asyncio.sleep mock to prevent actual exponential backoff delays when testing RequestError propagation. Without this mock, the retry logic performs real delays (0.60s, 1.25s, 2.24s, 4.13s) that exceed the 5-second test timeout in CI. This is similar to the fix in test_graphql_timeout_exception - tests that raise exceptions triggering retry logic must mock asyncio.sleep to avoid actual delays during exponential backoff. Fixes CI timeout in test_mcp_server_tools.py that was causing test suite to fail after ~8 seconds of accumulated sleep time. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_mcp_server_tools.py (1)
384-384: Consider patching asyncio.sleep at the module level for better test isolation.While patching
asyncio.sleepglobally works, it's more precise and explicit to patch where it's used:mcp_github_pr_review.server.asyncio.sleep. This avoids potential conflicts with other tests and makes the test's intent clearer.Apply this diff:
- with patch("asyncio.sleep", new_callable=AsyncMock): + with patch("mcp_github_pr_review.server.asyncio.sleep", new_callable=AsyncMock):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_mcp_server_tools.py(30 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Ruff and adhere to a maximum line length of 88 characters
Ensure imports are sorted per Ruff’s configuration
Code must be compatible with and target Python 3.10+ features/configuration
Address security findings covered by Ruff’s bandit rules; avoid insecure patterns (e.g., unsafe subprocess, eval)
Pass static type checks with mypy; add/maintain type annotations as needed
Python files must compile without SyntaxError (compile-check must pass)
**/*.py: Code must pass ruff linting with zero violations (ruff check .)
Code must be formatted with ruff format .
Static type checking must pass with mypy
Use type hints for all function parameters and return values
Follow async/await patterns for I/O operations (GitHub API calls, file operations)
Handle errors gracefully with proper exception handling and logging
Log errors to stderr using print(..., file=sys.stderr)
Use traceback.print_exc(file=sys.stderr) for debugging when emitting stack traces
Implement retry logic for transient failures honoring HTTP_MAX_RETRIES
Validate all input parameters (URLs, filenames, numeric limits)
Use safe file operations (e.g., exclusive create, path validation)
Implement rate limiting and pagination safety caps (e.g., PR_FETCH_MAX_PAGES, PR_FETCH_MAX_COMMENTS) for GitHub API usage
Files:
tests/test_mcp_server_tools.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest for all tests and keep tests under the tests/ directory
Avoid live GitHub calls in tests by default; use mocks/fixtures, and keep real-call integration tests token-gated/conditionally skipped
Files:
tests/test_mcp_server_tools.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: Place test files under tests/ with filenames matching tests/test_.py
Name test functions with the test_ prefix
Use pytest-asyncio for async test functions
Files:
tests/test_mcp_server_tools.py
🧠 Learnings (1)
📚 Learning: 2025-10-17T08:04:11.505Z
Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : All GitHub API interactions should be async/await based
Applied to files:
tests/test_mcp_server_tools.py
🧬 Code graph analysis (1)
tests/test_mcp_server_tools.py (2)
src/mcp_github_pr_review/server.py (3)
PRReviewServer(744-1183)handle_call_tool(891-1064)fetch_pr_comments(455-658)tests/conftest.py (3)
mcp_server(265-269)get(128-136)get(430-438)
🔇 Additional comments (4)
tests/test_mcp_server_tools.py (4)
11-15: LGTM: Import paths correctly updated.The imports now reference the renamed package
mcp_github_pr_review.serverand the renamed classPRReviewServer, consistent with the package-wide refactoring.
21-21: LGTM: Markdown header updated to remove legacy "Spec" terminology.The expected header now reads "Pull Request Review Comments" instead of "Pull Request Review Spec", aligning with the PR's objective to remove deprecated spec-file terminology.
43-220: LGTM: Type hints and monkeypatch targets correctly updated.All test functions now use the
PRReviewServertype hint, and monkeypatch targets correctly referencemcp_github_pr_review.serverfor the renamed module. The tests comprehensively validate tool listing, parameter validation, error handling, and output formats.
222-608: LGTM: Comprehensive test coverage for auto-resolve and git context integration.The tests thoroughly validate:
- Auto-resolution of PR URLs from git context
- Git context detection with
git_detect_repo_branch- Host parameter handling for GitHub Enterprise
- Explicit parameter overrides vs. git context fallback
- Proper argument forwarding to underlying functions
All type hints and monkeypatch targets are correctly updated to use the renamed
PRReviewServerclass andmcp_github_pr_review.servermodule path.
…eption Add asyncio.sleep mock to prevent actual exponential backoff delays when testing TimeoutException handling in REST API error handling tests. Without this mock, the retry logic performs real delays (0.69s, 1.12s, 2.16s, 4.22s) that exceed the 5-second test timeout in CI. This completes the pattern of mocking asyncio.sleep in all tests that raise exceptions triggering retry logic: - test_graphql_timeout_exception ✓ - test_fetch_pr_comments_propagates_request_error ✓ - test_fetch_pr_comments_handles_timeout_exception ✓ (this fix) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_mcp_server_tools.py (1)
372-388: Fix RequestError construction to include a Requesthttpx.RequestError typically requires a Request instance. Construct it with a minimal Request to avoid TypeError and improve forward-compatibility.
- # Create a request error that would occur during network issues - request_error = httpx.RequestError("Network connection failed") + # Create a request error that would occur during network issues + req = httpx.Request("GET", "https://example.invalid/") + request_error = httpx.RequestError("Network connection failed", request=req)
🧹 Nitpick comments (4)
tests/test_mcp_server_tools.py (1)
119-121: Deduplicate repeated GraphQL fetch monkeypatching with a fixtureSame patch target repeated across tests. Extract a small fixture in tests/conftest.py to reduce duplication and improve readability.
Also applies to: 234-236, 266-272
src/mcp_github_pr_review/cli.py (3)
37-45: Allow --max-retries=0 (disables retries) to match server semanticsServer accepts HTTP_MAX_RETRIES >= 0. Loosen CLI validation only for this flag.
def _positive_int(value: str) -> int: @@ return ivalue +def _non_negative_int(value: str) -> int: + try: + ivalue = int(value) + except ValueError as exc: # pragma: no cover - argparse handles messaging + raise argparse.ArgumentTypeError("must be an integer") from exc + if ivalue < 0: + raise argparse.ArgumentTypeError("must be non-negative") + return ivalue @@ parser.add_argument( "--max-retries", - type=_positive_int, - help="Override HTTP_MAX_RETRIES for this process.", + type=_non_negative_int, + help="Override HTTP_MAX_RETRIES for this process (0 disables retries).", )Also applies to: 73-76
95-99: Construct server inside the overrides contextEnsures any env-dependent initialization respects CLI overrides.
- server = PRReviewServer() try: - with _temporary_env_overrides(env_overrides): - asyncio.run(server.run()) + with _temporary_env_overrides(env_overrides): + server = PRReviewServer() + asyncio.run(server.run())
5-12: Emit traceback on unexpected errors (stderr) per guidelinesPrint the stack trace to stderr before re-raising for easier debugging. As per coding guidelines.
import argparse import asyncio import os import sys +import traceback @@ except Exception: # pragma: no cover - surfaces unexpected errors - print("Unexpected server error", file=sys.stderr) + print("Unexpected server error", file=sys.stderr) + traceback.print_exc(file=sys.stderr) raiseAlso applies to: 101-103
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/mcp_github_pr_review/cli.py(1 hunks)tests/test_cli.py(1 hunks)tests/test_mcp_server_tools.py(30 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_cli.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Ruff and adhere to a maximum line length of 88 characters
Ensure imports are sorted per Ruff’s configuration
Code must be compatible with and target Python 3.10+ features/configuration
Address security findings covered by Ruff’s bandit rules; avoid insecure patterns (e.g., unsafe subprocess, eval)
Pass static type checks with mypy; add/maintain type annotations as needed
Python files must compile without SyntaxError (compile-check must pass)
**/*.py: Code must pass ruff linting with zero violations (ruff check .)
Code must be formatted with ruff format .
Static type checking must pass with mypy
Use type hints for all function parameters and return values
Follow async/await patterns for I/O operations (GitHub API calls, file operations)
Handle errors gracefully with proper exception handling and logging
Log errors to stderr using print(..., file=sys.stderr)
Use traceback.print_exc(file=sys.stderr) for debugging when emitting stack traces
Implement retry logic for transient failures honoring HTTP_MAX_RETRIES
Validate all input parameters (URLs, filenames, numeric limits)
Use safe file operations (e.g., exclusive create, path validation)
Implement rate limiting and pagination safety caps (e.g., PR_FETCH_MAX_PAGES, PR_FETCH_MAX_COMMENTS) for GitHub API usage
Files:
tests/test_mcp_server_tools.pysrc/mcp_github_pr_review/cli.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest for all tests and keep tests under the tests/ directory
Avoid live GitHub calls in tests by default; use mocks/fixtures, and keep real-call integration tests token-gated/conditionally skipped
Files:
tests/test_mcp_server_tools.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/test_*.py: Place test files under tests/ with filenames matching tests/test_.py
Name test functions with the test_ prefix
Use pytest-asyncio for async test functions
Files:
tests/test_mcp_server_tools.py
🧠 Learnings (1)
📚 Learning: 2025-10-17T08:04:11.505Z
Learnt from: CR
PR: cool-kids-inc/github-pr-review-mcp-server#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T08:04:11.505Z
Learning: Applies to mcp_server.py : All GitHub API interactions should be async/await based
Applied to files:
tests/test_mcp_server_tools.py
🧬 Code graph analysis (2)
tests/test_mcp_server_tools.py (2)
src/mcp_github_pr_review/server.py (3)
PRReviewServer(744-1183)handle_call_tool(891-1064)fetch_pr_comments(455-658)tests/conftest.py (3)
mcp_server(265-269)get(128-136)get(430-438)
src/mcp_github_pr_review/cli.py (1)
src/mcp_github_pr_review/server.py (2)
PRReviewServer(744-1183)run(1158-1183)
🔇 Additional comments (1)
tests/test_mcp_server_tools.py (1)
11-15: LGTM: Rename wiring and header expectation updates align with new APIImports, tool names, and the “Pull Request Review Comments” header assertions look consistent with PRReviewServer.
Also applies to: 21-21, 43-50
* feat: package server and add mkdocs documentation site * refactor: rename package and remove legacy 'spec-maker' branding Complete package rename from mcp-github-pr-review-spec-maker to mcp-github-pr-review, removing legacy "spec" terminology throughout. ## Package Changes - Rename: mcp_github_pr_review_spec_maker → mcp_github_pr_review - Console script: mcp-github-pr-review-spec-maker → mcp-github-pr-review - Update GitHub org: petersouter → cool-kids-inc - Repository: github-pr-review-mcp-server ## Code Changes - Rename class: ReviewSpecGenerator → PRReviewServer - Update server name: github_review_spec_generator → github_pr_review - Update User-Agent: mcp-pr-review-spec-maker/1.0 → mcp-pr-review/1.0 - Dynamic version from importlib.metadata - Enhanced tool descriptions with key features ## Documentation Updates - Update all module references and import paths - Update mkdocs.yml with new org and package name - Fix run-server.sh with correct paths and server name - Update README, CLAUDE.md, AGENTS.md, SECURITY.md - Polish all docs/ content for consistency - Remove obsolete "spec" terminology ## Testing - Update 190 tests with new imports and assertions - All tests passing (100%) - All quality checks passing (ruff, mypy, compile-check) This refactor provides clearer branding and aligns the package name with its actual functionality: fetching and formatting PR review comments, not creating specification files. 🤖 Generated with Claude Code * fix: use mcp-github-pr-review entry point in run-server.sh Update all MCP registration commands to use the proper entry point 'mcp-github-pr-review' instead of running mcp_server.py directly. This fixes ImportError from relative imports. Changes: - Claude CLI registration: use uv --directory with entry point - Claude Desktop config: generate uv command with proper args - Codex CLI config: use uv with --directory flag - Gemini CLI config: use uv command with proper args - Server execution: use entry point in both uv and fallback paths - Display instructions: update all example commands 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: address PR review comments from AI reviewers Fix typos, redundant phrasing, and update stale file paths based on feedback from gemini-code-assist and coderabbitai reviewers. Changes: - Fix redundant phrasing in AGENTS.md ("fetches...and fetches") - Fix redundant text in SECURITY.md ("MCP Server MCP server") - Remove duplicate "markdown" and "specification" references - Fix docs/architecture/overview.md specification → document - Fix git clone commands: remove 'uvx' prefix, correct directory names - Fix docs/getting-started/configuration.md: MPC → MCP, heading levels, add LOG_LEVEL default - Update docs/guides/remote-uv-endpoint.md: use 'uv tool install' instead of 'uv add' for standalone service installation - Fix docs/reference/environment.md: "Currently used by future" → "Reserved for future" - Update all file paths: mcp_github_pr_review_spec_maker → mcp_github_pr_review in prompts/ and specs/ - Consolidate pyproject.toml dev dependencies: remove redundant [dependency-groups] section, keep [project.optional-dependencies] with comprehensive dependency list 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix(ci): restore dependency-groups for uv sync --dev Restore [dependency-groups] section alongside [project.optional-dependencies] to maintain compatibility with CI workflow using 'uv sync --dev'. Both sections serve different purposes: - [project.optional-dependencies]: Standard Python packaging (pip, etc.) - [dependency-groups]: uv-specific for efficient dev dependency management This resolves the GitHub Actions test failure: error: Failed to spawn: `pytest` Caused by: No such file or directory (os error 2) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: address CodeRabbit review comments Apply critical fixes from CodeRabbit automated review: 1. Update pyproject.toml isort config - Change known-first-party from "mcp_server" to "mcp_github_pr_review" - Ensures proper import sorting with new package name 2. Fix git_pr_resolver.py URL construction - Ensure get_url() always returns HTML URLs, not API URLs - Remove fallback to pr_dict.get("url") which returns API endpoint - Construct HTML URL from PR number if html_url is missing 3. Add deprecation warning to mcp_server.py - Emit DeprecationWarning when legacy module is imported - Guides users to migrate to mcp_github_pr_review.server 4. Fix SECURITY.md redundant wording - Change "This MCP server originally" to "This server originally" - Removes unnecessary duplication 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * refactor: apply nitpick improvements from code review Implements 7 nitpick suggestions from CodeRabbit review: **Code Quality:** - Tighten dulwich version constraint to prevent breaking changes (<1.0.0) - Add ssh:// URL format support to REMOTE_REGEXES for broader compatibility - Honor HTTP_PER_PAGE env var when listing PRs for consistency - Make User-Agent dynamic with package version from importlib.metadata - Expose __version__ in __init__.py for better package introspection **Documentation:** - Add GH_HOST notes and security reminders to editor-integrations.md - Add text language specifier to ASCII diagram in remote-uv-endpoint.md **Test Updates:** - Update User-Agent assertion to check dynamic version prefix - Add ssh:// URL test cases to success_cases - Remove ssh:// from unsupported format tests All changes maintain backward compatibility and follow best practices. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * docs: fix malformed Cursor mcp.json configuration example The Cursor configuration example was showing a standalone server object with a "name" field instead of the required top-level "mcpServers" mapping. This would cause Cursor to fail loading the MCP server. Fixed by: - Removing the "name" property - Wrapping server configuration inside "mcpServers" object - Using "pr-review" as the server key - Placing "command" and "env" under the server key This now matches the correct MCP configuration format used by Cursor and is consistent with the Gemini CLI example in the same file. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: correct distribution name in importlib.metadata.version() calls The version() calls were using the underscored module name "mcp_github_pr_review" instead of the hyphenated distribution name "mcp-github-pr-review" from pyproject.toml. This caused PackageNotFoundError and fallback to version "0". Fixed in both locations: - src/mcp_github_pr_review/github_api_constants.py (line 13) - src/mcp_github_pr_review/__init__.py (line 9) Now correctly detects version as "0.1.0" from package metadata. Verified: - __version__ = "0.1.0" ✓ - GITHUB_USER_AGENT = "mcp-github-pr-review/0.1.0" ✓ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * test: add comprehensive CLI and package metadata tests Significantly improves test coverage by adding tests for previously untested modules: **New Test Coverage:** - cli.py: 0% → 100% (37 statements) - __main__.py: 0% → 100% (1 statement) - Overall coverage: 92% → 98% **tests/test_cli.py (22 tests):** - TestPositiveIntValidator: Tests for _positive_int validator (5 tests) - TestParseArgs: Tests for argument parsing (10 tests) - TestMain: Tests for main() function (7 tests) - Environment variable handling - CLI flag precedence - KeyboardInterrupt handling (exit 130) - Exception handling **tests/test_package_metadata.py (5 tests):** - TestMainEntry: Tests python -m mcp_github_pr_review execution - TestVersionFallback: Documents version fallback logic - TestVersionDetection: Tests version detection from metadata **Note on Remaining Coverage Gaps:** - Lines 10-11 in __init__.py (PackageNotFoundError fallback) - Lines 14-15 in github_api_constants.py (PackageNotFoundError fallback) These lines are defensive programming that catch the case where the package is not installed. They are difficult to test without breaking the test environment and are simple enough to be considered safe. Test suite: 190 → 217 tests (+27 tests) All tests pass with 0 failures 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: prevent test timeout in test_graphql_timeout_exception The test was raising TimeoutException on every retry without mocking asyncio.sleep, causing the retry logic to actually wait through exponential backoff delays: - 1st retry: ~0.5-0.75s - 2nd retry: ~1.0-1.25s - 3rd retry: ~2.0-2.25s - 4th retry: ~4.0-4.25s Total: ~7.5-8.5 seconds, exceeding the 5-second test timeout Fixed by adding asyncio.sleep mock to avoid actual delays during retries. The test now completes in ~0.3 seconds instead of timing out. This was causing CI failures with: "tests/test_graphql_error_handling.py ... Timeout" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: disable coverage in subprocess to prevent Python 3.12 segfault Disable coverage instrumentation in subprocess by setting COVERAGE_PROCESS_START="" environment variable when running `python -m mcp_github_pr_review --help` in test. This prevents exit code 139 (segmentation fault) in CI on Python 3.12/Linux caused by pytest-cov instrumenting subprocess execution. Fixes GitHub Actions CI failure where tests would segfault after test_mcp_server_tools.py completes on Python 3.12. Tests pass locally on Python 3.10 and now should pass on all Python versions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: mock asyncio.sleep in test_fetch_pr_comments_propagates_request_error Add asyncio.sleep mock to prevent actual exponential backoff delays when testing RequestError propagation. Without this mock, the retry logic performs real delays (0.60s, 1.25s, 2.24s, 4.13s) that exceed the 5-second test timeout in CI. This is similar to the fix in test_graphql_timeout_exception - tests that raise exceptions triggering retry logic must mock asyncio.sleep to avoid actual delays during exponential backoff. Fixes CI timeout in test_mcp_server_tools.py that was causing test suite to fail after ~8 seconds of accumulated sleep time. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: mock asyncio.sleep in test_fetch_pr_comments_handles_timeout_exception Add asyncio.sleep mock to prevent actual exponential backoff delays when testing TimeoutException handling in REST API error handling tests. Without this mock, the retry logic performs real delays (0.69s, 1.12s, 2.16s, 4.22s) that exceed the 5-second test timeout in CI. This completes the pattern of mocking asyncio.sleep in all tests that raise exceptions triggering retry logic: - test_graphql_timeout_exception ✓ - test_fetch_pr_comments_propagates_request_error ✓ - test_fetch_pr_comments_handles_timeout_exception ✓ (this fix) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix(cli): scope env overrides to server run * test(server): patch sleep where it is used --------- Co-authored-by: Claude <noreply@anthropic.com>
1 participant
Summary
Complete package rename from
mcp-github-pr-review-spec-makertomcp-github-pr-review, removing legacy "spec" terminology that originated from a deprecated feature (writing spec files to disk, removed for security reasons).This refactor provides clearer branding and aligns the package name with its actual functionality: fetching and formatting PR review comments.
Package Changes
Naming
mcp_github_pr_review_spec_maker→mcp_github_pr_reviewmcp-github-pr-review-spec-maker→mcp-github-pr-reviewmcp-github-pr-review-spec-maker→mcp-github-pr-reviewpr-review-spec→pr-reviewOrganization
petersouter→cool-kids-incgithub-pr-review-mcp-serverCode Changes
Core Refactoring
ReviewSpecGenerator→PRReviewServergithub_review_spec_generator→github_pr_reviewmcp-pr-review-spec-maker/1.0→mcp-pr-review/1.0Improvements
importlib.metadata.version()(single source of truth)File Changes
Source Code
src/mcp_github_pr_review_spec_maker/→src/mcp_github_pr_review/mcp.json)Configuration
pyproject.toml: Package name, URLs, console scriptsrun-server.sh: All paths and server names updatedmkdocs.yml: Site metadata and GitHub URLsmcp.json: Command name and metadataDocumentation
README.md: Installation commands, examples, URLsCLAUDE.md: Development commands, architecture descriptionAGENTS.md: Agent-specific workflowsSECURITY.md: Security considerationsdocs/files: Consistent terminology and pathsTests
Quality Assurance
All Checks Passing ✅
Test Coverage
Breaking Changes⚠️
For users upgrading from the old package:
Installation
Console Script
Python Imports
MCP Configuration
Update your Claude Desktop, Claude CLI, or other MCP client configs to use the new command name:
{ "mcpServers": { "pr-review": { "command": "mcp-github-pr-review", "args": [] } } }Migration Path
pip uninstall mcp-github-pr-review-spec-makerpip install mcp-github-pr-reviewTesting Instructions
Related Issues
Closes #[issue-number-if-any]
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
mcp-github-pr-reviewto fetch and format GitHub PR review comments; tools to fetch reviews and resolve PR URLs.Documentation
Improvements
Tests