Repository navigation
Report failed pull request runs, and state that a failed post reaches nobody - #14
Conversation
… 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 <Gavin@L337.org>
Claude review: changes requestedSummaryThe change adds One problem should be settled before merge. Because the hygiene check now requires every repository to report Findings1. Bug (security): fork pull requests can post text they choose to the organisation's Slack channel, and the hygiene check now makes every repository opt in Where:
Suggested fix: in the reporter's 2. Bug (minor): a pull request run cancelled by its timeout can be hidden by a different pull request's run Where:
3. Maintainability: the README's table still describes the reporter as posting only unwatched runs
4. Maintainability: the hygiene finding's example
Recorded-state checkI went through every added or edited line in the README, the workflow comments and input descriptions, and the script's comments and docstrings. None of them records state that will go out of date: they state rules, reasons, and accepted limitations named as limitations. The "Tested:" paragraph is in the commit message, which is the right place for it. What I checked and what I didn'tChecked:
Not checked:
full review of |
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 <Gavin@L337.org>
|
Addressed in 8fb9b45:
|
Claude review: approveDelta review of
|
What
pull_requestalongsideschedule,pushandrelease, so each repository must list its pull request workflows inreport-failures.yamlwithpull_requestamong the watcher'sevents. The reporter itself is unchanged: it posts any trigger the caller lists. Manual runs stay out.Effect on other repositories
Nothing changes in a repository until its pins move to the release carrying this. The pull request that moves them must also add
pull_requestto the watcher'seventsand list the repository's pull request workflows, or the hygiene check fails. A Dependabot pin bump alone would fail the same way.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. The reporter skips a
pull_requestrun whose head repository is not the repository itself.Testing
pull_requestmissing fromevents.