feat: Implement Typer-based CLI skeleton with placeholder commands#52
feat: Implement Typer-based CLI skeleton with placeholder commands#52
Conversation
…ty improvements Comprehensive review and improvement of the CLI/UX redesign specification to align with MCP best practices, modern Python architecture, and enhanced user experience. Major improvements: - CRITICAL: Changed agent integration to snippet generation (no auto-writing to agent configs) - CRITICAL: Server execution always via subprocess (never in-process, MCP stdio protocol) - CRITICAL: Clear config precedence (CLI flags > env vars > TOML > .env) - CRITICAL: Secrets only in .env with secure permissions, never in TOML Architecture enhancements: - Separated settings models (GitHubSettings, ServerSettings, LoggingSettings, AppConfig) - Added config/repository.py for atomic config operations with backups - Added config/security.py for secure file permissions (0o600 Unix, ACL Windows) - Added config/github_validator.py for reusable auth validation - Optional [cli] dependencies for server-only users New commands: - quickstart: Interactive first-time setup wizard - config show/migrate/edit/reset: Enhanced config management - config rollback: Restore from automatic backups - agents snippet/verify: Generate snippets + test connectivity (no auto-write) - server debug/logs: Enhanced server management - doctor: Comprehensive health checks with detailed implementation spec - version --check: Version info and update checking Documentation plan: - New: docs/cli.md (complete CLI reference with examples) - New: docs/migration.md (comprehensive migration guide) - New: docs/ci-migration.md (CI/CD pipeline updates) - New: docs/quickstart.md (new user onboarding) - Updates: README.md, AGENTS.md, CLAUDE.md, UV_COMMANDS.md, SECURITY.md Testing strategy: - Comprehensive unit tests for all commands - Integration tests with mocked GitHub API (pytest-httpx) - Cross-platform tests (Windows/macOS/Linux) - Security tests (permissions, token masking, .gitignore) - Migration tests (various legacy scenarios) - Snapshot tests (help text stability) - Documentation tests (validate code examples) Security enhancements: - Token masking in all output (ghp_abc...xyz) - Secure .env file permissions (auto-set) - .gitignore validation for .env files - Optional audit logging for config changes - No secrets in TOML files ever Migration strategy: - Three-phase approach (introduce → promote → remove) - Automatic backups before migrations - Dry-run mode for safe testing - Rollback capability - Mixed state detection in doctor command Doctor command implementation: - 10 comprehensive health checks with detailed specs - Multiple output formats (human-readable, JSON, CI mode) - Clear exit codes and actionable error messages - Fail-fast on critical errors - Rate limit and scope checking Risk mitigation: - Expanded risks & mitigations table with 11 scenarios - Impact/likelihood assessment for each risk - Concrete mitigation strategies - Breaking changes mitigation for existing users Acceptance criteria: - Expanded from 5 to 50+ specific, measurable criteria - Organized by category (config, agents, server, health, security, etc.) - Performance targets included - Test coverage requirements (>=90%) This specification is now aligned with MCP best practices and ready for implementation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add a modern CLI interface using Typer and Rich, maintaining backward compatibility with the existing argparse CLI. ## Changes - **New CLI Package Structure:** - Created `src/mcp_github_pr_review/cli/` package - Renamed `cli.py` → `server_runner.py` (preserves legacy argparse CLI) - Added modular command groups: config, agents, server, doctor, quickstart - **Dependencies:** - Added `[cli]` optional dependency group with typer>=0.12.0 and rich>=13.7.0 - Install with: `uv sync --extra cli` - **Console Script Entrypoints:** - `mcp-github-pr` → New Typer-based CLI (recommended) - `mcp-github-pr-review` → Legacy argparse CLI (backward compatible) - **Implemented Commands (placeholders):** - `config`: init, set, unset, show, validate, migrate, edit, reset - `agents`: list, snippet, verify - `server`: run, debug, logs - `doctor`: comprehensive health checks with --ci and --json flags - `quickstart`: interactive setup wizard - **Features:** - Version command using importlib.metadata - Rich terminal formatting with colors and tables - Graceful error handling for missing CLI dependencies - Comprehensive help text with --help on all commands - Shell completion support via Typer - **Testing:** - Added 51 new tests for Typer CLI in test_cli_new.py - Updated 27 existing tests to reference server_runner module - All 379 tests passing - **Quality:** - Passes ruff format, ruff check, mypy, compile-check - Fixed linting issues (shadowed builtins, unused imports) ## Backward Compatibility The existing `mcp-github-pr-review` command continues to work unchanged, ensuring no breaking changes for current users. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
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. WalkthroughThis PR adds a Typer-based CLI (new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Shell
participant pyproject as pyproject script
participant CLI as mcp-github-pr (Typer app)
participant SR as server_runner
Note over Shell,pyproject: When running installed scripts
User->>Shell: run `mcp-github-pr ...`
Shell->>pyproject: launch entrypoint -> `mcp_github_pr_review.cli:cli_main`
pyproject->>CLI: invoke `cli_main()`
CLI->>CLI: parse args, dispatch subcommand
alt subcommand is server-run
CLI->>SR: call into server_runner (if invoked)
else placeholder command
CLI->>CLI: print "Not yet implemented"
end
sequenceDiagram
autonumber
participant User
participant Shell
participant pyproject as pyproject script
participant MP as mcp-github-pr-review script
participant SR as server_runner
Note over Shell,MP: server runtime entry
User->>Shell: run `mcp-github-pr-review ...`
Shell->>pyproject: launch entrypoint -> `mcp_github_pr_review.server_runner:main`
pyproject->>SR: invoke `main()`
SR->>SR: start server/run logic
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
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 marks the initial phase of a significant CLI/UX redesign, transitioning the project towards a modern, Typer-based command-line interface. The primary goal is to enhance user experience for configuration, validation, and execution tasks, replacing the current bash-script-heavy approach with a more structured and testable Python-native solution. While introducing a new 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
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![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 pull request introduces a well-structured skeleton for a new Typer-based CLI, which is a great step towards improving the user experience as outlined in the new specification document. The separation of the new CLI from the legacy one ensures backward compatibility, and the use of optional dependencies for the CLI is a good practice.
My review includes a few suggestions to improve maintainability by removing duplicated code in the pyproject.toml file and across the new CLI modules. I've also pointed out a minor formatting issue in the new specification document that affects its readability.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/mcp_github_pr_review/cli/server.py (1)
7-44: Resolve mypy type check failures (same pattern as agents.py).This file has the same mypy failures as
agents.py: missing type stubs fortyper(line 8) and untyped decorators on lines 25, 33, and 40. Apply the same fixes described in theagents.pyreview.src/mcp_github_pr_review/cli/doctor.py (1)
7-54: Resolve mypy type check failures (same pattern as other CLI modules).This file has the same mypy failures: missing type stubs for
typer(line 8) and an untyped decorator on line 25. Apply the same fixes described in theagents.pyreview.src/mcp_github_pr_review/cli/__init__.py (1)
8-69: Resolve mypy type check failures (same pattern as other CLI modules).This file has the same mypy failures: missing type stubs for
typer(line 9) and an untyped decorator on line 50. Apply the same fixes described in theagents.pyreview.src/mcp_github_pr_review/cli/quickstart.py (1)
7-40: Resolve mypy type check failures (same pattern as other CLI modules).This file has the same mypy failures: missing type stubs for
typer(line 8) and an untyped decorator on line 25. Apply the same fixes described in theagents.pyreview.src/mcp_github_pr_review/cli/config.py (1)
7-82: Resolve mypy type check failures (same pattern as other CLI modules).This file has the same mypy failures: missing type stubs for
typer(line 8) and untyped decorators on lines 25, 32, 39, 46, 53, 62, 71, and 78. Apply the same fixes described in theagents.pyreview to all eight command decorators.
🧹 Nitpick comments (3)
specs/CLI_CLI_UX_REDESIGN.md (1)
65-65: Polish docs: capitalization, fenced languages, and markdownlint fixes.
- Capitalize “GitHub” consistently (Lines 5, 27, 70, 117, 243).
- Avoid bare URLs (Lines 65, 586, 641). Use link text.
- Add a language to the fenced block (Line 646).
- Ensure tables are surrounded by blank lines (Lines 189–196).
- Use hyphenated “rate-limiting” when adjectival (Line 243).
Examples:
- 1. Generate a new token: https://github.com/settings/tokens/new?scopes=repo + 1. Generate a new token: [Create a new token](https://github.com/settings/tokens/new?scopes=repo)-#### Default (Human-Readable) -``` +#### Default (Human-Readable) +```text Running MCP GitHub PR Review health checks... ...```diff - |------|-----------| + +|------|-----------|Also applies to: 586-586, 641-641, 646-646, 189-196, 5-5, 27-27, 70-70, 117-117, 243-243
pyproject.toml (1)
42-45: Keep “cli” dependencies in sync across extras and dependency-groups.You define “cli” in both
[project.optional-dependencies]and[dependency-groups]. This is fine, but it can drift. Consider adding a small CI check to verify parity, or centralize maintenance policy.Also applies to: 181-201
tests/test_cli_new.py (1)
61-96: Placeholders tightly coupled in tests; plan for future behavior changes.As commands get implemented, assertions like “Not yet implemented” will break. Consider marking these tests with a custom marker (e.g.,
@pytest.mark.placeholder) or matching on broader invariants (exit codes/help sections) to ease the transition.Also applies to: 107-124, 135-152, 163-182, 193-197
📜 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 (11)
pyproject.toml(3 hunks)specs/CLI_CLI_UX_REDESIGN.md(1 hunks)src/mcp_github_pr_review/__main__.py(1 hunks)src/mcp_github_pr_review/cli/__init__.py(1 hunks)src/mcp_github_pr_review/cli/agents.py(1 hunks)src/mcp_github_pr_review/cli/config.py(1 hunks)src/mcp_github_pr_review/cli/doctor.py(1 hunks)src/mcp_github_pr_review/cli/quickstart.py(1 hunks)src/mcp_github_pr_review/cli/server.py(1 hunks)tests/test_cli.py(9 hunks)tests/test_cli_new.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use async/await patterns for all I/O operations (GitHub API calls, file operations)
Use type hints for all function parameters and return values
Handle errors gracefully with proper logging (log to stderr)
Pass ruff linting with zero violations
Format code with ruff format
Code must pass static type checks with mypy
Implement retry logic for transient failures (e.g., GitHub API)
Validate all input parameters (URLs, filenames, numeric limits)
Use safe file operations (exclusive create, path validation)
Enforce pagination safety caps and rate limiting when calling GitHub APIs
Ensure code is compatible with Python 3.10+
Run make compile-check to catch import/syntax errors early; code must be syntactically valid
Return meaningful error messages to MCP clients
Files:
src/mcp_github_pr_review/cli/quickstart.pysrc/mcp_github_pr_review/cli/agents.pysrc/mcp_github_pr_review/cli/doctor.pysrc/mcp_github_pr_review/cli/__init__.pysrc/mcp_github_pr_review/__main__.pytests/test_cli.pysrc/mcp_github_pr_review/cli/server.pytests/test_cli_new.pysrc/mcp_github_pr_review/cli/config.py
tests/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
Place test files under tests/ with filename pattern tests/test_*.py
Files:
tests/test_cli.pytests/test_cli_new.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Name test functions with the test_ prefix
Use pytest-asyncio for async test functions
Mock external dependencies (GitHub API, filesystem) in tests
Files:
tests/test_cli.pytests/test_cli_new.py
🧬 Code graph analysis (3)
src/mcp_github_pr_review/cli/__init__.py (2)
src/mcp_github_pr_review/cli/doctor.py (1)
doctor(26-54)src/mcp_github_pr_review/cli/quickstart.py (1)
quickstart(26-40)
src/mcp_github_pr_review/__main__.py (2)
src/mcp_github_pr_review/cli/__init__.py (1)
main(51-69)src/mcp_github_pr_review/server_runner.py (1)
main(80-104)
tests/test_cli.py (1)
src/mcp_github_pr_review/server_runner.py (3)
_positive_int(37-44)main(80-104)parse_args(47-77)
🪛 GitHub Actions: Lint
src/mcp_github_pr_review/cli/quickstart.py
[error] 8-8: Cannot find implementation or library stub for module named "typer" [import-not-found]
[error] 25-25: Untyped decorator makes function "quickstart" untyped [misc]
src/mcp_github_pr_review/cli/agents.py
[error] 8-8: Cannot find implementation or library stub for module named "typer" [import-not-found]
[error] 25-25: Untyped decorator makes function "list_agents" untyped [misc]
[error] 32-32: Untyped decorator makes function "snippet" untyped [misc]
[error] 40-40: Untyped decorator makes function "verify" untyped [misc]
src/mcp_github_pr_review/cli/doctor.py
[error] 8-8: Cannot find implementation or library stub for module named "typer" [import-not-found]
[error] 25-25: Untyped decorator makes function "doctor" untyped [misc]
src/mcp_github_pr_review/cli/__init__.py
[error] 9-9: Cannot find implementation or library stub for module named "typer" [import-not-found]
[error] 50-50: Untyped decorator makes function "main" untyped [misc]
src/mcp_github_pr_review/cli/server.py
[error] 8-8: Cannot find implementation or library stub for module named "typer" [import-not-found]
[error] 25-25: Untyped decorator makes function "run" untyped [misc]
[error] 33-33: Untyped decorator makes function "debug" untyped [misc]
[error] 40-40: Untyped decorator makes function "logs" untyped [misc]
src/mcp_github_pr_review/cli/config.py
[error] 8-8: Cannot find implementation or library stub for module named "typer" [import-not-found]
[error] 25-25: Untyped decorator makes function "init" untyped [misc]
[error] 32-32: Untyped decorator makes function "set_value" untyped [misc]
[error] 39-39: Untyped decorator makes function "unset" untyped [misc]
[error] 46-46: Untyped decorator makes function "show" untyped [misc]
[error] 53-53: Untyped decorator makes function "validate" untyped [misc]
[error] 62-62: Untyped decorator makes function "migrate" untyped [misc]
[error] 71-71: Untyped decorator makes function "edit" untyped [misc]
[error] 78-78: Untyped decorator makes function "reset" untyped [misc]
🪛 LanguageTool
specs/CLI_CLI_UX_REDESIGN.md
[style] ~4-~4: To elevate your writing, try using a synonym here.
Context: ...ript spans hundreds of lines, making it hard to maintain, test, and extend. - Shell ...
(HARD_TO)
[uncategorized] ~5-~5: The official name of this software platform is spelled with a capital “H”.
Context: ...on (for example, GitHub token checks in src/mcp_github_pr_review/server.py) and forces contri...
(GITHUB)
[uncategorized] ~27-~27: The official name of this software platform is spelled with a capital “H”.
Context: ...ports scripting, uses dotted paths like github.token). - config validate: verify ...
(GITHUB)
[uncategorized] ~65-~65: The official name of this software platform is spelled with a capital “H”.
Context: ...\n 2. Update: mcp-github-pr config set github.token ghp_xxx\n 3. Verify: mcp-github-...
(GITHUB)
[uncategorized] ~70-~70: The official name of this software platform is spelled with a capital “H”.
Context: ...server. - Settings layer: Introduce src/mcp_github_pr_review/config/ package using `pydan...
(GITHUB)
[uncategorized] ~117-~117: The official name of this software platform is spelled with a capital “H”.
Context: ...HTTP client setup from server.py into config/github_validator.py for reuse. Provide async ...
(GITHUB)
[uncategorized] ~243-~243: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ction) - Network timeout handling - Rate limiting responses - Retry logic on transient ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~599-~599: The official name of this software platform is spelled with a capital “H”.
Context: ..._TOKEN or run: mcp-github-pr config set github.token ghp_xxx" 5. **GitHub API Connect...
(GITHUB)
[uncategorized] ~641-~641: The official name of this software platform is spelled with a capital “H”.
Context: ... - Mask secrets - Example: "Config: github.api_url=https://api.github.com (TOML), ...
(GITHUB)
[uncategorized] ~641-~641: The official name of this software platform is spelled with a capital “H”.
Context: ....api_url=https://api.github.com (TOML), github.token=ghp_abc...xyz (ENV)" ### Output ...
(GITHUB)
[uncategorized] ~739-~739: The official name of this software platform is spelled with a capital “H”.
Context: ...onfig setsupports dotted paths (e.g.,github.token) and validates values - ✅ mcp-g...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
specs/CLI_CLI_UX_REDESIGN.md
65-65: Bare URL used
(MD034, no-bare-urls)
189-189: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
586-586: Bare URL used
(MD034, no-bare-urls)
641-641: Bare URL used
(MD034, no-bare-urls)
646-646: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (3)
src/mcp_github_pr_review/__main__.py (1)
1-4: Entrypoint shim looks good.Clean handoff to server_runner.main with proper exit code propagation.
tests/test_cli.py (1)
106-121: Solid coverage of legacy argparse entrypoint.Good assertions on dotenv behavior, env overrides scoping, and exit code 130 on SIGINT.
Also applies to: 232-245, 246-265
pyproject.toml (1)
21-24: ****The function
cli_mainis already defined at line 72 ofsrc/mcp_github_pr_review/cli/__init__.py. The script entry pointmcp-github-pr = "mcp_github_pr_review.cli:cli_main"correctly resolves to this existing callable. There is no packaging or runtime risk.Likely an incorrect or invalid review comment.
Resolve code review comments from gemini-code-assist and coderabbitai:
1. Remove duplicate [dependency-groups] section
- Removed redundant dependency-groups section that duplicated
[project.optional-dependencies]
- Standard pyproject.toml structure now followed
2. Fix markdown rendering issues
- Removed stray escaped backticks in specs/CLI_CLI_UX_REDESIGN.md
- Code blocks now render correctly
3. Remove redundant ImportError handling
- Removed try/except ImportError blocks from CLI submodules
- Main CLI __init__.py already handles missing dependencies
- Cleaner, DRY code
4. Add mypy overrides for optional CLI dependencies
- Added mypy.overrides for typer.* and rich.* modules
- Resolves "cannot find implementation or library stub" errors
- Allows mypy to pass when CLI dependencies are optional
All quality checks passing:
- ruff format ✅
- ruff check ✅
- mypy ✅
- pytest (379 tests) ✅
🤖 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 (6)
specs/CLI_CLI_UX_REDESIGN.md (6)
65-65: Wrap bare URLs in Markdown link syntax.Line 65 contains a bare URL that should be wrapped in
[link text](URL)format for consistency and proper rendering.- - Example: "✗ GitHub token invalid (401 Unauthorized)\n\nYour token at GITHUB_TOKEN may have expired or lack required scopes.\n\nNext steps:\n 1. Generate a new token: https://github.com/settings/tokens/new?scopes=repo\n 2. Update: mcp-github-pr config set github.token ghp_xxx\n 3. Verify: mcp-github-pr config validate" + - Example: "✗ GitHub token invalid (401 Unauthorized)\n\nYour token at GITHUB_TOKEN may have expired or lack required scopes.\n\nNext steps:\n 1. Generate a new token: [create new token](https://github.com/settings/tokens/new?scopes=repo)\n 2. Update: mcp-github-pr config set github.token ghp_xxx\n 3. Verify: mcp-github-pr config validate"
189-189: Add blank lines around table for Markdown compliance.Per
markdownlintrule MD058, tables should be surrounded by blank lines. Add blank lines before line 189 and after the table ends (around line 196).
381-382: Wrap bare URL and specify code fence language.Line 381 contains a bare URL that should be wrapped in Markdown link syntax. Additionally, line 382's code fence is missing a language identifier for proper syntax highlighting.
-**See also:** https://github.com/.../docs/cli.md#validate -``` +**See also:** [CLI Reference](https://github.com/.../docs/cli.md#validate) +```bash
424-440: Add blank line before table and ensure all code fence language specifiers are present.Line 424 (table start) and lines 440+ need formatting adjustments:
- Add blank line before the table at line 424
- Verify lines 440+ have language specifiers on code fences (line 440 appears to be missing
yamlidentifier)
478-486: Add blank lines around table and specify code fence language.Lines 478–486 show a table that is missing blank lines above (before line 478) and after (after line 485). Additionally, line 486's code fence is missing a language identifier.
+ | Task | Command | |------|---------| | Configure server | `mcp-github-pr config init` | ... | Get agent snippets | `mcp-github-pr agents snippet claude-cli` | -``` + +```bash
550-560: Specify language identifiers for all code fences.Multiple code fences are missing language identifiers for syntax highlighting:
- Line 550: Code fence for audit log format (likely
textorlog)- Line 560: Code fence for best practices (likely
pythonortomlcontext)- Line 662: Code fence for default output (likely
textorconsole)Specify appropriate language identifiers (e.g.,
```log,```text) to enable syntax highlighting and comply with Markdown standards.Also applies to: 662-662
📜 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 (7)
pyproject.toml(3 hunks)specs/CLI_CLI_UX_REDESIGN.md(1 hunks)src/mcp_github_pr_review/cli/agents.py(1 hunks)src/mcp_github_pr_review/cli/config.py(1 hunks)src/mcp_github_pr_review/cli/doctor.py(1 hunks)src/mcp_github_pr_review/cli/quickstart.py(1 hunks)src/mcp_github_pr_review/cli/server.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use async/await patterns for all I/O operations (GitHub API calls, file operations)
Use type hints for all function parameters and return values
Handle errors gracefully with proper logging (log to stderr)
Pass ruff linting with zero violations
Format code with ruff format
Code must pass static type checks with mypy
Implement retry logic for transient failures (e.g., GitHub API)
Validate all input parameters (URLs, filenames, numeric limits)
Use safe file operations (exclusive create, path validation)
Enforce pagination safety caps and rate limiting when calling GitHub APIs
Ensure code is compatible with Python 3.10+
Run make compile-check to catch import/syntax errors early; code must be syntactically valid
Return meaningful error messages to MCP clients
Files:
src/mcp_github_pr_review/cli/quickstart.pysrc/mcp_github_pr_review/cli/agents.pysrc/mcp_github_pr_review/cli/config.pysrc/mcp_github_pr_review/cli/doctor.pysrc/mcp_github_pr_review/cli/server.py
🪛 LanguageTool
specs/CLI_CLI_UX_REDESIGN.md
[style] ~4-~4: To elevate your writing, try using a synonym here.
Context: ...ript spans hundreds of lines, making it hard to maintain, test, and extend. - Shell ...
(HARD_TO)
[uncategorized] ~5-~5: The official name of this software platform is spelled with a capital “H”.
Context: ...on (for example, GitHub token checks in src/mcp_github_pr_review/server.py) and forces contri...
(GITHUB)
[uncategorized] ~27-~27: The official name of this software platform is spelled with a capital “H”.
Context: ...ports scripting, uses dotted paths like github.token). - config validate: verify ...
(GITHUB)
[uncategorized] ~65-~65: The official name of this software platform is spelled with a capital “H”.
Context: ...\n 2. Update: mcp-github-pr config set github.token ghp_xxx\n 3. Verify: mcp-github-...
(GITHUB)
[uncategorized] ~70-~70: The official name of this software platform is spelled with a capital “H”.
Context: ...server. - Settings layer: Introduce src/mcp_github_pr_review/config/ package using `pydan...
(GITHUB)
[uncategorized] ~117-~117: The official name of this software platform is spelled with a capital “H”.
Context: ...HTTP client setup from server.py into config/github_validator.py for reuse. Provide async ...
(GITHUB)
[uncategorized] ~243-~243: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ction) - Network timeout handling - Rate limiting responses - Retry logic on transient ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~481-~481: The official name of this software platform is spelled with a capital “H”.
Context: ...-pr config init| | Set GitHub token |mcp-github-pr config set github.token ghp_xxx| | Validate setup |mc...
(GITHUB)
[uncategorized] ~739-~739: The official name of this software platform is spelled with a capital “H”.
Context: ...onfig setsupports dotted paths (e.g.,github.token) and validates values - ✅ mcp-g...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
specs/CLI_CLI_UX_REDESIGN.md
65-65: Bare URL used
(MD034, no-bare-urls)
189-189: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
381-381: Bare URL used
(MD034, no-bare-urls)
382-382: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
440-440: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
478-478: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
485-485: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
486-486: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
550-550: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
560-560: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
662-662: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (8)
specs/CLI_CLI_UX_REDESIGN.md (3)
130-156: Verify Phase 1 scope alignment with implementation.The specification outlines Phase 1 as including three subphases:
- CLI skeleton (Typer app with placeholders and
--version)- Settings layer (Pydantic models +
SettingsRepository)- Core commands implementation
- Testing foundation
- Documentation
However, the PR objectives mention "Phase 1.2 settings layer (Pydantic)" as a next step, suggesting the current PR (Phase 1) may focus only on the skeleton (items 1, 4, 5). Please clarify:
- Is Phase 1 (current PR) the skeleton-only version with placeholder commands and deferred settings layer?
- Or does Phase 1 include settings layer implementation?
This distinction affects test coverage expectations and documentation priorities. If only the skeleton is included, ensure the acceptance criteria (lines 725–837) are adjusted accordingly, as many criteria assume a working settings/config layer.
725-837: Acceptance criteria scope verification needed.The acceptance criteria (lines 725–837) are comprehensive and well-defined, but many assume a fully functional settings/config layer and command implementations. Examples:
- "✅
mcp-github-pr config initcreates bothconfig.tomland.envwith proper structure and permissions"- "✅
mcp-github-pr config setsupports dotted paths (e.g.,github.token)"- "✅
mcp-github-pr agents snippet <name>generates syntactically valid configuration snippets"If the current PR (Phase 1) is skeleton-only with placeholder commands (as suggested in PR objectives: "Phase 1.2 settings layer" and "Phase 1.3 core command implementations"), these criteria should be marked as deferred to Phase 1.2/1.3 rather than current acceptance targets.
Suggested clarification:
Add a subsection distinguishing between:
- Phase 1 acceptance criteria (skeleton: help text, version, placeholder commands, structure)
- Phase 1.2+ acceptance criteria (settings layer, config commands, agent integration)
1-100: Specification is comprehensive and well-aligned with PR objectives.The CLI/UX redesign specification is thorough, covering architecture, migration strategy, testing, documentation, and risk mitigations. The command structure, settings layer design, and phased rollout approach are clearly articulated and appear consistent with the PR's implementation of Phase 1 skeleton components.
Primary action items for maintainers:
- Clarify Phase 1 scope (skeleton-only vs. including settings layer) — affects acceptance criteria interpretation
- Fix markdown formatting — bare URLs, code fence language specifiers, table spacing (flagged in separate comments above)
- Track deferred criteria — Phase 1.2 (settings layer) and 1.3 (core commands) criteria should be documented separately to avoid confusion
src/mcp_github_pr_review/cli/agents.py (1)
1-36: LGTM - Clean placeholder implementation for Phase 1.The placeholder commands follow a consistent pattern with proper type hints. The "Not yet implemented" messages provide clear guidance for future usage. When implementing the actual logic in Phase 1.3, ensure these commands follow the coding guidelines: use async/await for GitHub API calls, implement proper error handling and logging, validate input parameters, and implement retry logic for transient failures.
Note: The past review comments about duplicate try...except blocks and mypy configuration appear to have been addressed or are outside the scope of these module files.
src/mcp_github_pr_review/cli/server.py (1)
1-35: LGTM - Consistent placeholder pattern.The server management commands follow the same clean pattern as other CLI modules. When implementing these commands in later phases, remember that the
logscommand (line 32-35) will need async file I/O operations and safe file path validation per the coding guidelines.src/mcp_github_pr_review/cli/quickstart.py (1)
16-31: LGTM - Proper callback pattern for interactive wizard.The callback decorator with
invoke_without_command=Trueand thectx.invoked_subcommandcheck is the correct pattern for a standalone command. The outlined 5-step wizard flow is clear and aligns with the PR objectives. Future implementation will need async/await for GitHub API connectivity checks (step 3) and file operations (steps 1, 4) per coding guidelines.src/mcp_github_pr_review/cli/config.py (1)
1-73: LGTM - Comprehensive config command set with proper naming.The config module provides a complete set of 8 commands covering initialization, CRUD operations, validation, and migration. Good use of
name="set"for theset_valuefunction (line 23) to avoid conflicts with the Python built-in. When implementing these commands, note that most will require async file I/O operations, input validation, and safe file path handling per the coding guidelines.src/mcp_github_pr_review/cli/doctor.py (1)
16-45: LGTM - Well-designed health check command with output modes.The doctor command properly implements the callback pattern with two useful flags (
--ciand--json). The comprehensive docstring (lines 22-34) clearly documents the 8 planned health checks. When implementing the actual checks, ensure GitHub API calls use async/await with retry logic, subprocess checks handle errors gracefully, and JSON output mode provides structured, machine-readable results per the coding guidelines.
1 participant
Summary
This PR implements Phase 1 of the CLI/UX redesign as specified in
specs/CLI_CLI_UX_REDESIGN.md. It introduces a modern Typer-based CLI interface with Rich terminal formatting while maintaining full backward compatibility with the existing argparse CLI.Changes
New CLI Package Structure
src/mcp_github_pr_review/cli/packagecli.py→server_runner.py(preserves legacy functionality)config,agents,server,doctor,quickstartDependencies
[cli]optional dependency group:typer>=0.12.0- Modern CLI frameworkrich>=13.7.0- Beautiful terminal formattinguv sync --extra cliConsole Script Entrypoints
mcp-github-pr→ New Typer-based CLI (recommended)mcp-github-pr-review→ Legacy argparse CLI (backward compatible)Implemented Commands (Placeholders)
All commands display helpful "Not yet implemented" messages with usage hints:
config- Configuration managementinit- Initialize configuration filesset- Set configuration valueunset- Unset configuration valueshow- Display current configurationvalidate- Validate GitHub tokenmigrate- Migrate from legacy .env setupedit- Open config in $EDITORreset- Reset to defaultsagents- Agent integrationlist- Show supported agentssnippet- Generate agent config snippetverify- Test agent connectivityserver- Server lifecyclerun- Spawn MCP serverdebug- Run with verbose logginglogs- Tail server logsdoctor- Health checks--ciand--jsonflagsquickstart- Interactive setupFeatures
✅ Version command using
importlib.metadata✅ Rich terminal formatting with colors and tables
✅ Graceful error handling for missing CLI dependencies
✅ Comprehensive
--helptext on all commands✅ Shell completion support via Typer
✅ Clean command structure following CLI best practices
Testing
tests/test_cli_new.pyQuality Checks
✅
ruff format- Code formatted✅
ruff check- All linting issues resolved✅
mypy- Type checking passed✅
make compile-check- Syntax validation passed✅
pytest- All tests passingBackward Compatibility
🔒 The existing
mcp-github-pr-reviewcommand continues to work unchanged, ensuring no breaking changes for current users.Try It Out
Next Steps
This PR lays the foundation for implementing the actual functionality in subsequent PRs:
Related
specs/CLI_CLI_UX_REDESIGN.md🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
Documentation
Tests