psa-systems/mokosh-server
Watch
9
0
Fork
You've already forked mokosh-server
 1
Code Issues Pull requests Projects Releases 10 Packages Wiki Activity Actions

feat(e2e): close PMS-142 - un-fixme the auth-ui login + logout round-trip #112

Merged
nrupard merged 3 commits from feat/pms-142-auth-ui-unfixme into main 2026-06-05 20:51:57 +02:00
Conversation 0 Commits 3 Files changed 3 +128 -19
nrupard commented 2026-06-05 19:19:35 +02:00
Owner
Copy link

Closes PMS-142.

What

The auth-ui browser-driven login + logout round-trip was test.fixme'd in PMS-140 phase 1 because it flaked. Two observed failure modes are addressed here:

  1. SPA form submit not progressing past /login - bunyip's hub form is HTMX, not WASM, so a hydration race is unlikely on that side. Remaining hypotheses are transient (rate-limit blip, network latency). loginViaSpa itself stays as-is; CI data will tell us whether more is needed.

  2. Logout assertion stalling on /dashboard - root cause is the Dioxus WASM hydration race: the avatar button rendered before its onclick was wired up, so the first click no-op'd. Each click toggles the menu's open state (mokosh-clients/src/components/layout.rs:386), so a subsequent click on the now-hydrated handler opens it. The fix is a click-retry loop in logout(): after each click, wait for the popup role="menu" to actually appear; if it doesn't within 3s, click again. Capped at 3 attempts so a genuinely broken SPA fails clearly.

Diagnostics

Extracted the URL-trail + request-log pattern that global.setup.ts already uses into lib/page-diagnostics.ts. The auth-ui test now wraps both halves of the round-trip in try/catch with attachPageDiagnostics(page).snapshot(...), so future failures name exactly what the SPA did instead of speculating.

AC delta

Per the issue's AC: "Two consecutive post-merge CI runs show auth-ui passing on the first attempt (not retry-recovered)." That's an operator-observed quality bar AFTER merge - the workflow itself cannot gate on it. If a clear regression surfaces in the next handful of runs, file a follow-up against PMS-142.

Test plan

  • This PR's own gate run exercises the new path against staging.
  • If logout() flakes again, the new diagnostic dump names URL trail + last 30 requests; iterate from there.
Closes PMS-142. ## What The auth-ui browser-driven login + logout round-trip was `test.fixme`'d in PMS-140 phase 1 because it flaked. Two observed failure modes are addressed here: 1. **SPA form submit not progressing past `/login`** - bunyip's hub form is HTMX, not WASM, so a hydration race is unlikely on that side. Remaining hypotheses are transient (rate-limit blip, network latency). `loginViaSpa` itself stays as-is; CI data will tell us whether more is needed. 2. **Logout assertion stalling on `/dashboard`** - root cause is the Dioxus WASM hydration race: the avatar button rendered before its onclick was wired up, so the first click no-op'd. Each click toggles the menu's `open` state (`mokosh-clients/src/components/layout.rs:386`), so a subsequent click on the now-hydrated handler opens it. The fix is a click-retry loop in `logout()`: after each click, wait for the popup `role="menu"` to actually appear; if it doesn't within 3s, click again. Capped at 3 attempts so a genuinely broken SPA fails clearly. ## Diagnostics Extracted the URL-trail + request-log pattern that `global.setup.ts` already uses into `lib/page-diagnostics.ts`. The auth-ui test now wraps both halves of the round-trip in try/catch with `attachPageDiagnostics(page).snapshot(...)`, so future failures name exactly what the SPA did instead of speculating. ## AC delta Per the issue's AC: "Two consecutive post-merge CI runs show auth-ui passing on the first attempt (not retry-recovered)." That's an operator-observed quality bar AFTER merge - the workflow itself cannot gate on it. If a clear regression surfaces in the next handful of runs, file a follow-up against PMS-142. ## Test plan - [ ] This PR's own gate run exercises the new path against staging. - [ ] If `logout()` flakes again, the new diagnostic dump names URL trail + last 30 requests; iterate from there.
feat(e2e): close PMS-142 - un-fixme the auth-ui login + logout round-trip
Some checks failed
E2E (staging) / Playwright against staging (pull_request) Failing after 1m58s
Details
Check / fmt + clippy + compile + tests (pull_request) Successful in 2m10s
Details
826e5b4018 
The auth-ui test was quarantined in PMS-140 phase 1 because the browser-driven round-trip flaked non-deterministically against staging. Two observed failure modes:

1. SPA form submit didn't progress past /login. Bunyip's hub form is HTMX, not WASM, so a hydration race is unlikely on that side; remaining hypotheses are transient (rate-limit blip, network latency). loginViaSpa itself stays as-is; CI data will tell if more is needed.

2. Logout assertion stalled on /dashboard. Root cause: Dioxus WASM hydration race - the avatar button rendered before its onclick handler was wired up, so the first click no-op'd. Each click toggles the menu's `open` state (mokosh-clients/src/components/layout.rs:386), so a subsequent click on the now-hydrated handler opens it. Implement a click-retry loop in `logout()`: after each click, wait for the popup `role="menu"` to actually appear; if it doesn't within MENU_OPEN_WAIT_MS (3s), click again. Cap at MENU_OPEN_ATTEMPTS (3) so a genuinely broken SPA fails clearly with a useful message.

Extracted the URL-trail + request-log diagnostic pattern from global.setup.ts into `lib/page-diagnostics.ts` so the auth-ui test gets the same precision on failure as setup does (`attachPageDiagnostics(page)` returns a `snapshot(label, page)` method that folds into thrown errors). Both halves of the round-trip wrap in try/catch with snapshots, so a future failure names exactly what the SPA did instead of speculating.

Drop `test.fixme`. The merge-gate (PMS-141) means this PR's own CI run exercises the new path; if it flakes, the diagnostic dump tells us what to harden next. AC "two consecutive green post-merge runs on first attempt" is an operator-observed quality bar after merge - the workflow itself cannot gate on it. Revisit if a clear regression pattern surfaces in the next handful of runs.

README quarantine note dropped from the "What it covers" row and the "Harness shape" auth-ui paragraph.

#PMS-142
test(e2e): tighten page-diagnostics API + use Error cause for original stack
Some checks failed
Check / fmt + clippy + compile + tests (pull_request) Successful in 1m27s
Details
E2E (staging) / Playwright against staging (pull_request) Failing after 1m31s
Details
8c50b19e59 
Review follow-up on PR #112:

- e2e/lib/page-diagnostics.ts: `snapshot(label, page)` redundantly took `page` when the function already closed over it. Drop the second arg so callers cannot accidentally hand in a stale or different page; inline the return type at the same time since `PageDiagnostics` had no external consumers.
- e2e/tests/auth.spec.ts: rewrite the catch blocks to use `throw new Error(msg, { cause: err })` instead of string-concatenating `err` into the message. Playwright's reporter surfaces the cause's stack + failed-matcher details under the diagnostic dump, instead of dropping them when the original Error gets stringified.

#PMS-142
test(e2e): re-quarantine auth-ui - bunyip /logout does not kill OP session
Some checks failed
E2E (staging) / Playwright against staging (pull_request) Successful in 33s
Details
Check / fmt + clippy + compile + tests (pull_request) Failing after 1m30s
Details
Create release / Create release from merged PR (pull_request) Has been skipped
Details
b6d96191ed 
CI diagnostic from the un-fixme attempt found the root cause is an upstream bunyip bug, not a harness issue. After Logout is clicked:

- The SPA navigates to `https://a8n.systems/logout` (verified in the request log).
- Bunyip's GET /logout calls `auth_api::logout`, which POSTs `/v1/auth/logout`, which calls `AuthCookies::clear` to drop `access_token` and `refresh_token` cookies.
- Immediately afterwards the SPA fires `/oauth2/authorize` and the OP issues a fresh `code` (URL trail and request log both confirm). The SPA exchanges the code, lands back on /dashboard, the test's `expectAtLoginScreen` times out.

So `/oauth2/authorize` is still accepting the user as authenticated after the supposed logout - some persistence path (OP session cookie, hub session cookie, refresh-bearing path) survives `AuthCookies::clear`. The SPA author actually predicted this in `mokosh-clients/src/components/layout.rs:367-373` ("Without that round-trip, the OP cookie would still be live and a subsequent visit here would silently sign the user back in via the SSO bridge.") - the round-trip IS happening, but the server-side clear isn't enough on staging.

Re-fixme with a sharp comment naming the upstream gap. KEEP the new harness infrastructure in tree for the eventual un-fixme:

- `lib/page-diagnostics.ts` (shared URL trail + request log capture).
- The `attachPageDiagnostics` + Error-cause try/catch wrapping in auth.spec.ts.
- The click-retry-if-menu-not-open loop in `logout()` (Dioxus WASM hydration race defense).

README "What it covers" row + harness shape paragraph re-mark the test as quarantined with the new upstream-bug reason. Filing the bunyip-side issue is left to the operator (CLAUDE auto-mode declined to create issues in a different project without explicit ask).

#PMS-142
nrupard deleted branch feat/pms-142-auth-ui-unfixme 2026-06-05 20:51:57 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!112
No description provided.