From bada4ffff593625b36ff8bf173cc7b4f93649237 Mon Sep 17 00:00:00 2001 From: cluade-reviewer-andresmgsl Date: Wed, 5 Aug 2026 14:37:37 +0000 Subject: [PATCH] test(issueflow): each of the three sites is caught by behaviour, not only by the pin (#210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @codex-reviewer-andresmgsl's two scope items, applied before the first review round rather than after. 1. THE GUARD IS COMMENT-AWARE, WITH CONTROLS. It already stripped comments — it has to, because the #188 warning that explains why has("pull_request") is wrong contains the string. Without controls that was an untested property, and the pressure it creates is real: a raw grep would push a builder into deleting the very warning that prevents recurrence. Two fixtures now prove it: the explanatory comment is allowed, an executable jq filter is rejected. 2. ALL THREE SITES ARE DRIVEN BY BEHAVIOUR. The pin makes any revert red, but a pin proves a string is absent, not that each replacement means the intended thing: BOARD_RECORDS the forgejo-shaped board is not read as empty release_bodies an open `release` issue whose gate holds an open member makes a claimable NON-member draw a window flag — empty carriers, no flag, so the row discriminates the site instead of merely reaching it reconcile_issue_pass the scalar payload: key-present-null is an issue, object-valued is a PR, key-absent is still an issue The release_bodies row did NOT discriminate on its first write — it asserted an issue number that BOARD_RECORDS also produces, so reverting the site left it green. Caught by mutating each site separately rather than trusting the suite total. Mutation, per site: BOARD_RECORDS 3 red, release_bodies 2 red, reconcile_issue_pass 2 red. test/run.sh 28/28; issueflow 510/510; shellcheck 0.10.0 clean. Refs #210 --- changelog.d/210.md | 7 +++- test/issueflow-reconcile.test.sh | 58 ++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/changelog.d/210.md b/changelog.d/210.md index afd7927..ed46cf2 100644 --- a/changelog.d/210.md +++ b/changelog.d/210.md @@ -19,4 +19,9 @@ (#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). + 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). diff --git a/test/issueflow-reconcile.test.sh b/test/issueflow-reconcile.test.sh index 512658d..8dcca61 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -1815,6 +1815,47 @@ check "...the sweep completes over it" 0 "" \ check "...and the PR row is still excluded from the issue set" 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. +pass_disc() { # $1 = the payload -> the exit status the guard would take + jq -e '.pull_request == null' <<<"$1" >/dev/null && echo issue || echo pr +} +check "a forgejo issue payload (key present, null) reads as an issue" 0 "issue" \ + pass_disc '{"number":60,"pull_request":null}' +check "...and an object-valued one reads as a PR" 0 "pr" \ + pass_disc '{"number":61,"pull_request":{"merged":false}}' +check "...and a github-shaped payload (key absent) still reads as an issue" 0 "issue" \ + pass_disc '{"number":60}' + # -- 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 @@ -1830,6 +1871,23 @@ no_has_pull_request() { } 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,