Real-mode agent host metrics are written to the metrics store keyed by
host.ID (monitor_agents.go writes "agent"/host.ID, pinned by a canonical
guardrail), and ingestHost registers that same ID as the SourceAgent
mapping. But BuildMetricsTarget preferred the Proxmox/VMware source ID
for agent resources, so a host that is also a Proxmox node (delly,
minipc, pi) advertised an "agent"-typed metrics target (e.g.
"homelab-delly") that nothing ever writes. Every reader that trusts the
registry target queried zero rows: performance reports resolved via
MetricsResourceID rendered "Data Points: 0", and the resource drawer
history endpoint fell back to in-memory node history (losing
temperature and disk I/O series the agent records).
Flip the priority: the agent source wins for agent resources, platform
sources (Proxmox, VMware, TrueNAS, Docker) remain fallbacks for hosts
without a reporting agent. This matches the write path exactly - the
bulk charts feed (hostAgentChartRequest) already preferred the agent ID,
and pure-agent hosts already resolved to it, so merged hosts simply
become consistent. No history is orphaned: "agent" store rows were
always keyed by host.ID, and node-poller rows ("node"/<node.ID>) were
never reachable through the "agent"-typed target.
Verified live: all five real agents now resolve metricsTarget to their
agent UUID, /api/metrics-store/history returns store rows for the new
target, and the delly performance report renders 312 data points where
it previously rendered zero.
Regression tests: BuildMetricsTarget prefers the agent source for
merged-source resources, and a registry ingest test asserts the merged
node+agent resource resolves its target to host.ID - the agent store
write key.
A design-partner review of real output called the PDFs out as thin, and
the page renders agreed: fleet reports spent one nearly blank A4 page
per resource, metric cards showed raw store keys with unformatted
values ('diskread 880000.00'), and value cells overlapped the adjacent
column for long readings.
- Fleet per-resource blocks now flow several to a page with separators,
breaking only when the next block will not fit (a 6-resource report
drops from 8 pages to 3); each block gains an availability line
- Rate metrics get display names and humane units (Disk Read 859.38
KiB/s instead of diskread 880000.00); byte units are self-describing
so the old +unit suffix no longer renders '12.00 GiBbytes'
- Metric card stat columns use bounded cells so long values cannot run
under the Avg/Samples labels
- Single reports merge Resource Details and Performance Summary onto a
shared page when they fit instead of stranding a third of a page of
content on each of two pages
Verified by regenerating branded fleet and single reports and reviewing
every rendered page.
Performance reports answered 'what were the averages' but never 'was my
infrastructure up' - the question a managed-service client reads a
monthly report for. Reports now carry an Availability summary derived
from the recorded resource change timeline (state_transition entries
keyed by the canonical unified ID):
- uptime percent over the observed portion of the window, outage count,
total downtime, and longest outage, rendered in the executive summary
with an explicit semantics note; fleet summaries gain a per-resource
Uptime column and CSV exports gain availability header lines
- absent/unknown spans are unobserved time: excluded from the uptime
math entirely and disclosed as coverage, never counted as downtime.
The journal records a registry absence for every monitor restart, so
treating gaps as outages would invent fleet-wide downtime every time
the operator restarts Pulse
- warning states count as up (the resource is reachable and serving);
the uptime label clamps rounding so any real downtime can never
display as a clean 100%
- resources with no timeline render no availability section at all
rather than a fabricated number
Verified live against a real 7-day window: uptime/outage/downtime
figures reconcile with the raw resource_changes journal.
Free-form strings entering the PDF generator (AI narrative prose, resource
names, alert messages, brand display names) were written to fpdf core fonts
as raw UTF-8, and the cp1252-decoding fonts rendered em dashes and curly
quotes as mojibake. Generate and GenerateMulti now run every string field
reachable from ReportData/MultiReportData through fpdf's cp1252 translator
once before rendering, so write sites stay encoding-free. The translator is
built per call: fpdf's closure reuses an internal buffer and the generator
is shared across concurrent requests. Runes outside cp1252 degrade to '.'.
Tests render AI-shaped narratives with em dashes and curly quotes for both
the single-resource and fleet paths and assert the extracted content
streams decode without mojibake.
Performance reports were structurally disconnected from the v6 ID
space: the UI (and any API caller working from /api/state) addresses
resources by canonical unified ID, while the metrics store is keyed by
each platform's native source ID (the resource's metricsTarget). The
engine queried the store with the unified ID verbatim, so every report
rendered 'Data Points: 0' regardless of how much history existed, and
covers showed raw hash IDs a report reader cannot map to a machine.
- MetricReportRequest gains MetricsResourceID: handlers resolve the
unified ID through the tenant monitor's resource store (new
Monitor.MetricsTargetForResource accessor; the registry computes
targets on demand, they are not persisted on snapshot structs) and
the engine uses it for store queries only. Recovery points and
Patrol findings stay keyed by the unified ID.
- Legacy snapshot models and their alerts are keyed by the metrics
target ID, so enrichment now matches either ID space and resource
names/status resolve again on covers, headers, and fleet rows.
- Fleet summaries mirror the single-report guard: zero data points
across the fleet renders a muted NO DATA card instead of a green
HEALTHY 'All systems operating normally' - false reassurance is the
worst failure mode for a client-facing stability report.
- Em dashes in PDF-bound literals become hyphens; fpdf core fonts are
cp1252 and rendered them as mojibake.
In dev/demo builds HasFeature returned the implicit dev grant verdict
unconditionally, so features excluded from that grant (white_label,
multi_user, unlimited, env-gated multi_tenant) were denied even when an
explicitly activated, signature-verified license carried them - while
/api/license/status reported them active from the same claims. That
inconsistency made branded report rendering impossible to exercise in
dev builds. Excluded features now fall through to real entitlement
evaluation, so an activated license behaves the same in dev as in
release builds; the no-license dev posture is unchanged.
A cluster has one API connection, but every member node row repeated the
connection-level "API" badge next to the node's discovered URL. In the
rc.6 stacked layout that read as per-node API keys to a real user (#1493:
"API keys ??? I would say URL!!!"), and it still implied per-node
credentials in the flattened table.
Member rows now only badge sources that are actually node-local: an
attached Pulse Agent. API-only members show a muted dash whose tooltip
explains the node is monitored through the cluster's single API
connection. The cluster row itself keeps its API badge under the Method
header, and standalone connections (which do own their credentials) are
unchanged.
Refs #1493
Cluster endpoints were discovered once at add time and never re-read:
detectClusterMembership returned immediately for instances already marked
IsCluster, so when a cluster moved subnets Pulse kept dialing the dead
per-node addresses for failover and kept displaying them on Settings ->
Infrastructure, even though monitoring still worked through the main host
fallback. The manual /refresh-cluster endpoint exists but is not wired to
any UI control, so nothing ever corrected the stored addresses.
The 5-minute cluster re-check now also covers cluster instances: it
re-reads /cluster/status through the configured main host, and when a
node's address or the node set actually changed (volatile fields like
Online/LastSeen are ignored) it updates the stored endpoints, persists the
config, and rebuilds the failover client so polling moves to the new
addresses. User-managed fields (IP overrides, guest URLs, fingerprints)
are preserved by the existing discovery helpers, and fields the cluster
status API omits are inherited from the stored endpoint rather than
erased.
Refs #1493
recordTaskResult reset the failure counters on a successful poll but left
LastErrorAt/LastErrorMessage/LastErrorCategory in place forever. The
connections aggregator surfaces that field as a live error banner on
Settings -> Infrastructure and derives the Unauthorized state from it, so
one transient cluster outage (or startup blip) pinned a red "no healthy
nodes available" banner onto a connection that polls fine, and a single
past 401 could pin a healthy connection to Unauthorized.
LastError now means "current outstanding failure since the last success":
recorded on failure, cleared on success. Connections that are genuinely
failing (no successful poll) keep their banner.
Refs #1493
pollPVEInstance returned early on a GetNodes error, so a host that
stopped answering entirely (shutdown, network loss) kept its last
successful snapshot in state forever: node row frozen at online with
stale uptime. The existing 60s offline grace policy only ran for
cluster-reported peer-down and empty-poll cycles, never for a whole
instance going dark.
Route the poll error path through the same grace policy: nodes of an
unreachable instance stay online (connection degraded) within the
grace window and flip to offline with cleared uptime and CPU once it
lapses, matching the cluster-reported node-down behavior.
Verified live: registered a synthetic PVE instance, killed it, and the
node flips to offline one poll cycle after the grace period.
Fixes#1441
On the #1483 reporter's Proxmox node a SATA SSD (INTEL SSDSC2BW240A4) never
appeared in Pulse even though smartctl reads it fine, because the agent's
discovery and probing both had single points of failure:
- Discovery trusted smartctl --scan-open exclusively whenever it returned at
least one target. The scan silently omits any device it fails to open at
scan time (the failure is only a #-comment in its output), so one bad open
hid a real disk while its neighbours were listed.
- A scan-provided -d type got exactly one probe attempt on Linux. A type whose
full query (-i -A -H) fails or returns nothing dropped the disk silently.
- A probe yielding no usable SMART data dropped the disk entirely instead of
reporting the identity the kernel can prove.
Fixes, all in the agent collector:
- Union discovery: the kernel block device list (/sys/block, lsblk fallback)
is the ground truth for which disks exist; smartctl --scan-open only
contributes device-type hints. Any physical disk the scan misses gets an
untyped probe target.
- Untyped retry: a typed Linux probe that errors, yields no usable data, or
fails to open (exit bit 2, distinct from -n standby,3's exit 3) retries with
smartctl auto-detection before giving up. Multiplexed controller members
(megaraid, cciss, ...) are exempt since dropping -d would probe the array.
- Identity-only fallback: when every probe fails but the disk exists in
/sys/block with nonzero capacity, report device/model/serial/size with
health UNKNOWN instead of hiding it. No SMART data is fabricated;
multiplexed array paths and zero-capacity media are excluded.
- Exclusion follow-through: --disk-exclude now also matches the canonical
post-refine name (nvme0n1), not just the scan label (nvme0).
Regression tests use smartctl/lsblk fixtures captured from PVE 9.1.9 hosts
and the issue report. Verified live on two PVE nodes: NVMe keyed by
namespace with byte-exact pvesh sizes, SATA disk reported with full identity.
Completes the #1483 fix started in bd20069c6 (namespace devpath, authoritative
sizes, merge downgrade guard). Closes#1483.
Polling direction (provider to client site, 8006/8007) versus agent
check-in direction, per-client steps with the guided pveum setup command,
privilege-separated token caveat, TOFU certificate pinning, and the
overlapping-RFC1918 note that container-per-client isolation makes moot.
The install proof passed green this morning while every provisioned
workspace ran unlicensed; the entitlement gap survived because nothing
asserted lease health. The workspace proof now reads each workspace's
provisioned billing state and verifies the lease exactly the way a
release-build client runtime will: against the hosted entitlement trust
root, through the provider MSP license chain, requiring white_label.
A present license with an unverifiable lease fails the proof with the
specific reason; the environment-fallback plan (dev, no license) reports
entitlement_skipped_reason=no_provider_msp_license instead of asserting.
Proof and install-proof workspace output lines gain
entitlement_lease_checked/verified, entitlement_white_label, and the
skip reason.
The variable signs hosted entitlement leases; the trial-activation name is
left over from the retired trial era and reads as trial machinery to an
operator generating their licensing root key. The provider MSP bundle has
no installed base yet, so the canonical rename is free today and frozen
the moment the first design partner installs.
- Control plane reads CP_ENTITLEMENT_SIGNING_PRIVATE_KEY first and falls
back to CP_TRIAL_ACTIVATION_PRIVATE_KEY, so existing Pulse-hosted cloud
deployments (deploy/cloud, hibernated snapshot) keep working unchanged.
- deploy/provider-msp (.env.example, compose, setup.sh), MSP.md, and the
install-test pins use the canonical name; error messages name it too.
- deploy/cloud intentionally keeps the legacy name: that stack historically
signed hosted trial activations, and its snapshot predates the rename.
- MSP.md now leads with deploy/provider-msp/ (compose stack, setup.sh,
upgrade.sh, run-install-proof.sh), documents the HTTPS requirement
(__Host- portal session cookie) and the pulse.provider-msp.role labels
workspace provisioning requires, and explains the licence/lease chain
including licence-expiry behavior.
- setup.sh derives and prints the lease signing public key the provider
MSP licence must bind (also via --print-lease-signing-public-key), and
the missing-licence error now includes it with request instructions.
- .env.example documents the CP_TRIAL_ACTIVATION_PRIVATE_KEY binding.
Provider-hosted MSP client workspaces previously sat at Community tier
forever: the runtime refreshed leases against the built-in Pulse Cloud URL
(hibernated, 522) instead of the provider control plane, and release-build
images verify leases only against the embedded Pulse key, which an
operator-generated CP_TRIAL_ACTIVATION_PRIVATE_KEY can never satisfy.
- Inject PULSE_PRO_TRIAL_SIGNUP_URL=CP_BASE_URL into client containers so
lease refresh targets the provider control plane.
- Chain trust through the Pulse-signed provider MSP license: the license
binds the provider's lease signing public key
(entitlement_signing_public_key claim); the control plane embeds the
license in every lease (provider_license claim); release-build runtimes
verify embedded Pulse root -> provider license -> lease signature.
- Cap chain-verified leases at ProviderChainedLeaseCapabilities: MSP tier
plus white_label (branded per-client reports), minus Pulse-service-backed
relay/mobile_app/push_notifications, which otherwise loop doomed
registrations against Pulse's relay.
- Fail fast at control-plane startup when the license does not bind the
configured signing key, instead of provisioning silently unlicensed
client workspaces.
Verified live on a Colima harness: release-tagged tenant image with test
embedded root, Traefik TLS, full provider-msp proof, tenant reports
valid=true plan_version=msp_growth with white_label and zero relay
failures.
The Core E2E workflow on this branch lost the 'docker build -t
pulse:test --target runtime .' line that release/5.1 still has, so
every dispatched run failed at compose-up with 'pull access denied for
pulse'. Masked until now because the workflow only auto-triggers on
main.
Automates the docs/MSP.md validation checklist and the WEBHOOKS.md
delivery contract against the real server, covering the cross-layer
seams unit tests cannot see (today's two isolation bugs both lived
there):
- org-bound token allowed in its own org, denied 403 in sibling orgs
AND the default org (leaked client-site token must not read the
provider estate)
- dedicated agent-ingest port serves only /api/agents/* (management
paths 404, agent route auth-gated)
- client-org webhook delivery through the instance-wide private-target
allowlist saved in default-org context, with HMAC signature verified
against the documented recipe, X-Pulse-Event-ID idempotency header,
and tenant id/name stamped in the payload
- restart inheritance: after docker restart the persisted allowlist
still applies to lazily recreated tenant monitors (skips when the
docker CLI cannot manage the container)
Compose: pulse-test gains PULSE_AGENT_INGEST_PORT=7656 (+ port map)
and a host-gateway extra_host so the spec's capture listener is
reachable from the container.
Two gaps found by exercising the MSP pilot path live on a throwaway
multi-tenant instance:
1. CheckAccess granted any authenticated principal access to the default
org, so a token bound to a client org could read the provider's own
default-org estate if it leaked from a client site. Org-bound tokens
now fall through to the explicit binding check for the default org;
authenticated users and legacy unbound tokens keep default-org access,
and binding "default" explicitly still grants it.
2. The webhook private-target allowlist (instance-wide system setting)
only ever reached the default org's notification manager on
startup/reload, and only the request-context org on settings update.
Tenant orgs' webhooks to private targets (per-client Gotify over VPN,
the canonical MSP alert route) failed SSRF validation with no org-side
remedy, and any allowlist died with a restart. Settings updates and
reloads now fan out to every live tenant manager via the new
MultiTenantMonitor.ForEachMonitor, and tenant monitors inherit the
persisted allowlist and public URL at creation.
Both fixes verified live: org-bound token vs default org returns 403;
client-org webhooks to a private target succeed after restart and for
orgs created after the allowlist was saved. MSP.md validation checklist
gains the default-org probe and the allowlist guidance; MULTI_TENANT.md
documents the binding semantics. Contracts updated for api-contracts,
security-privacy, and monitoring with adjacency notes for
agent-lifecycle, storage-recovery, and performance-and-scalability.
Hosted tenant containers received PULSE_TENANT_ID but not
PULSE_TENANT_NAME, so alert webhook payloads from provider-hosted client
runtimes fell back to the raw tenant ID instead of a human-readable
workspace label. Resolve the display name from the tenant registry at
container-create time via a ManagerConfig resolver and stamp it
alongside the tenant ID. Display-name changes after creation apply on
the next runtime rollout, which recreates the container with freshly
resolved env.
Adds TestContract_MetadataGetPayloadsUseZeroRecordsInsteadOf404: empty
guest/docker metadata maps must serialize as {} (never null) and a
missing resource must return a 200 zero record echoing the requested ID
(never a 404). This is the proof companion to the
metadata_handlers_shared.go consolidation in the previous commit — it
was authored with that change but lost to a shared-index race at commit
time.
Alert webhook payloads now carry the tenant that fired them, so MSP/PSA
receivers (ConnectWise and similar) can route tickets by client without
inferring the tenant from which webhook endpoint fired.
- WebhookPayloadData gains TenantID/TenantName, exposed to custom
templates as {{.TenantID}}/{{.TenantName}}.
- Defaults come from PULSE_TENANT_ID/PULSE_TENANT_NAME (already injected
into provider-hosted client runtimes; name falls back to ID).
- Shared-process multi-tenant orgs override via a lazy org-backed
resolver wired in MultiTenantMonitor.GetMonitor, so display-name
renames are picked up without restart.
- Generic service template emits a tenant block when identity is set,
omits it otherwise; single-tenant payloads are unchanged.
- Notifications and monitoring subsystem contracts updated with the
tenant-identity ownership boundary; guardrail test pins the org
wiring.
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.
Clears the ten dupl pairs across internal/ai:
- patrol_intelligence.go, tools_query.go, tools_storage.go: VM and LXC
system-container paths collapse into generics over read-state view
method subsets (gatherGuestIntelligenceFromViews, canonicalGuestGetResult
+ guestViewGetResult, addCanonicalGuestSearchMatches +
addGuestViewSearchMatches, appendGuestDiskSummaries).
- tools_file.go: append/write share executeFileMutation driven by
fileMutationSpec (approval-command text, shell redirect, verification
strategy stay per-action and verbatim).
- tools_kubernetes.go: deployment restart / pod delete share
executeKubernetesResourceAction driven by kubernetesResourceAction.
- providers/anthropic.go + anthropic_oauth.go: message conversion shared
via convertMessagesToAnthropic (the OAuth copy was annotated 'same as
regular client').
- memory/changes.go + memory/remediation.go: history loading shared via
the generic loadMemoryHistory in memory/paths.go (10 MiB cap, sort,
missing-file semantics preserved via a found flag).
- findings.go and unified/alerts.go: Finding/findingJSON and
UnifiedFinding/unifiedFindingJSON are deliberate marshal-mirror twins
(AlertIdentifier json:"-" vs alert_identifier round-trip); merging
would break every public literal. Suppressed with nolint:dupl and
enforced instead by new reflect-based mirror-sync tests.
Contract Extension Points name the shared helpers and the mirror
invariant. Full ./internal/ai/... test tree passes.
Clears the six dupl pairs in internal/monitoring:
- poll_providers.go: PVE/PBS/PMG listInstances / describeInstances /
connectionStatus closures collapse into generic sortedClientNames,
describeProviderInstances, and providerConnectionStatuses helpers; the
PBS/PMG adapters are now built by newPrefixedPollProvider from a
prefixedPollProviderSpec (prefix drives status keys, health keys, and
the fallback instance name).
- truenas_poller.go / vmware_poller.go: the Start scheduling loop
(double-start guard, stopped-channel handshake, sync+poll cadence)
moves to startPollerLoop, and the active-connection config policy
(enabled instances keyed by trimmed connection ID, defaults applied)
moves to loadActiveInstanceConfigs, both in the new
platform_poller_shared.go.
- monitor_polling_vm.go / monitor_polling_containers.go: traditional
polling now records guest series via the existing canonical
recordGuestMetric helper (io/network sentinels preserve the historical
cpu/memory/disk-only behavior on this path); the source-shape
guardrails in canonical_guardrails_test.go and
memory_source_catalog_test.go pin the new delegation.
- mock_metrics_history.go: native and TrueNAS disk seeding share one
seedDiskTelemetry closure.
Proof: new TestTrueNASPollerDoubleStartKeepsSingleLoop pins the shared
lifecycle loop; new TestSeedMockMetricsHistory_DiskTelemetryParityAcrossNativeAndTrueNAS
pins four-series parity for both disk sources. Contract Extension Points
name the shared wiring layer and poller scaffold.
Full internal/monitoring test suite passes.
initDockerWithRetry and initKubernetesWithRetry were the same exponential
backoff loop (5s doubling to a 5m cap, cancel-aware wait, structured
retry logging) duplicated per module; dupl flagged the pair. Extract
initModuleWithRetry[T] and keep the two as thin wrappers so the
test-overridable newDockerAgent/newKubeAgent vars and call sites stay
unchanged. Per-module log strings are preserved verbatim.
Existing cmd/pulse-agent retry tests (success, cancel-before-connect,
cancel-during-wait, both modules) pass unchanged.
The k8s workload/config adapters in unifiedresources each hand-rolled the
same Resource literal (Technology, LastSeen, UpdatedAt, Kubernetes facet,
label tags) plus namespacedKubernetesIdentity return; dupl flagged the
StatefulSet/Job and ReplicaSet/DaemonSet clones. Fold the scaffold into
namespacedKubernetesResource and delegate all 19 namespaced adapters
(Service included) to it; cluster-scoped kinds (PV, StorageClass,
Namespace) keep bespoke identity construction.
Contract: unified-resources Extension Points now name the shared scaffold
as the way new namespaced kinds assemble resources. Proof: new
TestNamespacedKubernetesResourceScaffold pins the scaffold fields and
namespaced hostname identity.
Full internal/unifiedresources test suite passes.
Completes the deferred UI half of dad1152fc: the content_replaced
lifecycle event (emitted when a same-key re-detection's text is
substantially different from the existing finding — a key collision)
now renders as "Re-detected with different details" in the finding
timeline instead of the identifier-formatter fallback, so the operator
reads what actually happened in plain language. The event metadata
already carries the previous and new titles.
Ceremony this line deferred from the backend commit: patrol-intelligence
contract Current State entry (lifecycle label map rule + the new label),
ai-runtime cross-reference updated, and the label pinned in
FindingsPanel.test.ts lifecycleLabels (the subsystem's accepted proof).
111 FindingsPanel tests green.
Patrol finding identity is the LLM-assigned key (resource+category+key
hash -> ID), and recordFinding adds by plain ID merge: when the LLM
reuses a key for a genuinely different issue on the same resource, the
new report silently overwrites the existing finding's title,
description, and evidence while inheriting its lifecycle — and on a
resolved finding, the reactivation counts as a regression of the OLD
issue, inflating the regression counter with a fiction.
Forking the key on dissimilar content would be worse: LLM titles vary
run to run, and splitting one real issue into duplicate findings is a
bigger trust hit than a conflated history. So the merge semantics stand,
and the identity shift is recorded honestly: when a same-ID
re-detection's title shares essentially no keywords with the existing
title (keywordOverlap <= findingIdentityShiftMaxTitleOverlap, 0.2 —
resource/category/key are equal by construction so text is the only
discriminating signal), FindingsStore.Add appends a content_replaced
lifecycle event preserving both titles in metadata and logs the
collision for frequency observability. Rephrasings and identical
re-detections stay event-free per the heartbeat rule. The UI renders
the event through the existing identifier-formatter fallback ("Content
replaced"); a dedicated label is patrol-intelligence UI work and lands
with that subsystem's own ceremony.
Teeth in findings_lifecycle_test.go: distinct-issue collision records
exactly one event with both titles; rephrased and identical
re-detections record none; the resolved-finding collision shows BOTH
content_replaced and regressed so the regression can be read for what
it is. Contract: Current State entry in ai-runtime.md. Full internal/ai
tree green.
SaveAIUsageHistory and SavePatrolRunHistory were byte-for-byte clones
(lock, EnsureConfigDir, marshal, optional encrypt, write, debug log)
flagged by dupl. Fold the shared shape into saveHistoryData, the save-
side counterpart of the existing loadHistoryData generic, preserving
the exact error-wrap strings and log messages.
Full internal/config test suite passes.
The finding lifecycle was asymmetric around the deterministic-resolve
gate: the gate correctly blocks LLM resolves of event/persistent
findings (backup, reliability, security, general) when the verifier
still detects the signal, but nothing ever cleared such a finding when
the underlying issue WAS fixed — absence-based stale auto-resolve only
covers performance/capacity, so a fixed backup stayed an active finding
indefinitely unless the LLM happened to call patrol_resolve_finding.
reconcileStaleFindings now runs the deterministic verifier for seeded,
unreported event/persistent findings whose key has one, and resolves
ONLY on an affirmative "signal gone" verification. Still-present or
inconclusive results fail closed (same standard as the resolve gate);
verifications are capped per run (3) with deferred candidates logged
and retried next run. Test seam: PatrolService.verifyFixResolvedFn.
Two enabling repairs found during verification:
- hasDeterministicVerifierForKey listed 2 of the 7 keys the
verifyFixDeterministically dispatch handles, so the LLM-resolve gate
silently skipped existing verifiers for backup-stale and
guest-unreachable findings. Now aligned; documented as the single
source of truth for both consumers.
- The finding-key vocabulary was forked: the patrol_report_finding tool
suggested keys (high-cpu, ...) the verifier switch never matched, so
verification rarely engaged for new findings. normalizeFindingKey now
aliases unambiguous directional synonyms onto the canonical verifier
vocabulary (high-cpu -> cpu-high etc.; pbs-job-failed and node-offline
deliberately NOT aliased — different resource models), and the tool
description teaches the canonical keys.
NOT changed: the synthetic ai-patrol-error finding — verification showed
the reported "accumulation" is a non-problem (deterministic ID, store
merges repeats as heartbeats, successful runs auto-resolve it).
Teeth: 8 new tests in patrol_reconcile_test.go including the
idempotence invariant (repeat reconcile over unchanged state produces
zero resolves and zero lifecycle growth) and the cap-defers case.
Contract: verified stale-resolve clause added beside the
deterministic-resolve-gate in ai-runtime.md. Full internal/ai tree green.
golangci-lint run ./... failed on ~190 pre-existing errcheck violations and
5 unformatted files, burying any new regression in noise. Fix all of them:
- Test files that hand-rolled mock-mode set/restore (vmware, truenas, and
friends) now use the canonical setMockModeForTest/testutil.SetMockMode
helper instead of drift copies that ignored SetEnabled errors.
- internal/mock and internal/monitoring tests get package-local
mustSetEnabled/mustSetMockEnabled/mustSetMonitorMockMode helpers that
fail the test on toggle errors.
- pkg/auth/sqlite_manager.go, pkg/metrics/store.go, pkg/server/server.go:
rollbacks in defers use the explicit-discard idiom, migration renames and
rollup commits log failures, the hosted reaper goroutine logs an error
exit, shutdown mock-disable logs failures.
- Remaining test sites check errors with t.Fatalf/t.Errorf or explicitly
discard best-effort calls (restore-chmods, handler-closure unmarshals)
per existing repo style.
- gofmt: internal/api/maintenance_verification.go, internal/ai/demo.go and
three findings test files.
Only dupl findings remain (44 pre-existing production-code duplication
pairs) — those need real refactors, not mechanical fixes.
Full test suites pass for every touched package.
Consolidates the OpenCode-restraint arc's user-visible promises into one
runnable corpus, mirroring the Discovery corpus pattern
(internal/servicediscovery/scenario_corpus_test.go). Each scenario
drives a full ExecuteStream turn against a scripted provider and pins
the browser-facing event stream: ordered/forbidden event types,
answer-text teeth, and payload teeth.
Six scenarios: clean plain-answer turn (done stamped with the model
route, no tool noise), greeting answered directly with tools offered
but unused, tool turn rendered as compact tool events with real tool
names and no provider call ids in the answer, clean no-narrative
fallback sentence (no raw JSON / call_ ids), invisible pre-event
provider retry (no error event), and exactly one clear error event on
terminal provider failure.
Teeth proven by mutation probes: a wrong ordered type and a wrong
answer assertion both fail with the promise named in the message.
Corpus documented in ai-runtime.md Current State as the canonical home
for interaction-quality regressions, so Patrol-phase churn on the
shared agentic loop cannot silently regress the chat feel.
The system prompt framed ask-first as the only safe behavior for a
missing target ("Missing target information is not a safe default...
ask for the missing target"), so any "run X" / "check Y" request
without an explicit host deflected back to the operator — including on
single-host deployments with no real ambiguity. OpenCode-parity gap:
a competent operator looks first.
New policy: resolve-before-asking. Use read-only query/topology tools
to identify plausible targets; if exactly one plausible target matches,
run read-only diagnostics against it and name it in the answer; ask
only when several plausible targets remain or the action changes state.
Unchanged safety: placeholder targets (current_resource outside an
attached-resource turn) stay forbidden in all modes, never guess an
unresolved target, and write actions still need an explicit target.
Contract: resolve-before-asking entries in Extension Points and Current
State supersede the ask-first framing. Proof:
TestBuildSystemPrompt_CurrentResourceRequiresResourceHandoff pins the
new boundary strings. Live "just run X" behavior check is owed to the
interaction-quality corpus (next unit); dev-instance agent exec is
currently broken (known command-token issue), so it cannot be observed
end-to-end from here today.
assistantToolScopeForPrompt classified prompt wording into text-only /
query-only / full tool scopes — the prompt-keyword router the ai-runtime
contract forbids. It withheld every tool from greetings and exact-reply
turns, zeroed the manifest on false-positive phrases ("before using any
tools, tell me your plan"), filtered inventory prompts to pulse_query
only, and its query-only path injected a prefetched topology payload
into the user message and then withheld tools entirely.
Now every interactive turn that reaches the selected model carries the
full governed manifest from toolsForExecutionMode; the model decides
whether to answer, ask, or call tools. Removed with the router: the
query-only topology prefetch + text-only downgrade, the inventory
sanitizer allow-list span, and the summary-only pulse_query input
rewrite (preferSummaryOnlyQueries) that mutated model-chosen tool
inputs. The two contract-sanctioned behaviors survive unchanged: the
context-only resource handoff manifest boundary, and the deterministic
count-only local answer (pulse:local-inventory), now gated directly by
assistantPromptQualifiesForLocalInventoryCount whose false positives
fail safe — to the model, never away from it.
Contract: superseded the query-only/direct-text scoping paragraphs in
ai-runtime.md with the model-owned manifest rule. Proofs:
TestService_ExecuteStream_ToolManifestIsModelOwned,
TestService_ExecuteStream_InventoryBreakdownIsModelOwned,
TestAssistantPromptQualifiesForLocalInventoryCount. Full internal/ai +
internal/api trees green; chat package lints clean.
An agent enrolled for metrics but whose token the server doesn't recognise (or
that lacks the agent:exec scope, or is bound to a different agent) was rejected
on the command-exec WebSocket with a bare 'Invalid token' and — for the
token-not-found case — no server log at all. The agent then retried forever,
logging only 'Invalid token', so the operator had no signal that discovery
deep-scan was failing or why. (Confirmed live: delly/minipc agents pointed at a
backend that didn't recognise their token retried thousands of times; discovery
abstained for every guest as a result.)
- agentexec/server.go: the registration-rejection message the agent logs
verbatim now says 'agent token not authorized for command execution — re-run
the agent installer to enroll an agent:exec-scoped token'.
- api/agent_exec_token_binding.go: the previously-silent token-not-recognised
branch now logs the specific reason with the agent hostname.
Contract-neutral: same rejection behaviour, just legible. Regression test:
TestHandleWebSocket_RejectionMessageIsActionable. Verified live end-to-end.
The Discovery tab stated the same updated_at timestamp twice: once in the
'Discovery run / Last run: X ago' control block and again as 'Last observed
X ago' on the identified-service card (and 'Last scanned: X ago' on the
not-identified state). Since every drawer that renders DiscoveryTab passes
showManualRunAction, the control block is always present, making the other two
pure duplicates.
Gate the service-card 'Last observed' and the not-identified 'Last scanned'
timestamps on !showManualRunAction(), so recency appears once — in the control
block when it's rendered, in place otherwise. The provenance attribution
('Observed by Discovery', 'Available to Pulse Assistant') and the cross-tab
drawer-header freshness banner are unchanged. Regression test added; verified
live (grafana drawer now shows 'Last run' once, no duplicate 'Last observed').
The metadata-only abstention is only reached when command scanning is already
enabled but the host agent returned no command output. The old reason text told
the user to 'Enable Pulse Commands' — a toggle that is provably already on when
this message appears — so the guidance was self-contradictory. The reason now
states commands are enabled and points at the actual gap: confirm the host
agent is connected and its API token has the agent:exec scope.
The Proxmox overview's shared search box is a VM/LXC filter, but filteredNodes
applied the same term to the nodes table independently. Searching a guest name
(e.g. 'debian-go') matched no node name, so the nodes table collapsed to the
'No Proxmox VE nodes' empty state — which reads as 'you have no Proxmox
infrastructure' even though the matching guest was listed right below it on its
host node.
Extract filterProxmoxNodesForSearch: keep a node when it matches the term
directly OR when it hosts a guest that matches the term. A guest search now
keeps that guest's host node visible for context; a node-name search still
narrows to the matching node; an empty term still shows every node. Regression
tests cover all three cases; verified live (searching 'debian-go' now shows the
minipc host node instead of the empty state).
When discovery has no in-guest command evidence, the backend abstains (empty
service, confidence 0) rather than confabulate a service, and explains why in
ai_reasoning (e.g. 'Discovery could not run commands on this resource... Enable
Pulse Commands (Settings → Infrastructure)...'). The Discovery tab discarded
that explanation: hasMeaningfulDiscoveryContext() ignores ai_reasoning, so an
abstention failed the render gate and fell into the generic 'Unknown Service /
Discovery completed but couldnt identify a known service' state with no reason
and no next step. After a 'Discovery complete' run, the user saw a dead-end.
The not-identified branch now surfaces ai_reasoning (the actionable
explanation) and a clickable 'Open Settings → Infrastructure' link when command
execution isn't confirmed enabled, with a neutral 'Service not identified'
heading. Identified guests are unaffected (they render via the valid-discovery
block). Regression test added; verified live against a mock guest that abstains.
The install-time auto-register path (auto_register_pve_node) parsed the API
token secret out of pveum's box-drawing table output with a fragile awk
column-split. The web-setup render path was already hardened to request
'pveum ... --output-format json' first and parse the value field, but this
secondary install.sh path was never ported.
auto_register_pve_node now requests --output-format json first (falling back
to the bare --privsep 1 form only when an older pveum rejects the JSON flag,
which keeps the secure-installer contract pin on that form satisfied) and
extracts the secret via a new extract_pve_token_value helper: JSON value-field
parse first, then a locale-independent box-drawing table fallback (normalizes
the column separator to a plain pipe byte-wise before splitting, so it works
regardless of host locale). This mirrors the hardened render path and removes
the silent-failure / mis-parse risk when pveum table formatting drifts.
Functional + contract tests in root_install_sh_test.go; deployment-installability
contract documents the deterministic extraction. The host-agent path
(internal/hostagent/proxmox_setup.go setupPVEToken) carries an agent-lifecycle
token-permission proof obligation and is left for a governed lane.
A Proxmox host upgraded from v5 may still carry the legacy pulse-sensor-proxy
footprint (binary, systemd units, runtime/state dirs, dedicated service user,
and managed SSH keys in root's authorized_keys). install.sh --uninstall removed
everything for the Pulse server itself but left that legacy footprint behind,
so a 'complete uninstall' was not complete -- most notably it left SSH key
entries in /root/.ssh/authorized_keys.
uninstall_pulse now calls cleanup_local_sensor_proxy, which removes the LOCAL
footprint only: stop/disable the units, remove the binary/units/runtime/state
dirs, strip the '# pulse-managed-key' / '# pulse-proxy-key' entries, and remove
the service user/group. It is presence-gated (silent no-op when no proxy was
installed). The aggressive cluster-wide authorized_keys removal and
pulse-monitor@pam API-user deletion stay behind the explicit standalone
scripts/uninstall-sensor-proxy.sh, which we print a pointer to.
Functional + contract tests in scripts/installtests/root_install_sh_test.go;
deployment-installability contract documents the new uninstall removal scope.
Finishes the deferred half of the v5->v6 parity fix for #1323 (the
pulse-auto-update.sh half landed in 672e81985). The interactive install.sh
update/reinstall flow stopped a running Pulse then called start_pulse, which
tolerates a silent start failure (common on unprivileged LXC) by printing a note
and returning 0 — leaving Pulse stopped under an "installation completed!" message.
- stop_pulse_for_update records whether Pulse was running before the update.
- start_pulse, only when Pulse was running before (PULSE_WAS_ACTIVE), no longer
accepts a silent start failure: it verifies the service became active
(wait_for_service_active, 20s), retries one explicit start, and surfaces a clear
error + diagnostics if it still will not come up. Fresh installs are unchanged
(the flag stays false, so the reassuring container note is kept).
- Wired into all three update/reinstall sites; added a BASH_SOURCE guard so the
installer's functions can be unit-tested without running the installer.
Scope: fixes the #1323 'restart silently failed' case. Does NOT add a binary
rollback (download_pulse deletes bin/pulse.old right after the swap) — a
bad-release rollback is a separate concern.
Test: scripts/tests/test-install-update-resilience.sh (sources install.sh, stubs
systemctl, asserts was-active capture + retry + clear error). Go installtests +
bash -n confirm the guard does not change installer execution.
Discovery records for Proxmox guests (VM / system-container) are canonically
keyed by node name + VMID — the form the background fingerprint loop and the
Assistant prefetch already use. But the browser action path addresses guests by
the linked agent UUID (discoveryTarget.agentId, the action-authorization target),
and normalizeDiscoveryRequest was a no-op for guest types — so a UUID-targeted
trigger/lookup stored and read records under a second, divergent key. Result: the
resource drawer reported "Not discovered" for a guest the background loop had
already discovered, and duplicate records accumulated (node-keyed + UUID-keyed).
normalizeDiscoveryRequest now canonicalizes guest targets to the node name:
resolve a linked-agent-UUID target back to its hosting node (node->linked-agent
map, falling back to the agent host's hostname when it is a known node). Both
creation (DiscoverResource) and lookup (GetDiscoveryByResource) funnel through
this chokepoint, so every path converges on the node-name key; the caller already
aliases the original target, so pre-existing UUID-keyed records are still found
and consolidated onto the canonical key on the next run.
The agent UUID is unchanged as the action-authorization target — this governs
only the record key, so the discoveryTarget security contract is untouched.
- Test: TestService_ProxmoxGuestDiscoveryCanonicalizesToNodeKey.
- Live-verified on the dev instance: the Home Assistant LXC drawer readiness went
missing -> fresh (discoveryId system-container:delly:101).
Extend the cloud-context e2e test to stamp the discovery with an UpdatedAt and
assert "Last discovered: 2 days ago" reaches the model on both the handoff and
@-mention paths — proving freshness survives the full ExecuteStream path (cloud
policy, sanitizer allow-list, prompt injection), not just the formatter unit.
Test-only.
formatSingleDiscovery (the local/full path, FormatForAIContext) reported a
resource's service, access, paths, and ports but not how old the discovery was —
so Ollama/local users lacked the recency signal cloud users just got. Emit a
"Last Discovered: <age>" header fact when the timestamp is known, completing
freshness parity across the cloud-safe and full paths. Omitted when unknown.
Test: TestFormatSingleDiscovery_IncludesFreshness (present when set, absent when
zero).
The model now receives provenance — per-fact source/confidence and discovery
freshness — but nothing told it to use it, so answers stated facts the user
couldn't trace or weigh. Add a GROUNDING & PROVENANCE section to the base system
prompt: briefly attribute facts to their source ("Debian 12, per
/etc/os-release"), note recency for time-sensitive claims, do not present stale
context as current, and keep attribution concise rather than citing every line.
This is the prose-attribution layer of the provenance work (the visible part
chosen as the first, lowest-risk step over a heavier citations UI). Whether the
model attributes well in practice is a live-behavior question to verify by
exercising the Assistant — the prompt only guarantees the instruction reaches it.
- Test: TestBuildSystemPrompt_IncludesProvenanceGuidance.
- Contract: ai-runtime base prompt must instruct provenance attribution.
rcourtman