Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 80 additions & 1 deletion .github/workflows/auto-merge-docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,91 @@ jobs:
const docsOnly = files.data.length > 0 && files.data.every(f => isDocsFile(f.filename));

core.setOutput('docs-only', docsOnly);
// Named, so the refusal comment can say *which* file broke the rule instead of only that
// something did. Capped: a PR that breaks the rule in forty files does not need forty lines.
core.setOutput('offenders', files.data.filter(f => !isDocsFile(f.filename))
.map(f => f.filename).slice(0, 20).join('\n'));
core.info(`Docs-only: ${docsOnly} (${files.data.length} files)`);

# A judgement nobody can see did not happen. Until 2026-09-21 this workflow had **no** comment
# step at all (the lesson channel has three), so a refusal existed only as one `core.info` line
# inside a green run: #1801 was given the maintainer's opt-in label four times on 2026-09-19, the
# gate ran and refused it (`Docs-only: false (3 files)`), and from the author's side nothing ever
# happened — the PR sat there labelled and untouched for three days.
- name: Report the refusal on the pull request
if: steps.check.outputs.docs-only != 'true'
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9
env:
OFFENDERS: ${{ steps.check.outputs.offenders }}
with:
script: |
const marker = '<!-- auto-merge-docs -->';
const offenders = (process.env.OFFENDERS || '').split('\n').filter(Boolean);
const listed = offenders.length
? offenders.map(f => `- \`${f}\``).join('\n')
: '- (the file list came back empty)';
const body = [
marker,
'### ⏭️ Auto-merge skipped: this PR is not docs-only',
'',
'The `auto-merge-eligible` label is on, so this gate looked at the PR — and stopped here,',
'because the docs-only rule was not satisfied. A PR qualifies only when **every** file is',
'under `docs/` (or is one of `CONTRIBUTING.md`, `README.md`, `JOIN.md`, `CHANGELOG.md`).',
'`lessons/**`, `docs/.well-known/**` and `docs/index.html` are never auto-merged: lesson',
'content is executed by agents, and those two paths are filed as machine input and the',
'site frame.',
'',
'Files that broke the rule:',
'',
listed,
'',
'**What to do.** Either split the non-docs files into a separate PR, or — if the PR is',
'docs-only after all, because the list above looks wrong — remove and re-apply the',
'`auto-merge-eligible` label. That re-label is not busywork: this gate only re-evaluates on',
'`labeled`, `unlabeled`, `edited`, `synchronize` and `ready_for_review`, so a corrected PR',
'that is pushed *after* the label was applied does get looked at again, while one that was',
'already pushed does not.',
'',
`_Posted by [the docs auto-merge gate](${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/blob/main/.github/workflows/auto-merge-docs.yml); this comment is updated in place, not stacked._`,
].join('\n');

const issue_number = context.payload.pull_request.number;
const { data: comments } = await github.rest.issues.listComments({
owner: context.repo.owner, repo: context.repo.repo, issue_number, per_page: 100,
});
const previous = comments.find(c => (c.body || '').includes(marker));
if (previous) {
await github.rest.issues.updateComment({
owner: context.repo.owner, repo: context.repo.repo, comment_id: previous.id, body,
});
core.info(`Updated the refusal comment on #${issue_number}`);
} else {
await github.rest.issues.createComment({
owner: context.repo.owner, repo: context.repo.repo, issue_number, body,
});
core.info(`Posted the refusal comment on #${issue_number}`);
}

- name: Enable auto-merge
if: steps.check.outputs.docs-only == 'true'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
# `SHELDON_PAT`, not `GITHUB_TOKEN`, and this is not a preference.
#
# A merge performed with `GITHUB_TOKEN` produces a push that **starts no workflows**, so the
# whole post-merge chain silently does not run for PRs merged here: measured on 2026-09-21,
# the merge commit of #1969 (this gate, GITHUB_TOKEN) has **0** push-triggered runs, while
# #1965 (merged with the PAT) and #1962 have 5 each — `Release Please`, `Leaderboard Watch`,
# `Build Live Feed`, `Update Badge Counts`, `Deploy Documentation` and the rest all keyed on
# `push`. The repository has paid for this lesson twice already (`auto-sync-prs.yml`, and
# `lessons/contrib/ci-github-token-push-does-not-trigger-workflows.md`); this was the third
# place.
#
# It does **not** need the `workflow` scope: GitHub refuses a PAT-without-`workflow` only for
# PRs that modify `.github/workflows/`, and by the rule above this gate merges only docs
# (see the docs-only check: nothing outside `docs/` plus four root files ever gets here).
# Do not widen this to `workflow` "just in case" — that would let this channel merge a change
# to CI itself.
GH_TOKEN: ${{ secrets.SHELDON_PAT }}
run: |
PR_NUM="${{ github.event.pull_request.number }}"

Expand Down
122 changes: 122 additions & 0 deletions tests/test_auto_merge_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,125 @@ def test_the_rule_returns_what_the_gate_needs_for_each_path(tmp_path):
"the docs-only rule misclassifies paths:\n" + (result.stderr or result.stdout)
)
assert "as expected" in result.stdout

# ── the two gaps that share this file: what the merge does downstream, and what a refusal says ───
#
# Both were measured on 2026-09-21, and they fail in opposite directions: the merge was invisible to
# the rest of the repository, and the refusal was invisible to the person who opted in.
#
# Each check is a function over the workflow *text*, so the mutation cases below feed a mutated copy
# through the same code the repository is judged by — asserting "the mutation took" and then asserting
# something trivially true is not a guard, it is decoration.


def _steps_of(text: str) -> list[dict]:
return yaml.safe_load(text)["jobs"]["auto-merge"]["steps"]


def _step_named(text: str, name: str) -> dict:
for step in _steps_of(text):
if step.get("name") == name:
return step
raise AssertionError(f"no step named {name!r} — the steps are {[s.get('name') for s in _steps_of(text)]}")


def _executable(step: dict) -> str:
"""A step's code, comments stripped.

The comments in this workflow name the constructs the rules forbid (that is what they are for), so
a check that reads the raw text is satisfied by the explanation of the bug it is looking for.
"""
text = (step.get("run") or "") + "\n" + (step.get("with", {}).get("script") or "")
return "\n".join(line for line in text.splitlines() if not line.strip().startswith("#"))


def merge_token_problems(text: str) -> list[str]:
"""`GITHUB_TOKEN` on the merge, or a token widened to the `workflow` scope."""
problems = []
step = _step_named(text, "Enable auto-merge")
token = (step.get("env") or {}).get("GH_TOKEN", "")
if "SHELDON_PAT" not in token:
problems.append(f"the merge does not use the PAT (GH_TOKEN={token!r})")
if "GITHUB_TOKEN" in token:
problems.append(
"a `GITHUB_TOKEN` merge produces a push that starts no workflows, so the post-merge chain "
"(Release Please, Leaderboard Watch, the count/badge mirrors, docs deploy) never runs")
if "workflow" in json.dumps(step.get("env") or {}):
problems.append(
"the PAT must not need the `workflow` scope: this gate merges docs only, and that scope "
"would let an auto-merged PR change CI itself")
return problems


def refusal_report_problems(text: str) -> list[str]:
"""The refusal path must post (and update) a comment that names the reason and the way back in."""
problems = []
reporting = [s for s in _steps_of(text)
if (s.get("if") or "").strip() == "steps.check.outputs.docs-only != 'true'"]
if not reporting:
return ["no step runs when docs-only is false: the gate can refuse a PR carrying the "
"maintainer's opt-in label and leave no trace anywhere a human looks"]
code = _executable(reporting[0])
if "createComment" not in code or "updateComment" not in code:
problems.append(
"the comment must be upserted (create + update): the gate wakes on labeled/unlabeled/"
"edited/synchronize, so one PR can be evaluated many times")
if "OFFENDERS" not in code and "offenders" not in code:
problems.append("the refusal must name the files that broke the rule")
if "label" not in code:
problems.append(
"the refusal must say how to be re-evaluated (re-apply `auto-merge-eligible`): "
"eligibility can change without a push, and the reader cannot guess which events wake "
"this gate")
if "pr merge" in code:
problems.append("the refusal path must not be able to merge")
# A refusal that cannot see the check step's output cannot name the files.
if "steps.check.outputs.offenders" not in json.dumps(reporting[0]):
problems.append("the reporting step does not read the offender list from the check step")
return problems


@pytest.mark.parametrize("check", [merge_token_problems, refusal_report_problems])
def test_the_two_gaps_stay_closed(check):
text = WORKFLOW.read_text(encoding="utf-8")
assert check(text) == [], "; ".join(check(text))


def _mutate(tmp_path, mutate) -> str:
text = WORKFLOW.read_text(encoding="utf-8")
mutated = mutate(text)
assert mutated != text, "the mutation did not take — a mutation that does not mutate asserts nothing"
return mutated


def test_reverting_the_merge_token_is_caught(tmp_path):
mutated = _mutate(tmp_path, lambda t: t.replace("GH_TOKEN: ${{ secrets.SHELDON_PAT }}",
"GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}"))
assert merge_token_problems(mutated), "putting `GITHUB_TOKEN` back must be caught"


def test_granting_the_workflow_scope_is_caught(tmp_path):
"""The tempting "just in case" edit: it would let this channel merge a change to CI."""
mutated = _mutate(tmp_path, lambda t: t.replace(
"GH_TOKEN: ${{ secrets.SHELDON_PAT }}",
"GH_TOKEN: ${{ secrets.WORKFLOW_PAT }}\n WIDENED: workflow"))
problems = merge_token_problems(mutated)
assert any("workflow" in p for p in problems), problems


def test_dropping_the_refusal_report_is_caught(tmp_path):
def drop(text: str) -> str:
lines = text.splitlines(keepends=True)
start = next(i for i, l in enumerate(lines) if "name: Report the refusal on the pull request" in l)
end = next(i for i, l in enumerate(lines) if "name: Enable auto-merge" in l)
return "".join(lines[:start - 1] + lines[end:])

assert refusal_report_problems(_mutate(tmp_path, drop)), (
"removing the reporting step must leave the rule with nothing to point at")


def test_a_stacked_comment_is_caught(tmp_path):
"""Regression shape: a refusal that only creates comments stacks one per event on a busy PR."""
mutated = _mutate(tmp_path, lambda t: t.replace("updateComment", "createComment"))
problems = refusal_report_problems(mutated)
assert any("upsert" in p for p in problems), problems
Loading