From 64650c61a17f241e6af9069672e138d09b8440ee Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Mon, 18 May 2026 22:25:11 -0400 Subject: [PATCH 1/9] =?UTF-8?q?docs(ci):=20clarify=20PAT=20needs=20Workflo?= =?UTF-8?q?ws:write=20=E2=80=94=20distributor=20writes=20.github/workflows?= =?UTF-8?q?/*?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First run of the digest workflow returned 403 "Resource not accessible by personal access token" on every fork for both the merge-upstream call and the distributor's file write. Root cause: fine-grained PATs separate Contents permission from Workflows permission, and writing files under .github/workflows/ requires the latter. Updates the docstring on the workflow file and the README so the PAT permissions list matches what's actually required to run end-to-end. --- .github/scripts/README.md | 8 ++++++-- .github/workflows/fork-sync-and-digest.yml | 10 +++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/.github/scripts/README.md b/.github/scripts/README.md index c207933..80dba63 100644 --- a/.github/scripts/README.md +++ b/.github/scripts/README.md @@ -56,8 +56,12 @@ Edit `.github/forks.yml`. Each entry: Set at the org or `ledoent/.github` repo level: - `LEDOENT_FORK_SYNC_TOKEN` — fine-grained PAT, scopes: - - `Contents: write` on `ledoent/*` - - `Metadata: read` + - `Contents: Read and write` on `ledoent/*` (merge-upstream + file writes) + - `Workflows: Read and write` on `ledoent/*` — **required** because the + distributor writes `.github/workflows/forward-port.yml`; without this + every distributor PUT returns `403 Resource not accessible by personal + access token` + - `Metadata: Read-only` (auto-selected when picking a repo) - `public_repo` (for searching OCA upstream PRs) - `SMTP_SERVER` / `SMTP_USERNAME` / `SMTP_PASSWORD` / `SMTP_TO` / `SMTP_FROM` - `SMTP_USERNAME`: SES SMTP-credential AKID diff --git a/.github/workflows/fork-sync-and-digest.yml b/.github/workflows/fork-sync-and-digest.yml index 05e0d73..e6983fd 100644 --- a/.github/workflows/fork-sync-and-digest.yml +++ b/.github/workflows/fork-sync-and-digest.yml @@ -16,9 +16,13 @@ name: fork-sync-and-digest # doesn't require touching this workflow or the scripts. # # Secrets required (org-level or repo-level on ledoent/.github): -# LEDOENT_FORK_SYNC_TOKEN PAT scoped to ledoent/*, with -# Contents:write (sync + distributor), -# Metadata:read, public_repo (upstream search) +# LEDOENT_FORK_SYNC_TOKEN Fine-grained PAT scoped to ledoent/*, with +# Contents: Read and write (merge-upstream +# sync + distributor file writes), +# Workflows: Read and write (REQUIRED — the +# distributor writes .github/workflows/*), +# Metadata: Read-only (auto), +# public_repo (upstream OCA search). # SMTP_SERVER, SMTP_USERNAME, SMTP_PASSWORD, SMTP_TO, SMTP_FROM # email delivery. Recommended: AWS SES (the # ledoweb.com domain is already verified there). From 481a7ed84a8d3259ef10936091a121039b490d32 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Tue, 19 May 2026 21:51:28 -0400 Subject: [PATCH 2/9] fix(digest): treat 404 "Branch not found" as skipped, not failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit merge-upstream returns two API shapes for the same "branch doesn't exist anywhere yet" condition: 422 + "does not exist" — branch isn't on the fork 404 + "Branch not found" — branch isn't on the upstream either Previously only 422 was treated as skipped, so forks that don't yet have a 19.0 branch upstream (ddmrp, report-print-send) showed up in the failure bucket and made the daily subject line noisy. After this fix, both cases bucket into "skipped (branch n/a)" and the "Sync failures" header only appears when there's a real failure to look at. --- .github/scripts/fork_sync_digest.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/scripts/fork_sync_digest.py b/.github/scripts/fork_sync_digest.py index 4d5abc1..1d9778f 100644 --- a/.github/scripts/fork_sync_digest.py +++ b/.github/scripts/fork_sync_digest.py @@ -105,15 +105,23 @@ def load_forks() -> list[dict]: def sync_branch(repo: str, branch: str) -> dict: status, body = gh("POST", f"/repos/{repo}/merge-upstream", {"branch": branch}) + msg = body.get("message", "") + # "Branch n/a" responses we expect on forks that haven't cut a 20.0 + # (or any future release) yet. Two API shapes for the same condition: + # 422 + "does not exist" — branch isn't on the fork at all + # 404 + "Branch not found" — branch isn't on the upstream either + # Both are "skipped", not "failed". + skipped = ( + (status == 422 and "does not exist" in msg.lower()) + or (status == 404 and "branch not found" in msg.lower()) + ) return { "repo": repo, "branch": branch, "status": status, - "message": body.get("message", ""), + "message": msg, "merge_type": body.get("merge_type"), - # 422 with "branch does not exist" is the no-op we expect on - # forks that haven't cut a 19.0 yet — treat it as "skipped". - "skipped": status == 422 and "does not exist" in body.get("message", "").lower(), + "skipped": skipped, } From b21d1372f94925013b0442f4cc1fd5cb23795172 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Tue, 19 May 2026 22:05:36 -0400 Subject: [PATCH 3/9] sec: pin 3rd-party actions to commit SHAs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mutable tags like @v3/@v4 mean a maintainer (or attacker) can swap the underlying code without our consent — the workflow runs whatever that ref points to, with our PAT in env. Pin to commit SHA so updates become deliberate review events. Same pin treatment for forward-port.yml template (every fork's CI is running that file with write-scoped GITHUB_TOKEN). Also documents WHY the template uses `pull_request_target` rather than `pull_request`: the action needs to write back after merge, which requires the elevated permission set; PR-author code never executes in the job, so the usual security concern doesn't apply. Comment lets the next reader see the reasoning instead of guessing. Comments next to each pin show the version tag the SHA currently maps to, so the next bump can be a one-line review against the upstream changelog. --- .github/templates/forward-port.yml | 17 +++++++++++++++-- .github/workflows/fork-sync-and-digest.yml | 10 +++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/.github/templates/forward-port.yml b/.github/templates/forward-port.yml index 149bb49..bc9ada1 100644 --- a/.github/templates/forward-port.yml +++ b/.github/templates/forward-port.yml @@ -17,6 +17,19 @@ name: forward-port +# `pull_request_target` (not `pull_request`) is deliberate: the action +# needs to push a new branch + open a PR after merge, which requires +# write-scoped GITHUB_TOKEN and access to repository secrets. The usual +# `pull_request_target` security concern — running PR-controlled code +# with elevated permissions — does NOT apply here: we never check out +# the PR head SHA, only the base ref via actions/checkout, and the +# backport action operates on the already-merged commit on the base +# branch. No PR-author-controlled code executes in this job. +# +# Action pins: SHA pins prevent a tag retag from swapping in code that +# runs with our PAT-equivalent permissions. Comment shows the version +# each SHA currently maps to; bump deliberately when updating. + on: pull_request_target: types: [closed] @@ -30,11 +43,11 @@ jobs: if: github.event.pull_request.merged == true runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: fetch-depth: 0 - name: Cherry-pick to labelled branches - uses: korthout/backport-action@v3 + uses: korthout/backport-action@d07416681cab29bf2661702f925f020aaa962997 # v3 with: # Label format: `port:19.0` → forward-port to branch 19.0. # The action calls these "backport" but mechanically it's diff --git a/.github/workflows/fork-sync-and-digest.yml b/.github/workflows/fork-sync-and-digest.yml index e6983fd..39cf02f 100644 --- a/.github/workflows/fork-sync-and-digest.yml +++ b/.github/workflows/fork-sync-and-digest.yml @@ -48,7 +48,11 @@ jobs: timeout-minutes: 15 steps: - - uses: actions/checkout@v4 + # Third-party Actions pinned to commit SHAs so a tag retag (or a + # compromised maintainer) can't silently swap the code our PAT + # runs against. Comment shows the human-readable version each SHA + # currently maps to; bump both when reviewing for updates. + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - name: Sync forks + assemble digest env: @@ -69,7 +73,7 @@ jobs: run: echo "value=$(cat digest.subject)" >> "$GITHUB_OUTPUT" - name: Send digest email - uses: dawidd6/action-send-mail@v3 + uses: dawidd6/action-send-mail@4226df7daafa6fc901a43789c49bf7ab309066e7 # v3 with: server_address: ${{ secrets.SMTP_SERVER }} server_port: 587 @@ -88,7 +92,7 @@ jobs: - name: Upload digest artifact (kept 7 days for debugging) if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: digest-${{ github.run_id }} path: | From 7901840e1de09f813ed3ec2456cf8883aba5e33c Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Tue, 19 May 2026 22:06:01 -0400 Subject: [PATCH 4/9] ci: add concurrency lock + heredoc \$GITHUB_OUTPUT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small robustness fixes: 1. Concurrency group on the workflow name so the daily cron + a manual dispatch can't race and double-call merge-upstream on the same branches. cancel-in-progress: false so a manual test fired during a cron run queues + completes against the synced state instead of leaving forks half-touched. 2. Subject line is now written to \$GITHUB_OUTPUT via heredoc. The old `echo "value=\$(cat …)" >> \$GITHUB_OUTPUT` form silently corrupts the variable if the value contains `=`, backticks, or a newline. Subjects don't currently include those, but a future render tweak adding `[12 sync fail = 4 forks]` would land as a hard-to-trace email-step failure. --- .github/workflows/fork-sync-and-digest.yml | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/.github/workflows/fork-sync-and-digest.yml b/.github/workflows/fork-sync-and-digest.yml index 39cf02f..812b3d8 100644 --- a/.github/workflows/fork-sync-and-digest.yml +++ b/.github/workflows/fork-sync-and-digest.yml @@ -42,6 +42,14 @@ on: permissions: contents: read +# Serialize runs. The daily cron + a manual dispatch could otherwise +# race and double-bill the same merge-upstream call. Queue (not cancel) +# so a manual test triggered during a cron run completes against the +# already-synced state. +concurrency: + group: ${{ github.workflow }} + cancel-in-progress: false + jobs: sync-and-digest: runs-on: ubuntu-latest @@ -70,7 +78,17 @@ jobs: - name: Read digest subject id: subject - run: echo "value=$(cat digest.subject)" >> "$GITHUB_OUTPUT" + # Heredoc form because `key=value` syntax breaks if the value + # contains `=`, backticks, or a newline. Subject lines are + # short and we control the content, but defending here is one + # line of YAML versus a future debugging session. + run: | + { + echo "value<> "$GITHUB_OUTPUT" - name: Send digest email uses: dawidd6/action-send-mail@4226df7daafa6fc901a43789c49bf7ab309066e7 # v3 From 3b290692c6b103f36f748506df3bb60dcad9336c Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Tue, 19 May 2026 22:12:34 -0400 Subject: [PATCH 5/9] refactor(scripts): extract shared github HTTP helper + friendlier missing-token error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related cleanups in one commit because both touch the same lines: 1. _github.py absorbs the gh() helper that was copy-pasted across fork_sync_digest.py and distribute_forward_port.py. Same behaviour, one source of truth — and a place to add retry / rate-limit handling later without forking the change. 2. require_token() replaces the bare os.environ["GH_TOKEN"] reads. The old form raises `KeyError: 'GH_TOKEN'` with no hint about the secret name or where to set it; the new form prints the exact `env:` block needed and exits 2. Spent ten minutes debugging that the first time, so worth a one-off helper. Both scripts continue to work exactly as before — the public surface (gh(method, path, body)) is unchanged. --- .github/scripts/_github.py | 72 ++++++++++++++++++++++ .github/scripts/distribute_forward_port.py | 31 ++-------- .github/scripts/fork_sync_digest.py | 30 ++------- 3 files changed, 80 insertions(+), 53 deletions(-) create mode 100644 .github/scripts/_github.py diff --git a/.github/scripts/_github.py b/.github/scripts/_github.py new file mode 100644 index 0000000..dc98cdb --- /dev/null +++ b/.github/scripts/_github.py @@ -0,0 +1,72 @@ +"""Minimal GitHub REST helper shared by the org-control scripts. + +Kept dependency-free (stdlib only) because the workflow runs without +pip and adding a setup-python step for one HTTP call isn't worth it. +""" + +from __future__ import annotations + +import json +import os +import sys +import urllib.error +import urllib.request + + +GH = "https://api.github.com" +_USER_AGENT_DEFAULT = "ledoent-org-control/1.0" + + +def require_token(var: str = "GH_TOKEN") -> str: + """Read a token from env or exit with a useful pointer. + + Cryptic `KeyError: 'GH_TOKEN'` tracebacks waste a workflow run when + the secret name is mis-spelled. Print the actionable message and + exit non-zero instead. + """ + token = os.environ.get(var) + if not token: + sys.stderr.write( + f"error: {var} is not set in the environment.\n" + f"In the workflow, ensure the step has\n" + f" env:\n" + f" {var}: ${{{{ secrets.LEDOENT_FORK_SYNC_TOKEN }}}}\n" + f"and that the secret exists at the repo or org level.\n" + ) + sys.exit(2) + return token + + +def make_headers(token: str, user_agent: str = _USER_AGENT_DEFAULT) -> dict: + return { + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": user_agent, + } + + +def request( + method: str, + path: str, + *, + headers: dict, + body: dict | None = None, + timeout: int = 30, +) -> tuple[int, dict]: + """Single HTTP call returning (status, parsed_json_or_error_dict).""" + url = path if path.startswith("http") else f"{GH}{path}" + data = json.dumps(body).encode() if body else None + req = urllib.request.Request(url, data=data, method=method, headers=headers) + if body: + req.add_header("Content-Type", "application/json") + try: + with urllib.request.urlopen(req, timeout=timeout) as r: + payload = r.read().decode() + return r.status, (json.loads(payload) if payload else {}) + except urllib.error.HTTPError as e: + try: + payload = json.loads(e.read().decode()) + except Exception: + payload = {"message": str(e)} + return e.code, payload diff --git a/.github/scripts/distribute_forward_port.py b/.github/scripts/distribute_forward_port.py index 1286101..e06b38f 100644 --- a/.github/scripts/distribute_forward_port.py +++ b/.github/scripts/distribute_forward_port.py @@ -17,42 +17,19 @@ import base64 import json -import os import sys -import urllib.error -import urllib.request from pathlib import Path -GH = "https://api.github.com" -TOKEN = os.environ["GH_TOKEN"] -HEADERS = { - "Authorization": f"Bearer {TOKEN}", - "Accept": "application/vnd.github+json", - "X-GitHub-Api-Version": "2022-11-28", - "User-Agent": "ledoent-fp-distributor/1.0", -} +from _github import make_headers, request, require_token + +HEADERS = make_headers(require_token(), user_agent="ledoent-fp-distributor/1.0") TEMPLATE = Path(".github/templates/forward-port.yml") DEST_PATH = ".github/workflows/forward-port.yml" def gh(method: str, path: str, body: dict | None = None) -> tuple[int, dict]: - data = json.dumps(body).encode() if body else None - req = urllib.request.Request( - f"{GH}{path}", data=data, method=method, headers=HEADERS - ) - if body: - req.add_header("Content-Type", "application/json") - try: - with urllib.request.urlopen(req, timeout=30) as r: - payload = r.read().decode() - return r.status, (json.loads(payload) if payload else {}) - except urllib.error.HTTPError as e: - try: - payload = json.loads(e.read().decode()) - except Exception: - payload = {"message": str(e)} - return e.code, payload + return request(method, path, headers=HEADERS, body=body) def push_workflow(repo: str, content_bytes: bytes) -> tuple[str, str]: diff --git a/.github/scripts/fork_sync_digest.py b/.github/scripts/fork_sync_digest.py index 1d9778f..e9004a3 100644 --- a/.github/scripts/fork_sync_digest.py +++ b/.github/scripts/fork_sync_digest.py @@ -21,41 +21,19 @@ import html import json -import os import re import sys -import urllib.error import urllib.parse -import urllib.request from datetime import datetime, timedelta, timezone from pathlib import Path -GH = "https://api.github.com" -TOKEN = os.environ["GH_TOKEN"] -HEADERS = { - "Authorization": f"Bearer {TOKEN}", - "Accept": "application/vnd.github+json", - "X-GitHub-Api-Version": "2022-11-28", - "User-Agent": "ledoent-fork-digest/2.0", -} +from _github import make_headers, request, require_token + +HEADERS = make_headers(require_token(), user_agent="ledoent-fork-digest/2.0") def gh(method: str, path: str, body: dict | None = None) -> tuple[int, dict]: - url = path if path.startswith("http") else f"{GH}{path}" - data = json.dumps(body).encode() if body else None - req = urllib.request.Request(url, data=data, method=method, headers=HEADERS) - if body: - req.add_header("Content-Type", "application/json") - try: - with urllib.request.urlopen(req, timeout=30) as r: - payload = r.read().decode() - return r.status, (json.loads(payload) if payload else {}) - except urllib.error.HTTPError as e: - try: - payload = json.loads(e.read().decode()) - except Exception: - payload = {"message": str(e)} - return e.code, payload + return request(method, path, headers=HEADERS, body=body) def load_forks() -> list[dict]: From 441b2b0919f94a87d2cd13eff4f9a6029c29a15a Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Wed, 20 May 2026 07:18:07 -0400 Subject: [PATCH 6/9] feat(digest): surface distributor outcomes in the email body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Splits the pipeline into collect → distribute → render so the forward-port distributor's results land in the daily email, not just in the artifact tarball. Before: render lived inside fork_sync_digest.py. The distributor ran AFTER render, so its 14/14 failure on our first run looked like "digest OK" in the inbox. The actual failures were only visible if you downloaded the artifact and grep'd through JSON. After: 1. fork_sync_digest.py → collects sync + MIG data, writes sync-results.json + mig-buckets.json + forks-parsed.json. No render. 2. distribute_forward_port.py → writes forward-port-distribution.json (unchanged behaviour). 3. render_digest.py → reads all three JSONs, writes digest.{html,subject,exit}. The render step has `if: always()` so a crash in the distributor doesn't suppress the digest — the email still reports what synced and flags the dist failure in its own section. Subject line now tags both `⚠️ N sync fail` AND `⚠️ N dist fail` separately so the inbox view distinguishes the two failure modes at a glance. Verified locally against the live PAT: 36-entry sync, simulated 1-dist-failure → subject reads "Ledoent digest YYYY-MM-DD — ⚠️ 1 dist fail — 4 MIG" and the body contains a "Distributor failures" section with the per-fork detail. --- .github/scripts/fork_sync_digest.py | 144 +++++-------------- .github/scripts/render_digest.py | 158 +++++++++++++++++++++ .github/workflows/fork-sync-and-digest.yml | 19 ++- .gitignore | 4 + 4 files changed, 216 insertions(+), 109 deletions(-) create mode 100644 .github/scripts/render_digest.py diff --git a/.github/scripts/fork_sync_digest.py b/.github/scripts/fork_sync_digest.py index e9004a3..67da84b 100644 --- a/.github/scripts/fork_sync_digest.py +++ b/.github/scripts/fork_sync_digest.py @@ -1,25 +1,26 @@ #!/usr/bin/env python3 -"""Sync ledoent forks with upstream + emit a daily digest. - -Two responsibilities: - 1. For each fork in .github/forks.yml, POST /repos/{repo}/merge-upstream - for every branch listed in `branches:`. Failures collected into the - digest. Branches that don't exist on the fork are skipped quietly. - 2. For each fork whose `upstream_org` is set, query upstream for `[MIG]` - PRs closed in the last 24h on the configured `upstream_track`. - -Outputs `digest.html`, `digest.subject`, `digest.exit` in CWD which the -workflow consumes verbatim. Also writes `forks-parsed.json` so the -forward-port-distributor workflow can read the same fork list without -re-parsing the YAML. - -Auth: `GH_TOKEN` env var — a PAT scoped to ledoent/* with Contents:write -(for merge-upstream) and public_repo (for upstream PR listing). +"""Collect fork-sync + upstream-MIG data for the daily digest. + +Single responsibility: **collect**. No rendering, no email assembly — +those live in render_digest.py and the workflow's email step. + +Reads `.github/forks.yml`, then for each fork: + 1. POSTs /repos/{repo}/merge-upstream for every branch in `branches:`. + 2. (If `upstream_org` is set) queries upstream for `[MIG]` PRs that + merged in the last 24h on `upstream_track`. + +Writes three JSON files in CWD which render_digest.py consumes: + - forks-parsed.json the parsed forks.yml (also consumed by + distribute_forward_port.py) + - sync-results.json list of per-branch sync outcomes + - mig-buckets.json {"OCA/repo@track": [pr, ...]} + +Auth: `GH_TOKEN` — fine-grained PAT, resource owner `ledoent` org, +Contents:write + Workflows:write + public_repo. """ from __future__ import annotations -import html import json import re import sys @@ -36,15 +37,15 @@ def gh(method: str, path: str, body: dict | None = None) -> tuple[int, dict]: return request(method, path, headers=HEADERS, body=body) -def load_forks() -> list[dict]: - """Parse .github/forks.yml — block-style entries only. +def load_forks(path: str | Path = ".github/forks.yml") -> list[dict]: + """Parse forks.yml — block-style entries only. The format is restricted enough that we don't pull in PyYAML; one `- repo:` line per entry, followed by indented `key: value` lines until the next entry or a blank line. Lists are written as `[item, item]`. Comments stripped. """ - text = Path(".github/forks.yml").read_text() + text = Path(path).read_text() out: list[dict] = [] cur: dict | None = None @@ -67,7 +68,6 @@ def load_forks() -> list[dict]: if m_kv and cur is not None: k, v = m_kv.group(1), m_kv.group(2) if v.startswith("[") and v.endswith("]"): - # ["18.0", "19.0"] → list of strings cur[k] = [s.strip().strip('"') for s in v[1:-1].split(",") if s.strip()] elif v == "null": cur[k] = None @@ -89,9 +89,8 @@ def sync_branch(repo: str, branch: str) -> dict: # 422 + "does not exist" — branch isn't on the fork at all # 404 + "Branch not found" — branch isn't on the upstream either # Both are "skipped", not "failed". - skipped = ( - (status == 422 and "does not exist" in msg.lower()) - or (status == 404 and "branch not found" in msg.lower()) + skipped = (status == 422 and "does not exist" in msg.lower()) or ( + status == 404 and "branch not found" in msg.lower() ) return { "repo": repo, @@ -103,7 +102,9 @@ def sync_branch(repo: str, branch: str) -> dict: } -def list_recent_mig_prs(org: str, repo_name: str, track: str, since: datetime) -> list[dict]: +def list_recent_mig_prs( + org: str, repo_name: str, track: str, since: datetime +) -> list[dict]: q = ( f"repo:{org}/{repo_name} is:pr is:merged base:{track} " f"merged:>={since.strftime('%Y-%m-%dT%H:%M:%SZ')} " @@ -124,81 +125,9 @@ def list_recent_mig_prs(org: str, repo_name: str, track: str, since: datetime) - ] -def render(sync_results: list[dict], mig_buckets: dict[str, list[dict]]) -> tuple[str, str]: - today = datetime.now(timezone.utc).strftime("%Y-%m-%d") - fail = [r for r in sync_results if r["status"] >= 400 and not r["skipped"]] - updated = [r for r in sync_results if r.get("merge_type") in ("fast-forward", "merge")] - nochange = [r for r in sync_results if r.get("merge_type") == "none" and r["status"] < 400] - skipped = [r for r in sync_results if r["skipped"]] - - lines: list[str] = [ - '', - f"

Ledoent fork digest — {today}

", - f"

Sync: {len(updated)} updated · {len(nochange)} already current · " - f"{len(skipped)} skipped (branch n/a) · " - f'{len(fail)} failed

', - ] - - if fail: - lines.append('

⚠️ Sync failures

    ') - for r in fail: - lines.append( - f"
  • {html.escape(r['repo'])} " - f"({html.escape(r['branch'])}): " - f"HTTP {r['status']} — {html.escape(r['message'])}
    • " - ) - lines.append("
") - - if updated: - lines.append("

Synced

    ") - for r in updated: - lines.append( - f"
  • {html.escape(r['repo'])} " - f"({html.escape(r['branch'])}): " - f"{html.escape(r.get('merge_type') or '')}
    • " - ) - lines.append("
") - - total_migs = sum(len(v) for v in mig_buckets.values()) - if total_migs: - lines.append(f"

Upstream [MIG] PRs merged in last 24h ({total_migs})

") - for repo, items in sorted(mig_buckets.items()): - if not items: - continue - lines.append(f"

{html.escape(repo)}

    ") - for it in items: - lines.append( - f'
  • #{it["number"]} ' - f"{html.escape(it['title'])} — " - f"@{html.escape(it.get('user') or '')} " - f"{html.escape(it.get('merged_at') or '')}
    • " - ) - lines.append("
") - else: - lines.append("

No upstream [MIG] PRs merged in the last 24h.

") - - lines.append( - '

' - "Generated by ledoent/.github/.github/workflows/fork-sync-and-digest.yml — " - "edit .github/forks.yml to add/remove tracked forks.

" - ) - lines.append("") - - subject_parts = [f"Ledoent digest {today}"] - if fail: - subject_parts.append(f"⚠️ {len(fail)} sync fail") - if total_migs: - subject_parts.append(f"{total_migs} MIG") - subject = " — ".join(subject_parts) - - return subject, "\n".join(lines) - - def main() -> int: forks = load_forks() print(f"Loaded {len(forks)} forks from .github/forks.yml", file=sys.stderr) - - # Stash the parsed list for downstream workflows (forward-port distributor). Path("forks-parsed.json").write_text(json.dumps(forks, indent=2)) sync_results: list[dict] = [] @@ -206,8 +135,11 @@ def main() -> int: for branch in f.get("branches", []): res = sync_branch(f["repo"], branch) sync_results.append(res) - tag = "SKIP" if res["skipped"] else (res.get("merge_type") or str(res["status"])) + tag = "SKIP" if res["skipped"] else ( + res.get("merge_type") or str(res["status"]) + ) print(f" {res['repo']}@{branch:>10} -> {tag}", file=sys.stderr) + Path("sync-results.json").write_text(json.dumps(sync_results, indent=2)) since = datetime.now(timezone.utc) - timedelta(hours=24) mig_buckets: dict[str, list[dict]] = {} @@ -220,16 +152,18 @@ def main() -> int: if key in seen_upstream: continue seen_upstream.add(key) - prs = list_recent_mig_prs(f["upstream_org"], repo_name, f["upstream_track"], since) + prs = list_recent_mig_prs( + f["upstream_org"], repo_name, f["upstream_track"], since + ) if prs: mig_buckets[f"{f['upstream_org']}/{repo_name}@{f['upstream_track']}"] = prs + Path("mig-buckets.json").write_text(json.dumps(mig_buckets, indent=2)) - subject, body_html = render(sync_results, mig_buckets) - Path("digest.html").write_text(body_html) - Path("digest.subject").write_text(subject) - - fail_count = sum(1 for r in sync_results if r["status"] >= 400 and not r["skipped"]) - Path("digest.exit").write_text(str(fail_count)) + print( + f"Wrote sync-results.json ({len(sync_results)} entries) + " + f"mig-buckets.json ({sum(len(v) for v in mig_buckets.values())} PRs)", + file=sys.stderr, + ) return 0 diff --git a/.github/scripts/render_digest.py b/.github/scripts/render_digest.py new file mode 100644 index 0000000..5c4bb0e --- /dev/null +++ b/.github/scripts/render_digest.py @@ -0,0 +1,158 @@ +#!/usr/bin/env python3 +"""Render the daily digest email from collected JSON artifacts. + +Reads sync-results.json, mig-buckets.json, and (optionally) +forward-port-distribution.json from CWD. Writes digest.html, +digest.subject, and digest.exit which the workflow's email step +consumes. + +Splitting render from collection means the distributor's outcomes +land in the email body — previously they only appeared in the +artifact tarball, so a 14/14 failure (like our first run) sent a +"looks fine" digest. With this split, both the per-branch sync and +the forward-port distribution show up in the inbox. +""" + +from __future__ import annotations + +import html +import json +import sys +from datetime import datetime, timezone +from pathlib import Path + + +def _load(path: str, default): + p = Path(path) + return json.loads(p.read_text()) if p.exists() else default + + +def render( + sync_results: list[dict], + mig_buckets: dict[str, list[dict]], + distribution: list[dict], +) -> tuple[str, str, str]: + today = datetime.now(timezone.utc).strftime("%Y-%m-%d") + sync_fail = [ + r for r in sync_results if r["status"] >= 400 and not r["skipped"] + ] + sync_updated = [ + r for r in sync_results if r.get("merge_type") in ("fast-forward", "merge") + ] + sync_nochange = [ + r for r in sync_results + if r.get("merge_type") == "none" and r["status"] < 400 + ] + sync_skipped = [r for r in sync_results if r["skipped"]] + + dist_fail = [r for r in distribution if r["action"] == "failed"] + dist_created = [r for r in distribution if r["action"] == "created"] + dist_updated = [r for r in distribution if r["action"] == "updated"] + dist_unchanged = [r for r in distribution if r["action"] == "unchanged"] + + lines: list[str] = [ + '', + f"

Ledoent fork digest — {today}

", + f"

Sync: {len(sync_updated)} updated · " + f"{len(sync_nochange)} already current · " + f"{len(sync_skipped)} skipped (branch n/a) · " + f'' + f"{len(sync_fail)} failed

", + ] + if distribution: + lines.append( + f"

Forward-port distribution: " + f"{len(dist_created)} created · {len(dist_updated)} updated · " + f"{len(dist_unchanged)} unchanged · " + f'' + f"{len(dist_fail)} failed

" + ) + + if sync_fail: + lines.append('

⚠️ Sync failures

    ') + for r in sync_fail: + lines.append( + f"
  • {html.escape(r['repo'])} " + f"({html.escape(r['branch'])}): " + f"HTTP {r['status']} — {html.escape(r['message'])}
    • " + ) + lines.append("
") + + if dist_fail: + lines.append('

⚠️ Distributor failures

    ') + for r in dist_fail: + lines.append( + f"
  • {html.escape(r['repo'])}: " + f"{html.escape(r['detail'])}
    • " + ) + lines.append("
") + + if sync_updated: + lines.append("

Synced

    ") + for r in sync_updated: + lines.append( + f"
  • {html.escape(r['repo'])} " + f"({html.escape(r['branch'])}): " + f"{html.escape(r.get('merge_type') or '')}
    • " + ) + lines.append("
") + + total_migs = sum(len(v) for v in mig_buckets.values()) + if total_migs: + lines.append( + f"

Upstream [MIG] PRs merged in last 24h ({total_migs})

" + ) + for repo, items in sorted(mig_buckets.items()): + if not items: + continue + lines.append(f"

{html.escape(repo)}

    ") + for it in items: + lines.append( + f'
  • ' + f"#{it['number']} " + f"{html.escape(it['title'])} — " + f"@{html.escape(it.get('user') or '')} " + f"{html.escape(it.get('merged_at') or '')}" + "
    • " + ) + lines.append("
") + else: + lines.append("

No upstream [MIG] PRs merged in the last 24h.

") + + lines.append( + '

' + "Generated by ledoent/.github/.github/workflows/fork-sync-and-digest.yml " + "— edit .github/forks.yml to add/remove tracked forks.

" + ) + lines.append("") + + subject_parts = [f"Ledoent digest {today}"] + if sync_fail: + subject_parts.append(f"⚠️ {len(sync_fail)} sync fail") + if dist_fail: + subject_parts.append(f"⚠️ {len(dist_fail)} dist fail") + if total_migs: + subject_parts.append(f"{total_migs} MIG") + subject = " — ".join(subject_parts) + + fail_count = len(sync_fail) + len(dist_fail) + return subject, "\n".join(lines), str(fail_count) + + +def main() -> int: + sync_results = _load("sync-results.json", []) + mig_buckets = _load("mig-buckets.json", {}) + distribution = _load("forward-port-distribution.json", []) + + subject, body_html, exit_marker = render(sync_results, mig_buckets, distribution) + Path("digest.html").write_text(body_html) + Path("digest.subject").write_text(subject) + Path("digest.exit").write_text(exit_marker) + + print(f"Wrote digest.html ({len(body_html)} bytes), subject: {subject}", file=sys.stderr) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/fork-sync-and-digest.yml b/.github/workflows/fork-sync-and-digest.yml index 812b3d8..c7bbe31 100644 --- a/.github/workflows/fork-sync-and-digest.yml +++ b/.github/workflows/fork-sync-and-digest.yml @@ -62,7 +62,11 @@ jobs: # currently maps to; bump both when reviewing for updates. - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - name: Sync forks + assemble digest + # The pipeline is split into collect → distribute → render so the + # forward-port distributor's outcomes land in the email body, not + # just in the artifact tarball where nobody looks. + + - name: Collect sync + MIG data env: GH_TOKEN: ${{ secrets.LEDOENT_FORK_SYNC_TOKEN }} run: python3 .github/scripts/fork_sync_digest.py @@ -70,12 +74,17 @@ jobs: - name: Distribute forward-port workflow to opted-in forks env: GH_TOKEN: ${{ secrets.LEDOENT_FORK_SYNC_TOKEN }} - # Don't fail the whole run if one fork's distributor push fails — - # the failure shows up in the digest's "Sync failures" section - # tomorrow if anything's actually broken. + # continue-on-error so a flaky distributor doesn't suppress the + # digest — the render step will surface the failures in the email. continue-on-error: true run: python3 .github/scripts/distribute_forward_port.py + - name: Render digest HTML + subject + # if: always() so the digest goes out even if the distributor + # step crashed — we still want to know what synced. + if: always() + run: python3 .github/scripts/render_digest.py + - name: Read digest subject id: subject # Heredoc form because `key=value` syntax breaks if the value @@ -118,5 +127,7 @@ jobs: digest.subject digest.exit forks-parsed.json + sync-results.json + mig-buckets.json forward-port-distribution.json retention-days: 7 diff --git a/.gitignore b/.gitignore index 10fa070..3cf887f 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,7 @@ __pycache__/ digest.html digest.subject digest.exit +sync-results.json +mig-buckets.json +forks-parsed.json +forward-port-distribution.json From f2dbc26e7c8ba14ec0ac2a6fdb43dd3578ff83a8 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Wed, 20 May 2026 07:36:05 -0400 Subject: [PATCH 7/9] test: pytest coverage for parser + sync state mapping + renderer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 19 tests covering the three pieces of behaviour most likely to silently regress: 1. load_forks() — the hand-rolled YAML parser. Tests cover the happy path, multi-branch lists, null upstreams, inline comments, blank-line termination, and a smoke test against the real forks.yml shipped in this repo. 2. sync_branch() — the response → state mapping for merge-upstream. Tests pin the 200-fast-forward / 200-none / 422-skip / 404-skip / 403-real-fail / 409-real-fail cases so a refactor can't accidentally re-classify "branch n/a" as failure. 3. render() — subject tagging and failure surfacing. Tests pin "⚠️ N sync fail" vs "⚠️ N dist fail" separation, MIG counts in the subject, and HTML escaping of failure messages. Running pytest exposed a real bug in load_forks(): comment-only lines inside an entry (like the OpenUpgrade entry's two indented `# Default branch is …` lines) were being misread as blank-line entry separators, truncating the entry to just `{repo: ledoent/OpenUpgrade}`. The upstream comment-stripping path was firing before the blank-line check. Fix moves the blank-line check ahead of comment-stripping so only TRULY empty lines terminate; comment-only lines are no-ops. Also adds .github/workflows/tests.yml so pytest runs on PRs that touch the scripts. No secrets or API calls — safe to run from forks. --- .github/scripts/fork_sync_digest.py | 12 +- .github/scripts/tests/__init__.py | 0 .github/scripts/tests/conftest.py | 16 +++ .github/scripts/tests/test_load_forks.py | 130 ++++++++++++++++++ .github/scripts/tests/test_render_digest.py | 120 ++++++++++++++++ .../scripts/tests/test_sync_branch_state.py | 58 ++++++++ .github/workflows/tests.yml | 33 +++++ 7 files changed, 367 insertions(+), 2 deletions(-) create mode 100644 .github/scripts/tests/__init__.py create mode 100644 .github/scripts/tests/conftest.py create mode 100644 .github/scripts/tests/test_load_forks.py create mode 100644 .github/scripts/tests/test_render_digest.py create mode 100644 .github/scripts/tests/test_sync_branch_state.py create mode 100644 .github/workflows/tests.yml diff --git a/.github/scripts/fork_sync_digest.py b/.github/scripts/fork_sync_digest.py index 67da84b..3820c4e 100644 --- a/.github/scripts/fork_sync_digest.py +++ b/.github/scripts/fork_sync_digest.py @@ -50,12 +50,20 @@ def load_forks(path: str | Path = ".github/forks.yml") -> list[dict]: cur: dict | None = None for raw in text.splitlines(): - line = raw.split("#", 1)[0].rstrip() - if not line.strip(): + # Blank-line check uses the ORIGINAL line, before comment + # stripping. Otherwise comment-only lines inside an entry + # (e.g. an indented `# Default branch is …` between two keys) + # look identical to a true blank separator and silently + # truncate the entry. Comment lines are a no-op; only truly + # empty/whitespace-only lines terminate an entry. + if not raw.strip(): if cur: out.append(cur) cur = None continue + line = raw.split("#", 1)[0].rstrip() + if not line.strip(): + continue # comment-only line — keep accumulating into cur m_new = re.match(r"^ - repo:\s*(.+?)\s*$", line) if m_new: diff --git a/.github/scripts/tests/__init__.py b/.github/scripts/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/.github/scripts/tests/conftest.py b/.github/scripts/tests/conftest.py new file mode 100644 index 0000000..da4dcab --- /dev/null +++ b/.github/scripts/tests/conftest.py @@ -0,0 +1,16 @@ +"""Make the parent scripts/ importable as flat modules. + +Each script under .github/scripts/ is run via `python3 .github/scripts/foo.py` +which puts that directory on sys.path. Mirror that here so tests can +`import fork_sync_digest` etc. +""" + +import os +import sys + +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) + +# Token isn't needed for the parser/rendering tests — but require_token() +# runs at module import (`HEADERS = make_headers(require_token(), …)`), +# so stub it so the script-level imports don't exit(2). +os.environ.setdefault("GH_TOKEN", "test-token-not-real") diff --git a/.github/scripts/tests/test_load_forks.py b/.github/scripts/tests/test_load_forks.py new file mode 100644 index 0000000..c3f388d --- /dev/null +++ b/.github/scripts/tests/test_load_forks.py @@ -0,0 +1,130 @@ +"""Cover the hand-rolled YAML parser in fork_sync_digest.load_forks(). + +The parser is fragile (regex-driven, not PyYAML) so this is where we +spend tests. If load_forks() ever stops parsing a real-world entry, +the workflow silently runs against an incomplete fork list — which +looks identical to "nothing changed" until somebody notices. +""" + +from pathlib import Path + +import fork_sync_digest + + +def _write(tmp_path: Path, content: str) -> Path: + p = tmp_path / "forks.yml" + p.write_text(content) + return p + + +def test_minimal_entry(tmp_path): + p = _write( + tmp_path, + """ +forks: + - repo: ledoent/web + branches: ["18.0"] + upstream_org: OCA + upstream_track: "19.0" + install_forward_port: true +""", + ) + forks = fork_sync_digest.load_forks(p) + assert forks == [ + { + "repo": "ledoent/web", + "branches": ["18.0"], + "upstream_org": "OCA", + "upstream_track": "19.0", + "install_forward_port": True, + } + ] + + +def test_multi_branch_list_with_two_entries(tmp_path): + p = _write( + tmp_path, + """ +forks: + - repo: ledoent/web + branches: ["18.0", "19.0"] + upstream_org: OCA + upstream_track: "19.0" + install_forward_port: true + + - repo: ledoent/sale-workflow + branches: ["18.0", "19.0"] + upstream_org: OCA + upstream_track: "19.0" + install_forward_port: false +""", + ) + forks = fork_sync_digest.load_forks(p) + assert len(forks) == 2 + assert forks[0]["branches"] == ["18.0", "19.0"] + assert forks[0]["install_forward_port"] is True + assert forks[1]["install_forward_port"] is False + + +def test_null_upstream_for_non_oca_fork(tmp_path): + p = _write( + tmp_path, + """ +forks: + - repo: ledoent/odoo + branches: ["18.0"] + upstream_org: null + upstream_track: null + install_forward_port: false +""", + ) + forks = fork_sync_digest.load_forks(p) + assert forks[0]["upstream_org"] is None + assert forks[0]["upstream_track"] is None + + +def test_inline_comments_stripped(tmp_path): + p = _write( + tmp_path, + """ +forks: + - repo: ledoent/calendar # active feature work, 5 open PRs + branches: ["18.0", "19.0"] # both releases tracked + upstream_org: OCA + upstream_track: "19.0" + install_forward_port: true +""", + ) + forks = fork_sync_digest.load_forks(p) + assert forks[0]["repo"] == "ledoent/calendar" + assert forks[0]["branches"] == ["18.0", "19.0"] + + +def test_blank_lines_terminate_entries(tmp_path): + p = _write( + tmp_path, + """ +forks: + - repo: ledoent/a + branches: ["18.0"] + + - repo: ledoent/b + branches: ["19.0"] +""", + ) + forks = fork_sync_digest.load_forks(p) + assert [f["repo"] for f in forks] == ["ledoent/a", "ledoent/b"] + + +def test_real_forks_yml_parses(tmp_path): + # Smoke test against the actual file shipped in this repo so a + # malformed edit there fails fast in CI rather than at run time. + real = Path(__file__).parent.parent.parent / "forks.yml" + forks = fork_sync_digest.load_forks(real) + assert len(forks) >= 15 # current count is 20; allow churn + for f in forks: + assert "repo" in f + assert "branches" in f and isinstance(f["branches"], list) + assert "upstream_org" in f + assert "upstream_track" in f + assert "install_forward_port" in f diff --git a/.github/scripts/tests/test_render_digest.py b/.github/scripts/tests/test_render_digest.py new file mode 100644 index 0000000..a3650db --- /dev/null +++ b/.github/scripts/tests/test_render_digest.py @@ -0,0 +1,120 @@ +"""Cover the digest rendering — subject tagging and failure surfacing. + +The render() function is small but it owns the inbox UX. If it stops +flagging dist failures in the subject (because of a refactor that +forgets the branch), the next 14/14-failure run looks identical to a +healthy one. Pin the behavior here. +""" + +import render_digest + + +def _sync(status: int, merge_type=None, skipped=False, **kw): + return { + "repo": "ledoent/x", + "branch": "18.0", + "status": status, + "message": kw.get("message", ""), + "merge_type": merge_type, + "skipped": skipped, + } + + +def test_clean_subject_no_failures(): + subj, body, exit_marker = render_digest.render( + sync_results=[_sync(200, "fast-forward")], + mig_buckets={}, + distribution=[{"repo": "ledoent/x", "action": "unchanged", "detail": "abc"}], + ) + assert "Ledoent digest" in subj + assert "fail" not in subj.lower() + assert exit_marker == "0" + assert "Distributor failures" not in body + + +def test_sync_failure_tagged_in_subject(): + subj, body, exit_marker = render_digest.render( + sync_results=[ + _sync(200, "fast-forward"), + _sync(403, message="Forbidden"), + ], + mig_buckets={}, + distribution=[], + ) + assert "⚠️ 1 sync fail" in subj + assert exit_marker == "1" + assert "Sync failures" in body + assert "Forbidden" in body + + +def test_distributor_failure_tagged_separately(): + subj, body, exit_marker = render_digest.render( + sync_results=[_sync(200, "fast-forward")], + mig_buckets={}, + distribution=[ + {"repo": "ledoent/x", "action": "failed", "detail": "PUT 403"}, + {"repo": "ledoent/y", "action": "created", "detail": "abc"}, + ], + ) + assert "⚠️ 1 dist fail" in subj + # Sync was clean — sync-fail shouldn't appear + assert "sync fail" not in subj + assert exit_marker == "1" + assert "Distributor failures" in body + assert "PUT 403" in body + + +def test_skipped_branches_are_not_failures(): + # 404 + "Branch not found" is the "branch n/a on upstream" case; + # 422 + "does not exist" is the "branch n/a on fork" case. Neither + # should bump the failure counter. + subj, _, exit_marker = render_digest.render( + sync_results=[ + _sync(404, skipped=True, message="Branch not found"), + _sync(422, skipped=True, message="does not exist"), + ], + mig_buckets={}, + distribution=[], + ) + assert "fail" not in subj.lower() + assert exit_marker == "0" + + +def test_mig_count_in_subject(): + subj, body, _ = render_digest.render( + sync_results=[_sync(200, "none")], + mig_buckets={ + "OCA/web@19.0": [ + {"title": "[19.0][MIG] foo", "url": "http://x", "number": 1, + "merged_at": "2026-05-20T01:02:03Z", "user": "alice"} + ] + }, + distribution=[], + ) + assert "1 MIG" in subj + assert "[19.0][MIG] foo" in body + assert "@alice" in body + + +def test_both_sync_and_dist_failures_in_subject(): + subj, _, exit_marker = render_digest.render( + sync_results=[_sync(403, message="X")], + mig_buckets={}, + distribution=[{"repo": "y", "action": "failed", "detail": "Y"}], + ) + assert "⚠️ 1 sync fail" in subj + assert "⚠️ 1 dist fail" in subj + assert exit_marker == "2" + + +def test_html_escaping_in_failure_message(): + # Defensive: an API error message containing < > & shouldn't break + # the rendered HTML (or open an XSS hole if anyone ever pipes the + # digest into a different surface). + subj, body, _ = render_digest.render( + sync_results=[_sync(500, message="")], + mig_buckets={}, + distribution=[], + ) + assert "" not in body + assert "<script>" in body diff --git a/.github/scripts/tests/test_sync_branch_state.py b/.github/scripts/tests/test_sync_branch_state.py new file mode 100644 index 0000000..4f9efc9 --- /dev/null +++ b/.github/scripts/tests/test_sync_branch_state.py @@ -0,0 +1,58 @@ +"""Cover the response → state mapping in sync_branch. + +The function is a thin wrapper around merge-upstream, but the +422/404 → "skipped" branch is load-bearing: get it wrong and +forks without a 19.0 branch yet flood the digest with false alarms. +""" + +import fork_sync_digest + + +def _patch_gh(monkeypatch, returns): + """Make gh(method, path, body) return a fixed (status, body).""" + monkeypatch.setattr(fork_sync_digest, "gh", lambda *a, **kw: returns) + + +def test_fast_forward_is_success(monkeypatch): + _patch_gh(monkeypatch, (200, {"merge_type": "fast-forward"})) + r = fork_sync_digest.sync_branch("ledoent/x", "18.0") + assert r["status"] == 200 + assert r["merge_type"] == "fast-forward" + assert r["skipped"] is False + + +def test_no_change_is_success(monkeypatch): + _patch_gh(monkeypatch, (200, {"merge_type": "none"})) + r = fork_sync_digest.sync_branch("ledoent/x", "18.0") + assert r["skipped"] is False + assert r["merge_type"] == "none" + + +def test_422_branch_not_on_fork_is_skipped(monkeypatch): + _patch_gh(monkeypatch, (422, {"message": "Branch does not exist"})) + r = fork_sync_digest.sync_branch("ledoent/x", "20.0") + assert r["status"] == 422 + assert r["skipped"] is True + + +def test_404_branch_not_on_upstream_is_skipped(monkeypatch): + _patch_gh(monkeypatch, (404, {"message": "Branch not found"})) + r = fork_sync_digest.sync_branch("ledoent/x", "20.0") + assert r["status"] == 404 + assert r["skipped"] is True + + +def test_real_403_is_not_skipped(monkeypatch): + # The first-run PAT-scope error class. MUST flag as failure. + _patch_gh(monkeypatch, (403, {"message": "Resource not accessible by personal access token"})) + r = fork_sync_digest.sync_branch("ledoent/x", "18.0") + assert r["skipped"] is False + assert r["status"] == 403 + + +def test_conflict_409_is_not_skipped(monkeypatch): + # Fork has diverged from upstream — real failure, action required. + _patch_gh(monkeypatch, (409, {"message": "Merge conflict"})) + r = fork_sync_digest.sync_branch("ledoent/x", "18.0") + assert r["skipped"] is False + assert r["status"] == 409 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..4b07524 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,33 @@ +name: tests + +# Runs the org-control script tests on every PR + push to main. +# Pure-Python pytest, no secrets or external API calls — safe to fire +# from forks. + +on: + pull_request: + paths: + - ".github/scripts/**" + - ".github/forks.yml" + - ".github/workflows/tests.yml" + push: + branches: [main] + paths: + - ".github/scripts/**" + - ".github/forks.yml" + - ".github/workflows/tests.yml" + +permissions: + contents: read + +jobs: + pytest: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: "3.x" + - run: pip install pytest + - run: python -m pytest .github/scripts/tests/ -v From 65d39c1ad51f7438bb02733b499c3da1d4cee0e8 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Wed, 20 May 2026 07:37:03 -0400 Subject: [PATCH 8/9] docs: repo-root README + clarify PAT resource-owner trap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a top-level README so https://github.com/ledoent/.github gives a visitor the org-control overview without making them dig through .github/scripts/. Covers what runs daily, the secret list, and how to add a fork. The README explicitly calls out the PAT resource-owner trap: the token must be issued under the ledoent ORG, not a personal account — a fine-grained PAT scoped to "all repositories owned by you" (where you = a personal account) returns 403 on every ledoent/* repo, even when permissions are correct. That's the failure mode that cost us a day on the initial setup; flagging it in two places (root README + scripts/README.md) so the next person doesn't repeat it. The scripts/ README is rewritten as internals-focused since the overview lives at root now. New table maps each stage of the pipeline to its inputs/outputs so the data flow is greppable. --- .github/scripts/README.md | 97 ++++++++++++++++++++------------------- README.md | 76 ++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 48 deletions(-) create mode 100644 README.md diff --git a/.github/scripts/README.md b/.github/scripts/README.md index 80dba63..3a64c9d 100644 --- a/.github/scripts/README.md +++ b/.github/scripts/README.md @@ -1,72 +1,73 @@ -# Org workflow scripts +# Org-control scripts -Helpers invoked by workflows in `../workflows/`. Two scripts run daily as a -single chain inside `fork-sync-and-digest.yml`: +Helpers invoked by `.github/workflows/fork-sync-and-digest.yml` and +`.github/workflows/tests.yml`. The repo-root [`../../README.md`](../../README.md) +has the high-level overview; this file documents internals. -## 1. `fork_sync_digest.py` — sync + digest assembly +## Three-stage daily pipeline -- Reads `../forks.yml`. -- For each fork, calls `POST /repos/{repo}/merge-upstream` on every branch - in `branches:` (e.g. `["18.0", "19.0"]`). Non-existent branches return - HTTP 422 and are marked as skipped, not failed. -- Queries each unique upstream `(org, repo)` once for PRs with `[MIG]` in - the title merged in the last 24h on `upstream_track`. -- Writes `digest.html`, `digest.subject`, `digest.exit` for the email step. -- Writes `forks-parsed.json` for the next stage. +| Stage | Script | Reads | Writes | +|---|---|---|---| +| Collect | `fork_sync_digest.py` | `../forks.yml` | `forks-parsed.json`, `sync-results.json`, `mig-buckets.json` | +| Distribute | `distribute_forward_port.py` | `forks-parsed.json`, `../templates/forward-port.yml` | `forward-port-distribution.json` | +| Render | `render_digest.py` | `sync-results.json`, `mig-buckets.json`, `forward-port-distribution.json` | `digest.html`, `digest.subject`, `digest.exit` | -## 2. `distribute_forward_port.py` — push `forward-port.yml` to opted-in forks +The render stage is separate from collect so the distributor's +outcomes land in the email body — without the split, a 14/14 +distributor failure would only appear in the artifact tarball. -- Reads `forks-parsed.json`. -- For each fork with `install_forward_port: true`, PUTs - `.github/workflows/forward-port.yml` (from `../templates/forward-port.yml`) - onto the fork's default branch via the Contents API. -- Idempotent: no-ops when the file already matches the template. +## Shared helper -## The forward-port workflow itself +`_github.py` holds the stdlib-only HTTP wrapper used by both scripts. +Calls `require_token()` at startup to fail fast with an actionable +message when `GH_TOKEN` is unset, then `make_headers()` and +`request()` for the actual API calls. Add retry / rate-limit handling +here (one place) if it becomes necessary. -Lives at `../templates/forward-port.yml`. Once installed on a fork, label -any PR with `port:` (e.g. `port:19.0`) and on merge the -[`korthout/backport-action`](https://github.com/korthout/backport-action) -cherry-picks the squash commit onto the named branch and opens a -follow-up PR. Conflicts are reported in the PR body for manual resolution. +## Forward-port workflow template -## Running locally +`../templates/forward-port.yml` is the file the distributor pushes +to every opted-in fork. It uses `pull_request_target` (deliberate; +see comment in the file) and SHA-pinned actions. + +## Tests + +`tests/` covers the parser, the merge-upstream state mapping, and the +digest rendering. Run with: ```sh -export GH_TOKEN=... # PAT with Contents:write on ledoent/* + public_repo -python3 .github/scripts/fork_sync_digest.py -python3 .github/scripts/distribute_forward_port.py -cat digest.html +python3 -m pytest .github/scripts/tests/ -v ``` +CI runs them via `.github/workflows/tests.yml` on every PR that +touches `.github/scripts/**`, `.github/forks.yml`, or the tests +workflow itself. + ## Adding a fork -Edit `.github/forks.yml`. Each entry: +Edit `../forks.yml`. Each entry: | Key | Purpose | |---|---| | `repo` | `ledoent/` | -| `branches` | List of branches to keep synced. Add `20.0` here when OCA cuts it. | +| `branches` | List of branches to keep synced. Add `"20.0"` here when OCA cuts it. | | `upstream_org` | `OCA` for OCA forks, `null` to skip MIG digest | | `upstream_track` | Branch on upstream to scan for `[MIG]` PRs (typically the next major) | | `install_forward_port` | `true` to push `forward-port.yml` to this fork | +## Running locally + +```sh +export GH_TOKEN=$(gh auth token) # or a fine-grained PAT +python3 .github/scripts/fork_sync_digest.py +python3 .github/scripts/distribute_forward_port.py +python3 .github/scripts/render_digest.py +cat digest.subject && open digest.html # macOS +``` + ## Secrets -Set at the org or `ledoent/.github` repo level: - -- `LEDOENT_FORK_SYNC_TOKEN` — fine-grained PAT, scopes: - - `Contents: Read and write` on `ledoent/*` (merge-upstream + file writes) - - `Workflows: Read and write` on `ledoent/*` — **required** because the - distributor writes `.github/workflows/forward-port.yml`; without this - every distributor PUT returns `403 Resource not accessible by personal - access token` - - `Metadata: Read-only` (auto-selected when picking a repo) - - `public_repo` (for searching OCA upstream PRs) -- `SMTP_SERVER` / `SMTP_USERNAME` / `SMTP_PASSWORD` / `SMTP_TO` / `SMTP_FROM` - - `SMTP_USERNAME`: SES SMTP-credential AKID - - `SMTP_PASSWORD`: derived from the IAM secret via SES's signing - algorithm (NOT the raw IAM secret) - - `SMTP_FROM`: full From: header, e.g. `Ledoent CI ` - on a domain SES has verified (`ledoweb.com` is already verified - in account 058264328562, region us-east-1) +See [`../../README.md`](../../README.md). The single load-bearing +detail: the PAT's **resource owner must be the `ledoent` org**, not +a personal account. A PAT issued under a personal account can't see +ledoent-owned forks even with full permissions. diff --git a/README.md b/README.md new file mode 100644 index 0000000..2a7a9ae --- /dev/null +++ b/README.md @@ -0,0 +1,76 @@ +# `ledoent/.github` — org control plane + +This repo holds two things that apply to the whole `ledoent` org: + +1. **Community health files** under `profile/` — org-level README, avatar, + banner (rendered on https://github.com/ledoent). + +2. **Automation that operates across all ledoent forks** — daily + upstream sync, MIG-PR digest email, and forward-port workflow + distribution. See `.github/workflows/fork-sync-and-digest.yml` and + `.github/scripts/README.md` for details. + +## Automation at a glance + +| Workflow | Trigger | What it does | +|---|---|---| +| `fork-sync-and-digest` | daily 13:00 UTC + manual | Syncs each fork's tracked branches from upstream, distributes the forward-port workflow to opted-in forks, queries upstream OCA for `[MIG]` PRs, and emails an HTML digest | +| `tests` | PR + push to main | Runs pytest on the org-control scripts | + +The tracked fork list lives in [`.github/forks.yml`](.github/forks.yml). +Edit that file (not the workflow YAML) when adding or removing forks. + +## Secrets required + +Set at the org level (Settings → Secrets and variables → Actions) or +on this repo: + +| Secret | What it is | Notes | +|---|---|---| +| `LEDOENT_FORK_SYNC_TOKEN` | Fine-grained PAT | **Resource owner MUST be the `ledoent` org**, not a personal account — the forks live under the org. Scopes: Contents R/W, Workflows R/W, Metadata Read, public_repo. | +| `SMTP_SERVER` | SMTP host | e.g. `email-smtp.us-east-1.amazonaws.com` for SES | +| `SMTP_USERNAME` | SMTP auth username | For SES, the SMTP credential AKID | +| `SMTP_PASSWORD` | SMTP auth password | For SES, derived from the IAM secret via the [SES SMTP-credential algorithm](https://docs.aws.amazon.com/ses/latest/dg/smtp-credentials.html#smtp-credentials-convert) — **NOT the raw IAM secret** | +| `SMTP_TO` | Digest recipient | e.g. `dkendall@ledoweb.com` | +| `SMTP_FROM` | Full `From:` header | e.g. `Ledoent CI ` — domain must be SES-verified | + +## Adding a new fork + +Edit `.github/forks.yml` and add an entry: + +```yaml +- repo: ledoent/ + branches: ["18.0", "19.0"] + upstream_org: OCA + upstream_track: "19.0" + install_forward_port: true +``` + +The next scheduled run (or a manual dispatch) picks it up. Add `"20.0"` +to `branches:` when OCA cuts that release. + +## Forward-port workflow + +Forks that opt in (`install_forward_port: true`) get +`.github/workflows/forward-port.yml` installed by the distributor. +Label any fork-internal PR with `port:19.0` (or other branch name); +on merge, the labelled commit is cherry-picked onto that branch and a +follow-up PR is opened. Conflicts surface in the PR body for manual +resolution. + +## Local development + +```sh +# Run the tests +python3 -m pytest .github/scripts/tests/ -v + +# Run the collect step against the live API (uses your gh CLI token — +# limit blast radius accordingly) +GH_TOKEN=$(gh auth token) python3 .github/scripts/fork_sync_digest.py + +# Then render the digest from the collected JSON +python3 .github/scripts/render_digest.py +``` + +Generated `digest.html` opens in any browser; `digest.subject` is +what the email step sends. From 219a624ee6d04eb8074b15c1afee94477e989669 Mon Sep 17 00:00:00 2001 From: Don Kendall Date: Wed, 20 May 2026 10:32:51 -0400 Subject: [PATCH 9/9] fix(forks): stop syncing OpenUpgrade's `ledoent` CI overlay branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fork's default branch on ledoent/OpenUpgrade is `ledoent`, which is a CI overlay (= 19.0 + lab CI customizations baked on top). It is NOT a copy of an upstream branch. merge-upstream was trying to merge OCA/OpenUpgrade@19.0 into this overlay, which is the wrong direction: upstream/19.0 ----A----B----C----D \\ ledoent --------+ci-overlay--+ ←← merging A..D in here conflicts with the overlay's intentional drift, returns 409 The actual workflow is the reverse: when 19.0 drifts too far, the human rebases the CI overlay on top of 19.0 manually (per docs/branch-model.md on the lab side). PRs always target OCA's 19.0, never the overlay. Removing "ledoent" from branches[] so only 19.0 gets auto-synced. The overlay can stay as the fork's default branch — the digest just won't touch it. Drops one false-failure entry from tomorrow's email. --- .github/forks.yml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/forks.yml b/.github/forks.yml index 8b1d618..fcc8713 100644 --- a/.github/forks.yml +++ b/.github/forks.yml @@ -104,9 +104,14 @@ forks: install_forward_port: true - repo: ledoent/OpenUpgrade - # Default branch is "ledoent" (= 19.0 + CI customizations). Sync that - # alongside the upstream version-pinned branches. - branches: ["ledoent", "19.0"] + # NOTE: the fork's default branch `ledoent` is a CI overlay + # (= 19.0 + lab CI customizations). It is deliberately NOT in + # `branches:` — upstream PRs always target OCA's `19.0`, never + # the overlay, so merging upstream into `ledoent` is the wrong + # direction (would drop the overlay's customizations under a + # conflict). The overlay is rebased on top of 19.0 manually + # when it drifts too far. Only `19.0` gets auto-synced here. + branches: ["19.0"] upstream_org: OCA upstream_track: "19.0" install_forward_port: false