Cover eval scenario catalog, control-plane printers and pure helpers

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

cmd/pulse-control-plane/branchcov0723pm_test.go

@@ -0,0 +1,696 @@
+package main
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/cloudcp"
+	cpDocker "github.com/rcourtman/pulse-go-rewrite/internal/cloudcp/docker"
+	"github.com/rcourtman/pulse-go-rewrite/internal/cloudcp/registry"
+)
+
+// This file adds branch-coverage for the stdout formatter helpers in this
+// package. Every target is a pure printer over an in-memory struct, so they
+// can be exercised directly with no network, SSH, daemon or database. Output is
+// captured by reusing the package's existing os.Stdout redirect helper
+// (captureStdoutForProviderMSPRecoverTest) verbatim; the thin wrappers below
+// only tidy the call site and route every returned error through t.Helper.
+
+func capturePrint0723pm(t *testing.T, fn func()) string {
+	t.Helper()
+	var buf bytes.Buffer
+	restore := captureStdoutForProviderMSPRecoverTest(t, &buf)
+	fn()
+	restore()
+	return buf.String()
+}
+
+func assertContains0723pm(t *testing.T, out, want string) {
+	t.Helper()
+	if !strings.Contains(out, want) {
+		t.Fatalf("output missing %q:\n%s", want, out)
+	}
+}
+
+func assertNotContains0723pm(t *testing.T, out, want string) {
+	t.Helper()
+	if strings.Contains(out, want) {
+		t.Fatalf("output unexpectedly contains %q:\n%s", want, out)
+	}
+}
+
+func mustIndexBefore0723pm(t *testing.T, out, first, second string) {
+	t.Helper()
+	i := strings.Index(out, first)
+	j := strings.Index(out, second)
+	if i < 0 || j < 0 {
+		t.Fatalf("missing substring for ordering check (%q=%d, %q=%d):\n%s", first, i, second, j, out)
+	}
+	if i >= j {
+		t.Fatalf("%q (idx %d) not before %q (idx %d):\n%s", first, i, second, j, out)
+	}
+}
+
+// TestBranchcov0723pmPrintTenantRuntimeReconcilePlan covers every branch of
+// printTenantRuntimeReconcilePlan: the nil-plan early return, the empty-tenants
+// path, each arm of the action switch (rollout/noop/default), nil item skipping,
+// and every optional-field present/absent gate.
+func TestBranchcov0723pmPrintTenantRuntimeReconcilePlan(t *testing.T) {
+	t.Run("nil plan prints total=0 and no rollup counters", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() { printTenantRuntimeReconcilePlan(nil) })
+		assertContains0723pm(t, out, "summary_total=0\n")
+		assertNotContains0723pm(t, out, "summary_rollout=")
+		assertNotContains0723pm(t, out, "tenant_id=")
+	})
+
+	t.Run("non-nil empty plan prints zero counters", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() {
+			printTenantRuntimeReconcilePlan(&cloudcp.TenantRuntimeContractReconcilePlan{})
+		})
+		assertContains0723pm(t, out, "summary_rollout=0\nsummary_noop=0\nsummary_skip=0\nsummary_total=0\n")
+		assertNotContains0723pm(t, out, "tenant_id=")
+	})
+
+	t.Run("item with all optional fields populated prints them", func(t *testing.T) {
+		plan := &cloudcp.TenantRuntimeContractReconcilePlan{
+			Tenants: []*cloudcp.TenantRuntimeContractReconcilePlanItem{
+				{
+					TenantID: "t-ROLL", Action: "rollout", Reason: "image drift",
+					LiveContainerID:  "cid-roll",
+					ImageRef:         "img:v2",
+					LiveRouteHost:    "live.example.com",
+					DesiredRouteHost: "desired.example.com",
+					LivePublicURL:    "https://live.example.com",
+					DesiredPublicURL: "https://desired.example.com",
+				},
+			},
+		}
+		out := capturePrint0723pm(t, func() { printTenantRuntimeReconcilePlan(plan) })
+		for _, w := range []string{
+			"tenant_id=t-ROLL",
+			"action=rollout",
+			"live_container_id=cid-roll",
+			"image_ref=img:v2",
+			"live_route_host=live.example.com",
+			"desired_route_host=desired.example.com",
+			"live_public_url=https://live.example.com",
+			"desired_public_url=https://desired.example.com",
+			"summary_rollout=1\n",
+		} {
+			assertContains0723pm(t, out, w)
+		}
+		assertNotContains0723pm(t, out, "summary_noop=1")
+		assertNotContains0723pm(t, out, "summary_skip=1")
+	})
+
+	t.Run("item with all optional fields empty omits them", func(t *testing.T) {
+		plan := &cloudcp.TenantRuntimeContractReconcilePlan{
+			Tenants: []*cloudcp.TenantRuntimeContractReconcilePlanItem{
+				{TenantID: "t-NOOPT", Action: "noop", Reason: "no drift"},
+			},
+		}
+		out := capturePrint0723pm(t, func() { printTenantRuntimeReconcilePlan(plan) })
+		assertContains0723pm(t, out, "tenant_id=t-NOOPT")
+		assertContains0723pm(t, out, "action=noop")
+		assertContains0723pm(t, out, "summary_noop=1\n")
+		for _, absent := range []string{
+			"live_container_id=",
+			"image_ref=",
+			"live_route_host=",
+			"desired_route_host=",
+			"live_public_url=",
+			"desired_public_url=",
+		} {
+			assertNotContains0723pm(t, out, absent)
+		}
+	})
+
+	t.Run("action switch default arm + nil item counted in total but skipped", func(t *testing.T) {
+		// 4 entries: one nil (skipped before the switch and before printing),
+		// one rollout, one noop, one unknown action (falls through to default ->
+		// skip). summary_total uses len(plan.Tenants) which is 4, while only 3
+		// reach the switch.
+		plan := &cloudcp.TenantRuntimeContractReconcilePlan{
+			Tenants: []*cloudcp.TenantRuntimeContractReconcilePlanItem{
+				nil,
+				{TenantID: "t-R", Action: "rollout", Reason: "r"},
+				{TenantID: "t-N", Action: "noop", Reason: "n"},
+				{TenantID: "t-S", Action: "skip", Reason: "s"},
+			},
+		}
+		out := capturePrint0723pm(t, func() { printTenantRuntimeReconcilePlan(plan) })
+		assertContains0723pm(t, out, "summary_rollout=1\nsummary_noop=1\nsummary_skip=1\nsummary_total=4\n")
+		// All three non-nil tenants are printed; the nil entry never emits a block.
+		assertContains0723pm(t, out, "tenant_id=t-R\n")
+		assertContains0723pm(t, out, "tenant_id=t-N\n")
+		assertContains0723pm(t, out, "tenant_id=t-S\n")
+		assertContains0723pm(t, out, "action=skip")
+	})
+}
+
+// TestBranchcov0723pmPrintTenantRuntimeImageRolloutPlan covers every branch of
+// printTenantRuntimeImageRolloutPlan. It is structurally a sibling of the
+// reconcile printer but carries a different optional-field set (State,
+// LiveImageRef, TargetImageRef), so each gate is pinned independently.
+func TestBranchcov0723pmPrintTenantRuntimeImageRolloutPlan(t *testing.T) {
+	t.Run("nil plan prints total=0 and no rollup counters", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() { printTenantRuntimeImageRolloutPlan(nil) })
+		assertContains0723pm(t, out, "summary_total=0\n")
+		assertNotContains0723pm(t, out, "summary_rollout=")
+		assertNotContains0723pm(t, out, "tenant_id=")
+	})
+
+	t.Run("non-nil empty plan prints zero counters", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() {
+			printTenantRuntimeImageRolloutPlan(&cloudcp.TenantRuntimeImageRolloutPlan{})
+		})
+		assertContains0723pm(t, out, "summary_rollout=0\nsummary_noop=0\nsummary_skip=0\nsummary_total=0\n")
+	})
+
+	t.Run("item with all optional fields populated prints them", func(t *testing.T) {
+		plan := &cloudcp.TenantRuntimeImageRolloutPlan{
+			Tenants: []*cloudcp.TenantRuntimeImageRolloutPlanItem{
+				{
+					TenantID: "t-ROLL", Action: "rollout", Reason: "target image differs",
+					State:            "active",
+					LiveContainerID:  "cid-roll",
+					LiveImageRef:     "img:v1",
+					TargetImageRef:   "img:v2",
+					LiveRouteHost:    "live.example.com",
+					DesiredRouteHost: "desired.example.com",
+					LivePublicURL:    "https://live.example.com",
+					DesiredPublicURL: "https://desired.example.com",
+				},
+			},
+		}
+		out := capturePrint0723pm(t, func() { printTenantRuntimeImageRolloutPlan(plan) })
+		for _, w := range []string{
+			"tenant_id=t-ROLL",
+			"tenant_state=active",
+			"live_container_id=cid-roll",
+			"live_image_ref=img:v1",
+			"target_image_ref=img:v2",
+			"live_route_host=live.example.com",
+			"desired_route_host=desired.example.com",
+			"live_public_url=https://live.example.com",
+			"desired_public_url=https://desired.example.com",
+			"summary_rollout=1\n",
+		} {
+			assertContains0723pm(t, out, w)
+		}
+	})
+
+	t.Run("item with all optional fields empty omits them", func(t *testing.T) {
+		plan := &cloudcp.TenantRuntimeImageRolloutPlan{
+			Tenants: []*cloudcp.TenantRuntimeImageRolloutPlanItem{
+				{TenantID: "t-NOOPT", Action: "noop", Reason: "same image"},
+			},
+		}
+		out := capturePrint0723pm(t, func() { printTenantRuntimeImageRolloutPlan(plan) })
+		assertContains0723pm(t, out, "tenant_id=t-NOOPT")
+		assertContains0723pm(t, out, "summary_noop=1\n")
+		for _, absent := range []string{
+			"tenant_state=",
+			"live_container_id=",
+			"live_image_ref=",
+			"target_image_ref=",
+			"live_route_host=",
+			"desired_route_host=",
+			"live_public_url=",
+			"desired_public_url=",
+		} {
+			assertNotContains0723pm(t, out, absent)
+		}
+	})
+
+	t.Run("action switch default arm + nil item counted in total but skipped", func(t *testing.T) {
+		plan := &cloudcp.TenantRuntimeImageRolloutPlan{
+			Tenants: []*cloudcp.TenantRuntimeImageRolloutPlanItem{
+				nil,
+				{TenantID: "t-R", Action: "rollout", Reason: "r"},
+				{TenantID: "t-N", Action: "noop", Reason: "n"},
+				{TenantID: "t-S", Action: "pause", Reason: "s"},
+			},
+		}
+		out := capturePrint0723pm(t, func() { printTenantRuntimeImageRolloutPlan(plan) })
+		assertContains0723pm(t, out, "summary_rollout=1\nsummary_noop=1\nsummary_skip=1\nsummary_total=4\n")
+		assertContains0723pm(t, out, "action=pause")
+	})
+}
+
+// TestBranchcov0723pmPrintCloudAuditReport covers the audit report printer,
+// which has the richest branch set: nil report, the fixed per-state count loop
+// (with a nil map), the conditional docker_unavailable / storage section, the
+// storage filesystem status (ok/fail + error gate), the build-cache status
+// (ok/fail + error gate), the stale-proof loops, the orphan-entitlement loop,
+// the unhealthy-container filter, and the failures loop.
+func TestBranchcov0723pmPrintCloudAuditReport(t *testing.T) {
+	t.Run("nil report prints audit_ok=false and nothing else", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() { printCloudAuditReport(nil) })
+		assertContains0723pm(t, out, "audit_ok=false\n")
+		assertNotContains0723pm(t, out, "audit_ok=true")
+		assertNotContains0723pm(t, out, "tenant_total=")
+		assertNotContains0723pm(t, out, "docker_managed_total=")
+	})
+
+	t.Run("minimal report prints zero counts and omits optional sections", func(t *testing.T) {
+		report := &cloudcp.CloudAuditReport{OK: true}
+		out := capturePrint0723pm(t, func() { printCloudAuditReport(report) })
+		assertContains0723pm(t, out, "audit_ok=true\n")
+		assertContains0723pm(t, out, "tenant_total=0\n")
+		// The per-state loop always emits, even against a nil map (zero value).
+		assertContains0723pm(t, out, "tenant_provisioning=0\n")
+		assertContains0723pm(t, out, "tenant_active=0\n")
+		assertContains0723pm(t, out, "tenant_failed=0\n")
+		assertContains0723pm(t, out, "docker_managed_total=0\n")
+		assertContains0723pm(t, out, "proof_tenant_stale_count=0\n")
+		assertContains0723pm(t, out, "proof_account_stale_count=0\n")
+		assertContains0723pm(t, out, "hosted_paid_orphan_entitlement_count=0\n")
+		// Optional sections absent.
+		assertNotContains0723pm(t, out, "docker_unavailable=")
+		assertNotContains0723pm(t, out, "storage_guardrails_enabled=")
+		assertNotContains0723pm(t, out, "docker_build_cache_status=")
+		assertNotContains0723pm(t, out, "docker_unhealthy_container=")
+		assertNotContains0723pm(t, out, "failure=")
+	})
+
+	t.Run("non-zero state counts and docker_unavailable are emitted", func(t *testing.T) {
+		report := &cloudcp.CloudAuditReport{
+			OK:                false,
+			TenantTotal:       5,
+			DockerUnavailable: "daemon unreachable",
+			TenantCounts: map[registry.TenantState]int{
+				registry.TenantStateActive:       3,
+				registry.TenantStateProvisioning: 2,
+			},
+			RegistryUnhealthyActive: 1,
+			DockerManagedTotal:      4,
+			DockerManagedRunning:    3,
+			DockerManagedUnhealthy:  1,
+		}
+		out := capturePrint0723pm(t, func() { printCloudAuditReport(report) })
+		assertContains0723pm(t, out, "audit_ok=false\n")
+		assertContains0723pm(t, out, "tenant_total=5\n")
+		assertContains0723pm(t, out, "tenant_active=3\n")
+		assertContains0723pm(t, out, "tenant_provisioning=2\n")
+		// States absent from the map still print as zero.
+		assertContains0723pm(t, out, "tenant_suspended=0\n")
+		assertContains0723pm(t, out, "tenant_canceled=0\n")
+		assertContains0723pm(t, out, "tenant_deleting=0\n")
+		assertContains0723pm(t, out, "tenant_deleted=0\n")
+		assertContains0723pm(t, out, "tenant_failed=0\n")
+		assertContains0723pm(t, out, "tenant_registry_unhealthy_active=1\n")
+		assertContains0723pm(t, out, "docker_managed_total=4\n")
+		assertContains0723pm(t, out, "docker_managed_running=3\n")
+		assertContains0723pm(t, out, "docker_managed_unhealthy=1\n")
+		assertContains0723pm(t, out, "docker_unavailable=daemon unreachable\n")
+	})
+
+	t.Run("storage ok filesystem emits status=ok and no error line", func(t *testing.T) {
+		report := &cloudcp.CloudAuditReport{
+			Storage: &cloudcp.StorageGuardrailReport{
+				Enabled: true,
+				OK:      true,
+				Filesystems: []cloudcp.StorageFilesystemReport{
+					{
+						Name: "data", Path: "/data",
+						AvailableBytes: 1000, MinAvailableBytes: 500, TotalBytes: 2000,
+						OK: true,
+					},
+				},
+				BuildCache: cloudcp.StorageBuildCacheReport{
+					TotalBytes: 1000, MaxBytes: 5000, ReclaimableBytes: 800, OK: true,
+				},
+			},
+		}
+		out := capturePrint0723pm(t, func() { printCloudAuditReport(report) })
+		assertContains0723pm(t, out, "storage_guardrails_enabled=true\n")
+		assertContains0723pm(t, out, "storage_ok=true\n")
+		assertContains0723pm(t, out, "storage_data_status=ok\n")
+		assertContains0723pm(t, out, "storage_data_path=/data\n")
+		assertContains0723pm(t, out, "storage_data_available_bytes=1000\n")
+		assertContains0723pm(t, out, "storage_data_min_available_bytes=500\n")
+		assertContains0723pm(t, out, "storage_data_total_bytes=2000\n")
+		assertNotContains0723pm(t, out, "storage_data_error=")
+		assertContains0723pm(t, out, "docker_build_cache_status=ok\n")
+		assertContains0723pm(t, out, "docker_build_cache_total_bytes=1000\n")
+		assertContains0723pm(t, out, "docker_build_cache_max_bytes=5000\n")
+		assertContains0723pm(t, out, "docker_build_cache_reclaimable_bytes=800\n")
+		assertNotContains0723pm(t, out, "docker_build_cache_error=")
+	})
+
+	t.Run("storage fail filesystem and build-cache emit status=fail plus error lines", func(t *testing.T) {
+		report := &cloudcp.CloudAuditReport{
+			Storage: &cloudcp.StorageGuardrailReport{
+				Enabled: true,
+				OK:      false,
+				Filesystems: []cloudcp.StorageFilesystemReport{
+					{
+						Name: "docker", Path: "/var/lib/docker",
+						AvailableBytes: 100, MinAvailableBytes: 500, TotalBytes: 2000,
+						OK: false, Error: "stat failed",
+					},
+				},
+				BuildCache: cloudcp.StorageBuildCacheReport{
+					TotalBytes: 6000, MaxBytes: 5000, ReclaimableBytes: 800,
+					OK: false, Error: "build cache over limit",
+				},
+			},
+		}
+		out := capturePrint0723pm(t, func() { printCloudAuditReport(report) })
+		assertContains0723pm(t, out, "storage_ok=false\n")
+		assertContains0723pm(t, out, "storage_docker_status=fail\n")
+		assertContains0723pm(t, out, "storage_docker_error=stat failed\n")
+		assertContains0723pm(t, out, "docker_build_cache_status=fail\n")
+		assertContains0723pm(t, out, "docker_build_cache_error=build cache over limit\n")
+	})
+
+	t.Run("stale proof tenants, accounts, orphans and failures are printed", func(t *testing.T) {
+		report := &cloudcp.CloudAuditReport{
+			OK: false,
+			StaleProofTenants: []cloudcp.ProofTenantAuditItem{
+				{
+					TenantID: "t-STALE", State: registry.TenantStateProvisioning,
+					AccountID: "acc-1", Email: "canary@example.com",
+					Age: 90 * time.Second,
+				},
+			},
+			StaleProofAccounts: []cloudcp.ProofAccountAuditItem{
+				{
+					AccountID: "acc-STALE", Kind: registry.AccountKindMSP,
+					Age: 120 * time.Second,
+				},
+			},
+			OrphanPaidHostedEntitlements: []cloudcp.HostedEntitlementAuditItem{
+				{EntitlementID: "ent-1", TenantID: "t-MISSING", Kind: registry.HostedEntitlementKindPaid},
+			},
+			Failures: []string{"audit failed"},
+		}
+		out := capturePrint0723pm(t, func() { printCloudAuditReport(report) })
+		assertContains0723pm(t, out, "proof_tenant_stale_count=1\n")
+		assertContains0723pm(t, out, "proof_tenant_stale=t-STALE state=provisioning account_id=acc-1 email=canary@example.com age=1m30s\n")
+		assertContains0723pm(t, out, "proof_account_stale_count=1\n")
+		assertContains0723pm(t, out, "proof_account_stale=acc-STALE kind="+string(registry.AccountKindMSP)+" age=2m0s\n")
+		assertContains0723pm(t, out, "hosted_paid_orphan_entitlement_count=1\n")
+		assertContains0723pm(t, out, "hosted_paid_orphan_entitlement=ent-1 tenant_id=t-MISSING kind="+string(registry.HostedEntitlementKindPaid)+"\n")
+		assertContains0723pm(t, out, "failure=audit failed\n")
+	})
+
+	t.Run("unhealthy container filter skips healthy/none/empty and prints the rest", func(t *testing.T) {
+		// running+healthy, running+none and running+empty-health are all skipped;
+		// running+unhealthy and any non-running state are printed.
+		report := &cloudcp.CloudAuditReport{
+			ManagedRuntimeContainers: []cpDocker.RuntimeContainerSummary{
+				{ID: "c-HEALTHY", Name: "n1", State: "running", HealthStatus: "healthy", Status: "Up"},
+				{ID: "c-NONE", Name: "n2", State: "running", HealthStatus: "none", Status: "Up"},
+				{ID: "c-EMPTY", Name: "n3", State: "running", HealthStatus: "", Status: "Up"},
+				{ID: "c-UNHEALTHY", Name: "n4", State: "running", HealthStatus: "unhealthy", Status: "Up"},
+				{ID: "c-EXITED", Name: "n5", State: "exited", HealthStatus: "healthy", Status: "Exited"},
+			},
+		}
+		out := capturePrint0723pm(t, func() { printCloudAuditReport(report) })
+		assertContains0723pm(t, out, "docker_unhealthy_container=c-UNHEALTHY name=n4 state=running health=unhealthy status=Up\n")
+		assertContains0723pm(t, out, "docker_unhealthy_container=c-EXITED name=n5 state=exited health=healthy status=Exited\n")
+		assertNotContains0723pm(t, out, "docker_unhealthy_container=c-HEALTHY")
+		assertNotContains0723pm(t, out, "docker_unhealthy_container=c-NONE")
+		assertNotContains0723pm(t, out, "docker_unhealthy_container=c-EMPTY")
+	})
+}
+
+// TestBranchcov0723pmPrintProviderMSPPortalLinkResult covers the portal-link
+// printer. Its only branch is the nil guard; the populated arm is unconditional.
+func TestBranchcov0723pmPrintProviderMSPPortalLinkResult(t *testing.T) {
+	t.Run("nil result prints ok=false and no fields", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() { printProviderMSPPortalLinkResult(nil) })
+		assertContains0723pm(t, out, "provider_msp_portal_link_ok=false\n")
+		assertNotContains0723pm(t, out, "provider_msp_portal_link_ok=true")
+		assertNotContains0723pm(t, out, "email=")
+	})
+
+	t.Run("populated result prints ok=true and all fields", func(t *testing.T) {
+		result := &cloudcp.ProviderMSPPortalLinkResult{
+			Email:        "ops@example.com",
+			AccessState:  "active",
+			Role:         "owner",
+			MagicLinkURL: "https://msp.example.com/magic/abc",
+		}
+		out := capturePrint0723pm(t, func() { printProviderMSPPortalLinkResult(result) })
+		assertContains0723pm(t, out, "provider_msp_portal_link_ok=true\n")
+		assertContains0723pm(t, out, "email=ops@example.com\n")
+		assertContains0723pm(t, out, "access_state=active\n")
+		assertContains0723pm(t, out, "role=owner\n")
+		assertContains0723pm(t, out, "portal_magic_link=https://msp.example.com/magic/abc\n")
+	})
+}
+
+// TestBranchcov0723pmPrintProviderMSPBootstrapResult covers the bootstrap
+// printer: nil guard plus the conditional MagicLinkURL gate.
+func TestBranchcov0723pmPrintProviderMSPBootstrapResult(t *testing.T) {
+	t.Run("nil result prints ok=false and no fields", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() { printProviderMSPBootstrapResult(nil) })
+		assertContains0723pm(t, out, "provider_msp_bootstrap_ok=false\n")
+		assertNotContains0723pm(t, out, "provider_msp_bootstrap_ok=true")
+		assertNotContains0723pm(t, out, "account_id=")
+	})
+
+	t.Run("populated result without magic link omits portal_magic_link", func(t *testing.T) {
+		result := &cloudcp.ProviderMSPBootstrapResult{
+			AccountID: "acc-1", AccountName: "Acme MSP",
+			OwnerUserID: "user-1", OwnerEmail: "ops@example.com",
+			PlanVersion: "msp_growth", PlanSource: "license-file",
+			LicenseID: "lic-1", LicenseEmail: "ops@example.com",
+			WorkspaceLimit: 15,
+		}
+		out := capturePrint0723pm(t, func() { printProviderMSPBootstrapResult(result) })
+		assertContains0723pm(t, out, "provider_msp_bootstrap_ok=true\n")
+		assertContains0723pm(t, out, "account_id=acc-1\n")
+		assertContains0723pm(t, out, "account_name=Acme MSP\n")
+		assertContains0723pm(t, out, "owner_user_id=user-1\n")
+		assertContains0723pm(t, out, "owner_email=ops@example.com\n")
+		assertContains0723pm(t, out, "plan_version=msp_growth\n")
+		assertContains0723pm(t, out, "plan_source=license-file\n")
+		assertContains0723pm(t, out, "license_id=lic-1\n")
+		assertContains0723pm(t, out, "license_email=ops@example.com\n")
+		assertContains0723pm(t, out, "workspace_limit=15\n")
+		assertNotContains0723pm(t, out, "portal_magic_link=")
+	})
+
+	t.Run("populated result with magic link prints it", func(t *testing.T) {
+		result := &cloudcp.ProviderMSPBootstrapResult{
+			AccountID: "acc-1", MagicLinkURL: "https://msp.example.com/magic/xyz",
+		}
+		out := capturePrint0723pm(t, func() { printProviderMSPBootstrapResult(result) })
+		assertContains0723pm(t, out, "portal_magic_link=https://msp.example.com/magic/xyz\n")
+	})
+}
+
+// TestBranchcov0723pmPrintProviderMSPBackupCreateResult covers the create
+// printer: nil guard plus the populated arm that delegates to the manifest
+// printer (verified by a single manifest line) and emits entry counts.
+func TestBranchcov0723pmPrintProviderMSPBackupCreateResult(t *testing.T) {
+	t.Run("nil result prints created=false", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupCreateResult(nil) })
+		assertContains0723pm(t, out, "provider_msp_backup_created=false\n")
+		assertNotContains0723pm(t, out, "provider_msp_backup_created=true")
+		assertNotContains0723pm(t, out, "archive_path=")
+	})
+
+	t.Run("populated result prints created=true, manifest and counts", func(t *testing.T) {
+		result := &cloudcp.ProviderMSPBackupCreateResult{
+			ArchivePath:         "/data/backups/provider-msp/latest.tar.gz",
+			BytesWritten:        4096,
+			ControlPlaneEntries: 3,
+			TenantEntries:       5,
+			LicenseEntries:      1,
+			Manifest: cloudcp.ProviderMSPBackupManifest{
+				Version:          cloudcp.ProviderMSPBackupManifestVersion,
+				ControlPlaneMode: string(cloudcp.ControlPlaneModeProviderHostedMSP),
+			},
+		}
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupCreateResult(result) })
+		assertContains0723pm(t, out, "provider_msp_backup_created=true\n")
+		assertContains0723pm(t, out, "archive_path=/data/backups/provider-msp/latest.tar.gz\n")
+		assertContains0723pm(t, out, "archive_bytes=4096\n")
+		assertContains0723pm(t, out, "control_plane_entries=3\n")
+		assertContains0723pm(t, out, "tenant_entries=5\n")
+		assertContains0723pm(t, out, "license_entries=1\n")
+		// Delegation to the manifest printer happened.
+		assertContains0723pm(t, out, "manifest_version="+cloudcp.ProviderMSPBackupManifestVersion+"\n")
+	})
+}
+
+// TestBranchcov0723pmPrintProviderMSPBackupRestoreResult covers the restore
+// printer. Its notable branch is provider_msp_backup_restored being the negation
+// of DryRun, plus the nil guard and the RestoredRuntimeTenantIDs loop.
+func TestBranchcov0723pmPrintProviderMSPBackupRestoreResult(t *testing.T) {
+	t.Run("nil result prints restored=false", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupRestoreResult(nil) })
+		assertContains0723pm(t, out, "provider_msp_backup_restored=false\n")
+		assertNotContains0723pm(t, out, "archive_path=")
+	})
+
+	t.Run("dry run prints restored=false and dry_run=true", func(t *testing.T) {
+		result := &cloudcp.ProviderMSPBackupRestoreResult{
+			DryRun:               true,
+			ArchivePath:          "/b/a.tar.gz",
+			TargetDataDir:        "/data",
+			ControlPlaneDir:      "/data/control-plane",
+			TenantsDir:           "/data/tenants",
+			LicenseOutputPath:    "/data/provider-msp-license.jwt",
+			ReplaceExisting:      true,
+			VerifiedArchiveBytes: 2048,
+		}
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupRestoreResult(result) })
+		// restored is !DryRun -> false even though result is non-nil.
+		assertContains0723pm(t, out, "provider_msp_backup_restored=false\n")
+		assertNotContains0723pm(t, out, "provider_msp_backup_restored=true")
+		assertContains0723pm(t, out, "provider_msp_backup_restore_dry_run=true\n")
+		assertContains0723pm(t, out, "replace_existing=true\n")
+		assertContains0723pm(t, out, "archive_bytes=2048\n")
+	})
+
+	t.Run("real restore prints restored=true and runtime tenant ids", func(t *testing.T) {
+		result := &cloudcp.ProviderMSPBackupRestoreResult{
+			DryRun:                      false,
+			ArchivePath:                 "/b/a.tar.gz",
+			TargetDataDir:               "/data",
+			ControlPlaneDir:             "/data/control-plane",
+			TenantsDir:                  "/data/tenants",
+			LicenseOutputPath:           "/data/provider-msp-license.jwt",
+			ReplaceExisting:             false,
+			VerifiedArchiveBytes:        4096,
+			ControlPlaneEntriesRestored: 3,
+			TenantEntriesRestored:       5,
+			LicenseEntriesRestored:      1,
+			RestoredRegistryTenantCount: 2,
+			RestoredRuntimeTenantIDs:    []string{"t-1", "t-2"},
+		}
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupRestoreResult(result) })
+		assertContains0723pm(t, out, "provider_msp_backup_restored=true\n")
+		assertContains0723pm(t, out, "provider_msp_backup_restore_dry_run=false\n")
+		assertContains0723pm(t, out, "control_plane_entries_restored=3\n")
+		assertContains0723pm(t, out, "tenant_entries_restored=5\n")
+		assertContains0723pm(t, out, "license_entries_restored=1\n")
+		assertContains0723pm(t, out, "restored_registry_tenant_count=2\n")
+		// Both runtime ids printed, in order.
+		assertContains0723pm(t, out, "restored_runtime_tenant_id=t-1\n")
+		assertContains0723pm(t, out, "restored_runtime_tenant_id=t-2\n")
+		mustIndexBefore0723pm(t, out, "restored_runtime_tenant_id=t-1", "restored_runtime_tenant_id=t-2")
+	})
+
+	t.Run("empty restored runtime tenant list omits the loop line", func(t *testing.T) {
+		result := &cloudcp.ProviderMSPBackupRestoreResult{DryRun: false}
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupRestoreResult(result) })
+		assertNotContains0723pm(t, out, "restored_runtime_tenant_id=")
+	})
+}
+
+// TestBranchcov0723pmPrintProviderMSPBackupVerifyResult covers the verify
+// printer: nil guard plus the ControlPlaneDBFiles and RuntimeTenantDirs loops.
+func TestBranchcov0723pmPrintProviderMSPBackupVerifyResult(t *testing.T) {
+	t.Run("nil result prints verified=false", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupVerifyResult(nil) })
+		assertContains0723pm(t, out, "provider_msp_backup_verified=false\n")
+		assertNotContains0723pm(t, out, "provider_msp_backup_verified=true")
+		assertNotContains0723pm(t, out, "archive_path=")
+	})
+
+	t.Run("empty slices omit the loop lines", func(t *testing.T) {
+		result := &cloudcp.ProviderMSPBackupVerifyResult{
+			ArchivePath:          "/b/a.tar.gz",
+			VerifiedArchiveBytes: 4096,
+			ControlPlaneEntries:  3,
+			TenantEntries:        5,
+			LicenseEntries:       1,
+			HasTenantRegistryDB:  true,
+			HasLicenseFile:       false,
+		}
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupVerifyResult(result) })
+		assertContains0723pm(t, out, "provider_msp_backup_verified=true\n")
+		assertContains0723pm(t, out, "archive_bytes=4096\n")
+		assertContains0723pm(t, out, "control_plane_entries=3\n")
+		assertContains0723pm(t, out, "tenant_entries=5\n")
+		assertContains0723pm(t, out, "license_entries=1\n")
+		assertContains0723pm(t, out, "tenant_registry_db_present=true\n")
+		assertContains0723pm(t, out, "license_file_present=false\n")
+		assertNotContains0723pm(t, out, "control_plane_db_backup=")
+		assertNotContains0723pm(t, out, "runtime_tenant_dir=")
+	})
+
+	t.Run("populated slices emit both loops", func(t *testing.T) {
+		result := &cloudcp.ProviderMSPBackupVerifyResult{
+			ControlPlaneDBFiles: []string{"/data/registry.db.bak", "/data/audit.db.bak"},
+			RuntimeTenantDirs:   []string{"t-1", "t-2"},
+		}
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupVerifyResult(result) })
+		assertContains0723pm(t, out, "control_plane_db_backup=/data/registry.db.bak\n")
+		assertContains0723pm(t, out, "control_plane_db_backup=/data/audit.db.bak\n")
+		assertContains0723pm(t, out, "runtime_tenant_dir=t-1\n")
+		assertContains0723pm(t, out, "runtime_tenant_dir=t-2\n")
+	})
+}
+
+// TestBranchcov0723pmPrintProviderMSPBackupManifest covers the manifest printer.
+// It takes the manifest BY VALUE so there is no nil branch; coverage targets the
+// zero-value form (empty slices -> no loop lines) and the populated form, plus
+// the fixed CreatedAt timestamp formatting.
+func TestBranchcov0723pmPrintProviderMSPBackupManifest(t *testing.T) {
+	t.Run("zero-value manifest prints zero counts and no loop lines", func(t *testing.T) {
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupManifest(cloudcp.ProviderMSPBackupManifest{}) })
+		assertContains0723pm(t, out, "manifest_version=\n")
+		// Zero time.Time formats deterministically in this layout.
+		assertContains0723pm(t, out, "created_at=0001-01-01T00:00:00Z\n")
+		assertContains0723pm(t, out, "control_plane_mode=\n")
+		assertContains0723pm(t, out, "workspace_limit=0\n")
+		assertContains0723pm(t, out, "registry_account_count=0\n")
+		assertContains0723pm(t, out, "registry_tenant_count=0\n")
+		assertContains0723pm(t, out, "runtime_tenant_count=0\n")
+		assertContains0723pm(t, out, "license_included=false\n")
+		assertNotContains0723pm(t, out, "runtime_tenant_id=")
+		assertNotContains0723pm(t, out, "manifest_db_backup=")
+	})
+
+	t.Run("populated manifest prints all fields, formatted timestamp and both loops", func(t *testing.T) {
+		created := time.Date(2026, 7, 23, 12, 34, 56, 0, time.UTC)
+		manifest := cloudcp.ProviderMSPBackupManifest{
+			Version:              cloudcp.ProviderMSPBackupManifestVersion,
+			CreatedAt:            created,
+			ControlPlaneMode:     string(cloudcp.ControlPlaneModeProviderHostedMSP),
+			Environment:          "production",
+			BaseURL:              "https://msp.example.com",
+			PlanVersion:          "msp_growth",
+			PlanSource:           "license-file",
+			LicenseID:            "lic-1",
+			LicenseEmail:         "ops@example.com",
+			LicenseIncluded:      true,
+			WorkspaceLimit:       15,
+			RegistryAccountCount: 2,
+			RegistryTenantCount:  5,
+			RuntimeTenantCount:   5,
+			RuntimeTenantIDs:     []string{"t-1", "t-2"},
+			ControlPlaneDBBackups: []string{
+				"/data/backups/provider-msp/control-plane/registry.db.bak",
+			},
+		}
+		out := capturePrint0723pm(t, func() { printProviderMSPBackupManifest(manifest) })
+		assertContains0723pm(t, out, "manifest_version="+cloudcp.ProviderMSPBackupManifestVersion+"\n")
+		assertContains0723pm(t, out, "created_at=2026-07-23T12:34:56Z\n")
+		assertContains0723pm(t, out, "control_plane_mode="+string(cloudcp.ControlPlaneModeProviderHostedMSP)+"\n")
+		assertContains0723pm(t, out, "environment=production\n")
+		assertContains0723pm(t, out, "base_url=https://msp.example.com\n")
+		assertContains0723pm(t, out, "plan_version=msp_growth\n")
+		assertContains0723pm(t, out, "plan_source=license-file\n")
+		assertContains0723pm(t, out, "license_id=lic-1\n")
+		assertContains0723pm(t, out, "license_email=ops@example.com\n")
+		assertContains0723pm(t, out, "license_included=true\n")
+		assertContains0723pm(t, out, "workspace_limit=15\n")
+		assertContains0723pm(t, out, "registry_account_count=2\n")
+		assertContains0723pm(t, out, "registry_tenant_count=5\n")
+		assertContains0723pm(t, out, "runtime_tenant_count=5\n")
+		assertContains0723pm(t, out, "runtime_tenant_id=t-1\n")
+		assertContains0723pm(t, out, "runtime_tenant_id=t-2\n")
+		assertContains0723pm(t, out, "manifest_db_backup=/data/backups/provider-msp/control-plane/registry.db.bak\n")
+	})
+}

internal/ai/eval/branchcov0723pm_test.go

@@ -0,0 +1,691 @@
+package eval
+
+import (
+	"os"
+	"regexp"
+	"sort"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/agentcapabilities"
+)
+
+// This file adds catalog-invariant and branch-coverage tests for the scenario
+// constructors in scenarios.go and patrol_scenarios.go. The constructors are
+// pure (they read env vars and assemble structs), so every target is exercised
+// directly without a network, SSH, daemon, or database.
+//
+// These tests assert real invariants that would catch a malformed scenario
+// added later (unique names, populated required fields, runnable assertions,
+// well-formed tool references, and the conditional append branches). They do
+// NOT echo per-field literals of each constructor.
+
+var branchcov0723pmToolTokenRe = regexp.MustCompile(`(?:pulse|patrol)_[a-z0-9_]+`)
+
+// branchcov0723pmScenarioCtors is the complete catalog of public Scenario
+// constructors in scenarios.go. Parity with the source file is enforced by
+// TestBranchcov0723pm_ScenarioCatalogParity, which scans scenarios.go itself -
+// the length assertion alone would be circular, since it only reads this table.
+func branchcov0723pmScenarioCtors() []struct {
+	name string
+	fn   func() Scenario
+} {
+	return []struct {
+		name string
+		fn   func() Scenario
+	}{
+		{"ReadOnlyInfrastructureScenario", ReadOnlyInfrastructureScenario},
+		{"RoutingValidationScenario", RoutingValidationScenario},
+		{"RoutingMismatchRecoveryScenario", RoutingMismatchRecoveryScenario},
+		{"LogTailingScenario", LogTailingScenario},
+		{"ReadOnlyViolationRecoveryScenario", ReadOnlyViolationRecoveryScenario},
+		{"SearchByIDScenario", SearchByIDScenario},
+		{"AmbiguousResourceDisambiguationScenario", AmbiguousResourceDisambiguationScenario},
+		{"ContextTargetCarryoverScenario", ContextTargetCarryoverScenario},
+		{"ResourceContextHandoffScenario", ResourceContextHandoffScenario},
+		{"DiscoveryScenario", DiscoveryScenario},
+		{"QuickSmokeTest", QuickSmokeTest},
+		{"TroubleshootingScenario", TroubleshootingScenario},
+		{"DeepDiveScenario", DeepDiveScenario},
+		{"ConfigInspectionScenario", ConfigInspectionScenario},
+		{"ResourceAnalysisScenario", ResourceAnalysisScenario},
+		{"MultiNodeScenario", MultiNodeScenario},
+		{"DockerInDockerScenario", DockerInDockerScenario},
+		{"ContextChainScenario", ContextChainScenario},
+		{"WriteVerifyScenario", WriteVerifyScenario},
+		{"ReadOnlyEnforcementScenario", ReadOnlyEnforcementScenario},
+		{"StrictResolutionScenario", StrictResolutionScenario},
+		{"StrictResolutionRecoveryScenario", StrictResolutionRecoveryScenario},
+		{"StrictResolutionBlockScenario", StrictResolutionBlockScenario},
+		{"ApprovalScenario", ApprovalScenario},
+		{"ApprovalComboScenario", ApprovalComboScenario},
+		{"ApprovalApproveScenario", ApprovalApproveScenario},
+		{"ApprovalDenyScenario", ApprovalDenyScenario},
+		{"GuestControlStopScenario", GuestControlStopScenario},
+		{"GuestControlIdempotentScenario", GuestControlIdempotentScenario},
+		{"GuestControlDiscoveryScenario", GuestControlDiscoveryScenario},
+		{"GuestControlNaturalLanguageScenario", GuestControlNaturalLanguageScenario},
+		{"GuestControlMultiMentionScenario", GuestControlMultiMentionScenario},
+		{"ReadOnlyModelChoiceScenario", ReadOnlyModelChoiceScenario},
+		{"ReadLoopRecoveryScenario", ReadLoopRecoveryScenario},
+		{"AmbiguousIntentScenario", AmbiguousIntentScenario},
+		{"NonInteractiveGuardrailScenario", NonInteractiveGuardrailScenario},
+	}
+}
+
+func branchcov0723pmRunStepAssertions(t *testing.T, step Step) []AssertionResult {
+	t.Helper()
+	zero := &StepResult{}
+	out := make([]AssertionResult, 0, len(step.Assertions))
+	for i, a := range step.Assertions {
+		if a == nil {
+			t.Fatalf("nil Assertion at index %d in step %q", i, step.Name)
+		}
+		out = append(out, a(zero))
+	}
+	return out
+}
+
+func branchcov0723pmNameSet(results []AssertionResult) map[string]int {
+	m := make(map[string]int, len(results))
+	for _, r := range results {
+		m[r.Name]++
+	}
+	return m
+}
+
+func branchcov0723pmHasName(m map[string]int, name string) bool { return m[name] > 0 }
+
+func branchcov0723pmRunPatrolAssertions(t *testing.T, ps PatrolScenario) []AssertionResult {
+	t.Helper()
+	zero := &PatrolRunResult{}
+	out := make([]AssertionResult, 0, len(ps.Assertions))
+	for i, a := range ps.Assertions {
+		if a == nil {
+			t.Fatalf("nil PatrolAssertion at index %d in %q", i, ps.Name)
+		}
+		out = append(out, a(zero))
+	}
+	return out
+}
+
+// branchcov0723pmKnownToolNames returns the canonical tool-name registry from
+// internal/agentcapabilities. The eval package has no registry of its own, so
+// this is the authoritative set referenced by the task.
+func branchcov0723pmKnownToolNames() map[string]struct{} {
+	return map[string]struct{}{
+		agentcapabilities.PulseQueryToolName:                 {},
+		agentcapabilities.PulseDiscoveryToolName:             {},
+		agentcapabilities.PulseMetricsToolName:               {},
+		agentcapabilities.PulseStorageToolName:               {},
+		agentcapabilities.PulseDockerToolName:                {},
+		agentcapabilities.PulseKubernetesToolName:            {},
+		agentcapabilities.PulseAlertsToolName:                {},
+		agentcapabilities.PulseReadToolName:                  {},
+		agentcapabilities.PulseControlToolName:               {},
+		agentcapabilities.PulseFileEditToolName:              {},
+		agentcapabilities.PulseKnowledgeToolName:             {},
+		agentcapabilities.PulsePMGToolName:                   {},
+		agentcapabilities.PulseSummarizeToolName:             {},
+		agentcapabilities.PulseRunCommandToolName:            {},
+		agentcapabilities.PulseControlGuestToolName:          {},
+		agentcapabilities.PulseControlDockerToolName:         {},
+		agentcapabilities.PulseSearchResourcesToolName:       {},
+		agentcapabilities.PulseGetResourceToolName:           {},
+		agentcapabilities.PulseGetTopologyToolName:           {},
+		agentcapabilities.PulseListInfrastructureToolName:    {},
+		agentcapabilities.PulseGetConnectionHealthToolName:   {},
+		agentcapabilities.PulseGetDockerLogsToolName:         {},
+		agentcapabilities.PulseGetPerformanceMetricsToolName: {},
+		agentcapabilities.PulseGetTemperaturesToolName:       {},
+		agentcapabilities.PulseGetBaselinesToolName:          {},
+		agentcapabilities.PulseGetPatternsToolName:           {},
+		agentcapabilities.PatrolGetFindingsToolName:          {},
+		agentcapabilities.PatrolAssessFindingToolName:        {},
+		agentcapabilities.PatrolReportFindingToolName:        {},
+		agentcapabilities.PatrolResolveFindingToolName:       {},
+		agentcapabilities.PatrolProposeActionToolName:        {},
+		agentcapabilities.PatrolActionCapabilitiesToolName:   {},
+	}
+}
+
+// branchcov0723pmRegisteredOrPrefix reports whether tok is an exact registered
+// tool name, and (separately) whether it is at least a prefix of one (a tool
+// "family" reference such as pulse_file -> pulse_file_edit). A token that is
+// neither is a genuine typo/malformation.
+func branchcov0723pmRegisteredOrPrefix(tok string, known map[string]struct{}) (exact, prefix bool) {
+	if _, ok := known[tok]; ok {
+		return true, true
+	}
+	for name := range known {
+		if strings.HasPrefix(name, tok) {
+			return false, true
+		}
+	}
+	return false, false
+}
+
+// TestBranchcov0723pm_ScenarioCatalogInvariants walks every Scenario
+// constructor and asserts structural invariants that would catch a malformed
+// scenario added later: non-empty unique names, populated descriptions, at
+// least one step, well-formed steps (name/prompt/assertions), unique step
+// names within a scenario, valid ApprovalDecision constants, well-formed
+// mentions, and assertions that actually run (no nil/panic) and emit a
+// non-empty Name+Message.
+// branchcov0723pmSourceCtorRe matches an exported, zero-argument constructor
+// returning a Scenario, which is the shape every entry in scenarios.go uses.
+var branchcov0723pmSourceCtorRe = regexp.MustCompile(`(?m)^func ([A-Z][A-Za-z0-9]*)\(\) Scenario \{`)
+
+// TestBranchcov0723pm_ScenarioCatalogParity makes the catalog table's
+// completeness claim real: it reads scenarios.go and fails if the file declares
+// a constructor the table does not list (or the table lists one the file no
+// longer declares). Without this, adding a 37th constructor would silently go
+// untested while every other assertion in this file still passed.
+func TestBranchcov0723pm_ScenarioCatalogParity(t *testing.T) {
+	src, err := os.ReadFile("scenarios.go")
+	require.NoError(t, err, "scenarios.go must be readable from the package dir")
+
+	var inSource []string
+	for _, m := range branchcov0723pmSourceCtorRe.FindAllStringSubmatch(string(src), -1) {
+		inSource = append(inSource, m[1])
+	}
+	require.NotEmpty(t, inSource, "regex must find constructors; update it if the source style changed")
+
+	var inTable []string
+	for _, c := range branchcov0723pmScenarioCtors() {
+		inTable = append(inTable, c.name)
+	}
+
+	sort.Strings(inSource)
+	sort.Strings(inTable)
+	assert.Equal(t, inSource, inTable,
+		"scenarios.go constructors and the catalog table must match exactly")
+}
+
+func TestBranchcov0723pm_ScenarioCatalogInvariants(t *testing.T) {
+	ctors := branchcov0723pmScenarioCtors()
+	require.Len(t, ctors, 36, "catalog table must list every scenario constructor")
+
+	seen := make(map[string]string, len(ctors))
+
+	for _, c := range ctors {
+		c := c
+		t.Run(c.name, func(t *testing.T) {
+			s := c.fn()
+
+			require.NotEmpty(t, s.Name, "Scenario.Name must be non-empty")
+			require.NotEmpty(t, strings.TrimSpace(s.Description), "Scenario.Description must be non-empty")
+			require.NotEmpty(t, s.Steps, "Scenario must define at least one Step")
+
+			if prev, dup := seen[s.Name]; dup {
+				t.Fatalf("duplicate Scenario.Name %q (also produced by %s)", s.Name, prev)
+			}
+			seen[s.Name] = c.name
+
+			stepNames := make(map[string]struct{}, len(s.Steps))
+			for i, step := range s.Steps {
+				require.NotEmpty(t, strings.TrimSpace(step.Name), "step %d: Name must be non-empty", i)
+				require.NotEmpty(t, strings.TrimSpace(step.Prompt), "step %d (%s): Prompt must be non-empty", i, step.Name)
+				require.NotEmpty(t, step.Assertions, "step %d (%s): must define Assertions", i, step.Name)
+
+				if _, dup := stepNames[step.Name]; dup {
+					t.Errorf("duplicate Step.Name %q within %s", step.Name, c.name)
+				}
+				stepNames[step.Name] = struct{}{}
+
+				switch step.ApprovalDecision {
+				case ApprovalNone, ApprovalApprove, ApprovalDeny:
+				default:
+					t.Errorf("step %d (%s): invalid ApprovalDecision %q", i, step.Name, step.ApprovalDecision)
+				}
+
+				for j, m := range step.Mentions {
+					assert.NotEmpty(t, m.ID, "step %d mention %d: ID required", i, j)
+					assert.NotEmpty(t, m.Name, "step %d mention %d: Name required", i, j)
+					assert.NotEmpty(t, m.Type, "step %d mention %d: Type required", i, j)
+				}
+
+				for k, res := range branchcov0723pmRunStepAssertions(t, step) {
+					assert.NotEmpty(t, res.Name, "step %d (%s) assertion %d: Name empty", i, step.Name, k)
+					assert.NotEmpty(t, res.Message, "step %d (%s) assertion %d: Message empty", i, step.Name, k)
+				}
+
+				// Handoff resources, when present, must carry an identity.
+				for j, hr := range step.HandoffResources {
+					assert.NotEmpty(t, hr.ID, "step %d handoff %d: ID required", i, j)
+					assert.NotEmpty(t, hr.Name, "step %d handoff %d: Name required", i, j)
+				}
+			}
+		})
+	}
+
+	// Every catalog name must be distinct (cross-checked a second way for clarity).
+	assert.Len(t, seen, len(ctors), "Scenario names must be unique across the catalog")
+}
+
+// TestBranchcov0723pm_ScenarioCatalogToolNames extracts every tool-name token
+// referenced by the catalog's assertions (via their observable result Names)
+// and asserts each is either an exact registered tool or a tool-family prefix.
+// Tokens that are only a prefix (not an exact tool) are logged for review.
+func TestBranchcov0723pm_ScenarioCatalogToolNames(t *testing.T) {
+	known := branchcov0723pmKnownToolNames()
+	zero := &StepResult{}
+
+	referenced := make(map[string]struct{})
+	for _, c := range branchcov0723pmScenarioCtors() {
+		s := c.fn()
+		for _, step := range s.Steps {
+			for _, a := range step.Assertions {
+				require.NotNil(t, a)
+				res := a(zero)
+				for _, raw := range branchcov0723pmToolTokenRe.FindAllString(res.Name, -1) {
+					// Assertion Names embed the tool name followed by a
+					// _contains / _contains_any suffix (e.g.
+					// "tool_input:pulse_query_contains:..."). Strip those known
+					// suffixes so the token resolves to the real tool name.
+					tok := strings.TrimSuffix(raw, "_contains_any")
+					tok = strings.TrimSuffix(tok, "_contains")
+					referenced[tok] = struct{}{}
+				}
+			}
+		}
+	}
+	require.NotEmpty(t, referenced, "expected the catalog to reference at least one tool")
+
+	var prefixOnly, bogus []string
+	for tok := range referenced {
+		exact, prefix := branchcov0723pmRegisteredOrPrefix(tok, known)
+		switch {
+		case exact:
+			// registered tool
+		case prefix:
+			prefixOnly = append(prefixOnly, tok)
+		default:
+			bogus = append(bogus, tok)
+		}
+	}
+
+	sort.Strings(bogus)
+	sort.Strings(prefixOnly)
+
+	// Hard invariant: every referenced tool token is a real tool or a real
+	// tool family. A bare typo (e.g. "pulse_qery") lands here and fails.
+	assert.Empty(t, bogus, "referenced tool tokens are not registered tools or prefixes: %v", bogus)
+
+	// Soft signal: tokens that are only a family prefix (not an exact tool)
+	// are surfaced for review without failing the suite.
+	if len(prefixOnly) > 0 {
+		t.Logf("tool tokens referenced only as a family prefix (not exact registered tools): %v", prefixOnly)
+	}
+}
+
+// --- Conditional-append branch coverage ---
+//
+// The constructors below append different assertions depending on env-driven
+// evalTargets flags. Each subtest pins the relevant flag and asserts the
+// observable difference (presence/absence of specific assertion result Names
+// and exact counts), exercising both arms of every conditional.
+
+func TestBranchcov0723pm_WriteVerifyScenario_Branches(t *testing.T) {
+	t.Run("require_write_verify_false", func(t *testing.T) {
+		ensureEnvUnset(t, "EVAL_REQUIRE_WRITE_VERIFY")
+		s := WriteVerifyScenario()
+		require.Len(t, s.Steps, 1)
+		res := branchcov0723pmRunStepAssertions(t, s.Steps[0])
+		names := branchcov0723pmNameSet(res)
+		assert.Len(t, res, 4)
+		assert.True(t, branchcov0723pmHasName(names, "no_error"))
+		assert.True(t, branchcov0723pmHasName(names, "eventual_success"))
+		assert.False(t, branchcov0723pmHasName(names, "tool_used:pulse_control"))
+		assert.False(t, branchcov0723pmHasName(names, "tool_used:pulse_read"))
+		assert.False(t, branchcov0723pmHasName(names, "tool_sequence"))
+	})
+
+	t.Run("require_write_verify_true", func(t *testing.T) {
+		t.Setenv("EVAL_REQUIRE_WRITE_VERIFY", "true")
+		s := WriteVerifyScenario()
+		require.Len(t, s.Steps, 1)
+		res := branchcov0723pmRunStepAssertions(t, s.Steps[0])
+		names := branchcov0723pmNameSet(res)
+		assert.Len(t, res, 7)
+		assert.True(t, branchcov0723pmHasName(names, "tool_used:pulse_control"))
+		assert.True(t, branchcov0723pmHasName(names, "tool_used:pulse_read"))
+		assert.True(t, branchcov0723pmHasName(names, "tool_sequence"))
+	})
+}
+
+func TestBranchcov0723pm_StrictResolutionScenario_Branches(t *testing.T) {
+	cases := []struct {
+		name             string
+		strict, recovery bool
+	}{
+		{"both_false", false, false},
+		{"strict_only", true, false},
+		{"recovery_only", false, true},
+		{"both_true", true, true},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.strict {
+				t.Setenv("EVAL_STRICT_RESOLUTION", "true")
+			} else {
+				ensureEnvUnset(t, "EVAL_STRICT_RESOLUTION")
+			}
+			if tc.recovery {
+				t.Setenv("EVAL_REQUIRE_STRICT_RECOVERY", "true")
+			} else {
+				ensureEnvUnset(t, "EVAL_REQUIRE_STRICT_RECOVERY")
+			}
+
+			s := StrictResolutionScenario()
+			require.Len(t, s.Steps, 2)
+
+			step1 := branchcov0723pmRunStepAssertions(t, s.Steps[0])
+			n1 := branchcov0723pmNameSet(step1)
+			if tc.strict {
+				assert.Len(t, step1, 3)
+				assert.True(t, branchcov0723pmHasName(n1, "tool_output:pulse_control_contains_any"))
+			} else {
+				assert.Len(t, step1, 2)
+				assert.False(t, branchcov0723pmHasName(n1, "tool_output:pulse_control_contains_any"))
+			}
+
+			step2 := branchcov0723pmRunStepAssertions(t, s.Steps[1])
+			n2 := branchcov0723pmNameSet(step2)
+			if tc.recovery {
+				assert.Len(t, step2, 4)
+				assert.True(t, branchcov0723pmHasName(n2, "tool_sequence"))
+			} else {
+				assert.Len(t, step2, 3)
+				assert.False(t, branchcov0723pmHasName(n2, "tool_sequence"))
+			}
+		})
+	}
+}
+
+func TestBranchcov0723pm_StrictResolutionRecoveryScenario_Branches(t *testing.T) {
+	cases := []struct {
+		name             string
+		strict, recovery bool
+		wantTotal        int
+	}{
+		{"both_false", false, false, 2},
+		{"strict_only", true, false, 6},
+		{"recovery_only", false, true, 3},
+		{"both_true", true, true, 7},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.strict {
+				t.Setenv("EVAL_STRICT_RESOLUTION", "true")
+			} else {
+				ensureEnvUnset(t, "EVAL_STRICT_RESOLUTION")
+			}
+			if tc.recovery {
+				t.Setenv("EVAL_REQUIRE_STRICT_RECOVERY", "true")
+			} else {
+				ensureEnvUnset(t, "EVAL_REQUIRE_STRICT_RECOVERY")
+			}
+
+			s := StrictResolutionRecoveryScenario()
+			require.Len(t, s.Steps, 1)
+
+			// Auto-deny is wired unconditionally so the eval never hangs.
+			assert.Equal(t, ApprovalDeny, s.Steps[0].ApprovalDecision)
+			assert.NotEmpty(t, s.Steps[0].ApprovalReason)
+
+			res := branchcov0723pmRunStepAssertions(t, s.Steps[0])
+			names := branchcov0723pmNameSet(res)
+			assert.Len(t, res, tc.wantTotal)
+
+			if tc.strict {
+				assert.True(t, branchcov0723pmHasName(names, "tool_used:pulse_control"))
+				assert.True(t, branchcov0723pmHasName(names, "tool_used:pulse_query"))
+				assert.True(t, branchcov0723pmHasName(names, "tool_output:pulse_control_contains_any"))
+				assert.True(t, branchcov0723pmHasName(names, "model_recovered"))
+			} else {
+				assert.False(t, branchcov0723pmHasName(names, "tool_used:pulse_control"))
+				assert.False(t, branchcov0723pmHasName(names, "model_recovered"))
+			}
+			if tc.recovery {
+				assert.True(t, branchcov0723pmHasName(names, "tool_sequence"))
+			} else {
+				assert.False(t, branchcov0723pmHasName(names, "tool_sequence"))
+			}
+		})
+	}
+}
+
+func TestBranchcov0723pm_StrictResolutionBlockScenario_Branches(t *testing.T) {
+	t.Run("strict_false_omits_output_assertion", func(t *testing.T) {
+		ensureEnvUnset(t, "EVAL_STRICT_RESOLUTION")
+		s := StrictResolutionBlockScenario()
+		require.Len(t, s.Steps, 1)
+		assert.Equal(t, ApprovalDeny, s.Steps[0].ApprovalDecision)
+		res := branchcov0723pmRunStepAssertions(t, s.Steps[0])
+		names := branchcov0723pmNameSet(res)
+		assert.Len(t, res, 4)
+		assert.True(t, branchcov0723pmHasName(names, "tool_used:pulse_query"))
+		assert.True(t, branchcov0723pmHasName(names, "tool_used:pulse_control"))
+		assert.False(t, branchcov0723pmHasName(names, "tool_output:pulse_control_contains_any"))
+	})
+
+	t.Run("strict_true_adds_output_assertion", func(t *testing.T) {
+		t.Setenv("EVAL_STRICT_RESOLUTION", "true")
+		s := StrictResolutionBlockScenario()
+		require.Len(t, s.Steps, 1)
+		res := branchcov0723pmRunStepAssertions(t, s.Steps[0])
+		names := branchcov0723pmNameSet(res)
+		assert.Len(t, res, 5)
+		assert.True(t, branchcov0723pmHasName(names, "tool_output:pulse_control_contains_any"))
+	})
+
+	// The local writeCmd defaulting mirrors approvalWriteCommand: an empty or
+	// literal "true" command is rewritten to a safe touch target.
+	t.Run("default_write_command_rewrites_to_touch", func(t *testing.T) {
+		ensureEnvUnset(t, "EVAL_WRITE_COMMAND")
+		ensureEnvUnset(t, "EVAL_STRICT_RESOLUTION")
+		s := StrictResolutionBlockScenario()
+		require.Len(t, s.Steps, 1)
+		assert.Contains(t, s.Steps[0].Prompt, "touch /tmp/pulse_eval_strict")
+	})
+
+	t.Run("explicit_true_write_command_rewrites_to_touch", func(t *testing.T) {
+		t.Setenv("EVAL_WRITE_COMMAND", "true")
+		ensureEnvUnset(t, "EVAL_STRICT_RESOLUTION")
+		s := StrictResolutionBlockScenario()
+		require.Len(t, s.Steps, 1)
+		assert.Contains(t, s.Steps[0].Prompt, "touch /tmp/pulse_eval_strict")
+	})
+
+	t.Run("custom_write_command_preserved", func(t *testing.T) {
+		t.Setenv("EVAL_WRITE_COMMAND", "echo branchcov0723pm-custom")
+		ensureEnvUnset(t, "EVAL_STRICT_RESOLUTION")
+		s := StrictResolutionBlockScenario()
+		require.Len(t, s.Steps, 1)
+		assert.Contains(t, s.Steps[0].Prompt, "echo branchcov0723pm-custom")
+		assert.NotContains(t, s.Steps[0].Prompt, "touch /tmp/pulse_eval_strict")
+	})
+}
+
+// TestBranchcov0723pm_ApprovalFamily_Branches covers the ExpectApproval
+// conditional in the four Approval constructors. When the flag is set, an
+// approval_requested assertion is appended; ApprovalApprove/Deny/Combo also
+// swap an eventual_success assertion for tool-output + approval assertions.
+// The ApprovalDecision constants wired by each constructor are pinned too.
+func TestBranchcov0723pm_ApprovalFamily_Branches(t *testing.T) {
+	ctors := []struct {
+		name string
+		fn   func() Scenario
+	}{
+		{"ApprovalScenario", ApprovalScenario},
+		{"ApprovalApproveScenario", ApprovalApproveScenario},
+		{"ApprovalDenyScenario", ApprovalDenyScenario},
+		{"ApprovalComboScenario", ApprovalComboScenario},
+	}
+	for _, c := range ctors {
+		c := c
+		t.Run(c.name+"/expect_approval_false", func(t *testing.T) {
+			ensureEnvUnset(t, "EVAL_EXPECT_APPROVAL")
+			s := c.fn()
+			branchcov0723pmAssertApprovalBranch(t, s, c.name, false)
+		})
+		t.Run(c.name+"/expect_approval_true", func(t *testing.T) {
+			t.Setenv("EVAL_EXPECT_APPROVAL", "true")
+			s := c.fn()
+			branchcov0723pmAssertApprovalBranch(t, s, c.name, true)
+		})
+	}
+}
+
+func branchcov0723pmAssertApprovalBranch(t *testing.T, s Scenario, ctorName string, expectApproval bool) {
+	t.Helper()
+
+	switch ctorName {
+	case "ApprovalScenario":
+		require.Len(t, s.Steps, 1)
+		res := branchcov0723pmRunStepAssertions(t, s.Steps[0])
+		names := branchcov0723pmNameSet(res)
+		if expectApproval {
+			assert.Len(t, res, 3)
+			assert.True(t, branchcov0723pmHasName(names, "approval_requested"))
+		} else {
+			assert.Len(t, res, 2)
+			assert.False(t, branchcov0723pmHasName(names, "approval_requested"))
+			assert.False(t, branchcov0723pmHasName(names, "eventual_success"))
+		}
+
+	case "ApprovalApproveScenario":
+		require.Len(t, s.Steps, 1)
+		assert.Equal(t, ApprovalApprove, s.Steps[0].ApprovalDecision)
+		res := branchcov0723pmRunStepAssertions(t, s.Steps[0])
+		names := branchcov0723pmNameSet(res)
+		if expectApproval {
+			assert.Len(t, res, 4)
+			assert.True(t, branchcov0723pmHasName(names, "approval_requested"))
+			assert.True(t, branchcov0723pmHasName(names, "tool_output:pulse_control_contains_any"))
+			assert.False(t, branchcov0723pmHasName(names, "eventual_success"))
+		} else {
+			assert.Len(t, res, 3)
+			assert.False(t, branchcov0723pmHasName(names, "approval_requested"))
+			assert.True(t, branchcov0723pmHasName(names, "eventual_success"))
+		}
+
+	case "ApprovalDenyScenario":
+		require.Len(t, s.Steps, 1)
+		assert.Equal(t, ApprovalDeny, s.Steps[0].ApprovalDecision)
+		res := branchcov0723pmRunStepAssertions(t, s.Steps[0])
+		names := branchcov0723pmNameSet(res)
+		if expectApproval {
+			assert.Len(t, res, 4)
+			assert.True(t, branchcov0723pmHasName(names, "approval_requested"))
+			assert.True(t, branchcov0723pmHasName(names, "tool_output:pulse_control_contains_any"))
+			assert.False(t, branchcov0723pmHasName(names, "eventual_success"))
+		} else {
+			assert.Len(t, res, 3)
+			assert.False(t, branchcov0723pmHasName(names, "approval_requested"))
+			assert.True(t, branchcov0723pmHasName(names, "eventual_success"))
+		}
+
+	case "ApprovalComboScenario":
+		require.Len(t, s.Steps, 2)
+		assert.Equal(t, ApprovalApprove, s.Steps[0].ApprovalDecision)
+		assert.Equal(t, ApprovalDeny, s.Steps[1].ApprovalDecision)
+		for i, step := range s.Steps {
+			res := branchcov0723pmRunStepAssertions(t, step)
+			names := branchcov0723pmNameSet(res)
+			if expectApproval {
+				assert.Len(t, res, 4, "combo step %d", i)
+				assert.True(t, branchcov0723pmHasName(names, "approval_requested"), "combo step %d", i)
+				assert.True(t, branchcov0723pmHasName(names, "tool_output:pulse_control_contains_any"), "combo step %d", i)
+				assert.False(t, branchcov0723pmHasName(names, "eventual_success"), "combo step %d", i)
+			} else {
+				assert.Len(t, res, 3, "combo step %d", i)
+				assert.False(t, branchcov0723pmHasName(names, "approval_requested"), "combo step %d", i)
+				assert.True(t, branchcov0723pmHasName(names, "eventual_success"), "combo step %d", i)
+			}
+		}
+	}
+}
+
+// --- Patrol scenario catalog ---
+
+func branchcov0723pmPatrolCtors() []struct {
+	name string
+	fn   func() PatrolScenario
+} {
+	return []struct {
+		name string
+		fn   func() PatrolScenario
+	}{
+		{"PatrolBasicScenario", PatrolBasicScenario},
+		{"PatrolInvestigationScenario", PatrolInvestigationScenario},
+		{"PatrolFindingQualityScenario", PatrolFindingQualityScenario},
+		{"PatrolSignalCoverageScenario", PatrolSignalCoverageScenario},
+	}
+}
+
+// TestBranchcov0723pm_PatrolScenarioCatalogInvariants asserts the structural
+// invariants for every PatrolScenario constructor: non-empty unique names,
+// populated descriptions, at least one assertion, and assertions that run on a
+// zero result without panicking while emitting non-empty Name+Message.
+func TestBranchcov0723pm_PatrolScenarioCatalogInvariants(t *testing.T) {
+	ctors := branchcov0723pmPatrolCtors()
+	require.Len(t, ctors, 4)
+
+	seen := make(map[string]string, len(ctors))
+	for _, c := range ctors {
+		c := c
+		t.Run(c.name, func(t *testing.T) {
+			ps := c.fn()
+			require.NotEmpty(t, ps.Name)
+			require.NotEmpty(t, strings.TrimSpace(ps.Description))
+			require.NotEmpty(t, ps.Assertions, "PatrolScenario must define Assertions")
+
+			if prev, dup := seen[ps.Name]; dup {
+				t.Fatalf("duplicate PatrolScenario.Name %q (also produced by %s)", ps.Name, prev)
+			}
+			seen[ps.Name] = c.name
+
+			for k, res := range branchcov0723pmRunPatrolAssertions(t, ps) {
+				assert.NotEmpty(t, res.Name, "assertion %d: Name empty", k)
+				assert.NotEmpty(t, res.Message, "assertion %d: Message empty", k)
+			}
+		})
+	}
+	assert.Len(t, seen, len(ctors), "PatrolScenario names must be unique")
+}
+
+// TestBranchcov0723pm_AllPatrolScenariosOrdering covers AllPatrolScenarios.
+// Its per-entry contents are already covered by the catalog-invariant test
+// above, and comparing them here against the same constructors AllPatrolScenarios
+// itself calls would be circular, so this asserts only what is independently
+// observable: the four entries, in declaration order, none dropped or repeated.
+func TestBranchcov0723pm_AllPatrolScenariosOrdering(t *testing.T) {
+	all := AllPatrolScenarios()
+	require.Len(t, all, 4)
+
+	// The only non-circular signal available here is ORDER and completeness:
+	// comparing each entry field-by-field against the same constructors the
+	// function itself calls would assert nothing. So assert the identity and
+	// sequence of the four names, and that none is dropped or duplicated.
+	gotNames := []string{all[0].Name, all[1].Name, all[2].Name, all[3].Name}
+	assert.Equal(t, []string{
+		PatrolBasicScenario().Name,
+		PatrolInvestigationScenario().Name,
+		PatrolFindingQualityScenario().Name,
+		PatrolSignalCoverageScenario().Name,
+	}, gotNames, "AllPatrolScenarios must return the four constructors in declaration order")
+
+	seen := map[string]bool{}
+	for _, s := range all {
+		assert.False(t, seen[s.Name], "duplicate scenario %q in AllPatrolScenarios", s.Name)
+		seen[s.Name] = true
+		assert.NotEmpty(t, s.Assertions, "scenario %q must carry assertions", s.Name)
+	}
+}

internal/ai/memory/branchcov0723pm_test.go

@@ -0,0 +1,383 @@
+package memory
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+// newBranchcov0723pmLog constructs a RemediationLog through the package's own
+// constructor, pointing persistence at a per-test temp directory so the
+// background saveToDisk goroutine spawned by Log/MarkRolledBack writes somewhere
+// harmless.
+//
+// Log and MarkRolledBack persist via a fire-and-forget goroutine. t.TempDir's
+// auto-cleanup (RemoveAll) would otherwise race those goroutines and fail with
+// "directory not empty". We therefore register a Cleanup that runs *before*
+// TempDir's RemoveAll (cleanups are LIFO, and TempDir registered first) and
+// waits for the persisted file to quiesce. This mirrors the package's existing
+// poll-for-file convention (see TestIncidentStore_SaveAsyncAndPersistence).
+func newBranchcov0723pmLog(t *testing.T) *RemediationLog {
+	t.Helper()
+	dataDir := t.TempDir()
+	rl := NewRemediationLog(RemediationLogConfig{DataDir: dataDir})
+	t.Cleanup(func() { waitForRemediationSaveQuiescence(t, dataDir) })
+	return rl
+}
+
+// waitForRemediationSaveQuiescence blocks until the remediation history file in
+// dataDir has stopped changing (no background save goroutine is still writing),
+// or until the deadline expires. It never fails the test; its only job is to
+// keep the temp directory stable long enough for RemoveAll to succeed.
+func waitForRemediationSaveQuiescence(t *testing.T, dataDir string) {
+	t.Helper()
+	path := filepath.Join(dataDir, remediationHistoryFileName)
+	deadline := time.Now().Add(2 * time.Second)
+	var lastMod time.Time
+	stableChecks := 0
+	for time.Now().Before(deadline) {
+		info, err := os.Stat(path)
+		if err == nil && info.ModTime().Equal(lastMod) {
+			stableChecks++
+			if stableChecks >= 3 { // ~30ms with no mutation: writes have drained
+				return
+			}
+		} else if err == nil {
+			lastMod = info.ModTime()
+			stableChecks = 0
+		} else {
+			stableChecks = 0
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+}
+
+func TestBranchcov0723pmGetByID(t *testing.T) {
+	t.Run("HitReturnsRightRecord", func(t *testing.T) {
+		rl := newBranchcov0723pmLog(t)
+		want := RemediationRecord{
+			ID:         "rec-hit",
+			ResourceID: "vm-1",
+			Problem:    "disk full",
+			Action:     "rm -rf /tmp/x",
+			Outcome:    OutcomeResolved,
+			Timestamp:  time.Date(2025, 1, 2, 3, 4, 5, 0, time.UTC),
+		}
+		if err := rl.Log(want); err != nil {
+			t.Fatalf("Log: %v", err)
+		}
+		// Decoy to confirm the scan picks the right element, not just the only one.
+		if err := rl.Log(RemediationRecord{ID: "rec-other", Problem: "p", Action: "a", Timestamp: time.Now()}); err != nil {
+			t.Fatalf("Log decoy: %v", err)
+		}
+
+		got, ok := rl.GetByID("rec-hit")
+		if !ok {
+			t.Fatalf("expected ok=true for existing id")
+		}
+		if got == nil {
+			t.Fatalf("expected non-nil record for existing id")
+		}
+		if got.ID != "rec-hit" {
+			t.Fatalf("ID = %q, want %q", got.ID, "rec-hit")
+		}
+		if got.ResourceID != "vm-1" || got.Problem != "disk full" || got.Action != "rm -rf /tmp/x" || got.Outcome != OutcomeResolved {
+			t.Fatalf("returned record = %+v, want the logged record", got)
+		}
+		if !got.Timestamp.Equal(want.Timestamp) {
+			t.Fatalf("Timestamp = %v, want %v", got.Timestamp, want.Timestamp)
+		}
+	})
+
+	t.Run("MissReturnsNilFalse", func(t *testing.T) {
+		rl := newBranchcov0723pmLog(t)
+		if err := rl.Log(RemediationRecord{ID: "rec-present", Problem: "p", Action: "a", Timestamp: time.Now()}); err != nil {
+			t.Fatalf("Log: %v", err)
+		}
+
+		got, ok := rl.GetByID("does-not-exist")
+		if ok {
+			t.Fatalf("expected ok=false for unknown id, got true")
+		}
+		if got != nil {
+			t.Fatalf("expected nil record for unknown id, got %+v", got)
+		}
+	})
+
+	t.Run("EmptyStoreAndEmptyID", func(t *testing.T) {
+		rl := newBranchcov0723pmLog(t)
+		// Empty store: nothing to find.
+		if got, ok := rl.GetByID("anything"); ok || got != nil {
+			t.Fatalf("empty store: expected (nil,false), got (%+v,%v)", got, ok)
+		}
+		// Empty id against a store where no record carries an empty id -> miss.
+		if err := rl.Log(RemediationRecord{ID: "has-id", Problem: "p", Action: "a", Timestamp: time.Now()}); err != nil {
+			t.Fatalf("Log: %v", err)
+		}
+		if got, ok := rl.GetByID(""); ok || got != nil {
+			t.Fatalf("empty id lookup: expected (nil,false), got (%+v,%v)", got, ok)
+		}
+	})
+}
+
+func TestBranchcov0723pmMarkRolledBack(t *testing.T) {
+	t.Run("UnknownIDReturnsErrorAndLeavesRecordsUntouched", func(t *testing.T) {
+		rl := newBranchcov0723pmLog(t)
+		if err := rl.Log(RemediationRecord{ID: "rec-a", Problem: "p", Action: "a", Timestamp: time.Now()}); err != nil {
+			t.Fatalf("Log: %v", err)
+		}
+
+		err := rl.MarkRolledBack("missing-id", "rb-1", "alice")
+		if err == nil {
+			t.Fatalf("expected error for unknown id, got nil")
+		}
+		if !strings.Contains(err.Error(), "missing-id") {
+			t.Fatalf("expected error to mention id %q, got %q", "missing-id", err.Error())
+		}
+
+		// Existing record must be untouched on the error path.
+		got, ok := rl.GetByID("rec-a")
+		if !ok || got == nil {
+			t.Fatalf("expected rec-a to still exist after failed rollback")
+		}
+		if got.Rollback != nil {
+			t.Fatalf("expected rec-a Rollback to remain nil, got %+v", got.Rollback)
+		}
+	})
+
+	t.Run("NilRollbackCreatesStructAndSetsEveryField", func(t *testing.T) {
+		rl := newBranchcov0723pmLog(t)
+		if err := rl.Log(RemediationRecord{ID: "rec-nil", Problem: "p", Action: "a", Timestamp: time.Now()}); err != nil {
+			t.Fatalf("Log: %v", err)
+		}
+		// Precondition: Rollback is nil before the call.
+		if pre, _ := rl.GetByID("rec-nil"); pre.Rollback != nil {
+			t.Fatalf("precondition: expected nil Rollback, got %+v", pre.Rollback)
+		}
+
+		before := time.Now()
+		if err := rl.MarkRolledBack("rec-nil", "rb-99", "bob"); err != nil {
+			t.Fatalf("MarkRolledBack: %v", err)
+		}
+		after := time.Now()
+
+		got, ok := rl.GetByID("rec-nil")
+		if !ok || got == nil {
+			t.Fatalf("expected rec-nil to exist after rollback")
+		}
+		rb := got.Rollback
+		if rb == nil {
+			t.Fatalf("expected Rollback struct to be created")
+		}
+		// Fields the function assigns.
+		if !rb.RolledBack {
+			t.Errorf("expected RolledBack=true, got false")
+		}
+		if rb.RolledBackBy != "bob" {
+			t.Errorf("expected RolledBackBy=%q, got %q", "bob", rb.RolledBackBy)
+		}
+		if rb.RollbackID != "rb-99" {
+			t.Errorf("expected RollbackID=%q, got %q", "rb-99", rb.RollbackID)
+		}
+		if rb.RolledBackAt == nil {
+			t.Fatalf("expected RolledBackAt to be set")
+		}
+		if rb.RolledBackAt.Before(before) || rb.RolledBackAt.After(after) {
+			t.Errorf("RolledBackAt=%v not within [%v,%v]", rb.RolledBackAt, before, after)
+		}
+		// Fields the function does NOT touch: must remain at the zero value of
+		// a freshly-created RollbackInfo{}.
+		if rb.Reversible {
+			t.Errorf("expected Reversible to remain false on newly-created struct, got true")
+		}
+		if rb.RollbackCmd != "" {
+			t.Errorf("expected RollbackCmd to remain empty, got %q", rb.RollbackCmd)
+		}
+		if rb.PreState != "" {
+			t.Errorf("expected PreState to remain empty, got %q", rb.PreState)
+		}
+	})
+
+	t.Run("ExistingRollbackOverwritesTrackedFieldsPreservesRest", func(t *testing.T) {
+		rl := newBranchcov0723pmLog(t)
+		oldTime := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+		if err := rl.Log(RemediationRecord{
+			ID:        "rec-existing",
+			Problem:   "p",
+			Action:    "a",
+			Timestamp: time.Now(),
+			Rollback: &RollbackInfo{
+				Reversible:   true,
+				RollbackCmd:  "undo-cmd",
+				PreState:     `{"cpu":4}`,
+				RolledBack:   false,
+				RolledBackAt: &oldTime,
+				RolledBackBy: "prev-user",
+				RollbackID:   "prev-rb",
+			},
+		}); err != nil {
+			t.Fatalf("Log: %v", err)
+		}
+
+		before := time.Now()
+		if err := rl.MarkRolledBack("rec-existing", "new-rb", "carol"); err != nil {
+			t.Fatalf("MarkRolledBack: %v", err)
+		}
+		after := time.Now()
+
+		got, ok := rl.GetByID("rec-existing")
+		if !ok || got == nil {
+			t.Fatalf("expected rec-existing to exist after rollback")
+		}
+		rb := got.Rollback
+		if rb == nil {
+			t.Fatalf("expected Rollback to still be present")
+		}
+		// Overwritten tracked fields.
+		if !rb.RolledBack {
+			t.Errorf("expected RolledBack overwritten to true, got false")
+		}
+		if rb.RolledBackBy != "carol" {
+			t.Errorf("expected RolledBackBy overwritten to %q, got %q", "carol", rb.RolledBackBy)
+		}
+		if rb.RollbackID != "new-rb" {
+			t.Errorf("expected RollbackID overwritten to %q, got %q", "new-rb", rb.RollbackID)
+		}
+		if rb.RolledBackAt == nil {
+			t.Fatalf("expected RolledBackAt overwritten to a new time")
+		}
+		if rb.RolledBackAt.Equal(oldTime) {
+			t.Errorf("expected RolledBackAt to be replaced, still equals oldTime")
+		}
+		if rb.RolledBackAt.Before(before) || rb.RolledBackAt.After(after) {
+			t.Errorf("RolledBackAt=%v not within [%v,%v]", rb.RolledBackAt, before, after)
+		}
+		// Preserved fields (not touched by MarkRolledBack).
+		if !rb.Reversible {
+			t.Errorf("expected Reversible preserved as true, got false")
+		}
+		if rb.RollbackCmd != "undo-cmd" {
+			t.Errorf("expected RollbackCmd preserved as %q, got %q", "undo-cmd", rb.RollbackCmd)
+		}
+		if rb.PreState != `{"cpu":4}` {
+			t.Errorf("expected PreState preserved, got %q", rb.PreState)
+		}
+	})
+}
+
+func TestBranchcov0723pmGetRollbackable(t *testing.T) {
+	// eligibleRecord builds a record that satisfies GetRollbackable's
+	// eligibility condition: Rollback != nil, Reversible, not rolled back,
+	// and not itself a rollback record.
+	eligibleRecord := func(id string, ts time.Time) RemediationRecord {
+		return RemediationRecord{
+			ID:        id,
+			Problem:   "p",
+			Action:    "a",
+			Timestamp: ts,
+			Rollback:  &RollbackInfo{Reversible: true},
+		}
+	}
+
+	t.Run("EmptyStoreAndNonPositiveLimit", func(t *testing.T) {
+		rl := newBranchcov0723pmLog(t)
+		// Empty store: loop body never executes.
+		if got := rl.GetRollbackable(5); len(got) != 0 {
+			t.Fatalf("empty store: expected 0 results, got %d", len(got))
+		}
+		// limit <= 0 makes the loop guard `len(result) < limit` false on the
+		// first evaluation, so even an eligible record is excluded.
+		if err := rl.Log(eligibleRecord("r1", time.Now())); err != nil {
+			t.Fatalf("Log: %v", err)
+		}
+		if got := rl.GetRollbackable(0); len(got) != 0 {
+			t.Fatalf("limit=0: expected 0 results, got %d", len(got))
+		}
+		if got := rl.GetRollbackable(-3); len(got) != 0 {
+			t.Fatalf("limit=-3: expected 0 results, got %d", len(got))
+		}
+	})
+
+	t.Run("FewerThanLimitReturnsAllNewestFirst", func(t *testing.T) {
+		rl := newBranchcov0723pmLog(t)
+		base := time.Now()
+		// Insert oldest -> newest (slice order, which is what the reverse
+		// iteration walks).
+		ids := []string{"old", "mid", "new"}
+		for i, id := range ids {
+			if err := rl.Log(eligibleRecord(id, base.Add(time.Duration(i)*time.Hour))); err != nil {
+				t.Fatalf("Log %s: %v", id, err)
+			}
+		}
+
+		got := rl.GetRollbackable(10)
+		if len(got) != 3 {
+			t.Fatalf("expected all 3 eligible records, got %d", len(got))
+		}
+		// Ordering is reverse slice order: newest first.
+		wantOrder := []string{"new", "mid", "old"}
+		for i, w := range wantOrder {
+			if got[i].ID != w {
+				t.Errorf("got[%d].ID = %q, want %q", i, got[i].ID, w)
+			}
+		}
+	})
+
+	t.Run("LimitHonouredAndOrdered", func(t *testing.T) {
+		rl := newBranchcov0723pmLog(t)
+		base := time.Now()
+		ids := []string{"e1", "e2", "e3", "e4", "e5"}
+		for i, id := range ids {
+			if err := rl.Log(eligibleRecord(id, base.Add(time.Duration(i)*time.Hour))); err != nil {
+				t.Fatalf("Log %s: %v", id, err)
+			}
+		}
+
+		got := rl.GetRollbackable(3)
+		if len(got) != 3 {
+			t.Fatalf("expected limit honoured at 3, got %d", len(got))
+		}
+		// The 3 most recent, newest first.
+		wantOrder := []string{"e5", "e4", "e3"}
+		for i, w := range wantOrder {
+			if got[i].ID != w {
+				t.Errorf("got[%d].ID = %q, want %q", i, got[i].ID, w)
+			}
+		}
+	})
+
+	t.Run("NoneEligibleAcrossEveryIneligibilityReason", func(t *testing.T) {
+		rl := newBranchcov0723pmLog(t)
+		ts := time.Now()
+		// One record per falsy arm of the eligibility condition.
+		records := []RemediationRecord{
+			{ID: "nil-rollback", Timestamp: ts, Rollback: nil},                                                 // Rollback == nil
+			{ID: "not-reversible", Timestamp: ts, Rollback: &RollbackInfo{Reversible: false}},                  // !Reversible
+			{ID: "already-rolled", Timestamp: ts, Rollback: &RollbackInfo{Reversible: true, RolledBack: true}}, // RolledBack
+			{ID: "is-rollback", Timestamp: ts, Rollback: &RollbackInfo{Reversible: true}, IsRollback: true},    // IsRollback
+		}
+		for _, rec := range records {
+			if err := rl.Log(rec); err != nil {
+				t.Fatalf("Log %s: %v", rec.ID, err)
+			}
+		}
+
+		if got := rl.GetRollbackable(10); len(got) != 0 {
+			t.Fatalf("expected 0 eligible across all ineligibility reasons, got %d: %+v", len(got), got)
+		}
+
+		// Adding a single eligible record must surface only it.
+		if err := rl.Log(eligibleRecord("the-eligible", ts)); err != nil {
+			t.Fatalf("Log eligible: %v", err)
+		}
+		got := rl.GetRollbackable(10)
+		if len(got) != 1 {
+			t.Fatalf("expected exactly 1 eligible after adding one, got %d", len(got))
+		}
+		if got[0].ID != "the-eligible" {
+			t.Errorf("expected only the-eligible, got %q", got[0].ID)
+		}
+	})
+}

internal/alerts/config/branchcov0723pm_test.go

@@ -0,0 +1,222 @@
+package config_test
+
+import (
+	"reflect"
+	"testing"
+
+	alertconfig "github.com/rcourtman/pulse-go-rewrite/internal/alerts/config"
+)
+
+// This file adds branch coverage for (*AlertConfig).UnmarshalJSON
+// (types.go:276). UnmarshalJSON decodes into an alias type, then re-decodes
+// the same bytes into a raw map, then calls NormalizeAlertConfigAliases. The
+// tests assert the actual normalization result (legacy alias keys stripped,
+// canonical keys retained), the malformed-JSON error path, and the
+// nil-vs-present map branches.
+//
+// Purity: every case is exercised with in-memory []byte payloads -- no
+// network, SSH, daemon, or database is required.
+
+// TestBranchcov0723pmUnmarshalJSONErrorArms covers the first json.Unmarshal
+// error return (types.go:279-281). It also demonstrates, via valid inputs, that
+// the SECOND json.Unmarshal error return (types.go:285-287) is unreachable:
+// both calls consume the same `data`, and decoding into map[string]json.RawMessage
+// is strictly more permissive than decoding into the struct alias (it accepts
+// any valid JSON object or null and never validates field types). Every input
+// that makes the map decode fail (array/number/string/scalar) makes the struct
+// decode fail first, so control never reaches the second error return when the
+// first succeeds. See GLM_REPORT_go-alertcfg.md.
+func TestBranchcov0723pmUnmarshalJSONErrorArms(t *testing.T) {
+	cases := []struct {
+		name string
+		data string
+	}{
+		{name: "truncated object", data: `{bad`},
+		{name: "trailing garbage after valid object", data: `{"enabled":true}garbage`},
+		{name: "bare EOF", data: ``},
+		{name: "top level array instead of object", data: `[1,2,3]`},
+		{name: "top level number instead of object", data: `42`},
+		{name: "top level string instead of object", data: `"hello"`},
+		{name: "field type mismatch on bool", data: `{"enabled":"notabool"}`},
+		{name: "field type mismatch nested", data: `{"guestDefaults":{"cpu":{"trigger":"x"}}}`},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Pre-populate to prove the error path returns BEFORE mutating `*c`.
+			cfg := &alertconfig.AlertConfig{Enabled: true, MinimumDelta: 7}
+			err := cfg.UnmarshalJSON([]byte(tc.data))
+			if err == nil {
+				t.Fatalf("UnmarshalJSON(%q) err = nil, want non-nil", tc.data)
+			}
+			if cfg.Enabled != true {
+				t.Fatalf("Enabled = %v, want true (config must be untouched on first-unmarshal error)", cfg.Enabled)
+			}
+			if cfg.MinimumDelta != 7 {
+				t.Fatalf("MinimumDelta = %v, want 7 (config must be untouched on first-unmarshal error)", cfg.MinimumDelta)
+			}
+		})
+	}
+
+	// Empirically pin the unreachability of the second error arm: every input
+	// the struct decoder accepts also round-trips through the raw-map decode
+	// without error and reaches normalization.
+	t.Run("valid object and null never hit second error arm", func(t *testing.T) {
+		for _, data := range []string{`{}`, `null`, `{"enabled":true,"timeThresholds":{"guest":1}}`} {
+			cfg := &alertconfig.AlertConfig{}
+			if err := cfg.UnmarshalJSON([]byte(data)); err != nil {
+				t.Fatalf("UnmarshalJSON(%q) err = %v, want nil (second arm should not fail)", data, err)
+			}
+		}
+	})
+}
+
+// TestBranchcov0723pmUnmarshalJSONNormalization covers the success path of
+// UnmarshalJSON and every observable branch of NormalizeAlertConfigAliases that
+// it invokes: TimeThresholds nil (skip block) vs present (iterate), the
+// "typeKey == all" / "typeKey == empty" continue arms (keys retained), the
+// legacy-unsupported delete arm, and the canonical-survives arm.
+func TestBranchcov0723pmUnmarshalJSONNormalization(t *testing.T) {
+	cases := []struct {
+		name string
+		data string
+		// wantTime is the exact expected TimeThresholds map after normalization.
+		wantTime map[string]int
+		// wantTimeNil asserts the map is nil (covers the nil-skip-block branch).
+		wantTimeNil bool
+	}{
+		{
+			// Empty object: both maps nil -> TimeThresholds block skipped
+			// (!= nil false) and MetricTimeThresholds early-returns (len 0).
+			name:        "empty object leaves TimeThresholds nil",
+			data:        `{}`,
+			wantTimeNil: true,
+		},
+		{
+			// JSON null: struct decode succeeds (zero value), second decode of
+			// null into the map leaves it nil; normalize skips both blocks.
+			name:        "null json leaves TimeThresholds nil",
+			data:        `null`,
+			wantTimeNil: true,
+		},
+		{
+			// Canonical keys only -> all retained, none deleted.
+			name:     "canonical time threshold keys retained",
+			data:     `{"timeThresholds":{"guest":30,"node":60,"storage":15}}`,
+			wantTime: map[string]int{"guest": 30, "node": 60, "storage": 15},
+		},
+		{
+			// Mixed: legacy alias keys (qemu/lxc/host) are unsupported -> deleted;
+			// canonical keys (guest/node) survive. Asserts the delete arm AND the
+			// retain arm in a single payload.
+			name:     "legacy alias keys stripped while canonical survive",
+			data:     `{"timeThresholds":{"qemu":10,"lxc":20,"host":40,"guest":30,"node":60}}`,
+			wantTime: map[string]int{"guest": 30, "node": 60},
+		},
+		{
+			// "all" maps to CanonicalAlertResourceType "all" -> continue (kept).
+			// "" maps to "" -> continue (kept). guest is canonical -> kept.
+			// None are deleted; this exercises both `continue` arms.
+			name: "all and empty-string keys retained via continue arm",
+			data: `{"timeThresholds":{"all":99,"":7,"guest":30}}`,
+			wantTime: map[string]int{
+				"all":   99,
+				"":      7,
+				"guest": 30,
+			},
+		},
+		{
+			// "kubernetes-cluster" is in the unsupported switch -> deleted;
+			// "agent disk" canonicalizes to "agent disk" (default arm, not in
+			// any multi-word canonical case) and is in the unsupported switch ->
+			// deleted; "pbs" canonical -> kept.
+			name:     "kubernetes-cluster and agent disk stripped pbs kept",
+			data:     `{"timeThresholds":{"kubernetes-cluster":8,"agent disk":9,"pbs":12}}`,
+			wantTime: map[string]int{"pbs": 12},
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := &alertconfig.AlertConfig{}
+			if err := cfg.UnmarshalJSON([]byte(tc.data)); err != nil {
+				t.Fatalf("UnmarshalJSON(%q) err = %v, want nil", tc.data, err)
+			}
+			if tc.wantTimeNil {
+				if cfg.TimeThresholds != nil {
+					t.Fatalf("TimeThresholds = %v, want nil", cfg.TimeThresholds)
+				}
+				return
+			}
+			if cfg.TimeThresholds == nil {
+				t.Fatalf("TimeThresholds = nil, want %v", tc.wantTime)
+			}
+			if !reflect.DeepEqual(cfg.TimeThresholds, tc.wantTime) {
+				t.Fatalf("TimeThresholds = %v, want %v", cfg.TimeThresholds, tc.wantTime)
+			}
+		})
+	}
+
+	// Separately assert that a non-threshold canonical field round-trips, so the
+	// test is not solely about map normalization.
+	t.Run("canonical enabled flag decodes through alias", func(t *testing.T) {
+		cfg := &alertconfig.AlertConfig{}
+		if err := cfg.UnmarshalJSON([]byte(`{"enabled":true}`)); err != nil {
+			t.Fatalf("err = %v, want nil", err)
+		}
+		if !cfg.Enabled {
+			t.Fatalf("Enabled = false, want true")
+		}
+	})
+}
+
+// TestBranchcov0723pmUnmarshalJSONMetricThresholds covers the
+// MetricTimeThresholds half of NormalizeAlertConfigAliases: the len==0 early
+// return (covered above by the empty/null cases) vs the iteration path, where
+// legacy type keys are deleted, "all"/empty-string keys are retained via the
+// continue arm, and canonical keys survive.
+func TestBranchcov0723pmUnmarshalJSONMetricThresholds(t *testing.T) {
+	t.Run("legacy type keys stripped canonical and all retained", func(t *testing.T) {
+		cfg := &alertconfig.AlertConfig{}
+		data := `{"metricTimeThresholds":{"qemu":{"cpu":5},"guest":{"mem":10},"all":{"disk":15},"":{"net":20}}}`
+		if err := cfg.UnmarshalJSON([]byte(data)); err != nil {
+			t.Fatalf("err = %v, want nil", err)
+		}
+		if cfg.MetricTimeThresholds == nil {
+			t.Fatal("MetricTimeThresholds = nil, want non-nil")
+		}
+		if _, ok := cfg.MetricTimeThresholds["qemu"]; ok {
+			t.Fatalf("qemu should have been stripped, map=%+v", cfg.MetricTimeThresholds)
+		}
+		want := map[string]map[string]int{
+			"guest": {"mem": 10},
+			"all":   {"disk": 15},
+			"":      {"net": 20},
+		}
+		if !reflect.DeepEqual(cfg.MetricTimeThresholds, want) {
+			t.Fatalf("MetricTimeThresholds = %+v, want %+v", cfg.MetricTimeThresholds, want)
+		}
+	})
+
+	t.Run("all legacy type keys stripped leaves empty map", func(t *testing.T) {
+		cfg := &alertconfig.AlertConfig{}
+		data := `{"metricTimeThresholds":{"docker":{"cpu":1},"k8s":{"mem":2}}}`
+		if err := cfg.UnmarshalJSON([]byte(data)); err != nil {
+			t.Fatalf("err = %v, want nil", err)
+		}
+		if cfg.MetricTimeThresholds == nil {
+			t.Fatal("MetricTimeThresholds = nil, want non-nil empty map")
+		}
+		if len(cfg.MetricTimeThresholds) != 0 {
+			t.Fatalf("MetricTimeThresholds = %+v, want empty (both keys unsupported)", cfg.MetricTimeThresholds)
+		}
+	})
+
+	t.Run("absent metricTimeThresholds leaves field nil", func(t *testing.T) {
+		cfg := &alertconfig.AlertConfig{}
+		if err := cfg.UnmarshalJSON([]byte(`{"enabled":true}`)); err != nil {
+			t.Fatalf("err = %v, want nil", err)
+		}
+		if cfg.MetricTimeThresholds != nil {
+			t.Fatalf("MetricTimeThresholds = %+v, want nil (len==0 early return)", cfg.MetricTimeThresholds)
+		}
+	})
+}

internal/api/branchcov0723pm_test.go

@@ -0,0 +1,174 @@
+package api
+
+import (
+	"context"
+	"errors"
+	"testing"
+)
+
+// branchcov0723pmBoolPtr boxes a bool so NodeConfigRequest.VerifySSL can be
+// driven through its *bool pointer in both the nil and the explicitly-set
+// arms of testProxmoxPlatformConnection.
+func branchcov0723pmBoolPtr(v bool) *bool {
+	return &v
+}
+
+// branchcov0723pmSpyConnect returns an injectable connect func matching the
+// signature testProxmoxPlatformConnection expects. It records the verifySSL
+// value it was actually called with (so a test can assert the req.VerifySSL
+// dereference happened) and then returns either connectErr (exercising the
+// create_client error arm) or a probe whose own result is probeErr
+// (exercising the connection error arm, or the success path when both nil).
+func branchcov0723pmSpyConnect(captured *bool, probeErr, connectErr error) func(verifySSL bool) (func(context.Context) error, error) {
+	return func(verifySSL bool) (func(context.Context) error, error) {
+		*captured = verifySSL
+		if connectErr != nil {
+			return nil, connectErr
+		}
+		return func(context.Context) error {
+			return probeErr
+		}, nil
+	}
+}
+
+// TestBranchcov0723pmPlatformConnection_VerifySSL covers both arms of the
+// req.VerifySSL dereference: a nil pointer must default to false before it
+// reaches connect, while an explicit true / false pointer must be passed
+// through verbatim. The spy asserts the exact bool handed to connect.
+func TestBranchcov0723pmPlatformConnection_VerifySSL(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name      string
+		verifySSL *bool
+		want      bool
+	}{
+		{"nil_defaults_to_false", nil, false},
+		{"explicit_true_passed_through", branchcov0723pmBoolPtr(true), true},
+		{"explicit_false_passed_through", branchcov0723pmBoolPtr(false), false},
+	}
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			req := NodeConfigRequest{VerifySSL: tc.verifySSL}
+			var captured bool
+			got := testProxmoxPlatformConnection(req, "ok", branchcov0723pmSpyConnect(&captured, nil, nil))
+
+			if captured != tc.want {
+				t.Errorf("verifySSL handed to connect = %v, want %v", captured, tc.want)
+			}
+			if status, _ := got["status"].(string); status != "success" {
+				t.Fatalf("status = %q, want success (probe should succeed); message=%v", status, got["message"])
+			}
+		})
+	}
+}
+
+// TestBranchcov0723pmPlatformConnection_ConnectError covers the arm where
+// connect itself fails: the result status must be "error" and the message
+// must be the sanitized create_client string (not the raw error), proving
+// the error flowed through sanitizeErrorMessage with that context.
+func TestBranchcov0723pmPlatformConnection_ConnectError(t *testing.T) {
+	t.Parallel()
+
+	req := NodeConfigRequest{}
+	var captured bool
+	got := testProxmoxPlatformConnection(req, "ok", branchcov0723pmSpyConnect(&captured, nil, errors.New("boom: dial tcp 10.0.0.1:443")))
+
+	if status, _ := got["status"].(string); status != "error" {
+		t.Fatalf("status = %q, want error", status)
+	}
+	const wantMsg = "Failed to initialize connection"
+	if msg, _ := got["message"].(string); msg != wantMsg {
+		t.Errorf("message = %q, want sanitized %q (create_client context)", msg, wantMsg)
+	}
+	// connect was still invoked with the defaulted verifySSL (false) before failing.
+	if captured != false {
+		t.Errorf("verifySSL handed to connect = %v, want false (default)", captured)
+	}
+	// The error arm must not surface a latency reading.
+	if _, ok := got["latency"]; ok {
+		t.Errorf("latency key present on connect-error arm, want absent")
+	}
+}
+
+// TestBranchcov0723pmPlatformConnection_ProbeError covers the arm where
+// connect succeeds but the probe call itself returns an error: the result
+// status must be "error" and the message must be the sanitized connection
+// string, proving the error flowed through sanitizeErrorMessage with the
+// "connection" context (distinct from the create_client context above).
+func TestBranchcov0723pmPlatformConnection_ProbeError(t *testing.T) {
+	t.Parallel()
+
+	req := NodeConfigRequest{}
+	var seenVerifySSL bool
+	got := testProxmoxPlatformConnection(req, "ok", func(verifySSL bool) (func(context.Context) error, error) {
+		seenVerifySSL = verifySSL
+		return func(context.Context) error {
+			return errors.New("boom: GetVersion rpc failed: 401 Unauthorized")
+		}, nil
+	})
+
+	if seenVerifySSL != false {
+		t.Errorf("verifySSL handed to connect = %v, want false (default)", seenVerifySSL)
+	}
+	if status, _ := got["status"].(string); status != "error" {
+		t.Fatalf("status = %q, want error", status)
+	}
+	const wantMsg = "Connection failed. Please check your credentials and network settings"
+	if msg, _ := got["message"].(string); msg != wantMsg {
+		t.Errorf("message = %q, want sanitized %q (connection context)", msg, wantMsg)
+	}
+	if _, ok := got["latency"]; ok {
+		t.Errorf("latency key present on probe-error arm, want absent")
+	}
+}
+
+// TestBranchcov0723pmPlatformConnection_Success covers the success path:
+// status "success", the caller-supplied successMsg passed through verbatim,
+// a latency key present and non-negative, and confirmation that the probe
+// was actually invoked with a context carrying the 10s deadline the
+// function sets up.
+func TestBranchcov0723pmPlatformConnection_Success(t *testing.T) {
+	t.Parallel()
+
+	const successMsg = "Connected to PBS instance"
+	req := NodeConfigRequest{}
+
+	var probeCalled bool
+	got := testProxmoxPlatformConnection(req, successMsg, func(verifySSL bool) (func(context.Context) error, error) {
+		if verifySSL != false {
+			t.Errorf("verifySSL = %v, want false (default)", verifySSL)
+		}
+		return func(ctx context.Context) error {
+			probeCalled = true
+			if _, ok := ctx.Deadline(); !ok {
+				t.Errorf("probe ctx has no deadline, want the 10s timeout set by the function")
+			}
+			return nil
+		}, nil
+	})
+
+	if !probeCalled {
+		t.Fatal("probe was not invoked on the success path")
+	}
+	if status, _ := got["status"].(string); status != "success" {
+		t.Fatalf("status = %q, want success", status)
+	}
+	if msg, _ := got["message"].(string); msg != successMsg {
+		t.Errorf("message = %q, want the caller-supplied successMsg %q", msg, successMsg)
+	}
+	latency, ok := got["latency"]
+	if !ok {
+		t.Fatal("latency key missing on success path")
+	}
+	ms, ok := latency.(int64)
+	if !ok {
+		t.Fatalf("latency value type = %T, want int64", latency)
+	}
+	if ms < 0 {
+		t.Errorf("latency = %d, want >= 0", ms)
+	}
+}

internal/config/branchcov0723pm_test.go

@@ -0,0 +1,242 @@
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// Tests in this file use the TestBranchcov0723pm prefix so the scoped run
+//
+//	go test ./internal/config/ -run '^TestBranchcov0723pm' -count=1
+//
+// selects only them. They raise branch coverage for two previously-uncovered
+// filesystem functions in migration.go:
+//   - copyFile              (migration.go:153)
+//   - RunMigrationIfNeeded  (migration.go:202)
+//
+// Every case is isolated under t.TempDir(); nothing escapes the test process.
+// Both targets are pure filesystem functions (no network, SSH, daemon or live
+// database), so neither is skipped on purity grounds.
+
+// TestBranchcov0723pm_CopyFile exercises every return path of copyFile.
+func TestBranchcov0723pm_CopyFile(t *testing.T) {
+	t.Run("missing source returns read error and creates no destination", func(t *testing.T) {
+		dir := t.TempDir()
+		missing := filepath.Join(dir, "does-not-exist")
+		dst := filepath.Join(dir, "dst")
+
+		err := copyFile(missing, dst)
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "read source file",
+			"missing source must surface the read-error branch")
+		// The destination must not be created when the copy aborts at read time.
+		_, statErr := os.Stat(dst)
+		require.True(t, os.IsNotExist(statErr),
+			"destination must not exist after a failed source read")
+	})
+
+	t.Run("destination parent missing returns write error", func(t *testing.T) {
+		dir := t.TempDir()
+		src := filepath.Join(dir, "src")
+		require.NoError(t, os.WriteFile(src, []byte("payload"), 0o600))
+		// dst sits under a directory that does not exist; os.WriteFile does not
+		// create parent directories, so this deterministically hits the
+		// write-error branch of copyFile.
+		dst := filepath.Join(dir, "no-such-parent", "dst")
+
+		err := copyFile(src, dst)
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "write destination file",
+			"uncreatable destination must surface the write-error branch")
+	})
+
+	t.Run("success copies bytes and creates a previously-absent destination", func(t *testing.T) {
+		dir := t.TempDir()
+		want := []byte("line one\nline two\n")
+		src := filepath.Join(dir, "src")
+		require.NoError(t, os.WriteFile(src, want, 0o600))
+		dst := filepath.Join(dir, "dst")
+
+		// Precondition: prove the destination does not yet exist, so a passing
+		// copy genuinely *creates* it rather than copying over something that
+		// was already there.
+		_, preStatErr := os.Stat(dst)
+		require.True(t, os.IsNotExist(preStatErr),
+			"precondition: destination must not exist before copyFile runs")
+
+		require.NoError(t, copyFile(src, dst))
+
+		got, err := os.ReadFile(dst)
+		require.NoError(t, err)
+		assert.Equal(t, want, got, "copied bytes must be byte-identical to the source")
+	})
+
+	t.Run("success propagates the source file's permission bits", func(t *testing.T) {
+		dir := t.TempDir()
+		src := filepath.Join(dir, "src")
+		// Owner-only mode is deliberately distinct from a typical 0o644 default,
+		// so a copyFile that hardcoded a mode instead of stat-ing the source
+		// would fail this assertion. (Umask is absorbed by reading src's actual
+		// on-disk mode rather than asserting a literal.)
+		require.NoError(t, os.WriteFile(src, []byte("x"), 0o600))
+		srcInfo, err := os.Stat(src)
+		require.NoError(t, err)
+		wantPerm := srcInfo.Mode().Perm()
+
+		dst := filepath.Join(dir, "dst")
+		require.NoError(t, copyFile(src, dst))
+
+		dstInfo, err := os.Stat(dst)
+		require.NoError(t, err)
+		assert.Equal(t, wantPerm, dstInfo.Mode().Perm(),
+			"copyFile must write the destination using the source's permission mode")
+	})
+
+	t.Run("existing destination is overwritten with source bytes", func(t *testing.T) {
+		dir := t.TempDir()
+		src := filepath.Join(dir, "src")
+		require.NoError(t, os.WriteFile(src, []byte("new contents"), 0o600))
+		dst := filepath.Join(dir, "dst")
+		// Pre-existing destination with distinct contents that must be replaced.
+		require.NoError(t, os.WriteFile(dst, []byte("old contents"), 0o600))
+
+		require.NoError(t, copyFile(src, dst))
+
+		got, err := os.ReadFile(dst)
+		require.NoError(t, err)
+		assert.Equal(t, "new contents", string(got),
+			"existing destination must be overwritten with the source bytes")
+	})
+}
+
+// TestBranchcov0723pm_RunMigrationIfNeeded exercises every return path of
+// RunMigrationIfNeeded: the no-op short-circuit (already migrated, nothing to
+// migrate, and empty data dir), a successful real migration, and the
+// migration-error wrapping branch.
+func TestBranchcov0723pm_RunMigrationIfNeeded(t *testing.T) {
+	t.Run("already migrated short-circuits and leaves legacy files untouched", func(t *testing.T) {
+		dataDir := t.TempDir()
+
+		// Seed a legacy file at the data root so migration *would* be needed if
+		// the marker were absent.
+		legacyPath := filepath.Join(dataDir, "system.json")
+		require.NoError(t, os.WriteFile(legacyPath, []byte("legacy-system"), 0o600))
+
+		// Plant the migration marker; its presence makes IsMigrationNeeded
+		// return false, so RunMigrationIfNeeded must short-circuit.
+		defaultOrgDir := filepath.Join(dataDir, "orgs", "default")
+		require.NoError(t, os.MkdirAll(defaultOrgDir, 0o700))
+		require.NoError(t, os.WriteFile(filepath.Join(defaultOrgDir, ".migrated"), []byte("done"), 0o600))
+
+		require.NoError(t, RunMigrationIfNeeded(dataDir),
+			"already-migrated data dir must short-circuit with nil error")
+
+		// The short-circuit must not have run the migration: the legacy file is
+		// still a regular file (not a symlink) at the data root with its
+		// original bytes...
+		info, err := os.Lstat(legacyPath)
+		require.NoError(t, err)
+		assert.Equal(t, os.FileMode(0), info.Mode()&os.ModeSymlink,
+			"short-circuit must not turn the legacy file into a symlink")
+		got, err := os.ReadFile(legacyPath)
+		require.NoError(t, err)
+		assert.Equal(t, "legacy-system", string(got),
+			"short-circuit must not alter legacy file contents")
+		// ...and the file must not have been copied into the default org dir.
+		_, err = os.Stat(filepath.Join(defaultOrgDir, "system.json"))
+		require.True(t, os.IsNotExist(err),
+			"short-circuit must not migrate the file into the default org dir")
+	})
+
+	t.Run("nothing to migrate on empty data dir returns nil", func(t *testing.T) {
+		dataDir := t.TempDir()
+
+		require.NoError(t, RunMigrationIfNeeded(dataDir),
+			"data dir with no legacy files and no marker must return nil")
+
+		// A no-op run must not materialize the orgs tree at all.
+		_, err := os.Stat(filepath.Join(dataDir, "orgs"))
+		require.True(t, os.IsNotExist(err),
+			"nothing-to-migrate must not create the orgs directory")
+	})
+
+	t.Run("empty data dir string returns nil", func(t *testing.T) {
+		// IsMigrationNeeded returns false for an empty string, so the very first
+		// guard in RunMigrationIfNeeded must short-circuit before any I/O.
+		require.NoError(t, RunMigrationIfNeeded(""),
+			"empty data dir must short-circuit with nil error")
+	})
+
+	t.Run("real migration moves files, symlinks originals, and writes marker", func(t *testing.T) {
+		dataDir := t.TempDir()
+
+		// Seed every file listed in filesToMigrate with distinctive contents so
+		// the resulting on-disk layout can be asserted precisely.
+		contents := map[string]string{
+			"nodes.enc":          "enc-bytes",
+			"system.json":        "{\"k\":\"v\"}",
+			"alerts.json":        "[]",
+			"notifications.json": "{}",
+			"audit.db":           "SQLITE-HEADER",
+		}
+		for _, name := range filesToMigrate {
+			require.NoError(t, os.WriteFile(filepath.Join(dataDir, name), []byte(contents[name]), 0o600))
+		}
+
+		require.NoError(t, RunMigrationIfNeeded(dataDir),
+			"seeded data dir must migrate without error")
+
+		defaultOrgDir := filepath.Join(dataDir, "orgs", "default")
+		for _, name := range filesToMigrate {
+			// File relocated into the default org dir with byte-identical contents.
+			moved := filepath.Join(defaultOrgDir, name)
+			got, err := os.ReadFile(moved)
+			require.NoError(t, err, "file %s should exist in default org dir", name)
+			assert.Equal(t, contents[name], string(got),
+				"migrated file %s must keep its original bytes", name)
+
+			// Original location is now a backward-compat symlink that resolves
+			// back to the relocated bytes.
+			linkInfo, err := os.Lstat(filepath.Join(dataDir, name))
+			require.NoError(t, err, "symlink %s should remain at data root", name)
+			assert.Equal(t, os.ModeSymlink, linkInfo.Mode()&os.ModeSymlink,
+				"%s should be a symlink after migration", name)
+			viaLink, err := os.ReadFile(filepath.Join(dataDir, name))
+			require.NoError(t, err)
+			assert.Equal(t, contents[name], string(viaLink),
+				"backward-compat symlink for %s must resolve to the moved file", name)
+		}
+
+		// Marker must exist, and re-checking must now report no migration needed.
+		_, err := os.Stat(filepath.Join(defaultOrgDir, ".migrated"))
+		require.NoError(t, err, "migration marker must be written")
+		assert.False(t, IsMigrationNeeded(dataDir),
+			"after a successful migration the data dir must no longer need migrating")
+	})
+
+	t.Run("migration failure is wrapped with run-migration context", func(t *testing.T) {
+		dataDir := t.TempDir()
+
+		// Seed a legacy file so IsMigrationNeeded returns true and execution
+		// reaches MigrateToMultiTenant.
+		require.NoError(t, os.WriteFile(filepath.Join(dataDir, "system.json"), []byte("{}"), 0o600))
+
+		// Make "orgs" a regular file rather than a directory, so MigrateToMultiTenant's
+		// os.MkdirAll(dataDir/orgs/default) fails with a real filesystem error
+		// (a path component is not a directory). This is the only deterministic,
+		// non-mock way to drive the error arm of RunMigrationIfNeeded.
+		require.NoError(t, os.WriteFile(filepath.Join(dataDir, "orgs"), []byte("not a dir"), 0o600))
+
+		err := RunMigrationIfNeeded(dataDir)
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "run multi-tenant migration",
+			"RunMigrationIfNeeded must wrap the underlying migration error with its own context")
+	})
+}

internal/mock/branchcov0723pm_test.go

@@ -0,0 +1,391 @@
+package mock
+
+import (
+	"math"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
+	"github.com/rcourtman/pulse-go-rewrite/internal/vmware"
+)
+
+// branchcov0723pmVMwareFixtureGraph builds a FixtureGraph carrying a single
+// controlled VMware host with one recent task and one recent event. It is the
+// sole input to (FixtureGraph).SupplementalChanges in these tests, so every
+// returned ResourceChange is attributable to known, inspectable fixture data
+// rather than whatever the default mock graph happens to contain.
+func branchcov0723pmVMwareFixtureGraph() FixtureGraph {
+	taskStarted := time.Date(2026, 7, 23, 10, 0, 0, 0, time.UTC)
+	taskCompleted := taskStarted.Add(90 * time.Second)
+	eventAt := time.Date(2026, 7, 23, 11, 30, 0, 0, time.UTC)
+
+	host := vmware.InventoryHost{
+		Host: "host-1",
+		Name: "host-one",
+		RecentTasks: []vmware.InventoryTask{{
+			Task:          "task-branchcov-1",
+			Name:          "Relocate VM",
+			State:         "success",
+			DescriptionID: "VirtualMachine.migrate",
+			StartedAt:     taskStarted,
+			CompletedAt:   taskCompleted,
+		}},
+		RecentEvents: []vmware.InventoryEvent{{
+			Event:     "event-branchcov-1",
+			Type:      "UserLoginSessionEvent",
+			Message:   "User administrator@vsphere.local logged in",
+			User:      "administrator@vsphere.local",
+			CreatedAt: eventAt,
+		}},
+	}
+
+	return FixtureGraph{
+		PlatformFixtures: PlatformFixtures{
+			VMware: vmware.InventorySnapshot{
+				ConnectionID: "vc-branchcov",
+				Hosts:        []vmware.InventoryHost{host},
+			},
+		},
+	}
+}
+
+// TestBranchcov0723pm_AvailabilityFixtures covers both branches of
+// AvailabilityFixtures (the mock-disabled nil return and the mock-enabled
+// catalog return) and asserts catalog invariants on the enabled result that
+// would catch a malformed fixture added later: a non-empty catalog, unique
+// identifiers, every required field populated, and the documented
+// relationships between fields (http probes carry a path, tcp probes carry a
+// port, service targets carry a linked resource, offline targets carry a
+// failure reason, online targets report a latency).
+func TestBranchcov0723pm_AvailabilityFixtures(t *testing.T) {
+	t.Run("disabled returns nil", func(t *testing.T) {
+		setMockEnabledForTest(t, false)
+		if got := AvailabilityFixtures(); got != nil {
+			t.Fatalf("AvailabilityFixtures() = %v, want nil when mock disabled", got)
+		}
+	})
+
+	t.Run("enabled catalog satisfies invariants", func(t *testing.T) {
+		setMockEnabledForTest(t, true)
+		fixtures := AvailabilityFixtures()
+
+		if len(fixtures) == 0 {
+			t.Fatal("expected a non-empty availability fixture catalog when mock enabled")
+		}
+
+		seenIDs := make(map[string]int, len(fixtures))
+		sawAvailable, sawUnavailable := false, false
+		validKinds := map[string]bool{"machine": true, "service": true, "device": true}
+		validProtocols := map[string]bool{"icmp": true, "tcp": true, "http": true}
+
+		for i, f := range fixtures {
+			target := f.Target
+
+			// Identity: non-empty, whitespace-normalized, globally unique.
+			if target.ID == "" {
+				t.Fatalf("fixture[%d]: Target.ID is empty", i)
+			}
+			if target.ID != strings.TrimSpace(target.ID) {
+				t.Fatalf("fixture[%d]: Target.ID %q is not trimmed", i, target.ID)
+			}
+			if prev := seenIDs[target.ID]; prev != 0 {
+				t.Fatalf("fixture[%d]: duplicate Target.ID %q (also at index %d)", i, target.ID, prev-1)
+			}
+			seenIDs[target.ID] = i + 1
+
+			// Required descriptive fields.
+			if target.Name == "" {
+				t.Fatalf("fixture[%d] %q: Target.Name is empty", i, target.ID)
+			}
+			if target.Address == "" {
+				t.Fatalf("fixture[%d] %q: Target.Address is empty", i, target.ID)
+			}
+
+			// Constrained enumerations.
+			if !validKinds[target.TargetKind] {
+				t.Fatalf("fixture[%d] %q: TargetKind %q is not a documented kind", i, target.ID, target.TargetKind)
+			}
+			if !validProtocols[target.Protocol] {
+				t.Fatalf("fixture[%d] %q: Protocol %q is not a documented probe", i, target.ID, target.Protocol)
+			}
+
+			// The default catalog ships every target enabled with sane
+			// polling cadence; a zero here would surface as a broken probe.
+			if !target.Enabled {
+				t.Fatalf("fixture[%d] %q: expected Enabled=true in the default catalog", i, target.ID)
+			}
+			if target.PollIntervalSecs <= 0 {
+				t.Fatalf("fixture[%d] %q: PollIntervalSecs = %d, want > 0", i, target.ID, target.PollIntervalSecs)
+			}
+			if target.TimeoutMillis <= 0 {
+				t.Fatalf("fixture[%d] %q: TimeoutMillis = %d, want > 0", i, target.ID, target.TimeoutMillis)
+			}
+			if target.FailureThreshold <= 0 {
+				t.Fatalf("fixture[%d] %q: FailureThreshold = %d, want > 0", i, target.ID, target.FailureThreshold)
+			}
+
+			// Documented protocol/field relationships.
+			if target.Protocol == "http" && strings.TrimSpace(target.Path) == "" {
+				t.Fatalf("fixture[%d] %q: http probe must carry a Path", i, target.ID)
+			}
+			if target.Protocol == "tcp" && target.Port <= 0 {
+				t.Fatalf("fixture[%d] %q: tcp probe must carry a Port > 0", i, target.ID)
+			}
+			if target.TargetKind == "service" && strings.TrimSpace(target.LinkedResourceID) == "" {
+				t.Fatalf("fixture[%d] %q: service target must reference a LinkedResourceID", i, target.ID)
+			}
+
+			// A probe must have run at least once.
+			if f.LastChecked.IsZero() {
+				t.Fatalf("fixture[%d] %q: LastChecked is zero", i, target.ID)
+			}
+
+			// Documented availability/state relationships.
+			if f.Available {
+				sawAvailable = true
+				if f.LatencyMillis <= 0 {
+					t.Fatalf("fixture[%d] %q: available target reports LatencyMillis = %d, want > 0", i, target.ID, f.LatencyMillis)
+				}
+			} else {
+				sawUnavailable = true
+				if f.ConsecutiveFailures < 1 {
+					t.Fatalf("fixture[%d] %q: unavailable target has ConsecutiveFailures = %d, want >= 1", i, target.ID, f.ConsecutiveFailures)
+				}
+				if strings.TrimSpace(f.LastError) == "" {
+					t.Fatalf("fixture[%d] %q: unavailable target must carry a LastError reason", i, target.ID)
+				}
+			}
+		}
+
+		// The catalog intentionally demonstrates both healthy and failing
+		// endpoints so the UI has something to render in each state.
+		if !sawAvailable {
+			t.Fatal("catalog has no available fixture; expected at least one online target")
+		}
+		if !sawUnavailable {
+			t.Fatal("catalog has no unavailable fixture; expected at least one offline target")
+		}
+	})
+}
+
+// TestBranchcov0723pm_FixtureGraphSupplementalChanges exercises every
+// DataSource arm of (FixtureGraph).SupplementalChanges, whose switch only
+// handles SourceVMware. It covers the handled VMware arm (including the
+// "vmware-vsphere" alias routed through normalizeSupplementalSource) and the
+// default nil-return for sources the switch does NOT handle (TrueNAS,
+// Availability), plus an unknown and an empty source. For the VMware arm it
+// asserts the concrete ResourceChange contents projected from the controlled
+// fixture task and event.
+func TestBranchcov0723pm_FixtureGraphSupplementalChanges(t *testing.T) {
+	graph := branchcov0723pmVMwareFixtureGraph()
+	const wantResourceID = "vc-branchcov:host:host-1"
+
+	t.Run("VMware arm projects task and event changes", func(t *testing.T) {
+		changes := graph.SupplementalChanges(unifiedresources.SourceVMware)
+		if len(changes) != 2 {
+			t.Fatalf("SupplementalChanges(SourceVMware) returned %d changes, want 2 (1 task + 1 event)", len(changes))
+		}
+
+		sawTask, sawEvent := false, false
+		for _, c := range changes {
+			// Canonical timeline contract for provider activity.
+			if c.Kind != unifiedresources.ChangeActivity {
+				t.Fatalf("change %q: Kind = %q, want %q", c.ID, c.Kind, unifiedresources.ChangeActivity)
+			}
+			if c.SourceType != unifiedresources.SourcePlatformEvent {
+				t.Fatalf("change %q: SourceType = %q, want %q", c.ID, c.SourceType, unifiedresources.SourcePlatformEvent)
+			}
+			if c.SourceAdapter != unifiedresources.AdapterVMware {
+				t.Fatalf("change %q: SourceAdapter = %q, want %q", c.ID, c.SourceAdapter, unifiedresources.AdapterVMware)
+			}
+			if c.Confidence != unifiedresources.ConfidenceHigh {
+				t.Fatalf("change %q: Confidence = %q, want %q", c.ID, c.Confidence, unifiedresources.ConfidenceHigh)
+			}
+
+			// Resource identity projected from the controlled host fixture.
+			if c.ResourceID != wantResourceID {
+				t.Fatalf("change %q: ResourceID = %q, want %q", c.ID, c.ResourceID, wantResourceID)
+			}
+
+			// Stable activity identifier and observed timestamp.
+			if !strings.HasPrefix(c.ID, "activity-") {
+				t.Fatalf("change ID %q must carry the activity- prefix", c.ID)
+			}
+			if c.ObservedAt.IsZero() {
+				t.Fatalf("change %q: ObservedAt is zero", c.ID)
+			}
+			if !c.ObservedAt.Equal(c.ObservedAt.UTC()) {
+				t.Fatalf("change %q: ObservedAt %s is not UTC-normalized", c.ID, c.ObservedAt)
+			}
+			if strings.TrimSpace(c.Reason) == "" {
+				t.Fatalf("change %q: Reason is empty", c.ID)
+			}
+
+			// Provider context preserved verbatim from the fixture.
+			if c.Metadata["vmwareConnectionId"] != "vc-branchcov" {
+				t.Fatalf("change %q: vmwareConnectionId = %v, want vc-branchcov", c.ID, c.Metadata["vmwareConnectionId"])
+			}
+			if c.Metadata["vmwareEntityType"] != "host" {
+				t.Fatalf("change %q: vmwareEntityType = %v, want host", c.ID, c.Metadata["vmwareEntityType"])
+			}
+			if c.Metadata["vmwareManagedObjectId"] != "host-1" {
+				t.Fatalf("change %q: vmwareManagedObjectId = %v, want host-1", c.ID, c.Metadata["vmwareManagedObjectId"])
+			}
+
+			switch c.Metadata["activity_type"] {
+			case "vmware_task":
+				sawTask = true
+				if c.Metadata["vmwareTask"] != "task-branchcov-1" {
+					t.Fatalf("task change %q: vmwareTask = %v, want task-branchcov-1", c.ID, c.Metadata["vmwareTask"])
+				}
+			case "vmware_event":
+				sawEvent = true
+				if c.Metadata["vmwareEvent"] != "event-branchcov-1" {
+					t.Fatalf("event change %q: vmwareEvent = %v, want event-branchcov-1", c.ID, c.Metadata["vmwareEvent"])
+				}
+			default:
+				t.Fatalf("change %q: unexpected activity_type %v", c.ID, c.Metadata["activity_type"])
+			}
+		}
+
+		if !sawTask {
+			t.Fatal("expected one vmware_task change projected from the host RecentTask")
+		}
+		if !sawEvent {
+			t.Fatal("expected one vmware_event change projected from the host RecentEvent")
+		}
+	})
+
+	t.Run("vmware-vsphere alias routes to the VMware arm", func(t *testing.T) {
+		alias := graph.SupplementalChanges(unifiedresources.DataSource("vmware-vsphere"))
+		if len(alias) != 2 {
+			t.Fatalf("vmware-vsphere alias returned %d changes, want 2", len(alias))
+		}
+	})
+
+	t.Run("switch does not handle TrueNAS", func(t *testing.T) {
+		if got := graph.SupplementalChanges(unifiedresources.SourceTrueNAS); got != nil {
+			t.Fatalf("SupplementalChanges(SourceTrueNAS) = %v, want nil (no switch arm)", got)
+		}
+	})
+
+	t.Run("switch does not handle Availability", func(t *testing.T) {
+		if got := graph.SupplementalChanges(unifiedresources.SourceAvailability); got != nil {
+			t.Fatalf("SupplementalChanges(SourceAvailability) = %v, want nil (no switch arm)", got)
+		}
+	})
+
+	t.Run("unknown source hits default arm", func(t *testing.T) {
+		if got := graph.SupplementalChanges(unifiedresources.DataSource("kubernetes")); got != nil {
+			t.Fatalf("SupplementalChanges(unknown) = %v, want nil via default arm", got)
+		}
+	})
+
+	t.Run("empty and whitespace-only source normalize away and hit default arm", func(t *testing.T) {
+		if got := graph.SupplementalChanges(unifiedresources.DataSource("")); got != nil {
+			t.Fatalf("SupplementalChanges(\"\") = %v, want nil via default arm", got)
+		}
+		if got := graph.SupplementalChanges(unifiedresources.DataSource("   ")); got != nil {
+			t.Fatalf("SupplementalChanges(whitespace) = %v, want nil via default arm", got)
+		}
+	})
+
+	t.Run("empty graph yields no VMware changes", func(t *testing.T) {
+		empty := FixtureGraph{}
+		if got := empty.SupplementalChanges(unifiedresources.SourceVMware); len(got) != 0 {
+			t.Fatalf("empty graph SupplementalChanges(SourceVMware) = %d changes, want 0", len(got))
+		}
+	})
+}
+
+// TestBranchcov0723pm_GenerateMockHostRate covers every arm of the
+// generateMockHostRate switch. generateMockHostRate first delegates to
+// generateRealisticIO and only falls through to its own switch when that
+// helper reports an idle (zero) rate, and the package-global math/rand source
+// cannot be made deterministic without re-implementing the helper, so these
+// subtests assert the documented OUTPUT BANDS rather than exact values:
+//
+//   - For an unknown ioType, generateRealisticIO has no case and returns 0,
+//     so ONLY the default switch arm can run. Its band is tight and exclusive:
+//     (32 + Intn(512)) * 1024 -> [32768, 556032].
+//
+//   - For each known ioType the observable output is the union of a non-idle
+//     generateRealisticIO value (which the function returns directly) and the
+//     switch fallback (used when generateRealisticIO is idle). The asserted
+//     band is therefore [switch-fallback min, generateRealisticIO active max].
+//
+// Every code path produces a whole number of bytes that is an exact multiple
+// of 1024 (switch arms multiply by 1024; generateRealisticIO emits multiples
+// of 1024*1024 or 1024*1024/8), so that structural invariant is checked too.
+func TestBranchcov0723pm_GenerateMockHostRate(t *testing.T) {
+	const samples = 4000
+
+	multipleOf1024 := func(rate float64) bool {
+		return math.Mod(rate, 1024) == 0
+	}
+
+	t.Run("unknown ioType lands exclusively in the default arm band", func(t *testing.T) {
+		const (
+			minBand = float64(32 * 1024)         // (32 + 0) * 1024
+			maxBand = float64((32 + 511) * 1024) // (32 + 511) * 1024
+			midBand = float64((32 + 256) * 1024) // midpoint of the arm's range
+		)
+		belowMid, aboveMid := false, false
+		for i := 0; i < samples; i++ {
+			rate := generateMockHostRate("unknown-io-type")
+			if rate < minBand || rate > maxBand {
+				t.Fatalf("sample %d: rate %g outside default band [%g, %g]", i, rate, minBand, maxBand)
+			}
+			if !multipleOf1024(rate) {
+				t.Fatalf("sample %d: rate %g is not a multiple of 1024", i, rate)
+			}
+			if rate < midBand {
+				belowMid = true
+			} else {
+				aboveMid = true
+			}
+		}
+		// The default arm's Intn(512) should span its full range; seeing
+		// values on both sides of the midpoint proves the arm is live and
+		// not clamped to a single bucket.
+		if !belowMid || !aboveMid {
+			t.Fatalf("default arm did not span its range: belowMid=%v aboveMid=%v", belowMid, aboveMid)
+		}
+	})
+
+	// Each known ioType: [switch-fallback min, generateRealisticIO active max].
+	knownBands := []struct {
+		name    string
+		minBand float64
+		maxBand float64
+	}{
+		// network-in: switch (128 + Intn(4096))*1024 floor; realisticIO high
+		// ceiling (100 + Intn(400)) * 1024*1024/8 = 499 * 131072.
+		{"network-in", float64(128 * 1024), float64(499 * 1024 * 1024 / 8)},
+		// network-out: switch (96 + Intn(3072))*1024 floor; realisticIO high
+		// ceiling (50 + Intn(200)) * 1024*1024/8 = 249 * 131072.
+		{"network-out", float64(96 * 1024), float64(249 * 1024 * 1024 / 8)},
+		// disk-read: switch (64 + Intn(2048))*1024 floor; realisticIO high
+		// ceiling (25 + Intn(75)) * 1024*1024 = 99 * 1048576.
+		{"disk-read", float64(64 * 1024), float64(99 * 1024 * 1024)},
+		// disk-write: switch (32 + Intn(1536))*1024 floor; realisticIO high
+		// ceiling (18 + Intn(32)) * 1024*1024 = 49 * 1048576.
+		{"disk-write", float64(32 * 1024), float64(49 * 1024 * 1024)},
+	}
+
+	for _, kb := range knownBands {
+		kb := kb
+		t.Run(kb.name+" stays within its output band", func(t *testing.T) {
+			for i := 0; i < samples; i++ {
+				rate := generateMockHostRate(kb.name)
+				if rate < kb.minBand || rate > kb.maxBand {
+					t.Fatalf("%s sample %d: rate %g outside band [%g, %g]", kb.name, i, rate, kb.minBand, kb.maxBand)
+				}
+				if !multipleOf1024(rate) {
+					t.Fatalf("%s sample %d: rate %g is not a multiple of 1024", kb.name, i, rate)
+				}
+			}
+		})
+	}
+}

internal/servicediscovery/branchcov0723pm_test.go

@@ -0,0 +1,200 @@
+package servicediscovery
+
+import (
+	"testing"
+)
+
+// TestBranchcov0723pmNeedsDeepScan_Branches exercises every return arm of
+// (*Service).needsDeepScan (service.go:525). needsDeepScan is a pure
+// predicate: it never reads receiver state, so a zero-value &Service{} is
+// sufficient and no network/SSH/daemon/database rig is required.
+//
+// Arms covered:
+//   - nil discovery                                  -> return true
+//   - non-empty RawCommandOutput (short-circuit)     -> return false
+//   - Confidence < 0.7                               -> return true
+//   - Confidence == 0.7 boundary (skips conf arm)    -> falls through
+//   - ServiceType == ""                              -> return true
+//   - ServiceType == "unknown"                       -> return true
+//   - all of Facts/ConfigPaths/LogPaths empty        -> return true
+//   - final fall-through (at least one path present) -> return false
+func TestBranchcov0723pmNeedsDeepScan_Branches(t *testing.T) {
+	s := &Service{} // zero-value receiver; needsDeepScan never dereferences s
+
+	cases := []struct {
+		name      string
+		discovery *ResourceDiscovery
+		want      bool
+	}{
+		{
+			name:      "nil-discovery-returns-true",
+			discovery: nil,
+			want:      true,
+		},
+		{
+			// RawCommandOutput short-circuits before the confidence and
+			// service-type checks, so even a zero-confidence, empty-type
+			// discovery must return false here.
+			name: "raw-command-output-present-short-circuits-to-false",
+			discovery: &ResourceDiscovery{
+				RawCommandOutput: map[string]string{"uname -a": "Linux node 6.1.0"},
+				Confidence:       0.0,
+				ServiceType:      "",
+			},
+			want: false,
+		},
+		{
+			name: "confidence-well-below-threshold-returns-true",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.5,
+				ServiceType: "postgres",
+			},
+			want: true,
+		},
+		{
+			name: "confidence-just-below-threshold-returns-true",
+			// 0.69 as a float64 is strictly less than the 0.7 literal used
+			// in the source, so the confidence arm fires.
+			discovery: &ResourceDiscovery{
+				Confidence:  0.69,
+				ServiceType: "postgres",
+			},
+			want: true,
+		},
+		{
+			// Boundary: Confidence == 0.7 is NOT < 0.7, so the confidence
+			// arm is skipped. With a known service type and a non-empty
+			// path, every other guard is also cleared and the function
+			// reaches the final `return false`.
+			name: "confidence-exactly-at-threshold-skips-confidence-arm",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.7,
+				ServiceType: "postgres",
+				ConfigPaths: []string{"/etc/postgresql/15/main/postgresql.conf"},
+			},
+			want: false,
+		},
+		{
+			// Boundary corroboration: Confidence == 0.7 skips the confidence
+			// arm, then the empty ServiceType arm fires. If 0.7 were treated
+			// as < 0.7 this would be indistinguishable; pairing it with the
+			// previous case pins the boundary precisely.
+			name: "confidence-exactly-at-threshold-then-empty-service-type-returns-true",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.7,
+				ServiceType: "",
+			},
+			want: true,
+		},
+		{
+			name: "empty-service-type-returns-true",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.9,
+				ServiceType: "",
+			},
+			want: true,
+		},
+		{
+			name: "literal-unknown-service-type-returns-true",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.9,
+				ServiceType: "unknown",
+			},
+			want: true,
+		},
+		{
+			// Case-sensitivity check: "Unknown" != "unknown", so the
+			// service-type guard is cleared and with a path present the
+			// function falls through to false. This is an observable
+			// behaviour, not a restatement of the comparison.
+			name: "capitalized-unknown-is-not-unknown-and-falls-through",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.9,
+				ServiceType: "Unknown",
+				ConfigPaths: []string{"/etc/foo.conf"},
+			},
+			want: false,
+		},
+		{
+			name: "all-paths-empty-returns-true",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.9,
+				ServiceType: "postgres",
+				// Facts, ConfigPaths, LogPaths all zero-length
+			},
+			want: true,
+		},
+		{
+			name: "only-facts-non-empty-returns-false",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.9,
+				ServiceType: "postgres",
+				Facts:       []DiscoveryFact{{Key: "arch", Value: "amd64"}},
+			},
+			want: false,
+		},
+		{
+			name: "only-configpaths-non-empty-returns-false",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.9,
+				ServiceType: "postgres",
+				ConfigPaths: []string{"/etc/postgresql/postgresql.conf"},
+			},
+			want: false,
+		},
+		{
+			name: "only-logpaths-non-empty-returns-false",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.9,
+				ServiceType: "postgres",
+				LogPaths:    []string{"/var/log/postgresql/postgresql.log"},
+			},
+			want: false,
+		},
+		{
+			name: "all-three-paths-non-empty-returns-false",
+			discovery: &ResourceDiscovery{
+				Confidence:  0.9,
+				ServiceType: "postgres",
+				Facts:       []DiscoveryFact{{Key: "arch", Value: "amd64"}},
+				ConfigPaths: []string{"/etc/postgresql/postgresql.conf"},
+				LogPaths:    []string{"/var/log/postgresql/postgresql.log"},
+			},
+			want: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := s.needsDeepScan(tc.discovery)
+			if got != tc.want {
+				t.Fatalf("needsDeepScan(%+v) = %v, want %v", tc.discovery, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestBranchcov0723pmNeedsDeepScan_NilReceiverPurity asserts the purity
+// guarantee that needsDeepScan never dereferences its receiver: a nil *Service
+// must not panic and must behave identically to a zero-value receiver. This is
+// an observable property of the implementation, not a restatement of it.
+func TestBranchcov0723pmNeedsDeepScan_NilReceiverPurity(t *testing.T) {
+	var s *Service // nil receiver
+
+	t.Run("nil-receiver-nil-discovery-returns-true", func(t *testing.T) {
+		if !s.needsDeepScan(nil) {
+			t.Fatal("nil receiver + nil discovery must return true")
+		}
+	})
+
+	t.Run("nil-receiver-healthy-discovery-returns-false", func(t *testing.T) {
+		d := &ResourceDiscovery{
+			Confidence:  0.95,
+			ServiceType: "redis",
+			ConfigPaths: []string{"/etc/redis/redis.conf"},
+		}
+		if s.needsDeepScan(d) {
+			t.Fatal("nil receiver + healthy discovery must return false")
+		}
+	})
+}