fix: dedup request_execution, persistent approval bar, JSON payload

- Add dedup in request_execution: check entities(type,name) uniqueness before creating duplicate executions. Returns 'already queued' message to the LLM, preventing tool-calling loops. - Fix createApproval JSON payload: use json.Marshal instead of fmt.Sprintf to escape params (could contain unescaped double quotes from JSON config). - Add ON CONFLICT DO NOTHING to entity/execution inserts for dedup race safety. - Persistent approval bar at top of Chat.svelte: aggregates pendingApprovals from all messages, fixed position (won't scroll away). Approve/deny/approve-all. - Update SOUL.md: agent must STOP after queuing a gated action. - Fix ToolCallGroup reactivity: wasActive = (active).
2026-07-09 23:19:55 +02:00
parent d9683cfe29
commit 9376dc7d89
3 changed files with 111 additions and 8 deletions
--- a/internal/mcp/server.go
+++ b/internal/mcp/server.go
@@ -281,14 +281,39 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server {
 			return textResult(fmt.Sprintf("target not found: %s", targetSlug)), nil
 		}

+		// Deduplicate: if a pending execution already exists for the same
+		// target+action, return the existing one instead of creating a
+		// duplicate. Prevents the LLM from re-requesting the same gated
+		// action in a tool-calling loop. Uses the entities type+name
+		// UNIQUE constraint as the dedup key (one execution per
+		// action:target pair).
+		if action == "systemctl" || action == "apt_upgrade" || action == "pct_create" {
+			execName := action + " on " + targetSlug
+			var existingID, existingStatus string
+			err := pool.QueryRow(ctx, `
+				SELECT e.id::text, COALESCE(ex.status,'') FROM entities e
+				LEFT JOIN executions ex ON ex.entity_id = e.id
+				WHERE e.type = 'execution' AND e.name = $1
+				ORDER BY e.created_at DESC LIMIT 1`, execName).Scan(&existingID, &existingStatus)
+			if err == nil && existingID != "" && existingStatus != "completed" && existingStatus != "failed" {
+				return textResult(fmt.Sprintf("%s on %s is already queued for approval — execution %s. Wait for operator approval. Do not re-request.",
+					action, targetSlug, existingID)), nil
+			}
+		}
+
 		id, _ := uuid.NewV7()
 		correlationID := uuid.New().String()

-		// Write execution record
+		// Write execution record. If the (type,name) UNIQUE constraint
+		// fires (dedup race), the INSERT silently does nothing and the
+		// existing record wins.
 		execSlug := "exec:" + targetSlug + ":" + id.String()[:8]
-		pool.Exec(ctx, `INSERT INTO entities (id, slug, type, name, attributes) VALUES ($1, $2, 'execution', $3, '{}')`,
+		_, err := pool.Exec(ctx, `INSERT INTO entities (id, slug, type, name, attributes) VALUES ($1, $2, 'execution', $3, '{}') ON CONFLICT (type, name) DO NOTHING`,
 			id, execSlug, action+" on "+targetSlug)
-		pool.Exec(ctx, `INSERT INTO executions (entity_id, target_entity_id, action, risk_class, status, correlation_id, agent_id) VALUES ($1, $2, $3, 'reversible_low', 'running', $4, $5)`,
+		if err != nil {
+			return textResult(fmt.Sprintf("error: failed to create execution: %v", err)), nil
+		}
+		pool.Exec(ctx, `INSERT INTO executions (entity_id, target_entity_id, action, risk_class, status, correlation_id, agent_id) VALUES ($1, $2, $3, 'reversible_low', 'running', $4, $5) ON CONFLICT DO NOTHING`,
 			id, targetID, action+":"+params, correlationID, agentID)

 		// Execute reversible actions immediately
@@ -978,7 +1003,8 @@ func resolveHost(ctx context.Context, pool *db.Pool, entitySlug string) (hostIP
 }

 func createApproval(ctx context.Context, pool *db.Pool, execID, targetID uuid.UUID, action, params, riskClass string) {
-	payload := fmt.Sprintf(`{"action":"%s","params":"%s","execution_id":"%s"}`, action, params, execID)
+	p := map[string]any{"action": action, "params": params, "execution_id": execID.String()}
+	payload, _ := json.Marshal(p)
 	// approvals.entity_id is PK + FK to entities(id). Reuse the execution's
 	// entity (already inserted by request_execution) so the FK is satisfied —
 	// a fresh UUID here had no matching entities row, so the INSERT silently
@@ -989,7 +1015,7 @@ func createApproval(ctx context.Context, pool *db.Pool, execID, targetID uuid.UU
 			kind, payload, status, expires_at, created_at)
 		VALUES ($1, $2, $3, $4, 'execution', $5::jsonb, 'pending',
 			now() + interval '1 hour', now())`,
-		execID, targetID, action, riskClass, payload); err != nil {
+		execID, targetID, action, riskClass, string(payload)); err != nil {
 		slog.Error("createApproval: insert approval", "error", err, "execution", execID)
 		return
 	}
--- a/nomos/SOUL.md
+++ b/nomos/SOUL.md
@@ -59,6 +59,11 @@ Before calling `request_execution`:
 - If `destructive` or `config_mutation`: escalate to operator
 - If `reversible_low` with validated pattern: auto-act allowed

+**After requesting a gated action that queues for approval: STOP.** Present the
+plan to the operator and wait. Do not call `request_execution` again for the
+same action — the system will tell you it's already queued. One approval per
+action is enough. The operator will approve (or deny) from the chat UI.
+
 ## Token efficiency

 Use MCP tools over raw queries. MCP responses are already compressed. When
--- a/web/src/pages/Chat.svelte
+++ b/web/src/pages/Chat.svelte
@@ -1,13 +1,17 @@
 <script lang="ts">
-  import { messages, streaming, sendMessage, cancelStream, error } from '$lib/stores/chat'
+  import { messages, streaming, sendMessage, cancelStream, error, type PendingApproval } from '$lib/stores/chat'
+  import { decideApproval } from '$lib/api'
  import SessionRail from '$lib/components/SessionRail.svelte'
  import SessionGraph from '$lib/components/SessionGraph.svelte'
  import ToolCallGroup from '$lib/components/ToolCallGroup.svelte'
-  import InlineApproval from '$lib/components/InlineApproval.svelte'
  import { Button } from '$lib/components/ui/button'
  import { Textarea } from '$lib/components/ui/textarea'
  import ArrowUpIcon from '@lucide/svelte/icons/arrow-up'
  import SquareIcon from '@lucide/svelte/icons/square'
+  import CheckIcon from '@lucide/svelte/icons/check'
+  import XIcon from '@lucide/svelte/icons/x'
+  import ShieldCheckIcon from '@lucide/svelte/icons/shield-check'
+  import LoaderCircleIcon from '@lucide/svelte/icons/loader-circle'
  import { marked } from 'marked'
  import DOMPurify from 'dompurify'

@@ -15,6 +19,43 @@

  let input = $state('')
  let messagesEnd = $state<HTMLDivElement | null>(null)
+  let approving = $state<string | null>(null)
+  let approvedIds = $state(new Set<string>())
+
+  const pendingApprovals = $derived.by(() => {
+    const msgs = $messages
+    const all: PendingApproval[] = []
+    for (const m of msgs) {
+      all.push(...m.pendingApprovals)
+    }
+    return all.filter(a => !approvedIds.has(a.executionId))
+  })
+
+  async function approveAll() {
+    for (const a of pendingApprovals) {
+      approving = a.executionId
+      await decideApproval(a.executionId, 'approve')
+      approvedIds.add(a.executionId)
+      approvedIds = approvedIds
+    }
+    approving = null
+  }
+
+  async function approveOne(a: PendingApproval) {
+    approving = a.executionId
+    await decideApproval(a.executionId, 'approve')
+    approvedIds.add(a.executionId)
+    approvedIds = approvedIds
+    approving = null
+  }
+
+  async function denyOne(a: PendingApproval) {
+    approving = a.executionId
+    await decideApproval(a.executionId, 'deny')
+    approvedIds.add(a.executionId)
+    approvedIds = approvedIds
+    approving = null
+  }

  // Resizable right rail (session graph). Persisted so it survives reloads.
  const RAIL_MIN = 260
@@ -89,6 +130,38 @@
    </div>
  {/if}
  <div class="flex min-w-0 flex-1 flex-col">
+    {#if pendingApprovals.length > 0}
+      <div class="shrink-0 border-b border-warning/30 bg-warning/5 px-4 py-2">
+        {#each pendingApprovals as a (a.executionId)}
+          <div class="flex items-center gap-2">
+            <ShieldCheckIcon class="size-4 shrink-0 text-warning" />
+            <span class="flex-1 text-xs font-medium">
+              {a.action} on {a.target}
+            </span>
+            {#if approving === a.executionId}
+              <LoaderCircleIcon class="size-4 animate-spin text-muted-foreground" />
+            {:else}
+              <Button size="sm" variant="default" class="h-7 px-2.5 text-xs" disabled={approving !== null} onclick={() => approveOne(a)}>
+                <CheckIcon class="size-3" />
+                <span class="ml-1">Approve</span>
+              </Button>
+              <Button size="sm" variant="outline" class="h-7 px-2.5 text-xs" disabled={approving !== null} onclick={() => denyOne(a)}>
+                <XIcon class="size-3" />
+                <span class="ml-1">Deny</span>
+              </Button>
+            {/if}
+          </div>
+        {/each}
+        {#if pendingApprovals.length > 1}
+          <div class="mt-1">
+            <Button size="sm" variant="default" class="h-6 px-2 text-xs" disabled={approving !== null} onclick={approveAll}>
+              <CheckIcon class="size-3" />
+              <span class="ml-1">Approve all</span>
+            </Button>
+          </div>
+        {/if}
+      </div>
+    {/if}
    <div class="min-h-0 flex-1 overflow-y-auto">
      <div class="mx-auto flex max-w-3xl flex-col gap-5 p-4">
        {#if $messages.length === 0}
@@ -114,7 +187,6 @@
            {:else}
              <div class="flex w-full flex-col gap-2">
                <ToolCallGroup tools={msg.tools} active={$streaming && i === $messages.length - 1} />
-                <InlineApproval approvals={msg.pendingApprovals} />
                {#if msg.text}
                  <div class="prose-chat max-w-none text-sm leading-relaxed">
                    <!-- eslint-disable-next-line svelte/no-at-html-tags — sanitized via DOMPurify -->