Repository navigation
Guard E2E Depot runner identity - #4944
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.
|
📝 WalkthroughWalkthroughAdds an early E2E workflow step that enforces Depot runner identity when ChangesRunner Identity Validation Guard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (15 passed)
✨ 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 SummaryAdds a runner identity guard to the manual E2E workflow so that jobs requesting a
Confidence Score: 5/5CI-only change that adds a fail-fast guard and matching test assertions; no application code, auth, or data paths are touched. Both changed files are CI workflow and test scaffolding. The guard logic is straightforward — a case statement with set -euo pipefail, a correctly scoped if: condition that covers all depot-macos-* runner choices, and a safe env-var approach to read runner.name without clashing with built-in variables. The test extension uses grep and awk patterns that accurately mirror the workflow content. No production behavior is affected. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[workflow_dispatch triggered] --> B{inputs.runner starts\nwith depot-macos-?}
B -- No --> E[Skip identity guard]
B -- Yes --> C[Validate Depot runner identity step\nREQUESTED_RUNNER = inputs.runner OR depot-macos-latest\nRUNNER_CONTEXT_NAME = runner.name]
C --> D{RUNNER_CONTEXT_NAME\nstarts with depot-*?}
D -- Yes --> F[Log: Resolved runner matches depot-*? yes\nProceed to Checkout]
D -- No --> G[echo ::error:: misrouting message\nexit 1 — job fails fast]
E --> F
Reviews (4): Last reviewed commit: "Guard E2E Depot runner identity" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - name: Validate Depot runner identity | ||
| if: ${{ (inputs.runner || 'depot-macos-latest') == 'depot-macos-latest' }} | ||
| env: | ||
| REQUESTED_RUNNER: ${{ inputs.runner || 'depot-macos-latest' }} | ||
| RUNNER_CONTEXT_NAME: ${{ runner.name }} | ||
| RUNNER_CONTEXT_ENVIRONMENT: ${{ runner.environment }} | ||
| run: | | ||
| set -euo pipefail | ||
| echo "Requested runner: $REQUESTED_RUNNER" | ||
| echo "Actual runner: $RUNNER_CONTEXT_NAME ($RUNNER_CONTEXT_ENVIRONMENT)" | ||
| case "$RUNNER_CONTEXT_NAME" in | ||
| depot-*) ;; | ||
| *) | ||
| echo "::error::depot-macos-latest resolved to '$RUNNER_CONTEXT_NAME', expected a Depot runner named depot-*. Remove depot-macos-latest from non-Depot self-hosted runners or choose an explicit runner." | ||
| exit 1 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
inputs.runner resolves to depot-macos-latest. If depot-macos-14 is accidentally registered on a non-Depot self-hosted runner (the same scenario that motivated this PR), the step is skipped entirely and the job silently continues on the wrong host. Dropping the == 'depot-macos-latest' equality check and instead always running when the requested label starts with depot- would extend coverage to both options.
| - name: Validate Depot runner identity | |
| if: ${{ (inputs.runner || 'depot-macos-latest') == 'depot-macos-latest' }} | |
| env: | |
| REQUESTED_RUNNER: ${{ inputs.runner || 'depot-macos-latest' }} | |
| RUNNER_CONTEXT_NAME: ${{ runner.name }} | |
| RUNNER_CONTEXT_ENVIRONMENT: ${{ runner.environment }} | |
| run: | | |
| set -euo pipefail | |
| echo "Requested runner: $REQUESTED_RUNNER" | |
| echo "Actual runner: $RUNNER_CONTEXT_NAME ($RUNNER_CONTEXT_ENVIRONMENT)" | |
| case "$RUNNER_CONTEXT_NAME" in | |
| depot-*) ;; | |
| *) | |
| echo "::error::depot-macos-latest resolved to '$RUNNER_CONTEXT_NAME', expected a Depot runner named depot-*. Remove depot-macos-latest from non-Depot self-hosted runners or choose an explicit runner." | |
| exit 1 | |
| ;; | |
| esac | |
| - name: Validate Depot runner identity | |
| if: ${{ startsWith(inputs.runner || 'depot-macos-latest', 'depot-') }} | |
| env: | |
| REQUESTED_RUNNER: ${{ inputs.runner || 'depot-macos-latest' }} | |
| RUNNER_CONTEXT_NAME: ${{ runner.name }} | |
| RUNNER_CONTEXT_ENVIRONMENT: ${{ runner.environment }} | |
| run: | | |
| set -euo pipefail | |
| echo "Requested runner: $REQUESTED_RUNNER" | |
| echo "Actual runner: $RUNNER_CONTEXT_NAME ($RUNNER_CONTEXT_ENVIRONMENT)" | |
| case "$RUNNER_CONTEXT_NAME" in | |
| depot-*) ;; | |
| *) | |
| echo "::error::$REQUESTED_RUNNER resolved to '$RUNNER_CONTEXT_NAME', expected a Depot runner named depot-*. Remove $REQUESTED_RUNNER from non-Depot self-hosted runners or choose an explicit runner." | |
| exit 1 | |
| ;; | |
| esac |
|
Actionable comments posted: 0 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
c518883 to
73febe5
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 73febe5. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 56-57: The error message hardcodes "depot-macos-latest" instead of
reflecting the actual requested runner; update the echo in the echo/exit block
that references RUNNER_CONTEXT_NAME to use the REQUESTED_RUNNER variable (use
$REQUESTED_RUNNER) so the output reads "$REQUESTED_RUNNER resolved to
'$RUNNER_CONTEXT_NAME', expected a Depot runner named depot-*..." ensuring
failures report the exact requested label.
In `@tests/test_ci_self_hosted_guard.sh`:
- Around line 66-69: The test currently greps for the allow-branch pattern
'depot-*) ;;' which validates the wrong case; change the assertions to verify
the reject/mismatch branch in "$E2E_FILE" instead by asserting that the mismatch
branch contains the misrouting error message and an 'exit 1' call (i.e., ensure
the branch that handles non-Depot resolution logs the misrouting error and exits
with status 1); apply this fix to both occurrences that currently check for
'depot-*) ;;' so the test actually validates fail-fast behavior for
test-e2e.yml.
🪄 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: 476a7fdc-2e7b-4213-b2c1-d3a0d058a20e
📒 Files selected for processing (2)
.github/workflows/test-e2e.ymltests/test_ci_self_hosted_guard.sh
73febe5 to
afb954a
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 51-57: Remove raw runner identifiers from outputs: stop echoing
RUNNER_CONTEXT_NAME and RUNNER_CONTEXT_ENVIRONMENT and avoid including
RUNNER_CONTEXT_NAME in the error text. Instead, print a non-sensitive status
(e.g., "Requested runner: $REQUESTED_RUNNER" and "Resolved runner: <redacted>
(matches depot-*? yes/no)") and change the error to a generic message that
mentions REQUESTED_RUNNER only in sanitized form or uses a generic placeholder;
keep the existing case guard and exit behavior around the depot-* check (refer
to the variables RUNNER_CONTEXT_NAME, REQUESTED_RUNNER,
RUNNER_CONTEXT_ENVIRONMENT and the case block) so the logic is unchanged while
removing unredacted identifiers from logs and error output.
🪄 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: 23e8a6d1-b3c9-4cb6-a363-9df6e9bdfed9
📒 Files selected for processing (2)
.github/workflows/test-e2e.ymltests/test_ci_self_hosted_guard.sh
afb954a to
29176a0
Compare
|
Actionable comments posted: 0 |
Stale CodeRabbit review addressed in 29176a0; final CodeRabbit follow-up posted 0 actionable comments and all checks passed.
This branch was successfully deployed
1 participant
Summary
depot-macos-latestlabel resolves to a non-Depot runner.Why
The strict activation proof in https://github.com/manaflow-ai/cmux/actions/runs/26570108351 silently routed
depot-macos-latestto an AWS self-hosted runner namedcmux-aws-m4pro-5, then failed with a misleading app activation error. After removing that accidental external label, https://github.com/manaflow-ai/cmux/actions/runs/26571172281 resolved to a true Depot runner nameddepot-w8l2gw3nfv. This PR makes that runner-class invariant explicit in the workflow.Testing
actionlint -oneline .github/workflows/test-e2e.yml./tests/test_ci_self_hosted_guard.shgit diff --check@coderabbitai review
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Low Risk
Workflow-only guard and test assertions; no application runtime, auth, or data-path changes.
Overview
Adds an early Validate Depot runner identity step to the manual E2E workflow when the selected label is
depot-macos-*. It compares the requested runner to${{ runner.name }}and fails fast with a clear::error::if the job did not land on adepot-*host (avoiding misleading GUI/activation failures on mislabeled self-hosted runners).Extends
tests/test_ci_self_hosted_guard.shso CI asserts that guard stays wired:runner.nameis set, all Depot macOS choices are gated, and misrouting triggers the documented error plusexit 1.Reviewed by Cursor Bugbot for commit 29176a0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Add an identity guard to the E2E workflow so jobs requesting
depot-macos-*fail fast if routed to a non-Depot runner. This prevents misleading activation failures and makes the runner class explicit..github/workflows/test-e2e.ymlfor anydepot-macos-*(defaultdepot-macos-latest) that echoes the requested label, checksrunner.namestarts withdepot-, and fails with a clear message if not.tests/test_ci_self_hosted_guard.shto assert therunner.nameinspection, thestartsWith(inputs.runner || 'depot-macos-latest', 'depot-macos-')gate, the misrouting::error::plusexit 1, and that nocontinue-on-erroris used.Written for commit 29176a0. Summary will update on new commits.
Review in cubic
Summary by CodeRabbit