fix: fix prod dashboard SSO login for recent CI failures - Playwright login#1298
Conversation
- Assert username Fill stuck before Next; fail fast on SSO login-required error - Detect profile-completion gate after login and dump URL/screenshot on failure - Accept TrustArc "Accept all" cookie variant with shorter optional wait Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
WalkthroughUpdates DevSandbox setup and login helpers with required configuration checks, resilient TrustArc consent dismissal, explicit SSO synchronization, profile-gate completion, dashboard verification, and expanded failure diagnostics. ChangesDevSandbox authentication flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Setup
participant TrustArc
participant LoginPage
participant SSO
participant Dashboard
Setup->>TrustArc: install dismissal handler
Setup->>LoginPage: navigate and start login
LoginPage->>SSO: fill credentials and submit
TrustArc-->>SSO: dismiss consent overlay when present
SSO-->>LoginPage: password, validation error, dashboard, or profile gate
LoginPage->>SSO: complete profile gate when present
SSO->>Dashboard: display Developer Sandbox
LoginPage->>Dashboard: verify dashboard header
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
coderabbitai
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@testsupport/devsandbox-dashboard/login.go`:
- Around line 198-221: Update the login failure diagnostics around the URL, page
text, and screenshot capture to avoid exposing SSO data: log only the URL
origin/path with query and fragment removed, and gate page-text logging and
screenshot creation on the explicitly approved artifact-retention policy used by
Setup. Ensure sensitive diagnostics are omitted or redacted when that policy is
not enabled.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 5cd17005-f756-4dd6-8aba-0483bab3597d
📒 Files selected for processing (2)
testsupport/devsandbox-dashboard/login.gotestsupport/devsandbox-dashboard/setup.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/api(manual)codeready-toolchain/toolchain-common(manual)codeready-toolchain/host-operator(manual)codeready-toolchain/toolchain-e2e(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: govulncheck
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
testsupport/devsandbox-dashboard/setup.gotestsupport/devsandbox-dashboard/login.go
🔇 Additional comments (10)
testsupport/devsandbox-dashboard/setup.go (3)
48-50: LGTM!
120-143: LGTM!
146-158: 🎯 Functional CorrectnessVerify the combined locator is always single-match.
Locator.Oris strict: if a consent variant exposes more than one of these actions,IsVisibleorClickcan fail rather than choosing one. Confirm the buttons are mutually exclusive; otherwise apply an explicit priority (for example,.First()) or probe candidates sequentially.testsupport/devsandbox-dashboard/login.go (7)
4-24: LGTM!
76-79: LGTM!
81-106: LGTM!
115-123: LGTM!
125-142: LGTM!
152-163: 🎯 Functional CorrectnessVerify dashboard readiness before the generic header check.
lp.Headerispage.Locator("header"). If the SSO profile page has any<header>, this returns success before detecting the profile gate, so the completion click and failure screenshot are skipped. Use a dashboard-specific selector/text condition, or check the profile gate first.
175-191: LGTM!
- Log origin/path only; skip page text/screenshots in CI (ARTIFACT_DIR) - Check profile gate before dashboard markers to avoid SSO header false positives - Probe TrustArc accept buttons sequentially instead of Locator.Or Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
- Auto-dismiss TrustArc mid-login via Page.AddLocatorHandler - Retry username Fill until the value sticks after SSO remount - Cover Accept default / Accept all / Required Cookies only buttons Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
fbm3307
changed the title
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fbm3307, rajivnathan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
- Deduplicate div[name=trustarc_cm] literal flagged by SonarCloud Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Feny Mehta <fbm3307@gmail.com>
|
2 participants
Fixes flaky
ci-daily-prodPlaywright login. Video analysis showed #1287 (60s Password wait) addressed the wrong root cause: Mode A failures were TrustArc mid-login clearing the username before Next ("Red Hat login is required"), not a slow password field. Mode B is an SSO profile-completion gate after login.This change:
AddLocatorHandler) and covers Accept default / Accept all / Required Cookies onlyCI failures
Mode A — TrustArc race / "Red Hat login is required" (looked like Password timeout)
Mode B — SSO profile completion gate after login
Assisted by: Cursor
Summary by CodeRabbit
Summary by CodeRabbit