fix(forge): an empty REPO cannot become a fact, and the backend verbs are tested
All checks were successful
CI / test (pull_request) Successful in 1m28s
CI / release-exercise (pull_request) Successful in 11s
CI / self-guards (pull_request) Successful in 6s
CI / action-exercise (pull_request) Successful in 5s
CI / docs-sync-exercise (pull_request) Successful in 5s
labels / labels (pull_request) Successful in 1m31s

Both panel blockers on c63a550.

@kimi found the one that mattered: facts.sh got the REPO fix, release.yml's
own four call sites did not. A workflow `run:` shell carries no `set -u`, so
an unset REPO expands empty and the verb addresses `repos//…` — which 404s,
and the 404 is then read as an ANSWER. Reproduced read-only against this
instance before fixing:

  forge_release_exists 0.4.1   -> "no", rc 0
  forge_commit_pulls 7fc9afe4  -> "[]", rc 0    (the !189 merge, which HAS a
                                                 merged PR behind it)

The first would have let the nothing-exists assert proceed to CREATE; the
second is the drill's original fabricated `labeled=no`, one step after the
fix meant to kill it.

Fixed once rather than at four call sites, as kimi suggested: forge_select
defaults REPO from GITHUB_REPOSITORY, and forgejo_api_base — which every
verb reaches the network through — refuses an empty REPO outright. No fifth
call site can forget it.

@grok and @kimi both blocked on the same AC gap: the backend suite did not
cover the five new verbs, so the two measured asymmetries had no offline
coverage. test/forge-backends.test.sh now has 15 cases for them — singular
/pull wrapped to an array, 404 as an empty array, 500 refusing, release
present/absent/unreadable, POST /tags vs /git/refs, the publish body, and
the REPO-empty must-fail. Mutation-checked: reading the plural path fails
one case, dropping the REPO guard fails the two must-fails.

1029 assertions, 22 suites, shellcheck-all and actionlint clean.

Refs #191

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
cluade-reviewer-andresmgsl 2026-08-04 11:53:28 +00:00
parent c63a55067e
commit 21c70e06a4
4 changed files with 147 additions and 0 deletions

View file

@ -22,3 +22,7 @@
- Forgejo creates tags at `POST /tags` — it serves `/git/refs` GET-only,
so GitHub's ref-POST would have 404'd there forever (#191).
- `forgejo_api_base` refuses when `REPO` is empty. Every verb interpolates
it and every call reaches the network through there, so `repos//…`
whose 404 reads as "no release" and "no PRs" — is now impossible (#191).

View file

@ -18,6 +18,30 @@ forgejo_api_base() {
echo "forgejo_api_base: no GITHUB_API_URL or CEREMONY_FORGE_API — cannot reach the forge (#188)" >&2
return 1
fi
# Every verb in this backend interpolates $REPO into its path, and every
# one of them reaches the network through here — so this is the one place
# that can make `repos//…` impossible.
#
# THE TRAP, measured on this instance with REPO unset (#191, caught by
# @kimi on !193 before it shipped):
#
# forge_release_exists 0.4.1 -> "no", rc 0 (repos//releases/tags/0.4.1
# 404s; a repo-less path read
# as "the release does not
# exist" — and the
# nothing-exists assert would
# then proceed to CREATE)
# forge_commit_pulls <sha> -> "[]", rc 0 (a commit that HAS a merged
# PR behind it, read as none)
#
# A workflow `run:` shell carries no `set -u`, so an unset REPO expands
# empty and 404s into a fabricated fact instead of crashing. That is the
# exact failure #191 exists to remove, so it refuses here rather than
# anywhere later.
if [ -z "${REPO:-}" ]; then
echo "forgejo_api_base: REPO is empty — refusing to address 'repos//…', whose 404 would read as a fact (#191)" >&2
return 1
fi
printf '%s\n' "${base%/}"
}

View file

@ -126,6 +126,12 @@ FORGE_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
# environment decides via forge_detect.
forge_select() {
local forge="${1:-}"
# One default for every consumer: the forgejo backend addresses the repo
# through REPO, the github backend reads GITHUB_REPOSITORY. Defaulting
# here means no call site — workflow step or script — can forget it and
# get a repo-less path (#191). The reconcilers still assert their own.
REPO="${REPO:-${GITHUB_REPOSITORY:-}}"
export REPO
if [ -z "$forge" ]; then
forge="$(forge_detect)" || return 1
fi

View file

@ -44,6 +44,12 @@ check "select with no argument reads the environment" 0 "" \
. "$ROOT/lib/forge-github.sh"
. "$ROOT/lib/forge-forgejo.sh"
# Every /api/v1 call ceremony makes is repo-scoped, and since #191 the
# backend refuses to build `repos//…` — so the suite names a repo up front,
# the way every real caller does.
REPO=o/r
export REPO
check "github: a bare path gets a query" 0 "" \
eq 'repos/o/r/issues?per_page=100' github_page_url 'repos/o/r/issues'
check "github: an existing query is preserved" 0 "" \
@ -616,4 +622,111 @@ check "...and never fetches a zero-comment review" 1 "" \
check "...and never hits the flat /pulls/{n}/comments endpoint" 1 "" \
grep -E '/pulls/[0-9]+/comments(\?|$)' "$activity_calls"
# --- the release door's verbs, both backends (#191) -----------------------
# The five verbs the release path now goes through. These carry two measured
# asymmetries that would 404 forever if wrong, and neither is visible to a
# github-only suite:
#
# PRs behind a commit GitHub GET /commits/{sha}/pulls -> ARRAY
# Forgejo GET /commits/{sha}/pull -> ONE OBJECT
# (the plural 404s)
# tag creation GitHub POST /git/refs
# Forgejo POST /tags (/git/refs is GET-only)
# release_stub <code> <body> — a curl stub answering one canned response and
# recording the method+path it was asked for.
release_stub() {
# Globals, not locals: the curl closure below runs long after this
# function returns, exactly as stub_writes does above.
STUB_CODE="$1" STUB_BODY="$2"
: >"$WRITES"
# shellcheck disable=SC2317 # invoked indirectly, by the forge verbs
curl() {
local hdr="" out="" method=GET url="" payload=""
while [ $# -gt 0 ]; do
case "$1" in
-D) hdr="$2"; shift ;;
-o) out="$2"; shift ;;
-X) method="$2"; shift ;;
-d) payload="$2"; shift ;;
-H | -F) shift ;;
-*) ;;
*) url="$1" ;;
esac
shift
done
[ -n "$hdr" ] && printf 'HTTP/1.1 %s x\r\n\r\n' "$STUB_CODE" >"$hdr"
[ -n "$out" ] && printf '%s' "$STUB_BODY" >"$out"
printf '%s %s %s\n' "$method" "${url##*/api/v1/}" "$payload" >>"$WRITES"
return 0
}
}
GITHUB_API_URL=https://forge.example/api/v1
export GITHUB_API_URL
# Helpers so the assertions run in THIS shell, where the verbs are defined.
pulls_is_array() { forge_commit_pulls "$1" | jq -e 'type == "array" and length == 1' >/dev/null && echo array-of-1; }
writes_after() { "$@" >/dev/null 2>&1; cat "$WRITES"; }
repo_empty_release() { REPO='' forge_release_exists 1.2.3; }
repo_empty_pulls() { REPO='' forge_commit_pulls deadbeef; }
release_stub 200 '{"number":7,"merged_at":"2026-01-01T00:00:00Z","labels":[{"name":"release"}]}'
check "forgejo: one PR object becomes a one-element array" 0 '"number":7' \
forge_commit_pulls deadbeef
check "forgejo: the array is what the call site's jq expects" 0 "array-of-1" \
pulls_is_array deadbeef
check "forgejo: it reads the SINGULAR path" 0 "commits/deadbeef/pull " \
writes_after forge_commit_pulls deadbeef
release_stub 404 '{"message":"pull request does not exist"}'
check "forgejo: 404 is an empty array, not a failure" 0 "[]" forge_commit_pulls deadbeef
release_stub 500 '{}'
check "forgejo: a 500 refuses rather than saying 'none'" 1 "the answer is unknown, not 'none'" \
forge_commit_pulls deadbeef
release_stub 200 '{"tag_name":"1.2.3"}'
check "forgejo: a present release is yes" 0 "yes" forge_release_exists 1.2.3
release_stub 404 '{}'
check "forgejo: an absent release is no" 0 "no" forge_release_exists 1.2.3
release_stub 503 '{}'
check "forgejo: an unreadable release refuses, not 'no'" 1 "the answer is unknown, not 'no'" \
forge_release_exists 1.2.3
# THE MUST-FAIL (#191, found by @kimi on !193 before it shipped): with REPO
# empty every path becomes repos//… , whose 404 would read as a fact — "no"
# and "[]" with rc 0. That is the bug this issue exists to remove.
release_stub 404 '{}'
check "REPO empty refuses instead of fabricating 'no'" 1 "refusing to address 'repos//" \
repo_empty_release
check "REPO empty refuses instead of fabricating '[]'" 1 "refusing to address 'repos//" \
repo_empty_pulls
release_stub 201 '{"id":42}'
check "forgejo: a tag is created at /tags, not /git/refs" 0 "POST repos/o/r/tags" \
writes_after forge_tag_create 1.2.3 cafebabe
release_stub 201 '{"id":42}'
check "forgejo: the tag body names the target sha" 0 '"target":"cafebabe"' \
writes_after forge_tag_create 1.2.3 cafebabe
printf 'notes body\n' >"$TMP/notes.md"
release_stub 201 '{"id":42}'
check "forgejo: the publish POSTs to /releases with the notes as body" 0 '"body":"notes body' \
writes_after forge_release_create 1.2.3 1.2.3 "$TMP/notes.md"
# --- the github twins address their own paths ----------------------------
. "$ROOT/lib/forge-github.sh"
GITHUB_REPOSITORY=o/r
export GITHUB_REPOSITORY
GH_CALLS="$TMP/ghcalls"
# shellcheck disable=SC2317 # invoked indirectly, by the forge verbs
gh() { printf '%s\n' "$*" >>"$GH_CALLS"; case "$*" in *commits/*) echo '[]' ;; esac; return 0; }
gh_after() { : >"$GH_CALLS"; "$@" >/dev/null 2>&1; cat "$GH_CALLS"; }
check "github: the tag goes to /git/refs" 0 "git/refs" \
gh_after forge_tag_create 1.2.3 cafebabe
check "github: PRs behind a commit use the PLURAL path" 0 "commits/deadbeef/pulls" \
gh_after forge_commit_pulls deadbeef
summary