architecture review: findings for azd-copilot

[jongio]

architecture review

• Inverted dependency - internal/skills/skills.go imports cmd/copilot/commands (to read commands.Version), which violates the fundamental layering rule that cmd depends on internal, not the other way around. This creates a cycle risk and breaks module boundaries. Fix: expose Version via a parameter or a shared internal/version package instead.
• MCP tool handlers list_agents and list_skills in cmd/copilot/commands/mcp.go return hardcoded, static agent and skill lists instead of calling assets.ListAgents() / assets.ListSkills() from internal/assets. This means adding or removing an agent/skill from the embedded files will not be reflected in the MCP API, creating a permanent data inconsistency between layers.
• magefile.go lives inside the application's Go module (cli/) rather than in a separate tools module, causing github.com/magefile/mage to appear as a direct (non-indirect) dependency in go.mod. Build tooling should not appear as a first-class application dependency; the idiomatic fix is a separate tools/ module or using go:generate / a tools.go approach.
• Domain/business logic (buildProjectContext) is implemented directly in the cmd entrypoint main.go - including reading and string-parsing azure.yaml - rather than in an internal package. The cmd layer should orchestrate, not implement; this logic belongs in internal/ where it can be tested and reused independently.
• internal/copilot exposes a mutable package-level variable var Version = "0.1.0" that main.go overwrites at startup (copilot.Version = commands.Version). Mutable global state for initialization data bypasses Go's safe initialization patterns; Version should be passed as a parameter to Launch() or injected via a struct/interface.

Automated analysis - 5 finding(s)