Eight new branch-coverage tests taking thirty-one previously unreached
functions from zero to covered, with no source or existing test touched.
internal/kubernetesagent: twenty-one pure report helpers, including the pointer
converters proved non-aliasing in both directions, the ingress host and address
collectors across their trim, dedupe and insertion-order arms, the endpoint
slice readiness count where a nil Ready field counts as ready, and the target
role predicate.
internal/agentexec: the sudo long-option value gate over the real option list
including the inline equals form, and the approval grant verification error
unwrapped through errors.Is.
internal/alerts: the alert config alias normalization across the nil config
guard, the empty threshold early return, the blank type-key continue arm and
the legacy-delete versus supported-keep split, asserting both maps stay
independent.
internal/alerts/specs: the resource incident rollup evidence validation, each
failure arm asserted on its concrete error and the check order pinned when
several fields are invalid at once.
internal/cloudcp/docker: the not-found predicate through a wrapped error, the
route host label precedence, and the Traefik host rule parser across quoting
styles, combined matchers, multiple host clauses and malformed input.
internal/cloudcp/portal: the anonymous bootstrap builder, asserting no tenant
or user identity field is ever populated on the anonymous result.
internal/config: the legacy OIDC environment provider, including the arm where
an already-configured provider is present and the redirect derivation from a
public URL with a trailing slash.
internal/dockeragent: the update-all payload decode across wrong-typed and
missing fields, and the docker filter conversion.
Contract-Neutral: test-only branch coverage, no contract surface touched
Percentage metric specs derive their critical escalation as trigger+10
capped at 99. A user-set trigger of 99 or 100 produced critical <=
trigger, which failed spec validation and silently dropped the whole
metric from evaluation while logging a warn every poll cycle. Omit the
escalation threshold instead so the warning-level threshold keeps
working.
Seen in the wild as the 'Skipping invalid canonical unified metric
spec' warn spam in #1593's diagnostics.
Contract-Neutral: behavior fix with regression test; alerts contract docs carry another agent's WIP on the shared tree, delta deferred
A PBS datastore reached the unified registry twice, once from the PBS
snapshot adapter under the canonical <instance-id>/<name> source ID and
once from the PBS poller's models.Storage conversion under the legacy
<instance-id>-<name> storage ID. The thresholds page rendered two cards
per datastore and their alert overrides split across both key formats,
with only the legacy key honoured at evaluation time.
Skip the redundant storage-conversion ingest when the canonical
datastore resource is already registered, carry the canonical ID as a
storage alias so evaluation accepts overrides in either format, and add
the legacy dash key to the frontend candidate list so pre-existing
overrides stay bound to the single surviving card and re-home onto the
canonical key on the next save.
Fixes#1591
Contract-Neutral: behavior fix with regression tests across registry/alerts/frontend; contract docs carry another agent's WIP on the shared tree, delta deferred
The canonical alert evaluation reactivation branch read and mutated
recentlyResolved and resolvedAlias while holding only m.mu, while the
broadcaster and recovery paths guarded them with resolvedMutex, so the
two lock domains did not exclude each other and a flapping resource
re-firing within the five-minute cooldown concurrently with the state
broadcast crashed the daemon with a concurrent map access fault
(issue #1590, reported with exact sites and a race repro).
Both eval sites now take resolvedMutex for the lookup and removal only,
keeping history and dispatch outside the lock. GetResolvedAlert takes
the write lock because getResolvedAlertNoLock backfills resolvedAlias
on a canonical-identity miss, which made even read-locked lookups
racy against each other. The lock ordering documentation now states
the real rule the cleanup path already relied on: resolvedMutex is
subordinate to m.mu and may be acquired while holding it, never the
reverse.
Contract-Neutral: crash bugfix restoring the documented resolved-map lock guarantee (#1590); no contract surface changes
The DisableAllDockerContainers and DisableAllDockerServices config sweeps
matched only legacy docker-container-/docker-service- alert ID prefixes.
Canonical stateful alerts (image-update, state, health, replica-gap) are
stored under <resourceID>::<specID> state IDs, so existing alerts survived
the toggle and lingered indefinitely, re-notifying past cooldown.
Match on the docker: resource ID scheme as well as the legacy prefixes.
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.
rcourtman