forked from heavy-duty/ceremony
fix(refs-not-closing): report and skip on a forge it cannot speak, rather than reddening every PR (#198)
The first head's `Refs guard` failed on this PR, correctly: spec 4's CEREMONY_FORGE_CLIENT=gh declaration made forge_preflight refuse by name on this forge. But that workflow runs on every pull request here, so the declaration as first written turns every future PR red until #199 lands — blocking the board for a gap that already has its own issue. Refusing and scheduling are different questions. This action must never produce a verdict from a graph it did not read, and it does not: on a forge it cannot speak it now says so by name, cites #199, states that no verdict was produced, and reaches the forge zero times. A preflight failure for any other reason stays fatal, and on a forge it CAN speak nothing changes. Also: five SC2016 findings in test/no-runtime-gh.test.sh. They were invisible locally because shellcheck-all.sh lints TRACKED files and the guard was still untracked when I ran it — a new file is exactly the case that check cannot see. Verified this time against CI's pinned shellcheck 0.10.0 with the file committed. test/run.sh: 28 test files, 0 failed, under CI's CEREMONY_REQUIRE_* env. shellcheck, actionlint, self-ref, marker and vendored guards all clean. Refs #198
This commit is contained in:
parent
2900529533
commit
97e63acef0
4 changed files with 50 additions and 6 deletions
|
|
@ -22,7 +22,26 @@ set -euo pipefail
|
|||
# shellcheck source=lib/forge.sh
|
||||
. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/forge.sh"
|
||||
export CEREMONY_FORGE_CLIENT=gh
|
||||
forge_preflight || exit 1
|
||||
# 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
|
||||
|
||||
owner="${GITHUB_REPOSITORY%%/*}"
|
||||
name="${GITHUB_REPOSITORY#*/}"
|
||||
|
|
|
|||
|
|
@ -32,6 +32,10 @@
|
|||
- The post-merge nudge links the issue on the forge in play rather than a
|
||||
hard-coded `github.com` (#198).
|
||||
|
||||
- `actions/refs-not-closing` reports and skips on a forge it cannot speak,
|
||||
naming the client and #199, instead of standing red on every PR. It reaches
|
||||
the forge zero times, so no verdict is produced either way (#198).
|
||||
|
||||
- `.github/scripts/release-path.sh` names `lib/forge.sh`: #191 put the shim on
|
||||
the release doors' executable path here, so a doors-unchanged record that
|
||||
omitted it was measuring the wrong set (#198).
|
||||
|
|
|
|||
|
|
@ -118,14 +118,17 @@ check "no runtime gh outside the github backend or a declared-client file" 0 ""
|
|||
TMP="$(mktemp -d)"
|
||||
trap 'rm -rf "$TMP"' EXIT
|
||||
|
||||
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
||||
printf '%s\n' '#!/usr/bin/env bash' 'gh api "repos/$REPO/issues/1"' >"$TMP/bad.sh"
|
||||
check "a reintroduced gh api read is seen" 0 "gh api" gh_calls "$TMP/bad.sh"
|
||||
|
||||
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
||||
printf '%s\n' '#!/usr/bin/env bash' 'run gh issue comment "$n" --body x' >"$TMP/bad2.sh"
|
||||
check "a reintroduced gh issue write is seen, staged or not" 0 "gh issue" \
|
||||
gh_calls "$TMP/bad2.sh"
|
||||
|
||||
# The exact shape the 0.6.0 merge reintroduced, indented inside a function.
|
||||
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
||||
printf '%s\n' '#!/usr/bin/env bash' 'f() {' \
|
||||
' guarded_read bodies gh api --paginate "repos/$REPO/issues/$1/comments"' '}' \
|
||||
>"$TMP/bad3.sh"
|
||||
|
|
@ -136,12 +139,14 @@ printf '%s\n' '#!/usr/bin/env bash' '# gh api used to live here (#188)' \
|
|||
'# run gh issue comment — retired' >"$TMP/prose.sh"
|
||||
check "prose about gh is not a call site" 1 "" gh_calls "$TMP/prose.sh"
|
||||
|
||||
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
||||
printf '%s\n' '#!/usr/bin/env bash' 'forge_api "repos/$REPO/issues/1"' \
|
||||
'echo "the gh client speaks /api/v3"' >"$TMP/good.sh"
|
||||
check "the shim verb is not mistaken for a call site" 1 "" gh_calls "$TMP/good.sh"
|
||||
|
||||
# Neighbouring identifiers must not read as the binary: `gh_calls`, `$gh`,
|
||||
# a path ending in /gh, and `regh api` are all not an invocation of gh.
|
||||
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
||||
printf '%s\n' '#!/usr/bin/env bash' 'gh_calls() { :; }' 'regh api foo' \
|
||||
'echo "$gh api"' >"$TMP/lookalike.sh"
|
||||
check "lookalike identifiers are not call sites" 1 "" gh_calls "$TMP/lookalike.sh"
|
||||
|
|
|
|||
|
|
@ -100,6 +100,9 @@ mkdir -p "$TMP/bin"
|
|||
cat >"$TMP/bin/gh" <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
set -u
|
||||
# Every call is recorded, so a probe can assert the gather did NOT run — a
|
||||
# refusal that still reads is not a refusal (#198).
|
||||
[ -z "${GH_CALL_LOG:-}" ] || printf '%s\n' "$*" >>"$GH_CALL_LOG"
|
||||
case "${FAKE_GH_MODE:-success}" in
|
||||
failure)
|
||||
echo "fake GraphQL read failed" >&2
|
||||
|
|
@ -138,18 +141,31 @@ action_boundary() {
|
|||
# Forgejo forge it must refuse by name, never produce a verdict from a graph
|
||||
# it did not read. #199 removes the declaration by making the gather REST.
|
||||
forgejo_boundary() {
|
||||
env PATH="$TMP/bin:$PATH" FAKE_GH_MODE=success \
|
||||
env PATH="$TMP/bin:$PATH" FAKE_GH_MODE=success GH_CALL_LOG="$TMP/gh-calls" \
|
||||
CEREMONY_FORGE=forgejo \
|
||||
GITHUB_REPOSITORY="heavy-duty/ceremony" PR_NUMBER=268 \
|
||||
GITHUB_ACTION_PATH="$ROOT/actions/refs-not-closing" \
|
||||
bash "$ENTRYPOINT"
|
||||
}
|
||||
check "on a forgejo forge the action refuses instead of verdicting" 1 \
|
||||
# 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 \
|
||||
"cannot speak it" forgejo_boundary
|
||||
check "...and the refusal names the client it declared" 1 "'gh' client" \
|
||||
forgejo_boundary
|
||||
check "...and names the client the forge actually needs" 1 "'rest' client" \
|
||||
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
|
||||
# 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() {
|
||||
: >"$TMP/gh-calls"
|
||||
forgejo_boundary >/dev/null 2>&1
|
||||
wc -l <"$TMP/gh-calls"
|
||||
}
|
||||
check "...and reached the forge zero times" 0 "0" forgejo_read_count
|
||||
|
||||
check "action boundary fails when GraphQL read fails" 42 \
|
||||
"fake GraphQL read failed" action_boundary failure
|
||||
|
|
|
|||
Loading…
Reference in a new issue