diff --git a/actions/issueflow-reconcile/issueflow-reconcile.sh b/actions/issueflow-reconcile/issueflow-reconcile.sh index e0b3174..6a2ba11 100644 --- a/actions/issueflow-reconcile/issueflow-reconcile.sh +++ b/actions/issueflow-reconcile/issueflow-reconcile.sh @@ -157,9 +157,19 @@ post_merge_decision() { # $1 merged Refs PR, $2 linked open PR, $3 already handl fi } -post_merge_pr_for_issue() { # $1 issue; records are ISSUEPR - awk -F '\t' -v issue="$1" '$1 == issue { print $2 }' \ - <<<"${MERGED_REF_PR_RECORDS:-}" | sort -n | tail -n1 +post_merge_pr_for_issue() { # $1 issue; records are ISSUEPRMERGED_AT + # The deliverable is the PR that merged last, not the one numbered highest. + # Merge order is not number order in this family: crew#176's two Refs PRs + # merged #184 at 19:05:16Z and #182 at 19:05:18Z — the higher number two + # seconds earlier. Number order is also what spends a marker on the wrong + # PR: crew#321 carries `post-merge-transition-pr-326` while its real + # deliverable crew#322 — a lower number, merging later — is still open, so + # under the old rule the transition it owes could never fire (#242). + # mergedAt is ISO-8601 UTC, so it sorts as a string; ties break by highest + # PR number so the answer never depends on input order. + awk -F '\t' -v issue="$1" '$1 == issue { print $3 "\t" $2 }' \ + <<<"${MERGED_REF_PR_RECORDS:-}" \ + | sort -t $'\t' -k1,1 -k2,2n | tail -n1 | cut -f2 } post_merge_transition_marker() { # $1 merged PR number @@ -507,16 +517,24 @@ main() { query($owner: String!, $name: String!, $endCursor: String) { repository(owner: $owner, name: $name) { pullRequests(first: 100, states: MERGED, after: $endCursor) { - nodes { number body } + nodes { number mergedAt body } pageInfo { hasNextPage endCursor } } } }' --jq '.data.repository.pullRequests.nodes[] - | .number as $pr | .body | split("\n")[] - | [$pr, .] | @tsv' \ - | while IFS=$'\t' read -r pr body; do + | .number as $pr | .mergedAt as $merged | .body | split("\n")[] + | [$pr, $merged, .] | @tsv' \ + | 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\n' "$issue" "$pr" + [ -n "$issue" ] && printf '%s\t%s\t%s\n' "$issue" "$pr" "$merged" done < <(refs_references <<<"$body") done)" 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..c389e3f 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -106,6 +106,44 @@ 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 +477,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 +696,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"