From ad53b886da78bb2065112418760f617c98237cb7 Mon Sep 17 00:00:00 2001 From: znerol2 <6800371+znerol2@users.noreply.github.com> Date: Sat, 7 Nov 2026 14:54:39 +0100 Subject: [PATCH] fix(console): close two pending-state gaps A topology poll whose sources held a null entry crashed the views that read it, and an entry without an id was shown or offered as a source. Validate every entry with one shared check and treat anything else as malformed, which fails closed as before; the pending switch snapshot still holds. ReasonConfirm also told the operator to confirm again when inventory went stale while a request was already in flight. Hide that guidance while pending; it returns once the request settles. Closes dmfdeploy/dmfdeploy#863 Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com> --- .../reasonConfirmPendingStale.test.tsx | 38 +++++++++++++++++++ .../__tests__/switchSourcePending.test.tsx | 32 +++++++++++++++- frontend/src/components/ReasonConfirm.tsx | 4 +- frontend/src/lib/topologySources.ts | 17 +++++++++ .../src/pages/MediaWorkloads/WorkloadHome.tsx | 3 +- .../MediaWorkloads/stages/ConfigureStage.tsx | 3 +- .../MediaWorkloads/stages/DesignStage.tsx | 3 +- 7 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 frontend/src/__tests__/reasonConfirmPendingStale.test.tsx create mode 100644 frontend/src/lib/topologySources.ts diff --git a/frontend/src/__tests__/reasonConfirmPendingStale.test.tsx b/frontend/src/__tests__/reasonConfirmPendingStale.test.tsx new file mode 100644 index 0000000..667d40c --- /dev/null +++ b/frontend/src/__tests__/reasonConfirmPendingStale.test.tsx @@ -0,0 +1,38 @@ +/** + * dmfdeploy/dmfdeploy#863: once a request is sent, ReasonConfirm must not tell + * the operator to confirm again when the inventory goes stale. The pre-submit + * guidance returns after the request settles. + */ +import { afterEach, describe, expect, it } from 'vitest' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import ReasonConfirm from '../components/ReasonConfirm' +import { InventoryWritesBlockedProvider } from '../lib/inventoryFreshness' + +afterEach(cleanup) + +const GUIDANCE = /Confirm again after the next successful read/ + +function panel(blocked: boolean, pending: boolean) { + return ( + + {}} onCancel={() => {}} /> + + ) +} + +describe('ReasonConfirm inventory guidance while pending (#863)', () => { + it('hides the confirm-again guidance while pending and restores it after settle', () => { + const view = render(panel(false, false)) + fireEvent.change(screen.getByRole('textbox'), { target: { value: 'go' } }) + view.rerender(panel(false, true)) + expect(screen.queryByText(GUIDANCE)).toBeNull() + // Inventory goes stale while the request is in flight. + view.rerender(panel(true, true)) + expect(screen.queryByText(GUIDANCE)).toBeNull() + expect(screen.queryByText(/cannot be sent right now/)).toBeNull() + // The request settles with the inventory still stale. + view.rerender(panel(true, false)) + expect(screen.getByText(GUIDANCE)).toBeTruthy() + expect((screen.getByRole('textbox') as HTMLTextAreaElement).value).toBe('go') + }) +}) diff --git a/frontend/src/__tests__/switchSourcePending.test.tsx b/frontend/src/__tests__/switchSourcePending.test.tsx index aaf228f..f830186 100644 --- a/frontend/src/__tests__/switchSourcePending.test.tsx +++ b/frontend/src/__tests__/switchSourcePending.test.tsx @@ -54,7 +54,7 @@ const SOURCES = [ ] const KEY = ['media-workloads-topology', 'viewer-1'] -type TopologyMode = 'ok' | 'error' | 'malformed' +type TopologyMode = 'ok' | 'error' | 'malformed' | 'null-entry' | 'idless-entry' function setup() { let active = 'source-a' @@ -84,7 +84,14 @@ function setup() { if (mode === 'error') return json({ error: 'boom' }, 500) return json({ receiver_instance: 'viewer-1', - sources: mode === 'malformed' ? 'not-a-list' : sources, + sources: + mode === 'malformed' + ? 'not-a-list' + : mode === 'null-entry' + ? [null] + : mode === 'idless-entry' + ? [{}] + : sources, active_source: active, provenance: 'observed-flow', observed_at: new Date().toISOString(), @@ -122,6 +129,8 @@ function setup() { const state = qc.getQueryState(KEY) if (mode === 'error') expect(state?.status).toBe('error') else if (mode === 'malformed') expect(qc.getQueryData<{ sources: unknown }>(KEY)?.sources).toBe('not-a-list') + else if (mode === 'null-entry') expect(qc.getQueryData<{ sources: unknown }>(KEY)?.sources).toEqual([null]) + else if (mode === 'idless-entry') expect(qc.getQueryData<{ sources: unknown }>(KEY)?.sources).toEqual([{}]) else expect(qc.getQueryData<{ active_source: string }>(KEY)?.active_source).toBe(active) }) // The query notifies React on a later task; let that render happen. @@ -235,6 +244,25 @@ describe('switch source while the request is pending (#861)', () => { }) }) +describe.each(['null-entry', 'idless-entry'] as const)('a sources array with a %s (#863)', (mode) => { + it('fails closed like a non-array when nothing is pending, without throwing', async () => { + const { poll } = setup() + await screen.findByRole('button', { name: 'Switch source' }) + await poll({ mode }) + expect(screen.queryByRole('button', { name: 'Switch source' })).toBeNull() + expect(screen.queryByText(/Live source is unknown or stale/)).toBeNull() + }) + + it('keeps the snapshot panel while a switch is pending', async () => { + const { poll, settlePost } = setup() + await armAndPick('source-b') + await submitAndWaitPending() + await poll({ mode }) + expectPendingPanel() + settlePost('ok') + }) +}) + describe('switch source before submit (#861)', () => { it('a poll that drops the selected target shows the hint and disables Confirm', async () => { const { poll } = setup() diff --git a/frontend/src/components/ReasonConfirm.tsx b/frontend/src/components/ReasonConfirm.tsx index 0fdd98f..721290e 100644 --- a/frontend/src/components/ReasonConfirm.tsx +++ b/frontend/src/components/ReasonConfirm.tsx @@ -207,7 +207,9 @@ export default function ReasonConfirm({ {errorMessage ?? requestFailureText(error)}

)} - {writesBlocked && ( + {/* Hidden while the request is in flight: it was already sent, so there + is nothing to confirm again. The guidance returns once it settles. */} + {writesBlocked && !pending && (

The inventory is out of date, so this change cannot be sent right now. Your reason is kept. Confirm again after the next successful read. diff --git a/frontend/src/lib/topologySources.ts b/frontend/src/lib/topologySources.ts new file mode 100644 index 0000000..2b45d97 --- /dev/null +++ b/frontend/src/lib/topologySources.ts @@ -0,0 +1,17 @@ +import type { InstanceTopology } from '../api/types' + +/** + * True when `topology.sources` is an array whose every entry is an object + * with a string `id`. Anything else is malformed: reading `s.id` on a `null` + * entry throws, and an id-less entry would be shown or offered as a source + * with no id, so a consumer treats the whole payload as unknown instead. + */ +export function hasValidSources(topology: InstanceTopology | null | undefined): topology is InstanceTopology { + const sources: unknown = topology?.sources + return ( + Array.isArray(sources) && + sources.every( + (s) => typeof s === 'object' && s !== null && typeof (s as { id?: unknown }).id === 'string', + ) + ) +} diff --git a/frontend/src/pages/MediaWorkloads/WorkloadHome.tsx b/frontend/src/pages/MediaWorkloads/WorkloadHome.tsx index f2a8bd2..fe218a7 100644 --- a/frontend/src/pages/MediaWorkloads/WorkloadHome.tsx +++ b/frontend/src/pages/MediaWorkloads/WorkloadHome.tsx @@ -11,6 +11,7 @@ import { LOCKED_REASON } from './WorkloadSetup' import WorkloadTile from './WorkloadTile' import InstanceLiveModal from './InstanceLiveModal' import { settleQuery } from '../../lib/queryState' +import { hasValidSources } from '../../lib/topologySources' import { LIVE_TILE_CAP, useDocumentVisible, usePrefersReducedMotion } from './liveView' import PageHeading from '../../components/PageHeading' import { usePageTitle } from '../../hooks/usePageTitle' @@ -745,7 +746,7 @@ function InstanceActiveSource({ onResolved: (instance: string, has: boolean) => void }) { const topology = useInstanceTopology(instance, { enabled: hasTopology !== false }) - const has = !topology.isError && Boolean(topology.data && Array.isArray(topology.data.sources)) + const has = !topology.isError && hasValidSources(topology.data) useEffect(() => onResolved(instance, has), [instance, has, onResolved]) // An instance without a topology renders nothing — not an empty card, not diff --git a/frontend/src/pages/MediaWorkloads/stages/ConfigureStage.tsx b/frontend/src/pages/MediaWorkloads/stages/ConfigureStage.tsx index 78fe1b6..a014fdd 100644 --- a/frontend/src/pages/MediaWorkloads/stages/ConfigureStage.tsx +++ b/frontend/src/pages/MediaWorkloads/stages/ConfigureStage.tsx @@ -8,6 +8,7 @@ import StageCard from './StageCard' import { SWITCH_REQUEST_FAILED_TEXT, switchFailureText } from '../switchResultText' import { settleQuery } from '../../../lib/queryState' import { useGuardWrite } from '../../../lib/inventoryFreshness' +import { hasValidSources } from '../../../lib/topologySources' /** * Configure — source selection and the switch, relocated from @@ -184,7 +185,7 @@ function InstanceSwitchControl({ const pending = switchMutation.isPending const frozen = pending ? submitted : null const live = - topology.failed || !topology.data || !Array.isArray(topology.data.sources) ? null : topology.data + topology.failed || !hasValidSources(topology.data) ? null : topology.data if (!live && !frozen) return null const sources = live?.sources ?? [] diff --git a/frontend/src/pages/MediaWorkloads/stages/DesignStage.tsx b/frontend/src/pages/MediaWorkloads/stages/DesignStage.tsx index 99a6e6a..a9e033e 100644 --- a/frontend/src/pages/MediaWorkloads/stages/DesignStage.tsx +++ b/frontend/src/pages/MediaWorkloads/stages/DesignStage.tsx @@ -1,4 +1,5 @@ import { useInstanceTopology } from '../../../api/hooks' +import { hasValidSources } from '../../../lib/topologySources' import type { CatalogEntry, MediaWorkload } from '../../../api/types' import type { StageState } from '../../../lib/workloadLifecycle' import StageCard from './StageCard' @@ -192,7 +193,7 @@ function InstanceComposition({ instance, hasTopology }: { instance: string; hasT // // (umbrella #385) retrofitted onto settleQuery. const topology = settleQuery(useInstanceTopology(instance, { enabled: hasTopology !== false })) - if (topology.failed || !topology.data || !Array.isArray(topology.data.sources)) return null + if (topology.failed || !hasValidSources(topology.data)) return null const { sources, active_source } = topology.data if (sources.length === 0) return null