Skip to content

Conversation

@peteski22
Copy link
Contributor

@peteski22 peteski22 commented Oct 24, 2025

Summary

Implement mcpd config plugins get command to display top-level plugin config, or config for a specific plugin within a category.

Get top-level plugin config:

mcpd config plugins get  
Plugin Configuration:
  Directory: /Users/pwilson/temp/plugins

Get plugin config for named plugin in category:

mcpd config plugins get --category audit --name tool-audit-plugin
result:
  pluginentry:
    name: tool-audit-plugin
    flows:
      - request
  category: audit

Missing plugin:

mcpd config plugins get --category audit --name foobar
Error: plugin 'foobar' not found in category 'audit'

No plugin config

mcpd config plugins get
Plugin Configuration:
  Directory: (not configured)

Different formats:

JSON

mcpd config plugins get --category audit --name tool-audit-plugin --format json
{
  "result": {
    "name": "tool-audit-plugin",
    "flows": [
      "request"
    ],
    "category": "audit"
  }
}

YAML

mcpd config plugins get --category audit --name tool-audit-plugin --format yaml
result:
  pluginentry:
    name: tool-audit-plugin
    flows:
      - request
  category: audit

Summary by CodeRabbit

  • New Features

    • Added a user-facing get command to show plugin configuration or a specific plugin by category and name.
    • Output available in JSON and YAML; category matching is case-insensitive.
  • Bug Fixes / Validation

    • Improved flag validation to require related flags together and clearer not-found/invalid-category messages.
  • Tests

    • Comprehensive tests covering command behaviour, output formats and printers.

Implement 'mcpd config plugins get' command to display top-level plugin config, or config for a specific plugin within a category.
@coderabbitai
Copy link

coderabbitai bot commented Oct 24, 2025

Walkthrough

Adds a fully implemented plugins get CLI command, new printers for plugin-level and entry outputs, a BaseCmd flag validation helper, shared test utilities, and comprehensive unit tests across the new components.

Changes

Cohort / File(s) Change Summary
Plugin Get Command
cmd/config/plugins/get.go, cmd/config/plugins/get_test.go
Implements GetCmd, command construction and RunE logic with category, name, and format flags; validates flag combinations; loads config and prints either top-level plugin config or a specific plugin entry; adds extensive tests for outputs, formats, validation and error cases.
Plugin Output Printers
internal/printer/plugin_config_printer.go, internal/printer/plugin_config_printer_test.go, internal/printer/plugin_entry_printer.go, internal/printer/plugin_entry_printer_test.go
Adds PluginConfigPrinter and PluginEntryPrinter types with optional header/footer hooks and Item rendering; introduces PluginConfigResult and PluginEntryResult types; tests verify textual formatting, headers/footers and JSON/YAML marshaling behaviour.
Shared Test Utilities
cmd/config/plugins/helpers_test.go, cmd/config/plugins/list_test.go
Introduces mockLoader test helper and updates list_test.go to use it, consolidating loader mocking across plugin tests.
Command Infrastructure
internal/cmd/basecmd.go, internal/cmd/basecmd_test.go
Adds BaseCmd.RequireTogether to validate that specified flags are provided together or not at all; includes unit tests covering various flag combinations and error message formatting.
Configuration
internal/config/config.go
Minor formatting change: inserted blank line after nil-check in Plugin(...); no behavioural change.

Possibly related PRs

  • Add plugins config command #213: Earlier implementation and changes around the cmd/config/plugins/get.go command and related printers/tests, showing a direct code-level connection to the current get command work.
  • Add plugin list command #215: Changes to other plugins CLI commands and printers (e.g., list) that operate on the same plugin config API and share patterns such as internalcmd.BaseCmd usage.

Suggested labels

cli, plugins

Suggested reviewers

  • dpoulopoulos
  • agpituk

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Add plugin get command" directly and accurately describes the main change in this changeset. The PR introduces a fully implemented config plugins get command through the new GetCmd type in cmd/config/plugins/get.go, along with supporting infrastructure (tests, helpers, and printer types). The title is concise, clear, and specific enough that a reviewer scanning the git history would understand the primary addition is a new plugin retrieval command. The title does not attempt to cover every detail, which is appropriate for a PR summary.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch peteski22/cli-plugins-get

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 83179d6 and a401b8c.

📒 Files selected for processing (1)
  • cmd/config/plugins/get.go (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-27T14:06:41.058Z
Learnt from: peteski22
PR: mozilla-ai/mcpd#217
File: cmd/config/plugins/get.go:60-75
Timestamp: 2025-10-27T14:06:41.058Z
Learning: In the mozilla-ai/mcpd repository, `SilenceUsage: true` is set on the RootCmd in cmd/root.go, so child commands automatically inherit this behavior and do not need to set it individually.

Applied to files:

  • cmd/config/plugins/get.go
🧬 Code graph analysis (1)
cmd/config/plugins/get.go (7)
internal/cmd/basecmd.go (2)
  • BaseCmd (45-47)
  • FormatHandler (170-186)
internal/cmd/output/types.go (1)
  • Printer (27-42)
internal/printer/plugin_config_printer.go (2)
  • PluginConfigResult (13-15)
  • PluginConfigPrinter (18-21)
internal/printer/plugin_entry_printer.go (2)
  • PluginEntryResult (14-18)
  • PluginEntryPrinter (21-24)
internal/cmd/format.go (3)
  • OutputFormat (9-9)
  • FormatText (16-16)
  • AllowedOutputFormats (19-29)
internal/config/plugin_config.go (3)
  • Category (76-76)
  • OrderedCategories (488-490)
  • PluginEntry (111-124)
internal/cmd/options/options.go (1)
  • CmdOption (10-10)
🔇 Additional comments (6)
cmd/config/plugins/get.go (6)

15-19: Flag constants are now complete.

All flag names are properly centralized. The addition of flagFormat addresses the previous review comment.


21-43: Well-structured command type.

The GetCmd struct is cleanly designed with clear separation of concerns: configuration loading, output formatting, and flag values. The embedding of BaseCmd provides access to shared validation and loading utilities.


46-58: Excellent initialization pattern.

The constructor properly handles options, sets sensible defaults (text format), and uses dependency injection for the config loader, which facilitates testing.


60-75: Cobra command configuration is correct.

The command definition is comprehensive with clear documentation and examples. Per the retrieved learning, SilenceUsage is correctly inherited from RootCmd, so it does not need to be set here.

Based on learnings


77-96: All flags now use centralized constants.

The flag registration correctly uses the defined constants (flagCategory, flagName, flagFormat) throughout, addressing all previous review comments. The help text appropriately shows allowed values.


101-147: Solid implementation that matches PR objectives.

The run function correctly:

  • Validates that category and name are provided together using RequireTogether
  • Handles the top-level plugin configuration case with a defensive nil check
  • Retrieves and displays specific plugin entries when both category and name are provided
  • Returns a clear error message when a plugin is not found
  • Respects the format flag for output rendering

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 81ef131 and 83179d6.

📒 Files selected for processing (11)
  • cmd/config/plugins/get.go (1 hunks)
  • cmd/config/plugins/get_test.go (1 hunks)
  • cmd/config/plugins/helpers_test.go (1 hunks)
  • cmd/config/plugins/list_test.go (10 hunks)
  • internal/cmd/basecmd.go (2 hunks)
  • internal/cmd/basecmd_test.go (1 hunks)
  • internal/config/config.go (1 hunks)
  • internal/printer/plugin_config_printer.go (1 hunks)
  • internal/printer/plugin_config_printer_test.go (1 hunks)
  • internal/printer/plugin_entry_printer.go (1 hunks)
  • internal/printer/plugin_entry_printer_test.go (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-24T09:54:59.278Z
Learnt from: peteski22
PR: mozilla-ai/mcpd#215
File: internal/plugin/types_test.go:83-90
Timestamp: 2025-10-24T09:54:59.278Z
Learning: In internal/plugin/types_test.go, TestOrderedCategories_ReturnsCorrectOrder uses hard-coded assertions to act as a regression check. If the category order changes in the config package, this test will catch it. This is intentional to ensure order stability.

Applied to files:

  • cmd/config/plugins/get_test.go
🧬 Code graph analysis (9)
internal/cmd/basecmd.go (1)
internal/runtime/runtimes.go (1)
  • Join (40-46)
internal/printer/plugin_config_printer_test.go (1)
internal/printer/plugin_config_printer.go (2)
  • PluginConfigResult (13-15)
  • PluginConfigPrinter (18-21)
cmd/config/plugins/get.go (7)
internal/cmd/basecmd.go (2)
  • BaseCmd (45-47)
  • FormatHandler (170-186)
internal/cmd/output/types.go (1)
  • Printer (27-42)
internal/printer/plugin_config_printer.go (2)
  • PluginConfigResult (13-15)
  • PluginConfigPrinter (18-21)
internal/printer/plugin_entry_printer.go (2)
  • PluginEntryResult (14-18)
  • PluginEntryPrinter (21-24)
internal/cmd/format.go (3)
  • OutputFormat (9-9)
  • FormatText (16-16)
  • AllowedOutputFormats (19-29)
internal/config/plugin_config.go (3)
  • Category (76-76)
  • OrderedCategories (488-490)
  • PluginEntry (111-124)
internal/cmd/options/options.go (1)
  • CmdOption (10-10)
internal/printer/plugin_config_printer.go (1)
internal/cmd/output/types.go (2)
  • Printer (27-42)
  • WriteFunc (25-25)
internal/printer/plugin_entry_printer_test.go (2)
internal/printer/plugin_entry_printer.go (2)
  • PluginEntryResult (14-18)
  • PluginEntryPrinter (21-24)
internal/config/plugin_config.go (5)
  • PluginEntry (111-124)
  • Flow (79-79)
  • FlowRequest (38-38)
  • FlowResponse (41-41)
  • Category (76-76)
cmd/config/plugins/get_test.go (7)
internal/cmd/basecmd.go (1)
  • BaseCmd (45-47)
cmd/config/plugins/get.go (1)
  • NewGetCmd (45-98)
internal/config/types.go (1)
  • Config (83-88)
internal/config/plugin_config.go (8)
  • PluginConfig (84-108)
  • PluginEntry (111-124)
  • Flow (79-79)
  • FlowRequest (38-38)
  • FlowResponse (41-41)
  • CategoryObservability (30-30)
  • Category (76-76)
  • CategoryAudit (33-33)
internal/cmd/options/options.go (1)
  • WithConfigLoader (43-48)
internal/printer/plugin_config_printer.go (1)
  • PluginConfigResult (13-15)
internal/printer/plugin_entry_printer.go (1)
  • PluginEntryResult (14-18)
internal/printer/plugin_entry_printer.go (2)
internal/cmd/output/types.go (2)
  • Printer (27-42)
  • WriteFunc (25-25)
internal/config/plugin_config.go (2)
  • PluginEntry (111-124)
  • Category (76-76)
cmd/config/plugins/helpers_test.go (1)
internal/config/types.go (1)
  • Config (83-88)
internal/cmd/basecmd_test.go (1)
internal/cmd/basecmd.go (1)
  • BaseCmd (45-47)
🔇 Additional comments (21)
internal/config/config.go (1)

137-137: LGTM!

The added blank line improves readability by visually separating the early return from the subsequent logic.

internal/cmd/basecmd_test.go (1)

13-122: LGTM!

Comprehensive test coverage for the RequireTogether validation method. The test cases cover all relevant scenarios including edge cases (single flag, partial provision of multiple flags, and sorting validation). The assertions properly verify both error presence and message content.

internal/cmd/basecmd.go (1)

203-219: LGTM!

The RequireTogether validation method is well-implemented with clear logic and helpful error messaging. Sorting the flag names ensures consistent error output regardless of the order in which flags are specified.

cmd/config/plugins/list_test.go (1)

151-151: LGTM!

The migration from mockLoaderForList to the unified mockLoader test helper is consistent across all test cases and maintains the existing test behaviour.

Also applies to: 252-252, 336-336, 432-432, 491-491, 545-545, 569-569, 608-608, 634-634, 684-684

cmd/config/plugins/helpers_test.go (1)

1-18: LGTM!

The mockLoader test helper provides a clean and simple mock implementation for config.Loader, enabling deterministic testing by injecting either a predefined configuration or an error.

internal/printer/plugin_config_printer_test.go (2)

11-50: LGTM!

The test cases appropriately verify the PluginConfigPrinter.Item behaviour for both configured and unconfigured directory states, ensuring user-friendly output in each scenario.


52-102: LGTM!

The header and footer customisation tests comprehensively verify both the hook functionality and the default behaviour when hooks are not set.

internal/printer/plugin_entry_printer_test.go (2)

13-94: LGTM!

The test cases thoroughly verify the PluginEntryPrinter.Item behaviour across various field combinations, including the correct handling of optional fields and the default Required: false behaviour when nil.


96-146: LGTM!

The header and footer tests follow the same solid testing pattern as the PluginConfigPrinter tests, ensuring consistent behaviour across printer implementations.

cmd/config/plugins/get_test.go (6)

17-35: LGTM!

The flag constant and command initialisation tests provide basic sanity checks ensuring the command is properly configured.


37-174: LGTM!

The plugin-level configuration tests comprehensively verify all output formats (text, JSON, YAML) across different configuration states (configured, nil, empty).


176-323: LGTM!

The specific plugin entry tests thoroughly verify retrieval and rendering of individual plugin entries across all output formats, with good coverage of optional field handling.


325-378: LGTM!

The flag validation tests properly verify that the category and name flags must be provided together, ensuring the RequireTogether validation is correctly applied.


380-454: LGTM!

The error case tests comprehensively verify error handling for invalid categories and missing plugins, including the edge case where plugin configuration is nil.


456-490: LGTM!

The case-insensitive category test verifies user-friendly input handling whilst ensuring the canonical category name appears in the output.

cmd/config/plugins/get.go (3)

100-105: Flag pairing validation looks good.


111-129: Top‑level config branch is null‑safe and printer integration is correct.


130-146: Entry lookup and error path are sound.

Found‑case printing is correctly delegated; not‑found error matches the documented example.

internal/printer/plugin_config_printer.go (1)

35-46: LGTM: clear, user‑friendly text output with sensible empty handling.

The “(not configured)” fallback is helpful; header/footer hooks are properly optional.

internal/printer/plugin_entry_printer.go (2)

38-47: Text rendering for an entry reads well; JSON/YAML marshalling via embedding is appropriate.


38-47: No issues found—helpers exist and nil handling is correct.

Both formatFlows() and formatRequired() are defined in internal/printer/plugin_list_printer.go. The formatRequired() function properly handles nil by returning "false", which is a sensible default. The code usage is correct.

Copy link
Member

@agpituk agpituk left a comment

Choose a reason for hiding this comment

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

amazing!

@peteski22 peteski22 merged commit 723be7c into main Oct 28, 2025
3 checks passed
@peteski22 peteski22 deleted the peteski22/cli-plugins-get branch October 28, 2025 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants