From 79c643b42b1e6ae9198a1cf61c757eaedf4fbc78 Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 2 Aug 2026 07:56:53 +0200 Subject: [PATCH] fix(stalker): make account-profile refresh own the auth slot Review round five (Codex P2s on #1330): - Claim the pendingAuth slot in a loop and publish it before the first await. One settled promise releases every waiter at once, so a single pre-check let two queued refreshes both start handshakes that invalidate each other on strict portals. - Retire the cached token before the handshake: ensureToken() reads tokenCache before pendingAuth, so catalog and watchdog requests starting mid-handshake were handed a token this refresh was about to kill instead of queueing on the slot. - Render the portal type from the same resolver the fetch path uses, so a restored backup without an explicit flag is no longer labelled a legacy panel while authenticating as a full portal. Co-Authored-By: Claude Opus 5 --- .../src/lib/stalker-session.service.spec.ts | 77 +++++++++++++++++++ .../src/lib/stalker-session.service.ts | 57 ++++++++++---- .../stalker-account-info.component.html | 4 +- .../stalker-account-info.component.spec.ts | 43 +++++++++++ .../stalker-account-info.component.ts | 7 ++ 5 files changed, 171 insertions(+), 17 deletions(-) diff --git a/libs/portal/stalker/data-access/src/lib/stalker-session.service.spec.ts b/libs/portal/stalker/data-access/src/lib/stalker-session.service.spec.ts index 04c9a1491..ef618e1e1 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-session.service.spec.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-session.service.spec.ts @@ -206,6 +206,83 @@ describe('StalkerSessionService identity payloads', () => { expect(service.getCachedToken(playlist._id)).toBe('profile-token'); }); + it('lets only one of several queued refreshes authenticate at a time', async () => { + const playlist = { + _id: 'playlist-3', + portalUrl, + macAddress, + isFullStalkerPortal: true, + } as Playlist; + + const releases: Array<(value: { token: string }) => void> = []; + const authenticate = jest + .spyOn(service, 'authenticate') + .mockImplementation( + () => + new Promise((resolve) => { + releases.push(resolve); + }) + ); + + // Both refreshes queue behind the same in-flight ensureToken, so + // one settled promise releases both waiters at once. + const pending = service.ensureToken(playlist); + const first = service.refreshAccountProfile(playlist); + const second = service.refreshAccountProfile(playlist); + + await Promise.resolve(); + expect(authenticate).toHaveBeenCalledTimes(1); + + releases[0]({ token: 'session-token' }); + await pending; + await new Promise((resolve) => setTimeout(resolve)); + + // The released waiters must not both start a handshake. + expect(authenticate).toHaveBeenCalledTimes(2); + + releases[1]({ token: 'first-refresh-token' }); + await first; + await new Promise((resolve) => setTimeout(resolve)); + + expect(authenticate).toHaveBeenCalledTimes(3); + releases[2]({ token: 'second-refresh-token' }); + await second; + + expect(service.getCachedToken(playlist._id)).toBe( + 'second-refresh-token' + ); + }); + + it('retires the cached token before the refresh handshake starts', async () => { + const playlist = { + _id: 'playlist-4', + portalUrl, + macAddress, + isFullStalkerPortal: true, + } as Playlist; + + service.setCachedToken(playlist._id, 'stale-token'); + + let release: (value: { token: string }) => void = () => undefined; + jest.spyOn(service, 'authenticate').mockImplementation( + () => + new Promise((resolve) => { + release = resolve; + }) + ); + + const refresh = service.refreshAccountProfile(playlist); + await Promise.resolve(); + + // ensureToken() reads the cache before pendingAuth, so a token the + // handshake is invalidating must not stay readable meanwhile. + expect(service.getCachedToken(playlist._id)).toBeNull(); + + release({ token: 'fresh-token' }); + await refresh; + expect(service.getCachedToken(playlist._id)).toBe('fresh-token'); + }); + it('refreshes the account profile even when a pending authentication fails', async () => { const playlist = { _id: 'playlist-2', diff --git a/libs/portal/stalker/data-access/src/lib/stalker-session.service.ts b/libs/portal/stalker/data-access/src/lib/stalker-session.service.ts index f28ee91c1..4bc756f9e 100644 --- a/libs/portal/stalker/data-access/src/lib/stalker-session.service.ts +++ b/libs/portal/stalker/data-access/src/lib/stalker-session.service.ts @@ -542,41 +542,68 @@ export class StalkerSessionService { const macAddress = playlist.macAddress; const identity = getStalkerPortalIdentityFromPlaylist(playlist); - const inFlight = this.pendingAuth.get(playlist._id); - if (inFlight) { + // Claim the per-playlist slot. Re-check after every await: one + // settled promise releases every waiter at once, so a single + // pre-check would let them all start competing handshakes. + for ( + let inFlight = this.pendingAuth.get(playlist._id); + inFlight; + inFlight = this.pendingAuth.get(playlist._id) + ) { this.logger.debug('Waiting for pending authentication...'); // A failed pending auth must not abort the refresh; this call // performs its own handshake either way. await inFlight.catch(() => undefined); } - let accountInfo: StalkerProfileResponse['js']['account_info']; - const authPromise = (async () => { + // Publish the slot before the first await so no other waiter can + // observe it as free while this handshake is starting. + // No-op defaults: the executor runs synchronously and overwrites + // both, but the compiler cannot prove that (TS2454). + let settleSlot: (value: { + token: string; + serialNumber?: string; + }) => void = () => undefined; + let failSlot: (reason: unknown) => void = () => undefined; + const slot = new Promise<{ token: string; serialNumber?: string }>( + (resolve, reject) => { + settleSlot = resolve; + failSlot = reject; + } + ); + // Waiters attach their own handlers; this one only keeps a + // rejected slot from surfacing as an unhandled rejection. + void slot.catch(() => undefined); + this.pendingAuth.set(playlist._id, slot); + + // ensureToken() reads tokenCache before pendingAuth, so leaving the + // old token there would hand a token this handshake is about to + // invalidate to catalog and watchdog requests. Retiring it first + // makes them queue on the slot instead. + this.clearCachedToken(playlist._id); + + try { const result = await this.authenticate( portalUrl, macAddress, identity ); - accountInfo = result.accountInfo; this.setCachedToken(playlist._id, result.token); - return { + settleSlot({ token: result.token, serialNumber: identity.serialNumber, - }; - })(); - - this.pendingAuth.set(playlist._id, authPromise); - try { - await authPromise; + }); + return result.accountInfo; + } catch (error) { + failSlot(error); + throw error; } finally { // Only retire our own entry: a caller that started a later // authentication owns the map slot from then on. - if (this.pendingAuth.get(playlist._id) === authPromise) { + if (this.pendingAuth.get(playlist._id) === slot) { this.pendingAuth.delete(playlist._id); } } - - return accountInfo; } /** diff --git a/libs/portal/stalker/feature/src/lib/stalker-account-info/stalker-account-info.component.html b/libs/portal/stalker/feature/src/lib/stalker-account-info/stalker-account-info.component.html index 82492e211..282e971db 100644 --- a/libs/portal/stalker/feature/src/lib/stalker-account-info/stalker-account-info.component.html +++ b/libs/portal/stalker/feature/src/lib/stalker-account-info/stalker-account-info.component.html @@ -3,7 +3,7 @@