perimeter_check.py cannot post a check run: check-runs require a GitHub App token and the tool never mints one nf-w7zu ← Beads

closed priority 2 task unassigned

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

Unclaimed / unlabelled — inert backlog. Found by nf-dev-argo answering @aedanpope's question *"How do I specifically add the check condition? What do I search for"* — the answer turned out to be blocked one step earlier.

Measured

``` $ echo '{"name":"…","head_sha":"cb90a55…","status":"completed","conclusion":"neutral", …}' \ | gh api -X POST repos/cfsh/ninefold/check-runs --input - {"message":"You must authenticate via a GitHub App.", "status":"403"} ```

POST /repos/{owner}/{repo}/check-runs cannot be called by a user token at all — not jcantsp-bot's, not a PAT. Only a GitHub App installation token works. That is an endpoint-level rule, not a permission that checks: write on a user grants.

The gap

tools/pr/perimeter_check.py (#704) calls gh.api_json(...) through the ordinary Gh, which uses whatever gh is authenticated as. It contains zero references to app_token or GH_TOKEN:

``` $ grep -c "app_token\|GH_TOKEN" tools/pr/perimeter_check.py 0 ```

Its own docstring says *"This runs from the reviewer dispatcher"*, which is the right home — dispatch.py mints per-invocation tokens via nflib.app_token.mint() (dispatch.py:691,721). But nothing in the tool requires that, and run any other way it fails with:

``` #704 ERROR could not post check: … 403 ```

which names neither the cause nor the fix.

⚠️ --dry-run is unaffected, which is why this was not caught: every verification of #704 used --dry-run against the live queue, and that path never posts. A tool whose only untested path is the one that mutates GitHub is the shape worth naming.

Suggested fix
  1. Mint the token, like dispatch.py doesfrom nflib.app_token import mint, set it on the Gh the poster uses, or accept one via GH_TOKEN and refuse early with a readable message when it is absent or is not an App token.
  2. Refuse early rather than per-PR. With --all the current shape prints one opaque ERROR line per open PR — eight identical 403s that all mean "no App credential here".
  3. Say which box can do it. The reviewer PEM is machine-local (reviewer/README.md: *"A box with no reviewer credential — every coder clone, Aedan's…"*), so most clones cannot post and should say so instead of erroring eight times.
⚠️ Consequence for the ruleset ordering

A required check that has never been posted cannot be selected in the ruleset UI — GitHub only offers checks it has seen recently, and this repo has 0 check runs on every recent commit (verified across the last 5 on main). So the adoption order is forced:

  1. grant checks: write on the App and accept the permission change on the installation;
  2. fix this, and post at least one run from a box holding the PEM;
  3. *then* add the required_status_checks rule naming merge-authority.

Doing (3) first is not possible, and doing it via the API with a name GitHub has never seen would create a rule that no check can satisfy — which blocks every PR, the exact failure #704's docstring warns about.

Refs #704, #633.

Dependencies

None.

Comments

No comments.

Add a comment