From f4afaa1346739e443f01ea3d044af08f2d51d254 Mon Sep 17 00:00:00 2001 From: cndgrr <59120057+cndgrr@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:23:15 +0000 Subject: [PATCH 1/3] fix(issueflow): the deliverable PR is the last merged, not the highest numbered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit post_merge_pr_for_issue answered "which merged Refs PR is this issue's deliverable?" with sort -n | tail -n1. Merge order is not number order: crew#176's two Refs PRs merged #184 at 19:05:16Z and #182 at 19:05:18Z. MERGED_REF_PR_RECORDS gains mergedAt as a third column — a field on the merged-PR node set already fetched, so no additional GraphQL request — and the selection sorts on it, breaking ties by highest PR number so the answer never depends on input order. Refs #242 --- .../issueflow-reconcile.sh | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/actions/issueflow-reconcile/issueflow-reconcile.sh b/actions/issueflow-reconcile/issueflow-reconcile.sh index e0b3174..b4d8e3c 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,16 @@ 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=$'\t' read -r pr merged body; do 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)" 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 2/3] 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" From 544d4a060307dce4132be699ad61683a69943978 Mon Sep 17 00:00:00 2001 From: cndgrr <59120057+cndgrr@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:29:21 +0000 Subject: [PATCH 3/3] style(test): separate the merge-order block from the offsite decisions Refs #242 --- test/issueflow-reconcile.test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/test/issueflow-reconcile.test.sh b/test/issueflow-reconcile.test.sh index 4e7e95d..c389e3f 100644 --- a/test/issueflow-reconcile.test.sh +++ b/test/issueflow-reconcile.test.sh @@ -143,6 +143,7 @@ check "the sweep still issues exactly two GraphQL queries" 0 "2" \ check "...with mergedAt selected on the merged-PR node it already fetched" 0 "" \ grep -qF 'nodes { number mergedAt body }' \ "$ROOT/actions/issueflow-reconcile/issueflow-reconcile.sh" + check "one closed offsite PR nudges" 0 "NUDGE" offsite_resolved_decision <<<"CLOSED" check "two closed offsite PRs nudge" 0 "NUDGE" offsite_resolved_decision <<< $'CLOSED\nCLOSED' check "one open offsite PR keeps quiet" 0 "QUIET" offsite_resolved_decision <<< $'CLOSED\nOPEN'