Custom Flow - addOverlayMenu does not re-appear on SharePoint Online after Microsoft Authentication redirect

### Description When running a custom flow scan (`runCustom`) against a site that uses Microsoft Authentication — specifically **SharePoint Online** — the `addOverlayMenu` overlay does not re-appear after the user completes the Microsoft login flow and is redirected back to the SharePoint site. This is a regression in user experience for enterprise sites that sit behind Azure AD / MSAL-based authentication, as the user is left with no overlay and no way to trigger a scan after logging in. --- ### Steps to Reproduce 1. Run a custom flow scan (`runCustom`) with entry URL set to a SharePoint Online site 2. The browser opens and is immediately redirected to `login.microsoftonline.com` for authentication 3. Complete the Microsoft login (enter credentials, pass MFA if required) 4. Microsoft redirects back to the SharePoint site 5. Observe: the `addOverlayMenu` overlay **does not appear** on the final SharePoint page --- ### Expected Behaviour After authentication completes and the browser lands on the SharePoint site, the `addOverlayMenu` overlay should re-appear, allowing the user to scan the page. --- ### Actual Behaviour The overlay does not appear. The user is left on the SharePoint page with no overlay and no ability to initiate a scan without restarting the session. --- ### Root Cause Analysis The failure is caused by two compounding issues triggered by the **specific redirect chain** that SharePoint Online's MSAL implementation produces. #### The SharePoint Online Auth Redirect Chain Unlike most sites that perform a single login redirect and return, SharePoint Online produces **three** sequential navigations: ``` 1. connectnpedu.sharepoint.com/sites/ICT → 302 → login.microsoftonline.com 2. login.microsoftonline.com → (user logs in) → sharepoint.com/sites/ICT?code=… 3. sharepoint.com/sites/ICT?code=… → MSAL window.location.replace() → sharepoint.com/sites/ICT ``` Step 3 is the trigger. After the user logs in and is redirected back to SharePoint with an auth code (`?code=…`), SharePoint's MSAL library processes the token exchange at `DOMContentLoaded` and **immediately** calls `window.location.replace()` to strip the auth params from the URL. This triggers a third, unexpected navigation. --- #### Issue 1: `waitForLoadState` Inside `addOverlayMenu` Executes on the Wrong Page `addOverlayMenu` begins with the following call (`utils.ts:309`): ```ts await page.waitForLoadState('domcontentloaded'); ``` This line is intended to ensure the page is ready before injecting the overlay. However, `addOverlayMenu` is only ever called **from within** a `domcontentloaded` event handler, meaning the page is already at that state when the call is made. Under normal circumstances it resolves immediately and causes no harm. Under the SharePoint Online auth flow, however, the following sequence occurs on the `?code=…` URL: 1. `domcontentloaded` fires for `sharepoint.com/sites/ICT?code=…` 2. The handler calls `addOverlayMenu()`, which hits `waitForLoadState('domcontentloaded')` 3. **At this exact moment**, MSAL's `window.location.replace()` fires, starting a new navigation to the clean URL 4. Playwright resets the page's internal load state back to `loading` — the `?code=…` page is no longer considered to be at `domcontentloaded` 5. `waitForLoadState('domcontentloaded')` is now forced to **wait for the next page's `DOMContentLoaded`** — the clean SharePoint URL This leaves a **lingering `addOverlayMenu` call (Call A)** suspended mid-execution, waiting to resume on an entirely different page than the one that originally triggered it. --- #### Issue 2: Race Condition on the Final SharePoint Page When the clean SharePoint URL finishes loading, **two separate execution paths resume simultaneously**: | Call A — lingering from `?code=…` handler | Handler B — fresh handler for clean SharePoint URL | |---|---| | `waitForLoadState` unblocks on the new page's DCL | `domcontentloaded` event fires, handler starts | | Skips the `#oobeeShadowHost` existence check (that check was in the handler, not here) | Checks for `#oobeeShadowHost` at line 1293 — not found, calls `addOverlayMenu()` | | Proceeds directly to `page.evaluate()` to inject the overlay | `waitForLoadState` in its own `addOverlayMenu` resolves immediately, proceeds to `page.evaluate()` | | Injects `#oobeeShadowHost` into `document.body` | Also injects `#oobeeShadowHost` into `document.body` | The race occurs because the **duplicate check** for an existing overlay lives in the `domcontentloaded` handler at line 1293, not inside `addOverlayMenu` itself. By the time Handler B's check runs, Call A has not yet injected the overlay, so the check returns `null` and both proceed independently. This results in **two `#oobeeShadowHost` elements** being appended to `document.body`, which is invalid, and depending on execution order, can produce a broken or non-functional overlay state. --- #### Issue 3: Failures Are Silently Swallowed If either `page.evaluate()` call throws — for example with `"Execution context was destroyed"` when a navigation races ahead of the call — the error is caught at `utils.ts:1144`: ```ts .catch(error => { consoleLogger.error('Overlay menu: failed to add', error); }); ``` Critically, this `.catch()` **does not re-throw**. The `await addOverlayMenu(...)` call in the handler always resolves successfully regardless of whether the overlay was actually injected. This makes the failure completely invisible to the calling code and prevents any retry logic from being triggered. --- ### Why This Is Specific to SharePoint Online Most sites that use Microsoft Authentication perform a **single** redirect: the user is sent to `login.microsoftonline.com` and returned directly to the target site with an authenticated session. One navigation out, one navigation back — the `domcontentloaded` handler fires cleanly on the final page, `waitForLoadState` resolves immediately, and the overlay is injected without issue. SharePoint Online differs in two key ways: 1. **It produces a third navigation.** After the Microsoft login, the browser lands on `sharepoint.com/sites/ICT?code=…`. SharePoint's MSAL library uses `window.location.replace()` — not `history.replaceState()` — to clean up the auth params. `window.location.replace()` triggers a full Playwright navigation and resets the page load state. `history.replaceState()` does not, and would not cause this issue. 2. **The third navigation happens synchronously at `DOMContentLoaded`.** MSAL's token exchange and the subsequent `window.location.replace()` call occur as part of the page's initial script execution, before our async `domcontentloaded` handler has a chance to complete its `page.evaluate()` call. This is the window in which `waitForLoadState` gets "hijacked" to wait for the next page. --- ### Proposed Fix Remove `await page.waitForLoadState('domcontentloaded')` from `addOverlayMenu` at `utils.ts:309`. ```ts // Before export const addOverlayMenu = async (page, urlsCrawled, menuPos, opts = { ... }) => { await page.waitForLoadState('domcontentloaded'); // ← remove this ... }; // After export const addOverlayMenu = async (page, urlsCrawled, menuPos, opts = { ... }) => { // page is already at domcontentloaded — called from domcontentloaded handler ... }; ``` `addOverlayMenu` is only ever invoked from a `domcontentloaded` event handler or from within `handleOnScanClick` after a scan completes on the same live page. In both cases the page is already at the required load state. The `waitForLoadState` call is redundant in the normal case and actively harmful in the multi-redirect case, as it detaches the function from the page context that triggered it and opens the door to the race condition described above. --- ### Affected Files - `src/crawlers/custom/utils.ts:309` — spurious `waitForLoadState` inside `addOverlayMenu` - `src/crawlers/custom/utils.ts:1144` — silent `.catch()` in `addOverlayMenu` masks injection failures - `src/crawlers/custom/utils.ts:1283` — `domcontentloaded` handler that calls `addOverlayMenu`