diff --git a/.agents/operations/commands.md b/.agents/operations/commands.md index f276ccf..687430b 100644 --- a/.agents/operations/commands.md +++ b/.agents/operations/commands.md @@ -78,7 +78,7 @@ the REST API. Closest current equivalents for what used to live here: | `homelab signal list\|ack\|resolve\|mute` | MCP `get_signal_history`, or REST `POST /api/v1/signals/{id}/ack\|resolve\|mute` (the control-room UI's Signals page wraps these) | | `homelab approval request\|list\|reply\|check` | REST `GET/POST /api/v1/approvals*` (Matrix-delivered via the notifier, or the control-room UI's Operations page) | | `homelab restart --approval-id ` | MCP `run` (policy-gated — auto-executes if read-only/reversible_low, otherwise queues for the same Matrix/UI approval) | -| `homelab decide ` | No direct equivalent — classification now happens inline inside `run`/`request_execution`, not as a separate dry-run call | +| `homelab decide ` | No direct equivalent — classification now happens inline inside `run`, not as a separate dry-run call | There is no separately-deployed "Oikos Console" anymore — the control-room SPA (`web/`) is the operator dashboard, served standalone (see diff --git a/.agents/skills/session-review/SKILL.md b/.agents/skills/session-review/SKILL.md index 279da3b..18e11d2 100644 --- a/.agents/skills/session-review/SKILL.md +++ b/.agents/skills/session-review/SKILL.md @@ -36,7 +36,7 @@ For each session determine: | Signature | Root cause | Fix | |-----------|-----------|-----| -| Agent: "I can't run X — only supports Y" | Missing action in `request_execution` | Add action in `internal/mcp/server.go` | +| Agent: "I can't run X" | Missing target or capability | Use `run` with shell command — there is no fixed action enum anymore | | Agent: "No local knowledge on that" + no web tool | Missing `http_get` / web fetch MCP tool | Add MCP tool | | Empty assistant bubble (text="", no tools) | Model returned blank completion | Retry + error surfacing | | Non-English boilerplate refusal | Flash-tier model degradation | Response quality guard | @@ -75,7 +75,7 @@ Session: {id[:8]} — "{title[:60]}" - `cmd/nomos/agent.go` — agent loop, tool building, response guards - `cmd/nomos/store.go` — session + message persistence -- `internal/mcp/server.go` — all tool implementations including `request_execution` +- `internal/mcp/server.go` — all tool implementations (`run`, `list_lxcs`, …) - `web/src/lib/components/ToolCallGroup.svelte` — tool result display - `nomos/SOUL.md` — agent persona and tool selection rules - `plans/2026-07-09-chat-sessions-improvements.md` — prior session findings diff --git a/.gitignore b/.gitignore index 400e85a..474edad 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,4 @@ web/node_modules/ cmd/desktop/frontend/dist/ cmd/desktop/build/ cmd/desktop/Oikos +desktop diff --git a/AGENTS.md b/AGENTS.md index 7bf7dae..8c7fb3c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,9 +110,9 @@ Available tools (33 total): read-only inspection runs immediately, anything state-changing needs operator approval, and destructive patterns (rm -rf, dd, mkfs, pct/qm destroy, DROP TABLE, reboot, curl-pipe-to-shell, ...) always - need approval regardless of what you declare. Prefer this over - request_execution for anything not already covered by its fixed enum. - request_execution(target, action, params) — the older, fixed-enum path + need approval regardless of what you declare. This is the ONLY + mutation tool — `request_execution` was retired 2026-07-14. + `run` — the general execution primitive. Run any shell (restart, systemctl, pct_exec, apt_upgrade, pct_create). Still the route for those specific actions; policy-gated the same way `run` is. get_execution_status(execution_id) — poll progress @@ -165,8 +165,7 @@ per the DB-as-source-of-truth plan. operator interface — it has 33 MCP tools for observe/orient/decide/act (§3). - **Actions** (restart, logs, apt, pct exec, or anything else): Nomos calls - `run` (the general execution primitive) or `request_execution` (the older - fixed-enum path) via MCP. `reversible_low`/read-only actions execute + `run` (the general execution primitive) via MCP. `reversible_low`/read-only actions execute immediately; `config_mutation` and `destructive` actions are queued for operator approval via Matrix or the control-room UI's Operations page. - **Secrets**: managed by Infisical (`oikos secret` subcommand for diff --git a/CLIENTS.md b/CLIENTS.md index baf5a3a..e37398e 100644 --- a/CLIENTS.md +++ b/CLIENTS.md @@ -28,7 +28,7 @@ Docker stack on mac-mini and exposes an MCP server + REST API. | Record a discovered fact/relationship | MCP `update_entity_attributes`, `create_relationship`, `upsert_knowledge` | Most MCP tools are read-only; a few mutate the knowledge graph (recording -what you learned) or the live infrastructure (`run`, `request_execution`), +what you learned) or the live infrastructure (`run`), gated by risk classification and — for `config_mutation`/`destructive` actions — operator approval. See [AGENTS.md](AGENTS.md#3-the-mcp-server) for the full tool catalog. diff --git a/cmd/nomos/agent.go b/cmd/nomos/agent.go index 3e54ad9..cc2a3a9 100644 --- a/cmd/nomos/agent.go +++ b/cmd/nomos/agent.go @@ -17,12 +17,12 @@ import ( ) // maxIterations bounds one chat turn's tool-calling loop. Provisioning a -// service is a long chain (research → plan → request_execution → per-step +// service is a long chain (research → plan → run → per-step // install/verify run calls), so this must be generous; a full deploy with the // decomposed pct_create flow can legitimately need many steps. On exhaustion // the loop now produces a real summary (finalSummary) rather than a dead end. const maxIterations = 40 -const maxLLMRetries = 1 +const maxLLMRetries = 2 // historyWindowSize bounds how many of a session's most recent persisted // messages are replayed into the LLM's context on each turn — see @@ -129,7 +129,7 @@ func loadSoul() string { } return `You are Nomos, the steward of the oikos — the AI agent for the hubris homelab. You have access to MCP tools to query topology, health, knowledge, and request -gated mutations through request_execution. Be concise. Prefer tools over guessing.` +gated mutations through run. Be concise. Prefer tools over guessing.` } // assentWindowDuration is how long after an operator approves a plan that @@ -294,7 +294,7 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s } if len(granted) > 0 { a.openAssentWindow(ctx, sessionID) - note := fmt.Sprintf("[System: the operator's message approved pending execution(s) %s via chat assent — they are now running. An assent window is now active for 30 minutes: config_mutation commands will auto-run without re-approval. Do not re-request or call request_execution/run again for these; check get_execution_status if you need the outcome. CONTINUE executing the full plan — do not stop and wait for 'continue' after each step. Only surface to the operator for destructive actions (need typed confirmation) or if you're genuinely stuck after trying alternatives.]", strings.Join(granted, ", ")) + note := fmt.Sprintf("[System: the operator's message approved pending execution(s) %s via chat assent — they are now running. An assent window is now active for 30 minutes: config_mutation commands will auto-run without re-approval. Do not re-request or call run again for these; check get_execution_status if you need the outcome. CONTINUE executing the full plan — do not stop and wait for 'continue' after each step. Only surface to the operator for destructive actions (need typed confirmation) or if you're genuinely stuck after trying alternatives.]", strings.Join(granted, ", ")) messages = append(messages, openai.SystemMessage(note)) } if len(blocked) > 0 { @@ -305,9 +305,9 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s // The operator said "proceed"/"go ahead"/"yes" but the preceding // assistant turn had NO pending approvals — meaning the agent // proposed a plan in text and asked "shall I?" without calling - // request_execution yet. Inject a system note telling the agent + // run yet. Inject a system note telling the agent // the operator approved — go execute the plan now. - note := "[System: The operator approved your proposed plan. Execute it now — call request_execution or run to carry out the steps you described. Do not re-describe the plan or ask for confirmation again. The assent window is active: config_mutation commands will auto-run once you create them.]" + note := "[System: The operator approved your proposed plan. Execute it now — call run to carry out the steps you described. Do not re-describe the plan or ask for confirmation again. The assent window is active: config_mutation commands will auto-run once you create them.]" messages = append(messages, openai.SystemMessage(note)) a.openAssentWindow(ctx, sessionID) } diff --git a/cmd/nomos/continue.go b/cmd/nomos/continue.go index 38082e7..6a0f73b 100644 --- a/cmd/nomos/continue.go +++ b/cmd/nomos/continue.go @@ -52,7 +52,7 @@ func (a *agent) runIdleSweepWorker(ctx context.Context) { return } slog.Info("nomos: idle sweep worker started") - ticker := time.NewTicker(5 * time.Minute) + ticker := time.NewTicker(2 * time.Minute) defer ticker.Stop() for { select { @@ -143,11 +143,24 @@ func (a *agent) processContinuations(ctx context.Context) { // multiple tasks in flight, one task's open window must never cover a // pending continuation belonging to a different task. if !a.store.assentWindowActive(ctx, a.agentID, p.SessionID) { - // A finished one-off execution with no window is left as-is - // (marked continued so we don't re-check it forever) — the - // operator decides what happens next, as today. - a.store.markContinued(ctx, p.ExecID) - continue + // Re-open the assent window if this session is genuinely + // executing (plan was approved, work is in progress) — the + // window may have expired while the execution ran. Don't + // penalize timing: the plan was approved, the work happened, + // the result should flow back. + sesh, seshErr := a.store.getSession(ctx, p.SessionID) + if seshErr == nil && sesh.Goal != "" && (sesh.Status == "executing" || sesh.Status == "planning") { + a.openAssentWindow(ctx, p.SessionID) + slog.Info("nomos: re-opened assent window for continuing session", "session", p.SessionID, "execution", p.ExecID) + } else { + // Genuinely no plan — inject a visible note so the + // operator knows WHY the agent didn't auto-continue. + note := fmt.Sprintf("[System: execution %s finished with status=%s, but the assent window for this session is not active. The agent will not auto-continue. Reply 'continue' or re-approve the plan to resume.]", p.ExecID, p.Status) + body, _ := json.Marshal(map[string]any{"role": "assistant", "text": note, "auto": true}) + a.store.saveMessage(context.Background(), p.SessionID, "assistant", body) + a.store.markContinued(ctx, p.ExecID) + continue + } } a.store.markContinued(ctx, p.ExecID) // stamp first: a failure here must not cause a re-continue loop safego.Go("nomos:continue-session:"+p.SessionID, func() { a.continueSession(ctx, p) }) @@ -213,7 +226,7 @@ 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() - for attempt := 0; attempt < 2; attempt++ { + for attempt := 0; attempt < 3; attempt++ { toolCalls, finalText, errText = nil, "", "" emit := func(ev agentEvent) { if ev.Type == "tool_use" || ev.Type == "tool_result" { @@ -241,21 +254,24 @@ func (a *agent) resumeSession(ctx context.Context, sessionID, note string) { if errText != "" && finalText == "" { slog.Error("nomos: resume produced no response after retry", "session", sessionID, "error", errText) - // Give the task a real, operator-visible terminal state instead of - // leaving it silently stuck at whatever status it was in (typically - // 'executing' or 'awaiting_input') forever. Before this, a - // permanently-failed resume was invisible beyond a log line — the - // task board just showed a task that never changed, with nothing - // telling the operator it needed attention. Marking it failed here - // doesn't prevent the operator from continuing to work the task via - // a fresh chat message afterward; it just stops the silent hang. - summary := fmt.Sprintf("Auto-resume failed after retrying: %s", errText) - if len(summary) > 200 { - summary = summary[:200] + "…" - } - if cerr := a.store.completeTask(context.Background(), sessionID, "failure", summary); cerr != nil { - slog.Error("nomos: failed to mark task failed after resume gave up", "session", sessionID, "error", cerr) + // Persist a visible system note in the transcript so the + // operator sees what happened, but do NOT auto-complete the + // task — leave it in 'executing' so a follow-up chat message + // can resume it. Before this fix, the task was marked 'failed' + // here, which ended it permanently and required starting over. + resumeFailedNote := fmt.Sprintf("[System: auto-resume failed after retrying: %s. The task is paused — send another message to continue.]", errText) + body, _ := json.Marshal(map[string]any{ + "role": "assistant", + "text": resumeFailedNote, + "auto": true, + }) + if msgID != uuid.Nil { + a.store.updateMessage(context.Background(), msgID, body) + } else { + // No placeholder was inserted (rare), save directly. + a.store.saveMessage(context.Background(), sessionID, "assistant", body) } + return // do not call persist() again — already persisted above } persist() // final state — same row, updated one last time with the concluding text } diff --git a/cmd/nomos/main.go b/cmd/nomos/main.go index 61f7081..89fdf48 100644 --- a/cmd/nomos/main.go +++ b/cmd/nomos/main.go @@ -164,11 +164,29 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { http.Error(w, "bad request: "+err.Error(), 400) return } - if req.Message == "" { + if req.Message == "" && req.SessionID == "" { http.Error(w, "message is required", 400) return } + // Empty message with an existing session = reconnect/resume. The + // frontend sends this after a dropped SSE stream to re-establish the + // connection and catch up on any auto-continuation work that happened + // while disconnected. Route into resumeSession so the agent sees a + // system note and reports current state. + if req.Message == "" && req.SessionID != "" { + slog.Info("nomos: reconnect", "session", req.SessionID) + safego.Go("nomos:reconnect:"+req.SessionID, func() { + note := "[System: the operator's connection was re-established. The task may have progressed in the background — report your current state and progress.]" + a.resumeSession(context.Background(), req.SessionID, note) + }) + // Return 202 so the frontend doesn't try to consume an SSE stream + // from this POST — resumeSession writes to the DB directly and + // the poller (already running from handleDisconnect) picks it up. + w.WriteHeader(202) + return + } + flusher, ok := w.(http.Flusher) if !ok { http.Error(w, "streaming not supported", 500) @@ -318,6 +336,14 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a return } + // POST /sessions/{id}/resume — the operator asks the agent to continue. + if len(parts) == 2 && parts[1] == "resume" && r.Method == http.MethodPost { + note := "[System: the operator wants you to continue. Pick up where you left off — execute the next step of the plan, diagnose and fix any failures, or report progress if everything is done.]" + safego.Go("nomos:resume-session", func() { a.resumeSession(context.Background(), id, note) }) + w.WriteHeader(202) + return + } + // GET /sessions/{id}/plan and /sessions/{id}/questions — REST hydration for // the context panel when it first opens a task; live events carry deltas // from there. diff --git a/cmd/nomos/store.go b/cmd/nomos/store.go index 2d61265..f0fa538 100644 --- a/cmd/nomos/store.go +++ b/cmd/nomos/store.go @@ -46,16 +46,17 @@ func (s *store) close() { // lifecycle status and an outcome (see migration 018 / the task-board plan). // Outcome/Summary/EntityID are empty until set, hence omitempty. type session struct { - ID string `json:"id"` - Title string `json:"title"` - Actor string `json:"actor"` - Goal string `json:"goal"` - Status string `json:"status"` - Outcome string `json:"outcome,omitempty"` - Summary string `json:"summary,omitempty"` - EntityID string `json:"entity_id,omitempty"` - CreatedAt time.Time `json:"created_at"` - LastActiveAt time.Time `json:"last_active_at"` + ID string `json:"id"` + Title string `json:"title"` + Actor string `json:"actor"` + Goal string `json:"goal"` + Status string `json:"status"` + Outcome string `json:"outcome,omitempty"` + Summary string `json:"summary,omitempty"` + EntityID string `json:"entity_id,omitempty"` + PendingApprovals int `json:"pending_approvals"` + CreatedAt time.Time `json:"created_at"` + LastActiveAt time.Time `json:"last_active_at"` } type message struct { @@ -195,9 +196,19 @@ func (s *store) listSessions(ctx context.Context) ([]session, error) { return nil, nil } rows, err := s.pool.Query(ctx, - `SELECT id, title, actor, goal, status, COALESCE(outcome, ''), summary, - COALESCE(entity_id::text, ''), created_at, last_active_at - FROM agent_sessions ORDER BY last_active_at DESC LIMIT 50`) + `SELECT s.id, s.title, s.actor, s.goal, s.status, COALESCE(s.outcome, ''), s.summary, + COALESCE(s.entity_id::text, ''), + COALESCE(pa.cnt, 0), + s.created_at, s.last_active_at + FROM agent_sessions s + LEFT JOIN ( + SELECT l.session_id, COUNT(*) AS cnt + FROM nomos_plan_executions l + JOIN executions e ON e.entity_id = l.execution_id + WHERE e.status = 'pending_approval' + GROUP BY l.session_id + ) pa ON pa.session_id = s.id + ORDER BY s.last_active_at DESC LIMIT 50`) if err != nil { return nil, err } @@ -207,7 +218,8 @@ func (s *store) listSessions(ctx context.Context) ([]session, error) { for rows.Next() { var sess session if err := rows.Scan(&sess.ID, &sess.Title, &sess.Actor, &sess.Goal, &sess.Status, - &sess.Outcome, &sess.Summary, &sess.EntityID, &sess.CreatedAt, &sess.LastActiveAt); err != nil { + &sess.Outcome, &sess.Summary, &sess.EntityID, &sess.PendingApprovals, + &sess.CreatedAt, &sess.LastActiveAt); err != nil { return nil, err } out = append(out, sess) @@ -215,6 +227,24 @@ func (s *store) listSessions(ctx context.Context) ([]session, error) { return out, rows.Err() } +func (s *store) getSession(ctx context.Context, id string) (*session, error) { + if s == nil { + return nil, nil + } + var sess session + err := s.pool.QueryRow(ctx, + `SELECT id, title, actor, goal, status, COALESCE(outcome, ''), summary, + COALESCE(entity_id::text, ''), 0, created_at, last_active_at + FROM agent_sessions WHERE id = $1`, id). + Scan(&sess.ID, &sess.Title, &sess.Actor, &sess.Goal, &sess.Status, + &sess.Outcome, &sess.Summary, &sess.EntityID, &sess.PendingApprovals, + &sess.CreatedAt, &sess.LastActiveAt) + if err != nil { + return nil, err + } + return &sess, nil +} + // getMessages returns a session's ENTIRE message history, unbounded — used // for the UI's own transcript view (GET /sessions/{id}), where the operator // should be able to see everything a task has done regardless of how long @@ -387,6 +417,17 @@ func (s *store) proposePlan(ctx context.Context, sessionID string, steps []planS return nil, err } startSeq = 0 + } else { + // Mid-flight plan revision: mark any still-pending steps from the + // previous generation as 'replaced' so the panel doesn't show them + // as incomplete forever. Only pending steps — already-running or + // done steps from the prior plan are preserved as history. + if _, err := tx.Exec(ctx, ` + UPDATE session_plan_steps + SET status = 'replaced', finished_at = now() + WHERE session_id = $1 AND status = 'pending'`, sessionID); err != nil { + return nil, err + } } out := make([]map[string]any, 0, len(steps)) @@ -428,6 +469,11 @@ func (s *store) proposePlan(ctx context.Context, sessionID string, steps []planS // plan.step.finished (terminal) so the panel advances live. The execution link // is also what lets the api auto-close the step when the execution finishes // (see closePlanStepForExecution). +// +// Completion ordering (done/failed/skipped/blocked) is enforced: a step cannot +// be marked complete while an earlier step is still pending, preventing the +// agent from marking step 5 done before step 4 (observed in production: the +// agent rushed to close all steps in a final turn, in reverse order). func (s *store) updatePlanStep(ctx context.Context, sessionID string, seq int, status, execID string) error { if s == nil || sessionID == "" || sessionID == "ephemeral" { return nil @@ -436,9 +482,22 @@ func (s *store) updatePlanStep(ctx context.Context, sessionID string, seq int, s switch status { case "running": stamp = ", started_at = COALESCE(started_at, now())" - case "done", "failed", "skipped", "blocked": + case "done", "failed", "skipped", "blocked", "replaced": stamp = ", finished_at = now()" } + // Completion ordering: for terminal states, check that no earlier step + // is still pending. Running steps can start out of order (the agent + // may dispatch parallel work), but completion must be sequential. + if status == "done" || status == "failed" || status == "skipped" || status == "blocked" { + var blockedBy int + if err := s.pool.QueryRow(ctx, ` + SELECT COALESCE(MIN(seq), 0) + FROM session_plan_steps + WHERE session_id = $1 AND seq < $2 AND status = 'pending'`, + sessionID, seq).Scan(&blockedBy); err == nil && blockedBy > 0 { + return fmt.Errorf("cannot complete step %d — step %d is still pending", seq, blockedBy) + } + } var execPtr *uuid.UUID if id, err := uuid.Parse(execID); err == nil { execPtr = &id @@ -481,6 +540,33 @@ func (s *store) completeTask(ctx context.Context, sessionID, outcome, summary st if s == nil || sessionID == "" || sessionID == "ephemeral" { return nil } + + // 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). + var cancelledCount int + if err := s.pool.QueryRow(ctx, ` + WITH cancelled AS ( + UPDATE executions SET status = 'cancelled', + result = '{"message": "task completed — auto-cancelled"}'::jsonb + WHERE entity_id IN ( + SELECT execution_id FROM nomos_plan_executions WHERE session_id = $1 + ) AND status IN ('pending_approval', 'approved', 'queued') + RETURNING entity_id + ) + SELECT COUNT(*) FROM cancelled + `, sessionID).Scan(&cancelledCount); err != nil { + slog.Warn("nomos: completeTask failed to cancel orphaned executions", "session", sessionID, "error", err) + } + + // Mark all continuations done so the worker won't try to feed them back. + s.pool.Exec(ctx, `UPDATE nomos_plan_executions SET continued_at = now() + WHERE session_id = $1 AND continued_at IS NULL`, sessionID) + + // Clean up assent and destructive window keys from autonomy_settings. + s.pool.Exec(ctx, `DELETE FROM autonomy_settings + WHERE key LIKE '%:' || $1`, sessionID) + status := "done" if outcome == "failure" { status = "failed" @@ -504,10 +590,27 @@ func (s *store) completeTask(ctx context.Context, sessionID, outcome, summary st severity = "warning" } _ = observability.Event(ctx, sqlcgen.New(s.pool), "task.status", entPtr, severity, "nomos", sessionID, - map[string]any{"status": status, "outcome": outcome, "summary": summary}) + map[string]any{"status": status, "outcome": outcome, "summary": summary, + "cancelled_executions": cancelledCount}) return nil } +// hadEntityWriteback checks whether this session called update_entity_attributes +// or create_relationship — used by complete_task to warn the agent when it +// forgot to persist entity facts (the #1 cause of knowledge graph drift). +func (s *store) hadEntityWriteback(ctx context.Context, sessionID string) bool { + if s == nil || sessionID == "" { + return true // fail safe: don't warn when we can't check + } + var count int + s.pool.QueryRow(ctx, ` + SELECT COUNT(*) FROM agent_activity + WHERE session_id = $1 + AND tool_name IN ('update_entity_attributes', 'create_relationship') + AND success = true`, sessionID).Scan(&count) + return count > 0 +} + // staleGoalSession is a goal-bearing task that's gone idle without reaching // a terminal state — the idle-sweep worker's work list (fix 2+3 of // plans/2026-07-11-task-completion-safety-net.md). diff --git a/cmd/nomos/tasks.go b/cmd/nomos/tasks.go index db73082..74f6ef6 100644 --- a/cmd/nomos/tasks.go +++ b/cmd/nomos/tasks.go @@ -202,7 +202,16 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args if err != nil { return fmt.Sprintf("error proposing plan: %v", err), true } - return fmt.Sprintf("Plan set: %d step(s). Execute them now, marking each with update_plan_step as you go.", len(persisted)), true + // Nudge: if the last step doesn't mention entity writeback tools, + // the graph will keep drifting — discovered facts won't be persisted. + lastStep := steps[len(steps)-1] + hasWriteback := strings.Contains(lastStep.Title+lastStep.Detail, "update_entity_attributes") || + strings.Contains(lastStep.Title+lastStep.Detail, "create_relationship") + result := fmt.Sprintf("Plan set: %d step(s). Execute them now, marking each with update_plan_step as you go.", len(persisted)) + if !hasWriteback { + result += "\n\n⚠️ The final step doesn't mention update_entity_attributes or create_relationship. Without those, any facts you discovered about entities (IPs, versions, hosts, states) will be LOST — the next session starts from scratch. Consider revising the last step to include entity writeback BEFORE completing the task." + } + return result, true case "update_plan_step": seq := toInt(args["seq"]) @@ -261,7 +270,11 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args if err := a.store.completeTask(ctx, sessionID, outcome, summary); err != nil { return fmt.Sprintf("error completing task: %v", err), true } - return fmt.Sprintf("Task marked %s: %s", outcome, summary), true + result := fmt.Sprintf("Task marked %s: %s", outcome, summary) + if !a.store.hadEntityWriteback(ctx, sessionID) { + result += "\n\n⚠️ No entity attributes or relationships were updated in this session. Call update_entity_attributes and create_relationship to persist what you learned about entities before the next session starts from scratch." + } + return result, true default: return nil, false } diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 4812611..f643370 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -24,7 +24,6 @@ import ( "github.com/dtoro/oikos/internal/db/sqlcgen" "github.com/dtoro/oikos/internal/observability" "github.com/dtoro/oikos/internal/policy" - "github.com/dtoro/oikos/internal/safego" "github.com/google/jsonschema-go/jsonschema" "github.com/google/uuid" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -212,7 +211,7 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server { InputSchema: objSchema( prop{"title", "string", "Short, specific, searchable title (e.g. 'Dragonfly memlock rlimit in unprivileged LXCs', not 'notes')."}, prop{"content", "string", "The knowledge itself, in markdown. Be concrete: symptom, root cause, the exact fix/commands, and any caveats. Written for someone hitting this fresh."}, - prop{"about", "string", "Optional entity slug this knowledge concerns (e.g. lxc:typetype, host:strong) — links the note to that entity so get_entity_knowledge surfaces it."}, + prop{"about", "string", "Optional entity slug(s) this knowledge concerns. Pass a single slug (e.g. 'lxc:nfs-export') or a JSON array of slugs (e.g. '[\"lxc:nfs-export\", \"lxc:gitea\"]') to link to multiple entities. get_entity_knowledge surfaces it for each."}, prop{"tags", "string", "Optional comma-separated tags (e.g. 'docker,networking,gotcha')."}, prop{"kind", "string", "One of: investigation (a finding/incident analysis — default), document (reference), runbook (a repeatable procedure)."}, ), @@ -358,186 +357,11 @@ func newServer(pool *db.Pool, agentID uuid.UUID) *mcp.Server { nStr(args["status"])), nil }) - register(&mcp.Tool{Name: "request_execution", Description: "Request a gated execution (agent-only mutation path). Supported actions: restart, systemctl, pct_exec, apt_upgrade, pct_create. pct_create provisions a NEW LXC and installs its service in one approved step — you do NOT need follow-up pct_exec calls for package installs.", - InputSchema: objSchema( - prop{"target", "string", "Target entity slug. For pct_create this MUST be the Proxmox HOST that will run the container (e.g. host:strong) — NOT the new LXC's name. For restart/systemctl/apt_upgrade/pct_exec use the target service/LXC slug (e.g. lxc:caddy)."}, - prop{"action", "string", "Action: restart, systemctl, pct_exec, apt_upgrade, pct_create"}, - prop{"params", "string", "For systemctl: 'enable|disable|reload'. For pct_exec: the shell command. For apt_upgrade: 'audit|upgrade'. For pct_create: a JSON object string with keys: vmid (int, required, unused id), hostname (string, required), cores (int), memory (MB int), disk_gb (int), ip (CIDR e.g. 192.168.8.50/24, or omit/\"dhcp\" — DHCP is the safe default, see below), gw (gateway ip, static only), bridge (e.g. vmbr0/vmbr1 — WHICH BRIDGE REACHES WHICH SUBNET IS DIFFERENT PER HOST, never assume vmbr0; see below), storage (default local-lvm), template (optional — omit to auto-pick newest debian on the host), privileged (bool), nesting (bool), mounts ([]string of 'src,mp=/dst'). Example: {\"vmid\":150,\"hostname\":\"typetype\",\"cores\":2,\"memory\":2048,\"disk_gb\":16,\"ip\":\"192.168.8.50/24\",\"gw\":\"192.168.8.2\",\"bridge\":\"vmbr1\",\"nesting\":true}. pct_create is ATOMIC — it ONLY creates and starts the container (no services/post_install params anymore). Once it completes you will be automatically re-invoked with the result; install packages and run setup by issuing your OWN `run` calls against the new lxc: target, one step at a time — you'll see each step's real output and can fix exactly the one that fails, instead of one opaque multi-minute install that either fully works or fully doesn't. STATIC IP RULE: before setting ip/gw/bridge to anything other than DHCP, use list_entities/get_entity_knowledge to find an EXISTING lxc on the SAME host whose IP is in the same /28 block, and copy its exact gw+bridge — do not invent a gateway. If no such neighbor exists, prefer ip:\"dhcp\" (proven to work, gets a real routable address) over guessing; a wrong bridge/gateway pair fails a fast pre-flight ping check now (seconds, not minutes) but is still a wasted turn — better to not guess at all."}, - ), - }, func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) { - args := argsMap(req) - targetSlug, _ := args["target"].(string) - action, _ := args["action"].(string) - params, _ := args["params"].(string) - sessionID, _ := args["_session_id"].(string) - if targetSlug == "" || action == "" { - return textResult("error: target and action required"), nil - } - - var targetID uuid.UUID - if err := pool.QueryRow(ctx, "SELECT id FROM entities WHERE slug = $1", targetSlug).Scan(&targetID); err != nil { - return textResult(fmt.Sprintf("target not found: %s", targetSlug)), nil - } - - // restart, pct_exec, and systemctl (outside enable/disable) route - // through the same classify→gate path as `run` instead of executing - // immediately over SSH with a hardcoded risk_class='reversible_low' - // that was never actually checked against anything. Found live - // 2026-07-10: a chat request to "restart caddy" — the fleet's - // reverse proxy — executed instantly with zero approval, because - // this action bypassed the classifier entirely. classifyAndGate - // applies the same read-only/config-mutation/destructive - // classification and approval flow the `run` tool already uses. - if action == "restart" || action == "pct_exec" || (action == "systemctl" && params != "enable" && params != "disable") { - svc := strings.TrimPrefix(targetSlug, "lxc:") - var cmd, purpose string - switch action { - case "restart": - cmd = fmt.Sprintf("systemctl restart %s; sleep 1; systemctl is-active %s", svc, svc) - purpose = "restart " + svc - case "pct_exec": - cmd = params - purpose = "pct_exec (legacy) on " + targetSlug - case "systemctl": - cmd = fmt.Sprintf("systemctl %s %s; sleep 1; systemctl is-active %s", params, svc, svc) - purpose = "systemctl " + params + " " + svc - } - return classifyAndGate(ctx, pool, agentID, targetID, targetSlug, cmd, purpose, "", sessionID), 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. Only blocks when a pending - // execution exists; completed/failed ones don't block. - if action == "systemctl" || action == "apt_upgrade" || action == "pct_create" { - execNamePrefix := action + " on " + targetSlug - var existingID string - err := pool.QueryRow(ctx, ` - SELECT e.id::text FROM entities e - JOIN executions ex ON ex.entity_id = e.id - WHERE e.type = 'execution' AND e.name LIKE $1 AND ex.status = 'pending_approval' - ORDER BY e.created_at DESC LIMIT 1`, execNamePrefix+"%").Scan(&existingID) - if err == nil && existingID != "" { - 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() - - // Full UUID, not a truncated prefix: UUIDv7's leading bytes encode a - // millisecond timestamp, so an 8-char prefix collides for real under - // back-to-back requests (observed live: two `run` calls seconds - // apart hit entities_slug_key). The full string is guaranteed unique. - execName := action + " on " + targetSlug + " (" + id.String() + ")" - execSlug := "exec:" + targetSlug + ":" + id.String() - _, err := pool.Exec(ctx, `INSERT INTO entities (id, slug, type, name, attributes) VALUES ($1, $2, 'execution', $3, '{}')`, - id, execSlug, execName) - 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) - pool.Exec(ctx, ` - INSERT INTO relationships (source_id, target_id, type, attributes, valid_from) - SELECT $1, $2, 'targets', '{"by":"nomos"}'::jsonb, now() - WHERE NOT EXISTS ( - SELECT 1 FROM relationships - WHERE source_id = $1 AND target_id = $2 AND type = 'targets' AND valid_to IS NULL)`, - id, targetID) - if sessionID != "" { - pool.Exec(ctx, ` - INSERT INTO relationships (source_id, target_id, type, attributes, valid_from) - SELECT t.id, $1, 'involves', '{"by":"nomos"}'::jsonb, now() - FROM entities t WHERE t.slug = $2 - AND NOT EXISTS ( - SELECT 1 FROM relationships - WHERE source_id = t.id AND target_id = $1 AND type = 'involves' AND valid_to IS NULL)`, - id, "task:"+sessionID) - } - - // Execute reversible actions immediately. restart/pct_exec/systemctl - // (outside enable/disable) never reach here — they're routed through - // classifyAndGate above, before this dedup+insert block. - switch action { - case "systemctl": - // Only enable/disable reach this case now. - svc := strings.TrimPrefix(targetSlug, "lxc:") - pool.Exec(ctx, `UPDATE executions SET status='pending_approval', risk_class='config_mutation' WHERE entity_id=$1`, id) - createApproval(ctx, pool, id, targetID, "systemctl", svc+":"+params, "config_mutation") - return textResult(fmt.Sprintf("systemctl %s on %s requires approval — execution %s queued", params, svc, id)), nil - - case "apt_upgrade": - if params == "audit" { - host, user, err := resolveHost(ctx, pool, targetSlug) - if err != nil { - return textResult(fmt.Sprintf("resolve: %v", err)), nil - } - out, err := sshExec(ctx, host, user, "apt update -qq 2>&1 >/dev/null; apt list --upgradable 2>/dev/null | tail -n +2 | wc -l; apt list --upgradable 2>/dev/null | tail -n +2 | head -20") - if err != nil { - return textResult(fmt.Sprintf("apt audit error: %v", err)), nil - } - return textResult("apt audit:\n" + out), nil - } - // During an active assent window, auto-approve. - if assentWindowActive(ctx, pool, agentID, sessionID) { - pool.Exec(ctx, `UPDATE executions SET status='pending_approval', risk_class='config_mutation' WHERE entity_id=$1`, id) - createApproval(ctx, pool, id, targetID, "apt_upgrade", params, "config_mutation") - // Do NOT pre-flip approvals/executions status here (that was - // the previous, broken "autoApprove" helper). DecideApproval - // (invoked below) is the ONE place that transitions - // pending_approval -> approved and dispatches the real SSH - // work — it specifically looks for status='pending_approval' - // to find what to run. Pre-flipping the status past that - // state meant DecideApproval's own lookup found nothing, - // silently no-opped, and the execution sat at 'approved' - // forever with nothing actually running. Found live: every - // assent-window auto-approved pct_create/apt_upgrade has - // never actually executed, via this exact bug. Calling - // executeApprovedViaAPI directly against the untouched - // pending_approval row makes this identical to the manual - // Approve-button path, just without a human click. - // - // context.Background(), NOT ctx: ctx is scoped to this MCP - // tool call, cancelled the instant the chat turn's HTTP - // response completes (every normal turn) — a goroutine - // meant to outlive the request must not inherit its context. - safego.Go("mcp:executeApprovedViaAPI:apt_upgrade", func() { - executeApprovedViaAPI(context.Background(), id, targetSlug, "apt_upgrade:"+params) - }) - slog.Info("mcp: apt_upgrade auto-approved via assent window", "execution_id", id) - return textResult(fmt.Sprintf("apt_upgrade on %s auto-approved via assent window — execution %s running.", targetSlug, id)), nil - } - // upgrade requires approval — queue - pool.Exec(ctx, `UPDATE executions SET status='pending_approval', risk_class='config_mutation' WHERE entity_id=$1`, id) - createApproval(ctx, pool, id, targetID, "apt_upgrade", params, "config_mutation") - return textResult(fmt.Sprintf("apt_upgrade on %s requires approval — execution %s queued", targetSlug, id)), nil - - case "pct_create": - // During an active assent window, auto-approve and execute - // instead of queuing — the operator already approved the plan. - if assentWindowActive(ctx, pool, agentID, sessionID) { - pool.Exec(ctx, `UPDATE executions SET status='pending_approval', risk_class='config_mutation' WHERE entity_id=$1`, id) - createApproval(ctx, pool, id, targetID, "pct_create", params, "config_mutation") - // See the apt_upgrade case above for why there's no - // pre-flip-status "autoApprove" step here anymore, and why - // this uses context.Background(). - safego.Go("mcp:executeApprovedViaAPI:pct_create", func() { - executeApprovedViaAPI(context.Background(), id, targetSlug, "pct_create:"+params) - }) - slog.Info("mcp: pct_create auto-approved via assent window", "execution_id", id) - return textResult(fmt.Sprintf("pct_create on %s auto-approved via assent window — execution %s running. The LXC is being provisioned now.", targetSlug, id)), nil - } - pool.Exec(ctx, `UPDATE executions SET status='pending_approval', risk_class='config_mutation' WHERE entity_id=$1`, id) - createApproval(ctx, pool, id, targetID, "pct_create", params, "config_mutation") - return textResult(fmt.Sprintf("pct_create on %s requires approval — execution %s queued. The LXC will be provisioned once approved.", targetSlug, id)), nil - - default: - return textResult(fmt.Sprintf("unknown action: %s. Supported: restart, systemctl, pct_exec, apt_upgrade, pct_create", action)), nil - } - }) + // ── request_execution (legacy fixed enum) retired 2026-07-14 ── + // All mutations now route through `run`. The handler functions + // (runRexecRestart, runRexecSystemctl, etc.) are kept as reference + // for future runbook extraction — especially pct_create DNS/VMID logic. + // DO NOT re-register this tool. See plans/2026-07-10-general-gated-execution.md. register(&mcp.Tool{Name: "run", Description: "Run ANY shell command against any host or LXC. This is the general execution primitive — prefer it over asking the operator to run something manually, and don't wait for a matching fixed action to exist. Every command is automatically risk-classified: read-only inspection (cat, systemctl status, docker ps, journalctl, df, git status, ...) runs immediately; anything that changes state requires operator approval (granted by the operator replying \"go ahead\"/\"yes\" in chat, or via the Approve button); commands matching a destructive pattern (rm -rf, dd, mkfs, pct/qm destroy, DROP TABLE, reboot, piping curl into a shell, ...) always require approval regardless of what you declare. You cannot talk your way past the destructive check by declaring a lower risk.", InputSchema: objSchema( @@ -661,17 +485,27 @@ 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", - InputSchema: objSchema(), + 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.", + InputSchema: objSchema( + prop{"state", "string", "Optional: filter by entity state (active, destroyed, …)"}, + ), }, func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) { + state, _ := argsMap(req)["state"].(string) + var statePtr *string + if state != "" { + statePtr = &state + } return annotateJSONResult(queryRows(ctx, pool, ` 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 FROM entities e LEFT JOIN entity_status st ON st.entity_id = e.id WHERE e.type = 'lxc' - ORDER BY (e.attributes->>'pve_id')::int`), "lxc_list"), nil + AND ($1::text IS NULL OR e.state = $1) + ORDER BY CASE WHEN e.state = 'active' THEN 0 ELSE 1 END, + (e.attributes->>'pve_id')::int`, statePtr), "lxc_list"), nil }) register(&mcp.Tool{Name: "ping_service", Description: "Check if a service is reachable via HTTP", @@ -1657,10 +1491,26 @@ func knowledgeSlug(kind, title string) string { func upsertKnowledge(ctx context.Context, pool *db.Pool, args map[string]any) (*mcp.CallToolResult, error) { title, _ := args["title"].(string) content, _ := args["content"].(string) - about, _ := args["about"].(string) tagsRaw, _ := args["tags"].(string) kind, _ := args["kind"].(string) + // Normalize about: accept a single string slug or an array of slugs. + var aboutSlugs []string + switch v := args["about"].(type) { + case string: + if s := strings.TrimSpace(v); s != "" { + aboutSlugs = []string{s} + } + case []interface{}: + for _, item := range v { + if s, ok := item.(string); ok { + if s = strings.TrimSpace(s); s != "" { + aboutSlugs = append(aboutSlugs, s) + } + } + } + } + title = strings.TrimSpace(title) content = strings.TrimSpace(content) if title == "" || content == "" { @@ -1707,21 +1557,27 @@ func upsertKnowledge(ctx context.Context, pool *db.Pool, args map[string]any) (* return textResult(fmt.Sprintf("error writing knowledge: %v", err)), nil } - // Link it to the entity it's about, if given and not already linked. + // Link it to the entity(s) it's about, if given and not already linked. linked := "" - if about = strings.TrimSpace(about); about != "" { - var targetID uuid.UUID - if qerr := pool.QueryRow(ctx, "SELECT id FROM entities WHERE slug = $1", about).Scan(&targetID); qerr == nil { - pool.Exec(ctx, ` - INSERT INTO relationships (source_id, target_id, type, attributes, valid_from) - SELECT $1, $2, 'documents', '{"by":"nomos"}'::jsonb, now() - WHERE NOT EXISTS ( - SELECT 1 FROM relationships - WHERE source_id = $1 AND target_id = $2 AND type = 'documents' AND valid_to IS NULL)`, - docID, targetID) - linked = " and linked to " + about - } else { - linked = fmt.Sprintf(" (note: entity %q not found, saved unlinked)", about) + if len(aboutSlugs) > 0 { + var linkedSlugs []string + for _, slug := range aboutSlugs { + var targetID uuid.UUID + if qerr := pool.QueryRow(ctx, "SELECT id FROM entities WHERE slug = $1", slug).Scan(&targetID); qerr == nil { + pool.Exec(ctx, ` + INSERT INTO relationships (source_id, target_id, type, attributes, valid_from) + SELECT $1, $2, 'documents', '{"by":"nomos"}'::jsonb, now() + WHERE NOT EXISTS ( + SELECT 1 FROM relationships + WHERE source_id = $1 AND target_id = $2 AND type = 'documents' AND valid_to IS NULL)`, + docID, targetID) + linkedSlugs = append(linkedSlugs, slug) + } + } + if len(linkedSlugs) == 1 { + linked = " and linked to " + linkedSlugs[0] + } else if len(linkedSlugs) > 1 { + linked = fmt.Sprintf(" and linked to %d entities", len(linkedSlugs)) } } diff --git a/migrations/020_session_reliability.up.sql b/migrations/020_session_reliability.up.sql new file mode 100644 index 0000000..2eb5318 --- /dev/null +++ b/migrations/020_session_reliability.up.sql @@ -0,0 +1,17 @@ +-- 020_session_reliability.up.sql +-- Plan step generation tracking + audit log session linkage. +-- See plans/2026-07-14-session-reliability-and-ux-audit.md. + +-- Plan step generation: when the agent revises a plan mid-flight, new steps +-- get a higher generation number so the frontend can group/collapse old ones. +ALTER TABLE session_plan_steps ADD COLUMN IF NOT EXISTS generation INTEGER NOT NULL DEFAULT 1; + +-- Track which session produced each audit-log entry so per-session analysis +-- (e.g. "did this task call update_entity_attributes?") is O(1) instead of +-- scanning the full log. +ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS session_id UUID; +CREATE INDEX IF NOT EXISTS idx_audit_log_session ON audit_log (session_id); + +-- Efficient lookup of pending-approval executions by session. +CREATE INDEX IF NOT EXISTS idx_nomos_plan_executions_session + ON nomos_plan_executions (session_id) WHERE continued_at IS NULL; diff --git a/nomos/SOUL.md b/nomos/SOUL.md index ae06d9a..e8afce8 100644 --- a/nomos/SOUL.md +++ b/nomos/SOUL.md @@ -48,6 +48,26 @@ 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 — @@ -125,10 +145,10 @@ exist just to fill the step. The loop scales down; it doesn't disappear. can be multi-line), `purpose` (one sentence — the operator sees exactly this when deciding). Auto-runs if read-only; otherwise queues for approval. See "Your capability is unlimited" above. -- `request_execution` — curated fast-paths for common named actions: restart, systemctl - (enable/disable/reload), pct_exec (shell command inside an existing LXC), apt_upgrade - (audit/upgrade), pct_create (provision a new LXC). Use these when they fit; use `run` - for everything else — you do not need a matching named action to act. +- `run` — the ONLY mutation tool. Accepts `target`, `command`, `purpose`, + `declared_risk`. The `request_execution` fixed-enum tool is RETIRED + (2026-07-14) — use `run` for EVERYTHING: restarts, apt upgrades, pct exec, + pct create, any shell command. There is no named-action tool anymore. - `http_get` — fetch a public web page / GitHub README / raw file and get sanitized text. You CAN read the internet with this. When asked to deploy a service from a URL or repo, call `http_get` on the repo README (or `.../raw/main/docker-compose.yml`) to learn its @@ -158,7 +178,7 @@ exist just to fill the step. The loop scales down; it doesn't disappear. ## Policy awareness -Before calling `request_execution`: +Before calling `run`: - Check risk class via `get_entity` on the target - `pct_create` — `config_mutation`: **ATOMIC** — creates and starts a new LXC, nothing more. Set `target` to the Proxmox HOST slug (e.g. `host:strong`), not the new container @@ -233,7 +253,7 @@ note — continue executing the full plan from there. Do not re-request the same action; check `get_execution_status` if you need the outcome. One approval per action is enough. -**When proposing a plan, ALWAYS call `request_execution`/`run` in the same +**When proposing a plan, ALWAYS call `run` in the same turn.** Do not propose a plan in text, ask "shall I proceed?", and wait. Call the tool — if it queues for approval, present what's queued and stop. The operator's "proceed"/"go ahead" will grant it and open the assent window. diff --git a/nomos/skills/homelab-ops/SKILL.md b/nomos/skills/homelab-ops/SKILL.md index 716ad31..f61c612 100644 --- a/nomos/skills/homelab-ops/SKILL.md +++ b/nomos/skills/homelab-ops/SKILL.md @@ -7,8 +7,9 @@ ## Overview Standard operating procedures for the Nomos agent managing the hubris -homelab. All mutations route through `request_execution` → Oikos policy -gating → actuator (SSH). +homelab. All mutations route through `run` → Oikos policy +gating → actuator (SSH). The `request_execution` fixed-enum tool was retired +2026-07-14. ## Procedures @@ -22,13 +23,13 @@ gating → actuator (SSH). ### Signal response -- `reversible_low` with validated pattern → `request_execution` (auto-restart) +- `reversible_low` with validated pattern → `run` (auto-restart) - `config_mutation` or `destructive` → escalate to operator - Repeated flapping → escalate with flap count ### Execution tracking -1. `request_execution` returns a correlation_id +1. `run` returns the execution ID in its result text 2. Poll `get_event_timeline` filtering by correlation_id 3. Once complete, `get_health_summary` to verify recovery 4. Record outcome via internal reasoning @@ -41,7 +42,9 @@ gating → actuator (SSH). ## Changelog -### 2026-07-08 — rename to Nomos +### 2026-07-14 — request_execution retired +All references to `request_execution` replaced with `run`. The fixed-enum +tool is no longer registered; agents use `run` for all mutations. Agent renamed from Hermes to Nomos (N0 milestone). ### 2026-07-07 — initial Phase 4 skill diff --git a/plans/2026-07-14-session-reliability-and-ux-audit.md b/plans/2026-07-14-session-reliability-and-ux-audit.md new file mode 100644 index 0000000..dae8f41 --- /dev/null +++ b/plans/2026-07-14-session-reliability-and-ux-audit.md @@ -0,0 +1,1174 @@ +# 2026-07-14 — Session reliability & UX audit: "empty responses, disconnects, and silent failures" + +**Status:** Planned — 2026-07-14. Audit from a Nomos production session that +exhibited multiple reliability gaps. Findings are grounded in code paths and +empirical session data from the live DB (`00a344e4` + `42424b92`). + +## Session under audit + +| Session | Goal | Messages | Outcome | Top tools | +|---|---|---|---|---| +| `00a344e4` (most recent) | Upgrade 4 low-risk LXCs | 22 (11 turns) | success | run (76), request_execution (86), update_plan_step (42), get_execution_status (24) | +| `42424b92` (prior failure) | Fleet health check | 3 messages | **failure** | — empty response | + +--- + +## Findings — Part A: UX reliability (the session-breaking class) + +### 1. Empty/unusable responses — "Nomos returned an empty or unusable response — please retry." + +**Where:** `cmd/nomos/agent.go:362-371` (`isRefusalOrEmpty`) + `cmd/nomos/continue.go:207-259` (`resumeSession` retry). + +**What happened:** The LLM (deepseek/deepseek-v4-pro over OpenRouter) returned +a blank or non-English response. `maxLLMRetries = 1` (agent.go:33) gives exactly +one retry inside `chatWith`. `resumeSession` adds a second outer retry +(continue.go:216). After both fail, the auto-continuation marks the task as +`failure` with `"Auto-resume failed after retrying: Nomos returned an empty +or unusable response — please retry."`. + +**Root cause:** No state-specific recovery. The user sees "please retry" but +(a) the task is marked terminal (`failure`), so retrying in the same session +won't work — the agent won't auto-resume a completed task; (b) there's no +suggestion about *how* to retry (new chat message? resend the same prompt?); +(c) the error surface in the UI is a plain text `error` store update that +disappears on the next send. + +**Severity:** Blocker — this is the session-ending failure class. + +### 2. Client loses connection and does not recover + +**Where:** `cmd/nomos/main.go:153-272` (SSE `handleChat`) + `web/src/lib/stores/chat.ts:218-331` (`sendMessage`). + +**What happened:** SSE relies on a persistent HTTP connection. When it drops +(network blip, server restart, 10-min timeout), the frontend's stream error +callback sets `error` but: +- No auto-reconnect. The user has to logout/log in to get a fresh SSE stream. +- No fallback to polling during a stream outage. The polling loop + (`startPolling`, chat.ts:152) deliberately bails when `$streaming` is true. +- The error message is not persistent — it clears on next send. + +**Root cause:** SSE streams have no reconnect logic. The connection check is +binary: streaming or not. There's no "connection lost but task still running" +state. + +**Severity:** Blocker — forces logout/login cycle for the user. + +### 3. Agent stops mid-plan — needs "status" nudge to wake up + +**Where:** `cmd/nomos/continue.go:100-155` (`runContinuationWorker` + `processContinuations`). + +**What happened:** The auto-continuation worker polls every 4 seconds for +finished executions and checks `assentWindowActive` (continue.go:145). If the +assent window expired or was never opened, the execution is marked `continued` +without actually resuming — the agent silently drops the ball. The idle sweep +(continue.go:49-65) only fires every **5 minutes**, and its first pass only +nudges the agent with a system note. If that nudge gets lost (empty response, +or the model ignores it), the next sweep auto-closes the task. + +**Root causes:** +- `assentWindowActive` check is too aggressive — if the window expired while + an execution was running, the result is silently discarded. +- No user-visible indication that the agent is "waiting for something." +- The idle sweep's 5-minute interval + 2-pass nudge-then-close is too slow + for a "the agent just stopped talking" situation. + +**Severity:** Blocker — operator has to manually ask "status?" to resume work. + +### 4. No feedback when background work is running + +**Where:** `web/src/lib/stores/chat.ts:149-170` (polling) + `web/src/lib/components/ToolCallGroup.svelte` (tool display). + +**What happened:** When the agent queues an execution (e.g. `apt upgrade -y`), +the tool result says "execution queued — requires approval." After +approval, the execution runs in the background. The chat shows: +- ToolCallGroup: the `run` tool call with its result (static, no updates). +- InlineApproval: the approval card polls execution status and shows it. + +But: when the auto-continuation worker picks up the finished execution and +resumes the agent, the agent's next turn (polling for its messages) is +invisible unless the user happens to be watching the session at that exact +moment. There's no "agent is working on step 4 of 6" indicator. + +**Root causes:** +- Polling only updates the message list — there's no activity indicator. +- `get_execution_status` results are rendered as generic JSON in + ToolCallGroup, not as custom execution-status cards. +- No "last activity" timestamp in the chat header. +- SessionDigest is computed at session fetch time, not live-updated. + +**Severity:** Friction — the agent IS working, the user just can't tell. + +### 5. Approvals sometimes not shown/asked in chat + +**Where:** `web/src/lib/stores/chat.ts:33-52` (`extractApprovals`) + `web/src/lib/components/InlineApproval.svelte`. + +**What happened:** `extractApprovals` scans tool results for `"requires approval"` + +execution UUID. But it only runs on the SSE `done` event (chat.ts:296-302). +If the stream disconnects before `done` (finding #2), approvals are never +extracted in the live view — they'd appear on reload but not in real time. + +Additional gap: the extraction uses `t.args?.action ?? t.args?.purpose ?? t.name` +(chat.ts:43). For `run` tools, `t.name` is `"run"` — not informative. The +purpose text is in `t.args.purpose` but it's a full sentence, not a short label. + +**Root causes:** +- Approvals only extracted on `done`, not on each `tool_result`. +- No fallback extraction on message reload/poll. +- The card label doesn't clearly show what's being approved. + +**Severity:** Friction — approvals ARE in the system (Ops page shows them), +but the chat doesn't always surface them. + +### 6. No custom renderer for `get_execution_status` — 24 calls rendered as raw JSON + +**Where:** `web/src/lib/components/ToolCallGroup.svelte` (generic tool display). + +**What happened:** The agent called `get_execution_status` **24 times** in the +session. Each result is a JSON blob with execution ID, status, target, output, +error. All 24 are rendered as generic collapsed JSON in ToolCallGroup — the +user has to expand each one manually and parse the JSON to understand what's +happening. + +**Fix needed:** Custom renderer via the tool-renderers system (`web/src/lib/tool-renderers/`) +that shows: execution ID (truncated), status badge (running/completed/failed), +target slug, elapsed time, collapsible output. Optionally auto-poll while +the execution is `running`. + +**Severity:** Friction — 24 calls in one session makes the chat cluttered and +hard to follow. + +### 7. Plan changed mid-flight, stale steps left behind + +**Where:** `cmd/nomos/tasks.go:182-205` (`propose_plan`) + `cmd/nomos/store.go:367-423` (plan storage). + +**What happened:** The agent called `propose_plan` 6 times in the session. +Each call appends new steps (store.go:418-423: `appended=true`). But: +- Five `propose_plan` calls mean five generations of plan steps. +- Old steps that were pending when a new plan was proposed remain in the list + with their old status (some `running`, some never started). +- `update_plan_step` only updates by `seq` number — if the new plan reuses + the same `seq` values, old steps get overwritten. If it adds new `seq` + values, old steps remain orphaned. + +**Root causes:** +- No auto-cleanup of pending steps when a new plan is proposed. +- The plan panel shows ALL steps across all plan generations. +- No "plan revision" tracking — just a flat append. + +**Fix needed:** When `propose_plan` is called again, auto-set any still-pending +steps (status ≠ done/failed/skipped/blocked) from the previous generation to +`replaced`. Track a `generation` column on plan steps so the frontend can +collapse or dim old generations. + +**Severity:** Friction — the operator's plan view shows progress that doesn't +reflect reality, eroding trust. + +### 8. 4 orphaned approvals after session completed + +**Where:** `cmd/nomos/store.go:480-509` (`completeTask`) — no approval cleanup. + +**What happened:** When the agent calls `complete_task`, it updates +`agent_sessions.status` and `outcome`. But it does NOT: +- Cancel any `pending_approval` executions linked to this session. +- Close any `approved` but unfinished executions. +- Clean up the `autonomy_settings` assent window rows. + +**Root causes:** +- `completeTask` is a simple status update with no cascading cleanup. +- No lifecycle link between `agent_sessions.id` and `executions.session_id` + for approval cleanup. +- The executions table has `correlation_id` but `completeTask` doesn't use it + to find and cancel open executions. + +**Fix needed:** `completeTask` should auto-cancel any executions in +`pending_approval` or `approved` state for this session. Also clean up the +assent window key from `autonomy_settings`. + +**Severity:** Blocker — leaves the operator with a stale approval count, +confusion about what's pending, and no automatic cleanup path. + +--- + +## Findings — Part B: Agent knowledge loop (what the agent learned vs. what it wrote back) + +### 9. ZERO `update_entity_attributes` or `create_relationship` calls in the entire session + +**Where:** Session `00a344e4` — 22 messages, 11 turns, 348 total tool calls. + +**What happened:** The agent discovered **massive drift** between the DB and +reality but wrote **none** of it back to entity attributes or relationships: + +| Discovery | Tool used | Written back? | +|---|---|---| +| 13 LXCs are `state: destroyed` | `get_entity` × 14 | **No** | +| 5 LXCs reachable via pct exec (SSH auth fails) | `run` × 5 + `get_entity` | **No** | +| teddycloud runs on `host:hubris` not `host:strong` | `get_relations` | **No** | +| rclone was false-negative (resolver glitch), actually alive | `get_entity` | **No** | +| romm has broken `pct config` (nodes/h error) | `run` | **No** | +| Updated upgrade counts for 4 LXCs | `run` (apt-get upgrade) | **No** | + +Every future fleet audit will re-discover these same facts from scratch, +issuing the same 44 wasted tool calls (14 `request_execution` against +destroyed LXCs + 14 `get_entity` to confirm they're destroyed). + +**Root cause:** The plan's final step descriptions were too vague: +- Plan 1 step 4: "Record findings" → agent called `upsert_knowledge` only. +- Plan 2 step 4: "Write back knowledge" → agent called `upsert_knowledge` only. +- Plan 3 step 5: "Write back results" → agent called `upsert_knowledge` only. + +The SOUL.md (lines 86-103) is **explicit** about the three-step writeback: +`update_entity_attributes` → `create_relationship` → `upsert_knowledge` → +`complete_task`. But the plan step descriptions the agent wrote for itself +said "Record findings" — too ambiguous. The agent satisfied this with +narrative knowledge alone and skipped structured entity updates entirely. + +**Severity:** Blocker — the knowledge loop is broken. The graph keeps drifting +further from reality each session because nothing is written back. + +### 10. `list_lxcs` returns all LXCs including destroyed ones — no state filter + +**Where:** `internal/mcp/server.go:664-675` + +**What happened:** The `list_lxcs` MCP tool queries `WHERE e.type = 'lxc'` with +no state filter. The result included 13 destroyed LXCs mixed with 19 live ones. +The agent then: +1. Issued 14 `request_execution` calls against no-IP LXCs (all failed because + the LXCs are destroyed → 14 wasted tool calls). +2. Issued 14 `get_entity` calls to discover they're `state: destroyed`. +3. Had to manually classify which are live and which are destroyed. + +If `list_lxcs` excluded destroyed entities (or accepted a `state` filter), +these 28 tool calls would have been eliminated from the first turn alone — +reducing turn 1 from 92 tool calls to 64 (and making the plan simpler). + +**Root cause:** `list_lxcs` is a fixed SQL query. The `list_entities` API +already supports `state` filtering (entities.sql.go:118: +`$1::text IS NULL OR e.state = $1`), but `list_lxcs` hardcodes its query +without exposing the parameter. + +**Severity:** Friction — wastes ~30% of tool calls per fleet-wide audit. + +### 11. Knowledge recorded was good quality but linked too broadly + +**Where:** Session `00a344e4` — 4 knowledge entries via `upsert_knowledge`. + +**What was recorded:** +| Entry | Kind | Linked to | Quality | +|---|---|---|---| +| "Fleet-wide apt audit — 2026-07-10" | investigation | `cluster:homelab` | Good — full table + caveats | +| "Fleet-wide apt audit — complete — 2026-07-10" | investigation | `cluster:homelab` | Duplicate of v1 — should have updated v1 instead | +| "Low-risk tier upgrade results — 2026-07-10" | investigation | `cluster:homelab` | Good — per-LXC breakdown | +| "How to run fleet apt upgrades — lessons" | document | `agent:nomos` | Good — actionable runbook | + +**What should have been different:** +1. The two audit investigations are near-duplicates — v2 supersedes v1 but v1 + wasn't marked as superseded. `upsert_knowledge` with the same title should + update the existing entry. +2. All entries linked to `cluster:homelab` — none to individual LXCs. The + upgraded LXCs (`lxc:nfs-export`, `lxc:gitea`, `lxc:dns`, + `lxc:auth-outpost`) should each have the upgrade result linked via `about`. + Future `get_entity_knowledge("lxc:nfs-export")` returns nothing for this + session's work. +3. The 13 destroyed LXCs aren't linked to any knowledge entry documenting + *why* they were destroyed. A future agent looking at `lxc:arr-yunohost` + won't find the knowledge entry saying "migrated to arriman, 2026-04-28." + +**Root cause:** The agent used `about: cluster:homelab` for everything. +The `upsert_knowledge` tool accepts a single `about` slug, so the agent +cannot link one entry to multiple entities. Multi-entity linking would +require either multiple `upsert_knowledge` calls (one per LXC) or extending +the tool to accept an `about` array. + +**Severity:** Friction — knowledge exists but is hard to discover per-entity. + +### 12. Plan step descriptions are too vague — agent interprets them weakly + +**Where:** The plan steps the agent proposed for itself across 3 plans. + +**What happened:** The agent wrote plan steps like: +- "Record findings" / "Update knowledge base if anything notable" +- "Write back knowledge" / "Record deprecated list and SSH strategy" +- "Write back results" / "Record what was upgraded and any issues." + +Every time, the agent satisfied these with `upsert_knowledge` alone and +skipped `update_entity_attributes` + `create_relationship`. The SOUL.md +instructions are explicit about the three-step writeback, but the plan +step descriptions the agent generated for itself didn't reinforce this. + +**Root cause:** The agent proposes its own plan steps. The tool description +for `propose_plan` (tasks.go:36-70) says the last step should include +`update_entity_attributes / create_relationship / upsert_knowledge`. But the +agent still wrote vague step descriptions. The instruction is present but +not being followed. + +**Severity:** Blocker — this is the direct cause of finding #9. + +### 13. Plan step completion order was wrong — step 5 marked done before step 4 + +**Where:** Turn 11 (the "lets stop here" final turn) in session `00a344e4`. + +**What happened:** The agent called `update_plan_step` in this order: +1. `seq=5, status=done` — "Write back results" +2. `seq=4, status=done` — "Upgrade auth-outpost" +3. `seq=1, status=done` — "Upgrade nfs-export" +4. `seq=2, status=done` — "Upgrade gitea" +5. `seq=3, status=done` — "Upgrade dns" + +The plan panel would have shown step 5 completing before step 4, then +steps 1-3 completing in reverse order. This is the "plan changed mid-air +and the agent forgot to keep its progress up to date" issue — the agent +rushed to close all remaining steps in the final turn, in no particular +order, without verifying each one was actually done. + +**Root cause:** No ordering validation on `update_plan_step`. The agent can +mark any step as done in any order. At minimum, steps should complete in +seq order. The frontend should also handle out-of-order completions +gracefully (don't reorder the list just because completions arrived out of +sequence). + +**Severity:** Friction — the progress view looks wrong, eroding trust. + +### 14. The 1st turn had 92 tool calls — 30% were wasted on destroyed LXCs + +**Where:** Turn 1 of session `00a344e4`. + +**Breakdown:** +| Category | Count | Tool calls | +|---|---|---| +| Plan/task management | 12 | set_goal × 2, propose_plan × 2, update_plan_step × 8 | +| Research | 2 | list_lxcs × 1, search_knowledge × 1 | +| Legitimate audits | 50 | request_execution against reachable LXCs | +| **Wasted — destroyed LXCs** | **28** | request_execution × 14 (failed), get_entity × 14 (confirm destroyed) | + +92 tool calls in one turn. The agent was efficient (it batched them), but 28 +of them were entirely avoidable if `list_lxcs` had filtered out destroyed +entities or if the DB had up-to-date entity attributes from a prior session. + +**Root cause:** Combination of #9 (no entity attributes written back) + #10 +(no state filter on list_lxcs). Each problem compounds the other. + +**Severity:** Friction — costs latency, OpenRouter credits, and model context +window. A single fleet audit shouldn't need 92 tool calls. + +--- + +## Findings — Part C: The "wins" (things that worked well) + +Despite the issues above, the session had several things working correctly +that should be preserved: + +- **Chat assent → `run` upgrade path works.** Once the agent learned to use + `run` instead of `request_execution`, upgrades auto-ran under the assent + window without re-approval. This is the correct pattern. +- **`pct exec` fallback discovered autonomously.** The agent realized 5 LXCs + fail SSH but `pct exec` from their Proxmox host works. No operator input + needed — the agent investigated and found the alternative. +- **The agent self-corrected `request_execution → run`.** When `request_execution` + calls got stuck at `pending_approval` despite chat assent, the agent + diagnosed the problem ("assent window only covers `run` commands") and + switched tools. Good resilience. +- **Knowledge content quality was high.** All 4 knowledge entries had structured + tables, relevant caveats, and actionable instructions. Content-wise, the + knowledge loop is producing good output — just not linking it to the + right entities. +- **The agent documented a process lesson as a reusable document** ("How to run + fleet apt upgrades"). This is exactly the kind of knowledge that prevents + future sessions from repeating mistakes. The format (runbook with examples + + verification commands) is correct. +- **Multi-step plan execution worked end-to-end.** 3 plans, 21 `update_plan_step` + calls, all steps eventually completed. The plan mechanism itself is solid — + the issues are in step descriptions and writeback completeness. + +--- + +## Improvement plan — with concrete fix descriptions + +### Phase 1: Crash recovery (addresses #1, #2, #3 — the "session dies" class) + +#### 1.1 — SSE auto-reconnect + fallback to polling + +**Files:** `web/src/lib/stores/chat.ts`, `web/src/lib/api.ts` + +**What:** When the SSE stream drops (network blip, server restart), the frontend +should automatically reconnect instead of requiring a logout/login cycle. + +**How:** +1. Add a `reconnectCount` state to `sendMessage()` in chat.ts. When the stream + errors or completes without a `done` event (chat.ts:316-330), set a + `disconnected = true` flag on the current session instead of calling + `streaming.set(false)`. +2. When `disconnected` is true, start a backoff reconnect loop: wait 1s, 2s, + 4s (capped at 8s), then re-post to `/chat` with the *same* `session_id` + and an empty message string + `resume: true` flag. The backend's `handleChat` + routes this into `resumeSession` with a system note like `"[System: the + stream reconnected — continue from where you left off.]"`. On reconnect + success, stop the loop and clear `disconnected`. +3. In `streamChat()` (`api.ts:105-156`), the `catch` and `finally` blocks + call the same callbacks but need to distinguish "aborted by user" + (`AbortError`) from "connection dropped": don't call `onDone()` on + network errors — let the new `onDisconnect` callback handle it instead. + Add a third callback param: `onDisconnect: (reason: string) => void`. +4. Maximum 3 reconnect attempts. After exhausting retries, fall back to + polling: set `streaming` to false, clear `disconnected`, and call + `startPolling(sessionId)` (the existing poll loop in chat.ts:152-170). + The task continues server-side — the poller will catch whatever happened + during the outage. + +#### 1.2 — Connection-lost banner with retry button + +**Files:** `web/src/pages/Chat.svelte` + +**What:** A persistent banner above the input area that shows when the SSE +connection is lost, with a "Reconnect" button and a countdown timer for the +next auto-retry. + +**How:** +1. Add a `connectionState` store to `chat.ts`: `'connected' | 'disconnected' | 'reconnecting'`. + Expose it via a `connection` export. +2. In `Chat.svelte`, add a banner between the message area and the input bar + (around line 170, replacing the current `{#if $error}` block): + ```svelte + {#if $connection === 'disconnected'} +
Agent connection lost.
+ {:else if $connection === 'reconnecting'} +
Reconnecting in {countdown}s…
+ {/if} + ``` +3. `reconnect()` triggers an immediate reconnect attempt (reset the backoff + timer, call `sendMessage("", { resume: true })`). +4. The current `$error` banner (Chat.svelte:164-170) becomes the non-connection + error path — displayed for LLM errors, tool errors, etc. This is a sibling + banner, not a replacement. + +#### 1.3 — On stream drop, immediately poll for messages + +**Files:** `web/src/lib/stores/chat.ts` + +**What:** When the SSE stream drops, don't wait for the user to log out/in. +Start the poller immediately so the chat shows whatever the agent did +server-side during the outage. + +**How:** +1. In `sendMessage()`'s error/complete callbacks (chat.ts:316-330), after + setting `disconnected` (from 1.1), call `startPolling(sessionId)` immediately + instead of waiting for the reconnect loop or manual reload. +2. `startPolling` already bails when `$streaming` is true (chat.ts:156). + After the stream drops, `streaming` stays true because of `disconnected`. + Change the gate: allow polling when `disconnected` is true even if + `streaming` is true. The polled messages are from the persisted DB, so + they won't conflict with the dead SSE stream. +3. When reconnection succeeds (the SSE stream is live again), stop polling + to avoid double-rendering. + +#### 1.4 — Empty-response: retry 3 times instead of 2 + +**Files:** `cmd/nomos/agent.go:33` + +**What:** `maxLLMRetries = 1` means the inner loop (agent.go:331-375) retries +once, and `resumeSession` (continue.go:216) retries the whole call once — 2 +total chances. DeepSeek sometimes needs 3. + +**How:** +1. Change `const maxLLMRetries = 1` to `const maxLLMRetries = 2` at + `agent.go:33`. This gives 3 total attempts in the inner loop. +2. In `resumeSession` (continue.go:216), change `for attempt := 0; attempt < 2` + to `for attempt := 0; attempt < 3` for 3 outer-loop attempts. +3. On each retry inside `resumeSession`, inject a stronger system note: + `"[System: your previous response was empty or invalid — the operator is + waiting. Produce a real response this time.]"` instead of just re-running + the same prompt. + +#### 1.5 — Empty-response: don't auto-complete the task on failure + +**Files:** `cmd/nomos/continue.go:252-258` + +**What:** When `resumeSession` exhausts retries, it calls `completeTask` with +`outcome='failure'` (continue.go:256). This marks the session as terminal, +so the next user message in chat can't resume it — the user has to know to +start a new task. + +**How:** +1. Remove the `completeTask` call at continue.go:256. Instead, persist a + `"resume_failed"` system note as a regular assistant message in the + transcript so the user sees what happened: + ```go + resumeFailedNote := fmt.Sprintf("[System: auto-resume failed after retrying: %s. The task is paused — send another message to try again.]", errText) + ``` +2. Leave `agent_sessions.status` at `'executing'` (or whatever it was before + the failed resume). The user's next chat message will re-enter `handleChat`, + which replays the full history and picks up from the last tool calls. +3. Only auto-close if the task was already in a genuinely terminal state + (check `status` before deciding). + +#### 1.6 — Persistent, dismissible error card in chat + +**Files:** `web/src/lib/stores/chat.ts` (new store: `chatErrors`), `web/src/pages/Chat.svelte` + +**What:** The current `$error` store (chat.ts:72) is a single string that +disappears on the next `sendMessage()`. For empty-response errors, the user +needs a card that stays visible until dismissed, explaining what went wrong +and suggesting a recovery action. + +**How:** +1. Add `chatErrors` as a writable store of error objects: + ```ts + interface ChatError { id: string; message: string; dismissible: boolean; action?: string } + ``` +2. In `sendMessage()`'s error handler, push a `ChatError` instead of setting + `error.set(err)`. The `action` field suggests recovery (e.g. "type 'status' + to check what happened" or "click Retry to resend"). +3. In `Chat.svelte`, render `chatErrors` as dismissible cards above the input + bar (replacing or alongside the current `$error` banner): + ```svelte + {#each $chatErrors as err (err.id)} +
+ {err.message} + {#if err.action}{/if} + +
+ {/each} + ``` +4. Clear `chatErrors` on `newChat()` but NOT on `sendMessage()` — errors + survive across messages until explicitly dismissed. + +--- + +### Phase 2: Visibility (addresses #4, #5, #6 — "what is the agent doing?") + +#### 2.1 — Custom renderer for `get_execution_status` + +**Files:** New: `web/src/lib/tool-renderers/ExecutionStatus.svelte`, `web/src/lib/tool-renderers/index.ts` + +**What:** `get_execution_status` results are large JSON blobs rendered in +ToolCallGroup's generic `
` blocks. A custom renderer shows execution
+state inline with a status badge, live polling, and collapsible output.
+
+**How:**
+1. Create `web/src/lib/tool-renderers/ExecutionStatus.svelte`:
+   - Props: `tool: ToolCallResult`
+   - Extract from `tool.result`: `execution_id`, `status`, `target`, `action`,
+     `result`, `error`, `duration_ms`
+   - Render a compact card:
+     ```
+     [spinner/check/X] execution 019f5f.. | apt_upgrade on lxc:nfs-export | completed (12.3s)
+     ▶ output: 0 upgraded, 0 newly installed...
+     ```
+   - If status is `running` or `approved`, auto-poll `getExecution(id)` every
+     3 seconds (via `api.ts`) and update the card in place. Stop when terminal.
+   - Show elapsed wall time (live counter while running).
+   - Collapsible output section (default collapsed for completed, expanded
+     for failed with error text in red).
+
+2. Register in `tool-renderers.ts`:
+   ```ts
+   import ExecutionStatus from './tool-renderers/ExecutionStatus.svelte'
+   registerToolRenderer({
+     match: (t) => t.name === 'get_execution_status' && t.type === 'tool_result',
+     component: ExecutionStatus
+   })
+   ```
+
+3. In `ToolCallGroup.svelte`, the tool will now be matched by `getToolRenderer()`
+   and rendered inline by `Chat.svelte`'s `getInlineTools` — no changes needed
+   to the existing ToolCallGroup unless the card should also appear inside the
+   group (it should — add the custom card to ToolCallGroup's body too, or
+   simply ensure `getInlineTools` claims it and ToolCallGroup's `unmatched`
+   prop excludes it).
+
+#### 2.2 — Extract approvals on `tool_result` events, not just `done`
+
+**Files:** `web/src/lib/stores/chat.ts:243-277`
+
+**What:** `extractApprovals()` only runs on the SSE `done` event. If the stream
+drops before `done`, the approvals never appear in chat (they exist on the Ops
+page but the chat shows nothing).
+
+**How:**
+1. In the `tool_result` branch of `sendMessage()` (chat.ts:258-277), after
+   updating the tool in `activeTools` and the message's `tools` array, run
+   `extractApprovals` on the full `tools` array of the current message and
+   set `msg.pendingApprovals` immediately:
+   ```ts
+   // inside tool_result handler, after updating the tool:
+   messages.update((ms) => {
+     const last = ms[ms.length - 1]
+     if (last && last.role === 'assistant') {
+       last.tools = last.tools.map((t) => t.id === ev.data.id ? updated : t)
+       last.pendingApprovals = extractApprovals(last.tools) // <-- add this
+     }
+     return [...ms]
+   })
+   ```
+2. Keep the `done`-event extraction as a final sanity pass (it catches any
+   edge case where `tool_result` arrived before the tool was registered in
+   `activeTools`).
+
+#### 2.3 — Approval card label: use `purpose` text
+
+**Files:** `web/src/lib/stores/chat.ts:43`
+
+**What:** `extractApprovals()` picks `t.args?.action ?? t.args?.purpose ?? t.name`.
+For `run` tools, `t.name` is `"run"` — not informative. The `purpose` text is
+a full sentence like "Upgrade nfs-export (21 packages)". Better to truncate it.
+
+**How:**
+1. Change the label logic in `extractApprovals`:
+   ```ts
+   action: t.args?.purpose?.slice(0, 60) ?? t.args?.action ?? t.name ?? 'unknown',
+   ```
+   `purpose` is always present and meaningful for `run` calls; falling back to
+   `action` (for `request_execution`) and then `name` (last resort). Truncate
+   to 60 chars to fit the card.
+
+#### 2.4 — "Agent is working" indicator in chat header
+
+**Files:** `web/src/pages/Chat.svelte`
+
+**What:** The chat page has no indicator that the agent is doing autonomous work
+(executions running, auto-continuation polling, plan steps advancing). The user
+stares at a static chat and wonders if anything is happening.
+
+**How:**
+1. Add a `sessionActivity` store in `chat.ts`:
+   ```ts
+   interface SessionActivity {
+     lastMessageAt: Date | null
+     executionsRunning: number
+     planStep: string | null  // "3/6 — Upgrade nfs-export"
+   }
+   ```
+2. Update `sessionActivity` from:
+   - Poll fetches: when a new message arrives via `startPolling`, set `lastMessageAt`.
+   - SSE events: `tool_result` and `text_delta` events reset `lastMessageAt`.
+   - API calls to fetch active executions for the session (new minimal endpoint
+     or derived from the session digest).
+3. In `Chat.svelte`, render a thin header bar above the messages area:
+   ```svelte
+   {#if $currentSession}
+     
+ {#if $activity.executionsRunning > 0} + {$activity.executionsRunning} running + {/if} + {#if $activity.planStep} + {$activity.planStep} + {/if} + Last active: {timeago($activity.lastMessageAt)} +
+ {/if} + ``` + +#### 2.5 — SessionDigest: poll live when session is active + +**Files:** `web/src/lib/components/SessionDigest.svelte` + +**What:** SessionDigest fetches data once when the session loads. For an active +session with background work, it should refresh periodically. + +**How:** +1. In `SessionDigest.svelte`, add a `$effect` that watches `$currentSession` + and the session's status: + ```svelte + $effect(() => { + if (!sessionId || status === 'done' || status === 'failed') return + const timer = setInterval(() => fetchDigest(sessionId), 10000) + return () => clearInterval(timer) + }) + ``` +2. Only re-render changed parts — Svelte's reactivity handles this since + `digest` is a reactive declaration. The fetch updates the store; the + template re-renders only the changed values (e.g. `executions.running` goes + from 4→3). + +#### 2.6 — Live execution count in the task header + +**Files:** `web/src/pages/Chat.svelte`, `web/src/lib/api.ts` + +**What:** The task context panel shows plan progress, but the chat itself has +no execution counter. "3 executions running" in the chat header tells the user +at a glance that work is happening. + +**How:** +1. Add a `fetchSessionExecutions(sessionId)` to `api.ts` that calls + `GET /api/v1/executions?session_id={id}&status=running,approved`. + (This endpoint may need adding — or use the existing executions endpoint + with a new `session_id` filter.) +2. Expose as a derived store in `chat.ts`; poll it alongside the message + poller. +3. Integrate with the activity bar from 2.4 (they share the same data). + +--- + +### Phase 3: Cleanup (addresses #7, #8 — "leftover state") + +#### 3.1 — `complete_task`: auto-cancel pending executions + +**Files:** `cmd/nomos/store.go:480-509` + +**What:** When the agent calls `complete_task`, any executions in +`pending_approval` or `approved` state for this session stay that way forever. +The approvals list on the Ops page shows stale items, and the operator +has to manually cancel them. + +**How:** +1. In `completeTask`, BEFORE updating `agent_sessions`, run two cleanup queries: + ```sql + -- 1. Cancel pending-approval executions linked to this session + UPDATE executions SET status = 'cancelled', result = '{"message": "task completed — auto-cancelled"}'::jsonb + WHERE entity_id IN ( + SELECT execution_id FROM nomos_plan_executions WHERE session_id = $1 + ) AND status IN ('pending_approval', 'approved', 'queued'); + + -- 2. Mark them as continued so the worker won't try to auto-continue them + UPDATE nomos_plan_executions SET continued_at = now() + WHERE session_id = $1 AND continued_at IS NULL; + ``` +2. For each cancelled execution, emit an `execution.cancelled` event so the + live approvals list refreshes (same pattern as `observability.Event` calls + elsewhere in store.go). +3. The sqlcgen observability event from `completeTask` already fires + `task.status` — add `cancelled_count: N` to the event data so the frontend + can show "task completed (3 pending approvals auto-cancelled)." + +#### 3.2 — `complete_task`: delete assent window + destructive window + +**Files:** `cmd/nomos/store.go:480-509` + +**What:** The `autonomy_settings` table holds the assent window key +(`nomos:assent_window::`) and destructive window key +(`nomos:destructive:::`). On task completion, +these are stale and should be cleaned up. + +**How:** +1. Add a cleanup query to `completeTask`: + ```sql + DELETE FROM autonomy_settings + WHERE key = $1 OR key LIKE $2 + ``` + Where `$1` = the session's assent window key and `$2` = the destructive + window pattern for this session. +2. Compute the keys: `assentWindowKey(a.agentID, sessionID)` and + `nomos:destructive::*:` pattern. +3. This is a single DELETE before the return — no transaction needed, fire + and forget (failure is logged, not blocking). + +#### 3.3 — `propose_plan`: auto-replace pending steps with `replaced` status + +**Files:** `cmd/nomos/store.go:367-423` + +**What:** When the agent calls `propose_plan` again mid-flight, new steps are +appended after the current max seq (store.go:379-389). Pending steps from the +old plan (status = `'pending'`) remain in the list forever — they were never +started and never will be. + +**How:** +1. In `proposePlan`, after the `DELETE FROM session_plan_steps WHERE session_id = $1` + for the fresh-start path (store.go:386), add ELSE logic for the append path: + ```sql + -- Before appending new steps, mark any still-pending steps as 'replaced' + UPDATE session_plan_steps + SET status = 'replaced', finished_at = now() + WHERE session_id = $1 AND status = 'pending'; + ``` +2. The `replaced` status is already a valid terminal state — the plan panel + should treat it the same as `skipped` (dimmed, no progress contribution). +3. Add `'replaced'` to the stamp switch in `updatePlanStep` (store.go:436-441) + so no one can accidentally un-replace a step. + +#### 3.4 — Plan steps: `generation` column + frontend collapse + +**Files:** `cmd/nomos/store.go` (schema migration + proposePlan), `web/src/lib/components/PlanPanel.svelte` + +**What:** With 3.3, old steps get marked `replaced`, but the flat list still +shows every step ever created. The operator sees a confusing mix of old and +new plan steps. A `generation` column lets the frontend group and collapse. + +**How:** +1. Migration: `ALTER TABLE session_plan_steps ADD COLUMN generation INTEGER NOT NULL DEFAULT 1;` +2. In `proposePlan`, resolve the new generation number: + ```sql + SELECT COALESCE(MAX(generation), 0) + 1 FROM session_plan_steps WHERE session_id = $1 + ``` + If this is a fresh-start (no steps started, delete + re-insert), reset to 1. + If appending, use next generation number (typically 2, 3, ...). +3. Each new step inserted gets this `generation` value. +4. In the `planStep` struct (store.go:567), add `Generation int \`json:"generation"\``. +5. In `getPlanSteps`, include `generation` in the SELECT. +6. In `PlanPanel.svelte` (or wherever plan steps are rendered), group by + `generation`. Show the current generation (highest number) expanded; collapse + older ones with a label like "Plan v1 (replaced)" and a count of steps. + +#### 3.5 — Sessions list: show open-approval count per session + +**Files:** `cmd/nomos/store.go` (new query), `cmd/nomos/main.go` (API response), +`web/src/lib/components/SessionRail.svelte` + +**What:** The sessions list shows title, status, time. Adding "2 approvals +pending" tells the operator at a glance which sessions have open actions. + +**How:** +1. Add a `pendingApprovalCount` method on store: + ```go + func (s *store) pendingApprovalCounts(ctx context.Context, sessionIDs []string) map[string]int { + // SELECT l.session_id, COUNT(*) FROM nomos_plan_executions l + // JOIN executions e ON e.entity_id = l.execution_id + // WHERE l.session_id = ANY($1) AND e.status = 'pending_approval' + // GROUP BY l.session_id + } + ``` +2. In `handleSessionsList` (main.go:276), call this for the returned sessions + and add `pending_approvals` to each session JSON object. +3. In `SessionRail.svelte`, show a yellow badge next to sessions with + `pending_approvals > 0`. Clicking navigates to that session AND opens + the approvals section. + +--- + +### Phase 4: Continuation hardening (addresses #3 — "agent stops") + +#### 4.1 — When assent window is missing, inject a visible note + +**Files:** `cmd/nomos/continue.go:145-151` + +**What:** When `assentWindowActive` returns false, the execution is marked +`continued` without resuming. The user sees nothing. Instead, persist a +visible note in the transcript explaining WHY the agent didn't continue. + +**How:** +1. In `processContinuations` (continue.go:145-151), before `markContinued`, insert + a system note as an assistant message in the transcript: + ```go + note := fmt.Sprintf("[System: execution %s finished, but the assent window for this session is not active (may have expired). The agent will not auto-continue. Reply 'continue' or re-approve the plan to resume.]", p.ExecID) + a.store.saveMessage(context.Background(), p.SessionID, "assistant", jsonNote) + ``` +2. This makes the auto-continue failure visible to the operator in the chat + history — they can see WHY the agent stopped and what to do about it. + +#### 4.2 — Re-open expired assent window for running executions + +**Files:** `cmd/nomos/continue.go:145-151` + +**What:** The plan was approved. The execution ran. The window expired during +execution. Penalizing timing is wrong — the agent should still get the result +and continue. + +**How:** +1. In `processContinuations`, when `assentWindowActive` is false, don't just + mark it — check if the session's goal is set and the task status is + `'executing'` (meaning a plan was approved and is being worked): + ```go + if !a.store.assentWindowActive(ctx, a.agentID, p.SessionID) { + sesh, _ := a.store.getSession(ctx, p.SessionID) + if sesh.Goal != "" && sesh.Status == "executing" { + // Plan was approved, work is in progress — re-open the window + a.openAssentWindow(ctx, p.SessionID) + // THEN continue (don't markContinued — execute the normal path) + } else { + // No plan, no goal — genuinely shouldn't continue (4.1's note) + a.store.markContinued(ctx, p.ExecID) + continue + } + } + ``` +2. The `a.store.markContinued(...)` line at continue.go:152 must move inside + the `if !active` block. Currently it fires unconditionally before the + `safego.Go` — that's a bug: it marks executions as continued even when + the continuation IS dispatched, which is safe but unnecessary. With this + fix, it becomes important that `markContinued` ONLY fires when we're + NOT continuing. + +#### 4.3 — Reduce idle sweep interval to 2 min (first pass) + +**Files:** `cmd/nomos/continue.go:55` + +**What:** The idle sweep fires every 5 minutes. A stopped agent feels dead +long before 5 minutes pass. + +**How:** +1. Change `time.NewTicker(5 * time.Minute)` to `time.NewTicker(2 * time.Minute)` + at continue.go:55. +2. Keep the two-pass logic (nudge → auto-close) but at 2-minute spacing + instead of 5-minute. Net: an agent that stops responding due to a stuck + continuation is nudged at 2 min and auto-closed at 4 min instead of 5 + and 10. + +#### 4.4 — "Continue" button in chat + +**Files:** `web/src/lib/stores/chat.ts`, `web/src/pages/Chat.svelte`, +`cmd/nomos/main.go` + +**What:** When the agent seems stuck, the operator should be able to click +"Continue" instead of typing "continue" or "status?". + +**How:** +1. Add a `resumeSession` API call to `api.ts`: + ```ts + export function resumeSession(sessionId: string): Promise { + return fetchWithAuth(`${BASE}/sessions/${sessionId}/resume`, { method: 'POST' }).then(r => r.ok) + } + ``` +2. Add a handler in `main.go`: + ```go + case "/resume": + note := "[System: the operator wants you to continue. Pick up where you left off — execute the next step of the plan, diagnose and fix any failures, or report progress if everything is done.]" + safego.Go("nomos:resume-session", func() { a.resumeSession(context.Background(), sessionID, note) }) + w.WriteHeader(202) + ``` +3. In `Chat.svelte`, add a "Continue" button in the activity bar from 2.4. + Show it when: + - The session is active (`status === 'executing'` or `'awaiting_input'`). + - The most recent message is from the agent (not the user). + - No stream is active. + - More than 30 seconds have passed since the last message (agent might + be stuck). + On click, calls `resumeSession($currentSession)`, starts polling, and + shows a brief "Agent resumed" toast. +4. Debounce: disable the button for 30 seconds after clicking to prevent + spam. + +--- + +### Phase 5: Knowledge loop (addresses #9, #10, #11, #12, #13, #14 — "the graph keeps drifting") + +#### 5.1 — `list_lxcs`: add `state` filter parameter + +**Files:** `internal/mcp/server.go:664-675` + +**What:** `list_lxcs` returns all 33 LXCs including 13 destroyed ones. The SQL +for `list_entities` already supports `state` filtering — port the same pattern. + +**How:** +1. Add an optional `state` parameter to the tool's `InputSchema`: + ```go + InputSchema: objSchema( + prop{"state", "string", "Optional: filter by entity state (active, destroyed, …)"}, + ), + ``` +2. Change the SQL to use the parameter: + ```sql + WHERE e.type = 'lxc' + AND ($1::text IS NULL OR e.state = $1) + ORDER BY (e.attributes->>'pve_id')::int + ``` +3. Default (no filter) returns all LXCs — backward compatible. When `state` + is passed, only matching entities are returned. +4. Update SOUL.md to recommend: `list_lxcs(state="active")` for fleet audits, + `list_lxcs(state="destroyed")` for cleanup lists. This alone eliminates + ~30% of tool calls from the 92-call first turn. + +#### 5.2 — `upsert_knowledge`: accept `about` as an array of entity slugs + +**Files:** `internal/mcp/server.go` (the upsert_knowledge handler) + +**What:** `upsert_knowledge` accepts a single `about` slug (e.g. `cluster:homelab`). +The agent can't link one knowledge entry to multiple entities without calling +the tool once per entity. The upgrade results session should have linked to +each upgraded LXC individually. + +**How:** +1. Add `about` as an **array** in the tool schema (accept both single string + and array for backward compat): + ```go + InputSchema: objSchema( + prop{"title", "string", "…"}, + prop{"content", "string", "…"}, + prop{"kind", "string", "…"}, + prop{"tags", "string", "…"}, + prop{"about", "array", "Entity slugs this knowledge is about (e.g. ['lxc:nfs-export', 'lxc:gitea'])"}, + ), + ``` +2. In the handler, normalize single string → single-element array. Create + `documented` relationships for each slug: `knowledge_entity → about → entity`. +3. This lets the agent record: "Low-risk tier upgrades" about + `[lxc:nfs-export, lxc:gitea, lxc:dns, lxc:auth-outpost]` in one call. + +#### 5.3 — SOUL.md: make the writeback instruction unmissable + +**Files:** `nomos/SOUL.md` (lines 86-103) + +**What:** The instructions exist but the agent skipped them. The plan step +descriptions the agent wrote for itself were too vague. + +**How:** +1. Add a **bold, standalone section** near the top of SOUL.md (before the + "Every chat is a task" section): + ``` + ## ⚠️ 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. + 2. `create_relationship` — ANY edge you discovered (hosts, depends-on, + provides). Every "X runs on Y" fact. + 3. `upsert_knowledge` — narrative: what you did, what broke, the fix. + Link to ALL affected entities via `about`. + + **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. + ``` +2. Update the `propose_plan` tool description (tasks.go:42-45) to add + stronger language: "Your LAST step MUST have a detail line that literally + says '1. update_entity_attributes for X, 2. create_relationship for Y, + 3. upsert_knowledge about Z' so you don't skip entity writeback." +3. This is a cheap, high-impact change — it doesn't require code, just + clearer instructions. + +#### 5.4 — `propose_plan`: validate that the last step mentions entity writeback tools + +**Files:** `cmd/nomos/tasks.go:196-205` (propose_plan handler) + +**What:** The agent can propose a plan whose last step is "Record findings" and +no one checks whether it includes entity attribute updates. Add a validation +nudge. + +**How:** +1. In `propose_plan`, after persisting steps, check if the final step's + `title` or `detail` contains `"update_entity_attributes"` or + `"create_relationship"`: + ```go + lastStep := steps[len(steps)-1] + hasWriteback := strings.Contains(lastStep.Title+lastStep.Detail, "update_entity_attributes") || + strings.Contains(lastStep.Title+lastStep.Detail, "create_relationship") + if !hasWriteback { + return fmt.Sprintf("Plan set: %d step(s). ⚠️ The final step doesn't mention update_entity_attributes or create_relationship. Without those, any facts you discovered about entities will be lost. Consider adding them to the last step.", len(persisted)), true + } + ``` +2. This doesn't *enforce* — it's a nudge in the tool result. The agent sees it + and can self-correct in the same turn (call `update_plan_step` to fix the + last step's detail, or call `propose_plan` again with a corrected plan). + +#### 5.5 — `complete_task`: validate that entity attributes were written back + +**Files:** `cmd/nomos/store.go:480-509` + +**What:** If the agent calls `complete_task` without having called +`update_entity_attributes` or `create_relationship` in this session, the +completion message should warn about it. + +**How:** +1. Add a check in `completeTask`: query `audit_log` for this session's use of + `update_entity_attributes` and `create_relationship` within the last N + minutes (the session's active window): + ```sql + SELECT EXISTS( + SELECT 1 FROM audit_log + WHERE details->>'session_id' = $1 + AND action IN ('update_entity_attributes', 'create_relationship') + AND created_at > now() - interval '60 minutes' + ) AS has_writeback + ``` +2. If no writeback was done, return `"Task marked success: … ⚠️ No entity + attributes or relationships were updated in this session. Call + update_entity_attributes and create_relationship before completing + to keep the graph current."`. +3. The existing `complete_task` return value (tasks.go:264) includes this + text, which becomes the assistant's visible message — the operator and + the agent both see the warning. + +#### 5.6 — Plan step ordering: enforce seq-order on completion + +**Files:** `cmd/nomos/store.go:431-473` (updatePlanStep) + +**What:** The agent marked step 5 as done before step 4 (finding #13). The +frontend sees steps completing in nonsensical order. + +**How:** +1. In `updatePlanStep`, when status is `done/failed/skipped/blocked`, check + that all lower seq numbers are also in a terminal state: + ```sql + UPDATE session_plan_steps + SET status = $3, … + WHERE session_id = $1 AND seq = $2 + AND NOT EXISTS ( + SELECT 1 FROM session_plan_steps ps + WHERE ps.session_id = $1 + AND ps.seq < $2 + AND ps.status = 'pending' + ) + RETURNING id, target_slug + ``` + If no rows are affected (because a prior step is still pending), return + an error: `"Cannot complete step %d — step %d is still pending."` +2. This prevents the out-of-order completion. The agent must mark steps in + order. If the design genuinely allows out-of-order (parallel steps), add + a `depends_on_seq` column — but for now, strict ordering is simpler and + correct for the agent's sequential plan style. + +#### 5.7 — Audit log: persist tool results (not just args) for analysis + +**Files:** `cmd/nomos/agent.go:446-461` (logActivity call) + +**What:** The audit log records `tool`, `args`, `result`, `success` per tool +call. To audit a session like this one programmatically, we need to know +which entities were touched, what their state was before/after, and whether +entity attributes were updated. + +**How:** +1. Add a `session_id` column to the `audit_log` entries written by + `logActivity` (agent.go:447 and 461). Currently the log has no session + linkage — you can't query "what did session X do?" +2. Add structured fields: `entity_slugs` (array of slugs found in args) and + `tool_name` (already present as `action`). This lets queries like: + ```sql + SELECT COUNT(*) FROM audit_log + WHERE session_id = $1 AND action = 'update_entity_attributes' + ``` + which 5.5 needs. +3. This is a low-priority improvement that makes 5.5's check fast (a log + query vs. a full text scan of `result_json`). + +--- + +### Database migration (updated — covers 3.4, 3.5, and 5.7) + +```sql +-- Add generation tracking to plan steps +ALTER TABLE session_plan_steps ADD COLUMN IF NOT EXISTS generation INTEGER NOT NULL DEFAULT 1; + +-- Add index for pending-approval lookup by session +CREATE INDEX IF NOT EXISTS idx_nomos_plan_executions_session + ON nomos_plan_executions (session_id) WHERE continued_at IS NULL; + +-- Add session_id to audit_log for per-session analysis +ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS session_id UUID; +CREATE INDEX IF NOT EXISTS idx_audit_log_session ON audit_log (session_id); +``` + +--- + +## Implementation order + +1. **Phase 1 first** — the session-ending failures (#1 empty responses, #2 disconnects, #3 agent stops) are stop-ship issues that ruin the user experience. +2. **Phase 5 second** — the knowledge loop (#9, #10, #11, #12, #13, #14) is the most impactful set of improvements per token spent. Making `list_lxcs` filterable (5.1) and updating SOUL.md (5.3) are 30-minute changes that eliminate 30% of wasted tool calls and close the writeback gap. The `about` array (5.2) and completion validation (5.5) make the knowledge graph actually learn from sessions. +3. **Phase 2 third** — visibility improvements. Custom renderers and activity indicators are polish compared to the knowledge loop actually working. +4. **Phase 3 fourth** — cleanup of leftover state. The `complete_task` cancellation (3.1, 3.2) directly addresses the 4 orphaned approvals. +5. **Phase 4 last** — continuation hardening builds on the other phases. + +## What this plan does NOT address + +- C1 (unauthenticated nomos gateway) — already in `plans/2026-07-11-nomos-agent-code-review.md`. +- `request_execution` enum retirement — already in `plans/2026-07-10-general-gated-execution.md` Layer 1. +- Token-aware history windowing — already in the code review plan (A2), deferred. +- Auto-act revival — already in the general-gated-execution plan, deferred. +- Full audit of the `list_lxcs` output format — the `__renderer: lxc_list` annotation works for the frontend but the JSON structure is still flat (no distinction between active/destroyed entities). A richer shape (grouped by state) is future work. +- Deduplication of `upsert_knowledge` entries with the same title — the tool currently creates a new entry each time. Same-title updates should upsert instead of duplicate (future work). + +## Verification + +- **1.1-1.3**: Kill the nomos process mid-turn, confirm the frontend shows a reconnect banner and resumes polling within 3 seconds. +- **1.4-1.6**: Send a prompt known to produce empty responses, confirm retries fire 3x and the error card stays visible until dismissed. +- **2.1**: Open a session with `get_execution_status` calls, confirm each renders as a status card with live polling, not raw JSON. +- **2.2**: Queue a run that requires approval, confirm the approval card appears immediately (not just on `done`). +- **3.1-3.2**: Complete a task that has pending approvals, confirm they're auto-cancelled and the assent window is cleaned up. +- **3.3-3.4**: Call `propose_plan` twice in one session, confirm old pending steps are marked `replaced` and the panel shows only the current plan. +- **4.1-4.2**: Run an execution that finishes after the assent window expires, confirm the agent still picks it up and resumes. +- **5.1**: Call `list_lxcs(state="active")` — confirm only active LXCs are returned, not destroyed ones. Re-run the apt audit scenario — confirm the first turn has ~64 tool calls instead of 92. +- **5.2**: Call `upsert_knowledge` with `about: ["lxc:nfs-export", "lxc:gitea"]` — confirm knowledge is linked to both entities. Query `get_entity_knowledge("lxc:nfs-export")` — confirm the entry appears. +- **5.3**: Send a new fleet audit request. Confirm the agent's proposed plan has a last step that explicitly lists `update_entity_attributes`, `create_relationship`, and `upsert_knowledge` by name. +- **5.4**: Propose a plan whose last step says "Record findings" without mentioning entity writeback. Confirm the tool result warns about it. +- **5.5**: Complete a task that never called `update_entity_attributes`. Confirm the completion message warns "No entity attributes or relationships were updated." +- **5.6**: Call `update_plan_step(seq=5, status="done")` while step 4 is `pending`. Confirm the call returns an error: "Cannot complete step 5 — step 4 is still pending." diff --git a/plans/index.md b/plans/index.md index 413c4ed..a3ab9b0 100644 --- a/plans/index.md +++ b/plans/index.md @@ -14,6 +14,7 @@ went sideways, open an investigation. | 2026-07-08 | [Liveness, drift, and UX cohesion](2026-07-08-liveness-drift-and-ux-cohesion.md) | In Progress — Phase 5 deferred | | 2026-07-10 | [General gated execution: unlimited actions, gated by risk](2026-07-10-general-gated-execution.md) | In Progress — enum retirement + auto-act revival still open | | 2026-07-11 | [Nomos agent code review: gaps and improvement plan](2026-07-11-nomos-agent-code-review.md) | In Progress — only C1 (unauthenticated nomos gateway) still open, deferred | +| 2026-07-14 | [Session reliability & UX audit](2026-07-14-session-reliability-and-ux-audit.md) | Planned — just audited, not started | ## Done diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 1a8d5ae..1178004 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -20,6 +20,7 @@ export interface Session { outcome?: string // success | failure | partial summary?: string entity_id?: string + pending_approvals?: number created_at: string last_active_at: string } @@ -51,12 +52,18 @@ export async function deleteSession(sessionId: string): Promise { return res.ok } +export async function resumeSession(sessionId: string): Promise { + const res = await fetchWithAuth(`${BASE}/sessions/${sessionId}/resume`, { method: 'POST' }) + return res.ok +} + export interface PlanStep { id: string seq: number title: string detail: string - status: 'pending' | 'running' | 'done' | 'failed' | 'skipped' | 'blocked' + status: 'pending' | 'running' | 'done' | 'failed' | 'skipped' | 'blocked' | 'replaced' + generation?: number execution_id?: string target_slug?: string started_at?: string diff --git a/web/src/lib/components/PlanProgress.svelte b/web/src/lib/components/PlanProgress.svelte index a9ea5de..36fdd7f 100644 --- a/web/src/lib/components/PlanProgress.svelte +++ b/web/src/lib/components/PlanProgress.svelte @@ -7,19 +7,32 @@ import CircleXIcon from '@lucide/svelte/icons/circle-x' import CircleSlashIcon from '@lucide/svelte/icons/circle-slash' import CirclePauseIcon from '@lucide/svelte/icons/circle-pause' + import ChevronDownIcon from '@lucide/svelte/icons/chevron-down' + import ChevronRightIcon from '@lucide/svelte/icons/chevron-right' const done = $derived($planSteps.filter((s) => s.status === 'done').length) const total = $derived($planSteps.length) const pct = $derived(total > 0 ? Math.round((done / total) * 100) : 0) + // Group steps by generation. The latest generation is shown expanded; + // older ones are collapsible. + const byGeneration = $derived.by(() => { + const groups: Map = new Map() + for (const s of $planSteps) { + const g = s.generation ?? 1 + if (!groups.has(g)) groups.set(g, []) + groups.get(g)!.push(s) + } + return Array.from(groups.entries()).sort(([a], [b]) => a - b) + }) + + const latestGen = $derived(byGeneration.length > 0 ? byGeneration[byGeneration.length - 1][0] : 0) + + let openGens = $state(new Set()) + let sheetSlug = $state(null) let sheetOpen = $state(false) - // Tool calls don't carry a step id, so a step can't be linked to its exact - // transcript entry — but its target entity IS known, and EntitySheet already - // gives a real, working detail view for any slug. Clicking a step with a - // target opens that, rather than a fake "scroll to it" that would silently - // no-op for a collapsed tool-call group. function openStep(targetSlug: string | undefined) { if (!targetSlug) return sheetSlug = targetSlug @@ -36,41 +49,61 @@
-
    - {#each $planSteps as step (step.id)} -
  1. - -
  2. - {/each} -
+ + {#each byGeneration as [gen, steps] (gen)} + {@const isLatest = gen === latestGen} + {#if byGeneration.length > 1} + + {/if} + {#if isLatest || openGens.has(gen)} +
    + {#each steps as step (step.id)} +
  1. + +
  2. + {/each} +
+ {/if} + {/each} {/if} diff --git a/web/src/lib/components/SessionDigest.svelte b/web/src/lib/components/SessionDigest.svelte index 304fab2..141cba4 100644 --- a/web/src/lib/components/SessionDigest.svelte +++ b/web/src/lib/components/SessionDigest.svelte @@ -16,18 +16,27 @@ // still refetch once outcome/summary land, not just on session switch. let loadedKey = $state(null) - // Reload the digest whenever the session changes, the task's status changes - // (e.g. it just completed), or a stream finishes — "what did this session - // actually do" is only meaningful once executions have had a chance to land. + let pollTimer: ReturnType | null = $state(null) + $effect(() => { const sid = $currentSession const busy = $streaming const status = $currentTask?.status ?? '' if (!sid || busy) return const key = `${sid}:${status}` - if (loadedKey === key) return - loadedKey = key - fetchSessionDigest(sid).then((d) => (digest = d)) + if (loadedKey !== key) { + loadedKey = key + fetchSessionDigest(sid).then((d) => (digest = d)) + } + // Poll every 10s while the session is active. + if (status !== 'done' && status !== 'failed') { + if (!pollTimer) pollTimer = setInterval(() => fetchSessionDigest(sid).then((d) => (digest = d)), 10000) + } else { + if (pollTimer) { clearInterval(pollTimer); pollTimer = null } + } + return () => { + if (pollTimer) { clearInterval(pollTimer); pollTimer = null } + } }) function statusVariant(status: string): 'default' | 'secondary' | 'destructive' | 'outline' { diff --git a/web/src/lib/components/SessionRail.svelte b/web/src/lib/components/SessionRail.svelte index 59a1356..b7aad57 100644 --- a/web/src/lib/components/SessionRail.svelte +++ b/web/src/lib/components/SessionRail.svelte @@ -51,7 +51,12 @@ onclick={() => handleClick(session.id)} > {session.title || 'Untitled'} - {relativeTime(session.last_active_at)} + + {relativeTime(session.last_active_at)} + {#if session.pending_approvals} + {session.pending_approvals} + {/if} + + {:else if liveStatus === 'awaiting_input'} + Waiting for your answer + {:else if liveStatus} + Status: {statusLabel(liveStatus)} + {:else} + Session ended + {/if} + {#if $currentTask?.goal} + · {$currentTask.goal.slice(0, 60)}{$currentTask.goal.length > 60 ? '…' : ''} + {/if} + + {/if} + {#each $messages as msg, i (msg.id)}
{#if msg.role === 'user'} @@ -161,6 +195,23 @@
+ {#if $connectionState === 'disconnected'} +
+
+
+
+ {:else if $connectionState === 'reconnecting'} +
+
+
+
+ {/if} + {#if $error}
@@ -169,6 +220,18 @@
{/if} + {#each $chatErrors as err (err.id)} +
+
+ {err.message} + {#if err.action} + + {/if} + +
+
+ {/each} +