ADR 0018 — internal/diag is the sole diagnostics emitter

  • Status: Accepted
  • Date: 2026-07-01
  • Tracking: legacy tracker 1528 (diagnostics visibility epic), legacy tracker 1549 (migration), legacy tracker 1550, legacy tracker 1548

Context

madtea ships as both a CLI binary and an MCP server (ADR 0014). The two surfaces have different output channels:

  • CLI: user-facing warnings, notices, and errors go to os.Stderr. ANSI color is appropriate when a TTY is detected.
  • MCP tool call: there is no stderr the agent can see. Diagnostics must be captured and delivered inside the tool result — a severity-prefixed block that the agent reads as part of the response.

Before legacy tracker 1528, every warning or notice in the codebase was written as a raw fmt.Fprintf(os.Stderr, "Warning: ...") call. This worked on the CLI surface but was invisible on the MCP surface: an agent calling madt_issues that triggered a credential-fallback warning or a pagination-truncation notice would get the main response with no indication anything was unusual, because the warning had gone to the server’s stderr log and not into the tool result.

Additionally, raw severity-prefixed prints had inconsistent chrome: some used “Warning:”, some used “WARNING:”, some colored the whole line (not just the prefix), and some wrote to stdout instead of stderr. Untrusted forge content (issue titles, branch names) flowing into these messages was not sanitized, creating a potential escape-sequence injection path.

Decision

internal/diag is the only path for warnings, errors, notices, success messages, and unadorned info lines. No fmt.Fprint*(os.Stderr|os.Stdout, "<severity-prefix>...") call may appear outside internal/diag itself.

Package design

internal/diag (diag.go, sink.go) is a leaf package — it imports only the standard library and internal/util. This lets it be called from every layer (cmd, mcp, service, format, config) with no import cycle risk.

The package exposes two call styles:

  • Context-free (Errorf, Warnf, Noticef, Successf, Infof) — the CLI surface. These call emit(context.Background(), …). Since context.Background() never carries a sink, they always write to os.Stderr. Behavior is identical to a raw fmt.Fprintf(os.Stderr, …) call, but with consistent chrome and sanitized message text.
  • Context-aware (Errorc, Warnc, Noticec, Successc, Infoc) — for service/config code running under an MCP tool call. These call emit(ctx, …). If the context carries a Sink (installed by the MCP middleware before invoking the handler), the diagnostic is routed to the sink — captured into the tool result — and no stderr write occurs. If no sink is present (the same code path invoked from the CLI), the behavior falls back to stderr. The same call site works correctly on both surfaces.

The Sink interface

diag.Sink (sink.go) is a simple interface: Emit(Diagnostic). A Diagnostic carries a Severity and an already-sanitized message. The MCP receiving middleware installs a fresh per-request Sink via diag.WithSink(ctx, s) before invoking a tool handler and drains the collected Diagnostic slice into the tool result after the handler returns. The sink is stored under a private context key (sinkKey{}), so it cannot collide with any other package’s context value.

Security invariant

Every emitter sanitizes the caller-supplied message through util.SanitizeControl before it reaches any output stream. ANSI color is applied only to the fixed, code-controlled severity prefix — never to the message. Untrusted forge text (issue titles, branch names, API error bodies) routinely flows into diag calls; coloring the message would allow a crafted field to smuggle its own ANSI sequences in alongside the diagnostic chrome.

Enforcement

internal/parity/diag_drift.go and diag_drift_test.go (TestDiagDriftGuard and TestDiagDetectorClassification) enforce this as a CI gate. The detector AST-scans every non-test .go file under internal/ and cmd/ (excluding internal/diag itself) for:

fmt.Fprint(os.Stderr|os.Stdout,  "<literal>...")
fmt.Fprintf(os.Stderr|os.Stdout, "<literal>...")
fmt.Fprintln(os.Stderr|os.Stdout, "<literal>...")

where the first literal argument leads with a severity prefix word (ERROR, WARNING, NOTICE, NOTE, SUCCESS). A match is a CI failure. The diagDriftAllowlist is intentionally empty after the legacy tracker 1549 migration: there are no legitimate inline severity-prefixed diagnostics outside internal/diag, so every allowlist entry would represent a real regression, not a documented exception.

The detector is keyed on the exact literal (not file:line) so entries survive line moves, following the same allowlist style as allowlist.go and mutation_drift.go.

Consequences

  • MCP agents receive diagnostics. A credential-fallback warning, a pagination-truncation notice, or a config-resolution error emitted during an MCP tool call appears in the tool result as a severity-labelled block the agent can read and act on. Before legacy tracker 1528 these were silent.
  • CLI behavior is unchanged. The context-free emitters write to os.Stderr with the same timing as the old raw calls. Color gating (SetColorEnabled) is the same point of control.
  • One chrome implementation. Severity prefix wording, color codes, and the sanitize-message / color-chrome split are implemented once in emit. No call site can author its own variant.
  • Import discipline matters. Because internal/diag is a leaf, it can be imported everywhere. If internal/diag ever gains a dependency on a non-leaf package (e.g. service or format), it would introduce an import cycle. The layering guard (layering.go) would catch this, but the intended model is: diag stays a leaf forever and the sink-based routing keeps it independent of surface specifics.
  • This is a consequence of the dual-surface split (ADR 0014). Two output channels (CLI stderr and MCP tool result) require a single routing layer that knows, at call time, which channel is active. internal/diag is that layer. Without it, every call site would need to carry its own channel-selection logic, which would drift.

Alternatives considered

  • Raw stderr prints everywhere (status quo before legacy tracker 1528). Rejected: silent on MCP. An agent calling a madtea tool has no visibility into the server’s stderr. Diagnostics that affect correctness (credential fallback, config resolution) are lost.
  • Per-call ad-hoc handling (each call site decides whether to print or return a structured error). Rejected: inconsistent severity labeling, color handling, and sanitization. Would not solve the MCP invisibility problem without reimplementing the sink routing at every call site.
  • Structured error return for all diagnostics (no stderr writes at all). Rejected: some diagnostics (notices, warnings) are not errors — they do not halt the operation. Returning them as errors changes the call signature of every function that emits them and discards the “operation succeeded with a caveat” semantic that the CLI surface needs.
  • A global mutable sink (not context-carried). Rejected: MCP handles concurrent tool calls; a single global sink would mix diagnostics from parallel calls. The context-carried sink is per-request and concurrency-safe by construction.

Relates to ADR 0014 (layered dual-surface), which established that CLI and MCP share one service layer. This ADR is the corresponding decision for the cross-layer diagnostic path that the dual-surface design makes necessary.

Canonical source: docs/adr/0018-diagnostics-via-internal-diag.md in the madtea repo.