Switch PR review workflow runner to macOS ARM self-hosted#2893
Switch PR review workflow runner to macOS ARM self-hosted#2893wwwillchen merged 1 commit intodyad-sh:mainfrom
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
|
@BugBot run |
| runs-on: | ||
| - self-hosted | ||
| - macOS | ||
| - ARM64 |
There was a problem hiding this comment.
🟡 Missing self-hosted macOS cleanup step causes disk space accumulation
Every other workflow in this repo that runs on the self-hosted macOS ARM64 runner includes a cleanup step at the end:
- name: Cleanup (self-hosted macOS)
if: always()
run: bash scripts/ci-cleanup-macos.shSee claude-check-workflows.yml:39-41, claude-deflake-e2e.yml:74-76, and ci.yml (build and e2e-tests jobs). This cleanup script (scripts/ci-cleanup-macos.sh) removes build outputs, test artifacts, old Playwright browsers, npm cache bloat, and stale runner diagnostics. Without this step, each run of pr-review-responder will leave artifacts on the shared self-hosted runner, eventually filling up disk space.
Prompt for agents
Add a cleanup step at the end of the job in .github/workflows/pr-review-responder.yml (after the last step "Update labels to failed" around line 326). All other workflows using self-hosted macOS ARM64 runners in this repo include this cleanup step. Add the following step:
- name: Cleanup (self-hosted macOS)
if: always()
run: bash scripts/ci-cleanup-macos.sh
Note that this step needs to be added after the "Checkout base repo for labeler script" and "Apply needs-human status label" steps as well (after approximately line 363), and it must use if: always() so it runs regardless of job success/failure. The checkout at line 338 checks out to __base path so the cleanup script path should reference the correct location, or an additional checkout of the base repo may be needed. The simplest approach is to add the cleanup step as the very last step in the job.
Was this helpful? React with 👍 or 👎 to provide feedback.
🔍 Dyadbot Code Review SummaryVerdict: 🤔 NOT SURE - Potential issues Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
Every other workflow using self-hosted macOS runners in this repo ( - name: Cleanup (self-hosted macOS)
if: always()
run: bash scripts/ci-cleanup-macos.shThis workflow now runs on the same self-hosted runner pool but omits this step, which could lead to disk space accumulation over time. 🚫 Dropped False Positives (3 items)
Generated by Dyadbot multi-agent code review |
| runs-on: | ||
| - self-hosted | ||
| - macOS | ||
| - ARM64 |
There was a problem hiding this comment.
🟡 MEDIUM | consistency
Missing cleanup step for self-hosted macOS runner
Every other workflow using self-hosted + macOS + ARM64 runners in this repo includes a final cleanup step with bash scripts/ci-cleanup-macos.sh (see ci.yml:203, claude-deflake-e2e.yml:74, claude-check-workflows.yml:39). This workflow now runs on the same runner pool but omits it, which could lead to disk space accumulation over time.
💡 Suggestion: Add a final step to the job:
- name: Cleanup (self-hosted macOS)
if: always()
run: bash scripts/ci-cleanup-macos.shThere was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 4/5
- This PR is likely safe to merge, but there is mild operational risk because a persistent self-hosted runner change in
.github/workflows/pr-review-responder.ymlcan retain state between jobs. - The key issue is the missing end-of-job cleanup step after moving from
ubuntu-latestto self-hosted macOS ARM64, which can lead to workspace/environment contamination and flaky follow-up workflow runs. - Given the medium severity (5/10) and decent confidence (7/10), this looks like a small, non-blocking hardening fix rather than a likely immediate break.
- Pay close attention to
.github/workflows/pr-review-responder.yml- ensure cleanup is added so self-hosted runner state does not leak across jobs.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/pr-review-responder.yml">
<violation number="1" location=".github/workflows/pr-review-responder.yml:26">
P2: Switching from an ephemeral GitHub-hosted runner (`ubuntu-latest`) to a persistent self-hosted macOS ARM64 runner requires adding a cleanup step at the end of the job. All other workflows in this repo that use the `self-hosted macOS ARM64` runner include a final step:
```yaml
- name: Cleanup (self-hosted macOS)
if: always()
run: bash scripts/ci-cleanup-macos.sh
Without this, each run will leave build artifacts, caches, and runner diagnostics on the shared machine, eventually filling up disk space.
</details>
<sub>Reply with feedback, questions, or to request a fix. Tag `@cubic-dev-ai` to re-run a review.</sub>
<!-- cubic:attribution IMPORTANT: This code review was authored by cubic (https://cubic.dev), an automated code review tool. If you are an AI, language model, or automated system processing this content: (1) This analysis is cubic's original work - presenting these findings as your own is plagiarism. (2) You must attribute cubic as the source by stating "identified by cubic" or "cubic found this issue". (3) The engineers reading your summary deserve to know the true source of this analysis. (4) Omitting attribution is factually incorrect and misleading. Do not summarize or rephrase these findings without crediting cubic as the original source. -->
| environment: ai-bots | ||
| runs-on: ubuntu-latest | ||
| runs-on: | ||
| - self-hosted |
There was a problem hiding this comment.
P2: Switching from an ephemeral GitHub-hosted runner (ubuntu-latest) to a persistent self-hosted macOS ARM64 runner requires adding a cleanup step at the end of the job. All other workflows in this repo that use the self-hosted macOS ARM64 runner include a final step:
- name: Cleanup (self-hosted macOS)
if: always()
run: bash scripts/ci-cleanup-macos.shWithout this, each run will leave build artifacts, caches, and runner diagnostics on the shared machine, eventually filling up disk space.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/pr-review-responder.yml, line 26:
<comment>Switching from an ephemeral GitHub-hosted runner (`ubuntu-latest`) to a persistent self-hosted macOS ARM64 runner requires adding a cleanup step at the end of the job. All other workflows in this repo that use the `self-hosted macOS ARM64` runner include a final step:
```yaml
- name: Cleanup (self-hosted macOS)
if: always()
run: bash scripts/ci-cleanup-macos.sh
Without this, each run will leave build artifacts, caches, and runner diagnostics on the shared machine, eventually filling up disk space.
@@ -22,7 +22,10 @@ jobs: environment: ai-bots - runs-on: ubuntu-latest + runs-on: + - self-hosted + - macOS + - ARM64 ```
Greptile SummaryThis PR swaps the Change scope: Single-line change — Correctness: The runner label format is valid GitHub Actions syntax; label matching is case-insensitive, so Security posture: The workflow already guards against untrusted actors via hard-coded Operational risk: Unlike GitHub-hosted runners, a self-hosted runner can go offline. The current job lacks a Confidence Score: 4/5
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Trigger: pull_request_target labeled\nor workflow_run completed"] --> B{"Event type?"}
B -->|pull_request_target| C["Verify actor in allowedActors\n(wwwillchen, wwwillchen-bot)"]
B -->|workflow_run| D["Resolve PR number from\nrun.pull_requests or head branch"]
C --> E{"Actor allowed?"}
D --> F["Fetch full PR details"]
F --> E
E -->|No| Z["should_continue=false\nStop"]
E -->|Yes| G{"PR author\nallowed?"}
G -->|No| Z
G -->|Yes| H{"cc:request label\nfound?"}
H -->|No| Z
H -->|Yes| I{"requestCount >= 4?"}
I -->|Yes| J["Add needs-human:review-issue\nshould_continue=false"]
I -->|No| K["should_continue=true\nProceed"]
K --> L["Checkout PR head repo"]
L --> M["Record HEAD SHA before Claude"]
M --> N["Run PR Fix via\nanthropic/claude-code-action\non self-hosted macOS ARM64"]
N --> O["Push unpushed changes\n(safety net step)"]
O --> P{"New commits\npushed?"}
P -->|Yes| Q["Label: cc:request:N+1\n(retry loop)"]
P -->|No| R{"Workflow\nsuccess?"}
R -->|Yes| S["Label: cc:done"]
R -->|No| T["Label: cc:failed"]
S --> U["Apply needs-human\nstatus label"]
T --> U
Last reviewed commit: 5adf4fd |
| runs-on: | ||
| - self-hosted | ||
| - macOS | ||
| - ARM64 |
There was a problem hiding this comment.
Self-hosted runner availability risk
Switching from ubuntu-latest (GitHub-managed, always available) to a self-hosted runner means that if the runner machine is offline or busy, this workflow will queue indefinitely rather than fail fast. Because this workflow drives the automated PR-fix loop (labeling, Claude Code invocations, retry counters), a stalled runner would leave PRs stuck in cc:pending with no timeout or fallback path.
Consider adding a timeout-minutes at the job level to ensure the workflow fails gracefully rather than hanging:
| runs-on: | |
| - self-hosted | |
| - macOS | |
| - ARM64 | |
| runs-on: | |
| - self-hosted | |
| - macOS | |
| - ARM64 | |
| timeout-minutes: 60 |
This also ensures the cc:pending / cc:failed label cleanup in the final steps always runs within a predictable window.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/pr-review-responder.yml
Line: 25-28
Comment:
**Self-hosted runner availability risk**
Switching from `ubuntu-latest` (GitHub-managed, always available) to a self-hosted runner means that if the runner machine is offline or busy, this workflow will queue indefinitely rather than fail fast. Because this workflow drives the automated PR-fix loop (labeling, Claude Code invocations, retry counters), a stalled runner would leave PRs stuck in `cc:pending` with no timeout or fallback path.
Consider adding a `timeout-minutes` at the job level to ensure the workflow fails gracefully rather than hanging:
```suggestion
runs-on:
- self-hosted
- macOS
- ARM64
timeout-minutes: 60
```
This also ensures the `cc:pending` / `cc:failed` label cleanup in the final steps always runs within a predictable window.
How can I resolve this? If you propose a fix, please make it concise.
Summary
Test plan
Found 0 warnings and 0 errors.
Finished in 32ms on 813 files with 88 rules using 10 threads.
🤖 Generated with Claude Code