-
Notifications
You must be signed in to change notification settings - Fork 0
ci: sync Antigravity reviewer to the fixed comment-selection version #259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| #!/usr/bin/env bash | ||
| # | ||
| # agy-review-selftest.sh -- guards the comment-selection logic in `agy-review.sh`. | ||
| # | ||
| # Why this exists: that filter decides which PR comments the bot DELETES, and it has been wrong | ||
| # twice, both times invisibly. | ||
| # | ||
| # 1. The just-posted comment was not reliably excluded. `new_comment_id` came from re-querying | ||
| # the comment list, which races GitHub's read replication; on a miss the exclusion became | ||
| # `select(.id != null)`, true for every id, and the run deleted the review it had just | ||
| # published. | ||
| # 2. jq's `--arg`/`--argjson` were handed to `gh api`, which has no such flags. It exited | ||
| # non-zero, `2>/dev/null` hid the message, and `set -o pipefail` + `set -e` killed the script | ||
| # AFTER posting — so stale comments silently accumulated and the job went red with nothing in | ||
| # the log explaining why. | ||
| # | ||
| # Neither was catchable by looking at the review the bot posted: both times it posted fine. So the | ||
| # filter is tested here directly, offline, against fixtures — no network, no `gh`, no runner. | ||
| # | ||
| # Run: bash scripts/agy-review-selftest.sh | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" | ||
|
|
||
| # Source the constants out of the reviewer without running it. `agy-review.sh` does its work at | ||
| # top level, so it cannot simply be sourced; the two values under test are lifted by pattern | ||
| # instead. That coupling is deliberate: if either declaration is renamed or reshaped, this test | ||
| # fails loudly rather than silently checking a stale copy of the filter. | ||
| extract_marker() { | ||
| sed -n 's/^MARKER="\(.*\)"$/\1/p' "$SCRIPT_DIR/agy-review.sh" | head -n 1 | ||
| } | ||
| extract_filter() { | ||
| sed -n "/^SELECT_STALE_JQ='/,/'\$/p" "$SCRIPT_DIR/agy-review.sh" \ | ||
| | sed "1s/^SELECT_STALE_JQ='//; \$s/'\$//" | ||
| } | ||
|
|
||
| MARKER="$(extract_marker)" | ||
| FILTER="$(extract_filter)" | ||
|
|
||
| [ -n "$MARKER" ] || { echo "FAIL: could not extract MARKER from agy-review.sh" >&2; exit 1; } | ||
| [ -n "$FILTER" ] || { echo "FAIL: could not extract SELECT_STALE_JQ from agy-review.sh" >&2; exit 1; } | ||
|
|
||
| # A non-empty extraction is not the same as a COMPLETE one. The `sed` range above ends at the | ||
| # first line closing with a quote, so a filter whose body ever ends a line that way would be | ||
| # truncated — and a truncated jq program can still be valid and still return ids, which is the | ||
| # silent-wrong-answer this whole file exists to prevent. Two independent guards: | ||
| # | ||
| # 1. it must compile (a truncated program is usually, though not always, a syntax error); | ||
| # 2. it must END with the projection, which is what makes it a complete pipeline rather than a | ||
| # prefix of one. | ||
| # The named args must be supplied here too: the filter references `$marker`/`$new_id`, and jq | ||
| # rejects an undefined variable at COMPILE time — so omitting them fails a perfectly good program. | ||
| if ! printf '[]' | jq --arg marker x --argjson new_id 0 "$FILTER" >/dev/null 2>&1; then | ||
| echo "FAIL: extracted SELECT_STALE_JQ is not a valid jq program (truncated?):" >&2 | ||
| printf '%s\n' "$FILTER" >&2 | ||
| exit 1 | ||
| fi | ||
| case "$(printf '%s' "$FILTER" | tr -d '[:space:]')" in | ||
| *'|.id') : ;; | ||
| *) echo "FAIL: extracted SELECT_STALE_JQ does not end in '| .id'; extraction truncated" >&2 | ||
| printf '%s\n' "$FILTER" >&2 | ||
| exit 1 ;; | ||
| esac | ||
|
|
||
| fixture() { | ||
| cat <<JSON | ||
| [ | ||
| {"id": 111, "user": {"type": "Bot", "login": "github-actions[bot]"}, "body": "$MARKER\nold review"}, | ||
| {"id": 222, "user": {"type": "Bot", "login": "github-actions[bot]"}, "body": "$MARKER\nolder still"}, | ||
| {"id": 333, "user": {"type": "User", "login": "someone"}, "body": "$MARKER\nnot ours"}, | ||
| {"id": 444, "user": {"type": "Bot", "login": "other-bot"}, "body": "$MARKER\nwrong bot"}, | ||
| {"id": 555, "user": {"type": "Bot", "login": "github-actions[bot]"}, "body": "an ordinary bot comment"}, | ||
| {"id": 999, "user": {"type": "Bot", "login": "github-actions[bot]"}, "body": "$MARKER\nJUST POSTED"} | ||
| ] | ||
| JSON | ||
| } | ||
|
|
||
| # Ids as a single space-separated line, with no trailing space — so the expected values below read | ||
| # as what they are rather than carrying padding an assertion would have to mirror. | ||
| select_ids() { | ||
| fixture | jq -r --arg marker "$MARKER" --argjson new_id "$1" "$FILTER" | sort -n | paste -sd' ' - | ||
| } | ||
|
|
||
| fails=0 | ||
| check() { | ||
| local name="$1" want="$2" got="$3" | ||
| if [ "$got" = "$want" ]; then | ||
| echo " ok $name" | ||
| else | ||
| echo " FAIL $name" | ||
| echo " want: [$want]" | ||
| echo " got: [$got]" | ||
| fails=$((fails + 1)) | ||
| fi | ||
| } | ||
|
|
||
| echo "agy-review comment-selection self-test" | ||
|
|
||
| # The whole point: the comment just published is never selected for deletion. | ||
| check "excludes the just-posted comment" "111 222" "$(select_ids 999)" | ||
|
|
||
| # The author filter is a security control, not tidiness: without it any user could paste the | ||
| # marker into a comment and have the bot delete comments on the next run. | ||
| check "ignores other users and other bots" "111 222" "$(select_ids 999)" | ||
|
|
||
| # A bot comment without the marker is somebody else's feature (a CI summary, a deploy note). | ||
| check "ignores bot comments without the marker" "111 222" "$(select_ids 999)" | ||
|
|
||
| # Regression #1, pinned: an unknown id must not select everything. The caller now refuses to run | ||
| # the delete at all in this case, but the filter itself is checked so the two guards are | ||
| # independent rather than one relying on the other. | ||
| check "an id of 0 still excludes nothing real" "111 222 999" "$(select_ids 0)" | ||
|
|
||
| # A different id in the set behaves the same way, so the exclusion is genuinely by value. | ||
| check "excludes whichever id it is given" "222 999" "$(select_ids 111)" | ||
|
|
||
| # Regression #2, pinned: `--arg`/`--argjson` belong to jq. If they are ever moved onto `gh api` | ||
| # again, that command exits non-zero — assert the flags are not passed to `gh api` in the script. | ||
| # Line continuations are folded first: `--arg` moved onto a continuation line would otherwise sit | ||
| # on a different physical line from `gh api`, and a line-by-line grep would report a false pass on | ||
| # exactly the mistake this check exists to catch. | ||
| if sed -e ':a' -e '/\\$/{N;s/\\\n//;ba' -e '}' "$SCRIPT_DIR/agy-review.sh" \ | ||
| | grep -qE 'gh api[^|]*--(arg|argjson)'; then | ||
| echo " FAIL --arg/--argjson passed to \`gh api\` (jq flags; gh api rejects them)" | ||
| fails=$((fails + 1)) | ||
| else | ||
| echo " ok --arg/--argjson are not passed to \`gh api\`" | ||
| fi | ||
|
|
||
| if [ "$fails" -ne 0 ]; then | ||
| echo "$fails check(s) failed" >&2 | ||
| exit 1 | ||
| fi | ||
| echo "all checks passed" |
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
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.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Doesgh pr commentprint the new comment's URL (with#issuecomment-<id>) to stdout on success?💡 Result:
No, the
gh pr commentcommand does not print the URL of the newly created comment to stdout upon success [1][2]. Whilegh pr commentis designed to add a comment to a pull request, the official GitHub CLI documentation and manual pages do not list the output of a comment URL as part of its standard behavior [1][2]. In many cases, the command executes silently on success, or may provide minimal feedback depending on the terminal environment, but it does not return the structured URL (such as...#issuecomment-<id>) that some users might expect for automation or scripting purposes [1][2]. If you require the URL of a newly created comment, you may need to use an alternative approach, such as: 1. Usinggh apito perform the request directly, which allows you to inspect the full JSON response containing thehtml_urlfield [3][4]. 2. Listing the comments after the command has executed to retrieve the most recent one [5][6]. For programmatic workflows where the URL is needed, leveraging thegh apicommand with the appropriate REST API endpoint (typically/repos/{owner}/{repo}/issues/{pull_number}/comments) is the recommended way to retrieve the created comment's metadata, including its URL [3][7][8].Citations:
🏁 Script executed:
Repository: doublegate/RustyN64
Length of output: 18075
🏁 Script executed:
Repository: doublegate/RustyN64
Length of output: 18191
🌐 Web query:
site:github.com/cli/cli gh pr comment source output URL NewComment💡 Result:
The
gh pr commentcommand does not natively output the URL of a newly created comment to standard output in a simple, direct format by default [1]. However, you can obtain the URL of a newly created comment by using the command in combination with shell utilities or by capturing its output if the specific version ofghyou are using supports it [2]. 1. Capture the output: Recent updates toghhave improved the behavior of command output. When runninggh pr comment, you can often pipe or capture the output to verify the URL [2]. For example, in automated environments, you might see the URL printed to stdout if the command is executed successfully [2]. 2. Alternative Approach (Manual Retrieval): If the command does not provide the URL directly in your environment, you can retrieve it by querying the pull request's comments immediately after creation usinggh pr viewcombined withjq[3][4]. Example usingjqto find the most recent comment: gh pr view --json comments --jq '.comments[-1].url' 3. Best Practice for Automation: If you are working in CI/CD or scripts, the most reliable way to get the URL is to use theghJSON output capabilities to inspect the PR state or to use the GitHub API directly viagh apito create the comment, which returns the full comment object including thehtml_url[3][5][1]. If you find that the command is not outputting the URL as expected, it is recommended to check your version of the GitHub CLI (gh --version) and use thegh pr viewmethod for deterministic results in scripts [3][4].Citations:
gh issue/pr comment] Add--create-if-noneand prompts to create a comment if no comment already exists cli/cli#10427gh pr/issue commentallow editing selected comments cli/cli#10865🌐 Web query:
site:cli.github.com/manual/gh_pr_comment "URL" output💡 Result:
gh pr commentaccepts a pull-request URL as its argument:gh pr comment https://github.com/OWNER/REPO/pull/123 --body "Comment text"The command’s output behavior is not documented as returning a URL; it adds, edits, or deletes a comment. (cli.github.com)
Citations:
🏁 Script executed:
Repository: doublegate/RustyN64
Length of output: 10533
Pin or assert the GitHub CLI version
ghv2.96.0 prints the created comment URL on stdout. The workflow does not pingh, so this output format is not guaranteed on the self-hosted runner. Pin or assert the CLI version to preserve determinism.🤖 Prompt for AI Agents