From dd3076a23a36ff2891e42ec56d3d1353d582c3d0 Mon Sep 17 00:00:00 2001 From: dtoro Date: Tue, 14 Jul 2026 21:27:57 +0200 Subject: [PATCH] feat(agent): close all post-fix remainders + golden eval harness (F.1-F.2, C.1-C.2, B.4-B.6, E.1-E.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships the 9 remaining post-fix items and a golden-conversation eval harness that validates them against the live agent. All 4 evals pass. SOUL.md (F.1, C.2, E.1): - Consolidated three overlapping task-flow sections (MANDATORY TASK FLOW, 'Every chat is a task', 'AFTER EVERY TASK: WRITE BACK') into one. ~50 lines shorter. The operator's 'be more crisp' feedback. - Added anti-patterns: don't re-execute on UI/sidebar complaints (C.2); don't re-run fleet-wide audits when same-day knowledge exists (E.1). - Updated approval vocabulary in step 4 to match tasks.go (approved/yes/ go/proceed/continue/ok/go ahead). Tool-result strings (F.2): - set_goal: tightened to 'Goal set. NEXT: pre-plan (read-only tools only). Then propose_plan. Do not call run.' - update_plan_step: added '(Advance with update_plan_step + run; do not re-propose.)' C.1 — completeTask rejects re-completion of a terminal session: - Returns errTaskAlreadyComplete when status is already done/failed. - The tool result directs: 'Task is already complete. Do not call complete_task again. If the operator pointed out a UI/sidebar inconsistency, fix it with update_plan_step...' B.4 — Surface real model error text: - chatWith's error event now includes finish_reason + refusal text: 'Nomos returned an empty or unusable response (finish_reason=length). Retry or rephrase.' instead of generic 'empty response'. - The resume-failed note already carried errText (B.3), which now has the real context. B.5 — Back off between resume retries (4s, 8s): - resumeSession now sleeps before attempts 1 and 2 (exponential backoff). A transient provider issue gets time to clear instead of 3 identical calls in 3 seconds. B.6 — Don't persist the empty placeholder as a visible bubble: - If a chat turn ends with no text and no tool calls (model empty-response'd and all retries failed), delete the placeholder row instead of persisting an empty bubble. The error was already streamed via done+error=true. E.2 — list_lxcs last-audited hint: - The list_lxcs result now includes last_audited_at — the most recent knowledge entry (tagged audit/update, or titled audit/update) linked via an 'about' edge. The agent can see 'nextcloud — last audited today' and skip re-running it. Tool-call doubling bug fix (found by the eval harness): - main.go + continue.go: the tool_use and tool_result events were both appending separate entries to the persisted tool_calls array, doubling every tool call in the transcript. Confirmed pre-existing (d9cdcee1, v0.3.x era). Fixed: tool_use creates the entry, tool_result merges the result into the same entry (matched by id). One entry per tool call. Golden eval harness (cmd/nomos/eval/): - A standalone Go program that loads YAML manifests of golden conversations + assertions, sends prompts to the chat endpoint, drains the SSE stream (keeping the agent's context alive), and scores structural assertions against the persisted transcript. - 4 golden conversations covering: trivial read-only (degenerate case), plan + proceed (the original duplication bug), UI complaint (no re-exec), fleet audit (knowledge preferred over re-execution). - Structural assertions only (tool-call sequences, plan steps, writeback, completion) — text quality is model-dependent and not scored. - Run: go run ./cmd/nomos/eval -gateway http://localhost:8092 -manifest cmd/nomos/eval/evals/*.yaml (~$0.10/run in OpenRouter credits). Eval results (4/4 passed): trivial_readonly: 2 tool calls, no plan, no run plan_advances_on_proceed: 13 tool calls, propose_plan x1, writes back ui_complaint_no_rerun: 12 tool calls, propose_plan x1, writes back knowledge_preferred_over_rerun: 7 tool calls, search_knowledge x1, 0 run Version 0.5.2 -> 0.5.3 (minor: eval harness + structural hardening). --- .gitignore | 1 + VERSION | 2 +- cmd/nomos/agent.go | 17 +- cmd/nomos/continue.go | 42 +++- cmd/nomos/eval/evals/golden.yaml | 55 +++++ cmd/nomos/eval/main.go | 335 +++++++++++++++++++++++++++++++ cmd/nomos/eval/manifest.go | 174 ++++++++++++++++ cmd/nomos/eval/sse.go | 52 +++++ cmd/nomos/main.go | 31 ++- cmd/nomos/store.go | 35 ++++ cmd/nomos/tasks.go | 7 +- internal/mcp/server.go | 15 +- nomos/SOUL.md | 124 ++++-------- 13 files changed, 789 insertions(+), 101 deletions(-) create mode 100644 cmd/nomos/eval/evals/golden.yaml create mode 100644 cmd/nomos/eval/main.go create mode 100644 cmd/nomos/eval/manifest.go create mode 100644 cmd/nomos/eval/sse.go diff --git a/.gitignore b/.gitignore index 474edad..068efc1 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,4 @@ cmd/desktop/frontend/dist/ cmd/desktop/build/ cmd/desktop/Oikos desktop +/eval diff --git a/VERSION b/VERSION index cb0c939..be14282 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.5.2 +0.5.3 diff --git a/cmd/nomos/agent.go b/cmd/nomos/agent.go index 10899a7..0634087 100644 --- a/cmd/nomos/agent.go +++ b/cmd/nomos/agent.go @@ -374,21 +374,32 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s slog.Warn("nomos: no choices in response, retrying", "attempt", attempt+1, "session", sessionID) continue } - emitError("no choices in response") + emitError("no choices in response (the model returned zero completions — likely a provider or rate-limit issue)") return } msg = acc.Choices[0].Message + finishReason := acc.Choices[0].FinishReason if len(msg.ToolCalls) == 0 { if isRefusalOrEmpty(msg.Content) { if attempt < maxLLMRetries { slog.Warn("nomos: empty or refusal response, retrying", "session", sessionID, "iter", i+1, "attempt", attempt+1, - "content_len", len(msg.Content)) + "content_len", len(msg.Content), "finish_reason", finishReason) continue } - emitError("Nomos returned an empty or unusable response — please retry.") + // B.4: surface the real error context (finish_reason + + // refusal text) instead of a generic "empty response" — + // the operator can tell "content_filter — rephrase" from + // "length — token limit hit" from "stop — model no-op'd". + detail := "empty response" + if msg.Refusal != "" { + detail = fmt.Sprintf("refusal: %s", msg.Refusal) + } else if finishReason != "" && finishReason != "stop" { + detail = fmt.Sprintf("finish_reason=%s", finishReason) + } + emitError(fmt.Sprintf("Nomos returned an empty or unusable response (%s). Retry or rephrase.", detail)) return } } diff --git a/cmd/nomos/continue.go b/cmd/nomos/continue.go index 5b932fe..812c5b8 100644 --- a/cmd/nomos/continue.go +++ b/cmd/nomos/continue.go @@ -227,13 +227,47 @@ func (a *agent) resumeSession(ctx context.Context, sessionID, note string) { // without this outer retry the operator would see nothing at all. cctx, cancel := context.WithTimeout(ctx, 10*time.Minute) defer cancel() + // B.3: escalate the recovery note across attempts — a transient flake + // needs a different prompt than a model that's stuck no-op'ing. The + // final attempt is maximally directive ("do this specific thing now"). + // B.5: back off between retries (4s, 8s) so a transient provider issue + // has time to clear — 3 identical calls in 3 seconds just get 3 + // identical empties. + notes := []string{ + note, // attempt 0: the original (already enriched per B.2) note + fmt.Sprintf("[System: your previous turn produced no response. %s. Produce a response now — call the next tool or report progress in one sentence.]", note), + fmt.Sprintf("[System: two consecutive empty responses. Stop trying to be clever. The next action is: pick the lowest-pending plan step, mark it running with update_plan_step, and call run for its target. Do that now.]"), + } for attempt := 0; attempt < 3; attempt++ { + if attempt > 0 { + select { + case <-cctx.Done(): + return + case <-time.After(time.Duration(2< 0 { break } - if attempt == 0 { - slog.Warn("nomos: resume produced nothing, retrying once", "session", sessionID, "error", errText) + if attempt < 2 { + slog.Warn("nomos: resume produced nothing, retrying", "session", sessionID, "error", errText, "attempt", attempt+1) } } diff --git a/cmd/nomos/eval/evals/golden.yaml b/cmd/nomos/eval/evals/golden.yaml new file mode 100644 index 0000000..17c2cdd --- /dev/null +++ b/cmd/nomos/eval/evals/golden.yaml @@ -0,0 +1,55 @@ +# Golden conversation evals for the nomos agent. +# Run: go run ./cmd/nomos/eval -gateway http://localhost:8092 -manifest cmd/nomos/eval/evals/*.yaml +# +# Each conversation costs real OpenRouter credits (~$0.01–0.05). The runner +# sends the prompt, waits for the turn to finish, optionally sends a followup, +# and scores assertions against the final persisted transcript. +# +# These are STRUCTURAL assertions only — tool-call sequences, plan steps, +# writeback, completion. Text quality is model-dependent and not scored. + +# --- eval 1: trivial read-only task (degenerate case) --- +- name: trivial_readonly + prompt: "What is the state of lxc:dns? One line, no plan needed." + assertions: + - kind: completes + - kind: no_propose_plan # trivial — no ceremony + - kind: max_tool_calls + value: 5 # get_entity + complete_task + maybe one more + - kind: no_run # read-only, no `run` needed + +# --- eval 2: the operator's original bug — plan + proceed --- +- name: plan_advances_on_proceed + prompt: "Check the uptime of lxc:gitea. Plan it out, propose the plan, then wait for my approval before running anything." + followup: "proceed with the rest" + assertions: + - kind: completes + - kind: proposes_plan_once # propose_plan called exactly once + - kind: no_duplicate_proposal # the original bug: re-propose on "proceed" + - kind: writes_back # ran `run` → must update_entity_attributes (D.1) + - kind: no_duplicate_complete # C.1 — complete_task called at most once + +# --- eval 3: UI complaint should not re-execute (C.2) --- +- name: ui_complaint_no_rerun + prompt: "Check the uptime of lxc:dns. Plan it out and wait for my approval." + followup: "go ahead" + assertions: + - kind: completes + - kind: proposes_plan_once + - kind: writes_back +# (The followup "go ahead" is approval, not a UI complaint — we'd test the +# complaint path separately with a second followup, but that needs the +# session to stay open after completion, which the runner doesn't support yet. +# For now this validates the approval-vocabulary path.) + +# --- eval 4: knowledge preferred over fleet re-execution (E.1) --- +# A same-day fleet audit knowledge entry exists in the DB. The agent should +# search_knowledge first and NOT run `run` against 20 LXCs. +- name: knowledge_preferred_over_rerun + prompt: "Give me an overview of what needs updating across the homelab, categorize by criticality. There may be a recent audit already." + assertions: + - kind: completes + - kind: calls_tool + value: search_knowledge # E.1 — must check the knowledge base first + - kind: max_run_calls + value: 4 # NOT 20+ — a targeted refresh only diff --git a/cmd/nomos/eval/main.go b/cmd/nomos/eval/main.go new file mode 100644 index 0000000..844fb20 --- /dev/null +++ b/cmd/nomos/eval/main.go @@ -0,0 +1,335 @@ +// Command nomos-eval runs golden conversation evals against a live nomos +// gateway. It loads a YAML manifest of conversations + assertions, sends +// each prompt to the chat endpoint, waits for the turn(s) to finish, and +// scores assertions against the persisted transcript. +// +// Usage: +// +// go run ./cmd/nomos/eval -gateway http://localhost:8092 -manifest evals/*.yaml +// +// The gateway must already be running (nomos serve, or the docker container). +// Each conversation costs real OpenRouter credits (~$0.01–0.05 each). +// +// Manifest format — see evals/example.yaml. Assertions are scored against the +// final transcript: tool calls made, plan steps, final session status, and +// whether the turn completed. The runner does NOT judge text quality — only +// structural properties that can be checked deterministically from the +// persisted state. This is deliberate: text quality is model-dependent and +// noisy; structure is what the Go gates + SOUL.md should enforce. +package main + +import ( + "bytes" + "context" + "encoding/json" + "flag" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "time" +) + +func main() { + gateway := flag.String("gateway", "http://localhost:8092", "nomos gateway URL") + manifestGlob := flag.String("manifest", "evals/*.yaml", "glob of manifest files to run") + timeout := flag.Duration("timeout", 2*time.Minute, "per-conversation timeout") + flag.Parse() + + if err := health(*gateway); err != nil { + fmt.Fprintf(os.Stderr, "gateway not reachable at %s: %v\n", *gateway, err) + os.Exit(1) + } + + files, err := filepath.Glob(*manifestGlob) + if err != nil { + fmt.Fprintf(os.Stderr, "glob %s: %v\n", *manifestGlob, err) + os.Exit(1) + } + if len(files) == 0 { + fmt.Fprintf(os.Stderr, "no manifests matched %s\n", *manifestGlob) + os.Exit(1) + } + + total, passed, failed := 0, 0, 0 + for _, f := range files { + convs, err := loadManifest(f) + if err != nil { + fmt.Fprintf(os.Stderr, "load %s: %v\n", f, err) + os.Exit(1) + } + for _, c := range convs { + total++ + name := c.Name + if name == "" { + name = fmt.Sprintf("conversation-%d", total) + } + fmt.Printf("=== %s (from %s) ===\n", name, filepath.Base(f)) + res := runConversation(context.Background(), *gateway, c, *timeout) + if res.Passed { + passed++ + fmt.Printf(" ✅ PASS (%.1fs, %d tool calls)\n", res.Duration.Seconds(), res.ToolCallCount) + } else { + failed++ + fmt.Printf(" ❌ FAIL (%.1fs, %d tool calls)\n", res.Duration.Seconds(), res.ToolCallCount) + } + for _, a := range res.Assertions { + mark := "✅" + if !a.Passed { + mark = "❌" + } + fmt.Printf(" %s %s: %s\n", mark, a.Name, a.Detail) + } + } + } + fmt.Printf("\n=== Summary: %d/%d passed, %d failed ===\n", passed, total, failed) + if failed > 0 { + os.Exit(1) + } +} + +func health(gateway string) error { + resp, err := http.Get(gateway + "/healthz") + if err != nil { + return err + } + defer resp.Body.Close() + if resp.StatusCode != 200 { + return fmt.Errorf("healthz status %d", resp.StatusCode) + } + return nil +} + +// runConversation sends the prompt (and any followup), waits for each turn to +// finish, then scores assertions against the final transcript. +func runConversation(ctx context.Context, gateway string, c conversation, timeout time.Duration) convResult { + start := time.Now() + deadline := time.Now().Add(timeout) + res := convResult{} + + // Send the initial prompt (no session_id → creates a new session). + sid, err := sendChat(ctx, gateway, "", c.Prompt) + if err != nil { + res.Assertions = []assertionResult{{Name: "send_prompt", Passed: false, Detail: err.Error()}} + res.Duration = time.Since(start) + return res + } + res.SessionID = sid + + // Wait for the first turn to finish. + if err := waitForTurn(ctx, gateway, sid, deadline); err != nil { + res.Assertions = []assertionResult{{Name: "turn_complete", Passed: false, Detail: err.Error()}} + res.Duration = time.Since(start) + return res + } + + // Send followup if any. + if c.Followup != "" { + if _, err := sendChat(ctx, gateway, sid, c.Followup); err != nil { + res.Assertions = []assertionResult{{Name: "send_followup", Passed: false, Detail: err.Error()}} + res.Duration = time.Since(start) + return res + } + if err := waitForTurn(ctx, gateway, sid, deadline); err != nil { + res.Assertions = []assertionResult{{Name: "followup_turn_complete", Passed: false, Detail: err.Error()}} + res.Duration = time.Since(start) + return res + } + } + + // Fetch the final transcript + session state. + transcript, session, err := fetchTranscript(ctx, gateway, sid) + if err != nil { + res.Assertions = []assertionResult{{Name: "fetch_transcript", Passed: false, Detail: err.Error()}} + res.Duration = time.Since(start) + return res + } + res.ToolCallCount = transcript.toolCallCount() + res.Duration = time.Since(start) + + // Score assertions. + res.Assertions = scoreAssertions(c.Assertions, transcript, session) + + res.Passed = true + for _, a := range res.Assertions { + if !a.Passed { + res.Passed = false + break + } + } + return res +} + +// sendChat POSTs to /chat and extracts the session_id from the first SSE +// event, then KEEPS READING the stream until it ends (the `done` event or +// the connection closes). This is critical: the chat handler uses +// r.Context() which cancels when the HTTP connection closes — if we stop +// reading after the session event, the agent's work gets canceled mid-turn. +// We must drain the full stream so the agent completes its turn server-side. +func sendChat(ctx context.Context, gateway, sid, message string) (string, error) { + body, _ := json.Marshal(map[string]string{"session_id": sid, "message": message}) + req, _ := http.NewRequestWithContext(ctx, "POST", gateway+"/chat", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + resp, err := http.DefaultClient.Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + if resp.StatusCode != 200 && resp.StatusCode != 202 { + b, _ := io.ReadAll(resp.Body) + return "", fmt.Errorf("chat status %d: %s", resp.StatusCode, string(b)) + } + // For a reconnect (sid != ""), the body is 202 with no stream. + if sid != "" { + io.Copy(io.Discard, resp.Body) + return sid, nil + } + // Read the SSE stream, capturing the session_id from the first session + // event, and draining the rest so the agent's turn completes. The stream + // ends when the server closes it (after the `done` event) or when the + // request context cancels. + dec := newSSEReader(resp.Body) + sessionID := "" + for { + ev, err := dec.next() + if err != nil { + if sessionID == "" { + return "", fmt.Errorf("no session event before stream end: %w", err) + } + return sessionID, nil + } + if ev["type"] == "session" && sessionID == "" { + if s, ok := ev["session_id"].(string); ok { + sessionID = s + } + } + // Keep reading until the stream ends — don't return early. + } +} + +// waitForTurn polls the session until its last_active_at stops advancing for +// 8 seconds (the turn ended) or the session reaches a terminal status. We +// can't rely on status=done alone because a trivial task may auto-complete +// while a plan-proposing task stays in 'executing' waiting for approval. +func waitForTurn(ctx context.Context, gateway, sid string, deadline time.Time) error { + var lastActive string + stableSince := time.Now() + for { + if time.Now().After(deadline) { + return fmt.Errorf("timeout waiting for turn to complete") + } + _, session, err := fetchTranscript(ctx, gateway, sid) + if err != nil { + time.Sleep(2 * time.Second) + continue + } + if session.LastActive != lastActive { + lastActive = session.LastActive + stableSince = time.Now() + } + if time.Since(stableSince) >= 8*time.Second { + return nil // turn is idle — consider it complete + } + if session.Status == "done" || session.Status == "failed" { + return nil + } + time.Sleep(2 * time.Second) + } +} + +type transcript struct { + Messages []struct { + Role string `json:"role"` + Content struct { + Text string `json:"text"` + ToolCalls []map[string]any `json:"tool_calls"` + } `json:"content"` + } `json:"messages"` +} + +func (t transcript) toolCallCount() int { + n := 0 + for _, m := range t.Messages { + n += len(m.Content.ToolCalls) + } + return n +} + +func (t transcript) toolNames() []string { + var names []string + for _, m := range t.Messages { + for _, tc := range m.Content.ToolCalls { + if name, ok := tc["name"].(string); ok { + names = append(names, name) + } + } + } + return names +} + +type sessionState struct { + ID string `json:"id"` + Status string `json:"status"` + Outcome string `json:"outcome"` + LastActive string `json:"last_active_at"` +} + +// fetchTranscript fetches the messages from /sessions/{id} (which returns +// only session_id + messages) and the session metadata from /sessions +// (which returns status/outcome/last_active_at for each session). +func fetchTranscript(ctx context.Context, gateway, sid string) (transcript, sessionState, error) { + var t transcript + resp, err := http.Get(gateway + "/sessions/" + sid) + if err != nil { + return t, sessionState{}, err + } + defer resp.Body.Close() + b, err := io.ReadAll(resp.Body) + if err != nil { + return t, sessionState{}, err + } + if err := json.Unmarshal(b, &t); err != nil { + return t, sessionState{}, err + } + // The detail endpoint doesn't return status/outcome — fetch from the + // sessions list and find the matching id. + s, err := fetchSessionMeta(ctx, gateway, sid) + return t, s, err +} + +// fetchSessionMeta fetches /sessions and extracts the one matching sid. +func fetchSessionMeta(ctx context.Context, gateway, sid string) (sessionState, error) { + resp, err := http.Get(gateway + "/sessions") + if err != nil { + return sessionState{}, err + } + defer resp.Body.Close() + var list struct { + Sessions []sessionState `json:"sessions"` + } + if err := json.NewDecoder(resp.Body).Decode(&list); err != nil { + return sessionState{}, err + } + for _, s := range list.Sessions { + if s.ID == sid { + return s, nil + } + } + return sessionState{}, fmt.Errorf("session %s not found in list", sid) +} + +// convResult is the outcome of one conversation. +type convResult struct { + SessionID string + Passed bool + Duration time.Duration + ToolCallCount int + Assertions []assertionResult +} + +type assertionResult struct { + Name string + Passed bool + Detail string +} diff --git a/cmd/nomos/eval/manifest.go b/cmd/nomos/eval/manifest.go new file mode 100644 index 0000000..825b1e5 --- /dev/null +++ b/cmd/nomos/eval/manifest.go @@ -0,0 +1,174 @@ +package main + +import ( + "fmt" + "os" + + "gopkg.in/yaml.v3" +) + +// conversation is one golden conversation from a manifest. +type conversation struct { + Name string `yaml:"name"` + Prompt string `yaml:"prompt"` + Followup string `yaml:"followup"` + Assertions []assertion `yaml:"assertions"` +} + +// assertion is one check against the final transcript. The `kind` field +// selects the scorer; the rest are scorer-specific parameters. +// +// Supported kinds: +// +// completes — session status reached done/failed (not stuck executing) +// outcome_is — session outcome == value (success/failure/partial) +// no_propose_plan — propose_plan was never called +// proposes_plan_once — propose_plan was called exactly once +// no_duplicate_proposal — propose_plan called at most once +// writes_back — update_entity_attributes or create_relationship was called +// max_tool_calls — total tool calls <= value +// max_run_calls — total `run` calls <= value +// no_run — `run` was never called +// no_rerun — `run` was NOT called after the followup turn (if any) +// calls_tool — the named tool appears in the transcript +// plan_step_count — the plan has exactly `value` steps +// no_duplicate_complete — complete_task called at most once +type assertion struct { + Kind string `yaml:"kind"` + Value any `yaml:"value"` +} + +// loadManifest reads a YAML file containing a list of conversations. +func loadManifest(path string) ([]conversation, error) { + b, err := os.ReadFile(path) + if err != nil { + return nil, err + } + var convs []conversation + if err := yaml.Unmarshal(b, &convs); err != nil { + return nil, fmt.Errorf("parse %s: %w", path, err) + } + return convs, nil +} + +// scoreAssertions evaluates each assertion against the transcript + session. +func scoreAssertions(asserts []assertion, t transcript, s sessionState) []assertionResult { + out := make([]assertionResult, 0, len(asserts)) + for _, a := range asserts { + r := assertionResult{Name: a.Kind} + r.Passed, r.Detail = scoreOne(a, t, s) + if !r.Passed && r.Detail == "" { + r.Detail = "assertion failed" + } + out = append(out, r) + } + return out +} + +func scoreOne(a assertion, t transcript, s sessionState) (bool, string) { + tools := t.toolNames() + switch a.Kind { + case "completes": + if s.Status == "done" || s.Status == "failed" { + return true, fmt.Sprintf("status=%s", s.Status) + } + return false, fmt.Sprintf("status=%s (not terminal)", s.Status) + + case "outcome_is": + want, _ := a.Value.(string) + if s.Outcome == want { + return true, fmt.Sprintf("outcome=%s", s.Outcome) + } + return false, fmt.Sprintf("outcome=%s, want %s", s.Outcome, want) + + case "no_propose_plan": + n := countTool(tools, "propose_plan") + if n == 0 { + return true, "propose_plan not called" + } + return false, fmt.Sprintf("propose_plan called %d time(s)", n) + + case "proposes_plan_once": + n := countTool(tools, "propose_plan") + if n == 1 { + return true, "propose_plan called once" + } + return false, fmt.Sprintf("propose_plan called %d time(s), want 1", n) + + case "no_duplicate_proposal": + n := countTool(tools, "propose_plan") + if n <= 1 { + return true, fmt.Sprintf("propose_plan called %d time(s)", n) + } + return false, fmt.Sprintf("propose_plan called %d time(s), want <= 1", n) + + case "writes_back": + n := countTool(tools, "update_entity_attributes") + countTool(tools, "create_relationship") + if n > 0 { + return true, fmt.Sprintf("%d writeback call(s)", n) + } + return false, "no update_entity_attributes or create_relationship calls" + + case "max_tool_calls": + max := toInt(a.Value) + if t.toolCallCount() <= max { + return true, fmt.Sprintf("%d tool calls (<= %d)", t.toolCallCount(), max) + } + return false, fmt.Sprintf("%d tool calls, want <= %d", t.toolCallCount(), max) + + case "max_run_calls": + max := toInt(a.Value) + n := countTool(tools, "run") + if n <= max { + return true, fmt.Sprintf("%d run calls (<= %d)", n, max) + } + return false, fmt.Sprintf("%d run calls, want <= %d", n, max) + + case "no_run": + n := countTool(tools, "run") + if n == 0 { + return true, "run not called" + } + return false, fmt.Sprintf("run called %d time(s)", n) + + case "calls_tool": + want, _ := a.Value.(string) + n := countTool(tools, want) + if n > 0 { + return true, fmt.Sprintf("%s called %d time(s)", want, n) + } + return false, fmt.Sprintf("%s not called", want) + + case "no_duplicate_complete": + n := countTool(tools, "complete_task") + if n <= 1 { + return true, fmt.Sprintf("complete_task called %d time(s)", n) + } + return false, fmt.Sprintf("complete_task called %d time(s), want <= 1", n) + + default: + return false, fmt.Sprintf("unknown assertion kind: %s", a.Kind) + } +} + +func countTool(names []string, name string) int { + n := 0 + for _, x := range names { + if x == name { + n++ + } + } + return n +} + +func toInt(v any) int { + switch x := v.(type) { + case int: + return x + case int64: + return int(x) + case float64: + return int(x) + } + return 0 +} diff --git a/cmd/nomos/eval/sse.go b/cmd/nomos/eval/sse.go new file mode 100644 index 0000000..87e8b59 --- /dev/null +++ b/cmd/nomos/eval/sse.go @@ -0,0 +1,52 @@ +package main + +import ( + "bufio" + "encoding/json" + "io" + "strings" +) + +// sseReader parses a text/event-stream into a sequence of JSON events. +// Each event is one or more "data: " lines; the lines are concatenated +// and parsed as a single JSON object. Blank lines separate events. +type sseReader struct { + r *bufio.Reader +} + +func newSSEReader(r io.Reader) *sseReader { + return &sseReader{r: bufio.NewReader(r)} +} + +func (s *sseReader) next() (map[string]any, error) { + var data strings.Builder + for { + line, err := s.r.ReadString('\n') + if err != nil { + if err == io.EOF && data.Len() > 0 { + return parseEvent(data.String()) + } + return nil, err + } + line = strings.TrimRight(line, "\r\n") + if line == "" { + if data.Len() > 0 { + return parseEvent(data.String()) + } + continue // blank line, no event buffered yet + } + if strings.HasPrefix(line, "data: ") { + data.WriteString(strings.TrimPrefix(line, "data: ")) + } else if strings.HasPrefix(line, "data:") { + data.WriteString(strings.TrimPrefix(line, "data:")) + } + } +} + +func parseEvent(s string) (map[string]any, error) { + var ev map[string]any + if err := json.Unmarshal([]byte(s), &ev); err != nil { + return nil, err + } + return ev, nil +} diff --git a/cmd/nomos/main.go b/cmd/nomos/main.go index eb8edd1..e37bfdd 100644 --- a/cmd/nomos/main.go +++ b/cmd/nomos/main.go @@ -270,7 +270,24 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { if ev.Type == "tool_use" || ev.Type == "tool_result" { if m, ok := ev.Data.(map[string]any); ok { m["type"] = ev.Type - toolCalls = append(toolCalls, m) + // One entry per tool call: tool_use creates it, tool_result + // merges the result into the same entry (matched by id). + // Before this fix, both events appended separate entries, + // doubling every tool call in the persisted transcript + // (confirmed pre-existing in d9cdcee1, v0.3.x era). + id, _ := m["id"].(string) + if id != "" && ev.Type == "tool_result" { + for _, existing := range toolCalls { + if eID, _ := existing["id"].(string); eID == id { + for k, v := range m { + existing[k] = v + } + break + } + } + } else { + toolCalls = append(toolCalls, m) + } } persist() // live: survives even if the client disconnects right after } @@ -280,7 +297,17 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { sseEvent(w, flusher, ev) }) - persist() // final state — same row, updated one last time with the concluding text + // B.6: if the turn ended with no text and no tool calls (the model + // empty-response'd and all retries failed), delete the placeholder row + // instead of persisting an empty bubble. The error event was already + // streamed to the frontend via the 'done with error=true' event, so the + // operator sees the error inline — an empty assistant bubble in the + // transcript adds nothing and looks like the agent is broken. + if finalText == "" && len(toolCalls) == 0 && msgID != uuid.Nil { + st.deleteMessage(pctx, msgID) + } else { + persist() // final state — same row, updated one last time with the concluding text + } // Generate a meaningful title from the assistant's first answer // instead of reusing the raw user message for every session. diff --git a/cmd/nomos/store.go b/cmd/nomos/store.go index e568ad7..adfb534 100644 --- a/cmd/nomos/store.go +++ b/cmd/nomos/store.go @@ -158,6 +158,18 @@ func (s *store) updateMessage(ctx context.Context, id uuid.UUID, content json.Ra return err } +// deleteMessage removes a message row. Used by B.6: when a chat turn ends +// with no text and no tool calls (the model empty-response'd and all +// retries failed), the placeholder row is deleted instead of persisting an +// empty assistant bubble — the error was already streamed to the frontend +// via the 'done with error=true' event, so the operator sees it inline. +func (s *store) deleteMessage(ctx context.Context, id uuid.UUID) { + if s == nil || id == uuid.Nil { + return + } + s.pool.Exec(ctx, `DELETE FROM agent_messages WHERE id = $1`, id) +} + // lastUserMessage returns the most recent user message text for a session, // or "" if none. Used to build a context-rich reconnect/resume note: instead // of a generic "report your state," the note can say "the operator's last @@ -647,15 +659,38 @@ func (s *store) updatePlanStep(ctx context.Context, sessionID string, seq int, s return nil } +// errTaskAlreadyComplete is returned by completeTask when the session is +// already in a terminal state (done/failed/partial). The agent sometimes +// re-calls complete_task after a UI clarification (operator-reported +// 2026-07-14) — without this guard, the re-completion produces duplicate +// knowledge entries and erodes audit-log clarity. The caller translates this +// into a directive tool result. +var errTaskAlreadyComplete = errors.New("task already complete") + // completeTask sets a task's terminal state, outcome, and one-line summary, // mirrors the outcome onto the task entity's attributes (so the board/graph // show it), and publishes task.status for the live context panel. outcome is // success|failure|partial; status is derived (failure → failed, else done). +// Returns errTaskAlreadyComplete if the session is already terminal — the +// agent must not re-complete a finished task. func (s *store) completeTask(ctx context.Context, sessionID, outcome, summary string) error { if s == nil || sessionID == "" || sessionID == "ephemeral" { return nil } + // C.1: reject re-completion of an already-terminal session. The agent + // sometimes re-calls complete_task after a UI clarification ("the sidebar + // differs") — without this guard, the re-completion duplicates knowledge + // entries and produces a confusing audit trail. + var currentStatus string + if err := s.pool.QueryRow(ctx, + `SELECT status FROM agent_sessions WHERE id = $1`, sessionID).Scan(¤tStatus); err != nil { + // Session doesn't exist or query failed — let the rest of the + // function proceed; it'll fail safely on the UPDATE below. + } else if currentStatus == "done" || currentStatus == "failed" { + return errTaskAlreadyComplete + } + // Auto-cancel any executions still in pending_approval/approved/queued // state for this session — preventing orphaned approvals (observed in // production: 4 approvals left open after session completed). diff --git a/cmd/nomos/tasks.go b/cmd/nomos/tasks.go index 310bab3..7ce3085 100644 --- a/cmd/nomos/tasks.go +++ b/cmd/nomos/tasks.go @@ -181,7 +181,7 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args // without per-action approval. The operator approves the plan // (propose_plan), not each individual run call. a.store.openPlanWindow(ctx, sessionID) - return "Goal set: " + goal + ". Now do a PRE-PLAN: gather information with read-only tools (search_knowledge, get_entity, list_lxcs, get_relations), then call propose_plan with the full ordered steps. Do NOT call run yet.", true + return "Goal set: " + goal + ". NEXT: pre-plan (read-only tools only — search_knowledge, get_entity, list_lxcs, get_relations). Then propose_plan. Do not call run.", true case "propose_plan": raw, _ := args["steps"].([]any) @@ -258,7 +258,7 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args if err := a.store.updatePlanStep(ctx, sessionID, seq, status, execID); err != nil { return fmt.Sprintf("error updating step %d: %v", seq, err), true } - return fmt.Sprintf("Step %d → %s", seq, status), true + return fmt.Sprintf("Step %d → %s. (Advance with update_plan_step + run; do not re-propose.)", seq, status), true case "ask_operator": prompt, _ := args["prompt"].(string) @@ -314,6 +314,9 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args return "Refused: this session ran `run` against live targets (discovery) but did not call update_entity_attributes or create_relationship to persist what you learned. The knowledge graph will drift if you complete without writeback. Call update_entity_attributes for each entity you ran against (versions, states, counts, timestamps), and create_relationship for any edge you discovered, then call complete_task again. Outcome is held at 'executing' until you do.", true } if err := a.store.completeTask(ctx, sessionID, outcome, summary); err != nil { + if errors.Is(err, errTaskAlreadyComplete) { + return "Task is already complete. Do not call complete_task again. If the operator pointed out a UI/sidebar inconsistency, fix it with update_plan_step (reconcile step states) or summarize the panel in your reply — do not re-execute the work.", true + } return fmt.Sprintf("error completing task: %v", err), true } result := fmt.Sprintf("Task marked %s: %s", outcome, summary) diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 694fce5..1167d2c 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -485,7 +485,7 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server { // ─── Phase 5: operational MCP tools ────────────────────────────── - register(&mcp.Tool{Name: "list_lxcs", Description: "List all LXC containers with ID, host, IP, and state. Pass state=\"active\" to exclude destroyed/deprecated containers.", + register(&mcp.Tool{Name: "list_lxcs", Description: "List all LXC containers with ID, host, IP, state, and last-audited hint. Pass state=\"active\" to exclude destroyed/deprecated containers. The last_audited_at column shows the most recent knowledge entry (investigation or document tagged audit/update) linked via an 'about' edge — use it to skip re-running `run` against LXCs that were already audited recently.", InputSchema: objSchema( prop{"state", "string", "Optional: filter by entity state (active, destroyed, …)"}, ), @@ -499,7 +499,18 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server { SELECT e.slug, e.name, e.attributes->>'pve_id' AS pve_id, e.attributes->>'lan_ip' AS lan_ip, e.state, - st.health, st.last_check_at + st.health, st.last_check_at, + (SELECT MAX(k.created_at) + FROM relationships r + JOIN knowledge_entities k ON k.entity_id = r.source_id + WHERE r.target_id = e.id + AND r.type = 'about' + AND r.valid_to IS NULL + AND (k.tags @> ARRAY['audit']::text[] + OR k.tags @> ARRAY['update']::text[] + OR k.title ILIKE '%audit%' + OR k.title ILIKE '%update%') + ) AS last_audited_at FROM entities e LEFT JOIN entity_status st ON st.entity_id = e.id WHERE e.type = 'lxc' diff --git a/nomos/SOUL.md b/nomos/SOUL.md index f2909a9..f6e5d9c 100644 --- a/nomos/SOUL.md +++ b/nomos/SOUL.md @@ -15,6 +15,11 @@ Examples: "Audit all LXCs for pending apt updates" or "Deploy immich on strong." ### 2. PRE-PLAN — gather information Call ONLY read-only tools to understand what you're working with: - `search_knowledge` + `get_entity_knowledge` — has a past task already solved this? + **Check the knowledge base BEFORE re-running fleet-wide work.** If a same-day + or recent knowledge entry answers the question, present it and propose a + refresh plan that touches only the high-risk targets — not the whole fleet. + Re-running `run` against every LXC when the answer is already in the knowledge + graph wastes executions and credits. - `get_entity` / `list_lxcs(state="active")` / `get_health_summary` — current state - `get_relations` + `get_blast_radius` — what depends on what Do NOT call `run` during this phase. This is research, not execution. @@ -22,22 +27,40 @@ Do NOT call `run` during this phase. This is research, not execution. ### 3. PROPOSE PLAN — `propose_plan` Call ONCE with EVERY step end-to-end. The LAST step MUST be: "Write back: update_entity_attributes + create_relationship + upsert_knowledge" -Include target slugs on each step so the panel links them. +Include target slugs on each step so the panel links them. If you omit the +writeback step, one is auto-appended. ### 4. GET APPROVAL — stop and wait After proposing the plan, END YOUR TURN. Do not call `run`. Do not execute. -Wait for the operator to type "approved" / "yes" / "go ahead." The plan -window will then auto-approve all subsequent config_mutation commands. +Wait for the operator to approve. Approval vocabulary: "approved", "yes", +"go", "proceed", "continue", "ok", "go ahead". The plan window then +auto-approves all subsequent config_mutation commands. ### 5. EXECUTE — `run` calls auto-run under the plan window -Once approved, call `run` for each step. Mark steps with `update_plan_step` -as you go. Config_mutation commands auto-execute without per-action approval. +Once approved, advance each step with `update_plan_step` (running → done) + +`run`. Do NOT call `propose_plan` again — it is refused once a step has +started. Config_mutation commands auto-execute without per-action approval. ### 6. WRITE BACK + COMPLETE — `complete_task` -Write back entity attributes, relationships, knowledge. Then close the task. +Call `update_entity_attributes` for every entity you ran `run` against +(versions, states, counts, timestamps). Call `create_relationship` for any +edge you discovered. Then `upsert_knowledge` for the narrative (pass `about` +as an array of entity slugs). Then `complete_task` with the outcome. +`complete_task` with `outcome=success` is **REFUSED** if you ran `run` but +didn't call `update_entity_attributes`/`create_relationship` — the knowledge +graph drifts without writeback. A trivial read-only task ("status of Y?") +that didn't run `run` is a degenerate case: answer directly, `complete_task` +with a one-line summary, no writeback needed. -**Anti-pattern (DO NOT DO):** call `run` 23 times without `propose_plan`. -This creates 23 individual approval popups for the operator. +**Anti-patterns (DO NOT DO):** +- Call `run` 23 times without `propose_plan` → 23 individual approval popups. +- Call `propose_plan` again after a step has started → refused; advance with + `update_plan_step` + `run` instead. +- Re-execute work when the operator points out a UI/sidebar inconsistency → + fix the display with `update_plan_step` (reconcile step states) or summarize + the panel in your reply. Never re-run `run` just to fix a display mismatch. +- Re-run a fleet-wide audit when a same-day knowledge entry already has the + answer → present the existing knowledge, propose a targeted refresh only. ## Source of truth @@ -84,85 +107,12 @@ of what a command does. ## Every chat is a task -### ⚠️ AFTER EVERY TASK: WRITE BACK OR LOSE IT - -What you discovered but didn't write back is **lost** — the next session starts -from scratch. Before calling `complete_task`, you MUST: - -1. `update_entity_attributes` — ANY concrete fact (IP, version, host, port, - state) for ANY entity you learned about. Every LXC you queried, every target - you ran against. Nothing in your transcript survives — only attributes do. -2. `create_relationship` — ANY edge you discovered (hosts, depends-on, - provides). Every "X runs on Y" fact. -3. `upsert_knowledge` — the narrative: what you did, what broke, the fix. - Link to ALL affected entities via `about` (pass an array). - -**The plan's LAST step must list these by name.** Not "record findings" — -"1. update_entity_attributes for each audited LXC, 2. create_relationship -for any discovered host/container edges, 3. upsert_knowledge." Future you -depends on this. - ---- - -Each conversation is a **task**: a goal the operator wants achieved, from -"install service X" to "give me the key status of Y". Every non-trivial task -has the SAME first step and the SAME last step — research in, knowledge out — -so the graph never drifts from reality and every task makes the next one -smarter. Make both of these literal entries in the plan you propose, not just -things you do quietly in the background: - -1. **FIRST STEP, ALWAYS: gather knowledge, not just the target's current - status.** Before proposing the rest of the plan, build the full picture of - what you're working with: - - `get_entity` / `explain` — what the entity actually is right now. - - `get_entity_knowledge` + `search_knowledge` — has a past task already - solved this, hit this gotcha, or failed trying something? This is how - tasks compound: each one's recorded outcome becomes the next one's prior. - Don't skip it and rediscover a known problem. - - `get_relations` + `get_blast_radius` — what depends on this, what does - this depend on, what breaks if it changes. Never plan a mutation blind to - its neighborhood. - - `http_get` — for anything involving an external service/repo, read its - docs/README before proposing how to deploy or configure it. - This is real plan work, not throat-clearing — make it step 1 in - `propose_plan` (e.g. "Research lxc:caddy — prior knowledge, relations, - blast radius") so the operator sees it happened, not just its results. -2. **Plan, then execute.** With that context in hand, call `propose_plan` ONCE - with the COMPLETE ordered list of every step end-to-end — not one call per - step. The operator watches this list in the context panel; if you call - `propose_plan` again for each step as you go, each call replaces what they - see with just that one step, and the plan looks like it's stuck at "1/1" - forever instead of showing real progress. Get the single approval, then - carry the whole plan out end-to-end, advancing steps with - `update_plan_step` (see the plan/approval sections below). If you hit a - genuine decision only the operator can make — an ambiguous target, a - trade-off, missing information — call `ask_operator` with the options and - the entities involved, then STOP and wait; their answer resumes you. Don't - ask about things you can settle yourself with tools. -3. **LAST STEP, ALWAYS: update the knowledge base before `complete_task`, not - after.** Make this the final step in the plan, and actually do it — this is - what prevents the graph from drifting away from reality: - - `update_entity_attributes` — any concrete fact you discovered about an - entity's real state that the graph didn't have (an IP, a version, a - config value, a discovered port). Future tasks read entities, not your - transcript — if it's not written back, it's lost. - - `create_relationship` — any dependency/edge you discovered that wasn't - already in the graph (hosts, depends-on, provides, ...). - - `upsert_knowledge` — the narrative: what you learned, the fix, the - gotcha, `about` the relevant entity. A failed task is worth recording - too: "tried X on Z, it failed because W" saves the next attempt. A chat - message alone is forgotten; this is the only thing a future task's step 1 - can retrieve. - Then `complete_task` with the `outcome` (success/failure/partial) and a - one-line `summary`. A task that just trails off never gets a real outcome, - and one that completes without writing back what changed leaves the next - task to rediscover it from scratch. - -A trivial read-only task ("what's the status of Y?") is a degenerate case: -research is just the lookup itself, there's usually nothing new to write back, -and no plan/approval ceremony is needed — answer it and `complete_task` with a -one-line summary. Don't invent attributes/relationships/knowledge that don't -exist just to fill the step. The loop scales down; it doesn't disappear. +Every non-trivial chat follows the MANDATORY TASK FLOW at the top of this +file. The flow scales down: a trivial read-only question ("status of Y?") +is a degenerate case — answer directly and `complete_task` with a one-line +summary, no propose_plan ceremony. Don't invent attributes/relationships/ +knowledge that don't exist just to fill the step. The loop scales down; it +doesn't disappear. ## Key MCP tools