rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

ci: add PR ready gate to clear self-reviewed label on push #56

Merged
aweiker merged 1 commits from ci/pr-ready-gate into main 2026-05-10 15:41:37 +00:00
Conversation 5 Commits 1 Files Changed 1
+32
1 changed files with 32 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit b24c4dcc86 - Show all commits
.gitea/workflows/pr-ready-gate.yml
+32
View File
@@ -0,0 +1,32 @@
name: PR Ready Gate
on:
pull_request:
types: [synchronize]
jobs:
clear-labels:
runs-on: ubuntu-24.04
if: contains(github.event.pull_request.labels.*.name, 'self-reviewed')
gpt-review-bot commented 2026-05-10 15:41:23 +00:00
Review
Copy Link
Copy Source

[MINOR] The job-level condition uses contains(github.event.pull_request.labels.*.name, 'self-reviewed'). Depending on Gitea/GitHub Actions expression support, star expansion may not behave as expected for contains. Consider using join(...) around the names to ensure a string search works reliably.

**[MINOR]** The job-level condition uses contains(github.event.pull_request.labels.*.name, 'self-reviewed'). Depending on Gitea/GitHub Actions expression support, star expansion may not behave as expected for contains. Consider using join(...) around the names to ensure a string search works reliably.
steps:
- name: Remove self-reviewed label, reassign to author
env:
GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
run: |
security-review-bot commented 2026-05-10 15:41:29 +00:00
Review
Copy Link
Copy Source

[MINOR] This workflow runs on pull_request synchronize and uses a repository secret. Ensure the CI platform does not expose secrets to workflows triggered from forked repositories or untrusted contributors. Restrict secret usage to trusted contexts to avoid potential exfiltration via modified workflows.

**[MINOR]** This workflow runs on pull_request synchronize and uses a repository secret. Ensure the CI platform does not expose secrets to workflows triggered from forked repositories or untrusted contributors. Restrict secret usage to trusted contexts to avoid potential exfiltration via modified workflows.
PR_NUMBER=${{ github.event.pull_request.number }}
sonnet-review-bot commented 2026-05-10 15:40:36 +00:00
Review
Copy Link
Copy Source

[NIT] PR_NUMBER and AUTHOR are derived from github.event context expressions interpolated directly into a shell script. While these values come from trusted Gitea event payloads (not user-supplied strings in the PR body/title), it is generally safer to pass context values through environment variables (already done for GITEA_TOKEN) rather than inline shell interpolation, to guard against unexpected characters. The current form is low-risk but worth noting.

**[NIT]** PR_NUMBER and AUTHOR are derived from github.event context expressions interpolated directly into a shell script. While these values come from trusted Gitea event payloads (not user-supplied strings in the PR body/title), it is generally safer to pass context values through environment variables (already done for GITEA_TOKEN) rather than inline shell interpolation, to guard against unexpected characters. The current form is low-risk but worth noting.
AUTHOR=${{ github.event.pull_request.user.login }}
sonnet-review-bot commented 2026-05-10 15:40:36 +00:00
Review
Copy Link
Copy Source

[MINOR] SELF_REVIEWED_LABEL_ID=37 is a hardcoded numeric ID that is opaque and fragile. If the label is ever recreated (different ID) or this workflow is reused in another repository, it will silently fail to remove the label (the DELETE returns a non-2xx which is swallowed by '|| true'). Consider looking up the label by name via the API, or at minimum documenting why 37 is the correct ID in a comment.

**[MINOR]** SELF_REVIEWED_LABEL_ID=37 is a hardcoded numeric ID that is opaque and fragile. If the label is ever recreated (different ID) or this workflow is reused in another repository, it will silently fail to remove the label (the DELETE returns a non-2xx which is swallowed by '|| true'). Consider looking up the label by name via the API, or at minimum documenting why 37 is the correct ID in a comment.
SELF_REVIEWED_LABEL_ID=37
gpt-review-bot commented 2026-05-10 15:41:23 +00:00
Review
Copy Link
Copy Source

[MAJOR] Hard-coded label ID (SELF_REVIEWED_LABEL_ID=37) is brittle and likely repository-specific. If the ID differs or changes, the label removal will silently do nothing.

**[MAJOR]** Hard-coded label ID (SELF_REVIEWED_LABEL_ID=37) is brittle and likely repository-specific. If the ID differs or changes, the label removal will silently do nothing.
# Remove self-reviewed label if present
sonnet-review-bot commented 2026-05-10 15:40:36 +00:00
Review
Copy Link
Copy Source

[MINOR] The DELETE curl call has '|| true', meaning a failure (wrong label ID, token permission issue, etc.) is silently ignored. The subsequent echo will still print 'Cleared self-reviewed label', giving a false success signal. Consider checking the HTTP response code (-w '%{http_code}') and logging a warning on unexpected status codes, while still allowing the workflow to continue for the reassignment step.

**[MINOR]** The DELETE curl call has '|| true', meaning a failure (wrong label ID, token permission issue, etc.) is silently ignored. The subsequent echo will still print 'Cleared self-reviewed label', giving a false success signal. Consider checking the HTTP response code (-w '%{http_code}') and logging a warning on unexpected status codes, while still allowing the workflow to continue for the reassignment step.
curl -sS -X DELETE \
-H "Authorization: token $GITEA_TOKEN" \
gpt-review-bot commented 2026-05-10 15:41:23 +00:00
Review
Copy Link
Copy Source

[MAJOR] curl requests are not configured to fail the step on HTTP errors. The DELETE uses '|| true' and neither request uses '-f'; this can mask failures (e.g., 4xx/5xx) and still echo success, making debugging difficult.

**[MAJOR]** curl requests are not configured to fail the step on HTTP errors. The DELETE uses '|| true' and neither request uses '-f'; this can mask failures (e.g., 4xx/5xx) and still echo success, making debugging difficult.
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/issues/${PR_NUMBER}/labels/${SELF_REVIEWED_LABEL_ID}" || true
# Reassign to author
curl -sS -X PATCH \
-H "Authorization: token $GITEA_TOKEN" \
-H "Content-Type: application/json" \
-d "{\"assignees\": [\"${AUTHOR}\"]}" \
security-review-bot commented 2026-05-10 15:41:29 +00:00
Review
Copy Link
Copy Source

[MINOR] The JSON payload for assignees embeds the unescaped AUTHOR value directly inside a shell-quoted string. While typical login names are constrained, untrusted values could break JSON formatting or cause unexpected behavior. Consider constructing the JSON safely (e.g., using jq to escape values) to prevent injection/formatting issues.

**[MINOR]** The JSON payload for assignees embeds the unescaped AUTHOR value directly inside a shell-quoted string. While typical login names are constrained, untrusted values could break JSON formatting or cause unexpected behavior. Consider constructing the JSON safely (e.g., using jq to escape values) to prevent injection/formatting issues.
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/pulls/${PR_NUMBER}"
gpt-review-bot commented 2026-05-10 15:41:23 +00:00
Review
Copy Link
Copy Source

[MAJOR] PATCHing assignees via /pulls/{PR_NUMBER} may not be supported by Gitea for setting assignees; Gitea typically updates assignees on the issues endpoint (/issues/{index}). This risks the reassignment silently failing.

**[MAJOR]** PATCHing assignees via /pulls/{PR_NUMBER} may not be supported by Gitea for setting assignees; Gitea typically updates assignees on the issues endpoint (/issues/{index}). This risks the reassignment silently failing.
echo "Cleared self-reviewed label and reassigned PR #${PR_NUMBER} to ${AUTHOR}"
gpt-review-bot commented 2026-05-10 15:41:23 +00:00
Review
Copy Link
Copy Source

[MINOR] The success message is printed unconditionally even if the API calls failed. This can mislead operators when actions did not actually occur.

**[MINOR]** The success message is printed unconditionally even if the API calls failed. This can mislead operators when actions did not actually occur.