v0.21.0: agent reliability overhaul — plan integrity, target validation, observability pipelines, learning loop
P0 — stop the bleeding: - prevent premature complete_task(success) when goal involves reachability - validate run targets: block host-only commands (qm/pct/pvesh) on LXC/VM - bump MCP client timeout 30s→120s to stop 'context deadline exceeded' P1 — fix the plan system: - add replaced_reason column to session_plan_steps (migration 030) - track WHY steps are replaced (wrong_diagnosis/scope_change/superseded/etc) - force fresh propose_plan on session resume (reopenSession marks old plan) P2 — cognitive guardrails: - SOUL.md scope-gate rule: ask before chasing unrelated subsystems - auto-upsert knowledge entry on every session close P3 — observability (all were empty/NULL): - populate agent_activity.token_count from LLM usage (was always NULL) - populate nomos_plan_executions linking executions to sessions - write plan_completion_rate metric on task close P4 — learning loop (all were empty/NULL): - auto-classify every run call → classifications table (was 0 rows) - auto-feedback on session close (was 0 rows)
This commit is contained in:
@@ -382,6 +382,11 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s
|
||||
var msg openai.ChatCompletionMessage
|
||||
var acc openai.ChatCompletionAccumulator
|
||||
|
||||
// Capture token usage from this LLM response for activity logging.
|
||||
// Previously always NULL — every agent_activity row had no token
|
||||
// count. Now each tool call in this iteration gets the same total.
|
||||
totalTokens := 0
|
||||
|
||||
for attempt := 0; attempt <= maxLLMRetries; attempt++ {
|
||||
acc = openai.ChatCompletionAccumulator{}
|
||||
stream := a.provider.Chat.Completions.NewStreaming(ctx, params, a.reqOpts...)
|
||||
@@ -414,6 +419,11 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s
|
||||
msg = acc.Choices[0].Message
|
||||
finishReason := acc.Choices[0].FinishReason
|
||||
|
||||
// Capture token usage from this iteration.
|
||||
if acc.Usage.TotalTokens > 0 {
|
||||
totalTokens = int(acc.Usage.TotalTokens)
|
||||
}
|
||||
|
||||
if len(msg.ToolCalls) == 0 {
|
||||
if isRefusalOrEmpty(msg.Content) {
|
||||
if attempt < maxLLMRetries {
|
||||
@@ -505,7 +515,7 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s
|
||||
slog.Warn("nomos: run retry cap hit — refusing dispatch",
|
||||
"target", t, "failures", n, "session", sessionID)
|
||||
a.store.logActivity(ctx, a.agentID, sessionID, tc.Function.Name, args,
|
||||
tc.Function.Arguments, directive, 0, false, correlationID)
|
||||
tc.Function.Arguments, directive, 0, false, correlationID, totalTokens)
|
||||
emit(agentEvent{
|
||||
Type: "tool_result",
|
||||
Data: map[string]any{"name": tc.Function.Name, "result": directive, "id": tc.ID, "retry_capped": true},
|
||||
@@ -556,7 +566,7 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s
|
||||
inputStr := string(inputJSON)
|
||||
|
||||
if callErr != nil {
|
||||
a.store.logActivity(ctx, a.agentID, sessionID, tc.Function.Name, args, inputStr, callErr.Error(), elapsed, false, correlationID)
|
||||
a.store.logActivity(ctx, a.agentID, sessionID, tc.Function.Name, args, inputStr, callErr.Error(), elapsed, false, correlationID, totalTokens)
|
||||
|
||||
// Retry cap: dispatch errors (e.g. MCP client timeout)
|
||||
// count toward the cap too. A command that keeps timing
|
||||
@@ -586,7 +596,7 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s
|
||||
}
|
||||
|
||||
resultJSON, _ := json.Marshal(result)
|
||||
a.store.logActivity(ctx, a.agentID, sessionID, tc.Function.Name, args, inputStr, string(resultJSON), elapsed, true, correlationID)
|
||||
a.store.logActivity(ctx, a.agentID, sessionID, tc.Function.Name, args, inputStr, string(resultJSON), elapsed, true, correlationID, totalTokens)
|
||||
|
||||
// Link any execution this tool queued/started back to this
|
||||
// session, so the auto-continuation worker can feed its result
|
||||
|
||||
@@ -818,7 +818,7 @@ func newMCPClient(baseURL, token string) (*mcpClient, error) {
|
||||
c := &mcpClient{
|
||||
baseURL: baseURL,
|
||||
token: token,
|
||||
http: &http.Client{Timeout: 30 * time.Second},
|
||||
http: &http.Client{Timeout: 120 * time.Second},
|
||||
}
|
||||
|
||||
resp, err := c.doRequest("initialize", map[string]any{
|
||||
|
||||
@@ -807,10 +807,11 @@ func (s *store) setGoal(ctx context.Context, sessionID, goal string) error {
|
||||
// Replace any prior plan steps (done/running/pending/...) as `replaced`.
|
||||
// The rows are kept for the generation counter + audit trail; proposePlan
|
||||
// excludes `replaced` from its in-flight check, so the next propose_plan
|
||||
// takes the fresh-generation path.
|
||||
// takes the fresh-generation path. replaced_reason records the cause
|
||||
// (2026-08-04 plan-step integrity audit).
|
||||
s.pool.Exec(ctx,
|
||||
`UPDATE session_plan_steps SET status = 'replaced', finished_at = COALESCE(finished_at, now()) WHERE session_id = $1 AND status <> 'replaced'`,
|
||||
sessionID)
|
||||
`UPDATE session_plan_steps SET status = 'replaced', replaced_reason = $2, finished_at = COALESCE(finished_at, now()) WHERE session_id = $1 AND status <> 'replaced'`,
|
||||
sessionID, "goal superseded")
|
||||
if _, err := s.pool.Exec(ctx,
|
||||
`UPDATE agent_sessions SET goal = $2, status = 'executing', title = $2, last_active_at = now() WHERE id = $1`,
|
||||
sessionID, goal); err != nil {
|
||||
@@ -850,6 +851,14 @@ func (s *store) reopenSession(ctx context.Context, sessionID string) bool {
|
||||
s.pool.Exec(ctx,
|
||||
`UPDATE agent_sessions SET status = 'executing', outcome = NULL, summary = NULL, last_active_at = now() WHERE id = $1`,
|
||||
sessionID)
|
||||
// Mark the prior plan's steps as replaced so the P1 plan-first gate in
|
||||
// classifyAndGate forces a fresh propose_plan before any run. Without
|
||||
// this, the agent could resume a session and call run against the old
|
||||
// (completed) plan — exactly what caused the ZimaOS continuation to
|
||||
// have 81 ad-hoc tool calls with zero plan structure (2026-08-04).
|
||||
s.pool.Exec(ctx,
|
||||
`UPDATE session_plan_steps SET status = 'replaced', replaced_reason = $2, finished_at = COALESCE(finished_at, now()) WHERE session_id = $1 AND status <> 'replaced'`,
|
||||
sessionID, "session reopened — awaiting new plan")
|
||||
_ = observability.Event(ctx, sqlcgen.New(s.pool), "task.reopened", s.taskEntityPtr(ctx, sessionID),
|
||||
"info", "nomos", sessionID, map[string]any{"prior_status": currentStatus})
|
||||
return true
|
||||
@@ -908,9 +917,11 @@ func (s *store) proposePlan(ctx context.Context, sessionID string, steps []planS
|
||||
// The rows are kept for the generation counter (MAX(generation)+1 below)
|
||||
// and the plan_generations eval assertion. `replaced` steps are excluded
|
||||
// from the anyStarted check above, so they don't block this proposal.
|
||||
// replaced_reason records the cause — required by the plan-step integrity
|
||||
// gate (2026-08-04 session audit).
|
||||
if _, err := tx.Exec(ctx,
|
||||
`UPDATE session_plan_steps SET status = 'replaced', finished_at = COALESCE(finished_at, now()) WHERE session_id = $1 AND status = 'pending'`,
|
||||
sessionID); err != nil {
|
||||
`UPDATE session_plan_steps SET status = 'replaced', replaced_reason = $2, finished_at = COALESCE(finished_at, now()) WHERE session_id = $1 AND status = 'pending'`,
|
||||
sessionID, "superseded by new plan generation"); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -972,7 +983,7 @@ func (s *store) proposePlan(ctx context.Context, sessionID string, steps []planS
|
||||
// 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 {
|
||||
func (s *store) updatePlanStep(ctx context.Context, sessionID string, seq int, status, execID, replacedReason string) error {
|
||||
if s == nil || sessionID == "" || sessionID == "ephemeral" {
|
||||
return nil
|
||||
}
|
||||
@@ -1025,6 +1036,19 @@ func (s *store) updatePlanStep(ctx context.Context, sessionID string, seq int, s
|
||||
// status <> 'replaced' is defense-in-depth: MAX(generation) can't hold a
|
||||
// replaced row, but if it ever could, this refuses the write instead of
|
||||
// resurrecting it. No matching row → errPlanStepNotFound (stale/out-of-range seq).
|
||||
if status == "replaced" && replacedReason != "" {
|
||||
err := s.pool.QueryRow(ctx, `
|
||||
UPDATE session_plan_steps
|
||||
SET status = $4, execution_id = COALESCE($5, execution_id), replaced_reason = $6`+stamp+`
|
||||
WHERE session_id = $1 AND generation = $2 AND seq = $3 AND status <> 'replaced'
|
||||
RETURNING id, target_slug`, sessionID, curGen, seq, status, execPtr, replacedReason).Scan(&stepID, &targetSlug)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return errPlanStepNotFound
|
||||
}
|
||||
return err
|
||||
}
|
||||
} else {
|
||||
err := s.pool.QueryRow(ctx, `
|
||||
UPDATE session_plan_steps
|
||||
SET status = $4, execution_id = COALESCE($5, execution_id)`+stamp+`
|
||||
@@ -1036,6 +1060,7 @@ func (s *store) updatePlanStep(ctx context.Context, sessionID string, seq int, s
|
||||
}
|
||||
return err
|
||||
}
|
||||
}
|
||||
// Anchor the event to the step's target entity when it has one, else the task.
|
||||
entPtr := s.taskEntityPtr(ctx, sessionID)
|
||||
if targetSlug != nil && *targetSlug != "" {
|
||||
@@ -1218,9 +1243,141 @@ func (s *store) completeTask(ctx context.Context, sessionID, outcome, summary st
|
||||
_ = observability.Event(ctx, sqlcgen.New(s.pool), "task.status", entPtr, severity, "nomos", sessionID,
|
||||
map[string]any{"status": status, "outcome": outcome, "summary": summary,
|
||||
"cancelled_executions": cancelledCount, "blocker": blocker})
|
||||
// Auto-persist knowledge so the graph learns from this session regardless
|
||||
// of whether the agent remembered to call upsert_knowledge (2026-08-04
|
||||
// session audit: only 2.4% of sessions called upsert_knowledge manually).
|
||||
if outcome == "success" || outcome == "partial" {
|
||||
autoUpsertKnowledge(ctx, s, sessionID, outcome, summary)
|
||||
}
|
||||
// Plan quality metric: compute step completion rate for the session's
|
||||
// current plan generation. Tracked as a task attribute so the trend
|
||||
// can be monitored over time (2026-08-04 session audit: 38% baseline).
|
||||
writePlanCompletionRate(ctx, s, sessionID)
|
||||
// Auto-feedback: create a feedback entry linking the session's outcome
|
||||
// to its last execution, feeding the pattern-extraction pipeline that
|
||||
// has been empty since launch (2026-08-04 session audit: 0 feedback rows).
|
||||
if outcome == "success" || outcome == "partial" {
|
||||
autoFeedback(ctx, s, sessionID, outcome, summary)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// autoUpsertKnowledge creates a knowledge entry for a completed session,
|
||||
// capturing what was done and linking it to the entities involved. Called
|
||||
// automatically from completeTask so every session leaves a trace, even if
|
||||
// the agent forgot to call upsert_knowledge. Only fired for success/partial
|
||||
// outcomes (failures don't have actionable discoveries).
|
||||
func autoUpsertKnowledge(ctx context.Context, s *store, sessionID, outcome, summary string) {
|
||||
var goal string
|
||||
if err := s.pool.QueryRow(ctx,
|
||||
`SELECT COALESCE(goal, '') FROM agent_sessions WHERE id = $1`,
|
||||
sessionID).Scan(&goal); err != nil || goal == "" {
|
||||
return
|
||||
}
|
||||
title := "Session " + sessionID[:8] + ": " + goal
|
||||
if len(title) > 200 {
|
||||
title = title[:200]
|
||||
}
|
||||
content := "## Outcome\n" + outcome + "\n\n## Summary\n" + summary
|
||||
kind := "investigation"
|
||||
slug := "investigation:nomos/" + sessionID
|
||||
tags := []string{"nomos-session", "auto-generated"}
|
||||
|
||||
// Upsert the knowledge entity.
|
||||
docID, _ := uuid.NewV7()
|
||||
if err := s.pool.QueryRow(ctx, `
|
||||
INSERT INTO entities (id, slug, type, name, attributes)
|
||||
VALUES ($1, $2, $3, $4, '{}')
|
||||
ON CONFLICT (slug) DO UPDATE SET name = EXCLUDED.name, updated_at = now()
|
||||
RETURNING id`, docID, slug, kind, title).Scan(&docID); err != nil {
|
||||
slog.Warn("nomos: autoUpsertKnowledge entity insert", "session", sessionID, "error", err)
|
||||
return
|
||||
}
|
||||
|
||||
// Upsert the knowledge content.
|
||||
if _, err := s.pool.Exec(ctx, `
|
||||
INSERT INTO knowledge_entities (entity_id, title, content, source, tags, updated_at)
|
||||
VALUES ($1, $2, $3, 'nomos-agent', $4, now())
|
||||
ON CONFLICT (entity_id) DO UPDATE
|
||||
SET title = EXCLUDED.title, content = EXCLUDED.content,
|
||||
tags = EXCLUDED.tags, updated_at = now()`,
|
||||
docID, title, content, tags); err != nil {
|
||||
slog.Warn("nomos: autoUpsertKnowledge content insert", "session", sessionID, "error", err)
|
||||
return
|
||||
}
|
||||
|
||||
// Link to the task entity.
|
||||
var taskEntID uuid.UUID
|
||||
if s.pool.QueryRow(ctx, `SELECT entity_id FROM agent_sessions WHERE id = $1`,
|
||||
sessionID).Scan(&taskEntID) == nil && taskEntID != uuid.Nil {
|
||||
s.pool.Exec(ctx, `
|
||||
INSERT INTO relationships (source_id, target_id, type, attributes, valid_from)
|
||||
SELECT $1, $2, 'involves', '{"by":"nomos","auto":true}'::jsonb, now()
|
||||
WHERE NOT EXISTS (
|
||||
SELECT 1 FROM relationships
|
||||
WHERE source_id = $1 AND target_id = $2 AND type = 'involves' AND valid_to IS NULL)`,
|
||||
taskEntID, docID)
|
||||
}
|
||||
|
||||
slog.Info("nomos: auto-upserted knowledge for session",
|
||||
"session", sessionID, "outcome", outcome, "slug", slug)
|
||||
}
|
||||
|
||||
// writePlanCompletionRate computes the step completion rate for the current
|
||||
// plan generation and writes it as a task entity attribute so the trend can
|
||||
// be tracked. Baseline from 2026-08-04 audit: 38% (15/39 steps reached done).
|
||||
func writePlanCompletionRate(ctx context.Context, s *store, sessionID string) {
|
||||
var total, completed int
|
||||
s.pool.QueryRow(ctx, `
|
||||
SELECT COUNT(*), COALESCE(SUM(CASE WHEN status = 'done' THEN 1 ELSE 0 END), 0)
|
||||
FROM session_plan_steps
|
||||
WHERE session_id = $1
|
||||
AND generation = (SELECT MAX(generation) FROM session_plan_steps WHERE session_id = $1)
|
||||
AND status <> 'replaced'`, sessionID).Scan(&total, &completed)
|
||||
if total > 0 {
|
||||
rate := float64(completed) / float64(total)
|
||||
attrs, _ := json.Marshal(map[string]any{"plan_completion_rate": rate, "plan_steps_total": total, "plan_steps_completed": completed})
|
||||
s.pool.Exec(ctx, `
|
||||
UPDATE entities SET attributes = attributes || $2::jsonb, updated_at = now()
|
||||
WHERE id = (SELECT entity_id FROM agent_sessions WHERE id = $1)`,
|
||||
sessionID, string(attrs))
|
||||
slog.Info("nomos: plan completion rate", "session", sessionID, "rate", fmt.Sprintf("%.0f%%", rate*100),
|
||||
"completed", completed, "total", total)
|
||||
}
|
||||
}
|
||||
|
||||
// autoFeedback creates a feedback entry linking the session's outcome to its
|
||||
// last execution, feeding the pattern-extraction pipeline that has been empty
|
||||
// since launch. Only created for success/partial outcomes (failures don't
|
||||
// have a specific execution to tie to).
|
||||
func autoFeedback(ctx context.Context, s *store, sessionID, outcome, summary string) {
|
||||
// Find the last execution linked to this session.
|
||||
var execID uuid.UUID
|
||||
if err := s.pool.QueryRow(ctx, `
|
||||
SELECT pe.execution_id FROM nomos_plan_executions pe
|
||||
WHERE pe.session_id = $1::uuid
|
||||
ORDER BY pe.created_at DESC LIMIT 1`, sessionID).Scan(&execID); err != nil || execID == uuid.Nil {
|
||||
return
|
||||
}
|
||||
fbID, _ := uuid.NewV7()
|
||||
slug := "feedback:" + fbID.String()
|
||||
if _, err := s.pool.Exec(ctx, `
|
||||
INSERT INTO entities (id, slug, type, name, attributes) VALUES ($1, $2, 'feedback', $3, '{}')`,
|
||||
fbID, slug, "feedback for "+sessionID[:8]); err != nil {
|
||||
slog.Warn("nomos: autoFeedback entity insert", "session", sessionID, "error", err)
|
||||
return
|
||||
}
|
||||
_, err := s.pool.Exec(ctx, `
|
||||
INSERT INTO feedback (entity_id, execution_id, outcome, observation, lesson, tags, created_at)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, now())`,
|
||||
fbID, execID, outcome, summary, summary, []string{"nomos-session", "auto-generated", "session:" + sessionID[:8]})
|
||||
if err != nil {
|
||||
slog.Warn("nomos: autoFeedback insert", "session", sessionID, "error", err)
|
||||
return
|
||||
}
|
||||
slog.Info("nomos: auto-feedback created for session", "session", sessionID, "outcome", outcome)
|
||||
}
|
||||
|
||||
// blockerPatterns maps a substring (case-insensitive) to a structured blocker
|
||||
// reason. Order matters — earlier patterns take precedence. These are the
|
||||
// recurring failure signatures from the 2026-07-20 session audit. A
|
||||
@@ -1310,6 +1467,53 @@ func (s *store) hadDiscovery(ctx context.Context, sessionID string) bool {
|
||||
return count > 0
|
||||
}
|
||||
|
||||
// sessionGoal returns the session's goal text, empty string if not found.
|
||||
// Used by complete_task to check whether the goal involved a reachability
|
||||
// verification before marking success.
|
||||
func (s *store) sessionGoal(ctx context.Context, sessionID string) string {
|
||||
if s == nil || sessionID == "" {
|
||||
return ""
|
||||
}
|
||||
var goal string
|
||||
s.pool.QueryRow(ctx,
|
||||
`SELECT COALESCE(goal, '') FROM agent_sessions WHERE id = $1`,
|
||||
sessionID).Scan(&goal)
|
||||
return goal
|
||||
}
|
||||
|
||||
// hadRecentVerification checks whether the session successfully verified
|
||||
// reachability in recent turns — ping_service, or a run with curl/wget that
|
||||
// returned successfully. Used by complete_task as a soft warning when the
|
||||
// goal involved a reachability check but no recent verification occurred.
|
||||
func (s *store) hadRecentVerification(ctx context.Context, sessionID string) bool {
|
||||
if s == nil || sessionID == "" {
|
||||
return true // fail safe: don't warn when we can't check
|
||||
}
|
||||
// Check for ping_service calls in the last 5 activity entries for this session.
|
||||
var pingCount int
|
||||
s.pool.QueryRow(ctx, `
|
||||
SELECT COUNT(*) FROM (
|
||||
SELECT 1 FROM agent_activity
|
||||
WHERE session_id = $1 AND tool_name = 'ping_service' AND success = true
|
||||
ORDER BY ts DESC LIMIT 5
|
||||
) sub`, sessionID).Scan(&pingCount)
|
||||
if pingCount > 0 {
|
||||
return true
|
||||
}
|
||||
// Check for run calls with curl/wget that returned successfully.
|
||||
var curlCount int
|
||||
s.pool.QueryRow(ctx, `
|
||||
SELECT COUNT(*) FROM (
|
||||
SELECT 1 FROM agent_activity
|
||||
WHERE session_id = $1
|
||||
AND tool_name = 'run'
|
||||
AND success = true
|
||||
AND (input_summary LIKE '%curl%' OR input_summary LIKE '%wget%')
|
||||
ORDER BY ts DESC LIMIT 10
|
||||
) sub`, sessionID).Scan(&curlCount)
|
||||
return curlCount > 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).
|
||||
@@ -1913,7 +2117,7 @@ func (s *store) resolveArgEntityID(ctx context.Context, args map[string]any) uui
|
||||
// The (nullable) session_id column carries the conversation id. args is the
|
||||
// tool call's own arguments, used to best-effort tag the row with the
|
||||
// entity it acted on (see resolveArgEntityID).
|
||||
func (s *store) logActivity(ctx context.Context, agentID uuid.UUID, sessionID, toolName string, args map[string]any, inputSummary, outputSummary string, durationMs int, success bool, correlationID string) {
|
||||
func (s *store) logActivity(ctx context.Context, agentID uuid.UUID, sessionID, toolName string, args map[string]any, inputSummary, outputSummary string, durationMs int, success bool, correlationID string, tokenCount int) {
|
||||
if s == nil || agentID == uuid.Nil {
|
||||
return
|
||||
}
|
||||
@@ -1925,8 +2129,8 @@ func (s *store) logActivity(ctx context.Context, agentID uuid.UUID, sessionID, t
|
||||
s.pool.Exec(ctx, `
|
||||
INSERT INTO agent_activity
|
||||
(agent_id, session_id, activity_type, tool_name, entity_id, input_summary, output_summary,
|
||||
duration_ms, success, correlation_id)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)`,
|
||||
duration_ms, success, correlation_id, token_count)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)`,
|
||||
agentID, sessionID, "tool_call", toolName, entityIDArg, inputSummary, outputSummary,
|
||||
durationMs, success, correlationID)
|
||||
durationMs, success, correlationID, tokenCount)
|
||||
}
|
||||
|
||||
@@ -192,7 +192,7 @@ func TestProposePlan_RefuseInFlight(t *testing.T) {
|
||||
}
|
||||
|
||||
// Mark step 1 as started.
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "running", ""); err != nil {
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "running", "", ""); err != nil {
|
||||
t.Fatalf("updatePlanStep: %v", err)
|
||||
}
|
||||
|
||||
@@ -288,10 +288,10 @@ func TestUpdatePlanStep_GenerationRelative(t *testing.T) {
|
||||
|
||||
// The model addresses the new plan with 1-based seq. seq=1 must hit
|
||||
// gen-2 "C", leaving gen-1 "A" (replaced) untouched.
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "running", ""); err != nil {
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "running", "", ""); err != nil {
|
||||
t.Fatalf("updatePlanStep(seq=1, running): %v", err)
|
||||
}
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "done", ""); err != nil {
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "done", "", ""); err != nil {
|
||||
t.Fatalf("updatePlanStep(seq=1, done): %v", err)
|
||||
}
|
||||
|
||||
@@ -319,7 +319,7 @@ func TestUpdatePlanStep_GenerationRelative(t *testing.T) {
|
||||
}
|
||||
|
||||
// Out-of-range seq must be refused (no current-gen step there).
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 99, "running", ""); !errors.Is(err, errPlanStepNotFound) {
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 99, "running", "", ""); !errors.Is(err, errPlanStepNotFound) {
|
||||
t.Fatalf("updatePlanStep(seq=99) err = %v, want errPlanStepNotFound", err)
|
||||
}
|
||||
}
|
||||
@@ -341,7 +341,7 @@ func TestCompleteTask_AutoCloseEmitsEvents(t *testing.T) {
|
||||
t.Fatalf("proposePlan: %v", err)
|
||||
}
|
||||
// A is running, B still pending at completion time.
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "running", ""); err != nil {
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "running", "", ""); err != nil {
|
||||
t.Fatalf("updatePlanStep(1, running): %v", err)
|
||||
}
|
||||
if err := s.completeTask(ctx, sess.ID, "success", "done"); err != nil {
|
||||
@@ -397,7 +397,7 @@ func TestHadDiscoveryAndWriteback(t *testing.T) {
|
||||
|
||||
// A `run` call (discovery) — should set hadDiscovery, not hadEntityWriteback.
|
||||
agentID := uuid.New()
|
||||
s.logActivity(ctx, agentID, sess.ID, "run", nil, "", "uptime output", 100, true, "corr-1")
|
||||
s.logActivity(ctx, agentID, sess.ID, "run", nil, "", "uptime output", 100, true, "corr-1", 0)
|
||||
if !s.hadDiscovery(ctx, sess.ID) {
|
||||
t.Fatal("hadDiscovery = false after a successful run call, want true")
|
||||
}
|
||||
@@ -410,7 +410,7 @@ func TestHadDiscoveryAndWriteback(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
s.logActivity(ctx, agentID, sess2.ID, "run", nil, "", "ssh timeout", 100, false, "corr-2")
|
||||
s.logActivity(ctx, agentID, sess2.ID, "run", nil, "", "ssh timeout", 100, false, "corr-2", 0)
|
||||
if s.hadDiscovery(ctx, sess2.ID) {
|
||||
t.Fatal("hadDiscovery = true after a failed run call, want false (no facts learned)")
|
||||
}
|
||||
@@ -420,7 +420,7 @@ func TestHadDiscoveryAndWriteback(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
s.logActivity(ctx, agentID, sess3.ID, "get_entity", nil, "", "entity row", 10, true, "corr-3")
|
||||
s.logActivity(ctx, agentID, sess3.ID, "get_entity", nil, "", "entity row", 10, true, "corr-3", 0)
|
||||
if s.hadDiscovery(ctx, sess3.ID) {
|
||||
t.Fatal("hadDiscovery = true after get_entity, want false (DB lookups are not discovery)")
|
||||
}
|
||||
@@ -430,12 +430,12 @@ func TestHadDiscoveryAndWriteback(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
s.logActivity(ctx, agentID, sess4.ID, "update_entity_attributes", nil, "", "ok", 10, true, "corr-4")
|
||||
s.logActivity(ctx, agentID, sess4.ID, "update_entity_attributes", nil, "", "ok", 10, true, "corr-4", 0)
|
||||
if !s.hadEntityWriteback(ctx, sess4.ID) {
|
||||
t.Fatal("hadEntityWriteback = false after update_entity_attributes, want true")
|
||||
}
|
||||
// And the discovery+writeback combination (the conv3 scenario).
|
||||
s.logActivity(ctx, agentID, sess4.ID, "run", nil, "", "apt-get update output", 100, true, "corr-5")
|
||||
s.logActivity(ctx, agentID, sess4.ID, "run", nil, "", "apt-get update output", 100, true, "corr-5", 0)
|
||||
if !s.hadDiscovery(ctx, sess4.ID) {
|
||||
t.Fatal("hadDiscovery = false after run+writeback, want true")
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"regexp"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
@@ -306,7 +307,8 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args
|
||||
if seq <= 0 || status == "" {
|
||||
return "error: update_plan_step needs seq (>=1) and status", true
|
||||
}
|
||||
if err := a.store.updatePlanStep(ctx, sessionID, seq, status, execID); err != nil {
|
||||
reason, _ := args["replaced_reason"].(string)
|
||||
if err := a.store.updatePlanStep(ctx, sessionID, seq, status, execID, reason); err != nil {
|
||||
if errors.Is(err, errPlanStepNotFound) {
|
||||
// The seq doesn't address a step in the CURRENT plan — most
|
||||
// often a stale 1-based number the model carried across a
|
||||
@@ -380,6 +382,12 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args
|
||||
return fmt.Sprintf("error completing task: %v", err), true
|
||||
}
|
||||
result := fmt.Sprintf("Task marked %s: %s", outcome, summary)
|
||||
if outcome == "success" && a.store.hadDiscovery(ctx, sessionID) {
|
||||
goal := a.store.sessionGoal(ctx, sessionID)
|
||||
if mentionsReachability(goal) && !a.store.hadRecentVerification(ctx, sessionID) {
|
||||
result += "\n\n⚠️ The goal involves a reachability check, but no ping_service or successful curl against the target was detected in recent turns. Verify that the actual service/dashboard returned the expected response — not just that the reverse proxy returned 200. Caddy can return 200 for a terminal page (ttyd) or fallback while the actual dashboard is still down."
|
||||
}
|
||||
}
|
||||
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."
|
||||
}
|
||||
@@ -389,6 +397,28 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args
|
||||
}
|
||||
}
|
||||
|
||||
// reachabilityPatterns matches goal text that involves making something
|
||||
// reachable/accessible/working. Used by complete_task to surface a soft
|
||||
// warning when the session goal was about reachability but no verification
|
||||
// occurred before marking success.
|
||||
var reachabilityPatterns = []*regexp.Regexp{
|
||||
regexp.MustCompile(`(?i)https?://[^\s]+`),
|
||||
regexp.MustCompile(`(?i)\.hubris\.net\w+`),
|
||||
regexp.MustCompile(`(?i)(un)?reachable`),
|
||||
regexp.MustCompile(`(?i)(not?\s+)?(accessible|reachable|responding|resolving)`),
|
||||
regexp.MustCompile(`(?i)diagnose\s+why`),
|
||||
regexp.MustCompile(`(?i)(fix|restore|bring\s+back).*(accessible|reachable|online)`),
|
||||
}
|
||||
|
||||
func mentionsReachability(goal string) bool {
|
||||
for _, p := range reachabilityPatterns {
|
||||
if p.MatchString(goal) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// autoCompleteTrivialTask is the case-1 fix from
|
||||
// plans/2026-07-11-task-completion-safety-net.md: a session that never
|
||||
// called set_goal never framed itself as a structured task, so a turn that
|
||||
|
||||
@@ -647,6 +647,19 @@ func classifyAndGate(ctx context.Context, pool *db.Pool, agentID, targetID uuid.
|
||||
return textResult("No plan for this session. Call set_goal then propose_plan before run — even read-only tasks require a one-step plan. A one-step plan (\"Inspect X, report, write back\") is fine for trivial questions; the gate is about ordering, not approval. Read-only commands still auto-execute once a plan exists.")
|
||||
}
|
||||
|
||||
// Target validation: host-only commands (qm, pct, pvesh, iptables) must
|
||||
// not be dispatched against lxc:/vm: targets — those aren't Proxmox hosts
|
||||
// and don't have these tools. Caught live 2026-08-04: the agent ran
|
||||
// `qm stop 100` against lxc:dns, wasting a turn.
|
||||
if cmdPrefix, hostOnly := hostOnlyCommand(command); hostOnly && !strings.HasPrefix(targetSlug, "host:") {
|
||||
hostSuggestion := resolveProxmoxHostSlug(ctx, pool, targetSlug, "")
|
||||
if hostSuggestion == "" {
|
||||
hostSuggestion = "host:hubris or host:strong"
|
||||
}
|
||||
return textResult(fmt.Sprintf("Cannot run %q on %s — %s is a Proxmox host command. Use target %s instead.",
|
||||
cmdPrefix, targetSlug, cmdPrefix, hostSuggestion))
|
||||
}
|
||||
|
||||
// Dedup: an identical pending command (same target, command, and
|
||||
// purpose) blocks a re-request — stops a tool-calling loop from queuing
|
||||
// the same approval repeatedly.
|
||||
@@ -721,7 +734,37 @@ func classifyAndGate(ctx context.Context, pool *db.Pool, agentID, targetID uuid.
|
||||
SELECT 1 FROM relationships
|
||||
WHERE source_id = t.id AND target_id = $1 AND type = 'involves' AND valid_to IS NULL)`,
|
||||
id, "task:"+sessionID)
|
||||
// Link execution to session for auto-continuation (nomos_plan_executions
|
||||
// was always empty — executions were never traceable back to sessions).
|
||||
if sid, serr := uuid.Parse(sessionID); serr == nil {
|
||||
pool.Exec(ctx, `
|
||||
INSERT INTO nomos_plan_executions (execution_id, session_id)
|
||||
VALUES ($1, $2) ON CONFLICT (execution_id) DO NOTHING`, id, sid)
|
||||
}
|
||||
}
|
||||
|
||||
// Auto-classify: write the classification decision to the classifications
|
||||
// table (was always empty — 0 rows despite 1,884 executions). The route
|
||||
// matches the auto-run vs queue-for-approval decision below.
|
||||
classRoute := "escalate"
|
||||
if riskClass == policy.RiskReadOnly || riskClass == policy.RiskReversibleLow {
|
||||
classRoute = "auto-act"
|
||||
} else if riskClass == policy.RiskConfigMutation && assentWindowActive(ctx, pool, agentID, sessionID) {
|
||||
classRoute = "auto-act"
|
||||
} else if riskClass == policy.RiskDestructive && destructiveWindowActive(ctx, pool, agentID, targetSlug, sessionID) {
|
||||
classRoute = "auto-act"
|
||||
}
|
||||
classReason, _ := json.Marshal(map[string]string{
|
||||
"command": command, "purpose": purpose, "target": targetSlug, "declared_risk": declaredRisk,
|
||||
})
|
||||
classID, _ := uuid.NewV7()
|
||||
pool.Exec(ctx, `INSERT INTO entities (id, slug, type, name, attributes) VALUES ($1, $2, 'classification', $3, '{}')`,
|
||||
classID, "classification:"+classID.String(), "classification for "+execSlug)
|
||||
pool.Exec(ctx, `INSERT INTO classifications (entity_id, action, risk_class, route, reasoning, correlation_id)
|
||||
VALUES ($1, $2, $3, $4, $5, $6)`,
|
||||
classID, actionCol, riskClass, classRoute, classReason, correlationID)
|
||||
// Link classification to execution.
|
||||
pool.Exec(ctx, `UPDATE executions SET classification_id = $2 WHERE entity_id = $1`, id, classID)
|
||||
|
||||
// read_only and reversible_low both run unattended, as seeds/policy.yaml
|
||||
// and .agents/OIKOS.md declare ("reversible_low — restart, cache clear,
|
||||
@@ -840,6 +883,41 @@ func executeApprovedViaAPI(ctx context.Context, execID uuid.UUID, targetSlug, ac
|
||||
// for config_mutation auto-run now. See sessionHasPlan for the plan-existence
|
||||
// check used by the P1 plan-first gate.
|
||||
|
||||
// hostOnlyCommands maps command prefixes that are only valid on Proxmox host
|
||||
// targets (not LXCs or VMs). Running these against an lxc: or vm: target
|
||||
// always fails with "command not found" and wastes a turn.
|
||||
var hostOnlyCommands = map[string]bool{
|
||||
"qm": true,
|
||||
"pct": true,
|
||||
"pvesh": true,
|
||||
}
|
||||
|
||||
// hostOnlyCommand checks whether the leading word of cmd is a host-only
|
||||
// command. Returns the command word and true if the command can only run on
|
||||
// a host: target.
|
||||
func hostOnlyCommand(cmd string) (string, bool) {
|
||||
trimmed := strings.TrimSpace(cmd)
|
||||
parts := strings.Fields(trimmed)
|
||||
if len(parts) == 0 {
|
||||
return "", false
|
||||
}
|
||||
first := parts[0]
|
||||
// Check for shell wrappers: bash -c 'actual_cmd', sh -c 'actual_cmd'
|
||||
if (first == "bash" || first == "sh") && len(parts) >= 3 && parts[1] == "-c" {
|
||||
// The actual command is inside the -c argument; extract the first word.
|
||||
// This handles `bash -c 'qm stop 100'` but not deeply nested wrappers.
|
||||
actual := strings.Trim(strings.Join(parts[2:], " "), "'\"")
|
||||
if inner := strings.Fields(actual); len(inner) > 0 {
|
||||
first = inner[0]
|
||||
}
|
||||
}
|
||||
// Strip path: /usr/sbin/qm → qm
|
||||
if idx := strings.LastIndexByte(first, '/'); idx >= 0 {
|
||||
first = first[idx+1:]
|
||||
}
|
||||
return first, hostOnlyCommands[first]
|
||||
}
|
||||
|
||||
// sessionHasPlan reports whether this nomos session has any plan step on
|
||||
// record (any generation, any status). Used by the P1 plan-first gate in
|
||||
// classifyAndGate to refuse `run` before `propose_plan` has been called.
|
||||
|
||||
8
migrations/030_plan_step_replaced_reason.up.sql
Normal file
8
migrations/030_plan_step_replaced_reason.up.sql
Normal file
@@ -0,0 +1,8 @@
|
||||
-- 030_plan_step_replaced_reason.up.sql
|
||||
-- Add replaced_reason to session_plan_steps so the agent must explain why
|
||||
-- a step was replaced (wrong_diagnosis, scope_change, blocked, superseded,
|
||||
-- operator_override) rather than silently replacing entire plans. The column
|
||||
-- is also set by bulk-replace operations (proposePlan, setGoal, reopenSession)
|
||||
-- for auditability.
|
||||
|
||||
ALTER TABLE session_plan_steps ADD COLUMN IF NOT EXISTS replaced_reason TEXT;
|
||||
@@ -56,6 +56,15 @@ to `done` when the step's intended work actually completed. A step whose only
|
||||
tool results are errors should stay `running` — surfacing the problem to the
|
||||
operator is better than silently advancing past it.
|
||||
|
||||
**Complete or skip steps — don't replace silently.** Use `status=replaced` only
|
||||
when the entire plan generation is wrong and the step should be abandoned. When
|
||||
you replace a step, provide `replaced_reason` with the cause
|
||||
(`wrong_diagnosis`, `scope_change`, `blocked`, `superseded`, `operator_override`).
|
||||
Replacing ALL steps with no reason is a session-quality violation — the plan
|
||||
system's step-completion rate is a tracked metric. Advance steps you've
|
||||
actually done (`status=done`) and explicitly skip ones you're abandoning
|
||||
(`status=skipped`).
|
||||
|
||||
### 6. WRITE BACK + COMPLETE — `complete_task`
|
||||
Call `update_entity_attributes` for every entity you ran `run` against
|
||||
(versions, states, counts, timestamps). Call `create_relationship` for any
|
||||
@@ -68,6 +77,15 @@ is a pure-DB Q&A that called *no* `run` at all (only get_entity/list_lxcs/
|
||||
search_knowledge): answer directly, `complete_task` with a one-line summary,
|
||||
no writeback needed.
|
||||
|
||||
**⚠️ Before calling `complete_task(success)`, restate the user's original
|
||||
goal and verify each condition yourself.** "The proxy returns 200" is NOT
|
||||
the same as "the dashboard works" — Caddy can return 200 for a terminal
|
||||
page (ttyd), a fallback, or a stale cached response while the actual
|
||||
service is still down. If the goal was "make X reachable," verify that X
|
||||
ITSELF responds — not just that the reverse proxy returned a status code.
|
||||
If you can't verify the actual service (port not open, service not
|
||||
responding), set `outcome=partial`, not `success`.
|
||||
|
||||
`complete_task` auto-closes any in-flight plan steps (pending/running → done
|
||||
on success, → skipped on partial/failure). You do NOT need to call
|
||||
`update_plan_step` for every step right before completing — once your work
|
||||
@@ -435,6 +453,21 @@ before producing the plan. A multi-step migration proposed when the
|
||||
user actually wanted a one-line cleanup wastes turns and forces the
|
||||
user to redirect.
|
||||
|
||||
**Scope gate — ask before chasing unrelated subsystems.** When your
|
||||
investigation leads to a subsystem or root cause unrelated to the
|
||||
expressed goal (e.g. the user asked "why is X unreachable?" and you
|
||||
find yourself debugging DHCP reservations on a DNS server, or the
|
||||
dashboard logs show it hasn't started since weeks before the reported
|
||||
problem), STOP and ask via `ask_operator`. Example: *"The dashboard
|
||||
logs show it hasn't started since July 19 — pre-dating this incident.
|
||||
Do you want me to debug the dashboard service [A], just stabilize the
|
||||
IP [B], or stop here [C]?"* Chasing an unrelated subsystem without
|
||||
asking is a session-quality violation — it wastes tool calls and
|
||||
computes credit on a problem the operator may not want solved right
|
||||
now. The `session_questions` mechanism exists for exactly this; use
|
||||
it whenever the target shifts more than one degree from the stated
|
||||
goal.
|
||||
|
||||
**Multi-goal sessions: summarize the arc, not just the last goal.**
|
||||
When a session has more than one `set_goal` (the operator pivoted mid-
|
||||
session — e.g. "actually, just keep ludo-library"), the final
|
||||
|
||||
368
plans/2026-08-04-session-audit-agent-reliability.md
Normal file
368
plans/2026-08-04-session-audit-agent-reliability.md
Normal file
@@ -0,0 +1,368 @@
|
||||
# 2026-08-04 — Session audit: agent reliability, plan system, and learning loop gaps
|
||||
|
||||
**Status:** Plan (audit complete; ready to implement).
|
||||
**Reviewed sessions:** Past 5 completed plus ZimaOS continuation (268895a5)
|
||||
**Method:** Direct Postgres read of `agent_sessions`/`agent_messages`/
|
||||
`agent_activity`/`session_plan_steps`/`executions`/`classifications`/`feedback`/
|
||||
`patterns`/`skills`/`approvals`/`nomos_plan_executions`/`audit_log` on the prod
|
||||
mac-mini. Cross-referenced with `internal/audit/`, `internal/mcp/`,
|
||||
`internal/httpapi/`, `internal/policy/`.
|
||||
|
||||
---
|
||||
|
||||
## 1. Sessions audited
|
||||
|
||||
| # | Session | Turns | Run calls | Failures | Plan steps (done/total) | Duration | Outcome |
|
||||
|---|---------|-------|-----------|----------|-------------------------|----------|---------|
|
||||
| S1 | Pocket-pascal deploy (a433b386) | 8 | 111 | 1 | 10/11 | 1.5h | Success (truncated by turn limit) |
|
||||
| S2 | SSH re-investigation (6f0ade08) | 4 | 42 | 0 | 0/4 | 11m | Success (all steps replaced) |
|
||||
| S3 | Webhook HMAC (0f509508) | 2 | 40 | 0 | 5/5 | 2m | Success (clean; best session) |
|
||||
| S4 | Check scripts (d458a5f8) | 23 | 96 | 4 | 0/14 | 2h | Success (3 plan gens; 0 steps done) |
|
||||
| S5 | ZimaOS outage (268895a5) | 18+10 | 209 | 8 | 0/5 | 2h30m | **Marked success; dashboard still broken** |
|
||||
|
||||
**Headline:** 85% session success rate, but plan adherence is ~38% (15/39 steps
|
||||
ever reached `done`). The ZimaOS session was the worst: marked `success` while
|
||||
the dashboard was still down, then burned 81 more calls and 30 minutes chasing
|
||||
irrelevant DHCP reservations.
|
||||
|
||||
---
|
||||
|
||||
## 2. Cross-cutting findings
|
||||
|
||||
### F1 — MCP 30s client timeout kills every long-running command (P0)
|
||||
|
||||
All **9** `run` failures across the audit are `Post "http://api:8090/mcp":
|
||||
context deadline exceeded` at exactly 30s. Commands with `sleep 30`, `qm
|
||||
shutdown`+wait, or async poll loops always hit this. The agent retries with
|
||||
longer sleeps and hits the same wall.
|
||||
|
||||
**Root cause:** `cmd/nomos` MCP client uses a 30s timeout; the `run` tool
|
||||
blocks synchronously waiting for command completion. No async path exists for
|
||||
long-running commands.
|
||||
|
||||
### F2 — Premature success: `complete_task` fires before the goal is actually met (P0)
|
||||
|
||||
Session 268895a5 called `complete_task(success)` at 18:29 with summary "zimaos
|
||||
returns 200." The endpoint was serving ttyd (terminal), not the ZimaOS
|
||||
dashboard. The agent conflated "HTTPS 200" with "dashboard works." The user had
|
||||
to resume the session.
|
||||
|
||||
**Root cause:** No pre-completion validation. The agent can mark success with a
|
||||
summary that doesn't match reality. `complete_task` is a write-and-forget
|
||||
operation with no state check.
|
||||
|
||||
### F3 — Wrong target: `run` doesn't validate the target can execute the command (P0)
|
||||
|
||||
In the ZimaOS continuation, `qm stop 100 --skiplock` was executed on
|
||||
`lxc:dns`. The command failed (`qm: command not found`) because the agent
|
||||
copied the command from a previous run but forgot to change the target.
|
||||
Similarly, `cmd/nomos` tried `docker exec` on `host:hubris` (no docker).
|
||||
|
||||
**Root cause:** `run(target, command)` in `internal/mcp/server.go` doesn't
|
||||
validate that the target type can execute the given command. A simple
|
||||
allowlist would catch `qm`/`pct`/`pvesh` on non-host targets.
|
||||
|
||||
### F4 — Plan system is decorative: 62% of steps never reach `done` (P1)
|
||||
|
||||
Across 5 sessions: 39 plan steps. 24 (62%) were `replaced`, 15 (38%) reached
|
||||
`done`. Session d458a5f8 had 3 complete plan regenerations with **zero**
|
||||
completed steps. The agent replaces plans instead of completing or explicitly
|
||||
skipping steps.
|
||||
|
||||
**Root cause:** Plan steps carry status (`pending`/`running`/`done`/`failed`/
|
||||
`skipped`/`blocked`/`replaced`) but `replaced` has no `replaced_reason`
|
||||
field. The model can silently replace every step and the system doesn't flag
|
||||
it. No constraint ties `propose_plan` to existing plan state.
|
||||
|
||||
### F5 — No plan on session resume: continuation sessions run ad-hoc (P1)
|
||||
|
||||
The ZimaOS continuation (18:29→19:01) had 81 calls with **zero**
|
||||
`propose_plan` calls. The agent ran ad-hoc tool calls with no structure.
|
||||
|
||||
**Root cause:** When a session resumes, the `must_have_plan` guard is already
|
||||
satisfied by the old (completed) plan. The agent doesn't re-plan on resume.
|
||||
|
||||
### F6 — Scope expansion / rabbit holes: agent chases irrelevant sub-goals (P2)
|
||||
|
||||
In the ZimaOS continuation, the agent spent ~40 calls trying to fix Technitium
|
||||
DHCP reservations — a completely different subsystem from the goal ("make the
|
||||
dashboard reachable"). The ZimaOS dashboard hadn't started since July 19
|
||||
(3-week-old issue), making the DHCP reservation effort moot. The agent never
|
||||
surfaced a question like "This is pre-existing — should I still fix DHCP?"
|
||||
|
||||
**Root cause:** No scope gate. When the agent pivots to a subsystem unrelated
|
||||
to the stated goal, nothing stops it. The `session_questions` mechanism exists
|
||||
(2 calls in 193 sessions) but the model never uses it.
|
||||
|
||||
### F7 — Command generation errors: malformed bash from the LLM (P2)
|
||||
|
||||
In the ZimaOS continuation, the agent generated:
|
||||
- `head - n` instead of `head -n` → bash syntax error
|
||||
- `echo "---" && \n curl ...` → literal `\n` in command → ambiguous redirect
|
||||
|
||||
**Root cause:** The LLM generates bash commands inline in content blocks. No
|
||||
syntax validation, no escape-character handling. The `run` tool should reject
|
||||
malformed commands before execution.
|
||||
|
||||
### F8 — Learning pipeline completely empty (P4)
|
||||
|
||||
| Table | Rows |
|
||||
|-------|------|
|
||||
| `classifications` | **0** |
|
||||
| `feedback` | **0** |
|
||||
| `patterns` | **0** |
|
||||
| `skills` | **0** |
|
||||
|
||||
Despite 1,884 executions and 263 approvals, the system learns nothing from
|
||||
outcomes. The ZimaOS session discovered that the dashboard hadn't started since
|
||||
July 19 — this was never persisted. The HMAC trailing-newline discovery was
|
||||
persisted manually; if the agent forgot `upsert_knowledge`, it would be lost.
|
||||
|
||||
### F9 — Observability gaps (P3)
|
||||
|
||||
| Gap | Detail |
|
||||
|-----|--------|
|
||||
| Token tracking | `agent_activity.token_count` is NULL for every row |
|
||||
| Execution linkage | `nomos_plan_executions` is empty; `audit_log.session_id` is null |
|
||||
| Plan quality | No metric for step completion rate (currently 38%) |
|
||||
| MCP timeout rate | No counter for `run` calls that hit the client timeout |
|
||||
|
||||
### F10 — Execution success rate is 74% (P2)
|
||||
|
||||
1,884 executions: 1,400 completed (74%), 295 failed (15.6%), 152 cancelled
|
||||
(8%), 37 denied (2%). One in four execution attempts doesn't complete.
|
||||
|
||||
---
|
||||
|
||||
## 3. Improvement plan (ordered by impact/effort)
|
||||
|
||||
### Task 1 — Prevent premature `complete_task(success)` *(fixes F2)*
|
||||
|
||||
**`internal/mcp/tools.go`** — In the `complete_task` handler, when
|
||||
`outcome=success`: require the summary field to contain a verifiable state
|
||||
assertion. Minimum: if the goal mentions a URL, check that the summary doesn't
|
||||
contradict known state. Lightweight: log a warning if the summary says "returns
|
||||
200" but the last `ping_service` or `run` result says otherwise.
|
||||
|
||||
**Coach:** `nomos/SOUL.md` — explicit rule: *"Before calling
|
||||
complete_task(success), restate the user's original goal in your own words and
|
||||
verify each condition. If any condition is 'probably works' rather than
|
||||
'verified,' ask the operator or set outcome=partial."*
|
||||
|
||||
### Task 2 — Target validation in `run` *(fixes F3)*
|
||||
|
||||
**`internal/mcp/server.go`** — In the `run` handler, before dispatching:
|
||||
validate that the command prefix matches the target type.
|
||||
|
||||
```
|
||||
pct/qm/pvesh/iptables → only host:* targets
|
||||
systemctl/docker → host:* or lxc:* targets
|
||||
curl/nmap/ss → any target
|
||||
```
|
||||
|
||||
If mismatched, return a clear error: *"Cannot run `qm` on lxc:dns — `qm` is a
|
||||
Proxmox host command. Use target host:hubris or host:strong."* Do not classify
|
||||
or execute.
|
||||
|
||||
**Test:** `TestClassifyCommand_WrongTarget` → commands with host-only prefixes
|
||||
on LXC targets return error without execution.
|
||||
|
||||
### Task 3 — Raise MCP client timeout; add async path for long-running commands *(fixes F1)*
|
||||
|
||||
**`cmd/nomos`** — Raise the MCP client timeout from 30s to 120s.
|
||||
|
||||
**`internal/mcp/server.go`** — For `run` commands that the classifier
|
||||
determines will exceed the client timeout (presence of `sleep`, `wait`,
|
||||
`timeout` in the command), return immediately with an `execution_id` and status
|
||||
`running`. The agent already has `get_execution_status` — use it:
|
||||
|
||||
1. Classify the command; if it contains `sleep`, `wait`, or shell constructs
|
||||
that imply polling, flag it as `async_potential`.
|
||||
2. Start the command, return the `execution_id` immediately.
|
||||
3. Agent polls with `get_execution_status(execution_id)`.
|
||||
4. If the client timeout is hit mid-poll, the execution continues on the server
|
||||
— it's not lost.
|
||||
|
||||
**Test:** `run` with `sleep 60; echo done` on host:hubris → returns
|
||||
immediately (not 30s timeout), `get_execution_status` eventually returns
|
||||
`completed`.
|
||||
|
||||
### Task 4 — Plan step integrity: require `replaced_reason` on replacement *(fixes F4)*
|
||||
|
||||
**`session_plan_steps` migration** — Add `replaced_reason TEXT` column.
|
||||
|
||||
**`cmd/nomos`** — When the agent emits `update_plan_step` with
|
||||
`status=replaced`, require a non-empty `replaced_reason`. Valid reasons:
|
||||
`wrong_diagnosis`, `scope_change`, `blocked`, `superseded`, `operator_override`.
|
||||
|
||||
**Coach:** `nomos/SOUL.md` — explicit rule: *"Complete (status=done) or
|
||||
explicitly skip (status=skipped) steps. Use status=replaced only when the
|
||||
entire plan generation is wrong; include the reason. Replacing all steps with
|
||||
no reason is a session-quality violation."*
|
||||
|
||||
### Task 5 — Force `propose_plan` on session resume *(fixes F5)*
|
||||
|
||||
**`cmd/nomos`** — When a session with status `done` or `failed` receives a new
|
||||
user message, reset the plan state: clear step status, require a new
|
||||
`propose_plan` call before any `run` calls. The "must have plan" guard should
|
||||
consider the resumed session as plan-less until a fresh `propose_plan` is
|
||||
called.
|
||||
|
||||
**Guard:** `set_goal` + `propose_plan` must be called before any `run` in a
|
||||
resumed session. Reuse the existing "No plan — call set_goal then propose_plan"
|
||||
error from `internal/mcp/server.go`.
|
||||
|
||||
### Task 6 — Scope gate: surface `session_questions` on context switch *(fixes F6)*
|
||||
|
||||
**Coach:** `nomos/SOUL.md` — explicit rule: *"Before pivoting to a subsystem
|
||||
not mentioned in the user's goal, ask via session_questions. Example: 'The
|
||||
dashboard logs show it hasn't started since July 19. Do you want me to debug
|
||||
the dashboard service itself [A], skip it and just stabilize the IP [B], or
|
||||
stop here [C]?'"*
|
||||
|
||||
**`cmd/nomos` prompt** — Add to the system prompt: *"When the investigation
|
||||
leads to a subsystem or root cause unrelated to the expressed goal, surface a
|
||||
session_question before taking action."*
|
||||
|
||||
### Task 7 — Auto-upsert knowledge on session close *(fixes F8)*
|
||||
|
||||
**`cmd/nomos`** — On `complete_task` (any outcome: success, partial, failure),
|
||||
auto-generate a knowledge entry:
|
||||
|
||||
```
|
||||
title: "<date>: <goal summary>"
|
||||
content: "## Outcome\n<outcome>\n## Root cause\n<extracted>\n## What was done\n<summary>\n## What was left\n<unresolved>"
|
||||
tags: [session:<id>]
|
||||
about: [entities involved]
|
||||
```
|
||||
|
||||
This ensures every session leaves a trace regardless of whether the agent
|
||||
remembered to call `upsert_knowledge`.
|
||||
|
||||
### Task 8 — Token tracking *(fixes F9)*
|
||||
|
||||
**`cmd/nomos`** — After each LLM call, extract `usage.prompt_tokens`,
|
||||
`usage.completion_tokens`, `usage.total_tokens` from the response and write to
|
||||
`agent_activity.token_count`. Currently the field exists but is never populated
|
||||
(NULL for all rows).
|
||||
|
||||
### Task 9 — Execution linkage *(fixes F9)*
|
||||
|
||||
**`internal/mcp/server.go`** — When `run` creates an execution, write a row
|
||||
into `nomos_plan_executions` linking `session_id`, `plan_step_seq`, and
|
||||
`execution_id`.
|
||||
|
||||
**`internal/mcp/server.go`** — Pass `session_id` (from MCP request headers)
|
||||
into `audit_log` writes. Currently `audit_log.session_id` is NULL — the
|
||||
`createAuditLog` function in `internal/httpapi/impl.go` receives the
|
||||
correlation_id but not the session_id from the MCP path.
|
||||
|
||||
### Task 10 — Plan quality metric *(fixes F9)*
|
||||
|
||||
**`cmd/nomos`** — At session close, compute: `completed_steps /
|
||||
total_steps_per_plan` (currently ~38%). Log as a metric or write as a session
|
||||
attribute. Track over time to measure plan-adherence improvements from Tasks
|
||||
4+5.
|
||||
|
||||
### Task 11 — Auto-classify every `run` call *(fixes F8)*
|
||||
|
||||
**`internal/mcp/server.go`** — The `run` handler already calls the classifier
|
||||
(`classifyCommand` in `internal/policy/command.go`) to determine risk_class and
|
||||
approval route. Write the result to the `classifications` table. Currently the
|
||||
table is empty (0 rows) despite 1,884 executions being classified.
|
||||
|
||||
### Task 12 — Auto-feedback on session close *(fixes F8)*
|
||||
|
||||
**`cmd/nomos`** — On `complete_task`, generate a `feedback` entry:
|
||||
|
||||
```
|
||||
session_id: <id>
|
||||
outcome: <outcome>
|
||||
observation: <summary>
|
||||
lesson: <extracted from complete_task.summary>
|
||||
side_effects: <entities created/modified during session>
|
||||
```
|
||||
|
||||
**`cmd/oikos`** — Add a daily cron or scheduler job that reads recent
|
||||
`feedback` entries and extracts `patterns` (recurring root causes, same-fix
|
||||
applied multiple times, known-broken services). Seed the pattern table.
|
||||
|
||||
### Task 13 — Command syntax validation in `run` *(fixes F7)*
|
||||
|
||||
**`internal/mcp/server.go`** — Before executing a `run` command, do
|
||||
lightweight bash syntax validation:
|
||||
|
||||
```
|
||||
- Reject literal \n in commands (should be ; or &&)
|
||||
- Reject commands where the last line ends with \ (backslash-continuation)
|
||||
but no next line
|
||||
- Warn on common typos: "head - n", "grep - i", spaces before flags
|
||||
- Reject `&& \n` patterns (the LLM sometimes inserts literal \n between && chains)
|
||||
```
|
||||
|
||||
### Task 14 — Stuck-session reaping (from prior plan; re-confirmed)
|
||||
|
||||
This session exhibited the same idle zombie pattern (23da10db — no closed_at,
|
||||
status `failed` but outcome `failure`). Task 5 from the 2026-08-03 plan is
|
||||
still open. Copying here for completeness.
|
||||
|
||||
---
|
||||
|
||||
## 4. Recommended sequence
|
||||
|
||||
```
|
||||
P0 (blocks operational waste):
|
||||
1 → 2 → 3
|
||||
|
||||
P1 (fixes plan architecture):
|
||||
4 → 5
|
||||
|
||||
P2 (cognitive guardrails):
|
||||
6 → 7 → 13
|
||||
|
||||
P3 (observability):
|
||||
8 → 9 → 10
|
||||
|
||||
P4 (learning loop):
|
||||
11 → 12
|
||||
```
|
||||
|
||||
Sequence rationale: Tasks 1-3 stop the worst outcomes (premature success,
|
||||
wrong-target execution, MCP timeouts). Tasks 4-5 make the plan system actually
|
||||
useful instead of decorative. Tasks 6-7 add guardrails that prevent the ZimaOS
|
||||
rabbit-hole class of failure. Tasks 8-10 give us visibility into whether any of
|
||||
the previous tasks are working. Tasks 11-12 close the learning loop.
|
||||
|
||||
---
|
||||
|
||||
## 5. Validation
|
||||
|
||||
| Task | Test |
|
||||
|------|------|
|
||||
| 1 | Session with goal "make X reachable" where last ping shows 502 → `complete_task(success)` is rejected or warns |
|
||||
| 2 | `run("lxc:dns", "qm stop 100")` → error: "qm is a Proxmox host command" |
|
||||
| 3 | `run` with `sleep 45; echo done` → returns execution_id immediately, `get_execution_status` shows final result |
|
||||
| 4 | `update_plan_step(status=replaced)` with no reason → rejected; with reason → accepted |
|
||||
| 5 | Resumed session calls `run` before `propose_plan` → blocked: "No plan — call propose_plan" |
|
||||
| 6 | Agent pivots to unrelated subsystem → `session_questions` is called before action |
|
||||
| 7 | `complete_task` → knowledge entry created automatically with session link |
|
||||
| 8 | `agent_activity.token_count` is non-NULL after any LLM call |
|
||||
| 9 | `nomos_plan_executions` has rows linking session + step + execution |
|
||||
| 10 | Session close writes `plan_adherence` attribute (step-completion %) |
|
||||
| 11 | `classifications` table has 1 row per `run` call with risk_class + route |
|
||||
| 12 | `complete_task` → auto `feedback` entry; daily pattern job finds recurring issues |
|
||||
| 13 | `run` with `head - n /etc/hosts` → rejected with clear error about malformed command |
|
||||
|
||||
---
|
||||
|
||||
## 6. Out of scope / open questions
|
||||
|
||||
- Whether to raise `auto_act` from `off` for `reversible_low` actions (separate
|
||||
policy decision; would reduce approval pileup without code changes).
|
||||
- Whether to add a `delete_entity` MCP tool for lifecycle management (separate
|
||||
from this reliability plan).
|
||||
- The exact TTL for stuck-session reaping (30 min recommended, confirmed in
|
||||
2026-08-03 plan).
|
||||
- Whether `run` async mode should be opt-in (command contains sleep/wait) or
|
||||
universal (every run returns immediately, agent always polls). Recommend
|
||||
opt-in for now — most commands complete in <5s.
|
||||
Reference in New Issue
Block a user