From 7cc0826c2c3c8c154f79b3dc1576472c57d6d1fb Mon Sep 17 00:00:00 2001 From: sudolulo Date: Mon, 13 Jul 2026 20:28:30 +0000 Subject: [PATCH] The matrix bot would never have worked: wrong permissions, wrong forge Two bugs, both of which would have failed silently on the first scheduled run: - permissions were while the step pushes a branch and opens a PR. It would have died with a 403 and I would have had a bot that never worked. - it opened the PR on GITHUB, which is a one-way MIRROR. A PR merged there would be clobbered by the next fleet-repos mirror push from Gitea. A bot opening PRs against a mirror is a bot doing nothing, slowly. Now: contents+pull-requests write, and the PR is opened on Gitea (canonical) via its API. One long-lived PR, force-pushed in place -- a daily PR is the same mistake as a daily comment, wearing a hat. --- .github/workflows/compat.yml | 80 ++++++++++++++++++++++++++---------- tests/test_workflows.py | 32 +++++++++++++-- 2 files changed, 87 insertions(+), 25 deletions(-) diff --git a/.github/workflows/compat.yml b/.github/workflows/compat.yml index 2a93ba4..593cf66 100644 --- a/.github/workflows/compat.yml +++ b/.github/workflows/compat.yml @@ -27,7 +27,10 @@ on: - ".github/workflows/compat.yml" permissions: - contents: read + # write, because the matrix refresh pushes a branch and opens a PR. It does NOT get + # to move `main` -- that is the whole reason it is a PR. See the refresh step below. + contents: write + pull-requests: write issues: write jobs: @@ -130,19 +133,27 @@ jobs: - name: matrix run: cat /tmp/matrix.md - # Keep the README's table true — but as a PULL REQUEST, not a push to main. + # Keep the README's table true — as a PULL REQUEST, on the CANONICAL forge. # - # This used to `git push origin HEAD:main` from CI. Unattended writes to main are - # exactly what the release barrier exists to prevent; a bot that can move main can - # move it somewhere nobody looked. A PR is reviewable, and nothing lands by itself. + # Two things this gets right that the obvious version gets wrong: + # + # 1. It is a PR, not a push to main. This used to `git push origin HEAD:main` + # from CI. An unattended write to main is exactly what the release barrier + # exists to prevent — a bot that can move main can move it somewhere nobody + # looked. Nothing lands by itself. + # + # 2. It runs on GITEA, not GitHub. GitHub is a one-way MIRROR: a PR merged there + # would be silently clobbered by the next `fleet-repos mirror` push from Gitea. + # A bot opening PRs against a mirror is a bot doing nothing, slowly. # # A stale support matrix is not a stale doc — it is a false promise to somebody - # deciding whether to trust this with their backups. So it still gets refreshed - # daily; it just asks first. - - name: refresh the README matrix (as a PR) - if: ${{ github.event_name == 'schedule' && contains(github.server_url, 'github.com') }} + # deciding whether to trust this with their backups. So it is refreshed daily; it + # just asks first. + - name: refresh the README matrix (PR on the canonical forge) + if: ${{ github.event_name == 'schedule' && !contains(github.server_url, 'github.com') }} env: - GH_TOKEN: ${{ github.token }} + TOKEN: ${{ secrets.GITEA_TOKEN || github.token }} + API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }} run: | python3 - <<'PY' import json, sys @@ -153,25 +164,52 @@ jobs: print("changed" if compat.update_readme(rows) else "unchanged") PY - git diff --quiet -- README.md && { echo "matrix unchanged"; exit 0; } + git diff --quiet -- README.md && { echo "matrix unchanged — nothing to propose"; exit 0; } git config user.name "truecloud-patch bot" git config user.email "bot@onetick.ninja" - branch="bot/compat-matrix" - git checkout -B "$branch" + BRANCH=bot/compat-matrix + git checkout -B "$BRANCH" git add README.md git commit -m "docs: refresh the TrueNAS compatibility matrix" - git push -f origin "$branch" + git push -f origin "$BRANCH" - # One long-lived PR, updated in place — not a new one every morning. - if ! gh pr view "$branch" --json number >/dev/null 2>&1; then - gh pr create --head "$branch" --base main \ - --title "docs: refresh the TrueNAS compatibility matrix" \ - --body "The daily compatibility check found that the support matrix in the README no longer matches what iXsystems' middleware actually looks like. + # ONE long-lived PR, force-pushed in place — not a new one every morning. + # (A daily PR is the same mistake as a daily comment, wearing a hat.) + BRANCH="$BRANCH" python3 - <<'PY' + import json, os, urllib.error, urllib.request - This PR only touches the block between the \`COMPAT MATRIX\` markers. It is regenerated by \`tools/compat.py --matrix --update-readme\` and force-pushed, so it always reflects the latest run." - fi + api, token, branch = os.environ["API"], os.environ["TOKEN"], os.environ["BRANCH"] + h = {"Authorization": f"token {token}", "Content-Type": "application/json"} + + def call(url, method="GET", data=None): + r = urllib.request.Request( + url, method=method, headers=h, + data=json.dumps(data).encode() if data else None) + with urllib.request.urlopen(r) as resp: # noqa: S310 + return json.load(resp) if resp.length != 0 else {} + + existing = [ + p for p in call(f"{api}/pulls?state=open") + if p["head"]["ref"] == branch + ] + if existing: + print(f"PR #{existing[0]['number']} already open; the force-push updated it") + else: + pr = call(f"{api}/pulls", "POST", { + "head": branch, "base": "main", + "title": "docs: refresh the TrueNAS compatibility matrix", + "body": ( + "The daily compatibility check found that the support matrix in " + "the README no longer matches iXsystems' actual middleware.\n\n" + "This only touches the block between the `COMPAT MATRIX` markers. " + "It is regenerated by `tools/compat.py --matrix --update-readme` " + "and force-pushed, so it always reflects the latest run." + ), + }) + print(f"opened PR #{pr['number']}") + PY # ONE bug report, kept in sync. It is edited in place when the findings change and # says NOTHING when they do not. diff --git a/tests/test_workflows.py b/tests/test_workflows.py index e760798..e617213 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -127,12 +127,36 @@ class TestTheBotDoesNotSpam: # Checked against CODE, not comments — the step's own commentary explains what # it replaced, and that mention must not read as the thing itself. with open(os.path.join(WORKFLOWS, "compat.yml"), encoding="utf-8") as fh: - code = "\n".join( - ln for ln in fh.read().splitlines() if not ln.lstrip().startswith("#") - ) - assert "gh pr create" in code + src = fh.read() + code = "\n".join( + ln for ln in src.splitlines() if not ln.lstrip().startswith("#") + ) + assert "/pulls" in code, "the matrix refresh must open a PR" assert "HEAD:main" not in code, "CI still pushes straight to main" + def test_the_matrix_PR_targets_the_CANONICAL_forge_not_the_mirror(self): + # GitHub is a one-way mirror: a PR merged there would be silently clobbered by + # the next `fleet-repos mirror` push from Gitea. A bot opening PRs against a + # mirror is a bot doing nothing, slowly. + with open(os.path.join(WORKFLOWS, "compat.yml"), encoding="utf-8") as fh: + src = fh.read() + i = src.index("refresh the README matrix") + step = src[i:i + 400] + assert "!contains(github.server_url, 'github.com')" in step, ( + "the matrix PR must be opened on Gitea (canonical), not GitHub (mirror)" + ) + + def test_the_workflow_has_the_permissions_its_steps_actually_need(self): + # It shipped with `contents: read` while the step pushed a branch and opened a + # PR — it would have died with a 403 on the first scheduled run, and I would + # have had a bot that silently never worked. + with open(os.path.join(WORKFLOWS, "compat.yml"), encoding="utf-8") as fh: + src = fh.read() + perms = src[src.index("permissions:"):src.index("jobs:")] + assert "contents: write" in perms, "pushing a branch needs contents: write" + assert "pull-requests: write" in perms, "opening a PR needs pull-requests: write" + assert "issues: write" in perms + class TestCompatCannotSilentlyPass: def test_the_exit_code_is_captured_not_swallowed(self):