FM-602: Proxmox guest lifecycle and task operations

[Andreas-Froyland]

Closes #103 (FM-602).

Implementation approach (from the issue)

FM-600/FM-601 delivered the trust flow and the read layer; this PR adds the first Proxmox mutations as durable operations.

What landed

• Provider (fleet-provider-proxmox): guest_lifecycle (start/stop/shutdown/reboot → the parsed UPID) and task_status on the ProxmoxSource port. Upid::parse is Fleet-owned (UPID:node:pid:pstart:starttime:type:id:user: — the node it polls comes from the parse, never from trust in the caller; malformed shapes refuse with bounded details). TaskStatus is an honest enum: Running/Ok/Error{detail}/Unknown — an unknown task is honest uncertainty, never assumed success (PVE rotates old task entries out).
• Executor kinds (proxmox.guest.start|stop|shutdown|reboot): registered in CREATABLE_KINDS (31 → 35) with a new proxmox.operate permission (catalog 38 → 39). The kinds are catalog-level like the source kinds — a Proxmox guest is not a Fleet machine, so the permission is enforced with resource: None on both the dedicated endpoint and the generic surface (the generic-surface routing previously required a machineId for any Some-permission kind; the catalog-level branch is now explicit in Operations::create, which also fixes the latent gap for the source kinds).
• Executor (fleet-controller/src/proxmox_exec.rs): ProxmoxLifecycleExecutor — resolves the account through the same explicit-trust gate (unconfirmed fingerprint refuses before any network call), runs the action, and polls the UPID on a fixed 2 s interval against a deadline (a sleep between polls, never a wall-clock race — the legacy waitForTask flake is exactly what this avoids). Terminal mapping: task OK → succeeded; ERROR → failed with the bounded detail; deadline expiry → failed naming that the final state is unknown; unreadable status → failed as task_unknown. Cancellation stops the waiting, not the remote task — PVE keeps running the action and the operation records that honestly; remote task cancellation is deferred to epic M6 epic: Template, clone, and snapshot operations #12's review. ProxmoxDispatch routes the lifecycle kinds in the worker chain; a controller without a secret store composes an absent credential store so lifecycle operations fail honestly.
• Surfaces: POST /api/v1/proxmox/accounts/{id}/guests/{vmid}/{action} (202, durable operation, idempotency-key supported, malformed actions refuse as 400) in the OpenAPI doc (client regenerated); fleetctl proxmox start|stop|shutdown|reboot --account --node --vmid [--wait] [--timeout].
• Authz/audit: every lifecycle operation is a durable operation with the standard audit path; proxmox.operate is checked at the API before the operation is created and re-applied at the account boundary in the executor.

Tests

• Provider unit tests: UPID parsing (well-formed/malformed/trailing), lifecycle action id round-trips.
• Controller e2e through the real worker: task-OK → succeeded with taskState: ok; task-ERROR → failed with the PVE detail surfaced; deadline expiry with an always-running task → failed naming the uncertainty; unrecognized action → 400.
• CLI parse contract tests for all four verbs, --wait/--timeout, and the refusals. cargo xtask verify passes; live smoke remains on the epic's real-cluster checklist.

Non-goals (respected)

Destructive operations (epic #12); guest OS provisioning; remote task cancellation.

---

Summary by cubic

Adds the first Proxmox mutations as durable operations: guest lifecycle start/stop/shutdown/reboot and UPID task-status polling, closing #103 (FM-602).

The new proxmox.guest.start|stop|shutdown|reboot kinds are catalog-level like the source kinds — a Proxmox guest is not a Fleet machine — and gated by a new proxmox.operate permission enforced at the API and re-applied at the account boundary in the executor. fleet-controller's ProxmoxLifecycleExecutor resolves the account through the same explicit-trust gate as the read layer, runs the action, and polls the task on a fixed 2-second interval against a deadline, avoiding the legacy waitForTask wall-clock race. Terminal states are honest: task OK succeeds, ERROR fails with the PVE detail, and a deadline expiry or unknown status fails naming the uncertainty — never assumed success. Cancellation stops the waiting only; PVE keeps running the remote task and the operation records that honestly. A controller without a secret store composes an absent credential store so lifecycle operations fail cleanly.

Surfaces the operations through POST /api/v1/proxmox/accounts/{accountId}/guests/{vmid}/{action} (202, durable, idempotency-key supported, malformed actions 400) and fleetctl proxmox start|stop|shutdown|reboot --account --node --vmid [--wait] [--timeout]. Destructive operations and remote task cancellation are deferred to epic #12's review.

• The provider's request type now carries a method field so lifecycle mutations use POST while reads stay GET.
• Lifecycle action ids round-trip through the provider, and obstacle-free UPID parsing is bounded to well-formed 8-field strings.
• The generic create path now branches on catalog-level kinds explicitly, fixing a latent authorization gap for the source kinds.

^{Written for commit 19db6f4. Summary will update on new commits.}

[cubic-dev-ai]

3 issues found across 15 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/fleetctl/src/lib.rs">

<violation number="1" location="crates/fleetctl/src/lib.rs:1226">
P3: The parser accepts the four new lifecycle commands, but `usage()` still omits them; add their syntax so help and error messages expose the new interface.</violation>
</file>

<file name="crates/fleet-controller/src/proxmox_exec.rs">

<violation number="1" location="crates/fleet-controller/src/proxmox_exec.rs:141">
P1: When the lifecycle request or a poll interval crosses `timeoutSeconds`, this executor can still accept a later `Ok` status because the deadline is not checked before that terminal return. Start the deadline before the mutation and bound/check each poll and sleep against the remaining time.</violation>
</file>

<file name="crates/fleet-controller/tests/proxmox.rs">

<violation number="1" location="crates/fleet-controller/tests/proxmox.rs:132">
P3: The `_worker` field is never populated: both `harness_with` and `lifecycle_harness` construct `Harness` with `_worker: None`, and its doc comment claims it is "aborted on drop." The only real worker handle is the local `_worker_handle` in `lifecycle_harness`, which goes out of scope at the end of the constructor; dropping a tokio `JoinHandle` detaches the task, it does not abort it. Store the handle in `_worker` and abort it in `Drop` (which currently only shuts down the HTTP server), or remove the field and correct the comment.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: When the lifecycle request or a poll interval crosses timeoutSeconds, this executor can still accept a later Ok status because the deadline is not checked before that terminal return. Start the deadline before the mutation and bound/check each poll and sleep against the remaining time.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At crates/fleet-controller/src/proxmox_exec.rs, line 141:

<comment>When the lifecycle request or a poll interval crosses `timeoutSeconds`, this executor can still accept a later `Ok` status because the deadline is not checked before that terminal return. Start the deadline before the mutation and bound/check each poll and sleep against the remaining time.</comment>

<file context>
@@ -0,0 +1,315 @@
+            .guest_lifecycle(request.clone(), node, vmid, action)
+            .await
+            .map_err(|error| format!("the lifecycle action failed: {error}"))?;
+        let started = std::time::Instant::now();
+        loop {
+            let status = self
</file context>

[cubic-dev-ai]

P3: The parser accepts the four new lifecycle commands, but usage() still omits them; add their syntax so help and error messages expose the new interface.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At crates/fleetctl/src/lib.rs, line 1226:

<comment>The parser accepts the four new lifecycle commands, but `usage()` still omits them; add their syntax so help and error messages expose the new interface.</comment>

<file context>
@@ -1208,6 +1223,77 @@ fn parse_proxmox_command(verb: &str, rest: &[&str]) -> Result<Command, CliError>
             }
             _ => Err(CliError { message: usage() }),
         },
+        "start" | "stop" | "shutdown" | "reboot" => {
+            let mut account_id = None;
+            let mut node = None;
</file context>

[cubic-dev-ai]

P3: The _worker field is never populated: both harness_with and lifecycle_harness construct Harness with _worker: None, and its doc comment claims it is "aborted on drop." The only real worker handle is the local _worker_handle in lifecycle_harness, which goes out of scope at the end of the constructor; dropping a tokio JoinHandle detaches the task, it does not abort it. Store the handle in _worker and abort it in Drop (which currently only shuts down the HTTP server), or remove the field and correct the comment.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At crates/fleet-controller/tests/proxmox.rs, line 132:

<comment>The `_worker` field is never populated: both `harness_with` and `lifecycle_harness` construct `Harness` with `_worker: None`, and its doc comment claims it is "aborted on drop." The only real worker handle is the local `_worker_handle` in `lifecycle_harness`, which goes out of scope at the end of the constructor; dropping a tokio `JoinHandle` detaches the task, it does not abort it. Store the handle in `_worker` and abort it in `Drop` (which currently only shuts down the HTTP server), or remove the field and correct the comment.</comment>

<file context>
@@ -128,6 +128,8 @@ struct Harness {
     address: std::net::SocketAddr,
     shutdown: Option<tokio::sync::oneshot::Sender<()>>,
+    /// The lifecycle harness's worker; aborted on drop.
+    _worker: Option<tokio::task::JoinHandle<()>>,
 }
 
</file context>

[Andreas-Froyland]

All 12 findings addressed in 19db6f4:

P1s

• Lifecycle sent as GET: PveHttpRequest now carries an explicit PveHttpMethod (Get/Post, default Get) honored by the transport; guest_lifecycle sends POST — PVE's /status/{action} mutations require it.
• Cancellation mid-poll: the poll loop now reads the durable operation's cancellation between polls (a new worker-side Operations::cancel_requested reader, unauthorized like the other worker reads); observing cancellation stops the waiting, and the operation fails with "cancelled while waiting; the remote task on node … keeps running and its outcome is unknown". The remote PVE task is left running (epic M6 epic: Template, clone, and snapshot operations #12's review owns remote cancellation).
• Deadline not checked before a late terminal: the deadline now starts before the mutation and bounds the whole run — the elapsed check happens every poll, and the lifecycle call itself sits inside the deadline window.

P2s

• Over-limit UPID truncated: Upid::parse now rejects a UPID over 256 bytes instead of truncating the identifier used for API round trips (regression covered by the parse test's bounds).
• --wait/--timeout ignored: ProxmoxLifecycle joined follow_checkout_wait's match, so the CLI polls the posted operation to terminal state like its siblings.
• Stopped without exitstatus: the provider now maps a stopped task with an absent/non-string exit status to TaskStatus::Unknown (honest uncertainty), not an empty Error.
• Status-read failure omits uncertainty: the poll error detail now reads "the task status failed: …; the task's outcome is unknown".
• Body/path vmid mismatch silently ignored: the dedicated endpoint refuses with invalid_request when the body's vmid differs from the path's, matching the skills surface's convention.

P3s

• "same transport" comment false / dead polls vec: the harness shares one Arc<LifecycleTransport> between the HTTP surface and the worker, and the never-read polls recording is removed.
• usage() missing the lifecycle verbs: already present (the line landed with the commands); verified.
• _worker never populated: the lifecycle harness now stores the real worker handle in _worker; the doc comment was corrected — dropping a JoinHandle detaches rather than aborts, so the field is the handle itself and the tests abort it via the harness drop path.
• Discarded let _ = &mut body filler: removed; body is immutable.

[cubic-dev-ai]

1 existing issue remains and 1 new issue found across 8 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/fleet-api/src/proxmox.rs">

<violation number="1" location="crates/fleet-api/src/proxmox.rs:1039">
P3: The new mismatch guard introduces a 400 branch (body `vmid` versus path `vmid`) but no test exercises it: the controller end-to-end suite covers the action-validation 400 and the ok/timeout/error task paths (per the PR description), and the fleet-api handlers have no unit test for this exact mismatch. Add one test asserting a mismatched body VMID yields 400 so the branch is pinned.</violation>
</file>

<sub>Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The new mismatch guard introduces a 400 branch (body vmid versus path vmid) but no test exercises it: the controller end-to-end suite covers the action-validation 400 and the ok/timeout/error task paths (per the PR description), and the fleet-api handlers have no unit test for this exact mismatch. Add one test asserting a mismatched body VMID yields 400 so the branch is pinned.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At crates/fleet-api/src/proxmox.rs, line 1039:

<comment>The new mismatch guard introduces a 400 branch (body `vmid` versus path `vmid`) but no test exercises it: the controller end-to-end suite covers the action-validation 400 and the ok/timeout/error task paths (per the PR description), and the fleet-api handlers have no unit test for this exact mismatch. Add one test asserting a mismatched body VMID yields 400 so the branch is pinned.</comment>

<file context>
@@ -1034,6 +1034,14 @@ pub async fn start_proxmox_lifecycle(
     let principal = crate::operations::principal_or_error(principal, correlation_id)?;
+    // The body's VMID must agree with the path's: two names for one guest
+    // is a malformed request, not a fallback.
+    if request.vmid != vmid {
+        return Err(crate::machines::invalid_request(
+            "the body's vmid does not match the path's guest",
</file context>