Skip to content

Skip zig Ghostty helper builds in CI #2416

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lawrencecchen wants to merge 1 commit into from
Closed

Skip zig Ghostty helper builds in CI #2416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ jobs:
- name: Validate unit-test SwiftPM retry guard
run: ./tests/test_ci_unit_test_spm_retry.sh

- name: Validate CI skips zig Ghostty helper builds on macOS
run: ./tests/test_ci_skip_zig_build_guard.sh

- name: Validate cmux scheme test configuration
run: ./tests/test_ci_scheme_testaction_debug.sh

Expand Down Expand Up @@ -77,6 +80,8 @@ jobs:
tests:
runs-on: warp-macos-15-arm64-6x
timeout-minutes: 30
env:
CMUX_SKIP_ZIG_BUILD: 1
Comment on lines +83 to +84

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Scope CMUX_SKIP_ZIG_BUILD away from helper regression build

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 👍 / 👎.

steps:
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
Expand Down Expand Up @@ -241,6 +246,8 @@ jobs:
# still run via test-e2e.yml on GitHub-hosted runners.
runs-on: warp-macos-15-arm64-6x
timeout-minutes: 20
env:
CMUX_SKIP_ZIG_BUILD: 1
steps:
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
Expand Down Expand Up @@ -403,6 +410,8 @@ jobs:
ui-regressions:
runs-on: warp-macos-15-arm64-6x
timeout-minutes: 25
env:
CMUX_SKIP_ZIG_BUILD: 1
steps:
- name: Checkout
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
Expand Down
36 changes: 36 additions & 0 deletions tests/test_ci_skip_zig_build_guard.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/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"

@cubic-dev-ai cubic-dev-ai Bot Mar 31, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Fix with Cubic


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
Comment on lines +18 to +19

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate zig-skip behavior instead of grepping workflow YAML

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 👍 / 👎.

}
END {
if (!in_job || !found) {
exit 1
}
}
Comment on lines +10 to +25

@coderabbitai coderabbitai Bot Mar 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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 -e

Repository: 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 f

Repository: 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"
fi

Repository: 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.yml

Repository: manaflow-ai/cmux

Length of output: 163

🏁 Script executed:

# Read the CI workflow file to understand the job structure
head -100 .github/workflows/ci.yml

Repository: 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 -40

Repository: 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.yml

Repository: 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.sh

Repository: 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.sh

Repository: 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.

' "$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"
Comment on lines +1 to +36

@greptile-apps greptile-apps Bot Mar 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test quality policy conflict

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!

Loading