Skip to content

Refactor duplicated code in tool name constants and permission handlers#418

Merged
Mossaka merged 2 commits into
mainfrom
copilot/refactor-duplicate-code
Oct 22, 2025
Merged

Refactor duplicated code in tool name constants and permission handlers#418
Mossaka merged 2 commits into
mainfrom
copilot/refactor-duplicate-code

Conversation

Copilot AI commented Oct 17, 2025

Copy link
Copy Markdown
Contributor

This PR refactors duplicated code across the codebase to improve maintainability and reduce technical debt by consolidating similar logic into reusable helper functions and centralized constants.

Changes Made

1. Tool Name Constants (src/main.rs)

Before: Tool name strings were duplicated across both TryFrom and AsRef trait implementations, requiring changes in two places whenever a tool name needed updating.

impl TryFrom<&str> for ToolName {
    fn try_from(value: &str) -> Result<Self, Self::Error> {
        match value {
            "load-component" => Ok(Self::LoadComponent),
            "unload-component" => Ok(Self::UnloadComponent),
            // ... more duplicated strings

After: Centralized const definitions provide a single source of truth:

impl ToolName {
    const LOAD_COMPONENT: &'static str = "load-component";
    const UNLOAD_COMPONENT: &'static str = "unload-component";
    // ... all strings defined once
    
    const fn as_str(&self) -> &'static str {
        match self {
            Self::LoadComponent => Self::LOAD_COMPONENT,
            // ... references to constants

2. Grant Permission Handlers (crates/mcp-server/src/tools.rs)

Before: Four nearly identical functions (~50 lines each) for granting different permission types:

  • handle_grant_storage_permission
  • handle_grant_network_permission
  • handle_grant_environment_variable_permission
  • handle_grant_memory_permission

Each function duplicated the same logic: extract arguments, validate component_id, ensure component loaded, call lifecycle_manager, format response.

After: Generic helper function with thin wrappers:

async fn handle_grant_permission_generic(
    req: &CallToolRequestParam,
    lifecycle_manager: &LifecycleManager,
    permission_type: &str,
    permission_display_name: &str,
) -> Result<CallToolResult> {
    // Consolidated logic here
}

pub async fn handle_grant_storage_permission(...) -> Result<CallToolResult> {
    handle_grant_permission_generic(req, lifecycle_manager, "storage", "storage").await
}

Impact: Reduced ~150 lines of duplicated code while maintaining identical functionality and error handling.

3. Revoke Permission Handlers (crates/mcp-server/src/tools.rs)

Before: Two nearly identical functions for revoking permissions:

  • handle_revoke_network_permission
  • handle_revoke_environment_variable_permission

After: Generic helper function following the same pattern as grant handlers.

Note: handle_revoke_storage_permission intentionally kept separate because it uses a different internal API (revoke_storage_permission_by_uri vs revoke_permission).

Results

  • Net reduction: 128 lines of code removed (247 deletions, 119 insertions)
  • Files changed: 3 (src/main.rs, crates/mcp-server/src/tools.rs, CHANGELOG.md)
  • ✅ All relevant tests pass (24 mcp-server tests, 16 main binary tests)
  • ✅ Code formatted with cargo +nightly fmt
  • ✅ No clippy warnings
  • ✅ 100% backward compatible

Benefits

  1. Single Source of Truth: Tool names and permission handling logic now defined in one place
  2. Easier Maintenance: Future changes to permission handling require updates in fewer locations
  3. Improved Consistency: All handlers follow the same pattern for error handling and logging
  4. Better Readability: Less code duplication makes the codebase easier to understand
  5. Type Safety: Centralized constants prevent typos and inconsistencies

This refactoring addresses technical debt without introducing new features or breaking changes, making the codebase more maintainable for future development.

Original prompt

Find and refactor duplicated code


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Refactor duplicated code to improve maintainability Refactor duplicated code in tool name constants and permission handlers Oct 17, 2025
Copilot AI requested a review from Mossaka October 17, 2025 23:22
@Mossaka Mossaka marked this pull request as ready for review October 22, 2025 03:19
Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
@Mossaka Mossaka force-pushed the copilot/refactor-duplicate-code branch from 93ac4a7 to 45714ef Compare October 22, 2025 03:19
@Mossaka Mossaka requested a review from Copilot October 22, 2025 03:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors duplicated code to improve maintainability by consolidating tool name strings and permission handler logic.

  • Introduces centralized const definitions for tool names in src/main.rs, creating a single source of truth
  • Consolidates grant permission handlers into a generic helper function, reducing duplication across four permission types
  • Consolidates revoke permission handlers into a generic helper function, reducing duplication across two permission types

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/main.rs Refactored ToolName enum to use centralized const definitions, eliminating string duplication between TryFrom and AsRef implementations
crates/mcp-server/src/tools.rs Introduced generic helper functions for grant and revoke permission handlers, consolidating previously duplicated logic
CHANGELOG.md Documented the refactoring changes in the Changed section

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/main.rs
Comment thread crates/mcp-server/src/tools.rs
Comment thread crates/mcp-server/src/tools.rs
Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>
@Mossaka Mossaka merged commit f2f4244 into main Oct 22, 2025
16 checks passed
@Mossaka Mossaka deleted the copilot/refactor-duplicate-code branch October 22, 2025 04:37
Mossaka added a commit to Mossaka/wassette that referenced this pull request Feb 2, 2026
…rs (microsoft#418)

Signed-off-by: Jiaxiao Zhou <duibao55328@gmail.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com>
Co-authored-by: Jiaxiao Zhou <duibao55328@gmail.com>
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