diff --git a/internal/httpapi/phase3.go b/internal/httpapi/phase3.go index fec290c..ccbdca8 100644 --- a/internal/httpapi/phase3.go +++ b/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" diff --git a/web/src/lib/components/InlineApproval.svelte b/web/src/lib/components/InlineApproval.svelte index d750d6c..3618d13 100644 --- a/web/src/lib/components/InlineApproval.svelte +++ b/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(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() 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 - }) -{#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'} +
+ + Approved — provisioning. Track progress in the Executions view. +
+ {:else if state === 'denied'} +
+ + Denied. +
+ {:else if state === 'failed'} +
+ + Decision failed to send. + +
+ {:else}
{approval.action} on {approval.target} requires approval - {#if pending && doneId === approval.executionId} + {#if state === 'pending'} {:else} {/if}
- {:else if done === 'approved'} -
- - Approved. The action is running. -
- {:else} -
- - Denied. -
{/if} {/each}