From 5797b418b9a378948c255fa6ed5a07f4792b355d Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Sun, 2 Aug 2026 18:41:03 +0000 Subject: [PATCH] feat(forge): replace both gh api graphql sites with REST + a body parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Term 3 of #188. Forgejo has no GraphQL API, so these two gathers could not be translated — there is no endpoint to translate them to. A real forgejo-runner job says so from the other side: GITHUB_GRAPHQL_URL arrives set to the empty string (probe task 278). MERGED_REF_PR_RECORDS was already a body parse; GraphQL was buying pagination, nothing semantic. OPEN_PR_ISSUES used GitHub's own parse of the closing keywords, so it becomes lib/closes_references.sh — a sibling of refs_references, sharing its LOCAL/CROSS classifier so rig#112 can still never be read as local #112 (#61). Both gathers now read /pulls, which /api/v3 and /api/v1 return in the same shape (measured on both). merged_at replaces GraphQL's states: MERGED. Bodies travel base64: jq's @tsv escapes a newline to a literal backslash-n, which a line parser reads as one line and loses every declaration after the first. The accepted delta, written down rather than rediscovered: GitHub also records closing links attached through the PR development sidebar, which live in no body. This family declares links in the body, so the delta is zero here. Refs #188 --- .../issueflow-reconcile.sh | 56 ++++++------ changelog.d/188.md | 10 +++ lib/closes_references.sh | 79 +++++++++++++++++ lib/forge.sh | 2 +- test/closes_references.test.sh | 87 +++++++++++++++++++ test/issueflow-reconcile.test.sh | 20 +++-- 6 files changed, 220 insertions(+), 34 deletions(-) create mode 100644 lib/closes_references.sh create mode 100644 test/closes_references.test.sh diff --git a/actions/issueflow-reconcile/issueflow-reconcile.sh b/actions/issueflow-reconcile/issueflow-reconcile.sh index d876ff4..6f18063 100644 --- a/actions/issueflow-reconcile/issueflow-reconcile.sh +++ b/actions/issueflow-reconcile/issueflow-reconcile.sh @@ -29,6 +29,8 @@ TRIAGE_ACTORS=() . "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/ruling.sh" # shellcheck source=lib/forge.sh . "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/forge.sh" +# shellcheck source=lib/closes_references.sh +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../lib/closes_references.sh" log() { printf 'issueflow: %s\n' "$*"; } run() { if [ -n "${DRY_RUN:-}" ]; then log "DRY_RUN: $*"; else "$@"; fi; } @@ -492,40 +494,42 @@ main() { # reported a green blind sweep. CEREMONY_FORGE_CLIENT="${CEREMONY_FORGE_CLIENT:-gh}" forge_preflight || return 1 - local owner name REPO="${REPO:?set REPO to owner/name}" LABELS_CONF="${LABELS_CONF:-.github/labels.conf}" load_issueflow_config "$LABELS_CONF" if [ "${EVENT_NAME:-}" = issues ] && [ "${EVENT_ACTION:-}" = opened ]; then reconcile_opened_issue "${EVENT_ISSUE:?set EVENT_ISSUE for issues:opened}" fi - owner="${REPO%%/*}" - name="${REPO#*/}" - OPEN_PR_ISSUES="$(gh api graphql --paginate -f owner="$owner" -f name="$name" -f query=' - query($owner: String!, $name: String!, $endCursor: String) { - repository(owner: $owner, name: $name) { - pullRequests(first: 100, states: OPEN, after: $endCursor) { - nodes { closingIssuesReferences(first: 100) { nodes { number } } } - pageInfo { hasNextPage endCursor } - } - } - }' --jq '.data.repository.pullRequests.nodes[].closingIssuesReferences.nodes[].number' \ - | sort -nu)" - MERGED_REF_PR_RECORDS="$(gh api graphql --paginate -f owner="$owner" -f name="$name" -f query=' - query($owner: String!, $name: String!, $endCursor: String) { - repository(owner: $owner, name: $name) { - pullRequests(first: 100, states: MERGED, after: $endCursor) { - nodes { number body } - pageInfo { hasNextPage endCursor } - } - } - }' --jq '.data.repository.pullRequests.nodes[] - | .number as $pr | .body | split("\n")[] - | [$pr, .] | @tsv' \ - | while IFS=$'\t' read -r pr body; do + # owner/name split out here until #188 — the GraphQL query took them as + # separate variables. REST takes the owner/name path whole, so it is gone. + # Both gathers were `gh api graphql` until #188. Forgejo has NO GraphQL + # API — a real forgejo-runner job even arrives with GITHUB_GRAPHQL_URL set + # to the empty string (probe task 278) — so these could not be translated + # to a Forgejo endpoint; there is none. They are REST + a parser this repo + # owns, over `number` and `body`, which /api/v3 and /api/v1 both return in + # the same shape (measured on both, 2026-08-02). + # + # Bodies travel base64 because they contain newlines: jq's @tsv escapes a + # newline to a literal backslash-n, which a line-oriented parser reads as + # one line and silently loses every declaration after the first. The old + # GraphQL gather sidestepped that with `split("\n")[]`; base64 is the same + # protection without needing the split to be correct. + OPEN_PR_ISSUES="$(gh api --paginate "repos/$REPO/pulls?state=open&per_page=100" \ + --jq '.[] | .body // "" | @base64' \ + | while IFS= read -r b64; do + [ -n "$b64" ] && printf '%s' "$b64" | base64 -d | closes_references + done | sort -nu)" + # closes_references, not refs_references: GitHub's closingIssuesReferences + # meant the CLOSING relation specifically, and reading Refs as closing + # would make every referenced issue look closeable — the distinction #151 + # was reopened by hand over. + MERGED_REF_PR_RECORDS="$(gh api --paginate "repos/$REPO/pulls?state=closed&per_page=100" \ + --jq '.[] | select(.merged_at != null) | "\(.number)\t\(.body // "" | @base64)"' \ + | while IFS=$'\t' read -r pr b64; do + [ -n "$b64" ] || continue while IFS= read -r issue; do [ -n "$issue" ] && printf '%s\t%s\n' "$issue" "$pr" - done < <(refs_references <<<"$body") + done < <(printf '%s' "$b64" | base64 -d | refs_references) done)" local n diff --git a/changelog.d/188.md b/changelog.d/188.md index 4c7bc52..e6357dd 100644 --- a/changelog.d/188.md +++ b/changelog.d/188.md @@ -7,6 +7,16 @@ - The reconcilers and `labels-scope` run that preflight first, so a GitHub-shaped client on a Forgejo instance is a named refusal instead of a sweep that reads nothing and reports success (#188). +- `lib/closes_references.sh` — the closing-keyword parser, sibling of + `refs_references`, so "which issues does this PR close" is answered from a + PR body rather than from GitHub's GraphQL API (#188). + +### Changed + +- `issueflow-reconcile` gathers open and merged PRs over REST instead of + `gh api graphql`. Forgejo serves no GraphQL at all, so the two queries were + replaced rather than translated; both forges return `number` and `body` + from `/pulls` in the same shape (#188). ### Fixed diff --git a/lib/closes_references.sh b/lib/closes_references.sh new file mode 100644 index 0000000..d77bef8 --- /dev/null +++ b/lib/closes_references.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +# lib/closes_references.sh — "which issues does this PR body close?", parsed +# here rather than asked of a forge (issue #188, term 3). +# +# Sourced, never executed: no set -e/-u — the sourcing script owns its shell +# options, as lib/version.sh and lib/forge.sh do. +# +# WHY THIS EXISTS. issueflow-reconcile asked GitHub's GraphQL API for +# `closingIssuesReferences` — GitHub's own parse of the closing keywords in +# a PR body. **Forgejo has no GraphQL API at all**, and the runner confirms +# it from the other side: a real forgejo-runner job arrives with +# GITHUB_GRAPHQL_URL set to the empty string (probe task 278, 2026-08-02). +# So that call site could not be translated to a Forgejo endpoint — there is +# nothing to translate it to. It had to be replaced by a parse this repo +# owns, over a field both forges already return: +# `GET /repos/{owner}/{repo}/pulls` carries `number` and `body` on +# /api/v3 and /api/v1 alike (measured on both). +# +# That the replacement is honest is the point. The sibling half of the same +# GraphQL query, MERGED_REF_PR_RECORDS, was ALREADY a body parse — it pulled +# `number` and `body` and ran them through refs_references. GraphQL was +# buying pagination convenience there, nothing semantic. This file makes the +# other half symmetric: one parser this repo controls and can test, for both +# link kinds, on both forges. +# +# THE ACCEPTED DELTA, stated so it is not rediscovered as a bug: GitHub also +# records closing links attached through the pull request's development +# sidebar, which live in no body and which no body parse can see. This +# family declares its links in the body — that is what BUILDER.md's PR +# template asks for — so the delta is zero in practice here. A consumer that +# links through the sidebar would see those issues go unclosed by the sweep; +# they would need to say so in the body instead. +# +# DEPENDENCY: issue_references, from issueflow-reconcile.sh — the LOCAL / +# CROSS classifier that keeps rig#112 from ever being read as local #112 +# (#61). Bash resolves function calls at call time, so the order of sourcing +# does not matter; both must simply be defined before closes_references runs. +# refs_references depends on it exactly the same way. + +# closes_references — PR body on stdin -> local issue numbers this body +# declares it CLOSES, sorted, unique. +# +# The keyword set is GitHub's documented one, all three verbs in all three +# tenses. Matching is case-insensitive because bodies are written by humans +# and agents both ("Closes", "closes", "CLOSES"). +# +# Deliberately NOT matched: "Refs #N". That is the other relation entirely — +# refs_references owns it, and conflating them would make every referenced +# issue look closeable, which is the post-merge transition #151 had to be +# reopened by hand over. +closes_references() { + awk ' + { + line = $0 + lower = tolower(line) + # Every occurrence contributes, not just the first: a body that says + # "Closes #1. Closes #2." declares two, and binding to the first + # occurrence dropped the later ones — the same defect #184 fixed in + # blocked_reference_records, kept fixed here by construction. + while (match(lower, /(^|[^[:alnum:]_-])(close[sd]?|fix(e[sd])?|resolve[sd]?)[[:space:]:]+/)) { + # BOTH cursors advance together. Advancing only `lower` left the + # next match offset indexing the ORIGINAL line, so the second + # declaration on a line came back as garbage — caught by the + # "two closes on one line" case, which is why it is a case. + rest = substr(line, RSTART + RLENGTH) + line = rest + lower = tolower(rest) + if (rest ~ /^(#|([[:alnum:]_.-]+\/)?[[:alnum:]_.-]+#)[0-9]+/) { + token = rest + # Stop at the first thing that cannot be part of a reference, so + # "Closes #12, and more prose" yields #12 and not the sentence. + sub(/[^[:alnum:]_.\/#-].*/, "", token) + print token + } + } + } + ' | issue_references \ + | awk -F '\t' '$1 == "LOCAL" { print $2 }' | sort -nu +} diff --git a/lib/forge.sh b/lib/forge.sh index 8100001..7b6e03c 100644 --- a/lib/forge.sh +++ b/lib/forge.sh @@ -170,7 +170,7 @@ EOF # to happen. local missing_bins=() bin case "$want" in - gh) for bin in gh; do command -v "$bin" >/dev/null 2>&1 || missing_bins+=("$bin"); done ;; + gh) command -v gh >/dev/null 2>&1 || missing_bins+=(gh) ;; rest) for bin in curl jq; do command -v "$bin" >/dev/null 2>&1 || missing_bins+=("$bin"); done ;; esac if [ "${#missing_bins[@]}" -gt 0 ]; then diff --git a/test/closes_references.test.sh b/test/closes_references.test.sh new file mode 100644 index 0000000..330ed1e --- /dev/null +++ b/test/closes_references.test.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# Contract tests for lib/closes_references.sh (issue #188, term 3). +# set -u, not -e: failing commands are behavior for the harness to inspect. +set -u + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +# shellcheck source=test/harness.sh +. "$ROOT/test/harness.sh" +# issue_references (the LOCAL/CROSS classifier) lives here; closes_references +# calls it, exactly as refs_references does. +# shellcheck source=actions/issueflow-reconcile/issueflow-reconcile.sh +. "$ROOT/actions/issueflow-reconcile/issueflow-reconcile.sh" +# shellcheck source=lib/closes_references.sh +. "$ROOT/lib/closes_references.sh" + +# closes — the parse of is exactly +# . Exact, not substring: "12" is contained in "123". +closes() { + local want="$1" body="$2" got + got="$(printf '%s' "$body" | closes_references)" + [ "$got" = "$want" ] +} + +# --- the three verbs, the three tenses ---------------------------------- +# GitHub's documented keyword set. All of them, because a body that says +# "Fixed #4" and goes unclosed is a silent failure of the post-merge +# transition, not a loud one. + +check "closes" 0 "" closes 1 'Closes #1' +check "close" 0 "" closes 1 'Close #1' +check "closed" 0 "" closes 1 'Closed #1' +check "fixes" 0 "" closes 2 'Fixes #2' +check "fix" 0 "" closes 2 'Fix #2' +check "fixed" 0 "" closes 2 'Fixed #2' +check "resolves" 0 "" closes 3 'Resolves #3' +check "resolve" 0 "" closes 3 'Resolve #3' +check "resolved" 0 "" closes 3 'Resolved #3' +check "case-insensitive" 0 "" closes 4 'CLOSES #4' +check "lowercase" 0 "" closes 4 'closes #4' +check "colon form" 0 "" closes 5 'Closes: #5' + +# --- Refs is NOT a closing link ----------------------------------------- +# The relation this file must not swallow. refs_references owns Refs, and +# conflating them makes every referenced issue look closeable — the +# post-merge transition #151 was reopened by hand over exactly that +# distinction. + +check "Refs is not a close" 0 "" closes '' 'Refs #7' +check "Refs and Closes in one body keeps only the close" 0 "" \ + closes 8 $'Refs #7\nCloses #8' + +# --- cross-repo references stay out (#61) ------------------------------- +# rig#112 must never be read as local #112. The classifier is shared with +# refs_references precisely so this rule has one implementation. + +check "qualified reference is not local" 0 "" closes '' 'Closes rig#112' +check "owner-qualified reference is not local" 0 "" \ + closes '' 'Closes heavy-duty/rig#112' +check "a local and a cross reference keep only the local" 0 "" \ + closes 9 $'Closes rig#112\nCloses #9' + +# --- every occurrence contributes --------------------------------------- +# Binding to the first occurrence is the defect #184 fixed in +# blocked_reference_records; this parser must not reintroduce it. + +check "two closes on one line" 0 "" closes $'1\n2' 'Closes #1. Closes #2.' +check "two closes on two lines" 0 "" closes $'1\n2' $'Closes #1\nCloses #2' +check "sorted and deduplicated" 0 "" closes $'2\n10' $'Closes #10\nCloses #2\nCloses #10' + +# --- prose must not be swallowed ---------------------------------------- + +check "trailing prose is not part of the reference" 0 "" \ + closes 12 'Closes #12, and adds the guard' +check "a sentence terminator ends the reference" 0 "" closes 13 'Closes #13.' +check "no reference means no output" 0 "" closes '' 'Closes the door behind it' +check "a bare issue mention is not a close" 0 "" closes '' 'See #14 for context' +# "unclosed" contains "close" — a naive word match would fire on it. +check "a word merely containing a verb does not fire" 0 "" \ + closes '' 'This left #15 unclosed' + +# --- the shapes a real PR body carries ---------------------------------- + +check "the template's leading declaration" 0 "" \ + closes 188 $'Closes #188\n\n## Acceptance criteria\n\n- [ ] a thing' +check "an empty body yields nothing" 0 "" closes '' '' + +summary diff --git a/test/issueflow-reconcile.test.sh b/test/issueflow-reconcile.test.sh index ef41045..4812d2e 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -577,9 +577,11 @@ echo "gh stub: unexpected call: gh $*" >&2 exit 97 EOF chmod +x "$ARRIVAL/stub/gh" -printf '%s\n' \ - '{"data":{"repository":{"pullRequests":{"nodes":[],"pageInfo":{"hasNextPage":false,"endCursor":null}}}}}' \ - >"$ARRIVAL/fixtures/graphql.json" +# The two PR gathers were GraphQL until #188; they are REST now, so the +# fixtures are the /pulls list both forges return. Empty by default — the +# merged-Refs case below fills the closed one. +printf '[]\n' >"$ARRIVAL/fixtures/repos_owner_repo_pulls_state_open_per_page_100.json" +printf '[]\n' >"$ARRIVAL/fixtures/repos_owner_repo_pulls_state_closed_per_page_100.json" arrival_fixture() { printf '%s\n' "$1" >"$ARRIVAL/fixtures/repos_owner_repo_issues_91.json"; } # CEREMONY_FORGE=github below, and at the executable-sweep driver further # down: these fixtures ARE a GitHub board (a gh stub on PATH answering @@ -626,11 +628,15 @@ check "...and the sweep still runs" 0 "" \ grep -qF 'issueflow: reconciled.' <<<"$pr_out" # The merged-Refs transition must survive the executable's set -e path too. -# Keep this at main() granularity: the GraphQL gather and loop are the code -# a sourced decision probe cannot exercise (#91's lesson). +# Keep this at main() granularity: the PR gather and loop are the code a +# sourced decision probe cannot exercise (#91's lesson). +# +# merged_at is what makes this PR merged rather than merely closed — the +# REST replacement for GraphQL's states: MERGED filter (#188). Both forges +# return the field, and both return null on a closed-unmerged PR. printf '%s\n' \ - '{"data":{"repository":{"pullRequests":{"nodes":[{"number":400,"body":"Refs #40","closingIssuesReferences":{"nodes":[]}}],"pageInfo":{"hasNextPage":false,"endCursor":null}}}}}' \ - >"$ARRIVAL/fixtures/graphql.json" + '[{"number":400,"body":"Refs #40","merged_at":"2026-07-30T00:00:00Z"},{"number":401,"body":"Refs #40","merged_at":null}]' \ + >"$ARRIVAL/fixtures/repos_owner_repo_pulls_state_closed_per_page_100.json" printf '[{"number":40}]\n' \ >"$ARRIVAL/fixtures/repos_owner_repo_issues_state_open_per_page_100.json" jq -n --arg at "$(iso_at "$INOW")" \