diff --git a/electron/main.js b/electron/main.js index f42d3d08..16495c53 100644 --- a/electron/main.js +++ b/electron/main.js @@ -1872,6 +1872,7 @@ function sendToRenderer(channel, ...args) { // Extracted to electron/updateErrorMessage.js so the mapping is unit-testable; see node --test there. const { friendlyUpdateError } = require('./updateErrorMessage'); const { experimentalChannelDecision } = require('./experimentalChannelDecision'); +const { popupRoute } = require('./popupRoute'); const { diagnoseSilentUpdateCheck } = require('./updateCheckDiagnosis'); // Squirrel's built-in updater reports only via events; when AV or a proxy kills its request @@ -2943,7 +2944,9 @@ app.on('web-contents-created', (_event, contents) => { } contents.setWindowOpenHandler(({ url, disposition }) => { - if (disposition === 'foreground-tab' || disposition === 'background-tab') { + // A popup opened by a browser card must become a card too, or it lands in a native window with + // no browser_id and the agent driving that card goes blind mid-auth (ENG-279). + if (popupRoute(contents.getType(), disposition) === 'card') { if (mainWindow && !mainWindow.isDestroyed()) { mainWindow.webContents.send('webview-new-window', url, contents.id, disposition); } diff --git a/electron/popupRoute.js b/electron/popupRoute.js new file mode 100644 index 00000000..317dfd4b --- /dev/null +++ b/electron/popupRoute.js @@ -0,0 +1,29 @@ +// Where should a window.open from inside the app end up? (ENG-279) +// +// Two very different callers share one handler: +// +// the MAIN window opening a provider sign-in (Anthropic's OAuth popup). That wants a real +// native child window and always has; it is app-level auth, nothing to do with browsing. +// +// a BROWSER CARD's guest page opening "Sign in with Google", an SSO handoff, a consent or +// payment screen. That used to spawn a native window too, which sits outside the browser-card +// system and therefore has no browser_id. An agent driving that card simply cannot see it: the +// flow stops dead at the popup and the run looks stalled for no visible reason. +// +// So the opener decides. A tab disposition was already routed into a card; now anything opened by +// a card goes to a card as well, which makes "a popup from a card that no agent can address" +// unrepresentable rather than merely unlikely. + +/** + * @param {string} contentsType webContents.getType(): 'webview' for a browser card's guest + * @param {string} disposition Electron's window-open disposition + * @returns {'card'|'native'} + */ +function popupRoute(contentsType, disposition) { + if (disposition === 'foreground-tab' || disposition === 'background-tab') return 'card'; + // Anything a browser card opens belongs to that card's world, whatever the disposition. + if (contentsType === 'webview') return 'card'; + return 'native'; +} + +module.exports = { popupRoute }; diff --git a/electron/popupRoute.test.js b/electron/popupRoute.test.js new file mode 100644 index 00000000..72418046 --- /dev/null +++ b/electron/popupRoute.test.js @@ -0,0 +1,32 @@ +// Run: node --test electron/popupRoute.test.js +// +// ENG-279. A "Sign in with Google" opened from inside a browser card used to become a native +// window with no browser_id, so an agent driving that card could not see or touch it and the run +// stalled with nothing on screen to explain why. +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { popupRoute } = require('./popupRoute'); + +test('a popup opened BY a browser card becomes a card, so an agent can address it', () => { + for (const d of ['new-window', 'default', 'other', undefined]) { + assert.equal(popupRoute('webview', d), 'card', `disposition ${d} escaped the card system`); + } +}); + +test('provider sign-in from the main window keeps its native popup', () => { + for (const d of ['new-window', 'default', undefined]) { + assert.equal(popupRoute('window', d), 'native', `disposition ${d} hijacked provider auth`); + } +}); + +test('tab dispositions still route to a card from anywhere', () => { + for (const t of ['webview', 'window', 'browserView']) { + assert.equal(popupRoute(t, 'foreground-tab'), 'card'); + assert.equal(popupRoute(t, 'background-tab'), 'card'); + } +}); + +test('both routes are reachable, so the decision is not one-armed', () => { + const seen = new Set([popupRoute('webview', 'new-window'), popupRoute('window', 'new-window')]); + assert.equal(seen.size, 2, `only reached ${[...seen].join(', ')}`); +});