fix(ci): let an administrator merge their own green PR
gate / gate (pull_request) Failing after 1m44s
gate / conformance (pull_request) Has been skipped

`required_approvals: 1` on `main` was a lock-out, not a protection. Gitea
refuses to let a PR's author approve their own PR, and `jakman85` is the only
collaborator — so no account in existence could approve a PR the operator
opened, including the PR that would fix that. Measured 2026-10-01 on PR #35:
gate green in 1m41s, reviews `[]`, `required_approvals: 1`, unmergeable.

It was never the documented design either: `ci/approval-policy.sh`'s header and
`ci/gate.sh:194` both already describe the model as "required_approvals drops
to 0 and the GATE decides", and `require-gate.sh`'s own closing note named this
failure and prescribed this remedy. 2026-09-30's entry flagged the 1-vs-0
disagreement as unresolved; this settles it at 0.

Layer 1 — branch protection. required_approvals 1 -> 0 in the declaration, plus
infra/gitea/allow-admin-self-merge.sh to apply it live (target read FROM the
declaration, so the script cannot drift from what the gate checks).
require-gate.sh moves to 0 too; left at 1 it would re-impose the lock-out on its
next run. The narrow choice is deliberate: the other way to unblock an owner is
block_admin_merge_override: false, which would re-open what
block-admin-override.sh was written to close (PRs #5-#9, zero approvals). This
drops the approval COUNT only — block_admin_merge_override and
enable_status_check stay true and are asserted after the PATCH, so a red gate
stays unmergeable by everyone, administrators included.

Layer 2 — ci/approval-policy.sh. Sensitive paths still need a non-author
approval the sole admin cannot get, so an administrator may waive it with an
`/approve-override <reason>` comment: directive must start a line, reason
mandatory, permission re-checked LIVE, invalidated by a new commit, and the
overrider + reason printed in the gate log and job summary. Candidate selection
is the pure, offline ci/approval-override.py (23 unit assertions, all passing);
authorisation stays in the fail-closed stage. Every new failure mode blocks.

The cost, stated not buried: for an administrator, "a change to the gate cannot
be gated by the gate" is now advisory rather than structural. Unavoidable with
one human; what is bought back is an attributed, commit-scoped record instead of
a silent bypass. And the uncomfortable part, accepted as an operator decision:
authorisation is by live permission alone, and the AGENT account `gitea-admin`
also holds `owner` — so an agent can clear its own PR this way. Documented in
approval-policy.json, the stage header, and CLAUDE.md (which tells agents not
to). Narrowing needs no code change: add `override_admins` and the stage already
requires membership.

Adjacent fix, in scope because `make -C ci test` is the only runner for the
suite added here: ci/tests/gate-decision.test.sh hand-maintained a stub list
that "must name EVERY stage gate.sh runs" and omitted `approval-policy` (added
to gate.sh by PR #17), so run_stage hit exit 127 and 15 of 50 assertions failed.
Verified pre-existing on pristine origin/main @ 0c7e8dd. The list is now derived
from gate.sh's own run_stage lines, so the omission cannot recur; the suite goes
15-failed -> 50 passing and `make -C ci test` goes FAILED -> all suites passed.

Toolbox gate: rc 3 UNDETERMINED — every stage green except `gitea-settings`
(no SETTINGS_API_TOKEN locally) and `conformance` (no cluster). NOT a green run.

The live rule is UNCHANGED and still reads 1 — mutating it is an operator
action. Until allow-admin-self-merge.sh is run, ci/gitea-settings.sh will
correctly report DRIFT and this PR will be red.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
jakman85andClaude Opus 5 committed 2026-10-01 23:54:51 -05:00
1 parent 0c7e8dd3ba
commit 7b9f733c97
11 files changed
+833 -17

No files matched your search

@@ -0,0 +1,182 @@
---
date: 2026-10-01
branch: fix/admin-approval-override
title: Administrators can merge their own green PR — required_approvals 1 → 0, plus an attributed /approve-override for sensitive paths
---
## What changed
**The problem, measured first.** `required_approvals: 1` on `main` was a lock-out, not a
protection. Gitea refuses to let a pull request's author approve their own PR, and `jakman85` is
the **only** collaborator (permission `owner`). So no account in existence could approve a PR the
operator opened. Measured on the live API, 2026-10-01:
| | |
|---|---|
| PR #35 (`bugfix/arr-to-plex`, author `jakman85`) | `gate / gate (pull_request)` = **success, 1m41s** |
| its reviews | `[]` |
| live `required_approvals` | `1` |
| live `block_admin_merge_override` | `true` |
Green gate, zero reviews, no path to a merge — and the same was true of any PR that would have
fixed it.
It was also never the design this repo wrote down. `ci/approval-policy.sh`'s own header and
`ci/gate.sh:194` both already described the model as *"`required_approvals` drops to 0 and the
GATE decides"*, and `infra/gitea/require-gate.sh`'s closing note named this exact failure and
prescribed this exact remedy. `decisions/2026-09-30-flux-deploy-notification-inhibited.md` had
flagged the 1-vs-0 disagreement as unresolved and not that session's to settle. This settles it
at 0.
**Two layers were blocking, and both are addressed.**
*Layer 1 — Gitea branch protection.* `required_approvals: 1 → 0` in
`infra/gitea/expected-branch-protection.json`, with the rationale in the file, plus
`infra/gitea/allow-admin-self-merge.sh` to apply it to the live rule (it reads the target **from
the declaration**, so the script cannot drift from what the gate checks). `require-gate.sh` was
changed to 0 as well — left at 1 it would silently re-impose the lock-out on its next run.
The narrow choice matters: the *other* way to unblock an owner is
`block_admin_merge_override: false`, and that would have re-opened precisely what
`infra/gitea/block-admin-override.sh` was written to close (PRs #5–#9 merged with zero approvals
while the settings read as enforced). This change drops the approval **count** only.
`block_admin_merge_override` and `enable_status_check` both stay `true`, and the script asserts
both afterwards and fails loudly if the PATCH disturbed either. **A red gate remains unmergeable
by everyone, administrators included.**
*Layer 2 — `ci/approval-policy.sh`.* Sensitive paths (`ci/**`, `infra/gitea/**`, `CLAUDE.md`,
kyverno, rbac, sops, Flux wiring) still require a non-author `APPROVED` review, which the sole
administrator still cannot obtain. An administrator may now waive it by commenting on the PR:
/approve-override <reason>
- the directive must **start a line** (a quoted or mid-sentence mention does not count, or
repeating someone else's override would re-arm it);
- a **reason is mandatory** — a bare directive records that review was waived but not why;
- the commenter's permission is **re-checked live** against
`/collaborators/{login}/permission` when the gate runs, so an old comment from a demoted
account carries nothing;
- it is **invalidated by a new commit** (the comment must post-date the head commit), mirroring
Gitea's own `dismiss_stale_approvals`;
- the overrider, timestamp and reason are printed in the gate log and the job summary, and a
self-waiver by the PR's own author is called out as such.
Selection of the candidate comment is `ci/approval-override.py` — pure, offline, no authority of
its own — so it could be unit-tested; authorisation stays in the fail-closed stage.
`ci/tests/approval-override.test.sh` adds **23 assertions**, all passing, each constructing a
case the parser must refuse. Every new failure mode blocks: unreadable head-commit timestamp,
unreadable permission, parser error.
**The cost, stated rather than buried.** For an administrator, *"a change to the gate cannot be
gated by the gate"* is now **advisory, not structural**. That is unavoidable with one human — the
alternative is a repository its owner cannot change — and what is bought back is an attributed,
reasoned, commit-scoped record of each waiver instead of a silent bypass.
**The uncomfortable part, accepted with eyes open (operator decision).** Authorisation is by live
permission **alone**, with no allowlist of human accounts. The **agent account `gitea-admin` also
reports permission `owner`** on this repo (measured), so an agent *can* clear the sensitive-path
review on its own PR — and agents author most PRs here, so that is the common case, not an edge
case. I raised this before implementing; the operator chose permission-only. It is documented in
three places rather than left to be discovered: `approval-policy.json`
(`_known_consequence_agent_accounts`), the stage's header, and `CLAUDE.md`, which now tells
agents plainly **not** to use it on their own work. The narrowing path needs no code change —
add an `override_admins` list to `approval-policy.json` and the stage already requires membership
in addition to the permission check.
Also updated: `CLAUDE.md` §1 and the §2 enforcement table (the "approval required" row is now
explicitly ⬜ with the reason, rather than a stale ✅), and `infra/gitea/README.md` gains §6 and
marks its historical step 4 as reversed — do not re-apply.
**One adjacent fix, deliberately in scope: `ci/tests/gate-decision.test.sh` was already broken.**
It hand-maintains a list of stub stage scripts that "must name EVERY stage `gate.sh` runs", and it
did not name `approval-policy` — added to `gate.sh` by PR #17 and never added here. `run_stage`
therefore got exit 127 from a missing file and recorded a FAILED stage in every case, so **15 of
50 assertions failed**, including every one expecting a PASS/UNDETERMINED headline. Measured on
pristine `origin/main` (`0c7e8dd`) before touching it, so the attribution is not a guess.
I fixed it because `make -C ci test` is the *only* runner for the suite this change adds — leaving
it red would have made the new tests unverifiable in practice. The file's own comment already
narrated this exact failure for `gitea-settings` and did not prevent the repeat, which says the
comment was not the fix. So the stub list is now **derived from `gate.sh`'s own `run_stage` lines**
and asserts it parsed at least 8 of them: a stage added to the gate can no longer be silently
missing from its test. `make -C ci test` goes from **FAILED to all suites passed**.
## What I verified
- **The lock-out is real, and PR #35 is the proof.** Queried live: head sha
`8a094c46`, statuses `gate / gate (pull_request) = success :: Successful in 1m41s`,
`/pulls/35/reviews` → `[]`, `/branch_protections/main` → `required_approvals: 1`. I read the
**rule object**, not `/branches/main`, per §2b.
- **`jakman85` is the only collaborator**: `/collaborators` → one entry. Permission `owner`.
- **`gitea-admin` also reports `owner`** — this is why the agent-account consequence above is a
statement of fact and not a hypothetical.
- **PR #35 touches no sensitive paths** (`apps/wave-0/plex.yaml`, `apps/wave-1/{radarr,sonarr}.yaml`,
`catalog/{radarr,sonarr}.yaml`, its decisions entry) — so `ci/approval-policy.sh` was never its
blocker. Layer 1 alone unblocks it.
- **API shapes verified before depending on them**, rather than assumed: `/git/commits/{sha}`
returns `.commit.committer.date` as RFC3339 **with an offset** (`2026-10-01T23:09:08-05:00`),
not a trailing `Z` — a parser handling only `Z` would have silently dropped every real
override, so there is a test for the offset form. PR conversation comments are under
`/issues/{n}/comments`; `/pulls/{n}/comments` returns diff comments only and would never have
seen an override.
- **`ci/tests/approval-override.test.sh`: 23 passed, 0 failed.** Covers the happy path, bare
directive, mid-sentence and `>`-quoted directives, stale and equal-timestamp overrides,
unparseable timestamps, missing login, invalid JSON, non-list JSON, latest-wins, offset
timestamps, and a configurable directive.
- `bash -n` clean on `ci/approval-policy.sh`, `infra/gitea/allow-admin-self-merge.sh`,
`infra/gitea/require-gate.sh`; `jq -e .` clean on both JSON declarations.
- **`gate-decision.test.sh`'s 15 failures are pre-existing, measured not assumed.** Extracted
`origin/main`'s `ci/` at `0c7e8dd` into a throwaway directory and ran the suite from there:
**FAIL, 15 failed / 35 passed**. After deriving the stub list from `gate.sh`: **PASS, 50
assertions**.
- **`make -C ci test` in the pinned toolbox: `ci/tests: all suites passed`, rc 0** — approval-override
23, gate-decision 50, touched-requirements-base 31, test-attribution 15, touched-requirements 26,
decisions-log pass. It was rc 2 (`ci/tests: FAILED`) before the stub-list fix.
- **Toolbox merge gate** (`./infra/devcontainer/dev.sh run bash -c './ci/gate.sh'`), run on the
rebased branch: **rc 3, `MERGE GATE: UNDETERMINED`**. Green stages: spec-immutability,
decisions-log, lint, l0 (23 offline cases, 0 failures), render-golden including the new
`catalog/render.py --selftest`, l1 (3 cases), approval-policy (N/A offline). Skipped:
`gitea-settings` (no `SETTINGS_API_TOKEN` on this host) and `conformance` (no cluster).
**This is NOT a green run and I am not reporting it as one** — rc 3 is UNDETERMINED per the
CLAUDE.md §3 contract.
## What I did NOT verify
- **The live Gitea rule still says `required_approvals: 1`. I did not change it.** Mutating live
platform settings is an operator action; `infra/gitea/allow-admin-self-merge.sh` is written,
syntax-checked, and **unrun**. Until it runs, `ci/gitea-settings.sh` will report **DRIFT** on
this branch (declared 0, live 1) and the PR will be **red** — correctly. The declaration and
the live rule must move together.
- **I have not seen the override accepted by a real CI run.** The code path has never executed
against live Gitea. The first proof will be this PR itself: it touches `ci/**`,
`infra/gitea/**` and `CLAUDE.md`, so it needs an override to pass its own new stage.
Because CI checks out the PR's merged tree, the *new* `approval-policy.sh` is what runs — it is
self-bootstrapping — but that is reasoning, not a measurement. **If the override does not work,
this PR cannot merge**, and the recovery is to PATCH `required_approvals` and read the job log.
- **I have not seen the gate still REFUSE a merge** now that approvals are 0. The claim that
`block_admin_merge_override: true` keeps a red gate unmergeable rests on Gitea's documented
behaviour and on the 2026-09-29 session, **not** on a provoked refusal in this one. That probe
is written out at the foot of `allow-admin-self-merge.sh` and should be done, exactly as the
push rejection was provoked rather than assumed.
- **Clock skew in the staleness check is unhandled.** Staleness compares the comment's
server-side `created_at` against the head commit's *committer* date, which comes from the
committer's machine. A commit dated in the future would reject legitimate overrides until real
time catches up (fail-closed, so loud, but annoying). Gitea's own `dismiss_stale_approvals`
compares SHAs instead; a comment cannot. Not hit here — PR #35's commit date is ~21 min behind
server time — and not fixed.
- **`ci/tests/approval-override.test.sh` does not run in the merge gate.** `ci/gate.sh` runs no
`ci/tests/*`; they run only via `make -C ci test`. So a regression in the parser would not be
caught by CI. PR #33 (merged while this branch was being written, and rebased onto here) fixed
the *same class* of gap for `catalog/render.py --selftest` by invoking it from
`ci/render-golden.sh` — it did **not** wire `ci/tests/*` in, which I confirmed after rebasing
rather than assuming. Wiring them is a one-line stage and I did not widen scope to it.
- **Authorisation itself is untested code.** `ci/approval-override.py` is unit-tested; the live
permission lookup, the `override_admins` allowlist branch, and every new BLOCK path in
`ci/approval-policy.sh` need Gitea and have never been exercised. The allowlist branch in
particular is implemented but **unused** — `override_admins` is absent by design.
- **The other open PRs will go red** once the live value changes, until they are rebased onto the
merged declaration: as of writing those are **#29** and **#37** (both `gitea-admin`), plus
**#35**. I did not rebase them. Merging them first avoids it entirely; the script's header gives
the order. PRs #33, #34 and #36 merged while this branch was being written — this branch is
rebased onto `0c7e8dd`, so the list was re-measured rather than carried forward.
- No cluster state was read or changed; nothing here touches Flux, Kyverno or any workload.