forked from heavy-duty/ceremony
fix(198): the action fails closed, the caller decides scheduling, the guard decides the forge (#198)
@codex-reviewer-andresmgsl's second review, both points taken. The refs action goes back to `forge_preflight || exit 1`.97e63achad it exit 0 with a notice so the PR check would not be red, and that conflated two different questions: "this action cannot produce a verdict" is the ACTION's contract and must stay a refusal, while "this check should not block the board" is the CALLER's decision. The caller now carries it — refs-guard.yml skips unless github.server_url is github.com, mirroring forge_detect positively. A skipped check is a green head; an action that reports success it did not earn is not. The leaked preflight_err temp file goes with the revert. The workflow guard asked the wrong question. `command -v gh` alone passes the moment a Forgejo runner image happens to ship gh, and then dispatches against a forge that cannot serve it — the client/forge mismatch forge_preflight exists to prevent. It decides the FORGE first now, mirroring forge_detect positively, and the binary second. The source guard splits to match: a declaration guarded only by binary presence is reported, with a fixture that fails on exactly that shape. The warning text was also wrong on the facts, as noted: issue-event sweeps ARE this caller's event-driven wakes, so they are precisely what is lost. It now says the hourly scheduled sweep survives and every event-driven wake through this caller does not, until #205. Point 1 of that review — jq 1.6 accepting an empty payload — was already fixed in728102a, pushed before the review landed. Verified under the runner's jq 1.6 as well as 1.7: 28 test files, 0 failed both ways. shellcheck 0.10.0 (CI's pin), actionlint, self-ref, marker, vendored, changelog-armed all clean with every file tracked. Refs #198
This commit is contained in:
parent
728102a3ba
commit
adf907c963
6 changed files with 103 additions and 53 deletions
24
.github/workflows/labels.yml
vendored
24
.github/workflows/labels.yml
vendored
|
|
@ -134,13 +134,25 @@ jobs:
|
|||
# below; #205 owns the REST port that removes both.
|
||||
CEREMONY_FORGE_CLIENT: gh
|
||||
run: |
|
||||
# Never `command not found`. On a runner without gh the wake is
|
||||
# genuinely lost, and that is worth a warning rather than a failed
|
||||
# job: this trigger is the misconfiguration alarm for a CONSUMER's
|
||||
# missing sweep caller, and reddening every sweep on a forge whose
|
||||
# runner has no gh would drown that signal in a known gap (#205).
|
||||
# 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.
|
||||
#
|
||||
# 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
|
||||
fi
|
||||
if ! command -v gh >/dev/null 2>&1; then
|
||||
echo "::warning::labels: the sweep was NOT woken from this trigger — it dispatches with \`gh\`, which this runner does not carry. #205 ports it to REST. Scheduled and issue-event sweeps are unaffected; only this caller's event-driven wake is lost."
|
||||
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
|
||||
fi
|
||||
gh workflow run "$SWEEP_WORKFLOW" -R "$GITHUB_REPOSITORY" -f bootstrap=no
|
||||
|
|
|
|||
12
.github/workflows/refs-guard.yml
vendored
12
.github/workflows/refs-guard.yml
vendored
|
|
@ -12,6 +12,18 @@ permissions:
|
|||
|
||||
jobs:
|
||||
refs-not-closing:
|
||||
# The action is gh-only until #199: its whole gather is a GraphQL query,
|
||||
# and Forgejo serves no GraphQL at all. The ACTION refuses by name on a
|
||||
# backend it cannot speak (that is its contract, and its contract test);
|
||||
# scheduling it where it can only refuse is this workflow's decision, and
|
||||
# a permanently red required check would block every merge on this forge
|
||||
# for a gap #199 already owns. So the job does not run there — a skipped
|
||||
# check is a green head, an invented verdict is not.
|
||||
#
|
||||
# The condition mirrors lib/forge.sh's forge_detect positively: only
|
||||
# github.com is accepted, and anything else — Forgejo, or a host this
|
||||
# file has not met — is not run. "Never 'probably github'."
|
||||
if: ${{ github.server_url == 'https://github.com' }}
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
|
||||
|
|
|
|||
|
|
@ -22,26 +22,14 @@ set -euo pipefail
|
|||
# shellcheck source=lib/forge.sh
|
||||
. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/forge.sh"
|
||||
export CEREMONY_FORGE_CLIENT=gh
|
||||
# The refusal and the SCHEDULING are two different questions, and #198's first
|
||||
# head conflated them: preflight refused correctly on this forge and turned
|
||||
# every PR's `Refs guard` red, which blocks the board rather than protecting
|
||||
# it. A guard that cannot run here must not claim a verdict — but it also must
|
||||
# not stand permanently red for a port that has its own issue.
|
||||
#
|
||||
# So: not-runnable-here is reported and skipped, loudly and by name, and only
|
||||
# a preflight failure for any OTHER reason is fatal. The distinction is the
|
||||
# forge, not the exit code — on a forge this action CAN speak, a preflight
|
||||
# failure is still a hard refusal, which is the case the tests drive.
|
||||
preflight_err="$(mktemp)"
|
||||
trap 'rm -f "$preflight_err"' EXIT
|
||||
if ! forge_preflight 2>"$preflight_err"; then
|
||||
cat "$preflight_err" >&2
|
||||
if [ "$(forge_detect 2>/dev/null)" != github ]; then
|
||||
printf '::notice::refs-not-closing: not run — this action is still gh-only (its gather is GraphQL, which this forge does not serve) and #199 ports it. No verdict was produced.\n'
|
||||
exit 0
|
||||
fi
|
||||
exit 1
|
||||
fi
|
||||
# Fail CLOSED, at the action boundary. An earlier head here exited 0 with a
|
||||
# notice so the PR check would not be red; @codex-reviewer-andresmgsl was
|
||||
# right that this conflates two different questions. "This action cannot
|
||||
# produce a verdict" is the ACTION's contract and must stay a refusal; "this
|
||||
# check should not block the board" is the CALLER's decision, and it belongs
|
||||
# in .github/workflows/refs-guard.yml, which skips on a backend this action
|
||||
# cannot speak until #199 ports it.
|
||||
forge_preflight || exit 1
|
||||
|
||||
owner="${GITHUB_REPOSITORY%%/*}"
|
||||
name="${GITHUB_REPOSITORY#*/}"
|
||||
|
|
|
|||
|
|
@ -37,8 +37,13 @@
|
|||
the forge zero times, so no verdict is produced either way (#198).
|
||||
|
||||
- `.github/workflows/labels.yml`'s sweep dispatch declares the client it
|
||||
speaks and refuses by name on a runner without it, instead of dying with
|
||||
`command not found` on every sweep. #205 ports it to REST (#198).
|
||||
speaks and decides the FORGE before the binary, so a Forgejo runner that
|
||||
happens to ship `gh` cannot dispatch against a forge that cannot serve it.
|
||||
#205 ports it to REST (#198).
|
||||
|
||||
- `actions/refs-not-closing` fails closed on a forge it cannot speak, and
|
||||
`.github/workflows/refs-guard.yml` carries the scheduling decision — the
|
||||
action never reports a success it did not earn (#198).
|
||||
|
||||
- `issue_payload_valid` refuses an empty payload on jq 1.6 as well as 1.7.
|
||||
`jq -e` exits 4 on empty input under 1.7 and **0** under 1.6, and this
|
||||
|
|
|
|||
|
|
@ -48,19 +48,34 @@ ALLOWED_FILE='lib/forge-github.sh'
|
|||
# would ride in free, and it lets a declaration exist without a refusal.
|
||||
declares_gh_client() { grep -qE '^[[:space:]]*(export[[:space:]]+)?CEREMONY_FORGE_CLIENT[=:][[:space:]]*gh[[:space:]]*$' "$1"; }
|
||||
|
||||
# Declaring is half of it. #197's bar is "declared AND refuses loudly", so a
|
||||
# declaring file must also carry the refusal — forge_preflight for a script,
|
||||
# an inline availability check for a workflow that has no shell to call it
|
||||
# from. A declaration without one is a permission slip for `command not found`.
|
||||
# Comments stripped first, for the same reason gh_calls strips them and with
|
||||
# the same lesson learned the hard way: the first version of this predicate
|
||||
# was satisfied by the word `forge_preflight` inside labels.yml's own comment
|
||||
# EXPLAINING that it has no forge_preflight to call. A guard that reads prose
|
||||
# as evidence is the blind sweep again, and it passed its own mutation test
|
||||
# because of it.
|
||||
# Declaring is half of it. #197's bar is "declared AND refuses loudly", and the
|
||||
# refusal has TWO halves that a single check conflates
|
||||
# (@codex-reviewer-andresmgsl, #198):
|
||||
#
|
||||
# * the FORGE — a gh dispatch is wrong on a forge that cannot serve it, and
|
||||
# asking only "is gh installed?" passes the moment a Forgejo runner image
|
||||
# happens to ship gh, which is the client/forge mismatch forge_preflight
|
||||
# exists to prevent;
|
||||
# * the BINARY — present or not on this runner.
|
||||
#
|
||||
# forge_preflight answers both, so a script that calls it satisfies both. A
|
||||
# workflow has no shell to call it from and must do both inline.
|
||||
#
|
||||
# Comments are stripped first, for the same reason gh_calls strips them and
|
||||
# with the same lesson learned the hard way: the first version of this
|
||||
# predicate was satisfied by the word `forge_preflight` inside labels.yml's own
|
||||
# comment EXPLAINING that it has no forge_preflight to call. A guard that reads
|
||||
# prose as evidence is the blind sweep again, and it passed its own mutation
|
||||
# test because of it.
|
||||
strip_comments() { sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$1"; }
|
||||
refuses_wrong_forge() {
|
||||
strip_comments "$1" | grep -qE 'forge_preflight|GITHUB_SERVER_URL.*github\.com'
|
||||
}
|
||||
refuses_missing_binary() {
|
||||
strip_comments "$1" | grep -qE 'forge_preflight|command -v gh'
|
||||
}
|
||||
refuses_when_unavailable() {
|
||||
sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$1" \
|
||||
| grep -qE 'forge_preflight|command -v gh'
|
||||
refuses_wrong_forge "$1" && refuses_missing_binary "$1"
|
||||
}
|
||||
|
||||
# A runtime invocation, not the word. `gh` must be at a command position and
|
||||
|
|
@ -175,13 +190,28 @@ printf '%s\n' 'jobs:' ' t:' ' steps:' ' - env:' \
|
|||
' run: |' \
|
||||
' command -v gh >/dev/null || { echo "::warning::not woken"; exit 0; }' \
|
||||
' gh workflow run x' >"$TMP/declared-refusing.yml"
|
||||
check "...and a declaration WITH one is" 0 "" \
|
||||
# Binary presence ALONE is not a refusal: a Forgejo runner that ships gh would
|
||||
# sail past it and dispatch against a forge that cannot serve the call.
|
||||
check "...and a declaration guarded only by binary presence still is not" 1 "" \
|
||||
refuses_when_unavailable "$TMP/declared-refusing.yml"
|
||||
check "...though it does satisfy the binary half on its own" 0 "" \
|
||||
refuses_missing_binary "$TMP/declared-refusing.yml"
|
||||
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
||||
printf '%s\n' 'jobs:' ' t:' ' steps:' ' - env:' \
|
||||
' CEREMONY_FORGE_CLIENT: gh' \
|
||||
' run: |' \
|
||||
' [ "$GITHUB_SERVER_URL" = "https://github.com" ] || exit 0' \
|
||||
' command -v gh >/dev/null || exit 0' \
|
||||
' 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 "" \
|
||||
declares_gh_client "$ROOT/.github/workflows/labels.yml"
|
||||
check "...and refuses by name rather than dying on command not found" 0 "" \
|
||||
refuses_when_unavailable "$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"
|
||||
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' \
|
||||
|
|
|
|||
|
|
@ -147,17 +147,16 @@ forgejo_boundary() {
|
|||
GITHUB_ACTION_PATH="$ROOT/actions/refs-not-closing" \
|
||||
bash "$ENTRYPOINT"
|
||||
}
|
||||
# The contract on a forge this action cannot speak: say so by name, produce
|
||||
# NO verdict, and do not stand red. Red would be honest about the port and
|
||||
# dishonest about the PR — it blocks every merge on this forge for a gap #199
|
||||
# owns, which is a worse failure than the one it reports (#198).
|
||||
check "on a forgejo forge the action names the client mismatch" 0 \
|
||||
# The contract on a forge this action cannot speak: refuse, by name, non-zero,
|
||||
# and read nothing. FAIL CLOSED — an earlier head made this exit 0 so the PR
|
||||
# check would not be red, which conflated the ACTION's contract with the
|
||||
# CALLER's scheduling decision (@codex-reviewer-andresmgsl, #198). The caller
|
||||
# is .github/workflows/refs-guard.yml, which skips on a backend this action
|
||||
# cannot speak; the action itself never reports success it did not earn.
|
||||
check "on a forgejo forge the action refuses, non-zero" 1 \
|
||||
"cannot speak it" forgejo_boundary
|
||||
check "...naming the client it declared" 0 "'gh' client" forgejo_boundary
|
||||
check "...and the client the forge actually needs" 0 "'rest' client" forgejo_boundary
|
||||
check "...says explicitly that it produced no verdict" 0 "No verdict was produced" \
|
||||
forgejo_boundary
|
||||
check "...points at the issue that ports it" 0 "#199" forgejo_boundary
|
||||
check "...naming the client it declared" 1 "'gh' client" forgejo_boundary
|
||||
check "...and the client the forge actually needs" 1 "'rest' client" forgejo_boundary
|
||||
# The teeth: it must not have READ anything. The stub counts its own calls, so
|
||||
# a gather that ran despite the refusal is visible here.
|
||||
forgejo_read_count() {
|
||||
|
|
@ -166,6 +165,10 @@ forgejo_read_count() {
|
|||
wc -l <"$TMP/gh-calls"
|
||||
}
|
||||
check "...and reached the forge zero times" 0 "0" forgejo_read_count
|
||||
# The caller carries the scheduling half, positively: only github.com runs it.
|
||||
check "the caller skips the job on any non-github forge" 0 \
|
||||
"github.server_url == 'https://github.com'" \
|
||||
grep -F "if:" "$ROOT/.github/workflows/refs-guard.yml"
|
||||
|
||||
check "action boundary fails when GraphQL read fails" 42 \
|
||||
"fake GraphQL read failed" action_boundary failure
|
||||
|
|
|
|||
Loading…
Reference in a new issue