forked from heavy-duty/ceremony
Merge pull request 'issueflow-reconcile — the board discriminator is .pull_request == null, not has(), or this forge has no issues (#210)' (#211) from build/210-discriminator into main
Reviewed-on: heavy-duty/ceremony#211 Reviewed-by: codex-reviewer-andresmgsl <andres+2@heavyduty.builders> Reviewed-by: kimi-reviewer-andresmgsl <andres+4@heavyduty.builders>
This commit is contained in:
commit
03143ff0ed
3 changed files with 179 additions and 3 deletions
|
|
@ -1159,7 +1159,10 @@ reconcile_issue_pass() { # $1 = issue — one issue's whole pass, in its own sub
|
|||
|| skip_issue "$n" "could not read the issue: $(read_failure_reason "$READ_FAILURE_STDERR")"
|
||||
issue_payload_valid "$n" <<<"$ISSUE_JSON" \
|
||||
|| skip_issue "$n" "the issue read answered a payload that is not issue #$n carrying a label array"
|
||||
jq -e 'has("pull_request") | not' <<<"$ISSUE_JSON" >/dev/null || exit 0
|
||||
# `.pull_request == null`, never `has("pull_request") | not` (#188, #210):
|
||||
# every Forgejo entry CARRIES the key, valued null on an issue, so the
|
||||
# has() form selects zero rows here — silently, forever.
|
||||
jq -e '.pull_request == null' <<<"$ISSUE_JSON" >/dev/null || exit 0
|
||||
ISSUE_LABELS="$(jq -r '.labels[].name' <<<"$ISSUE_JSON")"
|
||||
reconcile_issue "$n" || exit $?
|
||||
commit_staged_effects
|
||||
|
|
@ -1273,7 +1276,8 @@ main() {
|
|||
log "could not read the issue board: $(read_failure_reason "$READ_FAILURE_STDERR")"
|
||||
return 1
|
||||
fi
|
||||
BOARD_RECORDS="$(jq -r '.[] | select(has("pull_request") | not)
|
||||
# `.pull_request == null`, never `has("pull_request") | not` (#188, #210).
|
||||
BOARD_RECORDS="$(jq -r '.[] | select(.pull_request == null)
|
||||
| [(.number | tostring), ((.labels // []) | map(.name) | join(",")), (.title // "")]
|
||||
| @tsv' \
|
||||
<<<"$board_json")"
|
||||
|
|
@ -1284,7 +1288,7 @@ main() {
|
|||
# is exactly the emptied gate the release's own `blocked` -> `ready`
|
||||
# promotion answers, which is why a `ready` release leaves the flag
|
||||
# dormant rather than flagging the whole board.
|
||||
release_bodies="$(jq -r '.[] | select(has("pull_request") | not)
|
||||
release_bodies="$(jq -r '.[] | select(.pull_request == null)
|
||||
| select((.labels // []) | map(.name) | index("release"))
|
||||
| [(.number | tostring), ((.body // "") | gsub("[\t\r\n]"; " "))] | @tsv' \
|
||||
<<<"$board_json")"
|
||||
|
|
|
|||
27
changelog.d/210.md
Normal file
27
changelog.d/210.md
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
### Fixed
|
||||
|
||||
- `issueflow-reconcile` sees this forge's issues again. The board gather used
|
||||
`has("pull_request")`, and every Forgejo entry carries that key — so it
|
||||
selected zero rows on every sweep while printing `reconciled.` (#210).
|
||||
|
||||
- Three sites take `.pull_request == null`, the discriminator the file's own
|
||||
comment already specified and that one of its four call sites already used
|
||||
(#210).
|
||||
|
||||
- `post-merge` transitions can fire again: they could not, because the sweep
|
||||
saw no issues to transition (#210).
|
||||
|
||||
### Added
|
||||
|
||||
- A gather-level case drives the real board read against a Forgejo-shaped
|
||||
fixture — every entry carrying the key. The existing discriminator cases
|
||||
assert `jq` expressions in isolation and passed throughout this regression
|
||||
(#210).
|
||||
|
||||
- A source pin forbids `has("pull_request")` on this surface, because the rule
|
||||
was stated in a comment and violated forty lines below it. It strips comments,
|
||||
so the #188 warning that explains the trap is allowed to stay (#210).
|
||||
|
||||
- All three sites are covered behaviourally, not only by the pin: the board
|
||||
gather, the release-body gather through an observable window flag, and the
|
||||
per-issue payload check (#210).
|
||||
|
|
@ -1779,6 +1779,151 @@ check "a PR reads as a PR on either shape" 0 "pr" \
|
|||
check "the old has() test misreads a forgejo issue as a PR" 0 "pr" \
|
||||
bash -c 'echo "{\"number\":1,\"pull_request\":null}" | jq -e "has(\"pull_request\") | not" >/dev/null && echo issue || echo pr'
|
||||
|
||||
# ...AND THE GATHER, because the rows above assert jq expressions in isolation
|
||||
# and passed all the way through #210 — a sweep that saw zero issues on every
|
||||
# pass and printed `reconciled.` The 0.6.0 merge reintroduced the has() form in
|
||||
# the board gather; these cases could not see it because they never ran it.
|
||||
#
|
||||
# The fixture is FORGEJO-SHAPED: every entry carries `pull_request`, valued
|
||||
# null on an issue and an object on a PR. On a GitHub-shaped board (key absent
|
||||
# on issues) both discriminators agree, which is why this needs its own board.
|
||||
FORGEJO_BOARD="$TMP/forgejo-board"
|
||||
mkdir -p "$FORGEJO_BOARD"
|
||||
cp "$ARRIVAL/labels.conf" "$FORGEJO_BOARD/labels.conf" 2>/dev/null || true
|
||||
printf '[]\n' >"$FORGEJO_BOARD/repos_owner_repo_pulls_state_open.json"
|
||||
printf '[]\n' >"$FORGEJO_BOARD/repos_owner_repo_pulls_state_closed.json"
|
||||
printf '%s\n' \
|
||||
'[{"number":60,"pull_request":null,"labels":[],"title":"an issue with no queue state"},
|
||||
{"number":61,"pull_request":{"merged":false},"labels":[],"title":"a pull request"}]' \
|
||||
>"$FORGEJO_BOARD/repos_owner_repo_issues_state_open.json"
|
||||
jq -n --arg at "$(iso_at "$INOW")" \
|
||||
'{number:60,user:{login:"triage-one"},created_at:$at,body:"",pull_request:null,
|
||||
labels:[],assignees:[]}' \
|
||||
>"$FORGEJO_BOARD/repos_owner_repo_issues_60.json"
|
||||
printf '[]\n' >"$FORGEJO_BOARD/repos_owner_repo_issues_60_comments.json"
|
||||
forgejo_board_run() {
|
||||
: >"$FORGEJO_BOARD/edits"
|
||||
env PATH="$ARRIVAL/stub:$PATH" CEREMONY_FORGE=github GH_FIXTURES="$FORGEJO_BOARD" \
|
||||
ISSUEFLOW_NOW="$INOW" REPO=owner/repo LABELS_CONF="$ARRIVAL/labels.conf" \
|
||||
bash "$ROOT/actions/issueflow-reconcile/issueflow-reconcile.sh" 2>&1
|
||||
}
|
||||
fjb_out="$(forgejo_board_run)"
|
||||
check "a forgejo-shaped board is NOT read as empty" 1 "" \
|
||||
grep -qF 'issueflow: no open issues.' <<<"$fjb_out"
|
||||
check "...the sweep completes over it" 0 "" \
|
||||
grep -qF 'issueflow: reconciled.' <<<"$fjb_out"
|
||||
# TRAVERSAL, not merely a non-empty gather: the null-valued row has a
|
||||
# deterministic outcome — no queue state means needs-triage is minted — so this
|
||||
# proves reconcile_issue_pass actually ran over it, which "the board is not
|
||||
# empty" does not (@codex-reviewer-andresmgsl, #210 review).
|
||||
check "...the null-valued row is TRAVERSED, with an observable outcome" 0 "" \
|
||||
grep -qE '^issueflow: #60: needs-triage' <<<"$fjb_out"
|
||||
check "...and the object-valued PR row is not reconciled as an issue" 1 "" \
|
||||
grep -qE '^issueflow: #61' <<<"$fjb_out"
|
||||
|
||||
# release_bodies is the THIRD producer and has its own has() site. A `release`
|
||||
# issue on a forgejo-shaped board must reach the window gather, or the #292
|
||||
# flags are decided over an empty set (@codex-reviewer-andresmgsl, #210).
|
||||
printf '%s\n' \
|
||||
'[{"number":60,"pull_request":null,"labels":[{"name":"ready"}],"title":"an issue"},
|
||||
{"number":62,"pull_request":null,"labels":[{"name":"release"}],"title":"Release 9.9.9","body":"Blocked by #60."},
|
||||
{"number":63,"pull_request":null,"labels":[{"name":"ready"}],"title":"a claimable non-member"},
|
||||
{"number":61,"pull_request":{"merged":false},"labels":[],"title":"a pull request"}]' \
|
||||
>"$FORGEJO_BOARD/repos_owner_repo_issues_state_open.json"
|
||||
jq -n --arg at "$(iso_at "$INOW")" \
|
||||
'{number:63,user:{login:"triage-one"},created_at:$at,body:"",pull_request:null,
|
||||
labels:[{name:"ready"}],assignees:[]}' \
|
||||
>"$FORGEJO_BOARD/repos_owner_repo_issues_63.json"
|
||||
printf '[]\n' >"$FORGEJO_BOARD/repos_owner_repo_issues_63_comments.json"
|
||||
jq -n --arg at "$(iso_at "$INOW")" \
|
||||
'{number:62,user:{login:"triage-one"},created_at:$at,body:"Blocked by #60.",pull_request:null,
|
||||
labels:[{name:"release"}],assignees:[]}' \
|
||||
>"$FORGEJO_BOARD/repos_owner_repo_issues_62.json"
|
||||
printf '[]\n' >"$FORGEJO_BOARD/repos_owner_repo_issues_62_comments.json"
|
||||
fjb2_out="$(forgejo_board_run)"
|
||||
# The observable effect of release_bodies being NON-empty: an open `release`
|
||||
# issue whose gate still holds an open member makes every claimable non-member
|
||||
# draw a window flag. With that gather empty there are no carriers and no flag,
|
||||
# so this row discriminates the site rather than merely reaching it.
|
||||
check "a release issue on a forgejo-shaped board reaches the window gather" 0 "" \
|
||||
grep -qE '#63: window flag' <<<"$fjb2_out"
|
||||
check "...and the sweep still completes" 0 "" \
|
||||
grep -qF 'issueflow: reconciled.' <<<"$fjb2_out"
|
||||
|
||||
# The THIRD site is reconcile_issue_pass's per-issue payload check. Same key,
|
||||
# scalar rather than a list: null means issue, an object means PR.
|
||||
# THE SCALAR SITE, ISOLATED. `BOARD_RECORDS` filters an object-valued row out
|
||||
# of the LIST before the per-issue guard ever sees it, so a board fixture alone
|
||||
# cannot prove the scalar stand-down (@codex-reviewer-andresmgsl, #210 review).
|
||||
#
|
||||
# The fixture that isolates it: the LIST row is null-valued, so the board
|
||||
# gather admits it — and the INDIVIDUAL payload the sweep then fetches is
|
||||
# object-valued. Only reconcile_issue_pass's own guard can stand that down.
|
||||
printf '%s\n' \
|
||||
'[{"number":65,"pull_request":null,"labels":[],"title":"list says issue, payload says PR"}]' \
|
||||
>"$FORGEJO_BOARD/repos_owner_repo_issues_state_open.json"
|
||||
jq -n --arg at "$(iso_at "$INOW")" \
|
||||
'{number:65,user:{login:"triage-one"},created_at:$at,body:"",
|
||||
pull_request:{merged:false},labels:[],assignees:[]}' \
|
||||
>"$FORGEJO_BOARD/repos_owner_repo_issues_65.json"
|
||||
printf '[]\n' >"$FORGEJO_BOARD/repos_owner_repo_issues_65_comments.json"
|
||||
fjb3_out="$(forgejo_board_run)"
|
||||
check "the per-issue guard stands down an object-valued payload" 1 "" \
|
||||
grep -qE '^issueflow: #65: needs-triage' <<<"$fjb3_out"
|
||||
check "...and the sweep still completes" 0 "" \
|
||||
grep -qF 'issueflow: reconciled.' <<<"$fjb3_out"
|
||||
|
||||
# The same fixture with a null-valued payload MUST reconcile — otherwise the row
|
||||
# above would pass on a guard that stands everything down.
|
||||
jq -n --arg at "$(iso_at "$INOW")" \
|
||||
'{number:65,user:{login:"triage-one"},created_at:$at,body:"",
|
||||
pull_request:null,labels:[],assignees:[]}' \
|
||||
>"$FORGEJO_BOARD/repos_owner_repo_issues_65.json"
|
||||
fjb4_out="$(forgejo_board_run)"
|
||||
check "...while a null-valued payload at the same site reconciles" 0 "" \
|
||||
grep -qE '^issueflow: #65: needs-triage' <<<"$fjb4_out"
|
||||
|
||||
# And the GitHub shape — key absent entirely — is still an issue.
|
||||
jq -n --arg at "$(iso_at "$INOW")" \
|
||||
'{number:65,user:{login:"triage-one"},created_at:$at,body:"",labels:[],assignees:[]}' \
|
||||
>"$FORGEJO_BOARD/repos_owner_repo_issues_65.json"
|
||||
fjb5_out="$(forgejo_board_run)"
|
||||
check "...and a github-shaped payload (key absent) reconciles too" 0 "" \
|
||||
grep -qE '^issueflow: #65: needs-triage' <<<"$fjb5_out"
|
||||
|
||||
# -- the rule is pinned at the source, because a comment did not hold --------
|
||||
# `.pull_request == null` is stated in this file's own header AND at
|
||||
# issueflow-reconcile.sh:1113 — and the merge put `has("pull_request")` back 40
|
||||
# lines below that comment, in three places. Prose is not a guard (#210).
|
||||
# In-process, not `bash -c`: a subshell cannot see this file's functions, and a
|
||||
# pin that silently inspected nothing would be the same defect one level up.
|
||||
no_has_pull_request() {
|
||||
local hits
|
||||
hits="$(sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' \
|
||||
"$ROOT/actions/issueflow-reconcile/issueflow-reconcile.sh" \
|
||||
| grep -n 'has("pull_request")')"
|
||||
[ -z "$hits" ] || { printf 'executable has("pull_request") at:\n%s\n' "$hits" >&2; return 1; }
|
||||
}
|
||||
check "no executable has(\"pull_request\") survives on this surface" 0 "" \
|
||||
no_has_pull_request
|
||||
# The controls, because this guard MUST tolerate the #188 comment that explains
|
||||
# why the form is wrong — a raw grep would either fail forever or pressure a
|
||||
# builder into deleting the very warning that prevents recurrence
|
||||
# (@codex-reviewer-andresmgsl, #210 review).
|
||||
DTMP="$(mktemp -d)"; trap 'rm -rf "$DTMP"' EXIT
|
||||
strip_and_find() { # $1 = file -> 0 when an EXECUTABLE use survives
|
||||
sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$1" | grep -q 'has("pull_request")'
|
||||
}
|
||||
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
||||
printf '%s\n' '#!/usr/bin/env bash' \
|
||||
'# `.pull_request == null`, NOT has("pull_request") | not (#188)' \
|
||||
'jq -e ".pull_request == null" <<<"$J"' >"$DTMP/prose.sh"
|
||||
check "the explanatory #188 comment is allowed" 1 "" strip_and_find "$DTMP/prose.sh"
|
||||
# single-quoted so the fixture holds the LITERAL form the guard looks for
|
||||
printf '%s\n' '#!/usr/bin/env bash' \
|
||||
"jq -r '.[] | select(has(\"pull_request\") | not)' <<<\"\$J\"" >"$DTMP/exec.sh"
|
||||
check "...while an executable jq filter is rejected" 0 "" strip_and_find "$DTMP/exec.sh"
|
||||
|
||||
# -- the OPEN-pull gather, at main() granularity ----------------------------
|
||||
# The closed/merged half above proves one REST path; this proves the other,
|
||||
# which is a DIFFERENT pipeline: `.body | @base64` -> base64 -d ->
|
||||
|
|
|
|||
Loading…
Reference in a new issue