Repurpose @RCGitBot please test to approve CircleCI hold job#6274
Repurpose @RCGitBot please test to approve CircleCI hold job#6274ajpallares wants to merge 4 commits intopallares/change-ci-structure-for-PRsfrom
@RCGitBot please test to approve CircleCI hold job#6274Conversation
Instead of triggering a new `all-tests` pipeline, the GitHub Actions workflow now approves the `approve-full-tests` hold job in the already-running `run-all-tests` workflow via the CircleCI v2 API. Co-authored-by: Cursor <cursoragent@cursor.com>
- Rename trigger_all_tests.yml to approve_full_tests.yml - Add thumbs-up/down reaction on the triggering PR comment - Handle already-approved hold jobs gracefully - Replace silent curl flags with explicit timeouts - Update comments to reflect new approve-hold-job behavior Co-authored-by: Cursor <cursoragent@cursor.com>
rickvdl
left a comment
rickvdl
left a comment
There was a problem hiding this comment.
Nice touch! Thanks for adding this 🙌
| | jq -r '.items[0].id') | ||
|
|
||
| if [[ -z "$PIPELINE_ID" || "$PIPELINE_ID" == "null" ]]; then | ||
| echo "::error::No CircleCI pipeline found for branch '$BRANCH'" |
There was a problem hiding this comment.
It would probably add a lot of complexity (updating comments later again), but would it be possible to communicate this back to the author through the comment (same thing on line 75)?
| types: [created] | ||
|
|
||
| jobs: | ||
| approve-full-tests: |
There was a problem hiding this comment.
Nitpicky and I don't have a great alternative, but the job name confuses me sometimes 😅 The first thing that comes to mind when I read it is that I can "approve the result of the tests". I also get your point about trigger_all_tests but it might be a bite more explicit? Or approve_run_all_tests / approve_trigger_all_tests?
Please ignore if this was already discussed 🙈
tonidero
left a comment
tonidero
left a comment
There was a problem hiding this comment.
Nice touch! I do wonder if we should just remove this TBH... feels like extra complexity and things to learn when there should already be a straightforward way to approve that hold job... Wdyt?
…rove-hold-job-with-comment
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| if [[ -z "$WORKFLOW_ID" || "$WORKFLOW_ID" == "null" ]]; then | ||
| echo "::error::No '$WORKFLOW_NAME' workflow found in pipeline $PIPELINE_ID" | ||
| exit 1 |
There was a problem hiding this comment.
Approves pipeline not tied to PR SHA
Medium Severity
The CircleCI lookup selects the “most recent pipeline for this branch” via project/$PROJECT_SLUG/pipeline?branch=$BRANCH, which can return an older pipeline (e.g., if new commits were pushed but CircleCI hasn’t created the latest pipeline yet). This can approve the hold job on the wrong run and leave the intended PR run unapproved.
| on: | ||
| issue_comment: | ||
| types: [created] | ||
|
|
There was a problem hiding this comment.
Missing token permissions for gh reactions
Medium Severity
The workflow performs gh pr view and posts a comment reaction, but no explicit permissions: are set for GITHUB_TOKEN. In repos/orgs where the default token permissions are read-only, the reaction call (needs issues: write) can fail and flip the job to failed even if the CircleCI approval succeeded.
Additional Locations (1)
3 participants
Motivation
With the new CI structure, PRs no longer need a separate
all-testspipeline triggered from scratch. The full test suite is now gated behind anapprove-full-testshold job in therun-all-testsworkflow.This repurposes the existing
@RCGitBot please testcomment trigger to approve that hold job instead.Description
trigger-circleci-pipeline-actionstep with direct CircleCI v2 API calls that find the existing pipeline/workflow for the PR branch and approve theapprove-full-testshold job.trigger_all_tests.ymltoapprove_full_tests.yml.Note:
issue_commentworkflows always run from the default branch, so this won't take effect until merged tomain.Note
Medium Risk
CI-only change but it modifies automation that can kick off expensive full test runs and depends on correct CircleCI API/job name resolution and secret handling.
Overview
Repurposes the
@RCGitBot please testissue-comment trigger to approve the existing CircleCIapprove-full-testshold job in therun-all-testsworkflow (via CircleCI v2 API lookups), rather than triggering a new CircleCI pipeline.Renames/replaces the old workflow (
trigger_all_tests.yml) withapprove_full_tests.yml, keeps the RevenueCat org membership gate, handles the already-approved case, and adds a GitHub reaction on the triggering comment to signal success/failure.Written by Cursor Bugbot for commit 18ba974. This will update automatically on new commits. Configure here.