From de52f7cec631c9f97c1b2d02cf8ba26e8676b5e6 Mon Sep 17 00:00:00 2001 From: Gavin Lucas Date: Fri, 9 Oct 2026 16:25:32 +0100 Subject: [PATCH 1/2] Report failed pull request runs, and state that a failed post reaches nobody A failing pull request run is worth a post too: such failures are rare, and the run's page is easy to miss. The hygiene check now counts `pull_request` with `schedule`, `push` and `release`, so every repository must list its pull request workflows in its watcher with that trigger among its `events`, and the check's label and messages no longer call these runs unattended. The reporter needs no change: it already posts any trigger the caller lists. A manual run is still left out, because whoever started it is watching. A pull request from a fork posts as well, through the base repository's own watcher and secret, once GitHub has let its checks run. The README and the reporter's header said a broken webhook shows as a red watcher run "rather than a failure that reached nobody". The watcher's run is itself unwatched and nothing reports it, so a webhook that stops working is noticed only by the absence of posts. Both now state that as an accepted limitation, with the reasons the alternatives were not taken. This repository's own watcher lists its Code review workflow, which the extended check found. Tested: the check's function against eighteen fixtures, including a listed pull request workflow, one left off, and `pull_request` missing from `events`; and the whole script over a fresh clone of each repository, where it names exactly the pull request workflows each has yet to list. Signed-off-by: Gavin Lucas --- .github/workflows/report-failures.yaml | 3 +- .github/workflows/slack-on-failure.yaml | 10 +++--- README.md | 36 +++++++++++-------- actions/repo-hygiene/check-repo-hygiene.py | 41 +++++++++++----------- 4 files changed, 48 insertions(+), 42 deletions(-) diff --git a/.github/workflows/report-failures.yaml b/.github/workflows/report-failures.yaml index 7483ac6..f161b7e 100644 --- a/.github/workflows/report-failures.yaml +++ b/.github/workflows/report-failures.yaml @@ -6,6 +6,7 @@ on: workflow_run: workflows: - 'Check inbound changes' + - 'Code review' types: [completed] permissions: @@ -18,6 +19,6 @@ jobs: name: Report to Slack uses: ./.github/workflows/slack-on-failure.yaml with: - events: '["schedule", "push", "release"]' + events: '["schedule", "push", "release", "pull_request"]' secrets: SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }} diff --git a/.github/workflows/slack-on-failure.yaml b/.github/workflows/slack-on-failure.yaml index e1d486e..210b508 100644 --- a/.github/workflows/slack-on-failure.yaml +++ b/.github/workflows/slack-on-failure.yaml @@ -1,8 +1,7 @@ name: 'Report a failed run to Slack' # Called from a repository's watcher workflow, which is started by `workflow_run` when one of the -# workflows it names completes. A run nobody is watching - started by a schedule, a push to the -# default branch or a release - that fails is otherwise one red entry in a list nobody opens; +# workflows it names completes. A failed run is otherwise one red entry in a list nobody opens; # this posts it to the channel behind `SLACK_WEBHOOK` instead. # # The caller says which triggers to report with `events`. A push is reported on the default @@ -12,13 +11,14 @@ name: 'Report a failed run to Slack' # nobody anticipated is posted rather than passed over. The filters are here, not in each # caller, so no caller can get them wrong. # -# A post Slack does not accept fails this job, so a broken webhook is a red run of the watcher -# rather than a failure that silently reached nobody. +# A post Slack does not accept fails this job, with Slack's answer in the error. Nothing +# reports the watcher's own run, so a webhook that stops working is noticed only by the absence +# of posts: an accepted limitation (README.md). on: workflow_call: inputs: events: - description: 'The triggers whose failed runs are reported, as a JSON list of event names, such as `["schedule", "push", "release"]`. `pull_request` is accepted but posts every red push to every pull request.' + description: 'The triggers whose failed runs are reported, as a JSON list of event names, such as `["schedule", "push", "release", "pull_request"]`. A manual run, `workflow_dispatch`, is best left out, because whoever started it is watching.' type: string required: true secrets: diff --git a/README.md b/README.md index f7a27d3..b435419 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ the bumps. | Path | What it is | Call it as | |---|---|---| -| `actions/repo-hygiene` | Composite action: the conventions every repository shares - no internal references, every CI job bounded, every unattended run reporting its failures, the instruction layer intact, the detail layer routed. | a step, after `actions/checkout` | +| `actions/repo-hygiene` | Composite action: the conventions every repository shares - no internal references, every CI job bounded, every failed run reported, the instruction layer intact, the detail layer routed. | a step, after `actions/checkout` | | `actions/action-pins` | Composite action: every `uses:` names a 40-hex commit SHA. | a step, after `actions/checkout` | | `.github/workflows/claude-review.yaml` | Reusable workflow: a code review by Claude, full on the first round and a delta of the new commits after that. | a job with `uses:` | | `.github/workflows/slack-on-failure.yaml` | Reusable workflow: posts a failed run nobody was watching to Slack, naming the workflow and its failed jobs and linking the run. | a job with `uses:`, in a workflow triggered by `workflow_run` | @@ -101,9 +101,10 @@ inputs are at the top of `.github/workflows/claude-review.yaml`. ## Reporting failed runs to Slack -A run started by a schedule, a push or a release has nobody watching it, so its failure is -posted to Slack instead. Each repository has one watcher workflow, `report-failures.yaml`, -naming the workflows to report; adding such a workflow means adding its `name:` there. +A failed run is otherwise one red entry in a list nobody opens, so a failure started by a +schedule, a push, a release or a pull request is posted to Slack instead. Each repository has +one watcher workflow, `report-failures.yaml`, naming the workflows to report; adding such a +workflow means adding its `name:` there. ```yaml name: 'Report failures' @@ -111,6 +112,7 @@ on: workflow_run: workflows: - 'Check inbound changes' + - 'Code review' - 'Release' - 'Nightly fuzzing' types: [completed] @@ -122,7 +124,7 @@ jobs: name: Report to Slack uses: L337-org/github-workflows/.github/workflows/slack-on-failure.yaml@ # vX.Y.Z with: - events: '["schedule", "push", "release"]' + events: '["schedule", "push", "release", "pull_request"]' secrets: SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }} ``` @@ -130,25 +132,29 @@ jobs: - **What is reported.** A completed run of a listed workflow started by one of the `events`, with any conclusion but success, skipped or neutral, so a cancelled or timed-out run is posted as well as a failed one. A push is reported on the default branch and on a tag; a - push to another branch has its pusher watching. A pull request's run is watched by its - author and a manual run by whoever started it, so neither belongs in `events`. The post - names the repository, the workflow, its trigger, its conclusion and the jobs that did not + push to another branch has its pusher watching. A manual run is watched by whoever started + it, so it does not belong in `events`. A pull request's run is posted whichever repository + its branch is in, so a fork's failing checks post too, once GitHub has let them run. The + post names the repository, the workflow, its trigger, its conclusion and the jobs that did not succeed, and links the run. - **A cancelled run that a newer one replaced is not posted**, as when `cancel-in-progress` cancels the first of two merges landing together: the newer run is reported if it fails. **Accepted limitation:** a run cancelled by its timeout while a newer run of the same workflow was already queued looks the same, so it is not posted either. -- **The hygiene check holds the list.** It fails on a workflow triggered by `schedule`, `push` - or `release` that no watcher reports for that trigger, on one with no `name:`, and on a - listed name no workflow has, since GitHub matches on the name and a rename leaves the list - reporting nothing. +- **The hygiene check holds the list.** It fails on a workflow triggered by `schedule`, `push`, + `release` or `pull_request` that no watcher reports for that trigger, on one with no `name:`, + and on a listed name no workflow has, since GitHub matches on the name and a rename leaves the + list reporting nothing. - **Where it goes.** To the channel of the incoming webhook in the `SLACK_WEBHOOK` secret. A secret set on the organisation is every repository's default, and a repository secret of the same name overrides it. An incoming webhook posts to one channel and can do nothing else. - **A post that fails, fails the watcher's run**, with Slack's answer in the error: a missing - secret, a webhook Slack refuses, or Slack unreachable after retries. So a broken webhook shows - as a red watcher run rather than a failure that reached nobody. A transient refusal is - retried, which can post the same failure twice. + secret, a webhook Slack refuses, or Slack unreachable after retries. A transient refusal is + retried, which can post the same failure twice. **Accepted limitation:** the watcher's run is + itself a run nobody is watching, and nothing reports it, so a webhook that stops working is + noticed only by the absence of posts. Filing an issue instead would scatter them across + repositories if filed in the failing one, and if filed here would need either a credential + that can write across repositories or a scheduled sweep of every repository's watcher runs. - **The watcher only fires from the default branch**, where GitHub reads `workflow_run` triggers. So a change to it is tested only once merged. - **The caller must grant** `actions: read` and `contents: read`, as the example does, or diff --git a/actions/repo-hygiene/check-repo-hygiene.py b/actions/repo-hygiene/check-repo-hygiene.py index 7867bd3..98b9281 100755 --- a/actions/repo-hygiene/check-repo-hygiene.py +++ b/actions/repo-hygiene/check-repo-hygiene.py @@ -106,10 +106,10 @@ BINARY_SUFFIXES = (".png", ".jpg", ".jpeg", ".gif", ".ico", ".pdf", ".gz", ".zip", ".deb", ".whl") -# --- unattended runs report their failures -------------------------------------------------- -# The triggers of a run nobody is watching. A pull request's run is watched by its author, and -# a manual run by whoever started it. -UNATTENDED = ("schedule", "push", "release") +# --- failed runs are reported ----------------------------------------------------------------- +# The triggers whose failed runs are posted to Slack. A manual run is left out, because whoever +# started it is watching. +REPORTED = ("schedule", "push", "release", "pull_request") # A call to the shared Slack reporter, from another repository or from this one. REPORTER = re.compile( @@ -522,8 +522,8 @@ def workflow_triggers(relative, document): raise CannotEvaluate(f"{relative} has no `on:` this check can read (got {on!r})") -def unattended_run_findings(documents): - """The findings of check_unattended_runs_report_failures, over parsed workflows. +def reported_run_findings(documents): + """The findings of check_failed_runs_are_reported, over parsed workflows. Args: documents (list): `(repo-relative path, document)` pairs, as workflow_documents returns. @@ -580,22 +580,22 @@ def unattended_run_findings(documents): name = document.get("name") if isinstance(name, str): named.add(name) - unattended = workflow_triggers(relative, document) & set(UNATTENDED) - if not unattended: + triggers = workflow_triggers(relative, document) & set(REPORTED) + if not triggers: continue if not isinstance(name, str) or not name: findings.append( - f"{relative} runs on {', '.join(sorted(unattended))} but has no `name:`, which a " + f"{relative} runs on {', '.join(sorted(triggers))} but has no `name:`, which a " f"watcher's `workflows:` list matches on" ) continue - missing = unattended - reported.get(name, set()) + missing = triggers - reported.get(name, set()) if missing: where = " or ".join(str(w) for w in watchers) if watchers else "a watcher workflow" findings.append( - f"{relative} ({name!r}) runs on {', '.join(sorted(missing))} unattended, and " - f"{where} does not report its failures there: list it in `workflows:` and those " - f"triggers in `events:`" + f"{relative} ({name!r}) runs on {', '.join(sorted(missing))}, and {where} does " + f"not report its failures there: list it in `workflows:` and those triggers in " + f"`events:`" ) for name in sorted(set(reported) - named): findings.append( @@ -605,13 +605,12 @@ def unattended_run_findings(documents): return findings -def check_unattended_runs_report_failures(root, tracked, files): - """Every workflow that runs with nobody watching has its failures posted to Slack. +def check_failed_runs_are_reported(root, tracked, files): + """Every workflow started by a schedule, a push, a release or a pull request reports failures. - A run started by a schedule, a push or a release has no pull request to show its failure - on, so a failure is one red entry in a list nobody opens. Each such workflow must be named - in a watcher - a workflow triggered by `workflow_run` that calls the shared Slack reporter - - with each of its unattended triggers among the `events` the watcher reports. A name in a + A failed run is otherwise one red entry in a list nobody opens, so each such workflow must + be named in a watcher - a workflow triggered by `workflow_run` that calls the shared Slack + reporter - with each of those triggers among the `events` the watcher reports. A name in a watcher's list that no workflow has is reported too, because GitHub matches on the name and a rename leaves the list reporting nothing. @@ -627,7 +626,7 @@ def check_unattended_runs_report_failures(root, tracked, files): CannotEvaluate: A workflow could not be read or parsed, via workflow_documents, or its `on:` could not be read. """ - return unattended_run_findings(workflow_documents(root, tracked)) + return reported_run_findings(workflow_documents(root, tracked)) # Markdown links plus backtick-quoted paths, since a router routes both ways. @@ -833,7 +832,7 @@ def check_the_detail_layer_routing(root, tracked, files): ("the scan is real", check_the_scan_is_real), ("no internal references in a public repository", check_no_internal_references), ("every CI job is bounded", check_ci_jobs_are_bounded), - ("unattended runs report their failures", check_unattended_runs_report_failures), + ("failed runs are reported", check_failed_runs_are_reported), ("the instruction layer is intact", check_the_instruction_layer), ("the detail layer and the router agree", check_the_detail_layer_routing), ) From 8fb9b45d64c5dafef9bf6822921e74f781f3ef6a Mon Sep 17 00:00:00 2001 From: Gavin Lucas Date: Fri, 9 Oct 2026 16:31:21 +0100 Subject: [PATCH 2/2] Do not post failed pull request runs from forks A pull request from a fork runs the fork's own copy of the workflow file, so its author writes the job names and the branch name the post would carry. The reporter escapes Slack's markup, but a bare URL still becomes a link, so a fork could put text of its choosing - a fake sign-in prompt, say - in the channel through the repository's own webhook, once its runs are allowed to start. The reporter now skips a `pull_request` run whose head repository is not the repository itself, as the review refuses forks for the same reason. Its failure still shows on the pull request. This also removes the case where a fork's run on a common branch name, such as `patch-1`, hid another pull request's cancelled run. The README's table no longer describes the reporter as posting only unwatched runs, and the hygiene check's example `events` now includes `pull_request`, matching the others. Signed-off-by: Gavin Lucas --- .github/workflows/slack-on-failure.yaml | 7 ++++++- README.md | 11 ++++++----- actions/repo-hygiene/check-repo-hygiene.py | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/.github/workflows/slack-on-failure.yaml b/.github/workflows/slack-on-failure.yaml index 210b508..b27c71f 100644 --- a/.github/workflows/slack-on-failure.yaml +++ b/.github/workflows/slack-on-failure.yaml @@ -6,7 +6,10 @@ name: 'Report a failed run to Slack' # # The caller says which triggers to report with `events`. A push is reported on the default # branch and on a tag, which is how a release is often started; a push to any other branch has -# its pusher watching. Only a completed run is reported, whatever `types:` the caller gives +# its pusher watching. A pull request from a fork is not reported: its run uses the fork's own +# workflow file, so its author writes the job names and branch the post would carry, and could +# put text of their choosing in the channel through this webhook. The same reasoning has the +# review refuse forks. Only a completed run is reported, whatever `types:` the caller gives # `workflow_run`, and with every conclusion but success, skipped and neutral, so a conclusion # nobody anticipated is posted rather than passed over. The filters are here, not in each # caller, so no caller can get them wrong. @@ -34,6 +37,8 @@ jobs: github.event_name != 'workflow_run' || (github.event.action == 'completed' && contains(fromJSON(inputs.events), github.event.workflow_run.event) && + (github.event.workflow_run.event != 'pull_request' || + github.event.workflow_run.head_repository.full_name == github.repository) && !contains(fromJSON('["success", "skipped", "neutral"]'), github.event.workflow_run.conclusion)) runs-on: [ubuntu-latest] name: Report the failure to Slack diff --git a/README.md b/README.md index b435419..ad300de 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ the bumps. | `actions/repo-hygiene` | Composite action: the conventions every repository shares - no internal references, every CI job bounded, every failed run reported, the instruction layer intact, the detail layer routed. | a step, after `actions/checkout` | | `actions/action-pins` | Composite action: every `uses:` names a 40-hex commit SHA. | a step, after `actions/checkout` | | `.github/workflows/claude-review.yaml` | Reusable workflow: a code review by Claude, full on the first round and a delta of the new commits after that. | a job with `uses:` | -| `.github/workflows/slack-on-failure.yaml` | Reusable workflow: posts a failed run nobody was watching to Slack, naming the workflow and its failed jobs and linking the run. | a job with `uses:`, in a workflow triggered by `workflow_run` | +| `.github/workflows/slack-on-failure.yaml` | Reusable workflow: posts a failed run to Slack, naming the workflow and its failed jobs and linking the run. | a job with `uses:`, in a workflow triggered by `workflow_run` | ## Using them @@ -133,10 +133,11 @@ jobs: with any conclusion but success, skipped or neutral, so a cancelled or timed-out run is posted as well as a failed one. A push is reported on the default branch and on a tag; a push to another branch has its pusher watching. A manual run is watched by whoever started - it, so it does not belong in `events`. A pull request's run is posted whichever repository - its branch is in, so a fork's failing checks post too, once GitHub has let them run. The - post names the repository, the workflow, its trigger, its conclusion and the jobs that did not - succeed, and links the run. + it, so it does not belong in `events`. A pull request from a fork is not posted: its run + uses the fork's own workflow file, so its author writes the job names and branch a post + would carry, and could put text of their choosing in the channel. Its failure still shows + on the pull request. The post names the repository, the workflow, its trigger, its + conclusion and the jobs that did not succeed, and links the run. - **A cancelled run that a newer one replaced is not posted**, as when `cancel-in-progress` cancels the first of two merges landing together: the newer run is reported if it fails. **Accepted limitation:** a run cancelled by its timeout while a newer run of the same workflow diff --git a/actions/repo-hygiene/check-repo-hygiene.py b/actions/repo-hygiene/check-repo-hygiene.py index 98b9281..858eb8a 100755 --- a/actions/repo-hygiene/check-repo-hygiene.py +++ b/actions/repo-hygiene/check-repo-hygiene.py @@ -567,7 +567,7 @@ def reported_run_findings(documents): if not (isinstance(events, list) and all(isinstance(e, str) for e in events)): findings.append( f"{relative} passes the Slack reporter `events: {raw!r}`, which is not a JSON " - f"list of event names such as '[\"schedule\", \"push\", \"release\"]'" + f"list of event names such as '[\"schedule\", \"push\", \"release\", \"pull_request\"]'" ) continue for name in names: