From d11c1793a2e6b1dffbde90ec3def84187630203d Mon Sep 17 00:00:00 2001 From: Michiel de Jong Date: Tue, 29 Sep 2026 13:20:56 +0200 Subject: [PATCH] Redeem a proxy connect return once under StrictMode In dev, StrictMode ran ProxyConnectReturn's effect twice. The first run consumed the pending handoff and was cancelled before redirecting; the second found no handoff and showed "This connection return is not one this browser started." Share one redemption per integration_state so the second run awaits the first and redirects. Closes #1883 Co-Authored-By: Claude Opus 5.5 --- .../AppPage/ProxyConnectReturn.test.tsx | 136 ++++++++++++++++++ .../src/chunks/AppPage/ProxyConnectReturn.tsx | 44 +++++- 2 files changed, 175 insertions(+), 5 deletions(-) create mode 100644 browser/data-browser/src/chunks/AppPage/ProxyConnectReturn.test.tsx diff --git a/browser/data-browser/src/chunks/AppPage/ProxyConnectReturn.test.tsx b/browser/data-browser/src/chunks/AppPage/ProxyConnectReturn.test.tsx new file mode 100644 index 0000000000..8f6753a439 --- /dev/null +++ b/browser/data-browser/src/chunks/AppPage/ProxyConnectReturn.test.tsx @@ -0,0 +1,136 @@ +// @vitest-environment jsdom +// @wc-ignore-file +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import React, { StrictMode } from 'react'; +import { cleanup, render, screen, waitFor } from '@testing-library/react'; +import { StoreContext, type Store } from '@tomic/react'; + +/** + * #1883: under StrictMode the return effect ran twice. The first run consumed + * the pending handoff and was then cancelled; the second found nothing and + * showed "not one this browser started" instead of going back to the app. + */ + +/** Handoffs this "browser" started; `finish` consumes them, like the real one. */ +const started = new Set(); +const finish = vi.fn(); + +vi.mock('@helpers/integrationProxy', () => ({ + getIntegrationProxy: () => 'https://proxy.example', +})); + +vi.mock('@helpers/proxyConnections', () => ({ + ProxyConnections: class { + isReturn(params: URLSearchParams) { + return started.has(params.get('integration_state') ?? ''); + } + + finish(params: URLSearchParams) { + return finish(params); + } + }, +})); + +const { ProxyConnectReturn } = await import('./ProxyConnectReturn'); + +const store = { + getAgent: () => ({ subject: 'did:ad:agent:test' }), + on: () => () => undefined, +} as unknown as Store; + +const replace = vi.fn(); + +function renderReturn(state: string) { + // jsdom's `location.replace` can be neither spied on nor navigate. + vi.stubGlobal('location', { + pathname: '/app/integrations', + search: `?integration_state=${state}&platform=github&connection_code=c`, + replace, + }); + + return render( + + + +

app

+ + + , + ); +} + +beforeEach(() => { + started.clear(); + finish.mockReset(); + finish.mockImplementation(async (params: URLSearchParams) => { + const state = params.get('integration_state') ?? ''; + + if (!started.delete(state)) { + throw new Error( + 'This connection return is not one this browser started.', + ); + } + + return { returnTo: `/app/drive?after=${state}`, connected: true }; + }); + replace.mockReset(); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +describe('ProxyConnectReturn', () => { + it('redeems once and returns to the app under StrictMode', async () => { + started.add('s1'); + renderReturn('s1'); + + await waitFor(() => + expect(replace).toHaveBeenCalledWith('/app/drive?after=s1'), + ); + expect(finish).toHaveBeenCalledTimes(1); + expect(replace).toHaveBeenCalledTimes(1); + expect(screen.queryByRole('alert')).toBeNull(); + }); + + it('shows a failed redemption once', async () => { + started.add('s2'); + finish.mockImplementation(async () => { + throw new Error('The proxy refused the code.'); + }); + renderReturn('s2'); + + expect(await screen.findByRole('alert')).toHaveProperty( + 'textContent', + 'The proxy refused the code.', + ); + expect(finish).toHaveBeenCalledTimes(1); + expect(replace).not.toHaveBeenCalled(); + }); + + it('still refuses a return this browser did not start', async () => { + // Recognised at mount (say, another tab consumed it since), gone by the + // time it is redeemed. + started.add('s3'); + finish.mockImplementation(async () => { + throw new Error( + 'This connection return is not one this browser started.', + ); + }); + renderReturn('s3'); + + expect(await screen.findByRole('alert')).toHaveProperty( + 'textContent', + 'This connection return is not one this browser started.', + ); + expect(replace).not.toHaveBeenCalled(); + }); + + it('renders the app for a page load that is no return', () => { + renderReturn('unknown'); + + expect(screen.getByText('app')).toBeTruthy(); + expect(finish).not.toHaveBeenCalled(); + }); +}); diff --git a/browser/data-browser/src/chunks/AppPage/ProxyConnectReturn.tsx b/browser/data-browser/src/chunks/AppPage/ProxyConnectReturn.tsx index f1fc1d73d1..a10a9320c7 100644 --- a/browser/data-browser/src/chunks/AppPage/ProxyConnectReturn.tsx +++ b/browser/data-browser/src/chunks/AppPage/ProxyConnectReturn.tsx @@ -42,11 +42,8 @@ export function ProxyConnectReturn({ history.replaceState(history.state, '', location.pathname); let cancelled = false; - // Redeeming is signed with the user key, which may still be loading this - // early in a page load. - waitForAgent(store) - .then(() => pending.connections.finish(pending.params)) - .then(({ returnTo }) => { + redeemOnce(store, pending.connections, pending.params) + .then(returnTo => { if (!cancelled) location.replace(returnTo); }) .catch((e: Error) => { @@ -67,6 +64,43 @@ export function ProxyConnectReturn({ ); } +/** + * Redemptions in flight, by `integration_state`. `finish` consumes the pending + * handoff, so a second call for the same return always fails; StrictMode in + * development runs the effect twice, and the second run must wait for the + * first run's redemption rather than start its own (#1883). + */ +const redemptions = new Map>(); + +/** How long a settled redemption stays shared, for a remount right after. */ +const KEEP_SETTLED_MS = 10_000; + +function redeemOnce( + store: ReturnType, + connections: ProxyConnections, + params: URLSearchParams, +): Promise { + const state = params.get('integration_state') ?? ''; + const known = redemptions.get(state); + + if (known) return known; + + // Redeeming is signed with the user key, which may still be loading this + // early in a page load. + const redemption = waitForAgent(store) + .then(() => connections.finish(params)) + .then(({ returnTo }) => returnTo); + redemptions.set(state, redemption); + + const forget = () => + setTimeout(() => { + if (redemptions.get(state) === redemption) redemptions.delete(state); + }, KEEP_SETTLED_MS); + redemption.then(forget, forget); + + return redemption; +} + /** The signed-in agent, once there is one; gives up after 30 s. */ function waitForAgent(store: ReturnType): Promise { const now = store.getAgent();