Author Proxmox VM/CT lifecycle preflight context for approval review

Pulse's primary monitored platform is Proxmox, but the per-command-class
preflight catalog only covered systemd services, Docker containers, and
Kubernetes deployments. Operators approving a Patrol-proposed
qm restart/qm stop/qm shutdown saw the generic preflight without the
operational nuance that distinguishes those verbs (qm stop is a hard halt,
qm shutdown is graceful with a 60s ACPI timeout, etc.).

Adds eight new classes covering qm and pct lifecycle (reboot/restart,
stop, start, shutdown). Each gets hand-authored safety and verification
copy that names the actual semantics — pct stop is destructive vs pct
shutdown's lxc-attach handoff — so the operator sees concrete context at
approval time.

Broker-level VerificationCommandForCommand intentionally does not derive
qm status / pct status from these classes — pulse_control's
verifyGuestAction already runs those checks at the tool layer, so adding
a parallel broker dispatch would double-run. The preflight copy still
names what the tool-layer verification will read.
This commit is contained in:
rcourtman
2026-05-09 13:12:52 +01:00
parent 8504d97c5b
commit f99bce7ee4
3 changed files with 150 additions and 11 deletions
@@ -321,15 +321,23 @@ runtime cost control, and shared AI transport surfaces.
`approvalCommandClassPreflightAdditions` in
`internal/ai/tools/tools_control.go` bucket common Pulse remediation
actions (service-restart, service-stop, service-start, service-reload,
container-restart, container-stop, k8s-rollout-restart) and return
hand-authored operational copy: what the command actually touches,
how Pulse will read back success. The additions append onto the
default safety/verification arrays rather than replacing them, so
the broker's structural posture (org scope, hash match, single-use
container-restart, container-stop, k8s-rollout-restart, plus the
Proxmox VM lifecycle classes proxmox-vm-reboot, proxmox-vm-stop,
proxmox-vm-start, proxmox-vm-shutdown and the matching pct-driven
proxmox-ct-* container lifecycle classes) and return hand-authored
operational copy: what the command actually touches, how Pulse will
read back success. The additions append onto the default
safety/verification arrays rather than replacing them, so the
broker's structural posture (org scope, hash match, single-use
approval) remains visible alongside the class-specific copy.
Unknown command classes must return empty additions rather than
fabricated padding — operators see only the default content, not
invented assertions about what an unrecognized command will do.
The Proxmox classes intentionally do not derive a broker-level
`VerificationCommandForCommand` check because pulse_control's
`verifyGuestAction` already runs `qm status` / `pct status` at the
tool layer; adding a parallel broker dispatch would double-run the
same read-after-write check.
Drift refusal must also persist a Failed audit record with the
Request, Plan, and Approvals snapshots intact and a Result whose
ErrorMessage is prefixed `plan_drift:` so the audit trail shows
@@ -24,6 +24,23 @@ func TestClassifyApprovalCommand_BucketsKnownCommandShapes(t *testing.T) {
{"podman restart", "agent", "podman restart pihole", "container-restart"},
{"docker stop", "docker", "docker stop oldcontainer", "container-stop"},
{"kubectl rollout restart", "kubernetes", "kubectl rollout restart deployment/api", "k8s-rollout-restart"},
// Proxmox VM lifecycle classes — Pulse's primary monitored platform.
// Each verb has distinct semantics so they map to distinct classes.
{"qm reboot", "agent", "qm reboot 101", "proxmox-vm-reboot"},
{"qm restart alias", "agent", "qm restart 101", "proxmox-vm-reboot"},
{"qm stop hard halt", "agent", "qm stop 101", "proxmox-vm-stop"},
{"qm start", "agent", "qm start 101", "proxmox-vm-start"},
{"qm shutdown graceful", "agent", "qm shutdown 101", "proxmox-vm-shutdown"},
// Proxmox LXC container lifecycle classes.
{"pct reboot", "agent", "pct reboot 200", "proxmox-ct-reboot"},
{"pct restart alias", "agent", "pct restart 200", "proxmox-ct-reboot"},
{"pct stop hard halt", "agent", "pct stop 200", "proxmox-ct-stop"},
{"pct start", "agent", "pct start 200", "proxmox-ct-start"},
{"pct shutdown graceful", "agent", "pct shutdown 200", "proxmox-ct-shutdown"},
// Negative cases: tool prefix without a recognized verb must NOT
// classify (defends against `qm migrate`, `pct destroy`, etc.).
{"qm migrate not a lifecycle verb", "agent", "qm migrate 101 pve-2", ""},
{"pct destroy not a lifecycle verb", "agent", "pct destroy 200", ""},
{"unknown free-form command", "agent", "echo hello world", ""},
{"empty command", "agent", "", ""},
}
@@ -48,6 +65,15 @@ func TestApprovalCommandClassPreflightAdditions_AuthorsConcreteContextForKnownCl
{"service-stop warns dependent services", "systemctl stop postgres", "dependent services", "inactive"},
{"container-restart names docker inspect", "docker restart homepage", "briefly unavailable", "docker inspect"},
{"k8s rollout names rollout status", "kubectl rollout restart deployment/api", "PodDisruptionBudget", "rollout status"},
// Proxmox classes must concretely warn about the destructive vs
// graceful split (qm stop = hard halt, qm shutdown = graceful) and
// name the canonical `qm status` / `pct status` read-after-write
// check the operator should see post-dispatch.
{"qm reboot names qm status", "qm reboot 101", "ACPI shutdown", "qm status"},
{"qm stop warns it is hard not graceful", "qm stop 101", "hard stop", "stopped"},
{"qm shutdown names timeout fallback", "qm shutdown 101", "ACPI shutdown", "qm status"},
{"pct stop warns it is hard not graceful", "pct stop 200", "hard stop", "stopped"},
{"pct shutdown names lxc-attach", "pct shutdown 200", "lxc-attach", "pct status"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
@@ -92,6 +118,17 @@ func TestVerificationCommandForCommand_DerivesPerClassReadAfterWriteCheck(t *tes
// the tool layer, so adding a broker-level dispatch would double-run.
{"docker restart deferred to tool layer", "docker", "docker restart homepage", "", false},
{"podman restart deferred to tool layer", "agent", "podman restart pihole", "", false},
// Proxmox classes are intentionally excluded from broker-level
// verification — pulse_control's verifyGuestAction already runs
// `qm status` and `pct status` at the tool layer, so adding a
// parallel broker-level dispatch would double-run the same check.
// The preflight copy still names the verification narrative; only
// the broker-side derivation is suppressed.
{"qm reboot deferred to pulse_control", "agent", "qm reboot 101", "", false},
{"qm stop deferred to pulse_control", "agent", "qm stop 101", "", false},
{"qm shutdown deferred to pulse_control", "agent", "qm shutdown 101", "", false},
{"pct stop deferred to pulse_control", "agent", "pct stop 200", "", false},
{"pct shutdown deferred to pulse_control", "agent", "pct shutdown 200", "", false},
{"unknown command", "agent", "echo hello", "", false},
{"systemctl with single-quote in unit", "agent", `systemctl restart nasty'name`, "systemctl is-active 'nasty'\\''name'", true},
}
+100 -6
View File
@@ -1525,12 +1525,19 @@ func approvalPreflight(req *approval.ApprovalRequest) *approval.ActionPreflight
// classes without a derivable check; the broker must skip verification
// rather than fabricate one.
//
// Container classes (container-restart, container-stop) are intentionally
// excluded here: pulse_docker already runs its own per-container
// `docker inspect` verification at the tool layer, and adding a parallel
// broker-level dispatch would double-run the same check. If the tool layer
// stops doing its own verification, this function should grow a docker
// branch using `extractContainerName`.
// Tool-layer-verified classes are intentionally excluded here:
// - Container classes (container-restart, container-stop) are verified
// by pulse_docker via per-container `docker inspect` at the tool
// layer.
// - Proxmox VM/CT classes (proxmox-vm-*, proxmox-ct-*) are verified by
// pulse_control via `verifyGuestAction` (which dispatches `qm status`
// or `pct status` already) at the tool layer.
//
// Adding a parallel broker-level dispatch for these would double-run the
// same check. The preflight copy authored by
// approvalCommandClassPreflightAdditions still names what the
// tool-layer verification will read so the operator-facing narrative
// stays accurate.
func VerificationCommandForCommand(targetType, command string) (string, bool) {
class := classifyApprovalCommand(targetType, command)
switch class {
@@ -1627,6 +1634,29 @@ func classifyApprovalCommand(targetType, command string) string {
return "container-stop"
case strings.HasPrefix(cmd, "kubectl rollout restart"):
return "k8s-rollout-restart"
// Proxmox VM lifecycle (qm) — Pulse's primary monitored platform. Each
// verb has distinct operational semantics (graceful shutdown vs hard
// stop vs reboot) so they map to distinct classes rather than being
// folded into the generic service-* buckets.
case strings.HasPrefix(cmd, "qm reboot ") || strings.HasPrefix(cmd, "qm restart "):
return "proxmox-vm-reboot"
case strings.HasPrefix(cmd, "qm stop "):
return "proxmox-vm-stop"
case strings.HasPrefix(cmd, "qm start "):
return "proxmox-vm-start"
case strings.HasPrefix(cmd, "qm shutdown "):
return "proxmox-vm-shutdown"
// Proxmox LXC container lifecycle (pct). Same per-verb split as qm
// because pct shutdown initiates an in-guest shutdown via lxc-attach
// while pct stop is an immediate halt.
case strings.HasPrefix(cmd, "pct reboot ") || strings.HasPrefix(cmd, "pct restart "):
return "proxmox-ct-reboot"
case strings.HasPrefix(cmd, "pct stop "):
return "proxmox-ct-stop"
case strings.HasPrefix(cmd, "pct start "):
return "proxmox-ct-start"
case strings.HasPrefix(cmd, "pct shutdown "):
return "proxmox-ct-shutdown"
}
return ""
}
@@ -1695,6 +1725,70 @@ func approvalCommandClassPreflightAdditions(targetType, command string) (safetyC
"Watch `kubectl rollout status` until the deployment converges.",
"Verify pod readiness with `kubectl get pods -l <selector>` after rollout.",
}
case "proxmox-vm-reboot":
return []string{
"`qm reboot` performs an ACPI shutdown followed by a start; in-guest workloads see a clean OS reboot.",
"VM RAM state is not preserved (no live migration); guests with non-persistent state will lose it.",
}, []string{
"Read back `qm status <vmid>` and confirm `status: running`.",
"Verify the guest's monitoring agent reconnects (Pulse will detect and update the resource state).",
}
case "proxmox-vm-stop":
return []string{
"`qm stop` is an immediate hard stop, NOT a graceful shutdown — guest filesystems may be left dirty.",
"Use `qm shutdown` instead if the workload needs to flush state cleanly; only approve `qm stop` when the guest is unresponsive.",
}, []string{
"Read back `qm status <vmid>` and confirm `status: stopped`.",
"Verify no auto-start policy will immediately restart the VM.",
}
case "proxmox-vm-start":
return []string{
"VM will boot from disk with the configuration currently on the host; uncommitted config drift will take effect.",
"Boot order, attached disks, and network bridges all apply as currently defined — verify before approving.",
}, []string{
"Read back `qm status <vmid>` and confirm `status: running`.",
"Tail `journalctl -u qmeventd` for the start lifecycle event.",
}
case "proxmox-vm-shutdown":
return []string{
"`qm shutdown` issues an ACPI shutdown to the guest and waits for the VM to power off cleanly.",
"Default timeout is 60s; an unresponsive guest will force a stop after the timeout, leaving filesystems in the same state as `qm stop`.",
}, []string{
"Read back `qm status <vmid>` and confirm `status: stopped`.",
"Verify the shutdown was clean (exit code 0) rather than a forced stop after timeout.",
}
case "proxmox-ct-reboot":
return []string{
"`pct reboot` performs an in-guest reboot; container processes restart while the LXC instance stays managed by Proxmox.",
"Mounted bind-mounts and shared volumes persist through the reboot; only in-memory state is lost.",
}, []string{
"Read back `pct status <ctid>` and confirm `status: running`.",
"Verify the container's foreground service has come back up before treating the reboot as complete.",
}
case "proxmox-ct-stop":
return []string{
"`pct stop` is an immediate hard stop — in-guest processes do not get a chance to flush state cleanly.",
"Use `pct shutdown` instead for graceful in-guest shutdown; reserve `pct stop` for unresponsive containers.",
}, []string{
"Read back `pct status <ctid>` and confirm `status: stopped`.",
"Verify no on-boot auto-start policy will immediately restart the container.",
}
case "proxmox-ct-start":
return []string{
"Container will start with the configuration currently on the host; uncommitted config drift takes effect.",
"Mounted volumes, network bridges, and resource limits all apply as currently defined.",
}, []string{
"Read back `pct status <ctid>` and confirm `status: running`.",
"Verify the container's foreground service is reachable from the host network.",
}
case "proxmox-ct-shutdown":
return []string{
"`pct shutdown` issues a graceful in-guest shutdown via lxc-attach and waits for the container to halt.",
"Default timeout falls back to a hard stop on unresponsive containers, leaving processes in the same state as `pct stop`.",
}, []string{
"Read back `pct status <ctid>` and confirm `status: stopped`.",
"Verify the shutdown was clean rather than a forced stop after timeout.",
}
}
return nil, nil
}