fix: structured approvals + ToolCallGroup reactivity

- Replace text-based regex parsing in InlineApproval with structured pendingApprovals extracted from request_execution tool results. The tool result text is deterministic (not LLM-generated), making UUID extraction reliable regardless of how the LLM rephrases the response. - Fix ToolCallGroup reactivity: wasActive = active captured initial value. Now uses (active) so re-runs on prop changes. - Extract approvals in both live streaming (done event) and history loading for consistent behavior on resumed sessions.
2026-07-09 11:35:30 +02:00
parent ea62d744ed
commit d9683cfe29
4 changed files with 88 additions and 52 deletions
--- a/web/src/lib/components/InlineApproval.svelte
+++ b/web/src/lib/components/InlineApproval.svelte
@@ -1,4 +1,5 @@
 <script lang="ts">
+  import type { PendingApproval } from '$lib/stores/chat'
  import { decideApproval } from '$lib/api'
  import { Button } from '$lib/components/ui/button'
  import CheckIcon from '@lucide/svelte/icons/check'
@@ -6,62 +7,57 @@
  import ShieldCheckIcon from '@lucide/svelte/icons/shield-check'
  import LoaderCircleIcon from '@lucide/svelte/icons/loader-circle'

-  let { text }: { text: string } = $props()
-
-  const RE = /\bexec[uecution]*\s+([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\b/i
-  const match = $derived(text.match(RE))
+  let { approvals }: { approvals: PendingApproval[] } = $props()

  let pending = $state(false)
-  let done = $state<'approved' | 'denied' | null>(null)
+  let done = $state<string | null>(null)
+  let doneId = $state('')

-  async function approve() {
-    if (!match) return
+  async function decide(approval: PendingApproval, decision: 'approve' | 'deny') {
    pending = true
-    const result = await decideApproval(match[1], 'approve')
+    doneId = approval.executionId
+    const result = await decideApproval(approval.executionId, decision)
    pending = false
-    done = result ? 'approved' : 'denied'
-  }
-
-  async function deny() {
-    if (!match) return
-    pending = true
-    const result = await decideApproval(match[1], 'deny')
-    pending = false
-    done = result ? 'denied' : 'denied'
+    done = result ? decision : 'failed'
  }

  $effect(() => {
    done = null
+    doneId = ''
    pending = false
-    void text
+    void approvals
  })
 </script>

-{#if match && !done}
-  <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">This action requires approval</span>
-    {#if 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={approve}>
-        <CheckIcon class="size-3" />
-        <span class="ml-1">Approve</span>
-      </Button>
-      <Button size="sm" variant="outline" class="h-7 px-2.5 text-xs" onclick={deny}>
-        <XIcon class="size-3" />
-        <span class="ml-1">Deny</span>
-      </Button>
-    {/if}
-  </div>
-{:else if done}
-  <div class="my-2 flex items-center gap-2 rounded-lg border px-3 py-2 text-xs {done === 'approved' ? 'border-success/40 bg-success/5 text-success' : 'border-destructive/40 bg-destructive/5 text-destructive'}">
-    {#if done === 'approved'}
+{#each approvals.filter(a => !done || a.executionId !== doneId) as approval (approval.executionId)}
+  {#if !done || approval.executionId !== doneId}
+    <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}
+        <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')}>
+          <CheckIcon class="size-3" />
+          <span class="ml-1">Approve</span>
+        </Button>
+        <Button size="sm" variant="outline" class="h-7 px-2.5 text-xs" onclick={() => decide(approval, 'deny')}>
+          <XIcon class="size-3" />
+          <span class="ml-1">Deny</span>
+        </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>
-    {:else}
+    </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>
-    {/if}
-  </div>
-{/if}
+    </div>
+  {/if}
+{/each}
--- a/web/src/lib/components/ToolCallGroup.svelte
+++ b/web/src/lib/components/ToolCallGroup.svelte
@@ -10,7 +10,7 @@
  let { tools, active = false }: { tools: ToolCallResult[]; active?: boolean } = $props()

  let open = $state(false)
-  let wasActive = active
+  let wasActive = $state(active)

  $effect(() => {
    if (active && !wasActive) {
--- a/web/src/lib/stores/chat.ts
+++ b/web/src/lib/stores/chat.ts
@@ -2,11 +2,38 @@ import { writable, get } from 'svelte/store'
 import { streamChat, fetchSessions, fetchMessages, deleteSession as apiDeleteSession } from '$lib/api'
 import type { ChatEvent, Session, Message } from '$lib/api'

+export interface PendingApproval {
+  executionId: string
+  action: string
+  target: string
+}
+
 export interface ChatMessage {
  id: string
  role: 'user' | 'assistant'
  text: string
  tools: ToolCallResult[]
+  pendingApprovals: PendingApproval[]
+}
+
+const APPROVAL_RE = /execution\s+([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})/i
+
+function extractApprovals(tools: ToolCallResult[]): PendingApproval[] {
+  const out: PendingApproval[] = []
+  for (const t of tools) {
+    if (t.name !== 'request_execution' || t.type !== 'tool_result') continue
+    const text = typeof t.result === 'string' ? t.result : JSON.stringify(t.result ?? '')
+    if (!text.includes('requires approval')) continue
+    const m = text.match(APPROVAL_RE)
+    if (m) {
+      out.push({
+        executionId: m[1],
+        action: t.args?.action ?? 'unknown',
+        target: t.args?.target ?? 'unknown'
+      })
+    }
+  }
+  return out
 }

 export interface ToolCallResult {
@@ -57,12 +84,16 @@ export async function loadSessionMessages(sessionId: string) {
  currentSession.set(sessionId)
  const msgs = await fetchMessages(sessionId)
  sessionMessages.set(msgs)
-  const chatMsgs: ChatMessage[] = msgs.map((m) => ({
-    id: m.id,
-    role: m.role as 'user' | 'assistant',
-    text: m.content?.text ?? (typeof m.content === 'string' ? m.content : ''),
-    tools: mergeToolCalls(m.content?.tool_calls)
-  }))
+  const chatMsgs: ChatMessage[] = msgs.map((m) => {
+    const tools = mergeToolCalls(m.content?.tool_calls)
+    return {
+      id: m.id,
+      role: m.role as 'user' | 'assistant',
+      text: m.content?.text ?? (typeof m.content === 'string' ? m.content : ''),
+      tools,
+      pendingApprovals: extractApprovals(tools)
+    }
+  })
  messages.set(chatMsgs)
 }

@@ -74,7 +105,8 @@ export function sendMessage(text: string) {
    id: mid(),
    role: 'user',
    text,
-    tools: []
+    tools: [],
+    pendingApprovals: []
  }
  messages.update((ms) => [...ms, userMsg])

@@ -82,7 +114,8 @@ export function sendMessage(text: string) {
    id: mid(),
    role: 'assistant',
    text: '',
-    tools: []
+    tools: [],
+    pendingApprovals: []
  }
  messages.update((ms) => [...ms, assistantMsg])

@@ -147,6 +180,13 @@ export function sendMessage(text: string) {
          return [...ms]
        })
      } else if (ev.type === 'done') {
+        messages.update((ms) => {
+          const last = ms[ms.length - 1]
+          if (last && last.role === 'assistant') {
+            last.pendingApprovals = extractApprovals(last.tools)
+          }
+          return [...ms]
+        })
        currentSession.set(ev.data?.session_id ?? ev.session_id)
      } else if (ev.type === 'error') {
        error.set(ev.data)
--- a/web/src/pages/Chat.svelte
+++ b/web/src/pages/Chat.svelte
@@ -114,7 +114,7 @@
            {:else}
              <div class="flex w-full flex-col gap-2">
                <ToolCallGroup tools={msg.tools} active={$streaming && i === $messages.length - 1} />
-                <InlineApproval text={msg.text} />
+                <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 -->