From 9dc6bd0ffe4de520bf9afc43ad10fb169026325d Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Fri, 2 Oct 2026 15:15:25 +0100 Subject: [PATCH] Keep shared History responses bound to the selected target Invalidate superseded requests and polling, clear stale samples on selection changes, and exercise late completions in runtime and browser regressions. Contract-Neutral: Correct request ownership in the existing shared chart without changing API, props, entitlements or public surface; dedicated runtime regressions accompany the repair. Change-source: pulse-maintainer --- .../subsystems/frontend-primitives.md | 18 ++ .../browser-tests/history-selection.cjs | 110 ++++++++++ .../browser-tests/history-selection.html | 12 ++ .../browser-tests/history-selection.tsx | 71 +++++++ frontend-modern/browser-verification.json | 31 ++- .../__tests__/useHistoryChartState.test.tsx | 188 ++++++++++++++++++ .../components/shared/useHistoryChartState.ts | 145 ++++++-------- 7 files changed, 475 insertions(+), 100 deletions(-) create mode 100644 frontend-modern/browser-tests/history-selection.cjs create mode 100644 frontend-modern/browser-tests/history-selection.html create mode 100644 frontend-modern/browser-tests/history-selection.tsx create mode 100644 frontend-modern/src/components/shared/__tests__/useHistoryChartState.test.tsx diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index a5bfabf51..1465e603f 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -7667,3 +7667,21 @@ uses the production PBS table, resource drawers, History query and CSS with synthetic APIs, checking three separately mapped drawers and range/refresh behaviour in desktop and phone-emulated engines. This is presentation proof, not installed PBS/VirtualBox collection or a complete #1723 acceptance result. + +### Shared canvas History responses belong to their selection + +`useHistoryChartState` cancels and invalidates old requests when the resource, +metric, range, sampling cap, supplied-data mode or access state changes. Late +successes and failures cannot replace the current selection's samples, loading +or error state, even if cancellation is ignored. Selection changes clear old +readings and hover state while loading; current initial failures remain visible. +Matching background refresh failures retain already loaded samples. Polls never +overlap and stop for supplied data (including empty arrays), unavailable targets, +locked ranges and unmount. + +`useHistoryChartState.test.tsx` checks delayed success/failure, each selection +field, polling, supplied-data transitions, locked/empty targets and cleanup. +`browser-tests/history-selection.cjs` exercises the production canvas chart and +accessible description in desktop Chromium and phone WebKit, including a late +old-target response and current-target loading/failure. Synthetic response proof +is not native PBS collection or whole-report #1723 acceptance. diff --git a/frontend-modern/browser-tests/history-selection.cjs b/frontend-modern/browser-tests/history-selection.cjs new file mode 100644 index 000000000..de9810090 --- /dev/null +++ b/frontend-modern/browser-tests/history-selection.cjs @@ -0,0 +1,110 @@ +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { chromium, webkit } = require('playwright'); +(async () => { + const root = '/workspace/frontend-modern'; + process.chdir(root); + const artifacts = path.join(root, 'node_modules/history-selection-proof'); + fs.mkdirSync(artifacts, { recursive: true }); + const { createServer } = await import(path.join(root, 'node_modules/vite/dist/node/index.js')); + const server = await createServer({ + root, + configFile: path.join(root, 'vite.config.ts'), + cacheDir: fs.mkdtempSync(path.join(artifacts, 'vite-')), + server: { host: '127.0.0.1', port: 5222, strictPort: true }, + }); + const observations = []; + let browser; + try { + await server.listen(); + for (const scenario of [ + { engine: chromium, name: 'chromium', width: 1365, height: 900 }, + { engine: webkit, name: 'webkit', width: 390, height: 844 }, + ]) { + browser = await scenario.engine.launch( + scenario.name === 'chromium' + ? { headless: true, channel: 'chromium', args: ['--no-sandbox'] } + : { headless: true }, + ); + const page = await browser.newPage({ + viewport: { width: scenario.width, height: scenario.height }, + isMobile: scenario.name === 'webkit', + hasTouch: scenario.name === 'webkit', + }); + const errors = []; + page.on('pageerror', (e) => errors.push(e.message)); + await page.route('**/*', (route) => { + const url = new URL(route.request().url()); + if (url.origin !== 'http://127.0.0.1:5222') return route.abort(); + if (!url.pathname.startsWith('/api/')) return route.continue(); + return route.fulfill({ + json: + url.pathname === '/api/license/runtime-capabilities' + ? { + capabilities: [], + limits: [], + max_history_days: 7, + hosted_mode: false, + runtime: { build: 'community' }, + blocked_capabilities: [], + } + : { data: [], enabled: false }, + }); + }); + await page.goto('http://127.0.0.1:5222/browser-tests/history-selection.html'); + if (scenario.name === 'webkit') + await page.evaluate(() => document.documentElement.classList.add('dark')); + const chart = page.getByRole('img', { name: 'CPU usage chart' }); + await chart.waitFor(); + const description = page.locator('#' + (await chart.getAttribute('aria-describedby'))); + await page.getByRole('button', { name: 'Select b', exact: true }).click(); + await page.getByRole('button', { name: 'Finish b', exact: true }).click(); + for (const [phase, button, expected] of [ + ['b-loaded', null, '80.0%'], + ['old-a-finished', 'Finish old a', '80.0%'], + ['c-loading', 'Select c', 'Loading'], + ['c-failed', 'Fail c', 'could not be loaded'], + ]) { + if (button) await page.getByRole('button', { name: button, exact: true }).click(); + await page.waitForTimeout(100); + const actual = await description.textContent(); + const screenshot = path.join(artifacts, `${scenario.name}-${phase}.png`); + await page.screenshot({ path: screenshot, fullPage: true }); + assert.ok(actual.includes(expected), `${scenario.name} ${phase}: ${actual}`); + if (phase.startsWith('c-')) assert.ok(!actual.includes('80.0%')); + const dimensions = await page.evaluate(() => ({ + scroll: document.documentElement.scrollWidth, + width: innerWidth, + })); + assert.ok(dimensions.scroll <= dimensions.width + 1, JSON.stringify(dimensions)); + observations.push({ + browser: scenario.name, + version: browser.version(), + phase, + actual, + dimensions, + screenshot, + }); + } + assert.deepEqual(errors, []); + await browser.close(); + browser = null; + } + fs.writeFileSync( + path.join(artifacts, 'result.json'), + JSON.stringify( + { playwright: require('playwright/package.json').version, observations }, + null, + 2, + ), + ); + console.log(JSON.stringify({ result: 'passed', states: observations.length })); + } finally { + if (browser) await browser.close(); + await server.close(); + } +})().catch((error) => { + console.error(error); + process.exitCode = 1; +}); diff --git a/frontend-modern/browser-tests/history-selection.html b/frontend-modern/browser-tests/history-selection.html new file mode 100644 index 000000000..88453fe54 --- /dev/null +++ b/frontend-modern/browser-tests/history-selection.html @@ -0,0 +1,12 @@ + + + + + + History selection verification + + +
+ + + diff --git a/frontend-modern/browser-tests/history-selection.tsx b/frontend-modern/browser-tests/history-selection.tsx new file mode 100644 index 000000000..3f8753889 --- /dev/null +++ b/frontend-modern/browser-tests/history-selection.tsx @@ -0,0 +1,71 @@ +// Synthetic delayed transport deliberately ignores AbortSignal to test response ownership. +import { createSignal } from 'solid-js'; +import { render } from 'solid-js/web'; +import { ChartsAPI, type SingleMetricHistoryResponse } from '../src/api/charts'; +import { HistoryChart } from '../src/components/shared/HistoryChart'; +import '../src/index.css'; +const pending = new Map< + string, + Array<{ + resolve: (response: SingleMetricHistoryResponse) => void; + reject: (error: Error) => void; + }> +>(); +ChartsAPI.getMetricsHistory = (params) => + new Promise((resolve, reject) => { + pending.set(params.resourceId, [ + ...(pending.get(params.resourceId) ?? []), + { resolve, reject }, + ]); + }); +const complete = (target: string, value: number, fail = false) => { + for (const request of pending.get(target) ?? []) { + if (fail) request.reject(new Error('Synthetic current-target failure')); + else + request.resolve({ + points: [0, 1, 2].map((i) => ({ + timestamp: 1790942400000 + i * 60000, + value, + min: value, + max: value, + })), + source: 'store', + } as SingleMetricHistoryResponse); + } + pending.delete(target); +}; +const Fixture = () => { + const [target, setTarget] = createSignal('a'); + return ( +
+

History selection verification

+

Synthetic delayed responses; selected target: {target()}

+
+ {['b', 'c'].map((name) => ( + + ))} + + + +
+ +
+ ); +}; +render(() => , document.getElementById('root')!); diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index 2b8e8c49b..24e4d168d 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,16 +1,16 @@ { "version": 1, - "base_sha": "60a6c933e0057435dd49cb440995f275f6a567d6", - "verified_at": "2026-10-02T12:57:15.855691Z", + "base_sha": "46d2727828d20680cdc6d3a1ff1b419c3ec2bca4", + "verified_at": "2026-10-02T14:15:24.462877+00:00", "result": "passed", "changed_paths": [ - "frontend-modern/src/features/storageBackups/storagePoolDetailPresentation.ts" + "frontend-modern/src/components/shared/useHistoryChartState.ts" ], "content_sha256": { - "frontend-modern/src/features/storageBackups/storagePoolDetailPresentation.ts": "59ca48c06cb3c63f30ccb46cb30536cbc6d86fda1b50e6ea49ecefb9eb377fae" + "frontend-modern/src/components/shared/useHistoryChartState.ts": "ff9dff0db01ae3141b3a130477cb8128ea4da8ea4e93f23c360278f6a47b152a" }, "routes": [ - "/browser-tests/pool-capacity.html" + "/browser-tests/history-selection.html" ], "viewports": [ { @@ -23,26 +23,23 @@ } ], "states": [ - "Production pool drawer with synthetic partial PBS snapshots, not native collector or installed acceptance", - "Missing used/free/usage remain n/a; measured empty and full pools preserve 0 B and 0%", - "Independent byte observations remain visible without total; absent capacity stays n/a", - "Live transitions across five states on desktop Chromium light and phone WebKit dark; no page errors or horizontal overflow" + "Synthetic delayed transport using the production shared chart, with hidden selector matching storage drawers; not native PBS acceptance", + "Selected B remains 80% after old A completes at 10%; new C clears readings while loading and displays its own failure", + "Desktop Chromium light and phone WebKit dark: eight states, no page errors or overflow in storage layout" ], "interactions": [ - "Switch missing, empty, full, partial and absent snapshots without reload", - "Read Used, Free, Total and Usage values in the actual Configuration rows" + "Select B, complete B then delayed A", + "Select C then fail its request; verify accessible description and plotted extrema" ], - "command": "pulse-worker-browser frontend-modern/browser-tests/pool-capacity.cjs", + "command": "pulse-worker-browser frontend-modern/browser-tests/history-selection.cjs", "browser_versions": { "playwright": "1.56.1", "chromium": "141.0.7390.37", "webkit": "26.0" }, "artifacts": [ - "/var/lib/pulse-maintainer/worker-outputs/web-product-5pdbcfmm/browser/result.json", - "/var/lib/pulse-maintainer/worker-outputs/web-product-5pdbcfmm/browser/chromium-partial.png", - "/var/lib/pulse-maintainer/worker-outputs/web-product-5pdbcfmm/browser/webkit-missing.png", - "/var/lib/pulse-maintainer/worker-outputs/web-product-5pdbcfmm/browser/chromium-full.png", - "/var/lib/pulse-maintainer/worker-outputs/web-product-5pdbcfmm/browser/webkit-empty.png" + "/var/lib/pulse-maintainer/worker-outputs/web-product-wwhurxzu/browser/result.json", + "/var/lib/pulse-maintainer/worker-outputs/web-product-wwhurxzu/browser/webkit-old-a-finished.png", + "/var/lib/pulse-maintainer/worker-outputs/web-product-wwhurxzu/browser/chromium-c-loading.png" ] } diff --git a/frontend-modern/src/components/shared/__tests__/useHistoryChartState.test.tsx b/frontend-modern/src/components/shared/__tests__/useHistoryChartState.test.tsx new file mode 100644 index 000000000..0942f115e --- /dev/null +++ b/frontend-modern/src/components/shared/__tests__/useHistoryChartState.test.tsx @@ -0,0 +1,188 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createSignal } from 'solid-js'; +import { cleanup, render } from '@solidjs/testing-library'; +import { ChartsAPI } from '@/api/charts'; +import type { HistoryChartProps } from '../historyChartModel'; +import { useHistoryChartState, type HistoryChartState } from '../useHistoryChartState'; + +vi.mock('@/stores/license', () => ({ + isRangeLocked: (range: string) => range === '90d', + loadRuntimeCapabilities: vi.fn(), + maxHistoryDays: () => 7, +})); +vi.mock('@/api/charts', () => ({ ChartsAPI: { getMetricsHistory: vi.fn() } })); +const request = vi.mocked(ChartsAPI.getMetricsHistory); +const points = (value: number) => [{ timestamp: 1000, value, min: value, max: value }]; +const settle = async () => { + await Promise.resolve(); + await Promise.resolve(); +}; +function deferred() { + let resolve!: (result: Awaited>) => void; + let reject!: (error: Error) => void; + const promise = new Promise>>( + (yes, no) => { + resolve = yes; + reject = no; + }, + ); + return { promise, resolve, reject }; +} +function mount() { + const [props, setProps] = createSignal({ + resourceId: 'a', + resourceType: 'agent', + metric: 'cpu', + range: '1h', + }); + let state!: HistoryChartState; + const view = render(() => { + state = useHistoryChartState( + { + get resourceId() { + return props().resourceId; + }, + get resourceType() { + return props().resourceType; + }, + get metric() { + return props().metric; + }, + get range() { + return props().range; + }, + get data() { + return props().data; + }, + }, + { getCanvas: () => undefined, getContainer: () => undefined }, + ); + return
; + }); + return { + state, + unmount: view.unmount, + change: (next: Partial) => setProps((p) => ({ ...p, ...next })), + }; +} + +describe('History request ownership', () => { + beforeEach(() => { + vi.useFakeTimers(); + request.mockReset(); + }); + afterEach(() => { + cleanup(); + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it.each(['success', 'failure'])( + 'ignores a superseded %s even when cancellation is ignored', + async (completion) => { + const old = deferred(), + current = deferred(); + request.mockReturnValueOnce(old.promise).mockReturnValueOnce(current.promise); + const { state, change } = mount(); + const signal = request.mock.calls[0][0].signal!; + change({ resourceId: 'b' }); + expect(signal.aborted).toBe(true); + current.resolve({ points: points(80), source: 'store' } as never); + await settle(); + if (completion === 'success') old.resolve({ points: points(10), source: 'store' } as never); + else old.reject(new Error('old error')); + await settle(); + expect(state.data()).toEqual(points(80)); + expect(state.error()).toBeNull(); + expect(state.loading()).toBe(false); + }, + ); + + it.each([{ resourceId: 'b' }, { resourceType: 'node' }, { metric: 'memory' }, { range: '6h' }])( + 'clears old readings on selection change %j and exposes its failure', + async (next) => { + request.mockResolvedValueOnce({ points: points(10), source: 'store' } as never); + const { state, change } = mount(); + await settle(); + const current = deferred(); + request.mockReturnValueOnce(current.promise); + change(next as Partial); + expect(state.data()).toEqual([]); + expect(state.loading()).toBe(true); + vi.spyOn(console, 'error').mockImplementation(() => {}); + current.reject(new Error('current failed')); + await settle(); + expect(state.error()).toBe('Failed to load history data'); + expect(state.data()).toEqual([]); + }, + ); + + it('does not overlap polling and preserves matching samples on refresh failure', async () => { + const initial = deferred(); + request.mockReturnValueOnce(initial.promise); + const { state } = mount(); + vi.advanceTimersByTime(120_000); + expect(request).toHaveBeenCalledTimes(1); + initial.resolve({ points: points(10), source: 'store' } as never); + await settle(); + const refresh = deferred(); + request.mockReturnValueOnce(refresh.promise); + vi.advanceTimersByTime(120_000); + expect(request).toHaveBeenCalledTimes(2); + vi.spyOn(console, 'error').mockImplementation(() => {}); + refresh.reject(new Error('refresh failed')); + await settle(); + expect(state.data()).toEqual(points(10)); + expect(state.error()).toBeNull(); + expect(state.loading()).toBe(false); + }); + + it('cancels fetched data when supplied data takes ownership, including empty samples', async () => { + const old = deferred(); + request.mockReturnValueOnce(old.promise); + const { state, change } = mount(); + change({ data: [] }); + expect(request.mock.calls[0][0].signal?.aborted).toBe(true); + old.resolve({ points: points(10), source: 'store' } as never); + await settle(); + vi.advanceTimersByTime(120_000); + expect(request).toHaveBeenCalledTimes(1); + expect(state.data()).toEqual([]); + expect(state.source()).toBe('live'); + change({ data: points(80) }); + expect(state.data()).toEqual(points(80)); + request.mockResolvedValueOnce({ points: points(30), source: 'store' } as never); + change({ data: undefined }); + await settle(); + expect(state.data()).toEqual(points(30)); + }); + + it.each([{ range: '90d' }, { resourceId: '' }])( + 'invalidates in-flight work for unavailable selection %j', + async (next) => { + const old = deferred(); + request.mockReturnValueOnce(old.promise); + const { state, change } = mount(); + change(next as Partial); + old.resolve({ points: points(10), source: 'store' } as never); + await settle(); + vi.advanceTimersByTime(120_000); + expect(request).toHaveBeenCalledTimes(1); + expect(state.data()).toEqual([]); + expect(state.loading()).toBe(false); + }, + ); + + it('aborts and stops polling on unmount without consuming a late completion', async () => { + const old = deferred(); + request.mockReturnValueOnce(old.promise); + const { state, unmount } = mount(); + unmount(); + expect(request.mock.calls[0][0].signal?.aborted).toBe(true); + old.resolve({ points: points(10), source: 'store' } as never); + await settle(); + vi.advanceTimersByTime(120_000); + expect(request).toHaveBeenCalledTimes(1); + expect(state.data()).toEqual([]); + }); +}); diff --git a/frontend-modern/src/components/shared/useHistoryChartState.ts b/frontend-modern/src/components/shared/useHistoryChartState.ts index 2581d680d..e32f39d77 100644 --- a/frontend-modern/src/components/shared/useHistoryChartState.ts +++ b/frontend-modern/src/components/shared/useHistoryChartState.ts @@ -40,8 +40,6 @@ export function useHistoryChartState( null, ); const [maxPoints, setMaxPoints] = createSignal(null); - const [refreshTick, setRefreshTick] = createSignal(0); - const [hasLoadedOnce, setHasLoadedOnce] = createSignal(false); const [localHoveredTimestamp, setLocalHoveredTimestamp] = createSignal(null); const [hoveredPoint, setHoveredPoint] = createSignal(null); const [chartWidth, setChartWidth] = createSignal(300); @@ -63,14 +61,6 @@ export function useHistoryChartState( } }); - createEffect(() => { - if (props.data) { - setData(props.data); - if (!hasLoadedOnce()) setHasLoadedOnce(true); - setSource('live'); - } - }); - const updateRange = (nextRange: HistoryTimeRange) => { setRange(nextRange); props.onRangeChange?.(nextRange); @@ -100,82 +90,71 @@ export function useHistoryChartState( const dataMin = createMemo(() => getHistoryChartDataMin(data())); const dataMax = createMemo(() => getHistoryChartDataMax(data())); - const loadData = async ( - chartRange: HistoryTimeRange, - pointsCap: number | null, - isBackgroundRefresh: boolean, - ) => { - if (!isBackgroundRefresh && !hasLoadedOnce()) { - setLoading(true); - } - setError(null); - if (!isBackgroundRefresh) { - setSource(null); - } - - try { - const result = await ChartsAPI.getMetricsHistory({ - resourceType: props.resourceType, - resourceId: props.resourceId, - metric: props.metric, - range: chartRange, - maxPoints: pointsCap ?? undefined, - }); - - if ('points' in result) { - setData(result.points || []); - setSource(result.source ?? 'store'); - } else { - setData([]); - setSource(result.source ?? 'store'); - } - if (!hasLoadedOnce()) { - setHasLoadedOnce(true); - } - } catch (err) { - console.error('Failed to fetch metrics history:', err); - if (!hasLoadedOnce()) { - setError('Failed to load history data'); - } - setSource(null); - } finally { - setLoading(false); - } - }; - - createEffect(async () => { - if (props.data) return; - if (!props.resourceId || !props.resourceType) return; - + // One effect owns a selection, its request and its polling timer. Cleanup + // invalidates completions even when a transport ignores cancellation. + createEffect(() => { + const suppliedData = props.data; + const resourceId = props.resourceId; + const resourceType = props.resourceType; + const metric = props.metric; const chartRange = range(); - const locked = isLocked(); const pointsCap = maxPoints(); - - if (locked) { - setLoading(false); - setError(null); - setSource(null); - return; - } - - void loadData(chartRange, pointsCap, false); - }); - - createEffect(() => { - const tick = refreshTick(); - if (tick === 0) return; - if (!props.resourceId || !props.resourceType || isLocked()) return; - - void loadData(range(), maxPoints(), true); - }); - - createEffect(() => { + const locked = isLocked(); const interval = refreshIntervalMs(); - if (!interval || interval <= 0) return; - const timer = window.setInterval(() => { - setRefreshTick((value) => value + 1); - }, interval); - onCleanup(() => window.clearInterval(timer)); + let active = true; + let pending = false; + let hasLoaded = false; + let controller: AbortController | undefined; + let timer: number | undefined; + + onCleanup(() => { + active = false; + controller?.abort(); + if (timer !== undefined) window.clearInterval(timer); + }); + + setData(suppliedData ?? []); + setSource(suppliedData !== undefined ? 'live' : null); + setError(null); + setLoading(false); + setHoveredPoint(null); + setHoveredTimestamp(null); + if (suppliedData !== undefined || locked || !resourceId || !resourceType) return; + + const loadData = async () => { + if (!active || pending) return; + pending = true; + controller = new AbortController(); + if (!hasLoaded) setLoading(true); + setError(null); + try { + const result = await ChartsAPI.getMetricsHistory({ + resourceType, + resourceId, + metric, + range: chartRange, + maxPoints: pointsCap ?? undefined, + signal: controller.signal, + }); + if (!active) return; + setData('points' in result ? (result.points ?? []) : []); + setSource(result.source ?? 'store'); + hasLoaded = true; + } catch (err) { + if (!active) return; + console.error('Failed to fetch metrics history:', err); + if (!hasLoaded) setError('Failed to load history data'); + setSource(null); + } finally { + if (active) { + pending = false; + setLoading(false); + } + } + }; + + void loadData(); + if (interval > 0) timer = window.setInterval(() => void loadData(), interval); }); const drawChart = () => {