diff --git a/.github/scripts/fork_sync_digest.py b/.github/scripts/fork_sync_digest.py index cc8affb..2ebdb88 100644 --- a/.github/scripts/fork_sync_digest.py +++ b/.github/scripts/fork_sync_digest.py @@ -30,7 +30,34 @@ from _github import make_headers, request, require_token -HEADERS = make_headers(require_token(), user_agent="ledoent-fork-digest/2.0") +# Lazy-init so the module is importable without GH_TOKEN (render_digest +# imports MANAGED_COMMIT_PATTERNS from here, and runs in a step without +# the token env). Populated by main() before any gh() call. +HEADERS: dict = {} + +# Ahead-commit subjects that the sync pipeline itself produces, so an +# overlay of these on top of upstream is expected — not "diverged" in +# the alarming sense. Two sources: +# 1. distribute_forward_port.py writes forward-port.yml via the +# Contents API, which lands as a commit on the default (series) +# branch. The workflow file MUST live on the branch where +# pull_request_target events fire, so this commit is structural. +# 2. merge-upstream produces a "Merge branch 'OCA:' into " +# commit any time the fork is non-empty (i.e. carries #1). Also +# structural — not a sign of human error on a series branch. +# Anything outside these patterns is a real divergence (someone pushed +# work to a series branch directly) and stays flagged. +MANAGED_COMMIT_PATTERNS = ( + re.compile( + r"^chore\(ci\): (created|updated) forward-port\.yml " + r"from ledoent/\.github distributor" + ), + re.compile(r"^Merge branch '[^']+' into "), +) + + +def _is_managed_commit(subject: str) -> bool: + return any(p.match(subject) for p in MANAGED_COMMIT_PATTERNS) def gh(method: str, path: str, body: dict | None = None) -> tuple[int, dict]: @@ -109,32 +136,48 @@ def sync_branch(repo: str, branch: str, upstream_org: str | None = None) -> dict "skipped": skipped, "ahead_by": None, "behind_by": None, + "ahead_commits": [], "diverged": False, + "managed_overlay": False, } # Post-sync divergence check. merge-upstream silently produces a # merge commit (instead of fast-forwarding) when the fork has local # commits the upstream doesn't have — returning success either way. # Without this check, accidental pushes to a series-named branch on - # the fork (e.g. someone "Merge pull request"-ing a feature branch - # into 18.0) accumulate undetected and the daily sync keeps adding - # merge commits on top. Compare against upstream to surface drift. + # the fork accumulate undetected. + # + # But: forks with install_forward_port carry a managed chore commit + # on every series branch (the distributor's forward-port.yml write), + # and merge-upstream then layers a "Merge branch 'OCA:'" commit + # on top each time upstream advances. Both are structural — not + # human error. Classify them as `managed_overlay` and reserve + # `diverged` for genuinely unexpected ahead-commits. if upstream_org and not skipped and status < 400: - ahead, behind = _compare_with_upstream(repo, upstream_org, branch) + ahead, behind, subjects = _compare_with_upstream( + repo, upstream_org, branch + ) result["ahead_by"] = ahead result["behind_by"] = behind - result["diverged"] = bool(ahead and ahead > 0) + result["ahead_commits"] = subjects + if ahead and ahead > 0: + unmanaged = [s for s in subjects if not _is_managed_commit(s)] + if unmanaged: + result["diverged"] = True + else: + result["managed_overlay"] = True return result def _compare_with_upstream( fork_repo: str, upstream_org: str, branch: str -) -> tuple[int | None, int | None]: - """Return (ahead_by, behind_by) for fork branch vs upstream branch. +) -> tuple[int | None, int | None, list[str]]: + """Return (ahead_by, behind_by, ahead_commit_subjects). - ahead_by > 0 means the fork has commits the upstream doesn't — - almost always a mistake on a series-named tracking branch. - Returns (None, None) on any API error so a flaky compare doesn't - mask a successful sync. + `ahead_commit_subjects` is the first line of each commit the fork + has that upstream doesn't — used to classify the divergence as + managed (structural overlay) vs. real (someone pushed work to a + series branch). Returns (None, None, []) on any API error so a + flaky compare doesn't mask a successful sync. """ fork_owner = fork_repo.split("/", 1)[0] repo_name = fork_repo.split("/", 1)[1] @@ -144,8 +187,12 @@ def _compare_with_upstream( ) status, body = gh("GET", path) if status != 200: - return None, None - return body.get("ahead_by"), body.get("behind_by") + return None, None, [] + subjects = [ + (c.get("commit", {}).get("message") or "").split("\n", 1)[0] + for c in body.get("commits", []) + ] + return body.get("ahead_by"), body.get("behind_by"), subjects def list_recent_mig_prs( @@ -172,6 +219,8 @@ def list_recent_mig_prs( def main() -> int: + global HEADERS + HEADERS = make_headers(require_token(), user_agent="ledoent-fork-digest/2.0") forks = load_forks() print(f"Loaded {len(forks)} forks from .github/forks.yml", file=sys.stderr) Path("forks-parsed.json").write_text(json.dumps(forks, indent=2)) diff --git a/.github/scripts/render_digest.py b/.github/scripts/render_digest.py index 90fe8e3..adc7d8c 100644 --- a/.github/scripts/render_digest.py +++ b/.github/scripts/render_digest.py @@ -21,6 +21,8 @@ from datetime import datetime, timezone from pathlib import Path +from fork_sync_digest import MANAGED_COMMIT_PATTERNS + def _load(path: str, default): p = Path(path) @@ -44,14 +46,22 @@ def render( if r.get("merge_type") == "none" and r["status"] < 400 ] sync_skipped = [r for r in sync_results if r["skipped"]] - # Divergence = fork branch has commits upstream doesn't. merge-upstream - # returns success and produces a merge commit; the digest needs to - # surface this separately so it gets fixed (force-reset the branch) - # instead of accumulating forever. + # Divergence = fork branch has commits upstream doesn't, AND those + # commits aren't part of the managed overlay (forward-port.yml + # chore commit + sync-produced merge commits). Real divergence + # means someone pushed work to a series branch — needs manual fix. sync_diverged = [ r for r in sync_results if r.get("diverged") and not r["skipped"] ] + # Managed-overlay branches are 1+ ahead but only by structural + # commits. Counted in a small footer for transparency, never in the + # subject line — they're the steady state for install_forward_port + # forks and would otherwise spam the inbox every day. + sync_managed_overlay = [ + r for r in sync_results + if r.get("managed_overlay") and not r["skipped"] + ] dist_fail = [r for r in distribution if r["action"] == "failed"] dist_created = [r for r in distribution if r["action"] == "created"] @@ -95,18 +105,33 @@ def render( if sync_diverged: lines.append( '

⚠️ Diverged from upstream

' - "

These fork branches have commits the upstream doesn't — " - "merge-upstream silently produced a merge commit instead of " - "fast-forwarding. Reset the branch to upstream HEAD " - "(after backing it up).

    " + "

    These fork branches have non-managed commits the upstream " + "doesn't — i.e. work landed directly on a series-named branch. " + "Investigate before the next sync layers more merge commits " + "on top.

      " ) for r in sync_diverged: ahead = r.get("ahead_by") or 0 behind = r.get("behind_by") or 0 + unmanaged = [ + s for s in r.get("ahead_commits", []) + if s and not any(p.match(s) for p in MANAGED_COMMIT_PATTERNS) + ] + commits_html = "" + if unmanaged: + items = "".join( + f"
    • {html.escape(s)}
      • " + for s in unmanaged[:5] + ) + more = ( + f"
    • … +{len(unmanaged) - 5} more
      • " + if len(unmanaged) > 5 else "" + ) + commits_html = f"
        {items}{more}
      " lines.append( f"
    • {html.escape(r['repo'])} " f"({html.escape(r['branch'])}): " - f"{ahead} ahead, {behind} behind
      • " + f"{ahead} ahead, {behind} behind{commits_html}" ) lines.append("
    ") @@ -151,6 +176,14 @@ def render( else: lines.append("

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

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

    ' + f"Managed overlay: {len(sync_managed_overlay)} branches " + "are 1+ ahead of upstream only by structural commits " + "(forward-port.yml distributor + sync-produced merges). " + "Expected; not divergence.

    " + ) lines.append( '

    ' "Generated by ledoent/.github/.github/workflows/fork-sync-and-digest.yml " diff --git a/.github/scripts/tests/test_render_digest.py b/.github/scripts/tests/test_render_digest.py index aa52dc8..955a991 100644 --- a/.github/scripts/tests/test_render_digest.py +++ b/.github/scripts/tests/test_render_digest.py @@ -120,7 +120,12 @@ def test_diverged_branch_warning_in_subject_and_body(): "skipped": False, "ahead_by": 30, "behind_by": 0, + "ahead_commits": [ + "chore(ci): created forward-port.yml from ledoent/.github distributor", + "[FIX] something real", + ], "diverged": True, + "managed_overlay": False, }, ], mig_buckets={}, @@ -133,6 +138,42 @@ def test_diverged_branch_warning_in_subject_and_body(): assert "Diverged from upstream" in body assert "ledoent/social" in body assert "30 ahead" in body + # Unmanaged subject is shown; managed one is suppressed. + assert "[FIX] something real" in body + assert "forward-port.yml from ledoent/.github distributor" not in body + + +def test_managed_overlay_is_quiet_footer_not_alarm(): + # The everyday state for install_forward_port forks: 1 ahead by the + # chore commit (+ possibly a sync merge). Must NOT alarm in subject + # or body, but should show a single-line footer for transparency. + subj, body, exit_marker = render_digest.render( + sync_results=[ + { + "repo": "ledoent/web", + "branch": "18.0", + "status": 200, + "message": "", + "merge_type": "none", + "skipped": False, + "ahead_by": 1, + "behind_by": 0, + "ahead_commits": [ + "chore(ci): created forward-port.yml from ledoent/.github distributor", + ], + "diverged": False, + "managed_overlay": True, + }, + ], + mig_buckets={}, + distribution=[], + ) + assert "diverged" not in subj.lower() + assert "fail" not in subj.lower() + assert exit_marker == "0" + assert "Diverged from upstream" not in body + assert "Managed overlay:" in body + assert "1 branches" in body def test_html_escaping_in_failure_message(): diff --git a/.github/scripts/tests/test_sync_branch_state.py b/.github/scripts/tests/test_sync_branch_state.py index 0132bac..576cf12 100644 --- a/.github/scripts/tests/test_sync_branch_state.py +++ b/.github/scripts/tests/test_sync_branch_state.py @@ -64,18 +64,32 @@ def _patch_gh_sequence(monkeypatch, *returns): monkeypatch.setattr(fork_sync_digest, "gh", lambda *a, **kw: next(it)) +def _commits(*subjects): + return [{"commit": {"message": s}} for s in subjects] + + def test_diverged_branch_is_flagged_when_upstream_org_provided(monkeypatch): # merge-upstream succeeds with a merge commit; compare reports 30 - # local commits ahead. This is the case that motivated the check — - # ledoent/social:18.0 in May 2026 silently accumulated drift because - # merge-upstream returns success either way. + # local commits ahead, one of which isn't a managed overlay. This + # is the case that motivated the check — ledoent/social:18.0 in + # May 2026 silently accumulated drift because merge-upstream + # returns success either way. _patch_gh_sequence( monkeypatch, (200, {"merge_type": "merge"}), - (200, {"ahead_by": 30, "behind_by": 0}), + (200, { + "ahead_by": 30, + "behind_by": 0, + "commits": _commits( + "chore(ci): created forward-port.yml from ledoent/.github distributor", + "Merge branch 'OCA:18.0' into 18.0", + "[FIX] some real human commit that shouldn't be here", + ), + }), ) r = fork_sync_digest.sync_branch("ledoent/social", "18.0", upstream_org="OCA") assert r["diverged"] is True + assert r["managed_overlay"] is False assert r["ahead_by"] == 30 assert r["behind_by"] == 0 @@ -84,13 +98,75 @@ def test_clean_fast_forward_is_not_diverged(monkeypatch): _patch_gh_sequence( monkeypatch, (200, {"merge_type": "fast-forward"}), - (200, {"ahead_by": 0, "behind_by": 0}), + (200, {"ahead_by": 0, "behind_by": 0, "commits": []}), ) r = fork_sync_digest.sync_branch("ledoent/x", "18.0", upstream_org="OCA") assert r["diverged"] is False + assert r["managed_overlay"] is False assert r["ahead_by"] == 0 +def test_only_distributor_chore_is_managed_not_diverged(monkeypatch): + # The steady state for install_forward_port forks: 1 ahead by the + # chore commit alone. Must NOT trip the diverged alarm. + _patch_gh_sequence( + monkeypatch, + (200, {"merge_type": "none"}), + (200, { + "ahead_by": 1, + "behind_by": 0, + "commits": _commits( + "chore(ci): created forward-port.yml from ledoent/.github distributor", + ), + }), + ) + r = fork_sync_digest.sync_branch("ledoent/web", "18.0", upstream_org="OCA") + assert r["diverged"] is False + assert r["managed_overlay"] is True + assert r["ahead_by"] == 1 + + +def test_chore_plus_sync_merge_is_managed_not_diverged(monkeypatch): + # After an upstream advance, the chore + a "Merge branch 'OCA:...'" + # land together. Still structural, still not divergence. + _patch_gh_sequence( + monkeypatch, + (200, {"merge_type": "merge"}), + (200, { + "ahead_by": 2, + "behind_by": 0, + "commits": _commits( + "chore(ci): created forward-port.yml from ledoent/.github distributor", + "Merge branch 'OCA:18.0' into 18.0", + ), + }), + ) + r = fork_sync_digest.sync_branch("ledoent/web", "18.0", upstream_org="OCA") + assert r["diverged"] is False + assert r["managed_overlay"] is True + assert r["ahead_by"] == 2 + + +def test_updated_chore_subject_is_also_managed(monkeypatch): + # When the distributor template content changes, the chore commit + # message switches from "created" to "updated". Both shapes must + # be recognised. + _patch_gh_sequence( + monkeypatch, + (200, {"merge_type": "none"}), + (200, { + "ahead_by": 1, + "behind_by": 0, + "commits": _commits( + "chore(ci): updated forward-port.yml from ledoent/.github distributor", + ), + }), + ) + r = fork_sync_digest.sync_branch("ledoent/web", "18.0", upstream_org="OCA") + assert r["managed_overlay"] is True + assert r["diverged"] is False + + def test_no_upstream_org_skips_divergence_check(monkeypatch): # Non-OCA forks (upstream_org: null) get no compare call. Verified by # patching gh to fail loudly if called more than once. @@ -120,6 +196,7 @@ def test_compare_failure_does_not_mask_successful_sync(monkeypatch): assert r["status"] == 200 assert r["merge_type"] == "fast-forward" assert r["diverged"] is False + assert r["managed_overlay"] is False assert r["ahead_by"] is None