From 9c690f02b7975b52b4f3148e2408eeaa82ddf123 Mon Sep 17 00:00:00 2001 From: cndgrr <59120057+cndgrr@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:27:43 +0000 Subject: [PATCH] test(issueflow): drive the merge-order selection, and the spent-marker shape end to end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit issue_probe's merged-PR argument becomes a spec list — `PR` or `PR@` — so a probe can state merge order; the bare form keeps every existing call site literal. The direct-drive cases cover crew#176's shape (the lower number merged later), agreeing orders, interleaved issues, the mergedAt tie broken by highest PR number under both input orders, and the empty answer. The end-to-end probe is crew#321's: a marker already standing for the later-merged, lower-numbered PR must suppress the transition, which selecting by number could never do. Two static pins keep the request count honest — the sweep issues exactly two GraphQL queries, with mergedAt selected on the merged-PR node it already fetched. Refs #242 --- .../issueflow-reconcile.sh | 10 ++- changelog.d/242.md | 5 ++ test/issueflow-reconcile.test.sh | 81 ++++++++++++++++--- 3 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 changelog.d/242.md diff --git a/actions/issueflow-reconcile/issueflow-reconcile.sh b/actions/issueflow-reconcile/issueflow-reconcile.sh index b4d8e3c..6a2ba11 100644 --- a/actions/issueflow-reconcile/issueflow-reconcile.sh +++ b/actions/issueflow-reconcile/issueflow-reconcile.sh @@ -524,7 +524,15 @@ main() { }' --jq '.data.repository.pullRequests.nodes[] | .number as $pr | .mergedAt as $merged | .body | split("\n")[] | [$pr, $merged, .] | @tsv' \ - | while IFS=$'\t' read -r pr merged body; do + | while IFS= read -r record; do + # Split on exact tabs rather than IFS: tab is IFS whitespace, so bash + # collapses a run of them, and a middle column that ever came back + # empty would silently shift the body one field left. The body is + # arbitrary text and stays last, where the remainder belongs. + pr="${record%%$'\t'*}" + rest="${record#*$'\t'}" + merged="${rest%%$'\t'*}" + body="${rest#*$'\t'}" while IFS= read -r issue; do [ -n "$issue" ] && printf '%s\t%s\t%s\n' "$issue" "$pr" "$merged" done < <(refs_references <<<"$body") diff --git a/changelog.d/242.md b/changelog.d/242.md new file mode 100644 index 0000000..73e8d48 --- /dev/null +++ b/changelog.d/242.md @@ -0,0 +1,5 @@ +### Fixed + +- The issue-flow sweep now reads an issue's deliverable as the `Refs` PR that + merged last, not the one numbered highest — merge order is not number order, + and the old rule spent the transition marker on the wrong PR (#242). diff --git a/test/issueflow-reconcile.test.sh b/test/issueflow-reconcile.test.sh index 7f7c10b..4e7e95d 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -106,6 +106,43 @@ check "a handled merged Refs episode does not transition again" 0 "KEEP" \ post_merge_decision 12 false true <<<"- [ ] verify after merge" check "merged Refs with all criteria checked does not transition" 0 "KEEP" \ post_merge_decision 12 false false PRMERGED_AT (#242). A spec is `PR` or + # `PR@`; the bare form takes a fixed hour-old merge, which is every + # probe that does not care about merge order. An empty list is no record + # at all, so the no-merged-PR probes read exactly as they did. + MERGED_REF_PR_RECORDS="$( + # shellcheck disable=SC2086 # the spec list is deliberately word-split + for spec in $merged_ref_prs; do + pr="${spec%%@*}" + merged_at="${spec#*@}" + [ "$merged_at" != "$spec" ] || merged_at="$(iso_at $((INOW - 3600)))" + printf '%s\t%s\t%s\n' "$1" "$pr" "$merged_at" + done)" run() { "$@"; } gh() { issue_stub_gh "$@"; } reconcile_issue "$1" 2>&1 @@ -432,6 +476,25 @@ check "a later merged Refs PR gets an episode-specific transition comment" 0 "" check "...and the later episode still transitions" 0 "" \ grep -qF 'merged Refs PR -> post-merge; claim released' <<<"$second_transition" +# End to end on the crew#321 shape: the later merge is the *lower*-numbered +# PR, and its marker is already on the issue. Selecting by number would find +# no marker for #461, fire the transition a second time, and release a claim +# the board already released (#242). +recent_timeline 46 +jq -n --arg b '' \ + --arg at "$(iso_at $((INOW - 60)))" \ + '[{"body":$b,"created_at":$at}]' >"$(cfix 46)" +spent_edit_count="$(wc -l <"$TMP/issue-edits")" +spent="$(issue_probe 46 claimed 1 false \ + "461@$(iso_at $((INOW - 7200))) 460@$(iso_at $((INOW - 3600)))" \ + '- [ ] verify after merge')" +check "the marker of the later-merged lower-numbered PR is the one read" 0 "" \ + test -z "$spent" +# shellcheck disable=SC2016 # positional parameters belong to bash -c +check "...so the spent transition is not fired a second time" 0 "" \ + bash -c 'test "$1" -eq "$(wc -l <"$2")" && test ! -f "$3"' _ \ + "$spent_edit_count" "$TMP/issue-edits" "$TMP/posted-46" + printf '[]\n' >"$(cfix 45)" issue_probe 45 $'claimed\npost-merge' >/dev/null # shellcheck disable=SC2016 # Markdown backticks are literal evidence @@ -632,7 +695,7 @@ check "...and the sweep still runs" 0 "" \ # Keep this at main() granularity: the GraphQL gather and loop are the code # a sourced decision probe cannot exercise (#91's lesson). printf '%s\n' \ - '{"data":{"repository":{"pullRequests":{"nodes":[{"number":400,"body":"Refs #40","closingIssuesReferences":{"nodes":[]}}],"pageInfo":{"hasNextPage":false,"endCursor":null}}}}}' \ + '{"data":{"repository":{"pullRequests":{"nodes":[{"number":400,"mergedAt":"2026-07-30T19:05:16Z","body":"Refs #40","closingIssuesReferences":{"nodes":[]}}],"pageInfo":{"hasNextPage":false,"endCursor":null}}}}}' \ >"$ARRIVAL/fixtures/graphql.json" printf '[{"number":40}]\n' \ >"$ARRIVAL/fixtures/repos_owner_repo_issues_state_open_per_page_100.json"