From 31a9bcebf575e0182b4d146ea1f2ece72f273af8 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 20:11:39 +0100 Subject: [PATCH] Harden discovery assurance feedback --- frontend-modern/browser-verification.json | 32 ++- .../Discovery/AvailabilityProposalCard.tsx | 227 +++++++++++------- .../AvailabilityProposalCard.test.tsx | 180 +++++++++++++- 3 files changed, 341 insertions(+), 98 deletions(-) diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index 895f15791e..afead0ca67 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,13 +1,17 @@ { "version": 1, - "base_sha": "defc24af837b91428fbee939d09cd31e9559fb4f", - "verified_at": "2026-08-30T18:33:03Z", + "base_sha": "b0de28c7c406468310dfbef95b00bd71400d8abb", + "verified_at": "2026-08-30T19:10:01Z", "result": "passed", - "changed_paths": ["frontend-modern/src/components/Discovery/AvailabilityProposalCard.tsx"], + "changed_paths": [ + "frontend-modern/src/components/Discovery/AvailabilityProposalCard.tsx" + ], "content_sha256": { - "frontend-modern/src/components/Discovery/AvailabilityProposalCard.tsx": "8432b423e90ebb0e8dbc56ae82535b992bb7715be688cf6f95be34c1d12564e4" + "frontend-modern/src/components/Discovery/AvailabilityProposalCard.tsx": "0e3c4c5c2bb63ac5c8cd2549cb1fccea3b17d86a8bffa4dd7d0bf0aca7f45000" }, - "routes": ["/docker/overview?q=uptime-kuma"], + "routes": [ + "/browser-assurance-harness.html (temporary current-build Vite harness for the /docker/overview discovery proposal card)" + ], "viewports": [ { "width": 1280, @@ -19,14 +23,18 @@ } ], "states": [ - "current-build AvailabilityProposalCard with a discovered Uptime Kuma verification proposal", - "inline proposal evidence and canonical attachment copy", - "open machine assurance suggestions dialog with one reviewable HTTP service", - "desktop and phone-width proposal card and dialog layouts" + "existing-check inventory loading, failure, retry recovery, empty inventory, and post-create duplicate detection", + "proposal test transport success, application failure, and request failure with scoped status or alert feedback", + "created active-check confirmation alongside duplicate protection", + "machine-review loading, populated, and load-failure states without false empty-state overlap", + "settled desktop and narrow card/dialog pixels from the final current-source Vite render; managed full-app startup was blocked by unrelated pulse-pro go.mod drift" ], "interactions": [ - "opened Review machine suggestions and verified the evidence-specific dismissal copy", - "closed the machine suggestions dialog with Escape", - "verified card and dialog bounds, wrapping, scrolling, and absence of horizontal overflow at desktop and phone widths" + "confirmed Create active check stays disabled until existing-check inventory succeeds and retry restores it", + "ran successful and failed Test proposal actions and confirmed focus returns after the native loading-button blur", + "created an active check and confirmed status plus duplicate feedback", + "opened machine review, checked loading and populated states, closed with Escape, and confirmed focus return", + "reopened machine review on failure and confirmed proposal errors stay outside the dialog and no false empty state appears", + "inspected desktop and 390x844 settled pixels, topmost dialog hit testing, bounds, clipping, scrolling, and horizontal overflow" ] } diff --git a/frontend-modern/src/components/Discovery/AvailabilityProposalCard.tsx b/frontend-modern/src/components/Discovery/AvailabilityProposalCard.tsx index dd3d45b152..d05916192d 100644 --- a/frontend-modern/src/components/Discovery/AvailabilityProposalCard.tsx +++ b/frontend-modern/src/components/Discovery/AvailabilityProposalCard.tsx @@ -77,7 +77,8 @@ export const AvailabilityProposalCard: Component const [saving, setSaving] = createSignal(false); const [testing, setTesting] = createSignal(false); const [updatingDisposition, setUpdatingDisposition] = createSignal(false); - const [error, setError] = createSignal(null); + const [proposalError, setProposalError] = createSignal(null); + const [bulkReviewError, setBulkReviewError] = createSignal(null); const [testResult, setTestResult] = createSignal(null); const [createdTarget, setCreatedTarget] = createSignal(null); const [showBulkReview, setShowBulkReview] = createSignal(false); @@ -97,18 +98,20 @@ export const AvailabilityProposalCard: Component setName(current.service_name || props.discovery.service_name || props.discovery.hostname); setTestResult(null); setCreatedTarget(null); - setError(null); + setProposalError(null); + setBulkReviewError(null); }); const duplicate = createMemo(() => { const current = proposal(); - if (!current) return null; + if (!current || targets.error) return null; return findAvailabilityProposalDuplicate(current, props.canonicalResourceId, targets() ?? []); }); const dismissed = createMemo(() => isAvailabilityProposalDismissed(props.discovery)); - const bulkItems = createMemo(() => - reviewableAvailabilitySummaries(machineDiscoveries()?.discoveries ?? []), - ); + const bulkItems = createMemo(() => { + if (machineDiscoveries.error) return []; + return reviewableAvailabilitySummaries(machineDiscoveries()?.discoveries ?? []); + }); const buildTarget = (): AvailabilityTarget | null => { const current = proposal(); @@ -122,32 +125,33 @@ export const AvailabilityProposalCard: Component }); }; - const handleTest = async () => { + const handleTest = async (trigger?: HTMLButtonElement) => { const target = buildTarget(); if (!target) return; setTesting(true); - setError(null); + setProposalError(null); setTestResult(null); try { setTestResult(await AvailabilityTargetsAPI.test(target)); } catch (cause) { - setError(cause instanceof Error ? cause.message : 'Failed to test this proposal.'); + setProposalError(cause instanceof Error ? cause.message : 'Failed to test this proposal.'); } finally { setTesting(false); + queueMicrotask(() => trigger?.focus()); } }; const handleCreate = async () => { const target = buildTarget(); - if (!target || duplicate()?.kind === 'endpoint') return; + if (!target || targets.loading || targets.error || duplicate()?.kind === 'endpoint') return; setSaving(true); - setError(null); + setProposalError(null); try { const created = await AvailabilityTargetsAPI.create(target); setCreatedTarget(created); await refetchTargets(); } catch (cause) { - setError(cause instanceof Error ? cause.message : 'Failed to create this check.'); + setProposalError(cause instanceof Error ? cause.message : 'Failed to create this check.'); } finally { setSaving(false); } @@ -158,11 +162,11 @@ export const AvailabilityProposalCard: Component if (!current) return; const fingerprint = current.evidence_fingerprint?.trim(); if (!fingerprint) { - setError('Run discovery again before changing this legacy suggestion.'); + setProposalError('Run discovery again before changing this legacy suggestion.'); return; } setUpdatingDisposition(true); - setError(null); + setProposalError(null); try { const updated = await updateAvailabilityProposal( props.resourceType, @@ -174,7 +178,7 @@ export const AvailabilityProposalCard: Component props.onDiscoveryUpdated(updated); if (showBulkReview()) await refetchMachineDiscoveries(); } catch (cause) { - setError( + setProposalError( cause instanceof Error ? cause.message : 'Failed to update this proposal review state.', ); } finally { @@ -190,11 +194,11 @@ export const AvailabilityProposalCard: Component if (!current) return; const fingerprint = current.evidence_fingerprint?.trim(); if (!fingerprint) { - setError('Run discovery again before changing this legacy suggestion.'); + setBulkReviewError('Run discovery again before changing this legacy suggestion.'); return; } setUpdatingDisposition(true); - setError(null); + setBulkReviewError(null); try { const updated = await updateAvailabilityProposal( summary.resource_type, @@ -206,7 +210,9 @@ export const AvailabilityProposalCard: Component if (summary.id === props.discovery.id) props.onDiscoveryUpdated(updated); await refetchMachineDiscoveries(); } catch (cause) { - setError(cause instanceof Error ? cause.message : 'Failed to update the selected proposal.'); + setBulkReviewError( + cause instanceof Error ? cause.message : 'Failed to update the selected proposal.', + ); } finally { setUpdatingDisposition(false); } @@ -234,7 +240,14 @@ export const AvailabilityProposalCard: Component Nothing is created until you choose the active-check action below.

- @@ -330,13 +343,37 @@ export const AvailabilityProposalCard: Component {(match) => ( -
+
{match().kind === 'endpoint' ? `Already covered by “${match().target.name || match().target.address}”. Pulse will not create a duplicate endpoint check.` : `This resource already has “${match().target.name || match().target.address}”, but it covers a different endpoint. Review both before adding another.`}
)} + +
+ Checking existing active checks… +
+ + +
+

+ Pulse could not check for existing active checks. Creating a check is unavailable + until that safety check succeeds. +

+ +
+ {(result) => ( @@ -346,6 +383,7 @@ export const AvailabilityProposalCard: Component ? 'border-emerald-300 bg-emerald-50 text-emerald-900 dark:border-emerald-800 dark:bg-emerald-950/30 dark:text-emerald-200' : 'border-red-300 bg-red-50 text-red-900 dark:border-red-800 dark:bg-red-950/30 dark:text-red-200' }`} + role={result().success ? 'status' : 'alert'} > {testResultLabel(result())}
@@ -353,15 +391,20 @@ export const AvailabilityProposalCard: Component {(created) => ( -
+
)} - + {(message) => ( -

{message()}

+ )} @@ -370,7 +413,12 @@ export const AvailabilityProposalCard: Component size="sm" variant="primary" isLoading={saving()} - disabled={Boolean(createdTarget()) || duplicate()?.kind === 'endpoint'} + disabled={ + targets.loading || + Boolean(targets.error) || + Boolean(createdTarget()) || + duplicate()?.kind === 'endpoint' + } onClick={() => void handleCreate()} > Create active check @@ -379,7 +427,7 @@ export const AvailabilityProposalCard: Component size="sm" variant="secondary" isLoading={testing()} - onClick={() => void handleTest()} + onClick={(event) => void handleTest(event.currentTarget)} > Test proposal @@ -424,69 +472,82 @@ export const AvailabilityProposalCard: Component
-
+
Loading discovered services…
- 0} - fallback={ -

- No availability suggestions are currently available for this machine. -

- } - > - - {(item) => { - const itemProposal = () => item.suggested_availability_probe!; - const itemDismissed = () => isAvailabilityProposalDismissed(item); - return ( -
-
-
-
-
- - {endpointLabel(itemProposal())} - -

- {expectedBehaviorLabel(itemProposal())} · {itemProposal().reason} -

-
- -
-
- ); - }} - + + - + + 0} + fallback={ +

+ No availability suggestions are currently available for this machine. +

+ } + > + + {(item) => { + const itemProposal = () => item.suggested_availability_probe!; + const itemDismissed = () => isAvailabilityProposalDismissed(item); + return ( +
+
+
+
+
+ + {endpointLabel(itemProposal())} + +

+ {expectedBehaviorLabel(itemProposal())} · {itemProposal().reason} +

+
+ +
+
+ ); + }} + + + + {(message) => ( -

{message()}

+ )}
diff --git a/frontend-modern/src/components/Discovery/__tests__/AvailabilityProposalCard.test.tsx b/frontend-modern/src/components/Discovery/__tests__/AvailabilityProposalCard.test.tsx index 31fd7bdb43..1c29edcc5f 100644 --- a/frontend-modern/src/components/Discovery/__tests__/AvailabilityProposalCard.test.tsx +++ b/frontend-modern/src/components/Discovery/__tests__/AvailabilityProposalCard.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen, waitFor } from '@solidjs/testing-library'; +import { fireEvent, render, screen, waitFor, within } from '@solidjs/testing-library'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { AvailabilityTargetsAPI } from '@/api/availabilityTargets'; import { listDiscoveriesByAgent, updateAvailabilityProposal } from '@/api/discovery'; @@ -120,6 +120,9 @@ describe('AvailabilityProposalCard', () => { await fireEvent.change(screen.getByLabelText('Observation location · you control'), { target: { value: 'edge-1' }, }); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Create active check' })).toBeEnabled(), + ); await fireEvent.click(screen.getByRole('button', { name: 'Create active check' })); await waitFor(() => expect(AvailabilityTargetsAPI.create).toHaveBeenCalledTimes(1)); @@ -135,7 +138,10 @@ describe('AvailabilityProposalCard', () => { }), }), ); - expect(screen.getByText(/created and attached to this resource/i)).toBeInTheDocument(); + expect(screen.getByText(/created and attached to this resource/i)).toHaveAttribute( + 'role', + 'status', + ); }); it('blocks an equivalent standalone endpoint and records evidence-bound dismissal', async () => { @@ -164,7 +170,10 @@ describe('AvailabilityProposalCard', () => { )); await waitFor(() => - expect(screen.getByText(/already covered by “Existing Grafana”/i)).toBeInTheDocument(), + expect(screen.getByText(/already covered by “Existing Grafana”/i)).toHaveAttribute( + 'role', + 'status', + ), ); expect(screen.getByRole('button', { name: 'Create active check' })).toBeDisabled(); @@ -180,4 +189,169 @@ describe('AvailabilityProposalCard', () => { ); expect(onDiscoveryUpdated).toHaveBeenCalled(); }); + + it('keeps creation unavailable until duplicate checking succeeds', async () => { + let resolveTargets!: (targets: []) => void; + vi.mocked(AvailabilityTargetsAPI.list).mockImplementation( + () => new Promise((resolve) => (resolveTargets = resolve)), + ); + + render(() => ( + + )); + + expect(screen.getByText('Checking existing active checks…')).toHaveAttribute('role', 'status'); + const createButton = screen.getByRole('button', { name: 'Create active check' }); + expect(createButton).toBeDisabled(); + await fireEvent.click(createButton); + expect(AvailabilityTargetsAPI.create).not.toHaveBeenCalled(); + + resolveTargets([]); + await waitFor(() => expect(createButton).toBeEnabled()); + }); + + it('fails duplicate checking closed and explains why creation is unavailable', async () => { + vi.mocked(AvailabilityTargetsAPI.list) + .mockRejectedValueOnce(new Error('inventory unavailable')) + .mockResolvedValueOnce([]); + + render(() => ( + + )); + + const failure = await screen.findByText(/could not check for existing active checks/i); + expect(failure).toHaveAttribute('role', 'alert'); + const createButton = screen.getByRole('button', { name: 'Create active check' }); + expect(createButton).toBeDisabled(); + + await fireEvent.click(screen.getByRole('button', { name: 'Retry existing-check scan' })); + await waitFor(() => expect(createButton).toBeEnabled()); + }); + + it('announces proposal test success and failure without moving focus', async () => { + let resolveTest!: (result: { + success: true; + latencyMillis: number; + application: { outcome: 'passed'; statusCode: number }; + }) => void; + vi.mocked(AvailabilityTargetsAPI.test).mockImplementationOnce( + () => new Promise((resolve) => (resolveTest = resolve)), + ); + + render(() => ( + + )); + + const testButton = screen.getByRole('button', { name: 'Test proposal' }); + testButton.focus(); + await fireEvent.click(testButton); + testButton.blur(); + resolveTest({ + success: true, + latencyMillis: 18, + application: { outcome: 'passed', statusCode: 200 }, + }); + const success = await screen.findByText(/application response passed with HTTP 200/i); + expect(success).toHaveAttribute('role', 'status'); + expect(document.activeElement).toBe(testButton); + + vi.mocked(AvailabilityTargetsAPI.test).mockResolvedValueOnce({ + success: false, + latencyMillis: 0, + error: 'Connection refused', + }); + await fireEvent.click(testButton); + const failure = await screen.findByText('Connection refused'); + expect(failure).toHaveAttribute('role', 'alert'); + expect(document.activeElement).toBe(testButton); + }); + + it('does not claim machine suggestions are empty while they are still loading', async () => { + let resolveDiscoveries!: (result: { discoveries: []; total: 0 }) => void; + vi.mocked(listDiscoveriesByAgent).mockImplementation( + () => new Promise((resolve) => (resolveDiscoveries = resolve)), + ); + + render(() => ( + + )); + + await fireEvent.click(screen.getByRole('button', { name: 'Review machine suggestions' })); + expect(screen.getByText('Loading discovered services…')).toHaveAttribute('role', 'status'); + expect(screen.queryByText(/no availability suggestions are currently available/i)).toBeNull(); + + resolveDiscoveries({ discoveries: [], total: 0 }); + expect( + await screen.findByText(/no availability suggestions are currently available/i), + ).toBeInTheDocument(); + }); + + it('does not leak a proposal action failure into machine review', async () => { + vi.mocked(AvailabilityTargetsAPI.test).mockRejectedValueOnce(new Error('Probe test failed')); + + render(() => ( + + )); + + await fireEvent.click(screen.getByRole('button', { name: 'Test proposal' })); + expect(await screen.findByText('Probe test failed')).toHaveAttribute('role', 'alert'); + + await fireEvent.click(screen.getByRole('button', { name: 'Review machine suggestions' })); + expect(within(screen.getByRole('dialog')).queryByText('Probe test failed')).toBeNull(); + }); + + it('reports machine suggestion load failures instead of showing an empty state', async () => { + vi.mocked(listDiscoveriesByAgent).mockRejectedValue(new Error('discovery unavailable')); + + render(() => ( + + )); + + await fireEvent.click(screen.getByRole('button', { name: 'Review machine suggestions' })); + const failure = await screen.findByText(/could not load this machine’s assurance suggestions/i); + expect(failure).toHaveAttribute('role', 'alert'); + expect(screen.queryByText(/no availability suggestions are currently available/i)).toBeNull(); + }); });