forked from heavy-duty/ceremony
fix(labels): refuse an unset API root, name transport failures, update the docs
Three corrections from @codex-reviewer-andresmgsl's review of 935a813.
1. `api="${GITHUB_API_URL:-https://api.github.com}"` guessed GitHub when the
variable was absent — driven with a recording curl, it reported success
after POSTing to api.github.com from this forge. That is the "Never
'probably github'" rule, and the same unset-environment refusal #201 just
established for docs-sync. It now refuses before any request, and the test
asserts zero calls were made: refusing after a POST is not refusing.
2. The "never silenced with || true" invariant was still asserted by grepping
the gh line this port removed, so it passed on any REST implementation
including one that swallows a failed POST. It is rebound behaviourally: a
curl that dies at the transport must fail the step. Doing that revealed the
step failed with a bare exit 7 and no sentence, so it now names the failure
— owning the diagnostic is the whole point of the surrounding code.
3. docs/CONSUMERS.md and both caller comments still described `gh workflow
run` as the mechanism. They describe the REST dispatch now, and the manual
bootstrap command carries a forge-neutral curl form beside the gh one: a
cross-forge runbook that sends this forge to a missing binary is wrong even
where the prose around it is right.
Refs #205
This commit is contained in:
parent
935a813d75
commit
37e31ffd85
6 changed files with 103 additions and 12 deletions
2
.github/workflows/labels-sweep.yml
vendored
2
.github/workflows/labels-sweep.yml
vendored
|
|
@ -87,7 +87,7 @@ jobs:
|
||||||
# equivalent of.
|
# equivalent of.
|
||||||
#
|
#
|
||||||
# bootstrap: every trigger-driven wake arrives as workflow_dispatch
|
# bootstrap: every trigger-driven wake arrives as workflow_dispatch
|
||||||
# too (that is how `gh workflow run` wakes the caller), so the event
|
# too (that is how the trigger's dispatch wakes the caller), so the event
|
||||||
# name alone no longer separates the operator's manual full-board
|
# name alone no longer separates the operator's manual full-board
|
||||||
# bootstrap from an event-woken sweep — the caller's `bootstrap`
|
# bootstrap from an event-woken sweep — the caller's `bootstrap`
|
||||||
# dispatch input does: the trigger passes "no", a bare manual
|
# dispatch input does: the trigger passes "no", a bare manual
|
||||||
|
|
|
||||||
28
.github/workflows/labels.yml
vendored
28
.github/workflows/labels.yml
vendored
|
|
@ -143,7 +143,18 @@ jobs:
|
||||||
# STILL LOUD on failure, per this job's contract: a consumer missing
|
# STILL LOUD on failure, per this job's contract: a consumer missing
|
||||||
# the sweep caller, its `bootstrap` input, or `actions: write` must
|
# the sweep caller, its `bootstrap` input, or `actions: write` must
|
||||||
# fail HERE and visibly, not sweep silently never again.
|
# fail HERE and visibly, not sweep silently never again.
|
||||||
api="${GITHUB_API_URL:-https://api.github.com}"
|
# NEVER "probably github" (lib/forge.sh). Defaulting an unset
|
||||||
|
# GITHUB_API_URL to api.github.com would send this forge's dispatch
|
||||||
|
# to GitHub and report success — the same unset-environment guess
|
||||||
|
# #201 just refused for docs-sync. The API root is injected by the
|
||||||
|
# forge running us; if it is absent we do not know where we are, and
|
||||||
|
# a guess is worse than a red trigger
|
||||||
|
# (@codex-reviewer-andresmgsl, #205 review).
|
||||||
|
api="${GITHUB_API_URL:-}"
|
||||||
|
if [ -z "$api" ]; then
|
||||||
|
echo "::error::labels: the sweep was NOT woken — GITHUB_API_URL is unset, so the forge's API root is unknown. Refusing to guess a forge."
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
# `gh workflow run` defaulted the ref to the repository's default
|
# `gh workflow run` defaulted the ref to the repository's default
|
||||||
# branch; REST has no default and 400s without one. Prefer the event
|
# branch; REST has no default and 400s without one. Prefer the event
|
||||||
|
|
@ -161,12 +172,21 @@ jobs:
|
||||||
fi
|
fi
|
||||||
|
|
||||||
out="$(mktemp)"
|
out="$(mktemp)"
|
||||||
trap 'rm -f "$out"' EXIT
|
err="$(mktemp)"
|
||||||
code="$(curl -sS -o "$out" -w '%{http_code}' -X POST \
|
trap 'rm -f "$out" "$err"' EXIT
|
||||||
|
# A transport failure is named, not merely propagated. Letting `set
|
||||||
|
# -e` carry curl's own exit code out of the assignment DID fail the
|
||||||
|
# job — the invariant holds — but it failed with a bare status and no
|
||||||
|
# sentence, which is the opposite of this step owning its diagnostic.
|
||||||
|
if ! code="$(curl -sS -o "$out" -w '%{http_code}' -X POST \
|
||||||
-H "Authorization: Bearer $GITHUB_TOKEN" \
|
-H "Authorization: Bearer $GITHUB_TOKEN" \
|
||||||
-H 'Content-Type: application/json' \
|
-H 'Content-Type: application/json' \
|
||||||
-d "$(jq -nc --arg ref "$branch" '{ref: $ref, inputs: {bootstrap: "no"}}')" \
|
-d "$(jq -nc --arg ref "$branch" '{ref: $ref, inputs: {bootstrap: "no"}}')" \
|
||||||
"$api/repos/$GITHUB_REPOSITORY/actions/workflows/$SWEEP_WORKFLOW/dispatches")"
|
"$api/repos/$GITHUB_REPOSITORY/actions/workflows/$SWEEP_WORKFLOW/dispatches" \
|
||||||
|
2>"$err")"; then
|
||||||
|
echo "::error::labels: the sweep was NOT woken — the request to $api never completed: $(tr -d '\n' <"$err")"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
if [ "$code" != "204" ]; then
|
if [ "$code" != "204" ]; then
|
||||||
# Own the diagnostic rather than pass the status through. This
|
# Own the diagnostic rather than pass the status through. This
|
||||||
|
|
|
||||||
2
.github/workflows/self-labels.yml
vendored
2
.github/workflows/self-labels.yml
vendored
|
|
@ -39,7 +39,7 @@ permissions:
|
||||||
contents: read
|
contents: read
|
||||||
checks: read # mergeability/check-rollup read for PR state
|
checks: read # mergeability/check-rollup read for PR state
|
||||||
statuses: read # commit-status rollup read for PR state
|
statuses: read # commit-status rollup read for PR state
|
||||||
actions: write # the trigger job's `gh workflow run` dispatch of the sweep caller (#209)
|
actions: write # the trigger job's dispatch of the sweep caller (#209, #205)
|
||||||
issues: write
|
issues: write
|
||||||
pull-requests: write
|
pull-requests: write
|
||||||
jobs:
|
jobs:
|
||||||
|
|
|
||||||
|
|
@ -28,3 +28,17 @@
|
||||||
- That test also drives the failure path: any non-204 still fails the job, so
|
- 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
|
the misconfiguration alarm the trigger exists to be cannot decay into a
|
||||||
warning (#205).
|
warning (#205).
|
||||||
|
|
||||||
|
- An unset `GITHUB_API_URL` refuses before any request instead of defaulting
|
||||||
|
to `api.github.com`. Guessing sent this forge's dispatch to GitHub and
|
||||||
|
reported success — the "Never 'probably github'" rule, and the same
|
||||||
|
unset-environment refusal #201 established for docs-sync (#205).
|
||||||
|
|
||||||
|
- A dispatch that never reaches the forge names the failure. Letting `set -e`
|
||||||
|
carry curl's exit code out did fail the job, but with a bare status and no
|
||||||
|
sentence (#205).
|
||||||
|
|
||||||
|
- `docs/CONSUMERS.md` and both caller comments describe the REST dispatch, and
|
||||||
|
the manual bootstrap command carries a forge-neutral form beside the `gh`
|
||||||
|
one — a cross-forge runbook that directs this forge to a missing binary is
|
||||||
|
wrong even where the surrounding prose is right (#205).
|
||||||
|
|
|
||||||
|
|
@ -339,7 +339,9 @@ together at the same pin:
|
||||||
- **`labels.yml`** — the event-facing half, called on PR and issue events.
|
- **`labels.yml`** — the event-facing half, called on PR and issue events.
|
||||||
Two jobs: additive path-based `scope:*` labels, and a few-seconds
|
Two jobs: additive path-based `scope:*` labels, and a few-seconds
|
||||||
`trigger` job that wakes the sweep by dispatching the consumer's sweep
|
`trigger` job that wakes the sweep by dispatching the consumer's sweep
|
||||||
caller (`gh workflow run`, plain `GITHUB_TOKEN` — `workflow_dispatch` is
|
caller (a REST `POST` to the forge's own
|
||||||
|
`${GITHUB_API_URL}/repos/{owner}/{repo}/actions/workflows/{file}/dispatches`,
|
||||||
|
plain `GITHUB_TOKEN` — `workflow_dispatch` is
|
||||||
one of the two documented exemptions from the token's no-retrigger rule,
|
one of the two documented exemptions from the token's no-retrigger rule,
|
||||||
so no PAT anywhere in the path and no loop: the sweep dispatches
|
so no PAT anywhere in the path and no loop: the sweep dispatches
|
||||||
nothing).
|
nothing).
|
||||||
|
|
@ -404,7 +406,7 @@ permissions:
|
||||||
contents: read
|
contents: read
|
||||||
checks: read # mergeability/check-rollup read for PR state
|
checks: read # mergeability/check-rollup read for PR state
|
||||||
statuses: read # commit-status rollup read for PR state
|
statuses: read # commit-status rollup read for PR state
|
||||||
actions: write # the trigger job's `gh workflow run` dispatch of the sweep caller (#209)
|
actions: write # the trigger job's dispatch of the sweep caller (#209, #205)
|
||||||
issues: write
|
issues: write
|
||||||
pull-requests: write
|
pull-requests: write
|
||||||
jobs:
|
jobs:
|
||||||
|
|
@ -468,8 +470,8 @@ repositories allow check data to be read regardless, but a private consumer
|
||||||
needs the explicit reads above; without them the failure appears as an empty
|
needs the explicit reads above; without them the failure appears as an empty
|
||||||
`state:*` axis on the board rather than a red workflow run. The labels
|
`state:*` axis on the board rather than a red workflow run. The labels
|
||||||
caller's `actions: write` is different — it is required everywhere, public
|
caller's `actions: write` is different — it is required everywhere, public
|
||||||
repos included: the trigger job's `gh workflow run` is a write, and without
|
repos included: the trigger job's dispatch is a write, and without it every
|
||||||
it every event run goes red at the trigger.
|
event run goes red at the trigger.
|
||||||
|
|
||||||
**The failure mode to know before bumping**: a consumer that bumps its pin
|
**The failure mode to know before bumping**: a consumer that bumps its pin
|
||||||
to a #209-carrying tag without adding the sweep caller keeps green-looking
|
to a #209-carrying tag without adding the sweep caller keeps green-looking
|
||||||
|
|
@ -526,8 +528,8 @@ carrying the split:
|
||||||
fires both callers into the one shared `labels-reconcile` group — so
|
fires both callers into the one shared `labels-reconcile` group — so
|
||||||
displacement goes **up**, and the fix reads as the bug getting worse.
|
displacement goes **up**, and the fix reads as the bug getting worse.
|
||||||
4. **`actions: write` on the labels caller** — consumers carry
|
4. **`actions: write` on the labels caller** — consumers carry
|
||||||
`actions: read` today (crew does); the trigger job's `gh workflow run`
|
`actions: read` today (crew does); the trigger job's dispatch is a
|
||||||
is a write. The sweep caller keeps `actions: read`.
|
write. The sweep caller keeps `actions: read`.
|
||||||
|
|
||||||
Bump without the sweep caller and the trigger job goes red on every PR
|
Bump without the sweep caller and the trigger job goes red on every PR
|
||||||
and issue event — the loud failure mode above — so never split these
|
and issue event — the loud failure mode above — so never split these
|
||||||
|
|
@ -596,10 +598,25 @@ to bootstrap labels on a fresh repository. A bare dispatch is also the
|
||||||
operator's general manual full-board sweep — the answer when the board
|
operator's general manual full-board sweep — the answer when the board
|
||||||
looks wrong now rather than after the next scheduled cadence:
|
looks wrong now rather than after the next scheduled cadence:
|
||||||
|
|
||||||
|
On GitHub, with the `gh` CLI:
|
||||||
|
|
||||||
```sh
|
```sh
|
||||||
gh workflow run labels-sweep.yml -R <owner>/<repo>
|
gh workflow run labels-sweep.yml -R <owner>/<repo>
|
||||||
```
|
```
|
||||||
|
|
||||||
|
On any forge — including Forgejo, whose runners carry no `gh` — the same
|
||||||
|
dispatch over REST, which is what the trigger job itself sends (#205):
|
||||||
|
|
||||||
|
```sh
|
||||||
|
curl -sS -X POST \
|
||||||
|
-H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' \
|
||||||
|
-d '{"ref":"main","inputs":{"bootstrap":"yes"}}' \
|
||||||
|
"$API/repos/<owner>/<repo>/actions/workflows/labels-sweep.yml/dispatches"
|
||||||
|
```
|
||||||
|
|
||||||
|
`$API` is the forge's API root — `https://api.github.com` on GitHub,
|
||||||
|
`<instance>/api/v1` on Forgejo — and success is `204` with an empty body.
|
||||||
|
|
||||||
Ceremony dogfoods the callers under the filenames `self-labels.yml` and
|
Ceremony dogfoods the callers under the filenames `self-labels.yml` and
|
||||||
`self-labels-sweep.yml`, so the equivalent command in this repository
|
`self-labels-sweep.yml`, so the equivalent command in this repository
|
||||||
substitutes that filename. Scheduled and trigger-driven runs only
|
substitutes that filename. Scheduled and trigger-driven runs only
|
||||||
|
|
|
||||||
|
|
@ -129,6 +129,46 @@ check "...and naming the consumer causes the alarm exists for" 1 "actions: write
|
||||||
check "any non-204 fails, not only the statuses the API documents" 1 "forbidden" \
|
check "any non-204 fails, not only the statuses the API documents" 1 "forbidden" \
|
||||||
run_step 403 '{"message":"forbidden"}' DEFAULT_BRANCH=main
|
run_step 403 '{"message":"forbidden"}' DEFAULT_BRANCH=main
|
||||||
|
|
||||||
|
# --- the API root is not guessed ---------------------------------------------
|
||||||
|
# Defaulting an unset GITHUB_API_URL to api.github.com sent this forge's
|
||||||
|
# dispatch to GitHub and reported success (@codex-reviewer-andresmgsl). The
|
||||||
|
# teeth are the call count: refusing AFTER a request is not refusing.
|
||||||
|
run_step_no_api() {
|
||||||
|
rm -rf "${TMP:?}/bin"; mkdir -p "$TMP/bin"; : >"$TMP/calls"
|
||||||
|
make_curl 204 ""
|
||||||
|
env PATH="$TMP/bin:$PATH" GITHUB_TOKEN=tok GITHUB_REPOSITORY=owner/repo \
|
||||||
|
SWEEP_WORKFLOW=self-labels-sweep.yml DEFAULT_BRANCH=main "$STEP"
|
||||||
|
}
|
||||||
|
check "an unset GITHUB_API_URL refuses rather than guessing GitHub" 1 \
|
||||||
|
"GITHUB_API_URL is unset" run_step_no_api
|
||||||
|
no_calls_made() { [ ! -s "$TMP/calls" ]; }
|
||||||
|
check "...having made zero requests: refusing after a POST is not refusing" 0 "" no_calls_made
|
||||||
|
|
||||||
|
# --- a transport failure is not silence --------------------------------------
|
||||||
|
# The old `|| true` invariant was asserted by grepping the gh line this port
|
||||||
|
# removed, so it passed on any REST implementation including one that swallows
|
||||||
|
# a failed POST (@codex-reviewer-andresmgsl). Driven instead: curl itself exits
|
||||||
|
# non-zero, which `-w` cannot report because nothing is written.
|
||||||
|
make_failing_curl() {
|
||||||
|
printf '%s\n' '#!/usr/bin/env bash' 'echo "curl: (7) failed to connect" >&2' 'exit 7' \
|
||||||
|
>"$TMP/bin/curl"
|
||||||
|
chmod +x "$TMP/bin/curl"
|
||||||
|
}
|
||||||
|
run_step_curl_dies() {
|
||||||
|
rm -rf "${TMP:?}/bin"; mkdir -p "$TMP/bin"; : >"$TMP/calls"
|
||||||
|
make_failing_curl
|
||||||
|
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 DEFAULT_BRANCH=main "$STEP"
|
||||||
|
}
|
||||||
|
check "a POST that never reaches the forge fails the step, and says so" 1 \
|
||||||
|
"never completed" run_step_curl_dies
|
||||||
|
|
||||||
|
# A code-aware guard alongside the behavioural one: no swallowing operator on
|
||||||
|
# the dispatch itself.
|
||||||
|
never_silenced() { sed 's/#.*//' "$STEP" | grep -qE '\|\|[[:space:]]*true'; }
|
||||||
|
check "...and the step carries no || true" 1 "" never_silenced
|
||||||
|
|
||||||
# --- what the port removed ---------------------------------------------------
|
# --- what the port removed ---------------------------------------------------
|
||||||
# Strip comments first: the step's prose NAMES `gh workflow run` and
|
# 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
|
# CEREMONY_FORGE_CLIENT to explain what it replaced, so a raw grep asserts on
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue