ci: add PR ready gate to clear self-reviewed label on push #56
Reference in New Issue
Block a user
Delete Branch "ci/pr-ready-gate"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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:
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
.gitea/workflows/pr-ready-gate.yml.gitea/workflows/pr-ready-gate.yml.gitea/workflows/pr-ready-gate.ymlRecommendation
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@@ -0,0 +13,4 @@env:GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}run: |PR_NUMBER=${{ github.event.pull_request.number }}[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 }}[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[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.
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
.gitea/workflows/pr-ready-gate.yml.gitea/workflows/pr-ready-gate.yml.gitea/workflows/pr-ready-gate.yml.gitea/workflows/pr-ready-gate.yml.gitea/workflows/pr-ready-gate.ymlRecommendation
REQUEST_CHANGES — To make this workflow robust and correct:
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@@ -0,0 +7,4 @@jobs:clear-labels:runs-on: ubuntu-24.04if: contains(github.event.pull_request.labels.*.name, 'self-reviewed')[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[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 presentcurl -sS -X DELETE \-H "Authorization: token $GITEA_TOKEN" \[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}"[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}"[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
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
.gitea/workflows/pr-ready-gate.yml.gitea/workflows/pr-ready-gate.ymlRecommendation
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@@ -0,0 +12,4 @@- name: Remove self-reviewed label, reassign to authorenv:GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}run: |[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}\"]}" \[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.