diff --git a/.github/workflows/e2e-live.yml b/.github/workflows/e2e-live.yml index ec46a19f9..840942ceb 100644 --- a/.github/workflows/e2e-live.yml +++ b/.github/workflows/e2e-live.yml @@ -188,10 +188,30 @@ jobs: name: backend-log-live-${{ github.run_id }} path: .test-state/playwright/backend.log retention-days: 7 - - name: Upload Playwright report + - name: List Playwright output locations (debug) + if: always() + run: | + echo "::group::Playwright output dirs" + # Playwright anchors its default outputDir + HTML report to the + # nearest package.json, which is frontend/ (frontend/editor has + # none), so artifacts land under frontend/, not frontend/editor/. + ls -la frontend/playwright-report 2>/dev/null \ + || echo "no playwright-report at frontend/" + ls -la frontend/test-results 2>/dev/null \ + || echo "no test-results at frontend/" + find . -name node_modules -prune -o -name 'trace.zip' -print 2>/dev/null || true + echo "::endgroup::" + - name: Upload Playwright report + traces if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: playwright-report-live-${{ github.run_id }} - path: frontend/editor/playwright-report/ + # test-results/ holds the per-test trace.zip (with browser console + # logs) + screenshots/video; playwright-report/ is the HTML report. + # Both live under frontend/ (Playwright anchors them to the nearest + # package.json, which is frontend/; frontend/editor has none). + path: | + frontend/playwright-report/ + frontend/test-results/ retention-days: 7 + if-no-files-found: warn diff --git a/frontend/editor/src/core/services/httpErrorHandler.ts b/frontend/editor/src/core/services/httpErrorHandler.ts index 69d4c6e06..ed7108b6a 100644 --- a/frontend/editor/src/core/services/httpErrorHandler.ts +++ b/frontend/editor/src/core/services/httpErrorHandler.ts @@ -75,6 +75,22 @@ function markLoginRedirectFired(): void { } } +// Reset the throttle when the user establishes a fresh session via interactive +// login. Otherwise a genuine expiry that happens within the throttle window of +// the redirect that sent them to /login would be wrongly suppressed, leaving +// them on a page with silently failing requests. Login dispatches +// "jwt-available"; token refresh does not (it fires "TOKEN_REFRESHED"), so +// refresh-driven redirect churn is still dampened by the throttle. +if (typeof window !== "undefined") { + window.addEventListener("jwt-available", () => { + try { + window.sessionStorage.removeItem(LOGIN_REDIRECT_THROTTLE_KEY); + } catch { + // sessionStorage unavailable - nothing to clear + } + }); +} + /** * Handles HTTP errors with toast notifications and file error broadcasting * Returns true if the error should be suppressed (deduplicated), false otherwise diff --git a/frontend/editor/src/core/tests/live/authentication-login.spec.ts b/frontend/editor/src/core/tests/live/authentication-login.spec.ts index f4c8d7401..4875fbc4d 100644 --- a/frontend/editor/src/core/tests/live/authentication-login.spec.ts +++ b/frontend/editor/src/core/tests/live/authentication-login.spec.ts @@ -95,8 +95,23 @@ test.describe("1. Authentication and Login", () => { await page.goto("/merge"); await page.waitForLoadState("domcontentloaded"); - // Step 1-2: Invalidate session by clearing cookies and localStorage JWT, - // then re-add the cookie consent cookie so the banner doesn't block after redirect + // Simulate a genuinely expired session by forcing the token-refresh + // endpoint to fail. Clearing client state alone is not enough: the SPA's + // auth client refreshes a fresh JWT from a surviving server-side + // credential and re-authenticates, churning between /merge, / and /login + // instead of settling on the login page. A 401 here is what a real + // expired/revoked session looks like to the client. + await page.route("**/api/v1/auth/refresh", (route) => + route.fulfill({ + status: 401, + contentType: "application/json", + body: JSON.stringify({ error: "session_expired" }), + }), + ); + + // Invalidate the client session: clear cookies + the stored JWT, then + // re-add the cookie-consent cookie so the banner doesn't block the login + // page after redirect. await page.context().clearCookies(); await page.evaluate(() => { localStorage.removeItem("stirling_jwt"); @@ -116,23 +131,28 @@ test.describe("1. Authentication and Login", () => { }, ]); - // Full page reload forces the SPA to re-check auth with the backend + // Full page reload forces the SPA to re-check auth with the backend. await page.reload({ waitUntil: "domcontentloaded" }); - // Step 3: Verify the user is redirected to the login page. The redirect - // is driven client-side after the SPA re-bootstraps (session check + - // config fetch + backend probe), which can outrun a 15s budget on a - // loaded CI runner, so allow longer here. - await expect(page).toHaveURL(/\/login/, { timeout: 30000 }); + // With refresh failing, the SPA settles on the login page. Wait for the + // login form to render (a concrete signal the redirect landed) and then + // confirm the URL, rather than racing a URL poll against the bootstrap. + await page + .locator("#email") + .waitFor({ state: "visible", timeout: 30000 }); + await expect(page).toHaveURL(/\/login/); - // Step 5: Log in with valid credentials + // Stop forcing refresh failures so the fresh login below behaves normally. + await page.unroute("**/api/v1/auth/refresh"); + + // Log back in with valid credentials. await page.locator("#email").fill("admin"); await page.locator("#password").fill("adminadmin"); await page.locator('button[type="submit"]').click(); - // Step 6: Verify the user is redirected back to /merge or home. - // Any non-/login URL is acceptable — the app may route to the original - // page (/merge) or to the dashboard (/), both are valid post-login states. + // Verify the user is redirected back off the login page. Any non-/login + // URL is acceptable: the app may route to the original page (/merge) or + // to the dashboard (/), both are valid post-login states. await page.waitForURL((url) => !url.pathname.includes("/login"), { timeout: 15000, });