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
Owner

When a PR is pushed after being marked self-reviewed, the label is now stale and should be removed. This matches the gargoyle CI behavior.

On synchronize:

  • Remove self-reviewed label if present
  • Reassign PR back to the author
When a PR is pushed after being marked self-reviewed, the label is now stale and should be removed. This matches the gargoyle CI behavior. On synchronize: - Remove self-reviewed label if present - Reassign PR back to the author
rodin self-assigned this 2026-05-10 15:39:31 +00:00
rodin added 1 commit 2026-05-10 15:39:32 +00:00
ci: add PR ready gate to clear self-reviewed label on push
CI / test (pull_request) Successful in 14s
CI / review (/anthropic/v1, anthropic--claude-4.6-sonnet, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Successful in 26s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m10s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m5s
b24c4dcc86
When a PR is pushed after being marked self-reviewed, the label is now
stale and should be removed. This matches the gargoyle CI behavior.

On synchronize:
- Remove self-reviewed label if present
- Reassign PR back to the author
sonnet-review-bot approved these changes 2026-05-10 15:40:36 +00:00
sonnet-review-bot left a comment
First-time contributor

Sonnet Review

Summary

New Gitea Actions workflow that removes the 'self-reviewed' label and reassigns the PR to its author on push (synchronize). The logic is straightforward, CI is passing, and the implementation is correct for the stated purpose.

Findings

# Severity File Line Finding
1 [MINOR] .gitea/workflows/pr-ready-gate.yml 17 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 '
2 [MINOR] .gitea/workflows/pr-ready-gate.yml 20 The DELETE curl call has '
3 [NIT] .gitea/workflows/pr-ready-gate.yml 16 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.

Recommendation

APPROVE — The workflow is functionally correct and CI passes. The two MINOR findings are worth addressing: (1) the hardcoded label ID 37 should at minimum have a comment explaining it, and ideally be resolved by name to be robust; (2) the swallowed DELETE error combined with the unconditional success echo can mask real failures. Neither is a blocker for this automation use-case, but improving observability (log the HTTP status of each curl call) would make debugging much easier. Approving as-is is reasonable given CI passes and the logic is sound.


Review by sonnet


Evaluated against b24c4dcc

# Sonnet Review ## Summary New Gitea Actions workflow that removes the 'self-reviewed' label and reassigns the PR to its author on push (synchronize). The logic is straightforward, CI is passing, and the implementation is correct for the stated purpose. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `.gitea/workflows/pr-ready-gate.yml` | 17 | 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. | | 2 | [MINOR] | `.gitea/workflows/pr-ready-gate.yml` | 20 | 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. | | 3 | [NIT] | `.gitea/workflows/pr-ready-gate.yml` | 16 | 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. | ## Recommendation **APPROVE** — The workflow is functionally correct and CI passes. The two MINOR findings are worth addressing: (1) the hardcoded label ID 37 should at minimum have a comment explaining it, and ideally be resolved by name to be robust; (2) the swallowed DELETE error combined with the unconditional success echo can mask real failures. Neither is a blocker for this automation use-case, but improving observability (log the HTTP status of each curl call) would make debugging much easier. Approving as-is is reasonable given CI passes and the logic is sound. --- *Review by sonnet* <!-- review-bot:sonnet --> --- *Evaluated against b24c4dcc*
@@ -0,0 +13,4 @@
env:
GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
run: |
PR_NUMBER=${{ github.event.pull_request.number }}
First-time contributor

[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.
@@ -0,0 +14,4 @@
GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
run: |
PR_NUMBER=${{ github.event.pull_request.number }}
AUTHOR=${{ github.event.pull_request.user.login }}
First-time contributor

[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.
@@ -0,0 +17,4 @@
AUTHOR=${{ github.event.pull_request.user.login }}
SELF_REVIEWED_LABEL_ID=37
# Remove self-reviewed label if present
First-time contributor

[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.
aweiker approved these changes 2026-05-10 15:40:38 +00:00
Dismissed
gpt-review-bot requested changes 2026-05-10 15:41:23 +00:00
gpt-review-bot left a comment
First-time contributor

Gpt Review

Summary

The workflow intent is clear and CI passed, but there are reliability and correctness issues that can silently prevent the desired behavior. In particular, the hard-coded label ID, potential misuse of the pulls endpoint for assignees, and lack of proper HTTP error handling should be addressed.

Findings

# Severity File Line Finding
1 [MAJOR] .gitea/workflows/pr-ready-gate.yml 18 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.
2 [MAJOR] .gitea/workflows/pr-ready-gate.yml 30 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.
3 [MAJOR] .gitea/workflows/pr-ready-gate.yml 22 curl requests are not configured to fail the step on HTTP errors. The DELETE uses '
4 [MINOR] .gitea/workflows/pr-ready-gate.yml 10 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.
5 [MINOR] .gitea/workflows/pr-ready-gate.yml 32 The success message is printed unconditionally even if the API calls failed. This can mislead operators when actions did not actually occur.

Recommendation

REQUEST_CHANGES — To make this workflow robust and correct:

  • Avoid hard-coding the label ID. Query the repository labels to resolve the ID by name at runtime, or use an endpoint that allows removing a label by name if available. For example, GET /repos/{owner}/{repo}/labels to map 'self-reviewed' to its current ID before deletion.
  • Use the issues endpoint to manage PR metadata that is shared with issues. For reassignment, switch to PATCH https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/issues/${PR_NUMBER} with the assignees payload, which aligns with Gitea’s API for assignees.
  • Make HTTP failures fail the step unless you explicitly want to ignore a specific status (like 404 when the label is not present). Add -fS to curl, capture and check HTTP status codes, and only suppress failures for expected cases. For example, use: curl -sS -w '%{http_code}' -o /dev/null ... and test for 204/200.
  • Adjust the conditional to a more reliably evaluated expression, e.g., if: contains(join(github.event.pull_request.labels.*.name, ','), 'self-reviewed'). This avoids ambiguity in how contains handles arrays across runners.
  • Gate the echo message on actual success of the API calls, or log granular results (e.g., 'label removed: yes/no', 'reassigned: yes/no') based on checked status codes.
    Implementing these changes will ensure the label is actually cleared and the PR is reassigned reliably, and that failures are surfaced appropriately.

Review by gpt


Evaluated against b24c4dcc

# Gpt Review ## Summary The workflow intent is clear and CI passed, but there are reliability and correctness issues that can silently prevent the desired behavior. In particular, the hard-coded label ID, potential misuse of the pulls endpoint for assignees, and lack of proper HTTP error handling should be addressed. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MAJOR] | `.gitea/workflows/pr-ready-gate.yml` | 18 | 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. | | 2 | [MAJOR] | `.gitea/workflows/pr-ready-gate.yml` | 30 | 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. | | 3 | [MAJOR] | `.gitea/workflows/pr-ready-gate.yml` | 22 | 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. | | 4 | [MINOR] | `.gitea/workflows/pr-ready-gate.yml` | 10 | 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. | | 5 | [MINOR] | `.gitea/workflows/pr-ready-gate.yml` | 32 | The success message is printed unconditionally even if the API calls failed. This can mislead operators when actions did not actually occur. | ## Recommendation **REQUEST_CHANGES** — To make this workflow robust and correct: - Avoid hard-coding the label ID. Query the repository labels to resolve the ID by name at runtime, or use an endpoint that allows removing a label by name if available. For example, GET /repos/{owner}/{repo}/labels to map 'self-reviewed' to its current ID before deletion. - Use the issues endpoint to manage PR metadata that is shared with issues. For reassignment, switch to PATCH https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/issues/${PR_NUMBER} with the assignees payload, which aligns with Gitea’s API for assignees. - Make HTTP failures fail the step unless you explicitly want to ignore a specific status (like 404 when the label is not present). Add -fS to curl, capture and check HTTP status codes, and only suppress failures for expected cases. For example, use: curl -sS -w '%{http_code}' -o /dev/null ... and test for 204/200. - Adjust the conditional to a more reliably evaluated expression, e.g., if: contains(join(github.event.pull_request.labels.*.name, ','), 'self-reviewed'). This avoids ambiguity in how contains handles arrays across runners. - Gate the echo message on actual success of the API calls, or log granular results (e.g., 'label removed: yes/no', 'reassigned: yes/no') based on checked status codes. Implementing these changes will ensure the label is actually cleared and the PR is reassigned reliably, and that failures are surfaced appropriately. --- *Review by gpt* <!-- review-bot:gpt --> --- *Evaluated against b24c4dcc*
@@ -0,0 +7,4 @@
jobs:
clear-labels:
runs-on: ubuntu-24.04
if: contains(github.event.pull_request.labels.*.name, 'self-reviewed')
First-time contributor

[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.
@@ -0,0 +15,4 @@
run: |
PR_NUMBER=${{ github.event.pull_request.number }}
AUTHOR=${{ github.event.pull_request.user.login }}
SELF_REVIEWED_LABEL_ID=37
First-time contributor

[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.
@@ -0,0 +19,4 @@
# Remove self-reviewed label if present
curl -sS -X DELETE \
-H "Authorization: token $GITEA_TOKEN" \
First-time contributor

[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.
@@ -0,0 +27,4 @@
-H "Authorization: token $GITEA_TOKEN" \
-H "Content-Type: application/json" \
-d "{\"assignees\": [\"${AUTHOR}\"]}" \
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/pulls/${PR_NUMBER}"
First-time contributor

[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.
@@ -0,0 +29,4 @@
-d "{\"assignees\": [\"${AUTHOR}\"]}" \
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/pulls/${PR_NUMBER}"
echo "Cleared self-reviewed label and reassigned PR #${PR_NUMBER} to ${AUTHOR}"
First-time contributor

[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.
security-review-bot requested review from security-review-bot 2026-05-10 15:41:29 +00:00
security-review-bot approved these changes 2026-05-10 15:41:29 +00:00
security-review-bot left a comment
Collaborator

Security Review

Summary

The workflow correctly removes a stale label and reassigns the PR to the author on synchronize events, using a token securely. CI passed. Minor hardening is suggested around JSON construction to avoid potential injection/format issues.

Findings

# Severity File Line Finding
1 [MINOR] .gitea/workflows/pr-ready-gate.yml 29 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.
2 [MINOR] .gitea/workflows/pr-ready-gate.yml 15 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.

Recommendation

APPROVE — Overall, the change is sound and aligns with the stated behavior to clear a stale self-reviewed label and reassign the PR to its author on synchronize events. To harden the implementation, construct the PATCH JSON body safely to avoid any issues with special characters in author login names, e.g., using: -d "$(jq -n --arg author "$AUTHOR" '{assignees: [$author]}')". Also verify that the CI environment does not expose the GITEA_TOKEN to untrusted PR contexts (such as forks), or limit the workflow to trusted branches/authors if necessary. With these minor adjustments, the workflow would be more robust against edge cases while maintaining current functionality.


Review by security


Evaluated against b24c4dcc

# Security Review ## Summary The workflow correctly removes a stale label and reassigns the PR to the author on synchronize events, using a token securely. CI passed. Minor hardening is suggested around JSON construction to avoid potential injection/format issues. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `.gitea/workflows/pr-ready-gate.yml` | 29 | 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. | | 2 | [MINOR] | `.gitea/workflows/pr-ready-gate.yml` | 15 | 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. | ## Recommendation **APPROVE** — Overall, the change is sound and aligns with the stated behavior to clear a stale self-reviewed label and reassign the PR to its author on synchronize events. To harden the implementation, construct the PATCH JSON body safely to avoid any issues with special characters in author login names, e.g., using: -d "$(jq -n --arg author "$AUTHOR" '{assignees: [$author]}')". Also verify that the CI environment does not expose the GITEA_TOKEN to untrusted PR contexts (such as forks), or limit the workflow to trusted branches/authors if necessary. With these minor adjustments, the workflow would be more robust against edge cases while maintaining current functionality. --- *Review by security* <!-- review-bot:security --> --- *Evaluated against b24c4dcc*
@@ -0,0 +12,4 @@
- name: Remove self-reviewed label, reassign to author
env:
GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
run: |
Collaborator

[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.
@@ -0,0 +26,4 @@
curl -sS -X PATCH \
-H "Authorization: token $GITEA_TOKEN" \
-H "Content-Type: application/json" \
-d "{\"assignees\": [\"${AUTHOR}\"]}" \
Collaborator

[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.
aweiker approved these changes 2026-05-10 15:41:33 +00:00
aweiker merged commit 0e3c85f05c into main 2026-05-10 15:41:37 +00:00
Sign in to join this conversation.
5 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#56