docs(decisions): D24 says "Patchwarden never approves" but #129 shipped a guarded contract_publish APPROVED adapter — reconcile the authority record + clarify approver identity #132

New issue

Closed

opened 2026-06-23 19:50:24 +02:00 by claude · 0 comments

claude commented

2026-06-23 19:50:24 +02:00

Collaborator

The inconsistency (governance-doc ↔ shipped code)

The canonical authority record now disagrees with itself and with the code. Patchwarden's entire value is the precision of its authority boundary, so this matters more here than in an ordinary repo.

D24 (docs/decisions.md, D24 bounds): "D20 intact: Patchwarden never merges/approves; it emits the verdict + handoff. The controller acts; Patchwarden vouches." and "it is not Patchwarden merging itself."
D20 (detail section): "The only APPROVED adapter belongs to src/patchwarden/contract_publish.py after a passing deterministic Contract Run and explicit operator workflow opt-in … D20 lint confines that surface to contract_publish.py + forgejo_client.py and still bans merge endpoints."
Shipped (#129): publish-contract --execute --approve-on-pass calls client.ensure_pr_approval(...) → Patchwarden's own CLI posts a visible APPROVED review (guarded by the controller_approval verdict).

So D24 asserts "Patchwarden never approves," while D20-detail + the #129 code say "the deterministic contract_publish adapter may post a guarded APPROVED review, opt-in." Both can't be literally true.

The deeper question this surfaces (the real architectural one)

In publish-contract --approve-on-pass, whose identity posts the visible APPROVED review? The approval is written via FORGEJO_TOKEN.

If that token is patchwarden-bot (id9) → the "controller acts; Patchwarden vouches" separation in D24 has effectively collapsed: Patchwarden is the visible approver. That's a real evolution of D20, not a wording nit.
If the token is a separate controller identity (ClawSweeper / codex-controller) → the separation holds, and contract_publish is just the mechanism the external controller drives.

The decision record doesn't say which, so a reader can't tell whether the D20/D24 sensor-vs-actor invariant actually survived #129. (no-self-approval only guarantees approver ≠ PR author — it does not guarantee approver ≠ Patchwarden.)

Proposed reconciliation (operator to ratify; ~edit is small)

Clarify D24 (and/or a short D26) to state the actual four-way boundary precisely:

Merge — never, by anyone called Patchwarden; D20-lint enforced. (unchanged)
Reviewer-lane approval — never; lanes are sensors. (unchanged)
Deterministic contract_publish approval — permitted under guard: opt-in (--approve-on-pass), passing exact-head Contract Run, ready zero-blocker controller_approval, no-self-approval, non-hard-manual, fail-closed.
Visible-controller approval by an external actor (ClawSweeper/OpenClaw consuming the verdict) — the target end-state (PW-G012, still partial).

And state explicitly which identity posts the #3 approval, and whether #3 is the intended steady state or a transitional dogfood mechanism on the way to #4. If #3 posts under patchwarden-bot, either (a) amend D24's "never approves" to "never merges; approves only via the guarded deterministic adapter," or (b) require #3 to run under a non-Patchwarden controller identity to preserve the separation.

Not urgent / nothing is broken in the code — the guards are sound (verified in #127). This is about keeping the canonical authority record honest now that real approval power shipped. — claude (architect loop)

## The inconsistency (governance-doc ↔ shipped code) The canonical authority record now disagrees with itself and with the code. Patchwarden's entire value is the *precision* of its authority boundary, so this matters more here than in an ordinary repo. - **D24** (`docs/decisions.md`, D24 bounds): *"**D20 intact**: Patchwarden never merges/approves; it emits the verdict + handoff. The controller acts; Patchwarden vouches."* and *"it is **not** Patchwarden merging itself."* - **D20** (detail section): *"The only `APPROVED` adapter belongs to `src/patchwarden/contract_publish.py` after a passing deterministic Contract Run and explicit operator workflow opt-in … D20 lint confines that surface to `contract_publish.py` + `forgejo_client.py` and still bans merge endpoints."* - **Shipped (#129)**: `publish-contract --execute --approve-on-pass` calls `client.ensure_pr_approval(...)` → Patchwarden's own CLI posts a **visible APPROVED review** (guarded by the controller_approval verdict). So D24 asserts "Patchwarden never approves," while D20-detail + the #129 code say "the deterministic `contract_publish` adapter *may* post a guarded APPROVED review, opt-in." Both can't be literally true. ## The deeper question this surfaces (the real architectural one) In `publish-contract --approve-on-pass`, **whose identity posts the visible APPROVED review?** The approval is written via `FORGEJO_TOKEN`. - If that token is **patchwarden-bot (id9)** → the "controller acts; Patchwarden vouches" separation in D24 has effectively collapsed: Patchwarden *is* the visible approver. That's a real evolution of D20, not a wording nit. - If the token is a **separate controller identity** (ClawSweeper / codex-controller) → the separation holds, and `contract_publish` is just the mechanism the external controller drives. The decision record doesn't say which, so a reader can't tell whether the D20/D24 sensor-vs-actor invariant actually survived #129. (`no-self-approval` only guarantees approver ≠ PR author — it does **not** guarantee approver ≠ Patchwarden.) ## Proposed reconciliation (operator to ratify; ~edit is small) Clarify D24 (and/or a short D26) to state the *actual* four-way boundary precisely: 1. **Merge** — never, by anyone called Patchwarden; D20-lint enforced. (unchanged) 2. **Reviewer-lane approval** — never; lanes are sensors. (unchanged) 3. **Deterministic `contract_publish` approval** — *permitted under guard*: opt-in (`--approve-on-pass`), passing exact-head Contract Run, ready zero-blocker `controller_approval`, no-self-approval, non-hard-manual, fail-closed. 4. **Visible-controller approval by an external actor** (ClawSweeper/OpenClaw consuming the verdict) — the target end-state (PW-G012, still `partial`). And state explicitly **which identity** posts the #3 approval, and whether #3 is the intended steady state or a transitional dogfood mechanism on the way to #4. If #3 posts under patchwarden-bot, either (a) amend D24's "never approves" to "never *merges*; approves only via the guarded deterministic adapter," or (b) require #3 to run under a non-Patchwarden controller identity to preserve the separation. Not urgent / nothing is broken in the code — the guards are sound (verified in #127). This is about keeping the canonical authority record honest now that real approval power shipped. — claude (architect loop)

Rows
Columns