fix(198): the workflow declares and refuses instead of being exempted by name (#198)
Some checks failed
CI / test (pull_request) Failing after 3m3s
CI / release-exercise (pull_request) Successful in 11s
CI / self-guards (pull_request) Successful in 7s
CI / action-exercise (pull_request) Successful in 5s
CI / docs-sync-exercise (pull_request) Successful in 6s
Refs guard / refs-not-closing (pull_request) Successful in 5s
labels / labels (pull_request) Successful in 46s
Some checks failed
CI / test (pull_request) Failing after 3m3s
CI / release-exercise (pull_request) Successful in 11s
CI / self-guards (pull_request) Successful in 7s
CI / action-exercise (pull_request) Successful in 5s
CI / docs-sync-exercise (pull_request) Successful in 6s
Refs guard / refs-not-closing (pull_request) Successful in 5s
labels / labels (pull_request) Successful in 46s
@codex-reviewer-andresmgsl's blocker 1 is right and the filename exemption was
the wrong shape. It exempted the whole FILE — any later `gh` call anywhere in
labels.yml would have ridden in free — and it let the merge ship a step that
dies with `command not found` on every sweep on this forge, which #197's bar
does not permit.
The declaration mechanism already existed; a workflow simply could not reach
it. It can: `CEREMONY_FORGE_CLIENT: gh` in the step's env is the same
declaration actions/refs-not-closing carries, and the refusal that a script
gets from forge_preflight is inline here because a workflow has no shell to
call it from. The dispatch now warns by name, cites #205, and exits 0 rather
than reddening every sweep for a known gap.
So the guard needs no exemption list at all. It now requires the pair —
declared AND refusing — and reports a declaration that carries no refusal,
which is a permission slip for `command not found`.
That predicate was wrong on its first write, and its mutation test caught it:
`refuses_when_unavailable` matched the word `forge_preflight` inside
labels.yml's own comment explaining that it has NO forge_preflight to call. A
guard reading prose as evidence is the blind sweep again, in the guard written
to forbid it. Comments are stripped now, as gh_calls already stripped them.
Blocker 4: the nudge strips a trailing slash from the server URL. Reverting the
strip reds two cases.
Blockers 2 and 3 were already fixed in 97e63ac, before either review landed.
test/run.sh 28 files 0 failed under CI's env; shellcheck 0.10.0 (CI's pin),
actionlint, self-ref, marker, vendored and changelog-armed all clean, with
every file tracked this time.
Refs #198
This commit is contained in:
parent
97e63acef0
commit
06f05aebec
5 changed files with 90 additions and 23 deletions
17
.github/workflows/labels.yml
vendored
17
.github/workflows/labels.yml
vendored
|
|
@ -128,4 +128,19 @@ jobs:
|
|||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
SWEEP_WORKFLOW: ${{ inputs.sweep_workflow }}
|
||||
run: gh workflow run "$SWEEP_WORKFLOW" -R "$GITHUB_REPOSITORY" -f bootstrap=no
|
||||
# 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
|
||||
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).
|
||||
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."
|
||||
exit 0
|
||||
fi
|
||||
gh workflow run "$SWEEP_WORKFLOW" -R "$GITHUB_REPOSITORY" -f bootstrap=no
|
||||
|
|
|
|||
|
|
@ -949,7 +949,12 @@ The merge releases the claim; no builder owes a draft. Triage owes completion in
|
|||
# how the wrong addressee comes back.
|
||||
if [ "$(ruling_nudge_decision "$NOW" "$evidence_age")" = NUDGE ]; then
|
||||
local quiet_days=$(((NOW - evidence_age) / 86400))
|
||||
run forge_issue_comment "$n" "@${TRIAGE_ACTORS[0]} — this \`post-merge\` item has had no comment for ${quiet_days} days: ${GITHUB_SERVER_URL:-}/$REPO/issues/$n
|
||||
# Trailing slash stripped so a server URL that carries one does not
|
||||
# render `//owner/repo` — `:-` first so an absent value is empty rather
|
||||
# than fatal, `%/` second so a present one is normalized (#198).
|
||||
local server="${GITHUB_SERVER_URL:-}"
|
||||
server="${server%/}"
|
||||
run forge_issue_comment "$n" "@${TRIAGE_ACTORS[0]} — this \`post-merge\` item has had no comment for ${quiet_days} days: ${server}/$REPO/issues/$n
|
||||
|
||||
Its wake evidence is still owed. \`post-merge\` means the merge landed and
|
||||
triage owns completion — judge the remaining criteria against the evidence
|
||||
|
|
|
|||
|
|
@ -36,6 +36,13 @@
|
|||
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/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).
|
||||
|
||||
- The post-merge nudge strips a trailing slash from the server URL, so a forge
|
||||
URL carrying one does not render `//owner/repo` (#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).
|
||||
|
|
|
|||
|
|
@ -785,6 +785,18 @@ check "...addressed to the triage actor, never the human reviewer" 0 "" \
|
|||
bash -c 'grep -qF "@triage-one" "$1" && ! grep -qF "@danmt" "$1"' _ "$TMP/posted-80"
|
||||
check "...with the issue link as the payload" 0 "" \
|
||||
grep -qF 'https://github.com/owner/repo/issues/80' "$TMP/posted-80"
|
||||
# The host comes from the environment, and a server URL that carries a
|
||||
# trailing slash must not render `//owner/repo` (@codex-reviewer-andresmgsl,
|
||||
# #198). Same forge, same issue, one character of difference in the input.
|
||||
quiet_comment 80 $((8 * 86400))
|
||||
PROBE_SERVER_URL=https://forgejo.example.test/ issue_probe 80 post-merge 0 >/dev/null
|
||||
check "a trailing slash on the server URL does not double the separator" 0 "" \
|
||||
grep -qF 'https://forgejo.example.test/owner/repo/issues/80' "$TMP/posted-80"
|
||||
check "...and no doubled separator appears at all" 1 "" \
|
||||
grep -qF 'forgejo.example.test//owner' "$TMP/posted-80"
|
||||
quiet_comment 80 $((8 * 86400))
|
||||
: >"$TMP/posted-80"
|
||||
issue_probe 80 post-merge 0 >/dev/null
|
||||
check "...carrying the do-not-add-a-marker warning in the comment" 0 "" \
|
||||
grep -qF 'Do not add a marker.' "$TMP/posted-80"
|
||||
# Asserted directly, not merely omitted: a marker would turn "once per 7
|
||||
|
|
|
|||
|
|
@ -36,30 +36,32 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
|||
# The backend that is ALLOWED to speak gh — it is the whole point of the file.
|
||||
ALLOWED_FILE='lib/forge-github.sh'
|
||||
|
||||
# The one exemption a FILE cannot declare for itself. A workflow has no shell
|
||||
# to source lib/forge.sh from and no forge_preflight to refuse with, so the
|
||||
# CEREMONY_FORGE_CLIENT escape hatch that covers actions/refs-not-closing is
|
||||
# unavailable to it. `.github/workflows/labels.yml`'s trigger job dispatches
|
||||
# the sweep caller with `gh workflow run`; the 0.6.0 merge introduced it
|
||||
# (#209 upstream) and it is the eighth call site that merge brought in — the
|
||||
# one every reviewer's `*.sh` grep missed, this one included, until this guard
|
||||
# read the workflows too.
|
||||
#
|
||||
# It is NOT ported here, deliberately. Forgejo's dispatch route exists but
|
||||
# does not answer like GitHub's: `GET /actions/workflows` 404s on this
|
||||
# instance while `POST .../dispatches` returns 500 rather than a 4xx, which is
|
||||
# the same mis-status class #192 is open about. Porting on that evidence would
|
||||
# be guessing, and the only way to finish measuring it is to dispatch a real
|
||||
# workflow run on the operator's repo. So it is named here with its reason and
|
||||
# its follow-up, which is what an exemption is for — an unnamed one is just a
|
||||
# hole. #205 owns the port; remove this entry when it lands.
|
||||
EXEMPT_WORKFLOWS='.github/workflows/labels.yml'
|
||||
|
||||
# A file may opt out by declaring the client it speaks, which makes
|
||||
# forge_preflight refuse by name on a forge that cannot serve it. Today that
|
||||
# is actions/refs-not-closing, whose only gather is GraphQL and which Forgejo
|
||||
# therefore cannot run at all (#199 ports it and drops the declaration).
|
||||
declares_gh_client() { grep -qE '^[[:space:]]*(export[[:space:]]+)?CEREMONY_FORGE_CLIENT=gh\b' "$1"; }
|
||||
# Both spellings, because both surfaces must be able to declare: `=` for a
|
||||
# shell script, `:` for a workflow's env block. A filename exemption was the
|
||||
# first shape here and @codex-reviewer-andresmgsl was right to reject it —
|
||||
# it exempts the whole FILE, so any later gh call anywhere in that workflow
|
||||
# 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.
|
||||
refuses_when_unavailable() {
|
||||
sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$1" \
|
||||
| grep -qE 'forge_preflight|command -v gh'
|
||||
}
|
||||
|
||||
# A runtime invocation, not the word. `gh` must be at a command position and
|
||||
# followed by a gh subcommand — and comment lines are stripped first, because
|
||||
|
|
@ -92,9 +94,12 @@ offenders() {
|
|||
local rel abs
|
||||
while IFS= read -r rel; do
|
||||
[ "$rel" = "$ALLOWED_FILE" ] && continue
|
||||
[ "$rel" = "$EXEMPT_WORKFLOWS" ] && continue
|
||||
abs="$ROOT/$rel"
|
||||
declares_gh_client "$abs" && continue
|
||||
if declares_gh_client "$abs"; then
|
||||
refuses_when_unavailable "$abs" && continue
|
||||
printf '%s: declares CEREMONY_FORGE_CLIENT=gh but carries no refusal\n' "$rel"
|
||||
continue
|
||||
fi
|
||||
gh_calls "$abs" | sed "s|^|$rel:|"
|
||||
done < <(scanned_files)
|
||||
}
|
||||
|
|
@ -154,6 +159,29 @@ check "lookalike identifiers are not call sites" 1 "" gh_calls "$TMP/lookalike.s
|
|||
printf '%s\n' '#!/usr/bin/env bash' 'export CEREMONY_FORGE_CLIENT=gh' \
|
||||
'gh api graphql -f query=x' >"$TMP/declared.sh"
|
||||
check "a declared-client file opts out" 0 "" declares_gh_client "$TMP/declared.sh"
|
||||
# A workflow declares in YAML, not shell — both spellings must count, or the
|
||||
# only surface that cannot call forge_preflight is also the only one that
|
||||
# cannot declare.
|
||||
printf '%s\n' 'jobs:' ' t:' ' steps:' ' - env:' \
|
||||
' CEREMONY_FORGE_CLIENT: gh' ' run: gh workflow run x' \
|
||||
>"$TMP/declared.yml"
|
||||
check "...and so does a workflow declaring it in YAML" 0 "" \
|
||||
declares_gh_client "$TMP/declared.yml"
|
||||
# Declared is not enough: #197's bar is declared AND refuses loudly.
|
||||
check "a declaration without a refusal is not enough" 1 "" \
|
||||
refuses_when_unavailable "$TMP/declared.yml"
|
||||
printf '%s\n' 'jobs:' ' t:' ' steps:' ' - env:' \
|
||||
' CEREMONY_FORGE_CLIENT: gh' \
|
||||
' 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 "" \
|
||||
refuses_when_unavailable "$TMP/declared-refusing.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 "...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' \
|
||||
|
|
|
|||
Loading…
Reference in a new issue