madtea.lol

ADR 0014 — CLI and MCP are thin surfaces over one service layer

  • Status: Accepted
  • Date: 2026-07-01
  • Tracking: legacy tracker 1492 (SoC audit), legacy tracker 1493 (client-construction dedup), legacy tracker 1495 (assignee set-merge dedup)

Context

madtea ships two distinct user-facing surfaces: the Cobra CLI (internal/cmd) and the MCP server (internal/mcp). Before the SoC audit (epic legacy tracker 1492), both surfaces contained business logic, API client construction calls, and result-assembly code that was either duplicated or subtly divergent. Concrete examples found in the audit:

  • gitea.NewClientWithConfig was called directly in individual MCP tool files, duplicating the exact same construction logic the CLI used.
  • Assignee set-merge (build a map[string]struct{}, loop-add, loop-delete, reconcile with an assignee identifier) was copy-pasted into multiple locations across both internal/mcp and internal/cmd.

Neither of these duplications was caught by any test; behavior could drift between surfaces silently. The immediate risk is that a change to business logic is applied to one surface and not the other, making the CLI and MCP behave differently for the same operation.

Decision

The CLI (internal/cmd) and the MCP server (internal/mcp) are two co-equal, thin surfaces over a single shared business-logic layer (internal/service, supported by internal/cmdutil). No business logic lives in cmd/ or mcp/.

Layer model

Dependencies point strictly downward across five tiers:

graph TB
    L5["Entry / cmd/madtea/main.go · internal/cmd/root.go"]
    L4["Surfaces / internal/cmd (CLI) · internal/mcp (MCP)"]
    L3["Business logic / internal/service · internal/cmdutil"]
    L2["Infrastructure / gitea · git · config · format · util · cmdresult"]
    L1["External / Gitea or Forgejo API · git binary · OS keychain"]
    L5 --> L4 --> L3 --> L2 --> L1
    L2 -. types-only .-> L3

There is exactly one deliberate upward edge: internal/format imports internal/service types (service.User, service.RankedIssue, service.Issue, etc. in internal/service/types.go) to render them. This edge is types-only — formatters reference structs but never call service.New* or any service function, so no behaviour flows upward and the graph stays acyclic. The same shape holds for sub-package edges (format/git → service/git, format/orchestrate → service/orchestrate, format/prune → service/prune). service/workflow imports format in the normal downward direction.

Deduplication locked in by the anti-duplication guard

Two specific de-duplications (identified in legacy tracker 1493 and legacy tracker 1495) are enforced by internal/parity/duplication.go — TestMCPToolsDoNotConstructClient and TestNoInlineAssigneeMerge:

  • Client construction — the only MCP tool file allowed to call gitea.NewClientWithConfig is internal/mcp/tools_api.go (the raw API passthrough). Every other tools_*.go routes construction through a shared helper.
  • Assignee set-merge — the add/remove set-merge lives once as service.MergeAssignees in internal/service/assignees.go. The inline set-merge pattern must not reappear in internal/mcp or internal/cmd.

Enforcement

The layering is guarded by two CI tests in internal/parity/:

  • TestInternalNoImportCycles and TestInternalLayeringBoundaries (layering.go) — derives the live internal import graph via go list -json ./internal/... (no AST walk, so it cannot drift from the compiler’s view) and asserts no import cycles and no forbidden upward/sideways edges. On failure it names the exact offending from → to edge. The rules are encoded in the layerRules slice: gitea may not import service, cmd, mcp, or format; format may not import cmd, mcp, gitea, or config (the format → service upward edge is explicitly allowed); service may not import cmd or mcp.
  • TestMCPToolsDoNotConstructClient and TestNoInlineAssigneeMerge (duplication.go) — AST-scans the MCP and cmd sources for the forbidden duplicated patterns.

Consequences

  • Behavior cannot drift between surfaces. The service layer is the single implementation of every forge operation. A bug fixed in service.MergeIssue is fixed on both the CLI and MCP with one change; no mirrored fix is needed.
  • Both surfaces are independently testable. Neither internal/cmd nor internal/mcp carries logic that requires mocking the other; both depend on internal/service which can be tested against a fake gitea.Client implementation.
  • The one upward edge is load-bearing. The decoded response types live in service (not in a separate shared-types package) because service is the logical owner of what an operation returns. This is the trade-off: format depends upward on service for types only, keeping the struct definitions consolidated. Any attempt to move structs to a third package to “fix” this edge should be weighed against the coherence cost.
  • New tool files must route through the shared client helper. The CI guard will fail any MCP tool that calls gitea.NewClientWithConfig directly; follow the existing pattern in internal/mcp/.

Alternatives considered

  • Duplicate business logic in each surface. Rejected: this was the status quo before legacy tracker 1492. Behavior drifted silently between CLI and MCP; there was no test to catch it, and the delta grew with every new action.
  • No explicit layering; allow any intra-internal import. Rejected: without the acyclic constraint, gitea can import service can import gitea — a cycle that makes the package untestable in isolation. The go list-based guard rules this out structurally.
  • Shared types in a separate package below both surfaces. Considered during the audit. Rejected: the types belong conceptually to the service layer (they are the decoded results of service operations), and moving them introduces a third import chain without eliminating the upward reference — formatters still need them, wherever they live. The types-only upward edge is the documented trade-off.

Relates to ADR 0001 (the MCP layer is the Tier-1 source of truth for agent conventions; the service layer is what both surfaces call) and ADR 0008 (the OperationSpec registry, which the neutral opspec package imports without violating this layering).

Canonical source: docs/adr/0014-layered-dual-surface.md in the madtea repo.