fix(issueflow): the board discriminator is .pull_request == null, not has() (#210)
All checks were successful
CI / test (pull_request) Successful in 3m8s
CI / release-exercise (pull_request) Successful in 11s
CI / self-guards (pull_request) Successful in 7s
CI / action-exercise (pull_request) Successful in 6s
CI / docs-sync-exercise (pull_request) Successful in 6s
Refs guard / refs-not-closing (pull_request) Has been skipped
labels / labels (pull_request) Successful in 8s
All checks were successful
CI / test (pull_request) Successful in 3m8s
CI / release-exercise (pull_request) Successful in 11s
CI / self-guards (pull_request) Successful in 7s
CI / action-exercise (pull_request) Successful in 6s
CI / docs-sync-exercise (pull_request) Successful in 6s
Refs guard / refs-not-closing (pull_request) Has been skipped
labels / labels (pull_request) Successful in 8s
issueflow-reconcile has been blind on this forge since the 0.6.0 merge landed. Run 368 — #198's own post-merge acceptance probe — printed: issueflow: no open issues. issueflow: reconciled. over a board of nine. Every Forgejo entry CARRIES the `pull_request` key, valued null on an issue, so `select(has("pull_request") | not)` selects zero rows. Measured again today: #209 (an issue) has the key valued null; #208 and #207 (PRs) have it valued as objects. This is mine. #188 fixed exactly this and the file's own comment at :1113 states the rule, with :1121 already using it correctly. Resolving hunk 4 of the merge I took upstream's board block wholesale and carried the wrong discriminator into three sites — the gather, the release-body gather, and reconcile_issue_pass — in the PR whose stated purpose was to stop blind sweeps reporting success. Cost while it stood: no issue transitions, no claim reclaims, no nudges, no board flags — and no `post-merge` transitions, which is why #192 and #198 both still read `claimed` after their PRs merged, and why #198's own closure criterion could not complete. Two guards, because a comment did not hold: * A GATHER-LEVEL CASE against a Forgejo-shaped fixture — every entry carrying the key. The existing discriminator cases assert jq expressions in isolation and passed throughout this regression; they never ran the gather that uses them, which is precisely how it survived review. * A SOURCE PIN forbidding has("pull_request") on this surface, so a future sync cannot reintroduce it 40 lines below the comment forbidding it. Reverting the board gather reds both. Reverting reconcile_issue_pass reds the pin. test/run.sh 28/28 under jq 1.7 and jq 1.6; issueflow 503/503; shellcheck 0.10.0 and actionlint clean. Refs #210
This commit is contained in:
parent
e236318647
commit
877e09e015
3 changed files with 81 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")"
|
|| skip_issue "$n" "could not read the issue: $(read_failure_reason "$READ_FAILURE_STDERR")"
|
||||||
issue_payload_valid "$n" <<<"$ISSUE_JSON" \
|
issue_payload_valid "$n" <<<"$ISSUE_JSON" \
|
||||||
|| skip_issue "$n" "the issue read answered a payload that is not issue #$n carrying a label array"
|
|| 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")"
|
ISSUE_LABELS="$(jq -r '.labels[].name' <<<"$ISSUE_JSON")"
|
||||||
reconcile_issue "$n" || exit $?
|
reconcile_issue "$n" || exit $?
|
||||||
commit_staged_effects
|
commit_staged_effects
|
||||||
|
|
@ -1273,7 +1276,8 @@ main() {
|
||||||
log "could not read the issue board: $(read_failure_reason "$READ_FAILURE_STDERR")"
|
log "could not read the issue board: $(read_failure_reason "$READ_FAILURE_STDERR")"
|
||||||
return 1
|
return 1
|
||||||
fi
|
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 // "")]
|
| [(.number | tostring), ((.labels // []) | map(.name) | join(",")), (.title // "")]
|
||||||
| @tsv' \
|
| @tsv' \
|
||||||
<<<"$board_json")"
|
<<<"$board_json")"
|
||||||
|
|
@ -1284,7 +1288,7 @@ main() {
|
||||||
# is exactly the emptied gate the release's own `blocked` -> `ready`
|
# is exactly the emptied gate the release's own `blocked` -> `ready`
|
||||||
# promotion answers, which is why a `ready` release leaves the flag
|
# promotion answers, which is why a `ready` release leaves the flag
|
||||||
# dormant rather than flagging the whole board.
|
# 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"))
|
| select((.labels // []) | map(.name) | index("release"))
|
||||||
| [(.number | tostring), ((.body // "") | gsub("[\t\r\n]"; " "))] | @tsv' \
|
| [(.number | tostring), ((.body // "") | gsub("[\t\r\n]"; " "))] | @tsv' \
|
||||||
<<<"$board_json")"
|
<<<"$board_json")"
|
||||||
|
|
|
||||||
22
changelog.d/210.md
Normal file
22
changelog.d/210.md
Normal file
|
|
@ -0,0 +1,22 @@
|
||||||
|
### 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 (#210).
|
||||||
|
|
@ -1779,6 +1779,58 @@ 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" \
|
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'
|
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":[{"name":"ready"}],"title":"an issue"},
|
||||||
|
{"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:[{name:"ready"}],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"
|
||||||
|
check "...and the PR row is still excluded from the issue set" 1 "" \
|
||||||
|
grep -qE '^issueflow: #61' <<<"$fjb_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 OPEN-pull gather, at main() granularity ----------------------------
|
# -- the OPEN-pull gather, at main() granularity ----------------------------
|
||||||
# The closed/merged half above proves one REST path; this proves the other,
|
# The closed/merged half above proves one REST path; this proves the other,
|
||||||
# which is a DIFFERENT pipeline: `.body | @base64` -> base64 -d ->
|
# which is a DIFFERENT pipeline: `.body | @base64` -> base64 -d ->
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue