Skip to content

code smell detection: findings for azd-copilot #50

@jongio

Description

@jongio

code smell detection

  • [Change Preventer - HIGH] ConfigureMCPServer() in cli/src/internal/copilot/launcher.go (lines 501-603) duplicates the 6-server MCP configuration in two places: once as individual raw JSON strings in the requiredServers map (for merge path) and again as a monolithic JSON string literal (for fresh-creation path). Adding, removing, or updating any server requires changes in both locations and they can silently diverge. Refactor by defining server configs as typed structs, marshaling to JSON once, and using a single source of truth for both code paths.
  • [Couplers / Change Preventer - HIGH] The list_agents MCP tool in cli/src/cmd/copilot/commands/mcp.go (lines 88-109) lists 7 agents while the azd-copilot://agents resource in the same file (lines 183-195) lists 10 (azure-cost, azure-storage, azure-networking are missing from the tool). Similarly list_skills tool lists 5 skills while the skills resource lists 10. Both tools hardcode data instead of reading from internal/assets, causing the tool output to already diverge from resources and ensuring continued drift as agents/skills are added. Refactor to read agent and skill lists from the embedded assets package.
  • [Bloater - HIGH] cli/src/internal/copilot/launcher.go is 674 lines and contains at least 6 distinct responsibilities: platform-specific Copilot CLI discovery (FindCopilotCLI), process launching (Launch, launchViaConsole), argument construction (buildArgs), environment variable construction (buildEnv), MCP server configuration (ConfigureMCPServer), and extension management (EnsureExtensionsInstalled). This Large Class makes the file hard to navigate, test, and extend. Refactor into focused packages: e.g., copilotfinder, launcher, mcpconfig, extmanager.
  • [Modern / Architectural - HIGH] requiredServers in ConfigureMCPServer() (launcher.go lines 502-539) stores MCP server configurations as raw JSON string literals inside a Go map[string]string. Each entry must be manually json.Unmarshal-ed back into interface{} for merging. This raw-string-JSON-in-Go anti-pattern makes configs hard to read, validate, or refactor. Define a typed mcpServerConfig struct, populate the map with struct values, and marshal to JSON only when writing to disk.

Automated analysis - 4 finding(s)

Metadata

Metadata

Assignees

No one assigned

    Labels

    automatedFiled by automated analysis

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions