v6.1.0's identity rework (eab73d245) made the VMID-only fallback refuse
to fire whenever a typed VMID exists on more than one PVE location.
Root-namespace snapshots with no matching comment then score zero for
every guest, so on setups with two clusters and overlapping VMIDs most
guests showed no backup at all - while PVE itself listed the backups
fine, because monitoring discards pbs-type storage contents entirely
whenever a direct PBS connection is configured.
Attribution is now evidence-driven instead of dropped:
Storage backup polling keeps a per-connection record of every snapshot
its own pbs-type storage listed (type, VMID, backup time) even though
the raw entries stay out of the PVE backup list. Which cluster listed a
snapshot is deterministic attribution, and it survives fully mirrored
clusters that share one datastore and token. The evidence is
monitoring-internal, cleared on instance retirement or when the storage
poll stops seeing pbs content, and never serialized into state payloads
or snapshots.
Guest backup-time sync and the recovery-point mapper additionally learn
each PBS submission source's cluster (owner token, datastore, PBS
instance - strongest first, scoped to the PBS instance) from the poll's
attributable snapshots, then resolve collision VMIDs whose snapshots
carry no evidence of their own. A source seen from several clusters is
not a discriminator, an unfamiliar component stops resolution rather
than deferring to weaker ones, and a snapshot decisively attributed to
another cluster is kept away from this one. Unattributable snapshots
still drop rather than guess.
Backup-age alert attribution no longer suffix-matches the subject ref's
connection label against guest locations. The label there is a PVE or
PBS instance name, not a PBS namespace, and loose matching could
cross-attribute clusters sharing a VMID; it now requires exact
normalized equality.
Reported in #1639 (two PVE clusters with PBS 4.0/4.1, VM 173 shown 974
days overdue despite valid verified backups).
Fixes#1639
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TestUnifiedProviderIncidentRecoveryConfirmationSurvivesRestart hardcoded
observedAt as 2026-07-24, but the restore path drops persisted alerts
older than 24h, so the test began failing once the calendar moved past
the fixed date. Use a recent relative timestamp instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Branch-coverage tests for twenty partially-covered pure helpers that the
existing suites only reached through higher-level paths, so their arms were
never pinned directly.
internal/alerts read model and filter evaluation: metadataBoolValue 25.0 to
100, metadataIntValue 27.8 to 100 and numericConditionValue 22.2 to 100, each
covering every accepted underlying type plus the wrong-type and missing-key
arms; the three sort-rank helpers now assert an exact rank for every input
class including the default, which is what pins the ordering.
internal/alerts guest and backup identity: snapshotAlertStillTriggered 15.4 to
100 with the threshold pinned at, below and above; parseStableGuestOverrideKey
30.0 to 100; extractGuestSnapshot 35.7 to 100 across its type switch; and
hasActiveAlertTrackingKeyNoLock 40.0 to 100 including the canonical-scan and
nil-skip arms.
internal/alerts/specs: both Validate methods 57.1 and 58.8 to 100 with every
rejection reason asserted on its specific error, and metricTriggered and
metricStillLatched 50.0 to 100 with the metric pinned at, below and above the
threshold in both latch states.
internal/actionplanner: validateParamType 29.4 to 100 over every parameter type
plus the unknown default, validateParamValue 38.9 to 100 per rejection reason,
and both normalize helpers 38.5 and 40.0 to 100 over nil, empty, duplicate and
well-formed input. enumString also moved 50.0 to 100 as a consequence.
Package statement coverage moves to 87.1 percent for internal/alerts, 81.8 for
internal/alerts/specs and 88.5 for internal/actionplanner. Tests only, no
source change.
A third pass on partially covered functions, led by the alert evaluation
predicates where a wrong arm means a missed or spurious alert.
- internal/alerts/specs: matches 47.9 to 100, and all six matches helpers
(severity threshold, change threshold, baseline anomaly, health assessment,
posture threshold, and the severity latch) from 50 to 75 percent up to 100.
Each threshold is pinned at, just below and just above, and the latch arm is
exercised both latched and unlatched with concrete verdicts.
- pkg/audit: exportCSV 76 to 88 with commas, quotes and newlines in the detail
field asserted through a parsed round-trip; NewSigner error arms, both
IsPersistent predicates and VerifySignature against a tampered payload and a
wrong key.
- internal/agentupdate: retryBackoffDelay, sleepWithContext, Snapshot and
writeSelfTestTokenFile to 100, the token file exercised under t.TempDir
including the unwritable-directory arm.
- internal/notifications: writeMultipartBodyPart and alertNodeDisplay to 100,
attachment handling to 69, all asserted on the produced MIME text. No test
opens a network or SMTP connection.
- internal/unifiedresources: the three pure action-dispatch helpers to 100.
- internal/alerts/config: CanonicalResourceTypeKeys 34.3 to 78.4.
Five targets deliberately did not move and are recorded rather than faked:
the error arms of writeEmailThreadingHeaders, buildMultipartEmailMessage and
copyWebhookConfig are unreachable because those functions write only into a
local bytes.Buffer, which never errors; exportJSON's only gap is a
json.MarshalIndent failure that its event struct cannot produce; and
verifyBinaryMagic's remaining gap is a deferred close-error handler.
No source file is modified. Adversarial review returned no rejects and flagged
seven near-duplicate subtests; all were removed and every target function
re-measured at an identical percentage.
PULSE_ALLOW_CONTRACT_NEUTRAL_COMMIT=test-only branch coverage, no source or contract change
A second pass targeting PARTIALLY covered functions rather than untouched
ones, so every case here is an arm the existing suites never reached.
Percentages are per-function coverage, measured before and after.
- pkg/securityutil: the SSRF guards, which is where the uncovered arms
actually matter. isCarrierGradeNATIPv4 and isLocalNetworkIP are pinned at
the first and last address of 100.64.0.0/10 and just outside both ends,
across loopback, link-local, every RFC1918 range and IPv6 unique-local.
joinURLPath, IsLocalNetworkHost, resolveOutboundIPAddrs and
cloneRestrictedTransport to 100 percent, resolvePermittedOutboundIPs to
96.8, with the transport clone asserted independent of its source.
- internal/models: eleven deep-copy helpers from as low as 25 percent to 100.
Every one asserts real independence, mutating each nested slice, map and
pointer field of the clone and checking the original is untouched, which is
the failure mode a deep-copy helper actually has.
- internal/alerts: metricClearThreshold 28.6 to 100, resourceTypeLabel and
alertspecsMetricTriggered 50 to 100, the four canonical spec-id and
tracking-key builders 66.7 to 100, inferCanonicalKindFromLegacyAlert to 100,
and the backup-snapshot and ack-identity predicates.
- internal/servicediscovery: the four fingerprint generators to 100, each
asserted for both stability and sensitivity; the three command builders and
ValidateResourceID on their exact output and each rejection reason.
- internal/storagehealth: zfsScanActive and firstNonEmpty.
cephClusterSourceRank is deliberately left at 75 percent: its default arm is
unreachable because normalizeCephClusterSource can only return the two cases
above it. That is recorded rather than faked.
No source file is modified. Adversarial review returned no rejects and flagged
nine re-hit subtests; all nine were removed and every target function
re-measured at an identical percentage, proving they carried nothing.
PULSE_ALLOW_CONTRACT_NEUTRAL_COMMIT=test-only branch coverage, no source or contract change
Seventeen files closing the zero-coverage functions the current source drop
left behind. Every named target was measured off 0.0 percent by a per-function
coverage delta, re-measured against current main.
- config: the five new durable Proxmox cluster-node identity helpers
(deterministic id, endpoint equality, alias lookup, id existence, lookup by
id) to 100 percent; VMware and agent-profile persistence round-trips under
t.TempDir including that AppendProfileChangeLog appends rather than
replaces; AI chat session save, load, delete, per-user scoping and age
cleanup, with explicit timestamps rather than time.Now-relative fuzz;
PVEInstance.DeepCopy asserted for nested independence.
- truenas: incidentFromPoolStatus over every pool health string,
RecordsFromSnapshot over nil, empty and populated snapshots, both
TransportStatus accessors, and the RPC handshake and auth typed errors
through errors.Is and errors.As.
- unifiedresources: the maintenance-window operator-state lifecycle on
MemoryStore including the not-found and already-cleared arms, plus the
four remaining View accessors asserted on their exact formatted output.
- api: restoreAgentExecMetadata, buildAlertConnectionSnapshotsWithRuntimeSources
and both mock series generators, asserted on shape, ordering and
determinism rather than non-emptiness.
- cmd/pulse-control-plane: the four remaining MSP and mobile proof report
printers, asserted on the concrete strings in captured stdout.
- ai: cost.EmptySummary, approval.emptyExecutionState, demo.IsDemoRuntimeIntended
and tools.findCanonicalAppContainerResourceByReferences across no-match,
first-match, later-match and ambiguous references.
- monitoring, models, alerts: trueNASAppRunning,
supplementalProviderOwnedSourcesForOrg, IOCounterPresence.Effective,
ValidAlertIntentSignal and intentTimePointer.
No source file is modified. Adversarial review returned no rejects across all
seventeen files and flagged four padding cases plus one dead table field; all
were removed and the per-function coverage re-measured as identical, proving
they carried nothing.
PULSE_ALLOW_CONTRACT_NEUTRAL_COMMIT=test-only branch coverage, no source or contract change
Adds the subsystem contract deltas and verification artifacts for the
preceding wearout and physical-disk-node fixes, which landed without them.
The contracts now state the rule rather than leaving it implicit in each
caller. Wearout has two absent states and one real zero, -1 is the unreported
sentinel, and 0 is evidence only from a device that reports endurance at all.
storagehealth.WearoutReported is the single authority, and alerting, the
unified risk projection, and the frontend all gate on it. Storage-recovery
records that disk-to-node association is evidence-directional, so a disk with
no Proxmox scope still matches on node name while a scoped disk stays inside
its instance.
Proofs cover the predicate itself, a spent SSD alerting where it previously
stayed silent, absent evidence staying quiet on both the unreported sentinel
and a rotational zero, the view returning the sentinel rather than the struct
zero value, and the presentation helpers agreeing with the server.
Verified: full frontend suite 1061 files / 19405 tests, canonical completion
guard, gofmt, go build ./..., and internal/alerts, internal/monitoring,
internal/storagehealth, internal/unifiedresources.
Physical disks reported by a host agent carry no Proxmox scope, so tightening
matchesPhysicalDiskNode to require an instance on both sides dropped every
agent-reported disk off the Proxmox node it lives on. The Physical Disks node
filter rendered no options at all, grouping lost the node, and metric target
resolution could not find it. Match on node name when the disk side has no
instance evidence, and keep rejecting a Proxmox-scoped disk against a node
outside that instance, so cluster isolation still holds in the direction that
motivated the change.
Wearout had two competing readings of 0. The presentation layer treated it as
no endurance remaining while the alert path treated it as absent and stayed
silent, so a disk could read critical in the UI and never alert. Two sources
also published 0 for a disk that reported nothing: PhysicalDiskView.Wearout
returned the struct zero value on a nil view, as did the nil-view branch of
physicalDiskFromReadStateView. Both now return the documented -1 sentinel.
storagehealth.WearoutReported is the single authority for whether a reading is
evidence at all: -1 is absent, 0 is real only from a device that reports
endurance, and rotational disks never do. Risk assessment, alerting, and the
frontend all gate on it rather than each reinventing the boundary.
Verified: full frontend suite 1061 files / 19402 tests, eslint, theme and
canonical-platform audits, tsc, go build ./..., and internal/alerts,
internal/monitoring, internal/storagehealth, internal/unifiedresources.
Route canonical cooldown refires through one resolved-state critical section and document the m.mu to resolvedMutex invariant. Exercise production refire, recovery, broadcast, alias repair, cleanup, and shutdown paths under the race detector while preserving cooldown history and start-time semantics.
Adds branch-coverage tests for eight packages whose target functions were
measured at 0% before this change. Every named target was verified to move
by running each package's coverage with and without the new file.
- internal/ai/eval: all 36 Scenario constructors and the four PatrolScenario
constructors 0% -> 100%. These are catalog-invariant tests, not literal
echoes: unique names, populated required fields, runnable assertions, tool
references checked against the agentcapabilities registry, and exact
assertion-count deltas across the env-gated conditional appends. A parity
test scans scenarios.go itself, so adding a constructor without registering
it in the table now fails rather than silently going untested.
- cmd/pulse-control-plane: nine MSP and tenant-runtime print helpers
0% -> 100%, covering the nil, empty-slice and optional-field arms.
- internal/ai/memory: RemediationLog GetByID, MarkRolledBack and
GetRollbackable 0% -> 100%, pinning the overwrite-vs-preserve contract on
RollbackInfo and each falsy arm of the rollbackable predicate.
- internal/alerts/config: AlertConfig.UnmarshalJSON 0% -> 90% and
NormalizeAlertConfigAliases 52.9% -> 94.1%.
- internal/config: RunMigrationIfNeeded 0% -> 100%, copyFile 0% -> 88.9%.
- internal/mock: AvailabilityFixtures, FixtureGraph.SupplementalChanges and
generateMockHostRate 0% -> 100%.
- internal/api: testProxmoxPlatformConnection 0% -> 100% through its injected
connect func, so no network is involved.
- internal/servicediscovery: needsDeepScan 0% -> 100% across every return arm
including the confidence boundary.
No source file is modified. Adversarial review found no rejects; two findings
were acted on before committing, replacing a circular catalog-count assertion
with the real source-parity scan and reducing an AllPatrolScenarios test that
compared the function against the same constructors it calls to the ordering
and completeness signal that is actually independent.
PULSE_ALLOW_CONTRACT_NEUTRAL_COMMIT=test-only branch coverage, no source or contract change
Contract-Neutral: test-only branch coverage, no source or contract change
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.
courtmanr@gmail.com