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 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5 committed 2026-08-02 09:16:00 +02:00
1 parent 4272c2bf3a
commit 79c643b42b
5 files changed
+171 -17

No files matched your search

@@ -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',
@@ -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;
}
/**
@@ -3,7 +3,7 @@
<div class="account-dialog__eyebrow">
<span class="account-dialog__eyebrow-mark"></span>
Stalker
@if (playlist.isFullStalkerPortal) {
@if (isFullPortal) {
<span class="account-dialog__eyebrow-divider"></span>
{{ 'STALKER.ACCOUNT_INFO.PORTAL_TYPE_FULL' | translate }}
}
@@ -174,7 +174,7 @@
{{ 'STALKER.ACCOUNT_INFO.PORTAL_INFO' | translate }}
</p>
<h3>
@if (playlist.isFullStalkerPortal) {
@if (isFullPortal) {
{{
'STALKER.ACCOUNT_INFO.PORTAL_TYPE_FULL'
| translate
@@ -198,6 +198,49 @@ describe('StalkerAccountInfoComponent', () => {
);
});
it('labels a restored full portal by URL when the flag is absent', async () => {
await TestBed.resetTestingModule();
await TestBed.configureTestingModule({
imports: [
StalkerAccountInfoComponent,
NoopAnimationsModule,
TranslateModule.forRoot(),
],
providers: [
{
provide: MAT_DIALOG_DATA,
useValue: {
playlist: {
...playlist,
portalUrl:
'http://portal.example/stalker_portal/c/',
isFullStalkerPortal: undefined,
},
},
},
{
provide: StalkerAccountInfoService,
useValue: accountInfoService,
},
{ provide: PlaylistsService, useValue: playlistsService },
],
}).compileComponents();
const restored = TestBed.createComponent(StalkerAccountInfoComponent);
restored.detectChanges();
await restored.whenStable();
// Presentation must agree with the fetch path, which resolves the
// portal type from the URL when the flag is missing.
expect(restored.componentInstance.isFullPortal).toBe(true);
expect(restored.nativeElement.textContent).toContain(
'STALKER.ACCOUNT_INFO.PORTAL_TYPE_FULL'
);
expect(restored.nativeElement.textContent).not.toContain(
'STALKER.ACCOUNT_INFO.PORTAL_TYPE_LEGACY'
);
});
it('renders no status pill for unknown status values', async () => {
accountInfoService.fetchAccountInfo.mockResolvedValue({
...freshSnapshot,
@@ -11,6 +11,7 @@ import { MatIconModule } from '@angular/material/icon';
import { TranslatePipe } from '@ngx-translate/core';
import { firstValueFrom } from 'rxjs';
import {
isFullStalkerPortalPlaylist,
normalizeStoredStalkerAccountInfo,
StalkerAccountInfoService,
StalkerAccountSnapshot,
@@ -59,6 +60,12 @@ export class StalkerAccountInfoComponent {
private readonly logger = createLogger('StalkerAccountInfo');
readonly playlist: PlaylistMeta = this.data.playlist;
/**
* Presentation must use the same resolution as the fetch path: a
* playlist restored from an older backup carries no explicit flag,
* and the raw field would label a real Ministra portal as legacy.
*/
readonly isFullPortal = isFullStalkerPortalPlaylist(this.playlist);
readonly loadState = signal<AccountLoadState>('loading');
readonly snapshot = signal<StalkerAccountSnapshot | null>(null);
readonly snapshotSource = signal<SnapshotSource | null>(null);