fix(e2e): make Playwright auth setup actually authenticate (#556)#579
Merged
Conversation
The harness landed in #571 but its auth.setup.js had two bugs that prevented every downstream test from being authenticated. Both surfaced on the first CI run. 1. Admin-login toggle button text is "🔐 Admin Login" — Playwright's getByRole accessible-name match doesn't normalize the emoji prefix reliably. Switch to `button:has-text("Admin Login")`. 2. Saving storageState immediately after the password field is hidden captured an empty state (0 cookies, 0 localStorage entries). The auth store's `localStorage.setItem('token', ...)` write happens async after the form unmounts. Poll until the token actually lands in localStorage before saving. Also tighten supporting selectors: - Submit button → `form button[type="submit"]` (no copy dependency) - Smoke nav check → `getByRole('link', { name: 'X', exact: true })` Verified locally against a live ./scripts/deploy/start.sh stack: 5 passed (5.1s) — auth setup + 4 smoke specs. Refs #556 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
oleksandr-korin
added
the
ui
CI was failing the auth.setup because the frontend redirected to /setup — the first-time setup wizard. On a fresh install: 1. Migration #19 (setup_completed_backfill) runs *before* admin user creation, finds no admin, skips the backfill 2. _ensure_admin_user creates the admin from ADMIN_PASSWORD env var 3. Nothing ever sets setup_completed=true → wizard appears Production avoids this because admin already exists when migrations run. For CI we explicitly mark setup complete via docker exec right after backend health. Also bump the fallback ADMIN_PASSWORD from `ci-test-password` (no upper, no digit, no special — fails OWASP ASVS 2.1) to `CiTestPassword!1` so the backend doesn't warn / silently misbehave on startup. Refs #556
3 tasks
vybe
approved these changes
Apr 29, 2026
vybe
left a comment
vybe
left a comment
Contributor
There was a problem hiding this comment.
LGTM — both bugs well-diagnosed (emoji selector + localStorage race), locally verified 5/5 passing.
vybe
pushed a commit
that referenced
this pull request
Apr 29, 2026
…#582) Frontend equivalent of /test-runner. Existing /test-runner only knows about pytest; nothing covered the Playwright suite added in #571 / #579. The skill encodes: - Pre-flight checks (backend + frontend health, admin password sanity) - npm run test:e2e orchestration with --update-snapshots / --headed / --ui / --grep variants - Failure classification with concrete recovery commands for the six patterns we hit while building the harness: A. Stack down (port-zombie diagnostics + Docker Desktop restart) B. Stuck on /setup wizard (fresh-DB CI scenario, fix via db.set_setting('setup_completed','true')) C. Invalid username/password (env drift between shell and container) D. Submit button disabled (form precondition issue, not stack) E. Auth setup passes but smoke specs land on /login (storageState saved before localStorage.token write — known issue fixed in #579) F. Visual regression baseline mismatch (intentional vs unintentional decision tree) - Reminder to add the `ui` PR label so frontend-e2e CI runs Refs #556 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Summary
Follow-up to #571 (PR #571 / merged commit `747d156e`). The harness landed but its `auth.setup.js` had two bugs that meant every downstream smoke test landed on `/login` and failed. Both surfaced on the first CI run on `dev`.
What was wrong
Also tightened supporting selectors:
Verification
Verified locally against a fresh `./scripts/deploy/start.sh` stack:
```
[setup] authenticate as admin (2.5s) ✓
[chromium] dashboard renders for authenticated admin (860ms) ✓
[chromium] agents page loads (917ms) ✓
[chromium] operating room page loads (897ms) ✓
[chromium] templates page loads (842ms) ✓
5 passed (5.1s)
```
CI on this PR (`ui` label set) will confirm the same in a clean environment.
Files
```
src/frontend/e2e/auth.setup.js | 31 ++++++++++++++++++-------------
src/frontend/e2e/smoke.spec.js | 6 +++---
2 files changed, 28 insertions(+), 16 deletions(-)
```
Refs #556
🤖 Generated with Claude Code