Adds table-driven branch-coverage unit tests for previously uncovered pure
functions across internal/alerts/config, internal/ai/modelresolution,
internal/monitoring, internal/ai, internal/ai/qualification and pkg/licensing.
New test files only, with no source changes.
Covers per-subsystem alert-default normalization, configured model and provider
resolution, fleet-doctor identity and cluster-endpoint helpers, docker-state and
infrastructure-key mapping, investigation and SMART issue helpers, JSON and
autonomy normalization, and licensing feature-tier resolution. 65 TestBranchCov
functions in 7 files, all vet and gofmt clean.
Adds table-driven branch-coverage unit tests for previously untested pure
helper functions across internal/alerts/config, internal/config,
internal/ai/safety, internal/ai/modelresolution, internal/operationreceipt,
internal/models, internal/securityutil and pkg/securityutil. New test files
only, with no source changes.
Covers alert-config normalization and validation, sensitive-path and
redaction classifiers, URL normalizers, provider model resolution,
operation-receipt decoding, credential masking, and account-to-org role
mapping. 57 TestBranchCov functions in 12 files, all vet and gofmt clean.
Guest alerts are keyed by node-scoped resource IDs, and only metric
threshold alerts were re-homed after a live migration. Lifecycle alerts
(powered-state and future guest kinds) stayed keyed to the old node
forever because their resolvers only look up the new-node key, node
existence GC never fires while the source node is still online, and
nothing else consumes the move.
Generalize the migration helper to match lifecycle spec IDs (which
embed the node-scoped resource ID) as well as node-independent metric
spec IDs, and call it from the canonical lifecycle and stateful
evaluators, the powered-off resolver, and the disabled-threshold early
returns in both metric paths. Guest-wide sweep clears (metric clear on
stop, per-disk cleanup, tag suppression) now match the stable
instance+vmid identity across nodes too.
Reported by Johannes Strasser: a VM live-migrated between Proxmox
nodes stranded its pre-existing active alert with no way to clear it.
DiskTempByType unconditionally replaced the resolved host threshold, so a
host-level (or inherited linked-resource) diskTemperature override never
applied to nvme/sas/sata disks. Explicit overrides now win; the per-type
map still beats the global agent default.
Refs #1540
Add deduplicateHistory() to HistoryManager that collapses consecutive
same-alert entries within a 5-minute window. The flapping/re-fire bugs
(lifecycle and stateful paths) created many duplicate history entries
before the cooldown fixes were deployed. This cleanup runs on startup
after loadHistory and periodically alongside cleanOldEntries, so both
existing noise and any future edge-case duplicates are handled.
On this install: 332 entries → 130 (60% reduction).
Stateful alerts (ZFS pool/device, storage topology) were creating
duplicate history entries every poll cycle because SyncStorageAlertsForInstance
clears alerts between evaluations, making the existing-alert check miss.
Add the same 5-minute re-fire cooldown used in the lifecycle path: when a
stateful alert re-fires within 5 minutes of resolution, reactivate the
original (preserving StartTime) and update the existing history entry's
LastSeen instead of appending a new entry.
When an alert fires, resolves, and re-fires within 5 minutes (the
recently-resolved retention window), the previous implementation created
a new history entry for each fire cycle. For a flapping connection like
an unreachable Proxmox node, this produced hundreds of duplicate entries
in a single night — 302 identical 'pi unreachable' alerts in one case.
The fix checks the recently-resolved map before creating a new history
entry. If the alert was resolved within the cooldown window, it
reactivates the original alert (preserving StartTime) and updates the
existing history entry's LastSeen instead of appending a new one.
Existing alerts kept their stale message from when they first fired.
Now the message is regenerated on every evaluation cycle so label
fixes and wording improvements take effect immediately.
The critical (severity-escalation) threshold was hardcoded as
Trigger + 10 for all metric types. For percentage metrics (cpu,
memory, disk, usage) with high triggers, this made critical
escalation unreachable: a CPU trigger of 95% produced a critical
threshold of 105%, which is impossible for a 0-100% metric.
Fix: add computeCriticalThreshold() helper that caps the critical
threshold at 99 for percentage metrics. Non-percentage metrics
(temperature, diskRead, diskWrite, networkIn, networkOut) keep
the Trigger + 10 offset unchanged.
Applied to all three code paths that derive critical from trigger:
- buildCanonicalMetricSpec (canonical evaluation)
- checkMetric new alert creation (legacy path)
- checkMetric existing alert update (legacy path)
SyncStorageAlertsForInstance matched alert.ResourceID against valid
storage IDs (plain inventory IDs like 'inst-node-local-zfs'). But ZFS
health assessment alerts carry a composite spec resource ID (e.g.
'inst-node-local-zfs/zfs-pool:data') set via params.Spec.ResourceID in
evaluateCanonicalStatefulAlert. Every ZFS alert was immediately cleared
by the sync sweep right after creation, then re-fired on the next poll —
an infinite flapping loop every ~10 seconds.
Fix: when the exact resource ID doesn't match, check whether the prefix
before the first '/' is a valid storage ID. Storage IDs use hyphens
(never slashes), so the prefix cleanly extracts the inventory ID from
the composite spec resource ID.
Added two test cases:
- preserves zfs alerts with composite spec resource ids
- clears zfs alerts when storage is removed (ensures fix doesn't
prevent legitimate cleanup)
The alert engine fired powered-off alerts for every stopped VM and
container regardless of whether the guest was configured to autostart.
This meant intentionally-stopped guests (onboot=0) generated alarm
fatigue — in the live lab, 7 of 12 alerts were noise from stopped VMs
that are deliberately off.
Now the alert engine checks the Proxmox onboot setting before firing:
- onboot=true → stopped is unexpected, alert (preserved behavior)
- onboot=false → stopped is expected, suppress
- onboot=nil → unknown, alert (preserved behavior)
For containers, onboot is parsed from the already-fetched container
config in enrichContainerMetadata (no new API call). For VMs, the
config is fetched only for stopped VMs via GetVMConfig (one extra call
per stopped VM, running VMs are unaffected).
Models: add OnBoot *bool to VM and Container.
Alerts: add onboot-aware suppression branch in CheckGuest.
Monitoring: add parseProxmoxOnBoot helper and fetchVMOnBoot.
The alert engine stamps metadata.resourceType on every alert, but the
websocket state path converts alerts.Alert to models.Alert, which had no
Metadata field, so every active alert reached the frontend stripped. The
history Type badge then fell back to unified-store lookups that miss
nodes (alert.resourceId is the platform-native node ID while unified
resources mint canonical ids, and alert.resourceName is the raw node
name while unified resources prefer the display name), rendering
Unknown. In mock mode the generated history rows had the same gap.
models.Alert gained the Metadata field in f62f35e24 (it rode along with
the memory-cache commit); this completes the transport:
- copy Metadata in activeAlertsSnapshot (websocket active alerts),
GetRecentlyResolved (resolved alerts to state), and the mock
UpdateAlertSnapshots conversion; sources are deep clones already
- deep-copy Metadata in models cloneAlert to keep the snapshot
clone contract honest
- stamp resourceType in the mock history generator using the real
engine vocabulary (node, vm, system-container)
- recognize system-container in the history Type badge map; that is
what the v6 engine stamps for LXC guests
Verified live in mock mode: history previously resolved 277 of 780
rows to Unknown (all node alerts); now 718/718 rows and 19/19 active
alerts carry resourceType and zero badges render Unknown.
Clears the dupl pairs outside internal/api:
- internal/alerts/pmg.go: the total/deferred/hold per-node queue checks
share evaluatePMGNodeQueueAlert; the historical short-circuit (a
below-threshold clear or invalid spec skips the node's remaining
checks) is preserved via the helper's skip-node return.
- internal/vmware/provider.go: the cloneInventory* family rides generic
cloneSliceWith / cloneShallowSlice helpers instead of fourteen copies
of the same nil/make/loop scaffold.
- internal/vmware/client.go + client_signals.go: the byte-identical
Automation and VI/JSON fetchers delegate to one getSessionScopedJSON.
- internal/vmware/fixtures.go: added to the dupl exclude list in
.golangci.yml — literal mock fixture catalogs are the same category as
the existing internal/mock/ exclusion.
- pkg/licensing/license_server_client.go: Activate and
ExchangeLegacyLicense share postActivationRequest (idempotent POST +
shared activation response decode).
- pkg/proxmox/client.go: LXC/VM RRD fetches share getGuestRRDData.
- pkg/pulsecli/actions.go is intentionally left for the api-contracts
slice.
Full test suites pass for internal/alerts, internal/vmware,
pkg/licensing, pkg/proxmox.
Follow-up to a85ec40a4 (#19), caught by independent re-review. Ceph pool
storage ("<instance>-ceph-pool-<name>") is raised/cleared by the separate
Ceph poll path (checkCephPoolStorage) and is NOT in the allStorage
inventory passed to SyncStorageAlertsForInstance — which runs before
pollCephCluster. Without a guard the sweep cleared the ceph pool usage
alert every storage poll and pollCephCluster re-raised it every cycle
(flapping: notification spam + StartTime reset). Skip any alert whose
ResourceID contains '-ceph-pool-'. Adds a regression test.
Back-port v5 fix 117ec901d to v6, adapted to v6's config subpackage.
Adds NormalizePBSDefaults (called from UpdateConfig alongside the other
Normalize*Defaults) and pbs.cpu/pbs.memory to ValidateHysteresisThresholds.
PBS was the only monitored resource type whose thresholds were not
normalized on UpdateConfig, so user-supplied configs with nil/negative
PBS thresholds were never defaulted and a Trigger=0 'disable' sentinel
did not clear the Clear value. Adds regression test.
Back-port v5 fix a4834ed80 to v6. hostDisplayName now appends
'(Host Agent)' (or returns 'Host Agent' for an unnamed agent) when the
host is linked to a Proxmox node/VM/container, and the disk-temperature
resource name uses hostDisplayName(host) instead of host.DisplayName.
Without this, a host agent running alongside the Proxmox node/VM it is
linked to produced alerts with an identical name, making them
indistinguishable. Adapted to v6's 'Agent' fallback. Adds test cases.
Back-port v5 fix 398ef8117 to v6, adapted to v6's canonical alert keying.
pollStorageWithNodes now calls SyncStorageAlertsForInstance after the
per-storage checks, clearing storage-inventory alerts whose ResourceID is
no longer present. Matches by ResourceID (every v6 storage alert sets
ResourceID=storage.ID) rather than reconstructed alert keys, so it is
robust to v6's composite alert-state IDs (e.g. ...::metric-threshold:usage).
Conservative: live storage keeps its alert (ResourceID still valid),
non-storage alerts are untouched, Ceph alias IDs are treated as valid.
Without this, a storage that disappears or changes identity left its
alert visible until the multi-day stale-alert TTL. Adds unit + poll
integration regression tests.
Back-port v5 fix 9fb76579c to v6. CheckBackupsWithInventory now filters
same-VMID guest candidates by backup kind (qemu/lxc) before attributing
a backup, in both the canonical-key lookup and the bare-VMID secondary
path. Without this, a vm backup with vmid=101 was attributed to an lxc
container with the same vmid (or vice-versa), suppressing the
orphaned-backup alert and attaching the wrong guest identity.
Adds guestMatchesBackupType/filterGuestsByBackupType helpers and a
regression test.
Verified via deadcode/ts-prune detection + 317-agent cross-repo verification swarm.
Gated: go build + go vet + test-compile (pulse module), tsc --noEmit (full project
incl tests), npm run lint. Net -2549 lines across 161 files.
Apply max alerts/hour to cooldown and critical re-notification paths so active alerts cannot bypass the UI delivery cap.
Persist LastNotified on active alert records when dispatching, including clone dispatches after reload.
Refs #1444
Dead-code sweep. Functions flagged unreachable by golang.org/x/tools/cmd/deadcode
and confirmed unused across pulse, pulse-enterprise, pulse-pro and pulse-mobile by
adversarial cross-repo verification. Cross-module reachability was checked
explicitly (only pkg/ exported symbols are importable by other modules; internal/
packages and _test.go files are not). go build, go vet and test-compile all pass.
Seeds AlertConfig.DiskFillByType in defaultAlertConfig() with the three
forward-compatible keys (nvme 92/87, sata 90/85, hdd 85/80). Adds
NormalizeDiskFillByType to lowercase keys on load, seed defaults when
the map is nil, and reset non-positive trigger/clear values to the
default for that key. NormalizeAgentDefaults invokes the new helper so
it runs through the existing UpdateConfig normalization chain. Round
trip tests cover nil seed, customized survival, negative reset, and
mixed case normalization, plus a defaultAlertConfig seed assertion.
Only the "nvme" key is consulted by the host evaluation today (the
inference helper only matches /dev/nvme*); "sata" and "hdd" are
forward-compatible placeholders for when the agent protocol carries
hardware type explicitly.
Inserts a per-type threshold lookup before the global AgentDefaults.Disk
fallback in CheckHost's disk evaluation. inferDiskHardwareType maps the
disk device path to "nvme" (only NVMe is inferable today); when the
DiskFillByType map carries a matching key, that hysteresis threshold is
used, otherwise evaluation falls through to the existing global default.
Storage-type unified_eval branch is unchanged.
Tests prove: (a) NVMe device at 91% does not alert when DiskFillByType
sets the nvme trigger to 92; (b) the same device at 93% alerts at the
nvme threshold; (c) /dev/sda1 falls back to the global threshold; (d)
the storage-type alert branch is not regressed.
Adds DiskFillByType map to AlertConfig (keys: nvme, sata, hdd ->
HysteresisThreshold). Adds inferDiskHardwareType helper that returns
"nvme" for /dev/nvme* device paths (case-insensitive) and "" for
anything else, since sata vs hdd cannot be reliably inferred from
device paths today. Unit tests cover the four input shapes (nvme,
sata, empty, mixed case).
Substrate only. No wire-in to host alert evaluation yet.
When the alert manager suppresses an alert because it is flapping, today
the suppression is silent: the operator sees no alert and no diagnosis.
That trains people not to trust Pulse on flapping resources -- the
detector did its job, but the user just sees absence.
Add a one-shot Manager callback that fires exactly when a tracking key
crosses from quiet into the flapping cooldown window. The callback is
the hook downstream surfaces (patrol, findings) will use to explain
what is flapping and why Pulse stopped notifying.
Lock-safety: checkFlappingLocked now returns (suppress, justTransitioned).
The caller (dispatchAlert) reads the transition signal, then dispatches
the callback from a fresh goroutine. The alerts manager mutex is never
held while the callback runs, so subscribers may take their own locks
or re-enter the alerts package without deadlock.
Subsequent dispatches inside the cooldown window remain silent: the
transition flag is only set on the call that flips flappingActive from
false to true.
A Proxmox host wedged on a ZFS deadlock yesterday took the cluster API poll
with it (context deadline exceeded). The unified connections aggregator
flipped the Connection from active to stale to unreachable, and the
Settings / Infrastructure page rendered the right badges, but no top-nav
alert ever fired because nothing was actively notifying off that derived
state. Patrol's deterministic triage flagged it every minute, but its LLM
investigation stage has been broken since 2026-02-26 so flags never
escalated into user-visible findings. Result: a 3 hour outage I only
noticed because I happened to open Settings.
This wires an active notification off the same connection state the
Settings badges already use:
- internal/alerts/connection.go: new CheckConnection +
clearConnectionDegradedAlert that fire connection-degraded after three
consecutive stale or unreachable observations. Severity scales: stale
warning, unreachable / unauthorized critical. Clear runs through the
same recovery-confirmation gate as clearNodeOfflineAlert so a single
flap back to active doesn't silently resolve a real outage. Paused,
disabled, and non-platform connections are no-ops.
- internal/api/connections_alerts.go: snapshot translator that turns
api.Connection into the narrow alerts.ConnectionSnapshot view. Keeping
the snapshot type inside the alerts package preserves the existing
api -> monitoring import direction; the monitor would have cycled if
it called back into api directly.
- internal/monitoring: new SetConnectionsSnapshotLister hook + a
per-tick checkConnectionAlerts call in the main poll loop, alongside
the existing evaluate*Agents passes.
- internal/api/router.go: register the lister closure on r.monitor so
the alerts loop sees the same Connection rows the HTTP handler does.
- internal/alerts/specs/types.go: add "connection" to the migration
bridge list of accepted ResourceTypes, alongside node / docker-host /
proxmox-disk / etc. The connection concept doesn't have a canonical
unified resource type yet; this matches the existing pattern for
alert-keyed resources that aren't first-class canonical.
Test coverage in internal/alerts/connection_test.go covers active never
fires, three stale observations escalate from pending to warning,
unreachable escalates warning to critical, unauthorized fires critical
cold, paused / disabled / agent never fire, recovery confirmation gate,
and a stale flap during recovery resets the gate.
TestResourceAlertSpecValidateAllowsConnectionMigrationBridgeType mirrors
the existing migration-bridge proof tests for the new type.
CleanupAlertsForNodes removes alerts whose Node isn't in
existingNodes. That map is built upstream only from state.Nodes
(Proxmox nodes) and state.PBSInstances (PBS instances) — no agent
resources. Agent-sourced alerts (Unraid, standalone Linux hosts,
TrueNAS, anything reached via Pulse Agent) typically have Node=""
or an agent UUID, so they fall into the cleanup branch every
cycle. Then the next poll re-creates the alert as new, calls
AddAlert, and appends a fresh history row. Observed in the wild:
3,980 alert history entries in 7 days, with the same canonical
alert ID (e.g. "Unraid array running without parity protection")
appearing every 30 seconds.
Adds a carve-out for ResourceID prefixes starting with "agent:",
matching the existing pattern for "docker-" / "docker:" and
"pbs-" / "pbs-offline". Locks in the behaviour with a new subtest
that mixes agent-sourced and Proxmox-sourced alerts and asserts
that only the legitimately stale Proxmox alert is removed.
Delete the residual internal/alerts/alerts.go catch-all and move its remaining ownership groups into config_facade, model, constants, metric_hooks, manager, default_config, lifecycle, and escalation files.
Keep compatibility aliases in the alerts package, preserve the type-alias identity test, add a default-config pointer isolation regression test, and record the new ownership split in the alerts subsystem contract.
Proof: git diff --check -- docs/release-control/v6/internal/subsystems/alerts.md internal/alerts
Proof: go test ./internal/alerts/... -run 'TestDefaultAlertConfigUsesIndependentBackupAlertOrphanedPointer|TestTypeAliasIdentity|TestSetMetricHooks|TestSetLicenseCheckerStoresChecker|TestHistoryManager_Stop|TestEscalationDisabledWhenAlertsDisabled|TestEscalationDisabledWhenActivationPending|TestEscalationDisabledWhenActivationSnoozed|TestEscalationSkipsWhenScheduleDisabled|TestEscalationSkipsAcknowledgedAlerts|TestEscalationAdvancesLevels|TestEscalationDoesNotRepeatSameLevel|TestEscalationUsesCallback' -count=1
Proof: go test ./internal/alerts/... -count=1
Proof: go test ./internal/api -run Alert -count=1
Move active alert TTL cleanup, auto-ack cleanup, node cleanup, and full active-state reset into internal/alerts/active_cleanup.go.
Record active_cleanup.go in the alerts subsystem contract and broaden ClearActiveAlerts coverage for alias, Docker, recovery, and canonical acknowledgement maps.
Proof: go test ./internal/alerts/... -run 'TestClearActiveAlertsEmptyMaps|TestClearActiveAlertsWithExistingAlerts|TestCleanup|TestCleanupAlertsForNodes' -count=1
Proof: go test ./internal/alerts/... -count=1
Proof: go test ./internal/api -run Alert -count=1
Move active alert acknowledgement, manual clear, recovery clear, state preservation, resolved registration, and no-lock removal helpers into internal/alerts/active_lifecycle.go.
Record active_lifecycle.go in the alerts subsystem contract and add a canonical-alias manual-clear characterization test.
Proof: go test ./internal/alerts/... -run 'TestClearAlertMarksResolutionAndReturnsStatus|TestClearAlertByCanonicalAliasRemovesActiveState|TestAddRecentlyResolvedUsesCanonicalStorageKey|TestAcknowledgeAlertNotFound|TestUnacknowledgeAlertSuccess|TestUnacknowledgeAlertByCanonicalAlias' -count=1
Proof: go test ./internal/alerts/... -count=1
Proof: go test ./internal/api -run Alert -count=1
Move active alert save/load, secure storage handling, startup restore migration, periodic persistence, and stale tracking cleanup into dedicated alerts package owners.
Record active_persistence.go and tracking_cleanup.go in the alerts subsystem contract and add a load-time file-permission hardening characterization test.
Proof: go test ./internal/alerts/... -run 'TestCleanupStaleMaps|TestLoadActiveAlerts|TestLoadActiveAlertsHardensExistingFilePermissions|TestSaveAndLoadActiveAlerts_UsesManagerDataDirAndSecurePermissions|TestSaveActiveAlertsBackfillsCanonicalIdentityOnDiskWithoutMutatingLiveAlert' -count=1
Proof: go test ./internal/alerts/... -count=1
Proof: go test ./internal/api -run Alert -count=1
Move alert configuration normalization, activation migration, global-disable cleanup, active-alert reevaluation, and threshold override cloning into internal/alerts/config_runtime.go.
Record config_runtime.go as the alert config runtime owner in the alerts subsystem contract and add an activation-state preservation characterization test.
Proof: go test ./internal/alerts/... -count=1
Proof: go test ./internal/api -run Alert -count=1
rcourtman