ADR 0002 — Safe inline body updates: anchored capture-then-update with optimistic-lock write-back

  • Status: Accepted
  • Date: 2026-06-03
  • Tracking: legacy tracker 1349 (RFC / brainstorm). Supersedes and subsumes legacy tracker 1348 (reconcile CLI --body with the MCP body-edit block; add a trample-safe checklist toggle). Unblocks the stale tracking-issue bodies on legacy tracker 1272, legacy tracker 934, legacy tracker 1282.

Context

madtea lets an agent or a human edit issue bodies, pull-request bodies, and comment bodies. The motivating failure is clobbering: an agent regenerates a tracking issue’s body from a stale read and silently discards everything that was there — closed sub-issue checkboxes, hand-written notes, a progress counter. The blunt fix that shipped earlier blocks issue-body edits on the MCP (agent) surface entirely and tells the agent to use a comment instead. That stopped the clobbering, but it also forbade every legitimate inline update (tick a box, fix a typo, append a section, sync a checklist), so tracking-issue bodies rot while the truth lives in comments — exactly the live state on legacy tracker 1272, legacy tracker 934, and legacy tracker 1282.

The protection is also inconsistent. An audit of the three body surfaces found the guard covers only one of them, and even that one only on a single entry point:

SurfaceMCP (agent)CLI (human)
Issue bodyBlocked — tools_issues_crud.go rejects the call if body is presentUnprotected — edit --body / --file-body do a full replace
PR bodyUnprotected — full replaceUnprotected — full replace
Comment bodyUnprotected — full replaceUnprotected — full replace

The block is a single MCP-layer check; the service and Gitea-client layers happily write any body. So the guard is shallow (one layer), narrow (one surface), and asymmetric (an agent shelling out to the CLI, or editing a PR body, bypasses it). The README and docs/COMPAT.md advertise “agents can’t silently overwrite an issue body” — true today only for issue bodies on MCP, which is a claim we want to make truthful and blanket.

What the live forge actually supports (verified, Gitea 1.25.4)

The design depends on whether Gitea offers a server-side optimistic-concurrency primitive, so this was confirmed against the running instance rather than assumed:

  • Issue bodies — native support. The Issue object returned by GET .../issues/{index} carries a content_version integer (“the version of the issue content for optimistic locking”). EditIssueOption (the PATCH body) accepts a matching content_version field. On a mismatch the PATCH returns HTTP 412 Precondition Failed.
  • PR bodies — native support. Identical: PullRequest GET carries content_version, EditPullRequestOption accepts it, conflict is 412.
  • Comment bodies — no support at all. The Comment GET response has no content_version, and EditIssueCommentOption is { body } only. There is no server-side lock for comments on this version.

Two consequences fall out of the verification. First, the concurrency token travels in the PATCH request body, not as an X-Gitea-Content-Version header — so no conditional-header plumbing or response-header capture is needed; it is one field on the existing edit-options structs plus one field surfaced on the get. Second, internal/gitea/client_errors.go currently maps 401/403/404/413/422 but not 412, so the conflict status needs a mapping.

Decision

Policy

No caller — agent or human, CLI or MCP — may replace an issue body or PR body wholesale. Every change to those bodies is a surgical, anchored edit applied to the live body, and is rejected if the anchor drifted or the body changed underneath it. Comment bodies keep full-replace but gain a write-time conflict guard. Creating a new issue, PR, or comment still takes a full body, because there is nothing yet to clobber.

This closes the asymmetry by levelling up: there is no wholesale-overwrite path for issue or PR bodies anywhere, on any surface, for any caller. There is no --force backdoor.

The protected spine

Every body mutation runs the same protected cycle, entirely inside madtea — the caller never sends a whole body for issue/PR edits.

flowchart LR
  A[GET live body plus content_version] --> B[apply op in memory]
  B --> C{anchors matched and op valid}
  C -- no --> R[reject, return current body and reason]
  C -- yes --> D[PATCH new body with captured content_version]
  D --> E{HTTP 412 conflict}
  E -- yes --> R
  E -- no --> F[done]
  • The anchor is the proof of a fresh read. A caller cannot author a matching old_string without having seen current state, so no separate capture token is required. madtea captures content_version itself on the apply-time GET and sends it back on the PATCH. Gitea rejects with 412 if anything changed in the window between madtea’s read and its write, so a concurrent edit to any part of the body — not just the region being patched — cannot be lost.
  • Anchor matching guards the region; content_version guards the whole body. Together they make both lost-update and drifted-context outcomes impossible: the first via the 412, the second via the failed anchor match.
  • Atomic. A multi-replacement patch applies all-or-nothing; if any single anchor fails to match, nothing is written.
  • Capture is just a read. get is extended to surface content_version, so a normal read is the capture. No dedicated “capture read” endpoint is introduced.

Comments have no native lock, so their guard is client-side: the caller passes the expected prior body (or a hash of it); madtea re-fetches the comment, compares, rejects on drift, and otherwise writes the new full body. This keeps full-replace ergonomics for what are usually short, self-authored comments while still refusing a silent lost-update.

The four body verbs (issue and PR bodies)

Exposed under a body namespace. patch is the general primitive; the other three are intent-revealing sugar over the same spine for the cases that actually recur in tracking issues.

VerbBehaviourSafety rule
patchA set of (old → new) anchored string replacements applied to the live bodyUnique-match by default: zero matches (stale/drifted) → reject; more than one (ambiguous) → reject and ask for more surrounding context. A per-replacement replace_all: true is the explicit escape for deliberate multi-site edits.
appendAdd content to the end of the bodyNo anchor needed (end-of-body is unambiguous); still goes through the content_version write-back.
task-toggleSet the one - [ ] / - [x] line matched by substring or issue-ref to an explicit checked stateUnique-match on the target line. Takes an explicit target state (not a blind flip), so it is idempotent — already in the requested state is a no-op-with-notice, not an error. Subsumes legacy tracker 1348’s checklist primitive.
section-replaceReplace the content between named markers, targeted by nameMarkers are <!-- madtea:section:NAME --><!-- /madtea:section:NAME -->. Reject if the markers are absent or unbalanced. A missing section can be seeded by an --ensure flag that appends a fresh marked block.

The operator-facing reference for these ops lives in docs/safety/safe-body-edits.md; this ADR records only the decision and its rationale.

Surfaces

  • CLI. A body subcommand group: madtea issue body patch|append|task-toggle|section-replace, and the same under madtea pr body …. The existing madtea issue/pr edit --body and --file-carried body full-replace are removed. This is a breaking CLI change; per the pre-public versioning policy it ships as a 0.14.x patch bump.
  • MCP. A single action=body on madt_issues and madt_prs, dispatched by an op field (patch | append | task_toggle | section_replace) — one new action per noun rather than four flat body_* actions, mirroring the CLI’s body <op> grouping. The body field is removed from action=edit. Op names match the CLI verbs under the standard kebab→snake convention (task-toggletask_toggle, section-replacesection_replace).
  • Comments. comment_edit keeps its full-replace shape on both surfaces but gains an expected_body (or hash) parameter that drives the client-side conflict guard.

Failure UX

A rejection — failed anchor match, ambiguity, a 412, or a comment drift — returns the current body alongside the reason, so the caller can re-author against fresh state in a single round-trip instead of guessing.

Defaults chosen within this decision

These were settled as part of the design and are recorded so they are not silently re-litigated:

  • MCP encoding is one action=body with an op field (not four body_* actions).
  • task-toggle takes an explicit target state and is idempotent (no-op-with-notice when already satisfied).
  • section-replace seeds a missing section via --ensure (a marked append) rather than failing hard when a body has not yet adopted the marker convention.
  • Multi-replacement patch is transactional (all-or-nothing).
  • The comment guard accepts either the expected prior body or a hash of it; the hash form keeps the payload small.

Consequences

  • The no-clobber guarantee becomes real and blanket. Issue and PR bodies have no wholesale-overwrite path on any surface; comments are guarded against lost updates. The README and COMPAT.md claim can move from “issue body” to all three body surfaces.
  • Small, well-scoped client work for bodies. Native content_version means: add a content_version field to UpdateIssueOptions and UpdatePROptions (sent in the PATCH body), surface content_version on the issue/PR get and its formatter, capture it on the apply-time fetch, and add a 412 → conflict mapping in client_errors.go. No header plumbing, no Response-metadata capture.
  • New surface to maintain. Four verbs × two nouns on two surfaces, plus the comment guard, each carrying its parity entry, COMPAT.md row, and generated-docs entry. This is paid down once and then enforced mechanically (below).
  • legacy tracker 1348 is absorbed. Its two asks — reconcile the CLI/MCP body-edit divergence, and add a trample-safe checklist toggle — are the “drop full replace everywhere” policy and the task-toggle verb respectively. legacy tracker 1348 is superseded by this ADR and tracked as sub-issues of the legacy tracker 1349 epic.
  • The rotting checklists get a fix. Once shipped, the stale bodies on legacy tracker 1272, legacy tracker 934, and legacy tracker 1282 can be corrected with task-toggle and section-replace instead of leaving the truth stranded in comments.
  • Marker convention is opt-in. section-replace only works on bodies that carry the markers; --ensure lets a tracking issue adopt them incrementally. Bodies that never adopt markers are unaffected.

Verification and enforcement

  • Rejection tests are the definition of done. Tests must prove a stale or blind update is rejected, covering both failure modes: the lost-update case (the body’s content_version advanced between read and write → 412 → reject) and the drifted-context case (the anchored old_string no longer matches, or matches ambiguously → reject). Comment drift (expected body no longer current → reject) is tested the same way.
  • Parity. Every body op appears on both CLI and MCP with matching canonical names; the internal/parity/ test fails CI on any single-surface drift without a justified allow-list entry. The comment_edit expected_body parameter is added on both surfaces.
  • Docs in sync. docs/COMPAT.md rows for issue/PR/comment body editing are updated to describe the anchored ops and the guard; madtea docs generate regenerates the reference docs; the README differentiation claim is widened to all body surfaces.
  • 412 handling is covered. client_errors.go gains a 412 case mapped to a clear, retry-suggesting conflict error, with a test.

Alternatives considered

  • Optimistic-concurrency guard as the primary mechanism, full-body edits re-enabled behind it (design-space option 1). Rejected as the primary surface: it still accepts a whole body from the caller, which keeps the regenerate-from-stale-read footgun alive even when technically conflict-checked — the agent can re-read, then still send a wholesale body that drops content it never noticed. content_version is retained as the write-back guard under anchored editing, not as the caller-facing primitive.
  • Structured region ops only, no general edit primitive (option 3 alone). Rejected as insufficient: marker-delimited and checklist ops cannot express an arbitrary in-place edit (rewrite a sentence mid-paragraph that is not in a marked section), so a body would still need a general escape — and the only escape left would be full replace. Anchored patch is that general primitive, with the structured ops layered on top.
  • Anchored patch on MCP only; CLI keeps full --body replace. Rejected: this preserves exactly the silent CLI/MCP divergence legacy tracker 1348 calls out. The policy levels up instead — no wholesale path on either surface.
  • Full --body replace behind a --force/confirmation gate on the CLI. Rejected in favour of removing the wholesale path entirely; a gated footgun is still a footgun, and a human who wants a large rewrite can express it as a patch or a section-replace.
  • Heading-delimited or caller-supplied-anchor sections for section-replace. Rejected in favour of an explicit madtea marker convention: headings are brittle (rename, repeat, or re-nest and the boundary moves), and caller-supplied start/end anchors collapse into a special case of patch. Named invisible markers are stable across edits, render invisibly in Gitea, and are self-documenting.
  • Drop the comment guard entirely (document comments as an exception). Rejected: with the client-side hash guard, the no-clobber guarantee can cover all three body surfaces, which keeps the differentiation claim honest and blanket. The cost is one parameter and a re-fetch on comment edits.
  • X-Gitea-Content-Version request header for the write-back. Not applicable: verification showed Gitea 1.25.4 carries the token as a body field on the edit options, not a header.

Canonical source: docs/adr/0002-safe-inline-body-updates.md in the madtea repo.