diff --git a/.github/workflows/auto-merge-to-develop.yml b/.github/workflows/auto-merge-to-develop.yml index 3179443..73ad91b 100644 --- a/.github/workflows/auto-merge-to-develop.yml +++ b/.github/workflows/auto-merge-to-develop.yml @@ -9,13 +9,65 @@ name: auto-merge-to-develop # check_suite-triggered workflows from the DEFAULT branch, so the CALLER # stub in each leaf repo must live on that repo's default branch (main) — # same requirement the original file had. +# +# *** A github.token MERGE FIRES NOTHING ON develop. *** GitHub suppresses +# workflow triggers for pushes made with the default workflow token +# (recursive-run protection), and `gh pr merge` below pushes the merge commit +# with exactly that token — so every commit this sweep lands arrives on +# develop with ZERO check runs. Measured across the scitex-ai org +# (2026-07-22, 74 repos enumerated): 58 repos carry at least one bot-merged +# develop head whose `GET /commits//check-runs` returns +# `total_count: 0` while their user-pushed neighbours on the same branch +# carry 5-19 each. Examples: figrecipe 3ebb01c0 -> 0 vs b33a8ed3 -> 19; +# scitex-io 49d709be -> 0 (4 of 4 sampled bot heads at 0) vs f9fe3dd5 -> 8; +# scitex-app 0e69dfd5 -> 0 (5 of 5) vs 827b75c9 -> 16. Runners were online +# and idle — not a capacity problem. +# +# WHY THAT IS WORSE THAN A RED BUILD: a downstream develop-health gate reads +# "no checks present" as "no red signal to honour" and keeps merging. +# Green-by-absence — the hole conceals itself, so a repo can sit in this +# state for months and look healthy. +# +# `workflow_dispatch` IS exempt from the suppression, so after a successful +# merge this workflow explicitly DISPATCHES develop's post-merge gates — +# ONCE per call, not once per merged PR, skipped on dry-run, and +# `::error::`-loud + red if a dispatch fails (an un-CI'd develop head is +# exactly the silence this block exists to end). Deliberately NO PAT and no +# bot token: explicit dispatch is the sanctioned workaround. +# +# Reference implementation this mirrors: scitex-ai/scitex-agent-container +# PR #809 (merged e947b450), carried to main via #811. on: - workflow_call: {} + workflow_call: + inputs: + dry_run: + description: >- + Report what would be merged/dispatched without doing it. + type: boolean + default: false + required: false + post_merge_gates: + description: >- + Space-separated workflow FILENAMES to dispatch on develop after a + merge (e.g. "pytest-matrix-on-ubuntu-py3-11-3-12-3-13.yml + quality-audit-on-ubuntu-latest.yml"). Leave empty to auto-discover + every active workflow on develop that accepts workflow_dispatch, + minus the deny-list below. Auto-discovery is the default because + leaf repos genuinely differ (scitex-io ships + scitex-io-quality-audit-on-ubuntu-latest.yml, scitex-writer ships + quality-audit-on-ubuntu-latest.yml) — a hardcoded org-wide list + would 404 on half the org and red every sweep. + type: string + default: "" + required: false permissions: contents: write pull-requests: write + # gh workflow run — without this every dispatch 403s and the sweep is + # right back to landing un-CI'd commits on develop. + actions: write jobs: automerge: @@ -27,8 +79,17 @@ jobs: GH_TOKEN: ${{ github.token }} REPO: ${{ github.repository }} HEAD_SHA: ${{ github.event.check_suite.head_sha }} + DRY_RUN: ${{ inputs.dry_run }} + POST_MERGE_GATES: ${{ inputs.post_merge_gates }} + # Workflows that must NEVER be auto-dispatched after a merge: + # the sweep itself (recursion), release/publish paths (they cut + # real artifacts), and the CLA bot (needs PR context). + GATE_DENY_RE: '^(auto-merge-to-develop|autobump-release-sweep|cla|pypi-publish|.*release.*|.*publish.*)' run: | set -uo pipefail + + merges=0 + if [ -n "${HEAD_SHA:-}" ]; then prs=$(gh api "repos/$REPO/commits/$HEAD_SHA/pulls" --jq '.[] | select(.state=="open") | .number' 2>/dev/null || true) else @@ -54,8 +115,78 @@ jobs: | (.conclusion // .state // "") | select(. != "SUCCESS" and . != "NEUTRAL" and . != "SKIPPED" and . != "") ] | length' 2>/dev/null || echo 1) if [ "$pending" = "0" ]; then - gh pr merge "$pr" --repo "$REPO" --merge --admin --delete-branch && echo "MERGED #$pr -> develop" || echo "blocked #$pr" + if [ "$DRY_RUN" = "true" ]; then + echo "DRY RUN: would merge #$pr -> develop" + merges=$((merges + 1)) + elif gh pr merge "$pr" --repo "$REPO" --merge --admin --delete-branch; then + echo "MERGED #$pr -> develop" + merges=$((merges + 1)) + else + echo "blocked #$pr" + fi else echo "PR #$pr not green ($pending) — skip" fi done + + echo "sweep complete: $merges merge(s) this run" + + # --------------------------------------------------------------- + # POST-MERGE CI DISPATCH — a github.token merge fires NOTHING. + # --------------------------------------------------------------- + # The merge commit we just pushed carries the default workflow + # token, so GitHub suppresses every push-triggered workflow on it + # (see header). Without this block develop's new head has ZERO + # check runs, and a develop-health gate reads that absence as "no + # red signal" — green-by-absence. workflow_dispatch is exempt from + # the suppression, so fire the gates explicitly. ONCE per call + # (merges is this call's total), never per PR. + if [ "$merges" -gt 0 ]; then + if [ "$DRY_RUN" = "true" ]; then + echo "DRY RUN: would dispatch post-merge gates on develop" + else + gates="$POST_MERGE_GATES" + if [ -z "$gates" ]; then + # Auto-discover: active workflows that exist ON develop and + # accept a bare workflow_dispatch. Reading the file from the + # develop ref (not main) is the point — a gate that only + # exists on main cannot check the head we just landed. + candidates=$(gh api "repos/$REPO/actions/workflows?per_page=100" \ + --jq '.workflows[] | select(.state=="active") | .path | sub("^\\.github/workflows/"; "")' \ + 2>/dev/null || true) + for wf in $candidates; do + echo "$wf" | grep -Eq "$GATE_DENY_RE" && continue + body=$(gh api "repos/$REPO/contents/.github/workflows/$wf?ref=develop" \ + --jq '.content' 2>/dev/null | base64 -d 2>/dev/null || true) + [ -z "$body" ] && continue + echo "$body" | grep -q 'workflow_dispatch' || continue + gates="$gates $wf" + done + fi + + if [ -z "$(echo "$gates" | tr -d '[:space:]')" ]; then + # A merge landed and NOTHING can check it. That is the + # green-by-absence state itself, not a clean result. + echo "::error::merged $merges PR(s) onto develop but found NO dispatchable post-merge gate. develop's new head is UN-CHECKED. Add a workflow_dispatch trigger to this repo's CI, or pass post_merge_gates explicitly." + exit 1 + fi + + echo "post-merge gates: $gates" + dispatch_failures=0 + for wf in $gates; do + if gh workflow run "$wf" --repo "$REPO" --ref develop; then + echo "dispatched $wf on develop" + else + # LOUD, never silent: an undispatched gate leaves develop's + # new head UN-CHECKED — the exact hole this block closes. + echo "::error::failed to dispatch $wf on develop — the merged head has NO CI of its own until something fires it" + dispatch_failures=$((dispatch_failures + 1)) + fi + done + if [ "$dispatch_failures" -gt 0 ]; then + echo "::error::$dispatch_failures post-merge gate dispatch(es) failed. The merge itself LANDED; this run is red ON PURPOSE so an un-CI'd develop head cannot pass unnoticed." + exit 1 + fi + echo "post-merge gates dispatched on develop" + fi + fi diff --git a/.github/workflows/self-test.yml b/.github/workflows/self-test.yml new file mode 100644 index 0000000..1c10d4c --- /dev/null +++ b/.github/workflows/self-test.yml @@ -0,0 +1,30 @@ +name: self-test + +# The reusable workflows in this repo are the org's shared CI implementation. +# A defect here is a defect in every repo that calls it — so the workflows +# themselves get tests, and those tests run on every change to this repo. +# +# tests/test_auto_merge_dispatch.py is file-only (parses the YAML, asserts on +# the shell body). No network, no gh, cannot flake. + +on: + push: + branches: [main, develop] + pull_request: + workflow_dispatch: + +permissions: + contents: read + +jobs: + pytest: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: install + run: python -m pip install --upgrade pip pytest pyyaml + - name: run workflow contract tests + run: python -m pytest tests/ -v diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..a160f49 --- /dev/null +++ b/.gitignore @@ -0,0 +1,3 @@ +__pycache__/ +*.py[cod] +.pytest_cache/ diff --git a/tests/test_auto_merge_dispatch.py b/tests/test_auto_merge_dispatch.py new file mode 100755 index 0000000..034398c --- /dev/null +++ b/tests/test_auto_merge_dispatch.py @@ -0,0 +1,242 @@ +"""The org auto-merge sweep must DISPATCH develop's post-merge gates itself. + +GitHub SUPPRESSES workflow triggers for pushes made with the default +``github.token`` (recursive-run protection), and auto-merge-to-develop.yml +merges with exactly that token — so every commit the sweep lands arrives on +develop with ZERO check runs. Measured across the scitex-ai org on +2026-07-22: 58 of 74 enumerated repos carry at least one bot-merged develop +head with ``check-runs total_count: 0`` while their user-pushed neighbours on +the same branch carry 5-19 each (figrecipe 3ebb01c0 -> 0 vs b33a8ed3 -> 19; +scitex-io 49d709be -> 0 vs f9fe3dd5 -> 8; scitex-app 0e69dfd5 -> 0 vs +827b75c9 -> 16), with runners online and idle. + +A downstream develop-health gate then reads "no checks" as "no red signal" +and keeps merging — green-by-absence. ``workflow_dispatch`` IS exempt from +the suppression, so the merge step must explicitly dispatch the gates after +a successful merge. + +These tests pin that wiring AS TEXT — no network, no gh, cannot flake. +Mutation-checked: reverting the workflow to its pre-fix body, neutering the +``gh workflow run`` line, or flipping ``--ref develop`` to ``--ref main`` +each turn at least one test red. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +import yaml + +_REPO = Path(__file__).resolve().parents[1] +_AUTO_MERGE = _REPO / ".github" / "workflows" / "auto-merge-to-develop.yml" + + +@pytest.fixture(scope="module") +def workflow() -> dict: + return yaml.safe_load(_AUTO_MERGE.read_text(encoding="utf-8")) + + +@pytest.fixture(scope="module") +def merge_step(workflow: dict) -> dict: + """The step that performs the merges (found by behaviour, not by name).""" + steps = workflow["jobs"]["automerge"]["steps"] + merging = [s for s in steps if "gh pr merge" in s.get("run", "")] + assert len(merging) == 1, "expected exactly one merging step" + return merging[0] + + +def _dispatch_lines(merge_step: dict) -> list[str]: + return [ + line for line in merge_step["run"].splitlines() if "gh workflow run" in line + ] + + +# --------------------------------------------------------------------------- +# The dispatch exists at all. +# --------------------------------------------------------------------------- + + +def test_merge_step_runs_workflow_dispatch(merge_step: dict) -> None: + # Arrange + run = merge_step["run"] + # Act + lines = [line for line in run.splitlines() if "gh workflow run" in line] + # Assert + assert lines, ( + "the merge step never runs `gh workflow run` — a github.token merge " + "fires no triggers, so develop's new head would sit un-checked" + ) + + +# --------------------------------------------------------------------------- +# THE WRONG-REF HOLE. `--ref main` is a dispatch that exists but checks the +# wrong branch — it would satisfy a naive "is there a dispatch?" test while +# leaving the merged develop head just as un-verified as before. +# --------------------------------------------------------------------------- + + +def test_every_dispatch_targets_develop(merge_step: dict) -> None: + # Arrange: non-emptiness is pinned by test_merge_step_runs_workflow_dispatch. + lines = _dispatch_lines(merge_step) + # Act + off_ref = [line for line in lines if "--ref develop" not in line] + # Assert + assert off_ref == [], ( + "a dispatch without `--ref develop` runs the gate against another " + f"branch, not the head the merge just produced: {off_ref}" + ) + + +def test_no_dispatch_targets_a_non_develop_ref(merge_step: dict) -> None: + # Arrange + lines = _dispatch_lines(merge_step) + # Act: catch the mutation from the other side — `--ref main` must never + # appear even alongside a correct `--ref develop` on some other line. + wrong = [line for line in lines if "--ref main" in line or "--ref master" in line] + # Assert + assert wrong == [], f"dispatch targets a non-develop ref: {wrong}" + + +def _discovery_lines(merge_step: dict) -> list[str]: + return [ + line + for line in merge_step["run"].splitlines() + if "contents/.github/workflows/" in line and "ref=" in line + ] + + +def test_gate_discovery_reads_workflow_files(merge_step: dict) -> None: + # Arrange + step = merge_step + # Act + discovery = _discovery_lines(step) + # Assert + assert discovery, "no gate-discovery read of .github/workflows found" + + +def test_gate_discovery_reads_the_develop_ref(merge_step: dict) -> None: + # Arrange: non-emptiness is pinned by the previous test. + discovery = _discovery_lines(merge_step) + # Act: auto-discovery must inspect workflow files as they exist ON + # develop. Discovering from main would happily "find" a gate that does + # not exist on the branch being merged into. + off_ref = [line for line in discovery if "ref=develop" not in line] + # Assert + assert off_ref == [], f"gate discovery reads a non-develop ref: {off_ref}" + + +# --------------------------------------------------------------------------- +# Reachable only AFTER a merge — once per call, never per PR, never dry. +# --------------------------------------------------------------------------- + + +def test_dispatch_is_guarded_by_the_merge_count(merge_step: dict) -> None: + # Arrange + run = merge_step["run"] + # Act: locate the end of the per-PR merge loop (the first column-0 `done` + # after `gh pr merge`), the merge-count guard, and the dispatch. + loop_end_at = run.index("\ndone\n", run.index("gh pr merge")) + guard_at = run.index('[ "$merges" -gt 0 ]') + dispatch_at = run.index("gh workflow run") + # Assert: the guard opens after the merge loop and before the dispatch, + # so N merges fan out to ONE dispatch round, not N. + assert loop_end_at < guard_at < dispatch_at, ( + "the dispatch must be guarded on `merges > 0` after the merge loop — " + "a per-PR or unconditional dispatch is the wrong shape" + ) + + +def test_dry_run_does_not_dispatch(merge_step: dict) -> None: + # Arrange + run = merge_step["run"] + # Act: the dry-run branch must be decided between the guard and the + # dispatch — a dry run merges nothing, so firing real CI from it would be + # dispatching gates for a merge that never happened. + guarded_block = run[run.index('[ "$merges" -gt 0 ]') : run.index("gh workflow run")] + # Assert + assert '"$DRY_RUN" = "true"' in guarded_block + + +def test_dispatch_failure_is_loud(merge_step: dict) -> None: + # Arrange + run = merge_step["run"] + # Act + after_guard = run[run.index('[ "$merges" -gt 0 ]') :] + # Assert: a failed dispatch leaves develop's new head UN-CHECKED — the + # precise silence this wiring exists to end — so it must go ::error:: + # loud, not vanish into an `|| true`. + assert "::error::" in after_guard, ( + "a failed gate dispatch must be loud (::error::), not silently " + "swallowed" + ) + + +def test_dispatch_failure_reds_the_run(merge_step: dict) -> None: + # Arrange + run = merge_step["run"] + # Act + after_guard = run[run.index('[ "$merges" -gt 0 ]') :] + # Assert + assert "exit 1" in after_guard, "a failed dispatch must red the run" + + +def test_empty_gate_set_is_an_error_not_a_pass(merge_step: dict) -> None: + # Arrange + run = merge_step["run"] + after_guard = run[run.index('[ "$merges" -gt 0 ]') :] + # Act: "we merged and found nothing to dispatch" IS the green-by-absence + # state. A check that could not run must never report what a check that + # passed reports. + empty_branch = [ + line + for line in after_guard.splitlines() + if "::error::" in line and "NO dispatchable" in line + ] + # Assert + assert empty_branch, ( + "an empty gate set must be a loud error — silently dispatching " + "nothing reproduces the exact hole this fix closes" + ) + + +# --------------------------------------------------------------------------- +# The token can actually honour the dispatch. +# --------------------------------------------------------------------------- + + +def test_workflow_token_may_dispatch(workflow: dict) -> None: + # Arrange + permissions = workflow.get("permissions", {}) + # Act: `gh workflow run` needs actions:write; without it every dispatch + # 403s and the sweep is back to landing un-CI'd commits. + granted = permissions.get("actions") + # Assert + assert granted == "write" + + +def _call_inputs(workflow: dict) -> dict: + # YAML 1.1 parses a bare `on:` key as boolean True — read both. + triggers = workflow.get("on", workflow.get(True, {})) + return (triggers.get("workflow_call") or {}).get("inputs") or {} + + +def test_dry_run_input_is_declared(workflow: dict) -> None: + # Arrange + doc = workflow + # Act + inputs = _call_inputs(doc) + # Assert + assert "dry_run" in inputs, "callers cannot request a dry run" + + +def test_post_merge_gates_input_is_declared(workflow: dict) -> None: + # Arrange + doc = workflow + # Act + inputs = _call_inputs(doc) + # Assert + assert "post_merge_gates" in inputs, ( + "callers cannot override auto-discovery — a repo whose gate names " + "the deny-list misjudges would have no escape hatch" + )