d9571fc6d8732cd64d33c2babebfaa0d8abe5ded
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  jakman85andClaude Opus 5
|
b52c8d1af6 |
fix(ci): accept the override from a PR review, and stop misdiagnosing an unreadable comments API
The override did not work on its first real test, and the reason was invisible from the outside. Run 203 on PR #38: a well-formed `/approve-override` posted by jakman85 in the PR comment box, 29 minutes newer than the head commit, and the stage still blocked with: approval-override: COMMENTS_JSON is not a list SETTINGS_API_TOKEN is minted [read:repository, read:admin], and Gitea puts /issues/{n}/comments behind the *issue* scope — so Gitea answered with 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. 1. The diagnosis no longer lies. COMMENTS_JSON leaves the must-parse-or-block loop and is checked for being an ARRAY; when it is not, the stage prints Gitea's own .message, names the missing scope and says where to post instead. An unreadable source can only cause a false BLOCK, never a false pass, so a loud warning is the honest behaviour and blocking was not. 2. An override is accepted from a PR REVIEW body as well as a comment, so the hatch is not hostage to one API scope — a review of type 'Comment' works on the token CI has today. The gate names which source it used; PENDING (unsubmitted, author-only) reviews are dropped. 3. set-ci-token.sh requests read:issue, making the ordinary comment box work — on a RE-RUN only, since an existing token's scopes cannot be widened. ci/tests/approval-override.test.sh: 23 -> 38 assertions, covering the reviews source, PENDING reviews, an error object from the comments API, 'an unreadable comments source must not suppress a valid review override', latest-wins across both sources, and that the diagnostic quotes Gitea's message. Also measured and recorded: run 203 shows gitea-settings PASS after the operator ran the live PATCH, with block_admin_merge_override still true — the narrow change held, the approval count moved and the gate's teeth did not. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> |
||
|
jakman85andClaude Opus 5
|
8ccfdab180 |
fix(ci): let an administrator merge their own green PR
`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 @
|
||
|
jakman85andClaude Opus 5
|
0c6949ed98 |
feat(ci): the pipeline is the approver; humans approve changes to the pipeline
Implements the operator intent: "CI/CD pipeline is the end-all-be-all for approval. If the CI pipeline is not good enough to be trusted for this, then it is not good enough to do its intended job and needs to be improved. Manual approvers should only be for architecturally or security relevant changes." Adds ci/approval-policy.sh as a gate stage. A green gate becomes sufficient for an ordinary change; the one exception is structural rather than a matter of taste: A CHANGE TO THE GATE CANNOT BE GATED BY THE GATE. A PR that weakens a check would be approved by the weakened check. The same holds for the admission policies, the Flux wiring that decides what reconciles, the SOPS recipients and the RBAC. Those paths -- and only those -- still require an APPROVED review from someone other than the author. The list is data with a reason per entry (infra/gitea/approval-policy.json) and is itself a sensitive path, so the policy cannot be widened or narrowed without a human. FAIL-CLOSED, and deliberately the opposite of gitea-settings. There, a missing credential must not block every PR -- that is a lock-out. Here, a missing credential would let an unreviewed change to the gate merge, so "cannot evaluate" is a BLOCK. Verified against real PRs rather than fixtures: PR #16 (sensitive paths, approved by jakman85, author gitea-admin) -> PASS PR #6 (.gitea/workflows/gate.yml, ZERO reviews) -> BLOCK, naming the file push event (not a PR) -> N/A, exit 0 bogus token -> BLOCK, not skip Also caught before shipping: the first draft passed the PR file list with a second stdin redirect after a heredoc, which silently wins and leaves the script body unread. Running it found that; reading it would not have. required_approvals is NOT changed here. Flipping it live must happen together with the declaration in expected-branch-protection.json, or gitea-settings correctly fails on the drift -- that is a separate, deliberate change. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> |