Repository navigation
Skip zig Ghostty helper builds in CI - #2416
lawrencecchen wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdded a new CI workflow guard test to validate that the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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: c23f1fc2cd
ℹ️ 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".
| env: | ||
| CMUX_SKIP_ZIG_BUILD: 1 |
There was a problem hiding this comment.
Setting CMUX_SKIP_ZIG_BUILD: 1 at the tests job level also affects the xcodebuild invocation inside tests/test_bundled_ghostty_theme_picker_helper.sh. In scripts/build-ghostty-cli-helper.sh, that env path emits a stub bin/ghostty that exits with failure, so the “bundled Ghostty theme picker helper regression” now only verifies executable presence of a stub and can no longer catch a broken runtime helper. This weakens an existing regression check and can let helper breakage pass CI unnoticed.
Useful? React with 👍 / 👎.
| in_job && $0 ~ /CMUX_SKIP_ZIG_BUILD: 1/ { | ||
| found = 1 |
There was a problem hiding this comment.
This guard test only scans .github/workflows/ci.yml text for CMUX_SKIP_ZIG_BUILD: 1 via awk, so it is a configuration-string assertion rather than a behavioral check of the CI outcome. That means it can still pass while the actual macOS build path regresses (for example, if a step overrides the env and zig helper builds run again), which creates false confidence in the guard. A runtime/artifact-level assertion would make this regression test reliable.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR fixes a CI failure caused by the zig 0.15.2 MachO linker's inability to resolve
Confidence Score: 5/5Safe to merge — the change is targeted, well-understood, and addresses a confirmed CI breakage; remaining findings are P2 cleanup. All findings are P2 (unused zig install steps, test policy note). The core fix is correct, consistent with the approach already used in ci-macos-compat.yml, and the downstream stub logic in build-ghostty-cli-helper.sh is already validated. No files require special attention; both changed files are low-risk CI config and a new guard script. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CI macOS job starts
tests / tests-build-and-lag / ui-regressions] --> B[env: CMUX_SKIP_ZIG_BUILD=1]
B --> C[Cache / Download
GhosttyKit.xcframework]
C --> D[Install zig step
zig downloaded but unused]
D --> E[xcodebuild invokes
build-ghostty-cli-helper.sh]
E --> F{CMUX_SKIP_ZIG_BUILD == 1?}
F -- Yes --> G[Write stub binary
chmod +x, exit 0]
F -- No --> H[Run: zig build cli-helper]
G --> I[xcodebuild continues
Run Script check passes]
H --> I
I --> J[Run tests / lag / UI regressions]
subgraph guard [workflow-guard-tests job on Ubuntu]
K[test_ci_skip_zig_build_guard.sh] --> L[awk scan ci.yml
for each job]
L --> M{CMUX_SKIP_ZIG_BUILD: 1
found in job block?}
M -- No --> N[FAIL: exit 1]
M -- Yes --> O[PASS]
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #!/usr/bin/env bash | ||
| # Regression test to keep CI macOS jobs from live-building the Ghostty helper. | ||
| set -euo pipefail | ||
|
|
||
| ROOT_DIR="$(cd "$(dirname "$0")/.." && pwd)" | ||
| WORKFLOW_FILE="$ROOT_DIR/.github/workflows/ci.yml" | ||
|
|
||
| check_job() { | ||
| local job_name="$1" | ||
| if ! awk -v job_name="$job_name" ' | ||
| $0 == " " job_name ":" { | ||
| in_job = 1 | ||
| next | ||
| } | ||
| in_job && $0 ~ /^ [^ ]/ { | ||
| exit found ? 0 : 1 | ||
| } | ||
| in_job && $0 ~ /CMUX_SKIP_ZIG_BUILD: 1/ { | ||
| found = 1 | ||
| } | ||
| END { | ||
| if (!in_job || !found) { | ||
| exit 1 | ||
| } | ||
| } | ||
| ' "$WORKFLOW_FILE"; then | ||
| echo "FAIL: $job_name in ci.yml must set CMUX_SKIP_ZIG_BUILD: 1" >&2 | ||
| exit 1 | ||
| fi | ||
| } | ||
|
|
||
| check_job "tests" | ||
| check_job "tests-build-and-lag" | ||
| check_job "ui-regressions" | ||
|
|
||
| echo "PASS: CI macOS jobs skip zig Ghostty helper builds" |
There was a problem hiding this comment.
CLAUDE.md explicitly prohibits tests that read checked-in metadata/config files to assert key-value patterns:
"Do not add tests that read checked-in metadata or project files such as
Resources/Info.plist,project.pbxproj,.xcconfig, or source files only to assert that a key, string, plist entry, or snippet exists."
This script opens .github/workflows/ci.yml and uses awk to grep for CMUX_SKIP_ZIG_BUILD: 1 — precisely the pattern the policy forbids. The policy recommends: "If no meaningful behavioral or artifact-level test is practical, skip the fake regression test and state that explicitly."
That said, the repository already has several guard tests (test_ci_self_hosted_guard.sh, test_ci_ghosttykit_checksum_verification.sh, etc.) that follow the same pattern, so this PR is consistent with established practice even if it conflicts with the stated policy. Worth a team discussion on whether to grandfather the guard-test pattern or align with the documented policy.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@tests/test_ci_skip_zig_build_guard.sh`:
- Around line 10-25: The awk job-block matcher currently treats any occurrence
of "CMUX_SKIP_ZIG_BUILD: 1" inside the job block as valid; change the logic in
the awk script (the block that uses job_name, in_job, found) to also track an
in_env state: set in_env when you encounter the job-level "env:" line (correct
indentation relative to the job header), only set found when in_env is true and
the line matches "CMUX_SKIP_ZIG_BUILD: 1", and clear/exit the in_env state when
you hit the next same-or-less-indented job-level key so that matches in step
commands or comments are ignored. Ensure the termination condition still
requires in_job && found to pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4604a18c-668c-4675-9ba7-1c5debf071f8
📒 Files selected for processing (2)
.github/workflows/ci.ymltests/test_ci_skip_zig_build_guard.sh
| if ! awk -v job_name="$job_name" ' | ||
| $0 == " " job_name ":" { | ||
| in_job = 1 | ||
| next | ||
| } | ||
| in_job && $0 ~ /^ [^ ]/ { | ||
| exit found ? 0 : 1 | ||
| } | ||
| in_job && $0 ~ /CMUX_SKIP_ZIG_BUILD: 1/ { | ||
| found = 1 | ||
| } | ||
| END { | ||
| if (!in_job || !found) { | ||
| exit 1 | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat >/tmp/ci_guard_false_positive.yml <<'YAML'
jobs:
tests:
runs-on: macos-latest
steps:
- run: echo "CMUX_SKIP_ZIG_BUILD: 1"
next-job:
runs-on: ubuntu-latest
YAML
echo "Current matcher (expected: fail, actual: passes -> bug):"
set +e
awk -v job_name="tests" '
$0 == " " job_name ":" { in_job = 1; next }
in_job && $0 ~ /^ [^ ]/ { exit found ? 0 : 1 }
in_job && $0 ~ /CMUX_SKIP_ZIG_BUILD: 1/ { found = 1 }
END { if (!in_job || !found) exit 1 }
' /tmp/ci_guard_false_positive.yml
echo "exit_code=$?"
set -eRepository: manaflow-ai/cmux
Length of output: 128
🏁 Script executed:
# Find the test file and examine it
fd "test_ci_skip_zig_build_guard.sh" -t fRepository: manaflow-ai/cmux
Length of output: 97
🏁 Script executed:
# If the file exists, read it
if [ -f "tests/test_ci_skip_zig_build_guard.sh" ]; then
wc -l "tests/test_ci_skip_zig_build_guard.sh"
echo "---"
cat -n "tests/test_ci_skip_zig_build_guard.sh"
else
echo "File not found"
fiRepository: manaflow-ai/cmux
Length of output: 1163
🏁 Script executed:
# List files in tests directory to understand the structure
ls -la tests/ 2>/dev/null || echo "tests directory not found"Repository: manaflow-ai/cmux
Length of output: 8497
🏁 Script executed:
# Find and check the CI workflow file
ls -la .github/workflows/ci.yml
wc -l .github/workflows/ci.ymlRepository: manaflow-ai/cmux
Length of output: 163
🏁 Script executed:
# Read the CI workflow file to understand the job structure
head -100 .github/workflows/ci.ymlRepository: manaflow-ai/cmux
Length of output: 2991
🏁 Script executed:
# Look specifically for the "tests" job and how env is structured
grep -A 30 "^ tests:" .github/workflows/ci.yml | head -40Repository: manaflow-ai/cmux
Length of output: 1077
🏁 Script executed:
# Check the other jobs mentioned in the test
grep -A 15 "^ tests-build-and-lag:" .github/workflows/ci.yml
echo "---"
grep -A 15 "^ ui-regressions:" .github/workflows/ci.ymlRepository: manaflow-ai/cmux
Length of output: 1144
🏁 Script executed:
# Now test the proposed fix pattern to ensure it correctly identifies env section
cat > /tmp/test_proposed_fix.sh <<'EOF'
#!/bin/bash
cat >/tmp/ci_test.yml <<'YAML'
jobs:
tests:
runs-on: macos-latest
env:
CMUX_SKIP_ZIG_BUILD: 1
steps:
- run: echo "CMUX_SKIP_ZIG_BUILD: 1"
tests-build-and-lag:
runs-on: macos-latest
env:
CMUX_SKIP_ZIG_BUILD: 1
steps:
- run: echo "something"
ui-regressions:
runs-on: macos-latest
env:
CMUX_SKIP_ZIG_BUILD: 1
steps:
- run: echo "something"
bad-job:
runs-on: macos-latest
steps:
- run: echo "CMUX_SKIP_ZIG_BUILD: 1"
YAML
# Test the proposed awk pattern for each job
for job in "tests" "tests-build-and-lag" "ui-regressions" "bad-job"; do
echo "Testing job: $job"
set +e
awk -v job_name="$job" '
$0 == " " job_name ":" {
in_job = 1
in_env = 0
found = 0
next
}
in_job && $0 ~ /^ [^ ]/ {
exit found ? 0 : 1
}
in_job && $0 ~ /^ env:[[:space:]]*$/ {
in_env = 1
next
}
in_job && in_env && $0 ~ /^ [^ ]/ {
in_env = 0
}
in_job && in_env && $0 ~ /^ CMUX_SKIP_ZIG_BUILD:[[:space:]]*"?1"?[[:space:]]*$/ {
found = 1
}
END {
if (!in_job || !found) {
exit 1
}
}
' /tmp/ci_test.yml
result=$?
echo "exit_code=$result"
set -e
echo ""
done
EOF
bash /tmp/test_proposed_fix.shRepository: manaflow-ai/cmux
Length of output: 212
🏁 Script executed:
# Test the CURRENT broken pattern to confirm it produces false positives
cat > /tmp/test_current_broken.sh <<'EOF'
#!/bin/bash
cat >/tmp/ci_test.yml <<'YAML'
jobs:
tests:
runs-on: macos-latest
env:
CMUX_SKIP_ZIG_BUILD: 1
steps:
- run: echo "CMUX_SKIP_ZIG_BUILD: 1"
tests-build-and-lag:
runs-on: macos-latest
env:
CMUX_SKIP_ZIG_BUILD: 1
steps:
- run: echo "something"
ui-regressions:
runs-on: macos-latest
env:
CMUX_SKIP_ZIG_BUILD: 1
steps:
- run: echo "something"
bad-job:
runs-on: macos-latest
steps:
- run: echo "CMUX_SKIP_ZIG_BUILD: 1"
YAML
# Test the CURRENT broken awk pattern for each job
for job in "tests" "tests-build-and-lag" "ui-regressions" "bad-job"; do
echo "Testing job: $job (CURRENT broken pattern)"
set +e
awk -v job_name="$job" '
$0 == " " job_name ":" { in_job = 1; next }
in_job && $0 ~ /^ [^ ]/ { exit found ? 0 : 1 }
in_job && $0 ~ /CMUX_SKIP_ZIG_BUILD: 1/ { found = 1 }
END { if (!in_job || !found) exit 1 }
' /tmp/ci_test.yml
result=$?
echo "exit_code=$result"
set -e
echo ""
done
EOF
bash /tmp/test_current_broken.shRepository: manaflow-ai/cmux
Length of output: 312
Constrain the awk check to job-level env to avoid false passes.
The current matcher accepts CMUX_SKIP_ZIG_BUILD: 1 anywhere in the job block, not just in the env section. This allows the guard to pass even when the environment variable is not actually set at the job level—for example, if the string appears in a step command or comment. Testing confirms the current pattern incorrectly passes for a job with the variable only in a step command, where it should fail.
The proposed fix constrains the pattern to match only within the env section using state tracking (in_env flag) and proper indentation checks to ensure CMUX_SKIP_ZIG_BUILD: 1 is only accepted as a job-level environment variable.
🔧 Proposed fix
check_job() {
local job_name="$1"
if ! awk -v job_name="$job_name" '
$0 == " " job_name ":" {
in_job = 1
+ in_env = 0
+ found = 0
next
}
in_job && $0 ~ /^ [^ ]/ {
exit found ? 0 : 1
}
- in_job && $0 ~ /CMUX_SKIP_ZIG_BUILD: 1/ {
+ in_job && $0 ~ /^ env:[[:space:]]*$/ {
+ in_env = 1
+ next
+ }
+ in_job && in_env && $0 ~ /^ [^ ]/ {
+ in_env = 0
+ }
+ in_job && in_env && $0 ~ /^ CMUX_SKIP_ZIG_BUILD:[[:space:]]*"?1"?[[:space:]]*$/ {
found = 1
}
END {
if (!in_job || !found) {
exit 1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_ci_skip_zig_build_guard.sh` around lines 10 - 25, The awk
job-block matcher currently treats any occurrence of "CMUX_SKIP_ZIG_BUILD: 1"
inside the job block as valid; change the logic in the awk script (the block
that uses job_name, in_job, found) to also track an in_env state: set in_env
when you encounter the job-level "env:" line (correct indentation relative to
the job header), only set found when in_env is true and the line matches
"CMUX_SKIP_ZIG_BUILD: 1", and clear/exit the in_env state when you hit the next
same-or-less-indented job-level key so that matches in step commands or comments
are ignored. Ensure the termination condition still requires in_job && found to
pass.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
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="tests/test_ci_skip_zig_build_guard.sh">
<violation number="1" location="tests/test_ci_skip_zig_build_guard.sh:6">
P2: This test validates CI behavior by grepping workflow YAML text (`ci.yml`) instead of exercising runtime behavior, which violates the repository’s test-quality policy and creates brittle failures on non-behavioral workflow edits.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| set -euo pipefail | ||
|
|
||
| ROOT_DIR="$(cd "$(dirname "$0")/.." && pwd)" | ||
| WORKFLOW_FILE="$ROOT_DIR/.github/workflows/ci.yml" |
There was a problem hiding this comment.
P2: This test validates CI behavior by grepping workflow YAML text (ci.yml) instead of exercising runtime behavior, which violates the repository’s test-quality policy and creates brittle failures on non-behavioral workflow edits.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_ci_skip_zig_build_guard.sh, line 6:
<comment>This test validates CI behavior by grepping workflow YAML text (`ci.yml`) instead of exercising runtime behavior, which violates the repository’s test-quality policy and creates brittle failures on non-behavioral workflow edits.</comment>
<file context>
@@ -0,0 +1,36 @@
+set -euo pipefail
+
+ROOT_DIR="$(cd "$(dirname "$0")/.." && pwd)"
+WORKFLOW_FILE="$ROOT_DIR/.github/workflows/ci.yml"
+
+check_job() {
</file context>
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
2 participants
Fixes the current main CI failure from https://github.com/manaflow-ai/cmux/actions/runs/23787431354.
This sets
CMUX_SKIP_ZIG_BUILDon the macOS CI jobs that already download the prebuiltGhosttyKit.xcframework, and adds a workflow guard test so the skip does not regress.Validation:
./tests/test_ci_skip_zig_build_guard.sh./tests/test_ci_ghosttykit_checksum_verification.shCMUX_SKIP_ZIG_BUILD=1xcodebuild ... build-for-testingSummary by cubic
Skip building the Zig Ghostty helper on macOS CI by setting CMUX_SKIP_ZIG_BUILD=1 for jobs that use prebuilt
GhosttyKit, fixing the current main CI failure. Adds a guard test to ensure this skip stays in place.Written for commit c23f1fc. Summary will update on new commits.
Summary by CodeRabbit