Conversation
- Modified the description for the actual intention of the project otherwise I will forget - Changed executable to be static
- extracted and enhanced the `find_rails_root` logic - Added a `determine_rails_path` cli option - Improved a check to see if we're currently in a rails project: `rails_project?` - Tried to implement rails detection both automatically and with `auto-detection` - More error handling!
- Removed default config options - Added more output for configs - Added `to_h` and `merge!` - Search for env variables to be used in the MCP configs - Allow json config
- Used AI to generate this validator but will have to look at tidying it up
- Added some integration tests
GoodPie
added
documentation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @GoodPie, 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 significantly enhances the Rails Active MCP gem by making it globally installable, providing a comprehensive CLI, improving configuration management, and adding project validation capabilities. These changes simplify the setup process, improve usability, and ensure compatibility with a wider range of Rails projects.
Highlights
- Global Installation: The gem can now be installed globally and used with any Rails project, simplifying setup and usage.
- CLI Interface: Introduced a Thor-based CLI for managing the Rails Active MCP server, including commands for starting the server, generating configuration files, and validating projects.
- Configuration Management: Implemented a hierarchical configuration system that supports defaults, global configuration files, project-specific configuration files, environment variables, and CLI arguments.
- Project Validation: Added a project validation feature to ensure compatibility with Rails Active MCP, including checks for Rails structure, Gemfile, and database configuration.
- Dependency Updates: Updated dependencies to ensure compatibility with Rails 7 and modern versions of related gems.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Pull Request Overview
This PR converts Rails Active MCP into a globally installable tool with a streamlined Thor-based CLI, hierarchical configuration loading, and Rails 7 compatibility.
- Introduces a Thor CLI (
exe/rails-active-mcp-server→lib/rails_active_mcp/cli.rb) and removes the old startup script - Refactors
Configurationfor defaults, global/project JSON, and env var hierarchy; addsto_handmerge! - Updates gemspec, Gemfile, and README for global installation, dependency bumps, and version 3.0.0
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spec/integration/global_installation_spec.rb | New integration tests covering global install, auto-detect, config precedence |
| rails_active_mcp.gemspec | Bumped version, updated summary, executables, dependencies |
| lib/rails_active_mcp/configuration.rb | Refactored for hierarchical config, added to_h and merge! |
| lib/rails_active_mcp/cli.rb | Added Thor CLI commands (start, generate_config, validate_project) |
| exe/rails-active-mcp-server | Simplified executable to delegate to Thor CLI |
| README.md | Updated documentation and Quick Start for global CLI usage |
| Gemfile | Updated development/test deps, added MCP SDK, Rails 7 support |
| lib/rails_active_mcp/version.rb | Bumped gem version to 3.0.0 |
| lib/rails_active_mcp/project_validator.rb | Added comprehensive Rails project validation logic |
Comments suppressed due to low confidence (4)
lib/rails_active_mcp/configuration.rb:66
- Add tests for the
#to_hmethod to verify it serializes all expected configuration attributes correctly.
def to_h
lib/rails_active_mcp/configuration.rb:83
- Add tests for the
#merge!method to ensure merging from Hash and Configuration objects correctly updates settings.
def merge!(other_config)
README.md:173
- Update the sample configuration in the README to use
custom_safety_patternsinstead ofblocked_patternsto match the code.
{
lib/rails_active_mcp/configuration.rb:12
- The
command_timeout,log_level,enable_logging, andallowed_commandsattributes lack accessors, so calls likeconfig.command_timeoutwill fail. Addattr_accessorfor these attributes.
attr_accessor :safe_mode, :max_results, :log_executions, :audit_file, :enabled
| bundle exec rspec bundle exec test | ||
| git status git log git diff | ||
| ], | ||
| blocked_patterns: %w[*.delete_all *.destroy_all system exec `], |
There was a problem hiding this comment.
The generate_config default uses blocked_patterns, but the Configuration class expects custom_safety_patterns. Rename this key to custom_safety_patterns to ensure the generated config is loaded properly.
| blocked_patterns: %w[*.delete_all *.destroy_all system exec `], | |
| custom_safety_patterns: %w[*.delete_all *.destroy_all system exec `], |
There was a problem hiding this comment.
Code Review
This pull request introduces significant updates to convert the Rails Active MCP gem into a global tool. The changes include replacing the exe/rails-active-mcp-server script with a streamlined Thor CLI implementation, introducing hierarchical configuration loading, and updating the Gemfile dependencies for Rails 7 compatibility. There are a few high and medium severity issues that should be addressed.
| gem 'rspec', '~> 3.1' | ||
| gem 'rspec-rails' | ||
| gem 'sqlite3', '~> 2.7' | ||
| gem 'mcp', '~> 0.1.0' # Official MCP Ruby SDK for migration |
There was a problem hiding this comment.
Consider adding a comment explaining why mcp gem is included in the development, test group. This clarifies its purpose for developers. Also, it is unusual to have a gem that is required for production to be included in the development group. This is a high severity issue because it will likely break the gem in production.
| rails-active-mcp-server start --auto-detect | ||
| ``` |
There was a problem hiding this comment.
Consider adding a newline character after the command rails-active-mcp-server start --auto-detect to improve readability.
| ```bash | ||
| rails-active-mcp-server start --project /path/to/rails/app |
There was a problem hiding this comment.
Consider adding a newline character after the command rails-active-mcp-server start --project /path/to/rails/app to improve readability.
| puts "FATAL: Failed to load rails_active_mcp gem: #{e.message}" | ||
| exit(1) |
There was a problem hiding this comment.
If the gem fails to load, the error message is printed to $stdout instead of $stderr. This could cause issues with error reporting, as standard error streams are typically used for conveying error messages. Consider printing to $stderr instead.
|
|
||
| # Validate and expand project path with security checks | ||
| def self.validate_project_path(path) | ||
| return nil if path.blank? |
There was a problem hiding this comment.
The blank? method is an Active Support extension. While Rails is a dependency, this CLI is designed to be a global tool and might be run outside of a project's bundle exec context where Active Support may not be loaded. This creates a brittle dependency.
To make the tool more robust and self-contained, it's best to use standard Ruby methods instead.
return nil if path.nil? || path.to_s.strip.empty?| \x5 rails-active-mcp-server start --auto-detect | ||
| \x5 rails-active-mcp-server start --project /path/to/rails/app | ||
| \x5 rails-active-mcp-server start --dry-run # Show config without starting | ||
| \x5 rails-active-mcp-server start --safe-mode --timeout 60 |
There was a problem hiding this comment.
The command examples in the long_desc for the start command are prefixed with \x5. This will appear in the CLI help output and makes the example commands non-copy-pasteable for users. This prefix should be removed. This issue is also present in other long_desc blocks in this file.
rails-active-mcp-server start --auto-detect
rails-active-mcp-server start --project /path/to/rails/app
rails-active-mcp-server start --dry-run # Show config without starting
rails-active-mcp-server start --safe-mode --timeout 60| @safe_mode = other_config.safe_mode unless other_config.safe_mode.nil? | ||
| @command_timeout = other_config.command_timeout unless other_config.command_timeout.nil? | ||
| @log_level = other_config.log_level unless other_config.log_level.nil? | ||
| @enable_logging = other_config.enable_logging unless other_config.enable_logging.nil? | ||
| @max_results = other_config.max_results unless other_config.max_results.nil? | ||
| @log_executions = other_config.log_executions unless other_config.log_executions.nil? | ||
| @audit_file = other_config.audit_file unless other_config.audit_file.nil? | ||
| @enabled = other_config.enabled unless other_config.enabled.nil? | ||
| @allowed_commands = other_config.allowed_commands unless other_config.allowed_commands.nil? | ||
| @custom_safety_patterns = other_config.custom_safety_patterns unless other_config.custom_safety_patterns.nil? | ||
| @allowed_models = other_config.allowed_models unless other_config.allowed_models.nil? | ||
| end |
There was a problem hiding this comment.
The merge_configuration! method is quite verbose and requires manually listing every configuration attribute. This is prone to errors when adding new configuration options in the future.
You can make this more maintainable and less redundant by leveraging the to_h and merge_hash! methods that are already defined. This would automatically handle any new configuration options added to to_h.
def merge_configuration!(other_config)
# Use to_h to get all config values and compact to remove nils,
# then merge using the existing logic that includes validation.
merge_hash!(other_config.to_h.compact)
end| source 'https://rubygems.org' | ||
| git_source(:github) { |repo| "https://github.com/\#{repo}.git" } | ||
|
|
||
| ruby '3.0.0' |
There was a problem hiding this comment.
The mock Gemfile created for integration tests specifies ruby '3.0.0', but the rails_active_mcp.gemspec requires required_ruby_version = '>= 3.3.0'. This inconsistency means the tests are running against a Ruby version that is not officially supported by the gem.
To ensure the tests are valid and relevant, the Ruby version in the test setup should be aligned with the gem's requirements.
ruby '3.3.0'1 participant
Issue
Previously, the package had to be added to a project and it got clunky running on multiple. This is an effort to have it globally installed where it can access many projects.
Generated Summary
This pull request introduces significant updates to the Rails Active MCP gem, focusing on simplifying the codebase, enhancing configuration management, and improving compatibility with Rails 7. The most notable changes include replacing the
exe/rails-active-mcp-serverscript with a streamlined Thor CLI implementation, introducing hierarchical configuration loading, and updating theGemfiledependencies for Rails 7 compatibility.Configuration Enhancements:
lib/rails_active_mcp/configuration.rb: Refactored theConfigurationclass to support hierarchical loading of configuration from defaults, global settings, project-specific files, and environment variables. Added methods for merging configurations, serializing to hash, and applying environment variables. [1] [2]lib/rails_active_mcp.rb: Addedproject_validatorto enhance project validation functionality.CLI Simplification:
exe/rails-active-mcp-server: Replaced the complex server initialization script with a streamlined Thor CLI implementation, removing redundant code for Rails environment detection and stdio transport handling.Dependency Updates:
Gemfile: Updated dependencies to ensure compatibility with Rails 7, including addingsprockets-railsand replacing older gems likefactory_bot_railsandrspecwithsimplecovandmcp.Minor Improvements:
lib/rails_active_mcp/configuration.rb: Addedrequire 'json'to support JSON-based configuration parsing.