merge-authority cannot be a required check yet: the tool cannot post one, and nothing runs it nf-0od ← Beads

open priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/712 by tools/beads/import_github.py's reconcile pass.

Filed unclaimed. Found while answering @aedanpope's question *"how do I add the check? what do I search for?"* about #704, at 75bfe77. merge-authority cannot become a required check yet, for two reasons neither of which is in #704's setup list — and #704's list also names a prerequisite that is already satisfied.

1. The tool cannot post a check run at all, as documented

``` $ ./tools/pr/perimeter_check.py 709 #709 ERROR could not post check: command failed (exit 1): gh api repos/cfsh/ninefold/check-runs -X POST --input - gh: You must authenticate via a GitHub App. (HTTP 403) ```

--dry-run on the same PR prints #709 SUCCESS Approved by aedanpope, so the *decision* is right — it is only publication that fails.

Cause. main() builds a bare Gh(os.environ.get("REPO", "cfsh/ninefold")) (tools/pr/perimeter_check.py:182), which shells out to gh under whatever ambient auth exists. On every box that is the jcantsp-bot user token. The check-runs endpoint is one of the few GitHub refuses to a user token *by kind* — POST /repos/{owner}/{repo}/check-runs accepts only a GitHub App installation token. No scope grant on the user token can fix it; it is not a permissions gap.

nflib.app_token.mint() already exists and already returns exactly the right token — it is simply never called here. Confirmed by passing one in by hand:

``` $ GH_TOKEN=$(python3 -c 'import sys;sys.path.insert(0,"tools");import nflib;from nflib import app_token;print(app_token.mint())') \ ./tools/pr/perimeter_check.py 709 #709 SUCCESS Approved by aedanpope — human approval (aedanpope)

$ gh api repos/cfsh/ninefold/commits/cb85533/check-runs -q '[.check_runs[]|{name,conclusion,app:.app.slug}]' [{"app":"ninefold-reviewer","conclusion":"success","name":"merge-authority"}] ```

So the fix is small — mint inside main(), or take a documented GH_TOKEN — but the *test suite cannot currently tell*: test_perimeter_check.py drives a fake Gh, so the one thing that is broken is the one thing no test exercises. Whatever lands should pin the auth kind, not just the call.

⚠️ This is why #704's "run it for a week and compare its calls against yours" adoption plan has produced nothing. The tool looks like it works — --dry-run is green, --all exits 0 by design — and the sweep prints one ERROR line per PR that nobody is reading, because nothing is running it. Which is the second problem.

2. Nothing invokes it

grep -rn perimeter_check --include='*.py' --include='*.md' --include='*.sh' . returns only the file itself. It is in no cron, no dispatcher tick, no merge_pr.py path, and is not mentioned in notes/merging.md or CLAUDE.md. The module docstring says *"This runs from the dispatcher tick"* (perimeter_check.py:148) — it does not; that is a plan written in the present tense, the class of claim #542/#450 exist for.

⚠️ This is the fail-closed hazard #704 itself names, arriving through the back door. #704 warns that a required check which never posts blocks every PR forever, and answers it with @aedanpope as a bypass actor on ruleset 2. But that warning is about the dispatcher going *stale*; here the check has never posted once — measured, zero merge-authority check runs existed on this repo before the manual one above. Making it required today would freeze the queue immediately, and the bypass actor would be the only path to any merge at all.

Required order, then: (1) fix the auth, (2) wire a caller on head-change, (3) watch it post green/red for a week, (4) *then* create ruleset 2.

3. checks: write is already granted — #704's step 1 is stale

#704 says the App *"has Pull requests R/W + Contents R today"*. Measured against GET /app/installations with the App JWT:

```json {"checks": "write", "issues": "write", "contents": "read", "metadata": "read", "pull_requests": "write"} ```

checks: write is present. Whoever sets up ruleset 2 should not go looking for a permission grant that has already happened — and the 403 above will *not* be fixed by re-granting it, which is the wrong-diagnosis trap this note exists to close.

Suggested direction (not claimed)
  1. perimeter_check.py mints its own App token, and refuses loudly rather than posting ERROR per-PR when it cannot — a sweep that can authenticate for nothing is a host problem, not 26 PR problems (the same distinction #662 drew for the probe wipeout).
  2. A test that pins the token kind, since the fake Gh cannot see this class of failure.
  3. A caller. On head change is the natural trigger; the dispatcher already wakes on exactly that.
  4. notes/merging.md gains the ruleset-2 recipe once (1)–(3) hold, so the setup is not reconstructed from a merged PR's comments.

Related: #704 (the check), #633 (the perimeter it publishes), #586/#702 (review cadence — same dispatcher), #542 #450 #481 (prose asserting what nobody measured).

Dependencies

None.

Comments

No comments.

Add a comment