From 4724556a59089c8c17d4254a44ebb09a3c576e43 Mon Sep 17 00:00:00 2001 From: Jon Vanausdeln Date: Wed, 29 Jul 2026 10:25:01 -0700 Subject: [PATCH 1/3] e2e: handle Databricks "Authorize as.." consent screen in OAuth flow Databricks added a consent screen at /oauth-prompt/select-group to the end of its OAuth flow. The Workbench managed-credentials test never got past it, and failed 30s later on an unrelated-looking assertion: expect(locator('[aria-label*="Databricks"][aria-label*="Enabled"]')) failed The direct cause was not the missing click but the success check. We waited on waitForURL(/oauth_redirect_callback|localhost:8787/) and the consent screen's own URL carries redirect_uri=http%3A%2F%2Flocalhost%3A8787%2Foauth_redirect_callback in its query string. Underscores aren't percent-encoded, so the substring regex matched while we were still parked on the consent screen. The trace shows the wait resolving in 2.2s of its 15s budget, then the OAuth tab being closed -- the flow was abandoned mid-consent while the logs reported an accepted OTP. Changes: - Match OAuth completion with a (url: URL) => boolean predicate on host, not a substring regex over the whole URL. Every step of an OAuth flow carries the callback URL in its query string, so substring matching is unsafe by construction. - Treat reaching either the consent screen or the callback as OTP-accepted, so the retry loop stops correctly on both paths. - Add confirmDatabricksAuthorizePrompt(): click Continue (a link, not a button; the group dropdown already preselects the service account's group), then wait for the callback. No-ops when Databricks skips the screen. Snowflake is unaffected -- it verifies via widget-state polling, and no other call site used the regex. --- test/e2e/pages/workbench/dashboard.page.ts | 59 +++++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/test/e2e/pages/workbench/dashboard.page.ts b/test/e2e/pages/workbench/dashboard.page.ts index fda623753fbd..f666b68f46b5 100644 --- a/test/e2e/pages/workbench/dashboard.page.ts +++ b/test/e2e/pages/workbench/dashboard.page.ts @@ -3,12 +3,27 @@ * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. *--------------------------------------------------------------------------------------------*/ -import { expect, BrowserContext } from '@playwright/test'; +import { expect, BrowserContext, Page } from '@playwright/test'; import { Code } from '../../infra/code.js'; import { QuickInput } from '../quickInput.js'; import { generateTOTP } from '../../utils/totp.js'; import { isOktaLockedOut, otpRetryDelayMs } from '../../utils/otpRetry.js'; +/** + * The OAuth flow is complete once the provider redirects back to Workbench. + * + * Matched on host rather than by searching the whole URL for the callback path: Databricks + * carries `redirect_uri=...%2Foauth_redirect_callback` in the query string of its own consent + * screen, so a substring match reports "complete" while still sitting on the provider's page. + */ +const isWorkbenchCallback = (url: URL) => url.host === 'localhost:8787'; + +/** + * Databricks ends its OAuth flow on an "Authorize as.." consent screen under `/oauth-prompt/` + * (currently `/oauth-prompt/select-group`) before redirecting to the callback. + */ +const isDatabricksAuthorizePrompt = (url: URL) => url.pathname.startsWith('/oauth-prompt/'); + export class DashboardPage { get title() { return this.code.driver.currentPage.getByRole('link', { name: 'Workbench projects' }); } get launchButton() { return this.code.driver.currentPage.getByRole('button', { name: 'Launch' }); } @@ -252,7 +267,13 @@ export class DashboardPage { await verifyOtpButton.click(); try { - await oauthPage.waitForURL(/oauth_redirect_callback|localhost:8787/, { timeout: 15000 }); + // Okta hands the flow back to Databricks, which either redirects straight to the + // callback or first shows its "Authorize as.." consent screen. Reaching either one + // means the OTP was accepted, so stop retrying and finish the flow below. + await oauthPage.waitForURL( + url => isWorkbenchCallback(url) || isDatabricksAuthorizePrompt(url), + { timeout: 15000 } + ); otpAccepted = true; break; } catch { @@ -274,6 +295,10 @@ export class DashboardPage { } } + if (otpAccepted && !oauthPage.isClosed()) { + await this.confirmDatabricksAuthorizePrompt(oauthPage); + } + try { if (otpAccepted) { await oauthPage.waitForTimeout(2000); @@ -290,6 +315,36 @@ export class DashboardPage { this.code.logger.log('Databricks OAuth setup complete'); } + /** + * Confirms the "Authorize as.." consent screen Databricks shows at the end of its OAuth flow. + * + * The screen preselects the group to authorize as (the service account belongs to one), so we + * only have to confirm it. "Continue" is rendered as a link, not a button. Databricks skips the + * screen in some cases (e.g. an existing grant), so a missing prompt is not a failure -- the + * caller's credential-widget assertion is the real verdict. + * + * @param oauthPage The OAuth tab, parked on the consent screen or already past it + */ + private async confirmDatabricksAuthorizePrompt(oauthPage: Page): Promise { + if (!isDatabricksAuthorizePrompt(new URL(oauthPage.url()))) { + this.code.logger.log('No Databricks "Authorize as.." prompt; OAuth flow continued without it'); + return; + } + + const continueLink = oauthPage.getByRole('link', { name: 'Continue', exact: true }); + await expect(continueLink).toBeVisible({ timeout: 15000 }); + await continueLink.click(); + this.code.logger.log('Confirmed Databricks "Authorize as.." prompt'); + + try { + await oauthPage.waitForURL(isWorkbenchCallback, { timeout: 15000 }); + } catch { + // The tab often closes itself the moment the callback lands, which cancels the wait. + // Defer to the caller's credential-widget check rather than failing here. + this.code.logger.log('Did not observe the OAuth callback after confirming; deferring to widget check'); + } + } + /** * Sets up Snowflake managed credential via OAuth flow * @param context BrowserContext for handling OAuth in new tab From 0ae827876301ee6fc93b33cf2247d28be9a29718 Mon Sep 17 00:00:00 2001 From: Jon Vanausdeln Date: Wed, 29 Jul 2026 12:21:10 -0700 Subject: [PATCH 2/3] e2e: restore a UI fallback for getMetadata in the Workbench lane #15163 rewrote sessions.getMetadata() to read session metadata through the internal `_positron.session.getMetadata` command instead of scraping the console info popup. Both halves of that path are gated on `--enable-smoke-test-driver`: - `window.driver`, the bridge executeCommand travels over (browser/window.ts) - the command itself, via registerCommandIfSmokeTestDriver (smokeTestCommands.ts) Our Electron and browser harnesses pass that flag themselves, but the Workbench lane connects to a server Posit Workbench launched, so neither exists there. Every Workbench test that starts an R or Python session then died in the fixture with: page.evaluate: TypeError: Cannot read properties of undefined (reading 'executeCommand') at Sessions.getMetadata That `window.driver` is absent in this lane isn't new -- saveWebClientLogs has been failing the same way for as long as the lane has existed, harmlessly, because nothing on the critical path used the driver. #15163 put it there. Restore the popup-scraping read as a fallback, selected by a new `driver.isDriverAvailable()` capability probe. The command path stays the default everywhere it works, so the flakiness #15163 removed does not come back for Electron/browser; only the Workbench lane pays the UI cost. The fallback body is a faithful restoration of the pre-#15163 logic, kept structurally identical (down to the console.focus() that getSessionCount() does on the way past) since that is the version proven against this lane. Note that isAlive() cannot serve as the probe: evaluateHandle('window.driver') resolves to a handle wrapping undefined rather than throwing. --- test/e2e/infra/playwrightDriver.ts | 22 ++++++++++ test/e2e/pages/sessions.ts | 68 +++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/test/e2e/infra/playwrightDriver.ts b/test/e2e/infra/playwrightDriver.ts index 41cc694ab566..01afa8ab34cf 100644 --- a/test/e2e/infra/playwrightDriver.ts +++ b/test/e2e/infra/playwrightDriver.ts @@ -755,6 +755,28 @@ export class PlaywrightDriver { [await this.getDriverHandle(), id, args] as const ); } + + /** + * Whether `window.driver` is present on the page, i.e. whether the smoke-test driver + * (and with it `executeCommand` and the `registerCommandIfSmokeTestDriver` commands) + * is available. + * + * The driver only registers when the app runs with `--enable-smoke-test-driver`, which + * our Electron and browser harnesses pass themselves. The Workbench lane connects to a + * server that Posit Workbench launched, so there is no such flag and no driver -- tests + * that would otherwise use the driver need a UI-based path there. Note that `isAlive()` + * cannot answer this: `evaluateHandle('window.driver')` resolves to an `undefined` + * handle rather than throwing. + */ + async isDriverAvailable(): Promise { + try { + return await this.page.evaluate(() => Reflect.get(window, 'driver') !== undefined); + } catch { + // Page navigating or closed; treat the driver as unavailable rather than throwing + // from what callers use as a capability probe. + return false; + } + } // --- End Positron --- private async getDriverHandle(): Promise> { diff --git a/test/e2e/pages/sessions.ts b/test/e2e/pages/sessions.ts index 61a45d35b63c..b0b2d3564502 100644 --- a/test/e2e/pages/sessions.ts +++ b/test/e2e/pages/sessions.ts @@ -753,11 +753,19 @@ export class Sessions { */ async getMetadata(sessionId?: string): Promise { return await test.step(`Get metadata for: ${sessionId ?? 'current session'}`, async () => { - // Read the metadata directly from the runtime session service via the + // Prefer reading the metadata directly from the runtime session service via the // internal `_positron.session.getMetadata` command (registered in // languageRuntimeActionsForSmokeTests.ts). This avoids selecting the // session tab and scraping the info popup, which is slower and flaky // when notification toasts overlay the tab. + // + // Both the command and the `executeCommand` bridge it travels over are gated on + // `--enable-smoke-test-driver`, which the Workbench lane never has: Posit Workbench + // launches the Positron server itself. Scrape the info popup there instead. + if (!await this.code.driver.isDriverAvailable()) { + return await this.getMetadataFromDialog(sessionId); + } + const metadata = await this.code.driver.executeCommand( '_positron.session.getMetadata', sessionId @@ -771,6 +779,64 @@ export class Sessions { }); } + /** + * Helper: Read session metadata by opening the console session info popup. + * + * Fallback for environments without the smoke-test driver -- see `getMetadata()`. Unlike + * the command-based read this drives the UI, so it has to bring the target session tab to + * the foreground first and dismiss the popup afterwards. + * + * @param sessionId the session ID to get metadata for, otherwise will use the current session + * @returns the metadata of the session + */ + private async getMetadataFromDialog(sessionId?: string): Promise { + // Kept structurally identical to the pre-command implementation (including the + // `console.focus()` that getSessionCount() does on the way past), since that is the + // version proven against the Workbench lane. + const isSingleSession = (await this.getSessionCount()) === 1; + + if (!isSingleSession && sessionId) { + const targetTab = this.getSessionTab(sessionId); + + await expect(async () => { + // Toasts render over the session tab bar, and their auto-dismiss timer + // is held open while the pointer sits on them -- keep the mouse clear. + await this.page.mouse.move(0, 0); + + // No force: let Playwright wait until the tab itself receives the + // click, rather than dispatching it into whatever is on top. Inner + // timeouts stay well under the outer budget, otherwise the first + // attempt consumes it and this never retries. + await targetTab.click({ timeout: 5000 }); + await expect(targetTab).toHaveClass(/tab-button--active/, { timeout: 2000 }); + }, `Select session tab: ${sessionId}`).toPass({ timeout: 30000 }); + } + + let metadata: SessionMetaData | undefined; + + await expect(async () => { + await this.openMetadataDialog(sessionId); + const [name, id, state, path, source] = await Promise.all([ + this.metadataDialog.getByTestId('session-name').textContent(), + this.metadataDialog.getByTestId('session-id').textContent(), + this.metadataDialog.getByTestId('session-state').textContent(), + this.metadataDialog.getByTestId('session-path').textContent(), + this.metadataDialog.getByTestId('session-source').textContent(), + ]); + metadata = { + name: (name ?? '').trim(), + id: (id ?? '').replace('Session ID: ', ''), + state: (state ?? '').replace('State: ', '') as SessionState, + path: (path ?? '').replace('Path: ', ''), + source: (source ?? '').replace('Source: ', ''), + }; + }, 'Extract session metadata').toPass({ intervals: [500], timeout: 10000 }); + + await this.page.keyboard.press('Escape'); + + return metadata!; + } + /** * Helper: Get all available runtimes with their categories */ From c0e51255bd261b06fcd4ed7e083c0cfe6945e705 Mon Sep 17 00:00:00 2001 From: Jon Vanausdeln Date: Wed, 29 Jul 2026 15:00:03 -0700 Subject: [PATCH 3/3] e2e: re-run CI to pick up updated test tags (+jupyter, -win)