fix(seidroid-review): recheck the head before withdrawing, and find a batch review that landed without an answer #116
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2619,6 +2619,7 @@ jobs: | |
| review_body="Findings on the changed lines. The verdict and the summary are in this tool's comment on this pull request." | ||
| echo "::warning::the driver left no verdict for the review on $REPO#$PR to carry, so it names where one will be and the verdict is posted on its own" | ||
| fi | ||
| review_body+=$'\n\n'"<!-- seidroid-run:${GITHUB_RUN_ID:-}-${GITHUB_RUN_ATTEMPT:-} -->" | ||
| review_event=COMMENT | ||
| if [ "$carries_verdict" = true ] && [ -n "${EVENT:-}" ]; then | ||
| review_event="$EVENT" | ||
|
|
@@ -2666,6 +2667,23 @@ jobs: | |
| landed=true | ||
| fi | ||
| fi | ||
| found_id="" | ||
| if [ "$landed" = false ]; then | ||
| case "$(sed -n '1s|^HTTP/[0-9.]* \([0-9][0-9][0-9]\).*|\1|p' "$response" || true)" in | ||
| 4??) ;; | ||
| *) | ||
| found_id="$(gh api --paginate "repos/$REPO/pulls/$PR/reviews" 2>/dev/null \ | ||
| | jq -rs --arg body "$review_body" --arg sha "$head_sha" \ | ||
| '[.[][] | select(.body == $body and .commit_id == $sha)] | last | .id // empty' \ | ||
| 2>/dev/null || true)" | ||
|
There was a problem hiding this comment. Reviews lookup omits page sizeMedium Severity The 5xx recovery lists PR reviews without   Triggered by learned rule: seidroid-review: VERDICT_MARKER via $ENV; GET page size in path Reviewed by Cursor Bugbot for commit 384281e. Configure here. |
||
| case "$found_id" in (''|*[!0-9]*) found_id="" ;; esac | ||
| if [ -n "$found_id" ]; then | ||
| echo "::warning::the review carrying $anchored comment(s) on $REPO#$PR returned no answer but landed as review $found_id" | ||
| review_event="$(jq -r '.event' "$request")" | ||
| landed=true | ||
| fi ;; | ||
| esac | ||
| fi | ||
| if [ "$landed" = true ]; then | ||
| on_line=$((on_line + anchored)) | ||
| # The call carries every anchored comment and creates all of them or | ||
|
|
@@ -2689,7 +2707,7 @@ jobs: | |
| # JSON begins after the first empty line. A value that is not a number | ||
| # is no id, and reaching the API with one would edit nothing and report | ||
| # that it had. | ||
| review_id="${found_id:-$(sed '1,/^[[:space:]]*$/d' "$response" | jq -r '.id // empty' 2>/dev/null || true)}" | ||
| case "$review_id" in (''|*[!0-9]*) review_id="" ;; esac | ||
| if [ -z "$review_id" ]; then | ||
| echo "::warning::the review carrying the verdict posted on $REPO#$PR but its id could not be read, so what the verdict step would append to it goes to this run's log" | ||
|
|
@@ -2954,6 +2972,14 @@ jobs: | |
| conclusion="${CONCLUSION:-unknown}" | ||
| head_state="${HEAD_STATE:-unknown}" | ||
| head_sha="${REVIEWED_SHA:-}" | ||
| if [ "$head_state" = same ]; then | ||
| now="$(gh api "repos/$REPO/pulls/$PR" --jq '.head.sha // empty' 2>/dev/null || true)" | ||
| case "$now" in | ||
| (''|*[!0-9a-f]*) head_state=unknown ;; | ||
| ("$head_sha") ;; | ||
| (*) head_state=moved ;; | ||
| esac | ||
| fi | ||
|
|
||
| # commit_id is omitted rather than sent empty when the commit is unknown: | ||
| # the API rejects an empty one, and its own default is the pull request's | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[suggestion] Matching only on
body == review_bodyandcommit_id == head_shacan select a review that this POST did not create. When the driver writes no verdict,review_bodyis the fixed string "Findings on the changed lines. The verdict and the summary are in this tool's comment…", so any earlier run on the same commit also matches (a re-run, or a rerun after a failed attempt). The same happens if a verdict repeats, or another identity posted the same text. On a false match the run:landed=trueand drops the anchored findings, which reach neither the diff nor the summary,position_posted, so the position step skips its review, andTo narrow it:
.id > $before/submitted_at >= $start..user.loginmatching the posting identity.$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT) in a hidden HTML comment inreview_bodyand match on that.(Codex raised the same point.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 384281e. The batch review body now ends with a hidden
<!-- seidroid-run:$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT -->marker. Thebody == review_bodymatch therefore only finds this attempt's own review, not an earlier run's, even for the fixed no-verdict text. The marker comes after the text, so the body still opens withVERDICT_MARKER. On the missing test: uci has no workflow test harness yet, so that would be a separate piece of work.