madtea.lol

ADR 0003 — Side-effect confirmation is a structural, enforced convention

  • Status: Accepted
  • Date: 2026-06-03
  • Tracking: legacy tracker 1370 (epic). This ADR is sub-issue legacy tracker 1371; the convention is implemented across legacy tracker 1372–legacy tracker 1381.

Context

A command that performs a side-effect must tell the caller what it did — otherwise the caller re-runs work that already happened. The concrete failure that prompted this: madtea finish ends by running git checkout <default> && git pull (returnToDefaultBranch), but its output says only “Returned to <default>” — never that it pulled, or to which commit. A caller can’t tell the pull happened and re-runs madtea pull (an observed redundancy).

Two facts shape the design:

The two consumers have opposite cost models, on already-separate streams. A human at the CLI reads stdout, where output is free and live, scannable feedback is “nice.” An agent reads the single MCP final message, where every word is tokens. madtea already routes these apart: MCP callers pass nil for the stdout writer, so anything streamed to a human never enters the agent’s context. So “token-efficient for the agent” and “nice for the human” are not a trade-off — they are different renderings on different streams.

Most of this already exists, and is already enforced — for one class. Forge mutations (35+ commands: create / close / edit / merge / delete / …) all route through format.MutationResult, and internal/parity/mutation_drift.go AST-scans internal/cmd and internal/mcp and fails CI if a handler authors a mutation confirmation inline instead of through that seam (allowlist empty by design). The repo runs a whole family of such guards — mutation, CLI↔MCP parity, and the MCP description-gotcha guard — each shaped as detector (non-test file) + test + reasons-in-source allowlist + a currency test, with an empty allowlist as the target state.

The gaps are everything outside forge mutations:

  • Local-git ops are explicitly excluded from the mutation guard (their messages — “Switched to branch …”, “Pushing …” — don’t use mutation verbs, and they sit in the parity IntentionalCLIOnly allowlist). The hidden finish pull lives in this class.
  • Composite workflows (finish, init) print steps to stdout but surface no structured per-step trail, and their implicit side-effects (the checkout, the pull) are not in the structured result.
  • ~15 commands print ad-hoc via fmt.Print* (repo watch / unwatch, teams add / remove, notifications mark-read, pushmirror delete / sync, githooks delete), bypassing the blessed layer, setting no cmdresult, with no MCP counterpart.

Half-applying this — a nice ledger on finish and nowhere else — is worse than not doing it: it teaches the reader to expect a confirmation, then surprises them at the first command that lacks one. A convention that lives only as a helper-plus-a-doc rots (“death by a thousand cuts”). So it must be consistent across the whole tool and enforced so it cannot re-drift.

Decision

Every side-effecting command confirms what it did, through one blessed layer; the confirmation is structurally unskippable and a drift guard backstops it. The same facts render twice — a human ledger on stdout, one compact line for the agent.

flowchart LR
  H[side-effecting handler] --> C[returns a typed Confirmation]
  C --> R{rendered by the framework}
  R -->|stdout / human / free| L[lean ledger - committed, pushed, merged, synced sha]
  R -->|message / agent / metered| M[one line - net effect plus named implicit side-effects]

  1. One blessed confirmation layer. A common Confirmation covers the single-mutation case (the existing format.MutationResult), the git-op case, and a new multi-step ledger (an ordered list of {verb, detail} steps). One shared layer renders it.

  2. A hard structural funnel. A side-effecting handler returns a typed Confirmation rather than printing — on the CLI through a typed-handler wrapper, on MCP by returning it from the consolidated handler. The framework renders it. There is no ad-hoc fmt.Print path for a side-effect’s confirmation: omitting the return, or printing instead, does not produce output the funnel will render.

  3. Two-stream rendering. The human stream (stdout) gets a lean ✓-prefixed step ledger for composites and the existing RenderConfirmation wording for single ops. The agent stream (the MCP message) gets one compact line naming the net effect plus any implicit side-effect — e.g. Merged PR #42 (work 1a2b3c4) → master @ 9f8e7d6 (the merged PR, the work commit, and the default branch’s landing sha). Because MCP discards stdout, the human ledger costs the agent zero tokens; the agent pays only for the one line.

  4. A CI drift guard backstop. A new guard joins the family (detector + test + reasons-in-source allowlist + currency test): a side-effecting command that prints ad-hoc, or returns no Confirmation, fails CI. It includes a planted-bypass test so a regression is caught, not merely discouraged.

  5. Local-git comes under the convention. checkout / pull / push / branch / … return a structured Confirmation; implicit side-effects performed inside a composite are named in that composite’s ledger and agent line. The blanket local-git exclusion is removed.

  6. Codified. The convention is documented in docs/architecture/README.md (Conventions → side-effect confirmation), of which the existing Mutation output subsection is the already-realized forge-write instance.

The “do not author a confirmation inline” rule of the mutation guard generalizes: a side-effect’s confirmation has exactly one author (the shared render layer) and exactly one carrier (the returned Confirmation).

Consequences

  • The largest blast radius of any decision so far. The funnel changes the side-effecting command contract from “return error, print along the way” to “return a Confirmation,” touching nearly every command and its tests. It shipped as a pre-public 0.14.x patch across legacy tracker 1372–legacy tracker 1381: local-git, the composite workflows, and the ~15 ad-hoc commands were brought onto the funnel, and the drift guard (internal/parity/sideeffect_drift.go) was added.
  • finish’s checkpoint/resume folds into the ledger. A cancelled run returns a partial ledger (the steps that completed) — which is exactly the structured “what already happened” the resume path wants; the ledger subsumes the ad-hoc partial-result message.
  • The re-run cure is general, not a one-off. Because every effect is named on the stream its consumer reads, neither a human nor an agent re-does a step the tool already performed — the specific finish → re-pull redundancy is one instance.
  • A new convention to honor when adding a command. Any new side-effecting command returns a Confirmation; the guard rejects a handler that prints its own. This is one more line in the “adding a command” checklist, paid down once.

Verification and enforcement

  • The new drift guard fails on a planted ad-hoc-print bypass and passes for the migrated tree; allowlist entries each carry a grounded source reason; a currency test rejects stale entries.
  • finish names its checkout + pull (with the resulting short-sha) on both streams, proven by a test that asserts the agent message and the human synced line both carry the sha.
  • The whole gate (build, race tests, parity, the guard family, docs generate --verify) is green; CLI↔MCP parity holds for any new MCP counterparts added to the ad-hoc commands.

Alternatives considered

  • finish only (the original ask). Rejected: a single confirmed command amid unconfirmed ones is the inconsistency that erodes — worse than none.
  • A soft funnel: the blessed layer plus the guard, with no signature change. A real option — the guard alone makes a bypass un-mergeable, which is most of “can’t drift” without rewriting every RunE. The hard structural funnel was chosen for the strongest guarantee (omission cannot compile, not merely cannot merge). The soft funnel remains the documented fallback if the contract change proves too invasive mid-implementation; it satisfies “consistent + enforced” at a much smaller blast radius.
  • A documented convention with no guard. Rejected: precisely the death-by-a-thousand-cuts path. madtea’s culture is codify-and-enforce; a convention without a CI guard rots on the next commit.
  • Keeping local-git excluded (mutation guard covers forge only). Rejected: the motivating gap — the hidden finish pull — is a local-git side-effect; excluding the class leaves the exact bug unaddressed.

Canonical source: docs/adr/0003-side-effect-confirmation.md in the madtea repo.