fix: switch to GraphQL for issue assignment and PR checks #1746
fix: switch to GraphQL for issue assignment and PR checks #1746MonaaEid wants to merge 5 commits intohiero-ledger:mainfrom
Conversation
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1746 +/- ##
=======================================
Coverage 93.25% 93.25%
=======================================
Files 141 141
Lines 9061 9061
=======================================
Hits 8450 8450
Misses 611 611 🚀 New features to boost your workflow:
|
WalkthroughReplaced REST-based timeline/event lookups in the issue reminder script with GitHub GraphQL queries to: (1) fetch the most recent ASSIGNED_EVENT createdAt timestamp and (2) detect linked open PRs via closedByPullRequestsReferences. Added GraphQL error/null handling and preserved reminder posting and threshold logic. Changes
Sequence Diagram(s)sequenceDiagram
participant Bot as Issue Reminder Script
participant GH as GitHub GraphQL API
participant Issue as Issue (timeline & refs)
Bot->>GH: Query timelineItems for latest ASSIGNED_EVENT (issue number)
GH-->>Bot: ASSIGNED_EVENT { createdAt } or null / error
alt createdAt present
Bot->>Bot: calculate days since assignment
Bot->>GH: Query closedByPullRequestsReferences for linked PRs
GH-->>Bot: list of PR refs (with statuses)
alt open PR found
Bot->>Bot: skip — linked open PR exists
else no open PR
Bot->>Bot: check threshold -> post reminder if needed
end
else missing createdAt or error
Bot-->>Bot: log warning for issue and skip
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/bot-issue-reminder-no-pr.sh (1)
167-183:⚠️ Potential issue | 🔴 CriticalSame error-capture issue: GraphQL failure makes the bot silently skip reminders.
When
gh api graphqlfails, the error message is captured intoOPEN_PR_FOUND. Since it is non-empty, line 179 treats it as a linked PR and skips the reminder — the exact false-negative bug this PR is fixing (#1731).Apply the same
if !pattern here:Proposed fix
- OPEN_PR_FOUND=$(gh api graphql -f query=" + if ! OPEN_PR_FOUND=$(gh api graphql -f query=" query { repository(owner: \"${REPO%/*}\", name: \"${REPO#*/}\") { issue(number: $ISSUE) { closedByPullRequestsReferences(first: 100, includeClosedPrs: false) { nodes { number state } } } } } - " --jq '[.data.repository.issue.closedByPullRequestsReferences.nodes[] | select(.state == "OPEN") | .number] | first // empty' 2>&1) || true + " --jq '[.data.repository.issue.closedByPullRequestsReferences.nodes[] | select(.state == "OPEN") | .number] | first // empty' 2>&1); then + echo "[ERROR] Failed to fetch linked PRs for issue #$ISSUE: $OPEN_PR_FOUND. Skipping." + continue + fi if [ -n "$OPEN_PR_FOUND" ]; then
Signed-off-by: MontyPokemon <59332150+MonaaEid@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/bot-issue-reminder-no-pr.sh (1)
170-186:⚠️ Potential issue | 🔴 CriticalCritical: GraphQL error messages leak into
OPEN_PR_FOUND, causing silent false negatives.When
gh api graphqlfails (network error, auth failure, rate limit),2>&1captures the error text intoOPEN_PR_FOUNDand|| truesuppresses the exit code. Since the error string is non-empty, line 182 treats it as a linked PR and skips the reminder — exactly the false-negative bug this PR is meant to fix.The assignment-timestamp query on lines 135–156 already uses the correct
if !pattern. Apply the same pattern here for consistency and correctness.Proposed fix
- OPEN_PR_FOUND=$(gh api graphql -f query=" + if ! OPEN_PR_FOUND=$(gh api graphql -f query=" query { repository(owner: \"${REPO%/*}\", name: \"${REPO#*/}\") { issue(number: $ISSUE) { closedByPullRequestsReferences(first: 100, includeClosedPrs: false) { nodes { number state } } } } } - " --jq '[.data.repository.issue.closedByPullRequestsReferences.nodes[] | select(.state == "OPEN") | .number] | first // empty' 2>&1) || true + " --jq '[.data.repository.issue.closedByPullRequestsReferences.nodes[] | select(.state == "OPEN") | .number] | first // empty' 2>&1); then + echo "[WARN] GraphQL query failed for PR check on issue #$ISSUE: $OPEN_PR_FOUND. Skipping." + continue + fiAs per coding guidelines, "MUST log key decisions and early exits".
exploreriii
left a comment
There was a problem hiding this comment.
Would you mind creating a PAT, setting it in your env, and using that to run a query on what the real counts are for the python sdk locally, using a similar count logic to what you are using here?
Seems to be working well on your fork, but it is hard to simulate the data we have
Description:
This pull request refactors the
.github/scripts/bot-issue-reminder-no-pr.shscript to use the GitHub GraphQL API instead of the REST API for retrieving assignment times and checking for open pull requests linked to issues. This change improves efficiency and reliability when processing issue assignment and PR linkage.API usage improvements:
Fixes #1731
Checklist
.github/scripts/bot-issue-reminder-no-pr.sh