Repository navigation
Run E2E xcodebuild in GUI bootstrap - #4940
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRuns xcodebuild inside the console GUI user's session with Automation Mode when available and captures its output via a RUN_XCODEBUILD wrapper. FeedSidebarUITests now calls app.launch() directly, removing the prior XCTExpectFailure tolerance. ChangesUI Test Execution Reliability
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 16 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (16 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 moves the hosted E2E
Confidence Score: 5/5Safe to merge; the double-sudo chain correctly resolves the previously flagged root-owned DerivedData problem and the fallback paths are non-fatal. Both changed files make targeted, well-reasoned changes. The workflow correctly addresses the previous root-process concern by chaining an inner sudo re-identification so xcodebuild never runs as root. Detection and fallback paths are guarded with warnings. The Swift change cleanly removes a non-strict expected-failure mask now that the underlying CI activation issue is fixed at the workflow level. .github/workflows/test-e2e.yml — the HOME/USER env vars in XCODEBUILD_ENV reflect the runner user's identity and override sudo -H's home-dir substitution; worth a second look if the runner user and console GUI user are ever different accounts. Important Files Changed
Sequence DiagramsequenceDiagram
participant GH as GitHub Actions Runner
participant Shell as Bootstrap Step
participant LC as launchctl asuser
participant GUI as GUI User Process
participant XC as xcodebuild
GH->>Shell: start "Run xcodebuild" step
Shell->>Shell: "stat -f %Su /dev/console -> GUI_USER"
alt Non-root GUI user found AND passwordless sudo available
Shell->>Shell: automationmodetool enable (optional)
Shell->>LC: sudo -n launchctl asuser GUI_UID
LC->>GUI: sudo -n -H -u GUI_USER /usr/bin/env XCODEBUILD_ENV[]
GUI->>XC: xcodebuild test (GUI bootstrap context)
XC-->>Shell: exit code + output
else No GUI user OR no passwordless sudo
Shell->>Shell: warn ::warning::
Shell->>XC: env XCODEBUILD_ENV[] xcodebuild test (fallback)
XC-->>Shell: exit code + output
end
Shell->>GH: OUTPUT / EXIT_CODE
Reviews (3): Last reviewed commit: "Run E2E xcodebuild in GUI bootstrap" | Re-trigger Greptile |
| if [ "$GUI_USER" = "$CURRENT_USER" ]; then | ||
| RUN_XCODEBUILD=(sudo -n launchctl asuser "$GUI_UID" /usr/bin/env "${XCODEBUILD_ENV[@]}") |
There was a problem hiding this comment.
Root-owned DerivedData on persistent self-hosted runners
When GUI_USER == CURRENT_USER, the command is sudo -n launchctl asuser "$GUI_UID" /usr/bin/env … xcodebuild. launchctl asuser only sets the bootstrap context; it does not re-identify the process — so xcodebuild runs as root (UID 0). Even though HOME is forwarded explicitly, all files written under ~/Library/Developer/Xcode/DerivedData will be owned by root. On a persistent self-hosted runner the subsequent rm -rf ~/Library/Developer/Xcode/DerivedData/cmux-* (run as the non-root runner user, no sudo) will fail with permission denied on the next job, breaking the clean-up step. The GUI_USER != CURRENT_USER branch avoids this by chaining an inner sudo -n -H -u "$GUI_USER" to re-identify as the GUI user — the same technique could be used here (i.e. sudo -n launchctl asuser "$GUI_UID" sudo -n -H -u "$GUI_USER" /usr/bin/env …).
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 96275e3. Configure here.
96275e3 to
7fc7eb2
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 @.github/workflows/test-e2e.yml:
- Around line 286-316: The RUN_XCODEBUILD invocation can end up running
xcodebuild as root or with a caller's env instead of the console GUI user: in
the GUI_USER == CURRENT_USER branch replace the current sudo -n launchctl asuser
... /usr/bin/env invocation with a sudo -n -H -u "$GUI_USER" launchctl asuser
"$GUI_UID" /usr/bin/env ... (so the process runs with the GUI_USER credentials),
and in the other branch stop forcing caller-specific vars
(HOME/USER/LOGNAME/TMPDIR) into XCODEBUILD_ENV — either construct XCODEBUILD_ENV
from the target user's environment via sudo -u -H /usr/bin/env or remove those
overrides so launchctl asuser + sudo -u run with the target user's
HOME/USER/LOGNAME/TMPDIR to avoid desyncs; update the RUN_XCODEBUILD assignments
accordingly (symbols: XCODEBUILD_ENV, RUN_XCODEBUILD, GUI_USER, CURRENT_USER,
GUI_UID, launchctl asuser, sudo -n -H -u).
🪄 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: 924a32f9-739e-4947-955e-82a08e203efe
📒 Files selected for processing (2)
.github/workflows/test-e2e.ymlcmuxUITests/FeedSidebarUITests.swift
7fc7eb2 to
75ec1ac
Compare
- a791621 Reduce browser WebView input latency (manaflow-ai#4863) - f2dbc31 Fix file preview Open With menu (manaflow-ai#4932) - 95d4c2f Wait for E2E virtual display readiness (manaflow-ai#4928) - 86558d1 Add cmux diff CodeView command (manaflow-ai#4451) - 6dcddde Run E2E xcodebuild in GUI bootstrap (manaflow-ai#4940) Conflicts resolved: - CLINotifyProcessIntegrationRegressionTests.swift: kept both fork and upstream test additions. - Workspace.swift: kept fork's per-layoutTab snapshot logic; rawLayout aliases the selected tab's layout to satisfy upstream's downstream uses.

Moves hosted E2E xcodebuild into the console user GUI bootstrap when the runner exposes one, then removes the FeedSidebar launch expected-failure mask as the first coverage restoration.
This makes foreground activation a workflow invariant instead of a per-test non-strict expected failure.
Validation:
git diff --checkactionlint -oneline .github/workflows/test-e2e.yml./tests/test_ci_self_hosted_guard.shapp.launch()on commit142fbc7fb, run https://github.com/manaflow-ai/cmux/actions/runs/26565560398, artifact https://github.com/manaflow-ai/cmux-dev-artifacts/issues/193875ec1ac, run https://github.com/manaflow-ai/cmux/actions/runs/26566604865