From 71a40fec67968dc397702b3c85ed59589c4b227d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 16 Oct 2026 02:35:52 +0200 Subject: [PATCH 1/3] fix(frontend): send the provider account ID on RI exchange lists The RI Exchange page forwarded the account chip value, a CloudAccount UUID, as account_id / subscription_id. The backend compares those to the provider account number (AWS) or looks up the subscription by external_id (Azure), so the AWS list was always empty and Azure was empty or 404 whenever a chip was selected. Resolve the selected UUID to its external_id via listAccountsMinimal before calling the loaders. An unknown UUID shows an error and makes no list call. A load superseded by a chip change during the lookup is dropped. Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/riexchange.test.ts | 51 ++++++++++++++++++++--- frontend/src/riexchange.ts | 33 ++++++++++++--- 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index 8ec4b2ef3..2e3564e69 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -5,6 +5,7 @@ // Mock the api module defensively (riexchange.ts imports it) jest.mock('../api', () => ({ listConvertibleRIs: jest.fn(), + listAccountsMinimal: jest.fn(), listExchangeableAzureRIs: jest.fn(), getRIUtilization: jest.fn(), getReshapeRecommendations: jest.fn(), @@ -1024,6 +1025,11 @@ describe('RI Exchange global filter scoping (issue #871)', () => { (api.listConvertibleRIs as jest.Mock).mockResolvedValue([]); (api.listExchangeableAzureRIs as jest.Mock).mockResolvedValue([]); + (api.listAccountsMinimal as jest.Mock).mockResolvedValue([ + { id: 'uuid-1', name: 'Prod', external_id: '123456789012', provider: 'aws' }, + { id: 'uuid-2', name: 'Other', external_id: '210987654321', provider: 'aws' }, + { id: 'uuid-az', name: 'Sub', external_id: 'aaaaaaaa-0000-0000-0000-bbbbbbbbbbbb', provider: 'azure' }, + ]); (api.getRIUtilization as jest.Mock).mockResolvedValue([]); (api.getReshapeRecommendations as jest.Mock).mockResolvedValue({ recommendations: [], recs_staleness: '', recs_collected_at: null }); (api.getRIExchangeHistory as jest.Mock).mockResolvedValue([]); @@ -1047,11 +1053,46 @@ describe('RI Exchange global filter scoping (issue #871)', () => { }); it('forwards the single selected account to the AWS list endpoint', async () => { - stateMod().getCurrentAccountIDs.mockReturnValue(['123456789012']); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); await loadRIExchange(); expect(api.listConvertibleRIs).toHaveBeenCalledWith('123456789012'); }); + it('forwards the Azure subscription GUID (external_id), not the account UUID', async () => { + stateMod().getCurrentProvider.mockReturnValue('azure'); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-az']); + await loadRIExchange(); + expect(api.listExchangeableAzureRIs).toHaveBeenCalledWith('aaaaaaaa-0000-0000-0000-bbbbbbbbbbbb'); + }); + + it('shows an error and calls no list endpoint when the selected account is unknown', async () => { + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-missing']); + await loadRIExchange(); + expect(api.listConvertibleRIs).not.toHaveBeenCalled(); + expect(instancesEl.textContent).toContain('uuid-missing'); + expect(instancesEl.querySelector('.error')).not.toBeNull(); + }); + + it('forwards undefined and skips the account lookup when no account is selected', async () => { + await loadRIExchange(); + expect(api.listAccountsMinimal).not.toHaveBeenCalled(); + expect(api.listConvertibleRIs).toHaveBeenCalledWith(undefined); + }); + + it('drops a superseded load when the account chip changes mid-lookup', async () => { + let release: (v: unknown) => void = () => undefined; + (api.listAccountsMinimal as jest.Mock).mockReturnValueOnce(new Promise(r => { release = r; })); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); + const first = loadRIExchange(); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-2']); + const second = loadRIExchange(); + await second; + release([{ id: 'uuid-1', name: 'Prod', external_id: '123456789012', provider: 'aws' }]); + await first; + expect(api.listConvertibleRIs).toHaveBeenCalledTimes(1); + expect(api.listConvertibleRIs).toHaveBeenCalledWith('210987654321'); + }); + it('does not forward an account id when more than one account is selected', async () => { stateMod().getCurrentAccountIDs.mockReturnValue(['111111111111', '222222222222']); await loadRIExchange(); @@ -1059,7 +1100,7 @@ describe('RI Exchange global filter scoping (issue #871)', () => { }); it('renders a scoped empty-state naming the AWS account', async () => { - stateMod().getCurrentAccountIDs.mockReturnValue(['123456789012']); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); (api.listConvertibleRIs as jest.Mock).mockResolvedValue([]); await loadRIExchange(); expect(instancesEl.textContent).toContain('123456789012'); @@ -1081,11 +1122,11 @@ describe('RI Exchange global filter scoping (issue #871)', () => { it('shows a provider-aware empty state for Azure with no reservations', async () => { stateMod().getCurrentProvider.mockReturnValue('azure'); - stateMod().getCurrentAccountIDs.mockReturnValue(['sub-abc']); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-az']); (api.listExchangeableAzureRIs as jest.Mock).mockResolvedValue([]); await loadRIExchange(); - expect(api.listExchangeableAzureRIs).toHaveBeenCalledWith('sub-abc'); - expect(instancesEl.textContent).toContain('No exchangeable reservations for Azure subscription sub-abc'); + expect(api.listExchangeableAzureRIs).toHaveBeenCalledWith('aaaaaaaa-0000-0000-0000-bbbbbbbbbbbb'); + expect(instancesEl.textContent).toContain('No exchangeable reservations for Azure subscription aaaaaaaa-0000-0000-0000-bbbbbbbbbbbb'); }); it('shows the not-available empty state for GCP and calls no list endpoint', async () => { diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index 109e48478..aa928a8ba 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -41,6 +41,10 @@ const exchangePaymentPattern = /^\d+(?:\.\d+)?$/; // Generation counter to prevent stale utilization data from overwriting fresh data let utilizationGeneration = 0; +// Generation counter so a superseded loadRIExchange (chip changed while the +// account lookup was in flight) never issues its list call. +let scopeGeneration = 0; + // The AWS account the convertible-RI list is currently scoped to (the // single-select account chip value, or undefined for all-accounts). Tracked // at module scope so the asynchronous utilization re-render preserves the @@ -58,13 +62,17 @@ function providerLabel(provider: string): string { // resolveScope reads the Main Header global Provider/Account chips so every // RI Exchange load is scoped consistently with the other Inventory sub-tabs -// (issue #871). The account chip is single-select; an account_id is forwarded -// only when exactly one account is active. -function resolveScope(): { provider: Provider | ''; accountID?: string } { +// (issue #871). The account chip holds a CloudAccount UUID, but the RI +// endpoints expect the provider account number / subscription GUID +// (external_id), so the single selected account is translated here. +async function resolveScope(): Promise<{ provider: Provider | ''; accountID?: string }> { const provider = state.getCurrentProvider(); const accountIDs = state.getCurrentAccountIDs(); - const accountID = accountIDs.length === 1 ? accountIDs[0] : undefined; - return { provider, accountID }; + if (accountIDs.length !== 1) return { provider }; + const accounts = await api.listAccountsMinimal(); + const account = accounts.find(a => a.id === accountIDs[0]); + if (!account) throw new Error(`Selected account ${accountIDs[0]} was not found`); + return { provider, accountID: account.external_id }; } // Mode label mapping — single source of truth @@ -89,7 +97,20 @@ void MODE_VALUES; * for GCP" empty state and no list endpoint is called. */ export async function loadRIExchange(): Promise { - const { provider, accountID } = resolveScope(); + const generation = ++scopeGeneration; + let scope: Awaited>; + try { + scope = await resolveScope(); + } catch (error) { + if (generation !== scopeGeneration) return; + const container = document.getElementById('ri-exchange-instances-list'); + if (container) { + container.innerHTML = `

Failed to resolve the selected account: ${escapeHtml((error as Error).message)}

`; + } + return; + } + if (generation !== scopeGeneration) return; + const { provider, accountID } = scope; if (provider === 'gcp') { renderGCPEmptyStates(); From 3e388e6d689e5c3dc14ffd62780866d06a0a50b3 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 16 Oct 2026 02:50:37 +0200 Subject: [PATCH 2/3] fix(frontend): clear the previous RI scope when account resolution fails When the selected account could not be resolved, the error was rendered but the previous scope's state stayed live: an in-flight utilization fetch could re-render the old account's table over the error, and the recommendations and history sections kept showing the old scope. Invalidate the utilization generation, reset the module state and put the other sections into an empty state on that path. Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/riexchange.test.ts | 42 +++++++++++++++++++++++ frontend/src/riexchange.ts | 11 ++++++ 2 files changed, 53 insertions(+) diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index 2e3564e69..0d97f5a53 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -1073,6 +1073,48 @@ describe('RI Exchange global filter scoping (issue #871)', () => { expect(instancesEl.querySelector('.error')).not.toBeNull(); }); + it('a pending utilization fetch from a prior scope cannot overwrite the resolve error', async () => { + const sampleRI = { + reserved_instance_id: 'ri-old', instance_type: 'm5.large', availability_zone: 'us-east-1a', + instance_count: 1, start: '2026-01-01T00:00:00Z', end: '2027-01-01T00:00:00Z', + offering_type: 'Convertible', fixed_price: 0, usage_price: 0, state: 'active', normalization_factor: 4, + }; + let releaseUtil: (v: unknown) => void = () => undefined; + (api.getRIUtilization as jest.Mock).mockReturnValueOnce(new Promise(r => { releaseUtil = r; })); + (api.listConvertibleRIs as jest.Mock).mockResolvedValue([sampleRI]); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); + await loadRIExchange(); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-missing']); + await loadRIExchange(); + releaseUtil([]); + await new Promise(r => setTimeout(r, 0)); + expect(instancesEl.querySelector('.error')).not.toBeNull(); + expect(instancesEl.textContent).not.toContain('ri-old'); + expect(recsEl.textContent).toContain('Select a valid account'); + expect(historyEl.textContent).toContain('Select a valid account'); + }); + + it('drops a superseded failing resolve instead of rendering its error', async () => { + let rejectLookup: (e: Error) => void = () => undefined; + (api.listAccountsMinimal as jest.Mock).mockReturnValueOnce(new Promise((_, rej) => { rejectLookup = rej; })); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); + const first = loadRIExchange(); + stateMod().getCurrentAccountIDs.mockReturnValue([]); + await loadRIExchange(); + rejectLookup(new Error('boom')); + await first; + expect(instancesEl.querySelector('.error')).toBeNull(); + expect(instancesEl.textContent).not.toContain('boom'); + }); + + it('shows the request failure and makes no list call when listAccountsMinimal rejects', async () => { + (api.listAccountsMinimal as jest.Mock).mockRejectedValueOnce(new Error('HTTP 500')); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); + await loadRIExchange(); + expect(api.listConvertibleRIs).not.toHaveBeenCalled(); + expect(instancesEl.textContent).toContain('HTTP 500'); + }); + it('forwards undefined and skips the account lookup when no account is selected', async () => { await loadRIExchange(); expect(api.listAccountsMinimal).not.toHaveBeenCalled(); diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index aa928a8ba..9a644681a 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -103,6 +103,17 @@ export async function loadRIExchange(): Promise { scope = await resolveScope(); } catch (error) { if (generation !== scopeGeneration) return; + utilizationGeneration++; + currentRIs = []; + currentUtilization = new Map(); + currentRecommendations = []; + currentRIAccountID = undefined; + const message = 'Select a valid account to load this section.'; + for (const id of ['ri-exchange-recommendations-list', 'ri-exchange-history-list']) { + const el = document.getElementById(id); + if (el) renderEmptyState(el, message); + } + renderReshapeStalenessBanner('', null); const container = document.getElementById('ri-exchange-instances-list'); if (container) { container.innerHTML = `

Failed to resolve the selected account: ${escapeHtml((error as Error).message)}

`; From 6eddcbf934324174925adfc15286bbe6a1391b14 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 16 Oct 2026 03:04:05 +0200 Subject: [PATCH 3/3] fix(frontend): drop superseded RI exchange loads and reject a blank account ID Loaders started by an earlier loadRIExchange could finish after a later load and re-render the old scope over a resolve error or newer data. Pass the load generation into each loader and return after every await when it is stale. resolveScope now throws when the matched account has no external_id, since the API helpers would otherwise drop the param and send the request unscoped. Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/riexchange.test.ts | 40 +++++++++++++++++++++++ frontend/src/riexchange.ts | 30 +++++++++++------ 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index 0d97f5a53..fc42efb2d 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -1115,6 +1115,46 @@ describe('RI Exchange global filter scoping (issue #871)', () => { expect(instancesEl.textContent).toContain('HTTP 500'); }); + it('fails loudly when the selected account has no external_id', async () => { + (api.listAccountsMinimal as jest.Mock).mockResolvedValueOnce([ + { id: 'uuid-1', name: 'Prod', external_id: '', provider: 'aws' }, + ]); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-1']); + await loadRIExchange(); + expect(api.listConvertibleRIs).not.toHaveBeenCalled(); + expect(instancesEl.querySelector('.error')).not.toBeNull(); + }); + + it('in-flight loaders from a superseded load cannot overwrite a later resolve error', async () => { + const pending = () => { + let resolve: (v: T) => void = () => undefined; + const promise = new Promise(r => { resolve = r; }); + return { promise, resolve }; + }; + const ris = pending(); + const recs = pending(); + const hist = pending(); + (api.listConvertibleRIs as jest.Mock).mockReturnValueOnce(ris.promise); + (api.getReshapeRecommendations as jest.Mock).mockReturnValueOnce(recs.promise); + (api.getRIExchangeHistory as jest.Mock).mockReturnValueOnce(hist.promise); + const loadA = loadRIExchange(); + await new Promise(r => setTimeout(r, 0)); + stateMod().getCurrentAccountIDs.mockReturnValue(['uuid-missing']); + await loadRIExchange(); + ris.resolve([{ + reserved_instance_id: 'ri-old', instance_type: 'm5.large', availability_zone: 'us-east-1a', + instance_count: 1, start: '2026-01-01T00:00:00Z', end: '2027-01-01T00:00:00Z', + offering_type: 'Convertible', fixed_price: 0, usage_price: 0, state: 'active', normalization_factor: 4, + }]); + recs.resolve({ recommendations: [], recs_staleness: '', recs_collected_at: null }); + hist.resolve([]); + await loadA; + expect(instancesEl.querySelector('.error')).not.toBeNull(); + expect(instancesEl.textContent).not.toContain('ri-old'); + expect(recsEl.textContent).toContain('Select a valid account'); + expect(historyEl.textContent).toContain('Select a valid account'); + }); + it('forwards undefined and skips the account lookup when no account is selected', async () => { await loadRIExchange(); expect(api.listAccountsMinimal).not.toHaveBeenCalled(); diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index 9a644681a..c4ad1f52f 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -72,6 +72,7 @@ async function resolveScope(): Promise<{ provider: Provider | ''; accountID?: st const accounts = await api.listAccountsMinimal(); const account = accounts.find(a => a.id === accountIDs[0]); if (!account) throw new Error(`Selected account ${accountIDs[0]} was not found`); + if (!account.external_id) throw new Error(`Selected account ${accountIDs[0]} has no provider account ID`); return { provider, accountID: account.external_id }; } @@ -130,8 +131,8 @@ export async function loadRIExchange(): Promise { if (provider === 'azure') { await Promise.all([ - loadExchangeableAzureRIs(accountID), - loadExchangeHistory(), + loadExchangeableAzureRIs(accountID, generation), + loadExchangeHistory(generation), ]); renderReshapeNotApplicable('azure'); return; @@ -139,9 +140,9 @@ export async function loadRIExchange(): Promise { // AWS (default / empty provider): full convertible-RI flow. await Promise.all([ - loadConvertibleRIs(accountID), - loadReshapeRecommendations(), - loadExchangeHistory(), + loadConvertibleRIs(accountID, generation), + loadReshapeRecommendations(generation), + loadExchangeHistory(generation), ]); } @@ -284,7 +285,7 @@ function closeExchangeModalIfOpen(): void { // Convertible RIs table // ────────────────────────────────────────────── -async function loadConvertibleRIs(accountID?: string): Promise { +async function loadConvertibleRIs(accountID?: string, generation: number = scopeGeneration): Promise { const container = document.getElementById('ri-exchange-instances-list'); if (!container) return; @@ -300,12 +301,15 @@ async function loadConvertibleRIs(accountID?: string): Promise { try { // issue #871: scope to the selected account so the page honours the // Main Header global filter. - currentRIs = await api.listConvertibleRIs(accountID); + const ris = await api.listConvertibleRIs(accountID); + if (generation !== scopeGeneration) return; + currentRIs = ris; renderRIsTable(container, accountID); // Load utilization asynchronously (Cost Explorer is slow) utilizationGeneration++; void loadUtilization(utilizationGeneration); } catch (error) { + if (generation !== scopeGeneration) return; teardownSkeleton(container); const err = error as Error; container.innerHTML = `

Failed to load convertible RIs: ${escapeHtml(err.message)}

`; @@ -319,7 +323,7 @@ async function loadConvertibleRIs(accountID?: string): Promise { * The optional accountID is the selected subscription chip; it scopes the * capacity-provider registration check on the backend. */ -async function loadExchangeableAzureRIs(accountID?: string): Promise { +async function loadExchangeableAzureRIs(accountID: string | undefined, generation: number): Promise { const container = document.getElementById('ri-exchange-instances-list'); if (!container) return; @@ -336,8 +340,10 @@ async function loadExchangeableAzureRIs(accountID?: string): Promise { try { const reservations = await api.listExchangeableAzureRIs(accountID); + if (generation !== scopeGeneration) return; renderAzureRIsTable(container, reservations, accountID); } catch (error) { + if (generation !== scopeGeneration) return; teardownSkeleton(container); const err = error as Error; container.innerHTML = `

Failed to load Azure reservations: ${escapeHtml(err.message)}

`; @@ -511,7 +517,7 @@ function appendAzureCell(tr: HTMLTableRowElement, text: string): void { // Reshape recommendations // ────────────────────────────────────────────── -export async function loadReshapeRecommendations(): Promise { +export async function loadReshapeRecommendations(generation: number = scopeGeneration): Promise { const container = document.getElementById('ri-exchange-recommendations-list'); if (!container) return; @@ -523,10 +529,12 @@ export async function loadReshapeRecommendations(): Promise { try { const resp: ReshapeRecommendationsResponse = await api.getReshapeRecommendations(); + if (generation !== scopeGeneration) return; currentRecommendations = resp.recommendations ?? []; renderRecommendations(container); renderReshapeStalenessBanner(resp.recs_staleness, resp.recs_collected_at); } catch (error) { + if (generation !== scopeGeneration) return; teardownSkeleton(container); const err = error as Error; container.innerHTML = `

Failed to load recommendations: ${escapeHtml(err.message)}

`; @@ -2284,7 +2292,7 @@ function canApproveRIExchangeRow(rec: RIExchangeHistoryRecord): boolean { return canAccess('approve-own', 'purchases') && rec.created_by_user_id === user.id; } -async function loadExchangeHistory(): Promise { +async function loadExchangeHistory(generation: number = scopeGeneration): Promise { const container = document.getElementById('ri-exchange-history-list'); if (!container) return; @@ -2296,8 +2304,10 @@ async function loadExchangeHistory(): Promise { try { const records = await api.getRIExchangeHistory(); + if (generation !== scopeGeneration) return; renderExchangeHistory(container, records); } catch (error) { + if (generation !== scopeGeneration) return; const err = error as Error; container.textContent = ''; const errorP = document.createElement('p');