e2e: unbreak the Workbench lane (Databricks consent screen + getMetadata driver fallback) - #15209
Open
jonvanausdeln wants to merge 3 commits into
Open
e2e: unbreak the Workbench lane (Databricks consent screen + getMetadata driver fallback)#15209jonvanausdeln wants to merge 3 commits into
jonvanausdeln wants to merge 3 commits into
Conversation
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.
|
E2E Tests 🚀 Why these tags?
More on automatic tags from changed files. |
#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.
jonvanausdeln
changed the title
jonvanausdeln
marked this pull request as ready for review
July 29, 2026 20:14
midleman
reviewed
Jul 29, 2026
| * @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<SessionMetaData> { |
Contributor
There was a problem hiding this comment.
oh no! it's all coming back? is there no way to make the command work on workbench? 😭
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent fixes, both needed to get the Workbench e2e lane green again.
1. Databricks added a consent screen to its OAuth flow
Databricks now ends its OAuth flow on an "Authorize as.." screen at
/oauth-prompt/select-group. The managed-credentials test never got past it, so theworkbench-databricksshard has been failing nightly (e.g.positron-builds run 29989485459)
with an assertion that points nowhere near the real problem:
The direct cause wasn't the missing click, it was the success check. We waited on:
and the consent screen's own URL carries the callback in its query string:
Underscores aren't percent-encoded, so
oauth_redirect_callbackappears literally and thesubstring regex matched while we were still sitting on the consent screen. The trace shows the
wait resolving in 2.2s of its 15s budget, then the tab being closed -- the flow was abandoned
mid-consent while the logs reported an accepted OTP.
In
test/e2e/pages/workbench/dashboard.page.ts:(url: URL) => booleanpredicate onurl.host, not a substringregex 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.
loop stops correctly on both paths.
confirmDatabricksAuthorizePrompt(): click Continue (a link, not a button; the groupdropdown 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.
2. #15163 broke every Workbench test that starts a session
With the OAuth fix in place the databricks shard got all the way to session startup and then hit
this, as did the
defaultandsnowflakeshards (14 failed / 18 flaky ondefault, which doesno OAuth at all):
#15163 rewrote
getMetadata()to read via the internal_positron.session.getMetadatacommand.Both halves of that path are gated on
--enable-smoke-test-driver:window.driver, the bridgeexecuteCommandtravels over (window.ts:330)registerCommandIfSmokeTestDriver(smokeTestCommands.ts:65)Our Electron and browser harnesses pass that flag themselves. The Workbench lane connects to a
server Posit Workbench launched, so neither exists there.
This isn't new --
saveWebClientLogshas failed the same way in this lane for as long as it hasexisted (visible in the 07-23 nightly log as
Cannot read properties of undefined (reading 'getLogs')), harmlessly, because nothing on the critical path used the driver. #15163 put it there.Fix: restore the popup-scraping read as a fallback, selected by a new
driver.isDriverAvailable()capability probe. The command path stays the default everywhere itworks, 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()thatgetSessionCount()does on the waypast) since that is the version proven against this lane.
driver.executeCommandhas exactly one caller, sogetMetadatais the whole blast radius.Carried in this PR rather than a separate one: Chris is out this week and this blocks the whole
Workbench lane, so it needs to land with the OAuth fix for either to be verifiable in CI. Reviewers
wanting just one of the two can review by commit --
4724556a59is the Databricks consent screen,0ae8278763is the getMetadata fallback.Release Notes
New Features
Bug Fixes
Validation Steps
@:workbench @:jupyter @:sessions @:web @:data-explorer
@:workbenchruns all four Workbench shards --databricksis the one the OAuth fix targets,and all of them exercise the
getMetadatafallback. Green on the previous run: databricks,default, snowflake and azure all passed, where default had been 14 failed / 18 flaky.
@:jupyteris here to check a second lane.app-jupyter,app-workbenchandapp-externalall reach Positron through an external server, so none of them get
--enable-smoke-test-driverand none of them have
window.driver. Jupyter-tagged tests such as data-explorer-r do start Rsessions through
sessions.start, so #15163 should have broken that lane the same way -- nonightly has run since it landed, so this is the first look. If so, the capability probe fixes
Jupyter for free.
@:sessions @:web @:data-explorermirrors what #15163 validated with, to confirm the capabilityprobe hasn't disturbed the command path in the lanes that do have the driver. The Windows tag is
dropped for turnaround time; electron, chromium and the four Workbench shards already passed on
the previous run. (Deliberately not naming that tag literally here --
pr-tags-parse.shgreps thewhole description, so even mentioning it in prose re-enables the lane.)
Note that
@:workbench-databrickson its own does not enable the lane --pr-tags-parse.shmatches a bare
@:workbenchand explicitly excludes the longer variants. Each shard then appliesits own hardcoded grep.