diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 925ff2f..6c24bbe 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -126,33 +126,56 @@ jobs: steps: - name: dispatch the sweep env: - GH_TOKEN: ${{ github.token }} + GITHUB_TOKEN: ${{ github.token }} SWEEP_WORKFLOW: ${{ inputs.sweep_workflow }} - # This step speaks gh and says so, the same declaration - # actions/refs-not-closing carries (#198 spec 4). A workflow has no - # shell to call forge_preflight from, so the refusal is inline - # below; #205 owns the REST port that removes both. - CEREMONY_FORGE_CLIENT: gh + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | - # Two questions, not one. @codex-reviewer-andresmgsl: a guard that - # only asks `command -v gh` passes the moment a Forgejo runner image - # happens to ship gh — and then runs a GitHub dispatch against a - # forge that cannot serve it, which is the client/forge mismatch - # forge_preflight exists to prevent. So the FORGE is decided first, - # mirroring forge_detect positively (only github.com is accepted; - # anything else, known or not, is refused — "Never 'probably - # github'"), and the binary is checked second. + # REST, not `gh` (#205). The workflow-dispatch endpoint has the SAME + # shape on both forges — + # POST {api}/repos/{owner}/{repo}/actions/workflows/{file}/dispatches + # {"ref": "", "inputs": {...}} -> 204, empty body + # — so this step no longer decides a forge at all. That is why the + # `CEREMONY_FORGE_CLIENT: gh` declaration and both inline refusals are + # gone rather than ported: there is nothing left to refuse. Measured + # on this instance (Forgejo 8.0.3+gitea-1.22.0) and published in its + # own swagger; run 459 was raised this way. # - # A warning, not a failure: this trigger is the misconfiguration - # alarm for a CONSUMER's missing sweep caller, and reddening every - # sweep on a forge for a gap #205 already owns would drown that - # signal. #205 ports the dispatch to REST and removes all of this. - if [ "${GITHUB_SERVER_URL:-}" != "https://github.com" ]; then - echo "::warning::labels: the sweep was NOT woken from this trigger — it dispatches with \`gh\` against GitHub, and this is not a GitHub forge (GITHUB_SERVER_URL=${GITHUB_SERVER_URL:-unset}). #205 ports it to REST. The hourly SCHEDULED sweep still runs; every event-driven wake through this caller — issue events included — is unavailable until then." - exit 0 + # STILL LOUD on failure, per this job's contract: a consumer missing + # the sweep caller, its `bootstrap` input, or `actions: write` must + # fail HERE and visibly, not sweep silently never again. + api="${GITHUB_API_URL:-https://api.github.com}" + + # `gh workflow run` defaulted the ref to the repository's default + # branch; REST has no default and 400s without one. Prefer the event + # payload, fall back to an API read: on a `pull_request_target` run + # GITHUB_REF_NAME is `/merge`, which is not a branch and would + # dispatch nothing. + branch="${DEFAULT_BRANCH:-}" + if [ -z "$branch" ]; then + branch="$(curl -fsS -H "Authorization: Bearer $GITHUB_TOKEN" \ + "$api/repos/$GITHUB_REPOSITORY" | jq -r '.default_branch // empty')" fi - if ! command -v gh >/dev/null 2>&1; then - echo "::warning::labels: the sweep was NOT woken from this trigger — this runner does not carry \`gh\`. #205 ports the dispatch to REST. The hourly SCHEDULED sweep still runs; every event-driven wake through this caller is unavailable until then." - exit 0 + if [ -z "$branch" ]; then + echo "::error::labels: the sweep was NOT woken — could not determine the default branch to dispatch $SWEEP_WORKFLOW on." + exit 1 fi - gh workflow run "$SWEEP_WORKFLOW" -R "$GITHUB_REPOSITORY" -f bootstrap=no + + out="$(mktemp)" + trap 'rm -f "$out"' EXIT + code="$(curl -sS -o "$out" -w '%{http_code}' -X POST \ + -H "Authorization: Bearer $GITHUB_TOKEN" \ + -H 'Content-Type: application/json' \ + -d "$(jq -nc --arg ref "$branch" '{ref: $ref, inputs: {bootstrap: "no"}}')" \ + "$api/repos/$GITHUB_REPOSITORY/actions/workflows/$SWEEP_WORKFLOW/dispatches")" + + if [ "$code" != "204" ]; then + # Own the diagnostic rather than pass the status through. This + # Forgejo answers an unknown workflow name — and a bare ref that + # does not resolve — with `500` and an EMPTY body, so the raw + # status alone sends the reader looking for a server fault that is + # not there. + echo "::error::labels: the sweep was NOT woken — POST $api/repos/$GITHUB_REPOSITORY/actions/workflows/$SWEEP_WORKFLOW/dispatches (ref=$branch) returned HTTP $code: $(tr -d '\n' <"$out")" + echo "::error::labels: check that $SWEEP_WORKFLOW exists on $branch, declares a \`bootstrap\` workflow_dispatch input, and that this caller grants \`actions: write\`. An empty 500 body from Forgejo means the workflow name or the ref did not resolve." + exit 1 + fi + echo "labels: sweep dispatched — $SWEEP_WORKFLOW on $branch (bootstrap=no)" diff --git a/changelog.d/205.md b/changelog.d/205.md new file mode 100644 index 0000000..42c608c --- /dev/null +++ b/changelog.d/205.md @@ -0,0 +1,30 @@ +### Fixed + +- `.github/workflows/labels.yml` wakes the sweep over REST instead of + `gh workflow run`, so a board event reconciles within seconds on any forge + rather than waiting up to an hour for the scheduled sweep (#205). + +- The workflow-dispatch endpoint has the same shape on both forges, so that + step no longer decides one: the `CEREMONY_FORGE_CLIENT=gh` declaration and + both inline refusals are gone rather than ported (#205). + +- The dispatch supplies its `ref` explicitly, because REST has no default + branch where `gh workflow run` had one, and refuses without it (#205). + +- It takes that ref from the repository, never from `GITHUB_REF_NAME` — on a + `pull_request_target` run that is `/merge`, which is not a branch (#205). + +- A failed dispatch names the endpoint, the ref and the status, and says that + an empty `500` body from Forgejo means the workflow name or the ref did not + resolve — a bare status sends the reader after a server fault that is not + there (#205). + +### Added + +- `test/labels-dispatch.test.sh` extracts the shipped step and executes it + against a recording stub, asserting the method, endpoint, ref and + `inputs.bootstrap` actually sent (#205). + +- That test also drives the failure path: any non-204 still fails the job, so + the misconfiguration alarm the trigger exists to be cannot decay into a + warning (#205). diff --git a/test/labels-dispatch.test.sh b/test/labels-dispatch.test.sh new file mode 100755 index 0000000..376ca94 --- /dev/null +++ b/test/labels-dispatch.test.sh @@ -0,0 +1,142 @@ +#!/usr/bin/env bash +# The sweep dispatch in .github/workflows/labels.yml (#205). +# +# This EXTRACTS the shipped step's `run:` script and EXECUTES it against a +# recording stub, rather than grepping the YAML for strings. A grep here would +# pass on a step that assembles a perfect request and never sends it — the +# shape of defect this repo keeps finding in its own tests. So every case +# asserts on what the step actually sent, or on what it actually did when the +# forge refused. +set -uo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +# shellcheck source=test/harness.sh +. "$ROOT/test/harness.sh" + +WORKFLOW="$ROOT/.github/workflows/labels.yml" +TMP="$(mktemp -d)" +trap 'rm -rf "${TMP:?}"' EXIT + +# --- the step under test, taken from the shipped workflow -------------------- +STEP="$TMP/step.sh" +{ + printf '%s\n' '#!/usr/bin/env bash' 'set -e' + yq -r '.jobs.trigger.steps[] | select(.name == "dispatch the sweep") | .run' "$WORKFLOW" +} >"$STEP" +chmod +x "$STEP" + +step_extracted() { [ "$(wc -l <"$STEP")" -ge 10 ]; } +check "the step's script was extracted from the shipped workflow" 0 "" step_extracted + +# --- stubs ------------------------------------------------------------------- +# `curl` records every invocation and answers with the code the case wants. It +# parses only what the step actually passes, so a step that stopped sending +# `-d`, or changed the method, fails here rather than recording nothing. +make_curl() { # make_curl + cat >"$TMP/bin/curl" <>"$TMP/calls"; shift 2 ;; + -fsS|-sS|-s) shift ;; + *) url="\$1"; shift ;; + esac +done +printf '%s\n' "METHOD \$method" "URL \$url" "DATA \$data" >>"$TMP/calls" +# the default-branch read is a plain GET whose stdout the step pipes to jq +if [ "\$method" = GET ]; then printf '{"default_branch":"trunk"}'; exit 0; fi +[ -n "\$out" ] && printf '%s' '$2' >"\$out" +[ -n "\$write" ] && printf '%s' '$1' +exit 0 +STUB + chmod +x "$TMP/bin/curl" +} + +run_step() { # run_step [env assignments...] + local code="$1" body="$2" + shift 2 + rm -rf "${TMP:?}/bin" + mkdir -p "$TMP/bin" + : >"$TMP/calls" + make_curl "$code" "$body" + env PATH="$TMP/bin:$PATH" \ + GITHUB_TOKEN=tok \ + GITHUB_API_URL=https://forge.example/api/v1 \ + GITHUB_REPOSITORY=owner/repo \ + SWEEP_WORKFLOW=self-labels-sweep.yml \ + "$@" \ + "$STEP" +} + +sent() { grep -h "^$1 " "$TMP/calls" | tail -1 | cut -d' ' -f2-; } +sent_field() { jq -r "$1" <<<"$(sent DATA)"; } + +# --- the success path -------------------------------------------------------- +check "a 204 dispatch succeeds, naming what it woke and where" 0 \ + "sweep dispatched — self-labels-sweep.yml on main" \ + run_step 204 "" DEFAULT_BRANCH=main + +posted() { [ "$(sent METHOD)" = POST ]; } +check "...by POST, not GET" 0 "" posted + +right_endpoint() { + [ "$(sent URL)" = \ + "https://forge.example/api/v1/repos/owner/repo/actions/workflows/self-labels-sweep.yml/dispatches" ] +} +check "...to the dispatches endpoint of the workflow it was told to wake" 0 "" right_endpoint + +ref_is_main() { [ "$(sent_field .ref)" = main ]; } +check "...carrying a ref, because REST has no default and refuses without one" 0 "" ref_is_main + +bootstrap_is_string_no() { [ "$(sent_field '.inputs.bootstrap')" = no ]; } +check "...and bootstrap=no as a STRING input, not a bare flag" 0 "" bootstrap_is_string_no + +bearer_sent() { grep -qF 'HEADER Authorization: Bearer tok' "$TMP/calls"; } +check "...under the bearer header both forges accept" 0 "" bearer_sent + +# --- the ref it must NOT inherit --------------------------------------------- +# On pull_request_target GITHUB_REF_NAME is `/merge`. A step that reaches +# for it dispatches at something that is not a branch — and the forge answers +# that with the opaque 500, so it would look like an outage. +check "a pull_request_target run still dispatches at the branch" 0 "" \ + run_step 204 "" DEFAULT_BRANCH=main GITHUB_REF_NAME=203/merge +ref_is_not_a_merge_ref() { case "$(sent_field .ref)" in *merge*) return 1 ;; *) return 0 ;; esac; } +check "...never at its merge ref" 0 "" ref_is_not_a_merge_ref + +# --- the fallback ------------------------------------------------------------ +check "an absent default branch is read from the forge, not guessed" 0 "" \ + run_step 204 "" DEFAULT_BRANCH= +ref_is_trunk() { [ "$(sent_field .ref)" = trunk ]; } +check "...and the dispatch uses what the read returned" 0 "" ref_is_trunk + +# --- failure is loud, and the diagnostic is owned ---------------------------- +# `gh workflow run` failing WAS the misconfiguration alarm. The port keeps that +# contract: a consumer missing the caller, its input, or `actions: write` must +# fail here rather than sweep silently never again. +check "an empty 500 fails the step — the alarm still rings" 1 \ + "/self-labels-sweep.yml/dispatches (ref=main)" \ + run_step 500 "" DEFAULT_BRANCH=main +check "...explaining Forgejo's EMPTY 500 rather than passing it through" 1 \ + "empty 500 body from Forgejo means the workflow name or the ref did not resolve" \ + run_step 500 "" DEFAULT_BRANCH=main +check "...and naming the consumer causes the alarm exists for" 1 "actions: write" \ + run_step 500 "" DEFAULT_BRANCH=main +check "any non-204 fails, not only the statuses the API documents" 1 "forbidden" \ + run_step 403 '{"message":"forbidden"}' DEFAULT_BRANCH=main + +# --- what the port removed --------------------------------------------------- +# Strip comments first: the step's prose NAMES `gh workflow run` and +# CEREMONY_FORGE_CLIENT to explain what it replaced, so a raw grep asserts on +# the explanation instead of the code. +step_code() { sed 's/#.*//' "$STEP"; } +invokes_gh() { step_code | grep -qE '(^|[^[:alnum:]_])gh[[:space:]]'; } +decides_forge() { step_code | grep -qE 'GITHUB_SERVER_URL|CEREMONY_FORGE_CLIENT'; } +check "the step no longer INVOKES gh, its comments about it aside" 1 "" invokes_gh +check "...and no longer decides a forge, because REST needs no branch" 1 "" decides_forge + +summary diff --git a/test/labels-triggers.test.sh b/test/labels-triggers.test.sh index c547ae9..c941e67 100644 --- a/test/labels-triggers.test.sh +++ b/test/labels-triggers.test.sh @@ -70,10 +70,17 @@ check "the sweep keeps the ONE shared concurrency group" 0 "group: labels-reconc grep -F 'group: labels-reconcile' "$SWEEP" check "labels.yml carries the trigger job" 0 " trigger:" \ grep -E '^ trigger:' "$REUSABLE" +# #205 ported this dispatch from `gh workflow run` to REST. The assertion is +# the same one it always was — the sweep caller is woken BY NAME and never +# bootstrapped — but it now has to hold against a request rather than a CLI +# line. What the step actually SENDS is driven in test/labels-dispatch.test.sh; +# these two keep the wiring pinned here alongside the rest of the trigger. # shellcheck disable=SC2016 # $SWEEP_WORKFLOW is the workflow's own env var, asserted literally -check "the trigger dispatches the sweep caller, never bootstrapping" 0 \ - 'gh workflow run "$SWEEP_WORKFLOW" -R "$GITHUB_REPOSITORY" -f bootstrap=no' \ - grep -F 'gh workflow run' "$REUSABLE" +check "the trigger dispatches the sweep caller by name" 0 \ + 'actions/workflows/$SWEEP_WORKFLOW/dispatches' \ + grep -F '/dispatches' "$REUSABLE" +check "...never bootstrapping" 0 'bootstrap: "no"' \ + grep -F 'bootstrap' "$REUSABLE" # shellcheck disable=SC2016 # $1 expands in the nested bash, not here check "the trigger dispatch is never silenced with || true" 1 "" \ bash -c 'grep -F "gh workflow run" "$1" | grep -qF "|| true"' _ "$REUSABLE" diff --git a/test/no-runtime-gh.test.sh b/test/no-runtime-gh.test.sh index 7ef6b59..bb2d2ec 100755 --- a/test/no-runtime-gh.test.sh +++ b/test/no-runtime-gh.test.sh @@ -205,13 +205,20 @@ printf '%s\n' 'jobs:' ' t:' ' steps:' ' - env:' \ ' gh workflow run x' >"$TMP/declared-both.yml" check "...and a declaration guarding BOTH forge and binary is" 0 "" \ refuses_when_unavailable "$TMP/declared-both.yml" -# The shipped workflow is the real customer for that pair. -check "labels.yml declares the client it speaks" 0 "" \ +# labels.yml WAS the real customer for that pair. #205 ported its dispatch to +# REST, so it no longer speaks gh and must no longer declare a client — the +# exemption is spent, not inherited. Asserting its ABSENCE is what stops the +# declaration coming back as cover for a re-added `gh` call: an opt-out with no +# gh behind it is a standing permission slip. +check "labels.yml no longer declares a client, because it speaks none (#205)" 1 "" \ declares_gh_client "$ROOT/.github/workflows/labels.yml" -check "...decides the forge before dispatching" 0 "" \ - refuses_wrong_forge "$ROOT/.github/workflows/labels.yml" -check "...and checks the binary too, rather than dying on command not found" 0 "" \ - refuses_missing_binary "$ROOT/.github/workflows/labels.yml" +# shellcheck disable=SC2016 # `$SWEEP_WORKFLOW` is the literal the YAML must +# carry: the endpoint has to be built from the caller's input, not hardcoded. +labels_yml_dispatches_by_rest() { + grep -qF 'actions/workflows/$SWEEP_WORKFLOW/dispatches' \ + "$ROOT/.github/workflows/labels.yml" +} +check "...and dispatches the sweep over REST instead" 0 "" labels_yml_dispatches_by_rest check "...and an undeclared one does not" 1 "" declares_gh_client "$TMP/bad.sh" # A mention of the variable in prose is not a declaration. printf '%s\n' '#!/usr/bin/env bash' '# CEREMONY_FORGE_CLIENT=gh would opt out' \