fix(ci): let an administrator merge their own green PR — required_approvals 1 → 0, plus an attributed /approve-override #38

Merged
jakman85 merged 2 commits from fix/admin-approval-override into main 2026-10-02 06:17:17 +00:00
Owner

The lock-out, measured first

required_approvals: 1 on main is 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 — so no account in existence can approve a PR the operator opens, including the PR that would fix that.

Measured against the live API on 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
collaborators exactly one: jakman85 (owner)

It was also never the design this repo wrote down. 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 infra/gitea/require-gate.sh's own closing note named this exact failure and prescribed this exact remedy. decisions/2026-09-30-flux-deploy-notification-inhibited.md flagged the 1-vs-0 disagreement as unresolved. This settles it at 0.

Layer 1 — branch protection

required_approvals: 1 → 0 in infra/gitea/expected-branch-protection.json, plus infra/gitea/allow-admin-self-merge.sh to apply it to the live rule. The script reads its target from the declaration, so it cannot drift from what ci/gitea-settings.sh checks. require-gate.sh moves to 0 as well — left at 1 it would silently re-impose the lock-out on its next run.

The narrow choice is the point. The other way to unblock an owner is block_admin_merge_override: false, and that would re-open 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:

before after
required_approvals 1 0
block_admin_merge_override true true (asserted after the PATCH)
enable_status_check true true (asserted after the PATCH)

So a red gate remains unmergeable by everyone, administrators included. What changed is that a green one no longer needs a signature that cannot exist.

Layer 2 — ci/approval-policy.sh

Sensitive paths still require a non-author APPROVED review the sole administrator cannot obtain. An administrator may now waive it by commenting on the PR:

/approve-override <reason>
  • directive must start a line — a quoted or mid-sentence mention does not count, or repeating someone else's override would re-arm it;
  • reason mandatory — a bare directive records that review was waived but not why;
  • permission re-checked live against /collaborators/{login}/permission when the gate runs, so an old comment from a since-demoted account carries nothing;
  • invalidated by a new commit (the comment must post-date the head commit), mirroring Gitea's own dismiss_stale_approvals;
  • overrider, timestamp and reason printed in the gate log and the job summary; a self-waiver by the author is called out as such.

Candidate selection is ci/approval-override.py — pure, offline, no authority of its own — so it is unit-testable; authorisation stays in the fail-closed stage. Every new failure mode blocks: unreadable head-commit timestamp, unreadable permission, parser error.

What this costs, stated rather than buried

For an administrator, "a change to the gate cannot be gated by the gate" becomes 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, an accepted operator decision: authorisation is by live permission alone, with no allowlist. The agent account gitea-admin also reports owner, so an agent can clear the sensitive-path review on its own PR — and agents author most PRs here. This was raised before implementing and chosen deliberately. It is documented in three places rather than left to be discovered (approval-policy.json _known_consequence_agent_accounts, the stage header, and CLAUDE.md, which tells agents plainly not to use it on their own work). Narrowing needs no code change: add an override_admins list to approval-policy.json and the stage already requires membership.

Adjacent fix

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. Fixed because make -C ci test is the only runner for the suite this PR adds. The list is now derived from gate.sh's own run_stage lines, so the omission cannot recur.

Verification

  • ci/tests/approval-override.test.sh — 23 assertions, all passing: bare directive, mid-sentence and >-quoted directives, stale and equal-timestamp overrides, unparseable timestamps, missing login, invalid/non-list JSON, latest-wins, RFC3339 offset timestamps (which is how Gitea actually emits them — a Z-only parser would have dropped every real override), configurable directive.
  • make -C ci test in the pinned toolbox: all suites passed, rc 0 (was rc 2).
  • Toolbox merge gate: rc 3 UNDETERMINED — spec-immutability, decisions-log, lint, l0, render-golden (incl. the new --selftest), l1, approval-policy all green; gitea-settings skipped (no local SETTINGS_API_TOKEN), conformance skipped (no cluster). Not a green run, and not reported as one.
  • API shapes verified before being depended on: /git/commits/{sha} → .commit.committer.date; PR conversation comments live under /issues/{n}/comments (/pulls/{n}/comments returns diff comments only and would never have seen an override).

⬜️ What is left: post the override as a REVIEW, not a comment

required_approvals: 0 is live and gitea-settings is green (run 206). The one remaining blocker is this PR's own approval-policy stage, and the first attempt at clearing it found a real defect — now fixed in this PR.

What went wrong on the first try. A well-formed /approve-override posted in the PR comment box, 29 minutes newer than the head commit, was invisible to CI. Gitea's own words, from run 206:

token does not have at least one of required scope(s),
required=[read:issue], token scope=read:admin,read:repository

SETTINGS_API_TOKEN is minted [read:repository, read:admin], and /issues/{n}/comments sits behind the issue scope — so Gitea returned an error object, jq -e . accepted it as valid JSON, and the stage fell through to "no approving review". A permission failure wearing the costume of a missing override. /pulls/{n}/files and /pulls/{n}/reviews are under read:repository, which is why every other part of the same stage worked.

Three fixes, the first mattering most: the diagnosis no longer lies (it prints Gitea's .message and names the scope); an override is accepted from a PR review body as well as a comment, so the hatch is not hostage to one API scope; and set-ci-token.sh now requests read:issue so the comment box works after a re-run.

To land this PR

  1. Files → Review → "Comment" on this PR (not the plain comment box at the bottom), with the body starting:
    /approve-override <reason>
    
    It must be newer than head commit b52c8d1 (pushed 2026-10-02T05:50:20Z), so post it now — earlier overrides are stale by design.
  2. Re-run the gate — from the Actions UI ("Re-run all jobs"), or by closing and reopening the PR. Posting the override does not re-trigger it: gate.yml has no issue_comment trigger, and that was left alone on purpose (a comment trigger would check out PR-head code while the job holds SETTINGS_API_TOKEN — the classic pwn-request shape). Do not push an empty commit to re-trigger, that moves the head and makes the override stale again.
  3. approval-policy should report OVERRIDDEN by jakman85 (permission owner, via PR review, self-waived as author) with your reason. Merge.
  4. Optional, afterwards: re-run ./infra/gitea/set-ci-token.sh to pick up read:issue, and the ordinary comment box works next time. Token scopes cannot be widened in place.

Still not verified: the accepting branch of the override (directive found → live permission lookup → exit 0) has never executed — step 1 is its first real test, and a third round would not be a surprise. I also did not confirm Gitea lets a PR author submit a COMMENT-type review (expected: it blocks self-approval only), and I deliberately did not test it by posting an override myself — doing that as gitea-admin is exactly the agent self-override CLAUDE.md now forbids. Full gap list in decisions/2026-10-01-admin-approval-override.md.

🤖 Generated with Claude Code

## The lock-out, measured first `required_approvals: 1` on `main` is 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 — so no account in existence can approve a PR the operator opens, *including the PR that would fix that.* Measured against the live API on 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` | | collaborators | exactly one: `jakman85` (`owner`) | It was also never the design this repo wrote down. `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 `infra/gitea/require-gate.sh`'s own closing note named this exact failure and prescribed this exact remedy. `decisions/2026-09-30-flux-deploy-notification-inhibited.md` flagged the 1-vs-0 disagreement as unresolved. **This settles it at 0.** ## Layer 1 — branch protection `required_approvals: 1 → 0` in `infra/gitea/expected-branch-protection.json`, plus `infra/gitea/allow-admin-self-merge.sh` to apply it to the live rule. The script reads its target **from the declaration**, so it cannot drift from what `ci/gitea-settings.sh` checks. `require-gate.sh` moves to 0 as well — left at 1 it would silently re-impose the lock-out on its next run. **The narrow choice is the point.** The other way to unblock an owner is `block_admin_merge_override: false`, and that would re-open 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: | | before | after | |---|---|---| | `required_approvals` | 1 | **0** | | `block_admin_merge_override` | true | true (asserted after the PATCH) | | `enable_status_check` | true | true (asserted after the PATCH) | So **a red gate remains unmergeable by everyone, administrators included.** What changed is that a *green* one no longer needs a signature that cannot exist. ## Layer 2 — `ci/approval-policy.sh` Sensitive paths still require a non-author `APPROVED` review the sole administrator cannot obtain. An administrator may now waive it by commenting on the PR: ``` /approve-override <reason> ``` - directive must **start a line** — a quoted or mid-sentence mention does not count, or repeating someone else's override would re-arm it; - **reason mandatory** — a bare directive records that review was waived but not why; - permission **re-checked live** against `/collaborators/{login}/permission` when the gate runs, so an old comment from a since-demoted account carries nothing; - **invalidated by a new commit** (the comment must post-date the head commit), mirroring Gitea's own `dismiss_stale_approvals`; - overrider, timestamp and reason printed in the gate log **and** the job summary; a self-waiver by the author is called out as such. Candidate selection is `ci/approval-override.py` — pure, offline, no authority of its own — so it is unit-testable; authorisation stays in the fail-closed stage. Every new failure mode **blocks**: unreadable head-commit timestamp, unreadable permission, parser error. ## What this costs, stated rather than buried For an administrator, *"a change to the gate cannot be gated by the gate"* becomes **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, an accepted operator decision:** authorisation is by live permission **alone**, with no allowlist. The **agent account `gitea-admin` also reports `owner`**, so an agent *can* clear the sensitive-path review on its own PR — and agents author most PRs here. This was raised before implementing and chosen deliberately. It is documented in three places rather than left to be discovered (`approval-policy.json` `_known_consequence_agent_accounts`, the stage header, and `CLAUDE.md`, which tells agents plainly **not** to use it on their own work). Narrowing needs **no code change**: add an `override_admins` list to `approval-policy.json` and the stage already requires membership. ## Adjacent fix `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`. Fixed because `make -C ci test` is the only runner for the suite this PR adds. The list is now **derived from `gate.sh`'s own `run_stage` lines**, so the omission cannot recur. ## Verification - `ci/tests/approval-override.test.sh` — **23 assertions, all passing**: bare directive, mid-sentence and `>`-quoted directives, stale and equal-timestamp overrides, unparseable timestamps, missing login, invalid/non-list JSON, latest-wins, RFC3339 **offset** timestamps (which is how Gitea actually emits them — a `Z`-only parser would have dropped every real override), configurable directive. - `make -C ci test` in the pinned toolbox: **`all suites passed`, rc 0** (was rc 2). - Toolbox merge gate: **rc 3 `UNDETERMINED`** — spec-immutability, decisions-log, lint, l0, render-golden (incl. the new `--selftest`), l1, approval-policy all green; `gitea-settings` skipped (no local `SETTINGS_API_TOKEN`), `conformance` skipped (no cluster). **Not a green run, and not reported as one.** - API shapes verified before being depended on: `/git/commits/{sha}` → `.commit.committer.date`; PR conversation comments live under `/issues/{n}/comments` (`/pulls/{n}/comments` returns diff comments only and would never have seen an override). ## ⬜️ What is left: post the override as a **REVIEW**, not a comment `required_approvals: 0` is live and `gitea-settings` is **green** (run 206). The one remaining blocker is this PR's own `approval-policy` stage, and the first attempt at clearing it found a real defect — now fixed in this PR. **What went wrong on the first try.** A well-formed `/approve-override` posted in the PR comment box, 29 minutes newer than the head commit, was **invisible to CI**. Gitea's own words, from run 206: ``` token does not have at least one of required scope(s), required=[read:issue], token scope=read:admin,read:repository ``` `SETTINGS_API_TOKEN` is minted `[read:repository, read:admin]`, and `/issues/{n}/comments` sits behind the *issue* scope — so Gitea returned an error **object**, `jq -e .` accepted it as valid JSON, and the stage fell through to "no approving review". A permission failure wearing the costume of a missing override. `/pulls/{n}/files` and `/pulls/{n}/reviews` are under `read:repository`, which is why every other part of the same stage worked. **Three fixes, the first mattering most:** the diagnosis no longer lies (it prints Gitea's `.message` and names the scope); an override is accepted from a **PR review body** as well as a comment, so the hatch is not hostage to one API scope; and `set-ci-token.sh` now requests `read:issue` so the comment box works after a re-run. ### To land this PR 1. **Files → Review → "Comment"** on this PR (not the plain comment box at the bottom), with the body starting: ``` /approve-override <reason> ``` It must be **newer than head commit `b52c8d1`** (pushed 2026-10-02T05:50:20Z), so post it now — earlier overrides are stale by design. 2. **Re-run the gate** — from the Actions UI ("Re-run all jobs"), or by closing and reopening the PR. Posting the override does **not** re-trigger it: `gate.yml` has no `issue_comment` trigger, and that was left alone on purpose (a comment trigger would check out PR-head code while the job holds `SETTINGS_API_TOKEN` — the classic pwn-request shape). **Do not push an empty commit to re-trigger**, that moves the head and makes the override stale again. 3. `approval-policy` should report `OVERRIDDEN by jakman85 (permission owner, via PR review, self-waived as author)` with your reason. **Merge.** 4. Optional, afterwards: re-run `./infra/gitea/set-ci-token.sh` to pick up `read:issue`, and the ordinary comment box works next time. Token scopes cannot be widened in place. **Still not verified:** the *accepting* branch of the override (directive found → live permission lookup → exit 0) has never executed — step 1 is its first real test, and a third round would not be a surprise. I also did not confirm Gitea lets a PR author submit a `COMMENT`-type review (expected: it blocks self-*approval* only), and I deliberately did not test it by posting an override myself — doing that as `gitea-admin` is exactly the agent self-override `CLAUDE.md` now forbids. Full gap list in `decisions/2026-10-01-admin-approval-override.md`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
gitea-admin added 1 commit 2026-10-02 04:56:57 +00:00
fix(ci): let an administrator merge their own green PR
gate / gate (pull_request) Failing after 1m44s
gate / conformance (pull_request) Has been skipped
7b9f733c97
`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]>
gitea-admin force-pushed fix/admin-approval-override from 7b9f733c97 to 616a4974f3 2026-10-02 05:02:22 +00:00 Compare
Owner

/approve-override Need to ensure approval override work as expected

/approve-override Need to ensure approval override work as expected
gitea-admin force-pushed fix/admin-approval-override from 616a4974f3 to b946c475c6 2026-10-02 05:46:56 +00:00 Compare
gitea-admin force-pushed fix/admin-approval-override from b946c475c6 to b52c8d1af6 2026-10-02 05:50:23 +00:00 Compare
Owner

/approve-override Need to ensure approval override work as expected

/approve-override Need to ensure approval override work as expected
jakman85 approved these changes 2026-10-02 06:13:08 +00:00
jakman85 merged commit 5ffd0c98bc into main 2026-10-02 06:17:17 +00:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Home-Network/home-network-k3s-impl#38