diff --git a/cmd/nomos/agent.go b/cmd/nomos/agent.go index 53080df..0087333 100644 --- a/cmd/nomos/agent.go +++ b/cmd/nomos/agent.go @@ -170,14 +170,22 @@ func (a *agent) chat(ctx context.Context, sessionID, message string, emit func(a // authorization ("go ahead", "yes", ...), grant them now — this is the // primary approval path; the Approve button in the UI is a fallback for // when the operator wants to click instead of type. Destructive-risk - // actions are never granted by loose assent. - if pending := extractPendingApprovals(lastAssistantCalls); len(pending) > 0 && isAssent(message) { + // actions are never granted by loose assent — they need the stricter + // isTypedConfirmation ("I confirm ...", per SOUL.md's guidance for what + // to ask the operator to type). + pending := extractPendingApprovals(lastAssistantCalls) + assent := isAssent(message) + typedConfirm := isTypedConfirmation(message) + if len(pending) > 0 && (assent || typedConfirm) { var granted, blocked []string for _, p := range pending { - if p.destructive { + if p.destructive && !typedConfirm { blocked = append(blocked, p.execID) continue } + if !p.destructive && !assent { + continue // typed-confirm alone doesn't grant a non-destructive item without also reading as assent + } ok, status, aerr := a.approveExecution(ctx, p.execID) if aerr != nil { slog.Error("nomos: chat-assent approve", "execution", p.execID, "error", aerr) diff --git a/cmd/nomos/assent.go b/cmd/nomos/assent.go index 5681d40..e1f3089 100644 --- a/cmd/nomos/assent.go +++ b/cmd/nomos/assent.go @@ -85,6 +85,23 @@ func isAssent(msg string) bool { return false } +// isTypedConfirmation reports whether msg is an explicit confirmation strong +// enough to grant a DESTRUCTIVE pending action. Deliberately a separate, +// stricter check from isAssent: a bare "yes"/"go ahead"/"proceed" must never +// grant something destructive, only an explicit "confirm" statement does — +// this is the typed-confirmation phrase SOUL.md tells the operator to use +// ("I confirm destroy 135"). Still negation-aware for the same reason as +// isAssent: "don't confirm yet" must not accidentally match. +func isTypedConfirmation(msg string) bool { + m := " " + strings.ToLower(strings.TrimSpace(msg)) + " " + for _, w := range negationWords { + if strings.Contains(m, " "+w+" ") || strings.HasPrefix(strings.TrimSpace(m), w+" ") || strings.HasPrefix(strings.TrimSpace(m), w+",") { + return false + } + } + return strings.Contains(m, "confirm") +} + // approveExecution grants (or denies) a pending execution via the same HTTP // endpoint the chat UI's Approve button calls, so both paths share one code // path server-side (executeApprovedAction) and one audit trail. Returns the diff --git a/cmd/nomos/assent_test.go b/cmd/nomos/assent_test.go index 72c10fb..64f4507 100644 --- a/cmd/nomos/assent_test.go +++ b/cmd/nomos/assent_test.go @@ -45,6 +45,29 @@ func TestIsAssent_NegationBeatsAssentWord(t *testing.T) { } } +func TestIsTypedConfirmation(t *testing.T) { + positive := []string{ + "I confirm destroy 135 in strong", + "confirm", + "Confirmed.", + "yes I confirm", + } + for _, c := range positive { + if !isTypedConfirmation(c) { + t.Errorf("isTypedConfirmation(%q) = false, want true", c) + } + } + negative := []string{ + "yes", "go ahead", "do it", "proceed", "lgtm", // loose assent must NOT satisfy this + "no, don't confirm yet", "wait", "", + } + for _, c := range negative { + if isTypedConfirmation(c) { + t.Errorf("isTypedConfirmation(%q) = true, want false (only explicit confirm should pass)", c) + } + } +} + func TestExtractPendingApprovals(t *testing.T) { mkCall := func(text string) persistedCall { b, _ := json.Marshal(text) diff --git a/web/src/lib/components/InlineApproval.svelte b/web/src/lib/components/InlineApproval.svelte index 897a4f3..01c1c86 100644 --- a/web/src/lib/components/InlineApproval.svelte +++ b/web/src/lib/components/InlineApproval.svelte @@ -26,6 +26,12 @@ return typeof v === 'string' && v ? v : 'Execution failed.' } + function outputText(e: Execution | undefined): string { + const r = e?.result as Record | 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'} -
- - Provisioned successfully{e?.duration_ms ? ` in ${Math.round(e.duration_ms / 1000)}s` : ''}. See the Executions view for details. +
+
+ + Completed{e?.duration_ms ? ` in ${Math.round(e.duration_ms / 1000)}s` : ''} on {approval.target}. +
+ {#if outputText(e)} + 
{outputText(e)}
+ {/if}
{:else if p === 'failed'}
@@ -116,7 +127,21 @@ {:else if p === 'running' || p === 'deciding'}
- {p === 'deciding' ? 'Submitting approval…' : `Provisioning ${approval.target}… (this can take a minute)`} + {p === 'deciding' ? 'Submitting approval…' : `Running on ${approval.target}… (this can take a minute)`} +
+ {:else if approval.destructive} +
+ + + DESTRUCTIVE — {approval.action} on {approval.target}. Type + "I confirm" in chat, or use the button. + + +
{:else}
diff --git a/web/src/lib/stores/chat.ts b/web/src/lib/stores/chat.ts index bc03c7a..1dae6de 100644 --- a/web/src/lib/stores/chat.ts +++ b/web/src/lib/stores/chat.ts @@ -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) }) } } diff --git a/web/src/pages/Chat.svelte b/web/src/pages/Chat.svelte index 16d68d8..e829d05 100644 --- a/web/src/pages/Chat.svelte +++ b/web/src/pages/Chat.svelte @@ -1,17 +1,13 @@