fix: approval UI was never mounted; add typed confirmation for destructive
Root cause of "chat gave me no further feedback — had to go to Ops": two compounding bugs, found by reading the actual production session transcript. 1. InlineApproval.svelte — all of last session's live-status/self-heal work — was never imported or rendered anywhere. Chat.svelte had its own separate, much dumber approval bar (no status tracking, no destructive handling, just silently disappears after clicking) that WAS the one users actually saw. Deleted the dead bar and its state; InlineApproval now renders per-message. 2. chat.ts's extractApprovals hardcoded `tool.name === 'request_execution'`, so any approval raised by the newer `run` tool was invisible — no card, no feedback, nothing to self-heal, forcing the operator to the Ops page with zero acknowledgement in the conversation. This was the actual proximate cause of last night's destroy-135 session. Fixed to match on response shape, not tool name, so it doesn't silently break again for the next new gated tool. 3. Nomos was telling operators "type something like 'I confirm destroy 135'" for destructive actions (SOUL.md) but no backend path ever consumed that phrase — chat-assent explicitly (and correctly) excludes destructive from loose assent, but I never built the alternative. Added isTypedConfirmation() (cmd/nomos/assent.go): stricter than loose assent, requires an explicit "confirm" statement, only applies to destructive- flagged pending approvals. 4. InlineApproval's completed-state hardcoded "Provisioned successfully" — wrong/confusing for a destroy or arbitrary `run` command. Now says "Completed on <target>" and shows the actual command output, verified live against the real destroy-135 execution. Verified live in a real browser against the production API/DB (dev server proxying to :8090): the historical stuck session now retroactively renders both executions as resolved with correct wording and real output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -26,6 +26,12 @@
|
||||
return typeof v === 'string' && v ? v : 'Execution failed.'
|
||||
}
|
||||
|
||||
function outputText(e: Execution | undefined): string {
|
||||
const r = e?.result as Record<string, unknown> | undefined | null
|
||||
const v = r?.output
|
||||
return typeof v === 'string' ? v.trim() : ''
|
||||
}
|
||||
|
||||
// Poll the execution until it reaches a terminal state, so the operator sees
|
||||
// provisioning progress and the final outcome without leaving the chat.
|
||||
async function track(id: string) {
|
||||
@@ -96,9 +102,14 @@
|
||||
{@const p = phase.get(approval.executionId)}
|
||||
{@const e = exec.get(approval.executionId)}
|
||||
{#if p === 'completed'}
|
||||
<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 shrink-0" />
|
||||
<span>Provisioned successfully{e?.duration_ms ? ` in ${Math.round(e.duration_ms / 1000)}s` : ''}. See the Executions view for details.</span>
|
||||
<div class="my-2 flex flex-col gap-1 rounded-lg border border-success/40 bg-success/5 px-3 py-2 text-xs text-success">
|
||||
<div class="flex items-center gap-2">
|
||||
<CheckIcon class="size-4 shrink-0" />
|
||||
<span>Completed{e?.duration_ms ? ` in ${Math.round(e.duration_ms / 1000)}s` : ''} on {approval.target}.</span>
|
||||
</div>
|
||||
{#if outputText(e)}
|
||||
<pre class="max-h-32 overflow-y-auto whitespace-pre-wrap break-words pl-6 opacity-80">{outputText(e)}</pre>
|
||||
{/if}
|
||||
</div>
|
||||
{:else if p === 'failed'}
|
||||
<div class="my-2 flex flex-col gap-1 rounded-lg border border-destructive/40 bg-destructive/5 px-3 py-2 text-xs text-destructive">
|
||||
@@ -116,7 +127,21 @@
|
||||
{:else if p === 'running' || p === 'deciding'}
|
||||
<div class="my-2 flex items-center gap-2 rounded-lg border border-warning/40 bg-warning/5 px-3 py-2 text-xs text-muted-foreground">
|
||||
<LoaderCircleIcon class="size-4 shrink-0 animate-spin text-warning" />
|
||||
<span>{p === 'deciding' ? 'Submitting approval…' : `Provisioning ${approval.target}… (this can take a minute)`}</span>
|
||||
<span>{p === 'deciding' ? 'Submitting approval…' : `Running on ${approval.target}… (this can take a minute)`}</span>
|
||||
</div>
|
||||
{:else if approval.destructive}
|
||||
<div class="my-2 flex items-center gap-2 rounded-lg border border-destructive/50 bg-destructive/10 px-3 py-2">
|
||||
<ShieldCheckIcon class="size-4 shrink-0 text-destructive" />
|
||||
<span class="flex-1 text-xs text-destructive">
|
||||
<strong>DESTRUCTIVE</strong> — {approval.action} on {approval.target}. Type
|
||||
"I confirm" in chat, or use the button.
|
||||
</span>
|
||||
<Button size="sm" variant="destructive" class="h-7 px-2.5 text-xs" onclick={() => decide(approval, 'approve')}>
|
||||
<CheckIcon class="size-3" /><span class="ml-1">Confirm</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>
|
||||
</div>
|
||||
{:else}
|
||||
<div class="my-2 flex items-center gap-2 rounded-lg border border-warning/40 bg-warning/5 px-3 py-2">
|
||||
|
||||
@@ -6,6 +6,7 @@ export interface PendingApproval {
|
||||
executionId: string
|
||||
action: string
|
||||
target: string
|
||||
destructive: boolean
|
||||
}
|
||||
|
||||
export interface ChatMessage {
|
||||
@@ -18,18 +19,28 @@ export interface ChatMessage {
|
||||
|
||||
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
|
||||
|
||||
// Deliberately NOT filtered by tool name. There is no fixed set of gated
|
||||
// tools — `run` can execute anything, and any future tool that queues an
|
||||
// approval should surface a card the same way. A prior version hardcoded
|
||||
// `t.name === 'request_execution'`, so approvals raised by the newer `run`
|
||||
// tool were silently invisible in chat: no card, no feedback, nothing to
|
||||
// self-heal, forcing the operator to the Ops page with zero acknowledgement
|
||||
// back in the conversation. Matching on the response shape (not the tool
|
||||
// name) is what makes this robust to new gated tools without another
|
||||
// silent breakage.
|
||||
function extractApprovals(tools: ToolCallResult[]): PendingApproval[] {
|
||||
const out: PendingApproval[] = []
|
||||
for (const t of tools) {
|
||||
if (t.name !== 'request_execution' || t.type !== 'tool_result') continue
|
||||
if (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'
|
||||
action: t.args?.action ?? t.args?.purpose ?? t.name ?? 'unknown',
|
||||
target: t.args?.target ?? 'unknown',
|
||||
destructive: /\bDESTRUCTIVE\b/.test(text)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,17 +1,13 @@
|
||||
<script lang="ts">
|
||||
import { messages, streaming, sendMessage, cancelStream, error, type PendingApproval } from '$lib/stores/chat'
|
||||
import { decideApproval } from '$lib/api'
|
||||
import { messages, streaming, sendMessage, cancelStream, error } from '$lib/stores/chat'
|
||||
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'
|
||||
|
||||
@@ -19,43 +15,6 @@
|
||||
|
||||
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
|
||||
@@ -167,6 +126,9 @@
|
||||
<span class="size-1.5 animate-bounce rounded-full bg-current"></span>
|
||||
</div>
|
||||
{/if}
|
||||
{#if msg.pendingApprovals.length > 0}
|
||||
<InlineApproval approvals={msg.pendingApprovals} />
|
||||
{/if}
|
||||
</div>
|
||||
{/if}
|
||||
</div>
|
||||
@@ -183,37 +145,6 @@
|
||||
</div>
|
||||
{/if}
|
||||
|
||||
{#if pendingApprovals.length > 0}
|
||||
<div class="shrink-0 border-t 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}
|
||||
<Button size="sm" variant="default" class="mt-1 h-6 px-2 text-xs" disabled={approving !== null} onclick={approveAll}>
|
||||
<CheckIcon class="size-3" />
|
||||
<span class="ml-1">Approve all</span>
|
||||
</Button>
|
||||
{/if}
|
||||
</div>
|
||||
{/if}
|
||||
|
||||
<div class="border-t bg-card/50 p-3">
|
||||
<form
|
||||
class="mx-auto flex max-w-3xl items-end gap-2"
|
||||
|
||||
Reference in New Issue
Block a user