Make a stopped apply schedule visible on a pull request (#23)

[AcoPiper]

Summary

A consumer's scheduled apply can stop without anybody noticing — GitHub disables a scheduled workflow in a repository nobody has touched for 60 days, and does it silently. The only symptom is the absence of pull requests nobody was expecting on a particular day.

check-drift.yml runs on every pull request in every consumer and already reads the pin, so it now resolves the latest release too and emits a ::warning:: annotation when the pin is behind. The check still passes: failing because a release exists upstream would break pull requests that have nothing to do with the instructions, which is what pinning is for, and a check that cries wolf gets ignored.

It is silent in four cases, because a warning that is always there is furniture:

• The repository is on the latest release.
• The pin is ahead of the latest release — somebody trying an unreleased tag by hand, whom dispatching the apply would move backwards.
• The branch shared-instructions/<latest release> already exists — the apply pushed it, so the schedule ran, and an unmerged update pull request is a different situation. The branch rather than an open pull request, because listing pull requests needs pull-requests: read, and a called workflow can only narrow the caller's token — every consumer's ci.yml would have to grant it by hand, in the one directory nothing upstream may write. A branch is a ref, which contents: read already covers. The whole branch name, matched whole: git ls-remote matches its pattern at slash boundaries, so somebody's feature/shared-instructions/<latest> comes back in the listing and must not answer for the apply.
• The branch listing could not be fetched. An empty listing says the apply pushed nothing; an unreadable one rules nothing out, so it is passed as --branches-unknown rather than as no branches — otherwise a remote hiccup warns the repository whose update branch is sitting right there. The warning repeats on every pull request, so the skipped run loses nothing permanent. An unresolvable latest release is the same: the step says so in the log and leaves the pull request alone.

The comparison is scripts/check_drift.py, driven from the workflow the way pin_file.py and render.py already are, since every test here exercises a script and none exercises a workflow file. scripts/test_check_drift.py covers it, and ci.yml runs it.

Files: .github/workflows/check-drift.yml, .github/workflows/ci.yml, scripts/check_drift.py (new), scripts/test_check_drift.py (new), README.md, CHANGELOG.md.

Closes #23

Test plan

• A consumer whose pin is behind the latest release sees a warning annotation on its pull requests, and its checks still pass
• A consumer on the latest release sees nothing
• A consumer pinned ahead of the latest release sees nothing, and the log names the latest release rather than calling the pin it
• A consumer that already has the shared-instructions/<latest release> branch sees nothing
• A consumer holding only an older shared-instructions/* branch still gets warned, as does one holding a feature/shared-instructions/<latest> that only ends in the apply's branch name
• A consumer whose branch listing could not be fetched sees nothing — green and annotation-free — and the log names the branch it could not rule out
• The warning names the pin, the latest release, and the "Update shared instructions" workflow to dispatch, in one line
• Tags are ordered as numbers, not text, and across differing depth: 0.9.0 is behind 0.10.0, 1.2 and 1.2.0 are the same release, and the pre-scheme v1 is behind
• python3 scripts/test_check_drift.py passes, covering every outcome
• ci.yml runs test_check_drift.py
• The step cannot turn a pull request red: unresolvable release, unfetchable branch listing, and a full pipe under pipefail — including on the quiet unknown-listing path, which never reads the listing it was handed — all leave the job green, and continue-on-error catches whatever else
• The README documents it, beside the empty-Checks-tab section

[AcoPiper]

[Reviewer Round 1]

• [P1] Do not treat an unreadable branch list as an empty one. At .github/workflows/check-drift.yml:137, a transient git ls-remote failure assigns branches="" and invokes the script, which warns whenever the pin is behind. That produces the exact false-positive the branch exception is meant to avoid when shared-instructions/<latest> already exists (for example, a just-pushed, unmerged apply PR during a remote/authentication hiccup). The release lookup is correctly fail-open at lines 126–130; make the branch lookup do the same—log that it could not be checked and exit successfully without emitting the drift annotation. The stated test plan covers this failure path only for a green exit, so add an assertion that it remains annotation-free.

[AcoPiper]

[Review Verdict Round 1: NOT_APPROVED]

[AcoPiper]

[Author Round 1]

• [P1] Do not treat an unreadable branch list as an empty one — Fixed. Agreed, and the old comment in the workflow talked itself into the wrong trade: it argued that the worst a stale listing does is warn a repository whose update is already waiting, which is precisely the false positive the branch exception exists to remove. The asymmetry is not between "warning" and "red build", it is between one skipped run of a warning that reappears on the next pull request, and a warning that shows up wrongly and teaches its reader to skim past it.

  Where the fix went is the one place I deviated from the suggestion. Rather than an exit 0 in the YAML mirroring the release lookup above it, git ls-remote failing now passes --branches-unknown to scripts/check_drift.py, and the script takes the quiet path — because "unknown" is a third input state to the same decision, not a shell detail, and this issue's scope says logic written inline in the YAML has nowhere to be tested from. That is also what makes the assertion you asked for possible, which an exit 0 in the workflow would not have been.

  • .github/workflows/check-drift.yml: unknown="--branches-unknown" on a non-zero exit from ls-remote, with a log line, in place of branches="" standing in for it.
  • scripts/check_drift.py: the flag, and a quiet path that names the branch it could not rule out (pinned to 0.1.0, and this repository's branches could not be listed: not warning, since shared-instructions/0.3.0 may already be there). Drains stdin first, like every other path.
  • scripts/test_check_drift.py: a new case asserting exit 0, no ::warning::, and the branch named in the log — and a fourth entry in the full-pipe loop, since the unknown path is the one that never reads the listing it was handed and so is the likeliest to leave the writer on a closed pipe.
  • README and CHANGELOG: "silent in two cases" is now three, with the reasoning.

Verified locally with a stub git that exits 128, driving the workflow's exact shell: the annotation is gone, the log line is there, and the step exits 0. python3 scripts/test_check_drift.py passes.

[AcoPiper]

[Reviewer Round 2]

• [P1] Resolved. The git ls-remote failure is no longer collapsed into an empty listing: .github/workflows/check-drift.yml:141 supplies --branches-unknown, and scripts/check_drift.py:158 takes the quiet path after draining stdin. scripts/test_check_drift.py:159 specifically asserts both a zero exit and no ::warning::, while the full-pipe case covers that quiet path too. This addresses the false positive from a transient branch-listing failure.

No remaining findings. The updated implementation covers the issue’s three decision paths, keeps the warning non-blocking, names the required pin/latest/workflow action, and documents the behavior.

[AcoPiper]

[Review Verdict Round 2: APPROVED]

[AcoPiper]

Suggested squash commit

Title

Make a stopped apply schedule visible on a pull request

Body

A consumer's apply runs weekly, and a schedule can stop without anybody
noticing. GitHub disables a scheduled workflow in a repository that has
seen no activity for 60 days, and does it silently -- no failed run, no
notification, nothing on any pull request. A repository that goes quiet
for two months stops proposing releases to itself, and the only symptom
is the absence of pull requests nobody was expecting on a particular day.
`workflow_dispatch` recovers it, but only for somebody who already
suspects. That is the failure this whole mechanism exists to remove,
arriving by a different door.

`check-drift.yml` runs on every pull request in every consumer and
already reads the pin, so it resolves the latest release too and warns
when the pin is behind. A `::warning::` annotation, not a failure: the
check still passes. Turning a repository red because a release exists
upstream would break pull requests that have nothing to do with the
instructions, which is what pinning is for, and a check that cries wolf
gets ignored -- including about the drift it was written to catch.

It is quiet whenever the warning would be furniture. On the latest
release, and on a pin ahead of it, where dispatching the apply would
move somebody backwards. Quiet too when `shared-instructions/<latest>`
is already on the repository: the apply pushed it, so the schedule ran,
and an unmerged update pull request is a different situation. The branch
rather than the pull request, though the pull request is what a reader
would think of -- listing pull requests needs `pull-requests: read`, a
called workflow can only narrow the caller's token, and every consumer's
`ci.yml` would have to grant it by hand, in the one directory nothing
upstream may write. A branch is a ref, which `contents: read` already
covers, and it answers the same question.

An unreadable branch listing is not an empty one, and the two must not
arrive as the same thing. Empty says the apply pushed nothing;
unreadable rules nothing out, and reading the second as the first warns
the repository whose update branch is sitting right there, during a
hiccup, on a pull request that has nothing to do with any of this. The
workflow says which it got with `--branches-unknown`. The warning
repeats on every pull request, so the run that skips it loses nothing
that does not come back -- the same trade as an unresolvable latest
release, which the step notes in the log and otherwise leaves alone.
Nothing here can turn a pull request red: every path exits 0, the
listing is drained even by the branch that never looks at it, so a full
pipe cannot become SIGPIPE under `pipefail`, and `continue-on-error`
catches whatever else.

The comparison is `scripts/check_drift.py`, driven from the workflow the
way `pin_file.py` and `render.py` already are. Every test in this
repository exercises a script and none exercises a workflow file, so
logic written inline in the YAML would have nowhere to be tested from.
`scripts/test_check_drift.py` covers it -- behind, current, ahead, the
branch already pushed, an older branch under the same prefix, a
`feature/shared-instructions/<latest>` that only ends in the apply's
branch name, tag ordering as numbers across differing depth, the
unreadable listing, and each of those past a full pipe buffer. `ci.yml`
runs it, and the README documents the behaviour beside the
empty-Checks-tab section, since both are things a reader meets without
looking for them.

Closes #23