fix: pct_create false-success, VMID collision, and stuck approval banner

Real production failure when the operator clicked Approve in chat: nothing
provisioned, banner never cleared, execution marked completed.

Three root causes:
- sshExec swallowed non-zero exits when the command produced output, so a
  `pct create` that printed "CT 132 already exists" and failed was reported
  as success and a bogus lxc entity was registered. Now any non-zero exit
  returns an error (with output) so the execution is correctly marked failed.
- The LLM reused VMID 132 (belongs to lxc:rclone; VMIDs are cluster-wide).
  pct_create now checks in-use VMIDs via `pvesh get /cluster/resources` and
  falls back to `pvesh get /cluster/nextid` when the requested id is taken.
- InlineApproval.svelte reset its state on every prop change (done was also
  compared against the wrong string), so the banner never cleared and each
  click re-POSTed /decision. Rewritten to track outcome per executionId,
  clear on success, and block resubmits.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

internal/httpapi/phase3.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"log/slog"
 	"os"
+	"strconv"
 	"strings"
 	"time"
 
@@ -103,10 +104,18 @@ func sshExec(ctx context.Context, host, user, command string) (string, error) {
 	defer session.Close()
 
 	out, err := session.CombinedOutput(command)
-	if err != nil && out == nil {
-		return "", fmt.Errorf("exec: %w", err)
+	text := strings.TrimSpace(string(out))
+	// A non-zero exit MUST surface as an error. The previous guard only
+	// errored when there was no output, so a `pct create` that printed
+	// "CT 132 already exists" and exited non-zero was reported as success —
+	// the execution was marked completed though nothing was provisioned.
+	if err != nil {
+		if text != "" {
+			return text, fmt.Errorf("%w: %s", err, text)
+		}
+		return text, fmt.Errorf("exec: %w", err)
 	}
-	return strings.TrimSpace(string(out)), nil
+	return text, nil
 }
 
 func resolveHostSSH(ctx context.Context, pool *db.Pool, entitySlug string) (string, string, error) {
@@ -276,6 +285,33 @@ func executeApprovedAction(ctx context.Context, pool *db.Pool, execID uuid.UUID,
 			return
 		}
 
+		// VMID collision guard. Proxmox VMIDs are cluster-wide, so the model's
+		// guess (e.g. 132) can collide with a container on another node — pct
+		// create then fails with "CT N already exists on node X". Fetch the set
+		// of in-use VMIDs across the cluster; if the requested id is taken (or
+		// absent), fall back to the cluster's next free id so provisioning
+		// still succeeds instead of dead-ending on the operator's approval.
+		usedRaw, _ := sshExec(ctx, host, user, `pvesh get /cluster/resources --type vm --output-format json 2>/dev/null | grep -o '"vmid":[0-9]*' | grep -o '[0-9]*' || true`)
+		used := map[int]bool{}
+		for _, l := range strings.Fields(usedRaw) {
+			if n, e := strconv.Atoi(strings.TrimSpace(l)); e == nil {
+				used[n] = true
+			}
+		}
+		if cfg.VMID == 0 || used[cfg.VMID] {
+			nextRaw, nerr := sshExec(ctx, host, user, `pvesh get /cluster/nextid 2>/dev/null`)
+			nextID, cerr := strconv.Atoi(strings.TrimSpace(nextRaw))
+			if nerr != nil || cerr != nil || nextID == 0 {
+				msg := fmt.Sprintf("VMID %d is already in use on the cluster and could not resolve a free id", cfg.VMID)
+				pool.Exec(ctx, `UPDATE executions SET status='failed', result=$2::jsonb WHERE entity_id=$1`,
+					execID, fmt.Sprintf(`{"error":%q}`, msg))
+				emitExecutionEvent(ctx, pool, execID, "failed", map[string]any{"target": targetSlug, "error": msg})
+				return
+			}
+			slog.Info("httpapi: pct_create VMID reassigned", "requested", cfg.VMID, "assigned", nextID)
+			cfg.VMID = nextID
+		}
+
 		privFlag := "--unprivileged 1"
 		if cfg.Privileged {
 			privFlag = "--unprivileged 0"

web/src/lib/components/InlineApproval.svelte

@@ -2,6 +2,7 @@
   import type { PendingApproval } from '$lib/stores/chat'
   import { decideApproval } from '$lib/api'
   import { Button } from '$lib/components/ui/button'
+  import { SvelteMap } from 'svelte/reactivity'
   import CheckIcon from '@lucide/svelte/icons/check'
   import XIcon from '@lucide/svelte/icons/x'
   import ShieldCheckIcon from '@lucide/svelte/icons/shield-check'
@@ -9,34 +10,51 @@
 
   let { approvals }: { approvals: PendingApproval[] } = $props()
 
-  let pending = $state(false)
-  let done = $state<string | null>(null)
-  let doneId = $state('')
+  // Per-execution outcome, keyed by executionId. A resolved entry hides the
+  // action buttons for that approval permanently so the banner clears after a
+  // click and can never re-POST /decision. 'pending' guards against double
+  // submits (the old component reset its state on every prop change, so the
+  // banner never cleared and each click fired another decision).
+  type State = 'pending' | 'approved' | 'denied' | 'failed'
+  const outcome = new SvelteMap<string, State>()
 
   async function decide(approval: PendingApproval, decision: 'approve' | 'deny') {
-    pending = true
-    doneId = approval.executionId
-    const result = await decideApproval(approval.executionId, decision)
-    pending = false
-    done = result ? decision : 'failed'
+    const id = approval.executionId
+    const cur = outcome.get(id)
+    if (cur === 'pending' || cur === 'approved' || cur === 'denied') return // no resubmit
+    outcome.set(id, 'pending')
+    const ok = await decideApproval(id, decision)
+    outcome.set(id, ok ? (decision === 'approve' ? 'approved' : 'denied') : 'failed')
   }
-
-  $effect(() => {
-    done = null
-    doneId = ''
-    pending = false
-    void approvals
-  })
 </script>
 
-{#each approvals.filter(a => !done || a.executionId !== doneId) as approval (approval.executionId)}
-  {#if !done || approval.executionId !== doneId}
+{#each approvals as approval (approval.executionId)}
+  {@const state = outcome.get(approval.executionId)}
+  {#if state === 'approved'}
+    <div class="my-2 flex items-center gap-2 rounded-lg border border-success/40 bg-success/5 px-3 py-2 text-xs text-success">
+      <CheckIcon class="size-4" />
+      <span>Approved — provisioning. Track progress in the Executions view.</span>
+    </div>
+  {:else if state === 'denied'}
+    <div class="my-2 flex items-center gap-2 rounded-lg border border-destructive/40 bg-destructive/5 px-3 py-2 text-xs text-destructive">
+      <XIcon class="size-4" />
+      <span>Denied.</span>
+    </div>
+  {:else if state === 'failed'}
+    <div class="my-2 flex items-center gap-2 rounded-lg border border-destructive/40 bg-destructive/5 px-3 py-2 text-xs text-destructive">
+      <XIcon class="size-4" />
+      <span class="flex-1">Decision failed to send.</span>
+      <Button size="sm" variant="outline" class="h-7 px-2.5 text-xs" onclick={() => { outcome.delete(approval.executionId); decide(approval, 'approve') }}>
+        Retry
+      </Button>
+    </div>
+  {:else}
     <div class="my-2 flex items-center gap-2 rounded-lg border border-warning/40 bg-warning/5 px-3 py-2">
       <ShieldCheckIcon class="size-4 shrink-0 text-warning" />
       <span class="flex-1 text-xs text-muted-foreground">
         {approval.action} on {approval.target} requires approval
       </span>
-      {#if pending && doneId === approval.executionId}
+      {#if state === 'pending'}
         <LoaderCircleIcon class="size-4 animate-spin text-muted-foreground" />
       {:else}
         <Button size="sm" variant="default" class="h-7 px-2.5 text-xs" onclick={() => decide(approval, 'approve')}>
@@ -49,15 +67,5 @@
         </Button>
       {/if}
     </div>
-  {:else if done === 'approved'}
-    <div class="my-2 flex items-center gap-2 rounded-lg border border-success/40 bg-success/5 px-3 py-2 text-xs text-success">
-      <CheckIcon class="size-4" />
-      <span>Approved. The action is running.</span>
-    </div>
-  {:else}
-    <div class="my-2 flex items-center gap-2 rounded-lg border border-destructive/40 bg-destructive/5 px-3 py-2 text-xs text-destructive">
-      <XIcon class="size-4" />
-      <span>Denied.</span>
-    </div>
   {/if}
 {/each}