Add macOS compatibility CI: unit tests + smoke test on macos-14/15 - #769
Conversation
New workflow runs on GitHub-hosted macos-14 and macos-15 runners (matrix strategy). Each run: unit tests via cmux-unit scheme, then a smoke test that builds the app, launches it, sends a command via the socket, and verifies it stays alive for 15 seconds.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds macOS compatibility testing infrastructure by introducing a GitHub Actions workflow that runs on macOS 14 and 15, downloading a pre-built GhosttyKit.xcframework with retry mechanisms, executing unit tests, and performing an app smoke test via socket communication validation. Changes
Possibly Related PRs
Poem
🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 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)
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: d3f41245e2
ℹ️ 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".
| s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) | ||
| s.connect('$SOCKET_PATH') | ||
| s.settimeout(5.0) | ||
| s.sendall(b'send time\\\n\n') |
There was a problem hiding this comment.
The Python bytes literal here sends send time\ followed by two real newlines, not send time\\n plus the command terminator. In Sources/TerminalController.swift (sendInput), only the literal sequence \n is converted to Enter, so this payload does not actually execute time in the terminal and the smoke test can pass without validating terminal command delivery.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/ci-macos-compat.yml (2)
24-42: Consider usingfindinstead oflsfor Xcode discovery.Static analysis flagged
ls -dusage. While Xcode app names are predictable, usingfindwould be more robust:🔧 Suggested improvement
- XCODE_APP="$(ls -d /Applications/Xcode*.app 2>/dev/null | head -n 1 || true)" + XCODE_APP="$(find /Applications -maxdepth 1 -name 'Xcode*.app' -print -quit 2>/dev/null || true)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci-macos-compat.yml around lines 24 - 42, Replace the fragile ls-based discovery in the "Select Xcode" step by using a find-based lookup to set XCODE_APP and then XCODE_DIR: locate the first /Applications/Xcode*.app using find (instead of the current XCODE_APP="$(ls -d /Applications/Xcode*.app ... )"), fall back to the existing /Applications/Xcode.app check, and then export DEVELOPER_DIR and write DEVELOPER_DIR="$XCODE_DIR" to GITHUB_ENV as before; update references to XCODE_APP, XCODE_DIR, and DEVELOPER_DIR accordingly so behavior remains identical but discovery uses find.
135-148: Virtual display PID is stored but not cleaned up.
VDISPLAY_PIDis saved toGITHUB_ENVbut never used for cleanup. This is fine for ephemeral CI runners, but if you want explicit cleanup, consider adding a post-step or cleanup in the smoke test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci-macos-compat.yml around lines 135 - 148, The step that builds and launches the helper binary (/tmp/create-virtual-display) saves the background process PID to VDISPLAY_PID and writes it to GITHUB_ENV but never stops the process; add explicit cleanup by capturing VDISPLAY_PID (from GITHUB_ENV) and killing the process in a subsequent step or a post-job step (e.g., a "Cleanup virtual display" step or a post-action) that uses kill (or pkill) against VDISPLAY_PID and removes the /tmp/create-virtual-display binary; reference the created binary name (/tmp/create-virtual-display), the environment variable VDISPLAY_PID, and the source scripts/create-virtual-display.m when locating where to add the complementary cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci-macos-compat.yml:
- Around line 24-42: Replace the fragile ls-based discovery in the "Select
Xcode" step by using a find-based lookup to set XCODE_APP and then XCODE_DIR:
locate the first /Applications/Xcode*.app using find (instead of the current
XCODE_APP="$(ls -d /Applications/Xcode*.app ... )"), fall back to the existing
/Applications/Xcode.app check, and then export DEVELOPER_DIR and write
DEVELOPER_DIR="$XCODE_DIR" to GITHUB_ENV as before; update references to
XCODE_APP, XCODE_DIR, and DEVELOPER_DIR accordingly so behavior remains
identical but discovery uses find.
- Around line 135-148: The step that builds and launches the helper binary
(/tmp/create-virtual-display) saves the background process PID to VDISPLAY_PID
and writes it to GITHUB_ENV but never stops the process; add explicit cleanup by
capturing VDISPLAY_PID (from GITHUB_ENV) and killing the process in a subsequent
step or a post-job step (e.g., a "Cleanup virtual display" step or a
post-action) that uses kill (or pkill) against VDISPLAY_PID and removes the
/tmp/create-virtual-display binary; reference the created binary name
(/tmp/create-virtual-display), the environment variable VDISPLAY_PID, and the
source scripts/create-virtual-display.m when locating where to add the
complementary cleanup.
macos-14 runners default to Xcode 15.4, but sentry-cocoa needs Swift tools version 6.0 (Xcode 16+). Pick the latest Xcode_*.app instead of the default symlink.
Greptile SummaryThis PR adds a comprehensive macOS compatibility CI workflow that tests on both macos-14 (Sonoma) and macos-15 (Sequoia) runners. The implementation follows established patterns from the existing Key changes:
The smoke test provides a lightweight sanity check that complements the existing comprehensive test suite. All code follows repository conventions with clean error handling and appropriate timeouts. Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 62db3d1 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62db3d190b
ℹ️ 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".
| s.close() | ||
| print(data) | ||
| ") | ||
| echo "Send response: $SEND_RESPONSE" |
There was a problem hiding this comment.
The smoke test captures SEND_RESPONSE but never checks that it is OK, so this workflow can pass even when the terminal command path is broken (for example, if no terminal is focused or send returns an error) as long as the two ping checks succeed. That makes the new compatibility CI report false positives for exactly the command-delivery behavior it is meant to validate.
Useful? React with 👍 / 👎.
Using `open` can fail silently on CI runners. Launch the binary directly with env vars set, capture stdout/stderr, and add process health checks with diagnostic output (debug log tail, crash reports) on failure.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2333bc439
ℹ️ 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".
| # Pick the latest Xcode installed on the runner. GitHub-hosted macos-14 | ||
| # defaults to Xcode 15.4, but the project needs Xcode 16+ (Swift tools | ||
| # version 6.0 required by sentry-cocoa). | ||
| XCODE_APP="$(ls -d /Applications/Xcode_*.app 2>/dev/null | sort | tail -n 1)" |
There was a problem hiding this comment.
Because this step enables set -euo pipefail, the command substitution at XCODE_APP="$(ls -d /Applications/Xcode_*.app ... | sort | tail -n 1)" exits non-zero when no Xcode_*.app matches, and the shell terminates before the /Applications/Xcode.app fallback branch executes. On runner images that only provide Xcode.app (or use a different naming convention), the workflow fails during Xcode selection even though a usable Xcode is installed.
Useful? React with 👍 / 👎.
…anaflow-ai#769) * Add macOS compatibility CI: unit tests + smoke test on macos-14/15 New workflow runs on GitHub-hosted macos-14 and macos-15 runners (matrix strategy). Each run: unit tests via cmux-unit scheme, then a smoke test that builds the app, launches it, sends a command via the socket, and verifies it stays alive for 15 seconds. * Select latest Xcode on runner (fix macos-14 Swift tools version) macos-14 runners default to Xcode 15.4, but sentry-cocoa needs Swift tools version 6.0 (Xcode 16+). Pick the latest Xcode_*.app instead of the default symlink. * Launch app binary directly in smoke test for better CI compatibility Using `open` can fail silently on CI runners. Launch the binary directly with env vars set, capture stdout/stderr, and add process health checks with diagnostic output (debug log tail, crash reports) on failure.
This branch was successfully deployed
1 participant
Summary
Adds a new
ci-macos-compat.ymlworkflow that runs on GitHub-hostedmacos-14(Sonoma) andmacos-15(Sequoia) runners. The existing CI uses Depot runners (depot-macos-latest), which only covers one macOS version.Each matrix entry runs:
cmux-unitscheme (no XCUITests)timecommand, and verifies the app stays alive for 15 secondsThe smoke test script (
scripts/smoke-test-ci.sh) is standalone and reusable. It pings the socket, sends a command, waits 15 seconds, then pings again to confirm the app is still responsive.Testing
Related
Summary by CodeRabbit