fix(e2e): let the admin persona wait out the OIDC popup handoff

Playwright declares the timeout option on locator.isVisible() and isHidden() as deprecated and ignored - both return immediately. 7538b8a0b already fixed two call sites in this file and deliberately left four, arguing that is the password field STILL there and is a Login control STILL showing are correctly answered at once. That holds for the native login path it measured. It does not hold for the OIDC popup path.

keycloak.js polls the POPUP url and is satisfied once that lands on the callback, while the opener has not re-rendered yet - and admin.js inspects the opener. The trace of EspoCRM variant 0 shows the check starting 5.5 ms after the credential submit and taking 10.7 ms, the error-context snapshot shows the Sign in button visible but disabled, mid-submit, and the network log has oauth-callback.php and api/v1/App/user both returning 200 about 150 ms after the assertion had already failed. Biber passed the same journey in one attempt because biber.js:147 waits; administrator burned three attempts because it did not. That commit assumed EspoCRM 0 failed for an unrelated back-channel reason; it fails at admin.js:191, inside this branch.

The wait uses base 12000, not the 60000 biber carried. resolveTimeout multiplies by 5 on an onion target, so this is 60 s there. Across 13 onion traces the largest wait of any kind that ever SUCCEEDED was 790 ms and the worst navigation 1893 ms; 60 s is 76 times the former. At 60000 the same line would burn 300 s per attempt, and it burns only on the path that already fails, which is the path that has to stay diagnosable. Biber moves to the same base so the two personas cannot drift again.

The three remaining ignored arguments are deleted rather than kept. 7538b8a0b called them inert but harmless; inert is right, harmless is the part that cost it two call sites - the argument reads as a wait that never existed. A repository lint now rejects the construct for isVisible and isHidden only; isChecked, isDisabled, isEditable and isEnabled take a real timeout and are deliberately not matched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

roles/test-e2e-playwright/files/personas/admin.js

@@ -90,7 +90,7 @@ async function runAdminFlow(page, opts = {}) {
       await passwordField.fill(adminNativePassword || adminPassword).catch(() => {});
       await passwordField.press("Enter").catch(() => {});
       await page.waitForLoadState("networkidle", { timeout: resolveTimeout(15_000) }).catch(() => {});
-      if (await passwordField.isVisible({ timeout: resolveTimeout(2_000) }).catch(() => false)) {
+      if (await passwordField.isVisible().catch(() => false)) {
         await page
           .getByRole("button", { name: /log\s*in|sign\s*in|login|submit/i })
           .or(page.locator("button[type='submit'], input[type='submit']"))
@@ -116,7 +116,7 @@ async function runAdminFlow(page, opts = {}) {
     const passwordStillVisible = await page
       .locator("input[type='password']:visible")
       .first()
-      .isVisible({ timeout: resolveTimeout(2_000) })
+      .isVisible()
       .catch(() => false);
     nativeLoginCompleted =
       loginAttempted &&
@@ -151,7 +151,7 @@ async function runAdminFlow(page, opts = {}) {
       if (frame === page.mainFrame()) continue;
       const fUrl = frame.url();
       if (!fUrl || fUrl === "about:blank") continue;
-      if (await adminAuthMarker(frame).first().isVisible({ timeout: resolveTimeout(1_000) }).catch(() => false)) {
+      if (await adminAuthMarker(frame).first().isVisible().catch(() => false)) {
         adminReachedAuthenticated = true;
         break;
       }
@@ -174,12 +174,14 @@ async function runAdminFlow(page, opts = {}) {
     }
   }
   if (adminReachedAuthenticated) {
-    const loginStillVisible = await page
+    const loginControl = page
       .getByRole("link", { name: LOGIN_CONTROL_NAME })
       .or(page.getByRole("button", { name: LOGIN_CONTROL_NAME }))
-      .first()
-      .isVisible({ timeout: resolveTimeout(2_000) })
-      .catch(() => false);
+      .first();
+    const loginStillVisible = await loginControl
+      .waitFor({ state: "hidden", timeout: resolveTimeout(12_000) })
+      .then(() => false)
+      .catch(() => loginControl.isVisible().catch(() => false));
     if (loginStillVisible) {
       expect(
         false,

roles/test-e2e-playwright/files/personas/biber.js

@@ -116,7 +116,7 @@ async function runBiberFlow(page, opts = {}) {
       if (frame === page.mainFrame()) continue;
       const fUrl = frame.url();
       if (!fUrl || fUrl === "about:blank") continue;
-      if (await authMarker(frame).first().isVisible({ timeout: resolveTimeout(1_000) }).catch(() => false)) {
+      if (await authMarker(frame).first().isVisible().catch(() => false)) {
         reachedAuthenticated = true;
         break;
       }
@@ -144,7 +144,7 @@ async function runBiberFlow(page, opts = {}) {
       .or(page.getByRole("button", { name: LOGIN_CONTROL_NAME }))
       .first();
     const loginStillVisible = await loginControl
-      .waitFor({ state: "hidden", timeout: resolveTimeout(60_000) })
+      .waitFor({ state: "hidden", timeout: resolveTimeout(12_000) })
       .then(() => false)
       .catch(() => loginControl.isVisible().catch(() => false));
     if (loginStillVisible) {

roles/test-e2e-playwright/files/personas/guest.js

@@ -89,7 +89,7 @@ async function runGuestFlow(page = {}) {
   const signedInMarker = page
     .getByRole("button", { name: /^(log\s*out|sign\s*out|abmelden)$/i })
     .or(page.getByRole("link", { name: /^(log\s*out|sign\s*out|abmelden)$/i }));
-  const isSignedIn = await signedInMarker.first().isVisible({ timeout: resolveTimeout(1_000) }).catch(() => false);
+  const isSignedIn = await signedInMarker.first().isVisible().catch(() => false);
   expect(
     isSignedIn,
     `guest must NOT end on a signed-in surface (URL ${finalUrl}, reachedAuthChain=${reachedAuthChain})`,

roles/test-e2e-playwright/files/personas/utils/logout.js

@@ -39,11 +39,11 @@ const { resolveTimeout } = require("../../timeouts");
 const LOGOUT_NAME_RE = /log\s*out|sign\s*out|sign-out|abmelden/i;
 const ACCOUNT_MENU_NAME_RE = /(account|\bprofile\b|user.?menu|^menu$|signed\s*in)/i;
 
-async function clickFirstVisible(loc, { timeout = resolveTimeout(5_000) } = {}) {
+async function clickFirstVisible(loc) {
   const count = await loc.count().catch(() => 0);
   for (let i = 0; i < count; i++) {
     const cand = loc.nth(i);
-    if (await cand.isVisible({ timeout }).catch(() => false)) {
+    if (await cand.isVisible().catch(() => false)) {
       await cand.click({ timeout: resolveTimeout(5_000) }).catch(() => {});
       return true;
     }
@@ -96,7 +96,7 @@ async function waitForAnyLogoutCandidate(page, timeoutMs = resolveTimeout(30_000
       const count = await loc.count().catch(() => 0);
       for (let i = 0; i < count; i++) {
         const cand = loc.nth(i);
-        if (await cand.isVisible({ timeout: resolveTimeout(500) }).catch(() => false)) {
+        if (await cand.isVisible().catch(() => false)) {
           return true;
         }
       }
@@ -115,7 +115,7 @@ async function tryLogoutViaMenus(page) {
     const count = await triggerLoc.count().catch(() => 0);
     for (let i = 0; i < count; i++) {
       const trigger = triggerLoc.nth(i);
-      if (!(await trigger.isVisible({ timeout: resolveTimeout(1_000) }).catch(() => false))) continue;
+      if (!(await trigger.isVisible().catch(() => false))) continue;
       const key = await trigger.evaluate((el) => el.outerHTML.slice(0, 200)).catch(() => "");
       if (key && tried.has(key)) continue;
       tried.add(key);
@@ -141,7 +141,7 @@ async function openAccountSettings(page) {
   const count = await links.count().catch(() => 0);
   for (let i = 0; i < count; i++) {
     const link = links.nth(i);
-    if (!(await link.isVisible({ timeout: resolveTimeout(1_000) }).catch(() => false))) continue;
+    if (!(await link.isVisible().catch(() => false))) continue;
     await link.click({ timeout: resolveTimeout(5_000) }).catch(() => {});
     await page.waitForLoadState("domcontentloaded", { timeout: resolveTimeout(30_000) }).catch(() => {});
     return true;
@@ -156,10 +156,11 @@ async function openAccountSettings(page) {
  * without it the Keycloak SSO session — and every app session — survives.
  */
 async function confirmKeycloakLogoutIfPrompted(page) {
+  if (!/\/protocol\/openid-connect\/logout/i.test(page.url())) return;
   const confirmBtn = page
     .locator("#kc-logout, form[action*='logout-confirm'] input[type='submit'], form[action*='logout-confirm'] button")
     .first();
-  if (await confirmBtn.isVisible({ timeout: resolveTimeout(5_000) }).catch(() => false)) {
+  if (await confirmBtn.waitFor({ state: "visible", timeout: resolveTimeout(2_000) }).then(() => true).catch(() => false)) {
     await confirmBtn.click({ timeout: resolveTimeout(10_000) }).catch(() => {});
     await page.waitForLoadState("domcontentloaded", { timeout: resolveTimeout(30_000) }).catch(() => {});
   }

tests/lint/repository/test_playwright_ignored_timeout.py

@@ -0,0 +1,82 @@
+"""Flag ``timeout`` passed to Playwright predicates that ignore it.
+
+``locator.isVisible()`` and ``locator.isHidden()`` declare the option
+``@deprecated This option is ignored`` in playwright-core's own types: both
+return immediately and never wait. A call written as
+``isVisible({ timeout: resolveTimeout(10_000) })`` therefore measures the DOM
+at that instant, while reading to a human as if it waited ten seconds.
+
+Usually the argument is merely dead. Sometimes it hides a race: in
+``personas/admin.js`` the pattern judged an SPA ~10 ms after a credential
+submit and reported the still-in-flight login button as a failed OIDC token
+exchange, which is a false negative, not a flake.
+
+The rule does not prescribe the repair, because the two repairs differ in
+cost. Verify per site what the call was meant to do:
+
+* a presence check ("is it there right now?") keeps its exact meaning once
+  the dead argument is removed;
+* a call that was meant to wait needs ``waitFor({ state: "visible" })`` or
+  ``waitFor({ state: "hidden" })`` — which spends real wall-clock, multiplied
+  by the onion timeout factor, against a CI budget that already kills jobs.
+
+The sibling predicates ``isChecked``/``isDisabled``/``isEditable``/
+``isEnabled`` take a REAL timeout and are deliberately not matched.
+
+Per-line opt-out: ``// nocheck: playwright-ignored-timeout`` on the offending
+line or the immediately preceding one.
+"""
+
+from __future__ import annotations
+
+import re
+import unittest
+from pathlib import Path
+
+from utils.annotations.suppress import is_suppressed_at
+from utils.cache.files import iter_project_files, read_text
+
+from . import PROJECT_ROOT
+
+_RULE = "playwright-ignored-timeout"
+
+_IGNORED_TIMEOUT = re.compile(r"\.is(?:Visible|Hidden)\s*\(\s*\{[^}]*\btimeout\b")
+
+
+class TestPlaywrightIgnoredTimeout(unittest.TestCase):
+    def test_no_timeout_on_immediate_predicates(self) -> None:
+        offenders: list[str] = []
+        for path_str in iter_project_files(
+            extensions=(".js",),
+            exclude_tests=True,
+            exclude_dirs=("docs", "node_modules"),
+        ):
+            rel = Path(path_str).relative_to(PROJECT_ROOT).as_posix()
+            lines = read_text(path_str).splitlines()
+            for lineno, line in enumerate(lines, 1):
+                stripped = line.lstrip()
+                if stripped.startswith(("//", "*", "/*")):
+                    continue
+                if not _IGNORED_TIMEOUT.search(line):
+                    continue
+                if is_suppressed_at(lines, lineno, _RULE):
+                    continue
+                offenders.append(f"{rel}:{lineno}: {stripped}")
+
+        if offenders:
+            self.fail(
+                f"{len(offenders)} call(s) pass `timeout` to a Playwright "
+                "predicate that ignores it. isVisible()/isHidden() return "
+                "immediately (playwright-core types: `@deprecated This option "
+                "is ignored`), so each of these measures the DOM at that "
+                "instant while reading as if it waited. Most are probably dead "
+                "arguments; verify each site rather than sweeping. A presence "
+                "check keeps its meaning without the argument, whereas a call "
+                "that was meant to wait needs waitFor({state}) and costs real "
+                f"wall-clock. Deliberate instant checks take `// nocheck: {_RULE}` "
+                "on the offending or preceding line.\n" + "\n".join(offenders)
+            )
+
+
+if __name__ == "__main__":
+    unittest.main()