fix(ci): accept the override from a PR review, and stop misdiagnosing an unreadable comments API
gate / gate (pull_request) Successful in 1m46s
gate / conformance (pull_request) Has been skipped

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]>
This commit is contained in:
jakman85andClaude Opus 5 committed 2026-10-02 00:50:20 -05:00
1 parent 8ccfdab180
commit b52c8d1af6
8 files changed
+357 -84

No files matched your search

+86 -15
View File
@@ -87,6 +87,43 @@ Also updated: `CLAUDE.md` §1 and the §2 enforcement table (the "approval requi
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.
**Second pass, 2026-10-02 — the override did not work the first time, and why.** The operator ran
the live PATCH (`required_approvals: 0` confirmed live, with `block_admin_merge_override` and
`enable_status_check` still `true`), merged PR #35, and posted `/approve-override Need to ensure
approval override work as expected` on PR #38. The gate re-ran (run 203) and **still blocked.**
The comment was not at fault: posted by `jakman85`, directive at the start of the line, reason
present, and 29 minutes **newer** than head commit `616a497`, so it cleared the staleness rule.
What failed was that **CI could not read it.** Run 203's log:
approval-override: COMMENTS_JSON is not a list
`SETTINGS_API_TOKEN` is minted `["read:repository", "read:admin"]` by
`infra/gitea/set-ci-token.sh`, and Gitea puts `/issues/{n}/comments` 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 exactly why every
other part of the same stage worked and this one did not.
Three fixes, and the first is the one that mattered most:
1. **The diagnosis is no longer misleading.** `COMMENTS_JSON` is out of the must-parse-or-block
loop and is explicitly 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 the override instead. An
unreadable source can only cause a false BLOCK, never a false pass, so degrading it to a loud
warning is safe — and blocking on it was never the honest behaviour.
2. **The 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 draft) reviews are dropped.
3. **`set-ci-token.sh` now requests `read:issue`**, which makes the ordinary comment box work —
the thing anybody reaches for first. It takes effect only on a **re-run**: an existing token's
scopes cannot be widened in place.
`ci/tests/approval-override.test.sh` grows from 23 to **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 actually quotes Gitea's message.
**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`
@@ -130,8 +167,30 @@ missing from its test. `make -C ci test` goes from **FAILED to all suites passed
**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,
**38**, 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.
- **The live flip worked, and the drift detector proved it.** Run 202 (before): `gitea-settings:
FAIL`, `required_approvals: DECLARED 0 but LIVE is 1`. Run 203 (after the operator ran the
PATCH): `gitea-settings: pass`, with `block_admin_merge_override = true` among the keys it
checked. So the narrow change held — the approval count moved and the gate's teeth did not.
- **The first override attempt failed for a measured reason, not a guessed one.** Run 203 log:
`approval-override: COMMENTS_JSON is not a list`. Token scopes read from `set-ci-token.sh`:
`[read:repository, read:admin]`. The same run successfully read `/pulls/38` and
`/pulls/38/files` (author and sensitive paths both resolved) and got past the reviews
validation, which is how I know `/pulls/{n}/reviews` IS readable with that token — from
behaviour, not from documentation.
- **The comment itself was valid.** `created_at` 2026-10-02T05:31:24Z vs head commit `616a497`
committer date 2026-10-02T00:02:20-05:00 → the comment is 29m04s newer, so it passed the
staleness rule. Ruling the comment out is what left the token scope as the only candidate.
- **`?limit=1000` is not the culprit.** Queried `/issues/38/comments` with `limit=1000`, `limit=50`
and no limit using my own credentials: `type=array` in all three. So the difference is the
credential, not the query.
- **The root cause is confirmed by Gitea, in its own words.** Run 206 (the fix in place):
`[warn] cannot read PR COMMENTS from Gitea — token does not have at least one of required
scope(s), required=[read:issue], token scope=read:admin,read:repository.` followed by the
instruction to post the override as a review. That is the diagnosis the first attempt should
have produced, and `gitea-settings: pass` in the same run. The stage still ends `FAIL`, correctly
— there is no valid override on the PR yet.
- **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
@@ -142,20 +201,32 @@ missing from its test. `make -C ci test` goes from **FAILED to all suites passed
## 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**. The declaration and the live rule must move together.
*Now measured, not predicted:* PR #38's CI run (run 202, job 0) fails at **1m44s** with
`required_approvals: DECLARED 0 but LIVE is 1` → `gitea-settings: FAIL`. Correct behaviour, and
the PR stays red until the script is run.
- **The override has never been ACCEPTED by a real run** — only offered. Measured in the same CI
log: the stage reached `approval-policy: FAIL` on the sensitive paths and printed the *new*
code's guidance, `Or, if you hold admin/owner on this repository, comment on the PR:
/approve-override <reason> ... NEWER than head commit 7b9f733c`. So the new stage **is** what
runs on this PR (self-bootstrapping confirmed by measurement, not by reasoning), and the
authorising branch — comment parse → live permission lookup → exit 0 — is still **unexercised**.
Posting the comment on PR #38 is its first real test. If it does not work, the recovery is to
PATCH `required_approvals` and read run 202's successor log.
- **The override has still never been ACCEPTED by a real run.** This is the second attempt at
this bullet, which is itself the point: the first pass called the hatch unverified, and the
first real test found a defect that no amount of local testing would have (an API scope CI has
and I did not). What is now measured is that the parse → *reject* path works end to end and
diagnoses itself. The **accepting** branch — directive found → live permission lookup → exit 0 —
has never executed. Posting the override as a **review** on PR #38 is its first real test, and
I would not be surprised by a third round.
- **`read:issue` is the right scope name — Gitea said so itself, so this is no longer a guess.**
Run 206 printed Gitea's own message: `token does not have at least one of required scope(s),
required=[read:issue], token scope=read:admin,read:repository`. That both confirms the root
cause and names the string, which matters because this repo has been burned by a token *name*
that could not exist (`GITEA_SETTINGS_TOKEN`). What is **still** unverified is that the token
*mint* accepts it — `POST /users/{u}/tokens` with that scope has not been run. The review route
does not depend on it.
- **I did not verify that Gitea lets a PR's own author submit a `COMMENT`-type review.** Gitea
blocks author self-*approval*; "Comment" is expected to be allowed, and that expectation is now
load-bearing for the recommended route. It is one click to find out, and I deliberately did not
test it by posting an override myself: doing so as `gitea-admin` is precisely the agent
self-override that `CLAUDE.md` now forbids.
- **The whole re-run mechanism is manual.** `gate.yml` triggers on `pull_request` and `push: main`
only — there is no `issue_comment` trigger, so posting an override does **not** re-run the gate.
Re-run the job from the Actions UI, or close and reopen the PR (which fires `pull_request` and
leaves the head SHA alone). **Do not push an empty commit to re-trigger it:** that moves the head
commit and makes the override stale by the rule above, so it would have to be posted again. A
comment trigger was considered and not built — it would need to check out PR head code while the
job holds `SETTINGS_API_TOKEN`, which is the classic pwn-request shape and not worth it here.
- **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