fix(ci): preserve exact Harness E2E candidate stack - #721
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ytallo
added
the
no-ticket
Contributor
skill-check — worker0 verified, 54 skipped (no docs/).
Four for four. Nicely done. |
📝 WalkthroughWalkthroughThe PR adds canonical profile-digest validation to Harness E2E workflows. It also changes deployed E2E worker installation to use the stable support channel and dependency-first exact-version ordering. ChangesProfile validation
Deployed worker installation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant ProfileResolver
participant Catalog
Workflow->>ProfileResolver: Provide expected catalog SHA and profile digest
ProfileResolver->>Catalog: Resolve scenario profile
Catalog-->>ProfileResolver: Return canonical digest and catalog SHA
ProfileResolver-->>Workflow: Accept profile or reject changed profile
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/scripts/tests/test_harness_e2e_profiles.py (1)
60-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the catalog-SHA fallback path.
The call always supplies
expected_profile_digest. It does not exercise thenot expected_profile_digestfallback branch. Add a case with a changed catalog SHA and no profile digest.Proposed test
+def test_rejects_catalog_changes_without_profile_digest(): + catalog = load_profile_catalog() + with pytest.raises(ValueError, match="catalog moved"): + resolve_profile( + catalog, + available=list(catalog.ids), + profile="release", + requested=[], + catalog_sha="b" * 40, + expected_catalog_sha="a" * 40, + )🤖 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 @.github/scripts/tests/test_harness_e2e_profiles.py around lines 60 - 79, Extend test_rejects_unknown_scenario_and_changed_profile to call resolve_profile with a changed expected_catalog_sha and without expected_profile_digest, asserting the profile-changed ValueError. Keep the existing digest-mismatch case intact and ensure the new case exercises the catalog-SHA fallback branch.
🤖 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.
Nitpick comments:
In @.github/scripts/tests/test_harness_e2e_profiles.py:
- Around line 60-79: Extend test_rejects_unknown_scenario_and_changed_profile to
call resolve_profile with a changed expected_catalog_sha and without
expected_profile_digest, asserting the profile-changed ValueError. Keep the
existing digest-mismatch case intact and ensure the new case exercises the
catalog-SHA fallback branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 971e7027-32d2-47e5-9fa4-188760ddee7a
📒 Files selected for processing (5)
.github/scripts/harness_e2e_profiles.py.github/scripts/tests/test_harness_e2e_profiles.py.github/workflows/_harness-e2e.yml.github/workflows/harness-e2e-deployed.ymlharness/tests/e2e/run-deployed-ci.sh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
latestinstead of applying the candidate channel to unrelated workersWhy
Version bump commits move
mainafter a release preview without changing the selected E2E profile. The previous full-SHA comparison rejected those safe commits. After that was bypassed for diagnosis, the deployed stack appliednextto auxiliary workers, selecting an outdated database release and requesting tags that do not exist forfpand provider workers.The candidate identity remains the exact
stack_versionsmap. Stable support workers no longer depend on candidate-channel availability.Validation
python3 -m compileallfor the profile resolver and testsbash -n harness/tests/e2e/run-deployed-ci.shgit diff --checkSummary by CodeRabbit
Bug Fixes
CI/CD Improvements