Remove fork PR guards from CI workflows#2092
Conversation
Fork PRs are already gated by GitHub's "Require approval for outside collaborators" setting. The workflow-level guards were redundant and prevented WarpBuild jobs from running even after maintainer approval.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRemoved job-level Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR removes the workflow-level
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Fork PR opened] --> B{GitHub repo setting:\nRequire approval for\noutside collaborators?}
B -- No approval yet --> C[Workflows queued\nbut not run]
B -- Approved by maintainer --> D[All CI jobs triggered]
D --> E[ubuntu-latest jobs\nworkflow-guard-tests\nremote-daemon-tests\nweb-typecheck]
D --> F[WarpBuild paid runners\ntests\ntests-build-and-lag\nui-regressions\nbuild-ghosttykit\ncompat-tests]
F --> G{secrets available?}
G -- GITHUB_TOKEN only\nfor fork PRs --> H[Jobs run normally\nGHOSTTY_RELEASE_TOKEN\nnot available to forks]
G -- Full secrets\nfor internal PRs --> I[All steps including\nrelease upload succeed]
Reviews (1): Last reviewed commit: "Remove fork PR guards from CI workflows" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #!/usr/bin/env bash | ||
| # Regression test for https://github.com/manaflow-ai/cmux/issues/385. | ||
| # Ensures paid/gated CI jobs are never run for fork pull requests. | ||
| # Ensures paid CI jobs use WarpBuild runners. | ||
| # Fork PRs are gated by GitHub's built-in "Require approval for outside | ||
| # collaborators" setting, so workflow-level fork guards are not needed. | ||
| set -euo pipefail | ||
|
|
||
| ROOT_DIR="$(cd "$(dirname "$0")/.." && pwd)" | ||
| WORKFLOW_FILE="$ROOT_DIR/.github/workflows/ci.yml" | ||
|
|
||
| EXPECTED_IF="if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository" | ||
|
|
||
| if ! grep -Fq "$EXPECTED_IF" "$WORKFLOW_FILE"; then | ||
| echo "FAIL: Missing fork pull_request guard in $WORKFLOW_FILE" | ||
| echo "Expected line:" | ||
| echo " $EXPECTED_IF" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # tests: must use WarpBuild runner with fork guard (paid runner) | ||
| # tests: must use WarpBuild runner (paid runner) | ||
| if ! awk ' | ||
| /^ tests:/ { in_tests=1; next } | ||
| in_tests && /^ [^[:space:]]/ { in_tests=0 } | ||
| in_tests && /runs-on: warp-macos-15-arm64-6x/ { saw_warp=1 } | ||
| in_tests && /github.event.pull_request.head.repo.full_name == github.repository/ { saw_guard=1 } | ||
| END { exit !(saw_warp && saw_guard) } | ||
| END { exit !(saw_warp) } | ||
| ' "$WORKFLOW_FILE"; then | ||
| echo "FAIL: tests block must keep both warp-macos-15-arm64-6x runner and fork guard" | ||
| echo "FAIL: tests block must use warp-macos-15-arm64-6x runner" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # tests-build-and-lag: must use WarpBuild runner with fork guard (paid runner) | ||
| # tests-build-and-lag: must use WarpBuild runner (paid runner) | ||
| if ! awk ' | ||
| /^ tests-build-and-lag:/ { in_tests=1; next } | ||
| in_tests && /^ [^[:space:]]/ { in_tests=0 } | ||
| in_tests && /runs-on: warp-macos-15-arm64-6x/ { saw_warp=1 } | ||
| in_tests && /github.event.pull_request.head.repo.full_name == github.repository/ { saw_guard=1 } | ||
| END { exit !(saw_warp && saw_guard) } | ||
| END { exit !(saw_warp) } | ||
| ' "$WORKFLOW_FILE"; then | ||
| echo "FAIL: tests-build-and-lag block must keep both warp-macos-15-arm64-6x runner and fork guard" | ||
| echo "FAIL: tests-build-and-lag block must use warp-macos-15-arm64-6x runner" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # ui-regressions: must use WarpBuild runner with fork guard (paid runner) | ||
| # ui-regressions: must use WarpBuild runner (paid runner) | ||
| if ! awk ' | ||
| /^ ui-regressions:/ { in_tests=1; next } | ||
| in_tests && /^ [^[:space:]]/ { in_tests=0 } | ||
| in_tests && /runs-on: warp-macos-15-arm64-6x/ { saw_warp=1 } | ||
| in_tests && /github.event.pull_request.head.repo.full_name == github.repository/ { saw_guard=1 } | ||
| END { exit !(saw_warp && saw_guard) } | ||
| END { exit !(saw_warp) } | ||
| ' "$WORKFLOW_FILE"; then | ||
| echo "FAIL: ui-regressions block must keep both warp-macos-15-arm64-6x runner and fork guard" | ||
| echo "FAIL: ui-regressions block must use warp-macos-15-arm64-6x runner" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "PASS: tests WarpBuild runner fork guard is present" | ||
| echo "PASS: tests-build-and-lag WarpBuild runner fork guard is present" | ||
| echo "PASS: ui-regressions WarpBuild runner fork guard is present" | ||
| echo "PASS: tests WarpBuild runner is present" | ||
| echo "PASS: tests-build-and-lag WarpBuild runner is present" | ||
| echo "PASS: ui-regressions WarpBuild runner is present" |
There was a problem hiding this comment.
The script validates WarpBuild runner usage for tests, tests-build-and-lag, and ui-regressions in ci.yml, but the PR also removes fork guards from build-ghosttykit.yml (build-ghosttykit job) and ci-macos-compat.yml (compat-tests job). Neither of those workflows is checked here.
If someone later accidentally swaps those jobs off the WarpBuild runner (e.g. to ubuntu-latest) the test won't catch it. Consider adding analogous awk checks against build-ghosttykit.yml and ci-macos-compat.yml, similar to what already exists for the three ci.yml jobs:
# build-ghosttykit: must use WarpBuild runner (paid runner)
GHOSTTYKIT_WORKFLOW="$ROOT_DIR/.github/workflows/build-ghosttykit.yml"
if ! awk '
/^ build-ghosttykit:/ { in_job=1; next }
in_job && /^ [^[:space:]]/ { in_job=0 }
in_job && /runs-on: warp-macos-15-arm64-6x/ { saw_warp=1 }
END { exit !(saw_warp) }
' "$GHOSTTYKIT_WORKFLOW"; then
echo "FAIL: build-ghosttykit block must use warp-macos-15-arm64-6x runner"
exit 1
fi
echo "PASS: build-ghosttykit WarpBuild runner is present"![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a84165666
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| build-ghosttykit: | ||
| # Never run WarpBuild jobs for fork pull requests (avoid billing on external PRs). | ||
| if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository | ||
| runs-on: warp-macos-15-arm64-6x |
There was a problem hiding this comment.
Without the removed job-level if on build-ghosttykit, fork pull_request runs can now reach the upload path, but Upload xcframework release authenticates with secrets.GHOSTTY_RELEASE_TOKEN, which is not provided to fork PR workflows. In the case where a fork PR updates ghostty to a SHA that does not already have an xcframework-* release, check-release sets exists=false and the job fails at publish time, making those PRs unable to pass CI until a maintainer reruns from a trusted branch.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_ci_self_hosted_guard.sh`:
- Around line 9-10: The test currently hardcodes
WORKFLOW_FILE="$ROOT_DIR/.github/workflows/ci.yml", which narrows scope to a
single workflow; change it to iterate over an array (e.g.,
WORKFLOW_FILES=("ci.yml" "build-ghosttykit.yml" "compat-tests.yml") or gather
all .github/workflows/*.yml) and loop through each file, running the same
validation logic for each; update the validation to assert that the relevant
jobs (names like build-ghosttykit and compat-tests) reference the
WarpBuild/warpbuild runner or expected runner label, and remove the duplicated
per-file blocks (the repeated logic in the region currently duplicated around
lines 11-42) by extracting the check into a single loop or helper function so
each workflow is validated consistently.
- Around line 4-5: Restore explicit fork-PR guards for jobs that use
paid/self-hosted runners by adding the condition check used by GitHub Actions
(e.g., if: github.event.pull_request.head.repo.full_name == github.repository)
to the jobs invoked from tests/test_ci_self_hosted_guard.sh (the jobs that
currently only check runner presence around the checks referenced in the
comment), or alternatively add a short documented justification in the same file
explaining why relying solely on the repository-level "Require approval for
outside collaborators" is acceptable; ensure you update the job definitions that
trigger expensive WarpBuilds to include the guard expression so fork PRs cannot
run those runners unless the PR is from the same repo.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28ce61a4-a6ec-4bce-a76d-73bc4017cda0
📒 Files selected for processing (4)
.github/workflows/build-ghosttykit.yml.github/workflows/ci-macos-compat.yml.github/workflows/ci.ymltests/test_ci_self_hosted_guard.sh
💤 Files with no reviewable changes (3)
- .github/workflows/ci-macos-compat.yml
- .github/workflows/build-ghosttykit.yml
- .github/workflows/ci.yml
| # Fork PRs are gated by GitHub's built-in "Require approval for outside | ||
| # collaborators" setting, so workflow-level fork guards are not needed. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="$(gh repo view --json nameWithOwner -q .nameWithOwner)"
echo "Repo: $repo"
echo "== Fork PR contributor approval setting =="
gh api "repos/$repo/actions/permissions/fork-pr-contributor-approval" | jq
echo
echo "== WarpBuild/self-hosted jobs without explicit fork guard =="
fd -e yml -e yaml . .github/workflows | while read -r f; do
awk '
BEGIN {in_jobs=0; job=""; has_warp=0; has_guard=0}
/^jobs:/ {in_jobs=1; next}
in_jobs && /^[^[:space:]]/ {in_jobs=0}
in_jobs && /^ [A-Za-z0-9_-]+:/ {
if (job != "" && has_warp && !has_guard) print FILENAME ": " job
job=$1; sub(":","",job); has_warp=0; has_guard=0; next
}
in_jobs && job != "" && /runs-on: .*warp-/ {has_warp=1}
in_jobs && job != "" && /^ if:/ && /github\.event\.pull_request\.head\.repo\.full_name == github\.repository/ {has_guard=1}
END { if (job != "" && has_warp && !has_guard) print FILENAME ": " job }
' "$f"
doneRepository: manaflow-ai/cmux
Length of output: 158
🏁 Script executed:
cat -n tests/test_ci_self_hosted_guard.shRepository: manaflow-ai/cmux
Length of output: 2018
🏁 Script executed:
# Check the related workflow files mentioned in the PR objective
fd -e yml -e yaml . .github/workflows | head -20Repository: manaflow-ai/cmux
Length of output: 352
🏁 Script executed:
# Look at the git history to understand what changed
git log --oneline tests/test_ci_self_hosted_guard.sh | head -10Repository: manaflow-ai/cmux
Length of output: 107
🏁 Script executed:
git diff 7a84165~1 7a84165 tests/test_ci_self_hosted_guard.sh | head -100Repository: manaflow-ai/cmux
Length of output: 249
🏁 Script executed:
git diff 7a84165~1 7a84165 .github/workflows/ | head -200Repository: manaflow-ai/cmux
Length of output: 249
🏁 Script executed:
# Check if any fork guards remain in the actual workflows
rg "github\.event\.pull_request\.head\.repo\.full_name|github\.repository" .github/workflows/ -A 2 -B 2Repository: manaflow-ai/cmux
Length of output: 822
🏁 Script executed:
# Check the main CI workflow file
cat -n .github/workflows/ci.yml | head -100Repository: manaflow-ai/cmux
Length of output: 3704
🏁 Script executed:
# Look for any conditions related to forks or PRs in the workflow files
rg "if:" .github/workflows/ci.yml -A 1 -B 1 | head -50Repository: manaflow-ai/cmux
Length of output: 499
🏁 Script executed:
# Check the git log to see recent changes to these files
git log --oneline .github/workflows/ci.yml | head -10Repository: manaflow-ai/cmux
Length of output: 107
🏁 Script executed:
# Check if there are any remaining guard patterns or fork-checking logic in other test files
rg "fork|pull_request" tests/*.sh -lRepository: manaflow-ai/cmux
Length of output: 94
🏁 Script executed:
# Look at the specific issue `#385` referenced in the test file comment
git log --all --oneline | rg "385|fork.*guard" | head -5Repository: manaflow-ai/cmux
Length of output: 339
🏁 Script executed:
# Check the other workflows mentioned in the PR objective
cat -n .github/workflows/ci-macos-compat.yml | grep -E "^[[:space:]]+(tests|runs-on)" | head -20Repository: manaflow-ai/cmux
Length of output: 42
Removing hardcoded fork guards weakens defense-in-depth when you depend on mutable repository settings.
Lines 4-5 justify the removal based on GitHub's built-in fork PR approval setting, but lines 16, 27, and 38 no longer enforce any fork-related constraints—only runner presence. This shifts security enforcement from versioned workflow code to a repository configuration that maintainers could accidentally disable.
If the approval setting is misconfigured or disabled, fork pull requests will run expensive WarpBuild jobs without additional safeguards. Restore explicit fork guards (e.g., if: github.event.pull_request.head.repo.full_name == github.repository) in jobs that use paid runners, or document why GitHub's approval setting is sufficient for this use case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_ci_self_hosted_guard.sh` around lines 4 - 5, Restore explicit
fork-PR guards for jobs that use paid/self-hosted runners by adding the
condition check used by GitHub Actions (e.g., if:
github.event.pull_request.head.repo.full_name == github.repository) to the jobs
invoked from tests/test_ci_self_hosted_guard.sh (the jobs that currently only
check runner presence around the checks referenced in the comment), or
alternatively add a short documented justification in the same file explaining
why relying solely on the repository-level "Require approval for outside
collaborators" is acceptable; ensure you update the job definitions that trigger
expensive WarpBuilds to include the guard expression so fork PRs cannot run
those runners unless the PR is from the same repo.
- Guard test now covers build-ghosttykit.yml and ci-macos-compat.yml (not just ci.yml) - Skip xcframework upload when GHOSTTY_RELEASE_TOKEN is unavailable (fork PRs), so the build still validates without failing at publish
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-ghosttykit.yml:
- Line 91: The conditional uses the secrets context directly (if:
steps.check-release.outputs.exists == 'false' && secrets.GHOSTTY_RELEASE_TOKEN
!= ''), which is invalid; instead, export the secret into an environment
variable at the job or workflow level (e.g., set env.GHOSTTY_RELEASE_TOKEN = ${{
secrets.GHOSTTY_RELEASE_TOKEN }}), then change the if expression to reference
env.GHOSTTY_RELEASE_TOKEN (for example: if: steps.check-release.outputs.exists
== 'false' && env.GHOSTTY_RELEASE_TOKEN != ''), keeping the rest of the
condition intact so the step checks the output from steps.check-release and the
presence of the token via env.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5831ba90-a5a9-4f5f-964b-dfe529b0a589
📒 Files selected for processing (2)
.github/workflows/build-ghosttykit.ymltests/test_ci_self_hosted_guard.sh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 742d4ccbb2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tests: | ||
| # Never run WarpBuild jobs for fork pull requests (avoid billing on external PRs). | ||
| if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository | ||
| runs-on: warp-macos-15-arm64-6x |
There was a problem hiding this comment.
Removing the fork-PR guard from this WarpBuild job makes fork PRs execute ./scripts/download-prebuilt-ghosttykit.sh, but those PRs still cannot publish missing GhosttyKit releases because build-ghosttykit now skips upload when GHOSTTY_RELEASE_TOKEN is unavailable. As a result, any fork PR that updates ghostty to a SHA without an existing xcframework-* release will deterministically fail these CI jobs (and the same pattern in ci-macos-compat.yml), so contributors cannot get a green run for that class of changes.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
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/build-ghosttykit.yml">
<violation number="1" location=".github/workflows/build-ghosttykit.yml:91">
P2: `secrets.*` is used directly in a step `if`, which GitHub Actions does not support for conditionals. Gate on an `env` value instead.</violation>
</file>
<file name="tests/test_ci_self_hosted_guard.sh">
<violation number="1" location="tests/test_ci_self_hosted_guard.sh:18">
P2: The runner guard can false-pass matrix jobs because it accepts any `os: warp-macos...` line without verifying that `runs-on` actually uses `matrix.os`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if ! awk -v job="$job" ' | ||
| $0 ~ "^ "job":" { in_job=1; next } | ||
| in_job && /^ [^[:space:]]/ { in_job=0 } | ||
| in_job && /runs-on:.*warp-macos-.*-arm64/ { saw_warp=1 } |
There was a problem hiding this comment.
P2: The runner guard can false-pass matrix jobs because it accepts any os: warp-macos... line without verifying that runs-on actually uses matrix.os.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_ci_self_hosted_guard.sh, line 18:
<comment>The runner guard can false-pass matrix jobs because it accepts any `os: warp-macos...` line without verifying that `runs-on` actually uses `matrix.os`.</comment>
<file context>
@@ -6,41 +6,32 @@
+ if ! awk -v job="$job" '
+ $0 ~ "^ "job":" { in_job=1; next }
+ in_job && /^ [^[:space:]]/ { in_job=0 }
+ in_job && /runs-on:.*warp-macos-.*-arm64/ { saw_warp=1 }
+ in_job && /os: warp-macos-.*-arm64/ { saw_warp=1 }
+ END { exit !(saw_warp) }
</file context>
secrets context can't be reliably used in step if: conditions. Check the env var inside the script instead.
* Remove fork PR guards from CI workflows Fork PRs are already gated by GitHub's "Require approval for outside collaborators" setting. The workflow-level guards were redundant and prevented WarpBuild jobs from running even after maintainer approval. * Address review feedback: extend guard test, skip upload on fork PRs - Guard test now covers build-ghosttykit.yml and ci-macos-compat.yml (not just ci.yml) - Skip xcframework upload when GHOSTTY_RELEASE_TOKEN is unavailable (fork PRs), so the build still validates without failing at publish * Check GHOSTTY_RELEASE_TOKEN at runtime instead of step if secrets context can't be reliably used in step if: conditions. Check the env var inside the script instead. --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
1 participant
Summary
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repositoryguards from all WarpBuild jobs (tests,tests-build-and-lag,ui-regressions,build-ghosttykit,compat-tests)Test plan
workflow-guard-testspasses (the updatedtest_ci_self_hosted_guard.sh)Summary by cubic
Removed fork PR guards from CI so WarpBuild jobs run on approved fork PRs. We now rely on GitHub’s “Require approval for outside collaborators,” and skip GhosttyKit uploads on forks when no token is present.
Refactors
ifguards from WarpBuild jobs inci.yml(tests, tests-build-and-lag, ui-regressions),build-ghosttykit.yml, andci-macos-compat.yml.tests/test_ci_self_hosted_guard.shto verify WarpBuild runner usage across all three workflows.Bug Fixes
GHOSTTY_RELEASE_TOKENat runtime in the GhosttyKit upload step and skip when missing to avoid fork PR failures.Written for commit 334fd7d. Summary will update on new commits.
Summary by CodeRabbit