diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 826d9b7..5cfa995 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -4,7 +4,8 @@ Title format: : # fix(execute): #98 bound a hung request under --watch --timeout The issue must carry the `accepted` label before you open this. See ISSUES.md. -Exempt: docs / chore / style changes from the "no issue required" list. +Exempt: docs / chore / style / release changes from the "no issue required" list. +`Closes #97` below, or an `issue-97` branch name, satisfies the check too. The type prefix drives release-please, so keep it accurate. --> diff --git a/.github/workflows/decision-label-check.yml b/.github/workflows/decision-label-check.yml new file mode 100644 index 0000000..202807d --- /dev/null +++ b/.github/workflows/decision-label-check.yml @@ -0,0 +1,56 @@ +name: Decision Label Check + +# Blocks merge while the `decision-needed` label is present. +# +# `decision-needed` means the PR is waiting on a maintainer, not on the contributor: +# an API shape that becomes a contract, a new dependency, a behaviour change to +# an existing default, anything touching pricing, limits, auth or permissions, +# or a product judgement the diff alone cannot settle. +# +# Why a merge gate rather than a convention: an open decision is easy to lose. +# It lives in a review comment, the PR otherwise looks healthy, CI is green, and +# it merges with the question never answered - the decision is then made by +# default rather than on purpose. This turns that into a red check. +# +# Workflow: +# 1. Reviewer raises a needs-decision finding and adds `decision-needed`. +# 2. This check fails, so the PR cannot merge. +# 3. A maintainer answers the question in a comment on the PR, so the +# reasoning is on the record and not just in someone's head. +# 4. They remove the label. The check re-runs and passes on `unlabeled`. +# +# Removing the label is the act of deciding. Do not remove it to unblock a +# merge without answering - that is the failure mode this exists to prevent. + +on: + pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] + branches: + - main + - main + +permissions: + contents: read + +jobs: + decision-label-check: + runs-on: ubuntu-latest + steps: + - name: Fail while the decision-needed label is present + env: + # The label name lives here, once. This gate shipped reading + # `decision`; the label was renamed to `decision-needed` the next day + # and the workflow was missed, so it passed on every pull request it + # existed to stop. Keep this in step with DECISION_LABEL in the + # babysit-prs sweep, which is what applies it. + LABEL: decision-needed + LABELS_JSON: ${{ toJson(github.event.pull_request.labels.*.name) }} + run: | + set -euo pipefail + if printf '%s' "$LABELS_JSON" | jq -e --arg L "$LABEL" 'any(. == $L)' >/dev/null; then + echo "::error::This PR carries the '$LABEL' label: it is blocked on a maintainer decision, not on the contributor." + echo "::error::Answer the open question in a comment on this PR, then remove the '$LABEL' label. This check re-runs on unlabeled." + echo "::error::Removing the label without answering defeats the gate - the decision then gets made by default at merge time." + exit 1 + fi + echo "No '$LABEL' label; nothing is blocked on a maintainer." diff --git a/.github/workflows/pr-issue-link.yml b/.github/workflows/pr-issue-link.yml index e1393e4..8b0d401 100644 --- a/.github/workflows/pr-issue-link.yml +++ b/.github/workflows/pr-issue-link.yml @@ -1,104 +1,256 @@ name: PR Issue Link -# Runs on pull_request_target so it also runs on fork pull requests, which are -# the ones this gate exists for. Fork runs of `pull_request` need per-run -# maintainer approval, which would leave the gate silent exactly where it is -# needed. +# Runs on pull_request_target so it also runs on fork pull requests. Fork runs +# of `pull_request` need per-run maintainer approval, which would leave the +# gate silent exactly where it is needed. # -# SAFETY: pull_request_target runs with the base repository's token. This job -# therefore never checks out, builds, or executes pull request code. It reads -# the title, the labels, and the referenced issue through the API and nothing -# else. Do not add a checkout step to this file. +# Every pull request into main must reference the work it belongs +# to, in its title, its description, or its branch name: +# +# - a GitHub issue (`#1978`) that exists, is an issue, and carries the +# `accepted` label - the issue-first policy in ISSUES.md; or +# - a Linear ticket (`KEEP-1234`), which only counts on a team pull +# request, since outside contributors cannot have one. Team branches +# are named `feat/KEEP-1234-description`, which is also what Linear's +# GitHub integration links on. The ticket is matched by shape only; +# Linear is not consulted. +# +# A pull request is a team pull request when its head branch lives in this +# repository (only push access can create one) or when the payload's +# author_association is OWNER, MEMBER or COLLABORATOR. The head-repository +# test comes first because author_association is computed from what the +# workflow token can see: a member whose organization membership is private +# is reported as CONTRIBUTOR, which is why the old author-only skip failed +# team pull requests. +# +# HTML comments are removed from the description before it is searched, so +# the examples in the pull request template can neither satisfy nor confuse +# the check. A GitHub issue is taken from the branch name only in an explicit +# form (issue-1978, issue/1978, gh-1978): a bare number in a branch name is +# not a reference, and can coincide with an unrelated accepted issue. +# +# When the check fails it leaves one comment on the pull request saying what +# it found and what to do, and edits that same comment on every later run, +# down to a one-line resolved note once the check passes. When a referenced +# issue is open and simply not yet accepted, the comment says the wait is on +# the maintainers and gives the contributor nothing to do. The comment never +# echoes the title, description or branch name back: they are contributor- +# controlled text and, rendered as Markdown, could carry mentions or links. +# The job log keeps them. Issue numbers in the comment sit in code spans so +# they neither autolink nor leave "mentioned" events on the issues. The +# concurrency group keeps two overlapping runs (an open followed at once by +# a label, say) from each creating a comment. +# +# The job runs on synchronize as well as open and edit so that a result +# exists for every head commit. A required status check is evaluated per +# commit; without synchronize it would sit at "Expected" after each push +# until someone edited the title. The job never checks out code, so +# rerunning it is free. +# +# SAFETY: pull_request_target runs with the base repository's token, which +# here can write to pull requests so the job can leave its comment. The job +# therefore never checks out, builds, or executes pull request code. It +# reads the title, body, branch name and labels from the event payload and +# the referenced issue through the API, and writes nothing but its own +# comment. Do not add a checkout step to this file. on: pull_request_target: - types: [opened, edited, reopened, labeled, unlabeled] + types: [opened, edited, reopened, synchronize, labeled, unlabeled] branches: - main +concurrency: + group: pr-issue-link-${{ github.event.pull_request.number }} + cancel-in-progress: true + permissions: contents: read issues: read - pull-requests: read + pull-requests: write jobs: check-issue-link: runs-on: ubuntu-latest steps: - - name: Require an accepted issue in the PR title + - name: Require an issue or ticket reference env: # Untrusted input. Passed through the environment and never # interpolated into the script body. PR_TITLE: ${{ github.event.pull_request.title }} + PR_BODY: ${{ github.event.pull_request.body }} + HEAD_REF: ${{ github.event.pull_request.head.ref }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} PR_LABELS: ${{ toJSON(github.event.pull_request.labels.*.name) }} + AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association }} + PR_NUMBER: ${{ github.event.pull_request.number }} REPO: ${{ github.repository }} GH_TOKEN: ${{ github.token }} EXEMPT_LABEL: no-issue-required ACCEPTED_LABEL: accepted - # Types that never require an issue. Keep in step with the - # "not required" list in ISSUES.md. - EXEMPT_TYPES: docs chore style + LINEAR_TEAM_KEY: KEEP + # Types that never require a reference. Keep in step with the + # "not required" list in ISSUES.md. release is kept in step with + # keeperhub so one exempt list is correct for both repositories. + EXEMPT_TYPES: docs chore style release run: | set -euo pipefail + is_team=false + if [ -n "$HEAD_REPO" ] && [ "$HEAD_REPO" = "$REPO" ]; then + is_team=true + fi + case "$AUTHOR_ASSOCIATION" in + OWNER|MEMBER|COLLABORATOR) is_team=true ;; + esac + + # The description is searched with HTML comments removed, so the + # pull request template's own examples never become candidates. + body=$(printf '%s' "$PR_BODY" | perl -0pe 's///gs') + + # The one comment this check owns on the pull request, found by its + # marker. Edited in place if present; created only when asked to. + # A comment failure must never change the check result, so every + # API write degrades to a warning. + marker='' + comment() { + local body id + body=$(printf '%s\n%s\n' "$marker" "$1") + id=$(gh api "repos/$REPO/issues/$PR_NUMBER/comments" --paginate \ + --jq '.[] | select(.user.login == "github-actions[bot]") | select(.body | startswith("")) | .id' \ + 2>/dev/null | head -n1 || true) + if [ -n "$id" ]; then + jq -n --arg body "$body" '{body: $body}' \ + | gh api -X PATCH "repos/$REPO/issues/comments/$id" --input - >/dev/null \ + || echo "::warning::Could not update the check-issue-link comment." + elif [ "${2:-}" = "create" ]; then + jq -n --arg body "$body" '{body: $body}' \ + | gh api -X POST "repos/$REPO/issues/$PR_NUMBER/comments" --input - >/dev/null \ + || echo "::warning::Could not create the check-issue-link comment." + fi + } + + pass() { + echo "$1" + comment "Resolved. $1" + exit 0 + } + + # fail [reasons] [awaiting]. With awaiting set, a referenced + # issue is open and simply not yet accepted: the contributor has done + # their part, so the comment says the wait is ours and asks nothing. fail() { + local headline="$1" reasons="${2:-}" awaiting="${3:-}" md + if [ -n "$awaiting" ]; then + md="### This pull request is waiting on issue triage"$'\n\n' + else + md="### This pull request needs an issue or ticket reference"$'\n\n' + fi + md+="$headline"$'\n\n' + if [ -n "$reasons" ]; then + md+="$(printf '%s' "$reasons" | sed 's/^ */- /')"$'\n\n' + fi + if [ "$is_team" = true ]; then + md+="**Team pull requests** carry the Linear ticket in the branch name, " + md+="\`feat/$LINEAR_TEAM_KEY-1234-short-description\`. Rename the branch and " + md+="GitHub moves this pull request with it. Referencing an accepted GitHub " + md+="issue also works."$'\n\n' + fi + if [ -n "$awaiting" ]; then + md+="**Nothing to do on your side.** The issue is filed and is waiting for a " + md+="maintainer to triage it and apply the \`$ACCEPTED_LABEL\` label; that step " + md+="is ours, not yours. This check reruns when the pull request is pushed or " + md+="edited, not when the issue is labelled, so once \`$ACCEPTED_LABEL\` lands, " + md+="re-run the job or edit the title to retrigger it." + else + md+="**Outside contributions** start with an issue. Open one, wait for a " + md+="maintainer to apply the \`$ACCEPTED_LABEL\` label, then put \`#N\` in the " + md+="title, for example \`fix: #1978 return 403 with a body on public /api/chains\`. " + md+="No issue is needed for typos, broken links, formatting, or docs corrected to " + md+="match existing behaviour; retitle with one of: \`$EXEMPT_TYPES\`. Full policy: " + md+="[ISSUES.md](https://github.com/$REPO/blob/main/ISSUES.md)."$'\n\n' + md+="This check reruns on every push and edit, but not when the issue changes; " + md+="re-run the job or edit the title to retrigger it." + fi + echo "----------------------------------------------" - echo " ERROR: $1" + echo " ERROR: $headline" echo "----------------------------------------------" echo "" - echo " Got title: $PR_TITLE" - echo "" - echo " KeeperHub takes issues before pull requests. Open an issue," - echo " wait for a maintainer to apply the '$ACCEPTED_LABEL' label," - echo " then reference it in this PR's title:" - echo "" - echo " feat: #97 add --require-verified to execute status" - echo " fix(execute): #98 bound a hung request under --watch" + echo " Got title: $PR_TITLE" + echo " Got branch: $HEAD_REF" echo "" - echo " No issue needed for typos, help-text wording, or docs that" - echo " match existing behaviour. Retitle as one of:" - echo " $EXEMPT_TYPES" - echo "" - echo " Full policy: https://github.com/$REPO/blob/main/ISSUES.md" - echo "" - echo " Already labelled '$ACCEPTED_LABEL'? This check does not rerun" - echo " by itself when the issue changes - re-run the job, or edit" - echo " the PR title to retrigger it." + printf '%s\n' "$md" + comment "$md" create + echo "::error title=check-issue-link::$headline" exit 1 } if printf '%s' "$PR_LABELS" | grep -qF "\"$EXEMPT_LABEL\""; then - echo "Exempt: pull request carries the '$EXEMPT_LABEL' label." - exit 0 + pass "Exempt: pull request carries the '$EXEMPT_LABEL' label." fi pr_type=$(printf '%s' "$PR_TITLE" | sed -nE 's/^([a-zA-Z]+)(\([^)]*\))?!?:.*/\1/p' | tr '[:upper:]' '[:lower:]') for exempt in $EXEMPT_TYPES; do if [ "$pr_type" = "$exempt" ]; then - echo "Exempt: '$pr_type' changes do not require an issue." - exit 0 + pass "Exempt: '$pr_type' changes do not require a reference." fi done - issue_number=$(printf '%s' "$PR_TITLE" | grep -oE '#[0-9]+' | head -n1 | tr -d '#') - if [ -z "$issue_number" ]; then - fail "PR title carries no issue reference." - fi + # Every grep below may legitimately match nothing. `|| true` keeps + # pipefail from ending the script before fail() can explain why. - if ! issue_json=$(gh api "repos/$REPO/issues/$issue_number" 2>/dev/null); then - fail "#$issue_number does not resolve to an issue in $REPO." + if [ "$is_team" = true ]; then + ticket=$(printf '%s\n%s\n%s\n' "$PR_TITLE" "$body" "$HEAD_REF" \ + | grep -oiE "\b$LINEAR_TEAM_KEY-[0-9]+\b" | head -n1 | tr '[:lower:]' '[:upper:]' || true) + if [ -n "$ticket" ]; then + pass "References Linear ticket $ticket." + fi fi - if printf '%s' "$issue_json" | jq -e '.pull_request' >/dev/null 2>&1; then - fail "#$issue_number is a pull request, not an issue." - fi + # `#N` in the title or body, plus an explicit issue form in the + # branch name (issue-2057, issue/2057, gh-2057). A bare number in a + # branch name is not a reference: it can coincide with an unrelated + # accepted issue. At most ten distinct candidates are looked up. + candidates=$( + { + printf '%s\n%s\n' "$PR_TITLE" "$body" | grep -oE '#[0-9]+' | tr -d '#' || true + printf '%s\n' "$HEAD_REF" | grep -oiE '(^|/)(issue|gh)[-/]?[0-9]+' | grep -oE '[0-9]+$' || true + } | awk '!seen[$0]++ { if (++n <= 10) print }' + ) - if ! printf '%s' "$issue_json" | jq -e --arg l "$ACCEPTED_LABEL" \ - '.labels | map(.name) | index($l)' >/dev/null 2>&1; then - state=$(printf '%s' "$issue_json" | jq -r '.state') - labels=$(printf '%s' "$issue_json" | jq -r '[.labels[].name] | join(", ")') - fail "#$issue_number is not marked '$ACCEPTED_LABEL' (state: $state; labels: ${labels:-none})." + if [ -z "$candidates" ]; then + fail "No issue or ticket reference was found in the title, description, or branch name." fi - echo "PR title references accepted issue #$issue_number." + reasons="" + awaiting="" + for n in $candidates; do + if ! issue_json=$(gh api "repos/$REPO/issues/$n" 2>/dev/null); then + reasons+=" \`#$n\` does not resolve to an issue in $REPO."$'\n' + continue + fi + if printf '%s' "$issue_json" | jq -e '.pull_request' >/dev/null 2>&1; then + reasons+=" \`#$n\` is a pull request, not an issue."$'\n' + continue + fi + if ! printf '%s' "$issue_json" | jq -e --arg l "$ACCEPTED_LABEL" \ + '.labels | map(.name) | index($l)' >/dev/null 2>&1; then + state=$(printf '%s' "$issue_json" | jq -r '.state') + labels=$(printf '%s' "$issue_json" | jq -r '[.labels[].name] | join(", ")') + reasons+=" \`#$n\` is not marked '$ACCEPTED_LABEL' (state: $state; labels: ${labels:-none})."$'\n' + # An open issue without the label is in our triage queue, not + # the contributor's hands. A closed one is a decision already made. + if [ "$state" = "open" ]; then + awaiting=true + fi + continue + fi + pass "References accepted issue #$n." + done + + if [ -n "$awaiting" ]; then + fail "A referenced issue is filed but not yet marked '$ACCEPTED_LABEL'." "$reasons" "$awaiting" + fi + fail "None of the referenced issues can be merged against:" "$reasons" diff --git a/.github/workflows/recheck-issue-link.yml b/.github/workflows/recheck-issue-link.yml new file mode 100644 index 0000000..e83073d --- /dev/null +++ b/.github/workflows/recheck-issue-link.yml @@ -0,0 +1,141 @@ +name: Recheck Issue Link + +# `pr-issue-link.yml` decides whether a pull request references an issue that +# carries `accepted`. It runs on `pull_request_target`, so it re-evaluates on +# pull request events and never on issue events - which means a red result +# outlives the condition that produced it. Label an issue `accepted` and every +# open pull request referencing it keeps its failing `check-issue-link` until +# someone pushes, edits the title, or churns a label to force a re-run. +# +# That is not hypothetical, and this repository documented it. The gate this +# replaces ended its failure text with: "Already labelled 'accepted'? This +# check does not rerun by itself when the issue changes - re-run the job, or +# edit the PR title to retrigger it." That instruction is what this removes. +# +# This workflow closes the loop from the other side: when an issue gains or +# loses `accepted`, find the open pull requests that reference it and re-run +# their gate. It changes no verdict of its own. `pr-issue-link.yml` remains the +# single place the rule lives; this only makes it look again. +# +# The candidate extraction below is duplicated from that workflow rather than +# shared. It has to be: the gate runs as `pull_request_target` and is forbidden +# from checking out, so there is no file both jobs can read. When the rule +# changes there, change it here. The two must agree, or this re-runs the wrong +# pull requests and leaves the right ones red. +# +# SAFETY: this reads pull request titles, bodies and branch names, all of which +# are contributor-controlled. They are never interpolated into a shell command +# or a workflow expression - the API response goes to a file and jq does the +# matching. The only event data used directly is the issue number and the label +# name, and the label name only in an `if:` expression. + +on: + issues: + types: [labeled, unlabeled] + +# Keyed by issue, so two issues labelled at once do not queue behind each +# other. Not cancel-in-progress: a run part-way through its pull request list +# has already re-run some and not others, and cancelling it would leave the +# rest red with nothing to trigger them again. +concurrency: + group: recheck-issue-link-${{ github.event.issue.number }} + cancel-in-progress: false + +permissions: + actions: write + contents: read + pull-requests: read + +jobs: + recheck: + if: github.event.label.name == 'accepted' + runs-on: ubuntu-latest + + steps: + - name: Re-run the issue-link gate on pull requests referencing this issue + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + ISSUE_NUMBER: ${{ github.event.issue.number }} + GATE_PATH: .github/workflows/pr-issue-link.yml + # Kept in step with the `branches:` list in pr-issue-link.yml. A pull + # request the gate never runs on has nothing to re-run. + GATE_BASES: main + run: | + set -euo pipefail + + gh api "repos/$REPO/pulls?state=open&per_page=100" --paginate > pulls.json + + # Same reference shapes as pr-issue-link.yml: every `#N` in the title + # or the body with HTML comments stripped, so the pull request + # template's own examples do not count, plus an explicit issue-N / + # issue/N / gh-N branch name. A bare number in a branch name is not a + # reference. First ten distinct candidates only, matching the gate - + # if this issue is the eleventh, the gate ignores it and so must we. + jq -r --argjson want "$ISSUE_NUMBER" --arg bases "$GATE_BASES" ' + ($bases | split(" ")) as $ok + | .[] + | select(.base.ref as $b | $ok | index($b)) + | . as $pr + | (($pr.body // "") | gsub(""; ""; "m")) as $body + | ( [ ($pr.title, $body) | match("#([0-9]+)"; "g").captures[0].string ] + + [ ($pr.head.ref // "") + | match("(^|/)(issue|gh)[-/]?([0-9]+)"; "gi").captures[2].string ] + ) + | map(tonumber) + | reduce .[] as $n ([]; if index($n) then . else . + [$n] end) + | .[0:10] + | select(index($want)) + | "\($pr.number) \($pr.head.sha)" + ' pulls.json > referencing.txt || true + + if [ ! -s referencing.txt ]; then + echo "No open pull request references #$ISSUE_NUMBER. Nothing to re-run." + exit 0 + fi + + rerun=0 + skipped=0 + missing=0 + + while read -r pr sha; do + [ -n "$pr" ] || continue + + run_json=$(gh api "repos/$REPO/actions/runs?head_sha=$sha&per_page=100" \ + --jq "[.workflow_runs[] | select(.path == \"$GATE_PATH\")] + | sort_by(.created_at) | last // empty" 2>/dev/null || true) + + if [ -z "$run_json" ]; then + echo "PR #$pr: no $GATE_PATH run on $sha - nothing to re-run." + missing=$((missing + 1)) + continue + fi + + run_id=$(printf '%s' "$run_json" | jq -r '.id') + status=$(printf '%s' "$run_json" | jq -r '.status') + conclusion=$(printf '%s' "$run_json" | jq -r '.conclusion // ""') + + if [ "$status" != "completed" ]; then + echo "PR #$pr: run $run_id is $status - it will pick up the label itself." + skipped=$((skipped + 1)) + continue + fi + + if [ "$conclusion" = "success" ]; then + echo "PR #$pr: run $run_id already passed." + skipped=$((skipped + 1)) + continue + fi + + # Re-runs the whole run rather than only its failed jobs: a run + # cancelled by the gate's own concurrency group has no failed job + # to re-run, and this workflow has a single job either way. + if gh api -X POST "repos/$REPO/actions/runs/$run_id/rerun" >/dev/null 2>&1; then + echo "PR #$pr: re-ran run $run_id (was $conclusion)." + rerun=$((rerun + 1)) + else + echo "::warning title=recheck-issue-link::PR #$pr: could not re-run run $run_id." + fi + done < referencing.txt + + echo "Re-ran $rerun, skipped $skipped, no run found for $missing." diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d8e7dd2..8556ba7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,7 +10,8 @@ before the pull request. **[ISSUES.md](ISSUES.md) is the policy** - what needs a issue, what goes straight to a pull request, and what happens after you file one. The short version: open an issue, wait for the `accepted` label, then reference -it in your pull request title (`feat: #97 description`). Typos, help-text +it in your pull request title (`feat: #97 description`), its description, or +an `issue-97` branch name. Typos, help-text wording, and docs matching existing behaviour skip all of that. ## Development setup diff --git a/ISSUES.md b/ISSUES.md index 8eaa10a..fcf0448 100644 --- a/ISSUES.md +++ b/ISSUES.md @@ -95,22 +95,28 @@ the other to be correct, and the timeout changed behaviour for every existing Once your issue carries `accepted`: -1. **Reference the issue in the pull request title**, after the conventional - commit type: +1. **Reference the issue from the pull request.** The title is the place for + it, after the conventional commit type: ``` feat: #97 add --require-verified to execute status fix(execute): #98 bound a hung request under --watch --timeout ``` - The type prefix drives release-please, so keep it accurate. + `Closes #97` in the description or an `issue-97` branch name also satisfies + the check; a bare number in a branch name does not. The type prefix drives + release-please, so keep it accurate. 2. Fill in the pull request template. 3. Target `main`. This repo has no `staging` branch. -`docs`, `chore`, and `style` pull requests are exempt from the issue check -automatically. A maintainer can apply `no-issue-required` to exempt anything else. +`docs`, `chore`, `style` and `release` pull requests are exempt from the issue +check automatically. A maintainer can apply `no-issue-required` to exempt +anything else. + +Once your issue is labelled `accepted`, the check re-runs on its own and turns +green - there is nothing to re-trigger by hand. ## Security