diff --git a/.github/workflows/dependabot-title.yml b/.github/workflows/dependabot-title.yml deleted file mode 100644 index 4ed20b7..0000000 --- a/.github/workflows/dependabot-title.yml +++ /dev/null @@ -1,76 +0,0 @@ -name: Dependabot Title - -# Dependabot capitalises the subject of the messages it writes -- `build(deps): -# Bump ...` -- which is exactly what the `subjectPattern` in lint-pr.yml -# rejects. That capital is not configurable: dependabot-core decides it from a -# heuristic over recent commit messages, and on this repository it lands on -# "capitalise" against a history that is entirely lowercase. -# -# So the choice was to exempt the bot from the title rule or to fix the title. -# Exempting it would leave `Bump` in the log on `main` forever and carve a hole -# in a rule that otherwise has none. This lowercases the first letter instead, -# and the rule stays single. See docs/adr/0016-dependency-update-policy.md. -# -# Three things about this file are load-bearing: -# -# 1. `pull_request_target`, because a `pull_request` run on a fork-context -# Dependabot pull request gets a read-only token and cannot retitle it. -# That means this workflow runs FROM THE BASE BRANCH WITH A WRITE TOKEN, -# so it must never check out, build, or execute anything from the head -- -# and it does not: there is no checkout step at all. -# 2. The title reaches the shell through `env:`, never through a `${{ }}` -# expansion inside `run:`. Interpolating pull-request-controlled text into -# a privileged shell is the textbook injection, whoever opened it. -# 3. It is idempotent, which is what makes listening to `edited` safe: our own -# edit re-fires the event, the second run finds nothing to change and -# stops. There is no loop. -# -# It is a NORMALISER, not a gate -- `Conventional PR title` in lint-pr.yml is -# still the thing that judges the result. Expect one transient red on `opened`: -# that check and this job start together, this one edits the title, and the -# edit re-runs the check green. - -on: - pull_request_target: - types: [opened, reopened, edited] - -permissions: - pull-requests: write - -# Queue rather than cancel: a cancelled retitle would leave the pull request -# with the title the check has already rejected. -concurrency: - group: dependabot-title-${{ github.event.pull_request.number }} - cancel-in-progress: false - -jobs: - subject: - name: Lowercase subject - if: github.event.pull_request.user.login == 'dependabot[bot]' - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: - - name: Lowercase the first letter of the subject - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUMBER: ${{ github.event.pull_request.number }} - PR_TITLE: ${{ github.event.pull_request.title }} - run: | - set -euo pipefail - - # Case only, and only after a conventional-SHAPED prefix. Whether - # the type and scope in that prefix are real is the title check's - # business, not this job's: rewriting them here would paper over a - # broken .github/dependabot.yml, which is the other half of this. - if [[ ! "$PR_TITLE" =~ ^([a-z]+(\([a-z][a-z-]*\))?!?:[[:space:]])([A-Z].*)$ ]]; then - echo "Nothing to lowercase: ${PR_TITLE}" - exit 0 - fi - - prefix="${BASH_REMATCH[1]}" - subject="${BASH_REMATCH[3]}" - fixed="${prefix}${subject,}" - - echo " ${PR_TITLE}" - echo "→ ${fixed}" - gh pr edit "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --title "$fixed" diff --git a/.github/workflows/lint-pr.yml b/.github/workflows/lint-pr.yml index f466c8f..f59050c 100644 --- a/.github/workflows/lint-pr.yml +++ b/.github/workflows/lint-pr.yml @@ -1,12 +1,20 @@ name: Lint PR +# `pull-requests: write` is here for the normalisation step below, which fixes +# one thing it is not worth exempting a bot from. This workflow runs on +# `pull_request_target`, so it holds that token while running FROM THE BASE +# BRANCH -- it must never check out, build, or execute anything from the head, +# and it does not: there is no checkout step. Values from the pull request +# reach the shell through `env:`, never through a `${{ }}` expansion inside +# `run:`. + on: pull_request_target: types: [opened, edited, synchronize, reopened] permissions: contents: read - pull-requests: read + pull-requests: write jobs: title: @@ -14,6 +22,49 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: + # Dependabot capitalises the subject it writes -- `build(deps): Bump ...` + # -- which is exactly what `subjectPattern` below rejects, and that + # capital is not configurable: dependabot-core decides it from a + # heuristic over recent commit messages, and on this repository it lands + # on "capitalise" against a history that is entirely lowercase. + # + # So the choice was to exempt the bot from the rule or to fix the title. + # This fixes the title, and the rule stays single. + # + # It belongs in THIS job rather than a workflow of its own. A retitle + # made with GITHUB_TOKEN does not trigger a new workflow run -- GitHub + # suppresses that to prevent loops -- so a separate normaliser leaves the + # title check sitting on the stale red it produced before the edit, until + # some unrelated event happens to re-run it. Correcting the title in the + # step before the check closes that race: the action re-reads the title + # from the API rather than the event payload, so it judges the corrected + # one. See docs/adr/0016-dependency-update-policy.md. + - name: Lowercase a bot's capitalised subject + if: github.event.pull_request.user.login == 'dependabot[bot]' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + set -euo pipefail + + # Read the title from the API, not the payload: on a re-run the + # payload is whatever it was when the run was created. + title="$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json title --jq '.title')" + + # Case only, and only after a conventional-SHAPED prefix. Whether the + # type and scope in that prefix are real is the check's business, not + # this step's: rewriting them here would paper over a broken + # .github/dependabot.yml, which is the other half of this. + if [[ ! "$title" =~ ^([a-z]+(\([a-z][a-z-]*\))?!?:[[:space:]])([A-Z].*)$ ]]; then + echo "Nothing to lowercase: ${title}" + exit 0 + fi + + fixed="${BASH_REMATCH[1]}${BASH_REMATCH[3],}" + echo " ${title}" + echo "→ ${fixed}" + gh pr edit "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --title "$fixed" + # The vocabulary here must stay in step with # .github/conventional-commits.yaml and the lefthook commit-msg hook. # The scope drives release-please: feat(component) cuts component/v1.x.0, diff --git a/docs/adr/0016-dependency-update-policy.md b/docs/adr/0016-dependency-update-policy.md index 2970898..e30c779 100644 --- a/docs/adr/0016-dependency-update-policy.md +++ b/docs/adr/0016-dependency-update-policy.md @@ -83,20 +83,34 @@ specification release whichever of them is used. ### 3. The capitalised subject is corrected, not excused -`.github/workflows/dependabot-title.yml` lowercases the first letter of the -subject on a Dependabot pull request, and the title check then judges the result -like any other. The rule stays single: there is no author exempt from it, and -nothing lands on `main` reading `Bump`. - -The workflow needs `pull_request_target` — a `pull_request` run on a Dependabot -pull request holds a read-only token and cannot retitle anything — which means -it runs from the base branch holding a write token. Three properties keep that -safe, and are stated in the file so they survive editing: it never checks out -the head (there is no checkout step), the title reaches the shell through `env:` -rather than a `${{ }}` expansion inside `run:`, and it rewrites case only, after -a conventional-*shaped* prefix. Whether the type and scope in that prefix are -real remains the title check's business; correcting them here would paper over -exactly the `dependabot.yml` bug described above. +The first step of the `Conventional PR title` job in +`.github/workflows/lint-pr.yml` lowercases the first letter of the subject on a +Dependabot pull request. The action then judges the corrected title like any +other. The rule stays single: there is no author exempt from it, and nothing +lands on `main` reading `Bump`. + +**It has to be the same job, not a workflow of its own.** That was tried first, +and it does not hold: a retitle made with `GITHUB_TOKEN` does not trigger a new +workflow run — GitHub suppresses that to prevent loops — so the title check sits +on the red it produced *before* the edit, with a title that is now correct, until +some unrelated event happens to re-run it. On #66 that red simply stayed. On #60 +it cleared, but only because Dependabot pushed again a minute later, which is +luck rather than design. + +Correcting the title in the step before the check closes the race, and it works +because `amannn/action-semantic-pull-request` re-reads the title from the REST +API rather than trusting the event payload — deliberately, for exactly this +reason. Confirmed empirically: re-running the stale failed run on #66, with its +original payload, passed. + +The job runs on `pull_request_target`, so it holds a write token while running +from the base branch. Three properties keep that safe, and are stated in the +file so they survive editing: it never checks out the head (there is no checkout +step), values from the pull request reach the shell through `env:` rather than a +`${{ }}` expansion inside `run:`, and it rewrites case only, after a +conventional-*shaped* prefix. Whether the type and scope in that prefix are real +remains the check's business; correcting them here would paper over exactly the +`dependabot.yml` bug described above. ### 4. Every copy of the vocabulary is checked, including the bot's @@ -132,6 +146,9 @@ is a weaker thing to maintain than a rule with none. **Relax `subjectPattern` for everyone** — retires a repository-wide convention to accommodate a bot, and takes the guard away from humans too. +**A separate normaliser workflow** — cleaner to read, and broken for the reason +in decision 3: nothing re-runs the check it invalidates. + **Exempt bot commits from the DCO check entirely** — simpler than matching on name, and wrong: Dependabot's sign-off is real and there is no reason to stop reading it. Skipping the check would also skip it for any future app whose @@ -155,11 +172,9 @@ The vocabulary now has five copies and a check that holds all five, rather than three copies and two that drifted unobserved. `wrangler` moves only in a pull request opened for it. -`dependabot-title.yml` is a normaliser, not a gate. Like -`codeowners-notice.yml` it must never be added to the required checks in -`.github/rulesets/main-branch.json`. Expect one transient red on `opened`: the -title check and the normaliser start together, the normaliser edits the title, -and the edit re-runs the check green. +The normalisation step is not a gate; the action in the same job is. Because +they share a run, a Dependabot pull request goes green on its first attempt +rather than showing a transient red that something else has to clear. ## Follow-ups