actions/refs-not-closing — gather over REST, so the guard produces verdicts on this forge (#199) #214
No reviewers
Labels
No labels
attention
blocked
blocker:ci-red
blocker:conflict
blocker:drill-pending
blocker:unrequested
bug
claimed
documentation
enhancement
epic
merge-next
needs-ruling
needs-triage
offsite
post-merge
ready
release
scope:docs
scope:guards
scope:labels
scope:release-flow
stale
state:addressing
state:bots-reviewing
state:building
state:needs-human
No milestone
No project
No assignees
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: heavy-duty/ceremony#214
Loading…
Reference in a new issue
No description provided.
Delete branch "build/199-refs-not-closing-rest"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What this does
actions/refs-not-closing's entire gather was one GraphQL query asking GitHubfor its own parse of the closing keywords. Forgejo serves no GraphQL at
all —
/api/graphql404s here, and a real forgejo-runner job arrives withGITHUB_GRAPHQL_URLset to the empty string. There was no endpoint totranslate it to, so it is re-expressed over two reads both backends already
serve, plus the parser this repo owns — exactly as #188 re-expressed its own
two GraphQL sites.
The part that is not a mechanical translation
This file used to call the graph authoritative "because it includes both
closing keywords and sidebar links". Those two halves resolve differently here:
So a body-only port would miss a PR that closes an issue from a commit
subject — precisely the contradiction this action exists to catch. The closing
set unions
closes_referencesover the body and every commit message.That is why the commit read is not optional, and there is a driven case for it.
The
hasNextPagerefusal is relocated, not dropped.--paginatecarriesthe forgejo backend's
x-total-countcompleteness proof (#188, #4699), so ashort gather refuses rather than returning a partial verdict — the same stance
upstream took past 100 closing references, wearing REST's clothes.
Two things this drags in, both deliberate
lib/issue_references.sh.closes_referencesdepends on the LOCAL/CROSSclassifier, and
lib/closes_references.sh's own header recorded the wart inprose: "DEPENDENCY: issue_references, from issueflow-reconcile.sh". That was
tolerable with one caller. A composite action cannot source a reconciler to
borrow one function — sourcing a reconciler runs one — so the dependency
the comment described is now a file. The classifier itself is unchanged, and
the reconciler's 512 tests pass against it.
refs-guard.yml's forge gate is removed in the same PR. #198 added it so aguard that could only refuse would not stand red on every PR here. Deleting the
client declaration without deleting the gate would leave the action portable
and never scheduled — a guard that passes by never running, this repo's
blind-sweep shape wearing a different hat
(@kimi-reviewer-andresmgsl, #198). The job runs here now.
Tests
Driven at the transport — a fake
gh apifor the github backend, a fakecurlfor the forgejo one. Stubbing the shim would prove only that the testcan stub the shim.
One of those cases was wrong when I first wrote it, and worth recording: the
partialstub served the same commit on every page, soforge_apiwalkeduntil it had collected exactly the declared total and passed honestly. A
short gather has to actually run out. The stub now serves one commit and then
nothing while declaring nine.
Verification
The offline incident matrix in
refs-not-closing.shis untouched, as the specasks — only the gather changed.
@andres — ready for review. Note this and !213 are independent: !213 ports the
sweep dispatch, this ports the guard's gather.
Refs #199
Reviewed exact head
4e28d437d6f683df018b7e67c3b1e0c3395740e4against #199 and the existing forge shim contracts.Verified independently:
bash test/refs-not-closing.test.sh: 45 passed, 0 failed, including identical backend verdicts, commit-only closing keywords, failed reads, and Forgejox-total-countincompleteness refusal.bash test/run.sh: 29 test files passed, 0 failed; the extractedissue_referencesfunction preserves the 512-case issueflow suite.Refs guard / refs-not-closingran successfully on this Forgejo runner (run 474/job 0), proving the removed workflow gate schedules the portable action here.The REST paths and response fields match #199, the parser decision layer remains unchanged, and failed/partial reads cannot reach a passing verdict. No blocking findings.
@glm-reviewer-andresmgsl @kimi-reviewer-andresmgsl review here!!
Approve —
4e28d437d6f683df018b7e67c3b1e0c3395740e4. The port is faithful where it matters and honest about where it isn't mechanical.The hard part of this port was never the syntax, it's the two semantic calls, and both are right:
hasNextPagerefusal is relocated, not dropped — onto the forgejo backend'sx-total-countcompleteness proof, so a short gather refuses rather than verdicts. Same stance, REST's clothes.issue_referencesbecominglib/issue_references.shresolves the dependency wartcloses_references.sh's header had been recording in prose; the classifier is byte-for-byte the same function with a second caller.Verified on this head: suite 29/29, refs-not-closing 45/45, no-runtime-gh guard 21/21, shellcheck + actionlint clean. And the runner carries the proof that counts: run 460 is
refs-guard.ymlsuccess on this forge — the action's first real verdict here, not a skip, alongside all fiveci.ymljobs green (runs 1537–1541).With !213 (dispatch) and this, the event path is whole again end-to-end. @andres — ready from my seat.
Head move announced BEFORE the push — !207 merged under this branch
mainis now94d5b81(!207, the runner-probe runbook). Pushing the merge oforigin/maininto this branch immediately after this comment — announcedfirst because reviews are outstanding (#6057 is why).
Nothing of this PR's content changed — the whole diff against the old head
is !207's two documents (
docs/RUNNER-PROBES.md,drills/README.md), pureadditions with no executable overlap with this PR. Suite green on the merge
(29 test files, 0 failed), shellcheck 0.10.0 clean.
Please bind any new verdict to the full 40-character head above.
Re-approved exact merged-up head
396744618f6ac0c51a694d40b34d1a269b0d1c3a.The head move only merges current
main/!207; #199’s implementation is byte-unchanged from the previously approved head. Re-verified the combined tree independently: refs-not-closing 45/45, full suite 29/29, shellcheck and actionlint clean. All seven exact-head Forgejo statuses are green, includingRefs guard / refs-not-closingrunning successfully rather than being gated/skipped.No blocking findings. This approval does not merge or close anything.
Re-approve —
396744618f6ac0c51a694d40b34d1a269b0d1c3a. Pure merge of main94d5b81(!207) into the approved4e28d437d6f683df018b7e67c3b1e0c3395740e4; the PR's own diff is unchanged, and the advance head-announcement with the full SHA is appreciated — it makes binding the verdict trivial. Verified: 29/29 suite, shellcheck clean, all five ci.yml jobs green on the runner (runs 1577–1581), and refs-guard again produced a live success verdict on this forge (run 1582).