diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index ec3fc5d064..b88a424f2d 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -2727,6 +2727,11 @@ agent inventory, registration state, or command-channel readiness. reports, but workspace logo material remains API/security/reporting ownership and must not become agent credential, install-token, or fleet lifecycle state. + Report alert rows built there carry an alert's handover resolution, so a + node alert that moved to its linked Pulse agent reads as moved, not + recovered. That is report presentation only: the report never decides + agent linkage, ownership of a metric, or agent freshness, and it adds no + lifecycle state. The same isolation rule applies to Patrol investigation-record propagation through shared AI intelligence handlers and `internal/api/router.go`: lifecycle surfaces may observe the resulting resource context, but they must diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 56b2a47c66..d233ab4bf9 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -4651,9 +4651,14 @@ metadata when it projects from the canonical timeline). `TestService_buildAlertContext_HandoverIsNotARecovery`, `TestExecuteListResolvedAlertsReportsHandover` and `TestIncidentStore_HandoverCloseIsNotARecovery` pin it. Performance reports -(`pkg/reporting`, the report narrator) do not carry the resolution yet: a -report generated within the five-minute recently-resolved window after a -handover still lists that alert as resolved. The projection also collapses open/resolved churn: +carry the resolution too: the report narrator sends such an alert with +`resolved: false` and the engine's summary as `resolution`, plus +`successor_alert_listed` when the same report lists the alert the resource +it moved to raised for that metric. Its system prompt forbids counting the +alert as resolved or calling the resource healthy on its account, and tells +the model to describe a covered condition once, from the successor's alert +(`TestBuildReportNarratorPayload_HandoverIsNotARecovery`). The +deterministic verdict lives in `pkg/reporting` (see api-contracts). The projection also collapses open/resolved churn: when a record's timeline holds at least four open-to-resolved or resolved-to-open transitions inside the last 24 hours, the item carries a `flapping` summary (transition count, window, first and latest transition) and diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 8454240e42..b8d575681b 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -9598,6 +9598,39 @@ narrator must fail closed: nil provider, parse failure, timeout, or empty response causes the engine to fall back to the heuristic narrative without surfacing the AI failure to the caller, so reporting is never blocked by AI availability. +Report alerts keep the alert engine's resolution. The node, VM and +container enrichers in `internal/api/metrics_reporting_handlers.go` build +every row through `reportActiveAlertInfo` and `reportResolvedAlertInfo`, +which carry the alert's `ResourceID` and copy `models.AlertResolution` into +`reporting.AlertInfo.Resolution`, so a node alert that moved to its Pulse +agent inside the five-minute recently-resolved window is never reported as +a recovery (`TestContract_ReportingAlertsCarryHandoverResolution`). In +`pkg/reporting`, `AlertInfo.Recovered()` is true only for a close without a +resolution, and `unresolvedAlerts` treats an alert that closed without +recovering as still live unless the report also shows its successor's own +reading for that metric: an alert of the same type on the successor or one +of its children that is open, or recovered at or after the handover (an +earlier successor recovery does not count). That alert then carries the +condition, so it is counted once. Fleet reports match each handover against +every resource's alerts (`unresolvedAlertsAmong` over +`MultiReportData.fleetAlerts`). The single-resource PDF health card, Quick +Stats and active list and the heuristic narrator's verdict, observations and +recommendations read that rule: a lone handover renders WARNING (or CRITICAL +at its level) with `Alert moved to - check the agent for the current +reading`, and the alerts table shows `Moved` in neutral text with the +engine's summary as a note. Fleet reports count it as an active alert in the +aggregate, health card, table and resource blocks, and the fleet heuristic +recommends checking the agent it moved to. "Triggered and resolved" counts +recoveries only, and the report narrator's payload marks a covered handover +with `successor_alert_listed` so the model describes the condition once. +Reports requested as `agent`, which is how the Reports picker addresses every +Proxmox node and standalone agent in v6, carry no alert rows yet: +`enrichReportRequest` enriches node, VM and container reports only, so an +agent report's deterministic verdict ignores its machine's alerts (HEALTHY, +or NO DATA without metrics). The rule above reaches those reports once they +attach the machine's alerts. The fleet heuristic offers its all-clear +pattern and "No fleet-wide action required" only while no alert, moved +ones included, is active. Multi-resource fleet reports (`engine.GenerateMulti`) now also carry an optional fleet-level narrative through a distinct `pkg/reporting.FleetNarrator` interface, kept separate from the diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 710f4ef07e..fc27a49e3a 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -7120,6 +7120,12 @@ row carries the account in the badge title and the phone card renders it under the message, while `getAlertHistoryStatusPresentation` and `getAlertResolutionDetail` in `utils/alertIncidentPresentation.ts` own the wording. +Resource-change readers follow the same split: `getResourceChangePresentation` +and `formatResourceChangeHeadline` in `utils/resourceChangePresentation.ts` +own the `Alert moved` label, its neutral-blue tone and the summary headline +for an `alert_resolved` change with `alert_resolution` metadata, and the +Patrol assessment handoff consumes that headline instead of composing its own +kind prefix. `frontend-modern/src/features/alerts/useAlertHistoryState.ts` re-exposes the `getResource` resolver it is already given, and `frontend-modern/src/features/alerts/AlertResourceIncidentsPanel.tsx` reads it diff --git a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md index 251bfc8252..46765dbb24 100644 --- a/docs/release-control/v6/internal/subsystems/patrol-intelligence.md +++ b/docs/release-control/v6/internal/subsystems/patrol-intelligence.md @@ -1388,6 +1388,12 @@ condition went, and that it is not a recovery) sits directly under the summary rather than only inside Evidence and history (`getAttentionHandoverClose` in `patrolHomePresentation.ts`). The timeline names that cause `Ownership transferred`, never `Recovery evidence`. +The assessment handoff tells the same close the same way: a recent +`alert_resolved` change carrying `alert_resolution` metadata enters the +handoff context and briefing evidence as the alert engine's summary through +the shared `formatResourceChangeHeadline`, never as `Alert resolved: ...` +(`patrolInvestigationContextModel.ts`, pinned by +`patrolInvestigationContextModel.test.ts`). The active Patrol queue now uses compact severity-accented rows for prioritisation and a single focused review panel for the selected issue. The diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 626b08f763..e90edfe932 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -1470,6 +1470,10 @@ recovery scope, or a storage/recovery-owned secret source. generated report output when a separate reporting surface exposes it, but workspace logo settings are not backup artifacts, recovery-point metadata, restore evidence, or storage-provider credentials. + The alert rows that request assembly builds now keep an alert's + handover resolution (moved to a Pulse agent, not recovered). That changes + how a report states alert health only; it opens no storage, backup or + recovery path, and report backups still come from the recovery store. Update-plan readiness payloads and apply-route readiness enforcement are adjacent shared API context only. Storage and recovery surfaces may observe the resulting update state if a future settings flow links to recovery diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 7fe6d54e2f..829435c249 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -4228,10 +4228,22 @@ An `alert_resolved` change for a close that was not a recovery carries the alert engine's summary as its reason and the reason code as `alert_resolution` metadata (`AlertTimelineChange.ResolutionReason` and `ResolutionSummary`), so incident projection titles the close with where the alert went instead of -"Alert resolved: ". The resource drawer still labels the change -by kind (`Alert resolved`) and prefixes its headline with that label; only the -reason after it says the alert moved. Ordinary recoveries omit the key, and -every other lifecycle kind ignores the fields. +"Alert resolved: ". Frontend readers label the change through +`getResourceChangePresentation(change)` rather than by kind alone: with the +metadata set, the drawer's change history titles the entry `Alert moved` and +the `ResourceChangeSummary` badge reads `Alert moved` in the neutral blue of +the alert history's "moved to agent" badge (`Alert closed` for a reason code +the build does not know), and +`formatResourceChangeHeadline` uses the summary as the whole headline instead +of prefixing `Alert resolved:`; Patrol's assessment context reuses that +headline. A node's own drawer does not list the move yet: node and guest +alert changes are still recorded under their legacy alert resource IDs +(`MonitorAdapter.RecordChange` maps only Docker IDs), and Proxmox platform +rows open the drawer in the `table-row` presentation, which fetches no +remote history. The facet badges +still count these changes under `alert_resolved`, because the counts carry no +metadata. Ordinary recoveries omit the key, and every other lifecycle kind +ignores the fields. Alert-scoped incident memory may still project those events for one investigation thread, but the durable source of truth for resource-affecting alert lifecycle and diff --git a/frontend-modern/browser-verification/3cc753f21d394de0.json b/frontend-modern/browser-verification/3cc753f21d394de0.json new file mode 100644 index 0000000000..2c2808e61e --- /dev/null +++ b/frontend-modern/browser-verification/3cc753f21d394de0.json @@ -0,0 +1,45 @@ +{ + "version": 1, + "base_sha": "d26a4837315b40b47a73dbf1ff1cb4c8d6075225", + "verified_at": "2026-10-06T15:52:00Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Infrastructure/ResourceChangeSummary.tsx", + "frontend-modern/src/components/Infrastructure/ResourceDetailDrawerOverviewTab.tsx", + "frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts", + "frontend-modern/src/utils/resourceChangePresentation.ts" + ], + "content_sha256": { + "frontend-modern/src/components/Infrastructure/ResourceChangeSummary.tsx": "a7d8728f72de5b9acd73832064f037af97978b78859cb2c69a01e25f0750e6d5", + "frontend-modern/src/components/Infrastructure/ResourceDetailDrawerOverviewTab.tsx": "f0f868d614ef70165b77e80f220cc5927936bdfcfa4eff3a8f18b728acaf5b52", + "frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts": "e172bfead859ece6f7f9998dcd765c6f348bbfc25214089d6ba1a150f30d6fec", + "frontend-modern/src/utils/resourceChangePresentation.ts": "5a8d67301975d89545c866116e1e0e7e633881b3b3ce4da37b825a46cf65265d" + }, + "routes": [ + "/proxmox/overview?agent=Production+West-pve1", + "/standalone/availability?view=fleet" + ], + "viewports": [ + { + "width": 1280, + "height": 900 + }, + { + "width": 375, + "height": 812 + } + ], + "states": [ + "Private mock stack (backend :7741 from this branch plus a local-only go build -overlay hook, Vite :5241 from this clone): the real alerts manager raised pve1 node memory and CPU alerts, the hook closed the memory alert through clearAlertWithResolution with a moved_to_agent resolution (successor pve1 (Host Agent)), and threshold restore recovered CPU normally; the monitor recorded both alert_resolved changes, the moved one with the engine summary as reason and alert_resolution metadata", + "Proxmox overview, West Production A inline drawer (table-row presentation): Overview and History tabs render, no change history section, no facets request; table-row drawers skip remote history by design and node alert changes are stored under the legacy node ID mock-cluster-1-pve1, so the move cannot appear in a node drawer today", + "Availability checks Fleet view, Rack UPS network card full drawer, with the two producer-made alert_resolved rows (plus their alert_fired rows) copied from mock-cluster-1-pve1 to this endpoint's canonical ID in unified_resources.db as a fixture: at 1280 the change history lists 'Alert moved' over 'Alert moved to pve1 (Host Agent). This is not a recovery: check the agent for the current reading.' and the ordinary close still reads 'Alert resolved' over 'Alert resolved: Node cpu 5-minute average at 33.7% (current 33.7%)'", + "Same drawer at 375: both entries wrap inside the card, the moved entry is titled Alert moved, no horizontal page scroll; facet chips still read 'Alert resolved 2' because kind counts carry no metadata (documented in unified-resources)", + "Console: only the known private-port websocket rejection (ws://127.0.0.1:5241/ws, ALLOWED_ORIGINS lists 5173/7655), no errors from the drawer" + ], + "interactions": [ + "Expanded West Production A on /proxmox/overview, opened its History tab and checked the DOM and network log for a change history section and a facets request", + "Switched Availability checks to the Fleet view through View preferences and opened the Rack UPS network card tile", + "Scrolled the change history to the Alert moved entry at 1280x900 and read every entry title and reason", + "Reloaded at 375x812, reopened the tile, read the entries and compared document width with the viewport" + ] +} diff --git a/frontend-modern/src/components/Infrastructure/ResourceChangeSummary.tsx b/frontend-modern/src/components/Infrastructure/ResourceChangeSummary.tsx index 05518bf5e9..a05fd6374e 100644 --- a/frontend-modern/src/components/Infrastructure/ResourceChangeSummary.tsx +++ b/frontend-modern/src/components/Infrastructure/ResourceChangeSummary.tsx @@ -2,7 +2,7 @@ import { type Component, For, Show, createMemo } from 'solid-js'; import type { ResourceChange } from '@/types/resource'; import { formatRelativeTime } from '@/utils/format'; import { - getResourceChangeKindPresentation, + getResourceChangePresentation, getResourceChangeSourceAdapterPresentation, getResourceChangeSourceTypePresentation, formatResourceChangeHeadline, @@ -83,7 +83,7 @@ export const ResourceChangeSummary: Component = (pro const shouldShowReason = reason.length > 0 && reason !== headline; const resourceHref = buildResourceHref(change.resourceId); const relatedResources = (change.relatedResources ?? []).slice(0, 3); - const kindPresentation = getResourceChangeKindPresentation(change.kind); + const kindPresentation = getResourceChangePresentation(change); const sourceTypePresentation = getResourceChangeSourceTypePresentation( change.sourceType, ); diff --git a/frontend-modern/src/components/Infrastructure/ResourceDetailDrawerOverviewTab.tsx b/frontend-modern/src/components/Infrastructure/ResourceDetailDrawerOverviewTab.tsx index dc927cdc7f..189a4f6942 100644 --- a/frontend-modern/src/components/Infrastructure/ResourceDetailDrawerOverviewTab.tsx +++ b/frontend-modern/src/components/Infrastructure/ResourceDetailDrawerOverviewTab.tsx @@ -48,6 +48,7 @@ import { RESOURCE_CHANGE_SOURCE_ADAPTER_ORDER, RESOURCE_CHANGE_SOURCE_TYPE_ORDER, getResourceChangeKindPresentation, + getResourceChangePresentation, getResourceChangeSourceAdapterPresentation, getResourceChangeSourceTypePresentation, } from '@/utils/resourceChangePresentation'; @@ -592,7 +593,8 @@ export const ResourceDetailDrawerOverviewTab: Component {(change) => { - const kindPresentation = getResourceChangeKindPresentation(change.kind); + // A close that was not a recovery reads as a move, not "Alert resolved". + const kindPresentation = getResourceChangePresentation(change); const sourceTypePresentation = getResourceChangeSourceTypePresentation( change.sourceType, ); diff --git a/frontend-modern/src/components/Infrastructure/__tests__/ResourceDetailDrawer.history.test.tsx b/frontend-modern/src/components/Infrastructure/__tests__/ResourceDetailDrawer.history.test.tsx index 800c3f0b7f..f5e53ea7de 100644 --- a/frontend-modern/src/components/Infrastructure/__tests__/ResourceDetailDrawer.history.test.tsx +++ b/frontend-modern/src/components/Infrastructure/__tests__/ResourceDetailDrawer.history.test.tsx @@ -916,6 +916,66 @@ describe('ResourceDetailDrawer change history section', () => { expect(panel.queryByText('Runs on')).toBeNull(); }); + it('labels an alert that moved to its Pulse agent as a move, not a recovery', async () => { + const summary = + 'Alert moved to pve1 (Host Agent). This is not a recovery: check the agent for the current reading.'; + facetBundleMock.getFacetBundle.mockResolvedValueOnce({ + recentChanges: [ + { + id: 'change-moved', + observedAt: '2026-10-06T15:00:00Z', + occurredAt: '2026-10-06T15:00:00Z', + resourceId: 'node:pve1', + kind: 'alert_resolved', + sourceType: 'heuristic', + confidence: 'high', + reason: summary, + metadata: { + alert_identifier: 'pve1-memory', + alert_type: 'memory', + alert_resolution: 'moved_to_agent', + }, + }, + { + id: 'change-recovered', + observedAt: '2026-10-06T14:00:00Z', + resourceId: 'node:pve1', + kind: 'alert_resolved', + sourceType: 'heuristic', + confidence: 'high', + reason: 'Alert resolved: Node CPU at 91%', + metadata: { alert_identifier: 'pve1-cpu', alert_type: 'cpu' }, + }, + ], + counts: { recentChanges: 2, recentChangeKinds: { alert_resolved: 2 } }, + }); + + render(() => ( + + )); + + await screen.findByText('Change history'); + const panel = within(screen.getByTestId('resource-change-history-section')); + const movedReason = await panel.findByText(summary); + const movedEntry = movedReason.parentElement as HTMLElement; + expect(within(movedEntry).getByText('Alert moved')).toBeInTheDocument(); + expect(within(movedEntry).queryByText('Alert resolved')).toBeNull(); + + // An ordinary recovery keeps its kind label. + const recoveredEntry = panel.getByText('Alert resolved: Node CPU at 91%') + .parentElement as HTMLElement; + expect(within(recoveredEntry).getByText('Alert resolved')).toBeInTheDocument(); + }); + it('surfaces resource-scoped action history from the canonical action audit API', async () => { actionAuditMock.listActionAudits.mockResolvedValueOnce({ available: true, diff --git a/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts b/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts index 9e11a59853..3b9656c15b 100644 --- a/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts +++ b/frontend-modern/src/features/patrol/__tests__/patrolInvestigationContextModel.test.ts @@ -165,6 +165,33 @@ describe('patrolInvestigationContextModel', () => { expect(handoff.context.handoffContext).not.toContain('online to online'); }); + it("tells a moved alert close in the alert engine's words, not as a recovery", () => { + const summary = + 'Alert moved to pve1 (Host Agent). This is not a recovery: check the agent for the current reading.'; + const handoff = buildPatrolAssessmentAssistantHandoff({ + assessment: { title: 'Issues detected' }, + supportingEvidence: { + recentChanges: [ + { + id: 'change-moved', + observedAt: '2026-10-06T15:00:00Z', + resourceId: 'node:pve1', + kind: 'alert_resolved', + sourceType: 'heuristic', + confidence: 'high', + reason: summary, + metadata: { alert_resolution: 'moved_to_agent', alert_type: 'memory' }, + }, + ], + }, + activeFindings: [], + }); + + expect(handoff.context.handoffContext).toContain(summary); + expect(handoff.context.handoffContext).not.toContain('Alert resolved'); + expect(handoff.context.briefing?.evidence?.join('\n')).toContain(summary); + }); + it('includes bounded related-resource context in assessment handoff evidence', () => { const longRelatedResource = 'storage-pool-with-a-very-long-description-that-keeps-going-beyond-the-handoff-limit-for-operators'; diff --git a/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts b/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts index 1d1a648191..2cf8f21eaa 100644 --- a/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts +++ b/frontend-modern/src/features/patrol/patrolInvestigationContextModel.ts @@ -15,7 +15,9 @@ import type { } from '@/stores/aiChat'; import type { ResourceChange } from '@/types/resource'; import { + formatResourceChangeHeadline, formatResourceChangeKind, + getResourceChangeAlertResolution, sortResourceChangesByObservedAt, } from '@/utils/resourceChangePresentation'; import { @@ -1736,6 +1738,12 @@ function formatAssessmentCorrelationContextLine( } function formatAssessmentRecentChangeSummary(change: ResourceChange): string { + // An alert that closed without recovering (moved to its Pulse agent) is + // told in the alert engine's words, never as "Alert resolved". + if (getResourceChangeAlertResolution(change)) { + return truncateContextText(formatResourceChangeHeadline(change), 160); + } + const kind = formatResourceChangeKind(change.kind); if (isCommandBearingResourceChange(change)) { return `${kind}: execution event recorded`; diff --git a/frontend-modern/src/utils/__tests__/resourceChangePresentation.test.ts b/frontend-modern/src/utils/__tests__/resourceChangePresentation.test.ts index c6539e2bbf..60c2bb9be0 100644 --- a/frontend-modern/src/utils/__tests__/resourceChangePresentation.test.ts +++ b/frontend-modern/src/utils/__tests__/resourceChangePresentation.test.ts @@ -3,7 +3,9 @@ import { describe, expect, it } from 'vitest'; import { formatResourceChangeHeadline, formatResourceChangeKind, + getResourceChangeAlertResolution, getResourceChangeKindPresentation, + getResourceChangePresentation, getResourceChangeSourceAdapterPresentation, getResourceChangeSourceTypePresentation, sortResourceChangesByObservedAt, @@ -90,6 +92,59 @@ describe('resourceChangePresentation utils', () => { }); }); + it('presents an alert close that was not a recovery as a move', () => { + const summary = + 'Alert moved to pve1 (Host Agent). This is not a recovery: check the agent for the current reading.'; + const moved = { + id: 'change-moved', + resourceId: 'node:pve1', + kind: 'alert_resolved' as const, + observedAt: '2026-10-06T15:00:00Z', + sourceType: 'heuristic' as const, + confidence: 'high' as const, + reason: summary, + metadata: { alert_resolution: 'moved_to_agent', alert_type: 'memory' }, + }; + + expect(getResourceChangeAlertResolution(moved)).toBe('moved_to_agent'); + expect(getResourceChangePresentation(moved)).toEqual({ + label: 'Alert moved', + plural: 'Alerts moved', + className: 'bg-blue-100 text-blue-700 dark:bg-blue-900/25 dark:text-blue-300', + }); + // The engine's account is the whole headline, never "Alert resolved: ...". + expect(formatResourceChangeHeadline(moved)).toBe(summary); + expect(formatResourceChangeHeadline({ ...moved, reason: undefined })).toBe( + 'Alert moved: node:pve1', + ); + + // A reason code this build does not know still never reads as a recovery. + expect( + getResourceChangePresentation({ + kind: 'alert_resolved', + metadata: { alert_resolution: 'some_future_reason' }, + }).label, + ).toBe('Alert closed'); + + // Ordinary recoveries, and other kinds, keep their kind presentation. + const recovered = { + ...moved, + metadata: { alert_type: 'memory' }, + reason: 'Node memory at 95%', + }; + expect(getResourceChangeAlertResolution(recovered)).toBeUndefined(); + expect(getResourceChangePresentation(recovered)).toEqual( + getResourceChangeKindPresentation('alert_resolved'), + ); + expect(formatResourceChangeHeadline(recovered)).toBe('Alert resolved: Node memory at 95%'); + expect( + getResourceChangePresentation({ + kind: 'alert_fired', + metadata: { alert_resolution: 'moved_to_agent' }, + }).label, + ).toBe('Alert fired'); + }); + it('formats shared confidence percentages', () => { expect(formatConfidencePercentage(0.5)).toBe('50%'); expect(formatConfidencePercentage(0.875)).toBe('88%'); diff --git a/frontend-modern/src/utils/resourceChangePresentation.ts b/frontend-modern/src/utils/resourceChangePresentation.ts index 8e5dd04848..bcb612cba1 100644 --- a/frontend-modern/src/utils/resourceChangePresentation.ts +++ b/frontend-modern/src/utils/resourceChangePresentation.ts @@ -210,6 +210,62 @@ export function getResourceChangeKindPresentation( ); } +// Metadata key carrying the reason code of an alert_resolved change whose +// close was not a recovery (unifiedresources.MetadataAlertResolution). +// Ordinary recoveries omit it. +const ALERT_RESOLUTION_METADATA_KEY = 'alert_resolution'; + +// A close that was not a recovery is neither a live problem nor a healthy +// outcome, so it reads in the same neutral blue as the alert history's +// "moved to agent" badge. +const ALERT_CLOSED_WITHOUT_RECOVERY_CLASS = + 'bg-blue-100 text-blue-700 dark:bg-blue-900/25 dark:text-blue-300'; + +const ALERT_CLOSED_WITHOUT_RECOVERY_PRESENTATIONS: Record = + { + moved_to_agent: { + label: 'Alert moved', + plural: 'Alerts moved', + className: ALERT_CLOSED_WITHOUT_RECOVERY_CLASS, + }, + }; + +const ALERT_CLOSED_WITHOUT_RECOVERY_FALLBACK: ResourceChangeLabelPresentation = { + label: 'Alert closed', + plural: 'Alerts closed', + className: ALERT_CLOSED_WITHOUT_RECOVERY_CLASS, +}; + +/** + * The reason code of an alert_resolved change that closed without recovering, + * such as "moved_to_agent" when a node alert moved to its Pulse agent. + * Undefined for an ordinary recovery and every other kind. + */ +export function getResourceChangeAlertResolution( + change: Pick, +): string | undefined { + if (change.kind !== 'alert_resolved') return undefined; + const reason = change.metadata?.[ALERT_RESOLUTION_METADATA_KEY]; + return typeof reason === 'string' && reason.trim() !== '' ? reason.trim() : undefined; +} + +/** + * Presentation for one change: its kind's, except that an alert close that + * was not a recovery reads as a move instead of a green "Alert resolved". + */ +export function getResourceChangePresentation( + change: Pick, +): ResourceChangeLabelPresentation { + const resolution = getResourceChangeAlertResolution(change); + if (resolution) { + return ( + ALERT_CLOSED_WITHOUT_RECOVERY_PRESENTATIONS[resolution] ?? + ALERT_CLOSED_WITHOUT_RECOVERY_FALLBACK + ); + } + return getResourceChangeKindPresentation(change.kind); +} + export function getResourceChangeSourceTypePresentation( sourceType: ResourceChangeSourceType | string, ): ResourceChangeLabelPresentation { @@ -266,6 +322,12 @@ export function formatResourceChangeKind(kind: ResourceChange['kind']): string { } export function formatResourceChangeHeadline(change: ResourceChange): string { + // The alert engine's account of a close that was not a recovery already + // says where the alert went, so it is the headline on its own. + if (getResourceChangeAlertResolution(change)) { + const reason = change.reason?.trim(); + return reason || `${getResourceChangePresentation(change).label}: ${change.resourceId}`; + } if (change.kind === 'state_transition' && change.from && change.to) { return `${formatResourceChangeKind(change.kind)}: ${change.from} → ${change.to}`; } diff --git a/internal/ai/report_narrator.go b/internal/ai/report_narrator.go index 8ea890cbc0..7160e880bd 100644 --- a/internal/ai/report_narrator.go +++ b/internal/ai/report_narrator.go @@ -38,6 +38,7 @@ You MUST: - Keep observations to short, concrete sentences. Avoid hedging adverbs ("perhaps", "seems"). State the fact and its implication. - When prior-period data is supplied, write a period_comparison paragraph describing the most material deltas (resource trends, new or resolved alerts, new findings). When no prior data is supplied, leave period_comparison empty. - Do NOT invent recommendations for problems that aren't in the data. If everything is healthy, say so plainly. +- An alert with a "resolution" closed without recovering, for example because the Pulse agent now monitoring the machine took its metric over. It is not a recovery: never count it as resolved or call the resource healthy because of it, and say where it went using the resolution text. When it also has "successor_alert_listed": true, the alerts list holds the alert the resource it moved to raised for that metric; describe the condition once, from that alert. DETECTION BOUNDARY — this is critical: Pulse Patrol is the canonical detection layer for this product. The patrol_findings array, when populated, contains issues Patrol has already classified with its own severity. Your job is to SUMMARIZE those findings, not to function as a parallel detector competing with them. @@ -118,7 +119,14 @@ type reportNarratorAlert struct { Message string `json:"message"` Value float64 `json:"value,omitempty"` Threshold float64 `json:"threshold,omitempty"` - Resolved bool `json:"resolved"` + // Resolved is true only for a recovery. An alert that closed without + // recovering, such as a node alert that moved to its Pulse agent, carries + // the alert engine's account in Resolution instead, and + // SuccessorAlertListed when this list also holds the alert the resource + // it moved to raised for the same metric. + Resolved bool `json:"resolved"` + Resolution string `json:"resolution,omitempty"` + SuccessorAlertListed bool `json:"successor_alert_listed,omitempty"` } type reportNarratorStorage struct { @@ -318,12 +326,14 @@ func buildReportNarratorPayload(in reporting.NarrativeInput) reportNarratorPaylo for _, alert := range in.Alerts { payload.Alerts = append(payload.Alerts, reportNarratorAlert{ - Type: alert.Type, - Level: alert.Level, - Message: alert.Message, - Value: alert.Value, - Threshold: alert.Threshold, - Resolved: alert.ResolvedTime != nil, + Type: alert.Type, + Level: alert.Level, + Message: alert.Message, + Value: alert.Value, + Threshold: alert.Threshold, + Resolved: alert.Recovered(), + Resolution: alert.ResolutionSummary(), + SuccessorAlertListed: alert.SuccessorAlertListed(in.Alerts), }) } diff --git a/internal/ai/report_narrator_test.go b/internal/ai/report_narrator_test.go index cd233f386b..fc56b4354a 100644 --- a/internal/ai/report_narrator_test.go +++ b/internal/ai/report_narrator_test.go @@ -272,3 +272,45 @@ func TestBuildReportNarratorPayload_PeriodFormatting(t *testing.T) { t.Errorf("Period.Hours = %d, want 24", payload.Period.Hours) } } + +// A node alert that moved to its Pulse agent closed without recovering, so the +// model must not see it as resolved: it gets the engine's account instead. +func TestBuildReportNarratorPayload_HandoverIsNotARecovery(t *testing.T) { + resolvedAt := time.Date(2026, 10, 6, 15, 0, 0, 0, time.UTC) + summary := "Alert moved to pve1 (Host Agent). This is not a recovery: check the agent for the current reading." + payload := buildReportNarratorPayload(reporting.NarrativeInput{ + Alerts: []reporting.AlertInfo{ + { + Type: "memory", + Level: "warning", + Message: "Node memory at 95%", + ResolvedTime: &resolvedAt, + Resolution: &reporting.AlertResolution{ + Reason: reporting.AlertResolutionMovedToAgent, + SuccessorResourceID: "agent:host-1", + SuccessorName: "pve1 (Host Agent)", + Summary: summary, + }, + }, + {Type: "cpu", Level: "warning", Message: "Node CPU at 91%", ResolvedTime: &resolvedAt}, + {Type: "memory", Level: "warning", Message: "Agent memory at 94%", ResourceID: "agent:host-1"}, + }, + }) + if len(payload.Alerts) != 3 { + t.Fatalf("alerts = %+v", payload.Alerts) + } + moved, recovered, successor := payload.Alerts[0], payload.Alerts[1], payload.Alerts[2] + if moved.Resolved || moved.Resolution != summary || !moved.SuccessorAlertListed { + t.Fatalf("moved alert must carry its resolution, point at the listed agent alert and not read as resolved, got %+v", moved) + } + if successor.Resolved || successor.Resolution != "" || successor.SuccessorAlertListed { + t.Fatalf("the agent's open alert is an ordinary open alert, got %+v", successor) + } + if !recovered.Resolved || recovered.Resolution != "" { + t.Fatalf("an ordinary recovery stays resolved, got %+v", recovered) + } + if !strings.Contains(reportNarratorSystemPrompt, `An alert with a "resolution" closed without recovering`) || + !strings.Contains(reportNarratorSystemPrompt, `"successor_alert_listed": true`) { + t.Fatal("system prompt must tell the model a resolution is not a recovery and how to count it once") + } +} diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 32bfb19a12..6284d3e0a6 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -5272,6 +5272,67 @@ func TestContract_ReportingRequestCarriesEntitledReportBranding(t *testing.T) { } } +// A node alert that moved to its linked Pulse agent closed without +// recovering. The report request must keep that resolution and each alert's +// resource, so the report neither calls the node healthy nor counts the +// condition twice beside the agent's own alert. +func TestContract_ReportingAlertsCarryHandoverResolution(t *testing.T) { + now := time.Now() + summary := "Alert moved to pve1 (Host Agent). This is not a recovery: check the agent for the current reading." + snapshot := reportingEnrichmentSnapshot{ + Nodes: []models.Node{{ID: "pve1-node", Name: "pve1"}}, + ActiveAlerts: []models.Alert{{ + ID: "agent:host-1-memory", + Type: "memory", + Level: "warning", + ResourceID: "agent:host-1", + Node: "pve1", + Message: "Agent memory at 94%", + StartTime: now.Add(-time.Minute), + }}, + RecentlyResolved: []models.ResolvedAlert{{ + Alert: models.Alert{ + ID: "pve1-node-memory", + Type: "memory", + Level: "warning", + ResourceID: "pve1-node", + Node: "pve1", + Message: "Node memory at 95%", + StartTime: now.Add(-time.Hour), + Resolution: &models.AlertResolution{ + Reason: "moved_to_agent", + SuccessorResourceID: "agent:host-1", + SuccessorName: "pve1 (Host Agent)", + Summary: summary, + }, + }, + ResolvedTime: now.Add(-2 * time.Minute), + }}, + } + req := reporting.MetricReportRequest{ResourceType: "node", ResourceID: "pve1-node"} + NewReportingHandlers(nil, nil).enrichNodeReport(&req, snapshot, now.Add(-24*time.Hour), now) + + if len(req.Alerts) != 2 { + t.Fatalf("expected the agent alert and the moved node alert, got %+v", req.Alerts) + } + active, moved := req.Alerts[0], req.Alerts[1] + if active.ResourceID != "agent:host-1" || active.ResolvedTime != nil || active.Resolution != nil { + t.Fatalf("active alert row = %+v", active) + } + if moved.ResourceID != "pve1-node" || moved.ResolvedTime == nil || moved.Recovered() { + t.Fatalf("moved alert must stay resolved-but-not-recovered, got %+v", moved) + } + want := reporting.AlertResolution{ + Reason: reporting.AlertResolutionMovedToAgent, + SuccessorResourceID: "agent:host-1", + SuccessorName: "pve1 (Host Agent)", + Summary: summary, + } + if moved.Resolution == nil || *moved.Resolution != want { + t.Fatalf("resolution = %+v, want %+v", moved.Resolution, want) + } +} + func TestContract_ReportBrandingSettingsRejectWorkspaceLogoPath(t *testing.T) { err := validateReportBrandingSettings(map[string]interface{}{ "displayName": "Client One", diff --git a/internal/api/metrics_reporting_handlers.go b/internal/api/metrics_reporting_handlers.go index ebf7c91682..ba2cbee35a 100644 --- a/internal/api/metrics_reporting_handlers.go +++ b/internal/api/metrics_reporting_handlers.go @@ -750,6 +750,37 @@ func reportSubjectIDMatches(req *reporting.MetricReportRequest, candidateID stri return req.MetricsResourceID != "" && candidateID == req.MetricsResourceID } +// reportActiveAlertInfo is the report row for an open alert. +func reportActiveAlertInfo(alert models.Alert) reporting.AlertInfo { + return reporting.AlertInfo{ + Type: alert.Type, + Level: alert.Level, + Message: alert.Message, + Value: alert.Value, + Threshold: alert.Threshold, + StartTime: alert.StartTime, + ResourceID: alert.ResourceID, + } +} + +// reportResolvedAlertInfo is the report row for a recently resolved alert. It +// keeps the alert's resolution, so a node alert that moved to its Pulse agent +// is never reported as a recovery. +func reportResolvedAlertInfo(resolved models.ResolvedAlert) reporting.AlertInfo { + info := reportActiveAlertInfo(resolved.Alert) + resolvedTime := resolved.ResolvedTime + info.ResolvedTime = &resolvedTime + if resolution := resolved.Resolution; resolution != nil { + info.Resolution = &reporting.AlertResolution{ + Reason: resolution.Reason, + SuccessorResourceID: resolution.SuccessorResourceID, + SuccessorName: resolution.SuccessorName, + Summary: resolution.Summary, + } + } + return info +} + // enrichNodeReport adds node-specific data to the report request func (h *ReportingHandlers) enrichNodeReport(req *reporting.MetricReportRequest, snapshot reportingEnrichmentSnapshot, start, end time.Time) { // Find the node @@ -791,29 +822,13 @@ func (h *ReportingHandlers) enrichNodeReport(req *reporting.MetricReportRequest, // Find alerts for this node for _, alert := range snapshot.ActiveAlerts { if reportSubjectIDMatches(req, alert.ResourceID) || alert.Node == node.Name { - req.Alerts = append(req.Alerts, reporting.AlertInfo{ - Type: alert.Type, - Level: alert.Level, - Message: alert.Message, - Value: alert.Value, - Threshold: alert.Threshold, - StartTime: alert.StartTime, - }) + req.Alerts = append(req.Alerts, reportActiveAlertInfo(alert)) } } for _, resolved := range snapshot.RecentlyResolved { if (reportSubjectIDMatches(req, resolved.ResourceID) || resolved.Node == node.Name) && resolved.ResolvedTime.After(start) && resolved.ResolvedTime.Before(end) { - resolvedTime := resolved.ResolvedTime - req.Alerts = append(req.Alerts, reporting.AlertInfo{ - Type: resolved.Type, - Level: resolved.Level, - Message: resolved.Message, - Value: resolved.Value, - Threshold: resolved.Threshold, - StartTime: resolved.StartTime, - ResolvedTime: &resolvedTime, - }) + req.Alerts = append(req.Alerts, reportResolvedAlertInfo(resolved)) } } @@ -922,29 +937,13 @@ func (h *ReportingHandlers) enrichVMReport(ctx context.Context, orgID string, re // Find alerts for this VM for _, alert := range snapshot.ActiveAlerts { if reportSubjectIDMatches(req, alert.ResourceID) { - req.Alerts = append(req.Alerts, reporting.AlertInfo{ - Type: alert.Type, - Level: alert.Level, - Message: alert.Message, - Value: alert.Value, - Threshold: alert.Threshold, - StartTime: alert.StartTime, - }) + req.Alerts = append(req.Alerts, reportActiveAlertInfo(alert)) } } for _, resolved := range snapshot.RecentlyResolved { if reportSubjectIDMatches(req, resolved.ResourceID) && resolved.ResolvedTime.After(start) && resolved.ResolvedTime.Before(end) { - resolvedTime := resolved.ResolvedTime - req.Alerts = append(req.Alerts, reporting.AlertInfo{ - Type: resolved.Type, - Level: resolved.Level, - Message: resolved.Message, - Value: resolved.Value, - Threshold: resolved.Threshold, - StartTime: resolved.StartTime, - ResolvedTime: &resolvedTime, - }) + req.Alerts = append(req.Alerts, reportResolvedAlertInfo(resolved)) } } @@ -1201,29 +1200,13 @@ func (h *ReportingHandlers) enrichContainerReport(ctx context.Context, orgID str // Find alerts for this container for _, alert := range snapshot.ActiveAlerts { if reportSubjectIDMatches(req, alert.ResourceID) { - req.Alerts = append(req.Alerts, reporting.AlertInfo{ - Type: alert.Type, - Level: alert.Level, - Message: alert.Message, - Value: alert.Value, - Threshold: alert.Threshold, - StartTime: alert.StartTime, - }) + req.Alerts = append(req.Alerts, reportActiveAlertInfo(alert)) } } for _, resolved := range snapshot.RecentlyResolved { if reportSubjectIDMatches(req, resolved.ResourceID) && resolved.ResolvedTime.After(start) && resolved.ResolvedTime.Before(end) { - resolvedTime := resolved.ResolvedTime - req.Alerts = append(req.Alerts, reporting.AlertInfo{ - Type: resolved.Type, - Level: resolved.Level, - Message: resolved.Message, - Value: resolved.Value, - Threshold: resolved.Threshold, - StartTime: resolved.StartTime, - ResolvedTime: &resolvedTime, - }) + req.Alerts = append(req.Alerts, reportResolvedAlertInfo(resolved)) } } diff --git a/pkg/reporting/alert_state.go b/pkg/reporting/alert_state.go new file mode 100644 index 0000000000..98cf4b1e8a --- /dev/null +++ b/pkg/reporting/alert_state.go @@ -0,0 +1,229 @@ +package reporting + +import ( + "fmt" + "strings" +) + +// AlertResolutionMovedToAgent is the reason code of an alert that closed +// because a linked Pulse agent took its metric over +// (alerts.AlertResolutionMovedToAgent). +const AlertResolutionMovedToAgent = "moved_to_agent" + +// Recovered reports whether the alert closed because its condition cleared. +// An open alert, or one that closed without recovering, is not recovered. +func (a AlertInfo) Recovered() bool { + return a.ResolvedTime != nil && a.Resolution == nil +} + +// ClosedWithoutRecovery reports whether the alert closed while its condition +// may still hold, such as a node alert handed to its Pulse agent. +func (a AlertInfo) ClosedWithoutRecovery() bool { + return a.ResolvedTime != nil && a.Resolution != nil +} + +// ResolutionOutcome is the short form of a close that was not a recovery, +// such as "moved to pve1 (Host Agent)". It is empty for an open alert and for +// an ordinary recovery. +func (a AlertInfo) ResolutionOutcome() string { + if !a.ClosedWithoutRecovery() { + return "" + } + if a.Resolution.Reason == AlertResolutionMovedToAgent { + return "moved to " + a.successorLabel() + } + return "closed without recovering" +} + +// ResolutionSummary is the alert engine's account of a close that was not a +// recovery, falling back to the short form when the engine supplied none. It +// is empty for an open alert and for an ordinary recovery. +func (a AlertInfo) ResolutionSummary() string { + if !a.ClosedWithoutRecovery() { + return "" + } + if summary := strings.TrimSpace(a.Resolution.Summary); summary != "" { + return summary + } + return fmt.Sprintf("Alert %s. This is not a recovery.", a.ResolutionOutcome()) +} + +func (a AlertInfo) successorLabel() string { + if a.Resolution != nil { + if name := strings.TrimSpace(a.Resolution.SuccessorName); name != "" { + return name + } + } + return "its Pulse agent" +} + +// unresolvedAlerts returns the alerts a report must treat as live: every open +// alert, and every alert that closed without recovering unless the report +// also shows its successor's own reading for that metric. That alert carries +// the condition's current state, so one condition is never counted twice and +// a handover never reads as a recovery. +func unresolvedAlerts(alerts []AlertInfo) []AlertInfo { + return unresolvedAlertsAmong(alerts, alerts) +} + +// unresolvedAlertsAmong is unresolvedAlerts with successors looked up in +// pool, so a fleet report matches a handover against every resource's alerts. +func unresolvedAlertsAmong(alerts, pool []AlertInfo) []AlertInfo { + out := make([]AlertInfo, 0, len(alerts)) + for _, alert := range alerts { + switch { + case alert.ResolvedTime == nil: + out = append(out, alert) + case alert.ClosedWithoutRecovery() && successorAlertFor(alert, pool) == nil: + out = append(out, alert) + } + } + return out +} + +// successorAlertFor returns the alert in pool that carries a moved alert's +// condition: one of the same type on its successor or one of the successor's +// children (an agent's filesystem alerts sit under "/disk:"), +// still open or recovered at or after the handover. A successor recovery from +// before the handover says nothing about the reading since, so it does not +// count. +func successorAlertFor(moved AlertInfo, pool []AlertInfo) *AlertInfo { + if !moved.ClosedWithoutRecovery() { + return nil + } + successor := strings.TrimSpace(moved.Resolution.SuccessorResourceID) + if successor == "" { + return nil + } + for i := range pool { + candidate := &pool[i] + if candidate.Type != moved.Type || candidate.ClosedWithoutRecovery() { + continue + } + if candidate.ResolvedTime != nil && candidate.ResolvedTime.Before(*moved.ResolvedTime) { + continue + } + resourceID := strings.TrimSpace(candidate.ResourceID) + if resourceID == successor || strings.HasPrefix(resourceID, successor+"/") { + return candidate + } + } + return nil +} + +// SuccessorAlertListed reports whether alerts shows the reading of the +// resource this alert moved to: an alert of the same type on the successor, +// open or recovered since the handover. The report then counts the condition +// once, through that alert. It is false for an open alert or a recovery. +func (a AlertInfo) SuccessorAlertListed(alerts []AlertInfo) bool { + return successorAlertFor(a, alerts) != nil +} + +// fleetAlerts is every alert listed across a fleet report's resources. +func (d *MultiReportData) fleetAlerts() []AlertInfo { + if d == nil { + return nil + } + var pool []AlertInfo + for _, rd := range d.Resources { + if rd != nil { + pool = append(pool, rd.Alerts...) + } + } + return pool +} + +// alertHealth is the health verdict a report's alerts support. The PDF health +// card and the heuristic narrator both read it, so they never disagree. +type alertHealth struct { + Status string // HEALTHY, WARNING or CRITICAL + Message string + Critical int + Warning int + // Unresolved is every alert the report treats as live (unresolvedAlerts). + Unresolved []AlertInfo +} + +func assessAlertHealth(alerts []AlertInfo) alertHealth { + health := alertHealth{Unresolved: unresolvedAlerts(alerts)} + open := 0 + var moved []AlertInfo + for _, alert := range health.Unresolved { + if alert.Level == "critical" { + health.Critical++ + } else { + health.Warning++ + } + if alert.ClosedWithoutRecovery() { + moved = append(moved, alert) + } else { + open++ + } + } + + switch { + case health.Critical > 0: + health.Status = "CRITICAL" + case health.Warning > 0: + health.Status = "WARNING" + default: + health.Status = "HEALTHY" + } + + switch { + case open == 0 && len(moved) > 0: + health.Message = movedAlertsHealthMessage(moved) + case health.Critical == 1: + health.Message = "1 critical issue requires immediate attention" + case health.Critical > 1: + health.Message = fmt.Sprintf("%d critical issues require immediate attention", health.Critical) + case health.Warning == 1: + health.Message = "1 warning detected - review recommended" + case health.Warning > 1: + health.Message = fmt.Sprintf("%d warnings detected - review recommended", health.Warning) + default: + health.Message = "All systems operating normally" + } + return health +} + +// movedAlertsHealthMessage says where alerts that closed without recovering +// went, so the report points at the resource that now holds the reading +// instead of calling this one healthy. +func movedAlertsHealthMessage(moved []AlertInfo) string { + first := moved[0] + for _, alert := range moved[1:] { + if alert.ResolutionOutcome() != first.ResolutionOutcome() { + return fmt.Sprintf("%d alerts closed without recovering - review recommended", len(moved)) + } + } + if first.Resolution.Reason != AlertResolutionMovedToAgent { + if len(moved) == 1 { + return "1 alert closed without recovering - review recommended" + } + return fmt.Sprintf("%d alerts closed without recovering - review recommended", len(moved)) + } + if len(moved) == 1 { + return fmt.Sprintf("Alert %s - check the agent for the current reading", first.ResolutionOutcome()) + } + return fmt.Sprintf("%d alerts %s - check the agent for the current reading", len(moved), first.ResolutionOutcome()) +} + +// movedAlertSuccessors names, once each and in report order, the Pulse agents +// that unresolved alerts moved to. +func movedAlertSuccessors(unresolved []AlertInfo) []string { + var successors []string + seen := make(map[string]struct{}) + for _, alert := range unresolved { + if !alert.ClosedWithoutRecovery() || alert.Resolution.Reason != AlertResolutionMovedToAgent { + continue + } + successor := alert.successorLabel() + if _, ok := seen[successor]; ok { + continue + } + seen[successor] = struct{}{} + successors = append(successors, successor) + } + return successors +} diff --git a/pkg/reporting/alert_state_test.go b/pkg/reporting/alert_state_test.go new file mode 100644 index 0000000000..175f019b78 --- /dev/null +++ b/pkg/reporting/alert_state_test.go @@ -0,0 +1,273 @@ +package reporting + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/go-pdf/fpdf" +) + +const movedAlertSummary = "Alert moved to pve1 (Host Agent). This is not a recovery: check the agent for the current reading." + +// movedNodeAlert is a node memory alert that closed because the linked Pulse +// agent took the metric over, as GetRecentlyResolved reports it. +func movedNodeAlert(level string) AlertInfo { + resolved := time.Now().Add(-2 * time.Minute) + return AlertInfo{ + Type: "memory", + Level: level, + Message: "Node memory at 95%", + StartTime: time.Now().Add(-time.Hour), + ResolvedTime: &resolved, + ResourceID: "pve1-node", + Resolution: &AlertResolution{ + Reason: AlertResolutionMovedToAgent, + SuccessorResourceID: "agent:host-1", + SuccessorName: "pve1 (Host Agent)", + Summary: movedAlertSummary, + }, + } +} + +// A node alert that moved to its Pulse agent within the recently-resolved +// window must not turn the report HEALTHY: nothing saw the condition clear. +func TestHeuristicNarrator_MovedAlertIsNotARecovery(t *testing.T) { + out, err := HeuristicNarrator{}.Narrate(context.Background(), NarrativeInput{ + MetricStats: map[string]MetricStats{"memory": {Avg: 60, Max: 96}}, + Alerts: []AlertInfo{movedNodeAlert("warning")}, + }) + if err != nil { + t.Fatalf("Narrate: %v", err) + } + if out.HealthStatus != "WARNING" { + t.Fatalf("HealthStatus = %q, want WARNING", out.HealthStatus) + } + if want := "Alert moved to pve1 (Host Agent) - check the agent for the current reading"; out.HealthMessage != want { + t.Fatalf("HealthMessage = %q, want %q", out.HealthMessage, want) + } + + var movedBullet *NarrativeBullet + for i, bullet := range out.Observations { + if strings.Contains(bullet.Text, "triggered and resolved") { + t.Fatalf("moved alert counted as triggered and resolved: %+v", out.Observations) + } + if bullet.Text == movedAlertSummary { + movedBullet = &out.Observations[i] + } + } + if movedBullet == nil || movedBullet.Severity != NarrativeSeverityInfo { + t.Fatalf("expected an observation recording the handover, got %+v", out.Observations) + } + + recs := strings.Join(out.Recommendations, "\n") + if !strings.Contains(recs, "Check the current reading on pve1 (Host Agent)") { + t.Fatalf("expected a recommendation to check the agent, got %q", recs) + } + if strings.Contains(recs, "No immediate action required") { + t.Fatalf("a moved alert is not a reason to stand down, got %q", recs) + } + + critical := assessAlertHealth([]AlertInfo{movedNodeAlert("critical")}) + if critical.Status != "CRITICAL" || critical.Critical != 1 { + t.Fatalf("a moved critical alert keeps its level, got %+v", critical) + } + + // An ordinary recovery is still a recovery. + recovered := movedNodeAlert("warning") + recovered.Resolution = nil + plain, _ := HeuristicNarrator{}.Narrate(context.Background(), NarrativeInput{ + MetricStats: map[string]MetricStats{"memory": {Avg: 60, Max: 70}}, + Alerts: []AlertInfo{recovered}, + }) + if plain.HealthStatus != "HEALTHY" || plain.HealthMessage != "All systems operating normally" { + t.Fatalf("recovered alert should leave the report healthy, got %q / %q", plain.HealthStatus, plain.HealthMessage) + } +} + +// When the report also lists the alert the successor raised for the same +// metric, that alert carries the condition: the moved alert must not count +// it a second time. +func TestUnresolvedAlerts_SuccessorAlertCarriesTheCondition(t *testing.T) { + agentMemory := AlertInfo{Type: "memory", Level: "warning", Message: "Agent memory at 94%", ResourceID: "agent:host-1"} + health := assessAlertHealth([]AlertInfo{movedNodeAlert("warning"), agentMemory}) + if health.Warning != 1 || len(health.Unresolved) != 1 || health.Unresolved[0] != agentMemory { + t.Fatalf("expected only the agent's own alert to count, got %+v", health) + } + if health.Message != "1 warning detected - review recommended" { + t.Fatalf("HealthMessage = %q", health.Message) + } + out, _ := HeuristicNarrator{}.Narrate(context.Background(), NarrativeInput{ + MetricStats: map[string]MetricStats{"memory": {Avg: 60, Max: 96}}, + Alerts: []AlertInfo{movedNodeAlert("warning"), agentMemory}, + }) + if strings.Contains(strings.Join(out.Recommendations, "\n"), "Check the current reading") { + t.Fatalf("covered handover should not ask to check the agent: %+v", out.Recommendations) + } + + // The agent's filesystem alerts sit under the agent's resource ID. + movedDisk := movedNodeAlert("warning") + movedDisk.Type = "disk" + agentRoot := AlertInfo{Type: "disk", Level: "warning", ResourceID: "agent:host-1/disk:root"} + if got := unresolvedAlerts([]AlertInfo{movedDisk, agentRoot}); len(got) != 1 || got[0] != agentRoot { + t.Fatalf("expected the agent filesystem alert to cover the moved disk alert, got %+v", got) + } + + // A different metric, or an agent whose ID merely shares a prefix, does + // not carry the moved alert's condition. + agentCPU := AlertInfo{Type: "cpu", Level: "warning", ResourceID: "agent:host-1"} + otherAgent := AlertInfo{Type: "memory", Level: "warning", ResourceID: "agent:host-10"} + if got := unresolvedAlerts([]AlertInfo{movedNodeAlert("warning"), agentCPU, otherAgent}); len(got) != 3 { + t.Fatalf("expected the moved alert to stay unresolved beside unrelated alerts, got %+v", got) + } +} + +func TestExecutiveSummary_MovedAlertIsNotHealthy(t *testing.T) { + data := &ReportData{ + Title: "pve1", + ResourceType: "node", + ResourceID: "pve1-node", + Start: time.Now().Add(-time.Hour), + End: time.Now(), + GeneratedAt: time.Now(), + Summary: MetricSummary{ByMetric: map[string]MetricStats{ + "cpu": {Avg: 5, Max: 12, Count: 60}, + "memory": {Avg: 60, Max: 96, Count: 60}, + }}, + TotalPoints: 120, + Alerts: []AlertInfo{movedNodeAlert("warning")}, + } + // fpdf escapes parentheses inside PDF string literals. + text := strings.NewReplacer(`\(`, "(", `\)`, ")").Replace(renderExecutiveSummaryText(t, data)) + for _, want := range []string{ + "WARNING", + "Alert moved to pve1 (Host Agent) - check the agent for the current reading", + "Node memory at 95% (moved to pve1 (Host Agent))", + "Moved", + "Moved: " + movedAlertSummary, + } { + if !strings.Contains(text, want) { + t.Errorf("expected %q in executive summary, got:\n%s", want, text) + } + } + if strings.Contains(text, "HEALTHY") || strings.Contains(text, "All systems operating normally") { + t.Errorf("a handover must not render the healthy card, got:\n%s", text) + } + + // The health card does not wrap, so the handover message must fit it. + pdf := fpdf.New("P", "mm", "A4", "") + pdf.SetFont("Arial", "", 11) + pageWidth, _ := pdf.GetPageSize() + if width := pdf.GetStringWidth(assessAlertHealth(data.Alerts).Message); width > pageWidth-40 { + t.Fatalf("handover health message is %.1fmm wide, card is %.1fmm", width, pageWidth-40) + } +} + +// A successor's reading only counts from the handover on: an agent recovery +// from before the node alert moved says nothing about the condition since. +func TestUnresolvedAlerts_SuccessorReadingMustFollowTheHandover(t *testing.T) { + moved := movedNodeAlert("warning") + before := moved.ResolvedTime.Add(-time.Minute) + after := moved.ResolvedTime.Add(time.Minute) + agentRecoveredBefore := AlertInfo{Type: "memory", Level: "warning", ResourceID: "agent:host-1", ResolvedTime: &before} + agentRecoveredAfter := AlertInfo{Type: "memory", Level: "warning", ResourceID: "agent:host-1", ResolvedTime: &after} + + stale := assessAlertHealth([]AlertInfo{agentRecoveredBefore, moved}) + if stale.Status != "WARNING" || len(stale.Unresolved) != 1 || stale.Unresolved[0] != moved { + t.Fatalf("an earlier agent recovery must not cover a later handover, got %+v", stale) + } + if moved.SuccessorAlertListed([]AlertInfo{agentRecoveredBefore, moved}) { + t.Fatal("SuccessorAlertListed must ignore a recovery from before the handover") + } + + cleared := assessAlertHealth([]AlertInfo{agentRecoveredAfter, moved}) + if cleared.Status != "HEALTHY" || len(cleared.Unresolved) != 0 { + t.Fatalf("an agent recovery after the handover shows the condition cleared, got %+v", cleared) + } + if !moved.SuccessorAlertListed([]AlertInfo{agentRecoveredAfter, moved}) { + t.Fatal("SuccessorAlertListed must accept a recovery after the handover") + } +} + +func TestBuildFleetNarrativeInput_MovedAlertStaysActive(t *testing.T) { + now := time.Now() + movedElsewhere := movedNodeAlert("critical") + movedElsewhere.Resolution = &AlertResolution{ + Reason: AlertResolutionMovedToAgent, + SuccessorResourceID: "agent:host-3", + SuccessorName: "pve3 (Host Agent)", + Summary: "Alert moved to pve3 (Host Agent). This is not a recovery: check the agent for the current reading.", + } + multi := &MultiReportData{ + Title: "Fleet", + Start: now.Add(-time.Hour), + End: now, + Resources: []*ReportData{ + { + ResourceID: "pve1-node", + ResourceType: "node", + Resource: &ResourceInfo{Name: "pve1", Status: "online"}, + Alerts: []AlertInfo{movedNodeAlert("warning")}, + }, + { + // The agent pve1's alert moved to is its own fleet entry. + ResourceID: "agent:host-1", + ResourceType: "agent", + Resource: &ResourceInfo{Name: "pve1 (Host Agent)", Status: "online"}, + Alerts: []AlertInfo{{Type: "memory", Level: "warning", ResourceID: "agent:host-1"}}, + }, + { + ResourceID: "pve3-node", + ResourceType: "node", + Resource: &ResourceInfo{Name: "pve3", Status: "online"}, + Alerts: []AlertInfo{movedElsewhere}, + }, + }, + } + in := buildFleetNarrativeInput(multi) + if in.Aggregate.TotalResolvedAlerts != 0 { + t.Fatalf("moved alerts are not resolved, got %d", in.Aggregate.TotalResolvedAlerts) + } + // pve1's handover is carried by the agent's own alert in another fleet + // entry; pve3's has no successor reading, so it stays live at its level. + if in.Aggregate.TotalActiveAlerts != 2 || in.Aggregate.TotalCriticalAlerts != 1 { + t.Fatalf("expected the agent alert and pve3's moved alert active, got active=%d critical=%d", + in.Aggregate.TotalActiveAlerts, in.Aggregate.TotalCriticalAlerts) + } + if got := in.Resources[0]; got.ActiveAlerts != 0 || len(got.MovedTo) != 0 { + t.Fatalf("pve1's covered handover must not count again, got %+v", got) + } + if got := in.Resources[2]; got.ActiveAlerts != 1 || len(got.MovedTo) != 1 || got.MovedTo[0] != "pve3 (Host Agent)" { + t.Fatalf("pve3 should keep its moved alert and name the agent, got %+v", got) + } + + out, _ := HeuristicFleetNarrator{}.NarrateFleet(context.Background(), in) + if out.HealthStatus != "CRITICAL" { + t.Fatalf("fleet HealthStatus = %q, want CRITICAL", out.HealthStatus) + } + recs := strings.Join(out.Recommendations, "\n") + if !strings.Contains(recs, "Check the current reading on pve3 (Host Agent)") { + t.Fatalf("expected a recommendation naming pve3's agent, got %q", recs) + } + if strings.Contains(recs, "pve1 (Host Agent)") || strings.Contains(recs, "No fleet-wide action required") { + t.Fatalf("unexpected recommendation, got %q", recs) + } + + // A covered handover leaves only the agent's open warning: still no + // all-clear in the patterns or the recommendations. + multi.Resources = multi.Resources[:2] + covered, _ := HeuristicFleetNarrator{}.NarrateFleet(context.Background(), buildFleetNarrativeInput(multi)) + if covered.HealthStatus != "WARNING" { + t.Fatalf("fleet HealthStatus = %q, want WARNING", covered.HealthStatus) + } + for _, pattern := range covered.Patterns { + if strings.Contains(pattern.Text, "nominal thresholds") { + t.Fatalf("an open alert rules out the all-clear pattern, got %+v", covered.Patterns) + } + } + if recs := strings.Join(covered.Recommendations, "\n"); strings.Contains(recs, "No fleet-wide action required") || + !strings.Contains(recs, "Review the active alerts") { + t.Fatalf("an open alert needs a review recommendation, got %q", recs) + } +} diff --git a/pkg/reporting/fleet_narrative.go b/pkg/reporting/fleet_narrative.go index 478eeb02b0..8adef1b4c5 100644 --- a/pkg/reporting/fleet_narrative.go +++ b/pkg/reporting/fleet_narrative.go @@ -38,6 +38,9 @@ type FleetResourceSummary struct { UnhealthyDisks int StoragePoolsHigh int Findings int + // MovedTo names the Pulse agents this resource's unresolved alerts + // moved to without recovering; those alerts also count as active. + MovedTo []string } // FleetAggregate is the fleet-wide rollup. Means are means-of-means: @@ -313,7 +316,9 @@ func fleetHeuristicPatterns(in FleetNarrativeInput) []NarrativeBullet { Severity: NarrativeSeverityInfo, }) } - if len(out) == 0 { + // Open alerts, including ones that moved to an agent without + // recovering, rule out the all-clear even when no metric pattern fired. + if len(out) == 0 && in.Aggregate.TotalActiveAlerts == 0 { out = append(out, NarrativeBullet{ Text: fmt.Sprintf("Fleet of %d resources operating within nominal thresholds", in.Aggregate.ResourceCount), Severity: NarrativeSeverityOK, @@ -373,6 +378,19 @@ func fleetHeuristicRecommendations(in FleetNarrativeInput) []string { if disksHigh > 0 { recs = append(recs, "Replace failing or end-of-life disks before they cause outage") } + seenSuccessors := make(map[string]struct{}) + for _, r := range in.Resources { + for _, successor := range r.MovedTo { + if _, ok := seenSuccessors[successor]; ok { + continue + } + seenSuccessors[successor] = struct{}{} + recs = append(recs, fmt.Sprintf("Check the current reading on %s - an alert moved there without recovering", successor)) + } + } + if len(recs) == 0 && a.TotalActiveAlerts > 0 { + recs = append(recs, "Review the active alerts on the resources listed above") + } if len(recs) == 0 { recs = append(recs, "No fleet-wide action required - continue routine monitoring") } @@ -402,6 +420,7 @@ func buildFleetNarrativeInput(data *MultiReportData) FleetNarrativeInput { var sumCPU, sumMem, sumDisk float64 var countCPU, countMem, countDisk int + fleetAlerts := data.fleetAlerts() for _, rd := range data.Resources { if rd == nil { @@ -450,11 +469,16 @@ func buildFleetNarrativeInput(data *MultiReportData) FleetNarrativeInput { } for _, alert := range rd.Alerts { - if alert.ResolvedTime != nil { + if alert.Recovered() { summary.ResolvedAlerts++ in.Aggregate.TotalResolvedAlerts++ - continue } + } + // An alert that moved without recovering stays active until the + // fleet report lists its successor's own alert. + unresolved := unresolvedAlertsAmong(rd.Alerts, fleetAlerts) + summary.MovedTo = movedAlertSuccessors(unresolved) + for _, alert := range unresolved { summary.ActiveAlerts++ in.Aggregate.TotalActiveAlerts++ if alert.Level == "critical" { diff --git a/pkg/reporting/narrative.go b/pkg/reporting/narrative.go index 370f13085b..5473e425cd 100644 --- a/pkg/reporting/narrative.go +++ b/pkg/reporting/narrative.go @@ -145,53 +145,11 @@ func (HeuristicNarrator) Narrate(_ context.Context, in NarrativeInput) (Narrativ } func heuristicHealthStatus(in NarrativeInput) string { - criticalAlerts := 0 - warningAlerts := 0 - for _, alert := range in.Alerts { - if alert.ResolvedTime != nil { - continue - } - if alert.Level == "critical" { - criticalAlerts++ - } else { - warningAlerts++ - } - } - switch { - case criticalAlerts > 0: - return "CRITICAL" - case warningAlerts > 0: - return "WARNING" - default: - return "HEALTHY" - } + return assessAlertHealth(in.Alerts).Status } func heuristicHealthMessage(in NarrativeInput) string { - criticalAlerts := 0 - warningAlerts := 0 - for _, alert := range in.Alerts { - if alert.ResolvedTime != nil { - continue - } - if alert.Level == "critical" { - criticalAlerts++ - } else { - warningAlerts++ - } - } - switch { - case criticalAlerts == 1: - return "1 critical issue requires immediate attention" - case criticalAlerts > 1: - return fmt.Sprintf("%d critical issues require immediate attention", criticalAlerts) - case warningAlerts == 1: - return "1 warning detected - review recommended" - case warningAlerts > 1: - return fmt.Sprintf("%d warnings detected - review recommended", warningAlerts) - default: - return "All systems operating normally" - } + return assessAlertHealth(in.Alerts).Message } // heuristicObservations mirrors the original generateObservations rules. @@ -273,7 +231,7 @@ func heuristicObservations(in NarrativeInput) []NarrativeBullet { resolved := 0 for _, alert := range in.Alerts { - if alert.ResolvedTime != nil { + if alert.Recovered() { resolved++ } } @@ -283,6 +241,17 @@ func heuristicObservations(in NarrativeInput) []NarrativeBullet { Severity: NarrativeSeverityInfo, }) } + // A close that was not a recovery is recorded as what happened. The + // health verdict, the active list and the recommendations carry the + // warning while the successor's reading is not in this report. + for _, alert := range in.Alerts { + if alert.ClosedWithoutRecovery() { + obs = append(obs, NarrativeBullet{ + Text: alert.ResolutionSummary(), + Severity: NarrativeSeverityInfo, + }) + } + } for _, disk := range in.Disks { if disk.WearLevel > 0 && disk.WearLevel <= 10 { @@ -314,19 +283,8 @@ func heuristicObservations(in NarrativeInput) []NarrativeBullet { // heuristicRecommendations mirrors the original generateRecommendations rules. func heuristicRecommendations(in NarrativeInput) []string { - criticalAlerts := 0 - warningAlerts := 0 - for _, alert := range in.Alerts { - if alert.ResolvedTime != nil { - continue - } - if alert.Level == "critical" { - criticalAlerts++ - } else { - warningAlerts++ - } - } - _ = warningAlerts // retained for future shaping; matches original signature + health := assessAlertHealth(in.Alerts) + criticalAlerts := health.Critical var recs []string @@ -344,6 +302,9 @@ func heuristicRecommendations(in NarrativeInput) []string { if criticalAlerts > 0 { recs = append(recs, "Investigate and resolve critical alerts immediately") } + for _, successor := range movedAlertSuccessors(health.Unresolved) { + recs = append(recs, fmt.Sprintf("Check the current reading on %s - an alert moved there without recovering", successor)) + } if stats, ok := in.MetricStats["memory"]; ok { if stats.Avg > 85 { diff --git a/pkg/reporting/pdf.go b/pkg/reporting/pdf.go index b1db00fdc1..00b8cb51a7 100644 --- a/pkg/reporting/pdf.go +++ b/pkg/reporting/pdf.go @@ -464,41 +464,23 @@ func (g *PDFGenerator) writeCoverPage(pdf *fpdf.Fpdf, data *ReportData) { func (g *PDFGenerator) writeExecutiveSummary(pdf *fpdf.Fpdf, data *ReportData) { pageWidth, _ := pdf.GetPageSize() - // Determine overall health status - healthStatus := "HEALTHY" + // Determine overall health status. An alert that closed without + // recovering (moved to its Pulse agent) still counts until the report + // lists the successor's own alert for that metric. + health := assessAlertHealth(data.Alerts) + healthStatus := health.Status healthColor := colorAccent // Green - healthMessage := "All systems operating normally" + healthMessage := health.Message - activeAlerts := 0 - criticalAlerts := 0 - warningAlerts := 0 - for _, alert := range data.Alerts { - if alert.ResolvedTime == nil { - activeAlerts++ - if alert.Level == "critical" { - criticalAlerts++ - } else { - warningAlerts++ - } - } - } + unresolved := health.Unresolved + activeAlerts := len(unresolved) + criticalAlerts := health.Critical + warningAlerts := health.Warning if criticalAlerts > 0 { - healthStatus = "CRITICAL" healthColor = colorDanger - if criticalAlerts == 1 { - healthMessage = "1 critical issue requires immediate attention" - } else { - healthMessage = fmt.Sprintf("%d critical issues require immediate attention", criticalAlerts) - } } else if warningAlerts > 0 { - healthStatus = "WARNING" healthColor = colorWarning - if warningAlerts == 1 { - healthMessage = "1 warning detected - review recommended" - } else { - healthMessage = fmt.Sprintf("%d warnings detected - review recommended", warningAlerts) - } } else if data.TotalPoints == 0 && len(data.Summary.ByMetric) == 0 { // No metrics arrived for the requested window. Reporting // HEALTHY would green-light an empty report and mislead the @@ -633,8 +615,8 @@ func (g *PDFGenerator) writeExecutiveSummary(pdf *fpdf.Fpdf, data *ReportData) { pdf.SetFont("Arial", "", 9) alertCount := 0 - for _, alert := range data.Alerts { - if alert.ResolvedTime == nil && alertCount < 5 { + for _, alert := range unresolved { + if alertCount < 5 { if alert.Level == "critical" { pdf.SetTextColor(colorDanger[0], colorDanger[1], colorDanger[2]) pdf.CellFormat(8, 5, "!", "", 0, "C", false, 0, "") @@ -643,11 +625,7 @@ func (g *PDFGenerator) writeExecutiveSummary(pdf *fpdf.Fpdf, data *ReportData) { pdf.CellFormat(8, 5, "!", "", 0, "C", false, 0, "") } pdf.SetTextColor(colorTextDark[0], colorTextDark[1], colorTextDark[2]) - msg := alert.Message - if len(msg) > 70 { - msg = msg[:67] + "..." - } - pdf.CellFormat(0, 5, msg, "", 1, "L", false, 0, "") + pdf.CellFormat(0, 5, unresolvedAlertLine(alert, 70), "", 1, "L", false, 0, "") alertCount++ } } @@ -1509,8 +1487,12 @@ func (g *PDFGenerator) writeAlertsSection(pdf *fpdf.Fpdf, data *ReportData) { // Started pdf.CellFormat(colWidths[3], 6, alert.StartTime.Format("Jan 02 15:04"), "1", 0, "C", fill, 0, "") - // Resolved - if alert.ResolvedTime != nil { + // Resolved. A close that was not a recovery reads "Moved" in + // neutral text, never as a green resolve time. + if alert.ClosedWithoutRecovery() { + pdf.SetTextColor(colorTextMuted[0], colorTextMuted[1], colorTextMuted[2]) + pdf.CellFormat(colWidths[4], 6, closedWithoutRecoveryLabel(alert), "1", 0, "C", fill, 0, "") + } else if alert.ResolvedTime != nil { pdf.SetTextColor(colorAccent[0], colorAccent[1], colorAccent[2]) pdf.CellFormat(colWidths[4], 6, alert.ResolvedTime.Format("Jan 02 15:04"), "1", 0, "C", fill, 0, "") } else { @@ -1523,9 +1505,62 @@ func (g *PDFGenerator) writeAlertsSection(pdf *fpdf.Fpdf, data *ReportData) { fill = !fill } + // Say once what each "Moved" row means, in the alert engine's words. + var notes []string + seenNotes := make(map[string]struct{}) + for _, alert := range data.Alerts { + summary := alert.ResolutionSummary() + if summary == "" { + continue + } + note := closedWithoutRecoveryLabel(alert) + ": " + summary + if _, ok := seenNotes[note]; ok { + continue + } + seenNotes[note] = struct{}{} + notes = append(notes, note) + } + if len(notes) > 0 { + pdf.Ln(2) + pdf.SetFont("Arial", "I", 8) + pdf.SetTextColor(colorTextMuted[0], colorTextMuted[1], colorTextMuted[2]) + for _, note := range notes { + pdf.MultiCell(0, 4, note, "", "L", false) + } + pdf.SetTextColor(colorTextDark[0], colorTextDark[1], colorTextDark[2]) + } + pdf.Ln(10) } +// closedWithoutRecoveryLabel is the alerts table's Resolved cell for a close +// that was not a recovery. +func closedWithoutRecoveryLabel(alert AlertInfo) string { + if alert.Resolution != nil && alert.Resolution.Reason == AlertResolutionMovedToAgent { + return "Moved" + } + return "Closed" +} + +// unresolvedAlertLine is one alert in an "Active Alerts" list, cut to max +// characters. An alert that closed without recovering keeps its outcome, +// such as "(moved to pve1 (Host Agent))", and gives up message text instead. +func unresolvedAlertLine(alert AlertInfo, max int) string { + suffix := "" + if outcome := alert.ResolutionOutcome(); outcome != "" { + suffix = " (" + outcome + ")" + } + room := max - len(suffix) + if room < 10 { + room = 10 + } + msg := alert.Message + if len(msg) > room { + msg = msg[:room-3] + "..." + } + return msg + suffix +} + // writeStorageSection writes storage pools table func (g *PDFGenerator) writeStorageSection(pdf *fpdf.Fpdf, data *ReportData) { if len(data.Storage) == 0 { @@ -1831,8 +1866,10 @@ func (g *PDFGenerator) GenerateMulti(data *MultiReportData) ([]byte, error) { _, pageHeight := pdf.GetPageSize() bottomLimit := pageHeight - 30 pageOpen := false + fleetAlerts := data.fleetAlerts() for _, rd := range data.Resources { - startNewPage := !pageOpen || pdf.GetY()+condensedResourceBlockHeight(rd) > bottomLimit + unresolved := unresolvedAlertsAmong(rd.Alerts, fleetAlerts) + startNewPage := !pageOpen || pdf.GetY()+condensedResourceBlockHeight(rd, unresolved) > bottomLimit if startNewPage { pdf.AddPage() g.addMultiPageHeader(pdf, data, "Resource Details") @@ -1843,7 +1880,7 @@ func (g *PDFGenerator) GenerateMulti(data *MultiReportData) ([]byte, error) { pdf.Line(20, pdf.GetY(), 190, pdf.GetY()) pdf.Ln(5) } - g.writeCondensedResourcePage(pdf, rd) + g.writeCondensedResourcePage(pdf, rd, unresolved) } // Add page numbers to all pages except cover @@ -2043,15 +2080,14 @@ func (g *PDFGenerator) writeFleetSummary(pdf *fpdf.Fpdf, data *MultiReportData) totalActive := 0 totalCritical := 0 totalWarning := 0 + fleetAlerts := data.fleetAlerts() for _, rd := range data.Resources { - for _, alert := range rd.Alerts { - if alert.ResolvedTime == nil { - totalActive++ - if alert.Level == "critical" { - totalCritical++ - } else { - totalWarning++ - } + for _, alert := range unresolvedAlertsAmong(rd.Alerts, fleetAlerts) { + totalActive++ + if alert.Level == "critical" { + totalCritical++ + } else { + totalWarning++ } } } @@ -2215,12 +2251,7 @@ func (g *PDFGenerator) writeFleetSummary(pdf *fpdf.Fpdf, data *MultiReportData) pdf.CellFormat(colWidths[6], 6, fmt.Sprintf("%.1f%%", avgDisk), "1", 0, "C", fill, 0, "") // Alerts count - alertCount := 0 - for _, alert := range rd.Alerts { - if alert.ResolvedTime == nil { - alertCount++ - } - } + alertCount := len(unresolvedAlertsAmong(rd.Alerts, fleetAlerts)) pdf.SetTextColor(getAlertCountColor(alertCount)[0], getAlertCountColor(alertCount)[1], getAlertCountColor(alertCount)[2]) pdf.CellFormat(colWidths[7], 6, fmt.Sprintf("%d", alertCount), "1", 0, "C", fill, 0, "") @@ -2395,17 +2426,12 @@ func writeFleetNarrativeSection(pdf *fpdf.Fpdf, fn *FleetNarrative) { // resource block needs so the fleet layout can decide whether it fits on // the current page. Estimates err slightly high; a block that still // overflows is carried across pages by fpdf's auto page break. -func condensedResourceBlockHeight(rd *ReportData) float64 { +func condensedResourceBlockHeight(rd *ReportData, unresolved []AlertInfo) float64 { height := 11.0 + 6.0 + 27.0 + 8.0 // name header + availability line + stats bar + padding if len(rd.Metrics["cpu"]) >= 2 || len(rd.Metrics["memory"]) >= 2 { height += 58 // chart title + canvas + legend } - active := 0 - for _, alert := range rd.Alerts { - if alert.ResolvedTime == nil { - active++ - } - } + active := len(unresolved) if active > 0 { shown := active if shown > 3 { @@ -2422,7 +2448,9 @@ func condensedResourceBlockHeight(rd *ReportData) float64 { return height } -func (g *PDFGenerator) writeCondensedResourcePage(pdf *fpdf.Fpdf, rd *ReportData) { +// writeCondensedResourcePage writes one resource's block of a fleet report. +// unresolved is the resource's live alerts matched against the whole fleet. +func (g *PDFGenerator) writeCondensedResourcePage(pdf *fpdf.Fpdf, rd *ReportData, unresolved []AlertInfo) { // Resource header resourceName := rd.ResourceID if rd.Resource != nil && rd.Resource.Name != "" { @@ -2586,13 +2614,8 @@ func (g *PDFGenerator) writeCondensedResourcePage(pdf *fpdf.Fpdf, rd *ReportData } } - // Active alerts (up to 3) - activeAlerts := make([]AlertInfo, 0) - for _, alert := range rd.Alerts { - if alert.ResolvedTime == nil { - activeAlerts = append(activeAlerts, alert) - } - } + // Active alerts (up to 3), including any that moved without recovering + activeAlerts := unresolved if len(activeAlerts) > 0 { pdf.Ln(3) @@ -2615,11 +2638,7 @@ func (g *PDFGenerator) writeCondensedResourcePage(pdf *fpdf.Fpdf, rd *ReportData } pdf.CellFormat(6, 5, "!", "", 0, "C", false, 0, "") pdf.SetTextColor(colorTextDark[0], colorTextDark[1], colorTextDark[2]) - msg := alert.Message - if len(msg) > 80 { - msg = msg[:77] + "..." - } - pdf.CellFormat(0, 5, msg, "", 1, "L", false, 0, "") + pdf.CellFormat(0, 5, unresolvedAlertLine(alert, 80), "", 1, "L", false, 0, "") } if len(activeAlerts) > 3 { pdf.SetTextColor(colorTextMuted[0], colorTextMuted[1], colorTextMuted[2]) diff --git a/pkg/reporting/reporting.go b/pkg/reporting/reporting.go index cc424e6bc8..b7b83c4644 100644 --- a/pkg/reporting/reporting.go +++ b/pkg/reporting/reporting.go @@ -115,6 +115,28 @@ type AlertInfo struct { StartTime time.Time ResolvedTime *time.Time // nil if still active Acknowledged bool + // ResourceID is the resource the alert was raised on, so an alert that + // moved can find the alert its successor raised for the same metric. + ResourceID string + // Resolution is set only on a resolved alert whose close was not a + // recovery, such as a node alert handed to its Pulse agent. A report + // never counts such an alert as resolved. + Resolution *AlertResolution +} + +// AlertResolution says why an alert closed without its condition clearing. +// It mirrors models.AlertResolution so this package stays free of internal +// dependencies. +type AlertResolution struct { + // Reason is the alert engine's reason code, such as + // AlertResolutionMovedToAgent. + Reason string + SuccessorResourceID string + SuccessorName string + // Summary is the alert engine's one-line account, such as "Alert moved + // to pve1 (Host Agent). This is not a recovery: check the agent for the + // current reading." + Summary string } // BackupInfo contains backup information for VMs/containers