From e2b056709b86a372edf476090673307a25f5621d Mon Sep 17 00:00:00 2001 From: David May Date: Tue, 11 Aug 2026 10:34:17 +0200 Subject: [PATCH 1/5] feat(account-merge): handle the 202 job response from the confirmation link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The account-merge confirmation link answers HTTP 202 with a JobDto once the merge outruns the endpoint's wait window (DFXswiss/api#4496). `call()` resolves 202 through `response.ok` and never exposes the status code, so the screen took the ticket for a MergeResponseDto, read an undefined kycHash and left the user on the spinner indefinitely. Discriminate on the body instead: a ticket is polled via GET /job/:uid until it is terminal, then the merge endpoint is asked again for the result — the access token is issued in the HTTP context and deliberately not stored in the job. The polling budget is the job's own expectedSeconds, so it follows the API's group config rather than a second constant that would drift from it. Retry is not treated as terminal, a ticket that already carries Failed or DeadLetter is reported without polling, and polling stops when the screen unmounts. Refs #1304. Blocked on DFXswiss/api#4496. --- src/__tests__/account-merge.screen.test.tsx | 171 ++++++++++++++++++++ src/__tests__/job.test.ts | 139 ++++++++++++++++ src/screens/account-merge.screen.tsx | 99 +++++++++--- src/translations/languages/de.json | 2 + src/translations/languages/fr.json | 2 + src/translations/languages/it.json | 2 + src/util/job.ts | 71 ++++++++ 7 files changed, 462 insertions(+), 24 deletions(-) create mode 100644 src/__tests__/account-merge.screen.test.tsx create mode 100644 src/__tests__/job.test.ts create mode 100644 src/util/job.ts diff --git a/src/__tests__/account-merge.screen.test.tsx b/src/__tests__/account-merge.screen.test.tsx new file mode 100644 index 000000000..cd4aad7fa --- /dev/null +++ b/src/__tests__/account-merge.screen.test.tsx @@ -0,0 +1,171 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; + +const mockCall = jest.fn(); +const mockNavigate = jest.fn(); +const mockSetAuthToken = jest.fn(); + +jest.mock('@dfx.swiss/react', () => ({ + useApi: () => ({ call: mockCall }), + useAuthContext: () => ({ setAuthToken: mockSetAuthToken }), +})); + +jest.mock('@dfx.swiss/react-components', () => ({ + SpinnerSize: { LG: 'lg' }, + StyledButton: ({ label, onClick }: any) => , + StyledLoadingSpinner: () =>
, + StyledVerticalStack: ({ children }: any) =>
{children}
, +})); + +jest.mock('src/contexts/settings.context', () => ({ + useSettingsContext: () => ({ translate: (_key: string, text: string) => text }), +})); + +jest.mock('src/hooks/navigation.hook', () => ({ + useNavigation: () => ({ navigate: mockNavigate }), +})); + +jest.mock('src/hooks/layout-config.hook', () => ({ + useLayoutOptions: jest.fn(), +})); + +let mockUrlParams: URLSearchParams; +jest.mock('react-router-dom', () => ({ + useSearchParams: () => [mockUrlParams, jest.fn()], +})); + +import AccountMerge from '../screens/account-merge.screen'; +import { JobStatus } from '../util/job'; + +const MERGE_URL = 'auth/mail/confirm'; +const JOB = { uid: 'job-uid', expectedSeconds: 65 }; +// The screen polls once a second, so anything waiting on a poll has to outlast one full interval — +// the default 1000 ms of findBy/waitFor expires exactly as the first poll fires. +const POLL_TIMEOUT = 3000; + +/** Answers the merge endpoint and the job endpoint from two independent queues, by URL. */ +function respondWith({ merge = [] as any[], jobs = [] as any[] } = {}) { + const mergeQueue = [...merge]; + const jobQueue = [...jobs]; + + mockCall.mockImplementation(({ url }: { url: string }) => { + if (url.startsWith(MERGE_URL)) { + const next = mergeQueue.shift(); + return next instanceof Error ? Promise.reject(next) : Promise.resolve(next); + } + if (url.startsWith('job/')) { + const next = jobQueue.shift(); + return next ? Promise.resolve(next) : new Promise(() => undefined); + } + throw new Error(`unexpected url: ${url}`); + }); +} + +function mergeCalls(): number { + return mockCall.mock.calls.filter(([{ url }]) => url.startsWith(MERGE_URL)).length; +} + +beforeEach(() => { + jest.clearAllMocks(); + mockUrlParams = new URLSearchParams('otp=the-otp'); +}); + +describe('AccountMerge', () => { + it('shows the result directly when the merge finishes inside the wait window', async () => { + respondWith({ merge: [{ kycHash: 'hash', accessToken: 'token' }] }); + + render(); + + expect(await screen.findByText('Account merged successfully!')).toBeInTheDocument(); + expect(mockSetAuthToken).toHaveBeenCalledWith('token'); + expect(mergeCalls()).toBe(1); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + // The regression from the issue: a 202 used to leave the screen on the spinner forever. + it('polls the job and asks the merge endpoint again once it completes', async () => { + respondWith({ + merge: [ + { ...JOB, status: JobStatus.PENDING }, + { kycHash: 'hash', accessToken: 'token' }, + ], + jobs: [{ ...JOB, status: JobStatus.COMPLETE }], + }); + + render(); + + expect(await screen.findByText('Account merged successfully!', {}, { timeout: POLL_TIMEOUT })).toBeInTheDocument(); + expect(mockCall).toHaveBeenCalledWith({ url: 'job/job-uid', method: 'GET' }); + // The access token is only issued in the HTTP context, so the result has to be fetched again. + expect(mergeCalls()).toBe(2); + expect(mockSetAuthToken).toHaveBeenCalledWith('token'); + }); + + it('skips polling when the 202 ticket is already complete', async () => { + respondWith({ + merge: [{ ...JOB, status: JobStatus.COMPLETE }, { kycHash: 'hash' }], + }); + + render(); + + expect(await screen.findByText('Account merged successfully!')).toBeInTheDocument(); + expect(mockCall).not.toHaveBeenCalledWith(expect.objectContaining({ url: 'job/job-uid' })); + expect(mergeCalls()).toBe(2); + }); + + it('keeps showing the waiting state while the job is still running', async () => { + respondWith({ merge: [{ ...JOB, status: JobStatus.PENDING }] }); + + render(); + + await waitFor(() => expect(mockCall).toHaveBeenCalledWith({ url: 'job/job-uid', method: 'GET' }), { + timeout: POLL_TIMEOUT, + }); + expect(screen.getByText('Merging your accounts...')).toBeInTheDocument(); + expect(screen.queryByText('Account merged successfully!')).not.toBeInTheDocument(); + expect(mockNavigate).not.toHaveBeenCalled(); + }); + + it('reports a failed job with the message the API supplied', async () => { + respondWith({ + merge: [{ ...JOB, status: JobStatus.FAILED, error: 'Job job-uid failed, contact support if this persists.' }], + }); + + render(); + + await waitFor(() => + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/error', + search: 'msg=Job job-uid failed, contact support if this persists.', + }), + ); + expect(mergeCalls()).toBe(1); + }); + + it('falls back to a translated message when a dead-lettered job carries none', async () => { + respondWith({ merge: [{ ...JOB, status: JobStatus.DEAD_LETTER }] }); + + render(); + + await waitFor(() => + expect(mockNavigate).toHaveBeenCalledWith({ pathname: '/error', search: 'msg=Account merge failed' }), + ); + }); + + it('still maps the synchronous error codes', async () => { + respondWith({ merge: [Object.assign(new Error('nope'), { statusCode: 400 })] }); + + render(); + + await waitFor(() => expect(mockNavigate).toHaveBeenCalledWith({ pathname: '/error', search: 'msg=Invalid link' })); + }); + + it('redirects to KYC without an otp', () => { + mockUrlParams = new URLSearchParams(); + + render(); + + expect(mockNavigate).toHaveBeenCalledWith('/kyc'); + expect(mockCall).not.toHaveBeenCalled(); + }); +}); diff --git a/src/__tests__/job.test.ts b/src/__tests__/job.test.ts new file mode 100644 index 000000000..693f9714a --- /dev/null +++ b/src/__tests__/job.test.ts @@ -0,0 +1,139 @@ +// Mock @dfx.swiss/react to avoid ES module issues (src/util/job imports delay from src/util/utils) +jest.mock('@dfx.swiss/react', () => ({})); +jest.mock('src/dto/safe.dto', () => ({})); + +import { JobResponse, JobStatus, isJobResponse, isJobTerminal, pollJobUntilTerminal } from '../util/job'; + +function job(status: JobStatus, overrides: Partial = {}): JobResponse { + return { uid: 'job-uid', status, expectedSeconds: 65, ...overrides }; +} + +afterEach(() => { + jest.restoreAllMocks(); +}); + +describe('isJobResponse', () => { + it('detects a 202 job ticket', () => { + expect(isJobResponse(job(JobStatus.PENDING))).toBe(true); + }); + + it('rejects the 200 merge result', () => { + expect(isJobResponse({ kycHash: 'hash', accessToken: 'token' })).toBe(false); + }); + + it('rejects a merge result that carries no access token', () => { + expect(isJobResponse({ kycHash: 'hash' })).toBe(false); + }); + + it('rejects a body that is not an object', () => { + expect(isJobResponse(null)).toBe(false); + expect(isJobResponse(undefined)).toBe(false); + expect(isJobResponse('Complete')).toBe(false); + expect(isJobResponse({})).toBe(false); + }); +}); + +describe('isJobTerminal', () => { + it.each([[JobStatus.COMPLETE], [JobStatus.FAILED], [JobStatus.DEAD_LETTER]])('treats %s as terminal', (status) => { + expect(isJobTerminal(status)).toBe(true); + }); + + // Retry means the last attempt failed but the job will run again — polling must not stop there. + it.each([[JobStatus.PENDING], [JobStatus.PROCESSING], [JobStatus.RETRY]])('treats %s as still running', (status) => { + expect(isJobTerminal(status)).toBe(false); + }); +}); + +describe('pollJobUntilTerminal', () => { + it('returns an already terminal ticket without asking the API', async () => { + const fetchJob = jest.fn(); + + const result = await pollJobUntilTerminal(job(JobStatus.COMPLETE), fetchJob, { intervalSeconds: 0 }); + + expect(result.status).toBe(JobStatus.COMPLETE); + expect(fetchJob).not.toHaveBeenCalled(); + }); + + it('polls until the job completes', async () => { + const fetchJob = jest + .fn() + .mockResolvedValueOnce(job(JobStatus.PROCESSING)) + .mockResolvedValueOnce(job(JobStatus.COMPLETE)); + + const result = await pollJobUntilTerminal(job(JobStatus.PENDING), fetchJob, { intervalSeconds: 0 }); + + expect(result.status).toBe(JobStatus.COMPLETE); + expect(fetchJob).toHaveBeenCalledTimes(2); + expect(fetchJob).toHaveBeenCalledWith('job-uid'); + }); + + it('keeps polling while the job is retrying', async () => { + const fetchJob = jest + .fn() + .mockResolvedValueOnce(job(JobStatus.RETRY)) + .mockResolvedValueOnce(job(JobStatus.COMPLETE)); + + const result = await pollJobUntilTerminal(job(JobStatus.PROCESSING), fetchJob, { intervalSeconds: 0 }); + + expect(result.status).toBe(JobStatus.COMPLETE); + expect(fetchJob).toHaveBeenCalledTimes(2); + }); + + it('stops on a failed job and keeps its error', async () => { + const fetchJob = jest.fn().mockResolvedValue(job(JobStatus.FAILED, { error: 'Job job-uid failed' })); + + const result = await pollJobUntilTerminal(job(JobStatus.PENDING), fetchJob, { intervalSeconds: 0 }); + + expect(result.status).toBe(JobStatus.FAILED); + expect(result.error).toBe('Job job-uid failed'); + expect(fetchJob).toHaveBeenCalledTimes(1); + }); + + it('gives up, still running, once the expectedSeconds budget is spent', async () => { + // Deterministic clock: the first reading fixes the deadline, the last one is past it. + jest + .spyOn(Date, 'now') + .mockReturnValueOnce(0) + .mockReturnValueOnce(1_000) + .mockReturnValueOnce(2_000) + .mockReturnValue(99_000); + const fetchJob = jest.fn().mockResolvedValue(job(JobStatus.PROCESSING)); + + const result = await pollJobUntilTerminal(job(JobStatus.PENDING, { expectedSeconds: 10 }), fetchJob, { + intervalSeconds: 0, + }); + + expect(result.status).toBe(JobStatus.PROCESSING); + expect(fetchJob).toHaveBeenCalledTimes(2); + }); + + it('stops asking once the caller has cancelled', async () => { + let cancelled = false; + const fetchJob = jest.fn().mockImplementation(() => { + cancelled = true; + return Promise.resolve(job(JobStatus.PROCESSING)); + }); + + const result = await pollJobUntilTerminal(job(JobStatus.PENDING), fetchJob, { + intervalSeconds: 0, + isCancelled: () => cancelled, + }); + + expect(result.status).toBe(JobStatus.PROCESSING); + expect(fetchJob).toHaveBeenCalledTimes(1); + }); + + // A screen unmounted before the first interval elapses must not keep a timer alive for a whole + // interval: the long interval here would stall the test if the loop started waiting regardless. + it('returns straight away when it is cancelled before the first poll', async () => { + const fetchJob = jest.fn(); + + const result = await pollJobUntilTerminal(job(JobStatus.PENDING), fetchJob, { + intervalSeconds: 30, + isCancelled: () => true, + }); + + expect(result.status).toBe(JobStatus.PENDING); + expect(fetchJob).not.toHaveBeenCalled(); + }); +}); diff --git a/src/screens/account-merge.screen.tsx b/src/screens/account-merge.screen.tsx index ec2ffbf43..d2582ba35 100644 --- a/src/screens/account-merge.screen.tsx +++ b/src/screens/account-merge.screen.tsx @@ -1,16 +1,21 @@ import { ApiError, useApi, useAuthContext } from '@dfx.swiss/react'; import { SpinnerSize, StyledButton, StyledLoadingSpinner, StyledVerticalStack } from '@dfx.swiss/react-components'; -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { useSearchParams } from 'react-router-dom'; import { useSettingsContext } from 'src/contexts/settings.context'; import { useLayoutOptions } from 'src/hooks/layout-config.hook'; import { useNavigation } from 'src/hooks/navigation.hook'; +import { JobResponse, JobStatus, isJobResponse, isJobTerminal, pollJobUntilTerminal } from 'src/util/job'; interface MergeRedirect { kycHash: string; accessToken?: string; } +// The merge ran as a job and did not end in a usable result. Carries an already user-facing message, +// which is what tells it apart from an ApiError in the catch below. +class MergeJobError extends Error {} + export default function AccountMerge() { const { translate } = useSettingsContext(); const { setAuthToken } = useAuthContext(); @@ -19,37 +24,83 @@ export default function AccountMerge() { const [urlParams, setUrlParams] = useSearchParams(); const [kycHash, setKycHash] = useState(); + const isCancelled = useRef(false); const otp = urlParams.get('otp'); useEffect(() => { - if (otp) { - urlParams.delete('otp'); - setUrlParams(urlParams); - - call({ - url: `auth/mail/confirm?code=${otp}`, - method: 'GET', - }) - .then(({ kycHash, accessToken }: MergeRedirect) => { - setAuthToken(accessToken); - setKycHash(kycHash); - }) - .catch((error: ApiError) => { - const errorMessage = - error.statusCode === 400 - ? translate('screens/error', 'Invalid link') - : error.statusCode === 409 - ? translate('screens/error', 'Merge is already completed') - : error.message; - - navigate({ pathname: '/error', search: `msg=${errorMessage}` }); - }); - } else { + if (!otp) { navigate('/kyc'); + return; } + + urlParams.delete('otp'); + setUrlParams(urlParams); + + mergeAccounts(otp) + .then(({ kycHash, accessToken }: MergeRedirect) => { + if (isCancelled.current) return; + + setAuthToken(accessToken); + setKycHash(kycHash); + }) + .catch((error: ApiError | MergeJobError) => { + if (isCancelled.current) return; + + const errorMessage = + error instanceof MergeJobError + ? error.message + : error.statusCode === 400 + ? translate('screens/error', 'Invalid link') + : error.statusCode === 409 + ? translate('screens/error', 'Merge is already completed') + : error.message; + + navigate({ pathname: '/error', search: `msg=${errorMessage}` }); + }); + + return () => { + isCancelled.current = true; + }; }, []); + function confirmMerge(otp: string): Promise { + return call({ url: `auth/mail/confirm?code=${otp}`, method: 'GET' }); + } + + async function mergeAccounts(otp: string): Promise { + const response = await confirmMerge(otp); + if (!isJobResponse(response)) return response; + + // 202: the merge outran the endpoint's wait window and continues as a job. A ticket that already + // carries a terminal status is returned unpolled, so this covers that case too. + const job = await pollJobUntilTerminal(response, (uid) => call({ url: `job/${uid}`, method: 'GET' }), { + isCancelled: () => isCancelled.current, + }); + + if (job.status !== JobStatus.COMPLETE) throw mergeJobError(job); + + // The access token is issued in the HTTP context and never stored in the job, so the result has to + // come from the merge endpoint itself. The same otp maps to the same job, so this returns the + // finished merge instead of starting a second one. + const result = await confirmMerge(otp); + if (isJobResponse(result)) throw mergeJobError(result); + + return result; + } + + function mergeJobError(job: JobResponse): MergeJobError { + // Still running: the job may yet succeed, so this is a "come back later", not a failure. + if (!isJobTerminal(job.status)) + return new MergeJobError( + translate('screens/error', 'Merging your accounts is taking longer than expected. Please try again later.'), + ); + + // Failed carries a generic support hint from the API, DeadLetter a domain reason — both are meant + // for the user, so they are shown as-is instead of being mapped to a status code. + return new MergeJobError(job.error ?? translate('screens/error', 'Account merge failed')); + } + useLayoutOptions({}); return ( diff --git a/src/translations/languages/de.json b/src/translations/languages/de.json index ef9bfec00..fa63ca973 100644 --- a/src/translations/languages/de.json +++ b/src/translations/languages/de.json @@ -485,6 +485,8 @@ "Invalid link": "Ungültiger Link", "Merge is already completed": "Zusammenführung ist bereits abgeschlossen", + "Merging your accounts is taking longer than expected. Please try again later.": "Die Zusammenführung Deiner Konten dauert länger als erwartet. Bitte versuche es später erneut.", + "Account merge failed": "Zusammenführung der Konten fehlgeschlagen", "Please return to the previous page. If this problem persists, please contact our support.": "Bitte kehre zur vorherigen Seite zurück. Wenn dieses Problem weiterhin besteht, wende Dich bitte an unseren Support.", "Please reload this page. If this problem persists, please contact our support.": "Bitte lade diese Seite neu. Wenn dieses Problem weiterhin besteht, wende Dich bitte an unseren Support." }, diff --git a/src/translations/languages/fr.json b/src/translations/languages/fr.json index 3324e6ca0..17fd2472c 100644 --- a/src/translations/languages/fr.json +++ b/src/translations/languages/fr.json @@ -485,6 +485,8 @@ "Invalid link": "Lien invalide", "Merge is already completed": "La fusion est déjà terminée", + "Merging your accounts is taking longer than expected. Please try again later.": "La fusion de vos comptes prend plus de temps que prévu. Veuillez réessayer plus tard.", + "Account merge failed": "Échec de la fusion des comptes", "Please return to the previous page. If this problem persists, please contact our support.": "Veuillez revenir à la page précédente. Si le problème persiste, veuillez contacter notre service d'assistance.", "Please reload this page. If this problem persists, please contact our support.": "Veuillez recharger cette page. Si le problème persiste, veuillez contacter notre service d'assistance." }, diff --git a/src/translations/languages/it.json b/src/translations/languages/it.json index 33ab045d6..1be23c2d2 100644 --- a/src/translations/languages/it.json +++ b/src/translations/languages/it.json @@ -485,6 +485,8 @@ "Invalid link": "Link non valido", "Merge is already completed": "La fusione è già completata", + "Merging your accounts is taking longer than expected. Please try again later.": "La fusione dei tuoi account sta richiedendo più tempo del previsto. Riprova più tardi.", + "Account merge failed": "Fusione degli account non riuscita", "Please return to the previous page. If this problem persists, please contact our support.": "Tornare alla pagina precedente. Se il problema persiste, contattare l'assistenza.", "Please reload this page. If this problem persists, please contact our support.": "Ricaricare questa pagina. Se il problema persiste, contattare l'assistenza." }, diff --git a/src/util/job.ts b/src/util/job.ts new file mode 100644 index 000000000..69babde3e --- /dev/null +++ b/src/util/job.ts @@ -0,0 +1,71 @@ +import { delay } from './utils'; + +/** + * Async job contract of the API (DFXswiss/api#4496): an endpoint whose work outruns its short wait + * window answers HTTP 202 with a job ticket instead of the result. The client polls `GET /job/:uid` + * until the job is terminal and then asks the originating endpoint for the result again — the + * result is deliberately not carried in the job. + */ +export enum JobStatus { + PENDING = 'Pending', + PROCESSING = 'Processing', + COMPLETE = 'Complete', + RETRY = 'Retry', + FAILED = 'Failed', + DEAD_LETTER = 'DeadLetter', +} + +export interface JobResponse { + uid: string; + status: JobStatus; + expectedSeconds: number; + error?: string; +} + +// Retry is deliberately not terminal: the attempt failed but attempts remain and the job is waiting +// to run again. +const terminalStatus: JobStatus[] = [JobStatus.COMPLETE, JobStatus.FAILED, JobStatus.DEAD_LETTER]; + +export function isJobTerminal(status: JobStatus): boolean { + return terminalStatus.includes(status); +} + +/** + * Tells a 202 job ticket apart from the 200 result body. `useApi().call` resolves both through + * `response.ok` and never exposes the status code, so the body itself has to be the discriminator. + */ +export function isJobResponse(response: unknown): response is JobResponse { + const job = response as JobResponse | null | undefined; + return typeof job?.uid === 'string' && typeof job?.status === 'string'; +} + +interface PollJobOptions { + intervalSeconds?: number; + isCancelled?: () => boolean; +} + +/** + * Polls until the job reaches a terminal state, or until its own time budget is spent. + * + * The budget is `expectedSeconds` — the group's queue time plus run time — so the client follows the + * API's configuration instead of a second constant that would drift from it. A job that is still + * running when the budget is spent, or one abandoned through `isCancelled`, is returned as-is and + * therefore non-terminal; the caller decides how to present that. + */ +export async function pollJobUntilTerminal( + job: JobResponse, + fetchJob: (uid: string) => Promise, + { intervalSeconds = 1, isCancelled = () => false }: PollJobOptions = {}, +): Promise { + const deadline = Date.now() + job.expectedSeconds * 1000; + let current = job; + + while (!isJobTerminal(current.status) && Date.now() < deadline && !isCancelled()) { + await delay(intervalSeconds); + if (isCancelled()) break; + + current = await fetchJob(current.uid); + } + + return current; +} From b2251f97b216e46f01a418f59f62e09a91a7b3b5 Mon Sep 17 00:00:00 2001 From: David May Date: Tue, 11 Aug 2026 10:41:16 +0200 Subject: [PATCH 2/5] fix(account-merge): re-arm the cancellation flag when the effect re-runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The effect cleanup sets isCancelled so an unmounted screen stops polling and discards its result. The flag was never cleared again, so a second invocation on the same component instance — which is what StrictMode does — would run the merge and then throw the result away, leaving the spinner up forever: exactly the failure this screen was changed to stop producing. The app does not enable StrictMode today, so this is latent rather than live. Pinned by a test that renders the screen inside StrictMode; removing the re-arm turns it red. --- src/__tests__/account-merge.screen.test.tsx | 15 +++++++++++++++ src/screens/account-merge.screen.tsx | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/src/__tests__/account-merge.screen.test.tsx b/src/__tests__/account-merge.screen.test.tsx index cd4aad7fa..542c2ea07 100644 --- a/src/__tests__/account-merge.screen.test.tsx +++ b/src/__tests__/account-merge.screen.test.tsx @@ -168,4 +168,19 @@ describe('AccountMerge', () => { expect(mockNavigate).toHaveBeenCalledWith('/kyc'); expect(mockCall).not.toHaveBeenCalled(); }); + + // StrictMode runs the effect, its cleanup, then the effect again on the same instance — so the + // cancellation flag has to be re-armed, or the second run would discard its own result and strand + // the user on the spinner, which is the very failure this screen is meant to stop doing. + it('still completes when the effect is invoked twice on the same instance', async () => { + respondWith({ merge: [{ kycHash: 'hash', accessToken: 'token' }, { kycHash: 'hash', accessToken: 'token' }] }); + + render( + + + , + ); + + expect(await screen.findByText('Account merged successfully!')).toBeInTheDocument(); + }); }); diff --git a/src/screens/account-merge.screen.tsx b/src/screens/account-merge.screen.tsx index d2582ba35..af0411538 100644 --- a/src/screens/account-merge.screen.tsx +++ b/src/screens/account-merge.screen.tsx @@ -29,6 +29,10 @@ export default function AccountMerge() { const otp = urlParams.get('otp'); useEffect(() => { + // The cleanup below cancels the run; re-arm here so a second invocation on the same instance + // (StrictMode double-invokes effects) starts a live run instead of one that discards its result. + isCancelled.current = false; + if (!otp) { navigate('/kyc'); return; From 9b708e25e99942381495084110edf98910245d64 Mon Sep 17 00:00:00 2001 From: David May Date: Tue, 11 Aug 2026 10:49:11 +0200 Subject: [PATCH 3/5] test(account-merge): pin that an unmounted screen discards the merge result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The isCancelled guard in the success handler was unverified: removing it left the whole suite green. It is not a cosmetic guard — setAuthToken writes the global auth context rather than component state, so a merge landing after the user navigated away would sign them in from a screen that no longer exists. Resolves the merge from a deferred promise after unmounting and asserts the token is never set; removing the guard now turns the test red. --- src/__tests__/account-merge.screen.test.tsx | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/__tests__/account-merge.screen.test.tsx b/src/__tests__/account-merge.screen.test.tsx index 542c2ea07..b5f43902e 100644 --- a/src/__tests__/account-merge.screen.test.tsx +++ b/src/__tests__/account-merge.screen.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, waitFor } from '@testing-library/react'; +import { act, render, screen, waitFor } from '@testing-library/react'; import React from 'react'; const mockCall = jest.fn(); @@ -160,6 +160,22 @@ describe('AccountMerge', () => { await waitFor(() => expect(mockNavigate).toHaveBeenCalledWith({ pathname: '/error', search: 'msg=Invalid link' })); }); + // setAuthToken writes global auth context, not component state, so a merge that lands after the + // user has navigated away has to be dropped rather than silently signing them in. + it('discards the result when the screen unmounts before the merge lands', async () => { + let resolveMerge!: (result: unknown) => void; + mockCall.mockImplementation(() => new Promise((resolve) => (resolveMerge = resolve))); + + const { unmount } = render(); + unmount(); + + await act(async () => { + resolveMerge({ kycHash: 'hash', accessToken: 'token' }); + }); + + expect(mockSetAuthToken).not.toHaveBeenCalled(); + }); + it('redirects to KYC without an otp', () => { mockUrlParams = new URLSearchParams(); From 9f5aab649ccbf59b8d16a428b79bcffe9a90ccd1 Mon Sep 17 00:00:00 2001 From: David May Date: Tue, 11 Aug 2026 11:01:36 +0200 Subject: [PATCH 4/5] test(account-merge): bring both touched files to 100 % coverage CONTRIBUTING now requires 100 % statement, branch, function and line coverage for every file a pull request touches. Measured after the rebase, the screen sat at 95.55 % statements / 79.16 % branches and the helper at 92.3 % / 80 %. Covers what was missing rather than what was convenient: the poll helper's option defaults and the break that fires when cancellation lands during the wait, and on the screen the 409 mapping, the pass-through for an unmapped error, the budget-exhausted message, the account button, the catch-side cancellation guard, and a result call that answers with another job ticket. Both files now measure 100 % on all four metrics. --- src/__tests__/account-merge.screen.test.tsx | 79 ++++++++++++++++++++- src/__tests__/job.test.ts | 24 +++++++ 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/src/__tests__/account-merge.screen.test.tsx b/src/__tests__/account-merge.screen.test.tsx index b5f43902e..a19dbb4ef 100644 --- a/src/__tests__/account-merge.screen.test.tsx +++ b/src/__tests__/account-merge.screen.test.tsx @@ -1,4 +1,4 @@ -import { act, render, screen, waitFor } from '@testing-library/react'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import React from 'react'; const mockCall = jest.fn(); @@ -126,6 +126,30 @@ describe('AccountMerge', () => { expect(mockNavigate).not.toHaveBeenCalled(); }); + // Budget spent while the job is still running: it may yet succeed, so the user is told to come + // back rather than that the merge failed. expectedSeconds 0 exhausts the budget immediately. + it('tells the user to come back later when the job outlasts its budget', async () => { + respondWith({ merge: [{ ...JOB, expectedSeconds: 0, status: JobStatus.PENDING }] }); + + render(); + + await waitFor(() => + expect(mockNavigate).toHaveBeenCalledWith({ + pathname: '/error', + search: 'msg=Merging your accounts is taking longer than expected. Please try again later.', + }), + ); + }); + + it('sends the user to their account with the kycHash', async () => { + respondWith({ merge: [{ kycHash: 'hash', accessToken: 'token' }] }); + + render(); + fireEvent.click(await screen.findByRole('button', { name: 'My account' })); + + expect(mockNavigate).toHaveBeenCalledWith('/account?code=hash'); + }); + it('reports a failed job with the message the API supplied', async () => { respondWith({ merge: [{ ...JOB, status: JobStatus.FAILED, error: 'Job job-uid failed, contact support if this persists.' }], @@ -160,6 +184,59 @@ describe('AccountMerge', () => { await waitFor(() => expect(mockNavigate).toHaveBeenCalledWith({ pathname: '/error', search: 'msg=Invalid link' })); }); + it('maps a 409 to the already-completed message', async () => { + respondWith({ merge: [Object.assign(new Error('nope'), { statusCode: 409 })] }); + + render(); + + await waitFor(() => + expect(mockNavigate).toHaveBeenCalledWith({ pathname: '/error', search: 'msg=Merge is already completed' }), + ); + }); + + it('passes through an error it has no mapping for', async () => { + respondWith({ merge: [Object.assign(new Error('Network error: down'), { statusCode: 500 })] }); + + render(); + + await waitFor(() => + expect(mockNavigate).toHaveBeenCalledWith({ pathname: '/error', search: 'msg=Network error: down' }), + ); + }); + + // The result call answering with another ticket would mean the merge un-completed itself; report + // it rather than handing the render an object with no kycHash. + it('reports a failure when the result call returns another ticket', async () => { + respondWith({ + merge: [ + { ...JOB, status: JobStatus.PENDING }, + { ...JOB, status: JobStatus.FAILED, error: 'Job job-uid failed' }, + ], + jobs: [{ ...JOB, status: JobStatus.COMPLETE }], + }); + + render(); + + await waitFor( + () => expect(mockNavigate).toHaveBeenCalledWith({ pathname: '/error', search: 'msg=Job job-uid failed' }), + { timeout: POLL_TIMEOUT }, + ); + }); + + it('stays silent when the screen unmounts before the merge fails', async () => { + let rejectMerge!: (error: unknown) => void; + mockCall.mockImplementation(() => new Promise((_, reject) => (rejectMerge = reject))); + + const { unmount } = render(); + unmount(); + + await act(async () => { + rejectMerge(Object.assign(new Error('nope'), { statusCode: 400 })); + }); + + expect(mockNavigate).not.toHaveBeenCalled(); + }); + // setAuthToken writes global auth context, not component state, so a merge that lands after the // user has navigated away has to be dropped rather than silently signing them in. it('discards the result when the screen unmounts before the merge lands', async () => { diff --git a/src/__tests__/job.test.ts b/src/__tests__/job.test.ts index 693f9714a..07bde11f6 100644 --- a/src/__tests__/job.test.ts +++ b/src/__tests__/job.test.ts @@ -136,4 +136,28 @@ describe('pollJobUntilTerminal', () => { expect(result.status).toBe(JobStatus.PENDING); expect(fetchJob).not.toHaveBeenCalled(); }); + + // Cancellation that lands while the interval is being waited out must stop the next request, + // not just the next loop pass. + it('stops before fetching when cancellation lands during the wait', async () => { + const fetchJob = jest.fn(); + let checks = 0; + + const result = await pollJobUntilTerminal(job(JobStatus.PENDING), fetchJob, { + intervalSeconds: 0, + isCancelled: () => checks++ > 0, + }); + + expect(result.status).toBe(JobStatus.PENDING); + expect(fetchJob).not.toHaveBeenCalled(); + }); + + it('falls back to its own interval and cancellation defaults when given no options', async () => { + const fetchJob = jest.fn().mockResolvedValue(job(JobStatus.COMPLETE)); + + const result = await pollJobUntilTerminal(job(JobStatus.PENDING), fetchJob); + + expect(result.status).toBe(JobStatus.COMPLETE); + expect(fetchJob).toHaveBeenCalledTimes(1); + }); }); From 607892f58c76b8f281526e3856683804f5d3f38f Mon Sep 17 00:00:00 2001 From: David May Date: Tue, 11 Aug 2026 14:49:26 +0200 Subject: [PATCH 5/5] fix(account-merge): poll the job unauthenticated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by running the change against the real stack: api#4496 plus this frontend, with the endpoint's wait window shortened so every merge takes the 202 path. The API log read GET /v1/auth/mail/confirm 202 GET /v1/job/J372B31EAFABE44B5 404 and the screen sent the user to /error while the job was sitting in Postgres, Complete. The job is enqueued against the merge's master account, but whoever follows the confirmation link is still signed in as the slave. `useApi().call` attaches that session token, and the API's ownership guard — `jwt.account !== job.userData.id` — then 404s its own caller. Polling with `token: false` sends no Authorization header, which is the trust model the endpoint documents for this case: the uid is a random value known only to whoever triggered the job, the same level as the link that created it. Confirmed against the running API — the identical uid answers 404 with the session token and 200 without it. The merge call itself keeps its token; that is what the fresh access token is issued from. Both halves are pinned by a test. Re-ran the full-stack spec afterwards: 202 -> poll -> re-call -> 200, UI shows the success state, Postgres shows the merge completed. --- src/__tests__/account-merge.screen.test.tsx | 27 +++++++++++++++++++-- src/screens/account-merge.screen.tsx | 13 +++++++--- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/__tests__/account-merge.screen.test.tsx b/src/__tests__/account-merge.screen.test.tsx index a19dbb4ef..d2f201fd1 100644 --- a/src/__tests__/account-merge.screen.test.tsx +++ b/src/__tests__/account-merge.screen.test.tsx @@ -95,12 +95,35 @@ describe('AccountMerge', () => { render(); expect(await screen.findByText('Account merged successfully!', {}, { timeout: POLL_TIMEOUT })).toBeInTheDocument(); - expect(mockCall).toHaveBeenCalledWith({ url: 'job/job-uid', method: 'GET' }); + expect(mockCall).toHaveBeenCalledWith({ url: 'job/job-uid', method: 'GET', token: false }); // The access token is only issued in the HTTP context, so the result has to be fetched again. expect(mergeCalls()).toBe(2); expect(mockSetAuthToken).toHaveBeenCalledWith('token'); }); + // The job belongs to the merge's master while the caller is still signed in as the slave, so + // sending the session token makes the API reject its own caller with a 404. The merge call itself + // must keep its token — that is what the fresh access token is issued from. + it('polls the job unauthenticated but keeps the session token on the merge call', async () => { + respondWith({ + merge: [ + { ...JOB, status: JobStatus.PENDING }, + { kycHash: 'hash', accessToken: 'token' }, + ], + jobs: [{ ...JOB, status: JobStatus.COMPLETE }], + }); + + render(); + await screen.findByText('Account merged successfully!', {}, { timeout: POLL_TIMEOUT }); + + const [jobCall] = mockCall.mock.calls.map(([c]) => c).filter((c) => c.url.startsWith('job/')); + const mergeCallConfigs = mockCall.mock.calls.map(([c]) => c).filter((c) => c.url.startsWith(MERGE_URL)); + + expect(jobCall.token).toBe(false); + expect(mergeCallConfigs).toHaveLength(2); + mergeCallConfigs.forEach((c) => expect(c.token).toBeUndefined()); + }); + it('skips polling when the 202 ticket is already complete', async () => { respondWith({ merge: [{ ...JOB, status: JobStatus.COMPLETE }, { kycHash: 'hash' }], @@ -118,7 +141,7 @@ describe('AccountMerge', () => { render(); - await waitFor(() => expect(mockCall).toHaveBeenCalledWith({ url: 'job/job-uid', method: 'GET' }), { + await waitFor(() => expect(mockCall).toHaveBeenCalledWith({ url: 'job/job-uid', method: 'GET', token: false }), { timeout: POLL_TIMEOUT, }); expect(screen.getByText('Merging your accounts...')).toBeInTheDocument(); diff --git a/src/screens/account-merge.screen.tsx b/src/screens/account-merge.screen.tsx index af0411538..df446c825 100644 --- a/src/screens/account-merge.screen.tsx +++ b/src/screens/account-merge.screen.tsx @@ -78,9 +78,16 @@ export default function AccountMerge() { // 202: the merge outran the endpoint's wait window and continues as a job. A ticket that already // carries a terminal status is returned unpolled, so this covers that case too. - const job = await pollJobUntilTerminal(response, (uid) => call({ url: `job/${uid}`, method: 'GET' }), { - isCancelled: () => isCancelled.current, - }); + // + // Polled with `token: false`, i.e. deliberately unauthenticated. The job belongs to the merge's + // master account, while whoever follows the confirmation link is still signed in as the slave — + // sending that session token makes the API's ownership check reject its own caller with a 404. + // The uid is the proof of ownership here, the same trust level as the link that created the job. + const job = await pollJobUntilTerminal( + response, + (uid) => call({ url: `job/${uid}`, method: 'GET', token: false }), + { isCancelled: () => isCancelled.current }, + ); if (job.status !== JobStatus.COMPLETE) throw mergeJobError(job);