From 9376dc7d89c9cc4857387a94702213dc182abcab Mon Sep 17 00:00:00 2001 From: dtoro Date: Thu, 9 Jul 2026 23:19:55 +0200 Subject: [PATCH] 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). --- internal/mcp/server.go | 36 +++++++++++++++--- nomos/SOUL.md | 5 +++ web/src/pages/Chat.svelte | 78 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 111 insertions(+), 8 deletions(-) diff --git a/internal/mcp/server.go b/internal/mcp/server.go index e81dac1..b3cbade 100644 --- 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 } diff --git a/nomos/SOUL.md b/nomos/SOUL.md index e48c6f0..26d4873 100644 --- 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 diff --git a/web/src/pages/Chat.svelte b/web/src/pages/Chat.svelte index ff859a3..3692efb 100644 --- a/web/src/pages/Chat.svelte +++ b/web/src/pages/Chat.svelte @@ -1,13 +1,17 @@