Repository navigation
Fix self-hosted CI compatibility failures - #6289
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesNightly prune Python compatibility guard
App-host XCTest environment isolation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 21✅ Passed checks (21 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 |
Greptile SummaryThis PR fixes two self-hosted CI failure modes: older macOS runners on Python 3.9 failing to import the nightly prune script due to missing
Confidence Score: 5/5Safe to merge — all changes are additive CI scaffolding or a well-tested extraction of existing debug-socket resolution logic with a targeted XCTest isolation branch. The Swift change is a clean extraction plus a narrowly-scoped XCTest detection branch backed by five new unit tests. The Python change is a one-line import guard. The CI script changes add a wrapper with explicit failure gates. No production release-build paths are touched, and the new defaultSocketPath branch is gated on both isDebugBuild and the bare-debug bundle identifier, so stable builds are unaffected. scripts/ci/run-app-host-xcodebuild.sh — retry path uses pkill without waiting for process exit; SocketControlSettings+DefaultSocketPath.swift — the DYLD_INSERT_LIBRARIES-only fallback provides no per-session isolation when higher-priority XCTest env vars are absent. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[socketPath called] --> B[defaultSocketPath]
B --> C{isDebugBuild AND bare debug bundle ID?}
C -- No --> G[SocketPathMarkerFiles.defaultSocketPath]
C -- Yes --> D{launchTag == nil AND no CMUX_SOCKET_PATH?}
D -- No --> G
D -- Yes --> E{XCTest env vars present?}
E -- No --> G
E -- Yes --> F[FNV-1a hash first indicator → /tmp/cmux-xctest-hash.sock]
G --> H[stableSocketPath / debug / tagged path]
F --> I[Return xctest-scoped path]
H --> J{CMUX_SOCKET_PATH override set?}
J -- No --> K[Return fallback]
J -- Yes --> L{shouldHonorOverride?}
L -- Yes --> M[Return override]
L -- No --> K
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[socketPath called] --> B[defaultSocketPath]
B --> C{isDebugBuild AND bare debug bundle ID?}
C -- No --> G[SocketPathMarkerFiles.defaultSocketPath]
C -- Yes --> D{launchTag == nil AND no CMUX_SOCKET_PATH?}
D -- No --> G
D -- Yes --> E{XCTest env vars present?}
E -- No --> G
E -- Yes --> F[FNV-1a hash first indicator → /tmp/cmux-xctest-hash.sock]
G --> H[stableSocketPath / debug / tagged path]
F --> I[Return xctest-scoped path]
H --> J{CMUX_SOCKET_PATH override set?}
J -- No --> K[Return fallback]
J -- Yes --> L{shouldHonorOverride?}
L -- Yes --> M[Return override]
L -- No --> K
Reviews (9): Last reviewed commit: "ci: harden self-hosted CI compatibility" | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 34-35: Add a step using the `actions/setup-python` action with
`python-version: '3.9'` before the "Validate nightly prune Python compatibility"
step to ensure the test_ci_nightly_prune_python_compat.sh script runs under
Python 3.9 interpreter. This will provide genuine runtime compatibility
verification for Python 3.9 rather than relying on pattern-based checks alone.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 124f790a-a687-4097-8796-0252adaf965c
📒 Files selected for processing (3)
.github/workflows/ci.ymlscripts/prune_nightly_release_assets.pytests/test_ci_nightly_prune_python_compat.sh
a31e167 to
94c4018
Compare
94c4018 to
61d0604
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_ci_app_host_xctest_env.sh`:
- Around line 26-31: The grep -Fq check in the loop iterating through
REQUIRED_STEPS array does not exclude commented-out lines, allowing the
validation to pass even if the required step is only present in a comment (e.g.,
`# source scripts/ci/app-host-xctest-env.sh`). Modify the grep command to
exclude lines that are commented out by adding a pattern that filters for only
uncommented matches, ensuring the validation only passes when the required step
is actually active in the workflow file.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9253c9f4-eef2-41c9-a07c-22b26dc83fcf
📒 Files selected for processing (5)
.github/workflows/ci.ymlscripts/ci/app-host-xctest-env.shscripts/prune_nightly_release_assets.pytests/test_ci_app_host_xctest_env.shtests/test_ci_nightly_prune_python_compat.sh
| for step in "${REQUIRED_STEPS[@]}"; do | ||
| if ! grep -Fq "$step" "$WORKFLOW_FILE"; then | ||
| echo "FAIL: missing isolated app-host env in ci.yml: $step" >&2 | ||
| exit 1 | ||
| fi | ||
| done |
There was a problem hiding this comment.
Guard can false-pass on commented workflow lines.
grep -Fq matches raw substrings, so a commented # source scripts/ci/app-host-xctest-env.sh ... would still satisfy this check. That weakens the regression gate.
Proposed fix
for step in "${REQUIRED_STEPS[@]}"; do
- if ! grep -Fq "$step" "$WORKFLOW_FILE"; then
+ if ! awk -v s="$step" '
+ /^[[:space:]]*`#/` { next } # ignore comments
+ index($0, s) { found=1 }
+ END { exit(found ? 0 : 1) }
+ ' "$WORKFLOW_FILE"; then
echo "FAIL: missing isolated app-host env in ci.yml: $step" >&2
exit 1
fi
done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_ci_app_host_xctest_env.sh` around lines 26 - 31, The grep -Fq
check in the loop iterating through REQUIRED_STEPS array does not exclude
commented-out lines, allowing the validation to pass even if the required step
is only present in a comment (e.g., `# source
scripts/ci/app-host-xctest-env.sh`). Modify the grep command to exclude lines
that are commented out by adding a pattern that filters for only uncommented
matches, ensuring the validation only passes when the required step is actually
active in the workflow file.
61d0604 to
3cb8a8c
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/test_ci_app_host_xctest_env.sh (1)
28-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStrengthen workflow validation to exclude commented lines.
The
grep -Fqcheck will match the required step even if it appears only in a comment (e.g.,# source scripts/ci/app-host-xctest-env.sh split-theme), which would false-pass the regression gate. Although the current workflow has these as active commands, the test should be robust against accidental commenting.🔒 Proposed fix to exclude commented workflow lines
for step in "${REQUIRED_STEPS[@]}"; do - if ! grep -Fq "$step" "$WORKFLOW_FILE"; then + if ! grep -F "$step" "$WORKFLOW_FILE" | grep -qv '^\s*#'; then echo "FAIL: missing isolated app-host env in ci.yml: $step" >&2 exit 1 fi doneThis filters out lines starting with optional whitespace followed by
#before checking for matches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_ci_app_host_xctest_env.sh` around lines 28 - 33, The grep -Fq check in the loop over REQUIRED_STEPS will incorrectly match the step even if it appears only in a commented line (e.g., preceded by # characters), causing false-positive validation. Modify the grep command to exclude lines that start with optional whitespace followed by # before checking for the required step. You can accomplish this by piping the WORKFLOW_FILE through grep to filter out commented lines first, or by using a more complex grep pattern that ensures the step is not on a commented line.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@tests/test_ci_app_host_xctest_env.sh`:
- Around line 28-33: The grep -Fq check in the loop over REQUIRED_STEPS will
incorrectly match the step even if it appears only in a commented line (e.g.,
preceded by # characters), causing false-positive validation. Modify the grep
command to exclude lines that start with optional whitespace followed by #
before checking for the required step. You can accomplish this by piping the
WORKFLOW_FILE through grep to filter out commented lines first, or by using a
more complex grep pattern that ensures the step is not on a commented line.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1af31a0b-fdfa-4cd1-81fd-9033b606faaf
📒 Files selected for processing (5)
.github/workflows/ci.ymlscripts/ci/app-host-xctest-env.shscripts/prune_nightly_release_assets.pytests/test_ci_app_host_xctest_env.shtests/test_ci_nightly_prune_python_compat.sh
3cb8a8c to
8483087
Compare
e4da40d to
d9998a9
Compare
d9998a9 to
ce14d5d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ce14d5d. Configure here.
| python-version: "3.9" | ||
|
|
||
| - name: Validate nightly prune Python compatibility | ||
| run: PYTHON_BIN=python3.9 bash ./tests/test_ci_nightly_prune_python_compat.sh |
There was a problem hiding this comment.
Mid-job Python downgrades later steps
Low Severity
Adding actions/setup-python with 3.9 in the middle of workflow-guard-tests makes python3 point at 3.9 for every later step in that job, not only the nightly prune check. Steps that used to run on the runner’s default interpreter now run on 3.9 instead.
Reviewed by Cursor Bugbot for commit ce14d5d. Configure here.
b3eb9af to
8b7e19f
Compare
8b7e19f to
fcb7aa3
Compare


Summary
Tests
Note
Medium Risk
Socket default-path behavior changes for bare debug XCTest (isolation improvement); CI-only guards and a small Python compatibility tweak—low production blast radius but touches control-socket resolution used by tests and dev builds.
Overview
Fixes self-hosted CI failures from shared debug control sockets during app-host XCTest runs and from older runner Python importing the nightly prune script.
Socket control: Bare debug XCTest launches (no
CMUX_TAG, noCMUX_SOCKET_PATH) now default to/tmp/cmux-xctest-<hash>.sockinstead of/tmp/cmux-debug.sock, keyed off XCTest env indicators (includinglibXCTestinDYLD_INSERT_LIBRARIES). Tagged debug and explicit overrides are unchanged. Logic lives in a newSocketControlSettings+DefaultSocketPathextension with unit tests.CI: Focused app-host
xcodebuildsteps route throughscripts/ci/run-app-host-xcodebuild.sh, which fails the job if logs show the shared debug socket, requires listener evidence, and can retry on XCTest startup hangs. Workflow guard adds Python 3.9 andtest_ci_nightly_prune_python_compat.sh.Nightly prune:
from __future__ import annotationsonscripts/prune_nightly_release_assets.pyso 3.9 can import it.Reviewed by Cursor Bugbot for commit fcb7aa3. Bugbot is set up for automated code reviews on this repo. Configure here.